From 12b517dfcddf445f6762907cf075d7eeffb85cea Mon Sep 17 00:00:00 2001 From: Duncan Date: Sat, 8 Aug 2026 12:52:12 -0400 Subject: [PATCH] fix(agents): wire observed-state settlement, team D-field read-only, coordinator tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address all pass-1 CHANGES_REQUIRED findings from Thufir's review: CRITICAL-1+2: AgentEditDialog now delegates instance contexts entirely to AgentInstanceEditDialog (all I/L fields, correct testid). Definition-only contexts use AgentDefinitionDialog + Artifact 3 coordinator. Removed the type-cast hack (onUpdated?.(undefined as unknown as ManagedAgent)); the definition-only path correctly does not call onUpdated. IMPORTANT-3: AgentDefinitionDialog gains definitionReadOnly prop — when set, all D-fields render disabled and a 'Managed by team' notice is shown; canSubmit is gated; coordinator also guards against a misconfigured bypass. isDefinitionReadOnly in agentFormModel.ts checks sourceTeam at the diff layer too. IMPORTANT-4: Coordinator now settles from observed state on BOTH success and error paths (not just error). The old 'no firstError → success' branch is gone; the result is derived entirely from re-fetched observed store comparison. Absent entity after refetch = not persisted. Per-policy failure tracking: each policy tracked individually; unattempted policies also reported failed. IMPORTANT-5: Publish success toast uses personaSaveNotice (not generic 'saved'). Publication status tracked through updatePersonaAndPublish return value. Tests added: - agentFormModel.test.mjs: 3 new tests covering team D-field unemittability (test_team_definition_emits_no_personaInput_even_when_fields_differ, test_team_definition_with_instance_emits_no_personaInput_but_allows_instance_diff, plus the env-clobber variants already present) - agentSaveCoordinator.test.mjs (new file): 11 tests covering write ordering, local-save/publish failure, observed-state mismatch, and partial policy failure Full desktop test suite: 4553/4553 passing. TypeScript clean. Biome clean. File-size gate: AgentDefinitionDialog.tsx 1041 lines (limit 1041). Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../agents/ui/AgentDefinitionDialog.tsx | 44 +- .../features/agents/ui/AgentEditDialog.tsx | 212 ++++---- .../agents/ui/agentFormModel.test.mjs | 85 +++ .../agents/ui/agentSaveCoordinator.test.mjs | 482 ++++++++++++++++++ .../agents/ui/agentSaveCoordinator.ts | 192 +++++-- 5 files changed, 827 insertions(+), 188 deletions(-) create mode 100644 desktop/src/features/agents/ui/agentSaveCoordinator.test.mjs diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index b4606a0f6..509bb5b42 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -102,6 +102,8 @@ type AgentDefinitionDialogProps = { isPending: boolean; runtimes: AcpRuntimeCatalogEntry[]; runtimeCatalogStatus?: "loading" | "ready" | "error"; + /** When true, D-fields render disabled + "Managed by team" notice; submit blocked. */ + definitionReadOnly?: boolean; onDirtyChange?: (dirty: boolean) => void; onOpenChange: (open: boolean) => void; onSubmit: ( @@ -130,6 +132,7 @@ export function AgentDefinitionDialog({ isPending, runtimes, runtimeCatalogStatus = "ready" as const, + definitionReadOnly = false, onDirtyChange, onOpenChange, onSubmit, @@ -159,15 +162,9 @@ export function AgentDefinitionDialog({ // The seed the draft is diffed against at submit: an untouched quad // submits no behavior group, keeping unrelated edits hash-quiet. const behaviorSeedRef = React.useRef(emptyPersonaBehaviorDraft); - // Tracks when the runtime was auto-seeded by the default-runtime effect in - // edit mode (i.e. the user never explicitly chose a runtime). Used to omit - // the seeded runtime from the submit payload for builtin definitions whose - // canonical runtime is null — the sync would revert it anyway. + // Tracks when the runtime was auto-seeded (not an explicit user choice). const isRuntimeAutoSeededRef = React.useRef(false); // Guards the seeding effect so it fires at most once per dialog-open. - // Without this, clearing runtime back to "" via "No preference" would re- - // trigger the effect (the `runtime` dep would pass the length guard) and - // snap the dropdown back to the default — an edit-mode regression. const hasSeededForOpenRef = React.useRef(false); const [showAdvancedFields, setShowAdvancedFields] = React.useState(false); const [isAvatarUploadPending, setIsAvatarUploadPending] = @@ -484,12 +481,8 @@ export function AgentDefinitionDialog({ const modelFieldVisible = runtime.trim().length > 0 || blankRuntimeModelProviderEditable; const isExplicitModelRequired = aiConfigurationMode === "custom"; - // Gate the provider requirement on the field's actual visibility, not the raw - // runtime capability. Codex/Claude hide the provider picker (they drive their - // own provider), so Customize must not require a provider there. But a - // runtime-less legacy/builtin definition still exposes the picker via - // blankRuntimeModelProviderEditable, so it must keep requiring a provider — - // otherwise Save could persist `provider: undefined` despite the visible field. + // Gate provider requirement on visible field (Codex/Claude hide picker), + // but a runtime-less legacy definition must still require provider. const customAiPairSatisfied = agentAiConfigurationModeSatisfied( aiConfigurationMode, { provider, model }, @@ -501,6 +494,7 @@ export function AgentDefinitionDialog({ // Gate model/provider validity through missingNormalizedFields — single // source of truth with the readiness gate so display and Save can't drift. const canSubmit = + !definitionReadOnly && canSubmitPersonaDialog({ displayName, isPending }) && (!isCreateMode || runtime.trim().length > 0) && (!isCreateMode || selectedRuntimeIsAvailable) && @@ -752,7 +746,7 @@ export function AgentDefinitionDialog({ > { setHasUserChanges(true); @@ -766,6 +760,12 @@ export function AgentDefinitionDialog({ />
+ {definitionReadOnly ? ( +

+ This agent is managed by a team. Its configuration cannot be edited + here. +

+ ) : null}