diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d58d90f13..5b11c640ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ _Development cycle in progress — entries are added as work merges into `releas - **fix(mitm):** `getMitmStatus()` in the build-time stub (Docker image) now returns a graceful `{ running: false }` status instead of throwing, so the Agent Bridge UI shows a clean "stopped" state rather than an error banner in containerised deployments. ([#3390](https://github.com/diegosouzapw/OmniRoute/issues/3390)) - **fix(executor):** Mistral (and any provider in `PROVIDERS_REQUIRING_USER_LAST_MESSAGE`) no longer receives a trailing `assistant` message with plain text content — `stripTrailingAssistantForProvider` drops it on the upstream-send path, fixing the `400: Expected last role User or Tool … but got assistant` rejection. ([#3396](https://github.com/diegosouzapw/OmniRoute/issues/3396)) +- **fix(sanitizer):** `containsTextualToolCallContent()` now requires the complete `[Tool call: name]\nArguments:` header pattern instead of a bare `.includes("[Tool call:")` check — prevents the non-streaming response sanitizer from nulling out model responses that merely quote `[Tool call:]` in prose or code examples. ([#3355](https://github.com/diegosouzapw/OmniRoute/issues/3355)) +- **fix(stream):** the streaming textual tool-call guard now flushes any remaining buffered content as plain text when the stream ends, regardless of whether the buffer contains `"Arguments:"` — previously, a partial/incomplete tool-call header that arrived at end-of-stream was silently dropped. ([#3355](https://github.com/diegosouzapw/OmniRoute/issues/3355)) --- diff --git a/open-sse/handlers/responseSanitizer.ts b/open-sse/handlers/responseSanitizer.ts index 3f4d3849c3..35314f326a 100644 --- a/open-sse/handlers/responseSanitizer.ts +++ b/open-sse/handlers/responseSanitizer.ts @@ -170,9 +170,16 @@ function parseTextualToolCallContent(content: unknown): { name: string; args: un return null; } +// Matches the exact header format required by parseTextualToolCallContent: +// "[Tool call: name]\nArguments:" (with optional whitespace). Using the full +// header pattern prevents false positives when the model quotes "[Tool call:" +// in prose, code examples, or terminal output (#3355). +const TEXTUAL_TOOL_CALL_HEADER = /\[Tool call:[^\]\n]+\]\s*\nArguments:/; + function containsTextualToolCallContent(content: unknown): boolean { return ( - typeof content === "string" && stripInternalToolEnvelopeText(content).includes("[Tool call:") + typeof content === "string" && + TEXTUAL_TOOL_CALL_HEADER.test(stripInternalToolEnvelopeText(content)) ); } diff --git a/open-sse/utils/stream.ts b/open-sse/utils/stream.ts index a4e60c2650..452ae2b240 100644 --- a/open-sse/utils/stream.ts +++ b/open-sse/utils/stream.ts @@ -2055,10 +2055,11 @@ export function createSSEStream(options: StreamOptions = {}) { } clearPendingPassthroughEvent(); - if ( - passthroughBufferedTextualToolCallContent && - !passthroughBufferedTextualToolCallContent.includes("Arguments:") - ) { + if (passthroughBufferedTextualToolCallContent) { + // Flush any remaining buffered content as plain text. + // Previously gated on !includes("Arguments:"), which silently dropped + // incomplete tool-call headers (buffer held "Arguments:" but JSON was + // never finished before stream ended) — fix #3355 bug 2. let flushOutput = ""; if (clientExpectsResponsesStream) { const syntheticChunk = { diff --git a/tests/unit/textual-toolcall-sanitizer-false-positive.test.ts b/tests/unit/textual-toolcall-sanitizer-false-positive.test.ts new file mode 100644 index 0000000000..7684b41968 --- /dev/null +++ b/tests/unit/textual-toolcall-sanitizer-false-positive.test.ts @@ -0,0 +1,82 @@ +/** + * Regression tests for #3355: two bugs in the textual tool-call detection path + * triggered by Gemini/agy models in Claude Code. + * + * Bug 1 — Non-streaming response sanitizer false positive + * --------------------------------------------------------- + * `containsTextualToolCallContent()` in responseSanitizer.ts used + * `.includes("[Tool call:")` to check for textual tool calls. When the model's + * response contained the literal text "[Tool call:" as prose (e.g. quoting + * terminal output, explaining its own output, or generating code examples), + * the sanitizer set `sanitized.content = null`, silently erasing the response. + * + * Fix: `containsTextualToolCallContent` now requires the FULL header format + * `/\[Tool call:[^\]\n]+\]\s*\nArguments:/` — the same format that + * `parseTextualToolCallContent` requires — so quoting "[Tool call:" as prose + * is no longer classified as a tool call. + * + * Bug 2 — Streaming guard buffer swallow on stream end + * ----------------------------------------------------- + * `applyTextualToolCallStreamingGuard` in stream.ts accumulated incoming + * content in `passthroughBufferedTextualToolCallContent` when it looked like + * a textual tool call was starting. At stream flush, the buffered content was + * only emitted when it did NOT include "Arguments:" (line 2060). If the + * stream ended mid-parse (buffer contained "Arguments:" but incomplete JSON), + * the buffer was silently dropped — the user received an empty response. + * + * Fix: flush always emits whatever is in the buffer, regardless of whether + * it includes "Arguments:". A partial/incomplete tool-call header is emitted + * as plain text rather than swallowed. + */ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; +import { sanitizeOpenAIResponse } from "../../open-sse/handlers/responseSanitizer.ts"; + +// ─── Bug 1: Non-streaming false positive ─────────────────────────────────── + +const makeMsg = (content: string) => ({ + id: "chatcmpl-test", + object: "chat.completion", + created: 0, + model: "test-model", + choices: [ + { + index: 0, + message: { role: "assistant", content }, + finish_reason: "stop", + }, + ], +}); + +describe("Bug #1 — containsTextualToolCallContent false positives (#3355)", () => { + it("does NOT null out content that mentions [Tool call: x] as prose without Arguments line", () => { + const prose = + "Here is how a tool call looks: [Tool call: get_weather] — you would see this in the output."; + const result = sanitizeOpenAIResponse(makeMsg(prose)); + const content = (result.choices as { message: { content: unknown } }[])[0]?.message?.content; + assert.notStrictEqual(content, null, "content should NOT be nulled out for prose mentions"); + assert.ok(typeof content === "string" && content.length > 0, "content should be preserved"); + }); + + it("does NOT null out content containing [Tool call: x] mid-sentence without Arguments line", () => { + const midSentence = + "The response contained [Tool call: search_web] in the output but no arguments were shown."; + const result = sanitizeOpenAIResponse(makeMsg(midSentence)); + const content = (result.choices as { message: { content: unknown } }[])[0]?.message?.content; + assert.notStrictEqual( + content, + null, + "mid-sentence mention of [Tool call:] without Arguments: should not be stripped" + ); + }); + + it("DOES null out content that is a real textual tool call (header + Arguments + JSON)", () => { + const realToolCall = "[Tool call: terminal]\nArguments: {\"command\":\"echo hi\"}"; + const result = sanitizeOpenAIResponse(makeMsg(realToolCall)); + const choices = result.choices as { message: { content: unknown; tool_calls?: unknown[] } }[]; + const msg = choices[0]?.message; + // Parsed into tool_calls, so content is nulled and tool_calls populated + assert.strictEqual(msg?.content, null, "real tool call content should be nulled"); + assert.ok(Array.isArray(msg?.tool_calls) && msg.tool_calls.length > 0, "tool_calls populated"); + }); +});