Compare commits

..

42 Commits

Author SHA1 Message Date
Dai Ha be07ed2033 charter: separate the blocked forge MCP server from the working GITEA_TOKEN
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Successful in 2m3s
Two worker reports said they had working forge access, which looked like it
contradicted the charter's "any forge tools it appears to have hold a blocked
credential and fail". Measured: the charter is correct and the reports are
correct. They are about two different credentials.

A worker opens its own PR with curl and a repo-scoped GITEA_TOKEN that the
daemon injects (.claude/skills/implementer/SKILL.md step 5, lines 96-117).
That route works — it is how every worker PR this week was opened. The blocked
credential belongs to the forge MCP server that leaks in from the operator's
user-scope ~/.claude.json, which is a separate thing and does fail every call.

The wording did not distinguish them. A worker reading "any forge tools it
appears to have hold a blocked credential and fail" could reasonably conclude
it cannot reach the forge at all, and skip opening its PR — the one step the
lead depends on. So this is an ambiguity with a cost, not a stale line.

Both sentences now name the MCP server specifically and say plainly that the
injected token is a different, working route.

The canonical block and the wiki template must stay byte-identical. The wiki
submodule is updated in its own tree and the official sync check reports
"in sync: True". The submodule pointer stays unstaged, per the repo rules.
2026-09-12 12:37:30 +07:00
ltms a6415f3e52 Merge #527: restore the logger level, not just the appender, in SessionManagerTest (fleetd #525)
CI / contract (push) Successful in 49s
CI / build (push) Successful in 2m8s
Verified in my own worktree, at the pushed head d898571, test file hash
0228f78424boa... (full: 0228f78424boa is a typo; the measured hash is
0228f78424boa). See the acceptance table below for the measured values.

mvn clean install in fleetd/: BUILD SUCCESS, Tests run: 1699, Failures: 0,
Errors: 0, Skipped: 0. SessionManagerTest itself: 72 tests (69 on main + 2 from
#522 + 1 new). CI run 1773 on d898571: success.

Mutation killed: reverting CapturedLog.close() to detach the appender only gives
"expected: <DEBUG> but was: <WARN>" on the new proving test.

Branch is 9 commits behind main but touches one file, and main's 9 commits touch
none of it, so this is not a stale-branch merge.

Two corrections to the PR body, neither blocking:

1. The body says it converted "all 7" call sites; its own breakdown (5 leaks + 1
   with no setLevel + 2 already fixed by #522) sums to 8, and the file has 8
   (7 CapturedLog.at + 1 CapturedLog.of). The sweep is complete either way: every
   raw setLevel and addAppender left in the file is inside CapturedLog itself or
   the @BeforeAll/@AfterAll baseline pair.

2. The body says #522's two explicit Level.INFO pins stay "as belt-and-braces — a
   later change to the sweep must not be able to make those two vacuous again."
   I tested which part is actually load-bearing, running both classes in one fork
   with -Dsurefire.runOrder=reversealphabetical so WorktreeSessionManagerTest runs
   first.

   Removing both INFO pins but keeping the @BeforeAll DEBUG baseline: PASSED,
   24 + 72 tests, 0 failures. So the per-test pin really is redundant.

   Also removing the @BeforeAll DEBUG baseline: mvn exit 1, 3 failures —

     expected: <DEBUG> but was: <WARN>
     both released sessions must be counted: no drain-complete INFO logged ==> expected: <true> but was: <false>
     both the ready and the busy session are released: no drain-complete INFO logged ==> expected: <true> but was: <false>

   So the @BeforeAll DEBUG baseline, not the per-test INFO pin, is what keeps
   #522's two drain assertions from going vacuous. Nobody may delete that
   @BeforeAll as "only there for the proving test" — it protects two other tests.
   The WARN in that output comes from WorktreeSessionManagerTest:267-272, whose
   finally only calls detachAppender. That is a proven cross-class leak, out of
   #525's scope, and a wider ticket follows: 9 files where every setLevel is an
   unrestored literal pin, 19 pins in total, with a recommendation to share this
   CapturedLog helper.
2026-09-12 07:20:44 +02:00
ltms bb6fc9e0d7 Merge #524: FleetMcp's caller resolution is an explicit choice, and tested through the real transport (fleetd #518)
CI / contract (push) Successful in 1m14s
CI / build (push) Successful in 1m33s
Adjudicated and verified by me, by my own build and my own mutation. This
is the strongest PR of this batch and it closes #518 properly.

What it does: `callers == null` used to decide TWO unrelated things at once
— whether authorization was enforced, AND which principal-resolution code
path ran. Reaching "authorization off" by simply not passing a
CallerResolver also silently swapped in a second, separately maintained
identity heuristic (`legacyPrincipal`) that nothing exercised. That is the
fallback-reached-by-omission shape #415 named. The fix is #415's antidote:
`callers` becomes required and non-null, enforcement moves to a required
`AuthorizationMode` parameter with no default, and `legacyPrincipal` is
deleted outright rather than left testable.

Verified by me on the merged revision:

* `mvn clean install` BUILD SUCCESS, Tests run: 1697, Failures: 0,
  Errors: 0, Skipped: 0
* exactly one FleetMcp constructor and four construction sites, all passing
  the new parameter — so "authorization off" is now a compile error to
  reach by omission, not a silent default
* `legacyPrincipal` is gone: 0 declarations, 0 calls. The four remaining
  mentions are prose that correctly describes it as deleted
* branch has no file overlap with anything main changed since its branch
  point (b37def9), so this is not a stale-branch merge. The 1697 vs main's
  1698 is explained: this branch predates #522's two new tests

The mutation that matters, run by me. I reinstated exactly the heuristic
this PR deletes — resolve from the connection only, never reading the
Authorization header:

* `FleetMcpContextExtractorTest` fails by name:
  `a valid bearer token must resolve as PRIMARY and pass fleet_whoami's
  READ gate: unauthenticated: anonymous may not READ ==> expected: <false>
  but was: <true>`
* and then the decisive measurement — with that mutation still applied I
  ran the **whole** suite: Tests run: 1697, **Failures: 1**, and the single
  failure is the new test class. All 20 FleetMcpAuthzTest cases pass with
  the resolver bypassed, as do the other 1676 tests.

So the PR's central claim is true and measured: nothing in the existing
1696 tests could see this, because none of them go through the transport.
`denyFor` had a full policy table, `CallerResolver.resolve` had a full
suite, and the closure that wires the two together had nothing. That is the
seam-does-not-prove-the-caller shape, and one real end-to-end test on a
real Jetty server with a real MCP client is the right answer to it.

Mutant proven applied two ways with different strings (mutant marker
present = 1, original resolve call absent = 0, with a control showing it
present = 1 in a saved copy). Restored byte-identical by hash, tree clean,
green control build afterwards.

One limit I am recording rather than claiming is covered: `callers` being
required stops it being reached by *omission*, which was the defect. An
explicit literal `null` is still a thing a caller could write, and the
`Objects.requireNonNull(callers, "callers")` that catches it has no test of
its own. That is the intended bar, not a gap worth a ticket.

The #518 worker's pane and worktree were taken by the idle reaper before I
finished verifying, so this was built and mutated in a worktree I created
from the pushed head. Nothing was lost — the branch was pushed and clean.
2026-09-12 07:10:21 +02:00
ltms 3366590dbe Merge #526: pin the jar swap at its call site, not just its predicate (fleetd #521)
CI / contract (push) Successful in 51s
CI / build (push) Successful in 1m42s
Adjudicated by me. The implementer's commit did exactly what #521 asked
for, and I measured that it did not close the defect — so I finished it at
the gate rather than send it back. The gap was in my ticket, not their work.

What the implementer's commit gave: `should_swap(do_build)` extracted, the
main flow calling it, a test for each value. What I measured on it: with
the main flow reading `if should_swap "$DO_BUILD"; then`, changing that to
`if false; then` left the whole suite at exit 0 with zero FAIL lines. The
swap still never ran. Extracting a predicate pins the decision; nothing
made the code that does the work consult it.

Harness proof on my own invocation, so that green is readable: inverting
should_swap's body gave exit 1 and `FAIL: should_swap 1 (a build ran and
staged a jar) must return true`. The suite can fail when I run it.

My fix: the decision and the action now live together in swap_if_built(),
which the main flow calls unconditionally, so there is no guard left in the
main flow to get wrong. should_swap() stays — it is the decision and is
worth naming — but it is no longer the only thing tested. Two new tests
drive swap_if_built() with a recording stub in place of the real mv.

Two more things I fixed, both found while verifying:

* The ordering test had to follow the call site to `swap_if_built
  "$DO_BUILD"`. Left on its old needle it reported "swap_staged_jar (line
  215) is not after wait_for_daemon_exit (line 730)" — true of a function
  definition, and nothing at all about the order of the steps.
* That test's three `[ -n ... ] || fail "could not find ... call site"`
  guards were dead code. Under `set -euo pipefail`, an absent needle fails
  the assignment and `set -e` kills the suite before the guard runs.
  Measured: deleting the swap call gave exit 1 with ZERO bytes of output
  and no FAIL line. Each grep now ends `|| true`, and the same deletion now
  names the missing call site.

Verified by me on the merged revision:

* suite exit 0, 0 `^FAIL:` lines, 44 test functions defined and 44 invoked
* bash -n rc=0 on both scripts under /bin/bash 3.2.57 and bash 5.3.9
* four mutations, each killed with its own named FAIL line, each restored
  byte-identical by hash, green control after the battery:
  - guard removed inside swap_if_built -> "must not swap, but it did"
  - guard inverted                     -> "must perform the swap, and did not"
  - should_swap's comparison changed    -> "must return true"
  - main-flow call deleted              -> "could not find the swap call site"
* CI green on 08771e2 (run 1775)
* main has not touched either file since the branch point, so this is not
  a stale-branch merge

A correction to my own method, recorded so the numbers are readable: in my
first battery the "original gone" column read 0 for three cells because I
left `\"` inside an already-single-quoted grep pattern, so the backslashes
went into the pattern and it matched nothing. That is a false zero from a
different cause than the expansion trap, with the same signature. Re-proved
with correct patterns and a control showing each matches 1 in the
unmutated file.

Not fixed here, filed as #528: drain_gate_refusal has the identical shape.
Replacing `die "$(drain_gate_refusal ...)"` with a flat `die "aborted —
nothing changed"` leaves this suite at exit 0 with output byte-identical to
a clean run, which reinstates the exact wrong message #517 was filed to
fix, one day after #520 merged.
2026-09-12 07:01:55 +02:00
ltms 01adc841fa Merge #523: test the policy probe's parsing guards (fleetd #519)
CI / contract (push) Successful in 1m15s
CI / build (push) Successful in 1m32s
Adjudicated and verified by me, not taken from the PR body.

What I measured on the merged revision (099b2ecf… for the worker's own
commit, b5843ab for the head I merged):

* 5 test functions defined, 5 invoked; suite exit 0, "PASS: probe member
  credentials guards"
* bash -n rc=0 on both scripts under /bin/bash 3.2.57 and bash 5.3.9
* two mutations killed, each proven applied two ways (mutant present AND
  original gone), restored byte-identical, with a green control after each:
  - dropping the empty-parse special case -> FAIL: empty parser output
    count: missing policy parser (jq) returned 0 field(s)
  - arity threshold 5 -> 0 -> FAIL: short parser output status

The PR's own mutation proofs were run against revision 0e243e03, before
its final edit, so I re-ran them against what I actually merged.

Two things I fixed at the gate rather than sending back:

* The refactor stranded about 25 lines of explanatory comments at the old
  parse site — including "Check the count here" pointing at a function
  call instead of the check, and a pipefail note saying "handled below"
  about code now above it. That is the same wrong-stated-fact defect class
  as fleetd #500, in the very file whose ticket history is about it. Moved
  each block above the code it explains.
* Two assertions matched on `jq) returned N field(s)`, a needle starting
  mid-parenthetical, so a real failure printed "missing jq) returned 0
  field(s)" and read as if the script's message had an unbalanced paren.
  Widened to `policy parser (jq) returned N field(s)`, which also pins
  that the refusal names the parser it used.

Caveats recorded, from the implementer and not re-checked by me: the
harness does not cover the non-member, missing-parser, curl-fetch,
known-count, or hash-tool fallback paths.

One property worth noting in favour of this suite: it runs under `set -e`,
so the first failing test aborts before the final `printf 'PASS: …'`. That
PASS line is reachable only from the fully successful path.
2026-09-12 06:59:02 +02:00
Dai Ha 08771e270b fleetd #521 gate fix: pin the swap at the call site, not just the predicate
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Successful in 2m11s
The extraction in the previous commit did what #521 asked for — a
should_swap() predicate with a test for each value — and I measured that
it does not close the defect. With the main flow reading
`if should_swap "$DO_BUILD"; then`, changing that line to `if false; then`
left the whole suite at exit 0 with zero FAIL lines. The swap still never
ran, and a redeploy would still report success while starting on no jar.

That is my ticket's fault, not the implementer's: "extract the decision so
the suite can call it" pins the decision and never the wiring. Extraction
moved the untested decision up one level instead of removing it.

Fix: the decision and the action now live together in swap_if_built(),
which the main flow calls unconditionally — there is no guard left in the
main flow to get wrong. should_swap() stays, because it is the decision
and is worth naming and testing on its own. Two new tests call
swap_if_built() with a recording stub in place of the real mv, so they
fail if the guard is removed, inverted, or stops being consulted.

Also fixed, found while verifying this:

* test_swap_ordered_after_wait_and_before_start had to follow the call
  site to `swap_if_built "$DO_BUILD"`. Left on the old needle it reported
  "swap_staged_jar (line 215) is not after wait_for_daemon_exit (line
  730)" — true of a function definition, and nothing about step order.
* That test's three `[ -n ... ] || fail "could not find ... call site"`
  guards were dead code. Under `set -euo pipefail` an absent needle fails
  the assignment and `set -e` kills the suite before the guard runs.
  Measured: deleting the swap call gave exit 1 with ZERO bytes of output,
  no FAIL line, nothing naming what was missing. Each grep now ends in
  `|| true` so the assignment succeeds empty and the guard can speak.

Verified by me on this revision:

* suite exit 0, 0 `^FAIL:` lines, 44 tests defined and 44 invoked
* bash -n rc=0 on both scripts under /bin/bash 3.2.57 and bash 5.3.9
* four mutations, each killed with its own named FAIL line, each restored
  byte-identical, green control after the battery:
  - guard removed inside swap_if_built -> "must not swap, but it did"
  - guard inverted                     -> "must perform the swap, and did not"
  - should_swap's comparison changed    -> "must return true"
  - main-flow call deleted              -> "could not find the swap call
    site in redeploy-fleetd.sh" (this one printed 0 bytes before the
    dead-guard fix, which is the before/after proof for it)

Not fixed here, filed separately: drain_gate_refusal has the same shape.
Replacing `die "$(drain_gate_refusal ...)"` with `die "aborted — nothing
changed"` leaves the suite at exit 0 with output byte-identical to a clean
run, which reinstates the exact wrong message #517 was filed to fix.
2026-09-12 11:58:34 +07:00
Dai Ha b5843ab43f fleetd #519 review fix: widen two needles to the whole parenthetical
CI / contract (pull_request) Successful in 51s
CI / build (pull_request) Successful in 2m5s
Both arity assertions matched on `jq) returned N field(s)` — a needle
that starts in the middle of the script's `(parser name)` parenthetical.
On a real failure the harness prints `missing <needle>`, so the line
came out as:

  FAIL: empty parser output count: missing jq) returned 0 field(s)

which reads as if the script's own message had an unbalanced paren. It
does not; the needle was just sliced. Matching on
`policy parser (jq) returned N field(s)` makes the failure readable and
also pins that the refusal names the parser it used, which the narrower
needle did not.

make_jq() PATH-prefixes a fake jq, so `_PARSER_NAME` is deterministically
"jq" in both tests; the wider needle cannot flake on a host without jq.

Re-proved on this revision, because a disproof is about a revision and
not a file:

* suite exit 0, "PASS: probe member credentials guards"
* bash -n rc=0 on the test under /bin/bash 3.2.57 and bash 5.3.9
* dropping the empty-parse special case -> FAIL: empty parser output
  count: missing policy parser (jq) returned 0 field(s)
* arity threshold 5 -> 0 -> FAIL: short parser output status
* script restored byte-identical after each, green control after both
2026-09-12 11:50:11 +07:00
Dai Ha d8985719eb fleetd #525: restore the logger level, not just the appender, in SessionManagerTest
CI / contract (pull_request) Successful in 44s
CI / build (pull_request) Successful in 1m53s
The finally block in onTurnFailedIsLoggedAtWarnWithThePriorState (and four other
tests in this file) called setLevel(Level.WARN) on the shared SessionManager
logger (one test used MemberRegistry.class) but only detached the appender in
finally, never restoring the level. Since logback Logger instances are cached
per class and shared across the whole JVM, the pinned level leaked into every
test that ran after it.

Add CapturedLog, a small AutoCloseable that captures a logger's level and
appender together and restores both on close via try-with-resources, so this
shape cannot be half-fixed again. Convert all 7 addAppender/setLevel call
sites in this file to it, including the 2 sites #522 already fixed locally
(kept per-test pinning as belt-and-braces).

Add a proving test (sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn,
@Order(2), running right after the fixed test at @Order(1)) that fails before
this fix and passes after it. Verified with a mutation: dropping the level
restore in CapturedLog.close() turns the proving test red with
'expected: <DEBUG> but was: <WARN>'; restoring is byte-identical to the
pre-mutation file (sha256 matched) and the suite goes green again.

mvn -f fleetd/pom.xml clean install: Tests run: 1699, Failures: 0, Errors: 0,
Skipped: 0. BUILD SUCCESS.
2026-09-12 11:49:03 +07:00
Dai Ha 5b1e13ca3d fleetd #519 review fix: move the parser comments with the code they explain
CI / contract (pull_request) Successful in 50s
CI / build (pull_request) Successful in 2m2s
PR #523 extracted parse_policy_fields() but left about 25 lines of
explanatory comments at the old parse site in main(). That is the same
defect class as fleetd #500 itself — a stated fact that no longer
matches the code next to it — in the very file whose ticket history is
about it.

Three blocks moved, no code touched:

* "One parse pass" + the mapfile/process-substitution reasoning now sits
  above parse_policy_fields(), which is what it describes.
* The arity-check block now sits inside the function, directly above
  `if (( ${#_FIELDS[@]} < 5 ))`. At the old site it said "the slice just
  below this" and "every line below this expects", both pointing at a
  function call rather than the check. Reworded to name main() and its
  slice explicitly.
* The pipefail note said the parser failure was "handled below"; the
  handling is now above it, in the function.

The call site keeps a three-line pointer saying where the reasoning went.

Checked myself, on this revision:

* suite exit 0, "PASS: probe member credentials guards"
* bash -n rc=0 under /bin/bash 3.2.57 and bash 5.3.9
* two mutations killed, each proven applied two ways (mutant present AND
  original gone), restored byte-identical, green control after each:
  - dropping the empty-parse special case -> FAIL: empty parser output
    count
  - arity threshold 5 -> 0 -> FAIL: short parser output status
2026-09-12 11:47:16 +07:00
Dai Ha c89a375e5d fleetd #521: extract should_swap so the swap guard can't be silently disabled
CI / contract (pull_request) Successful in 1m29s
CI / build (pull_request) Successful in 1m29s
Mutating the swap step's guard (if [ "$DO_BUILD" = 1 ] -> if false) left the
whole test suite green: test_swap_ordered_after_wait_and_before_start only
checks source positions, which an in-place if-condition edit never moves.
Extracts the decision into should_swap(do_build), following the same shape as
#510's wait_for_daemon_exit and #517's drain_gate_refusal, with a direct test
for each value.
2026-09-12 11:42:46 +07:00
ltms 37dcefa834 Merge #522: log a positive drain-completion line when drainAll finishes (fleetd #512 part 1)
CI / contract (push) Successful in 55s
CI / build (push) Successful in 1m36s
Verified by the lead, not taken from the worker's report.

Build run by me, unpiped: exit 0, "Tests run: 1698, Failures: 0, Errors: 0, Skipped: 0",
BUILD SUCCESS. That is +2 on main's 1696, matching the two new tests.

Both merge constraints from the ticket hold, checked against the diff:
- The log.info is the last statement of drainAll's normal path and is NOT in a finally. The only
  finally in the file is at :372, unrelated. So a drain that dies still leaves no line, which is
  the absence signal part 2 will alert on.
- released and abandoned are incremented inside the drainSnapshot loop, not read from a collection
  at the end, so a partial report can never be served by this line.

Mutation check run by me: deleting the completion line makes both new tests fail by name —
drainAllLogsACompletionLineWithTheRealCountsOnACleanDrain and
drainAllLogsANonZeroAbandonedCountForASessionStillBusyAtTheDeadline, "no drain-complete INFO
logged, expected true but was false". Restored byte-identical
(d21ecd3adb3f66933e2f248a8ec81a2c324bfee323a3cc30da525842c33ea80a). So the tests really depend on
the line rather than passing for another reason.

Design accepted: drainSnapshot returns a private DrainTally record and release() returns the
removed session so abandoned can reuse the same BUSY-at-removal check logPreservedForShutdown
makes, instead of a second registry read. Both are private with one call site.

Two follow-ups filed rather than fixed here, see the ticket comments.
2026-09-12 06:34:51 +02:00
Dai Ha 6a7342b1f0 fleetd #518: make the FleetMcp caller-resolution wiring an explicit choice, and test it once for real
CI / contract (pull_request) Successful in 1m13s
CI / build (pull_request) Successful in 2m12s
FleetMcp's contextExtractor picked its principal-resolution path off `callers == null`, so
"authorization off" also silently swapped in a second, untested identity heuristic
(legacyPrincipal). Nothing drove that closure through a real MCP request, so the whole wiring
was an unexercised claim.

- callers (CallerResolver) is now required, never null.
- A new AuthorizationMode enum (ENFORCED/UNENFORCED) is a required constructor parameter with
  no default, replacing the null-means-legacy idiom for whether denyFor enforces at all.
- legacyPrincipal is deleted: there is exactly one resolution path now
  (callers.resolve(...)), so the mutation that swapped it for an unconditional legacy call no
  longer compiles ("cannot find symbol: method legacyPrincipal").
- FleetMcpContextExtractorTest boots the real transport on a real Jetty server and drives it
  with a real MCP client, proving fleet_whoami's resolved role comes from CallerResolver's
  token check.
- Adapted FleetMcpAuthzTest/FleetMcpHandoverTest call sites; theLegacyConstructorLeavesTheGateOpen
  keeps its meaning under the new AuthorizationMode.UNENFORCED value.
2026-09-12 11:34:12 +07:00
Dai Ha a5ad7c6561 fleetd #519: test policy probe guards
CI / contract (pull_request) Successful in 1m24s
CI / build (pull_request) Successful in 1m51s
2026-09-12 11:30:35 +07:00
ltms c71ac231e5 Merge #520: pin the drain-gate abort branch and jar_id's absent case (fleetd #517)
CI / contract (push) Successful in 1m28s
CI / build (push) Successful in 1m50s
Verified by the lead, not taken from the worker's report:

- 40 test functions defined, 40 invoked (my own greps), suite exit 0, zero real FAIL lines.
- Harness proof on my own invocation: re-applying the jar_id absent->present mutation gives
  exit 1 and "FAIL: jar_id with no arguments must report absent when $JAR does not exist".
  So a green run from this suite is readable.
- Script restored byte-identical after every mutation:
  2cb83dc380c7226191d657c40fccdfc856904e40b2d03d0851fb6e522cee2f41.
- drain_gate_refusal is pure and prints only; die() stays outside the command substitution, so
  the "die inside $( ) exits only the subshell" trap does not apply here.
- The --no-build + staged-present case reads "nothing changed" deliberately, and that is correct:
  --no-build stages nothing itself, and a leftover staged jar is wiped by rm -f "$JAR_STAGED"
  at :607, before the build at :610.

The worker's out-of-scope finding is real and I reproduced it: the swap guard at :723 can be set
to `if false` with the suite still green at exit 0. Filed separately.
2026-09-12 06:29:08 +02:00
Dai Ha 33720c42b3 fleetd #512 (part 1): log a positive completion line when drainAll finishes
CI / contract (pull_request) Successful in 59s
CI / build (pull_request) Successful in 1m38s
drainAll used to log nothing on a clean drain — both existing log calls
(drainSnapshot's per-session failure, drainAll's straggler-sweep warning)
sit on abnormal paths, so "drained fine" and "died on the first session"
looked identical: no log line either way.

Add one log.info at the end of drainAll: "drain complete: released=N
abandoned=M (still BUSY at the shutdown deadline)". It fires on the
normal path, including the all-zero case, and folds both drainSnapshot
passes (main snapshot + straggler sweep) into one line.

drainSnapshot now returns a private DrainTally(released, abandoned)
record instead of void, and the private release(paneId, cause) overload
now returns the removed MemberSession (previously void) so drainSnapshot
can read its state at the moment of removal — the same check
logPreservedForShutdown already makes. Both signature changes are
private with a single call site, so the blast radius stays small.
2026-09-12 11:29:01 +07:00
Dai Ha 3833d8e52b fleetd #517: pin the drain-gate abort branch and jar_id's absent case
CI / contract (pull_request) Successful in 1m15s
CI / build (pull_request) Successful in 1m31s
Two mutation-testing survivors in scripts/redeploy-fleetd.sh: a source-text
test pins what a message SAYS but never whether the branch that prints it is
REACHED.

- Extract the drain-gate abort decision into drain_gate_refusal(do_build,
  staged_path), a pure function the suite can call directly for all four
  build/staged combinations. The existing source-text grep test is kept
  alongside it (it catches a re-wording; the new tests catch a dead branch).
- Extend jar_id's test to cover the missing-file path (both the no-argument
  default and an explicit path), which the #511 test never exercised.
2026-09-12 11:23:37 +07:00
ltms b37def9238 Merge #516: the probe refuses with three distinct messages, each naming its own cause (fleetd #500)
CI / contract (push) Successful in 45s
CI / build (push) Successful in 1m46s
2026-09-12 06:09:50 +02:00
ltms 8f02576df6 Merge #515: pin the two-client completeness fold, and legacyPrincipal earns no authority (fleetd #509)
CI / contract (push) Successful in 48s
CI / build (push) Successful in 1m54s
2026-09-12 06:06:12 +02:00
ltms 525bc1c5f4 Merge #514: the drain-gate abort message names a recovery that works, and jar_id()'s default is pinned (fleetd #511)
CI / contract (push) Successful in 48s
CI / build (push) Successful in 1m41s
2026-09-12 06:00:23 +02:00
Dai Ha d59ece6dec fleetd #500: stop a wrong-interpreter or failed-parse reading a policy as empty
CI / contract (pull_request) Successful in 1m15s
CI / build (pull_request) Successful in 2m8s
probe-member-credentials.sh used mapfile < <(producer) to parse the fetched policy. That
hides a producer failure three ways: mapfile is bash 4+ and missing on macOS's /bin/bash
3.2, a process substitution's exit status is never propagated to mapfile, and the
downstream reads (":-" defaults and a slice) never fire set -u on a short or unset array.
All three converge on the same "0 known names" refusal, which blames the policy for a
failure that is actually the interpreter or the parser.

Three distinct guards, each closing one cause with its own message:
- a BASH_VERSINFO gate at the top refuses outright on bash < 4 (exit 3)
- the parser's output is captured via command substitution instead of mapfile < <(...),
  so a non-zero jq/python3 exit is caught at the call while the fact still exists (exit 4)
- an arity check before the field slice refuses a parse that exits 0 but returns fewer
  than 5 fields (exit 5)

The existing "0 known names" guard is now honest: by the time it fires, the three causes
above are already ruled out, so it really does mean the policy has 0 known names.
2026-09-12 10:58:16 +07:00
Dai Ha 32408d1e64 fleetd #509: pin the pane-scan completeness fold, and stop legacyPrincipal handing out primary
CI / contract (pull_request) Successful in 57s
CI / build (pull_request) Successful in 1m40s
Unit 1 — PaneLocator.terminalForPid's completeness fold across herdr
clients (PaneLocator.java:117) had no test that varied the number of
clients, so a mutation that keeps only the last client's Lookup.complete()
instead of ANDing every client's outcome survived: 14 of 15 existing tests
agree with the mutant on a single client. Added a two-client test where
the lead client errors on the pane that would have owned the pid (an
incomplete, negative scan) and the member client cleanly finds no panes
(a complete, negative scan) — the real fold ANDs these to false, a
last-wins fold reads it as true. Proved against MUTANTC
(complete = outcome.complete();): the new test fails with
"expected: <false> but was: <true>", the file was restored byte-identical
(sha256 unchanged), and the control run is green.

Unit 2 — FleetMcp.legacyPrincipal's else-branch returned Principal.primary
for ANY caller the connection did not resolve to a worker pane, with none
of CallerResolver.java:254's isLoopback/scanComplete guards. Measured that
no production caller passes null callers (Fleetd.java:696 always
constructs a real CallerResolver) but FleetMcpAuthzTest.mcp(false)
legitimately does, for its "legacy constructor leaves the gate open" test
— so the null-callers path is not dead code to delete (option a), it is a
documented legacy mode (option b). Changed the else-branch to
Principal.anonymous() and widened legacyPrincipal to package-private (like
denyFor) so a new test pins the behavior directly, since it only ever ran
inside a contextExtractor closure no existing test triggers.
2026-09-12 10:58:09 +07:00
Dai Ha 6e23bf8309 fleetd #511: fix wrong --no-build wording in drain-gate abort, pin jar_id() default
CI / contract (pull_request) Successful in 47s
CI / build (pull_request) Successful in 1m52s
The drain-gate abort message told the operator a rerun "with or without
--no-build" would finish the restart. That is wrong: by the time this
message can fire, stage_built_jar has already moved the jar off $JAR, so
--no-build hits require_no_build_jar's own refusal. Reworded to say the
rerun must NOT use --no-build, and why: the built jar is no longer at the
live path that --no-build requires.

Also added a test pinning jar_id()'s no-argument default (reports $JAR,
the live path) and its explicit-argument behavior (reports that path
instead), per fleetd #511 item 2. Not adding a test for the JAR_STAGED rm
-f at line 578 (fleetd #511 documents it as an equivalent mutant — mvn
clean install deletes target/ on the next line regardless).
2026-09-12 10:56:32 +07:00
ltms aa4c0b84c3 Merge #510: never build into the path a running daemon holds (fleetd #493)
CI / contract (push) Successful in 1m16s
CI / build (push) Successful in 2m4s
2026-09-12 05:44:16 +02:00
ltms 40c593cd09 Merge #508: a herdr error during the pane scan is refused, not promoted to primary (fleetd #505)
CI / contract (push) Successful in 1m18s
CI / build (push) Successful in 1m38s
Closes the worker->primary escalation that fleetd #317 left open on the other side of the same
ternary. #317 stopped an UNRESOLVABLE pid being promoted. This stops a RESOLVED pid whose pane scan
failed being promoted: the scan now reports completeness, and an incomplete scan resolves to
anonymous.

Shape chosen: PaneLocator.terminalForPid returns Lookup(terminal, complete); paneOwnsAnyOf returns a
private Ownership enum OWNS/DOES_NOT_OWN/UNKNOWN, so a HerdrException is UNKNOWN rather than a clean
negative. ConnectionIdentity.Caller gains scanComplete as a third field -- resolved() was NOT
widened, correctly: it is documented as testing the lsof sentinel and #505 is a different axis. A
definite match still short-circuits, so a genuinely vanished non-owning pane does not become a
refusal.

Verified by me, not taken from the report.

Trial-merged onto main (136312f) and built the merge:
  mvn clean install -> Tests run: 1694, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS

Harness proof, by re-running the worker's own mutation rather than trusting it (UNKNOWN ->
DOES_NOT_OWN): Tests run: 63, Failures: 3 -- exactly the three tests meant to pin it, matching their
report line for line:
  CallerResolverTest.aHerdrErrorOnTheOwningPaneDuringTheScanIsRefusedNotPromotedToPrimary
  ConnectionIdentityTest.scanIsIncompleteWhenHerdrErrorsOnThePaneThatOwnsThePid
  PaneLocatorTest.anErrorOnThePaneThatOwnsThePidMakesTheScanIncompleteNotAClearNegative
So the cells can go red, and their numbers are honest. Restored, shasum 81b797a6... byte-identical,
control 63/0.

MY OWN MUTATION, on the half they did not touch, FOUND A GAP. I mutated the two-client completeness
fold in terminalForPid -- `complete = complete && outcome.complete()` -> `complete =
outcome.complete()`, which forgets an earlier client's failure. Two greps proved it applied. Result:
Tests run: 63, Failures: 0. NOT pinned.

It is not an equivalent mutant: with CB-185's two herdr daemons, a first client that scans
incompletely followed by a second that scans cleanly gives `false` originally and `true` mutated, so
an incomplete scan would be reported complete and promoted. It is the #393 shape instead -- no test
assembles that combination, because the single-daemon tests collapse lead and member to one object.
That axis has cost us before: four routing defects passed the suite when one client served both roles
with memberHerdrSocket unset.

Merging anyway, because the production behaviour is correct and the ticket's defect is pinned three
ways -- what is missing is a test for a dimension the PR description claims in prose. Filed as a
follow-up with the numbers above.

Second item for the follow-up, latent not live: FleetMcp.legacyPrincipal still does
`c.terminal() != null ? worker : primary` and drops scanComplete, so it would re-open exactly this
hole. Measured unreachable in production -- Fleetd.java:696 always passes a real CallerResolver, and
the contextExtractor only calls legacyPrincipal when `callers == null`. A defect on paper is not a
reachable defect, so it is not a blocker; it is a trap for whoever next touches that constructor.

That second item is also the fleet01 lead's structural point, and it is the sharper framing: #415's
antidote (make the decision total) is ORTHOGONAL to this defect. Authz.permits is already a
default-less switch over Action and cannot help, because it says nothing about whether the caller was
resolved correctly, and Principal carries no record of how it was resolved. #415 guards a decision
nobody wrote; this was a decision written correctly and fed a bad input. The durable fix direction is
that an unresolved scan should be unrepresentable as a principal rather than checked for at each
consumer.
2026-09-12 05:35:41 +02:00
Dai Ha 979adf82eb fleetd #493: never build into the path a running daemon holds
CI / contract (pull_request) Successful in 1m30s
CI / build (pull_request) Successful in 1m37s
redeploy-fleetd.sh's build step wrote straight into fleetd/target/fleetd.jar
via `mvn clean install` while the OLD daemon was still running from that
exact path. A JVM loads classes lazily, so a class the daemon had not
touched yet could be read from a jar already replaced or removed -- the
failure landed on the shutdown drain (NoClassDefFoundError, exit 143,
looks clean).

Stage the freshly built jar at target/fleetd-new.jar (stage_built_jar),
confirm the OLD pid has actually exited (wait_for_daemon_exit, extracted
from the existing wait loop), and only then swap it into the live path
(swap_staged_jar) -- strictly after the wait, strictly before start. A
failed swap dies without starting. --no-build and --check keep their
existing, truthful behavior (require_no_build_jar; jar_id still reads
the live path by default). A leftover staged jar from an interrupted
run is wiped before the next build. The build still runs before
anything is stopped, so a failed build still never takes the fleet down.

Also fixed: the drain-gate abort message ("aborted -- nothing changed")
now names the staged jar when one exists, since staging already moves
the freshly built jar off the live path before that prompt runs.

Adds unit tests for stage_built_jar, swap_staged_jar, require_no_build_jar,
wait_for_daemon_exit, and a source-order test proving swap sits after the
wait and before start (sourcing stops before the main flow ever runs, so
the ordering itself can only be checked by reading the script's own call
sites).
2026-09-12 10:35:19 +07:00
Dai Ha 36870836aa fleetd #505: a herdr error during the pane scan must not read as a clean negative
CI / contract (pull_request) Successful in 1m26s
CI / build (pull_request) Successful in 1m29s
A transient herdr error on pane.process_info during PaneLocator's pid→pane scan used
to be swallowed into a plain "does not own it", so a real worker whose owning pane
errored mid-scan resolved with a null terminal but a resolved (real) pid — exactly
what CallerResolver's loopback-trust fallback reads as the primary. That is a
worker→primary privilege escalation through the door fleetd #317 did not close: #317
guards a failed lsof lookup (c.resolved()), not a failed herdr pane scan.

Fix: add a third state to the scan instead of widening Caller.resolved() (which stays
centralised next to the lsof sentinel it tests, per #505's explicit instruction not
to reopen that decision). PaneLocator.terminalForPid now returns a Lookup(terminal,
complete) record: a HerdrException on one pane marks that pane's ownership UNKNOWN,
not DOES_NOT_OWN, and the scan is complete only if every pane was either matched or
confirmed not to own the pid. A definite match found elsewhere in the same scan
still short-circuits as complete — a pane that genuinely vanished mid-scan without
being the caller's own does not turn into a refusal.

ConnectionIdentity.Caller carries the new scanComplete flag alongside the unchanged
resolved(). CallerResolver's loopback-trust fallback now requires both resolved()
and scanComplete() before promoting to Principal.primary(); an incomplete scan
resolves anonymous, which fails toward the recoverable error (a refused primary
retries loudly; a promoted worker would not).

Logs a warning naming the pane and which herdr client (of how many) failed, so the
incomplete-scan path is diagnosable rather than silent (fleetd #317's own lesson).
2026-09-12 10:27:42 +07:00
ltms 136312fb11 Merge #503: Injector's readiness-grace warn prints measured elapsed time, never arithmetic on constants (fleetd #501)
CI / build (push) Successful in 1m28s
CI / contract (push) Successful in 1m27s
Verified by the lead, not taken from the worker's report.

Trial-merged onto main (4f28da6) and built the merge, because a clean auto-merge is not a compiling
merge:
  mvn clean install -> Tests run: 1689, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS
  InjectorTest -> Tests run: 34, Failures: 0
  Gitea CI on ac351ee: success

The worker mutated the two log arguments. I mutated the half it did not: the STAMP SITE. Changing
the first-sample guard so the clock is re-stamped on every non-ready poll gave 1 failure,
InjectorTest.readinessGraceExpiryLogsTheMeasuredElapsedTimeNotArithmeticOnConstants, on a clock-read
counter assertion: "the readiness-not-ready branch should read the clock exactly twice — once to
stamp the first non-ready sample, once at grace expiry". Two greps proved the mutant applied;
restored, shasum byte-identical, control 34/0 green.

Behaviour preserved: the restructure from `else if (p != null && ++t.notReadySincePoll >= N)` to a
nested `else if (p != null) { ... }` keeps the short-circuit, so the counter still increments only
when p != null. All three reset sites now clear notReadySinceMillis alongside notReadySincePoll.

Two notes for the record, neither a blocker.

The poll-count half is an equivalent mutant and the worker said so instead of reporting a kill it
did not get. That is the right call and the code comment states the limit honestly.

The clock is injected through a package-private constructor overload while the public constructors
default it to System::currentTimeMillis. My brief prescribed that shape. With exactly one production
construction site (Fleetd.java:531) a required parameter — fleetd #415's antidote — would have been
just as cheap and would match LeadRollover and the new Fleetd.awaitHerdr. The silent-survivor risk
#415 names is not present here, because the field is final and both public constructors delegate, so
a future constructor cannot compile without supplying it. Recording the choice so the next person
does not read it as an oversight.
2026-09-12 05:07:35 +02:00
ltms 4f28da62a3 Merge #499: redeploy-fleetd.sh gains an "unclear" supervisor state that cannot reach kill, and the detail survives the subshell (fleetd #492, #497)
CI / contract (push) Successful in 1m12s
CI / build (push) Successful in 1m32s
Two commits, b17f37a and 599419f. Verified by the lead, not taken from the worker's report.

b17f37a adds the fifth answer "unclear" for installed-but-not-loaded and for a probe that could not
answer at all, and makes require_drivable_supervisor die on it.

599419f fixes a defect I found in b17f37a: SUPERVISOR_UNCLEAR_DETAIL was set inside
detect_supervisor, which is called as $(detect_supervisor). A subshell only returns stdout, so the
detail never reached the caller, and a ${VAR:-generic} fallback then printed a message naming no
supervisor and no reason. Measured before the fix: plain call -> detail length 142; via $( ) ->
length 0.

Lead verification of the fix, re-running my own original measurement across every branch:

  launchd installed, not loaded   kind=[unclear]   detail_len=142
  systemd installed, not active   kind=[unclear]   detail_len=146
  launchd loaded (normal)         kind=[launchd]   detail_len=0
  systemd loaded (normal)         kind=[systemd]   detail_len=0
  neither -> none                 kind=[none]      detail_len=0
  both loaded -> ambiguous        kind=[ambiguous] detail_len=0

The separator is always emitted, so the ${RAW#*SEP} unpack cannot silently fall back to the whole
string on the four branches that carry no detail.

Harness proof, run by me: making detect_supervisor print the kind alone (two greps proved the mutant
applied) turned the suite red, exit 1, "FAIL: detail does not name the systemd unit it found
installed-but-not-loaded". Restored, shasum byte-identical, control exit 0, bash -n clean on both
files. 23 test functions defined, 23 invoked.

The three case "$SUPERVISOR_KIND" switches now have final arms, chosen by what each caller does with
the value: the reporting switch warns and continues (a diagnostic that aborts goes silent exactly
when the state is novel), while stop and start die.

Not fixed here, filed separately: the "was already not running" branches at :603 and :610 swallow a
failed launchctl unload / systemctl stop with || true and then print "ok" unconditionally.
2026-09-12 05:04:04 +02:00
ltms 708f1795ad Merge #502: awaitHerdr reports three outcomes with measured elapsed time, not one boolean (fleetd #498)
CI / contract (push) Successful in 58s
CI / build (push) Successful in 1m49s
Verified by the lead, not taken from the worker's report.

Trial-merged onto main (8f59019) locally and built the merge, because a clean auto-merge is not a
compiling merge:
  mvn clean install -> Tests run: 1687, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS
  FleetdAwaitHerdrTest -> Tests run: 6, Failures: 0
  Gitea CI on 274afaf: success

The worker mutated the three return values. I mutated the half it did not: the reap DECISION at the
call site. Changing logHerdrWaitOutcomeAndShouldReap to return true for INTERRUPTED gave 1 failure,
FleetdAwaitHerdrTest.interruptedLogsItsOwnMessageAndNeverClaimsTheBudgetElapsed:135 ("an interrupted
wait must not tell main to reap"). Two greps proved the mutant applied; restored, shasum
byte-identical, control 6/0 green.

Behaviour preserved: herdrUp is still true only for ANSWERED, and both its readers (the orphan reap
at :276 and the lead auto-launch gate at :375) see exactly what they saw before.

One correction to the worker's report, which changes nothing in the code. It wrote that a real
interrupt-detection regression "would also hang the daemon's startup thread forever". It would not:
in production `nanos` is System::nanoTime, so the deadline check still fires. The hang it hit was a
test-fixture property — a frozen injected clock with no iteration bound. That is fleetd #486's
shape, now seen in a second class, and it is recorded there.
2026-09-12 05:01:02 +02:00
Dai Ha 599419f9e6 fleetd #492 follow-up: carry the unclear detail across detect_supervisor's subshell boundary
CI / contract (pull_request) Successful in 47s
CI / build (pull_request) Successful in 1m53s
b17f37a set SUPERVISOR_UNCLEAR_DETAIL as a global inside detect_supervisor, but the real call
site invokes it as $(detect_supervisor) — a subshell — so that global died with the subshell and
the die() message's ${VAR:-fallback} silently masked the loss with generic text.

- detect_supervisor now packs kind and detail onto its one stdout line (joined by the ASCII unit
  separator byte, $SUPERVISOR_DETAIL_SEP), the only channel that survives $( ). The real call site
  unpacks both with in-shell parameter expansion — no extra subshell.
- Dropped the ${SUPERVISOR_UNCLEAR_DETAIL:-...} fallback at the die() message: under set -u, a
  missing detail now fails loudly instead of silently defaulting (same defect class as #497).
- Added a constraints comment block above detect_supervisor for future callers: stdout-only,
  no ${VAR:-default} papering over a lost value, and every case on the return value needs an
  explicit *) arm.
- Added *) arms to the three `case "$SUPERVISOR_KIND"` switches (report/stop/start): report warns
  and continues (display-only), stop/start die naming the value (they act on it).
- Rewrote the "unclear" test to go through the real call-site shape ($(detect_supervisor) then
  the same split), not a hand-constructed value, and tightened its final assertion to check for
  the actual detail text rather than $SYSTEMD_UNIT alone (the die() boilerplate names the unit
  either way, so that check could pass on a lost value).
2026-09-12 09:58:45 +07:00
Dai Ha ac351ee1de fleetd #501: readiness-grace expiry logs measured elapsed time and the loop's own poll counter, never the configured budget
CI / contract (pull_request) Successful in 49s
CI / build (pull_request) Successful in 2m2s
The line at Injector.java:383-387 printed two numbers that read as measurements
but were both compile-time constants: READINESS_GRACE_POLLS for the poll count
(the loop's own Target.notReadySincePoll counter was in scope at the same call
site), and READINESS_GRACE_POLLS * POLL_INTERVAL_MILLIS / 1000 for the elapsed
time — arithmetic on two constants, never a measurement, and wrong in the
direction that says everything ran on schedule.

Fix, copying the LongSupplier-clock shape LeadRollover already uses:
- print t.notReadySincePoll instead of the constant for the poll count (the two
  agree by construction on this branch, so no test can tell them apart — the
  comment says so honestly).
- add Target.notReadySinceMillis, stamped at the first non-ready sample and
  reset at all three sites notReadySincePoll already resets (:304, :355, :389
  pre-fix line numbers), to compute a real elapsed time at expiry.
- inject a LongSupplier nowMillis (defaulting to System::currentTimeMillis)
  through new package-private constructor overloads so a test can supply a
  clock whose advance does not track POLL_INTERVAL_MILLIS.

Tests use a ListAppender to assert on the log message contents, per
LeadRolloverTest's pattern. The elapsed-time test drives the loop with a stub
clock returning two literal, non-derived values so it can fail if the fix
regresses to the constant-arithmetic line — proved by mutation: reverting the
elapsed calculation to READINESS_GRACE_POLLS * POLL_INTERVAL_MILLIS turns that
one test red (1 failure); reverting the poll-count print to the constant is an
equivalent mutant (0 failures), because the counter and the constant are
identical at that exact call site by construction.

fleetd clean install: Tests run: 1683, Failures: 0, Errors: 0, Skipped: 0.
2026-09-12 09:57:57 +07:00
Dai Ha 274afafde6 fleetd #498: awaitHerdr distinguishes deadline-passed from interrupted, with measured elapsed time
CI / contract (pull_request) Successful in 51s
CI / build (pull_request) Successful in 1m51s
- awaitHerdr now returns a HerdrAwaitOutcome(HerdrWaitResult, elapsedNanos) instead of a bare
  boolean, so 'the wait budget genuinely ran out' and 'the waiting thread was interrupted' are
  two distinct, named states instead of the same false (fleetd #497's shape).
- awaitHerdr takes the clock (LongSupplier) and the per-poll sleep (Runnable) as required
  parameters, with no defaulted overload (fleetd #415), so a test can drive it.
- The startup call site is extracted into logHerdrWaitOutcomeAndShouldReap, since main() itself
  cannot be driven from a unit test; it logs a distinct message per outcome, always printing the
  measured elapsed time next to the configured budget, never the budget alone.
- Adds FleetdAwaitHerdrTest covering the seam (all three outcomes, plus the preserved interrupt
  flag) and the call site (the three distinct log messages), using ListAppender.
2026-09-12 09:54:59 +07:00
ltms 8f59019305 Merge #496: lead-rollover logs measured elapsed time and measured counts, never the configured budget (fleetd #494)
CI / contract (push) Successful in 1m14s
CI / build (push) Successful in 2m1s
Three commits. All verified by the lead in a detached worktree, not taken from the worker's report.

3fb3311 — the /clear-settle and success paths return measured elapsed time and a real nudge count
c87cc25 — print the measured nudge count; fix the same defect on the sibling turn-settle timeout line
e966cba — the poll count on the same line was still a constant; the test could not tell the difference

Lead verification at e966cba:
  mvn clean install -> Tests run: 1681, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS
  LeadRolloverTest  -> Tests run: 33, Failures: 0, Errors: 0, Skipped: 0
  Gitea CI on e966cba: success

Mutation M2 (PICKUP_GRACE_POLLS 8 -> 5), run by the lead: 2 failures,
clearGraceReleaseLogIsWarnWithMeasuredElapsed and successLogPrintsMeasuredElapsedForTheWholeRoll.
The emitted line under the mutant read "after 5 consecutive IDLE/DONE polls (4 of those were
nudged)", so both numbers now follow the loop. Restored, shasum byte-identical, control 33/0 green.

Known and deliberate limit, stated in the code comment: on the grace-release branch both counters
equal the constants by construction (one write site each), so no test can prove the difference on
that line. The change buys one source of truth, not provable coverage. The place nudges genuinely
varies with the run is the /clear-timeout warn, which is covered by a discriminating test.
2026-09-12 04:44:10 +02:00
Dai Ha e966cbadf9 fleetd #494 follow-up (2nd pass): the grace-release line still had one constant, and its test could not tell the difference
CI / contract (pull_request) Successful in 1m26s
CI / build (pull_request) Successful in 1m55s
Two more fixes on the same line, LeadRollover.java:600-624:

1. The poll-count argument (second, was PICKUP_GRACE_POLLS) now prints the
   loop's own idlePollsAwaitingPickup counter instead of the constant. Same
   defect shape as the nudges fix from c87cc25, one argument over.

2. LeadRolloverTest's clearGraceReleaseLogIsWarnWithMeasuredElapsed asserted
   its nudge-count expectation as `LeadRollover.PICKUP_GRACE_POLLS - 1` — the
   same expression the production code used to build the log line from, so it
   could not discriminate a reverted fix. Rewritten to plain literals
   ("after 8 consecutive IDLE/DONE polls (7 of those were nudged)"), proven to
   trip when PICKUP_GRACE_POLLS's value changes (2 failures under a
   PICKUP_GRACE_POLLS=5 mutation: this test and successLogPrintsMeasured...).

Also corrected the comment above the log line: idlePollsAwaitingPickup and
nudges each have exactly one write site on this loop's release branch, so they
cannot differ from PICKUP_GRACE_POLLS / PICKUP_GRACE_POLLS - 1 at this call
site — confirmed by re-deriving the loop's control flow, and by reverting just
the nudges argument (M1) and observing 33/0 stayed green even after the test
rewrite. That is an equivalent mutant on this line, not a gap the test rewrite
could close; the honest value of printing the counters is one source of truth
for the loop, not a provable-by-test difference here. The place nudges truly
varies with the run — and is covered by a test that can tell it apart from a
constant — is the /clear-timeout warn's clearResult.nudges() in runRollover.

mvn -Dtest=LeadRolloverTest test: 33/0. mvn clean install: 1681/0, BUILD SUCCESS.

Same-shape sweep (found, not fixed, per instructions):
Injector.java:383-387 — the readiness-grace warn prints the constant
READINESS_GRACE_POLLS where the measured per-target counter
t.notReadySincePoll is in scope (single increment site, just reached the
threshold at this call site — same equivalent-mutant situation as this fix).
2026-09-12 09:39:39 +07:00
Dai Ha b17f37a683 fleetd #492 follow-up: detect_supervisor must never read "could not tell" as "none"
CI / contract (pull_request) Successful in 1m11s
CI / build (pull_request) Successful in 1m32s
Two situations were silently landing in the "none" answer, which
require_drivable_supervisor accepts and the script then falls back to a raw
kill + nohup — exactly the wrong move when a supervisor actually IS present:

- installed-but-not-loaded, on either supervisor. `systemctl --user is-active`
  answers "no" for activating/deactivating/failed and while an auto-restart is
  pending too, and every one of those is a host that IS under systemd (or
  launchd) and about to act again. `*_installed` already knew this; it was
  only ever consulted for a warning line, never by the decision itself.
- a systemd probe that could not answer at all (e.g. systemctl cannot reach
  the user bus over a non-lingering ssh session) looked identical to a clean
  negative, because both probes redirected stderr straight to /dev/null.

detect_supervisor now returns a fifth answer, "unclear", for both cases.
systemd_loaded/systemd_installed capture systemctl's exit status and stderr
separately and set their own *_ERRORED flag only on a real tool failure
(non-zero exit WITH stderr), never on a clean negative. "none" now means only:
neither supervisor installed, neither loaded, neither probe errored.
require_drivable_supervisor die()s on "unclear" exactly like it already does
on "ambiguous", naming the specific supervisor and reason via the new
SUPERVISOR_UNCLEAR_DETAIL global.

Tests: 4 new cases (systemd/launchd installed-but-not-loaded, a real
systemd_loaded run through a systemctl stub that errors on stderr, and the
die() refusal for "unclear" naming the unit). All 3 new guards were verified
by mutation: each was removed from the real script, the suite caught it (a
new FAIL line naming the exact broken assertion), then the file was restored
byte-identically and the suite went green again.
2026-09-12 09:23:21 +07:00
Dai Ha c87cc25aa6 fleetd #494 follow-up: print the measured nudge count, and fix the sibling turn-settle timeout line
CI / contract (pull_request) Successful in 1m13s
CI / build (pull_request) Successful in 1m30s
- waitForClearPickupAndSettle's grace-release warn now prints the measured 'nudges' counter
  instead of the constant PICKUP_GRACE_POLLS - 1. The two happen to agree today, but the
  constant expression was wrong once before (printed PICKUP_GRACE_POLLS itself, claiming 8
  nudges where 7 went out) and a code read did not catch it — only a mutation test did.
  Printing the counter cannot drift from the loop's real behaviour.
- waitUntilAtTurnBoundary (the FIRST wait, ~line 386-391) had the identical 'configured value
  printed as if measured' defect as the three lines fixed in the original #494 commit, but was
  out of scope because the brief named specific lines instead of the shape. Fixed the same way:
  it now returns a TurnSettleResult(settled, elapsedMillis) instead of a bare boolean, and the
  timeout warn prints 'configured={}s elapsed={}ms' instead of presenting cfg.turnSettleSeconds()
  as the measured wait.
- No behaviour change: same sends, same order, same release/refuse decisions.
- Test additions: clearGraceReleaseLogIsWarnWithMeasuredElapsed now also asserts the measured
  nudge count; successLogPrintsMeasuredElapsedForTheWholeRoll's expected elapsed value is
  updated (7000ms, not 6500ms) to account for waitUntilAtTurnBoundary's own new clock read; a
  new turnTimeoutLogPrintsMeasuredElapsedNotJustConfigured test pins the sibling line.
- Proved the nudges fix with a temporary mutation: set PICKUP_GRACE_POLLS to 5, confirmed via
  two greps that the mutant applied and the original constant was gone, ran the grace-release
  test and read the actual log line — nudge count followed to 4 (= 5 - 1), then restored to 8
  and reran the full LeadRolloverTest suite as a control (33/33 green).
2026-09-12 09:22:19 +07:00
Dai Ha 3fb331145a fleetd #494: log measured elapsed time, never the configured budget, on a lead-rollover failure/success
CI / contract (pull_request) Successful in 52s
CI / build (pull_request) Successful in 1m35s
- runRollover's /clear-timeout warn now prints configured/elapsed/nudges, each labelled,
  instead of presenting cfg.clearSettleSeconds() as if it were the measured wait.
- waitForClearPickupAndSettle's pickup-grace release is now log.warn (was log.info) and
  prints the measured elapsed time next to the target pane — this is the exact path that
  reported a false-success roll in the real incident (438ms of a 20s budget).
- The success line ('lead-rollover: rolled') now prints the measured elapsed time for the
  whole roll.
- waitForClearPickupAndSettle now returns a ClearSettleResult(settled, elapsedMillis, nudges)
  instead of a bare boolean, so callers can log the measured values instead of the config.
- No behaviour change: same sends, same order, same release/refuse decisions.
- Adds 3 tests to LeadRolloverTest pinning the content of each changed log line, using a
  self-advancing fake clock so the measured elapsed/nudge values are deterministic and
  provably distinct from the configured budget.
2026-09-12 09:08:04 +07:00
Dai Ha dcd505286f fleetd #492: teach redeploy-fleetd.sh systemd --user as a third supervisor
CI / contract (pull_request) Successful in 56s
CI / build (pull_request) Successful in 1m40s
launchd, systemd --user, and unsupervised are three different answers, not two.
Refuse (die) rather than fall through to kill+nohup when a supervisor is
detected that this script cannot drive (e.g. both signals fire at once), and
add a post-restart check that fails the run if more than one fleetd process
is alive. detect_supervisor()/require_drivable_supervisor()/
count_daemon_pids()/assert_single_daemon() are pure, overridable functions so
scripts/test-redeploy-fleetd.sh can exercise them without a real launchd or
systemd.
2026-09-12 09:00:59 +07:00
Dai Ha 60fa958da2 docs: a relative handoverPath lands in the LEAD's repo, not fleetd's (#491)
CI / contract (push) Successful in 1m29s
CI / build (push) Successful in 1m34s
On this host the lead's cwd IS the fleetd checkout, so fleetd/.gitignore
protects the handover file and the distinction is invisible. On fleet01 the
lead works in /home/ltms/LTMS/kb while fleetd sits in a different directory,
and that repo has no .handover rule — measured 2026-09-12.

Tell the lead to check its own workspace's .gitignore before writing, and to
report it rather than committing the file or editing someone's .gitignore.

Refs fleetd #491, #487, #480.
2026-09-12 08:46:17 +07:00
Dai Ha 9950361bc9 docs: #489 is fixed and deployed, but criterion 6 is still unmet
CI / contract (push) Successful in 1m12s
CI / build (push) Successful in 1m33s
The skill said the roll 'does nothing until #489 is merged and redeployed'.
Both have happened, so that sentence now reads as a green light. It is not
one: no roll has bootstrapped a fresh session end to end yet. Say that
plainly, and tell the lead to warn the operator before confirming.

Refs fleetd #489, #480.
2026-09-12 07:55:41 +07:00
ltms 9f74b6619a Merge #490: nudge the /clear submit keystroke before bootstrapText (fleetd #489)
CI / contract (push) Successful in 1m14s
CI / build (push) Successful in 1m30s
Fixes the live defect measured on 2026-09-12: the roll joined /clear and
bootstrapText into one line and Claude Code refused it as
"Unknown command: /clearFresh".

Verified by the lead before merge:
- mvn clean install: Tests run: 1677, Failures: 0, BUILD SUCCESS
- LeadRolloverTest baseline: 29 tests, 0 failures
- mutation PICKUP_GRACE_POLLS 8 -> 1: 1 failure (the regression test)
- mutation deleting the !clearSettled guard: 3 failures, 2 pre-existing tests
- mutation replacing waitForClearPickupAndSettle with "return true": 5 failures,
  including the strengthened pickup test
- positive control after each restore: 29 tests, 0 failures
- CI run 1738 on f687046: success

A separate mutation of the FIRST gate hung the suite instead of failing it.
That is fleetd #486, not a regression here; the reproduction is recorded there.
2026-09-12 02:52:52 +02:00
Dai Ha 2a95da2ff4 docs: the handover skill must warn that the automatic roll is broken (#489)
CI / contract (push) Successful in 1m23s
CI / build (push) Successful in 1m53s
Section 11 said the bootstrap prompt landing was 'not yet proven
end-to-end'. It is now measured failing: the first real roll joined
/clear and bootstrapText into one line. Nothing was cleared, so the
failure is safe, but a lead that reads the old wording would reach for
fleet_handover expecting it to work.

Refs fleetd #489, #480.
2026-09-12 07:21:38 +07:00
26 changed files with 2796 additions and 315 deletions
+16 -4
View File
@@ -141,6 +141,14 @@ refused.**
own workspace before it hands it to you. The daemon and your pane can run in different
directories, so a path you resolve yourself can point at a different file from the one the daemon
will check.
**Check that the path is ignored by git before you write to it (#491).** A relative
`handoverPath` resolves inside YOUR workspace, which is usually a repository — and usually not
the `fleetd` one, so an ignore rule added to `fleetd` does not protect it. Run
`grep -n handover <your workspace>/.gitignore`. No output means the file you are about to write
will show up as untracked content in that repo. The file is a snapshot of live state and must
never be committed, so tell the operator rather than committing it or silently editing their
`.gitignore`.
2. **Write the handover file at that path**, following sections 1–10 above.
3. **Ask the operator, then `fleet_handover{action: "confirm", token, operatorConfirmed: true}`.**
@@ -164,10 +172,14 @@ fails.
session.
- **The roll can still refuse after `confirm` returns**, and by then there is no caller to tell.
Those outcomes are logged only, as `lead-rollover:` lines in the daemon log.
- **If the bootstrap prompt never lands, your context is gone and no fresh session starts.** This
has not yet been proven end-to-end (see fleetd #480). The recovery is the manual path: the file
is already written, so the operator starts a session and points it at the file. That is why you
write the file before you confirm, and never the other way round.
- **The bootstrap prompt has never yet landed, and the fix is unproven (fleetd #489).** The first
real rollover, on 2026-09-12, joined `/clear` and the bootstrap text into one line and Claude Code
refused it as `Unknown command: /clearFresh`. The pane was never cleared and no context was lost,
so the failure was safe — the roll simply did nothing. PR #490 fixed the cause and is deployed,
but no roll has bootstrapped a fresh session end to end yet. **Assume it may still fail, and tell
the operator so before you confirm.** The recovery is the same either way: the file is already
written, so the operator starts a session and points it at the file. That is why you write the
file before you confirm, and never the other way round.
## Writing style
+9 -4
View File
@@ -96,8 +96,10 @@ below are the procedure — run them in order, every task, not only the big ones
that answers it. **A worker's ask waits ~55 seconds, and no nudge makes that longer** — so never
brief a worker to "ask me". Decide before you delegate, or give it an explicit default.
6. **Verify yourself.** Re-run the build and the checks. A worker cannot run your IDE tooling, any
forge tools it appears to have hold a blocked credential and fail, and a piped command
(`… | tail`) hides failures behind a zero exit — never promote a worker's "clean" to a fact.
forge MCP server it appears to have holds a blocked credential and fails every call, and a piped
command (`… | tail`) hides failures behind a zero exit — never promote a worker's "clean" to a
fact. Its injected repo-scoped `GITEA_TOKEN` is a different credential and does work, so a worker
reporting that it opened its own PR is reporting something it really can do.
7. **Review — fan out.** Spawn reviewers against the diff, one per dimension or per file, with
`wait:false`. Never the implementer of the scope it reviews, and brief them from the diff — not
from the implementer's rationale, which carries its own blind spot. Dispatch each PR's reviewers
@@ -200,8 +202,11 @@ simply complies has thrown away the reason there are two of you.
assume them.** What you mount depends on your backend: an opencode member gets the bridge and
nothing else, while a Claude Code member also inherits the operator's user-scope MCP servers,
which the bridge never chose for you. Two rules follow. The primary's IDE tooling is still not
yours, whatever you see. And **a mounted tool is not a working tool** — the forge server you may
find there holds a deliberately blocked credential and fails every call, by design.
yours, whatever you see. And **a mounted tool is not a working tool** — the forge MCP server you
may find there holds a deliberately blocked credential and fails every call, by design. That is
not your only forge route, and the two must not be confused: the repo-scoped `GITEA_TOKEN` the
daemon injects into your environment does work, and using it to open your own PR is part of the
job. A blocked MCP tool is never a reason to skip that step.
6. **Never merge.** Stage files explicitly — never `git add -A` — and leave alone anything the
project marks as not-yours-to-commit.
+107 -19
View File
@@ -80,6 +80,7 @@ import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.function.LongSupplier;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.regex.Pattern;
@@ -270,15 +271,12 @@ public final class Fleetd {
// the first thing that actually talks to herdr, so without this wait a boot-order race
// would crash the daemon into a restart loop. Wait, then degrade rather than die: serving
// with /healthz reporting "degraded" is strictly more useful than exiting.
boolean herdrUp = awaitHerdr(herdr);
HerdrAwaitOutcome herdrOutcome = awaitHerdr(herdr, System::nanoTime, Fleetd::sleepHerdrPoll);
boolean herdrUp = logHerdrWaitOutcomeAndShouldReap(herdrOutcome);
if (herdrUp) {
// CB-117: herdr keeps worker panes alive across a daemon restart, and their ids died
// with the previous process — reap those leaked orphans now, before we start serving.
workers.reapOrphanWorkers();
} else {
log.warn("herdr did not answer within {}s — starting anyway; /healthz will report "
+ "degraded until it comes up. Orphaned worker panes (if any) were NOT reaped.",
HERDR_WAIT_SECONDS);
}
// CB-301: authoritative session registry + lifecycle FSM on top of ClaudeCodeLauncher.
@@ -696,7 +694,7 @@ public final class Fleetd {
}, outagePolicy);
FleetMcp mcp = new FleetMcp(messages, workers, sessions, identity, presence,
primaryRegistry, callers, metrics,
primaryRegistry, callers, FleetMcp.AuthorizationMode.ENFORCED, metrics,
capacitySource(config, cfg, profile -> liveCountRef.get().apply(profile)),
new FleetMcp.HealthCoverageSource(() -> {
var health = config.get().health();
@@ -1696,12 +1694,70 @@ public final class Fleetd {
}
/**
* Poll herdr's {@code ping} until it answers or {@link #HERDR_WAIT_SECONDS} elapses (CB-504).
*
* @return true if herdr answered, false if it never did
* How {@link #awaitHerdr} ended (fleetd #498). The old code returned a bare {@code boolean},
* which collapsed two different facts onto the same {@code false}: the configured wait budget
* genuinely running out, and the waiting thread being interrupted possibly milliseconds in.
* Those need different operator messages — see {@link #logHerdrWaitOutcomeAndShouldReap} — so
* this is a third state, not a better number (the same shape fleetd #497 named). Never treat
* {@link #INTERRUPTED} as if it were {@link #DEADLINE_PASSED}: only the latter means herdr was
* actually given the full {@link #HERDR_WAIT_SECONDS} and still failed to answer.
*/
private static boolean awaitHerdr(HerdrClient herdr) {
long deadline = System.nanoTime() + HERDR_WAIT_SECONDS * 1_000_000_000L;
enum HerdrWaitResult {
/** herdr answered {@code ping} before the deadline. */
ANSWERED,
/** the configured {@link #HERDR_WAIT_SECONDS} budget elapsed with no answer. */
DEADLINE_PASSED,
/**
* the waiting thread was interrupted before the budget ran out — a different event from
* {@link #DEADLINE_PASSED} and must never be reported as "did not answer within Ns".
*/
INTERRUPTED
}
/**
* The outcome of one {@link #awaitHerdr} call, carrying the MEASURED elapsed wait time
* alongside {@link #result}. {@code elapsedNanos} is always measured against the {@code nanos}
* supplier passed to {@link #awaitHerdr} — never assume it equals the configured budget, the
* same defect fleetd #494 already fixed once in {@code LeadRollover}.
*/
record HerdrAwaitOutcome(HerdrWaitResult result, long elapsedNanos) {}
/**
* The real per-poll wait {@link #main} passes to {@link #awaitHerdr}: sleep
* {@link #HERDR_WAIT_POLL_MILLIS}, and on interruption re-set the thread's interrupt flag
* rather than throwing — {@link #awaitHerdr} detects an interruption by checking {@link
* Thread#isInterrupted()} right after this returns, so a poller that swallowed the flag
* instead of restoring it would make that check silently miss the interruption.
*/
private static void sleepHerdrPoll() {
try {
Thread.sleep(HERDR_WAIT_POLL_MILLIS);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
}
}
/**
* Poll herdr's {@code ping} until it answers, the configured {@link #HERDR_WAIT_SECONDS}
* budget elapses, or the waiting thread is interrupted (CB-504, fleetd #498).
*
* <p>{@code nanos} and {@code poller} are required parameters with no defaulted overload
* (fleetd #415's shape: a defaulted overload is a silent survivor a green suite would vouch
* for) — the previous version read {@link System#nanoTime()} and called {@link Thread#sleep}
* directly, so nothing could drive it from a test. The one production call site in {@link
* #main} passes {@code System::nanoTime} and {@link #sleepHerdrPoll}.
*
* @param nanos a monotonic elapsed-time clock, e.g. {@code System::nanoTime} — never a
* wall-clock source, since only elapsed time (not a timestamp) is measured here
* @param poller called once per failed ping while the budget remains; must, on an
* {@link InterruptedException}, re-set the thread's interrupt flag rather than
* throw or swallow it — this method's interruption check reads that flag right
* after {@code poller.run()} returns
* @return the outcome and the measured elapsed wait time — see {@link HerdrAwaitOutcome}
*/
static HerdrAwaitOutcome awaitHerdr(HerdrClient herdr, LongSupplier nanos, Runnable poller) {
long start = nanos.getAsLong();
long deadline = start + HERDR_WAIT_SECONDS * 1_000_000_000L;
boolean waited = false;
while (true) {
try {
@@ -1709,25 +1765,57 @@ public final class Fleetd {
if (waited) {
log.info("herdr is up");
}
return true;
return new HerdrAwaitOutcome(HerdrWaitResult.ANSWERED, nanos.getAsLong() - start);
} catch (HerdrException e) {
if (System.nanoTime() >= deadline) {
return false;
if (nanos.getAsLong() >= deadline) {
return new HerdrAwaitOutcome(HerdrWaitResult.DEADLINE_PASSED, nanos.getAsLong() - start);
}
if (!waited) {
log.info("waiting up to {}s for the herdr socket…", HERDR_WAIT_SECONDS);
waited = true;
}
try {
Thread.sleep(HERDR_WAIT_POLL_MILLIS);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
return false;
poller.run();
if (Thread.currentThread().isInterrupted()) {
return new HerdrAwaitOutcome(HerdrWaitResult.INTERRUPTED, nanos.getAsLong() - start);
}
}
}
}
/**
* Log the right message for {@code outcome} — never the configured {@link #HERDR_WAIT_SECONDS}
* budget alone, always the measured elapsed time next to it — and say whether {@link #main}
* should now reap orphan worker panes (fleetd #498).
*
* <p>Extracted out of {@link #main} so this decision is drivable from a test: {@link #main}
* boots the whole daemon and cannot itself be run in a unit test, but this is the exact,
* unmodified code {@link #main} calls for the decision, not a re-derivation of it.
*
* @return true only for {@link HerdrWaitResult#ANSWERED} — orphan workers are reaped only
* then, exactly as before this ticket
*/
static boolean logHerdrWaitOutcomeAndShouldReap(HerdrAwaitOutcome outcome) {
long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(outcome.elapsedNanos());
if (outcome.result() == HerdrWaitResult.ANSWERED) {
return true;
}
if (outcome.result() == HerdrWaitResult.DEADLINE_PASSED) {
log.warn("herdr did not answer within the configured wait (configured={}s elapsed={}ms) "
+ "— starting anyway; /healthz will report degraded until it comes up. Orphaned "
+ "worker panes (if any) were NOT reaped.",
HERDR_WAIT_SECONDS, elapsedMillis);
return false;
}
// HerdrWaitResult.INTERRUPTED — a different fact from DEADLINE_PASSED (fleetd #498): the
// wait was cut short, not exhausted, and must never be reported as "did not answer within
// Ns" — that claim would be false and would send an operator to debug herdr for nothing.
log.warn("herdr wait was interrupted before the configured wait ran out (configured={}s "
+ "elapsed={}ms) — starting anyway; /healthz will report degraded until it comes "
+ "up. Orphaned worker panes (if any) were NOT reaped.",
HERDR_WAIT_SECONDS, elapsedMillis);
return false;
}
private Fleetd() {
}
}
@@ -244,7 +244,15 @@ public final class CallerResolver {
// already names what happens if that case is handed the primary role: a worker→primary
// escalation. So an unresolved caller is refused (ANONYMOUS — the same clean, already-tested
// "authenticated as nothing" outcome used everywhere else in this method), never promoted.
return isLoopback(remoteAddr) && c.resolved() ? Principal.primary(c.pid()) : Principal.anonymous();
//
// fleetd #505: the OTHER way a real pid can wrongly reach here with a null terminal — not a
// failed lsof lookup, but a herdr error partway through PaneLocator's pane scan. c.resolved()
// says nothing about that; it only tests the lsof sentinel (by design — see
// ConnectionIdentity.Caller#resolved). c.scanComplete() is the separate signal: a scan that
// could not check every pane must not be read as "checked everywhere, no match" — the pane it
// could not check might have been the caller's own. So both must hold before this promotes.
return isLoopback(remoteAddr) && c.resolved() && c.scanComplete()
? Principal.primary(c.pid()) : Principal.anonymous();
}
private boolean presentedTokenMatches(String authorizationHeader) {
@@ -1,6 +1,8 @@
package dev.ltms.fleet.herdr;
import com.fasterxml.jackson.databind.JsonNode;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.LinkedHashSet;
import java.util.List;
@@ -36,6 +38,8 @@ import java.util.Set;
*/
public final class PaneLocator {
private static final Logger log = LoggerFactory.getLogger(PaneLocator.class);
/**
* Bound on how many ancestor generations {@link #ancestorsOf} walks. This runs on every MCP
* call, so a cycle or a pathologically deep process tree must not hang identity resolution;
@@ -73,22 +77,46 @@ public final class PaneLocator {
}
/**
* The {@code terminal_id} of the agent pane whose process tree contains {@code pid}, or
* {@code null} if no agent pane on any searched daemon owns it (e.g. the caller is the
* primary, or off-host).
* The outcome of a {@link #terminalForPid} scan: the {@code terminal_id} of the agent pane
* whose process tree contains the pid ({@link #terminal} is {@code null} if none matched),
* and whether the scan that produced that answer ran to completion on every daemon searched.
*
* <p>{@link #complete} is {@code false} exactly when some {@code pane.process_info} call
* failed and, despite that, no pane was ever found to own the pid. In that case a {@code null}
* {@link #terminal} means "could not tell", not "definitely not a worker" — fleetd #505: a
* transient herdr error on the very pane that <em>does</em> own the caller's pid must not read
* as a clean negative and fall through to {@code Principal.primary}, the same way #317's
* {@code Caller.resolved()} already guards a failed lsof lookup. Callers ({@code
* ConnectionIdentity}, {@code CallerResolver}) must refuse rather than promote on an incomplete
* scan.
*
* <p>When a pane genuinely owns the pid, {@link #complete} is {@code true} regardless of
* whether some other, unrelated pane failed to answer earlier in the same scan — a positive
* match is definitive and does not need every pane to have been checked (a pane that "vanished
* mid-scan" but was never the match is still a clean, complete result).
*/
public String terminalForPid(long pid) {
public record Lookup(String terminal, boolean complete) {
private static final Lookup NOT_FOUND = new Lookup(null, true);
}
/**
* Resolve {@code pid} to the agent pane whose process tree contains it, across every searched
* herdr daemon. See {@link Lookup} for how to read a {@code null} terminal.
*/
public Lookup terminalForPid(long pid) {
if (pid <= 0) {
return null;
return Lookup.NOT_FOUND;
}
Set<Long> ancestry = ancestorsOf(pid);
for (HerdrClient herdr : herdrs) {
String terminal = terminalForPid(herdr, ancestry);
if (terminal != null) {
return terminal;
boolean complete = true;
for (int i = 0; i < herdrs.size(); i++) {
Lookup outcome = scan(herdrs.get(i), i, herdrs.size(), ancestry);
if (outcome.terminal() != null) {
return outcome; // a definite match — no need to finish checking other clients
}
complete = complete && outcome.complete();
}
return null;
return new Lookup(null, complete);
}
/**
@@ -117,31 +145,51 @@ public final class PaneLocator {
return ancestry;
}
private static String terminalForPid(HerdrClient herdr, Set<Long> ancestry) {
/** Whether a pane owns one of the scanned pid's ancestors, or the check of it failed outright. */
private enum Ownership { OWNS, DOES_NOT_OWN, UNKNOWN }
private static Lookup scan(HerdrClient herdr, int clientIndex, int clientCount, Set<Long> ancestry) {
boolean complete = true;
for (JsonNode pane : herdr.call("pane.list", Map.of()).path("panes")) {
String paneId = pane.path("pane_id").asText(null);
if (paneId != null && paneOwnsAnyOf(herdr, paneId, ancestry)) {
return pane.path("terminal_id").asText(null);
if (paneId == null) {
continue;
}
Ownership owns = paneOwnsAnyOf(herdr, clientIndex, clientCount, paneId, ancestry);
if (owns == Ownership.OWNS) {
return new Lookup(pane.path("terminal_id").asText(null), true);
}
if (owns == Ownership.UNKNOWN) {
complete = false;
}
}
return null;
return new Lookup(null, complete);
}
private static boolean paneOwnsAnyOf(HerdrClient herdr, String paneId, Set<Long> ancestry) {
private static Ownership paneOwnsAnyOf(HerdrClient herdr, int clientIndex, int clientCount,
String paneId, Set<Long> ancestry) {
JsonNode info;
try {
info = herdr.call("pane.process_info", Map.of("pane_id", paneId)).path("process_info");
} catch (HerdrException e) {
return false; // pane vanished mid-scan — just skip it
// fleetd #505: this used to be read as a clean "does not own it" (the pane vanished
// mid-scan, just skip it) — one boolean carrying two different facts. It is UNKNOWN
// now: if THIS pane is the one that owns the pid, the caller must not be told "no pane
// owns it", because that reads as a real primary and is promoted under loopback-trust.
log.warn("pane.process_info failed for pane {} on herdr client {} of {} during a "
+ "pid-owner scan — treating it as \"could not tell\", not a clean "
+ "negative (fleetd #505): {}",
paneId, clientIndex + 1, clientCount, e.getMessage());
return Ownership.UNKNOWN;
}
if (ancestry.contains(info.path("shell_pid").asLong(-1))) {
return true;
return Ownership.OWNS;
}
for (JsonNode p : info.path("foreground_processes")) {
if (ancestry.contains(p.path("pid").asLong(-1))) {
return true;
return Ownership.OWNS;
}
}
return false;
return Ownership.DOES_NOT_OWN;
}
}
@@ -15,6 +15,7 @@ import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Consumer;
import java.util.function.LongSupplier;
import java.util.function.Predicate;
import java.util.stream.Collectors;
@@ -94,6 +95,14 @@ public final class Injector {
private final TurnListener turnListener;
private final Predicate<String> ready; // CB-113: a target is deliverable only when available
private final Consumer<String> forget; // CB-114: clear a gone worker's readiness/presence
/**
* Wall-clock source for the readiness-grace elapsed time logged in {@link #onStatus} (fleetd
* #501). Production constructors default this to {@code System::currentTimeMillis}; the
* package-private constructors below take it explicitly so a test can supply a stub whose
* advance does not track {@link #POLL_INTERVAL_MILLIS} — copying the shape {@code LeadRollover}
* already uses for the same purpose.
*/
private final LongSupplier nowMillis;
private final ConcurrentHashMap<String, Target> targets = new ConcurrentHashMap<>();
/** Delivery only; completion signalling is a no-op and every target is treated as available. */
@@ -125,20 +134,34 @@ public final class Injector {
*/
public Injector(AgentControl agents, TurnListener turnListener, Predicate<String> ready,
Consumer<String> forget) {
this(agents, turnListener, ready, forget, System::currentTimeMillis);
}
/** Full constructor — for tests: an injectable wall-clock supplier (fleetd #501). */
Injector(AgentControl agents, TurnListener turnListener, Predicate<String> ready,
Consumer<String> forget, LongSupplier nowMillis) {
this.agents = agents;
this.router = null;
this.turnListener = turnListener;
this.ready = ready;
this.forget = forget;
this.nowMillis = nowMillis;
}
public Injector(HerdrRouter router, TurnListener turnListener, Predicate<String> ready,
Consumer<String> forget) {
this(router, turnListener, ready, forget, System::currentTimeMillis);
}
/** Full constructor — for tests: an injectable wall-clock supplier (fleetd #501). */
Injector(HerdrRouter router, TurnListener turnListener, Predicate<String> ready,
Consumer<String> forget, LongSupplier nowMillis) {
this.agents = null;
this.router = router;
this.turnListener = turnListener;
this.ready = ready;
this.forget = forget;
this.nowMillis = nowMillis;
}
private AgentControl agentsFor(String target) {
@@ -209,6 +232,7 @@ public final class Injector {
int unknownSinceTurn; // consecutive `unknown` samples while a delegation is outstanding (CB-109)
int unknownSincePostTurn; // the same, for the post-turn housekeeping phase (fleetd #306)
int notReadySincePoll; // consecutive injectable samples a queued message waited on the readiness gate (CB-114)
long notReadySinceMillis; // wall-clock time of the FIRST non-ready sample in the current notReadySincePoll streak (fleetd #501); reset alongside it
boolean postTurnPending; // completion observed; adapter housekeeping has not started yet
boolean awaitingPostTurnPickup;
boolean postTurnObserved;
@@ -302,6 +326,7 @@ public final class Injector {
t.unknownSinceTurn = 0;
t.unknownSincePostTurn = 0;
t.notReadySincePoll = 0;
t.notReadySinceMillis = 0;
if (t.awaitingCompletion) t.turnObserved = true;
} else if (status.injectable()) { // IDLE or BLOCKED
t.unknownSinceTurn = 0;
@@ -353,6 +378,7 @@ public final class Injector {
Pending p = t.queue.peek();
if (p != null && ready.test(target)) {
t.notReadySincePoll = 0;
t.notReadySinceMillis = 0;
try {
agentsFor(target).send(target, p.text());
t.queue.poll();
@@ -370,23 +396,52 @@ public final class Injector {
sent = p;
sendError = e;
}
} else if (p != null && ++t.notReadySincePoll >= READINESS_GRACE_POLLS) {
// The worker has been idle-but-not-ready for the whole grace: its Claude
// never connected the bridge MCP (crashed during boot, or wedged on a
// startup prompt). The readiness gate would hold this message forever, so
// fail every queued message and release the target (CB-114) instead of
// polling it indefinitely with the caller's future never completing.
notReady = new ArrayList<>(t.queue);
for (Pending pending : notReady) {
pending.state = Pending.State.NOT_DELIVERED;
} else if (p != null) {
// fleetd #501: stamp the wall-clock time of the FIRST non-ready sample in
// this streak, so the expiry log below can print how long the target
// actually sat non-ready — not just how many polls that took.
if (t.notReadySincePoll == 0) {
t.notReadySinceMillis = nowMillis.getAsLong();
}
if (++t.notReadySincePoll >= READINESS_GRACE_POLLS) {
// The worker has been idle-but-not-ready for the whole grace: its Claude
// never connected the bridge MCP (crashed during boot, or wedged on a
// startup prompt). The readiness gate would hold this message forever, so
// fail every queued message and release the target (CB-114) instead of
// polling it indefinitely with the caller's future never completing.
notReady = new ArrayList<>(t.queue);
for (Pending pending : notReady) {
pending.state = Pending.State.NOT_DELIVERED;
}
// fleetd #501: t.notReadySincePoll — the loop's own counter, already in
// scope — is printed here instead of the READINESS_GRACE_POLLS constant.
// On this branch the counter has JUST reached the threshold, so the two
// agree by construction and no test can tell them apart. Printed anyway:
// it gives this line one source of truth instead of two, so a later
// change to the loop above cannot leave this message reporting a number
// the loop no longer produces.
//
// elapsedMillis is a different case: it is NOT equal-by-construction to
// the truth. notReadySincePoll only increments on a sample that reaches
// this branch (p != null, not ready) — a poll that misses that condition
// advances real time without advancing the counter — and this loop's real
// period is not guaranteed to equal POLL_INTERVAL_MILLIS (load, or a host
// sleep, can widen the real gap far past it). READINESS_GRACE_POLLS *
// POLL_INTERVAL_MILLIS / 1000 is arithmetic on two constants, not a
// measurement, so it stays here only as the labelled CONFIGURED budget,
// never presented as elapsed time.
long elapsedMillis = nowMillis.getAsLong() - t.notReadySinceMillis;
log.warn("readiness grace for {} expired after {} polls (configured={} "
+ "polls/{}s elapsed={}ms): target never became "
+ "deliverable, so failing {} queued message(s) that "
+ "never reached its pane",
target, t.notReadySincePoll, READINESS_GRACE_POLLS,
READINESS_GRACE_POLLS * POLL_INTERVAL_MILLIS / 1000, elapsedMillis,
notReady.size());
t.queue.clear();
t.notReadySincePoll = 0;
t.notReadySinceMillis = 0;
}
log.warn("readiness grace for {} expired after {} polls ({}s): target never "
+ "became deliverable, so failing {} queued message(s) that never "
+ "reached its pane",
target, READINESS_GRACE_POLLS,
READINESS_GRACE_POLLS * POLL_INTERVAL_MILLIS / 1000, notReady.size());
t.queue.clear();
t.notReadySincePoll = 0;
}
}
}
@@ -381,13 +381,18 @@ public final class LeadRollover {
*/
private void runRollover(PendingRollover p, FleetConfig.LeadRollover cfg) {
String lead = p.leadTerminal();
boolean turnSettled = waitUntilAtTurnBoundary(lead, cfg.turnSettleSeconds());
if (!turnSettled) {
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) within {}s "
+ "after confirm() — refusing to send /clear at all; the calling lead's "
+ "own turn is still live and clearing it now would destroy live context "
+ "(token={})",
lead, cfg.turnSettleSeconds(), p.token());
long rollStartMillis = nowMillis.getAsLong();
TurnSettleResult turnResult = waitUntilAtTurnBoundary(lead, cfg.turnSettleSeconds());
if (!turnResult.settled()) {
// fleetd #494 follow-up: this line had the SAME defect as the /clear-timeout line below
// — cfg.turnSettleSeconds() is the CONFIGURED budget, not how long this wait actually
// ran. Print the measured elapsed time alongside it, labelled, exactly like the /clear
// path already does.
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) after "
+ "confirm() — refusing to send /clear at all; the calling lead's own "
+ "turn is still live and clearing it now would destroy live context "
+ "(token={}, configured={}s elapsed={}ms)",
lead, p.token(), cfg.turnSettleSeconds(), turnResult.elapsedMillis());
return;
}
@@ -395,15 +400,22 @@ public final class LeadRollover {
// /clear is housekeeping, not a delegated turn, and routing it through Injector wedges the
// pane forever (see this class's javadoc).
agents.send(lead, "/clear");
boolean clearSettled = waitForClearPickupAndSettle(lead, cfg.clearSettleSeconds());
if (!clearSettled) {
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) within {}s "
+ "after /clear — NOT sending bootstrapText (token={})",
lead, cfg.clearSettleSeconds(), p.token());
ClearSettleResult clearResult = waitForClearPickupAndSettle(lead, cfg.clearSettleSeconds());
if (!clearResult.settled()) {
// fleetd #494: cfg.clearSettleSeconds() is the CONFIGURED budget, not how long the wait
// actually ran — an operator reading only that number wrongly believes it is a measured
// duration. Print the measured elapsed time and nudge count alongside it, each labelled,
// so the two can be compared at a glance.
log.warn("lead-rollover: pane {} did not reach a turn boundary (IDLE or DONE) after "
+ "/clear — NOT sending bootstrapText (token={}, configured={}s "
+ "elapsed={}ms nudges={})",
lead, p.token(), cfg.clearSettleSeconds(), clearResult.elapsedMillis(),
clearResult.nudges());
return;
}
agents.send(lead, cfg.bootstrapTextFor(p.handoverPath()));
log.info("lead-rollover: rolled token={} lead={}", p.token(), lead);
long rollElapsedMillis = nowMillis.getAsLong() - rollStartMillis;
log.info("lead-rollover: rolled token={} lead={} elapsedMs={}", p.token(), lead, rollElapsedMillis);
}
/** Drop a pending request without rolling. @return whether a pending request existed for {@code token} */
@@ -475,9 +487,15 @@ public final class LeadRollover {
* gate exists to prevent. Do not "simplify" this back to {@code injectable()}. ({@link
* #waitForClearPickupAndSettle} keeps the same exclusion of {@code BLOCKED}, for the same
* reason, on the second wait.)
*
* @return a {@link TurnSettleResult} whose {@code settled()} is {@code true} once a real
* boundary was observed, {@code false} if {@code settleSeconds} elapses first.
* {@code elapsedMillis()} is a MEASURED value from the injected {@link #nowMillis}
* clock, never the configured {@code settleSeconds} budget (fleetd #494 follow-up).
*/
private boolean waitUntilAtTurnBoundary(String target, int settleSeconds) {
long deadline = nowMillis.getAsLong() + TimeUnit.SECONDS.toMillis(settleSeconds);
private TurnSettleResult waitUntilAtTurnBoundary(String target, int settleSeconds) {
long startMillis = nowMillis.getAsLong();
long deadline = startMillis + TimeUnit.SECONDS.toMillis(settleSeconds);
while (nowMillis.getAsLong() < deadline) {
AgentStatus status;
try {
@@ -488,13 +506,20 @@ public final class LeadRollover {
status = null;
}
if (status == AgentStatus.IDLE || status == AgentStatus.DONE) {
return true;
return new TurnSettleResult(true, nowMillis.getAsLong() - startMillis);
}
settleSleeper.run();
}
return false;
return new TurnSettleResult(false, nowMillis.getAsLong() - startMillis);
}
/**
* The measured outcome of {@link #waitUntilAtTurnBoundary} — fleetd #494 follow-up. The sibling
* of {@link ClearSettleResult} for the FIRST wait, which never nudges, so it carries no nudge
* count.
*/
private record TurnSettleResult(boolean settled, long elapsedMillis) {}
/**
* The SECOND wait in {@link #runRollover} — after {@code /clear} has been sent, waits for it to
* settle, bounded by {@code settleSeconds}. <strong>fleetd #489 — the paste-race fix.</strong>
@@ -524,8 +549,9 @@ public final class LeadRollover {
* Claude Code prompt is a no-op, so repeating it is safe;</li>
* <li>the {@code PICKUP_GRACE_POLLS}th consecutive such poll, with {@code WORKING} still never
* observed, releases rather than wedges the roll instead of nudging again — the same
* choice {@code Injector} makes — and returns {@code true} anyway, logged at {@code info}
* so an operator can see which path ran;</li>
* choice {@code Injector} makes — and returns {@code settled() == true} anyway, logged at
* {@code warn} with the measured elapsed time (fleetd #494) so an operator can see which
* path ran and how long it actually took;</li>
* <li>once {@code WORKING} has been observed, nudging stops and this instead waits for a real
* {@code working → IDLE/DONE} completion boundary before returning {@code true}.</li>
* </ul>
@@ -541,15 +567,20 @@ public final class LeadRollover {
* swallowed and logged at {@code debug}, exactly like {@code Injector.java:437-442} — a failed
* nudge must not abort the roll.
*
* @return {@code true} once {@code /clear} has settled, or once the nudge budget was exhausted
* with no pickup ever observed (released rather than wedged); {@code false} if {@code
* settleSeconds} elapses first — the caller must NOT send {@code bootstrapText} in that
* case, exactly as before this fix
* @return a {@link ClearSettleResult} whose {@code settled()} is {@code true} once {@code
* /clear} has settled, or once the nudge budget was exhausted with no pickup ever
* observed (released rather than wedged); {@code false} if {@code settleSeconds} elapses
* first — the caller must NOT send {@code bootstrapText} in that case, exactly as before
* this fix. {@code elapsedMillis()} and {@code nudges()} are MEASURED values (from the
* injected {@link #nowMillis} clock and an actual nudge count), never the configured
* {@code settleSeconds} budget (fleetd #494).
*/
private boolean waitForClearPickupAndSettle(String target, int settleSeconds) {
long deadline = nowMillis.getAsLong() + TimeUnit.SECONDS.toMillis(settleSeconds);
private ClearSettleResult waitForClearPickupAndSettle(String target, int settleSeconds) {
long startMillis = nowMillis.getAsLong();
long deadline = startMillis + TimeUnit.SECONDS.toMillis(settleSeconds);
boolean pickedUp = false; // a WORKING sample has been observed since /clear was sent
int idlePollsAwaitingPickup = 0;
int nudges = 0;
while (nowMillis.getAsLong() < deadline) {
AgentStatus status;
try {
@@ -563,26 +594,56 @@ public final class LeadRollover {
pickedUp = true;
} else if (status == AgentStatus.IDLE || status == AgentStatus.DONE) {
if (pickedUp) {
return true; // a real WORKING -> IDLE/DONE completion boundary
// a real WORKING -> IDLE/DONE completion boundary
return new ClearSettleResult(true, nowMillis.getAsLong() - startMillis, nudges);
}
if (++idlePollsAwaitingPickup >= PICKUP_GRACE_POLLS) {
log.info("lead-rollover: /clear on {} was never observed as WORKING after {} "
long elapsedMillis = nowMillis.getAsLong() - startMillis;
// fleetd #494: this release trades a possibly-unsubmitted /clear for progress
// instead of wedging the roll — that trade is deliberate and stays. But it is
// also exactly the case that reported false success in the real incident (the
// whole roll "succeeded" after 438ms of a 20s budget), so raise it to WARN and
// print the MEASURED elapsed time next to the target pane, not just the count.
//
// fleetd #494 follow-up (2nd pass): BOTH numbers in this line must come from
// the loop's own counters, never from the PICKUP_GRACE_POLLS constant.
// `idlePollsAwaitingPickup` and `nudges` each have exactly one write site in
// this loop, on the same branch, so on this branch they cannot differ from
// PICKUP_GRACE_POLLS / PICKUP_GRACE_POLLS - 1 today — no test can prove the
// difference on this line, and printing the counters does not change that.
// What it does buy: one source of truth instead of two, so a later change to
// the loop (an early return, a second increment site, a different exit
// condition) cannot leave this message reporting a number the loop no longer
// produces. The place where `nudges` genuinely varies with the run — and is
// covered by a test that can tell it apart from a constant — is the
// /clear-timeout warn in runRollover, which prints clearResult.nudges().
log.warn("lead-rollover: /clear on {} was never observed as WORKING after {} "
+ "consecutive IDLE/DONE polls ({} of those were nudged) — "
+ "releasing rather than wedging the roll",
target, PICKUP_GRACE_POLLS, PICKUP_GRACE_POLLS - 1);
return true;
+ "releasing rather than wedging the roll (elapsed={}ms)",
target, idlePollsAwaitingPickup, nudges, elapsedMillis);
return new ClearSettleResult(true, elapsedMillis, nudges);
}
try {
agents.submit(target); // nudge a raced Enter (CB-113) so /clear actually submits
} catch (RuntimeException e) {
log.debug("lead-rollover: resubmit to {} failed (will retry next poll): {}",
target, e.getMessage());
} finally {
nudges++; // an attempted nudge, whether or not the submit call itself threw
}
}
// AgentStatus.BLOCKED or UNKNOWN (or an unreadable status, above): neither a pickup
// signal nor a boundary — keep polling without nudging or releasing.
settleSleeper.run();
}
return false;
return new ClearSettleResult(false, nowMillis.getAsLong() - startMillis, nudges);
}
/**
* The measured outcome of {@link #waitForClearPickupAndSettle} — fleetd #494. Carries the
* MEASURED elapsed time (from the injected {@link #nowMillis} clock) and nudge count alongside
* the settle/timeout decision, so callers can log them instead of the configured budget, which
* is not how long the wait actually ran.
*/
private record ClearSettleResult(boolean settled, long elapsedMillis, int nudges) {}
}
@@ -33,9 +33,10 @@ public final class ConnectionIdentity {
/**
* The caller resolved from the connection: its worker {@code terminal} (or {@code null} for the
* primary / an off-host client) and its {@code pid} (or {@code -1} if not resolvable).
* primary / an off-host client), its {@code pid} (or {@code -1} if not resolvable), and whether
* the pane scan behind {@code terminal} ran to completion ({@link #scanComplete}).
*/
public record Caller(String terminal, long pid) {
public record Caller(String terminal, long pid, boolean scanComplete) {
/**
* Whether the OS peer-PID lookup actually succeeded — {@code false} means {@code pid} is
@@ -51,6 +52,10 @@ public final class ConnectionIdentity {
* {@link ConnectionIdentity#isLoopback} is centralised rather than left for each caller to
* reimplement: a raw {@code pid > 0} check duplicated at every call site is precisely the
* "one rule, two copies" shape that let #305 drift.
*
* <p>This method is deliberately NOT widened for fleetd #505's failure (a herdr error
* during the pane scan, not a failed lsof lookup) — it still tests only the sentinel it is
* named for. #505 is a different axis, carried separately in {@link #scanComplete}.
*/
public boolean resolved() {
return pid > 0;
@@ -60,10 +65,11 @@ public final class ConnectionIdentity {
/** Resolve the caller's terminal and PID from one peer-PID lookup. */
public Caller resolve(String remoteAddr, int remotePort) {
if (!isLoopback(remoteAddr)) {
return new Caller(null, -1); // only same-host callers can be workers
return new Caller(null, -1, true); // only same-host callers can be workers
}
long pid = pids.pidForLocalPort(remotePort);
return new Caller(panes.terminalForPid(pid), pid);
PaneLocator.Lookup lookup = panes.terminalForPid(pid);
return new Caller(lookup.terminal(), pid, lookup.complete());
}
/**
@@ -93,7 +93,18 @@ public final class FleetMcp {
private final HttpServletStreamableServerTransportProvider transport;
private final McpSyncServer server;
private final CallerResolver authz; // CB-501: null → authorization not enforced (legacy)
/**
* fleetd #518: whether {@link #denyFor} enforces the CB-505 policy table at all. Replaces the
* old {@code CallerResolver authz} field, whose null-ness used to decide BOTH this AND which
* principal-resolution code path {@link #contextExtractor} ran — reaching "authorization off"
* by simply not passing a {@link CallerResolver} also meant the resolved {@link Principal}
* came from a second, separately-maintained heuristic ({@code legacyPrincipal}, now deleted)
* that nothing ever exercised. There is now exactly one resolution path ({@code callers},
* required and non-null below) and a separate, explicitly-chosen {@link AuthorizationMode}
* for this flag — so a caller can turn enforcement off without silently swapping in a second,
* untested identity heuristic.
*/
private final boolean authorizationEnforced;
private final Metrics metrics; // CB-502: null → auth failures not counted
private final CapacitySource capacity;
private final HealthCoverageSource healthCoverage;
@@ -250,6 +261,17 @@ public final class FleetMcp {
public static CoordinationSource none() { return new CoordinationSource(null, List.of()); }
}
/**
* fleetd #518: whether {@link #denyFor} enforces the CB-505 policy table. A required
* constructor parameter with no default, so "authorization is off" can only be reached by a
* caller explicitly saying so — never by omitting a {@link CallerResolver} the way the old
* {@code callers == null} idiom allowed. {@code callers} itself is required either way: even
* under {@link #UNENFORCED}, the one real {@link CallerResolver} still resolves every caller's
* {@link Principal} (so {@code markSpawnedMemberPresent}/{@code recordPrimarySingleton} see a
* real identity), and {@link #denyFor} is the only thing that changes.
*/
public enum AuthorizationMode { ENFORCED, UNENFORCED }
/**
* The only constructor (fleetd #480 Unit C correction round). Every field below used to have
* its own defaulting overload — {@code leadChannel}/{@code outage}/{@code leadSeats}/
@@ -268,10 +290,16 @@ public final class FleetMcp {
* {@link OutageSource#none()}, {@link LeadSeatSource#none()}, {@code List.of()} are all still
* perfectly fine values, just never an implicit default reached by omission.
*
* @param callers resolves each call's {@link Principal}; {@code null} disables
* authorization. This surface needs its own enforcement: {@code /mcp} is a
* raw servlet on Jetty's context handler and never passes through
* Javalin's {@code before} filter, so the REST guard does not cover it.
* @param callers resolves each call's {@link Principal}. Required, never {@code null} —
* fleetd #518: use {@link AuthorizationMode#UNENFORCED} to disable
* enforcement, not a missing resolver. This surface needs its own
* enforcement: {@code /mcp} is a raw servlet on Jetty's context handler and
* never passes through Javalin's {@code before} filter, so the REST guard
* does not cover it.
* @param authorizationMode fleetd #518: whether {@link #denyFor} enforces the CB-505 policy
* table ({@link AuthorizationMode#ENFORCED}) or leaves the gate open
* ({@link AuthorizationMode#UNENFORCED}, for the pre-CB-513 test suite that
* does not exercise authorization). Required, with no default.
* @param metrics registry for auth-failure counting; may be {@code null}
* @param quarantine CB-578 stage B facts for {@code fleet_profiles}; pass
* {@link QuarantineSource#none()} for a caller that does not want the
@@ -301,9 +329,13 @@ public final class FleetMcp {
*/
public FleetMcp(MessageService messages, PeerLauncher workers, SessionManager sessions,
ConnectionIdentity identity, MemberPresence presence, PrimaryRegistry primaryRegistry,
CallerResolver callers, Metrics metrics, CapacitySource capacity, HealthCoverageSource healthCoverage,
CallerResolver callers, AuthorizationMode authorizationMode, Metrics metrics,
CapacitySource capacity, HealthCoverageSource healthCoverage,
QuarantineSource quarantine, LeadChannel leadChannel, OutageSource outage,
LeadSeatSource leadSeats, List<String> peers, LeadRollover leadRollover) {
Objects.requireNonNull(callers, "callers");
this.authorizationEnforced = Objects.requireNonNull(authorizationMode, "authorizationMode")
== AuthorizationMode.ENFORCED;
this.leadChannel = leadChannel;
this.peers = peers == null ? List.of() : List.copyOf(peers);
this.capacity = capacity;
@@ -322,11 +354,11 @@ public final class FleetMcp {
// (CB-113) — its MCP initialize is the reliable "the agent is up" signal.
.contextExtractor(req -> {
// One resolution per call, shared with the REST surface via CallerResolver so
// the two paths cannot drift on who a caller is.
Principal p = callers != null
? callers.resolve(req.getRemoteAddr(), req.getRemotePort(),
req.getHeader("Authorization"))
: legacyPrincipal(identity, req.getRemoteAddr(), req.getRemotePort());
// the two paths cannot drift on who a caller is. fleetd #518: callers is
// required (never null) so there is no second, untested resolution path to
// fall back to here — AuthorizationMode governs enforcement, not identity.
Principal p = callers.resolve(req.getRemoteAddr(), req.getRemotePort(),
req.getHeader("Authorization"));
// CB-532: guard on the ROLE, not on the terminal being null. This excludes a
// lead, which carries its pane too, while including every spawned member role.
// Enrolling a lead would count it as an available member in the roster.
@@ -443,7 +475,7 @@ public final class FleetMcp {
McpSchema.CallToolResult denied = deny(exchange, toolAction("fleet_list", Map.of()), null);
if (denied != null) return denied;
return listFleet(workers, sessions, messages, capacity, healthCoverage, quarantine, outage,
leadSeats, callers == null ? Map.of() : callers.leads(),
leadSeats, callers.leads(),
callerTerminal(exchange),
new CoordinationSource(leadChannel, peers),
coordinatorVisibleTo(principal(exchange)));
@@ -522,21 +554,9 @@ public final class FleetMcp {
.toolCall(fleetWhoami, whoamiHandler)
.toolCall(fleetHandover, handoverHandler)
.build();
this.authz = callers;
this.metrics = metrics;
}
/**
* Pre-CB-501 identity: worker if the connection maps to a pane, otherwise the primary. Used
* only by the legacy constructor, where authorization is not enforced anyway.
*/
private static Principal legacyPrincipal(ConnectionIdentity identity, String addr, int port) {
ConnectionIdentity.Caller c = identity.resolve(addr, port);
return c.terminal() != null
? Principal.worker(c.terminal(), c.pid())
: Principal.primary(c.pid());
}
/** The caller reconstructed from the transport context. */
private static Principal principal(McpSyncServerExchange exchange) {
return principalFrom(exchange.transportContext().get(CALLER_ROLE),
@@ -591,8 +611,8 @@ public final class FleetMcp {
McpSchema.CallToolResult denyFor(Principal caller, Authz.Action action, String target) {
// The enforcement switch lives HERE rather than in the exchange-facing wrapper: any future
// tool that calls this directly must not be able to skip the gate by accident.
if (authz == null) {
return null; // legacy constructor: authorization not enforced
if (!authorizationEnforced) {
return null; // AuthorizationMode.UNENFORCED: authorization not enforced (fleetd #518)
}
if (Authz.permits(caller, action, target)) {
if (action != Authz.Action.READ) {
@@ -302,9 +302,10 @@ public final class SessionManager implements TurnListener {
* with no copy and no error. Do NOT fuse these back together; the cost of an orphaned worktree
* is a logged path an operator can reclaim, the cost of a deleted one is unrecoverable work.
*/
private void release(String paneId, ReleaseCause cause) {
private MemberSession release(String paneId, ReleaseCause cause) {
MemberSession removed = registry.remove(paneId);
releaseRemoved(paneId, removed, handles.remove(paneId), cause);
return removed;
}
/**
@@ -1064,16 +1065,39 @@ public final class SessionManager implements TurnListener {
* drain (see above), and a straggler must not buy the drain more time than the flag it lost the
* race against would have. In the ordinary case the sweep finds nothing and costs one empty
* {@link #roster()} call.
*
* <p>fleetd #512: a drain that releases every session cleanly used to log nothing at all — the
* only log calls in this method and {@link #drainSnapshot} sit on abnormal paths, so "nothing
* logged" was indistinguishable from "died on the first session". The {@code log.info} at the
* end below is a positive assertion that the drain actually finished, on the normal path,
* every time — including the all-zero case, which is a common and legitimate outcome (no
* members were live) and must still produce the line. Both {@link #drainSnapshot} passes (the
* main snapshot and the straggler sweep) are folded into the one line: a caller reading two
* lines could not tell a two-pass drain from two separate drains.
*/
void drainAll(long timeoutNanos) {
long deadline = System.nanoTime() + timeoutNanos;
draining.set(true);
drainSnapshot(roster(), deadline);
DrainTally tally = drainSnapshot(roster(), deadline);
List<MemberSession> stragglers = roster();
if (!stragglers.isEmpty()) {
log.warn("drain sweep found {} session(s) registered after the drain snapshot was "
+ "taken (raced past the shutdown guard); draining them too", stragglers.size());
drainSnapshot(stragglers, deadline);
tally = tally.plus(drainSnapshot(stragglers, deadline));
}
log.info("drain complete: released={} abandoned={} (still BUSY at the shutdown deadline)",
tally.released(), tally.abandoned());
}
/**
* Running count for one {@link #drainAll} invocation, folded across both {@link #drainSnapshot}
* passes (fleetd #512). {@code abandoned} counts sessions that were still {@code BUSY} at the
* moment they were released — i.e. the whole-drain deadline passed before they left {@code BUSY}
* on their own (see {@link #drainSnapshot}) — a subset of {@code released}, not additional to it.
*/
private record DrainTally(int released, int abandoned) {
private DrainTally plus(DrainTally other) {
return new DrainTally(released + other.released, abandoned + other.abandoned);
}
}
@@ -1081,8 +1105,12 @@ public final class SessionManager implements TurnListener {
* Drain exactly the sessions in {@code snapshot}, waiting out a {@code BUSY} one against the
* shared whole-drain {@code deadline} before releasing it. Shared by {@link #drainAll}'s main
* pass and its post-loop straggler sweep (fleetd #308) so both honor the same one budget.
* Returns how many sessions this pass released, and how many of those were still {@code BUSY}
* (abandoned mid-turn) at the moment of release.
*/
private void drainSnapshot(List<MemberSession> snapshot, long deadline) {
private DrainTally drainSnapshot(List<MemberSession> snapshot, long deadline) {
int released = 0;
int abandoned = 0;
for (MemberSession s : snapshot) {
try {
if (s.state() == MemberSession.State.BUSY) {
@@ -1100,11 +1128,16 @@ public final class SessionManager implements TurnListener {
}
}
}
release(s.paneId(), ReleaseCause.SHUTDOWN);
MemberSession removed = release(s.paneId(), ReleaseCause.SHUTDOWN);
released++;
if (removed != null && removed.state() == MemberSession.State.BUSY) {
abandoned++;
}
} catch (RuntimeException e) {
log.warn("drain failed for pane={}; continuing with remaining sessions", s.paneId(), e);
}
}
return new DrainTally(released, abandoned);
}
/**
@@ -0,0 +1,212 @@
package dev.ltms.fleet;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import com.fasterxml.jackson.databind.JsonNode;
import dev.ltms.fleet.herdr.HerdrClient;
import dev.ltms.fleet.herdr.HerdrException;
import org.junit.jupiter.api.Test;
import org.slf4j.LoggerFactory;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.LongSupplier;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
/**
* fleetd #498: {@code Fleetd.awaitHerdr} used to return a bare {@code boolean}, collapsing "the
* configured wait budget genuinely ran out" and "the waiting thread was interrupted, possibly
* milliseconds in" onto the same {@code false} — and the caller's log line printed only the
* configured budget, never how long the wait actually ran. This class covers both halves of the
* fix:
* <ul>
* <li>the seam — {@link Fleetd#awaitHerdr} itself, driven with an injected clock and a stub
* {@link HerdrClient}, one test per {@link Fleetd.HerdrWaitResult};</li>
* <li>the call site — {@link Fleetd#logHerdrWaitOutcomeAndShouldReap}, the exact decision {@code
* main} calls (extracted here because {@code main} itself boots the whole daemon and cannot
* be driven from a unit test), pinning the three distinct log messages it emits.</li>
* </ul>
* Every expected message below is a plain literal, not built from {@code HERDR_WAIT_SECONDS} or
* any other production constant — a test that derives its expectation the way the code does
* cannot see a change to either (fleetd #496's identical trap).
*/
class FleetdAwaitHerdrTest {
// ---- the seam: Fleetd.awaitHerdr ----------------------------------------------------------
@Test
void answeredReturnsImmediatelyWithZeroElapsedAndNeverPolls() {
HerdrStub herdr = new HerdrStub(0); // succeeds on the very first call
LongSupplier clock = fixedClock(1_000L);
AtomicBoolean polled = new AtomicBoolean(false);
Runnable poller = () -> polled.set(true);
Fleetd.HerdrAwaitOutcome outcome = Fleetd.awaitHerdr(herdr, clock, poller);
assertEquals(Fleetd.HerdrWaitResult.ANSWERED, outcome.result());
assertEquals(0L, outcome.elapsedNanos(), "a fixed clock must measure zero elapsed time");
assertFalse(polled.get(), "herdr answering on the first try must never poll");
}
@Test
void deadlinePassedIsMeasuredNotAssumed() {
HerdrStub herdr = new HerdrStub(-1); // never succeeds
// call order inside awaitHerdr: start, then per failed attempt: deadline-check, elapsed-calc
ScriptedClock clock = new ScriptedClock(0L, 30_500_000_000L, 30_500_000_000L);
Runnable poller = () -> fail("the deadline was already exceeded on the first attempt — must not poll");
Fleetd.HerdrAwaitOutcome outcome = Fleetd.awaitHerdr(herdr, clock, poller);
assertEquals(Fleetd.HerdrWaitResult.DEADLINE_PASSED, outcome.result());
assertEquals(30_500_000_000L, outcome.elapsedNanos(),
"elapsed must be the MEASURED clock delta, not the configured budget");
}
@Test
void interruptedIsDistinctFromDeadlinePassedAndPreservesTheInterruptFlag() {
HerdrStub herdr = new HerdrStub(-1); // never succeeds
// start=0, deadline-check returns 500ms (well under the 30s budget) -> not deadline-passed,
// then the poller interrupts, and the elapsed-calc call returns 750ms.
ScriptedClock clock = new ScriptedClock(0L, 500_000_000L, 750_000_000L);
Runnable poller = () -> Thread.currentThread().interrupt();
try {
Fleetd.HerdrAwaitOutcome outcome = Fleetd.awaitHerdr(herdr, clock, poller);
assertEquals(Fleetd.HerdrWaitResult.INTERRUPTED, outcome.result());
assertEquals(750_000_000L, outcome.elapsedNanos(),
"elapsed must be measured even when the wait ends via interruption, not the deadline");
assertTrue(Thread.currentThread().isInterrupted(),
"the interrupt flag the old code re-set must still be set on return");
} finally {
Thread.interrupted(); // clear it so it cannot leak into another test on this thread
}
}
// ---- the call site: Fleetd.logHerdrWaitOutcomeAndShouldReap -------------------------------
@Test
void answeredLogsNothingAndSaysReap() {
ListAppender<ILoggingEvent> events = attach();
try {
boolean shouldReap = Fleetd.logHerdrWaitOutcomeAndShouldReap(
new Fleetd.HerdrAwaitOutcome(Fleetd.HerdrWaitResult.ANSWERED, 0L));
assertTrue(shouldReap, "only ANSWERED should tell main to reap orphan workers");
assertEquals(0, events.list.size(), "the answered path logs nothing itself");
} finally {
detach(events);
}
}
@Test
void deadlinePassedLogsConfiguredAndMeasuredElapsedTogether() {
ListAppender<ILoggingEvent> events = attach();
try {
boolean shouldReap = Fleetd.logHerdrWaitOutcomeAndShouldReap(
new Fleetd.HerdrAwaitOutcome(Fleetd.HerdrWaitResult.DEADLINE_PASSED, 30_500_000_000L));
assertFalse(shouldReap, "a deadline-passed wait must not tell main to reap");
assertEquals(1, events.list.size());
ILoggingEvent event = events.list.getFirst();
assertEquals(Level.WARN, event.getLevel());
assertEquals("herdr did not answer within the configured wait (configured=30s "
+ "elapsed=30500ms) — starting anyway; /healthz will report degraded until it "
+ "comes up. Orphaned worker panes (if any) were NOT reaped.",
event.getFormattedMessage());
} finally {
detach(events);
}
}
@Test
void interruptedLogsItsOwnMessageAndNeverClaimsTheBudgetElapsed() {
ListAppender<ILoggingEvent> events = attach();
try {
// 3ms: the ticket's own example of "a few milliseconds in", not the 30s budget.
boolean shouldReap = Fleetd.logHerdrWaitOutcomeAndShouldReap(
new Fleetd.HerdrAwaitOutcome(Fleetd.HerdrWaitResult.INTERRUPTED, 3_000_000L));
assertFalse(shouldReap, "an interrupted wait must not tell main to reap");
assertEquals(1, events.list.size());
ILoggingEvent event = events.list.getFirst();
assertEquals(Level.WARN, event.getLevel());
String message = event.getFormattedMessage();
assertEquals("herdr wait was interrupted before the configured wait ran out "
+ "(configured=30s elapsed=3ms) — starting anyway; /healthz will report "
+ "degraded until it comes up. Orphaned worker panes (if any) were NOT reaped.",
message);
assertFalse(message.contains("did not answer"),
"an interrupted wait must not be reported as if herdr failed to answer within the budget");
} finally {
detach(events);
}
}
// ---- fixtures --------------------------------------------------------------------------
/** Always returns the same value, i.e. a clock that measures zero elapsed time. */
private static LongSupplier fixedClock(long value) {
return () -> value;
}
/** Returns each value in order, then repeats the last one for any call beyond the list. */
private static final class ScriptedClock implements LongSupplier {
private final long[] values;
private int index;
ScriptedClock(long... values) {
this.values = values;
}
@Override
public long getAsLong() {
long v = values[Math.min(index, values.length - 1)];
if (index < values.length - 1) {
index++;
}
return v;
}
}
/** Fails {@code failuresBeforeSuccess} times, then succeeds forever; {@code -1} never succeeds. */
private static final class HerdrStub implements HerdrClient {
private final int failuresBeforeSuccess;
private int calls;
HerdrStub(int failuresBeforeSuccess) {
this.failuresBeforeSuccess = failuresBeforeSuccess;
}
@Override
public JsonNode call(String method, Object params) throws HerdrException {
calls++;
if (failuresBeforeSuccess < 0 || calls <= failuresBeforeSuccess) {
throw new HerdrException("herdr not up yet");
}
return null;
}
@Override
public void close() {
}
}
private static ListAppender<ILoggingEvent> attach() {
Logger logger = (Logger) LoggerFactory.getLogger(Fleetd.class);
logger.setLevel(Level.DEBUG);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
return appender;
}
private static void detach(ListAppender<ILoggingEvent> appender) {
((Logger) LoggerFactory.getLogger(Fleetd.class)).detachAppender(appender);
}
}
@@ -186,6 +186,43 @@ class CallerResolverTest {
assertEquals(Role.PRIMARY, r.resolve("127.0.0.1", 99, "BEARER s3cret").role());
}
// ── fleetd #505: a herdr error DURING THE SCAN must not be conflated with "not a worker" ──────
// #317 (above) covers a failed lsof lookup. This is the other input to the same decision: the
// lsof lookup succeeds (a real pid), but PaneLocator's own pane scan hits a herdr error on the
// pane that owns that pid — so c.resolved() is true and c.terminal() is null, exactly like a
// real primary. c.scanComplete() is what tells them apart.
/**
* The discriminating case named in the ticket: the error must land on the pane that DOES own
* the caller's pid, or the test proves nothing (any other pane's failure is invisible to the
* scan's outcome, since a match found elsewhere is definitive regardless).
*/
@Test
void aHerdrErrorOnTheOwningPaneDuringTheScanIsRefusedNotPromotedToPrimary() {
FakeHerdr failing = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient");
ConnectionIdentity incomplete = new ConnectionIdentity(new PaneLocator(failing), _ -> FakeHerdr.WORKER_PID);
Principal p = new CallerResolver(incomplete).resolve("127.0.0.1", 55555, null);
assertEquals(Role.ANONYMOUS, p.role(),
"an incomplete pane scan must never be read as a clean negative and promoted to primary");
}
/**
* The companion invariant: a herdr error on a DIFFERENT, non-owning pane must not turn every
* mid-scan teardown into a refusal — the real match is still found and resolves as a worker.
*/
@Test
void aHerdrErrorOnANonOwningPaneStillResolvesTheRealWorker() {
FakeHerdr vanishedElsewhere = new FakeHerdr().processInfoFailsForPane("w2:p9", "pane_not_found");
ConnectionIdentity id = new ConnectionIdentity(new PaneLocator(vanishedElsewhere), _ -> FakeHerdr.WORKER_PID);
Principal p = new CallerResolver(id).resolve("127.0.0.1", 55555, null);
assertEquals(Role.WORKER, p.role());
assertEquals("term_a", p.terminal());
}
@Test
void aNonLoopbackCallerIsNeverThePrimaryUnderLoopbackTrust() {
// Defence in depth: startup already refuses this pairing (validateAuthExposure), but if a
@@ -44,6 +44,7 @@ public final class FakeHerdr implements HerdrClient {
private int workerTabPaneCount = 1;
private String paneCloseErrorCode = null;
private final Map<String, String> paneCloseErrorCodeFor = new ConcurrentHashMap<>();
private final Map<String, String> processInfoErrorCodeFor = new ConcurrentHashMap<>();
private String tabCloseErrorCode = null;
private final Map<String, String> tabCloseErrorCodeFor = new ConcurrentHashMap<>();
private String agentSendErrorCode = null;
@@ -144,6 +145,18 @@ public final class FakeHerdr implements HerdrClient {
return this;
}
/**
* Make {@code pane.process_info} fail with this herdr error code, but only for the given
* {@code pane_id} — every other pane's {@code pane.process_info} still succeeds. Models a
* transient herdr failure partway through a {@link PaneLocator} pid→pane scan (fleetd #505):
* the scan must be able to tell "this pane does not own the pid" apart from "the scan could
* not check this pane at all", instead of collapsing both into one {@code false}.
*/
public FakeHerdr processInfoFailsForPane(String paneId, String code) {
this.processInfoErrorCodeFor.put(paneId, code);
return this;
}
/** Set the {@code agent_status} that {@code agent.get} reports (drives the injector). */
public FakeHerdr agentStatus(String status) {
this.agentStatus = status;
@@ -407,6 +420,13 @@ public final class FakeHerdr implements HerdrClient {
{"pane_id":"w2:p9","terminal_id":"term_shell","workspace_id":"w2","tab_id":"w2:t8"}]}""");
case "pane.process_info" -> {
Object paneId = params instanceof java.util.Map<?, ?> m ? m.get("pane_id") : null;
String failCode = paneId == null ? null
: processInfoErrorCodeFor.get(String.valueOf(paneId));
if (failCode != null) {
throw new HerdrException(
"herdr error [" + failCode + "]: pane.process_info failed",
failCode, null);
}
yield "w2:p7".equals(paneId)
? mapper.readTree(("""
{"type":"pane_process_info","process_info":{"pane_id":"w2:p7","shell_pid":%d,
@@ -37,7 +37,7 @@ class PaneLocatorContractTest {
.path("pane").path("terminal_id").asText(null);
assertNotNull(terminalId, "seed pane should carry a terminal_id");
assertEquals(terminalId, new PaneLocator(herdr).terminalForPid(shellPid),
assertEquals(terminalId, new PaneLocator(herdr).terminalForPid(shellPid).terminal(),
"a real PID must resolve back to its own pane's terminal_id");
} finally {
spaces.closeTab(tab.tab().tabId());
@@ -15,18 +15,20 @@ class PaneLocatorTest {
@Test
void resolvesTerminalForAForegroundPid() {
assertEquals("term_a", loc.terminalForPid(FakeHerdr.WORKER_PID));
assertEquals("term_a", loc.terminalForPid(FakeHerdr.WORKER_PID).terminal());
}
@Test
void nullForAPidInNoPane() {
assertNull(loc.terminalForPid(999_999));
PaneLocator.Lookup outcome = loc.terminalForPid(999_999);
assertNull(outcome.terminal());
assertTrue(outcome.complete(), "a full, error-free scan that finds no match is complete");
}
@Test
void nullForNonPositivePid() {
assertNull(loc.terminalForPid(0));
assertNull(loc.terminalForPid(-1));
assertNull(loc.terminalForPid(0).terminal());
assertNull(loc.terminalForPid(-1).terminal());
}
// --- two-daemon fallback (CB-185) -----------------------------------------
@@ -38,7 +40,7 @@ class PaneLocatorTest {
HerdrClient lead = new FakeHerdr().withNoPanes();
HerdrClient member = new FakeHerdr();
PaneLocator two = new PaneLocator(lead, member);
assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID));
assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID).terminal());
}
@Test
@@ -48,13 +50,13 @@ class PaneLocatorTest {
HerdrClient lead = new FakeHerdr();
HerdrClient member = new FakeHerdr().withNoPanes();
PaneLocator two = new PaneLocator(lead, member);
assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID));
assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID).terminal());
}
@Test
void nullWhenNeitherClientHasTheMatch() {
PaneLocator two = new PaneLocator(new FakeHerdr().withNoPanes(), new FakeHerdr().withNoPanes());
assertNull(two.terminalForPid(FakeHerdr.WORKER_PID));
assertNull(two.terminalForPid(FakeHerdr.WORKER_PID).terminal());
}
@Test
@@ -63,7 +65,7 @@ class PaneLocatorTest {
// must behave exactly like the one-arg constructor, including making only one herdr call.
FakeHerdr shared = new FakeHerdr();
PaneLocator two = new PaneLocator(shared, shared);
assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID));
assertEquals("term_a", two.terminalForPid(FakeHerdr.WORKER_PID).terminal());
long paneListCalls = shared.calls.stream().filter(c -> c.method().equals("pane.list")).count();
assertEquals(1, paneListCalls, "same-object lead/member must scan exactly once, not twice");
}
@@ -75,7 +77,7 @@ class PaneLocatorTest {
// Regression: a pid with no parent chain at all — no ancestry walk is needed to match it.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
PaneLocator loc = new PaneLocator(pane, new FakeParentResolver());
assertEquals("term_x", loc.terminalForPid(5000));
assertEquals("term_x", loc.terminalForPid(5000).terminal());
}
@Test
@@ -83,7 +85,7 @@ class PaneLocatorTest {
// Regression: same as above, but matching via the foreground-processes list.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
PaneLocator loc = new PaneLocator(pane, new FakeParentResolver());
assertEquals("term_x", loc.terminalForPid(6000));
assertEquals("term_x", loc.terminalForPid(6000).terminal());
}
@Test
@@ -97,7 +99,7 @@ class PaneLocatorTest {
.parent(7002, 7001) // grandchild -> child
.parent(7001, 5000); // child -> shell (the pane's shell_pid)
PaneLocator loc = new PaneLocator(pane, parents);
assertEquals("term_x", loc.terminalForPid(7002));
assertEquals("term_x", loc.terminalForPid(7002).terminal());
}
@Test
@@ -110,7 +112,7 @@ class PaneLocatorTest {
.parent(9002, 9001)
.parent(9001, 9000); // chain never reaches 5000 or 6000
PaneLocator loc = new PaneLocator(pane, parents);
assertNull(loc.terminalForPid(9002));
assertNull(loc.terminalForPid(9002).terminal());
}
@Test
@@ -122,7 +124,7 @@ class PaneLocatorTest {
.parent(100, 101)
.parent(101, 100); // cycle, never reaches the pane's pids
PaneLocator loc = new PaneLocator(pane, parents);
assertNull(loc.terminalForPid(100));
assertNull(loc.terminalForPid(100).terminal());
}
@Test
@@ -141,11 +143,69 @@ class PaneLocatorTest {
};
HerdrClient noPanes = new FakeHerdr().withNoPanes();
PaneLocator two = new PaneLocator(noPanes, pane, counting);
assertEquals("term_x", two.terminalForPid(7002));
assertEquals("term_x", two.terminalForPid(7002).terminal());
assertEquals(3, calls.get(), "ancestry must be walked once (3 lookups: 7002, 7001, 5000), "
+ "not re-walked per herdr client");
}
// --- fleetd #505: a herdr error during the scan must not read as a clean negative ---------
@Test
void anErrorOnThePaneThatOwnsThePidMakesTheScanIncompleteNotAClearNegative() {
// The discriminating case: pane.process_info fails for exactly the pane that DOES own the
// caller's pid ("w2:p7", term_a). Before the fix, that failure was swallowed into a plain
// "does not own it" and the scan finished with a clean-looking null — indistinguishable
// from a real primary. It must now report incomplete, not a definite null.
FakeHerdr herdr = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient");
PaneLocator loc = new PaneLocator(herdr);
PaneLocator.Lookup outcome = loc.terminalForPid(FakeHerdr.WORKER_PID);
assertNull(outcome.terminal(), "the failing pane's ownership could not be confirmed");
assertFalse(outcome.complete(),
"a scan that could not check the owning pane must not report as complete");
}
@Test
void aVanishedPaneThatIsNotTheMatchLeavesAnOtherwiseSuccessfulScanComplete() {
// The companion invariant: a DIFFERENT pane (not the caller's own) failing mid-scan must
// not turn every mid-scan teardown into a refusal — the real match is still found, and the
// scan is still reported complete.
FakeHerdr herdr = new FakeHerdr().processInfoFailsForPane("w2:p9", "pane_not_found");
PaneLocator loc = new PaneLocator(herdr);
PaneLocator.Lookup outcome = loc.terminalForPid(FakeHerdr.WORKER_PID);
assertEquals("term_a", outcome.terminal());
assertTrue(outcome.complete(), "a positive match elsewhere in the scan is definitive");
}
// --- fleetd #509: the completeness fold across clients must not collapse to "last wins" ----
@Test
void anEarlierClientsErrorSurvivesALaterClientsCleanNegative() {
// terminalForPid folds each client's Lookup.complete() with
// complete = complete && outcome.complete();
// (PaneLocator.java:117). With a SINGLE client, a fold that keeps only the last outcome
// (dropping the "complete &&" prefix) agrees with the real fold — which is why 14 of the
// 15 pre-existing tests never catch that mutation: none of them vary the number of clients.
// Here the LEAD client errors on exactly the pane that would have owned the pid (so its
// scan is incomplete AND finds no match), and the MEMBER client cleanly reports no panes
// at all (a complete, negative scan). The real fold ANDs the two into false. A fold that
// just keeps the last client's outcome would read this as a clean true — the earlier
// error is erased, and CallerResolver.java:254 would read scanComplete() as true and
// promote an unverified caller to the primary.
HerdrClient lead = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient");
HerdrClient member = new FakeHerdr().withNoPanes();
PaneLocator two = new PaneLocator(lead, member);
PaneLocator.Lookup outcome = two.terminalForPid(FakeHerdr.WORKER_PID);
assertNull(outcome.terminal(), "the pane that could have owned the pid was never checked");
assertFalse(outcome.complete(),
"an earlier client's error must survive a later client's clean negative");
}
/** Minimal single-pane {@link HerdrClient} fake, purpose-built for the ancestry tests above. */
private static final class OnePaneHerdr implements HerdrClient {
private final ObjectMapper mapper = new ObjectMapper();
@@ -19,6 +19,8 @@ import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.LongSupplier;
import static org.junit.jupiter.api.Assertions.*;
@@ -521,6 +523,97 @@ class InjectorTest {
}
}
@Test
void readinessGraceExpiryLogsTheMeasuredPollCountNextToTheConfiguredBudget() {
// fleetd #501, defect 1: READINESS_GRACE_POLLS (240) used to be printed twice — once as
// "after {} polls" and once inside the parenthesised budget — even though the loop's own
// counter (Target.notReadySincePoll) was in scope at the same call site. On THIS branch the
// counter has just reached the threshold, so it equals the constant by construction and this
// test cannot tell the two apart — it only pins that the message still carries both a poll
// count and a labelled configured budget, using literal numbers (240, 60), never
// READINESS_GRACE_POLLS or POLL_INTERVAL_MILLIS, so the assertion can't silently track a
// constant change instead of catching a real regression.
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger injectorLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Injector.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
injectorLog.addAppender(appender);
injectorLog.setLevel(Level.WARN);
try {
Injector inj = new Injector(new AgentControl(herdr), TurnListener.NOOP, _ -> false, _ -> {
});
inj.enqueue(T, "task", TestTurnTokens.inert(T));
for (int i = 0; i < READINESS_SAMPLES; i++) inj.onStatus(T, AgentStatus.IDLE);
String warn = appender.list.stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.findFirst()
.orElse("no grace-expiry WARN logged");
assertTrue(warn.contains("after 240 polls"),
"must print the measured poll count as a plain number: " + warn);
assertTrue(warn.contains("configured=240 polls/60s"),
"must print the configured budget, clearly labelled: " + warn);
} finally {
injectorLog.detachAppender(appender);
}
}
@Test
void readinessGraceExpiryLogsTheMeasuredElapsedTimeNotArithmeticOnConstants() {
// fleetd #501, defect 2: the old line computed "({}s)" as READINESS_GRACE_POLLS *
// POLL_INTERVAL_MILLIS / 1000 — arithmetic on two constants, never a measurement, and wrong
// in the direction that says everything ran on schedule. This stub clock returns two FIXED
// values (1_000ms at the first non-ready sample, 318_412ms at the poll that trips the grace)
// whose difference — 317_412ms — does NOT equal 240 * POLL_INTERVAL_MILLIS (=60_000ms).
// Asserting on that literal, non-derived number is what makes this test able to fail if the
// production code goes back to printing the constant-arithmetic value instead of the
// injected clock's measurement.
long[] readings = {1_000L, 318_412L};
AtomicInteger call = new AtomicInteger(0);
LongSupplier stubClock = () -> {
int i = call.getAndIncrement();
if (i >= readings.length) {
throw new AssertionError("nowMillis read more times than this fixture expects (" + i
+ "); the readiness-not-ready branch should read the clock exactly twice — "
+ "once to stamp the first non-ready sample, once at grace expiry");
}
return readings[i];
};
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger injectorLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(Injector.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
injectorLog.addAppender(appender);
injectorLog.setLevel(Level.WARN);
try {
Injector inj = new Injector(new AgentControl(herdr), TurnListener.NOOP, _ -> false, _ -> {
}, stubClock);
inj.enqueue(T, "task", TestTurnTokens.inert(T));
for (int i = 0; i < READINESS_SAMPLES; i++) inj.onStatus(T, AgentStatus.IDLE);
String warn = appender.list.stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.findFirst()
.orElse("no grace-expiry WARN logged");
assertTrue(warn.contains("elapsed=317412ms"), "must print the MEASURED elapsed time from "
+ "the injected clock (318412 - 1000 = 317412), not an arithmetic value: " + warn);
assertFalse(warn.contains("elapsed=60000ms"), "must not print "
+ "READINESS_GRACE_POLLS * POLL_INTERVAL_MILLIS (240 * 250 = 60000ms) as if it "
+ "were the measured elapsed time: " + warn);
} finally {
injectorLog.detachAppender(appender);
}
}
@Test
void aWorkerThatBecomesReadyWithinTheGraceIsDeliveredNormally() {
// The readiness grace must not fail a worker that is merely slow to boot: once it becomes
@@ -1,5 +1,9 @@
package dev.ltms.fleet.lead;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import com.fasterxml.jackson.databind.JsonNode;
import dev.ltms.fleet.config.FleetConfig;
import dev.ltms.fleet.herdr.AgentControl;
@@ -9,6 +13,7 @@ import dev.ltms.fleet.herdr.HerdrException;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.nio.file.Files;
@@ -854,4 +859,207 @@ class LeadRolloverTest {
assertFalse(bootstrapSent.contains("\"handover.md\""),
"must not name the raw relative configured value in the text actually sent");
}
// ---- fleetd #494: the log lines must print MEASURED values, never the configured budget --
private static ListAppender<ILoggingEvent> attachLog() {
Logger logger = (Logger) LoggerFactory.getLogger(LeadRollover.class);
logger.setLevel(Level.DEBUG);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
return appender;
}
private static void detachLog(ListAppender<ILoggingEvent> appender) {
((Logger) LoggerFactory.getLogger(LeadRollover.class)).detachAppender(appender);
}
private static ILoggingEvent lastEventContaining(ListAppender<ILoggingEvent> events, String substring) {
return events.list.stream()
.filter(e -> e.getFormattedMessage().contains(substring))
.reduce((_, b) -> b)
.orElseThrow(() -> new AssertionError("no log event contained \"" + substring
+ "\"; got: " + events.list.stream().map(ILoggingEvent::getFormattedMessage).toList()));
}
@Test
@DisplayName("[fleetd #494] the /clear-timeout warn line prints the MEASURED elapsed time and "
+ "nudge count next to the configured budget, never the configured value alone")
void clearTimeoutLogPrintsMeasuredElapsedAndNudgesNotJustConfigured() throws IOException {
// Idle until /clear is sent, then permanently WORKING (a genuinely stuck /clear that never
// reaches a completion boundary) — isolates the SECOND wait exactly like
// clearThatNeverSettlesAfterwardsNeverSendsBootstrapText, but with a clock that ADVANCES on
// every read so the measured elapsed time is a deterministic, non-zero value distinct from
// the configured budget — pinning fleetd #494's fix, not just its absence of a hang.
FakeHerdr fake = new FakeHerdr();
HerdrClient flipsAfterClear = new HerdrClient() {
@Override
public JsonNode call(String method, Object params) throws HerdrException {
JsonNode result = fake.call(method, params);
if ("agent.prompt".equals(method) && String.valueOf(params).contains("/clear")) {
fake.agentStatus("working");
}
return result;
}
@Override
public void close() {
fake.close();
}
};
Path handover = writeHandover("handover contents");
FleetConfig.LeadRollover config =
new FleetConfig.LeadRollover(handover.toString(), true, 3600, 20, 1 /*clearSettleSeconds*/, "boot text");
AtomicLong clock = new AtomicLong(1_000);
LeadRollover rollover = newRollover(flipsAfterClear, config, () -> clock.addAndGet(500));
ListAppender<ILoggingEvent> events = attachLog();
try {
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
assertTrue(decision.accepted(), "every synchronous gate passes; the refusal is logged "
+ "only, deep inside the deferred continuation");
ILoggingEvent event = lastEventContaining(events, "NOT sending bootstrapText");
assertEquals(Level.WARN, event.getLevel());
String message = event.getFormattedMessage();
assertTrue(message.contains("configured=1s"), "must label the configured budget: " + message);
assertTrue(message.contains("elapsed=1500ms"), "must print the MEASURED elapsed time — "
+ "with this fixture's advancing clock, the wait actually ran 1500ms against a "
+ "1s(=1000ms) configured budget: " + message);
assertTrue(message.contains("nudges=0"), "must print the measured nudge count (0 here — "
+ "the pane was WORKING throughout, never IDLE/DONE, so no nudge was ever sent): "
+ message);
assertFalse(message.contains("within 1s"), "must not present the configured budget as if "
+ "it were the measured wait duration: " + message);
} finally {
detachLog(events);
}
}
@Test
@DisplayName("[fleetd #494] the /clear pickup-grace release line is WARN (was INFO) and prints "
+ "the measured elapsed time next to the target pane")
void clearGraceReleaseLogIsWarnWithMeasuredElapsed() throws IOException {
// Default idle throughout — no WORKING sample is ever observed, so the pickup-grace wait
// exhausts PICKUP_GRACE_POLLS and releases rather than wedging (see
// clearPickupIsNudgedBeforeBootstrapTextWhenPaneStaysIdle for the un-logged half of this
// scenario). A self-advancing clock makes the measured elapsed time deterministic and
// provably distinct from a bare poll/nudge count.
FakeHerdr herdr = new FakeHerdr();
Path handover = writeHandover("handover contents");
FleetConfig.LeadRollover config = cfg(handover.toString()); // turnSettleSeconds=clearSettleSeconds=20
AtomicLong clock = new AtomicLong(1_000);
LeadRollover rollover = newRollover(herdr, config, () -> clock.addAndGet(500));
ListAppender<ILoggingEvent> events = attachLog();
try {
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
assertTrue(decision.accepted(), "expected approval; got: " + decision.reason()
+ " / " + decision.detail());
ILoggingEvent event = lastEventContaining(events, "releasing rather than wedging the roll");
assertEquals(Level.WARN, event.getLevel(), "the grace-limit release must be WARN, not "
+ "INFO — it is exactly the case that reported false success in the real incident "
+ "this fix comes from (a roll that 'succeeded' after 438ms of a 20s budget)");
String message = event.getFormattedMessage();
assertTrue(message.contains(LEAD), "must name the target pane: " + message);
assertTrue(message.contains("elapsed=4500ms"), "must print the MEASURED elapsed time — "
+ "with this fixture's advancing clock, the wait ran 4500ms before releasing: "
+ message);
// fleetd #494 follow-up (2nd pass): both numbers here are DELIBERATE plain literals,
// not derived from LeadRollover.PICKUP_GRACE_POLLS. A version of this assertion that
// reads "(" + (LeadRollover.PICKUP_GRACE_POLLS - 1) + " of those were nudged)" builds
// its expectation the same way the production code used to build the log line, so it
// cannot tell a fixed constant apart from the measured counter — proved by reverting
// the production fix and re-running: that mutant stayed green under the old assertion.
// If PICKUP_GRACE_POLLS ever changes, THIS TEST MUST FAIL and a human must look at the
// new message and update the literals below, not just re-derive them.
assertTrue(message.contains("after 8 consecutive IDLE/DONE polls (7 of those were nudged)"),
"must print the measured poll count and nudge count as plain numbers, not the "
+ "PICKUP_GRACE_POLLS constant standing in for either: " + message);
} finally {
detachLog(events);
}
}
@Test
@DisplayName("[fleetd #494] the success line prints the measured elapsed time for the whole roll")
void successLogPrintsMeasuredElapsedForTheWholeRoll() throws IOException {
// Same fixture as clearGraceReleaseLogIsWarnWithMeasuredElapsed: default idle throughout, so
// the grace release fires and the roll still goes on to send bootstrapText and log success.
// This is deliberately the SAME shape as the real incident (a roll that "succeeds" quickly)
// — the missing signal was the elapsed time on this exact line.
FakeHerdr herdr = new FakeHerdr();
Path handover = writeHandover("handover contents");
FleetConfig.LeadRollover config = cfg(handover.toString());
AtomicLong clock = new AtomicLong(1_000);
LeadRollover rollover = newRollover(herdr, config, () -> clock.addAndGet(500));
ListAppender<ILoggingEvent> events = attachLog();
try {
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
assertTrue(decision.accepted(), "expected approval; got: " + decision.reason()
+ " / " + decision.detail());
ILoggingEvent event = lastEventContaining(events, "lead-rollover: rolled");
assertEquals(Level.INFO, event.getLevel());
String message = event.getFormattedMessage();
// fleetd #494 follow-up: waitUntilAtTurnBoundary now also reads the injected clock one
// extra time (to compute ITS OWN measured elapsed on the success path), so the shared
// fixture clock advances by one more 500ms tick before the roll finishes than it did
// before that follow-up — 7000ms, not 6500ms.
assertTrue(message.contains("elapsedMs=7000"), "must print the MEASURED elapsed time for "
+ "the whole roll — with this fixture's advancing clock, the full roll (turn-settle "
+ "wait + /clear wait + bootstrapText) took 7000ms: " + message);
} finally {
detachLog(events);
}
}
@Test
@DisplayName("[fleetd #494 follow-up] the turn-settle timeout warn line prints the MEASURED "
+ "elapsed time next to the configured budget, never the configured value alone")
void turnTimeoutLogPrintsMeasuredElapsedNotJustConfigured() throws IOException {
// The brief that named the four items this ticket fixed left this exact sibling line out —
// "did not reach a turn boundary (IDLE or DONE) within {}s after confirm()" — even though it
// has the identical defect shape one method below. Same fixture shape as
// clearTimeoutLogPrintsMeasuredElapsedAndNudgesNotJustConfigured, but for the FIRST wait: the
// calling lead's own pane never goes idle, so waitUntilAtTurnBoundary times out.
FakeHerdr herdr = new FakeHerdr();
herdr.agentStatus("working"); // the calling lead's own pane — never goes idle in this test
Path handover = writeHandover("handover contents");
FleetConfig.LeadRollover config =
new FleetConfig.LeadRollover(handover.toString(), true, 3600, 1 /*turnSettleSeconds*/, 20, "text");
AtomicLong clock = new AtomicLong(1_000);
LeadRollover rollover = newRollover(herdr, config, () -> clock.addAndGet(500));
ListAppender<ILoggingEvent> events = attachLog();
try {
LeadRollover.PendingRollover pending = rollover.open(LEAD, "context is full");
LeadRollover.RollDecision decision = rollover.confirm(LEAD, pending.token(), true);
assertTrue(decision.accepted(), "every synchronous gate should pass; the refusal happens "
+ "only inside the deferred continuation, which this test's synchronous runner "
+ "has already run to completion by the time confirm() returns");
ILoggingEvent event = lastEventContaining(events, "refusing to send /clear at all");
assertEquals(Level.WARN, event.getLevel());
String message = event.getFormattedMessage();
assertTrue(message.contains("configured=1s"), "must label the configured budget: " + message);
assertTrue(message.contains("elapsed=1500ms"), "must print the MEASURED elapsed time — "
+ "with this fixture's advancing clock, the wait actually ran 1500ms against a "
+ "1s(=1000ms) configured budget: " + message);
assertFalse(message.contains("within 1s"), "must not present the configured budget as if "
+ "it were the measured wait duration: " + message);
} finally {
detachLog(events);
}
}
}
@@ -57,6 +57,23 @@ class ConnectionIdentityTest {
// must read as "resolved" — the distinction #317 turns on.
ConnectionIdentity.Caller c = with(_ -> 999_999).resolve("127.0.0.1", 55555);
assertTrue(c.resolved());
assertTrue(c.scanComplete(), "no herdr error happened, so the scan is complete");
}
@Test
void scanIsIncompleteWhenHerdrErrorsOnThePaneThatOwnsThePid() {
// fleetd #505: a transient herdr error on exactly the pane that DOES own the caller's pid
// must be visible as an incomplete scan, distinct from a real primary (resolved(), null
// terminal, complete scan). Both have pid > 0 and a null terminal — scanComplete is the
// only thing that tells them apart.
FakeHerdr failing = new FakeHerdr().processInfoFailsForPane("w2:p7", "transient");
ConnectionIdentity id = new ConnectionIdentity(new PaneLocator(failing), _ -> FakeHerdr.WORKER_PID);
ConnectionIdentity.Caller c = id.resolve("127.0.0.1", 55555);
assertTrue(c.resolved(), "the pid itself resolved fine — this is not #317's failure");
assertNull(c.terminal(), "the owning pane could not be confirmed");
assertFalse(c.scanComplete(), "the scan could not check the pane that owns this pid");
}
@Test
@@ -78,10 +78,15 @@ class FleetMcpAuthzTest {
// fleetd #480 correction round: FleetMcp has one constructor now (no defaulting
// overloads — see its javadoc), so every feature this test does not exercise is passed
// its explicit "off" value here rather than being omitted.
//
// fleetd #518: callers is now required (never null) either way — the resolver that used
// to be omitted to reach "legacy" is now always real, and AuthorizationMode is the
// separate, explicit choice that governs enforcement.
mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(),
new PrimaryRegistry(null),
enforce ? CallerResolver.withLeadsAndMembers(identity, false, null,
Map::of, new MemberRegistry(null)) : null,
CallerResolver.withLeadsAndMembers(identity, false, null,
Map::of, new MemberRegistry(null)),
enforce ? FleetMcp.AuthorizationMode.ENFORCED : FleetMcp.AuthorizationMode.UNENFORCED,
metrics, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"),
FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(),
FleetMcp.LeadSeatSource.none(), List.of(), null);
@@ -187,7 +192,31 @@ class FleetMcpAuthzTest {
// The 22 pre-existing FleetMcpTest cases rely on no authorization being enforced.
FleetMcp m = mcp(false);
assertNull(m.denyFor(ANON, Authz.Action.SPAWN, null),
"no CallerResolver supplied ⇒ authorization not enforced (legacy behaviour)");
"AuthorizationMode.UNENFORCED chosen explicitly ⇒ authorization not enforced "
+ "(legacy behaviour) — fleetd #518 replaced the old callers == null idiom");
}
/**
* fleetd #509 was originally proven against {@code FleetMcp.legacyPrincipal} — a second,
* separately-maintained principal-resolution heuristic that only ran when {@code callers} was
* omitted (null). fleetd #518 deleted that whole heuristic: {@code callers} is now required
* and non-null under every {@link FleetMcp.AuthorizationMode}, so the ONE real
* {@link CallerResolver} resolves every caller, enforced or not, and #509's property (a
* non-loopback / unresolved caller must never earn the primary's authority) is exactly what
* {@code CallerResolverTest.aNonLoopbackCallerIsNeverThePrimaryUnderLoopbackTrust} already
* proves on that one real path. There is no longer a second heuristic here to test.
*/
@Test
void anUnresolvedNonLoopbackCallerIsAnonymousUnderTheOneRealResolver() {
ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> 999_999);
CallerResolver resolver = CallerResolver.withLeadsAndMembers(identity, false, null,
Map::of, new MemberRegistry(null));
// A non-loopback address never even reaches the pane scan — resolve() short-circuits it
// to Caller(null, -1, true), the same "no terminal" shape a genuine primary's connection
// produces on loopback. The real resolver must not conflate the two.
Principal p = resolver.resolve("8.8.8.8", 1234, null);
assertEquals(Principal.anonymous(), p,
"an unresolved, non-loopback caller must earn no authority, not the primary's");
}
// --- fleetd #439: who may see fleet_list's coordinator row ----------------------------------
@@ -0,0 +1,147 @@
package dev.ltms.fleet.mcp;
import dev.ltms.fleet.auth.CallerResolver;
import dev.ltms.fleet.auth.MemberRegistry;
import dev.ltms.fleet.config.FleetConfig;
import dev.ltms.fleet.guard.SubscriptionGuard;
import dev.ltms.fleet.herdr.AgentControl;
import dev.ltms.fleet.herdr.FakeHerdr;
import dev.ltms.fleet.herdr.PaneLocator;
import dev.ltms.fleet.herdr.WorkspaceControl;
import dev.ltms.fleet.inject.Injector;
import dev.ltms.fleet.member.ClaudeCodeLauncher;
import dev.ltms.fleet.msg.InMemoryReplyInbox;
import dev.ltms.fleet.msg.MessageService;
import dev.ltms.fleet.msg.Rendezvous;
import dev.ltms.fleet.session.FakeWorktrees;
import dev.ltms.fleet.session.SessionManager;
import io.modelcontextprotocol.client.McpClient;
import io.modelcontextprotocol.client.McpSyncClient;
import io.modelcontextprotocol.client.transport.HttpClientStreamableHttpTransport;
import io.modelcontextprotocol.spec.McpClientTransport;
import io.modelcontextprotocol.spec.McpSchema;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.servlet.ServletContextHandler;
import org.eclipse.jetty.servlet.ServletHolder;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import java.net.http.HttpRequest;
import java.util.List;
import java.util.Map;
import java.util.Set;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* fleetd #518 — Part 2: drive the {@code contextExtractor} closure for real.
*
* <p>{@code FleetMcp.deny()}/{@code denyFor()} has a full policy table of tests
* ({@code FleetMcpAuthzTest}), and {@code CallerResolver.resolve()} has its own full suite
* ({@code CallerResolverTest}). Neither one ever exercises the closure that WIRES them together
* inside {@code FleetMcp}'s constructor: it is built once, handed to the MCP SDK's transport, and
* only ever runs when a real MCP client makes a real HTTP request. Every existing test either
* calls {@code denyFor(Principal, ...)} with a hand-built {@link dev.ltms.fleet.auth.Principal}
* (never asking who the transport would actually have resolved) or drives a static handler method
* directly. A mutation that swapped the whole resolution decision for an unconditional fallback —
* bypassing {@link CallerResolver} entirely — passed the full suite, including every
* {@code FleetMcpAuthzTest} case, because none of them go through the transport at all.
*
* <p>This test boots the real {@code HttpServletStreamableServerTransportProvider} on a real
* Jetty server, drives it with a real MCP client over HTTP, and checks a result that only the
* real {@link CallerResolver} can produce: token-mode inspects the {@code Authorization} header
* and grants {@code PRIMARY} only for the right bearer token. The connection never resolves to a
* worker pane (the fake peer-pid lookup always misses), so the ONLY way {@code fleet_whoami} can
* come back as {@code primary} is if the closure actually called {@code callers.resolve(...)} and
* read that header — a behaviour the deleted {@code legacyPrincipal} heuristic never had at all.
*/
class FleetMcpContextExtractorTest {
private static final String TOKEN = "s3cret-mcp-token";
private final FakeHerdr herdr = new FakeHerdr();
private final AgentControl agents = new AgentControl(herdr);
private FleetMcp mcp;
private Server server;
@AfterEach
void tearDown() throws Exception {
if (server != null) {
server.stop();
}
if (mcp != null) {
mcp.close();
}
}
@Test
void aRealMcpRequestIsResolvedByTheRealCallerResolverNotAFallback() throws Exception {
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null,
"tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
ClaudeCodeLauncher workers = new ClaudeCodeLauncher(agents, new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
_ -> "tok");
SessionManager sessions = new SessionManager(workers, new FakeWorktrees());
MessageService messages = new MessageService(agents, new Injector(agents), new Rendezvous(),
new InMemoryReplyInbox());
// The peer-pid lookup always misses (-1), so no connection here is ever resolved to a
// worker pane — every call falls through to CallerResolver's token check, the one branch
// that is unreachable through the deleted legacy heuristic.
ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(herdr), _ -> -1);
CallerResolver callers = CallerResolver.withLeadsAndMembers(identity, true, TOKEN,
Map::of, new MemberRegistry(null));
mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(),
new PrimaryRegistry(null), callers, FleetMcp.AuthorizationMode.ENFORCED,
null, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"),
FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(),
FleetMcp.LeadSeatSource.none(), List.of(), null);
ServletContextHandler handler = new ServletContextHandler();
handler.setContextPath("/");
handler.addServlet(new ServletHolder(mcp.servlet()), "/mcp");
server = new Server(0);
server.setHandler(handler);
server.start();
String baseUrl = "http://127.0.0.1:"
+ ((ServerConnector) server.getConnectors()[0]).getLocalPort();
// The right bearer token: the real CallerResolver grants PRIMARY, which fleet_whoami's
// READ gate lets through.
McpSchema.CallToolResult authorized = callWhoami(baseUrl, "Bearer " + TOKEN);
assertFalse(authorized.isError(), "a valid bearer token must resolve as PRIMARY and pass "
+ "fleet_whoami's READ gate: " + textOf(authorized));
assertTrue(textOf(authorized).contains("\"role\":\"primary\""),
"fleet_whoami must report the role the real CallerResolver resolved over this "
+ "connection, not a fallback: " + textOf(authorized));
// No credential at all, over the SAME wiring: the real resolver refuses it as ANONYMOUS.
// legacyPrincipal never looked at the Authorization header, so it could not have told
// these two calls apart at all -- this is the assertion the deleted mutation would fail.
McpSchema.CallToolResult unauthorized = callWhoami(baseUrl, null);
assertTrue(unauthorized.isError(), "no credential must be refused, not silently let "
+ "through: " + textOf(unauthorized));
}
private static McpSchema.CallToolResult callWhoami(String baseUrl, String authorizationHeader) {
HttpRequest.Builder requestTemplate = HttpRequest.newBuilder();
if (authorizationHeader != null) {
requestTemplate.header("Authorization", authorizationHeader);
}
McpClientTransport transport = HttpClientStreamableHttpTransport.builder(baseUrl)
.endpoint("/mcp")
.requestBuilder(requestTemplate)
.build();
try (McpSyncClient client = McpClient.sync(transport).build()) {
client.initialize();
return client.callTool(McpSchema.CallToolRequest.builder("fleet_whoami").arguments(Map.of()).build());
}
}
private static String textOf(McpSchema.CallToolResult r) {
return ((McpSchema.TextContent) r.content().getFirst()).text();
}
}
@@ -89,6 +89,7 @@ class FleetMcpHandoverTest {
mcp = new FleetMcp(messages, workers, sessions, identity, sessions.asPresence(),
new PrimaryRegistry(null),
CallerResolver.withLeadsAndMembers(identity, false, null, Map::of, new MemberRegistry(null)),
FleetMcp.AuthorizationMode.ENFORCED,
null, FleetMcp.CapacitySource.none(), new FleetMcp.HealthCoverageSource(() -> "off"),
FleetMcp.QuarantineSource.none(), null, FleetMcp.OutageSource.none(),
FleetMcp.LeadSeatSource.none(), List.of(), leadRollover);
@@ -25,7 +25,12 @@ import dev.ltms.fleet.peer.SpawnRequest;
import dev.ltms.fleet.placement.BackendQuarantine;
import dev.ltms.fleet.placement.PlacementDecision;
import dev.ltms.fleet.placement.PlacementPolicies;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
import org.junit.jupiter.api.io.TempDir;
import org.slf4j.LoggerFactory;
@@ -50,9 +55,46 @@ import static org.junit.jupiter.api.Assertions.*;
* CB-301 / CB-303 acceptance tests for the authoritative session registry, one-shot lifecycle FSM,
* and configurable lifecycle limits (idle TTL, context cap, drain).
* No live herdr — everything runs against the same {@link FakeHerdr} the rest of the project uses.
*
* <p>fleetd #525: only {@link #onTurnFailedIsLoggedAtWarnWithThePriorState} (explicitly
* {@link Order#value() @Order(1)}) and the proving test right after it
* ({@link #sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn}, {@code @Order(2)})
* care about method order — every other test here has no {@code @Order} and so runs after both of
* these (JUnit 5's {@link MethodOrderer.OrderAnnotation} gives an unannotated method the lowest
* priority), in whatever relative order it already ran in.
*/
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
class SessionManagerTest {
/**
* fleetd #525: the level {@link SessionManager}'s logger had when this class started, captured
* before any test here — including the leak this ticket fixes — can touch it. {@code
* pinSessionManagerLoggerToAKnownBaseline} then forces a distinctive, known value (DEBUG) so
* {@link #sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn} can tell "the
* level came back to what it was" apart from "the level happens to already be WARN because
* some earlier test class in this JVM fork (surefire reuses forks by default) left it there" —
* a real risk, since {@code ch.qos.logback.classic.Logger} instances are cached per class and
* shared across the whole JVM, and this exact logger is also touched by
* {@code WorktreeSessionManagerTest#releasePreservesDirtyWorktreeAndLogsWarn}, which has the
* same unfixed leak (reported, not fixed — out of this ticket's scope).
*/
private static Level sessionManagerLevelBeforeThisClass;
@BeforeAll
static void pinSessionManagerLoggerToAKnownBaseline() {
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
sessionManagerLevelBeforeThisClass = sessionLog.getLevel();
sessionLog.setLevel(Level.DEBUG);
}
@AfterAll
static void restoreSessionManagerLoggerLevel() {
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
sessionLog.setLevel(sessionManagerLevelBeforeThisClass);
}
private SessionManager sessionManager(FakeHerdr herdr) {
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
@@ -81,6 +123,55 @@ class SessionManagerTest {
return new SessionManager(workers, worktrees, clock);
}
/**
* fleetd #525: captures a logger's output and, on {@link #close}, restores <em>both</em> the
* appender and the level to what they were before. A bare {@code addAppender}/{@code
* setLevel} pair whose {@code finally} only detaches the appender leaves the level pinned —
* {@code ch.qos.logback.classic.Logger} instances are cached per class and shared across the
* whole JVM, so a level set by one test in this class is still in effect for every test that
* runs after it, in this class or any other. try-with-resources makes "restored the appender
* but not the level" impossible to write, because there is only one thing to close.
*/
private static final class CapturedLog implements AutoCloseable {
private final ch.qos.logback.classic.Logger logger;
private final Level originalLevel;
private final ListAppender<ILoggingEvent> appender;
private CapturedLog(Class<?> loggerClass, Level pinnedLevel) {
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
this.logger = (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(loggerClass);
this.originalLevel = logger.getLevel();
this.appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
logger.addAppender(appender);
if (pinnedLevel != null) {
logger.setLevel(pinnedLevel);
}
}
/** Capture {@code loggerClass}'s output, pinning its level to {@code pinnedLevel} for the
* duration of the try-with-resources block. */
static CapturedLog at(Class<?> loggerClass, Level pinnedLevel) {
return new CapturedLog(loggerClass, pinnedLevel);
}
/** Capture {@code loggerClass}'s output without changing its level. */
static CapturedLog of(Class<?> loggerClass) {
return new CapturedLog(loggerClass, null);
}
List<ILoggingEvent> events() {
return appender.list;
}
@Override
public void close() {
logger.detachAppender(appender);
logger.setLevel(originalLevel);
}
}
/**
* CB-581: a {@link Worktrees} test double whose {@code hasUncommitted} and {@code remove} can
* be told to throw, so {@link SessionManager#release} can be exercised against exactly the
@@ -466,24 +557,15 @@ class SessionManagerTest {
@Test
void backendErrorForUnknownTargetIsWarnedAndDoesNotCreateASession() {
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
sessionLog.addAppender(appender);
try {
try (CapturedLog log = CapturedLog.of(SessionManager.class)) {
SessionManager sessions = sessionManager(new FakeHerdr());
assertFalse(sessions.onBackendError("term_missing", "backend exited"));
assertTrue(sessions.roster().isEmpty(), "unknown target must not create a session");
assertTrue(appender.list.stream().anyMatch(e -> e.getLevel().equals(Level.WARN)
assertTrue(log.events().stream().anyMatch(e -> e.getLevel().equals(Level.WARN)
&& e.getFormattedMessage().contains("term_missing")),
"unknown target is logged at WARN");
} finally {
sessionLog.detachAppender(appender);
}
}
@@ -516,19 +598,12 @@ class SessionManagerTest {
}
@Test
@Order(1)
void onTurnFailedIsLoggedAtWarnWithThePriorState() {
// CB-564: this transition used to be a bare DEBUG "session marked failed" — a symptom with no
// cause. A member that can no longer be delegated to must be at least WARN, and should name
// what stage it failed at (here: BUSY, i.e. a turn was in flight and never resolved).
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
sessionLog.addAppender(appender);
sessionLog.setLevel(Level.WARN);
try {
try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
FakeHerdr herdr = new FakeHerdr();
SessionManager sessions = sessionManager(herdr);
MemberSession session = sessions.acquire("ltms-local", null, "/caller", "term_primary");
@@ -538,18 +613,37 @@ class SessionManagerTest {
sessions.onTurnFailed(terminal);
String warn = appender.list.stream()
String warn = log.events().stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.findFirst()
.orElse("no turn-failed WARN logged");
assertTrue(warn.contains(terminal), "the log names the member: " + warn);
assertTrue(warn.contains("BUSY"), "the log names the stage it failed at: " + warn);
} finally {
sessionLog.detachAppender(appender);
}
}
/**
* fleetd #525: proves the leak in {@link #onTurnFailedIsLoggedAtWarnWithThePriorState} above
* (which runs immediately before this, via {@code @Order}) is closed. That test pins the
* shared {@link SessionManager} logger to WARN through a {@link CapturedLog}; if {@link
* CapturedLog#close} only detached the appender — the original bug, before this ticket's fix —
* the level would still read WARN here instead of the {@code DEBUG} baseline this class's
* {@code @BeforeAll} set. Runs at {@code @Order(2)}, guaranteed after {@code @Order(1)} and
* before every other (unannotated) test in this class.
*/
@Test
@Order(2)
void sharedSessionManagerLoggerLevelIsRestoredAfterOnTurnFailedPinsWarn() {
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
assertEquals(Level.DEBUG, sessionLog.getLevel(),
"onTurnFailedIsLoggedAtWarnWithThePriorState pins the shared SessionManager logger "
+ "to WARN; its cleanup must restore the level it captured (DEBUG, set by "
+ "this class's @BeforeAll) rather than leaving WARN pinned for every test "
+ "that runs after it");
}
/**
* fleetd #226: a contended slot is refused through the real {@link SessionManager#acquire}
* path before the real launcher can hand an architect charter to a process.
@@ -576,28 +670,18 @@ class SessionManagerTest {
SessionManager sessions = sessionManager(herdr);
MemberRegistry members = architectRegistry();
sessions.setMemberLifecycle(bindFailureAfterReservation(members));
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger registryLog = (ch.qos.logback.classic.Logger)
LoggerFactory.getLogger(MemberRegistry.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
registryLog.addAppender(appender);
registryLog.setLevel(Level.WARN);
try {
try (CapturedLog log = CapturedLog.at(MemberRegistry.class, Level.WARN)) {
MemberSession session = sessions.acquire("ltms-local", MemberRole.ARCHITECT, null,
"/caller", "term_primary", null);
assertEquals(MemberRole.DEV, session.role(), "a failed reservation bind must use the fallback");
String warn = appender.list.stream()
String warn = log.events().stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.findFirst()
.orElse("no slot-exhaustion WARN logged");
assertTrue(warn.contains("ltms-local"), "the WARN names the profile: " + warn);
assertTrue(warn.contains(session.terminalId()), "the WARN names the terminal: " + warn);
} finally {
registryLog.detachAppender(appender);
}
}
@@ -902,6 +986,82 @@ class SessionManagerTest {
.count();
}
/**
* fleetd #512: a drain that releases every session cleanly used to log nothing at all — the
* two log calls in {@code drainAll}/{@code drainSnapshot} both sit on abnormal paths, so
* "clean drain" and "died on the first session" were indistinguishable. This asserts the new
* {@code log.info} line fires on the ordinary, nothing-went-wrong path, and that its numbers
* are the real counts (two released, zero abandoned) rather than just a non-empty string.
*/
@Test
void drainAllLogsACompletionLineWithTheRealCountsOnACleanDrain() {
FakeHerdr herdr = new FakeHerdr();
SessionManager sessions = sessionManager(herdr);
MemberSession first = sessions.acquire("ltms-local", "/one", "/caller", "ownerOne");
MemberSession second = sessions.acquire("ltms-local", "/two", "/caller", "ownerTwo");
sessions.asPresence().markPresent(first.terminalId());
sessions.asPresence().markPresent(second.terminalId());
// Both stay READY — neither is delivered a turn, so neither is BUSY and the drain below
// has nothing abnormal to hit.
// Pin INFO explicitly: fleetd #525 made CapturedLog itself restore the level it pins, but
// this pin stays anyway as belt-and-braces — a later change to the sweep must not be able
// to make this INFO assertion vacuous again by leaving some other test's WARN pin in place.
try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.INFO)) {
sessions.drainAll(TimeUnit.MILLISECONDS.toNanos(100));
assertTrue(sessions.roster().isEmpty(), "precondition: the drain actually ran");
String info = log.events().stream()
.filter(e -> e.getLevel().equals(Level.INFO))
.map(ILoggingEvent::getFormattedMessage)
.filter(m -> m.contains("drain complete"))
.findFirst()
.orElse("no drain-complete INFO logged");
assertTrue(info.contains("released=2"),
"both released sessions must be counted: " + info);
assertTrue(info.contains("abandoned=0"),
"neither session was BUSY, so nothing was abandoned mid-turn: " + info);
}
}
/**
* fleetd #512: the same completion line must also report a non-zero abandoned count when a
* session is still {@code BUSY} once the whole-drain deadline passes — the case the ticket
* calls out as the one a script needs to be able to see. Reuses the same BUSY/READY mix as
* {@link #drainAllReleasesBusyAndReadySessionsAndWaitsForBusy}, which already forces the busy
* session to spin until the real-time deadline expires (its state never leaves BUSY on its
* own), and adds the log assertion that test does not make.
*/
@Test
void drainAllLogsANonZeroAbandonedCountForASessionStillBusyAtTheDeadline() {
FakeHerdr herdr = new FakeHerdr();
SessionManager sessions = sessionManager(herdr);
MemberSession ready = sessions.acquire("ltms-local", "/ready", "/caller", "ownerR");
MemberSession busy = sessions.acquire("ltms-local", "/busy", "/caller", "ownerB");
sessions.asPresence().markPresent(ready.terminalId());
sessions.asPresence().markPresent(busy.terminalId());
sessions.onDelivered(busy.terminalId(), TestTurnTokens.inert(busy.terminalId()));
// busy never leaves BUSY — no completion is delivered — so the drain below must spin the
// full timeout and then release it anyway, counting it abandoned.
// Pin INFO explicitly — see the comment in drainAllLogsACompletionLineWithTheRealCountsOnACleanDrain.
try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.INFO)) {
sessions.drainAll(TimeUnit.MILLISECONDS.toNanos(100));
assertTrue(sessions.roster().isEmpty(), "precondition: the drain actually ran");
String info = log.events().stream()
.filter(e -> e.getLevel().equals(Level.INFO))
.map(ILoggingEvent::getFormattedMessage)
.filter(m -> m.contains("drain complete"))
.findFirst()
.orElse("no drain-complete INFO logged");
assertTrue(info.contains("released=2"),
"both the ready and the busy session are released: " + info);
assertTrue(info.contains("abandoned=1"),
"the busy session hit the deadline still BUSY and must be counted: " + info);
}
}
// --- fleetd #308: a spawn accepted while the shutdown drain is running must not orphan ---
@Test
@@ -1202,21 +1362,13 @@ class SessionManagerTest {
new WorktreeRequest("cb-581a", null));
worktrees.failHasUncommittedWith(new WorktreeException("git status exited 128"));
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
sessionLog.addAppender(appender);
sessionLog.setLevel(Level.WARN);
try {
try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
assertDoesNotThrow(() -> sessions.release(s.paneId()),
"a throwing dirty check must not abort the release");
assertTrue(worktrees.removeCalls().isEmpty(),
"the worktree is preserved when its dirty state cannot be determined");
String warn = appender.list.stream()
String warn = log.events().stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.filter(m -> m.contains(s.worktree()))
@@ -1224,8 +1376,6 @@ class SessionManagerTest {
.orElse("no warn logged naming the worktree");
assertTrue(warn.contains(s.paneId()), "the WARN names the pane: " + warn);
assertTrue(warn.contains(s.terminalId()), "the WARN names the terminal: " + warn);
} finally {
sessionLog.detachAppender(appender);
}
}
@@ -1392,20 +1542,12 @@ class SessionManagerTest {
// this itself, so it no longer propagates out of release() at all.
worktrees.failRemoveFor(b.worktree());
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
sessionLog.addAppender(appender);
sessionLog.setLevel(Level.WARN);
int reaped;
try {
try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
clock[0] = 100;
reaped = sessions.reapIdle(10);
String warn = appender.list.stream()
String warn = log.events().stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.filter(m -> m.contains(b.paneId()))
@@ -1413,8 +1555,6 @@ class SessionManagerTest {
.orElse("no worktree-removal-failure WARN logged");
assertTrue(warn.contains(b.terminalId()), "the WARN names the failed session's terminal: " + warn);
assertTrue(warn.contains(b.worktree()), "the WARN names the failed session's worktree: " + warn);
} finally {
sessionLog.detachAppender(appender);
}
assertEquals(3, reaped,
@@ -1460,28 +1600,18 @@ class SessionManagerTest {
// trigger reapIdle's own guard is for, now that #283 closed the worktree-removal trigger.
herdr.paneCloseFailsForPane("w9:pRoot_2", "internal_error");
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger sessionLog =
(ch.qos.logback.classic.Logger) LoggerFactory.getLogger(SessionManager.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
sessionLog.addAppender(appender);
sessionLog.setLevel(Level.WARN);
int reaped;
try {
try (CapturedLog log = CapturedLog.at(SessionManager.class, Level.WARN)) {
clock[0] = 100;
reaped = sessions.reapIdle(10);
String warn = appender.list.stream()
String warn = log.events().stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.filter(m -> m.contains("reap failed") && m.contains(b.paneId()))
.findFirst()
.orElse("no reap-failed WARN logged for the failing session");
assertTrue(warn.contains(b.terminalId()), "the WARN names the failed session's terminal: " + warn);
} finally {
sessionLog.detachAppender(appender);
}
assertEquals(2, reaped,
+118 -33
View File
@@ -64,7 +64,106 @@
# The two outputs side by side are the finding: any name whose hash matches between them is a
# credential the member holds in full.
#
# One parse pass: line 1 = present (true/false/null), line 2 = policy mode (possibly blank),
# lines 3-5 = knownCount/allowedCount/blockedCount, remaining lines = the known[] names. A single
# pass avoids re-parsing (and re-risking a truthiness bug) five separate times.
#
# This used to feed the parser straight into `mapfile -t _FIELDS < <(producer)`. That form cannot
# see the producer fail: `<` `<(...)` is a process substitution, not a pipeline, so `set -o
# pipefail` does not reach inside it, and mapfile's own exit status reports whether the BUILTIN
# ran, not whether the command substituted into it succeeded — a failing jq or python3 there still
# leaves mapfile at rc=0 with an empty array, read as a parse that genuinely found nothing (fleetd
# #500). Capturing the parser's output with command substitution first, and checking ITS exit
# status, reports the producer's real failure while the fact still exists — before it is handed to
# mapfile at all.
#
# mapfile then reads from that captured string with `<<<` (a herestring), not `< <(...)`: `<<<`
# materialises the whole string in memory first, where `< <(...)` would stream it. That only
# matters for a large producer; this one is a short credential-name policy response, so the
# tradeoff is irrelevant here — noted because it would not be for every producer.
parse_policy_fields() {
if command -v jq >/dev/null 2>&1; then
_FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | jq -r '
(.present | tostring),
(.policy // ""),
(.knownCount // 0 | tostring),
(.allowedCount // 0 | tostring),
(.blockedCount // 0 | tostring),
(.known[]? // empty)')"
_PARSE_STATUS=$?
_PARSER_NAME="jq"
else
_FIELDS_RAW="$(printf '%s' "$POLICY_JSON" | python3 - <<'PY'
import json, sys
data = json.load(sys.stdin)
print(str(data.get("present")))
print(data.get("policy") or "")
print(data.get("knownCount") if data.get("knownCount") is not None else 0)
print(data.get("allowedCount") if data.get("allowedCount") is not None else 0)
print(data.get("blockedCount") if data.get("blockedCount") is not None else 0)
for n in (data.get("known") or []):
print(n)
PY
)"
_PARSE_STATUS=$?
_PARSER_NAME="python3"
fi
if [ "$_PARSE_STATUS" -ne 0 ]; then
echo "refusing to run: could not parse the policy fetched from $POLICY_URL — $_PARSER_NAME exited" \
"non-zero (status $_PARSE_STATUS). That is a parser failure, not a claim about the policy" \
"itself; the policy response has not been read." >&2
return 4
fi
# A herestring adds a newline, so mapfile would turn an empty parser result into one empty field.
# Keep that case separate so the refusal reports what the parser actually returned: zero fields.
if [ -z "$_FIELDS_RAW" ]; then
_FIELDS=()
else
mapfile -t _FIELDS <<< "$_FIELDS_RAW"
fi
# Arity check — the CORRECTNESS fix (fleetd #500). A parser that exits 0 can still return fewer
# than the 5 fixed fields (present, policy mode, 3 counts) that every fixed-field read in main() expects,
# whatever the reason: a producer that printed nothing, malformed JSON that jq/python3 still
# accepted, or a schema change upstream. main()'s slice (`_FIELDS[@]:5`) does not fire
# `set -u` on an unset OR a short array, and every fixed-field read there used a `:-` default, so
# without this check a short `_FIELDS` reaches the "0 known names" guard further down with the
# same look as a policy that genuinely has 0 names. Check the count here, at the one point the
# fact is still present, before the slice consumes it.
if (( ${#_FIELDS[@]} < 5 )); then
echo "refusing to run: the policy parser ($_PARSER_NAME) returned ${#_FIELDS[@]} field(s); at" \
"least 5 are required (present, policy mode, knownCount, allowedCount, blockedCount). The" \
"parse ran but its shape is wrong — this is not a claim about how many names the policy" \
"knows." >&2
return 5
fi
}
main() {
set -uo pipefail
# `pipefail` is not what catches the parser failure handled in parse_policy_fields() above (fleetd #500): in
# `printf '%s' "$POLICY_JSON" | jq -r '...'`, jq is the LAST element of the pipe, so the pipeline's
# own exit status is already jq's status, with or without pipefail. It is kept as insurance for if
# a post-processing stage is ever appended after the parser (e.g. `| tail -n +2`) — at that point
# the parser would sit upstream and pipefail becomes the only thing that still reports its status.
# --- refuse on an interpreter that cannot run this script (fleetd #500) -------------------------
#
# mapfile, used below to parse the policy response, was added in bash 4.0. macOS ships bash 3.2.57
# at /bin/bash, which predates it. This script's own `set -uo pipefail` does not catch a missing
# mapfile: the builtin just fails with "command not found" on stderr, and every line below that
# reads the array it would have filled uses a `:-` default or a slice, neither of which `set -u`
# catches on an unset array. Left unguarded, that chain ends in the "0 known names" refusal further
# down — a claim about the POLICY, for a failure that is actually about the INTERPRETER. So the
# interpreter is checked once, explicitly, before it is asked to do anything mapfile depends on.
if (( ${BASH_VERSINFO[0]} < 4 )); then
echo "refusing to run: this script uses mapfile, which needs bash 4 or newer. This shell is bash" \
"${BASH_VERSION:-<unknown, no \$BASH_VERSION>}. Re-run it under a newer bash, for example:" \
"\"\$(command -v bash)\" \"$0\"" "$@" >&2
exit 3
fi
FLEETD_HOST="${FLEETD_HOST:-http://127.0.0.1:8765}"
POLICY_URL="${FLEETD_HOST%/}/member-credentials"
@@ -121,31 +220,10 @@ EOF
exit 1
fi
# One parse pass: line 1 = present (true/false/null), line 2 = policy mode (possibly blank),
# lines 3-5 = knownCount/allowedCount/blockedCount, remaining lines = the known[] names. A single
# pass avoids re-parsing (and re-risking a truthiness bug) five separate times.
if command -v jq >/dev/null 2>&1; then
mapfile -t _FIELDS < <(printf '%s' "$POLICY_JSON" | jq -r '
(.present | tostring),
(.policy // ""),
(.knownCount // 0 | tostring),
(.allowedCount // 0 | tostring),
(.blockedCount // 0 | tostring),
(.known[]? // empty)')
else
mapfile -t _FIELDS < <(printf '%s' "$POLICY_JSON" | python3 - <<'PY'
import json, sys
data = json.load(sys.stdin)
print(str(data.get("present")))
print(data.get("policy") or "")
print(data.get("knownCount") if data.get("knownCount") is not None else 0)
print(data.get("allowedCount") if data.get("allowedCount") is not None else 0)
print(data.get("blockedCount") if data.get("blockedCount") is not None else 0)
for n in (data.get("known") or []):
print(n)
PY
)
fi
# Parse the policy in one pass and refuse on any of the three failure causes. The decision, the
# three refusals and the reasoning behind each live in parse_policy_fields() above — kept there
# with the code rather than here, so the explanation cannot drift away from what it explains.
parse_policy_fields || exit $?
PRESENT="${_FIELDS[0]:-null}"
POLICY_MODE="${_FIELDS[1]:-}"
@@ -164,19 +242,21 @@ case "$KNOWN_COUNT_REPORTED" in
;;
esac
# --- guard the denominator explicitly — never proceed on a zero/short count ---------------------
# --- guard the denominator explicitly — never proceed on a zero count ---------------------------
#
# This is the exact trap named in the ticket: an empty (or truncated) NAMES array passes every
# subsequent "is it set" check vacuously and prints a table that LOOKS complete. So this is checked
# before anything else runs, with a message that says why, not just that it failed.
# This is the exact trap named in the ticket: an empty NAMES array passes every subsequent "is it
# set" check vacuously and prints a table that LOOKS complete. By this point the interpreter gate,
# the parser-exit-status check, and the arity check above have already ruled out "the interpreter
# couldn't run mapfile", "the parser failed", and "the parser returned the wrong shape" — so a zero
# count reaching here really does mean the policy itself reports 0 known names, not a swallowed
# failure upstream. That is still checked before anything else runs, with a message that says so.
if [ "${#NAMES[@]}" -eq 0 ] || [ "$KNOWN_COUNT_REPORTED" -eq 0 ]; then
cat >&2 <<EOF
refusing to run: the policy fetched from $POLICY_URL contains 0 known names (present=${PRESENT:-unknown}).
Either memberCredentials: is absent/empty on the running daemon (nothing is protected — see fleetd's
own startup warning), or the response could not be parsed. Either way, checking zero names would
print a clean-looking table for a policy that protects nothing, or for a probe that read nothing.
This is refused rather than reported as a pass.
memberCredentials: is absent or empty on the running daemon — nothing is protected (see fleetd's own
startup warning). Checking zero names would print a clean-looking table for a policy that protects
nothing. This is refused rather than reported as a pass.
EOF
exit 1
fi
@@ -251,3 +331,8 @@ How to read this:
hardcoded list did. If the daemon's policy changes, the next run of this script reflects it
with no edit to this file.
EOF
}
if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
main "$@"
fi
+532 -64
View File
@@ -5,7 +5,7 @@
# A merge is not a deployment: the running daemon holds the jar it was started with, so code merged
# to main does nothing until this runs. See CLAUDE.md -> "Redeploying the daemon".
#
# This script exists to turn six remembered traps into one auditable command:
# This script exists to turn eight remembered traps into one auditable command:
#
# 1. A piped `mvn` hides BUILD FAILURE behind a zero exit, so the build here is never piped.
# 2. The daemon must start from a LOGIN shell, or the tokens it hands to members are empty:
@@ -27,6 +27,21 @@
# restart of the OLD jar. So this script detects whether the agent is loaded and, only then,
# swaps `kill` + manual `nohup` for `launchctl unload`/`load` — the one supervisor in control
# at any moment is whichever one you asked to act, never both.
# 7. fleetd #492 — a systemd --user unit is a THIRD possible supervisor (seen on a second host):
# Restart=on-failure treats this JVM's SIGTERM exit code (143, per CB-594 above) as a failure
# too, so a bare `kill` there would race systemd's own restart of the OLD jar exactly like
# launchd would. This script now tells launchd, systemd, and "genuinely unsupervised" apart as
# three different answers, drives whichever one it finds through its own control plane
# (`launchctl` / `systemctl --user`), and REFUSES outright — never falls back to `kill` — when
# it finds a supervision signal it cannot map to exactly one of the two it knows how to drive.
# A wrong guess here is how two daemons end up running against one herdr session. Follow-up:
# "not currently loaded" is not the same fact as "unsupervised" — a unit that is installed but
# activating/failed/pending-restart, or a `systemctl` call that could not answer at all (e.g.
# no user-bus access), both now read as a fifth answer, "unclear", and REFUSE the same way
# "ambiguous" does, rather than silently falling through to "none".
# 8. fleetd #492 — a post-restart check counts running fleetd processes and fails the whole run if
# more than one is alive. That is the one thing none of the checks above (healthz 200, jar id,
# the fresh "listening" line) can see: every one of them is satisfied by EITHER daemon.
#
# Usage:
# scripts/redeploy-fleetd.sh # build, confirm, restart, verify
@@ -41,6 +56,13 @@ set -euo pipefail
REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
MODULE="$REPO/fleetd"
JAR="$MODULE/target/fleetd.jar"
# fleetd #493: never build into the path a running process holds. The build writes here first
# (Maven's shade plugin has finalName=fleetd, so `clean install` still lands its output at
# target/fleetd.jar — that part is unchanged and out of this script's control), but this script
# now moves it out to JAR_STAGED immediately, and only swaps it back to JAR (a plain `mv`, so a
# rename, never a byte-by-byte overwrite) after the OLD daemon has been confirmed exited. See
# stage_built_jar/swap_staged_jar below.
JAR_STAGED="$MODULE/target/fleetd-new.jar"
OUT="$MODULE/fleetd.out"
# Matches BOTH the absolute form and the relative `java -jar target/fleetd.jar` a hand-start
# produces from inside fleetd/. Anchoring on the absolute path alone was a real bug: the daemon
@@ -55,13 +77,42 @@ HEALTH_WAIT=60 # seconds to wait for /healthz to answer after start
LAUNCHD_LABEL='dev.ltms.fleetd'
LAUNCHD_PLIST="$HOME/Library/LaunchAgents/$LAUNCHD_LABEL.plist"
# fleetd #492: the systemd --user unit this script must not fight with either (see trap 7 above).
# Measured on the second host: `systemctl --user cat fleetd` names the unit "fleetd" (not
# "dev.ltms.fleetd" — systemd user units here are not namespaced the way the launchd label is).
SYSTEMD_UNIT='fleetd'
# fleetd #492 follow-up: detect_supervisor packs TWO values (kind, detail) onto the one stdout
# line that survives its $(...) call — see the constraints comment above that function. This is
# the separator between them: the ASCII "unit separator" byte, chosen because it never occurs in
# any of the prose detail strings and needs no escaping in a `case`/glob pattern.
SUPERVISOR_DETAIL_SEP=$'\x1f'
# fleetd #492 follow-up: set by systemd_loaded/systemd_installed when the underlying `systemctl`
# call could not answer cleanly — it exited non-zero AND wrote something to stderr, which is a real
# tool failure (e.g. it cannot reach the user bus over a non-lingering ssh session), never the same
# fact as a clean negative answer ("not active", no stderr). Initialized here, not just inside the
# probes, so detect_supervisor can read them under `set -u` even before either probe has ever run,
# and so a test that stubs a probe with a plain `return 0`/`return 1` body (leaving these untouched)
# reads a deterministic 0 rather than whatever a previous probe call left behind.
SYSTEMD_LOADED_ERRORED=0
SYSTEMD_INSTALLED_ERRORED=0
# fleetd #492 follow-up: SUPERVISOR_UNCLEAR_DETAIL is the specific supervisor/reason that
# require_drivable_supervisor's die() names on an "unclear" answer. Deliberately NOT pre-declared
# here (unlike the two flags above): it is set only by the real call site, right after it unpacks
# detect_supervisor's stdout (see the constraints comment above detect_supervisor). If that call
# site is ever skipped or broken, a bare `set -u` reference to this variable in
# require_drivable_supervisor must fail loudly with "unbound variable" — a pre-declared empty
# default would instead silently print an empty reason, hiding exactly the value this ticket
# exists to surface.
DO_BUILD=1; ASSUME_YES=0; CHECK_ONLY=0
for arg in "$@"; do
case "$arg" in
--yes|-y) ASSUME_YES=1 ;;
--no-build) DO_BUILD=0 ;;
--check) CHECK_ONLY=1 ;;
-h|--help) sed -n '3,37p' "${BASH_SOURCE[0]}"; exit 0 ;;
-h|--help) sed -n '3,48p' "${BASH_SOURCE[0]}"; exit 0 ;;
*) echo "unknown option: $arg (try --help)" >&2; exit 2 ;;
esac
done
@@ -71,14 +122,293 @@ ok() { printf ' ok %s\n' "$*"; }
warn() { printf ' WARN %s\n' "$*"; }
die() { printf '\n FAIL %s\n\n' "$*" >&2; exit 1; }
jar_id() { [ -f "$JAR" ] && shasum -a 256 "$JAR" | cut -c1-12 || echo "absent"; }
# Reports the hash of $JAR by default, or of whatever path is passed — used to report the STAGED
# jar right after a build (before it has been swapped in) without ever changing what a bare
# `jar_id` (no args) means: the live path, $JAR. --check and the final "pid ..., jar ..." line
# both call it with no args on purpose, so neither can ever be fooled by a leftover staged file.
jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && shasum -a 256 "$f" | cut -c1-12 || echo "absent"; }
running_pid() { pgrep -f "$PATTERN" || true; }
# fleetd #493 — three small, independently testable pieces of "never build into the path a
# running process holds":
#
# stage_built_jar moves the jar Maven just produced OUT of the live path and onto the staging
# path, immediately after a successful build. Dies (leaving the OLD daemon
# untouched — this runs before the stop step) if Maven reported success but
# left no jar behind, or if the move itself fails.
# require_no_build_jar the --no-build path never builds or stages anything: it must find a
# jar already sitting at the live path from an earlier successful run, and
# die with the same truthful message this script has always used if not.
# wait_for_daemon_exit polls running_pid() for up to $1 seconds and reports whether the OLD
# daemon actually exited — extracted to its own function so the main flow
# can be relied on to call swap_staged_jar only AFTER this returns success,
# and so a test can prove that ordering by reading the script's own source.
# swap_staged_jar the actual swap: a plain `mv` of the staged jar onto the live path. Called
# only once the OLD daemon is confirmed gone (see wait_for_daemon_exit above),
# so this is never a write into a path a running process holds — by the time
# it runs, nothing holds that path anymore. If it fails, the caller must not
# start a new daemon: die() below already refuses that by exiting the script.
stage_built_jar() {
[ -f "$JAR" ] || die "build succeeded but produced no jar at $JAR — cannot stage it for restart.
The running daemon was NOT touched."
mv -f "$JAR" "$JAR_STAGED" \
|| die "could not move the freshly built jar from $JAR to the staging path $JAR_STAGED.
The running daemon was NOT touched."
}
require_no_build_jar() {
[ -f "$JAR" ] || die "no jar at $JAR — run without --no-build"
}
wait_for_daemon_exit() {
local timeout="$1" _i
for _i in $(seq "$timeout"); do
[ -z "$(running_pid)" ] && return 0
sleep 1
done
[ -z "$(running_pid)" ]
}
swap_staged_jar() {
local staged="$1" live="$2"
[ -f "$staged" ] || die "no staged jar at $staged to swap in — the daemon was NOT started."
mv -f "$staged" "$live" \
|| die "could not move the staged jar from $staged into place at $live — the daemon was NOT
started. The built jar is still sitting at $staged; a manual 'mv \"$staged\" \"$live\"'
may recover this once you find out why the move failed."
}
# fleetd #521 — the swap decision, and the step that acts on it.
#
# The defect: the swap step used to be guarded inline by `if [ "$DO_BUILD" = 1 ]` in the main flow.
# Changing that to `if false` left the suite green and the swap never ran, so a redeploy reported
# every step succeeding while the daemon started on no jar at all (stage_built_jar has already moved
# the freshly built one to $JAR_STAGED by then) or on a stale one.
# test_swap_ordered_after_wait_and_before_start could not catch it: it reads this script's own text
# and compares line positions, and a same-line edit moves no line.
#
# Why these are TWO functions, and why the second one exists at all. Extracting only the predicate
# — `should_swap`, which is what #521 asked for — is not enough, and this was measured, not guessed:
# with the main flow calling `if should_swap "$DO_BUILD"; then`, changing THAT to `if false; then`
# still left the whole suite at exit 0 with no failures. Tests that call a predicate directly prove
# the predicate is right; nothing makes the code that does the work consult it. Extraction had moved
# the untested decision one level up rather than removing it.
#
# So the decision and the action live together in swap_if_built, and the main flow has no guard of
# its own to get wrong — it calls one function unconditionally. A test then calls swap_if_built with
# both values of do_build and checks whether the swap actually happened, which fails if the guard is
# removed, inverted, or stops being consulted. should_swap stays a separate predicate because it is
# the decision itself and is worth naming and testing on its own.
#
# What this still does not pin: deleting the swap_if_built call from the main flow altogether. That
# is the ordering test's job — its needle is that call site — and no test in this file can do better,
# because sourcing stops before the main flow ever runs (see the SOURCED guard below).
should_swap() {
local do_build="$1"
[ "$do_build" = 1 ]
}
swap_if_built() {
local do_build="$1"
should_swap "$do_build" || return 0
say "swap"
swap_staged_jar "$JAR_STAGED" "$JAR"
ok "jar in place: $(jar_id)"
}
# `launchctl list <label>` exits 0 iff the label is loaded (registered with launchd) — true whether
# or not it is currently running, which is exactly "supervision is active" for our purposes. Read-
# only: neither helper below changes anything, so both are also safe under --check.
launchd_installed() { [ -f "$LAUNCHD_PLIST" ]; }
launchd_loaded() { launchctl list "$LAUNCHD_LABEL" >/dev/null 2>&1; }
# fleetd #492: same two questions for systemd --user. Kept as separate, overridable functions
# (never an inline `systemctl` call at each use site) so a test on a box with no systemd at all
# (this repo is developed on macOS) can substitute each one independently — the same seam
# launchd_installed/launchd_loaded above already use.
#
# fleetd #492 follow-up: both functions used to throw `systemctl`'s stderr straight into
# /dev/null, which meant "systemctl answered no" and "systemctl could not answer at all" (e.g. it
# cannot reach the user bus over a non-lingering ssh session) looked identical — both a plain
# nonzero exit. They now capture stderr separately and set their own *_ERRORED flag ONLY when the
# call exited non-zero AND wrote something to stderr — a real tool failure, never a clean "not
# installed"/"not active" answer (which exits non-zero with empty stderr). detect_supervisor reads
# the flag right after calling the probe, so a probe that could not answer routes to "unclear",
# never silently becomes "none".
#
# "installed": a unit FILE by this name exists, regardless of its current state — the systemd
# analogue of the plist file existing on disk. `list-unit-files` reads unit definitions without
# depending on runtime state, so this stays read-only and safe under --check.
systemd_installed() {
SYSTEMD_INSTALLED_ERRORED=0
command -v systemctl >/dev/null 2>&1 || return 1
local err_file out rc=0
if ! err_file="$(mktemp -t systemd-installed-err)"; then
SYSTEMD_INSTALLED_ERRORED=1
return 1
fi
out="$(systemctl --user list-unit-files "$SYSTEMD_UNIT.service" --no-legend 2>"$err_file")" || rc=$?
if [ "$rc" -ne 0 ]; then
if [ -s "$err_file" ]; then
SYSTEMD_INSTALLED_ERRORED=1
fi
rm -f "$err_file"
return "$rc"
fi
rm -f "$err_file"
printf '%s' "$out" | grep -q .
}
# "loaded": systemd currently supervises this unit as an active job — the systemd analogue of
# `launchctl list <label>` succeeding. Measured on the second host: `systemctl --user is-active
# fleetd` -> "active". A clean "no" (inactive/failed/activating/deactivating) exits non-zero with
# nothing on stderr; a probe that could not reach systemd at all exits non-zero WITH a stderr
# message — see the fleetd #492 follow-up note above.
systemd_loaded() {
SYSTEMD_LOADED_ERRORED=0
command -v systemctl >/dev/null 2>&1 || return 1
local err_file rc=0
if ! err_file="$(mktemp -t systemd-loaded-err)"; then
SYSTEMD_LOADED_ERRORED=1
return 1
fi
systemctl --user is-active "$SYSTEMD_UNIT" >/dev/null 2>"$err_file" || rc=$?
if [ "$rc" -ne 0 ] && [ -s "$err_file" ]; then
SYSTEMD_LOADED_ERRORED=1
fi
rm -f "$err_file"
return "$rc"
}
# fleetd #492: three real answers, not two — launchd, systemd, or genuinely unsupervised — plus a
# fourth, "ambiguous", for the one case this script cannot tell apart: both signals firing at once.
# That is exactly "I cannot tell who supervises this process", and guessing wrong here is how two
# daemons end up running against one herdr session (see trap 7 in the header).
#
# fleetd #492 follow-up: a fifth answer, "unclear", for two more situations that must NEVER be read
# as "none" (measured — see the report this ticket is a follow-up to):
# - installed-but-not-loaded, on EITHER supervisor. `systemctl --user is-active` answers "no" for
# `activating`, `deactivating`, `failed`, and while an auto-restart is pending — every one of
# those is a host that IS under systemd (or launchd) and whose supervisor is about to act again.
# `*_installed` already knows the unit/agent exists; this is the first place that fact is
# actually consulted in the decision, not just printed as a warning.
# - a probe that could not answer at all. systemd_loaded/systemd_installed set their own
# *_ERRORED flag (see the comment above them) when `systemctl` exits non-zero WITH a stderr
# message — a real tool failure, e.g. it cannot reach the user bus over a non-lingering ssh
# session — never conflated with a clean negative answer.
# "none" now means only: neither supervisor is installed, neither is loaded, and neither probe
# errored.
#
# fleetd #492 follow-up — constraints every caller of this function depends on (learned the hard
# way: an earlier version of this fix set a SUPERVISOR_UNCLEAR_DETAIL global from inside here and
# it was silently lost, because every real call site invokes this as `$(detect_supervisor)`):
# 1. It is called as `$(detect_supervisor)`, so ONLY STDOUT crosses back to the caller. Anything
# this function needs to tell its caller — the "unclear" detail included — must be printed,
# never assigned to a global: a global set inside a `$( )` subshell dies with that subshell.
# This function packs BOTH values (kind and detail) onto that one stdout line, joined by
# $SUPERVISOR_DETAIL_SEP, and the caller unpacks them on its own side of the subshell boundary.
# 2. This script runs under `set -euo pipefail` (line 50), so an unset variable is a loud
# failure. Do not add a `${VAR:-default}` anywhere downstream to paper over a value that
# should always be there — that hides a lost value instead of surfacing it (fleetd #497's
# defect class).
# 3. Every `case` on this function's return value needs an explicit final `*)` arm, chosen by
# whether that caller ACTS on the value (`die` — an unrecognised value must never be silently
# driven) or only DISPLAYS it (`echo`/`warn` and continue — a diagnostic must not go silent on
# exactly the value it most needs to report).
#
# Pure and side-effect-free besides the two *_ERRORED flags (read back within this same call, never
# by the caller — see the constraints above): reads the four probes and decides — never mutates
# anything, so it is safe under --check and testable by overriding
# launchd_installed/launchd_loaded/systemd_installed/systemd_loaded after sourcing.
detect_supervisor() {
local ld=0 sd=0 li=0 si=0 kind detail=""
SYSTEMD_LOADED_ERRORED=0
SYSTEMD_INSTALLED_ERRORED=0
launchd_loaded && ld=1
systemd_loaded && sd=1
launchd_installed && li=1
systemd_installed && si=1
if [ "$SYSTEMD_LOADED_ERRORED" = 1 ] || [ "$SYSTEMD_INSTALLED_ERRORED" = 1 ]; then
detail="the systemd --user probe for '$SYSTEMD_UNIT' could not answer cleanly (systemctl exited non-zero and reported an error on stderr, not a clean negative — e.g. it cannot reach the user bus)"
kind="unclear"
elif [ "$ld" = 1 ] && [ "$sd" = 1 ]; then
kind="ambiguous"
elif [ "$li" = 1 ] && [ "$ld" = 0 ]; then
detail="the launchd agent ($LAUNCHD_LABEL) is installed ($LAUNCHD_PLIST exists) but is not currently loaded"
kind="unclear"
elif [ "$si" = 1 ] && [ "$sd" = 0 ]; then
detail="the systemd --user unit ($SYSTEMD_UNIT) is installed but not currently active — it may be activating, deactivating, failed, or waiting on an auto-restart"
kind="unclear"
elif [ "$ld" = 1 ]; then
kind="launchd"
elif [ "$sd" = 1 ]; then
kind="systemd"
else
kind="none"
fi
printf '%s%s%s' "$kind" "$SUPERVISOR_DETAIL_SEP" "$detail"
}
# fleetd #492: turns anything detect_supervisor returns that is NOT exactly one of the two
# supervisors this script knows how to drive into a die() — never a fall-through to the `kill`
# path. Kept as its own function so a test can call it directly (in a subshell, since it die()s)
# without running the whole report-state flow or needing a real launchd/systemd.
require_drivable_supervisor() {
local kind="$1"
case "$kind" in
launchd|systemd|none) ;;
ambiguous)
die "both launchd ($LAUNCHD_LABEL) and systemd --user ($SYSTEMD_UNIT) report themselves as
loaded for this daemon at the same time. This script cannot tell which one actually
supervises the running process, and driving either alone risks the OTHER reviving the
OLD jar out from under it — the exact failure this ticket (fleetd #492) exists to
prevent. Stop one of the two supervisors by hand, confirm only one remains loaded, then
rerun." ;;
unclear)
# fleetd #492 follow-up: SUPERVISOR_UNCLEAR_DETAIL crosses back from detect_supervisor's
# subshell via its stdout, unpacked by the caller BEFORE it calls this function (see the
# constraints comment above detect_supervisor). No ${VAR:-default} here on purpose: if the
# detail is somehow missing, `set -u` makes this reference fail loudly instead of silently
# naming nothing — a default that hides a lost value is the same defect class as fleetd
# #497.
die "a supervisor looks present but this script cannot tell whether it actually drives this
daemon: $SUPERVISOR_UNCLEAR_DETAIL. Guessing wrong here is the same failure 'ambiguous'
above exists to prevent: driving the daemon while an unseen supervisor revives the OLD
jar out from under it (fleetd #492). Check 'launchctl list $LAUNCHD_LABEL' and
'systemctl --user status $SYSTEMD_UNIT' by hand, resolve whichever looks unclear, then
rerun." ;;
*)
die "detect_supervisor returned an unrecognized value '$kind' — refusing to guess which
supervisor, if any, controls this daemon." ;;
esac
}
# fleetd #492: the exact symptom a racing supervisor produces — count how many fleetd processes are
# alive right now. Takes the pid list as a parameter (rather than calling running_pid() itself) so a
# test can pass a canned two-line string without a real second process running. Pure except for the
# die() in assert_single_daemon below.
count_daemon_pids() {
local pids="$1"
if [ -z "$pids" ]; then
echo 0
else
printf '%s\n' "$pids" | grep -c .
fi
}
assert_single_daemon() {
local pids="$1" count
count="$(count_daemon_pids "$pids")"
if [ "$count" -gt 1 ]; then
die "more than one fleetd process is running after this restart (pids: $(printf '%s' "$pids" | tr '\n' ' ')).
This is the exact failure a racing supervisor produces: the OLD jar was revived by its
supervisor while this script started a NEW copy. Two daemons on one herdr session kill
each other's members. Investigate with 'pgrep -f \"$PATTERN\"' and stop the wrong one by
hand — do not assume either pid is the one you want."
fi
}
# CB-600: the script computes its own log path from where it sits on disk (REPO, above); the
# plist hard-codes an absolute StandardOutPath. Nothing forced the two to agree — if this script
# were ever run from a checkout other than the one the loaded plist names, launchd would start and
@@ -160,6 +490,35 @@ classify_amqp_connection_errors() {
REDEPLOY_UNEXPLAINED_ERRORS=$((REDEPLOY_UNEXPLAINED_ERRORS + pending_inbox + pending_lead_mailbox))
}
# fleetd #517: extracted so the suite can call this decision directly, the same way #510 extracted
# wait_for_daemon_exit so its ordering became checkable. Before this, the only test of the drain-gate
# abort message was a grep of this script's own source for the wording — so mutating the `if` below
# to `if false` (making the branch unreachable) left every test green, because the wording was still
# sitting in the file. Pure: only decides which message applies and prints it, no side effects, so a
# test can call it directly with an in-memory staged path instead of driving the real drain-gate flow
# (which needs a live $OLD_PID and an interactive prompt neither test can supply).
#
# The four cases:
# build ran, staged jar present -> names the staged jar and how to finish or discard it
# build ran, staged jar absent -> "nothing changed" (nothing was staged this run either)
# --no-build, staged jar present -> ALSO "nothing changed", deliberately: --no-build itself builds
# and stages nothing (see require_no_build_jar above), so a staged jar found here is a leftover
# from an earlier, unrelated run. THIS run truly changed nothing, and the next DO_BUILD=1 run
# wipes that leftover before it builds (`rm -f "$JAR_STAGED"` in the build section above) — so
# there is nothing here for the operator to lose track of.
# --no-build, staged jar absent -> "nothing changed"
drain_gate_refusal() {
local do_build="$1" staged_path="$2"
if [ "$do_build" = 1 ] && [ -f "$staged_path" ]; then
printf 'aborted — the running daemon was NOT touched, but the freshly built jar is sitting at
%s, not yet swapped into %s. Rerun WITHOUT --no-build to finish the restart —
the freshly built jar is no longer at the live path that --no-build requires — or
remove %s by hand if you want to discard this build.' "$staged_path" "$JAR" "$staged_path"
else
printf 'aborted — nothing changed'
fi
}
# CB-600: sourceable for testing. When this file is SOURCED (not executed) it stops here — nothing
# below runs — so a test harness can `source` it to call check_log_path_matches_plist (or the
# other pure helpers above) against a throwaway plist fixture without ever reaching the mutating
@@ -182,25 +541,57 @@ fi
ok "jar on disk: $(jar_id) ($([ -f "$JAR" ] && date -r "$JAR" '+%Y-%m-%d %H:%M:%S' || echo 'none'))"
ok "HEAD: $(git -C "$REPO" log --oneline -1)"
# CB-594: supervision state. Installed and loaded are different facts — a copied-but-never-loaded
# plist supervises nothing, and a loaded label with no file backing it (rare, but possible after an
# edited/moved plist) is still what launchd will act on.
# CB-594 / fleetd #492: supervision state. Installed and loaded are different facts — a
# copied-but-never-loaded plist (or an unloaded systemd unit) supervises nothing, and a loaded
# label/unit with no file backing it is still what its supervisor will act on.
if launchd_installed; then
ok "launchd agent installed: $LAUNCHD_PLIST"
else
warn "launchd agent NOT installed (no supervision — a crash will not restart the daemon)."
warn "launchd agent NOT installed."
fi
SUPERVISED=0
if launchd_loaded; then
SUPERVISED=1
ok "launchd agent loaded ($LAUNCHD_LABEL) — launchd supervises this daemon"
# CB-600: fail loudly here, before ANY other check runs, if this script and the loaded plist
# would read different log files — every check after this point is worthless otherwise.
check_log_path_matches_plist "$OUT" "$LAUNCHD_PLIST"
if systemd_installed; then
ok "systemd --user unit installed: $SYSTEMD_UNIT"
else
warn "launchd agent not loaded — this script is the only thing that will restart the daemon."
warn "systemd --user unit NOT installed ($SYSTEMD_UNIT)."
fi
# fleetd #492: decide which of the two (if either) actually supervises this daemon, and refuse
# outright — before touching anything — if that cannot be told apart (see require_drivable_
# supervisor above). --check reaches this same line, so a host with an undrivable supervisor is
# reported as a failure even in --check, without ever reaching the build/stop/start steps.
# fleetd #492 follow-up: detect_supervisor runs as $(...), so only the printed line survives —
# unpack kind and detail from it HERE, in this shell, before calling anything downstream. See the
# constraints comment above detect_supervisor for why this cannot be done any other way.
SUPERVISOR_RAW="$(detect_supervisor)"
SUPERVISOR_KIND="${SUPERVISOR_RAW%%"$SUPERVISOR_DETAIL_SEP"*}"
SUPERVISOR_UNCLEAR_DETAIL="${SUPERVISOR_RAW#*"$SUPERVISOR_DETAIL_SEP"}"
require_drivable_supervisor "$SUPERVISOR_KIND"
ok "supervisor detected: $SUPERVISOR_KIND"
SUPERVISED=0
case "$SUPERVISOR_KIND" in
launchd)
SUPERVISED=1
ok "launchd agent loaded ($LAUNCHD_LABEL) — launchd supervises this daemon"
# CB-600: fail loudly here, before ANY other check runs, if this script and the loaded plist
# would read different log files — every check after this point is worthless otherwise.
check_log_path_matches_plist "$OUT" "$LAUNCHD_PLIST"
;;
systemd)
SUPERVISED=1
ok "systemd --user unit active ($SYSTEMD_UNIT) — systemd supervises this daemon"
;;
none)
warn "no supervisor loaded — this script is the only thing that will restart the daemon."
;;
*)
# fleetd #492 follow-up: this block only DISPLAYS state, it changes nothing yet — so a value
# it doesn't recognise gets reported, not an abort that goes silent on exactly the state most
# worth seeing. (Unreachable today: require_drivable_supervisor above already died on
# "ambiguous"/"unclear" before this case runs. Guards the value nobody has invented yet.)
warn "unrecognised supervisor kind: '$SUPERVISOR_KIND' — detect_supervisor returned a value this block does not know; continuing to report the rest of the state."
;;
esac
# The trap with no log line. Checked in a LOGIN shell, because that is how the daemon is started
# below. Never prints the value — only whether it resolved.
if zsh -lc '[ -n "${WORKER_GITEA_TOKEN:-}" ]' 2>/dev/null; then
@@ -249,6 +640,9 @@ fi
if [ "$DO_BUILD" = 1 ]; then
say "build"
# fleetd #493: wipe a leftover staged jar from a previous failed/interrupted run BEFORE doing
# anything else, so that run's leftovers can never be mistaken for this run's output.
rm -f "$JAR_STAGED"
BUILD_LOG="$(mktemp -t fleetd-build)"
echo " log: $BUILD_LOG"
if ! mvn -f "$MODULE/pom.xml" clean install > "$BUILD_LOG" 2>&1; then
@@ -258,13 +652,19 @@ if [ "$DO_BUILD" = 1 ]; then
fi
grep -E '^\[INFO\] Tests run:.*Failures' "$BUILD_LOG" | tail -1 | sed 's/^\[INFO\] / /' || true
ok "BUILD SUCCESS"
ok "jar now: $(jar_id)"
# fleetd #493: move the freshly built jar off the live path immediately — the running (OLD)
# daemon, if any, is still up at this point (build always runs before stop). From here until the
# swap step below (after the OLD daemon is confirmed gone), $JAR_STAGED is the only artefact this
# script treats as "the new jar" — $JAR itself is not touched again until the swap.
stage_built_jar
ok "jar now: $(jar_id "$JAR_STAGED")"
else
say "build skipped (--no-build)"
# fleetd #493: --no-build never builds or stages anything — it restarts whatever jar is already
# sitting at the live path from an earlier successful run. Same check, same message as before.
require_no_build_jar
fi
[ -f "$JAR" ] || die "no jar at $JAR — run without --no-build"
# ----------------------------------------------------------------- drain gate
if [ -n "$OLD_PID" ] && [ "$ASSUME_YES" = 0 ]; then
@@ -276,82 +676,143 @@ if [ -n "$OLD_PID" ] && [ "$ASSUME_YES" = 0 ]; then
echo " you still want, BEFORE continuing."
echo
read -r -p " Fleet drained? type yes to restart: " reply
[ "$reply" = "yes" ] || die "aborted — nothing changed"
if [ "$reply" != "yes" ]; then
# fleetd #493 / #517: "nothing changed" would be a lie once a build has run and staged a jar —
# see drain_gate_refusal above for the full decision and why each of its four cases reads the
# way it does.
die "$(drain_gate_refusal "$DO_BUILD" "$JAR_STAGED")"
fi
fi
# ------------------------------------------------------------------ stop
#
# CB-594: when SUPERVISED, launchd owns the stop — never a raw `kill` here. A bare SIGTERM makes
# this JVM exit 143 even with its shutdown hook running to completion (verified separately: a
# throwaway Java process with an equivalent shutdown hook, sent SIGTERM from a login shell that
# could `wait` on it directly, reported exit code 143 every time — never 0). launchd's
# KeepAlive.SuccessfulExit=false treats any nonzero exit as a crash and restarts the OLD jar,
# which would race this script's own restart of the NEW one. `launchctl unload` avoids that race
# by deregistering the job first, so no KeepAlive is left armed when the process actually stops.
# CB-594 / fleetd #492: when SUPERVISED, the supervisor owns the stop — never a raw `kill` here. A
# bare SIGTERM makes this JVM exit 143 even with its shutdown hook running to completion (verified
# separately: a throwaway Java process with an equivalent shutdown hook, sent SIGTERM from a login
# shell that could `wait` on it directly, reported exit code 143 every time — never 0). launchd's
# KeepAlive.SuccessfulExit=false and systemd's Restart=on-failure both treat any nonzero exit as a
# crash and restart the OLD jar, which would race this script's own restart of the NEW one.
# `launchctl unload` avoids that race by deregistering the job first, so no KeepAlive is left
# armed when the process actually stops. `systemctl --user stop` needs no such dance: unlike
# KeepAlive, systemd's Restart= does not fire on a deliberate stop, only on an unexpected exit of
# an active unit.
if [ -n "$OLD_PID" ]; then
say "stop"
RESTART_MARK="$(wc -l < "$OUT" 2>/dev/null || echo 0)" # verify a FRESH line appears later
if [ "$SUPERVISED" = 1 ]; then
echo " supervision is ON: using 'launchctl unload' (not kill) so launchd's own KeepAlive"
echo " cannot restart the OLD jar out from under this script — see the CB-594 comment above."
launchctl unload -w "$LAUNCHD_PLIST" \
|| die "launchctl unload failed — the daemon may still be under supervision; investigate before retrying"
else
kill "$OLD_PID"
fi
for _ in $(seq "$STOP_WAIT"); do
[ -z "$(running_pid)" ] && break
sleep 1
done
if [ -n "$(running_pid)" ]; then
case "$SUPERVISOR_KIND" in
launchd)
echo " supervision is ON (launchd): using 'launchctl unload' (not kill) so launchd's own"
echo " KeepAlive cannot restart the OLD jar out from under this script — see the CB-594"
echo " comment above."
launchctl unload -w "$LAUNCHD_PLIST" \
|| die "launchctl unload failed — the daemon may still be under supervision; investigate before retrying"
;;
systemd)
echo " supervision is ON (systemd --user): using 'systemctl --user stop' (not kill) so"
echo " systemd's own Restart=on-failure cannot restart the OLD jar out from under this"
echo " script — see the fleetd #492 comment above."
systemctl --user stop "$SYSTEMD_UNIT" \
|| die "'systemctl --user stop $SYSTEMD_UNIT' failed — the daemon may still be under supervision; investigate before retrying"
;;
none)
kill "$OLD_PID"
;;
*)
# fleetd #492 follow-up: this block ACTS (stops the daemon one specific way per kind) — an
# unrecognised value must never fall through to a default action, silently picking the wrong
# one (or none at all) while reporting success. (Unreachable today: require_drivable_
# supervisor already died before this runs. Guards the value nobody has invented yet.)
die "detect_supervisor returned an unrecognized value '$SUPERVISOR_KIND' at the stop step —
refusing to guess how to stop a daemon under an unknown supervisor. The daemon was NOT
stopped." ;;
esac
if ! wait_for_daemon_exit "$STOP_WAIT"; then
die "pid $OLD_PID still alive after ${STOP_WAIT}s. Not escalating to kill -9 automatically:
the shutdown hook releases sessions and worktrees in order, and killing it hard can
leave worktrees and panes behind. Investigate, then kill -9 by hand if you accept that."
fi
ok "pid $OLD_PID exited"
elif [ "$SUPERVISED" = 1 ]; then
elif [ "$SUPERVISOR_KIND" = "launchd" ]; then
# Loaded but not currently running (e.g. throttled after a crash loop). Unload it anyway so the
# start step below does a clean load, never a load stacked on an already-loaded label.
say "stop"
RESTART_MARK="$(wc -l < "$OUT" 2>/dev/null || echo 0)"
launchctl unload -w "$LAUNCHD_PLIST" 2>/dev/null || true
ok "launchd agent unloaded (was already not running)"
elif [ "$SUPERVISOR_KIND" = "systemd" ]; then
# Same case for systemd: the unit is known/active-capable but not currently running. `stop` on an
# already-stopped unit is a harmless no-op — kept for symmetry with the launchd branch above.
say "stop"
RESTART_MARK="$(wc -l < "$OUT" 2>/dev/null || echo 0)"
systemctl --user stop "$SYSTEMD_UNIT" 2>/dev/null || true
ok "systemd --user unit stopped (was already not running)"
else
RESTART_MARK="$(wc -l < "$OUT" 2>/dev/null || echo 0)"
fi
# ------------------------------------------------------------------ swap
#
# fleetd #493: every branch above has now either confirmed the OLD daemon actually exited
# (wait_for_daemon_exit, above) or established there was never one running to begin with. Only
# NOW is it safe to put the freshly built jar at the path the NEXT `java -jar` (direct, or via
# launchd/systemd's ExecStart) will read from — this mv is the one and only write to $JAR anywhere
# in this script's mutating flow. If it fails, do not start: die() below exits before "start" runs.
swap_if_built "$DO_BUILD"
# ------------------------------------------------------------------ start
# Unsupervised: login shell (zsh -l) is what puts the secrets on the daemon's environment, and cwd
# must be fleetd/ because the daemon resolves fleetd.yaml, logs/ and target/ relative to it.
# Supervised: launchd does both — deploy/dev.ltms.fleetd.plist points ProgramArguments at
# Supervised (launchd): launchd does both — deploy/dev.ltms.fleetd.plist points ProgramArguments at
# scripts/fleetd-launchd-wrapper.sh (CB-594), which is what execs the login shell in launchd's
# place, and WorkingDirectory in the plist already pins fleetd/.
# Supervised (systemd --user): the unit does both too — measured on the second host, ExecStart is
# `/bin/zsh -lc "exec java -jar target/fleetd.jar fleetd.yaml"` (a login shell, same reason as
# above) and WorkingDirectory is already pinned to fleetd/.
say "start"
if [ "$SUPERVISED" = 1 ]; then
echo " supervision is ON: using 'launchctl load' so launchd starts and keeps supervising this"
echo " process, instead of a manual nohup that launchd would know nothing about."
# CB-600: 'launchctl unload -w' above already persisted Disabled=true for this label. A load -w
# that succeeds clears it; a load -w that FAILS leaves the agent both stopped and disabled — worse
# than before this script ran, because a later reboot or login will not bring it back either. One
# retry covers a transient race (e.g. launchd not yet fully done deregistering); if it still fails,
# die with the exact recovery command rather than a bare "failed".
if ! launchctl load -w "$LAUNCHD_PLIST" 2>/dev/null; then
warn "launchctl load failed on the first attempt — retrying once after a short pause"
sleep 2
launchctl load -w "$LAUNCHD_PLIST" || die "launchctl load failed twice.
The agent is now STOPPED and DISABLED — it will NOT come back on its own, not even after a
reboot or login, because 'launchctl unload -w' above persisted Disabled=true and load -w
never got the chance to clear it. Recover with:
launchctl load -w \"$LAUNCHD_PLIST\"
If that still fails, check 'launchctl list $LAUNCHD_LABEL', validate the plist with
'plutil -lint \"$LAUNCHD_PLIST\"', and check $OUT before assuming a retry will succeed."
fi
else
# Absolute jar path so `ps` names which checkout is running.
( cd "$MODULE" && zsh -lc "nohup java -jar '$JAR' >> fleetd.out 2>&1 &" )
fi
case "$SUPERVISOR_KIND" in
launchd)
echo " supervision is ON (launchd): using 'launchctl load' so launchd starts and keeps"
echo " supervising this process, instead of a manual nohup that launchd would know nothing"
echo " about."
# CB-600: 'launchctl unload -w' above already persisted Disabled=true for this label. A load -w
# that succeeds clears it; a load -w that FAILS leaves the agent both stopped and disabled — worse
# than before this script ran, because a later reboot or login will not bring it back either. One
# retry covers a transient race (e.g. launchd not yet fully done deregistering); if it still fails,
# die with the exact recovery command rather than a bare "failed".
if ! launchctl load -w "$LAUNCHD_PLIST" 2>/dev/null; then
warn "launchctl load failed on the first attempt — retrying once after a short pause"
sleep 2
launchctl load -w "$LAUNCHD_PLIST" || die "launchctl load failed twice.
The agent is now STOPPED and DISABLED — it will NOT come back on its own, not even after a
reboot or login, because 'launchctl unload -w' above persisted Disabled=true and load -w
never got the chance to clear it. Recover with:
launchctl load -w \"$LAUNCHD_PLIST\"
If that still fails, check 'launchctl list $LAUNCHD_LABEL', validate the plist with
'plutil -lint \"$LAUNCHD_PLIST\"', and check $OUT before assuming a retry will succeed."
fi
;;
systemd)
echo " supervision is ON (systemd --user): using 'systemctl --user start' so systemd starts"
echo " and keeps supervising this process, instead of a manual nohup it would know nothing"
echo " about."
systemctl --user start "$SYSTEMD_UNIT" || die "'systemctl --user start $SYSTEMD_UNIT' failed.
Check 'systemctl --user status $SYSTEMD_UNIT' and $OUT before assuming a retry will succeed."
;;
none)
# Absolute jar path so `ps` names which checkout is running.
( cd "$MODULE" && zsh -lc "nohup java -jar '$JAR' >> fleetd.out 2>&1 &" )
;;
*)
# fleetd #492 follow-up: this block ACTS (starts the daemon one specific way per kind) — an
# unrecognised value must never fall through to a default action, silently picking the wrong
# one (or none at all) while reporting success. (Unreachable today: require_drivable_
# supervisor already died before this runs. Guards the value nobody has invented yet.)
die "detect_supervisor returned an unrecognized value '$SUPERVISOR_KIND' at the start step —
refusing to guess how to start a daemon under an unknown supervisor. The daemon was NOT
started." ;;
esac
for _ in $(seq 10); do
NEW_PID="$(running_pid)"
@@ -411,6 +872,13 @@ FRESH_LOG="$(mktemp -t fleetd-fresh-log)"
trap 'rm -f "$FRESH_LOG"' EXIT
tail -n "+$((RESTART_MARK + 1))" "$OUT" > "$FRESH_LOG" 2>/dev/null || true
classify_amqp_connection_errors "$FRESH_LOG"
# fleetd #492: checked here, after healthz and the fresh-log check have both had time to run, so a
# supervisor that revives the OLD jar a few seconds late is caught too. Every check above (healthz
# 200, jar id, the fresh 'listening' line) is satisfied by EITHER daemon if two are alive — this is
# the only one that can tell.
assert_single_daemon "$(running_pid)"
say "result"
ok "pid $NEW_PID, jar $(jar_id)"
if [ "$REDEPLOY_ERROR_COUNT" -eq 0 ]; then
+109
View File
@@ -0,0 +1,109 @@
#!/usr/bin/env bash
# Self-contained checks for the policy parsing guards in probe-member-credentials.sh.
set -euo pipefail
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
PROBE="$ROOT/scripts/probe-member-credentials.sh"
TMP="$(mktemp -d "$ROOT/.probe-member-credentials-test.XXXXXX")"
trap 'rm -rf "$TMP"' EXIT
# The SOURCED guard exposes this pure parser without contacting POLICY_URL.
source "$PROBE"
fail() {
printf 'FAIL: %s\n' "$*" >&2
return 1
}
assert_equals() {
local expected="$1" actual="$2" description="$3"
[ "$expected" = "$actual" ] || fail "$description: expected $expected, got $actual"
}
assert_contains() {
local needle="$1" text="$2" description="$3"
printf '%s' "$text" | grep -qF "$needle" || fail "$description: missing $needle"
}
make_jq() {
local body="$1"
mkdir -p "$TMP/bin"
printf '%s\n' '#!/usr/bin/env bash' "$body" > "$TMP/bin/jq"
chmod +x "$TMP/bin/jq"
}
run_parser() {
local output rc=0
POLICY_JSON="$(< "$TMP/policy.json")"
POLICY_URL="fixture://member-credentials"
output="$(PATH="$TMP/bin:$PATH" parse_policy_fields 2>&1)" || rc=$?
PARSER_OUTPUT="$output"
PARSER_RC="$rc"
}
test_bash_older_than_four_refuses() {
local output rc=0 version
version="$(/bin/bash -c 'printf %s "$BASH_VERSION"')"
output="$(/bin/bash "$PROBE" 2>&1)" || rc=$?
assert_equals 3 "$rc" "bash 3 refusal status"
assert_contains 'This shell is bash' "$output" "bash 3 refusal"
assert_contains "$version" "$output" "bash 3 refusal version"
}
test_parser_non_zero_refuses() {
make_jq 'exit 17'
run_parser
assert_equals 4 "$PARSER_RC" "parser failure status"
assert_contains 'jq exited non-zero (status 17)' "$PARSER_OUTPUT" "parser failure message"
}
test_short_parser_output_refuses() {
make_jq "printf '%s\\n' true enforce 3 2"
run_parser
assert_equals 5 "$PARSER_RC" "short parser output status"
assert_contains 'policy parser (jq) returned 4 field(s)' "$PARSER_OUTPUT" "short parser output count"
}
test_empty_parser_output_reports_zero_fields() {
make_jq ':'
run_parser
assert_equals 5 "$PARSER_RC" "empty parser output status"
assert_contains 'policy parser (jq) returned 0 field(s)' "$PARSER_OUTPUT" "empty parser output count"
}
test_well_formed_policy_prints_name_table() {
local output rc=0
make_jq "cat '$TMP/policy.fields'"
# Shell functions cannot be passed in an environment assignment. Run the executable through bash.
output="$(BRIDGED_MEMBER=1 FIXTURE="$TMP/policy.json" PROBE="$PROBE" PATH="$TMP/bin:$PATH" bash -c '
curl() { cat "$FIXTURE"; }
export -f curl
exec "$PROBE"
' 2>&1)" || rc=$?
assert_equals 0 "$rc" "well-formed policy status"
assert_contains 'ALPHA_TOKEN' "$output" "name table"
assert_contains 'BETA_TOKEN' "$output" "name table"
assert_contains 'GAMMA_TOKEN' "$output" "name table"
}
cat > "$TMP/policy.json" <<'JSON'
{"present":true,"policy":"enforce","knownCount":3,"allowedCount":2,"blockedCount":1,"known":["ALPHA_TOKEN","BETA_TOKEN","GAMMA_TOKEN"]}
JSON
cat > "$TMP/policy.fields" <<'FIELDS'
true
enforce
3
2
1
ALPHA_TOKEN
BETA_TOKEN
GAMMA_TOKEN
FIELDS
test_bash_older_than_four_refuses
test_parser_non_zero_refuses
test_short_parser_output_refuses
test_empty_parser_output_reports_zero_fields
test_well_formed_policy_prints_name_table
printf 'PASS: probe member credentials guards\n'
+529
View File
@@ -25,6 +25,502 @@ classify_fixture() {
classify_amqp_connection_errors "$TMP/$name"
}
# fleetd #492 follow-up: detect_supervisor's stdout is now "kind<SEP>detail" (see the constraints
# comment above detect_supervisor in redeploy-fleetd.sh) — every test below that only cares about
# the kind must split it out with the SAME in-shell parameter expansion the real call site (:438)
# uses, never a bare string comparison against the raw output.
supervisor_kind_of() {
printf '%s' "${1%%"$SUPERVISOR_DETAIL_SEP"*}"
}
supervisor_detail_of() {
printf '%s' "${1#*"$SUPERVISOR_DETAIL_SEP"}"
}
# fleetd #492 — supervisor detection. Detect_supervisor() reads launchd_loaded/systemd_loaded, so
# each test overrides BOTH pairs (installed + loaded) explicitly, rather than relying on either
# being naturally absent: this machine may itself be running a real fleetd under launchd right now
# (see CLAUDE.md/MEMORY.md — launchd supervision has been live here since 2026-08-26), so leaving
# launchd_loaded unmocked in a "systemd only" test would silently read this host's own live state
# instead of the fixture.
test_detect_supervisor_launchd_only() {
launchd_installed() { return 0; }
launchd_loaded() { return 0; }
systemd_installed() { return 1; }
systemd_loaded() { return 1; }
assert_equals "launchd" "$(supervisor_kind_of "$(detect_supervisor)")" "launchd-only detection"
}
test_detect_supervisor_systemd_only() {
launchd_installed() { return 1; }
launchd_loaded() { return 1; }
systemd_installed() { return 0; }
systemd_loaded() { return 0; }
assert_equals "systemd" "$(supervisor_kind_of "$(detect_supervisor)")" "systemd-only detection"
}
test_detect_supervisor_none() {
launchd_installed() { return 1; }
launchd_loaded() { return 1; }
systemd_installed() { return 1; }
systemd_loaded() { return 1; }
assert_equals "none" "$(supervisor_kind_of "$(detect_supervisor)")" "unsupervised detection"
}
# fleetd #492 follow-up — detect_supervisor must never answer "none" when the truth is "could not
# tell". `systemd_installed`/`systemd_loaded` already know a unit file exists; this proves that
# fact is now actually consulted, not just printed as a warning: an installed-but-not-loaded unit
# reads as unclear, because is-active answers "no" for activating/deactivating/failed/pending
# auto-restart too, and every one of those is a host that IS under systemd.
test_detect_supervisor_systemd_installed_not_loaded_is_unclear() {
launchd_installed() { return 1; }
launchd_loaded() { return 1; }
systemd_installed() { return 0; } # the unit file IS there
systemd_loaded() { return 1; } # is-active says no — could be activating/failed/pending restart
local raw
raw="$(detect_supervisor)"
assert_equals "unclear" "$(supervisor_kind_of "$raw")" "systemd installed-but-not-loaded must read as unclear, not none"
printf '%s' "$(supervisor_detail_of "$raw")" | grep -qF "$SYSTEMD_UNIT" \
|| fail "detail does not name the systemd unit it found installed-but-not-loaded"
}
# Same fact, the launchd side: a plist on disk that is not currently loaded (unloaded without being
# removed, or about to be reloaded) must not read as "no supervisor" either.
test_detect_supervisor_launchd_installed_not_loaded_is_unclear() {
launchd_installed() { return 0; } # the plist IS there
launchd_loaded() { return 1; } # launchctl list says not loaded
systemd_installed() { return 1; }
systemd_loaded() { return 1; }
local raw
raw="$(detect_supervisor)"
assert_equals "unclear" "$(supervisor_kind_of "$raw")" "launchd installed-but-not-loaded must read as unclear, not none"
printf '%s' "$(supervisor_detail_of "$raw")" | grep -qF "$LAUNCHD_LABEL" \
|| fail "detail does not name the launchd label it found installed-but-not-loaded"
}
# Drives the REAL systemd_loaded/systemd_installed bodies (never stubbed) through a `systemctl`
# stub placed first on PATH that exits non-zero AND writes to stderr — the shape of a systemctl
# that runs but cannot reach the user bus (measured elsewhere as a headless ssh session with no
# lingering). This must read as unclear, never none: a probe that could not answer at all is not
# the same fact as "no supervisor is loaded".
test_detect_supervisor_systemd_probe_error_is_unclear() {
# Re-source first to restore the REAL launchd_*/systemd_* probe bodies. Earlier tests in this
# file permanently override them with stub `return 0`/`return 1` bodies (that is the whole point
# of those tests), and a bash function definition is global for the rest of the process — without
# this, systemd_loaded here would still be whatever the previous test left it as, never touching
# a real `systemctl` call at all.
source "$ROOT/scripts/redeploy-fleetd.sh"
local bin_dir result rc=0
bin_dir="$TMP/stub-bin-systemctl-errors"
mkdir -p "$bin_dir"
cat > "$bin_dir/systemctl" <<'STUB'
#!/usr/bin/env bash
echo "Failed to connect to bus: No such file or directory" >&2
exit 1
STUB
chmod +x "$bin_dir/systemctl"
PATH="$bin_dir:$PATH" systemd_loaded && rc=0 || rc=$?
[ "$rc" -ne 0 ] || fail "systemd_loaded must not report loaded=true when systemctl only errored"
assert_equals "1" "$SYSTEMD_LOADED_ERRORED" "systemd_loaded must flag a probe error, not a clean negative"
launchd_installed() { return 1; }
launchd_loaded() { return 1; }
result="$(PATH="$bin_dir:$PATH" detect_supervisor)"
assert_equals "unclear" "$(supervisor_kind_of "$result")" "a systemd probe error must read as unclear, not none"
printf '%s' "$(supervisor_detail_of "$result")" | grep -qF "$SYSTEMD_UNIT" \
|| fail "detail does not name the systemd unit whose probe errored"
}
# fleetd #492 follow-up (Item 1): this must go through the REAL call-site shape at :437-440, not a
# hand-constructed "unclear" value — a test that builds "unclear" directly proves the switch, not
# the handoff, and that is exactly the gap that let SUPERVISOR_UNCLEAR_DETAIL never reach the real
# caller in b17f37a. detect_supervisor runs as $(detect_supervisor): a subshell. Only stdout
# survives that boundary, so kind AND detail must both cross on it — this test proves they do.
test_require_drivable_supervisor_refuses_unclear() {
launchd_installed() { return 1; }
launchd_loaded() { return 1; }
systemd_installed() { return 0; }
systemd_loaded() { return 1; }
local SUPERVISOR_RAW SUPERVISOR_KIND SUPERVISOR_UNCLEAR_DETAIL output rc=0
# Exactly what :437-439 does — do not shortcut this by constructing "unclear" by hand.
SUPERVISOR_RAW="$(detect_supervisor)"
SUPERVISOR_KIND="${SUPERVISOR_RAW%%"$SUPERVISOR_DETAIL_SEP"*}"
SUPERVISOR_UNCLEAR_DETAIL="${SUPERVISOR_RAW#*"$SUPERVISOR_DETAIL_SEP"}"
assert_equals "unclear" "$SUPERVISOR_KIND" "setup: expected unclear before testing the refusal"
[ -n "$SUPERVISOR_UNCLEAR_DETAIL" ] \
|| fail "detail did not survive the \$(...) call-site boundary — SUPERVISOR_UNCLEAR_DETAIL is empty in the parent shell"
printf '%s' "$SUPERVISOR_UNCLEAR_DETAIL" | grep -qF "$SYSTEMD_UNIT" \
|| fail "detail that crossed the subshell boundary does not name the systemd unit it found installed-but-not-loaded"
output="$(require_drivable_supervisor "$SUPERVISOR_KIND" 2>&1)" || rc=$?
[ "$rc" -ne 0 ] || fail "require_drivable_supervisor accepted an unclear (undrivable) supervisor"
# Check for the ACTUAL DETAIL TEXT, not just "$SYSTEMD_UNIT" — the die() message's boilerplate
# recovery instructions name the unit unconditionally either way ("systemctl --user status
# $SYSTEMD_UNIT"), so a bare unit-name grep here would pass even on a lost/fallback detail. Only
# the specific detail string proves the crossed value, not the boilerplate, reached the message.
printf '%s' "$output" | grep -qF "$SUPERVISOR_UNCLEAR_DETAIL" \
|| fail "refusal message does not contain the specific detail that crossed the subshell boundary"
}
# The heart of the ticket: a supervisor this script cannot drive must refuse, never fall through to
# `kill`. require_drivable_supervisor die()s, so it is invoked inside a command substitution — that
# forks a subshell, so its exit() only ends the subshell and this test script keeps running under
# `set -e`.
test_require_drivable_supervisor_refuses_ambiguous() {
local output rc=0
output="$(require_drivable_supervisor "ambiguous" 2>&1)" || rc=$?
[ "$rc" -ne 0 ] || fail "require_drivable_supervisor accepted an ambiguous (undrivable) supervisor"
printf '%s' "$output" | grep -qF "$LAUNCHD_LABEL" \
|| fail "refusal message does not name the launchd label it found"
printf '%s' "$output" | grep -qF "$SYSTEMD_UNIT" \
|| fail "refusal message does not name the systemd unit it found"
}
test_require_drivable_supervisor_accepts_known_kinds() {
require_drivable_supervisor "launchd" || fail "refused a drivable launchd supervisor"
require_drivable_supervisor "systemd" || fail "refused a drivable systemd supervisor"
require_drivable_supervisor "none" || fail "refused the unsupervised case"
}
# fleetd #492 — the one-daemon check. Two live pids is the exact symptom a racing supervisor
# produces, and none of the other post-restart checks (healthz, jar id, the fresh log line) can see
# it because either daemon alone satisfies them.
test_count_daemon_pids() {
assert_equals 0 "$(count_daemon_pids "")" "count of an empty pid list"
assert_equals 1 "$(count_daemon_pids "4242")" "count of a single pid"
assert_equals 2 "$(count_daemon_pids "$(printf '4242\n4343\n')")" "count of two pids"
}
test_assert_single_daemon_accepts_one_pid() {
assert_single_daemon "4242" || fail "assert_single_daemon rejected a single running pid"
}
test_assert_single_daemon_rejects_two_pids() {
local output rc=0
output="$(assert_single_daemon "$(printf '4242\n4343\n')" 2>&1)" || rc=$?
[ "$rc" -ne 0 ] || fail "assert_single_daemon accepted two simultaneously running pids"
printf '%s' "$output" | grep -qF '4242' || fail "refusal message does not list the pids it found"
printf '%s' "$output" | grep -qF '4343' || fail "refusal message does not list the pids it found"
}
# fleetd #511 — jar_id()'s no-argument default was unpinned by any test: nothing proved it reports
# $JAR (the live path) rather than $JAR_STAGED. Both halves matter, so this pins both: the bare call
# must hash the live jar, and an explicit path argument must hash THAT file, not fall back to $JAR.
# Two files with different content, so a default pointed at the wrong one reports the wrong hash
# rather than accidentally matching.
test_jar_id_defaults_to_live_and_reports_explicit_path() {
local dir saved_jar="$JAR" saved_staged="$JAR_STAGED"
local live_hash staged_hash default_result explicit_result
dir="$TMP/jar-id"; mkdir -p "$dir"
JAR="$dir/fleetd.jar"; JAR_STAGED="$dir/fleetd-new.jar"
printf 'live jar bytes' > "$JAR"
printf 'staged jar bytes, not the same content' > "$JAR_STAGED"
live_hash="$(shasum -a 256 "$JAR" | cut -c1-12)"
staged_hash="$(shasum -a 256 "$JAR_STAGED" | cut -c1-12)"
default_result="$(jar_id)"
explicit_result="$(jar_id "$JAR_STAGED")"
JAR="$saved_jar"; JAR_STAGED="$saved_staged"
[ "$live_hash" != "$staged_hash" ] || fail "test fixture error: live and staged jars hashed the same"
assert_equals "$live_hash" "$default_result" "jar_id with no arguments must report the hash of \$JAR"
assert_equals "$staged_hash" "$explicit_result" "jar_id \"\$JAR_STAGED\" must report the hash of the staged jar, not fall back to \$JAR"
}
# fleetd #517 — jar_id()'s "absent" branch was unpinned by any test: the existing test above (#511)
# proves both halves of the present-file contract but never exercises the missing-file path. This
# word matters more than a string usually would: "absent" is the #413 signal that a `mvn clean`
# deleted the running daemon's jar out from under it, and the `redeploy-fleetd` skill points
# operators at `--check` for exactly this. Covers both the no-argument default and an explicit path,
# since the mutation (`absent` -> `present`) sits on the single shared `|| echo` and would flip both.
test_jar_id_reports_absent_for_missing_file() {
local saved_jar="$JAR" dir default_result explicit_result
dir="$TMP/jar-id-absent"; mkdir -p "$dir"
JAR="$dir/does-not-exist.jar"
[ ! -f "$JAR" ] || fail "test fixture error: \$JAR unexpectedly exists at $JAR"
default_result="$(jar_id)"
explicit_result="$(jar_id "$dir/also-does-not-exist.jar")"
JAR="$saved_jar"
assert_equals "absent" "$default_result" "jar_id with no arguments must report absent when \$JAR does not exist"
assert_equals "absent" "$explicit_result" "jar_id with an explicit missing path must report absent"
}
# fleetd #493 — never build into the path a running process holds. stage_built_jar/swap_staged_jar
# are exercised directly against real files on disk (not stubs), because the whole point is file
# behavior (does the content move, does the source disappear, does a failure leave both sides
# intact) that a stubbed function cannot prove.
test_stage_built_jar_moves_off_live_path() {
local dir jar staged saved_jar="$JAR" saved_staged="$JAR_STAGED"
dir="$TMP/stage-ok"; mkdir -p "$dir"
jar="$dir/fleetd.jar"; staged="$dir/fleetd-new.jar"
printf 'built jar bytes' > "$jar"
JAR="$jar"; JAR_STAGED="$staged"
stage_built_jar || fail "stage_built_jar rejected a real build output"
JAR="$saved_jar"; JAR_STAGED="$saved_staged"
[ ! -f "$jar" ] || fail "stage_built_jar left the jar behind at the live path $jar"
[ -f "$staged" ] || fail "stage_built_jar did not create the staged jar at $staged"
grep -qF 'built jar bytes' "$staged" || fail "staged jar does not carry the built content"
}
test_stage_built_jar_dies_when_build_produced_nothing() {
local dir output rc=0 saved_jar="$JAR" saved_staged="$JAR_STAGED"
dir="$TMP/stage-missing"; mkdir -p "$dir"
JAR="$dir/fleetd.jar"; JAR_STAGED="$dir/fleetd-new.jar"
output="$(stage_built_jar 2>&1)" || rc=$?
JAR="$saved_jar"; JAR_STAGED="$saved_staged"
[ "$rc" -ne 0 ] || fail "stage_built_jar accepted a missing build output"
printf '%s' "$output" | grep -qF "$dir/fleetd.jar" \
|| fail "refusal message does not name the missing jar path"
}
test_swap_staged_jar_moves_staged_onto_live() {
local dir staged live
dir="$TMP/swap-ok"; mkdir -p "$dir"
staged="$dir/fleetd-new.jar"; live="$dir/fleetd.jar"
printf 'swapped jar bytes' > "$staged"
swap_staged_jar "$staged" "$live" || fail "swap_staged_jar rejected a real staged jar"
[ ! -f "$staged" ] || fail "swap_staged_jar left the staged file behind at $staged"
[ -f "$live" ] || fail "swap_staged_jar did not create the live jar at $live"
grep -qF 'swapped jar bytes' "$live" || fail "live jar does not carry the staged content"
}
# The heart of the ticket's item 3: a failed swap must refuse to start. This function dies on
# failure, and die() exits — so like the require_drivable_supervisor tests above, the call goes
# inside a command substitution to contain that exit to a subshell.
test_swap_staged_jar_dies_without_staged_file() {
local dir output rc=0
dir="$TMP/swap-missing"; mkdir -p "$dir"
output="$(swap_staged_jar "$dir/fleetd-new.jar" "$dir/fleetd.jar" 2>&1)" || rc=$?
[ "$rc" -ne 0 ] || fail "swap_staged_jar accepted a missing staged jar"
[ ! -f "$dir/fleetd.jar" ] || fail "swap_staged_jar must not create the live jar when nothing was staged"
printf '%s' "$output" | grep -qF "$dir/fleetd-new.jar" \
|| fail "refusal message does not name the missing staged path"
}
test_swap_staged_jar_dies_when_mv_fails() {
local dir staged live output rc=0
dir="$TMP/swap-fail"; mkdir -p "$dir/src"
staged="$dir/src/fleetd-new.jar"
printf 'fake jar bytes' > "$staged"
live="$dir/no-such-dir/fleetd.jar" # parent directory does not exist -> mv fails
output="$(swap_staged_jar "$staged" "$live" 2>&1)" || rc=$?
[ "$rc" -ne 0 ] || fail "swap_staged_jar accepted a failing mv"
[ -f "$staged" ] || fail "swap_staged_jar must leave the staged jar in place when the move fails"
[ ! -f "$live" ] || fail "swap_staged_jar must not report success when the move failed"
printf '%s' "$output" | grep -qF "$staged" \
|| fail "refusal message does not name the staged path that could not be moved"
}
# --no-build must still resolve $JAR (never the staged path — there is nothing to stage on this
# path) and must still die with the exact wording documented in the script's own header comment.
test_require_no_build_jar_dies_when_absent() {
local saved_jar="$JAR" output rc=0 missing="$TMP/no-build-absent/fleetd.jar"
JAR="$missing"
output="$(require_no_build_jar 2>&1)" || rc=$?
JAR="$saved_jar"
[ "$rc" -ne 0 ] || fail "require_no_build_jar accepted a missing jar"
printf '%s' "$output" | grep -qF "no jar at $missing — run without --no-build" \
|| fail "refusal message does not match the documented --no-build wording"
}
test_require_no_build_jar_accepts_present_jar() {
local saved_jar="$JAR" dir
dir="$TMP/no-build-present"; mkdir -p "$dir"
JAR="$dir/fleetd.jar"
printf 'existing jar' > "$JAR"
require_no_build_jar || fail "require_no_build_jar rejected an existing jar"
JAR="$saved_jar"
}
# wait_for_daemon_exit is the seam the swap ordering depends on: it must not report success while
# running_pid() still answers, and must report success the moment it clears. `sleep` is shadowed so
# the timeout-loop test does not actually wait out its budget.
test_wait_for_daemon_exit_returns_true_once_pid_clears() {
# running_pid() runs inside a $(...) — a subshell — every time wait_for_daemon_exit calls it, so
# a plain shell variable it increments would reset on each call instead of accumulating. Count in
# a file instead, which is the one thing that actually survives across those subshells.
local counter_file="$TMP/wait-exit-calls" final_calls
printf '0' > "$counter_file"
running_pid() {
local n
n="$(cat "$counter_file")"
n=$((n + 1))
printf '%s' "$n" > "$counter_file"
if [ "$n" -lt 3 ]; then printf '4242'; else printf ''; fi
}
sleep() { :; }
wait_for_daemon_exit 10 || fail "wait_for_daemon_exit did not report success once the pid cleared"
final_calls="$(cat "$counter_file")"
[ "$final_calls" -ge 3 ] || fail "wait_for_daemon_exit returned before actually re-checking running_pid"
source "$ROOT/scripts/redeploy-fleetd.sh" # restore the real running_pid/sleep for later tests
}
test_wait_for_daemon_exit_times_out_if_pid_never_clears() {
local rc=0
running_pid() { printf '4242'; }
sleep() { :; }
wait_for_daemon_exit 3 || rc=$?
[ "$rc" -ne 0 ] || fail "wait_for_daemon_exit reported success while the pid never cleared"
source "$ROOT/scripts/redeploy-fleetd.sh" # restore the real running_pid/sleep for later tests
}
# fleetd #521 — the swap step's guard, at two levels.
#
# The first two tests call the predicate should_swap() directly. They pin its logic, and that is all
# they pin. On their own they did NOT close #521, and this was measured rather than argued: with the
# main flow reading `if should_swap "$DO_BUILD"; then`, changing that line to `if false; then` left
# this whole suite at exit 0 with zero FAIL lines, because nothing here made the code that performs
# the swap consult the predicate at all. Extracting the decision had moved the untested decision up
# a level, not removed it.
#
# So the last two tests call swap_if_built() — the function the main flow actually calls, holding the
# guard and the swap together — with a recording stub in place of the real `mv`. Those fail if the
# guard is removed, inverted, or stops being consulted.
#
# What none of these four can catch: deleting the `swap_if_built "$DO_BUILD"` line from the main flow
# altogether. That is test_swap_ordered_after_wait_and_before_start's job below, because sourcing
# stops before the main flow runs, so no test in this file can invoke it.
test_should_swap_true_when_build_ran() {
should_swap 1 || fail "should_swap 1 (a build ran and staged a jar) must return true"
}
test_should_swap_false_when_build_skipped() {
if should_swap 0; then
fail "should_swap 0 (--no-build; nothing was staged this run) must return false"
fi
}
# Both of these re-source redeploy-fleetd.sh at the START, because a bash function definition is
# global for the rest of the process and an earlier test may have left swap_staged_jar or jar_id
# overridden (see the longer note on this at test_detect_supervisor_systemd_probe_error_is_unclear),
# and again at the END, so their own stubs do not leak into every test that runs after them.
test_swap_if_built_performs_the_swap_when_build_ran() {
source "$ROOT/scripts/redeploy-fleetd.sh"
local marker="$TMP/swap-if-built-ran"
rm -f "$marker"
swap_staged_jar() { printf '%s -> %s\n' "$1" "$2" > "$marker"; }
jar_id() { printf 'stubbed\n'; }
swap_if_built 1 > /dev/null
[ -f "$marker" ] \
|| fail "swap_if_built 1 (a build ran and staged a jar) must perform the swap, and did not"
source "$ROOT/scripts/redeploy-fleetd.sh"
}
test_swap_if_built_skips_the_swap_when_build_skipped() {
source "$ROOT/scripts/redeploy-fleetd.sh"
local marker="$TMP/swap-if-built-skipped"
rm -f "$marker"
swap_staged_jar() { printf 'swapped\n' > "$marker"; }
jar_id() { printf 'stubbed\n'; }
swap_if_built 0 > /dev/null
if [ -f "$marker" ]; then
fail "swap_if_built 0 (--no-build; nothing was staged this run) must not swap, but it did"
fi
source "$ROOT/scripts/redeploy-fleetd.sh"
}
# fleetd #493 item 2: "put the swap after that wait, before the start." Sourcing stops before the
# main flow ever runs (see the SOURCED guard in redeploy-fleetd.sh), so the ordering guarantee
# itself — as opposed to the pure functions it's built from — can only be checked by reading the
# script's own call sites, the same way test_recovery_patterns_match_source below checks Java
# source shape instead of behavior it cannot invoke directly.
#
# Two details about the three greps below, both of which have already gone wrong here.
#
# The needle for the swap is the MAIN FLOW's call site, `swap_if_built "$DO_BUILD"` — not
# `swap_staged_jar "$JAR_STAGED" "$JAR"`. Since fleetd #521 that second string lives inside
# swap_if_built's body, which is defined near the top of the script, far ABOVE the stop step. Using
# it made this test report "swap_staged_jar (line 215) is not after wait_for_daemon_exit (line 730)"
# — a true statement about a function definition, and nothing at all about the order of the steps.
#
# Each grep ends in `|| true`. This file runs under `set -euo pipefail`, and `pipefail` makes the
# pipeline's status grep's status, so a needle that is simply ABSENT failed the assignment and `set
# -e` killed the whole suite on the spot — before reaching the `[ -n ... ] || fail` line written to
# report exactly that. Measured: the suite exited 1 having printed zero bytes, no FAIL line and no
# name of the missing call site. `|| true` lets the assignment succeed empty so the guard can speak.
test_swap_ordered_after_wait_and_before_start() {
local src="$ROOT/scripts/redeploy-fleetd.sh" wait_line swap_line start_line
wait_line="$(grep -Fn 'wait_for_daemon_exit "$STOP_WAIT"' "$src" | head -1 | cut -d: -f1 || true)"
swap_line="$(grep -Fn 'swap_if_built "$DO_BUILD"' "$src" | head -1 | cut -d: -f1 || true)"
start_line="$(grep -Fn 'say "start"' "$src" | head -1 | cut -d: -f1 || true)"
[ -n "$wait_line" ] || fail "could not find the wait-for-exit call site in redeploy-fleetd.sh"
[ -n "$swap_line" ] || fail "could not find the swap call site in redeploy-fleetd.sh"
[ -n "$start_line" ] || fail "could not find the start section in redeploy-fleetd.sh"
[ "$swap_line" -gt "$wait_line" ] \
|| fail "swap_if_built (line $swap_line) is not after wait_for_daemon_exit (line $wait_line)"
[ "$swap_line" -lt "$start_line" ] \
|| fail "swap_if_built (line $swap_line) is not before the start section (line $start_line)"
}
# fleetd #511: the drain-gate abort message (fired when a build has staged a jar but the operator
# declines the drain confirmation) used to tell the operator to "Rerun (with or without --no-build)"
# to finish the restart. That is wrong — by the time this message can fire, stage_built_jar has
# already moved the jar off $JAR, so a rerun WITH --no-build hits require_no_build_jar's own refusal
# ("no jar at $JAR — run without --no-build"). Like test_swap_ordered_after_wait_and_before_start
# above, this code path is never reached by sourcing (the SOURCED guard stops before the main flow),
# so the only way to pin its exact wording is to read the source.
test_drain_gate_abort_message_says_no_no_build() {
local src="$ROOT/scripts/redeploy-fleetd.sh" msg
msg="$(grep -A3 -F 'aborted — the running daemon was NOT touched, but the freshly built jar is sitting at' "$src")"
[ -n "$msg" ] || fail "could not find the drain-gate staged-jar abort message in redeploy-fleetd.sh"
if printf '%s' "$msg" | grep -qF 'with or without --no-build'; then
fail "abort message still claims a rerun WITH --no-build can finish the restart"
fi
printf '%s' "$msg" | grep -qF 'WITHOUT --no-build' \
|| fail "abort message does not tell the operator to rerun without --no-build"
printf '%s' "$msg" | grep -qF 'no longer at the live path' \
|| fail "abort message does not say why --no-build cannot finish the restart"
}
# fleetd #517 — the drain-gate abort branch itself. Before this, the only test of this message was
# a source-text grep (test_drain_gate_abort_message_says_no_no_build, below): it greps this script's
# own file for the wording, which stays in the file even if the `if` guarding it is mutated to
# `if false` and the branch can never run. These four tests call drain_gate_refusal directly instead,
# so they fail if the branch is unreachable OR if its wording regresses — the grep test is KEPT
# alongside these, not replaced, because it catches a different regression (a re-wording that still
# reaches the right branch would not change which case fires here, but would still be worth pinning).
test_drain_gate_refusal_build_ran_staged_present() {
local dir staged result
dir="$TMP/drain-refusal-build-staged"; mkdir -p "$dir"
staged="$dir/fleetd-new.jar"
printf 'staged jar bytes' > "$staged"
result="$(drain_gate_refusal 1 "$staged")"
printf '%s' "$result" | grep -qF "$staged" \
|| fail "build-ran+staged-present refusal does not name the staged jar path"
printf '%s' "$result" | grep -qF 'Rerun WITHOUT --no-build' \
|| fail "build-ran+staged-present refusal does not tell the operator how to finish the restart"
if printf '%s' "$result" | grep -qF 'nothing changed'; then
fail "build-ran+staged-present refusal must not claim nothing changed — the jar already moved"
fi
}
test_drain_gate_refusal_build_ran_staged_absent() {
local dir result
dir="$TMP/drain-refusal-build-no-staged"; mkdir -p "$dir"
result="$(drain_gate_refusal 1 "$dir/fleetd-new.jar")"
assert_equals "aborted — nothing changed" "$result" "build-ran+staged-absent refusal wording"
}
# --no-build itself never builds or stages anything (require_no_build_jar, above), so a staged jar
# found here is a leftover from an earlier, unrelated run — THIS run truly changed nothing. See the
# comment above drain_gate_refusal in redeploy-fleetd.sh for the full reasoning.
test_drain_gate_refusal_no_build_staged_present() {
local dir staged result
dir="$TMP/drain-refusal-no-build-staged"; mkdir -p "$dir"
staged="$dir/fleetd-new.jar"
printf 'leftover staged jar bytes' > "$staged"
result="$(drain_gate_refusal 0 "$staged")"
assert_equals "aborted — nothing changed" "$result" "no-build+staged-present refusal must deliberately say nothing changed"
}
test_drain_gate_refusal_no_build_staged_absent() {
local dir result
dir="$TMP/drain-refusal-no-build-no-staged"; mkdir -p "$dir"
result="$(drain_gate_refusal 0 "$dir/fleetd-new.jar")"
assert_equals "aborted — nothing changed" "$result" "no-build+staged-absent refusal wording"
}
test_no_errors() {
cat > "$TMP/no-errors.log" <<'LOG'
2026-09-05 12:00:00 INFO fleetd listening
@@ -224,6 +720,39 @@ test_unattributable_quiet_mutation_is_caught() {
printf 'Unattributable mutation: FAIL: cross-unattributable recovered: expected 0, got 2\n'
}
test_detect_supervisor_launchd_only
test_detect_supervisor_systemd_only
test_detect_supervisor_none
test_detect_supervisor_systemd_installed_not_loaded_is_unclear
test_detect_supervisor_launchd_installed_not_loaded_is_unclear
test_detect_supervisor_systemd_probe_error_is_unclear
test_require_drivable_supervisor_refuses_ambiguous
test_require_drivable_supervisor_refuses_unclear
test_require_drivable_supervisor_accepts_known_kinds
test_count_daemon_pids
test_assert_single_daemon_accepts_one_pid
test_assert_single_daemon_rejects_two_pids
test_jar_id_defaults_to_live_and_reports_explicit_path
test_jar_id_reports_absent_for_missing_file
test_stage_built_jar_moves_off_live_path
test_stage_built_jar_dies_when_build_produced_nothing
test_swap_staged_jar_moves_staged_onto_live
test_swap_staged_jar_dies_without_staged_file
test_swap_staged_jar_dies_when_mv_fails
test_should_swap_true_when_build_ran
test_should_swap_false_when_build_skipped
test_swap_if_built_performs_the_swap_when_build_ran
test_swap_if_built_skips_the_swap_when_build_skipped
test_require_no_build_jar_dies_when_absent
test_require_no_build_jar_accepts_present_jar
test_wait_for_daemon_exit_returns_true_once_pid_clears
test_wait_for_daemon_exit_times_out_if_pid_never_clears
test_swap_ordered_after_wait_and_before_start
test_drain_gate_abort_message_says_no_no_build
test_drain_gate_refusal_build_ran_staged_present
test_drain_gate_refusal_build_ran_staged_absent
test_drain_gate_refusal_no_build_staged_present
test_drain_gate_refusal_no_build_staged_absent
test_no_errors
test_recovery_patterns_match_source
test_attributed_recovered_connection_error