Prechádzať zdrojové kódy

fix(agent-loop): log tool/result under the originating call.id (P1-7)

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.
Tianyi Cui 3 mesiacov pred
rodič
commit
37576ade6a

+ 7 - 1
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 } : {},

+ 48 - 0
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'))
+    }
+  })
+})