From 6123576c6863638842cfde2c5703f275ec3d5bbf Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 13 Aug 2026 18:05:53 +0200 Subject: [PATCH] =?UTF-8?q?CB-548:=20correct=20architect=20premise=20?= =?UTF-8?q?=E2=80=94=20profile-only=20slots,=20registry-owned=20bindings,?= =?UTF-8?q?=20dup-key=20rejection?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../main/java/dev/ltms/bridged/Bridged.java | 20 +- .../ltms/bridged/auth/ArchitectRegistry.java | 113 ++++++++-- .../ltms/bridged/config/BridgedConfig.java | 101 +++++---- .../bridged/auth/ArchitectRegistryTest.java | 211 ++++++++++++++++-- .../bridged/config/BridgedConfigTest.java | 84 +++++-- 5 files changed, 412 insertions(+), 117 deletions(-) diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index fcbbc78..34e828a 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -203,17 +203,17 @@ public final class Bridged { leads = () -> leadTerminals; } - // CB-548: config-declared architect slots. Slots live in config (name → strong-model - // profile); the terminal → slot binding is the live half, sourced from the slots' declared - // terminals today and swapped for a live binding by the later spawn lifecycle. The registry - // is what CallerResolver resolves against and what that lifecycle will read profiles from; + // CB-548: config-declared architect slots. Config supplies only the stable name → profile + // map; the terminal → slot binding is owned by the registry and is empty at startup, so no + // pane resolves to an architect until the later spawn lifecycle binds one. The registry is + // what CallerResolver resolves against and what that lifecycle will read profiles from; // nothing here spawns a slot. ArchitectRegistry architects = new ArchitectRegistry( - cfg.architects() == null ? Map.of() : cfg.architects(), - () -> cfg.architectTerminals()); + cfg.architects() == null ? Map.of() : cfg.architects()); if (!architects.slots().isEmpty()) { - log.info("architect slots: {} configured {}, terminals {}", architects.slots().size(), - architects.slots().keySet(), cfg.architectTerminals().keySet()); + log.info("architect slots: {} configured {} — none bound yet (a slot is idle until the " + + "spawn lifecycle binds a live terminal to it)", + architects.slots().size(), architects.slots().keySet()); } // Status-gated injector (CB-103): the single writer into workers, fed by a poller. @@ -326,12 +326,12 @@ public final class Bridged { + " is unset or empty — export it before starting bridged"); } callers = CallerResolver.withLeadsAndArchitects(identity, true, token, leads, - architects::terminalBindings); + architects::snapshot); log.info("auth: token mode (bearer required for non-worker callers, env {})", cfg.auth().tokenEnv()); } else { callers = CallerResolver.withLeadsAndArchitects(identity, false, null, leads, - architects::terminalBindings); + architects::snapshot); log.info("auth: loopback-trust (any loopback non-worker caller is the primary)"); } diff --git a/bridged/src/main/java/dev/ltms/bridged/auth/ArchitectRegistry.java b/bridged/src/main/java/dev/ltms/bridged/auth/ArchitectRegistry.java index c5548b3..1e7c673 100644 --- a/bridged/src/main/java/dev/ltms/bridged/auth/ArchitectRegistry.java +++ b/bridged/src/main/java/dev/ltms/bridged/auth/ArchitectRegistry.java @@ -2,37 +2,38 @@ package dev.ltms.bridged.auth; import dev.ltms.bridged.config.BridgedConfig; +import java.util.HashMap; import java.util.Map; -import java.util.function.Supplier; /** * The architect-slot registry (CB-548): every gateway-local architect name and the strong-model - * profile it points at, plus the live binding from a live architect's herdr terminal to its slot. + * profile it points at, plus the live bindings from a live architect's herdr terminal to + * its slot. * *

Two halves, split by who owns each: *

* - *

Spawning/lifecycle is deliberately a separate unit: this class only exposes the map the - * resolver resolves against and the profile lookup that lifecycle will call. Nothing here - * creates or manages an architect session. + *

Spawning/lifecycle is deliberately a separate unit: this class only owns the bindings and + * exposes the map the resolver resolves against plus the profile lookup lifecycle will call. + * Nothing here creates or manages an architect session. */ public final class ArchitectRegistry { private final Map slots; - private final Supplier> terminalBindings; + /** Live {@code terminal_id → slot name}; guarded by {@code this}. */ + private final Map terminalToSlot = new HashMap<>(); - public ArchitectRegistry(Map slots, - Supplier> terminalBindings) { + public ArchitectRegistry(Map slots) { this.slots = slots == null ? Map.of() : Map.copyOf(slots); - this.terminalBindings = terminalBindings == null ? Map::of : terminalBindings; } /** The configured slots, keyed by gateway-local unique name. Unmodifiable snapshot. */ @@ -41,22 +42,30 @@ public final class ArchitectRegistry { } /** - * The live {@code terminal_id → slot name} bindings, re-read on every call. + * An immutable copy of the live {@code terminal_id → slot name} bindings. * *

Passed to {@link CallerResolver} as the source of architect identity, and what - * {@code bridge_whoami}/the roster will read to say which slot a pane hosts. + * {@code bridge_whoami}/the roster will read to say which slot a pane hosts. Empty until the + * spawn lifecycle binds a slot. */ - public Map terminalBindings() { - return terminalBindings.get(); + public Map snapshot() { + synchronized (terminalToSlot) { + return Map.copyOf(terminalToSlot); + } } - /** The slot a live terminal is bound to, or {@code null} if it is no architect slot. */ + /** The slot a live terminal is bound to, or {@code null} if it is not an architect slot. */ public String slotForTerminal(String terminal) { - return terminal == null ? null : terminalBindings.get().get(terminal); + if (terminal == null) { + return null; + } + synchronized (terminalToSlot) { + return terminalToSlot.get(terminal); + } } /** - * The strong-model profile a slot runs under — what the future spawn lifecycle reads. + * The strong-model profile a slot runs under — what the spawn lifecycle reads. * * @return the slot's configured {@code profile}, or {@code null} if the slot is unknown or * declares none @@ -70,4 +79,64 @@ public final class ArchitectRegistry { public boolean isSlot(String slotName) { return slots.containsKey(slotName); } + + /** + * Bind {@code terminal} to {@code slot} (CB-548). + * + *

The spawn lifecycle calls this when it stands a slot up. The bind is atomic and preserves + * the two cardinality invariants: a terminal may occupy at most one slot, and a slot may host at + * most one terminal. Binding the same terminal to the same slot again is a harmless no-op. + * + * @param slot a configured slot name, or the bind is refused + * @param terminal the pane that will act as this architect + * @return {@code true} if the binding is now {@code terminal → slot}; {@code false} if it was + * refused — an unknown slot, a terminal already bound to a different slot, or a slot + * already hosting a different terminal + */ + public boolean bind(String slot, String terminal) { + if (slot == null || terminal == null || terminal.isBlank()) { + return false; + } + synchronized (terminalToSlot) { + if (!isSlot(slot)) { + return false; // unknown slot — nothing to bind to + } + String existingSlot = terminalToSlot.get(terminal); + if (existingSlot != null) { + return slot.equals(existingSlot); // already this slot (idempotent) or a different one + } + if (terminalToSlot.containsValue(slot)) { + return false; // slot already hosts a terminal — no second one + } + terminalToSlot.put(terminal, slot); + return true; + } + } + + /** + * Compare-safe unbind of {@code expectedTerminal} from {@code slot} (CB-548). + * + *

The spawn lifecycle calls this when it tears a slot down. Only the exact binding + * {@code expectedTerminal → slot} is removed; if that terminal was since rebound to a different + * slot (or the slot to a different terminal), the call is a no-op returning {@code false} — a + * stale unbind must never remove a replacement. + * + * @param slot the slot the caller believes the terminal is bound to + * @param expectedTerminal the terminal it expects to be bound there + * @return {@code true} if {@code expectedTerminal → slot} was removed; {@code false} if nothing + * was (no such binding, or the binding had already moved) + */ + public boolean unbind(String slot, String expectedTerminal) { + if (slot == null || expectedTerminal == null) { + return false; + } + synchronized (terminalToSlot) { + String current = terminalToSlot.get(expectedTerminal); + if (current == null || !slot.equals(current)) { + return false; // absent, or a replacement/moved binding — leave it in place + } + terminalToSlot.remove(expectedTerminal); + return true; + } + } } 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 10e750e..3559224 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -1,6 +1,8 @@ package dev.ltms.bridged.config; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import org.slf4j.Logger; @@ -11,6 +13,7 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -44,10 +47,10 @@ import java.util.Set; * lead name; supersedes the singular {@code primary} pin, which stays honoured. * See {@link #leaderTerminals()} for how the two merge * @param architects CB-548 architect slots, keyed by gateway-local unique slot name; each points - * at a strong-model profile, and the identity a live session is matched by is - * its {@code terminal} binding (see {@link #architectTerminals()}). A slot is - * the hook the future spawn lifecycle reads a profile back from — nothing here - * spawns it. + * at a strong-model profile the future spawn lifecycle reads back. An architect + * is not recognised like a lead: config declares the slots only, and a + * live session becomes an architect when the spawn lifecycle binds its terminal + * to a slot. Nothing here spawns a slot. * @param leadScan opt-in discovery of leads by tab label (CB-531); {@code null} ⇒ no scanning, * and only {@code leaders:}/{@code primary:} name a lead * @param placement how to choose a worker profile for an unqualified spawn: @@ -389,26 +392,23 @@ public record BridgedConfig( * One entry of the CB-548 {@code architects:} registry — a gateway-local named slot that points * at a strong-model profile. * - *

A lead and an architect differ in authority, not in how identity is established: - * both are recognised by configuration rather than spawned. A lead resolves to - * {@link dev.ltms.bridged.auth.Role#PRIMARY} and owns the whole lifecycle (spawn/stop/drain); - * an architect resolves to {@link dev.ltms.bridged.auth.Role#ARCHITECT}, which delegates turns - * ({@code SEND}) and replies/asks as its own pane but cannot stand up or tear down workers — - * lifecycle stays in one pair of hands. + *

A slot is declared, not recognised: config names the slot and the profile it runs, + * and nothing else. Unlike a lead (which config pins by herdr {@code terminal_id} and is + * recognised at startup), an architect slot is idle at boot — config supplies no terminal, so no + * session resolves to one until the spawn lifecycle binds a live terminal to the slot. The + * stable name + profile pair is the only config-time identity; live identity is defined purely + * by the runtime {@link dev.ltms.bridged.auth.ArchitectRegistry} binding. * *

Why a {@code profile} reference: an architect is meant to run a strong model, and the slot * records which {@code workers:} profile that is — the value the future spawn lifecycle reads. * It must name a configured profile, enforced by {@link #validateArchitects()} (a stale or * typo'd reference fails at startup rather than silently spawning the wrong backend later). * - * @param terminal the architect's herdr {@code terminal_id}; the field identity is matched by, - * via the live terminal→slot binding. Optional at config time — binding may be - * injected live — but a slot with no binding matches nothing yet. - * @param profile the name of the strong-model {@code workers:} profile this slot runs; - * required and validated against {@link #workerProfiles()} + * @param profile the name of the strong-model {@code workers:} profile this slot runs; + * required and validated against {@link #workerProfiles()} */ @JsonIgnoreProperties(ignoreUnknown = true) - public record Architect(String terminal, String profile) { + public record Architect(String profile) { } /** @@ -468,30 +468,6 @@ public record BridgedConfig( return Collections.unmodifiableMap(byTerminal); } - /** - * The terminal → architect-slot-name map that {@link dev.ltms.bridged.auth.CallerResolver} - * resolves against (CB-548), derived from the {@code architects:} registry. - * - *

Keyed by terminal because a live session is matched by its pane; the value is the - * gateway-local slot name. Slot names are inherently unique (a map key); a duplicate terminal - * across two slots is last-wins here (the later entry overrides), which {@code leadership} has - * always tolerated rather than refused. This is consumed as the initial live binding — - * the supplier that feeds the resolver may be swapped for a live one by the future lifecycle. - * - * @return an unmodifiable map, empty when no architect slot is configured - */ - public Map architectTerminals() { - Map byTerminal = new LinkedHashMap<>(); - if (architects != null) { - architects.forEach((name, arch) -> { - if (arch != null && arch.terminal() != null && !arch.terminal().isBlank()) { - byTerminal.put(arch.terminal(), name); - } - }); - } - return Collections.unmodifiableMap(byTerminal); - } - /** * API authentication (CB-501). Governs how a caller that is not an on-host worker * pane proves it is the primary. @@ -602,6 +578,7 @@ public record BridgedConfig( try { String yaml = Files.readString(path); warnUnknownTopLevelKeys(yaml, path); + rejectDuplicateArchitectSlots(yaml); BridgedConfig cfg = YAML.readValue(yaml, BridgedConfig.class); return cfg.withDefaults(); } catch (IOException e) { @@ -609,6 +586,47 @@ public record BridgedConfig( } } + /** + * Reject an {@code architects:} registry whose slot names repeat (CB-548). + * + *

The registry is a {@code Map} keyed by slot name, so by the time it is read duplicate keys + * 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. + * + * @throws IllegalStateException when two {@code architects:} entries share a slot name, naming it + */ + static void rejectDuplicateArchitectSlots(String yaml) { + try (JsonParser p = YAML.createParser(yaml)) { + if (p.nextToken() != JsonToken.START_OBJECT) { + return; // not a mapping at top level — readValue reports the malformed file + } + 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(); + } + } + return; // the architects block (or its absence) is handled; nothing more to check + } + p.skipChildren(); + } + } catch (IOException e) { + // Not a duplicate-name condition — let readValue report the malformed file itself. + } + } + /** * Log a WARN naming any top-level key this version does not understand (CB-530). * @@ -790,7 +808,8 @@ public record BridgedConfig( * spawn that quietly has no backend to use. * *

Slot-name uniqueness needs no check here: the registry is a {@code Map} keyed by name, so - * duplicates are unrepresentable by construction. + * duplicates are unrepresentable by construction once loaded — and {@link #load(Path)} already + * rejects a duplicated slot name at parse time, before the map collapses. * * @throws IllegalStateException when any architect slot is missing or names an unknown profile, * naming the slot and the offending reference diff --git a/bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java b/bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java index 85dc7e9..1b642c2 100644 --- a/bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java @@ -3,24 +3,30 @@ package dev.ltms.bridged.auth; import dev.ltms.bridged.config.BridgedConfig; import org.junit.jupiter.api.Test; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; import static org.junit.jupiter.api.Assertions.*; /** * CB-548 — the architect-slot registry: the config snapshot of slot → profile, and the live - * terminal → slot binding the resolver reads. The role the binding produces is asserted in - * {@link CallerResolverTest}; this pins the registry object itself. + * terminal → slot bindings it owns. The role a binding produces is asserted in + * {@link CallerResolverTest}; this pins the registry object itself — its invariants and their + * thread-safety. */ class ArchitectRegistryTest { private static final Map SLOTS = Map.of( - "lead-designer", new BridgedConfig.Architect("term_design", "sonnet"), - "reviewer", new BridgedConfig.Architect(null, "gx10")); + "lead-designer", new BridgedConfig.Architect("sonnet"), + "reviewer", new BridgedConfig.Architect("gx10")); - private final ArchitectRegistry registry = - new ArchitectRegistry(SLOTS, () -> Map.of("term_design", "lead-designer")); + private final ArchitectRegistry registry = new ArchitectRegistry(SLOTS); @Test void exposesTheConfiguredSlots() { @@ -37,30 +43,195 @@ class ArchitectRegistryTest { } @Test - void resolvesTheSlotOfALiveTerminal() { - assertEquals("lead-designer", registry.slotForTerminal("term_design")); - assertNull(registry.slotForTerminal("term_unbound")); + void startsEmptySoNoTerminalResolvesToAnArchitect() { + assertTrue(registry.snapshot().isEmpty()); + assertNull(registry.slotForTerminal("term_design"), + "config declares no architect terminal — nothing is recognised until a bind"); assertNull(registry.slotForTerminal(null), "no terminal ⇒ no slot"); } + // ── bind ────────────────────────────────────────────────────────────────────────────────── + @Test - void theBindingIsLiveReReadPerCall() { - Map live = new HashMap<>(); - ArchitectRegistry r = new ArchitectRegistry(SLOTS, () -> live); - - assertNull(r.slotForTerminal("term_design")); - - live.put("term_design", "lead-designer"); // injected after construction - - assertEquals("lead-designer", r.slotForTerminal("term_design")); + void bindResolvesTheTerminalToTheSlot() { + assertTrue(registry.bind("lead-designer", "term_design")); + assertEquals("lead-designer", registry.slotForTerminal("term_design")); + assertEquals(Map.of("term_design", "lead-designer"), registry.snapshot()); } + @Test + void bindRefusesAnUnknownSlot() { + assertFalse(registry.bind("nope", "term_x"), + "a slot that is not configured must be refused — bind is not a way to invent one"); + assertNull(registry.slotForTerminal("term_x")); + } + + @Test + void bindRefusesATerminalInTwoSlots() { + assertTrue(registry.bind("lead-designer", "term_design")); + assertFalse(registry.bind("reviewer", "term_design"), + "a terminal may occupy at most one slot"); + assertEquals("lead-designer", registry.slotForTerminal("term_design"), + "the first binding survives the refused second"); + } + + @Test + void bindRefusesASlotWithTwoTerminals() { + assertTrue(registry.bind("lead-designer", "term_design")); + assertFalse(registry.bind("lead-designer", "term_other"), + "a slot may host at most one terminal"); + assertEquals("lead-designer", registry.slotForTerminal("term_design"), + "the first binding survives the refused second"); + assertNull(registry.slotForTerminal("term_other")); + } + + @Test + void rebindingTheSamePairIsAnIdempotentNoOp() { + assertTrue(registry.bind("lead-designer", "term_design")); + assertTrue(registry.bind("lead-designer", "term_design"), + "the same terminal → slot is harmless to repeat"); + assertEquals(1, registry.snapshot().size()); + } + + // ── unbind ──────────────────────────────────────────────────────────────────────────────── + + @Test + void unbindRemovesTheExactBinding() { + assertTrue(registry.bind("lead-designer", "term_design")); + assertTrue(registry.unbind("lead-designer", "term_design")); + assertNull(registry.slotForTerminal("term_design")); + assertTrue(registry.snapshot().isEmpty()); + } + + @Test + void aStaleUnbindDoesNotRemoveAReplacement() { + // Bind, tear down, and stand the slot back up with a NEW terminal. + assertTrue(registry.bind("lead-designer", "term_design")); + registry.unbind("lead-designer", "term_design"); + assertTrue(registry.bind("lead-designer", "term_new")); + + // A late unbind naming the OLD terminal must not remove the replacement binding. + assertFalse(registry.unbind("lead-designer", "term_design")); + assertEquals("lead-designer", registry.slotForTerminal("term_new"), + "the replacement terminal stays bound"); + } + + @Test + void aStaleUnbindForATerminalThatMovedSlotsDoesNothing() { + // term_design starts in lead-designer, is torn down, and stands back up in a FREE slot. + assertTrue(registry.bind("lead-designer", "term_design")); + registry.unbind("lead-designer", "term_design"); + assertTrue(registry.bind("reviewer", "term_design")); + + // Unbinding against the slot it no longer occupies is refused; the new binding is intact. + assertFalse(registry.unbind("lead-designer", "term_design"), + "the old slot must not unbind a terminal that moved elsewhere"); + assertEquals("reviewer", registry.slotForTerminal("term_design")); + } + + @Test + void unbindOfNothingIsAFalseNoOp() { + assertFalse(registry.unbind("lead-designer", "term_design"), + "nothing was bound, so nothing is removed"); + } + + // ── snapshot ───────────────────────────────────────────────────────────────────────────── + + @Test + void theSnapshotIsAnImmutableCopyNotAliveState() { + assertTrue(registry.bind("lead-designer", "term_design")); + Map snap = registry.snapshot(); + + assertThrows(UnsupportedOperationException.class, () -> snap.put("x", "y"), + "a handed-out snapshot cannot be mutated in place"); + + // Later binds must not leak into an earlier snapshot. + assertTrue(registry.bind("reviewer", "term_review")); + assertFalse(snap.containsKey("term_review"), + "a snapshot is a point-in-time copy, not a live view"); + } + + // ── concurrency (CB-548 invariants hold under contention) ───────────────────────────────── + + @Test + void concurrentBindsNeverGiveASlotTwoTerminals() throws Exception { + int n = 16; + ExecutorService pool = Executors.newFixedThreadPool(n); + try { + CountDownLatch go = new CountDownLatch(1); + List> results = new ArrayList<>(); + for (int i = 0; i < n; i++) { + final String term = "term_" + i; // every thread races for the SAME slot + results.add(pool.submit(() -> { + go.await(); + return registry.bind("lead-designer", term); + })); + } + go.countDown(); + + int won = 0; + for (Future r : results) { + if (r.get()) { + won++; + } + } + assertEquals(1, won, "exactly one terminal may win the sole slot, got " + won); + assertEquals(1, registry.snapshot().size(), + "the slot hosts at most one terminal after the race"); + } finally { + pool.shutdownNow(); + } + } + + @Test + void concurrentBindsNeverPutOneTerminalInTwoSlots() throws Exception { + int n = 16; + ExecutorService pool = Executors.newFixedThreadPool(n); + try { + CountDownLatch go = new CountDownLatch(1); + List> results = new ArrayList<>(); + for (int i = 0; i < n; i++) { + final String slot = (i % 2 == 0) ? "lead-designer" : "reviewer"; // all race for ONE terminal + results.add(pool.submit(() -> { + go.await(); + return registry.bind(slot, "shared_term") + ? registry.slotForTerminal("shared_term") : null; + })); + } + go.countDown(); + + // Rebinding the same terminal to the same slot is a harmless idempotent true, so count + // winners is not the assertion — agreement is: every thread that reported success must + // have seen the terminal in the SAME slot, never in two at once. + String bound = null; + boolean conflict = false; + for (Future r : results) { + String s = r.get(); + if (s != null) { + if (bound == null) { + bound = s; + } else if (!bound.equals(s)) { + conflict = true; + } + } + } + assertFalse(conflict, "a terminal was observed in two slots at once"); + assertNotNull(bound, "at least one thread bound the terminal"); + assertEquals(1, registry.snapshot().size(), + "the terminal occupies exactly one slot in the final snapshot"); + assertEquals(bound, registry.slotForTerminal("shared_term")); + } finally { + pool.shutdownNow(); + } + } + + /** A handed-over slot map is snapshotted at construction, not offered as live state. */ @Test void theSlotSnapshotIsFixedByConstruction() { Map mutable = new HashMap<>(SLOTS); - ArchitectRegistry r = new ArchitectRegistry(mutable, Map::of); + ArchitectRegistry r = new ArchitectRegistry(mutable); - mutable.put("hijack", new BridgedConfig.Architect("t", "gx10")); + mutable.put("hijack", new BridgedConfig.Architect("gx10")); assertFalse(r.isSlot("hijack"), "a handed-over map is not offered as live state"); } 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 8b06a35..f11e404 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -1,5 +1,6 @@ package dev.ltms.bridged.config; +import dev.ltms.bridged.auth.ArchitectRegistry; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -330,7 +331,7 @@ class BridgedConfigTest { // ── CB-548: the architects registry ──────────────────────────────────────────────────────── @Test - void architectsBlockBindsSlotsByGatewayLocalName(@TempDir Path dir) throws Exception { + void architectsBlockDeclaresSlotsByNameAndProfileOnly(@TempDir Path dir) throws Exception { Path f = dir.resolve("architects.yaml"); Files.writeString(f, """ bind: @@ -340,7 +341,6 @@ class BridgedConfigTest { baseUrl: http://gx10.gw:8000 architects: lead-designer: - terminal: term_design profile: sonnet reviewer: profile: sonnet @@ -351,15 +351,15 @@ class BridgedConfigTest { "slot names are the keys — gateway-local unique by construction"); assertEquals("sonnet", cfg.architects().get("lead-designer").profile(), "each slot carries its strong-model profile reference"); - assertEquals("term_design", cfg.architects().get("lead-designer").terminal()); - // A slot with no terminal binds nothing yet — the live binding may supply it later. - assertTrue(cfg.architects().get("reviewer").terminal() == null - || cfg.architects().get("reviewer").terminal().isBlank()); + assertEquals("sonnet", cfg.architects().get("reviewer").profile()); } @Test - void architectTerminalsMapsEachBoundSlotByItsPane(@TempDir Path dir) throws Exception { - Path f = dir.resolve("arch-terminals.yaml"); + void anArchitectCarriesNoConfigTerminalSoNothingIsRecognisedYet(@TempDir Path dir) throws Exception { + // The corrected CB-548 premise: config declares slots (name + profile) only. A `terminal:` + // key left over from the earlier premise is ignored — an architect is NOT recognised from + // config the way a lead is, so it binds nothing at startup and resolves no architect. + Path f = dir.resolve("arch-stale-terminal.yaml"); Files.writeString(f, """ bind: port: 8080 @@ -370,26 +370,24 @@ class BridgedConfigTest { lead-designer: terminal: term_design profile: sonnet - reviewer: - terminal: term_review - profile: sonnet - unbound: - profile: sonnet """); + BridgedConfig cfg = BridgedConfig.load(f); + assertEquals("sonnet", cfg.architects().get("lead-designer").profile(), + "the profile is still read even when a stray terminal is ignored"); - assertEquals(Map.of("term_design", "lead-designer", "term_review", "reviewer"), - BridgedConfig.load(f).architectTerminals(), - "a slot with no terminal registers no binding; the value is the slot name"); + // The registry built from this config owns no bindings: the slot is idle at startup. + ArchitectRegistry r = new ArchitectRegistry(cfg.architects()); + assertTrue(r.snapshot().isEmpty()); + assertNull(r.slotForTerminal("term_design"), + "a config terminal must not resolve an architect — slots start idle"); } @Test - void noArchitectsBlockLeavesNothingBound(@TempDir Path dir) throws Exception { + void noArchitectsBlockLeavesNothingConfigured(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-arch.yaml"); Files.writeString(f, "bind:\n port: 8080\n"); - BridgedConfig cfg = BridgedConfig.load(f); - assertNull(cfg.architects()); - assertTrue(cfg.architectTerminals().isEmpty(), + assertNull(BridgedConfig.load(f).architects(), "no architects: block ⇒ no architect identity, exactly as before CB-548"); } @@ -406,7 +404,6 @@ class BridgedConfigTest { baseUrl: http://gx10.gw:8000 architects: lead-designer: - terminal: term_design profile: ltms-local """); @@ -426,7 +423,6 @@ class BridgedConfigTest { baseUrl: http://gx10.gw:8000 architects: lead-designer: - terminal: term_design profile: sonnet """); BridgedConfig cfg = BridgedConfig.load(f); @@ -447,7 +443,6 @@ class BridgedConfigTest { baseUrl: http://gx10.gw:8000 architects: lead-designer: - terminal: term_design profile: "" """); BridgedConfig cfg = BridgedConfig.load(f); @@ -469,7 +464,6 @@ class BridgedConfigTest { baseUrl: http://gx10.gw:8000 architects: lead-designer: - terminal: term_design profile: sonnet reviewer: profile: gx10 @@ -486,6 +480,48 @@ class BridgedConfigTest { assertDoesNotThrow(() -> BridgedConfig.load(f).validateArchitects()); } + @Test + void duplicateArchitectSlotNamesAreRejectedAtParseTime(@TempDir Path dir) throws Exception { + Path f = dir.resolve("arch-dup.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + sonnet: + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + profile: sonnet + lead-designer: + profile: sonnet + """); + + IllegalStateException e = + assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("lead-designer"), + "the refusal names the duplicated slot, was: " + e.getMessage()); + assertTrue(e.getMessage().contains("duplicate architect"), + "the refusal says the slot name is duplicated"); + } + + @Test + void duplicateKeysOutsideArchitectsAreUnaffected(@TempDir Path dir) throws Exception { + // The duplicate check is scoped to the architects block — a duplicate elsewhere is not this + // guard's concern and must not change parsing of the rest of the config. + Path f = dir.resolve("dup-other.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + sonnet: + baseUrl: http://gx10.gw:8000 + sonnet: + baseUrl: http://gx10.gw:8000 + """); + // Last-wins for a non-architect duplicate is untouched: only the architects block is walked. + assertEquals(Set.of("sonnet"), BridgedConfig.load(f).workerProfiles().keySet()); + } + @Test void absentBrokerBlockLeavesInboxSoftState(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-broker.yaml");