-
-
Notifications
You must be signed in to change notification settings - Fork 173
Tool cancellation support #1161
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -90,6 +90,10 @@ interface MCPAppsRendererProps { | |
| onDisplayModeChange?: (mode: DisplayMode) => void; | ||
| onRequestFullscreen?: (toolCallId: string) => void; | ||
| onExitFullscreen?: (toolCallId: string) => void; | ||
| /** Whether the tool was cancelled (SEP-1865 ui/notifications/tool-cancelled) */ | ||
| toolCancelled?: boolean; | ||
| /** Reason for tool cancellation */ | ||
| toolCancelReason?: string; | ||
| } | ||
|
|
||
| class LoggingTransport implements Transport { | ||
|
|
@@ -171,6 +175,8 @@ export function MCPAppsRenderer({ | |
| onDisplayModeChange, | ||
| onRequestFullscreen, | ||
| onExitFullscreen, | ||
| toolCancelled, | ||
| toolCancelReason, | ||
| }: MCPAppsRendererProps) { | ||
| const sandboxRef = useRef<SandboxedIframeHandle>(null); | ||
| const themeMode = usePreferencesStore((s) => s.themeMode); | ||
|
|
@@ -322,6 +328,7 @@ export function MCPAppsRenderer({ | |
| const lastToolInputRef = useRef<string | null>(null); | ||
| const lastToolOutputRef = useRef<string | null>(null); | ||
| const lastToolErrorRef = useRef<string | null>(null); | ||
| const lastToolCancelledRef = useRef<boolean>(false); | ||
| const isReadyRef = useRef(false); | ||
|
|
||
| const onSendFollowUpRef = useRef(onSendFollowUp); | ||
|
|
@@ -335,9 +342,10 @@ export function MCPAppsRenderer({ | |
| const toolCallIdRef = useRef(toolCallId); | ||
| const pipWidgetIdRef = useRef(pipWidgetId); | ||
|
|
||
| // Fetch widget HTML when tool output is available or CSP mode changes | ||
| // Fetch widget HTML when tool input is available (for cancellation support) or output is available | ||
| // Loading early allows us to send cancellation notifications to the widget during execution | ||
| useEffect(() => { | ||
| if (toolState !== "output-available") return; | ||
| if (toolState !== "input-available" && toolState !== "output-available") return; | ||
|
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. Missing cancellation ref reset on CSP mode changeLow Severity The new |
||
| // Re-fetch if CSP mode changed (widget needs to reload with new CSP policy) | ||
| if (widgetHtml && loadedCspMode === cspMode) return; | ||
|
|
||
|
|
@@ -860,8 +868,10 @@ export function MCPAppsRenderer({ | |
| bridge.setHostContext(hostContext); | ||
| }, [hostContext, isReady]); | ||
|
|
||
| // Send tool input when available (works with input-available for early widget loading) | ||
| useEffect(() => { | ||
| if (!isReady || toolState !== "output-available") return; | ||
| if (!isReady) return; | ||
| if (toolState !== "input-available" && toolState !== "output-available") return; | ||
| const bridge = bridgeRef.current; | ||
| if (!bridge || lastToolInputRef.current !== null) return; | ||
|
|
||
|
|
@@ -902,10 +912,26 @@ export function MCPAppsRenderer({ | |
| }); | ||
| }, [isReady, toolErrorText, toolOutput, toolState]); | ||
|
|
||
| // SEP-1865: Send tool cancellation notification | ||
| useEffect(() => { | ||
| if (!isReady || !toolCancelled) return; | ||
| const bridge = bridgeRef.current; | ||
| if (!bridge) return; | ||
|
|
||
| // Prevent duplicate cancellation notifications | ||
| if (lastToolCancelledRef.current) return; | ||
| lastToolCancelledRef.current = true; | ||
|
|
||
| bridge.sendToolCancelled?.({ | ||
| reason: toolCancelReason ?? "Tool execution was cancelled", | ||
| }); | ||
| }, [isReady, toolCancelled, toolCancelReason]); | ||
|
|
||
| useEffect(() => { | ||
| lastToolInputRef.current = null; | ||
| lastToolOutputRef.current = null; | ||
| lastToolErrorRef.current = null; | ||
| lastToolCancelledRef.current = false; | ||
| }, [toolCallId]); | ||
|
|
||
| const handleSandboxMessage = (event: MessageEvent) => { | ||
|
|
@@ -945,11 +971,23 @@ export function MCPAppsRenderer({ | |
| ); | ||
| }; | ||
|
|
||
| // Loading states | ||
| if (toolState !== "output-available") { | ||
| // Loading states - show placeholder only during input-streaming (before input is complete) | ||
| // Once input-available, we load the widget for cancellation support | ||
| if (toolState !== "input-available" && toolState !== "output-available" && toolState !== "output-error") { | ||
| // Show cancelled state if tool was cancelled before input was available | ||
| if (toolCancelled) { | ||
| return ( | ||
| <div className="border border-orange-500/40 rounded-md bg-orange-500/10 text-xs text-orange-600 dark:text-orange-400 px-3 py-2"> | ||
| <div className="font-medium">Tool Cancelled</div> | ||
| <div className="text-orange-500/80 mt-1"> | ||
| {toolCancelReason ?? "Operation was cancelled"} | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| return ( | ||
| <div className="border border-border/40 rounded-md bg-muted/30 text-xs text-muted-foreground px-3 py-2"> | ||
| Waiting for tool to finish executing... | ||
| Waiting for tool input... | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
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.
Widget fetch missing output-error state causes infinite loading
Medium Severity
The rendering condition at line 976 was expanded to allow
output-errorstate (in addition toinput-availableandoutput-available), but the widget HTML fetch condition at line 348 only triggers forinput-availableoroutput-available. If a tool's state goes directly tooutput-errorwithout passing throughinput-available, the widget HTML fetch never starts, but rendering proceeds past the early-return check, causing the component to show "Preparing MCP App widget..." indefinitely sincewidgetHtmlwill never be populated.Additional Locations (1)
client/src/components/chat-v2/thread/mcp-apps-renderer.tsx#L975-L976