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

#359: stop dead lead tabs from accumulating and breaking lead coordination #367

Closed
agent wants to merge 0 commits from worker/359-dead-lead-tabs-f1253b-4 into main
pull from: worker/359-dead-lead-tabs-f1253b-4
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-09-05 08:21:18 +02:00
Member
Copy Link
Copy Source

fleetd #359 — dead lead tabs accumulate and make lead-to-lead coordination permanently undeliverable.

Root cause

LeadTabScanner.get() joined labelled tabs to panes with no liveness check, so a tab left behind by a crashed/relaunched lead was reported as a live lead forever. Once two tabs share a label, LeadCoordLoop.resolveLocalLead() correctly refuses to guess — but the daemon's own WARN then suggests naming a lead after coordinator.selfId, which stops being safe once duplicates exist.

Fix

  • LeadTabScanner.scan() now cross-checks every labelled tab against agent.list (the same signal LeadLauncher.countLeads trusts) and drops any tab with no agent running in it.
  • LeadLauncher.ensureLeads() closes a name's dead tabs in the same reconcile that decides whether to (re)launch, so restarts stop accumulating debris.

Review round 2 — two must-fix findings addressed

Finding 1 — the first version closed a dead tab on a single agent.list reading. The ticket's own live evidence showed that exact signal reporting "0 live" for a tab a plain ps confirmed was running a real session; closing on that reading would have destroyed the operator's actual lead. Fix: a dead reading now only flags the tab (PendingCloseMarker, a tab-label suffix); it is closed only if a later, independently-connected reconcile still finds it dead while flagged. A tab found live again has its flag cleared. Two alternatives were considered and rejected (see LeadLauncher's class javadoc): a second independent signal (nothing else herdr exposes proves liveness any better — a second call to the same source is not independent evidence), and closing all but the most recent dead tab (no reliable ordering across tab ids, and the real failure — a name never reconfirmed — would still grow unbounded).

Finding 2 — the new agent.list cross-check inside LeadTabScanner.scan() was not covered by get()'s "keep the cache on a failed scan" contract, which only fires on a thrown HerdrException. A successful-but-short agent.list could silently drop a lead CallerResolver had already resolved, demoting it to Role.WORKER, which refuses every orchestration call. Fix: a terminal already reported live gets one grace scan before being dropped; a terminal never reported live gets none, so the original exclusion invariant is unaffected.

Both mechanisms were mutation-tested: reverting either change turns exactly its own new tests red (2 for finding 1, 3 for finding 2) and nothing else.

Build

Merged origin/main first. mvn clean install, unpiped, from fleetd/: Tests run: 1412, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS.

Out of scope

PrimaryRegistry.leadByTarget (found during the required read-only sweep for the same defect shape) is a separate, real finding — not touched here; the lead is filing it separately.

fleetd #359 — dead lead tabs accumulate and make lead-to-lead coordination permanently undeliverable. ## Root cause `LeadTabScanner.get()` joined labelled tabs to panes with no liveness check, so a tab left behind by a crashed/relaunched lead was reported as a live lead forever. Once two tabs share a label, `LeadCoordLoop.resolveLocalLead()` correctly refuses to guess — but the daemon's own WARN then suggests naming a lead after `coordinator.selfId`, which stops being safe once duplicates exist. ## Fix - `LeadTabScanner.scan()` now cross-checks every labelled tab against `agent.list` (the same signal `LeadLauncher.countLeads` trusts) and drops any tab with no agent running in it. - `LeadLauncher.ensureLeads()` closes a name's dead tabs in the same reconcile that decides whether to (re)launch, so restarts stop accumulating debris. ## Review round 2 — two must-fix findings addressed **Finding 1** — the first version closed a dead tab on a single `agent.list` reading. The ticket's own live evidence showed that exact signal reporting "0 live" for a tab a plain `ps` confirmed was running a real session; closing on that reading would have destroyed the operator's actual lead. Fix: a dead reading now only *flags* the tab (`PendingCloseMarker`, a tab-label suffix); it is closed only if a **later**, independently-connected reconcile still finds it dead while flagged. A tab found live again has its flag cleared. Two alternatives were considered and rejected (see `LeadLauncher`'s class javadoc): a second independent signal (nothing else herdr exposes proves liveness any better — a second call to the same source is not independent evidence), and closing all but the most recent dead tab (no reliable ordering across tab ids, and the real failure — a name never reconfirmed — would still grow unbounded). **Finding 2** — the new `agent.list` cross-check inside `LeadTabScanner.scan()` was not covered by `get()`'s "keep the cache on a failed scan" contract, which only fires on a thrown `HerdrException`. A successful-but-short `agent.list` could silently drop a lead `CallerResolver` had already resolved, demoting it to `Role.WORKER`, which refuses every orchestration call. Fix: a terminal already reported live gets one grace scan before being dropped; a terminal never reported live gets none, so the original exclusion invariant is unaffected. Both mechanisms were mutation-tested: reverting either change turns exactly its own new tests red (2 for finding 1, 3 for finding 2) and nothing else. ## Build Merged `origin/main` first. `mvn clean install`, unpiped, from `fleetd/`: **Tests run: 1412, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS**. ## Out of scope `PrimaryRegistry.leadByTarget` (found during the required read-only sweep for the same defect shape) is a separate, real finding — not touched here; the lead is filing it separately.
agent added 1 commit 2026-09-05 08:21:18 +02:00
#359: LeadTabScanner drops dead lead tabs; LeadLauncher closes them on relaunch
CI / contract (pull_request) Successful in 58s
Details
CI / build (pull_request) Successful in 1m49s
Details
395b3b5c46 
LeadTabScanner.scan() joined labelled tabs to panes with no liveness check at
all, so a tab left behind by a crashed/relaunched lead read as a live lead
forever -- exactly the hazard its own javadoc predicted but excused. It now
cross-checks agent.list, the same signal LeadLauncher already trusted, and
drops any labelled tab with no agent running in it. That alone removes the
duplicate candidates LeadCoordLoop.resolveLocalLead() could pick from,
including the dead one its own WARN's advice (name a lead after
coordinator.selfId) could land a message in.

LeadLauncher never actually stopped the accumulation: relaunching on "0 live"
always created a brand new tab and left the old dead-labelled one right where
it was, so any restart that found 0 live for any reason (a real crash, or a
herdr read that missed a still-running agent) added one more dead tab,
forever. ensureLeads() now closes every dead-labelled tab for a lead as part
of the same reconcile that decides to relaunch, so at most one tab survives
per configured lead once a restart's reconcile has run.
agent added 2 commits 2026-09-05 10:55:03 +02:00
Merge remote-tracking branch 'origin/main' into worker/359-dead-lead-tabs-f1253b-4 a5d81c3f70
#359 review: require two independent readings before destroying a lead's tab
CI / contract (pull_request) Successful in 45s
Details
CI / build (pull_request) Successful in 1m33s
Details
96c406b968 
Finding 1 (LeadLauncher): closing a labelled tab on a single agent.list
miss could destroy a live lead's session — the ticket's own evidence
showed that exact signal missing a genuinely running agent. A dead
reading now only flags the tab (PendingCloseMarker); it is closed only
if a later, independently-connected reconcile still finds it dead
while flagged. A tab found live again has its flag cleared instead.

Finding 2 (LeadTabScanner): the new agent.list cross-check in scan()
was not covered by get()'s "keep the cache on a failed scan" contract,
which only fires on a thrown HerdrException. A successful-but-short
agent.list could silently drop a lead CallerResolver had already
resolved, demoting it to Role.WORKER. A terminal already reported live
now gets one grace scan before being dropped; a terminal never
reported live gets none, so the original #359 exclusion is unaffected.

Both mechanisms were mutation-tested: reverting either change turns
exactly its own new tests red and nothing else.
ltms commented 2026-09-06 07:10:26 +02:00
Owner
Copy Link
Copy Source

Merged locally as 6c61355 and pushed to main.

Verified on the merge itself, not taken from the worker's report:

mvn clean install -> Tests run: 1412, Failures: 0, Errors: 0, Skipped: 0
                     BUILD SUCCESS

Mutation run on merge, on a half the worker did not touch: made
PendingCloseMarker.strip() the identity function, so a flagged tab stops
matching its configured label. Result: 4 failures, BUILD FAILURE
(LeadTabScannerTest 1, LeadLauncherTest 3). The cross-class invariant is
pinned. Note that the scanner-side catch is incidental — it comes from
matchingIsCaseInsensitiveAndToleratesSurroundingWhitespace, not from a test
written for flagged tabs.

Accepted caveat, recorded in the merge message: ensureLeads() runs at startup
only, so the second reading arrives at the next restart. A tab whose agent dies
mid-session stays flagged and open until then. That is the right trade — the bug
is about repeated restarts, and one leftover tab is far cheaper than closing a
live lead on evidence that has already been seen to lie (fleet01's agent.list
said 0 live while ps showed one real claude).

Wiki entry added: 11-Features.md → "Dead lead tabs are cleaned up, and a live
one is never closed" (wiki 707d2a2).

Merged locally as `6c61355` and pushed to `main`. Verified on the merge itself, not taken from the worker's report: ``` mvn clean install -> Tests run: 1412, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` Mutation run on merge, on a half the worker did not touch: made `PendingCloseMarker.strip()` the identity function, so a flagged tab stops matching its configured label. Result: 4 failures, BUILD FAILURE (`LeadTabScannerTest` 1, `LeadLauncherTest` 3). The cross-class invariant is pinned. Note that the scanner-side catch is incidental — it comes from `matchingIsCaseInsensitiveAndToleratesSurroundingWhitespace`, not from a test written for flagged tabs. Accepted caveat, recorded in the merge message: `ensureLeads()` runs at startup only, so the second reading arrives at the next restart. A tab whose agent dies mid-session stays flagged and open until then. That is the right trade — the bug is about repeated restarts, and one leftover tab is far cheaper than closing a live lead on evidence that has already been seen to lie (fleet01's `agent.list` said 0 live while `ps` showed one real `claude`). Wiki entry added: `11-Features.md` → "Dead lead tabs are cleaned up, and a live one is never closed" (wiki `707d2a2`).
ltms closed this pull request 2026-09-06 07:10:30 +02:00
Some checks are pending
CI / contract (pull_request) Successful in 45s
Details
CI / build (pull_request) Successful in 1m33s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
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
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#367
Delete Branch "worker/359-dead-lead-tabs-f1253b-4"

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?