3762aca3075413fb9fdb1d3b0d64dae1553fa9b3
1007 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 ltms
|
3762aca307 |
Merge #606: wire the lead context gauge to the real configDir, and stop flapping on a torn line
Fixes the two defects recorded in #602's own body. 1. FleetMcp.contextView passed configDir=null, so the gauge read <user.home>/ .claude while this host's lead profile sets an override. Measured before the fix: 19 transcripts under the real directory, 0 under the fallback. The gauge would have deployed green and reported UNKNOWN forever, for every lead. Now threaded via FleetMcp.LeadConfigDirSource, built by Fleetd .leadConfigDirSource, following fleet.leaders.<name>.profile to that profile's configDir and reading config.get() live inside the lambda. 2. A torn final line no longer means UNKNOWN. fleet_list reads a transcript Claude Code may be mid-write on, so the last line can be cut. The old code treated that as fatal, which would make the gauge flap at random. The stated reason ("a format change should show as UNKNOWN") does not hold: a real format change makes EVERY line unparseable, and that case is still caught. Verified by the lead on the combined state with current main merged in, not on the branch alone: mvn -o clean install exit 0, 147 reports, 1841 tests, 0 failures, 0 errors, 0 skipped. Mutation-checked independently by the lead: Fleetd.leadConfigDirSource body -> none() -> KILLED (1 failure) FleetMcp.contextView configDir -> null -> KILLED (worker-measured) KNOWN RESIDUAL, documented rather than overclaimed. main's own one-line call to leadConfigDirSource could be swapped for none() and the suite stays green. Every member of this wiring-test family (loopHealthSource, capacitySource, healthCoverageSource) has the identical gap — no test runs Fleetd.main far enough to observe which factory it called. Filed separately as a class-wide problem rather than patched here. The empirical close is the dogfood check after redeploy. |
||
 Dai Ha
|
4e27bde2d7 |
fleetd #602 gauge-wiring follow-up: pin Fleetd.main's LeadConfigDirSource wiring
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. |
||
|
Dai Ha
|
d345b14e43 |
fleetd #602 gauge-wiring: thread a lead's configured configDir into the context gauge
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. |
||
|
Dai Ha
|
3ed7bfca67 |
fleetd: report a lead's live context usage in fleet_list
fleetd had no way to see how full a lead's context window is. On this host a lead auto-compacted 30 times in one session, discarding roughly 250,000 tokens and costing 46s to 3m16s each time, and nothing could see it coming. LeadContextGauge reads the transcript Claude Code itself writes, never the lead's pane. It finds <sessionId>.jsonl by NAME under <configDir>/projects/ rather than deriving the project slug, which is an undocumented internal. Three states, not two: OK, HIGH, UNKNOWN. Every path that cannot positively establish a token count reports UNKNOWN with no number, so a lead is never told it is fine when the honest answer is "I could not look". Bounded two ways: TAIL_BYTES caps bytes read off disk, and a 5s cache TTL caps how often that read happens, because fleet_list is polled constantly. Recovered by the lead: the authoring member ended on a backend error (DNS ENOTFOUND) with this work uncommitted and unpushed in its worktree. Verified before committing: mvn -o clean install exit 0, 1813 tests, 0 failures, 0 errors, 0 skipped, 144 reports; LeadContextGaugeTest 8/8. KNOWN INCOMPLETE - see the PR. The fleet_list call site passes configDir=null, which falls back to ~/.claude, but this host's lead profile sets configDir to an override. Measured: 19 transcripts under the real configDir, 0 under the fallback. The gauge is therefore INERT on this fleet until that is wired. |
||
|
ltms
|
6eb34a654f |
Merge #599: fleetd #589 groups 1+2 — wiring-test 6 sites in Fleetd.main()
Extracts 6 inline constructions in Fleetd.main() (:214-:468) to package-private factories, each pinned by a behavioural wiring test. Production behaviour unchanged. Gate: test-merged onto current main (already carrying #598, which rewrote 117 lines of the same file). No conflict — the two units insert at different anchors, #599 after capacitySource and #598 after loopHealthSource, as their briefs specified. mvn exit=0, 1805 tests / 0 failures / 0 errors from 143 surefire reports; all 6 new classes confirmed to have run with their expected counts. Diff confirmed extraction-only. No source-text assertions in any of the 6 new files; that zero carries a positive control (the same pattern finds 18 such files elsewhere in the repo). Worker self-reported fixing a mutation that threw NullPointerException rather than failing an assertion — the assertNotNull guard is present ahead of the matcher call, confirmed. |
||
|
ltms
|
7084d99b89 |
Merge #597: fleetd #593 — running_pid() counts only the daemon
running_pid() filters pgrep -f hits by `comm = java` (allowlist) instead of denying a fixed list of shell names. Closes two false-positive holes: an exited pid (empty comm matched no denied name) and any non-shell wrapper (ssh, perl, python3, ruby) carrying the pattern in its own argv. Gate: merged onto current main, bash suite exit 0 and structurally identical to the baseline run on main (the one "Unattributable mutation" line is pre-existing, confirmed by running the suite on origin/main). All four running_pid tests confirmed defined AND invoked. Mutation check run by me: neutering the allowlist makes the suite exit 1 with a named failure; restore is byte-identical to baseline by git hash-object and green again. Also checked and cleared: the new die-message advice `ps -eo pid,comm,args` does NOT expose process environments on macOS — `-e` with `-o` selects all processes, it does not imply `-E`. Verified with an isolated two-phase probe and a positive control, after three earlier probes gave false positives by self-matching (the grep's own argv, and the probe script's own text). |
||
|
Dai Ha
|
ae7845c375 |
fleetd #589 (Groups 1 & 2): pin 6 main() wiring sites with named factories
Extracts 6 inline wiring expressions from Fleetd.main() into named, directly-testable package-private static factories, following the FleetdLoopHealthSourceWiringTest (#584) shape, and adds one wiring test per factory: Group 1 (exhaustion/quarantine): - forwardingExhaustionSink(exhaustionSinkRef) — was inline ExhaustionSink.forwardingTo(exhaustionSinkRef::get) - publishExhaustionSink(...) — was two untested statements building the real sink and .set()-ing it into exhaustionSinkRef - liveExhaustedPatterns(config) — was inline new LiveExhaustedPatterns(() -> config.get().profiles()) - exhaustedPatternLookup(roster, liveExhaustedPatterns) — was an inline lambda resolving a herdr target to its profile's live pattern; silently losing this is the worst regression in the sweep, since a real usage-limit refusal would stop being classified as BACKEND_EXHAUSTED Group 2 (CB-596 credential policy): - claudeCodeLauncher(...) — was an inline `new ClaudeCodeLauncher(...)` whose memberCredentials supplier argument was untestable wiring - openCodeLauncher(...) — same, for OpenCodeLauncher Each new test pins its factory behaviorally (never via source-text assertions): built and confirmed RED by name against the named inert mutation, then confirmed GREEN again after restoring, and separately confirmed GREEN after a behavior-preserving reformat/local-variable extraction of the same call, to rule out a disguised source-text test. Suite: 1789 -> 1799 tests (+10, matching the 10 tests added), 0 failures, mvn -o clean install BUILD SUCCESS. Scope strictly limited to main()'s :214-:468 range per the ticket split with the concurrent worker handling Group 3 at line 500+. |
||
|
ltms
|
61115f6f61 |
Merge #598: fleetd #589 group 3 — wiring-test 5 sites in Fleetd.main()
Extracts 5 inline lambdas/method-refs in Fleetd.main() to package-private factories and pins each with a wiring test. Production behaviour unchanged; the releaseCleanup body moved verbatim. Gate: test-merged onto current main in a scratch worktree, mvn exit=0, 1795 tests / 0 failures / 0 errors from 137 surefire reports. Diff read in full. Worker's self-disclosed bare `git stash push` verified as recovered — all 3 surviving stash entries predate today, so no other worktree lost work. |
||
|
Dai Ha
|
4b9ebda1b3 |
fleetd #593 CORRECTION 1: allowlist comm=java, not a denylist of shells
The round-1 fix excluded known shell names (sh/bash/zsh/dash/ksh) from running_pid()'s pgrep candidates. Two holes remained, both the same false-positive shape the ticket exists to remove: 1. A pid pgrep lists can exit before the following `ps -o comm=` lookup runs. On a gone pid, ps prints nothing, comm is empty, and an empty string matches no denied shell name -- so a dead pid was still counted. 2. The denylist only knows the shells someone thought to name. ssh, perl, python3, ruby, tail -- anything else carrying the pattern in its own argv -- was still counted alongside the real daemon. The ticket names ssh as a live route. Both close with one change: allowlist comm=java instead of denying shells. The daemon is always `java -jar target/fleetd.jar`, so its comm is always `java`; an empty comm (hole 1) is not `java` either, closing that hole for free. Answers the objection in the code comment: an allowlist can under-count if fleetd ever stops being launched by `java` (a native image, a renamed launcher). That's a false negative, the worse direction for a guard -- but it is not a new assumption: PATTERN='target/fleetd.jar' already assumes a jar run by java, and that pattern breaks before this allowlist would. Replaces the round-1 "real second process" test (which gave its exec -a standin an argv[0] holding the pattern, but not comm=java) with one that forces comm=java via `exec -a java sh -c '...'`. Adds two stubbed pgrep/ps tests pinning the two holes directly (a non-java, non-shell comm such as perl; an empty comm from an already-exited pid) -- deterministic on every platform, unlike a live-process fixture, and immune to the BSD vs Linux difference in how `comm` is derived from a fabricated process. Adds a stubbed positive backstop (comm=java is counted). Confirmed the regression is caught: reverted to the round-1 denylist, reran the suite, watched the new non-shell-comm test fail at `set -e`'s first failure, then isolated the exited-pid test separately and confirmed it also fails against the same broken code. Restored the fix and reran green. Branch merged with origin/main (3 commits: hunter role + CLAUDE.md addendum) before this commit; unrelated, no conflicts. |
||
|
Dai Ha
|
1e68d7ee39 | Merge origin/main into worker/593-1a8025-5 | ||
|
Dai Ha
|
6cccd458d4 |
#589 Group 3: wiring-test the 5 sites below line 500 in Fleetd.main()
Extracts the inline lambdas/method references at the 5 assigned wiring sites into named package-private factories on Fleetd, following the FleetdLoopHealthSourceWiringTest pattern from #584: - turnRegistrar(CompletionResolver) — was completion::register (Injector) - healthFailTarget(MessageService) — was messages::abandon (FleetHealthMonitor) - releaseCleanup(MessageService, ReplyInbox, PrimaryRegistry) — was the inline sessions.onRelease(detail -> {...}) cleanup lambda - replyInboxOpener() — was AmqpReplyInbox::open passed to selectReplyInbox - leadMailboxOpener() — was LeadMailbox::open passed to openLeadMailbox Each factory has a new runtime test (not source-text) that drives real collaborators through public APIs: MessageService.poll(ticket).phase(), InMemoryReplyInbox.peek(), PrimaryRegistry.nudgeTargetFor(), and the opener tests connect to a guaranteed-closed local port to prove a real network attempt vs. an inert stub. releaseCleanup was done first per the brief: MessageService.abandon's javadoc documents that losing this cleanup leaves a torn-down worker's rendezvous waiter open forever. Tests: 1789 -> 1794 (+5), 0 failures, 0 errors. mvn -q -o test exit 0, no BUILD FAILURE, no piped exit status. Each new test verified RED on the inert form named in the ticket, and GREEN after reformatting the call across lines and extracting the argument into a local/factory. |
||
|
Dai Ha
|
42820fbe75 |
fleetd #593 (pid-count half): running_pid() no longer matches the caller
running_pid() was a bare `pgrep -f "$PATTERN"`, which matches ANY process whose full command line contains the pattern text -- including a shell that merely embeds it as literal text (a hand-typed investigation, an ssh-shaped `sh -c '...; ...'`, or a pipeline) rather than being the daemon. That self-match turns a working redeploy into a reported "racing supervisor" failure via assert_single_daemon. pgrep -c does not exist on BSD/macOS, so this can't be fixed by switching flags. running_pid() now keeps pgrep to find candidates (portable), then drops any candidate whose process name (comm) names a shell -- the daemon is always `java`, so a self-matching wrapper of this shape is always excluded while a genuine second daemon-shaped process still counts. assert_single_daemon's die message no longer hands the operator a bare `pgrep -f "$PATTERN"` as remediation -- that was exactly the self-matching invocation -- and now says in words that a pattern can match the caller. Adds three tests: a self-matching wrapper shell must be excluded, a real second daemon-shaped process must still be found, and the die message must not recommend the self-matching command. Verified the first test fails against the pre-fix implementation (confirmed the regression is caught). Leaves instance 1 (the fleetd.out log source, systemd-only) for a Linux host, per the ticket's scope split. |
||
 Dai Ha
|
d91ff886da |
#568 follow-up: fix the text defects the hunter-role merge introduced
Found by reading the diff at the merge gate, not reported by the worker. 1. FleetConfig.java: the operator-facing "unknown key" hint read "'fleet.architects', 'fleet.developers' or 'fleet.hunters' or 'fleet.reviewers'" — a double "or". This is text an operator reads at the moment their config is already wrong, so it should not itself be wrong. 2. MemberLifecycle.java: javadoc continuation asterisk indented 6 spaces, not 5. 3. MemberRegistry.java: javadoc asterisks moved from column 2 to column 4. 4. CallerResolver.java: a // comment indented one space past its block. 2-4 are the worker mangling alignment while widening enum lists to include HUNTER. No behaviour changes. CORRECTION to the #596 merge commit message. It claimed a fifth defect, "two javadoc lines pushed past the 100-column convention". There is no such convention in this repo: no checkstyle, no spotless, no .editorconfig, and 2975 of 32728 lines under fleetd/src/main/java already exceed 100 characters. I asserted the rule before measuring it. Those two lines are untouched. Verified: built in a scratch worktree, 1790 tests, 0 failures, 0 errors, 0 skipped, counted from the surefire XML. |
||
|
ltms
|
386e760a5c |
Merge #596: fleetd #568 — add the hunter member role
Verified by the lead before merge, not taken on the worker's report: - branch contains a639969; 1 ahead, 0 behind — clean fast-forward - CLAUDE.md change is +2 lines in the Project addendum, NOT the canonical block - canonical block sync check prints True on main and on this branch - built in a scratch worktree (never mvn clean in the main clone): 1790 tests, 0 failures, 0 errors, 0 skipped, counted from the surefire XML. Baseline 1789. - live fleetd.yaml still loads: the hunters pool is optional Five text defects found by reading the diff, not reported by the worker. They are fixed in a follow-up commit on main rather than a round trip: - FleetConfig.java operator-facing message reads "... 'fleet.developers' or 'fleet.hunters' or 'fleet.reviewers'" — a double "or" - misaligned javadoc continuation asterisks in MemberLifecycle and MemberRegistry - a misaligned // comment in CallerResolver - two javadoc lines pushed past the 100-column convention Known gap, tracked separately: fleet.hunters is absent from the live config, so a hunter cannot spawn on this host until the pool is added after the redeploy. The role ships correct and inert. |
||
|
Dai Ha
|
2e349139e9 | #568: add hunter member role | ||
|
Dai Ha
|
a639969a9a |
CLAUDE.md: a blocked lead consults architects, not the operator
The operator set this rule on 2026-09-19: when a decision blocks a lead, it consults one or more architect members, who are authorized to agree on one decision and unblock. The operator is not asked. Escalation stays open only for things outside the fleet's authority -- money, credentials, or a promise made to someone else. The paragraph also carries the reason the ticket record is mandatory rather than optional. The operator's old notification channel was the block itself: work stopped, so they found out. Taking the operator out of the loop removes that signal with it, so the decision goes on the ticket, which reaches them whether or not they are at a terminal when it is made. The rule has a second half aimed at architects, which lives in fleet.charters.architect and is applied per daemon -- filed as #591, because a charter can never reach a lead and charters do not travel between hosts. The missing notification event is #592. Block verified byte-identical with wiki 7-Use-Cases.md at 1d9bd1b. |
||
|
Dai Ha
|
17c3a69c57 |
docs: CB-591 gateway page had two claims that went stale
The gateway's chat model is served under the stable alias `acoder`, and the model behind that alias changed on 2026-08-28 — it is Qwen3.8-27B now, not DeepSeek-V4-Flash. The old name is still served, so nothing broke, but it names a model this is not. Two claims on the page were wrong as a result, and both were written as current facts rather than dated measurements: - `/v1/models` returns exactly `["deepseek-v4-flash"]` — it returns 6 ids now. This sat under a heading saying it needs no re-testing. - the status banner said `local` and `gx` are both at `weight: 100` — `local` is at 0. Measured today against the live gateway: /v1/models returns acoder, qwen3.8-27b-nvfp4, deepseek-v4-flash and three embedding names; a completion sent as `deepseek-v4-flash` comes back reporting `"model": "acoder"`, which is the alias in plain sight. /v1/deployment reports generation 2026-08-28-qwen3.8-27b-nvfp4. §2 and §3 are left alone. They are the August plan, and rewriting them would destroy the record of the migration. fleetd.yaml moved to `acoder` in the same change. It is not tracked here. |
||
|
ltms
|
49a5875586 |
Merge #583: fleetd #582 — assert pending message-id cleanup at every publish cleanup site
All ten assertions proven live: six by the implementer, the last four by the lead. One contract build with four deleted removal lines produced exactly four named failures, one per site, with the total unchanged at 1825. |
||
|
Dai Ha
|
634d33b50b | Merge worker/562-loop-health-wiring-test-99611c-5 | ||
|
Dai Ha
|
1db79bcaa9 | Merge worker/581-completionresolver-cas-sites-0542b7-6 | ||
|
Dai Ha
|
4507bc5a70 | Merge worker/571-attempted-outcome-5739f7-2 | ||
|
Dai Ha
|
d7239ed23b |
fleetd #571: pin FleetMcp.formatReply's TIMED_OUT_UNCONFIRMED wording
CORRECTION 5 on the ticket: mutating the new arm's message text to the queued/working arm's text survived every existing test, because nothing asserted the specific wording. This adds one test that asserts the unconfirmed-delivery message and asserts it does NOT carry the queued/working arm's retry invitation — the distinction #571 exists for. No production code changes; formatReply's TIMED_OUT_UNCONFIRMED arm was already correct. |
||
|
Dai Ha
|
dfeb9340b4 | fleetd #581: cover completion CAS removals | ||
|
Dai Ha
|
1513d4f260 |
fleetd #562 follow-up: extract loopHealthSource factory, pin its wiring
PR #579's inline `new FleetMcp.LoopHealthSource(poller::health, ...)` in Fleetd.main had nothing a test could call directly. Measured: replacing poller::health with a constant () -> RUNNING compiled clean and left all 1771 tests green (see issue #562 comment "HOLD on PR #579"). Extracts the inline construction to a package-private Fleetd.loopHealthSource factory, the same style as the sibling capacitySource/healthCoverageSource factories, and adds FleetdLoopHealthSourceWiringTest with three separate assertions: the statusPoller half, the sessionReaper half, and the reaper == null branch (still STOPPED). |
||
|
Dai Ha
|
4ca7d72303 | #582: assert pending message-id cleanup | ||
|
Dai Ha
|
c0545d003d |
fleetd #571: make FleetApp.writeReply's inner Outcome switch exhaustive, no default
Ticket comments (17126, 17127) corrected the original acceptance criterion after this unit was already in flight: a hand-listed grep for the enum's constant names goes stale silently the moment a new constant lands, so the compiler must be the enumeration instead. sendOutcomeLabel (MessageService.java) and formatReply (FleetMcp.java) were already default-free switch expressions. The one gap was writeReply's inner "status" switch, which had `default -> "done"` — the exact value that would have lied about TIMED_OUT_UNCONFIRMED. Remove the default and list every Outcome constant explicitly; REPLIED, COMPLETED_UNREPLIED, QUESTION and STALE_TURN get an arm too even though the outer switch always dispatches them first, so the inner switch stays exhaustive on its own. The outer switch (a statement, not an expression) keeps its own default — Java does not require exhaustiveness there regardless, and "everything not terminal is a 202" is an intentional catch-all. Verified with the proof the ticket asked for: added a scratch 11th Outcome constant after deleting all default arms and confirmed all three switch-expression sites (and no test file) fail to compile without an arm for it, one at a time, then removed the scratch constant. |
||
|
Dai Ha
|
275ac0d251 | fleetd #562: surface loop health | ||
|
Dai Ha
|
c1e06c9e12 |
fleetd #571: add TIMED_OUT_UNCONFIRMED so an ATTEMPTED delivery is not reported as never-arriving
MessageService.send's TimeoutException branch collapsed Injector.Cancellation.ATTEMPTED (fleetd #551 — the send call was made but its outcome is unknown) into Outcome.TIMED_OUT_QUEUED, which promises the caller the message will never arrive. On this route agent.prompt may already have pasted and submitted the text, so a caller's natural recovery (resend) risks a double delivery. Add Outcome.TIMED_OUT_UNCONFIRMED and route ATTEMPTED to it. Update the three readers found by searching for the enum's constant names (not `Outcome.`, which misses FleetMcp's unqualified `case REPLIED ->` switches and would false-positive on ConfigRef's unrelated Outcome record): - MessageService.sendOutcomeLabel: add it to the "timeout" metric label group. - FleetMcp.formatReply: its own case, warning against a blind retry (distinct from the generic "retry or poll status" message the other timeouts get). - FleetApp.writeReply: its own "unconfirmed" status and detail text, so it no longer falls through the switch's default -> "done" arm, which would have reported "the delegation completed" for the one case where delivery is unconfirmed. |
||
|
Dai Ha
|
204da67d66 |
Merge #576: fleetd #575 — one finally covers answer()'s STALE_TURN exit
Third instance of the #572 shape on this file: one invariant kept at N sites, asserted at fewer than N. Here answer()'s inner try opened AFTER the Task lookup/registration and the STALE_TURN early return, so that return was covered only by a hand-rolled copy of the finally's cleanup pair. The fix widens the try upward and deletes the copy, so every exit runs the one finally exactly once. rendezvous.open(workerSession) correctly stays outside it — nothing to clean if it never opened. This fixed no live leak, and the code comment says so: neither rendezvous.answerAsk nor clearAsyncQuestion(turnId, false) can throw, so nothing ever left through the old gap uncovered. It is a structure fix, the ticket's own fallback case. Verified here, not taken from the worker's report: - baseline on the branch: 1766 tests, 0 failures, from Maven and from an independent sum over target/surefire-reports/*.txt. - my own mutation, located fresh: move the inner `try {` back down below the STALE_TURN return — the exact pre-fix structure, minus the hand-rolled pair. Result: 1766 run, exactly 1 failure, and it is the new test — MessageServiceTest.answerLosingTheRaceToAnAlreadyAnsweredAskStillReturnsStaleTurnAndCleansUpOnce:545 "the forward waiter this answer() call opened must be closed after a STALE_TURN return". One failure, not a crowd: the new test is the only thing holding this path. - the worker's own mutation removed the whole finally and took 8 other tests with it. That proves the finally runs; it does not prove the STALE_TURN path reaches it. Mine does. The new test hook answerAskLapseRaceHookForTest follows the file's existing askTimeoutRaceHookForTest convention. Closes #575. |
||
|
Dai Ha
|
b091c51eee |
fleetd #575: widen answer()'s try so one finally covers its STALE_TURN exit
The waiter cleanup pair (asyncTasksByWaiter.remove + rendezvous.close) was duplicated: two sites sit in a finally, the third was hand-rolled inline before answer()'s early STALE_TURN return, structurally outside any finally. Both rendezvous.answerAsk and clearAsyncQuestion(turnId, false) are total (cannot throw), so the gap never leaked in practice. But the duplicate was untested: mutating it away left all 1765 tests green, while the two finally-protected sites are each killed by 8-36 tests. Same shape as #572. Fix: widen the try to wrap the Task registration and the STALE_TURN check, so the single finally covers every exit and the hand-rolled copy is gone. Added a race hook + regression test that deterministically reproduces the 'ask lapsed between the lookup and the unblock' case and proves the fix still returns STALE_TURN and cleans up exactly once. |
||
|
ltms
|
84d631b030 |
Merge #572: answer()'s session-lock release is pinned on all four exits
fleetd #572. MessageService releases its per-session lock in a finally at two sites. Removing the
send() one failed 22 tests and errored 1. Removing the answer() one left the whole suite green.
Skip that unlock and the thread holds the session lock forever, so every later send or answer to
that session blocks permanently — no exception, no log line.
Tests only. Against current main the diff is one file, MessageServiceTest.java, +178 lines.
Four new tests, one per exit of answer(): normal REPLIED, TIMED_OUT_WORKING, the ExecutionException
rethrow, and the InterruptedException rethrow. Each proves REACQUISITION rather than the return
value — a bounded follow-up send on the SAME session must not come back BUSY, and send reports BUSY
only when tryLock itself timed out, so a non-BUSY probe is specifically evidence the lock was free.
Lead verification, re-running rather than accepting the worker's numbers, on the branch merged with
main at
|
||
|
ltms
|
3f8c38fc54 |
Merge #567: pin LeadMailbox.inspect's probe-channel close
fleetd #567. LeadMailbox.inspect opens a probe channel and closes it in a finally. The production
code was already correct; nothing asserted it, so a future refactor could drop the close and leak an
AMQP channel per inspect() call with the suite green.
Test only. LeadMailbox.java is untouched — sha256 a2cd99be77345b7e... before and after.
The test asserts the CONSEQUENCE rather than the return value: it caps the connection at three
channels (LeadMailbox uses two, consume and publish), runs a successful inspect, then requires a
replacement channel. If the probe is left open, the broker has no channel number left and
createChannel() returns null. A test that only checked inspect()'s MailboxState would pass under the
mutation, which is the whole reason this hole existed.
Lead verification, re-running rather than accepting the worker's numbers, on the branch merged with
main at
|
||
|
Dai Ha
|
a4dbc8f8b7 |
fleetd #572: pin answer()'s session-lock release across all four exits
MessageService.answer() releases its per-session lock in an outer finally (MessageService.java:1218) that mutation testing showed was covered but unasserted: removing that line left all 1750 existing tests green, because every existing test on this path checks answer()'s return value, never that the lock it took is actually reacquirable afterward. If it leaked, a session would be wedged forever with no exception and no log line. Adds four tests, one per exit of answer() (normal REPLIED reply, TIMED_OUT_WORKING, ExecutionException rethrow, InterruptedException rethrow), each proving the lock is reacquirable via a bounded (300ms) follow-up send on the same session rather than merely checking answer()'s own outcome. No production change. |
||
|
ltms
|
ed2fd6646a |
Merge #561: the completion/session listener fan-out survives either half throwing, and both sites are pinned
fleetd #561. Fleetd composed two TurnListener halves as `completion.X(); sessions.X();`, so a throw from the first half skipped the second. The anonymous class is now a package-private factory, Fleetd.turnListener(completion, sessions), built on two helpers that always attempt both halves and rethrow whatever escaped — a second failure attached with addSuppressed rather than dropped, so it still reaches StatusPoller's catch (Throwable). The wiring at Fleetd.java:498 calls that factory, so the seam under test is the real caller. Lead verification, on a merged tree, re-running the checks rather than accepting the worker's: exit 0, 1761 tests from Maven and from an independent sum over 131 surefire reports. The interesting part is what the first round MISSED. Two helpers maintain one invariant — "the second half always runs" — and the first round's five tests asserted it at only one site. Measured: bothMustRun reverted to the pre-fix bug -> 1 named failure (pinned) bothMustRunKeepingSecondResult the SAME bug -> 1755/1755 GREEN (unpinned) failure.addSuppressed(t) deleted -> 1755/1755 GREEN (unpinned) Both survivors are now killed by new tests, re-verified by the lead after the fix: sessionHalfStillRunsWhenTheCompletionHalfThrowsSynchronouslyForPostAction and bothFailuresEscapeWhenBothHalvesThrowDistinctExceptions, each failing alone under its own mutation. The rule this cost us, worth carrying: COUNT ASSERTIONS PER SITE, NOT PER INVARIANT. The total being non-zero is what hides a zero at one site, and extracting a shared helper makes it worse rather than better — it does not reduce the number of sites, only how many are visible. Credit to the fleet01 lead, who predicted this shape before an instance was found. onDelivered stays deliberately unguarded. Its comment now gives the real reason — CompletionResolver.captureBaseline already catches RuntimeException around its scrape and fails open, so that half does not realistically throw — instead of the previous reason, which was true but about registration rather than about this pair. A correct conclusion resting on a wrong premise reads exactly like a verified one. |
||
|
Dai Ha
|
5441a2b321 | fleetd #567: assert inspect closes probe channel | ||
|
ltms
|
384867dfa3 |
Merge #551: ATTEMPTED is its own cancellation answer, and the javadoc stops claiming a timed-out send never arrived
fleetd #551. The injector polls a queued entry off the queue and marks it ATTEMPTED BEFORE the irreversible AgentControl#send call, not after. So a Throwable escaping that call can never leave the entry QUEUED at the head (the #546 re-send hazard) and can never be recorded as a confident NOT_DELIVERED for text that may already be in the pane. Cancellation.ATTEMPTED is added as a third answer. NOT_DELIVERED stays reserved for confirmed absence: the readiness grace expiring, drop(), or a herdr *_not_found error, which the rest of this codebase already reads as definitely-absent rather than inconclusive. Also fixes three javadoc/comment sites that claimed a timed-out send definitely did not arrive: TIMED_OUT_QUEUED, hasQueuedDelivery, the queuedDeliveries field, and the comment in send()'s timeout branch. Two claims were wrong, not merely stale: "on every route it will not arrive later" is false on the ATTEMPTED route, and "may already hold a partial paste" understates it — agent.prompt pastes AND SUBMITS in one call, so the target may hold a complete, running turn. Verified by the lead on a merged tree: mvn -o clean install from fleetd/, exit 0, 1754 tests from Maven and from an independent sum over 130 surefire reports. Mutation: folding ATTEMPTED back into NOT_DELIVERED in cancellationOf gives 3 red, each naming the property (anErrorFromSendRemovesTheMessageAndMarksItAttempted:740, aHerdrExceptionFromSendStillSurfacesButNowReportsAttempted:781, aHerdrExceptionAfterThePasteIsNeverRecordedAsConfidentlyNotDelivered:806). The final round is comment-only, proven mechanically rather than by reading: stripping every comment from MessageService.java before and after and collapsing whitespace gives byte-identical code. |
||
|
Dai Ha
|
f40c19ecf0 |
fleetd #551 shape sweep: fix stale ATTEMPTED-route javadoc/comments in MessageService
hasQueuedDelivery's javadoc, the queuedDeliveries field javadoc, and a code comment in send()'s timeout path all still made two claims that ATTEMPTED (fleetd #551) falsifies: a blanket "the message will not arrive later" across every route, and "the terminal may already hold a partial paste" — but agent.prompt pastes AND submits in one call, so the target may hold a complete, already-submitted turn. Each now names the three Injector.Cancellation routes (CANCELLED, NOT_DELIVERED, ATTEMPTED) and says plainly that only the first two establish the message will not arrive later. Comment/javadoc only. No behaviour change: Injector.java is untouched (sha256 0c689b6cf36275c0da45497a74b2bd4f5a3d80c4dbda77d46670c66004e53b69) and no test was added. mvn -o clean install: BUILD SUCCESS, Tests run: 1754, Failures: 0, Errors: 0 (unchanged from before this commit). |
||
|
Dai Ha
|
034e17bb32 |
fleetd #561 follow-up: pin the session half of bothMustRunKeepingSecondResult
Two helpers maintain one invariant (the second callback half always runs, even when the first throws): bothMustRun and bothMustRunKeepingSecondResult. Only bothMustRun's "session half still runs" direction was asserted (sessionHalfStillRunsWhenTheCompletionHalfThrowsSynchronously, via onTurnComplete). bothMustRunKeepingSecondResult — the helper onTurnCompleteWithPostAction uses — could be reverted to the pre-#561 broken shape and the suite stayed green. Adds two tests to FleetdTurnListenerCompositionTest: - sessionHalfStillRunsWhenTheCompletionHalfThrowsSynchronouslyForPostAction: mirrors the existing onTurnComplete case for onTurnCompleteWithPostAction/ bothMustRunKeepingSecondResult. - bothFailuresEscapeWhenBothHalvesThrowDistinctExceptions: proves a second, distinct failure from the session half is preserved via addSuppressed rather than silently dropped when both halves of bothMustRun throw. Also rewords the onDelivered comment in Fleetd.turnListener: it previously said this pair is safe because registration survives a throw via #556's Injector wiring, which is true but is not why THIS pair is unguarded. CompletionResolver.captureBaseline already catches RuntimeException around its scrape read and fails open, so completion.onDelivered does not realistically throw. Comment text only, no logic change. |
||
|
Dai Ha
|
68b428c484 |
fleetd #551 rework (comment 17058): fix TIMED_OUT_QUEUED javadoc for ATTEMPTED
Javadoc-only change. #551 added Injector.Cancellation.ATTEMPTED, which MessageService.send's timeout path folds into Outcome.TIMED_OUT_QUEUED alongside CANCELLED and NOT_DELIVERED (the collapse itself is unchanged behaviour and is being tracked as a separate follow-up ticket). TIMED_OUT_QUEUED's javadoc — landed by #513 to state the routes that reach it — named only two routes and said "on every route it will not arrive later", with a "may already hold a partial paste" caveat. Both claims are now stale: ATTEMPTED is a third route, and because agent.prompt pastes AND submits in one call, that route may mean the target holds a complete, already-submitted turn and is working on it right now. Names all three routes, says which one is uncertain, and drops the now-false blanket claim. No behaviour change. |
||
|
Dai Ha
|
e20ccab1eb |
fleetd #561: harden the completion/session TurnListener fan-out
Fleetd's turnListener composition had four callbacks (onTurnComplete, onTurnCompleteWithPostAction, and both onTurnFailed overloads) built from two bare, unguarded statements each. onDelivered's registration was already fixed structurally by #556; these four had the identical fragility and were still untested: nothing enforced that the completion resolver's half ran before the session half beyond call order in the source, so a future reorder (or a throwing session listener sequenced first) could silently skip the completion resolver's effect and strand a caller for its full timeout. Extracted the composition to a package-private static factory, Fleetd.turnListener(completion, sessions), and hardened it with bothMustRun/bothMustRunKeepingSecondResult: both callback halves are always attempted regardless of whether the other throws, and whatever escapes is rethrown afterward (never swallowed) so it still reaches StatusPoller's catch (Throwable) and logs at ERROR. FleetdTurnListenerCompositionTest builds this real composition from a real CompletionResolver and a throwing fake sessions half, and asserts the completion resolver's effect (the waiter resolving) survives the session half throwing, for all four callbacks, plus a mirror case showing the session half still runs when the completion half throws first. onTurnCompleteWithPostAction keeps completion-before-session as a functional requirement (resolveBeforePostAction must run before the context-reset housekeeping can erase the pane), not just fault tolerance, so it is not reorder-symmetric like the other three — documented in Fleetd.turnListener's javadoc. |
||
|
Dai Ha
|
d83821bbce |
fleetd #551: record the delivery attempt before the irreversible send
The Injector wrote the delivery outcome AFTER calling AgentControl.send(), so a HerdrException thrown from the response half of that call (herdr already replied, or may have) was recorded as a confident NOT_DELIVERED for text that may already be sitting in the worker's pane. Poll the queue entry and mark it Pending.State.ATTEMPTED before send() is called, not after. On success it is upgraded to DELIVERED; on an ordinary failure it stays ATTEMPTED (honest uncertainty), except a herdr *_not_found error, which the rest of this codebase already treats as a confirmed absence and which now still writes NOT_DELIVERED. cancellationOf gets a matching third answer (Cancellation.ATTEMPTED) instead of folding the new state into NOT_DELIVERED, so a caller that cancels an already-attempted delivery is told the truth too. |
||
|
ltms
|
ba2f4d16f8 |
Merge #555: the redeploy main flow is lifted into tested predicates, and the guard now catches functions below the SOURCED line
fleetd #555. Eight main-flow decisions in scripts/redeploy-fleetd.sh move into predicate and dispatch functions the suite can source and test. The guard test test_no_untested_main_flow_conditionals stops new bare conditionals reappearing. The rework closes a hole the lead found (comment 17012): a conditional wrapped in a function defined BELOW the SOURCED guard was invisible to the guard, and such a function can never be sourced, so it can never be tested. The guard now fails on any function definition after the guard line, on its own. Verified by the lead on a tree with main merged in, redeploy-fleetd.sh at its pristine sha 4ffacc5185807d39720a3484d85b922413806eb5347318265bd8897dfd61e8d9: bash 5.3.9 -> exit 0, 0 lines matching ^FAIL: /bin/bash 3.2.57 -> exit 0, 0 lines matching ^FAIL: Three mutations, each restored to the pristine sha afterwards: a bare conditional appended to the main flow -> exit 1, reported by line the same conditional wrapped in a function below the SOURCED guard (the found hole) -> exit 1, "function defined after the SOURCED guard (line 1038) - it cannot be sourced, so it cannot be tested" the new FUNC emission deleted from the guard -> that same case returns to exit 0, so the new assertion is what catches it. Anchor count 1 -> 0, test-redeploy-fleetd.sh restored to sha 0d713a3092a0c0ea8c05663ffa0cb1595e21b9d73872e662eb99b5035fd38151. |
||
|
ltms
|
db4c98ac60 |
Merge #556: the Injector owns turn registration, and the #553 backstop is pinned too
fleetd #556. Registration moves off the TurnListener fan-out onto its own narrow
TurnRegistrar seam, wired directly to CompletionResolver::register in Fleetd.java,
so it survives any listener throwing regardless of call order.
Verified by the lead on a tree with main (
|
||
|
Dai Ha
|
8f80d267a0 |
fleetd #555 rework: catch function definitions after the SOURCED guard
Comment 17012 on #555 found a hole in test_no_untested_main_flow_conditionals: the guard's function-body detection treats anything inside a function as "fine, out of scope for this scan" — but a function DEFINED after the SOURCED guard line can never be reached by sourcing this script (sourcing stops before the main flow runs), so its body is untestable by construction while still reading to the guard as safely inside a function. mainflow_bare_conditionals now also emits a FUNC record for every function opened after the guard line (reusing the same open-brace detection already used for depth tracking), and test_no_untested_main_flow_conditionals treats any such record as a violation on its own, independent of what the function's body contains or whether the allowlist would otherwise excuse a bare conditional inside it. Proof (redeploy-fleetd.sh restored to 4ffacc5185807d39720a3484d85b922413806eb5347318265bd8897dfd61e8d9 after each): - CONTROL — a bare conditional appended to the main flow is still caught: EXIT=1, "found 1 untested main-flow if/elif/case line(s) ... line 1341: if [ "$MY_CONTROL_BARE" = 1 ]; then :; fi" - CANDIDATE — the same conditional wrapped in a function defined after the boundary, previously invisible (EXIT=0), is now caught: EXIT=1, "line 1341: function defined after the SOURCED guard (line 1038) — it cannot be sourced, so it cannot be tested: newfunc_below_the_boundary() {" Full suite re-run green on both bash 5.3.9 and /bin/bash 3.2.57 (macOS system bash): exit 0, 0 FAIL lines, reached the final PASS line, on both. No change to redeploy-fleetd.sh; the 8 lifted decisions, their mutation proofs, and the allowlist all stand as before. |
||
|
Dai Ha
|
738d34a609 |
fleetd #556 rework: pin registration on the #553 finally backstop path
Comment 17009: there are TWO registrar.register(target, sent.token()) call sites in Injector's delivery method — the ordinary path inside `if (sent != null)`, and the fleetd #553 finally backstop, reached only when an earlier block throws before the ordinary path ever runs. The lead's mutation on Injector.java:703 (the backstop call) survived the full suite: the existing #553 regression test for this exact scenario (aRuntimeExceptionFromOnTurnCompleteStillCompletesTheNextDelivery) asserts only that the delivered future completes, never that the turn is registered with CompletionResolver — so a redesign that dropped registration from the backstop would reopen this ticket's own defect on precisely the path #553 exists for, with every existing test green. Adds aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath: drives the same construction as the existing #553 test (onTurnComplete throws for a previous turn, forcing the next turn's delivery down the finally backstop) and additionally asserts the new turn is registered with CompletionResolver and carries the correct waiter — the same assertion the ordinary-path test makes, now made on the recovery path. Proven by mutation: removing Injector.java:703 alone (exact-line anchor 1 -> 0) turns the new test red with its own assertion message; restored and confirmed byte-identical (sha256 8fcb698afccc254b0c99d3a4bf9e960c0e7c85e542024e870c1e815dce6d62a9, matching the pre-mutation tree); re-run green as a control. Full suite after restore: 1750 tests, 0 failures, 0 errors, 0 skipped (Maven's own summary and an independent sum over surefire-reports/*.txt agree), BUILD SUCCESS. |
||
|
Dai Ha
|
a46e4058ac |
fleetd #556: make turn registration structural, independent of any TurnListener
The Injector owns the invariant "every delivered turn has a registered waiter," but before this the only thing that satisfied it was CompletionResolver.captureBaseline, called from inside a TurnListener callback wired in Fleetd.java. Any TurnListener that throws (from onDelivered or elsewhere) could break the invariant with no way for the Injector to detect it. #553 only made the one reachable listener behave via a try/finally backstop; it did not remove this structural dependency. Add a narrow TurnRegistrar functional interface, decoupled from TurnListener, whose only job is registering a delivered turn's waiter. CompletionResolver now implements it via a new register() method (extracted from captureBaseline's registration half; captureBaseline keeps its own full body unchanged, so existing direct callers/tests are untouched). Injector gets an explicit registrar field/constructor family (auto-derived from the TurnListener via instanceof where that still works, explicit where Fleetd's anonymous fan-out listener can't implement two interfaces at once) and calls registrar.register(...) directly and unconditionally in both the ordinary delivery path and the #553 finally backstop, before turnListener.onDelivered(...) — so registration no longer depends on that notification callback succeeding. Fleetd.java wires completion::register explicitly as the registrar, bypassing the turnListener fan-out for registration purposes. CB-116 ordering (onTurnComplete reads the PREVIOUS turn's inFlight entry before the new turn's registrar.register() runs) and the two-arg inFlight.remove(target, turn) vs one-arg distinction on the completion path are both preserved unchanged. #561's order-dependent test (asserting Fleetd.java's completion.onDelivered -> sessions.onDelivered call order) does not exist anywhere in this repo at this branch point — nothing to delete. Adds 5 tests: a TurnListener that throws from every callback still leaves the delivered turn registered and resolvable; the new turn's registration still runs after the previous turn's completion is read (CB-116 guard, pinned as a call-order assertion); and three tests naming the two-arg-remove invariant directly (a superseded turn's terminal handling must not evict its successor's registration) across resolve()'s plain-completion branch, its echoed-noReportMessage sub-path, and fail(). |
||
|
ltms
|
f4f5f3106e |
Merge #513: TIMED_OUT_QUEUED has four routes, and the javadoc now says which
The old javadoc said a TIMED_OUT_QUEUED message was "still sitting in the injector's
per-target queue". It is not — send() has already given up on it and it will never
arrive. That wrong claim told an operator to wait for a message that was never coming.
The first fix replaced it with a narrower wrong claim: that Injector.cancel() cancelled
the entry and "the target never saw a word of it". That describes one of four routes.
TIMED_OUT_QUEUED is returned whenever injector.cancel() returns anything but DELIVERED:
CANCELLED the Pending was still queued and this call removed it
NOT_DELIVERED Injector.java:400 - the herdr agent.prompt call threw
NOT_DELIVERED Injector.java:419 - readiness grace expired, never attempted
NOT_DELIVERED Injector.java:691 - drop(), the target is gone
On the last three, cancel() cancels nothing: it reads a state another path already set
(cancellationOf, Injector.java:288-290). And on the herdr-threw route, agent.prompt
pastes and submits in one call, so a throw does not prove the pane stayed clean - an
operator told "never saw a word" will not go and look at the one place the evidence is.
The javadoc now states the two facts that hold on every route - the message will not
arrive later, and it is not in any queue - and attaches "the target saw nothing" only to
the CANCELLED case. Five blocks: the enum constant, queuedDeliveries, hasQueuedDelivery,
hasOrphanedDelegation, and the inline comment in the TimeoutException branch that seeded
the wording.
Comment-only; no logic changed.
Verified on a tree merged with main (fast-forward to
|
||
|
Dai Ha
|
8d79d229ff |
fleetd #555: lift 8 main-flow decisions into tested predicate/dispatch functions
redeploy-fleetd.sh's main flow had 8 bare if/case decisions (CHECK_ONLY
short-circuit, drain-gate entry+confirm, supervisor report/stop/start
dispatch, health-poll decision, HAD_OLD_PID computation) that lived outside
any function, so the 67-test suite could not reach them and any one could be
silently inverted with the whole suite green.
Follows the existing swap_if_built/refuse_drain_gate pattern: each bare
guard becomes a small predicate or dispatch function (should_stop_for_check,
drain_gate_required/drain_confirmed/run_drain_gate, report_supervisor_state,
dispatch_stop, dispatch_start, health_is_up/report_health,
compute_had_old_pid), called unconditionally by the main flow so the
decision itself is unit-testable in isolation.
Adds a structural guard, test_no_untested_main_flow_conditionals, that scans
the main flow (everything after the SOURCED guard) for bare if/elif/case
lines outside any function body, tracking function boundaries via this
file's one consistent name() { / } convention. It fails on any new bare
conditional not covered by MAIN_FLOW_ALLOWED_CONDITIONALS, an explicit
exact-text allowlist of the report-only/display conditionals and the two
#504-family supervisor elif branches that stay out of scope for this
ticket. This is the "shape, not the eight sites" guard the ticket asked
for: a ninth bare decision fails immediately, naming its line.
Out of scope, not touched: #504 items 2/3/4 and #528 item 2 (same
untested-main-flow family) — the seam here generalizes to make them
testable too, but lifting them was left for their own tickets.
|
||
|
Dai Ha
|
cb64bc8157 |
fleetd #513: rework — TIMED_OUT_QUEUED has four routes, not one
Comment 16984 on the ticket showed my first pass (
|
||
|
Dai Ha
|
ed28b51f12 |
fleetd #513: fix TIMED_OUT_QUEUED javadoc — cancelled, not queued
Two javadoc blocks (queuedDeliveries field, hasQueuedDelivery) said a timed-out message is still sitting in the injector's per-target queue. CB-640 made send() cancel it via Injector.cancel() instead, so the message is gone and will never arrive. Rewrote both to describe cancellation. Also fixed a third instance of the same stale claim in hasOrphanedDelegation's javadoc, and added a line to the TIMED_OUT_QUEUED enum constant's own comment clarifying the name is kept but no longer means the message stays queued. Per the ticket's follow-up comment: no rename (TIMED_OUT_QUEUED reaches FleetApp.java REST mapping and FleetMcp.java — out of scope here) and no behavior change; comments only. |