Replace killProcessTree references to shell-utils with process/kill-tree (#55213)

* Replace killProcessTree references to shell-utils with process/kill-tree

* Address grace timeout comment

* Align with existing process kill behavior

* bash: fail stop without pid

* bash: lazy-load kill tree on stop

---------

Co-authored-by: Jacob Tomlinson <jtomlinson@nvidia.com>
This commit is contained in:
mappel-nv
2026-03-27 07:25:56 -04:00
committed by GitHub
parent d80b67124b
commit 9d58f9e24f
4 changed files with 128 additions and 35 deletions

View File

@@ -1,11 +1,11 @@
import { afterEach, expect, test } from "vitest";
import { killProcessTree } from "../process/kill-tree.js";
import {
getFinishedSession,
getSession,
resetProcessRegistryForTests,
} from "./bash-process-registry.js";
import { createExecTool } from "./bash-tools.exec.js";
import { killProcessTree } from "./shell-utils.js";
const BACKGROUND_HOLD_CMD = 'node -e "setTimeout(() => {}, 5000)"';
const ABORT_SETTLE_MS = process.platform === "win32" ? 200 : 25;

View File

@@ -1,4 +1,3 @@
import { spawn } from "node:child_process";
import fs from "node:fs";
import path from "node:path";
@@ -166,27 +165,3 @@ export function sanitizeBinaryOutput(text: string): string {
}
return chunks.join("");
}
export function killProcessTree(pid: number): void {
if (process.platform === "win32") {
try {
spawn("taskkill", ["/F", "/T", "/PID", String(pid)], {
stdio: "ignore",
detached: true,
});
} catch {
// ignore errors if taskkill fails
}
return;
}
try {
process.kill(-pid, "SIGKILL");
} catch {
try {
process.kill(pid, "SIGKILL");
} catch {
// process already dead
}
}
}

View File

@@ -0,0 +1,119 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenClawConfig } from "../../config/config.js";
import type { MsgContext } from "../templating.js";
const { getSessionMock, getFinishedSessionMock, killProcessTreeMock } = vi.hoisted(() => ({
getSessionMock: vi.fn(),
getFinishedSessionMock: vi.fn(),
killProcessTreeMock: vi.fn(),
}));
vi.mock("../../agents/bash-process-registry.js", () => ({
getSession: getSessionMock,
getFinishedSession: getFinishedSessionMock,
markExited: vi.fn(),
}));
vi.mock("../../process/kill-tree.js", () => ({
killProcessTree: killProcessTreeMock,
}));
const { handleBashChatCommand } = await import("./bash-command.js");
function buildParams(commandBody: string) {
const cfg = {
commands: { bash: true },
} as OpenClawConfig;
const ctx = {
CommandBody: commandBody,
SessionKey: "session-key",
} as MsgContext;
return {
ctx,
cfg,
sessionKey: "session-key",
isGroup: false,
elevated: {
enabled: true,
allowed: true,
failures: [],
},
};
}
function buildRunningSession(overrides?: Record<string, unknown>) {
return {
id: "session-1",
scopeKey: "chat:bash",
backgrounded: true,
pid: 4242,
exited: false,
startedAt: Date.now(),
tail: "",
...overrides,
};
}
describe("handleBashChatCommand stop", () => {
beforeEach(() => {
getSessionMock.mockReset();
getFinishedSessionMock.mockReset();
killProcessTreeMock.mockReset();
});
it("returns immediately with a stopping message and fires killProcessTree", async () => {
const session = buildRunningSession();
getSessionMock.mockReturnValue(session);
getFinishedSessionMock.mockReturnValue(undefined);
const result = await handleBashChatCommand(buildParams("/bash stop session-1"));
expect(result.text).toContain("bash stopping");
expect(result.text).toContain("!poll session-1");
expect(killProcessTreeMock).toHaveBeenCalledWith(4242);
});
it("includes the full session ID so the user can poll after starting a new job", async () => {
const session = buildRunningSession({ id: "deep-forest-42" });
getSessionMock.mockReturnValue(session);
getFinishedSessionMock.mockReturnValue(undefined);
const result = await handleBashChatCommand(buildParams("/bash stop deep-forest-42"));
expect(result.text).toContain("!poll deep-forest-42");
});
it("does not call markExited synchronously (defers to supervisor lifecycle)", async () => {
const session = buildRunningSession();
getSessionMock.mockReturnValue(session);
getFinishedSessionMock.mockReturnValue(undefined);
await handleBashChatCommand(buildParams("/bash stop session-1"));
expect(session.exited).toBe(false);
});
it("returns no-running-job when session is not found", async () => {
getSessionMock.mockReturnValue(undefined);
getFinishedSessionMock.mockReturnValue(undefined);
const result = await handleBashChatCommand(buildParams("/bash stop session-1"));
expect(result.text).toContain("No running bash job found");
expect(killProcessTreeMock).not.toHaveBeenCalled();
});
it("fails stop when session has no pid", async () => {
const session = buildRunningSession({ pid: undefined, child: undefined });
getSessionMock.mockReturnValue(session);
getFinishedSessionMock.mockReturnValue(undefined);
const result = await handleBashChatCommand(buildParams("/bash stop session-1"));
expect(result.text).toContain("Unable to stop bash session");
expect(result.text).toContain("!poll session-1");
expect(killProcessTreeMock).not.toHaveBeenCalled();
});
});

View File

@@ -1,8 +1,7 @@
import { resolveSessionAgentId } from "../../agents/agent-scope.js";
import { getFinishedSession, getSession, markExited } from "../../agents/bash-process-registry.js";
import { getFinishedSession, getSession } from "../../agents/bash-process-registry.js";
import { createExecTool } from "../../agents/bash-tools.js";
import { resolveSandboxRuntimeStatus } from "../../agents/sandbox.js";
import { killProcessTree } from "../../agents/shell-utils.js";
import { isCommandFlagEnabled } from "../../config/commands.js";
import type { OpenClawConfig } from "../../config/config.js";
import { logVerbose } from "../../globals.js";
@@ -298,15 +297,15 @@ export async function handleBashChatCommand(params: {
};
}
const pid = running.pid ?? running.child?.pid;
if (pid) {
killProcessTree(pid);
}
markExited(running, null, "SIGKILL", "failed");
if (activeJob?.state === "running" && activeJob.sessionId === sessionId) {
activeJob = null;
if (!pid) {
return {
text: `⚠️ Unable to stop bash session ${formatSessionSnippet(sessionId)} because no process ID is available. Use !poll ${sessionId} to check whether it exits on its own.`,
};
}
const { killProcessTree } = await import("../../process/kill-tree.js");
killProcessTree(pid);
return {
text: `⚙️ bash stopped (session ${formatSessionSnippet(sessionId)}).`,
text: `⚙️ bash stopping (session ${formatSessionSnippet(sessionId)}). Use !poll ${sessionId} to confirm exit.`,
};
}