From eae899659024076ed48668a4966fae2fee1e2792 Mon Sep 17 00:00:00 2001 From: Ismael Briasco <1795203+briascoi@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:08:21 +0200 Subject: [PATCH] fix(ollama-native): defer a mid-turn conversational message instead of refusing the replay Codex writes mid-turn context items inside a single turn: a PostToolUse hook verdict from hooks.json, a context notice. One of them routinely lands between the assistant tool_calls message and that call's own tool result, so buildNativeMessages saw a conversational message while the batch was open, flushed it, found a call without a result, and threw: ollama-native tool call call_x is missing its tool result; refusing interrupted replay Codex surfaces that local validation failure as 502 Provider unreachable, and the item order is part of the persisted thread history, so every later turn of the affected thread failed the same way and the task could not be resumed. The chat adapter already repairs this shape: src/adapters/openai-chat/messages.ts defers barrier messages until the tool round completes, reattaches the real results to their original call occurrence, and answers an unresolvable call with an explicit "no tool result was recorded" tool message. The native transport now does both, request-locally. The strict pair checks are untouched: an orphan result, a duplicate result for the same call, and a result naming another tool still throw. structure/providers/chat-compat.md records that the native wire now carries the chat wire's deferred-barrier contract, so the two cannot drift apart silently. Reported in #4842. --- src/adapters/ollama-native.ts | 50 ++++++- structure/providers/chat-compat.md | 9 ++ tests/providers/ollama/ollama-native.test.ts | 146 +++++++++++++++++++ 3 files changed, 197 insertions(+), 8 deletions(-) diff --git a/src/adapters/ollama-native.ts b/src/adapters/ollama-native.ts index 569ae90622..c78f154d49 100644 --- a/src/adapters/ollama-native.ts +++ b/src/adapters/ollama-native.ts @@ -306,17 +306,37 @@ function buildNativeMessages( // owned by this adapter/request lifecycle rather than process-global state. reservedToolCallIds.clear(); let pending: PendingToolBatch | undefined; + // Codex records mid-turn injections (a PostToolUse hook verdict, a context notice) between an + // assistant tool call and that call's own tool result. Native Ollama needs the call and its + // results adjacent, so those conversational messages wait here instead of closing the batch + // early. The openai-chat adapter defers them the same way; refusing the replay killed the turn. + let deferred: OllamaNativeMessage[] = []; + + const releaseDeferred = (): void => { + if (deferred.length === 0) return; + messages.push(...deferred); + deferred = []; + }; const flushPending = (): void => { if (!pending) return; for (const call of pending.calls) { if (!call.result) { - throw new Error(`ollama-native tool call ${call.id} is missing its tool result; refusing interrupted replay`); + // No result exists anywhere in the replayed history: the turn was interrupted, or the + // result never reached it. State exactly that instead of inventing an outcome, and keep + // the conversation replayable. + messages.push({ + role: "tool", + tool_call_id: call.id, + tool_name: call.wireName, + // Same marker text as the chat adapter (openai-chat/messages.ts), so both adapters read + // the same in an operator's log. The name is this wire's flattened tool name, which is + // what the assistant turn above it carries. + content: `[ocx] no tool result was recorded for "${call.wireName}"; execution status unknown — do not treat this as success, failure, or user-provided input.`, + }); + continue; } - } - for (const call of pending.calls) { - const result = call.result!; - const translated = contentToNative(result.content, "tool result"); + const translated = contentToNative(call.result.content, "tool result"); messages.push({ role: "tool", tool_call_id: call.id, @@ -326,6 +346,7 @@ function buildNativeMessages( }); } pending = undefined; + releaseDeferred(); }; for (const message of parsed.context.messages) { @@ -347,9 +368,22 @@ function buildNativeMessages( continue; } - // Native Ollama requires the whole assistant tool-call turn followed by its tool results. A - // new conversational message is a hard boundary; unresolved calls are never fabricated. - if (pending) flushPending(); + // Native Ollama requires the whole assistant tool-call turn followed by its tool results. A + // conversational message that arrives while the batch is still open is held aside instead of + // closing it, so the call keeps its results adjacent; it is released right after the batch + // flushes. Anything else (a new assistant turn) settles the batch first. + if (pending) { + if (message.role === "user" || message.role === "developer") { + const translated = message.role === "user" + ? contentToNative(message.content, "user") + : contentToNative(message.content, "developer", false); + deferred.push(message.role === "user" + ? { role: "user", content: translated.content, ...(translated.images ? { images: translated.images } : {}) } + : { role: "system", content: translated.content }); + continue; + } + flushPending(); + } switch (message.role) { case "user": { diff --git a/structure/providers/chat-compat.md b/structure/providers/chat-compat.md index 9223e4462d..c96f612f43 100644 --- a/structure/providers/chat-compat.md +++ b/structure/providers/chat-compat.md @@ -55,6 +55,15 @@ messages until the round completes, reattaching real results to their original c and synthesizing explicit "no tool result was recorded" answers only when no real result exists (Kimi/Moonshot 400 `ocx-mrqaiw05-269`; unit `devlog/_fin/260718_dangling_toolcall_hardening`). +The native Ollama wire carries the same contract. `src/adapters/ollama-native.ts` +`buildNativeMessages` defers `user`/`developer` messages that arrive while a batch is open and +releases them after the tool messages, and answers a call with no result anywhere in the replayed +history with the same `[ocx] no tool result was recorded for ""` marker. The shape it +absorbs is ordinary Codex history, not a malformed one: Codex records mid-turn items (a +`PostToolUse` hook verdict, a context notice) between an assistant `tool_calls` message and that +call's own result. The strict pair checks (orphan result, duplicate result, result naming another +tool) still throw on both wires (#4842). + Forward-mode OpenAI passthrough also repairs replayed `call_id` values longer than the Responses API's 64-character limit. Sidechat/fork replay can namespace routed-provider ids beyond that limit, so each oversized id and all matching call/output items receive the same deterministic, diff --git a/tests/providers/ollama/ollama-native.test.ts b/tests/providers/ollama/ollama-native.test.ts index d22cdc1ce4..44a09e8a8c 100644 --- a/tests/providers/ollama/ollama-native.test.ts +++ b/tests/providers/ollama/ollama-native.test.ts @@ -261,4 +261,150 @@ describe("ollama-native — request shape", () => { { role: "user", content: [{ type: "video", videoUrl: "data:video/mp4;base64,AAAA" }] }, ]))).toThrow(/cannot send video/); }); + + test("a mid-turn developer message is deferred instead of closing the tool batch", async () => { + const adapter = createOllamaNativeAdapter(ollamaProvider()); + const { body } = await adapter.buildRequest(parsedWith([ + { role: "user", content: "continue", timestamp: 0 }, + { + role: "assistant", + content: [ + { type: "text", text: "applying the patch" }, + { type: "toolCall", id: "call_hook_split", name: "exec", arguments: { cmd: "ls" } }, + ], + timestamp: 1, + }, + { role: "developer", content: "[hook] design findings requiring review", timestamp: 2 }, + { role: "toolResult", toolCallId: "call_hook_split", toolName: "exec", content: "done", isError: false, timestamp: 3 }, + ])); + const messages = JSON.parse(String(body)).messages; + expect(messages.map((message: { role: string }) => message.role)) + .toEqual(["user", "assistant", "tool", "system"]); + expect(messages[2].tool_call_id).toBe("call_hook_split"); + expect(messages[2].content).toBe("done"); + expect(messages[3].content).toBe("[hook] design findings requiring review"); + }); + + test("a deferred user message keeps its text and images after the tool result", async () => { + const adapter = createOllamaNativeAdapter(ollamaProvider()); + const png = "data:image/png;base64,iVBORw0KGgo="; + const { body } = await adapter.buildRequest(parsedWith([ + { role: "user", content: "start", timestamp: 0 }, + { + role: "assistant", + content: [{ type: "toolCall", id: "call_mid_user", name: "exec", arguments: { cmd: "ls" } }], + timestamp: 1, + }, + { + role: "user", + content: [{ type: "text", text: "look at this" }, { type: "image", imageUrl: png }], + timestamp: 2, + }, + { role: "toolResult", toolCallId: "call_mid_user", toolName: "exec", content: "done", isError: false, timestamp: 3 }, + ])); + const messages = JSON.parse(String(body)).messages; + expect(messages.map((message: { role: string }) => message.role)) + .toEqual(["user", "assistant", "tool", "user"]); + expect(messages[2].tool_call_id).toBe("call_mid_user"); + expect(messages[2].content).toBe("done"); + expect(messages[3].content).toBe("look at this"); + expect(messages[3].images).toEqual(["iVBORw0KGgo="]); + }); + + test("out-of-order results inside a parallel batch still serialize in call order", async () => { + const adapter = createOllamaNativeAdapter(ollamaProvider()); + const { body } = await adapter.buildRequest(parsedWith([ + { role: "user", content: "continue", timestamp: 0 }, + { + role: "assistant", + content: [ + { type: "toolCall", id: "call_first", name: "exec", arguments: { cmd: "ls" } }, + { type: "toolCall", id: "call_second", name: "exec", arguments: { cmd: "pwd" } }, + ], + timestamp: 1, + }, + { role: "developer", content: "[hook] findings", timestamp: 2 }, + { role: "toolResult", toolCallId: "call_second", toolName: "exec", content: "second", isError: false, timestamp: 3 }, + { role: "toolResult", toolCallId: "call_first", toolName: "exec", content: "first", isError: false, timestamp: 4 }, + ])); + const messages = JSON.parse(String(body)).messages; + expect(messages.map((message: { role: string }) => message.role)) + .toEqual(["user", "assistant", "tool", "tool", "system"]); + expect(messages[2].tool_call_id).toBe("call_first"); + expect(messages[2].content).toBe("first"); + expect(messages[3].tool_call_id).toBe("call_second"); + expect(messages[3].content).toBe("second"); + }); + + test("a call with no recorded result answers with an explicit unknown status", async () => { + const adapter = createOllamaNativeAdapter(ollamaProvider()); + const { body } = await adapter.buildRequest(parsedWith([ + { role: "user", content: "start", timestamp: 0 }, + { + role: "assistant", + content: [{ type: "toolCall", id: "call_interrupted", name: "exec", arguments: { cmd: "ls" } }], + timestamp: 1, + }, + { role: "user", content: "continue", timestamp: 2 }, + ])); + const messages = JSON.parse(String(body)).messages; + expect(messages.map((message: { role: string }) => message.role)) + .toEqual(["user", "assistant", "tool", "user"]); + expect(messages[2].tool_call_id).toBe("call_interrupted"); + expect(messages[2].content).toContain("no tool result was recorded"); + expect(messages[2].content).toContain('"exec"'); + expect(messages[2].content).toContain("do not treat this as success, failure, or user-provided input"); + expect(messages[3].content).toBe("continue"); + }); + + test("a second assistant turn settles the first batch before its own", async () => { + const adapter = createOllamaNativeAdapter(ollamaProvider()); + const { body } = await adapter.buildRequest(parsedWith([ + { role: "user", content: "start", timestamp: 0 }, + { + role: "assistant", + content: [{ type: "toolCall", id: "call_first", name: "exec", arguments: { cmd: "ls" } }], + timestamp: 1, + }, + { role: "developer", content: "[hook] findings", timestamp: 2 }, + { role: "toolResult", toolCallId: "call_first", toolName: "exec", content: "first done", isError: false, timestamp: 3 }, + { + role: "assistant", + content: [{ type: "toolCall", id: "call_second", name: "exec", arguments: { cmd: "pwd" } }], + timestamp: 4, + }, + { role: "toolResult", toolCallId: "call_second", toolName: "exec", content: "second done", isError: false, timestamp: 5 }, + ])); + const messages = JSON.parse(String(body)).messages; + expect(messages.map((message: { role: string }) => message.role)) + .toEqual(["user", "assistant", "tool", "system", "assistant", "tool"]); + expect(messages[2].tool_call_id).toBe("call_first"); + expect(messages[2].content).toBe("first done"); + expect(messages[3].content).toBe("[hook] findings"); + expect(messages[4].tool_calls[0].id).toBe("call_second"); + expect(messages[5].tool_call_id).toBe("call_second"); + expect(messages[5].content).toBe("second done"); + }); + + test("an orphan tool result is still refused", () => { + const adapter = createOllamaNativeAdapter(ollamaProvider()); + expect(() => adapter.buildRequest(parsedWith([ + { role: "user", content: "hi" }, + { role: "toolResult", toolCallId: "call_ghost", toolName: "exec", content: "x", isError: false, timestamp: 1 }, + ]))).toThrow(/orphan tool result/); + }); + + test("a duplicate result for the same call is still refused", () => { + const adapter = createOllamaNativeAdapter(ollamaProvider()); + expect(() => adapter.buildRequest(parsedWith([ + { role: "user", content: "hi" }, + { + role: "assistant", + content: [{ type: "toolCall", id: "call_once", name: "exec", arguments: { cmd: "ls" } }], + timestamp: 1, + }, + { role: "toolResult", toolCallId: "call_once", toolName: "exec", content: "once", isError: false, timestamp: 2 }, + { role: "toolResult", toolCallId: "call_once", toolName: "exec", content: "twice", isError: false, timestamp: 3 }, + ]))).toThrow(/duplicate tool result/); + }); });