fleetd #726 unit 1: give LeadLauncher a public single-lead relaunch seam #731

Closed
agent wants to merge 0 commits from worker/726-10cbf0-1 into main
Member

Adds LeadLauncher.relaunch(String name), which starts exactly the named lead from the live config, outside of ensureLeads()'s instances bookkeeping. It resolves the Leader/Profile the same way ensureLeads() does, with the same three refusals (unknown name, recognise-only lead, unconfigured profile), and retries the whole launch attempt up to RELAUNCH_ATTEMPTS (3) times -- covering a failed ensureWorkspace, a failed createTab, a tab with no seed pane, or a herdr blip, none of which ResilientAgentLaunch's internal agent_name_taken/agent_pane_busy retries cover.

launch() now returns the started Agent (null on failure) instead of a boolean, so relaunch() and ensureLeads() share the same primitive. ensureLeads()'s own behaviour is unchanged.

This is unit 1 of 3 for #726 (a lead handover ending the old claude process and starting a fresh one so a CLI update is picked up). It does not touch LeadRollover, Fleetd, or FleetdAssembly -- those are units 2 and 3.

Tests added (LeadLauncherTest): relaunch returns the started agent with its real terminalId/paneId; the new tab is labelled after the start, not before; unknown lead name / tab-only lead / unconfigured profile each return null and start nothing; a launch failing on attempts 1-2 and succeeding on 3 returns the agent (3 tabs created, 2 closed); a launch failing every attempt returns null after exactly RELAUNCH_ATTEMPTS, with every created tab closed (no leaks).

Build: mvn clean install -- BUILD SUCCESS, Tests run: 2061, Failures: 0, Errors: 0, Skipped: 0 (LeadLauncherTest: 37, Failures: 0).

Mutation testing (each applied, run, shown RED, then reverted and shown GREEN):

  • relaunch returns null instead of the started agent -> killed relaunchReturnsTheStartedAgent, relaunchLabelsTheNewTabAfterStarting, relaunchRetriesTheWholeAttemptAndSucceedsOnTheThird
  • outer retry loop bounded to 1 instead of RELAUNCH_ATTEMPTS -> killed relaunchRetriesTheWholeAttemptAndSucceedsOnTheThird, relaunchGivesUpAfterExactlyRelaunchAttemptsAndLeaksNoTab
  • orphan-tab close removed from the catch -> killed relaunchGivesUpAfterExactlyRelaunchAttemptsAndLeaksNoTab, relaunchRetriesTheWholeAttemptAndSucceedsOnTheThird
  • tab rename moved before the start -> killed relaunchLabelsTheNewTabAfterStarting
Adds `LeadLauncher.relaunch(String name)`, which starts exactly the named lead from the live config, outside of `ensureLeads()`'s `instances` bookkeeping. It resolves the `Leader`/`Profile` the same way `ensureLeads()` does, with the same three refusals (unknown name, recognise-only lead, unconfigured profile), and retries the whole launch attempt up to `RELAUNCH_ATTEMPTS` (3) times -- covering a failed `ensureWorkspace`, a failed `createTab`, a tab with no seed pane, or a herdr blip, none of which `ResilientAgentLaunch`'s internal `agent_name_taken`/`agent_pane_busy` retries cover. `launch()` now returns the started `Agent` (`null` on failure) instead of a `boolean`, so `relaunch()` and `ensureLeads()` share the same primitive. `ensureLeads()`'s own behaviour is unchanged. This is unit 1 of 3 for #726 (a lead handover ending the old `claude` process and starting a fresh one so a CLI update is picked up). It does not touch `LeadRollover`, `Fleetd`, or `FleetdAssembly` -- those are units 2 and 3. **Tests added** (`LeadLauncherTest`): relaunch returns the started agent with its real terminalId/paneId; the new tab is labelled after the start, not before; unknown lead name / tab-only lead / unconfigured profile each return null and start nothing; a launch failing on attempts 1-2 and succeeding on 3 returns the agent (3 tabs created, 2 closed); a launch failing every attempt returns null after exactly RELAUNCH_ATTEMPTS, with every created tab closed (no leaks). **Build**: `mvn clean install` -- BUILD SUCCESS, Tests run: 2061, Failures: 0, Errors: 0, Skipped: 0 (LeadLauncherTest: 37, Failures: 0). **Mutation testing** (each applied, run, shown RED, then reverted and shown GREEN): - `relaunch` returns `null` instead of the started agent -> killed `relaunchReturnsTheStartedAgent`, `relaunchLabelsTheNewTabAfterStarting`, `relaunchRetriesTheWholeAttemptAndSucceedsOnTheThird` - outer retry loop bounded to 1 instead of `RELAUNCH_ATTEMPTS` -> killed `relaunchRetriesTheWholeAttemptAndSucceedsOnTheThird`, `relaunchGivesUpAfterExactlyRelaunchAttemptsAndLeaksNoTab` - orphan-tab close removed from the catch -> killed `relaunchGivesUpAfterExactlyRelaunchAttemptsAndLeaksNoTab`, `relaunchRetriesTheWholeAttemptAndSucceedsOnTheThird` - tab rename moved before the start -> killed `relaunchLabelsTheNewTabAfterStarting`
agent added 1 commit 2026-10-04 18:53:04 +02:00
fleetd #726 unit 1: give LeadLauncher a public single-lead relaunch seam
CI / shell-tests (pull_request) Failing after 8s
CI / contract (pull_request) Successful in 45s
CI / build (pull_request) Failing after 1m48s
38544d467c
Adds LeadLauncher.relaunch(name), which starts exactly the named lead
from the live config, outside of ensureLeads()'s instances bookkeeping.
It retries the whole launch attempt (not just the agent_name_taken/
agent_pane_busy cases ResilientAgentLaunch already retries inside one
agents.start call) up to RELAUNCH_ATTEMPTS times.

launch() now returns the started Agent (null on failure) instead of a
boolean, so relaunch() and ensureLeads() share the same primitive.
Owner

Review — I verified the build myself, and found three things to fix

My own reading of the pushed tree, not the worker's. Throwaway worktree on
origin/worker/726-10cbf0-1, tree hash 45ee69015857760c16b67ff53554ac554254a461 confirmed equal to
the branch's, then mvn clean install unpiped:

BUILD SUCCESS
Tests run: 2061, Failures: 0, Errors: 0, Skipped: 0     (summed over 177 surefire class reports)
dev.ltms.fleet.lead.LeadLauncherTest — Tests run: 37, Failures: 0, Errors: 0

That matches the report exactly. The mutation evidence is real and the tests are good: they assert the
rename happens after the start by comparing call indices, they count tabs created against tabs closed,
and the retry test drives two genuine failures by exhausting ResilientAgentLaunch.NAME_RETRIES twice
rather than by faking a counter. Three fixes before merge.

1. The javadoc claims a liveness the code does not have — must fix

/**
 * Start the named lead from the live config, ...
 */
public Agent relaunch(String name) {
    FleetConfig.Leader lead = cfg.fleet().leaders().get(name);

cfg is a FleetConfig snapshot stored at construction (LeadLauncher.java:112-117), and
production builds the launcher once, at boot (FleetdAssembly.java:304). It is not live, and under
unit 2 this object will be long-lived and called hours later.

The behaviour is correct — ConfigRef's own class doc says so: "fleet.leaders inside the same key
is frozen, which is exactly what makes fleet: split rather than hot"
. So a snapshot is the right
thing to read. It is the sentence that is wrong, and it is the kind of wrong that costs: unit 2's
implementer reading "live config" would reasonably assume a hot reload is picked up and design around
a guarantee that does not exist.

Say what it actually reads — the config snapshot this launcher was constructed with — and that
fleet.leaders is frozen anyway, so a restart is required to change a lead's profile. State the
contract, not the history.

2. A copied log line that is false in its new context — must fix

relaunch reuses ensureLeads()'s wording verbatim:

log.info("lead '{}' is not live, and names no profile — it can be recognised but not "
        + "launched. ...", name, name);

In ensureLeads() the "is not live" clause is true: that branch is only reached after the live count
came back short. In relaunch nothing has counted anything — the method deliberately does not, and
its own javadoc says so. The lead may well be live at that moment. A log line that asserts something
the method never checked is worse than no log line, because it will be read as evidence during an
incident.

Drop the liveness clause here. Keep the actionable half ("names no profile: … add profile: under
fleet.leaders.<name>").

3. The resolution is duplicated, log text and all — should fix

relaunch re-implements the same three-branch resolve that ensureLeads() already does
(not-creatable, unconfigured profile, and now unknown name), and copies two log messages word for
word. CLAUDE.md names this directly: "One fact, one place … Copies drift." Finding 2 is that drift
already happening on the first copy — the sentence was true where it was written and false where it
was pasted.

Pull the resolve into one private helper that returns the resolved lead and profile or null, having
logged, and call it from both. ensureLeads() keeps its own liveness-count logic; only the
declared/creatable/profile-configured resolution is shared.

Not blocking, but worth knowing

confirm()-style hole, in this unit's neighbour rather than this diff: if continuationRunner.accept
itself throws, runRollover never runs. Under unit 3 that also leaks the single-flight claim and the
terminal becomes permanently unrollable with no way to clear it. I have raised it on unit 3's PR; it
is pre-existing in kind (fleetd #615 covered throws inside the continuation, not a failure to start
it) and is not unit 1's problem.

What I am not asking for

No re-run of the four mutations — they are sound and I am not asking you to redo evidence. Fix the
three items, re-run mvn clean install, and report the test count. Add a mutation only for anything
you change in behaviour; items 1 and 2 are text, and item 3 must be behaviour-preserving, which the
existing 37 tests are the check for.

## Review — I verified the build myself, and found three things to fix **My own reading of the pushed tree**, not the worker's. Throwaway worktree on `origin/worker/726-10cbf0-1`, tree hash `45ee69015857760c16b67ff53554ac554254a461` confirmed equal to the branch's, then `mvn clean install` unpiped: ``` BUILD SUCCESS Tests run: 2061, Failures: 0, Errors: 0, Skipped: 0 (summed over 177 surefire class reports) dev.ltms.fleet.lead.LeadLauncherTest — Tests run: 37, Failures: 0, Errors: 0 ``` That matches the report exactly. The mutation evidence is real and the tests are good: they assert the rename happens after the start by comparing call indices, they count tabs created against tabs closed, and the retry test drives two genuine failures by exhausting `ResilientAgentLaunch.NAME_RETRIES` twice rather than by faking a counter. Three fixes before merge. ### 1. The javadoc claims a liveness the code does not have — must fix ```java /** * Start the named lead from the live config, ... */ public Agent relaunch(String name) { FleetConfig.Leader lead = cfg.fleet().leaders().get(name); ``` `cfg` is a `FleetConfig` **snapshot** stored at construction (`LeadLauncher.java:112-117`), and production builds the launcher once, at boot (`FleetdAssembly.java:304`). It is not live, and under unit 2 this object will be long-lived and called hours later. The behaviour is correct — `ConfigRef`'s own class doc says so: *"`fleet.leaders` inside the same key is frozen, which is exactly what makes `fleet:` split rather than hot"*. So a snapshot is the right thing to read. **It is the sentence that is wrong**, and it is the kind of wrong that costs: unit 2's implementer reading "live config" would reasonably assume a hot reload is picked up and design around a guarantee that does not exist. Say what it actually reads — the config snapshot this launcher was constructed with — and that `fleet.leaders` is frozen anyway, so a restart is required to change a lead's profile. State the contract, not the history. ### 2. A copied log line that is false in its new context — must fix `relaunch` reuses `ensureLeads()`'s wording verbatim: ```java log.info("lead '{}' is not live, and names no profile — it can be recognised but not " + "launched. ...", name, name); ``` In `ensureLeads()` the "is not live" clause is true: that branch is only reached after the live count came back short. In `relaunch` nothing has counted anything — the method deliberately does not, and its own javadoc says so. The lead may well be live at that moment. A log line that asserts something the method never checked is worse than no log line, because it will be read as evidence during an incident. Drop the liveness clause here. Keep the actionable half ("names no `profile:` … add `profile:` under `fleet.leaders.<name>`"). ### 3. The resolution is duplicated, log text and all — should fix `relaunch` re-implements the same three-branch resolve that `ensureLeads()` already does (not-creatable, unconfigured profile, and now unknown name), and copies two log messages word for word. `CLAUDE.md` names this directly: *"One fact, one place … Copies drift."* Finding 2 is that drift already happening on the first copy — the sentence was true where it was written and false where it was pasted. Pull the resolve into one private helper that returns the resolved lead and profile or null, having logged, and call it from both. `ensureLeads()` keeps its own liveness-count logic; only the declared/creatable/profile-configured resolution is shared. ### Not blocking, but worth knowing `confirm()`-style hole, in this unit's neighbour rather than this diff: if `continuationRunner.accept` itself throws, `runRollover` never runs. Under unit 3 that also leaks the single-flight claim and the terminal becomes permanently unrollable with no way to clear it. I have raised it on unit 3's PR; it is pre-existing in kind (fleetd #615 covered throws *inside* the continuation, not a failure to start it) and is not unit 1's problem. ### What I am not asking for No re-run of the four mutations — they are sound and I am not asking you to redo evidence. Fix the three items, re-run `mvn clean install`, and report the test count. Add a mutation only for anything you change in behaviour; items 1 and 2 are text, and item 3 must be behaviour-preserving, which the existing 37 tests are the check for.
agent added 1 commit 2026-10-04 19:00:02 +02:00
fleetd #726 unit 1 review fixes: correct relaunch's javadoc and dedupe its resolve logic
CI / shell-tests (pull_request) Failing after 10s
CI / contract (pull_request) Successful in 57s
CI / build (pull_request) Failing after 1m49s
b3b2bf3da6
relaunch's javadoc said it reads the live config; it actually reads the
FleetConfig snapshot this launcher was constructed with (fleet.leaders
is the frozen half), so say that and note a profile/tab edit needs a
daemon restart.

relaunch's recognise-only refusal reused ensureLeads()'s log wording,
which claims the lead 'is not live' — true in ensureLeads()'s context
(reached only after a short live count), false in relaunch's (which
never counts liveness, by design). Dropped that clause.

Pulled the declared/creatable/profile-configured resolution shared by
ensureLeads() and relaunch() into one private resolveLaunchable(name)
helper (returns a new ResolvedLead(lead, profile) record, or null
having logged), so the three refusals and their wording live in one
place instead of two copies that can drift. Behaviour-preserving:
ensureLeads() keeps its own liveness-count logic around the shared
resolve, and the existing 37 LeadLauncherTest cases are unchanged and
still pass.
Owner

Merged locally as 7b3beaa, pushed to main. Closing by hand — we merge locally, so Gitea does not close it itself.

All three review points are addressed, and I read the diff rather than the reply.

  1. Javadoc. The "live config" claim is gone. The new wording names the snapshot and the consequence (a profile: or tab: edit needs a daemon restart). That matches cfg being stored at construction.
  2. Log line. The recognise-only refusal no longer claims liveness. Your point that the dropped clause was true in ensureLeads()'s context is right, and dropping it there costs nothing — the actionable half is unchanged.
  3. Dedupe. resolveLaunchable(String) now holds all three refusals, and both former copies are gone. I checked the refactor did not change the order of the checks in ensureLeads(): it is still count → running >= wanted → resolve. The new lead == null branch inside the helper is unreachable from ensureLeads(), which iterates the map, and that is fine — it is reachable from relaunch, which is what it is for.

My own verification, not yours. I merged origin/main into your branch in a throwaway worktree and built the merge result, because main moved under you when #729 landed. The merge tree is f9aa883, byte-identical to the tree I then pushed, so the run below is this merge, not an approximation.

tests 2064  failures 0  errors 0  skipped 0
LeadLauncherTest tests 37  failures 0  errors 0
target/fleetd.jar present

2064 rather than your 2061, and that difference is expected: your branch is based on main before #729, which added 3 tests. Both numbers come out of the same base of 2054 plus 7 new tests here.

One check I could not run. ide_diagnostics is unavailable — the fleetd project is not currently open in IntelliJ, so the IDE answered project_not_found. I am reporting the Maven build only, and I have not run the IDE inspections on this file.

Unit 2 goes on this base once unit 3 lands.

Merged locally as `7b3beaa`, pushed to `main`. Closing by hand — we merge locally, so Gitea does not close it itself. All three review points are addressed, and I read the diff rather than the reply. 1. **Javadoc.** The "live config" claim is gone. The new wording names the snapshot and the consequence (a `profile:` or `tab:` edit needs a daemon restart). That matches `cfg` being stored at construction. 2. **Log line.** The recognise-only refusal no longer claims liveness. Your point that the dropped clause was true in `ensureLeads()`'s context is right, and dropping it there costs nothing — the actionable half is unchanged. 3. **Dedupe.** `resolveLaunchable(String)` now holds all three refusals, and both former copies are gone. I checked the refactor did not change the order of the checks in `ensureLeads()`: it is still count → `running >= wanted` → resolve. The new `lead == null` branch inside the helper is unreachable from `ensureLeads()`, which iterates the map, and that is fine — it is reachable from `relaunch`, which is what it is for. **My own verification, not yours.** I merged `origin/main` into your branch in a throwaway worktree and built the *merge result*, because `main` moved under you when #729 landed. The merge tree is `f9aa883`, byte-identical to the tree I then pushed, so the run below is this merge, not an approximation. ``` tests 2064 failures 0 errors 0 skipped 0 LeadLauncherTest tests 37 failures 0 errors 0 target/fleetd.jar present ``` 2064 rather than your 2061, and that difference is expected: your branch is based on `main` before #729, which added 3 tests. Both numbers come out of the same base of 2054 plus 7 new tests here. **One check I could not run.** `ide_diagnostics` is unavailable — the `fleetd` project is not currently open in IntelliJ, so the IDE answered `project_not_found`. I am reporting the Maven build only, and I have not run the IDE inspections on this file. Unit 2 goes on this base once unit 3 lands.
ltms closed this pull request 2026-10-04 19:07:22 +02:00
Some checks are pending
CI / shell-tests (pull_request) Failing after 10s
CI / contract (pull_request) Successful in 57s
CI / build (pull_request) Failing after 1m49s

Pull request closed

Sign in to join this conversation.