From 976eff8ad1d8f63b0f9ecfeaf39b192a9642da5e Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 15 Aug 2026 08:05:21 +0200 Subject: [PATCH] CB-579: resolve a lead by its tab name, drop the terminal-id pin Leader.terminal -> Leader.tab (exact tab label, case-insensitive match). LeadTabScanner matches an exact tab->name map instead of stripping a shared tabPrefix, and no longer merges configured leads into every scan result -- a stale pin can no longer outlive its tab. LeadLauncher.tabLabel() returns the configured tab directly; the terminalId pinned-terminal fallback in liveLeads() is gone. Config load now rejects a leftover fleet.leaders.*.terminal key instead of silently ignoring it. primary.terminal is untouched. --- bridged/bridged.example.yaml | 33 ++-- .../main/java/dev/ltms/bridged/Bridged.java | 42 ++--- .../ltms/bridged/config/BridgedConfig.java | 102 +++++++---- .../dev/ltms/bridged/config/ConfigRef.java | 6 +- .../ltms/bridged/herdr/LeadTabScanner.java | 61 ++++--- .../dev/ltms/bridged/lead/LeadLauncher.java | 44 ++--- .../bridged/config/BridgedConfigTest.java | 164 +++++++++++------- .../bridged/herdr/LeadTabScannerTest.java | 132 ++++++++++---- .../ltms/bridged/lead/LeadLauncherTest.java | 51 +++--- 9 files changed, 399 insertions(+), 236 deletions(-) diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index 6bd6c0f..35630a2 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -47,15 +47,18 @@ bind: # (say a Claude lead and an opencode lead) work as peers: the second is silently demoted and refused # every orchestration call. List each lead's pane here and all of them resolve as leads. # -# terminal → the ONLY field identity depends on; get it from that session's bridge_whoami +# tab → the ONLY field identity depends on (CB-579); the exact label of the tab hosting the lead. +# Label the tab yourself, or let bridged label one it launches — see `fleet.leaders:` below. # kind/model → descriptive; they document what runs in the pane and are echoed by bridge_whoami # -# A lead is never spawned — it pre-exists, which is exactly why it must be named rather than created. +# A lead's tab must already carry its label (or be launched by bridged, which labels it) — there is +# no terminal id to paste in and nothing to re-pin when the session restarts: the tab survives, so +# the same label resolves the same lead again on the next scan. # `bridge_whoami` reports `{"role":"primary","leader":""}`; role stays "primary" because a lead # IS a primary for authorization, so nothing that keys on the role breaks. # # KEEP `primary:` when adding leads: it still addresses the CB-307 push loop, which needs a single -# destination for its nudges. If both name the same terminal, the `fleet.leaders:` entry wins. +# destination for its nudges, and is a separate mechanism from lead identity — see `fleet.leaders:`. # # Leads are configured under `fleet.leaders:` — see THE FLEET further down. # @@ -309,15 +312,18 @@ fleet: # Panes that orchestrate rather than are orchestrated. A lead may now be CREATED as well as # recognised: give it a `profile:` and the daemon launches the shortfall when fewer than - # `instances` are live. Give it only a `terminal:` and it is recognise-only, as before. + # `instances` are live. Omit `profile:` and it is recognise-only, as before. # - # `tabPrefix` is the naming convention that finds a lead without pasting a terminal id: label the - # tab `lead: ` when you open it and the pane is recognised on the next rescan. Reopen the - # tab later and the id changes; the label does not. + # `tab:` (CB-579) is REQUIRED and is the only field identity depends on — the exact label of the + # tab hosting the lead, matched case-insensitively. Label the tab yourself and put that same + # string here, and the pane is recognised on the next rescan. Reopen the tab later, or the session + # inside it restarts — the terminal id changes; the tab, and its label, do not, so no config edit + # follows a restart. # - # A lead the daemon launches is labelled BY the daemon, using the same convention, so it is found + # A lead the daemon launches is labelled BY the daemon with this same `tab:` value, so it is found # by the same scan. A lead counts as live only when herdr also reports a running agent in that - # tab — a label left behind by a session that died does not block the relaunch. + # tab — a label left behind by a session that died does not block the relaunch, and a tab that is + # gone entirely drops out of the next scan rather than being remembered forever. # # An auto-launched lead is NOT a member: it gets no worker reply charter, is never registered with # the session lifecycle (the idle reaper would kill your orchestrator), and stays on the @@ -326,10 +332,9 @@ fleet: # opus-5.0: # profile: opus # omit to never create this lead, only recognise it # instances: 1 # desired live count; only the shortfall is launched. 0 = off - # terminal: term_0123456789abcd # optional hand-pin; usually found by tabPrefix instead. - # # A running agent on this terminal also counts as live, so a - # # lead you opened by hand is not relaunched under you. - # tabPrefix: "lead:" # `lead: opus-5.0` ⇒ a lead named opus-5.0 (case-insensitive) + # tab: "lead: opus-5.0" # REQUIRED — the exact tab label this lead lives in + # tabPrefix: "lead:" # only used to guard against a worker tabLabel colliding with + # # this convention at startup; plays no part in matching a lead # scanIntervalSeconds: 10 # rescan cadence, and the worst case before a new tab is seen # workspace: leads # where a launched lead's tab is created (default "leads"). # # MUST NOT be a member workspace — those are excluded from the @@ -337,7 +342,7 @@ fleet: # cwd: /path/to/repo # the launched lead's working directory (default: bridged's own) # kind: claude # descriptive; reported by bridge_whoami # gpt-sol-5.6: - # terminal: term_fedcba9876543 + # tab: "lead: gpt-sol-5.6" # kind: opencode # model: openai/gpt-5.6-terra diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index 70a10d9..3b79210 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -193,34 +193,34 @@ public final class Bridged { if (leadTerminals.size() > 1) { log.info("leads: {} panes recognised {}", leadTerminals.size(), leadTerminals.values()); } - // CB-531: on top of the static registry, discover leads by the tab labels the operator - // writes. CB-557 moved the settings onto the lead they describe, so scanning is on whenever - // a `fleet.leaders:` entry exists — with no leads configured the supplier is a constant and - // never touches herdr, exactly as a missing `leadScan:` block used to behave. + // CB-531: on top of the legacy primary.terminal pin, discover leads by the tab labels the + // operator writes. CB-557 moved the settings onto the lead they describe, so scanning is on + // whenever a `fleet.leaders:` entry exists — with no leads configured the supplier is a + // constant and never touches herdr, exactly as a missing `leadScan:` block used to behave. + // CB-579: each lead now names its own exact `tab:` label, so one scanner discovers every + // configured lead regardless of how differently their tabs are labelled — the old + // single-shared-tabPrefix limitation (and its warning) is gone. final Supplier> leads; var leaders = cfg.fleet().leaders(); if (!leaders.isEmpty()) { - // One scanner, so one prefix and one interval. Distinct per-lead prefixes would need a - // scanner each; until a config actually wants that, take the first entry's settings and - // say so, rather than silently honouring one lead's prefix and dropping another's. - var scan = leaders.values().iterator().next(); Set memberSpaces = cfg.profiles().values().stream() .map(BridgedConfig.Profile::workspace) .filter(Objects::nonNull) .collect(Collectors.toSet()); - leads = new LeadTabScanner(herdr, scan.tabPrefix(), memberSpaces, leadTerminals, - TimeUnit.SECONDS.toNanos(scan.scanIntervalSeconds()), System::nanoTime); - log.info("lead scan: tabs labelled '{}…' host a lead (rescan every {}s, member spaces {} " - + "excluded)", - scan.tabPrefix(), scan.scanIntervalSeconds(), memberSpaces); - long distinctPrefixes = leaders.values().stream() - .map(BridgedConfig.Leader::tabPrefix).distinct().count(); - if (distinctPrefixes > 1) { - log.warn("fleet.leaders declares {} different tabPrefix values; only '{}' is scanned " - + "for. Give every lead the same tabPrefix, or leads under the others " - + "will not be discovered.", - distinctPrefixes, scan.tabPrefix()); - } + Map tabToName = new LinkedHashMap<>(); + leaders.forEach((name, leader) -> { + if (leader != null && leader.tab() != null && !leader.tab().isBlank()) { + tabToName.put(leader.tab(), name); + } + }); + // One shared rescan cadence: still taken from the first entry, as before — it is an + // operational cadence, not identity, so there is no correctness reason to give every + // lead its own scanner. + int scanIntervalSeconds = leaders.values().iterator().next().scanIntervalSeconds(); + leads = new LeadTabScanner(herdr, tabToName, memberSpaces, + TimeUnit.SECONDS.toNanos(scanIntervalSeconds), System::nanoTime); + log.info("lead scan: tabs {} host a lead (rescan every {}s, member spaces {} excluded)", + tabToName.keySet(), scanIntervalSeconds, memberSpaces); } else { leads = () -> leadTerminals; } 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 d072ff0..30f74b2 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -458,24 +458,32 @@ public record BridgedConfig( * pre-existed, which is why it had to be recognised by configuration rather than created. With * {@code profile} and {@code instances} the daemon may stand one up when none is live, so the * pane no longer has to exist before the daemon does. Recognition still comes first: a lead - * already running under {@code tabPrefix} is adopted, and only the shortfall is launched. + * already running in its configured {@code tab} is adopted, and only the shortfall is launched. + * + *

{@code tab} replaced {@code terminal} (CB-579). A herdr {@code terminal_id} changes + * every time the lead's session restarts, so pinning one cost a config edit and a daemon restart + * per restart. A tab is stable: a human opens it once, it holds exactly one pane, and its label + * survives restarts of the agent inside it — so identity is now the tab label alone. * * @param profile the {@code profiles:} entry to launch this lead on when one must * be created; {@code null} ⇒ recognise-only, never create - * @param terminal the lead's herdr {@code terminal_id} when pinned by hand; the only - * field identity depends on. {@code null} ⇒ found by {@code tabPrefix} + * @param tab the exact tab label hosting this lead, matched case-insensitively; + * the only field identity depends on. Required — a lead with no + * {@code tab} can never be discovered, launched or not * @param instances how many of this lead should be live (default 1). The daemon * launches only the shortfall, so a restart adopts rather than doubles - * @param tabPrefix label prefix marking this lead's tab, matched case-insensitively; - * the remainder is the lead's name ({@code "lead: opus"} → - * {@code opus}). Default {@code "lead:"} + * @param tabPrefix no longer used to find a lead's tab — {@code tab} is matched + * exactly. Its only remaining job is the startup collision guard + * ({@link #validateLeadTabPrefixes()}), which still uses it to refuse + * a worker {@code tabLabel} template that could be misread as a lead. + * Default {@code "lead:"} * @param scanIntervalSeconds how long a tab scan is cached before herdr is asked again; also the * worst case before a newly-labelled tab is recognised. Default 10 * @param kind which agent runs there ({@code claude}, {@code opencode}, …) * @param model the model or selector it runs, for operators reading the roster */ @JsonIgnoreProperties(ignoreUnknown = true) - public record Leader(String profile, String terminal, Integer instances, String tabPrefix, + public record Leader(String profile, String tab, Integer instances, String tabPrefix, Integer scanIntervalSeconds, String kind, String model, String workspace, String cwd) { @@ -493,12 +501,13 @@ public record BridgedConfig( (scanIntervalSeconds == null || scanIntervalSeconds <= 0) ? 10 : scanIntervalSeconds; workspace = (workspace == null || workspace.isBlank()) ? DEFAULT_WORKSPACE : workspace.strip(); + tab = (tab == null || tab.isBlank()) ? null : tab.strip(); } /** Back-compat 7-arg form — no workspace or cwd, so both take their defaults. */ - public Leader(String profile, String terminal, Integer instances, String tabPrefix, + public Leader(String profile, String tab, Integer instances, String tabPrefix, Integer scanIntervalSeconds, String kind, String model) { - this(profile, terminal, instances, tabPrefix, scanIntervalSeconds, kind, model, null, null); + this(profile, tab, instances, tabPrefix, scanIntervalSeconds, kind, model, null, null); } /** True when this lead may be launched by the daemon rather than only recognised. */ @@ -506,9 +515,9 @@ public record BridgedConfig( return profile != null && !profile.isBlank() && instances > 0; } - /** The tab label an auto-launched instance of this lead gets — what the scanner reads back. */ - public String tabLabel(String name) { - return tabPrefix + " " + name; + /** The tab label an auto-launched instance of this lead gets — its configured {@code tab}. */ + public String tabLabel() { + return tab; } } @@ -707,28 +716,20 @@ public record BridgedConfig( } /** - * The terminal → lead-name map that {@link dev.ltms.bridged.auth.CallerResolver} resolves - * against, merging the {@code leaders:} registry with the legacy singular {@code primary:} pin. + * The terminal → lead-name map seeded from the legacy singular {@code primary:} pin (CB-530). * - *

Precedence: an explicit {@code leaders:} entry wins over the {@code primary:} pin for the - * same terminal. The pin is the older, less expressive spelling of the same fact, so when both - * name a pane the named entry is the one an operator meant. The pin is still honoured on its - * own — a config carrying only {@code primary:} behaves exactly as it did before CB-530. + *

{@code fleet.leaders} no longer carries a per-entry terminal pin (CB-579): a lead's identity + * comes from its {@code tab} alone, resolved live by {@code LeadTabScanner}. This method now + * exists only for the {@code primary.terminal} fallback — a config that never migrated off it + * still resolves that one pane as a lead named {@code "primary"}, exactly as before CB-530. * - * @return an unmodifiable map, empty when neither block is configured (nothing is pinned, and - * every pane therefore resolves as a worker — the pre-CB-307 behaviour) + * @return an unmodifiable map, empty when {@code primary.terminal} is not configured (nothing is + * pinned, and every pane therefore resolves as a worker — the pre-CB-307 behaviour) */ public Map leaderTerminals() { Map byTerminal = new LinkedHashMap<>(); - if (fleet != null) { - fleet.leaders().forEach((name, leader) -> { - if (leader != null && leader.terminal() != null && !leader.terminal().isBlank()) { - byTerminal.put(leader.terminal(), name); - } - }); - } if (primary != null && primary.terminal() != null && !primary.terminal().isBlank()) { - byTerminal.putIfAbsent(primary.terminal(), "primary"); + byTerminal.put(primary.terminal(), "primary"); } return Collections.unmodifiableMap(byTerminal); } @@ -840,6 +841,7 @@ public record BridgedConfig( try { String yaml = Files.readString(path); rejectRenamedTopLevelKeys(yaml); + rejectLeaderTerminalKey(yaml); warnUnknownTopLevelKeys(yaml, path); rejectDuplicateMemberSlots(yaml); BridgedConfig cfg = YAML.readValue(yaml, BridgedConfig.class); @@ -1064,6 +1066,43 @@ public record BridgedConfig( } } + /** + * Reject a config whose {@code fleet.leaders.} still carries the retired {@code terminal:} + * pin (CB-579), naming {@code tab:} as its replacement. + * + *

{@code Leader} is {@code @JsonIgnoreProperties(ignoreUnknown = true)}, so simply dropping + * the record component would make a leftover {@code terminal:} key silently no-op — the daemon + * would start, the pin would never take effect, and nothing would say why. Fatal and specific + * instead, exactly like {@link #rejectRenamedTopLevelKeys}, which this mirrors for a key one + * level deeper than the ones that method covers. + * + * @param yaml the raw config text + * @throws IllegalStateException when any {@code fleet.leaders..terminal} key is present + */ + static void rejectLeaderTerminalKey(String yaml) { + Map raw; + try { + raw = YAML.readValue(yaml, Map.class); + } catch (IOException | IllegalArgumentException e) { + return; // a malformed file is reported by the real parse, not here + } + if (raw == null || !(raw.get("fleet") instanceof Map fleet) + || !(fleet.get("leaders") instanceof Map leaders)) { + return; + } + List bad = leaders.entrySet().stream() + .filter(e -> e.getValue() instanceof Map leader && leader.containsKey("terminal")) + .map(e -> String.valueOf(e.getKey())) + .sorted() + .toList(); + if (!bad.isEmpty()) { + throw new IllegalStateException("refusing to start: fleet.leaders entries [" + + String.join(", ", bad) + "] still use the retired 'terminal:' key — replace it " + + "with 'tab:', the exact tab label hosting the lead. A terminal_id changes on " + + "every restart of the lead's session; a tab label does not."); + } + } + static List unknownTopLevelKeys(String yaml) { Map raw; try { @@ -1313,10 +1352,9 @@ public record BridgedConfig( + "', which is not a configured profiles: entry (have: " + profiles.keySet() + ")."); } - if (!leader.isCreatable() && (leader.terminal() == null || leader.terminal().isBlank())) { - bad.add("fleet.leaders." + name + " can neither be found nor created — it pins no " - + "terminal: and names no profile: to launch one on. Give it one or the " - + "other, or drop the entry."); + if (leader.tab() == null || leader.tab().isBlank()) { + bad.add("fleet.leaders." + name + " has no tab: — a lead is now found (and, if " + + "auto-launched, labelled) purely by its tab, so every entry must name one."); } }); if (!bad.isEmpty()) { diff --git a/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java b/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java index 6393994..d145555 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/ConfigRef.java @@ -30,7 +30,11 @@ import java.util.function.Supplier; * {@code fleet:} (every role pool, {@code charters}, and {@code tabLabel}), * {@code placement:}, and an existing profile's {@code weight} / {@code maxLoad}. Those * three are read through a supplier on {@code CompositePeerLauncher}, which is what makes - * them hot — not the fact that they are config. + * them hot — not the fact that they are config. This does NOT include + * {@code fleet.leaders}: {@code Bridged.main} reads {@code cfg.fleet().leaders()} + * once at startup to build the {@code LeadTabScanner} and the {@code LeadLauncher}, and + * neither is reconstructed on reload — so a lead added, removed, or re-{@code tab}'d under + * {@code fleet.leaders} needs a restart, the same as any deferred key below. *

  • Deferred — accepted into the new snapshot, but the wiring built at startup * keeps the old value until a restart: {@code lifecycle:}, {@code leadHeartbeat:}, * {@code spawnReadyTimeoutMs} / {@code spawnReadyPollMs}, {@code guard:}, diff --git a/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java b/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java index 522d581..6297d75 100644 --- a/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java +++ b/bridged/src/main/java/dev/ltms/bridged/herdr/LeadTabScanner.java @@ -6,6 +6,7 @@ import org.slf4j.LoggerFactory; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.function.LongSupplier; @@ -23,6 +24,15 @@ import java.util.function.Supplier; * by first starting the session and asking it. Scanning closes that loop: label the tab, and the * pane is recognised on the next resolve. * + *

    CB-579 — matched by name, not prefix. This used to strip one shared + * {@code tabPrefix} off a label to derive the lead's name, and merged a config-supplied + * {@code terminal_id} pin over every scan result so the pin could never expire. Both are gone: each + * lead now configures its own exact {@code tab} label ({@code fleet.leaders..tab}), so this + * class is handed a {@code tab → name} map up front and matches labels against it exactly + * (case-insensitively). There is no merge step — a scan result is the whole answer. That is the + * fix for the bug this replaces: a {@code terminal_id} pin surviving in config after the pane it + * named was gone, so the daemon kept treating a dead session as a live lead forever. + * *

    Direction of trust. The label names the lead; it never grants * anything a pane could take for itself. Three properties keep that honest: *

      @@ -48,7 +58,7 @@ import java.util.function.Supplier; * ever make a decision that removes something based on this map, add the same check. * The remaining hazard is an operator one — a worker {@code tabLabel} template that * happens to start with the same prefix would promote the whole fleet — and that is refused at - * startup by {@code BridgedConfig.validateLeadScan} rather than documented here. + * startup by {@code BridgedConfig.validateLeadTabPrefixes} rather than documented here. * *

      Caching. {@link #get()} is on the request path (every resolve), so the scan * is TTL-cached and a stale-but-valid map is preferred to a herdr round-trip. A failed scan keeps @@ -60,38 +70,46 @@ public final class LeadTabScanner implements Supplier> { private static final Logger log = LoggerFactory.getLogger(LeadTabScanner.class); private final HerdrClient herdr; - private final String tabPrefix; + private final Map tabToName; private final Set excludedWorkspaceLabels; - private final Map configuredLeads; private final long ttlNanos; private final LongSupplier clock; - private Map cached; + private Map cached = Map.of(); private long scannedAtNanos; private boolean everScanned; /** * @param herdr the herdr client to query ({@code workspace.list}, * {@code tab.list}, {@code pane.list} — all read-only) - * @param tabPrefix a tab whose label starts with this (case-insensitively) hosts a - * lead; the rest of the label, trimmed, is the lead's name + * @param tabToName every configured lead's exact tab label → its name + * ({@code fleet.leaders..tab}), matched case-insensitively * @param excludedWorkspaceLabels workspaces never scanned — the configured worker spaces - * @param configuredLeads the static {@code leaders:}/{@code primary:} registry, merged - * over every scan result. Explicit config outranks the - * convention, and survives a scan that cannot run at all * @param ttlNanos how long a scan result is reused before the next one * @param clock nanosecond time source ({@code System::nanoTime} in production) */ - public LeadTabScanner(HerdrClient herdr, String tabPrefix, Set excludedWorkspaceLabels, - Map configuredLeads, long ttlNanos, LongSupplier clock) { + public LeadTabScanner(HerdrClient herdr, Map tabToName, + Set excludedWorkspaceLabels, long ttlNanos, LongSupplier clock) { this.herdr = herdr; - this.tabPrefix = tabPrefix == null || tabPrefix.isBlank() ? "lead:" : tabPrefix.strip(); + this.tabToName = normalize(tabToName); this.excludedWorkspaceLabels = excludedWorkspaceLabels == null ? Set.of() : Set.copyOf(excludedWorkspaceLabels); - this.configuredLeads = configuredLeads == null ? Map.of() : Map.copyOf(configuredLeads); this.ttlNanos = ttlNanos; this.clock = clock; - this.cached = this.configuredLeads; + } + + /** Keys stripped and lower-cased once, so every lookup is a plain map hit. */ + private static Map normalize(Map tabToName) { + if (tabToName == null || tabToName.isEmpty()) { + return Map.of(); + } + Map out = new LinkedHashMap<>(); + tabToName.forEach((tab, name) -> { + if (tab != null && !tab.isBlank() && name != null && !name.isBlank()) { + out.put(tab.strip().toLowerCase(Locale.ROOT), name); + } + }); + return Collections.unmodifiableMap(out); } /** @@ -151,25 +169,20 @@ public final class LeadTabScanner implements Supplier> { } } } - byTerminal.putAll(configuredLeads); // an explicit pin outranks a label return Collections.unmodifiableMap(byTerminal); } /** - * The lead name a tab label declares, or {@code null} if it declares none. + * The lead name a tab label declares, or {@code null} if it names none of the configured leads. * - *

      {@code "lead: opus-5.0"} → {@code "opus-5.0"}. A bare {@code "lead:"} names nobody and is - * rejected: an unnamed lead would resolve as {@code PRIMARY} with nothing to attribute it to. + *

      Exact match (case-insensitive, ends stripped) against {@link #tabToName} — no prefix + * stripping, so an operator's {@code "lead: something-else"} tab is never mistaken for a + * configured lead just because it shares a prefix. */ private String leadNameOf(String label) { if (label == null) { return null; } - String l = label.strip(); - if (!l.regionMatches(true, 0, tabPrefix, 0, tabPrefix.length())) { - return null; - } - String name = l.substring(tabPrefix.length()).strip(); - return name.isEmpty() ? null : name; + return tabToName.get(label.strip().toLowerCase(Locale.ROOT)); } } diff --git a/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java b/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java index 0693893..d6d2f83 100644 --- a/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/lead/LeadLauncher.java @@ -59,7 +59,7 @@ public final class LeadLauncher { /** * @param agents herdr agent control (start, list) * @param spaces workspace / tab control (ensure, create, label, list) - * @param cfg the loaded config — {@code fleet.leaders}, {@code profiles} and the lead pins + * @param cfg the loaded config — {@code fleet.leaders}, {@code profiles} and each lead's tab */ public LeadLauncher(AgentControl agents, WorkspaceControl spaces, BridgedConfig cfg) { this.agents = agents; @@ -102,8 +102,8 @@ public final class LeadLauncher { continue; } if (!lead.isCreatable()) { - // A lead with a `terminal:` pin and no `profile:` is recognise-only by design: the - // operator opens it by hand. Say so once rather than looking like a silent failure. + // A lead with a `tab:` but no `profile:` is recognise-only by design: the operator + // opens it by hand. Say so once rather than looking like a silent failure. log.info("lead '{}' is not live, and names no profile — it can be recognised but not " + "launched. Add `profile:` under fleet.leaders.{} to have bridged start it.", name, name); @@ -127,18 +127,16 @@ public final class LeadLauncher { } /** - * How many live leads exist per configured name. + * How many live leads exist per configured name: a running agent in a tab labelled with that + * lead's exact {@code tab} (CB-579). Member workspaces are excluded, exactly as the scanner + * excludes them: a member must not be counted as a lead because it happens to sit in a matching + * tab. * - *

      Two independent pieces of evidence, because either alone double-spawns: - *

        - *
      • a running agent in a tab labelled {@code " "} — how an auto-launched - * lead, or an operator following the labelling convention, is found;
        • - *
      • a running agent on a terminal the config pins in {@code fleet.leaders..terminal} — - * how a lead the operator opened and pinned by hand is found. Without this, a pinned lead - * whose tab carries no matching label would be relaunched on every boot.
        • - *
      - * Member workspaces are excluded, exactly as the scanner excludes them: a member must not be - * counted as a lead because it happens to sit in a matching tab. + *

      There used to be a second path here — a running agent on the terminal a + * {@code fleet.leaders..terminal} pin named, for a lead opened and pinned by hand. That + * pin is retired: {@code tab} is now the only field identity depends on, and {@link Agent} + * already carries {@link Agent#tabId()} directly, so a hand-opened lead is found the same way an + * auto-launched one is — by labelling its tab to match. */ private Map liveLeads(Map leaders) { Set memberSpaces = cfg.profiles().values().stream() @@ -160,20 +158,9 @@ public final class LeadLauncher { } } - // terminalId → the lead name the config pins it to. - Map nameByPinnedTerminal = new LinkedHashMap<>(); - leaders.forEach((name, lead) -> { - if (lead.terminal() != null && !lead.terminal().isBlank()) { - nameByPinnedTerminal.put(lead.terminal().strip(), name); - } - }); - Map counts = new LinkedHashMap<>(); for (Agent a : agents.list()) { String name = nameByTab.get(a.tabId()); - if (name == null) { - name = nameByPinnedTerminal.get(a.terminalId()); - } if (name != null) { counts.merge(name, 1, Integer::sum); } @@ -184,7 +171,7 @@ public final class LeadLauncher { /** * The configured lead a tab label names, or {@code null} for a label that names none. * - *

      Matched against the declared lead names rather than by splitting on the prefix, so an + *

      Matched exactly (case-insensitively) against each lead's configured {@code tab}, so an * operator's {@code "lead: something-else"} tab is not mistaken for a configured lead. */ private String leadNameOf(String label, Map leaders) { @@ -193,7 +180,8 @@ public final class LeadLauncher { } String l = label.strip(); for (Map.Entry e : leaders.entrySet()) { - if (l.equalsIgnoreCase(e.getValue().tabLabel(e.getKey()).strip())) { + String tab = e.getValue().tabLabel(); + if (tab != null && l.equalsIgnoreCase(tab.strip())) { return e.getKey(); } } @@ -202,7 +190,7 @@ public final class LeadLauncher { /** Start one lead. Returns false (having logged) rather than throwing on any failure. */ private boolean launch(String name, BridgedConfig.Leader lead, BridgedConfig.Profile profile) { - String label = lead.tabLabel(name); + String label = lead.tabLabel(); String cwd = (lead.cwd() == null || lead.cwd().isBlank()) ? System.getProperty("user.dir") : lead.cwd(); 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 52f2284..475bc37 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -194,7 +194,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "lead: opus" """); BridgedConfig.Leader lead = BridgedConfig.load(f).fleet().leaders().get("opus"); @@ -212,7 +212,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "drive: opus" tabPrefix: "drive:" scanIntervalSeconds: 30 """); @@ -224,7 +224,9 @@ class BridgedConfigTest { /** * The pane no longer has to exist before the daemon does (CB-557): a lead naming a profile may - * be launched, while one that names only a terminal is recognised and never created. + * be launched, while one that names no profile is recognised and never created. Either way it + * still needs its own {@code tab:} (CB-579) — that part is unconditional, see + * {@link #aLeadWithNoTabRefusesToStart}. */ @Test void aLeadIsCreatableOnlyWhenItNamesAProfile(@TempDir Path dir) throws Exception { @@ -239,8 +241,9 @@ class BridgedConfigTest { leaders: launched: profile: opus + tab: "lead: launched" pinned: - terminal: term_opus + tab: "lead: pinned" """); var leaders = BridgedConfig.load(f).fleet().leaders(); @@ -249,8 +252,12 @@ class BridgedConfigTest { "no profile to launch on ⇒ recognise-only, the pre-CB-557 behaviour"); } + /** + * CB-579: {@code tab} is the only field a lead's identity depends on now, so it is required + * whether the entry is creatable or recognise-only — without it the entry can never be found. + */ @Test - void aLeadThatCanBeNeitherFoundNorCreatedRefusesToStart(@TempDir Path dir) throws Exception { + void aLeadWithNoTabRefusesToStart(@TempDir Path dir) throws Exception { Path f = dir.resolve("useless-lead.yaml"); Files.writeString(f, """ bind: @@ -264,6 +271,80 @@ class BridgedConfigTest { IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateMembers); assertTrue(e.getMessage().contains("ghost"), "the message must name the useless entry"); + assertTrue(e.getMessage().contains("tab:"), "the message must say what is missing"); + } + + /** + * CB-579 acceptance (2): a config still spelling {@code fleet.leaders..terminal} must fail + * loudly at load, not be silently dropped by {@code Leader}'s {@code @JsonIgnoreProperties}. + */ + @Test + void aLeaderTerminalKeyFailsLoadAndNamesTabAsTheReplacement(@TempDir Path dir) throws Exception { + Path f = dir.resolve("stale-terminal.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + opus: + terminal: term_opus + """); + + IllegalStateException e = + assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("opus"), "the message must name the offending entry"); + assertTrue(e.getMessage().contains("tab:"), "the message must name the replacement key"); + assertTrue(e.getMessage().contains("terminal"), "the message must name the retired key"); + } + + /** The same refusal, and it must name every offending entry, not just the first. */ + @Test + void everyLeaderStillUsingTerminalIsReportedAtOnce(@TempDir Path dir) throws Exception { + Path f = dir.resolve("stale-terminals.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + opus: + terminal: term_opus + sol: + terminal: term_sol + """); + + IllegalStateException e = + assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); + assertTrue(e.getMessage().contains("opus")); + assertTrue(e.getMessage().contains("sol")); + } + + /** A {@code terminal:} anywhere else in the document (not under a leader entry) is unaffected. */ + @Test + void aTerminalKeyOutsideFleetLeadersIsNotRejected(@TempDir Path dir) throws Exception { + Path f = dir.resolve("primary-terminal-ok.yaml"); + Files.writeString(f, "bind:\n port: 8080\nprimary:\n terminal: term_fixed\n"); + + assertDoesNotThrow(() -> BridgedConfig.load(f)); + } + + /** CB-579 acceptance (3): distinct `tab:` labels need no shared prefix — one scanner finds both. */ + @Test + void twoLeadersWithDifferentTabsAreBothConfigured(@TempDir Path dir) throws Exception { + Path f = dir.resolve("two-tabs.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + opus: + tab: "lead: opus" + sol: + tab: "captain: sol" + """); + + var leaders = BridgedConfig.load(f).fleet().leaders(); + assertEquals("lead: opus", leaders.get("opus").tab()); + assertEquals("captain: sol", leaders.get("sol").tab()); } // ── CB-551: the idle-lead heartbeat ───────────────────────────────────────────────────────── @@ -328,7 +409,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "lead: opus" tabPrefix: "lead:" """); BridgedConfig cfg = BridgedConfig.load(f); @@ -349,7 +430,7 @@ class BridgedConfigTest { tabLabel: "lead: {role} {profile}" leaders: opus: - terminal: term_opus + tab: "lead: opus" """); BridgedConfig cfg = BridgedConfig.load(f); @@ -374,7 +455,7 @@ class BridgedConfigTest { fleet: leaders: opus: - terminal: term_opus + tab: "lead: opus" """); assertDoesNotThrow(() -> BridgedConfig.load(f).validateLeadTabPrefixes()); @@ -400,7 +481,7 @@ class BridgedConfigTest { "a label that collides with a convention nobody reads is not a problem"); } - // ── CB-530: the leaders registry ──────────────────────────────────────────────────────────── + // ── CB-530/CB-579: the leaders registry ───────────────────────────────────────────────────── @Test void leadersBlockRegistersEveryPaneByName(@TempDir Path dir) throws Exception { @@ -411,10 +492,10 @@ class BridgedConfigTest { fleet: leaders: opus-5.0: - terminal: term_opus + tab: "lead: opus-5.0" kind: claude gpt-sol-5.6: - terminal: term_sol + tab: "lead: gpt-sol-5.6" kind: opencode model: openai/gpt-5.6-terra """); @@ -425,9 +506,9 @@ class BridgedConfigTest { assertEquals(Set.of("opus-5.0", "gpt-sol-5.6"), leaders.keySet()); assertEquals("opencode", leaders.get("gpt-sol-5.6").kind()); assertEquals("openai/gpt-5.6-terra", leaders.get("gpt-sol-5.6").model()); - // The whole point: BOTH panes resolve as leads, so neither is demoted to worker. - assertEquals(Map.of("term_opus", "opus-5.0", "term_sol", "gpt-sol-5.6"), - cfg.leaderTerminals()); + // Identity is the tab now (CB-579) — both entries carry their own, distinct label. + assertEquals("lead: opus-5.0", leaders.get("opus-5.0").tab()); + assertEquals("lead: gpt-sol-5.6", leaders.get("gpt-sol-5.6").tab()); } @Test @@ -439,42 +520,6 @@ class BridgedConfigTest { "configs that never migrate must behave exactly as they did before CB-530"); } - @Test - void anExplicitLeadersEntryWinsOverThePinForTheSameTerminal(@TempDir Path dir) throws Exception { - Path f = dir.resolve("both.yaml"); - Files.writeString(f, """ - bind: - port: 8080 - primary: - terminal: term_shared - fleet: - leaders: - opus-5.0: - terminal: term_shared - """); - - assertEquals(Map.of("term_shared", "opus-5.0"), BridgedConfig.load(f).leaderTerminals(), - "the pin is the older spelling of the same fact; the named entry is what was meant"); - } - - @Test - void bothBlocksTogetherRegisterTheUnionOfTheirTerminals(@TempDir Path dir) throws Exception { - Path f = dir.resolve("union.yaml"); - Files.writeString(f, """ - bind: - port: 8080 - primary: - terminal: term_pinned - fleet: - leaders: - gpt-sol-5.6: - terminal: term_sol - """); - - assertEquals(Map.of("term_pinned", "primary", "term_sol", "gpt-sol-5.6"), - BridgedConfig.load(f).leaderTerminals()); - } - @Test void neitherBlockLeavesNothingRegistered(@TempDir Path dir) throws Exception { Path f = dir.resolve("none.yaml"); @@ -483,22 +528,25 @@ class BridgedConfigTest { assertTrue(BridgedConfig.load(f).leaderTerminals().isEmpty()); } - /** A lead entry with no terminal identifies nothing — it must not register a null key. */ + /** + * CB-579: {@code fleet.leaders} no longer feeds {@code leaderTerminals()} at all — a lead's + * identity comes from the live tab scan, not a config-held terminal map. This method now exists + * only for the {@code primary.terminal} fallback. + */ @Test - void aLeadWithoutATerminalIsNotRegistered(@TempDir Path dir) throws Exception { - Path f = dir.resolve("no-terminal.yaml"); + void fleetLeadersNeverContributesToLeaderTerminals(@TempDir Path dir) throws Exception { + Path f = dir.resolve("leaders-only.yaml"); Files.writeString(f, """ bind: port: 8080 fleet: leaders: - sketch: - kind: opencode - real: - terminal: term_real + opus-5.0: + tab: "lead: opus-5.0" """); - assertEquals(Map.of("term_real", "real"), BridgedConfig.load(f).leaderTerminals()); + assertTrue(BridgedConfig.load(f).leaderTerminals().isEmpty(), + "no primary.terminal pin ⇒ nothing registered, even with fleet.leaders configured"); } // ── CB-548: the architects registry ──────────────────────────────────────────────────────── diff --git a/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java b/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java index 8ccb2d6..b06c405 100644 --- a/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/herdr/LeadTabScannerTest.java @@ -15,8 +15,9 @@ import java.util.concurrent.atomic.AtomicLong; import static org.junit.jupiter.api.Assertions.*; /** - * CB-531. A lead is never spawned, so the daemon has to find it: these assert that an - * operator-labelled tab is what makes a pane a lead, and — just as importantly — what does not. + * CB-531/CB-579. A lead is never spawned, so the daemon has to find it: these assert that + * an operator-labelled tab matching a configured {@code tab:} is what makes a pane a lead, and — + * just as importantly — what does not, and that a stale entry does not linger forever. */ class LeadTabScannerTest { @@ -120,23 +121,48 @@ class LeadTabScannerTest { .pane("w9:p1", "w9:t1", "term_worker"); } - private LeadTabScanner scanner(TopologyHerdr herdr, Map configured, + /** The {@code tab:} → name map {@code twoLeads()}'s two lead tabs are configured under. */ + private static Map twoLeadsConfigured() { + return Map.of("lead: opus-5.0", "opus-5.0", "lead: gpt-sol-5.6", "gpt-sol-5.6"); + } + + private LeadTabScanner scanner(TopologyHerdr herdr, Map tabToName, AtomicLong clock) { - return new LeadTabScanner(herdr, "lead:", Set.of("bridged-workers"), configured, TTL, - clock::get); + return new LeadTabScanner(herdr, tabToName, Set.of("bridged-workers"), TTL, clock::get); } @Test - void everyLabelledTabBecomesALeadNamedByItsLabel() { - Map leads = scanner(twoLeads(), Map.of(), new AtomicLong()).get(); + void everyConfiguredTabBecomesALeadNamedByItsEntry() { + Map leads = scanner(twoLeads(), twoLeadsConfigured(), new AtomicLong()).get(); assertEquals(Map.of("term_opus", "opus-5.0", "term_gpt", "gpt-sol-5.6"), leads, - "two leads discovered from labels alone — no terminal_id was ever configured"); + "two leads discovered by their configured tab — no terminal_id was ever configured"); } @Test - void anUnlabelledTabContributesNothing() { - assertFalse(scanner(twoLeads(), Map.of(), new AtomicLong()).get().containsKey("term_notes")); + void anUnconfiguredTabContributesNothing() { + assertFalse(scanner(twoLeads(), twoLeadsConfigured(), new AtomicLong()) + .get().containsKey("term_notes")); + } + + /** + * CB-579: matching is exact against the configured map now, not a shared prefix — two leads with + * completely different labels are both discovered by one scanner, no convention required. + */ + @Test + void twoLeadsWithCompletelyDifferentLabelsAreBothDiscovered() { + TopologyHerdr herdr = new TopologyHerdr() + .workspace("w1", "main") + .tab("w1:t1", "w1", "orchestrator: opus") + .tab("w1:t2", "w1", "captain: sol") + .pane("w1:p1", "w1:t1", "term_opus") + .pane("w1:p2", "w1:t2", "term_sol"); + Map tabToName = Map.of("orchestrator: opus", "opus", "captain: sol", "sol"); + + Map leads = scanner(herdr, tabToName, new AtomicLong()).get(); + + assertEquals(Map.of("term_opus", "opus", "term_sol", "sol"), leads, + "no shared prefix needed — each lead is matched by its own configured tab"); } /** @@ -148,25 +174,28 @@ class LeadTabScannerTest { void aTabInAWorkerSpaceIsNeverALeadEvenWhenItsLabelMatches() { TopologyHerdr herdr = twoLeads().tab("w9:t2", "w9", "lead: impostor") .pane("w9:p2", "w9:t2", "term_impostor"); + Map tabToName = new LinkedHashMap<>(twoLeadsConfigured()); + tabToName.put("lead: impostor", "impostor"); - assertFalse(scanner(herdr, Map.of(), new AtomicLong()).get().containsKey("term_impostor")); + assertFalse(scanner(herdr, tabToName, new AtomicLong()).get().containsKey("term_impostor")); } @Test - void aBarePrefixNamesNobodyAndIsRejected() { + void aLabelWithNoConfiguredEntryIsIgnored() { TopologyHerdr herdr = new TopologyHerdr().workspace("w1", "main") - .tab("w1:t1", "w1", "lead:").pane("w1:p1", "w1:t1", "term_a"); + .tab("w1:t1", "w1", "lead: nobody-configured").pane("w1:p1", "w1:t1", "term_a"); - assertEquals(Map.of(), scanner(herdr, Map.of(), new AtomicLong()).get(), - "a lead with no name would resolve as PRIMARY with nothing to attribute it to"); + assertEquals(Map.of(), scanner(herdr, twoLeadsConfigured(), new AtomicLong()).get(), + "a label that names no configured lead resolves nobody"); } @Test - void thePrefixMatchesCaseInsensitivelyAndTheNameIsTrimmed() { + void matchingIsCaseInsensitiveAndToleratesSurroundingWhitespace() { TopologyHerdr herdr = new TopologyHerdr().workspace("w1", "main") - .tab("w1:t1", "w1", " LEAD: opus-5.0 ").pane("w1:p1", "w1:t1", "term_a"); + .tab("w1:t1", "w1", " LEAD: Opus-5.0 ").pane("w1:p1", "w1:t1", "term_a"); - assertEquals(Map.of("term_a", "opus-5.0"), scanner(herdr, Map.of(), new AtomicLong()).get()); + assertEquals(Map.of("term_a", "opus-5.0"), + scanner(herdr, Map.of("lead: Opus-5.0", "opus-5.0"), new AtomicLong()).get()); } @Test @@ -175,18 +204,51 @@ class LeadTabScannerTest { // nothing bridged placed can land here (see the worker-space test above). TopologyHerdr herdr = twoLeads().pane("w1:p1b", "w1:t1", "term_opus_split"); - assertEquals("opus-5.0", scanner(herdr, Map.of(), new AtomicLong()).get().get("term_opus_split")); + assertEquals("opus-5.0", + scanner(herdr, twoLeadsConfigured(), new AtomicLong()).get().get("term_opus_split")); } + /** + * CB-579 acceptance (6): this is the bug the ticket closes. A stale pin used to be merged back + * over every scan and never expire; now a scan is the whole answer, so a lead whose tab is gone + * drops out on the very next scan. + */ @Test - void anExplicitlyConfiguredLeadIsMergedInAndOutranksALabel() { - Map configured = Map.of("term_opus", "pinned-name", "term_extra", "from-config"); + void aTabNoLongerPresentDropsTheLeadOnTheNextScan() { + TopologyHerdr herdr = twoLeads(); + AtomicLong clock = new AtomicLong(); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); + assertTrue(s.get().containsKey("term_opus")); - Map leads = scanner(twoLeads(), configured, new AtomicLong()).get(); + // The session behind term_opus restarted — herdr no longer reports that tab or pane at all. + herdr.tabs.remove("w1:t1"); + herdr.panes.remove("w1:p1"); + clock.addAndGet(TTL); - assertEquals("pinned-name", leads.get("term_opus"), "an explicit pin is the operator's last word"); - assertEquals("from-config", leads.get("term_extra"), "a configured lead needs no tab at all"); - assertEquals("gpt-sol-5.6", leads.get("term_gpt")); + assertFalse(s.get().containsKey("term_opus"), + "a stale entry must expire once the tab it named is gone, not be merged back forever"); + } + + /** + * CB-579 acceptance (5): the whole point of matching by tab instead of {@code terminal_id} — a + * restart changes the terminal, not the tab, so the lead resolves under the same name with no + * config edit. + */ + @Test + void aLeadRestartingInTheSameTabResolvesUnderTheSameName() { + TopologyHerdr herdr = twoLeads(); + AtomicLong clock = new AtomicLong(); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); + assertEquals("opus-5.0", s.get().get("term_opus")); + + // The session restarts: herdr assigns the pane a new terminal_id, same tab (w1:t1). + herdr.panes.remove("w1:p1"); + herdr.pane("w1:p1", "w1:t1", "term_opus_v2"); + clock.addAndGet(TTL); + + Map leads = s.get(); + assertEquals("opus-5.0", leads.get("term_opus_v2"), "the new terminal resolves immediately"); + assertFalse(leads.containsKey("term_opus"), "the old terminal_id is simply gone, not carried"); } // ── caching ───────────────────────────────────────────────────────────────────────────────── @@ -195,7 +257,7 @@ class LeadTabScannerTest { void aSecondLookupWithinTheTtlDoesNotTouchHerdr() { TopologyHerdr herdr = twoLeads(); AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); s.get(); int afterFirst = herdr.calls; @@ -210,21 +272,23 @@ class LeadTabScannerTest { void aTabLabelledAfterStartupIsPickedUpOnceTheTtlExpires() { TopologyHerdr herdr = twoLeads(); AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + Map tabToName = new LinkedHashMap<>(twoLeadsConfigured()); + tabToName.put("lead: late-arrival", "late-arrival"); + LeadTabScanner s = scanner(herdr, tabToName, clock); assertFalse(s.get().containsKey("term_notes")); herdr.tab("w1:t3", "w1", "lead: late-arrival"); // the operator renames their tab clock.addAndGet(TTL); assertEquals("late-arrival", s.get().get("term_notes"), - "the whole point over `leaders:`: no config edit, no restart"); + "the whole point over a config-held terminal_id: no config edit, no restart"); } @Test void aFailedScanKeepsTheLeadsAlreadyKnownRatherThanDemotingThem() { TopologyHerdr herdr = twoLeads(); AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); Map before = s.get(); herdr.failing = true; @@ -235,14 +299,14 @@ class LeadTabScannerTest { } @Test - void aFailedFirstScanStillHonoursTheConfiguredLeads() { + void aFailedFirstScanReturnsEmptyRatherThanThrowing() { TopologyHerdr herdr = twoLeads(); herdr.failing = true; - Map leads = scanner(herdr, Map.of("term_x", "opus-5.0"), new AtomicLong()).get(); + Map leads = scanner(herdr, twoLeadsConfigured(), new AtomicLong()).get(); - assertEquals(Map.of("term_x", "opus-5.0"), leads, - "config-named leads must not depend on herdr answering at all"); + assertEquals(Map.of(), leads, + "with nothing scanned yet and no override to fall back on, the map is simply empty"); } @Test @@ -250,7 +314,7 @@ class LeadTabScannerTest { TopologyHerdr herdr = twoLeads(); herdr.failing = true; AtomicLong clock = new AtomicLong(); - LeadTabScanner s = scanner(herdr, Map.of(), clock); + LeadTabScanner s = scanner(herdr, twoLeadsConfigured(), clock); s.get(); int afterFirst = herdr.calls; diff --git a/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java index 03f8bb6..3b3d771 100644 --- a/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/lead/LeadLauncherTest.java @@ -41,8 +41,8 @@ class LeadLauncherTest { null, null, fleet, null, "fixed", null).withDefaults(); } - private static BridgedConfig.Leader lead(String profile, String terminal, int instances) { - return new BridgedConfig.Leader(profile, terminal, instances, "lead:", 10, null, null, + private static BridgedConfig.Leader lead(String profile, String tab, int instances) { + return new BridgedConfig.Leader(profile, tab, instances, "lead:", 10, null, null, "leads", "/repo"); } @@ -70,16 +70,16 @@ class LeadLauncherTest { void startsTheDeclaredLeadWhenNoneIsRunning() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(1, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads()); + assertEquals(1, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertTrue(herdr.called("agent.start"), "a lead must actually be started"); assertEquals("lead-opus", startedName(herdr)); } - /** The tab is labelled so the scanner finds the lead on the next resolve. */ + /** The tab is labelled with the configured `tab:` so the scanner finds the lead on the next resolve. */ @Test - void labelsTheTabWithThePrefixTheScannerReadsBack() { + void labelsTheTabWithTheConfiguredTabValue() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); assertEquals("lead: opus", ((Map) herdr.lastCall("tab.rename").params()).get("label")); @@ -90,7 +90,7 @@ class LeadLauncherTest { void startsAsManyInstancesAsAreDeclared() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(2, launcher(herdr, configWith(lead("opus", null, 2))).ensureLeads()); + assertEquals(2, launcher(herdr, configWith(lead("opus", "lead: opus", 2))).ensureLeads()); assertEquals(2, herdr.calls.stream().filter(c -> c.method().equals("agent.start")).count()); } @@ -104,7 +104,7 @@ class LeadLauncherTest { .withTab("wL", "wL:t1", "lead: opus") .withAgent("lead-opus", "term_lead", "wL:p1", "wL:t1"); - assertEquals(0, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start"), "the live lead must not be duplicated"); } @@ -118,20 +118,23 @@ class LeadLauncherTest { .withWorkspace("wL", "leads") .withTab("wL", "wL:t1", "lead: opus"); // label only — nothing running in it - assertEquals(1, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(), + assertEquals(1, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(), "a stale label is not a lead; the lead must be relaunched"); } /** - * A lead the operator opened by hand and pinned with `terminal:` is live even though its tab - * carries no matching label. Counting labels alone would relaunch it on every boot. + * A lead the operator opened by hand is live once its tab carries the configured `tab:` label — + * CB-579 retired the `terminal:` pin, so a hand-opened lead is found the same way an + * auto-launched one is, by its tab, not by a terminal id nobody wrote down in advance. */ @Test - void aPinnedTerminalWithARunningAgentCountsAsLive() { + void aHandOpenedLeadWithTheConfiguredTabLabelCountsAsLive() { FakeHerdr herdr = new FakeHerdr() - .withAgent("hand-opened", "term_pinned", "wX:p1", "wX:t1"); + .withWorkspace("wX", "main") + .withTab("wX", "wX:t1", "lead: opus") + .withAgent("hand-opened", "term_hand", "wX:p1", "wX:t1"); - assertEquals(0, launcher(herdr, configWith(lead("opus", "term_pinned", 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -143,7 +146,7 @@ class LeadLauncherTest { .withTab("wM", "wM:t1", "lead: opus") // a member tab that looks like a lead .withAgent("claude-opus-x", "term_m", "wM:p1", "wM:t1"); - assertEquals(1, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(), + assertEquals(1, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(), "a member in a lead-labelled tab is not a lead, so the real lead is still missing"); } @@ -152,7 +155,7 @@ class LeadLauncherTest { void anUncountableHerdrStartsNothing() { FakeHerdr herdr = new FakeHerdr().healthy(false); - assertEquals(0, launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -166,7 +169,7 @@ class LeadLauncherTest { @Test void theLeadNeverReceivesTheWorkerReplyCharter() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); List args = startedArgs(herdr); assertFalse(args.contains("--append-system-prompt"), @@ -178,7 +181,7 @@ class LeadLauncherTest { @Test void theLeadMountsTheBridgeMcpAndPinsItsModel() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); List args = startedArgs(herdr); assertTrue(args.contains("--mcp-config")); @@ -192,7 +195,7 @@ class LeadLauncherTest { @Test void theLeadEnvCarriesNoAnthropicBinding() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); Map env = tabEnv(herdr); assertNull(env.get("ANTHROPIC_BASE_URL")); @@ -205,7 +208,7 @@ class LeadLauncherTest { @Test void theLeadTabIsCreatedOutsideEveryMemberWorkspace() { FakeHerdr herdr = new FakeHerdr(); - launcher(herdr, configWith(lead("opus", null, 1))).ensureLeads(); + launcher(herdr, configWith(lead("opus", "lead: opus", 1))).ensureLeads(); String label = (String) ((Map) herdr.lastCall("workspace.create").params()).get("label"); assertEquals("leads", label); @@ -214,12 +217,12 @@ class LeadLauncherTest { // ── recognise-only and misconfiguration ─────────────────────────────────────────────────── - /** A lead with a pin but no profile is recognise-only by design — not an error, not a launch. */ + /** A lead with a tab but no profile is recognise-only by design — not an error, not a launch. */ @Test void aLeadThatNamesNoProfileIsRecognisedButNeverLaunched() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(0, launcher(herdr, configWith(lead(null, "term_dead", 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead(null, "lead: dead", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -228,7 +231,7 @@ class LeadLauncherTest { void zeroInstancesLaunchesNothing() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(0, launcher(herdr, configWith(lead("opus", null, 0))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("opus", "lead: opus", 0))).ensureLeads()); assertFalse(herdr.called("agent.start")); } @@ -237,7 +240,7 @@ class LeadLauncherTest { void anUnknownProfileIsSkippedRatherThanThrown() { FakeHerdr herdr = new FakeHerdr(); - assertEquals(0, launcher(herdr, configWith(lead("nope", null, 1))).ensureLeads()); + assertEquals(0, launcher(herdr, configWith(lead("nope", "lead: opus", 1))).ensureLeads()); assertFalse(herdr.called("agent.start")); } -- 2.52.0