fix: Add session settings#42
Open
jasonjgardner wants to merge 1 commit into
Open
Conversation
Contributor
WalkthroughSession management improved with extended 30-minute inactivity timeout and ping-failure handling removed in favor of timeout-based cleanup. HTTP session initialization refactored to validate initialize requests separately and return proper error codes for missing sessions. Keep-alive ping now uses explicit JSON-RPC request. Documentation and metadata updated to align with version 1.6.1 release. ChangesSession Management and HTTP Handling
Sequence DiagramsequenceDiagram
participant Client
participant HTTPHandler as HTTP Handler
participant SessionGate as Session Gating
participant NewSession as newSession Closure
participant SessionServer as SessionServer
participant SessionMgr as SessionManager
Client->>HTTPHandler: POST with/without Mcp-Session-Id
HTTPHandler->>SessionGate: check if session exists
alt Session Header Valid
SessionGate->>SessionGate: session found
SessionGate->>Client: 200 (existing session)
else Session Header Missing/Invalid
SessionGate->>SessionGate: no active session
alt Request is initialize
SessionGate->>NewSession: create newSession closure
NewSession->>SessionServer: initialize server
SessionServer->>NewSession: return sessionServer
NewSession->>SessionMgr: getClientVersion from<br/>sessionServer.server
SessionServer->>SessionServer: connect transport
NewSession->>NewSession: assign transport
Client->>Client: session initialized
else Non-initialize Request
SessionGate->>Client: 400 error<br/>(session required)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Preview DeploymentYour changes have been deployed for preview: Plugin URL: Load in Blockbench via File > Plugins > Load Plugin from URL. This preview will be automatically cleaned up when the PR is closed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@index.ts`:
- Around line 71-74: Startup must perform a one-time migration to rewrite the
persisted mcp_session_timeout when it still equals the old broken default so
upgrades adopt the new timeout; before you call
toFiniteNumber(Settings.get("mcp_session_timeout"), 30) in index.ts, read the
raw value via Settings.get("mcp_session_timeout"), detect the old-broken default
numeric/string sentinel (the value used previously), and if matched call
Settings.set("mcp_session_timeout", <newDefault>) to overwrite it; then use
toFiniteNumber on the (possibly updated) Settings.get result. Reference the
existing Settings.get/Settings.set calls and the sessionTimeoutMin assignment in
index.ts and the setting declaration in ui/settings.ts when implementing this
migration.
In `@server/net.ts`:
- Around line 72-91: The isInitializeRequestBody helper currently treats any
object with method === 'initialize' as an initialization request; change it to
validate a real JSON-RPC 2.0 request by parsing the body and ensuring the parsed
value is a non-array object with jsonrpc === '2.0', method === 'initialize', and
a present request id (string or number) so notifications/malformed envelopes are
rejected; update isInitializeRequestBody to perform these checks (keep the
try/catch and return false on parse errors) so only a proper JSON-RPC initialize
request triggers session setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 695c967f-fe4a-41fa-84c5-66db0cce5a08
📒 Files selected for processing (7)
README.mdindex.tslib/sessions.tspackage.jsonprompts/manifest.jsonserver/net.tsui/settings.ts
Comment on lines
+72
to
+91
| /** | ||
| * Whether an HTTP request carries an MCP InitializeRequest. Only initialize | ||
| * requests may create a new session — anything else without a session ID is | ||
| * a client error, not a new connection. Per spec an InitializeRequest must | ||
| * not be part of a JSON-RPC batch, so only a sole non-batched message counts. | ||
| */ | ||
| function isInitializeRequestBody (method: string, body: string): boolean { | ||
| if (method !== 'POST' || !body) return false | ||
| try { | ||
| const parsed: unknown = JSON.parse(body) | ||
| if (Array.isArray(parsed)) return false | ||
| return ( | ||
| typeof parsed === 'object' && | ||
| parsed !== null && | ||
| (parsed as { method?: unknown }).method === 'initialize' | ||
| ) | ||
| } catch { | ||
| return false | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Require a real JSON-RPC request before creating a session.
Line 78 currently accepts any object with method: "initialize", including notifications or malformed envelopes with no request id. That means invalid traffic skips the 400 path and still enters the expensive per-session setup branch. Tighten this helper to require a proper JSON-RPC initialize request.
Suggested diff
function isInitializeRequestBody (method: string, body: string): boolean {
if (method !== 'POST' || !body) return false
try {
const parsed: unknown = JSON.parse(body)
if (Array.isArray(parsed)) return false
- return (
- typeof parsed === 'object' &&
- parsed !== null &&
- (parsed as { method?: unknown }).method === 'initialize'
- )
+ if (typeof parsed !== 'object' || parsed === null) return false
+ const request = parsed as {
+ jsonrpc?: unknown
+ id?: unknown
+ method?: unknown
+ }
+ return (
+ request.jsonrpc === '2.0' &&
+ request.method === 'initialize' &&
+ request.id !== undefined &&
+ request.id !== null
+ )
} catch {
return false
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/net.ts` around lines 72 - 91, The isInitializeRequestBody helper
currently treats any object with method === 'initialize' as an initialization
request; change it to validate a real JSON-RPC 2.0 request by parsing the body
and ensuring the parsed value is a non-array object with jsonrpc === '2.0',
method === 'initialize', and a present request id (string or number) so
notifications/malformed envelopes are rejected; update isInitializeRequestBody
to perform these checks (keep the try/catch and return false on parse errors) so
only a proper JSON-RPC initialize request triggers session setup.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #41
Summary by CodeRabbit
Documentation
Improvements
Chores