Skip to content

Commit c8f723a

Browse files
committed
fix: Windows path handling and MSYS2 process termination
- getDefaultBgOutputDir: return native path for Node fs APIs, POSIX conversion happens at shell command construction time - LocalBackgroundHandle.terminate: run winpid lookup via bash since /proc/PID/winpid is MSYS2's virtual filesystem, not visible to Node.js - Add toPosixPath tests for path conversion behavior - Add Windows POSIX path handling tests to backgroundCommands.test.ts
1 parent 99af592 commit c8f723a

File tree

5 files changed

+173
-9
lines changed

5 files changed

+173
-9
lines changed

src/node/runtime/LocalBackgroundHandle.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { BackgroundHandle } from "./Runtime";
22
import { parseExitCode, EXIT_CODE_SIGKILL, EXIT_CODE_SIGTERM } from "./backgroundCommands";
33
import { log } from "@/node/services/log";
44
import { execAsync } from "@/node/utils/disposableExec";
5+
import { getBashPath } from "@/node/utils/main/bashPath";
56
import * as fs from "fs/promises";
67
import * as path from "path";
78

@@ -53,12 +54,22 @@ export class LocalBackgroundHandle implements BackgroundHandle {
5354
const exitCodePath = path.join(this.outputDir, "exit_code");
5455

5556
// Windows: convert MSYS2 PID to Windows PID and use taskkill
57+
// Must run via bash because /proc/PID/winpid is MSYS2's virtual filesystem,
58+
// not visible to Node.js running as a native Windows process
5659
if (process.platform === "win32") {
5760
try {
58-
const winpidPath = `/proc/${this.pid}/winpid`;
59-
const winpid = (await fs.readFile(winpidPath, "utf-8")).trim();
60-
log.debug(`LocalBackgroundHandle: Killing Windows PID ${winpid} (MSYS2 PID ${this.pid})`);
61-
using proc = execAsync(`taskkill /PID ${winpid} /F /T`);
61+
// Script reads winpid from MSYS2's /proc, then uses taskkill
62+
// taskkill /F = force, /T = kill child processes
63+
// Double slashes needed because bash interprets single slash as path
64+
const terminateScript = `
65+
if [ -f /proc/${this.pid}/winpid ]; then
66+
winpid=$(cat /proc/${this.pid}/winpid)
67+
taskkill //PID $winpid //F //T 2>/dev/null
68+
fi
69+
kill -9 ${this.pid} 2>/dev/null || true
70+
`;
71+
log.debug(`LocalBackgroundHandle: Terminating MSYS2 PID ${this.pid} via bash`);
72+
using proc = execAsync(terminateScript, { shell: getBashPath() });
6273
await proc.result;
6374
} catch (error) {
6475
// Process already dead or winpid unavailable

src/node/runtime/WorktreeRuntime.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,9 @@ export class WorktreeRuntime extends LocalBaseRuntime {
287287

288288
// Force delete the directory (use bash shell for rm -rf on Windows)
289289
// Convert to POSIX path for Git Bash compatibility on Windows
290-
using rmProc = execAsync(`rm -rf "${toPosixPath(deletedPath)}"`, { shell: getBashPath() });
290+
using rmProc = execAsync(`rm -rf "${toPosixPath(deletedPath)}"`, {
291+
shell: getBashPath(),
292+
});
291293
await rmProc.result;
292294

293295
return { success: true, deletedPath };

src/node/runtime/backgroundCommands.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,4 +288,78 @@ describe("backgroundCommands", () => {
288288
expect(EXIT_CODE_SIGTERM).toBe(143); // 128 + 15
289289
});
290290
});
291+
292+
// Windows/MSYS2 path handling tests
293+
// These verify that POSIX-converted paths work correctly in shell commands
294+
describe("Windows POSIX path handling", () => {
295+
describe("buildWrapperScript with POSIX-converted paths", () => {
296+
it("works with POSIX-style paths from toPosixPath", () => {
297+
// Simulates paths after toPosixPath conversion on Windows:
298+
// C:\Users\test\exit_code → /c/Users/test/exit_code
299+
const result = buildWrapperScript({
300+
exitCodePath: "/c/Users/test/bg-123/exit_code",
301+
cwd: "/c/Projects/myapp",
302+
script: "npm start",
303+
});
304+
305+
expect(result).toContain("cd '/c/Projects/myapp'");
306+
expect(result).toContain("'/c/Users/test/bg-123/exit_code'");
307+
});
308+
309+
it("handles POSIX paths with spaces (Program Files)", () => {
310+
const result = buildWrapperScript({
311+
exitCodePath: "/c/Program Files/mux/exit_code",
312+
cwd: "/c/Program Files/project",
313+
script: "node server.js",
314+
});
315+
316+
expect(result).toContain("cd '/c/Program Files/project'");
317+
expect(result).toContain("'/c/Program Files/mux/exit_code'");
318+
});
319+
});
320+
321+
describe("buildSpawnCommand with POSIX-converted paths", () => {
322+
it("works with POSIX-style paths for redirection", () => {
323+
const result = buildSpawnCommand({
324+
wrapperScript: "echo test",
325+
stdoutPath: "/c/temp/mux-bashes/stdout.log",
326+
stderrPath: "/c/temp/mux-bashes/stderr.log",
327+
});
328+
329+
expect(result).toContain("> '/c/temp/mux-bashes/stdout.log'");
330+
expect(result).toContain("2> '/c/temp/mux-bashes/stderr.log'");
331+
});
332+
333+
it("handles paths with spaces in POSIX format", () => {
334+
const result = buildSpawnCommand({
335+
wrapperScript: "echo test",
336+
stdoutPath: "/c/Users/John Doe/AppData/Local/Temp/mux-bashes/stdout.log",
337+
stderrPath: "/c/Users/John Doe/AppData/Local/Temp/mux-bashes/stderr.log",
338+
});
339+
340+
expect(result).toContain("'/c/Users/John Doe/AppData/Local/Temp/mux-bashes/stdout.log'");
341+
expect(result).toContain("'/c/Users/John Doe/AppData/Local/Temp/mux-bashes/stderr.log'");
342+
});
343+
344+
it("handles quoted bash path with spaces (Git Bash default location)", () => {
345+
const result = buildSpawnCommand({
346+
wrapperScript: "echo test",
347+
stdoutPath: "/c/temp/stdout.log",
348+
stderrPath: "/c/temp/stderr.log",
349+
bashPath: "/c/Program Files/Git/bin/bash.exe",
350+
});
351+
352+
// Bash path should be quoted
353+
expect(result).toContain("'/c/Program Files/Git/bin/bash.exe'");
354+
});
355+
});
356+
357+
describe("buildTerminateCommand with POSIX paths", () => {
358+
it("works with POSIX-style exit code path", () => {
359+
const result = buildTerminateCommand(1234, "/c/temp/mux-bashes/bg-abc/exit_code");
360+
361+
expect(result).toContain("'/c/temp/mux-bashes/bg-abc/exit_code'");
362+
});
363+
});
364+
});
291365
});

src/node/runtime/runtimeFactory.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as os from "os";
2+
import * as path from "path";
23

34
import type { Runtime } from "./Runtime";
45
import { LocalRuntime } from "./LocalRuntime";
@@ -7,19 +8,20 @@ import { SSHRuntime } from "./SSHRuntime";
78
import type { RuntimeConfig } from "@/common/types/runtime";
89
import { hasSrcBaseDir } from "@/common/types/runtime";
910
import { isIncompatibleRuntimeConfig } from "@/common/utils/runtimeCompatibility";
10-
import { toPosixPath } from "@/node/utils/paths";
1111

1212
// Re-export for backward compatibility with existing imports
1313
export { isIncompatibleRuntimeConfig };
1414

1515
/**
1616
* Get the default output directory for background processes.
1717
* Uses os.tmpdir() for platform-appropriate temp directory.
18-
* On Windows, converts to POSIX path using cygpath for Git Bash compatibility.
18+
*
19+
* Returns native path format (Windows or POSIX) since this is used by Node.js
20+
* filesystem APIs. Conversion to POSIX for Git Bash shell commands happens
21+
* at command construction time via toPosixPath().
1922
*/
2023
function getDefaultBgOutputDir(): string {
21-
const tempDir = os.tmpdir();
22-
return `${toPosixPath(tempDir)}/mux-bashes`;
24+
return path.join(os.tmpdir(), "mux-bashes");
2325
}
2426

2527
/**

src/node/utils/paths.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, test, expect } from "bun:test";
22
import { PlatformPaths } from "./paths.main";
3+
import { toPosixPath } from "./paths";
34
import * as os from "os";
45
import * as path from "path";
56

@@ -144,3 +145,77 @@ describe("PlatformPaths", () => {
144145
});
145146
});
146147
});
148+
149+
describe("toPosixPath", () => {
150+
describe("on non-Windows platforms", () => {
151+
test("returns POSIX paths unchanged", () => {
152+
if (process.platform !== "win32") {
153+
expect(toPosixPath("/home/user/project")).toBe("/home/user/project");
154+
expect(toPosixPath("/tmp/mux-bashes")).toBe("/tmp/mux-bashes");
155+
}
156+
});
157+
158+
test("returns paths with spaces unchanged", () => {
159+
if (process.platform !== "win32") {
160+
expect(toPosixPath("/home/user/my project")).toBe("/home/user/my project");
161+
}
162+
});
163+
164+
test("returns relative paths unchanged", () => {
165+
if (process.platform !== "win32") {
166+
expect(toPosixPath("relative/path/file.txt")).toBe("relative/path/file.txt");
167+
}
168+
});
169+
170+
test("returns empty string unchanged", () => {
171+
if (process.platform !== "win32") {
172+
expect(toPosixPath("")).toBe("");
173+
}
174+
});
175+
});
176+
177+
describe("path format handling", () => {
178+
test("handles paths with special characters", () => {
179+
const input = "/path/with spaces/and-dashes/under_scores";
180+
const result = toPosixPath(input);
181+
expect(typeof result).toBe("string");
182+
if (process.platform !== "win32") {
183+
expect(result).toBe(input);
184+
}
185+
});
186+
187+
test("handles deeply nested paths", () => {
188+
const input = "/a/b/c/d/e/f/g/h/i/j/file.txt";
189+
const result = toPosixPath(input);
190+
expect(typeof result).toBe("string");
191+
if (process.platform !== "win32") {
192+
expect(result).toBe(input);
193+
}
194+
});
195+
});
196+
197+
// Windows-specific behavior documentation
198+
// These tests document expected behavior but can only truly verify on Windows CI
199+
describe("Windows behavior (documented)", () => {
200+
test("converts Windows drive paths to POSIX format on Windows", () => {
201+
// On Windows with Git Bash/MSYS2, cygpath converts:
202+
// "C:\\Users\\test" → "/c/Users/test"
203+
// "C:\\Program Files\\Git" → "/c/Program Files/Git"
204+
// "D:\\Projects\\mux" → "/d/Projects/mux"
205+
//
206+
// On non-Windows, this is a no-op (returns input unchanged)
207+
if (process.platform === "win32") {
208+
// Real Windows test - only runs on Windows CI
209+
const result = toPosixPath("C:\\Users\\test");
210+
expect(result).toMatch(/^\/c\/Users\/test$/i);
211+
}
212+
});
213+
214+
test("falls back to original path if cygpath unavailable", () => {
215+
// If cygpath is not available (edge case), the function catches
216+
// the error and returns the original path unchanged
217+
// This prevents crashes if Git Bash is misconfigured
218+
expect(true).toBe(true); // Cannot easily test without mocking execSync
219+
});
220+
});
221+
});

0 commit comments

Comments
 (0)