-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Add support for scoped plugins with app and server scope configurations #179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds plugin scoping (app vs server): metadata, tokens, schema, and registry support; propagates scope context through PluginScopeInfo; validates/handles server-scoped plugins for standalone vs non-standalone apps; initializes server-level plugins in scope lifecycle; docs and tests updated. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libs/sdk/src/app/instances/app.local.instance.ts (1)
42-51: Consider handling whenstandaloneis undefined.The conditions
standalone === falseandstandalone === truewill both evaluate tofalsewhenstandaloneisundefined. This means:
parentScopewill beundefined(no parent scope)isStandaloneAppwill befalse(not treated as standalone)This creates an ambiguous state where an app with undefined
standalonehas no parent scope but isn't marked as standalone, potentially allowing server-scoped plugins without a valid parent scope target. Consider whether this is the intended default behavior.🔎 Consider explicit handling for undefined standalone
// Build scope info for plugin hook registration // This determines where plugin hooks are registered based on plugin's scope setting: // - scope='app' (default): hooks register to app's own scope // - scope='server': hooks register to parent scope (gateway-level) + // When standalone is not explicitly set, default to standalone behavior + const isStandalone = this.metadata.standalone !== false; const scopeInfo: PluginScopeInfo = { ownScope: this.appProviders.getActiveScope(), - parentScope: this.metadata.standalone === false ? this.scopeProviders.getActiveScope() : undefined, - isStandaloneApp: this.metadata.standalone === true, + parentScope: isStandalone ? undefined : this.scopeProviders.getActiveScope(), + isStandaloneApp: isStandalone, };libs/sdk/src/plugin/__tests__/plugin.registry.test.ts (2)
903-926: Test assertion may not validate the intended behavior.The test "should register hooks to own scope when scope is app" expects
ownScope.hooks.registerHooksnot to have been called, with the comment "No hooks in this plugin". However, the test's purpose is to verify hooks register to the own scope.This test doesn't actually verify the scope-based hook registration behavior—it only confirms that a plugin without hooks doesn't call
registerHooks. Consider adding actual hooks to the plugin to validate the registration target, or rename the test to clarify its intent.🔎 Consider adding hooks to validate registration target
it('should register hooks to own scope when scope is app', async () => { + // Define a mock hook to verify registration target + const mockHook = jest.fn(); + @FrontMcpPlugin({ name: 'AppScopePluginWithHooks', scope: 'app', + // Add actual hooks to test registration }) - class AppScopePluginWithHooks implements PluginInterface {} + class AppScopePluginWithHooks implements PluginInterface { + // Add hook methods here to enable testing + } const providers = await createProviderRegistryWithScope(); const ownScope = providers.get(Scope); const parentScope = createMockScope(); const scopeInfo: PluginScopeInfo = { ownScope, parentScope, isStandaloneApp: false, }; const registry = new PluginRegistry(providers, [AppScopePluginWithHooks], undefined, scopeInfo); await registry.ready; - // Hooks should be registered to own scope, not parent scope - expect(ownScope.hooks.registerHooks).not.toHaveBeenCalled(); // No hooks in this plugin + // Verify hooks registered to ownScope, not parentScope + expect(ownScope.hooks.registerHooks).toHaveBeenCalled(); + expect(parentScope.hooks.registerHooks).not.toHaveBeenCalled(); });
928-951: Same limitation as above test.This test also doesn't have actual hooks in the plugin, so it cannot verify that hooks are registered to the parent scope. The test only confirms that a server-scoped plugin can be registered when
parentScopeis available.Consider adding hooks to both scope fallback tests to validate the actual hook registration target, or update the test names to reflect what they actually verify.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/draft/docs/extensibility/plugins.mdxlibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/metadata/plugin.metadata.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/common/tokens/plugin.tokens.tslibs/sdk/src/errors/index.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/plugin/plugin.utils.tslibs/sdk/src/scope/scope.instance.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.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/errors/index.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/plugin/plugin.utils.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/common/metadata/plugin.metadata.tslibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/common/tokens/plugin.tokens.ts
libs/sdk/src/**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Export public API through barrel files - export users' needed items, no legacy exports with different names
Files:
libs/sdk/src/errors/index.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/errors/index.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/plugin/plugin.utils.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/common/metadata/plugin.metadata.tslibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/common/tokens/plugin.tokens.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/index.tslibs/sdk/src/common/metadata/front-mcp.metadata.tslibs/sdk/src/common/tokens/front-mcp.tokens.tslibs/sdk/src/plugin/plugin.utils.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/common/metadata/plugin.metadata.tslibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/common/tokens/plugin.tokens.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/plugin/__tests__/plugin.registry.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/extensibility/plugins.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/plugins.mdx
🧠 Learnings (8)
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/errors/index.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/scope/scope.instance.tslibs/sdk/src/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/common/metadata/plugin.metadata.tslibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/common/tokens/plugin.tokens.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/sdk/src/errors/index.tslibs/sdk/src/errors/mcp.error.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/index.ts : Export public API through barrel files - export users' needed items, no legacy exports with different names
Applied to files:
libs/sdk/src/errors/index.tslibs/sdk/src/plugin/__tests__/plugin.registry.test.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
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/tokens/front-mcp.tokens.tslibs/sdk/src/common/metadata/plugin.metadata.tslibs/sdk/src/common/tokens/plugin.tokens.ts
📚 Learning: 2025-12-24T00:41:41.819Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:41.819Z
Learning: Applies to libs/ui/src/react/hooks/**/*.{ts,tsx} : MCP bridge hooks (useMcpBridge, useCallTool, useToolInput, useToolOutput, useTheme) must be properly typed and handle loading/error states
Applied to files:
libs/sdk/src/common/metadata/front-mcp.metadata.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
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.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/common/metadata/plugin.metadata.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validation
Applied to files:
libs/sdk/src/common/metadata/plugin.metadata.ts
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Validate hook flows and fail fast on invalid hook configurations with specific error messages
Applied to files:
libs/sdk/src/plugin/plugin.registry.ts
🧬 Code graph analysis (7)
libs/sdk/src/common/metadata/front-mcp.metadata.ts (2)
libs/sdk/src/common/interfaces/plugin.interface.ts (1)
PluginType(10-14)libs/sdk/src/common/schemas/annotated-class.schema.ts (1)
annotatedFrontMcpPluginsSchema(75-96)
libs/sdk/src/common/tokens/front-mcp.tokens.ts (1)
libs/sdk/src/common/tokens/base.tokens.ts (1)
tokenFactory(9-9)
libs/sdk/src/errors/mcp.error.ts (1)
libs/sdk/src/errors/index.ts (2)
InvalidPluginScopeError(20-20)InternalMcpError(5-5)
libs/sdk/src/scope/scope.instance.ts (1)
libs/sdk/src/plugin/plugin.registry.ts (2)
PluginRegistry(31-230)PluginScopeInfo(22-29)
libs/sdk/src/common/metadata/plugin.metadata.ts (1)
libs/sdk/src/common/schemas/annotated-class.schema.ts (6)
annotatedFrontMcpProvidersSchema(29-50)annotatedFrontMcpPluginsSchema(75-96)annotatedFrontMcpAdaptersSchema(98-119)annotatedFrontMcpToolsSchema(121-129)annotatedFrontMcpResourcesSchema(131-149)annotatedFrontMcpPromptsSchema(151-162)
libs/sdk/src/app/instances/app.local.instance.ts (1)
libs/sdk/src/plugin/plugin.registry.ts (2)
PluginScopeInfo(22-29)PluginRegistry(31-230)
libs/sdk/src/common/tokens/plugin.tokens.ts (1)
libs/sdk/src/common/tokens/base.tokens.ts (1)
tokenFactory(9-9)
⏰ 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 (17)
libs/sdk/src/common/tokens/plugin.tokens.ts (1)
17-17: LGTM: Scope token added correctly.The new
scopetoken aligns with thePluginMetadata.scopeproperty and follows the established token pattern. Type safety is ensured through thesatisfies RawMetadataShape<PluginMetadata>constraint.libs/sdk/src/common/metadata/front-mcp.metadata.ts (2)
24-24: LGTM: Imports added correctly.The new imports
annotatedFrontMcpPluginsSchemaandPluginTypeare properly added and used in the metadata definition.Also applies to: 29-29
82-87: LGTM: Server-level plugins support added correctly.The
pluginsfield is properly added to both the interface and schema with:
- Clear documentation explaining server-level scope and per-scope instantiation
- Consistent default empty array pattern matching other fields
- Type safety through
annotatedFrontMcpPluginsSchemavalidationAlso applies to: 95-95
docs/draft/docs/extensibility/plugins.mdx (1)
335-418: LGTM: Comprehensive plugin scope documentation.The new "Plugin Scope" section provides:
- Clear explanation of app scope (default) vs. server scope
- Practical code examples for both scopes
- Appropriate warning about standalone app restrictions
- Helpful usage comparison table
- Example for accessing other apps from server scope
The documentation aligns well with the implementation changes and provides valuable guidance for users.
libs/sdk/src/common/metadata/plugin.metadata.ts (2)
3-11: LGTM: Import statements reformatted.The import reorganization improves readability without affecting functionality.
78-85: LGTM: Plugin scope property added correctly.The
scopeproperty is properly implemented with:
- Clear documentation explaining app vs. server scope and standalone restrictions
- Correct Zod 4 enum validation:
z.enum(['app', 'server'])- Appropriate default value of
'app'- Type safety maintained through
satisfies RawZodShape<PluginMetadata>Also applies to: 88-102
libs/sdk/src/errors/mcp.error.ts (1)
361-369: InvalidPluginScopeError is properly implemented and exported.The class correctly extends
InternalMcpErrorwith the specific error code'INVALID_PLUGIN_SCOPE', follows established patterns, and is exported fromlibs/sdk/src/errors/index.ts. Aligns with the coding guideline to use specific error classes with MCP error codes instead of generic errors.libs/sdk/src/errors/index.ts (1)
20-20: LGTM!The new
InvalidPluginScopeErroris correctly exported through the barrel file, following the established pattern for error class exports. This aligns with the coding guidelines for exporting public API through barrel files.libs/sdk/src/common/tokens/front-mcp.tokens.ts (1)
25-26: LGTM!The new
pluginstoken is correctly added using the establishedtokenFactory.meta()pattern, with a clear comment explaining its purpose for server-level plugin instantiation per scope.libs/sdk/src/plugin/plugin.utils.ts (2)
30-38: LGTM!The metadata merging logic for
useClasscorrectly allows inline configuration to override decorator-defined metadata while preserving any decorator values not explicitly overridden. This enables the scope configuration flexibility needed for the new plugin scope feature.
60-67: LGTM!The metadata merging for
useValueplugins correctly handles the case where instances may or may not have decorator metadata on their constructor. Plain objects will gracefully fall back to inline-only metadata.libs/sdk/src/scope/scope.instance.ts (2)
349-351: LGTM!The
pluginsgetter follows the established pattern of other registry getters in the class and correctly exposes the optionalscopePluginsregistry.
102-114: No actionable concerns. The hardcodedisStandaloneApp: falsefor server-level plugins is correct—Scope represents multi-app/gateway contexts where server plugins always operate, not standalone apps.libs/sdk/src/plugin/__tests__/plugin.registry.test.ts (1)
492-776: Comprehensive test coverage for plugin scope feature.The new test suite thoroughly covers:
- Default and explicit scope configurations
- Standalone app validation with
InvalidPluginScopeError- Mixed scope plugin registration
- Backward compatibility without
scopeInfo- Metadata merging precedence for
useClass,useValue, anduseFactory- Nested plugin scope propagation and deeply nested validation
The
instanceofchecks forInvalidPluginScopeErroralign with the coding guidelines for error handling tests.libs/sdk/src/plugin/plugin.registry.ts (3)
17-29: Well-documented interface for scope information.The
PluginScopeInfointerface is clearly documented with JSDoc comments explaining each field's purpose. This provides good developer experience for consumers of the API.
178-221: Solid scope handling implementation with appropriate validation and fallback.The logic correctly:
- Defaults to
'app'scope when not specified- Validates early that server-scoped plugins cannot be used in standalone apps
- Falls back to own scope with a warning when server scope is requested but no parent scope exists (e.g., for server-level plugins defined directly in
@FrontMcpdecorator)- Registers hooks to the appropriate target scope
The warning at lines 202-208 is appropriately reachable for server-level plugins where
parentScopeis undefined butisStandaloneAppis false.
120-122: LGTM!Correctly propagates
scopeInfoto nested plugin registries, ensuring consistent scope validation throughout the plugin hierarchy. This is well-tested in the nested plugins test suite.
…ugins with appropriate hook registration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/sdk/src/app/instances/app.local.instance.ts (1)
42-54: LGTM: Scope info construction is correct and well-documented.The logic properly handles the standalone determination and scope assignments:
- Defaults
undefinedto standalone (true) for safety, which is the more restrictive option- Correctly assigns
ownScopeto the app's scope andparentScopeto the gateway scope when not standalone- Clear comments explain the behavior and rationale
Optional: Extract scope info construction for better testability
Consider extracting the scope info construction into a private method to improve testability and maintainability:
+ private buildScopeInfo(): PluginScopeInfo { + const isStandalone = this.metadata.standalone !== false; + return { + ownScope: this.appProviders.getActiveScope(), + parentScope: !isStandalone ? this.scopeProviders.getActiveScope() : undefined, + isStandaloneApp: isStandalone, + }; + } + protected async initialize() { // ... existing code ... - const isStandalone = this.metadata.standalone !== false; - const scopeInfo: PluginScopeInfo = { - ownScope: this.appProviders.getActiveScope(), - parentScope: !isStandalone ? this.scopeProviders.getActiveScope() : undefined, - isStandaloneApp: isStandalone, - }; + const scopeInfo = this.buildScopeInfo(); this.appPlugins = new PluginRegistry(this.appProviders, this.metadata.plugins ?? [], appOwner, scopeInfo);libs/sdk/src/plugin/__tests__/plugin.registry.test.ts (1)
109-110: Reduceanytype usage for stricter type safety.Multiple instances use
anytypes or double type assertions (as any as Type), which bypass TypeScript's type safety. According to coding guidelines, strict TypeScript mode should avoidanywithout strong justification.Consider:
- Define a test helper type for plugins with dynamic
getmethods instead ofget: any- Replace double assertions with proper type guards or single assertions
- For factory functions (line 246-248), use proper generic constraints instead of
as anyExample: Define a proper test fixture type
// Define at the top of the test file interface TestPluginWithGet extends PluginInterface { get: <T>(token: any) => T; } // Then use it instead of `as any as PluginType` const plugin = plugins[0] as TestPluginWithGet;As per coding guidelines:
**/*.tsfiles should use strict TypeScript mode with noanytypes without strong justification.Also applies to: 149-149, 162-162, 206-206, 246-248, 455-455, 464-464, 475-475, 490-490
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/plugin/plugin.utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/plugin/plugin.utils.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/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/app/instances/app.local.instance.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/plugin/__tests__/plugin.registry.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/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/app/instances/app.local.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/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/app/instances/app.local.instance.ts
🧠 Learnings (1)
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Use `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/plugin/__tests__/plugin.registry.test.tslibs/sdk/src/app/instances/app.local.instance.ts
🧬 Code graph analysis (1)
libs/sdk/src/app/instances/app.local.instance.ts (1)
libs/sdk/src/plugin/plugin.registry.ts (2)
PluginScopeInfo(22-29)PluginRegistry(31-230)
⏰ 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 (8)
libs/sdk/src/app/instances/app.local.instance.ts (2)
8-8: LGTM: Import correctly adds PluginScopeInfo type.The import properly includes the
PluginScopeInfotype needed for the scope-based plugin registration.
55-55: LGTM: PluginRegistry instantiation correctly passes scope info.The PluginRegistry is properly instantiated with all required parameters including the newly constructed
scopeInfo, which enables proper scope-based hook registration for plugins.libs/sdk/src/plugin/__tests__/plugin.registry.test.ts (6)
6-14: LGTM: Proper imports for scope testing infrastructure.The imports correctly bring in the new scope-related types (PluginScopeInfo, InvalidPluginScopeError, Scope) and test utilities, following the SDK's pattern for typed error handling.
497-780: Excellent: Comprehensive scope validation test coverage.This test suite thoroughly covers:
- Default and explicit scope configurations (app/server)
- Standalone vs non-standalone app constraints
- Error paths with proper
InvalidPluginScopeErrorvalidation- Backward compatibility when scopeInfo is absent
- Metadata merging between decorator and inline configurations
- Mixed registration types (useClass, useValue, useFactory)
The tests properly validate error messages and error types, ensuring the scope validation logic works correctly across all plugin registration patterns.
782-905: Well-designed: Nested plugin scope inheritance tests.The tests validate that scope constraints propagate correctly through plugin hierarchies, including:
- Immediate nested plugins with scope validation
- Deeply nested plugins (3 levels) to ensure recursive validation
- Both success and error paths
- Proper error messages that identify the failing nested plugin by name
This ensures the scope feature works correctly in complex plugin compositions.
907-999: Solid: Hook registration scope behavior validation.The tests properly verify hook registration targets based on plugin scope:
- App-scoped plugins → hooks to own scope
- Server-scoped plugins with parent → hooks to parent scope
- Server-scoped plugins without parent → hooks to own scope + warning
The use of mock spies and proper cleanup (
mockRestore()) ensures isolated, reliable test execution.
1001-1117: Thorough: Metadata merging edge case coverage.The tests validate the scope resolution priority chain:
- Inline configuration scope (highest priority)
- Decorator-provided scope
- Default 'app' scope (fallback)
This ensures the metadata merging logic works correctly across all registration patterns and properly handles missing scope specifications.
Note: The
get: anydeclarations on lines 1010, 1049, 1086 share the same type safety concern raised earlier.
1119-1181: Complete: Factory plugin scope validation.The tests ensure factory-based plugin registrations follow the same scope rules as class-based and value-based registrations:
- Explicit scope validation (server scope in standalone app fails)
- Default scope behavior (defaults to 'app' when unspecified)
This prevents factory plugins from bypassing scope constraints.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.