Skip to content

Commit 8de6d12

Browse files
ammar-agentammar-agent
authored
🤖 Fix zombie processes via explicit resource management (#239)
Prevents zombie processes from `exec()` calls by wrapping them in a disposable resource that guarantees cleanup via TypeScript's `using` declarations. ## Core Solution All git operations now use explicit resource management: ```typescript using proc = execAsync("git status"); const { stdout } = await proc.result; // Process is automatically cleaned up when scope exits ``` **Key implementation details:** - Uses `close` event instead of `exit` to ensure all stdio streams are fully flushed before resolving - Only kills processes that haven't exited naturally (checks `exitCode` and `signalCode`) - Rejects when process is killed by signal (e.g., `SIGTERM`) ## Additional Fixes - Added `.unref()` to detached terminal spawns on macOS/Windows - ESLint rule prevents future `promisify(exec)` usage - Centralized in `src/utils/disposableExec.ts` to avoid duplication ## Testing - ✅ All unit tests pass (418 tests) - ✅ All integration tests pass (74 tests) - ✅ All E2E tests pass - ✅ ESLint rule active _Generated with `cmux`_
1 parent 16931a0 commit 8de6d12

File tree

10 files changed

+608
-39
lines changed

10 files changed

+608
-39
lines changed

‎eslint.config.mjs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,46 @@ import react from "eslint-plugin-react";
44
import reactHooks from "eslint-plugin-react-hooks";
55
import tseslint from "typescript-eslint";
66

7+
/**
8+
* Custom ESLint plugin for zombie process prevention
9+
* Enforces safe child_process patterns
10+
*/
11+
const localPlugin = {
12+
rules: {
13+
"no-unsafe-child-process": {
14+
meta: {
15+
type: "problem",
16+
docs: {
17+
description: "Prevent unsafe child_process usage that can cause zombie processes",
18+
},
19+
messages: {
20+
unsafePromisifyExec:
21+
"Do not use promisify(exec) directly. Use DisposableExec wrapper with 'using' declaration to prevent zombie processes.",
22+
},
23+
},
24+
create(context) {
25+
return {
26+
CallExpression(node) {
27+
// Ban promisify(exec)
28+
if (
29+
node.callee.type === "Identifier" &&
30+
node.callee.name === "promisify" &&
31+
node.arguments.length > 0 &&
32+
node.arguments[0].type === "Identifier" &&
33+
node.arguments[0].name === "exec"
34+
) {
35+
context.report({
36+
node,
37+
messageId: "unsafePromisifyExec",
38+
});
39+
}
40+
},
41+
};
42+
},
43+
},
44+
},
45+
};
46+
747
export default defineConfig([
848
{
949
ignores: [
@@ -53,6 +93,7 @@ export default defineConfig([
5393
plugins: {
5494
react,
5595
"react-hooks": reactHooks,
96+
local: localPlugin,
5697
},
5798
settings: {
5899
react: {
@@ -137,6 +178,9 @@ export default defineConfig([
137178
"react/react-in-jsx-scope": "off",
138179
"react/prop-types": "off",
139180

181+
// Zombie process prevention
182+
"local/no-unsafe-child-process": "error",
183+
140184
// Allow console for this app (it's a dev tool)
141185
"no-console": "off",
142186

‎src/git.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as fs from "fs/promises";
77
import { exec } from "child_process";
88
import { promisify } from "util";
99

10+
// eslint-disable-next-line local/no-unsafe-child-process -- Test file needs direct exec access for setup
1011
const execAsync = promisify(exec);
1112

1213
describe("createWorktree", () => {

‎src/git.ts‎

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
import { exec } from "child_process";
2-
import { promisify } from "util";
31
import * as fs from "fs";
42
import * as path from "path";
53
import type { Config } from "./config";
6-
7-
const execAsync = promisify(exec);
4+
import { execAsync } from "./utils/disposableExec";
85

96
export interface WorktreeResult {
107
success: boolean;
@@ -17,9 +14,10 @@ export interface CreateWorktreeOptions {
1714
}
1815

1916
export async function listLocalBranches(projectPath: string): Promise<string[]> {
20-
const { stdout } = await execAsync(
17+
using proc = execAsync(
2118
`git -C "${projectPath}" for-each-ref --format="%(refname:short)" refs/heads`
2219
);
20+
const { stdout } = await proc.result;
2321
return stdout
2422
.split("\n")
2523
.map((line) => line.trim())
@@ -29,7 +27,8 @@ export async function listLocalBranches(projectPath: string): Promise<string[]>
2927

3028
async function getCurrentBranch(projectPath: string): Promise<string | null> {
3129
try {
32-
const { stdout } = await execAsync(`git -C "${projectPath}" rev-parse --abbrev-ref HEAD`);
30+
using proc = execAsync(`git -C "${projectPath}" rev-parse --abbrev-ref HEAD`);
31+
const { stdout } = await proc.result;
3332
const branch = stdout.trim();
3433
if (!branch || branch === "HEAD") {
3534
return null;
@@ -108,19 +107,26 @@ export async function createWorktree(
108107

109108
// If branch already exists locally, reuse it instead of creating a new one
110109
if (localBranches.includes(branchName)) {
111-
await execAsync(`git -C "${projectPath}" worktree add "${workspacePath}" "${branchName}"`);
110+
using proc = execAsync(
111+
`git -C "${projectPath}" worktree add "${workspacePath}" "${branchName}"`
112+
);
113+
await proc.result;
112114
return { success: true, path: workspacePath };
113115
}
114116

115117
// Check if branch exists remotely (origin/<branchName>)
116-
const { stdout: remoteBranchesRaw } = await execAsync(`git -C "${projectPath}" branch -a`);
118+
using remoteBranchesProc = execAsync(`git -C "${projectPath}" branch -a`);
119+
const { stdout: remoteBranchesRaw } = await remoteBranchesProc.result;
117120
const branchExists = remoteBranchesRaw
118121
.split("\n")
119122
.map((b) => b.trim().replace(/^(\*)\s+/, ""))
120123
.some((b) => b === branchName || b === `remotes/origin/${branchName}`);
121124

122125
if (branchExists) {
123-
await execAsync(`git -C "${projectPath}" worktree add "${workspacePath}" "${branchName}"`);
126+
using proc = execAsync(
127+
`git -C "${projectPath}" worktree add "${workspacePath}" "${branchName}"`
128+
);
129+
await proc.result;
124130
return { success: true, path: workspacePath };
125131
}
126132

@@ -131,9 +137,10 @@ export async function createWorktree(
131137
};
132138
}
133139

134-
await execAsync(
140+
using proc = execAsync(
135141
`git -C "${projectPath}" worktree add -b "${branchName}" "${workspacePath}" "${normalizedTrunkBranch}"`
136142
);
143+
await proc.result;
137144

138145
return { success: true, path: workspacePath };
139146
} catch (error) {
@@ -149,9 +156,10 @@ export async function removeWorktree(
149156
): Promise<WorktreeResult> {
150157
try {
151158
// Remove the worktree (from the main repository context)
152-
await execAsync(
159+
using proc = execAsync(
153160
`git -C "${projectPath}" worktree remove "${workspacePath}" ${options.force ? "--force" : ""}`
154161
);
162+
await proc.result;
155163
return { success: true };
156164
} catch (error) {
157165
const message = error instanceof Error ? error.message : String(error);
@@ -161,7 +169,8 @@ export async function removeWorktree(
161169

162170
export async function pruneWorktrees(projectPath: string): Promise<WorktreeResult> {
163171
try {
164-
await execAsync(`git -C "${projectPath}" worktree prune`);
172+
using proc = execAsync(`git -C "${projectPath}" worktree prune`);
173+
await proc.result;
165174
return { success: true };
166175
} catch (error) {
167176
const message = error instanceof Error ? error.message : String(error);
@@ -190,7 +199,8 @@ export async function moveWorktree(
190199
}
191200

192201
// Move the worktree using git (from the main repository context)
193-
await execAsync(`git -C "${projectPath}" worktree move "${oldPath}" "${newPath}"`);
202+
using proc = execAsync(`git -C "${projectPath}" worktree move "${oldPath}" "${newPath}"`);
203+
await proc.result;
194204
return { success: true, path: newPath };
195205
} catch (error) {
196206
const message = error instanceof Error ? error.message : String(error);
@@ -200,7 +210,8 @@ export async function moveWorktree(
200210

201211
export async function listWorktrees(projectPath: string): Promise<string[]> {
202212
try {
203-
const { stdout } = await execAsync(`git -C "${projectPath}" worktree list --porcelain`);
213+
using proc = execAsync(`git -C "${projectPath}" worktree list --porcelain`);
214+
const { stdout } = await proc.result;
204215
const worktrees: string[] = [];
205216
const lines = stdout.split("\n");
206217

@@ -223,7 +234,8 @@ export async function listWorktrees(projectPath: string): Promise<string[]> {
223234

224235
export async function isGitRepository(projectPath: string): Promise<boolean> {
225236
try {
226-
await execAsync(`git -C "${projectPath}" rev-parse --git-dir`);
237+
using proc = execAsync(`git -C "${projectPath}" rev-parse --git-dir`);
238+
await proc.result;
227239
return true;
228240
} catch {
229241
return false;
@@ -238,7 +250,8 @@ export async function isGitRepository(projectPath: string): Promise<boolean> {
238250
export async function getMainWorktreeFromWorktree(worktreePath: string): Promise<string | null> {
239251
try {
240252
// Get the worktree list from the worktree itself
241-
const { stdout } = await execAsync(`git -C "${worktreePath}" worktree list --porcelain`);
253+
using proc = execAsync(`git -C "${worktreePath}" worktree list --porcelain`);
254+
const { stdout } = await proc.result;
242255
const lines = stdout.split("\n");
243256

244257
// The first worktree in the list is always the main worktree

‎src/services/gitService.ts‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
1-
import { exec } from "child_process";
2-
import { promisify } from "util";
31
import * as fs from "fs";
42
import * as fsPromises from "fs/promises";
53
import * as path from "path";
64
import type { Config } from "@/config";
7-
8-
const execAsync = promisify(exec);
5+
import { execAsync } from "@/utils/disposableExec";
96

107
export interface WorktreeResult {
118
success: boolean;
@@ -35,7 +32,8 @@ export async function createWorktree(
3532
}
3633

3734
// Check if branch exists
38-
const { stdout: branches } = await execAsync(`git -C "${projectPath}" branch -a`);
35+
using branchesProc = execAsync(`git -C "${projectPath}" branch -a`);
36+
const { stdout: branches } = await branchesProc.result;
3937
const branchExists = branches
4038
.split("\n")
4139
.some(
@@ -47,10 +45,16 @@ export async function createWorktree(
4745

4846
if (branchExists) {
4947
// Branch exists, create worktree with existing branch
50-
await execAsync(`git -C "${projectPath}" worktree add "${workspacePath}" "${branchName}"`);
48+
using proc = execAsync(
49+
`git -C "${projectPath}" worktree add "${workspacePath}" "${branchName}"`
50+
);
51+
await proc.result;
5152
} else {
5253
// Branch doesn't exist, create new branch with worktree
53-
await execAsync(`git -C "${projectPath}" worktree add -b "${branchName}" "${workspacePath}"`);
54+
using proc = execAsync(
55+
`git -C "${projectPath}" worktree add -b "${branchName}" "${workspacePath}"`
56+
);
57+
await proc.result;
5458
}
5559

5660
return { success: true, path: workspacePath };
@@ -67,9 +71,8 @@ export async function createWorktree(
6771
export async function isWorktreeClean(workspacePath: string): Promise<boolean> {
6872
try {
6973
// Check for uncommitted changes (staged or unstaged)
70-
const { stdout: statusOutput } = await execAsync(
71-
`git -C "${workspacePath}" status --porcelain`
72-
);
74+
using proc = execAsync(`git -C "${workspacePath}" status --porcelain`);
75+
const { stdout: statusOutput } = await proc.result;
7376
return statusOutput.trim() === "";
7477
} catch {
7578
// If git command fails, assume not clean (safer default)
@@ -98,9 +101,10 @@ export async function removeWorktree(
98101
): Promise<WorktreeResult> {
99102
try {
100103
// Remove the worktree (from the main repository context)
101-
await execAsync(
104+
using proc = execAsync(
102105
`git -C "${projectPath}" worktree remove "${workspacePath}" ${options.force ? "--force" : ""}`
103106
);
107+
await proc.result;
104108
return { success: true };
105109
} catch (error) {
106110
const message = error instanceof Error ? error.message : String(error);
@@ -110,7 +114,8 @@ export async function removeWorktree(
110114

111115
export async function pruneWorktrees(projectPath: string): Promise<WorktreeResult> {
112116
try {
113-
await execAsync(`git -C "${projectPath}" worktree prune`);
117+
using proc = execAsync(`git -C "${projectPath}" worktree prune`);
118+
await proc.result;
114119
return { success: true };
115120
} catch (error) {
116121
const message = error instanceof Error ? error.message : String(error);
@@ -259,7 +264,8 @@ export async function moveWorktree(
259264
}
260265

261266
// Move the worktree using git (from the main repository context)
262-
await execAsync(`git -C "${projectPath}" worktree move "${oldPath}" "${newPath}"`);
267+
using proc = execAsync(`git -C "${projectPath}" worktree move "${oldPath}" "${newPath}"`);
268+
await proc.result;
263269
return { success: true, path: newPath };
264270
} catch (error) {
265271
const message = error instanceof Error ? error.message : String(error);
@@ -269,7 +275,8 @@ export async function moveWorktree(
269275

270276
export async function listWorktrees(projectPath: string): Promise<string[]> {
271277
try {
272-
const { stdout } = await execAsync(`git -C "${projectPath}" worktree list --porcelain`);
278+
using proc = execAsync(`git -C "${projectPath}" worktree list --porcelain`);
279+
const { stdout } = await proc.result;
273280
const worktrees: string[] = [];
274281
const lines = stdout.split("\n");
275282

@@ -292,7 +299,8 @@ export async function listWorktrees(projectPath: string): Promise<string[]> {
292299

293300
export async function isGitRepository(projectPath: string): Promise<boolean> {
294301
try {
295-
await execAsync(`git -C "${projectPath}" rev-parse --git-dir`);
302+
using proc = execAsync(`git -C "${projectPath}" rev-parse --git-dir`);
303+
await proc.result;
296304
return true;
297305
} catch {
298306
return false;
@@ -307,7 +315,8 @@ export async function isGitRepository(projectPath: string): Promise<boolean> {
307315
export async function getMainWorktreeFromWorktree(worktreePath: string): Promise<string | null> {
308316
try {
309317
// Get the worktree list from the worktree itself
310-
const { stdout } = await execAsync(`git -C "${worktreePath}" worktree list --porcelain`);
318+
using proc = execAsync(`git -C "${worktreePath}" worktree list --porcelain`);
319+
const { stdout } = await proc.result;
311320
const lines = stdout.split("\n");
312321

313322
// The first worktree in the list is always the main worktree

‎src/services/ipcMain.ts‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -654,16 +654,26 @@ export class IpcMain {
654654
// macOS - try Ghostty first, fallback to Terminal.app
655655
const terminal = this.findAvailableCommand(["ghostty", "terminal"]);
656656
if (terminal === "ghostty") {
657-
spawn("open", ["-a", "Ghostty", workspacePath], { detached: true });
657+
const child = spawn("open", ["-a", "Ghostty", workspacePath], {
658+
detached: true,
659+
stdio: "ignore",
660+
});
661+
child.unref();
658662
} else {
659-
spawn("open", ["-a", "Terminal", workspacePath], { detached: true });
663+
const child = spawn("open", ["-a", "Terminal", workspacePath], {
664+
detached: true,
665+
stdio: "ignore",
666+
});
667+
child.unref();
660668
}
661669
} else if (process.platform === "win32") {
662670
// Windows
663-
spawn("cmd", ["/c", "start", "cmd", "/K", "cd", "/D", workspacePath], {
671+
const child = spawn("cmd", ["/c", "start", "cmd", "/K", "cd", "/D", workspacePath], {
664672
detached: true,
665673
shell: true,
674+
stdio: "ignore",
666675
});
676+
child.unref();
667677
} else {
668678
// Linux - try terminal emulators in order of preference
669679
// x-terminal-emulator is checked first as it respects user's system-wide preference

0 commit comments

Comments
 (0)