mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-12 02:02:13 +03:00
fix(responses-api): tool call after a text message collided on the same output_index (#9843)
Live incident (2026-08-08): an OpenClaw agent sent a short preamble line
("Kör nu, på riktigt — apply_patch på vibe-scriptet:") followed by an
apply_patch tool call in the same turn. The client only spoke the preamble
and never executed the patch, even though OmniRoute's own recorded
responseBody had a complete, valid tool_calls entry.
Root cause: emitToolCall/closeToolCall computed a tool call's output_index
as `reasoningIndex + 1 + tcIdx`, assuming reasoningIndex + 1 was free for
the first tool call (tcIdx=0). But a text message emitted in the same turn
ALSO claims reasoningIndex + 1 (or index 0 with no reasoning) — so a
turn with reasoning + text content + a tool call collided the tool call's
added/delta/done events onto the same output_index as the just-closed
message. A client that tracks response items by output_index (as expected
for the Responses API) sees the tool call events land on an index it
already marked complete and can silently drop them.
Fix: track whether a message item was actually emitted at that index
(state.msgItemAdded) and, if so, tool calls start one slot after it.
Extracted a shared toolCallOutputIndexBase() helper so emitToolCall and
closeToolCall can no longer compute this independently and drift apart.
Confirmed via the live call log artifact (id 1786223153235-770a1c):
response.output_item.done for the text message and response.output_item.added
for the tool call both carried output_index=1 in the raw SSE stream, 1.84s
apart, exactly matching the reported symptom.
Co-authored-by: Markus Hartung <mail@hartmark.se>
This commit is contained in:
committed by
GitHub
parent
0f5699165b
commit
05940f4c7f
@@ -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."
|
||||
}
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -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"
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user