Skip to content

Conversation

@blackgirlbytes
Copy link
Contributor

@blackgirlbytes blackgirlbytes commented Jan 22, 2026

Summary

Fixes a deadlock in MCP elicitation that caused requests to timeout immediately, and improves the user experience by adding a visible countdown timer.

Before it would say my request immediately timed out
Screenshot 2026-01-22 at 4 26 41 PM

After, it would actually give some time for the time out and give the user an indication that it will time out

Screen.Recording.2026-01-22.at.4.21.26.PM.mov

Problem

When an MCP server called elicitInput(), the elicitation form would appear in the UI but immediately timeout. This was caused by a deadlock:

  1. Tool calls elicitInput() → blocks waiting for user response
  2. Elicitation message sent to channel
  3. Agent's tool loop waits on combined.next().await for tool results
  4. drain_elicitation_messages() only runs after a tool yields
  5. But the tool is blocked waiting for elicitation → deadlock

Solution

Rust (agent.rs)

Changed the tool execution loop to use tokio::select! with a 100ms timeout. This allows the loop to periodically check for elicitation messages even when tools are blocked:

loop {
    // Drain elicitation messages first
    for msg in self.drain_elicitation_messages(...).await {
        yield AgentEvent::Message(msg);
    }

    tokio::select! {
        tool_item = combined.next() => { /* handle tool result */ }
        _ = tokio::time::sleep(Duration::from_millis(100)) => {
            // Continue loop to check for elicitation messages
        }
    }
}

UI (ElicitationRequest.tsx)

Added visual feedback so users understand the time constraint:

  • Countdown timer showing time remaining (5 minutes)
  • Pulsing clock icon with "Waiting for your response" message
  • Urgent state - turns red when under 60 seconds
  • Expired state - clear message when time runs out

Testing

  1. Create an MCP server that uses elicitation
  2. Call a tool that triggers elicitInput()
  3. Verify the form appears with countdown timer
  4. Submit within 5 minutes → tool continues with response
  5. Let it expire → shows expired message

- Fix deadlock in agent.rs where elicitation messages weren't being
  delivered to the UI. The tool execution loop now uses tokio::select!
  with a 100ms timeout to periodically check for elicitation messages
  even when tools are blocked waiting for user input.

- Add countdown timer to ElicitationRequest component so users know
  they have 5 minutes to respond. Timer turns red when under 60 seconds
  and shows an expired state when time runs out.

- Add 'Waiting for your response' indicator with pulsing clock icon
  to make it clear the system is waiting for user input.
Copilot AI review requested due to automatic review settings January 22, 2026 21:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a deadlock where MCP elicitation requests could time out immediately by ensuring the agent loop continues to drain elicitation messages, and improves the desktop UI by showing a visible countdown/expiry state for elicitation prompts.

Changes:

  • Update the agent tool execution loop to periodically poll for elicitation messages via tokio::select! + timeout.
  • Add an elicitation countdown timer, urgent styling, and an “expired” UI state in the desktop app.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
ui/desktop/src/components/ElicitationRequest.tsx Adds countdown timer + urgent/expired UI for elicitation requests.
crates/goose/src/agents/agent.rs Prevents elicitation deadlock by draining elicitation messages while waiting on tool streams.

Comment on lines 19 to +33
const [submitted, setSubmitted] = useState(isClicked);
const [timeRemaining, setTimeRemaining] = useState(ELICITATION_TIMEOUT_SECONDS);
const startTimeRef = useRef(Date.now());

useEffect(() => {
if (submitted || isCancelledMessage || isClicked) return;
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

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.

Copilot AI review requested due to automatic review settings January 22, 2026 21:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines +29 to 46
const startTimeRef = useRef(Date.now());

useEffect(() => {
if (submitted || isCancelledMessage || isClicked) return;

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]);

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.
@blackgirlbytes
Copy link
Contributor Author

/goose please review

@github-actions
Copy link
Contributor

Summary: This PR fixes a real deadlock in MCP elicitation where the tool loop blocked waiting for results while elicitation messages couldn't be drained. The polling approach with tokio::select! and 100ms timeout is a pragmatic solution, and the UI countdown timer provides good user feedback. The fix is sound and solves the described problem.

🟡 Warnings

  1. Duplicate timeout constants (mcp_client.rs, ElicitationRequest.tsx)

    The 300-second timeout is hardcoded in two places with no shared source of truth:

    • Rust: Duration::from_secs(300) in crates/goose/src/agents/mcp_client.rs
    • UI: const ELICITATION_TIMEOUT_SECONDS = 300; in ElicitationRequest.tsx

    If these drift, the UI could show time remaining after the backend has already timed out (or vice versa). Consider either:

    • Having the backend send the timeout value (or expiry timestamp) as part of the elicitation message
    • Extracting a shared constant if that's not feasible

🟢 Suggestions

  1. SVG duplication (ElicitationRequest.tsx:93-106, 129-140)

    The clock icon SVG appears twice in the component. Consider extracting to a small ClockIcon component:

    const ClockIcon = ({ className }: { className?: string }) => (
      <svg className={className} 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>
    );

✅ Highlights

  • Correct use of biased; in tokio::select!: Prioritizes processing tool results when available, only falling through to the timeout branch when tools are blocked. This is exactly the right pattern.

  • Proper timer cleanup: The useEffect correctly returns a cleanup function to clear the interval, preventing memory leaks on unmount or re-render.

  • Correct state priority ordering: The submitted check comes before the isExpired check, so if a user submits just as the timer expires, the "submitted" state wins - good defensive ordering.

  • Good UX with visual urgency: The countdown turning red at 60 seconds provides clear visual feedback without being alarmist for the full 5 minutes.


Review generated by goose

@blackgirlbytes blackgirlbytes merged commit c1a916e into main Jan 23, 2026
23 of 24 checks passed
@blackgirlbytes blackgirlbytes deleted the fix-elicitation-deadlock-and-ux branch January 23, 2026 19:59
zanesq added a commit that referenced this pull request Jan 26, 2026
* origin/main:
  fix: dispatch ADD_ACTIVE_SESSION event before navigating from "View All" (#6679)
  Speed up Databricks provider init by removing fetch of supported models (#6616)
  fix: correct typos in documentation and Justfile (#6686)
  docs: frameDomains and baseUriDomains for mcp apps (#6684)
  docs: add Remotion video creation tutorial (#6675)
  docs: export recipe and copy yaml (#6680)
  Test against fastmcp (#6666)
  docs: mid-session changes (#6672)
  Fix MCP elicitation deadlock and improve UX (#6650)
  chore: upgrade to rmcp 0.14.0 (#6674)
  [docs] add MCP-UI to MCP Apps blog (#6664)
  ACP get working dir from args.cwd (#6653)
  Optimise load config in UI (#6662)

# Conflicts:
#	ui/desktop/src/components/Layout/AppLayout.tsx
katzdave added a commit that referenced this pull request Jan 26, 2026
…o dkatz/canonical-context

* 'dkatz/canonical-provider' of github.com:block/goose: (27 commits)
  docs: add Remotion video creation tutorial (#6675)
  docs: export recipe and copy yaml (#6680)
  Test against fastmcp (#6666)
  docs: mid-session changes (#6672)
  Fix MCP elicitation deadlock and improve UX (#6650)
  chore: upgrade to rmcp 0.14.0 (#6674)
  [docs] add MCP-UI to MCP Apps blog (#6664)
  ACP get working dir from args.cwd (#6653)
  Optimise load config in UI (#6662)
  Fix GCP Vertex AI global endpoint support for Gemini 3 models (#6187)
  fix: macOS keychain infinite prompt loop    (#6620)
  chore: reduce duplicate or unused cargo deps (#6630)
  feat: codex subscription support (#6600)
  smoke test allow pass for flaky providers (#6638)
  feat: Add built-in skill for goose documentation reference (#6534)
  Native images (#6619)
  docs: ml-based prompt injection detection (#6627)
  Strip the audience for compacting (#6646)
  chore(release): release version 1.21.0 (minor) (#6634)
  add collapsable chat nav (#6649)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants