OpenCodeLauncher decides a member's config and discovery roots from fleetd's own filesystem — the #213 defect, twice #219

Closed
opened 2026-09-01 08:27:55 +02:00 by ltms · 1 comment
Owner

Found by the #213 worker while fixing the ZDOTDIR scrub, and reported as out of scope. I checked both in the code on main.

Same shape as #213: fleetd reads its own process's filesystem to decide something about a member. Correct today, wrong the moment memberHerdrSocket: puts member panes under a different OS user.

The two sites

OpenCodeLauncher.java:

private static Path defaultConfigRoot() {
    return Path.of(System.getProperty("java.io.tmpdir"));
}

/** The default opencode storage root: {@code ~/.local/share/opencode} (the XDG data dir). */
private static Path defaultDiscoveryRoot() {
    return Path.of(System.getProperty("user.home"), ".local", "share", "opencode");
}

Both resolve against fleetd's process, not the member's.

Why the config root is worse than #213

configRoot is where the launcher writes the ephemeral opencode.json — the remote MCP server block and the member charter — and then points the member at it. On macOS java.io.tmpdir is the per-user $TMPDIR under /var/folders/..., mode drwx------ (0700).

A member running as a different uid cannot traverse that directory. So it does not get a degraded credential control the way #213 does — it gets a member that cannot read the config telling it where the bridge is. That is a broken launch, and per invariant 4 the failure would present as a member that never becomes deliverable: the send waits ~60s on the readiness gate and then fails without ever reaching the pane. Nothing in that signal points at a temp directory.

The fix is the one #213 already landed: when memberHerdrSocket: is configured, generate under the configured worktreeRoot and share with worktreeGroup:, and fall back rather than write somewhere the member cannot read. EnvAllowListScrub.generate(parentDir, allowed, group) is already there and does exactly this — reuse it rather than writing a second copy.

The discovery root is a different problem, not the same one

defaultDiscoveryRoot() reads user.home to find opencode.db. Under memberHerdrSocket: the member's opencode writes into its own user's ~/.local/share/opencode, not fleetd's — so discovery would look in the wrong home and find nothing.

That is not a permission failure, it is a wrong-location failure, and it fails quietly in a way #209 just made visible: agentSessionId would simply stay null forever again, which reads as "this backend does not support resume" rather than "fleetd looked in the wrong home directory". Whoever takes this must decide whether the member user's home is configurable, derivable, or whether discovery is simply unavailable under memberHerdrSocket: — and say which, in the code.

Not live today

memberHerdrSocket: is absent on this host, so neither site is currently wrong. This is a blocker for that rollout, alongside #213.

Acceptance

  • With memberHerdrSocket: set, the opencode config root is not under java.io.tmpdir, and is created with the same group sharing #213 established.
  • With memberHerdrSocket: set and worktreeRoot/worktreeGroup missing, the launcher does not write a config the member cannot read — decide and document whether that is a refused spawn or a spawn without the MCP mount, and make the log say which.
  • With memberHerdrSocket: absent, both paths are byte-identical to today.
  • The discovery-root decision is recorded in the javadoc with its reasoning, whichever way it goes.
  • Watch each test fail before keeping it.

Related: #213 (the same defect in the ZDOTDIR scrub, fixed in e1eb50c), #185 (parent), #209 (why a null agentSessionId is not a safe silent default).

Found by the #213 worker while fixing the ZDOTDIR scrub, and reported as out of scope. I checked both in the code on `main`. Same shape as #213: **fleetd reads its own process's filesystem to decide something about a member.** Correct today, wrong the moment `memberHerdrSocket:` puts member panes under a different OS user. ## The two sites `OpenCodeLauncher.java`: ```java private static Path defaultConfigRoot() { return Path.of(System.getProperty("java.io.tmpdir")); } /** The default opencode storage root: {@code ~/.local/share/opencode} (the XDG data dir). */ private static Path defaultDiscoveryRoot() { return Path.of(System.getProperty("user.home"), ".local", "share", "opencode"); } ``` Both resolve against **fleetd's** process, not the member's. ## Why the config root is worse than #213 `configRoot` is where the launcher writes the ephemeral `opencode.json` — the remote MCP server block and the member charter — and then points the member at it. On macOS `java.io.tmpdir` is the per-user `$TMPDIR` under `/var/folders/...`, mode `drwx------` (0700). A member running as a different uid cannot traverse that directory. So it does not get a degraded credential control the way #213 does — **it gets a member that cannot read the config telling it where the bridge is.** That is a broken launch, and per invariant 4 the failure would present as a member that never becomes deliverable: the send waits ~60s on the readiness gate and then fails without ever reaching the pane. Nothing in that signal points at a temp directory. The fix is the one #213 already landed: when `memberHerdrSocket:` is configured, generate under the configured `worktreeRoot` and share with `worktreeGroup:`, and fall back rather than write somewhere the member cannot read. `EnvAllowListScrub.generate(parentDir, allowed, group)` is already there and does exactly this — reuse it rather than writing a second copy. ## The discovery root is a different problem, not the same one `defaultDiscoveryRoot()` reads `user.home` to find `opencode.db`. Under `memberHerdrSocket:` the member's opencode writes into **its own** user's `~/.local/share/opencode`, not fleetd's — so discovery would look in the wrong home and find nothing. That is not a permission failure, it is a wrong-location failure, and it fails quietly in a way #209 just made visible: `agentSessionId` would simply stay `null` forever again, which reads as "this backend does not support resume" rather than "fleetd looked in the wrong home directory". Whoever takes this must decide whether the member user's home is configurable, derivable, or whether discovery is simply unavailable under `memberHerdrSocket:` — and say which, in the code. ## Not live today `memberHerdrSocket:` is absent on this host, so neither site is currently wrong. This is a blocker for that rollout, alongside #213. ## Acceptance - With `memberHerdrSocket:` set, the opencode config root is **not** under `java.io.tmpdir`, and is created with the same group sharing #213 established. - With `memberHerdrSocket:` set and `worktreeRoot`/`worktreeGroup` missing, the launcher does not write a config the member cannot read — decide and document whether that is a refused spawn or a spawn without the MCP mount, and make the log say which. - With `memberHerdrSocket:` absent, both paths are byte-identical to today. - The discovery-root decision is recorded in the javadoc with its reasoning, whichever way it goes. - Watch each test fail before keeping it. Related: #213 (the same defect in the ZDOTDIR scrub, fixed in `e1eb50c`), #185 (parent), #209 (why a null `agentSessionId` is not a safe silent default).
Author
Owner

Fixed by PR #221, merged to main.

Both sites are closed the way the ticket asked:

  • Config root — under memberHerdrSocket: the ephemeral directory goes under worktreeRoot and is shared with worktreeGroup: through EnvAllowListScrub#shareWithGroup, the same mechanism #213 built, widened to package-private and reused rather than copied. A missing worktreeRoot/worktreeGroup refuses the spawn instead of degrading, because this file is the member's only way to learn where the bridge MCP is.
  • Discovery root — declared unavailable under memberHerdrSocket:, with one WARN naming the gap, rather than scanning a home directory that structurally cannot hold the answer. The javadoc records all three options that were weighed and why this one was taken. No new config key was invented.

What I checked myself, rather than taking the report:

  • mvn clean install on the branch: BUILD SUCCESS, 1086 tests, 0 failures, 0 errors (main was 1081).
  • The memberHerdrSocket-absent path — the criterion that protects the running daemon, since the key is unset here. configParentDir() returns the old configRoot untouched, and both other new branches sit behind the same check.
  • The thing the report could not settle: shareWithGroup(dir, …) runs after all three files land in the directory (member-charter.md, ide-rules.md, opencode.json), and it walks one flat level, so every file is covered. This was the way the fix could have been quietly wrong.

A reviewer on a separate backend answered the same four questions independently and found no defect in the merged change. It made one correction I am recording here: the absent path is not instruction-byte-identical, because it now performs a ConfigRef.get() read in two places. That read is atomic and produces no changed result or side effect when the key is unset, so the behavioural claim stands.

Two follow-ups came out of this, both filed rather than folded in:

  • #222 — ClaudeCodeLauncher#writeCharterFile has the same site-1 shape, and #220 made that file the only charter delivery path, so it matters more than it looks.
  • #224 — worktreeRoot itself is never made group-traversable. That is the floor this fix stands on, and it belongs to GitWorktrees, not to the launcher.
Fixed by PR #221, merged to `main`. Both sites are closed the way the ticket asked: - **Config root** — under `memberHerdrSocket:` the ephemeral directory goes under `worktreeRoot` and is shared with `worktreeGroup:` through `EnvAllowListScrub#shareWithGroup`, the same mechanism #213 built, widened to package-private and reused rather than copied. A missing `worktreeRoot`/`worktreeGroup` **refuses the spawn** instead of degrading, because this file is the member's only way to learn where the bridge MCP is. - **Discovery root** — declared unavailable under `memberHerdrSocket:`, with one WARN naming the gap, rather than scanning a home directory that structurally cannot hold the answer. The javadoc records all three options that were weighed and why this one was taken. No new config key was invented. **What I checked myself, rather than taking the report:** - `mvn clean install` on the branch: BUILD SUCCESS, **1086 tests, 0 failures, 0 errors** (`main` was 1081). - The `memberHerdrSocket`-absent path — the criterion that protects the running daemon, since the key is unset here. `configParentDir()` returns the old `configRoot` untouched, and both other new branches sit behind the same check. - The thing the report could not settle: `shareWithGroup(dir, …)` runs **after** all three files land in the directory (`member-charter.md`, `ide-rules.md`, `opencode.json`), and it walks one flat level, so every file is covered. This was the way the fix could have been quietly wrong. A reviewer on a separate backend answered the same four questions independently and found no defect in the merged change. It made one correction I am recording here: the absent path is not *instruction*-byte-identical, because it now performs a `ConfigRef.get()` read in two places. That read is atomic and produces no changed result or side effect when the key is unset, so the behavioural claim stands. **Two follow-ups came out of this, both filed rather than folded in:** - **#222** — `ClaudeCodeLauncher#writeCharterFile` has the same site-1 shape, and #220 made that file the *only* charter delivery path, so it matters more than it looks. - **#224** — `worktreeRoot` itself is never made group-traversable. That is the floor this fix stands on, and it belongs to `GitWorktrees`, not to the launcher.
ltms closed this issue 2026-09-01 10:34:04 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#219