add tool approval flow for chat-panel#9507
Open
Light2Dark wants to merge 5 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bundle ReportChanges will increase total bundle size by 16.22kB (0.06%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: marimo-esmAssets Changed:
Files in
Files in
Files in
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an approval UX for tool invocations in the chat panel and hardens backend message parsing against AI SDK tool-part state transitions that can leak stale fields (causing strict pydantic-ai validation failures).
Changes:
- Introduces a tool-call UI that supports approval-requested, denied, error, and history states (replacing the prior single accordion).
- Sanitizes tool parts server-side by stripping fields not allowed by the pydantic-ai model for the part’s current state; adds regression tests.
- Updates AI SDK dependencies and adjusts frontend tool registry invocation API accordingly.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/_ai/test_pydantic_utils.py | Adds unit/regression tests for backend tool-part sanitization. |
| marimo/_ai/_pydantic_ai_utils.py | Implements _sanitize_part and schema-reflection to drop stale tool-part fields before pydantic-ai validation. |
| frontend/package.json | Bumps ai / @ai-sdk/react versions used by the chat UI. |
| pnpm-lock.yaml | Lockfile updates for the AI SDK upgrade and transitive deps. |
| frontend/src/core/ai/tools/registry.ts | Changes invoke to accept a single named-args object. |
| frontend/src/core/ai/tools/tests/registry.test.ts | Updates tests for the new invoke call shape. |
| frontend/src/components/chat/chat-panel.tsx | Wires addToolApprovalResponse through useChat, adds outgoing schema validation canary logging, removes dynamic-tool early-return. |
| frontend/src/components/chat/chat-display.tsx | Replaces ToolCallAccordion with ToolCallView, adds rendering for source-* parts and tool approval props wiring. |
| frontend/src/components/chat/chat-utils.ts | Updates tool invocation to use new registry API; updates auto-send logic to include approval-responded/output-* states. |
| frontend/src/components/chat/reasoning-accordion.tsx | Skips empty non-streaming reasoning parts and improves open/close behavior. |
| frontend/src/components/chat/chat-components.tsx | Adds reusable SourceChip for source-url/source-document parts. |
| frontend/src/components/chat/tool-call/tool-call-view.tsx | New state-based wrapper choosing approval/error/history presentations. |
| frontend/src/components/chat/tool-call/tool-approval-card.tsx | New approval UI for tool calls requiring user approval. |
| frontend/src/components/chat/tool-call/tool-error-card.tsx | New collapsible error presentation for failed tool runs. |
| frontend/src/components/chat/tool-call/tool-history-row.tsx | New accordion row for non-blocking tool-call states/history. |
| frontend/src/components/chat/tool-call/tool-args.tsx | New renderer for tool input arguments. |
| frontend/src/components/chat/tool-call/tool-result.tsx | New renderer for tool outputs, with “pretty” success formatting when schema matches. |
| frontend/src/components/chat/tool-call/shared.ts | Shared types/helpers for tool-call UI. |
| frontend/src/components/chat/tool-call-accordion.tsx | Removes the old monolithic tool-call accordion component. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
5 issues found across 19 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/components/chat/tool-call/tool-result.tsx">
<violation number="1" location="frontend/src/components/chat/tool-call/tool-result.tsx:59">
P2: `isEmpty(value)` incorrectly filters out valid primitive values like `false` and `0`, causing tool result fields to disappear.</violation>
</file>
<file name="frontend/src/components/chat/chat-display.tsx">
<violation number="1" location="frontend/src/components/chat/chat-display.tsx:123">
P2: Validate `source-url` before passing it to `href`; rendering untrusted schemes directly can create unsafe clickable links (e.g. `javascript:`).</violation>
</file>
<file name="frontend/src/components/chat/chat-components.tsx">
<violation number="1" location="frontend/src/components/chat/chat-components.tsx:208">
P1: Validate `href` before rendering the anchor; currently any scheme is allowed, which can enable `javascript:` URL injection when source URLs are untrusted.</violation>
</file>
<file name="frontend/src/components/chat/tool-call/tool-args.tsx">
<violation number="1" location="frontend/src/components/chat/tool-call/tool-args.tsx:19">
P2: `lodash.isEmpty` returns `true` for all primitives (including `0`, `false`), so a primitive `input` value will render as `"{}"` instead of its actual value. Guard the emptiness check so it only applies to objects/arrays/strings.</violation>
<violation number="2" location="frontend/src/components/chat/tool-call/tool-args.tsx:33">
P2: Render array tool arguments with `JSON.stringify` instead of `String(input)` to avoid lossy output like `[object Object]`.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client as Browser (Chat Panel)
participant ChatHook as useChat (AI SDK)
participant ToolView as ToolCallView
participant ToolRegistry as FrontendToolRegistry
participant Backend as Backend API (/api/ai/chat)
participant Sanitizer as _sanitize_part (Python)
participant PydanticAI as pydantic-ai Vercel Adapter
Note over Client,PydanticAI: Tool Approval Flow
Client->>ChatHook: useChat()
ChatHook->>Client: addToolApprovalResponse, addToolOutput
Note over Client,ToolView: Tool Call Execution
ChatHook->>Client: onToolCall (toolCall event)
Client->>ToolRegistry: invoke({toolName, rawArgs, toolContext})
ToolRegistry->>ToolRegistry: Validate args against schema
alt Valid args
ToolRegistry->>Client: InvokeResult (tool output)
Client->>ChatHook: addToolOutput({tool, toolCallId, result})
else Invalid args
ToolRegistry->>Client: InvokeResult with error
Client->>ChatHook: addToolOutput({tool, toolCallId, error})
end
Note over Client,ToolView: Approval Required Path
ChatHook->>Client: UIMessage with approval state
Client->>ToolView: render (state="approval-requested", approval)
ToolView->>Client: ToolApprovalCard (ShieldQuestionIcon, approve/deny buttons)
alt User Approves
Client->>ChatHook: addToolApprovalResponse({id, approved: true})
ChatHook->>Client: Tool state → "approval-responded"
Client->>Client: hasPendingToolCalls() → true (ready to send)
Client->>Backend: POST /api/ai/chat (with approval-responded parts)
else User Denies
Client->>ChatHook: addToolApprovalResponse({id, approved: false})
ChatHook->>Client: Tool state → "output-denied"
Client->>Client: hasPendingToolCalls() → true (ready to send)
Client->>Backend: POST /api/ai/chat (with output-denied + denial info)
end
Note over Client,Backend: Request Prep
Client->>ChatHook: prepareSendMessagesRequest
ChatHook->>ChatHook: NEW: safeValidateUIMessages (canary validation)
alt Validation fails
ChatHook->>ChatHook: Logger.debug (warning only, no block)
end
ChatHook->>Client: completionBody
Note over Backend,PydanticAI: Backend Sanitization
Client->>Backend: POST /api/ai/chat (messages with tool parts)
Backend->>Sanitizer: _sanitize_part(part) for each part
alt Tool part with state transition
Sanitizer->>Sanitizer: Look up allowed fields from pydantic-ai schema
Sanitizer->>Sanitizer: Drop stale fields (output, errorText from prior state)
Sanitizer->>Backend: Cleaned part dict
else Non-tool part
Sanitizer->>Backend: Pass through unchanged
end
Backend->>PydanticAI: convert_to_pydantic_messages(sanitized messages)
PydanticAI->>PydanticAI: Validate against strict schema (extra='forbid')
alt Valid
PydanticAI->>Backend: Parsed UIMessage objects
Backend->>Client: AI response
else Invalid
PydanticAI->>Backend: ValidationError (blocked by sanitizer)
Note over Backend: Would have failed without sanitization
end
Note over Client,ToolView: Tool State Display
alt Tool completes successfully
ChatHook->>Client: Tool state → "output-available"
Client->>ToolView: render (state="output-available")
ToolView->>Client: ToolHistoryRow (accordion, green checkmark)
Client->>Client: hasPendingToolCalls() → true
else Tool errors
ChatHook->>Client: Tool state → "output-error"
Client->>ToolView: render (state="output-error", isLive=true)
ToolView->>Client: ToolErrorCard (red, auto-expanded while live)
Note over ToolView: Auto-collapses when message is no longer last
else Tool denied
ChatHook->>Client: Tool state → "output-denied"
Client->>ToolView: render (state="output-denied")
ToolView->>Client: ToolHistoryRow (accordion, BanIcon, denial reason)
end
Note over Client,PydanticAI: Auto-send Logic
Client->>Client: hasPendingToolCalls() iterates last message parts
alt All tools ready (output-available, output-error, output-denied, approval-responded)
Client->>Client: Auto-trigger next send (no manual submit needed)
else Still pending (input-streaming, input-available, approval-requested)
Client->>Client: Wait for completion or user action
end
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Summary
The AI SDK's
addToolApprovalResponsetransitions tool parts via object spread, which carries over fields the AI SDK's ownZod schema marks
z.never().optional()for the new state. pydantic-ai'sextra='forbid'mirrors that intent and rightly rejects them. So, we sanitize these values on the backend by reflecting the Pydantic schema, which gives us future compatability.Tested a few tools to make sure no regression
📋 Pre-Review Checklist
✅ Merge Checklist