Skip to content

Dont show MCP UI/Apps until tool is approved #6492

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.

Sign up for GitHub

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

Merged
DOsinga merged 1 commit into from
Jan 21, 2026
Merged

Dont show MCP UI/Apps until tool is approved #6492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ui/desktop/src/components/GooseMessage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
getToolResponses,
getToolConfirmationContent,
getElicitationContent,
getPendingToolConfirmationIds,
NotificationEvent,
} from '../types/message';
import { Message } from '../api';
Expand Down Expand Up @@ -99,6 +100,8 @@ export default function GooseMessage({
return responseMap;
}, [messages, messageIndex, toolRequests]);

const pendingConfirmationIds = getPendingToolConfirmationIds(messages);

return (
<div className="goose-message flex w-[90%] justify-start min-w-0">
<div className="flex flex-col w-full min-w-0">
Expand Down Expand Up @@ -157,6 +160,7 @@ export default function GooseMessage({
toolResponse={toolResponsesMap.get(toolRequest.id)}
notifications={toolCallNotifications.get(toolRequest.id)}
isStreamingMessage={isStreaming}
isPendingApproval={pendingConfirmationIds.has(toolRequest.id)}
append={append}
/>
</div>
Expand Down
12 changes: 8 additions & 4 deletions ui/desktop/src/components/ToolCallWithResponse.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ interface ToolCallWithResponseProps {
toolResponse?: ToolResponseMessageContent;
notifications?: NotificationEvent[];
isStreamingMessage?: boolean;
isPendingApproval: boolean;
append?: (value: string) => void;
}

Expand Down Expand Up @@ -106,10 +107,8 @@ function McpAppWrapper({
? requestWithMeta.toolCall.value.arguments
: undefined;

// Memoize toolInput to prevent unnecessary re-renders
const toolInput = useMemo(() => ({ arguments: toolArguments || {} }), [toolArguments]);

// Memoize toolResult to prevent unnecessary re-renders
const toolResult = useMemo(() => {
if (!toolResponse) return undefined;
const resultWithMeta = toolResponse.toolResult as ToolResultWithMeta;
Expand Down Expand Up @@ -149,6 +148,7 @@ export default function ToolCallWithResponse({
toolResponse,
notifications,
isStreamingMessage,
isPendingApproval,
append,
}: ToolCallWithResponseProps) {
// Handle both the wrapped ToolResult format and the unwrapped format
Expand All @@ -169,6 +169,8 @@ export default function ToolCallWithResponse({
requestWithMeta._meta?.ui?.resourceUri || resultWithMeta?.value?._meta?.ui?.resourceUri
);

const shouldShowMcpContent = !isPendingApproval;
Copy link
Collaborator

@aharvard aharvard Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe use !isPendingApproval directly for conditions since content is a meaningful term in the MCP spec?

!isPendingApproval && !hasMcpAppResourceURI ...t( )

Copy link
Collaborator Author

@DOsinga DOsinga Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean with content is a meaningful term? I kinda like introducing variables like this to indicate intent. if you !isPendingApproval you have to think a moment, why do we only show this when we have no pending approval, while at the time of the variable introduction it just says, ah we should show content when there is no pending approvals. but if you think it is confusing, we can rename the var?

Copy link
Collaborator

@aharvard aharvard Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MCP Content https://modelcontextprotocol.io/specification/2025-11-25/schema#content (heads up, page might no auto scroll to the right section)

Yeah, intent indication sounds good.

Maybe shouldShowMcpApp is clearer than shouldShowMcpContent and follows the pattern of hasMcpAppResourceURI?


return (
<>
<div
Expand All @@ -187,7 +189,8 @@ export default function ToolCallWithResponse({
/>
</div>
{/* MCP UI — Inline */}
{!hasMcpAppResourceURI &&
{shouldShowMcpContent &&
!hasMcpAppResourceURI &&
toolResponse?.toolResult &&
getToolResultContent(toolResponse.toolResult).map((content, index) => {
const resourceContent = isEmbeddedResource(content)
Expand All @@ -210,7 +213,8 @@ export default function ToolCallWithResponse({
}
})}

{hasMcpAppResourceURI && sessionId && (
{/* MCP App */}
{shouldShowMcpContent && hasMcpAppResourceURI && sessionId && (
<McpAppWrapper
toolRequest={toolRequest}
toolResponse={toolResponse}
Expand Down
1 change: 1 addition & 0 deletions ui/desktop/src/components/sessions/SessionViewComponents.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ export const SessionMessages: React.FC<SessionMessagesProps> = ({
isCancelledMessage={
toolResponsesMap.get(toolRequest.id) == undefined
}
isPendingApproval={false}
key={toolRequest.id}
toolRequest={toolRequest}
toolResponse={toolResponsesMap.get(toolRequest.id)}
Expand Down
33 changes: 33 additions & 0 deletions ui/desktop/src/types/message.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,39 @@ export function getToolConfirmationContent(
);
}

export function getToolConfirmationId(
content: ActionRequired & { type: 'actionRequired' }
): string | undefined {
if (content.data.actionType === 'toolConfirmation') {
return content.data.id;
}
return undefined;
}

export function getPendingToolConfirmationIds(messages: Message[]): Set<string> {
const pendingIds = new Set<string>();
const respondedIds = new Set<string>();

for (const message of messages) {
const responses = getToolResponses(message);
for (const response of responses) {
respondedIds.add(response.id);
}
}

for (const message of messages) {
const confirmation = getToolConfirmationContent(message);
if (confirmation) {
const confirmationId = getToolConfirmationId(confirmation);
if (confirmationId && !respondedIds.has(confirmationId)) {
pendingIds.add(confirmationId);
}
}
}

return pendingIds;
}

export function getElicitationContent(
message: Message
): (ActionRequired & { type: 'actionRequired' }) | undefined {
Expand Down
Loading