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

Fleetd.main's injected wirings are unpinned: 16 call sites go inert with a fully green suite #612

New Issue
Open
opened 2026-09-20 12:26:00 +02:00 by ltms · 11 comments
ltms commented 2026-09-20 12:26:00 +02:00
Owner
Copy Link
Copy Source

What this is

Fleetd.main builds the daemon by handing injected "source" objects and lookups to FleetMcp,
to the loops, and to CompletionResolver. Most are a one-line call at the main call site.
Nothing calls Fleetd.main far enough to observe which object it actually passed, so a call
site can be swapped for its inert variant — X.none(), _ -> null, () -> Map.of() — and the
whole suite stays green.

This is not a theory. #602/#606 shipped exactly that defect: main passed
FleetMcp.LeadConfigDirSource.none(), every test passed, and the live daemon reported
"state":"unknown" for every lead. The fix extracted a factory and pinned the factory. The
call site is still open, and this sweep re-measured it.

Method

Each site: mutate the main call site to its inert equivalent → mvn -o compile → full
mvn -o test → record red/green → revert → confirm git diff --stat empty before the next one.

Baseline on 9a992d0: 1841 tests, 0 failures.

17 mutation cycles were run this way — 16 suspected sites plus 2 control mutations on sites
expected to be covered. Both controls went red, which is what makes the 16 greens evidence
rather than an absence:

  • LeadSeatSource → none() failed FleetdLeadSeatWiringTest.fleetMcpConstructionStillWiresLeadSeatLookup
  • worktreeBranchLookup → _ -> null failed FleetdCompletionResolverWiringTest.worktreeBranchLookupIsStillPassedAtTheCallSite

I reproduced the top-ranked gap myself, independently, on a clean origin/main worktree:

pristine anchor count: 1
419:        ExhaustedPatternLookup exhaustedPatterns = ExhaustedPatternLookup.none();
mvn exit: 0
[INFO] Tests run: 1841, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
reverted; anchor back: 1  diff lines: 0

Ranked by what a live daemon does, not by fix effort

Line numbers as of 9a992d0.

# Call site Consequence when inert
1 exhaustedPatternLookup / liveExhaustedPatterns (418-419) A genuine usage-limit refusal is handed back to a waiting caller as real completed work
2 publishExhaustionSink (472), forwardingExhaustionSink (227) A credential that hits a usage limit is never quarantined; spawns keep landing on a burnt-out subscription with no backoff. The two are independent — OpenCode's exhaustion path dies separately
3 replyInboxOpener (520) A configured durable AMQP inbox silently becomes the in-memory one, while the log still prints "reply inbox: AMQP broker (durable)". Real reply loss across a restart
4 quarantineSource (658), outageSource (660-663) Enforcement stays correct underneath; fleet_profiles/fleet_list confidently report "not quarantined"/"not cooling off" for a profile that is. An operator debugging failed spawns is actively misled
5 leadConfigDirSource (678) The literal #602/#606 defect, reopened one call site away from its own fix
6 releaseCleanup (627) Fires on every teardown: leaks a stuck rendezvous waiter, an unreleased reply-inbox consumer, and a stale lead binding each time
7 healthFailTarget (603) A dead member's waiting ticket sits PENDING for the full 30-minute async timeout instead of failing immediately
8 leadMailboxOpener (528) Lead-to-lead coordination silently never starts, with a WARN blaming the broker — pointing the operator at network debugging for a code regression
9 capacitySource (668) fleet_list reports zero configured profiles. Spawning unaffected. Loud, likely caught fast
10 loopHealthSource (665) Both loops always report STOPPED — noisy, but it also masks a real stall behind a constant false alarm
11 coordinator peers (683), healthCoverageSource (669) Reporting-only, cosmetic
12 turnRegistrar (511) Narrowest window: only when a turnListener callback throws between delivery and completion

Rank 1 deserves a note. The comment directly above those two lines already names this exact
outcome:

// this is the worst consequence in the whole #589 sweep: silently losing either wiring
// means a genuine usage-limit refusal is handed back as a real completion instead of
// BACKEND_EXHAUSTED.

A comment naming the worst outcome in the file, sitting on top of two unpinned call sites.

Two shapes, and they need different fixes

Shape A — a false report, enforcement intact (4, 9, 10, 11, and 5). The real gate is built
separately from the real object, so behaviour is right and only the report lies. Bad because an
operator trusts the report while debugging.

Shape B — behaviour silently degrades (1, 2, 3, 6, 7, 8). Nothing else enforces it. The
daemon does the wrong thing and says nothing.

Fix Shape B first. A wrong answer an operator can see beats a wrong action nobody can.

What a fix must not be

Three existing wiring tests pass by asserting the exact source text of the call site
(FleetdCompletionResolverWiringTest.backendErrorArgumentsAreStillNamedAtTheCallSite,
FleetdBackendQuarantineWiringTest, FleetdLeadRolloverWiringTest). That catches a deletion and
nothing else: it goes green on a call site that names the right symbols and still passes the
wrong thing, and it goes red on a harmless reformat. It is a fourth copy of the source, not a
test of behaviour.

The durable fix is structural: make main's composition callable by a test — extract the
assembly into one method a test can drive and inspect — rather than adding a seventeenth
per-site assertion. Whoever picks this up should propose that shape before writing tests.

Provenance

The 16 sites were measured by a hunter sweep (17 mutate/build/full-test/revert cycles, tree clean
after each, git status --porcelain empty at the end). Rank 1 I re-ran myself; the output is
above. Ranks 2-12 I have not personally reproduced — that is the hunter's measurement, and
its method was validated by the two controls.

Three further sites are listed in that sweep as "covered by source-text match only": the hunter
confirmed the asserted substring is present in the live file but did not mutate them. Treat those
three as unverified in both directions.

## What this is `Fleetd.main` builds the daemon by handing injected "source" objects and lookups to `FleetMcp`, to the loops, and to `CompletionResolver`. Most are a one-line call at the `main` call site. **Nothing calls `Fleetd.main` far enough to observe which object it actually passed**, so a call site can be swapped for its inert variant — `X.none()`, `_ -> null`, `() -> Map.of()` — and the whole suite stays green. This is not a theory. #602/#606 shipped exactly that defect: `main` passed `FleetMcp.LeadConfigDirSource.none()`, every test passed, and the live daemon reported `"state":"unknown"` for every lead. The fix extracted a factory and pinned **the factory**. The call site is still open, and this sweep re-measured it. ## Method Each site: mutate the `main` call site to its inert equivalent → `mvn -o compile` → **full** `mvn -o test` → record red/green → revert → confirm `git diff --stat` empty before the next one. Baseline on `9a992d0`: **1841 tests, 0 failures**. 17 mutation cycles were run this way — 16 suspected sites plus **2 control mutations on sites expected to be covered**. Both controls went red, which is what makes the 16 greens evidence rather than an absence: - `LeadSeatSource` → `none()` failed `FleetdLeadSeatWiringTest.fleetMcpConstructionStillWiresLeadSeatLookup` - `worktreeBranchLookup` → `_ -> null` failed `FleetdCompletionResolverWiringTest.worktreeBranchLookupIsStillPassedAtTheCallSite` **I reproduced the top-ranked gap myself**, independently, on a clean `origin/main` worktree: ``` pristine anchor count: 1 419: ExhaustedPatternLookup exhaustedPatterns = ExhaustedPatternLookup.none(); mvn exit: 0 [INFO] Tests run: 1841, Failures: 0, Errors: 0, Skipped: 0 [INFO] BUILD SUCCESS reverted; anchor back: 1 diff lines: 0 ``` ## Ranked by what a live daemon does, not by fix effort Line numbers as of `9a992d0`. | # | Call site | Consequence when inert | |---|---|---| | 1 | `exhaustedPatternLookup` / `liveExhaustedPatterns` (418-419) | A genuine usage-limit refusal is handed back to a waiting caller **as real completed work** | | 2 | `publishExhaustionSink` (472), `forwardingExhaustionSink` (227) | A credential that hits a usage limit is never quarantined; spawns keep landing on a burnt-out subscription with no backoff. The two are independent — OpenCode's exhaustion path dies separately | | 3 | `replyInboxOpener` (520) | A configured durable AMQP inbox silently becomes the in-memory one, **while the log still prints "reply inbox: AMQP broker (durable)"**. Real reply loss across a restart | | 4 | `quarantineSource` (658), `outageSource` (660-663) | Enforcement stays correct underneath; `fleet_profiles`/`fleet_list` confidently report "not quarantined"/"not cooling off" for a profile that is. An operator debugging failed spawns is actively misled | | 5 | `leadConfigDirSource` (678) | The literal #602/#606 defect, reopened one call site away from its own fix | | 6 | `releaseCleanup` (627) | Fires on **every** teardown: leaks a stuck rendezvous waiter, an unreleased reply-inbox consumer, and a stale lead binding each time | | 7 | `healthFailTarget` (603) | A dead member's waiting ticket sits PENDING for the full 30-minute async timeout instead of failing immediately | | 8 | `leadMailboxOpener` (528) | Lead-to-lead coordination silently never starts, with a WARN blaming the broker — pointing the operator at network debugging for a code regression | | 9 | `capacitySource` (668) | `fleet_list` reports zero configured profiles. Spawning unaffected. Loud, likely caught fast | | 10 | `loopHealthSource` (665) | Both loops always report STOPPED — noisy, but it also masks a real stall behind a constant false alarm | | 11 | coordinator peers (683), `healthCoverageSource` (669) | Reporting-only, cosmetic | | 12 | `turnRegistrar` (511) | Narrowest window: only when a turnListener callback throws between delivery and completion | Rank 1 deserves a note. The comment directly above those two lines already names this exact outcome: ```java // this is the worst consequence in the whole #589 sweep: silently losing either wiring // means a genuine usage-limit refusal is handed back as a real completion instead of // BACKEND_EXHAUSTED. ``` A comment naming the worst outcome in the file, sitting on top of two unpinned call sites. ## Two shapes, and they need different fixes **Shape A — a false report, enforcement intact** (4, 9, 10, 11, and 5). The real gate is built separately from the real object, so behaviour is right and only the report lies. Bad because an operator trusts the report while debugging. **Shape B — behaviour silently degrades** (1, 2, 3, 6, 7, 8). Nothing else enforces it. The daemon does the wrong thing and says nothing. Fix Shape B first. A wrong answer an operator can see beats a wrong action nobody can. ## What a fix must not be Three existing wiring tests pass by asserting the **exact source text** of the call site (`FleetdCompletionResolverWiringTest.backendErrorArgumentsAreStillNamedAtTheCallSite`, `FleetdBackendQuarantineWiringTest`, `FleetdLeadRolloverWiringTest`). That catches a deletion and nothing else: it goes green on a call site that names the right symbols and still passes the wrong thing, and it goes red on a harmless reformat. It is a fourth copy of the source, not a test of behaviour. The durable fix is structural: make `main`'s composition callable by a test — extract the assembly into one method a test can drive and inspect — rather than adding a seventeenth per-site assertion. Whoever picks this up should propose that shape before writing tests. ## Provenance The 16 sites were measured by a hunter sweep (17 mutate/build/full-test/revert cycles, tree clean after each, `git status --porcelain` empty at the end). Rank 1 I re-ran myself; the output is above. Ranks 2-12 I have **not** personally reproduced — that is the hunter's measurement, and its method was validated by the two controls. Three further sites are listed in that sweep as "covered by source-text match only": the hunter confirmed the asserted substring is present in the live file but did not mutate them. Treat those three as unverified in both directions.
ltms commented 2026-09-22 05:18:01 +02:00
Author
Owner
Copy Link
Copy Source

A 17th site landed today, and I measured it before merging it

Merged #616 (fleetd #613) adds reportRoleFallbackGaps(cfg) to Fleetd.main, right after cfg.validateAll(). It is a new instance of exactly this issue's shape. Recording it here so the structural fix has to cover it and it does not get lost.

I measured it with a mutation pair, on the PR branch before merging:

Mutation Result
Call site in main replaced with a comment Tests run: 1870, Failures: 0 — BUILD SUCCESS
Method body made inert (if (true) return;) Tests run: 1870, Failures: 2, Errors: 1 — BUILD FAILURE

Tree confirmed clean between runs (git diff --stat empty, mutant marker count back to 0).

The pair is what makes this evidence. The method is pinned — four new tests drive it directly and go red when it stops working. The call site is not: main can stop calling it entirely and the whole suite stays green. Without the second mutation the first green would just be an absence of evidence.

This one is Shape B, and close to the worst case

The feature is a boot log line. There is no separate enforcement path underneath that stays correct — if the call site goes inert, the capability is 100% absent and nothing anywhere says so. That puts it with ranks 1, 2, 3, 6, 7 and 8 rather than with the reporting-only sites.

It is also mildly self-referential: a call site that silently does not run, added to warn operators about config that silently does not do what they expect.

Why I merged it anyway

There is no cheap correct fix available today. The tests already use the right instrument — a real ListAppender on the logger, asserting getFormattedMessage(), not the call site's source text. Pinning the call site properly needs exactly what this issue asks for: main's composition made drivable by a test. Adding a per-site source-text assertion instead would produce the anti-pattern this issue explicitly rules out, and would be a fourth copy of the source.

So the gap is pre-existing in kind, not new in kind, and the change itself closes a real observability hole. Blocking it on a structural fix that is still being designed would trade a measured small gap for a known larger one.

What this asks of the fix

Whoever designs the shape should treat this as a 17th site and check it is covered — it is a plain static void call taking only cfg, so it is one of the easiest possible cases. A shape that cannot pin this one will not pin the harder resource-bound sites either (ranks 2, 3, 8). It is a good smoke test for a candidate design.

An architect is working on the shape now; I will add its proposal to this issue when it reports.

## A 17th site landed today, and I measured it before merging it Merged #616 (fleetd #613) adds `reportRoleFallbackGaps(cfg)` to `Fleetd.main`, right after `cfg.validateAll()`. It is a new instance of exactly this issue's shape. Recording it here so the structural fix has to cover it and it does not get lost. I measured it with a mutation pair, on the PR branch before merging: | Mutation | Result | |---|---| | Call site in `main` replaced with a comment | `Tests run: 1870, Failures: 0` — **BUILD SUCCESS** | | Method body made inert (`if (true) return;`) | `Tests run: 1870, Failures: 2, Errors: 1` — **BUILD FAILURE** | Tree confirmed clean between runs (`git diff --stat` empty, mutant marker count back to 0). The pair is what makes this evidence. The method **is** pinned — four new tests drive it directly and go red when it stops working. The **call site is not**: `main` can stop calling it entirely and the whole suite stays green. Without the second mutation the first green would just be an absence of evidence. ## This one is Shape B, and close to the worst case The feature *is* a boot log line. There is no separate enforcement path underneath that stays correct — if the call site goes inert, the capability is 100% absent and nothing anywhere says so. That puts it with ranks 1, 2, 3, 6, 7 and 8 rather than with the reporting-only sites. It is also mildly self-referential: a call site that silently does not run, added to warn operators about config that silently does not do what they expect. ## Why I merged it anyway There is no cheap correct fix available today. The tests already use the right instrument — a real `ListAppender` on the logger, asserting `getFormattedMessage()`, not the call site's source text. Pinning the call site properly needs exactly what this issue asks for: `main`'s composition made drivable by a test. Adding a per-site source-text assertion instead would produce the anti-pattern this issue explicitly rules out, and would be a fourth copy of the source. So the gap is pre-existing in kind, not new in kind, and the change itself closes a real observability hole. Blocking it on a structural fix that is still being designed would trade a measured small gap for a known larger one. ## What this asks of the fix Whoever designs the shape should treat this as a **17th site** and check it is covered — it is a plain `static void` call taking only `cfg`, so it is one of the easiest possible cases. A shape that cannot pin this one will not pin the harder resource-bound sites either (ranks 2, 3, 8). It is a good smoke test for a candidate design. An architect is working on the shape now; I will add its proposal to this issue when it reports.
ltms commented 2026-09-22 05:23:25 +02:00
Author
Owner
Copy Link
Copy Source

Architect proposal for the fix shape, with a delegation split

An architect worked this and settled on a shape. Recording it in full so implementation can start from it. I have not yet verified its code claims myself — it says plainly which parts it checked and which it reasoned about, and I have kept that separation below.

The decision

Use this issue's extraction idea, but do not return a test-only snapshot. Extract the real boot composition into a package-private assembly that owns the objects production uses:

static FleetdRuntime assembleAndStart(AssemblyInputs inputs, ResourcePorts ports)

Fleetd.main keeps config loading, the startup reports and validation, then calls this with ResourcePorts.system(). FleetdRuntime owns shutdown plus the built CompletionResolver, Injector, broker resources, FleetMcp, FleetApp, the loops, and the wiring bundles.

Fleetd.main
  -> FleetdAssembly.assembleAndStart(inputs, ResourcePorts.system())
       -> ExhaustionWiring
       -> BrokerResources
       -> RuntimeHooks
       -> FleetMcp.ReportingSources
       -> FleetdRuntime

FleetdRuntime is the observation seam: its package-private accessors expose the same bundles it owns and closes. Tests must never receive a second copy — that is what keeps this from becoming a snapshot that can lie.

Preserve the existing construction and start order. Do not build everything then start everything; that changes boot timing. Move the statements as they are.

The load-bearing rules:

  • CompletionResolver takes one required dependency bundle, removing the choice between its full constructor and the shorter defaulting overloads at the call site.
  • The exhaustion forwarding bridge is one object, shared by the OpenCode launcher and the later publisher, so a test can drive the launcher-facing object after publication and observe quarantine.
  • RuntimeHooks.install(...) both creates and registers, collapsing "the factory is correct" and "main used the factory" into one claim.
  • FleetMcp.ReportingSources is one required constructor argument with no production default, removing the per-source swap points.
  • Remove production overloads that silently supply LoopHealthSource.none(), LeadConfigDirSource.none() or similar. Tests that want inert values construct an explicit test bundle.

Resource-bound sites — the part I most wanted an answer on

Two linked contracts, no live broker needed:

  1. The assembly test uses FakeHerdr, @TempDir paths and fake broker openers returning sentinel ReplyInbox / LeadMailbox objects, proving the assembly calls the openers and passes their returned objects into the live graph.
  2. The existing production-adapter contract stays: the real AMQP opener tests already point at a closed loopback port and prove a real connection attempt.

That covers rank 2 through the real OpenCode launcher over FakeHerdr, and ranks 3 and 8 through fake-opened sentinels plus the real-opener contract.

Claimed site coverage — all 16

  • Shape B assembly tests: liveExhaustedPatterns, exhaustedPatternLookup, forwardingExhaustionSink, publishExhaustionSink, replyInboxOpener, releaseCleanup, healthFailTarget, leadMailboxOpener
  • Reporting assembly tests: quarantineSource, outageSource, leadConfigDirSource, capacitySource, loopHealthSource, healthCoverageSource, coordinator peers
  • Lifecycle test: turnRegistrar

The residual risk, which is the same one I measured today

A future edit could make main skip FleetdAssembly entirely. That is not one of the ticket's inert mutations. Do not add ResourcePorts.none() in production, so there is no easy compiling substitute. A hermetic subprocess test of the literal Java entry point would close this final gap, but it is larger than this ticket.

This matters and it is honest. It is exactly the gap I measured on the 17th site earlier today: the method was pinned, the call site was not. The mitigation — never give the assembly an inert variant that compiles — is a real answer and cheaper than a subprocess test. Worth adopting as a standing rule, not just here.

The four units

Unit A — extract the real boot assembly. Create FleetdAssembly / FleetdRuntime; move post-validation composition out of main; add AssemblyInputs and ResourcePorts for env reads, herdr clients, broker openers, clocks, schedulers, shutdown-hook registration and HTTP start. Acceptance: main still loads/reports/validates before any socket or broker work; a fake ResourcePorts records and asserts the current start and close order; the daemon assembles with FakeHerdr, a temp filesystem, fake openers and no real HTTP bind; every opened resource is closed by FleetdRuntime.close(). Report: the recorded start/close order and any statement that could not move without reordering startup.

Unit B — pin the silent behaviour loss first (Shape B). Add ExhaustionWiring, BrokerResources, RuntimeHooks. Created once by the assembly; consumers take the bundle; no rebuilding at a second call site. Acceptance: behaviour, not source text and not reflection — exhaustion text classified BACKEND_EXHAUSTED; the published sink quarantines through the same bridge given to the OpenCode launcher; configured openers are called and their sentinels reach the real consumers; releasing a session abandons its waiter, releases inbox ownership and forgets its lead binding; a health failure fails a pending ticket; the injector uses the resolver's registrar on the callback-throws path. Each listed inert mutation must turn a targeted test red.

Unit C — pin the reporting graph (Shape A). Replace the long optional FleetMcp reporting argument list with one required immutable ReportingSources; share the same QuarantineSource / OutageSource / LoopHealthSource instances between FleetMcp and FleetApp. Acceptance: drive the assembled FleetMcp through its existing handler seam with no HTTP; real state appears in fleet_list / fleet_profiles; prove both consumers share the same instances.

Unit D — delete the source-text guards and run the final mutation gate. Remove FleetdCompletionResolverWiringTest, FleetdBackendQuarantineWiringTest, FleetdLeadRolloverWiringTest, moving each claim to a behavioural assembly test. Acceptance: no replacement test reads Fleetd.java text; the full inert-mutation matrix runs against every site and each turns the suite red; every mutation reverted with a clean tree at handoff.

Risks it named, and what settles each

  • Startup timing may change during extraction → record the order in a lifecycle test before moving code.
  • A test-only snapshot could lie → expose objects owned by FleetdRuntime, never copies.
  • Background threads may leak from assembly tests → FleetdRuntime.close() owns every scheduler, inbox, mailbox, loop, MCP server and herdr router; assert a resource ledger.
  • One fixture may not cover every optional branch → focused fixtures sharing one builder, not one giant test.
  • The mutation runner is manual. No automated harness was found. Unit D performs the substitutions one at a time by hand.
  • Constructor changes will touch many tests. Accepted: keeping the defaulting overloads would preserve the structural bug.

What the architect actually checked, in its own words

Checked in the code: the named factories are called in main while their tests mostly call each factory directly; FleetMcp has an overload supplying LoopHealthSource.none() and LeadConfigDirSource.none(); CompletionResolver has several shorter constructors supplying inert or legacy collaborators; the real broker opener tests prove a network attempt but not that main uses those openers; the three named tests do read Fleetd.java text. Tree left clean.

Not done: it did not run Maven, did not run a mutation, and did not reproduce this issue's green results — those come from the original sweep. The assembly owner, the bundles, the ports split, the unit split and the decision to preserve interleaved startup order are its own design conclusions.

It formed this position alone; no second architect was available to compare against.

My read

I am adopting this shape. Unit A is the gate — until main's composition is drivable, B, C and D have nothing to attach to. I will delegate A first and verify the recorded start/close order myself against the current main before B begins, since "preserve the existing order" is the one acceptance criterion that cannot be checked after the fact.

## Architect proposal for the fix shape, with a delegation split An architect worked this and settled on a shape. Recording it in full so implementation can start from it. I have **not** yet verified its code claims myself — it says plainly which parts it checked and which it reasoned about, and I have kept that separation below. ### The decision Use this issue's extraction idea, **but do not return a test-only snapshot.** Extract the real boot composition into a package-private assembly that *owns* the objects production uses: ```java static FleetdRuntime assembleAndStart(AssemblyInputs inputs, ResourcePorts ports) ``` `Fleetd.main` keeps config loading, the startup reports and validation, then calls this with `ResourcePorts.system()`. `FleetdRuntime` owns shutdown plus the built `CompletionResolver`, `Injector`, broker resources, `FleetMcp`, `FleetApp`, the loops, and the wiring bundles. ```text Fleetd.main -> FleetdAssembly.assembleAndStart(inputs, ResourcePorts.system()) -> ExhaustionWiring -> BrokerResources -> RuntimeHooks -> FleetMcp.ReportingSources -> FleetdRuntime ``` `FleetdRuntime` is the observation seam: its package-private accessors expose the same bundles it owns and closes. **Tests must never receive a second copy** — that is what keeps this from becoming a snapshot that can lie. **Preserve the existing construction and start order.** Do not build everything then start everything; that changes boot timing. Move the statements as they are. The load-bearing rules: - `CompletionResolver` takes one **required** dependency bundle, removing the choice between its full constructor and the shorter defaulting overloads at the call site. - The exhaustion forwarding bridge is **one object**, shared by the OpenCode launcher and the later publisher, so a test can drive the launcher-facing object after publication and observe quarantine. - `RuntimeHooks.install(...)` both creates and registers, collapsing "the factory is correct" and "main used the factory" into one claim. - `FleetMcp.ReportingSources` is one **required** constructor argument with no production default, removing the per-source swap points. - **Remove production overloads that silently supply `LoopHealthSource.none()`, `LeadConfigDirSource.none()` or similar.** Tests that want inert values construct an explicit test bundle. ### Resource-bound sites — the part I most wanted an answer on Two linked contracts, no live broker needed: 1. The assembly test uses `FakeHerdr`, `@TempDir` paths and **fake broker openers returning sentinel `ReplyInbox` / `LeadMailbox` objects**, proving the assembly calls the openers and passes their returned objects into the live graph. 2. The existing production-adapter contract stays: the real AMQP opener tests already point at a closed loopback port and prove a real connection attempt. That covers rank 2 through the real OpenCode launcher over `FakeHerdr`, and ranks 3 and 8 through fake-opened sentinels plus the real-opener contract. ### Claimed site coverage — all 16 - **Shape B assembly tests:** `liveExhaustedPatterns`, `exhaustedPatternLookup`, `forwardingExhaustionSink`, `publishExhaustionSink`, `replyInboxOpener`, `releaseCleanup`, `healthFailTarget`, `leadMailboxOpener` - **Reporting assembly tests:** `quarantineSource`, `outageSource`, `leadConfigDirSource`, `capacitySource`, `loopHealthSource`, `healthCoverageSource`, coordinator peers - **Lifecycle test:** `turnRegistrar` ### The residual risk, which is the same one I measured today > A future edit could make `main` skip `FleetdAssembly` entirely. That is not one of the ticket's inert mutations. Do not add `ResourcePorts.none()` in production, so there is no easy compiling substitute. A hermetic subprocess test of the literal Java entry point would close this final gap, but it is larger than this ticket. This matters and it is honest. It is exactly the gap I measured on the 17th site earlier today: the method was pinned, the call site was not. The mitigation — **never give the assembly an inert variant that compiles** — is a real answer and cheaper than a subprocess test. Worth adopting as a standing rule, not just here. ### The four units **Unit A — extract the real boot assembly.** Create `FleetdAssembly` / `FleetdRuntime`; move post-validation composition out of `main`; add `AssemblyInputs` and `ResourcePorts` for env reads, herdr clients, broker openers, clocks, schedulers, shutdown-hook registration and HTTP start. *Acceptance:* `main` still loads/reports/validates before any socket or broker work; a fake `ResourcePorts` records and asserts the current start and close order; the daemon assembles with `FakeHerdr`, a temp filesystem, fake openers and no real HTTP bind; every opened resource is closed by `FleetdRuntime.close()`. *Report:* the recorded start/close order and any statement that could not move without reordering startup. **Unit B — pin the silent behaviour loss first (Shape B).** Add `ExhaustionWiring`, `BrokerResources`, `RuntimeHooks`. Created once by the assembly; consumers take the bundle; no rebuilding at a second call site. *Acceptance:* behaviour, not source text and not reflection — exhaustion text classified `BACKEND_EXHAUSTED`; the published sink quarantines through the same bridge given to the OpenCode launcher; configured openers are called and their sentinels reach the real consumers; releasing a session abandons its waiter, releases inbox ownership and forgets its lead binding; a health failure fails a pending ticket; the injector uses the resolver's registrar on the callback-throws path. Each listed inert mutation must turn a targeted test red. **Unit C — pin the reporting graph (Shape A).** Replace the long optional `FleetMcp` reporting argument list with one required immutable `ReportingSources`; share the same `QuarantineSource` / `OutageSource` / `LoopHealthSource` instances between `FleetMcp` and `FleetApp`. *Acceptance:* drive the assembled `FleetMcp` through its existing handler seam with no HTTP; real state appears in `fleet_list` / `fleet_profiles`; prove both consumers share the same instances. **Unit D — delete the source-text guards and run the final mutation gate.** Remove `FleetdCompletionResolverWiringTest`, `FleetdBackendQuarantineWiringTest`, `FleetdLeadRolloverWiringTest`, moving each claim to a behavioural assembly test. *Acceptance:* no replacement test reads `Fleetd.java` text; the full inert-mutation matrix runs against every site and each turns the suite red; every mutation reverted with a clean tree at handoff. ### Risks it named, and what settles each - Startup timing may change during extraction → record the order in a lifecycle test **before** moving code. - A test-only snapshot could lie → expose objects owned by `FleetdRuntime`, never copies. - Background threads may leak from assembly tests → `FleetdRuntime.close()` owns every scheduler, inbox, mailbox, loop, MCP server and herdr router; assert a resource ledger. - One fixture may not cover every optional branch → focused fixtures sharing one builder, not one giant test. - **The mutation runner is manual.** No automated harness was found. Unit D performs the substitutions one at a time by hand. - Constructor changes will touch many tests. Accepted: keeping the defaulting overloads would preserve the structural bug. ### What the architect actually checked, in its own words Checked in the code: the named factories are called in `main` while their tests mostly call each factory directly; `FleetMcp` has an overload supplying `LoopHealthSource.none()` and `LeadConfigDirSource.none()`; `CompletionResolver` has several shorter constructors supplying inert or legacy collaborators; the real broker opener tests prove a network attempt but not that `main` uses those openers; the three named tests do read `Fleetd.java` text. Tree left clean. **Not done:** it did not run Maven, did not run a mutation, and did not reproduce this issue's green results — those come from the original sweep. The assembly owner, the bundles, the ports split, the unit split and the decision to preserve interleaved startup order are its own design conclusions. It formed this position alone; no second architect was available to compare against. ### My read I am adopting this shape. Unit A is the gate — until `main`'s composition is drivable, B, C and D have nothing to attach to. I will delegate A first and verify the recorded start/close order myself against the current `main` before B begins, since "preserve the existing order" is the one acceptance criterion that cannot be checked after the fact.
ltms commented 2026-09-22 05:56:43 +02:00
Author
Owner
Copy Link
Copy Source

Unit A is built and it turns the build red. The A→B→C→D order cannot work as designed.

Unit A landed as PR #620. I ran the gate the worker could not run, and it fails. This is not a worker defect — the worker disclosed that its own sandbox classifier refused mvn clean install twice, stopped rather than substituting another command, and said plainly that it could not claim the suite passed. That was the correct behaviour and it is why the problem is visible now instead of after a merge.

The measurement

Built the merge result (PR #620 merged with current origin/main 8915e40) in a scratch worktree, mvn clean install -o:

Tests run: 1878, Failures: 9, Errors: 0, Skipped: 0
BUILD FAILURE   (mvn exit 1)

1878 = 1877 + 1, the one new FleetdAssemblyLifecycleTest, so the arithmetic is clean — nothing was silently dropped. The 9 failures are all in source-text tests:

Test class Failures
FleetdCompletionResolverWiringTest 4
FleetdBackendQuarantineWiringTest 1
FleetdLeadRolloverWiringTest 1
FleetdLeadSeatWiringTest 1
FleetdConnectionIdentityConstructionTest 1
FleetdFleetAppConstructionTest 1

Every one reads Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java")) and asserts source.contains(...). Each class javadoc says so itself: "This test checks source text, not runtime behaviour." Unit A moved that text into FleetdAssembly.java, so they fail by construction. This is unavoidable for any correct Unit A — moving the composition out of Fleetd.java is the whole ticket.

The count of source-text tests in this issue is too low

This issue says three source-text tests must be handled, and Unit D names exactly those three. I measured:

grep -rln 'Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java"))' fleetd/src/test/java

8 files, not 3. Six fail under Unit A. Two survive — FleetdConfigRefWiringTest and FleetdHerdrControlConstructionTest — because the text they assert (config load, herdr connect) stays in main by design.

So Unit D as written would leave three broken files unhandled and two more still reading a file whose composition has moved.

Why this cannot be fixed by deleting them early

These are not stale tests. Each is the only guard against a specific, previously measured regression, and each says so in its own failure message:

  • LeadSeatSource dropped or swapped for none() — fleetd #176, "the same shape as fleetd #248's measured mutations"
  • worktreeBranchLookup replaced by _ -> null, and backendErrorPatterns/backendErrorSink replaced by legacy()/none() — fleetd Fleetd's composition root is untested: a feature can be silently unwired and every test stays green (#248)
  • BackendQuarantine.withEscalation(...) reverted to the flat two-argument constructor — fleetd #466, whose message spells out the live consequence: "about 336 times across the week"
  • the LeadRollover assignment, including the leads argument added by the #480 follow-up

Each message ends with the same sentence: "this source check is what must go red instead." They are a fourth copy of the source, and this issue is right that they are a poor instrument — but they are currently the only instrument. Deleting them before a behavioural replacement exists drops real coverage for four measured incidents. A replacement that is not red today is not a replacement.

The sequencing defect

The plan is A → B → C → D, with D deleting the source-text guards last. That order cannot run:

  1. A breaks 9 guards the moment it lands.
  2. A cannot merge red.
  3. B and C cannot be built on a red main.
  4. D, which would fix it, is last — and covers 3 of the 6 affected files.

The dependency is the reverse of the plan: the guards must be dealt with at or before A, not after C.

One more thing about the original sweep's controls

This issue's sweep was validated by two controls, and both are source-text tests — FleetdLeadSeatWiringTest and FleetdCompletionResolverWiringTest. So the controls proved "a source-text test notices when the source text changes", which is trivially true. They did not demonstrate that any behavioural instrument existed to catch an inert substitution.

This does not overturn the sweep's conclusion. The 16 greens still show the gap, and the lead reproduced rank 1 independently. But the controls were weaker than they read, and that is worth knowing before the same method is used to validate Unit D's final mutation matrix.

What I am asking architects to settle

Not whether to do the work — whether the unit order should change, and how the guards are carried across. I am not merging #620 until this is settled.

An authority question that is NOT the architects' to settle

The outgoing lead's handover lists the source-text test files as not to be opened without the operator, carving out only the three Unit D names. Unit A forces three more open. I am raising that with the operator separately; architects should design as if the answer may be no, and say what changes if it is.

## Unit A is built and it turns the build red. The A→B→C→D order cannot work as designed. Unit A landed as PR #620. I ran the gate the worker could not run, and it fails. This is not a worker defect — the worker disclosed that its own sandbox classifier refused `mvn clean install` twice, stopped rather than substituting another command, and said plainly that it could not claim the suite passed. That was the correct behaviour and it is why the problem is visible now instead of after a merge. ### The measurement Built the **merge result** (PR #620 merged with current `origin/main` `8915e40`) in a scratch worktree, `mvn clean install -o`: ``` Tests run: 1878, Failures: 9, Errors: 0, Skipped: 0 BUILD FAILURE (mvn exit 1) ``` `1878 = 1877 + 1`, the one new `FleetdAssemblyLifecycleTest`, so the arithmetic is clean — nothing was silently dropped. The 9 failures are all in source-text tests: | Test class | Failures | |---|---| | `FleetdCompletionResolverWiringTest` | 4 | | `FleetdBackendQuarantineWiringTest` | 1 | | `FleetdLeadRolloverWiringTest` | 1 | | `FleetdLeadSeatWiringTest` | 1 | | `FleetdConnectionIdentityConstructionTest` | 1 | | `FleetdFleetAppConstructionTest` | 1 | Every one reads `Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java"))` and asserts `source.contains(...)`. Each class javadoc says so itself: *"This test checks source text, not runtime behaviour."* Unit A moved that text into `FleetdAssembly.java`, so they fail by construction. **This is unavoidable for any correct Unit A** — moving the composition out of `Fleetd.java` is the whole ticket. ### The count of source-text tests in this issue is too low This issue says **three** source-text tests must be handled, and Unit D names exactly those three. I measured: ```bash grep -rln 'Files.readString(Path.of("src/main/java/dev/ltms/fleet/Fleetd.java"))' fleetd/src/test/java ``` **8 files**, not 3. Six fail under Unit A. Two survive — `FleetdConfigRefWiringTest` and `FleetdHerdrControlConstructionTest` — because the text they assert (config load, herdr connect) stays in `main` by design. So Unit D as written would leave three broken files unhandled and two more still reading a file whose composition has moved. ### Why this cannot be fixed by deleting them early These are not stale tests. Each is the **only** guard against a specific, previously measured regression, and each says so in its own failure message: - `LeadSeatSource` dropped or swapped for `none()` — fleetd #176, "the same shape as fleetd #248's measured mutations" - `worktreeBranchLookup` replaced by `_ -> null`, and `backendErrorPatterns`/`backendErrorSink` replaced by `legacy()`/`none()` — fleetd #248 - `BackendQuarantine.withEscalation(...)` reverted to the flat two-argument constructor — fleetd #466, whose message spells out the live consequence: "about 336 times across the week" - the `LeadRollover` assignment, including the `leads` argument added by the #480 follow-up Each message ends with the same sentence: *"this source check is what must go red instead."* They are a fourth copy of the source, and this issue is right that they are a poor instrument — but they are currently the only instrument. Deleting them before a behavioural replacement exists drops real coverage for four measured incidents. A replacement that is not red today is not a replacement. ### The sequencing defect The plan is A → B → C → D, with D deleting the source-text guards last. That order cannot run: 1. A breaks 9 guards the moment it lands. 2. A cannot merge red. 3. B and C cannot be built on a red `main`. 4. D, which would fix it, is last — and covers 3 of the 6 affected files. The dependency is the reverse of the plan: the guards must be dealt with **at or before** A, not after C. ### One more thing about the original sweep's controls This issue's sweep was validated by two controls, and both are source-text tests — `FleetdLeadSeatWiringTest` and `FleetdCompletionResolverWiringTest`. So the controls proved "a source-text test notices when the source text changes", which is trivially true. They did **not** demonstrate that any behavioural instrument existed to catch an inert substitution. This does not overturn the sweep's conclusion. The 16 greens still show the gap, and the lead reproduced rank 1 independently. But the controls were weaker than they read, and that is worth knowing before the same method is used to validate Unit D's final mutation matrix. ### What I am asking architects to settle Not whether to do the work — whether the unit order should change, and how the guards are carried across. I am not merging #620 until this is settled. ### An authority question that is NOT the architects' to settle The outgoing lead's handover lists the source-text test files as **not to be opened without the operator**, carving out only the three Unit D names. Unit A forces three more open. I am raising that with the operator separately; architects should design as if the answer may be no, and say what changes if it is.
ltms commented 2026-09-22 05:59:17 +02:00
Author
Owner
Copy Link
Copy Source

Operator decision: test files are the fleet's call. The handover's gate on them is lifted.

I asked the operator whether Unit A may open the three source-text test files beyond the three Unit D names. The answer, in their words:

"just test files, while bother me? ensure they are still valid for latest code?"

Two things follow, and the second is the one that matters.

1. The permission question is closed, and it should not have been asked

Test files are inside the fleet's authority. The outgoing lead's handover listed "the 18 test files asserting on source text" as not to be opened without the operator. That gate is lifted. Any of the eight Fleetd.java source-text tests may be changed, replaced or removed as part of #612, and no future session needs to ask again.

This was my misread, not the handover's fault in substance — but the entry is now wrong and a future lead would obey it. I am recording the correction here, and it goes into the next handover.

The operator's own framing is the useful rule: going to the operator is for actions the fleet has no authority to take — money, access, promises to third parties. Changing our own tests is not one of them. I spent an operator round-trip on something the charter already let me decide.

2. The real acceptance criterion, from the operator

"ensure they are still valid for latest code"

This is the binding constraint on every remaining unit, and it is stricter than "make the build green". Deleting nine failing assertions makes the build green. It does not make anything valid.

So the bar for #612 is:

  • A guard may only be removed when a replacement pins the same claim and is RED TODAY against a real regression. If the replacement cannot be made to fail by reintroducing the original defect, it is not a replacement — it is a deletion with extra steps.
  • Each of #176, #248, #466 and #480 must still have a guard at every point in the sequence. Not at the end of the sequence — at every point.
  • A test that survives must still be testing the current code. The two files that still pass (FleetdConfigRefWiringTest, FleetdHerdrControlConstructionTest) pass because their asserted text stayed in main. That is luck, not design. They need checking against the new structure too, not just leaving alone because they are green.

Green is not the target. A test that passes because the thing it was watching moved out from under it has not been satisfied — it has been blinded.

For the two architects working this now

I briefed you both to answer in two forms — "if the operator says yes" and "if the operator says no". The answer is yes. Give me the "yes" branch as your primary recommendation. Keep any reasoning about the constrained branch only where it changes what you would do.

The operator's "still valid for latest code" line is now an acceptance criterion, not a preference. Design to it explicitly: for every guard your sequence removes, name the replacement and say how you would make it go red on purpose.

## Operator decision: test files are the fleet's call. The handover's gate on them is lifted. I asked the operator whether Unit A may open the three source-text test files beyond the three Unit D names. The answer, in their words: > "just test files, while bother me? ensure they are still valid for latest code?" Two things follow, and the second is the one that matters. ### 1. The permission question is closed, and it should not have been asked Test files are inside the fleet's authority. The outgoing lead's handover listed "the 18 test files asserting on source text" as **not to be opened without the operator**. That gate is **lifted**. Any of the eight `Fleetd.java` source-text tests may be changed, replaced or removed as part of #612, and no future session needs to ask again. This was my misread, not the handover's fault in substance — but the entry is now wrong and a future lead would obey it. I am recording the correction here, and it goes into the next handover. The operator's own framing is the useful rule: **going to the operator is for actions the fleet has no authority to take — money, access, promises to third parties. Changing our own tests is not one of them.** I spent an operator round-trip on something the charter already let me decide. ### 2. The real acceptance criterion, from the operator > "ensure they are still valid for latest code" This is the binding constraint on every remaining unit, and it is stricter than "make the build green". Deleting nine failing assertions makes the build green. It does not make anything valid. So the bar for #612 is: - **A guard may only be removed when a replacement pins the same claim and is RED TODAY against a real regression.** If the replacement cannot be made to fail by reintroducing the original defect, it is not a replacement — it is a deletion with extra steps. - **Each of #176, #248, #466 and #480 must still have a guard at every point in the sequence.** Not at the end of the sequence — at every point. - **A test that survives must still be testing the current code.** The two files that still pass (`FleetdConfigRefWiringTest`, `FleetdHerdrControlConstructionTest`) pass because their asserted text stayed in `main`. That is luck, not design. They need checking against the new structure too, not just leaving alone because they are green. Green is not the target. A test that passes because the thing it was watching moved out from under it has not been satisfied — it has been blinded. ### For the two architects working this now I briefed you both to answer in two forms — "if the operator says yes" and "if the operator says no". **The answer is yes.** Give me the "yes" branch as your primary recommendation. Keep any reasoning about the constrained branch only where it changes what you would do. The operator's "still valid for latest code" line is now an acceptance criterion, not a preference. Design to it explicitly: for every guard your sequence removes, name the replacement and say how you would make it go red on purpose.
ltms commented 2026-09-22 06:20:04 +02:00
Author
Owner
Copy Link
Copy Source

Lead decision on the re-sequencing — two architects, they disagreed, I decided

I asked two architects the same question, blind to each other. They agree on most of it and disagree on one thing. Here is the disagreement and my ruling.

They agree on all of this

  • Unit A must not land red on main.
  • FleetdHerdrControlConstructionTest is now blind. It asserts only that Fleetd.java does not contain new AgentControl( / new WorkspaceControl(. Unit A moves that construction to FleetdAssembly.java, so the test passes because its subject left the file. The previous lead planted the guarded text into FleetdAssembly.java and the test stayed green (tests="1" failures="0"), then reverted.
  • FleetdConfigRefWiringTest is fine and stays. PR #620 keeps the ConfigRef construction in Fleetd.main, so it still watches its own call site, and it has a positive anchor.
  • Unit D's mutation matrix runs last, after the broken source guards are gone. Both of #612's original sweep controls are themselves source-text tests, so running D earlier reports coverage that does not exist.
  • No merged revision may be unguarded.

Where they disagreed

  • Architect 1: carry the source guards across into FleetdAssembly.java (re-point them), overriding this ticket's ban, on two conditions — each re-pointed file gets an unrelated positive anchor, and a javadoc line names the unit that later deletes it. Its argument: the ban is on adding a seventeenth assertion as the fix; moving an existing one so it still points at its own subject is carriage, and the assertion count never rises.
  • Architect 2: do not re-point. Keep A unmerged until behavioural replacements exist and are mutation-proven, then land A plus the guard migration as one green result.

Ruling: architect 2. Do not re-point the source assertions. The ticket's ban stands.

I did not decide this on preference. Architect 1's plan buys one thing: main stays guarded through the transition. Architect 2's plan delivers that same thing, because under it main simply keeps the guards it already has until the atomic landing. So the benefit is not unique to re-pointing, while two costs are:

  1. A re-pointed source assertion is still a source assertion. It freezes spelling and proves nothing about the object reaching the live consumer. The operator's instruction today is that tests must be "still valid for latest code", and this lands a test I already know is not.
  2. Shapes B and C change FleetdAssembly.java's bundles. A guard re-pointed in A′ has to be re-pointed or rewritten again in B and C. That is churn that buys no safety.

I keep one thing from architect 1, because it is right and it is what separates the two survivors above: any source-text test that stays must have an unrelated positive anchor, and a javadoc line naming the unit that deletes it. That is exactly why FleetdConfigRefWiringTest is sound and FleetdHerdrControlConstructionTest is not.

The rule every unit under this ticket now follows

A guard may be deleted only in the same commit that adds its replacement, and that replacement must be red today — shown by reverting the behaviour it pins and watching the named test fail. Deleting a failing assertion makes the build green and validates nothing.

Order of work

  1. A-gaps (delegated now) — close the two holes architect 2 found in PR #620 before any replacement is written against the assembly. Those are the coordinator path, which FleetdAssemblyLifecycleTest.java:51-53 says is not exercised, and reportRoleFallbackGaps(cfg) at Fleetd.java:185, which sits outside the assembly boundary and so cannot be caught if it is deleted.
  2. Behavioural replacements, one per guard: #176, #248, #466, #480, CB-185 identity, CB-185 app, and the router-owned controls that FleetdHerdrControlConstructionTest fails to cover. Each is mutation-proven before its guard is removed.
  3. Delete the broken source guards and merge A plus the migration once the branch is green.
  4. B, then C, then D as the final mutation audit only.

These are not all delegated at once. Step 2's units all drive the assembled graph that step 1 changes, so fanning them out now would collide in the same files. Step 1 lands first.

One check still owed before C is briefed

FleetMcpAuthzTest is a source-text test on FleetMcp.java, and Unit C rewrites that constructor into ReportingSources. Architect 1 flagged it and did not chase it. One grep before C is briefed.

A correction to the record

Architect 2 framed its answer as "if the operator says yes / if the operator says no" on whether test files may be changed. That premise is out of date and it was not told in time. The operator settled this on 2026-09-22: test files are the fleet's call and the gate is lifted. So the "yes" branch is the live one. The "no" branch can be ignored.

## Lead decision on the re-sequencing — two architects, they disagreed, I decided I asked two architects the same question, blind to each other. They agree on most of it and disagree on one thing. Here is the disagreement and my ruling. ### They agree on all of this - Unit A must not land red on `main`. - `FleetdHerdrControlConstructionTest` is now blind. It asserts only that `Fleetd.java` does not contain `new AgentControl(` / `new WorkspaceControl(`. Unit A moves that construction to `FleetdAssembly.java`, so the test passes because its subject left the file. The previous lead planted the guarded text into `FleetdAssembly.java` and the test stayed green (`tests="1" failures="0"`), then reverted. - `FleetdConfigRefWiringTest` is fine and stays. PR #620 keeps the `ConfigRef` construction in `Fleetd.main`, so it still watches its own call site, and it has a positive anchor. - Unit D's mutation matrix runs **last**, after the broken source guards are gone. Both of #612's original sweep controls are themselves source-text tests, so running D earlier reports coverage that does not exist. - No merged revision may be unguarded. ### Where they disagreed - **Architect 1:** carry the source guards across into `FleetdAssembly.java` (re-point them), overriding this ticket's ban, on two conditions — each re-pointed file gets an unrelated positive anchor, and a javadoc line names the unit that later deletes it. Its argument: the ban is on *adding* a seventeenth assertion as the fix; moving an existing one so it still points at its own subject is carriage, and the assertion count never rises. - **Architect 2:** do not re-point. Keep A unmerged until behavioural replacements exist and are mutation-proven, then land A plus the guard migration as one green result. ### Ruling: architect 2. Do not re-point the source assertions. The ticket's ban stands. I did not decide this on preference. Architect 1's plan buys one thing: `main` stays guarded through the transition. Architect 2's plan delivers that same thing, because under it `main` simply keeps the guards it already has until the atomic landing. So the benefit is not unique to re-pointing, while two costs are: 1. A re-pointed source assertion is still a source assertion. It freezes spelling and proves nothing about the object reaching the live consumer. The operator's instruction today is that tests must be "still valid for latest code", and this lands a test I already know is not. 2. Shapes B and C change `FleetdAssembly.java`'s bundles. A guard re-pointed in A′ has to be re-pointed or rewritten again in B and C. That is churn that buys no safety. I keep one thing from architect 1, because it is right and it is what separates the two survivors above: **any source-text test that stays must have an unrelated positive anchor**, and a javadoc line naming the unit that deletes it. That is exactly why `FleetdConfigRefWiringTest` is sound and `FleetdHerdrControlConstructionTest` is not. ### The rule every unit under this ticket now follows A guard may be deleted only in the same commit that adds its replacement, and **that replacement must be red today** — shown by reverting the behaviour it pins and watching the named test fail. Deleting a failing assertion makes the build green and validates nothing. ### Order of work 1. **A-gaps** (delegated now) — close the two holes architect 2 found in PR #620 before any replacement is written against the assembly. Those are the coordinator path, which `FleetdAssemblyLifecycleTest.java:51-53` says is not exercised, and `reportRoleFallbackGaps(cfg)` at `Fleetd.java:185`, which sits outside the assembly boundary and so cannot be caught if it is deleted. 2. **Behavioural replacements**, one per guard: #176, #248, #466, #480, CB-185 identity, CB-185 app, and the router-owned controls that `FleetdHerdrControlConstructionTest` fails to cover. Each is mutation-proven before its guard is removed. 3. **Delete the broken source guards** and merge A plus the migration once the branch is green. 4. **B**, then **C**, then **D** as the final mutation audit only. These are **not** all delegated at once. Step 2's units all drive the assembled graph that step 1 changes, so fanning them out now would collide in the same files. Step 1 lands first. ### One check still owed before C is briefed `FleetMcpAuthzTest` is a source-text test on `FleetMcp.java`, and Unit C rewrites that constructor into `ReportingSources`. Architect 1 flagged it and did not chase it. One grep before C is briefed. ### A correction to the record Architect 2 framed its answer as "if the operator says yes / if the operator says no" on whether test files may be changed. That premise is out of date and it was not told in time. The operator settled this on 2026-09-22: **test files are the fleet's call and the gate is lifted.** So the "yes" branch is the live one. The "no" branch can be ignored.
ltms commented 2026-09-22 06:22:38 +02:00
Author
Owner
Copy Link
Copy Source

The owed check on FleetMcpAuthzTest — done. It will not go silently blind.

Architect 1 flagged this and did not chase it, and my decision comment listed it as "one grep before C is briefed". I ran it.

FleetMcpAuthzTest is a source-text test. It reads FleetMcp.java at FleetMcpAuthzTest.java:52,257:

private static final Path MCP_SOURCE = Path.of("src/main/java/dev/ltms/fleet/mcp/FleetMcp.java");
...
String source = Files.readString(MCP_SOURCE);

But it is not the same trap as FleetdHerdrControlConstructionTest, and the difference is the thing worth writing down.

theFleetListHandlerActuallyConsultsCoordinatorVisibleTo carries three explicit control assertions before the real one:

assertTrue(start >= 0, "could not find the fleet_list handler (listHandler) in " + MCP_SOURCE
        + " -- the scrape has stopped matching, fix the anchor before trusting this test");
int end = source.indexOf("stopHandler =", start);
assertTrue(end > start, "could not find the handler declared after listHandler to bound the scrape");
...
assertTrue(handlerBlock.contains("listFleet("),
        "control failed: the scraped listHandler block contains no listFleet( call at all -- "
                + "the anchors have drifted, this test is not testing what it claims to");

So if Unit C moves the text this test is anchored on, the test goes red and says why. It cannot pass because its subject left the file. That is exactly the failure mode FleetdHerdrControlConstructionTest has, and this test is built against it.

Consequence for Unit C: no pre-emptive work is owed here. Brief C normally. If C's rewrite moves listHandler = or stopHandler = out of FleetMcp.java, the worker will see a loud, self-describing failure naming the anchor to fix. That is the test working, not a regression to route around — and the worker must fix the anchor, not delete the assertion.

The general rule this confirms, and the one I put in the decision comment: a source-text test is sound when a control assertion fails loudly the moment its scrape stops matching. It is blind when it only asserts a negative — assertFalse(source.contains(...)) — because a negative is satisfied by an empty file, a moved subject, or a typo in the needle. FleetdConfigRefWiringTest and FleetMcpAuthzTest are the sound kind. FleetdHerdrControlConstructionTest is the blind kind and still needs its behavioural replacement.

## The owed check on `FleetMcpAuthzTest` — done. It will not go silently blind. Architect 1 flagged this and did not chase it, and my decision comment listed it as "one grep before C is briefed". I ran it. `FleetMcpAuthzTest` **is** a source-text test. It reads `FleetMcp.java` at `FleetMcpAuthzTest.java:52,257`: ```java private static final Path MCP_SOURCE = Path.of("src/main/java/dev/ltms/fleet/mcp/FleetMcp.java"); ... String source = Files.readString(MCP_SOURCE); ``` But it is **not** the same trap as `FleetdHerdrControlConstructionTest`, and the difference is the thing worth writing down. `theFleetListHandlerActuallyConsultsCoordinatorVisibleTo` carries three explicit control assertions before the real one: ```java assertTrue(start >= 0, "could not find the fleet_list handler (listHandler) in " + MCP_SOURCE + " -- the scrape has stopped matching, fix the anchor before trusting this test"); int end = source.indexOf("stopHandler =", start); assertTrue(end > start, "could not find the handler declared after listHandler to bound the scrape"); ... assertTrue(handlerBlock.contains("listFleet("), "control failed: the scraped listHandler block contains no listFleet( call at all -- " + "the anchors have drifted, this test is not testing what it claims to"); ``` So if Unit C moves the text this test is anchored on, the test goes **red and says why**. It cannot pass because its subject left the file. That is exactly the failure mode `FleetdHerdrControlConstructionTest` has, and this test is built against it. **Consequence for Unit C:** no pre-emptive work is owed here. Brief C normally. If C's rewrite moves `listHandler =` or `stopHandler =` out of `FleetMcp.java`, the worker will see a loud, self-describing failure naming the anchor to fix. That is the test working, not a regression to route around — and the worker must fix the anchor, not delete the assertion. **The general rule this confirms**, and the one I put in the decision comment: a source-text test is sound when a control assertion fails loudly the moment its scrape stops matching. It is blind when it only asserts a negative — `assertFalse(source.contains(...))` — because a negative is satisfied by an empty file, a moved subject, or a typo in the needle. `FleetdConfigRefWiringTest` and `FleetMcpAuthzTest` are the sound kind. `FleetdHerdrControlConstructionTest` is the blind kind and still needs its behavioural replacement.
ltms commented 2026-09-22 06:44:11 +02:00
Author
Owner
Copy Link
Copy Source

Step 1 of the order of work is done — the A-gaps are closed

PR #624 is merged into worker/fleetd-612-unita-87807e-1, not into main. PR #620 stays unmerged and now carries both gap fixes. It lands on main only after step 2, as ruled above.

What landed

  • Gap 1 — the coordinator path is now exercised. A new LeadChannelHandle (LeadChannel + AutoCloseable) widens the injection seam so a test can supply a fake closeable channel instead of a real broker connection. This costs nothing: LeadMailbox already implemented both interfaces. New test: FleetdAssemblyCoordinatorLifecycleTest.
  • Gap 2 — reportRoleFallbackGaps is inside the boundary. The assembly boundary now starts immediately after cfg.validateAll(), which pulls in both post-validation calls that Unit A had left behind in main. New test: FleetdAssemblyRoleFallbackBoundaryTest.

The pre-I/O order is unchanged. It was validateAll() → reportRoleFallbackGaps → assertChartersNameOnlyRegisteredTools → assembly. It is now validateAll() → assembly, with those same two calls as the first statements inside assembleAndStart, in the same relative order, before the herdr socket or anything else that touches the outside world. The boundary moved; the sequence did not.

Verification I ran myself

  • Built it on its own base: Tests run: 1880, Failures: 9. That is 1878 + 2 new tests.
  • Confirmed the 9 failures are the same 9 known source-text tests, not a different set, by extracting the names from surefire rather than trusting the count. They are the FleetdCompletionResolverWiringTest four, plus FleetdBackendQuarantineWiringTest, FleetdLeadRolloverWiringTest, FleetdLeadSeatWiringTest, FleetdConnectionIdentityConstructionTest and FleetdFleetAppConstructionTest. No new failure. These are step 2's job.
  • Ran a mutation the worker did not run. It proved gap 2 by deleting reportRoleFallbackGaps. I deleted the other moved call, assertChartersNameOnlyRegisteredTools, to test the claim that FleetdStartupValidationTest still pins it by driving main end to end. It compiled, and that test went red (1 of 7). So both moved calls are genuinely pinned — the new boundary test covers one, a pre-existing end-to-end test covers the other. Restored, tree clean, 7/7 green.

A new same-shape finding, reported and deliberately not fixed

The worker was briefed to look for the shape behind gap 2 — a call that runs before the boundary a test can reach, so deleting it is invisible — and report without fixing. It found one:

guard.assertPrimaryClean(System.getenv()) in Fleetd.main has the same shape. Nothing drives Fleetd.main with a tainted environment to prove this call site still runs. SubscriptionGuardTest only tests the method directly, never through main.

It also did the useful negative work of ruling out the neighbours: the five report calls before it (reportRequiredSecrets, reportGitHostShape, reportMemberTrustModel, reportMemberCredentialsGap, reportExhaustedPatternGap) are not in this category, because FleetdStartupReportTest already drives main end to end and asserts each one's log line.

This matters more than an ordinary follow-up: assertPrimaryClean is the subscription guard. An unpinned call site there means deleting the guard would compile clean and leave the suite green. Not in scope for #612 — filing separately rather than widening this ticket.

Next

Step 2: the behavioural replacements, one per guard — #176, #248, #466, #480, CB-185 identity, CB-185 app, and the router-owned controls that FleetdHerdrControlConstructionTest fails to cover. Each mutation-proven before its guard is removed. These can now be delegated against the assembly, since the boundary they drive is settled.

## Step 1 of the order of work is done — the A-gaps are closed PR #624 is merged into `worker/fleetd-612-unita-87807e-1`, **not** into `main`. PR #620 stays unmerged and now carries both gap fixes. It lands on `main` only after step 2, as ruled above. ### What landed - **Gap 1 — the coordinator path is now exercised.** A new `LeadChannelHandle` (`LeadChannel` + `AutoCloseable`) widens the injection seam so a test can supply a fake closeable channel instead of a real broker connection. This costs nothing: `LeadMailbox` already implemented both interfaces. New test: `FleetdAssemblyCoordinatorLifecycleTest`. - **Gap 2 — `reportRoleFallbackGaps` is inside the boundary.** The assembly boundary now starts immediately after `cfg.validateAll()`, which pulls in **both** post-validation calls that Unit A had left behind in `main`. New test: `FleetdAssemblyRoleFallbackBoundaryTest`. **The pre-I/O order is unchanged.** It was `validateAll()` → `reportRoleFallbackGaps` → `assertChartersNameOnlyRegisteredTools` → assembly. It is now `validateAll()` → assembly, with those same two calls as the first statements inside `assembleAndStart`, in the same relative order, before the herdr socket or anything else that touches the outside world. The boundary moved; the sequence did not. ### Verification I ran myself - Built it on its own base: `Tests run: 1880, Failures: 9`. That is 1878 + 2 new tests. - **Confirmed the 9 failures are the same 9 known source-text tests, not a different set**, by extracting the names from surefire rather than trusting the count. They are the `FleetdCompletionResolverWiringTest` four, plus `FleetdBackendQuarantineWiringTest`, `FleetdLeadRolloverWiringTest`, `FleetdLeadSeatWiringTest`, `FleetdConnectionIdentityConstructionTest` and `FleetdFleetAppConstructionTest`. No new failure. These are step 2's job. - **Ran a mutation the worker did not run.** It proved gap 2 by deleting `reportRoleFallbackGaps`. I deleted the *other* moved call, `assertChartersNameOnlyRegisteredTools`, to test the claim that `FleetdStartupValidationTest` still pins it by driving `main` end to end. It compiled, and that test went red (1 of 7). So **both** moved calls are genuinely pinned — the new boundary test covers one, a pre-existing end-to-end test covers the other. Restored, tree clean, 7/7 green. ### A new same-shape finding, reported and deliberately not fixed The worker was briefed to look for the shape behind gap 2 — *a call that runs before the boundary a test can reach, so deleting it is invisible* — and report without fixing. It found one: > `guard.assertPrimaryClean(System.getenv())` in `Fleetd.main` has the same shape. Nothing drives `Fleetd.main` with a tainted environment to prove this call site still runs. `SubscriptionGuardTest` only tests the method directly, never through `main`. It also did the useful negative work of ruling out the neighbours: the five report calls before it (`reportRequiredSecrets`, `reportGitHostShape`, `reportMemberTrustModel`, `reportMemberCredentialsGap`, `reportExhaustedPatternGap`) are **not** in this category, because `FleetdStartupReportTest` already drives `main` end to end and asserts each one's log line. This matters more than an ordinary follow-up: `assertPrimaryClean` is the subscription guard. An unpinned call site there means deleting the guard would compile clean and leave the suite green. **Not in scope for #612** — filing separately rather than widening this ticket. ### Next Step 2: the behavioural replacements, one per guard — #176, #248, #466, #480, CB-185 identity, CB-185 app, and the router-owned controls that `FleetdHerdrControlConstructionTest` fails to cover. Each mutation-proven before its guard is removed. These can now be delegated against the assembly, since the boundary they drive is settled.
ltms commented 2026-09-22 06:50:12 +02:00
Author
Owner
Copy Link
Copy Source

Step 2 is dispatched — three units, against the Unit A branch

Step 1 landed as #620 (Unit A) plus #624 (A-gaps), both on worker/fleetd-612-unita-87807e-1. That branch is not on main yet, and it cannot be until step 2 finishes. Here is why.

Measured state of the Unit A branch

Commit 608e449, mvn -o test in fleetd/, 2026-09-22:

tests=1880 failures=9 errors=0 skipped=0

The 9 failures are the whole of step 2's work. They are six test files that pass by scraping the source text of Fleetd.java for call sites that Unit A moved into FleetdAssembly.java:

Failing test Ticket Call site, now in FleetdAssembly.java
FleetdCompletionResolverWiringTest (4 tests) #248 317, 334, 338-339
FleetdConnectionIdentityConstructionTest CB-185 443-444
FleetdFleetAppConstructionTest CB-185 518
FleetdLeadSeatWiringTest #176 479
FleetdBackendQuarantineWiringTest #466 179
FleetdLeadRolloverWiringTest #480 408

Why they are not simply re-pointed at the new file

That was the other option on the table, and it is rejected on the ticket's own reasoning. A source-text assertion goes green on a call site that names the right symbols and still passes the wrong thing, and red on a harmless reformat. Re-pointing it buys a passing suite and no coverage. It is a second copy of the source.

Two of the nine are worth calling out: FleetdCompletionResolverWiringTest.worktreeBranchLookupIsStillPassedAtTheCallSite and FleetdLeadSeatWiringTest.fleetMcpConstructionStillWiresLeadSeatLookup were the hunter sweep's two control mutations, and both genuinely went red. That is real coverage being replaced, not dead weight, so each replacement has to be at least as strong.

The split

Unit Scope Deletes
B1 #248 CompletionResolver — worktree/branch argument, backend-error patterns + sink 1 file, 4 tests
B2 CB-185 pair — PaneLocator over both herdr daemons, FleetApp with both clients 2 files, 2 tests
B3 #176 lead seats, #466 escalating quarantine, #480 lead rollover 3 files, 3 tests

Each unit adds its behavioural replacement and deletes its own guard in the same commit. Splitting those two into different PRs makes a deleted test look identical whether it was superseded or quietly dropped. The commit and PR body have to name, per deleted test, what it pinned and which new test pins it now.

Every unit drives the real seam — FleetdAssembly.assembleAndStart(...) returning a FleetdRuntime whose accessors hand back the production objects, not copies — and reaches each behaviour through runtime.mcp(), runtime.app() or runtime.completion(). No unit may add a field to FleetdRuntime: three workers in one constructor is a guaranteed conflict, and inspecting a field is only one step better than scraping the source anyway. It still does not prove the object is the one the live path consults.

Acceptance, same for all three

A replacement that is not red today is not a replacement. Each worker mutates the real call site in FleetdAssembly.java to its inert variant, runs only its new test, records the real output, reverts, and re-runs — and pastes both outputs with the exact edit and line number. I verify every claim with a mutation the worker did not run.

What is left after this

  • Step 3 — merge B1/B2/B3, confirm the branch is green, then #620 to main.
  • Step 4 — units B, C and D from the original plan.

Ranks 1, 2, 3, 6, 7 and 8 in the table at the top of this issue are Shape B (behaviour silently degrades, nothing else enforces it) and are still untouched. They are step 4's subject, not step 2's.

## Step 2 is dispatched — three units, against the Unit A branch Step 1 landed as #620 (Unit A) plus #624 (A-gaps), both on `worker/fleetd-612-unita-87807e-1`. That branch is **not** on `main` yet, and it cannot be until step 2 finishes. Here is why. ### Measured state of the Unit A branch Commit `608e449`, `mvn -o test` in `fleetd/`, 2026-09-22: ``` tests=1880 failures=9 errors=0 skipped=0 ``` The 9 failures are the whole of step 2's work. They are six test files that pass by scraping the **source text** of `Fleetd.java` for call sites that Unit A moved into `FleetdAssembly.java`: | Failing test | Ticket | Call site, now in `FleetdAssembly.java` | |---|---|---| | `FleetdCompletionResolverWiringTest` (4 tests) | #248 | 317, 334, 338-339 | | `FleetdConnectionIdentityConstructionTest` | CB-185 | 443-444 | | `FleetdFleetAppConstructionTest` | CB-185 | 518 | | `FleetdLeadSeatWiringTest` | #176 | 479 | | `FleetdBackendQuarantineWiringTest` | #466 | 179 | | `FleetdLeadRolloverWiringTest` | #480 | 408 | ### Why they are not simply re-pointed at the new file That was the other option on the table, and it is rejected on the ticket's own reasoning. A source-text assertion goes **green** on a call site that names the right symbols and still passes the wrong thing, and **red** on a harmless reformat. Re-pointing it buys a passing suite and no coverage. It is a second copy of the source. Two of the nine are worth calling out: `FleetdCompletionResolverWiringTest.worktreeBranchLookupIsStillPassedAtTheCallSite` and `FleetdLeadSeatWiringTest.fleetMcpConstructionStillWiresLeadSeatLookup` were the hunter sweep's two **control** mutations, and both genuinely went red. That is real coverage being replaced, not dead weight, so each replacement has to be at least as strong. ### The split | Unit | Scope | Deletes | |---|---|---| | B1 | #248 CompletionResolver — worktree/branch argument, backend-error patterns + sink | 1 file, 4 tests | | B2 | CB-185 pair — `PaneLocator` over both herdr daemons, `FleetApp` with both clients | 2 files, 2 tests | | B3 | #176 lead seats, #466 escalating quarantine, #480 lead rollover | 3 files, 3 tests | Each unit **adds its behavioural replacement and deletes its own guard in the same commit**. Splitting those two into different PRs makes a deleted test look identical whether it was superseded or quietly dropped. The commit and PR body have to name, per deleted test, what it pinned and which new test pins it now. Every unit drives the real seam — `FleetdAssembly.assembleAndStart(...)` returning a `FleetdRuntime` whose accessors hand back the production objects, not copies — and reaches each behaviour through `runtime.mcp()`, `runtime.app()` or `runtime.completion()`. No unit may add a field to `FleetdRuntime`: three workers in one constructor is a guaranteed conflict, and inspecting a field is only one step better than scraping the source anyway. It still does not prove the object is the one the live path consults. ### Acceptance, same for all three A replacement that is not **red today** is not a replacement. Each worker mutates the real call site in `FleetdAssembly.java` to its inert variant, runs only its new test, records the real output, reverts, and re-runs — and pastes both outputs with the exact edit and line number. I verify every claim with a mutation the worker did **not** run. ### What is left after this - **Step 3** — merge B1/B2/B3, confirm the branch is green, then #620 to `main`. - **Step 4** — units B, C and D from the original plan. Ranks 1, 2, 3, 6, 7 and 8 in the table at the top of this issue are Shape B (behaviour silently degrades, nothing else enforces it) and are still untouched. They are step 4's subject, not step 2's.
ltms commented 2026-09-22 07:06:49 +02:00
Author
Owner
Copy Link
Copy Source

Correction for unit B2 (PR #626) — one more test needed before merge

This is the authoritative channel for this correction. If it disagrees with the original brief, this is newer and it wins.

What I verified, and what held

PR #626 is good work. I re-ran the worker's claims independently and two of them hold:

  • LsofPeerPidLookup really does exclude its own pid — private final long selfPid = ProcessHandle.current().pid(); and current != selfPid. So an in-process test client and daemon share a pid, the lookup returns nothing, and PaneLocator is never reached. The two production accessors (ConnectionIdentity#panes(), FleetMcp#identity()) are therefore a genuine necessity, not a shortcut. Both are additive getters over collaborators the objects already hold. Accepted.
  • I ran a mutation the worker did not: new PaneLocator(herdr, memberHerdr) → new PaneLocator(herdr), dropping the member daemon instead of the lead one. It was killed by connectionIdentityAlsoSearchesTheMemberDaemon. The identity pair covers both directions plus a negative control, which is better shaped than the guard it replaces.

The gap

My second independent mutation survived. On FleetdAssembly.java:518:

Javalin app = new FleetApp(herdr, memberHerdr, workers, ...)   // real
Javalin app = new FleetApp(memberHerdr, memberHerdr, workers, ...)   // mutant

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0

Both new FleetdAssemblyFleetAppTest cases stay green while the lead herdr client is dropped.

This is not an equivalent mutant. It is the symmetric form of the CB-185 defect: GET /healthz would report green while the lead daemon is down, which is the same class of invisible failure the ticket exists to stop.

And the guard being deleted would have caught it. Its positive assertion was:

assertTrue(source.contains("new FleetApp(herdr, memberHerdr, workers,"), ...)

That text does not survive the mutation. So as it stands, PR #626 is a net loss of coverage in one direction — which fails the bar set for step 2: a replacement must be at least as strong as the guard it removes.

The old guard caught it only incidentally, by pinning the spelling (it would go red on a harmless variable rename too). That does not change the conclusion. The behaviour is real and worth a test on its own merits.

What is needed

One more case in FleetdAssemblyFleetAppTest, symmetric to the one that already exists:

healthzGoesRedWhenTheLeadDaemonIsDownEvenThoughTheMemberIsUp

Prove it red with the new FleetApp(memberHerdr, memberHerdr, ...) mutation above, revert, touch the file, and re-run. Nothing else in the PR needs to change.

Explicitly not owed

The dropped GET /sessions merging assertion stays dropped. The worker tried it against the real assembly, hit a real 401 because Authz.Action.READ needs Caller.resolved() with pid > 0, and documented that in the new test's javadoc rather than inventing a pass. That is the right call, and FleetAppTwoDaemonTest still covers the merge itself. Do not reopen it.

## Correction for unit B2 (PR #626) — one more test needed before merge This is the authoritative channel for this correction. If it disagrees with the original brief, this is newer and it wins. ### What I verified, and what held PR #626 is good work. I re-ran the worker's claims independently and two of them hold: - `LsofPeerPidLookup` really does exclude its own pid — `private final long selfPid = ProcessHandle.current().pid();` and `current != selfPid`. So an in-process test client and daemon share a pid, the lookup returns nothing, and `PaneLocator` is never reached. The two production accessors (`ConnectionIdentity#panes()`, `FleetMcp#identity()`) are therefore a genuine necessity, not a shortcut. Both are additive getters over collaborators the objects already hold. **Accepted.** - I ran a mutation the worker did not: `new PaneLocator(herdr, memberHerdr)` → `new PaneLocator(herdr)`, dropping the **member** daemon instead of the lead one. It was killed by `connectionIdentityAlsoSearchesTheMemberDaemon`. The identity pair covers both directions plus a negative control, which is better shaped than the guard it replaces. ### The gap My second independent mutation **survived**. On `FleetdAssembly.java:518`: ```java Javalin app = new FleetApp(herdr, memberHerdr, workers, ...) // real Javalin app = new FleetApp(memberHerdr, memberHerdr, workers, ...) // mutant ``` ``` Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 ``` Both new `FleetdAssemblyFleetAppTest` cases stay green while the **lead** herdr client is dropped. This is not an equivalent mutant. It is the symmetric form of the CB-185 defect: `GET /healthz` would report green while the **lead** daemon is down, which is the same class of invisible failure the ticket exists to stop. And the guard being deleted **would have caught it**. Its positive assertion was: ```java assertTrue(source.contains("new FleetApp(herdr, memberHerdr, workers,"), ...) ``` That text does not survive the mutation. So as it stands, PR #626 is a net loss of coverage in one direction — which fails the bar set for step 2: a replacement must be at least as strong as the guard it removes. The old guard caught it only incidentally, by pinning the spelling (it would go red on a harmless variable rename too). That does not change the conclusion. The behaviour is real and worth a test on its own merits. ### What is needed One more case in `FleetdAssemblyFleetAppTest`, symmetric to the one that already exists: > `healthzGoesRedWhenTheLeadDaemonIsDownEvenThoughTheMemberIsUp` Prove it red with the `new FleetApp(memberHerdr, memberHerdr, ...)` mutation above, revert, `touch` the file, and re-run. Nothing else in the PR needs to change. ### Explicitly not owed The dropped `GET /sessions` merging assertion stays dropped. The worker tried it against the real assembly, hit a real `401` because `Authz.Action.READ` needs `Caller.resolved()` with `pid > 0`, and documented that in the new test's javadoc rather than inventing a pass. That is the right call, and `FleetAppTwoDaemonTest` still covers the merge itself. Do not reopen it.
ltms commented 2026-09-22 07:25:14 +02:00
Author
Owner
Copy Link
Copy Source

Correction for unit B3 (PR #628) — one gap, and two things that are NOT gaps

This is the authoritative channel for this correction. If it disagrees with the original brief, this is newer and it wins.

The branch is green

I trial-merged B3 onto the current Unit A tip (which already carries B1 and B2) and built it:

MERGED B1+B2+B3: tests=1881 failures=0 errors=0 skipped=0

FleetMcp.java was changed by both B2 and B3 and git merged the two additive hunks with no conflict — and the merged tree compiles and passes, which a clean auto-merge does not by itself prove. The three public accessors are accepted for the same reason B2's were: the assembly tests must live in dev.ltms.fleet to build ResourcePorts, FleetMcp lives in dev.ltms.fleet.mcp, so package-private is unreachable. The worker disclosed this up front rather than burying it.

What held

I ran mutations the worker did not, in the "keep every symbol, pass the wrong thing" shape rather than replacing a call with an inert variant:

  • Lead seats — Fleetd.leadSeatLookup(() -> config.get().profiles(), leaders, leads) → ..., java.util.Map.of(), leads). The call site still names the factory and is still called; only its leaders are starved. Killed by assembledLeadSeatSourceReportsALiveLeadsSeat (expected: <1> but was: <0>). Stronger than the guard it replaces.

Not a gap — recorded so it is not re-raised

The quarantine clock is unpinned, and that is not this PR's doing. BackendQuarantine.withEscalation(ports.nanoClock(), ...) → withEscalation(System::nanoTime, ...) survives. But the deleted guard asserted:

source.contains("BackendQuarantine quarantine = BackendQuarantine.withEscalation(System::nanoTime,\n ...")

That is the pre-Unit-A text. The old guard would have gone green on my mutant — it required exactly what I mutated to. So the replacement is strictly stronger here, and the unpinned ports.nanoClock() argument is a gap Unit A introduced, not a regression from B3. It belongs with #629, which is about the same ResourcePorts seam. Do not ask B3 to fix it.

The gap

FleetdAssembly.java:408:

LeadRollover leadRollover = Fleetd.leadRollover(cfg, router.leadAgents(), config, leads);   // real
LeadRollover leadRollover = Fleetd.leadRollover(cfg, router.memberAgents(), config, leads); // mutant

Tests run: 2, Failures: 0, Errors: 0, Skipped: 0

Both rollover tests stay green while the roll is wired to the member daemon's agent control instead of the lead's.

It survives only because the test configures a single FakeHerdr and no memberHerdrSocket. FleetdAssembly.java:140-142 then falls back to memberHerdr = herdr, so router.leadAgents() and router.memberAgents() wrap the same client and nothing could tell them apart. Under the live config on this host, which does set a member socket, they are different daemons — so the mutant sends /clear and the bootstrap text to the wrong daemon and the lead never rolls. That is a real fleetd #480 defect, not an equivalent mutant.

And the deleted guard would have caught it. It asserted the exact string LeadRollover leadRollover = leadRollover(cfg, router.leadAgents(), config, leads);, and its own failure message names this precise mode:

Dropping this call, or swapping one of its arguments for something that still compiles (e.g. null in place of router.leadAgents()), leaves every behavioural test green

So PR #628 currently trades away coverage in the one direction the guard's author explicitly warned about.

What is needed

Configure a distinct member herdr socket in FleetdLeadRolloverAssemblyTest so leadAgents() and memberAgents() are different clients, then assert the roll drives the lead daemon. Prove it red with the router.memberAgents() mutation above, revert, touch, re-run.

This is the same lesson B2 hit: a two-daemon wiring cannot be pinned by a one-daemon fixture. B2's FleetdAssemblyConnectionIdentityTest already builds two distinct fakes and is the pattern to copy.

Nothing else in the PR changes.

## Correction for unit B3 (PR #628) — one gap, and two things that are NOT gaps This is the authoritative channel for this correction. If it disagrees with the original brief, this is newer and it wins. ### The branch is green I trial-merged B3 onto the current Unit A tip (which already carries B1 and B2) and built it: ``` MERGED B1+B2+B3: tests=1881 failures=0 errors=0 skipped=0 ``` `FleetMcp.java` was changed by both B2 and B3 and git merged the two additive hunks with no conflict — and the merged tree compiles and passes, which a clean auto-merge does not by itself prove. The three `public` accessors are accepted for the same reason B2's were: the assembly tests must live in `dev.ltms.fleet` to build `ResourcePorts`, `FleetMcp` lives in `dev.ltms.fleet.mcp`, so package-private is unreachable. The worker disclosed this up front rather than burying it. ### What held I ran mutations the worker did not, in the "keep every symbol, pass the wrong thing" shape rather than replacing a call with an inert variant: - **Lead seats** — `Fleetd.leadSeatLookup(() -> config.get().profiles(), leaders, leads)` → `..., java.util.Map.of(), leads)`. The call site still names the factory and is still called; only its leaders are starved. **Killed** by `assembledLeadSeatSourceReportsALiveLeadsSeat` (`expected: <1> but was: <0>`). Stronger than the guard it replaces. ### Not a gap — recorded so it is not re-raised **The quarantine clock is unpinned, and that is not this PR's doing.** `BackendQuarantine.withEscalation(ports.nanoClock(), ...)` → `withEscalation(System::nanoTime, ...)` **survives**. But the deleted guard asserted: ```java source.contains("BackendQuarantine quarantine = BackendQuarantine.withEscalation(System::nanoTime,\n ...") ``` That is the **pre-Unit-A** text. The old guard would have gone *green* on my mutant — it required exactly what I mutated to. So the replacement is strictly stronger here, and the unpinned `ports.nanoClock()` argument is a gap **Unit A introduced**, not a regression from B3. It belongs with #629, which is about the same `ResourcePorts` seam. Do not ask B3 to fix it. ### The gap `FleetdAssembly.java:408`: ```java LeadRollover leadRollover = Fleetd.leadRollover(cfg, router.leadAgents(), config, leads); // real LeadRollover leadRollover = Fleetd.leadRollover(cfg, router.memberAgents(), config, leads); // mutant ``` ``` Tests run: 2, Failures: 0, Errors: 0, Skipped: 0 ``` Both rollover tests stay green while the roll is wired to the **member** daemon's agent control instead of the lead's. It survives only because the test configures a single `FakeHerdr` and no `memberHerdrSocket`. `FleetdAssembly.java:140-142` then falls back to `memberHerdr = herdr`, so `router.leadAgents()` and `router.memberAgents()` wrap the same client and nothing could tell them apart. **Under the live config on this host, which does set a member socket, they are different daemons** — so the mutant sends `/clear` and the bootstrap text to the wrong daemon and the lead never rolls. That is a real fleetd #480 defect, not an equivalent mutant. And the deleted guard would have caught it. It asserted the exact string `LeadRollover leadRollover = leadRollover(cfg, router.leadAgents(), config, leads);`, and its own failure message names this precise mode: > Dropping this call, **or swapping one of its arguments for something that still compiles** (e.g. null in place of `router.leadAgents()`), leaves every behavioural test green So PR #628 currently trades away coverage in the one direction the guard's author explicitly warned about. ### What is needed Configure a **distinct member herdr socket** in `FleetdLeadRolloverAssemblyTest` so `leadAgents()` and `memberAgents()` are different clients, then assert the roll drives the **lead** daemon. Prove it red with the `router.memberAgents()` mutation above, revert, `touch`, re-run. This is the same lesson B2 hit: a two-daemon wiring cannot be pinned by a one-daemon fixture. B2's `FleetdAssemblyConnectionIdentityTest` already builds two distinct fakes and is the pattern to copy. Nothing else in the PR changes.
ltms commented 2026-09-22 07:48:28 +02:00
Author
Owner
Copy Link
Copy Source

Steps 1–3 are done and live on main

main is 26f1986. Full suite, stale surefire reports cleared first:

report files: 153
tests=1883 failures=0 errors=0 skipped=0

Daemon redeployed: pid 63248, jar 85e64b1afb5c, fleetd listening at 12:47:49, fleet_whoami still primary, and a real sonnet spawn reached idle before being stopped. A green /healthz only proves herdr answers, so the spawn is the check that matters.

What landed

PR Unit Replaces
#620 + #624 Unit A + A-gaps — (the assembly boundary)
#627 B1 — #248 CompletionResolver 1 file, 4 tests
#626 B2 — CB-185 pair 2 files, 2 tests
#628 B3 — #176 / #466 / #480 3 files, 3 tests

All six source-text guard files are gone. Every replacement drives the real FleetdAssembly.assembleAndStart(...) and inspects the production objects through FleetdRuntime. None reads the source text of a .java file.

Verification

Every unit was checked with a mutation the worker did not run, in the "keep every symbol, pass the wrong thing" shape rather than swapping a call for an inert variant — because that is the shape a source-text guard is blind to by construction.

Killed: starving worktreeBranchLookup with an empty roster; starving backendErrorPatternLookup with an empty map; starving leadSeatLookup with empty leaders; dropping the member daemon from PaneLocator.

Two survived on first pass and were sent back rather than waved through:

  • B2 — new FleetApp(memberHerdr, memberHerdr, ...) left both tests green. The deleted guard asserted source.contains("new FleetApp(herdr, memberHerdr, workers,") and would have caught it, so the PR was a net loss of coverage in that direction. Fixed by the symmetric lead-down case.
  • B3 — Fleetd.leadRollover(cfg, router.memberAgents(), ...) left both tests green, because the fixture used one FakeHerdr and no memberHerdrSocket, so FleetdAssembly.java:140-142 collapsed both agent controls onto one client. Fixed by giving the test two distinct sockets. The deleted guard's own failure message named this exact mode.

Both are the same lesson: a two-daemon wiring cannot be pinned by a one-daemon fixture.

One survivor was ruled not a gap: the quarantine clock. The deleted guard required the pre-Unit-A text withEscalation(System::nanoTime, and would have gone green on that mutant too, so the replacement is strictly stronger. That gap belongs to #629.

The merge was not mechanical

main moved 8 commits while Unit A was in flight. #622 (fleetd #621) added requireOperatorConfirm inside the block Unit A had already moved. Taking Unit A's side of the Fleetd.java conflict — the resolution a merge tool suggests — would have dropped it and reverted the operator's #621 fix, with a clean build and no failing test, because the 13-argument LeadHeartbeatLoop overload still delegates with true. Carried across by hand in 72f46d7.

That is this ticket's own defect shape, found in this ticket's own merge.

Follow-ups filed, not delegated

  • #625 — the subscription guard's call site: deleting assertPrimaryClean from Fleetd.main leaves the suite green.
  • #629 — the herdr boot wait bypasses ResourcePorts, so any test with an unhealthy lead herdr pays 30 real seconds.
  • #630 — the assembly's requireOperatorConfirm wiring is unpinned (the one above).

All three are call sites the assembly owns and no test observes — the same family this ticket exists to close.

Still open here

Step 4: ranks 1, 2, 3, 6, 7 and 8 in the table at the top. Those are Shape B — behaviour silently degrades, nothing else enforces it — and they are the ones that actually hurt a running fleet. Rank 1 hands a genuine usage-limit refusal back to a waiting caller as real completed work.

## Steps 1–3 are done and live on `main` `main` is `26f1986`. Full suite, stale surefire reports cleared first: ``` report files: 153 tests=1883 failures=0 errors=0 skipped=0 ``` Daemon redeployed: pid 63248, jar `85e64b1afb5c`, `fleetd listening` at 12:47:49, `fleet_whoami` still `primary`, and a real sonnet spawn reached `idle` before being stopped. A green `/healthz` only proves herdr answers, so the spawn is the check that matters. ### What landed | PR | Unit | Replaces | |---|---|---| | #620 + #624 | Unit A + A-gaps | — (the assembly boundary) | | #627 | B1 — #248 CompletionResolver | 1 file, 4 tests | | #626 | B2 — CB-185 pair | 2 files, 2 tests | | #628 | B3 — #176 / #466 / #480 | 3 files, 3 tests | All six source-text guard files are gone. Every replacement drives the real `FleetdAssembly.assembleAndStart(...)` and inspects the production objects through `FleetdRuntime`. None reads the source text of a `.java` file. ### Verification Every unit was checked with a mutation the worker did **not** run, in the "keep every symbol, pass the wrong thing" shape rather than swapping a call for an inert variant — because that is the shape a source-text guard is blind to by construction. Killed: starving `worktreeBranchLookup` with an empty roster; starving `backendErrorPatternLookup` with an empty map; starving `leadSeatLookup` with empty leaders; dropping the member daemon from `PaneLocator`. Two survived on first pass and were sent back rather than waved through: - **B2** — `new FleetApp(memberHerdr, memberHerdr, ...)` left both tests green. The deleted guard asserted `source.contains("new FleetApp(herdr, memberHerdr, workers,")` and would have caught it, so the PR was a net loss of coverage in that direction. Fixed by the symmetric lead-down case. - **B3** — `Fleetd.leadRollover(cfg, router.memberAgents(), ...)` left both tests green, because the fixture used one `FakeHerdr` and no `memberHerdrSocket`, so `FleetdAssembly.java:140-142` collapsed both agent controls onto one client. Fixed by giving the test two distinct sockets. The deleted guard's own failure message named this exact mode. Both are the same lesson: **a two-daemon wiring cannot be pinned by a one-daemon fixture.** One survivor was ruled *not* a gap: the quarantine clock. The deleted guard required the pre-Unit-A text `withEscalation(System::nanoTime,` and would have gone green on that mutant too, so the replacement is strictly stronger. That gap belongs to #629. ### The merge was not mechanical `main` moved 8 commits while Unit A was in flight. #622 (fleetd #621) added `requireOperatorConfirm` **inside the block Unit A had already moved**. Taking Unit A's side of the `Fleetd.java` conflict — the resolution a merge tool suggests — would have dropped it and reverted the operator's #621 fix, with a clean build and no failing test, because the 13-argument `LeadHeartbeatLoop` overload still delegates with `true`. Carried across by hand in `72f46d7`. That is this ticket's own defect shape, found in this ticket's own merge. ### Follow-ups filed, not delegated - **#625** — the subscription guard's call site: deleting `assertPrimaryClean` from `Fleetd.main` leaves the suite green. - **#629** — the herdr boot wait bypasses `ResourcePorts`, so any test with an unhealthy lead herdr pays 30 real seconds. - **#630** — the assembly's `requireOperatorConfirm` wiring is unpinned (the one above). All three are call sites the assembly owns and no test observes — the same family this ticket exists to close. ### Still open here **Step 4**: ranks 1, 2, 3, 6, 7 and 8 in the table at the top. Those are Shape B — behaviour silently degrades, nothing else enforces it — and they are the ones that actually hurt a running fleet. Rank 1 hands a genuine usage-limit refusal back to a waiting caller as real completed work.
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#612
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?