fix(ai-persistence): preserve cancelled tool resumes - #1090
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPersistence resume-state reconstruction now includes cancelled tool-call IDs. Cancellation tests verify approval and client-tool handling, including persisted client-tool cancellation, completion, and clearing of pending interrupts. ChangesCancelled resume state
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant PersistedInterrupts
participant PersistenceMiddleware
participant ToolExecutor
PersistedInterrupts->>PersistenceMiddleware: provide cancelled interrupt entries
PersistenceMiddleware->>PersistenceMiddleware: collect cancelledToolCallIds
PersistenceMiddleware->>ToolExecutor: return cancellation-only resumeToolState
ToolExecutor->>ToolExecutor: emit cancellation error result
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai-persistence/tests/interrupts.test.ts`:
- Around line 381-397: Update the client-tool fixtures used by the test around
persistClientToolTurn and chat to use toolDefinition() with shared metadata
defined by a Zod schema such as z.object({ query: z.string() }). Keep the
fixture client-only by leaving it without execute or marking it with .client(),
and do not use .server().
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 45f26a9a-92b8-46bf-aeed-37b61331b835
📒 Files selected for processing (1)
packages/ai-persistence/tests/interrupts.test.ts
| it('completes a cancelled client-tool resume from persisted state with empty client messages', async () => { | ||
| const persistence = memoryPersistence() | ||
| await persistClientToolTurn(persistence, [clientTool('clientSearch')]) | ||
|
|
||
| const pending = await persistence.stores.interrupts!.get( | ||
| 'client_tool_tool-call-1', | ||
| ) | ||
| expect(pending?.status).toBe('pending') | ||
|
|
||
| const afterCancel = mockAdapter([ | ||
| [runStarted(), text('cancelled-and-done'), runFinished('r1')], | ||
| ]) | ||
| const chunks = await collect( | ||
| chat({ | ||
| adapter: afterCancel.adapter, | ||
| messages: [], | ||
| tools: [clientTool('clientSearch')], |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find repository examples of the required tool builder and implementations.
rg -n -C 4 --glob '*.{ts,tsx}' '\btoolDefinition\s*\(' packages
rg -n -C 3 --glob '*.{ts,tsx}' '\.(server|client)\s*\(' packagesRepository: TanStack/ai
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- target file imports and fixture definitions ---'
sed -n '1,130p' packages/ai-persistence/tests/interrupts.test.ts
rg -n -C 8 'clientTool|persistClientToolTurn|toolDefinition|from .*zod' packages/ai-persistence/tests/interrupts.test.ts
echo '--- toolDefinition API declarations and focused examples ---'
rg -n -C 6 'export .*toolDefinition|function toolDefinition|class ToolDefinition|\.server\(|\.client\(' packages/ai-core packages/ai-client packages/ai-persistence --glob '*.{ts,tsx}' | head -n 240Repository: TanStack/ai
Length of output: 26750
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- candidate source files ---'
fd -i 'tool' packages --type f | grep -E '(definition|types|index)\.(ts|tsx)$' | head -n 120
echo '--- toolDefinition declarations and Tool types ---'
rg -n -C 12 'toolDefinition|export type Tool|interface Tool|type ToolDefinition' packages --glob '*.{ts,tsx}' \
| grep -v '/tests/' | head -n 320
echo '--- persistence test imports and all fixture uses ---'
rg -n -C 3 'from .*(ai|zod)|clientTool\(|approvalClientTool\(' packages/ai-persistence/tests/interrupts.test.tsRepository: TanStack/ai
Length of output: 24031
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tool-definition implementation ---'
wc -l packages/ai/src/activities/chat/tools/tool-definition.ts
cat -n packages/ai/src/activities/chat/tools/tool-definition.ts
echo '--- exports and core tool types ---'
rg -n -C 8 'tool-definition|ToolDefinition|ServerTool|ClientTool|interface Tool|type Tool' packages/ai/src packages/ai-client/src --glob '*.{ts,tsx}' | head -n 360
echo '--- nearby persistence test tool fixtures and imports ---'
sed -n '1,145p' packages/ai-persistence/tests/interrupts.test.tsRepository: TanStack/ai
Length of output: 45004
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- runtime classification of tools without execute ---'
rg -n -C 10 'execute\?|typeof .*execute|__toolSide|ClientToolRequest|client tool|client-tool' packages/ai/src/activities packages/ai/src --glob '*.{ts,tsx}' | head -n 360
echo '--- target test assertions for the client-tool fixtures ---'
sed -n '260,455p' packages/ai-persistence/tests/interrupts.test.ts
echo '--- imports and package test conventions for Zod/toolDefinition ---'
rg -n -C 5 'from .*(zod|`@tanstack/ai`)|toolDefinition\(\{' packages/ai-persistence/tests packages/ai-persistence/src --glob '*.{ts,tsx}' | head -n 220Repository: TanStack/ai
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- exact tool execution branch ---'
sed -n '760,900p' packages/ai/src/activities/chat/tools/tool-calls.ts
echo '--- Tool base type and schema conversion ---'
rg -n -C 14 'export interface Tool|export type Tool|type AnyTool|inputSchema' packages/ai/src/types.ts packages/ai/src --glob '*.ts' \
| head -n 260
echo '--- direct-definition documentation and client-side examples ---'
sed -n '276,330p' packages/ai/src/activities/chat/tools/tool-definition.ts
rg -n -C 5 'toolDefinition\(\{[^}]*name: .*(client|lookup|search)|\.client\(\)' packages/ai-client/tests packages/ai/src --glob '*.{ts,tsx}' | head -n 180Repository: TanStack/ai
Length of output: 31148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
test = Path("packages/ai-persistence/tests/interrupts.test.ts").read_text()
definition = Path("packages/ai/src/activities/chat/tools/tool-definition.ts").read_text()
calls = Path("packages/ai/src/activities/chat/tools/tool-calls.ts").read_text()
raw = re.search(
r"const clientTool = \(name: string\): Tool => \(\{(?P<body>.*?)\n\}\)",
test,
re.S,
)
assert raw, "clientTool fixture not found"
body = raw.group("body")
assert "execute" not in body
assert "inputSchema" not in body
assert "toolDefinition" not in body
assert "Used directly in chat() on the server" in definition
assert re.search(r"server:\s*<", definition)
assert re.search(r"client:\s*<", definition)
client_branch = re.search(
r"// CASE 1: Client-side tool \(no execute function\)(?P<body>.*?)(?=\n\s*// CASE 2:|\Z)",
calls,
re.S,
)
assert client_branch, "client-side execution branch not found"
assert "if (!tool.execute)" in calls
assert "if (tool.inputSchema && isStandardSchema(tool.inputSchema))" in calls
print("raw_fixture_has_execute:", "execute" in body)
print("raw_fixture_has_input_schema:", "inputSchema" in body)
print("tool_definition_supports_direct_server_use:", True)
print("tool_definition_exposes_server_and_client_builders:", True)
print("runtime_classifies_no_execute_as_client_tool:", True)
print("runtime_validates_standard_input_schema:", True)
PYRepository: TanStack/ai
Length of output: 419
Use toolDefinition() for the client-tool fixtures.
Define the shared metadata with a Zod schema, such as z.object({ query: z.string() }). Keep this client-only fixture as a bare definition or use .client(); .server() is not required because chat() treats tools without execute as client-side.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ai-persistence/tests/interrupts.test.ts` around lines 381 - 397,
Update the client-tool fixtures used by the test around persistClientToolTurn
and chat to use toolDefinition() with shared metadata defined by a Zod schema
such as z.object({ query: z.string() }). Keep the fixture client-only by leaving
it without execute or marking it with .client(), and do not use .server().
Source: Coding guidelines
|
View your CI Pipeline Execution ↗ for commit 151423d
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-byteplus
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-codex
@tanstack/ai-cohere
@tanstack/ai-devtools-core
@tanstack/ai-durable-stream
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-daytona
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-isolate-quickjs-bun
@tanstack/ai-mcp
@tanstack/ai-memory
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-persistence
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
| @@ -829,6 +844,7 @@ describe('interrupt persistence', () => { | |||
| run.calls[0] as { clientToolResults?: ReadonlyMap<string, unknown> } | |||
| ).clientToolResults | |||
There was a problem hiding this comment.
[suggestion] The new assertions only prove the translator wrote cancelledToolCallIds. Both cancel tests still use empty messages, no tools, and no stored thread with a pending assistant tool call. The engine therefore never reaches checkForPendingToolCalls() / executeToolCalls(), so these tests cannot fail if a cancelled hydrated client tool still re-interrupts. The file already has the right shape for that path in applies persisted approval and client-tool resume decisions with empty client messages (persist tool-call turn, resume with empty client messages, assert chunks).
Suggestion: Add a regression that mirrors that hydrate path for status: 'cancelled': persist a client-tool interrupt plus the assistant tool-call message, resume with empty messages and the same tools, and assert a TOOL_CALL_RESULT of Tool execution cancelled plus no new client_tool_* interrupt. Keep the current onConfig Set check if you want, but do not treat it as coverage for #1088.
Summary
Fixes #1088
Validation
The repository Playwright/Chromium install was blocked during Windows extraction, so the E2E suite was not run.
Summary by CodeRabbit