Skip to content

fix(core): preserve activated venv/conda env in spawned shells #38817

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
freerainboxbox wants to merge 1 commit into
base: dev
Choose a base branch
from
+330 −6
Open

fix(core): preserve activated venv/conda env in spawned shells #38817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion packages/core/src/shell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,24 @@ function info(file: string): Item {

export function args(file: string, command: string, cwd: string) {
const n = name(file)
if (n === "nu" || n === "fish") return ["-c", command]
if (n === "nu") return ["-c", command]
if (n === "fish") {
return [
"-c",
`set -q CONDA_PREFIX; and test "$PATH[1]" != "$CONDA_PREFIX/bin"; and set -gx PATH $CONDA_PREFIX/bin $PATH; set -q VIRTUAL_ENV; and test "$PATH[1]" != "$VIRTUAL_ENV/bin"; and set -gx PATH $VIRTUAL_ENV/bin $PATH; ${command}`,
]
}
if (n === "zsh") {
return [
"-l",
"-c",
`
[[ -f ~/.zshenv ]] && source ~/.zshenv >/dev/null 2>&1 || true
[[ -f "\${ZDOTDIR:-$HOME}/.zshrc" ]] && source "\${ZDOTDIR:-$HOME}/.zshrc" >/dev/null 2>&1 || true
# Restore env bin dirs only if rc files cost them first position; a
# containment check would wrongly skip the restore when merely demoted.
[[ -n "\${CONDA_PREFIX:-}" && ":$PATH:" != ":$CONDA_PREFIX/bin:"* ]] && export PATH="$CONDA_PREFIX/bin:$PATH"
[[ -n "\${VIRTUAL_ENV:-}" && ":$PATH:" != ":$VIRTUAL_ENV/bin:"* ]] && export PATH="$VIRTUAL_ENV/bin:$PATH"
cd -- "$1"
eval ${JSON.stringify(command)}
`,
Expand All @@ -187,6 +197,10 @@ export function args(file: string, command: string, cwd: string) {
`
shopt -s expand_aliases
[[ -f ~/.bashrc ]] && source ~/.bashrc >/dev/null 2>&1 || true
# Restore env bin dirs only if rc files cost them first position; a
# containment check would wrongly skip the restore when merely demoted.
[[ -n "\${CONDA_PREFIX:-}" && ":$PATH:" != ":$CONDA_PREFIX/bin:"* ]] && export PATH="$CONDA_PREFIX/bin:$PATH"
[[ -n "\${VIRTUAL_ENV:-}" && ":$PATH:" != ":$VIRTUAL_ENV/bin:"* ]] && export PATH="$VIRTUAL_ENV/bin:$PATH"
cd -- "$1"
eval ${JSON.stringify(command)}
`,
Expand Down
232 changes: 231 additions & 1 deletion packages/core/test/shell.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, expect, test } from "bun:test"
import path from "path"
import { spawnSync } from "child_process"
import { mkdtempSync, mkdirSync, writeFileSync, chmodSync, rmSync } from "fs"
import { Shell } from "@opencode-ai/core/shell"
import { FSUtil } from "@opencode-ai/core/fs-util"
import { which } from "@opencode-ai/core/util/which"
Expand Down Expand Up @@ -29,6 +31,230 @@ describe("shell", () => {
}
})

test("preserves inherited venv after bash -l with rc reordering", () => {
if (process.platform === "win32") return
const bash = which("bash")
if (!bash) return

const tmp = mkdtempSync("/tmp/opencode-test-venv-")
const home = path.join(tmp, "home")
const venv = path.join(tmp, "venv")
const system = path.join(tmp, "system-bin")
try {
mkdirSync(path.join(venv, "bin"), { recursive: true })
mkdirSync(home, { recursive: true })
mkdirSync(system, { recursive: true })

writeFileSync(
path.join(venv, "bin", "python"),
"#!/usr/bin/env bash\necho 'venv-python'\nexit 0\n",
)
chmodSync(path.join(venv, "bin", "python"), 0o755)
writeFileSync(
path.join(system, "python"),
"#!/usr/bin/env bash\necho 'system-python'\nexit 1\n",
)
chmodSync(path.join(system, "python"), 0o755)
writeFileSync(path.join(home, ".bashrc"), `export PATH="${system}:\$PATH"`, "utf8")

const args = Shell.args(bash, "command -v python", "/tmp")
const result = spawnSync(bash, args, {
env: {
HOME: home,
VIRTUAL_ENV: venv,
PATH: `${venv}/bin:${system}:/usr/bin:/bin`,
LC_ALL: "C.UTF-8",
},
cwd: "/tmp",
stdio: ["ignore", "pipe", "ignore"],
timeout: 10_000,
})

expect(result.status).toBe(0)
expect(result.stdout.toString("utf8").trim()).toBe(path.join(venv, "bin", "python"))
} finally {
rmSync(tmp, { recursive: true, force: true })
}
})

test("preserves inherited conda after bash -l with rc reordering", () => {
if (process.platform === "win32") return
const bash = which("bash")
if (!bash) return

const tmp = mkdtempSync("/tmp/opencode-test-conda-")
const home = path.join(tmp, "home")
const conda = path.join(tmp, "conda")
const system = path.join(tmp, "system-bin")
try {
mkdirSync(path.join(conda, "bin"), { recursive: true })
mkdirSync(home, { recursive: true })
mkdirSync(system, { recursive: true })

writeFileSync(
path.join(conda, "bin", "python"),
"#!/usr/bin/env bash\necho 'conda-python'\nexit 0\n",
)
chmodSync(path.join(conda, "bin", "python"), 0o755)
writeFileSync(
path.join(system, "python"),
"#!/usr/bin/env bash\necho 'system-python'\nexit 1\n",
)
chmodSync(path.join(system, "python"), 0o755)
writeFileSync(path.join(home, ".bashrc"), `export PATH="${system}:\$PATH"`, "utf8")

const args = Shell.args(bash, "command -v python", "/tmp")
const result = spawnSync(bash, args, {
env: {
HOME: home,
CONDA_PREFIX: conda,
PATH: `${conda}/bin:${system}:/usr/bin:/bin`,
LC_ALL: "C.UTF-8",
},
cwd: "/tmp",
stdio: ["ignore", "pipe", "ignore"],
timeout: 10_000,
})

expect(result.status).toBe(0)
expect(result.stdout.toString("utf8").trim()).toBe(path.join(conda, "bin", "python"))
} finally {
rmSync(tmp, { recursive: true, force: true })
}
})

test("preserves inherited venv after zsh -l with rc reordering", () => {
if (process.platform === "win32") return
const zsh = which("zsh")
if (!zsh) return

const tmp = mkdtempSync("/tmp/opencode-test-zsh-venv-")
const home = path.join(tmp, "home")
const venv = path.join(tmp, "venv")
const system = path.join(tmp, "system-bin")
try {
mkdirSync(path.join(venv, "bin"), { recursive: true })
mkdirSync(home, { recursive: true })
mkdirSync(system, { recursive: true })

writeFileSync(
path.join(venv, "bin", "python"),
"#!/usr/bin/env zsh\necho 'zsh-venv-python'\nexit 0\n",
)
chmodSync(path.join(venv, "bin", "python"), 0o755)
writeFileSync(
path.join(system, "python"),
"#!/usr/bin/env zsh\necho 'system-python'\nexit 1\n",
)
chmodSync(path.join(system, "python"), 0o755)
writeFileSync(path.join(home, ".zshrc"), `export PATH="${system}:\$PATH"`, "utf8")

const args = Shell.args(zsh, "command -v python", "/tmp")
const result = spawnSync(zsh, args, {
env: {
HOME: home,
VIRTUAL_ENV: venv,
PATH: `${venv}/bin:${system}:/usr/bin:/bin`,
LC_ALL: "C.UTF-8",
},
cwd: "/tmp",
stdio: ["ignore", "pipe", "ignore"],
timeout: 10_000,
})

expect(result.status).toBe(0)
expect(result.stdout.toString("utf8").trim()).toBe(path.join(venv, "bin", "python"))
} finally {
rmSync(tmp, { recursive: true, force: true })
}
})

test("does not duplicate venv bin already first in PATH after bash -l", () => {
if (process.platform === "win32") return
const bash = which("bash")
if (!bash) return

const tmp = mkdtempSync("/tmp/opencode-test-venv-dup-")
const home = path.join(tmp, "home")
const venv = path.join(tmp, "venv")
try {
mkdirSync(path.join(venv, "bin"), { recursive: true })
mkdirSync(home, { recursive: true })
writeFileSync(path.join(home, ".bashrc"), "# rc leaves PATH untouched\n", "utf8")

const args = Shell.args(bash, "echo $PATH", "/tmp")
const result = spawnSync(bash, args, {
env: {
HOME: home,
VIRTUAL_ENV: venv,
PATH: `${venv}/bin:/usr/bin:/bin`,
LC_ALL: "C.UTF-8",
},
cwd: "/tmp",
stdio: ["ignore", "pipe", "ignore"],
timeout: 10_000,
})

expect(result.status).toBe(0)
const entries = result.stdout.toString("utf8").trim().split(":")
expect(entries[0]).toBe(path.join(venv, "bin"))
// macOS /etc/profile (path_helper) demotes the venv bin even when rc files
// leave PATH alone, so the guard re-prepends and an inert second copy
// remains mid-PATH. Elsewhere the untouched PATH keeps it first and the
// guard skips the prepend entirely.
if (process.platform !== "darwin") {
expect(entries.filter((entry) => entry === path.join(venv, "bin"))).toHaveLength(1)
}
} finally {
rmSync(tmp, { recursive: true, force: true })
}
})

test("does not duplicate venv bin already first in PATH in fish -c", () => {
if (process.platform === "win32") return
const fish = which("fish")
if (!fish) return

const tmp = mkdtempSync("/tmp/opencode-test-fish-dup-")
const home = path.join(tmp, "home")
const venv = path.join(tmp, "venv")
try {
mkdirSync(path.join(venv, "bin"), { recursive: true })
mkdirSync(home, { recursive: true })

const args = Shell.args(fish, "string join : $PATH", "/tmp")
const result = spawnSync(fish, args, {
env: {
HOME: home,
VIRTUAL_ENV: venv,
PATH: `${venv}/bin:/usr/bin:/bin`,
LC_ALL: "C.UTF-8",
},
cwd: "/tmp",
stdio: ["ignore", "pipe", "ignore"],
timeout: 10_000,
})

expect(result.status).toBe(0)
const entries = result.stdout.toString("utf8").trim().split(":")
expect(entries[0]).toBe(path.join(venv, "bin"))
expect(entries.filter((entry) => entry === path.join(venv, "bin"))).toHaveLength(1)
} finally {
rmSync(tmp, { recursive: true, force: true })
}
})

test("fish args includes venv prelude", () => {
if (process.platform === "win32") return
const fish = which("fish")
if (!fish) return

const args = Shell.args(fish, "command -v python", "/tmp")
expect(args[0]).toBe("-c")
expect(args[1]).toContain("set -q VIRTUAL_ENV")
expect(args[1]).toContain("set -q CONDA_PREFIX")
})

test("detects login shells", () => {
expect(Shell.login("/bin/bash")).toBe(true)
expect(Shell.login("C:/tools/pwsh.exe")).toBe(false)
Expand Down Expand Up @@ -56,7 +282,11 @@ describe("shell", () => {

test("builds command args per shell family", () => {
expect(Shell.args("/bin/sh", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
expect(Shell.args("/usr/bin/fish", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
expect(Shell.args("/bin/nu", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
const fish = Shell.args("/usr/bin/fish", "echo hi", "/tmp")
expect(fish[0]).toBe("-c")
expect(fish[1]).toContain("set -q VIRTUAL_ENV")
expect(fish[1]).toContain("set -q CONDA_PREFIX")
const zsh = Shell.args("/bin/zsh", "echo hi", "/tmp")
expect(zsh[0]).toBe("-l")
expect(zsh[1]).toBe("-c")
Expand Down
10 changes: 8 additions & 2 deletions packages/desktop/src/main/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { fileURLToPath } from "node:url"
import { app, utilityProcess } from "electron"
import type { Details } from "electron"
import { getLogger } from "./logging"
import { getUserShell, loadShellEnv } from "./shell-env"
import { getUserShell, loadShellEnv, restoreVirtualEnv } from "./shell-env"
import { getStore } from "./store"
import { DEFAULT_SERVER_URL_KEY } from "./store-keys"

Expand Down Expand Up @@ -43,13 +43,19 @@ export function setDefaultServerUrl(url: string | null) {

export function preferAppEnv(userDataPath: string) {
const shell = process.platform === "win32" ? null : getUserShell()
const venv = process.env.VIRTUAL_ENV
const conda = process.env.CONDA_PREFIX
const shellEnv = shell ? loadShellEnv(shell, getLogger()) : null
Object.assign(process.env, {
...(shell ? loadShellEnv(shell, getLogger()) : null),
...shellEnv,
OPENCODE_EXPERIMENTAL_ICON_DISCOVERY: "true",
OPENCODE_EXPERIMENTAL_FILEWATCHER: "true",
OPENCODE_CLIENT: "desktop",
XDG_STATE_HOME: process.env.XDG_STATE_HOME ?? userDataPath,
})
if (shellEnv && (venv || conda)) {
restoreVirtualEnv(process.env, { VIRTUAL_ENV: venv, CONDA_PREFIX: conda })
}
}

export async function spawnLocalServer(
Expand Down
52 changes: 51 additions & 1 deletion packages/desktop/src/main/shell-env.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, test } from "bun:test"

import { isNushell, mergeShellEnv, parseShellEnv, resolveUserShell } from "./shell-env"
import { isNushell, mergeShellEnv, parseShellEnv, resolveUserShell, restoreVirtualEnv } from "./shell-env"

describe("shell env", () => {
test("parseShellEnv supports null-delimited pairs", () => {
Expand Down Expand Up @@ -47,4 +47,54 @@ describe("shell env", () => {
expect(isNushell("C:\\Program Files\\nu.exe")).toBe(true)
expect(isNushell("/bin/zsh")).toBe(false)
})

test("restoreVirtualEnv prepends venv bin to PATH", () => {
const env: Record<string, string | undefined> = { PATH: "/usr/bin:/bin" }
restoreVirtualEnv(env, { VIRTUAL_ENV: "/tmp/venv", CONDA_PREFIX: undefined })
expect(env.VIRTUAL_ENV).toBe("/tmp/venv")
expect(env.PATH).toBe("/tmp/venv/bin:/usr/bin:/bin")
})

test("restoreVirtualEnv prepends conda bin when conda prefix set", () => {
const env: Record<string, string | undefined> = { PATH: "/usr/bin:/bin" }
restoreVirtualEnv(env, { VIRTUAL_ENV: undefined, CONDA_PREFIX: "/tmp/conda" })
expect(env.CONDA_PREFIX).toBe("/tmp/conda")
expect(env.PATH).toBe("/tmp/conda/bin:/usr/bin:/bin")
})

test("restoreVirtualEnv orders conda before venv in PATH", () => {
const env: Record<string, string | undefined> = { PATH: "/usr/bin:/bin" }
restoreVirtualEnv(env, { VIRTUAL_ENV: "/tmp/venv", CONDA_PREFIX: "/tmp/conda" })
expect(env.CONDA_PREFIX).toBe("/tmp/conda")
expect(env.VIRTUAL_ENV).toBe("/tmp/venv")
// conda prepended first, venv prepended second → venv wins at front
expect(env.PATH).toBe("/tmp/venv/bin:/tmp/conda/bin:/usr/bin:/bin")
})

test("restoreVirtualEnv no-op when neither venv nor conda set", () => {
const env: Record<string, string | undefined> = { PATH: "/usr/bin:/bin" }
restoreVirtualEnv(env, { VIRTUAL_ENV: undefined, CONDA_PREFIX: undefined })
expect(env.PATH).toBe("/usr/bin:/bin")
expect(env.VIRTUAL_ENV).toBeUndefined()
expect(env.CONDA_PREFIX).toBeUndefined()
})

test("restoreVirtualEnv sets PATH when no PATH existed", () => {
const env: Record<string, string | undefined> = {}
restoreVirtualEnv(env, { VIRTUAL_ENV: "/tmp/venv", CONDA_PREFIX: undefined })
expect(env.VIRTUAL_ENV).toBe("/tmp/venv")
expect(env.PATH).toBe("/tmp/venv/bin")
})

test("restoreVirtualEnv does not duplicate bin already first in PATH", () => {
const env: Record<string, string | undefined> = { PATH: "/tmp/venv/bin:/usr/bin:/bin" }
restoreVirtualEnv(env, { VIRTUAL_ENV: "/tmp/venv", CONDA_PREFIX: undefined })
expect(env.PATH).toBe("/tmp/venv/bin:/usr/bin:/bin")
})

test("restoreVirtualEnv re-prepends bin demoted from first in PATH", () => {
const env: Record<string, string | undefined> = { PATH: "/usr/bin:/tmp/venv/bin:/bin" }
restoreVirtualEnv(env, { VIRTUAL_ENV: "/tmp/venv", CONDA_PREFIX: undefined })
expect(env.PATH).toBe("/tmp/venv/bin:/usr/bin:/tmp/venv/bin:/bin")
})
})
Loading
Loading