Commit Graph

967 Commits

Author SHA1 Message Date
Dai Ha e20ccab1eb fleetd #561: harden the completion/session TurnListener fan-out
CI / shell-tests (pull_request) Successful in 8s
CI / contract (pull_request) Successful in 1m4s
CI / build (pull_request) Successful in 1m56s
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.
2026-09-12 17:47:53 +07:00
ltms ba2f4d16f8 Merge #555: the redeploy main flow is lifted into tested predicates, and the guard now catches functions below the SOURCED line
CI / shell-tests (push) Successful in 9s
CI / contract (push) Successful in 1m0s
CI / build (push) Successful in 1m52s
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.
2026-09-12 12:19:47 +02:00
ltms db4c98ac60 Merge #556: the Injector owns turn registration, and the #553 backstop is pinned too
CI / shell-tests (push) Successful in 6s
CI / contract (push) Successful in 51s
CI / build (push) Successful in 2m12s
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 (f4f5f31) merged in:
  mvn -o clean install exit 0; 1750 tests, agreed by Maven's own summary and an
  independent sum over 130 surefire report files.

Both registrar.register call sites are independently pinned:
  Injector.java:703 (the #553 finally backstop) removed -> exactly 1 failure, the
    new aRuntimeExceptionFromOnTurnCompleteStillLeavesTheNextDeliveryRegisteredOnTheRecoveryPath
  Injector.java:660 (the ordinary path) removed -> exactly 2 failures, the two
    original tests; the backstop test stays green
Anchor count 1 -> 0 on each, sha restored to
8fcb698afccc254b0c99d3a4bf9e960c0e7c85e542024e870c1e815dce6d62a9 both times.

The :703 line survived the full suite before this rework while being executed
twice — covered but unasserted. That gap is what the added test closes.
2026-09-12 12:18:15 +02:00
Dai Ha 8f80d267a0 fleetd #555 rework: catch function definitions after the SOURCED guard
CI / shell-tests (pull_request) Successful in 9s
CI / contract (pull_request) Successful in 1m20s
CI / build (pull_request) Successful in 3m3s
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.
2026-09-12 17:11:11 +07:00
Dai Ha 738d34a609 fleetd #556 rework: pin registration on the #553 finally backstop path
CI / shell-tests (pull_request) Successful in 7s
CI / contract (pull_request) Successful in 53s
CI / build (pull_request) Successful in 2m13s
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.
2026-09-12 17:08:44 +07:00
Dai Ha a46e4058ac fleetd #556: make turn registration structural, independent of any TurnListener
CI / shell-tests (pull_request) Successful in 4s
CI / contract (pull_request) Successful in 50s
CI / build (pull_request) Successful in 2m29s
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().
2026-09-12 16:50:09 +07:00
ltms f4f5f3106e Merge #513: TIMED_OUT_QUEUED has four routes, and the javadoc now says which
CI / shell-tests (push) Successful in 4s
CI / contract (push) Successful in 54s
CI / build (push) Failing after 1m51s
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 cb64bc8):
  mvn install exit 0
  1744 tests, 0 failures, 0 errors - Maven's own summary and an independent sum over
  130 surefire report files agree
  no unresolved javadoc reference on the three new links, with a positive control
  showing javadoc did analyse MessageService.java

Closes #513.
2026-09-12 11:48:54 +02:00
Dai Ha 8d79d229ff fleetd #555: lift 8 main-flow decisions into tested predicate/dispatch functions
CI / shell-tests (pull_request) Successful in 11s
CI / contract (pull_request) Successful in 1m31s
CI / build (pull_request) Successful in 1m46s
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.
2026-09-12 16:46:53 +07:00
Dai Ha cb64bc8157 fleetd #513: rework — TIMED_OUT_QUEUED has four routes, not one
CI / shell-tests (pull_request) Successful in 5s
CI / contract (pull_request) Successful in 49s
CI / build (pull_request) Successful in 2m16s
Comment 16984 on the ticket showed my first pass (ed28b51) replaced one
wrong invariant with a narrower one: it described injector.cancel()
cancelling a queued entry as THE mechanism, when three of its four
routes (Injector.java: the send-to-terminal call throwing, the
readiness grace expiring, or the target being torn down) never cancel
anything — cancel() just reports a state a different code path already
set. It also claimed the target never saw a word of the message, which
is not established when the herdr agent.prompt call throws after
already pasting.

Rewrote all four comments (the TIMED_OUT_QUEUED enum constant,
queuedDeliveries, hasQueuedDelivery, hasOrphanedDelegation) plus the
pre-existing inline comment that seeded the original bad wording, to
state only what holds on every route: the message will not arrive
later and is not sitting in a queue. The CANCELLED case is called out
as the only one where the target is known to have seen nothing; the
NOT_DELIVERED case (including the herdr-send-threw route) is flagged
as leaving that open.

Comment-only; no behavior change.
2026-09-12 16:45:35 +07:00
Dai Ha ed28b51f12 fleetd #513: fix TIMED_OUT_QUEUED javadoc — cancelled, not queued
CI / shell-tests (pull_request) Successful in 4s
CI / contract (pull_request) Successful in 52s
CI / build (pull_request) Successful in 2m22s
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.
2026-09-12 16:32:36 +07:00
ltms 4f9aba40e7 Merge #558: the ticket is the pull channel, and the brief is write-once
CI / shell-tests (push) Successful in 6s
CI / contract (push) Successful in 53s
CI / build (push) Failing after 1m51s
Docs only, 16 additions / 3 deletions — read by the lead in full, per the
under-50-lines self-review rule in CLAUDE.md.

Verified on the merged tree (main dab697f + 351ee1e): the canonical-block sync
check from the addendum prints `in sync: True`, comparing the MERGED CLAUDE.md
against wiki/7-Use-Cases.md. The wiki side is already pushed and verified by ref:
local wiki HEAD and `git ls-remote origin main` both read 20d2fc0.

CI run 1812 on 351ee1e: success.
2026-09-12 11:12:55 +02:00
ltms dab697fae0 Merge #559: progress watchdog for StatusPoller and SessionReaper loops (fleetd #544)
CI / shell-tests (push) Successful in 7s
CI / contract (push) Successful in 51s
CI / build (push) Successful in 2m19s
Verified by the lead on a merged tree (main 7a3b2bb + bfac141 = 5de807f):
mvn clean install exit 0, Tests run: 1744, Failures: 0, 130 surefire reports.

Three mutations run by the lead, all killed:
- StatusPoller.java:105 `watchdog.reset()` deleted (the surviving mutant from the
  first review) -> aRestartedLoopReportsRunningAgainNotStoppedForever:
  expected: <RUNNING> but was: <STOPPED>.
- LoopWatchdog.java:73 `lastRoundNanos = nowNanos.getAsLong()` deleted from reset()
  (not run by the worker) -> LoopWatchdogTest.resetClearsAPreviousStopAndTheStaleClock:
  expected: <RUNNING> but was: <STALLED>.
- LoopWatchdog.java:89 `>=` -> `>` boundary (not run by the worker) ->
  LoopWatchdogTest.reportsStalledOnceTheLastRoundAgesPastTheThreshold:
  expected: <STALLED> but was: <RUNNING>.

Each mutation counted the pristine full line 1 -> 0 by exact string equality
(awk '$0==p'), with a pristine control copy still reading its original count, and
was restored to a byte-identical file (shasum -a 256) before the next run.
Control build after all restores: exit 0, 1744 tests, 0 failures.
2026-09-12 11:11:46 +02:00
Dai Ha bfac14108f fleetd #544: pin the sticky-STOPPED-across-restart invariant
CI / contract (pull_request) Successful in 1m21s
CI / build (pull_request) Successful in 1m42s
Review of PR #559 (issue comment #16944) found a surviving mutant: removing
watchdog.reset() from StatusPoller.start() (and the identical line in
SessionReaper.start()) passed the entire suite.

stoppedByCaller is sticky and reset() — called only from start() — is the
only thing that clears it. Both loops document start() as idempotent and
loop()'s own error log says "it can be restarted", so stop() followed by
start() is an anticipated path. Without reset() wired into start(), health()
would report STOPPED forever after a restart even though the loop is
genuinely running again.

Add aRestartedLoopReportsRunningAgainNotStoppedForever to both
StatusPollerWatchdogTest and SessionReaperWatchdogTest, pinning "an
intentional stop must not outlive the restart that follows it". Verified via
the standard mutation cycle: exact-line anchor (not regex, to avoid the
\Q-style false match the reviewer flagged) counted pristine 1 -> mutated 0,
test goes red with its own message, restored, shasum -a 256 byte-identical,
green again.

mvn clean install: exit 0, BUILD SUCCESS, Tests run: 1734, Failures: 0,
Errors: 0, Skipped: 0 (cross-checked against 130 surefire report files).

No production code changed — the reset() call under test was already
correct; it simply had nothing pinning it.

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
2026-09-12 16:00:58 +07:00
ltms 7a3b2bb7ee Merge pull request 'fleetd #552: warn instead of aborting when the post-restart mktemp fails' (#560) from worker/552-post-restart-mktemp-abort-bc2672-4 into main
CI / shell-tests (push) Successful in 7s
CI / contract (push) Successful in 1m30s
CI / build (push) Successful in 2m9s
2026-09-12 10:58:10 +02:00
Dai Ha f188947750 fleetd #552: warn instead of aborting when the post-restart mktemp fails
CI / shell-tests (pull_request) Successful in 7s
CI / contract (pull_request) Successful in 49s
CI / build (pull_request) Successful in 2m7s
By the time the fresh-log mktemp ran, the daemon had already been stopped, the
jar swapped, and the new daemon started — an unguarded mktemp failure there
aborted the whole script anyway, so a caller read the resulting non-zero exit
as "the redeploy failed" and would restart an already-correctly-restarted
daemon.

Extract the mktemp into capture_fresh_log_region, guarded the same way
unload_launchd_if_loaded/stop_systemd_if_loaded guard their own, but warn
instead of die: there is nothing left to protect by refusing after a
successful restart. The trap is now installed before the assignment it cleans
up, using an FRESH_LOG="" sentinel readers can check.

classify_amqp_connection_errors and report_shutdown_drain both gain a new
state (REDEPLOY_AMQP_CHECK_SKIPPED / REDEPLOY_DRAIN_STATE=skipped) for an
uncapturable log region, distinct from "captured a region with nothing in
it" — and the result section gains a matching branch, so a skipped capture
can never read as a clean bill of health.
2026-09-12 15:55:04 +07:00
ltms f606fccf7f Merge pull request 'fleetd #553: register the rendezvous waiter in onStatus's finally backstop' (#557) from worker/553-onstatus-completion-leak-0da881-2 into main
CI / shell-tests (push) Successful in 4s
CI / contract (push) Successful in 54s
CI / build (push) Successful in 1m40s
2026-09-12 10:50:24 +02:00
Dai Ha 735b6af976 fleetd #544: progress watchdog for StatusPoller and SessionReaper loops
CI / contract (pull_request) Successful in 1m19s
CI / build (pull_request) Successful in 2m29s
Each loop's virtual-thread runner (StatusPoller, SessionReaper) can die or
get permanently parked in a herdr call with no read timeout, and nothing
observed it: /healthz stayed green and Thread.isAlive() kept reporting true
the whole time.

Add LoopWatchdog (dev.ltms.fleet.inject — see its javadoc for why not
dev.ltms.fleet.health, which would close a package cycle through session):
each loop now records a monotonic last-completed-round timestamp
(injectable LongSupplier clock, same pattern as Injector/SessionManager) and
exposes it as a three-state health() fact — RUNNING, STALLED (dead or
parked, indistinguishable from outside), STOPPED (stop() was called on
purpose, never an alarm). This is the fleetd #512 shape: one flag cannot
carry both "halted on purpose" and "halted unexpectedly", so stop() marks
its own state explicitly instead of leaving state() to infer it from
staleness.

Staleness thresholds are derived from each loop's own poll interval with a
documented multiplier: StatusPoller 40x (250ms -> 10s), SessionReaper 12x
(5000ms -> 60s).

Scope: observability only, per the ticket's own comment. No restart/recovery
mechanism, no /healthz or REST/MCP wiring beyond the public health() API, no
change to the per-item catch(Throwable) behavior (#543) or a process-wide
uncaught-exception handler (ruled out on #538), and no deadline added to the
herdr read itself (a separate, real ticket).

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
2026-09-12 15:48:07 +07:00
Dai Ha 8b4320ed24 fleetd #553: split sentHandled's two meanings so onDelivered's own throw still completes the future
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Successful in 1m43s
Lead review of PR #557 (ticket comment 16916) found one path left open: sentHandled
is set to true BEFORE onDelivered() runs (correctly, per the earlier fix), so when
onDelivered() itself throws on the normal path, the finally's 'if (sent != null &&
!sentHandled)' guard skipped the whole recovery -- completion included -- and left
sent.delivered() pending forever for a message that really was delivered.

sentHandled must guard only the onDelivered RE-CALL (the permanent-suppression
hazard), never the future completion, since CompletableFuture.complete/
completeExceptionally are idempotent and a no-op on the already-handled path.
Split the one flag's two jobs: the outer 'if (sent != null)' now always runs the
recovery block, and '!sentHandled' moved onto just the onDelivered call inside it.

Added anOnDeliveredThrowOnTheNormalPathStillCompletesTheDeliveryFuture, proven with
the lead's own mutation (reverting !sentHandled onto the outer if): the new test
goes red while anOnDeliveredThrowAfterItsOwnRegistrationDoesNotRunASecondTime stays
green, showing the two concerns are genuinely separate.
2026-09-12 15:45:36 +07:00
Dai Ha 351ee1ea6d CLAUDE.md: the ticket is the pull channel, and the brief is write-once
CI / shell-tests (pull_request) Successful in 6s
CI / contract (pull_request) Successful in 1m26s
CI / build (pull_request) Successful in 2m10s
A send to a working member is accepted and returns a ticket, then is never
delivered. That happened three times in one session here, and the member was
released still executing a brief that had been retracted twice. The receipt is
true — it is a fact about the mailbox, when what was needed was a fact about the
pane.

The fleet01 lead named the mechanism: a push delivery needs the recipient free at
send time, while a pull channel needs only that they look before acting. So the
ticket is not more reliable than the mailbox, it is a different direction, and its
success depends on the member's procedure rather than on the timing of the send.

Both halves have to be written down, because each is useless alone:

- Member (turn contract, new item 4): re-read the ticket before acting on anything
  told earlier, and again before committing. A ticket comment that contradicts the
  brief is newer and wins.
- Lead (step 5): all corrections go to the ticket, and the brief is write-once.
  The member cannot check which source is newer — it just always prefers the
  ticket — so revising a brief in place makes it obey the rule and do the wrong
  thing. A first brief for a unit not yet running is not a correction.

wiki/7-Use-Cases.md is updated to keep the canonical block byte-identical; the
sync check passes. The wiki submodule pointer is deliberately left unstaged.
2026-09-12 15:45:09 +07:00
Dai Ha d4a51c6274 fleetd #553: register the rendezvous waiter in onStatus's finally backstop, not just the delivery future
CI / contract (pull_request) Successful in 50s
CI / build (pull_request) Successful in 2m12s
The previous try/finally around onStatus's post-monitor region completed
sent.delivered() but never registered sent.token().waiter() when an earlier
listener threw. That waiter is registered only by turnListener.onDelivered(),
inside the very if (sent != null) block the finally backstops, so a caller
was told its send landed and then waited out its full timeout for an answer
that could never resolve (worse than a plain hang).

The finally now does that block's whole job on the unhandled path: it calls
onDelivered() (when sendError == null) before completing the future, guarded
by its own try/catch(Throwable) so a failure there cannot mask the original
throwable. A sentHandled flag, set true at the START of the normal block
(before any side effect), tells the finally whether that already ran, so a
throw partway through onDelivered cannot trigger a second, late captureBaseline
that would permanently suppress the turn's completion.

Also removed a leftover duplicated forget.accept(target) call (with a stray
'MUTATION-TEST-3' comment) in the notReady block — residue from the previous
worker's own mutation testing that was not fully reverted.
2026-09-12 15:35:53 +07:00
ltms 26f380a00b Merge #554: portable hash256, a third state for an unhashable jar, and the shell suite in CI (#550)
CI / shell-tests (push) Successful in 8s
CI / contract (push) Successful in 1m17s
CI / build (push) Successful in 2m24s
Closes fleetd #550, all three items.

Verified by me on the branch at b8182c9, in a scratch worktree, not from the implementer's report.

The decisive pair, both in ubuntu:latest where shasum is absent and sha256sum is present:

  main   (93a9ed3):  SUITE exit=127   anchored ^FAIL: count 0
                     scripts/test-redeploy-fleetd.sh: line 298: shasum: command not found
  branch (b8182c9):  SUITE exit=0     anchored ^FAIL: count 0

Identical FAIL counts, opposite exit codes. That is why the new shell-tests CI job gates on the
step's own exit status and deliberately does not grep for a FAIL count: a suite that dies before
running a single test prints exactly what a clean pass prints.

Also measured by me: macOS exit 0 / anchored count 0; 70 tests defined and 70 invoked with an
empty comm -3; bash -n exit 0 under both /bin/bash 3.2.57 and bash 5.3.9; ci.yml parses with jobs
build, shell-tests, contract, and shell-tests is ubuntu-latest + actions/checkout@v4 +
bash scripts/test-redeploy-fleetd.sh.

Three mutations, all killed, each restored byte-identical against
515d929bb53c9ec2c95042e47d3e4d611d60171345227b07feb0de0d20643d3a:

  drop the sha256sum branch from hash256   anchor 1->0   Linux exit 1,
      test_no_unguarded_macos_only_hasher_calls with its own message
  echo "unhashable" -> echo "absent"       anchor 1->0   FAIL: jar_id reported absent for a file
      that exists, only because no hasher was on PATH
  both hash256 arms -> shasum -a 1         anchors 1->0 and 1->0   FAIL: hash256 of the literal
      3-byte input 'abc' must be the known SHA-256 prefix, not some other algorithm's: expected
      ba7816bf8f01, got a9993e364706

The third mutation SURVIVED on the first head (3da44ee) and was sent back. Fixing item 2 had
rewired the reference hashes in test_jar_id_defaults_to_live_and_reports_explicit_path onto
hash256 itself, making the test's reference and its subject one instrument — they agree whatever
it computes, and the existing fixture guard catches only a constant return, not a wrong algorithm.
b8182c9 adds test_hash256_computes_a_real_sha256, pinning the FIPS 180 vector for "abc" as a
literal constant written into the test rather than taken from any hasher. That closes it: the
mutation now goes red, and a9993e364706 is SHA-1("abc"), which proves the mutated code really ran.

Root cause, for the record: shasum was the trigger, not the cause. Under set -euo pipefail a
missing hasher makes the pipeline status 127, the `|| echo "absent"` fires, and jar_id returns a
confident false "absent" for a jar that is right there. Without pipefail the same function returns
an empty string and is visibly broken. A default at the read site that maps every failure onto one
value which already means something specific is the defect; the fix separates "not there" from
"could not hash it".
2026-09-12 10:20:29 +02:00
Dai Ha b8182c96c2 fleetd #550: pin hash256's algorithm against a literal SHA-256 test vector
CI / shell-tests (pull_request) Successful in 5s
CI / contract (pull_request) Successful in 59s
CI / build (pull_request) Successful in 2m27s
test_jar_id_defaults_to_live_and_reports_explicit_path's reference hash is computed by calling
hash256 itself (needed so it doesn't call the Linux-crashing bare shasum directly). That made
subject and reference the same instrument: they agree no matter which algorithm hash256 actually
runs, so a mutation swapping both of hash256's arms for the wrong algorithm was invisible to the
suite.

Adds test_hash256_computes_a_real_sha256, pinned against the published SHA-256 test vector for the
3-byte input "abc" (ba7816bf8f01...), written as a literal constant rather than computed by any
hasher at test time. Verified the constant myself both ways (sha256sum and shasum -a 256) before
writing it in.
2026-09-12 15:16:43 +07:00
Dai Ha 3da44eed63 fleetd #550: replace shasum with a portable hash256 helper, add a Linux CI job for the shell suite
CI / shell-tests (pull_request) Successful in 5s
CI / contract (pull_request) Successful in 1m17s
CI / build (pull_request) Successful in 1m46s
jar_id() in redeploy-fleetd.sh called shasum directly, which does not exist on GNU coreutils
Linux (Debian/Ubuntu/etc.) — there it silently reported an existing jar as "absent" with exit 0,
because the missing command made `cut` succeed on empty input and pipefail's failure was then
swallowed by the `|| echo "absent"` fallback. The shell test suite hit the same tool at
test-redeploy-fleetd.sh:298-299 and died at exit 127 with zero FAIL lines printed — the same shape
as a clean pass on the one channel anyone would check.

Adds one hash256() helper (prefer sha256sum, fall back to shasum -a 256, same idiom already used
in probe-member-credentials.sh) and points jar_id and the test suite's own reference hash at it.
jar_id now has three distinct answers instead of two: absent, a hash, or "unhashable" when neither
hasher is on PATH — "absent" is never used for a file that exists.

Adds a CI job (shell-tests) that runs scripts/test-redeploy-fleetd.sh on ubuntu-latest, gated on
the step's own exit code rather than a FAIL-line count, since a suite that dies before running is
exactly what a green run also looks like by that count.

New tests: test_jar_id_reports_unhashable_when_no_hasher_on_path (stubbed PATH with neither
hasher) and test_no_unguarded_macos_only_hasher_calls (a shape check across every script under
scripts/, not named lines — #545 already showed this idiom spreading from two sites to six).
2026-09-12 15:06:15 +07:00
ltms 93a9ed3f83 Merge #549: widen Injector's delivery catch to Throwable (#546)
CI / contract (push) Successful in 56s
CI / build (push) Successful in 1m43s
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
ltms 0b032f5a1a Merge #548: fix mktemp -t templates for GNU coreutils, split the unclear-supervisor detail (#545)
CI / contract (push) Successful in 55s
CI / build (push) Successful in 2m16s
Verified by me on the branch, not on the worker's report.

Source audit, on a scratch worktree at a476a14:
- Every `mktemp -t` site in scripts/ now carries an X placeholder. The only remaining
  `mktemp -t` text with no X is a prose comment in the test file, not a call.

Suite, macOS (/bin/bash 3.2.57 and env bash 5.3.9):
- exit 0, anchored `^FAIL:` count 0. Unanchored `FAIL:` count 3 — the suite's own internal
  mutation-cell fixture lines, same as main.
- Test functions defined vs invoked: 67/67, `comm -3` empty.

Three mutations of my own, none of them one the worker used:
1. Removed `.XXXXXX` from the `fleetd-fresh-log` site (line 1089). Pristine anchor count went
   1 -> 0, so the mutation really applied. Suite exit 1, FAIL named that exact line.
2. Made the state-2 branch in `detect_supervisor` unreachable (`= 2` -> `= 9`). Suite exit 1:
   "a systemd probe setup failure must read as unclear, not none: expected unclear, got none".
3. Made `systemd_loaded` set the old value (`=2` -> `=1`) on setup failure. Suite exit 1:
   "must flag a SETUP failure (2), distinct from a probe-answered-with-stderr failure (1)".
All three restored; `shasum -a 256` back to 77fe15e5945c7d4ef9b1a2cd8f46e6f1d0be5004f595ea03964d1d7c16e859f7,
the same hash the worker reported independently. Green control after the restores: exit 0, 0
anchored FAILs.

A fourth attempt did not count. A perl `\Q...\E` pattern silently interpolated the shell
variables in it, so the file was never changed and the suite's exit 0 meant nothing. The proof
cell caught it: the pristine anchor count was still 1 after the "mutation". A mutation that did
not apply is not a surviving mutant.

The measurement macOS cannot make: I ran both arms under GNU coreutils 9.1 in a
debian:bookworm-slim container, with a `systemctl` stub that exits non-zero and writes NOTHING to
stderr — a clean negative answer.

  main (a476a14's base):
    mktemp: too few X's in template 'systemd-loaded-err'
    systemd_loaded rc=1  SYSTEMD_LOADED_ERRORED=1
    detect_supervisor => unclear | "systemctl exited non-zero and reported an error on stderr,
                                   not a clean negative — e.g. it cannot reach the user bus"

  this branch:
    systemd_loaded rc=1  SYSTEMD_LOADED_ERRORED=0
    detect_supervisor => none

So on Linux, main tells the operator that systemctl answered badly, when systemctl ran fine and
gave a clean negative. The message named a cause that was never measured. This branch removes it.

Found while doing this, NOT part of this PR, ticket to follow: the shell suite cannot run on
Linux at all. It dies at the first `shasum` call with "command not found" and exit 127, and the
anchored `^FAIL:` count reads 0 — identical to a green run. Gitea CI never runs this suite, so
nothing caught it.
2026-09-12 09:30:30 +02:00
ltms fad99c4c5e Merge #547: record the finally non-goal on drainAll's completion line
CI / contract (push) Successful in 49s
CI / build (push) Successful in 2m10s
Comment only. No behaviour change.

Verified by me before merging:
- `mvn -B clean install` in a scratch worktree: exit 0, Tests run: 1716, Failures: 0, Errors: 0,
  Skipped: 0. Same count as main, as expected for a javadoc-only change.
- #459's javadoc reference gate: exit 0, 0 reference errors.
- Gitea CI run 1801 on 5eb4267: success.

The constraint came from the fleet01 lead. Their point: the value of the `drain complete` line is
that it is MISSING when a drain does not finish. A `finally` block would print it after a drain
that threw, with partial counts, and destroy both halves at once. The code already avoids this;
what was missing was the sentence that stops a reviewer putting it back.
2026-09-12 09:29:46 +02:00
Dai Ha 87871eaefb fleetd #546: widen Injector's delivery catch to Throwable, stop re-delivery on Error
CI / contract (pull_request) Successful in 1m29s
CI / build (pull_request) Successful in 1m49s
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 a476a14f1c fleetd #545: fix mktemp -t templates for GNU coreutils, split unclear-supervisor detail
CI / contract (pull_request) Successful in 1m16s
CI / build (pull_request) Successful in 2m28s
Every mktemp -t template in redeploy-fleetd.sh lacked an X placeholder. BSD mktemp
(macOS) tolerates that and appends its own suffix; GNU mktemp (every Linux
distribution) refuses it and exits non-zero. All six sites now use .XXXXXX.

detect_supervisor's 'unclear' detail used to cover two different facts with one
message that always named 'systemctl exited non-zero and reported an error on
stderr' — even when systemctl was never run, because mktemp failed first. The
SYSTEMD_LOADED_ERRORED/SYSTEMD_INSTALLED_ERRORED flags now carry a third value
(2 = the probe's own mktemp setup failed) alongside the existing 1 (systemctl ran
and answered badly on stderr), and detect_supervisor gives each its own detail
text. kind stays 'unclear' in both cases; require_drivable_supervisor is unchanged.

Tests added to scripts/test-redeploy-fleetd.sh:
- test_mktemp_dash_t_templates_have_x_placeholders: source-text check, fails if
  any mktemp -t template lacks an X.
- test_detect_supervisor_systemd_probe_setup_failure_is_unclear: proves the
  SET-UP-FAILED detail when mktemp itself fails (systemctl never runs).
- test_detect_supervisor_systemd_probe_error_is_unclear: extended with assertions
  that the PROBE-ANSWERED-WITH-STDERR detail is present and the SET-UP-FAILED
  wording is absent, so swapping the two messages fails a test in both
  directions.
2026-09-12 14:22:57 +07:00
Dai Ha 5eb4267a4a fleetd #512 follow-up: record why the drain-complete line must not move into a finally
CI / contract (pull_request) Successful in 1m3s
CI / build (pull_request) Successful in 2m8s
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 7611b69667 Merge pull request 'fleetd #504 item 1: stop the false ok on the loaded-but-not-running stop path' (#541) from worker/504-failed-reported-clean-3cfd66-3 into main
CI / build (push) Successful in 1m35s
CI / contract (push) Successful in 1m42s
2026-09-12 09:09:00 +02: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
CI / contract (push) Successful in 51s
CI / build (push) Successful in 2m10s
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
CI / contract (push) Successful in 1m16s
CI / build (push) Successful in 2m28s
2026-09-12 08:55:46 +02:00
Dai Ha 343ce0f4c0 fleetd #538: recover polling loops after errors
CI / contract (pull_request) Successful in 44s
CI / build (pull_request) Successful in 2m0s
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
CI / contract (push) Successful in 1m4s
CI / build (push) Successful in 2m9s
2026-09-12 08:46:23 +02:00
Dai Ha 1850a5f324 fleetd #426: pin FleetHealthMonitor.coverage and its HealthCoverageSource call site
CI / contract (pull_request) Successful in 48s
CI / build (pull_request) Successful in 2m16s
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 e4eb3dbed4 fleetd #504 item 1: stop swallowing real launchctl/systemctl failures on the 'loaded but not running' path
CI / contract (pull_request) Successful in 52s
CI / build (pull_request) Successful in 1m55s
The two 'loaded but not currently running' branches in the stop step (launchd/systemd, reached
when $OLD_PID is empty) ran 'launchctl unload'/'systemctl --user stop' with '2>/dev/null || true'
and printed 'ok' unconditionally. That swallowed a real supervisor failure (e.g. launchd or the
systemd user bus unreachable) exactly like a harmless already-stopped answer, and let the script
proceed to start a new daemon believing nothing was loaded -- the two-daemons failure fleetd #492
exists to prevent.

Adds unload_launchd_if_loaded/stop_systemd_if_loaded, applying systemd_loaded's own pattern
(capture stderr separately; a non-zero exit WITH stderr is a real failure, a non-zero exit with
empty stderr is a clean already-stopped answer) to the write side. The two call sites now use
these functions instead of the bare '|| true'.

Adds 5 tests: dies-on-real-failure and tolerates-clean-negative for each function, plus a
source-text check that the main flow calls the new functions instead of the original bare
'2>/dev/null || true'. All 5 verified by mutation (reintroducing the swallow, and separately
over-correcting to die unconditionally) -- each goes red with its own message, restores
byte-identical (full sha256), and passes a green control.
2026-09-12 13:42:27 +07:00
ltms 57cd96f5e6 Merge pull request 'fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts' (#540) from worker/537-capturedlog-close-e4c437-2 into main
CI / contract (push) Successful in 47s
CI / build (push) Successful in 2m9s
2026-09-12 08:42:19 +02:00
Dai Ha 202e37e3b3 fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts
CI / contract (pull_request) Successful in 1m2s
CI / build (pull_request) Successful in 1m36s
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
CI / contract (pull_request) Successful in 1m30s
CI / build (pull_request) Successful in 2m9s
2026-09-12 13:36:37 +07:00
ltms f1640f5dcc Merge pull request 'fleetd #535: convert FleetdLeadMailboxSelectionTest to CapturedLog' (#536) from worker/535-appender-leak-fe74c1-1 into main
CI / contract (push) Successful in 1m13s
CI / build (push) Successful in 1m28s
2026-09-12 08:25:01 +02:00
Dai Ha c7903c1efe fleetd #535: convert FleetdLeadMailboxSelectionTest to CapturedLog
CI / contract (pull_request) Successful in 53s
CI / build (pull_request) Successful in 1m52s
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
ltms 7d711942fe Merge #534: detect a died shutdown drain the ERROR count is blind to (fleetd #512 part 2)
CI / contract (push) Successful in 48s
CI / build (push) Successful in 1m52s
Verified independently. The branch is based on 8335b12 while main is at bec87f9, so I merged locally first and tested the MERGED tree, not the branch — a clean auto-merge is not a working merge.

Merged tree checks:
- `bash -n` exit 0 on both scripts, under /bin/bash 3.2.57 and env bash 5.3.9.
- Suite exit 0, 0 lines matching `^FAIL:`, 255 bytes of output. 60 test functions defined, 60 invoked, and no defined-but-never-invoked orphan (checked with a comm against the invocation list, not by comparing two counts — two equal counts can both be wrong).
- My own comment fix from bec87f9 survived the merge and is still at :317.

I ran two mutations the worker did not, per "mutate the half the worker did not":

(A) The one that matters, because it is the defect this ticket exists to prevent: collapsed the `unknown` state into `complete`, so "cannot tell" reports as a pass. Result exit 1, one FAIL: `cannot-tell fixture must set REDEPLOY_DRAIN_STATE=unknown: expected unknown, got complete`. So the third state is genuinely load-bearing, not decoration.

(B) Broke the positive check: changed `find_drain_complete_line`'s pattern from `drain complete: released=` to `drain finished: released=`, one site. Result exit 1, one FAIL: `find_drain_complete_line did not capture the present line`.

Proof that (B) applied, against a pristine copy: the full grep line 1 -> 0, the mutant form 0 -> 1, and the bare phrase 2 -> 1 with the comment occurrence untouched. Both files restored byte-identical; `git diff --quiet` clean; green control re-run.

A note on my own proof cell for (B), because it was wrong the first time. I wrote the counts with escaped double quotes inside an already double-quoted command substitution, so the shell split the pattern on spaces and grep treated the words as filenames. It printed "2 and 2" alongside `ugrep: No such file or directory` warnings — a symmetric, plausible-looking pair that meant nothing. The kill itself was never in doubt, since the suite named the exact function, but the cell that was supposed to prove the mutation applied proved nothing. Re-done with single quotes. This is the same trap already written down for this repo, hit by me, in a cell whose only purpose was to guard against exactly this.

One thing I checked that no test covers: the main flow's `HAD_OLD_PID=0; [ -n "$OLD_PID" ] && HAD_OLD_PID=1` runs under `set -euo pipefail`, and on a cold start the test fails. Sourcing stops before the main flow, so no behavioural test reaches that line. If `set -e` fired there, every cold start would abort before the health checks. It does not: `set -e` exempts the left side of an `&&` list, confirmed by running it under both shells — `survived, HAD_OLD_PID=0` on 3.2.57 and on 5.3.9. Safe, but it is untested main-flow wiring, which is the same class as #528's item 1 and belongs on that list.

On the `n/a` fourth state, which the worker flagged for a reviewer's judgment rather than quietly keeping: accepted, and it is in scope. The ticket asked for a third state because a sentinel conflating "no" with "cannot tell" hides two causes needing opposite handling. "The question does not apply" is a third such cause, not a variant of "cannot tell". Without it, the new warning would fire on every clean cold start, and a warning that cries wolf on the most common path trains the operator to skip it — which destroys the absence signal just as surely as putting the completion line in a `finally` would. The worker also proved the gate is actually consulted, using a cold-start fixture whose content deliberately looks like a died drain, so the test would fail if the gate were skipped. That is the right way to test a gate.

Both remaining outcomes are correctly excluded from the "no ERROR lines since restart" summary: only `complete` and `n/a` let it print.
2026-09-12 08:04:15 +02:00
Dai Ha bec87f987c scripts: name the mechanism in detect_supervisor's constraint 2, not a line number
CI / contract (push) Successful in 1m30s
CI / build (push) Successful in 1m33s
Constraint 2 read "This script runs under `set -euo pipefail` (line 50), so an
unset variable is a loud failure." Two problems, both small and both the same
family as fleetd #494 — a comment that states the wrong reason.

The line number was stale: the `set` line is at 54, not 50. It was the only
line-number citation in the file, and a citation like that goes stale on the
next insert above it, silently, with nothing to catch it.

The mechanism was also misattributed. What makes an unset variable a loud
failure is `set -u`. Naming the whole `-euo pipefail` string invites the reader
to credit pipefail for it, which is the mistake fleet01 flagged on a different
cell this week: pipefail is insurance against a future pipeline stage, not what
catches the current shape.

Now names `set -u` and says where it is without a number, and records why the
number is gone so nobody adds one back.

Comment only. bash -n exit 0 under /bin/bash 3.2.57 and env bash 5.3.9;
scripts/test-redeploy-fleetd.sh exit 0 with 0 lines matching ^FAIL:.
2026-09-12 12:58:37 +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
ltms af9589783e Merge #533: promote CapturedLog to a shared test helper and close the logger-level leak (fleetd #529)
CI / contract (push) Successful in 53s
CI / build (push) Successful in 1m35s
Verified independently in a scratch worktree at 8ea5c2b, not promoted from the worker's report.

Build: `mvn -f fleetd/pom.xml clean install` exit 0, `Tests run: 1701, Failures: 0, Errors: 0, Skipped: 0`, BUILD SUCCESS.

Base arithmetic, measured rather than carried forward: a6415f3 (this branch's parent) has 1695 `@Test` plus 2 parameterized/repeated, and the branch has 1696 plus 2 — a delta of exactly +1, matching the per-file count (WorktreeSessionManagerTest 24 -> 25). So 1700 -> 1701 is the one new proving test and nothing else. A number I had carried from earlier in the session said 1699; that number was wrong and is retired. No Java file differs between a6415f3 and main at 8335b12, so the base count is the same on both.

Mutation (a), the shared instrument: removed `logger.setLevel(originalLevel);` from `CapturedLog.close()`. Result exit 1, `Tests run: 25, Failures: 1`, the single failure being `sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn` with `expected: <TRACE> but was: <WARN>`. Restored byte-identical.

Mutation (b), the use site: replaced the try-with-resources in `releasePreservesDirtyWorktreeAndLogsWarn` with the pre-#525 hand-rolled `ListAppender` + `setLevel` + `finally detachAppender` pattern. Result exit 1, one failure, the same assertion. Restored byte-identical.

Green control on the restored tree: `git diff --quiet` clean, full suite exit 0, 1701/0/0/0.

Both mutants were killed, so per the economy fleet01 proposed and #529 adopted, neither needs a separate harness-proof cell — the kill is the proof the cell can go red.

Leak survey, re-measured here rather than taken from the report: on a6415f3, 9 files carry 19 `setLevel` pins on a raw logback `Logger` with zero restoring call; on the branch that set is empty. The 7 remaining `setLevel` calls in `GitWorktreesTest` are `reportingLog.setLevel(...)` on the `CapturedLog` instance, whose `close()` restores the original, so they are re-pins and not leaks.

File hashes match the worker's report exactly, head and tail: CapturedLog.java 486d6f5b5a30dc5ef7f75e5e10be353e720fb0de503825e88e8d96e30a61a2f7, WorktreeSessionManagerTest.java a722a98d828c82e00415d2a924d341e177ded704262aaa5963ea2d09a683df94.

Two things follow this merge rather than block it, both filed separately: one javadoc sentence in the new helper overstates its own reach, and the worker's item 4 reports a separate appender leak outside this ticket's scope.
2026-09-12 07:57:25 +02:00
Dai Ha 190436c9cf fleetd #512 part 2: detect a died shutdown drain the ERROR count is blind to
CI / contract (pull_request) Successful in 1m15s
CI / build (pull_request) Successful in 2m4s
The previous daemon's dead shutdown drain (an uncaught exception in a
shutdown thread) never passes through the logger, so it never carries an
ERROR/SEVERE token, so redeploy-fleetd.sh's existing ERROR-count classifier
is structurally blind to it and prints a confident "no ERROR lines since
restart" while the drain actually died.

Add scan_uncaught_exceptions (greps the shutdown window for the failure's
real shape: `Exception in thread`, `NoClassDefFoundError`) and
find_drain_complete_line (checks for #522's SessionManager.drainAll
completion line). Compose both in report_shutdown_drain, a single
decision+action function the main flow calls unconditionally (same shape
as swap_if_built/refuse_drain_gate from #521/#528), which resolves to one
of four outcomes: complete, died, unknown ("cannot tell" — the line is
absent for either of two reasons that need opposite handling: the previous
daemon predates #522, or its drain failed without throwing), or n/a (no
previous daemon was actually stopped this run). Never fails the redeploy;
warns loudly instead.

Gate the "no ERROR lines since restart" summary line on the new outcome so
it never reads as reassurance when the drain died or the outcome is
"cannot tell" (item 4 of the ticket).

Tests: 11 new test functions (60 defined/invoked, was 49), covering both
pure classifiers, all four report_shutdown_drain outcomes, a source-grep
proof of the main-flow call site (sourcing stops before the main flow
runs), an ordering check, and the item-4 gating. Full suite green
(exit 0, 0 anchored FAIL lines). Five mutations applied and killed by hand
during review, each restored to a byte-identical file afterward.
2026-09-12 12:55:54 +07:00
Dai Ha 8ea5c2bb1f fleetd #529: promote CapturedLog to a shared test helper, close the logger-level leak
CI / contract (pull_request) Successful in 1m14s
CI / build (pull_request) Successful in 1m39s
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 8335b12562 Merge #532: pin drain_gate_refusal's call site, not just the predicate (fleetd #528)
CI / contract (push) Successful in 53s
CI / build (push) Successful in 1m47s
Verified independently in my own worktree at the pushed head 7c34e8f, not taken
from the worker's report. CI run 1781: success.

The shape is the one #528 asked for and the one #526 arrived at: refuse_drain_gate
composes the message via drain_gate_refusal AND calls die itself, and the main
flow calls it unconditionally at :710. No guard is left in the main flow to
remove, invert or bypass on its own.

My measurements:

  test functions defined / invoked   49 / 49   (was 44/44; +5)
  bash -n, /bin/bash 3.2.57          rc=0 on both files
  bash -n, env bash 5.3.9            rc=0 on both files
  clean control                      exit 0, 0 lines matching ^FAIL:, 256 bytes

Two mutations, both killed, each by a differently named failure:

  call site deleted (the item-1 mutation: :710 replaced by a flat
  die "aborted — nothing changed")
    -> exit 1, 1 ^FAIL: line, 87 bytes
       FAIL: could not find the main flow's refuse_drain_gate call site in redeploy-fleetd.sh

  refuse_drain_gate stops consulting the predicate (its body's
  die "$(drain_gate_refusal ...)" replaced by a flat message)
    -> exit 1, 1 ^FAIL: line
       FAIL: refuse_drain_gate build-ran+staged-present die message does not name the staged jar

The first cell is the point of the ticket. Before this change the same mutation
gave exit 0, zero FAIL lines and output byte-identical to a clean run at 256
bytes. It now exits 1 and names the missing call site. Each mutation was proven
applied with a uniquely tagged marker plus a second, different search string,
with a control against a pristine copy showing the exact inverse (1/0 mutated,
0/1 pristine), and the function definition confirmed still present so the
mutation targeted the call and not the function. Restored byte-identical to
0e5a99a22c9c65f72960d8f179ca5299307889e06bc42131f098a513e7b97bd6 and the final
control is green.

Needle uniqueness checked, because this is where it could have gone wrong:
grep -cF 'refuse_drain_gate "$DO_BUILD" "$JAR_STAGED"' on the production script
returns 1, at :710, the real call site. The worker hit the self-match trap while
writing the comment above refuse_drain_gate — their first draft quoted the
call-site string literally, which would have let the source-text test match the
comment instead of the call — caught it themselves, and reworded so the comment
cannot become a second match. That is the same trap that cost me a false pass on
a probe earlier today, and catching it unprompted is the better half of this PR.

The dead-check sweep came back as a real negative, with the reasoning shown
rather than asserted: of the five scripts under set -e with pipefail, every
pipe-into-assignment already carries || true or || echo, and the remaining two
scripts have no pipe-into-assignment at all. probe-member-credentials.sh and
deploy/herdr-inner.sh correctly excluded for not having set -e. No live
instances.

One inaccuracy in the report, in the report only: it abbreviates the restored
hash as "0e5a99a2...78f0a", and that tail does not occur in the actual hash,
which ends b97bd6. I hashed the committed file myself and confirmed the restore
matched, so the file is right and only the quoted abbreviation is wrong. Flagged
because an abbreviated hash that nobody can match against anything is worse than
no hash.

wait_for_daemon_exit's call site (item 2) stays open as the ticket scoped it —
source-text pinned only, "partially pinned, not audited". The seven untested
main-flow decisions are untouched; the worker correctly notes it changed the body
of one of those if blocks while leaving the guard condition itself untested, as
instructed.
2026-09-12 07:41:57 +02:00
ltms d25c863118 Merge #531: separate the blocked forge MCP server from the working GITEA_TOKEN
CI / build (push) Successful in 1m30s
CI / contract (push) Successful in 1m32s
Charter wording only — 9 insertions, 4 deletions, one file. CI run 1780 on
be07ed2: success.

Resolves the "charter may be stale" item I had been carrying. It was not stale.
Two workers reporting working forge access and the charter saying forge tools
hold a blocked credential were both correct, about two different credentials:
the repo-scoped GITEA_TOKEN the daemon injects (which opens every worker PR,
per implementer SKILL.md step 5) versus the forge MCP server that leaks in from
the operator's user-scope config (which is deliberately blocked). The wording
did not separate them, and a worker could have read it as "I cannot reach the
forge" and skipped opening its PR.

Both sentences now name the MCP server specifically and state that the injected
token is a separate, working route.

Canonical block and wiki template verified byte-identical after the edit — the
CLAUDE.md sync script reports "in sync: True". The wiki commit is d02a55d on
wiki's own main, pushed and verified by ref (ls-remote matched local HEAD), not
by exit code. The submodule pointer stayed unstaged.

Not re-measured in this change: that the blocked MCP credential does fail every
call. That claim is the existing charter's and I only narrowed what it refers
to. It would need its own probe with a request that cannot succeed on its
merits, so that a rejection can only mean the block.
2026-09-12 07:41:11 +02:00
Dai Ha 7c34e8f4f9 fleetd #528: pin drain_gate_refusal's call site, not just the predicate
CI / contract (pull_request) Successful in 53s
CI / build (pull_request) Successful in 1m31s
drain_gate_refusal composes the correct abort message and is well tested,
but the main flow built its own `die "$(drain_gate_refusal ...)"` call —
nothing proved that call site was ever consulted. Mutating it to a flat
`die "aborted -- nothing changed"` left the whole suite green, silently
reinstating the exact defect #517 was filed to fix.

Same shape as #521/#526's should_swap/swap_if_built: the decision and the
die() now live together in refuse_drain_gate, which the main flow calls
unconditionally. drain_gate_refusal stays separate and separately tested
for the message logic; four new behavioural tests stub die() to prove
refuse_drain_gate calls it correctly for all four cases, and a fifth
source-text test pins the main flow's call site itself (the only thing
that can catch deleting the call, since sourcing stops before the main
flow runs).
2026-09-12 12:38:16 +07:00