Fix worktree setup scripts losing PATH (#1908)

* fix(worktree): preserve PATH for lifecycle scripts

Lifecycle command strings were running through login shells, which could rewrite PATH before setup commands saw the daemon environment. Use the shared stable script-shell helper without login startup files, and route related project-authored command strings through it.

* fix(worktree): ignore Bash startup env hook

Project command strings should not source shell startup hooks before they see Paseo's supplied environment. Strip BASH_ENV from lifecycle, loop verify, and ACP string-command shells and pin the behavior in setup tests.

* fix(worktree): resolve Bash through PATH

* fix(worktree): preserve Windows cmd command strings

* test(acp): expect Windows cmd terminal strings
This commit is contained in:
Mohamed Boudra
2026-07-06 20:34:42 +08:00
committed by GitHub
parent 61df450366
commit 08a0d0c28d
10 changed files with 354 additions and 50 deletions
+9
View File
@@ -178,6 +178,15 @@ defaults. The default rotation is `10m` x `3` files everywhere.
`worktree.setup` and `worktree.teardown` accept either a multiline shell script or an array
of commands. Both run sequentially.
Lifecycle commands run in the worktree through a stable script shell: `bash`
resolved from `PATH` on macOS/Linux, and PowerShell with `-NoProfile` on
Windows. They inherit the daemon environment plus Paseo's lifecycle variables;
login and interactive shell startup files are not loaded, and Bash's `BASH_ENV`
hook is unset. Daemon-run loop verify checks and ACP single-string terminal
commands use the same non-login Bash behavior on macOS/Linux, but preserve their
existing `cmd.exe /c` string semantics on Windows. Service scripts are separate:
they launch in a terminal and receive the service environment described below.
```json
{
"worktree": {
@@ -46,6 +46,7 @@ import { transformPiModels } from "./pi/agent.js";
import type { AgentStreamEvent } from "../agent-sdk-types.js";
import type { AgentCapabilityFlags, AgentPersistenceHandle } from "../agent-sdk-types.js";
import { createTestLogger } from "../../../test-utils/test-logger.js";
import { buildStringCommandShellInvocation } from "../../../utils/string-command-shell.js";
import { asInternals } from "../../test-utils/class-mocks.js";
import * as spawnUtils from "../../../utils/spawn.js";
@@ -540,7 +541,10 @@ describe("ACPAgentSession terminal tools", () => {
const child = createTerminalChildStub();
const spawn = vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child);
const session = createSession();
const shell = spawnUtils.platformShell();
const shell = buildStringCommandShellInvocation({
command: "git -C /repo status --short",
windowsShell: "cmd",
});
await session.createTerminal({
sessionId: "session-1",
@@ -549,12 +553,50 @@ describe("ACPAgentSession terminal tools", () => {
});
expect(spawn).toHaveBeenCalledWith(
shell.command,
[...shell.flag, "git -C /repo status --short"],
expect.objectContaining({ cwd: "/repo" }),
shell.shell,
shell.args,
expect.objectContaining({
cwd: "/repo",
envOverlay: expect.objectContaining({ BASH_ENV: undefined }),
shell: false,
}),
);
});
test("preserves cmd semantics for single-string terminal commands on Windows", async () => {
const originalPlatform = process.platform;
Object.defineProperty(process, "platform", {
value: "win32",
configurable: true,
});
try {
const child = createTerminalChildStub();
const spawn = vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child);
const session = createSession();
await session.createTerminal({
sessionId: "session-1",
command: "echo %TEMP% && echo ok",
cwd: "C:\\repo",
});
expect(spawn).toHaveBeenCalledWith(
"cmd.exe",
["/c", "echo %TEMP% && echo ok"],
expect.objectContaining({
cwd: "C:\\repo",
envOverlay: expect.objectContaining({ BASH_ENV: undefined }),
shell: false,
}),
);
} finally {
Object.defineProperty(process, "platform", {
value: originalPlatform,
configurable: true,
});
}
});
test("preserves explicit terminal argv", async () => {
const child = createTerminalChildStub();
const spawn = vi.spyOn(spawnUtils, "spawnProcess").mockReturnValue(child);
@@ -102,7 +102,11 @@ import {
} from "../provider-launch-config.js";
import { renderPromptAttachmentAsText } from "../prompt-attachments.js";
import { appendOrReplaceGrowingAssistantMessage, runProviderTurn } from "./provider-runner.js";
import { platformShell, spawnProcess } from "../../../utils/spawn.js";
import {
buildStringCommandShellInvocation,
createStringCommandShellEnvOverlay,
} from "../../../utils/string-command-shell.js";
import { spawnProcess } from "../../../utils/spawn.js";
import {
type DiagnosticEntry,
toDiagnosticErrorMessage,
@@ -181,7 +185,7 @@ function toACPRequestError(error: unknown): Error {
function resolveTerminalCommand(
command: string,
args?: string[],
): { command: string; args: string[] } {
): { command: string; args: string[]; shell?: boolean } {
if (args && args.length > 0) {
return { command, args };
}
@@ -190,8 +194,8 @@ function resolveTerminalCommand(
return { command, args: [] };
}
const shell = platformShell();
return { command: shell.command, args: [...shell.flag, command] };
const shell = buildStringCommandShellInvocation({ command, windowsShell: "cmd" });
return { command: shell.shell, args: shell.args, shell: false };
}
function formatDurationMs(startedAt: number): string {
@@ -2159,12 +2163,15 @@ export class ACPAgentSession implements AgentSession, ACPClient {
(params.env ?? []).map((entry: EnvVariable) => [entry.name, entry.value]),
);
const terminalCommand = resolveTerminalCommand(params.command, params.args);
const commandEnvOverlays =
terminalCommand.shell === false ? [env, createStringCommandShellEnvOverlay()] : [env];
const child = spawnProcess(terminalCommand.command, terminalCommand.args, {
cwd: params.cwd ?? this.config.cwd,
...createProviderEnvSpec({
runtimeSettings: this.runtimeSettings,
overlays: [env],
overlays: commandEnvOverlays,
}),
shell: terminalCommand.shell,
stdio: ["ignore", "pipe", "pipe"],
});
+12 -3
View File
@@ -14,7 +14,11 @@ import type {
AgentTimelineItem,
AgentProvider,
} from "./agent/agent-sdk-types.js";
import { execCommand, platformShell } from "../utils/spawn.js";
import {
buildStringCommandShellInvocation,
createStringCommandShellEnvOverlay,
} from "../utils/string-command-shell.js";
import { execCommand } from "../utils/spawn.js";
import type {
ProviderSnapshotManager,
ResolvedProviderCreateConfig,
@@ -266,10 +270,15 @@ async function runVerifyCheck(options: {
}): Promise<LoopVerifyCheckResult> {
const startedAt = nowIso();
try {
const shell = platformShell();
const result = await execCommand(shell.command, [...shell.flag, options.command], {
const shell = buildStringCommandShellInvocation({
command: options.command,
windowsShell: "cmd",
});
const result = await execCommand(shell.shell, shell.args, {
cwd: options.cwd,
envOverlay: createStringCommandShellEnvOverlay(),
maxBuffer: MAX_VERIFY_OUTPUT_BYTES,
shell: false,
});
return {
command: options.command,
-16
View File
@@ -113,19 +113,3 @@ export async function execCommand(
windowsHide: true,
}) as Promise<ExecCommandResult>;
}
export function platformShell(): { command: string; flag: string[] } {
if (process.platform === "win32") {
return { command: "cmd.exe", flag: ["/c"] };
}
return { command: "/bin/sh", flag: ["-lc"] };
}
export function platformBash(): { command: string; flag: string[] } {
if (process.platform === "win32") {
return { command: "cmd.exe", flag: ["/c"] };
}
return { command: "/bin/bash", flag: ["-lc"] };
}
@@ -1,21 +1,76 @@
import { describe, expect, it } from "vitest";
import { execFileSync, spawnSync } from "node:child_process";
import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { delimiter, join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { buildStringCommandShellInvocation } from "./string-command-shell.js";
import {
buildStringCommandShellInvocation,
createStringCommandShellEnv,
} from "./string-command-shell.js";
function hasBashOnPath(): boolean {
const result = spawnSync("bash", ["-c", "true"], { stdio: "ignore" });
return !result.error && result.status === 0;
}
describe("buildStringCommandShellInvocation", () => {
it("uses bash login-command semantics on unix platforms", () => {
const tempDirs: string[] = [];
afterEach(() => {
for (const tempDir of tempDirs) {
rmSync(tempDir, { recursive: true, force: true });
}
tempDirs.length = 0;
});
it("uses bash script semantics on unix platforms", () => {
expect(
buildStringCommandShellInvocation({
command: 'echo "hello"',
platform: "darwin",
}),
).toEqual({
shell: "/bin/bash",
args: ["-lc", 'echo "hello"'],
shell: "bash",
args: ["-c", 'echo "hello"'],
});
});
it("uses powershell command semantics on windows", () => {
it.skipIf(process.platform === "win32" || !hasBashOnPath())(
"preserves the supplied PATH when login profiles rewrite it",
() => {
const home = mkdtempSync(join(tmpdir(), "paseo-shell-home-"));
tempDirs.push(home);
const binDir = join(home, "bin");
mkdirSync(binDir);
const shimPath = join(binDir, "paseo-shim");
writeFileSync(shimPath, "#!/bin/sh\nprintf 'shim:%s\\n' \"$1\"\n");
chmodSync(shimPath, 0o755);
writeFileSync(join(home, ".bash_profile"), "export PATH=/usr/bin:/bin\n");
const bashEnvPath = join(home, "bash-env");
writeFileSync(bashEnvPath, "export PATH=/usr/bin:/bin\n");
const invocation = buildStringCommandShellInvocation({
command: "command -v paseo-shim >/dev/null && paseo-shim ok",
});
const env: NodeJS.ProcessEnv = {
...process.env,
HOME: home,
PATH: `${binDir}${delimiter}${process.env.PATH ?? "/usr/bin:/bin"}`,
BASH_ENV: bashEnvPath,
};
const stdout = execFileSync(invocation.shell, invocation.args, {
encoding: "utf8",
env: createStringCommandShellEnv(env),
});
expect(stdout.trim()).toBe("shim:ok");
},
);
it("uses powershell command semantics on windows by default", () => {
expect(
buildStringCommandShellInvocation({
command: "Write-Output 'hello'",
@@ -33,4 +88,17 @@ describe("buildStringCommandShellInvocation", () => {
],
});
});
it("can preserve cmd command semantics on windows", () => {
expect(
buildStringCommandShellInvocation({
command: "echo %TEMP% && echo ok",
platform: "win32",
windowsShell: "cmd",
}),
).toEqual({
shell: "cmd.exe",
args: ["/c", "echo %TEMP% && echo ok"],
});
});
});
@@ -1,6 +1,7 @@
export interface BuildStringCommandShellInvocationOptions {
command: string;
platform?: NodeJS.Platform;
windowsShell?: "powershell" | "cmd";
}
export interface StringCommandShellInvocation {
@@ -8,12 +9,31 @@ export interface StringCommandShellInvocation {
args: string[];
}
export function createStringCommandShellEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
const sanitized = { ...env };
delete sanitized.BASH_ENV;
return sanitized;
}
export function createStringCommandShellEnvOverlay(): Record<string, string | undefined> {
return { BASH_ENV: undefined };
}
export function buildStringCommandShellInvocation(
options: BuildStringCommandShellInvocationOptions,
): StringCommandShellInvocation {
const platform = options.platform ?? process.platform;
// Project-authored command strings use a stable script shell. The caller supplies
// the environment; shell startup files should not rewrite it behind our back.
if (platform === "win32") {
if (options.windowsShell === "cmd") {
return {
shell: "cmd.exe",
args: ["/c", options.command],
};
}
return {
shell: "powershell",
args: [
@@ -28,7 +48,7 @@ export function buildStringCommandShellInvocation(
}
return {
shell: "/bin/bash",
args: ["-lc", options.command],
shell: "bash",
args: ["-c", options.command],
};
}
@@ -1,9 +1,12 @@
import { type ChildProcess } from "node:child_process";
import { EventEmitter } from "node:events";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const execFileMock = vi.hoisted(() => vi.fn());
const spawnProcessMock = vi.hoisted(() => vi.fn());
vi.mock("child_process", async () => {
const actual = await vi.importActual<typeof import("child_process")>("child_process");
@@ -13,11 +16,34 @@ vi.mock("child_process", async () => {
};
});
vi.mock("./spawn.js", async () => {
const actual = await vi.importActual<typeof import("./spawn.js")>("./spawn.js");
return {
...actual,
spawnProcess: spawnProcessMock,
};
});
function emitSuccessfulClose(child: ChildProcess): void {
child.emit("close", 0);
}
function createSpawnChildStub(): ChildProcess {
const child = new EventEmitter() as ChildProcess;
Object.assign(child, {
stdout: new EventEmitter(),
stderr: new EventEmitter(),
});
queueMicrotask(() => emitSuccessfulClose(child));
return child;
}
describe("worktree shell selection", () => {
const originalPlatform = process.platform;
beforeEach(() => {
execFileMock.mockReset();
spawnProcessMock.mockReset();
execFileMock.mockImplementation(
(
_file: string,
@@ -29,6 +55,7 @@ describe("worktree shell selection", () => {
return {};
},
);
spawnProcessMock.mockImplementation(createSpawnChildStub);
});
afterEach(() => {
@@ -46,6 +73,8 @@ describe("worktree shell selection", () => {
});
const worktreePath = mkdtempSync(join(tmpdir(), "worktree-shell-selection-"));
const originalBashEnv = process.env.BASH_ENV;
process.env.BASH_ENV = "should-not-leak";
try {
mkdirSync(join(worktreePath, ".git"), { recursive: true });
writeFileSync(
@@ -79,7 +108,74 @@ describe("worktree shell selection", () => {
expect.objectContaining({ cwd: worktreePath }),
expect.any(Function),
);
const execOptions = execFileMock.mock.calls[0]?.[2] as { env?: NodeJS.ProcessEnv };
expect(execOptions.env?.BASH_ENV).toBeUndefined();
} finally {
if (originalBashEnv === undefined) {
delete process.env.BASH_ENV;
} else {
process.env.BASH_ENV = originalBashEnv;
}
rmSync(worktreePath, { recursive: true, force: true });
}
});
it("routes streamed setup command execution through powershell on win32", async () => {
Object.defineProperty(process, "platform", {
value: "win32",
configurable: true,
});
const worktreePath = mkdtempSync(join(tmpdir(), "worktree-shell-selection-"));
const originalBashEnv = process.env.BASH_ENV;
process.env.BASH_ENV = "should-not-leak";
try {
writeFileSync(
join(worktreePath, "paseo.json"),
JSON.stringify({
worktree: {
setup: ["Write-Output 'setup'"],
},
}),
"utf8",
);
const { runWorktreeSetupCommands } = await import("./worktree.js");
await runWorktreeSetupCommands({
worktreePath,
branchName: "main",
cleanupOnFailure: false,
runtimeEnv: {
PASEO_SOURCE_CHECKOUT_PATH: worktreePath,
PASEO_ROOT_PATH: worktreePath,
PASEO_WORKTREE_PATH: worktreePath,
PASEO_BRANCH_NAME: "main",
PASEO_WORKTREE_PORT: "12345",
},
onEvent: () => {},
});
expect(spawnProcessMock).toHaveBeenCalledTimes(1);
expect(spawnProcessMock).toHaveBeenCalledWith(
"powershell",
[
"-NoProfile",
"-NonInteractive",
"-ExecutionPolicy",
"Bypass",
"-Command",
"Write-Output 'setup'",
],
expect.objectContaining({ cwd: worktreePath, shell: false }),
);
const spawnOptions = spawnProcessMock.mock.calls[0]?.[2] as { env?: NodeJS.ProcessEnv };
expect(spawnOptions.env?.BASH_ENV).toBeUndefined();
} finally {
if (originalBashEnv === undefined) {
delete process.env.BASH_ENV;
} else {
process.env.BASH_ENV = originalBashEnv;
}
rmSync(worktreePath, { recursive: true, force: true });
}
});
@@ -33,8 +33,9 @@ import {
realpathSync,
writeFileSync,
readFileSync,
chmodSync,
} from "fs";
import { dirname, join } from "path";
import { delimiter, dirname, join } from "path";
import { tmpdir } from "os";
import net from "node:net";
@@ -637,6 +638,68 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => {
);
});
it("runs setup commands with the daemon PATH instead of login profile PATH", async () => {
const home = join(tempDir, "host-home");
const binDir = join(tempDir, "daemon-bin");
mkdirSync(home);
mkdirSync(binDir);
const shimPath = join(binDir, "paseo-shim");
writeFileSync(shimPath, "#!/bin/sh\nprintf 'shim:%s\\n' \"$1\"\n");
chmodSync(shimPath, 0o755);
writeFileSync(join(home, ".bash_profile"), "export PATH=/usr/bin:/bin\n");
const bashEnvPath = join(home, "bash-env");
writeFileSync(bashEnvPath, "export PATH=/usr/bin:/bin\n");
writeFileSync(
join(repoDir, "paseo.json"),
JSON.stringify({
worktree: {
setup: "command -v paseo-shim >/dev/null && paseo-shim ok > setup-path.log",
},
}),
);
const originalHome = process.env.HOME;
const originalPath = process.env.PATH;
const originalBashEnv = process.env.BASH_ENV;
process.env.HOME = home;
process.env.PATH = `${binDir}${delimiter}${originalPath ?? "/usr/bin:/bin"}`;
process.env.BASH_ENV = bashEnvPath;
try {
await runWorktreeSetupCommands({
worktreePath: repoDir,
branchName: "main",
cleanupOnFailure: false,
runtimeEnv: {
PASEO_SOURCE_CHECKOUT_PATH: repoDir,
PASEO_ROOT_PATH: repoDir,
PASEO_WORKTREE_PATH: repoDir,
PASEO_BRANCH_NAME: "main",
PASEO_WORKTREE_PORT: "12345",
},
});
} finally {
if (originalHome === undefined) {
delete process.env.HOME;
} else {
process.env.HOME = originalHome;
}
if (originalPath === undefined) {
delete process.env.PATH;
} else {
process.env.PATH = originalPath;
}
if (originalBashEnv === undefined) {
delete process.env.BASH_ENV;
} else {
process.env.BASH_ENV = originalBashEnv;
}
}
expect(readFileSync(join(repoDir, "setup-path.log"), "utf8").trim()).toBe("shim:ok");
});
it("treats blank lifecycle strings as empty", () => {
writeFileSync(
join(repoDir, "paseo.json"),
+19 -13
View File
@@ -6,7 +6,10 @@ import { join, basename, dirname, isAbsolute, resolve, sep } from "path";
import net from "node:net";
import { createHash } from "node:crypto";
import stripAnsi from "strip-ansi";
import { buildStringCommandShellInvocation } from "./string-command-shell.js";
import {
buildStringCommandShellInvocation,
createStringCommandShellEnv,
} from "./string-command-shell.js";
import { readPaseoConfigJson, resolvePaseoConfigPath } from "./paseo-config-file.js";
export {
PaseoConfigRawSchema,
@@ -489,6 +492,7 @@ async function execSetupCommandStreamed(options: {
const child = spawnProcess(shellInvocation.shell, shellInvocation.args, {
cwd: options.cwd,
env: options.env,
shell: false,
stdio: ["ignore", "pipe", "pipe"],
});
@@ -607,7 +611,7 @@ export async function runWorktreeSetupCommands(options: {
branchName: options.branchName,
...(options.repoRootPath ? { repoRootPath: options.repoRootPath } : {}),
}));
const setupEnv = createExternalProcessEnv(process.env, runtimeEnv);
const setupEnv = createStringCommandShellEnv(createExternalProcessEnv(process.env, runtimeEnv));
const results: WorktreeSetupCommandResult[] = [];
for (const [index, cmd] of setupCommands.entries()) {
@@ -715,17 +719,19 @@ export async function runWorktreeTeardownCommands(options: {
options.branchName ?? (await resolveBranchNameForWorktreePath(options.worktreePath));
const worktreePort = readPaseoWorktreeRuntimePort(options.worktreePath);
const teardownEnv: NodeJS.ProcessEnv = createExternalProcessEnv(process.env, {
// Source checkout path is the original git repo root (shared across worktrees), not the
// worktree itself. This allows lifecycle scripts to copy or clean resources using paths
// from the source checkout.
PASEO_SOURCE_CHECKOUT_PATH: repoRootPath,
// Backward-compatible alias.
PASEO_ROOT_PATH: repoRootPath,
PASEO_WORKTREE_PATH: options.worktreePath,
PASEO_BRANCH_NAME: branchName,
...(worktreePort !== null ? { PASEO_WORKTREE_PORT: String(worktreePort) } : {}),
});
const teardownEnv: NodeJS.ProcessEnv = createStringCommandShellEnv(
createExternalProcessEnv(process.env, {
// Source checkout path is the original git repo root (shared across worktrees), not the
// worktree itself. This allows lifecycle scripts to copy or clean resources using paths
// from the source checkout.
PASEO_SOURCE_CHECKOUT_PATH: repoRootPath,
// Backward-compatible alias.
PASEO_ROOT_PATH: repoRootPath,
PASEO_WORKTREE_PATH: options.worktreePath,
PASEO_BRANCH_NAME: branchName,
...(worktreePort !== null ? { PASEO_WORKTREE_PORT: String(worktreePort) } : {}),
}),
);
const results: WorktreeTeardownCommandResult[] = [];
for (const cmd of teardownCommands) {