From 37576ade6a1ce28be72c5f79dd53c4e278812a3e Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Mon, 15 Jun 2026 00:10:19 +0800 Subject: [PATCH] fix(agent-loop): log tool/result under the originating call.id (P1-7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loop passed the authoritative call.id into ctx.tools.execute() but then appended tool/result using result.callId — the value a tools/execute waterfall listener returns — with no check. A listener returning a mismatched id silently recorded the result under the wrong call. callId is the model-transcript correlation id: deriveMessages() turns it into the tool-result block's toolCallId, which must pair with the assistant tool-call block; a wrong id orphans that pairing in the next model request. Append tool/result with callId: call.id (the loop's authoritative id). A listener-internal id, if ever worth keeping, belongs in a separate diagnostic field — never overloaded onto callId. Test: a tools/execute listener returns a wrong callId; assert the logged tool/result.callId equals call.id AND deriveMessages() yields a tool-result block whose toolCallId equals call.id (not the wrong returned id). Verified the test fails on the pre-fix result.callId behavior. --- packages/agent-loop/src/loop.ts | 8 +++- .../agent-loop/tests/review-fixes.spec.ts | 48 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/packages/agent-loop/src/loop.ts b/packages/agent-loop/src/loop.ts index 063ed21a9d..c556acfc20 100644 --- a/packages/agent-loop/src/loop.ts +++ b/packages/agent-loop/src/loop.ts @@ -420,7 +420,13 @@ async function runStep( }) session.append('tool/result', { turn, step, - callId: result.callId, + // The correlation id MUST be the loop's authoritative call.id (the + // model-transcript id that deriveMessages turns into toolCallId), NOT + // result.callId — a tools/execute waterfall listener returning a + // mismatched id would otherwise orphan the call↔result pairing in the + // next model request. A listener-internal id, if ever needed, belongs in + // a separate diagnostic field, never overloaded onto callId. + callId: call.id, content: result.content, isError: result.isError, ...result.error ? { error: result.error } : {}, diff --git a/packages/agent-loop/tests/review-fixes.spec.ts b/packages/agent-loop/tests/review-fixes.spec.ts index fb297307dd..71c4d7c649 100644 --- a/packages/agent-loop/tests/review-fixes.spec.ts +++ b/packages/agent-loop/tests/review-fixes.spec.ts @@ -887,3 +887,51 @@ describe('P1-5: a started turn (and any open step) is always closed on a boundar expect(boundaryCounts(agent).turnEnd).toBe(2) }) }) + +describe('P1-7: tool/result is logged under the originating call.id, not result.callId', () => { + it('a tools/execute listener returning a mismatched callId cannot orphan the call↔result pairing', async () => { + // Model emits a tool-call with id "c1", then a final text turn. + const adapter = new MockAdapter([ + toolCallResponse('c1', 'echo', { x: 1 }), + textResponse('done'), + ]) + const ctx = await harness(adapter) + ctx.tools.register(defineTool({ + name: 'echo', + description: 'echo', + parameters: { x: { type: 'number' } }, + async execute() { return [{ type: 'text', text: 'ok' }] }, + })) + + // A waterfall listener short-circuits with a result carrying the WRONG + // callId (a listener-internal/proxy id). The loop must still record the + // tool/result under the model's authoritative call.id. + ctx.on('tools/execute', (exec) => { + expect(exec.callId).toBe(CallId('c1')) // the loop passed the real id in + return Promise.resolve({ callId: CallId('wrong-proxy-id'), content: [{ type: 'text', text: 'ok' }], isError: false }) + }, { prepend: true }) + + const agent = ctx.agentLoop.create('a-callid', { model: 'mock' }) + send(agent, 'use tool') + await waitForIdle(ctx, agent) + + // The logged tool/result.callId is the originating call.id, NOT the + // listener's wrong id. + const resultEvent = [...agent.session.events].find(e => e.type === 'tool/result') + expect(resultEvent?.type).toBe('tool/result') + if (resultEvent?.type === 'tool/result') { + expect(resultEvent.data.callId).toBe(CallId('c1')) + } + + // And deriveMessages pairs the tool-result with the assistant tool-call: + // the derived tool-result block's toolCallId equals the original call.id. + const messages = agent.session.deriveMessages() + const toolResultBlock = messages + .flatMap(m => m.content) + .find(b => b.type === 'tool-result') + expect(toolResultBlock?.type).toBe('tool-result') + if (toolResultBlock?.type === 'tool-result') { + expect(toolResultBlock.toolCallId).toBe(CallId('c1')) + } + }) +})