Fix/tool race conditions #258
Conversation
- Add LLM simulator adapter for deterministic testing of tool flows - Add comprehensive unit tests for tool execution scenarios: - Server tool execution - Client tool execution - Approval workflows - Multi-tool and parallel tool execution - Tool sequences - Error handling - Add E2E test infrastructure for tool scenarios - Fix client tool race conditions by tracking pending tool executions - Prevent duplicate continuation attempts with continuationPending flag - Guard against concurrent stream processing in streamResponse - Add approval info to ToolCall type for server-side decision tracking - Include approval info in model message conversion for approval workflows - Check ModelMessage format for approval info extraction in chat activity This change improves the reliability of tool execution, especially for: - Client tools with async execute functions - Approval-based tool workflows - Sequential tool execution scenarios
📝 WalkthroughWalkthroughThis pull request addresses tool execution race conditions in the TypeScript AI client and server, introduces a comprehensive LLM simulator for testing, and establishes extensive test coverage for tool-calling workflows. Core changes include pending tool execution tracking to prevent concurrent continuation attempts, extension of the ToolCall type with optional approval metadata, and propagation of approval information through message conversion pipelines. The changes span the chat client, type definitions, message handling, adapter tests, end-to-end smoke tests, and testing panel UI components. Changes
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
View your CI Pipeline Execution ↗ for commit 4077ac6
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-devtools-core
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/typescript/ai/src/activities/chat/messages.ts (1)
178-187:⚠️ Potential issue | 🟠 MajorPreserve approval metadata when converting ModelMessage → UIMessage.
Right now approvals on
toolCall.approvalare dropped, so approval state/decision is lost in UI conversions.Suggested fix
- parts.push({ - type: 'tool-call', - id: toolCall.id, - name: toolCall.function.name, - arguments: toolCall.function.arguments, - state: 'input-complete', // Model messages have complete arguments - }) + const approval = toolCall.approval + ? { + id: toolCall.approval.id, + needsApproval: true, + approved: toolCall.approval.approved, + } + : undefined + parts.push({ + type: 'tool-call', + id: toolCall.id, + name: toolCall.function.name, + arguments: toolCall.function.arguments, + state: approval ? 'approval-responded' : 'input-complete', + ...(approval && { approval }), + })
🤖 Fix all issues with AI agents
In `@packages/typescript/ai-client/src/chat-client.ts`:
- Around line 317-320: The current guard in streamResponse() returns early when
isLoading is true, so calls to append() can add messages without triggering a
response; implement a follow-up queue: when append() is called while isLoading
is true, push a lightweight marker (e.g., a pendingStream flag or enqueue the
message id) onto a small queue/flag on the ChatClient instance and, when the
active stream finishes (in the completion/cleanup path of streamResponse()),
check that queue/flag and immediately call streamResponse() again to process the
queued append; update append(), streamResponse(), and the stream completion
handler (look for methods named append, streamResponse, and the isLoading
checks) to set/enqueue on append and to dequeue/clear and re-invoke
streamResponse after the current load completes.
In `@packages/typescript/smoke-tests/adapters/src/tests/tools/sequences.test.ts`:
- Line 301: The test declares callOrder with the disallowed shorthand type
`string[]`; update each declaration of `callOrder` (e.g., the `const callOrder:
string[] = []` instances around the tests) to use the preferred generic form
`Array<string>` so the lint rule passes (replace `string[]` with `Array<string>`
in both occurrences).
In `@packages/typescript/smoke-tests/e2e/package.json`:
- Around line 25-30: Package.json currently lists build-only plugin
"vite-tsconfig-paths" alongside runtime deps; move only "vite-tsconfig-paths"
into devDependencies. Edit package.json to remove "vite-tsconfig-paths" from the
dependencies block and add it under devDependencies, while keeping
"@tanstack/tests-adapters" and "zod" in dependencies because
"@tanstack/tests-adapters" is imported at runtime in
src/routes/api.tools-test.ts and "zod" is used at runtime in
src/routes/api.tools-test.ts and src/routes/tools-test.tsx; ensure package.json
retains correct versions for react/react-dom/tailwindcss and other runtime deps.
In `@packages/typescript/smoke-tests/e2e/src/routes/tools-test.tsx`:
- Around line 355-370: The click handler for the approve button currently uses
non-null assertions (tc.approval!.id!) which can throw; instead, read const
approval = tc.approval and const approvalId = approval?.id and early-return if
approvalId is falsy, then use approvalId for
respondedApprovals.current.has(...), respondedApprovals.current.add(...),
addEvent({... toolCallId: tc.id ...}) and addToolApprovalResponse({ id:
approvalId, approved: true }); update the handler in the approve button (the
onClick closure) to use these guarded variables and avoid any `!` assertions.
In `@packages/typescript/smoke-tests/e2e/tests/tools/race-conditions.spec.ts`:
- Line 1: Reorder and split the Playwright import so named exports are
alphabetized and the Page type is a type-only import: change the combined import
of "test, expect, type Page" into two imports — a value import with alphabetized
members (expect, test) and a separate type-only import for Page using "import
type { Page }" referencing the existing import statement in the file.
In `@testing/panel/src/routes/api.simulator-chat.ts`:
- Around line 5-12: The import list at the top is failing the sort-imports rule;
reorder the named imports from '@/lib/simulator-tools' into ascending
alphabetical order to satisfy ESLint (e.g., arrange clientServerTool,
clientServerToolWithApproval, clientToolDef, clientToolWithApprovalDef,
serverTool, serverToolWithApproval). Update the single import statement that
contains serverTool, serverToolWithApproval, clientServerTool,
clientServerToolWithApproval, clientToolDef, and clientToolWithApprovalDef so
the specifiers are alphabetized and then run the linter to confirm the error is
resolved.
- Around line 25-32: Replace the T[] bracket-style array type with the generic
Array<T> form: update the declaration of toolCalls and the function return type
in parseToolCalls from ParsedToolCall[] to Array<ParsedToolCall> (references:
ParsedToolCall type, parseToolCalls function, and the toolCalls variable) so the
file complies with the `@typescript-eslint/array-type` rule.
- Around line 23-55: The regex TOOL_CALL_REGEX is declared with the global flag,
so its internal lastIndex can persist between calls and cause missed matches;
inside parseToolCalls(message) reset TOOL_CALL_REGEX.lastIndex = 0 before
entering the while loop so each invocation starts scanning from the beginning of
the message, then proceed with the existing matching/JSON-parsing logic in
parseToolCalls.
In `@testing/panel/src/routes/simulator.tsx`:
- Around line 232-237: The UI directly calls JSON.parse on part.arguments inside
the JSX (the JSON.stringify(JSON.parse(part.arguments), null, 2) expression)
which will throw if the string is invalid; wrap the parse in a safe path: either
create a small helper (e.g., safeParseJSON) or run a try/catch around JSON.parse
to fall back to a sensible value (the raw part.arguments string or an error
message) before passing to JSON.stringify, and update the JSX to render that
safe result so invalid JSON doesn't crash the component.
- Around line 3-11: Reorder the named imports from 'lucide-react' so they
satisfy the project's sort rule: replace the current list (Send, Square, Zap,
FlaskConical, Server, Monitor, ShieldCheck) with an alphabetically-sorted list:
FlaskConical, Monitor, Send, Server, ShieldCheck, Square, Zap; apply the same
reordering to the other identical import on the other occurrence (lines
referenced in the review).
- Around line 202-209: The code registers remarkGfm in rehypePlugins so GFM
won't be applied; move remarkGfm into the remarkPlugins array for the
ReactMarkdown component. Locate the ReactMarkdown usage and remove remarkGfm
from rehypePlugins (which should keep rehypeRaw, rehypeSanitize,
rehypeHighlight) and add remarkGfm to remarkPlugins so that remarkGfm runs
during Markdown AST processing.
- Around line 488-499: The dynamic Tailwind classes using template literals
(e.g., text-${category.color}-400 and hover:border-${category.color}-500/30) are
not safe for JIT purging; create a static mapping object (e.g., colorClasses = {
cyan: { icon: 'text-cyan-400', hoverBorder: 'hover:border-cyan-500/30' },
purple: {...}, yellow: {...}, green: {...} }) and replace the template literals:
use category.icon with className={colorClasses[category.color].icon} and build
the button className by including colorClasses[category.color].hoverBorder (and
any other static pieces) instead of the template strings; mirror the approach
used in structured.tsx to ensure Tailwind picks up the classes.
🧹 Nitpick comments (11)
packages/typescript/smoke-tests/e2e/tests/tools/approval-flow.spec.ts (3)
1-1: Import statement can be improved per ESLint rules.The static analysis flags import sorting and inline type specifier style. Consider using a top-level type import.
🔧 Suggested fix
-import { test, expect, type Page } from '@playwright/test' +import type { Page } from '@playwright/test' +import { expect, test } from '@playwright/test'
18-46: Helper functions are duplicated across multiple test spec files.The
selectScenario,waitForTestComplete,getMetadata, andgetEventLoghelpers are nearly identical acrossapproval-flow.spec.ts,client-tool.spec.ts, andserver-client-sequence.spec.ts. Consider extracting these to a shared test utilities module to reduce maintenance burden.
119-124: Empty catch block silently swallows errors.The catch block returns an empty array without any indication that parsing failed. While acceptable for test helpers, consider logging a warning for debugging purposes.
🔧 Optional improvement
try { return JSON.parse(el.textContent || '[]') } catch { + console.warn('Failed to parse event-log-json') return [] }packages/typescript/smoke-tests/adapters/src/llm-simulator.ts (2)
1-6: Import sorting can be improved per ESLint rules.🔧 Suggested fix
import type { + ChatOptions, StreamChunk, - ChatOptions, StructuredOutputOptions, StructuredOutputResult, } from '@tanstack/ai'
302-389: Complex but necessary iteration detection logic.The
determineIterationFromMessagesmethod handles multiple message formats correctly:
- Server tool results (
role: 'tool')- UIMessage format (parts with
outputorapproval.approved)- ModelMessage format (toolCalls with approval info)
The logic is well-documented with inline comments explaining each case. Consider adding a unit test specifically for this method to guard against regressions.
packages/typescript/smoke-tests/e2e/src/routes/api.tools-test.ts (1)
9-12: Import statement can be improved per ESLint rules.🔧 Suggested fix
+import type { SimulatorScript } from '@tanstack/tests-adapters' import { createLLMSimulator, - type SimulatorScript, } from '@tanstack/tests-adapters'packages/typescript/smoke-tests/e2e/tests/tools/server-client-sequence.spec.ts (1)
1-1: Import statement can be improved per ESLint rules.🔧 Suggested fix
-import { test, expect, type Page } from '@playwright/test' +import type { Page } from '@playwright/test' +import { expect, test } from '@playwright/test'packages/typescript/smoke-tests/e2e/src/routes/tools-test.tsx (1)
1-5: Import statement can be improved per ESLint rules.🔧 Suggested fix
-import { useState, useCallback, useRef, useEffect } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { createFileRoute } from '@tanstack/react-router' -import { useChat, fetchServerSentEvents } from '@tanstack/ai-react' +import { fetchServerSentEvents, useChat } from '@tanstack/ai-react'packages/typescript/smoke-tests/e2e/tests/tools/client-tool.spec.ts (1)
1-1: Import statement can be improved per ESLint rules.🔧 Suggested fix
-import { test, expect, type Page } from '@playwright/test' +import type { Page } from '@playwright/test' +import { expect, test } from '@playwright/test'packages/typescript/smoke-tests/adapters/src/tests/tools/server-tool.test.ts (1)
7-16: ExtractcollectChunksinto a shared test helper.This helper is repeated across multiple tool test files; consider a shared utility to keep the test suite DRY.
testing/panel/src/routes/simulator.tsx (1)
55-126: RenameTOOL_TEMPLATESto camelCase.Keeping variable names in camelCase improves consistency and aligns with the style rule.
As per coding guidelines: Use camelCase for function and variable names throughout the codebase.♻️ Suggested rename
-const TOOL_TEMPLATES = [ +const toolTemplates = [ { category: 'Server Tools', icon: Server, color: 'cyan', tools: [ { name: 'Server Tool', template: 'serverTool({ text: "hello from server" })', description: 'Executes on server only', }, @@ - {TOOL_TEMPLATES.map((category) => ( + {toolTemplates.map((category) => (Also applies to: 485-505
| // Guard against concurrent streams - if already loading, skip | ||
| if (this.isLoading) { | ||
| return | ||
| } |
There was a problem hiding this comment.
Avoid dropping append() messages when a stream is active.
With the new guard, append() can add a message and then streamResponse() no-ops, leaving the conversation without a response. Consider queuing a follow‑up stream when append() is called while isLoading is true.
💡 One possible fix (queue a follow-up stream)
async append(message: UIMessage | ModelMessage): Promise<void> {
// Normalize the message to ensure it has id and createdAt
const normalizedMessage = normalizeToUIMessage(message, generateMessageId)
@@
// Add to messages
const messages = this.processor.getMessages()
this.processor.setMessages([...messages, uiMessage])
- await this.streamResponse()
+ if (this.isLoading) {
+ this.queuePostStreamAction(() => this.streamResponse())
+ return
+ }
+ await this.streamResponse()
}🤖 Prompt for AI Agents
In `@packages/typescript/ai-client/src/chat-client.ts` around lines 317 - 320, The
current guard in streamResponse() returns early when isLoading is true, so calls
to append() can add messages without triggering a response; implement a
follow-up queue: when append() is called while isLoading is true, push a
lightweight marker (e.g., a pendingStream flag or enqueue the message id) onto a
small queue/flag on the ChatClient instance and, when the active stream finishes
(in the completion/cleanup path of streamResponse()), check that queue/flag and
immediately call streamResponse() again to process the queued append; update
append(), streamResponse(), and the stream completion handler (look for methods
named append, streamResponse, and the isLoading checks) to set/enqueue on append
and to dequeue/clear and re-invoke streamResponse after the current load
completes.
| } | ||
| const adapter = createLLMSimulator(script) | ||
|
|
||
| const callOrder: string[] = [] |
There was a problem hiding this comment.
Use Array<string> for callOrder to satisfy lint.
The repo’s lint rule disallows string[] in favor of Array<string>.
🔧 Proposed fix
- const callOrder: string[] = []
+ const callOrder: Array<string> = []- const callOrder: string[] = []
+ const callOrder: Array<string> = []Also applies to: 371-371
🧰 Tools
🪛 ESLint
[error] 301-301: Array type using 'string[]' is forbidden. Use 'Array' instead.
(@typescript-eslint/array-type)
🤖 Prompt for AI Agents
In `@packages/typescript/smoke-tests/adapters/src/tests/tools/sequences.test.ts`
at line 301, The test declares callOrder with the disallowed shorthand type
`string[]`; update each declaration of `callOrder` (e.g., the `const callOrder:
string[] = []` instances around the tests) to use the preferred generic form
`Array<string>` so the lint rule passes (replace `string[]` with `Array<string>`
in both occurrences).
| "@tanstack/tests-adapters": "workspace:*", | ||
| "react": "^19.2.3", | ||
| "react-dom": "^19.2.3", | ||
| "tailwindcss": "^4.1.18", | ||
| "vite-tsconfig-paths": "^5.1.4" | ||
| "vite-tsconfig-paths": "^5.1.4", | ||
| "zod": "^4.2.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for imports of the three packages in the E2E test directory
echo "=== Searching for imports of `@tanstack/tests-adapters`, vite-tsconfig-paths, zod ==="
find packages/typescript/smoke-tests/e2e -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) | head -20
echo ""
echo "=== Looking for vite config (vite-tsconfig-paths usage) ==="
find packages/typescript/smoke-tests/e2e -name "vite.config.*" -o -name "vitest.config.*"
echo ""
echo "=== Searching for imports of these packages ==="
rg -n '@tanstack/tests-adapters|vite-tsconfig-paths|zod' packages/typescript/smoke-tests/e2e/ --type ts --type tsx --type js --type jsx 2>/dev/null || echo "No matches found with rg"Repository: TanStack/ai
Length of output: 1251
🏁 Script executed:
#!/bin/bash
# Check vite.config.ts for vite-tsconfig-paths usage
echo "=== vite.config.ts ==="
cat -n packages/typescript/smoke-tests/e2e/vite.config.ts
echo ""
echo "=== playwright.config.ts ==="
cat -n packages/typescript/smoke-tests/e2e/playwright.config.tsRepository: TanStack/ai
Length of output: 1555
🏁 Script executed:
#!/bin/bash
# Search more broadly for these packages in the entire e2e directory
echo "=== All files in packages/typescript/smoke-tests/e2e ==="
find packages/typescript/smoke-tests/e2e -type f | grep -v node_modules | grep -v dist | head -30
echo ""
echo "=== Searching for 'zod' anywhere in e2e package ==="
grep -r "zod" packages/typescript/smoke-tests/e2e --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" --include="*.json" 2>/dev/null | head -20
echo ""
echo "=== Searching for 'tests-adapters' anywhere in e2e package ==="
grep -r "tests-adapters" packages/typescript/smoke-tests/e2e --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" --include="*.json" 2>/dev/null || echo "Not found"Repository: TanStack/ai
Length of output: 1935
Move only vite-tsconfig-paths to devDependencies.
vite-tsconfig-paths is a build-time plugin (used in vite.config.ts) and should be in devDependencies. However, @tanstack/tests-adapters is imported in runtime API routes (src/routes/api.tools-test.ts) and zod is used for schema validation in both src/routes/api.tools-test.ts and src/routes/tools-test.tsx—both must remain in dependencies.
♻️ Dependency correction
"dependencies": {
"@tanstack/tests-adapters": "workspace:*",
"react": "^19.2.3",
"react-dom": "^19.2.3",
"tailwindcss": "^4.1.18",
- "vite-tsconfig-paths": "^5.1.4",
"zod": "^4.2.0"
},
"devDependencies": {
+ "vite-tsconfig-paths": "^5.1.4",
"@playwright/test": "^1.57.0",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@tanstack/tests-adapters": "workspace:*", | |
| "react": "^19.2.3", | |
| "react-dom": "^19.2.3", | |
| "tailwindcss": "^4.1.18", | |
| "vite-tsconfig-paths": "^5.1.4" | |
| "vite-tsconfig-paths": "^5.1.4", | |
| "zod": "^4.2.0" | |
| "@tanstack/tests-adapters": "workspace:*", | |
| "react": "^19.2.3", | |
| "react-dom": "^19.2.3", | |
| "tailwindcss": "^4.1.18", | |
| "zod": "^4.2.0" |
🤖 Prompt for AI Agents
In `@packages/typescript/smoke-tests/e2e/package.json` around lines 25 - 30,
Package.json currently lists build-only plugin "vite-tsconfig-paths" alongside
runtime deps; move only "vite-tsconfig-paths" into devDependencies. Edit
package.json to remove "vite-tsconfig-paths" from the dependencies block and add
it under devDependencies, while keeping "@tanstack/tests-adapters" and "zod" in
dependencies because "@tanstack/tests-adapters" is imported at runtime in
src/routes/api.tools-test.ts and "zod" is used at runtime in
src/routes/api.tools-test.ts and src/routes/tools-test.tsx; ensure package.json
retains correct versions for react/react-dom/tailwindcss and other runtime deps.
| <button | ||
| id={`approve-${tc.id}`} | ||
| className="approve-button" | ||
| onClick={() => { | ||
| if (!respondedApprovals.current.has(tc.approval!.id!)) { | ||
| respondedApprovals.current.add(tc.approval!.id!) | ||
| addEvent({ | ||
| type: 'approval-granted', | ||
| toolName: tc.name, | ||
| toolCallId: tc.id, | ||
| }) | ||
| addToolApprovalResponse({ | ||
| id: tc.approval!.id!, | ||
| approved: true, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Non-null assertions on approval data could throw.
Lines 359, 360, 367 use tc.approval!.id! which will throw if approval or id is undefined. While the filter on line 229-231 ensures approval?.needsApproval is truthy, approval.id could theoretically be missing.
🛡️ Safer approach with optional chaining
onClick={() => {
- if (!respondedApprovals.current.has(tc.approval!.id!)) {
- respondedApprovals.current.add(tc.approval!.id!)
+ const approvalId = tc.approval?.id
+ if (approvalId && !respondedApprovals.current.has(approvalId)) {
+ respondedApprovals.current.add(approvalId)
addEvent({
type: 'approval-granted',
toolName: tc.name,
toolCallId: tc.id,
})
addToolApprovalResponse({
- id: tc.approval!.id!,
+ id: approvalId,
approved: true,
})
}
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| id={`approve-${tc.id}`} | |
| className="approve-button" | |
| onClick={() => { | |
| if (!respondedApprovals.current.has(tc.approval!.id!)) { | |
| respondedApprovals.current.add(tc.approval!.id!) | |
| addEvent({ | |
| type: 'approval-granted', | |
| toolName: tc.name, | |
| toolCallId: tc.id, | |
| }) | |
| addToolApprovalResponse({ | |
| id: tc.approval!.id!, | |
| approved: true, | |
| }) | |
| } | |
| <button | |
| id={`approve-${tc.id}`} | |
| className="approve-button" | |
| onClick={() => { | |
| const approvalId = tc.approval?.id | |
| if (approvalId && !respondedApprovals.current.has(approvalId)) { | |
| respondedApprovals.current.add(approvalId) | |
| addEvent({ | |
| type: 'approval-granted', | |
| toolName: tc.name, | |
| toolCallId: tc.id, | |
| }) | |
| addToolApprovalResponse({ | |
| id: approvalId, | |
| approved: true, | |
| }) | |
| } | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In `@packages/typescript/smoke-tests/e2e/src/routes/tools-test.tsx` around lines
355 - 370, The click handler for the approve button currently uses non-null
assertions (tc.approval!.id!) which can throw; instead, read const approval =
tc.approval and const approvalId = approval?.id and early-return if approvalId
is falsy, then use approvalId for respondedApprovals.current.has(...),
respondedApprovals.current.add(...), addEvent({... toolCallId: tc.id ...}) and
addToolApprovalResponse({ id: approvalId, approved: true }); update the handler
in the approve button (the onClick closure) to use these guarded variables and
avoid any `!` assertions.
| @@ -0,0 +1,415 @@ | |||
| import { test, expect, type Page } from '@playwright/test' | |||
There was a problem hiding this comment.
Adjust Playwright import ordering and type-only import style.
This appears to violate lint rules for sorted members and type-only imports.
Suggested fix
-import { test, expect, type Page } from '@playwright/test'
+import { expect, test } from '@playwright/test'
+import type { Page } from '@playwright/test'🧰 Tools
🪛 ESLint
[error] 1-1: Member 'expect' of the import declaration should be sorted alphabetically.
(sort-imports)
[error] 1-1: Prefer using a top-level type-only import instead of inline type specifiers.
(import/consistent-type-specifier-style)
🤖 Prompt for AI Agents
In `@packages/typescript/smoke-tests/e2e/tests/tools/race-conditions.spec.ts` at
line 1, Reorder and split the Playwright import so named exports are
alphabetized and the Page type is a type-only import: change the combined import
of "test, expect, type Page" into two imports — a value import with alphabetized
members (expect, test) and a separate type-only import for Page using "import
type { Page }" referencing the existing import statement in the file.
| interface ParsedToolCall { | ||
| name: string | ||
| arguments: Record<string, any> | ||
| } | ||
|
|
||
| function parseToolCalls(message: string): ParsedToolCall[] { | ||
| const toolCalls: ParsedToolCall[] = [] | ||
| let match |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant lines
if [ -f "testing/panel/src/routes/api.simulator-chat.ts" ]; then
echo "=== File found, reading lines 25-32 ==="
sed -n '20,40p' "testing/panel/src/routes/api.simulator-chat.ts"
else
echo "File not found at expected path"
# Try to find it
find . -name "api.simulator-chat.ts" -type f 2>/dev/null
fiRepository: TanStack/ai
Length of output: 695
🏁 Script executed:
# Search for ESLint configuration files
echo "=== Searching for ESLint config files ==="
find . -maxdepth 3 -type f \( -name ".eslintrc*" -o -name "eslint.config.*" -o -name ".eslintignore" \) 2>/dev/null | head -20Repository: TanStack/ai
Length of output: 115
🏁 Script executed:
# Check if there's an ESLint config in package.json or tsconfig
echo "=== Checking package.json for ESLint config ==="
if [ -f "package.json" ]; then
cat package.json | jq '.eslintConfig // "No eslintConfig found"'
fiRepository: TanStack/ai
Length of output: 126
🏁 Script executed:
# Read the ESLint configuration
echo "=== ESLint configuration ==="
cat eslint.config.js | head -200Repository: TanStack/ai
Length of output: 632
🏁 Script executed:
# Search for array-type rule specifically
echo "=== Searching for array-type configuration ==="
rg "array-type" . --type ts --type js -A 3 -B 3 2>/dev/null | head -50Repository: TanStack/ai
Length of output: 101
🏁 Script executed:
# Search for actual array type usage patterns in the codebase
echo "=== Checking array type usage patterns in TypeScript files ==="
rg ":\s*(Array<|.*\[\])" . --type ts -A 1 -B 1 | head -100Repository: TanStack/ai
Length of output: 5469
🏁 Script executed:
# Look at similar files in the same directory
echo "=== Files in testing/panel/src/routes/ ==="
ls -la testing/panel/src/routes/ | head -20Repository: TanStack/ai
Length of output: 1327
🏁 Script executed:
# Check if there's package-lock or node_modules reference to `@tanstack/eslint-config`
echo "=== Checking `@tanstack/eslint-config` in dependencies ==="
if [ -f "package.json" ]; then
cat package.json | jq '.devDependencies[] | select(. | startswith("@tanstack")) , .dependencies[] | select(. | startswith("@tanstack"))'
fiRepository: TanStack/ai
Length of output: 202
🏁 Script executed:
# Check more files in testing/panel to see the pattern
echo "=== Array type patterns in testing/panel/src/routes/ ==="
rg ":\s*(Array<|.*\[\])" testing/panel/src/routes/ --type ts --type tsx -B 1 | head -80Repository: TanStack/ai
Length of output: 156
🏁 Script executed:
# Look at all the .ts files in testing/panel/src/routes to see if there's consistency
echo "=== Checking all array declarations in api.* files in same directory ==="
for file in testing/panel/src/routes/api.*.ts; do
echo "=== $file ==="
rg ":\s*(Array<|.*\[\])" "$file" -B 1 | head -20
doneRepository: TanStack/ai
Length of output: 988
🏁 Script executed:
# Try to understand `@tanstack/eslint-config` by checking if it's documented elsewhere
echo "=== Searching for eslint-config references in comments ==="
rg "@tanstack/eslint-config" . --type ts --type js -B 2 -A 2 2>/dev/null | head -40Repository: TanStack/ai
Length of output: 337
🏁 Script executed:
# Check if we can find more evidence about array type preference in the codebase
echo "=== Array type patterns across entire codebase ==="
rg ":\s*Array<" . -t ts -c | sort | uniq -c | sort -rn | head -20Repository: TanStack/ai
Length of output: 1099
🏁 Script executed:
# Count [] vs Array<> usage
echo "=== Counting array syntax patterns ==="
echo "Bracket notation (T[]):"
rg ":\s*\w+\[\]" . -t ts | wc -l
echo "Generic notation (Array<T>):"
rg ":\s*Array<" . -t ts | wc -lRepository: TanStack/ai
Length of output: 153
🏁 Script executed:
# Final verification: check all instances of ParsedToolCall[] in the specific file
echo "=== All array declarations in api.simulator-chat.ts ==="
cat -n testing/panel/src/routes/api.simulator-chat.ts | grep -E ":\s*.*\[\]|:\s*Array<"Repository: TanStack/ai
Length of output: 264
Change ParsedToolCall[] to Array<ParsedToolCall> to comply with @typescript-eslint/array-type.
The codebase enforces the Array<T> generic syntax for type annotations rather than T[] notation. Lines 30–31 use bracket notation inconsistently with the rest of the codebase (481 Array<T> vs 61 T[] occurrences).
Fix
-function parseToolCalls(message: string): ParsedToolCall[] {
- const toolCalls: ParsedToolCall[] = []
+function parseToolCalls(message: string): Array<ParsedToolCall> {
+ const toolCalls: Array<ParsedToolCall> = []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| interface ParsedToolCall { | |
| name: string | |
| arguments: Record<string, any> | |
| } | |
| function parseToolCalls(message: string): ParsedToolCall[] { | |
| const toolCalls: ParsedToolCall[] = [] | |
| let match | |
| interface ParsedToolCall { | |
| name: string | |
| arguments: Record<string, any> | |
| } | |
| function parseToolCalls(message: string): Array<ParsedToolCall> { | |
| const toolCalls: Array<ParsedToolCall> = [] | |
| let match |
🧰 Tools
🪛 ESLint
[error] 30-30: Array type using 'ParsedToolCall[]' is forbidden. Use 'Array' instead.
(@typescript-eslint/array-type)
[error] 31-31: Array type using 'ParsedToolCall[]' is forbidden. Use 'Array' instead.
(@typescript-eslint/array-type)
🤖 Prompt for AI Agents
In `@testing/panel/src/routes/api.simulator-chat.ts` around lines 25 - 32, Replace
the T[] bracket-style array type with the generic Array<T> form: update the
declaration of toolCalls and the function return type in parseToolCalls from
ParsedToolCall[] to Array<ParsedToolCall> (references: ParsedToolCall type,
parseToolCalls function, and the toolCalls variable) so the file complies with
the `@typescript-eslint/array-type` rule.
| import { | ||
| Send, | ||
| Square, | ||
| Zap, | ||
| FlaskConical, | ||
| Server, | ||
| Monitor, | ||
| ShieldCheck, | ||
| } from 'lucide-react' |
There was a problem hiding this comment.
Fix import member order to satisfy lint.
The current ordering violates the configured sort rule.
🔧 Proposed fix
-import {
- Send,
- Square,
- Zap,
- FlaskConical,
- Server,
- Monitor,
- ShieldCheck,
-} from 'lucide-react'
+import {
+ FlaskConical,
+ Monitor,
+ Send,
+ Server,
+ ShieldCheck,
+ Square,
+ Zap,
+} from 'lucide-react'-import {
- clientToolDef,
- clientToolWithApprovalDef,
- clientServerToolDef,
- clientServerToolWithApprovalDef,
- createClientResult,
-} from '@/lib/simulator-tools'
+import {
+ clientServerToolDef,
+ clientServerToolWithApprovalDef,
+ clientToolDef,
+ clientToolWithApprovalDef,
+ createClientResult,
+} from '@/lib/simulator-tools'Also applies to: 23-28
🧰 Tools
🪛 ESLint
[error] 7-7: Member 'FlaskConical' of the import declaration should be sorted alphabetically.
(sort-imports)
🤖 Prompt for AI Agents
In `@testing/panel/src/routes/simulator.tsx` around lines 3 - 11, Reorder the
named imports from 'lucide-react' so they satisfy the project's sort rule:
replace the current list (Send, Square, Zap, FlaskConical, Server, Monitor,
ShieldCheck) with an alphabetically-sorted list: FlaskConical, Monitor, Send,
Server, ShieldCheck, Square, Zap; apply the same reordering to the other
identical import on the other occurrence (lines referenced in the review).
| <ReactMarkdown | ||
| rehypePlugins={[ | ||
| rehypeRaw, | ||
| rehypeSanitize, | ||
| rehypeHighlight, | ||
| remarkGfm, | ||
| ]} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n testing/panel/src/routes/simulator.tsx | sed -n '200,215p'Repository: TanStack/ai
Length of output: 730
🌐 Web query:
ReactMarkdown remarkPlugins vs rehypePlugins plugin system
💡 Result:
In react-markdown, remarkPlugins and rehypePlugins hook into two different stages of the unified processing pipeline:
remarkPluginsrun on the Markdown AST (mdast) before Markdown is converted to an HTML-like tree. Use these for Markdown syntax/extensions and Markdown-structure transforms (e.g., GFM tables, footnotes, directive syntax). (github.com)- Then
react-markdownusesremark-rehypeto convert mdast → hast (HTML AST). Options for that bridge are passed viaremarkRehypeOptions. (github.com) rehypePluginsrun on the HTML AST (hast) after conversion. Use these for HTML-ish transforms like addingidanchors to headings, rewriting links, syntax highlighting, sanitization, etc. (github.com)
Rule of thumb: if the plugin is named remark-*, it belongs in remarkPlugins; if it’s rehype-*, it belongs in rehypePlugins, because they operate on different trees (mdast vs hast). (github.com)
Citations:
- 1: https://github.com/remarkjs/react-markdown?utm_source=openai
- 2: https://github.com/remarkjs/react-markdown?utm_source=openai
- 3: https://github.com/remarkjs/react-markdown?utm_source=openai
- 4: https://github.com/remarkjs/remark-rehype?utm_source=openai
remarkGfm belongs in remarkPlugins, not rehypePlugins.
GFM features won't be applied because the plugin is registered on the wrong processing stage. The remarkPlugins array processes the Markdown AST before HTML conversion, while rehypePlugins processes the HTML AST afterward. Since remarkGfm is a remark-* plugin that needs to recognize Markdown syntax extensions (tables, strikethrough, etc.), it must run in the remarkPlugins stage.
🔧 Proposed fix
- <ReactMarkdown
- rehypePlugins={[
- rehypeRaw,
- rehypeSanitize,
- rehypeHighlight,
- remarkGfm,
- ]}
- >
+ <ReactMarkdown
+ remarkPlugins={[remarkGfm]}
+ rehypePlugins={[
+ rehypeRaw,
+ rehypeSanitize,
+ rehypeHighlight,
+ ]}
+ >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <ReactMarkdown | |
| rehypePlugins={[ | |
| rehypeRaw, | |
| rehypeSanitize, | |
| rehypeHighlight, | |
| remarkGfm, | |
| ]} | |
| > | |
| <ReactMarkdown | |
| remarkPlugins={[remarkGfm]} | |
| rehypePlugins={[ | |
| rehypeRaw, | |
| rehypeSanitize, | |
| rehypeHighlight, | |
| ]} | |
| > |
🤖 Prompt for AI Agents
In `@testing/panel/src/routes/simulator.tsx` around lines 202 - 209, The code
registers remarkGfm in rehypePlugins so GFM won't be applied; move remarkGfm
into the remarkPlugins array for the ReactMarkdown component. Locate the
ReactMarkdown usage and remove remarkGfm from rehypePlugins (which should keep
rehypeRaw, rehypeSanitize, rehypeHighlight) and add remarkGfm to remarkPlugins
so that remarkGfm runs during Markdown AST processing.
| <pre className="bg-gray-800 p-2 rounded text-xs overflow-x-auto"> | ||
| {JSON.stringify( | ||
| JSON.parse(part.arguments), | ||
| null, | ||
| 2, | ||
| )} |
There was a problem hiding this comment.
Guard JSON.parse for tool-call arguments.
If part.arguments isn’t valid JSON, this will throw and blank the UI.
🛡️ Safer parse
- <pre className="bg-gray-800 p-2 rounded text-xs overflow-x-auto">
- {JSON.stringify(
- JSON.parse(part.arguments),
- null,
- 2,
- )}
- </pre>
+ <pre className="bg-gray-800 p-2 rounded text-xs overflow-x-auto">
+ {JSON.stringify(
+ (() => {
+ try {
+ return JSON.parse(part.arguments)
+ } catch {
+ return part.arguments
+ }
+ })(),
+ null,
+ 2,
+ )}
+ </pre>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <pre className="bg-gray-800 p-2 rounded text-xs overflow-x-auto"> | |
| {JSON.stringify( | |
| JSON.parse(part.arguments), | |
| null, | |
| 2, | |
| )} | |
| <pre className="bg-gray-800 p-2 rounded text-xs overflow-x-auto"> | |
| {JSON.stringify( | |
| (() => { | |
| try { | |
| return JSON.parse(part.arguments) | |
| } catch { | |
| return part.arguments | |
| } | |
| })(), | |
| null, | |
| 2, | |
| )} | |
| </pre> |
🤖 Prompt for AI Agents
In `@testing/panel/src/routes/simulator.tsx` around lines 232 - 237, The UI
directly calls JSON.parse on part.arguments inside the JSX (the
JSON.stringify(JSON.parse(part.arguments), null, 2) expression) which will throw
if the string is invalid; wrap the parse in a safe path: either create a small
helper (e.g., safeParseJSON) or run a try/catch around JSON.parse to fall back
to a sensible value (the raw part.arguments string or an error message) before
passing to JSON.stringify, and update the JSX to render that safe result so
invalid JSON doesn't crash the component.
| <category.icon className={`w-3 h-3 text-${category.color}-400`} /> | ||
| <span className="text-xs font-medium text-gray-400 uppercase"> | ||
| {category.category} | ||
| </span> | ||
| </div> | ||
| <div className="space-y-1"> | ||
| {category.tools.map((tool) => ( | ||
| <button | ||
| key={tool.name} | ||
| onClick={() => onInject(tool.template)} | ||
| className={`w-full text-left px-3 py-2 rounded-lg text-sm transition-colors bg-gray-800/50 hover:bg-gray-700/50 border border-gray-700/50 hover:border-${category.color}-500/30`} | ||
| title={tool.description} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
ls -la testing/panel/src/routes/simulator.tsxRepository: TanStack/ai
Length of output: 143
🏁 Script executed:
sed -n '488,499p' testing/panel/src/routes/simulator.tsxRepository: TanStack/ai
Length of output: 730
🏁 Script executed:
sed -n '460,510p' testing/panel/src/routes/simulator.tsxRepository: TanStack/ai
Length of output: 1831
🏁 Script executed:
find testing/panel -name "tailwind.config.*" -o -name "tailwind.config.ts" -o -name "tailwind.config.js" 2>/dev/null | head -20Repository: TanStack/ai
Length of output: 37
🏁 Script executed:
grep -n "TOOL_TEMPLATES" testing/panel/src/routes/simulator.tsx | head -5Repository: TanStack/ai
Length of output: 130
🏁 Script executed:
find testing -name "tailwind.config.*" 2>/dev/nullRepository: TanStack/ai
Length of output: 37
🏁 Script executed:
find . -maxdepth 2 -name "tailwind.config.*" 2>/dev/nullRepository: TanStack/ai
Length of output: 37
🏁 Script executed:
sed -n '56,150p' testing/panel/src/routes/simulator.tsxRepository: TanStack/ai
Length of output: 2465
🏁 Script executed:
find . -maxdepth 3 -name "tailwind.config.*" -type f 2>/dev/nullRepository: TanStack/ai
Length of output: 99
🏁 Script executed:
find . -maxdepth 3 -name "*.config.ts" -type f 2>/dev/null | grep -E "tailwind|css"Repository: TanStack/ai
Length of output: 37
🏁 Script executed:
cat testing/panel/tailwind.config.js 2>/dev/null || cat testing/panel/tailwind.config.ts 2>/dev/null || echo "No tailwind config found in testing/panel"Repository: TanStack/ai
Length of output: 96
🏁 Script executed:
cat testing/tailwind.config.js 2>/dev/null || cat testing/tailwind.config.ts 2>/dev/null || echo "No tailwind config found in testing"Repository: TanStack/ai
Length of output: 90
🏁 Script executed:
ls -la testing/panel/ | head -20Repository: TanStack/ai
Length of output: 840
🏁 Script executed:
cat testing/panel/package.json 2>/dev/null | grep -A 5 -B 5 "tailwind"Repository: TanStack/ai
Length of output: 736
🏁 Script executed:
find testing/panel -type f -name "*.json" | head -20Repository: TanStack/ai
Length of output: 151
🏁 Script executed:
cat testing/panel/vite.config.tsRepository: TanStack/ai
Length of output: 582
🏁 Script executed:
cat testing/panel/app.config.tsRepository: TanStack/ai
Length of output: 318
🏁 Script executed:
rg -l "safelist|colors" testing/panel --type ts --type tsx --type js 2>/dev/null | head -10Repository: TanStack/ai
Length of output: 37
🏁 Script executed:
rg "content:|safelist:" testing/panel/src --type ts --type tsxRepository: TanStack/ai
Length of output: 82
🏁 Script executed:
rg "@apply|@config" testing/panel/src --type ts --type tsx --type css | head -20Repository: TanStack/ai
Length of output: 82
🏁 Script executed:
find testing/panel/src -name "*.css" -type fRepository: TanStack/ai
Length of output: 120
🏁 Script executed:
cat testing/panel/src/styles.cssRepository: TanStack/ai
Length of output: 1005
🏁 Script executed:
cat testing/panel/src/routes/tanchat.cssRepository: TanStack/ai
Length of output: 3746
🏁 Script executed:
find . -maxdepth 2 -type f \( -name "tailwind.config.*" -o -name "*.config.ts" \) 2>/dev/null | xargs ls -la 2>/dev/nullRepository: TanStack/ai
Length of output: 2374
🏁 Script executed:
rg "safelist|colors:" . -A 5 --type ts --max-count 10 2>/dev/null | head -50Repository: TanStack/ai
Length of output: 525
🏁 Script executed:
ls -la testing/panel/src/Repository: TanStack/ai
Length of output: 697
🏁 Script executed:
find . -maxdepth 3 -name "postcss.config.*" -o -name ".postcssrc*" 2>/dev/nullRepository: TanStack/ai
Length of output: 37
🏁 Script executed:
cat testing/panel/tsconfig.jsonRepository: TanStack/ai
Length of output: 613
🏁 Script executed:
rg "text-cyan|text-purple|text-yellow|text-green|border-cyan|border-purple|border-yellow|border-green" testing/panel/src --max-count 20Repository: TanStack/ai
Length of output: 6433
🏁 Script executed:
rg "safelist" . --type ts --type js 2>/dev/null | head -20Repository: TanStack/ai
Length of output: 37
🏁 Script executed:
rg "text-\[|bg-\[|border-\[" testing/panel/src/routes/simulator.tsxRepository: TanStack/ai
Length of output: 37
🏁 Script executed:
cat testing/panel/src/routes/structured.tsx | grep -A 3 "const.*Color\|const.*color" | head -30Repository: TanStack/ai
Length of output: 215
Use static class mappings for dynamic color values instead of template literals.
Tailwind v4 (JIT compilation) cannot detect class names built with template literals like text-${category.color}-400 and hover:border-${category.color}-500/30 at build time without explicit safelist configuration, risking class purging. The codebase already follows this pattern elsewhere (see structured.tsx).
Create a static mapping object for the four category colors (cyan, purple, yellow, green):
Suggested implementation
+const categoryColors: Record<
+ string,
+ { text: string; hoverBorder: string }
+> = {
+ cyan: { text: 'text-cyan-400', hoverBorder: 'hover:border-cyan-500/30' },
+ purple: { text: 'text-purple-400', hoverBorder: 'hover:border-purple-500/30' },
+ yellow: { text: 'text-yellow-400', hoverBorder: 'hover:border-yellow-500/30' },
+ green: { text: 'text-green-400', hoverBorder: 'hover:border-green-500/30' },
+}- <category.icon className={`w-3 h-3 text-${category.color}-400`} />
+ <category.icon
+ className={`w-3 h-3 ${categoryColors[category.color].text}`}
+ />- className={`w-full text-left px-3 py-2 rounded-lg text-sm transition-colors bg-gray-800/50 hover:bg-gray-700/50 border border-gray-700/50 hover:border-${category.color}-500/30`}
+ className={`w-full text-left px-3 py-2 rounded-lg text-sm transition-colors bg-gray-800/50 hover:bg-gray-700/50 border border-gray-700/50 ${categoryColors[category.color].hoverBorder}`}🤖 Prompt for AI Agents
In `@testing/panel/src/routes/simulator.tsx` around lines 488 - 499, The dynamic
Tailwind classes using template literals (e.g., text-${category.color}-400 and
hover:border-${category.color}-500/30) are not safe for JIT purging; create a
static mapping object (e.g., colorClasses = { cyan: { icon: 'text-cyan-400',
hoverBorder: 'hover:border-cyan-500/30' }, purple: {...}, yellow: {...}, green:
{...} }) and replace the template literals: use category.icon with
className={colorClasses[category.color].icon} and build the button className by
including colorClasses[category.color].hoverBorder (and any other static pieces)
instead of the template strings; mirror the approach used in structured.tsx to
ensure Tailwind picks up the classes.
|
|
||
| for (const message of this.messages) { | ||
| // todo remove any and fix this | ||
| // Check for UIMessage format (parts array) |
There was a problem hiding this comment.
Maybe keep TODO about removing any at some point?
🎯 Changes
Fixes some issues around client side tool execution of multiple tool calls. Adds tests to validate the fixes.
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.