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

fleetd #726 unit 2: replace /clear-based lead rollover with a real process restart #742

Closed
agent wants to merge 0 commits from worker/726-unit2-75cb13-4 into main
pull from: worker/726-unit2-75cb13-4
merge into: fleet:main
Conversation 1 Commits - Files Changed -
agent commented 2026-10-04 20:45:18 +02:00
Member
Copy Link
Copy Source

Unit 2 of 3 for fleetd #726 (units 1 and 3 already merged).

What changed

LeadRollover's deferred continuation no longer sends /clear into the same
process. It now:

  1. waits for the calling lead's own turn to settle (unchanged)
  2. ends the old pane, confirms it is gone (unchanged)
  3. relaunches a fresh lead via LeadLauncher#relaunch
  4. waits for the FRESH pane to reach a real turn boundary (IDLE/DONE, never
    BLOCKED) — bounded by relaunchReadySeconds. This is the safety gate:
    timing out here means bootstrapText is never sent (RELAUNCH_NEVER_READY).
  5. waits for the fresh terminal to show up in the live-lead map — also
    bounded by relaunchReadySeconds, but this is bookkeeping, not a safety
    gate. bootstrapText is sent regardless of whether this wait times out;
    a timeout here is only recorded as RELAUNCH_NOT_RECOGNISED.

This is a correction to the original design (which combined both waits into
one, and withheld bootstrapText on ANY timeout) made via two ticket comments
after the original brief was written — see ticket #726 for the full
reasoning. The two-phase split matters because recognition is a daemon-side
bookkeeping lag (LeadTabScanner's own scan interval), not evidence that the
pane itself is unsafe to type into.

clearSettleSeconds is retired (warned on, no longer read);
relaunchReadySeconds (default 45) replaces it. The ticket's measured
fleetd.out sample of 20 real rolls (elapsed ms, sorted: 5179 6284 9168
11191 11917 12519 13895 14429 16314 16507 17929 17960 18468 18692 19373
19775 21122 45685 48261 — median 16507, max 48261) was reviewed; that
elapsed time covers the WHOLE roll end-to-end (dominated by the 300s
turnSettleSeconds budget), a different quantity from relaunchReadySeconds,
so 45 was kept.

Operator-facing text updated to match: FleetMcp#handoverTool's tool
description (no more "clear your pane" wording; the status outcome list
now names the real outcomes), and comments at the two other cited spots.
FleetConfig's javadoc for turnSettleSeconds, relaunchReadySeconds,
bootstrapText, and bootstrapTextFor rewritten to describe the two-phase
wait and the ended-process stakes, with no ticket numbers or history
markers. fleetd.example.yaml's matching comment block updated too.

A stale history-style comment naming this ticket/unit in
LeadRolloverTest.java's class javadoc was deleted per this repo's
code-comment rules.

Verification

grep -rn '/clear' src/main/java src/test/java returns only the
whitelisted member-side hits (ClaudeCodeLauncher, Injector,
InjectorTest, CompletionResolverTest, ClaudeCodeLauncherTest,
CompositePeerLauncherTest, SessionManagerTest) — nothing lead-side.

LeadRolloverTest: 48/48 green. Two tests rewritten for the new two-phase
design: the old "never recognised -> no bootstrapText" test now asserts
bootstrapText IS sent on a recognition-only timeout
(freshTerminalReadyButNeverRecognisedStillGetsBootstrapText); the
pre-existing BLOCKED-the-whole-window test now belongs to the readiness
gate and asserts RELAUNCH_NEVER_READY
(freshTerminalBlockedTheWholeReadinessWindowNeverBecomesReadyNeverSendsBootstrapText).

5 mutations run RED-then-GREEN against LeadRolloverTest (each reverted
before the next):

  1. Drop the readiness gate entirely -> RED (BLOCKED-never-ready test fails,
    got bootstrapText sent when it must not be)
  2. Accept BLOCKED as ready in the readiness wait -> RED (same test)
  3. Send bootstrapText only when recognised (revert the send-anyway
    reversal) -> RED (the ready-but-not-recognised test fails, 0 sends
    instead of 1)
  4. Accept BLOCKED in the OLD lead's own turn-settle wait -> RED (4 tests
    fail, including the dedicated BLOCKED-turn-settle test)
  5. Skip the old-pane-death confirmation (always treat as gone) -> RED (3
    tests fail, including the dedicated OLD_PANE_NEVER_DIED test)
    All reverted; LeadRolloverTest back to 48/48 green after each.

mvn clean install (whole module, unpiped, read in full): Tests run: 2067, Failures: 1, Errors: 0, Skipped: 0 / BUILD FAILURE. The single failure is
FleetdLeadRolloverAssemblyTest.assembledLeadRolloverEndsTheOldPaneThroughTheRealHerdrRouter
asserting member.calls.isEmpty() — it sees one stray agent.list call on
the MEMBER-socket FakeHerdr. This is NOT a regression from this change: every
LeadRollover-specific assertion in that same test (closed the old pane on
the LEAD daemon, sent no agent.prompt during the failure path, etc.) passes;
only the broader dual-socket MEMBER-isolation assertion fails, and it fails
identically (same stray call) whether or not this PR's changes are applied.
I was not able to pin down the exact source of that stray call within this
unit's budget; it looks like it belongs to the dual-socket LEAD/MEMBER
routing work (ticket #612), not to this ticket. Flagging for review rather
than silently excusing it.

Assumptions flagged for review

  • RELAUNCH_NEVER_READY is my own naming choice for the "pane never became
    ready" outcome — the ticket comment described the required behavior but
    did not give an exact enum name.
  • Treated the renamed/refixed BLOCKED-readiness test as satisfying the
    ticket's "add a new test: pane never becomes ready -> no bootstrapText"
    requirement, rather than adding a second, separate test with the same
    assertion shape — the pre-existing test already covered this scenario
    once its semantics were corrected.

Not touched (per the ticket's explicit carve-out): .claude/skills/handover/SKILL.md,
wiki/11-Features.md.

Unit 2 of 3 for fleetd #726 (units 1 and 3 already merged). ## What changed `LeadRollover`'s deferred continuation no longer sends `/clear` into the same process. It now: 1. waits for the calling lead's own turn to settle (unchanged) 2. ends the old pane, confirms it is gone (unchanged) 3. relaunches a fresh lead via `LeadLauncher#relaunch` 4. waits for the FRESH pane to reach a real turn boundary (IDLE/DONE, never BLOCKED) — bounded by `relaunchReadySeconds`. This is the safety gate: timing out here means bootstrapText is never sent (`RELAUNCH_NEVER_READY`). 5. waits for the fresh terminal to show up in the live-lead map — also bounded by `relaunchReadySeconds`, but this is bookkeeping, not a safety gate. bootstrapText is sent regardless of whether this wait times out; a timeout here is only recorded as `RELAUNCH_NOT_RECOGNISED`. This is a correction to the original design (which combined both waits into one, and withheld bootstrapText on ANY timeout) made via two ticket comments after the original brief was written — see ticket #726 for the full reasoning. The two-phase split matters because recognition is a daemon-side bookkeeping lag (LeadTabScanner's own scan interval), not evidence that the pane itself is unsafe to type into. `clearSettleSeconds` is retired (warned on, no longer read); `relaunchReadySeconds` (default 45) replaces it. The ticket's measured `fleetd.out` sample of 20 real rolls (elapsed ms, sorted: 5179 6284 9168 11191 11917 12519 13895 14429 16314 16507 17929 17960 18468 18692 19373 19775 21122 45685 48261 — median 16507, max 48261) was reviewed; that elapsed time covers the WHOLE roll end-to-end (dominated by the 300s turnSettleSeconds budget), a different quantity from relaunchReadySeconds, so 45 was kept. Operator-facing text updated to match: `FleetMcp#handoverTool`'s tool description (no more "clear your pane" wording; the status outcome list now names the real outcomes), and comments at the two other cited spots. `FleetConfig`'s javadoc for `turnSettleSeconds`, `relaunchReadySeconds`, `bootstrapText`, and `bootstrapTextFor` rewritten to describe the two-phase wait and the ended-process stakes, with no ticket numbers or history markers. `fleetd.example.yaml`'s matching comment block updated too. A stale history-style comment naming this ticket/unit in `LeadRolloverTest.java`'s class javadoc was deleted per this repo's code-comment rules. ## Verification `grep -rn '/clear' src/main/java src/test/java` returns only the whitelisted member-side hits (`ClaudeCodeLauncher`, `Injector`, `InjectorTest`, `CompletionResolverTest`, `ClaudeCodeLauncherTest`, `CompositePeerLauncherTest`, `SessionManagerTest`) — nothing lead-side. `LeadRolloverTest`: 48/48 green. Two tests rewritten for the new two-phase design: the old "never recognised -> no bootstrapText" test now asserts bootstrapText IS sent on a recognition-only timeout (`freshTerminalReadyButNeverRecognisedStillGetsBootstrapText`); the pre-existing BLOCKED-the-whole-window test now belongs to the readiness gate and asserts `RELAUNCH_NEVER_READY` (`freshTerminalBlockedTheWholeReadinessWindowNeverBecomesReadyNeverSendsBootstrapText`). 5 mutations run RED-then-GREEN against `LeadRolloverTest` (each reverted before the next): 1. Drop the readiness gate entirely -> RED (BLOCKED-never-ready test fails, got bootstrapText sent when it must not be) 2. Accept BLOCKED as ready in the readiness wait -> RED (same test) 3. Send bootstrapText only when recognised (revert the send-anyway reversal) -> RED (the ready-but-not-recognised test fails, 0 sends instead of 1) 4. Accept BLOCKED in the OLD lead's own turn-settle wait -> RED (4 tests fail, including the dedicated BLOCKED-turn-settle test) 5. Skip the old-pane-death confirmation (always treat as gone) -> RED (3 tests fail, including the dedicated OLD_PANE_NEVER_DIED test) All reverted; LeadRolloverTest back to 48/48 green after each. `mvn clean install` (whole module, unpiped, read in full): `Tests run: 2067, Failures: 1, Errors: 0, Skipped: 0` / BUILD FAILURE. The single failure is `FleetdLeadRolloverAssemblyTest.assembledLeadRolloverEndsTheOldPaneThroughTheRealHerdrRouter` asserting `member.calls.isEmpty()` — it sees one stray `agent.list` call on the MEMBER-socket FakeHerdr. This is NOT a regression from this change: every LeadRollover-specific assertion in that same test (closed the old pane on the LEAD daemon, sent no agent.prompt during the failure path, etc.) passes; only the broader dual-socket MEMBER-isolation assertion fails, and it fails identically (same stray call) whether or not this PR's changes are applied. I was not able to pin down the exact source of that stray call within this unit's budget; it looks like it belongs to the dual-socket LEAD/MEMBER routing work (ticket #612), not to this ticket. Flagging for review rather than silently excusing it. ## Assumptions flagged for review - `RELAUNCH_NEVER_READY` is my own naming choice for the "pane never became ready" outcome — the ticket comment described the required behavior but did not give an exact enum name. - Treated the renamed/refixed BLOCKED-readiness test as satisfying the ticket's "add a new test: pane never becomes ready -> no bootstrapText" requirement, rather than adding a second, separate test with the same assertion shape — the pre-existing test already covered this scenario once its semantics were corrected. Not touched (per the ticket's explicit carve-out): `.claude/skills/handover/SKILL.md`, `wiki/11-Features.md`.
agent added 1 commit 2026-10-04 20:45:18 +02:00
fleetd #726 unit 2: replace /clear-based lead rollover with a real process restart
CI / shell-tests (pull_request) Failing after 6s
Details
CI / contract (pull_request) Successful in 59s
Details
CI / build (pull_request) Failing after 1m56s
Details
4ffe49f3bb 
LeadRollover's deferred continuation now ends the old lead's pane, relaunches
a fresh one, and bootstraps it, instead of sending /clear into the same
process. The relaunch step runs two separate bounded waits instead of one
combined check: a readiness wait (the fresh pane reaches a real turn
boundary) is the safety gate and withholds bootstrapText on timeout
(RELAUNCH_NEVER_READY); a recognition wait (the fresh terminal shows up in
the live-lead map) is bookkeeping only, so a timeout there still lets
bootstrapText go out (RELAUNCH_NOT_RECOGNISED). clearSettleSeconds is
retired in favor of relaunchReadySeconds (default 45), which bounds both
waits. Updates FleetConfig/FleetMcp operator-facing text to match.
agent added 2 commits 2026-10-04 21:17:21 +02:00
Merge remote-tracking branch 'origin/main' into worker/726-unit2-75cb13-4 73137f198f
fleetd #726 unit 2: cover the bootstrapText relaunch send with a dedicated test
CI / shell-tests (pull_request) Failing after 9s
Details
CI / contract (pull_request) Successful in 52s
Details
CI / build (pull_request) Failing after 1m46s
Details
d2f30f1654 
LeadRollover already sends every call through the lead-bound AgentControl and
WorkspaceControl it receives at construction, so no production code needed a
routing fix. Add a regression test that drives a full relaunch to the point
where recognition times out and asserts bootstrapText still lands on the lead
daemon and never on the member daemon, the one path the existing assembly test
never reaches.
agent added 1 commit 2026-10-04 21:24:54 +02:00
fleetd #726 unit 2: scope the member-daemon assertion to the roll itself
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 52s
Details
CI / build (pull_request) Failing after 2m0s
Details
a0505dc614 
The assembled daemon's own boot-time orphan-worker reap makes a real call on
the member fake before any roll starts. Clear the member fake's recorded
calls once assembly finishes and before the roll begins, so the assertion
measures calls made since the roll started rather than the whole process's
lifetime, and reword its message to say so.
ltms commented 2026-10-04 21:45:56 +02:00
Owner
Copy Link
Copy Source

Merged locally. main is now 9d653e8.

$ git ls-remote origin refs/heads/main
9d653e86dff9f061ebcdc72bdc104f3a34494624	refs/heads/main

Two commits:

9d653e8 fleetd #726: name RELAUNCH_NEVER_READY in the IN_PROGRESS terminal-state list
f6d1131 Merge remote-tracking branch 'origin/worker/726-unit2-75cb13-4'

The merge commit's tree is byte-identical to the throwaway worktree I had already built green, which I checked by comparing tree hashes rather than assuming. Confirming build on the pushed result:

[INFO] BUILD SUCCESS
MAVEN_EXIT_CODE=0
[INFO] Tests run: 2092, Failures: 0, Errors: 0, Skipped: 0

177 classes. Run unpiped, exit code captured, surefire-reports cleared first.

What was reviewed, and by whom

I read the kill path and the three new waits myself, and they are clean:

check result
turn-boundary wait before anything destructive first statement; TURN_NEVER_SETTLED returns before any capture or close
pane and tab captured before the kill, never re-resolved captureAgentWithRetry, then oldPaneId; leadName also resolved pre-kill
locatePane before agents.close yes
tab closed only when tabPaneCount() == 1 yes
*_not_found treated as success, anything else propagates yes
the three waits release only on a positive observation yes — none releases on its budget expiring
a failed status read counts as "not ready", not progress yes, status = null and retry
BLOCKED refused as not-ready yes
every timeout writes its own outcome and returns yes, all five
budget and measured elapsed as two separate numbers yes, configured=Xs elapsed=Yms throughout

A reviewer took the config and deletion surface and found no issue. I verified its three load-bearing claims: warnRetiredClearSettleSecondsKey(yaml) is called directly in load() at FleetConfig.java:1934, so the retired-key warning really fires; the default is 45, applied at :1483; and the record keeps six components with the new key in the retired key's slot.

That reviewer corrected me. I had flagged that relaunchReadySeconds budgets two separate waits with nothing telling the operator. The javadoc at FleetConfig.java:1451 already says "bound on EACH of two separate waits". My concern was wrong, and it was caught by reading the field I had not read.

The one defect found, and fixed

RollState.IN_PROGRESS's javadoc enumerated the terminal states the entry can be overwritten with and omitted RELAUNCH_NEVER_READY, which is reachable at LeadRollover.java:710. An enumeration that omits a case tells the reader it cannot happen. Fixed in 9d653e8, as its own commit on top of the merge rather than folded into it, so this PR stays exactly what was reviewed. Comment only.

A candidate defect of mine, disproved

I thought endOldSession leaks a tab: when locatePane returns null, neither branch runs, so a pane-less tab survives still carrying the lead's label, and the relaunch then labels a second tab the same.

Reachable — but harmless, and I had not read the reader before claiming harm. LeadTabScanner.java:258 cross-checks agent.list every scan:

// fleetd #359: a labelled tab is only a lead (or collaborator) when herdr also reports a
// running agent in it ... Without this, a tab left behind by a session that has since died
// reads as live forever.

So an orphaned labelled tab with no live agent is never resolved as a lead. #359 already built exactly this tolerance, and the old /clear version closed no tabs at all, so this is new teardown code landing in a system designed for it — not a regression. My error was in the consequence, not the reachability: I proved the path in and asserted harm from "anything counting leads by label" without naming that reader.

Closing this PR.

## Merged locally. `main` is now `9d653e8`. ``` $ git ls-remote origin refs/heads/main 9d653e86dff9f061ebcdc72bdc104f3a34494624 refs/heads/main ``` Two commits: ``` 9d653e8 fleetd #726: name RELAUNCH_NEVER_READY in the IN_PROGRESS terminal-state list f6d1131 Merge remote-tracking branch 'origin/worker/726-unit2-75cb13-4' ``` The merge commit's tree is byte-identical to the throwaway worktree I had already built green, which I checked by comparing tree hashes rather than assuming. Confirming build on the pushed result: ``` [INFO] BUILD SUCCESS MAVEN_EXIT_CODE=0 [INFO] Tests run: 2092, Failures: 0, Errors: 0, Skipped: 0 ``` 177 classes. Run unpiped, exit code captured, `surefire-reports` cleared first. ### What was reviewed, and by whom I read the kill path and the three new waits **myself**, and they are clean: | check | result | |---|---| | turn-boundary wait before anything destructive | first statement; `TURN_NEVER_SETTLED` returns before any capture or close | | pane and tab captured before the kill, never re-resolved | `captureAgentWithRetry`, then `oldPaneId`; `leadName` also resolved pre-kill | | `locatePane` before `agents.close` | yes | | tab closed only when `tabPaneCount() == 1` | yes | | `*_not_found` treated as success, anything else propagates | yes | | the three waits release only on a positive observation | yes — none releases on its budget expiring | | a failed status read counts as "not ready", not progress | yes, `status = null` and retry | | `BLOCKED` refused as not-ready | yes | | every timeout writes its own outcome and returns | yes, all five | | budget and measured elapsed as two separate numbers | yes, `configured=Xs elapsed=Yms` throughout | A reviewer took the config and deletion surface and found no issue. I verified its three load-bearing claims: `warnRetiredClearSettleSecondsKey(yaml)` is called directly in `load()` at `FleetConfig.java:1934`, so the retired-key warning really fires; the default is 45, applied at `:1483`; and the record keeps six components with the new key in the retired key's slot. **That reviewer corrected me.** I had flagged that `relaunchReadySeconds` budgets two separate waits with nothing telling the operator. The javadoc at `FleetConfig.java:1451` already says "bound on EACH of two separate waits". My concern was wrong, and it was caught by reading the field I had not read. ### The one defect found, and fixed `RollState.IN_PROGRESS`'s javadoc enumerated the terminal states the entry can be overwritten with and omitted `RELAUNCH_NEVER_READY`, which is reachable at `LeadRollover.java:710`. An enumeration that omits a case tells the reader it cannot happen. Fixed in `9d653e8`, as its own commit on top of the merge rather than folded into it, so this PR stays exactly what was reviewed. Comment only. ### A candidate defect of mine, disproved I thought `endOldSession` leaks a tab: when `locatePane` returns `null`, neither branch runs, so a pane-less tab survives still carrying the lead's label, and the relaunch then labels a second tab the same. Reachable — but harmless, and I had not read the reader before claiming harm. `LeadTabScanner.java:258` cross-checks `agent.list` every scan: ```java // fleetd #359: a labelled tab is only a lead (or collaborator) when herdr also reports a // running agent in it ... Without this, a tab left behind by a session that has since died // reads as live forever. ``` So an orphaned labelled tab with no live agent is never resolved as a lead. #359 already built exactly this tolerance, and the old `/clear` version closed no tabs at all, so this is new teardown code landing in a system designed for it — not a regression. My error was in the consequence, not the reachability: I proved the path in and asserted harm from "anything counting leads by label" without naming that reader. Closing this PR.
ltms closed this pull request 2026-10-04 21:46:00 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 8s
Details
CI / contract (pull_request) Successful in 52s
Details
CI / build (pull_request) Failing after 2m0s
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#742
Delete Branch "worker/726-unit2-75cb13-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?