From a61df00384e606cfb377d61ef1809763017712cf Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 27 Jul 2026 15:56:52 +0000 Subject: [PATCH] fix(ui): include environment drafts in save review Co-Authored-By: Paperclip --- .../AgentConfigForm.render.test.tsx | 11 +++++++++ ui/src/components/AgentConfigForm.tsx | 23 ++++++++++++------- .../environment-variables-editor/index.tsx | 9 ++++++-- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/ui/src/components/AgentConfigForm.render.test.tsx b/ui/src/components/AgentConfigForm.render.test.tsx index 6d6969bd393..b3867b5174d 100644 --- a/ui/src/components/AgentConfigForm.render.test.tsx +++ b/ui/src/components/AgentConfigForm.render.test.tsx @@ -203,6 +203,7 @@ async function renderForm( visibleConfigurationSections?: ReadonlySet; onDirtyChange?: (dirty: boolean) => void; onDirtyDetailsChange?: (details: { count: number; sections: string[] }) => void; + onChangesetChange?: Parameters[0]["onChangesetChange"]; } = {}, ) { mockEnvironmentsApi.list.mockResolvedValue(environments); @@ -233,6 +234,7 @@ async function renderForm( sectionLayout={options.configurationShell ? "cards" : undefined} onDirtyChange={options.onDirtyChange} onDirtyDetailsChange={options.onDirtyDetailsChange} + onChangesetChange={options.onChangesetChange} /> , @@ -630,6 +632,7 @@ describe("AgentConfigForm environment selector", () => { it("reports editor-local environment drafts as unsaved configuration", async () => { const onDirtyChange = vi.fn(); const onDirtyDetailsChange = vi.fn(); + const onChangesetChange = vi.fn(); const result = await renderForm([ makeEnvironment({ id: "local-1", name: "Local", driver: "local" }), ], { @@ -641,6 +644,7 @@ describe("AgentConfigForm environment selector", () => { visibleConfigurationSections: new Set(["environment"]), onDirtyChange, onDirtyDetailsChange, + onChangesetChange, }); roots.push(result.root); @@ -657,6 +661,13 @@ describe("AgentConfigForm environment selector", () => { count: 1, sections: ["environment"], }); + expect(onChangesetChange.mock.lastCall?.[0]).toEqual([ + expect.objectContaining({ + key: "adapterConfig.env", + section: "Environment", + after: { API_TOKEN: { type: "plain", value: "draft-token" } }, + }), + ]); }); it("surfaces request failures instead of converting them into model test checks", async () => { diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index fc6dec88364..167af7df8a8 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -338,7 +338,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { // ---- Edit mode: overlay for dirty tracking ---- const [overlay, setOverlay] = useState(emptyOverlay); - const [environmentDraftDirty, setEnvironmentDraftDirty] = useState(false); + const [environmentDraft, setEnvironmentDraft] = useState | null>(null); const agentRef = useRef(null); // Clear overlay when agent data refreshes (after save) @@ -352,6 +352,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { }, [isCreate, !isCreate ? props.agent : undefined]); // eslint-disable-line react-hooks/exhaustive-deps const overlayDirty = !isCreate && isOverlayDirty(overlay); + const environmentDraftDirty = environmentDraft !== null; const isDirty = overlayDirty || environmentDraftDirty; const dirtyDetails = useMemo(() => { const details = getAgentConfigDirtyDetails(overlay); @@ -363,10 +364,16 @@ export function AgentConfigForm(props: AgentConfigFormProps) { : [...details.sections, "environment"], }; }, [environmentDraftDirty, overlay]); - const changes = useMemo( - () => isCreate ? [] : buildAgentConfigChanges(props.agent, overlay), - [isCreate, overlay, !isCreate ? props.agent : undefined], // eslint-disable-line react-hooks/exhaustive-deps - ); + const changes = useMemo(() => { + if (isCreate) return []; + const changesOverlay = environmentDraft && !("env" in overlay.adapterConfig) + ? { + ...overlay, + adapterConfig: { ...overlay.adapterConfig, env: environmentDraft }, + } + : overlay; + return buildAgentConfigChanges(props.agent, changesOverlay); + }, [environmentDraft, isCreate, overlay, !isCreate ? props.agent : undefined]); // eslint-disable-line react-hooks/exhaustive-deps const revertChange = useCallback((key: string) => { setOverlay((current) => revertAgentConfigChange(current, key)); }, []); @@ -418,7 +425,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { /** Build accumulated patch and send to parent */ const handleCancel = useCallback(() => { environmentVariablesEditorRef.current?.discardPendingDraft(); - setEnvironmentDraftDirty(false); + setEnvironmentDraft(null); setOverlay({ ...emptyOverlay }); }, []); @@ -1459,7 +1466,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { ? set!({ envBindings: env ?? {}, envVars: "" }) : mark("adapterConfig", "env", env) } - onDirtyChange={isCreate ? undefined : setEnvironmentDraftDirty} + onDraftChange={isCreate ? undefined : setEnvironmentDraft} /> } @@ -1645,7 +1652,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { ? set!({ envBindings: env ?? {}, envVars: "" }) : mark("adapterConfig", "env", env) } - onDirtyChange={isCreate ? undefined : setEnvironmentDraftDirty} + onDraftChange={isCreate ? undefined : setEnvironmentDraft} /> {!isCreate && ( diff --git a/ui/src/components/environment-variables-editor/index.tsx b/ui/src/components/environment-variables-editor/index.tsx index 39cf57807f3..35658671efa 100644 --- a/ui/src/components/environment-variables-editor/index.tsx +++ b/ui/src/components/environment-variables-editor/index.tsx @@ -135,6 +135,8 @@ export interface EnvironmentVariablesEditorProps { footerHint?: ReactNode | null; /** Reports editor-local draft changes that are not yet promoted to the parent value. */ onDirtyChange?: (dirty: boolean) => void; + /** Reports the current editor-local draft, or null when it matches the controlled value. */ + onDraftChange?: (draft: Record | null) => void; } export interface EnvironmentVariablesEditorHandle { @@ -158,6 +160,7 @@ export const EnvironmentVariablesEditor = forwardRef(null); @@ -238,8 +241,10 @@ export const EnvironmentVariablesEditor = forwardRef { - onDirtyChange?.(!disabled && hasUnsavedChanges); - }, [disabled, hasUnsavedChanges, onDirtyChange]); + const dirty = !disabled && hasUnsavedChanges; + onDirtyChange?.(dirty); + onDraftChange?.(dirty ? (draftValue ?? {}) : null); + }, [disabled, draftValue, hasUnsavedChanges, onDirtyChange, onDraftChange]); // Which variables differ from the committed baseline, so the unsaved-changes // banner can say *what* is unsaved instead of a bare label. A rename shows