From 44d94d0add78b3390874ab90c3f3a1a303dc8499 Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Thu, 9 Apr 2026 17:50:14 +0000 Subject: [PATCH] fix(ui): persist cleared agent env bindings on save Agent configuration edits already had an API path for replacing the full adapterConfig, but the edit form was still sending merge-style patches. That meant clearing the last environment variable serialized as undefined, the key disappeared from JSON, and the server merged the old env bindings back into the saved config. Build adapter config save payloads as full replacement patches, strip undefined keys before send, and reuse the existing replaceAdapterConfig contract so explicit clears persist correctly. Add regression coverage for the cleared-env case and for adapter-type changes that still need to preserve adapter-agnostic fields. Fixes #3179 --- ui/src/components/AgentConfigForm.tsx | 62 ++------------- ui/src/lib/agent-config-patch.test.ts | 108 ++++++++++++++++++++++++++ ui/src/lib/agent-config-patch.ts | 70 +++++++++++++++++ 3 files changed, 185 insertions(+), 55 deletions(-) create mode 100644 ui/src/lib/agent-config-patch.test.ts create mode 100644 ui/src/lib/agent-config-patch.ts diff --git a/ui/src/components/AgentConfigForm.tsx b/ui/src/components/AgentConfigForm.tsx index dd74c376..065405f9 100644 --- a/ui/src/components/AgentConfigForm.tsx +++ b/ui/src/components/AgentConfigForm.tsx @@ -49,6 +49,7 @@ import { shouldShowLegacyWorkingDirectoryField } from "../lib/legacy-agent-confi import { listAdapterOptions, listVisibleAdapterTypes } from "../adapters/metadata"; import { getAdapterLabel } from "../adapters/adapter-display-registry"; import { useDisabledAdaptersSync } from "../adapters/use-disabled-adapters"; +import { buildAgentUpdatePatch, type AgentConfigOverlay } from "../lib/agent-config-patch"; /* ---- Create mode values ---- */ @@ -89,15 +90,7 @@ type AgentConfigFormProps = { /* ---- Edit mode overlay (dirty tracking) ---- */ -interface Overlay { - identity: Record; - adapterType?: string; - adapterConfig: Record; - heartbeat: Record; - runtime: Record; -} - -const emptyOverlay: Overlay = { +const emptyOverlay: AgentConfigOverlay = { identity: {}, adapterConfig: {}, heartbeat: {}, @@ -107,7 +100,7 @@ const emptyOverlay: Overlay = { /** Stable empty object used as fallback for missing env config to avoid new-object-per-render. */ const EMPTY_ENV: Record = {}; -function isOverlayDirty(o: Overlay): boolean { +function isOverlayDirty(o: AgentConfigOverlay): boolean { return ( Object.keys(o.identity).length > 0 || o.adapterType !== undefined || @@ -211,7 +204,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { }); // ---- Edit mode: overlay for dirty tracking ---- - const [overlay, setOverlay] = useState(emptyOverlay); + const [overlay, setOverlay] = useState(emptyOverlay); const agentRef = useRef(null); // Clear overlay when agent data refreshes (after save) @@ -227,14 +220,14 @@ export function AgentConfigForm(props: AgentConfigFormProps) { const isDirty = !isCreate && isOverlayDirty(overlay); /** Read effective value: overlay if dirty, else original */ - function eff(group: keyof Omit, field: string, original: T): T { + function eff(group: keyof Omit, field: string, original: T): T { const o = overlay[group]; if (field in o) return o[field] as T; return original; } /** Mark field dirty in overlay */ - function mark(group: keyof Omit, field: string, value: unknown) { + function mark(group: keyof Omit, field: string, value: unknown) { setOverlay((prev) => ({ ...prev, [group]: { ...prev[group], [field]: value }, @@ -248,48 +241,7 @@ export function AgentConfigForm(props: AgentConfigFormProps) { const handleSave = useCallback(() => { if (isCreate || !isDirty) return; - const agent = props.agent; - const patch: Record = {}; - - if (Object.keys(overlay.identity).length > 0) { - Object.assign(patch, overlay.identity); - } - if (overlay.adapterType !== undefined) { - patch.adapterType = overlay.adapterType; - // When adapter type changes, replace adapter-specific fields but preserve - // adapter-agnostic fields (env, promptTemplate, etc.) that are shared - // across all adapter types. - const existing = (agent.adapterConfig ?? {}) as Record; - const adapterAgnosticKeys = [ - "env", - "promptTemplate", - "instructionsFilePath", - "cwd", - "timeoutSec", - "graceSec", - "bootstrapPromptTemplate", - ]; - const preserved: Record = {}; - for (const key of adapterAgnosticKeys) { - if (key in existing) { - preserved[key] = existing[key]; - } - } - patch.adapterConfig = { ...preserved, ...overlay.adapterConfig }; - } else if (Object.keys(overlay.adapterConfig).length > 0) { - const existing = (agent.adapterConfig ?? {}) as Record; - patch.adapterConfig = { ...existing, ...overlay.adapterConfig }; - } - if (Object.keys(overlay.heartbeat).length > 0) { - const existingRc = (agent.runtimeConfig ?? {}) as Record; - const existingHb = (existingRc.heartbeat ?? {}) as Record; - patch.runtimeConfig = { ...existingRc, heartbeat: { ...existingHb, ...overlay.heartbeat } }; - } - if (Object.keys(overlay.runtime).length > 0) { - Object.assign(patch, overlay.runtime); - } - - props.onSave(patch); + props.onSave(buildAgentUpdatePatch(props.agent, overlay)); }, [isCreate, isDirty, overlay, props]); useEffect(() => { diff --git a/ui/src/lib/agent-config-patch.test.ts b/ui/src/lib/agent-config-patch.test.ts new file mode 100644 index 00000000..f655e8e5 --- /dev/null +++ b/ui/src/lib/agent-config-patch.test.ts @@ -0,0 +1,108 @@ +// @vitest-environment node + +import { describe, expect, it } from "vitest"; +import type { Agent } from "@paperclipai/shared"; +import { buildAgentUpdatePatch, type AgentConfigOverlay } from "./agent-config-patch"; + +function makeAgent(): Agent { + return { + id: "agent-1", + companyId: "company-1", + name: "Agent", + role: "engineer", + title: "Engineer", + icon: null, + status: "active", + reportsTo: null, + capabilities: null, + adapterType: "claude_local", + adapterConfig: { + model: "claude-sonnet-4-6", + env: { + OPENAI_API_KEY: { + type: "plain", + value: "secret", + }, + }, + promptTemplate: "Work the issue.", + }, + runtimeConfig: { + heartbeat: { + enabled: true, + intervalSec: 300, + }, + }, + budgetMonthlyCents: 0, + spentMonthlyCents: 0, + pauseReason: null, + pausedAt: null, + lastHeartbeatAt: null, + createdAt: new Date("2026-01-01T00:00:00.000Z"), + updatedAt: new Date("2026-01-01T00:00:00.000Z"), + urlKey: "agent", + permissions: { + canCreateAgents: false, + }, + metadata: null, + }; +} + +function makeOverlay(patch?: Partial): AgentConfigOverlay { + return { + identity: {}, + adapterConfig: {}, + heartbeat: {}, + runtime: {}, + ...patch, + }; +} + +describe("buildAgentUpdatePatch", () => { + it("replaces adapter config and drops env when the last env binding is cleared", () => { + const patch = buildAgentUpdatePatch( + makeAgent(), + makeOverlay({ + adapterConfig: { + env: undefined, + }, + }), + ); + + expect(patch).toEqual({ + adapterConfig: { + model: "claude-sonnet-4-6", + promptTemplate: "Work the issue.", + }, + replaceAdapterConfig: true, + }); + }); + + it("preserves adapter-agnostic keys when changing adapter types", () => { + const patch = buildAgentUpdatePatch( + makeAgent(), + makeOverlay({ + adapterType: "codex_local", + adapterConfig: { + model: "gpt-5.4", + dangerouslyBypassApprovalsAndSandbox: true, + }, + }), + ); + + expect(patch).toEqual({ + adapterType: "codex_local", + adapterConfig: { + env: { + OPENAI_API_KEY: { + type: "plain", + value: "secret", + }, + }, + promptTemplate: "Work the issue.", + model: "gpt-5.4", + dangerouslyBypassApprovalsAndSandbox: true, + }, + replaceAdapterConfig: true, + }); + }); +}); diff --git a/ui/src/lib/agent-config-patch.ts b/ui/src/lib/agent-config-patch.ts new file mode 100644 index 00000000..323f9bf5 --- /dev/null +++ b/ui/src/lib/agent-config-patch.ts @@ -0,0 +1,70 @@ +import type { Agent } from "@paperclipai/shared"; + +export interface AgentConfigOverlay { + identity: Record; + adapterType?: string; + adapterConfig: Record; + heartbeat: Record; + runtime: Record; +} + +const ADAPTER_AGNOSTIC_KEYS = [ + "env", + "promptTemplate", + "instructionsFilePath", + "cwd", + "timeoutSec", + "graceSec", + "bootstrapPromptTemplate", +] as const; + +function omitUndefinedEntries(value: Record) { + return Object.fromEntries( + Object.entries(value).filter(([, entryValue]) => entryValue !== undefined), + ); +} + +export function buildAgentUpdatePatch(agent: Agent, overlay: AgentConfigOverlay) { + const patch: Record = {}; + + if (Object.keys(overlay.identity).length > 0) { + Object.assign(patch, overlay.identity); + } + + if (overlay.adapterType !== undefined) { + patch.adapterType = overlay.adapterType; + } + + if (overlay.adapterType !== undefined || Object.keys(overlay.adapterConfig).length > 0) { + const existing = (agent.adapterConfig ?? {}) as Record; + const nextAdapterConfig = + overlay.adapterType !== undefined + ? { + ...Object.fromEntries( + ADAPTER_AGNOSTIC_KEYS + .filter((key) => existing[key] !== undefined) + .map((key) => [key, existing[key]]), + ), + ...overlay.adapterConfig, + } + : { + ...existing, + ...overlay.adapterConfig, + }; + + patch.adapterConfig = omitUndefinedEntries(nextAdapterConfig); + patch.replaceAdapterConfig = true; + } + + if (Object.keys(overlay.heartbeat).length > 0) { + const existingRc = (agent.runtimeConfig ?? {}) as Record; + const existingHb = (existingRc.heartbeat ?? {}) as Record; + patch.runtimeConfig = { ...existingRc, heartbeat: { ...existingHb, ...overlay.heartbeat } }; + } + + if (Object.keys(overlay.runtime).length > 0) { + Object.assign(patch, overlay.runtime); + } + + return patch; +}