mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-26 03:21:13 +00:00
fix(shell): keep Git Bash coreutils on PATH (#108136)
* fix(windows): expose Git Bash coreutils to commands Co-authored-by: luyifan <al3060388206@gmail.com> * test(windows): inject duplicate PATH variants * test(windows): run Git Bash integration in CI * refactor(windows): keep shell env helper private --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -1982,7 +1982,7 @@
|
||||
"test:unit:fast:audit": "node scripts/test-unit-fast-audit.mjs",
|
||||
"test:voicecall:closedloop": "node scripts/test-voicecall-closedloop.mjs",
|
||||
"test:watch": "node scripts/test-projects.mjs --watch",
|
||||
"test:windows:ci": "node scripts/test-projects.mjs src/shared/runtime-import.test.ts src/process/exec.windows.test.ts src/process/windows-command.test.ts src/infra/windows-install-roots.test.ts src/node-host/invoke-system-run-allowlist.test.ts src/daemon/schtasks.startup-fallback.test.ts extensions/lobster/src/lobster-runner.test.ts test/scripts/format-generated-module.test.ts test/scripts/npm-runner.test.ts test/scripts/openclaw-cross-os-installer.windows.test.ts test/scripts/openclaw-cross-os-release-workflow.test.ts test/scripts/pnpm-runner.test.ts test/scripts/run-with-env.test.ts test/scripts/ts-topology.test.ts test/scripts/ui.test.ts test/scripts/vitest-process-group.test.ts",
|
||||
"test:windows:ci": "node scripts/test-projects.mjs src/shared/runtime-import.test.ts src/agents/sessions/windows-git-bash-path.test.ts src/process/exec.windows.test.ts src/process/windows-command.test.ts src/infra/windows-install-roots.test.ts src/node-host/invoke-system-run-allowlist.test.ts src/daemon/schtasks.startup-fallback.test.ts extensions/lobster/src/lobster-runner.test.ts test/scripts/format-generated-module.test.ts test/scripts/npm-runner.test.ts test/scripts/openclaw-cross-os-installer.windows.test.ts test/scripts/openclaw-cross-os-release-workflow.test.ts test/scripts/pnpm-runner.test.ts test/scripts/run-with-env.test.ts test/scripts/ts-topology.test.ts test/scripts/ui.test.ts test/scripts/vitest-process-group.test.ts",
|
||||
"tool-display:check": "node --import tsx scripts/tool-display.ts --check",
|
||||
"tool-display:write": "node --import tsx scripts/tool-display.ts --write",
|
||||
"ts-topology": "node --import tsx scripts/ts-topology.ts",
|
||||
|
||||
@@ -4,7 +4,11 @@
|
||||
*/
|
||||
|
||||
import { execSync, spawnSync } from "node:child_process";
|
||||
import { buildShellCommandInvocation, getBashShellConfig } from "../shell-utils.js";
|
||||
import {
|
||||
buildShellCommandInvocation,
|
||||
getBashShellConfig,
|
||||
getBashShellEnv,
|
||||
} from "../shell-utils.js";
|
||||
|
||||
// Cache for shell command results (persists for process lifetime)
|
||||
const commandResultCache = new Map<string, string | undefined>();
|
||||
@@ -27,7 +31,8 @@ function executeWithConfiguredShell(command: string): {
|
||||
value: string | undefined;
|
||||
} {
|
||||
try {
|
||||
const invocation = buildShellCommandInvocation(command, getBashShellConfig());
|
||||
const shellConfig = getBashShellConfig();
|
||||
const invocation = buildShellCommandInvocation(command, shellConfig);
|
||||
const [shell, ...args] = invocation.argv;
|
||||
const result = spawnSync(shell, args, {
|
||||
encoding: "utf-8",
|
||||
@@ -36,6 +41,7 @@ function executeWithConfiguredShell(command: string): {
|
||||
stdio: [invocation.stdin, "pipe", "ignore"],
|
||||
shell: false,
|
||||
windowsHide: true,
|
||||
env: getBashShellEnv(shellConfig.shell),
|
||||
});
|
||||
|
||||
if (result.error) {
|
||||
|
||||
@@ -18,7 +18,7 @@ import type { AgentTool } from "../../runtime/index.js";
|
||||
import {
|
||||
buildShellCommandInvocation,
|
||||
getBashShellConfig,
|
||||
getShellEnv,
|
||||
getBashShellEnv,
|
||||
killProcessTree,
|
||||
} from "../../shell-utils.js";
|
||||
import type { ToolDefinition, ToolRenderResultOptions } from "../extensions/types.js";
|
||||
@@ -76,7 +76,7 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas
|
||||
buffer: false,
|
||||
cwd,
|
||||
detached: process.platform !== "win32",
|
||||
env: env ?? getShellEnv(),
|
||||
env: env ?? getBashShellEnv(shellConfig.shell),
|
||||
...(invocation.input === undefined ? {} : { input: invocation.input }),
|
||||
reject: false,
|
||||
stdio: [invocation.stdin, "pipe", "pipe"],
|
||||
@@ -160,8 +160,9 @@ function resolveSpawnContext(
|
||||
command: string,
|
||||
cwd: string,
|
||||
spawnHook?: BashSpawnHook,
|
||||
shellPath?: string,
|
||||
): BashSpawnContext {
|
||||
const baseContext: BashSpawnContext = { command, cwd, env: { ...getShellEnv() } };
|
||||
const baseContext: BashSpawnContext = { command, cwd, env: getBashShellEnv(shellPath) };
|
||||
return spawnHook ? spawnHook(baseContext) : baseContext;
|
||||
}
|
||||
|
||||
@@ -322,7 +323,7 @@ export function createBashToolDefinition(
|
||||
void ctx;
|
||||
resolveBashTimeoutMs(timeout);
|
||||
const resolvedCommand = commandPrefix ? `${commandPrefix}\n${command}` : command;
|
||||
const spawnContext = resolveSpawnContext(resolvedCommand, cwd, spawnHook);
|
||||
const spawnContext = resolveSpawnContext(resolvedCommand, cwd, spawnHook, options?.shellPath);
|
||||
const output = new OutputAccumulator({ tempFilePrefix: "openclaw-bash" });
|
||||
let acceptingOutput = true;
|
||||
let updateTimer: NodeJS.Timeout | undefined;
|
||||
|
||||
23
src/agents/sessions/windows-git-bash-path.test.ts
Normal file
23
src/agents/sessions/windows-git-bash-path.test.ts
Normal file
@@ -0,0 +1,23 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolveConfigValueUncached } from "./resolve-config-value.js";
|
||||
import { createBashTool } from "./tools/bash.js";
|
||||
|
||||
const isWindows = process.platform === "win32";
|
||||
|
||||
describe.runIf(isWindows)("Git Bash PATH integration", () => {
|
||||
it("exposes Git coreutils through the Bash tool", async () => {
|
||||
const tool = createBashTool(process.cwd());
|
||||
|
||||
const result = await tool.execute("windows-git-bash-path", {
|
||||
command: "command -v cygpath",
|
||||
});
|
||||
|
||||
expect(result.content).toEqual([
|
||||
expect.objectContaining({ type: "text", text: expect.stringMatching(/usr\/bin\/cygpath/i) }),
|
||||
]);
|
||||
});
|
||||
|
||||
it("exposes Git coreutils to !command config resolution", () => {
|
||||
expect(resolveConfigValueUncached("!command -v cygpath")).toMatch(/usr\/bin\/cygpath/i);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,5 @@
|
||||
// Verifies shell selection, PATH lookup, and platform-specific shell helpers.
|
||||
import { spawnSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
@@ -8,7 +9,7 @@ import {
|
||||
buildShellCommandInvocation,
|
||||
detectRuntimeShell,
|
||||
getBashShellConfig,
|
||||
getShellEnv,
|
||||
getBashShellEnv,
|
||||
getShellConfig,
|
||||
sanitizeBinaryOutput,
|
||||
} from "./shell-utils.js";
|
||||
@@ -195,7 +196,7 @@ describe("getBashShellConfig", () => {
|
||||
let envSnapshot: ReturnType<typeof captureEnv>;
|
||||
|
||||
beforeEach(() => {
|
||||
envSnapshot = captureEnv(["ProgramFiles", "ProgramFiles(x86)", "PATH"]);
|
||||
envSnapshot = captureEnv(["ProgramFiles", "ProgramFiles(x86)", "PATH", "Path"]);
|
||||
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
});
|
||||
|
||||
@@ -225,6 +226,53 @@ describe("getBashShellConfig", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("prepends coreutils for a standard Git for Windows install", () => {
|
||||
const programFiles = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-git-bash-env-"));
|
||||
tempDirs.push(programFiles);
|
||||
const gitRoot = path.join(programFiles, "Git");
|
||||
const bashPath = path.join(gitRoot, "bin", "bash.exe");
|
||||
const usrBin = path.join(gitRoot, "usr", "bin");
|
||||
fs.mkdirSync(path.dirname(bashPath), { recursive: true });
|
||||
fs.mkdirSync(path.join(gitRoot, "cmd"), { recursive: true });
|
||||
fs.mkdirSync(usrBin, { recursive: true });
|
||||
fs.writeFileSync(bashPath, "");
|
||||
fs.writeFileSync(path.join(gitRoot, "cmd", "git.exe"), "");
|
||||
process.env.PATH = path.join(programFiles, "OtherBin");
|
||||
|
||||
const env = getBashShellEnv(bashPath);
|
||||
|
||||
expect(env.PATH?.split(path.delimiter)[0]).toBe(usrBin);
|
||||
expect(env.PATH).toContain(process.env.PATH);
|
||||
expect(Object.keys(env).filter((key) => key.toLowerCase() === "path")).toEqual(["PATH"]);
|
||||
});
|
||||
|
||||
it("recognizes portable Git for Windows installs", () => {
|
||||
const gitRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-portable-git-"));
|
||||
tempDirs.push(gitRoot);
|
||||
const bashPath = path.join(gitRoot, "usr", "bin", "bash.exe");
|
||||
const usrBin = path.dirname(bashPath);
|
||||
fs.mkdirSync(usrBin, { recursive: true });
|
||||
fs.mkdirSync(path.join(gitRoot, "cmd"), { recursive: true });
|
||||
fs.writeFileSync(bashPath, "");
|
||||
fs.writeFileSync(path.join(gitRoot, "cmd", "git.exe"), "");
|
||||
|
||||
expect(getBashShellEnv(bashPath).PATH?.split(path.delimiter)[0]).toBe(usrBin);
|
||||
});
|
||||
|
||||
it("leaves unrelated MSYS2 installs unchanged", () => {
|
||||
const msysRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-msys2-"));
|
||||
tempDirs.push(msysRoot);
|
||||
const bashPath = path.join(msysRoot, "usr", "bin", "bash.exe");
|
||||
fs.mkdirSync(path.dirname(bashPath), { recursive: true });
|
||||
fs.writeFileSync(bashPath, "");
|
||||
process.env.PATH = path.join(msysRoot, "ucrt64", "bin");
|
||||
|
||||
const env = getBashShellEnv(bashPath);
|
||||
|
||||
expect(env.PATH?.split(path.delimiter)[0]).not.toBe(path.dirname(bashPath));
|
||||
expect(env.PATH).toContain(process.env.PATH);
|
||||
});
|
||||
|
||||
it.each(["System32", "Sysnative"])(
|
||||
"uses stdin transport for the legacy %s WSL launcher",
|
||||
(systemDirectory) => {
|
||||
@@ -267,24 +315,49 @@ describe("getBashShellConfig", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("getShellEnv", () => {
|
||||
describe("getBashShellEnv", () => {
|
||||
let envSnapshot: ReturnType<typeof captureEnv>;
|
||||
|
||||
beforeEach(() => {
|
||||
envSnapshot = captureEnv(["PATH"]);
|
||||
envSnapshot = captureEnv(["PATH", "Path"]);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
envSnapshot.restore();
|
||||
});
|
||||
|
||||
it("returns an env object with the OpenClaw bin dir on PATH", () => {
|
||||
process.env.PATH = "/usr/bin";
|
||||
const env = getShellEnv();
|
||||
const env = getBashShellEnv();
|
||||
|
||||
expect(env.PATH).toContain("/usr/bin");
|
||||
expect(env.PATH).toContain(".openclaw");
|
||||
});
|
||||
|
||||
it("collapses case-insensitive PATH duplicates before Windows spawn", () => {
|
||||
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||
|
||||
const env = getBashShellEnv(undefined, { PATH: "/selected", Path: "/discarded" });
|
||||
|
||||
expect(Object.keys(env).filter((key) => key.toLowerCase() === "path")).toEqual(["PATH"]);
|
||||
expect(env.PATH).toContain("/selected");
|
||||
expect(env.PATH).not.toContain("/discarded");
|
||||
});
|
||||
|
||||
it.runIf(isWin)("passes one canonical PATH entry to a child process", () => {
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[
|
||||
"-e",
|
||||
"process.stdout.write(JSON.stringify(Object.keys(process.env).filter((key) => key.toLowerCase() === 'path')))",
|
||||
],
|
||||
{ encoding: "utf8", env: getBashShellEnv() },
|
||||
);
|
||||
|
||||
expect(result.status).toBe(0);
|
||||
expect(JSON.parse(result.stdout)).toEqual(["PATH"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("detectRuntimeShell", () => {
|
||||
|
||||
@@ -101,6 +101,64 @@ function resolveWindowsBashPath(env: NodeJS.ProcessEnv = process.env): string |
|
||||
return resolveShellFromPath("bash.exe", env) ?? resolveShellFromPath("bash", env);
|
||||
}
|
||||
|
||||
const WINDOWS_GIT_BASH_CACHE_LIMIT = 16;
|
||||
const windowsGitBashUsrBinCache = new Map<string, string | undefined>();
|
||||
let defaultWindowsGitBashUsrBinResolved = false;
|
||||
let defaultWindowsGitBashUsrBin: string | undefined;
|
||||
|
||||
function resolveWindowsGitBashUsrBin(shellPath: string): string | undefined {
|
||||
const cacheKey = path.resolve(shellPath).toLowerCase();
|
||||
if (windowsGitBashUsrBinCache.has(cacheKey)) {
|
||||
return windowsGitBashUsrBinCache.get(cacheKey);
|
||||
}
|
||||
|
||||
const normalized = path.normalize(shellPath);
|
||||
const shellName = path.basename(normalized).toLowerCase();
|
||||
const binDir = path.dirname(normalized);
|
||||
let gitRoot: string | undefined;
|
||||
if (
|
||||
(shellName === "bash.exe" || shellName === "bash") &&
|
||||
path.basename(binDir).toLowerCase() === "bin"
|
||||
) {
|
||||
const parent = path.dirname(binDir);
|
||||
gitRoot = path.basename(parent).toLowerCase() === "usr" ? path.dirname(parent) : parent;
|
||||
}
|
||||
|
||||
const usrBin = gitRoot ? path.join(gitRoot, "usr", "bin") : undefined;
|
||||
const resolved =
|
||||
gitRoot &&
|
||||
fs.existsSync(path.join(gitRoot, "cmd", "git.exe")) &&
|
||||
usrBin &&
|
||||
fs.existsSync(usrBin)
|
||||
? usrBin
|
||||
: undefined;
|
||||
if (windowsGitBashUsrBinCache.size >= WINDOWS_GIT_BASH_CACHE_LIMIT) {
|
||||
const oldestKey = windowsGitBashUsrBinCache.keys().next().value;
|
||||
if (oldestKey) {
|
||||
windowsGitBashUsrBinCache.delete(oldestKey);
|
||||
}
|
||||
}
|
||||
windowsGitBashUsrBinCache.set(cacheKey, resolved);
|
||||
return resolved;
|
||||
}
|
||||
|
||||
function getWindowsGitBashUsrBin(shellPath?: string): string | undefined {
|
||||
if (process.platform !== "win32") {
|
||||
return undefined;
|
||||
}
|
||||
if (shellPath) {
|
||||
return resolveWindowsGitBashUsrBin(shellPath);
|
||||
}
|
||||
if (!defaultWindowsGitBashUsrBinResolved) {
|
||||
defaultWindowsGitBashUsrBinResolved = true;
|
||||
const resolvedShell = resolveWindowsBashPath();
|
||||
defaultWindowsGitBashUsrBin = resolvedShell
|
||||
? resolveWindowsGitBashUsrBin(resolvedShell)
|
||||
: undefined;
|
||||
}
|
||||
return defaultWindowsGitBashUsrBin;
|
||||
}
|
||||
|
||||
function isLegacyWslBashPath(shellPath: string): boolean {
|
||||
const normalized = shellPath.replace(/\//g, "\\").toLowerCase();
|
||||
return /(?:^|\\)windows\\(?:system32|sysnative)\\bash\.exe$/.test(normalized);
|
||||
@@ -328,19 +386,46 @@ export function sanitizeBinaryOutput(
|
||||
return chunks.join("");
|
||||
}
|
||||
|
||||
export function getShellEnv(): NodeJS.ProcessEnv {
|
||||
function getShellEnv(sourceEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
|
||||
const binDir = getBinDir();
|
||||
const pathKey = Object.keys(process.env).find((key) => key.toLowerCase() === "path") ?? "PATH";
|
||||
const currentPath = process.env[pathKey] ?? "";
|
||||
const pathKeys = Object.keys(sourceEnv).filter((key) => key.toLowerCase() === "path");
|
||||
// Node sorts Windows environment keys and passes only the first case-insensitive match.
|
||||
// Collapse duplicates before spawning so callers and child processes see the same PATH.
|
||||
const sourcePathKey = process.platform === "win32" ? pathKeys.toSorted()[0] : pathKeys[0];
|
||||
const pathKey = process.platform === "win32" ? "PATH" : (sourcePathKey ?? "PATH");
|
||||
const currentPath = sourcePathKey ? (sourceEnv[sourcePathKey] ?? "") : "";
|
||||
const pathEntries = currentPath.split(path.delimiter).filter(Boolean);
|
||||
const updatedPath = pathEntries.includes(binDir)
|
||||
? currentPath
|
||||
: [binDir, currentPath].filter(Boolean).join(path.delimiter);
|
||||
const env = { ...sourceEnv };
|
||||
if (process.platform === "win32") {
|
||||
for (const key of pathKeys) {
|
||||
delete env[key];
|
||||
}
|
||||
}
|
||||
env[pathKey] = updatedPath;
|
||||
return env;
|
||||
}
|
||||
|
||||
return {
|
||||
...process.env,
|
||||
[pathKey]: updatedPath,
|
||||
};
|
||||
export function getBashShellEnv(
|
||||
shellPath?: string,
|
||||
sourceEnv: NodeJS.ProcessEnv = process.env,
|
||||
): NodeJS.ProcessEnv {
|
||||
const env = getShellEnv(sourceEnv);
|
||||
const usrBin = getWindowsGitBashUsrBin(shellPath);
|
||||
if (!usrBin) {
|
||||
return env;
|
||||
}
|
||||
|
||||
const currentPath = env.PATH ?? "";
|
||||
const pathEntries = currentPath.split(path.delimiter).filter(Boolean);
|
||||
const normalizedUsrBin = usrBin.toLowerCase();
|
||||
env.PATH = [
|
||||
usrBin,
|
||||
...pathEntries.filter((entry) => entry.toLowerCase() !== normalizedUsrBin),
|
||||
].join(path.delimiter);
|
||||
return env;
|
||||
}
|
||||
|
||||
export function killProcessTree(pid: number, opts?: KillProcessTreeOptions): void {
|
||||
|
||||
Reference in New Issue
Block a user