feat: update tool description retrieval to handle undefined cases and improve schema handling - #515
Conversation
… improve schema handling
|
Important Review skippedToo many files! This PR contains 206 files, which is 56 over the limit of 150. To get a review, narrow the scope: Upgrade to Pro+ to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (206)
You can disable this status message by setting the 📝 WalkthroughWalkthroughThe change adds MCP revision ChangesMCP 2026 protocol support
Internal version normalization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant HttpRequestFlow
participant HandleMcp2026Flow
participant Dispatcher
participant Scope
Client->>HttpRequestFlow: Send MCP 2026 HTTP request
HttpRequestFlow->>HandleMcp2026Flow: Route authorized request
HandleMcp2026Flow->>HandleMcp2026Flow: Validate headers and body
HandleMcp2026Flow->>Dispatcher: Dispatch JSON-RPC method
Dispatcher->>Scope: Execute MCP method
Scope-->>Dispatcher: Return result or protocol signal
Dispatcher-->>HandleMcp2026Flow: Build JSON-RPC response or SSE stream
HandleMcp2026Flow-->>Client: Return HTTP response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces support for the MCP 2026-07-28 protocol revision in @frontmcp/sdk (stateless, per-request negotiation, MRTR, and subscriptions/listen), adds a dedicated E2E conformance app/suite for the new revision while preserving legacy behavior, and adds automation to keep internal @frontmcp/* version pins consistent across the repo.
Changes:
- Add a new MCP 2026-07-28 transport pipeline (request validation, dispatcher, result decoration, MRTR, subscriptions stream) and route eligible requests to it.
- Add a new E2E fixture server + conformance tests for 2026-07-28, plus a new reserved E2E port range.
- Add
scripts/normalize-internal-versions.mjs(plus CI gating) and bump internal workspace versions/pins to1.5.7with lockfile refresh.
Reviewed changes
Copilot reviewed 80 out of 81 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Refresh lockfile for internal version bumps |
| scripts/normalize-internal-versions.test.mjs | Add tests for version normalization script |
| scripts/normalize-internal-versions.mjs | New script to normalize internal version pins |
| plugins/plugin-skilled-openapi/package.json | Bump version and internal deps to 1.5.7 |
| plugins/plugin-remember/package.json | Bump version and internal deps to 1.5.7 |
| plugins/plugin-feature-flags/package.json | Bump version and internal deps to 1.5.7 |
| plugins/plugin-dashboard/package.json | Bump version and internal deps to 1.5.7 |
| plugins/plugin-codecall/package.json | Bump version and internal deps to 1.5.7 |
| plugins/plugin-cache/package.json | Bump version and internal deps to 1.5.7 |
| plugins/plugin-approval/package.json | Bump version and internal deps to 1.5.7 |
| libs/utils/package.json | Bump version/peer pin to 1.5.7 |
| libs/uipack/package.json | Bump version/peer pin to 1.5.7 |
| libs/ui/package.json | Bump version and optional dep pin |
| libs/testing/src/server/port-registry.ts | Reserve port range for new protocol E2E app |
| libs/testing/package.json | Bump version and internal deps/peers to 1.5.7 |
| libs/storage-sqlite/package.json | Bump version and internal deps/peers to 1.5.7 |
| libs/skills/package.json | Bump version to 1.5.7 |
| libs/sdk/src/transport/transport.registry.ts | Register the new MCP 2026 flow |
| libs/sdk/src/transport/mcp-handlers/call-tool-request.handler.ts | Preserve MRTR signals through handler error mapping |
| libs/sdk/src/transport/mcp-2026/subscriptions.ts | Implement subscriptions/listen SSE stream builder |
| libs/sdk/src/transport/mcp-2026/result-decorator.ts | Add 2026 result envelope fields + cache hints |
| libs/sdk/src/transport/mcp-2026/request-validation.ts | 2026 request/header admission + validation |
| libs/sdk/src/transport/mcp-2026/protocol-2026.constants.ts | 2026 protocol constants (headers, versions, cacheables) |
| libs/sdk/src/transport/mcp-2026/mrtr.ts | MRTR exchange state encode/decode + exchange logic |
| libs/sdk/src/transport/mcp-2026/index.ts | Barrel exports for the 2026 transport module |
| libs/sdk/src/transport/mcp-2026/header-codec.ts | Header base64 sentinel encode/decode + comparisons |
| libs/sdk/src/transport/mcp-2026/dispatcher.ts | 2026 dispatcher reusing shared handlers + decorations |
| libs/sdk/src/transport/mcp-2026/discover.ts | Implement server/discover result/capabilities projection |
| libs/sdk/src/transport/mcp-2026/tests/result-decorator.spec.ts | Unit tests for result decoration/caching |
| libs/sdk/src/transport/mcp-2026/tests/request-validation.spec.ts | Unit tests for header/body validation |
| libs/sdk/src/transport/mcp-2026/tests/mrtr.spec.ts | Unit tests for MRTR exchange/state |
| libs/sdk/src/transport/mcp-2026/tests/header-codec.spec.ts | Unit tests for header codec rules |
| libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts | New HTTP flow for protocol 2026-07-28 |
| libs/sdk/src/tool/flows/call-tool.flow.ts | Ensure MRTR signals aren’t wrapped as tool failures |
| libs/sdk/src/scope/flows/http.request.flow.ts | Route claimed requests to the MCP 2026 flow |
| libs/sdk/src/errors/mrtr.error.ts | Add MRTR control-flow signal error types |
| libs/sdk/src/errors/index.ts | Export MRTR signal errors |
| libs/sdk/src/elicitation/helpers/elicit.helper.ts | Route elicitation through MRTR when present |
| libs/sdk/src/context/frontmcp-context.ts | Store/get MRTR exchange on request context |
| libs/sdk/src/common/utils/decide-request-intent.utils.ts | Add mcp-2026 intent type |
| libs/sdk/package.json | Bump version and internal deps/peers to 1.5.7 |
| libs/react/package.json | Bump version and internal deps/peers to 1.5.7 |
| libs/protocol/src/types-2026.ts | Add protocol 2026-07-28 types/constants in boundary |
| libs/protocol/src/index.ts | Re-export protocol 2026 types/constants |
| libs/protocol/package.json | Bump version to 1.5.7 |
| libs/plugins/package.json | Bump version and plugin pins to 1.5.7 |
| libs/observability/package.json | Bump version and peer pins to 1.5.7 |
| libs/nx-plugin/package.json | Bump version and internal dep pin to 1.5.7 |
| libs/lazy-zod/package.json | Bump version to 1.5.7 |
| libs/guard/package.json | Bump version and internal deps/peers to 1.5.7 |
| libs/edge/package.json | Bump version and peer pins to 1.5.7 |
| libs/di/package.json | Bump version and peer pin to 1.5.7 |
| libs/cli/package.json | Bump version and internal deps to 1.5.7 |
| libs/auth/package.json | Bump version and internal deps/peers to 1.5.7 |
| libs/adapters/package.json | Bump version and internal deps/peers to 1.5.7 |
| apps/e2e/demo-e2e-protocol-2026/webpack.config.js | Add webpack config for new E2E server |
| apps/e2e/demo-e2e-protocol-2026/tsconfig.json | Add TS project references for E2E app |
| apps/e2e/demo-e2e-protocol-2026/tsconfig.e2e.json | Add TS config for E2E tests |
| apps/e2e/demo-e2e-protocol-2026/tsconfig.app.json | Add TS config for server app build |
| apps/e2e/demo-e2e-protocol-2026/src/main.ts | New E2E server entrypoint targeting 2026 support |
| apps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/region-query.tool.ts | Tool fixture for x-mcp-header validation |
| apps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/echo.tool.ts | Basic tool fixture for stateless calls |
| apps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/confirm.tool.ts | Tool fixture for MRTR elicitation round-trips |
| apps/e2e/demo-e2e-protocol-2026/src/apps/proto/resources/config.resource.ts | Resource fixture for read/list coverage |
| apps/e2e/demo-e2e-protocol-2026/src/apps/proto/prompts/greeting.prompt.ts | Prompt fixture for prompt list/get coverage |
| apps/e2e/demo-e2e-protocol-2026/src/apps/proto/index.ts | Register proto app fixtures (tools/resources/prompts) |
| apps/e2e/demo-e2e-protocol-2026/project.json | Nx project definition for new E2E app |
| apps/e2e/demo-e2e-protocol-2026/jest.e2e.config.ts | Jest config for the new E2E suite |
| apps/e2e/demo-e2e-protocol-2026/e2e/subscriptions-listen.e2e.spec.ts | E2E coverage for subscriptions/listen |
| apps/e2e/demo-e2e-protocol-2026/e2e/stateless-requests.e2e.spec.ts | E2E coverage for stateless request rules |
| apps/e2e/demo-e2e-protocol-2026/e2e/request-headers.e2e.spec.ts | E2E coverage for header/body agreement |
| apps/e2e/demo-e2e-protocol-2026/e2e/mrtr.e2e.spec.ts | E2E coverage for MRTR input_required |
| apps/e2e/demo-e2e-protocol-2026/e2e/helpers/mcp-2026-client.ts | Raw client helpers for 2026 wire format tests |
| apps/e2e/demo-e2e-protocol-2026/e2e/errors-and-removals.e2e.spec.ts | E2E coverage for removed methods + errors |
| apps/e2e/demo-e2e-protocol-2026/e2e/discover.e2e.spec.ts | E2E coverage for server/discover |
| apps/e2e/demo-e2e-protocol-2026/e2e/cacheable-results.e2e.spec.ts | E2E coverage for CacheableResult fields |
| apps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.ts | E2E coverage asserting legacy revisions unchanged |
| .github/workflows/push.yml | Add CI job to gate internal version drift |
| .github/workflows/publish-release.yml | Use normalize script for release workflows |
| .github/workflows/create-release-branch.yml | Use normalize script for release-branch workflows |
| .github/workflows/cherry-pick-prompt.yml | Auto-normalize internal pins after cherry-picks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…-28 with new tools and error handling
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 116 out of 118 changed files in this pull request and generated no new comments.
Suppressed comments (5)
libs/sdk/src/transport/mcp-2026/header-codec.ts:34
decodeHeaderValue/encodeHeaderValueuseBufferunconditionally. This will throw in Web/edge runtimes (and contradicts the goal of reusing the 2026 client/transport code across Node and Webfetchenvironments). GuardBufferusage and fall back toatob/btoa+TextEncoder/TextDecoderso this codec is runtime-agnostic.
libs/sdk/src/transport/mcp-2026/client/header-params.ts:44validateHeaderParamssaysx-mcp-headernames must be case-insensitively unique, but the duplicate check uses the rawnamestring. That allows duplicates like "Region" and "region" to slip through and produce ambiguous/duplicateMcp-Param-*headers.
libs/sdk/src/remote-mcp/mcp-client.service.ts:818tryConnect2026returns aMcpClientConnectionwithtransportset toundefinedat runtime, but theMcpClientConnectioninterface requires a concreteTransport. This breaks the type contract for any downstream consumer that readsconnection.transport(and can cause runtime errors). Consider makingtransportoptional for 2026 connections or introducing a separate connection type/union rather than castingundefinedintoTransport.
libs/sdk/src/auth/instances/instance.local-primary-auth.ts:1194- Issuer comparisons here are stricter than the RFC 9207
issvalidation logic below: a harmless formatting change like adding/removing a trailing slash would trigger credential discard. To avoid unnecessary token invalidation, normalize trailing slashes before comparing (matchingvalidateAuthorizationIssuer).
.github/workflows/push.yml:42 - The PR title/description mention a narrow change (“tool description retrieval…schema handling”), but this PR also adds full MCP protocol 2026-07-28 support, new E2E fixtures, CI workflow changes, and repo-wide version pin normalization. This makes the PR metadata misleading for reviewers/release notes; please update the PR title/description (or split) to reflect the actual scope.
# Guards against a release branch's version pins leaking onto another line (the
# usual source is a cherry-pick from release/* onto main). When they do, the very
# first `yarn install --immutable` in every other job dies with a bare YN0028
# "The lockfile would have been modified by this install" — which says nothing
# about the actual cause. This job needs no install, so it fails first and names
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (15)
.github/workflows/push.yml-49-50 (1)
49-50: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winSet
persist-credentials: falseon this read-only checkout.This job only reads
package.jsonfiles to check version consistency; no step needs push or remote-write credentials afterward. Static analysis flags the checkout for not disabling credential persistence. Addpersist-credentials: falseto reduce the residual credential exposure on the runner.🔒 Proposed fix
- name: Checkout code - uses: actions/checkout@v6 + uses: actions/checkout@v6 + with: + persist-credentials: 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 @.github/workflows/push.yml around lines 49 - 50, Update the “Checkout code” actions/checkout@v6 step to set persist-credentials to false, keeping the read-only version-consistency workflow unchanged.Source: Linters/SAST tools
.github/workflows/push.yml-38-60 (1)
38-60: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGate dependent jobs on
version-consistency.
lint,build,discover-e2e, anddiscover-unitcurrently depend only onsetup, so they run in parallel withversion-consistency. Addversion-consistencyto theirneedslists so downstream test and coverage jobs receive the fail-first behavior transitively.🤖 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 @.github/workflows/push.yml around lines 38 - 60, Update the job definitions for lint, build, discover-e2e, and discover-unit to include version-consistency alongside setup in their needs lists, ensuring they wait for the consistency check before running and preserve transitive gating for downstream test and coverage jobs.apps/e2e/demo-e2e-protocol-2026/e2e/mrtr.e2e.spec.ts-82-98 (1)
82-98: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAssert the interim result before you destructure
inputRequests.If the first call does not return
input_required,Object.keys(interim.inputRequests)throws aTypeErrorand the failure message hides the real cause.keycan also beundefined, and the computed property then becomes the literal string"undefined", which sends a valid-looking but wrong payload on the retry. Add explicit assertions first.Apply the same guard in the declined-elicitation test at Lines 107-120.🛡️ Proposed guard
const first = await mcp2026Fetch(server.info.baseUrl, { ...ELICITING_CALL, id: 5 }); const { result: interim } = first.json(); - const [key] = Object.keys(interim.inputRequests); + expect(interim.resultType).toBe('input_required'); + const [key] = Object.keys(interim.inputRequests ?? {}); + expect(typeof key).toBe('string');Also applies to: 107-120
🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr.e2e.spec.ts` around lines 82 - 98, In the `completes the call when the client retries with inputResponses` and declined-elicitation tests, assert that the first response result is the expected `input_required` interim shape before accessing `interim.inputRequests`. Also validate that `inputRequests` contains a key before constructing the retry payload, so an absent key fails explicitly rather than producing an `"undefined"` response key.apps/e2e/demo-e2e-protocol-2026/jest.e2e.config.ts-39-40 (1)
39-40: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAlign the Jest coverage directory with the Nx test output.
jest.e2e.coverage.preset.jsdoes not overridecoverageDirectory, but Jest writes tocoverage/e2e/demo-e2e-protocol-2026whileproject.jsondeclarescoverage/apps/e2e/demo-e2e-protocol-2026. Make both paths identical.🤖 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 `@apps/e2e/demo-e2e-protocol-2026/jest.e2e.config.ts` around lines 39 - 40, Update the coverageDirectory setting in the Jest configuration to exactly match the coverage output path declared in project.json, using the existing e2eCoveragePreset context without changing unrelated Jest settings.Source: Path instructions
apps/e2e/demo-e2e-protocol-2026/e2e/cacheable-results.e2e.spec.ts-61-67 (1)
61-67: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename the test to match what it asserts.
The title states that the list is scoped as
private, but the server runs in public mode and the assertion only checks thatcacheScopeis defined. Rename the test, for example to "always sets an explicit cacheScope on tools/list".🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/cacheable-results.e2e.spec.ts` around lines 61 - 67, Rename the test case currently titled “scopes an authenticated-context list as private” to accurately describe its assertion that tools/list always provides an explicit cacheScope, without changing the test behavior or assertions.apps/e2e/demo-e2e-protocol-2026/e2e/discover.e2e.spec.ts-47-55 (1)
47-55: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winThe test title mentions
instructions, but no assertion covers it.The test only asserts
capabilitiesand itstools,resources, andpromptsmembers. Add an assertion forresult.instructions, or remove "and instructions" from the title.🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/discover.e2e.spec.ts` around lines 47 - 55, Update the test named “returns server capabilities and instructions” to assert that result.instructions is defined, alongside the existing capability assertions; keep the title unchanged and add coverage for the response field it names.apps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.ts-114-114 (1)
114-114: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the non-null assertion on
res.body.The coding guidelines forbid non-null assertions. If the server returns an empty body,
res.body!produces a confusing runtime error instead of a clear assertion failure.🛡️ Proposed fix
- const reader = res.body!.getReader(); + const body = res.body; + if (!body) throw new Error('Legacy /sse response has no body stream'); + const reader = body.getReader();🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.ts` at line 114, Update the reader initialization in the backward-compatibility test to remove the non-null assertion on res.body and explicitly assert or validate that res.body exists before calling getReader(), producing a clear test failure for an empty response body.Source: Coding guidelines
apps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.ts-152-168 (1)
152-168: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe test does not match its title.
The title states "no protocol version at all", but the body still sends
params.protocolVersion: '2025-03-26'. Only theMCP-Protocol-Versionheader is absent. RemoveprotocolVersionfromparamsto test the stated case, or rename the test to describe a missing header.🐛 Proposed fix
- params: { protocolVersion: '2025-03-26', capabilities: {}, clientInfo: { name: 'x', version: '1' } }, + params: { capabilities: {}, clientInfo: { name: 'x', version: '1' } },🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.ts` around lines 152 - 168, Update the test body in the no-protocol-version request case to remove params.protocolVersion, so it actually verifies routing when neither the header nor request parameters provide a protocol version; keep the existing request and status assertion unchanged.apps/e2e/demo-e2e-protocol-2026/e2e/cacheable-results.e2e.spec.ts-39-59 (1)
39-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert
errorbefore readingresult.These tests destructure
resultand read fields from it. If the server returns a JSON-RPC error,resultisundefinedand the test fails with a TypeError instead of a readable assertion. Add theerrorcheck, as thettlMstest at Line 33 already does.🛡️ Proposed fix
test(`${call.method} returns a valid cacheScope`, async ({ server }) => { const res = await mcp2026Fetch(server.info.baseUrl, { ...call, id: 400 + index }); - const { result } = res.json(); + const { result, error } = res.json(); + expect(error).toBeUndefined(); expect(['public', 'private']).toContain(result.cacheScope); });- const { result } = res.json(); + const { result, error } = res.json(); + expect(error).toBeUndefined(); expect(result.ttlMs).toBeUndefined(); expect(result.cacheScope).toBeUndefined();🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/cacheable-results.e2e.spec.ts` around lines 39 - 59, Update both tests around the cacheable and non-cacheable result assertions to validate that the JSON-RPC response has no error before destructuring or reading result fields, matching the existing ttlMs test pattern. Keep the current cacheScope and ttlMs expectations unchanged after the error assertion.libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts-259-269 (1)
259-269: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winThe comment claims a request-scoped id, but the code reuses the session id.
Lines 263-266 state that a request-scoped id is derived so nothing leaks between calls. Line 266 passes
auth.session?.id, which is stable across every request that presents the same authorization. Any shared handler that keys per-request state onsessionIdtherefore shares that state between calls, which is the opposite of the stated guarantee. Either derive a per-request id, or correct the comment to describe the session-scoped behavior.🤖 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 `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts` around lines 259 - 269, Update the authInfo construction in the MCP flow so sessionId is genuinely unique per request rather than reusing auth.session?.id, and use the same request-scoped identifier in extra.sessionId. Preserve the existing token and user fields while keeping the comment accurate about preventing state sharing between calls.libs/sdk/src/transport/mcp-2026/header-codec.ts-66-75 (1)
66-75: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject empty and non-decimal header values in the numeric comparison.
Number('')returns0, so an emptyMcp-Param-*header matches a body value of0.Numberalso accepts' 42 'and'0x2a'as42. An intermediary that routes on the header then disagrees with the server that executes on the body, which is exactly the case this check exists to prevent. Thebooleanbranch on line 72 is also identical to the default branch on line 74 and can be removed.🐛 Proposed fix
export function headerMatchesBodyValue(headerValue: string, bodyValue: unknown): boolean { if (typeof bodyValue === 'number') { - const parsed = Number(headerValue); + if (!/^[+-]?(\d+(\.\d*)?|\.\d+)([eE][+-]?\d+)?$/.test(headerValue.trim())) return false; + const parsed = Number(headerValue.trim()); return Number.isFinite(parsed) && parsed === bodyValue; } - if (typeof bodyValue === 'boolean') { - return headerValue === String(bodyValue); - } return headerValue === String(bodyValue); }🤖 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 `@libs/sdk/src/transport/mcp-2026/header-codec.ts` around lines 66 - 75, Update headerMatchesBodyValue so numeric headers are accepted only when they are non-empty, decimal-formatted values matching the body number; reject whitespace-padded, hexadecimal, and other non-decimal representations instead of relying on Number alone. Remove the redundant boolean-specific branch and retain the shared string comparison fallback for non-numeric values.libs/sdk/src/transport/mcp-2026/header-codec.ts-30-50 (1)
30-50: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winReject invalid UTF-8 payloads in
decodeHeaderValue.
Buffer.toString('utf8')replaces malformed byte sequences with�, so malformed sentinel values are accepted instead of returningundefinedas documented. Decode with fatal UTF-8 validation before returning the value.🤖 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 `@libs/sdk/src/transport/mcp-2026/header-codec.ts` around lines 30 - 50, Update decodeHeaderValue to perform fatal UTF-8 validation on the decoded payload before returning it. Preserve the existing Base64 round-trip check and return undefined for malformed UTF-8 instead of accepting replacement characters.libs/sdk/src/transport/mcp-2026/subscriptions.ts-119-125 (1)
119-125: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe prefix match has no path boundary.
uri.startsWith(watched)treats any URI with the same leading characters as a sub-resource. A client that watchesproto://configthen receives updates forproto://configuration. Require an exact match or a separator after the prefix.🔧 Proposed boundary check
- if (uri && [...watchedUris].some((watched) => uri === watched || uri.startsWith(watched))) { + if ( + uri && + [...watchedUris].some( + (watched) => uri === watched || uri.startsWith(watched.endsWith('/') ? watched : `${watched}/`), + ) + ) { push('notifications/resources/updated', { uri }); }🤖 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 `@libs/sdk/src/transport/mcp-2026/subscriptions.ts` around lines 119 - 125, Update the URI matching condition in the updated-event branch to require an exact watched URI match or a valid path separator immediately after the watched prefix, preventing values such as “proto://configuration” from matching “proto://config”. Preserve the existing push behavior for valid matches.libs/sdk/src/transport/mcp-2026/subscriptions.ts-42-59 (1)
42-59: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
resourceSubscriptionsskips the capability check that the other branches apply.The three list-changed branches check
getCapabilities()before acknowledging.resourceSubscriptionsis acknowledged whenever the client sends a non-empty array. If the resource registry reportssubscribe: false, the server acknowledges per-URI subscriptions it cannot deliver, and the client waits for updates that never arrive. Gate this branch on the same capability the discovery result advertises.🔧 Proposed capability gate
- if (requested.resourceSubscriptions && requested.resourceSubscriptions.length > 0) { + if ( + requested.resourceSubscriptions && + requested.resourceSubscriptions.length > 0 && + scope.resources.getCapabilities().resources?.subscribe + ) { acknowledged.resourceSubscriptions = [...requested.resourceSubscriptions]; }🤖 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 `@libs/sdk/src/transport/mcp-2026/subscriptions.ts` around lines 42 - 59, Update resolveAcknowledgedFilter so resourceSubscriptions is acknowledged only when the resources capability advertises subscribe support, while preserving the existing non-empty array check and URI-copy behavior. Use scope.resources.getCapabilities().resources?.subscribe as the capability gate for this branch.libs/sdk/src/transport/mcp-2026/mrtr.ts-44-50 (1)
44-50: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the client-supplied
action/statusvalue.
toElicitResultcastsresponse['action'] ?? response['status']directly toElicitStatuswithout checking it against the known status values. An unrecognized string from a client passes through unchanged instead of falling back to'cancel'. Validate the value against the knownElicitStatusset before using it, and default to'cancel'otherwise, matching the existing default-on-absence behavior.🛡️ Proposed fix
+const VALID_STATUSES: readonly ElicitStatus[] = ['accept', 'decline', 'cancel']; + function toElicitResult(response: Record<string, unknown>): { status: ElicitStatus; content?: unknown } { - const action = (response['action'] ?? response['status']) as ElicitStatus | undefined; + const raw = response['action'] ?? response['status']; + const action = VALID_STATUSES.includes(raw as ElicitStatus) ? (raw as ElicitStatus) : undefined; return { status: action ?? 'cancel', ...(response['content'] === undefined ? {} : { content: response['content'] }), }; }🤖 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 `@libs/sdk/src/transport/mcp-2026/mrtr.ts` around lines 44 - 50, Update toElicitResult to validate the selected response action/status against the known ElicitStatus values before assigning it to status. Preserve recognized values, but fall back to 'cancel' for absent or unrecognized client-supplied values while leaving content handling unchanged.
🧹 Nitpick comments (17)
.github/workflows/create-release-branch.yml (1)
128-138: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRoute step outputs through
env:before interpolating into the normalize-internal-versions.mjs call. Both sites expand a${{ steps.version.outputs.* }}expression directly into therun:shell body to build the CLI argument. Static analysis flags this template-injection pattern; the values are already validated/derived from trusted internal sources, so treat this as a hardening/consistency fix rather than an active exploit.
.github/workflows/create-release-branch.yml#L128-L138: bindsteps.version.outputs.initial_versionto anenv:var (e.g.VERSION) on the step and callnode scripts/normalize-internal-versions.mjs "$VERSION"..github/workflows/publish-release.yml#L174-L184: bindsteps.version.outputs.versionto anenv:var on the step and callnode scripts/normalize-internal-versions.mjs "$VERSION".🔒 Proposed fix (apply the same pattern to both steps)
- name: Normalize internal `@frontmcp/`* dep ranges shell: bash + env: + VERSION: ${{ steps.version.outputs.initial_version }} run: | set -euo pipefail - node scripts/normalize-internal-versions.mjs "${{ steps.version.outputs.initial_version }}" + node scripts/normalize-internal-versions.mjs "$VERSION"- name: Normalize internal `@frontmcp/`* dep ranges shell: bash + env: + VERSION: ${{ steps.version.outputs.version }} run: | set -euo pipefail - node scripts/normalize-internal-versions.mjs "${{ steps.version.outputs.version }}" + node scripts/normalize-internal-versions.mjs "$VERSION"🤖 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 @.github/workflows/create-release-branch.yml around lines 128 - 138, Route the release version outputs through step-level env variables before invoking normalize-internal-versions.mjs. In .github/workflows/create-release-branch.yml lines 128-138, bind steps.version.outputs.initial_version as VERSION and pass "$VERSION"; apply the same change in .github/workflows/publish-release.yml lines 174-184 using steps.version.outputs.version, preserving the existing normalization behavior.Source: Linters/SAST tools
apps/e2e/demo-e2e-protocol-2026/e2e/errors-and-removals.e2e.spec.ts (2)
108-120: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueStrengthen the log-notification assertion.
The request asks for
text/event-stream, but the assertion passes trivially when the server answers with a JSON body, because a JSON result never contains the stringnotifications/message. Assert the response content type first, then assert the absence of the notification. This keeps the test meaningful if the transport switches to SSE for this call.🧪 Proposed change
expect(res.status).toBe(200); + const contentType = res.headers.get('content-type') ?? ''; // Whether the server answers with JSON or SSE, no log notification may // appear for a request that did not opt in via `_meta` logLevel. expect(res.text).not.toContain('notifications/message'); + if (contentType.includes('text/event-stream')) { + expect(res.text).toContain('data:'); + }🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/errors-and-removals.e2e.spec.ts` around lines 108 - 120, Strengthen the test around mcp2026Fetch by first asserting that the response content type is text/event-stream, then retain the assertion that res.text does not contain notifications/message. Remove the JSON-response caveat so the test fails if the requested SSE transport is not used.
12-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why
tasks/resultandtasks/listare inREMOVED_METHODS.The file header lists the methods that 2026-07-28 removes, and it does not mention the
tasks/*methods. These two entries therefore assert "not implemented", not "removed by the revision". If the SDK later implements tasks, this test fails and reads as a protocol regression. Add a short comment that separates removed methods from not-yet-implemented methods, or split the list.📝 Proposed clarification
const REMOVED_METHODS = [ 'ping', 'logging/setLevel', 'resources/subscribe', 'resources/unsubscribe', 'initialize', + // Not part of 2026-07-28 removals: these are not implemented by this server, + // so they must answer with the same 404 + -32601 shape. 'tasks/result', 'tasks/list', ];🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/errors-and-removals.e2e.spec.ts` around lines 12 - 20, Clarify the purpose of tasks/result and tasks/list in REMOVED_METHODS by separating them from methods removed by the 2026-07-28 revision, either with a short explanatory comment or a distinct list. Preserve their current assertion as not-yet-implemented methods so future task support does not appear to be a protocol removal.apps/e2e/demo-e2e-protocol-2026/e2e/subscriptions-listen.e2e.spec.ts (1)
82-101: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe test name and the assertions do not agree.
The name states that the server omits notification types it cannot honor. The body only checks key membership and value types, so it also passes when the server acknowledges an unsupported type. Assert the specific expectation, for example that
promptsListChangedis absent when the server does not support prompt-list notifications, or rename the test to describe the shape check it performs.🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/subscriptions-listen.e2e.spec.ts` around lines 82 - 101, The test named “omits notification types the server cannot honor” must assert that unsupported notifications are absent, not merely validate acknowledged key shapes. Update the assertions around the acknowledgment from `subscriptions/listen` to explicitly verify `promptsListChanged` is omitted when prompt-list notifications are unsupported; retain the existing allowed-key and value-shape checks as appropriate.apps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.ts (1)
129-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the successful subscribe result, not only "not 404".
expect(res.status).not.toBe(404)passes for a 400 or 500 response. The stated guarantee is thatresources/subscribestill works for pre-2026 clients. Assert status 200 and the absence of a JSON-RPC error.♻️ Proposed strengthening
- expect(res.status).not.toBe(404); + expect(res.status).toBe(200); + const text = await res.text(); + const trimmed = text.trim(); + const payload = /^(event:|data:|:)/.test(trimmed) + ? JSON.parse(parseSseEvents(trimmed).pop() as string) + : JSON.parse(trimmed); + expect(payload.error).toBeUndefined();🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.ts` around lines 129 - 150, Strengthen the assertions in the legacy resources/subscribe test around the fetch call so it requires HTTP status 200 and verifies the response contains no JSON-RPC error. Keep the existing legacy negotiation setup and request unchanged, using the parsed response body to assert successful subscription handling.apps/e2e/demo-e2e-protocol-2026/e2e/helpers/mcp-2026-client.ts (1)
71-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftReplace the
anydefaults withunknownand a typed JSON-RPC envelope.
Mcp2026Response.jsonand theSseStreamHandlemessage APIs useany. The coding guidelines prohibitanywithout strong justification and requireunknownfor generic type defaults. A small envelope type keeps the raw-wire intent of this helper and still gives the suites typed access toresultanderror.♻️ Proposed typing
+export interface JsonRpcResponseBody<TResult = Record<string, unknown>> { + jsonrpc: '2.0'; + id?: string | number | null; + result?: TResult; + error?: { code: number; message: string; data?: unknown }; +} + export interface Mcp2026Response { status: number; headers: Headers; text: string; - json: <T = any>() => T; + json: <T = JsonRpcResponseBody>() => T; }export interface SseStreamHandle { /** Resolves once `predicate` matches a received message, or rejects on timeout. */ - waitFor: (predicate: (msg: any) => boolean, timeoutMs?: number) => Promise<any>; + waitFor: (predicate: (msg: unknown) => boolean, timeoutMs?: number) => Promise<unknown>; /** Every message received so far, in arrival order. */ - received: () => any[]; + received: () => unknown[]; close: () => void; /** Response status + headers of the stream itself. */ status: number; headers: Headers; }Update
mcp2026Fetch(Line 146) and the internalmessages/waitersdeclarations (Lines 201-202) to match.Also applies to: 175-184
🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/helpers/mcp-2026-client.ts` around lines 71 - 76, Replace any-based defaults in Mcp2026Response.json and the SseStreamHandle message APIs with unknown and a shared typed JSON-RPC envelope exposing result and error. Update mcp2026Fetch plus the internal messages and waiters declarations to use the same envelope type, preserving typed access to raw-wire result/error fields without introducing any.Source: Coding guidelines
libs/sdk/src/transport/mcp-2026/protocol-2026.constants.ts (1)
31-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe SDK re-declares contracts that
@frontmcp/protocolalready exports.libs/protocol/src/types-2026.tsowns the 2026 contract surface, but the SDK defines its own copies of two of its members. Each copy can drift from the boundary module that the protocol package documents as the single place to change.
libs/sdk/src/transport/mcp-2026/protocol-2026.constants.ts#L31-L32: replace the literal list withMCP_2026_ADDED_METHODSimported from@frontmcp/protocol.libs/sdk/src/transport/mcp-2026/request-validation.ts#L55-L59: delete the localmetaProtocolVersionhelper and usereadDeclaredProtocolVersionfrom@frontmcp/protocol.🤖 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 `@libs/sdk/src/transport/mcp-2026/protocol-2026.constants.ts` around lines 31 - 32, The SDK duplicates protocol contracts in two locations. In libs/sdk/src/transport/mcp-2026/protocol-2026.constants.ts lines 31-32, import and reuse MCP_2026_ADDED_METHODS from `@frontmcp/protocol` instead of the literal list; in libs/sdk/src/transport/mcp-2026/request-validation.ts lines 55-59, remove the local metaProtocolVersion helper and use readDeclaredProtocolVersion from `@frontmcp/protocol` at its call sites.libs/sdk/src/transport/mcp-2026/request-validation.ts (1)
266-277: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueConsider skipping annotations whose argument value is not a scalar.
headerMatchesBodyValuefalls back toString(bodyValue). If a tool author annotates an object or array property withx-mcp-header, every call that supplies that argument is rejected with-32020, and the client cannot correct it. A scalar check keeps the failure with the schema author instead of the caller.🤖 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 `@libs/sdk/src/transport/mcp-2026/request-validation.ts` around lines 266 - 277, Update the annotation loop around headerMatchesBodyValue to validate that each present bodyValue is scalar before comparing it with the header. Skip object and array annotations instead of converting them with String or returning headerMismatch, while preserving the existing absent/null handling and scalar validation behavior.libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts (3)
135-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated
Scopecast into one private accessor.The double cast
this.scope as unknown as Scopeappears three times. A singleprivate get fullScope(): Scopekeeps the unsafe narrowing in one place.Also applies to: 216-216, 255-255
🤖 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 `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts` at line 135, Introduce one private accessor named fullScope returning Scope that contains the this.scope as unknown as Scope cast, then replace all three repeated casts in the relevant flow methods with this.fullScope. Keep the existing behavior unchanged and limit the change to centralizing the unsafe narrowing.
208-239: 🩺 Stability & Availability | 🔵 TrivialConsider a cap on concurrent
subscriptions/listenstreams.Each stream registers tool, prompt, and resource registry listeners plus a keepalive timer, and it lives until the socket closes. This revision has no unsubscribe RPC, so an unauthenticated-but-reachable or low-privilege caller can hold many streams open. Track open streams per authorization and per scope, and reject beyond a configured limit. Add a metric for open subscription count so the limit can be tuned.
🤖 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 `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts` around lines 208 - 239, Update the subscriptions/listen flow around createSubscriptionStream to track active streams by authorization and scope, enforce a configurable concurrent-stream limit, and reject new streams once the limit is reached. Increment the open-subscription metric only after acceptance and decrement it when the response socket closes, ensuring cleanup also releases the registry listeners and keepalive resources.
291-300: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused re-export; the stated consumer does not use it.
Line 300 re-exports
isProtocol2026Request,MCP_HEADERS, andreadHeader, and the comment names thehttp:requestrouter as the consumer. That router importsisProtocol2026Requestfrom../../transport/mcp-2026(line 36 oflibs/sdk/src/scope/flows/http.request.flow.ts), not from this file. The re-export is therefore dead, and it would force any consumer that used it to load the whole flow module to reach a pure validator.♻️ Proposed refactor
- -/** Re-exported so the router stage of `http:request` can classify without importing the module. */ -export { isProtocol2026Request, MCP_HEADERS, readHeader };Then drop
isProtocol2026Request,MCP_HEADERS, andreadHeaderfrom the import block on lines 31-39 if the flow does not use them.🤖 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 `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts` around lines 291 - 300, Remove the bottom re-export of isProtocol2026Request, MCP_HEADERS, and readHeader from the flow module. Then remove those symbols from the module’s import block if they are unused after the re-export is deleted, leaving imports required by the flow intact.libs/sdk/src/transport/mcp-2026/discover.ts (1)
51-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
Omit<DiscoverResult, keyof object>withDiscoverResult.
keyof objectresolves tonever, soOmit<DiscoverResult, keyof object>is identical toDiscoverResult. The annotation suggests a narrowing that does not happen, and the trailingascast suppresses real type checking of the returned literal. ReturnDiscoverResultdirectly so the compiler validates the shape against the protocol contract.As per coding guidelines: "Maintain strict TypeScript correctness".
♻️ Proposed typing simplification
-export function buildDiscoverResult(scope: Scope, instructions?: string): Omit<DiscoverResult, keyof object> { +export function buildDiscoverResult(scope: Scope, instructions?: string): DiscoverResult { const capabilities = buildDiscoverCapabilities(scope); return { supportedVersions: [...SUPPORTED_PROTOCOL_VERSIONS_2026], capabilities, ...(instructions ? { instructions } : {}), - } as Omit<DiscoverResult, keyof object>; + }; }If
DiscoverResultrequires decorator-added fields such asresultType, useOmit<DiscoverResult, 'resultType' | 'ttlMs' | 'cacheScope'>instead, so the omission is explicit.🤖 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 `@libs/sdk/src/transport/mcp-2026/discover.ts` around lines 51 - 59, Update buildDiscoverResult to return DiscoverResult directly instead of Omit<DiscoverResult, keyof object>, and remove the trailing type assertion so TypeScript validates the returned literal against the protocol contract. If decorator-added fields are intentionally absent, explicitly omit only resultType, ttlMs, and cacheScope.Source: Coding guidelines
libs/sdk/src/transport/mcp-2026/__tests__/result-decorator.spec.ts (1)
31-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the
ttlMszero-fallback branch.
decorateResultusesDEFAULT_CACHE_TTL_MS[method] ?? 0(result-decorator.ts Line 54). This suite iterates only the keys ofDEFAULT_CACHE_TTL_MS, so the?? 0branch never runs. Add a case for a method that is listed inCACHEABLE_METHODSbut has no default TTL, and assertttlMsis0.As per path instructions: "95%+ coverage target across statements/branches/functions/lines".
🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/result-decorator.spec.ts` around lines 31 - 48, Extend the result-decorator tests with a cacheable method present in CACHEABLE_METHODS but absent from DEFAULT_CACHE_TTL_MS, and call decorateResult for that method without an override. Assert that ttlMs is 0, covering the nullish fallback while preserving the existing cacheScope behavior as applicable.Source: Path instructions
libs/sdk/src/transport/mcp-2026/__tests__/request-validation.spec.ts (1)
167-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for the rejected
mcp-nameheader branches.
validate2026Requestrejects anmcp-nameheader with invalid characters, and rejects a header that is not valid base64 after the sentinel prefix (seelibs/sdk/src/transport/mcp-2026/request-validation.tsLines 200-208). This spec covers only the successful decode. Add one case per rejection branch so the malformed-header paths stay covered.As per path instructions: "95%+ coverage target across statements/branches/functions/lines" and tests must "cover error and undefined/schema paths".
🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/request-validation.spec.ts` around lines 167 - 175, Add rejection-focused cases alongside the existing sentinel-decoding test in the request-validation spec: one must assert that an mcp-name header containing invalid characters is rejected, and another must assert rejection when the value after the sentinel prefix is invalid base64. Use validate2026Request with otherwise valid tools/call requests and verify each result indicates failure.Source: Path instructions
libs/sdk/src/transport/mcp-2026/dispatcher.ts (1)
44-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse the public Zod literal API.
Zod 4.4.3exposesvaluesas aSetandvalueas a legacy public accessor. Replace the_def.valuesfallback withmethod.values, or add a declaredmethodfield toMcpHandler. Keepmethod.valuefor single-value literals.🤖 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 `@libs/sdk/src/transport/mcp-2026/dispatcher.ts` around lines 44 - 50, Update methodOfSchema to use the public method.values accessor instead of accessing method._def.values, accounting for its Set shape when selecting the first string value. Preserve the existing method.value handling for single-value literals, or alternatively expose a declared method field on McpHandler if that is the established schema path.libs/sdk/src/transport/mcp-2026/mrtr.ts (1)
159-165: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a concrete protocol type for the interim result.
buildInputRequiredResultreturnsRecord<string, unknown>. If@frontmcp/protocoldefines a concreteInputRequiredResulttype for this shape, use it here (e.g., via a return type annotation orsatisfies) instead of the loosely typed record, for stronger compile-time guarantees onresultType/inputRequests/requestState.As per coding guidelines, "For MCP protocol outputs, prefer concrete MCP result types rather than
unknown."🤖 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 `@libs/sdk/src/transport/mcp-2026/mrtr.ts` around lines 159 - 165, Update buildInputRequiredResult to use the concrete InputRequiredResult type from `@frontmcp/protocol`, either as its return annotation or through satisfies, while preserving the existing resultType, inputRequests, and requestState fields.Source: Coding guidelines
libs/sdk/src/elicitation/helpers/elicit.helper.ts (1)
100-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated Zod-to-JSON-Schema conversion.
Lines 101-102 convert
requestedSchemato a Zod object and calltoJSONSchemaon it. Lines 136-138 (unchanged, further down in the same function) perform the identical conversion for the fallback path. Extract this into a small local helper function and call it from both branches. Duplicated conversion logic increases the risk of the two paths silently diverging in the future.♻️ Proposed refactor
+ const toZodJsonSchema = (schema: S) => + toJSONSchema(schema instanceof z.ZodType ? schema : z.object(schema as z.ZodRawShape)) as Record<string, unknown>; + const mrtr = ctx?.getMrtrExchange?.(); if (mrtr) { - const zodSchema = - requestedSchema instanceof z.ZodType ? requestedSchema : z.object(requestedSchema as z.ZodRawShape); const answer = mrtr.resolveElicitation({ message, - requestedSchema: toJSONSchema(zodSchema) as Record<string, unknown>, + requestedSchema: toZodJsonSchema(requestedSchema), ...(options?.mode ? { mode: options.mode } : {}), }); return answer as ElicitResult<S extends ZodType<infer O> ? O : unknown>; }Then reuse
toZodJsonSchema(requestedSchema)at the later fallback site.🤖 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 `@libs/sdk/src/elicitation/helpers/elicit.helper.ts` around lines 100 - 109, Extract the repeated conversion logic from the elicitation function into a local helper named toZodJsonSchema, encapsulating the requestedSchema Zod-instance check, z.object conversion, and toJSONSchema call. Replace the inline conversion in both the mrtr branch and the later fallback path with this helper, preserving the existing schema output and typing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d04cfdb4-feb9-40bb-a84a-c15796bfac89
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (80)
.github/workflows/cherry-pick-prompt.yml.github/workflows/create-release-branch.yml.github/workflows/publish-release.yml.github/workflows/push.ymlapps/e2e/demo-e2e-protocol-2026/e2e/backward-compat.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/cacheable-results.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/discover.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/errors-and-removals.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/helpers/mcp-2026-client.tsapps/e2e/demo-e2e-protocol-2026/e2e/mrtr.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/request-headers.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/stateless-requests.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/subscriptions-listen.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/jest.e2e.config.tsapps/e2e/demo-e2e-protocol-2026/project.jsonapps/e2e/demo-e2e-protocol-2026/src/apps/proto/index.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/prompts/greeting.prompt.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/resources/config.resource.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/confirm.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/echo.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/region-query.tool.tsapps/e2e/demo-e2e-protocol-2026/src/main.tsapps/e2e/demo-e2e-protocol-2026/tsconfig.app.jsonapps/e2e/demo-e2e-protocol-2026/tsconfig.e2e.jsonapps/e2e/demo-e2e-protocol-2026/tsconfig.jsonapps/e2e/demo-e2e-protocol-2026/webpack.config.jslibs/adapters/package.jsonlibs/auth/package.jsonlibs/cli/package.jsonlibs/di/package.jsonlibs/edge/package.jsonlibs/guard/package.jsonlibs/lazy-zod/package.jsonlibs/nx-plugin/package.jsonlibs/observability/package.jsonlibs/plugins/package.jsonlibs/protocol/package.jsonlibs/protocol/src/index.tslibs/protocol/src/types-2026.tslibs/react/package.jsonlibs/sdk/package.jsonlibs/sdk/src/common/utils/decide-request-intent.utils.tslibs/sdk/src/context/frontmcp-context.tslibs/sdk/src/elicitation/helpers/elicit.helper.tslibs/sdk/src/errors/index.tslibs/sdk/src/errors/mrtr.error.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/transport/flows/handle.mcp-2026.flow.tslibs/sdk/src/transport/mcp-2026/__tests__/header-codec.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/mrtr.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/request-validation.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/result-decorator.spec.tslibs/sdk/src/transport/mcp-2026/discover.tslibs/sdk/src/transport/mcp-2026/dispatcher.tslibs/sdk/src/transport/mcp-2026/header-codec.tslibs/sdk/src/transport/mcp-2026/index.tslibs/sdk/src/transport/mcp-2026/mrtr.tslibs/sdk/src/transport/mcp-2026/protocol-2026.constants.tslibs/sdk/src/transport/mcp-2026/request-validation.tslibs/sdk/src/transport/mcp-2026/result-decorator.tslibs/sdk/src/transport/mcp-2026/subscriptions.tslibs/sdk/src/transport/mcp-handlers/call-tool-request.handler.tslibs/sdk/src/transport/transport.registry.tslibs/skills/package.jsonlibs/storage-sqlite/package.jsonlibs/testing/package.jsonlibs/testing/src/server/port-registry.tslibs/ui/package.jsonlibs/uipack/package.jsonlibs/utils/package.jsonplugins/plugin-approval/package.jsonplugins/plugin-cache/package.jsonplugins/plugin-codecall/package.jsonplugins/plugin-dashboard/package.jsonplugins/plugin-feature-flags/package.jsonplugins/plugin-remember/package.jsonplugins/plugin-skilled-openapi/package.jsonscripts/normalize-internal-versions.mjsscripts/normalize-internal-versions.test.mjs
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-08-02T00:43:52.408Z |
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (19)
libs/sdk/src/auth/flows/oauth.authorize.flow.ts (1)
253-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
resolveIssuer()implementation across two flow classes. The same private method — identical body and JSDoc — is defined separately inOauthAuthorizeFlowandOauthCallbackFlow. Extract one shared helper (for example, a function besidevalidateAuthorizationIssuerininstance.local-primary-auth.ts, takingscope.authas a parameter) and call it from both flows instead of maintaining two copies.
libs/sdk/src/auth/flows/oauth.authorize.flow.ts#L253-L263: remove the privateresolveIssuer()method and call the shared helper instead.libs/sdk/src/auth/flows/oauth.callback.flow.ts#L146-L157: remove the privateresolveIssuer()method and call the shared helper instead.🤖 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 `@libs/sdk/src/auth/flows/oauth.authorize.flow.ts` around lines 253 - 263, Duplicate issuer-resolution logic exists in both OAuth flows. Add one shared helper beside validateAuthorizationIssuer in instance.local-primary-auth.ts that accepts scope.auth and preserves the existing string validation, then remove resolveIssuer from libs/sdk/src/auth/flows/oauth.authorize.flow.ts lines 253-263 and libs/sdk/src/auth/flows/oauth.callback.flow.ts lines 146-157, updating both flows to call the shared helper.libs/sdk/src/context/frontmcp-context.ts (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing the log level type.
RequestNotificationSinkRef.logacceptslevel: string. The implementationRequestNotificationSink.logacceptsLoggingLevel. Callers that hold the ref can pass any string, so an invalid level reaches the sink without a compile error.LoggingLevelcomes from@frontmcp/protocol, so importing the type here does not add a transport-layer dependency.♻️ Proposed narrowing
export interface RequestNotificationSinkRef { - log(level: string, logger: string | undefined, data: unknown): boolean; + log(level: LoggingLevel, logger: string | undefined, data: unknown): boolean; progress(progress: number, total?: number, message?: string): boolean; }🤖 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 `@libs/sdk/src/context/frontmcp-context.ts` around lines 40 - 43, Update RequestNotificationSinkRef.log to use the LoggingLevel type from `@frontmcp/protocol` instead of string, adding the type import and preserving the existing logger and data parameters.Source: Coding guidelines
libs/sdk/src/transport/mcp-2026/client/header-params.ts (1)
91-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant ternary.
Both branches call
String(value), so the condition has no effect.♻️ Proposed change
- const asString = typeof value === 'boolean' ? String(value) : String(value); - headers[`Mcp-Param-${name}`] = encode(asString); + headers[`Mcp-Param-${name}`] = encode(String(value));🤖 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 `@libs/sdk/src/transport/mcp-2026/client/header-params.ts` around lines 91 - 92, In the header parameter conversion logic, simplify the redundant ternary assigned to asString in the surrounding parameter-building function by directly converting value with String(value), preserving the existing encoding and header assignment behavior.libs/sdk/src/remote-mcp/mcp-2026-client.adapter.ts (1)
87-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the doc comment with the probe implementation.
The comment states that the probe falls back "only ... when the failure is NOT a recognised modern error". The
catchblock on line 109 falls back for every thrown error, including a modern-shaped JSON-RPC error. Update the comment to describe the actual rule: any failed or non-advertisingserver/discoverselects the legacy path.🤖 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 `@libs/sdk/src/remote-mcp/mcp-2026-client.adapter.ts` around lines 87 - 113, Update the documentation comment for negotiateRemoteProtocol to state that any failed server/discover probe, including errors, or a response that does not advertise 2026-07-28 selects the legacy path. Keep the probe implementation and its explicit-version behavior unchanged.libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts (2)
219-249: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNote on the
ttlMs: nullcase.
TaskRecord.ttlMsallowsnull, which means unlimited lifetime, andtaskToWire2026copies it verbatim. Line 222 treats a non-number as60_000, so the client abandons an unlimited task after 60 seconds. Confirm that this cap is intended, or make the polling deadline configurable throughMcp2026ClientOptions.🤖 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 `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts` around lines 219 - 249, Update awaitTask to handle task.ttlMs === null as an unlimited lifetime instead of applying the 60-second fallback. Preserve numeric TTL behavior, and use the existing Mcp2026ClientOptions configuration if a finite polling deadline is required for unlimited tasks.
302-320: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a request timeout for
send().
send()passes nosignaltofetchImpl.Mcp2026ClientAdapteruses this client for remote proxying, so a remote server that accepts the connection and never answers holds the calling request open until the platform default expires. Add anAbortControllerwith a configurable timeout, and clear it in afinallyblock.🤖 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 `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts` around lines 302 - 320, Update Mcp2026ClientAdapter.send to create an AbortController, pass its signal to fetchImpl, and abort after a configurable request-timeout option. Clear the timeout in a finally block so it is released on both successful responses and errors, while preserving the existing response parsing and Mcp2026Error handling.libs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.ts (1)
1-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
dispatchTasksMethod.The suite covers the pure helpers but not
dispatchTasksMethod, which holds the branch logic and the security-relevant behavior: missing store, missingtaskId, owner-scoped not-found, terminal-task cancel acknowledgement, non-input_requiredupdate rejection, and theinputResponsesmerge plus resume call. A small faketaskStorecovers all of these. The SDK targets 95% coverage across branches.Do you want me to generate these test cases?
As per path instructions: "95%+ coverage target across statements/branches/functions/lines".🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.ts` around lines 1 - 150, Add focused tests for dispatchTasksMethod using a small fake taskStore, covering missing-store handling, missing taskId validation, owner-scoped not-found behavior, terminal-task cancel acknowledgement, rejection of updates when status is not input_required, and merging inputResponses before invoking resume. Follow the existing test style and exercise each branch to meet the stated coverage target.Source: Path instructions
libs/sdk/src/elicitation/helpers/mrtr-request.helper.ts (1)
73-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
ToolContext.sample()andToolContext.listRoots().Add both protected APIs to
docs/frontmcp/sdk-reference/contexts/tool-context.mdx, including MRTR, protocol version, capability, and deprecation requirements. The helper functions are internal; users access them through these methods.🤖 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 `@libs/sdk/src/elicitation/helpers/mrtr-request.helper.ts` around lines 73 - 100, Document the protected ToolContext.sample() and ToolContext.listRoots() APIs in tool-context.mdx, using the existing public method conventions. Describe their MRTR-only availability, protocol version 2026-07-28, required sampling or roots capability, and deprecation requirements; do not document performSample or performListRoots as user-facing APIs.Source: Path instructions
libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts (2)
106-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
hmacSha256at the top of the file.Line 108 uses
requireinside the test body. The rest of the file uses ESM imports, andrequireis not typed, so a signature change in@frontmcp/utilswould not fail the build here.♻️ Proposed change
+import { hmacSha256 } from '`@frontmcp/utils`'; + import { computeRequestBinding, @@ it('rejects a signed blob whose payload is not JSON', () => { // Signed by us, so the signature passes — the payload check must still catch it. - const { hmacSha256 } = require('`@frontmcp/utils`'); const body = Buffer.from('not json', 'utf8').toString('base64url');🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts` around lines 106 - 112, Move the hmacSha256 import from the test body into the file’s top-level ESM imports, then use that imported symbol in the “rejects a signed blob whose payload is not JSON” test. Remove the inline require while preserving the existing signing and assertion behavior.
115-150: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a test for the token-derived principal collision.
resolvePrincipalindispatcher.tsderives the principal from the first 16 characters of the bearer token. Add a case that asserts two different tokens with a shared 16-character prefix resolve to different principals. That test locks in the fix requested onlibs/sdk/src/transport/mcp-2026/dispatcher.tslines 73-79 and reaches the 95% coverage target for this path.🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts` around lines 115 - 150, Add a test covering resolvePrincipal in dispatcher.ts that supplies two distinct bearer tokens sharing the same first 16 characters and asserts their resolved principals differ. Keep the test focused on token-derived principal generation and ensure it exercises the collision fix in the relevant dispatcher path.Source: Coding guidelines
apps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.ts (1)
26-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the JSON-RPC code as well as the message.
Both tests match the message text
authenticated caller. That string lives inresolveTaskOwnerand can be reworded without a protocol change, which would break these tests for the wrong reason. Addexpect(res.json().error.code).toBe(-32602)so the contract is asserted on the code.🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.ts` around lines 26 - 46, Update the anonymous caller tests for tasks/get and tasks/update to assert that each response error code equals -32602 in addition to the existing message assertion. Keep the current message checks and request setup unchanged.apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts (2)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType the destructured input request.
Both lines cast to
[string, any]. The coding guidelines forbidanywithout strong justification. Declare a small local type for the input-request shape and reuse it.♻️ Proposed change
+type InputRequest = { method: string; params: Record<string, any> }; @@ - const [, request] = Object.entries(result.inputRequests)[0] as [string, any]; + const [, request] = Object.entries(result.inputRequests)[0] as [string, InputRequest];Use
Record<string, unknown>forparamsif the assertions still type-check.Also applies to: 97-97
🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts` at line 39, Replace the `[string, any]` casts in both `Object.entries(result.inputRequests)` destructuring sites with a local input-request type, using `Record<string, unknown>` for `params` if compatible with the existing assertions. Reuse the type for both occurrences and remove the forbidden `any` usage.Source: Coding guidelines
39-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
anyis used to read 2026 wire payloads across the new E2E suites. The suites parse untyped JSON and reach foranyat each read site. The coding guidelines forbidanywithout strong justification. Declare the wire shapes once, for example ine2e/helpers/mcp-2026-client.ts, and import them into each suite.
apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts#L39-L97: replace bothas [string, any]casts with a sharedInputRequesttype.apps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.ts#L17-L40: type thepollUntilpredicate, return value andlastwith a shared task shape, and reuse the sameInputRequesttype on line 223.apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts#L188-L189: replace(t: any) => t.namewith(t: { name: string }) => t.name.🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts` around lines 39 - 97, Replace untyped JSON reads with shared wire types: define and export InputRequest and the task shape in e2e/helpers/mcp-2026-client.ts, then import them into each suite. In apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts:39-97, replace both [string, any] casts with InputRequest. In apps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.ts:17-40, type the pollUntil predicate, return value, and last variable with the shared task shape, and use InputRequest at line 223. In apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts:188-189, replace the any callback parameter with an object type containing name: string.Source: Coding guidelines
apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts (1)
188-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
anywith a typed shape.Line 189 annotates the map callback parameter as
any. The coding guidelines forbidanywithout strong justification and requireunknownor a constrained type instead. The other suites readt['name']from aRecord<string, unknown>, as on line 32.♻️ Proposed change
- expect(tools.map((t: any) => t.name)).toContain('echo'); + expect(tools.map((t: { name: string }) => t.name)).toContain('echo');🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts` around lines 188 - 189, Replace the any annotation on the map callback parameter in the listTools assertion with the established Record<string, unknown> shape used by the other suites, while preserving the existing name extraction and echo assertion.Source: Coding guidelines
libs/sdk/src/transport/mcp-2026/dispatcher.ts (1)
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFreeze
MRTR_CAPABLE_METHODS.The array is exported and mutable. A consumer can push a method into it and widen where
input_requiredresults are permitted. Useas constand areadonly string[]check, the same shapeMCP_2026_REMOVED_METHODSuses on line 264.♻️ Proposed change
-export const MRTR_CAPABLE_METHODS = ['tools/call', 'prompts/get', 'resources/read']; +export const MRTR_CAPABLE_METHODS = ['tools/call', 'prompts/get', 'resources/read'] as const;Then update the check on line 393:
if (!(MRTR_CAPABLE_METHODS as readonly string[]).includes(method)) {🤖 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 `@libs/sdk/src/transport/mcp-2026/dispatcher.ts` at line 64, Freeze the exported MRTR_CAPABLE_METHODS tuple by declaring it with as const, matching the readonly shape of MCP_2026_REMOVED_METHODS. Update the membership check in the dispatcher to treat MRTR_CAPABLE_METHODS as a readonly string array when calling includes.libs/sdk/src/transport/mcp-2026/result-decorator.ts (1)
69-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNote the per-page scope of the ordering guarantee.
orderListResultsorts the entries present in one result. If a handler paginates with a cursor, each page is sorted on its own, so the concatenation of pages is not globally sorted. The comment on lines 77-86 promises deterministic ordering without stating that limit. Add one sentence to the comment so a later reader does not rely on a global order.🤖 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 `@libs/sdk/src/transport/mcp-2026/result-decorator.ts` around lines 69 - 101, The comment for orderListResult should state that ordering is guaranteed only within each individual result page, not across concatenated paginated pages. Add one sentence to the existing documentation near the deterministic-ordering explanation, without changing the sorting implementation.libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts (1)
109-156: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAbort the dispatch when the SSE client disconnects.
handleSubscriptionsties anAbortControllerto the responsecloseevent. The streamed message path does not.dispatchOptionsomitssignal, sodispatch2026builds its own signal that never aborts (signal ?? new AbortController().signal). If the client closes the connection during a longtools/call, the generator stops being drained, but the tool keeps running to completion and holds its resources.Tie a controller to the response socket and forward it as
signalin the streamed branch.♻️ Proposed change
if (sink.active && acceptsEventStream(request.headers as Record<string, unknown> | undefined)) { + const controller = new AbortController(); + const { response } = this.rawInput; + (response as unknown as { on?: (event: string, cb: () => void) => void })?.on?.('close', () => + controller.abort(), + ); this.respond({ kind: 'sse', status: 200, - stream: streamMessageResponse(dispatchOptions, sink, body['id'], this.buildContextRunner()), + stream: streamMessageResponse( + { ...dispatchOptions, signal: controller.signal }, + sink, + body['id'], + this.buildContextRunner(), + ),Also applies to: 398-412
🤖 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 `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts` around lines 109 - 156, Update the streamed message path around streamMessageResponse to create an AbortController tied to the response/socket close event, pass its signal through the dispatch options to dispatch2026, and clean up the close listener when streaming finishes. Ensure client disconnection aborts an in-flight dispatch while preserving normal notification draining and response completion.libs/sdk/src/remote-mcp/mcp-client.types.ts (1)
87-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
protocolVersionin the SDK reference.
McpHttpTransportOptionsis publicly exported, but itsprotocolVersionoption is absent fromdocs/frontmcp/**. Document its supported values and behavior.🤖 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 `@libs/sdk/src/remote-mcp/mcp-client.types.ts` around lines 87 - 96, Document the publicly exported McpHttpTransportOptions.protocolVersion option in the SDK reference under docs/frontmcp/**, covering the 'legacy', '2026-07-28', and 'auto' values and their respective transport and fallback behavior. Keep the documentation aligned with the existing property comment and avoid changing the TypeScript API.Source: Path instructions
libs/sdk/src/remote-mcp/mcp-client.service.ts (1)
798-803: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDocument or reject
protocolVersion: '2026-07-28'with SSE. Unconfigured and'legacy'HTTP connections return without a probe. SSE requests skip 2026 negotiation and silently use the legacy client, even when this option is set.🤖 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 `@libs/sdk/src/remote-mcp/mcp-client.service.ts` around lines 798 - 803, Update tryConnect2026 to explicitly handle protocolVersion '2026-07-28' for SSE transports: either document and support the option through the SSE path or reject it instead of silently falling back to the legacy client. Preserve the existing no-probe behavior for unconfigured and 'legacy' HTTP connections, and ensure configured 2026 requests cannot bypass negotiation.
🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts`:
- Around line 176-180: Update the discovery probe used by
negotiateRemoteProtocol to enforce a bounded timeout when Mcp2026Client.send
invokes fetchImpl. Create and pass an AbortSignal, abort it when the timeout
expires, and ensure the probe resolves to 'legacy' rather than hanging when the
remote is unreachable.
In `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts`:
- Around line 159-182: Update the test “rejects a requestState replayed onto a
different tool call” to omit inputResponses from the replayed request, making
the carried requestState the only possible completion source; assert the result
is input_required. Remove the now-unused key extraction from
interim.inputRequests.
In `@libs/sdk/src/auth/flows/oauth.register.flow.ts`:
- Around line 66-76: The OAuth registration flow must persist and enforce
application_type rather than only echoing it. Update the RegisteredClient model,
registration validator, and dcrClientRegistry.register() payload to carry the
value from request state, then make redirect validation allow native private-use
schemes such as com.example.app:/callback while retaining web-client
restrictions and existing allowlist behavior.
In `@libs/sdk/src/auth/instances/instance.local-primary-auth.ts`:
- Around line 203-216: Wire issuer through UpstreamProviderOptions,
ProviderConfig, and both automatic registration paths so the instance’s issuer
metadata is populated. Update discardProviderCredentials() to use the typed
TokenStore.deleteTokens(authorizationId, providerId) operation for every
authorization belonging to the provider, ensuring all credentials are removed.
Normalize trailing slashes on previous and new issuers before comparing them to
avoid unnecessary deletion.
In `@libs/sdk/src/index.ts`:
- Around line 430-442: Add SDK reference documentation under
docs/frontmcp/sdk-reference for the exported MCP 2026 symbols from the index:
Mcp2026Client, Mcp2026Error, TASKS_CLIENT_CAPABILITY, Mcp2026ClientOptions,
Mcp2026InputHandlers, validateHeaderParams, buildParamHeaders,
SUPPORTED_PROTOCOL_VERSIONS_2026, TASKS_EXTENSION_ID, Mcp2026ClientAdapter,
negotiateRemoteProtocol, and PROTOCOL_2026_07_28. Organize the reference
according to the existing SDK documentation conventions and document each API’s
purpose, usage, and relevant parameters or values.
In `@libs/sdk/src/remote-mcp/mcp-client.service.ts`:
- Around line 812-822: Update the McpClientConnection type to make transport
optional, remove the undefined cast in the returned connection object, and
narrow or guard transport at every use site before dereferencing it. Preserve
existing behavior for paths where a transport is present and handle the 2026
adapter path without runtime errors.
In `@libs/sdk/src/task/helpers/task-runner.ts`:
- Around line 98-115: Update the InputRequiredSignal handling in the task runner
and the underlying StorageTaskStore transition path to use a conditional atomic
update that only writes input_required when the task is not already terminal.
Ensure concurrent tasks/cancel operations cannot be overwritten, and base the
resulting notification/return behavior on the atomic update outcome rather than
a separate get/isTerminal check.
In `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts`:
- Around line 364-370: Validate the progressToken value before constructing
RequestNotificationSink: accept only strings or numbers, otherwise use
undefined. Update the progressToken extraction beside logLevel so invalid
client-provided objects and arrays cannot activate the sink or be echoed in
progress notifications.
In `@libs/sdk/src/transport/mcp-2026/client/header-params.ts`:
- Around line 33-44: Update validateHeaderParams to inspect every raw
x-mcp-header annotation rather than the deduplicated result of
collectHeaderParams. Preserve empty annotations for validation, track normalized
names with seen to reject case-insensitive duplicates, and retain the existing
token validation and error behavior; either add a duplicate-aware collector or
extend collectHeaderParams to report collisions.
In `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts`:
- Around line 394-397: Update the onMessage parameter in pumpSse to use
Record<string, unknown> instead of Record<string, any>, preserving the callback
signature and existing message handling.
- Around line 85-87: Update the Mcp2026Client constructor so maxInputRounds
retains its default when the provided option is explicitly undefined; normalize
that field after merging options, while preserving caller-supplied numeric
values and the existing options type contract.
- Around line 282-299: Update the acknowledgement promise in listen() to settle
when pumpSse completes without receiving
notifications/subscriptions/acknowledged, including stream closure, abort, or
error. Reuse the existing resolveAck path or add the necessary rejection
handling in the pumpSse completion flow, and ensure await acknowledged cannot
remain pending while preserving the current acknowledgement behavior.
In `@libs/sdk/src/transport/mcp-2026/dispatcher.ts`:
- Around line 73-79: Update resolvePrincipal to stop using a token prefix as the
principal; hash the full non-empty token with the existing sha256Hex utility
imported from `@frontmcp/utils`, while preserving clientId precedence and the
anonymous fallback.
In `@libs/sdk/src/transport/mcp-2026/mrtr.ts`:
- Around line 32-37: Update resolve() to validate elicitation requests against
the capability matching pending.mode (form or url), rather than only checking
that clientCapabilities.elicitation exists, before recording or replaying the
request. Make MissingClientCapabilityError use the corresponding required
capability shape for the requested mode, while preserving existing validation
for sampling and roots.
In `@libs/sdk/src/transport/mcp-2026/request-state.ts`:
- Around line 94-100: Move the custom timingSafeEqual implementation out of
request-state.ts and add the helper to `@frontmcp/utils` using its established
crypto utilities, then import and reuse that shared helper in the
signature-checking flow. Remove the local function while preserving
constant-time comparison behavior, including the unequal-length case.
- Around line 90-92: Update base64urlEncodeJson to use the base64url encoding
utility from `@frontmcp/utils` instead of Buffer, preserving JSON serialization
and the existing string result. Also replace any related decoding logic in the
request-state flow with base64urlDecode from `@frontmcp/utils` so the 2026
dispatcher works when Buffer is unavailable.
In `@libs/sdk/src/transport/mcp-2026/tasks-extension.ts`:
- Around line 181-188: Update the resumed-execution flow in
dispatchTasksMethod/resumeTask so tasks/update does not await tool execution:
schedule runner.run(...) in the background rather than awaiting resume, and
attach explicit rejection handling for failures. Preserve the existing update
payload, including inputRequests: undefined, and keep successful resumption
behavior unchanged.
---
Nitpick comments:
In `@apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts`:
- Around line 188-189: Replace the any annotation on the map callback parameter
in the listTools assertion with the established Record<string, unknown> shape
used by the other suites, while preserving the existing name extraction and echo
assertion.
In `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts`:
- Line 39: Replace the `[string, any]` casts in both
`Object.entries(result.inputRequests)` destructuring sites with a local
input-request type, using `Record<string, unknown>` for `params` if compatible
with the existing assertions. Reuse the type for both occurrences and remove the
forbidden `any` usage.
- Around line 39-97: Replace untyped JSON reads with shared wire types: define
and export InputRequest and the task shape in e2e/helpers/mcp-2026-client.ts,
then import them into each suite. In
apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts:39-97,
replace both [string, any] casts with InputRequest. In
apps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.ts:17-40, type the
pollUntil predicate, return value, and last variable with the shared task shape,
and use InputRequest at line 223. In
apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts:188-189, replace the any
callback parameter with an object type containing name: string.
In `@apps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.ts`:
- Around line 26-46: Update the anonymous caller tests for tasks/get and
tasks/update to assert that each response error code equals -32602 in addition
to the existing message assertion. Keep the current message checks and request
setup unchanged.
In `@libs/sdk/src/auth/flows/oauth.authorize.flow.ts`:
- Around line 253-263: Duplicate issuer-resolution logic exists in both OAuth
flows. Add one shared helper beside validateAuthorizationIssuer in
instance.local-primary-auth.ts that accepts scope.auth and preserves the
existing string validation, then remove resolveIssuer from
libs/sdk/src/auth/flows/oauth.authorize.flow.ts lines 253-263 and
libs/sdk/src/auth/flows/oauth.callback.flow.ts lines 146-157, updating both
flows to call the shared helper.
In `@libs/sdk/src/context/frontmcp-context.ts`:
- Around line 40-43: Update RequestNotificationSinkRef.log to use the
LoggingLevel type from `@frontmcp/protocol` instead of string, adding the type
import and preserving the existing logger and data parameters.
In `@libs/sdk/src/elicitation/helpers/mrtr-request.helper.ts`:
- Around line 73-100: Document the protected ToolContext.sample() and
ToolContext.listRoots() APIs in tool-context.mdx, using the existing public
method conventions. Describe their MRTR-only availability, protocol version
2026-07-28, required sampling or roots capability, and deprecation requirements;
do not document performSample or performListRoots as user-facing APIs.
In `@libs/sdk/src/remote-mcp/mcp-2026-client.adapter.ts`:
- Around line 87-113: Update the documentation comment for
negotiateRemoteProtocol to state that any failed server/discover probe,
including errors, or a response that does not advertise 2026-07-28 selects the
legacy path. Keep the probe implementation and its explicit-version behavior
unchanged.
In `@libs/sdk/src/remote-mcp/mcp-client.service.ts`:
- Around line 798-803: Update tryConnect2026 to explicitly handle
protocolVersion '2026-07-28' for SSE transports: either document and support the
option through the SSE path or reject it instead of silently falling back to the
legacy client. Preserve the existing no-probe behavior for unconfigured and
'legacy' HTTP connections, and ensure configured 2026 requests cannot bypass
negotiation.
In `@libs/sdk/src/remote-mcp/mcp-client.types.ts`:
- Around line 87-96: Document the publicly exported
McpHttpTransportOptions.protocolVersion option in the SDK reference under
docs/frontmcp/**, covering the 'legacy', '2026-07-28', and 'auto' values and
their respective transport and fallback behavior. Keep the documentation aligned
with the existing property comment and avoid changing the TypeScript API.
In `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts`:
- Around line 109-156: Update the streamed message path around
streamMessageResponse to create an AbortController tied to the response/socket
close event, pass its signal through the dispatch options to dispatch2026, and
clean up the close listener when streaming finishes. Ensure client disconnection
aborts an in-flight dispatch while preserving normal notification draining and
response completion.
In `@libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts`:
- Around line 106-112: Move the hmacSha256 import from the test body into the
file’s top-level ESM imports, then use that imported symbol in the “rejects a
signed blob whose payload is not JSON” test. Remove the inline require while
preserving the existing signing and assertion behavior.
- Around line 115-150: Add a test covering resolvePrincipal in dispatcher.ts
that supplies two distinct bearer tokens sharing the same first 16 characters
and asserts their resolved principals differ. Keep the test focused on
token-derived principal generation and ensure it exercises the collision fix in
the relevant dispatcher path.
In `@libs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.ts`:
- Around line 1-150: Add focused tests for dispatchTasksMethod using a small
fake taskStore, covering missing-store handling, missing taskId validation,
owner-scoped not-found behavior, terminal-task cancel acknowledgement, rejection
of updates when status is not input_required, and merging inputResponses before
invoking resume. Follow the existing test style and exercise each branch to meet
the stated coverage target.
In `@libs/sdk/src/transport/mcp-2026/client/header-params.ts`:
- Around line 91-92: In the header parameter conversion logic, simplify the
redundant ternary assigned to asString in the surrounding parameter-building
function by directly converting value with String(value), preserving the
existing encoding and header assignment behavior.
In `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts`:
- Around line 219-249: Update awaitTask to handle task.ttlMs === null as an
unlimited lifetime instead of applying the 60-second fallback. Preserve numeric
TTL behavior, and use the existing Mcp2026ClientOptions configuration if a
finite polling deadline is required for unlimited tasks.
- Around line 302-320: Update Mcp2026ClientAdapter.send to create an
AbortController, pass its signal to fetchImpl, and abort after a configurable
request-timeout option. Clear the timeout in a finally block so it is released
on both successful responses and errors, while preserving the existing response
parsing and Mcp2026Error handling.
In `@libs/sdk/src/transport/mcp-2026/dispatcher.ts`:
- Line 64: Freeze the exported MRTR_CAPABLE_METHODS tuple by declaring it with
as const, matching the readonly shape of MCP_2026_REMOVED_METHODS. Update the
membership check in the dispatcher to treat MRTR_CAPABLE_METHODS as a readonly
string array when calling includes.
In `@libs/sdk/src/transport/mcp-2026/result-decorator.ts`:
- Around line 69-101: The comment for orderListResult should state that ordering
is guaranteed only within each individual result page, not across concatenated
paginated pages. Add one sentence to the existing documentation near the
deterministic-ordering explanation, without changing the sorting implementation.
🪄 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: 01fa6c26-06e3-4177-952e-db5f96a5319e
📒 Files selected for processing (48)
apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/request-notifications.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/index.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/chatty.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/list-workspaces.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/summarize.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/tasks/index.tsapps/e2e/demo-e2e-protocol-2026/src/apps/tasks/tools/approve-job.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/tasks/tools/slow-job.tool.tsapps/e2e/demo-e2e-protocol-2026/src/main-tasks.tslibs/sdk/src/auth/__tests__/authorization-issuer.spec.tslibs/sdk/src/auth/flows/oauth.authorize.flow.tslibs/sdk/src/auth/flows/oauth.callback.flow.tslibs/sdk/src/auth/flows/oauth.provider-callback.flow.tslibs/sdk/src/auth/flows/oauth.register.flow.tslibs/sdk/src/auth/flows/well-known.oauth-authorization-server.flow.tslibs/sdk/src/auth/instances/instance.local-primary-auth.tslibs/sdk/src/common/interfaces/tool.interface.tslibs/sdk/src/context/frontmcp-context.tslibs/sdk/src/elicitation/helpers/index.tslibs/sdk/src/elicitation/helpers/mrtr-request.helper.tslibs/sdk/src/errors/index.tslibs/sdk/src/errors/mrtr.error.tslibs/sdk/src/index.tslibs/sdk/src/remote-mcp/mcp-2026-client.adapter.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/task/helpers/task-runner.tslibs/sdk/src/task/task.types.tslibs/sdk/src/transport/flows/handle.mcp-2026.flow.tslibs/sdk/src/transport/mcp-2026/__tests__/mrtr.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/request-notifications.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/result-decorator.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.tslibs/sdk/src/transport/mcp-2026/client/header-params.tslibs/sdk/src/transport/mcp-2026/client/index.tslibs/sdk/src/transport/mcp-2026/client/mcp-2026.client.tslibs/sdk/src/transport/mcp-2026/dispatcher.tslibs/sdk/src/transport/mcp-2026/index.tslibs/sdk/src/transport/mcp-2026/mrtr.tslibs/sdk/src/transport/mcp-2026/request-notifications.tslibs/sdk/src/transport/mcp-2026/request-state.tslibs/sdk/src/transport/mcp-2026/result-decorator.tslibs/sdk/src/transport/mcp-2026/tasks-extension.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/errors/index.ts
- libs/sdk/src/transport/mcp-2026/index.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 17
🧹 Nitpick comments (19)
libs/sdk/src/auth/flows/oauth.authorize.flow.ts (1)
253-263: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
resolveIssuer()implementation across two flow classes. The same private method — identical body and JSDoc — is defined separately inOauthAuthorizeFlowandOauthCallbackFlow. Extract one shared helper (for example, a function besidevalidateAuthorizationIssuerininstance.local-primary-auth.ts, takingscope.authas a parameter) and call it from both flows instead of maintaining two copies.
libs/sdk/src/auth/flows/oauth.authorize.flow.ts#L253-L263: remove the privateresolveIssuer()method and call the shared helper instead.libs/sdk/src/auth/flows/oauth.callback.flow.ts#L146-L157: remove the privateresolveIssuer()method and call the shared helper instead.🤖 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 `@libs/sdk/src/auth/flows/oauth.authorize.flow.ts` around lines 253 - 263, Duplicate issuer-resolution logic exists in both OAuth flows. Add one shared helper beside validateAuthorizationIssuer in instance.local-primary-auth.ts that accepts scope.auth and preserves the existing string validation, then remove resolveIssuer from libs/sdk/src/auth/flows/oauth.authorize.flow.ts lines 253-263 and libs/sdk/src/auth/flows/oauth.callback.flow.ts lines 146-157, updating both flows to call the shared helper.libs/sdk/src/context/frontmcp-context.ts (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing the log level type.
RequestNotificationSinkRef.logacceptslevel: string. The implementationRequestNotificationSink.logacceptsLoggingLevel. Callers that hold the ref can pass any string, so an invalid level reaches the sink without a compile error.LoggingLevelcomes from@frontmcp/protocol, so importing the type here does not add a transport-layer dependency.♻️ Proposed narrowing
export interface RequestNotificationSinkRef { - log(level: string, logger: string | undefined, data: unknown): boolean; + log(level: LoggingLevel, logger: string | undefined, data: unknown): boolean; progress(progress: number, total?: number, message?: string): boolean; }🤖 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 `@libs/sdk/src/context/frontmcp-context.ts` around lines 40 - 43, Update RequestNotificationSinkRef.log to use the LoggingLevel type from `@frontmcp/protocol` instead of string, adding the type import and preserving the existing logger and data parameters.Source: Coding guidelines
libs/sdk/src/transport/mcp-2026/client/header-params.ts (1)
91-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the redundant ternary.
Both branches call
String(value), so the condition has no effect.♻️ Proposed change
- const asString = typeof value === 'boolean' ? String(value) : String(value); - headers[`Mcp-Param-${name}`] = encode(asString); + headers[`Mcp-Param-${name}`] = encode(String(value));🤖 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 `@libs/sdk/src/transport/mcp-2026/client/header-params.ts` around lines 91 - 92, In the header parameter conversion logic, simplify the redundant ternary assigned to asString in the surrounding parameter-building function by directly converting value with String(value), preserving the existing encoding and header assignment behavior.libs/sdk/src/remote-mcp/mcp-2026-client.adapter.ts (1)
87-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the doc comment with the probe implementation.
The comment states that the probe falls back "only ... when the failure is NOT a recognised modern error". The
catchblock on line 109 falls back for every thrown error, including a modern-shaped JSON-RPC error. Update the comment to describe the actual rule: any failed or non-advertisingserver/discoverselects the legacy path.🤖 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 `@libs/sdk/src/remote-mcp/mcp-2026-client.adapter.ts` around lines 87 - 113, Update the documentation comment for negotiateRemoteProtocol to state that any failed server/discover probe, including errors, or a response that does not advertise 2026-07-28 selects the legacy path. Keep the probe implementation and its explicit-version behavior unchanged.libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts (2)
219-249: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNote on the
ttlMs: nullcase.
TaskRecord.ttlMsallowsnull, which means unlimited lifetime, andtaskToWire2026copies it verbatim. Line 222 treats a non-number as60_000, so the client abandons an unlimited task after 60 seconds. Confirm that this cap is intended, or make the polling deadline configurable throughMcp2026ClientOptions.🤖 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 `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts` around lines 219 - 249, Update awaitTask to handle task.ttlMs === null as an unlimited lifetime instead of applying the 60-second fallback. Preserve numeric TTL behavior, and use the existing Mcp2026ClientOptions configuration if a finite polling deadline is required for unlimited tasks.
302-320: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider a request timeout for
send().
send()passes nosignaltofetchImpl.Mcp2026ClientAdapteruses this client for remote proxying, so a remote server that accepts the connection and never answers holds the calling request open until the platform default expires. Add anAbortControllerwith a configurable timeout, and clear it in afinallyblock.🤖 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 `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts` around lines 302 - 320, Update Mcp2026ClientAdapter.send to create an AbortController, pass its signal to fetchImpl, and abort after a configurable request-timeout option. Clear the timeout in a finally block so it is released on both successful responses and errors, while preserving the existing response parsing and Mcp2026Error handling.libs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.ts (1)
1-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
dispatchTasksMethod.The suite covers the pure helpers but not
dispatchTasksMethod, which holds the branch logic and the security-relevant behavior: missing store, missingtaskId, owner-scoped not-found, terminal-task cancel acknowledgement, non-input_requiredupdate rejection, and theinputResponsesmerge plus resume call. A small faketaskStorecovers all of these. The SDK targets 95% coverage across branches.Do you want me to generate these test cases?
As per path instructions: "95%+ coverage target across statements/branches/functions/lines".🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.ts` around lines 1 - 150, Add focused tests for dispatchTasksMethod using a small fake taskStore, covering missing-store handling, missing taskId validation, owner-scoped not-found behavior, terminal-task cancel acknowledgement, rejection of updates when status is not input_required, and merging inputResponses before invoking resume. Follow the existing test style and exercise each branch to meet the stated coverage target.Source: Path instructions
libs/sdk/src/elicitation/helpers/mrtr-request.helper.ts (1)
73-100: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
ToolContext.sample()andToolContext.listRoots().Add both protected APIs to
docs/frontmcp/sdk-reference/contexts/tool-context.mdx, including MRTR, protocol version, capability, and deprecation requirements. The helper functions are internal; users access them through these methods.🤖 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 `@libs/sdk/src/elicitation/helpers/mrtr-request.helper.ts` around lines 73 - 100, Document the protected ToolContext.sample() and ToolContext.listRoots() APIs in tool-context.mdx, using the existing public method conventions. Describe their MRTR-only availability, protocol version 2026-07-28, required sampling or roots capability, and deprecation requirements; do not document performSample or performListRoots as user-facing APIs.Source: Path instructions
libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts (2)
106-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
hmacSha256at the top of the file.Line 108 uses
requireinside the test body. The rest of the file uses ESM imports, andrequireis not typed, so a signature change in@frontmcp/utilswould not fail the build here.♻️ Proposed change
+import { hmacSha256 } from '`@frontmcp/utils`'; + import { computeRequestBinding, @@ it('rejects a signed blob whose payload is not JSON', () => { // Signed by us, so the signature passes — the payload check must still catch it. - const { hmacSha256 } = require('`@frontmcp/utils`'); const body = Buffer.from('not json', 'utf8').toString('base64url');🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts` around lines 106 - 112, Move the hmacSha256 import from the test body into the file’s top-level ESM imports, then use that imported symbol in the “rejects a signed blob whose payload is not JSON” test. Remove the inline require while preserving the existing signing and assertion behavior.
115-150: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a test for the token-derived principal collision.
resolvePrincipalindispatcher.tsderives the principal from the first 16 characters of the bearer token. Add a case that asserts two different tokens with a shared 16-character prefix resolve to different principals. That test locks in the fix requested onlibs/sdk/src/transport/mcp-2026/dispatcher.tslines 73-79 and reaches the 95% coverage target for this path.🤖 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 `@libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts` around lines 115 - 150, Add a test covering resolvePrincipal in dispatcher.ts that supplies two distinct bearer tokens sharing the same first 16 characters and asserts their resolved principals differ. Keep the test focused on token-derived principal generation and ensure it exercises the collision fix in the relevant dispatcher path.Source: Coding guidelines
apps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.ts (1)
26-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the JSON-RPC code as well as the message.
Both tests match the message text
authenticated caller. That string lives inresolveTaskOwnerand can be reworded without a protocol change, which would break these tests for the wrong reason. Addexpect(res.json().error.code).toBe(-32602)so the contract is asserted on the code.🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.ts` around lines 26 - 46, Update the anonymous caller tests for tasks/get and tasks/update to assert that each response error code equals -32602 in addition to the existing message assertion. Keep the current message checks and request setup unchanged.apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts (2)
39-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType the destructured input request.
Both lines cast to
[string, any]. The coding guidelines forbidanywithout strong justification. Declare a small local type for the input-request shape and reuse it.♻️ Proposed change
+type InputRequest = { method: string; params: Record<string, any> }; @@ - const [, request] = Object.entries(result.inputRequests)[0] as [string, any]; + const [, request] = Object.entries(result.inputRequests)[0] as [string, InputRequest];Use
Record<string, unknown>forparamsif the assertions still type-check.Also applies to: 97-97
🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts` at line 39, Replace the `[string, any]` casts in both `Object.entries(result.inputRequests)` destructuring sites with a local input-request type, using `Record<string, unknown>` for `params` if compatible with the existing assertions. Reuse the type for both occurrences and remove the forbidden `any` usage.Source: Coding guidelines
39-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
anyis used to read 2026 wire payloads across the new E2E suites. The suites parse untyped JSON and reach foranyat each read site. The coding guidelines forbidanywithout strong justification. Declare the wire shapes once, for example ine2e/helpers/mcp-2026-client.ts, and import them into each suite.
apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts#L39-L97: replace bothas [string, any]casts with a sharedInputRequesttype.apps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.ts#L17-L40: type thepollUntilpredicate, return value andlastwith a shared task shape, and reuse the sameInputRequesttype on line 223.apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts#L188-L189: replace(t: any) => t.namewith(t: { name: string }) => t.name.🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts` around lines 39 - 97, Replace untyped JSON reads with shared wire types: define and export InputRequest and the task shape in e2e/helpers/mcp-2026-client.ts, then import them into each suite. In apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts:39-97, replace both [string, any] casts with InputRequest. In apps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.ts:17-40, type the pollUntil predicate, return value, and last variable with the shared task shape, and use InputRequest at line 223. In apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts:188-189, replace the any callback parameter with an object type containing name: string.Source: Coding guidelines
apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts (1)
188-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
anywith a typed shape.Line 189 annotates the map callback parameter as
any. The coding guidelines forbidanywithout strong justification and requireunknownor a constrained type instead. The other suites readt['name']from aRecord<string, unknown>, as on line 32.♻️ Proposed change
- expect(tools.map((t: any) => t.name)).toContain('echo'); + expect(tools.map((t: { name: string }) => t.name)).toContain('echo');🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts` around lines 188 - 189, Replace the any annotation on the map callback parameter in the listTools assertion with the established Record<string, unknown> shape used by the other suites, while preserving the existing name extraction and echo assertion.Source: Coding guidelines
libs/sdk/src/transport/mcp-2026/dispatcher.ts (1)
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFreeze
MRTR_CAPABLE_METHODS.The array is exported and mutable. A consumer can push a method into it and widen where
input_requiredresults are permitted. Useas constand areadonly string[]check, the same shapeMCP_2026_REMOVED_METHODSuses on line 264.♻️ Proposed change
-export const MRTR_CAPABLE_METHODS = ['tools/call', 'prompts/get', 'resources/read']; +export const MRTR_CAPABLE_METHODS = ['tools/call', 'prompts/get', 'resources/read'] as const;Then update the check on line 393:
if (!(MRTR_CAPABLE_METHODS as readonly string[]).includes(method)) {🤖 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 `@libs/sdk/src/transport/mcp-2026/dispatcher.ts` at line 64, Freeze the exported MRTR_CAPABLE_METHODS tuple by declaring it with as const, matching the readonly shape of MCP_2026_REMOVED_METHODS. Update the membership check in the dispatcher to treat MRTR_CAPABLE_METHODS as a readonly string array when calling includes.libs/sdk/src/transport/mcp-2026/result-decorator.ts (1)
69-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNote the per-page scope of the ordering guarantee.
orderListResultsorts the entries present in one result. If a handler paginates with a cursor, each page is sorted on its own, so the concatenation of pages is not globally sorted. The comment on lines 77-86 promises deterministic ordering without stating that limit. Add one sentence to the comment so a later reader does not rely on a global order.🤖 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 `@libs/sdk/src/transport/mcp-2026/result-decorator.ts` around lines 69 - 101, The comment for orderListResult should state that ordering is guaranteed only within each individual result page, not across concatenated paginated pages. Add one sentence to the existing documentation near the deterministic-ordering explanation, without changing the sorting implementation.libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts (1)
109-156: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAbort the dispatch when the SSE client disconnects.
handleSubscriptionsties anAbortControllerto the responsecloseevent. The streamed message path does not.dispatchOptionsomitssignal, sodispatch2026builds its own signal that never aborts (signal ?? new AbortController().signal). If the client closes the connection during a longtools/call, the generator stops being drained, but the tool keeps running to completion and holds its resources.Tie a controller to the response socket and forward it as
signalin the streamed branch.♻️ Proposed change
if (sink.active && acceptsEventStream(request.headers as Record<string, unknown> | undefined)) { + const controller = new AbortController(); + const { response } = this.rawInput; + (response as unknown as { on?: (event: string, cb: () => void) => void })?.on?.('close', () => + controller.abort(), + ); this.respond({ kind: 'sse', status: 200, - stream: streamMessageResponse(dispatchOptions, sink, body['id'], this.buildContextRunner()), + stream: streamMessageResponse( + { ...dispatchOptions, signal: controller.signal }, + sink, + body['id'], + this.buildContextRunner(), + ),Also applies to: 398-412
🤖 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 `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts` around lines 109 - 156, Update the streamed message path around streamMessageResponse to create an AbortController tied to the response/socket close event, pass its signal through the dispatch options to dispatch2026, and clean up the close listener when streaming finishes. Ensure client disconnection aborts an in-flight dispatch while preserving normal notification draining and response completion.libs/sdk/src/remote-mcp/mcp-client.types.ts (1)
87-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument
protocolVersionin the SDK reference.
McpHttpTransportOptionsis publicly exported, but itsprotocolVersionoption is absent fromdocs/frontmcp/**. Document its supported values and behavior.🤖 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 `@libs/sdk/src/remote-mcp/mcp-client.types.ts` around lines 87 - 96, Document the publicly exported McpHttpTransportOptions.protocolVersion option in the SDK reference under docs/frontmcp/**, covering the 'legacy', '2026-07-28', and 'auto' values and their respective transport and fallback behavior. Keep the documentation aligned with the existing property comment and avoid changing the TypeScript API.Source: Path instructions
libs/sdk/src/remote-mcp/mcp-client.service.ts (1)
798-803: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDocument or reject
protocolVersion: '2026-07-28'with SSE. Unconfigured and'legacy'HTTP connections return without a probe. SSE requests skip 2026 negotiation and silently use the legacy client, even when this option is set.🤖 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 `@libs/sdk/src/remote-mcp/mcp-client.service.ts` around lines 798 - 803, Update tryConnect2026 to explicitly handle protocolVersion '2026-07-28' for SSE transports: either document and support the option through the SSE path or reject it instead of silently falling back to the legacy client. Preserve the existing no-probe behavior for unconfigured and 'legacy' HTTP connections, and ensure configured 2026 requests cannot bypass negotiation.
🤖 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 `@apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts`:
- Around line 176-180: Update the discovery probe used by
negotiateRemoteProtocol to enforce a bounded timeout when Mcp2026Client.send
invokes fetchImpl. Create and pass an AbortSignal, abort it when the timeout
expires, and ensure the probe resolves to 'legacy' rather than hanging when the
remote is unreachable.
In `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts`:
- Around line 159-182: Update the test “rejects a requestState replayed onto a
different tool call” to omit inputResponses from the replayed request, making
the carried requestState the only possible completion source; assert the result
is input_required. Remove the now-unused key extraction from
interim.inputRequests.
In `@libs/sdk/src/auth/flows/oauth.register.flow.ts`:
- Around line 66-76: The OAuth registration flow must persist and enforce
application_type rather than only echoing it. Update the RegisteredClient model,
registration validator, and dcrClientRegistry.register() payload to carry the
value from request state, then make redirect validation allow native private-use
schemes such as com.example.app:/callback while retaining web-client
restrictions and existing allowlist behavior.
In `@libs/sdk/src/auth/instances/instance.local-primary-auth.ts`:
- Around line 203-216: Wire issuer through UpstreamProviderOptions,
ProviderConfig, and both automatic registration paths so the instance’s issuer
metadata is populated. Update discardProviderCredentials() to use the typed
TokenStore.deleteTokens(authorizationId, providerId) operation for every
authorization belonging to the provider, ensuring all credentials are removed.
Normalize trailing slashes on previous and new issuers before comparing them to
avoid unnecessary deletion.
In `@libs/sdk/src/index.ts`:
- Around line 430-442: Add SDK reference documentation under
docs/frontmcp/sdk-reference for the exported MCP 2026 symbols from the index:
Mcp2026Client, Mcp2026Error, TASKS_CLIENT_CAPABILITY, Mcp2026ClientOptions,
Mcp2026InputHandlers, validateHeaderParams, buildParamHeaders,
SUPPORTED_PROTOCOL_VERSIONS_2026, TASKS_EXTENSION_ID, Mcp2026ClientAdapter,
negotiateRemoteProtocol, and PROTOCOL_2026_07_28. Organize the reference
according to the existing SDK documentation conventions and document each API’s
purpose, usage, and relevant parameters or values.
In `@libs/sdk/src/remote-mcp/mcp-client.service.ts`:
- Around line 812-822: Update the McpClientConnection type to make transport
optional, remove the undefined cast in the returned connection object, and
narrow or guard transport at every use site before dereferencing it. Preserve
existing behavior for paths where a transport is present and handle the 2026
adapter path without runtime errors.
In `@libs/sdk/src/task/helpers/task-runner.ts`:
- Around line 98-115: Update the InputRequiredSignal handling in the task runner
and the underlying StorageTaskStore transition path to use a conditional atomic
update that only writes input_required when the task is not already terminal.
Ensure concurrent tasks/cancel operations cannot be overwritten, and base the
resulting notification/return behavior on the atomic update outcome rather than
a separate get/isTerminal check.
In `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts`:
- Around line 364-370: Validate the progressToken value before constructing
RequestNotificationSink: accept only strings or numbers, otherwise use
undefined. Update the progressToken extraction beside logLevel so invalid
client-provided objects and arrays cannot activate the sink or be echoed in
progress notifications.
In `@libs/sdk/src/transport/mcp-2026/client/header-params.ts`:
- Around line 33-44: Update validateHeaderParams to inspect every raw
x-mcp-header annotation rather than the deduplicated result of
collectHeaderParams. Preserve empty annotations for validation, track normalized
names with seen to reject case-insensitive duplicates, and retain the existing
token validation and error behavior; either add a duplicate-aware collector or
extend collectHeaderParams to report collisions.
In `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts`:
- Around line 394-397: Update the onMessage parameter in pumpSse to use
Record<string, unknown> instead of Record<string, any>, preserving the callback
signature and existing message handling.
- Around line 85-87: Update the Mcp2026Client constructor so maxInputRounds
retains its default when the provided option is explicitly undefined; normalize
that field after merging options, while preserving caller-supplied numeric
values and the existing options type contract.
- Around line 282-299: Update the acknowledgement promise in listen() to settle
when pumpSse completes without receiving
notifications/subscriptions/acknowledged, including stream closure, abort, or
error. Reuse the existing resolveAck path or add the necessary rejection
handling in the pumpSse completion flow, and ensure await acknowledged cannot
remain pending while preserving the current acknowledgement behavior.
In `@libs/sdk/src/transport/mcp-2026/dispatcher.ts`:
- Around line 73-79: Update resolvePrincipal to stop using a token prefix as the
principal; hash the full non-empty token with the existing sha256Hex utility
imported from `@frontmcp/utils`, while preserving clientId precedence and the
anonymous fallback.
In `@libs/sdk/src/transport/mcp-2026/mrtr.ts`:
- Around line 32-37: Update resolve() to validate elicitation requests against
the capability matching pending.mode (form or url), rather than only checking
that clientCapabilities.elicitation exists, before recording or replaying the
request. Make MissingClientCapabilityError use the corresponding required
capability shape for the requested mode, while preserving existing validation
for sampling and roots.
In `@libs/sdk/src/transport/mcp-2026/request-state.ts`:
- Around line 94-100: Move the custom timingSafeEqual implementation out of
request-state.ts and add the helper to `@frontmcp/utils` using its established
crypto utilities, then import and reuse that shared helper in the
signature-checking flow. Remove the local function while preserving
constant-time comparison behavior, including the unequal-length case.
- Around line 90-92: Update base64urlEncodeJson to use the base64url encoding
utility from `@frontmcp/utils` instead of Buffer, preserving JSON serialization
and the existing string result. Also replace any related decoding logic in the
request-state flow with base64urlDecode from `@frontmcp/utils` so the 2026
dispatcher works when Buffer is unavailable.
In `@libs/sdk/src/transport/mcp-2026/tasks-extension.ts`:
- Around line 181-188: Update the resumed-execution flow in
dispatchTasksMethod/resumeTask so tasks/update does not await tool execution:
schedule runner.run(...) in the background rather than awaiting resume, and
attach explicit rejection handling for failures. Preserve the existing update
payload, including inputRequests: undefined, and keep successful resumption
behavior unchanged.
---
Nitpick comments:
In `@apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts`:
- Around line 188-189: Replace the any annotation on the map callback parameter
in the listTools assertion with the established Record<string, unknown> shape
used by the other suites, while preserving the existing name extraction and echo
assertion.
In `@apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts`:
- Line 39: Replace the `[string, any]` casts in both
`Object.entries(result.inputRequests)` destructuring sites with a local
input-request type, using `Record<string, unknown>` for `params` if compatible
with the existing assertions. Reuse the type for both occurrences and remove the
forbidden `any` usage.
- Around line 39-97: Replace untyped JSON reads with shared wire types: define
and export InputRequest and the task shape in e2e/helpers/mcp-2026-client.ts,
then import them into each suite. In
apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts:39-97,
replace both [string, any] casts with InputRequest. In
apps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.ts:17-40, type the
pollUntil predicate, return value, and last variable with the shared task shape,
and use InputRequest at line 223. In
apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.ts:188-189, replace the any
callback parameter with an object type containing name: string.
In `@apps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.ts`:
- Around line 26-46: Update the anonymous caller tests for tasks/get and
tasks/update to assert that each response error code equals -32602 in addition
to the existing message assertion. Keep the current message checks and request
setup unchanged.
In `@libs/sdk/src/auth/flows/oauth.authorize.flow.ts`:
- Around line 253-263: Duplicate issuer-resolution logic exists in both OAuth
flows. Add one shared helper beside validateAuthorizationIssuer in
instance.local-primary-auth.ts that accepts scope.auth and preserves the
existing string validation, then remove resolveIssuer from
libs/sdk/src/auth/flows/oauth.authorize.flow.ts lines 253-263 and
libs/sdk/src/auth/flows/oauth.callback.flow.ts lines 146-157, updating both
flows to call the shared helper.
In `@libs/sdk/src/context/frontmcp-context.ts`:
- Around line 40-43: Update RequestNotificationSinkRef.log to use the
LoggingLevel type from `@frontmcp/protocol` instead of string, adding the type
import and preserving the existing logger and data parameters.
In `@libs/sdk/src/elicitation/helpers/mrtr-request.helper.ts`:
- Around line 73-100: Document the protected ToolContext.sample() and
ToolContext.listRoots() APIs in tool-context.mdx, using the existing public
method conventions. Describe their MRTR-only availability, protocol version
2026-07-28, required sampling or roots capability, and deprecation requirements;
do not document performSample or performListRoots as user-facing APIs.
In `@libs/sdk/src/remote-mcp/mcp-2026-client.adapter.ts`:
- Around line 87-113: Update the documentation comment for
negotiateRemoteProtocol to state that any failed server/discover probe,
including errors, or a response that does not advertise 2026-07-28 selects the
legacy path. Keep the probe implementation and its explicit-version behavior
unchanged.
In `@libs/sdk/src/remote-mcp/mcp-client.service.ts`:
- Around line 798-803: Update tryConnect2026 to explicitly handle
protocolVersion '2026-07-28' for SSE transports: either document and support the
option through the SSE path or reject it instead of silently falling back to the
legacy client. Preserve the existing no-probe behavior for unconfigured and
'legacy' HTTP connections, and ensure configured 2026 requests cannot bypass
negotiation.
In `@libs/sdk/src/remote-mcp/mcp-client.types.ts`:
- Around line 87-96: Document the publicly exported
McpHttpTransportOptions.protocolVersion option in the SDK reference under
docs/frontmcp/**, covering the 'legacy', '2026-07-28', and 'auto' values and
their respective transport and fallback behavior. Keep the documentation aligned
with the existing property comment and avoid changing the TypeScript API.
In `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts`:
- Around line 109-156: Update the streamed message path around
streamMessageResponse to create an AbortController tied to the response/socket
close event, pass its signal through the dispatch options to dispatch2026, and
clean up the close listener when streaming finishes. Ensure client disconnection
aborts an in-flight dispatch while preserving normal notification draining and
response completion.
In `@libs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.ts`:
- Around line 106-112: Move the hmacSha256 import from the test body into the
file’s top-level ESM imports, then use that imported symbol in the “rejects a
signed blob whose payload is not JSON” test. Remove the inline require while
preserving the existing signing and assertion behavior.
- Around line 115-150: Add a test covering resolvePrincipal in dispatcher.ts
that supplies two distinct bearer tokens sharing the same first 16 characters
and asserts their resolved principals differ. Keep the test focused on
token-derived principal generation and ensure it exercises the collision fix in
the relevant dispatcher path.
In `@libs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.ts`:
- Around line 1-150: Add focused tests for dispatchTasksMethod using a small
fake taskStore, covering missing-store handling, missing taskId validation,
owner-scoped not-found behavior, terminal-task cancel acknowledgement, rejection
of updates when status is not input_required, and merging inputResponses before
invoking resume. Follow the existing test style and exercise each branch to meet
the stated coverage target.
In `@libs/sdk/src/transport/mcp-2026/client/header-params.ts`:
- Around line 91-92: In the header parameter conversion logic, simplify the
redundant ternary assigned to asString in the surrounding parameter-building
function by directly converting value with String(value), preserving the
existing encoding and header assignment behavior.
In `@libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts`:
- Around line 219-249: Update awaitTask to handle task.ttlMs === null as an
unlimited lifetime instead of applying the 60-second fallback. Preserve numeric
TTL behavior, and use the existing Mcp2026ClientOptions configuration if a
finite polling deadline is required for unlimited tasks.
- Around line 302-320: Update Mcp2026ClientAdapter.send to create an
AbortController, pass its signal to fetchImpl, and abort after a configurable
request-timeout option. Clear the timeout in a finally block so it is released
on both successful responses and errors, while preserving the existing response
parsing and Mcp2026Error handling.
In `@libs/sdk/src/transport/mcp-2026/dispatcher.ts`:
- Line 64: Freeze the exported MRTR_CAPABLE_METHODS tuple by declaring it with
as const, matching the readonly shape of MCP_2026_REMOVED_METHODS. Update the
membership check in the dispatcher to treat MRTR_CAPABLE_METHODS as a readonly
string array when calling includes.
In `@libs/sdk/src/transport/mcp-2026/result-decorator.ts`:
- Around line 69-101: The comment for orderListResult should state that ordering
is guaranteed only within each individual result page, not across concatenated
paginated pages. Add one sentence to the existing documentation near the
deterministic-ordering explanation, without changing the sorting implementation.
🪄 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: 01fa6c26-06e3-4177-952e-db5f96a5319e
📒 Files selected for processing (48)
apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/request-notifications.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/index.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/chatty.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/list-workspaces.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/proto/tools/summarize.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/tasks/index.tsapps/e2e/demo-e2e-protocol-2026/src/apps/tasks/tools/approve-job.tool.tsapps/e2e/demo-e2e-protocol-2026/src/apps/tasks/tools/slow-job.tool.tsapps/e2e/demo-e2e-protocol-2026/src/main-tasks.tslibs/sdk/src/auth/__tests__/authorization-issuer.spec.tslibs/sdk/src/auth/flows/oauth.authorize.flow.tslibs/sdk/src/auth/flows/oauth.callback.flow.tslibs/sdk/src/auth/flows/oauth.provider-callback.flow.tslibs/sdk/src/auth/flows/oauth.register.flow.tslibs/sdk/src/auth/flows/well-known.oauth-authorization-server.flow.tslibs/sdk/src/auth/instances/instance.local-primary-auth.tslibs/sdk/src/common/interfaces/tool.interface.tslibs/sdk/src/context/frontmcp-context.tslibs/sdk/src/elicitation/helpers/index.tslibs/sdk/src/elicitation/helpers/mrtr-request.helper.tslibs/sdk/src/errors/index.tslibs/sdk/src/errors/mrtr.error.tslibs/sdk/src/index.tslibs/sdk/src/remote-mcp/mcp-2026-client.adapter.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/task/helpers/task-runner.tslibs/sdk/src/task/task.types.tslibs/sdk/src/transport/flows/handle.mcp-2026.flow.tslibs/sdk/src/transport/mcp-2026/__tests__/mrtr.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/request-notifications.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/result-decorator.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/tasks-extension.spec.tslibs/sdk/src/transport/mcp-2026/client/header-params.tslibs/sdk/src/transport/mcp-2026/client/index.tslibs/sdk/src/transport/mcp-2026/client/mcp-2026.client.tslibs/sdk/src/transport/mcp-2026/dispatcher.tslibs/sdk/src/transport/mcp-2026/index.tslibs/sdk/src/transport/mcp-2026/mrtr.tslibs/sdk/src/transport/mcp-2026/request-notifications.tslibs/sdk/src/transport/mcp-2026/request-state.tslibs/sdk/src/transport/mcp-2026/result-decorator.tslibs/sdk/src/transport/mcp-2026/tasks-extension.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/errors/index.ts
- libs/sdk/src/transport/mcp-2026/index.ts
🛑 Comments failed to post (2)
libs/sdk/src/transport/mcp-2026/request-state.ts (2)
90-92: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Does the SDK already avoid Buffer in favour of a runtime-neutral helper? rg -nP -C2 '\bBuffer\.(from|isBuffer)\b' libs/sdk/src --type=ts | head -50 rg -nP -C2 'base64url|toBase64Url|fromBase64Url' libs/utils/src --type=ts | head -50 fd -t f 'package.json' libs/edge --exec catRepository: agentfront/frontmcp
Length of output: 10415
🏁 Script executed:
#!/bin/bash set -e printf '%s\n' '--- request-state.ts ---' cat -n libs/sdk/src/transport/mcp-2026/request-state.ts | sed -n '1,180p' printf '%s\n' '--- utility exports and implementation ---' rg -n -C3 'base64urlEncode|base64urlDecode' libs/utils/src/index.ts libs/utils/src/crypto libs/utils/src --glob '*.ts' | head -160 printf '%s\n' '--- edge references to SDK transport/request state ---' rg -n -C2 'mcp-2026|request-state|`@frontmcp/sdk`|Buffer' libs/edge/src libs/edge/package.json --glob '*.ts' --glob 'package.json'Repository: agentfront/frontmcp
Length of output: 21720
🏁 Script executed:
#!/bin/bash set -e printf '%s\n' '--- request-state call sites and transport exports ---' rg -n -C3 'encodeRequestState|decodeRequestState|request-state|requestState|mcp-2026' libs/sdk/src libs/edge/src --glob '*.ts' | head -260 printf '%s\n' '--- runtime-neutral helper implementation ---' rg -n -C8 'function base64urlEncode|const base64urlEncode|base64urlEncode\s*=' libs/utils/src --glob '*.ts' printf '%s\n' '--- web handler and edge session host ---' rg -n -C5 'createWebFetchHandler|WebFetchSessionRouter|buildPersistentWebStandardMcp|runHttpRequestFlowWeb' libs/sdk/src libs/edge/src --glob '*.ts' | head -260Repository: agentfront/frontmcp
Length of output: 45720
🏁 Script executed:
#!/bin/bash set -e python3 - <<'PY' from pathlib import Path request_state = Path("libs/sdk/src/transport/mcp-2026/request-state.ts").read_text() utils = Path("libs/utils/src/crypto/index.ts").read_text() dispatcher = Path("libs/sdk/src/transport/mcp-2026/dispatcher.ts").read_text() edge = Path("libs/edge/src/index.ts").read_text() direct = [ "Buffer.from(JSON.stringify(value), 'utf8').toString('base64url')", "Buffer.from(hmacSha256(getRequestStateKey(), new TextEncoder().encode(body))).toString('base64url')", "JSON.parse(Buffer.from(body, 'base64url').toString('utf8'))", ] print("request-state direct Buffer calls:", all(x in request_state for x in direct)) print("request-state imports base64urlEncode/base64urlDecode:", "base64urlEncode" in request_state and "base64urlDecode" in request_state) print("utils guards Buffer:", "if (typeof Buffer !== 'undefined')" in utils) print("utils browser fallback:", "btoa" in utils and "atob" in utils) print("dispatcher invokes decodeRequestState:", "decodeRequestState(" in dispatcher) print("edge uses createWebFetchHandler:", "createWebFetchHandler(" in edge) PYRepository: agentfront/frontmcp
Length of output: 395
Use
@frontmcp/utilsfor base64url encoding and decoding. The 2026 dispatcher runs in the edge handler, whereBuffermay be absent. DirectBuffercalls can crash MRTR requests;base64urlEncodeandbase64urlDecodeprovide browser fallbacks.🤖 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 `@libs/sdk/src/transport/mcp-2026/request-state.ts` around lines 90 - 92, Update base64urlEncodeJson to use the base64url encoding utility from `@frontmcp/utils` instead of Buffer, preserving JSON serialization and the existing string result. Also replace any related decoding logic in the request-state flow with base64urlDecode from `@frontmcp/utils` so the 2026 dispatcher works when Buffer is unavailable.
94-100: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Move the constant-time comparison into
@frontmcp/utils.Lines 95-99 implement a hand-rolled constant-time comparison. The coding guidelines require all cryptographic operations to come from
@frontmcp/utilsand forbid custom crypto functions. Add atimingSafeEqualhelper to@frontmcp/utilsand import it here, so every signature check in the SDK shares one audited implementation.As per coding guidelines: "Always use
@frontmcp/utilsfor cryptographic operations ... Never usenode:cryptodirectly or implement custom crypto functions."🤖 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 `@libs/sdk/src/transport/mcp-2026/request-state.ts` around lines 94 - 100, Move the custom timingSafeEqual implementation out of request-state.ts and add the helper to `@frontmcp/utils` using its established crypto utilities, then import and reuse that shared helper in the signature-checking flow. Remove the local function while preserving constant-time comparison behavior, including the unequal-length case.Source: Coding guidelines
…-28 with new tools and error handling
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 124 changed files in this pull request and generated no new comments.
Suppressed comments (3)
libs/sdk/src/transport/mcp-2026/header-codec.ts:38
- decodeHeaderValue uses the global Buffer API unguarded. This will throw at runtime in Web/edge environments (e.g., when using Mcp2026Client in a browser) where Buffer is undefined; elsewhere in the codebase this is guarded (e.g. libs/sdk/src/auth/flows/oauth.token.flow.ts:102-104). Consider adding a Buffer/built-in base64 fallback so this module is runtime-safe across supported targets.
libs/sdk/src/transport/mcp-2026/header-codec.ts:50 - encodeHeaderValue also relies on global Buffer unguarded. For non-Node runtimes this will throw (or silently fail if Buffer is polyfilled inconsistently), but this function is used by Mcp2026Client for outgoing requests. Consider mirroring the Buffer/atob/btoa runtime check pattern used elsewhere so the 2026 client can run in browser/edge environments.
libs/sdk/src/transport/mcp-2026/discover.ts:52 - buildDiscoverResult returns an object missing the required CacheableResult fields (resultType/_meta/ttlMs/cacheScope) and works around this with
Omit<DiscoverResult, keyof object>, which effectively erases type checking. This makes it easy to accidentally return a malformed discover payload without TypeScript catching it. Prefer returning a precise "undecorated" shape (e.g. Pick of the method-specific fields) and let the decorator add the cache/meta fields at the transport boundary.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts (1)
313-330: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winValidate
params.notificationsand the JSON-RPC id before opening the subscription stream.
requestedis cast fromparams['notifications']toSubscriptionFilterwithout validation, andbody['id']is cast tostring | numberwithout validation. A client that sends a non-objectnotificationsvalue, or a non-arrayresourceSubscriptions, reachescreateSubscriptionStreamunchecked. This is the same gap flagged in a previous review on this file for lines 312-330: parseparams.notificationswith zod and validate the id shape before committing the SSE response, returning-32602on failure.🤖 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 `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts` around lines 313 - 330, Validate the JSON-RPC request before creating the response-bound subscription in the flow containing createSubscriptionStream: parse params.notifications with the existing zod schema so it rejects non-object filters and non-array resourceSubscriptions, and validate body.id against the supported JSON-RPC id shape. On either validation failure, return an -32602 invalid-params response and do not attach the response close handler or call createSubscriptionStream.
🧹 Nitpick comments (1)
libs/sdk/src/transport/mcp-2026/request-state.ts (1)
137-146: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReject oversized
requestStatebefore HMAC verification.
requestStateis attacker-controlled. Add a size limit that supports validInputResponses, and return{ ok: false, reason: 'malformed' }before callingsign(body). The existing4mblimit does not cover web-standard or pre-parsed request paths.🤖 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 `@libs/sdk/src/transport/mcp-2026/request-state.ts` around lines 137 - 146, Update decodeRequestState to enforce the requestState size limit before parsing or invoking sign(body), returning { ok: false, reason: 'malformed' } for oversized input. Use a limit that accommodates valid InputResponses across web-standard and pre-parsed request paths, while preserving existing validation for inputs within the limit.
🤖 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 `@docs/frontmcp/fundamentals/protocol-versions.mdx`:
- Around line 239-253: Update the SDK import in the documented client example to
reference a currently available public export from `@frontmcp/sdk`, or add the
required barrel export so Mcp2026Client is publicly available there. Keep the
example’s client initialization and usage unchanged.
---
Duplicate comments:
In `@libs/sdk/src/transport/flows/handle.mcp-2026.flow.ts`:
- Around line 313-330: Validate the JSON-RPC request before creating the
response-bound subscription in the flow containing createSubscriptionStream:
parse params.notifications with the existing zod schema so it rejects non-object
filters and non-array resourceSubscriptions, and validate body.id against the
supported JSON-RPC id shape. On either validation failure, return an -32602
invalid-params response and do not attach the response close handler or call
createSubscriptionStream.
---
Nitpick comments:
In `@libs/sdk/src/transport/mcp-2026/request-state.ts`:
- Around line 137-146: Update decodeRequestState to enforce the requestState
size limit before parsing or invoking sign(body), returning { ok: false, reason:
'malformed' } for oversized input. Use a limit that accommodates valid
InputResponses across web-standard and pre-parsed request paths, while
preserving existing validation for inputs within the limit.
🪄 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: 4b1830a5-bb06-42f0-9ed1-930a0e046044
📒 Files selected for processing (32)
apps/e2e/demo-e2e-protocol-2026/e2e/client.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/helpers/mcp-2026-client.tsapps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/mrtr.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/request-headers.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/request-notifications.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/stateless-requests.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.tsapps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.tsdocs/docs.jsondocs/frontmcp/fundamentals/protocol-versions.mdxlibs/sdk/src/auth/instances/instance.local-primary-auth.tslibs/sdk/src/context/frontmcp-context.tslibs/sdk/src/remote-mcp/mcp-2026-client.adapter.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/task/helpers/task-runner.tslibs/sdk/src/transport/flows/handle.mcp-2026.flow.tslibs/sdk/src/transport/mcp-2026/__tests__/header-params.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/mrtr.spec.tslibs/sdk/src/transport/mcp-2026/__tests__/request-state.spec.tslibs/sdk/src/transport/mcp-2026/client/header-params.tslibs/sdk/src/transport/mcp-2026/client/mcp-2026.client.tslibs/sdk/src/transport/mcp-2026/dispatcher.tslibs/sdk/src/transport/mcp-2026/header-codec.tslibs/sdk/src/transport/mcp-2026/mrtr.tslibs/sdk/src/transport/mcp-2026/request-state.tslibs/sdk/src/transport/mcp-2026/result-decorator.tslibs/sdk/src/transport/mcp-2026/tasks-extension.tslibs/skills/catalog/frontmcp-deployment/SKILL.mdlibs/skills/catalog/frontmcp-deployment/references/protocol-versions.mdlibs/skills/catalog/skills-manifest.json
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/e2e/demo-e2e-protocol-2026/e2e/tasks-anonymous.e2e.spec.ts
- apps/e2e/demo-e2e-protocol-2026/e2e/request-notifications.e2e.spec.ts
- libs/sdk/src/transport/mcp-2026/result-decorator.ts
- apps/e2e/demo-e2e-protocol-2026/e2e/request-headers.e2e.spec.ts
- apps/e2e/demo-e2e-protocol-2026/e2e/mrtr.e2e.spec.ts
- apps/e2e/demo-e2e-protocol-2026/e2e/stateless-requests.e2e.spec.ts
- libs/sdk/src/remote-mcp/mcp-2026-client.adapter.ts
- libs/sdk/src/transport/mcp-2026/tests/mrtr.spec.ts
- libs/sdk/src/task/helpers/task-runner.ts
- libs/sdk/src/remote-mcp/mcp-client.service.ts
- apps/e2e/demo-e2e-protocol-2026/e2e/tasks-extension.e2e.spec.ts
- libs/sdk/src/context/frontmcp-context.ts
- libs/sdk/src/transport/mcp-2026/dispatcher.ts
- libs/sdk/src/transport/mcp-2026/tasks-extension.ts
- libs/sdk/src/auth/instances/instance.local-primary-auth.ts
- libs/sdk/src/transport/mcp-2026/client/mcp-2026.client.ts
- libs/sdk/src/transport/mcp-2026/mrtr.ts
- libs/sdk/src/transport/mcp-2026/header-codec.ts
- apps/e2e/demo-e2e-protocol-2026/e2e/mrtr-sampling-roots.e2e.spec.ts
…26-07-28 to 20260728
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 256 out of 259 changed files in this pull request and generated no new comments.
Suppressed comments (2)
libs/sdk/src/transport/mcp-20260728/discover.ts:55
buildDiscoverResultis typed asOmit<DiscoverResult, keyof object>, butkeyof objectisnever, so this doesn’t omit anything and effectively lies about which fields are returned. That defeats type-safety around requiredCacheableResultfields (resultType,ttlMs,cacheScope) that this function intentionally does NOT include (they’re added by the result decorator).
libs/sdk/src/errors/mrtr.error.ts:58- There’s an orphaned/duplicated JSDoc block: the “missing client capability” documentation appears before
SamplingNotAvailableError, so it’s not attached to the class it describes. This makes the generated docs misleading and hides the intent ofMissingClientCapabilityError.
…
Summary by CodeRabbit
New Features
Documentation
Tests
Chores