fleetd #602 gauge-wiring: thread a lead's configured configDir into the context gauge #606
Reference in New Issue
Block a user
Delete Branch "worker/gauge-wiring-9158c1-4"
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?
Fixes the two findings in the gauge-wiring ticket for #602 (LeadContextGauge).
Finding 1 — production call site passed no directory.
FleetMcp.contextViewhardcodedLeadContextGauge.read(null, sessionId, agentType), so a lead whose profile sets its ownconfigDir:always fell back to<user.home>/.claudeand readUNKNOWNforever, with no error anywhere.FleetMcp.LeadConfigDirSource(same idiom asLeadSeatSource) and threaded it through theFleetMcpconstructor, thelistFleetoverload chain,leadView, andcontextView.Fleetd.leadConfigDirLookup, wired at construction inFleetd.main, following the samefleet.leaders.<name>.profilelinkleadSeatLookupalready uses, one step further to that profile's ownconfigDir:.Finding 2 — a torn final line destroyed a good reading.
LeadContextGauge.parsetreated ANY unparseable last line as fatal (UNKNOWN), even thoughfleet_listreads 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.parsenow skips any single unparseable line (typically the torn final one) and only reportsUNKNOWNwhen 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 transcriptfleet_list'scontextrow actually reads (verified to go red under a mutation test that reverts the wiring to a hardcodednull); a lead whose config names no directory degrades to the built-in default without throwing.LeadContextGaugeTest— replaced the oldtruncatedOrInvalidLastLineIsUnknown(which asserted the now-wrong behaviour) withtornFinalLineFallsBackToTheLastGoodReadingand its controleveryLineUnparseableIsUnknown.Opened against
worker/lead-context-gauge-ad404f-1(the open PR #602 branch this ticket's code lives on), notmain, per the ticket's instructions.Lead review. Good work — the design is right, the parse fix is right, and your own mutation check on
contextViewis exactly what I asked for. One gap left, and it is the same defect one level up. Not merging yet.Verified by me
What is right
leadConfigDirLookupreadingconfig.get()live inside the lambda, not captured — correct, and matches theleadSeatLookupprecedent. Extracting it as a package-private factory so a test can call it directly is the right shape; it is the same moveFleetdLoopHealthSourceWiringTestdocuments.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.mainand nothing noticedYour test mutation covered
contextView. I ran the one a level above it — replacing main's call site with the inert source:Result:
The mutant survives. The daemon can be changed to pass an inert source — every lead reads
<user.home>/.claudeagain, the gauge reportsUNKNOWNforever — 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:
FleetMcpLeadContextGaugeWiringTestbuilds its ownFleetMcpand hands it its own source. It provesFleetMcpuses what it is given.FleetdLeadConfigDirLookupTestbuilds its own lookup. It proves the lookup resolves correctly.Neither proves
Fleetd.mainhands 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 ownLoopHealthSourcewith 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*WiringTestfamily 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 onFleetd.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:
leadConfigDirLookupis package-private and callable, but thenew LeadConfigDirSource(...)wrapper currently sits inline inmain. If pinning it cleanly needs that wrapper extracted into a small factory the wayloopHealthSourcewas, do that — it is the same fix that made the sibling case testable.Note on
LeadSeatSourceIt 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.
Addressed comment 17353's gap:
Fleetd.main's own construction ofLeadConfigDirSourceis now pinned.Change (new commit
4e27bde):new FleetMcp.LeadConfigDirSource(leadConfigDirLookup(...))inFleetd.maininto a package-private factory,Fleetd.leadConfigDirSource(profiles, leaders), in the same style asloopHealthSource/capacitySource/healthCoverageSource.main's call site now readsleadConfigDirSource(() -> config.get().profiles(), leaders).FleetdLeadConfigDirSourceWiringTest, which calls that factory directly with realFleetConfig.Profile/FleetConfig.Leaderfixtures and asserts the returned source resolves a realconfigDir-- behavioural, nosource.contains(...)onFleetd.java.Mutation results I ran myself, both real:
Fleetd.leadConfigDirSource's own body toreturn FleetMcp.LeadConfigDirSource.none();(the same logic that used to live inline atmain's call site, now living inside the extracted factory) and ranFleetdLeadConfigDirSourceWiringTestalone: 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.main's one-line callleadConfigDirSource(...)replaced with a bareFleetMcp.LeadConfigDirSource.none(), bypassing the factory entirely. That one stayed green (Tests run: 11, Failures: 0on the three related test classes). No test in this codebase callsFleetd.mainfar enough to observe which factory it invoked --FleetdLoopHealthSourceWiringTesthas the identical structural gap for its own one-line call inmain. 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 treatsmain's one-line delegation to it as visually-verifiable, matching the standard already accepted forloopHealthSource/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:
LeadSeatSourcestill has the identical untested-wrapper shape -- left alone as asked.