DIS-002: shell:false + cross-spawn Windows-Fix im Runner #21

Merged
dev merged 1 commit from phase-0/harden-spawn into main 2026-04-09 08:34:43 +00:00
5 changed files with 1667 additions and 253 deletions

1657
package-lock.json generated

File diff suppressed because it is too large Load diff

View file

@ -6,18 +6,26 @@
"scripts": {
"build": "tsc",
"start": "node dist/index.js",
"dev": "tsc && node dist/index.js"
"dev": "tsc && node dist/index.js",
"test": "vitest run"
},
"vitest": {
"include": ["tests/**/*.test.ts"],
"environment": "node"
},
"dependencies": {
"better-sqlite3": "^11.0.0",
"cross-spawn": "^7.0.6",
"discord.js": "^14.16.0",
"dotenv": "^16.4.0",
"yaml": "^2.6.0"
},
"devDependencies": {
"@types/better-sqlite3": "^7.6.0",
"@types/cross-spawn": "^6.0.6",
"@types/node": "^22.0.0",
"typescript": "^5.7.0"
"typescript": "^5.7.0",
"vitest": "^4.1.4"
},
"engines": {
"node": ">=20.0.0"

View file

@ -1,8 +1,9 @@
import { spawn } from "child_process";
import spawn from "cross-spawn";
import * as path from "path";
import type { Conversation } from "../db/database";
import { loadAgentIdentity } from "./identity";
import { loadDisclawConfig } from "../config/loader";
import { resolveClaude } from "../runtime/resolve-claude";
export interface RunAgentOptions {
workspacePath: string;
@ -12,38 +13,20 @@ export interface RunAgentOptions {
abortController?: AbortController;
}
interface ClaudeSettings {
command: string;
timeoutMs: number;
}
/**
* Resolves the Claude CLI command and timeout from configuration.
* Priority: CLAUDE_PATH env var > disclaw.yaml claude_command > "claude" default.
* Resolves the Claude CLI timeout from configuration.
*/
function resolveClaudeSettings(): ClaudeSettings {
let command = "claude";
let timeoutMs = 120_000;
function resolveTimeoutMs(): number {
try {
const rootDir = path.resolve(__dirname, "../..");
const config = loadDisclawConfig(rootDir);
if (config.claude_command) {
command = config.claude_command;
}
if (config.claude_timeout_seconds) {
timeoutMs = config.claude_timeout_seconds * 1000;
return config.claude_timeout_seconds * 1000;
}
} catch {
// Config not available, use defaults
// Config not available, use default
}
// CLAUDE_PATH env var takes highest priority for the command
if (process.env.CLAUDE_PATH) {
command = process.env.CLAUDE_PATH;
}
return { command, timeoutMs };
return 120_000;
}
/**
@ -164,9 +147,9 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
// Verify the workspace has an agent.yaml (validates it exists)
loadAgentIdentity(workspacePath);
const settings = resolveClaudeSettings();
const claudeCommand = await resolveClaude();
const prompt = buildPrompt(userMessage, conversationHistory, channelName);
const timeoutMs = settings.timeoutMs;
const timeoutMs = resolveTimeoutMs();
return new Promise<string>((resolve, reject) => {
const args = [
@ -176,7 +159,7 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
"json",
];
const child = spawn(settings.command, args, {
const child = spawn(claudeCommand, args, {
cwd: workspacePath,
env: {
...process.env,
@ -184,7 +167,7 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
CI: "true",
},
stdio: ["pipe", "pipe", "pipe"],
shell: true,
windowsHide: true,
});
let stdout = "";
@ -192,14 +175,20 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
let killed = false;
// Collect stdout
child.stdout.on("data", (data: Buffer) => {
stdout += data.toString("utf-8");
if (child.stdout) {
child.stdout.setEncoding("utf8");
child.stdout.on("data", (data: string) => {
stdout += data;
});
}
// Collect stderr
child.stderr.on("data", (data: Buffer) => {
stderr += data.toString("utf-8");
if (child.stderr) {
child.stderr.setEncoding("utf8");
child.stderr.on("data", (data: string) => {
stderr += data;
});
}
// Timeout handling
const timer = setTimeout(() => {
@ -223,12 +212,12 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
});
}
child.on("error", (err) => {
child.on("error", (err: NodeJS.ErrnoException) => {
clearTimeout(timer);
if (err.message.includes("ENOENT")) {
if (err.code === "ENOENT") {
reject(
new Error(
`Claude Code CLI not found at "${settings.command}". ` +
`Claude Code CLI not found at "${claudeCommand}". ` +
`Install it with: npm install -g @anthropic-ai/claude-code\n` +
`Or set CLAUDE_PATH in your .env file.`
)
@ -257,6 +246,8 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
});
// Close stdin immediately since we pass everything via args
if (child.stdin) {
child.stdin.end();
}
});
}

View file

@ -0,0 +1,66 @@
import { execFile } from "child_process";
import { promisify } from "util";
const execFileAsync = promisify(execFile);
/** Cached result after first successful resolution. */
let cachedPath: string | null = null;
/**
* Resolves the Claude CLI executable name or path.
*
* Resolution order:
* 1. `CLAUDE_PATH` environment variable (absolute path or command name)
* 2. PATH lookup via `where` (Windows) or `which` (Unix)
*
* The result is cached in-memory after the first successful call so
* subsequent invocations are synchronous-cheap.
*
* On Windows, `claude` is installed as `claude.cmd`. `cross-spawn` handles
* `.cmd` wrappers transparently, so returning the bare name `"claude"` is
* correct cross-spawn will find and invoke `claude.cmd` automatically.
*
* @throws {Error} with a clear installation hint if claude cannot be found.
*/
export async function resolveClaude(): Promise<string> {
if (cachedPath !== null) {
return cachedPath;
}
// 1. Explicit override via environment variable
const envPath = process.env.CLAUDE_PATH;
if (envPath && envPath.trim() !== "") {
cachedPath = envPath.trim();
return cachedPath;
}
// 2. PATH lookup
const isWindows = process.platform === "win32";
const lookupCmd = isWindows ? "where" : "which";
try {
const { stdout } = await execFileAsync(lookupCmd, ["claude"], {
timeout: 5_000,
});
const firstLine = stdout.trim().split(/\r?\n/)[0].trim();
if (firstLine) {
cachedPath = firstLine;
return cachedPath;
}
} catch {
// where/which returned non-zero or was not found — fall through to error
}
throw new Error(
`Claude Code CLI not found. Install it with:\n` +
` npm install -g @anthropic-ai/claude-code\n` +
`Or set CLAUDE_PATH in your .env file to the full path of the claude executable.`
);
}
/**
* Resets the in-memory cache. Intended for use in tests only.
*/
export function _resetResolveClaudeCache(): void {
cachedPath = null;
}

View file

@ -0,0 +1,118 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
// We mock child_process so execFile never actually runs
vi.mock("child_process", async (importOriginal) => {
const actual = await importOriginal<typeof import("child_process")>();
return {
...actual,
execFile: vi.fn(),
};
});
// Import after mock is registered so the module picks up the mock
import { execFile } from "child_process";
import { resolveClaude, _resetResolveClaudeCache } from "../../src/runtime/resolve-claude";
// util.promisify wraps the callback form; we need to make execFile call its callback
const mockedExecFile = execFile as unknown as ReturnType<typeof vi.fn>;
/** Helper: make execFile invoke its last argument (callback) with success */
function mockExecFileSuccess(stdout: string): void {
mockedExecFile.mockImplementation(
(
_cmd: string,
_args: string[],
_opts: object,
callback: (err: null, result: { stdout: string; stderr: string }) => void
) => {
callback(null, { stdout, stderr: "" });
}
);
}
/** Helper: make execFile invoke its callback with an error */
function mockExecFileFailure(message: string): void {
mockedExecFile.mockImplementation(
(
_cmd: string,
_args: string[],
_opts: object,
callback: (err: Error) => void
) => {
callback(new Error(message));
}
);
}
describe("resolveClaude", () => {
const originalEnv = process.env;
beforeEach(() => {
// Reset module-level cache before each test
_resetResolveClaudeCache();
// Clone env so mutations don't bleed between tests
process.env = { ...originalEnv };
vi.clearAllMocks();
});
afterEach(() => {
process.env = originalEnv;
});
it("(a) returns CLAUDE_PATH directly when env var is set", async () => {
process.env.CLAUDE_PATH = "/usr/local/bin/claude";
const result = await resolveClaude();
expect(result).toBe("/usr/local/bin/claude");
// Should NOT have called execFile at all
expect(mockedExecFile).not.toHaveBeenCalled();
});
it("(b) falls back to PATH lookup when CLAUDE_PATH is not set", async () => {
delete process.env.CLAUDE_PATH;
mockExecFileSuccess("/home/user/.nvm/bin/claude\n");
const result = await resolveClaude();
expect(result).toBe("/home/user/.nvm/bin/claude");
expect(mockedExecFile).toHaveBeenCalledOnce();
});
it("(b) trims whitespace and picks first line from PATH lookup output", async () => {
delete process.env.CLAUDE_PATH;
// `where` on Windows can return multiple matches, one per line
mockExecFileSuccess(
"C:\\Users\\dev\\AppData\\Roaming\\npm\\claude.cmd\r\nC:\\Program Files\\claude\\claude.cmd\r\n"
);
const result = await resolveClaude();
expect(result).toBe("C:\\Users\\dev\\AppData\\Roaming\\npm\\claude.cmd");
});
it("(c) throws a clear error when neither CLAUDE_PATH nor PATH lookup finds claude", async () => {
delete process.env.CLAUDE_PATH;
mockExecFileFailure("not found");
await expect(resolveClaude()).rejects.toThrow(
/Claude Code CLI not found/
);
await expect(resolveClaude()).rejects.toThrow(
/npm install -g @anthropic-ai\/claude-code/
);
});
it("caches the result after first successful resolution", async () => {
delete process.env.CLAUDE_PATH;
mockExecFileSuccess("/usr/bin/claude");
const first = await resolveClaude();
const second = await resolveClaude();
expect(first).toBe("/usr/bin/claude");
expect(second).toBe("/usr/bin/claude");
// execFile should only have been called once due to caching
expect(mockedExecFile).toHaveBeenCalledOnce();
});
});