test: add end-to-end tests for Streamable HTTP Transport session ID and elicitation capabilities#300
Merged
frontegg-david merged 2 commits intorelease/1.0.xfrom Mar 19, 2026
Merged
Conversation
…nd elicitation capabilities
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughStateless HTTP transports now always provide a session-id generator returning Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/sdk/src/common/utils/decide-request-intent.utils.ts (2)
354-356:⚠️ Potential issue | 🟠 MajorStateless SSE branch is shadowed and never selected when streamable is disabled.
At Line 354, the
streamable disabledrule matches allCH_POST_SSErequests withB_STREAMABLE_EN=0, including stateless ones. That prevents the stateless branch at Line 364 from ever running.🐛 Suggested fix
{ - care: CH_MASK | B_STREAMABLE_EN, - match: CH_POST_SSE /* streamable disabled */, + care: CH_MASK | B_STREAMABLE_EN | B_STATELESS_EN, + match: CH_POST_SSE /* streamable disabled + stateless disabled */, outcome: { intent: 'unknown', reason: 'Streamable HTTP disabled.', recommendation: { httpStatus: 405, message: 'Streamable HTTP disabled' }, }, },Also applies to: 362-366
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/sdk/src/common/utils/decide-request-intent.utils.ts` around lines 354 - 356, The branch matching CH_POST_SSE with B_STREAMABLE_EN=0 is currently too broad and shadows the stateless SSE branch; either restrict that match to exclude stateless requests by adding a CH_MASK condition (e.g., require CH_MASK does not include the stateless flag) or reorder the rules so the stateless outcome (the rule referencing the stateless branch) is evaluated before the general CH_POST_SSE + B_STREAMABLE_EN=0 rule; update the match for the streamable-disabled case or move the stateless rule accordingly so the stateless outcome can ever be selected.
385-387:⚠️ Potential issue | 🟠 MajorStateless JSON branch is shadowed by the "JSON mode disabled" rule.
At Line 385, the rule does not account for
B_STATELESS_EN, so it matches before the stateless JSON branch at Line 395 whenstateful=falseandstreamable=false.🐛 Suggested fix
{ - care: CH_MASK | B_STATEFUL_EN | B_STREAMABLE_EN, - match: CH_POST_JSON /* neither enabled */, + care: CH_MASK | B_STATEFUL_EN | B_STREAMABLE_EN | B_STATELESS_EN, + match: CH_POST_JSON /* stateful disabled + streamable disabled + stateless disabled */, outcome: { intent: 'unknown', reason: 'JSON mode disabled.', recommendation: { httpStatus: 405, message: 'JSON mode disabled' }, }, },Also applies to: 393-397
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/sdk/src/common/utils/decide-request-intent.utils.ts` around lines 385 - 387, The rule that disables JSON mode is shadowing the stateless JSON branch because it doesn't check the B_STATELESS_EN flag; update the JSON-disabled rule (the entry using CH_MASK | B_STATEFUL_EN | B_STREAMABLE_EN and match CH_POST_JSON) to also care about B_STATELESS_EN and ensure the rule requires B_STATELESS_EN to be absent (i.e., include B_STATELESS_EN in the care mask and set its match bit to 0 or add an explicit check for !B_STATELESS_EN) so the stateless JSON branch (which depends on B_STATELESS_EN) can match; apply the same fix to the other identical rule block around the CH_POST_JSON branch at the other location.
🧹 Nitpick comments (1)
libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts (1)
87-87: Remove unnecessary non-null assertions after thesessionIdguard.Lines 87 and 105 use
sessionId!inside theif (sessionId)guard block (line 75), where type narrowing already ensuressessionIdis truthy. Remove the!operator to keep null-safety checks meaningful.Suggested fix
- scope.notifications.setClientCapabilities(sessionId!, clientCapabilities); + scope.notifications.setClientCapabilities(sessionId, clientCapabilities); ... - scope.notifications.setClientInfo(sessionId!, { + scope.notifications.setClientInfo(sessionId, {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts` at line 87, Remove the unnecessary non-null assertion usage for sessionId inside the existing if (sessionId) guard: wherever sessionId is used within that guarded block (e.g., the call scope.notifications.setClientCapabilities(sessionId!, clientCapabilities) and the other occurrence at the same guarded scope), drop the trailing "!" so the calls use sessionId directly; this preserves proper type-narrowing and avoids redundant non-null assertions in initialize-request.handler.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@libs/sdk/src/common/utils/decide-request-intent.utils.ts`:
- Around line 354-356: The branch matching CH_POST_SSE with B_STREAMABLE_EN=0 is
currently too broad and shadows the stateless SSE branch; either restrict that
match to exclude stateless requests by adding a CH_MASK condition (e.g., require
CH_MASK does not include the stateless flag) or reorder the rules so the
stateless outcome (the rule referencing the stateless branch) is evaluated
before the general CH_POST_SSE + B_STREAMABLE_EN=0 rule; update the match for
the streamable-disabled case or move the stateless rule accordingly so the
stateless outcome can ever be selected.
- Around line 385-387: The rule that disables JSON mode is shadowing the
stateless JSON branch because it doesn't check the B_STATELESS_EN flag; update
the JSON-disabled rule (the entry using CH_MASK | B_STATEFUL_EN |
B_STREAMABLE_EN and match CH_POST_JSON) to also care about B_STATELESS_EN and
ensure the rule requires B_STATELESS_EN to be absent (i.e., include
B_STATELESS_EN in the care mask and set its match bit to 0 or add an explicit
check for !B_STATELESS_EN) so the stateless JSON branch (which depends on
B_STATELESS_EN) can match; apply the same fix to the other identical rule block
around the CH_POST_JSON branch at the other location.
---
Nitpick comments:
In `@libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts`:
- Line 87: Remove the unnecessary non-null assertion usage for sessionId inside
the existing if (sessionId) guard: wherever sessionId is used within that
guarded block (e.g., the call
scope.notifications.setClientCapabilities(sessionId!, clientCapabilities) and
the other occurrence at the same guarded scope), drop the trailing "!" so the
calls use sessionId directly; this preserves proper type-narrowing and avoids
redundant non-null assertions in initialize-request.handler.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4c322f8d-fed2-4cf2-aa44-fdb1e693d004
📒 Files selected for processing (6)
apps/e2e/demo-e2e-elicitation/e2e/streamable-http-transport.e2e.spec.tslibs/sdk/src/common/utils/decide-request-intent.utils.tslibs/sdk/src/transport/adapters/__tests__/transport.streamable-http.adapter.spec.tslibs/sdk/src/transport/adapters/streamable-http-transport.tslibs/sdk/src/transport/adapters/transport.streamable-http.adapter.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts
Contributor
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 100 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-03-19T22:22:16.399Z |
…nd improve client capabilities setting
Contributor
Cherry-pick CreatedA cherry-pick PR to Please review and merge if this change should also be in If the cherry-pick is not needed, close the PR. |
This was referenced Mar 25, 2026
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.
Summary by CodeRabbit
Bug Fixes
Tests