From 22ad24db6cb243acb387da7d550597fedcd2b4fd Mon Sep 17 00:00:00 2001 From: Kevin Nguyen Date: Sat, 1 Aug 2026 22:34:30 +0700 Subject: [PATCH] =?UTF-8?q?CB-511:=20give=20workers=20a=20toolchain=20?= =?UTF-8?q?=E2=80=94=20propagate=20the=20daemon=20PATH,=20add=20profile=20?= =?UTF-8?q?env:?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Workers could not run `mvn` or `java`. Every delegated task that asked for a build came back "mvn is not on PATH", and the worker was right. Root cause: HerdrPeerLauncher seeded the worker environment with an EMPTY map, so bridged passed only the vars it explicitly set (OPENCODE_CONFIG, GITEA_TOKEN, ANTHROPIC_*) and never PATH. herdr merges that map into its own process env, so a worker inherited whatever PATH the herdr SERVER was started with. On this host that server (pid 79870, PPID 1) had been up since Jul 4 with a PATH containing neither the JDK nor Maven. Confirmed on a live worker: its PATH was byte-identical to herdr's, and the only var bridged had contributed was OPENCODE_CONFIG. The failure was invisible and non-deterministic: the fleet's capabilities depended on how a long-lived daemon happened to be launched weeks earlier. There are three herdr processes on this box with three different PATHs; the one owning the socket is the one without a toolchain. bridged itself HAD Maven on PATH the whole time — it just never passed it on. It also quietly contradicted the project's own principle that "a worker is a full peer of the primary", and the implementer skill's instruction to build, commit and open a PR. Every delegation so far has depended on the primary running the build gate. Fix: baseEnv(cfg) seeds each worker with the daemon's own PATH, then applies the profile's new optional env: map. Adapter-specific vars are layered on top and therefore win — that ordering is load-bearing, not incidental: it stops an env: entry from overwriting ANTHROPIC_BASE_URL and slipping past SubscriptionGuard, which is checked against the profile's baseUrl alone. Pinned by a test. Because the default is now the daemon's PATH, both supervision units set PATH explicitly — launchd and systemd do not source a login shell, so under CB-504 the daemon (and every worker) would otherwise get a bare /usr/bin:/bin and this bug would silently return in production. 324 tests (was 321): daemon-PATH propagation, profile env: passthrough including an explicit PATH override, and the guard-bypass ordering. Verified live: daemon restarted, worker spawned, and asked to run the tools — "Apache Maven 3.9.16", "java version 25.0.2". Previously both were absent. --- bridged/bridged.example.yaml | 17 ++++++ .../ltms/bridged/config/BridgedConfig.java | 21 ++++++-- .../bridged/worker/ClaudeCodeLauncher.java | 2 +- .../bridged/worker/HerdrPeerLauncher.java | 29 ++++++++++- .../ltms/bridged/worker/OpenCodeLauncher.java | 2 +- .../worker/ClaudeCodeLauncherTest.java | 52 +++++++++++++++++++ deploy/bridged.service | 4 ++ deploy/dev.ltms.bridged.plist | 8 +++ 8 files changed, 128 insertions(+), 7 deletions(-) diff --git a/bridged/bridged.example.yaml b/bridged/bridged.example.yaml index f02b183..fae1a03 100644 --- a/bridged/bridged.example.yaml +++ b/bridged/bridged.example.yaml @@ -60,6 +60,23 @@ herdrSocket: ~/.config/herdr/herdr.sock # SSH is unaffected). The token value itself is never stored in this file. # gitHostEnv → host env var holding the forge host (default GITEA_HOST). Injected as # GITEA_HOST *only* alongside a resolved gitTokenEnv. +# env → extra environment for this profile's workers, as a literal key/value map +# (CB-511). Use it to give workers a toolchain. +# +# A worker's environment does NOT come from your shell. bridged hands herdr an +# explicit env map and herdr merges it into ITS OWN process env — so before +# CB-511 a worker inherited whatever PATH the herdr server happened to be +# started with, which on a long-lived herdr can predate your toolchain entirely +# and leave workers unable to run `mvn` or `java` at all. +# bridged now propagates ITS OWN PATH to every worker by default; set `env:` +# only to override that or add more (JAVA_HOME, …). Since the default is the +# daemon's PATH, make sure the daemon is started with a good one — see the PATH +# lines in deploy/dev.ltms.bridged.plist and deploy/bridged.service. +# +# Adapter-owned variables always win over `env:`: ANTHROPIC_BASE_URL and the +# rest of the ANTHROPIC_*/CLAUDE_* wiring are applied after it, so an `env:` +# entry cannot repoint a worker past the SubscriptionGuard — which is checked +# against `baseUrl` alone. # Put `defaultMode: "auto"` in each ccs profile so the worker runs autonomously. workers: gx10: # ccs profile name (NOT a hostname) diff --git a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java index 20734f1..ed937dd 100644 --- a/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java +++ b/bridged/src/main/java/dev/ltms/bridged/config/BridgedConfig.java @@ -107,7 +107,8 @@ public record BridgedConfig( String cwd, List parityOverlay, String gitTokenEnv, String gitHostEnv, - String kind) { + String kind, + Map env) { /** Peer kind spawned by {@link dev.ltms.bridged.worker.ClaudeCodeLauncher} (the default). */ public static final String KIND_CLAUDE_CODE = "claude-code"; @@ -133,6 +134,7 @@ public record BridgedConfig( // gitTokenEnv stays null when unset (opt-in). gitHostEnv defaults so operators enabling // checkpoints need only set gitTokenEnv; it is injected only alongside a resolved token. gitHostEnv = (gitHostEnv == null || gitHostEnv.isBlank()) ? "GITEA_HOST" : gitHostEnv; + env = (env == null) ? Map.of() : Map.copyOf(env); } /** @@ -157,13 +159,26 @@ public record BridgedConfig( String placement, String workspace, String tabLabel, String mcpUrl, String cwd, List parityOverlay, String gitTokenEnv, String gitHostEnv) { this(profile, baseUrl, model, configDir, tokenEnv, argv, placement, workspace, tabLabel, - mcpUrl, cwd, parityOverlay, gitTokenEnv, gitHostEnv, null); + mcpUrl, cwd, parityOverlay, gitTokenEnv, gitHostEnv, null, null); + } + + /** + * Backward-compatible constructor without the CB-511 {@code env:} passthrough — the worker + * gets the daemon's PATH and nothing else. Keeps pre-CB-511 call sites working. + */ + public Worker(String profile, String baseUrl, String model, + String configDir, String tokenEnv, List argv, + String placement, String workspace, String tabLabel, String mcpUrl, + String cwd, List parityOverlay, String gitTokenEnv, String gitHostEnv, + String kind) { + this(profile, baseUrl, model, configDir, tokenEnv, argv, placement, workspace, tabLabel, + mcpUrl, cwd, parityOverlay, gitTokenEnv, gitHostEnv, kind, null); } /** A copy with {@code profile} set — used to default a profile to its {@code workers} key. */ public Worker withProfile(String p) { return new Worker(p, baseUrl, model, configDir, tokenEnv, argv, placement, workspace, tabLabel, - mcpUrl, cwd, parityOverlay, gitTokenEnv, gitHostEnv, kind); + mcpUrl, cwd, parityOverlay, gitTokenEnv, gitHostEnv, kind, env); } /** True when this profile is served by the Claude Code adapter (the default kind). */ diff --git a/bridged/src/main/java/dev/ltms/bridged/worker/ClaudeCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/worker/ClaudeCodeLauncher.java index eecb8a3..93c2cdb 100644 --- a/bridged/src/main/java/dev/ltms/bridged/worker/ClaudeCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/worker/ClaudeCodeLauncher.java @@ -119,7 +119,7 @@ public final class ClaudeCodeLauncher extends HerdrPeerLauncher { String baseUrl = cfg.baseUrl(); guard.assertWorker(baseUrl); // hard stop before we spawn anything - Map workerEnv = newEnv(); + Map workerEnv = baseEnv(cfg); workerEnv.put("ANTHROPIC_BASE_URL", baseUrl); putIfPresent(workerEnv, "ANTHROPIC_MODEL", cfg.model()); putIfPresent(workerEnv, "CLAUDE_CONFIG_DIR", cfg.configDir()); diff --git a/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java b/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java index cea92d1..1d3f7c2 100644 --- a/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/worker/HerdrPeerLauncher.java @@ -496,8 +496,33 @@ public abstract class HerdrPeerLauncher implements PeerLauncher { } /** A fresh mutable env map — the conventional starting point for {@link #buildLaunch}. */ - protected static Map newEnv() { - return new LinkedHashMap<>(); + /** + * Seed a worker's environment (CB-511): the daemon's own {@code PATH}, then the profile's + * {@code env:} entries. + * + *

Why this exists: bridged passes herdr an explicit env map, and herdr merges it into + * its own process environment. So before this, a worker inherited whatever PATH the + * herdr server happened to be started with — on this host, one from weeks earlier with no JDK + * and no Maven, which left workers unable to run the build they were being asked to run. The + * worker's toolchain must follow from configuration, not from how a long-lived daemon was + * launched. + * + *

Adapter-specific variables are layered on top of this by {@code buildLaunch} and therefore + * win. That ordering is deliberate and load-bearing: it stops a profile's {@code env:} from + * overriding {@code ANTHROPIC_BASE_URL} and slipping past {@link + * dev.ltms.bridged.guard.SubscriptionGuard}, which is checked against the profile's + * {@code baseUrl} and nothing else. + */ + protected Map baseEnv(BridgedConfig.Worker cfg) { + Map workerEnv = new LinkedHashMap<>(); + String path = env.apply("PATH"); + if (path != null && !path.isBlank()) { + workerEnv.put("PATH", path); + } + if (cfg != null && cfg.env() != null) { + workerEnv.putAll(cfg.env()); + } + return workerEnv; } /** Defensive copy of {@code argv} plus room to append launch flags. */ diff --git a/bridged/src/main/java/dev/ltms/bridged/worker/OpenCodeLauncher.java b/bridged/src/main/java/dev/ltms/bridged/worker/OpenCodeLauncher.java index b200a86..da73778 100644 --- a/bridged/src/main/java/dev/ltms/bridged/worker/OpenCodeLauncher.java +++ b/bridged/src/main/java/dev/ltms/bridged/worker/OpenCodeLauncher.java @@ -138,7 +138,7 @@ public final class OpenCodeLauncher extends HerdrPeerLauncher { */ @Override protected Launch buildLaunch(BridgedConfig.Worker cfg) { - Map workerEnv = newEnv(); + Map workerEnv = baseEnv(cfg); // A config file is needed for the bridge MCP mount, for a pinned endpoint (CB-508), or both. if (cfg.hasMcp() || hasCustomProvider(cfg)) { workerEnv.put("OPENCODE_CONFIG", writeConfig(cfg).toString()); diff --git a/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java b/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java index a87936d..7449157 100644 --- a/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/worker/ClaudeCodeLauncherTest.java @@ -417,4 +417,56 @@ class ClaudeCodeLauncherTest { assertEquals(0, paneCloseCount(herdr, handle.id()), "no orphan pane close from the gate path"); } + + // --- CB-511: worker environment seeding ----------------------------------------------------- + + @Test + void workerInheritsTheDaemonPath() { + FakeHerdr herdr = new FakeHerdr(); + BridgedConfig.Worker cfg = new BridgedConfig.Worker( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "w #{n}", null, null, null); + new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + k -> "PATH".equals(k) ? "/opt/tools/bin:/usr/bin" : null).spawn(); + + assertEquals("/opt/tools/bin:/usr/bin", startEnv(herdr).get("PATH"), + "a worker with no PATH cannot run the build it is asked to run"); + } + + @Test + void profileEnvIsInjectedIntoTheWorker() { + FakeHerdr herdr = new FakeHerdr(); + BridgedConfig.Worker cfg = new BridgedConfig.Worker( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "w #{n}", null, null, null, null, null, + null, Map.of("JAVA_HOME", "/opt/jdk", "PATH", "/profile/bin")); + new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + k -> "PATH".equals(k) ? "/daemon/bin" : null).spawn(); + + Map env = startEnv(herdr); + assertEquals("/opt/jdk", env.get("JAVA_HOME"), "profile env: is passed through"); + assertEquals("/profile/bin", env.get("PATH"), "an explicit profile PATH overrides the daemon's"); + } + + /** + * The security-relevant ordering. {@code SubscriptionGuard} is checked against the profile's + * {@code baseUrl} only, so if a profile's {@code env:} could overwrite ANTHROPIC_BASE_URL a + * worker could be pointed at an unguarded host while the guard passed on a benign one. + */ + @Test + void profileEnvCannotOverrideGuardCheckedAnthropicVars() { + FakeHerdr herdr = new FakeHerdr(); + BridgedConfig.Worker cfg = new BridgedConfig.Worker( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("claude"), "tab", "bridged-workers", "w #{n}", null, null, null, null, null, + null, Map.of("ANTHROPIC_BASE_URL", "http://evil.example.com")); + new ClaudeCodeLauncher(new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), + _ -> null).spawn(); + + assertEquals("http://gx00.gw:8000", startEnv(herdr).get("ANTHROPIC_BASE_URL"), + "the guard-checked baseUrl must win over any env: entry, or the boundary is bypassable"); + } } diff --git a/deploy/bridged.service b/deploy/bridged.service index 899f0a9..29945ee 100644 --- a/deploy/bridged.service +++ b/deploy/bridged.service @@ -27,6 +27,10 @@ WorkingDirectory=%h/src/claude-bridge/bridged ExecStart=/usr/lib/jvm/temurin-25-jdk/bin/java -jar target/bridged.jar bridged.yaml Environment=HERDR_SOCKET_PATH=%h/.config/herdr/herdr.sock +# PATH matters more than it looks (CB-511): bridged propagates its own PATH to every worker it +# spawns, so this line decides whether the fleet can run a build at all. systemd does not source a +# login shell, so without it the daemon — and every worker — gets a bare default with no JDK/Maven. +Environment=PATH=/usr/lib/jvm/temurin-25-jdk/bin:/usr/share/maven/bin:/usr/local/bin:/usr/bin:/bin # Secrets are NOT set here — this file is committed. Put the API/worker tokens in a private # drop-in that systemd reads with restrictive permissions: # systemctl --user edit bridged → [Service] / Environment=BRIDGED_API_TOKEN=... diff --git a/deploy/dev.ltms.bridged.plist b/deploy/dev.ltms.bridged.plist index 823cf06..8fcee44 100644 --- a/deploy/dev.ltms.bridged.plist +++ b/deploy/dev.ltms.bridged.plist @@ -41,6 +41,14 @@ /Users/CHANGEME/Tool/jdk-25.0.2.jdk/Contents/Home HERDR_SOCKET_PATH /Users/CHANGEME/.config/herdr/herdr.sock + + PATH + /Users/CHANGEME/Tool/jdk-25.0.2.jdk/Contents/Home/bin:/Users/CHANGEME/Tool/apache-maven-3.9.16/bin:/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin