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.
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.
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.
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.
tokenEnv no longer defaults to FLEETD_WORKER_TOKEN. A profile that names no
tokenEnv is stating it needs none, which is different from one naming a variable
that turns out to be unset. requiredSecretEnvVars now warns only for an explicitly
declared tokenEnv, so the permanent false alarm about a token no profile needs is
gone and the real warnings beside it stay trustworthy.
Decision recorded in a code comment: an explicit tokenEnv is checked for every
kind, opencode included. opencode can use its own provider credentials, but an
explicit tokenEnv declares a required host secret for its configured provider.
Second commit fixes a crash the first one introduced. Making tokenEnv nullable
changed what every reader of that value can receive, and ClaudeCodeLauncher:261
passed it straight to env.apply — System::getenv in production, which throws on a
null name. The live profile local-direct (kind: claude-code, baseUrl set, no
tokenEnv) would have crashed on spawn. It is weight: 0 today, so the failure would
have surfaced whenever someone re-enabled it. Now uses the superclass helper
resolveEnv, which already tolerates a null name and which OpenCodeLauncher was
already using.
Reader audit, all four: ClaudeCodeLauncher fixed; OpenCodeLauncher already safe via
resolveEnv; ConfigRef uses Objects.equals; MemberEnvAllowList drops null and blank
names in addIfPresent.
Verified by the lead before merge: reverting the resolveEnv fix makes the new test
fail with a NullPointerException from the null variable name, and restoring it
passes. Independent build: 1125 tests, 0 failures, 0 errors, 0 skipped.
Not verified: the ticket's acceptance criterion 4, a real boot on this host showing
no FLEETD_WORKER_TOKEN line while the WORKER_GITEA_TOKEN and AI_GATEWAY_TOKEN lines
are unchanged. A worker cannot restart the daemon it talks through. The lead checks
that at the next redeploy.
Two fixes to HerdrPeerLauncher.waitUntilInjectableOrThrow.
Fix 1: the status call is now guarded. A herdr *_not_found answer — what happens
when the backend process exited rather than being slow — used to escape as a raw
HerdrException, skipping stop() and leaking the pane and tab, with a message that
blamed a slow pane. It now fails immediately, runs the same teardown, and says the
process exited.
Fix 2: the gate can now resolve UNKNOWN with the same StatusRefiner the status
poller uses, behind two guards. It only runs for the claude adapter, because
StatusRefiner.classify reads a Claude Code TUI. And a refined result is accepted
only when the same agents.get sample still reports a non-null agentType. That
second guard closes a trap: a dead pane sits at a shell prompt containing the same
❯ glyph the classifier reads as idle, so refining without corroboration would turn
"the backend died" into "ready to inject".
Verified by the lead before merge: NAME_PREFIX really is "claude"/"opencode" so the
adapter split holds; AgentControl.status(t) was already get(t).status(), so moving
the loop to get() adds no herdr call; and a *_not_found already failed the spawn
before this change, so fix 1 improves an existing failure rather than creating one.
The adapter guard was mutation-tested independently (set it to `if (false)`, the new
opencode test fails with "Expected PeerUnreachableException to be thrown, but nothing
was thrown"; restored, it passes). Independent build: 1123 tests, 0 failures,
0 errors, 0 skipped.
Not fixed, and not claimed: that herdr reports a null agentType for a bare shell
pane is unverified against a live daemon. To be proved by a live spawn on both
backends after deploy. The seat-accounting suggestion in #176 was deliberately not
built — its cause was tested in that issue and not reproduced.
The agentType corroboration (guard b) had positive and negative tests, but the
per-adapter namePrefix guard (guard a) had none — nothing proved that an
opencode pane can never reach StatusRefiner.classify, only that a claude pane
with a null agentType is rejected. Since a live opencode pane always reports a
non-null agentType ("opencode"), guard (b) alone cannot catch a broken guard
(a).
Added opencodePaneIsNeverRefinedEvenWhenItsContentLooksLikeAnIdleClaudePrompt
to OpenCodeLauncherTest: agentType("opencode") (non-null, satisfies guard b on
its own) + pane content containing "❯" (would classify as IDLE) + raw status
UNKNOWN throughout. Asserts the gate still times out, and that no agent.read
call used source=detection (StatusRefiner.PROBE_SOURCE) — proving the refiner
was never even reached, not just that its answer was discarded. A blanket
"agent.read is never called" does not hold here: HerdrPeerLauncher.readPaneQuietly
reads the pane tail (source=recent) for the timeout log on every timeout,
regardless of adapter, so the assertion is scoped to the refiner's own probe
source instead.
Mutation check performed and reverted before this commit: temporarily changed
`if (!"claude".equals(namePrefix))` to `if (false)` in
HerdrPeerLauncher.refinedInjectable — the new test failed
("Expected PeerUnreachableException to be thrown, but nothing was thrown"),
confirming it actually exercises the guard. Restored the guard and reran —
test passes again (1/1). No production code changed in this commit.
mvn clean install: Tests run: 1123, Failures: 0, Errors: 0, Skipped: 0 -- BUILD SUCCESS
Fix 1: agents.get(paneId) inside waitUntilInjectableOrThrow was unguarded, so a
herdr *_not_found answer (the backend process exited) propagated as a raw
HerdrException instead of PeerUnreachableException, and skipped teardown
entirely, leaking the pane/tab. Now caught via isAlreadyGone(); fails
immediately (does not burn the rest of the timeout), runs the same teardown
the timeout path runs, and the exception message says the process exited
rather than that the pane was slow. Any other HerdrException still
propagates unchanged.
Fix 2: the gate now resolves a raw UNKNOWN into StatusRefiner's pane-content
classification, like StatusPoller already does mid-life. Guarded against the
trap noted in the ticket: a pane whose backend exited settles at a bare shell
prompt that can also contain the "❯" glyph classify() reads as idle. So a
refined result is accepted only when the corroborating agentType from the
SAME agent.get sample is non-null, and only for the "claude" adapter (namePrefix)
since StatusRefiner.classify is written for the Claude Code TUI only. Refine
only runs when the raw status is UNKNOWN, so a healthy spawn adds zero extra
herdr calls.
Tests added to ClaudeCodeLauncherTest (FakeHerdr gained agentType()/
agentGetFailsWithAfter() fixtures):
- spawnFailsFastWhenBackendProcessExitsMidWaitInsteadOfBurningTheTimeout
- spawnLetsAnUnrelatedHerdrErrorPropagateUnchanged
- refinedIdleIsNotAcceptedWhenAgentTypeIsNull
- refinedIdleIsAcceptedWhenAgentTypeCorroboratesLiveness
- refinementNeverFiresWhenRawStatusIsAlreadyInjectable
mvn clean install: Tests run: 1122, Failures: 0, Errors: 0, Skipped: 0 -- BUILD SUCCESS
Verified by the lead before merge. Read the full production diff, checkModelMatch, parseModel and actualModelForDirectory. Confirmed the check runs on the real #209 late-resolve path (not at spawn, which is why #203 was closed), that unknown/incomplete evidence never quarantines, and that claude-code is structurally excluded because SessionAwareHandle is only built by OpenCodeLauncher.spawn(). Round 2 closed the one gap I found: model JSON with an id but no providerID used to read as a mismatch for a provider-prefixed profile. Independent build: BUILD SUCCESS, 1117 tests, 0 failures, 0 skipped.
parseModel already tolerates a model JSON with an id but no providerID (a real shape
opencode can write). checkModelMatch's providerMatches check did not: a provider-
prefixed profile whose id matched but whose evidence had no providerID was reported
as a mismatch and quarantined on incomplete data, which acceptance rule 4 forbids.
Compare the provider only when BOTH the profile requested one AND the evidence has
one. A genuine id mismatch is still caught either way — narrows the check, does not
disable it.
opencode does not fail on an unknown -m <model> flag — it silently falls back to a
default model, which can be a paid credential. Extends the existing late-resolve
path (#209's SessionManager -> handle.agentSessionId() re-poll) so that once the
opencode session row exists, OpenCodeSessionDiscovery also reads its `model` JSON
column and OpenCodeLauncher's SessionAwareHandle compares it against the profile's
configured model.
Comparison rule: split the profile's model on the first '/' into provider+id. Compare
id always; compare provider only when the profile specified one. A bare model name
with no '/' matches on id alone. Absent/unparseable evidence is UNKNOWN, never a
mismatch, so a working profile is never quarantined on missing data. A real mismatch
logs an ERROR naming both models and the profile, then quarantines through the
existing ExhaustionSink path (wired via an AtomicReference forwarding sink in
Fleetd.java to break the sessions/workers/adapters construction cycle).
Verified by the lead before merge: read the full production diff, confirmed the memberHerdrSocket-absent branch is the literal unmodified Files.createTempFile call in its own branch, and that the refusal names the missing key. Measured behaviour recorded: claude 2.1.258 exits 1 immediately on an unreadable --append-system-prompt-file, so the pre-fix bug was the loud readiness-gate failure, not a silent charter-less member. The /tmp full-suite failure the worker reported was the two other #225 copies, fixed by #230 which is already on main; main is built and checked after this merge.
Verified by the lead before merge. Read the full production diff: locking is consistent (every MemberRegistry method uses synchronized(terminalToSlot)), and the refusal happens before the launcher starts a process. Proved the restored fallback test is real by removing the DEV fallback from SessionManager and re-running it: it failed with "expected: <DEV> but was: <ARCHITECT>", then reverted. Independent build: BUILD SUCCESS, 1092 tests, 0 failures, 0 skipped.
The helper I copied from OpenCodeLauncherTest read the CWD's owning
group instead of the process's real primary group, so it silently
picked up whatever group owns the directory Maven was started from
(staff in a home checkout, wheel under /private/tmp on macOS) rather
than a group the operator is actually in. Replaced with the id -gn
based resolution that landed on #230 for the other two copies of this
helper, same shape and skip wording.
Verified by the lead before merge: read the full production diff, confirmed `group` is normalised to null at GitWorktrees:148 so the `group == null` guard is complete, and confirmed the refusal runs before `git worktree add` so a failure leaves no half-made worktree. Independent build in the worker's worktree: BUILD SUCCESS, 1093 tests, 0 failures, 0 skipped.
#224: GitWorktrees#add created worktreeRoot with the daemon's umask and never shared it with
worktreeGroup, even though shareWithGroup shares every child underneath it (each worktree, and
the repo's common git dir). Under memberHerdrSocket: the member pane runs as a different OS
user, which needs execute on every ancestor directory to reach anything underneath, no matter
how carefully each child is shared — so a member could not read the opencode.json #219 places
under this root, could not reach its own worktree, and could not read #213's ZDOTDIR scrub when
placed here either.
Fix: add() now calls a new shareRootWithGroup(root) right after creating the root, chgrp+chmod
g+x on the root itself (non-recursive — each child is still shared individually by its own call
site). No-op when worktreeGroup is unset, so behaviour is byte-identical in today's only live
mode. On failure (group missing, or operator not a member of it) the spawn is refused with a
WorktreeException naming the root, its current mode, and the group — mirroring shareWithGroup's
existing refusal shape — before `git worktree add` ever runs, so no partial worktree is left
behind.
Also adds the assertion the #221 reviewer flagged as missing: a test driving
EnvAllowListScrub#shareWithGroup directly against a directory holding several flat files
(opencode.json, member-charter.md, ide-rules.md, plus an unrelated one) and asserting every one
of them gets group-readable/never-group-writable permissions, not just the two files someone
happened to think of.
#225: OpenCodeLauncherTest/HerdrPeerLauncherAllowListWiringTest's currentUserGroup() read the
group that owns the current working directory, not the process's own primary group, despite its
comment claiming the latter. Those coincide only by accident: a home checkout is typically owned
by a group the operator belongs to (staff), while a checkout under /private/tmp on macOS is
group wheel, which the operator is usually not a member of — so the same test fails for real
depending on where the repo happens to be checked out, and the existing assumeTrue only guarded
against "no POSIX groups at all", never "a resolvable but wrong group". Fixed by resolving the
process's REAL primary group via `id -gn` instead, with assumeTrue (skip, not fail) only when
that itself cannot be resolved on the host. The permission assertions these tests exist for are
unchanged.
Verified `mvn clean install` green from both a home checkout and a /private/tmp copy (mirroring
the exact repro in #225): 1093 tests, 0 failures, 0 errors in both locations.
ClaudeCodeLauncher#writeCharterFile used Files.createTempFile with no
directory argument, which resolves against fleetd's own java.io.tmpdir
(macOS: the per-user $TMPDIR, mode 0700). Under memberHerdrSocket: the
member pane runs as a different OS user and cannot read that directory,
and since #220 the charter file is the ONLY delivery path for
--append-system-prompt-file. Following #219's refusal decision (a
charter is the member's turn contract, not a degradable control): with
memberHerdrSocket configured, the charter now goes into a fresh
per-spawn directory under worktreeRoot, shared read-only via
EnvAllowListScrub.shareWithGroup (reusing #213/#219's mechanism); a
missing worktreeRoot/worktreeGroup refuses the spawn by name instead of
writing an unreadable file. With memberHerdrSocket absent the path is
unchanged.
An explicit-profile spawn bypasses role-pool placement (CompositePeerLauncher
only constrains an UNQUALIFIED spawn to fleet.<role>), so it was the one path
that could ask for role=architect on a profile no architect slot carries.
MemberRegistry silently held the session as a plain worker while GET /members
still reported the requested "architect" and only fleet_whoami (which reads
live bindings, not the request) told the truth.
- MemberLifecycle.requireSlotFor(role, profile): refuses the acquire before
anything spawns when no configured architect slot carries the profile,
naming the role, the profile, and the pools that do carry it. No-op for
dev/reviewer, which are placement candidates only, never a live identity
binding — refusing a profile mismatch there would break the documented
fleet_spawn{profile:"opus"} (role defaults to dev) flow.
- MemberLifecycle.acquired(...) now returns the role the session actually
holds, so a residual race (a slot exists but every instance is already
bound to a different terminal) still falls back to dev honestly instead of
lying — this case logs at WARN (was INFO), naming profile and terminal.
- SessionManager now records the role acquired() returns on MemberSession,
never the requested role, so GET /members and fleet_list can no longer
report a role the member does not hold; no changes needed to memberView/
rosterView, which just read session.role().
An architect's identity IS the slot it is bound to — binding a role with no
slot to bind means inventing an identity out of nothing, which is the quiet
failure the whole role system exists to prevent.
Tests: SessionManagerTest and FleetMcpTest each drive a real spawn through
FleetMcp.spawn -> SessionManager.acquire -> the real ClaudeCodeLauncher (via
FakeHerdr), then assert on GET /members and fleet_whoami for that same
session — not on MemberRegistry.bind directly (fleetd issue #113's mistake).
Site 1 (config root): under memberHerdrSocket, writeConfig() now places the
ephemeral opencode.json directory under worktreeRoot and shares it read-only
with worktreeGroup, reusing EnvAllowListScrub#shareWithGroup (widened to
package-private and generalized) — the same mechanism #213 built for the
ZDOTDIR scrub, rather than a second copy. Unlike the ZDOTDIR scrub's
degrade-to-overlay fallback, a missing worktreeRoot/worktreeGroup here
REFUSES the spawn (IllegalStateException from buildLaunch): this file is the
member's only way to learn where the bridge MCP is, so writing it somewhere
unreadable would just produce an undeliverable member with no signal
pointing at the cause. memberHerdrSocket absent stays byte-identical.
Site 2 (discovery root): under memberHerdrSocket, agentSessionId() now
declares session discovery unavailable and logs one WARN per launcher
instance instead of silently scanning fleetd's own $HOME (opencode.db lives
under the MEMBER's home under this config key). Decision + reasoning for why
this is a declare-unavailable rather than a new config key is in
defaultDiscoveryRoot()'s javadoc.
Widened HerdrPeerLauncher#memberHerdrSocketConfigured/memberScrubParentDir/
memberGroup to package-private so OpenCodeLauncher reuses the exact same
config resolution rather than re-deriving it.
Same-shape finding (not fixed, out of scope): ClaudeCodeLauncher#writeCharterFile
(line ~465) writes the role-charter temp file via Files.createTempFile with no
directory argument, i.e. under java.io.tmpdir — the same site-1 shape, unfixed
for the Claude Code adapter.
herdr does not exec a member's launch command — it TYPES it into the pane,
and a pty line buffer holds 1024 bytes (BSD/macOS MAX_CANON). Past that the
tail is dropped and NOTHING reports it: herdr answers "agent started", the
backend exits on the mangled argument it was handed, the pane closes, and the
only symptom is the readiness gate timing out 20 seconds later with no reason.
That is what broke every claude-code spawn after #214. The reply charter rode
inline on --append-system-prompt, so the command was already 978 bytes; adding
--session-id <uuid> made it 1028, and the 4 bytes cut off the end turned
--autocompact 250000 into --autocompact 25, which claude rejects. Measured on
the live pane, the cut is at byte 1024 exactly.
- ClaudeCodeLauncher: the charter ALWAYS travels as --append-system-prompt-file.
The file path already existed for the two-charter case; the inline form only
ever saved a temp file, and it cost ~800 bytes of the line budget. This takes
the prose off the command line for good.
- HerdrPeerLauncher.checkPaneCommandFits: refuse a command that cannot fit,
naming the byte count and the longest argument, instead of spawning something
that cannot work. The estimate is deliberately conservative — fleetd cannot
see herdr's quoting, and an under-estimate would let the silent truncation
back in.
- HerdrPeerLauncher.waitUntilInjectableOrThrow: log the pane tail and the last
herdr status BEFORE stop() closes the pane. Without it the gate reports only
that it timed out, which is true of every cause. This is what found the bug,
and it stays.
The guard also catches a case that was already over the limit: a profile with
ideMcpUrl set assembles 1084 bytes. It is now impossible to ship that silently.
3 tests, all watched failing first: with the inline charter restored the guard
fires in the new fit test, in the pre-existing autocompact test and in the IDE
mount test. Full suite 1081 tests green. Proven live: sonnet spawns again, the
member obeys the file-delivered charter and ends its turn with fleet_reply, and
fleet_list reports the #214 agentSessionId.
The normal backend-error path appends the pane tail to the failure reason on
purpose (fleetd#164): the BACKEND_ERROR pattern is a heuristic, and a member
that reported *about* an error while forgetting fleet_reply matches it too, so
dropping the rest of the pane destroys the report.
The new raw-scrape fallback did not do that. It matters more there, not less:
the fallback only runs when the trimmed assistant block was empty, so the raw
scrape is the ONLY copy of whatever the member managed to say. A lead read the
matched line and nothing else.
Clipped to the same cap the normal path uses, since a raw screen has no
boundary trimming to bound its size.
The assertion was watched failing without the fix:
AssertionFailedError: fleetd#164: the failure must carry the pane, not only
the matched line ... expected: <true> but was: <false>
mvn clean install: Tests run: 1079, Failures: 0, Errors: 0, Skipped: 0
The memberCredentials.policy: allow-list ZDOTDIR scrub decided zsh-vs-not
using fleetd's own process $SHELL and wrote the generated scrub dir into
fleetd's own java.io.tmpdir. Under memberHerdrSocket: (member panes run as
a different OS user than fleetd's own process) this silently protects
nothing: the wrong shell decides the gate, and the directory can be
unreachable to the member.
- New FleetConfig.memberLoginShell: the member OS user's login shell,
only ever read when memberHerdrSocket: is configured; fleetd's own
$SHELL keeps deciding everything when memberHerdrSocket: is absent
(byte-identical to before).
- HerdrPeerLauncher.applyEnvironmentAllowListPolicy: memberHerdrSocket +
memberLoginShell not configured/non-zsh falls back to the CB-596
sentinel overlay (warn loudly, never refuse to spawn). memberHerdrSocket
+ zsh memberLoginShell generates the ZDOTDIR under the configured
worktreeRoot instead of java.io.tmpdir, shared with the existing
worktreeGroup (reused, not a new key).
- EnvAllowListScrub: new generate(parentDir, allowedNames, group) overload
shares the generated directory via pure-Java POSIX group ownership
(rwxr-x--- dir, rw-r----- files) — no external process spawn.
- fleetd.example.yaml documents memberLoginShell: and worktreeGroup:'s
reuse for the scrub directory (the live fleetd.yaml is gitignored).
4 new tests in HerdrPeerLauncherAllowListWiringTest cover the acceptance
criteria; 3 of the 4 were watched failing against the pre-fix code.
CompletionResolver.resolve() returned an empty-scrape failure before the
BACKEND_EXHAUSTED / BACKEND_ERROR classification ever ran, whenever
lastAssistantBlock() found no usable text — most commonly a pane with no ⏺
marker at all, whose boundary scan starts at the top of the raw screen and
breaks immediately on the first line of TUI chrome. Since BACKEND_EXHAUSTED
is the only caller of exhaustionSink, this meant an exhausted backend was
recorded as "produced nothing" instead of being quarantined.
Fix: run the same two classifications against the raw (untrimmed) scrape as
a fallback, only inside the empty-scrape failure branch. A pane that already
yields a usable assistant block never reaches this branch, so the existing
narrow match is unchanged. lastAssistantBlock stays the sole source of the
reply text; only classification ever consults the raw scrape.
abandon() drained the target's stranded reply once and then reused that same
Reply for every open task it walked past. Two open tickets on one target
therefore both came back REPLIED with the same text — one of them a reply the
worker never gave for that delegation.
One worker answer can settle at most one delegation. It now goes to the oldest
open task (lowest createdNanos) and every other open task keeps the ordinary
WORKER_FAILED path. If the chosen task turns out to be already resolved by
another path, the drained reply is published back to the inbox instead of being
dropped.
reply()'s matching side gets the same rule: more than one candidate means the
reply goes to the inbox rather than to a guess.
Reachability, checked rather than assumed:
- matching.size() >= 2 alone is reachable and was a real bug before this change.
- More than one candidate in reply() is not reachable today — hasAsyncQuestion
matches any task with a stamped turnId, and answer()'s
clearAsyncQuestion(turnId, false) leaves that stamp until the resumed turn
resolves. Kept as defensive code, documented, no test seam added.
- matching.size() >= 2 together with a live strand is not reachable either:
send() and answer() are the only two lock holders and both open a Rendezvous
waiter inside the lock, so "lock held" and "waiter open" are one fact, and an
acceptance always clears the strand first.
I checked the last point by building it in a scratch worktree: it can be forced
by closing an accepted send's waiter directly through Rendezvous, and it does go
red against the pre-fix loop — but that breaks the lock-and-waiter invariant
from outside the class, so no such test is added. A note in abandon()'s javadoc
says so, to save the next reader the same round trip.
Worker's REPORT-cb137.md left out of main.
mvn clean install: Tests run: 1071, Failures: 0, Errors: 0, Skipped: 0
Defect 1 (reply()'s askAnsweredAsyncTasks returning >1 candidate): confirmed
unreachable today. Documented why in three places — hasAsyncQuestion matches
any task with a stamped turnId (not just an open question), and answer()'s
clearAsyncQuestion(turnId, false) leaves that stamp in place until the
resumed turn's own future resolves — so a second task can never reach the
same eligible state while a first one holds it. Kept the defensive
inbox-fallback branch as defence in depth against that guarantee weakening,
per review instruction; no test seam added.
Defect 2 (abandon()'s broader `matching` filter applying a stranded reply to
more than one task): matching.size() >= 2 alone IS reachable (already
covered by abandonFailsEveryPendingAsyncTicketForTheReleasedTarget) and was
a real pre-fix bug (97f6c33's parent reused one drained reply for every
matching task). But hadStrandedReply == true together with matching.size()
>= 2, at the instant abandon() runs, is not constructible through the public
API: send() and answer() are the only two sites that ever hold a target's
session lock, and both open a Rendezvous waiter for that target as the first
thing they do while holding it — so "lock held" and "waiter open" are the
same fact throughout this class, and reply()'s fast path always resolves an
open waiter directly instead of stranding. A strand can only be created
while no task is accepted, and the moment the lock is next taken, that
acceptance clears the strand again before abandon() can observe both facts
together. Documented this in abandon()'s javadoc and removed the earlier
attempt at a deterministic test for the conjunction, whose apparent failure
was an invalid premise (the "accepted" task's own acceptance silently
cleared the strand it was meant to race against), not the fix being absent.
The CB-205-recovery fix in #205 assumed a target has at most one open
async task, with no guard. Fix three consequences:
- reply(): askAnsweredAsyncTask -> askAnsweredAsyncTasks (List). Exactly
one candidate completes it (unchanged). Zero falls to the inbox
(unchanged). More than one now ALSO falls to the inbox instead of
picking an arbitrary ConcurrentHashMap iteration order, and logs a
WARN naming the target and every candidate ticket.
- abandon(): a stranded reply now settles at most one matching task —
the oldest by Task#createdNanos (a new field, the tiebreaker). Every
other matching task keeps WORKER_FAILED, same as today.
- abandon(): if the chosen recovery task's complete() loses a race
(another path resolved it first), the drained reply is republished
to the inbox instead of being silently dropped.
Single-task behavior is unchanged; only the ambiguous case changes.
SessionManager called handle.agentSessionId() once at spawn and froze it in the
immutable MemberSession. For opencode that value is always null - the session
row does not exist yet when the pane is created - so fleet_list never reported
an agentSessionId and fleet_spawn{resumeSessionId} was unusable for that
backend. The handle's javadoc said 'the caller re-calls later'; no caller did,
and SessionManager did not even retain the handle.
Retain the PeerHandle per pane and re-resolve while the stored id is still
null, sticky once found, CAS-swapped into the registry.
roster() stays non-resolving - it is the roster supplier for LeadHeartbeatLoop
and FleetHealthMonitor, and resolving there would open opencode's 841MB SQLite
database on every tick, for every unresolved member, forever. rosterResolved()
carries the resolve and is used only by fleet_list and the REST roster, the two
surfaces that report the id. get(paneId) and release() resolve too, both
caller-driven.
A test pins the split: the plain roster() must never call agentSessionId()
again.
Under memberHerdrSocket: member panes run as a different OS user, so
hostEnvNames (fleetd's own environment) no longer describes what a member
inherits. logCredentialGap now reports "unknown, not clean" in that mode
instead of its usual UNBLOCKED/blanked conclusions, once per launcher, naming
the config key and scoping its count to fleetd's own process.
Byte-identical when memberHerdrSocket: is absent, pinned by a test.
The endpoint became /members in the CB-634 rename but the body key stayed
"workers", so a caller that read "members" saw an empty fleet and reported
no members at all.
Emit the canonical "members" key. Keep "workers" as a deprecated alias so an
existing REST consumer keeps working - the out-of-band path a lead falls back
to when its MCP mount drops reads this endpoint.
The test now pins both keys and asserts they carry the same rows, so the alias
cannot silently drift. Watched failing without the fix:
"GET /members must return its rows under \"members\" ==> expected: not <null>"
roster() is the supplier for LeadHeartbeatLoop and FleetHealthMonitor
(both timer-driven) and for placement/exhaustion checks and the
metrics scrape — none of which read agentSessionId. Resolving there
meant every tick could open a lazy-resolving adapter's (opencode's)
on-disk session database once per member whose id was still unknown,
with no bound: a member whose id never appears would pay that cost for
the life of the process.
roster() goes back to its pre-#209 behavior (no resolve, no I/O). A
new rosterResolved() carries the resolve logic, and is used only by
the two surfaces that actually report agentSessionId to a caller:
fleet_list (FleetMcp.listFleet) and the REST roster
(FleetApp.listMembers). fleet_whoami's roster().stream() at
FleetMcp.java:768 does not surface the field, so it stays on the plain
roster(). get(paneId) (fleet_status) and the release() resolve are
caller-driven, not timers, and are unchanged.
Retargeted the roster-facing tests from #209 at rosterResolved(), and
added plainRosterDoesNotResolveAgentSessionId, which pins the split by
asserting the handle's agentSessionId() is not called again by
roster().
When memberHerdrSocket: is configured, member panes run under a different OS
user than fleetd's own process, so hostEnvNames (fleetd's own environment)
no longer describes what a member pane inherits. logCredentialGap now checks
for that config key and, when set, logs a single WARN saying the gap is
UNKNOWN (not clean) and names the key, instead of printing the "inherits
them UNBLOCKED" / "the scrub blanks them" conclusions as fact. Behaviour is
byte-identical when memberHerdrSocket is absent (the default and only mode
this host runs).
SessionManager used to call PeerHandle.agentSessionId() exactly once at
spawn and freeze the answer into the immutable MemberSession. For
opencode that call always came back null, because opencode has not
written its on-disk session row yet when the pane is created, and no
caller ever re-asked the handle — it went out of scope at the end of
the spawn method. fleet_list therefore never reported agentSessionId
for an opencode member, and resumeSessionId was unusable for it.
Retain each spawn's PeerHandle in SessionManager, keyed by paneId, and
re-resolve a still-null agentSessionId against it from roster(), get(),
and release() (so a released member's detail also carries a
late-resolved id). Resolution is bounded: only sessions with a still-
null id do any work, a resolved id is never looked up again, and a
throwing handle degrades to "unresolved" rather than breaking the
caller. MemberSession gains a withAgentSessionId wither in the same
style as withState/withActivity.
Stage 3 of #185. A provisioned worktree and the repo's git store are made
group-writable when worktreeGroup names an OS group; absent, nothing changes.
The share pass runs after overlayParity, not inside add(), because
overlayParity copies more files in after add() returns.
This isolates credentials, not the repository: a member in the group can
still write the operator's git objects and refs.
opencode migrated its session store from a JSON file tree to SQLite in
January. OpenCodeSessionDiscovery still scanned the frozen tree, so it
returned null for every member: agentSessionId was never known and
resumeSessionId silently did nothing for every opencode profile, through
57 member spawns, with nothing logging that the search found nothing.
Dai Ha
ltms