-
Notifications
You must be signed in to change notification settings - Fork 532
Fix client tool schemas lost after DO restart #897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@cloudflare/ai-chat": patch | ||
| --- | ||
|
|
||
| Fix client tool schemas lost after DO restart by re-sending them with CF_AGENT_TOOL_RESULT |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| import { env } from "cloudflare:test"; | ||
| import { describe, it, expect } from "vitest"; | ||
| import { MessageType } from "../types"; | ||
| import type { UIMessage as ChatMessage } from "ai"; | ||
| import { connectChatWS } from "./test-utils"; | ||
| import { getAgentByName } from "agents"; | ||
|
|
||
| describe("Client tools after reconnect", () => { | ||
| it("should use client tools from CF_AGENT_TOOL_RESULT for continuation", async () => { | ||
| const room = crypto.randomUUID(); | ||
|
|
||
| // Step 1: Set up a conversation with a pending tool call (simulates state before refresh) | ||
| const agentStub = await getAgentByName(env.TestChatAgent, room); | ||
|
|
||
| const userMessage: ChatMessage = { | ||
| id: "msg1", | ||
| role: "user", | ||
| parts: [{ type: "text", text: "Change the background" }] | ||
| }; | ||
|
|
||
| const toolCallId = "call_reconnect_test"; | ||
| const assistantMessage: ChatMessage = { | ||
| id: "assistant-1", | ||
| role: "assistant", | ||
| parts: [ | ||
| { | ||
| type: "tool-changeBackgroundColor", | ||
| toolCallId, | ||
| state: "input-available", | ||
| input: { color: "blue" } | ||
| } | ||
| ] as ChatMessage["parts"] | ||
| }; | ||
|
|
||
| // Persist messages directly (simulates state loaded from SQLite after DO restart) | ||
| await agentStub.persistMessages([userMessage, assistantMessage]); | ||
|
|
||
| // Step 2: Connect (simulates reconnect after refresh) | ||
| // Note: We intentionally do NOT send a CF_AGENT_USE_CHAT_REQUEST first, | ||
| // so _lastClientTools is never set — this simulates DO restart. | ||
| const { ws } = await connectChatWS(`/agents/test-chat-agent/${room}`); | ||
|
|
||
| // Wait for connection to be established | ||
| await new Promise((resolve) => setTimeout(resolve, 200)); | ||
|
|
||
| // Clear any captured context from connection setup | ||
| await agentStub.clearCapturedContext(); | ||
|
|
||
| // Step 3: Send tool result WITH clientTools (simulates client approval after reconnect) | ||
| const clientTools = [ | ||
| { | ||
| name: "changeBackgroundColor", | ||
| description: "Changes the background color", | ||
| parameters: { | ||
| type: "object", | ||
| properties: { color: { type: "string" } } | ||
| } | ||
| }, | ||
| { | ||
| name: "changeTextColor", | ||
| description: "Changes the text color", | ||
| parameters: { | ||
| type: "object", | ||
| properties: { color: { type: "string" } } | ||
| } | ||
| } | ||
| ]; | ||
|
|
||
| ws.send( | ||
| JSON.stringify({ | ||
| type: MessageType.CF_AGENT_TOOL_RESULT, | ||
| toolCallId, | ||
| toolName: "changeBackgroundColor", | ||
| output: { success: true }, | ||
| autoContinue: true, | ||
| clientTools | ||
| }) | ||
| ); | ||
|
|
||
| // Wait for tool result to be applied + 500ms stream wait + continuation | ||
| await new Promise((resolve) => setTimeout(resolve, 1500)); | ||
|
|
||
| // Step 4: Verify continuation received client tools | ||
| const capturedClientTools = await agentStub.getCapturedClientTools(); | ||
| expect(capturedClientTools).toBeDefined(); | ||
| expect(capturedClientTools).toHaveLength(2); | ||
| expect(capturedClientTools![0].name).toBe("changeBackgroundColor"); | ||
| expect(capturedClientTools![1].name).toBe("changeTextColor"); | ||
|
|
||
| ws.close(); | ||
| }); | ||
|
|
||
| it("should work without clientTools in CF_AGENT_TOOL_RESULT (backwards compat)", async () => { | ||
| const room = crypto.randomUUID(); | ||
|
|
||
| const agentStub = await getAgentByName(env.TestChatAgent, room); | ||
|
|
||
| // Persist a conversation with a pending tool call | ||
| await agentStub.persistMessages([ | ||
| { | ||
| id: "msg1", | ||
| role: "user", | ||
| parts: [{ type: "text", text: "Do something" }] | ||
| }, | ||
| { | ||
| id: "assistant-1", | ||
| role: "assistant", | ||
| parts: [ | ||
| { | ||
| type: "tool-testTool", | ||
| toolCallId: "call_compat_test", | ||
| state: "input-available", | ||
| input: {} | ||
| } | ||
| ] as ChatMessage["parts"] | ||
| } | ||
| ]); | ||
|
|
||
| const { ws } = await connectChatWS(`/agents/test-chat-agent/${room}`); | ||
| await new Promise((resolve) => setTimeout(resolve, 200)); | ||
| await agentStub.clearCapturedContext(); | ||
|
|
||
| // Send tool result WITHOUT clientTools (old client behavior) | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: MessageType.CF_AGENT_TOOL_RESULT, | ||
| toolCallId: "call_compat_test", | ||
| toolName: "testTool", | ||
| output: { success: true }, | ||
| autoContinue: true | ||
| // No clientTools field | ||
| }) | ||
| ); | ||
|
|
||
| await new Promise((resolve) => setTimeout(resolve, 1500)); | ||
|
|
||
| // Continuation should still fire, just without client tools | ||
| const capturedClientTools = await agentStub.getCapturedClientTools(); | ||
| expect(capturedClientTools).toBeUndefined(); | ||
|
|
||
| ws.close(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -121,6 +121,12 @@ export type IncomingMessage<ChatMessage extends UIMessage = UIMessage> = | |
| output: unknown; | ||
| /** Whether server should auto-continue the conversation after applying result */ | ||
| autoContinue?: boolean; | ||
| /** Client tool schemas for continuation (client is source of truth) */ | ||
| clientTools?: Array<{ | ||
| name: string; | ||
| description?: string; | ||
| parameters?: Record<string, unknown>; | ||
| }>; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type mismatch with This works at runtime (JSON is untyped on the wire), but TypeScript won't catch cases where a non-JSON-Schema object is passed. Two options:
Either way avoids the types drifting apart. Nit-level — not a blocker if you'd prefer to keep |
||
| } | ||
| | { | ||
| /** Client sends tool approval response to server (for tools with needsApproval) */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider also updating
_lastClientToolswhenclientToolsarrives viaCF_AGENT_TOOL_RESULT.Currently
clientToolsis used for this specific continuation but_lastClientToolsis never refreshed from it. If this continuation itself produces another tool call (multi-step tool chains), the next continuation falls back to the stale/undefined_lastClientTools— unless the client sends yet anotherCF_AGENT_TOOL_RESULTwithclientTools.In practice this likely works today because each tool result message from the client includes
clientTools, but it would be more robust and self-documenting to also update the cache. Consider adding around line 497, beforewaitForStream():This keeps
_lastClientToolsin sync regardless of how the tool result arrives, and any future code paths reading it will get the refreshed value.