fleet/fleetd
3
0
Fork 0
Code Issues 73 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

CB-579: resolve a lead by its tab name, and delete the terminal-id pin #51

New Issue
Closed
opened 2026-08-15 07:03:33 +02:00 by ltms · 0 comments
ltms commented 2026-08-15 07:03:33 +02:00
Owner
Copy Link
Copy Source

Why

A lead's identity is pinned by fleet.leaders.<name>.terminal, a herdr terminal_id. That id is
dynamic: it changes every time the lead's session restarts. So every restart costs a config edit,
and the id can only be learned by first starting the session and asking it (bridge_whoami).

The tab, by contrast, is stable. It is opened once by a human, it holds exactly one pane, and its
label survives every restart of the agent inside it. The tab name is the right identity.

Most of the machinery already exists. CB-531 added LeadTabScanner, which walks
workspace.list → tab.list → pane.list and returns terminal_id → lead name.
CallerResolver.resolve (auth/CallerResolver.java:208-228) consults it as a live Supplier on
every resolve, so a lead is already recognised by its tab label. What is missing is that the
terminal pin still exists beside it, outranks it, and never expires.

The bug this closes

LeadTabScanner.scan() ends with byTerminal.putAll(configuredLeads)
(herdr/LeadTabScanner.java:154) — the config pin is merged over the scan result. A pin is
therefore permanent: it names a terminal_id as a lead for as long as the daemon runs, whether or
not that pane still exists.

Observed live on 2026-08-15. bridged.yaml pinned term_658aa59414a3a7. That session had been
replaced by term_6590ec16d693560. bridge_list returned both as leads named opus: the new
one found by the tab scan, the dead one held up by the pin.

"leads":[{"sessionId":"term_6590ec16d693560","name":"opus","status":"working","self":true},
         {"sessionId":"term_658aa59414a3a7","name":"opus","status":"unknown"}]

A phantom lead is not cosmetic. Bridged.java:484-486 makes any lead terminal deliverable
without MCP presence, and PrimaryRegistry.nudgeTargetFor refuses to guess once more than one lead
exists — so a ghost row can degrade reply delivery from push to pull for the real lead.

Scope

A — tab: replaces terminal:

Add tab: to BridgedConfig.Leader (config/BridgedConfig.java:478): the exact tab label that
hosts this lead, matched case-insensitively.

fleet:
  leaders:
    opus:
      profile: opus
      tab: "lead: opus"      # stable — replaces terminal:
      instances: 1
      tabPrefix: "lead:"
      workspace: leads

Delete terminal: from the record. A config that still carries it must fail loudly at load,
naming the replacement. Note that Leader is annotated @JsonIgnoreProperties(ignoreUnknown = true)
(:477), so simply removing the component makes the key silently ignored — the operator would
get no error and a lead that resolves only by luck. Add terminal to the existing legacy-key
migration check (:1033-1034 already does this for leaders: → fleet.leaders and leadScan:)
rather than relying on the record shape.

B — match by name, not by prefix

LeadTabScanner takes a single tabPrefix and derives the lead name by stripping it
(:88, :164-174). Replace that with the configured tab: → name map, matched exactly.

This removes a real limitation: Bridged.java:206 builds one scanner from the first leader's
settings, and :216-223 warns that leads declaring a different tabPrefix will never be discovered.
With an explicit tab: per leader that constraint disappears — delete the warning with it.

tabPrefix stays, but its job narrows to one thing: the startup collision guard
(validateLeadTabPrefixes, :1156-1190) that refuses a member tabLabel starting with a lead's
prefix. Document that this is now its only purpose.

C — the auto-launch path

LeadLauncher already renames a tab it creates (lead/LeadLauncher.java:226, after agent.start
succeeds) and already counts a lead live by exact case-insensitive label equality
(:196) — so the launcher and the scanner stop disagreeing once both match exactly.

  • Leader.tabLabel(name) (config/BridgedConfig.java:510) currently synthesises tabPrefix + " " + name. It must return the configured tab: value instead.
  • LeadLauncher:164-169 builds terminalId → name from lead.terminal(). Delete it; the
    Agent record already carries both tabId() and terminalId() (herdr/Agent.java:23), so
    liveness is decidable from the label alone.
  • validateMembers (:1316) rejects a leader that is "neither findable nor creatable" via
    leader.terminal() == null || isBlank(). Switch that test to tab().

Out of scope — but write it down

  1. fleet.leaders is not actually hot. ConfigRef documents fleet: as a hot key, and it is —
    for the member path, which reads it through a supplier (CompositePeerLauncher.java:155). The
    lead path does not: Bridged.java:192 (leaderTerminals()), :201 (fleet().leaders()), :206
    (tabPrefix) and new LeadLauncher(…, cfg) at :233 all capture the startup snapshot. Only
    the scan result is live, on its 10s TTL. So a tab: edit needs a restart.

    This matters far less once the key is a stable tab name — the point of the change is that you stop
    editing it. Do not fix it in this unit. File a follow-up for making Bridged.java:200-226
    supplier-based, and in this unit correct the comments that claim it is hot, including
    bridged.yaml:116 and the Leader javadoc at :465-466 that still calls terminal "the only
    field identity depends on".

  2. primary.terminal is a different, already-deprecated key. It seeds PrimaryRegistry
    (Bridged.java:315-325) for nudge addressing, not identity, and CB-532's leadByTarget map is
    what retires it. Leave it alone here.

Acceptance criteria

  1. A lead is resolved to Role.PRIMARY purely from its tab label; no terminal id appears in
    fleet.leaders config.
  2. terminal: under fleet.leaders.<name> fails the config load with a message naming tab:. It is
    not silently ignored — prove this with a test, given @JsonIgnoreProperties(ignoreUnknown).
  3. Two leaders with different tab: values are both discovered. (Today only the first leader's
    prefix is scanned.)
  4. A tab label that matches no configured tab: yields no lead — a pane cannot name itself into the
    role.
  5. A lead whose session restarts in the same tab is resolved under the same name, with no config
    edit and no daemon restart.
  6. No permanent entry: when the tab is gone, the lead is gone from the map on the next scan. The
    "config pin outranks and never expires" merge at LeadTabScanner:154 is removed.
  7. A failed herdr scan still keeps the last good map (existing behaviour at :119-122 — do not
    regress it).
  8. LeadLauncher labels a tab it creates with exactly the tab: value, and finds it again.
  9. Comments claiming fleet.leaders is hot are corrected, and the deferred-key list names it.

Note on the silent-default trap — read before writing code

This repo has produced the same bug six times: a new dependency gets a default so existing wiring
compiles, and the capability ships turned off (CB-561, CB-572, CB-573 twice, TestTurnTokens, and
the rejected 3b2f395).

tab must be required where a lead is not creatable. Do not add a Leader constructor overload
that defaults tab to null so existing tests compile — the back-compat 7-arg ctor at
config/BridgedConfig.java:499 is exactly the shape to avoid growing. Update the call sites instead.
An overload that defaults the new field is an automatic rejection at review, because it also means no
test covers the new path.

## Why A lead's identity is pinned by `fleet.leaders.<name>.terminal`, a herdr `terminal_id`. That id is **dynamic**: it changes every time the lead's session restarts. So every restart costs a config edit, and the id can only be learned by first starting the session and asking it (`bridge_whoami`). The tab, by contrast, is stable. It is opened once by a human, it holds exactly one pane, and its label survives every restart of the agent inside it. The tab name is the right identity. Most of the machinery already exists. CB-531 added `LeadTabScanner`, which walks `workspace.list` → `tab.list` → `pane.list` and returns `terminal_id → lead name`. `CallerResolver.resolve` (`auth/CallerResolver.java:208-228`) consults it as a live `Supplier` on every resolve, so a lead is *already* recognised by its tab label. What is missing is that the terminal pin still exists beside it, outranks it, and never expires. ## The bug this closes `LeadTabScanner.scan()` ends with `byTerminal.putAll(configuredLeads)` (`herdr/LeadTabScanner.java:154`) — the config pin is merged **over** the scan result. A pin is therefore permanent: it names a `terminal_id` as a lead for as long as the daemon runs, whether or not that pane still exists. Observed live on 2026-08-15. `bridged.yaml` pinned `term_658aa59414a3a7`. That session had been replaced by `term_6590ec16d693560`. `bridge_list` returned **both** as leads named `opus`: the new one found by the tab scan, the dead one held up by the pin. ```json "leads":[{"sessionId":"term_6590ec16d693560","name":"opus","status":"working","self":true}, {"sessionId":"term_658aa59414a3a7","name":"opus","status":"unknown"}] ``` A phantom lead is not cosmetic. `Bridged.java:484-486` makes any lead terminal **deliverable** without MCP presence, and `PrimaryRegistry.nudgeTargetFor` refuses to guess once more than one lead exists — so a ghost row can degrade reply delivery from push to pull for the real lead. ## Scope ### A — `tab:` replaces `terminal:` Add `tab:` to `BridgedConfig.Leader` (`config/BridgedConfig.java:478`): the **exact** tab label that hosts this lead, matched case-insensitively. ```yaml fleet: leaders: opus: profile: opus tab: "lead: opus" # stable — replaces terminal: instances: 1 tabPrefix: "lead:" workspace: leads ``` **Delete `terminal:` from the record.** A config that still carries it must **fail loudly at load**, naming the replacement. Note that `Leader` is annotated `@JsonIgnoreProperties(ignoreUnknown = true)` (`:477`), so simply removing the component makes the key **silently ignored** — the operator would get no error and a lead that resolves only by luck. Add `terminal` to the existing legacy-key migration check (`:1033-1034` already does this for `leaders:` → `fleet.leaders` and `leadScan:`) rather than relying on the record shape. ### B — match by name, not by prefix `LeadTabScanner` takes a single `tabPrefix` and derives the lead name by stripping it (`:88, :164-174`). Replace that with the configured `tab: → name` map, matched exactly. This removes a real limitation: `Bridged.java:206` builds one scanner from the **first** leader's settings, and `:216-223` warns that leads declaring a different `tabPrefix` will never be discovered. With an explicit `tab:` per leader that constraint disappears — delete the warning with it. `tabPrefix` stays, but its job narrows to one thing: the startup collision guard (`validateLeadTabPrefixes`, `:1156-1190`) that refuses a member `tabLabel` starting with a lead's prefix. Document that this is now its only purpose. ### C — the auto-launch path `LeadLauncher` already renames a tab it creates (`lead/LeadLauncher.java:226`, after `agent.start` succeeds) and already counts a lead live by **exact case-insensitive label equality** (`:196`) — so the launcher and the scanner stop disagreeing once both match exactly. - `Leader.tabLabel(name)` (`config/BridgedConfig.java:510`) currently synthesises `tabPrefix + " " + name`. It must return the configured `tab:` value instead. - `LeadLauncher:164-169` builds `terminalId → name` from `lead.terminal()`. Delete it; the `Agent` record already carries both `tabId()` and `terminalId()` (`herdr/Agent.java:23`), so liveness is decidable from the label alone. - `validateMembers` (`:1316`) rejects a leader that is "neither findable nor creatable" via `leader.terminal() == null || isBlank()`. Switch that test to `tab()`. ## Out of scope — but write it down 1. **`fleet.leaders` is not actually hot.** `ConfigRef` documents `fleet:` as a hot key, and it is — for the *member* path, which reads it through a supplier (`CompositePeerLauncher.java:155`). The lead path does not: `Bridged.java:192` (`leaderTerminals()`), `:201` (`fleet().leaders()`), `:206` (`tabPrefix`) and `new LeadLauncher(…, cfg)` at `:233` all capture the **startup snapshot**. Only the scan *result* is live, on its 10s TTL. So a `tab:` edit needs a restart. This matters far less once the key is a stable tab name — the point of the change is that you stop editing it. Do **not** fix it in this unit. File a follow-up for making `Bridged.java:200-226` supplier-based, and in this unit **correct the comments that claim it is hot**, including `bridged.yaml:116` and the `Leader` javadoc at `:465-466` that still calls `terminal` "the only field identity depends on". 2. **`primary.terminal` is a different, already-deprecated key.** It seeds `PrimaryRegistry` (`Bridged.java:315-325`) for nudge addressing, not identity, and CB-532's `leadByTarget` map is what retires it. Leave it alone here. ## Acceptance criteria 1. A lead is resolved to `Role.PRIMARY` purely from its tab label; no terminal id appears in `fleet.leaders` config. 2. `terminal:` under `fleet.leaders.<name>` fails the config load with a message naming `tab:`. It is **not** silently ignored — prove this with a test, given `@JsonIgnoreProperties(ignoreUnknown)`. 3. Two leaders with different `tab:` values are both discovered. (Today only the first leader's prefix is scanned.) 4. A tab label that matches no configured `tab:` yields no lead — a pane cannot name itself into the role. 5. A lead whose session restarts in the same tab is resolved under the same name, with no config edit and no daemon restart. 6. No permanent entry: when the tab is gone, the lead is gone from the map on the next scan. The "config pin outranks and never expires" merge at `LeadTabScanner:154` is removed. 7. A failed herdr scan still keeps the last good map (existing behaviour at `:119-122` — do not regress it). 8. `LeadLauncher` labels a tab it creates with exactly the `tab:` value, and finds it again. 9. Comments claiming `fleet.leaders` is hot are corrected, and the deferred-key list names it. ## Note on the silent-default trap — read before writing code This repo has produced the same bug six times: a new dependency gets a default so existing wiring compiles, and the capability ships turned off (CB-561, CB-572, CB-573 twice, `TestTurnTokens`, and the rejected `3b2f395`). `tab` must be **required** where a lead is not creatable. Do not add a `Leader` constructor overload that defaults `tab` to `null` so existing tests compile — the back-compat 7-arg ctor at `config/BridgedConfig.java:499` is exactly the shape to avoid growing. Update the call sites instead. An overload that defaults the new field is an automatic rejection at review, because it also means no test covers the new path.
ltms closed this issue 2026-08-15 09:29:01 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#51
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?