944 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
ltms	93a9ed3f83	Merge #549: widen Injector's delivery catch to Throwable (#546) ... Closes the re-delivery window that merging #543 opened. I caused that; this closes it the same day. Verified by me on the branch at 87871ea, base 0b032f5 (not stale). Production diff is 11 lines: `catch (RuntimeException)` -> `catch (Throwable)` at Injector.java:391, and the `sendError` local widened to `Throwable` so it compiles. I checked every use of `sendError` myself — :533 `getMessage()` and :534 `completeExceptionally(Throwable)` — so the wider type reaches nothing that needed the narrow one. Build on the branch: exit 0, Tests run: 1719, Failures: 0, Errors: 0, Skipped: 0 (1716 on main plus the 3 new tests). #459's javadoc reference gate: exit 0, 0 reference errors. Gitea CI run 1803 on 87871ea: success. Three mutations of my own, none of them the ones the worker used: 1. Reverted the catch to `RuntimeException`. RED: `anErrorFromSendDoesNotRedeliverOnASecondRound` "expected: <1> but was: <2>" prompt calls, and `anErrorFromSendRemovesTheMessage...` reporting the Error escaping `onStatus`. That second message is the defect itself, stated by the test. 2. Deleted the `t.queue.poll()` in the catch arm. RED on the new test AND on the pre-existing `sendFailureDropsMessageAndFailsItsFuture` — so the new test is not carrying that behaviour alone. 3. Wrote `DELIVERED` instead of `NOT_DELIVERED` at :400, the catch arm only. RED on the new test and on `aHerdrExceptionFromSendStillProducesNotDeliveredUnchanged`, which is the control that proves the widening did not quietly change the ordinary path. All three restored; sha256 back to 97c560b6e33fc49a1772abec92e5bbab613f8991deba2220d30221a2f546ba14. Green control after the restores: InjectorTest 37/37, exit 0. My third mutation did not apply on its first attempt — it asserted a unique match on `p.state = Pending.State.NOT_DELIVERED;`, which occurs twice (:400 and :419), so the script wrote nothing and the test run came back exit 0. That is not a surviving mutant, it is a non-result wearing the same clothes. The pristine-anchor count catching it is the only reason I noticed. Deliberately NOT fixed here, filed as #551: the catch arm assumes that reaching it means nothing was sent, and nothing establishes that. `agent.prompt` pastes and submits in one call, and every failure in the response half of `UnixSocketHerdrClient.call()` — dropped connection, malformed line, error result — is a `HerdrException`, which is a `RuntimeException`, which this catch arm already caught before today. So "records NOT_DELIVERED for a delivery that happened" is older than this PR and is not created by it. The fleet01 lead argued it was a trap inside this change and asked to be argued out of it before the merge; the measurement above is the argument, and their underlying diagnosis is right and is now #551 with their wording on it.	2026-09-12 09:39:41 +02:00
ltms	0b032f5a1a	Merge #548: fix mktemp -t templates for GNU coreutils, split the unclear-supervisor detail (#545) ... 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 ... 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 ... 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 ... 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 ... 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	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	2026-09-12 09:02:48 +02:00
ltms	4a8a780274	Merge pull request 'fleetd #426: pin FleetHealthMonitor.coverage and its HealthCoverageSource call site' (#542) from worker/426-health-coverage-ef1fd4-4 into main	2026-09-12 08:55:46 +02:00
Dai Ha	343ce0f4c0	fleetd #538: recover polling loops after errors	2026-09-12 13:47:44 +07:00
ltms	cec3e191d4	Merge pull request 'fleetd #459: lint Javadoc references in CI' (#539) from worker/459-broken-link-targets-cadc17-5 into main	2026-09-12 08:46:23 +02:00
Dai Ha	1850a5f324	fleetd #426: pin FleetHealthMonitor.coverage and its HealthCoverageSource call site ... FleetHealthMonitor.coverage had zero references in the test tree — not the method, not either output string, not the field it populates. Inverting `enabled`, swapping "full"/"detection-only", or breaking the argument pairing at the HealthCoverageSource call site in Fleetd.java all shipped a green build. Extract the HealthCoverageSource lambda out of Fleetd.main into a package-private static factory (healthCoverageSource(ConfigRef)), the same shape capacitySource/quarantineSource already use for the identical argument-pairing risk (fleetd #415). #407's "keep the config invalid, assert on the log line before validateAll() throws" option does not apply here: this call site is built well after validateAll() and after a real herdr socket connect, so driving it through a real Fleetd.main would require the socket I/O this ticket's tests must not do. Add FleetHealthMonitorCoverageTest (the three-branch method itself) and FleetdHealthCoverageSourceWiringTest (the call site, via a real FleetConfig.load + ConfigRef against @TempDir fixtures, including a hot notifications-reload case). Output strings are unchanged — "detection-only" is still what a live fleet_list reports today. Measured: all three mutations killed by the new tests.	2026-09-12 13:43:50 +07:00
Dai Ha	e4eb3dbed4	fleetd #504 item 1: stop swallowing real launchctl/systemctl failures on the 'loaded but not running' path ... The two 'loaded but not currently running' branches in the stop step (launchd/systemd, reached when $OLD_PID is empty) ran 'launchctl unload'/'systemctl --user stop' with '2>/dev/null || true' and printed 'ok' unconditionally. That swallowed a real supervisor failure (e.g. launchd or the systemd user bus unreachable) exactly like a harmless already-stopped answer, and let the script proceed to start a new daemon believing nothing was loaded -- the two-daemons failure fleetd #492 exists to prevent. Adds unload_launchd_if_loaded/stop_systemd_if_loaded, applying systemd_loaded's own pattern (capture stderr separately; a non-zero exit WITH stderr is a real failure, a non-zero exit with empty stderr is a clean already-stopped answer) to the write side. The two call sites now use these functions instead of the bare '|| true'. Adds 5 tests: dies-on-real-failure and tolerates-clean-negative for each function, plus a source-text check that the main flow calls the new functions instead of the original bare '2>/dev/null || true'. All 5 verified by mutation (reintroducing the swallow, and separately over-correcting to die unconditionally) -- each goes red with its own message, restores byte-identical (full sha256), and passes a green control.	2026-09-12 13:42:27 +07:00
ltms	57cd96f5e6	Merge pull request 'fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts' (#540) from worker/537-capturedlog-close-e4c437-2 into main	2026-09-12 08:42:19 +02:00
Dai Ha	202e37e3b3	fleetd #537: pin CapturedLog.close()'s appender-detach and setLevel-immunity contracts ... Only the level-restore half of close() was pinned before this (WorktreeSessionManagerTest). Deleting logger.detachAppender(appender) from close() left mvn clean install green (1701 tests, 0 failures) -- the appender-detach half of the contract was unmeasured. Adds CapturedLogTest with three tests, each using a logger name no production class uses: - closeDetachesTheAppenderSoALaterLogIsNotCaptured: an event logged after close() must not land in events(). - closeRestoresTheLevelCapturedAtOpen: the helper's headline contract in one place, independent of any production class. - setLevelDuringCaptureDoesNotChangeWhatCloseRestores: setLevel()'s own javadoc claim that close() always restores the level captured at construction, never a value set through setLevel() mid-capture. Test-only change; CapturedLog.java itself is untouched.	2026-09-12 13:36:55 +07:00
Dai Ha	90253f832d	fleetd #459: lint Javadoc references in CI	2026-09-12 13:36:37 +07:00
ltms	f1640f5dcc	Merge pull request 'fleetd #535: convert FleetdLeadMailboxSelectionTest to CapturedLog' (#536) from worker/535-appender-leak-fe74c1-1 into main	2026-09-12 08:25:01 +02:00
Dai Ha	c7903c1efe	fleetd #535: convert FleetdLeadMailboxSelectionTest to CapturedLog ... Three call sites (captureFleetdLogs at :55) attached a ListAppender to the Fleetd.class logger with addAppender and never detached it, and never called appender.setContext(...) either. Logback Logger instances are cached per class and shared for the whole JVM, and surefire reuses forks, so all three appenders stayed attached for every later test in the fork. Convert all three call sites to CapturedLog.of(Fleetd.class) (added in #533) via try-with-resources, which detaches the appender and sets the context for free. Delete captureFleetdLogs(); nothing calls it now.	2026-09-12 13:17:21 +07:00
ltms	7d711942fe	Merge #534: detect a died shutdown drain the ERROR count is blind to (fleetd #512 part 2) ... Verified independently. The branch is based on 8335b12 while main is at bec87f9, so I merged locally first and tested the MERGED tree, not the branch — a clean auto-merge is not a working merge. Merged tree checks: - `bash -n` exit 0 on both scripts, under /bin/bash 3.2.57 and env bash 5.3.9. - Suite exit 0, 0 lines matching `^FAIL:`, 255 bytes of output. 60 test functions defined, 60 invoked, and no defined-but-never-invoked orphan (checked with a comm against the invocation list, not by comparing two counts — two equal counts can both be wrong). - My own comment fix from bec87f9 survived the merge and is still at :317. I ran two mutations the worker did not, per "mutate the half the worker did not": (A) The one that matters, because it is the defect this ticket exists to prevent: collapsed the `unknown` state into `complete`, so "cannot tell" reports as a pass. Result exit 1, one FAIL: `cannot-tell fixture must set REDEPLOY_DRAIN_STATE=unknown: expected unknown, got complete`. So the third state is genuinely load-bearing, not decoration. (B) Broke the positive check: changed `find_drain_complete_line`'s pattern from `drain complete: released=` to `drain finished: released=`, one site. Result exit 1, one FAIL: `find_drain_complete_line did not capture the present line`. Proof that (B) applied, against a pristine copy: the full grep line 1 -> 0, the mutant form 0 -> 1, and the bare phrase 2 -> 1 with the comment occurrence untouched. Both files restored byte-identical; `git diff --quiet` clean; green control re-run. A note on my own proof cell for (B), because it was wrong the first time. I wrote the counts with escaped double quotes inside an already double-quoted command substitution, so the shell split the pattern on spaces and grep treated the words as filenames. It printed "2 and 2" alongside `ugrep: No such file or directory` warnings — a symmetric, plausible-looking pair that meant nothing. The kill itself was never in doubt, since the suite named the exact function, but the cell that was supposed to prove the mutation applied proved nothing. Re-done with single quotes. This is the same trap already written down for this repo, hit by me, in a cell whose only purpose was to guard against exactly this. One thing I checked that no test covers: the main flow's `HAD_OLD_PID=0; [ -n "$OLD_PID" ] && HAD_OLD_PID=1` runs under `set -euo pipefail`, and on a cold start the test fails. Sourcing stops before the main flow, so no behavioural test reaches that line. If `set -e` fired there, every cold start would abort before the health checks. It does not: `set -e` exempts the left side of an `&&` list, confirmed by running it under both shells — `survived, HAD_OLD_PID=0` on 3.2.57 and on 5.3.9. Safe, but it is untested main-flow wiring, which is the same class as #528's item 1 and belongs on that list. On the `n/a` fourth state, which the worker flagged for a reviewer's judgment rather than quietly keeping: accepted, and it is in scope. The ticket asked for a third state because a sentinel conflating "no" with "cannot tell" hides two causes needing opposite handling. "The question does not apply" is a third such cause, not a variant of "cannot tell". Without it, the new warning would fire on every clean cold start, and a warning that cries wolf on the most common path trains the operator to skip it — which destroys the absence signal just as surely as putting the completion line in a `finally` would. The worker also proved the gate is actually consulted, using a cold-start fixture whose content deliberately looks like a died drain, so the test would fail if the gate were skipped. That is the right way to test a gate. Both remaining outcomes are correctly excluded from the "no ERROR lines since restart" summary: only `complete` and `n/a` let it print.	2026-09-12 08:04:15 +02:00
Dai Ha	bec87f987c	scripts: name the mechanism in detect_supervisor's constraint 2, not a line number ... Constraint 2 read "This script runs under `set -euo pipefail` (line 50), so an unset variable is a loud failure." Two problems, both small and both the same family as fleetd #494 — a comment that states the wrong reason. The line number was stale: the `set` line is at 54, not 50. It was the only line-number citation in the file, and a citation like that goes stale on the next insert above it, silently, with nothing to catch it. The mechanism was also misattributed. What makes an unset variable a loud failure is `set -u`. Naming the whole `-euo pipefail` string invites the reader to credit pipefail for it, which is the mistake fleet01 flagged on a different cell this week: pipefail is insurance against a future pipeline stage, not what catches the current shape. Now names `set -u` and says where it is without a number, and records why the number is gone so nobody adds one back. Comment only. bash -n exit 0 under /bin/bash 3.2.57 and env bash 5.3.9; scripts/test-redeploy-fleetd.sh exit 0 with 0 lines matching ^FAIL:.	2026-09-12 12:58:37 +07:00
Dai Ha	7f8a8829f9	fleetd #529 follow-up: the new helper's javadoc claimed a reach it does not have ... CapturedLog's class javadoc said it is "the one way to pin or capture a logger's level and output in this test tree". Measured on main at af95897, that is false: nine test files still hand-roll the ListAppender + setLevel + finally detachAppender pattern, with 42 setLevel calls on a raw logback Logger between them. None of those nine is a defect. Every one pairs its pin with a restore, so none is the fleetd #525 leak, and #529's scope was the 19 unrestored pins only. The problem is the sentence, not the code: a reader who believes "the one way" and then greps finds nine counter-examples and cannot tell a leftover from a violation. That is the same shape as a wrong reason in a comment — the text survives while the fact under it moves. Replaced with what is actually true: new code must use the helper, the pattern still exists elsewhere, and here is the list plus the two commands that re-measure it. The paragraph says to delete itself once the first command comes back empty, rather than to keep a count up to date. Javadoc only. mvn -f fleetd/pom.xml test-compile exit 0.	2026-09-12 12:58:26 +07:00
ltms	af9589783e	Merge #533: promote CapturedLog to a shared test helper and close the logger-level leak (fleetd #529) ... Verified independently in a scratch worktree at 8ea5c2b, not promoted from the worker's report. Build: `mvn -f fleetd/pom.xml clean install` exit 0, `Tests run: 1701, Failures: 0, Errors: 0, Skipped: 0`, BUILD SUCCESS. Base arithmetic, measured rather than carried forward: a6415f3 (this branch's parent) has 1695 `@Test` plus 2 parameterized/repeated, and the branch has 1696 plus 2 — a delta of exactly +1, matching the per-file count (WorktreeSessionManagerTest 24 -> 25). So 1700 -> 1701 is the one new proving test and nothing else. A number I had carried from earlier in the session said 1699; that number was wrong and is retired. No Java file differs between a6415f3 and main at 8335b12, so the base count is the same on both. Mutation (a), the shared instrument: removed `logger.setLevel(originalLevel);` from `CapturedLog.close()`. Result exit 1, `Tests run: 25, Failures: 1`, the single failure being `sharedSessionManagerLoggerLevelIsRestoredAfterDirtyWorktreeReleasePinsWarn` with `expected: <TRACE> but was: <WARN>`. Restored byte-identical. Mutation (b), the use site: replaced the try-with-resources in `releasePreservesDirtyWorktreeAndLogsWarn` with the pre-#525 hand-rolled `ListAppender` + `setLevel` + `finally detachAppender` pattern. Result exit 1, one failure, the same assertion. Restored byte-identical. Green control on the restored tree: `git diff --quiet` clean, full suite exit 0, 1701/0/0/0. Both mutants were killed, so per the economy fleet01 proposed and #529 adopted, neither needs a separate harness-proof cell — the kill is the proof the cell can go red. Leak survey, re-measured here rather than taken from the report: on a6415f3, 9 files carry 19 `setLevel` pins on a raw logback `Logger` with zero restoring call; on the branch that set is empty. The 7 remaining `setLevel` calls in `GitWorktreesTest` are `reportingLog.setLevel(...)` on the `CapturedLog` instance, whose `close()` restores the original, so they are re-pins and not leaks. File hashes match the worker's report exactly, head and tail: CapturedLog.java 486d6f5b5a30dc5ef7f75e5e10be353e720fb0de503825e88e8d96e30a61a2f7, WorktreeSessionManagerTest.java a722a98d828c82e00415d2a924d341e177ded704262aaa5963ea2d09a683df94. Two things follow this merge rather than block it, both filed separately: one javadoc sentence in the new helper overstates its own reach, and the worker's item 4 reports a separate appender leak outside this ticket's scope.	2026-09-12 07:57:25 +02:00
Dai Ha	190436c9cf	fleetd #512 part 2: detect a died shutdown drain the ERROR count is blind to ... The previous daemon's dead shutdown drain (an uncaught exception in a shutdown thread) never passes through the logger, so it never carries an ERROR/SEVERE token, so redeploy-fleetd.sh's existing ERROR-count classifier is structurally blind to it and prints a confident "no ERROR lines since restart" while the drain actually died. Add scan_uncaught_exceptions (greps the shutdown window for the failure's real shape: `Exception in thread`, `NoClassDefFoundError`) and find_drain_complete_line (checks for #522's SessionManager.drainAll completion line). Compose both in report_shutdown_drain, a single decision+action function the main flow calls unconditionally (same shape as swap_if_built/refuse_drain_gate from #521/#528), which resolves to one of four outcomes: complete, died, unknown ("cannot tell" — the line is absent for either of two reasons that need opposite handling: the previous daemon predates #522, or its drain failed without throwing), or n/a (no previous daemon was actually stopped this run). Never fails the redeploy; warns loudly instead. Gate the "no ERROR lines since restart" summary line on the new outcome so it never reads as reassurance when the drain died or the outcome is "cannot tell" (item 4 of the ticket). Tests: 11 new test functions (60 defined/invoked, was 49), covering both pure classifiers, all four report_shutdown_drain outcomes, a source-grep proof of the main-flow call site (sourcing stops before the main flow runs), an ordering check, and the item-4 gating. Full suite green (exit 0, 0 anchored FAIL lines). Five mutations applied and killed by hand during review, each restored to a byte-identical file afterward.	2026-09-12 12:55:54 +07:00
Dai Ha	8ea5c2bb1f	fleetd #529: promote CapturedLog to a shared test helper, close the logger-level leak ... ch.qos.logback.classic.Logger instances are cached per class and shared for the whole JVM, and surefire reuses forks. A test that pins a shared logger's level and restores only the appender leaves that level pinned for every test that runs after it, in the same class or a different one in the same fork. Move CapturedLog (merged in #527 for #525) out of SessionManagerTest into dev.ltms.fleet.testing.CapturedLog, and convert all 19 unrestored setLevel pins across 9 files to it, so there is exactly one way to capture and pin a logger in this test tree: - FleetdAwaitHerdrTest, FleetdReplyInboxSelectionTest, AuditLogTest, CompletionResolverTest, InjectorTest (4), LeadRolloverTest, AmqpConnectionFailureLoggerTest, GitWorktreesTest (8), WorktreeSessionManagerTest. AuditLog logs through a named "audit" logger rather than a class, so CapturedLog gains String-named at()/of() overloads alongside the existing Class-based ones, plus a setLevel() method so a fixture that already pinned a coarser baseline (GitWorktreesTest's @BeforeEach) can re-pin further for one test without losing what close() restores. Adds an ordered proving test to WorktreeSessionManagerTest asserting the SessionManager logger level is back to a known baseline after the dirty- worktree release test runs; this proves the within-class case only, since JUnit does not guarantee cross-class ordering. Every existing intentional pin (SessionManagerTest's two explicit INFO pins and its @BeforeAll DEBUG baseline) is left untouched, per the ticket.	2026-09-12 12:48:49 +07:00
ltms	8335b12562	Merge #532: pin drain_gate_refusal's call site, not just the predicate (fleetd #528) ... Verified independently in my own worktree at the pushed head 7c34e8f, not taken from the worker's report. CI run 1781: success. The shape is the one #528 asked for and the one #526 arrived at: refuse_drain_gate composes the message via drain_gate_refusal AND calls die itself, and the main flow calls it unconditionally at :710. No guard is left in the main flow to remove, invert or bypass on its own. My measurements: test functions defined / invoked 49 / 49 (was 44/44; +5) bash -n, /bin/bash 3.2.57 rc=0 on both files bash -n, env bash 5.3.9 rc=0 on both files clean control exit 0, 0 lines matching ^FAIL:, 256 bytes Two mutations, both killed, each by a differently named failure: call site deleted (the item-1 mutation: :710 replaced by a flat die "aborted — nothing changed") -> exit 1, 1 ^FAIL: line, 87 bytes FAIL: could not find the main flow's refuse_drain_gate call site in redeploy-fleetd.sh refuse_drain_gate stops consulting the predicate (its body's die "$(drain_gate_refusal ...)" replaced by a flat message) -> exit 1, 1 ^FAIL: line FAIL: refuse_drain_gate build-ran+staged-present die message does not name the staged jar The first cell is the point of the ticket. Before this change the same mutation gave exit 0, zero FAIL lines and output byte-identical to a clean run at 256 bytes. It now exits 1 and names the missing call site. Each mutation was proven applied with a uniquely tagged marker plus a second, different search string, with a control against a pristine copy showing the exact inverse (1/0 mutated, 0/1 pristine), and the function definition confirmed still present so the mutation targeted the call and not the function. Restored byte-identical to 0e5a99a22c9c65f72960d8f179ca5299307889e06bc42131f098a513e7b97bd6 and the final control is green. Needle uniqueness checked, because this is where it could have gone wrong: grep -cF 'refuse_drain_gate "$DO_BUILD" "$JAR_STAGED"' on the production script returns 1, at :710, the real call site. The worker hit the self-match trap while writing the comment above refuse_drain_gate — their first draft quoted the call-site string literally, which would have let the source-text test match the comment instead of the call — caught it themselves, and reworded so the comment cannot become a second match. That is the same trap that cost me a false pass on a probe earlier today, and catching it unprompted is the better half of this PR. The dead-check sweep came back as a real negative, with the reasoning shown rather than asserted: of the five scripts under set -e with pipefail, every pipe-into-assignment already carries || true or || echo, and the remaining two scripts have no pipe-into-assignment at all. probe-member-credentials.sh and deploy/herdr-inner.sh correctly excluded for not having set -e. No live instances. One inaccuracy in the report, in the report only: it abbreviates the restored hash as "0e5a99a2...78f0a", and that tail does not occur in the actual hash, which ends b97bd6. I hashed the committed file myself and confirmed the restore matched, so the file is right and only the quoted abbreviation is wrong. Flagged because an abbreviated hash that nobody can match against anything is worse than no hash. wait_for_daemon_exit's call site (item 2) stays open as the ticket scoped it — source-text pinned only, "partially pinned, not audited". The seven untested main-flow decisions are untouched; the worker correctly notes it changed the body of one of those if blocks while leaving the guard condition itself untested, as instructed.	2026-09-12 07:41:57 +02:00
ltms	d25c863118	Merge #531: separate the blocked forge MCP server from the working GITEA_TOKEN ... Charter wording only — 9 insertions, 4 deletions, one file. CI run 1780 on be07ed2: success. Resolves the "charter may be stale" item I had been carrying. It was not stale. Two workers reporting working forge access and the charter saying forge tools hold a blocked credential were both correct, about two different credentials: the repo-scoped GITEA_TOKEN the daemon injects (which opens every worker PR, per implementer SKILL.md step 5) versus the forge MCP server that leaks in from the operator's user-scope config (which is deliberately blocked). The wording did not separate them, and a worker could have read it as "I cannot reach the forge" and skipped opening its PR. Both sentences now name the MCP server specifically and state that the injected token is a separate, working route. Canonical block and wiki template verified byte-identical after the edit — the CLAUDE.md sync script reports "in sync: True". The wiki commit is d02a55d on wiki's own main, pushed and verified by ref (ls-remote matched local HEAD), not by exit code. The submodule pointer stayed unstaged. Not re-measured in this change: that the blocked MCP credential does fail every call. That claim is the existing charter's and I only narrowed what it refers to. It would need its own probe with a request that cannot succeed on its merits, so that a rejection can only mean the block.	2026-09-12 07:41:11 +02:00
Dai Ha	7c34e8f4f9	fleetd #528: pin drain_gate_refusal's call site, not just the predicate ... drain_gate_refusal composes the correct abort message and is well tested, but the main flow built its own `die "$(drain_gate_refusal ...)"` call — nothing proved that call site was ever consulted. Mutating it to a flat `die "aborted -- nothing changed"` left the whole suite green, silently reinstating the exact defect #517 was filed to fix. Same shape as #521/#526's should_swap/swap_if_built: the decision and the die() now live together in refuse_drain_gate, which the main flow calls unconditionally. drain_gate_refusal stays separate and separately tested for the message logic; four new behavioural tests stub die() to prove refuse_drain_gate calls it correctly for all four cases, and a fifth source-text test pins the main flow's call site itself (the only thing that can catch deleting the call, since sourcing stops before the main flow runs).	2026-09-12 12:38:16 +07:00
Dai Ha	be07ed2033	charter: separate the blocked forge MCP server from the working GITEA_TOKEN ... 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) ... Verified in my own worktree, at the pushed head d898571, test file hash 0228f78424boa... (full: 0228f78424boa is a typo; the measured hash is 0228f78424boa). See the acceptance table below for the measured values. mvn clean install in fleetd/: BUILD SUCCESS, Tests run: 1699, Failures: 0, Errors: 0, Skipped: 0. SessionManagerTest itself: 72 tests (69 on main + 2 from #522 + 1 new). CI run 1773 on d898571: success. Mutation killed: reverting CapturedLog.close() to detach the appender only gives "expected: <DEBUG> but was: <WARN>" on the new proving test. Branch is 9 commits behind main but touches one file, and main's 9 commits touch none of it, so this is not a stale-branch merge. Two corrections to the PR body, neither blocking: 1. The body says it converted "all 7" call sites; its own breakdown (5 leaks + 1 with no setLevel + 2 already fixed by #522) sums to 8, and the file has 8 (7 CapturedLog.at + 1 CapturedLog.of). The sweep is complete either way: every raw setLevel and addAppender left in the file is inside CapturedLog itself or the @BeforeAll/@AfterAll baseline pair. 2. The body says #522's two explicit Level.INFO pins stay "as belt-and-braces — a later change to the sweep must not be able to make those two vacuous again." I tested which part is actually load-bearing, running both classes in one fork with -Dsurefire.runOrder=reversealphabetical so WorktreeSessionManagerTest runs first. Removing both INFO pins but keeping the @BeforeAll DEBUG baseline: PASSED, 24 + 72 tests, 0 failures. So the per-test pin really is redundant. Also removing the @BeforeAll DEBUG baseline: mvn exit 1, 3 failures — expected: <DEBUG> but was: <WARN> both released sessions must be counted: no drain-complete INFO logged ==> expected: <true> but was: <false> both the ready and the busy session are released: no drain-complete INFO logged ==> expected: <true> but was: <false> So the @BeforeAll DEBUG baseline, not the per-test INFO pin, is what keeps #522's two drain assertions from going vacuous. Nobody may delete that @BeforeAll as "only there for the proving test" — it protects two other tests. The WARN in that output comes from WorktreeSessionManagerTest:267-272, whose finally only calls detachAppender. That is a proven cross-class leak, out of #525's scope, and a wider ticket follows: 9 files where every setLevel is an unrestored literal pin, 19 pins in total, with a recommendation to share this CapturedLog helper.	2026-09-12 07:20:44 +02:00
ltms	bb6fc9e0d7	Merge #524: FleetMcp's caller resolution is an explicit choice, and tested through the real transport (fleetd #518) ... Adjudicated and verified by me, by my own build and my own mutation. This is the strongest PR of this batch and it closes #518 properly. What it does: `callers == null` used to decide TWO unrelated things at once — whether authorization was enforced, AND which principal-resolution code path ran. Reaching "authorization off" by simply not passing a CallerResolver also silently swapped in a second, separately maintained identity heuristic (`legacyPrincipal`) that nothing exercised. That is the fallback-reached-by-omission shape #415 named. The fix is #415's antidote: `callers` becomes required and non-null, enforcement moves to a required `AuthorizationMode` parameter with no default, and `legacyPrincipal` is deleted outright rather than left testable. Verified by me on the merged revision: * `mvn clean install` BUILD SUCCESS, Tests run: 1697, Failures: 0, Errors: 0, Skipped: 0 * exactly one FleetMcp constructor and four construction sites, all passing the new parameter — so "authorization off" is now a compile error to reach by omission, not a silent default * `legacyPrincipal` is gone: 0 declarations, 0 calls. The four remaining mentions are prose that correctly describes it as deleted * branch has no file overlap with anything main changed since its branch point (b37def9), so this is not a stale-branch merge. The 1697 vs main's 1698 is explained: this branch predates #522's two new tests The mutation that matters, run by me. I reinstated exactly the heuristic this PR deletes — resolve from the connection only, never reading the Authorization header: * `FleetMcpContextExtractorTest` fails by name: `a valid bearer token must resolve as PRIMARY and pass fleet_whoami's READ gate: unauthenticated: anonymous may not READ ==> expected: <false> but was: <true>` * and then the decisive measurement — with that mutation still applied I ran the **whole** suite: Tests run: 1697, **Failures: 1**, and the single failure is the new test class. All 20 FleetMcpAuthzTest cases pass with the resolver bypassed, as do the other 1676 tests. So the PR's central claim is true and measured: nothing in the existing 1696 tests could see this, because none of them go through the transport. `denyFor` had a full policy table, `CallerResolver.resolve` had a full suite, and the closure that wires the two together had nothing. That is the seam-does-not-prove-the-caller shape, and one real end-to-end test on a real Jetty server with a real MCP client is the right answer to it. Mutant proven applied two ways with different strings (mutant marker present = 1, original resolve call absent = 0, with a control showing it present = 1 in a saved copy). Restored byte-identical by hash, tree clean, green control build afterwards. One limit I am recording rather than claiming is covered: `callers` being required stops it being reached by *omission*, which was the defect. An explicit literal `null` is still a thing a caller could write, and the `Objects.requireNonNull(callers, "callers")` that catches it has no test of its own. That is the intended bar, not a gap worth a ticket. The #518 worker's pane and worktree were taken by the idle reaper before I finished verifying, so this was built and mutated in a worktree I created from the pushed head. Nothing was lost — the branch was pushed and clean.	2026-09-12 07:10:21 +02:00
ltms	3366590dbe	Merge #526: pin the jar swap at its call site, not just its predicate (fleetd #521) ... 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) ... 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 ... 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 ... 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 ... 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 ... 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 ... 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) ... 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 ... 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	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) ... 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 ... 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 ... 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)	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)	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)	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 ... 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 ... 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 ... 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)	2026-09-12 05:44:16 +02:00