Compare commits

..

43 Commits

Author SHA1 Message Date
Dai Ha 0373b6c41b #213: fix ZDOTDIR credential scrub gate + directory under memberHerdrSocket
CI / contract (pull_request) Successful in 1m10s
CI / build (pull_request) Successful in 2m17s
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.
2026-09-01 11:07:11 +07:00
Dai Ha 966c58a3b8 #137: don't hand one stranded reply to every open async ticket (#215)
CI / contract (push) Successful in 1m31s
CI / build (push) Successful in 1m41s
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
2026-08-31 23:23:07 +07:00
Dai Ha de026b8f8a #137 follow-up: document defect-1 and defect-2 reachability, drop unconstructible test
CI / contract (pull_request) Successful in 48s
CI / build (pull_request) Successful in 1m28s
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.
2026-08-31 23:17:50 +07:00
Dai Ha 97f6c33a45 #137: don't guess when a target has more than one open async task
CI / contract (pull_request) Successful in 1m4s
CI / build (pull_request) Successful in 1m17s
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.
2026-08-31 22:46:23 +07:00
Dai Ha 735c837604 #209: resolve agentSessionId lazily via the retained PeerHandle
CI / contract (push) Successful in 48s
CI / build (push) Successful in 2m18s
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.
2026-08-31 22:21:16 +07:00
Dai Ha 457dc0330d #185 stage 2: the credential gap detector must not report on the wrong environment
CI / contract (push) Successful in 55s
CI / build (push) Successful in 1m59s
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.
2026-08-31 22:17:40 +07:00
Dai Ha a1a9015217 #199: /members returns rows under "members" 2026-08-31 22:17:35 +07:00
Dai Ha 5a811a3695 #199: GET /members returns its rows under "members", with "workers" kept as an alias
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>"
2026-08-31 22:17:22 +07:00
Dai Ha 1fdaa74eb3 fleetd #209 follow-up: keep roster() off the resolve path
CI / build (pull_request) Successful in 1m20s
CI / contract (pull_request) Successful in 1m21s
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().
2026-08-31 22:15:16 +07:00
Dai Ha 7d4a4339c2 Merge branch 'worker/cb-137-ask-ticket-e7760c-2' of https://git.ltms.dev/fleet/fleetd into worker/cb137-ambiguous-task-4df3d8-4 2026-08-31 22:13:37 +07:00
Dai Ha 847e8bd3fa #185 stage 2: stop the credential-gap detector reporting on the wrong environment
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Successful in 1m39s
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).
2026-08-31 22:12:56 +07:00
Dai Ha f9fb387427 fleetd #209: re-poll agentSessionId against the retained PeerHandle
CI / contract (pull_request) Successful in 45s
CI / build (pull_request) Successful in 1m40s
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.
2026-08-31 22:04:20 +07:00
Dai Ha a55079afbd #185: opt-in worktreeGroup, so a member running as another OS user can write its worktree
CI / contract (push) Successful in 50s
CI / build (push) Successful in 1m39s
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.
2026-08-31 21:47:03 +07:00
Dai Ha e18ad4723b Merge remote-tracking branch 'refs/remotes/origin/cb206' 2026-08-31 21:47:03 +07:00
Dai Ha 6de8ac8972 #206: read opencode session ids from opencode.db, and pin the read-only open
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.
2026-08-31 21:46:32 +07:00
Dai Ha a052975420 #206: pin the read-only open with a test that actually fails without it
CI / build (pull_request) Successful in 1m18s
CI / contract (pull_request) Successful in 1m44s
setReadOnly(true) is the whole thing keeping fleetd out of the operator's
live 841MB opencode.db, and no test failed when it was removed.

The obvious test does not work. Making the database file unwritable and
checking the read still succeeds passes either way, because SQLite silently
downgrades a read-write open of an unwritable file to read-only. I wrote that
test, watched it pass with the flag removed, and threw it away.

What works: extract a package-private openReadOnly(), then ask that connection
to INSERT and require the refusal. Watched red with the flag removed, green
with it restored.

Also switches the test's INSERT helper to a PreparedStatement -- hand-escaped
SQL in a test is a pattern that gets copied into main code.
2026-08-31 21:46:07 +07:00
Dai Ha 6d82ca95a4 #185: skip absent git paths, and resolve the git dir instead of assuming .git
CI / contract (pull_request) Successful in 56s
CI / build (pull_request) Successful in 1m38s
Two defects in the stage-3 share pass, both of which would have failed EVERY
provisioning spawn once worktreeGroup was set, not only the two-user case.

.git/logs was handed to chgrp unguarded while packed-refs was guarded. It does
not exist with core.logAllRefUpdates=false, or before the first ref update, and
chgrp on a missing path exits non-zero -- surfacing as a WorktreeException that
blames a group which is in fact fine. Every path is now skipped when absent.

repoRoot + "/.git" was hardcoded. That is a FILE, not a directory, when the
checkout is itself a linked worktree -- the very thing this class creates for
every member. It now asks git: rev-parse --git-common-dir, resolved against
repoRoot because git answers relatively for an ordinary checkout.

Both new tests were watched failing with the fix removed before being kept.
2026-08-31 21:34:49 +07:00
Dai Ha 8067ee4ec4 fleetd #185: opt-in worktreeGroup config for group-shared worktrees
CI / contract (pull_request) Successful in 1m7s
CI / build (pull_request) Successful in 1m45s
Adds worktreeGroup (top-level FleetConfig key), Worktrees.shareWithGroup
(GitWorktrees impl: git config core.sharedRepository group + one-time
chgrp/chmod g+rwX/setgid fix-up over the worktree, .git/objects, refs,
logs, worktrees, and packed-refs when present), and wires SessionManager
to call it AFTER overlayParity so overlay files are covered too. Off by
default (byte-identical behaviour when unset). Documents the
credentials-not-repository caveat in the javadoc and example config.
2026-08-31 15:53:01 +07:00
Dai Ha 3743789e8d fleetd #206: read opencode session ids from opencode.db (SQLite), not the frozen JSON tree
CI / contract (pull_request) Successful in 1m5s
CI / build (pull_request) Successful in 1m44s
opencode migrated its session store to SQLite in January 2026; the JSON tree under
storage/session/<projectID>/ses_*.json stopped being written, so OpenCodeSessionDiscovery
returned null for every member forever, and fleet_spawn{resumeSessionId} was unreachable.

- Add org.xerial:sqlite-jdbc 3.53.4.0, opened read-only (SQLiteConfig.setReadOnly), so it
  never disturbs a live opencode process writing the WAL-mode database.
- Rewrite sessionIdForDirectory to run a parameterized SELECT ... WHERE directory = ?
  ORDER BY time_updated DESC LIMIT 1 against the session table. Still never throws: a
  missing database, a locked/corrupt one, or no matching row all return null.
- Log the silence that let this go unnoticed: WARN once per instance when opencode.db
  itself is missing (the layout moved again), DEBUG when it exists but no row matches
  yet (the normal interim answer right after a spawn).
- Replace the JSON-fixture tests with a synthetic-SQLite-db fixture; delete the tests
  that only proved the old JSON scan worked.
2026-08-31 15:52:06 +07:00
Dai Ha 388aba7632 #137: an answered turn's reply completes its own ticket, instead of a false failure
CI / contract (push) Successful in 1m12s
CI / build (push) Successful in 1m17s
A wait:false delegation whose worker used fleet_ask ended with fleet_poll{ticket}
reporting 'the worker session was released before it replied' -- naming a
worktree, a branch and a snapshot commit, so it read as lost work. The worker had
in fact replied in full.

The ticket guessed the ask rendezvous detached the turn. The real cause is
narrower: answer() (behind fleet_send{turnId}) waits only for the lead's own
bounded MCP call window. A resumed turn doing real work -- edits, a build, a
push, a PR -- routinely outlives it. On timeout answer()'s finally closed the
waiter, so the worker's later fleet_reply found none and fell to the session
inbox, leaving the ticket's future unresolved until fleet_stop forced it FAILED.

reply() now looks for the async task parked on this exact answered turn and
completes it with the real reply. That is safe against completing the wrong
ticket: answer() calls clearAsyncQuestion(turnId, false), so the task keeps its
turnId and stays in asyncTasksByTurn, and hasAsyncQuestion therefore still makes
send() return BUSY for a second async send to that target. At most one candidate
task can exist per target.

abandon() keeps an independent check: if a reply was stranded, a released session
reports REPLIED with that text rather than a failure -- so the recovery hint that
implies lost work never prints once a reply exists.

Verified before merging: build green unpiped, and both new tests drive the full
delegation path (send async, ask, answer, reply, poll the ticket) rather than
handing a Reply to a sink, which is the trap this ticket called out.

Co-authored-by: fleetd worker <worker@ltms.dev>
2026-08-31 14:24:54 +07:00
Dai Ha ad587eafa3 #172: keep the broker URI, password and all, out of every member pane
CI / contract (push) Successful in 1m10s
CI / build (push) Successful in 1m15s
broker.uriEnv names an environment variable holding amqp://user:password@host,
and it was reaching every member. Its name is not credential-shaped -- no TOKEN,
KEY or SECRET in it -- so every name-pattern heuristic missed it, and it sat on
neither credential list.

fleetd already knows the name: the operator wrote it in broker.uriEnv. So derive
the exclusion from the config rather than hoping an operator also remembers to
deny it. coordinator.uriEnv has the same shape and is excluded too; on this host
both resolve to the same variable.

Excluded even when the operator lists the name under memberCredentials.allow:,
following the SSH_AUTH_SOCK precedent. There is no override, because a member has
no legitimate use for the broker password.

Reviewer finding, recorded rather than overstated: this is only a hard guarantee
under policy: allow-list, where the ZDOTDIR scrub runs after the pane's shell has
sourced the operator's chain. Under deny-list the name is removed from the
pre-shell env only, and a login shell re-exports it. That is deny-list's existing
weakness rather than a regression here, but the javadoc now says so plainly
instead of implying a guarantee that path cannot give.

Co-authored-by: fleetd worker <worker@ltms.dev>
2026-08-31 14:21:48 +07:00
Dai Ha 08ce9aef11 #161: resolve a pane by process ancestry, closing a worker->primary escalation
CI / contract (push) Successful in 50s
CI / build (push) Successful in 1m52s
PaneLocator's javadoc always claimed it found 'the agent pane whose process
tree contains' a pid. It did not: paneOwnsPid matched only the pane's shell_pid
and its foreground_processes. A process a member spawned -- python3, curl, any
helper opening its own connection to 127.0.0.1:8765 -- matched no pane, so
CallerResolver fell through to loopback-trust and resolved it as the PRIMARY.
A member escalated to lead by shelling out.

terminalForPid now builds the caller's ancestor set once (bounded at 32
generations, with a cycle guard) and matches any ancestor against a pane's pids.
The set is reused across both herdr clients on the CB-185 two-daemon path.

This only ever ADDS matches, which is the safe direction: the failure mode of
the fix is a member correctly restricted, while the failure mode of the bug is a
member acting as the lead. The no-match case still returns null, so the lead --
which maps to a pane named by leaders: -- still resolves as primary.

Ancestry is walked through a new ParentResolver seam so the tests drive it from
a fake pid->parent map rather than spawning real processes.

Co-authored-by: fleetd worker <worker@ltms.dev>
2026-08-31 14:15:39 +07:00
Dai Ha ee5f8b932b #137: complete an async ticket's own reply after answer() times out
CI / contract (pull_request) Successful in 49s
CI / build (pull_request) Successful in 1m44s
fleet_send{turnId} (MessageService.answer) blocks the primary only for its
own bounded MCP-call window (25s default, 120s max) — far shorter than a
worker's resumed turn can genuinely take. When that window expires, answer()
closes its rendezvous waiter, so the worker's eventual fleet_reply has no
live waiter to resolve and falls back to the session inbox. The async
ticket's future was never completed by that path, so fleet_poll{ticket}
stayed PENDING until fleet_stop's abandon() forced it FAILED with a
misleading "the worker session was released before it replied" reason,
even though the reply had genuinely arrived.

- MessageService.reply(): before falling to the inbox, look for the async
  task this exact turn belongs to (already answered — question cleared,
  turnId still stamped — but not yet resolved) and complete it directly
  with the real reply, so fleet_poll{ticket} returns it.
- MessageService.abandon(): defense in depth, independent of the above —
  never write a false WORKER_FAILED once a reply reached the inbox for
  this target; recover and use its real content instead.
- Two new tests drive the full delegation path (async send -> ask ->
  answer with a short timeout -> reply -> poll/abandon), not a reply sink
  directly; both fail with the fix disabled and pass with it restored.
2026-08-31 14:12:23 +07:00
Dai Ha 4ac688b6d9 #164: classify a backend-error scrape as WORKER_FAILED, carrying the whole pane
CI / contract (push) Successful in 48s
CI / build (push) Successful in 1m22s
main already shipped the core of #164 in 3bfa828: the MIN_TURN_NANOS floor and
the hard fail on an empty or unreadable scrape. This adds the one case that was
still resolving as a success -- a scrape that reads cleanly but whose content is
the backend's own rejection (e.g. "API Error: 400 invalid request body").

The BACKEND_ERROR pattern is deliberately narrow. A growing list of ad-hoc error
strings rots as backends change their wording, and broader backend-error
surfacing is #164 point 3.

Because the pattern is a heuristic, it also matches a member that forgot
fleet_reply while reporting *about* a backend error. So the failure reason
carries the whole pane tail, not just the matched line: a genuine backend error
reads as before, and a false positive keeps its report instead of losing it.

Checked before merging: 3bfa828 is an ancestor of main; the branch was current
with main; mvn clean install green unpiped (1037 tests, 0 [ERROR] lines); and
each of the 5 new tests fails with the fix commented out.

Co-authored-by: fleetd worker <worker@ltms.dev>
2026-08-31 10:56:40 +07:00
ltms a814d1ef00 #197: measure the async ticket TTL from completion, not from creation
CI / build (push) Successful in 1m15s
CI / contract (push) Successful in 1m17s
Lead-authored and lead-verified: full mvn clean install green at 1032 tests, and the new test proved by restoring the old comparison and watching it fail.
2026-08-31 05:10:46 +02:00
Dai Ha ea98856130 #197: measure the async ticket TTL from completion, not from creation
CI / contract (pull_request) Successful in 54s
CI / build (pull_request) Successful in 1m32s
pruneTerminalTickets compared the cutoff against createdNanos, so the real
window to collect a reply was "TTL minus however long the task ran". A
delegation that ran longer than the 10-minute TTL was already past the cutoff
the moment it finished, so the next prune destroyed its reply.

That is the normal case here, not an edge case. Real delegated work runs well
past ten minutes. Three workers in one session did, and two of their complete
reports were lost. The reply lives only in Task.future, so pruning it discards
the worker's whole report, and fleet_poll{target} returns [] rather than
holding it — there is no fallback.

Task now stamps completedNanos from a whenComplete hook registered in its
constructor, so every completion path stamps it (a reply, the completion
fallback, a timeout, a failure, an abandon on teardown) without each one having
to remember to. The stamp is a boxed Long, not a long with a sentinel:
System.nanoTime may return any value, so no number can mean "not stamped yet".
A task that is done but not yet stamped is left for the next sweep.

createdNanos had no other reader and is removed.

The TTL still bounds tasks — an uncollected finished ticket is still evicted
once the TTL passes since it finished. Both halves are pinned by a test, and
the first one was proved by restoring the old comparison and watching it fail.

Tests run: 1032, Failures: 0, Errors: 0, Skipped: 0
2026-08-31 10:10:16 +07:00
ltms a49e96835a CB-189: cover every remote, both URLs, and any non-SSH scheme in the credential check
CI / contract (push) Successful in 44s
CI / build (push) Successful in 1m11s
Lead-verified: merged onto current main (which already carries #194 and #196), full `mvn clean install` green at 1030 tests. Confirmed no plain `exec` call that reads a remote URL remains.

Review round 2 closed the half-fix: execRedacted was added but applied only to the new calls, leaving the three pre-existing URL readers (lines 143, 171, 334) still copying stdout into exception messages — the exact hole CB-189 named.

Kept the check before the origin strip, on the worker's reasoning: the credential really is in the config at that moment, so WARN-found followed by INFO-fixed is the full audit trail, whereas moving it after would silence the origin case entirely.
2026-08-31 04:32:14 +02:00
ltms 63c19dcba7 CB-185: fix two blockers to switching on memberHerdrSocket
CI / build (push) Successful in 1m31s
CI / contract (push) Successful in 1m19s
Lead-verified: merged with #194 onto an integration branch off main, full `mvn clean install` green at 1025 tests.

Review round 2 fixed the stale ambiguous-pane message and, more importantly, a real abort: probeOwner called list() unguarded, so one unreachable daemon made panes on a different healthy daemon un-stoppable too — the same bug blocker 1 exists to fix, through a new door. Worker proved it by removing the guard and quoting the failure.
2026-08-31 04:30:57 +02:00
ltms 2823349c8e CB-192: fix false credential-gap WARN under allow-list+zsh, split its log guard
CI / contract (push) Successful in 44s
CI / build (push) Successful in 1m11s
Lead-verified: merged with #196 onto an integration branch off main, full `mvn clean install` green at 1025 tests. Deny-by-default WARN confirmed byte-identical to main.

Review round 2 fixed a defect I found in round 1: the new INFO asserted that gap names were not on the derived allow-list without ever checking, so a name derived from a profile's tokenEnv/gitTokenEnv/env: would be reported as safe while the member actually inherited it. Now split with MemberEnvAllowList.keeps — the same predicate the generated scrub evaluates.
2026-08-31 04:30:48 +02:00
Dai Ha 6fc301d62c CB-189 review fix: redact the three pre-existing URL-reading exec calls
CI / contract (pull_request) Successful in 1m11s
CI / build (pull_request) Successful in 1m42s
Review found that execRedacted was applied only to the new remote-enumeration code
and left three pre-existing calls reading remote.origin.url through the plain,
unredacted exec: removeUserInfoFromHttpsOrigin, requireCredentialFreeHttpsOrigin, and
configureHttpsUrlRewriteForSshOrigin. A non-zero exit or timeout on any of those could
still have copied the credentialed URL into a WorktreeException message. Switches all
three to execRedacted; the set-url write in removeUserInfoFromHttpsOrigin is left on
plain exec with a comment explaining why (it writes the already-stripped URL, not a
read).

Widens the shared exec(Map, boolean, String...) overload to package-private, the same
test-seam pattern already used by the afterWorktreeAdded constructor parameter, and
adds a test that drives it directly with a synthetic failing command whose stdout
carries a marker (passed via env, not argv, so the always-printed command line can't
carry it) and asserts the marker never reaches the exception message.
2026-08-31 09:29:45 +07:00
Dai Ha bc99d64786 CB-185: fix ambiguous-pane message and unreachable-daemon abort in probeOwner (review)
CI / build (pull_request) Successful in 1m6s
CI / contract (pull_request) Successful in 1m5s
Lead review of PR #196 found two issues in CompositePeerLauncher.probeOwner:

1. The "more than one daemon claims this pane" throw kept the old
   pre-fix message ("no owning herdr daemon was recorded"), which was
   only true of the code it replaced. Reworded to say what actually
   happened: N configured herdr daemons report this pane, so it is
   genuinely ambiguous. Updated the one test pinning the old string.

2. probeOwner let list() propagate straight out of the probe loop, so
   one unreachable daemon aborted the whole probe and made a pane on a
   DIFFERENT, healthy daemon un-stoppable too — resurrecting the exact
   bug blocker 1 fixes. Now catches HerdrException per daemon, logs the
   exception class only, and treats that daemon as not knowing the pane
   so probing continues. New test proves this: verified it fails with
   the try/catch removed (HerdrException propagates and the stop that
   should succeed via the healthy daemon throws instead), then restored.

Full mvn clean install: 1019 tests, 0 failures, 0 errors.
2026-08-31 09:28:29 +07:00
Dai Ha 615af4ed0a CB-192 review fix: split the allow-list gap by what the scrub actually keeps
CI / build (pull_request) Successful in 1m5s
CI / contract (pull_request) Successful in 1m20s
Lead review on PR #194 found that the allow-list INFO wording claimed the
whole gap ("credential-shaped names on neither known: nor allow:") is blanked
by the scrub, without checking that against effectiveAllowed. effectiveAllowed
is a SUPERSET of known+allow — MemberEnvAllowList.derive also unions in every
profile's gitTokenEnv/gitHostEnv/tokenEnv/env: keys, and derivedAllowedNames
further unions in the spawn's own env keys — so a gap name can still be kept
by the derived list (e.g. a profile's tokenEnv names it) and reach the member
unblocked while the INFO said "no member pane keeps them". That inversion is
exactly what #192 exists to remove.

logCredentialGap now splits the gap with MemberEnvAllowList.keeps (the same
predicate the generated scrub itself evaluates, so this cannot drift from
what the scrub does): names it keeps get a WARN, guarded by the same
unprotectedGapLogged flag as the deny-by-default case (same severity — a name
reaching a member unprotected is equally serious either way); names it
blanks keep the existing INFO, guarded by allowListGapLogged. The
deny-by-default WARN text and the non-zsh fallback are untouched.

Added allowListWarnsWhenTheDerivedAllowListKeepsAnUncoveredName and
allowListSplitsAMixedGapBetweenTheWarnAndTheInfo to ClaudeCodeLauncherTest.
2026-08-31 09:28:10 +07:00
Dai Ha 045d229728 CB-185: fix two blockers to switching on memberHerdrSocket (#185)
CI / contract (pull_request) Successful in 46s
CI / build (pull_request) Successful in 1m39s
1. CompositePeerLauncher.stop() was permanently un-stoppable for any
   member that survived a daemon restart, because spawnedBy is in-memory
   only. On a cache miss with more than one configured herdr daemon, probe
   each distinct daemon's agent.list() for the pane instead of refusing
   outright: exactly one owner routes and caches; zero owners is treated
   as already-stopped (a no-op, matching the tolerance HerdrPeerLauncher
   already gives an already-gone pane); more than one owner is the
   genuine per-daemon-pane-id ambiguity and still throws.

2. FleetApp#healthz always reported the LEAD daemon's herdr version/
   protocol even when a second (member) daemon was configured, so a
   member-daemon protocol mismatch was invisible behind a green
   /healthz while every spawn silently failed. Added a separate "member"
   key alongside the unchanged "herdr" key, and a "protocolMismatch"
   flag when the two differ. Verified scripts/redeploy-fleetd.sh and
   scripts/rename-checkout.sh only check the HTTP status code and print
   the body verbatim — neither parses a specific field — so adding a key
   is safe.

Both fixes are covered by tests written to fail without the fix
(verified by reverting each fix and watching the new tests fail, then
restoring). Full `mvn clean install`: 1018 tests, 0 failures, 0 errors.
2026-08-31 09:20:14 +07:00
Dai Ha d1fd5700f5 CB-189: cover every remote, both URLs, and any non-SSH scheme in the credential check
CI / build (pull_request) Successful in 1m7s
CI / contract (pull_request) Successful in 1m7s
GitWorktrees only ever inspected origin's HTTPS fetch URL for embedded credentials. A
credential on any other remote, on a pushurl, or on a plain http:// URL passed through
unreported. Adds an additive, reporting-only check that enumerates every remote and both
its fetch and push URLs, flagging non-empty user-info on any non-SSH-family scheme.

The existing origin/https strip-and-refuse behaviour is untouched. The new check is
wrapped so it can never abort a provision, and on failure logs only the exception's
class, never its message, since the enumerating `git remote` call is not redacted.
Also adds execRedacted, an exec variant that never copies captured stdout into a
WorktreeException message, for commands whose stdout may itself be a credentialed URL.
2026-08-31 09:17:54 +07:00
Dai Ha 2d55b0b9a5 #168: correct the audit's MISSING claim — the feature is documented, its index row was not
CI / contract (push) Successful in 41s
CI / build (push) Successful in 1m42s
The memberHerdrSocket section exists at 11-Features.md:2174; what was absent was
its row in the index table. My omission when I added the section. Wiki fixed at
b24965c.
2026-08-31 09:17:41 +07:00
Dai Ha d89ae94a2e CB-192: fix false credential-gap WARN under allow-list+zsh, split its log guard
CI / contract (pull_request) Successful in 1m5s
CI / build (pull_request) Successful in 1m7s
logCredentialGap(creds) always emitted the WARN wording ("every member pane
inherits them UNBLOCKED"), even under memberCredentials.policy: allow-list on
a zsh login shell, where the generated ZDOTDIR scrub genuinely blanks the
name. The line reported the control working as though it were a hole.

Pass an effectiveAllowed set instead: null keeps the WARN (deny-by-default,
and the allow-list non-zsh fallback, where nothing is ever scrubbed); the
derived allow-list set (only reachable after applyEnvironmentAllowListPolicy's
own zsh gate) selects a new INFO wording that says the scrub will blank the
name instead of claiming it is inherited unblocked.

Also split the single credentialGapLogged AtomicBoolean into two guards
(unprotectedGapLogged / allowListGapLogged) — one per report kind. Since
memberCredentials is a live, re-read-per-spawn supplier, a shared flag let a
harmless allow-list INFO on one spawn permanently suppress a later spawn's
real deny-by-default WARN after a policy reload.

Fixes gitea #192.
2026-08-31 09:17:20 +07:00
ltms e6193c4098 Merge pull request '#168: audit current wiki snapshot' (#193) from worker/cb-168-wiki-audit-3ef3d1-3 into main
CI / build (push) Successful in 1m6s
CI / contract (push) Successful in 1m5s
2026-08-31 04:17:07 +02:00
Dai Ha b66f0677ed #168: audit current wiki snapshot
CI / contract (pull_request) Successful in 44s
CI / build (pull_request) Successful in 1m37s
2026-08-31 09:14:22 +07:00
ltms 23ada1981e CB-185: route members to a separate herdr daemon (#186)
CI / build (push) Successful in 1m6s
CI / contract (push) Successful in 10m30s
2026-08-29 01:24:31 +02:00
ltms a237fbff9d #185: refuse an unowned paneId when more than one herdr daemon could own it (#187)
CI / build (push) Successful in 1m4s
CI / contract (push) Successful in 1m17s
2026-08-29 01:10:21 +02:00
Dai Ha 31d5516991 #185 review: keep the stop owner on failure, key list() by daemon
CI / contract (pull_request) Successful in 43s
CI / build (pull_request) Successful in 1m30s
Three fixes on top of the pane-id PR, from my own read and the reviewer's:

- stop() removed the spawnedBy record BEFORE the delegate accepted the stop. A
  delegate that threw left the pane alive with its owner forgotten, so the retry
  fell into the ambiguous branch and refused the id for good. Remove after.
- list() deduplicated on the raw pane id. Pane ids are per-daemon counters, so
  two daemons can each hold w1:p1 on different panes, and one of the two real
  agents was silently dropped from fleet_list and every view built on it. The
  key is now (owning daemon, pane id). Delegates sharing one daemon still
  collapse, which is what the dedupe was for.
- The class javadoc still stated the single-herdr-connection premise as fact,
  next to the bullet this PR had just corrected for stop(). Fixed there too.

Also drops a redundantly qualified java.util.Collections.

Tests: 993 run, 0 failures, BUILD SUCCESS.
2026-08-29 06:09:43 +07:00
Ha Trong Dai 5ba05d0bdb #185: count herdr owners for stop fallback
CI / contract (pull_request) Successful in 43s
CI / build (pull_request) Successful in 1m8s
2026-08-28 09:42:24 +07:00
Ha Trong Dai 25726a5ae7 #185: reject ambiguous unowned pane ids 2026-08-28 09:36:39 +07:00
41 changed files with 3935 additions and 217 deletions
+208
View File
@@ -0,0 +1,208 @@
# Wiki audit for #168
**Source checked:** `.wiki-snapshot/` at `68e32c6` (2026-08-31). I did not use
`wiki/`. Code references below are from the current `fleetd` source tree. A quoted
line is a concrete claim that needs correction, unless the table says `KEEP`.
| Page | Verdict | One-line reason |
|---|---|---|
| `Home.md` | REVISE | Good overview, but it still names the retired product. |
| `_Sidebar.md` | REVISE | The heading still says `claude-bridge`. |
| `1-Architecture.md` | REBUILD | Its component contract mixes current names with removed tools, routes, and planned backends. |
| `2-Message-Server.md` | REBUILD | The claimed MCP schema, mount command, REST/SSE surface, and fallback paths are pre-build design. |
| `3-Approaches.md` | REVISE | Useful research history, but it presents unbuilt AgentAPI as a selectable fallback. |
| `4-Setup.md` | RETIRE | It is an intentional stub that only redirects to chapter 13. |
| `5-Operations.md` | RETIRE | It is an intentional stub that only redirects to chapter 13. |
| `6-Team.md` | REBUILD | It teaches role-addressed sends and a Claude-only team model that the shipped API does not have. |
| `7-Use-Cases.md` | REBUILD | Its flagship flow depends on removed `ccs` profiles and removed send parameters. |
| `8-Roadmap.md` | REBUILD | It is a historical plan, but it presents old implementation choices and planned work as the current stack. |
| `9-Implementation.md` | REBUILD | Its package, class, endpoint, and outcome map has drifted from the source. |
| `10-Cross-Host-Messaging.md` | REVISE | It labels most federation work proposed, but misses the shipped `coordinator:` lead channel. |
| `11-Features.md` | REVISE | It is the right catalogue, but code-path names are old and it misses the second-herdr-daemon capability. |
| `12-Claude-to-OpenCode.md` | REVISE | The porting guide is mostly current, but calls the product and spawned-member path a bridge. |
| `13-User-Guide.md` | REVISE | It is the best operator page, but needs the product rename and the second-herdr-daemon setup. |
## Pages needing work
### `Home.md` — REVISE
- Quote: `# claude-bridge` (line 1) and `` `claude-bridge` keeps`` (line 11).
The product is `fleet` / `fleetd`. The MCP server identifies itself as `fleet` in
`fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:313-315`.
- Quote: `AgentAPI ... swappable fallback injector` (lines 73-76).
There is no AgentAPI implementation under `fleetd/src/main/java`; the actual
launchers are selected by `Profile.kind` in
`fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java:265-270`.
### `_Sidebar.md` — REVISE
- Quote: `### 📖 claude-bridge` (line 1).
Rename it to `fleet`. `FleetMcp` registers the current product-facing tool set at
`fleetd/src/main/java/dev/ltms/fleet/mcp/FleetMcp.java:301-326`.
### `1-Architecture.md` — REBUILD
- Quote: `` `claude-bridge` lets`` (line 3). The product was renamed; the MCP
server name is `fleet` (`FleetMcp.java:313-315`).
- Quote: ``fleet_read`` in the tool list (line 102). No such tool is registered.
The complete registered list is `fleet_send` through `fleet_whoami` at
`FleetMcp.java:301-326`; `fleet_read` is absent.
- Quote: `SSE (GET /events)` (line 143). `FleetApp.build()` registers no `/events`
route; its routes are listed at `FleetApp.java:143-159`.
- Quote: `Redis Streams / NATS JetStream, or an embedded queue` (line 106).
The shipped durable inbox is AMQP, configured by `broker`, at
`FleetConfig.java:49-50` and `FleetConfig.java:655-714`.
- Quote: `AgentAPI (fallback)` (line 107). No AgentAPI adapter exists; shipped
launcher kinds are `claude-code` and `opencode` (`FleetConfig.java:265-270`).
### `2-Message-Server.md` — REBUILD
- Quote: `claude mcp add --transport http bridge http://127.0.0.1:8080/mcp`
(line 67). The daemon defaults to port `8765` in `FleetConfig.java:183-187`,
and identifies its server as `fleet` at `FleetMcp.java:313-315`.
- Quote: ``fleet_send(message, target?, {block, timeout_seconds, auto_spawn,
turn_id})`` (line 80). The real parameters are `sessionId`, `content`,
`timeoutMs`, `wait`, `turnId`, and `coordId` (`FleetMcp.java:1096-1108`).
- Quote: ``fleet_read(target, source)`` (line 85). It is not registered; see the
complete registration at `FleetMcp.java:301-326`.
- Quote: `docs/MCP-Contract.md ... normative` (lines 87-88). That is not a valid
reference: only §6 is current, as the current operator guide itself says at
`.wiki-snapshot/13-User-Guide.md:466`.
- Quote: `SSE (GET /events)` (line 45). No route exists in the built REST surface,
`FleetApp.java:143-159`.
### `3-Approaches.md` — REVISE
- Quote: `AgentAPI ... remains a swappable fallback injector` (lines 78-84).
It was never built. The shipped adapter selection is only `claude-code` or
`opencode` (`FleetConfig.java:265-270`). Keep it as discarded research, not an
operational fallback.
- Quote: `claude-bridge` (line 109). Rename the product to `fleet`; the runtime
package is `dev.ltms.fleet`, for example `FleetMcp.java:1`.
### `4-Setup.md` — RETIRE
It is a 25-line redirect and says its procedure was never written (lines 3-9).
Chapter 13 is the maintained install procedure. Keeping a second navigation page
adds no working documentation.
### `5-Operations.md` — RETIRE
It is a 35-line redirect and says its runbook was never written (lines 3-14).
Chapter 13 now owns run and recovery instructions.
### `6-Team.md` — REBUILD
- Quote: `fleet_send {role: w-claude, prompt: A}` (line 98). `fleet_send` accepts
`sessionId` and `content`, not `role` or `prompt` (`FleetMcp.java:1096-1108`).
- Quote: `some on Claude, some on the remote local LLM` (lines 3-5) and `Every
worker is ... Claude Code` (line 25). `opencode` is a first-class launcher kind,
not a Claude worker (`FleetConfig.java:265-270`).
- Quote: `fleetd's concurrency policy` (line 121). The configured capacity control
is per-profile `maxLoad` (`FleetConfig.java:251-264`), not the role routing model
described here.
### `7-Use-Cases.md` — REBUILD
- Quote: `ccs profile` (line 10), `ccs + herdr` (line 22), and `ccs-spawn`
(line 45). The configuration has `profiles` and `fleet`, not `ccs`:
`FleetConfig.java:34-58` and `FleetConfig.java:81-101`.
- Quote: `fleet_send({"to", "kind", "body", "block"})` (lines 55-62).
None of those are the shipped send parameters. The schema is
`FleetMcp.java:1096-1108`.
- Quote: `fleet_list() → { "profiles": ... }` (lines 74-80). `fleet_list` is a
roster view; `fleet_profiles` is the configured-backend view, as registered at
`FleetMcp.java:307-311` and described at `FleetMcp.java:1176-1182`.
### `8-Roadmap.md` — REBUILD
- Quote: `Java 21+` (line 43). The current project guidance and source use Java 25;
the `FleetConfig` source itself uses Java 25 unnamed lambda parameters, for
example `FleetConfig.java:102`.
- Quote: `herdr 0.7.0 / protocol 14` (line 46). The current REST health endpoint
reports the live protocol returned by herdr (`FleetApp.java:240-244`), while the
current operator guide records protocol 19 at
`.wiki-snapshot/13-User-Guide.md:76-85`.
- Quote: `ccs <profile> claude` and `ccs env <profile>` (lines 47-48). Shipped
configuration uses `Profile` records and launcher `kind`,
`FleetConfig.java:313-330` and `FleetConfig.java:265-270`.
- Quote: `Redis Streams via Lettuce` (line 50). The actual durable inbox is AMQP
`broker`, `FleetConfig.java:655-714`.
### `9-Implementation.md` — REBUILD
- Quote: `rest.FleetdApp` and `mcp.BridgeMcp` (lines 29-30). The classes are
`rest.FleetApp` and `mcp.FleetMcp` (`FleetApp.java:46`; `FleetMcp.java:67`).
- Quote: `dev.ltms.fleetd` (line 67). The source package is `dev.ltms.fleet`
(`FleetMcp.java:1`).
- Quote: `WorkerPresence` (line 110). The current class is `MemberPresence`, as
imported and used by `FleetMcp` at `FleetMcp.java:12` and `465-469`.
- Quote: the outcome list ending in `STALE_TURN` (lines 128-131). The code also
has `BACKEND_EXHAUSTED` (`FleetMcp.java:550-554`) and async `ASKING` handling
(`FleetMcp.java:664-668`).
- Quote: `FleetdApp` (line 207) and `FleetdConfig` (line 211). These names do not
resolve; current classes are `FleetApp` and `FleetConfig`.
### `10-Cross-Host-Messaging.md` — REVISE
- Quote: the chapter says the cross-host fabric is proposed except for the
single-host inbox (lines 3-8). Cross-host **lead-to-lead** delivery shipped:
`fleet_send` accepts `coordId` (`FleetMcp.java:1094-1107`) and publishes it at
`FleetMcp.java:616-641`; configuration has `coordinator` at
`FleetConfig.java:74-78` and `99-101`.
- Quote: `bridge.dlx` (line 90). This product name is stale. The shipped lead path
uses `LeadChannel`, not the proposed exchange flow (`FleetMcp.java:95-96` and
`616-641`). Keep the proposed federation design, but add a clear shipped/proposed
boundary for CB-637.
### `11-Features.md` — REVISE
- Quote: `mcp/BridgeMcp` (line 22), `config/FleetdConfig` (lines 25-27), and other
index references. These paths no longer resolve; the source classes are
`mcp/FleetMcp` (`FleetMcp.java:67`) and `config/FleetConfig`
(`FleetConfig.java:81`).
- Quote: `fleet_whoami` returns only `primary` or `worker` (lines 99-100).
It also returns `architect` (`FleetMcp.java:1235-1244`).
- The page needs the missing separate member-herdr-daemon feature listed below.
### `12-Claude-to-OpenCode.md` — REVISE
- Quote: `same bridge mount` (line 5) and `a bridge-spawned worker` (line 94).
Rename the product path to `fleet`. The daemon exposes the MCP server as `fleet`
(`FleetMcp.java:313-315`), and profiles select OpenCode with `kind: opencode`
(`FleetConfig.java:332-335`).
- Quote: the sample mount name is `fleetd` (line 67). The server name is `fleet`;
update the sample to avoid teaching a second product name.
### `13-User-Guide.md` — REVISE
- Quote: `The bridge is the only channel` (line 63). The invariant is correct, but
the product term needs the `fleet` rename. The daemon's MCP server name is
`fleet` (`FleetMcp.java:313-315`).
- Quote: it describes one herdr socket (lines 72-85). It needs the optional
`memberHerdrSocket` setup and two-daemon health meaning. The config key is in
`FleetConfig.java:34-37`, and `/healthz` checks both daemons when configured at
`FleetApp.java:210-245`.
## MISSING
`11-Features.md` has a body section for **routing members through a separate herdr daemon**
(`## memberHerdrSocket`, line 2174), but **no row in the index table** at the top of the page
(lines 20-95). That table is how the page is meant to be read, so a capability absent from it is
effectively undiscoverable. Lead note: this is my own omission — I added the section on 2026-08-31
and did not add the matching row. Fixed in the wiki at `68e32c6`'s successor.
The original audit stated the feature had no entry at all. That was wrong: the section exists. The
gap is the index row. Recorded here rather than silently corrected, because the difference matters —
"undocumented" and "documented but unindexed" are different jobs.
Evidence for the feature itself: `FleetConfig.java:34-37` and `FleetApp.java:103-115`, `210-245`,
and `247-263`.
## Audit method and coverage
I checked all 15 pages. I checked concrete tool, route, config, class, file, and
product-name claims claim-by-claim on 11 pages: Home, Sidebar, 1, 2, 4, 5, 6, 7, 9,
11, and 13. I skimmed the remaining four long historical or research pages (3, 8, 10,
12), then checked their concrete claims that affect the verdict. This is an audit of
the supplied snapshot, not a wiki rewrite.
+27
View File
@@ -117,6 +117,15 @@ herdrSocket: ~/.config/herdr/herdr.sock
# Optional socket for member panes. Omit this to use herdrSocket for both leads and members.
# memberHerdrSocket: /Users/member/.config/herdr/herdr.sock
# fleetd #213: the login shell the member OS user (memberHerdrSocket above) actually runs. ONLY
# read when memberHerdrSocket is set — fleetd's own $SHELL says nothing about a pane running
# under a different OS user, and there is no channel to ask herdr for that user's shell, so this
# must be told rather than guessed. Absent, blank, or anything not ending in "zsh" is treated the
# same as "not zsh": the memberCredentials.policy: allow-list ZDOTDIR scrub (see worktreeGroup
# below) is skipped in favour of the weaker CB-596 sentinel overlay — a degraded control, never a
# refusal to spawn. When memberHerdrSocket is absent this key is never consulted at all.
# memberLoginShell: /bin/zsh
# How member sessions are spawned. Define one or more named profiles (backends) under
# `profiles`; each key is the profile name (also the ccs profile). A profile says only WHICH
# BACKEND — model, CLI adapter, credentials, cost. It says nothing about what a member spawned on
@@ -634,6 +643,24 @@ guard:
# to a sibling directory of the repo root.
# worktreeRoot: /Users/me/src/.bridged-worktrees
# Worktree group sharing (fleetd #185 stage 3). OPTIONAL, off by default. Names an OS group
# that a provisioned worktree's repo is made group-writable for (git config
# core.sharedRepository group, plus a one-time chgrp/chmod/setgid fix-up), so a member spawned
# under a DIFFERENT OS user (see memberHerdrSocket) can write its own worktree, its
# per-worktree git metadata, and its own commit objects — without it, every file GitWorktrees
# creates is owned by fleetd's own uid and unwritable by another user.
# CAUTION: this isolates credentials, not the repository — a member in the group can still
# write the operator's git objects and refs in the shared repo. The operator running fleetd
# must already be a member of the named group, or every provisioning spawn fails loudly.
#
# fleetd #213: this is also the ONE group the memberCredentials.policy: allow-list ZDOTDIR scrub
# reuses when memberHerdrSocket is set — deliberately not a second config key. Under
# memberHerdrSocket, the scrub directory is generated under worktreeRoot (never java.io.tmpdir,
# which the member OS user cannot reach) and shared read-only with this group. If worktreeGroup
# is unset while memberHerdrSocket is set, the scrub cannot be guaranteed reachable by the member,
# so fleetd falls back to the weaker CB-596 sentinel overlay instead (a WARN names the gap).
# worktreeGroup: fleet-workers
# Session lifecycle limits (CB-303). All knobs are opt-in; omit or set to null to keep
# the feature disabled. By default the daemon never reaps, caps, or drains sessions.
# idleTtlSeconds → reap READY/DONE sessions idle longer than this (never BUSY/SPAWNING)
+18
View File
@@ -28,6 +28,7 @@
<testcontainers.version>1.20.4</testcontainers.version>
<commons-compress.version>1.27.1</commons-compress.version>
<commons-lang3.version>3.18.0</commons-lang3.version>
<sqlite-jdbc.version>3.53.4.0</sqlite-jdbc.version>
</properties>
<!--
@@ -44,6 +45,12 @@
3.0-rc5; bumping Jackson 3 to the patched 3.2.x breaks the SDK (annotation mismatch).
Only the loopback /mcp endpoint parses this JSON, from trusted local Claude clients.
The 11.0.23 -> 11.0.25 bump did clear jetty CVE-2024-8184 (5.9) and CVE-2024-6763.
fleetd #206: org.xerial:sqlite-jdbc 3.53.4.0 (added for OpenCodeSessionDiscovery) — the
only known advisory against this artifact is CVE-2023-32697 (RCE via an attacker-controlled
JDBC URL), fixed in 3.41.2.2; 3.53.4.0 is well past that fix and OSV.dev reports no open
advisory against it. Checked via the OSV.dev API (no Mend.io/JetBrains IDE MCP mount
available from this worktree) on 2026-08-31.
-->
<!-- Force the latest patched Jetty 11.x across all Javalin-pulled Jetty modules (no version
@@ -123,6 +130,17 @@
<version>${amqp.version}</version>
</dependency>
<!-- fleetd #206: opencode moved its session store from a JSON tree to SQLite
(opencode.db). This is the JDBC driver OpenCodeSessionDiscovery uses to read it
read-only. Ships bundled native libraries (linux/mac/windows, several archs), so it
is a heavier jar than most deps here — see the pom's dependency-security note below
for the size/CVE tradeoff actually measured. -->
<dependency>
<groupId>org.xerial</groupId>
<artifactId>sqlite-jdbc</artifactId>
<version>${sqlite-jdbc.version}</version>
</dependency>
<!-- Logging -->
<dependency>
<groupId>org.slf4j</groupId>
@@ -177,14 +177,14 @@ public final class Fleetd {
claudeProfiles, cfg.effectiveDefaultProfile(), System::getenv,
cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(),
() -> config.get().fleet(),
() -> config.get().memberCredentials()));
() -> config.get().memberCredentials(), null, config::get));
}
if (!opencodeProfiles.isEmpty()) {
adapters.add(new OpenCodeLauncher(router.memberAgents(), router.memberSpaces(),
opencodeProfiles, cfg.effectiveDefaultProfile(), System::getenv,
cfg.spawnReadyTimeoutMs(), cfg.spawnReadyPollMs(),
() -> config.get().fleet(),
() -> config.get().memberCredentials()));
() -> config.get().memberCredentials(), config::get));
}
AtomicReference<Function<String, Integer>> liveCountRef = new AtomicReference<>(_ -> 0);
// CB-578 stage B: one quarantine tracker for the whole daemon, shared between the launcher
@@ -224,7 +224,7 @@ public final class Fleetd {
contextCap = cfg.lifecycle().contextCap();
}
boolean clearAfterTurn = cfg.lifecycle() != null && cfg.lifecycle().clearAfterTurn();
SessionManager sessions = new SessionManager(workers, new GitWorktrees(cfg.worktreeRoot()),
SessionManager sessions = new SessionManager(workers, new GitWorktrees(cfg.worktreeRoot(), cfg.worktreeGroup()),
System::nanoTime, contextCap, clearAfterTurn);
liveCountRef.set(profileName -> (int) sessions.roster().stream()
.filter(s -> profileName.equals(s.profile()))
@@ -76,6 +76,26 @@ import java.util.Set;
* stay on {@code broker}'s vhost). {@code null} → no lead mailbox is opened.
* Config parsing + accessors only — nothing here wires it into a live
* {@code LeadMailbox}; that is a separate ticket. See {@link Coordinator}.
* @param worktreeGroup optional OS group name (fleetd #185 stage 3) that makes a provisioned
* worktree's repo group-shared, so a member running as a different OS user
* (see {@code memberHerdrSocket}) can write its own worktree, its per-worktree
* git metadata, and its own commit objects. {@code null}/blank/empty ⇒ off,
* today's behaviour unchanged (every file stays owned by fleetd's own uid).
* <strong>This isolates credentials, not the repository</strong>: a member in
* the group can still write the operator's git objects and refs in the shared
* repo. See {@link dev.ltms.fleet.session.Worktrees#shareWithGroup}.
* @param memberLoginShell fleetd #213: the login shell the member's OS user actually runs, ONLY
* meaningful (and only ever read) when {@code memberHerdrSocket} is
* configured — that mode spawns member panes under a different OS user than
* fleetd's own process, so fleetd's own {@code $SHELL} says nothing about what
* that pane runs. There is no channel to ask herdr for another user's shell, so
* this must be told, never guessed. {@code null}/blank (or a value not ending
* in {@code zsh}) is treated the same as "not zsh": the {@code
* memberCredentials.policy: allow-list} ZDOTDIR scrub is skipped in favour of
* the CB-596 sentinel overlay — a degraded control, never a refusal to spawn.
* When {@code memberHerdrSocket} is NOT configured this field is never
* consulted at all; fleetd keeps reading its own {@code $SHELL}, exactly as
* before this field existed.
*/
@JsonIgnoreProperties(ignoreUnknown = true)
public record FleetConfig(
@@ -98,7 +118,33 @@ public record FleetConfig(
ConfigReload configReload,
Integer quarantineCooldownSeconds,
MemberCredentials memberCredentials,
Coordinator coordinator) {
Coordinator coordinator,
String worktreeGroup,
String memberLoginShell) {
/** Back-compat form before the {@code memberLoginShell} key was added. */
public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map<String, Profile> profiles,
Guard guard, String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs,
Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet,
LeadHeartbeat leadHeartbeat, Health health, String placement, Auth auth,
ConfigReload configReload, Integer quarantineCooldownSeconds,
MemberCredentials memberCredentials, Coordinator coordinator, String worktreeGroup) {
this(bind, herdrSocket, memberHerdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs,
spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth,
configReload, quarantineCooldownSeconds, memberCredentials, coordinator, worktreeGroup, null);
}
/** Back-compat form before the {@code worktreeGroup} key was added. */
public FleetConfig(Bind bind, String herdrSocket, String memberHerdrSocket, Map<String, Profile> profiles,
Guard guard, String worktreeRoot, Lifecycle lifecycle, Integer spawnReadyTimeoutMs,
Integer spawnReadyPollMs, Broker broker, Primary primary, Fleet fleet,
LeadHeartbeat leadHeartbeat, Health health, String placement, Auth auth,
ConfigReload configReload, Integer quarantineCooldownSeconds,
MemberCredentials memberCredentials, Coordinator coordinator) {
this(bind, herdrSocket, memberHerdrSocket, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs,
spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth,
configReload, quarantineCooldownSeconds, memberCredentials, coordinator, null, null);
}
/** Back-compat form before the {@code coordinator:} block was added. */
public FleetConfig(Bind bind, String herdrSocket, Map<String, Profile> profiles, Guard guard,
@@ -109,7 +155,7 @@ public record FleetConfig(
MemberCredentials memberCredentials) {
this(bind, herdrSocket, null, profiles, guard, worktreeRoot, lifecycle, spawnReadyTimeoutMs,
spawnReadyPollMs, broker, primary, fleet, leadHeartbeat, health, placement, auth,
configReload, quarantineCooldownSeconds, memberCredentials, null);
configReload, quarantineCooldownSeconds, memberCredentials, null, null);
}
/** Back-compat form before the CB-596 {@code memberCredentials:} block was added. */
@@ -1325,7 +1371,7 @@ public record FleetConfig(
"bind", "herdrSocket", "memberHerdrSocket", "profiles", "guard", "worktreeRoot",
"lifecycle", "spawnReadyTimeoutMs", "spawnReadyPollMs", "broker", "primary", "fleet",
"leadHeartbeat", "health", "placement", "auth", "configReload", "quarantineCooldownSeconds",
"memberCredentials", "coordinator");
"memberCredentials", "coordinator", "worktreeGroup", "memberLoginShell");
/** Load and validate config from {@code path}. */
public static FleetConfig load(Path path) {
@@ -1943,9 +1989,14 @@ public record FleetConfig(
: new MemberCredentials(null, List.of(), List.of());
// coordinator is left as-is, like broker/primary above: null keeps no LeadMailbox opened,
// and this ticket's Coordinator is config-only anyway (nothing yet reads it at startup).
// worktreeGroup is left as-is (fleetd #185 stage 3): null/blank is "off", and there is no
// sane non-null default — an OS group name is operator-specific.
// memberLoginShell is left as-is (fleetd #213), like worktreeGroup: null/blank is "not
// configured", and there is no sane non-null default — a member's login shell is
// operator-specific and only meaningful when memberHerdrSocket is also set.
return new FleetConfig(b, herdrSocket, memberHerdrSocket, profiles, g, worktreeRoot, l, timeout, pollMs,
broker, primary, f, leadHeartbeat, health, placementOrDefault, a, configReload,
quarantineCooldown, mc, coordinator);
quarantineCooldown, mc, coordinator, worktreeGroup, memberLoginShell);
}
/**
@@ -38,6 +38,11 @@ public final class AgentControl {
this.herdr = herdr;
}
/** The herdr daemon this control object sends its agent calls to. */
public HerdrClient herdr() {
return herdr;
}
/** One agent-targeted call, translating a terminal id to its pane id (retrying once fresh). */
private JsonNode agentCall(String method, String target, Map<String, Object> extra) {
String resolved = resolveTarget(target);
@@ -2,8 +2,11 @@ package dev.ltms.fleet.herdr;
import com.fasterxml.jackson.databind.JsonNode;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.OptionalLong;
import java.util.Set;
/**
* Resolves which herdr pane a process belongs to — the herdr half of connection-based MCP
@@ -12,23 +15,46 @@ import java.util.Map;
* is calling without the worker sending anything spoofable.
*
* <p>herdr owns the PID→pane truth: {@code pane.process_info} reports each pane's {@code shell_pid}
* and foreground process PIDs. This scans agent panes; a spawn-time {@code pid→terminal} cache is
* the obvious optimization once wired into {@code ClaudeCodeLauncher}.
* and foreground process PIDs. A pid that is neither of those directly — e.g. a grandchild a
* worker spawned, such as a {@code python3} or {@code curl} helper that opens its own MCP
* connection — is resolved by walking its ancestry (via {@link ParentResolver}) up to the root and
* matching any ancestor against a pane's {@code shell_pid} or foreground pids (CB-161). Without
* this walk such a pid matches no pane, and the caller falls through to loopback-trust and is
* resolved as the primary — a worker→primary privilege escalation.
*
* <p>This scans agent panes; a spawn-time {@code pid→terminal} cache is the obvious optimization
* once wired into {@code ClaudeCodeLauncher}.
*
* <p>CB-185 split the fleet across two herdr daemons — lead operations on one, members on the
* other ({@code memberHerdrSocket}). A caller's pane can live on <em>either</em> daemon (a lead's
* MCP connection resolves against the lead daemon; a member's against the member daemon), so this
* must be able to search more than one client. {@link #PaneLocator(HerdrClient, HerdrClient)}
* searches the lead client first, then the member client, and collapses to a single scan when the
* two are the same object (the historical single-daemon deployment).
* two are the same object (the historical single-daemon deployment). The caller's ancestor set is
* computed once per {@link #terminalForPid} call and reused across every client searched — it
* does not depend on which daemon a pane happens to live on.
*/
public final class PaneLocator {
/**
* Bound on how many ancestor generations {@link #ancestorsOf} walks. This runs on every MCP
* call, so a cycle or a pathologically deep process tree must not hang identity resolution;
* 32 generations is far more than any real worker→helper process tree needs.
*/
private static final int MAX_ANCESTRY_DEPTH = 32;
private final List<HerdrClient> herdrs;
private final ParentResolver parentResolver;
/** Search only this client — the single-daemon deployment. */
public PaneLocator(HerdrClient herdr) {
this(herdr, ParentResolver.PROCESS_HANDLE);
}
/** Search only this client, resolving ancestry through {@code parentResolver} — for tests. */
public PaneLocator(HerdrClient herdr, ParentResolver parentResolver) {
this.herdrs = List.of(herdr);
this.parentResolver = parentResolver;
}
/**
@@ -37,7 +63,13 @@ public final class PaneLocator {
* collapses to one client and one scan, exactly {@link #PaneLocator(HerdrClient)}'s behaviour.
*/
public PaneLocator(HerdrClient lead, HerdrClient member) {
this(lead, member, ParentResolver.PROCESS_HANDLE);
}
/** Two-daemon deployment, resolving ancestry through {@code parentResolver} — for tests. */
public PaneLocator(HerdrClient lead, HerdrClient member, ParentResolver parentResolver) {
this.herdrs = lead == member ? List.of(lead) : List.of(lead, member);
this.parentResolver = parentResolver;
}
/**
@@ -49,8 +81,9 @@ public final class PaneLocator {
if (pid <= 0) {
return null;
}
Set<Long> ancestry = ancestorsOf(pid);
for (HerdrClient herdr : herdrs) {
String terminal = terminalForPid(herdr, pid);
String terminal = terminalForPid(herdr, ancestry);
if (terminal != null) {
return terminal;
}
@@ -58,28 +91,54 @@ public final class PaneLocator {
return null;
}
private static String terminalForPid(HerdrClient herdr, long pid) {
/**
* {@code pid} itself plus its ancestor chain, walked through {@link #parentResolver} up to
* {@link #MAX_ANCESTRY_DEPTH} generations or pid 1, whichever comes first. A vanished ancestor
* ({@link ParentResolver#parentOf} returning empty) ends the walk without error — it just means
* the chain is shorter than the bound. A cycle in a fake resolver is caught by the "already
* seen" check and also ends the walk, so this can never loop.
*/
private Set<Long> ancestorsOf(long pid) {
Set<Long> ancestry = new LinkedHashSet<>();
long current = pid;
for (int depth = 0; depth < MAX_ANCESTRY_DEPTH; depth++) {
if (current <= 0 || !ancestry.add(current)) {
break; // vanished/invalid pid, or a cycle back to a pid already recorded
}
if (current == 1) {
break; // reached the root of the process tree
}
OptionalLong parent = parentResolver.parentOf(current);
if (parent.isEmpty()) {
break; // vanished ancestor — not an error, just the end of the chain
}
current = parent.getAsLong();
}
return ancestry;
}
private static String terminalForPid(HerdrClient herdr, Set<Long> ancestry) {
for (JsonNode pane : herdr.call("pane.list", Map.of()).path("panes")) {
String paneId = pane.path("pane_id").asText(null);
if (paneId != null && paneOwnsPid(herdr, paneId, pid)) {
if (paneId != null && paneOwnsAnyOf(herdr, paneId, ancestry)) {
return pane.path("terminal_id").asText(null);
}
}
return null;
}
private static boolean paneOwnsPid(HerdrClient herdr, String paneId, long pid) {
private static boolean paneOwnsAnyOf(HerdrClient herdr, String paneId, Set<Long> ancestry) {
JsonNode info;
try {
info = herdr.call("pane.process_info", Map.of("pane_id", paneId)).path("process_info");
} catch (HerdrException e) {
return false; // pane vanished mid-scan — just skip it
}
if (info.path("shell_pid").asLong(-1) == pid) {
if (ancestry.contains(info.path("shell_pid").asLong(-1))) {
return true;
}
for (JsonNode p : info.path("foreground_processes")) {
if (p.path("pid").asLong(-1) == pid) {
if (ancestry.contains(p.path("pid").asLong(-1))) {
return true;
}
}
@@ -0,0 +1,21 @@
package dev.ltms.fleet.herdr;
import java.util.OptionalLong;
/**
* Resolves a pid's parent pid — the seam {@link PaneLocator} walks a process's ancestry through,
* so its tests can drive the walk from a fake pid→parent map instead of spawning real processes.
*
* <p>{@link #PROCESS_HANDLE} is the production implementation, backed by {@link ProcessHandle}.
*/
public interface ParentResolver {
/** The parent pid of {@code pid}, or empty if {@code pid} is gone or has no known parent. */
OptionalLong parentOf(long pid);
/** Production resolver: asks the JVM's {@link ProcessHandle} view of the OS process tree. */
ParentResolver PROCESS_HANDLE = pid -> ProcessHandle.of(pid)
.flatMap(ProcessHandle::parent)
.map(parent -> OptionalLong.of(parent.pid()))
.orElse(OptionalLong.empty());
}
@@ -74,6 +74,14 @@ public final class CompletionResolver implements TurnListener {
*/
public static final long MIN_TURN_NANOS = Duration.ofSeconds(2).toNanos();
/**
* fleetd#164 (part 2): one stable, explicit backend-failure marker seen on a Claude Code pane
* when the backend itself rejected the turn (e.g. {@code "API Error: 400 invalid request body"}).
* Kept deliberately narrow — a growing list of ad-hoc error strings rots as backends change their
* wording; broader backend-error surfacing is out of scope here (fleetd#164 point 3).
*/
private static final Pattern BACKEND_ERROR = Pattern.compile("(?i)\\bAPI Error\\s*:");
private static final String CLIPPED_PANE_TAIL_MARKER =
"[Pane tail clipped: member did not call fleet_reply.]";
@@ -278,6 +286,20 @@ public final class CompletionResolver implements TurnListener {
}
return;
}
// fleetd#164 (part 2): a scrape that read cleanly and produced content still isn't a real
// reply when that content is the backend's own rejection (e.g. an HTTP 400 before the worker
// did any work). Classify it as a failure naming the member, rather than handing the caller a
// scrape that reads like a completed answer.
String backendError = firstMatchingLine(assistantBlock, BACKEND_ERROR);
if (backendError != null) {
// Carry the whole scrape, not just the matched line. The pattern is a heuristic: a member
// that forgot fleet_reply while reporting *about* a backend error matches it too. Failing
// is still right — the caller must not read a scrape as an answer — but dropping the rest
// of the pane would destroy the report, which is the same defect fleetd#164 is about.
fail(target, turn, "member " + target + " ended on a backend error: " + backendError
+ "\n--- pane tail ---\n" + tail);
return;
}
String completion = clipped ? tail + "\n" + CLIPPED_PANE_TAIL_MARKER : tail;
if (rendezvous.resolveCompletion(waiter, completion)) {
inFlight.remove(target, turn);
@@ -951,7 +951,10 @@ public final class FleetMcp {
.sorted(Map.Entry.comparingByValue())
.map(e -> leadView(e.getKey(), e.getValue(), live.get(e.getKey()), selfTerm))
.toList();
List<MemberSession> roster = sessions.roster();
// fleetd #209: this is the caller-driven fleet_list read that actually reports
// agentSessionId (via memberCapacityView -> SessionManager.rosterView), so it uses the
// resolving roster; the heartbeat/health/metrics timers stay on the plain sessions.roster().
List<MemberSession> roster = sessions.rosterResolved();
List<Map<String, Object>> out = roster.stream()
.map(s -> memberCapacityView(s, live.get(s.terminalId()), messages, capacity.clock().getAsLong()))
.toList();
@@ -94,13 +94,26 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher {
public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard,
Map<String, FleetConfig.Profile> profiles, String defaultProfile,
Function<String, String> env,
long spawnReadyTimeoutMs, long spawnReadyPollMs,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
long spawnReadyTimeoutMs, long spawnReadyPollMs,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
this(agents, spaces, guard, profiles, defaultProfile, env, spawnReadyTimeoutMs, spawnReadyPollMs,
fleet, memberCredentials, null, null);
}
/** Production constructor, plus the live config for URI environment exclusions. */
public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard,
Map<String, FleetConfig.Profile> profiles, String defaultProfile,
Function<String, String> env,
long spawnReadyTimeoutMs, long spawnReadyPollMs,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials,
Supplier<Set<String>> hostEnvNames,
Supplier<FleetConfig> config) {
this(agents, spaces, guard, profiles, defaultProfile, env,
spawnReadyTimeoutMs,
System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs),
fleet, memberCredentials);
fleet, memberCredentials, hostEnvNames, config);
}
/**
@@ -153,10 +166,22 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher {
Function<String, String> env,
long spawnReadyTimeoutMs,
LongSupplier nowMillis, Runnable sleeper,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
this(agents, spaces, guard, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper,
fleet, memberCredentials, null, null);
}
/** Full testability constructor, plus the live config for URI environment exclusions. */
public ClaudeCodeLauncher(AgentControl agents, WorkspaceControl spaces, SubscriptionGuard guard,
Map<String, FleetConfig.Profile> profiles, String defaultProfile,
Function<String, String> env, long spawnReadyTimeoutMs,
LongSupplier nowMillis, Runnable sleeper, Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials,
Supplier<Set<String>> hostEnvNames,
Supplier<FleetConfig> config) {
super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env,
spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials);
spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, hostEnvNames, config);
this.guard = guard;
}
@@ -174,7 +199,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher {
Supplier<FleetConfig.MemberCredentials> memberCredentials,
Supplier<Set<String>> hostEnvNames) {
super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env,
spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, hostEnvNames);
spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, hostEnvNames, null);
this.guard = guard;
}
@@ -2,6 +2,8 @@ package dev.ltms.fleet.member;
import dev.ltms.fleet.config.FleetConfig;
import dev.ltms.fleet.herdr.Agent;
import dev.ltms.fleet.herdr.HerdrClient;
import dev.ltms.fleet.herdr.HerdrException;
import dev.ltms.fleet.peer.Capability;
import dev.ltms.fleet.peer.MemberRole;
import dev.ltms.fleet.peer.PeerHandle;
@@ -21,6 +23,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.IdentityHashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
@@ -44,12 +47,17 @@ import java.util.stream.Collectors;
* the single adapter that declares it. Profiles partition cleanly across adapters: the
* constructor rejects a name claimed by two.</li>
* <li><strong>By pane id</strong> — {@link #stop} routes to the adapter that spawned that pane
* (recorded at spawn time). A pane the composite never spawned (only real for a caller that
* hand-rolls an id) falls back to the first delegate; teardown is pane-id addressed and
* tab cleanup is single-occupant guarded, so it is safe either way.</li>
* (recorded at spawn time). A pane the composite never spawned, or one whose record was lost
* to a daemon restart (CB-185 blocker 1 — {@link #spawnedBy} is in-memory only), can use the
* fallback route in a one-daemon fleet. With more than one herdr daemon, {@link #probeOwner}
* asks each configured daemon which one actually knows the pane: exactly one match routes
* (and caches); no match is treated as already-gone; more than one match is a genuine
* ambiguity (pane ids are per-daemon counters, so two daemons really can both hold, say,
* {@code w1:p1}) and stop refuses rather than closing a pane on an arbitrary herdr daemon.</li>
* <li><strong>Fleet-wide</strong> — {@link #reapOrphanWorkers} and {@link #capabilities} fan out
* and combine. {@link #list} is deduplicated by pane id because every herdr-backed delegate
* shares one herdr connection and so reports the same global agent set.</li>
* and combine. {@link #list} is deduplicated by (owning daemon, pane id): delegates that share
* one herdr connection report the same global agent set, but two daemons can each hold a pane
* called {@code w1:p1}, so the daemon has to be part of the key.</li>
* </ul>
*
* <p>CB-518: an unqualified spawn is routed through a {@link PlacementPolicy}. The default
@@ -435,12 +443,98 @@ public final class CompositePeerLauncher implements PeerLauncher {
@Override
public void stop(String id) {
HerdrPeerLauncher d = spawnedBy.remove(id);
HerdrPeerLauncher d = spawnedBy.get(id);
if (d == null) {
log.debug("stop({}) — no recorded owner, routing to the first adapter (pane-addressed)", id);
d = delegates.getFirst();
if (herdrDaemonCount() == 1) {
log.debug("stop({}) — no recorded owner in a single-daemon fleet", id);
d = delegates.getFirst();
} else {
d = probeOwner(id);
if (d == null) {
// No configured herdr daemon has ever heard of this pane. CB-185 blocker 1: this
// is the normal case right after a daemon restart empties spawnedBy for a member
// that has ALREADY been torn down since — the caller retried a stop that already
// succeeded. Nothing to close and no owner to cache; matching the tolerance
// HerdrPeerLauncher#stop already gives an already-gone pane (agent.close swallows
// that as success), stop() here is a no-op rather than a refusal.
log.debug("stop({}) — no configured herdr daemon knows this pane; "
+ "treating as already stopped", id);
return;
}
}
}
// Drop the owner record only after the delegate accepted the stop. Removing it first meant a
// delegate that threw left the pane alive with its owner forgotten, so the retry fell into
// the ambiguous branch above and refused the id for good.
d.stop(id);
spawnedBy.remove(id);
}
/**
* CB-185 blocker 1: recover a spawnedBy cache miss by asking every distinct herdr daemon which
* one actually knows {@code id} — the fix for "after a restart, every surviving member becomes
* un-stoppable" (spawnedBy is in-memory only, so a restart empties it, and members intentionally
* outlive the daemon).
*
* <p>Grouped by daemon identity, not by delegate, for the same reason {@link #list()} groups
* that way: two adapters (claude-code, opencode) sharing one herdr connection would otherwise be
* probed twice, and a pane on their shared daemon would look owned by two adapters instead of
* one daemon.
*
* <p>A daemon that fails to answer {@code list()} (e.g. it is down) is treated as "does not know
* this pane" rather than aborting the whole probe — one unreachable daemon must never make a
* pane that a <em>different</em>, healthy daemon actually owns un-stoppable too, which would
* resurrect the exact bug this method exists to fix.
*
* @return the owning delegate — cached into {@link #spawnedBy} so the next call is free — or
* {@code null} when no daemon knows the pane
* @throws IllegalArgumentException when more than one daemon claims the pane: pane ids are
* per-daemon counters, so two daemons really can both hold, say, {@code w1:p1}, and there
* is no way to tell which one the caller means
*/
private HerdrPeerLauncher probeOwner(String id) {
Map<HerdrClient, HerdrPeerLauncher> byDaemon = new IdentityHashMap<>();
for (HerdrPeerLauncher delegate : delegates) {
byDaemon.putIfAbsent(delegate.herdr(), delegate);
}
List<HerdrPeerLauncher> owners = new ArrayList<>();
for (HerdrPeerLauncher representative : byDaemon.values()) {
List<Agent> agents;
try {
agents = representative.list();
} catch (HerdrException e) {
log.warn("stop({}) probe: a configured herdr daemon was unreachable ({}); "
+ "treating it as not knowing this pane", id, e.getClass().getSimpleName());
continue;
}
boolean knows = agents.stream().anyMatch(a -> id.equals(a.paneId()));
if (knows) {
owners.add(representative);
}
}
if (owners.size() > 1) {
throw new IllegalArgumentException("ambiguous paneId '" + id + "': "
+ owners.size() + " configured herdr daemons report this pane — "
+ "no way to tell which one the caller means");
}
if (owners.isEmpty()) {
return null;
}
HerdrPeerLauncher owner = owners.get(0);
spawnedBy.put(id, owner);
return owner;
}
/**
* Count actual herdr daemons, not peer adapter kinds. Identity is intentional: separate client
* objects may represent different daemons even if a client later implements value equality.
*/
private int herdrDaemonCount() {
Set<HerdrClient> daemons = Collections.newSetFromMap(new IdentityHashMap<>());
for (HerdrPeerLauncher delegate : delegates) {
daemons.add(delegate.herdr());
}
return daemons.size();
}
@Override
@@ -475,14 +569,24 @@ public final class CompositePeerLauncher implements PeerLauncher {
return route(profileName).capabilities();
}
/** Every herdr agent, deduplicated by pane id (all delegates share one herdr and list globally). */
/**
* Every herdr agent, deduplicated by (owning daemon, pane id).
*
* <p>Delegates that share one {@link HerdrClient} see the same global agent set, so listing them
* both would report every agent twice — that is what the dedupe is for. But pane ids are
* per-daemon counters, so two daemons really can both hold {@code w1:p1} on different panes.
* Keying on the pane id alone would silently drop one of them from {@code fleet_list} and from
* every status view built on it. The daemon is part of the key for exactly that reason.
*/
@Override
public List<Agent> list() {
Map<HerdrClient, Integer> daemonIndex = new IdentityHashMap<>();
Map<String, Agent> byPane = new LinkedHashMap<>();
for (HerdrPeerLauncher d : delegates) {
int daemon = daemonIndex.computeIfAbsent(d.herdr(), _ -> daemonIndex.size());
for (Agent a : d.list()) {
if (a.paneId() != null) {
byPane.putIfAbsent(a.paneId(), a);
byPane.putIfAbsent(daemon + "\u0000" + a.paneId(), a);
}
}
}
@@ -7,6 +7,9 @@ import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.GroupPrincipal;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermissions;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
@@ -116,6 +119,70 @@ public final class EnvAllowListScrub {
}
}
/**
* fleetd #213: as {@link #generate(Path, Set)}, plus share the generated directory with
* {@code group} — the member OS user's group (the operator's existing {@code worktreeGroup:}
* name, reused rather than inventing a second one) — so a member running under a different OS
* user than fleetd's own process can still read what it needs from a directory placed outside
* {@code java.io.tmpdir}. {@code group} null/blank ⇒ identical to {@link #generate(Path, Set)};
* this is the single-daemon (no {@code memberHerdrSocket}) shape, where the pane is fleetd's own
* uid and no group sharing is needed.
*
* @throws UncheckedIOException also when {@code group} does not resolve on this host, or a
* group-ownership/permission call is refused — the same "fail
* loudly rather than start unprotected" contract as above: a scrub
* the configured member user cannot even read is not a working
* control.
*/
public static Path generate(Path parentDir, Set<String> allowedNames, String group) {
Path dir = generate(parentDir, allowedNames);
if (group != null && !group.isBlank()) {
shareWithGroup(dir, group);
}
return dir;
}
/**
* chgrp/chmod-equivalent over the freshly generated directory and the startup files already
* written into it: owner keeps full access, {@code group} gets traverse+read on the directory
* ({@code rwxr-x---}, so a login shell under that group can find and source the files) and
* read-only on each file ({@code rw-r-----}) — deliberately no group WRITE anywhere, since a
* member never needs to add or change fleetd's own generated scrub. (The scrub script's own
* report write inside the pane consequently fails closed rather than open — see {@code
* scrub.zsh}'s trailing {@code 2>/dev/null} — which {@link
* dev.ltms.fleet.member.HerdrPeerLauncher#releaseZdotdir} already treats as "cannot be
* confirmed to have run" rather than success.)
*/
private static void shareWithGroup(Path dir, String group) {
try {
GroupPrincipal principal = dir.getFileSystem().getUserPrincipalLookupService()
.lookupPrincipalByGroupName(group);
setGroupAndPermissions(dir, principal, "rwxr-x---");
try (Stream<Path> entries = Files.list(dir)) {
for (Path file : entries.toList()) {
setGroupAndPermissions(file, principal, "rw-r-----");
}
}
} catch (IOException e) {
throw new UncheckedIOException("cannot share generated ZDOTDIR " + dir + " with group '"
+ group + "' — the group must exist, and the fleetd operator ("
+ System.getProperty("user.name") + ") must be a member of it", e);
} catch (UnsupportedOperationException e) {
throw new UncheckedIOException("cannot share generated ZDOTDIR " + dir + " with group '"
+ group + "' — this filesystem does not support POSIX group ownership",
new IOException(e));
}
}
private static void setGroupAndPermissions(Path path, GroupPrincipal group, String perms) throws IOException {
PosixFileAttributeView view = Files.getFileAttributeView(path, PosixFileAttributeView.class);
if (view == null) {
throw new IOException("POSIX file attributes are not supported for " + path);
}
view.setGroup(group);
Files.setPosixFilePermissions(path, PosixFilePermissions.fromString(perms));
}
/** One operator-sourcing startup file: source the {@code $HOME} counterpart, change nothing else. */
private static String homeSourcingFile(String name) {
return """
@@ -3,6 +3,7 @@ package dev.ltms.fleet.member;
import dev.ltms.fleet.config.FleetConfig;
import dev.ltms.fleet.herdr.Agent;
import dev.ltms.fleet.herdr.AgentControl;
import dev.ltms.fleet.herdr.HerdrClient;
import dev.ltms.fleet.herdr.HerdrException;
import dev.ltms.fleet.herdr.Tab;
import dev.ltms.fleet.herdr.Workspace;
@@ -111,6 +112,12 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
* daemon's own process is started the same way (a login shell sourcing the same secret store —
* see CB-592's investigation of {@code secrets.sh}), so on a single-host deployment its env
* mirrors what the pane's login shell is about to export.
*
* <p>fleetd #185 stage 2: that mirroring assumption holds only while the member pane runs under
* the SAME OS user as the daemon. When {@code memberHerdrSocket:} is configured, member panes
* run on a second herdr owned by a different user — different {@code $HOME}, different {@code
* secrets.sh}, different environment entirely — so this field's data no longer describes what a
* member pane inherits. See {@link #logCredentialGap} for how that mode is handled.
*/
private final Supplier<Set<String>> hostEnvNames;
@@ -169,6 +176,8 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
/** Guards {@link #warnNonZsh} to one WARN per launcher instance, not one per spawn. */
private final AtomicBoolean nonZshShellWarned = new AtomicBoolean();
/** Live config provides URI environment names that must never enter member panes. */
private final Supplier<FleetConfig> config;
/**
* @param namePrefix label prefix for this peer kind (drives naming and reap)
@@ -237,8 +246,19 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
long spawnReadyTimeoutMs,
LongSupplier nowMillis, Runnable sleeper,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials,
Supplier<Set<String>> hostEnvNames) {
this(namePrefix, agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis,
sleeper, fleet, memberCredentials, hostEnvNames, null);
}
/** As above, plus the live full config for secret-bearing URI environment names. */
protected HerdrPeerLauncher(String namePrefix, AgentControl agents, WorkspaceControl spaces,
Map<String, FleetConfig.Profile> profiles, String defaultProfile,
Function<String, String> env, long spawnReadyTimeoutMs,
LongSupplier nowMillis, Runnable sleeper, Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials,
Supplier<Set<String>> hostEnvNames) {
Supplier<Set<String>> hostEnvNames, Supplier<FleetConfig> config) {
this.fleet = fleet;
this.namePrefix = namePrefix;
this.agents = agents;
@@ -251,6 +271,7 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
this.sleeper = sleeper;
this.memberCredentials = memberCredentials;
this.hostEnvNames = hostEnvNames != null ? hostEnvNames : () -> System.getenv().keySet();
this.config = config;
}
// --- adapter seams -------------------------------------------------------------------------
@@ -520,6 +541,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
req.sessionName(), spawned.agentSessionId(), spawned.receipt());
}
/** The herdr daemon that owns this launcher's pane coordinates. */
public HerdrClient herdr() {
return agents.herdr();
}
@Override
public String effectiveCwd(SpawnRequest req) {
return effectiveCwd(req.profileName(), req.requestedCwd(), req.callerCwd());
@@ -1002,6 +1028,13 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
* applied before the login shell runs and a sourced file can (and did) undo it. The control is
* the ZDOTDIR scrub ({@link #applyEnvironmentAllowListPolicy}); {@code known}/{@code allow}
* remain as reporting only via {@link #logCredentialGap}.
*
* <p>CB-633 follow-up (#192): under {@code allow-list} this method does NOT call {@link
* #logCredentialGap} itself — at this point (called from {@link #baseEnv}, before {@link
* #applyEnvironmentAllowListPolicy} runs) we do not yet know whether the pane's shell is zsh, so
* we cannot yet pick correct wording. That decision, and the call, are deferred entirely to
* {@link #applyEnvironmentAllowListPolicy}, which knows by then whether the scrub will actually
* run.
*/
private void applyMemberCredentialPolicy(Map<String, String> workerEnv) {
FleetConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get();
@@ -1010,14 +1043,16 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
}
if (!creds.isAllowList()) {
overlayBlockedCredentials(workerEnv, creds);
logCredentialGap(creds, null);
}
logCredentialGap(creds);
}
/** Put {@link #BLOCKED_CREDENTIAL_SENTINEL} over every blocked name in the pane-creation env map. */
private static void overlayBlockedCredentials(Map<String, String> workerEnv,
FleetConfig.MemberCredentials creds) {
for (String name : creds.blockedSet()) {
private void overlayBlockedCredentials(Map<String, String> workerEnv,
FleetConfig.MemberCredentials creds) {
Set<String> blocked = new java.util.TreeSet<>(creds.blockedSet());
blocked.addAll(brokerUriEnvNames());
for (String name : blocked) {
workerEnv.put(name, BLOCKED_CREDENTIAL_SENTINEL);
}
}
@@ -1042,6 +1077,29 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
* name. It stays a one-off decision because it is a live handle to the operator's ssh-agent, not
* a value — a member holding it can sign with every key the agent holds, so letting it ride in
* on the generic {@code allow:} list would hand that out for an unrelated reason.
*
* <p>fleetd #213: which shell decides the zsh gate, and where the generated directory lives,
* both depend on whether {@code memberHerdrSocket:} is configured — see {@link
* #memberHerdrSocketConfigured()}'s javadoc for why fleetd's own {@code $SHELL} and {@code
* java.io.tmpdir} describe the wrong process once member panes run under a different OS user.
* <ul>
* <li>{@code memberHerdrSocket} ABSENT (today's only mode): byte-identical to before this
* fix — fleetd's own {@code $SHELL} decides zsh, and the directory is generated under
* {@code java.io.tmpdir}.</li>
* <li>{@code memberHerdrSocket} PRESENT: the configured {@code memberLoginShell:} decides
* zsh instead — fleetd's own {@code $SHELL} is never consulted, since it names a
* different user's shell, not the member's. Absent/non-zsh falls back exactly like the
* non-zsh case below. When it IS zsh, the directory still cannot go under {@code
* java.io.tmpdir} (mode 0700, unreadable by another uid — the exact gap fleetd #213
* exists to close), so it is generated under {@code worktreeRoot} instead and shared
* read-only with {@code worktreeGroup} — the same group {@link
* dev.ltms.fleet.session.Worktrees#shareWithGroup} already uses, reused rather than
* inventing a second group key. Either one missing means the scrub cannot be guaranteed
* reachable by the member, which is the same "cannot guarantee the scrub runs" case as a
* non-zsh shell, so it gets the identical fallback.</li>
* </ul>
* In every branch: never refuse to spawn. A degraded credential control must not become an
* outage for an opt-in feature.
*/
private Path applyEnvironmentAllowListPolicy(FleetConfig.Profile cfg, Launch launch) {
FleetConfig.MemberCredentials creds = memberCredentials == null ? null : memberCredentials.get();
@@ -1049,8 +1107,14 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
return null;
}
Set<String> allowed = derivedAllowedNames(creds, launch);
String loginShell = resolveEnv("SHELL");
boolean zsh = loginShell != null && (loginShell.endsWith("/zsh") || loginShell.equals("zsh"));
boolean memberHerdrSocket = memberHerdrSocketConfigured();
// fleetd #213 defect 1: under memberHerdrSocket the member pane runs as a DIFFERENT OS
// user, so fleetd's own $SHELL says nothing about what that pane runs — resolveEnv("SHELL")
// must not even be called on this path, only the explicit memberLoginShell: config can
// answer it. With memberHerdrSocket absent, nothing here changes: fleetd's own $SHELL is
// still the input, exactly as before this fix.
String loginShell = memberHerdrSocket ? configuredMemberLoginShell() : resolveEnv("SHELL");
boolean zsh = isZshShell(loginShell);
if (!zsh) {
// A non-zsh login shell ignores ZDOTDIR entirely: NO scrub would run, so pretending
// otherwise would be worse than saying so. Warn loudly and fall back to the CB-596
@@ -1061,13 +1125,38 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
// logCredentialGap's WARN (below) is the only signal for this path.
warnNonZsh(loginShell);
overlayBlockedCredentials(launch.env(), creds);
logCredentialGap(creds);
logCredentialGap(creds, null);
return null;
}
Path parentDir;
String group = null;
if (memberHerdrSocket) {
// fleetd #213 defect 2: java.io.tmpdir is fleetd's own per-user temp dir (mode 0700 on
// macOS) — a member running as a different uid cannot even traverse it, let alone read
// the generated files. worktreeRoot is the only configured location a different-uid
// member can be given access to, and only WITH worktreeGroup to grant that access —
// absent either, the scrub cannot be guaranteed reachable, so this falls back exactly
// like the non-zsh case above rather than generating a directory nothing can read.
parentDir = memberScrubParentDir();
group = memberGroup();
if (parentDir == null || group == null) {
warnCannotShareScrubDirectory();
overlayBlockedCredentials(launch.env(), creds);
logCredentialGap(creds, null);
return null;
}
} else {
parentDir = Path.of(System.getProperty("java.io.tmpdir"));
}
// Only reached when the scrub is actually about to run — the count below describes that
// scrub, so it must not be logged before this gate (see the non-zsh branch above).
// scrub, so it must not be logged before this gate (see the non-zsh branch above). Same
// reasoning gates logCredentialGap's wording: passing the derived `allowed` set (non-null)
// here, and ONLY here, is what tells it the scrub will really blank an unkept name — #192.
logAllowListCoverage(allowed);
Path dir = EnvAllowListScrub.generate(Path.of(System.getProperty("java.io.tmpdir")), allowed);
logCredentialGap(creds, allowed);
Path dir = memberHerdrSocket
? EnvAllowListScrub.generate(parentDir, allowed, group)
: EnvAllowListScrub.generate(parentDir, allowed);
launch.env().put("ZDOTDIR", dir.toAbsolutePath().toString());
log.info("memberCredentials policy=allow-list: profile={} generated ZDOTDIR {} — derived "
+ "allow-list holds {} name(s); the pane reports allowed N of M at release",
@@ -1075,21 +1164,76 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
return dir;
}
/** True when {@code shell} is a zsh login shell path or bare name — the ZDOTDIR gate. */
private static boolean isZshShell(String shell) {
return shell != null && (shell.endsWith("/zsh") || shell.equals("zsh"));
}
/**
* fleetd #213: the configured {@code memberLoginShell:}, or {@code null} when unconfigured.
* Called ONLY from the {@code memberHerdrSocket}-configured branch of {@link
* #applyEnvironmentAllowListPolicy} — fleetd's own {@code $SHELL} is never read on that path.
* {@link #config} being {@code null} (an older test call site, or a launcher that never
* threaded the full config through) is treated the same as "not configured".
*/
private String configuredMemberLoginShell() {
FleetConfig cfg = config == null ? null : config.get();
return cfg == null ? null : cfg.memberLoginShell();
}
/**
* fleetd #213: {@code worktreeRoot}, as the ZDOTDIR scrub's parent directory under {@code
* memberHerdrSocket}, or {@code null} when unconfigured — the same "cannot guarantee the scrub
* runs" gap as {@link #memberGroup()} being unset (see {@link
* #applyEnvironmentAllowListPolicy}). Deliberately no sibling-of-repo-root default here, unlike
* {@code GitWorktrees}' own {@code worktreeRoot} resolution: that default is a convenience for
* provisioning a worktree that will exist regardless, whereas an unconfigured value here means
* fleetd has no operator-endorsed location to put a credential-bearing directory a different OS
* user must reach, so falling back to the overlay is the honest answer, not a guess.
*/
private Path memberScrubParentDir() {
FleetConfig cfg = config == null ? null : config.get();
if (cfg == null || cfg.worktreeRoot() == null || cfg.worktreeRoot().isBlank()) {
return null;
}
return Path.of(cfg.worktreeRoot());
}
/**
* fleetd #213: the configured {@code worktreeGroup:}, or {@code null} when unset/blank. Reuses
* the group {@link dev.ltms.fleet.session.Worktrees#shareWithGroup} already establishes for
* provisioned worktrees, rather than a second group key — see {@link
* #applyEnvironmentAllowListPolicy}.
*/
private String memberGroup() {
FleetConfig cfg = config == null ? null : config.get();
if (cfg == null || cfg.worktreeGroup() == null || cfg.worktreeGroup().isBlank()) {
return null;
}
return cfg.worktreeGroup();
}
/**
* The full kept-name set for this spawn: the profile-derived names, unioned with {@code
* memberCredentials.allow:} (CB-633 follow-up — previously ignored by this whole policy), the
* ssh-agent handle when explicitly allowed, and the exact keys of THIS launch's own env map.
*/
private Set<String> derivedAllowedNames(FleetConfig.MemberCredentials creds, Launch launch) {
Set<String> brokerUriEnvNames = brokerUriEnvNames();
Set<String> allowed = new java.util.TreeSet<>(
MemberEnvAllowList.derive(profiles.values(), creds.allowSet()));
MemberEnvAllowList.derive(profiles.values(), creds.allowSet(), brokerUriEnvNames));
if (creds.sshAuthSockAllowed()) {
allowed.add(SSH_AUTH_SOCK);
} // blocked by default: absent from the set ⇒ blanked by the scrub like any other name
allowed.addAll(launch.env().keySet());
allowed.removeAll(brokerUriEnvNames);
return allowed;
}
private Set<String> brokerUriEnvNames() {
return MemberEnvAllowList.brokerUriEnvNames(config == null ? null : config.get());
}
/**
* CB-633 follow-up: one INFO line per allow-list spawn WHOSE SCRUB ACTUALLY RUNS, so an operator
* can read a single log line and know the scrub ran and how much of the visible environment it
@@ -1126,6 +1270,30 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
}
}
/** Guards {@link #warnCannotShareScrubDirectory} to one WARN per launcher instance. */
private final AtomicBoolean cannotShareScrubDirWarned = new AtomicBoolean();
/**
* fleetd #213: {@code memberHerdrSocket} is configured and the member login shell IS zsh, but
* {@code worktreeRoot} and/or {@code worktreeGroup} is missing, so the generated ZDOTDIR cannot
* be placed anywhere the member's OS user can reach — {@code java.io.tmpdir} is fleetd's own
* 0700 temp dir, unreadable by another uid, which is the exact gap this ticket exists to close.
* Say so once per launcher instance, instead of either generating a directory nothing can read
* (protection theatre) or refusing to spawn (turning a degraded credential control into an
* outage for an opt-in feature).
*/
private void warnCannotShareScrubDirectory() {
if (cannotShareScrubDirWarned.compareAndSet(false, true)) {
log.warn("memberCredentials policy=allow-list: memberHerdrSocket is configured and the "
+ "member login shell is zsh, but worktreeRoot and/or worktreeGroup is not "
+ "configured — the generated ZDOTDIR cannot be placed where the member's OS "
+ "user can read it (java.io.tmpdir is fleetd's own, unreadable by another uid), "
+ "so the scrub cannot be guaranteed to run. Falling back to the CB-596 sentinel "
+ "overlay. Configure both worktreeRoot and worktreeGroup to enable the "
+ "allow-list scrub under memberHerdrSocket.");
}
}
/**
* CB-633 teardown half: read the pane's scrub report (the denominator report the generated
* scrub wrote) and delete the directory. Called from {@link #stop}, which is the one funnel
@@ -1171,18 +1339,132 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
private static final Pattern CREDENTIAL_SHAPED_NAME =
Pattern.compile("(?i).*(TOKEN|SECRET|_KEY|APIKEY|PASSWORD|CREDENTIAL|AUTH).*");
/** Guards {@link #logCredentialGap} to one WARN per launcher instance, not one per spawn. */
private final AtomicBoolean credentialGapLogged = new AtomicBoolean();
/**
* Guards the {@code effectiveAllowed == null} branch of {@link #logCredentialGap} — the
* genuinely-unprotected report (deny-by-default, and the allow-list non-zsh fallback) — to one
* WARN per launcher instance, not one per spawn.
*
* <p>CB-633 follow-up (#192): kept SEPARATE from {@link #allowListGapLogged} on purpose.
* {@code memberCredentials} is a live, re-read-per-spawn supplier, so the policy can change
* between two spawns on the same launcher. A single shared flag would let a harmless allow-list
* INFO on spawn 1 permanently suppress the real deny-by-default WARN a later spawn deserves —
* the report that matters most getting hidden by the report that doesn't. Two flags mean each
* report kind fires exactly once, independent of what the other kind already logged.
*/
private final AtomicBoolean unprotectedGapLogged = new AtomicBoolean();
/**
* Guards the {@code effectiveAllowed != null} branch of {@link #logCredentialGap} — the
* allow-list-scrub-covered report — to one INFO per launcher instance. See {@link
* #unprotectedGapLogged}'s javadoc for why this is a separate flag rather than a shared one.
*/
private final AtomicBoolean allowListGapLogged = new AtomicBoolean();
/**
* fleetd #185 stage 2: guards {@link #warnUnknownMemberEnvironment} to one WARN per launcher
* instance, not one per spawn — the same one-per-instance shape as {@link #unprotectedGapLogged}
* and {@link #allowListGapLogged}, kept as its own flag for the same reason those two are split:
* this mode is orthogonal to which of the other two branches would otherwise have fired.
*/
private final AtomicBoolean unknownMemberEnvironmentWarned = new AtomicBoolean();
/**
* fleetd #185 stage 2: whether {@code memberHerdrSocket:} is configured, i.e. member panes run
* on a second herdr owned by a different OS user than the daemon's own process. Re-read from the
* live config on every call (same hot-reload shape as {@link #memberCredentials}), never cached,
* so a config reload takes effect on the next spawn without a restart.
*
* <p>{@link #config} is {@code null} on any call site that never threaded the full config
* through (every production {@code HerdrPeerLauncher} does; a handful of older tests do not) —
* treated the same as "not configured", which is the correct, permissive default: it is exactly
* today's single-daemon behaviour.
*/
private boolean memberHerdrSocketConfigured() {
if (config == null) {
return false;
}
FleetConfig cfg = config.get();
return cfg != null && cfg.memberHerdrSocket() != null && !cfg.memberHerdrSocket().isBlank();
}
/**
* fleetd #185 stage 2: the single replacement WARN for {@link #logCredentialGap}'s usual
* conclusions when {@code memberHerdrSocket:} is configured. {@link #hostEnvNames} (and
* everything derived from it — {@code known}/{@code allow} coverage, the allow-list scrub's
* derived set) describes the DAEMON's own environment; under this config key member panes run as
* a different OS user with a different environment entirely, so neither "every member pane
* inherits them UNBLOCKED" nor "the scrub blanks them" is evidence-backed here — both would be
* reporting on the wrong process. Logged once, names the config key, and states the honest
* conclusion: the gap for member panes is UNKNOWN, not clean, so {@code memberCredentials} cannot
* be verified from this daemon. The one count it does report is scoped explicitly to fleetd's own
* environment, never presented as if it said anything about the member's — see {@link
* #logCredentialGap}'s javadoc for why this branch exists.
*/
private void warnUnknownMemberEnvironment(FleetConfig.MemberCredentials creds) {
if (!unknownMemberEnvironmentWarned.compareAndSet(false, true)) {
return;
}
Set<String> covered = new HashSet<>(creds.known());
covered.addAll(creds.allow());
Set<String> hostNames = hostEnvNames.get();
long gapInFleetdsOwnEnv = hostNames.stream()
.filter(name -> CREDENTIAL_SHAPED_NAME.matcher(name).matches())
.filter(name -> !covered.contains(name))
.count();
log.warn("memberCredentials gap: memberHerdrSocket is configured, so member panes run under "
+ "a different OS user than fleetd's own process, with a different environment "
+ "entirely — fleetd has no channel to read that user's environment. {} of the "
+ "{} names in fleetd's OWN environment are credential-shaped and not on "
+ "known:/allow:, but that count describes fleetd's process, not the member "
+ "herdr's. The credential gap for member panes is UNKNOWN, not clean, and "
+ "memberCredentials cannot be verified from here.",
gapInFleetdsOwnEnv, hostNames.size());
}
/**
* CB-596 criterion 4: a credential-shaped host env var name on neither {@code known} nor
* {@code allow} is not silently allowed — it is reported. {@link #hostEnvNames} enumerates the
* daemon's own environment (see that field's javadoc for why the daemon's env is read rather
* than the spawned pane's, which the daemon has no channel to inspect at spawn time); this logs
* every such NAME, at WARN, at most once per launcher instance — never a value, a prefix of a
* value, or a hash of a value, so the log itself cannot leak anything.
* every such NAME — never a value, a prefix of a value, or a hash of a value, so the log itself
* cannot leak anything.
*
* <p>CB-633 follow-up (#192): {@code effectiveAllowed} picks the wording, and it must NOT be
* picked from {@code creds.isAllowList()} — see {@link #applyEnvironmentAllowListPolicy}'s
* javadoc for the reasoning this mirrors. {@code null} means no scrub-derived allow-list was
* computed for this call — true on the deny-by-default path AND on the allow-list non-zsh
* fallback, where nothing is ever scrubbed — so the whole gap is real and gets the WARN,
* unchanged from before this fix. Non-null means this call came from the zsh branch of {@link
* #applyEnvironmentAllowListPolicy}, reachable ONLY after that method's own zsh gate — but
* {@code effectiveAllowed} is a SUPERSET of {@code known ∪ allow}: {@link MemberEnvAllowList#derive}
* also unions in every profile's {@code gitTokenEnv}/{@code gitHostEnv}/{@code tokenEnv}/
* {@code env:} keys, and {@link #derivedAllowedNames} further unions in this very spawn's own
* env keys — so a name can be in the gap (uncovered by {@code known}/{@code allow}) AND still be
* kept by the derived allow-list, in which case the scrub does NOT blank it and the member DOES
* inherit it. Lead review on #192 caught this: the first cut of this fix reported the WHOLE gap
* as scrub-blanked without checking that, which reported a real leak as safe — the exact
* inversion #192 exists to remove. So on this path the gap is split with {@link
* MemberEnvAllowList#keeps}, the SAME predicate the generated scrub itself evaluates, so this
* split cannot drift from what the scrub actually does: the names it says are kept get the WARN
* (same severity, and same guard, as the deny-by-default case — a name genuinely reaching a
* member unprotected is equally serious whichever path put it there), and the names it says are
* blanked keep the INFO.
*
* <p>fleetd #185 stage 2: everything above assumes the member pane runs under the same OS user
* as the daemon, so {@link #hostEnvNames} mirrors what the pane inherits — see that field's
* javadoc. When {@code memberHerdrSocket:} is configured that assumption is false: the member
* pane runs on a second herdr owned by a <em>different</em> user, and neither conclusion below
* ("inherits them UNBLOCKED" / "the scrub blanks them") is backed by evidence about that user's
* environment. So this method checks that first and, when configured, reports the honest
* "unknown, not clean" conclusion instead — see {@link #warnUnknownMemberEnvironment}. When
* {@code memberHerdrSocket:} is absent (the default, and the only mode this host runs) this
* branch is never taken and every line below is unchanged.
*/
private void logCredentialGap(FleetConfig.MemberCredentials creds) {
private void logCredentialGap(FleetConfig.MemberCredentials creds, Set<String> effectiveAllowed) {
if (memberHerdrSocketConfigured()) {
warnUnknownMemberEnvironment(creds);
return;
}
Set<String> covered = new HashSet<>(creds.known());
covered.addAll(creds.allow());
List<String> gap = hostEnvNames.get().stream()
@@ -1193,7 +1475,37 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
if (gap.isEmpty()) {
return;
}
if (credentialGapLogged.compareAndSet(false, true)) {
if (effectiveAllowed == null) {
warnGapUnprotected(gap);
return;
}
List<String> keptByDerivedList = gap.stream()
.filter(name -> MemberEnvAllowList.keeps(effectiveAllowed, name))
.toList();
List<String> blankedByScrub = gap.stream()
.filter(name -> !MemberEnvAllowList.keeps(effectiveAllowed, name))
.toList();
if (!keptByDerivedList.isEmpty() && unprotectedGapLogged.compareAndSet(false, true)) {
log.warn("memberCredentials gap: {} credential-shaped env var name(s) are on neither "
+ "known: nor allow: — the derived allow-list keeps them anyway (a profile's "
+ "gitTokenEnv/gitHostEnv/tokenEnv/env: names one, or this spawn injects it), "
+ "so every member pane inherits them UNBLOCKED — {}. Add each to "
+ "memberCredentials.known (or .allow if a member legitimately needs it), or "
+ "remove it from whatever profile setting derives it in.",
keptByDerivedList.size(), keptByDerivedList);
}
if (!blankedByScrub.isEmpty() && allowListGapLogged.compareAndSet(false, true)) {
log.info("memberCredentials gap: {} credential-shaped env var name(s) are on neither "
+ "known: nor allow: — {}. The allow-list scrub blanks them anyway (they "
+ "are not on the derived allow-list), so no member pane keeps them; add "
+ "each to memberCredentials.known or .allow to make that explicit.",
blankedByScrub.size(), blankedByScrub);
}
}
/** The deny-by-default (and allow-list non-zsh fallback) WARN — unchanged byte-for-byte by #192. */
private void warnGapUnprotected(List<String> gap) {
if (unprotectedGapLogged.compareAndSet(false, true)) {
log.warn("memberCredentials gap: {} credential-shaped env var name(s) are on neither "
+ "known: nor allow: — every member pane inherits them UNBLOCKED — {}. "
+ "Add each to memberCredentials.known (blocked by default) or .allow "
@@ -40,8 +40,9 @@ import java.util.TreeSet;
* operator's own explicit list. Before this, {@code policy: allow-list} silently ignored every name
* an operator wrote under {@code allow:} unless a profile happened to carry it too, which meant
* turning the policy on could blank credentials working members already depended on. {@code
* SSH_AUTH_SOCK} is the one exception: even when the operator lists it under {@code allow:}, it is
* excluded here and added back ONLY by the caller when {@code sshAuthSock: allow} is explicitly set
* SSH_AUTH_SOCK} and configured broker URI environment names are exceptions: even when the operator
* lists them under {@code allow:}, they are excluded here. {@code SSH_AUTH_SOCK} is added back ONLY
* by the caller when {@code sshAuthSock: allow} is explicitly set
* (see {@link #SSH_AUTH_SOCK}'s javadoc) — it is a live handle to the operator's own ssh-agent, not
* a value, so treating it like any other allow-listed name would hand a member every key the
* operator's agent holds the moment they typed the name under {@code allow:} for an unrelated
@@ -106,6 +107,16 @@ public final class MemberEnvAllowList {
* run-to-run.
*/
public static Set<String> derive(Collection<FleetConfig.Profile> profiles, Set<String> configuredAllow) {
return derive(profiles, configuredAllow, Set.of());
}
/**
* As {@link #derive(Collection, Set)}, while excluding names that fleetd knows carry credentials.
* A configured broker URI contains its AMQP password inline, so it must never reach a member,
* even when an operator put its variable name in {@code memberCredentials.allow:}.
*/
public static Set<String> derive(Collection<FleetConfig.Profile> profiles, Set<String> configuredAllow,
Set<String> excludedNames) {
Set<String> derived = new TreeSet<>(INFRASTRUCTURE_PASSTHROUGH);
if (profiles != null) {
for (FleetConfig.Profile p : profiles) {
@@ -124,9 +135,37 @@ public final class MemberEnvAllowList {
}
}
}
if (excludedNames != null) {
derived.removeAll(excludedNames);
}
return Set.copyOf(derived);
}
/**
* The host environment names whose values are AMQP URIs with inline passwords. Both broker
* connections belong to fleetd, never to a member pane. Blank and absent configuration changes
* nothing.
*
* <p><b>How strong this exclusion is depends on the policy, and the difference matters.</b>
* Under {@code policy: allow-list} it is enforced by the generated ZDOTDIR scrub, which runs
* AFTER the pane's shell has sourced the operator's chain — so a login shell that re-exports the
* name is still blanked. Under the deny-list policy there is no scrub: the name is only removed
* from the pre-shell env map, and a login shell that sources the operator's secret store
* re-exports it. That is the long-standing weakness of deny-list (a sourced file can undo it),
* not something this exclusion introduces, but it means deny-list deployments do NOT get this
* guarantee. The same caveat applies to the non-zsh path, which has no scrub at all — see
* {@code HerdrPeerLauncher#applyEnvironmentAllowListPolicy}.
*/
public static Set<String> brokerUriEnvNames(FleetConfig config) {
if (config == null) {
return Set.of();
}
Set<String> names = new TreeSet<>();
addUriEnvIfPresent(names, config.broker());
addUriEnvIfPresent(names, config.coordinator());
return Set.copyOf(names);
}
/**
* Whether {@code name} survives the scrub when {@code allowedNames} is the derived set: an exact
* match, or an infrastructure-prefixed name ({@code LC_*}). Prefix rules live ONLY here and in
@@ -146,4 +185,16 @@ public final class MemberEnvAllowList {
into.add(name);
}
}
private static void addUriEnvIfPresent(Set<String> into, FleetConfig.Broker broker) {
if (broker != null && broker.hasUriEnv()) {
into.add(broker.uriEnv());
}
}
private static void addUriEnvIfPresent(Set<String> into, FleetConfig.Coordinator coordinator) {
if (coordinator != null && coordinator.hasUriEnv()) {
into.add(coordinator.uriEnv());
}
}
}
@@ -116,12 +116,23 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces,
Map<String, FleetConfig.Profile> profiles, String defaultProfile,
Function<String, String> env,
long spawnReadyTimeoutMs, long spawnReadyPollMs,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
long spawnReadyTimeoutMs, long spawnReadyPollMs,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, spawnReadyPollMs,
fleet, memberCredentials, null);
}
/** Production constructor, plus the live config for URI environment exclusions. */
public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces,
Map<String, FleetConfig.Profile> profiles, String defaultProfile,
Function<String, String> env, long spawnReadyTimeoutMs, long spawnReadyPollMs,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials,
Supplier<FleetConfig> config) {
this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs,
System::currentTimeMillis, () -> sleepUninterruptibly(spawnReadyPollMs),
defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials);
defaultConfigRoot(), defaultDiscoveryRoot(), fleet, memberCredentials, config);
}
/**
@@ -182,8 +193,20 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
Path configRoot, Path discoveryRoot,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials) {
this(agents, spaces, profiles, defaultProfile, env, spawnReadyTimeoutMs, nowMillis, sleeper,
configRoot, discoveryRoot, fleet, memberCredentials, null);
}
/** Full testability constructor, plus the live config for URI environment exclusions. */
public OpenCodeLauncher(AgentControl agents, WorkspaceControl spaces,
Map<String, FleetConfig.Profile> profiles, String defaultProfile,
Function<String, String> env, long spawnReadyTimeoutMs,
LongSupplier nowMillis, Runnable sleeper, Path configRoot, Path discoveryRoot,
Supplier<FleetConfig.Fleet> fleet,
Supplier<FleetConfig.MemberCredentials> memberCredentials,
Supplier<FleetConfig> config) {
super(NAME_PREFIX, agents, spaces, profiles, defaultProfile, env,
spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials);
spawnReadyTimeoutMs, nowMillis, sleeper, fleet, memberCredentials, null, config);
this.configRoot = configRoot;
this.discovery = new OpenCodeSessionDiscovery(discoveryRoot);
}
@@ -1,58 +1,87 @@
package dev.ltms.fleet.member;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.sqlite.SQLiteConfig;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.stream.Stream;
import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.concurrent.atomic.AtomicBoolean;
/**
* Resolves the opencode session id for a fleetd worker from opencode's on-disk storage — the
* only place this adapter touches opencode's private layout, and deliberately the <em>only</em>
* class that does.
*
* <p><strong>Why this is isolated behind one seam.</strong> The layout is version-coupled and not a
* stable contract: opencode writes one JSON file per session under
* {@code <storageRoot>/session/<projectID>/<ses_*.json>}, and each record carries a
* {@code "version"} field (e.g. {@code "1.1.31"}), so the exact directory shape, file naming, and
* field names can move between opencode releases. opencode also ships a headless HTTP server that
* may supersede file scanning entirely. Everything this adapter knows about that private storage —
* its shape, naming, and field names — lives here, so a layout change, or a switch to the HTTP
* server, changes exactly one class and nothing in {@link OpenCodeLauncher}.
* <p><strong>Why this is isolated behind one seam.</strong> The layout is version-coupled and not
* a stable contract: opencode persists its session state in a SQLite database at
* {@code <storageRoot>/opencode.db} (a {@code session} table, one row per session, keyed by id and
* carrying a {@code directory} column). That schema can move between opencode releases exactly
* like the JSON-file layout it replaced did (opencode migrated off a one-JSON-file-per-session
* tree under {@code <storageRoot>/storage/session/<projectID>/ses_*.json} in January 2026 — that
* tree is now a frozen migration artefact nothing writes, which is why this class no longer reads
* it). opencode also ships a headless HTTP server that may supersede both of these entirely.
* Everything this adapter knows about that private storage — its shape and column names — lives
* here, so a layout change, or a switch to the HTTP server, changes exactly one class and nothing
* in {@link OpenCodeLauncher}.
*
* <p>The determinism that makes this useful is structural, not a guess: every fleetd worker runs
* in its own unique git worktree, so the record's {@code directory} (its project root) equals the
* in its own unique git worktree, so the row's {@code directory} (its project root) equals the
* worker's cwd identifies <em>its</em> session unambiguously. We match on {@code directory} rather
* than diffing {@code opencode session list} before/after — that races under concurrent spawns, and
* the CLI listing does not even show the directory.
*
* <p>All reads are best-effort and never throw: a missing or unreadable storage root, a record that
* fails to parse, or a directory with no record yet all yield {@code null}, and the caller (the
* session handle) treats that as "identity not resolved yet" and retries later.
* <p>All reads are best-effort and never throw: a missing or unreadable database, a query that
* fails, or a directory with no row yet all yield {@code null}, and the caller (the session
* handle) treats that as "identity not resolved yet" and retries later. The database is opened
* read-only and never written to: opencode itself may be running and writing it concurrently (WAL
* mode), and this class must never disturb that.
*/
final class OpenCodeSessionDiscovery {
private static final Logger log = LoggerFactory.getLogger(OpenCodeSessionDiscovery.class);
private final Path storageRoot; // e.g. ~/.local/share/opencode (injectable for tests)
private final ObjectMapper json;
private final Path databasePath;
private final AtomicBoolean warnedMissingDatabase = new AtomicBoolean(false);
OpenCodeSessionDiscovery(Path storageRoot) {
this.storageRoot = storageRoot;
this.json = new ObjectMapper();
this.databasePath = storageRoot.resolve("opencode.db");
}
/**
* The opencode session id whose record references {@code directory} (the worker's cwd), or
* {@code null} when no record matches yet. When several records share the directory — e.g.
* repeated spawns into the same worktree — the <em>most recently modified</em> one wins: it is
* A connection to {@link #databasePath} opened with SQLite's {@code SQLITE_OPEN_READONLY}
* flag: it never creates the file, never writes, and never touches WAL or journal mode.
* opencode may be running and writing this database concurrently, and this class must never
* disturb it.
*
* <p>Package-private so a test can hold the connection and prove it refuses a write. That is
* the only way to pin this property: making the file unwritable does <em>not</em> work,
* because SQLite silently downgrades a read-write open of an unwritable file to read-only, so
* such a test passes whether or not the flag is set.
*/
Connection openReadOnly() throws SQLException {
SQLiteConfig config = new SQLiteConfig();
config.setReadOnly(true);
return config.createConnection("jdbc:sqlite:" + databasePath);
}
/**
* The opencode session id whose row references {@code directory} (the worker's cwd), or
* {@code null} when no row matches yet. When several rows share the directory — e.g. repeated
* spawns into the same worktree — the row with the highest {@code time_updated} wins: it is
* the session the pane most likely corresponds to.
*
* <p>Never throws: a missing {@code storageRoot}, an unreadable/malformed record, or a
* directory that has not been persisted yet all resolve to {@code null} rather than failing a
* spawn. A fleetd worker's session record is written lazily (when the session is first
* persisted), so {@code null} here is the normal answer right after the pane is ready, and the
* caller retries later.
* <p>Never throws: a missing {@code opencode.db}, a locked/unreadable database, a query
* failure, or a directory that has not been persisted yet all resolve to {@code null} rather
* than failing a spawn. A fleetd worker's session row is written lazily (when the session is
* first persisted), so {@code null} here is the normal answer right after the pane is ready,
* and the caller retries later.
*
* @param directory the worker's cwd, as resolved for this spawn
* @return the matching session id, or {@code null} if none is known yet
@@ -61,62 +90,31 @@ final class OpenCodeSessionDiscovery {
if (directory == null || directory.isBlank()) {
return null;
}
Path sessionRoot = storageRoot.resolve("session");
if (!Files.isDirectory(sessionRoot)) {
if (!Files.isRegularFile(databasePath)) {
if (warnedMissingDatabase.compareAndSet(false, true)) {
log.warn("opencode session database not found at {} — opencode's on-disk layout "
+ "may have moved again; session discovery will keep returning null",
databasePath);
}
return null;
}
String best = null;
long bestMtime = Long.MIN_VALUE;
try (Stream<Path> projectDirs = Files.list(sessionRoot)) {
for (Path projectDir : projectDirs.filter(Files::isDirectory).toList()) {
try (Stream<Path> records = Files.list(projectDir)) {
for (Path record : records.toList()) {
String id = matchId(record, directory);
if (id == null) {
continue;
}
long mtime = lastModifiedEpochMillis(record);
if (mtime > bestMtime) {
bestMtime = mtime;
best = id;
}
}
} catch (IOException ignored) {
// one project dir unreadable — skip it; another may still match
String sql = "SELECT id FROM session WHERE directory = ? ORDER BY time_updated DESC LIMIT 1";
try (Connection connection = openReadOnly();
PreparedStatement statement = connection.prepareStatement(sql)) {
statement.setString(1, directory);
try (ResultSet rows = statement.executeQuery()) {
if (rows.next()) {
return rows.getString("id");
}
}
} catch (IOException ignored) {
// storage root vanished or became unreadable — "no session known yet"
} catch (SQLException e) {
// Locked, corrupt, or otherwise unreadable — never fatal to a spawn. Not the
// "database moved" signal (the file exists), so this stays below WARN.
log.debug("opencode session database unreadable at {}: {}", databasePath, e.toString());
return null;
}
return best;
}
/**
* The record's session id when it references {@code directory}, else {@code null}. A record
* that is not JSON, lacks {@code id}/{@code directory}, or points at a different directory is
* simply not our session; a malformed one is skipped, never fatal.
*/
private String matchId(Path record, String directory) {
try {
JsonNode node = json.readTree(record.toFile());
JsonNode id = node == null ? null : node.get("id");
JsonNode dir = node == null ? null : node.get("directory");
if (id == null || dir == null || !directory.equals(dir.asText())) {
return null;
}
return id.asText();
} catch (IOException e) {
return null;
}
}
/** The record's last-modified epoch ms, or {@code Long.MIN_VALUE} if unreadable (never wins). */
private static long lastModifiedEpochMillis(Path record) {
try {
return Files.getLastModifiedTime(record).toMillis();
} catch (IOException e) {
return Long.MIN_VALUE;
}
log.debug("no opencode session row for directory (root={}, directory={})",
storageRoot, directory);
return null;
}
}
@@ -9,6 +9,7 @@ import dev.ltms.fleet.metrics.Metrics;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
@@ -167,15 +168,31 @@ public final class MessageService {
private static final class Task {
private final String ticket;
private final String target;
private final CompletableFuture<Reply> future = new CompletableFuture<>();
/**
* When this task was created (#137 fix): the tiebreaker for which of several open tasks on
* one target gets a recovered reply in {@link #abandon} — the oldest, since it is the one
* that has been waiting longest.
*/
private final long createdNanos;
private final CompletableFuture<Reply> future = new CompletableFuture<>();
/**
* When {@link #future} resolved, or {@code null} while it is still pending — the clock
* {@link #pruneTerminalTickets} measures the TTL from (#197). Deliberately a boxed
* {@code Long} rather than a {@code long} with a sentinel: {@link System#nanoTime} may
* legitimately return any value, zero and negatives included, so no numeric sentinel can mean
* "not stamped yet". Stamped by a {@code whenComplete} hook registered in the constructor, so
* every completion path stamps it — a reply, the completion fallback, a timeout, a failure,
* or an abandon on teardown — without each of those having to remember to.
*/
private volatile Long completedNanos;
private volatile Reply question;
private volatile String turnId;
private Task(String ticket, String target, long createdNanos) {
private Task(String ticket, String target, LongSupplier nowNanos) {
this.ticket = ticket;
this.target = target;
this.createdNanos = createdNanos;
this.createdNanos = nowNanos.getAsLong();
future.whenComplete((reply, ex) -> completedNanos = nowNanos.getAsLong());
}
}
@@ -357,21 +374,61 @@ public final class MessageService {
}
/**
* Route a worker's explicit {@code fleet_reply}: resolve an open send, or queue it in the
* inbox if no send is currently open. Unlike the bare {@link Rendezvous#resolve}, a no-waiter
* result is <em>not</em> a failure — the reply is held for later drain.
* Route a worker's explicit {@code fleet_reply}: resolve an open send, complete an async ticket
* still parked waiting on this exact turn's answer, or — only once neither applies — queue it in
* the inbox. Unlike the bare {@link Rendezvous#resolve}, a no-waiter result is <em>not</em> a
* failure — the reply is held for later drain.
*
* <p><strong>Do NOT use this for mid-turn questions.</strong> {@code fleet_ask} /
* {@link Rendezvous#resolveQuestion} must keep today's {@code NO_WAITER} behaviour — questions
* are interactive and must never be queued.
*
* @return always {@code true} — the reply either resolved a live send or was queued
* <p><strong>Ambiguous match also falls to the inbox.</strong> {@link #askAnsweredAsyncTasks}
* cannot actually return more than one entry today (see its own javadoc for why — in short,
* {@link #hasAsyncQuestion} keeps a target BUSY, so no second task can reach this state, for as
* long as an earlier one's {@code turnId} is still stamped). That is an emergent guarantee from
* two other facts, not one this method enforces, so this branch stays in as defence in depth
* rather than being removed as dead code: if it ever weakens, returning whichever candidate a
* {@code ConcurrentHashMap} iteration reaches first would let a genuine reply complete the
* <em>wrong</em> ticket — silently handing the lead something that reads like a correct answer to
* a delegation the worker never touched, which is worse than a failure because the lead acts on
* it. When more than one candidate exists, guessing is not safe: fall back to the inbox exactly
* as the zero-candidate case does, and let {@link #abandon} apply the eventual recovery
* deterministically instead.
*
* @return always {@code true} — the reply resolved a live send, completed a parked ticket, or
* was queued
*/
public boolean reply(String session, String content) {
if (rendezvous.resolve(session, content)) {
count(FleetMetrics.REPLIES, "path", "rendezvous");
return true; // a live send took it — unchanged fast path
}
// #137: no live rendezvous waiter, but this may be the worker's real fleet_reply resuming a
// turn that {@link #answer} already gave up waiting on. answer()'s own bounded wait (the
// primary's fleet_send{turnId} call, capped well under a minute) can time out and close its
// waiter long before the worker — now actually resuming real work — finishes and replies. That
// reply used to have nowhere to land but the session inbox, leaving the async ticket's future
// unresolved forever: fleet_poll{ticket} stayed PENDING until fleet_stop's abandon() forced it
// FAILED with a misleading "session released before it replied" reason, even though the reply
// had, in fact, arrived. Completing the matching ticket directly here means fleet_poll{ticket}
// sees the real reply instead.
List<Task> candidates = askAnsweredAsyncTasks(session);
if (candidates.size() == 1) {
Task orphan = candidates.get(0);
if (orphan.future.complete(new Reply(Outcome.REPLIED, content))) {
if (orphan.turnId != null) {
asyncTasksByTurn.remove(orphan.turnId, orphan);
}
count(FleetMetrics.REPLIES, "path", "async-recovered");
return true; // the ticket itself took it — no inbox stranding at all
}
} else if (candidates.size() > 1) {
List<String> tickets = candidates.stream().map(t -> t.ticket).toList();
log.warn("reply from {} matches {} open async tickets {} — cannot tell which one it "
+ "answers, queuing to the inbox instead of guessing", session, candidates.size(),
tickets);
}
inbox.publish(session, UUID.randomUUID().toString(), content);
// CB-640: record the stranding itself (not just the reply text) so fleet health can see a
// worker whose replies keep missing their waiter, not only the queue depth this leaves behind.
@@ -385,6 +442,41 @@ public final class MessageService {
return true; // held, not lost
}
/**
* Every still-open async task on {@code target} whose {@code fleet_ask} was already answered —
* its {@link Task#turnId} is stamped but its {@link Task#question} was cleared by {@link #answer}
* — yet whose future is not resolved yet (#137). Empty if no such task exists, including the
* common case where {@code target}'s worker never used {@code fleet_ask} at all (a task that was
* never asked has {@code turnId == null}, so it can never match here and only ever completes
* through the ordinary rendezvous fast path in {@link #reply}).
*
* <p><strong>Returns at most one entry today — verified, not assumed.</strong> {@link #send}
* refuses to open a waiter on {@code target} while {@link #hasAsyncQuestion} is true, and that
* check matches ANY task whose {@code turnId} is still stamped in {@code asyncTasksByTurn} —
* not only while its question is still open. {@link #answer} deliberately leaves that stamp in
* place ({@code clearAsyncQuestion(turnId, false)}) until the resumed turn's own future actually
* resolves, at which point {@link #finishAsyncTask} both removes the stamp AND completes that
* task's future in the same call. So a second task can never reach "{@code turnId} stamped, future
* still open" — the exact pair this method matches on — while a first one already holds it: by
* the time the stamp is gone, so is the eligibility. This is an emergent property of those two
* facts holding together, not something this method (or its callers) enforces on its own — flip
* {@code forgetTurn} to {@code true} in that one {@link #answer} call and it silently stops being
* true, with nothing left to fail loudly. The callers below still handle "more than one" as
* defence in depth against exactly that, not because they exercise it today: {@link #reply}
* treats it as unresolvable and falls back to the inbox; {@link #abandon} would pick the oldest
* deterministically (its own {@code matching} list has no such guarantee — see its javadoc).
*/
private List<Task> askAnsweredAsyncTasks(String target) {
List<Task> candidates = new ArrayList<>();
for (Task task : tasks.values()) {
if (target.equals(task.target) && task.question == null && task.turnId != null
&& !task.future.isDone()) {
candidates.add(task);
}
}
return candidates;
}
/** Record a counter sample when a registry is wired; a no-op in unit tests. */
private void count(String name, String... labels) {
if (metrics != null) {
@@ -426,19 +518,108 @@ public final class MessageService {
* <p>Resolving the waiter as a failure — rather than letting it time out — also means the
* outcome is counted, so a torn-down delegation stops being invisible to {@code /metrics}.
*
* @return true if a live waiter was failed
* <p><strong>#137 defence in depth.</strong> {@link #reply} already hands a worker's real
* {@code fleet_reply} straight to the async ticket it belongs to whenever exactly one is still
* parked waiting for it (see {@link #askAnsweredAsyncTasks}), so by the time a session is
* released its tasks are normally already resolved — this loop's {@code complete} calls are then
* harmless no-ops (a {@link CompletableFuture} can only resolve once). But should some other path
* someday strand a reply in the inbox without completing its ticket, checking
* {@link #hasStrandedReply(String)} here — before ever writing a failure — means a torn-down
* session whose worker in fact replied is still reported {@code REPLIED} with that reply's own
* text, never the misleading "the worker session was released before it replied" (which also
* means the snapshot/worktree recovery hint that follows it never prints once a reply exists).
*
* <p><strong>At most one task gets the recovered reply — and here, unlike {@link #reply}'s
* {@link #askAnsweredAsyncTasks}, {@code matching.size() >= 2} alone is reachable today.</strong>
* This method's {@code matching} filter has no {@code turnId != null} requirement, so it matches
* any plain (never-asked) open task too — and {@link #sendAsync} does not limit a target to one
* of those: a second {@code fleet_send{wait:false}} at a target that is still busy returns its own
* ticket immediately and simply parks its {@link #send} behind the target's session lock for up
* to {@link #ASYNC_TIMEOUT_MS}, exactly as {@code abandonFailsEveryPendingAsyncTicketForTheReleasedTarget}
* already proves. Before this fix, the loop below drained the strand once and then reused that
* same {@code Reply} for <em>every</em> task it walked past — so two open tasks really did both
* complete {@code REPLIED} with the same text (see the pre-fix loop in commit 97f6c33's parent).
* A stranded reply is one worker answer, so it can settle at most one open task on this target —
* never every open task, and never a guess. When more than one task is still open here, the
* recovered reply goes to the <em>oldest</em> (lowest {@link Task#createdNanos}) — it has been
* waiting longest, so it is the one most likely to be what the reply actually answers. Every
* other open task keeps the ordinary {@code WORKER_FAILED} path it would take without a stranded
* reply at all.
*
* <p><strong>{@code matching.size() >= 2} together with {@code hadStrandedReply} is a different
* question, and today it is defence in depth rather than a path this codebase's public API can
* drive.</strong> This class has exactly two sites that ever acquire a target's entry in
* {@code sessionLocks} — {@link #send} and {@link #answer} — and both open a {@link Rendezvous}
* waiter for that same target as the very first thing they do after acquiring the lock, then hold
* lock and waiter together for the rest of their critical section ({@link #send} also clears
* {@link #strandedReplies} right there, the instant it opens its waiter — before it ever enqueues
* delivery). So "the session lock is held" and "a live waiter is open for it" are the same fact
* throughout this class, and {@link #reply}'s fast path always resolves a currently-open waiter
* directly rather than stranding. The two facts this method wants therefore cannot be produced
* side by side: while the lock is held, a real reply resolves the open waiter directly and never
* reaches {@link #strandedReplies}; the instant the lock is free, any parked matching task's own
* {@link #send} that is scheduled next wins it and, by opening its waiter, clears the strand again
* before this method ever runs. There is no way to hold that lock open-but-unaccepted from outside
* {@link #send}/{@link #answer} to freeze a window in between. Constructing both facts at once
* through {@code sendAsync}/{@code reply}/{@code ask}/{@code answer} would need a race against
* virtual-thread scheduling, not a deterministic sequence — so the oldest-wins code below stays as
* defence in depth against a regression to that mechanism (e.g. clearing {@link #strandedReplies}
* on a narrower condition than "any acceptance"), not because today's test suite exercises the
* conjunction. {@code matching.size() >= 2} alone, without a strand, is exactly what
* {@code abandonFailsEveryPendingAsyncTicketForTheReleasedTarget} already covers.
*
* <p><strong>Do not "fix" that gap with a test that reaches past this class.</strong> A test can
* build both facts by calling {@link Rendezvous#close} itself on the waiter an accepted
* {@link #send} is still blocked on: the send keeps the lock, no waiter is registered any more, a
* second parked task stays open, and the next {@link #reply} then strands. That was checked, and
* such a test does go red against the pre-fix loop. But it only goes red because it broke the
* lock-and-waiter invariant above from outside — no caller of this class ever does that — so it
* pins a state production cannot reach, and would read to the next person as if it could.
*
* @return true if a live waiter or an async task was failed (never true for one recovered as a
* reply — see the note above)
*/
public boolean abandon(String target, String reason) {
boolean hadStrandedReply = hasStrandedReply(target);
// CB-640: the session is gone — nothing will ever accept or deliver into it now.
strandedReplies.remove(target);
queuedDeliveries.remove(target);
CompletableFuture<Rendezvous.Resolution> waiter = rendezvous.currentWaiter(target);
boolean failed = waiter != null && !waiter.isDone() && rendezvous.resolveFailure(waiter, reason);
boolean asyncFailed = false;
List<Task> matching = new ArrayList<>();
for (Task task : tasks.values()) {
if (target.equals(task.target) && task.question == null
&& task.future.complete(new Reply(Outcome.WORKER_FAILED, reason))) {
asyncFailed = true;
if (target.equals(task.target) && task.question == null && !task.future.isDone()) {
matching.add(task);
}
}
Task recoveryTask = null;
if (hadStrandedReply && !matching.isEmpty()) {
recoveryTask = matching.get(0);
for (Task candidate : matching) {
if (candidate.createdNanos < recoveryTask.createdNanos) {
recoveryTask = candidate;
}
}
}
Reply recovered = recoveryTask != null ? recoverStrandedReply(target) : null;
for (Task task : matching) {
boolean isRecovery = task == recoveryTask && recovered != null;
Reply outcome = isRecovery ? recovered : new Reply(Outcome.WORKER_FAILED, reason);
if (task.future.complete(outcome)) {
if (outcome.outcome() == Outcome.WORKER_FAILED) {
asyncFailed = true;
} else if (task.turnId != null) {
asyncTasksByTurn.remove(task.turnId, task);
}
} else if (isRecovery) {
// The recovered reply was already drained out of the inbox, but this task resolved
// through another path (e.g. a concurrent reply() or a second abandon() racing this
// one) between us choosing it and completing it here. Put the reply back rather than
// lose it silently — it may still belong to some other still-open task, or the next
// caller that drains this target's inbox.
inbox.publish(target, UUID.randomUUID().toString(), recovered.text());
}
}
if (failed) {
@@ -447,6 +628,25 @@ public final class MessageService {
return failed || asyncFailed;
}
/**
* Drain {@code target}'s inbox and hand its content back as a {@link Outcome#REPLIED} result
* (#137 defence in depth for {@link #abandon}) — {@code null} if it turned out empty (the
* stranding fact raced away, e.g. a lead's own {@code fleet_poll} on the raw session already
* drained it first). When more than one message is queued, only the newest is the worker's actual
* final answer ({@link #drainReplies} returns them oldest-first).
*
* <p>This does drain (removes the messages from the inbox) before the caller knows whether the
* task it is recovering for will actually accept them — {@link #abandon} is the one that puts a
* reply back if its {@code complete} call turns out to lose the race.
*/
private Reply recoverStrandedReply(String target) {
var messages = drainReplies(target);
if (messages.isEmpty()) {
return null;
}
return new Reply(Outcome.REPLIED, messages.get(messages.size() - 1).content());
}
/**
* Acknowledge a specific reply by {@code msgId} for {@code target}. Removes it from the inbox
* so that a subsequent drain or peek no longer returns it.
@@ -721,7 +921,7 @@ public final class MessageService {
*/
public String sendAsync(String target, String content, Runnable onAccepted) {
String ticket = "task-" + ticketSeq.incrementAndGet();
Task task = new Task(ticket, target, nowNanos.getAsLong());
Task task = new Task(ticket, target, nowNanos);
tasks.put(ticket, task);
if (pushLoop != null) {
// CB-588: task.future only ever completes on a terminal phase (DONE or a failure) — a
@@ -821,12 +1021,25 @@ public final class MessageService {
* (or one the reminder cap already gave up on) is pruned here but never collected there, so it
* lingers in {@code pendingTickets} forever and rides along on every later nudge to the same lead
* — naming a ticket {@code fleet_poll} can no longer find (CB-588 follow-up).
*
* <p>The TTL runs from **completion**, not from creation (#197). It used to compare against
* {@code createdNanos}, which made the real collection window {@code TTL minus however long the
* task ran}: a delegation that took longer than the TTL had its reply destroyed on the first
* sweep after it landed, every time. That is the normal case here — real work runs well past ten
* minutes — and the reply lives only in {@code future}, so pruning it discards the worker's whole
* report with nothing to fall back on. Measuring from completion gives every ticket the same full
* window whatever its runtime, and still bounds {@code tasks}.
*
* <p>A task whose future is done but whose {@code completedNanos} is not stamped yet is left
* alone. That window is the few instructions between {@code complete()} and the constructor's
* {@code whenComplete} hook running; the next sweep collects it.
*/
private void pruneTerminalTickets() {
long cutoff = nowNanos.getAsLong() - TICKET_TTL_NANOS;
tasks.entrySet().removeIf(e -> {
Task t = e.getValue();
boolean expired = t.future.isDone() && t.createdNanos < cutoff;
Long completed = t.completedNanos;
boolean expired = t.future.isDone() && completed != null && completed < cutoff;
if (expired && pushLoop != null) {
pushLoop.ticketCollected(e.getKey());
}
@@ -214,6 +214,17 @@ public final class FleetApp {
* second daemon configured, a member daemon that is down must not be masked by a healthy lead
* daemon — every spawn goes through the member daemon and would otherwise fail silently behind
* a green {@code /healthz}.
*
* <p>CB-185 blocker 2: the {@code herdr} key always carries the <em>lead</em> daemon's
* version/protocol, unchanged, because two consumers — {@code scripts/redeploy-fleetd.sh} and
* {@code scripts/rename-checkout.sh} — read this endpoint already (both only check the HTTP
* status code and print the body verbatim; neither parses a specific field, so adding a key
* alongside {@code herdr} is safe). But it is the <em>member</em> daemon's protocol that decides
* whether a spawn works, so when a second daemon is configured its version/protocol is reported
* too, under a separate {@code member} key — never folded into {@code herdr}, which would make a
* mismatch invisible to whichever consumer only reads that key. If the two protocol numbers
* differ, {@code protocolMismatch: true} calls it out explicitly rather than leaving it to be
* spotted by comparing two numbers by eye.
*/
private void healthz(Context ctx) {
JsonNode pong;
@@ -226,9 +237,15 @@ public final class FleetApp {
"detail", e.getMessage()));
return;
}
Map<String, Object> body = new LinkedHashMap<>();
body.put("status", "ok");
body.put("herdr", Map.of(
"version", pong.path("version").asText(""),
"protocol", pong.path("protocol").asInt()));
if (memberHerdr != herdr) {
JsonNode memberPong;
try {
memberHerdr.call("ping");
memberPong = memberHerdr.call("ping");
} catch (HerdrException e) {
ctx.status(503).json(Map.of(
"status", "degraded",
@@ -236,12 +253,16 @@ public final class FleetApp {
"detail", e.getMessage()));
return;
}
int leadProtocol = pong.path("protocol").asInt();
int memberProtocol = memberPong.path("protocol").asInt();
body.put("member", Map.of(
"version", memberPong.path("version").asText(""),
"protocol", memberProtocol));
if (leadProtocol != memberProtocol) {
body.put("protocolMismatch", true);
}
}
ctx.status(200).json(Map.of(
"status", "ok",
"herdr", Map.of(
"version", pong.path("version").asText(""),
"protocol", pong.path("protocol").asInt())));
ctx.status(200).json(body);
}
/**
@@ -294,10 +315,18 @@ public final class FleetApp {
.map(Agent.class::cast)
.filter(a -> a.terminalId() != null)
.collect(Collectors.toMap(Agent::terminalId, Function.identity(), (_, b) -> b));
List<Map<String, Object>> out = sessions.roster().stream()
// fleetd #209: this REST roster reports agentSessionId via SessionManager.rosterView, so it
// uses the resolving roster read (caller-driven, not a timer) rather than the plain one.
List<Map<String, Object>> out = sessions.rosterResolved().stream()
.map(s -> SessionManager.rosterView(s, live.get(s.terminalId())))
.toList();
Map<String, Object> body = new LinkedHashMap<>();
// fleetd #199: 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. "members" is the canonical key; "workers" stays 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. Drop the alias once nothing reads it.
body.put("members", out);
body.put("workers", out);
// CB-586: operator visibility for the refs/wip snapshot store without shelling into the
// repo — how many snapshot refs exist and roughly what they cost. Present only once a
@@ -23,6 +23,7 @@ import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.stream.Collectors;
/**
@@ -88,28 +89,69 @@ public final class GitWorktrees implements Worktrees {
);
private final String configuredRoot;
/** OS group name for {@link #shareWithGroup} (fleetd #185 stage 3); {@code null} ⇒ feature off. */
private final String group;
private final Consumer<String> afterWorktreeAdded;
/** How {@link #shareWithGroup}'s processes (git config / chgrp / chmod / find) actually run.
* Defaults to the real {@link #exec(String...)}. Package-private test seam so a unit test can
* prove "no group configured ⇒ zero processes spawned" and inspect exactly what a configured
* group runs, without a real second OS user or OS group on this host. */
private final Function<String[], String> shareGroupRunner;
private final SecureRandom random = new SecureRandom();
private final AtomicLong seq = new AtomicLong();
/** Default constructor: worktree root is derived per-repo as {@code <repoRoot>/../.bridged-worktrees}. */
public GitWorktrees() {
this(null);
this(null, (String) null);
}
/** @param configuredRoot nullable absolute or relative path; null/blank derives a sibling of the repo root. */
/**
* @param configuredRoot nullable absolute or relative path; null/blank derives a sibling of
* the repo root. No {@code worktreeGroup} configured — {@link #shareWithGroup}
* is a no-op.
*/
public GitWorktrees(String configuredRoot) {
this(configuredRoot, _ -> {});
this(configuredRoot, (String) null);
}
/**
* @param configuredRoot nullable absolute or relative path; null/blank derives a sibling of
* the repo root.
* @param group optional OS group name (fleetd #185 stage 3, {@code worktreeGroup:} in
* config); null/blank ⇒ {@link #shareWithGroup} is a no-op.
*/
public GitWorktrees(String configuredRoot, String group) {
this(configuredRoot, group, _ -> {});
}
/** Test seam for changing a real worktree between its creation and its security check. */
GitWorktrees(String configuredRoot, Consumer<String> afterWorktreeAdded) {
this(configuredRoot, null, afterWorktreeAdded);
}
/** Test seam combining a configurable {@code group} with {@link #afterWorktreeAdded}. */
GitWorktrees(String configuredRoot, String group, Consumer<String> afterWorktreeAdded) {
this(configuredRoot, group, afterWorktreeAdded, null);
}
/**
* Full test seam: also overrides how {@link #shareWithGroup}'s processes run (fleetd #185
* stage 3), so a unit test can prove "no group configured ⇒ no process spawned" and inspect
* exactly what commands a configured group runs, without a real second OS user/group.
*
* @param shareGroupRunner {@code null} ⇒ the real {@link #exec(String...)}.
*/
GitWorktrees(String configuredRoot, String group, Consumer<String> afterWorktreeAdded,
Function<String[], String> shareGroupRunner) {
this.configuredRoot = configuredRoot;
this.group = (group == null || group.isBlank()) ? null : group;
this.afterWorktreeAdded = afterWorktreeAdded == null ? _ -> {} : afterWorktreeAdded;
this.shareGroupRunner = shareGroupRunner != null ? shareGroupRunner : this::exec;
}
@Override
public String add(String repoRoot, String branch, String baseRef) {
reportRemoteUrlsWithUserInfo(repoRoot);
String base = (baseRef == null || baseRef.isBlank()) ? "HEAD" : baseRef;
String nonce = nonce();
Path root = resolveRoot(repoRoot);
@@ -139,7 +181,7 @@ public final class GitWorktrees implements Worktrees {
if (exitCode("git", "-C", repoRoot, "config", "--get", "remote.origin.url") != 0) {
return;
}
String origin = exec("git", "-C", repoRoot, "config", "--get", "remote.origin.url").trim();
String origin = execRedacted("git", "-C", repoRoot, "config", "--get", "remote.origin.url").trim();
URI uri;
try {
uri = new URI(origin);
@@ -155,6 +197,9 @@ public final class GitWorktrees implements Worktrees {
throw new WorktreeException("origin URL has invalid HTTPS user info; cannot provision safely");
}
String cleanOrigin = origin.substring(0, schemeEnd) + origin.substring(userInfoEnd + 1);
// Plain exec, not execRedacted, is correct here: this call WRITES cleanOrigin (already
// stripped of user-info above) rather than reading a URL back from stdout, so there is
// nothing secret left in either its argv or its stdout to redact.
exec("git", "-C", repoRoot, "remote", "set-url", "origin", cleanOrigin);
log.info("removed HTTPS user info from forge origin before provisioning worktree");
}
@@ -164,7 +209,7 @@ public final class GitWorktrees implements Worktrees {
if (exitCode("git", "-C", worktreePath, "remote", "get-url", "--all", "origin") != 0) {
return;
}
String origins = exec("git", "-C", worktreePath, "remote", "get-url", "--all", "origin");
String origins = execRedacted("git", "-C", worktreePath, "remote", "get-url", "--all", "origin");
for (String origin : origins.split("\\R")) {
try {
URI uri = new URI(origin);
@@ -177,6 +222,99 @@ public final class GitWorktrees implements Worktrees {
}
}
/**
* Report — never refuse — every remote whose fetch or push URL carries user-info outside SSH.
* A linked worktree shares its parent repository's git config, so a credential on ANY remote
* (not only {@code origin}) or in a {@code pushurl} is just as readable by a member as one on
* {@code origin}'s HTTPS fetch URL — the one case {@link #removeUserInfoFromHttpsOrigin} and
* {@link #requireCredentialFreeHttpsOrigin} already strip and refuse. This check is additive: it
* only logs a warning, it never mutates config and never refuses the provision.
*
* <p>A reporting-only check must never be able to abort a provision — PR #173 shipped one that
* ran unguarded at the top of {@link #add}, and every git call inside it can throw ({@link #exec}
* turns a non-zero exit or its 30-second timeout into a {@link WorktreeException}). Every git call
* here is therefore wrapped, and on failure only the exception's <em>class</em> is logged, never
* its message: the enumerating {@code git remote} call is not redacted, and its stderr is read
* from the very config that may hold the URL this check exists to find.
*/
private void reportRemoteUrlsWithUserInfo(String repoRoot) {
List<String> remotes;
try {
remotes = exec("git", "-C", repoRoot, "remote").lines()
.map(String::trim)
.filter(r -> !r.isBlank())
.toList();
} catch (RuntimeException e) {
log.warn("could not enumerate remotes to check for credentialed URLs in {}: {}",
Path.of(repoRoot).toAbsolutePath().normalize(), e.getClass().getName());
return;
}
for (String remote : remotes) {
reportOneRemoteUrlsWithUserInfo(repoRoot, remote);
}
}
private void reportOneRemoteUrlsWithUserInfo(String repoRoot, String remote) {
boolean leaks = remoteUrlsLeakUserInfo(repoRoot, remote, false)
|| remoteUrlsLeakUserInfo(repoRoot, remote, true);
if (leaks) {
log.warn("member worktree shares a remote URL containing user-info: remote={} repository={}; "
+ "remove credentials from the repository's git config",
remote, Path.of(repoRoot).toAbsolutePath().normalize());
}
}
/**
* True when any resolved fetch (or, if {@code push}, push) URL for {@code remote} carries
* non-empty user-info outside SSH. Never throws — a git failure here is caught, logged (its
* class only, per the javadoc above), and treated as "nothing found", so it cannot abort or
* otherwise affect provisioning. Uses {@link #execRedacted} because the command's stdout is
* itself the URL this check exists to find.
*/
private boolean remoteUrlsLeakUserInfo(String repoRoot, String remote, boolean push) {
try {
String out = push
? execRedacted("git", "-C", repoRoot, "remote", "get-url", "--push", "--all", remote)
: execRedacted("git", "-C", repoRoot, "remote", "get-url", "--all", remote);
return out.lines().anyMatch(url -> !url.isBlank() && urlLeaksUserInfo(url.trim()));
} catch (RuntimeException e) {
log.warn("could not read the {} URL for remote {} to check for credentials: {}",
push ? "push" : "fetch", remote, e.getClass().getName());
return false;
}
}
/**
* True when {@code rawUrl} parses as an absolute URI with a non-SSH-family scheme and non-empty
* user-info. An unparsable or scheme-less URL — including the ssh scp-like shorthand
* ({@code user@host:path}) — is not this check's concern and is treated as "no finding", the
* same way {@link #configureHttpsUrlRewriteForSshOrigin} leaves that shorthand untouched.
*/
private static boolean urlLeaksUserInfo(String rawUrl) {
URI uri;
try {
uri = new URI(rawUrl);
} catch (URISyntaxException e) {
return false;
}
String scheme = uri.getScheme();
if (scheme == null || isSshLikeScheme(scheme)) {
return false;
}
String userInfo = uri.getUserInfo();
return userInfo != null && !userInfo.isEmpty();
}
/**
* SSH-family schemes deliberately excluded from {@link #urlLeaksUserInfo}: there, the user part
* selects an account and authentication itself happens over SSH, so it is not a credential the
* way HTTPS/HTTP user-info is.
*/
private static boolean isSshLikeScheme(String scheme) {
return "ssh".equalsIgnoreCase(scheme) || "git+ssh".equalsIgnoreCase(scheme)
|| "ssh+git".equalsIgnoreCase(scheme);
}
/** Configure a per-worktree helper that supplies a token from the member environment at call time. */
private void configureEnvironmentCredentialHelper(String repoRoot, String worktreePath) {
exec("git", "-C", repoRoot, "config", "extensions.worktreeConfig", "true");
@@ -234,7 +372,7 @@ public final class GitWorktrees implements Worktrees {
if (exitCode("git", "-C", repoRoot, "config", "--get", "remote.origin.url") != 0) {
return;
}
String origin = exec("git", "-C", repoRoot, "config", "--get", "remote.origin.url").trim();
String origin = execRedacted("git", "-C", repoRoot, "config", "--get", "remote.origin.url").trim();
URI uri;
try {
uri = new URI(origin);
@@ -510,6 +648,126 @@ public final class GitWorktrees implements Worktrees {
return deleted;
}
/**
* {@inheritDoc}
*
* <p>fleetd #185 stage 3. No-op — no process spawned, nothing logged — when {@link #group} is
* null/blank. Otherwise:
* <ol>
* <li>{@code git -C repoRoot config core.sharedRepository group} so every future write by
* either uid stays group-writable;</li>
* <li>a one-time {@code chgrp}/{@code chmod g+rwX} fix-up over the worktree directory and,
* under the repo's <em>common</em> git directory, {@code objects}, {@code refs},
* {@code logs}, {@code worktrees} and {@code packed-refs} — with setgid
* ({@code chmod g+s}) applied only to the directories among them, so files created later
* inherit the group;</li>
* <li>one INFO line naming the group and the paths touched.</li>
* </ol>
*
* <p><b>Every path is skipped when it does not exist.</b> {@code .git/logs} is absent in a repo
* with {@code core.logAllRefUpdates=false} or one that has had no ref update yet, and
* {@code packed-refs} is absent until refs are packed. Passing a missing path to {@code chgrp}
* exits non-zero, which would fail <em>every</em> provisioning spawn with a message blaming a
* group that is in fact fine.
*
* <p><b>The git directory is resolved, not assumed.</b> {@code <repoRoot>/.git} is a
* <em>file</em>, not a directory, when the checkout is itself a linked worktree — the very
* thing this class creates for every member. {@code git rev-parse --git-common-dir} gives the
* real shared store, and it may answer relatively, so it is resolved against {@code repoRoot}.
*
* <p><b>The fix-up re-runs on every spawn, by design.</b> {@code core.sharedRepository=group}
* governs only what git writes <em>after</em> it is set; the walk is what covers everything
* already on disk. It is not redundant work to optimise away — dropping it silently leaves
* pre-existing objects unreadable to the member. It costs three walks of the object store per
* spawn (about 3000 files in this repo, well under a second, but it grows with the repo).
*
* <p>This only fixes up file ownership/permissions on the operator's shared repo so a
* different-uid member can write to it — it isolates credentials, not the repository. A member
* in the group can still write the operator's git objects and refs.
*
* <p>Fails loudly: a missing group, or a {@code chgrp}/{@code chmod} refused because the
* operator is not a member of it, becomes a {@link WorktreeException} naming the group — never
* a silent skip that leaves a member unable to work with nothing in the log to explain why.
*/
@Override
public void shareWithGroup(String repoRoot, String worktreePath) {
if (group == null) {
return;
}
List<String> touched = new ArrayList<>();
try {
shareGroupRunner.apply(new String[]{"git", "-C", repoRoot, "config", "core.sharedRepository", "group"});
String commonDir = gitCommonDir(repoRoot);
shareGroupPathIfPresent(worktreePath, true, touched);
for (String name : List.of("objects", "refs", "logs", "worktrees")) {
shareGroupPathIfPresent(commonDir + "/" + name, true, touched);
}
shareGroupPathIfPresent(commonDir + "/packed-refs", false, touched);
} catch (WorktreeException e) {
throw new WorktreeException("cannot share worktree with group '" + group + "': "
+ e.getMessage() + " — the group must exist, and the fleetd operator ("
+ System.getProperty("user.name") + ") must be a member of it", e);
}
log.info("worktreeGroup={} shared repoRoot={} worktreePath={} paths={}",
group, repoRoot, worktreePath, touched);
}
/**
* The repo's <em>common</em> git directory as an absolute path — where {@code objects},
* {@code refs} and {@code worktrees} actually live. {@code git rev-parse --git-common-dir}
* answers relative to {@code repoRoot} in the ordinary case ({@code .git}) and absolutely for a
* linked worktree, so the answer is resolved against {@code repoRoot} either way. Never
* hardcode {@code repoRoot + "/.git"}: that is a FILE when the checkout is itself a linked
* worktree.
*/
private String gitCommonDir(String repoRoot) {
String answer = shareGroupRunner.apply(
new String[]{"git", "-C", repoRoot, "rev-parse", "--git-common-dir"});
String trimmed = answer == null ? "" : answer.trim();
if (trimmed.isEmpty()) {
trimmed = ".git";
}
return Path.of(repoRoot).resolve(trimmed).normalize().toString();
}
/**
* {@link #shareGroupPath} when {@code path} exists, recording it in {@code touched}; otherwise
* nothing at all. A missing path is normal, not an error — see {@link #shareWithGroup}'s
* javadoc for which ones are routinely absent and why passing them to {@code chgrp} would fail
* every spawn.
*/
private void shareGroupPathIfPresent(String path, boolean recursive, List<String> touched) {
if (!Files.exists(Path.of(path))) {
return;
}
shareGroupPath(path, recursive);
touched.add(path);
}
/**
* {@code chgrp}/{@code chmod g+rwX} {@code path} to {@link #group}. When {@code recursive},
* also walks the directories under {@code path} (including {@code path} itself, when it is a
* directory) and sets setgid on each — directories only, per the javadoc on
* {@link #shareWithGroup}.
*/
private void shareGroupPath(String path, boolean recursive) {
List<String> chgrp = new ArrayList<>(List.of("chgrp"));
if (recursive) chgrp.add("-R");
chgrp.add(group);
chgrp.add(path);
shareGroupRunner.apply(chgrp.toArray(new String[0]));
List<String> chmod = new ArrayList<>(List.of("chmod"));
if (recursive) chmod.add("-R");
chmod.add("g+rwX");
chmod.add(path);
shareGroupRunner.apply(chmod.toArray(new String[0]));
if (recursive) {
shareGroupRunner.apply(new String[]{"find", path, "-type", "d", "-exec", "chmod", "g+s", "{}", "+"});
}
}
/**
* Every {@code refs/wip/*} ref (see {@link WipRef}). The committer date is read as a unix
* count of seconds and converted to millis. {@code %00} (NUL) separates the fields because a
@@ -603,6 +861,31 @@ public final class GitWorktrees implements Worktrees {
/** Same as {@link #exec(String...)}, with extra environment variables set on the child process. */
private String exec(Map<String, String> extraEnv, String... command) {
return exec(extraEnv, false, command);
}
/**
* Same as {@link #exec(String...)}, for a command whose stdout may itself carry a credential
* (e.g. {@code git remote get-url}, whose output is a URL). Stdout is still returned normally on
* success — callers still get the URL to inspect — but it is suppressed from BOTH the timeout
* message and the non-zero-exit message, so a failing call here can never copy it into a
* {@link WorktreeException}, and from there into a caller's log.
*/
private String execRedacted(String... command) {
return exec(Map.of(), true, command);
}
/**
* Shared implementation for {@link #exec(Map, String...)} and {@link #execRedacted(String...)}.
* {@code redactOutput} suppresses captured stdout from both failure messages below.
*
* <p>Package-private, not {@code private}: also a test seam, the same way the
* {@code afterWorktreeAdded} constructor parameter is. It lets a test drive the redaction
* guarantee directly — a synthetic failing command whose stdout carries a test marker passed
* through {@code extraEnv} rather than argv — without depending on finding a real git failure
* mode that happens to echo a URL onto stdout before exiting non-zero.
*/
String exec(Map<String, String> extraEnv, boolean redactOutput, String... command) {
String out;
int code;
Process p;
@@ -624,7 +907,8 @@ public final class GitWorktrees implements Worktrees {
try {
if (!p.waitFor(30, TimeUnit.SECONDS)) {
p.destroyForcibly();
throw new WorktreeException("command timed out: " + String.join(" ", command) + "\n" + out);
throw new WorktreeException("command timed out: " + String.join(" ", command)
+ (redactOutput || out.isBlank() ? "" : "\n" + out));
}
code = p.exitValue();
} catch (InterruptedException e) {
@@ -634,7 +918,7 @@ public final class GitWorktrees implements Worktrees {
}
if (code != 0) {
throw new WorktreeException("exit " + code + " for: " + String.join(" ", command)
+ (out.isBlank() ? "" : "\n" + out));
+ (redactOutput || out.isBlank() ? "" : "\n" + out));
}
return out;
}
@@ -89,4 +89,15 @@ public record MemberSession(
return new MemberSession(paneId, terminalId, profile, role, cwd, ownerTerminal, spawnedAtNanos,
nowNanos, turnCount + 1, state, worktree, branch, charterReceipt, agentSessionId);
}
/**
* Return a copy with {@code agentSessionId} resolved to a non-null value (fleetd #209). Some
* adapters (opencode) cannot answer {@link dev.ltms.fleet.peer.PeerHandle#agentSessionId()} at
* spawn time — the peer has not persisted its session record yet — so the id is discovered on
* a later poll and swapped into the otherwise-immutable session via this wither.
*/
public MemberSession withAgentSessionId(String agentSessionId) {
return new MemberSession(paneId, terminalId, profile, role, cwd, ownerTerminal, spawnedAtNanos,
lastActivityAtNanos, turnCount, state, worktree, branch, charterReceipt, agentSessionId);
}
}
@@ -46,6 +46,15 @@ public final class SessionManager implements TurnListener {
private final PeerLauncher launcher;
private final Worktrees worktrees;
private final ConcurrentHashMap<String /*paneId*/, MemberSession> registry = new ConcurrentHashMap<>();
/**
* fleetd #209: the live {@link PeerHandle} for every registered pane, retained solely so
* {@link #resolveAgentSessionId} can re-poll {@link PeerHandle#agentSessionId()} after spawn.
* The handle used to go out of scope at the end of the spawn method, so a launcher that answers
* the id lazily (opencode — the on-disk session row is written after the pane is created) could
* never be re-asked, and {@code fleet_list}/{@code fleet_spawn resumeSessionId} never saw it.
* Populated on every spawn path, removed on {@link #release}.
*/
private final ConcurrentHashMap<String /*paneId*/, PeerHandle> handles = new ConcurrentHashMap<>();
private final MemberPresence presence;
private final SecureRandom nonceRandom = new SecureRandom();
private final AtomicLong nonceSeq = new AtomicLong();
@@ -205,6 +214,7 @@ public final class SessionManager implements TurnListener {
handle.charterReceipt(),
handle.agentSessionId());
registry.put(handle.id(), session);
handles.put(handle.id(), handle);
memberLifecycle.acquired(session.role(), session.profile(), session.terminalId());
log.debug("acquired session id={} terminal={} profile={} owner={}",
handle.id(), handle.terminalId(), session.profile(), session.ownerTerminal());
@@ -257,6 +267,10 @@ public final class SessionManager implements TurnListener {
*/
private void release(String paneId, ReleaseCause cause) {
MemberSession removed = registry.remove(paneId);
// fleetd #209: remove right alongside the registry entry so a released session's handle is
// never leaked — but keep the local reference below, so the id can still be resolved for
// the ReleaseDetail this teardown notifies with.
PeerHandle removedHandle = handles.remove(paneId);
boolean preserveWorktree = cause == ReleaseCause.SHUTDOWN;
String snapshotRef = null;
if (removed != null) {
@@ -303,8 +317,11 @@ public final class SessionManager implements TurnListener {
// too, so a failed ticket's detail can point a lead at the same tree to re-dispatch.
// CB-584 (issue #65 criterion 5): carry agentSessionId alongside them, so a lead can
// also resume the member's conversation, not just re-dispatch onto its files.
notifyReleased(new ReleaseDetail(removed.terminalId(), removed.worktree(),
removed.branch(), snapshotRef, removed.agentSessionId()));
// fleetd #209: a late-resolving adapter (opencode) may only now have an id — resolve
// one last time so a released member's detail carries the id it now has.
MemberSession resolved = resolveAgentSessionId(removed, removedHandle);
notifyReleased(new ReleaseDetail(resolved.terminalId(), resolved.worktree(),
resolved.branch(), snapshotRef, resolved.agentSessionId()));
}
}
// CB-581: the pane must always stop, even if the dirty check above threw. A session removed
@@ -472,6 +489,10 @@ public final class SessionManager implements TurnListener {
try {
path = worktrees.add(repoRoot, branch, wt.baseRef());
worktrees.overlayParity(repoRoot, path, launcher.parityOverlay(preResolvedProfile));
// fleetd #185 stage 3: MUST run after overlayParity, not folded into add() — overlayParity
// copies more files into the worktree after add() returns, so sharing the group any earlier
// leaves those overlay files operator-owned and read-only for a different-uid member.
worktrees.shareWithGroup(repoRoot, path);
handle = launcher.spawn(new SpawnRequest(profile, path, callerCwd, sessionName, resumeSessionId, memberRole));
} catch (RuntimeException e) {
log.warn("spawn failed for profile={} role={} branch={} path={}: {}",
@@ -504,6 +525,7 @@ public final class SessionManager implements TurnListener {
handle.charterReceipt(),
handle.agentSessionId());
registry.put(handle.id(), session);
handles.put(handle.id(), handle);
memberLifecycle.acquired(session.role(), session.profile(), session.terminalId());
log.debug("acquired worktree session id={} terminal={} profile={} branch={} path={}",
handle.id(), handle.terminalId(), session.profile(), session.branch(), session.worktree());
@@ -535,16 +557,73 @@ public final class SessionManager implements TurnListener {
return launcher.defaultProfile();
}
/** The session for {@code paneId}, if it is still registered and not released. */
/**
* The session for {@code paneId}, if it is still registered and not released. fleetd #209:
* resolves a still-unknown {@code agentSessionId} against the retained handle before returning,
* so {@code fleet_status} sees an id a lazy-resolving adapter has since written.
*/
public Optional<MemberSession> get(String paneId) {
return Optional.ofNullable(registry.get(paneId));
return Optional.ofNullable(registry.get(paneId)).map(this::resolveAgentSessionId);
}
/** Fleet-owned roster: all registered sessions (acquired minus released). */
/**
* Fleet-owned roster: all registered sessions (acquired minus released). Deliberately does
* <strong>not</strong> resolve {@code agentSessionId} (fleetd #209 follow-up) — this is the
* roster supplier on the heartbeat and health-tick timers ({@code LeadHeartbeatLoop},
* {@code FleetHealthMonitor} in {@code Fleetd}), on the placement/exhaustion paths, and on the
* metrics scrape ({@code FleetMetrics}), all called far more often than any caller actually
* reads {@code agentSessionId}. Resolving here would mean every tick opens a lazy-resolving
* adapter's (opencode's) on-disk session store once per member whose id is still unknown — and
* for a member whose id never appears, that cost never stops, for the life of the process. Use
* {@link #rosterResolved()} instead wherever the id must be current.
*/
public List<MemberSession> roster() {
return List.copyOf(registry.values());
}
/**
* {@link #roster()}, with each session's still-unknown {@code agentSessionId} re-resolved
* against its retained handle (fleetd #209) — so a caller that actually reports the id (
* {@code fleet_list}, the REST roster) sees one a lazy-resolving adapter (opencode) has since
* written, rather than the null frozen in at spawn time. Reserved for caller-driven reads, not
* timers: see {@link #roster()}'s javadoc for why the plain roster must stay non-resolving.
*/
public List<MemberSession> rosterResolved() {
return registry.values().stream().map(this::resolveAgentSessionId).toList();
}
/**
* Resolve {@code session}'s {@code agentSessionId} if still unknown, re-polling the retained
* {@link PeerHandle} for this pane (fleetd #209). A no-op — returning {@code session} unchanged
* — once the id is already known, once no handle is retained for this pane (never spawned, or
* already released), or if the handle throws while answering. A resolved id is best-effort
* CAS-swapped into the registry via {@link #replace}; a lost race just means another caller
* already applied the same update, so the resolved value is returned either way.
*/
private MemberSession resolveAgentSessionId(MemberSession session) {
return resolveAgentSessionId(session, handles.get(session.paneId()));
}
private MemberSession resolveAgentSessionId(MemberSession session, PeerHandle handle) {
if (session.agentSessionId() != null || handle == null) {
return session;
}
String resolved;
try {
resolved = handle.agentSessionId();
} catch (RuntimeException e) {
log.debug("agentSessionId lookup failed for pane={} terminal={}: {}",
session.paneId(), session.terminalId(), e.toString());
return session;
}
if (resolved == null) {
return session;
}
MemberSession updated = session.withAgentSessionId(resolved);
replace(session, updated); // best-effort; a lost CAS just means the resolved value stands anyway
return updated;
}
/**
* CB-304 merged roster+live view. The registry is authoritative for worktree, branch,
* profile, owner, and state; the optional live agent supplies the herdr-reported status.
@@ -98,4 +98,21 @@ public interface Worktrees {
/** CB-586: the operator-visible census of {@code refs/wip/*} in one repository. */
record WipRefStats(int count, long costBytes) {
}
/**
* Make {@code repoRoot}'s git store and {@code worktreePath} writable by the configured group
* (fleetd #185 stage 3), so a member spawned as a different OS user (see
* {@code memberHerdrSocket}) can write its own worktree, its per-worktree git metadata, and
* its own commit objects. No-op when no group is configured.
*
* <p><strong>This isolates credentials, not the repository.</strong> A member in the group can
* still write the operator's git objects and refs in the shared repo — this only fixes file
* ownership/permissions so a different-uid member can work at all, it grants no narrower access
* than that.
*
* @param repoRoot the repository whose git store ({@code .git/objects}, {@code refs},
* {@code logs}, {@code worktrees}, {@code packed-refs}) needs sharing
* @param worktreePath the linked worktree's own directory
*/
void shareWithGroup(String repoRoot, String worktreePath);
}
@@ -1036,6 +1036,35 @@ class FleetConfigTest {
assertEquals(LeadMailbox.DEFAULT_PREFETCH, noEnv.prefetchOrDefault());
}
@Test
void absentWorktreeGroupLeavesItNull(@TempDir Path dir) throws Exception {
Path f = dir.resolve("no-worktree-group.yaml");
Files.writeString(f, "bind:\n port: 8080\n");
FleetConfig cfg = FleetConfig.load(f);
assertNull(cfg.worktreeGroup(), "no worktreeGroup: key → null → GitWorktrees.shareWithGroup is a no-op");
}
@Test
void worktreeGroupKeyParses(@TempDir Path dir) throws Exception {
Path f = dir.resolve("worktree-group.yaml");
Files.writeString(f, "bind:\n port: 8080\nworktreeGroup: fleet-workers\n");
FleetConfig cfg = FleetConfig.load(f);
assertEquals("fleet-workers", cfg.worktreeGroup());
}
@Test
void absentWorktreeGroupSurvivesTheBackCompatConstructorChain() {
// fleetd #185 stage 3: withDefaults() (and every pre-existing call site) must not silently
// drop a live worktreeGroup by routing through a back-compat constructor that defaults it
// to null.
FleetConfig cfg = new FleetConfig(null, null, null, Map.of(), null, null, null, null, null,
null, null, null, null, null, null, null, null, null, null, null, "fleet-workers");
assertEquals("fleet-workers", cfg.withDefaults().worktreeGroup(),
"withDefaults() must carry a configured worktreeGroup through unchanged");
}
@Test
void absentPrimaryBlockLeavesPrimaryNull(@TempDir Path dir) throws Exception {
Path f = dir.resolve("no-primary.yaml");
@@ -31,6 +31,8 @@ public final class FakeHerdr implements HerdrClient {
*/
public final List<Call> calls = new CopyOnWriteArrayList<>();
private boolean healthy = true;
private String pingVersion = "0.8.0";
private int pingProtocol = 19;
private final List<String> extraWorkspaces = new ArrayList<>();
private final List<String> extraAgents = new ArrayList<>();
/** workspaceId → extra tabs that {@code tab.list} reports for it (CB-558 lead scans). */
@@ -52,6 +54,16 @@ public final class FakeHerdr implements HerdrClient {
return this;
}
/**
* Make {@code ping} report this version/protocol instead of the default 0.8.0/19 — CB-185
* blocker 2's fixture for a lead and a member daemon running mismatched herdr versions.
*/
public FakeHerdr pingReports(String version, int protocol) {
this.pingVersion = version;
this.pingProtocol = protocol;
return this;
}
/** Reject the first {@code n} {@code agent.start} calls with {@code agent_name_taken}. */
public FakeHerdr agentNameTakenTimes(int n) {
this.agentNameTakenFor = n;
@@ -166,7 +178,8 @@ public final class FakeHerdr implements HerdrClient {
try {
return switch (method) {
case "ping" -> mapper.readTree(
"{\"type\":\"pong\",\"version\":\"0.8.0\",\"protocol\":19}");
("{\"type\":\"pong\",\"version\":\"%s\",\"protocol\":%d}")
.formatted(pingVersion, pingProtocol));
case "workspace.list" -> mapper.readTree(("""
{"type":"workspace_list","workspaces":[
{"workspace_id":"w1","label":"dev-mgnl","focused":true,"pane_count":7,"agent_status":"unknown"},
@@ -0,0 +1,27 @@
package dev.ltms.fleet.herdr;
import java.util.HashMap;
import java.util.Map;
import java.util.OptionalLong;
/**
* Fake {@link ParentResolver} backed by an explicit pid→parent map — lets {@link PaneLocatorTest}
* drive {@link PaneLocator}'s ancestry walk (grandchild pids, cycles) without spawning real OS
* processes.
*/
final class FakeParentResolver implements ParentResolver {
private final Map<Long, Long> parents = new HashMap<>();
/** {@code pid}'s parent is {@code parentPid}. A pid with no entry here has no known parent. */
FakeParentResolver parent(long pid, long parentPid) {
parents.put(pid, parentPid);
return this;
}
@Override
public OptionalLong parentOf(long pid) {
Long parent = parents.get(pid);
return parent == null ? OptionalLong.empty() : OptionalLong.of(parent);
}
}
@@ -1,7 +1,11 @@
package dev.ltms.fleet.herdr;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;
import java.util.concurrent.atomic.AtomicInteger;
import static org.junit.jupiter.api.Assertions.*;
/** Unit tests for PID → pane resolution (the herdr half of connection-based MCP identity). */
@@ -63,4 +67,123 @@ class PaneLocatorTest {
long paneListCalls = shared.calls.stream().filter(c -> c.method().equals("pane.list")).count();
assertEquals(1, paneListCalls, "same-object lead/member must scan exactly once, not twice");
}
// --- ancestry walk (CB-161: grandchild pids matched no pane, resolving as primary) --------
@Test
void stillResolvesAPidThatIsExactlyThePaneShellPid() {
// Regression: a pid with no parent chain at all — no ancestry walk is needed to match it.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
PaneLocator loc = new PaneLocator(pane, new FakeParentResolver());
assertEquals("term_x", loc.terminalForPid(5000));
}
@Test
void stillResolvesAPidThatIsExactlyAForegroundPid() {
// Regression: same as above, but matching via the foreground-processes list.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
PaneLocator loc = new PaneLocator(pane, new FakeParentResolver());
assertEquals("term_x", loc.terminalForPid(6000));
}
@Test
void resolvesAGrandchildPidTwoLevelsBelowTheShellPid() {
// The bug: a helper process a worker spawns (python3, curl, ...) is a grandchild of the
// pane's shell — not the shell_pid and not a foreground pid directly. Before the fix,
// paneOwnsPid only checked direct pid equality, so this pid matched no pane and the
// caller fell through to loopback-trust as the primary.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
FakeParentResolver parents = new FakeParentResolver()
.parent(7002, 7001) // grandchild -> child
.parent(7001, 5000); // child -> shell (the pane's shell_pid)
PaneLocator loc = new PaneLocator(pane, parents);
assertEquals("term_x", loc.terminalForPid(7002));
}
@Test
void nullForAPidWhoseAncestryMatchesNoPane() {
// Must not break the other direction: a pid that truly belongs to nothing here (e.g. the
// real primary) must still resolve to null. Resolving everything to a worker would demote
// the actual lead and refuse every orchestration call.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
FakeParentResolver parents = new FakeParentResolver()
.parent(9002, 9001)
.parent(9001, 9000); // chain never reaches 5000 or 6000
PaneLocator loc = new PaneLocator(pane, parents);
assertNull(loc.terminalForPid(9002));
}
@Test
void ancestryWalkTerminatesOnACycleInsteadOfHanging() {
// A fake (or corrupted) parent map that cycles must not hang identity resolution, which
// runs on every MCP call. The walk must still terminate and correctly resolve to null.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
FakeParentResolver parents = new FakeParentResolver()
.parent(100, 101)
.parent(101, 100); // cycle, never reaches the pane's pids
PaneLocator loc = new PaneLocator(pane, parents);
assertNull(loc.terminalForPid(100));
}
@Test
void ancestrySetIsComputedOnceAcrossBothClientsInTheTwoDaemonConstructor() {
// CB-185: the two-daemon constructor searches lead then member. The ancestor set is
// per-caller, not per-client — it must be walked once and reused, not recomputed for
// each client searched.
OnePaneHerdr pane = new OnePaneHerdr("term_x", "pX", 5000, 6000);
FakeParentResolver parents = new FakeParentResolver()
.parent(7002, 7001)
.parent(7001, 5000);
AtomicInteger calls = new AtomicInteger();
ParentResolver counting = pid -> {
calls.incrementAndGet();
return parents.parentOf(pid);
};
HerdrClient noPanes = new FakeHerdr().withNoPanes();
PaneLocator two = new PaneLocator(noPanes, pane, counting);
assertEquals("term_x", two.terminalForPid(7002));
assertEquals(3, calls.get(), "ancestry must be walked once (3 lookups: 7002, 7001, 5000), "
+ "not re-walked per herdr client");
}
/** Minimal single-pane {@link HerdrClient} fake, purpose-built for the ancestry tests above. */
private static final class OnePaneHerdr implements HerdrClient {
private final ObjectMapper mapper = new ObjectMapper();
private final String terminalId;
private final String paneId;
private final long shellPid;
private final long foregroundPid;
OnePaneHerdr(String terminalId, String paneId, long shellPid, long foregroundPid) {
this.terminalId = terminalId;
this.paneId = paneId;
this.shellPid = shellPid;
this.foregroundPid = foregroundPid;
}
@Override
public JsonNode call(String method, Object params) {
try {
return switch (method) {
case "pane.list" -> mapper.readTree(("""
{"type":"pane_list","panes":[
{"pane_id":"%s","terminal_id":"%s","workspace_id":"w1","tab_id":"w1:t1","agent":"claude"}]}""")
.formatted(paneId, terminalId));
case "pane.process_info" -> mapper.readTree(("""
{"type":"pane_process_info","process_info":{"pane_id":"%s","shell_pid":%d,
"foreground_processes":[{"pid":%d,"name":"node","argv0":"claude"}]}}""")
.formatted(paneId, shellPid, foregroundPid));
default -> throw new HerdrException("OnePaneHerdr has no canned response for " + method);
};
} catch (HerdrException e) {
throw e;
} catch (Exception e) {
throw new HerdrException("OnePaneHerdr decode failed for " + method, e);
}
}
@Override
public void close() {
}
}
}
@@ -566,6 +566,73 @@ class CompletionResolverTest {
assertEquals("The usage limit has been reached.", waiter.getNow(null).text());
}
// --- fleetd#164 (part 2 addendum): narrow BACKEND_ERROR pattern classification ---------
@Test
void classifiesABackendErrorLineAsAFailureInsteadOfACompletedReply() {
String block = "⏺ API Error: 400 invalid request body\n❯ ";
FakeHerdr herdr = new FakeHerdr().readText(block);
Rendezvous rendezvous = new Rendezvous();
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none());
var waiter = rendezvous.open("term_a");
resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null));
assertTrue(waiter.isDone(), "a backend-error scrape still resolves the blocked send");
assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(),
"a backend rejection is a failure, not a completed reply");
}
@Test
void theBackendErrorReasonNamesTheMemberAndCarriesTheMatchedLine() {
String block = "⏺ API Error: 400 invalid request body\n❯ ";
FakeHerdr herdr = new FakeHerdr().readText(block);
Rendezvous rendezvous = new Rendezvous();
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none());
var waiter = rendezvous.open("term_a");
resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null));
String reason = waiter.getNow(null).text();
assertTrue(reason.contains("term_a"), "the failure names the member: " + reason);
assertTrue(reason.contains("API Error: 400 invalid request body"),
"the failure carries the matched backend-error line: " + reason);
}
@Test
void aCaseInsensitiveApiErrorLineIsStillClassifiedAsABackendError() {
String block = "⏺ api error: rate limited\n❯ ";
FakeHerdr herdr = new FakeHerdr().readText(block);
Rendezvous rendezvous = new Rendezvous();
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none());
var waiter = rendezvous.open("term_a");
resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null));
assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind(), "the pattern is case-insensitive");
}
@Test
void aBackendErrorFailureStillCarriesTheRestOfTheScrape() {
// The pattern is a heuristic: a member that forgot fleet_reply while *reporting on* a backend
// error matches it too. Failing is still correct, but the report itself must survive — losing
// it would be the same information-destroying defect fleetd#164 exists to fix.
String block = "\u23fa I looked into the gateway problem.\n"
+ "The log line was: API Error: 400 invalid request body\n"
+ "The cause is a missing content-type header.\n\u276f ";
FakeHerdr herdr = new FakeHerdr().readText(block);
Rendezvous rendezvous = new Rendezvous();
CompletionResolver resolver = new CompletionResolver(new AgentControl(herdr), rendezvous, ExhaustedPatternLookup.none(), ExhaustionSink.none());
var waiter = rendezvous.open("term_a");
resolver.resolve("term_a", new CompletionResolver.InFlight(waiter, null));
String reason = waiter.getNow(null).text();
assertEquals(Rendezvous.Kind.FAILED, waiter.getNow(null).kind());
assertTrue(reason.contains("The cause is a missing content-type header."),
"the failure carries the rest of the pane, not only the matched line: " + reason);
}
@Test
void coverageIsOffWhenNoProfileHasAPatternConfigured() {
assertEquals("off (no profile has an exhaustedPattern configured; profiles: [terra])",
@@ -1,5 +1,6 @@
package dev.ltms.fleet.member;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
@@ -1055,6 +1056,288 @@ class ClaudeCodeLauncherTest {
"a name already on allow: is covered, not a gap");
}
/**
* CB-633 follow-up (#192): the deny-by-default WARN wording is a promise an operator relies on —
* pinned byte-for-byte so a future edit cannot drift it (e.g. while picking wording for the
* allow-list path) without a test noticing.
*/
@Test
void denyByDefaultKeepsTheExactCredentialGapWarn() {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
_ -> null, 0, System::currentTimeMillis, () -> {}, null, () -> TEST_MEMBER_CREDENTIALS,
() -> Set.of("A_BRAND_NEW_SECRET_TOKEN"));
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
svc.spawn();
} finally {
logger.detachAppender(appender);
}
assertTrue(appender.list.stream().anyMatch(e ->
("memberCredentials gap: 1 credential-shaped env var name(s) are on neither "
+ "known: nor allow: — every member pane inherits them UNBLOCKED — "
+ "[A_BRAND_NEW_SECRET_TOKEN]. Add each to memberCredentials.known "
+ "(blocked by default) or .allow (if a member legitimately needs it).")
.equals(e.getFormattedMessage())),
"the deny-by-default WARN text must not drift — got: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* CB-633 follow-up (#192), defect 1: under {@code policy: allow-list} on a zsh login shell the
* generated ZDOTDIR scrub genuinely blanks an unkept credential-shaped name, so the report must
* not say the pane inherits it UNBLOCKED — that claim is exactly what PR #174 got wrong. This
* goes through the real spawn path (not {@code HerdrPeerLauncherAllowListWiringTest}'s fixture,
* which overrides {@code buildLaunch} and bypasses none of the logic under test here — the
* shell-dependent branch lives in {@code applyEnvironmentAllowListPolicy}, which every spawn
* still passes through).
*/
@Test
void allowListPolicyOnZshReportsTheGapWithoutClaimingItIsUnblocked() {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
FleetConfig.MemberCredentials creds = new FleetConfig.MemberCredentials(
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST,
List.of("AI_GATEWAY_TOKEN"), List.of());
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
name -> "SHELL".equals(name) ? "/bin/zsh" : null,
0, System::currentTimeMillis, () -> {}, null, () -> creds,
() -> Set.of("AI_GATEWAY_TOKEN", "A_BRAND_NEW_SECRET_TOKEN"));
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.INFO);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
svc.spawn();
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
assertTrue(appender.list.stream().anyMatch(e ->
e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN")),
"the unkept name must still be reported — got: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
assertFalse(appender.list.stream().anyMatch(e ->
e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("UNBLOCKED")),
"on zsh the scrub genuinely blanks the name, so the report must not claim it is "
+ "inherited unblocked — got: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* CB-633 follow-up (#192): the mirror of the zsh test above. On a non-zsh login shell {@code
* ZDOTDIR} is ignored, so no scrub ever runs — the report must keep the WARN wording (a name here
* really is inherited unblocked) rather than claiming a scrub protects it. This is the trap PR
* #174 fell into the other direction: keying the wording on the shell, not on {@code
* creds.isAllowList()}, is what keeps this branch correct.
*/
@Test
void allowListPolicyOnNonZshKeepsTheWarnWording() {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
FleetConfig.MemberCredentials creds = new FleetConfig.MemberCredentials(
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST,
List.of("AI_GATEWAY_TOKEN"), List.of());
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
name -> "SHELL".equals(name) ? "/bin/bash" : null,
0, System::currentTimeMillis, () -> {}, null, () -> creds,
() -> Set.of("AI_GATEWAY_TOKEN", "A_BRAND_NEW_SECRET_TOKEN"));
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
svc.spawn();
} finally {
logger.detachAppender(appender);
}
assertTrue(appender.list.stream().anyMatch(e ->
e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("UNBLOCKED")
&& e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN")),
"no scrub runs on a non-zsh shell, so the WARN wording must be kept — got: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
assertFalse(appender.list.stream().anyMatch(e ->
e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().toLowerCase(java.util.Locale.ROOT).contains("scrub")),
"nothing is scrubbed on this path, so the report must not claim otherwise — got: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* CB-633 follow-up (#192), defect 2: {@code memberCredentials} is a live, re-read-per-spawn
* supplier, so the policy can change between two spawns on the same launcher. Before this fix a
* single {@code AtomicBoolean} guarded both report kinds, so the harmless allow-list INFO on the
* first spawn would permanently suppress the real deny-by-default WARN a later spawn deserves.
* This goes through {@link ClaudeCodeLauncher#buildLaunch}'s real {@code baseEnv()} path — the
* WARN this test pins fires from {@code applyMemberCredentialPolicy}, which {@code
* HerdrPeerLauncherAllowListWiringTest}'s fixture never reaches at all (see its class javadoc).
*/
@Test
void secondSpawnStillWarnsAfterPolicyChangesFromAllowListToDenyByDefault() {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN",
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
AtomicReference<FleetConfig.MemberCredentials> creds = new AtomicReference<>(
new FleetConfig.MemberCredentials(FleetConfig.MemberCredentials.POLICY_ALLOW_LIST,
List.of("AI_GATEWAY_TOKEN"), List.of()));
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
name -> "SHELL".equals(name) ? "/bin/zsh" : null,
0, System::currentTimeMillis, () -> {}, null, creds::get,
() -> Set.of("AI_GATEWAY_TOKEN", "A_BRAND_NEW_SECRET_TOKEN"));
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.INFO);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
try {
logger.addAppender(appender);
svc.spawn(); // allow-list + zsh: harmless INFO, sets the allow-list guard only
creds.set(new FleetConfig.MemberCredentials(FleetConfig.MemberCredentials.POLICY_DENY_BY_DEFAULT,
List.of("AI_GATEWAY_TOKEN"), List.of()));
svc.spawn(); // policy reloaded to deny-by-default: this WARN must NOT be suppressed
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
assertTrue(appender.list.stream().anyMatch(e ->
e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("UNBLOCKED")),
"the second spawn's deny-by-default WARN must still fire even though the first "
+ "spawn's allow-list INFO already logged the same underlying gap — got: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* CB-633 follow-up (#192), lead-review fix: {@code effectiveAllowed} is a SUPERSET of
* {@code known ∪ allow} — {@code MemberEnvAllowList.derive} also unions in every profile's
* {@code tokenEnv} (among other fields), so a credential-shaped name can be uncovered by
* {@code known:}/{@code allow:} and STILL survive the scrub because a profile's own
* {@code tokenEnv} names it. Here {@code tokenEnv} is deliberately set to a credential-shaped
* name the operator forgot to list — the misconfiguration this report exists to catch. The scrub
* genuinely keeps it, so the report must WARN, not claim (as the pre-lead-review cut of this fix
* did) that "no member pane keeps them".
*/
@Test
void allowListWarnsWhenTheDerivedAllowListKeepsAnUncoveredName() {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "SOME_LEAKY_TOKEN",
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
FleetConfig.MemberCredentials creds = new FleetConfig.MemberCredentials(
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of());
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
name -> "SHELL".equals(name) ? "/bin/zsh" : null,
0, System::currentTimeMillis, () -> {}, null, () -> creds,
() -> Set.of("SOME_LEAKY_TOKEN"));
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.INFO);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
svc.spawn();
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
assertTrue(appender.list.stream().anyMatch(e ->
e.getLevel() == Level.WARN
&& e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("SOME_LEAKY_TOKEN")
&& e.getFormattedMessage().contains("UNBLOCKED")),
"a name kept by the derived allow-list (via this profile's tokenEnv) must still WARN "
+ "— got: " + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
assertFalse(appender.list.stream().anyMatch(e ->
e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("SOME_LEAKY_TOKEN")
&& e.getFormattedMessage().contains("blanks them anyway")),
"the scrub does NOT blank this name, so the INFO wording must not claim it does — got: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* CB-633 follow-up (#192), lead-review fix: a mixed gap — one name the derived allow-list keeps
* (this profile's {@code tokenEnv}), one it does not — must split cleanly: the WARN names only
* the kept one, the INFO names only the blanked one. Proves the split uses {@code
* MemberEnvAllowList.keeps} per-name rather than an all-or-nothing decision for the whole gap.
*/
@Test
void allowListSplitsAMixedGapBetweenTheWarnAndTheInfo() {
FakeHerdr herdr = new FakeHerdr();
FleetConfig.Profile cfg = new FleetConfig.Profile(
"ltms-local", "http://gx00.gw:8000", "coder", null, "SOME_LEAKY_TOKEN",
List.of("claude"), "tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
FleetConfig.MemberCredentials creds = new FleetConfig.MemberCredentials(
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), List.of());
ClaudeCodeLauncher svc = new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(),
name -> "SHELL".equals(name) ? "/bin/zsh" : null,
0, System::currentTimeMillis, () -> {}, null, () -> creds,
() -> Set.of("SOME_LEAKY_TOKEN", "A_BRAND_NEW_SECRET_TOKEN"));
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.INFO);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
svc.spawn();
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
boolean warnNamesOnlyKept = appender.list.stream().anyMatch(e ->
e.getLevel() == Level.WARN
&& e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("SOME_LEAKY_TOKEN")
&& !e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN"));
boolean infoNamesOnlyBlanked = appender.list.stream().anyMatch(e ->
e.getLevel() == Level.INFO
&& e.getFormattedMessage().contains("memberCredentials gap")
&& e.getFormattedMessage().contains("A_BRAND_NEW_SECRET_TOKEN")
&& !e.getFormattedMessage().contains("SOME_LEAKY_TOKEN"));
assertTrue(warnNamesOnlyKept, "the WARN must name the derived-list-kept variable and only it "
+ "— got: " + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
assertTrue(infoNamesOnlyBlanked, "the INFO must name the scrub-blanked variable and only it "
+ "— got: " + appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* The half of CB-592 that can actually survive the pane's login shell. BRIDGED_MEMBER is a name
* secrets.sh never exports, so nothing overwrites it — measured: GITEA_TOKEN is injected the
@@ -8,6 +8,7 @@ import dev.ltms.fleet.guard.SubscriptionGuard;
import dev.ltms.fleet.herdr.Agent;
import dev.ltms.fleet.herdr.AgentControl;
import dev.ltms.fleet.herdr.FakeHerdr;
import dev.ltms.fleet.herdr.HerdrException;
import dev.ltms.fleet.herdr.WorkspaceControl;
import dev.ltms.fleet.peer.Capability;
import dev.ltms.fleet.peer.CharterReceipt;
@@ -248,6 +249,193 @@ class CompositePeerLauncherTest {
"stop routes to the spawning adapter and closes exactly that worker's pane");
}
@Test
void stopKeepsSameHerdrPaneIdSeparateByOwningAdapter() {
// Separate herdr daemons can both issue w9:pRoot_1. The opaque handles identify their
// spawning adapters, so each stop reaches only its recorded owner.
FakeHerdr first = new FakeHerdr();
FakeHerdr second = new FakeHerdr();
PeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(first), opencodeAdapter(second)), "claude");
PeerHandle claude = composite.spawn(new SpawnRequest("claude", null, null));
PeerHandle opencode = composite.spawn(new SpawnRequest("gemini", null, null));
assertNotEquals(claude.id(), opencode.id(), "each public paneId keeps its adapter owner");
composite.stop(claude.id());
assertTrue(first.calls.stream().anyMatch(c -> c.method().equals("pane.close")
&& "w9:pRoot_1".equals(((Map<?, ?>) c.params()).get("pane_id"))),
"the first daemon closes its own pane");
assertFalse(second.called("pane.close"), "the matching pane on the second daemon stays live");
composite.stop(opencode.id());
assertTrue(second.calls.stream().anyMatch(c -> c.method().equals("pane.close")
&& "w9:pRoot_1".equals(((Map<?, ?>) c.params()).get("pane_id"))),
"the second daemon then closes its own pane");
}
@Test
void stopAllowsALegacyBarePaneIdWithOneDaemon() {
FakeHerdr herdr = new FakeHerdr();
PeerLauncher composite = new CompositePeerLauncher(List.of(claudeAdapter(herdr)), "claude");
composite.stop("w9:pRoot_1");
assertTrue(herdr.calls.stream().anyMatch(c -> c.method().equals("pane.close")
&& "w9:pRoot_1".equals(((Map<?, ?>) c.params()).get("pane_id"))),
"one daemon keeps the legacy bare-pane routing behaviour");
}
@Test
void stopAllowsAnUnownedPaneIdWithTwoAdaptersSharingOneDaemon() {
FakeHerdr herdr = new FakeHerdr();
PeerLauncher composite = composite(herdr);
composite.stop("w9:pRoot_1");
assertTrue(herdr.calls.stream().anyMatch(c -> c.method().equals("pane.close")
&& "w9:pRoot_1".equals(((Map<?, ?>) c.params()).get("pane_id"))),
"two adapter kinds sharing one daemon keep the fallback route");
}
@Test
void listKeepsBothPanesWhenTwoDaemonsShareAPaneId() {
// herdr pane ids are per-daemon counters, so two daemons really can both hold w1:p1 on
// different panes. Deduplicating on the pane id alone dropped one of the two real agents.
FakeHerdr first = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1");
FakeHerdr second = new FakeHerdr().withAgent("y", "term_y", "w1:p1", "w1:t1");
CompositePeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(first), opencodeAdapter(second)), "claude");
List<Agent> agents = composite.list();
assertEquals(2, agents.stream().filter(a -> "w1:p1".equals(a.paneId())).count(),
"one w1:p1 per daemon survives — the pane id alone is not a unique key");
assertTrue(agents.stream().anyMatch(a -> "term_x".equals(a.terminalId())));
assertTrue(agents.stream().anyMatch(a -> "term_y".equals(a.terminalId())));
}
@Test
void listStillDeduplicatesTwoAdaptersSharingOneDaemon() {
// Both adapters ask the SAME daemon, so both see the same agent set. Without the dedupe this
// would report every agent twice; the daemon key must not break that.
FakeHerdr herdr = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1");
CompositePeerLauncher composite = composite(herdr);
List<Agent> agents = composite.list();
assertEquals(1, agents.stream().filter(a -> "w1:p1".equals(a.paneId())).count(),
"one daemon still reports each of its agents once");
}
@Test
void stopKeepsTheOwnerRecordWhenTheDelegateRefusesTheStop() {
// Removing the record before the delegate accepted the stop lost the owner on failure: the
// pane was still alive, but the retry landed in the ambiguous branch and refused it for good.
FakeHerdr first = new FakeHerdr().paneCloseFailsWith("pane_busy");
CompositePeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(first), opencodeAdapter(new FakeHerdr())), "claude");
PeerHandle claude = composite.spawn(new SpawnRequest("claude", null, null));
assertThrows(HerdrException.class, () -> composite.stop(claude.id()));
// The retry must still know its owner — a HerdrException, never "ambiguous paneId".
assertThrows(HerdrException.class, () -> composite.stop(claude.id()),
"the owner record survives a failed stop, so the retry is not ambiguous");
}
@Test
void stopRejectsAnUnownedPaneIdWhenMultipleDaemonsCouldOwnIt() {
// CB-185 blocker 1: genuine ambiguity — pane ids are per-daemon counters, so two daemons
// can each really hold an agent at "w1:p1". Neither claims ownership through spawnedBy
// (empty, as after a restart), so the probe must find BOTH and refuse rather than guess.
FakeHerdr first = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1");
FakeHerdr second = new FakeHerdr().withAgent("y", "term_y", "w1:p1", "w1:t1");
PeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(first), opencodeAdapter(second)), "claude");
IllegalArgumentException error = assertThrows(IllegalArgumentException.class,
() -> composite.stop("w1:p1"));
assertEquals("ambiguous paneId 'w1:p1': 2 configured herdr daemons report this pane — "
+ "no way to tell which one the caller means", error.getMessage());
}
@Test
void stopOnAPaneNoConfiguredDaemonKnowsIsTreatedAsAlreadyStopped() {
// CB-185 blocker 1, the zero-owner branch: spawnedBy is empty (as after a restart) and
// neither daemon's agent.list mentions this pane at all — it is already gone. A retried
// stop() on an already-gone pane must succeed quietly, not refuse forever.
FakeHerdr first = new FakeHerdr();
FakeHerdr second = new FakeHerdr();
PeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(first), opencodeAdapter(second)), "claude");
assertDoesNotThrow(() -> composite.stop("w1:p1"));
assertFalse(first.called("pane.close"), "no owner was found, so no delegate is told to close anything");
assertFalse(second.called("pane.close"), "no owner was found, so no delegate is told to close anything");
}
@Test
void stopWithEmptySpawnedByResolvesTheOwnerThroughAProbeAndSkipsTheOtherDaemon() {
// CB-185 blocker 1, the main fix: after a restart spawnedBy is empty for every surviving
// member. stop() must still find the one daemon that actually knows the pane and route
// only to it — never touching the daemon that never held it.
FakeHerdr first = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1");
FakeHerdr second = new FakeHerdr();
PeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(first), opencodeAdapter(second)), "claude");
composite.stop("w1:p1");
assertTrue(first.calls.stream().anyMatch(c -> c.method().equals("pane.close")
&& "w1:p1".equals(((Map<?, ?>) c.params()).get("pane_id"))),
"the daemon that actually knows the pane closes it");
assertFalse(second.called("pane.close"), "the daemon that never held the pane is never touched");
}
@Test
void aProbeSurvivesOneUnreachableDaemonAndStillFindsTheOwnerOnTheOtherOne() {
// CB-185 blocker 1 (lead review): a daemon that is DOWN while we probe must not abort the
// whole probe — the pane the OPERATOR actually wants stopped can live on a different,
// healthy daemon, and that pane must not become un-stoppable because a third one is down.
FakeHerdr down = new FakeHerdr().healthy(false);
FakeHerdr owner = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1");
PeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(down), opencodeAdapter(owner)), "claude");
assertDoesNotThrow(() -> composite.stop("w1:p1"),
"the unreachable daemon must be skipped, not fail the whole stop");
assertTrue(owner.calls.stream().anyMatch(c -> c.method().equals("pane.close")
&& "w1:p1".equals(((Map<?, ?>) c.params()).get("pane_id"))),
"the healthy daemon that actually owns the pane still closes it");
}
@Test
void aProbedOwnerIsCachedSoARetryAfterAFailedStopNeedsNoSecondProbe() {
// CB-185 blocker 1: the probe's whole point is to be cheap on repeat — a failed stop (e.g.
// "pane_busy") must not force another agent.list() round trip on every retry.
FakeHerdr first = new FakeHerdr().withAgent("x", "term_x", "w1:p1", "w1:t1")
.paneCloseFailsWith("pane_busy");
FakeHerdr second = new FakeHerdr();
CompositePeerLauncher composite = new CompositePeerLauncher(
List.of(claudeAdapter(first), opencodeAdapter(second)), "claude");
assertThrows(HerdrException.class, () -> composite.stop("w1:p1"));
long listCallsAfterFirst = first.calls.stream().filter(c -> c.method().equals("agent.list")).count()
+ second.calls.stream().filter(c -> c.method().equals("agent.list")).count();
assertTrue(listCallsAfterFirst > 0, "the first stop needed a probe");
assertThrows(HerdrException.class, () -> composite.stop("w1:p1"),
"still failing on the retry, but through the cached owner");
long listCallsAfterSecond = first.calls.stream().filter(c -> c.method().equals("agent.list")).count()
+ second.calls.stream().filter(c -> c.method().equals("agent.list")).count();
assertEquals(listCallsAfterFirst, listCallsAfterSecond,
"the retry is served from the cache — no additional agent.list probe");
}
@Test
void opencodeContextResetIsANoOpAndWarnsOnlyOnce() {
FakeHerdr herdr = new FakeHerdr();
@@ -12,20 +12,26 @@ import dev.ltms.fleet.peer.Capability;
import dev.ltms.fleet.peer.MemberRole;
import dev.ltms.fleet.peer.SpawnRequest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Function;
import java.util.function.Supplier;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
/**
* CB-633: proves the allow-list scrub is actually WIRED INTO the spawn path — not merely that its
@@ -119,6 +125,18 @@ class HerdrPeerLauncherAllowListWiringTest {
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, allow, List.of(), null);
}
/**
* Same as {@link #allowList()} but with an operator-configured {@code known:} list, so the
* fallback overlay (the non-zsh / no-memberLoginShell path) has something visible to shadow —
* {@link FleetConfig.MemberCredentials#blockedSet()} is {@code known - allow}, so an empty
* {@code known} (what {@link #allowList()} uses) blocks nothing and a fallback test would have
* no sentinel entry to assert on.
*/
private static Supplier<FleetConfig.MemberCredentials> allowListWithKnown(List<String> known) {
return () -> new FleetConfig.MemberCredentials(
FleetConfig.MemberCredentials.POLICY_ALLOW_LIST, List.of(), known, null);
}
/**
* CB-633 follow-up: a name that lives ONLY in {@code memberCredentials.allow:} — no profile
* mentions it — must survive the scrub the real spawn path generates. Calling {@code
@@ -161,6 +179,34 @@ class HerdrPeerLauncherAllowListWiringTest {
+ "it under allow: — sshAuthSock is unset here, so it defaults to block");
}
@Test
void brokerUriEnvStaysBlockedWhenListedInMemberCredentialsAllow() {
FakeHerdr herdr = new FakeHerdr();
WiringLauncher launcher = new WiringLauncher(herdr,
allowListWithAllow(List.of("BROKER_CONNECTION_URI")), "/bin/zsh", null,
() -> config("BROKER_CONNECTION_URI"));
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
Path dir = Path.of(launcher.env.get("ZDOTDIR"));
assertFalse(readAll(dir.resolve(EnvAllowListScrub.SCRUB_FILE)).contains("'BROKER_CONNECTION_URI'"),
"broker.uriEnv must not reach a member even when listed in memberCredentials.allow:");
}
@Test
void brokerUriEnvIsDeniedUnderTheDenyListPolicyEvenWhenAllowed() {
FakeHerdr herdr = new FakeHerdr();
WiringLauncher launcher = new WiringLauncher(herdr,
() -> new FleetConfig.MemberCredentials(null, List.of("BROKER_CONNECTION_URI"), List.of(), null),
"/bin/bash", null,
() -> config("BROKER_CONNECTION_URI"));
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
assertEquals("blocked-by-fleetd-cb596-see-gitea-issue-82", launcher.env.get("BROKER_CONNECTION_URI"),
"the deny-list overlay must deny broker.uriEnv even when allow: names it");
}
/**
* CB-633 follow-up criterion 3: on every allow-list spawn the daemon logs one INFO line, shaped
* "member credentials: allowed N of M", with real counts — not constants. Real path: the count
@@ -229,6 +275,268 @@ class HerdrPeerLauncherAllowListWiringTest {
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* fleetd #185 stage 2 pin: with {@code memberHerdrSocket:} absent (today's only mode, and the
* default — this host runs no other), the gap detector's WARN/INFO conclusions read exactly as
* they did before this fix. Real path: {@code FLEETD_WORKER_TOKEN} (the test profile's own
* {@code tokenEnv}) is a name the derived allow-list keeps, so it gets the "UNBLOCKED" WARN;
* {@code SOME_UNKNOWN_SECRET_TOKEN} is not derived from anywhere, so it gets the "scrub blanks
* them" INFO. This is the exact shape #185 stage 2 must not touch on this path.
*/
@Test
void gapConclusionsAreByteIdenticalWhenMemberHerdrSocketIsAbsent() {
FakeHerdr herdr = new FakeHerdr();
Set<String> hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN");
WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames,
() -> config(null));
List<String> messages = spawnAndCaptureLogs(launcher);
assertTrue(messages.contains("memberCredentials gap: 1 credential-shaped env var name(s) are on "
+ "neither known: nor allow: — the derived allow-list keeps them anyway (a "
+ "profile's gitTokenEnv/gitHostEnv/tokenEnv/env: names one, or this spawn "
+ "injects it), so every member pane inherits them UNBLOCKED — [FLEETD_WORKER_TOKEN]. "
+ "Add each to memberCredentials.known (or .allow if a member legitimately needs "
+ "it), or remove it from whatever profile setting derives it in."),
"expected the pre-existing UNBLOCKED WARN unchanged, got: " + messages);
assertTrue(messages.contains("memberCredentials gap: 1 credential-shaped env var name(s) are on "
+ "neither known: nor allow: — [SOME_UNKNOWN_SECRET_TOKEN]. The allow-list scrub "
+ "blanks them anyway (they are not on the derived allow-list), so no member pane "
+ "keeps them; add each to memberCredentials.known or .allow to make that explicit."),
"expected the pre-existing 'scrub blanks them' INFO unchanged, got: " + messages);
}
/**
* fleetd #185 stage 2: with {@code memberHerdrSocket:} configured, member panes run under a
* different OS user — {@link HerdrPeerLauncher#hostEnvNames} describes fleetd's own process, not
* that user's. Neither "inherits them UNBLOCKED" nor "scrub blanks them" is evidence-backed
* there, so neither may print; the single unknown-environment WARN must, naming the config key.
*/
@Test
void gapDetectorReportsUnknownInsteadOfAConclusionWhenMemberHerdrSocketIsConfigured() {
FakeHerdr herdr = new FakeHerdr();
Set<String> hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN");
WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames,
() -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock"));
List<String> messages = spawnAndCaptureLogs(launcher);
assertTrue(messages.stream().anyMatch(m -> m.contains("memberHerdrSocket")
&& m.contains("UNKNOWN") && m.contains("cannot be verified")),
"expected the unknown-member-environment WARN naming memberHerdrSocket, got: " + messages);
assertFalse(messages.stream().anyMatch(m -> m.contains("UNBLOCKED")),
"the 'inherits them UNBLOCKED' conclusion must not print once the evidence is about "
+ "the wrong (daemon's own) environment — got: " + messages);
assertFalse(messages.stream().anyMatch(m -> m.contains("scrub blanks them")),
"the 'scrub blanks them' conclusion must not print once the evidence is about the "
+ "wrong (daemon's own) environment — got: " + messages);
}
/**
* fleetd #185 stage 2: the unknown-environment WARN is a standing fact about this launcher's
* configuration, not per-spawn news — it must fire once per launcher instance, the same shape as
* every other one-time WARN in this class (e.g. {@code warnNonZsh}).
*/
@Test
void theUnknownEnvironmentWarnFiresOnceNotOncePerSpawn() {
FakeHerdr herdr = new FakeHerdr();
Set<String> hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN");
WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames,
() -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock"));
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.WARN);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
long count = appender.list.stream()
.filter(e -> e.getFormattedMessage().contains("cannot be verified from here"))
.count();
assertEquals(1, count, "the unknown-member-environment WARN must fire once per launcher "
+ "instance, not once per spawn — got " + count + " occurrence(s) among: "
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
}
/**
* Hard constraint: the gap detector must never log an env var VALUE, only its NAME. {@code
* SOME_UNKNOWN_SECRET_TOKEN} resolves to a distinctive canary value through the same {@code env}
* lookup the launcher uses elsewhere (SHELL, PATH, token resolution) — proving the value IS
* resolvable does not mean the detector reads it, since {@link HerdrPeerLauncher#hostEnvNames}
* (names only) is its data source, never {@code env.apply(name)} for those names.
*/
@Test
void theGapDetectorNeverLogsAnEnvVarValueOnlyItsName() {
FakeHerdr herdr = new FakeHerdr();
String canary = "sekrit-value-CANARY-9f3a1b7c";
Set<String> hostEnvNames = Set.of("FLEETD_WORKER_TOKEN", "SOME_UNKNOWN_SECRET_TOKEN");
WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/zsh", () -> hostEnvNames,
() -> config(null), Map.of("SOME_UNKNOWN_SECRET_TOKEN", canary));
List<String> messages = spawnAndCaptureLogs(launcher);
assertTrue(messages.stream().anyMatch(m -> m.contains("SOME_UNKNOWN_SECRET_TOKEN")),
"expected the credential-shaped NAME to appear in the log, got: " + messages);
assertFalse(messages.stream().anyMatch(m -> m.contains(canary)),
"the log must never contain an env var VALUE, only its NAME — got: " + messages);
}
/**
* fleetd #213 defect 1, acceptance criterion 1: {@code memberHerdrSocket} configured and {@code
* memberLoginShell} configured as non-zsh must fall back to the sentinel overlay exactly like a
* non-zsh {@code $SHELL} does today — and the "generated ZDOTDIR" INFO must not appear, since no
* scrub actually runs. The WiringLauncher's own {@code env("SHELL")} is deliberately set to
* {@code /bin/zsh} — the OPPOSITE of what {@code memberLoginShell} says — so a launcher that
* (incorrectly) fell back to fleetd's own {@code $SHELL} here would wrongly pass the gate and
* fail this test.
*/
@Test
void memberHerdrSocketWithNonZshMemberLoginShellFallsBackToTheOverlay() {
FakeHerdr herdr = new FakeHerdr();
WiringLauncher launcher = new WiringLauncher(herdr, allowListWithKnown(List.of("SOME_TOKEN")),
"/bin/zsh", null,
() -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock", "/bin/bash"));
List<String> messages = spawnAndCaptureLogs(launcher);
assertEquals("blocked-by-fleetd-cb596-see-gitea-issue-82", launcher.env.get("SOME_TOKEN"),
"a non-zsh memberLoginShell must fall back to the CB-596 sentinel overlay, exactly "
+ "like a non-zsh $SHELL does when memberHerdrSocket is absent");
assertFalse(launcher.env.containsKey("ZDOTDIR"),
"no scrub directory may be generated when the configured member login shell is not zsh");
assertFalse(messages.stream().anyMatch(m -> m.contains("generated ZDOTDIR")),
"the 'generated ZDOTDIR' INFO must not appear when the scrub never runs — got: " + messages);
}
/**
* fleetd #213 defect 1, acceptance criterion 2: {@code memberHerdrSocket} configured and NO
* {@code memberLoginShell} configured must fall back exactly like criterion 1 above — AND
* fleetd's own {@code $SHELL} must never even be consulted (not merely "not decisive"). The
* fixture's {@code env} function reports {@code /bin/zsh} for {@code SHELL} — a value that would
* WRONGLY pass the zsh gate if the fix regressed to reading it — while flagging whether it was
* ever asked for at all, so this test fails loudly on either kind of regression.
*/
@Test
void memberHerdrSocketWithNoMemberLoginShellFallsBackAndNeverConsultsFleetdsOwnShell() {
FakeHerdr herdr = new FakeHerdr();
AtomicBoolean shellQueried = new AtomicBoolean(false);
Function<String, String> env = name -> {
if ("SHELL".equals(name)) {
shellQueried.set(true);
return "/bin/zsh"; // would wrongly pass the zsh gate if this ever leaked through
}
return null;
};
WiringLauncher launcher = new WiringLauncher(herdr, allowListWithKnown(List.of("SOME_TOKEN")), env,
() -> configWithMemberHerdrSocket("/tmp/other-user-herdr.sock", null));
List<String> messages = spawnAndCaptureLogs(launcher);
assertFalse(shellQueried.get(), "fleetd's own $SHELL must never be consulted once "
+ "memberHerdrSocket is configured — only memberLoginShell: may decide the gate");
assertEquals("blocked-by-fleetd-cb596-see-gitea-issue-82", launcher.env.get("SOME_TOKEN"),
"no memberLoginShell configured must fall back to the sentinel overlay, same as a "
+ "configured non-zsh shell");
assertFalse(messages.stream().anyMatch(m -> m.contains("generated ZDOTDIR")),
"no scrub may run without a configured memberLoginShell — got: " + messages);
}
/**
* fleetd #213 defect 2, acceptance criterion 3: with {@code memberHerdrSocket} configured, a
* zsh {@code memberLoginShell}, and {@code worktreeRoot}/{@code worktreeGroup} both configured,
* the generated scrub directory must live under {@code worktreeRoot} — NEVER under {@code
* java.io.tmpdir}, which is fleetd's own 0700 temp dir and unreadable by the member's different
* OS user. {@code worktreeGroup} is set to the CURRENT process's own primary group so {@code
* EnvAllowListScrub}'s group-sharing step resolves on whatever host runs this test, rather than
* hardcoding a group name that may not exist here.
*/
@Test
void memberHerdrSocketWithZshMemberLoginShellPutsTheScrubOutsideJavaIoTmpdir(@TempDir Path worktreeRoot)
throws IOException {
String group = currentUserGroup();
FakeHerdr herdr = new FakeHerdr();
WiringLauncher launcher = new WiringLauncher(herdr, allowList(), "/bin/bash-should-be-ignored", null,
() -> configWithMemberHerdrSocketRootAndGroup("/tmp/other-user-herdr.sock", "/bin/zsh",
worktreeRoot.toString(), group));
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
String zdotdir = launcher.env.get("ZDOTDIR");
assertNotNull(zdotdir, "a zsh memberLoginShell with worktreeRoot+worktreeGroup configured "
+ "must still generate a ZDOTDIR");
Path dir = Path.of(zdotdir);
// The immediate PARENT is asserted (not merely startsWith(java.io.tmpdir)), because a
// JUnit @TempDir is itself carved out of the JVM's java.io.tmpdir — startsWith alone would
// pass by coincidence of the test fixture, not because the launcher used worktreeRoot.
assertEquals(worktreeRoot.toAbsolutePath().normalize(), dir.getParent(),
"the generated scrub directory's parent must be the configured worktreeRoot, not "
+ "System.getProperty(\"java.io.tmpdir\") — got parent " + dir.getParent());
}
/**
* fleetd #213, acceptance criterion 4: with {@code memberHerdrSocket} absent (today's only
* mode), behaviour must be byte-identical to before this fix — fleetd's own {@code $SHELL}
* still decides the gate (proven here, not merely assumed, by flagging the lookup), and the
* scrub still lands under {@code java.io.tmpdir}.
*/
@Test
void memberHerdrSocketAbsentStillConsultsFleetdsOwnShellAndBehavesAsBefore() {
FakeHerdr herdr = new FakeHerdr();
AtomicBoolean shellQueried = new AtomicBoolean(false);
Function<String, String> env = name -> {
if ("SHELL".equals(name)) {
shellQueried.set(true);
return "/bin/zsh";
}
return null;
};
WiringLauncher launcher = new WiringLauncher(herdr, allowList(), env, null); // memberHerdrSocket absent
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
assertTrue(shellQueried.get(), "with memberHerdrSocket absent, fleetd's own $SHELL must "
+ "still decide the zsh gate, unchanged from before this fix");
String zdotdir = launcher.env.get("ZDOTDIR");
assertNotNull(zdotdir, "SHELL=/bin/zsh with memberHerdrSocket absent must still generate a "
+ "ZDOTDIR, as before this fix");
Path dir = Path.of(zdotdir);
assertTrue(dir.startsWith(Path.of(System.getProperty("java.io.tmpdir"))),
"with memberHerdrSocket absent the scrub directory must still be generated under "
+ "java.io.tmpdir, unchanged from before this fix: " + dir);
}
/** The current process's own primary group — resolvable on whatever host runs this test. */
private static String currentUserGroup() throws IOException {
PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class);
assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership");
return view.readAttributes().group().getName();
}
/** Spawn once through the real launcher path, capturing every INFO+ line this class logs. */
private static List<String> spawnAndCaptureLogs(HerdrPeerLauncher launcher) {
Logger logger = (Logger) LoggerFactory.getLogger(HerdrPeerLauncher.class);
Level original = logger.getLevel();
logger.setLevel(Level.INFO);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.start();
logger.addAppender(appender);
try {
launcher.spawn(new SpawnRequest("test", null, null, null, null, MemberRole.DEV));
} finally {
logger.detachAppender(appender);
logger.setLevel(original);
}
return appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList();
}
private static String readAll(Path p) {
try {
return Files.readString(p);
@@ -260,15 +568,52 @@ class HerdrPeerLauncherAllowListWiringTest {
/** Plus an injectable {@code hostEnvNames} source, for the "allowed N of M" log line test. */
WiringLauncher(FakeHerdr herdr, Supplier<FleetConfig.MemberCredentials> creds, String shell,
Supplier<Set<String>> hostEnvNames) {
Supplier<Set<String>> hostEnvNames) {
this(herdr, creds, shell, hostEnvNames, null);
}
WiringLauncher(FakeHerdr herdr, Supplier<FleetConfig.MemberCredentials> creds, String shell,
Supplier<Set<String>> hostEnvNames, Supplier<FleetConfig> config) {
this(herdr, creds, shell, hostEnvNames, config, Map.of());
}
/**
* Plus a host-env value map (name → value), resolved through the same {@code env} lookup
* every adapter uses for {@code SHELL}/{@code PATH}/token resolution — fleetd #185 stage 2's
* "the gap detector never logs a value" tests use this to prove a value that IS resolvable
* for a credential-shaped name never reaches the log, since the detector only ever reads
* {@code hostEnvNames} (names), never {@code env.apply(name)} (values), for those names.
*/
WiringLauncher(FakeHerdr herdr, Supplier<FleetConfig.MemberCredentials> creds, String shell,
Supplier<Set<String>> hostEnvNames, Supplier<FleetConfig> config,
Map<String, String> extraEnvValues) {
super("test", new AgentControl(herdr), new WorkspaceControl(herdr),
Map.of("test", profile()), "test",
name -> "SHELL".equals(name) ? shell : null,
0, () -> 0L, () -> { }, null, creds, hostEnvNames);
name -> "SHELL".equals(name) ? shell
: (extraEnvValues != null && extraEnvValues.containsKey(name))
? extraEnvValues.get(name) : null,
0, () -> 0L, () -> { }, null, creds, hostEnvNames, config);
}
/**
* Full control over the {@code env} lookup, bypassing the {@code shell}/{@code
* extraEnvValues} convenience above entirely — fleetd #213's "fleetd's own $SHELL must
* never be consulted" tests need to OBSERVE whether {@code SHELL} was ever looked up, which
* a plain value substitution cannot do.
*/
WiringLauncher(FakeHerdr herdr, Supplier<FleetConfig.MemberCredentials> creds,
Function<String, String> env, Supplier<FleetConfig> config) {
super("test", new AgentControl(herdr), new WorkspaceControl(herdr),
Map.of("test", profile()), "test",
env, 0, () -> 0L, () -> { }, null, creds, null, config);
}
@Override
protected Launch buildLaunch(FleetConfig.Profile cfg, LaunchSpec spec) {
Map<String, String> launchEnv = baseEnv(cfg);
launchEnv.putAll(env);
env.clear();
env.putAll(launchEnv);
return new Launch(env, List.of("test"));
}
@@ -278,6 +623,88 @@ class HerdrPeerLauncherAllowListWiringTest {
}
}
private static FleetConfig config(String brokerUriEnv) {
return new FleetConfig(null, null, null, Map.of(), null, null, null, null, null,
new FleetConfig.Broker(null, brokerUriEnv, null), null, null, null, null, null,
null, null, null, null, null).withDefaults();
}
/**
* fleetd #185 stage 2: a config with {@code memberHerdrSocket:} set — member panes run on a
* second herdr owned by a different OS user, so {@link HerdrPeerLauncher#hostEnvNames} no
* longer describes what a member pane inherits.
*/
private static FleetConfig configWithMemberHerdrSocket(String memberHerdrSocket) {
return new FleetConfig(null, null, memberHerdrSocket, Map.of(), null, null, null, null, null,
null, null, null, null, null, null, null, null, null, null, null).withDefaults();
}
/**
* fleetd #213: as {@link #configWithMemberHerdrSocket(String)}, plus the {@code
* memberLoginShell:} the member's OS user actually runs — the config key {@link
* HerdrPeerLauncher#applyEnvironmentAllowListPolicy} must consult instead of fleetd's own
* {@code $SHELL} once {@code memberHerdrSocket} is configured.
*/
private static FleetConfig configWithMemberHerdrSocket(String memberHerdrSocket, String memberLoginShell) {
return new FleetConfig(
null, // bind
null, // herdrSocket
memberHerdrSocket, // memberHerdrSocket
Map.of(), // profiles
null, // guard
null, // worktreeRoot
null, // lifecycle
null, // spawnReadyTimeoutMs
null, // spawnReadyPollMs
null, // broker
null, // primary
null, // fleet
null, // leadHeartbeat
null, // health
null, // placement
null, // auth
null, // configReload
null, // quarantineCooldownSeconds
null, // memberCredentials
null, // coordinator
null, // worktreeGroup
memberLoginShell // memberLoginShell
).withDefaults();
}
/**
* fleetd #213: as above, plus {@code worktreeRoot:}/{@code worktreeGroup:} — both required for
* the ZDOTDIR scrub to run at all once {@code memberHerdrSocket} is configured; either missing
* falls back to the sentinel overlay, same as a non-zsh {@code memberLoginShell}.
*/
private static FleetConfig configWithMemberHerdrSocketRootAndGroup(String memberHerdrSocket,
String memberLoginShell, String worktreeRoot, String worktreeGroup) {
return new FleetConfig(
null, // bind
null, // herdrSocket
memberHerdrSocket, // memberHerdrSocket
Map.of(), // profiles
null, // guard
worktreeRoot, // worktreeRoot
null, // lifecycle
null, // spawnReadyTimeoutMs
null, // spawnReadyPollMs
null, // broker
null, // primary
null, // fleet
null, // leadHeartbeat
null, // health
null, // placement
null, // auth
null, // configReload
null, // quarantineCooldownSeconds
null, // memberCredentials
null, // coordinator
worktreeGroup, // worktreeGroup
memberLoginShell // memberLoginShell
).withDefaults();
}
/** The generated directory is a temp directory; make sure the test does not leave a pile. */
@Test
void theGeneratedDirectoryIsRemovedWhenThePaneIsStopped() {
@@ -121,6 +121,34 @@ class MemberEnvAllowListTest {
assertTrue(derived.contains("OTHER_NAME"), "other allow: names are unaffected");
}
@Test
void configuredBrokerAndCoordinatorUriEnvNamesAreExcludedEvenWhenAllowed() {
FleetConfig config = config("BROKER_CONNECTION_URI", "COORDINATOR_CONNECTION_URI");
Set<String> excluded = MemberEnvAllowList.brokerUriEnvNames(config);
Set<String> derived = MemberEnvAllowList.derive(List.of(),
Set.of("BROKER_CONNECTION_URI", "COORDINATOR_CONNECTION_URI", "OTHER_NAME"), excluded);
assertFalse(derived.contains("BROKER_CONNECTION_URI"),
"broker.uriEnv is secret-bearing and must not ride in on allow:");
assertFalse(derived.contains("COORDINATOR_CONNECTION_URI"),
"coordinator.uriEnv has the same inline-password shape");
assertTrue(derived.contains("OTHER_NAME"), "unrelated allow: entries are unaffected");
}
@Test
void absentOrBlankBrokerUriEnvAddsNoExclusions() {
assertTrue(MemberEnvAllowList.brokerUriEnvNames(config(null, null)).isEmpty());
assertTrue(MemberEnvAllowList.brokerUriEnvNames(config(" ", "")).isEmpty());
}
private static FleetConfig config(String brokerUriEnv, String coordinatorUriEnv) {
return new FleetConfig(null, null, null, Map.of(), null, null, null, null, null,
new FleetConfig.Broker(null, brokerUriEnv, null), null, null, null, null, null,
null, null, null, null,
new FleetConfig.Coordinator(null, coordinatorUriEnv, null, null)).withDefaults();
}
/** {@code LC_*} categories are infrastructure by prefix; everything else needs an exact match. */
@Test
void keepsMatchesExactlyPlusTheLocalePrefixRule() {
@@ -334,8 +334,7 @@ class OpenCodeLauncherTest {
assertNull(handle.agentSessionId(), "no record yet → null, not a spawn-time block");
// Once the record appears (here: same cwd), lazy discovery resolves it — the handle's
// session id matches its own worktree, not another's.
OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "p1", "ses_a.json",
"ses_resolved", "/work/dir", 1000L);
OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_resolved", "/work/dir", 1000L);
assertEquals("ses_resolved", handle.agentSessionId(),
"agentSessionId() re-scans and picks up a record that has since been written");
}
@@ -5,68 +5,89 @@ import org.junit.jupiter.api.io.TempDir;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.sql.Statement;
import static org.junit.jupiter.api.Assertions.*;
/**
* {@link OpenCodeSessionDiscovery} matches an opencode session record by the worker's cwd (its
* {@code directory}) against opencode's on-disk storage. These tests populate a TEMP storage root
* themselves — never the operator's real {@code ~/.local/share/opencode}.
* {@link OpenCodeSessionDiscovery} matches an opencode session row by the worker's cwd (its
* {@code directory}) against opencode's {@code opencode.db} SQLite database. These tests build a
* SYNTHETIC database themselves, in a JUnit temp directory — never the operator's real
* {@code ~/.local/share/opencode/opencode.db}, which a live opencode process may be writing.
*/
class OpenCodeSessionDiscoveryTest {
/**
* Write a session record {@code {"id":..., "directory":...}} under
* {@code <root>/session/<projectID>/<fileName>} and stamp it with a known last-modified time,
* so "most recently modified wins" is deterministic. Static so the launcher test can reuse it.
* Create {@code <root>/opencode.db} with a minimal {@code session} table (just the columns
* {@link OpenCodeSessionDiscovery} reads: {@code id}, {@code directory}, {@code time_updated})
* and insert one row. Static so {@link OpenCodeLauncherTest} can reuse it.
*/
static void writeRecord(Path root, String projectId, String fileName, String id,
String directory, long lastModifiedEpochMillis) throws Exception {
Path dir = root.resolve("session").resolve(projectId);
Files.createDirectories(dir);
Path file = dir.resolve(fileName);
Files.writeString(file, "{\"id\":\"" + id + "\",\"directory\":\"" + directory
+ "\",\"projectID\":\"" + projectId + "\",\"version\":\"1.1.31\"}");
Files.setLastModifiedTime(file, FileTime.fromMillis(lastModifiedEpochMillis));
static void writeRecord(Path root, String id, String directory, long timeUpdated) throws Exception {
Path db = root.resolve("opencode.db");
try (Connection connection = DriverManager.getConnection("jdbc:sqlite:" + db)) {
try (Statement statement = connection.createStatement()) {
statement.execute("CREATE TABLE IF NOT EXISTS session ("
+ "id TEXT PRIMARY KEY, directory TEXT, time_updated INTEGER)");
}
// Bound parameters, not string interpolation: the class under test uses a
// PreparedStatement, and a hand-escaped INSERT here is a pattern someone copies out.
try (PreparedStatement insert = connection.prepareStatement(
"INSERT INTO session (id, directory, time_updated) VALUES (?, ?, ?)")) {
insert.setString(1, id);
insert.setString(2, directory);
insert.setLong(3, timeUpdated);
insert.executeUpdate();
}
}
}
@Test
void findsTheRecordWhoseDirectoryEqualsTheCwd(@TempDir Path root) throws Exception {
writeRecord(root, "p1", "ses_a.json", "ses_aaa", "/w/a", 1000L);
writeRecord(root, "p2", "ses_b.json", "ses_bbb", "/w/b", 2000L);
void findsTheRowWhoseDirectoryEqualsTheCwd(@TempDir Path root) throws Exception {
writeRecord(root, "ses_aaa", "/w/a", 1000L);
writeRecord(root, "ses_bbb", "/w/b", 2000L);
assertEquals("ses_bbb", new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/b"),
"the record whose directory equals the cwd is the one found");
"the row whose directory equals the cwd is the one found");
assertEquals("ses_aaa", new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/a"));
}
@Test
void aNonMatchingDirectoryYieldsNullRatherThanAMismatch(@TempDir Path root) throws Exception {
writeRecord(root, "p1", "ses_a.json", "ses_aaa", "/w/a", 1000L);
writeRecord(root, "ses_aaa", "/w/a", 1000L);
assertNull(new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/other"),
"no record for this cwd yet → null, not a wrong session");
"no row for this cwd yet → null, not a wrong session");
}
@Test
void prefersTheMostRecentlyModifiedRecordWhenSeveralMatch(@TempDir Path root) throws Exception {
writeRecord(root, "p1", "old.json", "ses_old", "/w/a", 1000L);
writeRecord(root, "p2", "new.json", "ses_new", "/w/a", 5000L);
void prefersTheMostRecentlyUpdatedRowWhenSeveralMatch(@TempDir Path root) throws Exception {
writeRecord(root, "ses_old", "/w/a", 1000L);
writeRecord(root, "ses_new", "/w/a", 5000L);
assertEquals("ses_new", new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/a"),
"the freshest record for the cwd wins");
"the row with the highest time_updated for the cwd wins");
}
@Test
void aMissingOrEmptyStorageRootYieldsNullWithoutThrowing(@TempDir Path root) throws Exception {
// Missing: no session dir at all under the root.
void aMissingDatabaseYieldsNullWithoutThrowing(@TempDir Path root) {
// No opencode.db at all under the root.
assertNull(new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/a"));
}
// Present but empty: a session dir with nothing in it produces no match, not a throw.
Path emptyRoot = root.resolve("empty");
Files.createDirectories(emptyRoot.resolve("session"));
assertNull(new OpenCodeSessionDiscovery(emptyRoot).sessionIdForDirectory("/w/a"));
@Test
void anEmptyDatabaseYieldsNullWithoutThrowing(@TempDir Path root) throws Exception {
Path db = root.resolve("opencode.db");
try (Connection connection = DriverManager.getConnection("jdbc:sqlite:" + db);
Statement statement = connection.createStatement()) {
statement.execute("CREATE TABLE session (id TEXT PRIMARY KEY, directory TEXT, "
+ "time_updated INTEGER)");
}
assertNull(new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/a"));
}
@Test
@@ -76,15 +97,41 @@ class OpenCodeSessionDiscoveryTest {
assertNull(discovery.sessionIdForDirectory(" "));
}
/**
* The one line standing between fleetd and writing the operator's live {@code opencode.db} —
* 841MB, with a running opencode writing it — is {@code config.setReadOnly(true)} in
* {@link OpenCodeSessionDiscovery#openReadOnly()}. Delete it and every other test in this class
* still passes, so this is the test that guards it.
*
* <p>It asks the connection to write, and requires a refusal. The obvious alternative — make
* the database file unwritable and check the read still works — proves nothing: SQLite silently
* downgrades a read-write open of an unwritable file to read-only, so that test passes either
* way. It was tried and watched pass with the flag removed.
*/
@Test
void aMalformedRecordIsSkippedRatherThanFatal(@TempDir Path root) throws Exception {
// A record that fails to parse must not abort the scan of its siblings.
Path dir = root.resolve("session").resolve("p1");
Files.createDirectories(dir);
Files.writeString(dir.resolve("broken.json"), "{not valid json");
writeRecord(root, "p1", "good.json", "ses_good", "/w/a", 1000L);
void theDatabaseIsOpenedReadOnly(@TempDir Path root) throws Exception {
writeRecord(root, "ses_aaa", "/w/a", 1000L);
assertEquals("ses_good", new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/a"),
"an unreadable record is skipped; a later valid one still matches");
try (Connection connection = new OpenCodeSessionDiscovery(root).openReadOnly();
Statement statement = connection.createStatement()) {
SQLException refused = assertThrows(SQLException.class,
() -> statement.executeUpdate("INSERT INTO session (id, directory, time_updated) "
+ "VALUES ('ses_zzz', '/w/z', 1)"),
"the connection must REFUSE a write — opencode is writing this database live");
assertTrue(refused.getMessage().toLowerCase().contains("readonly")
|| refused.getMessage().toLowerCase().contains("read-only"),
"the refusal must be about read-only, not some other error: " + refused.getMessage());
}
}
@Test
void aCorruptDatabaseFileYieldsNullWithoutThrowing(@TempDir Path root) throws Exception {
// A file at opencode.db that is not a SQLite database at all — the open/query must fail
// safe, never fatal to a spawn.
Path db = root.resolve("opencode.db");
Files.writeString(db, "this is not a sqlite database");
assertNull(new OpenCodeSessionDiscovery(root).sessionIdForDirectory("/w/a"),
"an unreadable database resolves to null, not an exception");
}
}
@@ -86,6 +86,28 @@ class MessageServiceTest {
assertTrue(reply.completed(), "a scraped completion still counts as completed");
}
@Test
void backendErrorScrapeThroughMessageServiceFailsInsteadOfBecomingReplyText() throws Exception {
// fleetd#164 (part 2 addendum): a scrape that reads cleanly but is only the backend's own
// rejection (e.g. an HTTP 400) must reach the caller as WORKER_FAILED, not as a completed
// reply whose text happens to be the error line.
CompletableFuture<MessageService.Reply> send = sendAsync();
awaitWaiting();
herdr.readText("$ prompt");
injector.onStatus(T, AgentStatus.IDLE);
injector.onStatus(T, AgentStatus.WORKING);
herdr.readText("⏺ API Error: 400 invalid request body");
injector.onStatus(T, AgentStatus.IDLE);
MessageService.Reply reply = send.get(5, TimeUnit.SECONDS);
assertEquals(MessageService.Outcome.WORKER_FAILED, reply.outcome(),
"a backend rejection must use the caller's failure outcome, not a completed reply");
assertFalse(reply.completed(), "plain backend errors are never fallback reply content");
assertTrue(reply.text().contains("API Error: 400 invalid request body"),
"the visible backend error is carried as the failure reason: " + reply.text());
}
@Test
void explicitFleetReplyResolvesAsReplied() throws Exception {
CompletableFuture<MessageService.Reply> send = sendAsync();
@@ -666,6 +688,32 @@ class MessageServiceTest {
assertFailedTicket(third, "agent target term_a not found");
}
// --- #137 follow-up: abandon() must not guess when more than one task is open ---------------
//
// A test combining a genuine stranded reply (hasStrandedReply(T)==true) with two simultaneously
// open matching tasks was attempted here and removed after investigation showed the combination
// is not reachable through the public API today, not merely hard to time right:
//
// This class has exactly two call sites that ever hold a target's entry in the session-lock map
// (send() and answer()), and both open a Rendezvous waiter for that same target as the first thing
// they do after acquiring the lock, holding lock and waiter together for their whole critical
// section. So "the lock is held" and "a live waiter is open" are the same fact throughout this
// class. reply()'s fast path always resolves a currently-open waiter directly instead of
// stranding — so a strand can only be created while NO task is accepted (lock free), and the
// instant the lock is next taken (by any parked matching task's own send(), the moment it is
// scheduled), that acceptance clears strandedReplies again (see send()'s CB-640 comment) before
// abandon() can ever observe both facts together. Confirmed empirically too: an earlier version of
// this test stranded a reply, then created an "accepted" task (awaitWaiting()) followed by a
// "parked" one — and the accepted task's own acceptance silently cleared the strand it was
// supposed to be racing against, so the parked task came back WORKER_FAILED instead of DONE, not
// because the fix was missing but because the test's premise could not be constructed.
//
// The reachable half — matching.size() >= 2 alone, no strand — is exactly what
// abandonFailsEveryPendingAsyncTicketForTheReleasedTarget already covers (all fail, none guess).
// The oldest-wins code in abandon() stays as defence in depth (see its own javadoc) against a
// regression that would make the conjunction reachable, e.g. clearing strandedReplies on a
// narrower condition than "any acceptance" — not because this suite exercises it today.
@Test
void abandonDoesNotFailAnAsyncTicketWaitingForAnAnswer() throws Exception {
String ticket = messages.sendAsync(T, "task that asks");
@@ -688,6 +736,68 @@ class MessageServiceTest {
assertEquals(MessageService.Outcome.REPLIED, answer.get(5, TimeUnit.SECONDS).outcome());
}
// --- #137: a fleet_ask round-trip must not orphan the ticket's own reply -------------------
//
// The primary's fleet_send{turnId} answer call is itself bounded (a real MCP call, capped well
// under a minute) — far shorter than a resumed turn can genuinely take to finish real work. These
// drive the exact real delegation path (async send -> worker asks -> primary answers -> primary's
// own wait gives up -> worker's real fleet_reply arrives afterwards) rather than calling a reply
// sink directly, since the bug is specifically about which sink the resumed turn's reply reaches.
@Test
void aReplyAfterAnswerTimesOutStillCompletesTheAsyncTicket() throws Exception {
String ticket = messages.sendAsync(T, "task that asks");
awaitWaiting();
injectDelivery();
CompletableFuture<MessageService.AskResult> ask =
CompletableFuture.supplyAsync(() -> messages.ask(T, "which config?", 5000));
MessageService.TaskView asking = awaitTicketPhase(ticket, MessageService.Phase.ASKING);
// The primary answers, but its own bounded wait for the worker's resumed turn is short and
// expires before the worker (still genuinely working) gets back to it.
MessageService.Reply answerReply = messages.answer(asking.turnId(), "config.yaml", 150);
assertEquals("config.yaml", ask.get(5, TimeUnit.SECONDS).answer());
assertEquals(MessageService.Outcome.TIMED_OUT_WORKING, answerReply.outcome(),
"the primary's own bounded wait gives up before the worker finishes resuming");
// The worker keeps working past that window and only now calls fleet_reply.
assertTrue(messages.reply(T, "PR opened: https://example/pulls/42"));
MessageService.TaskView done = awaitTicketPhase(ticket, MessageService.Phase.DONE);
assertEquals("PR opened: https://example/pulls/42", done.reply(),
"fleet_poll{ticket} must return the worker's real reply, not stay pending forever");
assertEquals("reply", done.replySource());
assertFalse(messages.hasStrandedReply(T),
"the reply completed its own ticket directly and never touched the inbox");
}
@Test
void fleetStopAfterAnOrphanedReplyDoesNotFailTheTicket() throws Exception {
String ticket = messages.sendAsync(T, "task that asks");
awaitWaiting();
injectDelivery();
CompletableFuture<MessageService.AskResult> ask =
CompletableFuture.supplyAsync(() -> messages.ask(T, "which config?", 5000));
MessageService.TaskView asking = awaitTicketPhase(ticket, MessageService.Phase.ASKING);
MessageService.Reply answerReply = messages.answer(asking.turnId(), "config.yaml", 150);
assertEquals("config.yaml", ask.get(5, TimeUnit.SECONDS).answer());
assertEquals(MessageService.Outcome.TIMED_OUT_WORKING, answerReply.outcome());
assertTrue(messages.reply(T, "PR opened: https://example/pulls/42"));
// fleet_stop tears the worker's session down right after the reply landed — this must never
// report the misleading "the worker session was released before it replied": a reply is
// exactly what happened.
assertFalse(messages.abandon(T, "the worker session was released before it replied"),
"a reply already arrived, so nothing here is a genuine failure");
MessageService.TaskView view = awaitTicketPhase(ticket, MessageService.Phase.DONE);
assertEquals("PR opened: https://example/pulls/42", view.reply());
}
@Test
void unansweredAsyncQuestionReturnsTheTicketToPendingAndReleasesItsTarget() throws Exception {
String ticket = messages.sendAsync(T, "task that asks");
@@ -1057,6 +1167,74 @@ class MessageServiceTest {
}
}
/**
* #197: the ticket TTL must run from COMPLETION, not from creation.
*
* <p>It used to compare the cutoff against {@code createdNanos}, so the real window to collect a
* reply was {@code TTL minus however long the task ran}. A delegation that ran longer than the
* TTL was already past the cutoff the moment it finished, so the very next prune destroyed its
* reply — and the reply lives only in the task's future, so nothing could get it back. That is
* the normal case for real work here, not an edge case: three workers in one session ran well
* past ten minutes and two of their complete reports were lost this way.
*
* <p>The task below runs for longer than the whole TTL before it replies, which is exactly the
* shape that used to lose everything. Remove the fix and this fails: {@code poll} returns
* {@code null} because the ticket was pruned on arrival.
*/
@Test
void aTaskRunningLongerThanTheTtlStillKeepsItsReport() throws Exception {
java.util.concurrent.atomic.AtomicLong clock = new java.util.concurrent.atomic.AtomicLong(1_000_000_000L);
try (var wiring = wireWithPushLoop(1, 50, clock::get)) {
String slow = wiring.service().sendAsync(T, "a task that takes longer than the TTL");
awaitWaiting();
injectDelivery();
// The worker is still working, and has been for longer than the entire TTL. Nothing may
// be pruned yet — the ticket has not finished, so there is no report to keep or lose.
clock.addAndGet(MessageService.TICKET_TTL_NANOS + TimeUnit.SECONDS.toNanos(30));
// Only now does it reply. Under the old clock this reply was born already expired.
assertTrue(rendezvous.resolve(T, "the long report"));
awaitTicketPhaseOn(wiring.service(), slow, MessageService.Phase.DONE);
// A second delegation runs pruneTerminalTickets before it returns.
wiring.service().sendAsync(T, "an unrelated second task");
MessageService.TaskView view = wiring.service().poll(slow);
assertNotNull(view, "a ticket that completed just now must survive the prune, however "
+ "long its task ran — the TTL is the window to COLLECT the report, not the "
+ "budget for producing it");
assertEquals(MessageService.Phase.DONE, view.phase());
assertEquals("the long report", view.reply(),
"the worker's actual report must still be there, not just the ticket");
}
}
/**
* The other half of #197: the TTL must still bound {@code tasks}. Measuring from completion
* would be a leak if a finished ticket were then kept forever, so this pins the eviction that
* still has to happen — the same ticket, left uncollected for longer than the TTL AFTER it
* finished, is gone.
*/
@Test
void aFinishedTicketIsStillPrunedOnceTheTtlPassesSinceItFinished() throws Exception {
java.util.concurrent.atomic.AtomicLong clock = new java.util.concurrent.atomic.AtomicLong(1_000_000_000L);
try (var wiring = wireWithPushLoop(1, 50, clock::get)) {
String done = wiring.service().sendAsync(T, "a quick task");
awaitWaiting();
injectDelivery();
assertTrue(rendezvous.resolve(T, "quick result"));
awaitTicketPhaseOn(wiring.service(), done, MessageService.Phase.DONE);
// Nobody collected it, and the TTL has now passed since it FINISHED.
clock.addAndGet(MessageService.TICKET_TTL_NANOS + TimeUnit.SECONDS.toNanos(1));
wiring.service().sendAsync(T, "an unrelated second task");
assertNull(wiring.service().poll(done),
"the TTL must still evict an uncollected finished ticket, or tasks grows forever");
}
}
private MessageService.TaskView awaitTicketPhaseOn(MessageService svc, String ticket,
MessageService.Phase phase) throws Exception {
long deadline = System.currentTimeMillis() + 3000;
@@ -218,9 +218,17 @@ class FleetAppTest {
HttpResponse<String> res = req(port, "GET", "/members");
assertEquals(200, res.statusCode());
JsonNode workers = mapper.readTree(res.body()).get("workers");
assertEquals(1, workers.size());
JsonNode w = workers.get(0);
JsonNode body = mapper.readTree(res.body());
// fleetd #199: "members" is the canonical key. The endpoint is /members, so a caller that
// reads "members" must not see an empty fleet. "workers" is kept only as a deprecated alias
// and must carry the same rows — assert both, or the alias can silently drift.
JsonNode members = body.get("members");
assertNotNull(members, "GET /members must return its rows under \"members\"");
assertEquals(1, members.size());
JsonNode workers = body.get("workers");
assertNotNull(workers, "the deprecated \"workers\" alias is still emitted");
assertEquals(members, workers, "the alias must carry the same rows as \"members\"");
JsonNode w = members.get(0);
assertEquals(spawned.get("terminalId").asText(), w.get("sessionId").asText());
assertEquals(paneId, w.get("paneId").asText());
assertEquals("ltms-local", w.get("profile").asText());
@@ -96,4 +96,55 @@ class FleetAppTwoDaemonTest {
long calls = shared.calls.stream().filter(c -> c.method().equals("workspace.list")).count();
assertEquals(1, calls, "single-daemon deployment must call workspace.list exactly once");
}
// ── CB-185 blocker 2: /healthz must report the MEMBER daemon's protocol too ────────────────
@Test
void healthzReportsBothDaemonsWhenTheirProtocolsDiffer() throws Exception {
FakeHerdr lead = new FakeHerdr().pingReports("0.8.0", 19);
FakeHerdr member = new FakeHerdr().pingReports("0.7.0", 18);
int port = start(lead, member);
HttpResponse<String> res = get(port, "/healthz");
assertEquals(200, res.statusCode(), res.body());
assertTrue(res.body().contains("\"protocol\":19"),
"the herdr key keeps reporting the LEAD's protocol, unchanged: " + res.body());
assertTrue(res.body().contains("\"member\""), "a separate member key is present: " + res.body());
assertTrue(res.body().contains("\"protocol\":18"),
"the member key reports the member daemon's own protocol: " + res.body());
assertTrue(res.body().contains("\"protocolMismatch\":true"),
"a differing protocol is called out explicitly, not left to be spotted by eye: " + res.body());
}
@Test
void healthzReportsBothDaemonsWithNoMismatchWhenProtocolsMatch() throws Exception {
int port = start(new FakeHerdr(), new FakeHerdr());
HttpResponse<String> res = get(port, "/healthz");
assertEquals(200, res.statusCode(), res.body());
assertTrue(res.body().contains("\"member\""), "the member key is present whenever a second daemon "
+ "is configured, even when the protocols happen to agree: " + res.body());
assertFalse(res.body().contains("protocolMismatch"),
"matching protocols must not raise a mismatch flag: " + res.body());
}
@Test
void healthzWithOneDaemonCarriesNoMemberOrMismatchKey() throws Exception {
// The single-daemon deployment (no memberHerdrSocket) must see no change at all beyond the
// historical body: no "member" key, no "protocolMismatch" key. (Map.of()'s own key order is
// JVM-salted regardless of this fix, so this checks content, not exact key order.)
FakeHerdr shared = new FakeHerdr();
int port = start(shared, shared);
HttpResponse<String> res = get(port, "/healthz");
assertEquals(200, res.statusCode());
assertTrue(res.body().contains("\"status\":\"ok\""), res.body());
assertTrue(res.body().contains("\"protocol\":19"), res.body());
assertTrue(res.body().contains("\"version\":\"0.8.0\""), res.body());
assertFalse(res.body().contains("\"member\""), "no second daemon configured, so no member key: " + res.body());
assertFalse(res.body().contains("protocolMismatch"), res.body());
}
}
@@ -30,12 +30,19 @@ public final class FakeWorktrees implements Worktrees {
public record PruneCall(String repoRoot, long minAgeMillis) {
}
public record ShareCall(String repoRoot, String worktreePath) {
}
private final List<AddCall> addCalls = new CopyOnWriteArrayList<>();
private final List<RemoveCall> removeCalls = new CopyOnWriteArrayList<>();
private final List<OverlayCall> overlayCalls = new CopyOnWriteArrayList<>();
private final List<RepoRootCall> repoRootCalls = new CopyOnWriteArrayList<>();
private final List<SnapshotCall> snapshotCalls = new CopyOnWriteArrayList<>();
private final List<PruneCall> pruneCalls = new CopyOnWriteArrayList<>();
private final List<ShareCall> shareCalls = new CopyOnWriteArrayList<>();
/** Tags every {@code overlayParity}/{@code shareWithGroup} call in call order, so a test can
* pin that sharing runs after the overlay copy (fleetd #185 stage 3). */
private final List<String> overlayShareOrder = new CopyOnWriteArrayList<>();
private final Set<String> existingPaths = ConcurrentHashMap.newKeySet();
private final Set<String> trackedPaths = ConcurrentHashMap.newKeySet();
private final AtomicLong snapshotSeq = new AtomicLong();
@@ -136,6 +143,13 @@ public final class FakeWorktrees implements Worktrees {
}
overlayCalls.add(new OverlayCall(repoRoot, worktreePath, List.copyOf(overlay),
List.copyOf(copied), List.copyOf(skipped)));
overlayShareOrder.add("overlay:" + worktreePath);
}
@Override
public void shareWithGroup(String repoRoot, String worktreePath) {
shareCalls.add(new ShareCall(repoRoot, worktreePath));
overlayShareOrder.add("share:" + worktreePath);
}
@Override
@@ -203,4 +217,17 @@ public final class FakeWorktrees implements Worktrees {
public SnapshotCall lastSnapshot() {
return snapshotCalls.isEmpty() ? null : snapshotCalls.getLast();
}
public List<ShareCall> shareCalls() {
return List.copyOf(shareCalls);
}
public ShareCall lastShare() {
return shareCalls.isEmpty() ? null : shareCalls.getLast();
}
/** Call-order tags ({@code "overlay:<path>"}/{@code "share:<path>"}) — see field javadoc. */
public List<String> overlayShareOrder() {
return List.copyOf(overlayShareOrder);
}
}
@@ -1,13 +1,23 @@
package dev.ltms.fleet.session;
import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.IThrowableProxy;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
import org.slf4j.LoggerFactory;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.TimeUnit;
@@ -352,6 +362,151 @@ class GitWorktreesTest {
assertEquals("worktree origin contains HTTPS user info; refusing provision", error.getMessage());
}
// ---- CB-189: broader remote-URL coverage — every remote, both fetch and push URLs, any
// non-SSH scheme. Reporting only, additive to the origin/https strip-and-refuse tests above. ----
private Logger reportingLogger;
private ListAppender<ILoggingEvent> reportingAppender;
/** {@link GitWorktrees}'s own logger, captured fresh for each test so assertions never see a
* message left over from a previous test. */
@BeforeEach
void attachReportingLogCapture() {
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
reportingLogger = ctx.getLogger(GitWorktrees.class);
reportingLogger.setLevel(Level.WARN);
reportingAppender = new ListAppender<>();
reportingAppender.setContext(ctx);
reportingAppender.start();
reportingLogger.addAppender(reportingAppender);
}
@AfterEach
void detachReportingLogCapture() {
reportingLogger.detachAppender(reportingAppender);
}
private List<String> capturedMessages() {
return reportingAppender.list.stream().map(ILoggingEvent::getFormattedMessage).toList();
}
/** Asserts {@code secret} appears in no captured message, and in no attached exception's
* message either — the constraint is that a credential must never reach a log, however it
* would have gotten there. */
private void assertNoLeak(String secret) {
for (ILoggingEvent event : reportingAppender.list) {
assertFalse(event.getFormattedMessage().contains(secret),
"log message leaked a credential (" + secret + "): " + event.getFormattedMessage());
IThrowableProxy thrown = event.getThrowableProxy();
if (thrown != null && thrown.getMessage() != null) {
assertFalse(thrown.getMessage().contains(secret),
"logged exception leaked a credential (" + secret + "): " + thrown.getMessage());
}
}
}
/** Gap 1: only {@code origin} was ever inspected. A credential on any other remote's fetch URL
* must now be reported. */
@Test
void aCredentialedUrlOnANonOriginRemoteIsReported(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
git(repo, "remote", "add", "origin", "https://git.ltms.dev/akb/kb.git");
git(repo, "remote", "add", "upstream", "https://leaky-upstream-token@git.ltms.dev/akb/kb.git");
new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-189-a", "HEAD");
List<String> messages = capturedMessages();
assertTrue(messages.stream().anyMatch(m -> m.contains("remote=upstream")),
"expected a report naming the leaking non-origin remote:\n" + messages);
assertNoLeak("leaky-upstream-token");
assertNoLeak("https://leaky-upstream-token@git.ltms.dev/akb/kb.git");
assertNoLeak("git.ltms.dev");
}
/** Gap 2: push URLs were never inspected. A credential visible only on {@code pushurl} — the
* fetch URL for the same remote stays clean — must now be reported. */
@Test
void aCredentialedPushUrlIsReported(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
git(repo, "remote", "add", "origin", "https://git.ltms.dev/akb/kb.git");
git(repo, "remote", "add", "mirror", "https://git.ltms.dev/akb/mirror.git");
git(repo, "remote", "set-url", "--push", "mirror",
"https://leaky-push-token@git.ltms.dev/akb/mirror.git");
new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-189-b", "HEAD");
List<String> messages = capturedMessages();
assertTrue(messages.stream().anyMatch(m -> m.contains("remote=mirror")),
"expected a report naming the remote with the leaking pushurl:\n" + messages);
assertNoLeak("leaky-push-token");
assertNoLeak("https://leaky-push-token@git.ltms.dev/akb/mirror.git");
assertNoLeak("git.ltms.dev");
}
/** Gap 3: only {@code https} was handled. A plain {@code http://user:pass@…} remote — worse
* than https, not better — must now be reported. */
@Test
void anHttpUrlWithCredentialsIsReported(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
git(repo, "remote", "add", "origin", "https://git.ltms.dev/akb/kb.git");
git(repo, "remote", "add", "insecure", "http://plainuser:plainpass@git.ltms.dev/akb/kb.git");
new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-189-c", "HEAD");
List<String> messages = capturedMessages();
assertTrue(messages.stream().anyMatch(m -> m.contains("remote=insecure")),
"expected a report for the credentialed plain-http remote:\n" + messages);
assertNoLeak("plainuser");
assertNoLeak("plainpass");
assertNoLeak("plainuser:plainpass");
assertNoLeak("git.ltms.dev");
}
/** A normal {@code ssh://} remote and a credential-free {@code https://} remote must produce no
* report at all — the check must not cry wolf on ordinary, safe configuration. */
@Test
void anSshRemoteAndACleanHttpsRemoteProduceNoReport(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
git(repo, "remote", "add", "origin", "ssh://git@git.ltms.dev:2224/akb/kb.git");
git(repo, "remote", "add", "clean", "https://git.ltms.dev/akb/kb.git");
new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-189-d", "HEAD");
assertTrue(reportingAppender.list.isEmpty(),
"expected no report for an ssh remote and a credential-free https remote, got:\n"
+ capturedMessages());
}
/**
* CB-189 review fix. Even a FAILING command that read a credential onto its stdout must never
* let that value reach the thrown {@link WorktreeException}'s message — this is gap 4 from the
* CB-189 issue, and the reason {@link GitWorktrees#execRedacted} exists at all. Drives the
* shared {@code exec}/{@code execRedacted} seam directly (it is package-private for exactly this,
* the same way the {@code afterWorktreeAdded} constructor parameter is a test seam) with a
* synthetic, non-git command whose stdout carries a marker — passed through the environment,
* never through argv, so the marker cannot leak via the command line that IS always printed
* unconditionally in the exception message — and which exits non-zero. This isolates the
* redaction guarantee itself rather than depending on a specific git failure mode that happens to
* echo a URL onto stdout before failing: none of the git subcommands this class actually runs was
* found to have one (a corrupted config makes {@code git config --get} fail before it ever reads
* the target key, so its output never carries the URL either). The marker is generated per-test
* run and injected only by the test, never a real-looking credential, so even a failing assertion
* could not itself print a secret.
*/
@Test
void execRedactedNeverCopiesFailingCommandOutputIntoTheExceptionMessage(@TempDir Path tmp) {
String marker = "cb189-marker-" + System.nanoTime();
GitWorktrees worktrees = new GitWorktrees(tmp.toString());
WorktreeException thrown = assertThrows(WorktreeException.class, () -> worktrees.exec(
Map.of("MARKER", marker), true, "sh", "-c", "echo \"$MARKER\"; exit 7"));
assertNotNull(thrown.getMessage());
assertFalse(thrown.getMessage().contains(marker),
"a failing command's captured stdout leaked into the exception message: "
+ thrown.getMessage());
}
/** Neutralizing must not look like work in progress, or a worker would commit it into its PR. */
@Test
void theNeutralizedConfigIsNotAPendingLocalModification(@TempDir Path tmp) throws Exception {
@@ -769,4 +924,179 @@ class GitWorktreesTest {
assertEquals(2, stats.count(), "two snapshot refs are reported");
assertTrue(stats.costBytes() > 0, "the cost of the snapshots is a positive byte count");
}
/**
* fleetd #185 stage 3: a recording {@link java.util.function.Function} test seam stands in for
* every process {@link GitWorktrees#shareWithGroup} would run — no real second OS user/group
* exists on this host, so these are unit tests against that seam, not a live-group integration
* test (out of scope per the ticket).
*/
private static List<String> joined(String[] command) {
return List.of(command);
}
/** Remove {@code path} and anything under it. Tolerates an already-absent path. */
private static void deleteRecursively(Path path) throws Exception {
if (!Files.exists(path)) {
return;
}
if (Files.isDirectory(path)) {
try (java.util.stream.Stream<Path> children = Files.list(path)) {
for (Path child : children.toList()) {
deleteRecursively(child);
}
}
}
Files.delete(path);
}
/** {@code worktreeGroup} absent ⇒ zero processes spawned and no git config written. */
@Test
void shareWithGroupIsNoopWhenNoGroupConfigured(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
List<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> recordingRunner = cmd -> {
recorded.add(joined(cmd));
return "";
};
GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString(), null, _ -> {}, recordingRunner);
gitWorktrees.shareWithGroup(repo.toString(), repo.resolve("some-worktree").toString());
assertTrue(recorded.isEmpty(), "no group configured must spawn no process at all: " + recorded);
}
/** A configured group runs {@code git config core.sharedRepository group} first, then
* chgrp/chmod/setgid over every path {@link GitWorktrees#shareWithGroup} documents. */
@Test
void shareWithGroupRunsConfigThenChgrpChmodSetgidPerPath(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
String repoRoot = repo.toString();
Path worktree = Files.createDirectories(repo.resolve("some-worktree"));
String worktreePath = worktree.toString();
Files.createDirectories(repo.resolve(".git/worktrees"));
List<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> recordingRunner = cmd -> {
recorded.add(joined(cmd));
// What real git answers for an ordinary (non-linked) checkout: relative to repoRoot.
return List.of(cmd).contains("--git-common-dir") ? ".git\n" : "";
};
GitWorktrees gitWorktrees =
new GitWorktrees(tmp.resolve("wts").toString(), "devteam", _ -> {}, recordingRunner);
gitWorktrees.shareWithGroup(repoRoot, worktreePath);
assertEquals(List.of("git", "-C", repoRoot, "config", "core.sharedRepository", "group"), recorded.get(0),
"core.sharedRepository must be set first, so it keeps working after the one-time fix-up");
assertTrue(recorded.contains(List.of("git", "-C", repoRoot, "rev-parse", "--git-common-dir")),
"the git dir must be asked for, never hardcoded as <repoRoot>/.git — that is a FILE "
+ "when the checkout is itself a linked worktree: " + recorded);
for (String dir : List.of(worktreePath, repoRoot + "/.git/objects", repoRoot + "/.git/refs",
repoRoot + "/.git/logs", repoRoot + "/.git/worktrees")) {
assertTrue(recorded.contains(List.of("chgrp", "-R", "devteam", dir)), "missing chgrp -R for " + dir);
assertTrue(recorded.contains(List.of("chmod", "-R", "g+rwX", dir)), "missing chmod -R for " + dir);
assertTrue(recorded.contains(List.of("find", dir, "-type", "d", "-exec", "chmod", "g+s", "{}", "+")),
"missing setgid find pass for " + dir);
}
// packed-refs does not exist in a freshly-init'd repo (only git gc / pack-refs creates it) —
// tolerated absence, so it must not appear at all: no recursive/-R treatment for a plain file.
String packedRefs = repoRoot + "/.git/packed-refs";
assertTrue(recorded.stream().noneMatch(c -> c.contains(packedRefs)),
"packed-refs is absent here and must be skipped, not chgrp'd: " + recorded);
}
/**
* A path that does not exist is skipped, never handed to {@code chgrp}. {@code .git/logs} is
* absent whenever {@code core.logAllRefUpdates} is false or no ref has been updated yet, and
* {@code chgrp} on a missing path exits non-zero — which would fail EVERY provisioning spawn
* with a message blaming a group that is in fact fine.
*/
@Test
void shareWithGroupSkipsPathsThatDoNotExist(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
String repoRoot = repo.toString();
deleteRecursively(repo.resolve(".git/logs"));
assertFalse(Files.exists(repo.resolve(".git/logs")), "fixture: .git/logs must be gone");
List<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> recordingRunner = cmd -> {
recorded.add(joined(cmd));
return List.of(cmd).contains("--git-common-dir") ? ".git\n" : "";
};
GitWorktrees gitWorktrees =
new GitWorktrees(tmp.resolve("wts").toString(), "devteam", _ -> {}, recordingRunner);
gitWorktrees.shareWithGroup(repoRoot, repo.resolve("no-such-worktree").toString());
String logs = repoRoot + "/.git/logs";
assertTrue(recorded.stream().noneMatch(c -> c.contains(logs)),
"a missing .git/logs must be skipped, not chgrp'd: " + recorded);
assertTrue(recorded.stream().noneMatch(c -> c.contains(repo.resolve("no-such-worktree").toString())),
"a missing worktree path must be skipped too: " + recorded);
assertTrue(recorded.contains(List.of("chgrp", "-R", "devteam", repoRoot + "/.git/objects")),
"paths that DO exist are still shared: " + recorded);
}
/**
* The git store is located by {@code rev-parse --git-common-dir}, not by appending
* {@code /.git}. When git answers with an absolute path — what it does for a linked worktree,
* where {@code <repoRoot>/.git} is a file — every shared path must follow that answer.
*/
@Test
void shareWithGroupFollowsAnAbsoluteGitCommonDir(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
Path realGitDir = repo.resolve(".git");
List<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> recordingRunner = cmd -> {
recorded.add(joined(cmd));
return List.of(cmd).contains("--git-common-dir") ? realGitDir + "\n" : "";
};
GitWorktrees gitWorktrees =
new GitWorktrees(tmp.resolve("wts").toString(), "devteam", _ -> {}, recordingRunner);
gitWorktrees.shareWithGroup(tmp.resolve("some/linked/worktree").toString(),
repo.resolve("wt").toString());
assertTrue(recorded.contains(List.of("chgrp", "-R", "devteam", realGitDir + "/objects")),
"objects must be taken from the reported common dir, not <repoRoot>/.git: " + recorded);
}
/** {@code packed-refs}, when present, is chgrp/chmod'd but never setgid'd (it is a file, not a dir). */
@Test
void shareWithGroupIncludesPackedRefsWhenPresent(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
String repoRoot = repo.toString();
Path packedRefsPath = repo.resolve(".git/packed-refs");
Files.writeString(packedRefsPath, "");
List<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> recordingRunner = cmd -> {
recorded.add(joined(cmd));
return "";
};
GitWorktrees gitWorktrees =
new GitWorktrees(tmp.resolve("wts").toString(), "devteam", _ -> {}, recordingRunner);
gitWorktrees.shareWithGroup(repoRoot, repo.resolve("some-worktree").toString());
String packedRefs = packedRefsPath.toString();
assertTrue(recorded.contains(List.of("chgrp", "devteam", packedRefs)),
"packed-refs must be chgrp'd non-recursively when present: " + recorded);
assertTrue(recorded.contains(List.of("chmod", "g+rwX", packedRefs)),
"packed-refs must be chmod'd non-recursively when present: " + recorded);
assertTrue(recorded.stream().noneMatch(c -> c.contains("find") && c.contains(packedRefs)),
"packed-refs (a file) must never get the recursive setgid pass: " + recorded);
}
/** A group that does not exist (or that the operator is not a member of) fails loudly, naming it. */
@Test
void shareWithGroupThrowsNamingTheGroupWhenChgrpFails(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
GitWorktrees gitWorktrees = new GitWorktrees(tmp.resolve("wts").toString(), "cb185-nonexistent-group-zz");
String wt = new GitWorktrees(tmp.resolve("wts").toString()).add(repo.toString(), "cb-185-share", "HEAD");
WorktreeException e = assertThrows(WorktreeException.class,
() -> gitWorktrees.shareWithGroup(repo.toString(), wt));
assertTrue(e.getMessage().contains("cb185-nonexistent-group-zz"),
"exception must name the missing/refused group: " + e.getMessage());
}
}
@@ -148,6 +148,10 @@ class SessionManagerTest {
return 0;
}
@Override
public void shareWithGroup(String repoRoot, String worktreePath) {
}
List<String> removeCalls() {
return List.copyOf(removeCalls);
}
@@ -927,4 +931,235 @@ class SessionManagerTest {
assertTrue(e.getMessage().contains("SESSION_RESUME"), e.getMessage());
assertTrue(e.getMessage().contains("stub-profile"), e.getMessage());
}
// ── fleetd #209: agentSessionId resolved lazily against the retained handle ─────────────────
//
// The opencode adapter cannot answer PeerHandle.agentSessionId() at spawn time — the on-disk
// session row is written only after the pane is live — so the id must be re-polled on a LATER
// call, against the SAME handle instance the launcher returned at spawn. SessionManager used
// to let that handle go out of scope at the end of the spawn method, so no caller ever re-asked
// it and fleet_list/fleet_status never saw the id. LazyIdHandle below reproduces exactly that
// shape: null on the first N calls (the spawn-time call included), a real id after.
/**
* A {@link PeerHandle} whose {@link #agentSessionId()} answers {@code null} for its first
* {@code nullCalls} invocations, then either a fixed id or a configured throw on every call
* after that — the shape of the opencode bug (fleetd #209): the session row is not written
* until after the pane is live, so early polls come back empty and a later one finds it.
*/
private static final class LazyIdHandle implements PeerHandle {
private final String id;
private final String terminalId;
private final int nullCalls;
private final String resolvedId;
private final java.util.concurrent.atomic.AtomicInteger calls =
new java.util.concurrent.atomic.AtomicInteger();
private volatile RuntimeException throwAfter;
LazyIdHandle(String id, String terminalId, int nullCalls, String resolvedId) {
this.id = id;
this.terminalId = terminalId;
this.nullCalls = nullCalls;
this.resolvedId = resolvedId;
}
/** After the null calls are exhausted, throw instead of answering the resolved id. */
LazyIdHandle throwing(RuntimeException e) {
this.throwAfter = e;
return this;
}
@Override
public String id() {
return id;
}
@Override
public String terminalId() {
return terminalId;
}
@Override
public String agentSessionId() {
int n = calls.incrementAndGet();
if (n <= nullCalls) {
return null;
}
if (throwAfter != null) {
throw throwAfter;
}
return resolvedId;
}
@Override
public CharterReceipt charterReceipt() {
return null;
}
int callCount() {
return calls.get();
}
}
/** A minimal {@link PeerLauncher} that hands out pre-built {@link LazyIdHandle}s, one per spawn. */
private static final class LazyIdLauncher implements PeerLauncher {
private final java.util.Deque<LazyIdHandle> queued = new java.util.ArrayDeque<>();
LazyIdLauncher queue(LazyIdHandle handle) {
queued.add(handle);
return this;
}
@Override
public Set<Capability> capabilities() {
return Set.of(Capability.WORKTREE, Capability.SESSION_RESUME);
}
@Override
public Set<Capability> capabilitiesFor(String profileName) {
return capabilities();
}
@Override
public PeerHandle spawn(SpawnRequest req) {
LazyIdHandle handle = queued.poll();
if (handle == null) {
throw new IllegalStateException("no queued LazyIdHandle for this spawn");
}
return handle;
}
@Override
public Set<String> profiles() {
return Set.of("lazy");
}
@Override
public String defaultProfile() {
return "lazy";
}
@Override
public String effectiveCwd(SpawnRequest req) {
return "/cwd";
}
@Override
public List<String> parityOverlay(String profileName) {
return List.of();
}
@Override
public List<?> list() {
return List.of();
}
@Override
public int reapOrphanWorkers() {
return 0;
}
@Override
public void stop(String id) {
}
@Override
public boolean clearContext(String id) {
return false;
}
}
@Test
void plainRosterDoesNotResolveAgentSessionId() {
// fleetd #209 follow-up: roster() sits on the heartbeat/health-tick timers (and the metrics
// scrape), so it must never trigger the resolve lookup — for opencode that lookup opens an
// on-disk session database, and a member whose id never appears would pay that cost forever.
// rosterResolved() is the one to use when a caller actually reports the id.
LazyIdHandle handle = new LazyIdHandle("p0", "t0", 1, "oc-session-0");
SessionManager sessions = new SessionManager(new LazyIdLauncher().queue(handle));
MemberSession acquired = sessions.acquire("lazy", "/cwd", "/caller", null);
assertEquals(1, handle.callCount(), "sanity: only the spawn-time call happened so far");
List<MemberSession> roster = sessions.roster();
assertEquals(1, roster.size());
assertEquals(acquired.paneId(), roster.getFirst().paneId());
assertNull(roster.getFirst().agentSessionId(), "the plain roster must not resolve the id");
assertEquals(1, handle.callCount(),
"roster() must never call agentSessionId() again — it sits on the heartbeat/health timers");
}
@Test
void rosterResolvedResolvesALateAgentSessionIdFromTheRetainedHandle() {
LazyIdHandle handle = new LazyIdHandle("p1", "t1", 1, "oc-session-1");
SessionManager sessions = new SessionManager(new LazyIdLauncher().queue(handle));
MemberSession acquired = sessions.acquire("lazy", "/cwd", "/caller", null);
assertNull(acquired.agentSessionId(),
"opencode has not written its session row yet at spawn time");
List<MemberSession> roster = sessions.rosterResolved();
assertEquals(1, roster.size());
assertEquals("oc-session-1", roster.getFirst().agentSessionId(),
"fleet_list must see the id once the adapter can answer it");
Map<String, Object> view = SessionManager.rosterView(roster.getFirst(), null);
assertEquals("oc-session-1", view.get("agentSessionId"),
"rosterView renders whatever rosterResolved() resolved");
}
@Test
void getResolvesALateAgentSessionIdFromTheRetainedHandle() {
LazyIdHandle handle = new LazyIdHandle("p2", "t2", 1, "oc-session-2");
SessionManager sessions = new SessionManager(new LazyIdLauncher().queue(handle));
MemberSession acquired = sessions.acquire("lazy", "/cwd", "/caller", null);
MemberSession resolved = sessions.get(acquired.paneId()).orElseThrow();
assertEquals("oc-session-2", resolved.agentSessionId(),
"fleet_status (single-session lookup) must also see the late-resolved id");
}
@Test
void releaseCarriesALateResolvedAgentSessionIdIntoTheReleaseDetail() {
LazyIdHandle handle = new LazyIdHandle("p3", "t3", 1, "oc-session-3");
SessionManager sessions = new SessionManager(new LazyIdLauncher().queue(handle));
MemberSession acquired = sessions.acquire("lazy", "/cwd", "/caller", null);
java.util.List<String> released = new java.util.concurrent.CopyOnWriteArrayList<>();
sessions.onRelease(detail -> released.add(detail.agentSessionId()));
sessions.release(acquired.paneId());
assertEquals(java.util.List.of("oc-session-3"), released,
"a released member's detail carries the id it has since resolved, not the null "
+ "frozen in at spawn time");
}
@Test
void aThrowingHandleDoesNotBreakRosterResolved() {
LazyIdHandle handle = new LazyIdHandle("p4", "t4", 1, "oc-session-4")
.throwing(new RuntimeException("sqlite locked"));
SessionManager sessions = new SessionManager(new LazyIdLauncher().queue(handle));
sessions.acquire("lazy", "/cwd", "/caller", null);
List<MemberSession> roster = assertDoesNotThrow(sessions::rosterResolved,
"a handle that throws resolving its id must not break the roster read");
assertEquals(1, roster.size());
assertNull(roster.getFirst().agentSessionId(), "the id stays unresolved when the lookup throws");
}
@Test
void aResolvedAgentSessionIdIsNotLookedUpAgain() {
LazyIdHandle handle = new LazyIdHandle("p5", "t5", 1, "oc-session-5");
SessionManager sessions = new SessionManager(new LazyIdLauncher().queue(handle));
sessions.acquire("lazy", "/cwd", "/caller", null);
assertEquals(1, handle.callCount(), "sanity: only the spawn-time call happened so far");
sessions.rosterResolved();
assertEquals(2, handle.callCount(), "the first rosterResolved() read resolves the id");
sessions.rosterResolved();
assertEquals(2, handle.callCount(), "once resolved, the id must not be looked up again");
}
}
@@ -163,6 +163,37 @@ class WorktreeSessionManagerTest {
"tracked copied paths are --skip-worktree'd");
}
/**
* fleetd #185 stage 3, THE TRAP: {@code overlayParity} copies more files into the worktree
* AFTER {@code add} returns, so {@code shareWithGroup} must run after it, not folded into
* {@code add()} — otherwise every overlay file lands operator-owned and unwritable for a
* different-uid member, with a green test suite hiding it.
*/
@Test
void shareWithGroupRunsAfterOverlayParityNotBeforeIt() {
FakeHerdr herdr = new FakeHerdr();
FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt")
.track(".envrc");
SessionManager sessions = new SessionManager(workerService(herdr), worktrees);
MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
new WorktreeRequest("cb-185", null));
assertEquals(1, worktrees.overlayCalls().size(), "overlayParity ran exactly once");
assertEquals(1, worktrees.shareCalls().size(), "shareWithGroup ran exactly once");
FakeWorktrees.OverlayCall overlay = worktrees.lastOverlay();
FakeWorktrees.ShareCall share = worktrees.lastShare();
assertEquals(s.worktree(), overlay.worktreePath());
assertEquals(s.worktree(), share.worktreePath());
List<String> order = worktrees.overlayShareOrder();
int overlayIndex = order.indexOf("overlay:" + s.worktree());
int shareIndex = order.indexOf("share:" + s.worktree());
assertTrue(overlayIndex >= 0 && shareIndex >= 0, "both calls must be recorded: " + order);
assertTrue(overlayIndex < shareIndex,
"shareWithGroup MUST run after overlayParity, not before/inside add(): " + order);
}
@Test
void releaseRemovesWorktreeButDoesNotDeleteBranch() {
FakeHerdr herdr = new FakeHerdr();