diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index 1179042..17179f7 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -55,34 +55,14 @@ bind: # 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 `leaders:` entry wins. -# leaders: -# opus-5.0: -# terminal: term_0123456789abcd -# kind: claude -# gpt-sol-5.6: -# terminal: term_fedcba9876543 -# kind: opencode -# model: openai/gpt-5.6-terra - -# CB-531: FIND LEADS BY TAB NAME instead of pasting terminal ids. `leaders:` above needs an id that -# only exists once the session is running, so adding a lead is: open a tab, start the agent, ask it -# bridge_whoami, edit this file, restart the daemon. This block replaces all of that with a naming -# convention — label the tab `lead: ` when you open it and the pane is recognised on the next -# rescan, with no config edit and no restart. Reopen the tab later and the id changes; the label -# does not. +# destination for its nudges. If both name the same terminal, the `fleet.leaders:` entry wins. # -# bridged NEVER writes these labels. It renames worker tabs (see `tabLabel` below) but reads lead -# tabs read-only, so what is in the tab bar is always what you typed. Two things keep the convention -# from being a way to claim leadership: the configured worker spaces are excluded from the scan, so -# nothing bridged places can land in a matching tab; and startup REFUSES a `tabPrefix` that any -# worker `tabLabel` also matches, so the two namespaces cannot overlap by accident. +# Leads are configured under `fleet.leaders:` — see THE FLEET further down. # -# Opt-in on purpose — this widens who resolves as a lead, so upgrading the daemon must never switch -# it on for you. Absent block = leads come only from `leaders:`/`primary:`, exactly as before. -# leadScan: -# tabPrefix: "lead:" # `lead: opus-5.0` ⇒ a lead named opus-5.0 (case-insensitive; default "lead:") -# intervalSeconds: 10 # rescan cadence, and the worst case before a new tab is recognised +# Two things stop the tab-name convention from becoming a way to claim leadership: the configured +# member spaces are excluded from the scan, so nothing bridged places can land in a matching tab; +# and startup REFUSES a `tabPrefix` that the fleet tabLabel template, or any per-profile `tabLabel` +# override, also matches — so the two namespaces cannot overlap by accident. # CB-551: IDLE-LEAD HEARTBEAT — nudge the single lead back to work when it has been continuously # idle (no open bridge_send driving it) past the quiet period. The fleet is one lead + architects + @@ -108,11 +88,13 @@ bind: # (${HERDR_SOCKET_PATH:-~/.config/herdr/herdr.sock}). herdrSocket: ~/.config/herdr/herdr.sock -# How worker sessions are spawned. Define one or more named profiles (backends) under -# `workers`; each key is the profile name (also the ccs profile). `defaultWorker` picks -# which one a no-argument spawn uses (bridge_spawn with no profile / POST /workers). +# How member sessions are spawned. Define one or more named profiles (backends) under +# `profiles`; each key is the profile name (also the ccs profile). A profile says only WHICH +# BACKEND — model, CLI adapter, credentials, cost. It says nothing about what a member spawned on +# it is for; that is the member's role, and roles live under `fleet:` below. Which profile an +# unqualified spawn lands on comes from that role's pool, not from a global default. # -# Shared knobs (placement/workspace/tabLabel) can be repeated per profile; they usually match. +# Shared knobs (placement/workspace) can be repeated per profile; they usually match. # placement: tab → each worker lands in its OWN tab in a dedicated worker space (default). # Use `pane` for the legacy behaviour (split the focused tab). # mcpUrl → bridged mounts the bridge MCP (--mcp-config, inline) + reply charter @@ -167,7 +149,7 @@ profiles: model: coder placement: tab workspace: bridged-workers - tabLabel: "worker: {profile} #{n}" # {profile}/{model}/{n} substituted; {n} keeps sibling tabs distinct + # tabLabel: an optional per-profile override; the fleet template usually covers it mcpUrl: http://127.0.0.1:8765/mcp tokenEnv: BRIDGED_WORKER_TOKEN argv: ["ccs", "gx10"] @@ -182,7 +164,7 @@ profiles: baseUrl: http://gx01.gw:8000 # self-hosted; ccs handles the model + token placement: tab workspace: bridged-workers - tabLabel: "worker: {profile} #{n}" + # tabLabel: an optional per-profile override; the fleet template usually covers it mcpUrl: http://127.0.0.1:8765/mcp argv: ["ccs", "gx11"] weight: 0.5 @@ -239,30 +221,64 @@ profiles: # How an unqualified spawn chooses a profile: fixed (default, reproduces pre-CB-518 behaviour), # round-robin, or weighted. Omitting this key is a strict no-op for existing configs. placement: weighted -defaultProfile: gx10 -# Named member slots. A member is anything a lead spawns. Every member has two independent -# attributes: +# THE FLEET (CB-557) — who the daemon may run, and under which role. This one block replaced four +# older keys: `leaders:`, `members:`, `leadScan:` and `defaultProfile:`. +# +# A member is anything a lead spawns, and every member has two INDEPENDENT attributes: # role — which contract: architect, dev or reviewer. It picks the launch charter, the role # file, the playbook skill and the authz row. # profile — which backend: one of the `profiles:` keys above (model, CLI adapter, cost). # They vary on their own. A reviewer may run on the same profile as the dev whose diff it reads, # which is why the two cannot be one field. # -# Only members that need a STABLE IDENTITY are declared here, because a lead addresses the same -# slot across many tickets. Architects are such members. A dev or a reviewer is anonymous and -# short-lived — the lead spawns it per task and tears it down after — so it never appears here. +# The ROLE IS THE CONTAINING KEY, not a `role:` field. That is not only tidier: a misspelled role +# used to parse into a member with no contract at all, while a misspelled pool name here simply +# declares nothing. # -# A slot is declared, not recognised: config gives no terminal, so every slot is idle at boot and -# nothing resolves to it until the spawn lifecycle binds a live terminal. Nothing here spawns one. -# members: -# architect-1: -# role: architect -# profile: opus # a strong model, on the operator's subscription -# architect-2: -# role: architect -# profile: sol # a different vendor on purpose — two architects that share a -# # model share its blind spots +# Each pool lists the profiles that role MAY run on — these are pools, not identities. That is also +# what replaced `defaultProfile:`: an unqualified spawn names a role, and that role's pool supplies +# the candidates, in definition order. A dev and a reviewer staying anonymous is exactly compatible +# with being listed here; the entry key just names the entry. +fleet: + # Optional. Template for a member tab's label; {role}, {profile}, {model} and {n} are substituted. + # {n} counts per role+profile, so `dev: sonnet #2` really is the second sonnet dev. Because {role} + # comes from a closed enum, a generated label can never begin with a lead's tabPrefix. + # tabLabel: "{role}: {profile} #{n}" + + # 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. + # + # `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. bridged NEVER writes these labels — it renames + # member tabs but reads lead tabs read-only, so the tab bar always shows what you typed. + # leaders: + # opus-5.0: + # profile: opus # omit to never create this lead, only recognise it + # instances: 1 # desired live count; only the shortfall is launched + # terminal: term_0123456789abcd # optional hand-pin; usually found by tabPrefix instead + # tabPrefix: "lead:" # `lead: opus-5.0` ⇒ a lead named opus-5.0 (case-insensitive) + # scanIntervalSeconds: 10 # rescan cadence, and the worst case before a new tab is seen + # kind: claude # descriptive; reported by bridge_whoami + # gpt-sol-5.6: + # terminal: term_fedcba9876543 + # kind: opencode + # model: openai/gpt-5.6-terra + + # architects: + # architect-1: + # profile: opus # a strong model, on the operator's subscription + # architect-2: + # profile: sol # a different vendor on purpose — two architects that share a + # # model share its blind spots + developers: + gx10: + profile: gx10 + # reviewers: + # gx10: + # profile: gx10 # the same backend may serve two roles; that is the point # Subscription boundary. A worker's base_url host MUST be one of these; the primary # must carry none. Every profile above must have its host listed here. diff --git a/bridged/src/main/java/dev/ltms/bridged/Bridged.java b/bridged/src/main/java/dev/ltms/bridged/Bridged.java index f692244..479b188 100644 --- a/bridged/src/main/java/dev/ltms/bridged/Bridged.java +++ b/bridged/src/main/java/dev/ltms/bridged/Bridged.java @@ -88,7 +88,7 @@ public final class Bridged { // refuses remote connections to a loopback socket. This throws rather than warns so the // dangerous configuration cannot be reached by ignoring a log line. cfg.validateAuthExposure(); - cfg.validateLeadScan(); + cfg.validateLeadTabPrefixes(); // 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(); @@ -123,12 +123,14 @@ public final class Bridged { if (!claudeProfiles.isEmpty() || opencodeProfiles.isEmpty()) { adapters.add(new ClaudeCodeLauncher(agents, spaces, guard, claudeProfiles, cfg.effectiveDefaultProfile(), System::getenv, - cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs())); + cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), + cfg.fleet().tabLabel())); } if (!opencodeProfiles.isEmpty()) { adapters.add(new OpenCodeLauncher(agents, spaces, opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv, - cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs())); + cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(), + cfg.fleet().tabLabel())); } AtomicReference> liveCountRef = new AtomicReference<>(_ -> 0); PeerLauncher workers = new CompositePeerLauncher( @@ -186,20 +188,33 @@ public final class Bridged { 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. Opt-in, so a config with no `leadScan:` block resolves exactly as it did under - // CB-530 — the supplier is then a constant and never touches herdr. + // 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. final Supplier> leads; - if (cfg.leadScan() != null) { - var scan = cfg.leadScan(); - Set workerSpaces = cfg.profiles().values().stream() + 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(), workerSpaces, leadTerminals, - TimeUnit.SECONDS.toNanos(scan.intervalSeconds()), System::nanoTime); - log.info("lead scan: tabs labelled '{}…' host a lead (rescan every {}s, worker spaces {} " + 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.intervalSeconds(), workerSpaces); + 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()); + } } else { leads = () -> leadTerminals; } @@ -209,10 +224,9 @@ public final class Bridged { // 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. - MemberRegistry members = new MemberRegistry( - cfg.members() == null ? Map.of() : cfg.members()); + MemberRegistry members = new MemberRegistry(cfg.fleet()); if (!members.slots().isEmpty()) { - log.info("architect slots: {} configured {} — none bound yet (a slot is idle until the " + log.info("member slots: {} configured {} — none bound yet (a slot is idle until the " + "spawn lifecycle binds a live terminal to it)", members.slots().size(), members.slots().keySet()); } diff --git a/bridged/src/main/java/dev/ltms/bridged/auth/MemberRegistry.java b/bridged/src/main/java/dev/ltms/bridged/auth/MemberRegistry.java index 2338776..2fcdf56 100644 --- a/bridged/src/main/java/dev/ltms/bridged/auth/MemberRegistry.java +++ b/bridged/src/main/java/dev/ltms/bridged/auth/MemberRegistry.java @@ -1,8 +1,11 @@ package dev.ltms.bridged.auth; import dev.ltms.bridged.config.BridgedConfig; +import dev.ltms.bridged.peer.MemberRole; +import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.Map; /** @@ -28,19 +31,60 @@ import java.util.Map; */ public final class MemberRegistry { - private final Map slots; - /** Live {@code terminal_id → slot name}; guarded by {@code this}. */ - private final Map terminalToSlot = new HashMap<>(); - - public MemberRegistry(Map slots) { - this.slots = slots == null ? Map.of() : Map.copyOf(slots); + /** + * One flattened {@code fleet:} entry. + * + *

Flattened because a slot name is unique only within its pool — {@code sonnet} may + * legitimately be both a developer and a reviewer — while a terminal binds to exactly one thing. + * The qualified {@link #key()} is what that binding uses. + * + * @param name the slot's key inside its pool + * @param role the pool it came from + * @param profile the backend it runs on + */ + public record Entry(String name, MemberRole role, String profile) { + /** {@code "architect:opus"} — unique across pools, unlike {@link #name()}. */ + public String key() { + return role.wireName() + ":" + name; + } } - /** The configured slots, keyed by gateway-local unique name. Unmodifiable snapshot. */ - public Map slots() { + private final Map slots; + /** Live {@code terminal_id → qualified slot key}; guarded by {@code this}. */ + private final Map terminalToSlot = new HashMap<>(); + + /** Flatten every role pool in {@code fleet} into one registry. Leaders are not members. */ + public MemberRegistry(BridgedConfig.Fleet fleet) { + Map flat = new LinkedHashMap<>(); + if (fleet != null) { + for (MemberRole role : MemberRole.values()) { + fleet.pool(role).forEach((name, slot) -> { + if (slot != null) { + Entry e = new Entry(name, role, slot.profile()); + flat.put(e.key(), e); + } + }); + } + } + this.slots = Collections.unmodifiableMap(flat); + } + + /** The configured slots, keyed by qualified {@link Entry#key()}. Unmodifiable snapshot. */ + public Map slots() { return slots; } + /** The slots belonging to {@code role}, in definition order. */ + public Map slotsFor(MemberRole role) { + Map out = new LinkedHashMap<>(); + slots.forEach((key, e) -> { + if (e.role() == role) { + out.put(key, e); + } + }); + return Collections.unmodifiableMap(out); + } + /** * An immutable copy of the live {@code terminal_id → slot name} bindings. * @@ -71,8 +115,14 @@ public final class MemberRegistry { * declares none */ public String profileForSlot(String slotName) { - BridgedConfig.Member a = slots.get(slotName); - return (a == null || a.profile() == null) ? null : a.profile(); + Entry e = slots.get(slotName); + return (e == null || e.profile() == null) ? null : e.profile(); + } + + /** The role a qualified slot key belongs to, or {@code null} when the key is unknown. */ + public MemberRole roleForSlot(String slotName) { + Entry e = slots.get(slotName); + return e == null ? null : e.role(); } /** True when {@code slotName} is a configured architect slot. */ 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 568f0bc..2e6015c 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -13,6 +13,8 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashMap; @@ -33,9 +35,6 @@ import java.util.Set; * cost. It says nothing about what the member spawned on it is for; that is * the member's {@code role}. Each value's {@code profile} field is defaulted * to its key at construction, so this map is always normalized - * @param defaultProfile which {@code profiles} key a no-argument spawn uses ({@code null} → the - * sole/first profile). See {@link #effectiveDefaultProfile()} for the - * resolved value * @param guard subscription-boundary allowlist * @param worktreeRoot nullable root directory for provisioned worktrees; defaults to a sibling * of the repo root @@ -48,18 +47,11 @@ import java.util.Set; * @param primary optional pinned primary terminal config ({@code null} → derived from connection); * a non-blank {@code terminal} seeds {@code PrimaryRegistry} and prevents * connection-derived overrides, CB-307 - * @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 members named member slots, keyed by gateway-local unique slot name; each pairs a - * {@code role} with the {@code profile} it runs on. Only members that need a - * stable identity are declared here — architects, today. A {@code dev} or - * {@code reviewer} is anonymous and short-lived, so the lead spawns it per - * task and it never appears in config. A slot is not recognised like - * a lead: config declares it only, and a live session becomes that member when - * the spawn lifecycle binds its terminal to the slot. Nothing here spawns one. - * @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 fleet who the daemon may run and under which role (CB-557). One block replacing + * the former {@code leaders:}, {@code members:}, {@code leadScan:} and + * {@code defaultProfile:}. Role is the containing key — {@code leaders}, + * {@code architects}, {@code developers}, {@code reviewers} — and each entry + * names the {@code profiles:} backend it runs on. See {@link Fleet} * @param leadHeartbeat opt-in idle-lead heartbeat (CB-551); {@code null} ⇒ off, and an upgraded * daemon never nudges an idle lead on its own initiative * @param placement how to choose a profile for an unqualified spawn: @@ -72,7 +64,6 @@ public record BridgedConfig( Bind bind, String herdrSocket, Map profiles, - String defaultProfile, Guard guard, String worktreeRoot, Lifecycle lifecycle, @@ -80,9 +71,7 @@ public record BridgedConfig( Integer spawnReadyPollMs, Broker broker, Primary primary, - Map leaders, - Map members, - LeadScan leadScan, + Fleet fleet, LeadHeartbeat leadHeartbeat, String placement, Auth auth) { @@ -203,7 +192,10 @@ public record BridgedConfig( tokenEnv = (tokenEnv == null || tokenEnv.isBlank()) ? "BRIDGED_WORKER_TOKEN" : tokenEnv; placement = (placement == null || placement.isBlank()) ? "tab" : placement.toLowerCase(); workspace = (workspace == null || workspace.isBlank()) ? "bridged-workers" : workspace; - tabLabel = (tabLabel == null || tabLabel.isBlank()) ? "worker: {profile} #{n}" : tabLabel; + // CB-557: no per-profile default any more. A label is generated from the member's ROLE + // ("dev: sonnet #2"), which a profile cannot know, so the template lives on `fleet:` and + // this field is only an override for a profile that wants its own. Blank ⇒ defer. + tabLabel = (tabLabel == null || tabLabel.isBlank()) ? null : tabLabel; // CB-525: .mcp.json is deliberately NOT here. Replicating the primary's MCP config gave a // worker the primary's IDE servers, which are bound to the primary's checkout — so its // navigation returned paths outside its own worktree. GitWorktrees now neutralizes that @@ -327,11 +319,24 @@ public record BridgedConfig( } /** - * Render {@link #tabLabel} for the {@code n}-th worker (substitutes - * {@code {profile}}/{@code {model}}/{@code {n}}), so sibling worker tabs are distinct. + * Render this member's tab label (CB-557): {@code {role}}, {@code {profile}}, + * {@code {model}} and {@code {n}} are substituted. + * + *

Template precedence is this profile's own {@link #tabLabel} override, then + * {@code fleetTemplate}, then {@link Fleet#DEFAULT_TAB_LABEL}. Role leads the default + * template so the tab bar reads as the fleet, and {@code n} counts per role+profile — a + * global counter left gaps in the numbering, which reads as though a sibling had died. + * + * @param fleetTemplate the {@code fleet.tabLabel} template; {@code null}/blank ⇒ the default + * @param role the member's role; {@code null} renders {@code {role}} as empty + * @param n this member's number within its role+profile pair */ - public String renderTabLabel(long n) { - return tabLabel + public String renderTabLabel(String fleetTemplate, MemberRole role, long n) { + String template = (tabLabel != null && !tabLabel.isBlank()) ? tabLabel + : (fleetTemplate != null && !fleetTemplate.isBlank()) ? fleetTemplate + : Fleet.DEFAULT_TAB_LABEL; + return template + .replace("{role}", role == null ? "" : role.wireName()) .replace("{profile}", profile == null ? "" : profile) .replace("{model}", model == null ? "" : model) .replace("{n}", Long.toString(n)); @@ -409,74 +414,155 @@ public record BridgedConfig( * lead drives a fleet, and wrong the moment two leads (say an Opus lead and an opencode lead) * work as peers — the second is silently demoted and refused every orchestration call. * - *

{@code kind} and {@code model} are descriptive only at this stage: they document what runs - * in the pane and are reported back by {@code bridge_whoami}. Nothing spawns a lead — a lead - * pre-exists, which is precisely why it must be recognised by configuration rather than created. + *

{@code kind} and {@code model} are descriptive only: they document what runs in the pane + * and are reported back by {@code bridge_whoami}. * - * @param terminal the lead's herdr {@code terminal_id}; the only field identity depends on - * @param kind which agent runs there ({@code claude}, {@code opencode}, …); descriptive - * @param model the model or selector it runs, for operators reading the roster; descriptive + *

A lead is now also creatable (CB-557). Before, nothing spawned one — a lead + * 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. + * + * @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 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 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 terminal, String kind, String model) { + public record Leader(String profile, String terminal, Integer instances, String tabPrefix, + Integer scanIntervalSeconds, String kind, String model) { + public Leader { + instances = (instances == null || instances < 0) ? 1 : instances; + tabPrefix = (tabPrefix == null || tabPrefix.isBlank()) ? "lead:" : tabPrefix.strip(); + scanIntervalSeconds = + (scanIntervalSeconds == null || scanIntervalSeconds <= 0) ? 10 : scanIntervalSeconds; + } + + /** True when this lead may be launched by the daemon rather than only recognised. */ + public boolean isCreatable() { + return profile != null && !profile.isBlank() && instances > 0; + } } /** - * One entry of the {@code members:} registry — a gateway-local named slot that pairs a role - * with the profile it runs on. + * One entry of a {@code fleet:} role pool — a role paired with the backend it runs on. * - *

Role and profile are separate axes. The {@code role} answers which contract - * — it picks the launch charter, the role file, the playbook skill and the authz row. The + *

Role and profile are separate axes. The role (the pool this slot sits in) answers + * which contract — launch charter, role file, playbook skill, authz row. The * {@code profile} answers which backend — model, CLI adapter, credentials, cost. They * vary independently: a {@code reviewer} may run on the very same profile as the {@code dev} * whose diff it reviews, and that case is what proves the two are not one axis. * - *

Only members that need a stable identity are declared here. Architects are, because - * a lead addresses the same pair of them across many tickets. A {@code dev} or {@code reviewer} - * is anonymous and fungible — spawned per task, torn down after — so it never appears in config. + *

An entry declares that the role may run there; it is not an identity. The key + * names the entry for operators and for error messages, nothing more. Which of a pool's + * profiles an unqualified spawn actually lands on is the placement policy's choice, and + * definition order is the {@code fixed} policy's answer. * - *

A slot is declared, not recognised: config names the slot, its role and its - * profile, and nothing else. Unlike a lead (which config pins by herdr {@code terminal_id} and - * is recognised at startup), a member 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. - * - * @param role the contract this slot runs under — {@code architect}, {@code dev} or - * {@code reviewer}; required, and validated against {@link MemberRole} * @param profile the name of the {@code profiles:} entry this slot runs on; required and - * validated against {@link #profiles()} (a stale or typo'd reference fails at - * startup rather than silently spawning the wrong backend later) + * validated against {@link #profiles()}, so a stale or typo'd reference fails at + * startup rather than silently spawning the wrong backend later */ @JsonIgnoreProperties(ignoreUnknown = true) - public record Member(String role, String profile) { + public record Slot(String profile) { } /** - * Discover leads by tab label instead of by pasted {@code terminal_id} (CB-531). + * Who the daemon may run, and under which role (CB-557). * - *

Why: a lead is not spawned, so its {@code terminal_id} exists only once a human has opened - * the tab and started the agent — which makes {@code leaders:} a three-step ritual (start it, - * ask it its id, edit config, restart) repeated per lead. Naming the tab is one step, done at - * the moment the operator is already there. The convention also survives what an id does not: - * close the tab and reopen it and the id changes, while the label is retyped as-is. + *

This one block replaced four top-level keys — {@code leaders:}, {@code members:}, + * {@code leadScan:} and {@code defaultProfile:}. The change is not only a move. Role used to be + * a field on a slot ({@code role: architect}); it is now the containing key, so the + * config states the role × profile matrix directly and a role can no longer be misspelled into + * something that parses. * - *

Deliberately opt-in ({@code null} ⇒ off). Turning it on widens who resolves as - * {@link dev.ltms.bridged.auth.Role#PRIMARY}, and a config that never asked for it must not - * acquire that by upgrading the daemon. + *

It also reverses an earlier rule. Devs and reviewers were kept out of config because they + * are anonymous and spawned per task. They belong here now because these are pools, + * not identities: a pool says which backends a role is allowed to run on, and staying anonymous + * is exactly compatible with that. The pool is also what replaced {@code defaultProfile:} — an + * unqualified spawn names a role, and the role's pool supplies the candidates. * - *

bridged never writes these labels — see {@link dev.ltms.bridged.herdr.LeadTabScanner} for - * why that one-way direction is what keeps the convention trustworthy. - * - * @param tabPrefix label prefix marking a lead's tab, matched case-insensitively; the - * remainder is the lead's name ({@code "lead: opus-5.0"} → {@code - * opus-5.0}). Default {@code "lead:"} - * @param intervalSeconds how long a scan is cached before herdr is asked again; also the worst - * case before a newly-labelled tab is recognised. Default 10 + * @param leaders panes that orchestrate rather than are orchestrated, keyed by lead name + * @param architects profiles the {@code architect} role may run on + * @param developers profiles the {@code dev} role may run on + * @param reviewers profiles the {@code reviewer} role may run on + * @param tabLabel template for a member tab's label; {@code {role}}, {@code {profile}}, + * {@code {model}} and {@code {n}} (a per role+profile counter) are + * substituted. Default {@link #DEFAULT_TAB_LABEL} */ @JsonIgnoreProperties(ignoreUnknown = true) - public record LeadScan(String tabPrefix, Integer intervalSeconds) { - public LeadScan { - tabPrefix = (tabPrefix == null || tabPrefix.isBlank()) ? "lead:" : tabPrefix.strip(); - intervalSeconds = (intervalSeconds == null || intervalSeconds <= 0) ? 10 : intervalSeconds; + public record Fleet(Map leaders, + Map architects, + Map developers, + Map reviewers, + String tabLabel) { + + /** + * Role first, so the tab bar reads as the fleet and so the label shares a namespace with a + * lead's {@code tabPrefix}. Because {@code {role}} comes from a closed enum, a generated + * member label can never begin with {@code "lead:"} — the clash that + * {@link #validateLeadTabPrefixes()} used to have to check for is unrepresentable here. + */ + public static final String DEFAULT_TAB_LABEL = "{role}: {profile} #{n}"; + + public Fleet { + leaders = unmodifiableOrEmpty(leaders); + architects = unmodifiableOrEmpty(architects); + developers = unmodifiableOrEmpty(developers); + reviewers = unmodifiableOrEmpty(reviewers); + tabLabel = (tabLabel == null || tabLabel.isBlank()) ? DEFAULT_TAB_LABEL : tabLabel; + } + + /** + * Deliberately not {@code Map.copyOf}: its iteration order is salted per JVM run, which + * would discard YAML definition order. The {@code fixed} placement policy answers with a + * pool's first entry and {@code weighted} tie-breaks on candidate order, so losing that + * order makes placement unpredictable between restarts. + */ + private static Map unmodifiableOrEmpty(Map m) { + return (m == null || m.isEmpty()) + ? Map.of() : Collections.unmodifiableMap(new LinkedHashMap<>(m)); + } + + /** The pool for {@code role}, in definition order; empty when the role has none. */ + public Map pool(MemberRole role) { + if (role == null) { + return Map.of(); + } + return switch (role) { + case ARCHITECT -> architects; + case DEV -> developers; + case REVIEWER -> reviewers; + }; + } + + /** + * The profile names {@code role} may run on, in definition order, without repeats. + * + *

These are the candidates an unqualified spawn chooses between — the per-role successor + * to the old global {@code defaultProfile}. + */ + public List profilesFor(MemberRole role) { + return pool(role).values().stream() + .filter(s -> s != null && s.profile() != null && !s.profile().isBlank()) + .map(Slot::profile) + .distinct() + .toList(); + } + + /** Every role that has at least one profile configured, in enum order. */ + public List rolesConfigured() { + return Arrays.stream(MemberRole.values()) + .filter(r -> !profilesFor(r).isEmpty()) + .toList(); } } @@ -528,8 +614,8 @@ public record BridgedConfig( */ public Map leaderTerminals() { Map byTerminal = new LinkedHashMap<>(); - if (leaders != null) { - leaders.forEach((name, leader) -> { + if (fleet != null) { + fleet.leaders().forEach((name, leader) -> { if (leader != null && leader.terminal() != null && !leader.terminal().isBlank()) { byTerminal.put(leader.terminal(), name); } @@ -596,17 +682,38 @@ public record BridgedConfig( } /** - * The profile a no-argument spawn uses: {@code defaultProfile} if set, else the sole/first - * configured profile, else {@code null}. + * The candidate profiles an unqualified spawn of {@code role} chooses between, in definition + * order (CB-557). * - *

Named {@code effective…} because the record component {@code defaultProfile()} returns the - * raw config value, which may be {@code null}. This is the resolved one. + *

This replaced the global {@code defaultProfile:}. A single default could not survive roles + * being first-class: "the profile a no-argument spawn uses" has no one answer once a reviewer + * and a dev may legitimately want different backends. Asking per role gives each one its own + * pool, and definition order is the {@code fixed} policy's answer within it. + * + *

Falls back to every configured profile when the role has no pool, so a config that + * declares {@code profiles:} but no {@code fleet:} still spawns rather than failing — the + * pre-CB-557 behaviour for a config that named no slots. + */ + public List candidateProfiles(MemberRole role) { + List pool = (fleet == null) ? List.of() : fleet.profilesFor(role); + return pool.isEmpty() ? List.copyOf(profiles.keySet()) : pool; + } + + /** + * The profile an unqualified spawn of {@code role} lands on under the {@code fixed} policy: the + * first of {@link #candidateProfiles(MemberRole)}, or {@code null} when nothing is configured. + */ + public String defaultProfileFor(MemberRole role) { + List candidates = candidateProfiles(role); + return candidates.isEmpty() ? null : candidates.getFirst(); + } + + /** + * The last-resort profile for a spawn that names no role at all — the {@code dev} pool's first + * entry, since an unqualified spawn is a unit of work rather than a review or a design. */ public String effectiveDefaultProfile() { - if (defaultProfile != null && !defaultProfile.isBlank()) { - return defaultProfile; - } - return profiles.isEmpty() ? null : profiles.keySet().iterator().next(); + return defaultProfileFor(MemberRole.DEV); } private static final Logger log = LoggerFactory.getLogger(BridgedConfig.class); @@ -618,9 +725,9 @@ public record BridgedConfig( * {@link #warnUnknownTopLevelKeys}. Keep in step with the record components. */ private static final Set KNOWN_TOP_LEVEL_KEYS = Set.of( - "bind", "herdrSocket", "profiles", "defaultProfile", "guard", "worktreeRoot", - "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "leaders", - "members", "leadScan", "leadHeartbeat", "placement", "auth"); + "bind", "herdrSocket", "profiles", "guard", "worktreeRoot", + "lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet", + "leadHeartbeat", "placement", "auth"); /** Load and validate config from {@code path}. */ public static BridgedConfig load(Path path) { @@ -636,38 +743,46 @@ public record BridgedConfig( } } + /** The {@code fleet:} child blocks whose direct children are slot names. */ + private static final Set FLEET_POOL_KEYS = + Set.of("leaders", "architects", "developers", "reviewers"); + /** - * Reject an {@code members:} registry whose slot names repeat (CB-548). + * Reject a {@code fleet:} role pool whose slot names repeat (CB-548, re-homed by CB-557). * - *

The registry is a {@code Map} keyed by slot name, so by the time it is read duplicate keys + *

Each pool 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 - * top-level {@code members:} block is considered, and only its direct child keys (the - * slot names) — a nested field elsewhere, even one also named {@code members:}, is ignored, so - * parsing of the rest of the config is unaffected. + * default, so duplicates are caught here, at parse time, before the map is built. * - * @throws IllegalStateException when two {@code members:} entries share a slot name, naming it + *

Only the four pools directly under the top-level {@code fleet:} are considered, + * and only their direct child keys (the slot names). A nested field elsewhere, even one also + * named {@code developers:}, is ignored, so parsing of the rest of the config is unaffected. + * + *

Names repeat freely across pools and that is deliberate: {@code sonnet} appearing + * in both {@code developers:} and {@code reviewers:} is the role × profile matrix doing its job, + * not a mistake. Only a repeat within one pool is an error. + * + * @throws IllegalStateException when one pool has two entries sharing a slot name, naming both */ static void rejectDuplicateMemberSlots(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 } - // Scan the TOP-LEVEL mapping only. Every other field's value (however deep, including - // any nested field also literally named "members") is consumed whole by skipValue, so - // the loop below can only ever see the top-level field names — a nested `members:` can - // neither suppress the real block nor be misread as one. + // Scan the TOP-LEVEL mapping only. Every other field's value (however deep, including a + // nested field also literally named "fleet") is consumed whole by skipValue, so the loop + // below can only ever see top-level field names. JsonToken t; while ((t = p.nextToken()) != null && t != JsonToken.END_OBJECT) { if (t == JsonToken.FIELD_NAME) { String name = p.getCurrentName(); JsonToken value = p.nextToken(); - if ("members".equals(name)) { + if ("fleet".equals(name)) { if (value == JsonToken.START_OBJECT) { - rejectDuplicateChildSlotKeys(p); + rejectDuplicateSlotsInPools(p); } - return; // the single top-level members block is handled; nothing more to check + return; // the single top-level fleet block is handled; nothing more to check } skipValue(p, value); } @@ -678,24 +793,44 @@ public record BridgedConfig( } /** - * Reject a duplicated direct child key of the (already-positioned) {@code members:} - * 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 members:}) is never seen - * here and cannot masquerade as a duplicated slot name. - * - * @throws IllegalStateException when two {@code members:} entries share a slot name, naming it + * Walk the (already-positioned) {@code fleet:} mapping and check each role pool it contains. + * Any other {@code fleet:} child — {@code tabLabel}, say — is consumed whole and ignored. */ - private static void rejectDuplicateChildSlotKeys(JsonParser p) throws IOException { + private static void rejectDuplicateSlotsInPools(JsonParser p) throws IOException { + JsonToken t; + while ((t = p.nextToken()) != null && t != JsonToken.END_OBJECT) { + if (t == JsonToken.FIELD_NAME) { + String pool = p.getCurrentName(); + JsonToken value = p.nextToken(); + if (FLEET_POOL_KEYS.contains(pool) && value == JsonToken.START_OBJECT) { + rejectDuplicateChildSlotKeys(p, pool); + } else { + skipValue(p, value); + } + } + } + } + + /** + * Reject a duplicated direct child key of an already-positioned role pool — i.e. a + * duplicated slot name within that one pool. + * + *

Each slot's value is consumed whole by {@link #skipValue}, so a duplicated field + * inside a slot (two {@code profile:} keys, say) is never seen here and cannot + * masquerade as a duplicated slot name. + * + * @param pool the pool's key, named in the error so the operator knows which one to look at + * @throws IllegalStateException when two entries in {@code pool} share a slot name + */ + private static void rejectDuplicateChildSlotKeys(JsonParser p, String pool) 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 member slot name '" - + p.getCurrentName() + "' — slot names must be unique; a later entry would " - + "silently overwrite the earlier one"); + throw new IllegalStateException("refusing to start: duplicate slot name '" + + p.getCurrentName() + "' in fleet." + pool + " — names must be unique " + + "within a pool; a later entry would silently overwrite the earlier one"); } skipValue(p, p.nextToken()); // the slot's entire value } @@ -774,12 +909,24 @@ public record BridgedConfig( *

We are in active development, so the old spellings are not accepted as aliases. Accepting * both would leave two names for one thing in every config and doc, which is the cost the * rename was meant to remove. + * + *

Values are the advice shown to the operator, not bare key names: several of these did not + * move to one key. {@code defaultProfile:} has no successor at all — it became per-role — and a + * message naming a single replacement key would send the reader somewhere that does not exist. */ private static final Map RENAMED_TOP_LEVEL_KEYS = Map.of( - "workers", "profiles", - "worker", "profiles", - "defaultWorker", "defaultProfile", - "architects", "members"); + "workers", "'profiles'", + "worker", "'profiles'", + "defaultWorker", "a role pool under 'fleet:' — an unqualified spawn now names a role," + + " and that role's pool supplies the candidate profiles", + "defaultProfile", "a role pool under 'fleet:' — an unqualified spawn now names a role," + + " and that role's pool supplies the candidate profiles", + "architects", "'fleet.architects'", + "members", "a role pool under 'fleet:' — 'fleet.architects', 'fleet.developers' or" + + " 'fleet.reviewers'; the role is the containing key, not a 'role:' field", + "leaders", "'fleet.leaders'", + "leadScan", "'fleet.leaders..tabPrefix' and '.scanIntervalSeconds' — lead" + + " discovery is now configured on the lead it discovers"); /** * Reject a config that still uses a pre-rename top-level key, naming its replacement. @@ -801,12 +948,13 @@ public record BridgedConfig( .map(String::valueOf) .filter(RENAMED_TOP_LEVEL_KEYS::containsKey) .sorted() - .map(k -> "'" + k + "' is now '" + RENAMED_TOP_LEVEL_KEYS.get(k) + "'") + .map(k -> "'" + k + "' is now " + RENAMED_TOP_LEVEL_KEYS.get(k)) .toList(); if (!bad.isEmpty()) { throw new IllegalStateException("refusing to start: this config uses renamed top-level " + "keys — " + String.join("; ", bad) - + ". A profile says which backend to run; a member says which role runs on it."); + + ". A profile says which backend to run; a fleet role pool says which role may" + + " run on it."); } } @@ -838,14 +986,15 @@ public record BridgedConfig( String placementOrDefault = (placement != null && !placement.isBlank()) ? placement : "fixed"; // broker is left as-is: null (or an empty/blank uri) keeps the in-memory soft-state inbox. // primary is left as-is: null defaults to connection-derived identity. - // 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. - // leadHeartbeat is left as-is for the same reason (CB-551): null is "off", and LeadHeartbeat's - // own compact constructor defaults the fields of a block that IS present. - // members is left as-is: null is "none configured", and Member's fields have no - // defaults to fill. Defaulting it here would change nothing, so leave the call natural. - return new BridgedConfig(b, herdrSocket, profiles, defaultProfile, g, worktreeRoot, l, timeout, pollMs, broker, primary, leaders, members, leadScan, leadHeartbeat, placementOrDefault, a); + // fleet IS defaulted, unlike the leadScan: block it replaced, because an empty Fleet is not + // the same as an enabled one: every pool is empty, so no lead is scanned for or created and + // no role has a pool. Constructing it saves every reader a null check for no behaviour change. + Fleet f = (fleet != null) ? fleet : new Fleet(null, null, null, null, null); + // leadHeartbeat is left as-is (CB-551): null is "off", and LeadHeartbeat'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, profiles, g, worktreeRoot, l, timeout, pollMs, + broker, primary, f, leadHeartbeat, placementOrDefault, a); } /** @@ -876,40 +1025,65 @@ public record BridgedConfig( * Reject a lead-scan convention that a worker tab would also satisfy (CB-531). * *

The scan reads a tab label and concludes "a lead lives here". bridged also writes - * tab labels — every worker gets {@code tabLabel} rendered into its tab. Choose a - * {@code leadScan.tabPrefix} that a worker template matches and the daemon starts labelling its - * own workers as leads, promoting the entire fleet to {@link dev.ltms.bridged.auth.Role#PRIMARY} - * with no message and no diff. The worker-space exclusion in - * {@link dev.ltms.bridged.herdr.LeadTabScanner} already blocks the realistic path, but defence - * that depends on one workspace label holding is not defence enough for a privilege boundary. + * tab labels — every member gets one rendered into its tab. Choose a lead {@code tabPrefix} that + * a member template matches and the daemon starts labelling its own members as leads, promoting + * the entire fleet to {@link dev.ltms.bridged.auth.Role#PRIMARY} with no message and no diff. + * The member-space exclusion in {@link dev.ltms.bridged.herdr.LeadTabScanner} already blocks the + * realistic path, but defence that depends on one workspace label holding is not defence enough + * for a privilege boundary. + * + *

CB-557 shrank this check rather than removing it. The default template is + * {@code "{role}: {profile} #{n}"} and {@code {role}} comes from a closed enum, so a + * generated label can no longer collide by construction. What remains checkable is what + * an operator still writes by hand: the {@code fleet.tabLabel} template and any per-profile + * {@code tabLabel} override. * *

Fatal rather than a warning, unlike {@link #warnUnknownTopLevelKeys}: an unknown key means * a feature does nothing, while this means a feature does the opposite of what it says. * - * @throws IllegalStateException when any worker profile's {@code tabLabel} starts with the - * configured lead prefix + * @throws IllegalStateException when the fleet template or any profile's {@code tabLabel} + * override starts with a configured lead prefix */ - public void validateLeadScan() { - if (leadScan == null) { + public void validateLeadTabPrefixes() { + if (fleet == null || fleet.leaders().isEmpty()) { return; } - String prefix = leadScan.tabPrefix(); - List clashing = profiles().entrySet().stream() - .filter(e -> e.getValue().tabLabel() != null - && e.getValue().tabLabel().strip() - .regionMatches(true, 0, prefix, 0, prefix.length())) - .map(Map.Entry::getKey) - .sorted() - .toList(); - if (clashing.isEmpty()) { + List bad = new ArrayList<>(); + fleet.leaders().forEach((leadName, leader) -> { + if (leader == null) { + return; + } + String prefix = leader.tabPrefix(); + // The fleet-wide template is checked once per prefix: it labels every member that has no + // override, so one bad template promotes the entire fleet, not one profile. + if (startsWithIgnoreCase(fleet.tabLabel(), prefix)) { + bad.add("fleet.tabLabel=\"" + fleet.tabLabel() + "\" starts with the tabPrefix of " + + "lead '" + leadName + "' (\"" + prefix + "\")"); + } + profiles().entrySet().stream() + .filter(e -> startsWithIgnoreCase(e.getValue().tabLabel(), prefix)) + .map(Map.Entry::getKey) + .sorted() + .forEach(p -> bad.add("profile '" + p + "' overrides tabLabel with \"" + + profiles().get(p).tabLabel() + "\", which starts with the tabPrefix of " + + "lead '" + leadName + "' (\"" + prefix + "\")")); + }); + if (bad.isEmpty()) { return; } - throw new IllegalStateException( - "refusing to start: leadScan.tabPrefix=\"" + prefix + "\" also matches the tabLabel " - + "of worker profile(s) " + clashing + ". Every worker spawned under them " - + "would be read back as a lead and granted spawn/stop/send on the whole " - + "fleet. Change one of the two so worker tabs and lead tabs cannot be " - + "confused."); + throw new IllegalStateException("refusing to start: " + String.join("; ", bad) + + ". Every member labelled that way would be read back as a lead and granted " + + "spawn/stop/send on the whole fleet. Change one of the two so member tabs and " + + "lead tabs cannot be confused."); + } + + /** Case-insensitive prefix test that tolerates a null/blank label. */ + private static boolean startsWithIgnoreCase(String label, String prefix) { + if (label == null || prefix == null || prefix.isBlank()) { + return false; + } + String stripped = label.strip(); + return stripped.regionMatches(true, 0, prefix, 0, prefix.length()); } /** @@ -959,39 +1133,52 @@ public record BridgedConfig( * when something later tries to use it. Validating at startup names the mistake then, rather * than leaving it to be discovered months later by a spawn that quietly has no backend. * - *

The {@code role} is checked the same way and for the same reason: a typo'd role would - * otherwise pick no charter at all, and the member would run with no contract. + *

The role itself needs no check any more (CB-557): it is the containing key, so an + * unrecognised pool name is simply not a pool and cannot become a member with no contract. That + * is the main thing the pool shape bought over the old {@code role:} field. * - *

Slot-name uniqueness needs no check here: the registry is a {@code Map} keyed by name, so - * duplicates are unrepresentable by construction once loaded — and {@link #load(Path)} already - * rejects a duplicated slot name at parse time, before the map collapses. + *

Slot-name uniqueness needs no check here either: each pool is a {@code Map} keyed by name, + * so 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 member slot is missing or names an unknown role or - * profile, naming the slot and the offending reference + * @throws IllegalStateException when a slot names no profile or an unknown one, or when a lead + * can be neither found nor created, naming the offending entry */ public void validateMembers() { - if (members == null) { + if (fleet == null) { return; } - List bad = new java.util.ArrayList<>(); - members.forEach((name, m) -> { - if (m == null || m.profile() == null || m.profile().isBlank()) { - bad.add("member slot '" + name + "' has no profile: — give it the name of a " - + "profiles: entry (the backend it runs on)."); - } else if (!profiles.containsKey(m.profile())) { - bad.add("member slot '" + name + "' references profile '" + m.profile() + List bad = new ArrayList<>(); + for (MemberRole role : MemberRole.values()) { + String pool = "fleet." + role.configKey(); + fleet.pool(role).forEach((name, slot) -> { + if (slot == null || slot.profile() == null || slot.profile().isBlank()) { + bad.add(pool + "." + name + " has no profile: — give it the name of a profiles: " + + "entry (the backend this role runs on)."); + } else if (!profiles.containsKey(slot.profile())) { + bad.add(pool + "." + name + " references profile '" + slot.profile() + + "', which is not a configured profiles: entry (have: " + + profiles.keySet() + ")."); + } + }); + } + fleet.leaders().forEach((name, leader) -> { + if (leader == null) { + return; + } + // A lead's profile is optional: without one the lead is recognised but never created, + // which is the pre-CB-557 behaviour and still a legitimate choice. A profile that IS + // named must resolve, or the shortfall launch fails at the worst possible moment. + if (leader.profile() != null && !leader.profile().isBlank() + && !profiles.containsKey(leader.profile())) { + bad.add("fleet.leaders." + name + " references profile '" + leader.profile() + "', which is not a configured profiles: entry (have: " + profiles.keySet() + ")."); } - if (m == null || m.role() == null || m.role().isBlank()) { - bad.add("member slot '" + name + "' has no role: — give it one of architect, dev, " - + "reviewer."); - } else { - try { - MemberRole.parse(m.role()); - } catch (IllegalArgumentException e) { - bad.add("member slot '" + name + "': " + e.getMessage() + "."); - } + 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 (!bad.isEmpty()) { diff --git a/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java index b104136..1d66286 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/ClaudeCodeLauncher.java @@ -79,9 +79,24 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { Map profiles, String defaultProfile, Function env, long spawnReadyTimeoutMs, long spawnReadyPollMs) { + this(agents, spaces, guard, profiles, defaultProfile, env, + spawnReadyTimeoutMs, spawnReadyPollMs, null); + } + + /** + * Production constructor carrying the fleet-wide tab-label template (CB-557). The template comes + * from {@code fleet.tabLabel}, which a profile cannot know because it names the member's + * role; a profile may still override it with its own {@code tabLabel}. + */ + public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, long spawnReadyPollMs, + String tabLabelTemplate) { this(agents, spaces, guard, profiles, defaultProfile, env, spawnReadyTimeoutMs, - System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs)); + System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), + tabLabelTemplate); } /** @@ -106,8 +121,24 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { Function env, long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper) { + this(agents, spaces, guard, profiles, defaultProfile, env, + spawnReadyTimeoutMs, nowMillis, sleeper, null); + } + + /** + * Full testability constructor, plus the fleet-wide tab-label template (CB-557). + * + * @param tabLabelTemplate {@code fleet.tabLabel}; {@code null}/blank ⇒ + * {@link BridgedConfig.Fleet#DEFAULT_TAB_LABEL} + */ + public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + String tabLabelTemplate) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, - spawnReadyTimeoutMs, nowMillis, sleeper); + spawnReadyTimeoutMs, nowMillis, sleeper, tabLabelTemplate); this.guard = guard; } diff --git a/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java index 612b804..f6dac69 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/CompositePeerLauncher.java @@ -174,9 +174,10 @@ public final class CompositePeerLauncher implements PeerLauncher { } // CB-547a: route the chosen profile but keep the caller's session identity — dropping it - // here would silently sever the resume handle on every policy-routed spawn. + // here would silently sever the resume handle on every policy-routed spawn. CB-557: the + // role rides along for the same reason, or a routed spawn would be labelled as a dev. SpawnRequest routedReq = new SpawnRequest(chosen.profile(), req.requestedCwd(), req.callerCwd(), - req.sessionName(), req.resumeSessionId()); + req.sessionName(), req.resumeSessionId(), req.role()); try { PeerHandle handle = d.spawn(routedReq); spawnedBy.put(handle.id(), d); diff --git a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java index 0e79e35..2ce7d9b 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/HerdrPeerLauncher.java @@ -7,6 +7,7 @@ import dev.ltms.bridged.herdr.HerdrException; import dev.ltms.bridged.herdr.Tab; import dev.ltms.bridged.herdr.Workspace; import dev.ltms.bridged.herdr.WorkspaceControl; +import dev.ltms.bridged.peer.MemberRole; import dev.ltms.bridged.peer.PeerHandle; import dev.ltms.bridged.peer.PeerLauncher; import dev.ltms.bridged.peer.PeerUnreachableException; @@ -75,7 +76,27 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { /** Host env lookup (injectable for tests); adapters read it in {@link #buildLaunch}. */ protected final Function env; - private final AtomicLong nameSeq = new AtomicLong(); // per-peer counter (also the tab #) + private final AtomicLong nameSeq = new AtomicLong(); // per-peer counter (herdr agent names only) + + /** + * The {@code fleet.tabLabel} template; {@code null} ⇒ {@link BridgedConfig.Fleet#DEFAULT_TAB_LABEL}. + * A profile's own {@code tabLabel} still overrides it. + */ + private final String tabLabelTemplate; + + /** + * Tab numbers, counted per {@code role/profile} pair (CB-557). + * + *

Deliberately not {@link #nameSeq}. That counter is shared by every profile this launcher + * serves, because its job is to make herdr agent names unique. Reusing it for the tab + * label made the numbers global, so sibling tabs read {@code #4}, {@code #9}, {@code #17} — gaps + * that look like a member died. Counting per role+profile makes {@code dev: sonnet #2} mean the + * second sonnet dev, which is what a reader assumes it means. + * + *

Resets when the daemon restarts, and that is fine: the label is a human-facing hint, not an + * identity. Identity is {@link PeerHandle#id()}. + */ + private final ConcurrentMap labelSeq = new ConcurrentHashMap<>(); private final long spawnReadyTimeoutMs; // 0 = disable gate (legacy non-blocking spawn) private final LongSupplier nowMillis; // monotonic clock (injectable for tests) @@ -110,6 +131,25 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { Function env, long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper) { + this(namePrefix, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, + nowMillis, sleeper, null); + } + + /** + * As above, plus the {@code fleet.tabLabel} template (CB-557). + * + * @param tabLabelTemplate fleet-wide tab-label template; {@code null}/blank ⇒ + * {@link BridgedConfig.Fleet#DEFAULT_TAB_LABEL}. A separate constructor + * rather than a new parameter on the one above, so every existing call + * site keeps the default without an edit. + */ + protected HerdrPeerLauncher(String namePrefix, AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + String tabLabelTemplate) { + this.tabLabelTemplate = tabLabelTemplate; this.namePrefix = namePrefix; this.agents = agents; this.spaces = spaces; @@ -240,6 +280,13 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { return spawnInternal(profileName, requestedCwd, callerCwd, null, null).agent(); } + /** Pre-CB-557 shape: no explicit role, so the tab is labelled as a {@code dev}. */ + protected Spawned spawnInternal(String profileName, String requestedCwd, String callerCwd, + String sessionName, String resumeSessionId) { + return spawnInternal(profileName, requestedCwd, callerCwd, sessionName, resumeSessionId, + MemberRole.DEV); + } + /** * Spawn a peer with session identity (CB-547a). {@code sessionName} and {@code resumeSessionId} * are threaded from the {@link SpawnRequest} into {@link #buildLaunch(BridgedConfig.Profile, @@ -247,16 +294,27 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * so the caller can put it on the {@link PeerHandle}. */ protected Spawned spawnInternal(String profileName, String requestedCwd, String callerCwd, - String sessionName, String resumeSessionId) { + String sessionName, String resumeSessionId, MemberRole role) { BridgedConfig.Profile cfg = requireProfile(profileName); Launch launch = buildLaunch(cfg, sessionName, resumeSessionId); String cwd = resolveCwd(requestedCwd, cfg, callerCwd); Agent agent = cfg.tabPlacement() - ? spawnInTab(cfg, launch.env(), launch.argv(), cwd) + ? spawnInTab(cfg, launch.env(), launch.argv(), cwd, role) : spawnAsPane(cfg, launch.env(), launch.argv(), cwd); return new Spawned(agent, launch.agentSessionId()); } + /** + * The next tab number for {@code role} on {@code profile}, starting at 1. + * + *

Starts at 1 rather than 0 because the number is read by a person: {@code "dev: sonnet #1"} + * is the first one, and {@code #0} invites the question of where {@code #1} went. + */ + private long nextLabelSeq(MemberRole role, String profile) { + String key = (role == null ? "" : role.wireName()) + "/" + profile; + return labelSeq.computeIfAbsent(key, _ -> new AtomicLong()).incrementAndGet(); + } + /** * {@inheritDoc} * @@ -272,7 +330,7 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { @Override public PeerHandle spawn(SpawnRequest req) { Spawned spawned = spawnInternal(req.profileName(), req.requestedCwd(), req.callerCwd(), - req.sessionName(), req.resumeSessionId()); + req.sessionName(), req.resumeSessionId(), req.role()); Agent agent = spawned.agent(); String paneId = agent.paneId(); if (spawnReadyTimeoutMs > 0) { @@ -317,7 +375,7 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { /** Dedicated worker space → own tab (carrying cwd+env) → start the peer into the seed pane. */ private Agent spawnInTab(BridgedConfig.Profile cfg, Map workerEnv, - List argv, String cwd) { + List argv, String cwd, MemberRole role) { Workspace space = spaces.ensureWorkspace(cfg.workspace()); Tab.Created tab = spaces.createTab(space.workspaceId(), cwd, workerEnv); log.info("spawning {} profile={} space={} tab={} cwd={}", @@ -348,7 +406,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { // Labelling is cosmetic: it must not fail the spawn or orphan the running peer — on error // we log and still return it so the caller gets its paneId and can tear it down. tidy("label tab " + tab.tab().tabId(), - () -> spaces.renameTab(tab.tab().tabId(), cfg.renderTabLabel(started.seq()))); + () -> spaces.renameTab(tab.tab().tabId(), + cfg.renderTabLabel(tabLabelTemplate, role, nextLabelSeq(role, cfg.profile())))); log.info("{} started pane={} tab={} terminal={}", namePrefix, started.agent().paneId(), started.agent().tabId(), started.agent().terminalId()); return started.agent(); diff --git a/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java index 812de5d..cef1d9f 100644 --- a/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/member/OpenCodeLauncher.java @@ -112,9 +112,23 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { Map profiles, String defaultProfile, Function env, long spawnReadyTimeoutMs, long spawnReadyPollMs) { + this(agents, spaces, profiles, defaultProfile, env, + spawnReadyTimeoutMs, spawnReadyPollMs, null); + } + + /** + * Production constructor carrying the fleet-wide tab-label template (CB-557). The template comes + * from {@code fleet.tabLabel}, which a profile cannot know because it names the member's + * role; a profile may still override it with its own {@code tabLabel}. + */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, long spawnReadyPollMs, + String tabLabelTemplate) { this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs), - defaultConfigRoot(), defaultDiscoveryRoot()); + defaultConfigRoot(), defaultDiscoveryRoot(), tabLabelTemplate); } /** @@ -142,8 +156,25 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { long spawnReadyTimeoutMs, LongSupplier nowMillis, Runnable sleeper, Path configRoot, Path discoveryRoot) { + this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, + nowMillis, sleeper, configRoot, discoveryRoot, null); + } + + /** + * Full testability constructor, plus the fleet-wide tab-label template (CB-557). + * + * @param tabLabelTemplate {@code fleet.tabLabel}; {@code null}/blank ⇒ + * {@link BridgedConfig.Fleet#DEFAULT_TAB_LABEL} + */ + public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces, + Map profiles, String defaultProfile, + Function env, + long spawnReadyTimeoutMs, + LongSupplier nowMillis, Runnable sleeper, + Path configRoot, Path discoveryRoot, + String tabLabelTemplate) { super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env, - spawnReadyTimeoutMs, nowMillis, sleeper); + spawnReadyTimeoutMs, nowMillis, sleeper, tabLabelTemplate); this.configRoot = configRoot; this.discovery = new OpenCodeSessionDiscovery(discoveryRoot); } diff --git a/bridged/src/main/java/dev/ltms/bridged/peer/MemberRole.java b/bridged/src/main/java/dev/ltms/bridged/peer/MemberRole.java index 7185c3f..ebf902f 100644 --- a/bridged/src/main/java/dev/ltms/bridged/peer/MemberRole.java +++ b/bridged/src/main/java/dev/ltms/bridged/peer/MemberRole.java @@ -57,6 +57,40 @@ public enum MemberRole { return name().toLowerCase(Locale.ROOT); } + /** + * The {@code fleet:} block that holds this role's pool — {@code architects}, + * {@code developers}, {@code reviewers}. + * + *

Plural, and not always the wire name: the pool of things a {@code dev} may run on reads + * naturally as {@code developers:}. The wire name stays the singular {@code dev}, because that + * is what a tab label and a roster row say. + */ + public String configKey() { + return switch (this) { + case ARCHITECT -> "architects"; + case DEV -> "developers"; + case REVIEWER -> "reviewers"; + }; + } + + /** + * The role owning the {@code fleet:} pool named {@code key}, or {@code null} when the key is not + * a role pool ({@code leaders}, {@code tabLabel}, …). Null rather than a throw: callers use this + * to sort a fleet block's children, where a non-pool key is normal rather than an error. + */ + public static MemberRole fromConfigKey(String key) { + if (key == null) { + return null; + } + String k = key.trim().toLowerCase(Locale.ROOT); + for (MemberRole r : values()) { + if (r.configKey().equals(k)) { + return r; + } + } + return null; + } + /** * Parse a config/wire spelling, case-insensitively. * diff --git a/bridged/src/main/java/dev/ltms/bridged/peer/SpawnRequest.java b/bridged/src/main/java/dev/ltms/bridged/peer/SpawnRequest.java index ce63676..4cb60b8 100644 --- a/bridged/src/main/java/dev/ltms/bridged/peer/SpawnRequest.java +++ b/bridged/src/main/java/dev/ltms/bridged/peer/SpawnRequest.java @@ -1,24 +1,32 @@ package dev.ltms.bridged.peer; /** - * Parameters for a {@link PeerLauncher#spawn(SpawnRequest)} call — the peer-neutral - * aggregation of what the core knows at delegation time: which profile to use, the caller's - * requested working directory, and the caller's own cwd (to inherit when no other cwd is set). + * What a caller asks for when spawning a peer. * - *

A null or blank {@code profileName} means "use the launcher's default profile." - * A null or blank {@code requestedCwd} means "inherit from config or caller." - * A null {@code callerCwd} means "the request came from the daemon itself (not a primary)." - * - *

{@code sessionName} and {@code resumeSessionId} carry the session's durable identity (CB-547a): - * the bridge's LOGICAL name for the session (stable across restarts, meaningful to an operator) - * and the peer's OWN prior session id to resume, respectively. Both are opted in — either - * may be null/blank, in which case the launcher derives a display name and mints a fresh session. + * @param profileName the {@code profiles:} entry to spawn on; {@code null}/blank ⇒ the caller + * did not choose, and the role's pool supplies the candidates + * @param requestedCwd working directory asked for by the caller ({@code null} ⇒ unset) + * @param callerCwd the caller's own working directory, used when nothing else pins one + * @param sessionName agent session name (CB-547a); {@code null} ⇒ the launcher mints one + * @param resumeSessionId prior agent session to resume; {@code null} ⇒ a fresh session + * @param role the contract this member runs under (CB-557). Picks the tab label and, + * with the profile, the counter its tab number comes from. {@code null} is + * read as {@link MemberRole#DEV} — an unqualified spawn is a unit of work */ public record SpawnRequest(String profileName, String requestedCwd, String callerCwd, - String sessionName, String resumeSessionId) { + String sessionName, String resumeSessionId, MemberRole role) { + + public SpawnRequest { + role = (role == null) ? MemberRole.DEV : role; + } - /** Back-compat: a spawn with no session identity (fresh session, launcher-derived name). */ public SpawnRequest(String profileName, String requestedCwd, String callerCwd) { - this(profileName, requestedCwd, callerCwd, null, null); + this(profileName, requestedCwd, callerCwd, null, null, null); + } + + /** Pre-CB-557 shape: session identity without an explicit role (defaults to {@code dev}). */ + public SpawnRequest(String profileName, String requestedCwd, String callerCwd, + String sessionName, String resumeSessionId) { + this(profileName, requestedCwd, callerCwd, sessionName, resumeSessionId, null); } } diff --git a/bridged/src/test/java/dev/ltms/bridged/auth/MemberRegistryTest.java b/bridged/src/test/java/dev/ltms/bridged/auth/MemberRegistryTest.java index 53ea497..d3b8487 100644 --- a/bridged/src/test/java/dev/ltms/bridged/auth/MemberRegistryTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/auth/MemberRegistryTest.java @@ -1,12 +1,14 @@ package dev.ltms.bridged.auth; import dev.ltms.bridged.config.BridgedConfig; +import dev.ltms.bridged.peer.MemberRole; import org.junit.jupiter.api.Test; import java.util.ArrayList; -import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -22,23 +24,52 @@ import static org.junit.jupiter.api.Assertions.*; */ class MemberRegistryTest { - private static final Map SLOTS = Map.of( - "lead-designer", new BridgedConfig.Member("architect", "sonnet"), - "reviewer", new BridgedConfig.Member("architect", "gx10")); + /** + * Slot keys are qualified by role (CB-557): a bare name is unique only within its pool, so + * {@code sonnet} can be both a developer and a reviewer, while a terminal binds to exactly one. + */ + private static final String DESIGNER = "architect:lead-designer"; + private static final String REVIEWER = "architect:code-reviewer"; - private final MemberRegistry registry = new MemberRegistry(SLOTS); + private static BridgedConfig.Fleet fleetWith(Map architects) { + return new BridgedConfig.Fleet(Map.of(), architects, Map.of(), Map.of(), null); + } + + private static Map architects() { + Map pool = new LinkedHashMap<>(); + pool.put("lead-designer", new BridgedConfig.Slot("sonnet")); + pool.put("code-reviewer", new BridgedConfig.Slot("gx10")); + return pool; + } + + private final MemberRegistry registry = new MemberRegistry(fleetWith(architects())); @Test - void exposesTheConfiguredSlots() { - assertEquals(SLOTS.keySet(), registry.slots().keySet()); - assertTrue(registry.isSlot("reviewer")); + void exposesTheConfiguredSlotsQualifiedByRole() { + assertEquals(Set.of(DESIGNER, REVIEWER), registry.slots().keySet()); + assertTrue(registry.isSlot(REVIEWER)); assertFalse(registry.isSlot("nope")); + assertFalse(registry.isSlot("lead-designer"), + "the bare name is not the key — it is unique only inside its pool"); + } + + /** The case the pool shape exists for: one profile serving two roles is not a duplicate. */ + @Test + void oneProfileMayServeTwoRolesUnderTheSameSlotName() { + Map devs = Map.of("sonnet", new BridgedConfig.Slot("sonnet")); + Map revs = Map.of("sonnet", new BridgedConfig.Slot("sonnet")); + MemberRegistry r = new MemberRegistry( + new BridgedConfig.Fleet(Map.of(), Map.of(), devs, revs, null)); + + assertEquals(Set.of("dev:sonnet", "reviewer:sonnet"), r.slots().keySet()); + assertEquals(MemberRole.DEV, r.roleForSlot("dev:sonnet")); + assertEquals(MemberRole.REVIEWER, r.roleForSlot("reviewer:sonnet")); } @Test void theSpawnLifecycleReadsTheProfileBackFromASlot() { - assertEquals("sonnet", registry.profileForSlot("lead-designer")); - assertEquals("gx10", registry.profileForSlot("reviewer")); + assertEquals("sonnet", registry.profileForSlot(DESIGNER)); + assertEquals("gx10", registry.profileForSlot(REVIEWER)); assertNull(registry.profileForSlot("unknown"), "an unknown slot has no profile"); } @@ -54,9 +85,9 @@ class MemberRegistryTest { @Test 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()); + assertTrue(registry.bind(DESIGNER, "term_design")); + assertEquals(DESIGNER, registry.slotForTerminal("term_design")); + assertEquals(Map.of("term_design", DESIGNER), registry.snapshot()); } @Test @@ -68,27 +99,27 @@ class MemberRegistryTest { @Test void bindRefusesATerminalInTwoSlots() { - assertTrue(registry.bind("lead-designer", "term_design")); - assertFalse(registry.bind("reviewer", "term_design"), + assertTrue(registry.bind(DESIGNER, "term_design")); + assertFalse(registry.bind(REVIEWER, "term_design"), "a terminal may occupy at most one slot"); - assertEquals("lead-designer", registry.slotForTerminal("term_design"), + assertEquals(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"), + assertTrue(registry.bind(DESIGNER, "term_design")); + assertFalse(registry.bind(DESIGNER, "term_other"), "a slot may host at most one terminal"); - assertEquals("lead-designer", registry.slotForTerminal("term_design"), + assertEquals(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"), + assertTrue(registry.bind(DESIGNER, "term_design")); + assertTrue(registry.bind(DESIGNER, "term_design"), "the same terminal → slot is harmless to repeat"); assertEquals(1, registry.snapshot().size()); } @@ -97,8 +128,8 @@ class MemberRegistryTest { @Test void unbindRemovesTheExactBinding() { - assertTrue(registry.bind("lead-designer", "term_design")); - assertTrue(registry.unbind("lead-designer", "term_design")); + assertTrue(registry.bind(DESIGNER, "term_design")); + assertTrue(registry.unbind(DESIGNER, "term_design")); assertNull(registry.slotForTerminal("term_design")); assertTrue(registry.snapshot().isEmpty()); } @@ -106,32 +137,32 @@ class MemberRegistryTest { @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")); + assertTrue(registry.bind(DESIGNER, "term_design")); + registry.unbind(DESIGNER, "term_design"); + assertTrue(registry.bind(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"), + assertFalse(registry.unbind(DESIGNER, "term_design")); + assertEquals(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")); + assertTrue(registry.bind(DESIGNER, "term_design")); + registry.unbind(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"), + assertFalse(registry.unbind(DESIGNER, "term_design"), "the old slot must not unbind a terminal that moved elsewhere"); - assertEquals("reviewer", registry.slotForTerminal("term_design")); + assertEquals(REVIEWER, registry.slotForTerminal("term_design")); } @Test void unbindOfNothingIsAFalseNoOp() { - assertFalse(registry.unbind("lead-designer", "term_design"), + assertFalse(registry.unbind(DESIGNER, "term_design"), "nothing was bound, so nothing is removed"); } @@ -139,14 +170,14 @@ class MemberRegistryTest { @Test void theSnapshotIsAnImmutableCopyNotAliveState() { - assertTrue(registry.bind("lead-designer", "term_design")); + assertTrue(registry.bind(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")); + assertTrue(registry.bind(REVIEWER, "term_review")); assertFalse(snap.containsKey("term_review"), "a snapshot is a point-in-time copy, not a live view"); } @@ -164,7 +195,7 @@ class MemberRegistryTest { final String term = "term_" + i; // every thread races for the SAME slot results.add(pool.submit(() -> { go.await(); - return registry.bind("lead-designer", term); + return registry.bind(DESIGNER, term); })); } go.countDown(); @@ -191,7 +222,7 @@ class MemberRegistryTest { 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 + final String slot = (i % 2 == 0) ? DESIGNER : REVIEWER; // all race for ONE terminal results.add(pool.submit(() -> { go.await(); return registry.bind(slot, "shared_term") @@ -228,11 +259,11 @@ class MemberRegistryTest { /** A handed-over slot map is snapshotted at construction, not offered as live state. */ @Test void theSlotSnapshotIsFixedByConstruction() { - Map mutable = new HashMap<>(SLOTS); - MemberRegistry r = new MemberRegistry(mutable); + Map mutable = architects(); + MemberRegistry r = new MemberRegistry(fleetWith(mutable)); - mutable.put("hijack", new BridgedConfig.Member("architect", "gx10")); + mutable.put("hijack", new BridgedConfig.Slot("gx10")); - assertFalse(r.isSlot("hijack"), "a handed-over map is not offered as live state"); + assertFalse(r.isSlot("architect: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 cb10a51..49a853a 100644 --- a/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/config/BridgedConfigTest.java @@ -1,6 +1,7 @@ package dev.ltms.bridged.config; import dev.ltms.bridged.auth.MemberRegistry; +import dev.ltms.bridged.peer.MemberRole; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -63,13 +64,12 @@ class BridgedConfigTest { BridgedConfig cfg = BridgedConfig.load(f); assertEquals(Set.of("ltms-local"), cfg.profiles().keySet()); - assertNull(cfg.defaultProfile(), "nothing named a default"); assertEquals("ltms-local", cfg.effectiveDefaultProfile(), "with one profile configured there is nothing to choose between"); } @Test - void loadsMultipleWorkerProfilesWithADefault(@TempDir Path dir) throws Exception { + void loadsMultipleProfilesAndPicksADevDefaultFromTheirPool(@TempDir Path dir) throws Exception { Path f = dir.resolve("multi.yaml"); Files.writeString(f, """ profiles: @@ -79,7 +79,12 @@ class BridgedConfigTest { ollama: baseUrl: http://ollama.ltms.dev argv: ["ccs", "ollama"] - defaultProfile: gx10 + fleet: + developers: + gx10: + profile: gx10 + ollama: + profile: ollama guard: offSubscriptionHosts: [gx10.gw, ollama.ltms.dev] """); @@ -91,7 +96,10 @@ class BridgedConfigTest { assertEquals(java.util.List.of("gx10", "ollama"), java.util.List.copyOf(cfg.profiles().keySet()), "profiles must preserve YAML definition order"); - assertEquals("gx10", cfg.defaultProfile()); + assertEquals(List.of("gx10", "ollama"), cfg.candidateProfiles(MemberRole.DEV), + "the dev pool supplies the candidates, in definition order"); + assertEquals("gx10", cfg.effectiveDefaultProfile(), + "the fixed policy answers with the pool's first entry"); assertEquals("ollama", cfg.profiles().get("ollama").profile(), "profile defaults to its map key"); assertEquals("http://gx10.gw:8000", cfg.profiles().get("gx10").baseUrl()); } @@ -124,7 +132,6 @@ class BridgedConfigTest { port: 8080 herdrSocket: /tmp/s profiles: {} - defaultProfile: a guard: {} worktreeRoot: /tmp lifecycle: {} @@ -132,9 +139,7 @@ class BridgedConfigTest { spawnReadyPollMs: 1 broker: {} primary: {} - leaders: {} - members: {} - leadScan: {} + fleet: {} leadHeartbeat: {} placement: fixed auth: {} @@ -150,38 +155,93 @@ class BridgedConfigTest { // ── CB-531: lead discovery by tab label ───────────────────────────────────────────────────── @Test - void leadScanIsOffUnlessTheBlockIsPresent(@TempDir Path dir) throws Exception { + void leadScanIsOffUnlessALeadIsConfigured(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-scan.yaml"); Files.writeString(f, "bind:\n port: 8080\n"); - assertNull(BridgedConfig.load(f).leadScan(), + assertTrue(BridgedConfig.load(f).fleet().leaders().isEmpty(), "turning this on widens who resolves as PRIMARY — upgrading the daemon must not do that"); } @Test - void leadScanDefaultsItsFieldsWhenTheBlockIsPresentButBare(@TempDir Path dir) throws Exception { + void aLeadDefaultsItsScanFieldsWhenPresentButBare(@TempDir Path dir) throws Exception { Path f = dir.resolve("bare-scan.yaml"); - Files.writeString(f, "bind:\n port: 8080\nleadScan: {}\n"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + opus: + terminal: term_opus + """); - BridgedConfig.LeadScan scan = BridgedConfig.load(f).leadScan(); - assertEquals("lead:", scan.tabPrefix()); - assertEquals(10, scan.intervalSeconds()); + BridgedConfig.Leader lead = BridgedConfig.load(f).fleet().leaders().get("opus"); + assertEquals("lead:", lead.tabPrefix()); + assertEquals(10, lead.scanIntervalSeconds()); + assertEquals(1, lead.instances(), "one of a lead is the assumption worth defaulting to"); } @Test - void leadScanReadsAnExplicitPrefixAndInterval(@TempDir Path dir) throws Exception { + void aLeadReadsAnExplicitPrefixAndInterval(@TempDir Path dir) throws Exception { Path f = dir.resolve("scan.yaml"); Files.writeString(f, """ bind: port: 8080 - leadScan: - tabPrefix: "drive:" - intervalSeconds: 30 + fleet: + leaders: + opus: + terminal: term_opus + tabPrefix: "drive:" + scanIntervalSeconds: 30 """); - BridgedConfig.LeadScan scan = BridgedConfig.load(f).leadScan(); - assertEquals("drive:", scan.tabPrefix()); - assertEquals(30, scan.intervalSeconds()); + BridgedConfig.Leader lead = BridgedConfig.load(f).fleet().leaders().get("opus"); + assertEquals("drive:", lead.tabPrefix()); + assertEquals(30, lead.scanIntervalSeconds()); + } + + /** + * 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. + */ + @Test + void aLeadIsCreatableOnlyWhenItNamesAProfile(@TempDir Path dir) throws Exception { + Path f = dir.resolve("creatable.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + opus: + subscription: true + fleet: + leaders: + launched: + profile: opus + pinned: + terminal: term_opus + """); + + var leaders = BridgedConfig.load(f).fleet().leaders(); + assertTrue(leaders.get("launched").isCreatable()); + assertFalse(leaders.get("pinned").isCreatable(), + "no profile to launch on ⇒ recognise-only, the pre-CB-557 behaviour"); + } + + @Test + void aLeadThatCanBeNeitherFoundNorCreatedRefusesToStart(@TempDir Path dir) throws Exception { + Path f = dir.resolve("useless-lead.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + leaders: + ghost: + tabPrefix: "lead:" + """); + BridgedConfig cfg = BridgedConfig.load(f); + + IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateMembers); + assertTrue(e.getMessage().contains("ghost"), "the message must name the useless entry"); } // ── CB-551: the idle-lead heartbeat ───────────────────────────────────────────────────────── @@ -234,7 +294,8 @@ class BridgedConfigTest { * Overlap the two and every worker it spawns is read back as a lead. */ @Test - void aLeadPrefixThatAWorkerTabLabelAlsoMatchesRefusesToStart(@TempDir Path dir) throws Exception { + void aLeadPrefixThatAProfileTabLabelOverrideAlsoMatchesRefusesToStart(@TempDir Path dir) + throws Exception { Path f = dir.resolve("collide.yaml"); Files.writeString(f, """ bind: @@ -242,17 +303,45 @@ class BridgedConfigTest { profiles: gx10: tabLabel: "lead: {profile} #{n}" - leadScan: - tabPrefix: "lead:" + fleet: + leaders: + opus: + terminal: term_opus + tabPrefix: "lead:" """); BridgedConfig cfg = BridgedConfig.load(f); - IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateLeadScan); + IllegalStateException e = + assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes); assertTrue(e.getMessage().contains("gx10"), "the message must name the offending profile"); } + /** A bad fleet-wide template promotes every member, not one profile — so it is checked too. */ @Test - void theDefaultWorkerTabLabelDoesNotCollideWithTheDefaultLeadPrefix(@TempDir Path dir) throws Exception { + void aFleetTabLabelThatMatchesALeadPrefixRefusesToStart(@TempDir Path dir) throws Exception { + Path f = dir.resolve("collide-template.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + fleet: + tabLabel: "lead: {role} {profile}" + leaders: + opus: + terminal: term_opus + """); + BridgedConfig cfg = BridgedConfig.load(f); + + IllegalStateException e = + assertThrows(IllegalStateException.class, cfg::validateLeadTabPrefixes); + assertTrue(e.getMessage().contains("fleet.tabLabel")); + } + + /** + * The point of making role the label's first field: {@code {role}} comes from a closed enum, so + * a generated label cannot begin with {@code "lead:"} however the fleet is configured. + */ + @Test + void theDefaultTabLabelCannotCollideWithTheDefaultLeadPrefix(@TempDir Path dir) throws Exception { Path f = dir.resolve("ok.yaml"); Files.writeString(f, """ bind: @@ -260,14 +349,22 @@ class BridgedConfigTest { profiles: gx10: baseUrl: http://gx00.gw:8000 - leadScan: {} + fleet: + leaders: + opus: + terminal: term_opus """); - assertDoesNotThrow(() -> BridgedConfig.load(f).validateLeadScan()); + assertDoesNotThrow(() -> BridgedConfig.load(f).validateLeadTabPrefixes()); + for (MemberRole role : MemberRole.values()) { + assertFalse(BridgedConfig.Fleet.DEFAULT_TAB_LABEL + .replace("{role}", role.wireName()).startsWith("lead:"), + "no role renders a label that reads as a lead"); + } } @Test - void theCollisionGuardIsANoOpWhenScanningIsOff(@TempDir Path dir) throws Exception { + void theCollisionGuardIsANoOpWhenNoLeadIsConfigured(@TempDir Path dir) throws Exception { Path f = dir.resolve("off.yaml"); Files.writeString(f, """ bind: @@ -277,7 +374,7 @@ class BridgedConfigTest { tabLabel: "lead: {profile}" """); - assertDoesNotThrow(() -> BridgedConfig.load(f).validateLeadScan(), + assertDoesNotThrow(() -> BridgedConfig.load(f).validateLeadTabPrefixes(), "a label that collides with a convention nobody reads is not a problem"); } @@ -289,21 +386,23 @@ class BridgedConfigTest { Files.writeString(f, """ bind: port: 8080 - leaders: - opus-5.0: - terminal: term_opus - kind: claude - gpt-sol-5.6: - terminal: term_sol - kind: opencode - model: openai/gpt-5.6-terra + fleet: + leaders: + opus-5.0: + terminal: term_opus + kind: claude + gpt-sol-5.6: + terminal: term_sol + kind: opencode + model: openai/gpt-5.6-terra """); BridgedConfig cfg = BridgedConfig.load(f); + var leaders = cfg.fleet().leaders(); - assertEquals(Set.of("opus-5.0", "gpt-sol-5.6"), cfg.leaders().keySet()); - assertEquals("opencode", cfg.leaders().get("gpt-sol-5.6").kind()); - assertEquals("openai/gpt-5.6-terra", cfg.leaders().get("gpt-sol-5.6").model()); + 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()); @@ -326,9 +425,10 @@ class BridgedConfigTest { port: 8080 primary: terminal: term_shared - leaders: - opus-5.0: - terminal: term_shared + fleet: + leaders: + opus-5.0: + terminal: term_shared """); assertEquals(Map.of("term_shared", "opus-5.0"), BridgedConfig.load(f).leaderTerminals(), @@ -343,9 +443,10 @@ class BridgedConfigTest { port: 8080 primary: terminal: term_pinned - leaders: - gpt-sol-5.6: - terminal: term_sol + fleet: + leaders: + gpt-sol-5.6: + terminal: term_sol """); assertEquals(Map.of("term_pinned", "primary", "term_sol", "gpt-sol-5.6"), @@ -367,11 +468,12 @@ class BridgedConfigTest { Files.writeString(f, """ bind: port: 8080 - leaders: - sketch: - kind: opencode - real: - terminal: term_real + fleet: + leaders: + sketch: + kind: opencode + real: + terminal: term_real """); assertEquals(Map.of("term_real", "real"), BridgedConfig.load(f).leaderTerminals()); @@ -380,7 +482,7 @@ class BridgedConfigTest { // ── CB-548: the architects registry ──────────────────────────────────────────────────────── @Test - void architectsBlockDeclaresSlotsByNameAndProfileOnly(@TempDir Path dir) throws Exception { + void aRolePoolDeclaresSlotsByNameAndProfileOnly(@TempDir Path dir) throws Exception { Path f = dir.resolve("architects.yaml"); Files.writeString(f, """ bind: @@ -388,28 +490,81 @@ class BridgedConfigTest { profiles: sonnet: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - role: architect - profile: sonnet - reviewer: - role: architect - profile: sonnet + fleet: + architects: + lead-designer: + profile: sonnet + second-opinion: + profile: sonnet """); BridgedConfig cfg = BridgedConfig.load(f); - assertEquals(Set.of("lead-designer", "reviewer"), cfg.members().keySet(), - "slot names are the keys — gateway-local unique by construction"); - assertEquals("sonnet", cfg.members().get("lead-designer").profile(), - "each slot carries its strong-model profile reference"); - assertEquals("sonnet", cfg.members().get("reviewer").profile()); + var pool = cfg.fleet().pool(MemberRole.ARCHITECT); + assertEquals(Set.of("lead-designer", "second-opinion"), pool.keySet(), + "slot names are the keys — unique within their pool by construction"); + assertEquals("sonnet", pool.get("lead-designer").profile(), + "each slot carries its backend reference"); + assertEquals("sonnet", pool.get("second-opinion").profile()); + } + + /** + * The role is the containing key now (CB-557), so it cannot be misspelled into a member with no + * contract. A pool name that is not a role is simply not a pool. + */ + @Test + void theRoleIsTheContainingKeyNotAField(@TempDir Path dir) throws Exception { + Path f = dir.resolve("role-by-key.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + sonnet: + baseUrl: http://gx10.gw:8000 + fleet: + developers: + a: + profile: sonnet + reviewers: + b: + profile: sonnet + """); + + BridgedConfig cfg = BridgedConfig.load(f); + assertEquals(List.of("sonnet"), cfg.fleet().profilesFor(MemberRole.DEV)); + assertEquals(List.of("sonnet"), cfg.fleet().profilesFor(MemberRole.REVIEWER)); + assertTrue(cfg.fleet().profilesFor(MemberRole.ARCHITECT).isEmpty()); + assertEquals(List.of(MemberRole.DEV, MemberRole.REVIEWER), cfg.fleet().rolesConfigured()); + } + + /** The case the two axes exist for: one backend, two roles, and neither is a duplicate. */ + @Test + void oneProfileMayServeSeveralRoles(@TempDir Path dir) throws Exception { + Path f = dir.resolve("shared.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + sonnet: + baseUrl: http://gx10.gw:8000 + fleet: + developers: + sonnet: + profile: sonnet + reviewers: + sonnet: + profile: sonnet + """); + + BridgedConfig cfg = BridgedConfig.load(f); + assertDoesNotThrow(cfg::validateMembers); + assertEquals("sonnet", cfg.defaultProfileFor(MemberRole.DEV)); + assertEquals("sonnet", cfg.defaultProfileFor(MemberRole.REVIEWER)); } @Test - 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. + void aSlotCarriesNoConfigTerminalSoNothingIsRecognisedYet(@TempDir Path dir) throws Exception { + // Config declares slots (name + profile) only. A `terminal:` key is ignored — a member is + // NOT recognised from config the way a lead is, so it binds nothing at startup. Path f = dir.resolve("arch-stale-terminal.yaml"); Files.writeString(f, """ bind: @@ -417,35 +572,38 @@ class BridgedConfigTest { profiles: sonnet: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - terminal: term_design - profile: sonnet + fleet: + architects: + lead-designer: + terminal: term_design + profile: sonnet """); BridgedConfig cfg = BridgedConfig.load(f); - assertEquals("sonnet", cfg.members().get("lead-designer").profile(), + assertEquals("sonnet", cfg.fleet().pool(MemberRole.ARCHITECT).get("lead-designer").profile(), "the profile is still read even when a stray terminal is ignored"); // The registry built from this config owns no bindings: the slot is idle at startup. - MemberRegistry r = new MemberRegistry(cfg.members()); + MemberRegistry r = new MemberRegistry(cfg.fleet()); assertTrue(r.snapshot().isEmpty()); assertNull(r.slotForTerminal("term_design"), - "a config terminal must not resolve an architect — slots start idle"); + "a config terminal must not resolve a member — slots start idle"); } @Test - void noArchitectsBlockLeavesNothingConfigured(@TempDir Path dir) throws Exception { + void noFleetBlockLeavesEveryPoolEmpty(@TempDir Path dir) throws Exception { Path f = dir.resolve("no-arch.yaml"); Files.writeString(f, "bind:\n port: 8080\n"); - assertNull(BridgedConfig.load(f).members(), - "no members: block ⇒ no architect identity, exactly as before CB-548"); + BridgedConfig cfg = BridgedConfig.load(f); + assertNotNull(cfg.fleet(), "an absent block is an empty fleet, not a null one"); + assertTrue(cfg.fleet().rolesConfigured().isEmpty(), + "no fleet: block ⇒ no member 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. + void aSlotMayResolveToTheSoleProfileWithoutPrivileging(@TempDir Path dir) throws Exception { + // Even a single unqualified profile must be named explicitly — the reference is by name, + // not by position. Path f = dir.resolve("arch-single.yaml"); Files.writeString(f, """ bind: @@ -453,15 +611,16 @@ class BridgedConfigTest { profiles: ltms-local: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - role: architect - profile: ltms-local + fleet: + architects: + lead-designer: + profile: ltms-local """); BridgedConfig cfg = BridgedConfig.load(f); assertDoesNotThrow(cfg::validateMembers); - assertEquals("ltms-local", cfg.members().get("lead-designer").profile()); + assertEquals("ltms-local", + cfg.fleet().pool(MemberRole.ARCHITECT).get("lead-designer").profile()); } @Test @@ -473,10 +632,10 @@ class BridgedConfigTest { profiles: gx10: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - role: architect - profile: sonnet + fleet: + architects: + lead-designer: + profile: sonnet """); BridgedConfig cfg = BridgedConfig.load(f); @@ -494,10 +653,10 @@ class BridgedConfigTest { profiles: gx10: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - role: architect - profile: "" + fleet: + architects: + lead-designer: + profile: "" """); BridgedConfig cfg = BridgedConfig.load(f); @@ -516,13 +675,13 @@ class BridgedConfigTest { baseUrl: http://gx10.gw:8000 gx10: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - role: architect - profile: sonnet - reviewer: - role: architect - profile: gx10 + fleet: + architects: + lead-designer: + profile: sonnet + reviewers: + second-pair-of-eyes: + profile: gx10 """); assertDoesNotThrow(() -> BridgedConfig.load(f).validateMembers()); @@ -537,7 +696,7 @@ class BridgedConfigTest { } @Test - void duplicateArchitectSlotNamesAreRejectedAtParseTime(@TempDir Path dir) throws Exception { + void duplicateSlotNamesInOnePoolAreRejectedAtParseTime(@TempDir Path dir) throws Exception { Path f = dir.resolve("arch-dup.yaml"); Files.writeString(f, """ bind: @@ -545,26 +704,52 @@ class BridgedConfigTest { profiles: sonnet: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - role: architect - profile: sonnet - lead-designer: - role: architect - profile: sonnet + fleet: + 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 member"), - "the refusal says the slot name is duplicated"); + assertTrue(e.getMessage().contains("fleet.architects"), + "the refusal names the pool the duplicate is in, was: " + e.getMessage()); + } + + /** + * The same name in two different pools is the role × profile matrix, not a mistake — only a + * repeat within one pool loses an entry. + */ + @Test + void theSameSlotNameInTwoPoolsIsNotADuplicate(@TempDir Path dir) throws Exception { + Path f = dir.resolve("cross-pool.yaml"); + Files.writeString(f, """ + bind: + port: 8080 + profiles: + sonnet: + baseUrl: http://gx10.gw:8000 + fleet: + developers: + sonnet: + profile: sonnet + reviewers: + sonnet: + profile: sonnet + """); + + BridgedConfig cfg = assertDoesNotThrow(() -> BridgedConfig.load(f)); + assertEquals(Set.of("sonnet"), cfg.fleet().pool(MemberRole.DEV).keySet()); + assertEquals(Set.of("sonnet"), cfg.fleet().pool(MemberRole.REVIEWER).keySet()); } @Test - void duplicateKeysOutsideArchitectsAreUnaffected(@TempDir Path dir) throws Exception { - // The duplicate check is scoped to the architects block — a duplicate elsewhere is not this + void duplicateKeysOutsideTheFleetPoolsAreUnaffected(@TempDir Path dir) throws Exception { + // The duplicate check is scoped to the fleet pools — 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, """ @@ -576,47 +761,47 @@ class BridgedConfigTest { sonnet: baseUrl: http://gx10.gw:8000 """); - // Last-wins for a non-architect duplicate is untouched: only the architects block is walked. + // Last-wins for a non-pool duplicate is untouched: only the fleet pools are walked. assertEquals(Set.of("sonnet"), BridgedConfig.load(f).profiles().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). + void aNestedFleetFieldDoesNotSuppressRealDuplicateDetection(@TempDir Path dir) throws Exception { + // A field ALSO named `fleet` 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 - members: - nested-slot: - k: v - nested-slot: - k: v - members: - lead-designer: - role: architect - profile: sonnet - lead-designer: - role: architect - profile: sonnet + fleet: + architects: + nested-slot: + k: v + nested-slot: + k: v + fleet: + 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 member"), - "the refusal says the slot name is duplicated"); + assertFalse(e.getMessage().contains("nested-slot"), + "the nested block must not be inspected, was: " + e.getMessage()); } @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. + // child keys of a pool 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: @@ -624,20 +809,21 @@ class BridgedConfigTest { profiles: sonnet: baseUrl: http://gx10.gw:8000 - members: - lead-designer: - role: architect - profile: sonnet - extra: - a: 1 - a: 1 + fleet: + architects: + lead-designer: + profile: sonnet + extra: + a: 1 + a: 1 """); BridgedConfig cfg = assertDoesNotThrow(() -> BridgedConfig.load(f)); assertDoesNotThrow(cfg::validateMembers, "a nested duplicate inside a slot is not a duplicate slot and must not refuse startup"); - assertEquals(Set.of("lead-designer"), cfg.members().keySet()); - assertEquals("sonnet", cfg.members().get("lead-designer").profile()); + var pool = cfg.fleet().pool(MemberRole.ARCHITECT); + assertEquals(Set.of("lead-designer"), pool.keySet()); + assertEquals("sonnet", pool.get("lead-designer").profile()); } @Test @@ -853,7 +1039,7 @@ class BridgedConfigTest { BridgedConfig cfg = BridgedConfig.load(example); assertEquals(8765, cfg.bind().port(), "example binds the documented default port"); assertTrue(cfg.profiles().containsKey("gx10"), "example documents the gx10 profile"); - assertEquals("gx10", cfg.defaultProfile(), "example's defaultWorker resolves"); + assertEquals("gx10", cfg.effectiveDefaultProfile(), "example's dev pool resolves"); assertTrue(cfg.guard().hostSet().contains("gx01.gw"), "every example profile's base_url host must be in the example allowlist"); } @@ -1095,8 +1281,8 @@ class BridgedConfigTest { """); IllegalStateException e = assertThrows(IllegalStateException.class, () -> BridgedConfig.load(f)); - assertTrue(e.getMessage().contains("'architects' is now 'members'"), e.getMessage()); - assertTrue(e.getMessage().contains("'defaultWorker' is now 'defaultProfile'"), e.getMessage()); + assertTrue(e.getMessage().contains("'architects' is now 'fleet.architects'"), e.getMessage()); + assertTrue(e.getMessage().contains("'defaultWorker' is now a role pool"), e.getMessage()); assertTrue(e.getMessage().contains("'workers' is now 'profiles'"), e.getMessage()); } @@ -1109,20 +1295,21 @@ class BridgedConfigTest { baseUrl: http://gx00.gw:8000 opus: baseUrl: http://gx00.gw:8000 - members: - architect-1: - role: architect - profile: opus - reviewer-1: - role: reviewer - profile: gx10 + fleet: + architects: + architect-1: + profile: opus + reviewers: + reviewer-1: + profile: gx10 """); BridgedConfig cfg = BridgedConfig.load(f); assertDoesNotThrow(cfg::validateMembers); - assertEquals("architect", cfg.members().get("architect-1").role()); - assertEquals("opus", cfg.members().get("architect-1").profile()); - assertEquals("reviewer", cfg.members().get("reviewer-1").role()); + assertEquals("opus", cfg.fleet().pool(MemberRole.ARCHITECT).get("architect-1").profile()); + assertEquals("gx10", cfg.fleet().pool(MemberRole.REVIEWER).get("reviewer-1").profile()); + assertEquals(List.of(MemberRole.ARCHITECT, MemberRole.REVIEWER), + cfg.fleet().rolesConfigured(), "the pool a slot sits in IS its role"); } /** @@ -1136,56 +1323,77 @@ class BridgedConfigTest { profiles: gx10: baseUrl: http://gx00.gw:8000 - members: - dev-1: - role: dev - profile: gx10 - reviewer-1: - role: reviewer - profile: gx10 + fleet: + developers: + dev-1: + profile: gx10 + reviewers: + reviewer-1: + profile: gx10 """); BridgedConfig cfg = BridgedConfig.load(f); assertDoesNotThrow(cfg::validateMembers); - assertEquals(cfg.members().get("dev-1").profile(), cfg.members().get("reviewer-1").profile()); - assertNotEquals(cfg.members().get("dev-1").role(), cfg.members().get("reviewer-1").role()); + assertEquals(cfg.fleet().pool(MemberRole.DEV).get("dev-1").profile(), + cfg.fleet().pool(MemberRole.REVIEWER).get("reviewer-1").profile(), + "one backend, two roles — the axes are independent"); + assertEquals("gx10", cfg.defaultProfileFor(MemberRole.DEV)); + assertEquals("gx10", cfg.defaultProfileFor(MemberRole.REVIEWER)); } + /** + * What the pool shape bought over the old {@code role:} field. A misspelled role used to parse + * into a member with no contract, so it needed catching by name. Now it is a pool name nobody + * reads: the slot simply does not exist, and no member can run under a role that is not one. + */ @Test - void aMemberSlotWithAnUnknownRoleIsRejectedAndListsTheValidRoles(@TempDir Path dir) throws Exception { + void aMisspelledPoolNameDeclaresNoMembersRatherThanRolelessOnes(@TempDir Path dir) + throws Exception { Path f = dir.resolve("bridged.yaml"); Files.writeString(f, """ profiles: gx10: baseUrl: http://gx00.gw:8000 - members: - slot-1: - role: archtiect - profile: gx10 + fleet: + archtiects: + slot-1: + profile: gx10 """); BridgedConfig cfg = BridgedConfig.load(f); - IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateMembers); - assertTrue(e.getMessage().contains("archtiect"), e.getMessage()); - assertTrue(e.getMessage().contains("architect, dev, reviewer"), - "the error must list the valid spellings so the typo is fixable from it: " + e.getMessage()); + assertDoesNotThrow(cfg::validateMembers); + assertTrue(cfg.fleet().rolesConfigured().isEmpty(), + "a pool name that is not a role declares nothing at all"); + assertNull(MemberRole.fromConfigKey("archtiects")); + } + + /** Every role's pool key must round-trip, or a correctly-spelled block would be dropped. */ + @Test + void everyRolePoolKeyRoundTrips() { + for (MemberRole role : MemberRole.values()) { + assertEquals(role, MemberRole.fromConfigKey(role.configKey()), + role + " must be readable back from the key it is written under"); + } + assertNull(MemberRole.fromConfigKey("leaders"), "a lead is not a member role"); + assertNull(MemberRole.fromConfigKey("tabLabel"), "a non-pool fleet key is not a role"); } @Test - void aMemberSlotWithNoRoleIsRejected(@TempDir Path dir) throws Exception { + void aSlotWithNoProfileIsRejectedAndNamesItsPool(@TempDir Path dir) throws Exception { Path f = dir.resolve("bridged.yaml"); Files.writeString(f, """ profiles: gx10: baseUrl: http://gx00.gw:8000 - members: - slot-1: - profile: gx10 + fleet: + developers: + slot-1: {} """); BridgedConfig cfg = BridgedConfig.load(f); IllegalStateException e = assertThrows(IllegalStateException.class, cfg::validateMembers); - assertTrue(e.getMessage().contains("has no role:"), e.getMessage()); + assertTrue(e.getMessage().contains("fleet.developers.slot-1"), e.getMessage()); + assertTrue(e.getMessage().contains("has no profile:"), e.getMessage()); } @Test @@ -1221,7 +1429,9 @@ class BridgedConfigTest { """); BridgedConfig cfg = BridgedConfig.load(f); - assertNull(cfg.defaultProfile(), "the raw config value is absent"); - assertEquals("gx10", cfg.effectiveDefaultProfile(), "the resolved value is the first profile"); + assertTrue(cfg.fleet().profilesFor(MemberRole.DEV).isEmpty(), "no dev pool is configured"); + assertEquals("gx10", cfg.effectiveDefaultProfile(), + "with no pool to choose from, every configured profile is a candidate and the " + + "first one wins"); } } diff --git a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java index 62f8876..b421ffc 100644 --- a/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/member/ClaudeCodeLauncherTest.java @@ -7,6 +7,7 @@ import dev.ltms.bridged.herdr.AgentControl; import dev.ltms.bridged.herdr.FakeHerdr; import dev.ltms.bridged.herdr.WorkspaceControl; import dev.ltms.bridged.peer.Capability; +import dev.ltms.bridged.peer.MemberRole; import dev.ltms.bridged.peer.PeerHandle; import dev.ltms.bridged.peer.PeerUnreachableException; import dev.ltms.bridged.peer.SpawnRequest; @@ -782,4 +783,79 @@ class ClaudeCodeLauncherTest { assertEquals("/opt/jdk", env.get("JAVA_HOME"), "only the Anthropic binding keys are stripped; the rest of env: still applies"); } + + // ── CB-557: role-aware tab labels ───────────────────────────────────────────────────────── + + /** The {@code label} of every {@code tab.rename}, in call order. */ + private List tabLabels(FakeHerdr herdr) { + return herdr.calls.stream() + .filter(c -> c.method().equals("tab.rename")) + .map(c -> (String) ((Map) c.params()).get("label")) + .toList(); + } + + /** A profile with no {@code tabLabel:} of its own — the fleet template decides. */ + private ClaudeCodeLauncher labelService(FakeHerdr herdr, String fleetTemplate) { + BridgedConfig.Profile cfg = new BridgedConfig.Profile( + "sonnet", "http://gx00.gw:8000", "sonnet", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", null, null, null, null); + return new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + _ -> null, 0, 0L, fleetTemplate); + } + + /** + * The knob must reach the rename call. It was inert once — {@code HerdrPeerLauncher} accepted a + * template while {@code Bridged} passed none, and the label stayed right only because the + * fallback happened to match. Pin the wiring, not the coincidence. + */ + @Test + void theFleetTemplateNamesTheRoleTheMemberWasSpawnedFor() { + FakeHerdr herdr = new FakeHerdr(); + ClaudeCodeLauncher svc = labelService(herdr, "{role}: {profile} #{n}"); + + svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.REVIEWER)); + + assertEquals(List.of("reviewer: sonnet #1"), tabLabels(herdr)); + } + + /** The counter is per role+profile, so a dev and a reviewer on one profile both start at #1. */ + @Test + void theCounterRunsPerRoleAndProfileNotPerFleet() { + FakeHerdr herdr = new FakeHerdr(); + ClaudeCodeLauncher svc = labelService(herdr, "{role}: {profile} #{n}"); + + svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.DEV)); + svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.REVIEWER)); + svc.spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.DEV)); + + assertEquals(List.of("dev: sonnet #1", "reviewer: sonnet #1", "dev: sonnet #2"), + tabLabels(herdr)); + } + + /** No fleet template configured ⇒ the built-in default, still role-first. */ + @Test + void aBlankFleetTemplateFallsBackToTheRoleFirstDefault() { + FakeHerdr herdr = new FakeHerdr(); + labelService(herdr, null).spawn( + new SpawnRequest("sonnet", null, null, null, null, MemberRole.ARCHITECT)); + + assertEquals(List.of("architect: sonnet #1"), tabLabels(herdr)); + assertEquals("{role}: {profile} #{n}", BridgedConfig.Fleet.DEFAULT_TAB_LABEL); + } + + /** A profile that wants its own label still outranks the fleet template. */ + @Test + void aProfileTabLabelOverridesTheFleetTemplate() { + FakeHerdr herdr = new FakeHerdr(); + BridgedConfig.Profile cfg = new BridgedConfig.Profile( + "sonnet", "http://gx00.gw:8000", "sonnet", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "pinned {profile}", null, null, null); + new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + _ -> null, 0, 0L, "{role}: {profile} #{n}") + .spawn(new SpawnRequest("sonnet", null, null, null, null, MemberRole.REVIEWER)); + + assertEquals(List.of("pinned sonnet"), tabLabels(herdr)); + } }