fix: cleanup suspended CLI processes (#978) (thanks @Nachx639)

This commit is contained in:
Peter Steinberger
2026-01-16 01:37:50 +00:00
parent a70fcc8ae0
commit c51724f39b
4 changed files with 181 additions and 5 deletions

View File

@@ -20,6 +20,7 @@
- Auth/Status: keep auth profiles sticky per session (rotate on compaction/new), surface provider usage headers in `/status` and `clawdbot models status`, and update docs.
- Fix: guard model fallback against undefined provider/model values. (#954) — thanks @roshanasingh4.
- Fix: refactor session store updates, add chat.inject, and harden subagent cleanup flow. (#944) — thanks @tyler6204.
- Fix: clean up suspended CLI processes across backends. (#978) — thanks @Nachx639.
- Memory: make `node-llama-cpp` an optional dependency (avoid Node 25 install failures) and improve local-embeddings fallback/errors.
- Browser: add `snapshot refs=aria` (Playwright aria-ref ids) for self-resolving refs across `snapshot``act`.
- Browser: `profile="chrome"` now defaults to host control and returns clearer “attach a tab” errors.

View File

@@ -1,6 +1,8 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { CliBackendConfig } from "../config/types.js";
import { runCliAgent } from "./cli-runner.js";
import { cleanupSuspendedCliProcesses } from "./cli-runner/helpers.js";
const runCommandWithTimeoutMock = vi.fn();
const runExecMock = vi.fn();
@@ -17,7 +19,12 @@ describe("runCliAgent resume cleanup", () => {
});
it("kills stale resume processes for codex sessions", async () => {
runExecMock.mockResolvedValue({ stdout: "", stderr: "" });
runExecMock
.mockResolvedValueOnce({
stdout: " 1 S /bin/launchd\n",
stderr: "",
}) // cleanupSuspendedCliProcesses (ps)
.mockResolvedValueOnce({ stdout: "", stderr: "" }); // cleanupResumeProcesses (pkill)
runCommandWithTimeoutMock.mockResolvedValueOnce({
stdout: "ok",
stderr: "",
@@ -43,13 +50,103 @@ describe("runCliAgent resume cleanup", () => {
return;
}
expect(runExecMock).toHaveBeenCalledTimes(1);
const args = runExecMock.mock.calls[0] ?? [];
expect(args[0]).toBe("pkill");
const pkillArgs = args[1] as string[];
expect(runExecMock).toHaveBeenCalledTimes(2);
const pkillCall = runExecMock.mock.calls[1] ?? [];
expect(pkillCall[0]).toBe("pkill");
const pkillArgs = pkillCall[1] as string[];
expect(pkillArgs[0]).toBe("-f");
expect(pkillArgs[1]).toContain("codex");
expect(pkillArgs[1]).toContain("resume");
expect(pkillArgs[1]).toContain("thread-123");
});
});
describe("cleanupSuspendedCliProcesses", () => {
beforeEach(() => {
runExecMock.mockReset();
});
it("skips when no session tokens are configured", async () => {
await cleanupSuspendedCliProcesses(
{
command: "tool",
} as CliBackendConfig,
0,
);
if (process.platform === "win32") {
expect(runExecMock).not.toHaveBeenCalled();
return;
}
expect(runExecMock).not.toHaveBeenCalled();
});
it("matches sessionArg-based commands", async () => {
runExecMock
.mockResolvedValueOnce({
stdout: [
" 40 T+ claude --session-id thread-1 -p",
" 41 S claude --session-id thread-2 -p",
].join("\n"),
stderr: "",
})
.mockResolvedValueOnce({ stdout: "", stderr: "" });
await cleanupSuspendedCliProcesses(
{
command: "claude",
sessionArg: "--session-id",
} as CliBackendConfig,
0,
);
if (process.platform === "win32") {
expect(runExecMock).not.toHaveBeenCalled();
return;
}
expect(runExecMock).toHaveBeenCalledTimes(2);
const killCall = runExecMock.mock.calls[1] ?? [];
expect(killCall[0]).toBe("kill");
expect(killCall[1]).toEqual(["-9", "40"]);
});
it("matches resumeArgs with positional session id", async () => {
runExecMock
.mockResolvedValueOnce({
stdout: [
" 50 T codex exec resume thread-99 --color never --sandbox read-only",
" 51 T codex exec resume other --color never --sandbox read-only",
].join("\n"),
stderr: "",
})
.mockResolvedValueOnce({ stdout: "", stderr: "" });
await cleanupSuspendedCliProcesses(
{
command: "codex",
resumeArgs: [
"exec",
"resume",
"{sessionId}",
"--color",
"never",
"--sandbox",
"read-only",
],
} as CliBackendConfig,
1,
);
if (process.platform === "win32") {
expect(runExecMock).not.toHaveBeenCalled();
return;
}
expect(runExecMock).toHaveBeenCalledTimes(2);
const killCall = runExecMock.mock.calls[1] ?? [];
expect(killCall[0]).toBe("kill");
expect(killCall[1]).toEqual(["-9", "50", "51"]);
});
});

View File

@@ -13,6 +13,7 @@ import {
buildCliArgs,
buildSystemPrompt,
cleanupResumeProcesses,
cleanupSuspendedCliProcesses,
enqueueCliRun,
normalizeCliModel,
parseCliJson,
@@ -206,6 +207,7 @@ export async function runCliAgent(params: {
return next;
})();
await cleanupSuspendedCliProcesses(backend);
if (useResume && cliSessionIdToSend) {
await cleanupResumeProcesses(backend, cliSessionIdToSend);
}

View File

@@ -44,6 +44,82 @@ export async function cleanupResumeProcesses(
}
}
function buildSessionMatchers(backend: CliBackendConfig): RegExp[] {
const commandToken = path.basename(backend.command ?? "").trim();
if (!commandToken) return [];
const matchers: RegExp[] = [];
const sessionArg = backend.sessionArg?.trim();
const sessionArgs = backend.sessionArgs ?? [];
const resumeArgs = backend.resumeArgs ?? [];
const addMatcher = (args: string[]) => {
if (args.length === 0) return;
const tokens = [commandToken, ...args];
const pattern = tokens
.map((token, index) => {
const tokenPattern = tokenToRegex(token);
return index === 0 ? `(?:^|\\s)${tokenPattern}` : `\\s+${tokenPattern}`;
})
.join("");
matchers.push(new RegExp(pattern));
};
if (sessionArgs.some((arg) => arg.includes("{sessionId}"))) {
addMatcher(sessionArgs);
} else if (sessionArg) {
addMatcher([sessionArg, "{sessionId}"]);
}
if (resumeArgs.some((arg) => arg.includes("{sessionId}"))) {
addMatcher(resumeArgs);
}
return matchers;
}
function tokenToRegex(token: string): string {
if (!token.includes("{sessionId}")) return escapeRegex(token);
const parts = token.split("{sessionId}").map((part) => escapeRegex(part));
return parts.join("\\S+");
}
/**
* Cleanup suspended Clawdbot CLI processes that have accumulated.
* Only cleans up if there are more than the threshold (default: 10).
*/
export async function cleanupSuspendedCliProcesses(
backend: CliBackendConfig,
threshold = 10,
): Promise<void> {
if (process.platform === "win32") return;
const matchers = buildSessionMatchers(backend);
if (matchers.length === 0) return;
try {
const { stdout } = await runExec("ps", ["-ax", "-o", "pid=,stat=,command="]);
const suspended: number[] = [];
for (const line of stdout.split("\n")) {
const trimmed = line.trim();
if (!trimmed) continue;
const match = /^(\d+)\s+(\S+)\s+(.*)$/.exec(trimmed);
if (!match) continue;
const pid = Number(match[1]);
const stat = match[2] ?? "";
const command = match[3] ?? "";
if (!Number.isFinite(pid)) continue;
if (!stat.includes("T")) continue;
if (!matchers.some((matcher) => matcher.test(command))) continue;
suspended.push(pid);
}
if (suspended.length > threshold) {
// Verified locally: stopped (T) processes ignore SIGTERM, so use SIGKILL.
await runExec("kill", ["-9", ...suspended.map((pid) => String(pid))]);
}
} catch {
// ignore errors - best effort cleanup
}
}
export function enqueueCliRun<T>(key: string, task: () => Promise<T>): Promise<T> {
const prior = CLI_RUN_QUEUE.get(key) ?? Promise.resolve();
const chained = prior.catch(() => undefined).then(task);