From f8822aabdb96553ab10121badfbe068a18156a91 Mon Sep 17 00:00:00 2001 From: bellman Date: Fri, 5 Jun 2026 09:57:22 +0900 Subject: [PATCH] fix: config merge concatenates arrays instead of replacing (#106) deep_merge_objects now concatenates arrays when both layers provide the same key. Previously permissions.allow, hooks.PreToolUse, etc. from earlier config layers (e.g. ~/.claw/settings.json) were silently discarded when a later layer (e.g. project .claw/settings.json) set the same key. Now arrays are merged additively across layers. Generated with https://github.com/Yeachan-Heo/gajae-code Co-authored-by: Gajae Code --- ROADMAP.md | 2 +- rust/crates/runtime/src/config.rs | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index a54f42bc..b953d58a 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2931,7 +2931,7 @@ ear], /color [scheme], /effort [low|medium|high], /fast, /summary, /tag [label], **Source.** Jobdori dogfood 2026-04-18 against `/tmp/cdZ` on main HEAD `6580903` in response to Clawhip pinpoint nudge at `1494819785676947543`. Joins **truth-audit / diagnostic-integrity** (#80–#84, #86, #87, #89, #100, #102, #103) — status JSON lies about the active model. Joins **two-paths-diverge** (#91, #101, #104) — three separate model-resolution paths with incompatible outputs. Sibling of **#100** (status JSON missing commit identity) and **#102** (doctor silent on MCP reachability) — same pattern: status/doctor surfaces incomplete or wrong information about things they claim to report. Natural bundle: **#100 + #102 + #105** — status/doctor surface completeness triangle (commit identity + MCP reachability + model-resolution truth). Also **#91 + #101 + #104 + #105** — four-way parallel-entry-point asymmetry (config↔CLI parser, CLI↔env silent-vs-loud, slash↔CLI export, config↔status↔dispatch model). Session tally: ROADMAP #105. -106. **Config merge uses `deep_merge_objects` which recurses into nested objects but REPLACES arrays — so `permissions.allow`, `permissions.deny`, `permissions.ask`, `hooks.PreToolUse`, `hooks.PostToolUse`, `hooks.PostToolUseFailure`, and `plugins.externalDirectories` from an earlier config layer are silently discarded whenever a later layer sets the same key. A user-home `~/.claw/settings.json` with `permissions.deny: ["Bash(rm *)"]` is silently overridden by a project `.claw.json` with `permissions.deny: ["Bash(sudo *)"]` — the user's `Bash(rm *)` deny is GONE and never surfaced. Worse: a workspace-local `.claw/settings.local.json` with `permissions.deny: []` silently removes every deny rule from every layer above it** — dogfooded 2026-04-18 on main HEAD `71e7729` from `/tmp/cdAA`. MCP servers *are* merged by-key (distinct server names from different layers coexist), but permission-rule arrays and hook arrays are NOT — they are last-writer-wins for the entire list. This makes claw-code's config merge incompatible with any multi-tier permission policy (team default → project override → local tweak) that a security-conscious team would want, and it is the exact failure mode #91 / #94 / #101 warned about on adjacent axes. +106. **DONE — Config merge uses `deep_merge_objects` which recurses into nested objects but REPLACES arrays — so `permissions.allow`, `permissions.deny`, `permissions.ask`, `hooks.PreToolUse`, `hooks.PostToolUse`, `hooks.PostToolUseFailure`, and `plugins.externalDirectories` from an earlier config layer are silently discarded whenever a later layer sets the same key. A user-home `~/.claw/settings.json` with `permissions.deny: ["Bash(rm *)"]` is silently overridden by a project `.claw.json` with `permissions.deny: ["Bash(sudo *)"]` — the user's `Bash(rm *)` deny is GONE and never surfaced. Worse: a workspace-local `.claw/settings.local.json` with `permissions.deny: []` silently removes every deny rule from every layer above it** — dogfooded 2026-04-18 on main HEAD `71e7729` from `/tmp/cdAA`. MCP servers *are* merged by-key (distinct server names from different layers coexist), but permission-rule arrays and hook arrays are NOT — they are last-writer-wins for the entire list. This makes claw-code's config merge incompatible with any multi-tier permission policy (team default → project override → local tweak) that a security-conscious team would want, and it is the exact failure mode #91 / #94 / #101 warned about on adjacent axes. **Concrete repro.** ``` diff --git a/rust/crates/runtime/src/config.rs b/rust/crates/runtime/src/config.rs index 8d3b515b..3a2e6f5b 100644 --- a/rust/crates/runtime/src/config.rs +++ b/rust/crates/runtime/src/config.rs @@ -2412,6 +2412,10 @@ fn deep_merge_objects( (Some(JsonValue::Object(existing)), JsonValue::Object(incoming)) => { deep_merge_objects(existing, incoming); } + // #106: concatenate arrays instead of replacing + (Some(JsonValue::Array(existing)), JsonValue::Array(incoming)) => { + existing.extend(incoming.iter().cloned()); + } _ => { target.insert(key.clone(), value.clone()); } @@ -3385,14 +3389,19 @@ mod tests { .load() .expect("config should load valid hook entries and record invalid siblings"); - assert_eq!(loaded.hooks().pre_tool_use(), &["project".to_string()]); + // #106: arrays now concatenate across config layers, so both "base" and "project" are present + assert_eq!( + loaded.hooks().pre_tool_use(), + &["base".to_string(), "project".to_string()] + ); assert_eq!(loaded.hooks().invalid_count(), 1); assert_eq!(loaded.hooks().invalid_hooks()[0].event, "PreToolUse"); assert_eq!( loaded.hooks().invalid_hooks()[0].kind, "invalid_hooks_config" ); - assert_eq!(loaded.hooks().invalid_hooks()[0].index, Some(1)); + // #106: invalid entry at index 2 after array concatenation + assert_eq!(loaded.hooks().invalid_hooks()[0].index, Some(2)); assert!(loaded.hooks().invalid_hooks()[0] .reason .contains("must be a string or hook object"));