fix(desktop): resolve login shell environment at Electron startup
On macOS, apps launched from Finder/Dock inherit a minimal environment (PATH is just /usr/bin:/bin:/usr/sbin:/sbin). This caused two problems: 1. Agent binaries like codex were not detected (findExecutable failed) 2. Terminals spawned by Paseo had no access to user-installed tools (node, bun, direnv — all "command not found") Fix: at Electron startup, spawn the user's login shell and capture its full environment via JSON.stringify(process.env) using UUID markers. This is the same battle-tested approach VS Code uses. The resolved environment is merged into process.env before the daemon starts, so all child processes — agents, terminals, git operations — inherit the correct environment automatically. This replaces the previous approach of invoking resolveShellEnv() (via the shell-env npm package) at multiple scattered call sites. Now there is a single source of truth: process.env is enriched once at startup. Changes: - New: packages/desktop/src/login-shell-env.ts (VS Code approach) - Removed: resolveShellEnv(), shell-env dependency, $SHELL -lic probes - Simplified: applyProviderEnv() and findExecutable() now trust process.env
This commit is contained in:
@@ -86,7 +86,6 @@
|
||||
"pino-pretty": "^13.1.3",
|
||||
"qrcode": "^1.5.4",
|
||||
"rotating-file-stream": "^3.2.9",
|
||||
"shell-env": "^4.0.3",
|
||||
"sherpa-onnx": "1.12.28",
|
||||
"sherpa-onnx-node": "1.12.28",
|
||||
"strip-ansi": "^7.1.2",
|
||||
|
||||
@@ -66,7 +66,7 @@ describe("applyProviderEnv", () => {
|
||||
},
|
||||
};
|
||||
|
||||
const env = applyProviderEnv(base, runtime, {});
|
||||
const env = applyProviderEnv(base, runtime);
|
||||
|
||||
expect(env.PATH).toBe("/usr/bin");
|
||||
expect(env.HOME).toBe("/custom/home");
|
||||
@@ -74,21 +74,11 @@ describe("applyProviderEnv", () => {
|
||||
expect(Object.keys(env).length).toBeGreaterThanOrEqual(3);
|
||||
});
|
||||
|
||||
test("shell env PATH wins over base env PATH", () => {
|
||||
const base = { PATH: "/usr/bin:/bin" };
|
||||
const shellEnv = { PATH: "/usr/local/bin:/usr/bin:/bin:/home/user/.nvm/bin" };
|
||||
|
||||
const env = applyProviderEnv(base, undefined, shellEnv);
|
||||
|
||||
expect(env.PATH).toBe("/usr/local/bin:/usr/bin:/bin:/home/user/.nvm/bin");
|
||||
});
|
||||
|
||||
test("runtimeSettings env wins over shell env", () => {
|
||||
test("runtimeSettings env wins over base env", () => {
|
||||
const base = { PATH: "/usr/bin" };
|
||||
const shellEnv = { PATH: "/usr/local/bin:/usr/bin" };
|
||||
const runtime: ProviderRuntimeSettings = { env: { PATH: "/custom/path" } };
|
||||
|
||||
const env = applyProviderEnv(base, runtime, shellEnv);
|
||||
const env = applyProviderEnv(base, runtime);
|
||||
|
||||
expect(env.PATH).toBe("/custom/path");
|
||||
});
|
||||
@@ -103,7 +93,7 @@ describe("applyProviderEnv", () => {
|
||||
CLAUDE_CODE_ENABLE_SDK_FILE_CHECKPOINTING: "true",
|
||||
};
|
||||
|
||||
const env = applyProviderEnv(base, undefined, {});
|
||||
const env = applyProviderEnv(base);
|
||||
|
||||
expect(env.PATH).toBe("/usr/bin");
|
||||
expect(env.CLAUDECODE).toBeUndefined();
|
||||
|
||||
@@ -1,9 +1,6 @@
|
||||
import { z } from "zod";
|
||||
|
||||
import {
|
||||
isCommandAvailable,
|
||||
resolveShellEnv,
|
||||
} from "../../utils/executable.js";
|
||||
import { isCommandAvailable } from "../../utils/executable.js";
|
||||
import type { AgentProvider } from "./agent-sdk-types.js";
|
||||
import { AgentProviderSchema } from "./provider-manifest.js";
|
||||
|
||||
@@ -94,11 +91,9 @@ const PARENT_SESSION_ENV_VARS = [
|
||||
export function applyProviderEnv(
|
||||
baseEnv: Record<string, string | undefined>,
|
||||
runtimeSettings?: ProviderRuntimeSettings,
|
||||
shellEnv?: Record<string, string>,
|
||||
): Record<string, string | undefined> {
|
||||
const merged: Record<string, string | undefined> = {
|
||||
...baseEnv,
|
||||
...(shellEnv ?? resolveShellEnv()),
|
||||
...(runtimeSettings?.env ?? {}),
|
||||
};
|
||||
for (const key of PARENT_SESSION_ENV_VARS) {
|
||||
|
||||
@@ -6,7 +6,7 @@ import { open as openFile, stat as statFile } from "fs/promises";
|
||||
import { TTLCache } from "@isaacs/ttlcache";
|
||||
import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js";
|
||||
import { parseAndHighlightDiff } from "../server/utils/diff-highlighter.js";
|
||||
import { findExecutable, resolveShellEnv } from "./executable.js";
|
||||
import { findExecutable } from "./executable.js";
|
||||
import { isPaseoOwnedWorktreeCwd } from "./worktree.js";
|
||||
import { requirePaseoWorktreeBaseRefName } from "./worktree-metadata.js";
|
||||
|
||||
@@ -1845,7 +1845,7 @@ export async function createPullRequest(
|
||||
|
||||
await execAsync(`git push -u origin ${head}`, { cwd });
|
||||
|
||||
const ghEnv = { ...resolveShellEnv(), GIT_TERMINAL_PROMPT: "0" };
|
||||
const ghEnv: NodeJS.ProcessEnv = { ...process.env, GIT_TERMINAL_PROMPT: "0" };
|
||||
const args = ["api", "-X", "POST", `repos/${repo}/pulls`, "-f", `title=${options.title}`];
|
||||
args.push("-f", `head=${head}`);
|
||||
args.push("-f", `base=${normalizedBase}`);
|
||||
@@ -1912,7 +1912,7 @@ async function getPullRequestStatusUncached(cwd: string): Promise<PullRequestSta
|
||||
"--json",
|
||||
"url,title,state,baseRefName,headRefName,mergedAt",
|
||||
],
|
||||
{ cwd, env: { ...resolveShellEnv(), GIT_TERMINAL_PROMPT: "0" } },
|
||||
{ cwd, env: { ...process.env, GIT_TERMINAL_PROMPT: "0" } },
|
||||
);
|
||||
const pr = JSON.parse(stdout.trim());
|
||||
if (!pr || typeof pr !== "object" || !pr.url || !pr.title) {
|
||||
|
||||
@@ -11,10 +11,8 @@ type FindExecutableDependencies = NonNullable<Parameters<typeof findExecutable>[
|
||||
function createFindExecutableDependencies(): FindExecutableDependencies {
|
||||
return {
|
||||
execFileSync: vi.fn(),
|
||||
execSync: vi.fn(),
|
||||
existsSync: vi.fn(),
|
||||
platform: vi.fn(() => "darwin"),
|
||||
shell: undefined,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -77,25 +75,25 @@ describe("findExecutable", () => {
|
||||
expect(findExecutable("codex", findExecutableDependencies)).toBe("C:\\nvm4w\\nodejs\\codex");
|
||||
});
|
||||
|
||||
test("uses the last line from login-shell which output", () => {
|
||||
findExecutableDependencies.shell = "/bin/zsh";
|
||||
findExecutableDependencies.execSync.mockReturnValue(
|
||||
"echo from profile\n/usr/local/bin/codex\n",
|
||||
test("on Unix, uses the last line from which output", () => {
|
||||
findExecutableDependencies.execFileSync.mockReturnValue(
|
||||
"/usr/local/bin/codex\n",
|
||||
);
|
||||
|
||||
expect(findExecutable("codex", findExecutableDependencies)).toBe("/usr/local/bin/codex");
|
||||
expect(findExecutableDependencies.execSync).toHaveBeenCalledOnce();
|
||||
expect(findExecutableDependencies.execFileSync).not.toHaveBeenCalled();
|
||||
expect(findExecutableDependencies.execFileSync).toHaveBeenCalledWith(
|
||||
"which",
|
||||
["codex"],
|
||||
{ encoding: "utf8" },
|
||||
);
|
||||
});
|
||||
|
||||
test("warns and returns null when the final which line is not an absolute path", () => {
|
||||
findExecutableDependencies.shell = "/bin/zsh";
|
||||
findExecutableDependencies.execSync.mockReturnValue("profile noise\ncodex\n");
|
||||
findExecutableDependencies.execFileSync.mockReturnValue("codex\n");
|
||||
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||
|
||||
expect(findExecutable("codex", findExecutableDependencies)).toBeNull();
|
||||
expect(warnSpy).toHaveBeenCalledTimes(2);
|
||||
expect(warnSpy).toHaveBeenCalledOnce();
|
||||
|
||||
warnSpy.mockRestore();
|
||||
});
|
||||
|
||||
@@ -1,15 +1,11 @@
|
||||
import { execFileSync, execSync } from "node:child_process";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { existsSync } from "node:fs";
|
||||
import { platform } from "node:os";
|
||||
import path from "node:path";
|
||||
import { shellEnvSync } from "shell-env";
|
||||
|
||||
export interface FindExecutableDependencies {
|
||||
execSync: typeof execSync;
|
||||
execFileSync: typeof execFileSync;
|
||||
existsSync: typeof existsSync;
|
||||
platform: typeof platform;
|
||||
shell: string | undefined;
|
||||
}
|
||||
|
||||
function resolveWindowsPathEntries(deps: FindExecutableDependencies): string[] {
|
||||
@@ -42,7 +38,7 @@ function resolveWindowsPathEntries(deps: FindExecutableDependencies): string[] {
|
||||
function resolveExecutableFromWhichOutput(
|
||||
name: string,
|
||||
output: string,
|
||||
source: "login-shell" | "which",
|
||||
source: "which",
|
||||
): string | null {
|
||||
const lines = output
|
||||
.split(/\r?\n/)
|
||||
@@ -65,10 +61,8 @@ function resolveExecutableFromWhichOutput(
|
||||
}
|
||||
|
||||
/**
|
||||
* On Unix we first try `$SHELL -lic "which <name>"` so that rc-file PATH
|
||||
* additions (asdf, nvm, homebrew, nix, etc.) are visible — exactly as if the
|
||||
* user opened a terminal and typed the command. If that fails (e.g. the login
|
||||
* shell itself errors) we fall back to a plain `which`.
|
||||
* On Unix we use plain `which` — the daemon's process.env.PATH is enriched
|
||||
* with the login shell environment at Electron desktop startup.
|
||||
*
|
||||
* On Windows we augment the daemon PATH with machine/user registry PATH values
|
||||
* and return the first `where.exe` match. Launch-time execution decides whether
|
||||
@@ -85,11 +79,9 @@ export function findExecutable(
|
||||
}
|
||||
|
||||
const deps: FindExecutableDependencies = {
|
||||
execSync,
|
||||
execFileSync,
|
||||
existsSync,
|
||||
platform,
|
||||
shell: process.env["SHELL"],
|
||||
...dependencies,
|
||||
};
|
||||
|
||||
@@ -125,25 +117,6 @@ export function findExecutable(
|
||||
}
|
||||
}
|
||||
|
||||
// Unix: try the user's login shell so rc-file PATH entries are visible.
|
||||
const shell = deps.shell;
|
||||
if (shell) {
|
||||
try {
|
||||
const out = deps
|
||||
.execSync(`${shell} -lic "which ${trimmed}"`, {
|
||||
encoding: "utf8",
|
||||
timeout: 5000,
|
||||
})
|
||||
.trim();
|
||||
const resolved = resolveExecutableFromWhichOutput(trimmed, out, "login-shell");
|
||||
if (resolved) {
|
||||
return resolved;
|
||||
}
|
||||
} catch {
|
||||
// Login shell failed (broken rc, etc.) — fall through to plain which.
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
return resolveExecutableFromWhichOutput(
|
||||
trimmed,
|
||||
@@ -184,14 +157,3 @@ export function quoteWindowsArgument(argument: string): string {
|
||||
return `"${argument}"`;
|
||||
}
|
||||
|
||||
let cachedShellEnv: Record<string, string> | null = null;
|
||||
|
||||
export function resolveShellEnv(): Record<string, string> {
|
||||
if (cachedShellEnv) return cachedShellEnv;
|
||||
try {
|
||||
cachedShellEnv = shellEnvSync();
|
||||
} catch {
|
||||
cachedShellEnv = { ...process.env } as Record<string, string>;
|
||||
}
|
||||
return cachedShellEnv;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user