From d9168de43e7040e8bb819f2230504bc8242b6439 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Tue, 1 Sep 2026 14:38:48 +0700 Subject: [PATCH] fleetd#219: OpenCodeLauncher config/discovery roots must not assume fleetd's own filesystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Site 1 (config root): under memberHerdrSocket, writeConfig() now places the ephemeral opencode.json directory under worktreeRoot and shares it read-only with worktreeGroup, reusing EnvAllowListScrub#shareWithGroup (widened to package-private and generalized) — the same mechanism #213 built for the ZDOTDIR scrub, rather than a second copy. Unlike the ZDOTDIR scrub's degrade-to-overlay fallback, a missing worktreeRoot/worktreeGroup here REFUSES the spawn (IllegalStateException from buildLaunch): this file is the member's only way to learn where the bridge MCP is, so writing it somewhere unreadable would just produce an undeliverable member with no signal pointing at the cause. memberHerdrSocket absent stays byte-identical. Site 2 (discovery root): under memberHerdrSocket, agentSessionId() now declares session discovery unavailable and logs one WARN per launcher instance instead of silently scanning fleetd's own $HOME (opencode.db lives under the MEMBER's home under this config key). Decision + reasoning for why this is a declare-unavailable rather than a new config key is in defaultDiscoveryRoot()'s javadoc. Widened HerdrPeerLauncher#memberHerdrSocketConfigured/memberScrubParentDir/ memberGroup to package-private so OpenCodeLauncher reuses the exact same config resolution rather than re-deriving it. Same-shape finding (not fixed, out of scope): ClaudeCodeLauncher#writeCharterFile (line ~465) writes the role-charter temp file via Files.createTempFile with no directory argument, i.e. under java.io.tmpdir — the same site-1 shape, unfixed for the Claude Code adapter. --- .../ltms/fleet/member/EnvAllowListScrub.java | 27 ++- .../ltms/fleet/member/HerdrPeerLauncher.java | 18 +- .../ltms/fleet/member/OpenCodeLauncher.java | 132 +++++++++++- .../fleet/member/OpenCodeLauncherTest.java | 201 ++++++++++++++++++ 4 files changed, 361 insertions(+), 17 deletions(-) diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java index 6480720..b60e1a7 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/EnvAllowListScrub.java @@ -143,17 +143,24 @@ public final class EnvAllowListScrub { } /** - * chgrp/chmod-equivalent over the freshly generated directory and the startup files already - * written into it: owner keeps full access, {@code group} gets traverse+read on the directory - * ({@code rwxr-x---}, so a login shell under that group can find and source the files) and - * read-only on each file ({@code rw-r-----}) — deliberately no group WRITE anywhere, since a - * member never needs to add or change fleetd's own generated scrub. (The scrub script's own - * report write inside the pane consequently fails closed rather than open — see {@code - * scrub.zsh}'s trailing {@code 2>/dev/null} — which {@link + * chgrp/chmod-equivalent over a freshly generated directory and the flat files already written + * into it: owner keeps full access, {@code group} gets traverse+read on the directory ({@code + * rwxr-x---}, so a member process — a login shell reading it via {@code ZDOTDIR}, or another + * process simply opening a file under it — running under that group can find and read the + * files) and read-only on each file ({@code rw-r-----}) — deliberately no group WRITE anywhere, + * since a member never needs to add or change what fleetd generated. (For the ZDOTDIR scrub + * specifically, this also means the scrub script's own report write inside the pane fails + * closed rather than open — see {@code scrub.zsh}'s trailing {@code 2>/dev/null} — which {@link * dev.ltms.fleet.member.HerdrPeerLauncher#releaseZdotdir} already treats as "cannot be * confirmed to have run" rather than success.) + * + *

Package-private and named generically on purpose: fleetd #213 built this for the ZDOTDIR + * scrub directory, and fleetd #219 reuses it verbatim for {@link + * dev.ltms.fleet.member.OpenCodeLauncher}'s ephemeral {@code opencode.json} directory — both are + * "a fleetd-generated directory of flat files that a different-uid member process must read but + * never write," so the sharing mechanism is shared rather than copied a second time. */ - private static void shareWithGroup(Path dir, String group) { + static void shareWithGroup(Path dir, String group) { try { GroupPrincipal principal = dir.getFileSystem().getUserPrincipalLookupService() .lookupPrincipalByGroupName(group); @@ -164,11 +171,11 @@ public final class EnvAllowListScrub { } } } catch (IOException e) { - throw new UncheckedIOException("cannot share generated ZDOTDIR " + dir + " with group '" + throw new UncheckedIOException("cannot share generated directory " + dir + " with group '" + group + "' — the group must exist, and the fleetd operator (" + System.getProperty("user.name") + ") must be a member of it", e); } catch (UnsupportedOperationException e) { - throw new UncheckedIOException("cannot share generated ZDOTDIR " + dir + " with group '" + throw new UncheckedIOException("cannot share generated directory " + dir + " with group '" + group + "' — this filesystem does not support POSIX group ownership", new IOException(e)); } diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java index 88cbcf6..4f4f4ee 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/HerdrPeerLauncher.java @@ -1275,8 +1275,13 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * provisioning a worktree that will exist regardless, whereas an unconfigured value here means * fleetd has no operator-endorsed location to put a credential-bearing directory a different OS * user must reach, so falling back to the overlay is the honest answer, not a guess. + * + *

Package-private (fleetd #219) so {@link OpenCodeLauncher} can reuse the exact same + * "different OS user, put it under worktreeRoot instead of java.io.tmpdir" resolution for its + * own ephemeral {@code opencode.json} directory, rather than re-reading {@code config} a second + * time with a second copy of this null/blank handling. */ - private Path memberScrubParentDir() { + Path memberScrubParentDir() { FleetConfig cfg = config == null ? null : config.get(); if (cfg == null || cfg.worktreeRoot() == null || cfg.worktreeRoot().isBlank()) { return null; @@ -1289,8 +1294,11 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * the group {@link dev.ltms.fleet.session.Worktrees#shareWithGroup} already establishes for * provisioned worktrees, rather than a second group key — see {@link * #applyEnvironmentAllowListPolicy}. + * + *

Package-private (fleetd #219) — reused by {@link OpenCodeLauncher} alongside {@link + * #memberScrubParentDir()}; see that method's javadoc. */ - private String memberGroup() { + String memberGroup() { FleetConfig cfg = config == null ? null : config.get(); if (cfg == null || cfg.worktreeGroup() == null || cfg.worktreeGroup().isBlank()) { return null; @@ -1463,8 +1471,12 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { * through (every production {@code HerdrPeerLauncher} does; a handful of older tests do not) — * treated the same as "not configured", which is the correct, permissive default: it is exactly * today's single-daemon behaviour. + * + *

Package-private (fleetd #219) — {@link OpenCodeLauncher} reuses this same gate to decide + * where its own ephemeral {@code opencode.json} directory (site 1) and its opencode session + * discovery (site 2) may run, rather than re-deriving "is this a multi-uid fleet" a second way. */ - private boolean memberHerdrSocketConfigured() { + boolean memberHerdrSocketConfigured() { if (config == null) { return false; } diff --git a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java index 7f75538..7cc8187 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java +++ b/fleetd/src/main/java/dev/ltms/fleet/member/OpenCodeLauncher.java @@ -20,6 +20,8 @@ import java.util.EnumSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.BooleanSupplier; import java.util.function.Function; import java.util.function.LongSupplier; import java.util.function.Supplier; @@ -215,7 +217,43 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { return Path.of(System.getProperty("java.io.tmpdir")); } - /** The default opencode storage root: {@code ~/.local/share/opencode} (the XDG data dir). */ + /** + * The default opencode storage root: {@code ~/.local/share/opencode} (the XDG data dir) — + * always FLEETD's OWN {@code user.home}, whichever OS user runs the daemon. + * + *

fleetd #219 site 2 — a decision, not a patch. Under {@code memberHerdrSocket:} the + * member pane runs as a different OS user, and opencode writes {@code opencode.db} + * under that user's {@code $HOME}, not fleetd's. Scanning fleetd's own {@code + * user.home} is therefore looking in the wrong place — a wrong-LOCATION failure, not a + * wrong-PERMISSION one like site 1, and it fails quietly: {@link + * SessionAwareHandle#agentSessionId()} would keep returning {@code null} forever, which reads + * as "opencode does not support resume" rather than "fleetd looked in the wrong home." fleetd + * #209 is the reason that silence is unacceptable. + * + *

Three ways to close the gap were weighed: + *

    + *
  1. Make the member's home configurable. Correct in principle, but this ticket's + * scope is the two existing call sites, not a new config key — {@code memberHerdrSocket} + * already carries the second herdr's socket path, not its user's home, and inventing a + * parallel key here without also wiring it through discovery's actual callers is a + * half-shipped feature (the exact shape CB-596/CB-611 warn against).
    2. + *
  2. Derive it (e.g. from {@code worktreeRoot}'s owner, or {@code getent passwd}). + * Rejected: nothing in this codebase resolves a Unix username to a home directory today, + * and guessing wrong would silently point discovery at a THIRD wrong location — worse + * than the current gap, because it would look like it should work.
    3. + *
  3. Declare discovery unavailable under {@code memberHerdrSocket}, and say so once, + * loudly, instead of scanning a directory that structurally cannot hold the answer.
    4. + *
+ * + *

Option 3 is taken — the one this ticket says to default to when unsure. {@link + * OpenCodeLauncher#spawn} routes {@link SessionAwareHandle#agentSessionId()} through {@link + * HerdrPeerLauncher#memberHerdrSocketConfigured()} before ever calling {@link + * OpenCodeSessionDiscovery#sessionIdForDirectory}, so under {@code memberHerdrSocket} the + * database at this root is never even opened, and one WARN per launcher instance names the gap + * instead of the {@code null} return reading as "unsupported." Capability advertising is + * unaffected: {@link #capabilities()} always includes {@code SESSION_RESUME}, since {@code + * memberHerdrSocket} absent (today's only live mode) is unchanged by this decision. + */ private static Path defaultDiscoveryRoot() { return Path.of(System.getProperty("user.home"), ".local", "share", "opencode"); } @@ -331,7 +369,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { */ private Path writeConfig(FleetConfig.Profile cfg, String charterText, String cwd) { try { - Path dir = Files.createTempDirectory(configRoot, "fleetd-opencode-"); + Path dir = Files.createTempDirectory(configParentDir(), "fleetd-opencode-"); dir.toFile().deleteOnExit(); ObjectNode root = JSON.createObjectNode(); @@ -402,6 +440,14 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { // carries operator-supplied values (URL, model id, api key), so escaping must be real. Files.writeString(cfgFile, JSON.writerWithDefaultPrettyPrinter().writeValueAsString(root)); cfgFile.toFile().deleteOnExit(); + if (memberHerdrSocketConfigured()) { + // fleetd #219: the same "different OS user" gap fleetd #213 closed for the ZDOTDIR + // scrub — share read-only with worktreeGroup rather than leaving the directory under + // fleetd's own 0700 java.io.tmpdir, where the member's OS user could not even + // traverse it. memberGroup() cannot be null here: configParentDir() above already + // refused this spawn if either worktreeRoot or worktreeGroup was missing. + EnvAllowListScrub.shareWithGroup(dir, memberGroup()); + } return cfgFile; } catch (IOException e) { throw new UncheckedIOException( @@ -409,6 +455,57 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { } } + /** + * fleetd #219 site 1: where {@link #writeConfig} creates its per-spawn directory. + * + *

+ * + *

Unlike the ZDOTDIR scrub, a missing {@code worktreeRoot}/{@code worktreeGroup} here + * REFUSES the spawn instead of degrading. The ZDOTDIR scrub is a credential CONTROL: a + * degraded control (CB-596's sentinel overlay) is still worth having. This config file is not a + * control — it is the ONLY way the member learns where the bridge MCP lives. Writing it + * somewhere the member cannot read would not degrade anything; it would spawn a member that + * occupies a pane and never becomes deliverable, since {@code fleet_send} waits ~60s on the + * readiness gate and then fails with nothing pointing at a temp directory as the cause. Refusing + * up front, with a message that names the missing config key, is the honest failure — an + * undeliverable member is not a working spawn either way, so nothing is lost by refusing loudly + * instead of failing silently later. + * + * @throws IllegalStateException when {@code memberHerdrSocket} is configured but {@code + * worktreeRoot} and/or {@code worktreeGroup} is not + */ + private Path configParentDir() { + if (!memberHerdrSocketConfigured()) { + return configRoot; + } + Path root = memberScrubParentDir(); + String group = memberGroup(); + if (root == null || group == null) { + throw new IllegalStateException("memberHerdrSocket is configured, so opencode's config " + + "directory (opencode.json + member charter) must be placed where the member's " + + "OS user can read it — worktreeRoot, shared via worktreeGroup — but " + + (root == null ? "worktreeRoot" : "worktreeGroup") + " is not configured. " + + "Refusing to spawn rather than write a config the member cannot read: that " + + "member would occupy a pane and never become deliverable, with nothing " + + "pointing at the real cause. Configure both worktreeRoot and worktreeGroup to " + + "enable opencode member spawns under memberHerdrSocket."); + } + return root; + } + /** * Declare a custom OpenAI-compatible provider so the worker talks to a pinned endpoint (a local * vLLM, say) instead of opencode's default gateway (CB-508). @@ -517,11 +614,16 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { return afterScheme.contains("/") ? trimmed : trimmed + "/v1"; } + /** One WARN per launcher instance for the fleetd #219 site-2 discovery-unavailable gap. */ + private final AtomicBoolean discoveryUnavailableWarned = + new AtomicBoolean(); + /** Add lazy on-disk session discovery to the base handle. */ @Override public PeerHandle spawn(SpawnRequest req) { PeerHandle inner = super.spawn(req); - return new SessionAwareHandle(inner, discovery, effectiveCwd(req)); + return new SessionAwareHandle(inner, discovery, effectiveCwd(req), + this::memberHerdrSocketConfigured, discoveryUnavailableWarned); } /** @@ -536,11 +638,17 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { private final PeerHandle delegate; private final OpenCodeSessionDiscovery discovery; private final String cwd; + private final BooleanSupplier discoveryUnavailable; + private final AtomicBoolean discoveryUnavailableWarned; - SessionAwareHandle(PeerHandle delegate, OpenCodeSessionDiscovery discovery, String cwd) { + SessionAwareHandle(PeerHandle delegate, OpenCodeSessionDiscovery discovery, String cwd, + BooleanSupplier discoveryUnavailable, + AtomicBoolean discoveryUnavailableWarned) { this.delegate = delegate; this.discovery = discovery; this.cwd = cwd; + this.discoveryUnavailable = discoveryUnavailable; + this.discoveryUnavailableWarned = discoveryUnavailableWarned; } @Override @@ -565,6 +673,22 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { @Override public String agentSessionId() { + // fleetd #219 site 2: under memberHerdrSocket the member pane runs as a different OS + // user, so opencode.db lives under THAT user's $HOME, not the one discoveryRoot was + // built from (see OpenCodeLauncher#defaultDiscoveryRoot's javadoc for the full + // reasoning). Scanning fleetd's own $HOME under that config would only ever find "no + // row" and read as "resume unsupported" — declare it unavailable instead, once, loudly. + if (discoveryUnavailable.getAsBoolean()) { + if (discoveryUnavailableWarned.compareAndSet(false, true)) { + log.warn("opencode session discovery unavailable: memberHerdrSocket is " + + "configured, so opencode's on-disk session database lives under the " + + "MEMBER's own $HOME, not fleetd's ({}) — agentSessionId will stay null " + + "for every opencode member under this config, and SESSION_RESUME " + + "cannot be honored (fleetd #209/#219).", + System.getProperty("user.home")); + } + return null; + } // Lazy + retried, never a spawn-time blocker: opencode writes the session record only // when the session is first persisted, so null here is the correct interim answer and // the caller re-calls later (each call re-scans, picking up a record that has since diff --git a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java index 29878f6..d1028fe 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/member/OpenCodeLauncherTest.java @@ -1,5 +1,9 @@ package dev.ltms.fleet.member; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import dev.ltms.fleet.config.FleetConfig; @@ -14,9 +18,13 @@ import dev.ltms.fleet.peer.PeerUnreachableException; import dev.ltms.fleet.peer.SpawnRequest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.slf4j.LoggerFactory; +import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.attribute.PosixFileAttributeView; +import java.nio.file.attribute.PosixFilePermissions; import java.util.List; import java.util.Map; import java.util.concurrent.ExecutorService; @@ -25,6 +33,7 @@ import java.util.concurrent.Future; import java.util.function.Supplier; import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assumptions.assumeTrue; /** * The opencode adapter's launch build: a file-based MCP mount + reply-charter instructions (no @@ -610,4 +619,196 @@ class OpenCodeLauncherTest { assertTrue(json.path("mcp").path("intellij").isMissingNode(), "no IDE server when ideMcpUrl is unset"); } + + // --- fleetd #219: config root + discovery root under memberHerdrSocket ------------------------ + + /** A config with {@code memberHerdrSocket:} set, and optionally {@code worktreeRoot:}/{@code worktreeGroup:}. */ + private static FleetConfig configWithMemberHerdrSocket(String worktreeRoot, String worktreeGroup) { + return new FleetConfig( + null, // bind + null, // herdrSocket + "/tmp/other-user.sock", // memberHerdrSocket + Map.of(), // profiles + null, // guard + worktreeRoot, // worktreeRoot + null, // lifecycle + null, // spawnReadyTimeoutMs + null, // spawnReadyPollMs + null, // broker + null, // primary + null, // fleet + null, // leadHeartbeat + null, // health + null, // placement + null, // auth + null, // configReload + null, // quarantineCooldownSeconds + null, // memberCredentials + null, // coordinator + worktreeGroup, // worktreeGroup + null // memberLoginShell + ).withDefaults(); + } + + private static OpenCodeLauncher serviceWithConfig(FakeHerdr herdr, Path configRoot, Path discoveryRoot, + FleetConfig.Profile cfg, Supplier config) { + return new OpenCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null, + 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(); + } + + /** + * fleetd #219 site 1, acceptance criterion 1: with {@code memberHerdrSocket} configured and both + * {@code worktreeRoot}/{@code worktreeGroup} set, the generated {@code opencode.json} directory + * lives under {@code worktreeRoot} — NEVER under the injected {@code configRoot} (standing in for + * {@code java.io.tmpdir}, fleetd's own 0700 temp dir, unreadable by the member's different OS + * user) — and is shared read-only with the group via the SAME mechanism (fleetd #213's {@link + * EnvAllowListScrub#shareWithGroup}) the ZDOTDIR scrub uses. + */ + @Test + void memberHerdrSocketWithWorktreeRootAndGroupPutsConfigDirUnderWorktreeRootAndSharesIt( + @TempDir Path configRoot, @TempDir Path worktreeRoot) throws Exception { + String group = currentUserGroup(); + FakeHerdr herdr = new FakeHerdr(); + serviceWithConfig(herdr, configRoot, configRoot, + opencodeCfg("google/gemini-2.5-pro", "http://127.0.0.1:8765/mcp", null), + () -> configWithMemberHerdrSocket(worktreeRoot.toString(), group)).spawn(); + + String cfgPath = startEnv(herdr).get("OPENCODE_CONFIG"); + assertNotNull(cfgPath, "the profile still needs a config file"); + Path cfgFile = Path.of(cfgPath); + Path dir = cfgFile.getParent(); + assertEquals(worktreeRoot.toAbsolutePath().normalize(), dir.getParent(), + "the generated directory's parent must be worktreeRoot, not the injected configRoot " + + "standing in for java.io.tmpdir — got parent " + dir.getParent()); + assertFalse(dir.startsWith(configRoot), + "the generated directory must NOT be created under configRoot when memberHerdrSocket " + + "is configured: " + dir); + + assertEquals("rwxr-x---", PosixFilePermissions.toString(Files.getPosixFilePermissions(dir)), + "the directory must be group-traversable+readable, owner-only writable"); + assertEquals("rw-r-----", PosixFilePermissions.toString(Files.getPosixFilePermissions(cfgFile)), + "opencode.json must be group-readable, never group-writable"); + } + + /** + * fleetd #219 site 1, acceptance criterion 2: with {@code memberHerdrSocket} configured but + * NEITHER {@code worktreeRoot} nor {@code worktreeGroup} set, the launcher must refuse the spawn + * rather than write a config under {@code java.io.tmpdir} the member cannot read — that member + * would occupy a pane and never become deliverable, with nothing pointing at the real cause. + */ + @Test + void memberHerdrSocketWithoutWorktreeRootOrGroupRefusesTheSpawn(@TempDir Path configRoot) { + FakeHerdr herdr = new FakeHerdr(); + OpenCodeLauncher launcher = serviceWithConfig(herdr, configRoot, configRoot, + opencodeCfg("google/gemini-2.5-pro", "http://127.0.0.1:8765/mcp", null), + () -> configWithMemberHerdrSocket(null, null)); + + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> launcher.spawn(new SpawnRequest(null, null, null)), + "a missing worktreeRoot/worktreeGroup must refuse the spawn, not write an unreadable config"); + assertTrue(ex.getMessage().contains("worktreeRoot"), + "the refusal must name the missing config key — got: " + ex.getMessage()); + assertFalse(herdr.called("tab.create"), + "the spawn must be refused BEFORE any pane is created — got calls: " + herdr.calls); + } + + /** + * fleetd #219 site 1, acceptance criterion 2 (the other missing half): {@code worktreeRoot} set + * but {@code worktreeGroup} missing must ALSO refuse — either one alone is not enough to + * guarantee the member's OS user can read the directory. + */ + @Test + void memberHerdrSocketWithWorktreeRootButNoGroupRefusesTheSpawn( + @TempDir Path configRoot, @TempDir Path worktreeRoot) { + FakeHerdr herdr = new FakeHerdr(); + OpenCodeLauncher launcher = serviceWithConfig(herdr, configRoot, configRoot, + opencodeCfg("google/gemini-2.5-pro", "http://127.0.0.1:8765/mcp", null), + () -> configWithMemberHerdrSocket(worktreeRoot.toString(), null)); + + IllegalStateException ex = assertThrows(IllegalStateException.class, + () -> launcher.spawn(new SpawnRequest(null, null, null))); + assertTrue(ex.getMessage().contains("worktreeGroup"), + "worktreeRoot alone is not enough — got: " + ex.getMessage()); + } + + /** + * fleetd #219 site 1, acceptance criterion 3: with {@code memberHerdrSocket} ABSENT — even when + * a live, non-null {@code config} supplier is threaded through (not merely {@code config == null}, + * which every other test in this file already exercises) — the generated directory must still + * land directly under the injected {@code configRoot}, byte-identical to before this fix. + */ + @Test + void memberHerdrSocketAbsentStaysUnderConfigRootEvenWithALiveConfigSupplier(@TempDir Path configRoot) + throws Exception { + FakeHerdr herdr = new FakeHerdr(); + FleetConfig config = new FleetConfig(null, null, null, Map.of(), null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, null, null, null).withDefaults(); + serviceWithConfig(herdr, configRoot, configRoot, + opencodeCfg("google/gemini-2.5-pro", "http://127.0.0.1:8765/mcp", null), () -> config) + .spawn(); + + String cfgPath = startEnv(herdr).get("OPENCODE_CONFIG"); + assertNotNull(cfgPath); + assertTrue(Path.of(cfgPath).startsWith(configRoot), + "with memberHerdrSocket absent, the config directory must still be created directly " + + "under configRoot, unchanged from before this fix"); + } + + /** + * fleetd #219 site 2: under {@code memberHerdrSocket}, opencode session discovery must be + * declared unavailable rather than silently scanning fleetd's own {@code discoveryRoot} — which, + * under this config key, is NOT where the member's opencode actually writes its session + * database. This test proves the gate is real, not merely "no record yet": a matching record IS + * written to {@code discoveryRoot} (the exact fixture {@link + * #theHandleDiscoversTheSessionIdForTheWorkersCwdOnlyAfterItAppears} proves discovery would + * otherwise find), and {@code agentSessionId()} must still return {@code null} — proving the + * gate, not a coincidental absence of data, is what produced the null. One WARN is also logged, + * exactly once even across repeated calls. + */ + @Test + void discoveryIsUnavailableUnderMemberHerdrSocketEvenWhenARecordExists( + @TempDir Path configRoot, @TempDir Path worktreeRoot, @TempDir Path discRoot) throws Exception { + String group = currentUserGroup(); + OpenCodeSessionDiscoveryTest.writeRecord(discRoot, "ses_should_be_hidden", "/work/dir", 1000L); + + FakeHerdr herdr = new FakeHerdr(); + OpenCodeLauncher launcher = new OpenCodeLauncher(new AgentControl(herdr), + new WorkspaceControl(herdr), Map.of("gemini", opencodeCfg(null, null, null)), + "gemini", _ -> null, 0, System::currentTimeMillis, () -> { }, configRoot, discRoot, + null, null, () -> configWithMemberHerdrSocket(worktreeRoot.toString(), group)); + + Logger logger = (Logger) LoggerFactory.getLogger(OpenCodeLauncher.class); + Level original = logger.getLevel(); + logger.setLevel(Level.INFO); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + PeerHandle handle; + try { + handle = launcher.spawn(new SpawnRequest(null, "/work/dir", null)); + assertNull(handle.agentSessionId(), + "memberHerdrSocket configured: discovery must stay unavailable even though a " + + "matching record exists in discoveryRoot"); + assertNull(handle.agentSessionId(), "the gate must hold on a second call too"); + } finally { + logger.detachAppender(appender); + logger.setLevel(original); + } + List warnings = appender.list.stream() + .filter(e -> e.getLevel() == Level.WARN) + .map(ILoggingEvent::getFormattedMessage) + .toList(); + assertEquals(1, warnings.size(), + "exactly one WARN across two agentSessionId() calls — got: " + warnings); + assertTrue(warnings.get(0).contains("memberHerdrSocket"), + "the WARN must name memberHerdrSocket as the reason — got: " + warnings.get(0)); + } } -- 2.52.0