Skip to content

Commit b1d89d7

Browse files
committed
fix(bash): compose plugin PATH instead of stomping system PATH
When plugins return PATH entries via the shell.env hook, prepend them to the existing system PATH instead of replacing it entirely. This preserves standard paths like /usr/bin.
1 parent 3729fd5 commit b1d89d7

3 files changed

Lines changed: 80 additions & 2 deletions

File tree

packages/opencode/src/pty/index.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { InstanceState } from "@/effect/instance-state"
44
import { makeRuntime } from "@/effect/run-service"
55
import { Instance } from "@/project/instance"
66
import type { Proc } from "#pty"
7+
import path from "path"
78
import z from "zod"
89
import { Log } from "../util/log"
910
import { lazy } from "@opencode-ai/util/lazy"
@@ -183,10 +184,16 @@ export namespace Pty {
183184

184185
const cwd = input.cwd || s.dir
185186
const shell = yield* plugin.trigger("shell.env", { cwd }, { env: {} })
187+
// Extract plugin PATH before spreading to compose rather than stomp
188+
const pluginEnv: Record<string, string> = { ...shell.env }
189+
const pluginPath = pluginEnv.PATH ?? ""
190+
delete pluginEnv.PATH
186191
const env = {
187192
...process.env,
188193
...input.env,
189-
...shell.env,
194+
...pluginEnv,
195+
// Compose PATH: plugin bins prepended, system PATH preserved
196+
...(pluginPath && { PATH: `${pluginPath}${path.delimiter}${process.env.PATH ?? ""}` }),
190197
TERM: "xterm-256color",
191198
OPENCODE_TERMINAL: "1",
192199
} as Record<string, string>

packages/opencode/src/tool/bash.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,9 +363,15 @@ export const BashTool = Tool.define(
363363
{ cwd, sessionID: ctx.sessionID, callID: ctx.callID },
364364
{ env: {} },
365365
)
366+
// Extract plugin PATH before spreading to compose rather than stomp
367+
const env: Record<string, string> = { ...extra.env }
368+
const paths = env.PATH ?? ""
369+
delete env.PATH
366370
return {
367371
...process.env,
368-
...extra.env,
372+
...env,
373+
// Compose PATH: plugin bins prepended, system PATH preserved
374+
...(paths && { PATH: `${paths}${path.delimiter}${process.env.PATH ?? ""}` }),
369375
}
370376
})
371377

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { describe, test, expect } from "bun:test"
2+
import { readFileSync } from "fs"
3+
import { resolve } from "path"
4+
import path from "path"
5+
6+
const bashSrc = readFileSync(resolve(import.meta.dir, "../../src/tool/bash.ts"), "utf-8")
7+
8+
describe("shellEnv PATH composition", () => {
9+
// Regression: plugin PATH entries via shell.env hook used to stomp the
10+
// system PATH entirely (via spread ...extra.env). Commands like `ls` and
11+
// `git` broke because /usr/bin was gone. The fix extracts plugin PATH
12+
// before spreading, then prepends it to process.env.PATH.
13+
14+
test("extracts PATH from plugin env before spreading", () => {
15+
// The fix must extract PATH, delete it, then compose
16+
expect(bashSrc).toContain("const paths = env.PATH")
17+
expect(bashSrc).toContain("delete env.PATH")
18+
})
19+
20+
test("prepends plugin PATH to system PATH with delimiter", () => {
21+
// Must use path.delimiter for cross-platform support (: on Unix, ; on Windows)
22+
expect(bashSrc).toContain("path.delimiter")
23+
// Must reference process.env.PATH to preserve system PATH
24+
expect(bashSrc).toContain("process.env.PATH")
25+
})
26+
27+
test("does not stomp system PATH when plugin provides PATH", () => {
28+
// The old bug: ...extra.env would overwrite process.env.PATH
29+
// The fix: spread env (without PATH), then compose PATH separately
30+
const shellEnvMatch = bashSrc.match(/async function shellEnv[\s\S]*?^}/m)
31+
expect(shellEnvMatch).not.toBeNull()
32+
const body = shellEnvMatch![0]
33+
34+
// The return object must NOT spread ...extra.env directly (the old bug).
35+
// It's fine in a copy (`{ ...extra.env }`) — the key is the return spreads
36+
// the sanitized `...env` (with PATH removed) instead.
37+
const returnMatch = body.match(/return \{[\s\S]*\}/)
38+
expect(returnMatch).not.toBeNull()
39+
expect(returnMatch![0]).not.toContain("...extra.env")
40+
41+
// Must spread the sanitized env (PATH removed)
42+
expect(returnMatch![0]).toContain("...env")
43+
// Must compose PATH in a separate spread
44+
expect(body).toMatch(/paths && \{.*PATH:/)
45+
})
46+
47+
test("handles empty plugin PATH (no PATH override)", () => {
48+
// When plugin doesn't set PATH, paths === "" which is falsy,
49+
// so the conditional spread is skipped and system PATH from
50+
// ...process.env is preserved.
51+
const shellEnvMatch = bashSrc.match(/async function shellEnv[\s\S]*?^}/m)
52+
expect(shellEnvMatch).not.toBeNull()
53+
const body = shellEnvMatch![0]
54+
55+
// The conditional must use a truthy check on paths
56+
expect(body).toMatch(/paths && \{/)
57+
})
58+
59+
// Verify the composition formula is correct
60+
test("composition formula: plugin PATH + delimiter + system PATH", () => {
61+
// The composed PATH must be: `${pluginPath}${delimiter}${systemPath}`
62+
// This ensures plugin binaries are found first, system commands still work
63+
expect(bashSrc).toMatch(/`\$\{paths\}\$\{path\.delimiter\}\$\{process\.env\.PATH \?\? ""\}`/)
64+
})
65+
})

0 commit comments

Comments
 (0)