From b588fc9d35ed6306d5d4339e583aa6bbf3d173c6 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 24 Jun 2026 21:32:15 +0200 Subject: [PATCH] fix(config): same-level @E config suppresses `*` instead of merging it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §3 step 1 treats an environment scope as *eligibility*, not a merge tier: within a single level a value scoped `@E` shadows the same level's `*` fallback — it never layers on top of it. The resolver's map-run loop collected every leading object row regardless of (depth, scope), so a level holding both an `@E` map and a `*` map would deep-merge the two (and report a bogus `*` layer in `merged_from`) rather than letting the `@E` map win alone. Dedup the sorted candidates by depth before the map run: each level is a single owner (single-parent tree) with at most one `@E` and one `*` row per key after env-filtering, and `@E` sorts first, so keeping the first row per level yields the level's winner. Scalar/tombstone cases already went through the boundary path and were unaffected; only same-level map-vs-map mis-merged. Adds two regression tests (same-level suppression; suppressed `*` stays invisible to cross-level merge). Found in checkpoint review. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/manager-core/src/config_resolver.rs | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/manager-core/src/config_resolver.rs b/crates/manager-core/src/config_resolver.rs index e595005..7ddd74c 100644 --- a/crates/manager-core/src/config_resolver.rs +++ b/crates/manager-core/src/config_resolver.rs @@ -115,6 +115,14 @@ fn resolve_one(mut rows: Vec) -> Option<(Value, Provenance)> { .then(env_priority(&b.scope).cmp(&env_priority(&a.scope))) }); + // §3 step 1: env is eligibility, not a merge tier — within a single level + // `@E` *suppresses* `*` (it does not layer on top of it). Each level is one + // owner (single-parent tree) with at most one `@E` and one `*` row per key + // after env-filtering; keep only the level's winner (the `@E` row sorts + // first). Without this, a level holding both an `@E` map and a `*` map would + // deep-merge them instead of the `@E` shadowing the `*`. + rows.dedup_by_key(|r| r.depth); + // Collect the contiguous run of map-valued rows from the nearest. A // tombstone or scalar/array boundary stops the run (and, if it's the // nearest row, decides the result outright). @@ -323,6 +331,35 @@ mod tests { assert_eq!(p["ui"].merged_from.len(), 2); } + #[test] + fn same_level_env_map_suppresses_agnostic_map_no_merge() { + // §3 step 1: within ONE level, `@E` is not a merge layer over `*` — it + // shadows it. A level holding both an `@staging` map and a `*` map must + // resolve to the `@staging` map alone, never a deep-merge of the two. + let (v, p) = resolve(vec![ + cand(1, "*", "cfg", json!({"a": 1, "shared": "default"})), + cand(1, "staging", "cfg", json!({"shared": "staging"})), + ]); + assert_eq!(v["cfg"], json!({"shared": "staging"})); + assert_eq!(p["cfg"].scope, "staging"); + // Provenance must not list the suppressed `*` layer. + assert_eq!(p["cfg"].merged_from, vec![(1, "staging".to_string())]); + } + + #[test] + fn same_level_env_map_then_ancestor_map_merges_without_agnostic_sibling() { + // The suppressed same-level `*` must also be invisible to cross-level + // merge: leaf `@staging` map merges onto the ancestor map, but the + // leaf's own `*` map (shadowed at its level) never contributes. + let (v, _) = resolve(vec![ + cand(2, "*", "cfg", json!({"region": "eu", "tier": "base"})), + cand(0, "*", "cfg", json!({"leak": "should-not-appear"})), + cand(0, "staging", "cfg", json!({"tier": "leaf"})), + ]); + assert_eq!(v["cfg"], json!({"region": "eu", "tier": "leaf"})); + assert!(!v["cfg"].as_object().unwrap().contains_key("leak")); + } + #[test] fn nearer_scalar_replaces_whole_inherited_map() { let (v, _) = resolve(vec![