diff --git a/CHANGELOG.md b/CHANGELOG.md index dbe479a503..bd32eea869 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,7 +29,7 @@ Docs: https://docs.openclaw.ai - Web Fetch/Security: cap downloaded response body size before HTML parsing to prevent memory exhaustion from oversized or deeply nested pages. Thanks @xuemian168. - Infra/Fetch: ensure foreign abort-signal listener cleanup never masks original fetch successes/failures, while still preventing detached-finally unhandled rejection noise in `wrapFetchWithAbortSignal`. Thanks @Jackten. - 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. +- Gateway/Config: prevent `config.patch` object-array merges from falling back to full-array replacement when some patch entries lack `id`, so partial `agents.list` updates no longer drop 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. diff --git a/src/config/merge-patch.test.ts b/src/config/merge-patch.test.ts index 203227c33e..4c51578c17 100644 --- a/src/config/merge-patch.test.ts +++ b/src/config/merge-patch.test.ts @@ -72,7 +72,7 @@ describe("applyMergePatch", () => { agents: { list: [ { id: "primary", model: "new-model" }, - { workspace: "/tmp/orphan" }, // no id + { workspace: "/tmp/orphan" }, ], }, }; @@ -84,7 +84,6 @@ describe("applyMergePatch", () => { 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"); @@ -117,16 +116,55 @@ describe("applyMergePatch", () => { 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"); + const main = merged.agents?.list?.find((entry) => entry.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"); + expect(merged.agents?.list?.find((entry) => entry.id === "ota")?.workspace).toBe("/home/ota"); + expect(merged.agents?.list?.find((entry) => entry.id === "trading")?.workspace).toBe( + "/home/trading", + ); + expect(merged.agents?.list?.find((entry) => entry.id === "codex")?.workspace).toBe( + "/home/codex", + ); + }); + + it("keeps existing id entries when patch mixes id and primitive entries", () => { + const base = { + agents: { + list: [ + { id: "primary", workspace: "/tmp/one" }, + { id: "secondary", workspace: "/tmp/two" }, + ], + }, + }; + const patch = { + agents: { + list: [{ id: "primary", workspace: "/tmp/one-updated" }, "non-object entry"], + }, + }; + + const merged = applyMergePatch(base, patch, { + mergeObjectArraysById: true, + }) as { + agents?: { + list?: Array<{ id?: string; workspace?: string } | string>; + }; + }; + + expect(merged.agents?.list).toHaveLength(3); + const primary = merged.agents?.list?.find( + (entry): entry is { id?: string; workspace?: string } => + typeof entry === "object" && entry !== null && "id" in entry && entry.id === "primary", + ); + const secondary = merged.agents?.list?.find( + (entry): entry is { id?: string; workspace?: string } => + typeof entry === "object" && entry !== null && "id" in entry && entry.id === "secondary", + ); + expect(primary?.workspace).toBe("/tmp/one-updated"); + expect(secondary?.workspace).toBe("/tmp/two"); + expect(merged.agents?.list?.[2]).toBe("non-object entry"); }); it("falls back to replacement for non-id arrays even when enabled", () => { diff --git a/src/config/merge-patch.ts b/src/config/merge-patch.ts index 6e7627cde4..2afb4d62a0 100644 --- a/src/config/merge-patch.ts +++ b/src/config/merge-patch.ts @@ -13,25 +13,35 @@ function isObjectWithStringId(value: unknown): value is Record return typeof value.id === "string" && value.id.length > 0; } -function mergeObjectArraysById(base: unknown[], patch: unknown[], options: MergePatchOptions) { - // Require all *base* entries to have string ids — if the existing array - // isn't id-keyed there's nothing sensible to merge against. +/** + * Merge arrays of object-like entries keyed by `id`. + * + * Contract: + * - Base array must be fully id-keyed; otherwise return undefined (caller should replace). + * - Patch entries with valid id merge by id (or append when the id is new). + * - Patch entries without valid id append as-is, avoiding destructive full-array replacement. + */ +function mergeObjectArraysById( + base: unknown[], + patch: unknown[], + options: MergePatchOptions, +): unknown[] | undefined { if (!base.every(isObjectWithStringId)) { return undefined; } - const merged = [...base] as Array & { id: string }>; + const merged: unknown[] = [...base]; const indexById = new Map(); for (const [index, entry] of merged.entries()) { + if (!isObjectWithStringId(entry)) { + return undefined; + } indexById.set(entry.id, index); } for (const patchEntry of patch) { - // Patch entries without a valid id are appended as-is (best-effort). - // This prevents the entire merge from falling back to full replacement - // just because one patch element is missing an id field. if (!isObjectWithStringId(patchEntry)) { - merged.push(structuredClone(patchEntry) as Record & { id: string }); + merged.push(structuredClone(patchEntry)); continue; } @@ -41,10 +51,8 @@ function mergeObjectArraysById(base: unknown[], patch: unknown[], options: Merge indexById.set(patchEntry.id, merged.length - 1); continue; } - merged[existingIndex] = applyMergePatch(merged[existingIndex], patchEntry, options) as Record< - string, - unknown - > & { id: string }; + + merged[existingIndex] = applyMergePatch(merged[existingIndex], patchEntry, options); } return merged;