diff --git a/packages/memory-host-sdk/src/host/qmd-process.test.ts b/packages/memory-host-sdk/src/host/qmd-process.test.ts index 7ceaf03c8ac..8abf1324882 100644 --- a/packages/memory-host-sdk/src/host/qmd-process.test.ts +++ b/packages/memory-host-sdk/src/host/qmd-process.test.ts @@ -3,7 +3,17 @@ import { EventEmitter } from "node:events"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, + type MockInstance, +} from "vitest"; import { MAX_SAFE_TIMEOUT_DELAY_MS } from "../../../gateway-client/src/timeouts.js"; const spawnMock = vi.hoisted(() => vi.fn()); @@ -24,13 +34,15 @@ import { type QmdBinaryAvailability, } from "./qmd-process.js"; -function createMockChild() { +function createMockChild(params: { pid?: number } = {}) { const child = new EventEmitter() as EventEmitter & { + pid?: number; stdout: EventEmitter; stderr: EventEmitter; kill: ReturnType; closeWith: (code?: number | null, signal?: NodeJS.Signals | null) => void; }; + child.pid = params.pid; child.stdout = new EventEmitter(); child.stderr = new EventEmitter(); child.kill = vi.fn(); @@ -42,7 +54,7 @@ function createMockChild() { let fixtureRoot = ""; let tempDir = ""; -let platformSpy: { mockRestore(): void } | null = null; +let platformSpy: MockInstance<() => NodeJS.Platform> | null = null; let fixtureId = 0; const originalPath = process.env.PATH; const originalPathExt = process.env.PATHEXT; @@ -69,6 +81,7 @@ afterEach(() => { vi.useRealTimers(); process.env.PATH = originalPath; process.env.PATHEXT = originalPathExt; + platformSpy?.mockReturnValue("win32"); spawnMock.mockReset(); tempDir = ""; }); @@ -218,6 +231,34 @@ describe("checkQmdBinaryAvailability", () => { expect(timeoutSpy).toHaveBeenCalledWith(expect.any(Function), MAX_SAFE_TIMEOUT_DELAY_MS); }); + + it("kills timed-out availability probes by process group on POSIX", async () => { + platformSpy?.mockReturnValue("linux"); + const killProcess = vi.spyOn(process, "kill").mockImplementation(() => true); + const child = createMockChild({ pid: 4321 }); + spawnMock.mockReturnValueOnce(child); + + try { + await expect( + checkQmdBinaryAvailability({ + command: "qmd", + env: process.env, + cwd: tempDir, + timeoutMs: 1, + }), + ).resolves.toEqual({ + available: false, + reason: "binary", + error: "spawn qmd timed out after 1ms", + }); + + expect(spawnMock.mock.calls[0]?.[2]).toMatchObject({ detached: true }); + expect(killProcess).toHaveBeenCalledWith(-4321, "SIGKILL"); + expect(child.kill).not.toHaveBeenCalledWith("SIGKILL"); + } finally { + killProcess.mockRestore(); + } + }); }); describe("runCliCommand", () => { @@ -341,4 +382,33 @@ describe("runCliCommand", () => { expect(timeoutSpy).toHaveBeenCalledWith(expect.any(Function), MAX_SAFE_TIMEOUT_DELAY_MS); }); + + it("kills timed-out cli command process groups on POSIX", async () => { + platformSpy?.mockReturnValue("linux"); + const killProcess = vi.spyOn(process, "kill").mockImplementation(() => true); + const child = createMockChild({ pid: 8765 }); + spawnMock.mockReturnValueOnce(child); + + try { + const pending = runCliCommand({ + commandSummary: "qmd query test", + spawnInvocation: { command: "qmd", argv: ["query", "test", "--json"] }, + env: process.env, + cwd: tempDir, + maxOutputChars: 10_000, + timeoutMs: 1, + }); + const timeoutAssertion = expect(pending).rejects.toThrow( + "qmd query test timed out after 1ms", + ); + + await timeoutAssertion; + + expect(spawnMock.mock.calls[0]?.[2]).toMatchObject({ detached: true }); + expect(killProcess).toHaveBeenCalledWith(-8765, "SIGKILL"); + expect(child.kill).not.toHaveBeenCalledWith("SIGKILL"); + } finally { + killProcess.mockRestore(); + } + }); }); diff --git a/packages/memory-host-sdk/src/host/qmd-process.ts b/packages/memory-host-sdk/src/host/qmd-process.ts index d40bd96b17b..37a16589b18 100644 --- a/packages/memory-host-sdk/src/host/qmd-process.ts +++ b/packages/memory-host-sdk/src/host/qmd-process.ts @@ -11,6 +11,11 @@ export type CliSpawnInvocation = { windowsHide?: boolean; }; +type QmdChildProcess = { + pid?: number; + kill: (signal?: NodeJS.Signals) => boolean; +}; + export type QmdBinaryUnavailableReason = "binary" | "workspace-cwd"; export type QmdBinaryUnavailable = { @@ -92,10 +97,11 @@ export async function checkQmdBinaryAvailability(params: { shell: spawnInvocation.shell, windowsHide: spawnInvocation.windowsHide, stdio: "ignore", + detached: shouldUseQmdProcessGroup(), }); const timeoutMs = resolveSafeTimeoutDelayMs(params.timeoutMs ?? 2_000, { minMs: 0 }); const timer = setTimeout(() => { - child.kill("SIGKILL"); + signalQmdProcessTree(child, "SIGKILL"); finish({ available: false, reason: "binary", @@ -108,7 +114,7 @@ export async function checkQmdBinaryAvailability(params: { }); child.once("spawn", () => { didSpawn = true; - child.kill(); + signalQmdProcessTree(child); finish({ available: true }); }); child.once("close", () => { @@ -162,6 +168,7 @@ export async function runCliCommand(params: { cwd: params.cwd, shell: params.spawnInvocation.shell, windowsHide: params.spawnInvocation.windowsHide, + detached: shouldUseQmdProcessGroup(), }); let stdout = ""; let stderr = ""; @@ -172,7 +179,7 @@ export async function runCliCommand(params: { params.timeoutMs === undefined ? undefined : resolveSafeTimeoutDelayMs(params.timeoutMs); const timer = timeoutMs ? setTimeout(() => { - child.kill("SIGKILL"); + signalQmdProcessTree(child, "SIGKILL"); reject(new Error(`${params.commandSummary} timed out after ${timeoutMs}ms`)); }, timeoutMs) : null; @@ -224,6 +231,30 @@ export async function runCliCommand(params: { }); } +function shouldUseQmdProcessGroup(): boolean { + return process.platform !== "win32"; +} + +function signalQmdProcessTree(child: QmdChildProcess, signal?: NodeJS.Signals): void { + if (shouldUseQmdProcessGroup() && typeof child.pid === "number") { + try { + if (signal === undefined) { + process.kill(-child.pid); + } else { + process.kill(-child.pid, signal); + } + return; + } catch { + // Fall back to the direct child if the process group already disappeared. + } + } + if (signal === undefined) { + child.kill(); + } else { + child.kill(signal); + } +} + class CliCommandError extends Error { readonly code: number | null; readonly signal: NodeJS.Signals | null;