fix(sanitizer+stream): tighten textual tool-call detection, flush partial buffer (#3355) (#3410)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-08 02:14:08 -03:00
committed by GitHub
parent e0615a8194
commit 67d79f6c44
4 changed files with 97 additions and 5 deletions

View File

@@ -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))
---

View File

@@ -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))
);
}

View File

@@ -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 = {

View File

@@ -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");
});
});