diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index 19b2a3e9c4..081460c833 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -364,7 +364,7 @@ "open-sse/services/combo.ts": 3648, "open-sse/services/compression/strategySelector.ts": 1060, "open-sse/services/rateLimitManager.ts": 1167, - "open-sse/translator/response/openai-responses.ts": 1215, + "open-sse/translator/response/openai-responses.ts": 1224, "open-sse/utils/cursorAgentProtobuf.ts": 1505, "open-sse/utils/stream.ts": 2889, "src/app/(dashboard)/dashboard/HomePageClient.tsx": 1388, @@ -401,7 +401,7 @@ "src/shared/components/RequestLoggerV2.tsx": 1629, "src/shared/components/analytics/charts.tsx": 1035, "src/shared/services/cliRuntime.ts": 1122, - "src/sse/handlers/chat.ts": 1918, + "src/sse/handlers/chat.ts": 1904, "src/sse/services/auth.ts": 2520, "tests/unit/account-fallback-service.test.ts": 1572, "tests/unit/provider-validation-specialty.test.ts": 2985, @@ -561,5 +561,6 @@ "open-sse/translator/request/openai-to-kiro.ts": "1057", "open-sse/utils/sseHeartbeat.ts": "142", "_rebaseline_2026_08_04_9305_sse_comments": "#9305 fix: broadened sseCommentsEnabled()", - "_rebaseline_2026_08_09_v3850_release_close": "Release v3.8.50 close reconciliation on e0ce95c592: src/sse/handlers/chat.ts 1904->1918 is the irreducible request-pipeline wiring from #9759 that invokes the Modality Bridge guardrail without moving its implementation into the handler; covered by the 17 Vision Bridge canaries plus the PR-1 focused suite. open-sse/translator/response/openai-responses.ts 1204->1215 is #9168's Responses tool-call argument delta buffering/normalization at the existing translator state-machine chokepoint; covered by its dedicated translator regression tests. Both values are measured by check:file-size (split-newline semantics), and the gate remains frozen at the new exact sizes." + "_rebaseline_2026_08_09_v3850_release_close": "Release v3.8.50 close reconciliation on e0ce95c592: src/sse/handlers/chat.ts 1904->1918 is the irreducible request-pipeline wiring from #9759 that invokes the Modality Bridge guardrail without moving its implementation into the handler; covered by the 17 Vision Bridge canaries plus the PR-1 focused suite. open-sse/translator/response/openai-responses.ts 1204->1215 is #9168's Responses tool-call argument delta buffering/normalization at the existing translator state-machine chokepoint; covered by its dedicated translator regression tests. Both values are measured by check:file-size (split-newline semantics), and the gate remains frozen at the new exact sizes.", + "_rebaseline_2026_08_08_toolcall_message_index_collision": "fix(responses-api): tool call after a text message collided on the same output_index. own growth: open-sse/translator/response/openai-responses.ts 1204->1224 (+20, extracted toolCallOutputIndexBase() shared helper so emitToolCall/closeToolCall can no longer compute a tool call's output_index independently and collide with a text message emitted in the same turn). Live incident (2026-08-08, OpenClaw agent): a client that tracks response items by output_index saw the tool call's added/delta/done events land on an index it had already marked complete (the just-closed text message), and silently dropped them — the agent spoke its preamble and never executed the tool call, even though OmniRoute's own recorded responseBody had a complete, valid tool_calls entry. Covered by the new regression test in tests/unit/translator-resp-openai-responses.test.ts reproducing the exact live scenario." } diff --git a/open-sse/translator/response/openai-responses.ts b/open-sse/translator/response/openai-responses.ts index d459350d35..35571814e7 100644 --- a/open-sse/translator/response/openai-responses.ts +++ b/open-sse/translator/response/openai-responses.ts @@ -451,11 +451,22 @@ function closeMessage(state, emit, idx) { } } +// Tool calls sit after reasoning (if any) AND after a text message (if one was +// actually emitted this turn) — a model commonly emits a short preamble before +// calling a tool (e.g. "Kör nu, på riktigt — apply_patch..."), and that message +// claims the same reasoningIndex+1 slot the old per-call math (`reasoningIndex +// + 1 + tcIdx`) assumed was free for tcIdx=0. Not accounting for the message +// item collided the tool call's added/delta/done events onto the same +// output_index as the just-closed message, which a client keying per-item +// state by output_index can silently drop (live incident 2026-08-08). +function toolCallOutputIndexBase(state) { + const msgIdx = state.reasoningId ? normalizeOutputIndex(state.reasoningIndex) + 1 : 0; + return state.msgItemAdded[msgIdx] ? msgIdx + 1 : msgIdx; +} + function emitToolCall(state, emit, tc) { const tcIdx = tc.index ?? 0; - const outputIndex = state.reasoningId - ? normalizeOutputIndex(state.reasoningIndex) + 1 + normalizeOutputIndex(tcIdx) - : normalizeOutputIndex(tcIdx); + const outputIndex = toolCallOutputIndexBase(state) + normalizeOutputIndex(tcIdx); const newCallId = tc.id; const funcName = tc.function?.name; @@ -536,9 +547,7 @@ function emitToolCall(state, emit, tc) { function closeToolCall(state, emit, idx, recordAsCompleted = true) { const callId = state.funcCallIds[idx]; if (callId && !state.funcItemDone[idx]) { - const normalizedIndex = state.reasoningId - ? normalizeOutputIndex(state.reasoningIndex) + 1 + normalizeOutputIndex(idx) - : normalizeOutputIndex(idx); + const normalizedIndex = toolCallOutputIndexBase(state) + normalizeOutputIndex(idx); const args = state.funcArgsBuf[idx] || "{}"; const toolName = state.funcNames[idx] || ""; const isCustomTool = diff --git a/tests/unit/translator-resp-openai-responses.test.ts b/tests/unit/translator-resp-openai-responses.test.ts index 3eeef460b8..6a2aee49ae 100644 --- a/tests/unit/translator-resp-openai-responses.test.ts +++ b/tests/unit/translator-resp-openai-responses.test.ts @@ -726,3 +726,92 @@ test("OpenAI -> Responses: parallel tool calls with mixed content survive transl const outputFcs = completed.data.response.output.filter((item) => item.type === "function_call"); assert.equal(outputFcs.length, 2, "completed output should have both function_calls"); }); + +// Live incident (2026-08-08): an OpenClaw agent ("Ping") sent a preamble line +// ("Kör nu, på riktigt — apply_patch på vibe-scriptet:") followed by an +// apply_patch tool call in the same turn, with reasoning ahead of both. The +// text message and the tool call both computed to output_index=1 — the tool +// call's own index math (`reasoningIndex + 1 + tcIdx`) never accounted for +// the message item also claiming `reasoningIndex + 1`, so a completed +// message and a freshly-added tool call collided on the same output_index. +// A client that tracks response items by output_index (as Responses-API +// clients are expected to) sees the tool call's added/delta/done events land +// on an index it already marked complete, and can silently drop or ignore +// them — exactly the observed symptom: the agent spoke the preamble and +// never executed the patch. +test("OpenAI -> Responses: a text message and a following tool call in the same turn get distinct output_index values", () => { + const events = collectEvents([ + { + id: "chatcmpl-1", + model: "big-pickle", + choices: [ + { index: 0, delta: { reasoning_content: "thinking about the patch" }, finish_reason: null }, + ], + }, + { + id: "chatcmpl-1", + model: "big-pickle", + choices: [ + { + index: 0, + delta: { content: "Kör nu, på riktigt — apply_patch på vibe-scriptet:" }, + finish_reason: null, + }, + ], + }, + { + id: "chatcmpl-1", + model: "big-pickle", + choices: [ + { + index: 0, + delta: { + tool_calls: [ + { + index: 0, + id: "call_apply_patch", + type: "function", + function: { name: "apply_patch", arguments: '{"input":"*** Begin Patch ***"}' }, + }, + ], + }, + finish_reason: "tool_calls", + }, + ], + }, + null, + ]); + + const itemDoneEvents = events.filter((e) => e.event === "response.output_item.done"); + const messageDone = itemDoneEvents.find((e) => e.data.item?.type === "message"); + const toolCallDone = itemDoneEvents.find( + (e) => e.data.item?.type === "function_call" || e.data.item?.type === "custom_tool_call" + ); + assert.ok(messageDone, "message output_item.done should be present"); + assert.ok(toolCallDone, "tool call output_item.done should be present"); + assert.notEqual( + messageDone.data.output_index, + toolCallDone.data.output_index, + "message and tool call must not collide on the same output_index" + ); + + // The tool call's own added/delta events (what a streaming client actually + // keys its per-item state on) must also use the tool call's real index, + // not the message's. + const toolCallAdded = events.find( + (e) => + e.event === "response.output_item.added" && + (e.data.item?.type === "function_call" || e.data.item?.type === "custom_tool_call") + ); + assert.ok(toolCallAdded, "tool call output_item.added should be present"); + assert.equal(toolCallAdded.data.output_index, toolCallDone.data.output_index); + assert.notEqual(toolCallAdded.data.output_index, messageDone.data.output_index); + + const completed = events.find((e) => e.event === "response.completed"); + const outputTypes = completed.data.response.output.map((item) => item.type); + assert.ok(outputTypes.includes("message"), "completed output must include the message"); + assert.ok( + outputTypes.includes("function_call") || outputTypes.includes("custom_tool_call"), + "completed output must include the tool call" + ); +});