From 21cfc09f8e0c3b0a6c35b3a7f8e1823e0450be51 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 13 Aug 2026 17:28:14 +0200 Subject: [PATCH 1/3] CB-548: config-declared architect slots + Role.ARCHITECT authz --- .../main/java/dev/ltms/bridged/Bridged.java | 23 ++- .../ltms/bridged/auth/ArchitectRegistry.java | 73 ++++++++ .../java/dev/ltms/bridged/auth/Authz.java | 22 ++- .../dev/ltms/bridged/auth/CallerResolver.java | 67 +++++++- .../java/dev/ltms/bridged/auth/Principal.java | 22 ++- .../main/java/dev/ltms/bridged/auth/Role.java | 10 ++ .../ltms/bridged/config/BridgedConfig.java | 102 ++++++++++- .../java/dev/ltms/bridged/mcp/BridgeMcp.java | 23 ++- .../bridged/auth/ArchitectRegistryTest.java | 67 ++++++++ .../java/dev/ltms/bridged/auth/AuthzTest.java | 44 +++++ .../ltms/bridged/auth/CallerResolverTest.java | 91 ++++++++++ .../bridged/config/BridgedConfigTest.java | 160 ++++++++++++++++++ .../ltms/bridged/mcp/BridgeMcpAuthzTest.java | 33 ++++ .../dev/ltms/bridged/mcp/BridgeMcpTest.java | 18 ++ 14 files changed, 735 insertions(+), 20 deletions(-) create mode 100644 bridged/src/main/java/dev/ltms/bridged/auth/ArchitectRegistry.java create mode 100644 bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 267aa99..fcbbc78 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -14,6 +14,7 @@ import dev.ltms.bridged.inject.Injector; import dev.ltms.bridged.inject.StatusPoller; import dev.ltms.bridged.inject.TurnListener; import dev.ltms.bridged.inject.WorkerPresence; +import dev.ltms.bridged.auth.ArchitectRegistry; import dev.ltms.bridged.auth.CallerResolver; import dev.ltms.bridged.mcp.BridgeMcp; import dev.ltms.bridged.mcp.ConnectionIdentity; @@ -90,6 +91,9 @@ public final class Bridged { // CB-542: a subscription:true profile whose env: reseats ANTHROPIC_BASE_URL/AUTH_TOKEN would // reach an unguarded endpoint (the launcher skips SubscriptionGuard for it). Refuse at load. cfg.validateSubscriptionProfiles(); + // CB-548: every architect slot must name a configured workers: profile — the strong-model + // backend the future spawn lifecycle would read. A stale reference dies here, not later. + cfg.validateArchitects(); Path socket = cfg.herdrSocket() != null && !cfg.herdrSocket().isBlank() ? Path.of(cfg.herdrSocket()) @@ -199,6 +203,19 @@ 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; + // nothing here spawns a slot. + ArchitectRegistry architects = new ArchitectRegistry( + cfg.architects() == null ? Map.of() : cfg.architects(), + () -> cfg.architectTerminals()); + if (!architects.slots().isEmpty()) { + log.info("architect slots: {} configured {}, terminals {}", architects.slots().size(), + architects.slots().keySet(), cfg.architectTerminals().keySet()); + } + // Status-gated injector (CB-103): the single writer into workers, fed by a poller. // The blocking message endpoint (CB-104) is the producer; the poller is inert until then. // CB-106: a confirmed turn completion resolves a blocked send whose worker never replied. @@ -308,11 +325,13 @@ public final class Bridged { throw new IllegalStateException("auth.mode=token but env var " + cfg.auth().tokenEnv() + " is unset or empty — export it before starting bridged"); } - callers = CallerResolver.withLeads(identity, true, token, leads); + callers = CallerResolver.withLeadsAndArchitects(identity, true, token, leads, + architects::terminalBindings); log.info("auth: token mode (bearer required for non-worker callers, env {})", cfg.auth().tokenEnv()); } else { - callers = CallerResolver.withLeads(identity, false, null, leads); + callers = CallerResolver.withLeadsAndArchitects(identity, false, null, leads, + architects::terminalBindings); 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 new file mode 100644 index 0000000..c5548b3 --- /dev/null +++ b/bridged/src/main/java/dev/ltms/bridged/auth/ArchitectRegistry.java @@ -0,0 +1,73 @@ +package dev.ltms.bridged.auth; + +import dev.ltms.bridged.config.BridgedConfig; + +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. + * + *

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. + */ +public final class ArchitectRegistry { + + private final Map slots; + private final Supplier> terminalBindings; + + public ArchitectRegistry(Map slots, + Supplier> terminalBindings) { + 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. */ + public Map slots() { + return slots; + } + + /** + * The live {@code terminal_id → slot name} bindings, re-read on every call. + * + *

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. + */ + public Map terminalBindings() { + return terminalBindings.get(); + } + + /** The slot a live terminal is bound to, or {@code null} if it is no architect slot. */ + public String slotForTerminal(String terminal) { + return terminal == null ? null : terminalBindings.get().get(terminal); + } + + /** + * The strong-model profile a slot runs under — what the future spawn lifecycle reads. + * + * @return the slot's configured {@code profile}, or {@code null} if the slot is unknown or + * declares none + */ + public String profileForSlot(String slotName) { + BridgedConfig.Architect a = slots.get(slotName); + return (a == null || a.profile() == null) ? null : a.profile(); + } + + /** True when {@code slotName} is a configured architect slot. */ + public boolean isSlot(String slotName) { + return slots.containsKey(slotName); + } +} diff --git a/bridged/src/main/java/dev/ltms/bridged/auth/Authz.java b/bridged/src/main/java/dev/ltms/bridged/auth/Authz.java index 0f96310..a89d6aa 100644 --- a/bridged/src/main/java/dev/ltms/bridged/auth/Authz.java +++ b/bridged/src/main/java/dev/ltms/bridged/auth/Authz.java @@ -46,20 +46,28 @@ public final class Authz { return false; // authenticated as nothing ⇒ authorized for nothing } return switch (action) { - // Orchestration is the primary's alone. A worker driving spawn/stop/send would be a - // worker escalating into the orchestrator role. - case SPAWN, STOP, SEND, DRAIN -> caller.isPrimary(); + // Fleet lifecycle is the primary's alone — spawn, stop, drain. An architect + // deliberately does NOT get these (CB-548), so it cannot tear down or stand up workers + // even though it coordinates them; and a worker driving any of these would be a worker + // escalating into the orchestrator role. + case SPAWN, STOP, DRAIN -> caller.isPrimary(); + + // Delivering a turn is open to the primary and the architect: an architect delegates + // to workers (that is the role's point) but still has no lifecycle rights. A worker is + // excluded — sending would be it escalating. + case SEND -> caller.isPrimary() || caller.isArchitect(); // The load-bearing rule: a caller acts only as the pane it occupies. CB-532 widened who // that can be — a lead answering another lead is replying for its OWN terminal, which // this already permits — while the rule itself is unchanged, and is what stops anyone - // forging a reply for a rendezvous someone else is waiting on. An unnamed primary - // (token/loopback, no pane) owns nothing and is still excluded. + // forging a reply for a rendezvous someone else is waiting on. An architect's own pane + // passes through the same check, so it can answer a funnel that delegated to it. An + // unnamed primary (token/loopback, no pane) owns nothing and is still excluded. case REPLY, ASK -> caller.ownsSession(targetSession); - // Observation is open to both authenticated roles: a worker legitimately polls its own + // Observation is open to every authenticated role: a worker legitimately polls its own // status, and the roster carries no secrets. - case READ, METRICS -> caller.isPrimary() || caller.isWorker(); + case READ, METRICS -> caller.isPrimary() || caller.isWorker() || caller.isArchitect(); }; } diff --git a/bridged/src/main/java/dev/ltms/bridged/auth/CallerResolver.java b/bridged/src/main/java/dev/ltms/bridged/auth/CallerResolver.java index 5f10789..ccc5ad6 100644 --- a/bridged/src/main/java/dev/ltms/bridged/auth/CallerResolver.java +++ b/bridged/src/main/java/dev/ltms/bridged/auth/CallerResolver.java @@ -24,6 +24,10 @@ import java.util.function.Supplier; * that pane as a lead's own — without this rule a lead running inside a herdr pane * is misread as a worker and locked out of orchestration. More than one pane may be named, * so two leads can work as peers rather than one being demoted. + *

  • A loopback peer PID that maps to a pane bound to a CB-548 architect slot ⇒ + * {@link Role#ARCHITECT}, carrying the slot name. Just unforgeable as a worker's, and + * resolved from the live terminal→slot binding (never a request argument), before + * the generic worker fallback.
    • *
  • A loopback peer PID that maps to any other herdr pane ⇒ {@link Role#WORKER}. This is * unforgeable (the OS reports the PID, herdr owns the PID→pane map) and is honoured * regardless of auth mode, so enabling auth never breaks the fleet.
    • @@ -47,6 +51,15 @@ public final class CallerResolver { * it is TTL-cached, so this is a map lookup in the common case. */ private final Supplier> leadTerminals; + /** + * terminal_id → architect slot name; empty when nothing is configured. CB-548. + * + *

    Like {@link #leadTerminals}, a supplier rather than a fixed map, so a binding injected + * after startup — when the later spawn lifecycle establishes a live architect session, or an + * operator pins one — takes effect without a restart. Consulted per resolve; today's wiring + * in {@code Bridged} reads a constant from config, which is the degenerate live case. + */ + private final Supplier> architectTerminals; /** Loopback-trust resolver: no token required, historical behaviour. */ public CallerResolver(ConnectionIdentity identity) { @@ -87,7 +100,17 @@ public final class CallerResolver { */ public CallerResolver(ConnectionIdentity identity, boolean tokenMode, String token, Map leadTerminals) { - this(identity, tokenMode, token, fixed(leadTerminals)); + this(identity, tokenMode, token, fixed(leadTerminals), null); + } + + /** + * Map-form of both registries (CB-548): lead terminals and the initial architect terminal + * bindings, each snapshotted at construction (a handed-over map is not offered as live state). + */ + public CallerResolver(ConnectionIdentity identity, boolean tokenMode, String token, + Map leadTerminals, + Map architectTerminals) { + this(identity, tokenMode, token, fixed(leadTerminals), fixed(architectTerminals)); } /** @@ -101,7 +124,23 @@ public final class CallerResolver { public static CallerResolver withLeads(ConnectionIdentity identity, boolean tokenMode, String token, Supplier> leadTerminals) { - return new CallerResolver(identity, tokenMode, token, leadTerminals); + return new CallerResolver(identity, tokenMode, token, leadTerminals, null); + } + + /** + * Live-registry form for both {@code leadTerminals} and the CB-548 architect registry: both + * are consulted on every resolve, so a slot binding injected after startup takes effect + * without a restart. + * + *

    A static factory rather than a constructor overload, for the same reason as + * {@link #pinnedTo}: too many {@code Map}/{@code Supplier} combinations to make {@code null} + * unambiguous. + */ + public static CallerResolver withLeadsAndArchitects(ConnectionIdentity identity, + boolean tokenMode, String token, + Supplier> leadTerminals, + Supplier> architectTerminals) { + return new CallerResolver(identity, tokenMode, token, leadTerminals, architectTerminals); } private static Supplier> fixed(Map leadTerminals) { @@ -110,7 +149,8 @@ public final class CallerResolver { } private CallerResolver(ConnectionIdentity identity, boolean tokenMode, String token, - Supplier> leadTerminals) { + Supplier> leadTerminals, + Supplier> architectTerminals) { if (tokenMode && (token == null || token.isBlank())) { throw new IllegalArgumentException( "auth.mode=token requires a non-empty token; check that the env var named by " @@ -120,6 +160,7 @@ public final class CallerResolver { this.tokenMode = tokenMode; this.expectedToken = tokenMode ? token.getBytes(StandardCharsets.UTF_8) : null; this.leadTerminals = leadTerminals == null ? Map::of : leadTerminals; + this.architectTerminals = architectTerminals == null ? Map::of : architectTerminals; } /** @@ -135,6 +176,17 @@ public final class CallerResolver { return leadTerminals.get(); } + /** + * The currently-recognised architect slots, {@code terminal_id → slot name} (CB-548). + * + *

    Read from the same supplier {@link #resolve} consults, so a slot that is listed + * here but would not resolve (or the reverse) cannot drift apart. Live for the same + * reason as {@link #leads()}. + */ + public Map architects() { + return architectTerminals.get(); + } + /** * Resolve the caller of a request. * @@ -149,8 +201,17 @@ public final class CallerResolver { if (lead != null) { // The config names this pane as a lead's own. The pane mapping is exactly as // unforgeable as a worker's, so it outranks the token path — no credential needed. + // Checked before the architect registry so a pane named in BOTH is still the lead + // (CB-548 preserves every existing leader behaviour). return Principal.leader(lead, c.terminal(), c.pid()); } + String slot = architectTerminals.get().get(c.terminal()); + if (slot != null) { + // The config/live binding names this pane as an architect slot's own. Same + // unforgeable pane mapping; the live binding, never a request argument, decides. + // Checked before the generic worker fallback, per the CB-548 precedence order. + return Principal.architect(slot, c.terminal(), c.pid()); + } return Principal.worker(c.terminal(), c.pid()); // unforgeable; never token-gated } diff --git a/bridged/src/main/java/dev/ltms/bridged/auth/Principal.java b/bridged/src/main/java/dev/ltms/bridged/auth/Principal.java index e4803bf..6341e22 100644 --- a/bridged/src/main/java/dev/ltms/bridged/auth/Principal.java +++ b/bridged/src/main/java/dev/ltms/bridged/auth/Principal.java @@ -10,7 +10,8 @@ package dev.ltms.bridged.auth; * token or loopback trust, and for {@code ANONYMOUS} * @param pid the connecting process id, or {@code -1} when not resolvable (audit context) * @param name for a lead resolved from the CB-530 {@code leaders:} registry, which lead it is; - * {@code null} for every other caller, including an unnamed primary + * for an architect resolved from the CB-548 {@code architects:} registry, which + * slot it occupies; {@code null} for every other caller, including an unnamed primary */ public record Principal(Role role, String terminal, long pid, String name) { @@ -58,10 +59,28 @@ public record Principal(Role role, String terminal, long pid, String name) { return new Principal(Role.WORKER, terminal, pid); } + /** + * An architect (CB-548), identified by the slot it occupies and the pane bound to it. + * + *

    Carries {@link Role#ARCHITECT}. {@code slotName} is reporting only — it lets + * {@code bridge_whoami} say which architect slot is asking, and it is the key the + * (future) spawn lifecycle reads a profile back from. Identity is the {@code terminal}: like a + * worker's it comes from the connection and the live terminal→slot binding, so + * {@code ownsSession} works exactly as it does for a worker — an architect acts as its own + * pane and no other. + */ + public static Principal architect(String slotName, String terminal, long pid) { + return new Principal(Role.ARCHITECT, terminal, pid, slotName); + } + public boolean isPrimary() { return role == Role.PRIMARY; } + public boolean isArchitect() { + return role == Role.ARCHITECT; + } + public boolean isWorker() { return role == Role.WORKER; } @@ -90,6 +109,7 @@ public record Principal(Role role, String terminal, long pid, String name) { public String describe() { return switch (role) { case WORKER -> "worker:" + terminal; + case ARCHITECT -> "architect:" + name; case PRIMARY -> name == null ? "primary" : "leader:" + name; case ANONYMOUS -> "anonymous"; }; diff --git a/bridged/src/main/java/dev/ltms/bridged/auth/Role.java b/bridged/src/main/java/dev/ltms/bridged/auth/Role.java index 0d3da30..b43f3ac 100644 --- a/bridged/src/main/java/dev/ltms/bridged/auth/Role.java +++ b/bridged/src/main/java/dev/ltms/bridged/auth/Role.java @@ -24,6 +24,16 @@ public enum Role { */ WORKER, + /** + * A config-declared architect slot (CB-548): a gateway-local named session on a strong-model + * profile that coordinates and delegates turns but does not own the fleet. Unforgeable like a + * worker's — derived from the connection's pane and the live terminal→slot binding, never from + * a request argument. May {@code SEND} a turn, {@code REPLY}/{@code ASK} only as its own pane, + * and {@code READ}/{@code METRICS}; may not {@code SPAWN}/{@code STOP}/{@code DRAIN} + * (those stay the primary's, to keep lifecycle in one pair of hands). + */ + ARCHITECT, + /** Authenticated as nothing. Authorized for nothing but {@code /healthz}. */ ANONYMOUS } 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 6a3229f..10e750e 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -43,6 +43,11 @@ import java.util.Set; * @param leaders named panes that orchestrate rather than are orchestrated (CB-530), keyed by * 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. * @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: @@ -65,6 +70,7 @@ public record BridgedConfig( Broker broker, Primary primary, Map leaders, + Map architects, LeadScan leadScan, String placement, Auth auth) { @@ -379,6 +385,32 @@ public record BridgedConfig( public record Leader(String terminal, String kind, String model) { } + /** + * 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. + * + *

    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()} + */ + @JsonIgnoreProperties(ignoreUnknown = true) + public record Architect(String terminal, String profile) { + } + /** * Discover leads by tab label instead of by pasted {@code terminal_id} (CB-531). * @@ -436,6 +468,30 @@ 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. @@ -539,7 +595,7 @@ public record BridgedConfig( private static final Set KNOWN_TOP_LEVEL_KEYS = Set.of( "bind", "herdrSocket", "worker", "workers", "defaultWorker", "guard", "worktreeRoot", "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "leaders", - "leadScan", "placement", "auth"); + "architects", "leadScan", "placement", "auth"); /** Load and validate config from {@code path}. */ public static BridgedConfig load(Path path) { @@ -615,7 +671,9 @@ public record BridgedConfig( // leadScan is left as-is: null is "off", and LeadScan's own compact constructor defaults the // fields of a block that IS present. Defaulting it here would switch the feature on for // every config that never mentioned it. - return new BridgedConfig(b, herdrSocket, worker, workers, defaultWorker, g, worktreeRoot, l, timeout, pollMs, broker, primary, leaders, leadScan, placementOrDefault, a); + // architects is left as-is: null is "none configured", and Architect's fields have no + // defaults to fill. Defaulting it here would change nothing, so leave the call natural. + return new BridgedConfig(b, herdrSocket, worker, workers, defaultWorker, g, worktreeRoot, l, timeout, pollMs, broker, primary, leaders, architects, leadScan, placementOrDefault, a); } /** @@ -720,6 +778,46 @@ public record BridgedConfig( } } + /** + * Reject an architect slot whose profile reference does not resolve (CB-548). + * + *

    An architect's {@code profile} is the strong-model {@code workers:} profile the future + * spawn lifecycle will read to stand the slot up. A reference that names no configured profile + * is a typo or a stale config — and unlike a worker spawn (which fails loudly at its call site + * when it cannot resolve), an architect slot fails only when something later tries to use it. + * This config class does have access to {@link #workerProfiles()}, so the reference + * is validated at startup and the mistake is named then, not discovered months later by a + * 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. + * + * @throws IllegalStateException when any architect slot is missing or names an unknown profile, + * naming the slot and the offending reference + */ + public void validateArchitects() { + if (architects == null) { + return; + } + Map profiles = workerProfiles(); + List bad = new java.util.ArrayList<>(); + architects.forEach((name, arch) -> { + if (arch == null || arch.profile() == null || arch.profile().isBlank()) { + bad.add("architect slot '" + name + "' has no profile: — give it the name of a " + + "workers: profile (the strong-model backend it runs)."); + return; + } + if (!profiles.containsKey(arch.profile())) { + bad.add("architect slot '" + name + "' references profile '" + arch.profile() + + "', which is not a configured workers: profile (have: " + profiles.keySet() + + ")."); + } + }); + if (!bad.isEmpty()) { + throw new IllegalStateException("refusing to start: " + String.join(" ", bad)); + } + } + /** True for the loopback addresses and the unspecified-but-local forms we treat as same-host. */ private static boolean isLoopbackBind(String host) { if (host == null || host.isBlank()) { diff --git a/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java b/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java index 223f65e..3056344 100644 --- a/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java +++ b/bridged/src/main/java/dev/ltms/bridged/mcp/BridgeMcp.java @@ -507,6 +507,17 @@ public final class BridgeMcp { static McpSchema.CallToolResult whoami(Principal caller, SessionManager sessions) { Map m = new LinkedHashMap<>(); m.put("role", caller.role().name().toLowerCase()); + if (caller.isArchitect()) { + // CB-548: the role reads "architect"; the name is the gateway-local slot the pane is + // bound to, and the pane itself so a peer knows where to reach it. + if (caller.name() != null) { + m.put("architect", caller.name()); + } + if (caller.terminal() != null) { + m.put("sessionId", caller.terminal()); + } + return text(json(m)); + } if (!caller.isWorker()) { // CB-530: which lead, once more than one pane is configured as one. `role` deliberately // still reads "primary" — the fallback ladder in CLAUDE.md keys on it, and a lead IS a @@ -827,11 +838,13 @@ public final class BridgeMcp { "Report who YOU are on the bridge — your role is resolved from your connection " + "(unforgeable), never from anything you claim. Returns role 'primary' (you " + "orchestrate: spawn/send/stop; reply ONLY to answer a peer lead that " - + "messaged you, never to answer a worker) or 'worker' (you were delegated " - + "to: you must end every turn with exactly one bridge_reply, and cannot " - + "spawn or send), plus 'leader' naming which lead you are, your own " - + "sessionId, and profile/worktree/branch when you are a worker. Call this " - + "first when following role-conditional instructions rather than guessing.", + + "messaged you, never to answer a worker), 'architect' (you delegate turns " + + "and reply/ask as your own pane, but cannot spawn/stop/drain), or 'worker' " + + "(you were delegated to: you must end every turn with exactly one " + + "bridge_reply, and cannot spawn or send), plus 'leader'/'architect' naming " + + "which one you are, your own sessionId, and profile/worktree/branch when " + + "you are a worker. Call this first when following role-conditional " + + "instructions rather than guessing.", objectSchema(Map.of(), List.of())); } diff --git a/bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java b/bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java new file mode 100644 index 0000000..85dc7e9 --- /dev/null +++ b/bridged/src/test/java/dev/ltms/bridged/auth/ArchitectRegistryTest.java @@ -0,0 +1,67 @@ +package dev.ltms.bridged.auth; + +import dev.ltms.bridged.config.BridgedConfig; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; + +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. + */ +class ArchitectRegistryTest { + + private static final Map SLOTS = Map.of( + "lead-designer", new BridgedConfig.Architect("term_design", "sonnet"), + "reviewer", new BridgedConfig.Architect(null, "gx10")); + + private final ArchitectRegistry registry = + new ArchitectRegistry(SLOTS, () -> Map.of("term_design", "lead-designer")); + + @Test + void exposesTheConfiguredSlots() { + assertEquals(SLOTS.keySet(), registry.slots().keySet()); + assertTrue(registry.isSlot("reviewer")); + assertFalse(registry.isSlot("nope")); + } + + @Test + void theSpawnLifecycleReadsTheProfileBackFromASlot() { + assertEquals("sonnet", registry.profileForSlot("lead-designer")); + assertEquals("gx10", registry.profileForSlot("reviewer")); + assertNull(registry.profileForSlot("unknown"), "an unknown slot has no profile"); + } + + @Test + void resolvesTheSlotOfALiveTerminal() { + assertEquals("lead-designer", registry.slotForTerminal("term_design")); + assertNull(registry.slotForTerminal("term_unbound")); + assertNull(registry.slotForTerminal(null), "no terminal ⇒ no slot"); + } + + @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")); + } + + @Test + void theSlotSnapshotIsFixedByConstruction() { + Map mutable = new HashMap<>(SLOTS); + ArchitectRegistry r = new ArchitectRegistry(mutable, Map::of); + + mutable.put("hijack", new BridgedConfig.Architect("t", "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/auth/AuthzTest.java b/bridged/src/test/java/dev/ltms/bridged/auth/AuthzTest.java index 5b2b8c9..63cfd2c 100644 --- a/bridged/src/test/java/dev/ltms/bridged/auth/AuthzTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/auth/AuthzTest.java @@ -12,6 +12,8 @@ class AuthzTest { private static final Principal WORKER_A = Principal.worker("term_a", 200); private static final Principal WORKER_B = Principal.worker("term_b", 300); private static final Principal ANON = Principal.anonymous(); + private static final Principal ARCH_DESIGN = Principal.architect("lead-designer", "term_design", 400); + private static final Principal ARCH_OTHER = Principal.architect("reviewer", "term_review", 500); @Test void anonymousIsAuthorizedForNothing() { @@ -61,6 +63,48 @@ class AuthzTest { "an absent session id must not satisfy the own-session rule"); } + // ── CB-548: the architect matrix ─────────────────────────────────────────────────────────── + + @Test + void anArchitectMaySendButNotSpawnStopOrDrain() { + assertTrue(Authz.permits(ARCH_DESIGN, SEND, "term_worker"), + "delegating a turn to a worker IS the architect's job"); + assertTrue(Authz.permits(ARCH_DESIGN, SEND, null)); + + for (Authz.Action a : new Authz.Action[]{SPAWN, STOP, DRAIN}) { + assertFalse(Authz.permits(ARCH_DESIGN, a, null), + "an architect must not " + a + " — fleet lifecycle is the primary's alone, so " + + "a coordinator cannot also stand up or tear down the fleet"); + } + } + + @Test + void anArchitectMayReplyAndAskOnlyAsItsOwnPane() { + assertTrue(Authz.permits(ARCH_DESIGN, REPLY, "term_design"), "its own pane is its own"); + assertTrue(Authz.permits(ARCH_DESIGN, ASK, "term_design")); + + assertFalse(Authz.permits(ARCH_DESIGN, REPLY, "term_review"), + "architect 'lead-designer' must not reply on reviewer's pane"); + assertFalse(Authz.permits(ARCH_OTHER, ASK, "term_design"), + "reviewer must not ask as lead-designer — no terminal is another's"); + assertFalse(Authz.permits(ARCH_DESIGN, REPLY, null), + "an absent target must not pass the own-session rule"); + } + + @Test + void anArchitectMayReadAndScrapeMetrics() { + assertTrue(Authz.permits(ARCH_DESIGN, READ, null)); + assertTrue(Authz.permits(ARCH_DESIGN, METRICS, null)); + } + + @Test + void anArchitectIsNotCountedAsPrimaryOrWorker() { + assertFalse(Authz.permits(ARCH_DESIGN, SPAWN, null), "not a primary — no lifecycle"); + assertFalse(ARCH_DESIGN.isPrimary()); + assertFalse(ARCH_DESIGN.isWorker(), "an architect is its own role, not a widened worker"); + assertTrue(ARCH_DESIGN.isArchitect()); + } + @Test void observationIsOpenToBothAuthenticatedRoles() { assertTrue(Authz.permits(PRIMARY, READ, null)); diff --git a/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java b/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java index bea0417..46a79ed 100644 --- a/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/auth/CallerResolverTest.java @@ -298,6 +298,97 @@ class CallerResolverTest { assertEquals(Role.WORKER, r.resolve("127.0.0.1", 42, null).role()); } + // ── CB-548: architect slots ───────────────────────────────────────────────────────────────── + + @Test + void aBoundArchitectPaneResolvesToArchitectBeforeTheWorkerFallback() { + Principal p = CallerResolver.withLeadsAndArchitects(workerIdentity(), false, null, + Map::of, () -> Map.of("term_a", "lead-designer")) + .resolve("127.0.0.1", 42, null); + + assertEquals(Role.ARCHITECT, p.role(), + "a terminal bound to an architect slot is an architect, NOT the generic worker it " + + "would otherwise resolve to"); + assertEquals("lead-designer", p.name(), "whoami must say WHICH slot is asking"); + assertEquals("term_a", p.terminal(), "the pane identity is carried so ownsSession works"); + } + + @Test + void anArchitectNeedsNoTokenEvenInTokenMode() { + Principal p = CallerResolver.withLeadsAndArchitects(workerIdentity(), true, "s3cret", + Map::of, () -> Map.of("term_a", "lead-designer")) + .resolve("127.0.0.1", 42, null); + + assertEquals(Role.ARCHITECT, p.role(), + "the pane mapping is as unforgeable as a worker's — it outranks the token path"); + } + + @Test + void anUnboundPaneStillResolvesAsAWorker() { + Map arch = Map.of("term_elsewhere", "reviewer"); + Principal p = CallerResolver.withLeadsAndArchitects(workerIdentity(), false, null, + Map::of, () -> arch).resolve("127.0.0.1", 42, null); + + assertEquals(Role.WORKER, p.role()); + assertNull(p.name()); + } + + /** CB-548 precedence: lead > architect > worker, so a pane named in BOTH is still a lead. */ + @Test + void aLeadWinsOverAnArchitectBindingForTheSamePane() { + Principal p = CallerResolver.withLeadsAndArchitects(workerIdentity(), false, null, + () -> Map.of("term_a", "opus-5.0"), () -> Map.of("term_a", "lead-designer")) + .resolve("127.0.0.1", 42, null); + + assertEquals(Role.PRIMARY, p.role(), + "a pane the config calls a lead must keep resolving as a lead — no behaviour change " + + "when an architect binding is added to an existing fleet"); + assertEquals("opus-5.0", p.name()); + } + + /** The registry is live, like leads: a binding injected after construction is honoured. */ + @Test + void anArchitectBoundAfterConstructionIsHonouredWithoutRebuildingTheResolver() { + Map live = new java.util.HashMap<>(); + CallerResolver r = CallerResolver.withLeadsAndArchitects(workerIdentity(), false, null, + Map::of, () -> live); + + assertEquals(Role.WORKER, r.resolve("127.0.0.1", 42, null).role()); + + live.put("term_a", "lead-designer"); // the later lifecycle binds the slot + + assertEquals(Role.ARCHITECT, r.resolve("127.0.0.1", 42, null).role()); + assertEquals("lead-designer", r.architects().get("term_a")); + } + + @Test + void theArchitectMapFormIsCopiedSoLaterMutationCannotGrantArchitect() { + Map mutable = new java.util.LinkedHashMap<>(); + CallerResolver r = new CallerResolver(workerIdentity(), false, null, Map.of(), mutable); + + mutable.put("term_a", "sneaky"); + + assertEquals(Role.WORKER, r.resolve("127.0.0.1", 42, null).role()); + } + + @Test + void describeNamesTheArchitectSlot() { + assertEquals("architect:lead-designer", + Principal.architect("lead-designer", "term_a", 1).describe()); + } + + /** An architect acts only as its own pane — the same ownsSession rule as a worker or lead. */ + @Test + void anArchitectOwnsItsOwnPaneAndNoOther() { + Principal arch = CallerResolver.withLeadsAndArchitects(workerIdentity(), false, null, + Map::of, () -> Map.of("term_a", "lead-designer")).resolve("127.0.0.1", 42, null); + + assertTrue(arch.ownsSession("term_a")); + assertTrue(Authz.permits(arch, Authz.Action.REPLY, "term_a")); + assertFalse(arch.ownsSession("term_b")); + assertFalse(Authz.permits(arch, Authz.Action.REPLY, "term_b")); + } + @Test void tokenModeRequiresANonEmptyConfiguredToken() { ConnectionIdentity id = nonWorkerIdentity(); 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 b5bf45d..8b06a35 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -129,6 +129,7 @@ class BridgedConfigTest { broker: {} primary: {} leaders: {} + architects: {} leadScan: {} placement: fixed auth: {} @@ -326,6 +327,165 @@ class BridgedConfigTest { assertEquals(Map.of("term_real", "real"), BridgedConfig.load(f).leaderTerminals()); } + // ── CB-548: the architects registry ──────────────────────────────────────────────────────── + + @Test + void architectsBlockBindsSlotsByGatewayLocalName(@TempDir Path dir) throws Exception { + Path f = dir.resolve("architects.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + sonnet: + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + terminal: term_design + profile: sonnet + reviewer: + profile: sonnet + """); + + BridgedConfig cfg = BridgedConfig.load(f); + assertEquals(Set.of("lead-designer", "reviewer"), cfg.architects().keySet(), + "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()); + } + + @Test + void architectTerminalsMapsEachBoundSlotByItsPane(@TempDir Path dir) throws Exception { + Path f = dir.resolve("arch-terminals.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + sonnet: + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + terminal: term_design + profile: sonnet + reviewer: + terminal: term_review + profile: sonnet + unbound: + profile: sonnet + """); + + 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"); + } + + @Test + void noArchitectsBlockLeavesNothingBound(@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(), + "no architects: block ⇒ no architect identity, exactly as before CB-548"); + } + + @Test + void anArchitectSlotMayResolveToTheSoleProfileWithoutPrivileging(@TempDir Path dir) throws Exception { + // Even a single unqualified worker profile can back an architect slot — the reference is + // by name, not by position, so an explicit name is required. + Path f = dir.resolve("arch-single.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + worker: + profile: ltms-local + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + terminal: term_design + profile: ltms-local + """); + + BridgedConfig cfg = BridgedConfig.load(f); + assertDoesNotThrow(cfg::validateArchitects); + assertEquals("ltms-local", cfg.architects().get("lead-designer").profile()); + } + + @Test + void anArchitectProfileThatIsNotConfiguredRefusesToStart(@TempDir Path dir) throws Exception { + Path f = dir.resolve("arch-bad-profile.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + gx10: + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + terminal: term_design + profile: sonnet + """); + BridgedConfig cfg = BridgedConfig.load(f); + + IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateArchitects); + assertTrue(e.getMessage().contains("lead-designer"), "the refusal names the slot"); + assertTrue(e.getMessage().contains("sonnet"), "the refusal names the offending profile"); + } + + @Test + void anArchitectSlotMissingAProfileRefusesToStart(@TempDir Path dir) throws Exception { + Path f = dir.resolve("arch-no-profile.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + gx10: + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + terminal: term_design + profile: "" + """); + BridgedConfig cfg = BridgedConfig.load(f); + + IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateArchitects); + assertTrue(e.getMessage().contains("lead-designer"), "the refusal names the slot"); + } + + @Test + void aValidArchitectRegistryPassesValidation(@TempDir Path dir) throws Exception { + Path f = dir.resolve("arch-ok.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + workers: + sonnet: + baseUrl: http://gx10.gw:8000 + gx10: + baseUrl: http://gx10.gw:8000 + architects: + lead-designer: + terminal: term_design + profile: sonnet + reviewer: + profile: gx10 + """); + + assertDoesNotThrow(() -> BridgedConfig.load(f).validateArchitects()); + } + + @Test + void absentArchitectsBlockPassesValidation(@TempDir Path dir) throws Exception { + Path f = dir.resolve("no-arch.yaml"); + Files.writeString(f, "bind:\n port: 8080\n"); + + assertDoesNotThrow(() -> BridgedConfig.load(f).validateArchitects()); + } + @Test void absentBrokerBlockLeavesInboxSoftState(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-broker.yaml"); diff --git a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java index 5dc9a6d..1f53a04 100644 --- a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpAuthzTest.java @@ -77,6 +77,7 @@ class BridgeMcpAuthzTest { private static final Principal PRIMARY = Principal.primary(100); private static final Principal WORKER_A = Principal.worker("term_a", 200); private static final Principal ANON = Principal.anonymous(); + private static final Principal ARCH_DESIGN = Principal.architect("lead-designer", "term_design", 400); // --- the table, enforced on THIS path too --------------------------------------------------- @@ -119,6 +120,33 @@ class BridgeMcpAuthzTest { assertNotNull(m.denyFor(PRIMARY, Authz.Action.ASK, "term_a")); } + // --- CB-548: the architect on this path ------------------------------------------------ + + @Test + void anArchitectMaySendAndReadButNotOrchestrateOverMcp() { + BridgeMcp m = mcp(true); + assertNull(m.denyFor(ARCH_DESIGN, Authz.Action.SEND, "term_a"), + "delegating a turn is the architect's job"); + assertNull(m.denyFor(ARCH_DESIGN, Authz.Action.READ, null)); + + for (Authz.Action a : new Authz.Action[]{Authz.Action.SPAWN, Authz.Action.STOP, + Authz.Action.DRAIN}) { + McpSchema.CallToolResult denied = m.denyFor(ARCH_DESIGN, a, null); + assertNotNull(denied, a + " must be refused to an architect"); + assertTrue(denied.isError(), "a refusal is returned as an MCP tool error"); + } + } + + @Test + void anArchitectMayReplyAndAskOnlyAsItsOwnPaneOverMcp() { + BridgeMcp m = mcp(true); + assertNull(m.denyFor(ARCH_DESIGN, Authz.Action.REPLY, "term_design")); + assertNull(m.denyFor(ARCH_DESIGN, Authz.Action.ASK, "term_design")); + + assertNotNull(m.denyFor(ARCH_DESIGN, Authz.Action.REPLY, "term_a"), + "architect 'lead-designer' must not reply on worker term_a's session"); + } + @Test void anonymousIsRefusedEverythingAndCountedAsUnauthenticated() { BridgeMcp m = mcp(true); @@ -156,6 +184,11 @@ class BridgeMcpAuthzTest { assertEquals("term_a", BridgeMcp.principalFrom("WORKER", "term_a", 7).terminal()); assertEquals(Role.PRIMARY, BridgeMcp.principalFrom("PRIMARY", null, 7).role()); assertEquals(Role.ANONYMOUS, BridgeMcp.principalFrom("ANONYMOUS", null, -1).role()); + // CB-548: an architect round-trips through the same stash, carrying its slot name. + Principal arch = BridgeMcp.principalFrom("ARCHITECT", "term_design", 7, "lead-designer"); + assertEquals(Role.ARCHITECT, arch.role()); + assertEquals("lead-designer", arch.name()); + assertEquals("term_design", arch.terminal()); } @Test diff --git a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java index 6e65fa5..5daccde 100644 --- a/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/mcp/BridgeMcpTest.java @@ -466,4 +466,22 @@ class BridgeMcpTest { assertTrue(out.contains("\"sessionId\":\"term_orphan\""), out); assertFalse(out.contains("profile"), out); // nothing invented for a session we don't track } + + /** + * CB-548: an architect reports its role and which gateway-local slot its pane is bound to — + * the same shape as a lead, under the architect key, so it can tell a peer where to reach it. + */ + @Test + void whoamiReportsAnArchitectWithItsSlotAndPane() { + FakeHerdr h = new FakeHerdr(); + McpSchema.CallToolResult res = BridgeMcp.whoami( + Principal.architect("lead-designer", "term_design", 400), + sessionManager(h, "http://gx00.gw:8000", Set.of("gx00.gw"))); + + assertNotEquals(Boolean.TRUE, res.isError()); + String out = textOf(res); + assertTrue(out.contains("\"role\":\"architect\""), out); + assertTrue(out.contains("\"architect\":\"lead-designer\""), out); + assertTrue(out.contains("\"sessionId\":\"term_design\""), out); + } } -- 2.52.0 From 6123576c6863638842cfde2c5703f275ec3d5bbf Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 13 Aug 2026 18:05:53 +0200 Subject: [PATCH 2/3] =?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"); -- 2.52.0 From f004a0c654d6e92d31189a1ec75b6dd220afe8cc Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 13 Aug 2026 18:31:29 +0200 Subject: [PATCH 3/3] CB-548: make duplicate-architect-slot detection top-level-only and depth-safe --- .../ltms/bridged/config/BridgedConfig.java | 86 +++++++++++++++---- .../bridged/config/BridgedConfigTest.java | 57 ++++++++++++ 2 files changed, 127 insertions(+), 16 deletions(-) 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"); -- 2.52.0