fix: multi-pane tmux, EPERM handling, strict process matching
Codex review iteration 1 fixes: - Iterate all tmux pane TTYs instead of only the first — prevents false "exited" in multi-pane sessions - Handle EPERM from process.kill(pid, 0) as "process exists" — EPERM means the process is alive but we lack permission to signal it - Use word-boundary regex for process name matching — prevents false positives on similar names like "claude-code" or paths containing the substring - Add tests for EPERM handling, multi-pane detection, and strict name matching (134 tests total) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
9c5d69a226
commit
adc0d22f56
|
|
@ -212,6 +212,31 @@ describe("isProcessRunning", () => {
|
|||
mockExecFileAsync.mockRejectedValue(new Error("tmux gone"));
|
||||
expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false);
|
||||
});
|
||||
|
||||
it("returns true when PID exists but throws EPERM", async () => {
|
||||
const epermErr = Object.assign(new Error("EPERM"), { code: "EPERM" });
|
||||
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => {
|
||||
throw epermErr;
|
||||
});
|
||||
expect(await agent.isProcessRunning(makeProcessHandle(789))).toBe(true);
|
||||
killSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("finds aider on any pane in multi-pane session", async () => {
|
||||
mockExecFileAsync.mockImplementation((cmd: string) => {
|
||||
if (cmd === "tmux") {
|
||||
return Promise.resolve({ stdout: "/dev/ttys001\n/dev/ttys002\n", stderr: "" });
|
||||
}
|
||||
if (cmd === "ps") {
|
||||
return Promise.resolve({
|
||||
stdout: " PID TT ARGS\n 100 ttys001 bash\n 200 ttys002 aider --yes\n",
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
return Promise.reject(new Error("unexpected"));
|
||||
});
|
||||
expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
|
|
|
|||
|
|
@ -88,23 +88,21 @@ function createAiderAgent(): Agent {
|
|||
"-F",
|
||||
"#{pane_tty}",
|
||||
]);
|
||||
const tty = ttyOut.trim().split("\n")[0];
|
||||
if (!tty) return false;
|
||||
const ttys = ttyOut
|
||||
.trim()
|
||||
.split("\n")
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean);
|
||||
if (ttys.length === 0) return false;
|
||||
|
||||
const ttyShort = tty.replace(/^\/dev\//, "");
|
||||
// Use `args` instead of `comm` so we match the CLI name even when
|
||||
// running via a wrapper (e.g. python, pipx).
|
||||
const { stdout: psOut } = await execFileAsync("ps", ["-eo", "pid,tty,args"]);
|
||||
const ttySet = new Set(ttys.map((t) => t.replace(/^\/dev\//, "")));
|
||||
const processRe = /(?:^|\/)aider(?:\s|$)/;
|
||||
for (const line of psOut.split("\n")) {
|
||||
const cols = line.trimStart().split(/\s+/);
|
||||
if (cols.length < 3 || cols[1] !== ttyShort) continue;
|
||||
if (cols.length < 3 || !ttySet.has(cols[1] ?? "")) continue;
|
||||
const args = cols.slice(2).join(" ");
|
||||
if (
|
||||
args === "aider" ||
|
||||
args.startsWith("aider ") ||
|
||||
args.includes("/aider ") ||
|
||||
args.includes("/aider")
|
||||
) {
|
||||
if (processRe.test(args)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -117,7 +115,10 @@ function createAiderAgent(): Agent {
|
|||
try {
|
||||
process.kill(pid, 0);
|
||||
return true;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof Error && "code" in err && err.code === "EPERM") {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -288,6 +288,47 @@ describe("isProcessRunning", () => {
|
|||
mockExecFileAsync.mockRejectedValue(new Error("fail"));
|
||||
expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false);
|
||||
});
|
||||
|
||||
it("returns true when PID exists but throws EPERM", async () => {
|
||||
const epermErr = Object.assign(new Error("EPERM"), { code: "EPERM" });
|
||||
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => {
|
||||
throw epermErr;
|
||||
});
|
||||
expect(await agent.isProcessRunning(makeProcessHandle(789))).toBe(true);
|
||||
killSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("finds claude on any pane in multi-pane session", async () => {
|
||||
mockExecFileAsync.mockImplementation((cmd: string, args: string[]) => {
|
||||
if (cmd === "tmux" && args[0] === "list-panes") {
|
||||
return Promise.resolve({ stdout: "/dev/ttys001\n/dev/ttys002\n", stderr: "" });
|
||||
}
|
||||
if (cmd === "ps") {
|
||||
return Promise.resolve({
|
||||
stdout: " PID TT ARGS\n 100 ttys001 bash\n 200 ttys002 claude -p test\n",
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
return Promise.reject(new Error("unexpected"));
|
||||
});
|
||||
expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(true);
|
||||
});
|
||||
|
||||
it("does not match similar process names like claude-code", async () => {
|
||||
mockExecFileAsync.mockImplementation((cmd: string, args: string[]) => {
|
||||
if (cmd === "tmux" && args[0] === "list-panes") {
|
||||
return Promise.resolve({ stdout: "/dev/ttys001\n", stderr: "" });
|
||||
}
|
||||
if (cmd === "ps") {
|
||||
return Promise.resolve({
|
||||
stdout: " PID TT ARGS\n 100 ttys001 /usr/bin/claude-code\n",
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
return Promise.reject(new Error("unexpected"));
|
||||
});
|
||||
expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
|
|
|
|||
|
|
@ -253,26 +253,27 @@ async function findClaudeProcess(handle: RuntimeHandle): Promise<number | null>
|
|||
"-F",
|
||||
"#{pane_tty}",
|
||||
]);
|
||||
const tty = ttyOut.trim().split("\n")[0];
|
||||
if (!tty) return null;
|
||||
// Iterate all pane TTYs (multi-pane sessions) — succeed on any match
|
||||
const ttys = ttyOut
|
||||
.trim()
|
||||
.split("\n")
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean);
|
||||
if (ttys.length === 0) return null;
|
||||
|
||||
const ttyShort = tty.replace(/^\/dev\//, "");
|
||||
// Use `args` instead of `comm` so we can match the CLI name even when
|
||||
// the process runs via a wrapper (e.g. node, python). `comm` would
|
||||
// report "node" instead of "claude" in those cases.
|
||||
const { stdout: psOut } = await execFileAsync("ps", ["-eo", "pid,tty,args"]);
|
||||
const ttySet = new Set(ttys.map((t) => t.replace(/^\/dev\//, "")));
|
||||
// Match "claude" as a word boundary — prevents false positives on
|
||||
// names like "claude-code" or paths that merely contain the substring.
|
||||
const processRe = /(?:^|\/)claude(?:\s|$)/;
|
||||
for (const line of psOut.split("\n")) {
|
||||
const cols = line.trimStart().split(/\s+/);
|
||||
if (cols.length < 3 || cols[1] !== ttyShort) continue;
|
||||
// cols[2..] is the full command + arguments — check if any token
|
||||
// ends with the target process name (handles /path/to/claude, npx claude, etc.)
|
||||
if (cols.length < 3 || !ttySet.has(cols[1] ?? "")) continue;
|
||||
const args = cols.slice(2).join(" ");
|
||||
if (
|
||||
args === "claude" ||
|
||||
args.startsWith("claude ") ||
|
||||
args.includes("/claude ") ||
|
||||
args.includes("/claude")
|
||||
) {
|
||||
if (processRe.test(args)) {
|
||||
return parseInt(cols[0] ?? "0", 10);
|
||||
}
|
||||
}
|
||||
|
|
@ -286,7 +287,11 @@ async function findClaudeProcess(handle: RuntimeHandle): Promise<number | null>
|
|||
try {
|
||||
process.kill(pid, 0); // Signal 0 = check existence
|
||||
return pid;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
// EPERM means the process exists but we lack permission to signal it
|
||||
if (err instanceof Error && "code" in err && err.code === "EPERM") {
|
||||
return pid;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -211,6 +211,31 @@ describe("isProcessRunning", () => {
|
|||
mockExecFileAsync.mockRejectedValue(new Error("tmux not running"));
|
||||
expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(false);
|
||||
});
|
||||
|
||||
it("returns true when PID exists but throws EPERM", async () => {
|
||||
const epermErr = Object.assign(new Error("EPERM"), { code: "EPERM" });
|
||||
const killSpy = vi.spyOn(process, "kill").mockImplementation(() => {
|
||||
throw epermErr;
|
||||
});
|
||||
expect(await agent.isProcessRunning(makeProcessHandle(789))).toBe(true);
|
||||
killSpy.mockRestore();
|
||||
});
|
||||
|
||||
it("finds codex on any pane in multi-pane session", async () => {
|
||||
mockExecFileAsync.mockImplementation((cmd: string) => {
|
||||
if (cmd === "tmux") {
|
||||
return Promise.resolve({ stdout: "/dev/ttys001\n/dev/ttys002\n", stderr: "" });
|
||||
}
|
||||
if (cmd === "ps") {
|
||||
return Promise.resolve({
|
||||
stdout: " PID TT ARGS\n 100 ttys001 bash\n 200 ttys002 codex --model o3\n",
|
||||
stderr: "",
|
||||
});
|
||||
}
|
||||
return Promise.reject(new Error("unexpected"));
|
||||
});
|
||||
expect(await agent.isProcessRunning(makeTmuxHandle())).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// =========================================================================
|
||||
|
|
|
|||
|
|
@ -92,23 +92,21 @@ function createCodexAgent(): Agent {
|
|||
"-F",
|
||||
"#{pane_tty}",
|
||||
]);
|
||||
const tty = ttyOut.trim().split("\n")[0];
|
||||
if (!tty) return false;
|
||||
const ttys = ttyOut
|
||||
.trim()
|
||||
.split("\n")
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean);
|
||||
if (ttys.length === 0) return false;
|
||||
|
||||
const ttyShort = tty.replace(/^\/dev\//, "");
|
||||
// Use `args` instead of `comm` so we match the CLI name even when
|
||||
// running via a wrapper (e.g. node, npx).
|
||||
const { stdout: psOut } = await execFileAsync("ps", ["-eo", "pid,tty,args"]);
|
||||
const ttySet = new Set(ttys.map((t) => t.replace(/^\/dev\//, "")));
|
||||
const processRe = /(?:^|\/)codex(?:\s|$)/;
|
||||
for (const line of psOut.split("\n")) {
|
||||
const cols = line.trimStart().split(/\s+/);
|
||||
if (cols.length < 3 || cols[1] !== ttyShort) continue;
|
||||
if (cols.length < 3 || !ttySet.has(cols[1] ?? "")) continue;
|
||||
const args = cols.slice(2).join(" ");
|
||||
if (
|
||||
args === "codex" ||
|
||||
args.startsWith("codex ") ||
|
||||
args.includes("/codex ") ||
|
||||
args.includes("/codex")
|
||||
) {
|
||||
if (processRe.test(args)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
|
@ -121,7 +119,10 @@ function createCodexAgent(): Agent {
|
|||
try {
|
||||
process.kill(pid, 0);
|
||||
return true;
|
||||
} catch {
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof Error && "code" in err && err.code === "EPERM") {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue