Skip to content

Conversation

@shrey150
Copy link
Contributor

Fix unawaited notification() call that could cause message ordering issues on the transport. The notification and tool response were potentially being written concurrently to stdout

@shrey150 shrey150 requested a review from Kylejeong2 December 10, 2025 00:50
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 10, 2025

Greptile Overview

Greptile Summary

Added await to the MCP server notification() call to prevent concurrent writes to stdout transport. Without the await, the notification and tool response could race, causing message ordering issues.

Key Changes:

  • Fixed potential race condition in src/tools/screenshot.ts:106
  • Ensures notification completes before tool response is sent
  • Maintains proper message ordering on the transport layer

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • Single line change that correctly fixes a race condition by adding proper async/await handling. The fix is minimal, well-understood, and prevents potential message ordering issues without introducing any new logic or side effects
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/tools/screenshot.ts 5/5 Added await to notification() call to prevent race conditions between notification and tool response on stdout transport

Sequence Diagram

sequenceDiagram
    participant Client as MCP Client
    participant Server as MCP Server
    participant Tool as Screenshot Tool
    participant Browser as Browser/CDP
    participant Transport as stdout Transport
    
    Client->>Server: call_tool(browserbase_screenshot)
    Server->>Tool: handleScreenshot()
    Tool->>Browser: Page.captureScreenshot()
    Browser-->>Tool: screenshot data (base64)
    Tool->>Tool: Process & resize image
    Tool->>Tool: registerScreenshot(sessionId, name, data)
    
    Note over Tool,Transport: Critical Fix: await notification
    Tool->>Server: await notification(resources/list_changed)
    Server->>Transport: Write notification to stdout
    Transport-->>Server: Write complete
    Server-->>Tool: Notification sent
    
    Tool-->>Server: Return tool result
    Server->>Transport: Write tool response to stdout
    Transport-->>Client: Tool response
    
    Note over Transport: Messages properly ordered:<br/>1. Notification<br/>2. Tool Response
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@shrey150 shrey150 merged commit 5946678 into main Dec 10, 2025
1 check passed
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.

3 participants