Compare commits
4 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 5c56cb347f | |||
| dcf5fb3be3 | |||
| f9d2ee2a2b | |||
| 866c7f2e9a |
@@ -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.
|
||||
|
||||
+31
-6
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user