Skip to content

Commit ecfcaa0

Browse files
ooiuuiisteipete
ooiuuii
and
steipete
authored
fix(shell): keep Git Bash coreutils on PATH (#108136)
* fix(windows): expose Git Bash coreutils to commands Co-authored-by: luyifan <[email protected]> * 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 <[email protected]>
1 parent f3d1f02 commit ecfcaa0

6 files changed

Lines changed: 207 additions & 19 deletions

File tree

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1982,7 +1982,7 @@
19821982
"test:unit:fast:audit": "node scripts/test-unit-fast-audit.mjs",
19831983
"test:voicecall:closedloop": "node scripts/test-voicecall-closedloop.mjs",
19841984
"test:watch": "node scripts/test-projects.mjs --watch",
1985-
"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",
1985+
"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",
19861986
"tool-display:check": "node --import tsx scripts/tool-display.ts --check",
19871987
"tool-display:write": "node --import tsx scripts/tool-display.ts --write",
19881988
"ts-topology": "node --import tsx scripts/ts-topology.ts",

‎src/agents/sessions/resolve-config-value.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,11 @@
44
*/
55

66
import { execSync, spawnSync } from "node:child_process";
7-
import { buildShellCommandInvocation, getBashShellConfig } from "../shell-utils.js";
7+
import {
8+
buildShellCommandInvocation,
9+
getBashShellConfig,
10+
getBashShellEnv,
11+
} from "../shell-utils.js";
812

913
// Cache for shell command results (persists for process lifetime)
1014
const commandResultCache = new Map<string, string | undefined>();
@@ -27,7 +31,8 @@ function executeWithConfiguredShell(command: string): {
2731
value: string | undefined;
2832
} {
2933
try {
30-
const invocation = buildShellCommandInvocation(command, getBashShellConfig());
34+
const shellConfig = getBashShellConfig();
35+
const invocation = buildShellCommandInvocation(command, shellConfig);
3136
const [shell, ...args] = invocation.argv;
3237
const result = spawnSync(shell, args, {
3338
encoding: "utf-8",
@@ -36,6 +41,7 @@ function executeWithConfiguredShell(command: string): {
3641
stdio: [invocation.stdin, "pipe", "ignore"],
3742
shell: false,
3843
windowsHide: true,
44+
env: getBashShellEnv(shellConfig.shell),
3945
});
4046

4147
if (result.error) {

‎src/agents/sessions/tools/bash.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import type { AgentTool } from "../../runtime/index.js";
1818
import {
1919
buildShellCommandInvocation,
2020
getBashShellConfig,
21-
getShellEnv,
21+
getBashShellEnv,
2222
killProcessTree,
2323
} from "../../shell-utils.js";
2424
import type { ToolDefinition, ToolRenderResultOptions } from "../extensions/types.js";
@@ -76,7 +76,7 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas
7676
buffer: false,
7777
cwd,
7878
detached: process.platform !== "win32",
79-
env: env ?? getShellEnv(),
79+
env: env ?? getBashShellEnv(shellConfig.shell),
8080
...(invocation.input === undefined ? {} : { input: invocation.input }),
8181
reject: false,
8282
stdio: [invocation.stdin, "pipe", "pipe"],
@@ -160,8 +160,9 @@ function resolveSpawnContext(
160160
command: string,
161161
cwd: string,
162162
spawnHook?: BashSpawnHook,
163+
shellPath?: string,
163164
): BashSpawnContext {
164-
const baseContext: BashSpawnContext = { command, cwd, env: { ...getShellEnv() } };
165+
const baseContext: BashSpawnContext = { command, cwd, env: getBashShellEnv(shellPath) };
165166
return spawnHook ? spawnHook(baseContext) : baseContext;
166167
}
167168

@@ -322,7 +323,7 @@ export function createBashToolDefinition(
322323
void ctx;
323324
resolveBashTimeoutMs(timeout);
324325
const resolvedCommand = commandPrefix ? `${commandPrefix}\n${command}` : command;
325-
const spawnContext = resolveSpawnContext(resolvedCommand, cwd, spawnHook);
326+
const spawnContext = resolveSpawnContext(resolvedCommand, cwd, spawnHook, options?.shellPath);
326327
const output = new OutputAccumulator({ tempFilePrefix: "openclaw-bash" });
327328
let acceptingOutput = true;
328329
let updateTimer: NodeJS.Timeout | undefined;

‎src/agents/sessions/windows-git-bash-path.test.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { describe, expect, it } from "vitest";
2+
import { resolveConfigValueUncached } from "./resolve-config-value.js";
3+
import { createBashTool } from "./tools/bash.js";
4+
5+
const isWindows = process.platform === "win32";
6+
7+
describe.runIf(isWindows)("Git Bash PATH integration", () => {
8+
it("exposes Git coreutils through the Bash tool", async () => {
9+
const tool = createBashTool(process.cwd());
10+
11+
const result = await tool.execute("windows-git-bash-path", {
12+
command: "command -v cygpath",
13+
});
14+
15+
expect(result.content).toEqual([
16+
expect.objectContaining({ type: "text", text: expect.stringMatching(/usr\/bin\/cygpath/i) }),
17+
]);
18+
});
19+
20+
it("exposes Git coreutils to !command config resolution", () => {
21+
expect(resolveConfigValueUncached("!command -v cygpath")).toMatch(/usr\/bin\/cygpath/i);
22+
});
23+
});

‎src/agents/shell-utils.test.ts‎

Lines changed: 78 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// Verifies shell selection, PATH lookup, and platform-specific shell helpers.
2+
import { spawnSync } from "node:child_process";
23
import fs from "node:fs";
34
import os from "node:os";
45
import path from "node:path";
@@ -8,7 +9,7 @@ import {
89
buildShellCommandInvocation,
910
detectRuntimeShell,
1011
getBashShellConfig,
11-
getShellEnv,
12+
getBashShellEnv,
1213
getShellConfig,
1314
sanitizeBinaryOutput,
1415
} from "./shell-utils.js";
@@ -195,7 +196,7 @@ describe("getBashShellConfig", () => {
195196
let envSnapshot: ReturnType<typeof captureEnv>;
196197

197198
beforeEach(() => {
198-
envSnapshot = captureEnv(["ProgramFiles", "ProgramFiles(x86)", "PATH"]);
199+
envSnapshot = captureEnv(["ProgramFiles", "ProgramFiles(x86)", "PATH", "Path"]);
199200
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
200201
});
201202

@@ -225,6 +226,53 @@ describe("getBashShellConfig", () => {
225226
});
226227
});
227228

229+
it("prepends coreutils for a standard Git for Windows install", () => {
230+
const programFiles = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-git-bash-env-"));
231+
tempDirs.push(programFiles);
232+
const gitRoot = path.join(programFiles, "Git");
233+
const bashPath = path.join(gitRoot, "bin", "bash.exe");
234+
const usrBin = path.join(gitRoot, "usr", "bin");
235+
fs.mkdirSync(path.dirname(bashPath), { recursive: true });
236+
fs.mkdirSync(path.join(gitRoot, "cmd"), { recursive: true });
237+
fs.mkdirSync(usrBin, { recursive: true });
238+
fs.writeFileSync(bashPath, "");
239+
fs.writeFileSync(path.join(gitRoot, "cmd", "git.exe"), "");
240+
process.env.PATH = path.join(programFiles, "OtherBin");
241+
242+
const env = getBashShellEnv(bashPath);
243+
244+
expect(env.PATH?.split(path.delimiter)[0]).toBe(usrBin);
245+
expect(env.PATH).toContain(process.env.PATH);
246+
expect(Object.keys(env).filter((key) => key.toLowerCase() === "path")).toEqual(["PATH"]);
247+
});
248+
249+
it("recognizes portable Git for Windows installs", () => {
250+
const gitRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-portable-git-"));
251+
tempDirs.push(gitRoot);
252+
const bashPath = path.join(gitRoot, "usr", "bin", "bash.exe");
253+
const usrBin = path.dirname(bashPath);
254+
fs.mkdirSync(usrBin, { recursive: true });
255+
fs.mkdirSync(path.join(gitRoot, "cmd"), { recursive: true });
256+
fs.writeFileSync(bashPath, "");
257+
fs.writeFileSync(path.join(gitRoot, "cmd", "git.exe"), "");
258+
259+
expect(getBashShellEnv(bashPath).PATH?.split(path.delimiter)[0]).toBe(usrBin);
260+
});
261+
262+
it("leaves unrelated MSYS2 installs unchanged", () => {
263+
const msysRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-msys2-"));
264+
tempDirs.push(msysRoot);
265+
const bashPath = path.join(msysRoot, "usr", "bin", "bash.exe");
266+
fs.mkdirSync(path.dirname(bashPath), { recursive: true });
267+
fs.writeFileSync(bashPath, "");
268+
process.env.PATH = path.join(msysRoot, "ucrt64", "bin");
269+
270+
const env = getBashShellEnv(bashPath);
271+
272+
expect(env.PATH?.split(path.delimiter)[0]).not.toBe(path.dirname(bashPath));
273+
expect(env.PATH).toContain(process.env.PATH);
274+
});
275+
228276
it.each(["System32", "Sysnative"])(
229277
"uses stdin transport for the legacy %s WSL launcher",
230278
(systemDirectory) => {
@@ -267,24 +315,49 @@ describe("getBashShellConfig", () => {
267315
});
268316
});
269317

270-
describe("getShellEnv", () => {
318+
describe("getBashShellEnv", () => {
271319
let envSnapshot: ReturnType<typeof captureEnv>;
272320

273321
beforeEach(() => {
274-
envSnapshot = captureEnv(["PATH"]);
322+
envSnapshot = captureEnv(["PATH", "Path"]);
275323
});
276324

277325
afterEach(() => {
326+
vi.restoreAllMocks();
278327
envSnapshot.restore();
279328
});
280329

281330
it("returns an env object with the OpenClaw bin dir on PATH", () => {
282331
process.env.PATH = "/usr/bin";
283-
const env = getShellEnv();
332+
const env = getBashShellEnv();
284333

285334
expect(env.PATH).toContain("/usr/bin");
286335
expect(env.PATH).toContain(".openclaw");
287336
});
337+
338+
it("collapses case-insensitive PATH duplicates before Windows spawn", () => {
339+
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
340+
341+
const env = getBashShellEnv(undefined, { PATH: "/selected", Path: "/discarded" });
342+
343+
expect(Object.keys(env).filter((key) => key.toLowerCase() === "path")).toEqual(["PATH"]);
344+
expect(env.PATH).toContain("/selected");
345+
expect(env.PATH).not.toContain("/discarded");
346+
});
347+
348+
it.runIf(isWin)("passes one canonical PATH entry to a child process", () => {
349+
const result = spawnSync(
350+
process.execPath,
351+
[
352+
"-e",
353+
"process.stdout.write(JSON.stringify(Object.keys(process.env).filter((key) => key.toLowerCase() === 'path')))",
354+
],
355+
{ encoding: "utf8", env: getBashShellEnv() },
356+
);
357+
358+
expect(result.status).toBe(0);
359+
expect(JSON.parse(result.stdout)).toEqual(["PATH"]);
360+
});
288361
});
289362

290363
describe("detectRuntimeShell", () => {

‎src/agents/shell-utils.ts‎

Lines changed: 92 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,64 @@ function resolveWindowsBashPath(env: NodeJS.ProcessEnv = process.env): string |
101101
return resolveShellFromPath("bash.exe", env) ?? resolveShellFromPath("bash", env);
102102
}
103103

104+
const WINDOWS_GIT_BASH_CACHE_LIMIT = 16;
105+
const windowsGitBashUsrBinCache = new Map<string, string | undefined>();
106+
let defaultWindowsGitBashUsrBinResolved = false;
107+
let defaultWindowsGitBashUsrBin: string | undefined;
108+
109+
function resolveWindowsGitBashUsrBin(shellPath: string): string | undefined {
110+
const cacheKey = path.resolve(shellPath).toLowerCase();
111+
if (windowsGitBashUsrBinCache.has(cacheKey)) {
112+
return windowsGitBashUsrBinCache.get(cacheKey);
113+
}
114+
115+
const normalized = path.normalize(shellPath);
116+
const shellName = path.basename(normalized).toLowerCase();
117+
const binDir = path.dirname(normalized);
118+
let gitRoot: string | undefined;
119+
if (
120+
(shellName === "bash.exe" || shellName === "bash") &&
121+
path.basename(binDir).toLowerCase() === "bin"
122+
) {
123+
const parent = path.dirname(binDir);
124+
gitRoot = path.basename(parent).toLowerCase() === "usr" ? path.dirname(parent) : parent;
125+
}
126+
127+
const usrBin = gitRoot ? path.join(gitRoot, "usr", "bin") : undefined;
128+
const resolved =
129+
gitRoot &&
130+
fs.existsSync(path.join(gitRoot, "cmd", "git.exe")) &&
131+
usrBin &&
132+
fs.existsSync(usrBin)
133+
? usrBin
134+
: undefined;
135+
if (windowsGitBashUsrBinCache.size >= WINDOWS_GIT_BASH_CACHE_LIMIT) {
136+
const oldestKey = windowsGitBashUsrBinCache.keys().next().value;
137+
if (oldestKey) {
138+
windowsGitBashUsrBinCache.delete(oldestKey);
139+
}
140+
}
141+
windowsGitBashUsrBinCache.set(cacheKey, resolved);
142+
return resolved;
143+
}
144+
145+
function getWindowsGitBashUsrBin(shellPath?: string): string | undefined {
146+
if (process.platform !== "win32") {
147+
return undefined;
148+
}
149+
if (shellPath) {
150+
return resolveWindowsGitBashUsrBin(shellPath);
151+
}
152+
if (!defaultWindowsGitBashUsrBinResolved) {
153+
defaultWindowsGitBashUsrBinResolved = true;
154+
const resolvedShell = resolveWindowsBashPath();
155+
defaultWindowsGitBashUsrBin = resolvedShell
156+
? resolveWindowsGitBashUsrBin(resolvedShell)
157+
: undefined;
158+
}
159+
return defaultWindowsGitBashUsrBin;
160+
}
161+
104162
function isLegacyWslBashPath(shellPath: string): boolean {
105163
const normalized = shellPath.replace(/\//g, "\\").toLowerCase();
106164
return /(?:^|\\)windows\\(?:system32|sysnative)\\bash\.exe$/.test(normalized);
@@ -328,19 +386,46 @@ export function sanitizeBinaryOutput(
328386
return chunks.join("");
329387
}
330388

331-
export function getShellEnv(): NodeJS.ProcessEnv {
389+
function getShellEnv(sourceEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
332390
const binDir = getBinDir();
333-
const pathKey = Object.keys(process.env).find((key) => key.toLowerCase() === "path") ?? "PATH";
334-
const currentPath = process.env[pathKey] ?? "";
391+
const pathKeys = Object.keys(sourceEnv).filter((key) => key.toLowerCase() === "path");
392+
// Node sorts Windows environment keys and passes only the first case-insensitive match.
393+
// Collapse duplicates before spawning so callers and child processes see the same PATH.
394+
const sourcePathKey = process.platform === "win32" ? pathKeys.toSorted()[0] : pathKeys[0];
395+
const pathKey = process.platform === "win32" ? "PATH" : (sourcePathKey ?? "PATH");
396+
const currentPath = sourcePathKey ? (sourceEnv[sourcePathKey] ?? "") : "";
335397
const pathEntries = currentPath.split(path.delimiter).filter(Boolean);
336398
const updatedPath = pathEntries.includes(binDir)
337399
? currentPath
338400
: [binDir, currentPath].filter(Boolean).join(path.delimiter);
401+
const env = { ...sourceEnv };
402+
if (process.platform === "win32") {
403+
for (const key of pathKeys) {
404+
delete env[key];
405+
}
406+
}
407+
env[pathKey] = updatedPath;
408+
return env;
409+
}
410+
411+
export function getBashShellEnv(
412+
shellPath?: string,
413+
sourceEnv: NodeJS.ProcessEnv = process.env,
414+
): NodeJS.ProcessEnv {
415+
const env = getShellEnv(sourceEnv);
416+
const usrBin = getWindowsGitBashUsrBin(shellPath);
417+
if (!usrBin) {
418+
return env;
419+
}
339420

340-
return {
341-
...process.env,
342-
[pathKey]: updatedPath,
343-
};
421+
const currentPath = env.PATH ?? "";
422+
const pathEntries = currentPath.split(path.delimiter).filter(Boolean);
423+
const normalizedUsrBin = usrBin.toLowerCase();
424+
env.PATH = [
425+
usrBin,
426+
...pathEntries.filter((entry) => entry.toLowerCase() !== normalizedUsrBin),
427+
].join(path.delimiter);
428+
return env;
344429
}
345430

346431
export function killProcessTree(pid: number, opts?: KillProcessTreeOptions): void {

0 commit comments

Comments
 (0)