feat: Add support for request context management and session key validation #120
Conversation
frontegg-david
changed the title
WalkthroughReplaces the SessionProvider model with a unified FrontMcpContext backed by AsyncLocalStorage; removes the old SessionProvider export; adds FrontMcpContext, storage/provider tokens, trace-context utilities, session-key validation, session/context-scoped provider caching and locks, flow/adapter integrations, docs, and tests. Changes
sequenceDiagram
actor Client
participant HTTP as HTTP Router
participant CtxStore as FrontMcpContextStorage
participant FlowReg as FlowRegistry
participant ProvReg as ProviderRegistry
participant FlowInst as FlowInstance / FlowClass
Client->>HTTP: request (headers, body)
HTTP->>CtxStore: runFromHeaders(headers, args, fn)
CtxStore->>CtxStore: parseTraceContext(), extractMetadata()
CtxStore->>HTTP: execute handler inside AsyncLocalStorage (FrontMcpContext)
HTTP->>FlowReg: runFlow(..., ctx)
FlowReg->>ProvReg: buildViews(sessionKey, contextProviders)
ProvReg->>ProvReg: acquireSessionLock / build context providers
ProvReg-->>FlowReg: ProviderViews (global + context)
FlowReg->>FlowInst: construct with merged deps (includes FRONTMCP_CONTEXT)
FlowInst->>CtxStore: access FrontMcpContext (getAuthInfo, fetch, marks, storage)
FlowInst-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
libs/{sdk,adapters,plugins,cli}/src/**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
libs/**⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (2)📚 Learning: 2025-12-01T00:33:33.644ZApplied to files:
📚 Learning: 2025-12-01T00:33:33.644ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/adapters/src/openapi/openapi.tool.ts (1)
108-119: Verify auth data is properly validated before use.
authInfofromctx.getAuthInfo()is passed directly to bothinjectTransformedValuesandresolveToolSecuritywithout null checks. WhileresolveToolSecuritydoes validateauthInfo.tokenis a string (lines 125-126 in openapi.security.ts), andinjectTransformedValueshas error handling viasafeInjectfor transform failures, there's no upfront validation thatauthInfoitself is non-null or that required fields exist. Consider adding a guard at line 104 to ensureauthInfois valid before proceeding, or document the assumption thatctx.getAuthInfo()always returns a validAuthInfoobject.
🧹 Nitpick comments (10)
libs/sdk/src/context/request-context.provider.ts (1)
41-52: Consider removing theas anytype assertion.The factory logic is correct, but the
as anyassertion at line 52 bypasses TypeScript's type checking. This suggests a potential mismatch with theProviderFactoryTypedefinition.If the type assertion is necessary due to framework limitations, consider adding a comment explaining why it's required.
libs/sdk/src/scope/flows/http.request.flow.ts (1)
109-122: PreferrandomUUID()for fallbackrequestIdand avoid Math.random identifiers.
Line 111 fallback can collide under load; userandomUUID()(or a monotonic counter) ifctxis missing.libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (3)
24-27: Avoidanyin tests (strict TS).
Line 25:target: anycan beunknown(orabstract new (...args: unknown[]) => unknown) since it’s unused.
241-276: Use Jest fake timers for deterministic timing tests.
RealsetTimeout/Date.now()can be flaky under CI contention; switch these tojest.useFakeTimers()+jest.advanceTimersByTime()and (if needed) stubDate.now().Also applies to: 377-396
345-376: Reduce test iteration count with a configurable cache size for LRU eviction tests.All three LRU eviction tests (lines 345–376, 397–428, 787–813) use
MAX_SIZE = 10000with 10,000-iteration loops to fill the session cache before triggering eviction. These loops can slow CI unnecessarily. SinceProviderRegistry.MAX_SESSION_CACHE_SIZEis a private constant with no test-only override, consider adding a constructor parameter or test helper to use a smaller cache size (e.g., 10–50 entries) for these tests while keeping production code unchanged.libs/sdk/src/context/request-context-storage.ts (1)
185-198:extractClientIp()should handle array header values too.
If upstream adapters ever passstring[], you’ll miss client IP. Consider acceptingstring | string[]and using the first value.libs/sdk/src/context/request-context.ts (1)
17-28: NormalizeRequestMetadata.customHeadersdefensively (JS/runtime callers can omit it).
Right now you rely on TS to enforcecustomHeaders, but at runtimeargs.metadatacan still be missing it, yieldingundefineddownstream.constructor(args: RequestContextArgs) { @@ - this.metadata = args.metadata ?? { customHeaders: {} }; + const md = args.metadata; + this.metadata = { + ...md, + customHeaders: md?.customHeaders ?? {}, + };Also applies to: 94-105
libs/sdk/src/provider/provider.registry.ts (3)
41-65: Cache sizing/TTL constants should be configurable (or at least justified) for SDK consumers.
MAX_SESSION_CACHE_SIZE = 10000+ 1h TTL can be large depending on provider weight and deployment style.
933-934: New request/session build paths introduce additionalany—preferunknown(or tighten generics) per TS guidelines.
In particular:sessionStore as Map<Token, any>andconst args: any[] = []weaken type guarantees for new code paths.Also applies to: 969-1023
1034-1075: Resolution order is sensible; consider enriching failure errors with scope + session/request context.
Cannot resolve dependency ... from viewscan be hard to debug; including the provider scope + whether SESSION/REQUEST stores had it would cut triage time.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/demo/src/apps/expenses/providers/session-redis.provider.ts(2 hunks)docs/draft/docs.json(1 hunks)docs/draft/docs/adapters/openapi-adapter.mdx(1 hunks)docs/draft/docs/extensibility/providers.mdx(1 hunks)docs/draft/docs/extensibility/request-context.mdx(1 hunks)docs/draft/docs/servers/tools.mdx(2 hunks)libs/adapters/src/openapi/openapi.tool.ts(4 hunks)libs/sdk/src/common/index.ts(0 hunks)libs/sdk/src/common/interfaces/execution-context.interface.ts(3 hunks)libs/sdk/src/common/interfaces/flow.interface.ts(2 hunks)libs/sdk/src/common/providers/session.provider.ts(0 hunks)libs/sdk/src/context/__tests__/session-key.test.ts(1 hunks)libs/sdk/src/context/index.ts(1 hunks)libs/sdk/src/context/request-context-storage.ts(1 hunks)libs/sdk/src/context/request-context.provider.ts(1 hunks)libs/sdk/src/context/request-context.ts(1 hunks)libs/sdk/src/context/session-key.provider.ts(1 hunks)libs/sdk/src/context/trace-context.ts(1 hunks)libs/sdk/src/flows/flow.instance.ts(4 hunks)libs/sdk/src/index.ts(1 hunks)libs/sdk/src/notification/notification.service.ts(1 hunks)libs/sdk/src/provider/__tests__/provider-session-scope.test.ts(1 hunks)libs/sdk/src/provider/provider.registry.ts(4 hunks)libs/sdk/src/scope/flows/http.request.flow.ts(3 hunks)libs/sdk/src/scope/scope.instance.ts(2 hunks)libs/ui/project.json(2 hunks)
💤 Files with no reviewable changes (2)
- libs/sdk/src/common/index.ts
- libs/sdk/src/common/providers/session.provider.ts
🧰 Additional context used
📓 Path-based instructions (8)
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/adapters/openapi-adapter.mdxdocs/draft/docs/extensibility/providers.mdxdocs/draft/docs/extensibility/request-context.mdxdocs/draft/docs/servers/tools.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/adapters/openapi-adapter.mdxdocs/draft/docs/extensibility/providers.mdxdocs/draft/docs/extensibility/request-context.mdxdocs/draft/docs.jsondocs/draft/docs/servers/tools.mdx
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/notification/notification.service.tslibs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/context/request-context.provider.tslibs/sdk/src/index.tslibs/sdk/src/common/interfaces/flow.interface.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/adapters/src/openapi/openapi.tool.tslibs/sdk/src/context/index.tslibs/sdk/src/context/trace-context.tslibs/sdk/src/context/session-key.provider.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/request-context-storage.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/request-context.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/scope/flows/http.request.flow.tsapps/demo/src/apps/expenses/providers/session-redis.provider.tslibs/sdk/src/scope/scope.instance.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/notification/notification.service.tslibs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/context/request-context.provider.tslibs/sdk/src/index.tslibs/sdk/src/common/interfaces/flow.interface.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/adapters/src/openapi/openapi.tool.tslibs/sdk/src/context/index.tslibs/sdk/src/context/trace-context.tslibs/sdk/src/context/session-key.provider.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/request-context-storage.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/request-context.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/scope/scope.instance.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/notification/notification.service.tslibs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/context/request-context.provider.tslibs/sdk/src/index.tslibs/sdk/src/common/interfaces/flow.interface.tslibs/ui/project.jsonlibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/adapters/src/openapi/openapi.tool.tslibs/sdk/src/context/index.tslibs/sdk/src/context/trace-context.tslibs/sdk/src/context/session-key.provider.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/request-context-storage.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/request-context.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/scope/scope.instance.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
libs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Files:
libs/sdk/src/index.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/apps/expenses/providers/session-redis.provider.ts
🧠 Learnings (8)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
libs/sdk/src/context/__tests__/session-key.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
libs/sdk/src/context/__tests__/session-key.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/sdk/src/index.tslibs/sdk/src/context/index.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Applied to files:
libs/sdk/src/common/interfaces/flow.interface.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/common/interfaces/flow.interface.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
libs/sdk/src/context/index.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/scope/scope.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Create shared base classes for common functionality across context types (ExecutionContextBase provides get(), tryGet(), scope, fail(), mark(), fetch())
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.ts
🧬 Code graph analysis (10)
libs/sdk/src/notification/notification.service.ts (1)
libs/testing/src/client/mcp-test-client.ts (1)
sessionId(206-208)
libs/sdk/src/context/__tests__/session-key.test.ts (1)
libs/sdk/src/context/session-key.provider.ts (1)
SessionKey(25-62)
libs/sdk/src/common/interfaces/flow.interface.ts (3)
libs/sdk/src/context/index.ts (2)
RequestContext(17-17)RequestContextStorage(20-20)libs/sdk/src/context/request-context.ts (1)
RequestContext(60-258)libs/sdk/src/index.ts (2)
RequestContext(16-16)RequestContextStorage(19-19)
libs/adapters/src/openapi/openapi.tool.ts (4)
libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
authInfo(89-91)libs/sdk/src/context/request-context.ts (1)
authInfo(111-113)libs/adapters/src/openapi/openapi.types.ts (1)
InputTransformContext(13-20)libs/adapters/src/openapi/openapi.security.ts (1)
resolveToolSecurity(282-325)
libs/sdk/src/context/session-key.provider.ts (2)
libs/sdk/src/context/index.ts (1)
SessionKey(26-26)libs/sdk/src/index.ts (1)
SessionKey(25-25)
libs/sdk/src/common/interfaces/execution-context.interface.ts (4)
libs/sdk/src/context/request-context.ts (2)
authInfo(111-113)RequestContext(60-258)libs/sdk/src/context/index.ts (2)
RequestContext(17-17)REQUEST_CONTEXT(23-23)libs/sdk/src/index.ts (2)
RequestContext(16-16)REQUEST_CONTEXT(20-20)libs/sdk/src/context/request-context.provider.ts (1)
REQUEST_CONTEXT(30-30)
libs/sdk/src/provider/provider.registry.ts (2)
libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
ProviderViews(74-81)libs/sdk/src/context/session-key.provider.ts (1)
SessionKey(25-62)
libs/sdk/src/scope/flows/http.request.flow.ts (1)
libs/sdk/src/auth/session/record/session.base.ts (1)
user(157-159)
apps/demo/src/apps/expenses/providers/session-redis.provider.ts (3)
libs/sdk/src/context/index.ts (1)
SessionKey(26-26)libs/sdk/src/context/session-key.provider.ts (1)
SessionKey(25-62)libs/sdk/src/index.ts (1)
SessionKey(25-25)
libs/sdk/src/scope/scope.instance.ts (3)
libs/sdk/src/context/index.ts (2)
RequestContextStorage(20-20)RequestContextProvider(23-23)libs/sdk/src/index.ts (1)
RequestContextStorage(19-19)libs/sdk/src/context/request-context.provider.ts (1)
RequestContextProvider(41-52)
🪛 Biome (2.1.2)
libs/sdk/src/provider/provider.registry.ts
[error] 991-991: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (36)
libs/ui/project.json (1)
46-47: Clarify the relationship between external bundling changes and RequestContext feature.The addition of external packages (
onnxruntime-*,esbuild,@huggingface/transformers) to both build targets is unclear in the context of a PR focused on RequestContext and session key validation. These appear to be AI/ML-related dependencies.Please verify:
- Why these external dependencies are needed for the RequestContext feature
- Whether these packages are actually imported or used by the UI library code
- Whether documentation needs to be updated to reflect these bundling changes and any new consumer requirements
Additionally, marking
esbuildas external is unusual since it's typically a dev-only dependency. Please clarify the intent there.Also applies to: 89-90
docs/draft/docs/adapters/openapi-adapter.mdx (1)
187-191: LGTM! Clear documentation of auth data flow.The informational block accurately describes that
authInfois sourced from the authenticated request context and that authentication is verified before tool execution. This aligns with the broader request context changes in the PR.libs/sdk/src/context/__tests__/session-key.test.ts (1)
1-137: LGTM! Comprehensive test coverage for SessionKey validation.The test suite thoroughly covers:
- Constructor validation with edge cases (empty, null, undefined, length limits)
- Valid and invalid character patterns
- Static validation method
- Constants verification
- Edge cases for special characters (colons)
All critical paths are tested, aligning with the 95%+ coverage requirement.
Based on learnings, the test suite follows best practices for error class validation and covers all code paths including constructor validation and error scenarios.
libs/sdk/src/context/trace-context.ts (1)
1-162: LGTM! Solid W3C Trace Context implementation.The implementation correctly:
- Parses W3C traceparent headers with proper validation
- Falls back to custom headers and generation
- Validates trace IDs (32 hex chars, not all zeros)
- Validates parent IDs (16 hex chars, not all zeros)
- Generates trace contexts with proper formatting
- Creates child span contexts while preserving trace ID
All functions follow the W3C Trace Context specification accurately.
docs/draft/docs.json (1)
112-112: LGTM! Navigation update for new documentation page.The addition of the request-context documentation page to the Extensibility group is correctly positioned and aligns with the new request context features introduced in this PR.
libs/sdk/src/notification/notification.service.ts (1)
418-421: LGTM! Critical session cleanup to prevent memory leaks.The addition of
scope.providers.cleanupSession(sessionId)ensures that session-scoped providers are properly cleaned up when sessions are terminated. This prevents memory leaks from terminated sessions that would otherwise remain cached indefinitely.The placement is correct: cleanup occurs after unregistering the server but before tracking the termination, ensuring proper resource management.
libs/adapters/src/openapi/openapi.tool.ts (1)
103-104: LGTM! Unified auth retrieval pattern.The change to use
ctx.getAuthInfo()aligns with the SDK's unified authentication access pattern introduced in this PR. This ensures consistent auth data sourcing across all tool executions.libs/sdk/src/index.ts (1)
14-26: LGTM! Clean public API exports for request context.The new export block properly exposes the request context API (RequestContext, tracing utilities, SessionKey) following the barrel export pattern. No legacy exports or aliases are introduced, adhering to coding guidelines.
As per coding guidelines, the barrel exports cleanly expose the public API surface without legacy exports or aliases.
docs/draft/docs/servers/tools.mdx (2)
398-399: LGTM! Clear deprecation notice.The comment properly marks
this.authInfoas deprecated and directs users to preferthis.requestContext.authInfo, aligning with the unified auth access pattern introduced in this PR.
440-484: LGTM! Comprehensive request context documentation.The new section provides excellent examples of using the request context API, including:
- Tracing with W3C Trace Context (requestId, traceId, parentId)
- Authentication access via
ctx.authInfo- Session identification
- Performance timing with
mark()andelapsed()- Request metadata access
The examples are clear, practical, and demonstrate the full capabilities of the request context feature.
libs/sdk/src/scope/scope.instance.ts (2)
18-18: LGTM: Import aligns with new context module.The import correctly references the new RequestContextStorage and RequestContextProvider from the context module barrel.
273-282: LGTM: Clear migration to global RequestContextStorage.The comments effectively explain the architectural decision:
- RequestContextStorage is GLOBAL because it manages AsyncLocalStorage infrastructure
- RequestContextProvider factory (REQUEST scope) retrieves per-request data from the storage
The scope separation is correct and well-documented.
libs/sdk/src/context/session-key.provider.ts (4)
1-24: LGTM: Clear documentation with security rationale.The JSDoc effectively explains the DI injection pattern and articulates the security concerns (memory bombs, log injection, cache exhaustion) that validation prevents.
27-27: LGTM: Reasonable max length for session keys.256 characters is a sensible defensive limit that accommodates typical session ID formats (UUIDs, short JWTs, namespaced IDs) while preventing memory exhaustion attacks.
29-35: LGTM: Restrictive pattern prevents injection attacks.The regex pattern correctly allows safe characters for session IDs while blocking special characters that could enable log injection or other attacks. Colon support for namespaced IDs (e.g.,
"anon:uuid") is a good design choice.
44-61: LGTM: Solid validation with clear error messages.The validation logic is defensive and fails fast at construction time. The static
validate()method enables early validation before instantiation, and the constructor correctly delegates to avoid duplication.docs/draft/docs/extensibility/request-context.mdx (3)
25-101: LGTM: Code examples align with implementation.The examples correctly demonstrate:
this.requestContextgetter usagethis.get(REQUEST_CONTEXT)alternative- Safe access via
tryGetRequestContext()- REQUEST-scoped factory provider pattern
All examples are consistent with the implementation.
73-111: LGTM: W3C Trace Context documentation is accurate.The documented headers and format align with W3C Trace Context standards. The example for propagating trace context to downstream services is functional, though the span ID generation from
requestIdis somewhat ad-hoc (line 98).
224-266: LGTM: API reference is comprehensive and accurate.The tables correctly document the RequestContext API, including the important note that
authInfoisPartial<AuthInfo>because it's progressively populated during the request lifecycle.apps/demo/src/apps/expenses/providers/session-redis.provider.ts (2)
2-2: LGTM: Clean migration to SessionKey pattern.The import and constructor signature correctly adopt the new SessionKey class for DI injection in SESSION-scoped providers, aligning with the documented pattern.
Also applies to: 12-12
13-13: LGTM: Session ID access updated correctly.The migration from
sessionProvider.session!.idtosessionKey.valueis correct. The normalization logic (trim, whitespace→underscore, colon→underscore) is defensive, though whitespace replacement is redundant sinceSessionKey.validate()already rejects whitespace.libs/sdk/src/context/request-context.provider.ts (1)
30-30: LGTM: Global symbol registry prevents token duplication.Using
Symbol.for('frontmcp:REQUEST_CONTEXT')ensures the token is accessible across module boundaries and prevents DI token collisions through namespacing.docs/draft/docs/extensibility/providers.mdx (3)
76-154: LGTM: Request Context documentation is accurate and complete.The examples correctly demonstrate both access patterns (
this.requestContextandthis.get(REQUEST_CONTEXT)), and the API table provides a helpful quick reference with a link to the full guide.
158-258: LGTM: SESSION provider guidance is comprehensive.The documentation clearly explains:
- SessionKey injection pattern
- Practical Redis example
- Critical multi-pod deployment considerations
The warning about pod-local behavior and guidance for cross-pod data sharing via Redis are essential for production deployments.
213-216: Excellent: Critical multi-pod deployment warning.This warning prevents a common production pitfall. Clearly distinguishing between pod-local SESSION providers and shared transport state (via Redis) will save developers from debugging distributed state issues.
libs/sdk/src/context/index.ts (1)
1-26: LGTM: Clean barrel exports with helpful documentation.The exports follow the clean barrel pattern without aliases or legacy exports. The JSDoc header provides a helpful overview of the module's purpose and key exports.
Based on learnings, this aligns with the barrel export guidelines.
libs/sdk/src/common/interfaces/flow.interface.ts (3)
7-8: LGTM: Proper use of type-only import.Line 7 uses
import typefor RequestContext (type-only), while line 8 imports RequestContextStorage as a value (needed at runtime). This optimizes tree-shaking.
87-96: LGTM: Fail-fast context access is correct.The getter correctly retrieves RequestContextStorage from the DI container and calls
getStoreOrThrow()to fail fast when context is unavailable. The JSDoc clearly documents the error behavior.
98-109: LGTM: Safe context access for optional scenarios.The try-catch wrapper correctly handles cases where RequestContext might not be available (initialization, non-HTTP flows). This provides a graceful fallback without disrupting execution.
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (1)
47-161: Coverage looks strong forbuildViewsvalidation + caching behaviors.
Early validation, SESSION caching, and REQUEST non-caching are well covered and aligned with the PR goals.libs/sdk/src/flows/flow.instance.ts (1)
218-227: Verify SESSION provider precedence over GLOBAL-provided tokens.
The current code (lines 223–226) only adds SESSION providers when!mergedDeps.has(token). Sincedepscontains GLOBAL bindings, SESSION providers cannot override GLOBAL, resulting in REQUEST > GLOBAL > SESSION precedence. In a standard hierarchical DI system, the expected precedence should be REQUEST > SESSION > GLOBAL. If SESSION scope should override GLOBAL scope, change the condition to unconditionally set SESSION providers.libs/sdk/src/context/request-context.ts (4)
166-170: Logger prefixing looks good for correlating request/trace quickly.
Slicing IDs keeps log lines readable.
1-6: Public API addition: ensure matching docs are added underdocs/draft/docs/**(notdocs/docs/**).
This is a new core SDK API and should be documented as part of the release surface.# Check for corresponding draft docs updates describing RequestContext/RequestContextStorage usage. fd -a "docs" . rg -n "RequestContext|REQUEST_CONTEXT|RequestContextStorage" docs -SAlso applies to: 50-59
115-126: The@internalJSDoc comments are already present—verify ifstripInternalshould be enabled intsconfigto enforce them.
updateAuthInfo()(line 121) andupdateSessionMetadata()(line 151) are already marked with@internalbut are exported through the publicRequestContextAPI. If these should truly be internal, enablestripInternalin the TypeScript compiler options to automatically exclude them from generated.d.tsfiles; otherwise, document them as intentional public methods and remove@internal.Likely an incorrect or invalid review comment.
33-48: sessionId is appropriately required in RequestContextArgs.All flow execution paths provide sessionId automatically: either from the
mcp-session-idheader or generated asanon:{UUID}for anonymous requests. This pattern is intentional (see flow.instance.ts lines 177–178) to prevent session collision in unauthenticated flows. Callers never fabricate values; the system ensures sessionId is always available before RequestContext construction.libs/sdk/src/provider/provider.registry.ts (1)
1085-1094:getScoped()lookup order matches the documented precedence.
Nice, simple API for callers once they haveProviderViews.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
libs/sdk/src/flows/flow.instance.ts (1)
90-95: Remainingas anycast breaks strict TypeScript guarantees.Line 93 still uses
{ request, response, next } as anywhich was flagged in a previous review. Consider introducing a typed interface for middleware input to maintain type safety.+// At top of file or in common types +interface HttpMiddlewareInput { + request: ServerRequest; + response: unknown; // or specific response type + next: () => void; +} // In runWithContext call - { request, response, next } as any, + { request, response, next } as HttpMiddlewareInput,libs/sdk/src/context/request-context-storage.ts (1)
141-146: Previous review feedback implemented correctly.The
updateAuthInfomethod now mutates the existing context in place viacurrent.updateAuthInfo(authInfo), preserving internal state (marks, store, sessionMetadata) as suggested in prior review.libs/sdk/src/provider/provider.registry.ts (4)
215-223: Background cleanup initialization looks correct with dispose() now available.The cleanup is correctly started only for scope-level registries (with
parentProviders). Thedispose()method at lines 812-821 addresses the lifecycle concern from prior review.Verify that
dispose()is called during scope teardown:#!/bin/bash # Check if dispose() is called in scope teardown logic rg -n "\.dispose\(\)|providers\.dispose" libs/sdk/src --type=ts -C 2
700-719: LRU eviction correctly skips locked sessions.The eviction logic properly protects sessions being actively built. While the cache can temporarily exceed
MAX_SESSION_CACHE_SIZEif all sessions are locked (as noted in prior review), this edge case is unlikely in practice and self-corrects once locks release.
873-880: Timer leak fix correctly implemented.The
clearTimeout(timeoutHandle)afterPromise.raceproperly prevents timer leaks whenexisting.promisewins the race. This addresses the prior review concern.
932-973: Session key validation now usesInvalidInputErrorviaSessionKey.validate().The prior review concern about using MCP-coded error classes is addressed—
SessionKey.validate()(shown in relevant snippets) throwsInvalidInputError. The try-finally pattern correctly ensures lock release even on error.
🧹 Nitpick comments (1)
libs/sdk/src/scope/flows/http.request.flow.ts (1)
231-250: Duplicate AuthInfo mapping - consider extracting to a helper.This mapping (lines 235-246) duplicates the logic in
checkAuthorization(lines 178-189). Consider extracting to a shared helper function likebuildAuthInfoFromAuthorization(authorization)to ensure consistency and reduce maintenance burden.// Example helper (could be a private method or utility function) private buildAuthInfoFromAuthorization(authorization: { token: string; user?: { sub?: string; exp?: number }; session?: { id?: string; payload?: unknown } }): Partial<AuthInfo> { return { token: authorization.token, clientId: authorization.user?.sub, scopes: [], expiresAt: authorization.user?.exp ? authorization.user.exp * 1000 : undefined, extra: { user: authorization.user, sessionId: authorization.session?.id, sessionPayload: authorization.session?.payload, }, }; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/sdk/src/common/interfaces/execution-context.interface.ts(3 hunks)libs/sdk/src/context/request-context-storage.ts(1 hunks)libs/sdk/src/context/request-context.ts(1 hunks)libs/sdk/src/context/session-key.provider.ts(1 hunks)libs/sdk/src/errors/mcp.error.ts(1 hunks)libs/sdk/src/flows/flow.instance.ts(4 hunks)libs/sdk/src/provider/provider.registry.ts(4 hunks)libs/sdk/src/scope/flows/http.request.flow.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/errors/mcp.error.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/session-key.provider.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/context/request-context.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/request-context-storage.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/errors/mcp.error.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/session-key.provider.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/context/request-context.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/request-context-storage.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/errors/mcp.error.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/session-key.provider.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/context/request-context.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/request-context-storage.ts
🧠 Learnings (10)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/errors/mcp.error.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/provider/provider.registry.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Create shared base classes for common functionality across context types (ExecutionContextBase provides get(), tryGet(), scope, fail(), mark(), fetch())
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/provider/provider.registry.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Enable strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use type parameters with constraints instead of unconstrained generics, and prefer `unknown` over `any` for generic type defaults
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Applied to files:
libs/sdk/src/flows/flow.instance.ts
🧬 Code graph analysis (4)
libs/sdk/src/errors/mcp.error.ts (1)
libs/sdk/src/errors/index.ts (1)
InternalMcpError(5-5)
libs/sdk/src/context/session-key.provider.ts (3)
libs/sdk/src/context/index.ts (1)
SessionKey(26-26)libs/sdk/src/index.ts (1)
SessionKey(25-25)libs/sdk/src/errors/mcp.error.ts (1)
InvalidInputError(223-240)
libs/sdk/src/scope/flows/http.request.flow.ts (2)
libs/testing/src/client/mcp-test-client.ts (2)
request(818-867)session(210-226)libs/sdk/src/auth/session/record/session.base.ts (1)
user(157-159)
libs/sdk/src/provider/provider.registry.ts (3)
libs/sdk/src/common/interfaces/internal/registry.interface.ts (1)
ProviderViews(74-81)libs/sdk/src/context/session-key.provider.ts (1)
SessionKey(27-64)libs/sdk/src/context/index.ts (1)
SessionKey(26-26)
🪛 Biome (2.1.2)
libs/sdk/src/provider/provider.registry.ts
[error] 1020-1020: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (29)
libs/sdk/src/context/session-key.provider.ts (2)
46-58: LGTM - Clean validation implementation with specific error class.The validation logic is well-structured with clear security rationale. Minor note: the
value.length === 0check on line 47 is technically redundant since!valuealready covers empty strings, but keeping it explicit improves readability.
27-64: Well-designed value object with appropriate security constraints.Good use of
InvalidInputErrorper coding guidelines. The staticvalidate()method enables early validation before cache operations, and the constructor ensures all instances are valid.libs/sdk/src/scope/flows/http.request.flow.ts (2)
110-124: Good use of cryptographic randomUUID for fallback request IDs.Using
randomUUID()instead ofMath.random()prevents collision risks under load. The sessionId fallback chain is clear, with 'no-session' explicitly documented as a logging fallback only.
170-191: AuthInfo mapping correctly converts JWT exp to milliseconds.The
expiresAt: user.exp ? user.exp * 1000 : undefinedconversion (line 183) aligns with the SDK's millisecond-based comparisons. The extra fields provide useful context for downstream consumers.libs/sdk/src/errors/mcp.error.ts (1)
361-371: LGTM - Well-structured specific error class for missing request context.The
RequestContextNotAvailableErrorfollows the established error pattern, uses a specific MCP error code (REQUEST_CONTEXT_NOT_AVAILABLE), and provides an actionable default message. This is the correct approach per coding guidelines.libs/sdk/src/common/interfaces/execution-context.interface.ts (3)
53-68: Good implementation using the typed MCP error.The
requestContextgetter now throwsRequestContextNotAvailableErrorinstead of a genericError, addressing the previous review comment. The implementation correctly catches any provider resolution failure and throws the specific error.
92-107: Progressive auth info pattern with proper fallback.Returning
Partial<AuthInfo>correctly reflects that auth info is populated progressively during the request lifecycle. The preference forrequestContext.authInfowith fallback to_authInfoprovides backward compatibility.
70-81: Safe fallback pattern for optional context access.The
tryGetRequestContext()method provides a non-throwing alternative for code paths where context may not be available (e.g., during initialization). This is a good defensive pattern.libs/sdk/src/flows/flow.instance.ts (2)
174-195: Session ID derivation correctly handles empty strings.The implementation properly trims and checks for non-empty strings before using the header value, generating
anon:${randomUUID()}for anonymous sessions. TheauthInfo: {}is correctly typed asPartial<AuthInfo>since it's progressively populated.
201-231: Appropriate error for missing session context.The
RequestContextNotAvailableErrorwith a specific message (lines 211-213) correctly indicates a bug in context propagation rather than a user error. The view merging logic (lines 223-231) correctly prioritizes request-scoped providers.libs/sdk/src/context/request-context.ts (7)
1-13: LGTM - Clean module setup with proper imports.Good use of
createHashfor the session ID hashing fix from prior review feedback. The imports are minimal and appropriate.
14-48: Well-structured interfaces with good documentation.The type definitions are clean with appropriate optionality. The
Partial<AuthInfo>correctly models the progressive population pattern.
94-111: Good defensive initialization with proper defaults.The defensive normalization of
customHeaders(lines 101-107) is a smart safeguard against runtime type issues. The initial timing mark setup is clean.
129-132: Correct merge semantics for progressive auth info updates.The spread-based merge preserves existing fields while allowing updates. This correctly supports the progressive population pattern documented in the interface.
194-198: Clean timing API with sensible fallbacks.The fallback to
timestampfor missingfrommarks andDate.now()for missingtomarks provides ergonomic defaults without throwing on missing marks.
225-227: Type assertion inget<T>is acceptable for this pattern.While the generic is unconstrained, this is a standard pattern for typed key-value stores where the caller knows the stored type. The
as T | undefinedcast correctly preserves the undefined case.
257-268: Session ID hashing correctly implemented.The SHA-256 hash with 12-character truncation provides sufficient entropy for log correlation (48 bits) while preventing accidental exposure of user-identifying session identifiers. Good implementation of prior review feedback.
libs/sdk/src/context/request-context-storage.ts (3)
1-34: Good module structure with properly scoped AsyncLocalStorage.The module-level storage instance with DI-only access pattern is correct. The
@Providerdecorator withProviderScope.GLOBALcorrectly scopes the storage manager.
56-98: Clean implementation of context propagation methods.The three run methods correctly handle both sync and async callbacks.
runFromHeadersproperly combines header-derived trace context with caller-provided args.
174-194: Good header parsing with proper handling of array values.The
extractClientIpcorrectly handles comma-separatedx-forwarded-forlists and array header values from various adapters. Note that this IP is for observability purposes—not for authorization decisions—so header spoofing isn't a security concern here.libs/sdk/src/provider/provider.registry.ts (9)
41-65: Well-designed session cache infrastructure with appropriate constants.The static configuration constants (MAX_SESSION_CACHE_SIZE, SESSION_CACHE_TTL_MS, SESSION_CLEANUP_INTERVAL_MS, SESSION_LOCK_TIMEOUT_MS) are sensible defaults. The generation counter for lock versioning is a good approach to prevent zombie lock releases.
727-762: Session cleanup methods correctly handle lock resolution.Both methods properly resolve lock promises before deletion, preventing hung waiters. The pattern of resolving before clearing is correct for graceful cleanup.
768-787: Good interval lifecycle management with process exit handling.The
unref()call ensures the cleanup interval doesn't prevent graceful process shutdown—important for serverless environments. The guard against double-start is correct.
812-821: Comprehensive dispose method addresses lifecycle concerns.The
dispose()method properly cleans up all resources: stops the cleanup interval, clears session stores, and resolves all pending locks. This addresses the lifecycle hook requirement from prior review.
885-889: Good overflow protection for lock generation counter.While reaching
MAX_SAFE_INTEGERis practically impossible at normal operation rates, this defensive check is good practice for long-running systems.
998-1058: Request-scoped provider building follows existing patterns.The method correctly builds providers with access to session and global views for dependency resolution. The static analysis hint about React hooks at line 1020 is a false positive—this is provider registry code, not React component code.
1063-1104: Correct dependency resolution order across provider views.The resolution priority (request → session → global → instances → build) correctly implements the scoped provider model. Parent hierarchy lookup for global providers is properly handled.
1114-1123: Clean synchronous API for scoped provider retrieval.The
getScoped<T>method provides a type-safe way to retrieve providers from views with the correct resolution order. The fallback toinstanceshandles global providers not explicitly in the views.
25-25: SessionKey integration enables session validation and injection.The import enables both early validation in
buildViewsand injection ofSessionKeyinto session stores for SESSION-scoped provider access.
…equest scopes with a single context scope
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
libs/adapters/src/openapi/openapi.tool.ts (1)
102-202: FrontMcpContext wiring is good, but option callback signatures become breakingRouting everything through
toolCtx.contextand passingctxintoInputTransformContext,resolveToolSecurity,headersMapper, andbodyMapperaligns well with the new FrontMcpContext model and gives mappers access to richer state (trace, session, metadata, etc.).However, this also changes the runtime shape of the first argument passed to user-supplied callbacks:
- Previously:
headersMapper(authInfo, headers)/bodyMapper(authInfo, body)and transformctxcarryingauthInfo.- Now:
headersMapper(ctx, headers)/bodyMapper(ctx, body)and transformctxcarrying aFrontMcpContext.Existing consumers (especially plain JS) that treat the first parameter as
authInfo(e.g.authInfo.token) will now receive a different object and may fail at runtime, even if their TypeScript types are updated.Consider one of:
- Keeping callbacks backward-compatible by passing both, e.g.
headersMapper(ctx, headers)but withctxexposingauthInfoand clearly documenting the change, and bumping a major version for the adapter; or- Introducing new callbacks (e.g.
contextHeadersMapper) and deprecating the oldauthInfo-based ones over time instead of hard-switching semantics.Also, in a future pass you may want to evaluate whether using
ctx.fetchhere (with explicit opt-out for security headers to avoid conflicts with OpenAPI mapping) would improve trace propagation, but that’s optional and can be deferred.libs/adapters/src/openapi/openapi.types.ts (1)
527-531: Documentation example uses inconsistent parameter name.The example at line 528 still shows
(authInfo)as the parameter name, but the updated signature at line 475 uses(ctx: FrontMcpContext). This inconsistency could confuse developers.* authProviderMapper: { - * 'GitHubAuth': (authInfo) => authInfo.user?.githubToken, + * 'GitHubAuth': (ctx) => ctx.authInfo.user?.githubToken, * },docs/draft/docs/adapters/openapi-adapter.mdx (2)
662-688: Multi-Tenant SaaS example uses outdated API signature.This example still uses
(authInfo, headers)and(authInfo, body)signatures, but the API has changed to(ctx, headers)and(ctx, body). This will cause TypeScript errors for users copying this example.- headersMapper: (authInfo, headers) => { + headersMapper: (ctx, headers) => { + const authInfo = ctx.authInfo; // Add tenant ID to all requests if (authInfo.user?.tenantId) { headers.set('x-tenant-id', authInfo.user.tenantId); } // Add user authorization if (authInfo.token) { headers.set('authorization', `Bearer ${authInfo.token}`); } return headers; }, - bodyMapper: (authInfo, body) => { + bodyMapper: (ctx, body) => { + const authInfo = ctx.authInfo; // Add user context to all mutations return { ...body, tenantId: authInfo.user?.tenantId, userId: authInfo.user?.id, timestamp: new Date().toISOString(), }; },
704-715: Expense Management example uses outdated API signature.Same issue as the Multi-Tenant example - uses
(authInfo, headers)instead of(ctx, headers).- headersMapper: (authInfo, headers) => { - const token = authInfo.token; + headersMapper: (ctx, headers) => { + const token = ctx.authInfo.token; if (token) { headers.set('authorization', `Bearer ${token}`); } return headers; },
♻️ Duplicate comments (3)
libs/sdk/src/flows/flow.instance.ts (2)
90-95:as anycast still present for middleware flow input.This was flagged in a previous review. Replace
{ request, response, next } as anywith a properly typed interface (e.g.,HttpMiddlewareInput) or useunknownwith runtime validation to maintain strict TypeScript guarantees.
221-225: Add a comment justifying the type cast safety.Per previous review feedback, add a brief comment explaining that
instance as Typeis safe becausebuildViews()guarantees properly constructed provider instances:// Merge context-scoped providers into deps for resolution by FlowClass const mergedDeps = new Map(deps); + // Type cast is safe: buildViews() returns instances constructed by ProviderRegistry, + // which ensures all context-scoped providers are valid provider instances. for (const [token, instance] of views.context) { mergedDeps.set(token, instance as Type); }libs/sdk/src/provider/provider.registry.ts (1)
719-738: LRU eviction can temporarily exceed max cache size when all sessions are locked.If every cached session has an active lock,
evictOldestSession()evicts nothing and the cache grows beyondMAX_SESSION_CACHE_SIZE. Consider a fallback such as queuing the new session creation until a lock clears, or logging a warning when this edge case occurs.
🧹 Nitpick comments (6)
libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
61-67: Clarify comment: throws error instead of falling back.The comment at line 65 says "Fallback" but the code immediately throws
RequestContextNotAvailableError. This is misleading—the code doesn't fall back; it throws a specific error when context is unavailable.Apply this diff to clarify the comment:
} catch { - // Fallback: context not available, likely called during initialization + // Context not available (likely called during initialization or outside request scope) throw new RequestContextNotAvailableError();libs/sdk/src/provider/provider.types.ts (1)
3-30: ProviderViews docs clearly express the new CONTEXT modelThe added documentation cleanly explains
globalvscontextand markssession/requestas deprecated aliases. The note that both aliascontextis useful; just ensure the implementation always passes the sameMapinstance forcontext,session, andrequestso the aliasing guarantee in the comments remains true.libs/sdk/src/flows/flow.instance.ts (1)
246-246: Consider typingcontextinstead ofany.
context: anybypasses TypeScript checks for all subsequent usages. Per coding guidelines favoring strict TypeScript, consider usingFlowCtxOf<Name>or a more specific type to catch type errors at compile time.libs/sdk/src/scope/flows/http.request.flow.ts (1)
169-191: Context auth update is duplicated inrouterstage.The
ctx.updateAuthInfo()call here (Lines 178-189) is nearly identical to the one inrouter(Lines 235-246). IfcheckAuthorizationalways runs beforerouter(per the plan), the second update may be redundant. Consider:
- Removing the duplicate in
routerif auth info doesn't change, or- Extracting a helper method to avoid code duplication.
private buildAuthInfo(authorization: { token: string; user: { sub?: string; exp?: number }; session?: { id?: string; payload?: unknown } }) { return { token: authorization.token, clientId: authorization.user?.sub, scopes: [], expiresAt: authorization.user?.exp ? authorization.user.exp * 1000 : undefined, extra: { user: authorization.user, sessionId: authorization.session?.id, sessionPayload: authorization.session?.payload, }, }; }libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (1)
43-46: Remove unusedgetPrivatehelper or add tests that use it.The
getPrivatehelper is defined but doesn't appear to be used in any of the test cases. Either remove it to keep the test file clean, or add tests that actually need to access private members.- // Helper to access private members for testing - const getPrivate = (registry: ProviderRegistry, prop: string) => { - return (registry as any)[prop]; - };libs/sdk/src/context/frontmcp-context.provider.ts (1)
47-58: Consider documenting the type assertion limitation more specifically.The
as anycast at Line 58 is justified, but the comment could be more specific about which limitation it addresses (e.g., "Type assertion needed because ProviderFactoryType doesn't support readonly tuple inference for inject()"). This helps future maintainers understand when/if the cast can be removed.-} as any; // Type assertion needed due to ProviderFactoryType limitations +} as any; // Type assertion needed: ProviderFactoryType cannot infer readonly tuple from inject()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
docs/draft/blog/11-2025/mcp-run-out-of-socket.mdx(1 hunks)docs/draft/blog/11-2025/openapi-mcp-security-nightmare.mdx(7 hunks)docs/draft/docs/adapters/openapi-adapter.mdx(10 hunks)docs/draft/docs/extensibility/providers.mdx(3 hunks)docs/draft/docs/extensibility/request-context.mdx(1 hunks)libs/adapters/src/openapi/__tests__/openapi-security-unit.spec.ts(25 hunks)libs/adapters/src/openapi/openapi.security.ts(7 hunks)libs/adapters/src/openapi/openapi.tool.ts(4 hunks)libs/adapters/src/openapi/openapi.types.ts(7 hunks)libs/sdk/src/common/interfaces/execution-context.interface.ts(4 hunks)libs/sdk/src/common/interfaces/flow.interface.ts(2 hunks)libs/sdk/src/common/metadata/provider.metadata.ts(1 hunks)libs/sdk/src/context/frontmcp-context-storage.ts(1 hunks)libs/sdk/src/context/frontmcp-context.provider.ts(1 hunks)libs/sdk/src/context/frontmcp-context.ts(1 hunks)libs/sdk/src/context/index.ts(1 hunks)libs/sdk/src/flows/flow.instance.ts(4 hunks)libs/sdk/src/index.ts(1 hunks)libs/sdk/src/provider/__tests__/provider-session-scope.test.ts(1 hunks)libs/sdk/src/provider/provider.registry.ts(5 hunks)libs/sdk/src/provider/provider.types.ts(1 hunks)libs/sdk/src/scope/flows/http.request.flow.ts(4 hunks)libs/sdk/src/tool/README.md(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/sdk/src/common/metadata/provider.metadata.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/common/interfaces/flow.interface.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/index.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/sdk/src/context/frontmcp-context.provider.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/sdk/src/context/frontmcp-context.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/context/index.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/adapters/src/openapi/openapi.types.tslibs/sdk/src/provider/provider.types.tslibs/adapters/src/openapi/openapi.security.tslibs/sdk/src/provider/provider.registry.tslibs/adapters/src/openapi/openapi.tool.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/index.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/sdk/src/context/frontmcp-context.provider.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/sdk/src/context/frontmcp-context.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/context/index.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/adapters/src/openapi/openapi.types.tslibs/sdk/src/provider/provider.types.tslibs/adapters/src/openapi/openapi.security.tslibs/sdk/src/provider/provider.registry.tslibs/adapters/src/openapi/openapi.tool.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/index.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/sdk/src/context/frontmcp-context.provider.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/sdk/src/context/frontmcp-context.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/context/index.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/tool/README.mdlibs/adapters/src/openapi/openapi.types.tslibs/sdk/src/provider/provider.types.tslibs/adapters/src/openapi/openapi.security.tslibs/sdk/src/provider/provider.registry.tslibs/adapters/src/openapi/openapi.tool.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Files:
libs/sdk/src/index.ts
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/extensibility/request-context.mdxdocs/draft/docs/adapters/openapi-adapter.mdxdocs/draft/docs/extensibility/providers.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/extensibility/request-context.mdxdocs/draft/blog/11-2025/mcp-run-out-of-socket.mdxdocs/draft/blog/11-2025/openapi-mcp-security-nightmare.mdxdocs/draft/docs/adapters/openapi-adapter.mdxdocs/draft/docs/extensibility/providers.mdx
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
🧠 Learnings (13)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/adapters/src/openapi/openapi.types.tslibs/adapters/src/openapi/openapi.security.tslibs/sdk/src/provider/provider.registry.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Enable strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use type parameters with constraints instead of unconstrained generics, and prefer `unknown` over `any` for generic type defaults
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/scope/flows/http.request.flow.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/flows/flow.instance.tslibs/sdk/src/index.tslibs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/sdk/src/context/frontmcp-context.tslibs/adapters/src/openapi/openapi.types.tslibs/adapters/src/openapi/openapi.security.tslibs/sdk/src/provider/provider.registry.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/sdk/src/index.tslibs/sdk/src/context/index.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
libs/sdk/src/index.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/sdk/src/context/index.tslibs/adapters/src/openapi/openapi.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/adapters/src/openapi/__tests__/openapi-security-unit.spec.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Create shared base classes for common functionality across context types (ExecutionContextBase provides get(), tryGet(), scope, fail(), mark(), fetch())
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/provider/provider.registry.ts
🧬 Code graph analysis (4)
libs/adapters/src/openapi/__tests__/openapi-security-unit.spec.ts (4)
libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
authInfo(88-90)libs/sdk/src/context/frontmcp-context.ts (3)
authInfo(230-232)FrontMcpContext(133-489)FrontMcpContext(495-495)libs/adapters/src/openapi/openapi.security.ts (1)
createSecurityContextFromAuth(34-144)libs/adapters/src/openapi/__tests__/fixtures.ts (1)
mockAuthInfo(229-238)
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (2)
libs/sdk/src/context/session-key.provider.ts (1)
SessionKey(27-64)libs/sdk/src/provider/provider.registry.ts (1)
ProviderRegistry(27-1119)
libs/sdk/src/provider/provider.registry.ts (5)
libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
scope(134-136)libs/sdk/src/scope/scope.instance.ts (1)
providers(298-300)libs/sdk/src/provider/provider.types.ts (1)
ProviderViews(13-31)libs/sdk/src/context/session-key.provider.ts (1)
SessionKey(27-64)libs/sdk/src/utils/index.ts (2)
isPromise(34-34)tokenName(34-34)
libs/adapters/src/openapi/openapi.tool.ts (2)
libs/adapters/src/openapi/openapi.types.ts (1)
InputTransformContext(13-20)libs/adapters/src/openapi/openapi.security.ts (1)
resolveToolSecurity(283-326)
🪛 Biome (2.1.2)
libs/sdk/src/provider/provider.registry.ts
[error] 1020-1020: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (41)
libs/sdk/src/common/interfaces/execution-context.interface.ts (3)
70-81: LGTM: Safe context access patterns.The
tryGetContext()method andcontextLoggergetter provide safe ways to access context with appropriate fallback behavior. The implementation correctly handles cases where context may not be available (e.g., during initialization).Also applies to: 109-121
167-183: LGTM: Context-aware fetch with graceful degradation.The enhanced
fetch()implementation correctly delegates tocontext.fetch()when available (providing auto-injection of Authorization, traceparent, x-request-id, and custom headers) and gracefully falls back to standard fetch otherwise. The documentation clearly explains the behavior.
20-20: Documentation forauthInfobreaking change is present.The type change from
AuthInfotoPartial<AuthInfo>forExecutionContextBaseArgs.authInfohas been documented indocs/draft/docs/extensibility/request-context.mdx, which explains that "authInfo is Partial because authentication information is progressively populated during the request lifecycle." Additional docs indocs/draft/docs/servers/tools.mdxdocument the deprecation ofthis.authInfoin favor ofthis.requestContext.authInfo. The documentation updates satisfy the coding guideline requirement for SDK breaking changes.docs/draft/docs/extensibility/request-context.mdx (1)
1-385: FrontMcpContext docs look coherent and aligned with the new context modelThe page does a good job covering access patterns, tracing, timing, storage, and auth/logging surfaces, and the API reference matches the exported types (requestId, traceContext, sessionId, authInfo, metadata, fetch, etc.). Just make sure the documented defaults (e.g.,
autoInjectAuthHeaders,autoInjectTracingHeaders,requestTimeout) stay in lockstep withFrontMcpContext’s implementation over time to avoid drift.docs/draft/blog/11-2025/mcp-run-out-of-socket.mdx (1)
176-179: Scope terminology updated correctly to GLOBAL/CONTEXTThe DI + decorators card now references
GLOBALandCONTEXTscopes, which matches the new provider scope model and broader docs. Looks good.libs/adapters/src/openapi/__tests__/openapi-security-unit.spec.ts (1)
8-493: Tests correctly adapt to FrontMcpContext-based auth resolutionUsing a lightweight
createMockContextthat populatesauthInfo(plus basic ids) is a clean way to exercisecreateSecurityContextFromAuthagainst the newFrontMcpContextsignature. This keeps tests focused on auth behavior without over-mocking the entire context surface. If future tests start relying on trace/metadata, you can extend the helper in one place.libs/sdk/src/index.ts (1)
14-33: Context barrel exports are well-structured and follow the index.ts guidelinesRe-exporting
FrontMcpContext, theContextalias, storage, provider token, request metadata, and trace helpers from./contextgives consumers a single, coherent entry point and matches the barrel-export guidance forlibs/*/src/index.ts. No behavioral changes, and the surface is appropriately grouped under a “unified context” comment.docs/draft/docs/extensibility/providers.mdx (1)
20-277: Provider docs now accurately reflect CONTEXT scope and FrontMcpContext usageThe updates clearly communicate the move to
GLOBAL/CONTEXTscopes, deprecateSESSION/REQUEST, and show practical patterns for accessingFrontMcpContext(via getter andFRONTMCP_CONTEXT) from tools and CONTEXT-scoped providers. The Redis/session examples and session metadata snippet align with the unified context design and should be very helpful for users.libs/sdk/src/tool/README.md (1)
134-261: Tool system README is consistent with the unified context provider modelThe documentation now correctly describes provider views as
context+globalwith resolution ordercontext → global, the defaultbindProviderscope as'context', and the use of a singlecontextmap per invocation (unified session+request). This matches the ProviderViews/interface changes and should help contributors reason about DI behavior.libs/sdk/src/flows/flow.instance.ts (1)
163-194: LGTM - runWithContext implementation is well-structured.The session ID extraction correctly handles empty strings (Lines 179-180), and the method properly wraps the flow execution in AsyncLocalStorage context. The fallback for backward compatibility when storage is unavailable is appropriate.
libs/sdk/src/scope/flows/http.request.flow.ts (2)
105-137: LGTM - traceRequest improvements are well-implemented.Good improvements:
- Using
randomUUID()instead ofMath.random()for request IDs (Line 113)- Proper fallback chain for sessionId with clear documentation (Lines 121-123)
- Trace context propagation for correlation (Line 127)
230-251: Session metadata update is appropriately placed here.The
ctx.updateSessionMetadata(authorization.session.payload)at Lines 247-249 is correctly placed in therouterstage since it depends on the full authorization flow. The overall context enrichment pattern is good.libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (3)
107-132: Good test for fresh-per-request behavior.This test correctly validates that CONTEXT-scoped providers are built fresh on each
buildViews()call, confirming no caching between requests. The counter-based approach is a clean way to verify instantiation count.
382-421: Excellent concurrent context isolation test.This test validates that parallel
buildViews()calls create isolated contexts without contention. Using unique build IDs to verify instance uniqueness is a robust approach.
1-14: Well-documented test file header.The header clearly documents what the tests cover and the relationship to the unified FrontMcpContext model. This helps maintainers understand the test scope.
libs/sdk/src/context/frontmcp-context.provider.ts (1)
31-36: LGTM - Token definition with backward-compatible alias.Using
Symbol.for()ensures the token is shareable across module boundaries. TheREQUEST_CONTEXTalias with@deprecatedJSDoc is a clean approach for backward compatibility.libs/adapters/src/openapi/openapi.security.ts (2)
34-42: LGTM - Clean migration to context-based security resolution.The function signature change from
authInfotoctx: FrontMcpContextis a good improvement, providing access to the full request context (sessionId, traceId, etc.) beyond just auth info. The priority chain (custom resolver → provider mapper → static auth → default) is well-documented.
314-322: Excellent error message with actionable solutions.The error message clearly identifies the problem and provides four specific solutions with code examples. This will significantly help developers debug authentication configuration issues.
libs/adapters/src/openapi/openapi.types.ts (4)
4-4: LGTM! Import addition aligns with the context refactoring.The
FrontMcpContextimport is correctly added to support the new unified context model across adapter callbacks.
13-20: Good: InputTransformContext now uses unified FrontMcpContext.The change from
authInfo: AuthInfotoctx: FrontMcpContextprovides a richer context including session ID, trace context, and more metadata beyond just authentication.
411-426: LGTM! Header and body mapper signatures updated consistently.Both
headersMapperandbodyMappernow acceptFrontMcpContextas the first parameter, and the JSDoc comments are updated to reflect the new structure includingauthInfo,sessionId,traceId, etc.
439-475: LGTM! Security resolver and auth provider mapper signatures updated.The example code correctly demonstrates accessing
ctx.authInfoto extract authentication data, maintaining backward compatibility in usage patterns while providing the richer context object.docs/draft/blog/11-2025/openapi-mcp-security-nightmare.mdx (2)
238-240: LGTM! Blog examples updated consistently with new context pattern.The
authProviderMapperexamples correctly demonstrate the new(ctx) => ctx.authInfo.user?.tokenpattern, matching the updated type signatures.
384-413: LGTM! Headers and body mapper examples demonstrate ctx usage well.The examples show proper extraction of
authInfofromctxand also demonstrate the newctx.traceContext.traceIdfor observability, which is a nice addition enabled by the unified context.libs/sdk/src/context/frontmcp-context.ts (4)
99-120: LGTM! Session key validation is well-designed.The validation enforces reasonable constraints: max 256 characters, alphanumeric with common delimiters (
-_.:) for flexibility, and clear error messages viaInvalidInputError.
423-434: Good privacy practice: Session ID is hashed in log output.The
toLogContext()method correctly hashes the session ID using SHA-256 before including it in logs, preventing accidental exposure of user-identifying session identifiers while still allowing correlation.
453-488: Well-designed context-aware fetch with proper header precedence.The fetch method correctly:
- Only sets headers if not already present (respecting caller overrides)
- Auto-injects auth and tracing headers based on config flags
- Injects custom headers from request metadata
- Applies timeout via AbortSignal
271-278: ThesupportsElicit: trueis correct and safe—TypeScript's type contract guaranteessendElicitRequestexists.The
TransportReftype definition requiressendElicitRequestas a non-optional property, and thesetTransport()method enforces strict type checking. All transport adapters implement this method, and TypeScript prevents creating aTransportRefwithout it. The suggested runtime check is unnecessary and contradicts the type system's guarantees.Likely an incorrect or invalid review comment.
docs/draft/docs/adapters/openapi-adapter.mdx (2)
97-103: LGTM! Parameter documentation updated correctly.The
headersMapperandbodyMapperparameter fields now correctly document theFrontMcpContexttype and mention the available properties (authInfo,sessionId,traceContext).
187-192: Good addition: Info block explaining ctx parameter structure.This clarifies what's available in the
ctxparameter for users implementing these callbacks.libs/sdk/src/context/index.ts (1)
1-68: LGTM! Well-organized barrel exports with clear deprecation notices.The module organization is excellent:
- Primary exports (FrontMcpContext, ContextStorage) are clearly separated
- Backward compatibility exports are marked with
@deprecatedJSDoc- The alias
Contextprovides a convenient short name- Follows the barrel exports pattern per coding guidelines
libs/sdk/src/context/frontmcp-context-storage.ts (3)
1-26: LGTM!The module documentation clearly explains the purpose and usage pattern. Imports are appropriate, and the module-level AsyncLocalStorage instance is correctly scoped for singleton access.
148-193: LGTM - Header extraction logic is well-implemented.The
extractMetadataandextractClientIpfunctions correctly:
- Filter custom headers by
x-frontmcp-prefix with case normalization- Handle both string and array header values for
x-forwarded-for- Parse comma-separated IP lists correctly (taking the first/client IP)
- Fall back through multiple IP header sources
141-145: Context propagation is correct.The method properly mutates the context in place via
updateAuthInfo(), which uses object spread to merge partial auth info fields:this._authInfo = { ...this._authInfo, ...authInfo }. Since the same FrontMcpContext object remains in AsyncLocalStorage, no re-entry is needed.libs/sdk/src/provider/provider.registry.ts (7)
215-222: LGTM - Background cleanup appropriately scoped.Starting cleanup only for scope-level registries (those with
parentProviders) is correct since SESSION providers are stored in scope registries. Thedispose()method at lines 831-840 properly addresses the lifecycle concern from the previous review.
239-260: LGTM - Scope normalization for backwards compatibility.The
normalizeScopehelper correctly maps deprecatedSESSIONandREQUESTscopes to the unifiedCONTEXTscope while preservingGLOBALscope unchanged.
831-840: LGTM - Proper resource cleanup indispose().The
dispose()method correctly:
- Stops the cleanup interval
- Clears session stores for GC
- Resolves pending lock promises to prevent hung awaits
This addresses the lifecycle concern from the previous review.
865-916: LGTM - Lock acquisition with timeout and generation-based release.The implementation correctly:
- Clears the timeout handle after
Promise.race(line 898) to prevent timer leaks- Uses generation numbers to prevent zombie lock releases
- Handles overflow with MAX_SAFE_INTEGER check
- Uses a re-check loop to handle TOCTOU race conditions
This addresses the previous review concern about timer leaks.
1009-1052: Biome static analysis hint is a false positive.The Biome warning about "hooks being called conditionally" at line 1020 is incorrect. This is backend TypeScript code in a DI/provider registry, not React code. The variable
outis a standard assignment, not a React hook call.
1101-1118: LGTM -getScopedprovides clean view-based provider lookup.The method correctly searches context → global → instances with appropriate error messaging when a provider isn't found.
954-993: No action needed.SessionKey.validate()already throwsInvalidInputErrorfor all validation failures (empty string, exceeds max length, invalid characters). The implementation at lines 48, 51, and 54-56 oflibs/sdk/src/context/session-key.provider.tscorrectly uses the MCP error class as required by the coding guidelines.
…d improve metadata handling
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
libs/sdk/src/provider/provider.registry.ts (1)
41-57: Session cache infrastructure may be unused with unified context model.The
sessionStores,sessionBuildLocks, and related constants are defined butbuildViews()creates a freshcontextStoreper call (line 825) with the comment "We don't cache context providers because they're per-request."These structures only appear used by:
cleanupSession()/cleanupExpiredSessions()- for explicit cleanupgetSessionCacheStats()- returns 0 size per testsdispose()- clears themConsider documenting that these are retained for backwards compatibility APIs or removing if truly unused.
libs/sdk/src/flows/flow.instance.ts (1)
91-96:as anycast still present for middleware flow input.Per previous review and coding guidelines, replace
as anywith a typed interface orunknownwith narrowing.Consider defining a middleware input type:
+interface HttpMiddlewareInput { + request: ServerRequest; + response: ServerResponse; + next: () => void; +} + const result = await this.runWithContext( contextStorage, request, - { request, response, next } as any, + { request, response, next } as HttpMiddlewareInput, new Map(), );
🧹 Nitpick comments (6)
docs/draft/docs/adapters/openapi-adapter.mdx (2)
97-103: Standardize optional chaining patterns in callback examples for clarity.The documentation shows inconsistent use of optional chaining when accessing
ctx.authInfo:
authProviderMapperexamples (lines 165-171):ctx.authInfo.user?.githubToken(no chaining onauthInfo)- SaaS
headersMapper/bodyMapperexamples (lines 664-670):ctx.authInfo?.user?.tenantId(chaining onauthInfo)Per the Info block (lines 187-191), authentication is "verified and auth fields are populated" by execution time. If
authInfois guaranteed to exist, examples should consistently omit the?.beforeauthInfofor clarity. If it's not guaranteed in some contexts, the documentation should explicitly state when the optional chaining is necessary.Apply this pattern consistently across all examples:
- headersMapper: (ctx, headers) => { - const authInfo = ctx.authInfo; - if (authInfo?.token) { + headersMapper: (ctx, headers) => { + const authInfo = ctx.authInfo; + if (authInfo.token) {Or, if defensive coding is preferred:
- authProviderMapper: { - GitHubAuth: (ctx) => ctx.authInfo.user?.githubToken, + authProviderMapper: { + GitHubAuth: (ctx) => ctx.authInfo?.user?.githubToken,Also applies to: 255-283
187-191: Add import guidance for FrontMcpContext type.The Info block introduces
FrontMcpContextas the parameter type passed to callbacks. While documentation doesn't require imports, adding a note about where developers should import this type would improve clarity. Consider adding a brief line such as: "ImportFrontMcpContextfrom@frontmcp/sdk" (or the actual export path).libs/sdk/src/flows/flow.registry.ts (1)
105-114: Consider adding type guard instead ofanycast.Per coding guidelines,
anytypes should be avoided. The(input as any)?.ctxpattern loses type safety. Consider defining a type guard or a more specific type for MCP handler inputs:- // Extract session info from MCP handler context (input.ctx.authInfo) - // MCP handlers pass { request, ctx } where ctx has authInfo - const mcpCtx = (input as any)?.ctx; - const authInfo = mcpCtx?.authInfo; + // Extract session info from MCP handler context (input.ctx.authInfo) + // MCP handlers pass { request, ctx } where ctx has authInfo + const mcpCtx = this.extractMcpContext(input); + const authInfo = mcpCtx?.authInfo;Then add a private helper with proper typing:
private extractMcpContext(input: unknown): { authInfo?: Partial<AuthInfo> } | undefined { if (typeof input === 'object' && input !== null && 'ctx' in input) { const ctx = (input as { ctx?: unknown }).ctx; if (typeof ctx === 'object' && ctx !== null) { return ctx as { authInfo?: Partial<AuthInfo> }; } } return undefined; }This preserves backward compatibility while improving type safety. Based on coding guidelines requiring no
anytypes without strong justification.libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (1)
25-27: Minor: Consider usingunknowninstead ofanyfor the decorator target.Per coding guidelines,
anyshould be avoided without justification. For test fixtures, this is acceptable, but consider:function Injectable() { - return function (target: any) {}; + return function (target: unknown) {}; }libs/sdk/src/flows/flow.instance.ts (1)
222-226: Type cast from views.context is safe.The
instance as Typecast is justified becausebuildViews()constructs all providers throughProviderRegistry, which guarantees valid instances. Consider adding a brief comment for clarity:// Merge context-scoped providers into deps for resolution by FlowClass const mergedDeps = new Map(deps); +// Cast is safe: buildViews() guarantees properly constructed provider instances for (const [token, instance] of views.context) { mergedDeps.set(token, instance as Type); }libs/sdk/src/provider/provider.registry.ts (1)
913-917: Error handling during context-scoped provider construction.The error re-throw preserves the original message which aids debugging. Consider using a specific error class for consistency with MCP error patterns:
} catch (e: any) { const msg = e?.message ?? e; console.error(`Failed constructing (context-scoped):`, msg); - throw new Error(`Failed constructing (context-scoped) ${tokenName(token)}: ${msg}`); + throw new ProviderResolutionError(`Failed constructing (context-scoped) ${tokenName(token)}: ${msg}`); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/demo/src/apps/expenses/providers/session-redis.provider.ts(1 hunks)docs/draft/docs/adapters/openapi-adapter.mdx(12 hunks)libs/adapters/src/openapi/openapi.security.ts(7 hunks)libs/adapters/src/openapi/openapi.types.ts(8 hunks)libs/sdk/src/common/interfaces/execution-context.interface.ts(4 hunks)libs/sdk/src/context/frontmcp-context-storage.ts(1 hunks)libs/sdk/src/context/frontmcp-context.provider.ts(1 hunks)libs/sdk/src/flows/flow.instance.ts(6 hunks)libs/sdk/src/flows/flow.registry.ts(2 hunks)libs/sdk/src/front-mcp/front-mcp.providers.ts(2 hunks)libs/sdk/src/provider/__tests__/provider-session-scope.test.ts(1 hunks)libs/sdk/src/provider/provider.registry.ts(5 hunks)libs/sdk/src/scope/flows/http.request.flow.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/adapters/src/openapi/openapi.types.ts
- libs/adapters/src/openapi/openapi.security.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable strict TypeScript mode with noanytypes without strong justification - useunknowninstead for generic type defaults
Avoid non-null assertions (!) - use proper error handling and throw specific errors instead
Use specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/flows/flow.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/front-mcp/front-mcp.providers.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/frontmcp-context.provider.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/sdk/src/flows/flow.instance.tsapps/demo/src/apps/expenses/providers/session-redis.provider.tslibs/sdk/src/provider/provider.registry.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/flows/flow.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/front-mcp/front-mcp.providers.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/frontmcp-context.provider.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/provider/provider.registry.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/flows/flow.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/front-mcp/front-mcp.providers.tslibs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/frontmcp-context.provider.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/provider/__tests__/provider-session-scope.test.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/provider/provider.registry.ts
docs/draft/docs/**
⚙️ CodeRabbit configuration file
docs/draft/docs/**: This folder holds the draft/source docs that humans are expected to edit. When authors want to add or change documentation, they should do it here. The Codex workflow uses these drafts, together with the code diff, to generate the latest docs under docs/docs/. As a reviewer: - Encourage contributors to add/update content here instead of docs/docs/. - It is fine to do structural/content feedback here (clarity, examples, etc).
Files:
docs/draft/docs/adapters/openapi-adapter.mdx
docs/**
⚙️ CodeRabbit configuration file
docs/**: Repository documentation for the SDK, using MDX and hosted by Mintlify. See more specific rules for: - docs/docs/** (latest rendered docs, automation-only) - docs/v/** (archived versions, read-only) - docs/draft/docs/** (human-editable drafts) - docs/blogs/** (blogs, human edited) - docs/docs.json (Mintlify navigation)
Files:
docs/draft/docs/adapters/openapi-adapter.mdx
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/apps/expenses/providers/session-redis.provider.ts
🧠 Learnings (12)
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
libs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/front-mcp/front-mcp.providers.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/provider/provider.registry.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Create shared base classes for common functionality across context types (ExecutionContextBase provides get(), tryGet(), scope, fail(), mark(), fetch())
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/provider/provider.registry.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
libs/sdk/src/common/interfaces/execution-context.interface.tslibs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/provider/provider.registry.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/sdk/src/context/frontmcp-context-storage.tslibs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Enable strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use type parameters with constraints instead of unconstrained generics, and prefer `unknown` over `any` for generic type defaults
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Applied to files:
libs/sdk/src/flows/flow.instance.ts
🧬 Code graph analysis (4)
libs/sdk/src/flows/flow.registry.ts (5)
libs/sdk/src/context/frontmcp-context.ts (1)
flow(291-293)libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowOutputOf(11-11)libs/sdk/src/common/interfaces/execution-context.interface.ts (2)
authInfo(88-90)scope(134-136)libs/testing/src/client/mcp-test-client.ts (1)
sessionId(206-208)libs/sdk/src/context/frontmcp-context-storage.ts (1)
FrontMcpContextStorage(202-202)
libs/sdk/src/scope/flows/http.request.flow.ts (2)
libs/testing/src/client/mcp-test-client.ts (3)
request(818-867)sessionId(206-208)session(210-226)libs/sdk/src/auth/session/record/session.base.ts (1)
user(157-159)
libs/sdk/src/front-mcp/front-mcp.providers.ts (1)
libs/sdk/src/context/frontmcp-context-storage.ts (1)
FrontMcpContextStorage(202-202)
libs/sdk/src/flows/flow.instance.ts (8)
libs/sdk/src/context/frontmcp-context-storage.ts (1)
FrontMcpContextStorage(202-202)libs/sdk/src/context/index.ts (2)
FrontMcpContextStorage(34-34)FRONTMCP_CONTEXT(38-38)libs/sdk/src/index.ts (2)
FrontMcpContextStorage(21-21)FRONTMCP_CONTEXT(23-23)libs/sdk/src/common/interfaces/server.interface.ts (1)
ServerRequest(6-14)libs/sdk/src/common/interfaces/flow.interface.ts (3)
FlowInputOf(10-10)FlowOutputOf(11-11)context(93-96)libs/sdk/src/context/frontmcp-context.ts (1)
scope(306-308)libs/sdk/src/errors/mcp.error.ts (1)
RequestContextNotAvailableError(365-371)libs/sdk/src/context/frontmcp-context.provider.ts (1)
FRONTMCP_CONTEXT(31-31)
🪛 Biome (2.1.2)
libs/sdk/src/provider/provider.registry.ts
[error] 880-880: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (24)
docs/draft/docs/adapters/openapi-adapter.mdx (1)
1-827: Approve comprehensive documentation migration to FrontMcpContext.The documentation update comprehensively and consistently reflects the migration from direct
authInfoparameters to the unifiedFrontMcpContext (ctx)pattern. All major examples (authProviderMapper, headersMapper, bodyMapper, securityResolver) are updated with correct signatures. The new Info block effectively explains the context structure and guarantee that authentication fields are pre-populated. The security risk classifications and best practices remain accurate under the new API.libs/sdk/src/scope/flows/http.request.flow.ts (3)
110-136: LGTM! Context-aware request tracing with proper fallbacks.The implementation correctly:
- Uses
tryGetContext()to safely access context without throwing- Falls back to
randomUUID()for request ID generation (better thanMath.random()for collision avoidance)- Derives sessionId from context first, then header, with a logging-only fallback
- Truncates traceId appropriately for log output
169-191: AuthInfo population correctly handles unit conversion.The
expiresAt: user.exp ? user.exp * 1000 : undefinedcorrectly converts JWTexp(seconds) to milliseconds, aligning withDate.now()comparisons elsewhere in the codebase. Theextraobject appropriately preserves user and session details for downstream access.
230-237: Session metadata propagation looks correct.The guard
authorization.session?.payloadensures safe access before callingupdateSessionMetadata. This complements the auth info set incheckAuthorization.libs/sdk/src/front-mcp/front-mcp.providers.ts (1)
27-28: LGTM! FrontMcpContextStorage correctly added to global providers.The
FrontMcpContextStorageis GLOBAL-scoped and needs to be registered at the global level to enable context propagation across all flows. The change aligns with the new context management architecture.libs/sdk/src/flows/flow.registry.ts (2)
118-131: LGTM! Context wrapping implementation is well-structured.The flow execution is correctly wrapped with
contextStorage.run(), and the context initialization includes all necessary fields (sessionId,scopeId,authInfo). ThePromise.resolve()wrapper ensures consistent Promise return type.
133-143: LGTM! Graceful fallback for missing context storage.The try-catch pattern in
getContextStorage()ensures backward compatibility whenFrontMcpContextStorageis not registered. This is appropriate for a gradual migration path.apps/demo/src/apps/expenses/providers/session-redis.provider.ts (1)
6-14: LGTM! Clean migration from SessionProvider to FrontMcpContext.The provider correctly:
- Updates scope from
SESSIONtoCONTEXTto match the new model- Uses
ctx.sessionIddirectly instead ofsessionProvider.session!.id(eliminates the non-null assertion)- Maintains the same key prefixing logic with proper sanitization
This demonstrates the recommended pattern for migrating existing session-scoped providers.
libs/sdk/src/context/frontmcp-context.provider.ts (1)
31-36: LGTM! Proper deprecation pattern for backward compatibility.Using
Symbol.for()ensures the token is globally unique and can be resolved across module boundaries. The deprecatedREQUEST_CONTEXTalias allows gradual migration of existing code.libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (1)
42-473: Comprehensive test coverage for the unified context model.The test suite thoroughly covers:
- SessionKey validation (empty, max length, invalid characters)
- Context injection and aliasing (session/request → context)
- Fresh per-request instantiation (no caching)
- Scope normalization (SESSION/REQUEST → CONTEXT)
- Context isolation and global provider sharing
- Error propagation and getScoped behavior
- Cross-scope dependencies and concurrent context builds
- Backwards compatibility for session cache helpers
- Dispose idempotency
libs/sdk/src/common/interfaces/execution-context.interface.ts (3)
61-68: Proper use of typed MCP error class.The
contextgetter correctly throwsRequestContextNotAvailableErrorinstead of a genericError, addressing the previous review comment. The catch-all pattern appropriately handles any provider resolution failure.
88-107: Well-structured progressive auth info resolution.The
getAuthInfo()method properly:
- Returns
Partial<AuthInfo>(addressing previous review concern)- Prefers context source when available
- Falls back gracefully for backward compatibility
The deprecation notice on the
authInfogetter is clear.
177-183: Context-aware fetch delegation looks correct.The fallback to standard
fetchwhen context is unavailable maintains backward compatibility while enabling context-aware header injection when available.libs/sdk/src/flows/flow.instance.ts (3)
176-181: Session ID handling now correctly handles empty strings.The fix properly addresses the previous review concern:
- Empty strings are now explicitly handled (
'' ?? 'fallback'issue fixed)- Trimming prevents whitespace-only session IDs
- Anonymous sessions get unique UUIDs to prevent data leakage
207-214: Proper error handling with typed MCP error.The
RequestContextNotAvailableErroris correctly used with a descriptive message explaining the invariant violation. The comment explains this should never happen in normal operation.
247-249: Type erasure explanation is helpful.The comment explaining why
FlowBase<any>is used (runtime type erasure) provides good context for maintainers.libs/sdk/src/context/frontmcp-context-storage.ts (4)
44-60: Clean AsyncLocalStorage wrapper with proper DI integration.The
FrontMcpContextStorageis correctly scoped as GLOBAL since it manages the storage mechanism itself, not the per-context data. Therun()method creates a fresh context for each invocation.
115-123: Properly uses typed MCP error class.The
getStoreOrThrow()method now throwsRequestContextNotAvailableErrorinstead of a genericError, addressing the previous review comment.
176-196: Robust client IP extraction with proper edge case handling.The
extractClientIp()function correctly handles:
- Comma-separated
x-forwarded-forlists (takes first IP)- Array header values from some adapters
- Fallback to
x-real-ip- Safe undefined return when no IP is found
144-148: Unable to verify mutation safety claims — file not found in repository.The file
libs/sdk/src/context/frontmcp-context-storage.tscould not be located in the current codebase. Verify that:
- The file path is correct
- If this is a new file in a PR, ensure it's staged for review
- Once available, confirm that
FrontMcpContext.updateAuthInfo()has thread-safe mutation semantics and that no concurrent code paths assume immutability ofauthInfoduring callback executionlibs/sdk/src/provider/provider.registry.ts (4)
208-216: Background cleanup timer lifecycle properly managed.The
startSessionCleanup()is called only for scope-level registries (withparentProviders), and the timer is properlyunref()ed to allow graceful process exit. This addresses the previous review concern about orphaned intervals.
240-248: Clean scope normalization implementation.The
normalizeScope()method correctly maps deprecatedSESSIONandREQUESTscopes toCONTEXT, enabling backwards compatibility while unifying the scope model.
775-784: Proper dispose implementation addressing lifecycle concerns.The
dispose()method correctly:
- Stops the cleanup interval
- Clears session stores
- Resolves pending locks to prevent hung promises
This addresses the previous review concern about orphaned intervals and memory leaks.
814-817: No action needed—SessionKey.validate() already uses InvalidInputError.SessionKey.validate() correctly throws InvalidInputError for all validation failures (empty keys, exceeded length, invalid characters), as shown in the JSDoc and implementation. The code already follows the proper error handling guidelines.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.