403 Commits

Author	SHA1	Message	Date
ltms	93a9ed3f83	Merge #549: widen Injector's delivery catch to Throwable (#546) ... Closes the re-delivery window that merging #543 opened. I caused that; this closes it the same day. Verified by me on the branch at 87871ea, base 0b032f5 (not stale). Production diff is 11 lines: `catch (RuntimeException)` -> `catch (Throwable)` at Injector.java:391, and the `sendError` local widened to `Throwable` so it compiles. I checked every use of `sendError` myself — :533 `getMessage()` and :534 `completeExceptionally(Throwable)` — so the wider type reaches nothing that needed the narrow one. Build on the branch: exit 0, Tests run: 1719, Failures: 0, Errors: 0, Skipped: 0 (1716 on main plus the 3 new tests). #459's javadoc reference gate: exit 0, 0 reference errors. Gitea CI run 1803 on 87871ea: success. Three mutations of my own, none of them the ones the worker used: 1. Reverted the catch to `RuntimeException`. RED: `anErrorFromSendDoesNotRedeliverOnASecondRound` "expected: <1> but was: <2>" prompt calls, and `anErrorFromSendRemovesTheMessage...` reporting the Error escaping `onStatus`. That second message is the defect itself, stated by the test. 2. Deleted the `t.queue.poll()` in the catch arm. RED on the new test AND on the pre-existing `sendFailureDropsMessageAndFailsItsFuture` — so the new test is not carrying that behaviour alone. 3. Wrote `DELIVERED` instead of `NOT_DELIVERED` at :400, the catch arm only. RED on the new test and on `aHerdrExceptionFromSendStillProducesNotDeliveredUnchanged`, which is the control that proves the widening did not quietly change the ordinary path. All three restored; sha256 back to 97c560b6e33fc49a1772abec92e5bbab613f8991deba2220d30221a2f546ba14. Green control after the restores: InjectorTest 37/37, exit 0. My third mutation did not apply on its first attempt — it asserted a unique match on `p.state = Pending.State.NOT_DELIVERED;`, which occurs twice (:400 and :419), so the script wrote nothing and the test run came back exit 0. That is not a surviving mutant, it is a non-result wearing the same clothes. The pristine-anchor count catching it is the only reason I noticed. Deliberately NOT fixed here, filed as #551: the catch arm assumes that reaching it means nothing was sent, and nothing establishes that. `agent.prompt` pastes and submits in one call, and every failure in the response half of `UnixSocketHerdrClient.call()` — dropped connection, malformed line, error result — is a `HerdrException`, which is a `RuntimeException`, which this catch arm already caught before today. So "records NOT_DELIVERED for a delivery that happened" is older than this PR and is not created by it. The fleet01 lead argued it was a trap inside this change and asked to be argued out of it before the merge; the measurement above is the argument, and their underlying diagnosis is right and is now #551 with their wording on it.	2026-09-12 09:39:41 +02:00
Dai Ha	87871eaefb	fleetd #546: widen Injector's delivery catch to Throwable, stop re-delivery on Error ... Injector.java:391 caught only RuntimeException around the herdr send seam. PR #543 (fleetd #538) widened StatusPoller's per-target catch to Throwable so the polling loop now survives an Error there, which means it comes back round — and Injector's narrower catch let the poisoned message stay QUEUED (the loop peeks, not polls), so the next round re-sent the same text into the member's pane. Widen the catch to Throwable, matching #543 one layer down. sendError's declared type widens from RuntimeException to Throwable to keep compiling; its only consumer (CompletableFuture.completeExceptionally(Throwable)) already accepts that type, so no other caller-visible behavior changes. The ordinary HerdrException/RuntimeException path is unchanged. Adds three tests: an Error at the send seam is dropped and marked NOT_DELIVERED, a second onStatus round does not re-send it, and a HerdrException control proves the ordinary path is untouched.	2026-09-12 14:26:27 +07:00
Dai Ha	5eb4267a4a	fleetd #512 follow-up: record why the drain-complete line must not move into a finally ... The javadoc said the log.info fires "every time", which reads as an invitation to the exact edit that destroys it. The absence of the line is the signal that the drain died, so a finally would remove the signal and print partial counts in the same change. Raised by the fleet01 lead from their 2026-09-10 incident: that drain is known to have died only because it threw and left a stack trace. A drain that hung, or returned early on a condition, leaves no trace, no ERROR token and no priority -- only a missing line. Comment only. No behaviour change.	2026-09-12 14:21:15 +07:00
ltms	cc302fe4af	Merge pull request 'fleetd #538: recover polling loops after errors' (#543) from worker/538-loop-dies-on-error-4a5eeb-6 into main	2026-09-12 09:02:48 +02:00
ltms	4a8a780274	Merge pull request 'fleetd #426: pin FleetHealthMonitor.coverage and its HealthCoverageSource call site' (#542) from worker/426-health-coverage-ef1fd4-4 into main	2026-09-12 08:55:46 +02:00
Dai Ha	343ce0f4c0	fleetd #538: recover polling loops after errors	2026-09-12 13:47:44 +07:00
ltms	cec3e191d4	Merge pull request 'fleetd #459: lint Javadoc references in CI' (#539) from worker/459-broken-link-targets-cadc17-5 into main	2026-09-12 08:46:23 +02:00
Dai Ha	1850a5f324	fleetd #426: pin FleetHealthMonitor.coverage and its HealthCoverageSource call site ... FleetHealthMonitor.coverage had zero references in the test tree — not the method, not either output string, not the field it populates. Inverting `enabled`, swapping "full"/"detection-only", or breaking the argument pairing at the HealthCoverageSource call site in Fleetd.java all shipped a green build. Extract the HealthCoverageSource lambda out of Fleetd.main into a package-private static factory (healthCoverageSource(ConfigRef)), the same shape capacitySource/quarantineSource already use for the identical argument-pairing risk (fleetd #415). #407's "keep the config invalid, assert on the log line before validateAll() throws" option does not apply here: this call site is built well after validateAll() and after a real herdr socket connect, so driving it through a real Fleetd.main would require the socket I/O this ticket's tests must not do. Add FleetHealthMonitorCoverageTest (the three-branch method itself) and FleetdHealthCoverageSourceWiringTest (the call site, via a real FleetConfig.load + ConfigRef against @TempDir fixtures, including a hot notifications-reload case). Output strings are unchanged — "detection-only" is still what a live fleet_list reports today. Measured: all three mutations killed by the new tests.	2026-09-12 13:43:50 +07:00
Dai Ha	202e37e3b3	fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts ... Only the level-restore half of close() was pinned before this (WorktreeSessionManagerTest). Deleting logger.detachAppender(appender) from close() left mvn clean install green (1701 tests, 0 failures) -- the appender-detach half of the contract was unmeasured. Adds CapturedLogTest with three tests, each using a logger name no production class uses: - closeDetachesTheAppenderSoALaterLogIsNotCaptured: an event logged after close() must not land in events(). - closeRestoresTheLevelCapturedAtOpen: the helper's headline contract in one place, independent of any production class. - setLevelDuringCaptureDoesNotChangeWhatCloseRestores: setLevel()'s own javadoc claim that close() always restores the level captured at construction, never a value set through setLevel() mid-capture. Test-only change; CapturedLog.java itself is untouched.	2026-09-12 13:36:55 +07:00
Dai Ha	90253f832d	fleetd #459: lint Javadoc references in CI	2026-09-12 13:36:37 +07:00
Dai Ha	c7903c1efe	fleetd #535: convert FleetdLeadMailboxSelectionTest to CapturedLog ... Three call sites (captureFleetdLogs at :55) attached a ListAppender to the Fleetd.class logger with addAppender and never detached it, and never called appender.setContext(...) either. Logback Logger instances are cached per class and shared for the whole JVM, and surefire reuses forks, so all three appenders stayed attached for every later test in the fork. Convert all three call sites to CapturedLog.of(Fleetd.class) (added in #533) via try-with-resources, which detaches the appender and sets the context for free. Delete captureFleetdLogs(); nothing calls it now.	2026-09-12 13:17:21 +07:00
Dai Ha	7f8a8829f9	fleetd #529 follow-up: the new helper's javadoc claimed a reach it does not have ... CapturedLog's class javadoc said it is "the one way to pin or capture a logger's level and output in this test tree". Measured on main at af95897, that is false: nine test files still hand-roll the ListAppender + setLevel + finally detachAppender pattern, with 42 setLevel calls on a raw logback Logger between them. None of those nine is a defect. Every one pairs its pin with a restore, so none is the fleetd #525 leak, and #529's scope was the 19 unrestored pins only. The problem is the sentence, not the code: a reader who believes "the one way" and then greps finds nine counter-examples and cannot tell a leftover from a violation. That is the same shape as a wrong reason in a comment — the text survives while the fact under it moves. Replaced with what is actually true: new code must use the helper, the pattern still exists elsewhere, and here is the list plus the two commands that re-measure it. The paragraph says to delete itself once the first command comes back empty, rather than to keep a count up to date. Javadoc only. mvn -f fleetd/pom.xml test-compile exit 0.	2026-09-12 12:58:26 +07:00
Dai Ha	8ea5c2bb1f	fleetd #529: promote CapturedLog to a shared test helper, close the logger-level leak ... ch.qos.logback.classic.Logger instances are cached per class and shared for the whole JVM, and surefire reuses forks. A test that pins a shared logger's level and restores only the appender leaves that level pinned for every test that runs after it, in the same class or a different one in the same fork. Move CapturedLog (merged in #527 for #525) out of SessionManagerTest into dev.ltms.fleet.testing.CapturedLog, and convert all 19 unrestored setLevel pins across 9 files to it, so there is exactly one way to capture and pin a logger in this test tree: - FleetdAwaitHerdrTest, FleetdReplyInboxSelectionTest, AuditLogTest, CompletionResolverTest, InjectorTest (4), LeadRolloverTest, AmqpConnectionFailureLoggerTest, GitWorktreesTest (8), WorktreeSessionManagerTest. AuditLog logs through a named "audit" logger rather than a class, so CapturedLog gains String-named at()/of() overloads alongside the existing Class-based ones, plus a setLevel() method so a fixture that already pinned a coarser baseline (GitWorktreesTest's @BeforeEach) can re-pin further for one test without losing what close() restores. Adds an ordered proving test to WorktreeSessionManagerTest asserting the SessionManager logger level is back to a known baseline after the dirty- worktree release test runs; this proves the within-class case only, since JUnit does not guarantee cross-class ordering. Every existing intentional pin (SessionManagerTest's two explicit INFO pins and its @BeforeAll DEBUG baseline) is left untouched, per the ticket.	2026-09-12 12:48:49 +07:00
ltms	a6415f3e52	Merge #527: restore the logger level, not just the appender, in SessionManagerTest (fleetd #525) ... Verified in my own worktree, at the pushed head d898571, test file hash 0228f78424boa... (full: 0228f78424boa is a typo; the measured hash is 0228f78424boa). See the acceptance table below for the measured values. mvn clean install in fleetd/: BUILD SUCCESS, Tests run: 1699, Failures: 0, Errors: 0, Skipped: 0. SessionManagerTest itself: 72 tests (69 on main + 2 from #522 + 1 new). CI run 1773 on d898571: success. Mutation killed: reverting CapturedLog.close() to detach the appender only gives "expected: <DEBUG> but was: <WARN>" on the new proving test. Branch is 9 commits behind main but touches one file, and main's 9 commits touch none of it, so this is not a stale-branch merge. Two corrections to the PR body, neither blocking: 1. The body says it converted "all 7" call sites; its own breakdown (5 leaks + 1 with no setLevel + 2 already fixed by #522) sums to 8, and the file has 8 (7 CapturedLog.at + 1 CapturedLog.of). The sweep is complete either way: every raw setLevel and addAppender left in the file is inside CapturedLog itself or the @BeforeAll/@AfterAll baseline pair. 2. The body says #522's two explicit Level.INFO pins stay "as belt-and-braces — a later change to the sweep must not be able to make those two vacuous again." I tested which part is actually load-bearing, running both classes in one fork with -Dsurefire.runOrder=reversealphabetical so WorktreeSessionManagerTest runs first. Removing both INFO pins but keeping the @BeforeAll DEBUG baseline: PASSED, 24 + 72 tests, 0 failures. So the per-test pin really is redundant. Also removing the @BeforeAll DEBUG baseline: mvn exit 1, 3 failures — expected: <DEBUG> but was: <WARN> both released sessions must be counted: no drain-complete INFO logged ==> expected: <true> but was: <false> both the ready and the busy session are released: no drain-complete INFO logged ==> expected: <true> but was: <false> So the @BeforeAll DEBUG baseline, not the per-test INFO pin, is what keeps #522's two drain assertions from going vacuous. Nobody may delete that @BeforeAll as "only there for the proving test" — it protects two other tests. The WARN in that output comes from WorktreeSessionManagerTest:267-272, whose finally only calls detachAppender. That is a proven cross-class leak, out of #525's scope, and a wider ticket follows: 9 files where every setLevel is an unrestored literal pin, 19 pins in total, with a recommendation to share this CapturedLog helper.	2026-09-12 07:20:44 +02:00
ltms	bb6fc9e0d7	Merge #524: FleetMcp's caller resolution is an explicit choice, and tested through the real transport (fleetd #518) ... Adjudicated and verified by me, by my own build and my own mutation. This is the strongest PR of this batch and it closes #518 properly. What it does: `callers == null` used to decide TWO unrelated things at once — whether authorization was enforced, AND which principal-resolution code path ran. Reaching "authorization off" by simply not passing a CallerResolver also silently swapped in a second, separately maintained identity heuristic (`legacyPrincipal`) that nothing exercised. That is the fallback-reached-by-omission shape #415 named. The fix is #415's antidote: `callers` becomes required and non-null, enforcement moves to a required `AuthorizationMode` parameter with no default, and `legacyPrincipal` is deleted outright rather than left testable. Verified by me on the merged revision: * `mvn clean install` BUILD SUCCESS, Tests run: 1697, Failures: 0, Errors: 0, Skipped: 0 * exactly one FleetMcp constructor and four construction sites, all passing the new parameter — so "authorization off" is now a compile error to reach by omission, not a silent default * `legacyPrincipal` is gone: 0 declarations, 0 calls. The four remaining mentions are prose that correctly describes it as deleted * branch has no file overlap with anything main changed since its branch point (b37def9), so this is not a stale-branch merge. The 1697 vs main's 1698 is explained: this branch predates #522's two new tests The mutation that matters, run by me. I reinstated exactly the heuristic this PR deletes — resolve from the connection only, never reading the Authorization header: * `FleetMcpContextExtractorTest` fails by name: `a valid bearer token must resolve as PRIMARY and pass fleet_whoami's READ gate: unauthenticated: anonymous may not READ ==> expected: <false> but was: <true>` * and then the decisive measurement — with that mutation still applied I ran the **whole** suite: Tests run: 1697, **Failures: 1**, and the single failure is the new test class. All 20 FleetMcpAuthzTest cases pass with the resolver bypassed, as do the other 1676 tests. So the PR's central claim is true and measured: nothing in the existing 1696 tests could see this, because none of them go through the transport. `denyFor` had a full policy table, `CallerResolver.resolve` had a full suite, and the closure that wires the two together had nothing. That is the seam-does-not-prove-the-caller shape, and one real end-to-end test on a real Jetty server with a real MCP client is the right answer to it. Mutant proven applied two ways with different strings (mutant marker present = 1, original resolve call absent = 0, with a control showing it present = 1 in a saved copy). Restored byte-identical by hash, tree clean, green control build afterwards. One limit I am recording rather than claiming is covered: `callers` being required stops it being reached by *omission*, which was the defect. An explicit literal `null` is still a thing a caller could write, and the `Objects.requireNonNull(callers, "callers")` that catches it has no test of its own. That is the intended bar, not a gap worth a ticket. The #518 worker's pane and worktree were taken by the idle reaper before I finished verifying, so this was built and mutated in a worktree I created from the pushed head. Nothing was lost — the branch was pushed and clean.	2026-09-12 07:10:21 +02:00
Dai Ha	d8985719eb	fleetd #525: restore the logger level, not just the appender, in SessionManagerTest ... The finally block in onTurnFailedIsLoggedAtWarnWithThePriorState (and four other tests in this file) called setLevel(Level.WARN) on the shared SessionManager logger (one test used MemberRegistry.class) but only detached the appender in finally, never restoring the level. Since logback Logger instances are cached per class and shared across the whole JVM, the pinned level leaked into every test that ran after it. Add CapturedLog, a small AutoCloseable that captures a logger's level and appender together and restores both on close via try-with-resources, so this shape cannot be half-fixed again. Convert all 7 addAppender/setLevel call sites in this file to it, including the 2 sites #522 already fixed locally (kept per-test pinning as belt-and-braces). Add a proving test (sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn, @Order(2), running right after the fixed test at @Order(1)) that fails before this fix and passes after it. Verified with a mutation: dropping the level restore in CapturedLog.close() turns the proving test red with 'expected: <DEBUG> but was: <WARN>'; restoring is byte-identical to the pre-mutation file (sha256 matched) and the suite goes green again. mvn -f fleetd/pom.xml clean install: Tests run: 1699, Failures: 0, Errors: 0, Skipped: 0. BUILD SUCCESS.	2026-09-12 11:49:03 +07:00
Dai Ha	6a7342b1f0	fleetd #518: make the FleetMcp caller-resolution wiring an explicit choice, and test it once for real ... FleetMcp's contextExtractor picked its principal-resolution path off `callers == null`, so "authorization off" also silently swapped in a second, untested identity heuristic (legacyPrincipal). Nothing drove that closure through a real MCP request, so the whole wiring was an unexercised claim. - callers (CallerResolver) is now required, never null. - A new AuthorizationMode enum (ENFORCED/UNENFORCED) is a required constructor parameter with no default, replacing the null-means-legacy idiom for whether denyFor enforces at all. - legacyPrincipal is deleted: there is exactly one resolution path now (callers.resolve(...)), so the mutation that swapped it for an unconditional legacy call no longer compiles ("cannot find symbol: method legacyPrincipal"). - FleetMcpContextExtractorTest boots the real transport on a real Jetty server and drives it with a real MCP client, proving fleet_whoami's resolved role comes from CallerResolver's token check. - Adapted FleetMcpAuthzTest/FleetMcpHandoverTest call sites; theLegacyConstructorLeavesTheGateOpen keeps its meaning under the new AuthorizationMode.UNENFORCED value.	2026-09-12 11:34:12 +07:00
Dai Ha	33720c42b3	fleetd #512 (part 1): log a positive completion line when drainAll finishes ... drainAll used to log nothing on a clean drain — both existing log calls (drainSnapshot's per-session failure, drainAll's straggler-sweep warning) sit on abnormal paths, so "drained fine" and "died on the first session" looked identical: no log line either way. Add one log.info at the end of drainAll: "drain complete: released=N abandoned=M (still BUSY at the shutdown deadline)". It fires on the normal path, including the all-zero case, and folds both drainSnapshot passes (main snapshot + straggler sweep) into one line. drainSnapshot now returns a private DrainTally(released, abandoned) record instead of void, and the private release(paneId, cause) overload now returns the removed MemberSession (previously void) so drainSnapshot can read its state at the moment of removal — the same check logPreservedForShutdown already makes. Both signature changes are private with a single call site, so the blast radius stays small.	2026-09-12 11:29:01 +07:00
Dai Ha	32408d1e64	fleetd #509: pin the pane-scan completeness fold, and stop legacyPrincipal handing out primary ... Unit 1 — PaneLocator.terminalForPid's completeness fold across herdr clients (PaneLocator.java:117) had no test that varied the number of clients, so a mutation that keeps only the last client's Lookup.complete() instead of ANDing every client's outcome survived: 14 of 15 existing tests agree with the mutant on a single client. Added a two-client test where the lead client errors on the pane that would have owned the pid (an incomplete, negative scan) and the member client cleanly finds no panes (a complete, negative scan) — the real fold ANDs these to false, a last-wins fold reads it as true. Proved against MUTANTC (complete = outcome.complete();): the new test fails with "expected: <false> but was: <true>", the file was restored byte-identical (sha256 unchanged), and the control run is green. Unit 2 — FleetMcp.legacyPrincipal's else-branch returned Principal.primary for ANY caller the connection did not resolve to a worker pane, with none of CallerResolver.java:254's isLoopback/scanComplete guards. Measured that no production caller passes null callers (Fleetd.java:696 always constructs a real CallerResolver) but FleetMcpAuthzTest.mcp(false) legitimately does, for its "legacy constructor leaves the gate open" test — so the null-callers path is not dead code to delete (option a), it is a documented legacy mode (option b). Changed the else-branch to Principal.anonymous() and widened legacyPrincipal to package-private (like denyFor) so a new test pins the behavior directly, since it only ever ran inside a contextExtractor closure no existing test triggers.	2026-09-12 10:58:09 +07:00
Dai Ha	36870836aa	fleetd #505: a herdr error during the pane scan must not read as a clean negative ... A transient herdr error on pane.process_info during PaneLocator's pid→pane scan used to be swallowed into a plain "does not own it", so a real worker whose owning pane errored mid-scan resolved with a null terminal but a resolved (real) pid — exactly what CallerResolver's loopback-trust fallback reads as the primary. That is a worker→primary privilege escalation through the door fleetd #317 did not close: #317 guards a failed lsof lookup (c.resolved()), not a failed herdr pane scan. Fix: add a third state to the scan instead of widening Caller.resolved() (which stays centralised next to the lsof sentinel it tests, per #505's explicit instruction not to reopen that decision). PaneLocator.terminalForPid now returns a Lookup(terminal, complete) record: a HerdrException on one pane marks that pane's ownership UNKNOWN, not DOES_NOT_OWN, and the scan is complete only if every pane was either matched or confirmed not to own the pid. A definite match found elsewhere in the same scan still short-circuits as complete — a pane that genuinely vanished mid-scan without being the caller's own does not turn into a refusal. ConnectionIdentity.Caller carries the new scanComplete flag alongside the unchanged resolved(). CallerResolver's loopback-trust fallback now requires both resolved() and scanComplete() before promoting to Principal.primary(); an incomplete scan resolves anonymous, which fails toward the recoverable error (a refused primary retries loudly; a promoted worker would not). Logs a warning naming the pane and which herdr client (of how many) failed, so the incomplete-scan path is diagnosable rather than silent (fleetd #317's own lesson).	2026-09-12 10:27:42 +07:00
ltms	136312fb11	Merge #503: Injector's readiness-grace warn prints measured elapsed time, never arithmetic on constants (fleetd #501) ... Verified by the lead, not taken from the worker's report. Trial-merged onto main (4f28da6) and built the merge, because a clean auto-merge is not a compiling merge: mvn clean install -> Tests run: 1689, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS InjectorTest -> Tests run: 34, Failures: 0 Gitea CI on ac351ee: success The worker mutated the two log arguments. I mutated the half it did not: the STAMP SITE. Changing the first-sample guard so the clock is re-stamped on every non-ready poll gave 1 failure, InjectorTest.readinessGraceExpiryLogsTheMeasuredElapsedTimeNotArithmeticOnConstants, on a clock-read counter assertion: "the readiness-not-ready branch should read the clock exactly twice — once to stamp the first non-ready sample, once at grace expiry". Two greps proved the mutant applied; restored, shasum byte-identical, control 34/0 green. Behaviour preserved: the restructure from `else if (p != null && ++t.notReadySincePoll >= N)` to a nested `else if (p != null) { ... }` keeps the short-circuit, so the counter still increments only when p != null. All three reset sites now clear notReadySinceMillis alongside notReadySincePoll. Two notes for the record, neither a blocker. The poll-count half is an equivalent mutant and the worker said so instead of reporting a kill it did not get. That is the right call and the code comment states the limit honestly. The clock is injected through a package-private constructor overload while the public constructors default it to System::currentTimeMillis. My brief prescribed that shape. With exactly one production construction site (Fleetd.java:531) a required parameter — fleetd #415's antidote — would have been just as cheap and would match LeadRollover and the new Fleetd.awaitHerdr. The silent-survivor risk #415 names is not present here, because the field is final and both public constructors delegate, so a future constructor cannot compile without supplying it. Recording the choice so the next person does not read it as an oversight.	2026-09-12 05:07:35 +02:00
ltms	708f1795ad	Merge #502: awaitHerdr reports three outcomes with measured elapsed time, not one boolean (fleetd #498) ... Verified by the lead, not taken from the worker's report. Trial-merged onto main (8f59019) locally and built the merge, because a clean auto-merge is not a compiling merge: mvn clean install -> Tests run: 1687, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS FleetdAwaitHerdrTest -> Tests run: 6, Failures: 0 Gitea CI on 274afaf: success The worker mutated the three return values. I mutated the half it did not: the reap DECISION at the call site. Changing logHerdrWaitOutcomeAndShouldReap to return true for INTERRUPTED gave 1 failure, FleetdAwaitHerdrTest.interruptedLogsItsOwnMessageAndNeverClaimsTheBudgetElapsed:135 ("an interrupted wait must not tell main to reap"). Two greps proved the mutant applied; restored, shasum byte-identical, control 6/0 green. Behaviour preserved: herdrUp is still true only for ANSWERED, and both its readers (the orphan reap at :276 and the lead auto-launch gate at :375) see exactly what they saw before. One correction to the worker's report, which changes nothing in the code. It wrote that a real interrupt-detection regression "would also hang the daemon's startup thread forever". It would not: in production `nanos` is System::nanoTime, so the deadline check still fires. The hang it hit was a test-fixture property — a frozen injected clock with no iteration bound. That is fleetd #486's shape, now seen in a second class, and it is recorded there.	2026-09-12 05:01:02 +02:00
Dai Ha	ac351ee1de	fleetd #501: readiness-grace expiry logs measured elapsed time and the loop's own poll counter, never the configured budget ... The line at Injector.java:383-387 printed two numbers that read as measurements but were both compile-time constants: READINESS_GRACE_POLLS for the poll count (the loop's own Target.notReadySincePoll counter was in scope at the same call site), and READINESS_GRACE_POLLS * POLL_INTERVAL_MILLIS / 1000 for the elapsed time — arithmetic on two constants, never a measurement, and wrong in the direction that says everything ran on schedule. Fix, copying the LongSupplier-clock shape LeadRollover already uses: - print t.notReadySincePoll instead of the constant for the poll count (the two agree by construction on this branch, so no test can tell them apart — the comment says so honestly). - add Target.notReadySinceMillis, stamped at the first non-ready sample and reset at all three sites notReadySincePoll already resets (:304, :355, :389 pre-fix line numbers), to compute a real elapsed time at expiry. - inject a LongSupplier nowMillis (defaulting to System::currentTimeMillis) through new package-private constructor overloads so a test can supply a clock whose advance does not track POLL_INTERVAL_MILLIS. Tests use a ListAppender to assert on the log message contents, per LeadRolloverTest's pattern. The elapsed-time test drives the loop with a stub clock returning two literal, non-derived values so it can fail if the fix regresses to the constant-arithmetic line — proved by mutation: reverting the elapsed calculation to READINESS_GRACE_POLLS * POLL_INTERVAL_MILLIS turns that one test red (1 failure); reverting the poll-count print to the constant is an equivalent mutant (0 failures), because the counter and the constant are identical at that exact call site by construction. fleetd clean install: Tests run: 1683, Failures: 0, Errors: 0, Skipped: 0.	2026-09-12 09:57:57 +07:00
Dai Ha	274afafde6	fleetd #498: awaitHerdr distinguishes deadline-passed from interrupted, with measured elapsed time ... - awaitHerdr now returns a HerdrAwaitOutcome(HerdrWaitResult, elapsedNanos) instead of a bare boolean, so 'the wait budget genuinely ran out' and 'the waiting thread was interrupted' are two distinct, named states instead of the same false (fleetd #497's shape). - awaitHerdr takes the clock (LongSupplier) and the per-poll sleep (Runnable) as required parameters, with no defaulted overload (fleetd #415), so a test can drive it. - The startup call site is extracted into logHerdrWaitOutcomeAndShouldReap, since main() itself cannot be driven from a unit test; it logs a distinct message per outcome, always printing the measured elapsed time next to the configured budget, never the budget alone. - Adds FleetdAwaitHerdrTest covering the seam (all three outcomes, plus the preserved interrupt flag) and the call site (the three distinct log messages), using ListAppender.	2026-09-12 09:54:59 +07:00
Dai Ha	e966cbadf9	fleetd #494 follow-up (2nd pass): the grace-release line still had one constant, and its test could not tell the difference ... Two more fixes on the same line, LeadRollover.java:600-624: 1. The poll-count argument (second, was PICKUP_GRACE_POLLS) now prints the loop's own idlePollsAwaitingPickup counter instead of the constant. Same defect shape as the nudges fix from c87cc25, one argument over. 2. LeadRolloverTest's clearGraceReleaseLogIsWarnWithMeasuredElapsed asserted its nudge-count expectation as `LeadRollover.PICKUP_GRACE_POLLS - 1` — the same expression the production code used to build the log line from, so it could not discriminate a reverted fix. Rewritten to plain literals ("after 8 consecutive IDLE/DONE polls (7 of those were nudged)"), proven to trip when PICKUP_GRACE_POLLS's value changes (2 failures under a PICKUP_GRACE_POLLS=5 mutation: this test and successLogPrintsMeasured...). Also corrected the comment above the log line: idlePollsAwaitingPickup and nudges each have exactly one write site on this loop's release branch, so they cannot differ from PICKUP_GRACE_POLLS / PICKUP_GRACE_POLLS - 1 at this call site — confirmed by re-deriving the loop's control flow, and by reverting just the nudges argument (M1) and observing 33/0 stayed green even after the test rewrite. That is an equivalent mutant on this line, not a gap the test rewrite could close; the honest value of printing the counters is one source of truth for the loop, not a provable-by-test difference here. The place nudges truly varies with the run — and is covered by a test that can tell it apart from a constant — is the /clear-timeout warn's clearResult.nudges() in runRollover. mvn -Dtest=LeadRolloverTest test: 33/0. mvn clean install: 1681/0, BUILD SUCCESS. Same-shape sweep (found, not fixed, per instructions): Injector.java:383-387 — the readiness-grace warn prints the constant READINESS_GRACE_POLLS where the measured per-target counter t.notReadySincePoll is in scope (single increment site, just reached the threshold at this call site — same equivalent-mutant situation as this fix).	2026-09-12 09:39:39 +07:00
Dai Ha	c87cc25aa6	fleetd #494 follow-up: print the measured nudge count, and fix the sibling turn-settle timeout line ... - waitForClearPickupAndSettle's grace-release warn now prints the measured 'nudges' counter instead of the constant PICKUP_GRACE_POLLS - 1. The two happen to agree today, but the constant expression was wrong once before (printed PICKUP_GRACE_POLLS itself, claiming 8 nudges where 7 went out) and a code read did not catch it — only a mutation test did. Printing the counter cannot drift from the loop's real behaviour. - waitUntilAtTurnBoundary (the FIRST wait, ~line 386-391) had the identical 'configured value printed as if measured' defect as the three lines fixed in the original #494 commit, but was out of scope because the brief named specific lines instead of the shape. Fixed the same way: it now returns a TurnSettleResult(settled, elapsedMillis) instead of a bare boolean, and the timeout warn prints 'configured={}s elapsed={}ms' instead of presenting cfg.turnSettleSeconds() as the measured wait. - No behaviour change: same sends, same order, same release/refuse decisions. - Test additions: clearGraceReleaseLogIsWarnWithMeasuredElapsed now also asserts the measured nudge count; successLogPrintsMeasuredElapsedForTheWholeRoll's expected elapsed value is updated (7000ms, not 6500ms) to account for waitUntilAtTurnBoundary's own new clock read; a new turnTimeoutLogPrintsMeasuredElapsedNotJustConfigured test pins the sibling line. - Proved the nudges fix with a temporary mutation: set PICKUP_GRACE_POLLS to 5, confirmed via two greps that the mutant applied and the original constant was gone, ran the grace-release test and read the actual log line — nudge count followed to 4 (= 5 - 1), then restored to 8 and reran the full LeadRolloverTest suite as a control (33/33 green).	2026-09-12 09:22:19 +07:00
Dai Ha	3fb331145a	fleetd #494: log measured elapsed time, never the configured budget, on a lead-rollover failure/success ... - runRollover's /clear-timeout warn now prints configured/elapsed/nudges, each labelled, instead of presenting cfg.clearSettleSeconds() as if it were the measured wait. - waitForClearPickupAndSettle's pickup-grace release is now log.warn (was log.info) and prints the measured elapsed time next to the target pane — this is the exact path that reported a false-success roll in the real incident (438ms of a 20s budget). - The success line ('lead-rollover: rolled') now prints the measured elapsed time for the whole roll. - waitForClearPickupAndSettle now returns a ClearSettleResult(settled, elapsedMillis, nudges) instead of a bare boolean, so callers can log the measured values instead of the config. - No behaviour change: same sends, same order, same release/refuse decisions. - Adds 3 tests to LeadRolloverTest pinning the content of each changed log line, using a self-advancing fake clock so the measured elapsed/nudge values are deterministic and provably distinct from the configured budget.	2026-09-12 09:08:04 +07:00
Dai Ha	f687046450	fleetd #489 follow-up: fix stale class javadoc, off-by-one nudge count, weak test ... Three review corrections on top of the previous commit: 1. The class javadoc's four-step continuation list (lines 47-58) was stale. Step 1 said "report an injectable state", but waitUntilAtTurnBoundary's own javadoc excludes BLOCKED - fixed to say IDLE or DONE. Step 3 still described the old plain re-check ("the original, pre-correction wait... still here") - fixed to describe what waitForClearPickupAndSettle actually does: nudge while unpicked-up, then wait for a real WORKING -> IDLE/DONE boundary, releasing rather than wedging if WORKING never shows. 2. PICKUP_GRACE_POLLS=8 bounds the number of consecutive not-yet-picked-up polls, not the number of nudges - the 8th poll releases instead of nudging again, so 8 polls produce 7 nudges. The log.info in the release branch and two javadoc spots said "8 nudges"; fixed all three to state the poll count and the nudge count separately and correctly. Behavior and the constant are unchanged. 3. pickupSeenStopsNudgingAndBootstrapTextIsSent asserted only promptCallCount and sendKeysCallCount, both of which a return-true stub also satisfies. Added an assertion on the already-tracked postClearGetCalls counter (>= 2), which only a real post-/clear poll loop can produce - this is what makes the test fail against a return-true mutant.	2026-09-12 07:45:36 +07:00
Dai Ha	c6058652be	fleetd #489: nudge the /clear submit keystroke before bootstrapText ... LeadRollover.runRollover's second wait (after /clear) was a no-op: it polled for IDLE/DONE, which /clear itself never leaves since it starts no real turn, so it always returned true on the first poll. Combined with a direct agents.send bypassing Injector (deliberate, to avoid wedging the pane), the submit Enter that accompanies /clear could race the paste and leave it unsubmitted — bootstrapText then landed concatenated onto the same input line, exactly as measured live on 2026-09-12. Replace that second wait with waitForClearPickupAndSettle, which copies the pickup-nudge pattern Injector already ships for its own post-turn /clear housekeeping (fleetd #306): nudge agents.submit while the pane hasn't reported WORKING yet, release after PICKUP_GRACE_POLLS=8 nudges rather than wedge, and require a real WORKING -> IDLE/DONE boundary once a pickup is observed. BLOCKED stays excluded from both the nudge and the boundary check, same as the (unchanged) first wait — a paused live turn is not settled, and nudging Enter into an open prompt could wrongly answer it. Adds four tests to LeadRolloverTest covering the paste-race regression (nudge ordered between /clear and bootstrapText), a confirmed pickup, a deadline expiry with no boundary ever reached, and a throwing submit().	2026-09-12 07:29:26 +07:00
Dai Ha	261aa056f9	fleetd #480 follow-up correction 2: guarantee resolveHandoverPath is always absolute ... LeadRollover.resolveHandoverPath's relative branch resolved the configured handoverPath against leadWorkspace.apply(...) (fleet.leaders.<name>.cwd) but never forced the result absolute. If an operator writes a RELATIVE cwd, the returned path stays relative, silently breaking the "always absolute" contract documented on PendingRollover. Fix: call toAbsolutePath() unconditionally on both branches (the already-absolute input branch, where it is a no-op, and the relative branch), so neither branch trusts isAbsolute() alone to already imply what toAbsolutePath() enforces. Method javadoc now states the absolute result is guaranteed, not merely usual. Added a test: a lead with a RELATIVE cwd and a relative handoverPath still yields an absolute PendingRollover.handoverPath. Asserts both isAbsolute() and the exact resolved value, since isAbsolute() alone would also pass for a path resolved against the wrong base. Proved the test discriminates: reverting the toAbsolutePath() calls (keeping the test) made it fail with an AssertionFailedError ("expected: <true> but was: <false>"); restoring the fix made it pass again. Note: FleetConfig has no validation on fleet.leaders.<name>.cwd at config load (grep across every validate* method: 0 matches for .cwd()) — a relative cwd is silently accepted. Not adding validation here per instruction; that is a separate ticket.	2026-09-12 05:41:40 +07:00
Dai Ha	042b8c99dd	fleetd #480 follow-up correction: cover Fleetd.leadRollover(...)'s own wiring behaviourally ... Add FleetdLeadRolloverWorkspaceLookupTest, calling the package-private Fleetd.leadRollover(...) factory directly (with a real ConfigRef built from a temp fleetd.yaml, never the gitignored live one) to prove the terminal -> lead-name -> Leader.cwd() lookup it builds actually works: a relative handoverPath resolves against the calling lead's configured cwd; a terminal absent from the live lead-terminal map falls back to user.dir; and the lookup is read live, not snapshotted at construction time (a lead discovered by the tab scan after leadRollover(...) was built still resolves correctly). Proved this closes the gap: mutating the factory's lambda body (String leadName = null;, always "no lead found", which forces the daemon-cwd fallback this ticket exists to fix) left the full 1669-test suite green before this commit. With the new test added, the same one-line mutation now fails 2 of its 3 cases; reverting it goes green again (3/3). Mutation applied/reverted only during verification and is not part of this commit (git diff on Fleetd.java is empty). FleetdLeadRolloverWiringTest's class javadoc corrected: it previously claimed no behavioural test could catch this wiring dropping out, which was true only before this commit and only covered the factory's own body, not its call site. Restated what each test class actually covers: the source- text pin covers the call site's argument list; the new behavioural test covers the lambda's body.	2026-09-12 05:32:25 +07:00
Dai Ha	4bfab6b718	fleetd #480 follow-up: resolve a relative leadRollover.handoverPath against the calling lead's workspace ... LeadRollover.open() now resolves handoverPath to an absolute path exactly once, against the calling lead's fleet.leaders.<name>.cwd (falling back to the daemon's own user.dir when that lead has none configured), matching the LeadLauncher#launch precedent. PendingRollover stores only the resolved absolute path, so checkHandover's exists/empty/fresh checks, the path handed back to the lead in the fleet_handover open response, and the default bootstrapText sentence all see the same absolute location instead of a value resolved against whatever directory the daemon process happened to start in. FleetConfig.LeadRollover.bootstrapText is no longer defaulted in the compact constructor (it would otherwise still bake in the raw, possibly-relative handoverPath); a new bootstrapTextFor (resolvedHandoverPath) method builds the default sentence from the resolved path instead. Fleetd.leadRollover(...) gains a required liveLeadTerminals parameter to build the terminal to lead-name to Leader.cwd lookup, read live through the existing `leads` supplier and ConfigRef on every call, never off a startup snapshot.	2026-09-12 05:19:49 +07:00
ltms	9494a6b99a	Merge #485: fleetd #480 Unit C — the fleet_handover MCP tool ... fleet_handover{action: "open"|"confirm"|"cancel"} drives LeadRollover, which #483 and #484 landed with nothing calling it. Primary-only via a new Authz.Action.HANDOVER, on the same case line as SPAWN/STOP/DRAIN. The tool has NO terminal, session or leadTerminal parameter of any kind — the pane is always callerTerminal(exchange), resolved from the connection. A lead can therefore only ever roll itself, never another lead. That is charter invariant 3, and it is the second of the two corrections recorded in LeadRollover's class javadoc. Registered unconditionally, so the tool surface does not vary with config: with leadRollover: absent, every action returns a clean NOT_CONFIGURED refusal instead of failing, and open()'s IllegalStateException (config removed by a hot reload after construction) is caught and turned into the same refusal. A config-dependent tool set would have collided with #474's charter tool-surface gate and McpContractDocTest. Correction round applied before merge, and it is the reason this took two passes. The unit first shipped with a defaulted 15-argument FleetMcp constructor delegating to the new 16-argument one with leadRollover = null. I mutated the wiring rather than reasoning about it: deleting just the leadRollover argument from Fleetd.main's FleetMcp call compiled with 0 errors and passed all 1659 tests, BUILD SUCCESS — while the live daemon would have answered NOT_CONFIGURED to every fleet_handover call for ever. Neither FleetMcpHandoverTest (it builds its own FleetMcp) nor FleetdLeadRolloverWiringTest (it pins that LeadRollover is constructed, not that it is passed on) could see it. The worker then found the defect was wider than I had named: all five shorter constructors (11/12/13/14/15-arg) formed one defaulting chain into the 16-arg one, each silently supplying another feature's "off" value — leadChannel, outage, leadSeats, peers, and finally leadRollover. All five are deleted. FleetMcp now has exactly one public constructor, so every one of those features is compile-enforced at its call site, not just this one. Verified by the lead before merge, on PR head merged with current main (0176378): - CI run 1728 green on eb05576. - The identical mutation re-run by me: control against the original -> 1; mutant present (MUT-DROP-ARG2) -> 1; original gone -> 0; `mvn -o -q compile` now FAILS — "constructor FleetMcp ... cannot be applied to given types; reason: actual and formal argument lists differ in length" at Fleetd.java:[698,24]. The antidote holds: required parameter = compile error, defaulted overload = silent survivor a green suite vouches for. - Restored clean (0 changed files), then mvn clean install on the merged tree: BUILD SUCCESS 1, BUILD FAILURE 0, Tests run: 1662, Failures: 0, Errors: 0, Skipped: 0. - Three call sites of `new FleetMcp(` measured, all accounted for: Fleetd.java, FleetMcpAuthzTest.java, FleetMcpHandoverTest.java.	2026-09-11 02:27:56 +02:00
Dai Ha	eb0557621e	fleetd #480 correction round: collapse FleetMcp to one required constructor ... FleetMcp had a defaulted 15-argument constructor that delegated to the new 16-argument one with an implicit null for leadRollover. Dropping the leadRollover argument from Fleetd.main's FleetMcp(...) call fell back to that shorter overload, compiled fine, and left all 1659 tests green — the live daemon would then answer NOT_CONFIGURED to fleet_handover forever with nothing going red. Delete every overload that could reach the 16-arg constructor with a silently-defaulted leadRollover (11/12/13/14/15-arg forms all chained to it), leaving the 16-arg constructor as FleetMcp's sole public constructor. Update FleetMcpAuthzTest's call site to pass every parameter explicitly (leadChannel null, OutageSource.none(), LeadSeatSource.none(), List.of(), leadRollover null) — Fleetd.java and FleetMcpHandoverTest already called the full form. Proved with mvn -o -q compile: removing the leadRollover argument from Fleetd.main now fails to compile instead of silently defaulting. No behaviour changes — NOT_CONFIGURED refusals are unchanged.	2026-09-11 07:25:08 +07:00
Dai Ha	e2a91e883e	fleetd #480 Unit E correction: retire "injectable" wording from the log lines ... Both waitUntilAtTurnBoundary guard messages still said "never went idle" / "did not become injectable" — the old mental model the rename was meant to retire. Made both say what the code now actually waits for: a turn boundary (IDLE or DONE).	2026-09-11 07:18:51 +07:00
Dai Ha	62646957ea	fleetd #480 Unit C: wire fleet_handover MCP tool onto LeadRollover ... Adds the fleet_handover tool (open/confirm/cancel) as a thin adapter over LeadRollover, registered unconditionally so the charter tool-surface gate sees a stable set regardless of whether leadRollover: is configured. With a null LeadRollover every action degrades to a clean NOT_CONFIGURED refusal instead of throwing. Gated on a new Authz.Action.HANDOVER (primary-only, same as SPAWN/STOP/DRAIN). The caller's own connection-resolved terminal is the only lead identity ever used — the tool's input schema carries no terminal/session/leadTerminal parameter, so a lead can only ever roll itself. Fleetd.main now passes its existing leadRollover local into FleetMcp via a new trailing constructor parameter.	2026-09-11 07:07:24 +07:00
Dai Ha	802c0ab701	fleetd #480 Unit E: BLOCKED is not a settled turn boundary ... LeadRollover's waitUntilInjectable used AgentStatus#injectable(), which accepts BLOCKED. A BLOCKED pane is paused mid-turn on a prompt, not settled — reusing injectable() let /clear (or the bootstrap text after it) fire into an open approval prompt within the 20s settle window, destroying the lead's live context. Renamed the helper to waitUntilAtTurnBoundary and restricted both waits to IDLE or DONE only, with a comment explaining why this class does not reuse injectable() (it answers "may I deliver", not "has the turn ended"). Added tests for BLOCKED-forever on both waits (zero sends / exactly one send) and for DONE still completing the full roll.	2026-09-11 07:01:28 +07:00
Dai Ha	a94262271b	fleetd #480 correction round: defer the roll, and gate it on caller identity ... Two defects found after the fact, both from the original brief, both fixed here. 1. confirm() is called FROM the calling lead's own turn, so its pane is still WORKING and can never report injectable inside that same call. The old confirm() sent /clear before polling for that — the poll always timed out, but only after /clear had already fired and queued, destroying the lead's context with no fresh session ever started and a refusal return that lied about what had happened. Fix: confirm() now only validates and, if every gate passes, hands a one-shot continuation to a new continuationRunner (a real virtual thread in production, Runnable::run in tests) and returns RollDecision.approved() immediately - "scheduled", not "rolled". The continuation itself does the actual work, once the calling turn has ended: wait for the SAME pane to report injectable again (new turnSettleSeconds config key, default 20) - if this never happens, /clear is NEVER sent, at all - then /clear, then wait again (clearSettleSeconds, as before), then bootstrapText. The "no timer/scheduler, only confirm() can roll" invariant is restated precisely in LeadRollover's class javadoc: it is about initiative, not synchronicity - a single-shot continuation of an already-approved confirm() call still satisfies it; a recurring background loop would not. 2. confirm() resolved the pane to clear via PrimaryRegistry.primaryTerminal(), a single-slot lookup that is correct for a background loop with no caller but wrong here: on a daemon with more than one labelled lead tab, lead X's confirm() could clear lead Y's pane, violating the charter's "identity comes from the connection, never an argument" invariant. Fix: open() and confirm() now take the caller's terminal id as a parameter (resolved by the MCP layer from the connection - the later MCP-tool unit must pass it in, never accept it as a request field). confirm() refuses with a new NOT_YOUR_ROLLOVER reason unless it matches the terminal open() recorded. LeadRollover no longer depends on PrimaryRegistry at all. Also: renamed RollResult to RollDecision (rolled -> accepted) to reflect the new meaning - approved and scheduled, not necessarily cleared yet. Added turnSettleSeconds to the leadRollover: config block (documented in fleetd.example.yaml alongside the existing keys) and updated Fleetd.java's leadRollover(...) factory to drop the primaryRegistry parameter, with FleetdLeadRolloverWiringTest's source-text pin updated to match. New tests: turnThatNeverSettlesSendsNoClearAtAll (the branch that matters most - a turn that never ends means /clear is never sent) and aDifferentLeadTerminalCannotConfirmAnotherLeadsRollover (NOT_YOUR_ROLLOVER), plus a settle-after-clear timeout test and an open() input-validation test. LeadRolloverTest: 11 -> 14 tests.	2026-09-11 06:44:58 +07:00
Dai Ha	5c12865c25	fleetd #480 Unit A: lead rollover core (config block + executor) ... Adds the opt-in leadRollover: config block and LeadRollover, the executor a later unit's MCP tool will call. A lead writes a handover file, then open() records a token and confirm() verifies it (exists, non-empty, fresh) and an operator confirmation before clearing the lead's own pane via /clear (sent directly through AgentControl, bypassing Injector, same as ClaudeCodeLauncher#clearContext) and bootstrapping a fresh session. Nothing but an explicit confirm() call can ever roll a pane - no timer, no heartbeat, no background thread anywhere in this class. Wired into Fleetd.java exactly like LeadHeartbeatLoop: constructed only when leadRollover: is present at startup, and nothing calls it yet - the MCP tool is a separate, later unit. Classified leadRollover: as HOT in ConfigRef (joins placement/ memberCredentials/memberLoginShell/models): the executor holds Supplier<FleetConfig.LeadRollover> and reads every field fresh per call, unlike LeadHeartbeatLoop's frozen final fields. The one caveat: the object's construction is still gated on presence in the startup config snapshot, so a freshly-added block needs a restart before anything exists to call. Tests: LeadRolloverTest (14 cases covering the 6 hard requirements - no object without the config block, only confirm() can roll, missing/empty/ stale handover file each refuse by name, requireOperatorConfirm gating, and the injected wall-clock supplier) and FleetdLeadRolloverWiringTest (source- text pin on Fleetd.main's construction call, mirroring FleetdCompletionResolverWiringTest). Also updated the existing FleetConfigValidateAllTest, FleetConfigWithDefaultsPreservesEveryComponentTest, ConfigRefTopLevelCoverageTest and ConfigRefTopLevelReportingCoverageTest to account for the new record component.	2026-09-11 06:33:01 +07:00
Dai Ha	72d6a6878b	fleetd #474 follow-up: pin main's config wiring against M2 ... Fleetd.main's own choice of the three-argument ConfigRef constructor (with Fleetd::assertChartersNameOnlyRegisteredTools as extraValidation) was unpinned. Reverting Fleetd.java:154 to the plain two-argument constructor compiled with 0 errors and left the whole suite green, because ConfigRefTest and FleetdConfigRefCharterToolSurfaceWiringTest each build their own ConfigRef directly rather than through main. Adds FleetdConfigRefWiringTest, a source-text check on Fleetd.java following the FleetdBackendQuarantineWiringTest/FleetdLeadSeatWiringTest/ FleetdCompletionResolverWiringTest precedent: asserts the exact three-argument construction is present, asserts the plain two-argument form is absent, and guards against a vacuous pass on a broken/empty source read by first asserting an unrelated anchor is present.	2026-09-10 20:48:05 +07:00
Dai Ha	4466ee0ef2	fleetd #474: ConfigRef.reload() runs the charter tool-surface gate too ... A charter naming an MCP tool the server does not register refused Fleetd.main at startup but slipped through ConfigRef.reload(), because reload() only ran FleetConfig.validateAll(), which never looks at what a charter's text names. CharterToolSurface stays in the mcp package (config must not depend on it), so ConfigRef now accepts the check as a Consumer<FleetConfig> extraValidation, run inside reload()'s same try/catch as validateAll(). Fleetd.main wires a new package-private adapter, Fleetd.assertChartersNameOnlyRegisteredTools, into both the startup call site and ConfigRef's constructor, so the two call sites can never check different things. Tests: ConfigRefTest (reload refuses/accepts, via a locally-built equivalent consumer since Fleetd's method is package-private to dev.ltms.fleet) and the new FleetdConfigRefCharterToolSurfaceWiringTest (same proof through the exact Fleetd::assertChartersNameOnlyRegisteredTools reference production uses). Verified deleting the new extraValidation.accept(fresh) call site fails both new "refuses" tests by name. (cherry picked from commit 97e4c1d658) Lead review. Cherry-picked, not merged: the worker branched from 435e022, which is not an ancestor of main (I had reset and rewritten that commit's message while the worker was already on it). Merging its branch would have added a second merge commit for work already on main. Same content, no duplicated history. Rule learned: once a worker is spawned, its base commit is published. My own mutation battery, five cells, each a full `mvn -B clean test` on this commit's own tree: CONTROL 1, unmutated 1633 tests, 0 failures, BUILD SUCCESS M1 delete accept(fresh) KILLED 2 by name M3 3-arg ctor stores no-op KILLED 2 by name M4 set(fresh) before valid KILLED 6 M5 accept outside try KILLED 2 errors M2 main uses the 2-arg ctor SURVIVED M2 is a real gap and it is a test gap, not a defect: the production code here is correct. Reverting Fleetd.java:154 to `new ConfigRef(configPath, cfg)` turns the live reload gate off and leaves all 1633 tests green, because both new tests build their own ConfigRef with the method reference rather than reading what main wires. That is the fourth Fleetd.main call site to survive a battery (#446 M5 and M7, #466 M1, now this). Extracting a check into a well-tested helper moves the untested surface UP, into the line that chooses to call it. The repo already has the answer in three source-text wiring tests (FleetdBackendQuarantineWiringTest, FleetdLeadSeatWiringTest, FleetdCompletionResolverWiringTest); the worker followed the other sibling pattern, which builds the wiring itself and so cannot pin main. Delegated as a follow-up, with the surviving mutation as its acceptance criterion. Also mine to correct: my M3 cell's annotation said the no-op store "must be 2" and measured 1. The 2-arg constructor delegates rather than assigning, so my pattern only ever matched the mutant. The cell still stands on its other proof (requireNonNull left = 0). Third battery in a row with a wrong CONTROL annotation of my own.	2026-09-10 20:40:49 +07:00
Dai Ha	25ba7f16bb	Merge #473: fleet_profiles and fleet_list report which attempt a quarantine is on (fleetd #466 item 2) ... The escalating cooldown landed in 789b6a8, but the only thing an operator could see was quarantinedForSeconds. A long number does not say whether this is the first exhaustion or the fifth, and after escalation those look the same from outside: 3600 seconds could be a big base cooldown or a credential on its fourth strike. Both reports now carry quarantineAttempt beside quarantinedForSeconds. The part that matters is HOW, not that the field exists. BackendQuarantine#status(credentialId) does ONE quarantines.get() and returns both values from the same QuarantineState. It is not two accessors that a caller pairs up. That is the pattern CompositePeerLauncher.modelGateState() already documents: a report that reads a different source from the behaviour it describes will eventually disagree with it, and the disagreement is invisible because both halves look right on their own. This is reporting only. Escalation, the ceiling and the quiet-gap reset are untouched. The flat two-argument constructor still reports a real growing attempt count even though its cooldown stays flat - which is the honest answer, since the streak is real whether or not the cooldown uses it. The worker's report was lost: its fleet_reply never arrived and its inbox was empty. Nothing was lost with it, because the brief required pushing first and putting the report in the PR body. That is the second time that practice has saved a turn. Numbers and my own mutation battery are on the ticket, measured on this merge commit.	2026-09-10 20:21:14 +07:00
Dai Ha	5ba69c9cf2	fleetd #393: correct a comment that claimed two tests pin a call they cannot ... The comment above the charter writer said flipping that call back to putArray was "proven load-bearing" and pointed at two named tests. I mutated exactly that, on the merge commit, and it SURVIVED at 1618 green - so neither named test covers it, and neither can. The reason is structural, not a missing test: that writer runs first against an empty array, so putArray has nothing to replace and the two idioms are equivalent there. No test can distinguish them. The worker measured the same thing independently and said so in their report; the comment was left over from the pre-fix state, where the mutation being described was a DIFFERENT one (a later writer destroying the charter). A comment that names tests which do not cover the line is worse than no comment. The next person mutates the line, sees green, and concludes the tests are broken. The corrected version states what each cell actually measures: this line is unpinnable and why, the later writers ARE pinned and by which test names, and deleting this line entirely fails the charter-reaches-the-member assertion - the hole that predates #393.	2026-09-10 20:17:21 +07:00
Dai Ha	17052bb515	Merge #471: seeded skills reach an opencode member, and instructions[] stops depending on write order (fleetd #393) ... memberSkills: copied skill folders into every provisioned worktree's .claude/skills/ and stopped there. Claude Code reads that directory natively; opencode never does. So the feature was INERT for opencode members rather than broken: the copy succeeded, the files were correct, and nothing ever read them. No test failed because there was nothing to fail - the feature worked at the only layer it implemented. An opencode member's only channel for static guidance is the instructions[] array in its generated config. OpenCodeLauncher now adds each seeded skill's SKILL.md there. A folder with no SKILL.md is never delivered and the log names it. The second half is an ordering hazard fleet01 found by reading, and that I then measured. Three writers append to instructions[]: the role charter, the seeded skills, and the IDE rules. The charter used putArray (CREATE-OR-REPLACE) while the other two used withArray (get-or-create). That was safe only because the charter ran first against an empty array - an undeclared constraint that nothing tested. Measured on the earlier merge: making the skills writer use putArray left 1603 tests green while silently deleting the charter entry, so an opencode member would launch with no role contract at all. Worse than the bug being fixed, and invisible. All three writers now use withArray, and three tests pin the array's CONTENTS (never its size - a size assertion passes when putArray swaps two entries for two others) across the combinations that matter: charter-only, charter+ide, charter+skills+ide. WHY NOTHING CAUGHT IT, MEASURED RATHER THAN ASSUMED. fleet01 first said the missing axis was the COMBINATION of writers, then revised that to a stronger claim: that nothing asserted the charter reaches an opencode member at all. I checked the second claim on this merge and it is FALSE. Deleting the charter writer outright fails 6 tests, and 3 of those existed before #393: writesRemoteMcpConfigAndCharterInstructionsWhenMcpUrlSet, roleCharterWithoutMcpOrCustomProviderStillWritesAConfig, and aPinnedEndpointAndTheFleetMcpCoexistInOneConfig. The charter reaching a member was already pinned. So their FIRST diagnosis was the right one. The surviving mutation did not delete the charter write; it made a LATER writer replace the whole array. Every pre-existing test had exactly one writer active, and with one writer putArray and withArray are indistinguishable. The gap was never "is the charter delivered" - it was "are two writers ever active at once", which is the combination axis. Recording this because the stronger claim is the more quotable one and it would have sent the next reader looking for a hole that is not there. One honest residue: the charter writer's own idiom cannot be pinned. Flipping it back to putArray leaves the suite green, and always will, because it runs first against an empty array where the two idioms are equivalent. The edit removes an undeclared constraint for the next person to add a writer; it is not a change any test can detect. The comment in the source claimed two named tests cover it - that was wrong, and the commit after this one corrects it rather than leaving a false claim beside the code. What this does NOT do: opencode has no equivalent of Claude Code's Skill tool, so the content is static system-prompt text present from spawn, not something a member can invoke by name. This closes the DELIVERY gap and cannot close the ACTIVATION gap. That residue is opencode's design. Numbers and my own mutation battery are on the ticket, measured on this merge commit.	2026-09-10 20:17:21 +07:00
Dai Ha	e95ed99bf7	fleetd #466 scope item 2: report the quarantine repeat count, not only the seconds ... Add BackendQuarantine#status(credentialId) -> Optional<Status>, a single QuarantineState read that answers both remainingSeconds and repeatCount together -- the same "one accessor" pattern CompositePeerLauncher. modelGateState() already uses, so the two facts can never disagree. fleet_profiles/REST GET /profiles and fleet_list's capacity rows (FleetMcp.profilesView/capacityView) now call status() instead of remainingSeconds() and add a "quarantineAttempt" field beside "quarantinedForSeconds": 1 for a first occurrence, 2 for the second in a row, and so on. No change to the escalation, ceiling, or reset logic itself -- this unit is reporting only.	2026-09-10 20:09:13 +07:00
Dai Ha	1477e4358a	Merge #472: one canonical tool-name set, and charters are checked against it (fleetd #469) ... A role charter is free text in config that tells a member which tools to call, and nothing checked that those tools exist. A charter naming bridge_send - a name CB-634 removed - started the daemon cleanly, and the member found out at run time by calling something that was not there. #464 shipped a test for this, but it wrote its own charter into a @TempDir, so nothing anyone put in the real config could fail it. That was a defect in my acceptance criteria, not in that work. Now: FleetTool is one enum of the 11 registered wire names, and every reader goes through it. - FleetMcp's schema builders pass FleetTool.X.wireName() instead of a literal. - FleetMcp's constructor asserts at startup that what it registers with the SDK equals FleetTool.wireNames() exactly, in both directions. - toolAction(String, Map) resolves arbitrary wire input against FleetTool.byWireName() and keeps its run-time throw, which is necessary - network input has no closed compile-time form. It then hands off to authzAction(FleetTool, Map), a switch over the enum with NO default, so a new tool is a compile error at that layer. - CharterToolSurface lives in mcp, not config, and Fleetd.main calls it right after validateAll(). Config must not depend on the MCP server: config loads before the server exists. My brief undercounted the problem and the worker corrected it. I said there were two tool-name inventories plus a test fixture. There were FIVE: the registrations, the authz switch, and three separate source-text scrapes of FleetMcp.java in CharterToolSurfaceTest, FleetMcpAuthzTest and McpContractDocTest - none of which the ticket mentioned. Fixing those three was required, not scope creep: once the literals moved into FleetTool their regexes matched zero names, so one would have failed on its vacuity guard and the other two would have gone quietly vacuous. Inventories after: one. Verified independently on origin/main before accepting the wider diff: three test files did read FleetMcp.java as source text, with a control file at zero to prove the search discriminated. Build number and my own mutation results are on the ticket and the PR, measured on this merge commit rather than on the branch.	2026-09-10 19:59:03 +07:00
Dai Ha	9e4e423ad6	fleetd #393 follow-up: remove the instructions[] writer-ordering hazard ... OpenCodeLauncher.writeConfig has three writers into the instructions[] array (charter, seeded skills, IDE rules). The charter writer used putArray (create-or-REPLACE) instead of withArray (get-or-create), which "worked" only because it happened to run first against a still-empty array — an undeclared ordering dependency nothing tested. Found by the fleet01 lead and verified on this branch's merge: flipping the skills writer to putArray left the full 1603-test suite green while silently deleting the charter entry, which would launch an opencode member with no role contract at all. Fix: charter's putArray -> withArray (one-word change, behavior-identical today). Add three tests asserting instructions[] CONTENT as an exact ordered list (not size) across writer combinations: charter only, charter + IDE rules, and charter + IDE rules + seeded skills. Mutation testing (see PR body) shows the skills and IDE-rules writers are each independently detectable by name; the charter writer's own mutation is not detectable by any test, because it structurally always runs first against an empty array, so putArray and withArray are equivalent there.	2026-09-10 19:58:25 +07:00
Dai Ha	6c2d6e93cb	fleetd #469: one canonical FleetTool set backs registration, authz and charter checks ... FleetConfig.validateCharters() only checked that a charter key is a role wire name and its text is non-blank; #464's CharterToolSurfaceTest compared charter text against the registered tool surface, but wrote its own charter into a @TempDir fixture, so nothing anyone wrote into the live fleetd.yaml could ever fail it. Add FleetTool, an enum in dev.ltms.fleet.mcp holding the one canonical set of registered tool wire names. FleetMcp's tool schemas now derive their names from it, its constructor asserts at startup that what it actually registers with the SDK equals FleetTool.wireNames() exactly, and its authz dispatch (toolAction/authzAction) resolves the wire string against FleetTool before switching on the enum itself with no default -- adding a tool without pinning its Authz.Action is now a compile error, not just a test gap. Add CharterToolSurface (mcp package, not config -- config loads before the MCP server exists) and call it from Fleetd.main right after cfg.validateAll(), so a charter naming a tool the server does not register refuses the daemon's startup, naming both the charter key and the unknown tool. FleetdStartupValidationTest proves this through Fleetd.main itself against a live-shaped config fixture (bridge_send, CB-634's own removed name). CharterToolSurfaceTest, FleetMcpAuthzTest and McpContractDocTest each kept an independent regex scrape of FleetMcp.java's source for the registered side of their own comparison -- three more copies of the same list nothing tied together. All three now read FleetTool.wireNames() instead. Proved canonical by removal: deleting FleetTool.ACK while ackTool() still referenced it broke mvn compile in two places (FleetMcp.java:940,:1898); registering a schema under a literal not backed by FleetTool ("fleet_ack_v2") failed FleetMcp's new startup assertion in every test that constructs it (12 errors, IllegalStateException at FleetMcp.<init>). Both reverted before this commit.	2026-09-10 19:55:02 +07:00
Dai Ha	01462c9695	fleetd #466 follow-up: pin main's choice of the escalating quarantine factory ... BackendQuarantineTest proves BackendQuarantine.withEscalation itself escalates, is ceilinged, and resets. Nothing proved Fleetd.main still calls it: reverting main to the flat two-argument constructor compiled with 0 errors and left the full 1608-test suite green, because every other test builds its own BackendQuarantine directly. Adds FleetdBackendQuarantineWiringTest, a source-text assertion on Fleetd.java (same idiom as FleetdLeadSeatWiringTest and FleetdCompletionResolverWiringTest) pinning that main's declaration is built from BackendQuarantine.withEscalation(...), not `new BackendQuarantine(...)`. Measured: reverting to the flat constructor fails it (expected true, was false); renaming the anchored local variable while keeping behaviour identical also fails it loudly, not silently. The class doc states plainly this checks source text only, not that the call executes or that escalation reaches a real backend.	2026-09-10 19:47:59 +07:00
Dai Ha	d4a2cd720c	fleetd #393: deliver memberSkills to opencode members, and stop overclaiming seeding success ... GitWorktrees.seedSkills copies memberSkills:-seeded skill folders into every provisioned worktree's .claude/skills/ and logged "skill seeding: N of M" as if that were success — but .claude/skills/ is a Claude Code CLI convention. opencode has no such discovery, so a kind: opencode member never actually read a seeded skill even though the log said N of M succeeded. Two changes, both required: 1. Deliver it. OpenCodeLauncher.skillInstructionFiles scans <cwd>/.claude/skills/*/SKILL.md at spawn time (the one point the launcher knows both the kind and the cwd) and appends each to the generated opencode.json's instructions[] array, the same channel already used for the member charter and IDE rules. A skill folder with no SKILL.md is named and skipped rather than silently dropped. 2. Stop claiming it where the claim can't be verified. GitWorktrees.seedSkills' log now says explicitly that consumption depends on the member's kind and points at the launcher's own log; OpenCodeLauncher logs its own kind-aware "skill delivery: M of N ..." line once the kind is actually known, naming any folder it could not turn into an instructions[] entry. fleetd.example.yaml's memberSkills: doc previously claimed "Claude Code members only; an opencode member reads a different path (.opencode/agent) this key does not touch" — false as of this fix, corrected to name both kinds and how each consumes it. Tests: OpenCodeLauncherTest gains two cases driving the real GitWorktrees#add seeding path (not a hand-built fixture) into an opencode-kind spawn — one asserting a seeded skill's SKILL.md lands in instructions[] plus the honest log line, one covering a skill folder without SKILL.md (delivered skills still flow, the malformed one is named in the log and excluded from instructions[]). ClaudeCodeLauncher is untouched — its native .claude/skills/ discovery already worked and is out of scope. mvn -B clean test: Tests run: 1603, Failures: 0, Errors: 0, Skipped: 0 — BUILD SUCCESS	2026-09-10 19:37:00 +07:00