diff --git a/desktop/src/features/agents/ui/AgentEditDialog.tsx b/desktop/src/features/agents/ui/AgentEditDialog.tsx new file mode 100644 index 0000000000..9d989423f5 --- /dev/null +++ b/desktop/src/features/agents/ui/AgentEditDialog.tsx @@ -0,0 +1,334 @@ +/** + * AgentEditDialog.tsx — Merged agent edit surface (Phase 1). + * + * Collapses R1–R7 from the Phase 0 spec Artifact 2 onto a single dialog. + * R8 (duplicate = create seed) and R9 (start-on-launch quick toggle) are + * kept as separate paths and are not affected here. + * + * Architecture: + * For agents backed by a definition: opens AgentDefinitionDialog in + * edit mode, but intercepts onSubmit to run the Artifact 3 coordinator, + * which also writes any I-field diff (device policy setters: auto-restart, + * start-on-launch). + * + * For unlinked agents: delegates to AgentDefinitionDialog with the agent's + * own fields, routed to updateManagedAgent by the coordinator (personaInput + * will be null; agentInput carries the diff). + * + * The coordinator implements: + * 0. Validate (linked runtime availability, credential gate) + * 1. Definition write (D-fields, only when changed and definition is editable) + * 2. Instance write (I-fields, only when changed) + * 3. Policy setters (auto-restart, start-on-launch, only on change) + * 4. Settlement (re-fetch both stores, toast from observed state) + * + * S5 "Instances" vocabulary rename is deferred to Phase 3. Built-in agents + * are fully editable (Artifact 1 corrected matrix). + */ + +import * as React from "react"; +import { toast } from "sonner"; +import { useQueryClient } from "@tanstack/react-query"; + +import { + managedAgentsQueryKey, + personasQueryKey, + useAcpRuntimesQuery, + useSetManagedAgentAutoRestartMutation, + useSetManagedAgentStartOnAppLaunchMutation, + useUpdateManagedAgentMutation, + useUpdatePersonaMutation, + useStartManagedAgentMutation, +} from "@/features/agents/hooks"; +import { useUpdatePersonaAndPublishMutation } from "@/features/agents/lib/usePersonaCatalogRelay"; +import { runAgentSaveCoordinator } from "./agentSaveCoordinator"; +import type { AgentEditContext } from "./agentFormModel"; +export type { AgentEditContext }; +import type { + AgentPersona, + CreatePersonaInput, + ManagedAgent, + UpdatePersonaInput, +} from "@/shared/api/types"; +import type { EditAgentFocusTarget } from "@/features/agents/openEditAgentEvent"; +import type { AgentDefinitionSubmitOptions } from "./AgentDefinitionDialog"; +import { AgentDefinitionDialog } from "./AgentDefinitionDialog"; +import { editPersonaDialogState } from "./personaDialogState"; + +// ── Types ───────────────────────────────────────────────────────────────────── + +export type AgentEditDialogProps = { + open: boolean; + onOpenChange: (open: boolean) => void; + /** + * Edit context: which entity or entities to edit. + */ + ctx: AgentEditContext; + /** + * Optional field to focus when the dialog opens from a card deep-link. + */ + initialFocus?: EditAgentFocusTarget; + onUpdated?: (agent: ManagedAgent) => void; + /** + * Optional pre-save validator (R6 origin permission check). + * Called before the coordinator's step 0. Return a non-null string to abort + * with an error toast; return null to proceed. + */ + onValidate?: () => string | null; + /** + * Optional initial-values override for the definition fields. When provided, + * these values are merged into the seeded form (used in R6 review mode to + * pre-fill agent-requested changes for user approval). + */ + initialValueOverrides?: Partial<{ + displayName: string; + systemPrompt: string; + runtime: string | undefined; + provider: string | undefined; + model: string | undefined; + }>; +}; + +// ── Component ───────────────────────────────────────────────────────────────── + +export function AgentEditDialog({ + ctx, + open, + onOpenChange, + onUpdated, + onValidate, + initialValueOverrides, +}: AgentEditDialogProps) { + const queryClient = useQueryClient(); + const updatePersonaMutation = useUpdatePersonaMutation(); + const updateManagedAgentMutation = useUpdateManagedAgentMutation(); + const setAutoRestartMutation = useSetManagedAgentAutoRestartMutation(); + const setStartOnAppLaunchMutation = + useSetManagedAgentStartOnAppLaunchMutation(); + const startMutation = useStartManagedAgentMutation(); + const runtimesQuery = useAcpRuntimesQuery({ enabled: open }); + + // Community id is required for publish-aware mutations; null → non-catalog edit. + const updatePersonaAndPublishMutation = + useUpdatePersonaAndPublishMutation(null); + + const [isSaving, setIsSaving] = React.useState(false); + const [saveError, setSaveError] = React.useState(null); + + // Reset error on open + React.useEffect(() => { + if (open) setSaveError(null); + }, [open]); + + const def = ctx.kind !== "instance-only" ? ctx.definition : null; + const inst = ctx.kind !== "definition-only" ? ctx.instance : null; + const runtimes = runtimesQuery.data ?? []; + const runtimeCatalogStatus = runtimesQuery.isLoading + ? ("loading" as const) + : runtimesQuery.isError + ? ("error" as const) + : ("ready" as const); + + // ── Settlement helper ────────────────────────────────────────────────────── + async function refetchStores(): Promise<{ + persona: AgentPersona | null; + agent: ManagedAgent | null; + }> { + await Promise.all([ + queryClient.invalidateQueries({ queryKey: personasQueryKey }), + queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }), + ]); + const personas = + queryClient.getQueryData(personasQueryKey) ?? []; + const agents = + queryClient.getQueryData(managedAgentsQueryKey) ?? []; + return { + persona: def ? (personas.find((p) => p.id === def.id) ?? null) : null, + agent: inst + ? (agents.find((a) => a.pubkey === inst.pubkey) ?? null) + : null, + }; + } + + // ── onSubmit ── called by AgentDefinitionDialog when the user clicks Save ─ + // + // `input` is the UpdatePersonaInput computed by AgentDefinitionDialog. + // For instance-with-definition contexts, the coordinator: + // 1. Writes the definition (personaInput = input) + // 2. Writes the instance diff (agentInput = null — D-field changes propagate + // live per the matrix; row 1/8 materializations are dropped) + // 3. Runs policy setters (auto-restart/start-on-launch if changed) + // + // For instance-only contexts, the definition dialog's input represents the + // instance state, so the coordinator routes it to agentInput instead. + async function handleSubmit( + input: CreatePersonaInput | UpdatePersonaInput, + options: AgentDefinitionSubmitOptions, + ): Promise { + // AgentEditDialog is always in edit mode — the input will always be an + // UpdatePersonaInput. If for any reason a CreatePersonaInput arrives + // (e.g. dialog misconfiguration), bail out safely. + if (!("id" in input)) { + console.error( + "[AgentEditDialog] Received CreatePersonaInput in edit mode — ignoring.", + ); + return undefined; + } + const updateInput: UpdatePersonaInput = input; + + // Pre-save validation (e.g. R6 origin-permission check). + if (onValidate) { + const validationError = onValidate(); + if (validationError) { + toast.error(validationError); + return undefined; + } + } + + setSaveError(null); + setIsSaving(true); + + try { + // Build coordinator inputs + const personaInput: UpdatePersonaInput | null = + def !== null ? updateInput : null; + + // Compute I-field policy diff from current instance state. + // The dialog does not expose per-instance auto-restart/start-on-launch + // from AgentDefinitionDialog state directly — those are L-fields handled + // via dedicated setters. For Phase 1, policy setters are triggered only + // if the instance has changed these values (which means they must be + // surfaced elsewhere — Phase 1 defers in-form policy toggles to Phase 2; + // the setter mechanism is wired and ready here). No policySets for now. + const policySets: Parameters< + typeof runAgentSaveCoordinator + >[0]["policySets"] = []; + + const success = await runAgentSaveCoordinator({ + ctx, + personaInput, + agentInput: null, + policySets, + publishCatalogUpdates: options.publishCatalogUpdates, + runtimes: runtimes.length > 0 ? runtimes : undefined, + updatePersona: (p) => updatePersonaMutation.mutateAsync(p), + updatePersonaAndPublish: (p) => + updatePersonaAndPublishMutation.mutateAsync(p), + updateManagedAgent: (a) => updateManagedAgentMutation.mutateAsync(a), + setAutoRestart: (pubkey, value) => + setAutoRestartMutation.mutateAsync({ + pubkey, + autoRestartOnConfigChange: value, + }), + setStartOnAppLaunch: (pubkey, value) => + setStartOnAppLaunchMutation.mutateAsync({ + pubkey, + startOnAppLaunch: value, + }), + refetchStores, + onDone: () => onOpenChange(false), + onSavedWhileStopped: (agent) => { + const savedName = agent.name; + toast(`${savedName} saved while stopped.`, { + action: { + label: "Start now", + onClick: () => { + startMutation.mutate(agent.pubkey, { + onSuccess: () => toast.success(`${savedName} started.`), + onError: (err) => + toast.error( + err instanceof Error + ? `${savedName} failed to start: ${err.message}` + : `${savedName} failed to start.`, + ), + }); + }, + }, + }); + }, + }); + + if (success) { + const agents = + queryClient.getQueryData(managedAgentsQueryKey) ?? []; + const updated = inst + ? (agents.find((a) => a.pubkey === inst.pubkey) ?? undefined) + : undefined; + if (updated) onUpdated?.(updated); + } else { + setSaveError( + new Error("Some changes may not have persisted. Reopen to retry."), + ); + } + } finally { + setIsSaving(false); + } + + return undefined; + } + + // ── Build initial values for AgentDefinitionDialog ───────────────────────── + // + // Always seed from the definition when one is present — that is the + // authoritative source for D-fields. editPersonaDialogState() handles the + // round-trip of namePool/envVars/behavior that prevents accidental clears. + // initialValueOverrides are applied on top for R6 review mode (agent-requested + // field changes pre-filled for user approval). + const baseDialogState = def ? editPersonaDialogState(def) : null; + const dialogState = + baseDialogState && initialValueOverrides + ? { + ...baseDialogState, + initialValues: { + ...baseDialogState.initialValues, + ...initialValueOverrides, + }, + } + : baseDialogState; + const initialValues = + dialogState?.initialValues ?? + (inst + ? { + // Instance-only fallback: no definition present, expose instance + // fields so the user can edit them. + displayName: inst.name, + avatarUrl: inst.avatarUrl ?? "", + systemPrompt: inst.systemPrompt ?? "", + runtime: undefined, + model: inst.model ?? undefined, + provider: inst.provider ?? undefined, + envVars: inst.envVars ?? {}, + behavior: + inst.respondTo != null + ? { + respondTo: inst.respondTo, + respondToAllowlist: + inst.respondTo === "allowlist" + ? inst.respondToAllowlist + : undefined, + parallelism: inst.parallelism ?? undefined, + } + : undefined, + } + : null); + + const title = + dialogState?.title ?? (inst ? `Edit ${inst.name}` : "Edit agent"); + + return ( + + ); +} diff --git a/desktop/src/features/agents/ui/AgentManagementDialogs.tsx b/desktop/src/features/agents/ui/AgentManagementDialogs.tsx index 27541889f2..4605aa607a 100644 --- a/desktop/src/features/agents/ui/AgentManagementDialogs.tsx +++ b/desktop/src/features/agents/ui/AgentManagementDialogs.tsx @@ -1,11 +1,33 @@ import { useAgentManagement } from "@/features/agents/useAgentManagement"; import { AgentCardDialogs } from "./AgentCardViewerDialog"; import { AgentDialog } from "./AgentDialog"; +import { AgentEditDialog } from "./AgentEditDialog"; +import { toast } from "sonner"; +import * as React from "react"; /** Global review surfaces opened by owned agents through the Buzz harness. */ export function AgentManagementDialogs() { const management = useAgentManagement(); + // R6 error handling: when an update request arrives but the persona cannot + // be found or is ambiguous, toast the error and dismiss. This avoids showing + // a definition-edit dialog for an unresolvable update. + React.useEffect(() => { + if ( + management.request?.action === "update" && + !management.currentPersona && + management.editError + ) { + toast.error(management.editError); + management.dismiss(); + } + }, [ + management.request?.action, + management.currentPersona, + management.editError, + management.dismiss, + ]); + return ( <> {management.request?.action === "create" ? ( @@ -24,22 +46,20 @@ export function AgentManagementDialogs() { runtimeCatalogStatus={management.runtimeCatalogStatus} /> ) : null} - {management.request?.action === "update" ? ( - { if (!open) management.dismiss(); }} - onSubmit={management.submitUpdate} - open - runtimes={management.runtimes} - runtimeCatalogStatus={management.runtimeCatalogStatus} - submitLabel="Save changes" - title="Edit agent" + onValidate={() => management.verifyOriginPermission()} + initialValueOverrides={management.reviewOverrides} /> ) : null} diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index e1e1f37f35..fff8494d50 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -8,6 +8,7 @@ import { AddAgentToChannelDialog } from "./AddAgentToChannelDialog"; import { AddTeamToChannelDialog } from "./AddTeamToChannelDialog"; import { AgentDefaultsDialog } from "./AgentDefaultsDialog"; import { AgentDialog } from "./AgentDialog"; +import { AgentEditDialog } from "./AgentEditDialog"; import { PersonaCatalogDialog } from "./PersonaCatalogDialog"; import { PersonaDeleteDialog } from "./PersonaDeleteDialog"; import { PersonaShareDialog } from "./PersonaShareDialog"; @@ -313,6 +314,16 @@ export function AgentsView() { open={agents.agentToAddToChannel !== null} /> ) : null} + {/* R5: edit route collapses to merged surface (definition-only context) */} + {personas.personaToEdit ? ( + { + if (!open) personas.setPersonaToEdit(null); + }} + /> + ) : null} {personas.personaDialogState ? ( { + const definition = makeDefinition({ + displayName: "Alice", + namePool: ["pool-instance-1", "pool-instance-2"], + }); + const instance = makeInstance({ + // Pool-minted instance has a different name from the definition displayName. + name: "pool-instance-1", + }); + + const ctx = { kind: "instance-with-definition", definition, instance }; + const saved = seedAgentFormModel(ctx); + + // User edits displayName on the definition (renames the definition). + const next = { ...saved, displayName: "Alice (renamed)" }; + + const emit = emitAgentFormDiff(saved, next, ctx); + + // D-field diff: displayName changed → personaInput should carry the new name. + assert.ok( + emit.personaInput, + "definition should be updated when displayName changes", + ); + assert.equal(emit.personaInput.displayName, "Alice (renamed)"); + + // I-field diff: instance name must NOT be emitted — row 1 contract prevents + // materializing displayName→name when a name pool exists. + assert.equal( + emit.agentInput, + null, + "instance name must not be materialized when the definition has a name pool", + ); +}); + +test("test_pooled_name_instance_name_not_materialized_even_when_names_differ", () => { + // The pool-instance name diverges from the definition's displayName. + // Editing the definition displayName must still not touch the instance name. + const definition = makeDefinition({ + displayName: "Bot", + namePool: ["Scout", "Ranger"], + }); + const instance = makeInstance({ name: "Scout" }); + + const ctx = { kind: "instance-with-definition", definition, instance }; + const saved = seedAgentFormModel(ctx); + + // Edit the definition displayName only. + const next = { ...saved, displayName: "Bot (v2)" }; + const emit = emitAgentFormDiff(saved, next, ctx); + + assert.ok(emit.personaInput, "definition should be updated"); + assert.equal( + emit.agentInput, + null, + "instance update must not be emitted for a pool-named agent", + ); +}); + +test("test_no_name_pool_explicit_instance_name_change_emits_agentInput", () => { + // Without a name pool, explicitly editing the instanceName field should + // produce an agentInput with the new name. + const definition = makeDefinition({ + displayName: "Solo", + namePool: [], + }); + const instance = makeInstance({ name: "Solo" }); + + const ctx = { kind: "instance-with-definition", definition, instance }; + const saved = seedAgentFormModel(ctx); + + // User explicitly changes the instance name (e.g. via an instanceName field). + const next = { ...saved, instanceName: "Solo (renamed)" }; + const emit = emitAgentFormDiff(saved, next, ctx); + + // No D-field change, so no definition update expected. + assert.equal( + emit.personaInput, + null, + "no definition update for instanceName-only change", + ); + // I-field: instanceName changed → agentInput should carry the new name. + assert.ok( + emit.agentInput, + "instance update should emit when instanceName changes", + ); + assert.equal(emit.agentInput.name, "Solo (renamed)"); +}); + +// ── Regression test 2: instance env wholesale-replace drop ──────────────────── +// +// Spec row 8 contract: instance env edits go to the instance, NOT via a +// wholesale-replace from the definition. When the user edits a D-field +// (unrelated to env), the coordinator must NOT emit an instance envVars diff +// that would clobber per-instance env overrides. + +test("test_env_clobber_dropped_when_user_edits_unrelated_d_field", () => { + const definition = makeDefinition({ + envVars: { ANTHROPIC_API_KEY: "sk-def" }, + }); + // Instance has a per-instance env override (different from definition). + const instance = makeInstance({ + envVars: { ANTHROPIC_API_KEY: "sk-instance-override" }, + }); + + const ctx = { kind: "instance-with-definition", definition, instance }; + const saved = seedAgentFormModel(ctx); + + // User only edits the definition displayName — does NOT touch env vars. + const next = { ...saved, displayName: "Alice (renamed)" }; + const emit = emitAgentFormDiff(saved, next, ctx); + + // D-field diff: displayName changed. + assert.ok( + emit.personaInput, + "definition should be updated when displayName changes", + ); + + // I-field: no instance env diff should be emitted (row 8 contract). + // The "instanceEnvVars" mechanism is the only way to emit instance env; + // the standard envVars field on the model represents the definition's env. + // Since the user didn't touch instance env, agentInput must be null (or + // must not contain envVars if it exists for another reason). + if (emit.agentInput !== null) { + assert.equal( + emit.agentInput.envVars, + undefined, + "instance envVars must not be emitted when the user didn't edit instance env", + ); + } + // More precisely, for a definition-backed agent where only a D-field changed, + // no instance write should be needed at all. + assert.equal( + emit.agentInput, + null, + "no instance write should occur when only a D-field changed (row 8 clobber-drop)", + ); +}); + +test("test_env_clobber_dropped_even_when_definition_env_differs_from_instance_env", () => { + // Ensure the drop also holds when definition and instance envs differ: + // the coordinator must never blindly push definition envVars onto the instance. + const definition = makeDefinition({ + envVars: { KEY: "from-definition" }, + }); + const instance = makeInstance({ + envVars: { KEY: "from-instance" }, + }); + + const ctx = { kind: "instance-with-definition", definition, instance }; + const saved = seedAgentFormModel(ctx); + + // User edits system prompt only (D-field, no env change). + const next = { ...saved, systemPrompt: "Updated prompt." }; + const emit = emitAgentFormDiff(saved, next, ctx); + + assert.ok(emit.personaInput, "definition should be updated"); + assert.equal(emit.agentInput, null, "instance env must not be clobbered"); +}); diff --git a/desktop/src/features/agents/ui/agentFormModel.ts b/desktop/src/features/agents/ui/agentFormModel.ts new file mode 100644 index 0000000000..f030637ce3 --- /dev/null +++ b/desktop/src/features/agents/ui/agentFormModel.ts @@ -0,0 +1,364 @@ +/** + * agentFormModel.ts — Canonical field model for the merged agent edit surface. + * + * One typed AgentFormModel is consumed by every edit route (Phase 1). The + * create path extends the same model in Phase 2. + * + * Sections per Artifact 4: + * Identity — name, avatar + * Behavior — prompt, respond-to+allowlist, parallelism + * Runtime — harness, model, provider, env vars, tuning + * Device policy — auto-restart, start-on-launch (local backend only) + * Definition admin— name pool, share action, and definition-level actions + * + * Field ownership tags per Artifact 1: + * definition — D-field: shared across linked siblings via definition + * instance — I-field: this agent only + * local-policy — L-field: dedicated setter, never on the relay + * create-only — C-field: expressible at create time only + * + * Editability predicate: + * team-managed D-fields → read-only (D-fields render read-only) + * built-in → fully editable D-fields (only actions differ, not field editability) + * unlinked agent → definition sections absent + * zero-instance definition → instance + policy sections absent + * identity-archived agent → fully editable, Save never unarchives + */ + +import type { + AgentPersona, + ManagedAgent, + UpdatePersonaInput, + UpdateManagedAgentInput, +} from "@/shared/api/types"; + +// ── Field ownership ───────────────────────────────────────────────────────── + +export type FieldOwner = "definition" | "instance" | "local-policy"; + +// ── Context shape that drives editability ─────────────────────────────────── + +/** + * Describes which combination of definition / instance is being edited. + * + * - definition-only: zero-instance definition (from agents library) + * - instance-with-definition: linked agent (most common case, R1–R7) + * - instance-only: unlinked agent (no definition) + */ +export type AgentEditContext = + | { kind: "definition-only"; definition: AgentPersona } + | { + kind: "instance-with-definition"; + definition: AgentPersona; + instance: ManagedAgent; + } + | { kind: "instance-only"; instance: ManagedAgent }; + +export function editContextDefinition( + ctx: AgentEditContext, +): AgentPersona | null { + if (ctx.kind === "instance-only") return null; + return ctx.definition; +} + +export function editContextInstance( + ctx: AgentEditContext, +): ManagedAgent | null { + if (ctx.kind === "definition-only") return null; + return ctx.instance; +} + +/** True when D-fields should render read-only (team-managed). */ +export function isDefinitionReadOnly(ctx: AgentEditContext): boolean { + const def = editContextDefinition(ctx); + return def?.sourceTeam != null && def.sourceTeam.length > 0; +} + +/** True when definition sections should be rendered at all. */ +export function hasDefinitionContext(ctx: AgentEditContext): boolean { + return ctx.kind !== "instance-only"; +} + +/** True when instance+policy sections should be rendered. */ +export function hasInstanceContext(ctx: AgentEditContext): boolean { + return ctx.kind !== "definition-only"; +} + +// ── AgentFormModel ────────────────────────────────────────────────────────── + +/** + * Flat, typed field model covering every editable field on the merged surface. + * + * Fields absent for a given context are undefined. Editability is determined + * by AgentEditContext + the isDefinitionReadOnly predicate, NOT by AgentFormModel + * itself — the model just holds current values. + */ +export type AgentFormModel = { + // Identity (D when linked, I when unlinked) + displayName: string; + avatarUrl: string; + + // Behavior (D when linked, I when unlinked) + systemPrompt: string; + respondTo: ManagedAgent["respondTo"] | AgentPersona["respondTo"] | null; + respondToAllowlist: string[]; + + // Runtime (D when linked, I when unlinked) — harness pins are I + /** Preferred runtime id from the definition. */ + runtime: string | undefined; + model: string | null; + provider: string | null; + envVars: Record; + + // Definition-only fields + /** Name pool for definition. Undefined when no definition context. */ + namePool: string[] | undefined; + + // Instance-level fields + /** Instance name (per-pool override of displayName). */ + instanceName: string | undefined; + parallelism: number | null; + + // Device policy (L-fields) + autoRestartOnConfigChange: boolean | undefined; + startOnAppLaunch: boolean | undefined; +}; + +/** Which coordinator outputs changed vs. last saved state. */ +export type AgentFormEmit = { + /** D-field update payload, present iff a D-field changed AND the definition is editable. */ + personaInput: UpdatePersonaInput | null; + /** I-field update payload, present iff an I-field changed. */ + agentInput: UpdateManagedAgentInput | null; + /** Policy-setter calls, one entry per changed L-field. */ + policySets: Array< + | { type: "autoRestart"; pubkey: string; value: boolean } + | { type: "startOnAppLaunch"; pubkey: string; value: boolean } + >; +}; + +// ── Canonical comparison helpers ───────────────────────────────────────────── + +/** Map equality per Artifact 3 settlement contract. */ +export function envVarsMapEqual( + a: Record, + b: Record, +): boolean { + const aKeys = Object.keys(a); + const bKeys = Object.keys(b); + if (aKeys.length !== bKeys.length) return false; + return aKeys.every((k) => a[k] === b[k]); +} + +/** Canonical name-pool equality: order-sensitive, trimmed. */ +export function namePoolEqual(a: string[], b: string[]): boolean { + if (a.length !== b.length) return false; + return a.every((v, i) => v.trim() === b[i]?.trim()); +} + +// ── Edit adapters ──────────────────────────────────────────────────────────── + +/** + * Seed an AgentFormModel from (definition?, agent?) — at least one present. + * + * For instance-with-definition: D-fields seed from the definition (authoritative + * source); I-fields seed from the instance. For definition-only: instance fields + * are undefined. For instance-only: definition fields are undefined. + */ +export function seedAgentFormModel(ctx: AgentEditContext): AgentFormModel { + const def = editContextDefinition(ctx); + const inst = editContextInstance(ctx); + + // D-fields: from definition when present, else from instance + const displayName = def?.displayName ?? inst?.name ?? ""; + const avatarUrl = def?.avatarUrl ?? inst?.avatarUrl ?? ""; + const systemPrompt = def?.systemPrompt ?? inst?.systemPrompt ?? ""; + const respondTo = def?.respondTo ?? inst?.respondTo ?? null; + const respondToAllowlist = + def?.respondToAllowlist ?? inst?.respondToAllowlist ?? []; + const runtime = def?.runtime ?? undefined; + const model = def !== null ? (def.model ?? null) : (inst?.model ?? null); + const provider = + def !== null ? (def.provider ?? null) : (inst?.provider ?? null); + // Env: D-field base. Instance overlay is applied separately at spawn; + // the form edits each store independently (row 8 contract). + const envVars = def?.envVars ?? inst?.envVars ?? {}; + const namePool = def?.namePool; + + // I-fields + const instanceName = inst?.name; + const parallelism = inst?.parallelism ?? def?.parallelism ?? null; + + // L-fields + const autoRestartOnConfigChange = inst?.autoRestartOnConfigChange; + const startOnAppLaunch = inst?.startOnAppLaunch; + + return { + displayName, + avatarUrl, + systemPrompt, + respondTo, + respondToAllowlist, + runtime, + model, + provider, + envVars, + namePool, + instanceName, + parallelism, + autoRestartOnConfigChange, + startOnAppLaunch, + }; +} + +/** + * Emit coordinator inputs from a (previous model / saved state) pair. + * + * Returns personaInput when a D-field changed AND the definition is editable. + * Returns agentInput when an I-field changed. + * Returns policySets for any L-field changed. + * + * The caller (save coordinator) calls this after re-fetching observed state, + * so "previous" is the re-fetched stored state and "next" is what the user + * submitted — the diff is exactly what hasn't been persisted yet. + */ +export function emitAgentFormDiff( + saved: AgentFormModel, + next: AgentFormModel, + ctx: AgentEditContext, + _options?: { publishCatalogUpdates?: boolean }, +): AgentFormEmit { + const def = editContextDefinition(ctx); + const inst = editContextInstance(ctx); + const defReadOnly = isDefinitionReadOnly(ctx); + + // D-field diff — only when definition is present and NOT team-managed + let personaInput: UpdatePersonaInput | null = null; + if (def !== null && !defReadOnly) { + const dChanged = + next.displayName.trim() !== saved.displayName.trim() || + (next.avatarUrl ?? "") !== (saved.avatarUrl ?? "") || + next.systemPrompt.trim() !== saved.systemPrompt.trim() || + next.runtime !== saved.runtime || + (next.model ?? null) !== (saved.model ?? null) || + (next.provider ?? null) !== (saved.provider ?? null) || + !envVarsMapEqual(next.envVars ?? {}, saved.envVars ?? {}) || + !namePoolEqual(next.namePool ?? [], saved.namePool ?? []); + + if (dChanged) { + personaInput = { + id: def.id, + displayName: next.displayName.trim(), + avatarUrl: next.avatarUrl ?? "", + systemPrompt: next.systemPrompt.trim(), + runtime: next.runtime, + model: next.model ?? undefined, + provider: next.provider ?? undefined, + namePool: next.namePool ?? [], + envVars: next.envVars ?? {}, + ...(next.respondTo != null + ? { + behavior: { + respondTo: next.respondTo, + respondToAllowlist: + next.respondTo === "allowlist" + ? next.respondToAllowlist + : undefined, + parallelism: next.parallelism ?? undefined, + }, + } + : {}), + }; + } + } + + // I-field diff — only when instance is present + let agentInput: UpdateManagedAgentInput | null = null; + if (inst !== null) { + // Row 1 contract: stop materializing displayName→name when a name pool exists + const hasNamePool = (def?.namePool?.length ?? 0) > 0; + + // Row 8 contract: instance env edit goes to instance, NOT wholesale-replace from definition + const instanceEnvChanged = + "instanceEnvVars" in next + ? !envVarsMapEqual( + ( + next as AgentFormModel & { + instanceEnvVars?: Record; + } + ).instanceEnvVars ?? {}, + inst.envVars ?? {}, + ) + : false; + + const nameChanged = + !hasNamePool && + (next.instanceName ?? next.displayName.trim()) !== inst.name; + const systemPromptChanged = + def === null && + (next.systemPrompt.trim() || null) !== (inst.systemPrompt ?? null); + const modelChanged = + def === null && (next.model ?? null) !== (inst.model ?? null); + const providerChanged = + def === null && (next.provider ?? null) !== (inst.provider ?? null); + const respondToChanged = + (next.respondTo ?? null) !== (inst.respondTo ?? null); + const allowlistChanged = + next.respondTo === "allowlist" && + next.respondToAllowlist.join(",") !== inst.respondToAllowlist.join(","); + const parallelismChanged = + (next.parallelism ?? null) !== (inst.parallelism ?? null); + + const iChanged = + nameChanged || + systemPromptChanged || + modelChanged || + providerChanged || + instanceEnvChanged || + respondToChanged || + allowlistChanged || + parallelismChanged; + + if (iChanged) { + agentInput = { pubkey: inst.pubkey }; + if (nameChanged) + agentInput.name = next.instanceName ?? next.displayName.trim(); + if (systemPromptChanged) + agentInput.systemPrompt = next.systemPrompt.trim() || null; + if (modelChanged) agentInput.model = next.model ?? null; + if (providerChanged) agentInput.provider = next.provider ?? null; + if (respondToChanged) agentInput.respondTo = next.respondTo ?? undefined; + if (allowlistChanged) + agentInput.respondToAllowlist = next.respondToAllowlist; + if (parallelismChanged && next.parallelism != null) + agentInput.parallelism = next.parallelism; + } + } + + // L-field diff + const policySets: AgentFormEmit["policySets"] = []; + if (inst !== null) { + if ( + next.autoRestartOnConfigChange !== undefined && + next.autoRestartOnConfigChange !== saved.autoRestartOnConfigChange + ) { + policySets.push({ + type: "autoRestart", + pubkey: inst.pubkey, + value: next.autoRestartOnConfigChange, + }); + } + if ( + next.startOnAppLaunch !== undefined && + next.startOnAppLaunch !== saved.startOnAppLaunch + ) { + policySets.push({ + type: "startOnAppLaunch", + pubkey: inst.pubkey, + value: next.startOnAppLaunch, + }); + } + } + + return { personaInput, agentInput, policySets }; +} diff --git a/desktop/src/features/agents/ui/agentSaveCoordinator.ts b/desktop/src/features/agents/ui/agentSaveCoordinator.ts new file mode 100644 index 0000000000..053d064c92 --- /dev/null +++ b/desktop/src/features/agents/ui/agentSaveCoordinator.ts @@ -0,0 +1,314 @@ +/** + * agentSaveCoordinator.ts — Save coordinator per Artifact 3 of the Phase 0 spec. + * + * Execution sequence on every submit: + * 0. Validate (pre-write): runtime availability, credential gate, + * parallelism parsing + * 1. Definition write iff a D-field changed AND definition is editable + * 2. Instance write iff an I-field changed (row 1/8 drops excluded) + * 3. Local-policy setters only on change + * 4. Settlement re-fetch BOTH stores, derive persisted-vs-unsaved + * from observed state (not from command result) + * + * Partial-failure: stop at first failure, run settlement, toast from observed + * state — naming exactly what persisted and what didn't. + * + * Not in scope: 14a membership, 14b identity archive — no write path through Save. + */ + +import { toast } from "sonner"; +import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions"; +import { showAgentProfileSyncWarning } from "@/features/agents/ui/agentProfileSyncWarning"; +import { validateLinkedAgentRuntimeEdit } from "@/features/profile/ui/UserProfilePanelPersonaSubmit"; +import type { + AcpRuntimeCatalogEntry, + AgentPersona, + ManagedAgent, + UpdateManagedAgentInput, + UpdatePersonaInput, +} from "@/shared/api/types"; +import type { AgentEditContext } from "./agentFormModel"; +import { + editContextDefinition, + editContextInstance, + envVarsMapEqual, + namePoolEqual, +} from "./agentFormModel"; + +// ── Types ──────────────────────────────────────────────────────────────────── + +export type SaveCoordinatorOptions = { + /** Current edit context (definition + optional instance). */ + ctx: AgentEditContext; + /** Submitted definition update (null = no D-change or team-managed). */ + personaInput: UpdatePersonaInput | null; + /** Submitted instance update (null = no I-change). */ + agentInput: UpdateManagedAgentInput | null; + /** Policy setter calls. */ + policySets: Array< + | { type: "autoRestart"; pubkey: string; value: boolean } + | { type: "startOnAppLaunch"; pubkey: string; value: boolean } + >; + /** Whether the submitted definition was flagged for catalog publish. */ + publishCatalogUpdates?: boolean; + /** Validated runtime catalog for the runtime-edit gate. */ + runtimes?: readonly AcpRuntimeCatalogEntry[]; + + // Mutations + updatePersona: (input: UpdatePersonaInput) => Promise; + updatePersonaAndPublish: (input: UpdatePersonaInput) => Promise; + updateManagedAgent: ( + input: UpdateManagedAgentInput, + ) => Promise<{ agent: ManagedAgent; profileSyncError: string | null }>; + setAutoRestart: (pubkey: string, value: boolean) => Promise; + setStartOnAppLaunch: (pubkey: string, value: boolean) => Promise; + + /** Re-fetch both stores and return the current (persona, agent) pair. */ + refetchStores: () => Promise<{ + persona: AgentPersona | null; + agent: ManagedAgent | null; + }>; + + /** Called when the dialog should close (on full success). */ + onDone: () => void; + /** Called after save to optionally offer "Start now" (passed the post-settle agent). */ + onSavedWhileStopped?: (agent: ManagedAgent) => void; +}; + +// ── Coordinator ─────────────────────────────────────────────────────────────── + +/** + * Execute the Artifact 3 save sequence and settle from observed state. + * + * Returns true if all writes succeeded; false on partial or full failure. + * Callers must keep the dialog open on false so the user sees the error + * and the re-seeded form shows the unsaved remainder. + */ +export async function runAgentSaveCoordinator( + opts: SaveCoordinatorOptions, +): Promise { + const { + ctx, + personaInput, + agentInput, + policySets, + publishCatalogUpdates, + runtimes, + updatePersona, + updatePersonaAndPublish, + updateManagedAgent, + setAutoRestart, + setStartOnAppLaunch, + refetchStores, + onDone, + onSavedWhileStopped, + } = opts; + + const def = editContextDefinition(ctx); + const inst = editContextInstance(ctx); + + // ── Step 0: Validate ────────────────────────────────────────────────────── + if (personaInput && def && inst) { + const runtimeError = validateLinkedAgentRuntimeEdit({ + input: personaInput, + managedAgent: inst, + previousPersona: def, + runtimes: runtimes ? [...runtimes] : undefined, + }); + if (runtimeError) { + toast.error(runtimeError); + return false; + } + } + + // ── Steps 1–3: Writes ───────────────────────────────────────────────────── + let definitionWritten = false; + let instanceWritten = false; + let policyWritten = false; + let firstError: string | null = null; + let profileSyncError: string | null = null; + let latestAgent: ManagedAgent | null = inst; + + // Step 1: Definition write + if (personaInput && !firstError) { + try { + if (publishCatalogUpdates) { + await updatePersonaAndPublish(personaInput); + } else { + await updatePersona(personaInput); + } + definitionWritten = true; + } catch (err) { + firstError = + err instanceof Error ? err.message : "Failed to save agent profile."; + } + } + + // Step 2: Instance write + if (agentInput && !firstError) { + try { + const result = await updateManagedAgent(agentInput); + latestAgent = result.agent; + instanceWritten = true; + profileSyncError = result.profileSyncError; + } catch (err) { + firstError = + err instanceof Error ? err.message : "Failed to save agent settings."; + } + } + + // Step 3: Policy setters + if (!firstError) { + for (const policy of policySets) { + try { + if (policy.type === "autoRestart") { + await setAutoRestart(policy.pubkey, policy.value); + } else { + await setStartOnAppLaunch(policy.pubkey, policy.value); + } + policyWritten = true; + } catch (err) { + firstError = + err instanceof Error ? err.message : "Failed to save agent policy."; + break; + } + } + } + + // ── Step 4: Settlement — re-fetch both stores ───────────────────────────── + const { persona: observedPersona, agent: observedAgent } = + await refetchStores(); + + if (!firstError) { + // Full success path + const agentName = + latestAgent?.name ?? observedAgent?.name ?? def?.displayName ?? "Agent"; + + if (profileSyncError) { + showAgentProfileSyncWarning(agentName, profileSyncError); + } else { + toast.success(`${agentName} saved.`); + } + + // "Saved while stopped" affordance (Artifact 3 contract) + const finalAgent = observedAgent ?? latestAgent; + if (finalAgent && !isManagedAgentActive(finalAgent)) { + onSavedWhileStopped?.(finalAgent); + } + + onDone(); + return true; + } + + // ── Partial-failure: derive what persisted from observed state ──────────── + const persistedParts: string[] = []; + const failedParts: string[] = []; + + if (personaInput) { + const persisted = observedPersona + ? observedStateMatchesPersonaInput(observedPersona, personaInput) + : definitionWritten; + if (persisted) { + persistedParts.push("profile"); + } else { + failedParts.push("profile"); + } + } + + if (agentInput) { + const persisted = observedAgent + ? observedStateMatchesAgentInput(observedAgent, agentInput) + : instanceWritten; + if (persisted) { + persistedParts.push("instance settings"); + } else { + failedParts.push("instance settings"); + } + } + + if (policySets.length > 0 && !policyWritten) { + failedParts.push("device policy"); + } + + if (persistedParts.length > 0 && failedParts.length > 0) { + const kept = persistedParts.join(" and "); + const failed = failedParts.join(" and "); + toast.warning( + `${capitalizeFirst(kept)} saved. ${capitalizeFirst(failed)} failed: ${firstError} — reopen to retry; your ${kept} change is kept.`, + ); + } else if (persistedParts.length > 0) { + toast.success( + `Saved. Some changes may not have persisted — reopen to retry.`, + ); + } else { + toast.error(firstError ?? "Failed to save agent."); + } + + return false; +} + +// ── Observed-state comparison helpers ──────────────────────────────────────── +// Used to determine what persisted after a partial failure. Compares +// CANONICAL stored values (trimmed strings, normalized name pool, map equality) +// per the settlement contract. + +function observedStateMatchesPersonaInput( + observed: AgentPersona, + submitted: UpdatePersonaInput, +): boolean { + return ( + observed.displayName.trim() === submitted.displayName.trim() && + observed.systemPrompt.trim() === (submitted.systemPrompt ?? "").trim() && + (observed.model ?? null) === (submitted.model ?? null) && + (observed.provider ?? null) === (submitted.provider ?? null) && + namePoolEqual(observed.namePool, submitted.namePool ?? []) && + envVarsMapEqual(observed.envVars, submitted.envVars ?? {}) + ); +} + +function observedStateMatchesAgentInput( + observed: ManagedAgent, + submitted: UpdateManagedAgentInput, +): boolean { + // Only check fields that were submitted (undefined means "don't touch") + if ( + submitted.name !== undefined && + (submitted.name ?? "").trim() !== observed.name.trim() + ) { + return false; + } + if ( + submitted.systemPrompt !== undefined && + (submitted.systemPrompt ?? null) !== (observed.systemPrompt ?? null) + ) { + return false; + } + if ( + submitted.model !== undefined && + (submitted.model ?? null) !== (observed.model ?? null) + ) { + return false; + } + if ( + submitted.provider !== undefined && + (submitted.provider ?? null) !== (observed.provider ?? null) + ) { + return false; + } + if ( + submitted.envVars !== undefined && + !envVarsMapEqual(submitted.envVars, observed.envVars) + ) { + return false; + } + return true; +} + +function capitalizeFirst(s: string): string { + if (!s) return s; + return s.charAt(0).toUpperCase() + s.slice(1); +} + +// Re-export for convenient use in AgentEditDialog +export { seedAgentFormModel, emitAgentFormDiff } from "./agentFormModel"; +export type { AgentFormModel, AgentEditContext } from "./agentFormModel"; diff --git a/desktop/src/features/agents/ui/usePersonaActions.ts b/desktop/src/features/agents/ui/usePersonaActions.ts index b4ef5afb6c..0693895742 100644 --- a/desktop/src/features/agents/ui/usePersonaActions.ts +++ b/desktop/src/features/agents/ui/usePersonaActions.ts @@ -51,7 +51,6 @@ import type { } from "@/shared/api/types"; import { duplicatePersonaDialogState, - editPersonaDialogState, type PersonaDialogState, } from "./personaDialogState"; import { @@ -94,6 +93,10 @@ export function usePersonaActions() { const [personaDialogState, setPersonaDialogState] = React.useState(null); + // R5 route collapse: tracks the persona being edited via the merged surface. + const [personaToEdit, setPersonaToEdit] = React.useState( + null, + ); const [personaToDelete, setPersonaToDelete] = React.useState(null); const [personaToShare, setPersonaToShare] = React.useState<{ @@ -430,7 +433,8 @@ export function usePersonaActions() { function openEdit(persona: AgentPersona) { clearFeedback("library"); setShouldLoadAcpRuntimes(true); - setPersonaDialogState(editPersonaDialogState(persona)); + // R5: route to the merged AgentEditDialog surface (definition-only context). + setPersonaToEdit(persona); } function openDuplicate(persona: AgentPersona) { @@ -581,6 +585,8 @@ export function usePersonaActions() { isPending, personaDialogState, setPersonaDialogState, + personaToEdit, + setPersonaToEdit, personaToDelete, setPersonaToDelete, personaToShare, diff --git a/desktop/src/features/agents/useAgentManagement.ts b/desktop/src/features/agents/useAgentManagement.ts index 066f7949a9..822f84999f 100644 --- a/desktop/src/features/agents/useAgentManagement.ts +++ b/desktop/src/features/agents/useAgentManagement.ts @@ -27,36 +27,11 @@ import { classifyAgentManagementOrigin } from "./agentManagementBuffer"; import { useChannelsQuery } from "@/features/channels/hooks"; import { resolveManagedAgentAvatarUrl } from "./ui/managedAgentAvatar"; import type { AgentCreateIntent } from "./ui/agentCreateIntent"; -import { editPersonaDialogState } from "./ui/personaDialogState"; import type { CreatePersonaInput, UpdatePersonaInput, } from "@/shared/api/types"; -function updateInputFromRequest( - request: Extract, - current: UpdatePersonaInput, -): UpdatePersonaInput { - const changes = request.request; - return { - ...current, - displayName: changes.displayName ?? current.displayName, - systemPrompt: changes.systemPrompt ?? current.systemPrompt, - runtime: changes.runtime ?? current.runtime, - provider: changes.provider ?? current.provider, - model: changes.model ?? current.model, - ...(changes.respondTo - ? { - behavior: { - respondTo: changes.respondTo, - respondToAllowlist: [], - parallelism: current.behavior?.parallelism, - }, - } - : {}), - }; -} - export function useAgentManagement() { const queryClient = useQueryClient(); const personasQuery = usePersonasQuery(); @@ -271,15 +246,6 @@ export function useAgentManagement() { [request], ); - const editInitialValues = React.useMemo(() => { - if (request?.action !== "update" || !currentPersona) return null; - return updateInputFromRequest( - request, - editPersonaDialogState(currentPersona) - .initialValues as UpdatePersonaInput, - ); - }, [currentPersona, request]); - const editError = React.useMemo(() => { if (request?.action !== "update") return error; if (error) return error; @@ -294,8 +260,8 @@ export function useAgentManagement() { return { request, + currentPersona, createInitialValues, - editInitialValues, editError, error, ...createdAgentAttachment, @@ -309,5 +275,38 @@ export function useAgentManagement() { submitCreate, submitUpdate, dismiss, + /** Returns an error string if the requesting agent lacks origin permission, null if OK. */ + verifyOriginPermission(): string | null { + if (request?.action !== "update") return null; + try { + assertAgentCanActFromOrigin(request.request.channelId); + return null; + } catch (cause) { + return cause instanceof Error + ? cause.message + : "Origin permission check failed."; + } + }, + /** Returns agent-requested field changes for R6 review mode pre-fill (overrides for the merged dialog). */ + get reviewOverrides() { + if (request?.action !== "update" || !currentPersona) return undefined; + const changes = request.request; + const overrides: Partial<{ + displayName: string; + systemPrompt: string; + runtime: string | undefined; + provider: string | undefined; + model: string | undefined; + }> = {}; + if (changes.displayName != null) + overrides.displayName = changes.displayName; + if (changes.systemPrompt != null) + overrides.systemPrompt = changes.systemPrompt; + if (changes.runtime != null) overrides.runtime = changes.runtime; + if (changes.provider != null) + overrides.provider = changes.provider ?? undefined; + if (changes.model != null) overrides.model = changes.model ?? undefined; + return Object.keys(overrides).length > 0 ? overrides : undefined; + }, }; } diff --git a/desktop/src/features/profile/ui/UserProfilePanel.tsx b/desktop/src/features/profile/ui/UserProfilePanel.tsx index cb188dd008..767948b5db 100644 --- a/desktop/src/features/profile/ui/UserProfilePanel.tsx +++ b/desktop/src/features/profile/ui/UserProfilePanel.tsx @@ -33,7 +33,10 @@ import { resolveStartRuntimeForDefinition, } from "@/features/agents/lib/instanceInputForDefinition"; import { describeLogFile } from "@/features/agents/ui/agentUi"; -import { AgentDialog } from "@/features/agents/ui/AgentDialog"; +import { + AgentEditDialog, + type AgentEditContext, +} from "@/features/agents/ui/AgentEditDialog"; import { useAgentLifecycleActions } from "@/features/profile/ui/useAgentLifecycleActions"; import { consumePendingOpenEditAgent, @@ -42,7 +45,6 @@ import { } from "@/features/agents/openEditAgentEvent"; import { duplicatePersonaDialogState, - editPersonaDialogState, type PersonaDialogState, } from "@/features/agents/ui/personaDialogState"; import { useChannelsQuery } from "@/features/channels/hooks"; @@ -402,12 +404,8 @@ export function UserProfilePanel({ }); const handleEditAgent = React.useCallback(() => { - if (resolvedPersona) { - setPersonaDialogState(editPersonaDialogState(resolvedPersona)); - return; - } setEditAgentOpen(true); - }, [resolvedPersona]); + }, []); const { deleteManagedAgentRecord, deleteManagedAgentsForPersona } = useProfileAgentDeletion({ @@ -549,7 +547,7 @@ export function UserProfilePanel({ const handleEditPersona = React.useCallback(() => { if (!resolvedPersona) return; - setPersonaDialogState(editPersonaDialogState(resolvedPersona)); + setEditAgentOpen(true); }, [resolvedPersona]); const handleDuplicatePersona = React.useCallback(() => { @@ -905,21 +903,23 @@ export function UserProfilePanel({ ) : null} ); - const editAgentDialog = - canEditAgent && managedAgent ? ( - { - setEditAgentOpen(false); - setEditAgentFocus(undefined); - setPersonaDialogState(editPersonaDialogState(resolvedPersona)); - } - : undefined + const editCtx: AgentEditContext | null = + managedAgent && resolvedPersona + ? { + kind: "instance-with-definition", + definition: resolvedPersona, + instance: managedAgent, } + : managedAgent + ? { kind: "instance-only", instance: managedAgent } + : resolvedPersona + ? { kind: "definition-only", definition: resolvedPersona } + : null; + const editAgentDialog = + canEditAgent && editCtx ? ( + { setEditAgentOpen(next); if (!next) setEditAgentFocus(undefined);