Adds FleetdAssemblyAuthorizationModeTest: drives FleetMcp#denyFor (via reflection, since it is package-private to dev.ltms.fleet.mcp) on the real FleetMcp FleetdAssembly#assembleAndStart builds, asserting an unauthorized worker is refused SPAWN and the primary is still allowed. Mutating line 481 to UNENFORCED turns this test red and no other test.
Adds a test that reaches the real LeadTabScanner FleetdAssembly's
production boot path builds (via the LeadCoordLoop field that stores
the same leads supplier instance) and asserts, by reflection, that
the excludedWorkspaceLabels field is empty. Mutating line 265 to any
non-empty set now turns this test red.
FleetConfig declares eight public no-arg void validate* methods, at lines
2671, 2708, 2755, 2790, 2827, 2856, 2896 and 2958. The sweep's javadoc still
named a count. The wording is now count-free, so it cannot drift again.
validateAllReachesEveryOneOfTodaysSixValidators only covered six of
the eight real validators; the new validator was reachability-tested
only from FleetConfigTest, in a different file from the one whose job
is to enumerate every validateAll-reachability case.
Add the pane-placement case to the enumeration, rename the method to
drop the hardcoded count (validateAllReachesEveryOneOfTodaysRealValidators),
and correct the surrounding claims to say seven of eight, naming
validateLeadRollover as the one case still missing (fleetd #668, not
fixed here).
LeadLauncher.LeadCount's javadoc carried the same false member-space-
exclusion claim as the three comments fixed earlier in this ticket;
its neighbouring body comment in countLeads was already correct and
is unchanged.
FleetConfigValidateAllTest's canary test is renamed to drop the
number from its name (the count now lives only in the Set.of literal
and the javadoc, so the two cannot drift), which also fixes the
dangling {@link} to the old name and the stale 'seventh' wording.
The old text warned only that a 'mvn clean' deletes the running daemon's jar.
Replacement is enough: the shutdown drain loads its classes lazily, at
shutdown, from the jar file the JVM opened at boot. So any build that writes
fleetd/target/fleetd.jar under a live daemon breaks its drain, nothing warns
at the time, and the damage surfaces at the next restart where it looks like
the restart's fault.
Both files now state the real rule and the positive one: verify a merge by
building in a throwaway git worktree, and let only
scripts/redeploy-fleetd.sh touch the main clone's jar.
The canonical block is untouched (20938 bytes, identical to main) and the
wiki sync check passes against wiki/7-Use-Cases.md.
The 3-arg form delegated to the 4-arg one with a null window, so any caller
reaching for it silently got the fixed HIGH_THRESHOLD_TOKENS back instead of
the profile's effective auto-compact window. It had no production callers.
All 15 test call sites move to the 4-arg form.
Verified at 60fa86a in a throwaway worktree: Tests run: 1923, Failures: 0,
Errors: 0, Skipped: 0, BUILD SUCCESS, 170 surefire report files. Mutating
HIGH_THRESHOLD_TOKENS to 200_000 * 2 turns
noEffectiveWindowFallsBackToTheFixed200000Default red at line 81.
noEffectiveWindowFallsBackToTheFixed200000Default's third assertion
(legacyConfigDir/legacyGauge) used to call the 3-arg read() to prove
it behaved like a null window. With the 3-arg form gone, it is the
same call, same input (200_000) and same expectation as the atGauge
assertion above it, so it cannot fail unless that one already failed,
and its message named a method that no longer exists. Deleted it; the
first two assertions (199_999 -> OK, 200_000 -> HIGH) are unchanged.
A pane-placed member lands inside the focused tab rather than its own,
so it can land inside a lead's labelled tab and be read back as that
lead by LeadTabScanner, which does not exclude the member space in
production. Add FleetConfig.validatePanePlacementAgainstLeadTabs(),
wired automatically into validateAll() by the existing reflective
sweep, to refuse that combination at startup.
Also correct three stale comments that claimed a member-space
exclusion already blocked this path, in LeadTabScanner, FleetConfig's
validateLeadTabPrefixes javadoc, and LeadTabScannerTest.
The 3-arg read(configDir, sessionId, agentType) delegated to the 4-arg
form with a null window, silently restoring the fixed 200_000 fallback
that #637 moved away from. It had zero production callers; both
production call sites already use the 4-arg form.
Migrate all test call sites to the 4-arg form. For the generic property
tests in LeadContextGaugeTest, the window is irrelevant and null is
filler. In LeadContextGaugeHighThresholdTest's
noEffectiveWindowFallsBackToTheFixed200000Default, null is the
meaningful value under test, not filler; the assertion is unchanged.
FleetMcp.LeadConfigDirSource's 1-arg constructor, Fleetd.leadContextLookup's
4-arg overload, and Fleetd.leadContextSource's 4-arg overload each existed only
to keep old call sites compiling, and each silently resolved no auto-compact
window — reverting any caller that picked one up to LeadContextGauge's fixed
200,000 HIGH threshold, the exact defect #637 fixed. Removed all three and
updated the 8 call sites across 4 test files to pass the window lookup
explicitly.
Added FleetdLeadContextSourceWindowAssemblyTest: no existing test called the
real assembled LeadHeartbeatLoop far enough to prove FleetdAssembly's
window-lookup argument into Fleetd.leadContextSource actually reaches the
gauge. Mutating that argument to `_ -> null` compiled clean and left the whole
suite green; the new test fails against that mutation. Needed a small FakeHerdr
addition (agentSessionId(..)) since its default agent.get response carries no
session id.
Fleetd.leadConfigDirSource built its window argument with nothing calling the
factory itself to prove it, so a mutation to a no-op lookup left the whole
suite green. Add a wiring test that calls the factory directly, modeled on
FleetdLeadConfigDirSourceWiringTest's shape for the configDir half of the
same factory.
LeadContextGauge's result cache keyed only on (configDir, sessionId), but the
cached Reading's state now depends on the caller's resolved effective window.
Fold the derived threshold into the cache key so two reads of the same
session with different windows inside the TTL each report against their own
window.
HIGH_THRESHOLD_TOKENS was a hardcoded 200_000, while the event it warns about
(auto-compaction) is configured per profile via autoCompactWindow and can legally go
as low as 100_000 — making HIGH unreachable before a compaction on such a profile.
LeadContextGauge.read now takes an optional effective window and fires HIGH at 2/3 of
it, falling back to the fixed 200_000 when no window is resolvable (unresolved callers,
including the pre-existing 3-arg read(), keep today's behaviour exactly).
FleetConfig.Profile.effectiveAutoCompactWindow() resolves that window the way a
launched Claude Code session actually reads it: env.CLAUDE_CODE_AUTO_COMPACT_WINDOW
wins over the autoCompactWindow launch flag when both are set.
Wired into both real consumers: fleet_list's context row (FleetMcp.LeadConfigDirSource,
widened with a back-compat constructor so no unrelated call site changes) and the lead
heartbeat's context-high nudge (Fleetd.leadContextLookup/leadContextSource, widened the
same way).
Criterion 19 covers a block-scalar body whose key line falls outside
diff -u's default 3-line context (an 8-line body with only the 6th
line changed). Criterion 20 covers a blank line inside the value,
which used to reset the old indentation-anchored mask.
Both are RED against the pre-#639 redact() (git show 28ea0de) and
GREEN against the current one; each asserts both the secret's
absence and a non-secret control line's presence.
Add a positive anchor per watched file (Fleetd.java and FleetdAssembly.java),
matching FleetdConfigRefWiringTest's [SOURCE TEXT] style, so a broken read
fails loudly instead of passing the negative check vacuously. Fix dangling
javadoc @link references in FleetdConfigRefWiringTest to the *AssemblyTest
names those classes were renamed to.
FleetdAssemblyFleetAppTest's class javadoc stated that /sessions'
Authz.Action.READ gate is always refused, and that READ always needs
Caller.resolved(). That is true only under auth.mode: loopback-trust,
the mode this test runs under because it configures no auth: block.
Under auth.mode: token, CallerResolver.resolve() returns before ever
consulting Caller.resolved()/scanComplete(), so a valid bearer token
resolves to PRIMARY with no pid lookup on that path. Names the two
tests that already exercise that path against a real assembly.
Test-only, 243 lines, one new file. ZERO production change — this is the point
of the PR, and it replaces PR #647, which added three public accessors to
FleetMcp on a false premise.
#647 argued a same-JVM test caller can never reach fleet_list's READ gate,
because FleetdAssembly hardcodes a real LsofPeerPidLookup, so the caller
resolves ANONYMOUS. The premise about lsof is true; the conclusion is not. In
CallerResolver.resolve, once c.terminal() == null the next branch is:
if (tokenMode) {
return presentedTokenMatches(authorizationHeader)
? Principal.primary(c.pid()) : Principal.anonymous();
}
c.resolved() and c.scanComplete() guard only the LATER loopback-trust branch,
which token mode returns before reaching. So under auth.mode: token a bearer
token resolves to PRIMARY with no pid lookup on the path. This PR does exactly
that: real McpSyncClient callTool("fleet_list") over a real transport with an
Authorization: Bearer header, a faked leadMailboxOpener so no broker is touched,
and a real coordinator: block with a non-empty peers list. No reflection.
Lead verification, measured myself (not taken from the worker's report):
git diff --stat origin/main -- fleetd/src/main/java/ -> EMPTY
mutate :481 Fleetd.capacitySource(...) -> CapacitySource.none()
-> Tests run: 3, Failures: 1
fleetListReportsTheAssembledCapacitySource:222
the other two tests stayed GREEN
The failure message carries the live fleet_list JSON body, showing
healthCoverage, loopHealth and coordinator.peers present with capacity absent —
so the round-trip, the PRIMARY resolution and the coordinator visibility are all
real, and the mutation removed exactly one thing.
Worker also reported, each with a grep -c anchor of 1 restored: health mutation
-> 1 failure named fleetListReportsTheAssembledHealthCoverageSource; peers
mutation -> 1 failure named fleetListReportsTheAssembledCoordinatorPeers; final
mvn clean install Tests run: 1899, Failures: 0 / BUILD SUCCESS. I reproduced the
capacity cycle only; the other two are the worker's measurement, not mine.
Carries forward #647's genuine find: the peers ternary at :489 is
inert-equals-absent, so the test configures a real coordinator: block with peers.
Detail on #612 and #647.
Test-only, 368 lines, one new file. No production change.
Lead verification, measured myself in a throwaway detached worktree (not taken
from the worker's report):
starve FleetApp pass site :529 -> Tests run: 4, Failures: 2
(...ByTheRealAssembledFleetApp:333, :363)
both ...FleetMcp tests stayed GREEN
starve FleetMcp pass sites :484/:486 -> Tests run: 4, Failures: 2
(...ByTheRealAssembledFleetMcp:319, :349)
both ...FleetApp tests stayed GREEN
So the two windows are pinned independently. Failure messages carry the live
JSON response body from a real round-trip, not a source-text match.
Why this is worth merging — the REST window was completely unpinned. With this
PR's test parked and :529 starved, the FULL suite reported:
Tests run: 1892, Failures: 0, Errors: 0, Skipped: 0 / BUILD SUCCESS
Zero pre-existing tests notice the REST window losing its sources. An operator
reads GET /profiles exactly when the MCP mount is down. Arithmetic control:
1892 + this PR's 4 = 1896 = main at 6539efe.
Known caveat, recorded not fixed: the MCP-side quarantine assertion (:313) is
DUPLICATE coverage. A reviewer measured on clean origin/main that starving the
MCP quarantine arg already fails three pre-existing tests
(FleetdBackendQuarantineAssemblyTest:167, FleetdExhaustedPatternAssemblyTest:202,
FleetdOpenCodeExhaustionForwardingAssemblyTest:182), so that site was already
pinned and the PR's claim otherwise is wrong. Low severity, left in place as a
valid fleet_profiles output assertion. Detail on #648 and #612.
Reviewed by two reviewers against the diff (soundness: no issue; coupling: the
duplicate-coverage finding above).
FleetdAssembly.java's quarantineSource (:471-472) and outageSource (:473-476)
each feed two consumers: FleetMcp (fleet_profiles, :484/:486) and FleetApp
(GET /profiles, :529). No existing test distinguished the two windows for
either source.
New test drives the real FleetdAssembly.assembleAndStart, classifies a real
exhaustion/outage through the real CompletionResolver, and reads the result
back through a real McpSyncClient (fleet_profiles) and a real HttpClient
(GET /profiles), both authenticated via token-mode auth (sidesteps the
in-JVM pid-resolution dead end). No source text is read; no production code
changed.
Verified with six mutation cycles (3 per site x 2 sites: FleetMcp starved,
FleetApp starved, mis-wire with a disconnected collaborator), each run
against the full unfiltered suite and reverted after confirming the
expected test(s) alone went red. The quarantineSource FleetMcp-starve and
mis-wire cycles also trip three pre-existing tests that read the live
BackendQuarantine via FleetMcp#quarantineSource() for their own unrelated
assertions - a pre-existing incidental coupling, not newly introduced here.
Out of scope, noted per the ticket's dispatch comment: loopHealthSource
(FleetdAssembly.java:478) shares this same two-consumer shape and is
already assigned to a separate unit, r10.
Pins FleetdAssembly's loopHealth local at both of its pass sites: FleetMcp (:483, the fleet_list source) and FleetApp (:529, the real /healthz body). Two independent assertions, so starving one site leaves the other green.
Lead verification, independent of the implementer's own proof, in a throwaway detached worktree at 68397f7:
- :483 starved only -> Tests run: 2, Failures: 1. RED: fleetListUsesTheRunningLoopsInTheRealAssembledMcp, expected: <RUNNING> but was: <STOPPED>. REST test stayed GREEN.
- :529 starved only -> Tests run: 2, Failures: 1. RED: healthzUsesTheRunningLoopsInTheRealAssembledApp, with the real body {"loopHealth":{"sessionReaper":"STOPPED","statusPoller":"STOPPED"}}. MCP test stayed GREEN.
- full suite with the PR: Tests run: 1896, Failures: 0, Errors: 0 — BUILD SUCCESS (1894 baseline + 2).
The REST half binds port 0 (ephemeral) via runtime.app().start("127.0.0.1", 0), not the configured 8765, so it cannot clash with the live daemon. Teardown stops the bound app, closes the runtime, and asserts the shutdown hook was registered and the herdr client actually closed.
I chased the implementer's honestly-reported anomaly (an unrelated ClaudeCodeLauncherTest.noFixtureSeededTheDefaultClaudeJson failure in one cycle, and a suite total that moved between cycles). It is NOT caused by this PR: a baseline run at 68397f7 with this PR absent added the same 3 temp-dir project entries to the real ~/.claude.json as the run with it applied (65->68 without, 68->71 with), and ClaudeCodeLauncherTest passed in both. Filed separately.
Test-only diff, no production code touched.
Pins FleetdAssembly.java:349 behaviourally. The test installs a throwing wrapper on the real assembled Injector's listener to construct the narrow between-delivery-and-completion window, then asserts runtime.completion() still resolves the registered waiter. No sleep: it advances a controllable nano clock. Teardown runs the captured shutdown hook and asserts herdr actually closed.
Lead verification, independent of the implementer's own proof, in a throwaway detached worktree at dac5f88:
- unmutated: Tests run: 1, Failures: 0 — BUILD SUCCESS
- :349 registrar -> (_, _) -> { }: Failures: 1 — "must wire the Injector registrar to this runtime's real CompletionResolver" expected: <true> but was: <false>
- mis-wire I built myself (differs from the implementer's): Fleetd.turnRegistrar(new CompletionResolver(agents, new Rendezvous(), ...)) — a fresh Rendezvous so the registry genuinely differs: Failures: 1, same assertion
- reverted, full suite: Tests run: 1894, Failures: 0, Errors: 0 — BUILD SUCCESS, 58s (1892 baseline + r5 + r12)
Test-only diff, no production code touched. Reflection is used only to install the throwing listener; that is how the failure window is constructed, not how the assertion is made. No reviewer fan-out: member capacity is committed to the remaining Shape A implementers.
Pins the #602/#606 call site behaviourally: drives the real FleetdAssembly.assembleAndStart and asserts the assembled LeadConfigDirSource resolves a real configured configDir, which none() cannot produce.
Lead verification, run independently of the implementer's own proof, in a throwaway detached worktree at 141ae3b:
- unmutated: Tests run: 1, Failures: 0 — BUILD SUCCESS
- FleetdAssembly.java:488 -> FleetMcp.LeadConfigDirSource.none(): Tests run: 1, Failures: 1 — expected: </mnt/fake-lead-configdir> but was: <null>
- reverted, full suite: Tests run: 1893, Failures: 0, Errors: 0 — BUILD SUCCESS, 59s (baseline 1892)
Test-only diff, no production code touched. No reviewer fan-out was run: all member capacity is committed to the five Shape A implementers.
FleetdLeadConfigDirSourceWiringTest already pins Fleetd.leadConfigDirSource
itself, but by its own javadoc cannot cover whether FleetdAssembly.java:488
still calls it -- that call site could be swapped for a bare
FleetMcp.LeadConfigDirSource.none() (the literal fleetd #602/#606 defect)
and the whole suite would stay green.
Add FleetdLeadConfigDirSourceAssemblyTest: assembles the real FleetdRuntime
via FleetdAssembly.assembleAndStart, reads the leadConfigDirs field off the
real FleetMcp via reflection (no public accessor exists), and asserts it
resolves a configured lead's real configDir rather than none()'s hardcoded
null.
Verified: loud control (flip expected value) goes RED, reverts green;
mutation (i) inert none() at the call site goes RED; mutation (ii) mis-wire
(empty profile map, symbols otherwise intact) goes RED; both mutations
revert to an empty git diff. Full mvn clean install: 1893 tests, 0
failures, 0 errors, BUILD SUCCESS.
Comment text only, no behaviour change. Corrects the paragraph added in d7f94ca so it no longer
claims the continuation masking holds for any masked key "present or future". It holds only while
the masked key's own line is inside the printed hunk; diff -u's three lines of context routinely
leave it out, and a blank line inside a block scalar drops the anchor too.
Verified: suite green in a clean copy of the edited tree (16 criteria + 3 extras, exit 0), bash -n
clean, and a control on the edit itself (old claim gone, #639 reference present). Evidence in #639.
The paragraph added in d7f94ca ended with "This needs no knowledge of the key's
name and so protects a block scalar under any masked key, present or future."
The continuation masking is real and it is an improvement, but that sentence is
too strong: the masking only holds while the masked key's own line is inside the
hunk being printed.
redact() is fed `diff -u` output, which prints three lines of context. A block
scalar's body therefore often arrives with its key line left out. With no key
line, `masked` is never set and the body prints in full, with no "<redacted>"
anywhere. A blank line inside a block scalar loses the anchor the same way: a
blank diff line measures as indent 0, so `indent > masked_indent` is false and
the mask ends early — this time directly under a "<redacted>" marker.
Both were reproduced through the real script with --dry-run, each with a
positive control run first to prove the secret's lines actually reached the
output (without that control, "the secret never entered the diff" and "it
entered and was redacted" are indistinguishable). Filed as fleetd #639, which
also records that this is latent rather than live: today's fleetd.yaml holds 5
block scalars and all 5 sit under non-secret keys.
Comment text only. No change to redact() or to any other function, and no
change to the test suite. scripts/test-config-edit.sh still passes in a clean
copy (16 criteria + 3 extras, exit 0); bash -n clean.
The reason this is worth its own commit: #635 exists because an incomplete
redactor that looks complete is worse than one that visibly does nothing. A
comment that overstates the guarantee is the same defect in prose, and the next
session to read it has no other source.
Closes fleetd #635.
Verified by the lead before merge:
- scripts/test-config-edit.sh run in a PRISTINE copy of d7f94ca (git archive + git init, no
worktree, no daemon): 16 criteria + 3 extras, exit 0.
- Diff read in full. 4 files, 1325 additions, 0 deletions, no .java/pom.xml/.yaml (checked with a
positive control, so the negative is real) — no Maven gate needed.
- Defects 1-6 were verified by the previous lead by mutation; defects 7 and 8 (criteria 15a/15b/16)
are fixed in d7f94ca and the fix was re-measured here independently.
Known limitation, filed as #639 and NOT a regression: redact()'s continuation masking only holds
while the masked key line is itself inside the printed diff hunk. diff -u prints three lines of
context, so a block scalar's body can appear without its key, and then nothing is masked; a blank
line inside a block scalar loses the anchor the same way. Reproduced through the real script with
a positive control proving the body reached the output. Latent, not live: today's fleetd.yaml has
5 block scalars and all 5 sit under non-secret keys. The pre-fix code leaked these cases too, so
this commit is a strict improvement.
Two sentences in config-edit.sh's redact() comment and in this PR's body claim the masking holds
regardless of the key's name. That is too strong; see #639. Being corrected in a follow-up.
Defect 7 (comment 17670): redact() only masked a line that itself started with a
secret-looking key, so a YAML block scalar's value leaked on the lines that
followed the key while the key line right above it printed a reassuring
"<redacted>". Fixed by tracking the masked key's own indentation and masking
every following line indented deeper than it, stopping once indentation returns
to the key's level or shallower; the diff's leading +/-/space marker is stripped
before indentation is measured, per the comment's own pitfall. "passphrase" is
now also in the key-name backstop.
Defect 8 (comment 17673): apply_set_pairs echoed the operator's full
"path=value" input, unredacted, in both of its yq-failure die messages — a
failing --set with a secret-looking value printed that value right back. Fixed
to print only the path; deliberately not routed through redact, which would
pass a non-"key: value"-shaped string straight through.
Adds acceptance criteria 15a (block-scalar continuation), 15b (passphrase key),
and 16 (failing --set never echoes its value) to scripts/test-config-edit.sh,
each with a positive control proving the relevant line really was in the
printed output before asserting the secret is absent. All three confirmed RED
against the pre-fix code and GREEN after, in isolation, before being folded
into the full suite (16 criteria + 3 extras, exit 0).
Also updates PR #636's description per comment 17671: the redact() sentence now
names the continuation-masking rule and says plainly that the key-name list is
a backstop, never a complete list.
The --restore "no backup found" message still printed the old beside-the-config glob
(${CONFIG}.bak.*) even though newest_backup had already moved to searching the managed
.config-backups/ directory. The message was left behind when the search moved — the search
itself was already correct (ticket comment 17664). Fix is reporting-only: the message now
names the directory actually searched (via backup_dir_for), and separately says that a
backup written the old way, directly beside the config, is not searched any more, with the
one-line cp to recover one by hand. No search fallback was added — reading backups from
outside the managed directory stays unsupported, as instructed.
Acceptance criterion 14 proves both directions: the not-found message names the real
directory (confirmed red on the pre-fix code, green after), and a restore with a real backup
present in .config-backups/ still succeeds (confirmed this catches an "always not-found"
regression that direction 1 alone would miss).
All 14 criteria plus 3 extras pass in scripts/test-config-edit.sh.
Five fixes against PR #636, all verified by the lead's own review and reproduced here:
1. --set .a.b= (a forgotten value) is now refused outright instead of silently nulling the
field — a null numeric config value falls back to its default rather than erroring, which
widens capacity silently instead of failing loudly. A deliberate clear gets its own spelling,
--set .a.b=null, which writes a literal YAML null via yq, never through strenv(). (criteria
9, 10)
2. Backups move from beside fleetd.yaml to a dedicated fleetd/.config-backups/ directory,
gitignored at the repo root (so it also covers scripts/test-config-edit.sh's own throwaway
fixtures) and in fleetd/.gitignore, plus a fleetd.yaml.bak.* glob backstop for any stray
backup written the old way. A backup of a file that must never be committed inherits that
requirement. (criterion 11)
3. The live config's file mode now survives both an edit and a restore. mv from a mktemp
candidate used to carry mktemp's 0600 onto the live path forever, and cp onto an existing
file keeps the destination's mode, so a restore did not undo it either. (criterion 12)
4. A global CAND + single EXIT/INT/TERM trap prevents an uninstalled .config-edit.XXXXXX
candidate from leaking if the script is interrupted mid-run. No acceptance criterion is
gated on this — a reproducible leak could not be made to happen on demand — but it is cheap
and obviously right.
5. Acceptance criterion 7's redaction check gained a positive control: it now asserts the
output actually CONTAINS the redaction marker and the changed key, not only that it lacks
the secret. The prior two assertions were negative-only and passed just as happily when the
diff was never printed at all — confirmed by reproducing the lead's own mutation (deleting
the redacted diff print on the edit path) and watching it survive the old test and get
caught by the new one. (criterion 13)
All 13 acceptance criteria plus 3 extras pass in scripts/test-config-edit.sh. Criteria 9, 10,
11, 12 and 13 were each proven non-vacuous: criteria 9/10 by mutating the test's own expected
value and watching it fail by name, then reverting; criteria 11/12/13 by reverting or mutating
the corresponding fix in config-edit.sh and watching the matching criterion fail by name, then
restoring the fix and re-confirming a clean pass.
Backs up, builds a candidate off the live file, parse-checks it with yq before
install, installs atomically, then reads the daemon's own ConfigRef reload
verdict back out of fleetd.out (marked from before the edit, so a stale line
can never be mistaken for this edit's result). Four exit codes: 0 clean, 3
needs a restart, 4 refused (backup restored), 5 cannot tell (nothing
restored, printed --restore command). Every diff is redacted.
scripts/test-config-edit.sh drives it end to end against fixtures in a
throwaway temp dir, with no daemon involved.
Pins three call sites the assembly owns and nothing observed:
- healthFailTarget (FleetdAssembly:429) — inert, a dead member's waiting ticket sits
PENDING for the full 30-minute async timeout instead of failing immediately.
- releaseCleanup (:447) — inert, every teardown leaks three things: a stuck rendezvous
waiter, an unreleased reply-inbox consumer, and a stale lead binding.
- requireOperatorConfirm (:402/:409, fleetd #630) — dropping the 14th constructor
argument selects #621's 13-arg overload, which hardcodes true, silently reverting
the operator's fix on a host that set requireOperatorConfirm: false. Pinned in both
directions, plus an assertion that the two notice strings differ, so no constant
satisfies both.
Verified by the lead beyond the worker's proof: its releaseCleanup mutation killed all
three cleanups at once and so proved only the first assertion had teeth. Starving them
one at a time — abandon kept, release starved; then abandon and release kept, forget
starved — each fails its own named assertion. All three leaks are pinned independently.
A review pass found these three tests assembled real schedulers and never tore them
down, by any route: no close(), no shutdownHook, no @AfterEach, no finally. Surefire
runs one JVM fork for the whole suite, so those loops outlived their tests. Fixed by
capturing the hook and running it in a finally, with an assertion on FakeHerdr.closed
so the teardown itself is pinned rather than assumed.
MERGE RESOLUTION BY THE LEAD, the same collision as #633. These 3 test files each add
a ResourcePorts fake, and #633 landed first making herdrPollWait() abstract with no
default. Git reported a clean merge that did not compile. Added the override to all 3,
matching the established convention for an always-healthy fake — a Runnable that
throws, verified first that none of the three uses healthy(false), so the tripwire can
only fire if the test's herdr behaviour changes.
Full suite on the resolved merge: 1892 tests, 0 failures (1889 + this branch's 3).
#629: the herdr boot wait went through ports.nanoClock() but hardcoded
Fleetd::sleepHerdrPoll, so a test assembling against an unhealthy lead herdr burned
30 real seconds whatever clock it injected. The poll sleep now goes through
ResourcePorts.herdrPollWait(). FleetdAssemblyFleetAppTest's lead-down test drops from
30.276s to 0.062s. It also gains @Timeout(10, SEPARATE_THREAD) at class level: the
pin's failure mode is otherwise an infinite hang, because the test's fake clock only
advances when herdrPollWait() is called. SAME_THREAD cannot interrupt a real
Thread.sleep, so the thread mode is load-bearing, not decoration.
#625: guard.assertPrimaryClean(System.getenv()) could be deleted with a fully green
suite — the check behind charter invariant 1, which keeps the primary on the
operator's subscription. main() now delegates to main(String[], ResourcePorts) and
the guard reads ports.environment(), so a test can taint the environment without
touching the real process env. The guard still runs before cfg.validateAll() and
before any socket, broker or HTTP work; two tests with different fixtures pin that
ordering as two independently falsifiable claims, not one.
MERGE RESOLUTION BY THE LEAD. ResourcePorts.herdrPollWait() is abstract with no
default, by design (#629 keeps ResourcePorts free of a none() default). This branch
patched the 8 implementations that existed when it forked. PRs #631 and #634 merged
ahead of it and added 3 more fakes, so git reported a clean 14-file merge that did
not compile:
FleetdAssemblyAmqpOpenersTest.RecordingPorts is not abstract and does not
override abstract method herdrPollWait() in dev.ltms.fleet.ResourcePorts
(plus the two ControllableResourcePorts in #634's tests). I added the override to
those 3, matching this branch's own convention for an always-healthy fake: return a
Runnable that throws, so if one of those assemblies ever does start polling herdr it
fails loudly instead of sleeping quietly. All 13 implementations now carry it.
Full suite on the resolved merge: 1889 tests, 0 failures (1886 + this branch's 3).
Each of FleetdAssemblyHealthFailTargetBehaviouralTest,
FleetdAssemblyReleaseCleanupBehaviouralTest and
FleetdAssemblyRequireOperatorConfirmBehaviouralTest called
FleetdAssembly.assembleAndStart without ever tearing it down: no close(),
no shutdownHook, no @AfterEach, no finally. Surefire runs the whole suite
in one JVM fork, so every scheduler/loop these tests started kept running
for the rest of the suite.
Capture the shutdown hook in each test's fake ResourcePorts (the existing
pattern from FleetdAssemblyLifecycleTest et al.) and run it in a finally
block, on the failure path too. RequireOperatorConfirmBehaviouralTest
assembles twice in one method, so assembleHeartbeat now returns both the
loop and its ResourcePorts so each assembly gets its own teardown.
Proof the teardown actually runs: assert ports.herdr.closed after running
the hook (FakeHerdr.close() only flips that flag from inside the real
close chain). Verified the assertion is load-bearing by temporarily
removing one shutdownHook.run() call and confirming the test then fails.
No assertion, test name or reflection changed. Diff is test-only.
Pins all four exhaustion call sites in FleetdAssembly: liveExhaustedPatterns (:299),
exhaustedPatternLookup (:300), publishExhaustionSink (:327) and the independent
OpenCode forwardingExhaustionSink (:164). Rank 1 is the worst consequence in the
#612 sweep — an inert lookup hands a genuine usage-limit refusal back to a waiting
caller as real completed work instead of BACKEND_EXHAUSTED.
Two separate tests, so rank 2's OpenCode half is pinned independently: mutating
:164 fails only the forwarding test, which is the independence the ticket asserts.
Verified by the lead beyond the worker's own proof: wiring publishExhaustionSink to
a throwaway BackendQuarantine — every symbol kept at the call site, only the
collaborator identity changed — is caught by both tests. So these pins survive
mis-wiring, not just deletion.
Test-only; no production change. Tears down each assembly via the captured
shutdown hook in a finally.
Dai Ha
Dai Ha
ltms