From dac626026e0faa754e3e330d26ce5ffc4fc9fea2 Mon Sep 17 00:00:00 2001 From: Markus Hartung Date: Thu, 6 Aug 2026 14:50:35 +0200 Subject: [PATCH] fix(conversations): stop truncated tool_use previews from breaking JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A tool_use/tool_result turn's text is the tool call's raw JSON arguments (or a stringified result) — slicing that raw JSON at a fixed character offset (TEXT_PREVIEW_LENGTH) routinely landed mid-string, storing INVALID JSON. The conversations page's toTurn() then failed to JSON.parse it and fell back to showing the raw, still-escaped text verbatim: a large edit/write/apply_patch-style tool call with a long content field rendered with literal `\n` sequences visible instead of real line breaks, looking exactly like a JSON-escaping bug rather than a big diff. Confirmed live on omniroute-dev: 3 stored `edit` tool_use previews were sitting at exactly 8000 chars with "Unterminated string in JSON" on parse. buildTextPreview now parses first and caps oversized string VALUES inside the JSON instead of slicing the raw blob, so a truncated payload is always valid, re-parseable JSON. Plain text turns are unaffected (still a simple slice — a cut-off sentence is harmless). Also fixes JsonViewer's string rendering to preserve line breaks (whitespace-pre-wrap) — a correctly-parsed multi-line tool argument was still visually squashing onto one line without it. Unrelated cleanup found while editing: conversationTracker.ts had two literal NUL bytes (pre-existing, not introduced by this change) sitting where a template-literal space belonged, making the file register as binary to grep/rg/file. Restored to plain spaces. Co-authored-by: Markus Hartung --- open-sse/services/conversationTracker.ts | 55 ++++++++++++- .../components/shared/JsonViewer.tsx | 5 +- tests/unit/conversationTracker.test.ts | 78 +++++++++++++++++++ 3 files changed, 134 insertions(+), 4 deletions(-) diff --git a/open-sse/services/conversationTracker.ts b/open-sse/services/conversationTracker.ts index 47ae6224d2..7528a8337d 100644 --- a/open-sse/services/conversationTracker.ts +++ b/open-sse/services/conversationTracker.ts @@ -259,12 +259,61 @@ export function computeFingerprintHash(input: { // while still bounding pathological outliers. const TEXT_PREVIEW_LENGTH = 8000; +/** Caps every string leaf in a parsed JSON value to `maxLen`, recursing + * through arrays/objects — used so re-serializing after truncation always + * yields valid JSON (see buildTextPreview's doc comment). */ +function truncateStringsDeep(value: unknown, maxLen: number): unknown { + if (typeof value === "string") { + return value.length > maxLen ? `${value.slice(0, maxLen)}…` : value; + } + if (Array.isArray(value)) { + return value.map((item) => truncateStringsDeep(item, maxLen)); + } + if (value && typeof value === "object") { + const out: Record = {}; + for (const [key, val] of Object.entries(value as Record)) { + out[key] = truncateStringsDeep(val, maxLen); + } + return out; + } + return value; +} + +/** + * Bounds a turn's stored text_preview. Plain text turns are safe to slice at + * a fixed character offset — a cut-off sentence is still readable. A + * tool_use/tool_result turn's `text` is the tool call's raw JSON arguments + * (or a stringified result) — see stringifyContent's callers — so slicing + * the RAW JSON string at a fixed offset routinely lands mid-string, storing + * INVALID JSON. The frontend (src/app/(dashboard)/dashboard/conversations/ + * page.tsx's toTurn) tries to JSON.parse this for rendering and, on failure, + * falls back to showing the raw text verbatim — which for a truncated tool + * payload means the still-JSON-escaped text (literal `\n` sequences, quotes, + * etc.) shown as-is instead of real line breaks: a genuine large `edit`/ + * `write` tool call looking like a broken escaping bug rather than a big + * diff. Parse first and cap each string VALUE instead, so a truncated + * payload is always valid JSON the frontend can actually parse and render. + */ +function buildTextPreview(turn: CanonicalTurn): string { + if (turn.blockKind === "text" || turn.text.length <= TEXT_PREVIEW_LENGTH) { + return turn.text.slice(0, TEXT_PREVIEW_LENGTH); + } + try { + const parsed = JSON.parse(turn.text); + return JSON.stringify(truncateStringsDeep(parsed, TEXT_PREVIEW_LENGTH)); + } catch { + // Not valid JSON to begin with (rare/malformed upstream tool call) — a + // fixed-offset slice couldn't have preserved parseability either way. + return turn.text.slice(0, TEXT_PREVIEW_LENGTH); + } +} + function hashTurnContent(turn: CanonicalTurn): string { - return hashHex(`${turn.role}${turn.text}`); + return hashHex(`${turn.role} ${turn.text}`); } function chainNodeId(parentId: string, turn: CanonicalTurn): string { - return hashHex(`${parentId}${hashTurnContent(turn)}`); + return hashHex(`${parentId} ${hashTurnContent(turn)}`); } interface NewTurnNode { @@ -296,7 +345,7 @@ function buildNewNodes( parentId: parent === rootId ? null : parent, role: turn.role, contentHash: hashTurnContent(turn), - textPreview: turn.text.slice(0, TEXT_PREVIEW_LENGTH), + textPreview: buildTextPreview(turn), blockKind: turn.blockKind, toolName: turn.toolName, }); diff --git a/src/app/(dashboard)/dashboard/tools/traffic-inspector/components/shared/JsonViewer.tsx b/src/app/(dashboard)/dashboard/tools/traffic-inspector/components/shared/JsonViewer.tsx index c42d2ecb6a..fda6e13022 100644 --- a/src/app/(dashboard)/dashboard/tools/traffic-inspector/components/shared/JsonViewer.tsx +++ b/src/app/(dashboard)/dashboard/tools/traffic-inspector/components/shared/JsonViewer.tsx @@ -15,7 +15,10 @@ function JsonNode({ data, depth = 0 }: { data: unknown; depth?: number }) { if (data === null) return null; if (typeof data === "boolean") return {String(data)}; if (typeof data === "number") return {String(data)}; - if (typeof data === "string") return "{data}"; + if (typeof data === "string") + return ( + "{data}" + ); if (Array.isArray(data)) { if (data.length === 0) return []; diff --git a/tests/unit/conversationTracker.test.ts b/tests/unit/conversationTracker.test.ts index 51a9508749..f985360dfb 100644 --- a/tests/unit/conversationTracker.test.ts +++ b/tests/unit/conversationTracker.test.ts @@ -620,3 +620,81 @@ test("resolveConversationId: client-supplied X-Omniroute-Session-Id wins outrigh }); assert.equal(second.conversationId, headerValue); }); + +test("resolveConversationId: a plain text turn over the preview bound is simply sliced", async () => { + const longText = "y".repeat(9000); + const turn = await resolveConversationId({ + body: { model: "big-pickle", messages: [{ role: "user", content: longText }] }, + model: "big-pickle", + apiKeyId: "key-long-text", + clientSessionIdHeader: null, + correlationId: nextCorrelationId(), + }); + + const tree = getConversationTurnTree(turn.conversationId); + assert.equal(tree.length, 1); + assert.equal(tree[0].textPreview.length, 8000); + assert.equal(tree[0].textPreview, longText.slice(0, 8000)); +}); + +// Real bug (2026-08-06): a tool_use turn's `text` is the tool call's raw +// JSON `arguments` string (see extractCanonicalTurns/stringifyContent — a +// JSON string is passed through as-is, never parsed). Slicing that raw JSON +// at a fixed character offset routinely lands mid-string, storing INVALID +// JSON — /dashboard/conversations page.tsx's toTurn() then fails to +// JSON.parse it and falls back to showing the raw, still-escaped text +// verbatim: a large `edit`/`write`/apply_patch-style tool call with a long +// `content`/`new_string` field renders with literal `\n` sequences visible +// instead of real line breaks, looking exactly like a JSON-escaping bug. +test("resolveConversationId: an oversized tool_use turn stores truncated but still VALID JSON", async () => { + const bigContent = "line one\nline two\n".repeat(1000); // well over 8000 chars + const args = JSON.stringify({ path: "/tmp/big.md", content: bigContent }); + assert.ok( + args.length > 8000, + "the raw arguments JSON must exceed the preview bound for this test" + ); + + const turn = await resolveConversationId({ + body: { + model: "big-pickle", + input: [{ type: "function_call", name: "write", call_id: "c1", arguments: args }], + }, + model: "big-pickle", + apiKeyId: "key-big-tool-call", + clientSessionIdHeader: null, + correlationId: nextCorrelationId(), + }); + + const tree = getConversationTurnTree(turn.conversationId); + assert.equal(tree.length, 1); + assert.equal(tree[0].blockKind, "tool_use"); + + // The critical assertion: whatever got stored must be re-parseable JSON, + // never a mid-string cut fragment. + let parsed: { path?: string; content?: string } | undefined; + assert.doesNotThrow(() => { + parsed = JSON.parse(tree[0].textPreview); + }, "stored text_preview for a tool_use turn must always be valid, parseable JSON"); + assert.equal(parsed?.path, "/tmp/big.md"); + assert.ok(typeof parsed?.content === "string" && parsed.content.length > 0); + // The oversized string value was capped, not the whole serialized blob — + // real newlines inside it must survive (they did before truncation too). + assert.ok(parsed!.content!.includes("\n")); +}); + +test("resolveConversationId: a tool_use turn already within the preview bound is stored untouched", async () => { + const args = JSON.stringify({ command: "ls -la" }); + const turn = await resolveConversationId({ + body: { + model: "big-pickle", + input: [{ type: "function_call", name: "exec", call_id: "c1", arguments: args }], + }, + model: "big-pickle", + apiKeyId: "key-small-tool-call", + clientSessionIdHeader: null, + correlationId: nextCorrelationId(), + }); + + const tree = getConversationTurnTree(turn.conversationId); + assert.equal(tree[0].textPreview, args); +});