DIS-002: shell:false + cross-spawn Windows-Fix im Runner #21
5 changed files with 1667 additions and 253 deletions
1657
package-lock.json
generated
1657
package-lock.json
generated
File diff suppressed because it is too large
Load diff
12
package.json
12
package.json
|
|
@ -6,18 +6,26 @@
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"build": "tsc",
|
"build": "tsc",
|
||||||
"start": "node dist/index.js",
|
"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": {
|
"dependencies": {
|
||||||
"better-sqlite3": "^11.0.0",
|
"better-sqlite3": "^11.0.0",
|
||||||
|
"cross-spawn": "^7.0.6",
|
||||||
"discord.js": "^14.16.0",
|
"discord.js": "^14.16.0",
|
||||||
"dotenv": "^16.4.0",
|
"dotenv": "^16.4.0",
|
||||||
"yaml": "^2.6.0"
|
"yaml": "^2.6.0"
|
||||||
},
|
},
|
||||||
"devDependencies": {
|
"devDependencies": {
|
||||||
"@types/better-sqlite3": "^7.6.0",
|
"@types/better-sqlite3": "^7.6.0",
|
||||||
|
"@types/cross-spawn": "^6.0.6",
|
||||||
"@types/node": "^22.0.0",
|
"@types/node": "^22.0.0",
|
||||||
"typescript": "^5.7.0"
|
"typescript": "^5.7.0",
|
||||||
|
"vitest": "^4.1.4"
|
||||||
},
|
},
|
||||||
"engines": {
|
"engines": {
|
||||||
"node": ">=20.0.0"
|
"node": ">=20.0.0"
|
||||||
|
|
|
||||||
|
|
@ -1,8 +1,9 @@
|
||||||
import { spawn } from "child_process";
|
import spawn from "cross-spawn";
|
||||||
import * as path from "path";
|
import * as path from "path";
|
||||||
import type { Conversation } from "../db/database";
|
import type { Conversation } from "../db/database";
|
||||||
import { loadAgentIdentity } from "./identity";
|
import { loadAgentIdentity } from "./identity";
|
||||||
import { loadDisclawConfig } from "../config/loader";
|
import { loadDisclawConfig } from "../config/loader";
|
||||||
|
import { resolveClaude } from "../runtime/resolve-claude";
|
||||||
|
|
||||||
export interface RunAgentOptions {
|
export interface RunAgentOptions {
|
||||||
workspacePath: string;
|
workspacePath: string;
|
||||||
|
|
@ -12,38 +13,20 @@ export interface RunAgentOptions {
|
||||||
abortController?: AbortController;
|
abortController?: AbortController;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface ClaudeSettings {
|
|
||||||
command: string;
|
|
||||||
timeoutMs: number;
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Resolves the Claude CLI command and timeout from configuration.
|
* Resolves the Claude CLI timeout from configuration.
|
||||||
* Priority: CLAUDE_PATH env var > disclaw.yaml claude_command > "claude" default.
|
|
||||||
*/
|
*/
|
||||||
function resolveClaudeSettings(): ClaudeSettings {
|
function resolveTimeoutMs(): number {
|
||||||
let command = "claude";
|
|
||||||
let timeoutMs = 120_000;
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const rootDir = path.resolve(__dirname, "../..");
|
const rootDir = path.resolve(__dirname, "../..");
|
||||||
const config = loadDisclawConfig(rootDir);
|
const config = loadDisclawConfig(rootDir);
|
||||||
if (config.claude_command) {
|
|
||||||
command = config.claude_command;
|
|
||||||
}
|
|
||||||
if (config.claude_timeout_seconds) {
|
if (config.claude_timeout_seconds) {
|
||||||
timeoutMs = config.claude_timeout_seconds * 1000;
|
return config.claude_timeout_seconds * 1000;
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
// Config not available, use defaults
|
// Config not available, use default
|
||||||
}
|
}
|
||||||
|
return 120_000;
|
||||||
// CLAUDE_PATH env var takes highest priority for the command
|
|
||||||
if (process.env.CLAUDE_PATH) {
|
|
||||||
command = process.env.CLAUDE_PATH;
|
|
||||||
}
|
|
||||||
|
|
||||||
return { command, timeoutMs };
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -164,9 +147,9 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
||||||
// Verify the workspace has an agent.yaml (validates it exists)
|
// Verify the workspace has an agent.yaml (validates it exists)
|
||||||
loadAgentIdentity(workspacePath);
|
loadAgentIdentity(workspacePath);
|
||||||
|
|
||||||
const settings = resolveClaudeSettings();
|
const claudeCommand = await resolveClaude();
|
||||||
const prompt = buildPrompt(userMessage, conversationHistory, channelName);
|
const prompt = buildPrompt(userMessage, conversationHistory, channelName);
|
||||||
const timeoutMs = settings.timeoutMs;
|
const timeoutMs = resolveTimeoutMs();
|
||||||
|
|
||||||
return new Promise<string>((resolve, reject) => {
|
return new Promise<string>((resolve, reject) => {
|
||||||
const args = [
|
const args = [
|
||||||
|
|
@ -176,7 +159,7 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
||||||
"json",
|
"json",
|
||||||
];
|
];
|
||||||
|
|
||||||
const child = spawn(settings.command, args, {
|
const child = spawn(claudeCommand, args, {
|
||||||
cwd: workspacePath,
|
cwd: workspacePath,
|
||||||
env: {
|
env: {
|
||||||
...process.env,
|
...process.env,
|
||||||
|
|
@ -184,7 +167,7 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
||||||
CI: "true",
|
CI: "true",
|
||||||
},
|
},
|
||||||
stdio: ["pipe", "pipe", "pipe"],
|
stdio: ["pipe", "pipe", "pipe"],
|
||||||
shell: true,
|
windowsHide: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
let stdout = "";
|
let stdout = "";
|
||||||
|
|
@ -192,14 +175,20 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
||||||
let killed = false;
|
let killed = false;
|
||||||
|
|
||||||
// Collect stdout
|
// Collect stdout
|
||||||
child.stdout.on("data", (data: Buffer) => {
|
if (child.stdout) {
|
||||||
stdout += data.toString("utf-8");
|
child.stdout.setEncoding("utf8");
|
||||||
});
|
child.stdout.on("data", (data: string) => {
|
||||||
|
stdout += data;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// Collect stderr
|
// Collect stderr
|
||||||
child.stderr.on("data", (data: Buffer) => {
|
if (child.stderr) {
|
||||||
stderr += data.toString("utf-8");
|
child.stderr.setEncoding("utf8");
|
||||||
});
|
child.stderr.on("data", (data: string) => {
|
||||||
|
stderr += data;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
// Timeout handling
|
// Timeout handling
|
||||||
const timer = setTimeout(() => {
|
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);
|
clearTimeout(timer);
|
||||||
if (err.message.includes("ENOENT")) {
|
if (err.code === "ENOENT") {
|
||||||
reject(
|
reject(
|
||||||
new Error(
|
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` +
|
`Install it with: npm install -g @anthropic-ai/claude-code\n` +
|
||||||
`Or set CLAUDE_PATH in your .env file.`
|
`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
|
// Close stdin immediately since we pass everything via args
|
||||||
child.stdin.end();
|
if (child.stdin) {
|
||||||
|
child.stdin.end();
|
||||||
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
|
||||||
66
src/runtime/resolve-claude.ts
Normal file
66
src/runtime/resolve-claude.ts
Normal 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;
|
||||||
|
}
|
||||||
118
tests/unit/resolve-claude.test.ts
Normal file
118
tests/unit/resolve-claude.test.ts
Normal 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();
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue