fix(config): prevent config.patch from destroying arrays when patch entries lack id (#18030)

Merged via /review-pr -> /prepare-pr -> /merge-pr.

Prepared head SHA: a857df9e32
Co-authored-by: stakeswky <64798754+stakeswky@users.noreply.github.com>
Co-authored-by: sebslight <19554889+sebslight@users.noreply.github.com>
Reviewed-by: @sebslight

CHANGELOG.md

@@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai
 - Skills/Linux: harden go installer fallback on apt-based systems by handling root/no-sudo environments safely, doing best-effort apt index refresh, and returning actionable errors instead of failing with spawn errors. (#17687) Thanks @mcrolly.
 - Web Fetch/Security: cap downloaded response body size before HTML parsing to prevent memory exhaustion from oversized or deeply nested pages. Thanks @xuemian168.
 - Config/Gateway: make sensitive-key whitelist suffix matching case-insensitive while preserving `passwordFile` path exemptions, preventing accidental redaction of non-secret config values like `maxTokens` and IRC password-file paths. (#16042) Thanks @akramcodez.
+- Gateway/Config: prevent `config.patch` from falling back to full-array replacement when an object-array patch includes entries without `id`, so partial `agents.list` updates no longer risk deleting unrelated agents. (#17989) Thanks @stakeswky.
 - Dev tooling: harden git `pre-commit` hook against option injection from malicious filenames (for example `--force`), preventing accidental staging of ignored files. Thanks @mrthankyou.
 - Gateway/Agent: reject malformed `agent:`-prefixed session keys (for example, `agent:main`) in `agent` and `agent.identity.get` instead of silently resolving them to the default agent, preventing accidental cross-session routing. (#15707) Thanks @rodrigouroz.
 - Gateway/Chat: harden `chat.send` inbound message handling by rejecting null bytes, stripping unsafe control characters, and normalizing Unicode to NFC before dispatch. (#8593) Thanks @fr33d3m0n.

src/config/merge-patch.test.ts

@@ -59,6 +59,76 @@ describe("applyMergePatch", () => {
     expect(secondary?.workspace).toBe("/tmp/two");
   });
 
+  it("merges by id even when patch entries lack id (appends them)", () => {
+    const base = {
+      agents: {
+        list: [
+          { id: "primary", workspace: "/tmp/one" },
+          { id: "secondary", workspace: "/tmp/two" },
+        ],
+      },
+    };
+    const patch = {
+      agents: {
+        list: [
+          { id: "primary", model: "new-model" },
+          { workspace: "/tmp/orphan" }, // no id
+        ],
+      },
+    };
+
+    const merged = applyMergePatch(base, patch, {
+      mergeObjectArraysById: true,
+    }) as {
+      agents?: {
+        list?: Array<{ id?: string; workspace?: string; model?: string }>;
+      };
+    };
+    // Both original entries preserved, patch without id appended
+    expect(merged.agents?.list).toHaveLength(3);
+    const primary = merged.agents?.list?.find((entry) => entry.id === "primary");
+    expect(primary?.workspace).toBe("/tmp/one");
+    expect(primary?.model).toBe("new-model");
+    expect(merged.agents?.list?.[1]?.id).toBe("secondary");
+    expect(merged.agents?.list?.[2]?.workspace).toBe("/tmp/orphan");
+  });
+
+  it("does not destroy agents list when patching a single agent by id", () => {
+    const base = {
+      agents: {
+        list: [
+          { id: "main", default: true, workspace: "/home/main" },
+          { id: "ota", workspace: "/home/ota" },
+          { id: "trading", workspace: "/home/trading" },
+          { id: "codex", workspace: "/home/codex" },
+        ],
+      },
+    };
+    const patch = {
+      agents: {
+        list: [{ id: "main", model: "claude-opus-4-20250918" }],
+      },
+    };
+
+    const merged = applyMergePatch(base, patch, {
+      mergeObjectArraysById: true,
+    }) as {
+      agents?: {
+        list?: Array<{ id?: string; workspace?: string; model?: string; default?: boolean }>;
+      };
+    };
+    // All 4 agents must survive
+    expect(merged.agents?.list).toHaveLength(4);
+    const main = merged.agents?.list?.find((e) => e.id === "main");
+    expect(main?.model).toBe("claude-opus-4-20250918");
+    expect(main?.default).toBe(true);
+    expect(main?.workspace).toBe("/home/main");
+    // Others untouched
+    expect(merged.agents?.list?.find((e) => e.id === "ota")?.workspace).toBe("/home/ota");
+    expect(merged.agents?.list?.find((e) => e.id === "trading")?.workspace).toBe("/home/trading");
+    expect(merged.agents?.list?.find((e) => e.id === "codex")?.workspace).toBe("/home/codex");
+  });
+
   it("falls back to replacement for non-id arrays even when enabled", () => {
     const base = {
       channels: {

src/config/merge-patch.ts

@@ -14,23 +14,34 @@ function isObjectWithStringId(value: unknown): value is Record<string, unknown>
 }
 
 function mergeObjectArraysById(base: unknown[], patch: unknown[], options: MergePatchOptions) {
-  if (!base.every(isObjectWithStringId) || !patch.every(isObjectWithStringId)) {
+  // Require all *base* entries to have string ids — if the existing array
+  // isn't id-keyed there's nothing sensible to merge against.
+  if (!base.every(isObjectWithStringId)) {
     return undefined;
   }
+
   const merged = [...base] as Array<Record<string, unknown> & { id: string }>;
   const indexById = new Map<string, number>();
   for (const [index, entry] of merged.entries()) {
     indexById.set(entry.id, index);
   }
 
-  for (const entry of patch) {
+  for (const patchEntry of patch) {
-    const existingIndex = indexById.get(entry.id);
+    // Patch entries without a valid id are appended as-is (best-effort).
-    if (existingIndex === undefined) {
+    // This prevents the entire merge from falling back to full replacement
-      merged.push(structuredClone(entry));
+    // just because one patch element is missing an id field.
-      indexById.set(entry.id, merged.length - 1);
+    if (!isObjectWithStringId(patchEntry)) {
+      merged.push(structuredClone(patchEntry) as Record<string, unknown> & { id: string });
       continue;
     }
-    merged[existingIndex] = applyMergePatch(merged[existingIndex], entry, options) as Record<
+
+    const existingIndex = indexById.get(patchEntry.id);
+    if (existingIndex === undefined) {
+      merged.push(structuredClone(patchEntry));
+      indexById.set(patchEntry.id, merged.length - 1);
+      continue;
+    }
+    merged[existingIndex] = applyMergePatch(merged[existingIndex], patchEntry, options) as Record<
       string,
       unknown
     > & { id: string };

src/gateway/server.config-patch.e2e.test.ts

@@ -91,6 +91,47 @@ describe("gateway config methods", () => {
     expect(primary?.workspace).toBe("/tmp/primary-updated");
     expect(secondary?.workspace).toBe("/tmp/secondary");
   });
+
+  it("rejects mixed-id agents.list patches without mutating persisted config", async () => {
+    const setRes = await rpcReq<{ ok?: boolean }>(ws, "config.set", {
+      raw: JSON.stringify({
+        agents: {
+          list: [
+            { id: "primary", default: true, workspace: "/tmp/primary" },
+            { id: "secondary", workspace: "/tmp/secondary" },
+          ],
+        },
+      }),
+    });
+    expect(setRes.ok).toBe(true);
+
+    const beforeRes = await rpcReq<{ hash?: string }>(ws, "config.get", {});
+    expect(beforeRes.ok).toBe(true);
+    expect(typeof beforeRes.payload?.hash).toBe("string");
+
+    const patchRes = await rpcReq<{ ok?: boolean }>(ws, "config.patch", {
+      baseHash: beforeRes.payload?.hash,
+      raw: JSON.stringify({
+        agents: {
+          list: [
+            {
+              id: "primary",
+              workspace: "/tmp/primary-updated",
+            },
+            {
+              workspace: "/tmp/orphan-no-id",
+            },
+          ],
+        },
+      }),
+    });
+    expect(patchRes.ok).toBe(false);
+    expect(patchRes.error?.message ?? "").toContain("invalid config");
+
+    const afterRes = await rpcReq<{ hash?: string }>(ws, "config.get", {});
+    expect(afterRes.ok).toBe(true);
+    expect(afterRes.payload?.hash).toBe(beforeRes.payload?.hash);
+  });
 });
 
 describe("gateway server sessions", () => {