Merge pull request #3232 from officialasishkumar/fix/clear-empty-agent-env-bindings

fix(ui): persist cleared agent env bindings on save
2026-04-11 06:23:14 -05:00
parent 548721248e 44d94d0add
commit 62d05a7ae2
3 changed files with 185 additions and 55 deletions
@@ -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<string, unknown>;
-  adapterType?: string;
-  adapterConfig: Record<string, unknown>;
-  heartbeat: Record<string, unknown>;
-  runtime: Record<string, unknown>;
-}
-
-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<string, EnvBinding> = {};

-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<Overlay>(emptyOverlay);
+  const [overlay, setOverlay] = useState<AgentConfigOverlay>(emptyOverlay);
  const agentRef = useRef<Agent | null>(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<T>(group: keyof Omit<Overlay, "adapterType">, field: string, original: T): T {
+  function eff<T>(group: keyof Omit<AgentConfigOverlay, "adapterType">, 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<Overlay, "adapterType">, field: string, value: unknown) {
+  function mark(group: keyof Omit<AgentConfigOverlay, "adapterType">, 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<string, unknown> = {};
-
-    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<string, unknown>;
-      const adapterAgnosticKeys = [
-        "env",
-        "promptTemplate",
-        "instructionsFilePath",
-        "cwd",
-        "timeoutSec",
-        "graceSec",
-        "bootstrapPromptTemplate",
-      ];
-      const preserved: Record<string, unknown> = {};
-      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<string, unknown>;
-      patch.adapterConfig = { ...existing, ...overlay.adapterConfig };
-    }
-    if (Object.keys(overlay.heartbeat).length > 0) {
-      const existingRc = (agent.runtimeConfig ?? {}) as Record<string, unknown>;
-      const existingHb = (existingRc.heartbeat ?? {}) as Record<string, unknown>;
-      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(() => {
@@ -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>): 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,
+    });
+  });
+});
@@ -0,0 +1,70 @@
+import type { Agent } from "@paperclipai/shared";
+
+export interface AgentConfigOverlay {
+  identity: Record<string, unknown>;
+  adapterType?: string;
+  adapterConfig: Record<string, unknown>;
+  heartbeat: Record<string, unknown>;
+  runtime: Record<string, unknown>;
+}
+
+const ADAPTER_AGNOSTIC_KEYS = [
+  "env",
+  "promptTemplate",
+  "instructionsFilePath",
+  "cwd",
+  "timeoutSec",
+  "graceSec",
+  "bootstrapPromptTemplate",
+] as const;
+
+function omitUndefinedEntries(value: Record<string, unknown>) {
+  return Object.fromEntries(
+    Object.entries(value).filter(([, entryValue]) => entryValue !== undefined),
+  );
+}
+
+export function buildAgentUpdatePatch(agent: Agent, overlay: AgentConfigOverlay) {
+  const patch: Record<string, unknown> = {};
+
+  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<string, unknown>;
+    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<string, unknown>;
+    const existingHb = (existingRc.heartbeat ?? {}) as Record<string, unknown>;
+    patch.runtimeConfig = { ...existingRc, heartbeat: { ...existingHb, ...overlay.heartbeat } };
+  }
+
+  if (Object.keys(overlay.runtime).length > 0) {
+    Object.assign(patch, overlay.runtime);
+  }
+
+  return patch;
+}