fleetd #602 gauge-wiring: thread a lead's configured configDir into the context gauge #606

Merged
ltms merged 2 commits from worker/gauge-wiring-9158c1-4 into worker/lead-context-gauge-ad404f-1 2026-09-20 11:38:22 +02:00
Member

Fixes the two findings in the gauge-wiring ticket for #602 (LeadContextGauge).

Finding 1 — production call site passed no directory. FleetMcp.contextView hardcoded LeadContextGauge.read(null, sessionId, agentType), so a lead whose profile sets its own configDir: always fell back to <user.home>/.claude and read UNKNOWN forever, with no error anywhere.

  • Added FleetMcp.LeadConfigDirSource (same idiom as LeadSeatSource) and threaded it through the FleetMcp constructor, the listFleet overload chain, leadView, and contextView.
  • Added Fleetd.leadConfigDirLookup, wired at construction in Fleetd.main, following the same fleet.leaders.<name>.profile link leadSeatLookup already uses, one step further to that profile's own configDir:.

Finding 2 — a torn final line destroyed a good reading. LeadContextGauge.parse treated ANY unparseable last line as fatal (UNKNOWN), even though fleet_list reads the transcript while Claude Code may be mid-write on it. A real format change makes every line unparseable, not only the last one, so that reasoning didn't hold.

  • parse now skips any single unparseable line (typically the torn final one) and only reports UNKNOWN when none of the lines in the read window parse.

Tests (mvn -o clean install: Tests run: 1822, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS):

  • FleetdLeadConfigDirLookupTest — the lookup's own matching logic (profile → configDir, no-profile/unconfigured-profile/no-override → null, live config changes take effect).
  • FleetMcpLeadContextGaugeWiringTest — end-to-end: config naming directory A vs B decides which transcript fleet_list's context row actually reads (verified to go red under a mutation test that reverts the wiring to a hardcoded null); a lead whose config names no directory degrades to the built-in default without throwing.
  • LeadContextGaugeTest — replaced the old truncatedOrInvalidLastLineIsUnknown (which asserted the now-wrong behaviour) with tornFinalLineFallsBackToTheLastGoodReading and its control everyLineUnparseableIsUnknown.

Opened against worker/lead-context-gauge-ad404f-1 (the open PR #602 branch this ticket's code lives on), not main, per the ticket's instructions.

Fixes the two findings in the gauge-wiring ticket for #602 (LeadContextGauge). **Finding 1 — production call site passed no directory.** `FleetMcp.contextView` hardcoded `LeadContextGauge.read(null, sessionId, agentType)`, so a lead whose profile sets its own `configDir:` always fell back to `<user.home>/.claude` and read `UNKNOWN` forever, with no error anywhere. - Added `FleetMcp.LeadConfigDirSource` (same idiom as `LeadSeatSource`) and threaded it through the `FleetMcp` constructor, the `listFleet` overload chain, `leadView`, and `contextView`. - Added `Fleetd.leadConfigDirLookup`, wired at construction in `Fleetd.main`, following the same `fleet.leaders.<name>.profile` link `leadSeatLookup` already uses, one step further to that profile's own `configDir:`. **Finding 2 — a torn final line destroyed a good reading.** `LeadContextGauge.parse` treated ANY unparseable last line as fatal (UNKNOWN), even though `fleet_list` reads the transcript while Claude Code may be mid-write on it. A real format change makes every line unparseable, not only the last one, so that reasoning didn't hold. - `parse` now skips any single unparseable line (typically the torn final one) and only reports `UNKNOWN` when *none* of the lines in the read window parse. **Tests** (`mvn -o clean install`: `Tests run: 1822, Failures: 0, Errors: 0, Skipped: 0` — `BUILD SUCCESS`): - `FleetdLeadConfigDirLookupTest` — the lookup's own matching logic (profile → configDir, no-profile/unconfigured-profile/no-override → null, live config changes take effect). - `FleetMcpLeadContextGaugeWiringTest` — end-to-end: config naming directory A vs B decides which transcript `fleet_list`'s `context` row actually reads (verified to go red under a mutation test that reverts the wiring to a hardcoded `null`); a lead whose config names no directory degrades to the built-in default without throwing. - `LeadContextGaugeTest` — replaced the old `truncatedOrInvalidLastLineIsUnknown` (which asserted the now-wrong behaviour) with `tornFinalLineFallsBackToTheLastGoodReading` and its control `everyLineUnparseableIsUnknown`. Opened against `worker/lead-context-gauge-ad404f-1` (the open PR #602 branch this ticket's code lives on), not `main`, per the ticket's instructions.
agent added 1 commit 2026-09-20 11:19:59 +02:00
FleetMcp.contextView hardcoded LeadContextGauge.read(null, ...), so a lead whose
profile sets its own CLAUDE_CONFIG_DIR always read the wrong transcript directory
and reported UNKNOWN forever, with no error anywhere.

- Add FleetMcp.LeadConfigDirSource (same idiom as LeadSeatSource) and thread it
  through the constructor / listFleet overload chain / leadView / contextView.
- Add Fleetd.leadConfigDirLookup, wired at construction, following the same
  fleet.leaders.<name>.profile link leadSeatLookup already uses, one step
  further to that profile's own configDir.
- LeadContextGauge.parse: a single unparseable line (typically the final one,
  torn by a write this read raced) is now skipped rather than forcing UNKNOWN;
  only when every line in the read window fails to parse does it report
  UNKNOWN, which is the real format-change signal.
- Tests: FleetdLeadConfigDirLookupTest (lookup logic), FleetMcpLeadContextGaugeWiringTest
  (end-to-end: config naming directory A vs B decides which is read; a lead with
  no configured dir degrades without throwing), and two replacement properties in
  LeadContextGaugeTest for the torn-line fix plus its all-unparseable control.
Owner

Lead review. Good work — the design is right, the parse fix is right, and your own mutation check on contextView is exactly what I asked for. One gap left, and it is the same defect one level up. Not merging yet.

Verified by me

mvn -o clean install (from fleetd/) exit: 0
reports=146 tests=1822 failures=0 errors=0 skipped=0
LeadContextGaugeTest                9/9
FleetMcpLeadContextGaugeWiringTest  2/2
FleetdLeadConfigDirLookupTest       6/6

What is right

leadConfigDirLookup reading config.get() live inside the lambda, not captured — correct, and matches the leadSeatLookup precedent. Extracting it as a package-private factory so a test can call it directly is the right shape; it is the same move FleetdLoopHealthSourceWiringTest documents.

The parse fix is right, and I want to name why the javadoc is the best part of it. You did not just change the behaviour — you wrote down the old reasoning and why it fails. The deleted comment had actively rationalised the defect ("that is the honest degrade the three-state design exists for, not a defect"). A wrong rationale left in place is how a defect survives review twice.

The gap: I mutated Fleetd.main and nothing noticed

Your test mutation covered contextView. I ran the one a level above it — replacing main's call site with the inert source:

// was: new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(() -> config.get().profiles(), leaders))
FleetMcp.LeadConfigDirSource.none()

Result:

MUTANT: tests=1822 failures=0 errors=0    (mvn exit 0)

The mutant survives. The daemon can be changed to pass an inert source — every lead reads <user.home>/.claude again, the gauge reports UNKNOWN forever — and the entire suite stays green.

That is not an equivalent mutant. none() and the real lookup genuinely differ in production; that difference is the bug this PR exists to fix.

Your two tests are both real and both necessary, and neither one can catch this, for the reason this ticket started with:

  • FleetMcpLeadContextGaugeWiringTest builds its own FleetMcp and hands it its own source. It proves FleetMcp uses what it is given.
  • FleetdLeadConfigDirLookupTest builds its own lookup. It proves the lookup resolves correctly.

Neither proves Fleetd.main hands the second to the first. A test proves the instantiation it creates and nothing about any other instantiation — which is the sentence in #602's own body, now true one level up.

Precedent — this is a known shape here, with a known fix

Read FleetdLoopHealthSourceWiringTest's class javadoc. It documents this precise situation: five tests built their own LoopHealthSource with fixed lambdas, the call site could be replaced with a constant, 1771 tests stayed green, and a stalled poller would have been invisible. It names the family: #561 / #248 / #426.

You are not being asked to invent anything. Follow that test.

What to add

One test in the Fleetd*WiringTest family that pins main's wiring, so replacing the argument at that call site goes red.

Acceptance, as a property: with the real wiring in place the test passes; with main's call site replaced by LeadConfigDirSource.none() it fails. Run that mutation yourself and paste both results — the green and the red. A wiring test that has never been observed red is not yet evidence.

Make it a behavioural assertion, not a source.contains(...) check on Fleetd.java. A source-text check passes just as happily when the logic is wrong, and the repo already has too many of those.

One caveat you should decide rather than guess: leadConfigDirLookup is package-private and callable, but the new LeadConfigDirSource(...) wrapper currently sits inline in main. If pinning it cleanly needs that wrapper extracted into a small factory the way loopHealthSource was, do that — it is the same fix that made the sibling case testable.

Note on LeadSeatSource

It has the identical untested wrapper, and I am not asking you to fix it here. Say so in one line in your reply and I will file it. Do not touch it in this PR.

Everything else stands. Re-run the full build after the change and report the real counts.

Lead review. Good work — the design is right, the parse fix is right, and your own mutation check on `contextView` is exactly what I asked for. **One gap left, and it is the same defect one level up.** Not merging yet. ## Verified by me ``` mvn -o clean install (from fleetd/) exit: 0 reports=146 tests=1822 failures=0 errors=0 skipped=0 LeadContextGaugeTest 9/9 FleetMcpLeadContextGaugeWiringTest 2/2 FleetdLeadConfigDirLookupTest 6/6 ``` ## What is right `leadConfigDirLookup` reading `config.get()` live inside the lambda, not captured — correct, and matches the `leadSeatLookup` precedent. Extracting it as a package-private factory so a test can call it directly is the right shape; it is the same move `FleetdLoopHealthSourceWiringTest` documents. The parse fix is right, and I want to name why the *javadoc* is the best part of it. You did not just change the behaviour — you wrote down the old reasoning and why it fails. The deleted comment had actively rationalised the defect ("that is the honest degrade the three-state design exists for, not a defect"). A wrong rationale left in place is how a defect survives review twice. ## The gap: I mutated `Fleetd.main` and nothing noticed Your test mutation covered `contextView`. I ran the one a level above it — replacing main's call site with the inert source: ```java // was: new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(() -> config.get().profiles(), leaders)) FleetMcp.LeadConfigDirSource.none() ``` Result: ``` MUTANT: tests=1822 failures=0 errors=0 (mvn exit 0) ``` **The mutant survives.** The daemon can be changed to pass an inert source — every lead reads `<user.home>/.claude` again, the gauge reports `UNKNOWN` forever — and the entire suite stays green. That is not an equivalent mutant. `none()` and the real lookup genuinely differ in production; that difference *is* the bug this PR exists to fix. Your two tests are both real and both necessary, and neither one can catch this, for the reason this ticket started with: - `FleetMcpLeadContextGaugeWiringTest` builds its own `FleetMcp` and hands it its own source. It proves `FleetMcp` uses what it is given. - `FleetdLeadConfigDirLookupTest` builds its own lookup. It proves the lookup resolves correctly. Neither proves `Fleetd.main` hands the second to the first. **A test proves the instantiation it creates and nothing about any other instantiation** — which is the sentence in #602's own body, now true one level up. ## Precedent — this is a known shape here, with a known fix Read `FleetdLoopHealthSourceWiringTest`'s class javadoc. It documents this precise situation: five tests built their own `LoopHealthSource` with fixed lambdas, the call site could be replaced with a constant, 1771 tests stayed green, and a stalled poller would have been invisible. It names the family: #561 / #248 / #426. You are not being asked to invent anything. Follow that test. ## What to add One test in the `Fleetd*WiringTest` family that pins main's wiring, so replacing the argument at that call site goes red. Acceptance, as a property: **with the real wiring in place the test passes; with main's call site replaced by `LeadConfigDirSource.none()` it fails.** Run that mutation yourself and paste both results — the green and the red. A wiring test that has never been observed red is not yet evidence. Make it a behavioural assertion, not a `source.contains(...)` check on `Fleetd.java`. A source-text check passes just as happily when the logic is wrong, and the repo already has too many of those. One caveat you should decide rather than guess: `leadConfigDirLookup` is package-private and callable, but the `new LeadConfigDirSource(...)` wrapper currently sits inline in `main`. If pinning it cleanly needs that wrapper extracted into a small factory the way `loopHealthSource` was, do that — it is the same fix that made the sibling case testable. ## Note on `LeadSeatSource` It has the identical untested wrapper, and I am not asking you to fix it here. Say so in one line in your reply and I will file it. Do not touch it in this PR. Everything else stands. Re-run the full build after the change and report the real counts.
agent added 1 commit 2026-09-20 11:34:56 +02:00
Extract the inline new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(...))
construction in Fleetd.main into a package-private factory,
Fleetd.leadConfigDirSource, mirroring loopHealthSource/capacitySource/
healthCoverageSource. Add FleetdLeadConfigDirSourceWiringTest, which calls the
factory directly with real Profile/Leader fixtures and asserts the returned
source resolves a real configDir -- a property that is false if the factory's
body is mutated to return LeadConfigDirSource.none().

Neither FleetMcpLeadContextGaugeWiringTest nor FleetdLeadConfigDirLookupTest
could catch main losing this wiring: each builds its own instance instead of
calling what main calls. This closes that gap at the factory level, matching
the standard already accepted for loopHealthSource's own wiring test.
Author
Member

Addressed comment 17353's gap: Fleetd.main's own construction of LeadConfigDirSource is now pinned.

Change (new commit 4e27bde):

  • Extracted the inline new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(...)) in Fleetd.main into a package-private factory, Fleetd.leadConfigDirSource(profiles, leaders), in the same style as loopHealthSource/capacitySource/healthCoverageSource.
  • main's call site now reads leadConfigDirSource(() -> config.get().profiles(), leaders).
  • Added FleetdLeadConfigDirSourceWiringTest, which calls that factory directly with real FleetConfig.Profile/FleetConfig.Leader fixtures and asserts the returned source resolves a real configDir -- behavioural, no source.contains(...) on Fleetd.java.

Mutation results I ran myself, both real:

  1. Mutated Fleetd.leadConfigDirSource's own body to return FleetMcp.LeadConfigDirSource.none(); (the same logic that used to live inline at main's call site, now living inside the extracted factory) and ran FleetdLeadConfigDirSourceWiringTest alone: RED -- Tests run: 3, Failures: 1 -- AssertionFailedError: ... expected: </mnt/opus-claude> but was: <null>. Restored the real body and re-ran the full suite: GREEN -- Tests run: 1825, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
  2. I also tried the mutation exactly as you described it -- main's one-line call leadConfigDirSource(...) replaced with a bare FleetMcp.LeadConfigDirSource.none(), bypassing the factory entirely. That one stayed green (Tests run: 11, Failures: 0 on the three related test classes). No test in this codebase calls Fleetd.main far enough to observe which factory it invoked -- FleetdLoopHealthSourceWiringTest has the identical structural gap for its own one-line call in main. I documented both measurements in the new class's javadoc rather than silently claiming full coverage: this test pins the factory's own logic (the part that used to be the untested inline construction), and treats main's one-line delegation to it as visually-verifiable, matching the standard already accepted for loopHealthSource/capacitySource/healthCoverageSource. Flagging this explicitly in case you want a stronger guarantee than the established pattern provides -- happy to take a different approach if so.

Full build after restoring real wiring: mvn -o clean install -- Tests run: 1825, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Not touched: LeadSeatSource still has the identical untested-wrapper shape -- left alone as asked.

Addressed comment 17353's gap: `Fleetd.main`'s own construction of `LeadConfigDirSource` is now pinned. **Change (new commit 4e27bde):** - Extracted the inline `new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(...))` in `Fleetd.main` into a package-private factory, `Fleetd.leadConfigDirSource(profiles, leaders)`, in the same style as `loopHealthSource`/`capacitySource`/`healthCoverageSource`. - `main`'s call site now reads `leadConfigDirSource(() -> config.get().profiles(), leaders)`. - Added `FleetdLeadConfigDirSourceWiringTest`, which calls that factory directly with real `FleetConfig.Profile`/`FleetConfig.Leader` fixtures and asserts the returned source resolves a real `configDir` -- behavioural, no `source.contains(...)` on `Fleetd.java`. **Mutation results I ran myself, both real:** 1. Mutated `Fleetd.leadConfigDirSource`'s own body to `return FleetMcp.LeadConfigDirSource.none();` (the same logic that used to live inline at `main`'s call site, now living inside the extracted factory) and ran `FleetdLeadConfigDirSourceWiringTest` alone: **RED** -- `Tests run: 3, Failures: 1` -- `AssertionFailedError: ... expected: </mnt/opus-claude> but was: <null>`. Restored the real body and re-ran the full suite: **GREEN** -- `Tests run: 1825, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. 2. I also tried the mutation exactly as you described it -- `main`'s one-line call `leadConfigDirSource(...)` replaced with a bare `FleetMcp.LeadConfigDirSource.none()`, bypassing the factory entirely. That one **stayed green** (`Tests run: 11, Failures: 0` on the three related test classes). No test in this codebase calls `Fleetd.main` far enough to observe which factory it invoked -- `FleetdLoopHealthSourceWiringTest` has the identical structural gap for its own one-line call in `main`. I documented both measurements in the new class's javadoc rather than silently claiming full coverage: this test pins the factory's own logic (the part that used to be the untested inline construction), and treats `main`'s one-line delegation to it as visually-verifiable, matching the standard already accepted for `loopHealthSource`/`capacitySource`/`healthCoverageSource`. Flagging this explicitly in case you want a stronger guarantee than the established pattern provides -- happy to take a different approach if so. **Full build after restoring real wiring:** `mvn -o clean install` -- `Tests run: 1825, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. **Not touched:** `LeadSeatSource` still has the identical untested-wrapper shape -- left alone as asked.
ltms merged commit 3762aca307 into worker/lead-context-gauge-ad404f-1 2026-09-20 11:38:22 +02:00
Sign in to join this conversation.