Compare commits

..

17 Commits

Author SHA1 Message Date
Dai Ha b8182c96c2 fleetd #550: pin hash256's algorithm against a literal SHA-256 test vector
CI / shell-tests (pull_request) Successful in 5s
CI / contract (pull_request) Successful in 59s
CI / build (pull_request) Successful in 2m27s
test_jar_id_defaults_to_live_and_reports_explicit_path's reference hash is computed by calling
hash256 itself (needed so it doesn't call the Linux-crashing bare shasum directly). That made
subject and reference the same instrument: they agree no matter which algorithm hash256 actually
runs, so a mutation swapping both of hash256's arms for the wrong algorithm was invisible to the
suite.

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

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

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

New tests: test_jar_id_reports_unhashable_when_no_hasher_on_path (stubbed PATH with neither
hasher) and test_no_unguarded_macos_only_hasher_calls (a shape check across every script under
scripts/, not named lines — #545 already showed this idiom spreading from two sites to six).
2026-09-12 15:06:15 +07:00
ltms 93a9ed3f83 Merge #549: widen Injector's delivery catch to Throwable (#546)
CI / contract (push) Successful in 56s
CI / build (push) Successful in 1m43s
Closes the re-delivery window that merging #543 opened. I caused that; this closes it the same day.

Verified by me on the branch at 87871ea, base 0b032f5 (not stale).

Production diff is 11 lines: `catch (RuntimeException)` -> `catch (Throwable)` at Injector.java:391,
and the `sendError` local widened to `Throwable` so it compiles. I checked every use of `sendError`
myself — :533 `getMessage()` and :534 `completeExceptionally(Throwable)` — so the wider type
reaches nothing that needed the narrow one.

Build on the branch: exit 0, Tests run: 1719, Failures: 0, Errors: 0, Skipped: 0 (1716 on main plus
the 3 new tests). #459's javadoc reference gate: exit 0, 0 reference errors. Gitea CI run 1803 on
87871ea: success.

Three mutations of my own, none of them the ones the worker used:
1. Reverted the catch to `RuntimeException`. RED: `anErrorFromSendDoesNotRedeliverOnASecondRound`
   "expected: <1> but was: <2>" prompt calls, and `anErrorFromSendRemovesTheMessage...` reporting
   the Error escaping `onStatus`. That second message is the defect itself, stated by the test.
2. Deleted the `t.queue.poll()` in the catch arm. RED on the new test AND on the pre-existing
   `sendFailureDropsMessageAndFailsItsFuture` — so the new test is not carrying that behaviour alone.
3. Wrote `DELIVERED` instead of `NOT_DELIVERED` at :400, the catch arm only. RED on the new test and
   on `aHerdrExceptionFromSendStillProducesNotDeliveredUnchanged`, which is the control that proves
   the widening did not quietly change the ordinary path.

All three restored; sha256 back to 97c560b6e33fc49a1772abec92e5bbab613f8991deba2220d30221a2f546ba14.
Green control after the restores: InjectorTest 37/37, exit 0.

My third mutation did not apply on its first attempt — it asserted a unique match on
`p.state = Pending.State.NOT_DELIVERED;`, which occurs twice (:400 and :419), so the script wrote
nothing and the test run came back exit 0. That is not a surviving mutant, it is a non-result
wearing the same clothes. The pristine-anchor count catching it is the only reason I noticed.

Deliberately NOT fixed here, filed as #551: the catch arm assumes that reaching it means nothing was
sent, and nothing establishes that. `agent.prompt` pastes and submits in one call, and every failure
in the response half of `UnixSocketHerdrClient.call()` — dropped connection, malformed line, error
result — is a `HerdrException`, which is a `RuntimeException`, which this catch arm already caught
before today. So "records NOT_DELIVERED for a delivery that happened" is older than this PR and is
not created by it. The fleet01 lead argued it was a trap inside this change and asked to be argued
out of it before the merge; the measurement above is the argument, and their underlying diagnosis is
right and is now #551 with their wording on it.
2026-09-12 09:39:41 +02:00
ltms 0b032f5a1a Merge #548: fix mktemp -t templates for GNU coreutils, split the unclear-supervisor detail (#545)
CI / contract (push) Successful in 55s
CI / build (push) Successful in 2m16s
Verified by me on the branch, not on the worker's report.

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

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

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

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

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

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

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

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

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

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

The constraint came from the fleet01 lead. Their point: the value of the `drain complete` line is
that it is MISSING when a drain does not finish. A `finally` block would print it after a drain
that threw, with partial counts, and destroy both halves at once. The code already avoids this;
what was missing was the sentence that stops a reviewer putting it back.
2026-09-12 09:29:46 +02:00
Dai Ha 87871eaefb fleetd #546: widen Injector's delivery catch to Throwable, stop re-delivery on Error
CI / contract (pull_request) Successful in 1m29s
CI / build (pull_request) Successful in 1m49s
Injector.java:391 caught only RuntimeException around the herdr send seam. PR #543
(fleetd #538) widened StatusPoller's per-target catch to Throwable so the polling
loop now survives an Error there, which means it comes back round — and Injector's
narrower catch let the poisoned message stay QUEUED (the loop peeks, not polls),
so the next round re-sent the same text into the member's pane.

Widen the catch to Throwable, matching #543 one layer down. sendError's declared
type widens from RuntimeException to Throwable to keep compiling; its only consumer
(CompletableFuture.completeExceptionally(Throwable)) already accepts that type, so
no other caller-visible behavior changes. The ordinary HerdrException/RuntimeException
path is unchanged.

Adds three tests: an Error at the send seam is dropped and marked NOT_DELIVERED, a
second onStatus round does not re-send it, and a HerdrException control proves the
ordinary path is untouched.
2026-09-12 14:26:27 +07:00
Dai Ha a476a14f1c fleetd #545: fix mktemp -t templates for GNU coreutils, split unclear-supervisor detail
CI / contract (pull_request) Successful in 1m16s
CI / build (pull_request) Successful in 2m28s
Every mktemp -t template in redeploy-fleetd.sh lacked an X placeholder. BSD mktemp
(macOS) tolerates that and appends its own suffix; GNU mktemp (every Linux
distribution) refuses it and exits non-zero. All six sites now use .XXXXXX.

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

Tests added to scripts/test-redeploy-fleetd.sh:
- test_mktemp_dash_t_templates_have_x_placeholders: source-text check, fails if
  any mktemp -t template lacks an X.
- test_detect_supervisor_systemd_probe_setup_failure_is_unclear: proves the
  SET-UP-FAILED detail when mktemp itself fails (systemctl never runs).
- test_detect_supervisor_systemd_probe_error_is_unclear: extended with assertions
  that the PROBE-ANSWERED-WITH-STDERR detail is present and the SET-UP-FAILED
  wording is absent, so swapping the two messages fails a test in both
  directions.
2026-09-12 14:22:57 +07:00
Dai Ha 5eb4267a4a fleetd #512 follow-up: record why the drain-complete line must not move into a finally
CI / contract (pull_request) Successful in 1m3s
CI / build (pull_request) Successful in 2m8s
The javadoc said the log.info fires "every time", which reads as an invitation
to the exact edit that destroys it. The absence of the line is the signal that
the drain died, so a finally would remove the signal and print partial counts in
the same change.

Raised by the fleet01 lead from their 2026-09-10 incident: that drain is known to
have died only because it threw and left a stack trace. A drain that hung, or
returned early on a condition, leaves no trace, no ERROR token and no priority --
only a missing line.

Comment only. No behaviour change.
2026-09-12 14:21:15 +07:00
ltms 7611b69667 Merge pull request 'fleetd #504 item 1: stop the false ok on the loaded-but-not-running stop path' (#541) from worker/504-failed-reported-clean-3cfd66-3 into main
CI / build (push) Successful in 1m35s
CI / contract (push) Successful in 1m42s
2026-09-12 09:09:00 +02:00
ltms cc302fe4af Merge pull request 'fleetd #538: recover polling loops after errors' (#543) from worker/538-loop-dies-on-error-4a5eeb-6 into main
CI / contract (push) Successful in 51s
CI / build (push) Successful in 2m10s
2026-09-12 09:02:48 +02:00
ltms 4a8a780274 Merge pull request 'fleetd #426: pin FleetHealthMonitor.coverage and its HealthCoverageSource call site' (#542) from worker/426-health-coverage-ef1fd4-4 into main
CI / contract (push) Successful in 1m16s
CI / build (push) Successful in 2m28s
2026-09-12 08:55:46 +02:00
Dai Ha 343ce0f4c0 fleetd #538: recover polling loops after errors
CI / contract (pull_request) Successful in 44s
CI / build (pull_request) Successful in 2m0s
2026-09-12 13:47:44 +07:00
ltms cec3e191d4 Merge pull request 'fleetd #459: lint Javadoc references in CI' (#539) from worker/459-broken-link-targets-cadc17-5 into main
CI / contract (push) Successful in 1m4s
CI / build (push) Successful in 2m9s
2026-09-12 08:46:23 +02:00
Dai Ha 1850a5f324 fleetd #426: pin FleetHealthMonitor.coverage and its HealthCoverageSource call site
CI / contract (pull_request) Successful in 48s
CI / build (pull_request) Successful in 2m16s
FleetHealthMonitor.coverage had zero references in the test tree — not the
method, not either output string, not the field it populates. Inverting
`enabled`, swapping "full"/"detection-only", or breaking the argument pairing
at the HealthCoverageSource call site in Fleetd.java all shipped a green
build.

Extract the HealthCoverageSource lambda out of Fleetd.main into a
package-private static factory (healthCoverageSource(ConfigRef)), the same
shape capacitySource/quarantineSource already use for the identical
argument-pairing risk (fleetd #415). #407's "keep the config invalid, assert
on the log line before validateAll() throws" option does not apply here: this
call site is built well after validateAll() and after a real herdr socket
connect, so driving it through a real Fleetd.main would require the socket
I/O this ticket's tests must not do.

Add FleetHealthMonitorCoverageTest (the three-branch method itself) and
FleetdHealthCoverageSourceWiringTest (the call site, via a real
FleetConfig.load + ConfigRef against @TempDir fixtures, including a hot
notifications-reload case). Output strings are unchanged — "detection-only"
is still what a live fleet_list reports today.

Measured: all three mutations killed by the new tests.
2026-09-12 13:43:50 +07:00
ltms 57cd96f5e6 Merge pull request 'fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts' (#540) from worker/537-capturedlog-close-e4c437-2 into main
CI / contract (push) Successful in 47s
CI / build (push) Successful in 2m9s
2026-09-12 08:42:19 +02:00
Dai Ha 202e37e3b3 fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts
CI / contract (pull_request) Successful in 1m2s
CI / build (pull_request) Successful in 1m36s
Only the level-restore half of close() was pinned before this
(WorktreeSessionManagerTest). Deleting logger.detachAppender(appender)
from close() left mvn clean install green (1701 tests, 0 failures) --
the appender-detach half of the contract was unmeasured.

Adds CapturedLogTest with three tests, each using a logger name no
production class uses:
- closeDetachesTheAppenderSoALaterLogIsNotCaptured: an event logged
  after close() must not land in events().
- closeRestoresTheLevelCapturedAtOpen: the helper's headline contract
  in one place, independent of any production class.
- setLevelDuringCaptureDoesNotChangeWhatCloseRestores: setLevel()'s
  own javadoc claim that close() always restores the level captured
  at construction, never a value set through setLevel() mid-capture.

Test-only change; CapturedLog.java itself is untouched.
2026-09-12 13:36:55 +07:00
Dai Ha 90253f832d fleetd #459: lint Javadoc references in CI
CI / contract (pull_request) Successful in 1m30s
CI / build (pull_request) Successful in 2m9s
2026-09-12 13:36:37 +07:00
19 changed files with 965 additions and 68 deletions
+21
View File
@@ -39,6 +39,10 @@ jobs:
# that runs does so against the fake UDS herdr and fake ccs/claude stubs.
run: mvn -B clean install
- name: javadoc reference lint
working-directory: fleetd
run: mvn -B -DskipTests javadoc:javadoc -Ddoclint=reference
# Deliberately NOT actions/upload-artifact: this Gitea instance presents as GHES, and
# @actions/artifact v2+ (i.e. upload-artifact@v4) refuses to run there —
# "GHESNotSupportedError ... not currently supported on GHES", which red-Xes an otherwise
@@ -54,6 +58,23 @@ jobs:
done
exit 0
# fleetd #550 — nothing ran scripts/test-redeploy-fleetd.sh in CI before this, on any platform,
# so it had run only on macOS by hand and two Linux-only bugs (this issue's items 1 and 2)
# survived undetected: shasum is a macOS-only tool (it ships with Perl; GNU coreutils, i.e. every
# mainstream Linux distro including this runner's ubuntu-latest, does not have it and ships
# sha256sum instead). The gate here is the step's own exit code, nothing else: a `run:` step in
# Gitea/GitHub Actions already fails the job on a non-zero exit with no extra scripting needed,
# so this deliberately does NOT grep the output for a `FAIL:` count. That is the #550 item-2
# lesson one level up — a suite that dies before it runs a single test prints zero FAIL lines,
# which is exactly what a clean pass also prints, so counting FAIL lines can never be the gate.
shell-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: redeploy-fleetd.sh shell suite
run: bash scripts/test-redeploy-fleetd.sh
# CB-521 — actually run the AMQP contract test in CI, against a REAL broker. The broker is a
# RabbitMQ SERVICE CONTAINER, not Testcontainers-with-Docker: the runner image has no Docker, so
# AmqpReplyInboxContractTest reads AMQP_URI (set below to the service's network alias) and binds
@@ -696,11 +696,7 @@ public final class Fleetd {
FleetMcp mcp = new FleetMcp(messages, workers, sessions, identity, presence,
primaryRegistry, callers, FleetMcp.AuthorizationMode.ENFORCED, metrics,
capacitySource(config, cfg, profile -> liveCountRef.get().apply(profile)),
new FleetMcp.HealthCoverageSource(() -> {
var health = config.get().health();
return FleetHealthMonitor.coverage(health != null && health.isEnabled(),
health != null && health.notifications() != null && health.notifications().configured());
}),
healthCoverageSource(config),
quarantineSource,
leadMailbox,
outageSource,
@@ -1038,6 +1034,38 @@ public final class Fleetd {
cfg.profiles()::keySet, System::nanoTime);
}
/**
* fleetd #426: package-private factory for {@code fleet_list}'s {@code healthCoverage} source,
* extracted out of {@code main} for the same reason {@link #capacitySource} and {@link
* #quarantineSource} were — and the same reason {@link #exhaustedPatternCoverageLine}/{@link
* #errorPatternCoverageLine} exist: {@link FleetHealthMonitor#coverage}'s three-branch method
* is easy to pin directly (a plain {@code (boolean, boolean) -> String} call), but that proves
* nothing about whether <em>this call site</em> pairs the right boolean with the right meaning.
* fleetd #415's measured lesson is the reason this matters here — swapping the two arguments at
* a call site like this one compiled clean and left the full suite green, because every existing
* test exercised the method in both directions without ever exercising the pairing.
*
* <p>{@code #407}'s "keep the config invalid, assert on the log line before the throw" option
* does not apply to this call site: the five reporters #407 covers all run in {@code main}
* <em>before</em> {@code cfg.validateAll()} (line ~171), so an invalid config still exercises
* them. This call site is built during {@code FleetMcp} construction, which runs only after
* {@code UnixSocketHerdrClient.connect} has already opened a real herdr socket (line ~188) —
* reaching it at all means main already performed real I/O, which the no-socket constraint on
* this ticket rules out. So the pairing is pinned by extracting it to this directly-callable
* factory instead, the same shape {@link #capacitySource}/{@link #quarantineSource} already use.
*
* <p>Reads {@code config.get().health()} live (health.notifications is a {@code SPLIT_KEYS}
* entry — see {@link ConfigRef#SPLIT_KEYS}), so a hot-reloaded notifications block changes what
* {@code fleet_list} reports without a restart, exactly like {@link #capacitySource}'s maxLoad.
*/
static FleetMcp.HealthCoverageSource healthCoverageSource(ConfigRef config) {
return new FleetMcp.HealthCoverageSource(() -> {
var health = config.get().health();
return FleetHealthMonitor.coverage(health != null && health.isEnabled(),
health != null && health.notifications() != null && health.notifications().configured());
});
}
/**
* fleetd #248: package-private factory for the member worktree/branch lookup {@link
* CompletionResolver} uses to name a fallback report's worktree and branch (fleetd#241).
@@ -208,7 +208,7 @@ import java.util.function.Supplier;
* that is correctly hot and for a key nobody triaged. Three times now — {@code worktreeGroup} (#323),
* {@code primary}/{@code configReload} (#326), and {@code fleet.leaders} sitting in the escape hatch
* (#333) — the second kind hid among the first. A top-level coverage checker in the
* {@link ConfigRefProfileCoverageTest} shape (one level up, over {@code FleetConfig} itself rather
* {@code ConfigRefProfileCoverageTest} shape (one level up, over {@code FleetConfig} itself rather
* than {@code FleetConfig.Profile}) proves this file's four classes exhaust the record's components
* — see {@code ConfigRefTopLevelCoverageTest}. That test proves the record's <em>shape</em> is fully
* triaged; it does NOT prove a {@code SPLIT_KEYS}/{@code COLD_KEYS}/{@code DEFERRED_KEYS} member has
@@ -1670,7 +1670,7 @@ public record FleetConfig(
* <p>A herdr pane runs a login shell that re-sources the operator's own secret store, so a
* member inherits every credential the operator's shell holds — measured at 31 names on this
* host, of which only one ({@code GITEA_ACCESS_TOKEN}) used to be blocked, and that block was a
* single name hardcoded in {@link HerdrPeerLauncher} rather than driven by config (gitea issue
* single name hardcoded in {@link dev.ltms.fleet.member.HerdrPeerLauncher} rather than driven by config (gitea issue
* #82). This record replaces that hardcoded shadow with a config-driven one.
*
* <p><b>deny-by-default, not a deny-list.</b> A deny-list (block these specific names, let
@@ -306,7 +306,7 @@ public final class Injector {
if (t == null) return;
Pending sent = null;
RuntimeException sendError = null;
Throwable sendError = null;
boolean turnCompleted = false;
boolean turnFailed = false;
boolean resubmit = false;
@@ -388,9 +388,14 @@ public final class Injector {
t.turnObserved = false;
t.injectableSincePickup = 0;
sent = p;
} catch (RuntimeException e) {
} catch (Throwable e) {
// Delivery failed at herdr; drop the poisoned message and surface it
// rather than blocking the queue behind it.
// rather than blocking the queue behind it. Catches Throwable, not just
// RuntimeException: fleetd #546 — an Error escaping this send (e.g. a
// NoClassDefFoundError, see #413) would otherwise leave the entry QUEUED
// at the head of t.queue. Line :378 peeks rather than polls, so the next
// onStatus round would re-enter this try and send the same text again,
// typing the same brief into the member's pane a second time.
t.queue.poll();
p.state = Pending.State.NOT_DELIVERED;
sent = p;
@@ -64,34 +64,39 @@ public final class StatusPoller {
}
private void loop() {
while (running) {
Set<String> active = injector.activeTargets();
for (String target : active) {
if (!running) return;
try {
// herdr's agent_status can misreport a settled worker as `unknown`; refine it
// against the pane content before it drives delivery/completion (CB-115).
// CB-185: refine THROUGH the same control the raw status came from — a router
// splits lead/member targets across two herdr daemons, and reading a lead's pane
// through the (fixed) member refiner never finds it, wedging that lead at UNKNOWN.
AgentControl control = router != null ? router.agentsFor(target) : agents;
AgentStatus status = refiner.refine(target, control.status(target), control);
injector.onStatus(target, status);
} catch (HerdrException e) {
// The worker's agent is gone — stop trying and unblock its waiters.
if (e.code() != null && e.code().endsWith("_not_found")) {
log.debug("target {} gone; dropping its queue", target);
injector.drop(target, e);
} else {
log.debug("status poll for {} failed (will retry): {}", target, e.getMessage());
try {
while (running) {
Set<String> active = injector.activeTargets();
for (String target : active) {
if (!running) return;
try {
// herdr's agent_status can misreport a settled worker as `unknown`; refine it
// against the pane content before it drives delivery/completion (CB-115).
// CB-185: refine THROUGH the same control the raw status came from — a router
// splits lead/member targets across two herdr daemons, and reading a lead's pane
// through the (fixed) member refiner never finds it, wedging that lead at UNKNOWN.
AgentControl control = router != null ? router.agentsFor(target) : agents;
AgentStatus status = refiner.refine(target, control.status(target), control);
injector.onStatus(target, status);
} catch (HerdrException e) {
// The worker's agent is gone — stop trying and unblock its waiters.
if (e.code() != null && e.code().endsWith("_not_found")) {
log.debug("target {} gone; dropping its queue", target);
injector.drop(target, e);
} else {
log.debug("status poll for {} failed (will retry): {}", target, e.getMessage());
}
} catch (Throwable e) {
log.error("unexpected failure polling {}; skipping this round", target, e);
}
} catch (RuntimeException e) {
// Never let one target's unexpected error (e.g. an odd agent.get shape) kill
// the single poller thread and stall injection for every worker.
log.warn("unexpected error polling {}; skipping this round", target, e);
}
sleep();
}
sleep();
} finally {
if (running) {
log.error("status poller loop exited unexpectedly; it can be restarted");
}
running = false;
}
}
@@ -246,7 +246,7 @@ public final class AmqpReplyInbox implements ReplyInbox, AutoCloseable {
* neither. So before this method existed with a requeue step, it dropped {@link #held}'s entries
* for {@code target} while the broker still considered them outstanding: never acked, never
* nacked, never requeued, and no longer reachable by {@link #peek} — permanently invisible. This
* is unlike {@link #handleRecovery} and {@link #close()}, whose bare {@code held.clear()} is
* is unlike {@link RecoveryListener#handleRecovery(Recoverable)} and {@link #close()}, whose bare {@code held.clear()} is
* correct because each has already made the broker requeue (a real connection drop, or
* {@code channel.close()} respectively) before clearing local state.
*
@@ -713,7 +713,7 @@ public final class MessageService {
* failure.
*
* <p><strong>Without {@code sweepAsking} on the release path, a target torn down while
* genuinely {@code ASKING} was unrecoverable.</strong> {@link #resolveQuestion} had already
* genuinely {@code ASKING} was unrecoverable.</strong> {@link Rendezvous#resolveQuestion(String, String, String)} had already
* closed the forward waiter the instant the question surfaced (so the {@code waiter} branch
* below finds nothing to fail), the {@code question == null} guard excluded the task from
* {@code matching} (so the loop below skipped it too), and the worker's own {@code fleet_ask}
@@ -44,7 +44,7 @@ import java.util.stream.Collectors;
* still holds an unacked message ({@link #pendingReplies}), tickets not yet collected
* ({@link #pendingTickets}), and open questions not yet answered or lapsed
* ({@link #pendingQuestions}) — and sends at most one combined nudge per tick
* ({@link #injectNudge(String, int, int, int)}). Work that arrives while the lead is busy is
* ({@link #injectNudge(String, int, int, int, int, int)}). Work that arrives while the lead is busy is
* never lost: it is re-read fresh on every tick until the lead is injectable or its own reminder
* cap ({@link #maxReminders}) is reached — each source spends from its own budget, so one source
* exhausting its cap does not stop nudges about the others (post-CB-590 regression fix; see
@@ -1074,6 +1074,29 @@ public final class SessionManager implements TurnListener {
* 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.
*
* <p><strong>Non-goal: this line must never move into a {@code finally} block, and this method
* must never grow one around it.</strong> "Every time" above means every time the drain
* <em>finishes</em>, not every time this method exits. The absence of the line is the signal
* that the drain died, so a {@code finally} would destroy the signal and print confident
* partial counts in the same edit — the line would appear after a drain that threw, carrying
* whatever {@code tally} it had reached. Both halves of the value are lost at once. The line
* has to be the last statement of the successful path and reachable only from it.
*
* <p>This is written down because it is the obvious review comment ("shouldn't we always log
* the drain result?"), it sounds like thoroughness, and the paragraph above reads as an
* invitation to it. Raised by the fleet01 lead on 2026-09-12, from their 2026-09-10 incident:
* we only know that drain died on that host because it <em>threw</em>, and a
* {@code NoClassDefFoundError} reached the JVM's uncaught handler. A drain that hung on one
* session, or returned early on a condition rather than an exception, would leave no stack
* trace, no {@code ERROR} token and no priority — only a missing line. That makes the loud
* variant the one we have seen and the quiet variants the ones this line exists to catch.
*
* <p>Related: {@code released} and {@code abandoned} are counted incrementally inside {@link
* #drainSnapshot}'s loop and folded with {@link DrainTally#plus}, rather than derived from a
* collection read at the end, for the same reason. If a partial report is ever wanted it must
* be a different line with a different verb. One line must not serve both, or a reader cannot
* tell a finished drain from an interrupted one by its wording.
*/
void drainAll(long timeoutNanos) {
long deadline = System.nanoTime() + timeoutNanos;
@@ -60,14 +60,21 @@ public final class SessionReaper {
}
private void loop() {
while (running) {
try {
sessions.reapIdle(idleTtlNanos);
} catch (RuntimeException e) {
log.warn("session reaper iteration failed; continuing", e);
try {
while (running) {
try {
sessions.reapIdle(idleTtlNanos);
} catch (Throwable e) {
log.error("session reaper iteration failed; continuing", e);
}
maybeSweepWipRefs();
sleep();
}
maybeSweepWipRefs();
sleep();
} finally {
if (running) {
log.error("session reaper loop exited unexpectedly; it can be restarted");
}
running = false;
}
}
@@ -90,8 +97,8 @@ public final class SessionReaper {
log.info("refs/wip retention sweep deleted {} snapshot ref(s) older than 24h whose "
+ "content was already reachable from main", deleted);
}
} catch (RuntimeException e) {
log.warn("refs/wip retention sweep failed; continuing", e);
} catch (Throwable e) {
log.error("refs/wip retention sweep failed; continuing", e);
}
// Set even when the sweep threw, so a broken repo is retried on the slow cadence rather
// than hammering git on every 5-second iteration.
@@ -0,0 +1,160 @@
package dev.ltms.fleet;
import dev.ltms.fleet.config.ConfigRef;
import dev.ltms.fleet.config.FleetConfig;
import dev.ltms.fleet.mcp.FleetMcp;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import java.nio.file.Files;
import java.nio.file.Path;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* fleetd #426: {@code FleetHealthMonitor.coverage} had zero references anywhere in the test
* tree — not the method, not either output string, not the field it populates. {@code
* FleetHealthMonitorCoverageTest} (package {@code dev.ltms.fleet.health}) pins the three-branch
* method itself; that is the easy half.
*
* <p>The half that actually matters is this one: {@link Fleetd#healthCoverageSource} is the exact
* call site {@code Fleetd.main} wires into {@code FleetMcp}'s constructor, and it is what feeds
* {@code fleet_list}'s {@code healthCoverage} field (see {@code FleetMcp#listFleet}'s {@code
* result.put("healthCoverage", healthCoverage.value().get())}). Measured precedent on fleetd #423
* (for #415): swapping two arguments at a call site like this one — recreating #415's defect with
* the keys exchanged — compiled with 0 errors and ran the ENTIRE suite (1506 tests) green. A
* thoroughly-tested method proves nothing about whether the call site pairs its arguments correctly;
* only a test that drives the call site itself can catch that.
*
* <p><strong>Why this is not driven through a real {@code Fleetd.main} the way #407 drives its five
* reporters</strong> (keep the config invalid, assert on the log line emitted before {@code
* cfg.validateAll()} throws): all five of #407's reporters run in {@code main} before {@code
* validateAll()} (line ~171). {@link Fleetd#healthCoverageSource} is built during {@code FleetMcp}
* construction, which happens only after {@code UnixSocketHerdrClient.connect} has already opened a
* real herdr socket (line ~188) and after {@code sessions}/{@code workers} are constructed. Reaching
* this call site by actually running {@code main} would require a real socket connect — banned by
* this ticket's hard constraints — so #407's option 1 does not apply here. Instead {@link
* Fleetd#healthCoverageSource} is extracted to a directly-callable package-private factory, the same
* shape {@link Fleetd#capacitySource} and {@link Fleetd#quarantineSource} already use for the same
* reason (see {@code FleetdCapacitySourceWiringTest}, the direct precedent this test follows).
*
* <p>Uses a real {@link FleetConfig#load} + {@link ConfigRef} (no socket, no port bind, no spawn,
* nothing written outside {@code @TempDir}) so the fixture goes through the actual YAML parser and
* {@code FleetConfig.Health}/{@code Notifications} records, not a hand-built stand-in that could
* silently drift from what the parser actually produces.
*/
class FleetdHealthCoverageSourceWiringTest {
private static final String BASE = """
bind:
host: 127.0.0.1
port: 8765
herdrSocket: ~/.config/herdr/herdr.sock
""";
private static final String HEALTH_DISABLED_WITH_WEBHOOK = BASE + """
health:
enabled: false
notifications:
mode: webhook
""";
private static final String HEALTH_DETECTION_ONLY = BASE + """
health:
enabled: true
""";
private static final String HEALTH_FULL = BASE + """
health:
enabled: true
notifications:
mode: webhook
""";
private static final String HEALTH_ABSENT = BASE;
@Test
@DisplayName("enabled: false reports off, even with a webhook configured")
void disabledHealthReportsOff(@TempDir Path dir) throws Exception {
FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_DISABLED_WITH_WEBHOOK);
assertEquals("off", source.value().get(),
"health.enabled: false must report 'off' regardless of notifications — flipping "
+ "the 'enabled' argument at the HealthCoverageSource call site would report "
+ "'full' here instead");
}
@Test
@DisplayName("enabled with no notifications reports detection-only")
void enabledWithoutNotificationsReportsDetectionOnly(@TempDir Path dir) throws Exception {
FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_DETECTION_ONLY);
assertEquals("detection-only", source.value().get());
}
@Test
@DisplayName("enabled with a webhook configured reports full")
void enabledWithNotificationsReportsFull(@TempDir Path dir) throws Exception {
FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_FULL);
assertEquals("full", source.value().get(),
"health.enabled: true with notifications.mode: webhook must report 'full' — "
+ "swapping 'full' and 'detection-only' at the call site, or breaking the "
+ "enabled/notificationConfigured argument pairing, would report "
+ "'detection-only' here instead");
}
@Test
@DisplayName("an absent health: block reports off")
void absentHealthBlockReportsOff(@TempDir Path dir) throws Exception {
FleetMcp.HealthCoverageSource source = sourceFor(dir, HEALTH_ABSENT);
assertEquals("off", source.value().get());
}
/**
* fleetd #426, the live-wiring half: {@code health.notifications} is a {@code
* ConfigRef.SPLIT_KEYS} entry, and {@link Fleetd#healthCoverageSource} reads {@code
* config.get().health()} live (not the frozen startup {@code cfg}) — exactly like {@link
* Fleetd#capacitySource}'s {@code maxLoad} ({@code FleetdCapacitySourceWiringTest}'s {@code
* reloadedMaxLoadStillChangesWhatFleetListReports}). A hot-reloaded notifications block must
* change what {@code fleet_list} reports without a restart; a fix that froze the whole source
* against the startup snapshot would silently break that and every other test above would stay
* green, since none of them reload.
*/
@Test
@DisplayName("a hot notifications reload still changes what fleet_list reports")
void reloadedNotificationsStillChangeWhatFleetListReports(@TempDir Path dir) throws Exception {
Path file = dir.resolve("fleetd.yaml");
Files.writeString(file, HEALTH_DETECTION_ONLY);
FleetConfig cfg = FleetConfig.load(file);
ConfigRef config = new ConfigRef(file, cfg);
FleetMcp.HealthCoverageSource source = Fleetd.healthCoverageSource(config);
assertEquals("detection-only", source.value().get(),
"sanity: detection-only before any reload");
Files.writeString(file, HEALTH_FULL);
assertTrue(config.reload().applied());
// The live snapshot now carries the webhook — proves the reload really happened and this
// test is not accidentally passing because nothing changed.
assertTrue(config.get().health().notifications() != null
&& config.get().health().notifications().configured(),
"sanity: the reloaded config really carries a configured webhook");
assertEquals("full", source.value().get(),
"the SAME HealthCoverageSource instance must reflect a reloaded notifications "
+ "block without a restart — health.notifications is read live off "
+ "config.get(), exactly like capacitySource's maxLoad");
}
private static FleetMcp.HealthCoverageSource sourceFor(Path dir, String yaml) throws Exception {
Path file = dir.resolve("fleetd.yaml");
Files.writeString(file, yaml);
FleetConfig cfg = FleetConfig.load(file);
ConfigRef config = new ConfigRef(file, cfg);
return Fleetd.healthCoverageSource(config);
}
}
@@ -0,0 +1,40 @@
package dev.ltms.fleet.health;
import org.junit.jupiter.api.Test;
import static org.junit.jupiter.api.Assertions.assertEquals;
/**
* fleetd #426: {@link FleetHealthMonitor#coverage} had zero references anywhere in the test tree —
* not the method, not either output string, not the field it populates. This pins the method's own
* three branches directly.
*
* <p>This is the easy half. It proves the method words each combination correctly, but it proves
* nothing about whether {@code Fleetd.java}'s two call sites pass the right argument in the right
* position — see {@code FleetdHealthCoverageSourceWiringTest} (package {@code dev.ltms.fleet}) for
* the half that actually guards the call site, following the measured fleetd #415 lesson that a
* thoroughly-tested method and an untested argument pairing at its call site are different risks.
*
* <p><strong>The three output strings are load-bearing and must not change here.</strong> {@code
* "detection-only"} is read live off a running daemon's {@code fleet_list} today (measured
* 2026-09-12) — this test intentionally asserts the exact literal strings so a future edit to the
* wording trips it here first.
*/
class FleetHealthMonitorCoverageTest {
@Test
void disabledIsOffRegardlessOfNotificationConfig() {
assertEquals("off", FleetHealthMonitor.coverage(false, false));
assertEquals("off", FleetHealthMonitor.coverage(false, true));
}
@Test
void enabledWithoutNotificationsIsDetectionOnly() {
assertEquals("detection-only", FleetHealthMonitor.coverage(true, false));
}
@Test
void enabledWithNotificationsIsFull() {
assertEquals("full", FleetHealthMonitor.coverage(true, true));
}
}
@@ -2,9 +2,11 @@ package dev.ltms.fleet.inject;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.spi.ILoggingEvent;
import com.fasterxml.jackson.databind.JsonNode;
import dev.ltms.fleet.herdr.AgentControl;
import dev.ltms.fleet.herdr.AgentStatus;
import dev.ltms.fleet.herdr.FakeHerdr;
import dev.ltms.fleet.herdr.HerdrClient;
import dev.ltms.fleet.herdr.HerdrException;
import dev.ltms.fleet.msg.TestTurnTokens;
import dev.ltms.fleet.testing.CapturedLog;
@@ -664,4 +666,111 @@ class InjectorTest {
ExecutionException ex = assertThrows(ExecutionException.class, f::get);
assertInstanceOf(HerdrException.class, ex.getCause());
}
/**
* A {@link HerdrClient} that throws a non-{@link RuntimeException} {@link Error} from {@code
* agent.prompt} instead of delegating — the fleetd #546 case: a stray non-RuntimeException
* throwable (e.g. a {@code NoClassDefFoundError}, fleetd #413) escaping the send seam at
* {@code Injector.java:383}. Records every call it sees itself, including the ones it throws
* for, since the delegate's own recording is never reached for {@code agent.prompt} — so a test
* can assert on exactly what this fake actually received.
*/
private static final class ErrorOnPrompt implements HerdrClient {
private final FakeHerdr delegate;
private final List<FakeHerdr.Call> calls = new java.util.concurrent.CopyOnWriteArrayList<>();
private ErrorOnPrompt(FakeHerdr delegate) {
this.delegate = delegate;
}
List<FakeHerdr.Call> calls() {
return calls;
}
@Override
public JsonNode call(String method, Object params) {
calls.add(new FakeHerdr.Call(method, params));
if (method.equals("agent.prompt")) {
throw new AssertionError("simulated non-RuntimeException send failure (fleetd #546)");
}
return delegate.call(method, params);
}
@Override
public void close() {
delegate.close();
}
}
/**
* Mirrors StatusPoller's own per-target {@code catch (Throwable)} (fleetd #538 / PR #543): the
* production polling loop already swallows whatever escapes one target's round and comes back
* for the next one. A unit test that calls {@code onStatus} directly (bypassing StatusPoller)
* needs the same survival so it can observe what a SECOND round does, regardless of whether
* fleetd #546's fix is present.
*/
private static void pollOnceSurviving(Injector inj, String target, AgentStatus status) {
try {
inj.onStatus(target, status);
} catch (Throwable ignored) {
// matches StatusPoller.loop's own catch (Throwable) added by PR #543
}
}
@Test
void anErrorFromSendRemovesTheMessageAndMarksItNotDelivered() {
// fleetd #546, acceptance test 1: an Error (not a RuntimeException) escaping the send seam
// at Injector.java:383 must still be caught, the message dropped from the queue, and its
// state set to NOT_DELIVERED — never left QUEUED at the head.
ErrorOnPrompt throwing = new ErrorOnPrompt(new FakeHerdr());
Injector inj = new Injector(new AgentControl(throwing));
Injector.Delivery delivery = inj.enqueue(T, "brief", TestTurnTokens.inert(T));
assertDoesNotThrow(() -> inj.onStatus(T, AgentStatus.IDLE),
"fleetd #546: an Error from the send seam must be caught inside onStatus, not "
+ "escape it");
assertTrue(delivery.completion().isCompletedExceptionally(),
"the delivery's future must surface the send failure");
assertEquals(Injector.Cancellation.NOT_DELIVERED, inj.cancel(delivery),
"the message must be dropped and marked NOT_DELIVERED, not left QUEUED at the "
+ "head of the queue");
}
@Test
void anErrorFromSendDoesNotRedeliverOnASecondRound() {
// fleetd #546, acceptance test 2 — the test this ticket exists for. Before the fix, an
// Error at the send seam left the message QUEUED (Injector.java:378 peeks, not polls), so a
// second onStatus round re-entered the same try and sent the same text again: the member's
// pane got the same brief typed into it twice.
ErrorOnPrompt throwing = new ErrorOnPrompt(new FakeHerdr());
Injector inj = new Injector(new AgentControl(throwing));
inj.enqueue(T, "brief", TestTurnTokens.inert(T));
pollOnceSurviving(inj, T, AgentStatus.IDLE);
pollOnceSurviving(inj, T, AgentStatus.IDLE);
long promptCalls = throwing.calls().stream().filter(c -> c.method().equals("agent.prompt")).count();
assertEquals(1, promptCalls, "fleetd #546: the poisoned text must be sent exactly once — a "
+ "second onStatus round must not re-enter send for the same message");
}
@Test
void aHerdrExceptionFromSendStillProducesNotDeliveredUnchanged() {
// fleetd #546, acceptance test 3 (control): HerdrException extends RuntimeException, so it
// was already caught before this ticket's widening. This pins that the ordinary path is
// unchanged — still dropped, still NOT_DELIVERED, still surfaced to the caller.
FakeHerdr failing = new FakeHerdr().agentSendFailsWith("send_failed");
Injector inj = new Injector(new AgentControl(failing));
Injector.Delivery delivery = inj.enqueue(T, "boom", TestTurnTokens.inert(T));
inj.onStatus(T, AgentStatus.IDLE);
assertTrue(delivery.completion().isCompletedExceptionally(),
"a HerdrException at the send seam must still surface to the caller, unchanged by "
+ "fleetd #546's widening");
assertEquals(Injector.Cancellation.NOT_DELIVERED, inj.cancel(delivery),
"a HerdrException must still be dropped and marked NOT_DELIVERED, unchanged by the "
+ "wider Throwable catch");
}
}
@@ -0,0 +1,99 @@
package dev.ltms.fleet.inject;
import com.fasterxml.jackson.databind.JsonNode;
import dev.ltms.fleet.herdr.AgentControl;
import dev.ltms.fleet.herdr.FakeHerdr;
import dev.ltms.fleet.herdr.HerdrClient;
import dev.ltms.fleet.msg.TestTurnTokens;
import org.junit.jupiter.api.Test;
import java.lang.reflect.Field;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
class StatusPollerResilienceTest {
@Test
void anErrorForOneTargetDoesNotStopPollingTheNextTarget() throws Exception {
FakeHerdr fake = new FakeHerdr().withAgent("worker", "term_b", "w2:p8", "w2:t8");
CountDownLatch errorThrown = new CountDownLatch(1);
AgentControl agents = new AgentControl(new ErrorOnceForFirstTarget(fake, errorThrown));
Injector injector = new Injector(agents);
StatusPoller poller = new StatusPoller(agents, injector, 1);
poller.start();
try {
injector.enqueue("term_a", "first", TestTurnTokens.inert("term_a"));
assertTrue(errorThrown.await(2, TimeUnit.SECONDS),
"the first target must throw its test Error");
CompletableFuture<Void> delivered =
injector.enqueue("term_b", "second", TestTurnTokens.inert("term_b")).completion();
delivered.get(2, TimeUnit.SECONDS);
} finally {
poller.stop();
}
}
@Test
void anAbnormalExitClearsRunningSoStartCreatesANewLoop() throws Exception {
StatusPoller poller = new StatusPoller(new AgentControl(new FakeHerdr()), new Injector(new AgentControl(new FakeHerdr())), -1);
poller.start();
Thread first = threadOf(poller);
first.join(2000);
assertFalse(runningOf(poller), "an abnormal loop exit must clear running");
poller.start();
Thread restarted = threadOf(poller);
try {
assertNotSame(first, restarted, "start() must create a new loop after an abnormal exit");
restarted.join(2000);
} finally {
poller.stop();
}
}
private static Thread threadOf(StatusPoller poller) throws ReflectiveOperationException {
Field field = StatusPoller.class.getDeclaredField("thread");
field.setAccessible(true);
return (Thread) field.get(poller);
}
private static boolean runningOf(StatusPoller poller) throws ReflectiveOperationException {
Field field = StatusPoller.class.getDeclaredField("running");
field.setAccessible(true);
return field.getBoolean(poller);
}
private static final class ErrorOnceForFirstTarget implements HerdrClient {
private final FakeHerdr delegate;
private final CountDownLatch errorThrown;
private final AtomicBoolean first = new AtomicBoolean(true);
private ErrorOnceForFirstTarget(FakeHerdr delegate, CountDownLatch errorThrown) {
this.delegate = delegate;
this.errorThrown = errorThrown;
}
@Override
public JsonNode call(String method, Object params) {
if (method.equals("agent.get") && params instanceof Map<?, ?> map
&& "w2:p7".equals(map.get("target")) && first.compareAndSet(true, false)) {
errorThrown.countDown();
throw new AssertionError("test Error from the first poll target");
}
return delegate.call(method, params);
}
@Override
public void close() {
delegate.close();
}
}
}
@@ -0,0 +1,91 @@
package dev.ltms.fleet.session;
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.WorkspaceControl;
import dev.ltms.fleet.member.ClaudeCodeLauncher;
import org.junit.jupiter.api.Test;
import java.lang.reflect.Field;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.LongSupplier;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
class SessionReaperResilienceTest {
@Test
void anErrorInOneIterationDoesNotStopTheNextIteration() throws Exception {
CountDownLatch errorThrown = new CountDownLatch(1);
CountDownLatch nextIteration = new CountDownLatch(1);
AtomicBoolean first = new AtomicBoolean(true);
LongSupplier clock = () -> {
if (first.compareAndSet(true, false)) {
errorThrown.countDown();
throw new AssertionError("test Error from the first reap iteration");
}
nextIteration.countDown();
return System.nanoTime();
};
SessionReaper reaper = new SessionReaper(sessionManager(clock), 60, 1);
reaper.start();
try {
assertTrue(errorThrown.await(2, TimeUnit.SECONDS),
"the first reap iteration must throw its test Error");
assertTrue(nextIteration.await(2, TimeUnit.SECONDS),
"the reaper must continue to the next iteration after an Error");
} finally {
reaper.stop();
}
}
@Test
void anAbnormalExitClearsRunningSoStartCreatesANewLoop() throws Exception {
SessionReaper reaper = new SessionReaper(sessionManager(System::nanoTime), 60, -1);
reaper.start();
Thread first = threadOf(reaper);
first.join(2000);
assertFalse(runningOf(reaper), "an abnormal loop exit must clear running");
reaper.start();
Thread restarted = threadOf(reaper);
try {
assertNotSame(first, restarted, "start() must create a new loop after an abnormal exit");
restarted.join(2000);
} finally {
reaper.stop();
}
}
private static SessionManager sessionManager(LongSupplier clock) {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
List.of("ccs", "ltms-local"), "tab", "fleetd-workers",
"worker: {profile} #{n}", null, null, null);
ClaudeCodeLauncher launcher = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null);
return new SessionManager(launcher, new FakeWorktrees(), clock);
}
private static Thread threadOf(SessionReaper reaper) throws ReflectiveOperationException {
Field field = SessionReaper.class.getDeclaredField("thread");
field.setAccessible(true);
return (Thread) field.get(reaper);
}
private static boolean runningOf(SessionReaper reaper) throws ReflectiveOperationException {
Field field = SessionReaper.class.getDeclaredField("running");
field.setAccessible(true);
return field.getBoolean(reaper);
}
}
@@ -0,0 +1,96 @@
package dev.ltms.fleet.testing;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import org.junit.jupiter.api.Test;
import org.slf4j.LoggerFactory;
import static org.junit.jupiter.api.Assertions.assertEquals;
/**
* fleetd #537: pins {@link CapturedLog#close}'s own contract — the appender detach half, the level
* restore half, and that {@link CapturedLog#setLevel} does not change what {@code close} restores.
* Before this, only the level-restore half was pinned (by {@code
* WorktreeSessionManagerTest.sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn}).
* Measured: deleting {@code logger.detachAppender(appender);} from {@code close()} still left
* {@code mvn clean install} green — 1701 tests, 0 failures — before this file existed.
*
* <p>Every test below uses a logger name no production class uses, and unique per test, so this
* file cannot become the next entry in fleetd #525's leak family: {@link CapturedLog}'s own
* javadoc re-measure commands (top of that file) would otherwise need to start naming this class.
*/
class CapturedLogTest {
/**
* The appender must be detached on close: an event logged through the raw logger after close
* must not land in {@link CapturedLog#events()}. Asserting on the observable list (rather than
* {@code logger.iteratorForAppenders()}) is what the ticket asked for, and it is also what a
* real leak would actually break — a later test's own {@code ListAppender} silently gaining
* events emitted by code under test that has nothing to do with it.
*/
@Test
void closeDetachesTheAppenderSoALaterLogIsNotCaptured() {
String loggerName = "capturedlog-test-only.appender-detach";
Logger rawLogger = (Logger) LoggerFactory.getLogger(loggerName);
CapturedLog log = CapturedLog.of(loggerName);
rawLogger.info("while open");
int eventsWhileOpen = log.events().size();
assertEquals(1, eventsWhileOpen, "the event logged while open must be captured");
log.close();
rawLogger.info("after close");
assertEquals(eventsWhileOpen, log.events().size(),
"close() must detach the appender: an event logged after close must not be "
+ "captured, but the captured list grew from " + eventsWhileOpen + " to "
+ log.events().size());
}
/**
* The helper's own headline contract, pinned in one place independent of any production
* class's behaviour: {@code close()} restores the level the logger had before {@link
* CapturedLog#at} pinned it.
*/
@Test
void closeRestoresTheLevelCapturedAtOpen() {
String loggerName = "capturedlog-test-only.level-restore";
Logger rawLogger = (Logger) LoggerFactory.getLogger(loggerName);
rawLogger.setLevel(Level.DEBUG);
CapturedLog log = CapturedLog.at(loggerName, Level.ERROR);
assertEquals(Level.ERROR, rawLogger.getLevel(), "the pinned level took effect while open");
log.close();
assertEquals(Level.DEBUG, rawLogger.getLevel(),
"close() must restore the level captured when at() was called (DEBUG), not leave "
+ "the pinned level (ERROR) in place");
}
/**
* {@link CapturedLog#setLevel}'s javadoc claims that re-pinning the level mid-capture does not
* change what {@code close()} restores — that restore always uses the level captured when the
* instance was created, never a value set through {@code setLevel}. Nothing checked this
* before: open with a pinned WARN, call {@code setLevel(TRACE)}, close, and the result must be
* the level from BEFORE {@code at} — neither WARN nor TRACE.
*/
@Test
void setLevelDuringCaptureDoesNotChangeWhatCloseRestores() {
String loggerName = "capturedlog-test-only.setlevel-no-effect";
Logger rawLogger = (Logger) LoggerFactory.getLogger(loggerName);
rawLogger.setLevel(Level.DEBUG);
CapturedLog log = CapturedLog.at(loggerName, Level.WARN);
log.setLevel(Level.TRACE);
assertEquals(Level.TRACE, rawLogger.getLevel(), "setLevel took effect immediately");
log.close();
assertEquals(Level.DEBUG, rawLogger.getLevel(),
"close() must restore the level captured at open (DEBUG) regardless of any "
+ "later setLevel() call: it must be neither WARN (the level pinned by "
+ "at()) nor TRACE (the level set via setLevel() mid-capture), but got "
+ rawLogger.getLevel());
}
}
+68 -18
View File
@@ -96,13 +96,23 @@ SYSTEMD_UNIT='fleetd'
# 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.
# fleetd #492 follow-up, refined by fleetd #545: three states, not two, set by
# systemd_loaded/systemd_installed —
# 0 = no error, the probe ran and gave a clean answer.
# 1 = the probe RAN and answered badly: `systemctl` exited non-zero AND wrote something to
# stderr, 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).
# 2 = the probe could not even be SET UP: the `mktemp` call that makes a place to capture
# `systemctl`'s stderr failed before `systemctl` ever ran. This is a fleetd #545 fix: on GNU
# coreutils (every Linux distribution) a template with no `X`s made `mktemp` fail every
# single time, and the two states were folded into one flag and one message that named
# cause 1 ("systemctl exited non-zero and reported an error on stderr") for a failure that
# was actually cause 2 — systemctl was never executed at all. One flag with two meanings
# needing different messages was the defect; a third value is the fix, not a second flag.
# 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
@@ -130,11 +140,35 @@ ok() { printf ' ok %s\n' "$*"; }
warn() { printf ' WARN %s\n' "$*"; }
die() { printf '\n FAIL %s\n\n' "$*" >&2; exit 1; }
# fleetd #550 — shasum is macOS-only (it ships with Perl, which Debian/Ubuntu/etc. do not install
# by default); GNU coreutils (every mainstream Linux distro) ships sha256sum instead and has no
# shasum at all. Prefer sha256sum, fall back to shasum -a 256 — same idiom as
# probe-member-credentials.sh's `hasher` selection — and when NEITHER is on PATH, say so plainly.
# That third answer matters: without it, a missing hasher makes `cut` succeed on empty input, and
# under `set -o pipefail` the pipeline as a whole still fails, so a caller's own `|| echo "absent"`
# then reports a file that is right there as though it were gone. `hash256` never does that — it
# only ever hashes or says it could not.
hash256() {
local f="$1"
if command -v sha256sum >/dev/null 2>&1; then
sha256sum "$f" | cut -c1-12
elif command -v shasum >/dev/null 2>&1; then
shasum -a 256 "$f" | cut -c1-12
else
echo "unhashable"
fi
}
# 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"; }
# fleetd #550 — THREE distinct answers now, not two: `[ -f "$f" ]` already separates "the jar is
# not there" (-> "absent") from "the jar is there"; for the second case, hash256 itself separates
# "hashed it" (a 12-char hex string) from "could not hash it" (-> "unhashable", when no hasher is
# on PATH). "absent" must never be the answer for a file that exists — that conflation, on Linux,
# was the whole defect this ticket fixes.
jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && hash256 "$f" || echo "absent"; }
running_pid() { pgrep -f "$PATTERN" || true; }
# fleetd #493 — three small, independently testable pieces of "never build into the path a
@@ -238,12 +272,17 @@ launchd_loaded() { launchctl list "$LAUNCHD_LABEL" >/dev/null 2>&1; }
# 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
# nonzero exit. They now capture stderr separately and set their own *_ERRORED flag to 1 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".
#
# fleetd #545: the flag has a third value, 2, set when the `mktemp` call that sets up the probe's
# own stderr capture fails, before `systemctl` ever runs — see the SYSTEMD_LOADED_ERRORED /
# SYSTEMD_INSTALLED_ERRORED comment above their initialization for why this is a third value on the
# same flag, not a second flag.
#
# "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.
@@ -251,8 +290,8 @@ 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
if ! err_file="$(mktemp -t systemd-installed-err.XXXXXX)"; then
SYSTEMD_INSTALLED_ERRORED=2
return 1
fi
out="$(systemctl --user list-unit-files "$SYSTEMD_UNIT.service" --no-legend 2>"$err_file")" || rc=$?
@@ -275,8 +314,8 @@ 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
if ! err_file="$(mktemp -t systemd-loaded-err.XXXXXX)"; then
SYSTEMD_LOADED_ERRORED=2
return 1
fi
systemctl --user is-active "$SYSTEMD_UNIT" >/dev/null 2>"$err_file" || rc=$?
@@ -302,7 +341,7 @@ systemd_loaded() {
# lost value instead of surfacing it (fleetd #497's defect class).
unload_launchd_if_loaded() {
local err_file rc=0
if ! err_file="$(mktemp -t launchd-unload-err)"; then
if ! err_file="$(mktemp -t launchd-unload-err.XXXXXX)"; then
die "could not create a temp file to capture 'launchctl unload' stderr — cannot tell a real
failure from a clean already-unloaded answer, so refusing to guess. The daemon's
supervision state was NOT touched."
@@ -317,7 +356,7 @@ unload_launchd_if_loaded() {
stop_systemd_if_loaded() {
local err_file rc=0
if ! err_file="$(mktemp -t systemd-stop-err)"; then
if ! err_file="$(mktemp -t systemd-stop-err.XXXXXX)"; then
die "could not create a temp file to capture 'systemctl --user stop' stderr — cannot tell a
real failure from a clean already-stopped answer, so refusing to guess. The daemon's
supervision state was NOT touched."
@@ -349,6 +388,14 @@ stop_systemd_if_loaded() {
# "none" now means only: neither supervisor is installed, neither is loaded, and neither probe
# errored.
#
# fleetd #545: *_ERRORED carries a THIRD state (2 = the probe's own mktemp setup failed, before
# `systemctl` ever ran — see the flag's own comment above its initialization), and it must never be
# reported with the same detail text as state 1 (`systemctl` ran and answered badly on stderr). The
# two are different facts about different failures, and conflating them makes the "unclear" message
# assert a cause ("systemctl exited non-zero and reported an error on stderr") that was never
# measured when the real cause was state 2. detect_supervisor below picks the detail text off the
# flag's value, not off a single "errored at all" boolean.
#
# 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)`):
@@ -382,7 +429,10 @@ detect_supervisor() {
launchd_installed && li=1
systemd_installed && si=1
if [ "$SYSTEMD_LOADED_ERRORED" = 1 ] || [ "$SYSTEMD_INSTALLED_ERRORED" = 1 ]; then
if [ "$SYSTEMD_LOADED_ERRORED" = 2 ] || [ "$SYSTEMD_INSTALLED_ERRORED" = 2 ]; then
detail="the systemd --user probe for '$SYSTEMD_UNIT' could not even be set up (a temp file to capture systemctl's stderr could not be created) — systemctl was never run, so this says nothing about systemd, the user bus, or the unit itself"
kind="unclear"
elif [ "$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
@@ -830,7 +880,7 @@ if [ "$DO_BUILD" = 1 ]; then
# 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)"
BUILD_LOG="$(mktemp -t fleetd-build.XXXXXX)"
echo " log: $BUILD_LOG"
if ! mvn -f "$MODULE/pom.xml" clean install > "$BUILD_LOG" 2>&1; then
grep -E 'ERROR|BUILD FAILURE|Tests run:.*Failures: [1-9]|Tests run:.*Errors: [1-9]' "$BUILD_LOG" \
@@ -1060,7 +1110,7 @@ tail -n "+$((RESTART_MARK + 1))" "$OUT" 2>/dev/null \
# Errors since the restart, anchored to the marker so old noise cannot leak in. Keep the fresh
# region in a file because the classifier must preserve the order of errors and recoveries.
FRESH_LOG="$(mktemp -t fleetd-fresh-log)"
FRESH_LOG="$(mktemp -t fleetd-fresh-log.XXXXXX)"
trap 'rm -f "$FRESH_LOG"' EXIT
tail -n "+$((RESTART_MARK + 1))" "$OUT" > "$FRESH_LOG" 2>/dev/null || true
classify_amqp_connection_errors "$FRESH_LOG"
+166 -3
View File
@@ -128,8 +128,112 @@ STUB
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" \
local detail
detail="$(supervisor_detail_of "$result")"
printf '%s' "$detail" | grep -qF "$SYSTEMD_UNIT" \
|| fail "detail does not name the systemd unit whose probe errored"
# fleetd #545: this is the PROBE-RAN-AND-ANSWERED-BADLY case (systemctl actually executed and
# wrote to stderr) — it must carry that story and never the SET-UP-FAILED story (mktemp never
# even ran here), or the two "unclear" causes have collapsed back into one message that asserts a
# cause it did not measure, which is the exact defect this ticket exists to fix.
printf '%s' "$detail" | grep -qF "systemctl exited non-zero and reported an error on stderr" \
|| fail "detail does not say systemctl ran and answered with stderr: $detail"
printf '%s' "$detail" | grep -qF "could not even be set up" \
&& fail "detail wrongly claims the probe could not be set up, but systemctl actually ran and answered on stderr: $detail"
return 0
}
# fleetd #545 — the companion case to the probe-error test above: here `mktemp` itself fails
# (whatever the reason — the historical bug was a GNU-mktemp-rejects-a-template-with-no-Xs case,
# but this stub simulates ANY reason the probe's own stderr-capture temp file cannot be created,
# e.g. a full or unwritable temp dir) and `systemctl` is never invoked at all. Before this ticket,
# this collapsed into the SAME "systemctl exited non-zero and reported an error on stderr" detail
# as the sibling test above, which asserts a cause (systemctl ran and answered badly) that was
# never measured, because systemctl never ran. This proves the SET-UP-FAILED detail is distinct and
# does not claim systemctl said anything.
test_detect_supervisor_systemd_probe_setup_failure_is_unclear() {
# Re-source first for the same reason test_detect_supervisor_systemd_probe_error_is_unclear does:
# restore the REAL probe bodies before driving them through a stub PATH.
source "$ROOT/scripts/redeploy-fleetd.sh"
local bin_dir result rc=0
bin_dir="$TMP/stub-bin-mktemp-fails"
mkdir -p "$bin_dir"
# A systemctl stub that would fail loudly if it were ever actually invoked — proves the mktemp
# failure short-circuits the probe before systemctl runs, not merely that this test forgot to
# supply a working systemctl.
cat > "$bin_dir/systemctl" <<'STUB'
#!/usr/bin/env bash
echo "systemctl must never run when mktemp already failed" >&2
exit 1
STUB
chmod +x "$bin_dir/systemctl"
cat > "$bin_dir/mktemp" <<'STUB'
#!/usr/bin/env bash
echo "mktemp: cannot create temp file" >&2
exit 1
STUB
chmod +x "$bin_dir/mktemp"
PATH="$bin_dir:$PATH" systemd_loaded && rc=0 || rc=$?
[ "$rc" -ne 0 ] \
|| fail "systemd_loaded must not report loaded=true when its own mktemp setup failed"
assert_equals "2" "$SYSTEMD_LOADED_ERRORED" \
"systemd_loaded must flag a SETUP failure (2), distinct from a probe-answered-with-stderr failure (1)"
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 setup failure must read as unclear, not none"
local detail
detail="$(supervisor_detail_of "$result")"
printf '%s' "$detail" | grep -qF "could not even be set up" \
|| fail "detail does not say the probe could not be SET UP: $detail"
printf '%s' "$detail" | grep -qF "systemctl exited non-zero and reported an error on stderr" \
&& fail "detail wrongly asserts systemctl exited non-zero and reported an error on stderr, but systemctl was never run: $detail"
return 0
}
# fleetd #545 — source-text check: every `mktemp -t` template in redeploy-fleetd.sh must contain an
# `X` placeholder. BSD mktemp (macOS) tolerates a bare template with no `X`s and just appends its
# own random suffix, which is exactly why six such sites survived undetected here — GNU mktemp
# (every Linux distribution) refuses a template with fewer than three `X`s and exits non-zero. There
# is no BSD-vs-GNU seam to stub on this Mac, so this is a source-text check rather than a
# behavioural one, the same shape as test_refuse_drain_gate_call_site_present above. Anchored on
# `mktemp -t ` (with the trailing space) so it inspects only the `-t`-style templates this ticket is
# about, never the `mktemp -d` calls this file and test-probe-member-credentials.sh already use
# (both already carry their own `XXXXXX` and are a different mktemp mode entirely).
test_mktemp_dash_t_templates_have_x_placeholders() {
local src="$ROOT/scripts/redeploy-fleetd.sh" bad
bad="$(grep -n 'mktemp -t ' "$src" | grep -v 'XXX' || true)"
[ -z "$bad" ] \
|| fail "mktemp -t template(s) with no X placeholder (fails under GNU coreutils): $bad"
}
# fleetd #550 — the shape, not the named lines: #545 showed the exact same failure mode (a
# macOS-only idiom used with no portable fallback) spread from two sites to six across 91 commits
# before anyone tested the SHAPE rather than specific lines. This is the shasum sibling: any script
# under scripts/ that actually INVOKES the macOS-only hasher to compute a hash (as opposed to
# merely probing whether it exists with `command -v`, or mentioning it in prose) must also check
# for the portable one first in that same file — the prefer-portable-fall-back-to-macOS-only idiom
# probe-member-credentials.sh:273-279 and this ticket's own hash256 helper both follow.
#
# The needle is built from two concatenated pieces, deliberately never written as one literal
# string in this file: written whole, it would match THIS CHECK'S OWN source line once the loop
# below reaches this very file, and the check would then "pass" by matching itself rather than any
# real invocation elsewhere — a zero-findings result indistinguishable from a clean file.
test_no_unguarded_macos_only_hasher_calls() {
local needle f bad="" usage
needle='shasum'; needle="$needle -a"
for f in "$ROOT"/scripts/*.sh; do
[ -f "$f" ] || continue
usage="$(grep -Fn "$needle" "$f" || true)"
if [ -n "$usage" ]; then
grep -q 'command -v sha256sum' "$f" \
|| bad="$bad$(basename "$f") "
fi
done
[ -z "$bad" ] \
|| fail "script(s) invoke the macOS-only hasher with no portable-hasher-first fallback guard in the same file: $bad"
}
# fleetd #492 follow-up (Item 1): this must go through the REAL call-site shape at :437-440, not a
@@ -218,8 +322,11 @@ test_jar_id_defaults_to_live_and_reports_explicit_path() {
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)"
# fleetd #550: this reference hash must be computed the same portable way jar_id() itself now
# computes one — a bare, unguarded call to the macOS-only hasher here was exactly the item-2
# defect, dying with "command not found" on any Linux runner that has no such hasher at all.
live_hash="$(hash256 "$JAR")"
staged_hash="$(hash256 "$JAR_STAGED")"
default_result="$(jar_id)"
explicit_result="$(jar_id "$JAR_STAGED")"
JAR="$saved_jar"; JAR_STAGED="$saved_staged"
@@ -228,6 +335,26 @@ test_jar_id_defaults_to_live_and_reports_explicit_path() {
assert_equals "$staged_hash" "$explicit_result" "jar_id \"\$JAR_STAGED\" must report the hash of the staged jar, not fall back to \$JAR"
}
# fleetd #550 — closes a gap the test above leaves open. That test's own reference hash is now ALSO
# computed by calling hash256 (needed for item 2: the old bare macOS-only-hasher call there was the
# Linux crash), so its subject (jar_id, via hash256) and its reference (also hash256) share one
# instrument — they agree no matter which algorithm hash256 actually runs, so a mutation that swaps
# BOTH of hash256's arms for the wrong algorithm is invisible to it. This test's expected value
# comes from neither hasher: it is the published SHA-256 test vector for the 3-byte input "abc"
# (no trailing newline), written here as a literal constant, so it can still tell "hashed
# correctly" from "hashed, just with the wrong algorithm" — which is what this whole ticket is
# about.
test_hash256_computes_a_real_sha256() {
local dir f result
dir="$TMP/hash256-known-vector"; mkdir -p "$dir"
f="$dir/abc.txt"
printf 'abc' > "$f"
result="$(hash256 "$f")"
# SHA-256("abc") = ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad, the standard
# FIPS 180 test vector — first 12 hex chars, matching hash256's own `cut -c1-12`.
assert_equals "ba7816bf8f01" "$result" "hash256 of the literal 3-byte input 'abc' must be the known SHA-256 prefix, not some other algorithm's"
}
# 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`
@@ -246,6 +373,37 @@ test_jar_id_reports_absent_for_missing_file() {
assert_equals "absent" "$explicit_result" "jar_id with an explicit missing path must report absent"
}
# fleetd #550 — the whole point of this ticket: a jar that IS there but could not be hashed must
# never read the same as a jar that is not there at all. Drives the REAL hash256/jar_id bodies
# (never stubbed) through a stub PATH that contains neither of the two hashers this script knows —
# same technique test_detect_supervisor_systemd_probe_setup_failure_is_unclear uses for `mktemp`,
# except here the stub directory is used to REPLACE PATH rather than prepend to it, because the
# point is to make BOTH hashers unreachable, not to intercept one specific command while leaving
# everything else on the real PATH reachable. `[ -f ... ]` and the shell's own `command`/`echo`
# builtins need no PATH at all, so this is safe even with PATH reduced to an empty directory.
test_jar_id_reports_unhashable_when_no_hasher_on_path() {
local bin_dir dir saved_jar="$JAR" default_result explicit_result
bin_dir="$TMP/stub-bin-no-hasher"; mkdir -p "$bin_dir"
dir="$TMP/jar-id-no-hasher"; mkdir -p "$dir"
JAR="$dir/fleetd.jar"
printf 'a real jar that exists but nothing here can hash' > "$JAR"
[ -f "$JAR" ] || fail "test fixture error: \$JAR does not exist at $JAR"
default_result="$(PATH="$bin_dir" jar_id)"
explicit_result="$(PATH="$bin_dir" jar_id "$JAR")"
JAR="$saved_jar"
[ "$default_result" != "absent" ] \
|| fail "jar_id reported absent for a file that exists, only because no hasher was on PATH"
[ "$explicit_result" != "absent" ] \
|| fail "jar_id (explicit path) reported absent for a file that exists, only because no hasher was on PATH"
# A 12-char hex hash is the OTHER wrong answer here: with no hasher at all, nothing could have
# produced one, so a value that merely happens to look like one would mean the stub failed to
# hide the real hashers rather than that jar_id degraded correctly.
printf '%s' "$default_result" | grep -Eq '^[0-9a-f]{12}$' \
&& fail "test fixture error: PATH stub did not actually hide the real hasher(s) — got what looks like a real hash"
assert_equals "unhashable" "$default_result" "jar_id with no hasher on PATH must report a third, distinct state — never absent, never a hash"
assert_equals "unhashable" "$explicit_result" "jar_id (explicit path) with no hasher on PATH must report the same third state"
}
# 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
@@ -1088,6 +1246,9 @@ 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_detect_supervisor_systemd_probe_setup_failure_is_unclear
test_mktemp_dash_t_templates_have_x_placeholders
test_no_unguarded_macos_only_hasher_calls
test_require_drivable_supervisor_refuses_ambiguous
test_require_drivable_supervisor_refuses_unclear
test_require_drivable_supervisor_accepts_known_kinds
@@ -1095,7 +1256,9 @@ 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_hash256_computes_a_real_sha256
test_jar_id_reports_absent_for_missing_file
test_jar_id_reports_unhashable_when_no_hasher_on_path
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