Skip to content

Fix SSE/JSON race condition in session capability injection#4075

Merged
ChrisJBurns merged 1 commit intomainfrom
workaround-session-mcp-go
Mar 10, 2026
Merged

Fix SSE/JSON race condition in session capability injection#4075
ChrisJBurns merged 1 commit intomainfrom
workaround-session-mcp-go

Conversation

@aponcedeleonch
Copy link
Member

Summary

The mcp-go SDK's AddSessionTools / AddSessionResources methods send notifications through the session's notification channel. During OnRegisterSession, the notification-listening goroutine from the initialize request has already exited, so these notifications accumulate as stale messages and corrupt the next HTTP response — causing a race between the SSE upgrade and JSON response.

This change bypasses the SDK notification mechanism during session registration by writing tools and resources directly to the session object via SetSessionTools / SetSessionResources, which sets capabilities without triggering notifications.

  • Replace all AddSessionTools / AddSessionResources calls during registration with direct session setters
  • Add setSessionToolsDirect and setSessionResourcesDirect helper functions that merge capabilities into the session
  • Register resource capabilities explicitly at server creation (the implicit registration in AddSessionResources is now bypassed)

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)

Special notes for reviewers

This is a workaround for a limitation in the mcp-go SDK where session registration hooks cannot safely use AddSessionTools / AddSessionResources because the notification goroutine is not active during that lifecycle phase. The direct session setter approach (SetSessionTools / SetSessionResources) achieves the same capability injection without side effects. A future SDK update may provide a cleaner API for this use case.

🤖 Generated with Claude Code

Bypass the mcp-go SDK notification mechanism during session registration
to prevent stale notifications from corrupting HTTP responses. During
OnRegisterSession, the notification-listening goroutine from the
initialize request has already exited, so notifications sent by
AddSessionTools/AddSessionResources accumulate in the channel and race
with the next request's response writer.

Replace AddSessionTools/AddSessionResources calls with direct
SetSessionTools/SetSessionResources on the session object, which sets
capabilities without triggering notifications. Also register resource
capabilities explicitly at server creation since the implicit
registration in AddSessionResources is now bypassed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added the size/XS Extra small PR: < 100 lines changed label Mar 10, 2026
@codecov
Copy link

codecov bot commented Mar 10, 2026

Codecov Report

❌ Patch coverage is 57.14286% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.49%. Comparing base (5fa6252) to head (a22a063).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
pkg/vmcp/server/server.go 57.14% 7 Missing and 8 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4075      +/-   ##
==========================================
- Coverage   68.67%   68.49%   -0.19%     
==========================================
  Files         446      446              
  Lines       45433    45574     +141     
==========================================
+ Hits        31203    31216      +13     
- Misses      11818    11944     +126     
- Partials     2412     2414       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ChrisJBurns ChrisJBurns merged commit 4bc341c into main Mar 10, 2026
36 checks passed
@ChrisJBurns ChrisJBurns deleted the workaround-session-mcp-go branch March 10, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XS Extra small PR: < 100 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants