feat: Migrate transport configuration to a unified structure with Redis support #115
Conversation
WalkthroughIntroduces top-level Changes
sequenceDiagram
autonumber
participant Client
participant Server
participant Decorator as FrontMcp Decorator
participant Scope
participant Transport
participant Redis
Client->>Server: request/connect triggering init
Server->>Decorator: pass provided metadata
Decorator->>Decorator: applyMigration(metadata) → migrated metadata
Decorator->>Scope: create scope with migrated metadata
Scope->>Transport: init (reads transport.persistence & platformDetection)
Transport->>Redis: persistence read/write (if persistence.enabled)
Transport-->>Scope: return session id / session created
Scope-->>Server: session established
Server-->>Client: SSE/streamable response or established connection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 8
🧹 Nitpick comments (5)
libs/cli/src/commands/create.ts (2)
350-381: Env templates: consider aligning with top-levelredis/transport.persistencedocs
Right now.env.example/.env.dockerteach Redis connectivity, but they don’t hint at how these map into the new unified config shape (top-levelredis+transport.persistence). A short comment block would reduce confusion.
537-555: Scaffolding always writes Docker + Redis files; ensure that’s intended for all targets
This now unconditionally addsdocker-compose.yml,Dockerfile,.env.*, andREADME.md(Line 537-542). Iffrontmcp createsupports non-Docker workflows, consider gating this behind a prompt/flag (or only scaffold when Redis features are enabled).docs/draft/docs/deployment/redis-setup.mdx (2)
48-66: Doc should include the new top-levelredis+transport.persistenceconfiguration shape (not only CachePlugin init)
This page teaches Redis viaCachePlugin.init(...), but the PR’s main user-facing change is the unified config (redisandtransport). Add a short example showing@FrontMcp({ redis: {...}, transport: { persistence: { enabled: true }}})so readers don’t assume CachePlugin is the only integration path. Based on learnings, types/config should align with protocol-level surfaces.Also applies to: 110-145
50-62: UseparseInt(x, 10)in docs snippets
Avoid radix ambiguity in examples.- port: parseInt(process.env.REDIS_PORT || '6379'), + port: parseInt(process.env.REDIS_PORT || '6379', 10),Also applies to: 123-128, 151-167
libs/sdk/src/common/types/options/__tests__/transport.options.test.ts (1)
11-190: Good defaults/shape coverage; consider adding a test that documents intended runtime behavior for persistence+redis
Right now you testenabled: truewith inlineredis(Line 30-36) but not the “enabled=true without redis uses top-level redis” expectation. A small test that calls the higher-level config parser (where top-levelredisexists) would lock in the contract. Based on learnings, tests should cover error paths/validation contracts.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
docs/draft/docs.json(1 hunks)docs/draft/docs/deployment/redis-setup.mdx(1 hunks)docs/draft/docs/servers/server.mdx(3 hunks)libs/cli/src/commands/create.ts(2 hunks)libs/sdk/src/auth/session/record/session.base.ts(3 hunks)libs/sdk/src/auth/session/session.service.ts(3 hunks)libs/sdk/src/common/decorators/front-mcp.decorator.ts(1 hunks)libs/sdk/src/common/metadata/front-mcp.metadata.ts(3 hunks)libs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.ts(1 hunks)libs/sdk/src/common/migrate/auth-transport.migrate.ts(1 hunks)libs/sdk/src/common/migrate/index.ts(1 hunks)libs/sdk/src/common/tokens/front-mcp.tokens.ts(1 hunks)libs/sdk/src/common/types/options/__tests__/redis.options.test.ts(1 hunks)libs/sdk/src/common/types/options/__tests__/transport.options.test.ts(1 hunks)libs/sdk/src/common/types/options/auth.options.ts(7 hunks)libs/sdk/src/common/types/options/index.ts(1 hunks)libs/sdk/src/common/types/options/redis.options.ts(1 hunks)libs/sdk/src/common/types/options/transport.options.ts(1 hunks)libs/sdk/src/notification/notification.service.ts(1 hunks)libs/sdk/src/scope/scope.instance.ts(1 hunks)libs/sdk/src/transport/flows/handle.sse.flow.ts(1 hunks)libs/sdk/src/transport/flows/handle.streamable-http.flow.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/common/migrate/index.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/common/types/options/redis.options.tslibs/sdk/src/common/types/options/index.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/auth/session/record/session.base.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/auth/session/session.service.tslibs/sdk/src/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/migrate/auth-transport.migrate.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/notification/notification.service.tslibs/sdk/src/common/types/options/__tests__/redis.options.test.tslibs/cli/src/commands/create.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/common/types/options/auth.options.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/common/migrate/index.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/common/types/options/redis.options.tslibs/sdk/src/common/types/options/index.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/auth/session/record/session.base.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/auth/session/session.service.tslibs/sdk/src/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/migrate/auth-transport.migrate.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/notification/notification.service.tslibs/sdk/src/common/types/options/__tests__/redis.options.test.tslibs/cli/src/commands/create.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/common/types/options/auth.options.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/common/migrate/index.tslibs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/common/types/options/redis.options.tslibs/sdk/src/common/types/options/index.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/auth/session/record/session.base.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/auth/session/session.service.tslibs/sdk/src/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/migrate/auth-transport.migrate.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/notification/notification.service.tslibs/sdk/src/common/types/options/__tests__/redis.options.test.tslibs/cli/src/commands/create.tslibs/sdk/src/common/decorators/front-mcp.decorator.tslibs/sdk/src/common/types/options/auth.options.ts
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.jsondocs/draft/docs/deployment/redis-setup.mdxdocs/draft/docs/servers/server.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/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.tslibs/sdk/src/common/types/options/__tests__/redis.options.test.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/deployment/redis-setup.mdxdocs/draft/docs/servers/server.mdx
🧠 Learnings (11)
📓 Common learnings
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
📚 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/common/migrate/index.tslibs/sdk/src/common/types/options/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 : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/transport/flows/handle.sse.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: 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/transport/flows/handle.streamable-http.flow.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/auth/session/session.service.tslibs/sdk/src/notification/notification.service.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 `getCapabilities()` for dynamic capability exposure instead of hardcoding capabilities in adapters
Applied to files:
libs/sdk/src/common/types/options/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/*/src/common/records/**/*.ts : Centralize record types in common/records and import from there instead of from module-specific files
Applied to files:
libs/sdk/src/common/types/options/index.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/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/types/options/__tests__/redis.options.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/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.tslibs/sdk/src/common/types/options/__tests__/redis.options.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 **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
libs/sdk/src/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/types/options/__tests__/redis.options.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: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/types/options/__tests__/transport.options.test.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/types/options/__tests__/redis.options.test.tslibs/sdk/src/common/decorators/front-mcp.decorator.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 URIs per RFC 3986 at metadata level using Zod validation with custom refinements
Applied to files:
libs/sdk/src/common/types/options/__tests__/redis.options.test.ts
🧬 Code graph analysis (9)
libs/sdk/src/scope/scope.instance.ts (1)
libs/sdk/src/transport/transport.registry.ts (1)
TransportService(22-617)
libs/sdk/src/auth/session/record/session.base.ts (1)
libs/sdk/src/common/types/options/transport.options.ts (1)
TransportIdMode(15-15)
libs/sdk/src/auth/session/session.service.ts (3)
libs/sdk/src/store/adapters/store.memory.adapter.ts (1)
ScopedInMemoryStore(10-90)libs/sdk/src/auth/session/utils/session-id.utils.ts (1)
encryptJson(34-43)libs/sdk/src/auth/session/record/session.base.ts (1)
scope(106-108)
libs/sdk/src/common/types/options/__tests__/transport.options.test.ts (1)
libs/sdk/src/common/types/options/transport.options.ts (3)
transportPersistenceConfigSchema(25-56)transportOptionsSchema(66-157)TransportOptionsInput(171-171)
libs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.ts (1)
libs/sdk/src/common/migrate/auth-transport.migrate.ts (4)
resetDeprecationWarning(84-86)needsMigration(91-95)migrateAuthTransportConfig(102-174)applyMigration(182-208)
libs/sdk/src/common/types/options/transport.options.ts (2)
libs/sdk/src/common/types/options/redis.options.ts (1)
redisOptionsSchema(9-49)libs/sdk/src/common/types/options/session.options.ts (1)
platformDetectionConfigSchema(108-111)
libs/sdk/src/common/metadata/front-mcp.metadata.ts (3)
libs/sdk/src/common/types/options/redis.options.ts (2)
RedisOptionsInput(59-59)redisOptionsSchema(9-49)libs/sdk/src/common/types/options/transport.options.ts (2)
TransportOptionsInput(171-171)transportOptionsSchema(66-157)libs/sdk/src/common/types/options/session.options.ts (2)
SessionOptions(35-94)sessionOptionsSchema(113-123)
libs/sdk/src/common/types/options/__tests__/redis.options.test.ts (1)
libs/sdk/src/common/types/options/redis.options.ts (3)
redisOptionsSchema(9-49)RedisOptionsInput(59-59)RedisOptions(54-54)
libs/cli/src/commands/create.ts (1)
libs/cli/src/colors.ts (1)
c(13-13)
⏰ 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.json (1)
165-170: Navigation update looks consistent with the new draft pagelibs/sdk/src/common/tokens/front-mcp.tokens.ts (1)
10-13: Token additions align with new top-level metadata; keepingsessionfor compatibility is sensiblelibs/sdk/src/common/types/options/__tests__/redis.options.test.ts (1)
5-227: Strong coverage, but currently inconsistent with the schema (whitespace host + max port expectations)
Tests like “reject whitespace-only host” (Line 162-165) and “reject port 65536” (Line 130-133) require corresponding schema constraints. OnceredisOptionsSchemais updated, this suite should be a solid guardrail. Based on learnings, tests should cover error paths; this does.libs/sdk/src/common/types/options/transport.options.ts (1)
66-157: Unified transport schema looks coherent; exports align with TypeScript-first config philosophy
The consolidation of session lifecycle + protocol flags + persistence intotransportOptionsSchemawithTransportOptionsInput/TransportOptionsis a clean public surface. Based on learnings, keeping schema/types as the source of truth is the right direction.Also applies to: 163-186
libs/sdk/src/common/migrate/index.ts (1)
1-3: LGTM! Clean barrel export for migration utilities.The export pattern aligns with the project's barrel export standards and appropriately exposes migration helpers for the transport configuration restructuring.
libs/sdk/src/auth/session/session.service.ts (1)
33-33: Configuration path migration aligns with transport restructuring.The migration from
scope.metadata.session?.sessionModetoscope.metadata.transport?.sessionModecorrectly reflects the new top-level transport configuration structure introduced in this PR.libs/sdk/src/transport/flows/handle.streamable-http.flow.ts (1)
117-120: Correct migration to transport-based platform detection.The change from
this.scope.metadata.session?.platformDetectiontothis.scope.metadata.transport?.platformDetectionproperly aligns session creation with the new transport configuration structure.libs/sdk/src/transport/flows/handle.sse.flow.ts (1)
115-118: Consistent transport configuration migration for SSE flow.The platformDetection path update mirrors the change in handle.streamable-http.flow.ts, ensuring consistent transport configuration access across all transport types.
libs/sdk/src/notification/notification.service.ts (1)
756-773: Platform detection correctly sourced from transport configuration.The migration of platformDetection from
this.scope.metadata?.session?.platformDetectiontothis.scope.metadata.transport?.platformDetectionin thesetClientInfomethod aligns with the unified transport configuration structure.libs/sdk/src/common/types/options/index.ts (1)
6-7: Public API surface extended for transport and Redis options.The new exports appropriately expose the Redis and Transport option schemas, making them available to SDK consumers as part of the unified transport configuration surface.
libs/sdk/src/auth/session/record/session.base.ts (2)
82-83: Minor formatting improvement for readability.Splitting the exp extraction logic across two lines improves readability without changing functionality.
115-126: Transport ID mode migration with justified type casting.The migration from
scope.metadata.session?.transportIdModetoscope.metadata.transport?.transportIdModecorrectly aligns with the unified transport configuration. The explicit type cast at line 122-124 is necessary with Zod 4.0.0, as the z.function() factory API no longer provides adequate type inference when used in unions. The cast properly narrows the function type to(issuer: string) => Promise<TransportIdMode> | TransportIdMode, enabling correct type checking for the async/sync function call at line 124.libs/sdk/src/common/decorators/front-mcp.decorator.ts (1)
12-14: LGTM! Migration preprocessing is correctly positioned.The migration logic is applied before schema validation, ensuring deprecated configurations are automatically transformed to the new structure before validation occurs. This provides a smooth backward-compatibility path.
libs/sdk/src/common/metadata/front-mcp.metadata.ts (2)
36-52: LGTM! Well-designed public API with clear deprecation.The new
redisandtransportfields are properly documented, and the deprecatedsessionfield maintains backward compatibility. The JSDoc comments clearly explain the purpose of each field.
80-82: LGTM! Schema design ensures consistent defaults.The transform on
transportensures it's always defined with schema defaults even when omitted by the user. This provides a consistent API surface where consumers can rely ontransportbeing present.libs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.ts (1)
1-332: LGTM! Comprehensive test coverage for migration logic.The tests cover:
- Detection of deprecated configurations
- Migration transformations for all legacy paths
- In-place application of migrations
- Deprecation warning behavior (once per process, correct messaging)
The test organization is clear and the coverage appears thorough.
libs/sdk/src/common/types/options/auth.options.ts (2)
261-300: LGTM! Clear deprecation with removal target.The transport schemas are properly marked as deprecated with clear guidance to use top-level transport config. The v1.0.0 removal target gives users a clear timeline for migration.
634-649: LGTM! Type exports properly deprecated.The JSDoc deprecation annotations clearly direct users to the new TransportOptions types from transport.options.ts, maintaining type safety during the migration period.
libs/sdk/src/common/migrate/auth-transport.migrate.ts (3)
1-13: LGTM! Clear documentation of migration scope.The file header clearly documents what migrations are performed and marks this as temporary code to be removed in v1.0.0. This helps future maintainers understand the deprecation timeline.
102-174: LGTM! Migration logic comprehensively handles legacy configurations.The function correctly:
- Preserves existing transport config before merging
- Merges session fields into transport
- Merges auth.transport flags into transport
- Translates recreation → persistence with proper field mapping
- Extracts Redis config to top-level only when not already present (prevents overwriting)
- Strips transport from auth to create a clean config
The nullish coalescing pattern ensures new values don't overwrite existing ones.
182-208: LGTM! In-place mutation is appropriate for decorator use case.The mutation is intentional and well-suited for the decorator preprocessing workflow. The function name
applyMigrationclearly signals the in-place behavior, and it correctly:
- Merges migrated transport without losing existing values
- Only sets redis if migration produced one
- Replaces auth with the cleaned version
docs/draft/docs/servers/server.mdx (3)
71-102: LGTM! Clear documentation of new configuration structure.The documentation clearly presents the new
transportandredistop-level configuration with comments indicating this is new in v0.6. The structure is easy to understand and includes sensible defaults.
191-233: LGTM! Excellent protocol switches documentation.The table format clearly explains when to enable each transport type with practical use cases and notes. The tip about
enableStatelessHttpin production is particularly valuable for security considerations.
340-407: LGTM! Comprehensive migration guide.The migration guide is excellent:
- Clear deprecation warning
- Side-by-side before/after code examples
- Detailed mapping table showing old path → new path
- Explains the automatic migration behavior
This will help users migrate smoothly.
# Conflicts: # docs/draft/docs.json
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
libs/sdk/src/common/migrate/auth-transport.migrate.ts (1)
73-79: Documentation URL updated correctly.The deprecation warning now points to the correct documentation host at
docs.agentfront.dev. This addresses the previous review comment about the broken link.libs/sdk/src/common/types/options/transport.options.ts (1)
25-45: LGTM!The
transportPersistenceConfigSchemais cleanly defined. The previous review comment about the no-op.refine()block has been addressed - it's no longer present. The schema correctly definesenabled, optionalredis, anddefaultTtlMswith sensible defaults.docs/draft/docs/servers/server.mdx (1)
193-197: platformDetection default corrected.The table now correctly shows
platformDetectiondefaults toundefined, addressing the previous review comment about the incorrect{}default.
🧹 Nitpick comments (2)
libs/sdk/src/transport/transport.registry.ts (1)
39-49: Minor comment inconsistency with terminology.The comment at line 41 still references "transport recreation" while the code has been updated to use "persistence" terminology. Consider updating for consistency.
/** - * Redis session store for transport recreation + * Redis session store for transport persistence * Used to persist session metadata across server restarts */ private sessionStore?: RedisSessionStore; /** * Transport persistence configuration */ private persistenceConfig?: TransportPersistenceConfigInput;libs/sdk/src/common/migrate/auth-transport.migrate.ts (1)
126-135: Consider guarded destructuring instead of non-null assertion.Per coding guidelines, non-null assertions should be avoided. While the
hasOldSessioncheck makes this safe, using a guarded pattern would be more defensive:// Migrate session config if (hasOldSession) { - const session = metadata.session!; + const session = metadata.session; + if (!session) return result; // TypeScript guard result.transport = { ...result.transport, sessionMode: session.sessionMode, transportIdMode: session.transportIdMode, platformDetection: session.platformDetection, }; }However, since the check is immediately preceding, this is acceptable if you prefer the current form.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
docs/draft/docs/deployment/redis-setup.mdx(1 hunks)docs/draft/docs/servers/server.mdx(3 hunks)libs/sdk/src/auth/session/session.service.ts(3 hunks)libs/sdk/src/common/migrate/__tests__/auth-transport.migrate.test.ts(1 hunks)libs/sdk/src/common/migrate/auth-transport.migrate.ts(1 hunks)libs/sdk/src/common/types/options/transport.options.ts(1 hunks)libs/sdk/src/transport/transport.registry.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/sdk/src/common/migrate/tests/auth-transport.migrate.test.ts
- libs/sdk/src/auth/session/session.service.ts
- docs/draft/docs/deployment/redis-setup.mdx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/transport/transport.registry.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/migrate/auth-transport.migrate.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/transport/transport.registry.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/migrate/auth-transport.migrate.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/transport/transport.registry.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/migrate/auth-transport.migrate.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/servers/server.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/servers/server.mdx
🧠 Learnings (3)
📚 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/common/records/**/*.ts : Centralize record types in common/records and import from there instead of from module-specific files
Applied to files:
libs/sdk/src/common/types/options/transport.options.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/common/types/options/transport.options.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 URIs per RFC 3986 at metadata level using Zod validation with custom refinements
Applied to files:
libs/sdk/src/common/types/options/transport.options.ts
🧬 Code graph analysis (3)
libs/sdk/src/transport/transport.registry.ts (3)
libs/sdk/src/common/types/options/transport.options.ts (1)
TransportPersistenceConfigInput(170-170)libs/sdk/src/transport/transport.types.ts (1)
Transporter(51-63)libs/sdk/src/auth/session/redis-session.store.ts (1)
RedisSessionStore(13-218)
libs/sdk/src/common/types/options/transport.options.ts (2)
libs/sdk/src/common/types/options/redis.options.ts (1)
redisOptionsSchema(9-49)libs/sdk/src/common/types/options/session.options.ts (1)
platformDetectionConfigSchema(108-111)
libs/sdk/src/common/migrate/auth-transport.migrate.ts (3)
libs/sdk/src/common/types/options/redis.options.ts (1)
RedisOptionsInput(59-59)libs/sdk/src/common/types/options/session.options.ts (1)
SessionOptions(35-94)libs/sdk/src/common/types/options/transport.options.ts (1)
TransportOptionsInput(160-160)
⏰ 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 (13)
libs/sdk/src/transport/transport.registry.ts (3)
66-83: LGTM!The Redis session store initialization correctly requires both
persistenceConfig?.enabledandpersistenceConfig.redisto be truthy. This defensive check gracefully handles the case where persistence is enabled but Redis config is missing (falls back to in-memory only). The comment explaining the different key prefix from the auth module is helpful.
239-270: LGTM!The
doRecreateTransportermethod correctly capturespersistenceConfigfor closure use and passes the TTL to Redis operations. Error handling via.catch()with logging is appropriate for non-critical persistence failures.
324-363: LGTM!The
doCreateTransportermethod follows the same pattern as recreation, correctly usingpersistenceConfigfor TTL and handling Redis persistence errors gracefully with warnings.libs/sdk/src/common/migrate/auth-transport.migrate.ts (3)
91-95: LGTM!The
needsMigrationfunction correctly detects the presence of deprecated config paths.
182-208: LGTM!The
applyMigrationfunction correctly mutates metadata in place for decorator use. The type assertions are necessary here due to the generic constraint. The merge logic properly combines existing and migrated transport config.
54-63: LGTM!The module-level deprecation warning flag with one-time display is a good pattern for avoiding console spam during runtime migration.
libs/sdk/src/common/types/options/transport.options.ts (3)
98-146: LGTM!Protocol switches are well-defined with clear defaults and JSDoc documentation. The
persistencefield correctly uses the nested schema as optional.
152-175: LGTM!Type exports follow the standard pattern of providing both inferred types (
z.infer) and input types (z.input) for consumer flexibility.
69-86: The suggested refactor syntax is incorrect for Zod 4 — function schemas require explicit tuple input/output specs.The original diff uses
z.function({ input: [...], output: ... }), which is not valid Zod 4 syntax. In Zod 4, function schemas are created asz.function(inputTuple, outputSchema)where the input must be az.tuple().The correct pattern would be:
z.function(z.tuple([z.string()]), z.union([z.literal('stateful'), z.literal('stateless'), z.promise(z.union([...]))]))However, this approach has a deeper issue:
z.union([z.literal(...), z.function(...)])may not work as intended in Zod 4, sincez.function()returns a function factory rather than a validatable schema. Verify whether the current code (with barez.function()) actually validates correctly in Zod 4, and whether the union approach is the intended pattern for this use case. If stricter typing is needed, consider alternative patterns like discriminated unions or separate validator functions rather than inline function schemas in unions.docs/draft/docs/servers/server.mdx (4)
158-249: LGTM!The Transport section is well-documented with clear explanations of session lifecycle options, protocol switches, and helpful code examples. The info callout about migration is appropriately placed.
250-277: LGTM!The Session Persistence documentation clearly explains Redis-backed persistence and the shared configuration pattern. The info callout at line 274-276 helpfully explains the relationship between
redisandtransport.persistence.
340-407: LGTM!The migration section provides excellent before/after examples and a comprehensive mapping table. This will help users transition from the deprecated config structure smoothly.
510-530: LGTM!Best practices section is updated appropriately with guidance for the new configuration structure and warnings about deprecated patterns.
…or transport persistence
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/sdk/src/transport/transport.registry.ts (2)
325-357: Same TTL handling pattern - verify signature and consider inlining.This follows the same pattern as
doRecreateTransporter. ThepersistenceConfigconstant is assigned on line 325 but used only once on line 357.For consistency with the previous suggestion, consider inlining:
- const sessionStore = this.sessionStore; - const persistenceConfig = this.persistenceConfig; + const sessionStore = this.sessionStore; const transporter = new LocalTransporter(this.scope, key, res, () => { key.sessionId = sessionId; @@ -354,7 +353,7 @@ createdAt: Date.now(), lastAccessedAt: Date.now(), }; - sessionStore.set(sessionId, storedSession, persistenceConfig?.defaultTtlMs).catch((err) => { + sessionStore.set(sessionId, storedSession, this.persistenceConfig?.defaultTtlMs).catch((err) => {
240-264: Consider inliningpersistenceConfigconstant for clarity.The
persistenceConfigconstant (line 240) is used only once at line 264. TheRedisSessionStore.set()method correctly acceptsttlMs?: number, and passingpersistenceConfig?.defaultTtlMs(which evaluates tonumber | undefined) is safe. The optional chaining pattern and error handling are appropriate.For improved code clarity, inline the constant:
const sessionStore = this.sessionStore; - const persistenceConfig = this.persistenceConfig; // Create new transport const transporter = new LocalTransporter(this.scope, key, res, () => { // ... }); await transporter.ready(); this.insertLocal(key, transporter); // Update session access time in Redis if (sessionStore) { const updatedSession: StoredSession = { ...storedSession, lastAccessedAt: Date.now(), }; - sessionStore.set(sessionId, updatedSession, persistenceConfig?.defaultTtlMs).catch((err) => { + sessionStore.set(sessionId, updatedSession, this.persistenceConfig?.defaultTtlMs).catch((err) => {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/sdk/src/transport/transport.registry.ts(7 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/transport/transport.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/transport/transport.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/transport/transport.registry.ts
⏰ 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 (2)
libs/sdk/src/transport/transport.registry.ts (2)
57-59: This is not a breaking API change.TransportServiceis not exported in the public API (libs/sdk/src/index.ts). Users configure transport persistence through thetransport.persistencemetadata property, not by instantiating the class directly. The constructor parameter change fromrecreationConfigtopersistenceConfigis an internal implementation detail. Migration utilities inlibs/sdk/src/common/migrate/auth-transport.migrate.tshandle backward compatibility at the configuration level, automatically migrating oldauth.transport.recreationstructures to the newtransport.persistenceformat. Documentation indocs/draft/docs/servers/server.mdxalready reflects the new configuration structure and terminology.
57-78: No changes needed. The Redis property access is type-safe given the guard check at line 67 and theRedisConfiginterface which explicitly allows optional properties to be undefined.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.