fix(ui): include environment drafts in save review

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
DottaandPaperclip committed 2026-07-27 15:56:52 +00:00
1 parent c5d4d47f63
commit a61df00384
3 files changed
+33 -10

No files matched your search

@@ -203,6 +203,7 @@ async function renderForm(
visibleConfigurationSections?: ReadonlySet<string>;
onDirtyChange?: (dirty: boolean) => void;
onDirtyDetailsChange?: (details: { count: number; sections: string[] }) => void;
onChangesetChange?: Parameters<typeof AgentConfigForm>[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}
/>
</TooltipProvider>
</QueryClientProvider>,
@@ -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 () => {
+15 -8
View File
@@ -338,7 +338,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
// ---- Edit mode: overlay for dirty tracking ----
const [overlay, setOverlay] = useState<AgentConfigOverlay>(emptyOverlay);
const [environmentDraftDirty, setEnvironmentDraftDirty] = useState(false);
const [environmentDraft, setEnvironmentDraft] = useState<Record<string, EnvBinding> | null>(null);
const agentRef = useRef<Agent | null>(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}
/>
</Field>}
@@ -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 && (
<Field label="Secret access" hint={help.secretAccess}>
@@ -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<string, EnvBinding> | null) => void;
}
export interface EnvironmentVariablesEditorHandle {
@@ -158,6 +160,7 @@ export const EnvironmentVariablesEditor = forwardRef<EnvironmentVariablesEditorH
reservedPrefixes = DEFAULT_RESERVED_PREFIXES,
footerHint,
onDirtyChange,
onDraftChange,
}: EnvironmentVariablesEditorProps, ref) {
const toast = useOptionalToastActions();
const editorRootRef = useRef<HTMLDivElement | null>(null);
@@ -238,8 +241,10 @@ export const EnvironmentVariablesEditor = forwardRef<EnvironmentVariablesEditorH
const hasUnsavedChanges = draftValueKey !== committedValueKey;
useEffect(() => {
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