From 2cbca5866c2624f1e5303e75c550a1e124f92f5a Mon Sep 17 00:00:00 2001 From: dbarr5 Date: Fri, 9 Oct 2026 18:53:08 -0400 Subject: [PATCH] fix: keep Ollama call IDs and pasted feedback intact --- src/core/brain_ollama.ts | 10 +++++++-- src/ui/approval_feedback.ts | 18 +++++++++++++-- test/approval_feedback.test.ts | 26 +++++++++++++++++++++ test/brain_ollama.test.ts | 5 +++-- test/bridge.test.ts | 41 ++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 6 deletions(-) diff --git a/src/core/brain_ollama.ts b/src/core/brain_ollama.ts index a46cd2f6..22510744 100644 --- a/src/core/brain_ollama.ts +++ b/src/core/brain_ollama.ts @@ -87,6 +87,9 @@ export class OllamaBrain implements Brain { private withheld: string[] = []; // Incremented per model reply: tool calls batched into one reply share it. private round = 0; + // Ollama may reuse response-local IDs (including synthesized call-1) on + // later replies. The host deduplicates IDs for the whole run. + private nextCallId = 0; constructor(opts: OllamaBrainOptions = {}) { this.opts = opts; @@ -237,7 +240,10 @@ export class OllamaBrain implements Brain { break; } - const calls = reply.tool_calls ?? []; + const calls = (reply.tool_calls ?? []).map((call) => ({ + ...call, + id: `ollama-call-${++this.nextCallId}`, + })); this.round += 1; // A pause received while the network request was in flight takes // effect before any resulting tool call or answer becomes observable. @@ -277,7 +283,7 @@ export class OllamaBrain implements Brain { } // Record the assistant turn (with its tool calls) before the replies. - messages.push(assistantTurn(reply)); + messages.push(assistantTurn({ ...reply, tool_calls: calls })); for (const call of calls) { if (this.aborted) break; diff --git a/src/ui/approval_feedback.ts b/src/ui/approval_feedback.ts index 47c8bfaa..608c9ac5 100644 --- a/src/ui/approval_feedback.ts +++ b/src/ui/approval_feedback.ts @@ -44,9 +44,12 @@ export function promptDenialFeedback(io: ApprovalFeedbackIO = {}): Promise | undefined; const finish = (value: string | null): void => { if (settled) return; settled = true; + if (escapeTimer) clearTimeout(escapeTimer); input.off("data", onData); io.signal?.removeEventListener("abort", onAbort); if (changedRaw) { try { input.setRawMode?.(false); } catch { /* terminal may have closed */ } } @@ -69,9 +72,15 @@ export function promptDenialFeedback(io: ApprovalFeedbackIO = {}): Promise { - const sequences = splitKeys(typeof chunk === "string" ? chunk : decoder.write(chunk)); + if (escapeTimer) clearTimeout(escapeTimer); + const data = pendingSequence + (typeof chunk === "string" ? chunk : decoder.write(chunk)); + // A terminal may split a bracketed-paste marker at any byte. Keep an + // unfinished CSI/SS3 sequence until the next chunk before tokenizing it. + const incomplete = data.match(/\x1b(?:\[[0-9;:<=>?]*[ -/]*|O)?$/); + pendingSequence = incomplete?.[0] ?? ""; + const sequences = splitKeys(data.slice(0, data.length - pendingSequence.length)); const returnTail = (index: number): void => { - const tail = sequences.slice(index + 1).join(""); + const tail = sequences.slice(index + 1).join("") + pendingSequence; if (tail) setImmediate(() => input.emit("data", Buffer.from(tail, "utf8"))); }; for (const [index, seq] of sequences.entries()) { @@ -100,6 +109,11 @@ export function promptDenialFeedback(io: ApprovalFeedbackIO = {}): Promise { pendingSequence = ""; finish(null); }, 50); + } }; input.on("data", onData); io.signal?.addEventListener("abort", onAbort, { once: true }); diff --git a/test/approval_feedback.test.ts b/test/approval_feedback.test.ts index fbc2a8cb..509750cf 100644 --- a/test/approval_feedback.test.ts +++ b/test/approval_feedback.test.ts @@ -61,6 +61,32 @@ test("input batched after feedback submission returns to the next owner", async assert.equal(handedBack, "queued draft"); }); +test("split bracketed-paste delimiters keep pasted newlines inside feedback", async () => { + const io = lease(); + const pending = promptDenialFeedback(io); + let settled = false; + void pending.then(() => { settled = true; }); + io.input.write("\x1b[20"); + io.input.write("0~Use offline tests\nsecond line\x1b[20"); + io.input.write("1~"); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(settled, false, "pasted newline must not submit feedback"); + io.input.write("\r"); + assert.equal(await pending, "Use offline tests second line"); + let handedBack = ""; + io.input.on("data", chunk => { handedBack += String(chunk); }); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(handedBack, "", "pasted text must not leak to the next input owner"); +}); + +test("a paste opener split after Escape is still recognized", async () => { + const io = lease(); + const pending = promptDenialFeedback(io); + io.input.write("\x1b"); + io.input.write("[200~note\ncontinued\x1b[201~\r"); + assert.equal(await pending, "note continued"); +}); + test("turn cancellation releases the feedback lease without a note", async () => { const io = lease(); const controller = new AbortController(); diff --git a/test/brain_ollama.test.ts b/test/brain_ollama.test.ts index af6d2e15..e766d226 100644 --- a/test/brain_ollama.test.ts +++ b/test/brain_ollama.test.ts @@ -81,11 +81,12 @@ test("OllamaBrain emits a tool_call, accepts sendToolResult, then a done", async assert.equal(done?.type === "done" ? done.ok : false, true, "done.ok true on a clean finish"); // The tool_result must have been fed back to the model on the 2nd turn: the - // second chat call's message list includes a role:"tool" reply for c1. + // second chat call's message list includes a role:"tool" reply for this call. const second = calls()[1] ?? []; const toolMsg = second.find((m) => m.role === "tool"); assert.ok(toolMsg, "tool_result fed back into the conversation"); - assert.equal(toolMsg?.tool_call_id, "c1"); + assert.equal(toolMsg?.tool_call_id, call?.type === "tool_call" ? call.id : ""); + assert.equal(second.find((m) => m.role === "assistant")?.tool_calls?.[0]?.id, toolMsg?.tool_call_id); }); test("OllamaBrain answers immediately when the model makes no tool call", async () => { diff --git a/test/bridge.test.ts b/test/bridge.test.ts index edf6d6f8..7871133c 100644 --- a/test/bridge.test.ts +++ b/test/bridge.test.ts @@ -16,6 +16,8 @@ import { } from "../src/core/brain_protocol.js"; import { ToolExecutor, capHeadTail } from "../src/core/tool_executor.js"; import { EventQueue, type Brain, type TaskCommand } from "../src/core/brain.js"; +import { OllamaBrain } from "../src/core/brain_ollama.js"; +import type { ChatMessage, ChatReply } from "../src/core/ollama.js"; import { hostLoop, cmdCode } from "../src/commands/code.js"; import type { AppContext } from "../src/core/context.js"; @@ -379,6 +381,45 @@ test("two sequential tool calls: each result pairs to its own id, in order", asy } }); +test("hostLoop executes successive Ollama replies that reuse a response-local call ID", async () => { + const dir = tmpWorkspace("aether-ollama-ids-"); + try { + writeFileSync(join(dir, "a.txt"), "AAA"); + writeFileSync(join(dir, "b.txt"), "BBB"); + const replies: ChatReply[] = ["a.txt", "b.txt"].map((path) => ({ + role: "assistant", + content: "", + tool_calls: [{ id: "call-1", type: "function", function: { name: "read_file", arguments: JSON.stringify({ path }) } }], + })); + replies.push({ role: "assistant", content: "both read" }); + const requests: ChatMessage[][] = []; + let turn = 0; + const brain = new OllamaBrain({ chat: async (messages) => { + requests.push(messages.map((message) => ({ ...message }))); + return replies[turn++]!; + } }); + const ids: string[] = []; + const code = await hostLoop(brain, new ToolExecutor(dir), (event) => { + if (event.type === "tool_call") ids.push(event.id); + }, { type: "task", text: "read both", cwd: dir, poolGb: 5 }, undefined, undefined, undefined, { + failureBudget: false, + modelSegmentTimeoutMs: 2_000, + }); + assert.equal(code, 0); + assert.equal(ids.length, 2); + assert.notEqual(ids[0], ids[1], "IDs must be unique across model replies"); + const final = requests[2] ?? []; + const assistantCalls = final.filter((message) => message.role === "assistant") + .flatMap((message) => message.tool_calls ?? []); + const results = final.filter((message) => message.role === "tool"); + assert.deepEqual(assistantCalls.map((call) => call.id), ids); + assert.deepEqual(results.map((message) => message.tool_call_id), ids); + assert.deepEqual(results.map((message) => JSON.parse(message.content).content), ["AAA", "BBB"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + // --- probe 2: path-guard canonicalization (the security boundary) ---------- test("path-guard rejects traversal, absolute, and symlink escapes", () => { const dir = tmpWorkspace("aether-guard-");