Skip to content

Commit 008f015

Browse files
fix(ai-client): prevent drainPostStreamActions re-entrancy race condition (#429)
* fix(ai-client): prevent drainPostStreamActions re-entrancy stealing queued actions When multiple client tools complete in the same round, each addToolResult() queues a checkForContinuation action. The first drain executes one action which calls streamResponse(), whose finally block calls drainPostStreamActions() again (nested). The inner drain steals the remaining actions, permanently stalling the conversation. Add a draining flag to skip nested drain calls. The outer drain processes all actions sequentially, preventing action theft. Also fix shouldAutoSend() to require at least one tool call in the last assistant message. Previously it returned true for text-only responses (areAllToolsComplete() returns true when toolParts.length === 0), causing the second queued checkForContinuation action to incorrectly trigger an extra continuation round and produce duplicate content. Fixes #302 * ci: apply automated fixes * changeset: fix drain post-stream re-entrancy * fix: resolve type errors in drain re-entrancy test * test: add e2e regression test for drain re-entrancy stall (#302) Add a Playwright e2e test that verifies parallel client tools complete and the continuation fires with a follow-up text response. Without the drainPostStreamActions() re-entrancy guard, nested drain calls steal queued actions and permanently stall the conversation after both tools complete. The test asserts that the follow-up text "All displayed" arrives, which would time out without the fix. * ci: apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent dc71c72 commit 008f015

4 files changed

Lines changed: 204 additions & 5 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@tanstack/ai-client': patch
3+
---
4+
5+
fix(ai-client): prevent drainPostStreamActions re-entrancy stealing queued actions
6+
7+
When multiple client tools complete in the same round, nested `drainPostStreamActions()` calls from `streamResponse()`'s `finally` block could steal queued actions, permanently stalling the conversation. Added a re-entrancy guard and a `shouldAutoSend()` check requiring tool-call parts before triggering continuation.

packages/typescript/ai-client/src/chat-client.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export class ChatClient {
5454
// Tracks whether a queued checkForContinuation was skipped because
5555
// continuationPending was true (chained approval scenario)
5656
private continuationSkipped = false
57+
private draining = false
5758
private sessionGenerating = false
5859
private activeRunIds = new Set<string>()
5960

@@ -847,9 +848,15 @@ export class ChatClient {
847848
* Drain and execute all queued post-stream actions
848849
*/
849850
private async drainPostStreamActions(): Promise<void> {
850-
while (this.postStreamActions.length > 0) {
851-
const action = this.postStreamActions.shift()!
852-
await action()
851+
if (this.draining) return
852+
this.draining = true
853+
try {
854+
while (this.postStreamActions.length > 0) {
855+
const action = this.postStreamActions.shift()!
856+
await action()
857+
}
858+
} finally {
859+
this.draining = false
853860
}
854861
}
855862

@@ -885,9 +892,16 @@ export class ChatClient {
885892
}
886893

887894
/**
888-
* Check if all tool calls are complete and we should auto-send
895+
* Check if all tool calls are complete and we should auto-send.
896+
* Requires that there is at least one tool call in the last assistant message;
897+
* a text-only response has nothing to auto-send.
889898
*/
890899
private shouldAutoSend(): boolean {
900+
const messages = this.processor.getMessages()
901+
const lastAssistant = messages.findLast((m) => m.role === 'assistant')
902+
if (!lastAssistant) return false
903+
const hasToolCalls = lastAssistant.parts.some((p) => p.type === 'tool-call')
904+
if (!hasToolCalls) return false
891905
return this.processor.areAllToolsComplete()
892906
}
893907

packages/typescript/ai-client/tests/chat-client.test.ts

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import {
88
createApprovalToolCallChunks,
99
createCustomEventChunks,
1010
} from './test-utils'
11-
import type { ConnectionAdapter } from '../src/connection-adapters'
11+
import type {
12+
ConnectionAdapter,
13+
ConnectConnectionAdapter,
14+
} from '../src/connection-adapters'
1215
import type { StreamChunk } from '@tanstack/ai'
1316
import type { UIMessage } from '../src/types'
1417

@@ -1239,6 +1242,70 @@ describe('ChatClient', () => {
12391242
})
12401243
})
12411244

1245+
describe('drain re-entrancy guard (fix #302)', () => {
1246+
it('should continue after multiple client tools complete in the same round', async () => {
1247+
// Round 1: two simultaneous tool calls (triggers the re-entrancy bug)
1248+
const round1Chunks = createToolCallChunks([
1249+
{ id: 'tc-1', name: 'tool_one', arguments: '{}' },
1250+
{ id: 'tc-2', name: 'tool_two', arguments: '{}' },
1251+
])
1252+
// Round 2: final text response
1253+
const round2Chunks = createTextChunks('Done!', 'msg-2')
1254+
1255+
let callIndex = 0
1256+
const adapter: ConnectConnectionAdapter = {
1257+
async *connect(_messages, _data, abortSignal) {
1258+
callIndex++
1259+
const chunks = callIndex === 1 ? round1Chunks : round2Chunks
1260+
for (const chunk of chunks) {
1261+
if (abortSignal?.aborted) return
1262+
yield chunk
1263+
}
1264+
},
1265+
}
1266+
1267+
// Both tools execute immediately (synchronously resolve)
1268+
const client = new ChatClient({
1269+
connection: adapter,
1270+
tools: [
1271+
{
1272+
__toolSide: 'client' as const,
1273+
name: 'tool_one',
1274+
description: 'Tool one',
1275+
execute: async () => ({ result: 'one' }),
1276+
},
1277+
{
1278+
__toolSide: 'client' as const,
1279+
name: 'tool_two',
1280+
description: 'Tool two',
1281+
execute: async () => ({ result: 'two' }),
1282+
},
1283+
],
1284+
})
1285+
1286+
// Send initial message — triggers round 1 (two tool calls, both auto-executed)
1287+
await client.sendMessage('Run both tools')
1288+
1289+
// Wait for loading to stop and the continuation (round 2) to complete
1290+
await vi.waitFor(
1291+
() => {
1292+
expect(client.getIsLoading()).toBe(false)
1293+
// Ensure round 2 actually fired
1294+
expect(callIndex).toBeGreaterThanOrEqual(2)
1295+
},
1296+
{ timeout: 2000 },
1297+
)
1298+
1299+
// The final response "Done!" should appear in messages
1300+
const messages = client.getMessages()
1301+
const lastAssistant = [...messages]
1302+
.reverse()
1303+
.find((m) => m.role === 'assistant')
1304+
const textPart = lastAssistant?.parts.find((p) => p.type === 'text')
1305+
expect(textPart?.content).toBe('Done!')
1306+
})
1307+
})
1308+
12421309
describe('error handling', () => {
12431310
it('should set error state on connection failure', async () => {
12441311
const error = new Error('Network error')
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { test, expect } from '../fixtures'
2+
import {
3+
selectScenario,
4+
runTest,
5+
waitForTestComplete,
6+
getMetadata,
7+
getEventLog,
8+
getToolCalls,
9+
} from './helpers'
10+
11+
/**
12+
* Drain Re-Entrancy Guard E2E Tests
13+
*
14+
* Regression test for GitHub issue #302 / PR #429.
15+
*
16+
* When multiple client tools complete in the same round, each addToolResult()
17+
* queues a checkForContinuation action. Without a re-entrancy guard on
18+
* drainPostStreamActions(), the first action's streamResponse() → finally →
19+
* drainPostStreamActions() (nested) steals the remaining actions from the
20+
* queue, permanently stalling the conversation. The user sees tool results
21+
* but the model never produces its follow-up text response.
22+
*
23+
* The fix adds a re-entrancy guard to drainPostStreamActions() and a
24+
* shouldAutoSend() check requiring tool-call parts before triggering
25+
* continuation.
26+
*
27+
* This test uses the parallel-client-tools scenario (2 client tools in the
28+
* same turn) and verifies not just that both tools execute, but critically
29+
* that the **continuation fires and the follow-up text response arrives**.
30+
* Without the fix, the test would time out waiting for the follow-up text.
31+
*/
32+
33+
test.describe('Drain Re-Entrancy Guard (Regression #302)', () => {
34+
test('parallel client tools complete and continuation fires with follow-up text', async ({
35+
page,
36+
testId,
37+
aimockPort,
38+
}) => {
39+
await selectScenario(page, 'parallel-client-tools', testId, aimockPort)
40+
await runTest(page)
41+
42+
// Wait for the test to fully complete — this includes the continuation
43+
// round producing the follow-up text. Without the fix, this would
44+
// time out because the continuation never fires.
45+
await waitForTestComplete(page, 20000, 2)
46+
47+
// Verify both client tools executed
48+
const metadata = await getMetadata(page)
49+
expect(metadata.testComplete).toBe('true')
50+
expect(metadata.isLoading).toBe('false')
51+
52+
const events = await getEventLog(page)
53+
const toolNames = [...new Set(events.map((e) => e.toolName))]
54+
expect(toolNames).toContain('show_notification')
55+
expect(toolNames).toContain('display_chart')
56+
57+
// Verify both tools reached execution-complete state
58+
const completionEvents = events.filter(
59+
(e) => e.type === 'execution-complete',
60+
)
61+
expect(completionEvents.length).toBe(2)
62+
63+
// CRITICAL ASSERTION: Verify the follow-up text from round 2 was received.
64+
// Without the re-entrancy fix, the conversation stalls after both tools
65+
// complete — the continuation request is never sent, so this text never
66+
// arrives.
67+
const messages = await page.evaluate(() => {
68+
const el = document.getElementById('messages-json-content')
69+
if (!el) return []
70+
try {
71+
return JSON.parse(el.textContent || '[]')
72+
} catch {
73+
return []
74+
}
75+
})
76+
77+
const assistantMessages = messages.filter(
78+
(m: any) => m.role === 'assistant',
79+
)
80+
81+
// There should be at least 2 assistant messages:
82+
// 1. The tool-call round (with both tool calls + results)
83+
// 2. The continuation round (with the follow-up text)
84+
expect(assistantMessages.length).toBeGreaterThanOrEqual(2)
85+
86+
// The follow-up text from the continuation round should be present
87+
const allTextParts = assistantMessages.flatMap((m: any) =>
88+
m.parts.filter((p: any) => p.type === 'text').map((p: any) => p.content),
89+
)
90+
const allText = allTextParts.join(' ')
91+
expect(allText).toContain('All displayed')
92+
})
93+
94+
test.afterEach(async ({ page }, testInfo) => {
95+
if (testInfo.status !== testInfo.expectedStatus) {
96+
await page.screenshot({
97+
path: `test-results/drain-reentrance-failure-${testInfo.title.replace(/\s+/g, '-')}.png`,
98+
fullPage: true,
99+
})
100+
101+
const toolCalls = await getToolCalls(page)
102+
const metadata = await getMetadata(page)
103+
const events = await getEventLog(page)
104+
105+
console.log('Test failed. Debug info:')
106+
console.log('Metadata:', metadata)
107+
console.log('Tool calls:', toolCalls)
108+
console.log('Events:', events)
109+
}
110+
})
111+
})

0 commit comments

Comments
 (0)