diff --git a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java index 3559224..65e42fd 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -593,7 +593,9 @@ public record BridgedConfig( * have already collapsed last-wins — a duplicated slot name would silently drop one slot and the * daemon would never know. Jackson's YAML parser does not fail on duplicate mapping keys by * default, so duplicates are caught here, at parse time, before the map is built. Only the - * {@code architects:} block is walked, so parsing of the rest of the config is unaffected. + * top-level {@code architects:} block is considered, and only its direct child keys (the + * slot names) — a nested field elsewhere, even one also named {@code architects:}, is ignored, so + * parsing of the rest of the config is unaffected. * * @throws IllegalStateException when two {@code architects:} entries share a slot name, naming it */ @@ -602,31 +604,83 @@ public record BridgedConfig( if (p.nextToken() != JsonToken.START_OBJECT) { return; // not a mapping at top level — readValue reports the malformed file } + // Scan the TOP-LEVEL mapping only. Every other field's value (however deep, including + // any nested field also literally named "architects") is consumed whole by skipValue, so + // the loop below can only ever see the top-level field names — a nested `architects:` can + // neither suppress the real block nor be misread as one. JsonToken t; - while ((t = p.nextToken()) != null) { - if (t == JsonToken.FIELD_NAME && "architects".equals(p.getCurrentName())) { - if (p.nextToken() == JsonToken.START_OBJECT) { - Set seen = new HashSet<>(); - while ((t = p.nextToken()) != null && t != JsonToken.END_OBJECT) { - if (t == JsonToken.FIELD_NAME && !seen.add(p.getCurrentName())) { - throw new IllegalStateException("refusing to start: duplicate architect " - + "slot name '" + p.getCurrentName() + "' — slot names must be " - + "unique; a later entry would silently overwrite the earlier " - + "one"); - } - p.nextToken(); // the slot's value - p.skipChildren(); + while ((t = p.nextToken()) != null && t != JsonToken.END_OBJECT) { + if (t == JsonToken.FIELD_NAME) { + String name = p.getCurrentName(); + JsonToken value = p.nextToken(); + if ("architects".equals(name)) { + if (value == JsonToken.START_OBJECT) { + rejectDuplicateChildSlotKeys(p); } + return; // the single top-level architects block is handled; nothing more to check } - return; // the architects block (or its absence) is handled; nothing more to check + skipValue(p, value); } - p.skipChildren(); } } catch (IOException e) { // Not a duplicate-name condition — let readValue report the malformed file itself. } } + /** + * Reject a duplicated direct child key of the (already-positioned) {@code architects:} + * mapping — i.e. a duplicated {@code slot name}. + * + *

Each slot's value is consumed whole by {@link #skipValue}, so a duplicated field inside + * a slot (e.g. two {@code profile:} keys, or a duplicate nested {@code architects:}) is never seen + * here and cannot masquerade as a duplicated slot name. + * + * @throws IllegalStateException when two {@code architects:} entries share a slot name, naming it + */ + private static void rejectDuplicateChildSlotKeys(JsonParser p) throws IOException { + Set seen = new HashSet<>(); + JsonToken t; + while ((t = p.nextToken()) != null && t != JsonToken.END_OBJECT) { + if (t == JsonToken.FIELD_NAME) { + if (!seen.add(p.getCurrentName())) { + throw new IllegalStateException("refusing to start: duplicate architect slot name '" + + p.getCurrentName() + "' — slot names must be unique; a later entry would " + + "silently overwrite the earlier one"); + } + skipValue(p, p.nextToken()); // the slot's entire value + } + } + } + + /** + * Consume the whole value that starts at {@code start}, including every nested structure, and + * leave the parser positioned just past it. Used so depth is handled structurally rather than by + * a heuristic — a nested field is never interpreted as a top-level {@code architects:}. + */ + private static void skipValue(JsonParser p, JsonToken start) throws IOException { + switch (start) { + case START_OBJECT: { + JsonToken t; + while ((t = p.nextToken()) != null && t != JsonToken.END_OBJECT) { + if (t == JsonToken.FIELD_NAME) { + skipValue(p, p.nextToken()); + } + } + return; + } + case START_ARRAY: { + JsonToken t; + while ((t = p.nextToken()) != null && t != JsonToken.END_ARRAY) { + skipValue(p, t); + } + return; + } + default: + // A scalar (VALUE_* / VALUE_NULL) is already fully consumed by the nextToken that + // returned it — nothing further to skip. + } + } + /** * Log a WARN naming any top-level key this version does not understand (CB-530). * diff --git a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java index f11e404..fb8e17c 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -522,6 +522,63 @@ class BridgedConfigTest { assertEquals(Set.of("sonnet"), BridgedConfig.load(f).workerProfiles().keySet()); } + @Test + void aNestedArchitectsFieldDoesNotSuppressRealDuplicateDetection(@TempDir Path dir) throws Exception { + // A field ALSO named `architects` nested under another block carries its own duplicate and + // sits BEFORE the real top-level block. Only the top-level block is ever inspected: the + // refusal must name the real slot (lead-designer), not the nested one (nested-slot). + Path f = dir.resolve("nested-arch.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + architects: + nested-slot: + k: v + nested-slot: + k: v + architects: + lead-designer: + profile: sonnet + lead-designer: + profile: sonnet + """); + + IllegalStateException e = + assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("lead-designer"), + "the real top-level duplicate must be reported, was: " + e.getMessage()); + assertTrue(e.getMessage().contains("duplicate architect"), + "the refusal says the slot name is duplicated"); + } + + @Test + void nestedDuplicateFieldsInsideASlotAreNotDuplicateSlotNames(@TempDir Path dir) throws Exception { + // A duplicated field nested inside one slot's own value (here inside an ignored `extra:` + // sub-block) is not a duplicate SLOT name — it must not be rejected as one. Only the direct + // child keys of the architects mapping are slot names; whatever is deeper is the slot's + // business and must not masquerade as a duplicate slot. + Path f = dir.resolve("nested-dup-inside-slot.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + sonnet: + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + profile: sonnet + extra: + a: 1 + a: 1 + """); + + BridgedConfig cfg = assertDoesNotThrow(() -> BridgedConfig.load(f)); + assertDoesNotThrow(cfg::validateArchitects, + "a nested duplicate inside a slot is not a duplicate slot and must not refuse startup"); + assertEquals(Set.of("lead-designer"), cfg.architects().keySet()); + assertEquals("sonnet", cfg.architects().get("lead-designer").profile()); + } + @Test void absentBrokerBlockLeavesInboxSoftState(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-broker.yaml");