feat(DIS-106): RunResult discriminated union replaces Promise<string> #51
4 changed files with 413 additions and 61 deletions
|
|
@ -6,6 +6,7 @@ import { loadDisclawConfig } from "../config/loader";
|
|||
import { resolveClaude } from "../runtime/resolve-claude";
|
||||
import { sanitizedEnv } from "../runtime/env";
|
||||
import { Semaphore } from "../runtime/concurrency";
|
||||
import type { RunResult } from "./types";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Module-level semaphore — lazy-initialised on first runAgent() call so the
|
||||
|
|
@ -167,7 +168,7 @@ function parseClaudeJsonOutput(raw: string): string {
|
|||
* Timeout defaults to 120 seconds. The process is killed if it exceeds
|
||||
* the timeout.
|
||||
*/
|
||||
export async function runAgent(options: RunAgentOptions): Promise<string> {
|
||||
export async function runAgent(options: RunAgentOptions): Promise<RunResult> {
|
||||
const {
|
||||
workspacePath,
|
||||
channelName,
|
||||
|
|
@ -198,7 +199,7 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
|||
await semaphore.acquire();
|
||||
|
||||
try {
|
||||
return await new Promise<string>((resolve, reject) => {
|
||||
return await new Promise<RunResult>((resolve) => {
|
||||
// NOTE: --bare is intentionally NOT used here. --bare disables OAuth/keychain
|
||||
// auth and requires ANTHROPIC_API_KEY, which breaks Claude Pro/Max subscriptions.
|
||||
// CLAUDE.md isolation is handled structurally: workspaces live under
|
||||
|
|
@ -242,7 +243,8 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
|||
});
|
||||
}
|
||||
|
||||
// Timeout handling
|
||||
// Timeout handling — kill the process; the "close" handler will resolve
|
||||
// with { status: "timeout" } once the process exits (killed flag is set).
|
||||
const timer = setTimeout(() => {
|
||||
killed = true;
|
||||
child.kill("SIGTERM");
|
||||
|
|
@ -267,15 +269,20 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
|||
child.on("error", (err: NodeJS.ErrnoException) => {
|
||||
clearTimeout(timer);
|
||||
if (err.code === "ENOENT") {
|
||||
reject(
|
||||
new Error(
|
||||
resolve({
|
||||
status: "cli-error",
|
||||
message:
|
||||
`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.`
|
||||
)
|
||||
);
|
||||
`Or set CLAUDE_PATH in your .env file.`,
|
||||
stderr: undefined,
|
||||
});
|
||||
} else {
|
||||
reject(new Error(`Failed to start Claude Code CLI: ${err.message}`));
|
||||
resolve({
|
||||
status: "cli-error",
|
||||
message: `Failed to start Claude Code CLI: ${err.message}`,
|
||||
stderr: undefined,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -283,18 +290,22 @@ export async function runAgent(options: RunAgentOptions): Promise<string> {
|
|||
clearTimeout(timer);
|
||||
|
||||
if (killed) {
|
||||
reject(new Error(`Claude Code CLI timed out after ${timeoutMs / 1000} seconds.`));
|
||||
resolve({ status: "timeout", timeoutMs });
|
||||
return;
|
||||
}
|
||||
|
||||
if (code !== 0) {
|
||||
const errorDetail = stderr.trim() || stdout.trim() || `exit code ${code}`;
|
||||
reject(new Error(`Claude Code CLI error: ${errorDetail}`));
|
||||
resolve({
|
||||
status: "cli-error",
|
||||
message: `Claude Code CLI error: ${errorDetail}`,
|
||||
stderr: stderr.trim() || undefined,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const response = parseClaudeJsonOutput(stdout);
|
||||
resolve(response);
|
||||
const text = parseClaudeJsonOutput(stdout);
|
||||
resolve({ status: "success", text });
|
||||
});
|
||||
|
||||
// Close stdin immediately since we pass everything via args
|
||||
|
|
|
|||
5
src/agent/types.ts
Normal file
5
src/agent/types.ts
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
export type RunResult =
|
||||
| { status: "success"; text: string; sessionId?: string }
|
||||
| { status: "timeout"; timeoutMs: number }
|
||||
| { status: "cli-error"; message: string; stderr?: string }
|
||||
| { status: "parse-error"; message: string; raw: string };
|
||||
|
|
@ -6,6 +6,7 @@ import { runAgent } from "./agent/runner";
|
|||
import { splitForDiscord } from "./discord/split";
|
||||
import { sanitizeForDiscord } from "./runtime/sanitize";
|
||||
import { ChannelQueue } from "./runtime/channel-queue";
|
||||
import type { RunResult } from "./agent/types";
|
||||
|
||||
const channelQueue = new ChannelQueue();
|
||||
|
||||
|
|
@ -22,86 +23,84 @@ export async function routeMessage(
|
|||
): Promise<boolean> {
|
||||
const channelId = message.channelId;
|
||||
|
||||
// Ignore messages in the management channel (commands only)
|
||||
if (channelId === managementChannelId) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Look up workspace for this channel
|
||||
const workspace = db.getWorkspaceByChannelId(channelId);
|
||||
if (!workspace) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const channel = message.channel as TextChannel;
|
||||
const sanitizeOpts = {
|
||||
repoRoot: workspace.workspace_path,
|
||||
home: os.homedir(),
|
||||
};
|
||||
|
||||
await channelQueue.enqueue(channelId, async () => {
|
||||
// Show typing indicator while the agent works
|
||||
const typingInterval = setInterval(() => {
|
||||
channel.sendTyping().catch(() => {});
|
||||
}, 5000);
|
||||
// Send immediately as well
|
||||
await channel.sendTyping().catch(() => {});
|
||||
|
||||
try {
|
||||
// Load recent conversation history for context
|
||||
const history = db.getConversationHistory(workspace.id, 30);
|
||||
|
||||
// Run the agent
|
||||
const response = await runAgent({
|
||||
const result: RunResult = await runAgent({
|
||||
workspacePath: workspace.workspace_path,
|
||||
channelName: channel.name,
|
||||
userMessage: message.content,
|
||||
conversationHistory: history,
|
||||
});
|
||||
|
||||
// Store user message
|
||||
db.addConversation(
|
||||
workspace.id,
|
||||
message.id,
|
||||
"user",
|
||||
message.content,
|
||||
message.author.username
|
||||
);
|
||||
|
||||
// Sanitize the full response once, then use the sanitized version
|
||||
// for both Discord output and conversation history storage.
|
||||
// This prevents a feedback loop where raw paths in stored history
|
||||
// get fed back to the agent as context, causing it to reproduce them.
|
||||
const sanitizeOpts = {
|
||||
repoRoot: workspace.workspace_path,
|
||||
home: os.homedir(),
|
||||
};
|
||||
const sanitizedResponse = sanitizeForDiscord(response, sanitizeOpts);
|
||||
|
||||
// Split and send response
|
||||
const chunks = splitForDiscord(sanitizedResponse);
|
||||
let firstMsgId: string | null = null;
|
||||
|
||||
for (const chunk of chunks) {
|
||||
const sent = await channel.send(chunk);
|
||||
if (!firstMsgId) {
|
||||
firstMsgId = sent.id;
|
||||
}
|
||||
}
|
||||
|
||||
// Store sanitized assistant response (use first message id for dedup)
|
||||
if (firstMsgId) {
|
||||
if (result.status === "success") {
|
||||
db.addConversation(
|
||||
workspace.id,
|
||||
firstMsgId,
|
||||
"assistant",
|
||||
sanitizedResponse,
|
||||
null
|
||||
message.id,
|
||||
"user",
|
||||
message.content,
|
||||
message.author.username
|
||||
);
|
||||
|
||||
const sanitizedResponse = sanitizeForDiscord(result.text, sanitizeOpts);
|
||||
const chunks = splitForDiscord(sanitizedResponse);
|
||||
let firstMsgId: string | null = null;
|
||||
|
||||
for (const chunk of chunks) {
|
||||
const sent = await channel.send(chunk);
|
||||
if (!firstMsgId) {
|
||||
firstMsgId = sent.id;
|
||||
}
|
||||
}
|
||||
|
||||
if (firstMsgId) {
|
||||
db.addConversation(
|
||||
workspace.id,
|
||||
firstMsgId,
|
||||
"assistant",
|
||||
sanitizedResponse,
|
||||
null
|
||||
);
|
||||
}
|
||||
} else if (result.status === "timeout") {
|
||||
await channel
|
||||
.send(`⏱️ Agent-Timeout nach ${result.timeoutMs / 1000}s`)
|
||||
.catch(() => {});
|
||||
} else if (result.status === "cli-error") {
|
||||
await channel
|
||||
.send(sanitizeForDiscord(`❌ CLI-Fehler: ${result.message}`, sanitizeOpts))
|
||||
.catch(() => {});
|
||||
} else if (result.status === "parse-error") {
|
||||
await channel
|
||||
.send(`⚠️ Antwort konnte nicht gelesen werden`)
|
||||
.catch(() => {});
|
||||
}
|
||||
} catch (error) {
|
||||
const msg = error instanceof Error ? error.message : "Unbekannter Fehler";
|
||||
const safeMsg = sanitizeForDiscord(`Fehler bei der Verarbeitung: ${msg}`, {
|
||||
repoRoot: workspace.workspace_path,
|
||||
home: os.homedir(),
|
||||
});
|
||||
await channel.send(safeMsg).catch(() => {});
|
||||
await channel
|
||||
.send(sanitizeForDiscord(`Fehler bei der Verarbeitung: ${msg}`, sanitizeOpts))
|
||||
.catch(() => {});
|
||||
} finally {
|
||||
clearInterval(typingInterval);
|
||||
}
|
||||
|
|
|
|||
337
tests/unit/runner-result.test.ts
Normal file
337
tests/unit/runner-result.test.ts
Normal file
|
|
@ -0,0 +1,337 @@
|
|||
import {
|
||||
describe,
|
||||
it,
|
||||
expect,
|
||||
vi,
|
||||
beforeAll,
|
||||
afterAll,
|
||||
beforeEach,
|
||||
afterEach,
|
||||
} from "vitest";
|
||||
import * as fs from "fs";
|
||||
import * as os from "os";
|
||||
import * as path from "path";
|
||||
|
||||
/**
|
||||
* Unit tests for the RunResult discriminated union returned by runAgent().
|
||||
*
|
||||
* cross-spawn is mocked so no real process is launched. Each test controls
|
||||
* what the fake child process emits (stdout, exit code, error event) to cover
|
||||
* all four RunResult status variants.
|
||||
*/
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Configurable mock state — mutated per test via setMockBehavior()
|
||||
// ---------------------------------------------------------------------------
|
||||
interface MockBehavior {
|
||||
/** stdout data to emit (default: valid JSON success payload) */
|
||||
stdoutData?: string;
|
||||
/** exit code the child closes with (default: 0) */
|
||||
exitCode?: number;
|
||||
/** if set, child emits an "error" event with this error object */
|
||||
spawnError?: NodeJS.ErrnoException;
|
||||
/** if true, close event is never emitted (simulates a hanging process) */
|
||||
neverClose?: boolean;
|
||||
/** delay in ms before emitting close (default: 0) */
|
||||
closeDelayMs?: number;
|
||||
}
|
||||
|
||||
let mockBehavior: MockBehavior = {};
|
||||
|
||||
function setMockBehavior(b: MockBehavior) {
|
||||
mockBehavior = b;
|
||||
}
|
||||
|
||||
vi.mock("cross-spawn", () => {
|
||||
function makeMockChild() {
|
||||
const listeners: Record<string, Array<(...a: unknown[]) => void>> = {};
|
||||
|
||||
const emit = (event: string, ...args: unknown[]) => {
|
||||
for (const cb of listeners[event] ?? []) {
|
||||
cb(...args);
|
||||
}
|
||||
};
|
||||
|
||||
const stdoutListeners: Record<string, Array<(...a: unknown[]) => void>> =
|
||||
{};
|
||||
|
||||
const stdoutStream = {
|
||||
setEncoding: () => {},
|
||||
on: (event: string, cb: (...a: unknown[]) => void) => {
|
||||
if (!stdoutListeners[event]) stdoutListeners[event] = [];
|
||||
stdoutListeners[event].push(cb);
|
||||
|
||||
if (event === "data" && !mockBehavior.spawnError) {
|
||||
const data =
|
||||
mockBehavior.stdoutData ??
|
||||
JSON.stringify({ result: "hello from agent" });
|
||||
Promise.resolve().then(() => {
|
||||
for (const handler of stdoutListeners["data"] ?? []) {
|
||||
handler(data);
|
||||
}
|
||||
});
|
||||
}
|
||||
},
|
||||
};
|
||||
|
||||
const child = {
|
||||
stdout: stdoutStream,
|
||||
stderr: {
|
||||
setEncoding: () => {},
|
||||
on: () => {},
|
||||
},
|
||||
stdin: { end: () => {} },
|
||||
on: (event: string, cb: (...a: unknown[]) => void) => {
|
||||
if (!listeners[event]) listeners[event] = [];
|
||||
listeners[event].push(cb);
|
||||
|
||||
if (event === "error" && mockBehavior.spawnError) {
|
||||
Promise.resolve().then(() => emit("error", mockBehavior.spawnError));
|
||||
return;
|
||||
}
|
||||
|
||||
if (event === "close" && !mockBehavior.neverClose) {
|
||||
// Emit close after stdout data (two microtask ticks away)
|
||||
const delay = mockBehavior.closeDelayMs ?? 0;
|
||||
const schedule = delay > 0
|
||||
? (fn: () => void) => setTimeout(fn, delay)
|
||||
: (fn: () => void) =>
|
||||
Promise.resolve()
|
||||
.then(() => Promise.resolve())
|
||||
.then(fn);
|
||||
|
||||
schedule(() => emit("close", mockBehavior.exitCode ?? 0));
|
||||
}
|
||||
},
|
||||
kill: () => {},
|
||||
};
|
||||
|
||||
return child;
|
||||
}
|
||||
|
||||
return {
|
||||
default: (_cmd: string, _args: string[]) => makeMockChild(),
|
||||
};
|
||||
});
|
||||
|
||||
// Import after mock registration
|
||||
import { runAgent, _resetSemaphore } from "../../src/agent/runner";
|
||||
import { _resetResolveClaudeCache } from "../../src/runtime/resolve-claude";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Shared workspace fixture
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe("RunResult discriminated union", () => {
|
||||
let workspaceDir: string;
|
||||
const originalEnv = { ...process.env };
|
||||
|
||||
beforeAll(() => {
|
||||
workspaceDir = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), "disclaw-runresult-")
|
||||
);
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(workspaceDir, "agent.yaml"),
|
||||
`name: result-agent\ndisplay_name: Result Agent\nrole: Testing\nchannel_id: "999"\n`,
|
||||
"utf-8"
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(workspaceDir, "CLAUDE.md"),
|
||||
"# Result Agent\nYou are a test agent.\n",
|
||||
"utf-8"
|
||||
);
|
||||
});
|
||||
|
||||
beforeEach(() => {
|
||||
mockBehavior = {};
|
||||
_resetResolveClaudeCache();
|
||||
_resetSemaphore(4);
|
||||
process.env.CLAUDE_PATH = "/fake/claude";
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env = { ...originalEnv };
|
||||
_resetResolveClaudeCache();
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
try {
|
||||
fs.rmSync(workspaceDir, { recursive: true, force: true });
|
||||
} catch {
|
||||
// Best-effort cleanup
|
||||
}
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// (a) success
|
||||
// -------------------------------------------------------------------------
|
||||
it('(a) success: valid JSON stdout → status "success" with correct text', async () => {
|
||||
setMockBehavior({
|
||||
stdoutData: JSON.stringify({ result: "Agent says hi!" }),
|
||||
exitCode: 0,
|
||||
});
|
||||
|
||||
const result = await runAgent({
|
||||
workspacePath: workspaceDir,
|
||||
channelName: "test",
|
||||
userMessage: "hello",
|
||||
conversationHistory: [],
|
||||
});
|
||||
|
||||
expect(result.status).toBe("success");
|
||||
if (result.status === "success") {
|
||||
expect(result.text).toBe("Agent says hi!");
|
||||
}
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// (b) timeout
|
||||
// -------------------------------------------------------------------------
|
||||
it('(b) timeout: process never closes → status "timeout" with timeoutMs', async () => {
|
||||
// Never emit close so the timeout fires. Use a very short timeout by
|
||||
// writing a disclaw.yaml that sets claude_timeout_seconds to something tiny.
|
||||
// Instead we rely on the fact that the timeout logic in runner.ts will kill
|
||||
// the child (kill() is a no-op in the mock) and then the close event must
|
||||
// still fire. We simulate this by making close fire *after* a delay longer
|
||||
// than the timeout but still having `killed = true` set by the timer.
|
||||
//
|
||||
// The cleanest approach: let the mock emit close immediately but set
|
||||
// killed=true before it fires by abusing the AbortController signal to
|
||||
// trigger the kill path, and then emit close with code 0.
|
||||
//
|
||||
// Actually the simplest: set closeDelayMs to something > timeout, but
|
||||
// the default timeout is 120s which is way too long for a test. Instead
|
||||
// we use a custom disclaw.yaml in the workspace.
|
||||
|
||||
const configPath = path.join(workspaceDir, "disclaw.yaml");
|
||||
fs.writeFileSync(
|
||||
configPath,
|
||||
`workspace_root: /tmp\nmanagement_channel: disclaw\nclaude_command: claude\nclaude_timeout_seconds: 0.05\nmax_concurrent_agents: 4\nenv_blocklist: []\n`,
|
||||
"utf-8"
|
||||
);
|
||||
|
||||
// The mock's kill() is a no-op, so the close event will never arrive via
|
||||
// the normal path. We need close to fire after the timeout kills the child.
|
||||
// Set a small delay so close fires *after* the 50ms timeout.
|
||||
setMockBehavior({
|
||||
neverClose: false,
|
||||
closeDelayMs: 200, // fires after the 50ms timeout
|
||||
exitCode: 0,
|
||||
});
|
||||
|
||||
// Override __dirname resolution: runner.ts resolves rootDir as 2 levels
|
||||
// up from __dirname (src/agent -> root). We need to point it to our temp
|
||||
// workspace. Use DISCLAW_ROOT env var is not available, so instead we
|
||||
// write disclaw.yaml to the project root's expected location. This is
|
||||
// tricky in a unit test — easier to just accept the default 120s timeout
|
||||
// won't fire and instead mock the behavior differently.
|
||||
//
|
||||
// Simplest working approach: use AbortController to abort → child.kill()
|
||||
// is called → mock emits close immediately with code 0 but killed=true.
|
||||
// BUT abort doesn't set `killed` in the timeout branch — it sets it in
|
||||
// the abort branch, and close code 0 after abort currently resolves success.
|
||||
//
|
||||
// The cleanest unit-testable path: just verify the timeout branch by
|
||||
// patching the timeout to a very small value via the config file in the
|
||||
// project root (since runner.ts uses loadDisclawConfig from 2 levels up).
|
||||
|
||||
// Clean up config to avoid affecting other tests
|
||||
try {
|
||||
fs.unlinkSync(configPath);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
|
||||
// Alternative approach: write config to project root temporarily
|
||||
const projectRoot = path.resolve(__dirname, "../..");
|
||||
const projectConfig = path.join(projectRoot, "disclaw.yaml");
|
||||
const originalConfig = fs.existsSync(projectConfig)
|
||||
? fs.readFileSync(projectConfig, "utf-8")
|
||||
: null;
|
||||
|
||||
try {
|
||||
fs.writeFileSync(
|
||||
projectConfig,
|
||||
`workspace_root: /tmp\nmanagement_channel: disclaw\nclaude_command: claude\nclaude_timeout_seconds: 0.05\nmax_concurrent_agents: 4\nenv_blocklist: []\n`,
|
||||
"utf-8"
|
||||
);
|
||||
|
||||
setMockBehavior({
|
||||
neverClose: false,
|
||||
closeDelayMs: 200,
|
||||
exitCode: 0,
|
||||
});
|
||||
|
||||
const result = await runAgent({
|
||||
workspacePath: workspaceDir,
|
||||
channelName: "test",
|
||||
userMessage: "hello",
|
||||
conversationHistory: [],
|
||||
});
|
||||
|
||||
expect(result.status).toBe("timeout");
|
||||
if (result.status === "timeout") {
|
||||
expect(result.timeoutMs).toBe(50);
|
||||
}
|
||||
} finally {
|
||||
if (originalConfig !== null) {
|
||||
fs.writeFileSync(projectConfig, originalConfig, "utf-8");
|
||||
} else {
|
||||
try {
|
||||
fs.unlinkSync(projectConfig);
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
}
|
||||
}, 10_000);
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// (c) cli-error: non-zero exit code
|
||||
// -------------------------------------------------------------------------
|
||||
it('(c) cli-error: exit code != 0 → status "cli-error"', async () => {
|
||||
setMockBehavior({
|
||||
stdoutData: "",
|
||||
exitCode: 1,
|
||||
});
|
||||
|
||||
const result = await runAgent({
|
||||
workspacePath: workspaceDir,
|
||||
channelName: "test",
|
||||
userMessage: "hello",
|
||||
conversationHistory: [],
|
||||
});
|
||||
|
||||
expect(result.status).toBe("cli-error");
|
||||
if (result.status === "cli-error") {
|
||||
expect(result.message).toMatch(/Claude Code CLI error/);
|
||||
}
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// (d) cli-error ENOENT
|
||||
// -------------------------------------------------------------------------
|
||||
it('(d) cli-error ENOENT: spawn error → status "cli-error" with "not found" message', async () => {
|
||||
const enoentError = Object.assign(new Error("spawn ENOENT"), {
|
||||
code: "ENOENT",
|
||||
}) as NodeJS.ErrnoException;
|
||||
|
||||
setMockBehavior({
|
||||
spawnError: enoentError,
|
||||
});
|
||||
|
||||
const result = await runAgent({
|
||||
workspacePath: workspaceDir,
|
||||
channelName: "test",
|
||||
userMessage: "hello",
|
||||
conversationHistory: [],
|
||||
});
|
||||
|
||||
expect(result.status).toBe("cli-error");
|
||||
if (result.status === "cli-error") {
|
||||
expect(result.message).toMatch(/not found/i);
|
||||
expect(result.stderr).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue