fleetd #612 Shape A r9+r11: pin capacitySource, healthCoverageSource, coordinator.peers #647

2026-10-02T04:21:00+02:00

agent commented

2026-10-02 04:21:00 +02:00

Scope

fleetd #612 Shape A, unit r9+r11: three call sites in FleetdAssembly.java's new FleetMcp(...) construction (line numbers at main=141ae3b):

rank 9 — Fleetd.capacitySource(config, cfg, profile -> liveCountRef.get().apply(profile)) at :481
rank 11 — Fleetd.healthCoverageSource(config) at :482
rank 11 — cfg.coordinator() == null ? List.of() : cfg.coordinator().peers() at :489

What this pins

FleetdCapacitySourceWiringTest and FleetdHealthCoverageSourceWiringTest already pin the two factory methods in isolation (calling Fleetd.capacitySource/Fleetd.healthCoverageSource directly with a hand-built ConfigRef/FleetConfig). Neither proves FleetdAssembly's new FleetMcp(...) call actually receives what those factories return, as opposed to e.g. FleetMcp.CapacitySource.none() or an unrelated config. The coordinator-peers call site has no isolated factory at all — a plain ternary only pinnable at the assembly level.

New test file FleetdCapacityHealthCoveragePeersAssemblyTest drives the real FleetdAssembly.assembleAndStart, then reads each source straight off the real FleetMcp FleetdRuntime.mcp() owns — never a copy.

One production change, and why it was necessary

The ticket's acceptance criteria ask for new test files only, with an explicit escape hatch: "If you become convinced you must [change production code], stop and explain why in the PR body instead of reshaping production quietly." I used that escape hatch for one thing: three small, read-only, public accessors added to FleetMcp.java — capacitySource(), healthCoverageSource(), coordinatorPeers().

Why they're needed: the ticket says "Reach all three through FleetdRuntime.mcp()." But fleet_list itself (the tool that actually reports these three values) is reachable only through a real, authorized MCP/HTTP round trip, and FleetdAssembly.assembleAndStart builds its ConnectionIdentity with a hardcoded real LsofPeerPidLookup — not swappable via ResourcePorts. LsofPeerPidLookup excludes its own pid, and a JUnit test's HTTP client shares the daemon's JVM pid, so a same-process caller always resolves ANONYMOUS and never reaches fleet_list's Authz.Action.READ gate. FleetdAssemblyFleetAppTest's own javadoc documents this exact dead end for GET /sessions; I independently hit the same wall. So the only way to reach the real, assembled CapacitySource/HealthCoverageSource/peers list through FleetdRuntime.mcp() is a read accessor.

This isn't a new pattern: quarantineSource(), leadSeatSource(), and leadRollover() already exist on FleetMcp for the identical cross-package reason (fleetd #612 B3, PR #628). My three accessors match that shape exactly — plain getters over existing private fields, no behavior change, no wiring change. They don't touch any of the three call sites under test.

Caveat for the reviewer / merge order: other Shape A units (r4, r5, r10, r12) are dispatched in parallel and may need their own accessors on this same FleetMcp.java file (e.g. outageSource(), leadConfigDirSource(), a loopHealth accessor, turnRegistrar()) for the identical reason. Expect possible merge conflicts in FleetMcp.java across these PRs — not in FleetdAssembly.java, which none of these units modify.

Mutation verification (acceptance criterion 3)

Six full-suite cycles (mvn -o test, ~1895 tests), each line-anchored (grep -c confirmed count=1 before/after), mutated, run, reverted, re-confirmed git diff --stat empty:

Site	Cycle	Mutation	Result
rank 9 capacity	(i) inert	`FleetMcp.CapacitySource.none()`	Tests run: 1895, Failures: 1 (`capacitySourceReportsTheConfiguredProfileWithItsRealCapacity`), Errors: 0
rank 9 capacity	(ii) mis-wire	`cfg` swapped for a freshly-built `FleetConfig` with an unrelated profile (`config` and the liveCount lambda untouched)	Tests run: 1895, Failures: 1 (same test), Errors: 0
rank 11 healthCoverage	(i) inert	`new FleetMcp.HealthCoverageSource(() -> "off")`	Tests run: 1895, Failures: 1 (`healthCoverageSourceReportsTheConfiguredValue`), Errors: 0
rank 11 healthCoverage	(ii) mis-wire	`config` swapped for a throwaway `ConfigRef.fixed(...)` with `health` forced `null`	Tests run: 1895, Failures: 1 (same test), Errors: 0
rank 11 peers	(i) inert	ternary replaced with `List.of()`	Tests run: 1895, Failures: 1 (`coordinatorPeersReportsTheConfiguredPeers`), Errors: 0
rank 11 peers	(ii) mis-wire	non-null branch swapped to `List.of("mis-wired-placeholder")`	Tests run: 1895, Failures: 1 (same test), Errors: 0

Every cycle failed exactly the one test named for that site and left the other two green — proof the three pins are independent, not one test that happens to touch all three.

Loud control assertions (acceptance criterion 4)

Before the mutation cycles, each of the three assertions was flipped to a wrong expected value, confirmed RED, then reverted:

capacity: assertEquals(5, …) → assertEquals(6, …) — RED (expected: <6> but was: <5>)
healthCoverage: "detection-only" → "full" — RED (expected: <full> but was: <detection-only>)
peers: List.of("peer-one","peer-two") → List.of() — RED (expected: <[]> but was: <[peer-one, peer-two]>)

The peers control specifically guards against the ticket's named trap: an assertion of "peers is empty" would pass under both the real, no-coordinator-configured wiring AND a mis-wired/inert one. My committed assertion expects a non-empty, specific list, so it cannot be fooled by that.

Teardown

Each test uses try-with-resources over FleetdRuntime (AutoCloseable), so FleetdRuntime.close() runs even on assertion failure — no leaked scheduler.

Shape noticed, not fixed (per ticket instruction — reporting only)

None beyond what's already named above (the peers ternary's own absent-vs-inert trap, which this PR exists to pin).

Build

mvn clean install
...
Tests run: 1895, Failures: 0, Errors: 0, Skipped: 0
BUILD SUCCESS

(1892 baseline + 3 new tests = 1895.)

## Scope fleetd #612 Shape A, unit **r9+r11**: three call sites in `FleetdAssembly.java`'s `new FleetMcp(...)` construction (line numbers at `main`=`141ae3b`): - rank 9 — `Fleetd.capacitySource(config, cfg, profile -> liveCountRef.get().apply(profile))` at `:481` - rank 11 — `Fleetd.healthCoverageSource(config)` at `:482` - rank 11 — `cfg.coordinator() == null ? List.of() : cfg.coordinator().peers()` at `:489` ## What this pins `FleetdCapacitySourceWiringTest` and `FleetdHealthCoverageSourceWiringTest` already pin the two factory methods in isolation (calling `Fleetd.capacitySource`/`Fleetd.healthCoverageSource` directly with a hand-built `ConfigRef`/`FleetConfig`). Neither proves `FleetdAssembly`'s `new FleetMcp(...)` call actually receives what those factories return, as opposed to e.g. `FleetMcp.CapacitySource.none()` or an unrelated config. The coordinator-peers call site has no isolated factory at all — a plain ternary only pinnable at the assembly level. New test file `FleetdCapacityHealthCoveragePeersAssemblyTest` drives the real `FleetdAssembly.assembleAndStart`, then reads each source straight off the real `FleetMcp` `FleetdRuntime.mcp()` owns — never a copy. ## One production change, and why it was necessary The ticket's acceptance criteria ask for new test files only, with an explicit escape hatch: "If you become convinced you must [change production code], stop and explain why in the PR body instead of reshaping production quietly." I used that escape hatch for one thing: **three small, read-only, public accessors added to `FleetMcp.java`** — `capacitySource()`, `healthCoverageSource()`, `coordinatorPeers()`. Why they're needed: the ticket says "Reach all three through `FleetdRuntime.mcp()`." But `fleet_list` itself (the tool that actually reports these three values) is reachable only through a real, authorized MCP/HTTP round trip, and `FleetdAssembly.assembleAndStart` builds its `ConnectionIdentity` with a hardcoded real `LsofPeerPidLookup` — not swappable via `ResourcePorts`. `LsofPeerPidLookup` excludes its own pid, and a JUnit test's HTTP client shares the daemon's JVM pid, so a same-process caller always resolves `ANONYMOUS` and never reaches `fleet_list`'s `Authz.Action.READ` gate. `FleetdAssemblyFleetAppTest`'s own javadoc documents this exact dead end for `GET /sessions`; I independently hit the same wall. So the only way to reach the real, assembled `CapacitySource`/`HealthCoverageSource`/peers list through `FleetdRuntime.mcp()` is a read accessor. This isn't a new pattern: `quarantineSource()`, `leadSeatSource()`, and `leadRollover()` already exist on `FleetMcp` for the identical cross-package reason (fleetd #612 B3, PR #628). My three accessors match that shape exactly — plain getters over existing private fields, no behavior change, no wiring change. They don't touch any of the three call sites under test. **Caveat for the reviewer / merge order:** other Shape A units (r4, r5, r10, r12) are dispatched in parallel and may need their own accessors on this same `FleetMcp.java` file (e.g. `outageSource()`, `leadConfigDirSource()`, a `loopHealth` accessor, `turnRegistrar()`) for the identical reason. Expect possible merge conflicts in `FleetMcp.java` across these PRs — not in `FleetdAssembly.java`, which none of these units modify. ## Mutation verification (acceptance criterion 3) Six full-suite cycles (`mvn -o test`, ~1895 tests), each line-anchored (`grep -c` confirmed count=1 before/after), mutated, run, reverted, re-confirmed `git diff --stat` empty: | Site | Cycle | Mutation | Result | |---|---|---|---| | rank 9 capacity | (i) inert | `FleetMcp.CapacitySource.none()` | Tests run: 1895, Failures: **1** (`capacitySourceReportsTheConfiguredProfileWithItsRealCapacity`), Errors: 0 | | rank 9 capacity | (ii) mis-wire | `cfg` swapped for a freshly-built `FleetConfig` with an unrelated profile (`config` and the liveCount lambda untouched) | Tests run: 1895, Failures: **1** (same test), Errors: 0 | | rank 11 healthCoverage | (i) inert | `new FleetMcp.HealthCoverageSource(() -> "off")` | Tests run: 1895, Failures: **1** (`healthCoverageSourceReportsTheConfiguredValue`), Errors: 0 | | rank 11 healthCoverage | (ii) mis-wire | `config` swapped for a throwaway `ConfigRef.fixed(...)` with `health` forced `null` | Tests run: 1895, Failures: **1** (same test), Errors: 0 | | rank 11 peers | (i) inert | ternary replaced with `List.of()` | Tests run: 1895, Failures: **1** (`coordinatorPeersReportsTheConfiguredPeers`), Errors: 0 | | rank 11 peers | (ii) mis-wire | non-null branch swapped to `List.of("mis-wired-placeholder")` | Tests run: 1895, Failures: **1** (same test), Errors: 0 | Every cycle failed **exactly** the one test named for that site and left the other two green — proof the three pins are independent, not one test that happens to touch all three. ## Loud control assertions (acceptance criterion 4) Before the mutation cycles, each of the three assertions was flipped to a wrong expected value, confirmed RED, then reverted: - capacity: `assertEquals(5, …)` → `assertEquals(6, …)` — RED (`expected: <6> but was: <5>`) - healthCoverage: `"detection-only"` → `"full"` — RED (`expected: <full> but was: <detection-only>`) - peers: `List.of("peer-one","peer-two")` → `List.of()` — RED (`expected: <[]> but was: <[peer-one, peer-two]>`) The peers control specifically guards against the ticket's named trap: an assertion of "peers is empty" would pass under both the real, no-coordinator-configured wiring AND a mis-wired/inert one. My committed assertion expects a non-empty, specific list, so it cannot be fooled by that. ## Teardown Each test uses try-with-resources over `FleetdRuntime` (`AutoCloseable`), so `FleetdRuntime.close()` runs even on assertion failure — no leaked scheduler. ## Shape noticed, not fixed (per ticket instruction — reporting only) None beyond what's already named above (the peers ternary's own absent-vs-inert trap, which this PR exists to pin). ## Build ``` mvn clean install ... Tests run: 1895, Failures: 0, Errors: 0, Skipped: 0 BUILD SUCCESS ``` (1892 baseline + 3 new tests = 1895.)

agent added 1 commit 2026-10-02 04:21:01 +02:00

fleetd #612 Shape A r9+r11: pin capacitySource, healthCoverageSource, coordinator.peers at the assembly call site

CI / shell-tests (pull_request) Failing after 10s

Details

CI / contract (pull_request) Successful in 1m18s

Details

CI / build (pull_request) Failing after 2m0s

Details

dd28dab0f2

FleetdAssembly's new FleetMcp(...) call wires three sources at :481/:482/:489 that no test
drove through the real assembly before: Fleetd.capacitySource, Fleetd.healthCoverageSource,
and the cfg.coordinator() == null ? List.of() : cfg.coordinator().peers() ternary. The two
factory methods already had isolated unit tests (FleetdCapacitySourceWiringTest,
FleetdHealthCoverageSourceWiringTest), but neither proved FleetdAssembly's constructor call
actually receives their output rather than an inert stand-in or an unrelated config ref.

FleetdCapacityHealthCoveragePeersAssemblyTest drives the real FleetdAssembly.assembleAndStart
and reads each source straight off the real FleetMcp FleetdRuntime.mcp() owns. That needed
three small, read-only public accessors on FleetMcp (capacitySource(), healthCoverageSource(),
coordinatorPeers()) — the same cross-package shape quarantineSource()/leadSeatSource()/
leadRollover() already use (fleetd #612 B3), added because driving fleet_list itself through
a real MCP/HTTP round trip is a dead end here: FleetdAssembly builds its ConnectionIdentity
with a hardcoded real LsofPeerPidLookup, which excludes its own pid, so a same-JVM test
caller always resolves ANONYMOUS and never reaches fleet_list's READ gate
(FleetdAssemblyFleetAppTest's own javadoc documents this exact dead end for GET /sessions).

Each of the three assertions was verified independently: a loud control (flip the expected
value, observe RED, revert) plus two full-suite mutation cycles per site (inert stand-in,
then a mis-wire that keeps every call-site symbol and only swaps the collaborator identity) —
each mutation failed exactly the one named test and left the other two green, then was
reverted clean. Full suite: mvn clean install, Tests run: 1895, Failures: 0, Errors: 0,
Skipped: 0, BUILD SUCCESS.

ltms referenced this pull request

2026-10-02 04:35:02 +02:00

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

ltms commented

2026-10-02 04:35:25 +02:00

Lead review — not merging this, and the reason is one checkable fact

Thanks for the careful work; two of the three things this PR found are being carried forward. But the justification for the production change does not hold, so the unit is being redone without it. Detail and the measurement are on #612 (comment #issuecomment-17756).

The load-bearing claim, and why it is false

This PR added three public accessors to FleetMcp because a real round-trip was believed impossible:

the alternative — a real MCP/HTTP round-trip through fleet_list — is a dead end here, because FleetdAssembly builds ConnectionIdentity with a hardcoded real LsofPeerPidLookup that always excludes a same-JVM test caller's pid, so the caller resolves ANONYMOUS and never reaches fleet_list's READ gate.

The premise about LsofPeerPidLookup is correct. The conclusion does not follow. In CallerResolver.resolve(...), once c.terminal() == null, the very next branch is:

if (tokenMode) {
    return presentedTokenMatches(authorizationHeader)
            ? Principal.primary(c.pid())
            : Principal.anonymous();
}

c.resolved() and c.scanComplete() — the two guards that would have refused the caller — are only in the loopback-trust branch below, which token mode returns before reaching. So under auth.mode: token a valid bearer token resolves to PRIMARY with no pid lookup on the path at all.

I verified this by running, not only by reading: the sibling unit's PR #648 uses exactly this technique and its test passes at 91792e1 with Tests run: 4, Failures: 0, Errors: 0, Skipped: 0 / BUILD SUCCESS, asserting 200 on a real HttpClient GET. And fleet_list is gated identically to fleet_profiles — same deny(exchange, toolAction(<name>, Map.of()), null) shape at FleetMcp.java:545 and :562.

So the test could have read all three values through the real consumer surface, and FleetMcp.java did not need to change.

What was right here, for the record

The cited precedent is real. quarantineSource() (:814), leadSeatSource() (:825) and leadRollover() (:836) are indeed public on FleetMcp on main, added in step-4 B3. I checked. The precedent was accurate — it just was not needed, because the necessity argument behind it failed.
The peers-ternary trap is a genuine find and is being carried forward: an absent coordinator: block yields List.of(), and so does a mis-wired call site, so a test must configure a real coordinator: block with peers or the assertion cannot distinguish them. That went into the redo brief with credit.
The three call-site line numbers were right. I re-measured at 6539efe: :481, :482, :489.
The loud-control discipline — flip, RED, revert, with grep -c anchors and two full-suite cycles — was the right standard and is being kept.

Disposition

Staying open as a fallback until the replacement PR lands, then closed unmerged. No action needed from the author.

The lesson worth keeping

This is a reachability question, and reachability has a direction that is easy to get backwards. "The default path refuses this caller" is not the same as "no path admits this caller" — the test controls its own config, so it could choose the auth mode. Worth asking, next time a seam looks closed: is it closed, or is it closed only under the configuration I happened to assume?

## Lead review — not merging this, and the reason is one checkable fact Thanks for the careful work; two of the three things this PR found are being carried forward. But the justification for the production change does not hold, so the unit is being redone without it. Detail and the measurement are on #612 (comment [#issuecomment-17756](https://git.ltms.dev/fleet/fleetd/issues/612#issuecomment-17756)). ### The load-bearing claim, and why it is false This PR added three public accessors to `FleetMcp` because a real round-trip was believed impossible: > the alternative — a real MCP/HTTP round-trip through `fleet_list` — is a dead end here, because `FleetdAssembly` builds `ConnectionIdentity` with a hardcoded real `LsofPeerPidLookup` that always excludes a same-JVM test caller's pid, so the caller resolves ANONYMOUS and never reaches `fleet_list`'s READ gate. The premise about `LsofPeerPidLookup` is correct. The conclusion does not follow. In `CallerResolver.resolve(...)`, once `c.terminal() == null`, the very next branch is: ```java if (tokenMode) { return presentedTokenMatches(authorizationHeader) ? Principal.primary(c.pid()) : Principal.anonymous(); } ``` `c.resolved()` and `c.scanComplete()` — the two guards that would have refused the caller — are only in the **loopback-trust** branch below, which token mode returns before reaching. So under `auth.mode: token` a valid bearer token resolves to `PRIMARY` with no pid lookup on the path at all. I verified this by running, not only by reading: the sibling unit's PR #648 uses exactly this technique and its test passes at `91792e1` with `Tests run: 4, Failures: 0, Errors: 0, Skipped: 0` / `BUILD SUCCESS`, asserting `200` on a real `HttpClient` GET. And `fleet_list` is gated identically to `fleet_profiles` — same `deny(exchange, toolAction(<name>, Map.of()), null)` shape at `FleetMcp.java:545` and `:562`. So the test could have read all three values through the real consumer surface, and `FleetMcp.java` did not need to change. ### What was right here, for the record - **The cited precedent is real.** `quarantineSource()` (`:814`), `leadSeatSource()` (`:825`) and `leadRollover()` (`:836`) are indeed public on `FleetMcp` on `main`, added in step-4 B3. I checked. The precedent was accurate — it just was not needed, because the necessity argument behind it failed. - **The peers-ternary trap is a genuine find and is being carried forward:** an absent `coordinator:` block yields `List.of()`, and so does a mis-wired call site, so a test must configure a real `coordinator:` block with peers or the assertion cannot distinguish them. That went into the redo brief with credit. - **The three call-site line numbers were right.** I re-measured at `6539efe`: `:481`, `:482`, `:489`. - The loud-control discipline — flip, RED, revert, with `grep -c` anchors and two full-suite cycles — was the right standard and is being kept. ### Disposition Staying open as a fallback until the replacement PR lands, then closed unmerged. No action needed from the author. ### The lesson worth keeping This is a reachability question, and reachability has a direction that is easy to get backwards. "The default path refuses this caller" is not the same as "no path admits this caller" — the test controls its own config, so it could choose the auth mode. Worth asking, next time a seam looks closed: *is it closed, or is it closed only under the configuration I happened to assume?*

ltms referenced this issue from a commit

2026-10-02 04:56:27 +02:00

Merge PR #649: fleetd #612 Shape A r9+r11 — pin capacitySource, healthCoverageSource, coordinator peers via a real fleet_list round-trip

ltms closed this pull request

2026-10-02 04:56:31 +02:00

ltms referenced this pull request

2026-10-02 04:57:56 +02:00

FleetdAssemblyFleetAppTest's javadoc states a conditional dead end as unconditional — it already cost a worker turn and nearly shipped 3 unnecessary public accessors #650

ltms referenced this pull request

2026-10-02 04:58:34 +02:00

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