From a5938f4b0efe755d61f84e05ced7993917a64dfd Mon Sep 17 00:00:00 2001 From: 21hbguo <139569698+21hbguo@users.noreply.github.com> Date: Tue, 15 Sep 2026 23:41:57 +0800 Subject: [PATCH 1/2] fix(tools): absorb stray assistant text inside parallel tool-call runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex emits parallel function_calls as separate items, then an assistant text item, then the results — [tc, tc, text, tool, tool]. The interleave run stopped at the text-only assistant, so no interleave applied and the wire carried three consecutive ASSISTANT messages (call, call, text). The upstream validator rejects same-source runs >=3 with invalid_argument ("an internal error occurred"), which surfaced as deterministic response.failed on the identical retried payload. Fold the stray prose into the first call-bearing assistant of the run so the sequence stays interleaved call/result (verified on the wire: max same-source run is now 2), and emit the whole run verbatim when no results match so nothing is dropped. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/handlers/tool-emulation.js | 100 ++++++++++++++++++++------------- test/tool-emulation.test.js | 49 ++++++++++++++++ 2 files changed, 111 insertions(+), 38 deletions(-) diff --git a/src/handlers/tool-emulation.js b/src/handlers/tool-emulation.js index 98dc28a7..8027e54d 100644 --- a/src/handlers/tool-emulation.js +++ b/src/handlers/tool-emulation.js @@ -1148,56 +1148,80 @@ export function interleaveParallelToolMessages(messages) { let i = 0; while (i < messages.length) { const m = messages[i]; - if (m?.role === 'assistant' && Array.isArray(m.tool_calls) && m.tool_calls.length > 1) { - let j = i + 1; - const toolMsgs = []; - while (j < messages.length && messages[j]?.role === 'tool') { - toolMsgs.push(messages[j]); + if (m?.role === 'assistant' && Array.isArray(m.tool_calls) && m.tool_calls.length) { + let j = i; + const assistantMsgs = []; + const strayTexts = []; + while (j < messages.length && messages[j]?.role === 'assistant') { + if (Array.isArray(messages[j].tool_calls) && messages[j].tool_calls.length) { + assistantMsgs.push(messages[j]); + } else { + strayTexts.push(messages[j]); + } j++; } - let hasMatches = false; - const usedIndices = new Set(); - for (const tc of m.tool_calls) { - const tcid = String(tc?.id ?? ''); - if (tcid && toolMsgs.some((tm) => String(tm?.tool_call_id ?? '') === tcid)) { - hasMatches = true; - break; - } + let k = j; + const toolMsgs = []; + while (k < messages.length && messages[k]?.role === 'tool') { + toolMsgs.push(messages[k]); + k++; } - if (hasMatches) { - let first = true; - for (const tc of m.tool_calls) { - const singleAssistant = { - ...m, - content: first ? (m.content || null) : null, - tool_calls: [tc], - }; - if (!first) { - delete singleAssistant.reasoning_content; - delete singleAssistant.reasoning; + const toolCalls = assistantMsgs.flatMap((assistant) => assistant.tool_calls); + const hasMatches = toolCalls.some((tc) => + toolMsgs.some((tm) => String(tm?.tool_call_id ?? '') === String(tc?.id ?? '')), + ); + + if (toolCalls.length > 1 && hasMatches) { + if (strayTexts.length) { + const strayText = strayTexts + .map((s) => (s?.content == null ? '' : contentTextForPreambleCheck(s.content))) + .filter(Boolean) + .join('\n\n'); + if (strayText) { + const first = assistantMsgs[0]; + const cur = first.content == null ? '' : contentTextForPreambleCheck(first.content); + assistantMsgs[0] = { ...first, content: cur ? `${cur}\n\n${strayText}` : strayText }; } - out.push(singleAssistant); - - const tcid = String(tc?.id ?? ''); - const matchIdx = toolMsgs.findIndex( - (tm, idx) => !usedIndices.has(idx) && String(tm?.tool_call_id ?? '') === tcid, - ); - if (matchIdx !== -1) { - usedIndices.add(matchIdx); - out.push(toolMsgs[matchIdx]); + } + const usedIndices = new Set(); + for (const assistant of assistantMsgs) { + let first = true; + for (const tc of assistant.tool_calls) { + const singleAssistant = { + ...assistant, + content: first ? (assistant.content || null) : null, + tool_calls: [tc], + }; + if (!first) { + delete singleAssistant.reasoning_content; + delete singleAssistant.reasoning; + } + out.push(singleAssistant); + + const tcid = String(tc?.id ?? ''); + const matchIdx = toolMsgs.findIndex( + (tm, idx) => !usedIndices.has(idx) && String(tm?.tool_call_id ?? '') === tcid, + ); + if (matchIdx !== -1) { + usedIndices.add(matchIdx); + out.push(toolMsgs[matchIdx]); + } + first = false; } - first = false; } for (let idx = 0; idx < toolMsgs.length; idx++) { - if (!usedIndices.has(idx)) { - out.push(toolMsgs[idx]); - } + if (!usedIndices.has(idx)) out.push(toolMsgs[idx]); } - i = j; + i = k; continue; } + // No interleave applied: emit the whole assistant run verbatim (calls and + // stray text alike) so nothing is silently dropped. + for (let x = i; x < j; x++) out.push(messages[x]); + i = j; + continue; } out.push(m); i++; diff --git a/test/tool-emulation.test.js b/test/tool-emulation.test.js index e73ac367..1d57561e 100644 --- a/test/tool-emulation.test.js +++ b/test/tool-emulation.test.js @@ -991,6 +991,21 @@ describe('interleaveParallelToolMessages', () => { assert.equal(out[5].content, 'next'); }); + it('interleaves consecutive single-tool assistant turns from Responses clients', () => { + const messages = [ + { role: 'assistant', content: '', tool_calls: [{ id: 'c1', type: 'function', function: { name: 'f1', arguments: '{}' } }] }, + { role: 'assistant', tool_calls: [{ id: 'c2', type: 'function', function: { name: 'f2', arguments: '{}' } }] }, + { role: 'assistant', tool_calls: [{ id: 'c3', type: 'function', function: { name: 'f3', arguments: '{}' } }] }, + { role: 'tool', tool_call_id: 'c1', content: 'r1' }, + { role: 'tool', tool_call_id: 'c2', content: 'r2' }, + { role: 'tool', tool_call_id: 'c3', content: 'r3' }, + ]; + + const out = interleaveParallelToolMessages(messages); + assert.deepEqual(out.map((m) => m.role), ['assistant', 'tool', 'assistant', 'tool', 'assistant', 'tool']); + assert.deepEqual(out.map((m) => m.tool_calls?.[0]?.id || m.tool_call_id), ['c1', 'c1', 'c2', 'c2', 'c3', 'c3']); + }); + it('preserves unmatched tool results and non-parallel messages intact', () => { const messages = [ { @@ -1024,6 +1039,40 @@ describe('interleaveParallelToolMessages', () => { ]; assert.deepEqual(interleaveParallelToolMessages(pending), pending); }); + + it('absorbs a text-only assistant inside a parallel-call run instead of breaking it', () => { + // Codex emits parallel function_calls as separate items, then an assistant + // text item, then the results — i.e. [tc, tc, text, tool, tool]. Letting + // that through verbatim encodes as three consecutive ASSISTANT wire + // messages (call, call, text), which the upstream validator rejects with + // invalid_argument ("an internal error occurred"). The stray text belongs + // to the same turn, so fold it into the first call's assistant entry. + const messages = [ + { role: 'user', content: 'list resources' }, + { role: 'assistant', tool_calls: [{ id: 'a:0#x1', type: 'function', function: { name: 'list_mcp_resources', arguments: '{"server":"a"}' } }] }, + { role: 'assistant', tool_calls: [{ id: 'a:1#x2', type: 'function', function: { name: 'list_mcp_resources', arguments: '{"server":"b"}' } }] }, + { role: 'assistant', content: 'checking both servers' }, + { role: 'tool', tool_call_id: 'a:0#x1', content: 'err1' }, + { role: 'tool', tool_call_id: 'a:1#x2', content: 'ok1' }, + { role: 'user', content: 'go on' }, + ]; + + const out = interleaveParallelToolMessages(messages); + assert.deepEqual(out.map((m) => m.role), ['user', 'assistant', 'tool', 'assistant', 'tool', 'user']); + assert.equal(out[1].content, 'checking both servers', 'stray text folded into first call turn'); + assert.deepEqual(out[1].tool_calls.map((t) => t.id), ['a:0#x1']); + assert.deepEqual(out[3].tool_calls.map((t) => t.id), ['a:1#x2']); + }); + + it('preserves the full run verbatim when no tool results match', () => { + const messages = [ + { role: 'assistant', tool_calls: [{ id: 'c1', type: 'function', function: { name: 'f1', arguments: '{}' } }] }, + { role: 'assistant', tool_calls: [{ id: 'c2', type: 'function', function: { name: 'f2', arguments: '{}' } }] }, + { role: 'assistant', content: 'no results yet' }, + { role: 'user', content: 'next' }, + ]; + assert.deepEqual(interleaveParallelToolMessages(messages), messages); + }); }); // ─── The call site: normalizeMessagesForCascade must interleave on the native From bac13fe3cd6642ef20f2bdf38b907a32b8b13df2 Mon Sep 17 00:00:00 2001 From: 21hbguo <139569698+21hbguo@users.noreply.github.com> Date: Wed, 16 Sep 2026 22:54:00 +0800 Subject: [PATCH 2/2] fix(tools): guard stray-fold field loss and empty-id pairing in interleave MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the parallel tool-call interleave: - Array content on the first call-bearing assistant now gets a text part appended instead of being flattened to a string — non-text parts (image_url ...) encode as distinct wire types and were silently dropped before reaching extractInlineImages. Same rule isMergeableText already enforces on the upstream encoder. - An empty/missing tool_call_id can no longer pair with an empty result id: the `tcid &&` gate is restored in both the hasMatches entry check and the per-call findIndex consume. Two empty strings comparing equal is not evidence of call ownership — native encoding assigns a fresh UUID to a missing call id while an empty result id never reaches the role=4 branch. - Stray assistant entries carrying unmergeable own fields (reasoning_content/signature feeding native #11/#12 + sealed blob, name/annotations/refusal/audio) or non-text content parts are no longer folded — they are emitted verbatim after the interleaved pairs. Tradeoff, declared: a run of >=3 such strays can still chain same-source (pre-fix behaviour for that shape); the common Codex shape of one reasoning+text item per turn keeps the alternating 2,4,...,2 wire sequence with every field intact. Generated with [Devin](https://devin.ai) --- src/handlers/tool-emulation.js | 64 ++++++++++++++++++++++++--- test/tool-emulation.test.js | 79 ++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 7 deletions(-) diff --git a/src/handlers/tool-emulation.js b/src/handlers/tool-emulation.js index 8027e54d..c3574ae2 100644 --- a/src/handlers/tool-emulation.js +++ b/src/handlers/tool-emulation.js @@ -1142,6 +1142,28 @@ export function stripOrphanedToolResults(messages) { return dropped ? out : messages; } +// Fold-safety for stray (text-only) assistant entries inside a parallel-call +// run. Folding keeps only `content`, so a stray carrying any other own field +// would silently lose it — reasoning_content/signature feed the native #11/#12 +// reasoning + sealed-blob frames (responses.js attaches them for Codex's +// reasoning items), and name/annotations/refusal/audio have no generic merge +// semantics either. Likewise an array content folds losslessly only when every +// part is text — non-text parts encode as distinct wire types and must survive +// as objects (the same rule isMergeableText enforces upstream in +// devin-connect.js). Anything outside this envelope is emitted verbatim after +// the interleaved block instead of being folded. +const STRAY_FOLDABLE_KEYS = new Set(['role', 'content', 'tool_calls']); +function isStrayMergeable(msg) { + if (!msg || typeof msg !== 'object') return false; + for (const key of Object.keys(msg)) { + if (!STRAY_FOLDABLE_KEYS.has(key)) return false; + } + if (msg.tool_calls != null && (!Array.isArray(msg.tool_calls) || msg.tool_calls.length)) { + return false; + } + return !Array.isArray(msg.content) || msg.content.every((part) => part?.type === 'text'); +} + export function interleaveParallelToolMessages(messages) { if (!Array.isArray(messages)) return messages; const out = []; @@ -1169,20 +1191,42 @@ export function interleaveParallelToolMessages(messages) { } const toolCalls = assistantMsgs.flatMap((assistant) => assistant.tool_calls); - const hasMatches = toolCalls.some((tc) => - toolMsgs.some((tm) => String(tm?.tool_call_id ?? '') === String(tc?.id ?? '')), - ); + // An empty/missing call id must never pair: on the native path a missing + // call id gets a fresh UUID while an empty result id never reaches the + // role=4 branch — two empty strings comparing equal is not evidence of + // call ownership. Keep the `tcid &&` gate in the entry check here and in + // the per-call consume below. + const hasMatches = toolCalls.some((tc) => { + const tcid = String(tc?.id ?? ''); + return tcid && toolMsgs.some((tm) => String(tm?.tool_call_id ?? '') === tcid); + }); if (toolCalls.length > 1 && hasMatches) { + const verbatimStrays = []; if (strayTexts.length) { - const strayText = strayTexts + const foldableStrays = []; + for (const s of strayTexts) { + (isStrayMergeable(s) ? foldableStrays : verbatimStrays).push(s); + } + const strayText = foldableStrays .map((s) => (s?.content == null ? '' : contentTextForPreambleCheck(s.content))) .filter(Boolean) .join('\n\n'); if (strayText) { const first = assistantMsgs[0]; - const cur = first.content == null ? '' : contentTextForPreambleCheck(first.content); - assistantMsgs[0] = { ...first, content: cur ? `${cur}\n\n${strayText}` : strayText }; + if (Array.isArray(first.content)) { + // Append a text part — never flatten the array into a string: + // non-text parts (image_url …) encode as different wire types + // and extractInlineImages (Array.isArray gate) would lose them. + const hasTextPart = first.content.some((part) => part?.type === 'text'); + assistantMsgs[0] = { + ...first, + content: [...first.content, { type: 'text', text: `${hasTextPart ? '\n' : ''}${strayText}` }], + }; + } else { + const cur = first.content == null ? '' : contentTextForPreambleCheck(first.content); + assistantMsgs[0] = { ...first, content: cur ? `${cur}\n\n${strayText}` : strayText }; + } } } const usedIndices = new Set(); @@ -1202,7 +1246,7 @@ export function interleaveParallelToolMessages(messages) { const tcid = String(tc?.id ?? ''); const matchIdx = toolMsgs.findIndex( - (tm, idx) => !usedIndices.has(idx) && String(tm?.tool_call_id ?? '') === tcid, + (tm, idx) => tcid && !usedIndices.has(idx) && String(tm?.tool_call_id ?? '') === tcid, ); if (matchIdx !== -1) { usedIndices.add(matchIdx); @@ -1211,6 +1255,12 @@ export function interleaveParallelToolMessages(messages) { first = false; } } + // Unmergeable strays go out whole, after the interleaved pairs: appending + // them at the tail keeps the alternating source pattern (a same-source + // run of 3 needs ≥3 unmergeable strays — rare next to the single + // reasoning+text item Codex replays per turn), and placing them between + // pairs could split a call from its result. + for (const s of verbatimStrays) out.push(s); for (let idx = 0; idx < toolMsgs.length; idx++) { if (!usedIndices.has(idx)) out.push(toolMsgs[idx]); } diff --git a/test/tool-emulation.test.js b/test/tool-emulation.test.js index 1d57561e..1b53e8f3 100644 --- a/test/tool-emulation.test.js +++ b/test/tool-emulation.test.js @@ -1073,6 +1073,85 @@ describe('interleaveParallelToolMessages', () => { ]; assert.deepEqual(interleaveParallelToolMessages(messages), messages); }); + + it('keeps every original content part when absorbing stray text', () => { + const image = { type: 'image_url', image_url: { url: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==' } }; + const content = [{ type: 'text', text: 'see' }, image]; + const messages = [ + { role: 'assistant', content, tool_calls: [{ id: 'c1', type: 'function', function: { name: 'f', arguments: '{}' } }] }, + { role: 'assistant', tool_calls: [{ id: 'c2', type: 'function', function: { name: 'f', arguments: '{}' } }] }, + { role: 'assistant', content: 'stray note' }, + { role: 'tool', tool_call_id: 'c1', content: 'r1' }, + { role: 'tool', tool_call_id: 'c2', content: 'r2' }, + ]; + const out = interleaveParallelToolMessages(messages); + assert.equal(out[0].content[1], image, 'image part must survive as an object'); + assert.deepEqual(content, [{ type: 'text', text: 'see' }, image], 'caller array untouched'); + }); + + it('never pairs calls with results when the id is missing on both sides', () => { + // Two empty ids comparing equal is not evidence of call ownership: on the + // native path a missing call id gets a fresh UUID while an empty result id + // never reaches the role=4 branch. Pre-regression this shape interleaved; + // it must pass through verbatim. + const messages = [ + { + role: 'assistant', + tool_calls: [ + { type: 'function', function: { name: 'f', arguments: '{}' } }, + { type: 'function', function: { name: 'f', arguments: '{}' } }, + ], + }, + { role: 'tool', content: 'r1' }, + { role: 'tool', content: 'r2' }, + ]; + assert.deepEqual(interleaveParallelToolMessages(messages), messages); + }); + + it('emits strays carrying unmergeable fields verbatim instead of folding them', () => { + // Codex replays a reasoning item as an assistant entry carrying + // reasoning_content + signature (native #11/#12 reasoning + sealed blob). + // Folding keeps only `content`, so those fields would be silently dropped — + // the stray goes out whole, after the interleaved pairs, keeping the + // alternating call/result source pattern (2,4,2,4,2). + const stray = { + role: 'assistant', + content: 'checking both servers', + reasoning_content: 'thinking it through', + signature: 'sealed.v1.abc', + }; + const messages = [ + { role: 'assistant', tool_calls: [{ id: 'c1', type: 'function', function: { name: 'f', arguments: '{}' } }] }, + { role: 'assistant', tool_calls: [{ id: 'c2', type: 'function', function: { name: 'f', arguments: '{}' } }] }, + stray, + { role: 'tool', tool_call_id: 'c1', content: 'r1' }, + { role: 'tool', tool_call_id: 'c2', content: 'r2' }, + ]; + + const out = interleaveParallelToolMessages(messages); + assert.deepEqual(out.map((m) => m.role), ['assistant', 'tool', 'assistant', 'tool', 'assistant']); + assert.equal(out[4], stray, 'unmergeable stray emitted verbatim — same object, fields intact'); + assert.equal(out[0].content, null, 'stray text must NOT fold into the first call turn'); + assert.equal(out[0].signature, undefined, 'sealed signature must not leak onto the call turn'); + }); + + it('emits a stray whose content array carries non-text parts verbatim', () => { + const image = { type: 'image_url', image_url: { url: 'data:image/png;base64,AAAA' } }; + const strayContent = [{ type: 'text', text: 'look' }, image]; + const messages = [ + { role: 'assistant', tool_calls: [{ id: 'c1', type: 'function', function: { name: 'f', arguments: '{}' } }] }, + { role: 'assistant', tool_calls: [{ id: 'c2', type: 'function', function: { name: 'f', arguments: '{}' } }] }, + { role: 'assistant', content: strayContent }, + { role: 'tool', tool_call_id: 'c1', content: 'r1' }, + { role: 'tool', tool_call_id: 'c2', content: 'r2' }, + ]; + + const out = interleaveParallelToolMessages(messages); + const tail = out[out.length - 1]; + assert.equal(tail.role, 'assistant'); + assert.equal(tail.content[1], image, 'image part survives as an object on the verbatim stray'); + assert.deepEqual(strayContent, [{ type: 'text', text: 'look' }, image], 'caller array untouched'); + }); }); // ─── The call site: normalizeMessagesForCascade must interleave on the native