Fleetd.java: sweep every constructor-arg wiring site and report which ones no test would notice being rewired #587

Open
opened 2026-09-12 15:57:01 +02:00 by ltms · 2 comments
Owner

Read-only sweep. Change nothing. Follow-up to #562, where a merged PR shipped five green tests
that proved nothing about Fleetd.java's own wiring, and to a finding from the fleet01 lead.

Why this exists

Fleetd.java builds the whole daemon by passing method references into constructors. Each such
argument is a wiring decision: this collaborator's method is what that component will call. Point it
at the wrong object, or at a constant, and the daemon still compiles, still starts, and still passes
almost every test — because most tests build their own object graph instead of using the one
Fleetd.java builds.

#562 is the worked example. Five tests covered LoopHealthSource. All five constructed their own
LoopHealthSource, so replacing poller::health in Fleetd.java with a constant left all five
green. The fix was to extract a package-private factory that Fleetd.java actually calls, and test
that.

A TEST THAT SUPPLIES ITS OWN DEPENDENCY IS STRUCTURALLY ZERO EVIDENCE ABOUT THE PRODUCER. It
tests the consumer. That is not a weak test — it is a test of a different thing, and it looks
identical in a coverage report.

Measured starting point (main 49a5875)

Wiring-style tests that exist today — 10 for Fleetd, plus 2 elsewhere:

ls -1 fleetd/src/test/java/dev/ltms/fleet/*Wiring*Test.java | wc -l   # 10
find fleetd/src/test/java -name '*Wiring*' | wc -l                    # 12

FleetdBackendQuarantine, FleetdCapacitySource, FleetdCompletionResolver,
FleetdConfigRefCharterToolSurface, FleetdConfigRef, FleetdExhaustionDetectionArmed,
FleetdHealthCoverageSource, FleetdLeadRollover, FleetdLeadSeat, FleetdLoopHealthSource
(+ HerdrPeerLauncherAllowList, IdleSleepGuard).

Method references passed inside Fleetd.java, JDK and stream-plumbing ones removed
(System::nanoTime ×12, System::getenv ×2, Map::of, LinkedHashSet::new, Entry::getKey,
MemberSession::profile, Profile::profile):

sessions::roster (6)  poller::health (2)  config::get (2)  sessions::size  replyInbox::own
reasonByCredential::get  pushLoopRef::get  presence::forget  messages::abandon
exhaustionSinkRef::get  exhaustedPatterns::armed  completion::register
LeadMailbox::open  AmqpReplyInbox::open  Fleetd::sleepHerdrPoll
Fleetd::assertChartersNameOnlyRegisteredTools

16 distinct references, 23 occurrences. These counts are a starting point, not the answer. Re-run
the greps yourself — a reference can also be wired as a lambda (() -> x.y()) or a constructor
argument that is not a method reference at all, and those are wiring sites too. Say in your report
what your sweep could not see.

The work

For every wiring site in Fleetd.java, answer one question:

If I rewired this — pointed it at a constant, or at the wrong collaborator — would any test go
red?

Answer it by doing it, not by reading test names.

  1. Enumerate the sites. Include lambdas and plain constructor arguments, not only :: references.
  2. For each site, mutate Fleetd.java in place so the wiring is wrong but the code still
    compiles. The cheapest reliable form is replacing the reference with a constant or a no-op
    (() -> LoopWatchdog.State.RUNNING, _ -> null, and so on).
  3. Run the default profile. Record: the site, the mutation, and the named failures, or SURVIVED.
  4. Restore the file and prove it: shasum -a 256 must match the pristine value you captured first.

Acceptance

  1. A table with one row per wiring site: file and line, what is wired to what, the exact mutation
    you applied, and either the named test that went red or SURVIVED.
  2. Every SURVIVED row is a finding. For each, say in one sentence what would break in
    production if that wiring were wrong, so the lead can rank them.
  3. Gate every run on mvn -o -B -q compile before reading any test output. A build that fails to
    compile is not a survivor and not a kill — it proves nothing, and BUILD FAILURE looks like a
    kill if you do not check.
  4. Report the pristine shasum -a 256 of Fleetd.java and the same value after your last restore.
    git status --short must be clean at the end.
  5. Report the test total from a clean run, so the lead can check nothing was lost. The default
    profile is 1789 on main 49a5875.
  6. Change nothing. No fix, no new test, no production edit. This ticket is the measurement; the
    fixes are separate tickets the lead will file from your table.
  7. If your turn runs out, list by name every site you did not get to. An unmeasured site reported
    as measured is worse than no report. A partial table with an honest gap list is a good result.

Notes for whoever takes this

  • Do not trust a test name. FleetdCapacitySourceWiringTest may or may not pin Fleetd.java's own
    wiring; that is exactly what this sweep is for. #562's five tests were named perfectly and proved
    nothing.
  • Prefer a mutation that kills exactly one test. A mutation that takes a crowd down tells you
    less, because you cannot tell which test was actually pinning that site.
  • sessions::roster appears 6 times and poller::health twice. Those are separate sites and each
    needs its own row — one invariant maintained at N places needs N assertions.
Read-only sweep. **Change nothing.** Follow-up to #562, where a merged PR shipped five green tests that proved nothing about `Fleetd.java`'s own wiring, and to a finding from the fleet01 lead. ## Why this exists `Fleetd.java` builds the whole daemon by passing **method references** into constructors. Each such argument is a wiring decision: *this collaborator's method is what that component will call.* Point it at the wrong object, or at a constant, and the daemon still compiles, still starts, and still passes almost every test — because most tests build their own object graph instead of using the one `Fleetd.java` builds. #562 is the worked example. Five tests covered `LoopHealthSource`. All five **constructed their own** `LoopHealthSource`, so replacing `poller::health` in `Fleetd.java` with a constant left all five green. The fix was to extract a package-private factory that `Fleetd.java` actually calls, and test **that**. **A TEST THAT SUPPLIES ITS OWN DEPENDENCY IS STRUCTURALLY ZERO EVIDENCE ABOUT THE PRODUCER.** It tests the consumer. That is not a weak test — it is a test of a different thing, and it looks identical in a coverage report. ## Measured starting point (`main` `49a5875`) Wiring-style tests that exist today — 10 for `Fleetd`, plus 2 elsewhere: ```bash ls -1 fleetd/src/test/java/dev/ltms/fleet/*Wiring*Test.java | wc -l # 10 find fleetd/src/test/java -name '*Wiring*' | wc -l # 12 ``` `FleetdBackendQuarantine`, `FleetdCapacitySource`, `FleetdCompletionResolver`, `FleetdConfigRefCharterToolSurface`, `FleetdConfigRef`, `FleetdExhaustionDetectionArmed`, `FleetdHealthCoverageSource`, `FleetdLeadRollover`, `FleetdLeadSeat`, `FleetdLoopHealthSource` (+ `HerdrPeerLauncherAllowList`, `IdleSleepGuard`). Method references passed inside `Fleetd.java`, JDK and stream-plumbing ones removed (`System::nanoTime` ×12, `System::getenv` ×2, `Map::of`, `LinkedHashSet::new`, `Entry::getKey`, `MemberSession::profile`, `Profile::profile`): ``` sessions::roster (6) poller::health (2) config::get (2) sessions::size replyInbox::own reasonByCredential::get pushLoopRef::get presence::forget messages::abandon exhaustionSinkRef::get exhaustedPatterns::armed completion::register LeadMailbox::open AmqpReplyInbox::open Fleetd::sleepHerdrPoll Fleetd::assertChartersNameOnlyRegisteredTools ``` 16 distinct references, 23 occurrences. **These counts are a starting point, not the answer.** Re-run the greps yourself — a reference can also be wired as a lambda (`() -> x.y()`) or a constructor argument that is not a method reference at all, and those are wiring sites too. Say in your report what your sweep could **not** see. ## The work For every wiring site in `Fleetd.java`, answer one question: > **If I rewired this — pointed it at a constant, or at the wrong collaborator — would any test go > red?** Answer it by **doing it**, not by reading test names. 1. Enumerate the sites. Include lambdas and plain constructor arguments, not only `::` references. 2. For each site, mutate `Fleetd.java` **in place** so the wiring is wrong but the code still compiles. The cheapest reliable form is replacing the reference with a constant or a no-op (`() -> LoopWatchdog.State.RUNNING`, `_ -> null`, and so on). 3. Run the default profile. Record: the site, the mutation, and **the named failures**, or `SURVIVED`. 4. Restore the file and prove it: `shasum -a 256` must match the pristine value you captured first. ## Acceptance 1. **A table with one row per wiring site**: file and line, what is wired to what, the exact mutation you applied, and either the named test that went red or `SURVIVED`. 2. **Every `SURVIVED` row is a finding.** For each, say in one sentence what would break in production if that wiring were wrong, so the lead can rank them. 3. **Gate every run on `mvn -o -B -q compile` before reading any test output.** A build that fails to compile is not a survivor and not a kill — it proves nothing, and `BUILD FAILURE` looks like a kill if you do not check. 4. **Report the pristine `shasum -a 256` of `Fleetd.java` and the same value after your last restore.** `git status --short` must be clean at the end. 5. **Report the test total from a clean run**, so the lead can check nothing was lost. The default profile is 1789 on `main` `49a5875`. 6. **Change nothing.** No fix, no new test, no production edit. This ticket is the measurement; the fixes are separate tickets the lead will file from your table. 7. If your turn runs out, **list by name every site you did not get to**. An unmeasured site reported as measured is worse than no report. A partial table with an honest gap list is a good result. ## Notes for whoever takes this - Do not trust a test name. `FleetdCapacitySourceWiringTest` may or may not pin `Fleetd.java`'s own wiring; that is exactly what this sweep is for. #562's five tests were named perfectly and proved nothing. - Prefer a mutation that kills **exactly one** test. A mutation that takes a crowd down tells you less, because you cannot tell which test was actually pinning that site. - `sessions::roster` appears 6 times and `poller::health` twice. Those are separate sites and each needs its own row — one invariant maintained at N places needs N assertions.
Author
Owner

Delegated, split in two. Both workers are read-only and must open no PR.

Pristine Fleetd.java on main 49a5875 — both workers must restore to this exact value:

shasum -a 256 fleetd/src/main/java/dev/ltms/fleet/Fleetd.java
d3c693e9e595228f5b41b185e94a49d86f1cb9ddd50ff9ddc6be2b73918659ea    (2000 lines)

The split

Part A — lines 158 to 505 (ticket 587a, branch worker/587a-85090a-1)

line wiring
158 new ConfigRef(configPath, cfg, Fleetd::assertChartersNameOnlyRegisteredTools)
221 ExhaustionSink.forwardingTo(exhaustionSinkRef::get)
230 () -> config.get().memberCredentials(), null, config::get
237 () -> config.get().memberCredentials(), config::get, forwardingExhaustionSink
311 new IdleSleepGuard(new CaffeinateSleepAssertionMechanism(), sessions::size)
444 backendErrorPatternLookup(sessions::roster, …)
480 outagePolicy, pushLoopRef::get
492 worktreeBranchLookup(sessions::roster)
505 presence::forget and completion::register — two rows, one line

Part B — lines 512 to 888 (ticket 587b, branch worker/587b-28851f-2)

line wiring
512 selectReplyInbox(cfg.broker(), System.getenv(), AmqpReplyInbox::open)
518 openLeadMailbox(cfg.coordinator(), System.getenv(), LeadMailbox::open)
556 new LeadHeartbeatLoop(…, sessions::roster, …)
588 new FleetHealthMonitor(agents, sessions::roster, messages, healthScheduler, …)
591 messages::abandon
607 sessions.onAcquire(replyInbox::own)
885 exhaustedPatterns::armed and the profile -> {…} lambda — two rows, one line
888 reasonByCredential::get

Lines 1049, 1085 and 1380 are javadoc mentions, not wiring. Line 1065 is inside
loopHealthSource(StatusPoller, SessionReaper), the factory #562 extracted and already pinned by
FleetdLoopHealthSourceWiringTest; both workers are told to skip it.

Why the split is by line range and not by component

The two workers share one file. Line ranges are the only boundary that cannot overlap, and each
mutates in its own provisioned worktree, so neither can see the other's edit. Splitting by component
would have put sessions::roster — six occurrences spread from 444 to 588 — on both sides of the
boundary.

One instruction added to Part B that is not in Part A

Lines 512 and 518 wire broker openers and 556/588 wire live loops. If a mutation there makes a test
hang rather than fail, the worker records INCONCLUSIVE — mutation hangs with the mutation text
and moves on. A hang is a third outcome. Counting it as SURVIVED would invent a finding, and
waiting it out spends the whole turn on one row.

Both briefs also carry the compile gate: mvn -o -B -q compile before any test run, because a build
that fails to compile produces no test results at all — and BUILD FAILURE reads as a kill if you
were hoping for one.

Delegated, split in two. Both workers are read-only and must open no PR. **Pristine `Fleetd.java` on `main` `49a5875`** — both workers must restore to this exact value: ``` shasum -a 256 fleetd/src/main/java/dev/ltms/fleet/Fleetd.java d3c693e9e595228f5b41b185e94a49d86f1cb9ddd50ff9ddc6be2b73918659ea (2000 lines) ``` ### The split **Part A — lines 158 to 505** (ticket `587a`, branch `worker/587a-85090a-1`) | line | wiring | |---|---| | 158 | `new ConfigRef(configPath, cfg, Fleetd::assertChartersNameOnlyRegisteredTools)` | | 221 | `ExhaustionSink.forwardingTo(exhaustionSinkRef::get)` | | 230 | `() -> config.get().memberCredentials()`, `null`, `config::get` | | 237 | `() -> config.get().memberCredentials()`, `config::get`, `forwardingExhaustionSink` | | 311 | `new IdleSleepGuard(new CaffeinateSleepAssertionMechanism(), sessions::size)` | | 444 | `backendErrorPatternLookup(sessions::roster, …)` | | 480 | `outagePolicy, pushLoopRef::get` | | 492 | `worktreeBranchLookup(sessions::roster)` | | 505 | `presence::forget` **and** `completion::register` — two rows, one line | **Part B — lines 512 to 888** (ticket `587b`, branch `worker/587b-28851f-2`) | line | wiring | |---|---| | 512 | `selectReplyInbox(cfg.broker(), System.getenv(), AmqpReplyInbox::open)` | | 518 | `openLeadMailbox(cfg.coordinator(), System.getenv(), LeadMailbox::open)` | | 556 | `new LeadHeartbeatLoop(…, sessions::roster, …)` | | 588 | `new FleetHealthMonitor(agents, sessions::roster, messages, healthScheduler, …)` | | 591 | `messages::abandon` | | 607 | `sessions.onAcquire(replyInbox::own)` | | 885 | `exhaustedPatterns::armed` **and** the `profile -> {…}` lambda — two rows, one line | | 888 | `reasonByCredential::get` | Lines 1049, 1085 and 1380 are javadoc mentions, not wiring. Line 1065 is inside `loopHealthSource(StatusPoller, SessionReaper)`, the factory #562 extracted and already pinned by `FleetdLoopHealthSourceWiringTest`; both workers are told to skip it. ### Why the split is by line range and not by component The two workers share one file. Line ranges are the only boundary that cannot overlap, and each mutates in its own provisioned worktree, so neither can see the other's edit. Splitting by component would have put `sessions::roster` — six occurrences spread from 444 to 588 — on both sides of the boundary. ### One instruction added to Part B that is not in Part A Lines 512 and 518 wire broker openers and 556/588 wire live loops. If a mutation there makes a test **hang** rather than fail, the worker records `INCONCLUSIVE — mutation hangs` with the mutation text and moves on. A hang is a third outcome. Counting it as `SURVIVED` would invent a finding, and waiting it out spends the whole turn on one row. Both briefs also carry the compile gate: `mvn -o -B -q compile` before any test run, because a build that fails to compile produces no test results at all — and `BUILD FAILURE` reads as a kill if you were hoping for one.
Author
Owner

Result: 44 sites measured, and the headline is not the one either half reported

Both halves are in. Part A swept lines 158–505 (27 sites), Part B swept 512–888 (17 sites). One
mutation per build, full suite each time, file restored and sha-verified after every one — 17
restores checked individually in Part B, every restore checked in Part A.

Both reported the same baseline on main 49a5875: 1789 tests, 0 failures. I re-measured it
myself in a scratch worktree at origin/main: Tests run: 1789, Failures: 0, Errors: 0, Skipped: 0,
BUILD SUCCESS. Both worktrees ended at the pristine
d3c693e9e595228f5b41b185e94a49d86f1cb9ddd50ff9ddc6be2b73918659ea with a clean git status, which
I confirmed independently in both before collecting.

Raw totals: 44 sites, 6 killed, 38 survived.

The correction: "6 killed" is really "1 killed"

I classified each killing test by what it actually does. Five of the six kills are source-text
assertions
— Files.readString on Fleetd.java followed by source.contains("…"). They never
construct anything and never run main.

Kill kind Sites
Runtime (builds the object, asserts on behaviour) 1 — :885 exhaustedPatterns::armed, killed by aProfileWithNoPatternIsUnarmed, reloadedPatternArmsDetectionWithNoRestart, reloadedPatternRemovalDisarmsDetectionWithNoRestart in FleetdExhaustionDetectionArmedWiringTest
Source text only 5 — :158 (FleetdConfigRefWiringTest.mainStillWiresTheThreeArgumentConfigRefConstructor), :444 + :479 + :492 (FleetdCompletionResolverWiringTest), :678 (FleetdLeadSeatWiringTest.fleetMcpConstructionStillWiresLeadSeatLookup)
Nothing 38
$ grep -n 'source.contains' fleetd/src/test/java/dev/ltms/fleet/FleetdCompletionResolverWiringTest.java
59:        assertTrue(source.contains("worktreeBranchLookup(sessions::roster)"),
72:        assertTrue(source.contains(
84:        assertTrue(source.contains(

So the behavioural coverage of this file's wiring is 1 of 44, not 6 of 44.

To be fair to those tests: they are honest about themselves. Every one carries [SOURCE TEXT] in
its @DisplayName and says so in its javadoc — FleetdLeadSeatWiringTest's reads "This test
checks source text, not runtime behaviour.
It never constructs a FleetMcp and never runs
main." And they do guard the actual hazard those tickets were written for: silent deletion or
in-place substitution of an argument. The defect is in how my ticket's table reads them, not in how
they were written.

But a source-text test fails in both directions, which matters for what we do next:

  • it is blind to a behavioural break that keeps the characters — the named collaborator can
    itself be broken, or two arguments of the same type can be swapped;
  • it goes red on a harmless edit — reformat the call across lines, extract a local, rename a
    lambda parameter, and correct code fails;
  • it cannot survive the refactor it exists to protect: extract the wiring into a named factory
    (the normal fix here, and what #584 just did for loopHealthSource) and the string disappears
    while the wiring is perfect.

Credit where due: Part B spotted this for its own single row and quoted the javadoc. Neither half
noticed it held for every kill in the sweep — that only shows up reading down the column, which
is my job, not theirs.

Two survivors I verified myself, rather than relaying

Both in a scratch worktree at origin/main, compile-gated before reading any test output, file
restored and sha-checked after each.

1. :611-631 — the whole sessions.onRelease(...) release-cleanup lambda → detail -> { }

mutated sha: 821ce6628fd46814
compile OK
[INFO] Tests run: 1789, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
named failures: 0
restored sha: d3c693e9e595228f  (want d3c693e9e595228f)

SURVIVED, and the total is unchanged — no crowd effect, nothing perturbed. That single empty
lambda removes all three of messages.abandon(...), replyInbox.release(...) and
primaryRegistry.forgetDelegation(...).

This is the worst one in the sweep, and MessageService.abandon's own javadoc at :668 says why:

Without this, tearing a worker down left its rendezvous waiter open: a blocking fleet_send kept
blocking, and an async one kept reporting PENDING until ASYNC_TIMEOUT_MS — thirty minutes —
even though the worker provably no longer existed and the delegation could never complete.

So if this lambda regressed, every fleet_stop and every idle-reap would hang its lead's ticket for
the full thirty minutes. That is the same thirty minutes as #588, which I filed today after
measuring 41 ticket timeouts landing at exactly 1800 s.

And the shape is the familiar one — each collaborator is tested, the caller is not:

$ grep -rn 'void abandonFailsEveryPendingAsyncTicketForTheReleasedTarget' fleetd/src/test/java
MessageServiceTest.java:1171          # abandon itself: covered
$ grep -rn 'forgetDelegation' fleetd/src/test/java | head -3
ReplyPushLoopTest.java:215  * The bug: PrimaryRegistry.forgetDelegation is wired to a worker's release, never to
PrimaryRegistryTest.java:157        reg.forgetDelegation("term_worker");     # the method itself: covered

Three collaborators individually pinned, and the lambda in Fleetd.main that calls all three is
unpinned. ReplyPushLoopTest:215 even documents the wiring as load-bearing in prose — which, in
this tree, has reliably marked an untested invariant rather than a tested one.

2. :468 — exhaustionSinkRef.set(exhaustionSink) → exhaustionSinkRef.set(ExhaustionSink.none())

mutated sha: 9bf7403574498bc0
compile OK
[INFO] Tests run: 1789, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
named failures: 0
restored sha: d3c693e9e595228f  (want d3c693e9e595228f)

SURVIVED. This is the widest blast radius of anything in the sweep, because of the construction
order around it:

:214  AtomicReference<ExhaustionSink> exhaustionSinkRef = new AtomicReference<>(ExhaustionSink.none());
:221  ExhaustionSink forwardingExhaustionSink = ExhaustionSink.forwardingTo(exhaustionSinkRef::get);
:468  exhaustionSinkRef.set(exhaustionSink);          <- the only line that swaps the real sink in

The ref starts as a no-op, both launchers are handed the forwarding sink that reads it, and :468
is the single line that ever makes it real. Change that one line and every exhaustion signal
from every member on both adapters goes nowhere: CB-578 stage B quarantine never fires for
any profile, fleet-wide, and the daemon keeps spawning onto a dead credential. One line, compiles
clean, 1789 tests green.

Fix priority — the 38 survivors, ranked by what silently turns off

Not all 38 are worth a test. Ranked by blast radius:

Tier 1 — a control turns off fleet-wide, silently. :468 (quarantine never fires for any
profile), :221 (the forwarding sink upstream of it), :411 (no profile's exhaustedPattern is
seen after boot), :412-416 (a usage-limit refusal is handed back as a real completion, so a lead
acts on an exhausted account's "answer" as real work).

Tier 2 — a credential policy stops gating. :230 and :237 memberCredentials — per Part A,
this reopens the CB-592 credential-exposure gap that CB-596's policy closed, for claude-code and
opencode respectively. This is the security-relevant pair.

Tier 3 — a lead waits on work that can never arrive (all of these end in #588's thirty
minutes). :611-631 (verified above), :591 messages::abandon on the health path, :505
completion::register (a delivered turn never registered, so a completion can never resolve the
blocked send — the gap #556 closed), :518 LeadMailbox::open (lead-to-lead coordination
permanently off with coordinator: fully configured), :512 AmqpReplyInbox::open (durable
cross-restart reply delivery silently replaced by in-memory).

Tier 4 — live config goes stale. :230/:237 config::get, :229/:236 fleet supplier,
:393 MemberRegistry.live — a fleetd.yaml reload silently stops reaching that consumer, which
is exactly the regression #424 fixed once already.

Tier 5 — host resource and diagnostics. :311/:312/:313 (IdleSleepGuard: the host can
idle-sleep under a long turn, or stay awake forever), :589 (the #386 wall-clock correction, so
stall detection breaks after the host wakes), :276 (busy-spin), :537-538 and the thread-factory
names (names in thread dumps only).

Gaps both halves reported honestly, and I am not treating as measured

  • Part A, :260 — config passed into CompositePeerLauncher. Same shape as the other
    config::get sites; explicitly reported as "not measured — a genuine gap in my sweep, not a
    judgment exclusion."
  • Part B, :552, :580, :696 — the bridge-heartbeat- / bridge-health- / bridge-leadcoord-
    virtual-thread factories, textually identical to :537-538 which did survive. Part B declined to
    report an expected result as a measured one. Tier 5, so low value.
  • Part B's judgment exclusions — plain constructor args with no lambda form (:638-639,
    :644-656, :698, :711), and the exhaustionSink(...) / deliverableTo(...) factory bodies
    treated as covered by direct factory tests on a reading of their javadoc, not by mutation.
  • Part A's judgment exclusions — data-shaping lambdas that prepare a local value rather than
    inject a collaborator (:202-208, :420-423, :435-440), and JDK refs (System::nanoTime,
    System::getenv) per this ticket's own exclusion list.

Both halves flagged that their search for "sites the :: grep could not see" was itself textual
(awk for -> or ::), so neither can prove a wiring decision does not exist on a line carrying
neither token.

What I got wrong in this ticket

The KILLED/SURVIVED column I asked for merges two kinds of evidence that need opposite
treatment, and it presents the weaker kind in the more confident-looking cell. The brief should
have asked for the kill kind, not just the kill. That is on me, not on either worker — neither
reported a single cell incorrectly.

Follow-up work is filed as #589; leaving this ticket open until those tests land.

## Result: 44 sites measured, and the headline is not the one either half reported Both halves are in. Part A swept lines 158–505 (27 sites), Part B swept 512–888 (17 sites). One mutation per build, full suite each time, file restored and sha-verified after every one — 17 restores checked individually in Part B, every restore checked in Part A. Both reported the same baseline on `main` `49a5875`: **1789 tests, 0 failures**. I re-measured it myself in a scratch worktree at `origin/main`: `Tests run: 1789, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. Both worktrees ended at the pristine `d3c693e9e595228f5b41b185e94a49d86f1cb9ddd50ff9ddc6be2b73918659ea` with a clean `git status`, which I confirmed independently in both before collecting. Raw totals: **44 sites, 6 killed, 38 survived.** ### The correction: "6 killed" is really "1 killed" I classified each killing test by what it actually does. Five of the six kills are **source-text assertions** — `Files.readString` on `Fleetd.java` followed by `source.contains("…")`. They never construct anything and never run `main`. | Kill kind | Sites | |---|---| | **Runtime** (builds the object, asserts on behaviour) | **1** — `:885` `exhaustedPatterns::armed`, killed by `aProfileWithNoPatternIsUnarmed`, `reloadedPatternArmsDetectionWithNoRestart`, `reloadedPatternRemovalDisarmsDetectionWithNoRestart` in `FleetdExhaustionDetectionArmedWiringTest` | | **Source text only** | **5** — `:158` (`FleetdConfigRefWiringTest.mainStillWiresTheThreeArgumentConfigRefConstructor`), `:444` + `:479` + `:492` (`FleetdCompletionResolverWiringTest`), `:678` (`FleetdLeadSeatWiringTest.fleetMcpConstructionStillWiresLeadSeatLookup`) | | Nothing | 38 | ``` $ grep -n 'source.contains' fleetd/src/test/java/dev/ltms/fleet/FleetdCompletionResolverWiringTest.java 59: assertTrue(source.contains("worktreeBranchLookup(sessions::roster)"), 72: assertTrue(source.contains( 84: assertTrue(source.contains( ``` **So the behavioural coverage of this file's wiring is 1 of 44, not 6 of 44.** To be fair to those tests: they are honest about themselves. Every one carries `[SOURCE TEXT]` in its `@DisplayName` and says so in its javadoc — `FleetdLeadSeatWiringTest`'s reads "**This test checks source text, not runtime behaviour.** It never constructs a `FleetMcp` and never runs `main`." And they do guard the actual hazard those tickets were written for: silent *deletion* or in-place substitution of an argument. The defect is in how my ticket's table reads them, not in how they were written. But a source-text test fails in both directions, which matters for what we do next: - it is **blind** to a behavioural break that keeps the characters — the named collaborator can itself be broken, or two arguments of the same type can be swapped; - it goes **red on a harmless edit** — reformat the call across lines, extract a local, rename a lambda parameter, and correct code fails; - it **cannot survive the refactor it exists to protect**: extract the wiring into a named factory (the normal fix here, and what #584 just did for `loopHealthSource`) and the string disappears while the wiring is perfect. Credit where due: Part B spotted this for its own single row and quoted the javadoc. Neither half noticed it held for *every* kill in the sweep — that only shows up reading down the column, which is my job, not theirs. ### Two survivors I verified myself, rather than relaying Both in a scratch worktree at `origin/main`, compile-gated before reading any test output, file restored and sha-checked after each. **1. `:611-631` — the whole `sessions.onRelease(...)` release-cleanup lambda → `detail -> { }`** ``` mutated sha: 821ce6628fd46814 compile OK [INFO] Tests run: 1789, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS named failures: 0 restored sha: d3c693e9e595228f (want d3c693e9e595228f) ``` **SURVIVED**, and the total is unchanged — no crowd effect, nothing perturbed. That single empty lambda removes all three of `messages.abandon(...)`, `replyInbox.release(...)` and `primaryRegistry.forgetDelegation(...)`. This is the worst one in the sweep, and `MessageService.abandon`'s own javadoc at `:668` says why: > Without this, tearing a worker down left its rendezvous waiter open: a blocking `fleet_send` kept > blocking, and an async one kept reporting `PENDING` until `ASYNC_TIMEOUT_MS` — thirty minutes — > even though the worker provably no longer existed and the delegation could never complete. So if this lambda regressed, every `fleet_stop` and every idle-reap would hang its lead's ticket for the full thirty minutes. That is the same thirty minutes as #588, which I filed today after measuring 41 ticket timeouts landing at exactly 1800 s. And the shape is the familiar one — each collaborator is tested, the caller is not: ``` $ grep -rn 'void abandonFailsEveryPendingAsyncTicketForTheReleasedTarget' fleetd/src/test/java MessageServiceTest.java:1171 # abandon itself: covered $ grep -rn 'forgetDelegation' fleetd/src/test/java | head -3 ReplyPushLoopTest.java:215 * The bug: PrimaryRegistry.forgetDelegation is wired to a worker's release, never to PrimaryRegistryTest.java:157 reg.forgetDelegation("term_worker"); # the method itself: covered ``` Three collaborators individually pinned, and the lambda in `Fleetd.main` that calls all three is unpinned. `ReplyPushLoopTest:215` even documents the wiring as load-bearing in prose — which, in this tree, has reliably marked an *untested* invariant rather than a tested one. **2. `:468` — `exhaustionSinkRef.set(exhaustionSink)` → `exhaustionSinkRef.set(ExhaustionSink.none())`** ``` mutated sha: 9bf7403574498bc0 compile OK [INFO] Tests run: 1789, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS named failures: 0 restored sha: d3c693e9e595228f (want d3c693e9e595228f) ``` **SURVIVED.** This is the widest blast radius of anything in the sweep, because of the construction order around it: ``` :214 AtomicReference<ExhaustionSink> exhaustionSinkRef = new AtomicReference<>(ExhaustionSink.none()); :221 ExhaustionSink forwardingExhaustionSink = ExhaustionSink.forwardingTo(exhaustionSinkRef::get); :468 exhaustionSinkRef.set(exhaustionSink); <- the only line that swaps the real sink in ``` The ref starts as a no-op, both launchers are handed the forwarding sink that reads it, and `:468` is the single line that ever makes it real. Change that one line and **every** exhaustion signal from **every** member on **both** adapters goes nowhere: CB-578 stage B quarantine never fires for any profile, fleet-wide, and the daemon keeps spawning onto a dead credential. One line, compiles clean, 1789 tests green. ### Fix priority — the 38 survivors, ranked by what silently turns off Not all 38 are worth a test. Ranked by blast radius: **Tier 1 — a control turns off fleet-wide, silently.** `:468` (quarantine never fires for any profile), `:221` (the forwarding sink upstream of it), `:411` (no profile's `exhaustedPattern` is seen after boot), `:412-416` (a usage-limit refusal is handed back as a real completion, so a lead acts on an exhausted account's "answer" as real work). **Tier 2 — a credential policy stops gating.** `:230` and `:237` `memberCredentials` — per Part A, this reopens the CB-592 credential-exposure gap that CB-596's policy closed, for claude-code and opencode respectively. This is the security-relevant pair. **Tier 3 — a lead waits on work that can never arrive** (all of these end in #588's thirty minutes). `:611-631` (verified above), `:591` `messages::abandon` on the health path, `:505` `completion::register` (a delivered turn never registered, so a completion can never resolve the blocked send — the gap #556 closed), `:518` `LeadMailbox::open` (lead-to-lead coordination permanently off with `coordinator:` fully configured), `:512` `AmqpReplyInbox::open` (durable cross-restart reply delivery silently replaced by in-memory). **Tier 4 — live config goes stale.** `:230`/`:237` `config::get`, `:229`/`:236` fleet supplier, `:393` `MemberRegistry.live` — a `fleetd.yaml` reload silently stops reaching that consumer, which is exactly the regression #424 fixed once already. **Tier 5 — host resource and diagnostics.** `:311`/`:312`/`:313` (IdleSleepGuard: the host can idle-sleep under a long turn, or stay awake forever), `:589` (the #386 wall-clock correction, so stall detection breaks after the host wakes), `:276` (busy-spin), `:537-538` and the thread-factory names (names in thread dumps only). ### Gaps both halves reported honestly, and I am not treating as measured - **Part A, `:260`** — `config` passed into `CompositePeerLauncher`. Same shape as the other `config::get` sites; explicitly reported as "not measured — a genuine gap in my sweep, not a judgment exclusion." - **Part B, `:552`, `:580`, `:696`** — the `bridge-heartbeat-` / `bridge-health-` / `bridge-leadcoord-` virtual-thread factories, textually identical to `:537-538` which did survive. Part B declined to report an expected result as a measured one. Tier 5, so low value. - **Part B's judgment exclusions** — plain constructor args with no lambda form (`:638-639`, `:644-656`, `:698`, `:711`), and the `exhaustionSink(...)` / `deliverableTo(...)` factory bodies treated as covered by direct factory tests on a reading of their javadoc, not by mutation. - **Part A's judgment exclusions** — data-shaping lambdas that prepare a local value rather than inject a collaborator (`:202-208`, `:420-423`, `:435-440`), and JDK refs (`System::nanoTime`, `System::getenv`) per this ticket's own exclusion list. Both halves flagged that their search for "sites the `::` grep could not see" was itself textual (`awk` for `->` or `::`), so neither can prove a wiring decision does not exist on a line carrying neither token. ### What I got wrong in this ticket The `KILLED`/`SURVIVED` column I asked for merges two kinds of evidence that need opposite treatment, and it presents the weaker kind in the more confident-looking cell. The brief should have asked for the kill *kind*, not just the kill. That is on me, not on either worker — neither reported a single cell incorrectly. Follow-up work is filed as #589; leaving this ticket open until those tests land.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#587