Skip to content

fix: resolve initialization protocol violation#189

Closed
echozyr2001 wants to merge 1 commit into
wonderwhy-er:mainfrom
echozyr2001:fix-server-initialization
Closed

fix: resolve initialization protocol violation#189
echozyr2001 wants to merge 1 commit into
wonderwhy-er:mainfrom
echozyr2001:fix-server-initialization

Conversation

@echozyr2001
Copy link
Copy Markdown
Contributor

@echozyr2001 echozyr2001 commented Jul 21, 2025

Resolves MCP server initialization failures, where clients receive LoggingMessageNotification instead of expected initialization response.

Root Cause

The FilteredStdioServerTransport immediately redirected console output to JSON-RPC notifications, causing protocol violations during the initialization phase.

Solution

  • Added initialization state control: isInitialized flag prevents premature notifications
  • Conditional console redirection: Output goes to stderr during initialization, JSON-RPC after
  • Proper timing control: enableNotifications() method called after MCP connection established

Files Modified

  • src/custom-stdio.ts: Added state control and conditional redirection
  • src/index.ts: Proper timing for enabling notifications
  • src/server.ts: Fixed initialization handler logging

close #188

Summary by CodeRabbit

  • New Features

    • Improved notification control during initialization to prevent protocol violations. Notifications are now only enabled after the system is fully initialized.
  • Bug Fixes

    • Console and standard output messages during initialization are now correctly redirected to standard error, ensuring protocol compliance.
  • Chores

    • Enhanced logging to provide clearer status updates during the initialization phase.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 21, 2025

Walkthrough

A controlled initialization state was added to the server's transport layer, delaying JSON-RPC notifications until after the MCP protocol handshake is complete. Console and stdout outputs are redirected to stderr during this phase. Once initialized, notifications are enabled. Logging during initialization was adjusted to avoid protocol violations.

Changes

File(s) Change Summary
src/custom-stdio.ts Added initialization state to FilteredStdioServerTransport, with methods to enable and check notifications. Console and stdout output now conditionally emit JSON-RPC notifications or fallback to stderr logging based on initialization state.
src/index.ts Added server oninitialized event handler to enable notifications after MCP handshake, and log status to stderr.
src/server.ts Changed initialization log from console.log to process.stderr.write to avoid protocol violations.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Server
    participant FilteredStdioTransport

    Client->>Server: Connects and sends initialization request
    Server->>FilteredStdioTransport: (Notifications disabled)
    Server-->>Client: Sends initialization response
    Client->>Server: Sends "initialized" notification
    Server->>FilteredStdioTransport: enableNotifications()
    Note right of FilteredStdioTransport: Notifications enabled
    Server->>FilteredStdioTransport: Subsequent logs/outputs
    FilteredStdioTransport-->>Client: Sends JSON-RPC notifications
Loading

Estimated code review effort

2 (30–90 minutes)

Possibly related PRs

  • Fix for cursor #178: Implements the initial wrapping of console and stdout output into JSON-RPC notifications, which is directly extended and modified in this PR to add initialization gating.

Poem

Before the handshake, logs quietly creep,
To stderr they wander, not a peep.
But once we’re ready, with a hop and a cheer,
Notifications burst forth—protocol clear!
A bunny approves, with a wiggle and grin,
Initialization’s fixed—let the messages begin! 🐇


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 445cf7f and 0bc4c25.

📒 Files selected for processing (3)
  • src/custom-stdio.ts (3 hunks)
  • src/index.ts (1 hunks)
  • src/server.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: tillahoffmann
PR: wonderwhy-er/DesktopCommanderMCP#137
File: src/index.ts:160-162
Timestamp: 2025-05-25T16:02:52.184Z
Learning: In stateless MCP server implementations using the modelcontextprotocol/typescript-sdk, the correct pattern is to call both transport.close() and server.close() in the request close event handler. This is the official recommended approach according to the documentation for ensuring complete isolation between requests and preventing request ID collisions in concurrent scenarios.
Learnt from: tillahoffmann
PR: wonderwhy-er/DesktopCommanderMCP#137
File: src/index.ts:160-162
Timestamp: 2025-05-25T16:02:52.184Z
Learning: In stateless MCP server implementations using the modelcontextprotocol/typescript-sdk, the pattern of calling both transport.close() and server.close() in the request close event handler is the official recommended approach according to the documentation at https://github.com/modelcontextprotocol/typescript-sdk.
src/index.ts (2)

Learnt from: tillahoffmann
PR: #137
File: src/index.ts:160-162
Timestamp: 2025-05-25T16:02:52.184Z
Learning: In stateless MCP server implementations using the modelcontextprotocol/typescript-sdk, the correct pattern is to call both transport.close() and server.close() in the request close event handler. This is the official recommended approach according to the documentation for ensuring complete isolation between requests and preventing request ID collisions in concurrent scenarios.

Learnt from: tillahoffmann
PR: #137
File: src/index.ts:160-162
Timestamp: 2025-05-25T16:02:52.184Z
Learning: In stateless MCP server implementations using the modelcontextprotocol/typescript-sdk, the pattern of calling both transport.close() and server.close() in the request close event handler is the official recommended approach according to the documentation at https://github.com/modelcontextprotocol/typescript-sdk.

src/custom-stdio.ts (2)

Learnt from: tillahoffmann
PR: #137
File: src/index.ts:160-162
Timestamp: 2025-05-25T16:02:52.184Z
Learning: In stateless MCP server implementations using the modelcontextprotocol/typescript-sdk, the correct pattern is to call both transport.close() and server.close() in the request close event handler. This is the official recommended approach according to the documentation for ensuring complete isolation between requests and preventing request ID collisions in concurrent scenarios.

Learnt from: tillahoffmann
PR: #137
File: src/index.ts:160-162
Timestamp: 2025-05-25T16:02:52.184Z
Learning: In stateless MCP server implementations using the modelcontextprotocol/typescript-sdk, the pattern of calling both transport.close() and server.close() in the request close event handler is the official recommended approach according to the documentation at https://github.com/modelcontextprotocol/typescript-sdk.

🧬 Code Graph Analysis (1)
src/index.ts (1)
src/server.ts (1)
  • server (56-69)
🔇 Additional comments (6)
src/custom-stdio.ts (4)

27-27: LGTM: Clean initialization state management.

The isInitialized flag provides a clear, simple mechanism to control when JSON-RPC notifications are enabled, preventing protocol violations during the MCP handshake phase.


56-66: LGTM: Well-designed notification control API.

The enableNotifications() method and isNotificationsEnabled getter provide a clean interface for managing the initialization state. The stderr logging helps with debugging the initialization flow.


69-108: LGTM: Proper conditional notification handling.

The console method overrides correctly implement the dual behavior:

  • During initialization: Direct stderr output with clear prefixes to avoid protocol violations
  • After initialization: Standard JSON-RPC log notifications

This elegantly solves the core protocol violation issue described in the PR objectives.


127-132: LGTM: Consistent stdout filtering with initialization control.

The stdout filtering correctly applies the same initialization gating logic, ensuring non-JSON-RPC output is handled appropriately during both phases. The [STDOUT] prefix provides clear traceability during initialization.

src/server.ts (1)

100-101: LGTM: Consistent initialization phase logging.

Replacing console.log with direct process.stderr.write during the initialization handler prevents triggering the overridden console methods that would attempt to send JSON-RPC notifications before the protocol is fully established. This aligns perfectly with the initialization control implemented in FilteredStdioServerTransport.

src/index.ts (1)

118-125: LGTM: Proper initialization completion coordination.

The oninitialized event handler is correctly placed and timed. This event fires after the client sends the "initialized" notification, marking the completion of the MCP protocol handshake. Calling transport.enableNotifications() at this point ensures JSON-RPC notifications are only enabled when the protocol is fully established, which directly addresses the initialization protocol violation described in the PR objectives.

The stderr debug message provides valuable traceability for the initialization flow.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@wonderwhy-er
Copy link
Copy Markdown
Owner

Hey there, I created a new PR based on yours
#216

Need to fix some issues more in that direction. Closing yours, will merge soon new PR

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.

MCP server initialization fails in v0.2.5: expect initialized response, but received LoggingMessageNotification

2 participants