Compare commits
23 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| e028a0ae54 | |||
| 2fa673d4c0 | |||
| 9020d01b40 | |||
| 1006805027 | |||
| 27aefbf9a0 | |||
| d42c2bc204 | |||
| 3916adc372 | |||
| fa97f598dd | |||
| ea9aa4fd77 | |||
| a2b8caf6b5 | |||
| b5ddbe5757 | |||
| d223a93039 | |||
| 96d8191149 | |||
| f0e7ac73d6 | |||
| e2fe861b4d | |||
| 279d6f5fbd | |||
| 34480cebef | |||
| 9d0bf14c46 | |||
| 5a3ab5764c | |||
| 3bad9f5785 | |||
| 8308c0b68f | |||
| 3b3063eb2b | |||
| df9086263d |
+20
-4
@@ -31,11 +31,27 @@ Environment=HERDR_SOCKET_PATH=%h/.config/herdr/herdr.sock
|
||||
# spawns, so this line decides whether the fleet can run a build at all. systemd does not source a
|
||||
# login shell, so without it the daemon — and every worker — gets a bare default with no JDK/Maven.
|
||||
Environment=PATH=/usr/lib/jvm/temurin-25-jdk/bin:/usr/share/maven/bin:/usr/local/bin:/usr/bin:/bin
|
||||
# Secrets are NOT set here — this file is committed. Put the API/worker tokens in a private
|
||||
# drop-in that systemd reads with restrictive permissions:
|
||||
# systemctl --user edit fleetd → [Service] / Environment=FLEETD_API_TOKEN=...
|
||||
# or point EnvironmentFile at a 0600 file:
|
||||
# Secrets are NOT set here — this file is committed. Put ALL three tokens in a private drop-in
|
||||
# that systemd reads with restrictive permissions. In `systemctl --user edit fleetd`, add:
|
||||
# [Service]
|
||||
# Environment=FLEETD_API_TOKEN=...
|
||||
# Environment=WORKER_GITEA_TOKEN=...
|
||||
# Environment=AI_GATEWAY_TOKEN=...
|
||||
# FLEETD_API_TOKEN protects fleetd's API. WORKER_GITEA_TOKEN lets members open pull requests; if
|
||||
# it is missing, fleetd still starts, but a member fails when it later tries to open a pull request.
|
||||
# AI_GATEWAY_TOKEN authenticates gateway profiles; if it is missing, fleetd still starts, but a
|
||||
# gateway profile later returns HTTP 401. Or, put the same three variables in a 0600 file and add:
|
||||
# EnvironmentFile=%h/.config/fleetd/env
|
||||
# After starting, check which of them actually resolved. The daemon reports every secret a
|
||||
# configured profile references, by name, never by value:
|
||||
# journalctl --user -u fleetd | grep 'startup secret'
|
||||
# A resolved one logs "startup secret NAME: set (profile 'x' tokenEnv)". A missing one logs
|
||||
# "startup secret NAME: MISSING" at WARN — and the daemon starts anyway, which is the whole
|
||||
# problem: without this grep the first sign is a member that cannot open a pull request, hours
|
||||
# later and in a different component.
|
||||
# Note what the report can and cannot tell you. It lists only names some profile actually
|
||||
# references (tokenEnv, gitTokenEnv, and the broker uriEnv). A secret nothing references is never
|
||||
# reported, because nothing needs it.
|
||||
|
||||
Restart=on-failure
|
||||
RestartSec=10s
|
||||
|
||||
+51
-29
@@ -307,16 +307,22 @@ profiles:
|
||||
# and no refusal on cost; the cap on live members is the single thing standing between a
|
||||
# fan-out and your monthly limit. Set it deliberately and keep it small.
|
||||
#
|
||||
# GOTCHA 3 (fleetd #176) — `maxLoad` counts members, never the lead itself. The lead is a live
|
||||
# `claude` session on this SAME account (a lead is never moved off-subscription, whatever its
|
||||
# own profile says), so it already holds one seat before any member spawns. If a lead's
|
||||
# `fleet.leaders.<name>.profile` names THIS profile — or ANY OTHER `subscription: true`
|
||||
# profile that shares this one's account (see THE SENTINEL, just below, next to
|
||||
# `credentialId:`) — `fleet_list`'s `free` for this profile subtracts that lead's live
|
||||
# seat(s) automatically; see `profile:` under THE FLEET below. If no lead entry names a
|
||||
# profile sharing this account, fleetd has no way to know a lead holds a seat here, and `free`
|
||||
# will overstate what a fresh `fleet_spawn` actually gets by exactly the seats the lead is
|
||||
# quietly holding.
|
||||
# GOTCHA 3 (fleetd #176, corrected by fleetd #257) — `maxLoad` counts members, never the lead
|
||||
# itself. The lead is a live `claude` session on this SAME account (a lead is never moved
|
||||
# off-subscription, whatever its own profile says), so it already holds one seat before any
|
||||
# member spawns. If a lead's `fleet.leaders.<name>.profile` names THIS profile — or ANY OTHER
|
||||
# `subscription: true` profile that shares this one's account (see THE SENTINEL, just below,
|
||||
# next to `credentialId:`) — `fleet_list` reports that seat count under `leadSeats`; see
|
||||
# `profile:` under THE FLEET below. `free` itself is NEVER reduced by `leadSeats`: `free` means
|
||||
# "what the real placement gate (`CompositePeerLauncher#enforceMaxLoad`) will actually grant a
|
||||
# fresh `fleet_spawn` right now", and that gate only ever compares live members against
|
||||
# `maxLoad` — it has no notion of the lead's own seat. An earlier cut of this feature
|
||||
# subtracted `leadSeats` from `free` on the theory it made `free` describe the true ceiling on
|
||||
# the account, but no backend seat ceiling shared with the lead has ever actually been
|
||||
# measured, and the subtraction just made `free` disagree with the one thing it is supposed to
|
||||
# describe — the fleetd #257 fix. `maxLoad: 3` means 3 member slots, full stop; a lead sharing
|
||||
# the account is a fact you can see in `leadSeats`, not a reason `free` undercounts spawns that
|
||||
# will, in practice, succeed.
|
||||
#
|
||||
# THE SENTINEL (fleetd #176 stage 2, correcting an inert stage 1 fix): every `subscription:
|
||||
# true` profile that leaves `credentialId` unset shares ONE implicit account-wide credential
|
||||
@@ -536,17 +542,18 @@ fleet:
|
||||
# auto-launched: it is also how fleetd learns which account this lead's own session shares. A
|
||||
# `subscription: true` profile bills the operator's Claude account, and the lead itself is always
|
||||
# a live `claude` session on that same account — `maxLoad` never counted that seat. If a lead
|
||||
# entry here names a profile that shares a worker profile's account, `fleet_list`'s `free` for
|
||||
# that worker profile subtracts the lead's live seat(s) automatically. "Shares the account" is
|
||||
# decided by matching `effectiveCredentialId()`, which (fleetd #176 stage 2 — see THE SENTINEL,
|
||||
# next to `credentialId:`, in THE WORKERS above) means: an explicit, matching `credentialId:` on
|
||||
# both, OR — the common case, needing NO extra config — both being `subscription: true` with
|
||||
# `credentialId` left unset, since those all share one implicit account-wide id. A lead on `opus`
|
||||
# and workers on `sonnet` link automatically this way; they do NOT need the same profile name.
|
||||
# Setting `profile:` on an already-running, recognise-only lead is safe — the daemon only launches
|
||||
# the SHORTFALL below `instances`, so naming a profile here does not, by itself, start anything.
|
||||
# Omit it and fleetd has no way to derive the sharing — there is no other reliable signal on the
|
||||
# daemon's side — so that lead's seat goes uncounted, exactly as before this ticket.
|
||||
# entry here names a profile that shares a worker profile's account, `fleet_list` reports the
|
||||
# lead's live seat(s) on that worker profile under `leadSeats` — informational only, as of fleetd
|
||||
# #257 it is NEVER subtracted from `free` (see GOTCHA 3, next to `maxLoad:`, in THE WORKERS above,
|
||||
# for why). "Shares the account" is decided by matching `effectiveCredentialId()`, which (fleetd
|
||||
# #176 stage 2 — see THE SENTINEL, next to `credentialId:`, in THE WORKERS above) means: an
|
||||
# explicit, matching `credentialId:` on both, OR — the common case, needing NO extra config — both
|
||||
# being `subscription: true` with `credentialId` left unset, since those all share one implicit
|
||||
# account-wide id. A lead on `opus` and workers on `sonnet` link automatically this way; they do
|
||||
# NOT need the same profile name. Setting `profile:` on an already-running, recognise-only lead is
|
||||
# safe — the daemon only launches the SHORTFALL below `instances`, so naming a profile here does
|
||||
# not, by itself, start anything. Omit it and fleetd has no way to derive the sharing — there is
|
||||
# no other reliable signal on the daemon's side — so that lead's seat never appears in `leadSeats`.
|
||||
#
|
||||
# `tab:` (CB-579) is REQUIRED and is the only field identity depends on — the exact label of the
|
||||
# tab hosting the lead, matched case-insensitively. Label the tab yourself and put that same
|
||||
@@ -669,20 +676,35 @@ guard:
|
||||
# every name here NOT also in `allow` is overlaid with a non-secret sentinel value before
|
||||
# the pane's login shell runs — real protection only for names that shell does not itself
|
||||
# re-export (see the ROUND-2 CORRECTION note above). Under allow-list: reporting only.
|
||||
# sshAuthSock → whether SSH_AUTH_SOCK may pass through under allow-list ("allow") or must be
|
||||
# blanked like any other non-derived name ("block", the default). This is a decision you
|
||||
# have to make explicitly: SSH_AUTH_SOCK is a handle to YOUR ssh-agent, and a member
|
||||
# holding it can sign with your keys — it sits in no secret file and looks like no
|
||||
# credential, which is why it slipped past three earlier tickets (gitea #110). Blocking
|
||||
# it breaks git over SSH inside members (push/fetch authenticate as you); use HTTPS
|
||||
# remotes or scoped deploy keys instead of allowing it lightly.
|
||||
# sshAgentEnv → whether SSH_AUTH_SOCK may pass through under allow-list ("inherit") or is omitted
|
||||
# from the member environment ("omit", the default). Omitting it only omits the
|
||||
# inherited ssh-agent path. It discourages automatic use of the operator's agent.
|
||||
# It does not deny same-user access to that socket. It also does not block SSH keys that
|
||||
# are readable on disk. Git over SSH may still work from inside a member. Keep the block:
|
||||
# it is correct and costs nothing, but it is not a control. A member runs as the same OS
|
||||
# user as the lead. Inside one uid, ordinary Unix permissions provide no meaningful
|
||||
# confidentiality boundary. A real boundary needs a different OS user or OS-level
|
||||
# confinement, such as a container or VM. That is the open question in fleetd #184.
|
||||
#
|
||||
# Still do not set this to "inherit" casually. SSH_AUTH_SOCK is a live handle to YOUR
|
||||
# ssh-agent, so a member holding it can sign with EVERY key the agent holds. It sits in
|
||||
# no secret file and looks like no credential, which is why it slipped past three
|
||||
# earlier tickets (gitea #110). Blocking it does not contain a member, but allowing it
|
||||
# hands one a signing capability for no gain — the block costs nothing, so keep it.
|
||||
#
|
||||
# Both halves of this are measured, not argued. 2026-08-28: a member with
|
||||
# SSH_AUTH_SOCK blanked pushed to the forge over SSH successfully, because `ssh -G`
|
||||
# resolves an IdentityFile outside ~/.ssh that is readable and has no passphrase. An
|
||||
# earlier version of this comment claimed blocking the socket BREAKS git over SSH. It
|
||||
# does not. That claim came from looking only in ~/.ssh, which holds nothing but four
|
||||
# `Include` lines — looking in one place and concluding about the whole host.
|
||||
#
|
||||
# HOT-RELOADABLE the same way `fleet:` is (CB-559): read fresh on every spawn, so editing this list
|
||||
# and reloading config (or restarting) changes what the NEXT spawn inherits; already-running members
|
||||
# are unaffected either way.
|
||||
# memberCredentials:
|
||||
# policy: deny-by-default # or "deny-list", or "allow-list" (CB-633) — see above
|
||||
# sshAuthSock: block # allow-list only; see the sshAuthSock note above
|
||||
# sshAgentEnv: omit # allow-list only; see the sshAgentEnv note above
|
||||
# allow:
|
||||
# - AI_GATEWAY_TOKEN # named in a profile's tokenEnv (local/gx) — a member reaching the
|
||||
# # gateway is by design, not a leak
|
||||
|
||||
@@ -29,6 +29,7 @@
|
||||
<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>
|
||||
<archunit.version>1.5.0</archunit.version>
|
||||
</properties>
|
||||
|
||||
<!--
|
||||
@@ -176,6 +177,14 @@
|
||||
<version>${testcontainers.version}</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
|
||||
<!-- fleetd #131: package-boundary and cycle enforcement (PackageCyclesTest). -->
|
||||
<dependency>
|
||||
<groupId>com.tngtech.archunit</groupId>
|
||||
<artifactId>archunit-junit5</artifactId>
|
||||
<version>${archunit.version}</version>
|
||||
<scope>test</scope>
|
||||
</dependency>
|
||||
</dependencies>
|
||||
|
||||
<build>
|
||||
|
||||
@@ -121,6 +121,7 @@ public final class Fleetd {
|
||||
// else can fail on a silently-empty one. A daemon started without a login shell (launchd)
|
||||
// boots fine either way — this is the only thing that says so out loud.
|
||||
reportRequiredSecrets(cfg);
|
||||
reportMemberTrustModel(cfg);
|
||||
// CB-596: an absent (or empty) memberCredentials: block blocks NOTHING — no credential
|
||||
// name is hardcoded any more to fall back on. Say so loudly, the same way a missing
|
||||
// secret is reported above, so upgrading past this commit never silently drops CB-592's
|
||||
@@ -1136,6 +1137,24 @@ public final class Fleetd {
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #184: state the member trust model at startup. Environment controls and worktrees do
|
||||
* not make a sandbox when fleetd and its members use the same OS user. A separate herdr may
|
||||
* provide that boundary, but fleetd cannot inspect the uid at the other end of its socket.
|
||||
*/
|
||||
static void reportMemberTrustModel(FleetConfig cfg) {
|
||||
if (cfg.memberHerdrSocket() != null && !cfg.memberHerdrSocket().isBlank()) {
|
||||
log.info("member trust model: members are routed to a separate herdr through "
|
||||
+ "memberHerdrSocket. fleetd cannot see that herdr's uid, so confirm it runs "
|
||||
+ "as a different OS user before treating it as a boundary.");
|
||||
return;
|
||||
}
|
||||
log.info("member trust model: members run as the same OS user as fleetd, not in a sandbox. "
|
||||
+ "A member can read any file this user can read, including SSH keys and credential "
|
||||
+ "stores, whatever memberCredentials says. To add a real boundary, route members to "
|
||||
+ "a second herdr under a different OS user with memberHerdrSocket.");
|
||||
}
|
||||
|
||||
/**
|
||||
* CB-596: {@code known:} empty (block absent entirely, or present but empty) means {@link
|
||||
* FleetConfig.MemberCredentials#blockedSet()} is empty too — every member pane inherits the
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
package dev.ltms.fleet.config;
|
||||
|
||||
import com.fasterxml.jackson.annotation.JsonCreator;
|
||||
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
|
||||
import com.fasterxml.jackson.annotation.JsonProperty;
|
||||
import com.fasterxml.jackson.core.JsonParser;
|
||||
import com.fasterxml.jackson.core.JsonToken;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
@@ -1357,19 +1359,19 @@ public record FleetConfig(
|
||||
* deny-list/deny-by-default every name here that is NOT also in {@link #allow} is
|
||||
* overlaid with a non-secret sentinel value. Under allow-list this list is
|
||||
* reporting only.
|
||||
* @param sshAuthSock whether the member may inherit {@code SSH_AUTH_SOCK} under the allow-list
|
||||
* policy ({@code "allow"}) or must have it blanked ({@code "block"}, the default).
|
||||
* @param sshAgentEnv whether the member may inherit {@code SSH_AUTH_SOCK} under the allow-list
|
||||
* policy ({@code "inherit"}) or must have it omitted ({@code "omit"}, the default).
|
||||
* This is a DECISION, never a default: {@code SSH_AUTH_SOCK} is a handle to the
|
||||
* operator's ssh-agent, and a member holding it can sign with the operator's own
|
||||
* keys — but it appears in no secret file and is credential-shaped like nothing on
|
||||
* any list, which is why three earlier tickets missed it (gitea #110 / CB-607).
|
||||
* Blocking it breaks git over SSH inside the member; allow it only when members do
|
||||
* Omitting it does not prevent git over SSH inside the member; inherit it only when members do
|
||||
* not need to authenticate as the operator over SSH. Ignored under deny-list /
|
||||
* deny-by-default, which never touch the name.
|
||||
*/
|
||||
@JsonIgnoreProperties(ignoreUnknown = true)
|
||||
public record MemberCredentials(String policy, List<String> allow, List<String> known,
|
||||
String sshAuthSock) {
|
||||
String sshAgentEnv) {
|
||||
|
||||
/** Default policy: block every {@code known} name not in {@code allow}, via the env overlay. */
|
||||
public static final String POLICY_DENY_BY_DEFAULT = "deny-by-default";
|
||||
@@ -1386,11 +1388,25 @@ public record FleetConfig(
|
||||
*/
|
||||
public static final String POLICY_ALLOW_LIST = "allow-list";
|
||||
|
||||
/** The pre-CB-633 three-field form — {@code sshAuthSock} defaults to blocked. */
|
||||
/** The pre-CB-633 three-field form — {@code sshAgentEnv} defaults to omitted. */
|
||||
public MemberCredentials(String policy, List<String> allow, List<String> known) {
|
||||
this(policy, allow, known, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads both the current {@code sshAgentEnv} key and the compatible {@code sshAuthSock} key.
|
||||
* When both keys are present, {@code sshAgentEnv} wins, even if its value is unrecognised.
|
||||
*/
|
||||
@JsonCreator
|
||||
public static MemberCredentials fromYaml(@JsonProperty("policy") String policy,
|
||||
@JsonProperty("allow") List<String> allow,
|
||||
@JsonProperty("known") List<String> known,
|
||||
@JsonProperty("sshAgentEnv") String sshAgentEnv,
|
||||
@JsonProperty("sshAuthSock") String sshAuthSock) {
|
||||
return new MemberCredentials(policy, allow, known,
|
||||
sshAgentEnv != null ? sshAgentEnv : sshAuthSock);
|
||||
}
|
||||
|
||||
public MemberCredentials {
|
||||
String normalizedPolicy = (policy == null || policy.isBlank())
|
||||
? POLICY_DENY_BY_DEFAULT : policy.toLowerCase(java.util.Locale.ROOT);
|
||||
@@ -1399,8 +1415,9 @@ public record FleetConfig(
|
||||
policy = POLICY_DENY_LIST.equals(normalizedPolicy) ? POLICY_DENY_BY_DEFAULT : normalizedPolicy;
|
||||
allow = allow == null ? List.of() : List.copyOf(allow);
|
||||
known = known == null ? List.of() : List.copyOf(known);
|
||||
sshAuthSock = (sshAuthSock != null && "allow".equalsIgnoreCase(sshAuthSock.trim()))
|
||||
? "allow" : "block";
|
||||
sshAgentEnv = (sshAgentEnv != null
|
||||
&& ("inherit".equalsIgnoreCase(sshAgentEnv.trim())
|
||||
|| "allow".equalsIgnoreCase(sshAgentEnv.trim()))) ? "inherit" : "omit";
|
||||
}
|
||||
|
||||
/** True when this block selects the CB-633 derived-allow-list policy. */
|
||||
@@ -1409,8 +1426,8 @@ public record FleetConfig(
|
||||
}
|
||||
|
||||
/** True when {@code SSH_AUTH_SOCK} may pass through under the allow-list policy. Default: no. */
|
||||
public boolean sshAuthSockAllowed() {
|
||||
return "allow".equals(sshAuthSock);
|
||||
public boolean sshAgentEnvInherited() {
|
||||
return "inherit".equals(sshAgentEnv);
|
||||
}
|
||||
|
||||
/** {@link #allow} as a set, for membership checks. */
|
||||
|
||||
@@ -139,16 +139,24 @@ public final class FleetMcp {
|
||||
|
||||
/**
|
||||
* fleetd #176: the seats a profile's own live LEAD session(s) hold on the same Claude
|
||||
* subscription — the third reason (alongside {@link QuarantineSource} and {@link OutageSource})
|
||||
* {@code free} can overstate what a fresh {@code fleet_spawn} would actually get.
|
||||
* subscription — a fact {@code fleet_list} reports alongside {@code free} via the
|
||||
* {@code leadSeats} key.
|
||||
*
|
||||
* <p>{@code maxLoad} counts only <em>members</em>, never the lead itself. But a
|
||||
* {@code subscription: true} profile bills the operator's own Claude account, and the lead is
|
||||
* always a live {@code claude} session on that same account (it is never moved off-subscription
|
||||
* — see {@code LeadLauncher}). So a fan-out that fills every member slot still leaves the lead's
|
||||
* own seat unaccounted for, and the daemon reports a slot that was never really free. See
|
||||
* {@code Fleetd.leadSeatLookup} for how the count is derived — from {@code fleet.leaders.<name>
|
||||
* .profile} and each profile's {@code effectiveCredentialId()}, never a hardcoded constant.
|
||||
* — see {@code LeadLauncher}). See {@code Fleetd.leadSeatLookup} for how the count is derived —
|
||||
* from {@code fleet.leaders.<name>.profile} and each profile's {@code effectiveCredentialId()},
|
||||
* never a hardcoded constant.
|
||||
*
|
||||
* <p>fleetd #257: this count is reported, never subtracted from {@code free}. An earlier cut of
|
||||
* this feature subtracted it, on the theory that it made {@code free} describe the real ceiling
|
||||
* on the account — but the real spawn gate ({@code CompositePeerLauncher#enforceMaxLoad}) never
|
||||
* read this count at all, so the subtraction made {@code free} disagree with the one thing it is
|
||||
* supposed to describe: what a fresh {@code fleet_spawn} will actually get. No backend seat
|
||||
* ceiling shared with the lead has been measured either — see {@code fleetd.example.yaml}'s
|
||||
* {@code maxLoad} docs. {@code free} now always equals {@code max(0, maxLoad - live)}, and
|
||||
* {@code leadSeats} is reported purely as a fact the caller may act on however it likes.
|
||||
*/
|
||||
public record LeadSeatSource(Function<String, Integer> seatsFor) {
|
||||
/** Inert source — no profile is ever reported as sharing a seat with a lead. */
|
||||
@@ -1137,10 +1145,17 @@ public final class FleetMcp {
|
||||
* <p>fleetd #176: {@code maxLoad} counts panes, not subscription seats — it never counted the
|
||||
* lead's own seat on a {@code subscription: true} profile's account. {@link LeadSeatSource}
|
||||
* reports that count (0 for a non-subscription profile, or when no live lead shares its
|
||||
* credential), and it is subtracted from {@code free} the same way {@code live} already is —
|
||||
* {@code maxLoad} itself is left untouched, so the row still reports the configured cap. The
|
||||
* {@code leadSeats} key is added only when the count is positive, for the same
|
||||
* byte-identical-when-unused reason as the quarantine/cool-off keys above.
|
||||
* credential) via the {@code leadSeats} key, added only when the count is positive, for the
|
||||
* same byte-identical-when-unused reason as the quarantine/cool-off keys above.
|
||||
*
|
||||
* <p>fleetd #257: {@code leadSeatCount} is reported, never subtracted from {@code free}.
|
||||
* {@code free} means "what a fresh {@code fleet_spawn} on this profile will actually get", and
|
||||
* the real gate ({@code CompositePeerLauncher#enforceMaxLoad}) only ever compares {@code live}
|
||||
* against {@code maxLoad} — it has no notion of a lead's own seat. Subtracting
|
||||
* {@code leadSeatCount} here made {@code free} disagree with the gate it is supposed to
|
||||
* describe: it could report {@code free: 0} while a spawn on that exact profile still
|
||||
* succeeded. {@code leadSeats} stays in the row as a fact the caller can act on however it
|
||||
* likes, but it no longer changes what {@code free} means.
|
||||
*/
|
||||
private static Map<String, Object> capacityView(String profile, Function<String, Integer> liveCount,
|
||||
Function<String, Integer> maxLoad, List<MemberSession> roster,
|
||||
@@ -1155,7 +1170,12 @@ public final class FleetMcp {
|
||||
.count();
|
||||
Map<String, Object> row = new LinkedHashMap<>();
|
||||
row.put("profile", profile); row.put("maxLoad", cap); row.put("live", live);
|
||||
row.put("free", cap == null ? null : Math.max(0, cap - live - leadSeatCount));
|
||||
// fleetd #257: free must report what the real spawn gate (CompositePeerLauncher#enforceMaxLoad)
|
||||
// will actually grant, and that gate never reads leadSeatCount — only maxLoad and live. Not
|
||||
// subtracting the lead's seat here used to make free UNDERSTATE what a fresh fleet_spawn would
|
||||
// get, so a lead believing free:0 gave up on a profile the gate would still spawn onto.
|
||||
// leadSeatCount is still reported via the leadSeats key below, just never subtracted from free.
|
||||
row.put("free", cap == null ? null : Math.max(0, cap - live));
|
||||
row.put("reclaimable", reclaimable);
|
||||
if (leadSeatCount > 0) {
|
||||
row.put("leadSeats", leadSeatCount);
|
||||
@@ -1360,8 +1380,13 @@ public final class FleetMcp {
|
||||
+ "member spawned without one shares its directory with others and never reports "
|
||||
+ "an id, however long it runs (fleetd #249). An empty 'members' "
|
||||
+ "means no members are spawned; it says nothing about peers. When capacity "
|
||||
+ "facts are configured, a 'capacity' row per profile also reports free: 0 for "
|
||||
+ "a quarantined profile's credential (see fleet_profiles), whatever its "
|
||||
+ "facts are configured, a 'capacity' row per profile reports 'free' — the "
|
||||
+ "slots a fresh fleet_spawn on that profile will actually be granted right "
|
||||
+ "now (max(0, maxLoad - live)), the same check the spawn gate itself runs. A "
|
||||
+ "'leadSeats' key, when present, reports how many of those live slots are a "
|
||||
+ "lead session sharing this profile's subscription — informational only, "
|
||||
+ "already NOT subtracted from 'free' (fleetd #257). It also reports free: 0 "
|
||||
+ "for a quarantined profile's credential (see fleet_profiles), whatever its "
|
||||
+ "maxLoad/live — with credentialId and quarantinedForSeconds naming the "
|
||||
+ "quarantine, so 'free: 0, busy' can be told apart from 'free: 0, refusing "
|
||||
+ "for N seconds'.",
|
||||
|
||||
@@ -19,6 +19,7 @@ import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.StandardCopyOption;
|
||||
import java.nio.file.attribute.PosixFileAttributeView;
|
||||
import java.util.Arrays;
|
||||
import java.util.EnumSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@@ -519,6 +520,26 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher {
|
||||
* the first. Both exist because of a real incident: see {@link HerdrPeerLauncher#isProvisionedWorktree}'s javadoc
|
||||
* and {@link #writeAtomically}'s javadoc.
|
||||
*
|
||||
* <p><b>Compare-and-swap against a writer the lock cannot reach (fleetd #247).</b>
|
||||
* {@code TRUST_JSON_LOCK} only serialises calls this launcher itself makes inside this one JVM.
|
||||
* It does nothing about a writer outside it — and on a host where the profile's
|
||||
* {@code configDir} is the operator's own {@code CLAUDE_CONFIG_DIR}, the file this method writes
|
||||
* IS the operator's own live Claude Code session's config file, being read and written by that
|
||||
* session while it runs. Measured 2026-09-03: its mtime moved minutes after a spawn while that
|
||||
* session was active. A plain read-modify-write there is a routine lost update, not a rare one:
|
||||
* fleetd reads v1, the operator's session reads v1 and writes v2 with their own change, fleetd's
|
||||
* {@code ATOMIC_MOVE} then lands v3 built from v1 — atomic, but v2's change is gone. So before
|
||||
* the move this method re-reads {@code target}'s exact bytes and compares them with the bytes it
|
||||
* built its update from; a mismatch means someone else wrote in between, and it discards its
|
||||
* work and rebuilds from the fresh bytes, up to {@link #MAX_TRUST_JSON_CAS_ATTEMPTS} times.
|
||||
* <b>Exhausting the retries writes nothing</b> — see the WARN at the end of the loop for why
|
||||
* that, not a last write-anyway, is the safe failure: the member shows the trust dialog and
|
||||
* fails to reach an injectable state, which is visible, logged and recoverable; overwriting the
|
||||
* operator's live config with a stale copy is neither. This narrows the lost-update window, it
|
||||
* does not close it — a write landing between the final re-read and the {@code ATOMIC_MOVE}
|
||||
* itself is still lost, because there is no OS-level compare-and-swap on a plain file, only this
|
||||
* cooperative narrowing of the gap.
|
||||
*
|
||||
* @param configDir the profile's {@code CLAUDE_CONFIG_DIR} ({@code cfg.configDir()}), or
|
||||
* {@code null}/blank to target the default {@code ~/.claude.json}
|
||||
* @param cwd the spawn's resolved working directory — the exact key Claude Code will look
|
||||
@@ -528,55 +549,118 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher {
|
||||
if (!isProvisionedWorktree(cwd)) {
|
||||
return;
|
||||
}
|
||||
Path target = (configDir == null || configDir.isBlank())
|
||||
boolean unsetConfigDir = configDir == null || configDir.isBlank();
|
||||
Path target = unsetConfigDir
|
||||
? Path.of(System.getProperty("user.home"), ".claude.json")
|
||||
: Path.of(configDir, ".claude.json");
|
||||
if (unsetConfigDir) {
|
||||
// fleetd #247: configDir unset is the ONLY path that targets ~/.claude.json — the
|
||||
// operator's own home file, not a per-profile one — and it is the default, so a
|
||||
// profile that simply forgot to set configDir gets no signal at all short of the
|
||||
// operator noticing their own file changing. Say so loudly, every time it is about to
|
||||
// happen, rather than only once ever: each occurrence is a live write to a real
|
||||
// person's home config and deserves its own log line.
|
||||
log.warn("seedTrustDialog: profile has no configDir set, so the workspace-trust seed "
|
||||
+ "for cwd '{}' is about to write the operator's own default '{}' — set "
|
||||
+ "configDir on this profile to target a per-member config file instead",
|
||||
cwd, target);
|
||||
}
|
||||
synchronized (TRUST_JSON_LOCK) {
|
||||
try {
|
||||
if (target.getParent() != null) {
|
||||
Files.createDirectories(target.getParent());
|
||||
}
|
||||
ObjectNode root = null;
|
||||
if (Files.isRegularFile(target)) {
|
||||
JsonNode existing = TRUST_JSON.readTree(target.toFile());
|
||||
if (existing instanceof ObjectNode existingObject) {
|
||||
root = existingObject;
|
||||
for (int attempt = 1; attempt <= MAX_TRUST_JSON_CAS_ATTEMPTS; attempt++) {
|
||||
byte[] before = Files.isRegularFile(target) ? Files.readAllBytes(target) : null;
|
||||
ObjectNode root = parseTrustJsonOrEmpty(before);
|
||||
JsonNode projectsNode = root.get("projects");
|
||||
ObjectNode projects = projectsNode instanceof ObjectNode projectsObject
|
||||
? projectsObject : TRUST_JSON.createObjectNode();
|
||||
if (!(projectsNode instanceof ObjectNode)) {
|
||||
root.set("projects", projects);
|
||||
}
|
||||
JsonNode projectNode = projects.get(cwd);
|
||||
ObjectNode project = projectNode instanceof ObjectNode projectObject
|
||||
? projectObject : TRUST_JSON.createObjectNode();
|
||||
if (!(projectNode instanceof ObjectNode)) {
|
||||
projects.set(cwd, project);
|
||||
}
|
||||
// fleetd #247: ONLY hasTrustDialogAccepted. We used to write
|
||||
// hasCompletedProjectOnboarding beside it; do not put it back. Measured on
|
||||
// 2026-09-03, minutes after a live spawn seeded this file: 28 of 28 project
|
||||
// entries carried hasTrustDialogAccepted and 0 of 28 carried the onboarding key
|
||||
// — including the 27 entries Claude Code wrote for itself. Claude Code
|
||||
// normalises the whole file when it saves and drops that key every time, so
|
||||
// writing it achieved nothing except making the next reader think it mattered.
|
||||
// The member reached idle with the trust flag alone, which is the only outcome
|
||||
// this seed exists for. If a future Claude Code needs the second flag the
|
||||
// symptom returns as the trust dialog fleetd #149 describes — re-measure then,
|
||||
// do not restore it on a guess.
|
||||
project.put("hasTrustDialogAccepted", true);
|
||||
String newContent = TRUST_JSON.writerWithDefaultPrettyPrinter().writeValueAsString(root);
|
||||
|
||||
trustJsonCasTestHook.run();
|
||||
|
||||
// fleetd #247 CAS: re-read immediately before the move and compare with what
|
||||
// this attempt built its update from. A mismatch means another writer (most
|
||||
// plausibly the operator's own live Claude Code — see this method's javadoc)
|
||||
// landed a change in between; discard this attempt's work and rebuild from the
|
||||
// fresh bytes rather than blindly overwriting it.
|
||||
byte[] atMove = Files.isRegularFile(target) ? Files.readAllBytes(target) : null;
|
||||
if (!Arrays.equals(before, atMove)) {
|
||||
continue;
|
||||
}
|
||||
writeAtomically(target, newContent);
|
||||
return;
|
||||
}
|
||||
if (root == null) {
|
||||
root = TRUST_JSON.createObjectNode();
|
||||
}
|
||||
JsonNode projectsNode = root.get("projects");
|
||||
ObjectNode projects = projectsNode instanceof ObjectNode projectsObject
|
||||
? projectsObject : TRUST_JSON.createObjectNode();
|
||||
if (!(projectsNode instanceof ObjectNode)) {
|
||||
root.set("projects", projects);
|
||||
}
|
||||
JsonNode projectNode = projects.get(cwd);
|
||||
ObjectNode project = projectNode instanceof ObjectNode projectObject
|
||||
? projectObject : TRUST_JSON.createObjectNode();
|
||||
if (!(projectNode instanceof ObjectNode)) {
|
||||
projects.set(cwd, project);
|
||||
}
|
||||
// fleetd #247: ONLY hasTrustDialogAccepted. We used to write
|
||||
// hasCompletedProjectOnboarding beside it; do not put it back. Measured on
|
||||
// 2026-09-03, minutes after a live spawn seeded this file: 28 of 28 project
|
||||
// entries carried hasTrustDialogAccepted and 0 of 28 carried the onboarding key
|
||||
// — including the 27 entries Claude Code wrote for itself. Claude Code
|
||||
// normalises the whole file when it saves and drops that key every time, so
|
||||
// writing it achieved nothing except making the next reader think it mattered.
|
||||
// The member reached idle with the trust flag alone, which is the only outcome
|
||||
// this seed exists for. If a future Claude Code needs the second flag the
|
||||
// symptom returns as the trust dialog fleetd #149 describes — re-measure then,
|
||||
// do not restore it on a guess.
|
||||
project.put("hasTrustDialogAccepted", true);
|
||||
writeAtomically(target, TRUST_JSON.writerWithDefaultPrettyPrinter().writeValueAsString(root));
|
||||
// fleetd #247: deliberately do NOT write here. A member that starts without the
|
||||
// seed still starts — it may hit the trust dialog fleetd #149 describes and fail to
|
||||
// reach an injectable state, but that failure is visible (herdr reports it, the
|
||||
// spawn-readiness gate times out) and recoverable (retry the spawn). Writing our
|
||||
// stale copy over whatever the other writer left would be silent and, if that other
|
||||
// writer is the operator's own live session, could destroy real configuration —
|
||||
// fail toward the recoverable outcome, not the silent one.
|
||||
log.warn("seedTrustDialog: gave up seeding workspace-trust for cwd '{}' into '{}' "
|
||||
+ "after {} attempts — another writer (most plausibly the operator's own "
|
||||
+ "live Claude Code sharing this file) kept changing it faster than we could "
|
||||
+ "re-read it, so nothing was written; the member may show the trust dialog "
|
||||
+ "instead", cwd, target, MAX_TRUST_JSON_CAS_ATTEMPTS);
|
||||
} catch (Exception e) {
|
||||
log.debug("cannot seed workspace-trust entry for cwd '{}' into '{}'", cwd, target, e);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Bound on {@link #seedTrustDialog}'s fleetd #247 compare-and-swap retry loop. */
|
||||
private static final int MAX_TRUST_JSON_CAS_ATTEMPTS = 5;
|
||||
|
||||
/**
|
||||
* Test-only seam for fleetd #247: invoked once per CAS attempt inside {@link #seedTrustDialog}'s
|
||||
* retry loop, after that attempt has read the target's bytes and built its replacement content,
|
||||
* but immediately before the final re-read/compare that decides whether to write. A no-op in
|
||||
* production. Package-visible (not {@code private}) so {@code ClaudeCodeLauncherTest} can install
|
||||
* a hook here that writes to the target file, deterministically simulating a writer racing
|
||||
* fleetd's own read-modify-write at the exact instant the CAS is meant to catch — the same race
|
||||
* an external process (most plausibly the operator's own live Claude Code) creates, without
|
||||
* depending on real thread scheduling to land the interleaving. A test that sets this MUST
|
||||
* restore it to the no-op default in a {@code finally} block — it is shared, static state.
|
||||
*/
|
||||
static Runnable trustJsonCasTestHook = () -> {};
|
||||
|
||||
/**
|
||||
* Parse {@code bytes} as a {@code .claude.json} tree, or hand back a fresh empty object when
|
||||
* {@code bytes} is {@code null} (no file yet) or does not parse to a JSON object — the same
|
||||
* missing-or-unreadable-is-empty fallback {@link #seedTrustDialog} always used, factored out so
|
||||
* the fleetd #247 CAS loop can call it once per attempt.
|
||||
*/
|
||||
private static ObjectNode parseTrustJsonOrEmpty(byte[] bytes) throws IOException {
|
||||
if (bytes == null) {
|
||||
return TRUST_JSON.createObjectNode();
|
||||
}
|
||||
JsonNode existing = TRUST_JSON.readTree(bytes);
|
||||
return existing instanceof ObjectNode existingObject ? existingObject : TRUST_JSON.createObjectNode();
|
||||
}
|
||||
|
||||
/**
|
||||
* Write {@code content} to {@code target} atomically: serialise to a sibling temp file in the
|
||||
* <strong>same directory</strong> as {@code target} (an atomic move is only guaranteed within
|
||||
|
||||
@@ -117,9 +117,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
*
|
||||
* <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.
|
||||
* are routed to a second herdr, and fleetd has no channel to confirm what OS user that herdr
|
||||
* runs as — it may be a different user with a different {@code $HOME} and {@code secrets.sh},
|
||||
* or the same one the daemon runs as. Either way this field's data can no longer be trusted to
|
||||
* describe what a member pane inherits. See {@link #logCredentialGap} for how that mode is
|
||||
* handled.
|
||||
*/
|
||||
private final Supplier<Set<String>> hostEnvNames;
|
||||
|
||||
@@ -1471,9 +1473,9 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
Set<String> brokerUriEnvNames = brokerUriEnvNames();
|
||||
Set<String> allowed = new java.util.TreeSet<>(
|
||||
MemberEnvAllowList.derive(profiles.values(), creds.allowSet(), brokerUriEnvNames));
|
||||
if (creds.sshAuthSockAllowed()) {
|
||||
if (creds.sshAgentEnvInherited()) {
|
||||
allowed.add(SSH_AUTH_SOCK);
|
||||
} // blocked by default: absent from the set ⇒ blanked by the scrub like any other name
|
||||
} // omitted by default: absent from the set ⇒ blanked by the scrub like any other name
|
||||
allowed.addAll(launch.env().keySet());
|
||||
allowed.removeAll(brokerUriEnvNames);
|
||||
return allowed;
|
||||
@@ -1533,21 +1535,24 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
/**
|
||||
* 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).
|
||||
* be placed anywhere fleetd can be sure the member's OS user can reach — {@code java.io.tmpdir}
|
||||
* is fleetd's own 0700 temp dir, which is unreadable if the member pane runs as a different OS
|
||||
* user, and fleetd has no channel to confirm whether it does or not. Rather than gamble on that,
|
||||
* this treats memberHerdrSocket as reason enough to require an explicitly shared location, which
|
||||
* is the exact gap this ticket exists to close. Say so once per launcher instance, instead of
|
||||
* either generating a directory that might not be readable (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.");
|
||||
+ "configured — the generated ZDOTDIR cannot be placed where fleetd can be sure "
|
||||
+ "the member's OS user can read it (java.io.tmpdir is fleetd's own 0700 dir, "
|
||||
+ "unreadable if the member runs as a different OS user — fleetd has no channel "
|
||||
+ "to confirm whether it does), 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.");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1626,10 +1631,12 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
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.
|
||||
* fleetd #185 stage 2: whether {@code memberHerdrSocket:} is configured, i.e. member panes are
|
||||
* routed to a second herdr. This tests only that the config key is set — fleetd has no channel
|
||||
* to confirm what OS user that second herdr runs as, so a {@code true} result means "member
|
||||
* panes may run under a different OS user," not that they do. 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) —
|
||||
@@ -1652,14 +1659,15 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
* 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.
|
||||
* derived set) describes the DAEMON's own environment; under this config key member panes are
|
||||
* routed to a second herdr, and fleetd has no channel to confirm what OS user that herdr runs
|
||||
* as or to read its environment, 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)) {
|
||||
@@ -1672,13 +1680,13 @@ public abstract class HerdrPeerLauncher implements PeerLauncher {
|
||||
.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.",
|
||||
log.warn("memberCredentials gap: memberHerdrSocket is configured, so member panes are routed "
|
||||
+ "to a second herdr — fleetd has no channel to confirm what OS user that herdr "
|
||||
+ "runs as, so it cannot tell whether those panes inherit its own environment or "
|
||||
+ "a different one entirely. {} 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());
|
||||
}
|
||||
|
||||
|
||||
@@ -34,7 +34,7 @@ import java.util.TreeSet;
|
||||
*
|
||||
* <p>{@code SSH_AUTH_SOCK} is deliberately NOT here. It is a handle to the operator's ssh-agent — a
|
||||
* member holding it can sign with the operator's keys — so keeping it is a config decision
|
||||
* ({@code memberCredentials.sshAuthSock: allow}), not a derivation default.
|
||||
* ({@code memberCredentials.sshAgentEnv: inherit}), not a derivation default.
|
||||
*
|
||||
* <p><b>CB-633 follow-up:</b> the union also includes {@code memberCredentials.allow:} — the
|
||||
* operator's own explicit list. Before this, {@code policy: allow-list} silently ignored every name
|
||||
@@ -42,7 +42,7 @@ import java.util.TreeSet;
|
||||
* turning the policy on could blank credentials working members already depended on. {@code
|
||||
* 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
|
||||
* by the caller when {@code sshAgentEnv: inherit} 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
|
||||
@@ -53,7 +53,7 @@ public final class MemberEnvAllowList {
|
||||
/**
|
||||
* The operator's ssh-agent socket path. Deliberately excluded from {@link #derive}'s union of
|
||||
* {@code memberCredentials.allow:} — see the class javadoc's CB-633 follow-up note. Governed
|
||||
* ONLY by {@code memberCredentials.sshAuthSock}, never by appearing in {@code allow:}.
|
||||
* ONLY by {@code memberCredentials.sshAgentEnv}, never by appearing in {@code allow:}.
|
||||
*/
|
||||
public static final String SSH_AUTH_SOCK = "SSH_AUTH_SOCK";
|
||||
|
||||
|
||||
@@ -21,6 +21,7 @@ import java.util.EnumSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.function.BooleanSupplier;
|
||||
@@ -669,6 +670,16 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
|
||||
private final AtomicBoolean discoveryUnavailableWarned =
|
||||
new AtomicBoolean();
|
||||
|
||||
/**
|
||||
* fleetd #267: one WARN per PROFILE (not per launcher instance — several profiles can each hit
|
||||
* this gap independently) for the model-mismatch check (fleetd #175) never getting to run
|
||||
* because the spawn was not given a fleetd-provisioned worktree (fleetd #249). Profile names
|
||||
* accumulate here for the life of this launcher instance and are never removed — the same
|
||||
* one-shot treatment {@link #discoveryUnavailableWarned} already gets, just keyed per profile
|
||||
* instead of globally.
|
||||
*/
|
||||
private final Set<String> modelCheckSkippedWarned = ConcurrentHashMap.newKeySet();
|
||||
|
||||
/** Add lazy on-disk session discovery to the base handle. */
|
||||
@Override
|
||||
public PeerHandle spawn(SpawnRequest req) {
|
||||
@@ -695,7 +706,8 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
|
||||
// be running.
|
||||
FleetConfig.Profile cfg = requireProfile(req.profileName());
|
||||
return new SessionAwareHandle(inner, discovery, cwd, cfg,
|
||||
this::memberHerdrSocketConfigured, discoveryUnavailableWarned, exhaustionSink);
|
||||
this::memberHerdrSocketConfigured, discoveryUnavailableWarned,
|
||||
modelCheckSkippedWarned, exhaustionSink);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -720,6 +732,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
|
||||
private final FleetConfig.Profile cfg;
|
||||
private final BooleanSupplier discoveryUnavailable;
|
||||
private final AtomicBoolean discoveryUnavailableWarned;
|
||||
private final Set<String> modelCheckSkippedWarned;
|
||||
private final ExhaustionSink exhaustionSink;
|
||||
/** CAS'd true the first (and only) time a model mismatch is reported for this handle. */
|
||||
private final AtomicBoolean modelMismatchReported = new AtomicBoolean();
|
||||
@@ -750,6 +763,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
|
||||
FleetConfig.Profile cfg,
|
||||
BooleanSupplier discoveryUnavailable,
|
||||
AtomicBoolean discoveryUnavailableWarned,
|
||||
Set<String> modelCheckSkippedWarned,
|
||||
ExhaustionSink exhaustionSink) {
|
||||
this.delegate = delegate;
|
||||
this.discovery = discovery;
|
||||
@@ -757,6 +771,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
|
||||
this.cfg = cfg;
|
||||
this.discoveryUnavailable = discoveryUnavailable;
|
||||
this.discoveryUnavailableWarned = discoveryUnavailableWarned;
|
||||
this.modelCheckSkippedWarned = modelCheckSkippedWarned;
|
||||
this.exhaustionSink = exhaustionSink;
|
||||
this.worktreeProvisioned = isProvisionedWorktree(cwd);
|
||||
}
|
||||
@@ -807,9 +822,29 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher {
|
||||
// member's row apart from a sibling's in that case (measured: a three-day-old row from
|
||||
// a different profile). Refuse to guess — absent is the honest answer, and it is what
|
||||
// this codebase already returns elsewhere for absent evidence (fleetd #175's UNKNOWN).
|
||||
// No WARN here: unlike discoveryUnavailable above, this is the ordinary, expected shape
|
||||
// of the large majority of spawns (no worktree requested), not a configuration gap.
|
||||
// This IS the ordinary, expected shape of the large majority of spawns (no worktree
|
||||
// requested), not a configuration gap — but fleetd #267 found that same shape silently
|
||||
// switches off the fleetd #175 model-mismatch check for those spawns too, since
|
||||
// checkModelMatch's only call site is right below this gate. The check cannot be moved
|
||||
// off agentSessionId()'s resolved id: the id is the only safe way to key
|
||||
// actualModelForSessionId to THIS session's own row rather than "whatever is newest in
|
||||
// the shared directory" (fleetd #234) — re-deriving a second, independent answer via
|
||||
// `directory` here would reintroduce exactly the false-positive risk #234 fixed (a
|
||||
// sibling's differently-configured model looking like THIS profile's mismatch). So the
|
||||
// model genuinely is unknowable without a provisioned worktree, and unlike the silence
|
||||
// this branch used to keep, that gap now gets the same one-time, per-profile WARN
|
||||
// treatment discoveryUnavailable already gets above — but keyed by profile, since
|
||||
// several profiles can each hit this independently.
|
||||
if (!worktreeProvisioned) {
|
||||
if (cfg.model() != null && !cfg.model().isBlank()
|
||||
&& modelCheckSkippedWarned.add(cfg.profile())) {
|
||||
log.warn("opencode model-mismatch check (fleetd #175) cannot run for profile "
|
||||
+ "'{}': it was spawned without a fleetd-provisioned worktree (fleetd "
|
||||
+ "#249), so its cwd may be shared with other sessions and the actual "
|
||||
+ "model it is running cannot be safely told apart from a sibling's — "
|
||||
+ "spawn with worktree:true to enable the check for this profile.",
|
||||
cfg.profile());
|
||||
}
|
||||
return null;
|
||||
}
|
||||
// fleetd #234: once resolved, stay resolved. Re-deriving from `directory` on every call
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
package dev.ltms.fleet;
|
||||
|
||||
import ch.qos.logback.classic.Logger;
|
||||
import ch.qos.logback.classic.spi.ILoggingEvent;
|
||||
import ch.qos.logback.core.read.ListAppender;
|
||||
import dev.ltms.fleet.config.FleetConfig;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.io.TempDir;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||
|
||||
/**
|
||||
* fleetd #184: startup must state whether members share fleetd's OS user or use a separate herdr.
|
||||
*/
|
||||
class MemberTrustModelReportTest {
|
||||
|
||||
private static FleetConfig load(Path dir, String yaml) throws Exception {
|
||||
Path f = dir.resolve("fleetd.yaml");
|
||||
Files.writeString(f, yaml);
|
||||
return FleetConfig.load(f);
|
||||
}
|
||||
|
||||
private static ListAppender<ILoggingEvent> attach() {
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(Fleetd.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
return appender;
|
||||
}
|
||||
|
||||
private static void detach(ListAppender<ILoggingEvent> appender) {
|
||||
((Logger) LoggerFactory.getLogger(Fleetd.class)).detachAppender(appender);
|
||||
}
|
||||
|
||||
private static String report(FleetConfig cfg) {
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(Fleetd.class);
|
||||
ch.qos.logback.classic.Level original = logger.getLevel();
|
||||
logger.setLevel(ch.qos.logback.classic.Level.INFO);
|
||||
ListAppender<ILoggingEvent> appender = attach();
|
||||
try {
|
||||
Fleetd.reportMemberTrustModel(cfg);
|
||||
} finally {
|
||||
detach(appender);
|
||||
logger.setLevel(original);
|
||||
}
|
||||
return appender.list.getFirst().getFormattedMessage();
|
||||
}
|
||||
|
||||
@Test
|
||||
void unsetMemberHerdrSocketStatesThatMembersAreNotSandboxed(@TempDir Path dir) throws Exception {
|
||||
FleetConfig cfg = load(dir, "bind:\n host: 127.0.0.1\n port: 8765\n");
|
||||
|
||||
assertEquals("member trust model: members run as the same OS user as fleetd, not in a sandbox. "
|
||||
+ "A member can read any file this user can read, including SSH keys and credential "
|
||||
+ "stores, whatever memberCredentials says. To add a real boundary, route members to "
|
||||
+ "a second herdr under a different OS user with memberHerdrSocket.",
|
||||
report(cfg));
|
||||
}
|
||||
|
||||
@Test
|
||||
void configuredMemberHerdrSocketStatesThatFleetdCannotConfirmTheBoundary(@TempDir Path dir) throws Exception {
|
||||
FleetConfig cfg = load(dir, "memberHerdrSocket: /tmp/member-herdr.sock\n");
|
||||
|
||||
assertEquals("member trust model: members are routed to a separate herdr through "
|
||||
+ "memberHerdrSocket. fleetd cannot see that herdr's uid, so confirm it runs "
|
||||
+ "as a different OS user before treating it as a boundary.",
|
||||
report(cfg));
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,96 @@
|
||||
package dev.ltms.fleet;
|
||||
|
||||
import com.tngtech.archunit.base.DescribedPredicate;
|
||||
import com.tngtech.archunit.core.domain.JavaClass;
|
||||
import com.tngtech.archunit.core.domain.JavaClass.Predicates;
|
||||
import com.tngtech.archunit.core.importer.ClassFileImporter;
|
||||
import com.tngtech.archunit.core.importer.ImportOption;
|
||||
import com.tngtech.archunit.library.dependencies.SliceRule;
|
||||
import com.tngtech.archunit.library.dependencies.SlicesRuleDefinition;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
/**
|
||||
* fleetd #131 (CB-627): enforce package boundaries with an ArchUnit test instead of a
|
||||
* Maven module split.
|
||||
*
|
||||
* <p>This test fails the build the moment a NEW cycle appears between the top-level
|
||||
* {@code dev.ltms.fleet.*} packages. Today's cycles are recorded below as explicit,
|
||||
* narrow exceptions: each one ignores dependencies between exactly the two named
|
||||
* packages, in both directions, and nothing else. A cycle through any other pair of
|
||||
* packages -- or a brand new pair -- still fails this test.
|
||||
*
|
||||
* <p><b>Main code only.</b> The import excludes test classes
|
||||
* ({@link ImportOption.Predefined#DO_NOT_INCLUDE_TESTS}). Test code legitimately wires
|
||||
* across many packages for setup and mocking; that is not part of the shipped
|
||||
* architecture this rule protects. Verified: importing test classes too pulls in a much
|
||||
* larger, noisier cycle set -- {@code herdr}, {@code member}, {@code peer}, {@code
|
||||
* config}, {@code guard} and {@code placement} all show up in cycles that disappear the
|
||||
* moment test classes are excluded. Scanning off the classpath via {@code
|
||||
* importPackages(...)} (not a hardcoded {@code target/classes} path) also keeps this
|
||||
* test correct regardless of the working directory the build is invoked from.
|
||||
*
|
||||
* <p><b>No package moves here</b> -- ticket #131 is explicit that removing a cycle is
|
||||
* its own, later PR. See the comment on each exception below for which ticket step
|
||||
* removes it.
|
||||
*/
|
||||
class PackageCyclesTest {
|
||||
|
||||
@Test
|
||||
void packagesAreFreeOfCycles() {
|
||||
var classes = new ClassFileImporter()
|
||||
.withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS)
|
||||
.importPackages("dev.ltms.fleet");
|
||||
|
||||
SliceRule rule = SlicesRuleDefinition.slices()
|
||||
.matching("dev.ltms.fleet.(*)..")
|
||||
.should().beFreeOfCycles();
|
||||
|
||||
// fleetd #131 step 1: move ConnectionIdentity so authz stops depending on the
|
||||
// MCP layer. Evidence: auth/CallerResolver.java:3 imports mcp.ConnectionIdentity;
|
||||
// mcp/FleetMcp.java:3-7 imports auth.AuditLog, Authz, CallerResolver, Principal,
|
||||
// Role.
|
||||
rule = ignoreCycle(rule, "auth", "mcp");
|
||||
|
||||
// fleetd #131 step 2: PrimaryRegistry is used by loops in msg; move it, or put
|
||||
// an interface between msg and mcp. Evidence: msg/ReplyPushLoop.java:5 and
|
||||
// msg/LeadHeartbeatLoop.java:5 import mcp.PrimaryRegistry; mcp/FleetMcp.java:15-18
|
||||
// imports msg.LeadChannel, LeadMessage, MessageService, Rendezvous.
|
||||
rule = ignoreCycle(rule, "mcp", "msg");
|
||||
|
||||
// fleetd #131 -- found while implementing this test, NOT one of the ticket's
|
||||
// original three; it names its own follow-up step before removal. Evidence:
|
||||
// inject/CompletionResolver.java:4-5, inject/Injector.java:6 and
|
||||
// inject/TurnListener.java:3 import msg.Rendezvous / msg.TurnToken;
|
||||
// msg/MessageService.java:6 imports inject.Injector.
|
||||
rule = ignoreCycle(rule, "inject", "msg");
|
||||
|
||||
// fleetd #131 -- same as above, its own follow-up. Evidence:
|
||||
// metrics/FleetMetrics.java:3 imports msg.ReplyInbox; msg/MessageService.java:7-8,
|
||||
// msg/LeadHeartbeatLoop.java:6-7 and msg/ReplyPushLoop.java:6-7 import
|
||||
// metrics.FleetMetrics / metrics.Metrics.
|
||||
rule = ignoreCycle(rule, "metrics", "msg");
|
||||
|
||||
// fleetd #131 -- same as above, its own follow-up. Evidence:
|
||||
// session/SessionManager.java:7 imports msg.TurnToken;
|
||||
// msg/LeadHeartbeatLoop.java:8 imports session.MemberSession.
|
||||
rule = ignoreCycle(rule, "msg", "session");
|
||||
|
||||
rule.check(classes);
|
||||
}
|
||||
|
||||
/**
|
||||
* Accepts today's known cycle between two top-level packages, and nothing else.
|
||||
* Ignoring both directions removes exactly this pair from cycle detection; every
|
||||
* other dependency -- including any new one added later, between these same two
|
||||
* packages or any other pair -- is still checked.
|
||||
*/
|
||||
private static SliceRule ignoreCycle(SliceRule rule, String packageA, String packageB) {
|
||||
return rule
|
||||
.ignoreDependency(residesIn(packageA), residesIn(packageB))
|
||||
.ignoreDependency(residesIn(packageB), residesIn(packageA));
|
||||
}
|
||||
|
||||
private static DescribedPredicate<JavaClass> residesIn(String topLevelPackage) {
|
||||
return Predicates.resideInAPackage("dev.ltms.fleet." + topLevelPackage + "..");
|
||||
}
|
||||
}
|
||||
@@ -2010,22 +2010,93 @@ class FleetConfigTest {
|
||||
"deny-list normalizes onto the canonical deny-by-default value");
|
||||
}
|
||||
|
||||
/**
|
||||
* CB-633: {@code SSH_AUTH_SOCK} is a decision, never a default — absent, blank, or misspelled,
|
||||
* it stays BLOCKED; only the literal "allow" (any case) passes it through. A typo like "alow"
|
||||
* failing safe here is the whole point of making it a knob.
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockDefaultsToBlockedAndOnlyExplicitAllowUnblocksIt() {
|
||||
assertTrue(new FleetConfig.MemberCredentials("allow-list", List.of(), List.of()).sshAuthSock().equals("block"),
|
||||
"absent knob blocks SSH_AUTH_SOCK");
|
||||
assertFalse(new FleetConfig.MemberCredentials("allow-list", List.of(), List.of()).sshAuthSockAllowed());
|
||||
assertFalse(new FleetConfig.MemberCredentials(null, null, null, "").sshAuthSockAllowed(),
|
||||
"blank knob blocks SSH_AUTH_SOCK");
|
||||
assertFalse(new FleetConfig.MemberCredentials(null, null, null, "alow").sshAuthSockAllowed(),
|
||||
"a misspelled value fails SAFE, not open");
|
||||
assertTrue(new FleetConfig.MemberCredentials(null, null, null, "ALLOW").sshAuthSockAllowed(),
|
||||
"the literal allow (case-insensitive) unblocks SSH_AUTH_SOCK");
|
||||
void sshAgentEnvAcceptsEveryCompatibleKeyAndValuePair(@TempDir Path dir) throws Exception {
|
||||
String[][] spellings = {
|
||||
{"sshAuthSock", "block", "omit"},
|
||||
{"sshAuthSock", "allow", "inherit"},
|
||||
{"sshAuthSock", "omit", "omit"},
|
||||
{"sshAuthSock", "inherit", "inherit"},
|
||||
{"sshAgentEnv", "block", "omit"},
|
||||
{"sshAgentEnv", "allow", "inherit"},
|
||||
{"sshAgentEnv", "omit", "omit"},
|
||||
{"sshAgentEnv", "inherit", "inherit"}
|
||||
};
|
||||
|
||||
for (int i = 0; i < spellings.length; i++) {
|
||||
Path file = dir.resolve("member-credentials-ssh-agent-" + i + ".yaml");
|
||||
Files.writeString(file, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
""" + " " + spellings[i][0] + ": " + spellings[i][1] + "\n");
|
||||
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials();
|
||||
assertEquals(spellings[i][2], credentials.sshAgentEnv(),
|
||||
spellings[i][0] + ": " + spellings[i][1] + " must normalize correctly");
|
||||
assertEquals("inherit".equals(spellings[i][2]), credentials.sshAgentEnvInherited());
|
||||
}
|
||||
}
|
||||
|
||||
/** Protects the live {@code sshAuthSock: block} allow-list configuration during the rename. */
|
||||
@Test
|
||||
void legacySshAuthSockBlockKeepsLiveAllowListConfigOmitted(@TempDir Path dir) throws Exception {
|
||||
Path file = dir.resolve("live-member-credentials.yaml");
|
||||
Files.writeString(file, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
sshAuthSock: block
|
||||
""");
|
||||
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials();
|
||||
|
||||
assertEquals("omit", credentials.sshAgentEnv());
|
||||
assertFalse(credentials.sshAgentEnvInherited(), "the live config must omit SSH_AUTH_SOCK");
|
||||
}
|
||||
|
||||
@Test
|
||||
void sshAgentEnvWinsWhenBothCompatibleKeysArePresent(@TempDir Path dir) throws Exception {
|
||||
Path file = dir.resolve("both-ssh-agent-keys.yaml");
|
||||
Files.writeString(file, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
sshAuthSock: allow
|
||||
sshAgentEnv: omit
|
||||
""");
|
||||
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(file).memberCredentials();
|
||||
|
||||
assertEquals("omit", credentials.sshAgentEnv());
|
||||
assertFalse(credentials.sshAgentEnvInherited());
|
||||
}
|
||||
|
||||
@Test
|
||||
void sshAgentEnvDefaultsToOmitAndUnknownValuesFailClosed(@TempDir Path dir) throws Exception {
|
||||
Path absent = dir.resolve("member-credentials-ssh-agent-absent.yaml");
|
||||
Files.writeString(absent, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
""");
|
||||
assertEquals("omit", FleetConfig.load(absent).memberCredentials().sshAgentEnv());
|
||||
|
||||
Path unknown = dir.resolve("member-credentials-ssh-agent-unknown.yaml");
|
||||
Files.writeString(unknown, """
|
||||
bind:
|
||||
port: 8080
|
||||
memberCredentials:
|
||||
policy: allow-list
|
||||
sshAgentEnv: inhert
|
||||
""");
|
||||
FleetConfig.MemberCredentials credentials = FleetConfig.load(unknown).memberCredentials();
|
||||
assertEquals("omit", credentials.sshAgentEnv());
|
||||
assertFalse(credentials.sshAgentEnvInherited(), "an unknown value must fail closed");
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -34,6 +34,7 @@ import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.concurrent.CompletableFuture;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.function.Function;
|
||||
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
|
||||
@@ -810,13 +811,16 @@ class FleetMcpTest {
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #176: this is the exact shape measured on the Mac fleet — {@code maxLoad:3, live:2},
|
||||
* where one of the "free" three is really the lead's own seat on the same subscription. The old
|
||||
* formula ({@code max(0, cap - live)}) reported {@code free:1}; the real ceiling is {@code 0}
|
||||
* (two members plus the lead's own seat already fill all three).
|
||||
* fleetd #257: this is the exact shape measured on the Mac fleet — {@code maxLoad:3, live:2},
|
||||
* one lead sharing the subscription. The OLD formula ({@code max(0, cap - live - leadSeats)})
|
||||
* reported {@code free:0} here, disagreeing with the real spawn gate (which never read
|
||||
* {@code leadSeats} and would still grant one more spawn — see
|
||||
* {@code freeMatchesWhatTheRealPlacementGateActuallyGrants} below for that proof against the
|
||||
* actual gate). {@code free} must report {@code 1}: {@code leadSeats} is carried as a fact, not
|
||||
* subtracted.
|
||||
*/
|
||||
@Test
|
||||
void leadSeatSubtractsFromFreeTheSameWayLiveDoes() {
|
||||
void leadSeatIsReportedButNeverSubtractedFromFree() {
|
||||
FakeHerdr h = new FakeHerdr();
|
||||
SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")));
|
||||
FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource(
|
||||
@@ -829,17 +833,17 @@ class FleetMcpTest {
|
||||
|
||||
assertTrue(out.contains("\"maxLoad\":3"), "maxLoad itself must be left untouched: " + out);
|
||||
assertTrue(out.contains("\"live\":2"), out);
|
||||
assertTrue(out.contains("\"free\":0"), "2 live + 1 lead seat fills all 3: " + out);
|
||||
assertTrue(out.contains("\"leadSeats\":1"), out);
|
||||
assertTrue(out.contains("\"free\":1"), "free is maxLoad - live only, never minus leadSeats: " + out);
|
||||
assertTrue(out.contains("\"leadSeats\":1"), "leadSeats is still reported, just not subtracted: " + out);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #176: the OTHER measurement in the issue — a completely idle fleet still overstates
|
||||
* {@code free} by the lead's own seat. {@code maxLoad:3, live:0} must report {@code free:2}, the
|
||||
* real fan-out ceiling, not {@code 3}.
|
||||
* fleetd #257: the OTHER measurement in the issue — a completely idle fleet with a lead sharing
|
||||
* the subscription. {@code maxLoad:3, live:0} must report {@code free:3}, matching what the
|
||||
* spawn gate (which has no notion of a lead's seat) would actually grant.
|
||||
*/
|
||||
@Test
|
||||
void leadSeatLowersFreeOnAnOtherwiseIdleSubscriptionProfile() {
|
||||
void leadSeatDoesNotLowerFreeOnAnOtherwiseIdleSubscriptionProfile() {
|
||||
FakeHerdr h = new FakeHerdr();
|
||||
SessionManager sessions = new SessionManager(workerService(h, "http://gx00.gw:8000", Set.of("gx00.gw")));
|
||||
FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource(
|
||||
@@ -851,10 +855,76 @@ class FleetMcpTest {
|
||||
FleetMcp.QuarantineSource.none(), FleetMcp.OutageSource.none(), leadSeats, Map.of(), "", null));
|
||||
|
||||
assertTrue(out.contains("\"live\":0"), out);
|
||||
assertTrue(out.contains("\"free\":2"), "an idle fleet's real ceiling is 3 minus the lead's own seat: " + out);
|
||||
assertTrue(out.contains("\"free\":3"), "the real gate never subtracts the lead's seat: " + out);
|
||||
assertTrue(out.contains("\"leadSeats\":1"), out);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #257 — the defect, driven against the REAL placement gate, not a copy of its
|
||||
* arithmetic. {@code free} must equal exactly how many more spawns
|
||||
* {@link CompositePeerLauncher#spawn} (routed through the same {@link SessionManager} fleet_spawn
|
||||
* itself uses) will grant on this profile right now: this test reads whatever number
|
||||
* {@code fleet_list}'s real {@code listFleet} call reports, then drives that many spawns through
|
||||
* the REAL composite launcher and asserts every one succeeds, and the next one — one past what
|
||||
* fleet_list promised — is refused. A test that instead hand-computed {@code cap - live} and
|
||||
* compared it to {@code free} would pass even if both sides shared the same wrong formula (this
|
||||
* repo has been bitten by exactly that before); this one only passes when fleet_list's number and
|
||||
* the gate's real behaviour actually agree.
|
||||
*
|
||||
* <p>{@code liveCount} here is wired the same way {@code Fleetd.main} wires it in production: one
|
||||
* function, read by both the {@link CompositePeerLauncher}'s {@code maxLoad} gate and
|
||||
* {@code fleet_list}'s {@code CapacitySource}, off the SAME {@link SessionManager#roster()} — so
|
||||
* the two paths cannot silently drift on what "live" means.
|
||||
*/
|
||||
@Test
|
||||
void freeMatchesWhatTheRealPlacementGateActuallyGrants() {
|
||||
FakeHerdr h = new FakeHerdr();
|
||||
FleetConfig.Profile wcfg = new FleetConfig.Profile(
|
||||
"sonnet", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null,
|
||||
"tab", "fleetd-workers", "worker: {profile} #{n}", null,
|
||||
null, null, null, null, null, null, null, 3);
|
||||
Map<String, FleetConfig.Profile> profiles = Map.of(wcfg.profile(), wcfg);
|
||||
ClaudeCodeLauncher delegate = new ClaudeCodeLauncher(
|
||||
new AgentControl(h), new WorkspaceControl(h), new SubscriptionGuard(Set.of("gx00.gw")),
|
||||
profiles, wcfg.profile(), k -> "FLEETD_WORKER_TOKEN".equals(k) ? "tok" : null);
|
||||
|
||||
java.util.concurrent.atomic.AtomicReference<SessionManager> smRef =
|
||||
new java.util.concurrent.atomic.AtomicReference<>();
|
||||
Function<String, Integer> liveCount = profile -> (int) smRef.get().roster().stream()
|
||||
.filter(s -> profile.equals(s.profile())).count();
|
||||
CompositePeerLauncher composite = new CompositePeerLauncher(
|
||||
List.of(delegate), wcfg.profile(), profiles, PlacementPolicies.fixed(), liveCount);
|
||||
SessionManager sm = new SessionManager(composite);
|
||||
smRef.set(sm);
|
||||
|
||||
// Two members already live — the exact shape measured in fleetd #257 (maxLoad:3, live:2).
|
||||
assertFalse(FleetMcp.spawn(sm, "sonnet").isError(), "setup: first live member must spawn cleanly");
|
||||
assertFalse(FleetMcp.spawn(sm, "sonnet").isError(), "setup: second live member must spawn cleanly");
|
||||
|
||||
// A lead session shares this profile's subscription — leadSeats:1, same as the ticket.
|
||||
FleetMcp.LeadSeatSource leadSeats = new FleetMcp.LeadSeatSource(p -> "sonnet".equals(p) ? 1 : 0);
|
||||
String out = textOf(FleetMcp.listFleet(composite, sm, null,
|
||||
new FleetMcp.CapacitySource(liveCount, p -> profiles.get(p).maxLoad(), profiles::keySet, () -> 0),
|
||||
new FleetMcp.HealthCoverageSource(() -> "off"), FleetMcp.QuarantineSource.none(),
|
||||
FleetMcp.OutageSource.none(), leadSeats, Map.of(), "", null));
|
||||
int reportedFree = extractInt(out, "free");
|
||||
|
||||
for (int i = 0; i < reportedFree; i++) {
|
||||
McpSchema.CallToolResult res = FleetMcp.spawn(sm, "sonnet");
|
||||
assertFalse(res.isError(), "fleet_list promised free:" + reportedFree + "; spawn #" + (i + 1)
|
||||
+ " of that many was refused by the real gate: " + textOf(res));
|
||||
}
|
||||
McpSchema.CallToolResult overflow = FleetMcp.spawn(sm, "sonnet");
|
||||
assertTrue(overflow.isError(), "fleet_list reported free:" + reportedFree
|
||||
+ " but the real placement gate granted at least one more spawn than that: " + textOf(overflow));
|
||||
}
|
||||
|
||||
private static int extractInt(String json, String key) {
|
||||
java.util.regex.Matcher m = java.util.regex.Pattern.compile("\"" + key + "\":(-?\\d+)").matcher(json);
|
||||
assertTrue(m.find(), "no \"" + key + "\" field in: " + json);
|
||||
return Integer.parseInt(m.group(1));
|
||||
}
|
||||
|
||||
/** A profile with no lead seats reported must be byte-identical to before this ticket. */
|
||||
@Test
|
||||
void zeroLeadSeatsOmitsTheKeyAndLeavesFreeUnchanged() {
|
||||
|
||||
@@ -26,6 +26,7 @@ import org.junit.jupiter.api.io.TempDir;
|
||||
import org.slf4j.LoggerFactory;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.UncheckedIOException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.attribute.PosixFileAttributeView;
|
||||
@@ -39,6 +40,7 @@ import java.util.UUID;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicBoolean;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import java.util.function.Function;
|
||||
import java.util.function.Supplier;
|
||||
@@ -2594,4 +2596,150 @@ class ClaudeCodeLauncherTest {
|
||||
+ tornRead.get());
|
||||
assertEquals(newContent, Files.readString(target), "the final content must be the new content");
|
||||
}
|
||||
|
||||
// --- fleetd #247: CAS against a writer TRUST_JSON_LOCK cannot reach --------------------------
|
||||
//
|
||||
// fleetd #149's lock only serialises seedTrustDialog calls THIS launcher makes inside this one
|
||||
// JVM. It does nothing about the one writer that actually shares this file on a real host: the
|
||||
// operator's own live Claude Code, whose CLAUDE_CONFIG_DIR is routinely the very configDir this
|
||||
// profile is given. A plain read-modify-write there is a routine lost update — fleetd reads v1,
|
||||
// the operator's session writes v2, fleetd's ATOMIC_MOVE lands v3 built from v1, and v2 is gone,
|
||||
// atomically. The fix re-reads the file's exact bytes immediately before the move and compares
|
||||
// them with what the update was built from, retrying from fresh bytes on a mismatch.
|
||||
//
|
||||
// Both tests below drive the race through the real seedTrustDialog/spawn() path (not a
|
||||
// hand-rolled call to some extracted primitive) using trustJsonCasTestHook — a seam fired once
|
||||
// per CAS attempt, at the exact point between the read and the final compare, so the race is
|
||||
// deterministic instead of depending on real thread timing.
|
||||
|
||||
@Test
|
||||
void seedTrustDialogRetriesAndPreservesAConcurrentExternalWritersChange(
|
||||
@TempDir Path configDir, @TempDir Path worktree) throws Exception {
|
||||
markAsProvisionedWorktree(worktree);
|
||||
Path claudeJson = configDir.resolve(".claude.json");
|
||||
Files.writeString(claudeJson, "{\"projects\":{}}");
|
||||
|
||||
// Fires exactly once, on the first CAS attempt — simulating the operator's own live Claude
|
||||
// Code landing its own write to this SAME file in the gap between fleetd's read and write.
|
||||
AtomicBoolean fired = new AtomicBoolean(false);
|
||||
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {
|
||||
if (fired.compareAndSet(false, true)) {
|
||||
try {
|
||||
Files.writeString(claudeJson,
|
||||
"{\"projects\":{\"/operator/own/project\":"
|
||||
+ "{\"hasTrustDialogAccepted\":true}}}");
|
||||
} catch (IOException e) {
|
||||
throw new UncheckedIOException(e);
|
||||
}
|
||||
}
|
||||
};
|
||||
try {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = trustProfile(configDir.toString(), worktree.toString());
|
||||
new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
|
||||
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null)
|
||||
.spawn();
|
||||
} finally {
|
||||
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {};
|
||||
}
|
||||
|
||||
assertTrue(fired.get(), "the race hook must actually have fired during the spawn");
|
||||
JsonNode root = new ObjectMapper().readTree(claudeJson.toFile());
|
||||
assertTrue(root.path("projects").path("/operator/own/project")
|
||||
.path("hasTrustDialogAccepted").asBoolean(false),
|
||||
"the external writer's change, landed between fleetd's read and write, must SURVIVE "
|
||||
+ "— this is the whole point of the CAS: without it, fleetd's stale-built "
|
||||
+ "ATOMIC_MOVE would have silently discarded it. Final file: "
|
||||
+ Files.readString(claudeJson));
|
||||
assertTrue(root.path("projects").path(worktree.toString())
|
||||
.path("hasTrustDialogAccepted").asBoolean(false),
|
||||
"fleetd's own retry must still land its own trust entry, built from the fresh bytes");
|
||||
}
|
||||
|
||||
/**
|
||||
* Retry-exhaustion path: an external writer that changes the file on EVERY attempt (not just
|
||||
* once) exhausts all {@code MAX_TRUST_JSON_CAS_ATTEMPTS} retries. fleetd must then write nothing
|
||||
* at all — not even a partial/best-effort write — and log a WARN naming the file it gave up on.
|
||||
*/
|
||||
@Test
|
||||
void seedTrustDialogWritesNothingAndWarnsWhenCasRetriesAreExhausted(
|
||||
@TempDir Path configDir, @TempDir Path worktree) throws Exception {
|
||||
markAsProvisionedWorktree(worktree);
|
||||
Path claudeJson = configDir.resolve(".claude.json");
|
||||
Files.writeString(claudeJson, "{\"marker\":\"start\"}");
|
||||
|
||||
AtomicInteger hookCalls = new AtomicInteger();
|
||||
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {
|
||||
try {
|
||||
Files.writeString(claudeJson,
|
||||
"{\"marker\":\"race-" + hookCalls.incrementAndGet() + "\"}");
|
||||
} catch (IOException e) {
|
||||
throw new UncheckedIOException(e);
|
||||
}
|
||||
};
|
||||
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(ClaudeCodeLauncher.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = trustProfile(configDir.toString(), worktree.toString());
|
||||
new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
|
||||
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null)
|
||||
.spawn();
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
ClaudeCodeLauncher.trustJsonCasTestHook = () -> {};
|
||||
}
|
||||
|
||||
assertEquals(5, hookCalls.get(), "the race hook must fire exactly once per CAS attempt");
|
||||
String finalContent = Files.readString(claudeJson);
|
||||
assertEquals("{\"marker\":\"race-" + hookCalls.get() + "\"}", finalContent,
|
||||
"the file must be left exactly as the external writer last left it — fleetd must not "
|
||||
+ "have written at all once retries are exhausted");
|
||||
assertFalse(finalContent.contains("hasTrustDialogAccepted"),
|
||||
"the trust entry must never appear — its presence would mean the CAS gave up and "
|
||||
+ "wrote anyway instead of skipping the seed");
|
||||
|
||||
assertTrue(appender.list.stream().anyMatch(e -> e.getLevel() == Level.WARN
|
||||
&& e.getFormattedMessage().contains("gave up seeding workspace-trust")
|
||||
&& e.getFormattedMessage().contains(worktree.toString())
|
||||
&& e.getFormattedMessage().contains(claudeJson.toString())),
|
||||
"a WARN naming both the cwd and the file it gave up on must be logged: "
|
||||
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
|
||||
}
|
||||
|
||||
/**
|
||||
* The loud-default WARN (fleetd #247): a profile with no {@code configDir} targets the
|
||||
* operator's real {@code ~/.claude.json} (redirected here to a {@code @TempDir}), and every time
|
||||
* that happens must be logged, not just detected.
|
||||
*/
|
||||
@Test
|
||||
void seedTrustDialogWarnsEveryTimeItTargetsTheDefaultClaudeJson(
|
||||
@TempDir Path fakeHome, @TempDir Path worktree) throws Exception {
|
||||
markAsProvisionedWorktree(worktree);
|
||||
String originalHome = System.getProperty("user.home");
|
||||
System.setProperty("user.home", fakeHome.toString());
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(ClaudeCodeLauncher.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = trustProfile(null, worktree.toString());
|
||||
new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr),
|
||||
new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null)
|
||||
.spawn();
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
System.setProperty("user.home", originalHome);
|
||||
}
|
||||
|
||||
assertTrue(appender.list.stream().anyMatch(e -> e.getLevel() == Level.WARN
|
||||
&& e.getFormattedMessage().contains("no configDir set")
|
||||
&& e.getFormattedMessage().contains(worktree.toString())),
|
||||
"a WARN naming the cwd must fire when the profile sets no configDir: "
|
||||
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
|
||||
}
|
||||
}
|
||||
|
||||
+30
-3
@@ -170,10 +170,10 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
/**
|
||||
* {@code SSH_AUTH_SOCK} is a live ssh-agent handle, not a value — it must stay blocked under
|
||||
* {@code allow-list} even when the operator lists it under {@code allow:}, because {@code
|
||||
* sshAuthSock} defaults to blocked. Governed ONLY by {@code memberCredentials.sshAuthSock}.
|
||||
* sshAgentEnv} defaults to omit. Governed ONLY by {@code memberCredentials.sshAgentEnv}.
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockStaysBlockedEvenWhenListedInMemberCredentialsAllow() {
|
||||
void sshAgentEnvStaysOmittedEvenWhenListedInMemberCredentialsAllow() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
WiringLauncher launcher = new WiringLauncher(herdr,
|
||||
allowListWithAllow(List.of("SSH_AUTH_SOCK")));
|
||||
@@ -184,7 +184,7 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
String scrub = readAll(dir.resolve(EnvAllowListScrub.SCRUB_FILE));
|
||||
assertFalse(scrub.contains("'SSH_AUTH_SOCK'"),
|
||||
"SSH_AUTH_SOCK must not be on the derived allow-list just because the operator put "
|
||||
+ "it under allow: — sshAuthSock is unset here, so it defaults to block");
|
||||
+ "it under allow: — sshAgentEnv is unset here, so it defaults to omit");
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -387,6 +387,33 @@ class HerdrPeerLauncherAllowListWiringTest {
|
||||
+ appender.list.stream().map(ILoggingEvent::getFormattedMessage).toList());
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #184 item 5: the unknown-environment WARN must state the honest reason for the
|
||||
* UNKNOWN conclusion — fleetd has no channel to confirm what OS user the second herdr runs
|
||||
* as — and must NOT assert as fact that member panes run under a different OS user just
|
||||
* because {@code memberHerdrSocket} is configured. An operator may point it at a second herdr
|
||||
* running as the SAME user, for pane isolation alone; in that case members DO inherit fleetd's
|
||||
* environment, and asserting otherwise would tell the operator to disregard a real, known gap.
|
||||
*/
|
||||
@Test
|
||||
void unknownEnvironmentWarnStatesUncertaintyNotAnAssertedDifferentUser() {
|
||||
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", "/bin/zsh"));
|
||||
|
||||
List<String> messages = spawnAndCaptureLogs(launcher);
|
||||
|
||||
assertTrue(messages.stream().anyMatch(m -> m.contains("memberHerdrSocket")
|
||||
&& m.contains("no channel to confirm what OS user that herdr runs as")),
|
||||
"expected the WARN to name the actual uncertainty (no channel to confirm the "
|
||||
+ "herdr's uid), got: " + messages);
|
||||
assertFalse(messages.stream().anyMatch(m -> m.contains("member panes run under a different OS "
|
||||
+ "user than fleetd's own process")),
|
||||
"the WARN must not assert as fact that members run under a different OS user just "
|
||||
+ "because memberHerdrSocket is configured — got: " + messages);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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}
|
||||
|
||||
@@ -109,11 +109,11 @@ class MemberEnvAllowListTest {
|
||||
/**
|
||||
* {@code SSH_AUTH_SOCK} is a live handle to the operator's ssh-agent, never a value — so it must
|
||||
* stay excluded from the derived set even when the operator lists it under {@code allow:} for an
|
||||
* unrelated reason. It is governed ONLY by {@code memberCredentials.sshAuthSock}, applied
|
||||
* unrelated reason. It is governed ONLY by {@code memberCredentials.sshAgentEnv}, applied
|
||||
* separately by the caller ({@code HerdrPeerLauncher}).
|
||||
*/
|
||||
@Test
|
||||
void sshAuthSockInMemberCredentialsAllowIsStillExcluded() {
|
||||
void sshAgentEnvInMemberCredentialsAllowIsStillExcluded() {
|
||||
Set<String> derived = MemberEnvAllowList.derive(List.of(), Set.of("SSH_AUTH_SOCK", "OTHER_NAME"));
|
||||
|
||||
assertFalse(derived.contains("SSH_AUTH_SOCK"),
|
||||
|
||||
@@ -1440,4 +1440,131 @@ class OpenCodeLauncherTest {
|
||||
"the profile hint must survive the Fleetd-style forwarding hop and reach the real "
|
||||
+ "sink — a lambda forwarder drops it and this must go red");
|
||||
}
|
||||
|
||||
// --- fleetd #267: the #175 check never ran for the ordinary (no-worktree) spawn shape --------
|
||||
|
||||
/**
|
||||
* fleetd #267 acceptance criterion 2, half 1 — a regression guard for the NEW code path only:
|
||||
* a spawn WITH a fleetd-provisioned worktree must keep running the fleetd #175 model check
|
||||
* exactly as before (already proven thoroughly above), and must now ALSO never emit the new
|
||||
* fleetd #267 "cannot run" WARN, since the check is not skipped in this shape. Driven through
|
||||
* the real {@code SessionManager.acquire()}/{@code get()} late-resolve path (fleetd #209),
|
||||
* the same path the existing #175 tests already exercise.
|
||||
*/
|
||||
@Test
|
||||
void aProvisionedWorktreeSpawnRunsTheModelCheckThroughSessionManagerAndNeverLogsTheSkipWarn(
|
||||
@TempDir Path configRoot, @TempDir Path discRoot) throws Exception {
|
||||
String workDir = provisionedWorkDir(configRoot);
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = opencodeCfg("opencode/nemotron-3-ultra-free", null, null);
|
||||
List<String> exhausted = new ArrayList<>();
|
||||
ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason);
|
||||
OpenCodeLauncher launcher = serviceWithSink(herdr, configRoot, discRoot, cfg, sink);
|
||||
SessionManager sessions = new SessionManager(launcher);
|
||||
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(OpenCodeLauncher.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
MemberSession acquired = sessions.acquire(cfg.profile(), workDir, null, null);
|
||||
assertNull(acquired.agentSessionId(), "no opencode row yet");
|
||||
|
||||
OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_x", workDir, 1000L,
|
||||
"{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}");
|
||||
|
||||
Optional<MemberSession> after = sessions.get(acquired.paneId());
|
||||
assertEquals("ses_x", after.get().agentSessionId());
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
}
|
||||
|
||||
assertEquals(1, exhausted.size(),
|
||||
"the mismatch check still runs on the real path with a provisioned worktree: " + exhausted);
|
||||
boolean cannotRunWarn = appender.list.stream()
|
||||
.filter(e -> e.getLevel() == Level.WARN)
|
||||
.anyMatch(e -> e.getFormattedMessage().contains("cannot run"));
|
||||
assertFalse(cannotRunWarn, "a provisioned-worktree spawn must never log the fleetd #267 "
|
||||
+ "'cannot run' WARN — the check ran, it was not skipped: " + appender.list);
|
||||
}
|
||||
|
||||
/**
|
||||
* fleetd #267's central defect, reproduced and fixed: {@code OpenCodeLauncher.SessionAwareHandle
|
||||
* .agentSessionId()} is the ONLY caller of {@code checkModelMatch}, and it sits behind the
|
||||
* fleetd #249 worktree gate — so a plain {@code fleet_spawn} with no {@code worktree:true}
|
||||
* (the ticket's "ordinary, expected shape of the large majority of spawns") never reached
|
||||
* {@code checkModelMatch} at all. A test that called {@code checkModelMatch} directly, or built
|
||||
* a {@link OpenCodeLauncher.SessionAwareHandle}/{@link PeerHandle} in isolation, would have
|
||||
* passed on every single day this gap existed — it never drives {@code agentSessionId()}
|
||||
* through the worktree gate the way production does. This test instead drives the REAL
|
||||
* late-resolve path: {@code SessionManager.acquire()} (which calls {@code handle
|
||||
* .agentSessionId()} to build the very first {@code MemberSession}) and a re-poll via {@code
|
||||
* SessionManager.get()} (fleetd #209's retained-handle mechanism) — the exact sequence a live
|
||||
* pane goes through.
|
||||
*
|
||||
* <p>The fix chosen (see {@code OpenCodeLauncher}'s javadoc on the {@code !worktreeProvisioned}
|
||||
* branch) is the WARN path, not a decoupled check: {@code actualModelForSessionId} can only be
|
||||
* keyed safely by a RESOLVED session id (fleetd #234's fix for exactly this false-positive
|
||||
* risk), and without a provisioned worktree no id can ever be safely resolved (fleetd #249) —
|
||||
* re-deriving "whatever is newest in this shared directory" here would silently reintroduce the
|
||||
* false-positive risk #234 fixed. This test proves both halves: a plausible-looking mismatch
|
||||
* row for the shared, non-provisioned cwd never quarantines anything, AND the new one-time,
|
||||
* per-profile WARN replaces the old total silence.
|
||||
*/
|
||||
@Test
|
||||
void aSpawnWithoutAProvisionedWorktreeNeverRunsTheModelCheckButWarnsOncePerProfile(
|
||||
@TempDir Path configRoot, @TempDir Path discRoot) throws Exception {
|
||||
// Deliberately NOT provisionedWorkDir(...) / markAsProvisionedWorktree(...): a plain
|
||||
// directory with no .git marker — the exact "fleet_spawn with no worktree:" shape fleetd
|
||||
// #267 is about, and the ordinary shape the ticket says most spawns actually take.
|
||||
Path workDir = Files.createDirectories(configRoot.resolve("shared-cwd"));
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FleetConfig.Profile cfg = opencodeCfg("opencode/nemotron-3-ultra-free", null, null);
|
||||
List<String> exhausted = new ArrayList<>();
|
||||
ExhaustionSink sink = (target, reason, profile) -> exhausted.add(reason);
|
||||
OpenCodeLauncher launcher = serviceWithSink(herdr, configRoot, discRoot, cfg, sink);
|
||||
SessionManager sessions = new SessionManager(launcher);
|
||||
|
||||
Logger logger = (Logger) LoggerFactory.getLogger(OpenCodeLauncher.class);
|
||||
ListAppender<ILoggingEvent> appender = new ListAppender<>();
|
||||
appender.start();
|
||||
logger.addAppender(appender);
|
||||
try {
|
||||
// The real production entrypoint: acquire() calls handle.agentSessionId() itself to
|
||||
// build the very first MemberSession, BEFORE any row exists.
|
||||
MemberSession acquired = sessions.acquire(cfg.profile(), workDir.toString(), null, null);
|
||||
assertNull(acquired.agentSessionId(),
|
||||
"still refuses to guess an identity for a shared, non-provisioned cwd (fleetd #249)");
|
||||
|
||||
// A row for this exact (shared) directory appears, running a model that WOULD look like
|
||||
// a mismatch against cfg.model() if fleetd trusted the shared-directory heuristic —
|
||||
// exactly the false-positive shape fleetd #234 fixed for the id-resolved case.
|
||||
OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_sibling", workDir.toString(), 1000L,
|
||||
"{\"id\":\"gpt-5.6-sol\",\"providerID\":\"openai\"}");
|
||||
|
||||
// Re-drive the SAME real late-resolve path (fleetd #209) — repeatedly, to also prove
|
||||
// the new WARN fires at most once per profile, not once per poll.
|
||||
Optional<MemberSession> resolved = sessions.get(acquired.paneId());
|
||||
assertNull(resolved.get().agentSessionId(), "still no identity — the gate never opens");
|
||||
sessions.get(acquired.paneId());
|
||||
} finally {
|
||||
logger.detachAppender(appender);
|
||||
}
|
||||
|
||||
assertTrue(exhausted.isEmpty(),
|
||||
"must never quarantine off a shared-directory row it cannot trust as this session's "
|
||||
+ "own — fleetd #234's exact concern, now also for the model check: " + exhausted);
|
||||
|
||||
List<String> skipWarnings = appender.list.stream()
|
||||
.filter(e -> e.getLevel() == Level.WARN)
|
||||
.map(ILoggingEvent::getFormattedMessage)
|
||||
.filter(m -> m.contains("cannot run"))
|
||||
.toList();
|
||||
assertEquals(1, skipWarnings.size(),
|
||||
"exactly one 'cannot run' WARN across acquire() + two get() re-polls — the old code "
|
||||
+ "logged NOTHING here, which is the bug this ticket fixes; got: " + skipWarnings);
|
||||
assertTrue(skipWarnings.get(0).contains(cfg.profile()),
|
||||
"the WARN must name the profile, same treatment discoveryUnavailable already gets: "
|
||||
+ skipWarnings.get(0));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ import java.util.Locale;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Matcher;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Collectors;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
@@ -68,6 +69,32 @@ class RestRouteInventoryTest {
|
||||
"GET /tasks/{ticket}"
|
||||
);
|
||||
|
||||
private static final Pattern ROUTE_CALL =
|
||||
Pattern.compile("app\\.(get|post|delete|put|patch)\\(\\s*\"([^\"]+)\"");
|
||||
|
||||
/**
|
||||
* Drop whole-line comments before scraping. Without this the scrape reads commented-out code as
|
||||
* live: a registration disabled with {@code //} still matched, so the route stayed in the
|
||||
* inventory while the server no longer served it — a silent false PASS, measured on 2026-09-03
|
||||
* by commenting out {@code app.get("/tasks/{ticket}", ...)} and watching this test stay green.
|
||||
* Deleting the same line was caught correctly, so only the commented-out shape was blind.
|
||||
*
|
||||
* <p>Only lines whose first non-blank characters are {@code //}, {@code *} or {@code /*} are
|
||||
* dropped — deliberately NOT every {@code //} anywhere on a line, because that would also cut a
|
||||
* string literal containing {@code //} (a URL) and could silently delete a real registration
|
||||
* sharing that line. The remaining gap is a trailing comment on the same line as real code; no
|
||||
* registration in this file has that shape, and the vacuity test below would catch a scrape that
|
||||
* lost registrations wholesale.
|
||||
*/
|
||||
private static String withoutCommentLines(String source) {
|
||||
return source.lines()
|
||||
.filter(line -> {
|
||||
String t = line.stripLeading();
|
||||
return !(t.startsWith("//") || t.startsWith("*") || t.startsWith("/*"));
|
||||
})
|
||||
.collect(Collectors.joining("\n"));
|
||||
}
|
||||
|
||||
/**
|
||||
* Every {@code app.<verb>("<path>")} call in {@link FleetApp}'s source, as {@code "VERB path"}.
|
||||
* This matches inside an {@code if (...) { ... }} block just as well as a top-level statement —
|
||||
@@ -75,8 +102,7 @@ class RestRouteInventoryTest {
|
||||
* catches {@code GET /metrics} (registered conditionally on {@code metrics != null}).
|
||||
*/
|
||||
private static Set<String> routesTheServerRegisters() throws Exception {
|
||||
String source = Files.readString(REST_SOURCE);
|
||||
Matcher m = Pattern.compile("app\\.(get|post|delete|put|patch)\\(\\s*\"([^\"]+)\"").matcher(source);
|
||||
Matcher m = ROUTE_CALL.matcher(withoutCommentLines(Files.readString(REST_SOURCE)));
|
||||
Set<String> found = new LinkedHashSet<>();
|
||||
while (m.find()) {
|
||||
found.add(m.group(1).toUpperCase(Locale.ROOT) + " " + m.group(2));
|
||||
|
||||
@@ -45,6 +45,9 @@ public final class FakeWorktrees implements Worktrees {
|
||||
private final List<String> overlayShareOrder = new CopyOnWriteArrayList<>();
|
||||
private final Set<String> existingPaths = ConcurrentHashMap.newKeySet();
|
||||
private final Set<String> trackedPaths = ConcurrentHashMap.newKeySet();
|
||||
/** Worktree paths that currently exist, mirroring GitWorktrees' {@code Files.exists} check for
|
||||
* the already-gone case (CB-576 review, fleetd #116). */
|
||||
private final Set<String> worktreePaths = ConcurrentHashMap.newKeySet();
|
||||
private final AtomicLong snapshotSeq = new AtomicLong();
|
||||
private volatile RuntimeException addFailure;
|
||||
private volatile RuntimeException snapshotFailure;
|
||||
@@ -115,7 +118,15 @@ public final class FakeWorktrees implements Worktrees {
|
||||
}
|
||||
// The branch already carries a unique nonce, so the derived path is distinct per acquire
|
||||
// without an extra counter — keep it a pure function of the branch the test can predict.
|
||||
return prefix + "/" + branch.replace('/', '_');
|
||||
String path = prefix + "/" + branch.replace('/', '_');
|
||||
worktreePaths.add(path);
|
||||
return path;
|
||||
}
|
||||
|
||||
/** Model an operator / {@code git worktree prune} removing the worktree before release. */
|
||||
public FakeWorktrees markGone(String worktreePath) {
|
||||
worktreePaths.remove(worktreePath);
|
||||
return this;
|
||||
}
|
||||
|
||||
@Override
|
||||
@@ -125,6 +136,11 @@ public final class FakeWorktrees implements Worktrees {
|
||||
|
||||
@Override
|
||||
public boolean hasUncommitted(String worktreePath) {
|
||||
// A path that was never added, or was marked gone, is reported clean, mirroring
|
||||
// GitWorktrees' already-gone guard — never an error, so teardown still completes.
|
||||
if (!worktreePaths.contains(worktreePath)) {
|
||||
return false;
|
||||
}
|
||||
return dirty;
|
||||
}
|
||||
|
||||
|
||||
@@ -258,6 +258,38 @@ class WorktreeSessionManagerTest {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* CB-576 review (fleetd #116). A worktree that is already gone (operator cleanup,
|
||||
* {@code git worktree prune}, an earlier half-completed release) must not break teardown.
|
||||
* {@code hasUncommitted} reports the missing path clean (mirroring {@code GitWorktrees}), so
|
||||
* {@code release} still runs {@code notifyReleased} (the CB-516 fast-fail for a blocked
|
||||
* {@code fleet_send} caller) and {@code launcher.stop} (so the pane is not orphaned), and falls
|
||||
* through to the already-gone-tolerant {@code remove}. Since CB-581 this all happens because the
|
||||
* notify-and-stop work sits in {@code release}'s {@code finally}/post-try block rather than a
|
||||
* checked branch — this test pins that shape by construction.
|
||||
*/
|
||||
@Test
|
||||
void releaseStillStopsPaneAndNotifiesWhenWorktreeIsGone() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt");
|
||||
SessionManager sessions = new SessionManager(workerService(herdr), worktrees);
|
||||
List<SessionManager.ReleaseDetail> released = new java.util.concurrent.CopyOnWriteArrayList<>();
|
||||
sessions.onRelease(released::add);
|
||||
MemberSession s = sessions.acquire("ltms-local", null, "/caller/proj", null,
|
||||
new WorktreeRequest("cb-576g", null));
|
||||
|
||||
worktrees.markGone(s.worktree());
|
||||
sessions.release(s.paneId());
|
||||
|
||||
assertEquals(1, released.size(),
|
||||
"notifyReleased must still fire when the worktree is already gone (CB-516)");
|
||||
assertEquals(s.terminalId(), released.getFirst().terminalId());
|
||||
assertTrue(herdr.called("pane.close"),
|
||||
"the pane must still be stopped when the worktree is already gone");
|
||||
assertEquals(1, worktrees.removeCalls().size(),
|
||||
"release still calls the already-gone-tolerant remove");
|
||||
}
|
||||
|
||||
@Test
|
||||
void drainAllPreservesWorktreeOfIdleSession() {
|
||||
FakeHerdr herdr = new FakeHerdr();
|
||||
|
||||
Reference in New Issue
Block a user