From 377d560effe8fbeaf464bcdf8f5892eab925475a Mon Sep 17 00:00:00 2001 From: liyuanbin Date: Wed, 24 Jun 2026 17:31:04 +0800 Subject: [PATCH] fix(tools-manager): require clean exit in commandExists commandExists only checked spawnSync's result.error, so a binary that spawns but exits non-zero (e.g. one broken by a GLIBC / shared-lib mismatch after a system upgrade) was misreported as available. getToolPath then returned the broken command name and ensureTool skipped its auto-install fallback, leaving the agent without a working tool and no auto-recovery. Aligns commandExists with the sibling runExtractionCommand, which already requires !result.error && result.status === 0, and adds a 5s spawn timeout so a hung PATH entry cannot block startup. Co-Authored-By: Claude --- src/agents/utils/tools-manager.test.ts | 27 ++++++++++++++++++++++++++ src/agents/utils/tools-manager.ts | 9 ++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/agents/utils/tools-manager.test.ts b/src/agents/utils/tools-manager.test.ts index aff20ed788bc..00fd5f66e2a3 100644 --- a/src/agents/utils/tools-manager.test.ts +++ b/src/agents/utils/tools-manager.test.ts @@ -155,3 +155,30 @@ describe("ensureTool", () => { ); }); }); + +describe("getToolPath exit-status handling", () => { + it("treats a binary that spawns but exits non-zero as missing", async () => { + const { getToolPath } = await import("./tools-manager.js"); + // execve succeeded (no result.error) but the child exited non-zero — the + // signature of an installed-but-broken binary (GLIBC / shared-lib mismatch). + // Must not be reported as available, or ensureTool skips its download path. + spawnSyncMock.mockReturnValue({ + error: undefined, + status: 1, + stderr: Buffer.alloc(0), + stdout: Buffer.alloc(0), + }); + expect(getToolPath("fd")).toBeNull(); + }); + + it("reports a binary present when it spawns and exits 0", async () => { + const { getToolPath } = await import("./tools-manager.js"); + spawnSyncMock.mockReturnValue({ + error: undefined, + status: 0, + stderr: Buffer.alloc(0), + stdout: Buffer.alloc(0), + }); + expect(getToolPath("fd")).toBe("fd"); + }); +}); diff --git a/src/agents/utils/tools-manager.ts b/src/agents/utils/tools-manager.ts index d079f5565918..3a764d615e13 100644 --- a/src/agents/utils/tools-manager.ts +++ b/src/agents/utils/tools-manager.ts @@ -101,9 +101,12 @@ const TOOLS: Record = { // Check if a command exists in PATH by trying to run it function commandExists(cmd: string): boolean { try { - const result = spawnSync(cmd, ["--version"], { stdio: "pipe" }); - // Check for ENOENT error (command not found) - return result.error === undefined || result.error === null; + const result = spawnSync(cmd, ["--version"], { stdio: "pipe", timeout: 5_000 }); + // Require a clean exit, not just a successful spawn. An installed-but-broken + // binary (e.g. GLIBC mismatch after a system upgrade, missing shared lib) + // spawns fine but exits non-zero; without the status check it would be + // misreported as available and block ensureTool's auto-install fallback. + return !result.error && result.status === 0; } catch { return false; }