CB-548: correct architect premise — profile-only slots, registry-owned bindings, dup-key rejection

2026-08-13 18:05:53 +02:00
parent 21cfc09f8e
commit 6123576c68
5 changed files with 412 additions and 117 deletions
@@ -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)");
        }

@@ -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 <em>live</em> bindings from a live architect's herdr terminal to
+ * its slot.
 *
 * <p>Two halves, split by who owns each:
 * <ul>
 *   <li><b>slots</b> — configured once, keyed by the gateway-local unique name; each carries the
- *       {@code profile} reference the <em>future</em> spawn lifecycle will read when it stands the
- *       slot up. A read-only snapshot taken at construction.</li>
- *   <li><b>terminal bindings</b> — a {@link Supplier} consulted on every read, so a binding
- *       injected <em>after</em> startup (an operator pin, or the later lifecycle once it spawns a
- *       session) takes effect without a restart. {@link CallerResolver} reads this to turn a pane
- *       into an {@link Role#ARCHITECT}.</li>
+ *       {@code profile} reference the spawn lifecycle reads when it stands the slot up. A read-only
+ *       snapshot taken at construction.</li>
+ *   <li><b>terminal bindings</b> — owned by this registry and initially <em>empty</em>. Config
+ *       declares no architect terminal, so at startup every slot is idle and nothing resolves to an
+ *       architect; a session only becomes one when the spawn lifecycle {@linkplain #bind(String,
+ *       String) binds} its terminal to a slot. {@link CallerResolver} reads this through
+ *       {@link #snapshot()} to turn a pane into an {@link Role#ARCHITECT}.</li>
 * </ul>
 *
- * <p>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.
+ * <p>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<String, BridgedConfig.Architect> slots;
-    private final Supplier<Map<String, String>> terminalBindings;
+    /** Live {@code terminal_id → slot name}; guarded by {@code this}. */
+    private final Map<String, String> terminalToSlot = new HashMap<>();

-    public ArchitectRegistry(Map<String, BridgedConfig.Architect> slots,
-                             Supplier<Map<String, String>> terminalBindings) {
+    public ArchitectRegistry(Map<String, BridgedConfig.Architect> 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.
     *
     * <p>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<String, String> terminalBindings() {
-        return terminalBindings.get();
+    public Map<String, String> 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).
+     *
+     * <p>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).
+     *
+     * <p>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;
+        }
+    }
 }
@@ -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 <em>not</em> 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.
     *
-     * <p>A lead and an architect differ in <em>authority</em>, 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.
+     * <p>A slot is <em>declared</em>, 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.
     *
     * <p>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.
-     *
-     * <p>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 <em>initial</em> 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<String, String> architectTerminals() {
-        Map<String, String> 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 <em>not</em> 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).
+     *
+     * <p>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<String> 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.
     *
     * <p>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