Compare commits

..

4 Commits

Author SHA1 Message Date
Dai Ha 5c56cb347f fleetd #224 / #225: share worktreeRoot with the group, and fix the group-detection test helper
CI / contract (pull_request) Successful in 44s
CI / build (pull_request) Successful in 1m42s
#224: GitWorktrees#add created worktreeRoot with the daemon's umask and never shared it with
worktreeGroup, even though shareWithGroup shares every child underneath it (each worktree, and
the repo's common git dir). Under memberHerdrSocket: the member pane runs as a different OS
user, which needs execute on every ancestor directory to reach anything underneath, no matter
how carefully each child is shared — so a member could not read the opencode.json #219 places
under this root, could not reach its own worktree, and could not read #213's ZDOTDIR scrub when
placed here either.

Fix: add() now calls a new shareRootWithGroup(root) right after creating the root, chgrp+chmod
g+x on the root itself (non-recursive — each child is still shared individually by its own call
site). No-op when worktreeGroup is unset, so behaviour is byte-identical in today's only live
mode. On failure (group missing, or operator not a member of it) the spawn is refused with a
WorktreeException naming the root, its current mode, and the group — mirroring shareWithGroup's
existing refusal shape — before `git worktree add` ever runs, so no partial worktree is left
behind.

Also adds the assertion the #221 reviewer flagged as missing: a test driving
EnvAllowListScrub#shareWithGroup directly against a directory holding several flat files
(opencode.json, member-charter.md, ide-rules.md, plus an unrelated one) and asserting every one
of them gets group-readable/never-group-writable permissions, not just the two files someone
happened to think of.

#225: OpenCodeLauncherTest/HerdrPeerLauncherAllowListWiringTest's currentUserGroup() read the
group that owns the current working directory, not the process's own primary group, despite its
comment claiming the latter. Those coincide only by accident: a home checkout is typically owned
by a group the operator belongs to (staff), while a checkout under /private/tmp on macOS is
group wheel, which the operator is usually not a member of — so the same test fails for real
depending on where the repo happens to be checked out, and the existing assumeTrue only guarded
against "no POSIX groups at all", never "a resolvable but wrong group". Fixed by resolving the
process's REAL primary group via `id -gn` instead, with assumeTrue (skip, not fail) only when
that itself cannot be resolved on the host. The permission assertions these tests exist for are
unchanged.

Verified `mvn clean install` green from both a home checkout and a /private/tmp copy (mirroring
the exact repro in #225): 1093 tests, 0 failures, 0 errors in both locations.
2026-09-02 07:47:13 +07:00
ltms dcf5fb3be3 Merge pull request 'CB-619 / fleetd #123: refuse an architect spawn with no matching slot' (#223) from worker/cb-123-role-demotion-c600f7-2 into main
CI / contract (push) Successful in 1m11s
CI / build (push) Successful in 1m55s
2026-09-01 10:41:47 +02:00
ltms f9d2ee2a2b Merge pull request 'fleetd#219: fix OpenCodeLauncher config/discovery roots under memberHerdrSocket' (#221) from worker/cb-219-opencode-roots-1f677e-1 into main
CI / contract (push) Successful in 45s
CI / build (push) Successful in 2m16s
2026-09-01 10:33:04 +02:00
Dai Ha 866c7f2e9a CB-619 / fleetd #123: refuse an architect spawn with no matching slot
CI / build (pull_request) Successful in 1m16s
CI / contract (pull_request) Successful in 1m18s
An explicit-profile spawn bypasses role-pool placement (CompositePeerLauncher
only constrains an UNQUALIFIED spawn to fleet.<role>), so it was the one path
that could ask for role=architect on a profile no architect slot carries.
MemberRegistry silently held the session as a plain worker while GET /members
still reported the requested "architect" and only fleet_whoami (which reads
live bindings, not the request) told the truth.

- MemberLifecycle.requireSlotFor(role, profile): refuses the acquire before
  anything spawns when no configured architect slot carries the profile,
  naming the role, the profile, and the pools that do carry it. No-op for
  dev/reviewer, which are placement candidates only, never a live identity
  binding — refusing a profile mismatch there would break the documented
  fleet_spawn{profile:"opus"} (role defaults to dev) flow.
- MemberLifecycle.acquired(...) now returns the role the session actually
  holds, so a residual race (a slot exists but every instance is already
  bound to a different terminal) still falls back to dev honestly instead of
  lying — this case logs at WARN (was INFO), naming profile and terminal.
- SessionManager now records the role acquired() returns on MemberSession,
  never the requested role, so GET /members and fleet_list can no longer
  report a role the member does not hold; no changes needed to memberView/
  rosterView, which just read session.role().

An architect's identity IS the slot it is bound to — binding a role with no
slot to bind means inventing an identity out of nothing, which is the quiet
failure the whole role system exists to prevent.

Tests: SessionManagerTest and FleetMcpTest each drive a real spawn through
FleetMcp.spawn -> SessionManager.acquire -> the real ClaudeCodeLauncher (via
FakeHerdr), then assert on GET /members and fleet_whoami for that same
session — not on MemberRegistry.bind directly (fleetd issue #113's mistake).
2026-09-01 15:29:39 +07:00
10 changed files with 492 additions and 22 deletions
@@ -7,15 +7,43 @@ public interface MemberLifecycle {
MemberLifecycle NONE = new MemberLifecycle() {
@Override
public void acquired(MemberRole role, String profile, String terminal) {
public MemberRole acquired(MemberRole role, String profile, String terminal) {
return role; // no registry configured — nothing to bind against, so the request stands
}
@Override
public void released(String terminal) {
}
@Override
public void requireSlotFor(MemberRole role, String profile) {
// no registry configured — nothing to validate against, so nothing is refused
}
};
void acquired(MemberRole role, String profile, String terminal);
/**
* Try to bind a newly spawned {@code terminal} into the role it was granted.
*
* @return the role this session actually holds: {@code role} unchanged for a role with no
* live slot-binding semantics (dev, reviewer), or when the bind succeeded; a fallback
* role — never {@code role} — when a slot-bound role (architect) could not be bound.
* Callers must record THIS value on the session, never the requested {@code role}, so
* a later roster read never reports a role the session does not hold (CB-619). In
* normal operation this fallback should not happen once {@link #requireSlotFor} has
* refused every unbindable spawn upfront — but a slot can still be lost between that
* check and this call to a concurrent spawn racing for the same slot, so the honest
* answer is still needed here too.
*/
MemberRole acquired(MemberRole role, String profile, String terminal);
void released(String terminal);
/**
* Refuse an acquire before anything spawns when {@code role} requires a live slot binding and
* no configured slot carries {@code profile} (CB-619 / fleetd #123). A no-op for a role with
* no slot-binding semantics.
*
* @throws IllegalArgumentException naming the role, the profile, and the pools that do carry it
*/
void requireSlotFor(MemberRole role, String profile);
}
@@ -8,6 +8,7 @@ import org.slf4j.LoggerFactory;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -206,19 +207,72 @@ public final class MemberRegistry implements MemberLifecycle {
*
* <p>The role check is lifecycle policy. {@link CallerResolver} repeats it when resolving a
* binding, so a later lifecycle regression cannot turn a worker into an architect.
*
* <p>CB-619 / fleetd #123: the return value is the role this session actually holds, and the
* caller is required to record THAT — never the requested {@code role} — on the session. Before
* this fix the caller kept the requested role regardless of whether the bind below succeeded, so
* a demoted session's {@code GET /members} row still said {@code "architect"} while
* {@code fleet_whoami} (which reads the live binding, not the request) correctly said
* {@code "worker"} — three sources of truth that disagreed about one live member, silently.
*/
@Override
public void acquired(MemberRole role, String profile, String terminal) {
public MemberRole acquired(MemberRole role, String profile, String terminal) {
if (role != MemberRole.ARCHITECT || terminal == null || terminal.isBlank()) {
return;
return role;
}
// slotsFor preserves definition order, so duplicate-profile slots use the first free one.
for (Entry entry : slotsFor(MemberRole.ARCHITECT).values()) {
if (Objects.equals(profile, entry.profile()) && bind(entry.key(), terminal)) {
return;
return MemberRole.ARCHITECT;
}
}
log.info("member slot: no free architect slot for profile={}; session remains a worker", profile);
// fleetd #123: at least WARN — a role downgrade that the roster must now also reflect is
// not routine bookkeeping. requireSlotFor already refuses the config-gap case (no slot at
// all carries this profile) before a process ever spawns; reaching here means the config DID
// carry a matching slot but every one of them was already bound to a different terminal — a
// race this pre-spawn check cannot close on its own (see requireSlotFor's javadoc).
log.warn("member slot: no free architect slot for profile={} terminal={}; holding the session "
+ "as {} instead of the architect it asked for — every configured slot for this "
+ "profile is already bound to a different terminal", profile, terminal,
MemberRole.DEV.wireName());
return MemberRole.DEV;
}
/**
* CB-619 / fleetd #123: refuse an architect acquire before anything spawns when no configured
* slot carries {@code profile} — the config-gap case from the original defect report (a spawn
* asked for {@code role=architect, profile=sonnet}, and {@code fleet.architects} carried only
* {@code opus} and {@code sol}). A dev/reviewer acquire is always a no-op: those pools are
* placement candidates only (see {@code CompositePeerLauncher}), never a live identity binding,
* so there is nothing here to refuse — an explicit profile outside the pool for those roles is a
* documented operator override, not a defect.
*
* <p>This closes the config-gap case, not the live-capacity case: a profile that DOES carry a
* slot can still lose the race to a concurrent spawn between this check and the actual
* {@link #bind}, which is why {@link #acquired} must still answer honestly even after this
* check has passed.
*/
@Override
public void requireSlotFor(MemberRole role, String profile) {
if (role != MemberRole.ARCHITECT) {
return;
}
boolean hasSlot = slotsFor(MemberRole.ARCHITECT).values().stream()
.anyMatch(e -> Objects.equals(profile, e.profile()));
if (hasSlot) {
return;
}
List<String> pools = slotsFor(MemberRole.ARCHITECT).values().stream()
.map(Entry::profile)
.distinct()
.toList();
throw new IllegalArgumentException(
"no " + role.wireName() + " slot for profile '" + profile + "' — an architect's "
+ "identity IS the slot it is bound to, so there is nothing to bind this "
+ "session's identity to. fleet." + role.configKey() + " carries profiles: "
+ (pools.isEmpty() ? "(none configured)" : String.join(", ", pools))
+ "; add profile '" + profile + "' there, or spawn " + role.wireName()
+ " on one of those profiles instead");
}
/** Unbind a released terminal using the compare-safe registry operation. */
@@ -13,6 +13,7 @@ import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.PosixFilePermissions;
import java.security.SecureRandom;
import java.util.ArrayList;
import java.util.HashSet;
@@ -161,6 +162,7 @@ public final class GitWorktrees implements Worktrees {
} catch (IOException e) {
throw new WorktreeException("cannot create worktree root " + root + ": " + e.getMessage(), e);
}
shareRootWithGroup(root);
String wt = path.toAbsolutePath().toString();
log.info("adding worktree branch={} path={} base={}", branch, wt, base);
removeUserInfoFromHttpsOrigin(repoRoot);
@@ -712,6 +714,54 @@ public final class GitWorktrees implements Worktrees {
group, repoRoot, worktreePath, touched);
}
/**
* fleetd #224: make {@code worktreeRoot} ITSELF group-traversable — established once, here,
* where the root is created, never at a use site. {@link #shareWithGroup} shares each worktree
* (and the repo's common git dir) with {@link #group}, but never the PARENT directory that
* contains every worktree — and under {@code memberHerdrSocket:} the member pane runs as a
* different OS user, which needs the execute bit on every ancestor directory to reach anything
* underneath, no matter how carefully each child is shared. Without this, a member cannot read
* the ephemeral {@code opencode.json} #219 places under this root, cannot reach its own
* worktree, and cannot read #213's ZDOTDIR scrub when placed here either.
*
* <p>No-op — no process spawned — when {@link #group} is null/blank, so behaviour with
* {@code worktreeGroup:} unset (today's only live mode) is unchanged. Only {@code root} itself
* is touched (non-recursive): each child underneath is shared individually, either by
* {@link #shareWithGroup} for a worktree or by the launcher that generates it (fleetd #213/#219)
* for a scrub/config directory — sharing this level again would just duplicate that policy in
* the wrong layer.
*
* <p>Fails loudly, naming {@code root}, its mode at the time of the attempt, and {@link #group}:
* a member that starts and then cannot see its own checkout is worse than a refused spawn, since
* nothing about that failure mode points at a directory's permission bits.
*/
private void shareRootWithGroup(Path root) {
if (group == null) {
return;
}
String mode = currentPosixMode(root);
try {
shareGroupRunner.apply(new String[]{"chgrp", group, root.toString()});
shareGroupRunner.apply(new String[]{"chmod", "g+x", root.toString()});
} catch (WorktreeException e) {
throw new WorktreeException("cannot make worktree root " + root + " (mode " + mode
+ ") group-traversable for group '" + group + "': " + e.getMessage()
+ " — the group must exist, and the fleetd operator (" + System.getProperty("user.name")
+ ") must be a member of it", e);
}
log.info("worktreeGroup={} made worktree root {} group-traversable (was mode {})", group, root, mode);
}
/** {@code root}'s current POSIX permission string, or {@code "unknown"} on a filesystem that does
* not support POSIX permissions — used only to name the mode in a refusal message. */
private static String currentPosixMode(Path root) {
try {
return PosixFilePermissions.toString(Files.getPosixFilePermissions(root));
} catch (IOException | UnsupportedOperationException e) {
return "unknown";
}
}
/**
* The repo's <em>common</em> git directory as an absolute path — where {@code objects},
* {@code refs} and {@code worktrees} actually live. {@code git rev-parse --git-common-dir}
@@ -183,6 +183,13 @@ public final class SessionManager implements TurnListener {
String sessionName, String resumeSessionId) {
MemberRole memberRole = (role == null) ? MemberRole.DEV : role;
requireResumeCapability(profile, resumeSessionId);
// CB-619 / fleetd #123: an explicit profile bypasses placement (CompositePeerLauncher only
// constrains an UNQUALIFIED spawn to the role's pool), so it is the one path that can ask
// for a role with no slot to bind it to. Refuse before anything spawns. A blank profile is
// left to placement, which already restricts an unqualified spawn to the role's pool.
if (profile != null && !profile.isBlank()) {
memberLifecycle.requireSlotFor(memberRole, profile);
}
if (wt == null) {
// CB-557: the role must ride on the SpawnRequest, not stay a local. The launcher needs it
// to pick the profile out of that role's pool and to label the tab; a role kept only on
@@ -198,11 +205,15 @@ public final class SessionManager implements TurnListener {
String resolvedProfile = resolveProfile(handle, profile);
String cwd = launcher.effectiveCwd(new SpawnRequest(resolvedProfile, requestedCwd, callerCwd));
long now = nowNanos.getAsLong();
// CB-619: bind (or fail to bind) BEFORE the session is recorded, and store whatever role
// this call actually returns — never the requested memberRole — so the session's role,
// what GET /members and fleet_list report, is never a lie about what this terminal holds.
MemberRole actualRole = memberLifecycle.acquired(memberRole, resolvedProfile, handle.terminalId());
MemberSession session = new MemberSession(
handle.id(),
handle.terminalId(),
resolvedProfile,
memberRole,
actualRole,
cwd,
ownerTerminal,
now,
@@ -215,7 +226,6 @@ public final class SessionManager implements TurnListener {
handle.agentSessionId());
registry.put(handle.id(), session);
handles.put(handle.id(), handle);
memberLifecycle.acquired(session.role(), session.profile(), session.terminalId());
log.debug("acquired session id={} terminal={} profile={} owner={}",
handle.id(), handle.terminalId(), session.profile(), session.ownerTerminal());
notifyAcquired(session.terminalId());
@@ -509,11 +519,14 @@ public final class SessionManager implements TurnListener {
String resolvedProfile = resolveProfile(handle, profile);
String cwd = launcher.effectiveCwd(new SpawnRequest(resolvedProfile, path, callerCwd));
long now = nowNanos.getAsLong();
// CB-619: see the no-worktree path above — bind before recording, and store the returned
// actual role, so this session's role is never a lie about what it actually holds.
MemberRole actualRole = memberLifecycle.acquired(memberRole, resolvedProfile, handle.terminalId());
MemberSession session = new MemberSession(
handle.id(),
handle.terminalId(),
resolvedProfile,
memberRole,
actualRole,
cwd,
ownerTerminal,
now,
@@ -526,7 +539,6 @@ public final class SessionManager implements TurnListener {
handle.agentSessionId());
registry.put(handle.id(), session);
handles.put(handle.id(), handle);
memberLifecycle.acquired(session.role(), session.profile(), session.terminalId());
log.debug("acquired worktree session id={} terminal={} profile={} branch={} path={}",
handle.id(), handle.terminalId(), session.profile(), session.branch(), session.worktree());
notifyAcquired(session.terminalId());
@@ -1,10 +1,13 @@
package dev.ltms.fleet.mcp;
import dev.ltms.fleet.auth.CallerResolver;
import dev.ltms.fleet.auth.MemberRegistry;
import dev.ltms.fleet.auth.Principal;
import dev.ltms.fleet.config.FleetConfig;
import dev.ltms.fleet.guard.SubscriptionGuard;
import dev.ltms.fleet.herdr.AgentControl;
import dev.ltms.fleet.herdr.FakeHerdr;
import dev.ltms.fleet.herdr.PaneLocator;
import dev.ltms.fleet.herdr.WorkspaceControl;
import dev.ltms.fleet.inject.Injector;
import dev.ltms.fleet.msg.MessageService;
@@ -1024,6 +1027,87 @@ class FleetMcpTest {
assertTrue(textOf(res).contains("architect, dev, reviewer"), textOf(res));
}
// ── CB-619 / fleetd #123: a spawn asking for a role its profile has no slot for must be
// refused, never silently demoted with the roster still lying about it ───────────────────
/** Two profiles on one launcher, so a role's pool can name one and exclude the other. */
private static ClaudeCodeLauncher architectCapableLauncher(FakeHerdr h) {
FleetConfig.Profile opus = new FleetConfig.Profile(
"opus", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null,
"tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
FleetConfig.Profile sonnet = new FleetConfig.Profile(
"sonnet", "http://gx00.gw:8000", "coder", null, "FLEETD_WORKER_TOKEN", null,
"tab", "fleetd-workers", "worker: {profile} #{n}", null, null, null);
return new ClaudeCodeLauncher(new AgentControl(h), new WorkspaceControl(h),
new SubscriptionGuard(Set.of("gx00.gw")), Map.of("opus", opus, "sonnet", sonnet),
"sonnet", _ -> "tok");
}
/** {@code fleet.architects} carries only {@code opus} — {@code sonnet} has no matching slot. */
private static MemberRegistry architectRegistry() {
return new MemberRegistry(new FleetConfig.Fleet(Map.of(),
Map.of("opus", new FleetConfig.Slot("opus")), Map.of(), Map.of(), null));
}
/**
* The literal defect (fleetd #123): {@code role=architect, profile=sonnet}, where
* {@code fleet.architects} carries only {@code opus}. Drives the real path —
* {@link FleetMcp#spawn} calls {@link SessionManager#acquire}, which must refuse before ever
* reaching the real {@link ClaudeCodeLauncher} — never {@link MemberRegistry#bind} called
* directly, which would walk around the gate under test.
*/
@Test
void spawnRefusesAnArchitectWithNoMatchingSlotAndNeverTouchesTheLauncher() {
FakeHerdr h = new FakeHerdr();
SessionManager sessions = new SessionManager(architectCapableLauncher(h));
sessions.setMemberLifecycle(architectRegistry());
McpSchema.CallToolResult res = FleetMcp.spawn(sessions, "sonnet", "architect",
null, null, null, null, null, null);
assertEquals(Boolean.TRUE, res.isError(), textOf(res));
String msg = textOf(res);
assertTrue(msg.contains("architect"), "names the role asked for: " + msg);
assertTrue(msg.contains("sonnet"), "names the profile: " + msg);
assertTrue(msg.contains("opus"), "names the pool that does carry the role: " + msg);
assertTrue(sessions.roster().isEmpty(), "a refused spawn must register no session");
assertTrue(h.calls.stream().noneMatch(c -> c.method().equals("agent.start")),
"a refused spawn must never reach the launcher — no process should ever start");
}
/**
* Positive control / parity check: when the profile DOES carry a slot, the spawn succeeds, and
* {@code GET /members} ({@link FleetMcp#listFleet}) and {@code fleet_whoami}
* ({@link FleetMcp#whoami}) — resolved through the SAME live {@link MemberRegistry} binding via
* a real {@link CallerResolver}, exactly as the daemon resolves a real MCP caller — must never
* disagree about this one live member's role.
*/
@Test
void rosterAndWhoamiAgreeOnceTheArchitectSlotBinds() {
FakeHerdr h = new FakeHerdr();
// Pin the spawn onto the one pane FakeHerdr's canned pane.process_info maps to WORKER_PID,
// so a CallerResolver can resolve THIS session's own terminal, not a fixture double.
h.pinNextStarts(1, "term_a", "w2:p7");
SessionManager sessions = new SessionManager(architectCapableLauncher(h));
MemberRegistry members = architectRegistry();
sessions.setMemberLifecycle(members);
McpSchema.CallToolResult spawnRes = FleetMcp.spawn(sessions, "opus", "architect",
null, null, null, null, null, null);
assertNotEquals(Boolean.TRUE, spawnRes.isError(), textOf(spawnRes));
assertTrue(textOf(spawnRes).contains("\"role\":\"architect\""), textOf(spawnRes));
String roster = textOf(FleetMcp.listFleet(architectCapableLauncher(h), sessions, Map.of(), ""));
assertTrue(roster.contains("\"role\":\"architect\""), "GET /members: " + roster);
ConnectionIdentity identity = new ConnectionIdentity(new PaneLocator(h), _ -> FakeHerdr.WORKER_PID);
CallerResolver resolver = CallerResolver.withLeadsAndMembers(identity, false, null, Map::of, members);
Principal caller = resolver.resolve("127.0.0.1", 42, null);
assertTrue(caller.isArchitect(), "fleet_whoami's own resolver must agree the terminal is bound");
String whoami = textOf(FleetMcp.whoami(caller, sessions));
assertTrue(whoami.contains("\"role\":\"architect\""), "fleet_whoami: " + whoami);
}
// ── CB-584: fleet_spawn accepts sessionName/resumeSessionId; roster shows agentSessionId ──
@Test
@@ -114,6 +114,68 @@ class EnvAllowListScrubTest {
assertNull(EnvAllowListScrub.readReport(dir));
}
/**
* fleetd #224 criterion 5 (a gap the #221 reviewer flagged): {@link EnvAllowListScrub#shareWithGroup}
* lists one FLAT level of {@code dir} and shares every file it finds there — which does cover the
* OPTIONAL files a caller may or may not have written before calling it ({@code member-charter.md},
* {@code ide-rules.md} — both written by {@code OpenCodeLauncher#writeConfig}), but nothing
* pinned that down.
* Without this test, a future change that writes a file AFTER the sharing call, or into a
* subdirectory, would pass every existing test while quietly leaving that file unreadable to a
* different-uid member.
*
* <p>This drives {@code shareWithGroup} directly against a directory holding several flat files —
* not only {@code opencode.json}, but also the two optional ones named above plus a third,
* unrelated file, so the assertion is "every flat file", not "the two files someone thought of".
*/
@Test
void shareWithGroupCoversEveryFlatFileIncludingTheOptionalOnes(@TempDir Path dir) throws Exception {
String group = currentUserGroup();
Files.writeString(dir.resolve("opencode.json"), "{}\n");
Files.writeString(dir.resolve("member-charter.md"), "# charter\n");
Files.writeString(dir.resolve("ide-rules.md"), "# ide rules\n");
Files.writeString(dir.resolve("another-flat-file.txt"), "unrelated\n");
EnvAllowListScrub.shareWithGroup(dir, group);
assertEquals("rwxr-x---", java.nio.file.attribute.PosixFilePermissions.toString(
Files.getPosixFilePermissions(dir)),
"the directory itself must be group-traversable+readable, owner-only writable");
for (String name : List.of("opencode.json", "member-charter.md", "ide-rules.md", "another-flat-file.txt")) {
Path file = dir.resolve(name);
assertEquals("rw-r-----", java.nio.file.attribute.PosixFilePermissions.toString(
Files.getPosixFilePermissions(file)),
name + " must be group-readable, never group-writable");
}
}
/**
* The current process's REAL primary group — resolved via {@code id -gn}, never by reading a
* directory's owning group (fleetd #225: that reads wherever Maven happened to be started from,
* not the process's own group, and the two diverge outside a home checkout). Skips (never fails)
* when {@code id} is unavailable or its primary group cannot be resolved on this host.
*/
private static String currentUserGroup() {
String out;
boolean ok;
try {
Process p = new ProcessBuilder("id", "-gn").redirectErrorStream(true).start();
String raw = new String(p.getInputStream().readAllBytes(), StandardCharsets.UTF_8).trim();
out = raw;
ok = p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) && p.exitValue() == 0 && !raw.isBlank();
} catch (IOException e) {
out = null;
ok = false;
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
out = null;
ok = false;
}
assumeTrue(ok, "cannot resolve this process's real primary group via `id -gn` on this host "
+ "— skipping a POSIX-group-dependent test rather than failing it");
return out;
}
/**
* The same equality, for a shell that is INTERACTIVE but NOT a login shell — the shape herdr
* opens on Linux.
@@ -18,7 +18,6 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -513,11 +512,37 @@ class HerdrPeerLauncherAllowListWiringTest {
+ "java.io.tmpdir, unchanged from before this fix: " + dir);
}
/** The current process's own primary group — resolvable on whatever host runs this test. */
private static String currentUserGroup() throws IOException {
PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class);
assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership");
return view.readAttributes().group().getName();
/**
* The current process's REAL primary group — resolved via {@code id -gn}, never by reading a
* directory's owning group (fleetd #225). Those two coincide only by accident: a directory's
* group is whichever group happened to own the path Maven was started from — {@code staff} in
* a home checkout, {@code wheel} under {@code /private/tmp} on macOS — and the fix-up this test
* exercises then fails for real when the operator is not a member of that borrowed group,
* exactly the case {@code assumeTrue(view != null, ...)} never covered (it only detects a
* filesystem with no POSIX groups at all, not a resolvable-but-wrong one). Skips (never fails)
* when {@code id} is unavailable or its primary group cannot be resolved on this host.
*/
private static String currentUserGroup() {
String out;
boolean ok;
try {
Process p = new ProcessBuilder("id", "-gn").redirectErrorStream(true).start();
try (java.io.BufferedReader r = new java.io.BufferedReader(
new java.io.InputStreamReader(p.getInputStream(), java.nio.charset.StandardCharsets.UTF_8))) {
out = r.lines().collect(java.util.stream.Collectors.joining("\n")).trim();
}
ok = p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) && p.exitValue() == 0 && !out.isBlank();
} catch (IOException e) {
out = null;
ok = false;
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
out = null;
ok = false;
}
assumeTrue(ok, "cannot resolve this process's real primary group via `id -gn` on this host "
+ "— skipping a POSIX-group-dependent test rather than failing it");
return out;
}
/** Spawn once through the real launcher path, capturing every INFO+ line this class logs. */
@@ -23,7 +23,6 @@ import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.List;
import java.util.Map;
@@ -657,11 +656,37 @@ class OpenCodeLauncherTest {
0, System::currentTimeMillis, () -> { }, configRoot, discoveryRoot, null, null, config);
}
/** The current process's own primary group — resolvable on whatever host runs this test. */
private static String currentUserGroup() throws IOException {
PosixFileAttributeView view = Files.getFileAttributeView(Path.of("."), PosixFileAttributeView.class);
assumeTrue(view != null, "this host's filesystem does not support POSIX group ownership");
return view.readAttributes().group().getName();
/**
* The current process's REAL primary group — resolved via {@code id -gn}, never by reading a
* directory's owning group (fleetd #225). Those two coincide only by accident: a directory's
* group is whichever group happened to own the path Maven was started from — {@code staff} in
* a home checkout, {@code wheel} under {@code /private/tmp} on macOS — and the fix-up this test
* exercises then fails for real when the operator is not a member of that borrowed group,
* exactly the case {@code assumeTrue(view != null, ...)} never covered (it only detects a
* filesystem with no POSIX groups at all, not a resolvable-but-wrong one). Skips (never fails)
* when {@code id} is unavailable or its primary group cannot be resolved on this host.
*/
private static String currentUserGroup() {
String out;
boolean ok;
try {
Process p = new ProcessBuilder("id", "-gn").redirectErrorStream(true).start();
try (java.io.BufferedReader r = new java.io.BufferedReader(
new java.io.InputStreamReader(p.getInputStream(), java.nio.charset.StandardCharsets.UTF_8))) {
out = r.lines().collect(java.util.stream.Collectors.joining("\n")).trim();
}
ok = p.waitFor(5, java.util.concurrent.TimeUnit.SECONDS) && p.exitValue() == 0 && !out.isBlank();
} catch (IOException e) {
out = null;
ok = false;
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
out = null;
ok = false;
}
assumeTrue(ok, "cannot resolve this process's real primary group via `id -gn` on this host "
+ "— skipping a POSIX-group-dependent test rather than failing it");
return out;
}
/**
@@ -1099,4 +1099,82 @@ class GitWorktreesTest {
assertTrue(e.getMessage().contains("cb185-nonexistent-group-zz"),
"exception must name the missing/refused group: " + e.getMessage());
}
// --- fleetd #224: worktreeRoot itself must be group-traversable, established in add() ---------
/**
* {@code add} must make the worktree ROOT itself group-traversable when a group is configured —
* established once here, where the root is created, never at a use site (never inside a
* launcher). A recording runner stands in for chgrp/chmod, the same seam
* {@link #shareWithGroupRunsConfigThenChgrpChmodSetgidPerPath} uses for the per-worktree share.
*/
@Test
void addSharesWorktreeRootWithGroupWhenConfigured(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
Path root = tmp.resolve("wts");
List<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> recordingRunner = cmd -> {
recorded.add(joined(cmd));
return "";
};
GitWorktrees gitWorktrees = new GitWorktrees(root.toString(), "devteam", _ -> {}, recordingRunner);
gitWorktrees.add(repo.toString(), "cb-224-branch", "HEAD");
assertTrue(recorded.contains(List.of("chgrp", "devteam", root.toString())),
"the worktree root itself must be chgrp'd to the configured group: " + recorded);
assertTrue(recorded.contains(List.of("chmod", "g+x", root.toString())),
"the worktree root itself must gain group-execute so a different-uid member can "
+ "traverse into it: " + recorded);
}
/** {@code worktreeGroup} unset (today's only live mode) ⇒ {@code add} spawns no share process
* for the root at all — behaviour must be byte-identical to before fleetd #224. */
@Test
void addSharesNothingForTheRootWhenNoGroupConfigured(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
Path root = tmp.resolve("wts");
List<List<String>> recorded = new java.util.ArrayList<>();
java.util.function.Function<String[], String> recordingRunner = cmd -> {
recorded.add(joined(cmd));
return "";
};
GitWorktrees gitWorktrees = new GitWorktrees(root.toString(), null, _ -> {}, recordingRunner);
gitWorktrees.add(repo.toString(), "cb-224-nogroup", "HEAD");
assertTrue(recorded.isEmpty(), "no group configured must spawn no share process for the "
+ "root at all: " + recorded);
}
/**
* fleetd #224, acceptance criterion 2/3: when the root cannot be made group-traversable — here
* because the configured group does not exist, the same real-failure shape
* {@link #shareWithGroupThrowsNamingTheGroupWhenChgrpFails} drives for the per-worktree share —
* the spawn is refused with a message naming the root, its current mode, and the group. This
* drives the REAL {@code chgrp} (no recording runner), and the refusal happens before {@code git
* worktree add} ever runs, so no partial worktree is left behind either.
*/
@Test
void addRefusesWhenWorktreeRootCannotBeMadeGroupTraversable(@TempDir Path tmp) throws Exception {
Path repo = initRepo(tmp.resolve("repo"));
Path root = tmp.resolve("wts");
GitWorktrees gitWorktrees = new GitWorktrees(root.toString(), "cb224-nonexistent-group-zz");
WorktreeException e = assertThrows(WorktreeException.class,
() -> gitWorktrees.add(repo.toString(), "cb-224-refuse", "HEAD"));
assertTrue(e.getMessage().contains(root.toString()),
"refusal must name the worktree root: " + e.getMessage());
assertTrue(e.getMessage().contains("cb224-nonexistent-group-zz"),
"refusal must name the missing/refused group: " + e.getMessage());
assertTrue(Files.isDirectory(root), "the root is created before the group check runs");
String mode = java.nio.file.attribute.PosixFilePermissions.toString(Files.getPosixFilePermissions(root));
assertTrue(e.getMessage().contains(mode),
"refusal must name the root's current mode (" + mode + "): " + e.getMessage());
try (java.util.stream.Stream<Path> children = Files.list(root)) {
assertTrue(children.findAny().isEmpty(), "no worktree must be left behind under the root: "
+ "the refusal must happen before `git worktree add` ever runs");
}
}
}
@@ -4,6 +4,7 @@ import ch.qos.logback.classic.Level;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.read.ListAppender;
import dev.ltms.fleet.auth.MemberRegistry;
import dev.ltms.fleet.config.FleetConfig;
import dev.ltms.fleet.guard.SubscriptionGuard;
import dev.ltms.fleet.herdr.AgentControl;
@@ -333,6 +334,57 @@ class SessionManagerTest {
}
}
/**
* CB-619 / fleetd #123: {@code requireSlotFor} closes the config-gap case (no slot at all
* carries the profile) before anything spawns, but a profile that DOES carry a slot can still
* lose the bind to a concurrent spawn racing for the same slot. This drives that residual case
* through the REAL path — {@link SessionManager#acquire} against the real {@link
* dev.ltms.fleet.member.ClaudeCodeLauncher} and {@link FakeHerdr} — never {@link
* dev.ltms.fleet.auth.MemberRegistry#bind} directly for the session under test (only the
* precondition uses it, to occupy the slot before the real spawn happens). The session that
* loses the race must be held as a plain {@code dev}, never left claiming {@code architect} in
* the roster, and the daemon log must say so at WARN.
*/
@Test
void aSecondArchitectOnAnAlreadyBoundProfileIsHeldAsDevNotArchitectAndWarnsLoudly() {
LoggerContext ctx = (LoggerContext) LoggerFactory.getILoggerFactory();
ch.qos.logback.classic.Logger registryLog = (ch.qos.logback.classic.Logger)
LoggerFactory.getLogger(MemberRegistry.class);
ListAppender<ILoggingEvent> appender = new ListAppender<>();
appender.setContext(ctx);
appender.start();
registryLog.addAppender(appender);
registryLog.setLevel(Level.WARN);
try {
FakeHerdr herdr = new FakeHerdr();
SessionManager sessions = sessionManager(herdr);
MemberRegistry members = new MemberRegistry(
new FleetConfig.Fleet(Map.of(), Map.of("opus", new FleetConfig.Slot("ltms-local")),
Map.of(), Map.of(), null));
assertTrue(members.bind("architect:opus", "term_already_bound"),
"precondition: occupy the sole architect slot before the real spawn under test");
sessions.setMemberLifecycle(members);
MemberSession session = sessions.acquire("ltms-local", MemberRole.ARCHITECT, null,
"/caller", "term_primary", null);
assertEquals(MemberRole.DEV, session.role(),
"the slot is taken, so this session must be held as a plain member, never a lie");
assertEquals("dev", SessionManager.rosterView(session, null).get("role"),
"the roster must report what this session actually holds, not what it asked for");
String warn = appender.list.stream()
.filter(e -> e.getLevel().equals(Level.WARN))
.map(ILoggingEvent::getFormattedMessage)
.findFirst()
.orElse("no slot-exhaustion WARN logged");
assertTrue(warn.contains("ltms-local"), "the log names the profile: " + warn);
assertTrue(warn.contains(session.terminalId()), "the log names the terminal: " + warn);
} finally {
registryLog.detachAppender(appender);
}
}
@Test
void rosterReflectsAcquiredMinusReleased() {
FakeHerdr herdr = new FakeHerdr();