Skip to content

fix: use custom session id generator when provided#715

Merged
ezynda3 merged 3 commits intomark3labs:mainfrom
FlameHost10:fix/custom-session-id-generator
Feb 15, 2026
Merged

fix: use custom session id generator when provided#715
ezynda3 merged 3 commits intomark3labs:mainfrom
FlameHost10:fix/custom-session-id-generator

Conversation

@FlameHost10
Copy link
Contributor

@FlameHost10 FlameHost10 commented Feb 10, 2026

Description

TThis PR adds support for a user-provided custom session ID generator.

  1. In sse.go, a custom session ID generator is now used when configured; the default generator is used only if none is provided.

  2. In streamable_http.go, handleGet now generates session IDs using the custom generator instead of always using a UUID.

These changes ensure consistent and configurable session ID generation across both SSE and HTTP streams.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • MCP spec compatibility implementation
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring (no functional changes)
  • Performance improvement
  • Tests only (no functional changes)
  • Other (please describe):

Checklist

  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly

Additional Information

N/A

Summary by CodeRabbit

  • New Features

    • Configurable session ID generation for server-sent events, with a default UUID fallback.
    • Consistent honoring of custom session ID generators across request flows (GET/POST).
  • Bug Fixes

    • Improved error handling when custom session ID generation fails or returns an empty ID, returning an appropriate HTTP error.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 10, 2026

Walkthrough

Adds a configurable session ID generator to the SSE server (new SessionIDGenFunc, option, and default UUID generator) and updates streamable HTTP GET handling to use the resolved SessionIdManager for session ID generation when absent; SSE now returns HTTP 500 on generator error or empty ID.

Changes

Cohort / File(s) Summary
SSE session ID generator
server/sse.go
Adds type SessionIDGenFunc func(ctx context.Context, r *http.Request) (string, error), a sessionIDGenFunc field on SSEServer, and WithSessionIDGenerator option; sets default to UUID generator in NewSSEServer; handleSSE uses s.sessionIDGenFunc(...) and returns 500 on error or empty ID.
Streamable HTTP session handling
server/streamable_http.go
GET handler now generates a session ID via the resolved SessionIdManager when none is provided (honors custom session ID generators across GET/POST flows); updated comments to reflect MCP guidance.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

area: mcp spec

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: implementing support for custom session ID generators when provided, which aligns with the core modifications in both sse.go and streamable_http.go.
Description check ✅ Passed The description covers the main changes, specifies the type of change (bug fix), and completes the provided checklist. However, no tests were added despite claiming this is a bug fix, and the documentation was not updated.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
server/sse.go (1)

428-436: Consider logging the generator error before returning HTTP 500.

The error from sessionIDGenFunc is discarded silently, making it hard to diagnose failures in custom generators. A log.Printf (consistent with the rest of this file) before the http.Error would help operators troubleshoot.

🔧 Suggested improvement
 	sessionID, err := s.sessionIDGenFunc(r.Context(), r)
 	if err != nil {
+		log.Printf("session ID generation failed: %v", err)
 		http.Error(w, "Failed to create session ID", http.StatusInternalServerError)
 		return
 	}

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@server/sse.go`:
- Around line 428-432: The custom sessionIDGenFunc may return an empty string
which creates a session key of "" that handleMessage ignores; after calling
sessionIDGenFunc (the sessionID variable) add validation that sessionID is
non-empty and treat an empty value as an error (return http.Error with a 500
status and descriptive message), and optionally log the failure; update the code
paths that store sessions (where sessions are inserted using sessionID) to rely
on this validation so no session with key "" is ever created and handleMessage
continues to reject missing sessionId as intended.
🧹 Nitpick comments (1)
server/sse.go (1)

428-432: Consider logging the underlying error for observability.

The error returned by sessionIDGenFunc is silently discarded — the client gets a generic 500 but operators have no way to diagnose why the generator failed. A single log.Printf would match the logging style already used elsewhere in this file (e.g., line 603, 617).

💡 Suggested improvement
 	sessionID, err := s.sessionIDGenFunc(r.Context(), r)
 	if err != nil {
+		log.Printf("failed to generate session ID: %v", err)
 		http.Error(w, "Failed to create session ID", http.StatusInternalServerError)
 		return
 	}

Fedor Bushlya and others added 2 commits February 10, 2026 14:13
add sessionID check

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@ezynda3 ezynda3 merged commit 962f31b into mark3labs:main Feb 15, 2026
3 of 4 checks 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.

2 participants