diff --git a/src/commands/chat.ts b/src/commands/chat.ts index 43d2c15..7f04b53 100644 --- a/src/commands/chat.ts +++ b/src/commands/chat.ts @@ -31,6 +31,8 @@ import { loadHistory, appendHistory, historyPath, historyEnabled } from "../core import { VERSION } from "../version.js"; import { chooseBackend, type BackendPath } from "../core/backend.js"; import { OllamaBrain } from "../core/brain_ollama.js"; +import type { Brain } from "../core/brain.js"; +import type { ToolResult } from "../core/tool_executor.js"; import { ToolExecutor } from "../core/tool_executor.js"; import { HostRenderer } from "../ui/host_render.js"; import type { TaskCommand } from "../core/brain.js"; @@ -106,7 +108,9 @@ export async function runTurn( ): Promise { const backend = await resolveBackend(ctx); if (backend === "local") { - await runLocalTurn(ctx, prompt); + // The signal was dropped here, so the REPL's Ctrl+C controller could not + // reach a local turn at all — the abort fired and nothing observed it. + await runLocalTurn(ctx, prompt, signal); return; } await runCloudTurn(ctx, prompt, signal, onFrame, onPulsePaint); @@ -199,10 +203,20 @@ async function runCloudTurn( * each tool_call (one path-guarded ToolExecutor) and replies, the HostRenderer * draws every event. Identical UX to cloud, just an offline brain. */ -async function runLocalTurn(ctx: AppContext, prompt: string): Promise { +export interface LocalTurnDeps { + brain?: Brain; + exec?: { executeAsync(name: string, args: Record): Promise }; +} + +export async function runLocalTurn( + ctx: AppContext, + prompt: string, + signal?: AbortSignal, + deps: LocalTurnDeps = {}, +): Promise { const cwd = ctx.flags.cwd; - const brain = new OllamaBrain(ctx.flags.model ? { model: ctx.flags.model } : {}); - const exec = new ToolExecutor(cwd); + const brain = deps.brain ?? new OllamaBrain(ctx.flags.model ? { model: ctx.flags.model } : {}); + const exec = deps.exec ?? new ToolExecutor(cwd); const renderer = new HostRenderer({ poolGb: 5, json: ctx.flags.json }); const approveTool = async (name: string, args: Record): Promise => { const outcome = decideGate(name, ctx.cfg.permissionMode, ctx.cfg.autoApply, { @@ -226,8 +240,15 @@ async function runLocalTurn(ctx: AppContext, prompt: string): Promise { ...(ctx.flags.model ? { model: ctx.flags.model } : {}), }; let sawError: string | null = null; + // close() is what unblocks a loop parked on a tool result, so an abort that + // arrives mid-turn is observed rather than waiting out the whole turn. + // Registered before the loop starts so an already-aborted signal still fires. + const onAbort = (): void => brain.close(); + if (signal?.aborted) brain.close(); + signal?.addEventListener("abort", onAbort, { once: true }); try { for await (const ev of brain.run(task)) { + if (signal?.aborted) break; renderer.event(ev); if (ev.type === "error") sawError = ev.msg; if (ev.type === "done" && !ev.ok) sawError = ev.result || ev.reason || "turn did not complete"; @@ -241,8 +262,11 @@ async function runLocalTurn(ctx: AppContext, prompt: string): Promise { } } } finally { + signal?.removeEventListener("abort", onAbort); brain.close(); } + // An aborted turn is not a failed one — the user asked for it to stop. + if (signal?.aborted) return; // Mirror runCloudTurn/CONTRACTS.md invariant 5: a streamed error event is a // failed turn, not a silently-successful one — the renderer already painted // it, so callers (cmdChat/run.ts) special-case ChatTurnError to avoid a diff --git a/test/chat_local_abort.test.ts b/test/chat_local_abort.test.ts new file mode 100644 index 0000000..458a6dc --- /dev/null +++ b/test/chat_local_abort.test.ts @@ -0,0 +1,98 @@ +// The REPL builds an AbortController per turn and aborts it on Ctrl+C, but +// runTurn dropped the signal on the local branch — it was accepted as a +// parameter and simply not passed on. Ctrl+C therefore did nothing to a local +// turn: the abort fired and nothing was listening. +// +// These drive runLocalTurn with an injected brain, so no Ollama server, no +// child process and no real tool execution is involved. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { runLocalTurn } from "../src/commands/chat.js"; +import type { AppContext } from "../src/core/context.js"; +import type { Brain, TaskCommand } from "../src/core/brain.js"; +import type { BrainEvent } from "../src/core/brain_protocol.js"; +import type { ToolResult } from "../src/core/tool_executor.js"; + +/** + * A brain that emits one tool_call and then parks forever, exactly like the + * real one waiting on sendToolResult. close() is the only thing that frees it, + * so a turn that ignores the abort signal never returns. + */ +class ParkingBrain implements Brain { + closed = false; + private release: (() => void) | null = null; + + run(_task: TaskCommand): AsyncIterable { + const self = this; + return (async function* (): AsyncGenerator { + yield { type: "tool_call", id: "call-1", name: "read_file", args: { path: "a.ts" } }; + await new Promise((resolve) => { + if (self.closed) resolve(); + else self.release = resolve; + }); + yield { type: "done", ok: true, result: "done", remaining: 0, reason: "" }; + })(); + } + + sendToolResult(_id: string, _result: ToolResult): void {} + control(): void {} + close(): void { + this.closed = true; + this.release?.(); + this.release = null; + } +} + +function ctx(): AppContext { + return { + cfg: { permissionMode: "skip", autoApply: true }, + flags: { cwd: process.cwd(), yes: true, json: true }, + confirm: async () => true, + } as unknown as AppContext; +} + +const noExec = { + executeAsync: async (): Promise => ({ output: "ok", exitCode: 0 }), +}; + +test("aborting a local turn closes the brain instead of waiting the turn out", async () => { + const brain = new ParkingBrain(); + const controller = new AbortController(); + const turn = runLocalTurn(ctx(), "do a thing", controller.signal, { brain, exec: noExec }); + + // Let the turn reach the parked tool call, then Ctrl+C. + await new Promise((resolve) => setTimeout(resolve, 20)); + controller.abort(); + + // Without the signal wired through, this never settles. + await turn; + assert.equal(brain.closed, true, "abort must reach the brain"); +}); + +test("a signal already aborted stops the turn rather than starting it", async () => { + const brain = new ParkingBrain(); + const controller = new AbortController(); + controller.abort(); + await runLocalTurn(ctx(), "do a thing", controller.signal, { brain, exec: noExec }); + assert.equal(brain.closed, true); +}); + +test("an aborted local turn is not reported as a failed turn", async () => { + // A user-requested stop is not an error, and must not surface as one. + const brain = new ParkingBrain(); + const controller = new AbortController(); + const turn = runLocalTurn(ctx(), "do a thing", controller.signal, { brain, exec: noExec }); + await new Promise((resolve) => setTimeout(resolve, 20)); + controller.abort(); + await assert.doesNotReject(() => turn); +}); + +test("without a signal a local turn still completes normally", async () => { + const brain = new ParkingBrain(); + const turn = runLocalTurn(ctx(), "do a thing", undefined, { brain, exec: noExec }); + await new Promise((resolve) => setTimeout(resolve, 20)); + brain.close(); // stand in for the real brain finishing its wait + await turn; + assert.equal(brain.closed, true); +});