feat: Add auto-transport persistence transformation for Redis configu… #155
Conversation
WalkthroughAdds a runtime transform that auto-populates transport.persistence from top-level Redis/Vercel KV config during metadata schema parsing and adds comprehensive tests that validate behavior across multiple persistence and Redis configurations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/types/options/transport.options.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use 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 type parameters with constraints instead of unconstrained generics
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/src/**/*.ts: MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead ofunknown
Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()method for dynamic capability exposure instead of hardcoding capabilities
UsechangeScopeproperty instead ofscopein change events to avoid confusion with Scope class
Validate URIs per RFC 3986 at metadata level using proper validation
Validate hook flows and fail fast on invalid hook configurations with specific error messages
Files:
libs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.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/metadata/front-mcp.metadata.tslibs/sdk/src/common/types/options/transport.options.tslibs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including error cases and constructor validation
Include error classinstanceofchecks in tests to validate error handling
Files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
🧠 Learnings (4)
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
📚 Learning: 2025-12-22T13:01:03.646Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-22T13:01:03.646Z
Learning: Applies to libs/ui/src/universal/**/*.{ts,tsx} : Export FrontMCPProvider and UniversalApp from universal app shell for platform-agnostic React configuration
Applied to files:
libs/sdk/src/common/metadata/front-mcp.metadata.ts
📚 Learning: 2025-12-22T13:01:16.929Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-22T13:01:16.929Z
Learning: Applies to libs/uipack/src/{adapters,components,web-components}/**/*.test.ts : Test component behavior across platform configurations (OpenAI, Claude, etc.)
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to **/*.test.{ts,tsx} : Test all code paths including error cases and constructor validation
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
🔇 Additional comments (3)
libs/sdk/src/common/types/options/transport.options.ts (1)
23-47: Documentation updates accurately reflect auto-enable behavior.The updated documentation clearly describes:
- Auto-enabling when top-level
redisis configured- Explicit
enabled: falseandredis: {...}overrides- Redis/Vercel KV support
The documentation aligns well with the implementation in
front-mcp.metadata.ts.libs/sdk/src/common/metadata/front-mcp.metadata.ts (1)
142-177: Transform logic is correct and handles all cases properly.The four cases are well-structured:
- Respect explicit
enabled: false- Respect explicit
redisconfig- Auto-populate
rediswhenenabled: truewithoutredis- Auto-enable and populate when
persistenceis undefinedThe fall-through at line 176 correctly handles edge cases like
persistence: { defaultTtlMs: 5000 }whereenableddefaults tofalsethrough schema parsing.libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts (1)
3-56: Well-structured test isolation approach.The comment on lines 3-5 clearly explains why the transform is re-implemented locally rather than imported. This is a pragmatic solution to avoid circular dependencies during testing.
The re-implemented
applyAutoTransportPersistencefunction (lines 13-56) mirrors the production code, which allows for focused unit testing of the transform logic.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/sdk/src/common/metadata/front-mcp.metadata.ts (1)
202-204: Schema transform integration is correct.The transform is properly applied after the union schema parsing, ensuring the auto-persistence logic runs on validated data.
The documentation gap flagged in a prior review (explaining auto-enable behavior in
docs/draft/docs/servers/server.mdx) remains relevant.
🧹 Nitpick comments (4)
libs/sdk/src/common/metadata/front-mcp.metadata.ts (1)
122-125: Type guard return type doesn't reflectnullhandling.The function returns
truefornull(line 125), but the return type only includes| undefined. This means whenvalueisnull, TypeScript will incorrectly narrow it to the persistence object type.Since
nullis explicitly handled, consider including it in the return type for type accuracy:🔎 Suggested fix
function isPersistenceObject( value: unknown, -): value is { enabled?: boolean; redis?: unknown; defaultTtlMs?: number } | undefined { +): value is { enabled?: boolean; redis?: unknown; defaultTtlMs?: number } | undefined | null { if (value === undefined || value === null) return true;libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts (3)
24-75: Duplicated transform logic creates maintenance risk.The duplicated
applyAutoTransportPersistencefunction mirrors the source implementation, but changes to the source won't automatically update the test. Consider adding an integration test that imports and uses the actualfrontMcpMetadataSchemato catch drift, even if isolated unit tests must use this duplicate.Alternatively, add a comment warning future maintainers to keep these in sync:
// WARNING: This function must stay in sync with applyAutoTransportPersistence // in front-mcp.metadata.ts. Update both if logic changes.
264-278: Test assertion is incomplete for the fall-through case.The test verifies
defaultTtlMsis preserved but doesn't assert what happened toenabledandredis. Add explicit assertions to verify the expected fall-through behavior:🔎 Suggested improvement
const result = testSchema.parse(config); // persistence.enabled defaults to false in schema, so transform checks Case 3 (enabled: true without redis) // which won't match since enabled is false by default, so it falls through expect(result.transport?.persistence?.defaultTtlMs).toBe(7200000); + // Verify enabled defaulted to false (from schema) and redis was NOT auto-populated + expect(result.transport?.persistence?.enabled).toBe(false); + expect(result.transport?.persistence?.redis).toBeUndefined();Based on coding guidelines: "Test all code paths including error cases and constructor validation."
339-398: Good coverage of validation errors and type guard behavior.The tests cover:
- Schema validation errors (invalid host, port, defaultTtlMs)
- Type guard behavior for various inputs (undefined, null, valid/invalid objects)
One remaining gap from the prior review: testing
persistence: nullthrough the full schema parse (not just the type guard). While Zod likely rejects null, adding a test would document the expected behavior:🔎 Optional additional test
it('should handle persistence: null (schema behavior)', () => { const config = { redis: { host: 'global-redis' }, transport: { persistence: null as unknown as undefined, }, }; // Verify behavior - document whether it throws or treats as undefined expect(() => testSchema.parse(config)).toThrow(); // or .not.toThrow() based on actual behavior });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.tslibs/sdk/src/common/metadata/front-mcp.metadata.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Use 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 type parameters with constraints instead of unconstrained generics
Do not mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.tslibs/sdk/src/common/metadata/front-mcp.metadata.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including error cases and constructor validation
Include error classinstanceofchecks in tests to validate error handling
Files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/sdk/src/**/*.ts: MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead ofunknown
Use specific error classes with MCP error codes instead of generic errors
UsegetCapabilities()method for dynamic capability exposure instead of hardcoding capabilities
UsechangeScopeproperty instead ofscopein change events to avoid confusion with Scope class
Validate URIs per RFC 3986 at metadata level using proper validation
Validate hook flows and fail fast on invalid hook configurations with specific error messages
Files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.tslibs/sdk/src/common/metadata/front-mcp.metadata.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/metadata/__tests__/front-mcp.metadata.test.tslibs/sdk/src/common/metadata/front-mcp.metadata.ts
🧠 Learnings (9)
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.tslibs/sdk/src/common/metadata/front-mcp.metadata.ts
📚 Learning: 2025-12-22T13:01:16.929Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-22T13:01:16.929Z
Learning: Applies to libs/uipack/src/{adapters,components,web-components}/**/*.test.ts : Test component behavior across platform configurations (OpenAI, Claude, etc.)
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to **/*.test.{ts,tsx} : Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
📚 Learning: 2025-12-22T13:01:16.929Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-22T13:01:16.929Z
Learning: Applies to libs/uipack/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to **/*.test.{ts,tsx} : Test all code paths including error cases and constructor validation
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
📚 Learning: 2025-12-22T13:01:03.646Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-22T13:01:03.646Z
Learning: Applies to libs/ui/src/**/*.test.{ts,tsx} : Use React Testing Library for component tests with 95%+ coverage across statements, branches, functions, and lines
Applied to files:
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to libs/sdk/src/**/*.ts : MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead of `unknown`
Applied to files:
libs/sdk/src/common/metadata/front-mcp.metadata.ts
📚 Learning: 2025-12-19T02:04:46.464Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.464Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/sdk/src/common/metadata/front-mcp.metadata.ts
📚 Learning: 2025-12-22T13:01:03.646Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-22T13:01:03.646Z
Learning: Applies to libs/ui/src/universal/**/*.{ts,tsx} : Export FrontMCPProvider and UniversalApp from universal app shell for platform-agnostic React configuration
Applied to files:
libs/sdk/src/common/metadata/front-mcp.metadata.ts
🧬 Code graph analysis (1)
libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts (2)
libs/sdk/src/common/types/options/redis.options.ts (1)
redisOptionsSchema(171-171)libs/sdk/src/common/types/options/transport.options.ts (1)
transportOptionsSchema(65-156)
⏰ 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). (2)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
🔇 Additional comments (2)
libs/sdk/src/common/metadata/front-mcp.metadata.ts (1)
144-200: Transform logic is well-structured and addresses prior review concerns.The implementation correctly handles all documented cases:
- Early exit when no global redis
- Runtime type guard validation before proceeding
- Respects explicit
enabled: false- Respects explicit persistence redis config
- Auto-populates global redis when
enabled: truebut no redis specified- Auto-enables when persistence is completely undefined
The runtime type guard (
isPersistenceObject) addresses the prior concern about unsafe type assertions.libs/sdk/src/common/metadata/__tests__/front-mcp.metadata.test.ts (1)
77-83: Test schema setup is well-designed.Using the actual
redisOptionsSchemaandtransportOptionsSchemawhile isolating the transform logic provides good coverage of real-world schema behavior without the circular dependency issues.
…ration
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.