diff --git a/open-sse/translator/index.ts b/open-sse/translator/index.ts index 8497989f09..518e50a9db 100644 --- a/open-sse/translator/index.ts +++ b/open-sse/translator/index.ts @@ -735,6 +735,7 @@ export function initState(sourceFormat) { inThinking: false, parseTextualReasoningTags: false, funcArgsBuf: {}, + funcArgsEscapeState: {}, funcNames: {}, funcCallIds: {}, funcArgsDone: {}, diff --git a/open-sse/translator/response/openai-responses.ts b/open-sse/translator/response/openai-responses.ts index d459350d35..915c4942ba 100644 --- a/open-sse/translator/response/openai-responses.ts +++ b/open-sse/translator/response/openai-responses.ts @@ -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; diff --git a/tests/unit/translator-resp-openai-responses.test.ts b/tests/unit/translator-resp-openai-responses.test.ts index 3eeef460b8..f0b64efd97 100644 --- a/tests/unit/translator-resp-openai-responses.test.ts +++ b/tests/unit/translator-resp-openai-responses.test.ts @@ -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([ {