Skip to content

Fix MCP elicitation deadlock and improve UX #6650

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
blackgirlbytes merged 8 commits into from
Jan 23, 2026
Merged

Fix MCP elicitation deadlock and improve UX #6650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ffc08f8
Fix MCP elicitation deadlock and improve UX
blackgirlbytes Jan 22, 2026
03c88a1
Remove redundant comment about elicitation message handling
blackgirlbytes Jan 22, 2026
181e78a
Remove another redundant elicitation comment
blackgirlbytes Jan 22, 2026
43d21a5
Remove tokio::select comment
blackgirlbytes Jan 22, 2026
9588aae
Remove platform notifications comment
blackgirlbytes Jan 22, 2026
e84ff22
Remove tool streams completed comment
blackgirlbytes Jan 22, 2026
aef4031
Remove timeout comment
blackgirlbytes Jan 22, 2026
7cc24f5
Remove timeout comment from ElicitationRequest
blackgirlbytes Jan 22, 2026
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
78 changes: 45 additions & 33 deletions crates/goose/src/agents/agent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,7 +1288,7 @@ impl Agent {
let mut combined = stream::select_all(with_id);
let mut all_install_successful = true;

while let Some((request_id, item)) = combined.next().await {
loop {
if is_token_cancelled(&cancel_token) {
break;
}
Expand All @@ -1297,43 +1297,55 @@ impl Agent {
yield AgentEvent::Message(msg);
}

match item {
ToolStreamItem::Result(output) => {
let output = call_tool_result::validate(output);

// Platform extensions use meta as a way to publish notifications. Ideally we'd
// send the notifications directly, but the current plumbing doesn't support that
// well:
if let Ok(ref call_result) = output {
if let Some(ref meta) = call_result.meta {
if let Some(notification_data) = meta.0.get("platform_notification") {
if let Some(method) = notification_data.get("method").and_then(|v| v.as_str()) {
let params = notification_data.get("params").cloned();
let custom_notification = rmcp::model::CustomNotification::new(
method.to_string(),
params,
);

let server_notification = rmcp::model::ServerNotification::CustomNotification(custom_notification);
yield AgentEvent::McpNotification((request_id.clone(), server_notification));
tokio::select! {
biased;

tool_item = combined.next() => {
match tool_item {
Some((request_id, item)) => {
match item {
ToolStreamItem::Result(output) => {
let output = call_tool_result::validate(output);

if let Ok(ref call_result) = output {
if let Some(ref meta) = call_result.meta {
if let Some(notification_data) = meta.0.get("platform_notification") {
if let Some(method) = notification_data.get("method").and_then(|v| v.as_str()) {
let params = notification_data.get("params").cloned();
let custom_notification = rmcp::model::CustomNotification::new(
method.to_string(),
params,
);

let server_notification = rmcp::model::ServerNotification::CustomNotification(custom_notification);
yield AgentEvent::McpNotification((request_id.clone(), server_notification));
}
}
}
}

if enable_extension_request_ids.contains(&request_id)
&& output.is_err()
{
all_install_successful = false;
}
if let Some(response_msg) = request_to_response_map.get(&request_id) {
let metadata = request_metadata.get(&request_id).and_then(|m| m.as_ref());
let mut response = response_msg.lock().await;
*response = response.clone().with_tool_response_with_metadata(request_id, output, metadata);
}
}
ToolStreamItem::Message(msg) => {
yield AgentEvent::McpNotification((request_id, msg));
}
}
}
}

if enable_extension_request_ids.contains(&request_id)
&& output.is_err()
{
all_install_successful = false;
}
if let Some(response_msg) = request_to_response_map.get(&request_id) {
let metadata = request_metadata.get(&request_id).and_then(|m| m.as_ref());
let mut response = response_msg.lock().await;
*response = response.clone().with_tool_response_with_metadata(request_id, output, metadata);
None => break,
}
}
ToolStreamItem::Message(msg) => {
yield AgentEvent::McpNotification((request_id, msg));

_ = tokio::time::sleep(std::time::Duration::from_millis(100)) => {
// Continue loop to drain elicitation messages
}
}
}
Expand Down
80 changes: 78 additions & 2 deletions ui/desktop/src/components/ElicitationRequest.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,48 @@
import { useState } from 'react';
import { useState, useEffect, useRef } from 'react';
import { ActionRequired } from '../api';
import JsonSchemaForm from './ui/JsonSchemaForm';
import type { JsonSchema } from './ui/JsonSchemaForm';

const ELICITATION_TIMEOUT_SECONDS = 300;

interface ElicitationRequestProps {
isCancelledMessage: boolean;
isClicked: boolean;
actionRequiredContent: ActionRequired & { type: 'actionRequired' };
onSubmit: (elicitationId: string, userData: Record<string, unknown>) => void;
}

function formatTime(seconds: number): string {
const mins = Math.floor(seconds / 60);
const secs = seconds % 60;
return `${mins}:${secs.toString().padStart(2, '0')}`;
}

export default function ElicitationRequest({
isCancelledMessage,
isClicked,
actionRequiredContent,
onSubmit,
}: ElicitationRequestProps) {
const [submitted, setSubmitted] = useState(isClicked);
const [timeRemaining, setTimeRemaining] = useState(ELICITATION_TIMEOUT_SECONDS);
const startTimeRef = useRef(Date.now());

useEffect(() => {
if (submitted || isCancelledMessage || isClicked) return;
Comment on lines 19 to +32
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The countdown is anchored to startTimeRef = Date.now() on mount and never re-initialized for a new elicitationId (or persisted across unmount/remount), so the UI can show an incorrect remaining time after navigation or if the component is reused. Consider keying the start timestamp by elicitationId (e.g., a module-level Map) and/or resetting startTimeRef.current + timeRemaining whenever elicitationId changes (and include elicitationId in the effect deps).

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@blackgirlbytes blackgirlbytes Jan 22, 2026

Choose a reason for hiding this comment

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

The timer resets on mount, which is intentional. If the user navigates away, the server-side timeout continues and the elicitation will expire. When they return, it will be a new elicitation request with a new ID, so a fresh timer is correct.


const interval = setInterval(() => {
const elapsed = Math.floor((Date.now() - startTimeRef.current) / 1000);
const remaining = Math.max(0, ELICITATION_TIMEOUT_SECONDS - elapsed);
setTimeRemaining(remaining);

if (remaining === 0) {
clearInterval(interval);
}
}, 1000);

return () => clearInterval(interval);
}, [submitted, isCancelledMessage, isClicked]);

Comment on lines +29 to 46
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The countdown is anchored to component mount time (useRef(Date.now())), so if the elicitation message is rendered from history (or otherwise mounts late) the UI can incorrectly show a fresh 5:00 remaining even though the server-side 5 minute timeout has already partially/fully elapsed; consider basing the start time on the message’s created timestamp (pass it in) and/or resetting startTimeRef/timeRemaining when actionRequiredContent.data.id changes.

Copilot uses AI. Check for mistakes.
if (actionRequiredContent.data.actionType !== 'elicitation') {
return null;
Expand Down Expand Up @@ -57,17 +83,67 @@ export default function ElicitationRequest({
);
}

const isUrgent = timeRemaining <= 60;
const isExpired = timeRemaining === 0;

if (isExpired) {
return (
<div className="goose-message-content bg-background-muted rounded-2xl px-4 py-2 text-textStandard">
<div className="flex items-center gap-2 text-textSubtle">
<svg
className="w-5 h-5"
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
strokeWidth={2}
>
<path
strokeLinecap="round"
strokeLinejoin="round"
d="M12 8v4l3 3m6-3a9 9 0 11-18 0 9 9 0 0118 0z"
/>
</svg>
<span>This request has expired. The extension will need to ask again.</span>
</div>
</div>
);
}

return (
<div className="flex flex-col">
<div className="goose-message-content bg-background-muted rounded-2xl rounded-b-none px-4 py-2 text-textStandard">
{message || 'Goose needs some information from you.'}
<div className="flex justify-between items-start gap-4">
<span>{message || 'Goose needs some information from you.'}</span>
</div>
</div>
<div className="goose-message-content bg-background-default border border-borderSubtle dark:border-gray-700 rounded-b-2xl px-4 py-3">
<JsonSchemaForm
schema={requested_schema as JsonSchema}
onSubmit={handleSubmit}
submitLabel="Submit"
/>
<div
className={`mt-3 pt-3 border-t border-borderSubtle flex items-center gap-2 text-sm ${isUrgent ? 'text-red-500' : 'text-textSubtle'}`}
>
<svg
className="w-4 h-4 animate-pulse"
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
stroke="currentColor"
strokeWidth={2}
>
<path
strokeLinecap="round"
strokeLinejoin="round"
d="M12 8v4l3 3m6-3a9 9 0 11-18 0 9 9 0 0118 0z"
/>
</svg>
<span>
Waiting for your response ({formatTime(timeRemaining)} remaining)
</span>
</div>
</div>
</div>
);
Expand Down