worker/fleetd-252-a830e0-3
549 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Dai Ha
|
eb568ff451 |
fleetd #252: guard test for the REST route inventory
FleetApp's route list has never been checked against anything and has already drifted once (GET /member-credentials shipped hours before the #252 ticket and was missing from its list). Add RestRouteInventoryTest, modelled on McpContractDocTest, which scrapes FleetApp.java's app.<verb>("path") calls with a regex and compares them against an explicit expected inventory, failing loudly with the added/removed routes when they diverge. |
||
|
Dai Ha
|
ac790e4cce |
#258: stop two test fixtures writing the operator's real ~/.claude.json
seedTrustDialog targets ~/.claude.json when a profile sets no configDir. Two IDE-overlay fixtures built a worktree-shaped @TempDir, which opens the #149 isProvisionedWorktree gate, and left configDir null — so every test run added two project entries to the operator's real file. 116 had accumulated, none of them still existing on disk, and 32 of those came from current code. The #149 gate only closed the opposite case: a fixture with cwd unset falling back to user.dir. A fixture that builds a worktree on purpose walks straight through it. - ideProfile/ideProfileModule now take configDir first and mandatory, so each fixture states where the trust seed goes. - noFixtureSeededTheDefaultClaudeJson snapshots the temp-dir project keys in @BeforeAll and fails in @AfterAll on any key this class added. Differential, not absolute: an absolute check would fail on every host still carrying the historical entries, and such a check gets deleted rather than fixed. Not done: a blanket -Duser.home redirect in surefire. EnvAllowListScrubTest tests the credential scrub against the operator's real login chain and guards with assumeTrue($HOME/.zshrc exists), so the redirect would silently skip two security tests. Proof: with the bug put back on one fixture the guard fails and names the path; reverted and confirmed identical with diff -q. A full suite run under a fake home now creates no .claude.json at all. mvn clean install: 0 compile errors, 1255 tests, BUILD SUCCESS. |
||
|
Dai Ha
|
6b5f3f472f |
Merge #111: the credential probe reads the policy instead of copying it
scripts/probe-member-credentials.sh carried its own NAMES array of 31 names. The live policy has 34. The probe reported 26 blocked against a policy that blocks 29, exited 0, and printed a table that looked complete. A verification tool that under-reports is worse than none, because its clean output stops anyone looking. Same defect as #114, fixed the same way: DELETE the second copy rather than correct it. The NAMES array is gone, not updated. The daemon now serves GET /member-credentials — names and counts, never a value; MemberCredentialPolicyView reads no environment at all, so there is nothing to redact by construction. The probe fetches it and refuses with a non-zero exit when the daemon is unreachable, the policy is absent or empty, or knownCount disagrees with the length of known[]. No local fallback: a verification tool must not quietly degrade into a weaker check. The startup log line and the endpoint now share that one class, so the counting exists once. That also protects a subtlety I measured before briefing this: blocked is NOT known - allowed. Live, known=34 and allow=7, but only 5 of those 7 appear in known, so blocked=29 and the naive subtraction gives 27. The view reuses creds.blockedSet(), the existing derivation, so it keeps 29. Worker's mutation: MemberCredentialPolicyView.of(...) forced to return ABSENT turned 4 tests red with 0 compile errors — including MemberCredentialsGapReportTest, which proves the startup log really does run through this path. Reverted and confirmed with diff -q. It also caught a bug in its own first draft: jq's // operator treats false and 0 as missing, so `.present // empty` turned a genuine "present": false into "unknown". Fixed by reading the fields directly. NOT yet verified: acceptance criterion 5, the live 34/29/5 run. The route does not exist until the daemon is redeployed onto this jar, so that check comes next and is mine, not the worker's. Merged clean, then built on the merged tree: 1255 tests, 0 failures, 0 compile errors. |
||
|
Dai Ha
|
38dec72152 |
Merge #155: refuse the spawn when allow-list policy cannot be enforced
policy=allow-list is enforced by a ZDOTDIR scrub, and a non-zsh login
shell ignores ZDOTDIR entirely, so no scrub runs. The launcher already
DETECTED this and logged a WARN — then degraded to the weaker overlay
and spawned anyway. The operator asked for the blocking control and
silently got the weaker one, which is the defect the ticket is about.
Detection existed; refusal did not. Under policy=allow-list a non-zsh
shell now throws IllegalArgumentException before any ZDOTDIR or env
work, naming the actual shell and giving three ways out. Under
policy=deny-by-default nothing changes: that overlay is applied to the
pane before any shell runs, so it does not depend on the shell.
Checked against the live config myself, because this refuses spawns and
no worker can see fleetd.yaml:
memberHerdrSocket : NOT set -> the shell comes from fleetd's own
$SHELL, not the unset memberLoginShell
policy : allow-list
fleetd's $SHELL : zsh, proven by behaviour rather than by reading
the process env — the daemon log shows the ZDOTDIR
scrub generating a directory 147 times, most
recently minutes ago, and that only happens when
isZshShell() returned true
So the new refusal cannot fire on this host. Had memberHerdrSocket been
set, the unset memberLoginShell would have read as "<unset>", non-zsh,
and refused every spawn — worth knowing before anyone sets that key.
Worker's mutation evidence, re-stated: `if (!zsh)` -> `if (false)` turned
the refusal test RED with 0 compile errors, then reverted clean.
NOT verified: a live non-zsh member spawn. Forcing it means changing the
daemon's own environment, and the value of the test does not justify
that. The unit tests drive the real launcher.spawn entry point.
Merged clean, then built on the merged tree: 1250 tests, 0 failures,
0 compile errors.
|
||
|
Dai Ha
|
0e8bfb74fc |
Merge #176 stage 2: group subscription profiles by account, not by name
Stage 1 shipped INERT on this host and every test was green. The matcher compared effectiveCredentialId(), which fell back to the profile's own NAME when credentialId was unset. This host runs the lead on `opus` and members on `sonnet`; both are subscription:true with no credentialId, so it compared "opus" against "sonnet", never matched, and charged 0 seats. Every stage-1 test put the lead on the SAME profile name as the target, so the fixture encoded the one shape the live config does not have. Stage 2 returns a "<subscription>" sentinel when credentialId is unset and subscription is true. An explicit credentialId still wins, so an operator with two genuinely separate Claude logins can keep them apart. Verified by me on the live config shape, not by reasoning: opus.effectiveCredentialId() = <subscription> sonnet.effectiveCredentialId() = <subscription> seats charged to sonnet = 1 (was 0 before this change) Only opus and sonnet join the sentinel group on this host; local, local-direct, gx, xf, sol and terra are unaffected. free is clamped with Math.max(0, ...), so the subtraction cannot report a negative. Second, wider consequence, flagged by the worker and confirmed here: CompositePeerLauncher.credentialIdFor feeds enforceNotQuarantined and enforceNotCoolingOff, so quarantining one subscription profile now also refuses spawns on the other. That is correct — one Claude subscription hitting a usage limit really does take out every profile on it — but it is a behavioural change beyond fleet_list's numbers. Checked all 5 logical callers of effectiveCredentialId(); every one wants "this account", none wants "this exact profile". Merged clean, then built: 1250 tests, 0 failures, 0 compile errors. An auto-merge with no conflicts is not a compiling merge, so the build was run on the merged tree before this landed. |
||
|
Dai Ha
|
51f7b0a3ca |
fleetd #111: probe reads the live memberCredentials policy, no hardcoded name list
scripts/probe-member-credentials.sh carried its own hand-maintained NAMES array (31 names, recorded 2026-08-16), so a name added later to fleetd.yaml's memberCredentials.known was never checked and the probe still exited 0 with a clean-looking table. Same drift shape as #114's tool catalogue. - New dev.ltms.fleet.member.MemberCredentialPolicyView: the single place that turns a MemberCredentials policy into names + counts (never a value). Reused by Fleetd.reportMemberCredentialsGap (startup log line) and by the new GET /member-credentials REST endpoint (FleetApp), so the two can no longer drift apart the way the probe and the policy did. - FleetApp gains one route + handler + a Supplier<MemberCredentialPolicyView> constructor param (legacy constructors default to ::absent, so existing call sites are unaffected). - probe-member-credentials.sh now fetches its name list from GET /member-credentials instead of carrying one. No local fallback: an unreachable daemon, an empty/absent policy, or a knownCount/known[] length mismatch all refuse with a non-zero exit rather than silently checking zero names. Prints "policy contains N; this run checked N" so the two numbers are visibly equal. |
||
|
Dai Ha
|
21c539f22e |
#113: derive the config guard from the record tree, both directions
The existing guard walks KNOWN_TOP_LEVEL_KEYS and anchors its regex at column 0, so it sees only top-level keys. Every nested key was outside its scope and nothing said so, which is the shape #113 collects: a checker narrower than it looks, whose green run stops anyone looking. Two derived guards replace the assumption: everyNestedConfigKeyIsDocumentedInTheExample walks FleetConfig's record components (17 records, 83 distinct key names) and requires each to be documented in the example. everyLiveKeyInTheExampleBindsToARecordComponent resolves every live key path in the example against the record tree, so a documented key that binds to nothing fails here instead of being silently ignored in production. Neither carries a list, so a key added to any nested record is covered the moment it compiles (criterion 2). Both mutations run through the real caller, not the helper (criterion 1, which asks for exactly that): removed every mention of paneProbeIntervalSeconds from the example -> FAILS, naming health.paneProbeIntervalSeconds added a live bind.totallyMadeUpKnob to the example -> FAILS, naming bind.totallyMadeUpKnob 0 compile errors in both; both reverted and confirmed with diff -q. The first attempt at mutation 1 removed only the `key:` line and the run stayed green — correctly, because the key was still documented in prose. An incomplete mutation proves nothing, so it was redone. Denominators (criterion 3): both guards print how many keys they checked, and the floor for "did the walk descend?" is derived from KNOWN_TOP_LEVEL_KEYS.size() rather than being a literal. Scope is stated in the javadoc rather than implied: the guards do not check a key sits at the right path, do not parse commented prose for the reverse direction, and do not prove a parsed key is read by anything. paneProbeIntervalSeconds is parsed and read by nothing, and these guards pass it -- the example already says so in its own text. everyOptionalKnobDocumentedInTheExampleBinds keeps its hand-written list but is re-documented as a value-binding spot check, explicitly not a coverage guard; coverage now comes from the two derived tests. broker.uri is documented only in the example's prose convention (`# uri -> ...`), never as a copy-pasteable `uri:` key, because writing it out invites pasting a password into a file -- the thing uriEnv exists to avoid. The matcher accepts that convention rather than pushing the file toward doing it. Full build: 1236 tests, 0 failures, 0 compile errors. |
||
|
Dai Ha
|
bd2774b5f1 |
fleetd #155: refuse a member spawn under memberCredentials.policy=allow-list on a non-zsh shell
The ZDOTDIR scrub that enforces policy=allow-list only runs on zsh. The daemon already detected a non-zsh login shell (isZshShell/warnNonZsh, from #213), but degraded to the weaker CB-596 overlay and spawned anyway — the exact "control silently does nothing" defect this ticket is about. Now a non-zsh shell under policy=allow-list refuses the spawn (IllegalArgumentException, naming the shell), surfaced by FleetMcp.spawn's existing catch(IllegalArgumentException). policy=deny-by-default is unaffected in substance (its overlay never depended on the shell) but now also logs a one-time WARN naming the shell, since the stronger allow-list control is unavailable there. The worktreeRoot/worktreeGroup-missing degrade path under memberHerdrSocket is untouched — that gap is fleetd #213's scope, not this one. |
||
|
Dai Ha
|
c50f5b2d61 |
fleetd #176 stage 2: make effectiveCredentialId() subscription-aware
Stage 1's lead-seat matcher (leadSeatLookup) was correct but inert on the live host: the lead runs on profile 'opus', members on 'sonnet', both subscription:true with no explicit credentialId. Because effectiveCredentialId() fell back to the profile's own name, opus and sonnet never matched even though they share one Claude login, so the matcher charged zero seats. FleetConfig.Profile.effectiveCredentialId() now falls back to a shared sentinel (SUBSCRIPTION_CREDENTIAL_ID = "<subscription>") instead of the profile name when subscription:true and credentialId is unset. An explicit credentialId still wins, so two separate Claude logins on one host can still be kept apart. This is also BackendQuarantine's and BackendOutagePolicy's grouping key and CompositePeerLauncher's spawn-time enforcement key, so the fix also links quarantine/cool-off across subscription profiles sharing an account -- intentional: one usage limit really does take out every profile on that login, mirroring credentialId: openai-shared already doing this for off-subscription profiles. Every caller was reviewed; none wants "this exact profile" over "this account". Tests added: - FleetdLeadSeatLookupTest: the live shape itself (lead on a DIFFERENT subscription profile than the target, same account, neither sets credentialId) -- the case stage 1's suite never covered - FleetMcpTest: quarantining one subscription profile's shared account zeroes free on another sharing it, via the same effectiveCredentialId()-driven wiring Fleetd.main uses Mutation-tested: reverting the subscription branch to the old fall-back-to-profile-name behavior sends both new tests RED with 0 compile errors; reverting the mutation restores byte-identical (diff -q) source and green tests. fleetd.example.yaml's fleetd #176 notes are rewritten for the sentinel semantics and when to override it with an explicit credentialId. |
||
|
Dai Ha
|
01a840cc14 |
#248 follow-up: drive the real backendErrorSink, not a copy of it
BackendOutageFlowTest held a ~30-line hand-copy of the lambda in Fleetd.main, under a comment promising it mirrored production "EXACTLY". That promise was the defect. The test proved the copy, so any change to the real sink left the flow test green. #248 made Fleetd.backendErrorSink(...) public for exactly this reason. The test now calls it. Measured, same mutation in the real sink (an early return after sessions.onBackendError, dropping the cool-off and the lead nudge): old test (hand-copy): Tests run: 5, Failures: 0 -- blind new test (real sink): Tests run: 5, Failures: 4 -- catches it 0 compile errors in both runs, so both are real results. Production reverted and confirmed with diff -q. Full build: 1234 tests, 0 failures, 0 compile errors. |
||
|
Dai Ha
|
c4deef08be |
fleetd #249: withhold agentSessionId when the cwd is not a provisioned worktree
A member spawned without a worktree inherits the lead's cwd, which holds many old opencode session rows. sessionIdForDirectory picks the most recently updated row for that directory, so a brand-new member - which has not written its own row yet - resolves to somebody else's session. Measured: a row three days old, from a different profile. The damage was at the tool surface. fleet_list told the lead that agentSessionId is the id to pass as resumeSessionId, so acting on it would resume a stranger's conversation, with foreign context, and nothing to distinguish that from a correct resume. Fixed by refusing to answer rather than by making the heuristic smarter. #234 already established the heuristic cannot be made reliable at that layer, and its javadoc records why, so the SQL is untouched. agentSessionId() now returns null for a non-provisioned cwd, and spawn() refuses a resumeSessionId request for one outright, before anything starts. isProvisionedWorktree moved to HerdrPeerLauncher so both adapters share it. fleet_list and fleet_spawn descriptions no longer describe the id as always safe to resume. Verified rather than taken on trust: - the refusal reaches the lead as a readable message, not a stack trace - FleetMcp.spawn already catches IllegalArgumentException and returns error(e.getMessage()). - the message tells the lead to pass fleet_spawn{worktree:<slug>}, which is valid: worktree is typed string, 'true' or a ticket slug. - 13 existing tests moved off a placeholder "/work/dir" onto a real provisioned-worktree fixture. They cover #175/#234 model-mismatch machinery and would otherwise have tripped the new gate incidentally. Worker's mutation evidence, both reverted and diff-confirmed: - gate at OpenCodeLauncher:812 -> if(false): RED at OpenCodeLauncherTest:448, expected <null> but was <ses_someone_elses>. - resume refusal at OpenCodeLauncher:683 -> 'false &&': RED at OpenCodeLauncherTest:379, expected IllegalArgumentException. Both 0 compile errors. Closes #249. PR #253. |
||
|
Dai Ha
|
c796eac09c |
fleetd #176: subtract the lead's own subscription seat from free
maxLoad counted panes, never subscription seats: a subscription:true profile's lead is itself a live claude session on that same account, so free overstated capacity by the lead's own seat (measured free:1 with a real ceiling of 0, and free:3 on an idle fleet with a real ceiling of 2). Add FleetMcp.LeadSeatSource (same shape as QuarantineSource/ OutageSource) and Fleetd.leadSeatLookup, which derives the seat count from fleet.leaders.<name>.profile matched against the target profile by effectiveCredentialId() - no hardcoded "-1", and no new config key: profile: already exists for this exact "which account does this lead share" question. maxLoad itself is left untouched; only free (and a new, additive-only leadSeats field) changes. Exhaustion quarantine (cause 2 in the ticket) already forced free to 0 via the same BackendQuarantine capacityView already reads - confirmed by reading the exhaustionSink wiring, no code change needed there. |
||
|
Dai Ha
|
e897e5257b |
fleetd #114: delete the drifted tool catalogue, keep the flows, guard the names
docs/MCP-Contract.md was written 2026-07-14, before any MCP code existed,
and never caught up. CLAUDE.md points every session in the fleet at it.
Audited against the code today. The drift was not confined to the tool
table the ticket reported:
section 3 still described the OLD identity rule - "any connection that
does not map to a known worker is treated as a primary".
That was a real privilege bug, fixed since by the ancestry
walk in #161. The page still taught it.
section 4 names port 8080 (the mount is 8765) and says the pom does
not yet carry an MCP dependency.
section 5 named fleet_read and fleet_cancel, which do not exist, and
omitted fleet_poll, fleet_ack, fleet_profiles, fleet_whoami
and fleet_list, which do.
section 8 says turn_id where the code says turnId, and has no row for
the exhausted outcome CB-578 added.
sections
9, 10, 11 pre-build planning: "new work" columns, open decisions long
since decided, CB-1xx placeholders.
Every one of those is the same defect: a hand-maintained second copy of
something the code already states. So the copy is deleted rather than
corrected - correcting it just restarts the clock.
What survives is the flows and the status gating, because a flow is a
shape rather than a name, and shapes are what this page was ever good
for. They are rewritten with the names checked against the code, and
extended with what has been learned since: the ~60s cap on a blocking
send, the ~55s ask window, and the three ways the turn-done fallback
loses a report (clipped, echoed brief, slow member).
389 lines -> 188.
The names that remain are guarded. McpContractDocTest fails if the page
names a fleet_* tool FleetMcp does not register, and - because an empty
set is a subset of everything - a second test pins that both sides
actually found names, so the check cannot pass by checking nothing. A
third pins the "this is not the tool reference" sentence, which is the
fix itself: without it someone helpfully re-adds a tool table.
Mutation-tested both ways, 0 compile errors each: adding `fleet_read` to
the doc fails theDocNamesNoToolThatDoesNotExist ("names [fleet_read] ...
Checked 6 name(s)"); removing the disclaimer fails
theDocStillDisclaimsBeingTheToolReference.
All 5 mermaid diagrams render under mermaid-cli.
CLAUDE.md's pointer said "section 6 only" and now names the guard
instead. It is in the project addendum, so the canonical block is
untouched - verified still byte-identical with the wiki template.
REST is split out to #252: 14 routes, documented nowhere, and it IS a
supported operator surface - one of them drains on read.
1232 tests, 0 failures.
|
||
|
Dai Ha
|
2afa3652bb |
fleetd #249: withhold agentSessionId for a non-provisioned opencode cwd
OpenCodeSessionDiscovery.sessionIdForDirectory keys on the worker's cwd, which is reliable only when fleetd provisioned a unique git worktree for that member. Without one (the default no-worktree spawn), the cwd is shared with other sessions, and "most recently updated row for this directory" can pick a stranger's session — fleet_list would then hand a lead an agentSessionId that resumes someone else's conversation. Move isProvisionedWorktree from ClaudeCodeLauncher to the shared HerdrPeerLauncher base (both adapters need it now). OpenCodeLauncher.spawn now refuses a resumeSessionId spawn outright when the target cwd is not a provisioned worktree (fleetd can never verify or re-report that identity), and SessionAwareHandle.agentSessionId() withholds the id — returns null rather than guessing — for any member spawned without one, resumed or not. Corrected fleet_list/fleet_spawn's tool descriptions, which previously implied agentSessionId is always a safe resume handle. |
||
|
Dai Ha
|
80092ff359 |
fleetd #247: stop writing a trust key Claude Code strips on every save
seedTrustDialog wrote two keys into the shared .claude.json: hasTrustDialogAccepted and hasCompletedProjectOnboarding. Only the first one survives. Measured live on 2026-09-03, minutes after a spawn seeded the file: hasTrustDialogAccepted: 28 of 28 project entries hasCompletedProjectOnboarding: 0 of 28 project entries Our entry was written by the running jar and the key was already gone, so it was written and then removed. It is absent from the 27 entries Claude Code wrote for itself too, which says Claude Code normalises the whole file when it saves and drops that key every time. That reframes #247. I filed it as a race - a save landing between our read and our ATOMIC_MOVE. It is not a race. The other writer removes this key as its steady-state behaviour, with no window involved. So the compare-and-swap retry proposed there would not have helped: it would re-add a key that gets stripped again on the next save. The seed's whole job is to stop the workspace-trust dialog blocking a member (#149). The live probe reached idle with hasTrustDialogAccepted alone, so the second key was never doing that job. Writing it only added a contested key to a file two processes share, and made the next reader think it mattered. The atomic write and the lock stay. Both are still correct, both are cheap, and hasTrustDialogAccepted is genuinely shared state. The new assertion is assertFalse, not a deletion. Removing the old assertion would leave nothing to stop someone re-adding the key later as a plausible-looking completeness fix. Mutation-tested: restoring the production line fails seedTrustDialogWritesOnlyTheTrustFlagAndNotTheOnboardingKey:2198 with 0 compile errors. 1229 tests, 0 failures. |
||
|
Dai Ha
|
9d37f3aa29 |
fleetd #201: the coverage line must name the key its caller actually means
Found by reading a real boot log after the redeploy, not by a test.
coverage() is shared by two call sites — CB-578's exhaustedPattern line
and Unit 5's errorPattern line — but its 'off' branch hard-coded the
word exhaustedPattern. So this daemon printed:
backend-exhausted classification (CB-578 stage A): partial
(configured: [sol, terra]; not configured: [...])
backend-error classification (fleetd #201 Unit 5): off
(no profile has an exhaustedPattern configured; profiles: [...])
Two lines, one directly under the other, disagreeing about whether any
profile has an exhaustedPattern. Both were individually defensible and
together they were nonsense. Worse, the message sends an operator to
set the wrong key: the thing that is missing is errorPattern.
coverage now takes the key name. I changed the signature rather than
adding an overload, so the compiler found all three existing callers
instead of leaving them silently on the old path.
Every earlier coverage test passed the exhaustion case only, which is
why none of them could see this. The new test pins the errorPattern
case. Reverting the fix turns it red with 0 compile errors.
1229 tests, 0 failures, BUILD SUCCESS.
This is the second defect in two hours found only by reading the live
startup log — see #115, where the noise of a false warning had been
hiding a correct line saying a whole feature was off.
|
||
|
Dai Ha
|
eaf89abaf6 |
fleetd #248: make Fleetd's CompletionResolver wiring provable
Before this, dropping either #241's worktree lookup or Unit 5's backend-error pair at Fleetd.main's new CompletionResolver(...) call left all 1216 tests green with 0 compile errors. Every existing test built its own CompletionResolver, so they proved the class and never the wiring. BackendOutageFlowTest was the sharpest case: it copies main's sink lambda line-for-line, so it proves the copy and cannot notice the original being deleted. The three inline arguments are now package-private static factories on Fleetd, following the deliverableTo pattern the file already had, each with its own behaviour test. backendErrorSink is public so a cross-package test can drive the real production object rather than a hand-mirrored copy. The test that was actually missing is a source-text assertion. That is the honest fallback for a composition root with no seam, and it is labelled [SOURCE TEXT] in every test name and message so it cannot be misread as a behaviour check. It is not vacuous: two tests pin that the variables are assigned from the factories, and two pin that those variables reach the call site, so renaming a variable while assigning an inert value does not slip through. Known cost, accepted: the assertions match exact source substrings, so reformatting that statement will break them. That is the price of covering a main method, and a spurious failure here is loud and obvious, which is the right direction to fail. Verified by the lead, both mutations re-run against the merged code — see the merge check. PR #251 |
||
|
Dai Ha
|
d895f02bc1 |
fleetd #248: prove main() wires CompletionResolver's arguments, not just the class
Fleetd.main built three of CompletionResolver's 8 constructor arguments inline (a worktree/branch lookup lambda, and the backend-error pattern lookup + sink locals). Dropping any of them at the call site compiled clean and left every existing test green, because every existing test constructs its own CompletionResolver and only ever proves the class, never main's wiring. Extract each into a static factory on Fleetd (worktreeBranchLookup, backendErrorPatternLookup, backendErrorSink — the same static-factory pattern Fleetd.deliverableTo already uses), test each factory's own behaviour, and add a source-text assertion (FleetdCompletionResolverWiringTest) proving main's CompletionResolver call still passes all three. backendErrorSink is public so BackendOutageFlowTest can exercise the real production sink directly instead of the hand-mirrored copy its own class doc used to describe. No production behaviour changes — mechanical extraction only. |
||
|
Dai Ha
|
43206cac2f |
fleetd #148 point 2: drop .envrc from the default parity overlay
The default is now [.env], not [.env, .envrc]. .env is data, so copying it into a worker worktree can only move values. .envrc is executable shell that direnv runs on every cd, so copying it moves behaviour. Those are different risks and should not share a default. The knob is unchanged. An operator who wants .envrc copied writes parityOverlay: ['.env', '.envrc'] and owns that choice; a new test pins that escape hatch, because without it this would be a removal rather than a re-default. Decision recorded on the ticket, with the evidence it asked for first: this checkout has no .env and no .envrc, and direnv is not on PATH, so there was no live exposure. Point 1 (extend the credential scrub to direnv) is declined and the reason is on the ticket — the scrub is a one-shot .zlogin and a direnv hook runs on every cd, so no amount of work on the scrub can cover it. Not copying the executable file is the smaller change and removes the need. The worker also fixed WorktreeSessionManagerTest, which hardcoded the same default at another layer and broke the build. Outside its named scope, correctly flagged rather than done silently. Verified by the lead: 1216 tests, 0 failures, 0 compile errors. PR #250 |
||
|
Dai Ha
|
8bba3a8184 |
fleetd #148 (point 2): drop .envrc from the default parityOverlay
.env is data; .envrc is executable shell that direnv runs on every cd, so copying it into a worker moves behaviour, not just values. The default parityOverlay is now [.env] only. The knob is unchanged: an operator who wants .envrc copied can still write parityOverlay: [.env, .envrc] explicitly. Updates FleetConfig's default and javadoc, fleetd.example.yaml's two mentions of the default, and the FleetConfigTest coverage: renamed the default test, added parityOverlayExplicitEnvrcOptInStillWorks to prove the .envrc opt-in escape hatch still works, and fixed WorktreeSessionManagerTest#worktreeAcquireRunsParityOverlayWithProfileDefaults which also hardcoded the old default. |
||
|
Dai Ha
|
5cf3ca9a89 |
fleetd #241: never hand the lead back its own brief as the member's report
The completion fallback scrapes a member's pane when a turn ends with no fleet_reply. If the pane still shows the brief the lead injected, the scrape returned that brief, and the lead read its own words as the member's answer. A silent member looked like a member that had reported. echoesInjectedBrief now recognises that case and refuses it. Round 1 used plain containment in both directions, which destroyed real reports: a genuine report that quotes the brief contains it. Round 2 keeps the safe direction unbounded (the brief contains the scrape) and bounds the other one at MAX_ECHO_EXCESS_CHARS, so a scrape only counts as an echo when it adds almost nothing to the brief. Merge note — the Fleetd.java conflict: This call site was changed by both #201/#227 Unit 5 (backendErrorPatterns + backendErrorSink) and by this ticket (the worktree/branch lookup). I resolved it onto the full 8-argument constructor so neither feature is dropped; nowNanos has to be passed explicitly to reach that overload. Verified by the lead: 1215 tests, 0 failures, 0 compile errors. I also measured whether the resolution itself is protected, and it is NOT. Both mutations at this call site stay green: - drop the worktree lookup (pass _ -> null): 1215 tests, 0 failures - drop Unit 5's patterns/sink (legacy()/none()): 1215 tests, 0 failures Nothing in the suite covers Fleetd's composition root, so either feature could be silently unwired here and the build would still be clean. The tests prove the seams, not the caller. Filed separately rather than fixed in a merge commit. PR #245, branch worker/cb241-fallback-echo-1175e9-11 |
||
|
Dai Ha
|
ac474981e4 |
fleetd #201/#227 Unit 5: wire the backend-error cool-off into config, placement and the MCP surface
A profile's credential that throws two distinct backend errors inside 60
seconds now cools off for 60 seconds. Automatic placement skips it,
an explicit fleet_spawn naming it is refused before the adapter is
called, and fleet_list/fleet_profiles report it as coolingOffForSeconds
next to the separate CB-578 quarantinedForSeconds.
Verified by the lead: see the merge check below. The worker ran 7
mutations, all killed with 0 compile errors; M7 was NOT killed on the
first pass (the assertion only checked .contains("quarantined"), which
is true of both the correct message and the mutated fallback), and the
worker strengthened it to assertEquals on the exact literal and kept
that change. That is the right call and it is reported honestly.
Two deviations, both justified in the PR:
- FixedPlacementPolicy needed the same coolingOff filter because it
filters candidates inline instead of using PlacementPolicyUtil.
- BackendOutageFlowTest sits in dev.ltms.fleet.inject because
CompletionResolver.InFlight is package-private there.
The startup coverage log line is a code-reading claim, not a captured
line from a live daemon. The worker said so rather than overclaiming.
PR #246, branch worker/cb201-unit5-wiring-6c12e6-8
|
||
|
Dai Ha
|
ba04b2359b |
fleetd #201/#227 unit 5: wire errorPattern, cool-off spawn gate, and fleet views
Wires the already-merged units into production: - Per-profile errorPattern config (beside exhaustedPattern), compiled once at startup; falls back to the legacy (?i)\bAPI Error\s*: pattern when unset. Startup logs configured-vs-legacy coverage, same as exhaustedPattern. - One production BackendErrorSink in Fleetd.java: mark backend_error on the session, resolve the profile's credential fail-loud (never Optional.ifPresent), record it in BackendOutagePolicy, and push a lead nudge on a new incident. - CompositePeerLauncher's explicit and automatic spawn paths both refuse a cooling-off credential; exhaustion quarantine wins when both are active. PlacementContext gets a separate coolingOff set so refusal text says "cooling off", never "exhausted". - fleet_list/fleet_profiles report coolingOffForSeconds as an independent fact from quarantinedForSeconds; both can appear together. - fleetd.example.yaml documents errorPattern and the 2/60/60 cool-off policy; CLAUDE.md tells leads how to read the two independent outage states. Also: FixedPlacementPolicy.java, not in the original file list, needed the same coolingOff filtering as PlacementPolicyUtil (it does its own inline candidate filtering rather than delegating). 1190 tests, 0 failures (up from the 1163 baseline); BUILD SUCCESS. |
||
|
Dai Ha
|
2e5b63f6f6 |
fleetd #149: seed the workspace-trust entry before a claude-code spawn
Claude Code asks 'is this a project you trust?' the first time it starts in a directory it has not seen. It is interactive with no timeout, and every member spawned with worktree:true lands in a brand-new directory. The member never reaches its first turn and never replies, while herdr reports blocked/interactive_ready — which reads as healthy. ClaudeCodeLauncher now seeds projects.<cwd>.hasTrustDialogAccepted in the profile's .claude.json before the process starts. This is not a new grant: the operator already trusted the repo by configuring the profile against it, and a worktree is a checkout of it. The write is gated on isProvisionedWorktree(cwd) — a .git that is a regular gitdir-pointer file, never a real checkout. That gate exists because an earlier revision of this change, run under mutation testing, wrote to the operator's real ~/.claude.json and truncated it from 72KB to 919 bytes. Tests using a null configDir fall back to the real user.home, so an ungated seed reaches real files. The write is atomic (sibling temp file + ATOMIC_MOVE, never truncate-in-place) and the whole read-modify-write is under a lock, because .claude.json is large, live, and rewritten by Claude Code itself while fleetd runs. Two parallel spawns are normal here. Verified by the lead: 1173 tests, 0 failures. A truncating write turns the torn-read test red; removing the lock turns concurrentSeedsForDifferentCwdsBothSurvive red. Both with 0 compile errors. copyPosixPermissionsIfPresent is NOT covered by a test — its mutation stays green — but createTempFile is 0600 on POSIX by default, so the not-world-readable property holds without it; the line only preserves a non-default mode. |
||
|
Dai Ha
|
3437d6313d |
fleetd #241: bound the echo match so a real report is never swallowed
Round 1 used plain bidirectional containment. The direction that catches the real bug -- the pane holds the brief plus a status bar, so the scrape contains the brief -- also fires when a member restates the whole brief and then writes a genuine report under it. That threw the report away and told the lead nothing was produced, which is worse than the bug being fixed: it destroys a delivery instead of merely obscuring one. The safe direction (the scrape is a fragment of the brief) stays unbounded, because a fragment of the brief is by definition not a report. The dangerous direction now requires the scrape to add at most MAX_ECHO_EXCESS_CHARS beyond the brief, which is the amount of TUI chrome a real echo carries. Work by the cb241 worker, committed by the lead: its backend stopped answering after the fix was written, so two turns ended with no commit and no reply. Verified by the lead: 1169 tests, 0 failures; removing the bound turns pinsTheMaximumTuiChromeExcess and completionFallbackKeepsARealReportThatRestatesTheWholeBrief red with 0 compile errors. |
||
|
Dai Ha
|
743377d6cd |
fleetd #149 review round 2: make the trust-dialog seed atomic and lock-protected
Files.writeString truncates the target in place before writing, so there was a window where .claude.json could be observed empty or half-written - exactly the shape of the incident this ticket already hit once, but reachable in production too: a crash/kill mid-write, or two concurrent claude-code spawns (normal here - several run in parallel routinely) racing a naive read-modify-write and silently discarding one spawn's entry. Two independent fixes, each with its own dedicated test proving it (not the other): - ClaudeCodeLauncher.writeAtomically: serialise to a sibling temp file in the same directory, then Files.move with ATOMIC_MOVE + REPLACE_EXISTING, preserving the target's existing POSIX permissions (.claude.json ships 0600). A reader now only ever observes the fully-old or fully-new file, never a torn one. Package-visible so a test can drive it directly. - TRUST_JSON_LOCK: a process-wide lock around seedTrustDialog's whole read-modify-write, so two concurrent spawns for different cwds both keep their entry instead of the second write discarding the first. Sufficient because every spawn on this daemon runs in one JVM; it does NOT protect against a second daemon process or the operator's own live Claude Code writing at the same instant - writeAtomically covers that case instead. Both fail soft, same as before: any I/O failure here must never block a spawn. Four new tests: a large (30-project) existing file survives without collapsing (asserted on the restored key set, not just that the result parses); two concurrent spawns for different cwds both keep their entry (CountDownLatch-synchronised, not a sleep); existing 0600 permissions survive the write; and a direct test of writeAtomically with a busy-poll reader thread proving a concurrent reader never observes a torn file. See PR body for the full mutation-testing table, including an honest note on which of these tests the atomicity mutation actually caught (not the one implied by the numbering in review) and why. |
||
|
Dai Ha
|
5952d559c7 |
fleetd #134 point 3: tell the lead which files a worktree neutralizes
The daemon log and the worktree git config are both new in
|
||
|
Dai Ha
|
205ad823b0 |
fleetd #134: make tool-surface neutralization visible to the daemon and the worker
isolateToolSurface replaces .mcp.json, opencode.json and .autoenv with stubs in every provisioned worktree and marks them --skip-worktree. That neutralisation is correct and is unchanged here — the committed files would mount the primary's credentials. The problem was that it was invisible. A worker told to edit opencode.json read a 3-byte stub and reported, truthfully and wrongly, that the mount key did not exist. A missing file would have prompted a question; a plausible stub did not. Two changes, both visibility only. The daemon now logs one info summary per provisioning with the denominator, the files neutralised, and the consequence. And the list is recorded in worktree-scoped git config (fleet.neutralizedConfig / fleet.neutralizedConfigNote) so a worker can discover it from inside its own worktree with 'git config --worktree --get-all fleet.neutralizedConfig'. Worktree-scoped config was chosen over a file in the working tree because it lives in .git/worktrees/<nonce>/config.worktree and so can never appear in git status, and because configureEnvironmentCredentialHelper already uses the same mechanism in the same add() call. Verified by the lead: baseline 1172 tests, 0 failures. Reverting the summary to log.debug goes red (2 tests), and recording into --local rather than --worktree — which would leak the record into the shared repo config — goes red too. Both with 0 compile errors. |
||
|
Dai Ha
|
d654ccb818 |
fleetd #134: make tool-surface neutralization visible to the daemon and the worker
isolateToolSurface replaced .mcp.json/opencode.json/.autoenv with neutral stubs and
marked them --skip-worktree, but said nothing anywhere. A real worker read a 3-byte
{} stub for opencode.json, where the repo's real file is 30+ lines, and truthfully
(but wrongly) reported a mount key did not exist.
Two readers, two fixes:
- the daemon operator gets one info log per provisioning, naming the denominator,
what was neutralized, and why anything was not (same shape as overlayParity's
fix in #148 point 3).
- the worker gets the same fact recorded in worktree-scoped git config
(fleet.neutralizedConfig / fleet.neutralizedConfigNote), discoverable with
`git config --worktree --get-all fleet.neutralizedConfig` from inside its own
worktree, without asking the lead. Not a working-tree file: this repo already
uses worktree-scoped config for the credential helper and the SSH->HTTPS
rewrite, and it lives under .git/worktrees/<nonce>/ so it can never appear in
`git status` for the worker to trip on or commit.
The neutralization itself (stub content, --skip-worktree marking) is unchanged.
|
||
|
Dai Ha
|
a89dcc9b7e |
fleetd #149: seed the workspace-trust entry before a claude-code spawn
A claude-code member spawned into a fresh worktree hits an interactive, un-timed workspace-trust prompt on its first start in a directory it has never seen. It never reaches its first turn and never mounts the bridge. Fix: ClaudeCodeLauncher.seedTrustDialog writes projects.<cwd>.hasTrustDialogAccepted / hasCompletedProjectOnboarding into the profile's configDir/.claude.json (or ~/.claude.json when configDir is unset) BEFORE the herdr spawn call, additively (existing keys/projects are preserved). Gated to isProvisionedWorktree(cwd) - a .git that is a regular gitdir-pointer file, never a real checkout's .git directory - the same signal writeIdeOverlay already used, now shared between both. That gate is a fix for a real incident hit while building this: an earlier ungated version ran against this file's own pre-existing tests (configDir=null, no cwd -> falls back to the real user.dir and ~/.claude.json) and corrupted the operator's actual ~/.claude.json down to a single entry during a mutation-testing run. See PR body for the full incident report. FakeHerdr gained onAgentStart(Runnable) so a test can assert the seed is on disk at the exact instant herdr's agent.start call is reached - i.e. strictly before the peer process itself would start. |
||
|
Dai Ha
|
0d7b4fb026 |
fleetd #134 + #148 point 3: make the parity overlay say what it did
Both defects lived in one method. overlayParity logged every step at debug, so at the default level the copy was silent and nobody could tell which overlay files a member actually got. It also marked a copied tracked file --skip-worktree and said nothing, so a worker editing that file later found git ignoring the change with no error anywhere. The summary now reports the denominator, not a bare count: 'copied 1 of 2 candidates: .env (.envrc absent)'. A bare 'copied 1' is the same under-reporting shape as #113. Neutralised files are named with the consequence in the message itself. No marker file is written into the worktree: acceptance criterion 1 requires the worktree to hold exactly the configured overlay set, so a marker would violate the fix it documents. The copy and mark logic is unchanged — only logging is new. Verified by the lead: baseline 1168 tests, 0 failures. Reverting either log.info to log.debug goes red (2 reds and 1 red, 0 compile errors each), which is the regression that matters since the whole fix is the log level. |
||
|
Dai Ha
|
321d8dcbb5 | fleetd #241: suppress echoed fallback briefs | ||
|
Dai Ha
|
ef8c97871e |
fleetd #134/#148 point 3: make overlayParity's copy and skip-worktree visible
overlayParity logged everything at debug, so at the default level nobody could tell which overlay files a spawn actually received (#148 pt 3), and a tracked file marked --skip-worktree gave no warning that it can no longer be edited from that worktree (#134). Report the outcome at info: a per-spawn summary naming the denominator (every configured candidate), what was copied, and why anything was not — plus a separate line naming every file marked --skip-worktree, stating plainly that it cannot be committed from this worktree. No worktree-local marker file: the worktree must hold exactly the configured overlay set and nothing else, so an extra file would violate that invariant. |
||
|
Dai Ha
|
26bafe824b |
fleetd #201/#227 unit 1: classify a backend error at the scrape
CompletionResolver already reads the pane on every turn, so the classifier lives there rather than in a new watcher. A matched pattern resolves the waiter as a failure and fires BackendErrorSink, always inside the resolveFailure win-gate so exactly one thread reports one incident. The too-fast path now takes a fresh scrape instead of reusing the pre-turn text, so a backend that dies immediately is still classified. BackendErrorSink is a single-method functional interface by design — see #234 for what a default overload does to a lambda. |
||
|
Dai Ha
|
838a701109 |
fleetd #234: carry the profile hint to the exhaustion sink
An exhausted opencode member could not always be mapped back to a credential, because the launcher knew the profile and the sink did not. The sink now takes a profile hint. The interface is inverted on purpose: the three-argument method is the single abstract method and the two-argument one is the default. A lambda can only implement the abstract method, so every lambda is now forced to carry the profile. The first round of this fix added the third argument as a default overload, and the production forwarder in Fleetd was a two-argument lambda — so the fix compiled, passed its tests, and never ran. Verified by the lead: deleting the forwardingTo factory's override, and rewriting the Fleetd call site as a plain lambda, both fail to compile now rather than passing silently. |
||
|
Dai Ha
|
e5eb3534c7 |
fleetd #201/#227 unit 3: lead outage nudge
Backend incidents become a fourth source inside ReplyPushLoop, not a new scheduler — a second injector would race the one control that already owns lead-pane delivery. One notice per incident per affected lead (not per member), one-shot, waiting while the lead pane is not injectable, and combined into the same nudge as any pending failed ticket. A classified target that cannot be mapped to a credential gets its own truthful notice and its own pending/delivered records. It previously reused the incident message, which told the lead a credential was 'cooling for 0 remaining seconds' when nothing was cooling, and smuggled the free-text reason into the profiles field. Verified by the lead: 1129 tests green; routing the unmapped path back through onBackendIncident turns unmappedBackendTargetUsesTheKnownLeadSchedule red with 0 compile errors, and that test now asserts the whole rendered message rather than two substrings. |
||
|
Dai Ha
|
959c83534f |
fleetd #201/#227 unit 2: credential outage policy
Adds BackendOutagePolicy — a credential-keyed state machine on an injected monotonic clock. Two classified backend errors from two DISTINCT targets on one credential inside 60 seconds mint one incident and start a 60-second cool-off. Errors during cool-off neither extend it nor mint another; expiry clears evidence, so two fresh errors rearm. Correlated on credentialId, never on profile name or error text. Deliberately not BackendQuarantine: that restarts a 1800-second cooldown per exhaustion, and its name would make every refusal say 'backend exhausted', which is a different condition. Threshold counts distinct targets rather than raw events (lead decision): the classifier is a heuristic and a valid member report can quote an 'API Error:' line, so one member repeating that line must not remove a healthy credential's capacity. A real outage hits every member on the credential, so true detection is unaffected. Verified by the lead: 1135 tests green; reverting evidenceCount() to reasons.size() turns two BackendOutagePolicyTest cases red with 0 compile errors. |
||
|
Dai Ha
|
31b028e860 |
fleetd #234 round 4: invert ExhaustionSink's abstract method so the bug class is unrepresentable
Round 3's factory fixed the two known call sites but the underlying shape was still there: a lambda written against ExhaustionSink binds to whichever overload is abstract, and the 2-arg form held that position, so ANY lambda -- a call-site forwarder, a hand-built test double, a future caller who has never heard of fleetd #234 -- could still silently take the hint-dropping default. Two rounds shipped exactly that mistake in two different places. Fix: made the 3-arg onExhausted(target, reason, profile) the interface's single abstract method; the 2-arg form is now a default that delegates with a null profile. A lambda declared against ExhaustionSink today is forced by the compiler to take three parameters -- there is no overload left for it to bind to that can drop the hint. This is enforced by the type system, not by a test that has to remember to check for it. Knock-on changes: - ExhaustionSink.none() -- a 3-arg lambda, still a genuine no-op, now safe by construction rather than by care. - ExhaustionSink.forwardingTo(...) -- collapses to a one-line 3-arg lambda; kept as a named factory (round 3's lesson: a test must call the real object, not rebuild its shape). - Fleetd.java's real sink and the two OpenCodeLauncherTest sinks that used to be anonymous classes overriding both overloads are now plain lambdas too -- the 2-arg override each carried was pure boilerplate once the interface provides it as a default. - CompletionResolver.java itself: UNCHANGED, zero diff (confirmed via `git diff --stat` before staging) -- its two call sites still call the 2-arg onExhausted(target, reason), which is now the default and behaves identically. CompletionResolverTest (41 tests, 0 failures) proves this; its five ExhaustionSink lambdas needed a mechanical third parameter added to keep compiling against the new abstract method, no assertion changed. Mutation proof, re-run against the new shape: forwardingTo's body edited to call the 2-arg default instead of passing the hint through (the equivalent of round 3's "delete the 3-arg override" now that there is only one method to break) -- both new tests go red with the same assertions as round 3: ExhaustionSinkForwardingHazardTest...: expected: <gx> but was: <null> OpenCodeLauncherTest...ForwardingHop: expected: <true> but was: <false> Tests run: 68, Failures: 2 Restored, re-ran: green (Tests run: 109, Failures: 0, including CompletionResolverTest). Compiler proof (not committed -- a scratch file outside the worktree, compiled with the real ExhaustionSink.java on the classpath, then deleted): ExhaustionSink forwarder = (target, reason) -> System.out.println(target + reason); error: incompatible types: incompatible parameter types in lambda expression A 2-arg lambda against this interface no longer compiles at all. mvn clean install: Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS. |
||
|
Dai Ha
|
7840e9adf6 | CB-201: make unmapped target notice truthful | ||
|
Dai Ha
|
c3672f5472 |
fleetd #201/#227 unit 4: durable BACKEND_ERROR member outcome
Adds MemberSession.State.BACKEND_ERROR with a nullable failureReason, surfaced in rosterView, and SessionManager.onBackendError(target, reason). The CAS loop accepts both sides of the completion race (BUSY and DONE); BACKEND_ERROR is terminal. completeTurn now returns early when its CAS loses, so a stale DONE copy can no longer release the pane or reset its context behind a member that just went BACKEND_ERROR. Verified by the lead: 1130 tests green; mutating the completeTurn early return back to the old fall-through turns losingCompletionDoesNotReleaseOrClearABackendErrorMember red with 0 compile errors. |
||
|
Dai Ha
|
cf54aed451 |
CB-201 unit 2 review fix: threshold counts distinct targets, not raw events
Two errors from the same target inside the window must never trip the outage threshold on their own (a valid member report can legitimately quote an "API Error:" line twice) — only two DIFFERENT targets on the same credential do. Change evidenceCount() to targets.size() instead of reasons.size(); reasons() still keeps every event, including same-target repeats, so it can be longer than evidenceCount(). A real outage still hits every target on the credential, so this loses no true-positive coverage while cutting a real false-positive path. |
||
|
Dai Ha
|
bbf68f3e3c | CB-201: cover losing completion CAS | ||
|
Dai Ha
|
776743cbe2 |
fleetd#201 Unit 1: typed backend-error classification in CompletionResolver
Replace the hardcoded API-Error check with a target-keyed BackendErrorPatternLookup plus a BackendErrorSink, mirroring the existing ExhaustedPatternLookup/ExhaustionSink pair. Classifies in all three paths (normal block, #211 raw-scrape fallback, and the fleetd#164 MIN_TURN_NANOS floor). The sink fires only after Rendezvous.resolveFailure wins for the exact waiter. A target with no configured pattern still falls back to the narrow (?i)\bAPI Error\s*: compatibility pattern. Existing constructors keep compiling via BackendErrorPatternLookup.legacy() / BackendErrorSink.none() defaults. Public send result is unchanged (still a failed send) — the typed sink event is the internal seam Unit 5 will consume. |
||
|
Dai Ha
|
826e0aeb2a |
CB-201 unit 2: credential-keyed backend outage policy
Add BackendOutagePolicy: two classified backend errors on the same credentialId within a 60s window mint one Incident and start a 60s cool-off for that credential, one atomic ConcurrentHashMap.compute() per credentialId so a concurrent second and third event can never both cross the threshold. Errors during cool-off are ignored outright (no extension, no incident); once cool-off elapses the next error clears old evidence, requiring two fresh errors to rearm. This is a new class, deliberately not BackendQuarantine (wrong store, wrong 1800s duration, misleading "exhausted" semantics for a 60s transient fault). Knows nothing about panes, profiles, sessions, launchers, or leads — takes events in, returns incidents out. |
||
|
Dai Ha
|
c935b181dd |
fleetd #234 round 3: make ExhaustionSink's forwarder a shared factory, not a rebuilt-per-caller shape
Round 2's tests never reached Fleetd.java at all: both new tests declared
their OWN local copy of the forwarding shape instead of calling production's.
Mutating Fleetd.java's real forwarder back into the broken lambda left those
copies untouched, so the whole suite stayed green while production had
regressed to exactly the bug being fixed -- proven live by the reviewer.
Fix: extracted the forwarding shape into one named factory,
ExhaustionSink.forwardingTo(Supplier<ExhaustionSink> target), with the
"why a lambda here is wrong" explanation moved onto it (the one place the
shape is now written). Fleetd.java's forwarder collapses to one line:
ExhaustionSink forwardingExhaustionSink = ExhaustionSink.forwardingTo(exhaustionSinkRef::get);
Both new tests now call this same factory instead of rebuilding an anonymous
class inline, so they exercise the identical object production builds:
- ExhaustionSinkForwardingHazardTest: calls ExhaustionSink.forwardingTo
directly and asserts the hint reaches the real sink through it.
- OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:
same factory call, inside the full Fleetd-shaped construction order
(forwarder built first, real sink pointed at via the AtomicReference
afterward), driven through the real SessionManager.acquire() path.
Mutation proof, this time on production code only: deleted the factory's
3-arg override (falls back to the interface default, dropping the hint) --
both new tests go red with no test file touched:
ExhaustionSinkForwardingHazardTest...: expected: <gx> but was: <null>
OpenCodeLauncherTest...ForwardingHop: expected: <true> but was: <false>
Tests run: 68, Failures: 2
Restored, re-ran: green (Tests run: 68, Failures: 0). Confirmed Fleetd.java
carries no lambda ExhaustionSink anywhere (grep). ExhaustionSink.none() stays
a lambda on purpose -- both its overloads are true no-ops regardless of
arity, so there is no hint to drop.
mvn clean install: Tests run: 1129, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
|
||
|
Dai Ha
|
ee932fd85b | CB-201: nudge leads about backend outages | ||
|
Dai Ha
|
fe2e5ede34 | CB-201: retain backend failure outcome | ||
|
Dai Ha
|
c325054242 |
fleetd #234 round 2: fix the ExhaustionSink forwarding hop Fleetd.java actually uses
The round-1 fix was dead on the real production path. Fleetd.java:177 builds
a forwarding sink (needed because the adapters are constructed before
`sessions` exists, breaking a genuine cycle) as a LAMBDA:
ExhaustionSink forwardingExhaustionSink =
(target, reason) -> exhaustionSinkRef.get().onExhausted(target, reason);
A lambda can only implement the interface's one abstract method (the 2-arg
overload), so it silently inherited the 3-arg overload's default body, which
drops the profile hint and calls back into the 2-arg method. OpenCodeLauncher
is constructed with this forwarder, so the hint it supplies (its own
already-known profile name) was thrown away before it ever reached the real
sink built later in Fleetd.main -- reproducing the exact silent no-op round 1
was sent to fix. The 1127 tests from round 1 all injected a sink directly
into OpenCodeLauncher and never went through this forwarding hop, so none of
them could see it.
Fix: forwardingExhaustionSink is now an anonymous class overriding both
overloads, each delegating to whatever exhaustionSinkRef currently holds.
Audited every other ExhaustionSink value in main/: the only other one is
ExhaustionSink.none() (a lambda), which is safe regardless of arity since
both its 2-arg body and the inherited 3-arg default are true no-ops.
New tests:
- ExhaustionSinkForwardingHazardTest: isolates the hazard at the interface
level (a lambda forwarder drops the hint; an anonymous-class forwarder
does not), independent of Fleetd.java's specific wiring.
- OpenCodeLauncherTest#theSpawnTimeQuarantineSurvivesTheFleetdStyleForwardingHop:
replicates Fleetd.java's actual construction order (forwarder built and
handed to the launcher first, real sink built and pointed at via the
AtomicReference afterward) and drives the quarantine through it via the
real SessionManager.acquire() path.
Both proven by mutation: temporarily rewriting each fixed forwarder back
into the pre-fix lambda makes its test fail with a real assertion message
(both matched exactly: "expected: <gx> but was: <null>" for the interface
proof, "expected: <true> but was: <false>" for the composed-wiring test);
restoring makes it pass again. No reverts were committed.
mvn clean install: Tests run: 1130, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.
|
||
|
Dai Ha
|
7662e2d0c8 | fleetd #201/#227: refine backend outage work | ||
|
Dai Ha
|
4877992a70 |
fleetd #234: key the opencode model check on the resolved session id, and make the spawn-time quarantine actually happen
Defect 1: OpenCodeSessionDiscovery.actualModelForDirectory queried WHERE directory = ?, the same heuristic sessionIdForDirectory uses. Since a default fleet_spawn (no worktree:) shares the lead's cwd with every other worker and every past session ever run there, the model read-back could silently compare against a DIFFERENT session's row. Renamed to actualModelForSessionId(sessionId), keyed on the primary key id instead, and made OpenCodeLauncher's SessionAwareHandle cache the resolved id once non-null (AtomicReference) so a later sibling row in the same directory can never flip which session's evidence is read. sessionIdForDirectory (#209) is left directory-based on purpose, with a comment explaining why the heuristic is unavoidable at that layer. Defect 2: the ERROR log claimed "quarantining this profile's credential" but Fleetd's ExhaustionSink lambda resolved target -> roster -> profile -> credential, while OpenCodeLauncher's model-mismatch check fires from agentSessionId() during SessionManager.acquire(), before the session is registered in the roster -- the lookup found nothing and silently no-opped. Added a default 3-arg ExhaustionSink.onExhausted(target, reason, profile) overload (defaults to the 2-arg method, so CompletionResolver's two call sites are unchanged); OpenCodeLauncher now passes its own already-known profile name; Fleetd's sink became an anonymous class that tries the roster first, falls back to the hint, and logs loudly at ERROR naming target/reason when neither resolves, instead of silently no-oping. Both fixes proven by mutation: reverting each independently makes its new test fail with a real assertion message, restoring makes it pass again. mvn clean install: Tests run: 1127, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS. |