mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-12 02:02:13 +03:00
fix(sse): persist per-tool-call JSON escape state across SSE delta chunks
escapeJsonStringValues() reset its inString/pendingEscape state on every call instead of carrying it forward per tool-call index, so a raw newline byte (or an already-escaped \n) split across two delta chunks got corrupted in transit — the model's own output was correctly escaped, OmniRoute broke it. Root-caused via a dispatched investigation into real OpenClaw traffic that looked like model-generation quality but wasn't. Fix: escapeJsonStringValues now takes and mutates a persistent per-call state object (JsonStringEscapeState), keyed per tool-call index in the translator's init state and cleared when a tool call is superseded.
This commit is contained in:
committed by
diegosouzapw
parent
aae408f585
commit
3268b3b7af
@@ -735,6 +735,7 @@ export function initState(sourceFormat) {
|
||||
inThinking: false,
|
||||
parseTextualReasoningTags: false,
|
||||
funcArgsBuf: {},
|
||||
funcArgsEscapeState: {},
|
||||
funcNames: {},
|
||||
funcCallIds: {},
|
||||
funcArgsDone: {},
|
||||
|
||||
@@ -31,6 +31,19 @@ import {
|
||||
// normalizeUpstreamFailure is re-exported for external importers (tests).
|
||||
export { normalizeUpstreamFailure } from "./openai-responses/pureHelpers.ts";
|
||||
|
||||
/** Carries escapeJsonStringValues's scan state (whether we're inside a JSON
|
||||
* string, and whether the fragment ended mid-escape-sequence) across calls
|
||||
* for the SAME tool call — see escapeJsonStringValues's own doc comment for
|
||||
* why this must persist across chunks rather than reset per call. */
|
||||
interface JsonStringEscapeState {
|
||||
inString: boolean;
|
||||
pendingEscape: boolean;
|
||||
}
|
||||
|
||||
function createJsonStringEscapeState(): JsonStringEscapeState {
|
||||
return { inString: false, pendingEscape: false };
|
||||
}
|
||||
|
||||
/**
|
||||
* Escape control characters (newlines, tabs, carriage returns) that appear
|
||||
* inside JSON string values, ensuring the resulting string is valid JSON.
|
||||
@@ -38,18 +51,42 @@ export { normalizeUpstreamFailure } from "./openai-responses/pureHelpers.ts";
|
||||
* newlines (0x0A) instead of \n escapes inside tool call argument JSON.
|
||||
* Only escapes characters inside string contexts to avoid double-escaping
|
||||
* already-proper JSON or corrupting structural newlines.
|
||||
*
|
||||
* `arguments` deltas arrive as arbitrary fragments of one continuous JSON
|
||||
* string (OpenAI's Chat Completions streaming contract only guarantees each
|
||||
* `tool_calls[].function.arguments` delta is the next slice, not that it
|
||||
* starts/ends on a quote or escape boundary) — a large multi-line argument
|
||||
* value routinely gets split mid-string. `escapeState` must therefore be the
|
||||
* SAME object passed in on every call for a given tool call index, not a
|
||||
* fresh `{inString: false}` each time: resetting per call made the
|
||||
* in-string/out-of-string decision (and therefore whether a raw newline
|
||||
* gets escaped) depend on where a chunk boundary happened to fall, which
|
||||
* produced a real, reported bug — a single reassembled arguments string
|
||||
* with a mix of real newlines and literal two-character `\n` sequences,
|
||||
* breaking generated code (e.g. Python) that embeds multi-line content.
|
||||
*/
|
||||
function escapeJsonStringValues(json: string): string {
|
||||
function escapeJsonStringValues(json: string, escapeState: JsonStringEscapeState): string {
|
||||
let result = "";
|
||||
let inString = false;
|
||||
let { inString, pendingEscape } = escapeState;
|
||||
|
||||
for (let i = 0; i < json.length; i++) {
|
||||
const ch = json[i];
|
||||
|
||||
// Inside a string, skip over escape sequences
|
||||
// This char is the one immediately following a backslash from a
|
||||
// previous iteration (possibly in a prior fragment) — it's already
|
||||
// "consumed" by that escape sequence, pass it through untouched.
|
||||
if (pendingEscape) {
|
||||
result += ch;
|
||||
pendingEscape = false;
|
||||
continue;
|
||||
}
|
||||
|
||||
// Inside a string, an unescaped backslash starts an escape sequence —
|
||||
// the char AFTER it (next iteration, possibly in the next fragment)
|
||||
// must not be reinterpreted as a quote/control-char in its own right.
|
||||
if (inString && ch === "\\") {
|
||||
result += ch + (json[i + 1] ?? "");
|
||||
i++;
|
||||
result += ch;
|
||||
pendingEscape = true;
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -69,6 +106,8 @@ function escapeJsonStringValues(json: string): string {
|
||||
result += ch;
|
||||
}
|
||||
|
||||
escapeState.inString = inString;
|
||||
escapeState.pendingEscape = pendingEscape;
|
||||
return result;
|
||||
}
|
||||
|
||||
@@ -471,6 +510,7 @@ function emitToolCall(state, emit, tc) {
|
||||
delete state.funcArgsDone[tcIdx];
|
||||
delete state.funcItemAdded[tcIdx];
|
||||
delete state.funcItemDone[tcIdx];
|
||||
delete state.funcArgsEscapeState?.[tcIdx];
|
||||
}
|
||||
|
||||
if (funcName) state.funcNames[tcIdx] = funcName;
|
||||
@@ -517,7 +557,14 @@ function emitToolCall(state, emit, tc) {
|
||||
if (tc.function?.arguments) {
|
||||
const refCallId = state.funcCallIds[tcIdx] || newCallId;
|
||||
const existingArgs = state.funcArgsBuf[tcIdx] || "";
|
||||
const sanitized = escapeJsonStringValues(tc.function.arguments);
|
||||
if (!state.funcArgsEscapeState) state.funcArgsEscapeState = {};
|
||||
if (!state.funcArgsEscapeState[tcIdx]) {
|
||||
state.funcArgsEscapeState[tcIdx] = createJsonStringEscapeState();
|
||||
}
|
||||
const sanitized = escapeJsonStringValues(
|
||||
tc.function.arguments,
|
||||
state.funcArgsEscapeState[tcIdx]
|
||||
);
|
||||
const nextArgs = appendToolCallArgumentDelta(existingArgs, sanitized);
|
||||
const emittedDelta = nextArgs.slice(existingArgs.length);
|
||||
state.funcArgsBuf[tcIdx] = nextArgs;
|
||||
|
||||
@@ -668,6 +668,139 @@ test("OpenAI -> Responses: Python multi-line content with indentation survives t
|
||||
assert.ok(newlineCount > 5, "should have many actual newlines in Python code");
|
||||
});
|
||||
|
||||
test("OpenAI -> Responses: a raw newline byte split across two tool-call argument deltas (fragment boundary lands mid-string, not on a quote/escape) is still escaped correctly", () => {
|
||||
// Real reported bug: escapeJsonStringValues used to track "are we inside a
|
||||
// JSON string" as a LOCAL variable reset on every call instead of state
|
||||
// persisted across chunks for the same tool call. A provider that sends a
|
||||
// raw newline byte (0x0A, not a proper \n escape — Gemini/Gemma-style) mid
|
||||
// fragment worked fine when the whole arguments string arrived in one
|
||||
// chunk, but broke the moment the SSE stream happened to split the
|
||||
// fragment somewhere that wasn't a quote or a complete escape sequence:
|
||||
// the second fragment's call started fresh with inString=false even
|
||||
// though the true position was still inside the "content" string value,
|
||||
// so the raw newline in fragment 2 was never escaped — producing invalid
|
||||
// JSON that JSON.parse rejects outright ("Bad control character in
|
||||
// string literal").
|
||||
const events = collectEvents([
|
||||
{
|
||||
id: "chatcmpl-split-nl",
|
||||
model: "gemma-4-26b-a4b-it",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: {
|
||||
tool_calls: [
|
||||
{
|
||||
index: 0,
|
||||
id: "call_split_nl",
|
||||
type: "function",
|
||||
function: {
|
||||
name: "write",
|
||||
// Fragment 1 ends mid-string (no closing quote, no
|
||||
// trailing backslash) — this is the boundary that
|
||||
// exposed the bug.
|
||||
arguments: '{"path":"/tmp/x.txt","content":"line1',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
finish_reason: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id: "chatcmpl-split-nl",
|
||||
model: "gemma-4-26b-a4b-it",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: {
|
||||
// Fragment 2 starts with a RAW newline byte (real \n, not the
|
||||
// two-char escape) while still inside the "content" string.
|
||||
tool_calls: [{ index: 0, function: { arguments: '\nline2\nline3"}' } }],
|
||||
},
|
||||
finish_reason: "tool_calls",
|
||||
},
|
||||
],
|
||||
usage: { prompt_tokens: 10, completion_tokens: 20, total_tokens: 30 },
|
||||
},
|
||||
]);
|
||||
|
||||
const done = events.find(
|
||||
(e) => e.event === "response.output_item.done" && e.data.item?.type === "function_call"
|
||||
);
|
||||
assert.ok(done, "should emit output_item.done for function_call");
|
||||
|
||||
const argsStr = done.data.item.arguments;
|
||||
// The bug produced invalid JSON here (raw control character in a JSON
|
||||
// string) — JSON.parse must succeed and round-trip the real newlines.
|
||||
const parsed = JSON.parse(argsStr);
|
||||
assert.equal(parsed.path, "/tmp/x.txt");
|
||||
assert.equal(parsed.content, "line1\nline2\nline3");
|
||||
});
|
||||
|
||||
test("OpenAI -> Responses: a properly-escaped \\n split exactly between its backslash and the 'n' across two deltas is not corrupted", () => {
|
||||
// Second half of the same bug class as the test above, exercising the
|
||||
// OTHER new state field (pendingEscape, not just inString): a model that
|
||||
// correctly escaped a newline as the two characters `\` + `n` can still
|
||||
// have that pair split across an SSE chunk boundary — fragment 1 ends
|
||||
// with the lone backslash, fragment 2 starts with the "n". The old code's
|
||||
// per-call reset meant fragment 2 saw a bare "n" with no idea it was the
|
||||
// second half of an escape sequence; a naive re-implementation could
|
||||
// easily re-escape or mis-handle it. This must reassemble to exactly one
|
||||
// real newline, not a literal backslash-n or a doubled escape.
|
||||
const events = collectEvents([
|
||||
{
|
||||
id: "chatcmpl-split-esc",
|
||||
model: "gemma-4-26b-a4b-it",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: {
|
||||
tool_calls: [
|
||||
{
|
||||
index: 0,
|
||||
id: "call_split_esc",
|
||||
type: "function",
|
||||
function: {
|
||||
name: "write",
|
||||
// Ends right after the backslash of a "\n" escape — the "n"
|
||||
// itself is not yet in this fragment.
|
||||
arguments: '{"path":"/tmp/y.txt","content":"before\\',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
finish_reason: null,
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
id: "chatcmpl-split-esc",
|
||||
model: "gemma-4-26b-a4b-it",
|
||||
choices: [
|
||||
{
|
||||
index: 0,
|
||||
delta: {
|
||||
tool_calls: [{ index: 0, function: { arguments: 'nafter"}' } }],
|
||||
},
|
||||
finish_reason: "tool_calls",
|
||||
},
|
||||
],
|
||||
usage: { prompt_tokens: 10, completion_tokens: 20, total_tokens: 30 },
|
||||
},
|
||||
]);
|
||||
|
||||
const done = events.find(
|
||||
(e) => e.event === "response.output_item.done" && e.data.item?.type === "function_call"
|
||||
);
|
||||
assert.ok(done, "should emit output_item.done for function_call");
|
||||
|
||||
const parsed = JSON.parse(done.data.item.arguments);
|
||||
assert.equal(parsed.path, "/tmp/y.txt");
|
||||
assert.equal(parsed.content, "before\nafter");
|
||||
});
|
||||
|
||||
test("OpenAI -> Responses: parallel tool calls with mixed content survive translation", () => {
|
||||
const events = collectEvents([
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user