feat: Implement remote MCP server orchestration with proxy entries for tools, resources, and prompts #186
Conversation
…r tools, resources, and prompts
📝 WalkthroughWalkthroughAdds comprehensive remote MCP support: new McpClientService, remote app metadata/schemas, factories and instance adapters, remote-capability caching, resilience primitives (retry/circuit-breaker/health), registry/flow changes for remote adoption, new Remote* errors, E2E remote demo, cache plugin enhancements, and a LogLevel enum rename. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Gateway as FrontMCP Gateway
participant AppReg as AppRegistry
participant McpClient as McpClientService
participant Remote as Remote MCP App
participant ToolReg as ToolRegistry
participant Flow as CallToolFlow
Client->>Gateway: register remote app metadata
Gateway->>AppReg: normalizeApp -> create remote app entry
AppReg->>McpClient: connect(remote app config)
McpClient->>Remote: establish transport & negotiate
Remote-->>McpClient: connection + capabilities
McpClient->>ToolReg: build remote tool entries (factories)
Client->>Flow: call tool request
Flow->>McpClient: ensureRemoteCapabilities
Flow->>ToolReg: find tool (local then remote)
Flow->>McpClient: callTool(appId, toolName, args)
McpClient->>Remote: proxy execute (with retry/circuit/health)
Remote-->>McpClient: result or error
McpClient-->>Flow: normalized result (or Remote*Error)
Flow-->>Client: final response (may include _meta cache info)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Fix all issues with AI Agents 🤖
In @apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.ts:
- Around line 77-80: Replace the use of the open any in the tools.map callback
with unknown: change the parameter from (t: any) to (t: unknown) and then safely
access name with either a proper type from the test framework (preferred) or a
narrow/type-guard before reading .name (or use a cast only after checking t is
an object with a name property) so the console.log call in the tools.map line
handles unknown values safely and satisfies the strict TypeScript guideline.
In
@apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.ts:
- Around line 4-6: The inputSchema currently allows delayMs up to 300000 (5
minutes) which exceeds the Jest E2E timeout; change the max for delayMs in the
inputSchema to a value below the test timeout (e.g., 100000) to leave a buffer,
or alternatively add a comment above inputSchema documenting that delays must
not exceed the E2E timeout (120000ms) and enforce that constraint in the schema
by setting z.number().min(0).max(100000).describe('Delay in milliseconds (max
~100s to fit E2E timeout)').
In @docs/draft/docs/plugins/cache-plugin.mdx:
- Around line 218-220: The documented default for the bypass header (ParamField
path="bypassHeader") is missing the required prefix and should match the
implementation; update the default value from 'frontmcp-disable-cache' to
'x-frontmcp-disable-cache' so it aligns with the behavior described in
cache.types.ts and the note requiring the x-frontmcp-* prefix, ensuring docs and
code use the same header name.
- Around line 347-350: The example curl in the cache plugin docs uses the wrong
header name; replace the header string "frontmcp-disable-cache" with the correct
default "x-frontmcp-disable-cache" in the code block showing the cache-bypass
example so the curl command sends the proper header.
In @libs/plugins/src/cache/cache.plugin.ts:
- Around line 217-238: The code assumes cached is an object and spreads it into
cachedWithMeta which will throw for primitives returned by
getValue<T=unknown>(); update the logic in the cache hit path (symbols: cached,
cachedRecord, cachedWithMeta, flowCtx.state.rawOutput, toolContext.respond) to
first check the runtime type of cached: if cached is a non-null plain object
(typeof cached === 'object' && cached !== null && !Array.isArray(cached))
merge/assign _meta as you currently do; otherwise wrap the primitive/array into
a stable envelope (e.g. { value: cached, _meta: { cache: 'hit' } }) so meta is
always present and safe to access, then set flowCtx.state.rawOutput and call
toolContext.respond with that envelope; ensure this preserves existing object
behavior and avoids spreading non-objects (note getValue<T=unknown>() can return
any type).
In @libs/sdk/src/app/app.utils.ts:
- Around line 37-47: The code uses a redundant cast and an unsafe any: remove
the unnecessary "as RemoteAppMetadata" cast because isRemoteAppConfig already
narrows item, and replace "useValue: null as any" with a type-safe null (e.g.,
"null as unknown" or the concrete Remote instance type) so the return preserves
type safety; update the block that constructs provide, metadata,
AppKind.REMOTE_VALUE, and useValue accordingly (referencing isRemoteAppConfig,
RemoteAppMetadata, provide, and useValue).
In @libs/sdk/src/app/instances/app.remote.instance.ts:
- Around line 390-409: The getOrCreateMcpClientService function currently
accepts scope: any; replace this with a proper type (e.g., a new or existing
Scope interface that includes an optional mcpClientService: McpClientService and
logger) or use scope: unknown and add a type guard/assertion before accessing
properties. Update references inside getOrCreateMcpClientService to perform a
safe type check/cast (or assert) so scope.logger and scope.mcpClientService are
accessed only after confirming the shape, and ensure you set the
mcpClientService property on the correctly typed scope before returning the
service.
- Around line 299-307: The onCapabilityChange subscription created on
this.mcpClient is never stored or unsubscribed, causing a memory leak; store the
returned unsubscribe function (e.g. const unsubscribeCapability =
this.mcpClient.onCapabilityChange(...)) as a property on the instance (e.g.
this._unsubscribeCapability) and call it when the instance is torn down (add
invocation in the existing dispose/disconnect method or implement a dispose()
that calls this._unsubscribeCapability()). Ensure you also null out the stored
reference after calling it and guard against multiple disposals.
- Around line 455-460: Change the mapRemoteAuth signature to accept the proper
metadata type instead of any: replace the parameter type of
mapRemoteAuth(remoteAuth?: any) with remoteAuth?: RemoteAuthConfig (import
RemoteAuthConfig from the metadata types), keep the return type
McpRemoteAuthConfig | undefined and cast/convert remoteAuth to
McpRemoteAuthConfig as needed inside mapRemoteAuth; also add the
RemoteAuthConfig import to the top of the file and update any usages that call
mapRemoteAuth to satisfy the stronger type.
In @libs/sdk/src/errors/remote.errors.ts:
- Around line 122-158: Add MCP-specific auth error codes to the MCP_ERROR_CODES
enum/object by defining entries for -32001 (Missing credentials/token) and
-32003 (Invalid/insufficient credentials), then update RemoteAuthError and
RemoteAuthorizationError to use the appropriate MCP code (set
RemoteAuthError.mcpErrorCode = -32003 and set
RemoteAuthorizationError.mcpErrorCode = -32003 or to -32001 where you treat
missing credentials specifically); locate the symbols MCP_ERROR_CODES,
RemoteAuthError, and RemoteAuthorizationError in the diff and change their
mcpErrorCode values and the enum/object to include the new numeric constants.
In @libs/sdk/src/hooks/hook.registry.ts:
- Around line 161-173: The hook filtering in the function that returns
allHooks.filter(...) currently treats any non-scope/plugin owner as if it were
an app by falling back to ID comparison; update this predicate to explicitly
handle null owner, 'scope'/'plugin', and 'app' (check hook.metadata.owner.kind
=== 'app' and compare ids), and throw a clear Error for any other unexpected
owner kinds (e.g., 'adapter' or 'agent') so hooks with invalid owner kinds fail
fast with a descriptive message.
In @libs/sdk/src/index.ts:
- Line 14: The SDK currently exports the remote module but lacks user-facing
docs for the client API; add a new draft doc under docs/draft (e.g.,
docs/draft/docs/sdk/remote-client.mdx) that documents how to instantiate and use
McpClientService and the proxy entry classes ProxyToolEntry, ProxyResourceEntry,
and ProxyPromptEntry: include examples for creating a McpClientService
(auth/config hooks), registering/constructing
ProxyToolEntry/ProxyResourceEntry/ProxyPromptEntry, calling their primary
methods, and any lifecycle/teardown considerations; reference the public exports
in libs/sdk/src/index.ts so examples import from the SDK package (not internal
paths), and link this new page from the existing authentication remote docs
(remote.mdx and remote-proxy.mdx).
In @libs/sdk/src/remote/mcp-client.service.ts:
- Around line 588-601: The try/catch around new
StreamableHTTPClientTransport(...) is ineffective because the constructor is
synchronous and connection failures happen in connect(); change the logic so you
instantiate the StreamableHTTPClientTransport as before but perform the fallback
in its connect phase: when transportType === 'http' create the
StreamableHTTPClientTransport (using httpOptions?.fallbackToSSE ?? true),
attempt await transport.connect(), and if connect() throws and fallbackToSSE is
true then log the debug message, instantiate SSEClientTransport and call its
connect(); rethrow the original error only if fallback is disabled. Reference
StreamableHTTPClientTransport, SSEClientTransport, connect(), transportType and
httpOptions?.fallbackToSSE to find where to apply this change.
- Around line 518-524: The switch case for 'forward' declares headerName in the
case body without a block; wrap the case body in braces to create block scope
(e.g., case 'forward': { ... break; }) so headerName (and any other locals) are
scoped to that case; ensure you include the existing early return/logic
(gatewayAuthInfo check and this.logger.warn) and return statement inside the new
block and preserve the break to avoid fall-through.
- Around line 364-404: The authContext parameter is accepted but never forwarded
to the underlying MCP client; update callTool, readResource, and getPrompt to
pass authContext headers through to connection.client calls so call-level
authentication is forwarded. Locate the calls to connection.client.callTool,
connection.client.readResource, and connection.client.getPrompt inside the
methods callTool, readResource, and getPrompt and replace the current
placeholder/undefined options argument with the authContext headers (e.g. pass
an options/metadata object that includes headers: authContext?.headers),
ensuring you handle the case where authContext is undefined (preserve current
behavior when no headers are present) and keep error handling identical.
In @libs/sdk/src/resource/resource.registry.ts:
- Around line 136-145: The code unsafely casts ResourceEntry to ResourceInstance
when calling makeRow for entries from getResources()/getResourceTemplates();
change makeRow's signature to accept ResourceEntry (private makeRow(token:
Token, instance: ResourceEntry, lineage: EntryLineage, source:
ResourceRegistry): IndexedResource) and update all callers (including the loop
using getResources/getResourceTemplates and any other callsites) to pass
ResourceEntry directly; ensure any internal logic in makeRow that previously
relied on ResourceInstance methods narrows or type-guards for
ResourceInstance/ProxyResourceEntry as needed rather than using a cast so the
function correctly handles both ResourceInstance and ProxyResourceEntry.
In @libs/sdk/src/tool/tool.registry.ts:
- Around line 106-130: The code incorrectly duck-types remote apps via
getMcpClient and unsafely casts ProxyToolEntry to ToolInstance while lacking
error handling; instead, replace the fragile typeof check with a robust
metadata/interface check on the app (e.g., an isRemote flag or an interface
method on the app descriptor) before treating it as remote, wrap the call to
app.tools.getTools() in a try/catch and log errors, validate each item in
remoteTools (ensure it is a ProxyToolEntry or ToolEntry) and avoid casting to
ToolInstance—modify usage of makeRow/IndexedTool.instance to accept the correct
entry type or convert ProxyToolEntry to a real ToolInstance via an explicit
adapter function, and keep adoptFromChild for local ToolRegistry cases (symbols
to inspect: getMcpClient, app.tools.getTools, remoteTools, ProxyToolEntry,
ToolInstance, makeRow, IndexedTool.instance, adoptFromChild).
🧹 Nitpick comments (18)
libs/sdk/src/scope/flows/http.request.flow.ts (1)
337-404: Consider consolidating duplicate handler logic.The
handleSse,handleStreamableHttp, andhandleStatefulHttpmethods now have identical implementations—all invokehandle:streamable-httpand continue withnext(). While this works correctly, you could extract the common logic to reduce duplication and improve maintainability.💡 Example consolidation approach
You could create a shared helper method:
private async handleStreamableTransport(context: string) { try { const response = await this.scope.runFlow('handle:streamable-http', this.rawInput); if (response) { this.respond(response); } this.next(); } catch (error) { if (!(error instanceof FlowControl)) { this.logError(error, context); } throw error; } }Then simplify each handler:
@Stage('handleSse', { filter: ... }) async handleSse() { return this.handleStreamableTransport('handleSse'); } @Stage('handleStreamableHttp', { filter: ... }) async handleStreamableHttp() { return this.handleStreamableTransport('handleStreamableHttp'); } @Stage('handleStatefulHttp', { filter: ... }) async handleStatefulHttp() { return this.handleStreamableTransport('handleStatefulHttp'); }libs/sdk/src/transport/mcp-handlers/initialize-request.handler.ts (1)
112-114: Consider supporting a separatetitlefield in server metadata.Line 114 uses
configuredInfo.namefor thetitlefield. While this is functional, the MCP protocol'stitlefield is typically a human-readable display name that may differ from the technicalname. Consider whetherscope.metadata.infoshould support an optionaltitlefield for better customization.🔎 Suggested enhancement
- title: configuredInfo.name, + title: configuredInfo.title ?? configuredInfo.name,Then update the type definition of
scope.metadata.infoto include an optionaltitlefield.libs/plugins/src/cache/cache.plugin.ts (1)
145-165: Consider logging bypass header check failures.The error handling silently returns
falsewhen context storage is unavailable. While this is safe behavior, it could make debugging difficult when the bypass header isn't working as expected.🔎 Proposed enhancement
} catch { // Context storage not available - bypass header cannot be checked + this.logger?.debug('Cache bypass header check failed: context storage unavailable'); return false; }libs/sdk/src/errors/remote.errors.ts (1)
250-260: Consider addingmcpErrorCodetoRemoteUnsupportedTransportError.This error extends
PublicMcpErrorbut unlike other public errors in this file, it doesn't define anmcpErrorCodeproperty. For consistency and proper MCP protocol alignment, consider adding the appropriate error code.🔎 Proposed fix
export class RemoteUnsupportedTransportError extends PublicMcpError { readonly transportType: string; + readonly mcpErrorCode = MCP_ERROR_CODES.INVALID_REQUEST; constructor(transportType: string) {libs/sdk/src/resource/resource.registry.ts (1)
130-132: Use a proper type guard instead of inline type assertion.The current approach uses an inline type assertion to check for
getMcpClient. This pattern should be extracted into a reusable type guard function for better type safety and consistency across registries.🔎 Proposed type guard helper
Consider adding a type guard utility (e.g., in
libs/sdk/src/app/app.utils.ts):export function isRemoteApp(app: unknown): app is { getMcpClient: () => unknown } { return ( typeof app === 'object' && app !== null && 'getMcpClient' in app && typeof (app as any).getMcpClient === 'function' ); }Then use it here:
- // Check if this is a remote app (has getMcpClient method) - // Remote apps use RemoteResourceRegistry which isn't registered as a child registry - const isRemoteApp = typeof (app as { getMcpClient?: unknown }).getMcpClient === 'function'; + const isRemoteApp = isRemoteApp(app);apps/e2e/demo-e2e-remote/project.json (1)
72-80: Consider removingpassWithNoTests: truefrom E2E test target.The
test:e2etarget haspassWithNoTests: true(line 78), which allows the test suite to pass even when no tests are found. This is unusual for E2E tests and may mask issues where tests fail to load or discover properly.If this is a temporary measure during development, consider removing it once E2E tests are in place. Otherwise, document why this configuration is needed.
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/resources/status.resource.ts (1)
14-15: Consider documenting the module-scoped start time.The
serverStartTimetracks when the module was first loaded, not when the server actually started. For E2E tests this is likely acceptable, but consider adding a comment to clarify this behavior or moving the tracking to the server initialization if precise server uptime is needed.Optional: Add clarifying comment
+// Track module load time (approximates server start time for E2E tests) const serverStartTime = Date.now();apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/prompts/greeting.prompt.ts (1)
12-14: Defensive fallback contradictsrequired: trueschema declaration.The
nameargument is markedrequired: truein the decorator (line 7), but line 13 provides a fallback'World'ifargs['name']is falsy. If the framework enforces required arguments beforeexecute()is called, this fallback is unreachable dead code. If not, it silently masks a contract violation.Consider either:
- Removing the fallback if the framework guarantees required args
- Changing
required: falseif the fallback is intentionalapps/e2e/demo-e2e-remote/e2e/remote.e2e.test.ts (2)
34-38: Hardcoded delay is fragile for CI environments.The 2000ms sleep (line 37) after health check is a race condition waiting to happen. If MCP handlers need more initialization time, this arbitrary wait may be insufficient on slower CI runners or excessive on fast machines.
Consider implementing a readiness probe that polls the MCP endpoint until handlers respond, or increase the delay with a comment explaining the rationale.
214-230: Caching test doesn't verify cache behavior.The test acknowledges (line 228-229) that it doesn't actually verify caching occurred — it only confirms both calls succeed. Without comparing timestamps or checking cache headers/metrics, this test provides limited value beyond the existing tool execution tests.
Consider either:
- Adding actual cache verification (e.g., check response metadata or timing differences)
- Renaming to clarify it's a smoke test for repeated calls
libs/sdk/src/common/metadata/app.metadata.ts (2)
304-310: Consider adding validation constraints for numeric transport options.The schema allows any numbers for
timeout,retryAttempts, andretryDelayMs. Consider adding reasonable bounds to catch configuration errors early.🔎 Proposed schema refinements
const remoteTransportOptionsSchema = z.object({ - timeout: z.number().optional(), - retryAttempts: z.number().optional(), - retryDelayMs: z.number().optional(), + timeout: z.number().int().positive().max(300000).optional(), + retryAttempts: z.number().int().nonnegative().max(10).optional(), + retryDelayMs: z.number().int().nonnegative().max(60000).optional(), fallbackToSSE: z.boolean().optional(), headers: z.record(z.string(), z.string()).optional(), });
337-338: Verify URL validation for 'url' transport type.The
urlfield usesz.string()without URL validation. ForurlType: 'url', this should be a valid HTTP(S) URL. Consider adding conditional validation or at minimum a non-empty check.🔎 Proposed validation improvement
- url: z.string(), + url: z.string().min(1),For stricter validation, you could add a
.refine()that validates URL format whenurlType === 'url'.libs/sdk/src/remote/mcp-client.service.ts (1)
682-722: Capability change detection only compares counts, not content.The
emitCapabilityChangeIfNeededmethod only compares array lengths. If a tool is renamed or replaced with another tool (same count), no change event is emitted. Consider comparing tool names or using a hash for more accurate change detection.libs/sdk/src/app/instances/app.remote.instance.ts (1)
106-114: Unsafe cast from ProxyToolEntry[] to ToolInstance[].The cast
snapshot as unknown as readonly ToolInstance[]bypasses type checking. IfProxyToolEntryimplementsToolInstance, declare this relationship properly; otherwise, this could cause runtime issues.Consider having
ProxyToolEntryexplicitly implement or extend theToolInstanceinterface to make this relationship type-safe.libs/sdk/src/remote/entries/proxy-resource.entry.ts (1)
213-229: Empty URI fallback in parseOutput may produce invalid results.When
this.uriis undefined (for templates), the fallbackthis.uri || ''produces an empty URI in the wrapped response, which may confuse consumers.🔎 Proposed improvement
// Otherwise, wrap it in a text content return { contents: [ { - uri: this.uri || '', + uri: this.uri || this.uriTemplate || 'unknown', mimeType: 'text/plain', text: typeof raw === 'string' ? raw : JSON.stringify(raw), }, ], };libs/sdk/src/remote/entries/proxy-tool.entry.ts (3)
270-327: JSON Schema to Zod conversion is limited but functional.The
parseRemoteInputSchemamethod handles basic JSON Schema types but doesn't support:
- Nested objects (recursive schemas)
allOf,oneOf,anyOfcombinators$refreferences- Format constraints (email, uri, etc.)
For a proxy tool, this is acceptable since the remote server performs actual validation. Document this limitation or consider using a library like
json-schema-to-zodfor comprehensive conversion.
218-227: Input parsing re-creates Zod schema on every call.The
parseInputmethod creates a newz.object()on each invocation. Consider caching the compiled schema in the constructor for better performance.🔎 Proposed optimization
+ /** Cached input validator */ + private readonly inputValidator?: z.ZodObject<Record<string, z.ZodTypeAny>>; constructor(...) { // ... existing code ... this.inputSchema = this.parseRemoteInputSchema(remoteTool.inputSchema) as unknown as InSchema; + if (Object.keys(this.inputSchema).length > 0) { + this.inputValidator = z.object(this.inputSchema as Record<string, z.ZodTypeAny>); + } // ... } override parseInput(input: CallToolRequest['params']): CallToolRequest['params']['arguments'] { - if (this.inputSchema && Object.keys(this.inputSchema).length > 0) { - const inputSchemaObj = z.object(this.inputSchema); - return inputSchemaObj.parse(input.arguments); + if (this.inputValidator) { + return this.inputValidator.parse(input.arguments); } return input.arguments || {}; }
160-164: Unsafe cast for outputSchema.The cast
'json' as OutSchemabypasses type checking. While this works at runtime, it could mask type mismatches. Consider ifOutSchemashould default to a string literal type.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (62)
apps/e2e/demo-e2e-agents/src/main.tsapps/e2e/demo-e2e-cache/src/main.tsapps/e2e/demo-e2e-codecall/src/main.tsapps/e2e/demo-e2e-errors/src/main.tsapps/e2e/demo-e2e-multiapp/src/main.tsapps/e2e/demo-e2e-notifications/src/main.tsapps/e2e/demo-e2e-openapi/src/main.tsapps/e2e/demo-e2e-orchestrated/src/main.tsapps/e2e/demo-e2e-providers/src/main.tsapps/e2e/demo-e2e-public/src/main.tsapps/e2e/demo-e2e-redis/src/main.tsapps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tsapps/e2e/demo-e2e-remote/jest.e2e.config.tsapps/e2e/demo-e2e-remote/project.jsonapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/index.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/prompts/greeting.prompt.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/resources/status.resource.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/add.tool.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/echo.tool.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/ping.tool.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/main.tsapps/e2e/demo-e2e-remote/src/main.tsapps/e2e/demo-e2e-remote/tsconfig.app.jsonapps/e2e/demo-e2e-remote/tsconfig.jsonapps/e2e/demo-e2e-remote/webpack.config.jsapps/e2e/demo-e2e-remote/webpack.local.config.jsapps/e2e/demo-e2e-serverless/src/main.tsapps/e2e/demo-e2e-transparent/src/main.tsapps/e2e/demo-e2e-transport-recreation/src/main.tsapps/e2e/demo-e2e-ui/src/main.tsdocs/draft/docs/plugins/cache-plugin.mdxdocs/draft/docs/servers/apps.mdxlibs/plugins/src/cache/__tests__/cache.plugin.test.tslibs/plugins/src/cache/cache.plugin.tslibs/plugins/src/cache/cache.types.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/interfaces/app.interface.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/common/schemas/annotated-class.schema.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/common/types/options/logging.options.tslibs/sdk/src/errors/index.tslibs/sdk/src/errors/remote.errors.tslibs/sdk/src/hooks/hook.registry.tslibs/sdk/src/index.tslibs/sdk/src/logger/instances/instance.console-logger.tslibs/sdk/src/logger/instances/instance.logger.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/remote/entries/index.tslibs/sdk/src/remote/entries/proxy-prompt.entry.tslibs/sdk/src/remote/entries/proxy-resource.entry.tslibs/sdk/src/remote/entries/proxy-tool.entry.tslibs/sdk/src/remote/index.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.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/index.tsapps/e2e/demo-e2e-remote/src/main.tsapps/e2e/demo-e2e-remote/jest.e2e.config.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/ping.tool.tslibs/sdk/src/hooks/hook.registry.tsapps/e2e/demo-e2e-ui/src/main.tslibs/sdk/src/errors/index.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/prompts/greeting.prompt.tsapps/e2e/demo-e2e-agents/src/main.tslibs/sdk/src/logger/instances/instance.logger.tslibs/sdk/src/tool/flows/call-tool.flow.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/main.tsapps/e2e/demo-e2e-cache/src/main.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/index.tslibs/plugins/src/cache/__tests__/cache.plugin.test.tsapps/e2e/demo-e2e-transport-recreation/src/main.tsapps/e2e/demo-e2e-serverless/src/main.tsapps/e2e/demo-e2e-transparent/src/main.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/echo.tool.tslibs/sdk/src/common/types/options/logging.options.tsapps/e2e/demo-e2e-codecall/src/main.tslibs/plugins/src/cache/cache.plugin.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/resources/status.resource.tsapps/e2e/demo-e2e-notifications/src/main.tslibs/sdk/src/remote/entries/index.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/tool/tool.registry.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.tsapps/e2e/demo-e2e-redis/src/main.tsapps/e2e/demo-e2e-public/src/main.tslibs/sdk/src/scope/flows/http.request.flow.tsapps/e2e/demo-e2e-orchestrated/src/main.tslibs/sdk/src/common/tokens/app.tokens.tsapps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/plugins/src/cache/cache.types.tslibs/sdk/src/remote/mcp-client.service.tsapps/e2e/demo-e2e-providers/src/main.tsapps/e2e/demo-e2e-openapi/src/main.tslibs/sdk/src/remote/mcp-client.types.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/add.tool.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/schemas/annotated-class.schema.tslibs/sdk/src/resource/resource.registry.tsapps/e2e/demo-e2e-multiapp/src/main.tslibs/sdk/src/common/interfaces/app.interface.tslibs/sdk/src/remote/entries/proxy-prompt.entry.tslibs/sdk/src/remote/entries/proxy-tool.entry.tslibs/sdk/src/remote/entries/proxy-resource.entry.tslibs/sdk/src/remote/index.tsapps/e2e/demo-e2e-errors/src/main.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/logger/instances/instance.console-logger.tslibs/sdk/src/errors/remote.errors.tslibs/sdk/src/prompt/prompt.registry.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/index.tslibs/sdk/src/errors/index.tslibs/sdk/src/remote/entries/index.tslibs/sdk/src/remote/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/index.tslibs/sdk/src/hooks/hook.registry.tslibs/sdk/src/errors/index.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/sdk/src/logger/instances/instance.logger.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/common/types/options/logging.options.tslibs/sdk/src/remote/entries/index.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/schemas/annotated-class.schema.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/common/interfaces/app.interface.tslibs/sdk/src/remote/entries/proxy-prompt.entry.tslibs/sdk/src/remote/entries/proxy-tool.entry.tslibs/sdk/src/remote/entries/proxy-resource.entry.tslibs/sdk/src/remote/index.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/logger/instances/instance.console-logger.tslibs/sdk/src/errors/remote.errors.tslibs/sdk/src/prompt/prompt.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/index.tslibs/sdk/src/hooks/hook.registry.tslibs/sdk/src/errors/index.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tslibs/sdk/src/logger/instances/instance.logger.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/plugins/src/cache/__tests__/cache.plugin.test.tslibs/sdk/src/common/types/options/logging.options.tslibs/plugins/src/cache/cache.plugin.tslibs/sdk/src/remote/entries/index.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/scope/flows/http.request.flow.tslibs/sdk/src/common/tokens/app.tokens.tslibs/plugins/src/cache/cache.types.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/schemas/annotated-class.schema.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/common/interfaces/app.interface.tslibs/sdk/src/remote/entries/proxy-prompt.entry.tslibs/sdk/src/remote/entries/proxy-tool.entry.tslibs/sdk/src/remote/entries/proxy-resource.entry.tslibs/sdk/src/remote/index.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/logger/instances/instance.console-logger.tslibs/sdk/src/errors/remote.errors.tslibs/sdk/src/prompt/prompt.registry.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/plugins/src/cache/__tests__/cache.plugin.test.tsapps/e2e/demo-e2e-remote/e2e/remote.e2e.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/plugins/cache-plugin.mdxdocs/draft/docs/servers/apps.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/plugins/cache-plugin.mdxdocs/draft/docs/servers/apps.mdx
🧠 Learnings (24)
📓 Common learnings
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
📚 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/index.tslibs/sdk/src/errors/index.tslibs/sdk/src/remote/entries/index.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/remote/index.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
libs/sdk/src/index.tsapps/e2e/demo-e2e-remote/tsconfig.app.jsonlibs/sdk/src/remote/entries/index.tsapps/e2e/demo-e2e-orchestrated/src/main.tsapps/e2e/demo-e2e-remote/webpack.config.jslibs/sdk/src/remote/index.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/bundler/**/*.{ts,tsx} : The bundler module must re-export utilities from frontmcp/uipack/bundler and provide SSR component bundling functionality
Applied to files:
libs/sdk/src/index.tsapps/e2e/demo-e2e-remote/webpack.local.config.jsapps/e2e/demo-e2e-remote/webpack.config.js
📚 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 `getCapabilities()` method for dynamic capability exposure instead of hardcoding capabilities
Applied to files:
libs/sdk/src/index.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/common/records/**/*.ts : Centralize record types in common/records directory and import from there instead of module-specific files
Applied to files:
libs/sdk/src/index.tsapps/e2e/demo-e2e-remote/tsconfig.app.jsonapps/e2e/demo-e2e-remote/tsconfig.jsonlibs/sdk/src/remote/entries/index.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/remote/index.tslibs/sdk/src/logger/instances/instance.console-logger.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/index.tslibs/sdk/src/transport/mcp-handlers/initialize-request.handler.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/prompts/greeting.prompt.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/main.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/resources/status.resource.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/common/schemas/annotated-class.schema.tslibs/sdk/src/remote/entries/proxy-prompt.entry.tslibs/sdk/src/remote/index.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/errors/remote.errors.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/index.tsapps/e2e/demo-e2e-remote/src/main.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/ping.tool.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/main.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/index.tsapps/e2e/demo-e2e-transport-recreation/src/main.tsapps/e2e/demo-e2e-codecall/src/main.tslibs/plugins/src/cache/cache.plugin.tsapps/e2e/demo-e2e-public/src/main.tsapps/e2e/demo-e2e-orchestrated/src/main.tsapps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/sdk/src/remote/mcp-client.service.tsapps/e2e/demo-e2e-providers/src/main.tsapps/e2e/demo-e2e-openapi/src/main.tslibs/sdk/src/remote/mcp-client.types.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/add.tool.tslibs/sdk/src/common/schemas/annotated-class.schema.tslibs/sdk/src/common/interfaces/app.interface.tslibs/sdk/src/remote/index.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/errors/remote.errors.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/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
libs/sdk/src/index.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/index.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/common/schemas/annotated-class.schema.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{build,bundler}/**/*.{ts,tsx,js,jsx} : For server-side MDX rendering with bundled React, use frontmcp/ui/renderers instead of frontmcp/uipack/renderers
Applied to files:
libs/sdk/src/index.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/index.tslibs/sdk/src/errors/index.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/main.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/common/schemas/annotated-class.schema.tsapps/e2e/demo-e2e-errors/src/main.tslibs/sdk/src/errors/remote.errors.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{theme,adapters,bundler}/**/*.{test,spec}.{ts,tsx,js,jsx} : Test behavior across all supported platform configurations (OpenAI, Claude, etc.)
Applied to files:
apps/e2e/demo-e2e-remote/jest.e2e.config.tsapps/e2e/demo-e2e-remote/tsconfig.app.jsonapps/e2e/demo-e2e-remote/tsconfig.jsonapps/e2e/demo-e2e-remote/e2e/remote.e2e.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: 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/hooks/hook.registry.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 `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/hooks/hook.registry.tsapps/e2e/demo-e2e-remote/tsconfig.app.jsonapps/e2e/demo-e2e-remote/tsconfig.jsonapps/e2e/demo-e2e-transparent/src/main.tsapps/e2e/demo-e2e-errors/src/main.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{theme,build,bundler}/**/*.{ts,tsx,js,jsx} : Do not hard-code CDN URLs - use theme.cdn configuration instead
Applied to files:
apps/e2e/demo-e2e-remote/webpack.local.config.jsapps/e2e/demo-e2e-remote/webpack.config.js
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{package.json,*.ts,*.tsx,*.js,*.jsx} : Do not add React dependencies to frontmcp/uipack - it must remain React-free. Use frontmcp/ui for React components.
Applied to files:
apps/e2e/demo-e2e-remote/tsconfig.app.json
📚 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 **/*.ts : Use strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
apps/e2e/demo-e2e-remote/tsconfig.app.jsonapps/e2e/demo-e2e-remote/tsconfig.json
📚 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/**/*.test.{ts,tsx} : Use React Testing Library for component tests and include SSR/hydration tests for all interactive components
Applied to files:
apps/e2e/demo-e2e-remote/tsconfig.app.json
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx,js,jsx} : Do not expose internal error details in public APIs - use sanitized error messages
Applied to files:
libs/sdk/src/errors/index.tsapps/e2e/demo-e2e-errors/src/main.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 URIs per RFC 3986 at metadata level using proper validation
Applied to files:
libs/sdk/src/app/app.utils.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: Use Nx for build system with commands like `nx build sdk`, `nx test`, `nx run-many -t test`
Applied to files:
apps/e2e/demo-e2e-remote/project.json
📚 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 **/*.ts : Do not mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/scope/flows/http.request.flow.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/universal/**/*.{ts,tsx} : Universal app shell (UniversalApp, FrontMCPProvider) must provide platform-agnostic React context and initialization
Applied to files:
libs/sdk/src/common/interfaces/app.interface.ts
🧬 Code graph analysis (18)
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/ping.tool.ts (2)
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/add.tool.ts (1)
Tool(17-30)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/echo.tool.ts (1)
Tool(16-29)
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/prompts/greeting.prompt.ts (1)
libs/sdk/src/index.ts (1)
GetPromptResult(17-17)
apps/e2e/demo-e2e-remote/src/local-mcp-server/main.ts (2)
apps/e2e/demo-e2e-public/src/main.ts (1)
FrontMcp(6-25)apps/e2e/demo-e2e-remote/src/main.ts (1)
FrontMcp(8-60)
libs/plugins/src/cache/__tests__/cache.plugin.test.ts (1)
libs/plugins/src/cache/cache.symbol.ts (1)
CacheStoreToken(4-4)
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/echo.tool.ts (4)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (1)
inputSchema(12-17)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/add.tool.ts (1)
Tool(17-30)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/ping.tool.ts (1)
Tool(15-29)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.ts (1)
Tool(18-40)
libs/plugins/src/cache/cache.plugin.ts (4)
libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowCtxOf(13-13)libs/sdk/src/index.ts (1)
FrontMcpContextStorage(26-26)libs/plugins/src/cache/cache.symbol.ts (1)
CacheStoreToken(4-4)libs/sdk/src/store/adapters/store.vercel-kv.adapter.ts (1)
ttl(202-206)
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.ts (4)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (1)
inputSchema(12-17)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/add.tool.ts (1)
Tool(17-30)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/echo.tool.ts (1)
Tool(16-29)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/ping.tool.ts (1)
Tool(15-29)
libs/sdk/src/scope/flows/http.request.flow.ts (3)
libs/sdk/src/logger/instances/instance.logger.ts (1)
error(77-79)libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
error(188-190)libs/sdk/src/common/interfaces/flow.interface.ts (1)
FlowControl(19-43)
libs/sdk/src/common/tokens/app.tokens.ts (1)
libs/sdk/src/common/tokens/base.tokens.ts (1)
tokenFactory(9-9)
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.ts (3)
libs/sdk/src/logger/instances/instance.logger.ts (1)
error(77-79)libs/sdk/src/app/instances/app.remote.instance.ts (3)
tools(335-337)resources(339-341)prompts(343-345)scripts/bump-synchronized-versions.mjs (1)
result(130-130)
libs/sdk/src/remote/mcp-client.service.ts (3)
libs/sdk/src/remote/index.ts (14)
McpClientServiceOptions(33-33)McpClientService(11-11)McpClientConnection(16-16)McpRemoteCapabilities(30-30)McpConnectRequest(34-34)McpCapabilityChangeCallback(39-39)McpConnectionChangeCallback(40-40)McpRemoteAuthContext(28-28)McpUnsubscribeFn(41-41)McpRemoteAuthConfig(27-27)McpHttpTransportOptions(21-21)McpConnectionStatus(17-17)McpCapabilityChangeEvent(31-31)McpStaticCredentials(26-26)libs/sdk/src/remote/mcp-client.types.ts (13)
McpClientServiceOptions(207-218)McpClientConnection(32-49)McpRemoteCapabilities(176-187)McpConnectRequest(223-238)McpCapabilityChangeCallback(289-289)McpConnectionChangeCallback(294-294)McpRemoteAuthContext(162-167)McpUnsubscribeFn(299-299)McpRemoteAuthConfig(135-157)McpHttpTransportOptions(76-87)McpConnectionStatus(27-27)McpCapabilityChangeEvent(192-198)McpStaticCredentials(125-130)libs/sdk/src/errors/remote.errors.ts (10)
RemoteConnectionError(15-29)RemoteNotConnectedError(327-334)RemoteCapabilityDiscoveryError(269-281)RemoteToolNotFoundError(76-86)RemoteToolExecutionError(167-183)RemoteResourceNotFoundError(91-101)RemoteResourceReadError(188-204)RemotePromptNotFoundError(106-116)RemotePromptGetError(209-225)RemoteAuthError(125-139)
apps/e2e/demo-e2e-remote/webpack.config.js (1)
apps/e2e/demo-e2e-remote/webpack.local.config.js (2)
require(1-1)require(2-2)
libs/sdk/src/remote/mcp-client.types.ts (1)
libs/sdk/src/index.ts (1)
GetPromptResult(17-17)
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/add.tool.ts (4)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (1)
inputSchema(12-17)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/echo.tool.ts (1)
Tool(16-29)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/ping.tool.ts (1)
Tool(15-29)apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.ts (1)
Tool(18-40)
libs/sdk/src/app/instances/app.remote.instance.ts (6)
libs/sdk/src/common/interfaces/internal/registry.interface.ts (5)
ToolRegistryInterface(115-129)ResourceRegistryInterface(131-146)PromptRegistryInterface(148-160)PluginRegistryInterface(107-109)AdapterRegistryInterface(111-113)libs/sdk/src/remote/entries/proxy-tool.entry.ts (2)
ProxyToolEntry(108-339)createProxyToolEntry(385-394)libs/sdk/src/remote/index.ts (10)
ProxyToolEntry(46-46)ProxyResourceEntry(53-53)ProxyPromptEntry(60-60)McpClientService(11-11)McpConnectRequest(34-34)McpTransportType(20-20)McpRemoteAuthConfig(27-27)createProxyToolEntry(48-48)createProxyResourceEntry(55-55)createProxyPromptEntry(62-62)libs/sdk/src/remote/entries/proxy-resource.entry.ts (2)
ProxyResourceEntry(85-292)createProxyResourceEntry(346-355)libs/sdk/src/remote/entries/proxy-prompt.entry.ts (2)
ProxyPromptEntry(82-237)createProxyPromptEntry(278-287)libs/sdk/src/remote/mcp-client.types.ts (3)
McpConnectRequest(223-238)McpTransportType(71-71)McpRemoteAuthConfig(135-157)
libs/sdk/src/common/interfaces/app.interface.ts (1)
libs/sdk/src/common/metadata/app.metadata.ts (1)
RemoteAppMetadata(228-302)
libs/sdk/src/remote/entries/proxy-prompt.entry.ts (5)
libs/sdk/src/remote/index.ts (5)
ProxyPromptRecord(63-63)ProxyPromptContext(61-61)McpClientService(11-11)McpRemoteAuthContext(28-28)ProxyPromptEntry(60-60)libs/sdk/src/remote/mcp-client.service.ts (1)
McpClientService(73-760)libs/sdk/src/remote/mcp-client.types.ts (1)
McpRemoteAuthContext(162-167)libs/sdk/src/scope/scope.instance.ts (1)
Scope(37-380)libs/sdk/src/common/entries/prompt.entry.ts (3)
PromptGetExtra(11-13)ParsedPromptResult(15-15)PromptSafeTransformResult(16-16)
libs/sdk/src/remote/entries/proxy-resource.entry.ts (4)
libs/sdk/src/remote/mcp-client.service.ts (1)
McpClientService(73-760)libs/sdk/src/common/interfaces/resource.interface.ts (1)
ResourceCtorArgs(44-49)libs/sdk/src/remote/mcp-client.types.ts (1)
McpRemoteAuthContext(162-167)libs/sdk/src/common/entries/resource.entry.ts (3)
ResourceReadExtra(11-13)ParsedResourceResult(15-15)ResourceSafeTransformResult(16-16)
🪛 ast-grep (0.40.3)
libs/sdk/src/remote/entries/proxy-resource.entry.ts
[warning] 255-255: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${regexStr}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 Biome (2.1.2)
libs/sdk/src/remote/mcp-client.service.ts
[error] 567-569: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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 Libraries
apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/sdk/src/tool/flows/call-tool.flow.ts (2)
597-601: Add runtime type guard forrawMetabefore spreading.The code uses type assertions without runtime validation to extract
_metafromrawOutput. IfrawOutput._metaexists but is not actually a Record (e.g., a primitive or array), the spread operation on line 600 could behave unexpectedly.Consider adding a runtime type check to ensure type safety:
🔎 Proposed refactor with runtime type guard
// Preserve any _meta from rawOutput (e.g., cache plugin adds cache: 'hit') -const rawMeta = (rawOutput as Record<string, unknown>)?.['_meta'] as Record<string, unknown> | undefined; -if (rawMeta) { +const rawMeta = (rawOutput as Record<string, unknown>)?._meta; +if (rawMeta && typeof rawMeta === 'object' && !Array.isArray(rawMeta)) { - result._meta = { ...result._meta, ...rawMeta }; + result._meta = { ...result._meta, ...(rawMeta as Record<string, unknown>) }; }
597-614: Consider combining all_metamerges into a single operation.The code performs sequential spreads for
_meta(lines 600 and 612), creating intermediate objects. While this works correctly, combining them would improve readability and make the precedence order more explicit.🔎 Proposed refactor combining all merges
// Preserve any _meta from rawOutput (e.g., cache plugin adds cache: 'hit') const rawMeta = (rawOutput as Record<string, unknown>)?.['_meta'] as Record<string, unknown> | undefined; -if (rawMeta) { - result._meta = { ...result._meta, ...rawMeta }; -} // Apply UI result if available (from applyUI stage) if (uiResult) { result.content = uiResult.content; // Set structuredContent from UI result (contains raw tool output) // Cast to Record<string, unknown> since MCP protocol expects object type if (uiResult.structuredContent !== undefined && uiResult.structuredContent !== null) { result.structuredContent = uiResult.structuredContent as Record<string, unknown>; } - if (uiMeta) { - result._meta = { ...result._meta, ...uiMeta }; - } } +// Merge all metadata sources with explicit precedence: parse result < rawMeta < uiMeta +result._meta = { + ...result._meta, + ...(rawMeta || {}), + ...(uiMeta || {}), +};This makes the merge precedence explicit and reduces intermediate object allocations.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/sdk/src/tool/flows/call-tool.flow.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/tool/flows/call-tool.flow.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/tool/flows/call-tool.flow.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/tool/flows/call-tool.flow.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @libs/sdk/src/remote/mcp-client.service.ts:
- Around line 54-61: DEFAULT_OPTIONS advertises defaultTimeout, maxRetries and
retryDelayMs but the implementation never uses them; either remove these fields
from McpClientServiceOptions and DEFAULT_OPTIONS or wire them into the request
path. To implement: read options.defaultTimeout, options.maxRetries and
options.retryDelayMs in the McpClientService constructor and apply them inside
the HTTP request method (e.g., the method that performs outbound calls in this
service) by using an AbortController/timeout that cancels the fetch/axios call
after defaultTimeout, and wrap the request in a retry loop that retries up to
maxRetries with a delay of retryDelayMs between attempts; keep
capabilityRefreshInterval handling as-is. Ensure the DEFAULT_OPTIONS and public
McpClientServiceOptions type reflect whichever choice you pick (remove unused
keys if you remove support).
🧹 Nitpick comments (3)
libs/sdk/src/remote/mcp-client.service.ts (3)
122-127: Consider parameterizing client name and version.The MCP client is initialized with hardcoded
name: 'frontmcp-gateway'andversion: '1.0.0'. For better maintainability, consider sourcing these from package.json or accepting them as constructor options, especially if this service is used in different contexts or the version needs to track the SDK version.
260-269: Fetch resource templates in parallel with other capabilities.Resource templates are fetched sequentially after the parallel batch of tools, resources, and prompts. Include
listResourceTemplatesInternalin thePromise.allcall to reduce discovery latency.🔎 Proposed optimization
- const [toolsResult, resourcesResult, promptsResult] = await Promise.all([ + const [toolsResult, resourcesResult, resourceTemplatesResult, promptsResult] = await Promise.all([ this.listToolsInternal(connection), this.listResourcesInternal(connection), + this.listResourceTemplatesInternal(connection), this.listPromptsInternal(connection), ]); - - // Also fetch resource templates - const resourceTemplatesResult = await this.listResourceTemplatesInternal(connection);
725-739: Prevent overlapping capability refresh operations.The
setIntervalcallback invokes the asyncdiscoverCapabilities()without checking if a previous refresh is still in progress. If discovery takes longer thancapabilityRefreshInterval, multiple refreshes can run concurrently, potentially causing race conditions when updating capabilities, unnecessary load on the remote server, and duplicate change events.🔎 Recommended guard mechanism
Add a flag to track refresh state:
// Refresh timers private readonly refreshTimers: Map<string, ReturnType<typeof setInterval>> = new Map(); + private readonly refreshInProgress: Map<string, boolean> = new Map();Then guard the refresh callback:
private startCapabilityRefresh(appId: string): void { if (this.options.capabilityRefreshInterval <= 0) { return; } const timer = setInterval(async () => { + if (this.refreshInProgress.get(appId)) { + this.logger.debug(`Skipping capability refresh for ${appId} - previous refresh still in progress`); + return; + } + this.refreshInProgress.set(appId, true); try { await this.discoverCapabilities(appId); } catch (error) { this.logger.warn(`Capability refresh failed for ${appId}: ${(error as Error).message}`); + } finally { + this.refreshInProgress.set(appId, false); } }, this.options.capabilityRefreshInterval); this.refreshTimers.set(appId, timer); }Also clear the flag in
stopCapabilityRefresh:private stopCapabilityRefresh(appId: string): void { const timer = this.refreshTimers.get(appId); if (timer) { clearInterval(timer); this.refreshTimers.delete(appId); + this.refreshInProgress.delete(appId); } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/sdk/src/remote/mcp-client.service.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/remote/mcp-client.service.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/remote/mcp-client.service.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/remote/mcp-client.service.ts
🧠 Learnings (3)
📚 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/remote/mcp-client.service.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/remote/mcp-client.service.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/remote/mcp-client.service.ts
🧬 Code graph analysis (1)
libs/sdk/src/remote/mcp-client.service.ts (2)
libs/sdk/src/remote/mcp-client.types.ts (11)
McpClientServiceOptions(207-218)McpRemoteCapabilities(176-187)McpConnectRequest(223-238)McpConnectionInfo(54-62)McpRemoteAuthContext(162-167)McpUnsubscribeFn(299-299)McpRemoteAuthConfig(135-157)McpHttpTransportOptions(76-87)McpConnectionStatus(27-27)McpCapabilityChangeEvent(192-198)McpStaticCredentials(125-130)libs/sdk/src/errors/remote.errors.ts (10)
RemoteConnectionError(15-29)RemoteNotConnectedError(327-334)RemoteCapabilityDiscoveryError(269-281)RemoteToolNotFoundError(76-86)RemoteToolExecutionError(167-183)RemoteResourceNotFoundError(91-101)RemoteResourceReadError(188-204)RemotePromptNotFoundError(106-116)RemotePromptGetError(209-225)RemoteAuthError(125-139)
⏰ 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 Libraries
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @libs/sdk/src/app/app.utils.ts:
- Around line 24-30: The type guard isRemoteAppConfig currently only checks that
url is a string; update it to import and call isValidMcpUri and return false if
the URL does not pass RFC 3986 validation (so isRemoteAppConfig(item) must
verify typeof obj['url'] === 'string' && isValidMcpUri(obj['url'])). Also update
the frontMcpRemoteAppMetadataSchema to add the same RFC 3986 URL validation (use
a custom validator that delegates to isValidMcpUri) so both the runtime type
guard and the schema enforce the same URI rules; reference the symbols
isRemoteAppConfig, RemoteAppMetadata, frontMcpRemoteAppMetadataSchema, and
isValidMcpUri when making the changes.
♻️ Duplicate comments (6)
libs/sdk/src/app/app.utils.ts (1)
44-58: Past review partially addressed; consider type-safe alternatives for placeholder.The redundant cast has been removed (line 46) and
null as anyhas been replaced withnull as unknown as AppEntry(line 55), which is an improvement. However, the double cast pattern still bypasses type safety—the type system cannot enforce that this placeholder will be replaced before use.Consider whether AppRecord's
useValuecould be made optional for REMOTE_VALUE entries, or if initialization could use a factory pattern that returns properly typed instances. This would eliminate the need for type assertions entirely.💡 Example type-safe alternative
If the AppRecord type definition allows, consider:
return { kind: AppKind.REMOTE_VALUE, provide, useValue: undefined, // If useValue is optional metadata, };Or use a factory function that returns the properly typed instance after initialization.
libs/sdk/src/tool/tool.registry.ts (1)
108-130: Unsafe type cast persists despite past review feedback.Line 124 still casts
remoteTool as ToolInstance, butapp.tools.getTools()returnsToolEntry[]per the interface. ThemakeRowmethod signature expectsToolInstance, but you're passingProxyToolEntryobjects (which extendToolEntry, notToolInstance). This creates a type contract violation.The validation on line 120 only checks for a
nameproperty, which doesn't guarantee the object has allToolInstance-specific methods that callers ofIndexedTool.instancemay expect.Consider updating
makeRowto acceptToolEntryinstead ofToolInstance, similar to howResourceRegistrywas updated:🔎 Proposed fix
- private makeRow(token: Token, instance: ToolInstance, lineage: EntryLineage, source: ToolRegistry): IndexedTool { + private makeRow(token: Token, instance: ToolEntry, lineage: EntryLineage, source: ToolRegistry): IndexedTool {And update
IndexedTool.instancetype intool.types.tstoToolEntry.libs/sdk/src/remote/mcp-client.service.ts (4)
364-404:authContextparameter accepted but never forwarded.The
callToolmethod acceptsauthContext?: McpRemoteAuthContext(line 368) but never uses it. The MCP client call on line 384-387 doesn't forward any authentication headers. This means call-level authentication configured viaresolveAuthHeaderswon't actually reach the remote server.🔎 Proposed fix concept
The MCP SDK's
callToolmethod may need to receive auth headers through request options. Investigate whether theClient.callToolmethod supports a metadata/options parameter for headers:// If supported by the SDK: const result = await connection.client.callTool( { name: toolName, arguments: args }, undefined, { headers: authContext?.headers } // Forward auth headers );
395-397: Unsafe type cast bypasses type safety.The
as unknown as CallToolResultcast discards type information. Verify the actual return type from the MCP SDK and handle appropriately rather than force-casting.
409-409:authContextalso unused inreadResourceandgetPrompt.Both methods accept the
authContextparameter but never forward it to the underlying client calls, same issue ascallTool.Also applies to: 438-443
589-602: SSE fallback logic increateTransportwon't trigger as intended.The
new StreamableHTTPClientTransport()constructor is synchronous and won't throw connection errors. The try/catch here catches only constructor failures (e.g., invalid URL), not connection failures which occur duringclient.connect(). The SSE fallback should be restructured to handle errors at the connection level.🔎 Proposed restructuring
Move the fallback logic to be triggered when
client.connect(transport)fails in theconnect()method, rather than increateTransport():// In connect() method: try { await client.connect(transport); } catch (error) { if (transportType === 'http' && httpOptions?.fallbackToSSE !== false) { const sseTransport = new SSEClientTransport(new URL(url)); await client.connect(sseTransport); // Update connection.transport to sseTransport } else { throw error; } }
🧹 Nitpick comments (8)
libs/plugins/src/cache/cache.plugin.ts (3)
25-42: Consider edge case handling for empty or wildcard-only patterns.The pattern matching logic is generally sound, but doesn't guard against edge cases like empty patterns (
'') which would match empty tool names, or wildcard-only patterns ('***') which would match any tool name.While these may be rare in practice, adding validation would make the behavior more predictable.
🔎 Suggested guard
function matchesToolPattern(toolName: string, patterns: string[]): boolean { if (!patterns || patterns.length === 0) return false; return patterns.some((pattern) => { + // Skip empty or whitespace-only patterns + if (!pattern || pattern.trim().length === 0) return false; + if (pattern.includes('*')) {
145-165: Improve error handling: avoid silent catch-all.The try-catch block swallows all exceptions without logging, which could mask DI configuration issues or runtime problems. While the safe default (return
false) prevents cache bypass, silent failures make debugging difficult.Consider logging errors or being more specific about expected exceptions.
🔎 Suggested improvement
private shouldBypassCache(_flowCtx: FlowCtxOf<'tools:call-tool'>): boolean { const bypassHeader = this.options.bypassHeader ?? DEFAULT_BYPASS_HEADER; try { // Get custom headers from the context storage // Headers with x-frontmcp-* prefix are stored in metadata.customHeaders const contextStorage = this.get(FrontMcpContextStorage); const context = contextStorage?.getStore(); const customHeaders = context?.metadata?.customHeaders; if (!customHeaders) return false; // Headers are stored lowercase in customHeaders const headerKey = bypassHeader.toLowerCase(); const headerValue = customHeaders[headerKey]; return headerValue === 'true' || headerValue === '1'; - } catch { + } catch (error) { // Context storage not available - bypass header cannot be checked + // Log at debug level to aid troubleshooting without spamming logs + // this.logger?.debug?.('Failed to check bypass header:', error); return false; } }Note: Adjust logging based on your logger availability.
277-283: Clarify public API documentation: method only checks patterns, not metadata.The docstring states "based on metadata or tools list" but the method signature only accepts
toolName, so it cannot check metadata. The implementation only validates againsttoolPatterns.For a public API method in a publishable SDK, consider either:
- Updating the docstring to accurately describe that it only checks configured patterns
- Adding an optional metadata parameter to provide complete cacheability logic
- Renaming to
matchesToolPatternorisToolPatternCacheablefor clarity🔎 Suggested documentation fix
/** - * Check if a tool is cacheable based on metadata or tools list. + * Check if a tool matches any configured cache patterns. + * Note: This only checks toolPatterns configuration, not per-tool metadata. * This can be used by other plugins or flows to determine cacheability. */ isCacheable(toolName: string): boolean {Note: Based on coding guidelines for publishable SDK libraries.
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.ts (2)
34-38: Hardcoded 2-second delay may be fragile.The fixed 2-second wait after health check could cause flaky tests if the MCP handlers need more or less time on different environments. Consider implementing a polling mechanism that checks for MCP readiness explicitly.
🔎 Suggested approach
// Instead of fixed delay, poll for MCP readiness const waitForMcpReady = async (maxAttempts = 10, delayMs = 500) => { for (let i = 0; i < maxAttempts; i++) { try { // Attempt a lightweight MCP operation to verify readiness await localMcpServer?.checkMcpReady(); return; } catch { await new Promise((resolve) => setTimeout(resolve, delayMs)); } } throw new Error('MCP server did not become ready in time'); };
144-152: Timing assertion may be flaky in CI environments.The 50ms tolerance (450ms for 500ms delay) could cause intermittent failures on slower CI runners or under load. Consider increasing the tolerance or using a relative comparison.
🔎 Proposed fix
- expect(elapsed).toBeGreaterThanOrEqual(450); // Allow some tolerance + expect(elapsed).toBeGreaterThanOrEqual(400); // Allow 20% tolerance for CI variabilitylibs/sdk/src/remote/mcp-client.service.ts (1)
676-722: Capability change detection only compares counts, not content.The
emitCapabilityChangeIfNeededmethod only compares array lengths, so it won't detect changes where a tool is removed and a different one is added (same count). Consider comparing tool/resource/prompt names or using a hash of the capability set.libs/sdk/src/app/instances/app.remote.instance.ts (2)
117-126: Consider documenting the cast or adding a type guard.The cast from
ProxyToolEntry[]toToolInstance[]assumes that proxy entries implement theToolInstanceinterface. While this may be correct, consider:
- Adding a comment explaining why this cast is safe, or
- Using a type constraint in the
ProxyToolEntrydefinition to make this relationship explicit.📝 Add explanatory comment
- // Cast proxy entries as ToolInstance - they implement the same interface + // Cast is safe: ProxyToolEntry implements ToolInstance interface through duck typing snapshot: snapshot as unknown as readonly ToolInstance[],
109-113: Consider logging errors in callback notification.The empty catch block silently swallows errors from subscriber callbacks. While this prevents one subscriber from breaking others, consider logging these errors for debugging purposes.
🔎 Proposed enhancement
try { cb(event); - } catch { - // Ignore callback errors + } catch (error) { + // Log but don't propagate subscriber errors to prevent cascade failures + // Note: logger not available in registry; consider adding if needed }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tsapps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.tsdocs/draft/docs/plugins/cache-plugin.mdxlibs/plugins/src/cache/cache.plugin.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/errors/remote.errors.tslibs/sdk/src/hooks/hook.registry.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/resource/resource.events.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/resource/resource.types.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/tool/tool.registry.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/sdk/src/tool/flows/call-tool.flow.ts
- docs/draft/docs/plugins/cache-plugin.mdx
- apps/e2e/demo-e2e-remote/src/local-mcp-server/apps/local-test/tools/slow-operation.tool.ts
- libs/sdk/src/hooks/hook.registry.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/resource/resource.types.tslibs/sdk/src/resource/resource.events.tslibs/sdk/src/errors/mcp.error.tsapps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/plugins/src/cache/cache.plugin.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/errors/remote.errors.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/resource/resource.types.tslibs/sdk/src/resource/resource.events.tslibs/sdk/src/errors/mcp.error.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/errors/remote.errors.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/resource/resource.types.tslibs/sdk/src/resource/resource.events.tslibs/sdk/src/errors/mcp.error.tslibs/plugins/src/cache/cache.plugin.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/errors/remote.errors.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:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.ts
🧠 Learnings (19)
📚 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/common/records/**/*.ts : Centralize record types in common/records directory and import from there instead of module-specific files
Applied to files:
libs/sdk/src/resource/resource.types.tslibs/sdk/src/app/app.utils.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 `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/resource/resource.events.tslibs/sdk/src/app/instances/app.remote.instance.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/mcp.error.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/errors/remote.errors.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/errors/mcp.error.tsapps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/errors/remote.errors.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{theme,adapters,bundler}/**/*.{test,spec}.{ts,tsx,js,jsx} : Test behavior across all supported platform configurations (OpenAI, Claude, etc.)
Applied to files:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.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:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/plugins/src/cache/cache.plugin.tslibs/sdk/src/remote/mcp-client.service.tslibs/sdk/src/remote/mcp-client.types.tslibs/sdk/src/errors/remote.errors.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 **/*.ts : Use strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.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 **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/sdk/src/app/app.utils.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Do not use `any` type without justification in TypeScript code
Applied to files:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.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 **/*.test.{ts,tsx} : Test all code paths including error cases and constructor validation
Applied to files:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.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: Applies to **/*.test.{ts,tsx} : Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.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:
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/app/app.utils.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/remote/mcp-client.types.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/remote/mcp-client.service.tslibs/sdk/src/app/instances/app.remote.instance.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 URIs per RFC 3986 at metadata level using proper validation
Applied to files:
libs/sdk/src/app/app.utils.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
libs/sdk/src/app/app.utils.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/universal/**/*.{ts,tsx} : Universal app shell (UniversalApp, FrontMCPProvider) must provide platform-agnostic React context and initialization
Applied to files:
libs/sdk/src/app/app.utils.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/**/*.{ts,tsx} : Avoid using any type without justification; all props, return types, and generics must be properly typed
Applied to files:
libs/sdk/src/app/app.utils.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Always validate component options before use
Applied to files:
libs/sdk/src/app/app.utils.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 `getCapabilities()` method for dynamic capability exposure instead of hardcoding capabilities
Applied to files:
libs/sdk/src/app/instances/app.remote.instance.ts
🧬 Code graph analysis (6)
apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.ts (2)
libs/sdk/src/logger/instances/instance.logger.ts (1)
error(77-79)libs/sdk/src/app/instances/app.remote.instance.ts (3)
tools(349-351)resources(353-355)prompts(357-359)
libs/sdk/src/tool/tool.registry.ts (1)
libs/sdk/src/app/instances/app.remote.instance.ts (1)
AppRemoteInstance(251-549)
libs/sdk/src/app/app.utils.ts (1)
libs/sdk/src/common/metadata/app.metadata.ts (1)
RemoteAppMetadata(228-302)
libs/sdk/src/app/instances/app.remote.instance.ts (7)
libs/sdk/src/remote/mcp-client.service.ts (1)
McpClientService(72-770)libs/sdk/src/remote/index.ts (10)
McpClientService(11-11)ProxyToolEntry(46-46)ProxyResourceEntry(53-53)ProxyPromptEntry(60-60)McpConnectRequest(34-34)McpTransportType(20-20)McpRemoteAuthConfig(27-27)createProxyToolEntry(48-48)createProxyResourceEntry(55-55)createProxyPromptEntry(62-62)libs/sdk/src/common/interfaces/internal/registry.interface.ts (3)
ToolRegistryInterface(115-129)ResourceRegistryInterface(131-146)PromptRegistryInterface(148-160)libs/sdk/src/remote/entries/proxy-tool.entry.ts (2)
ProxyToolEntry(108-339)createProxyToolEntry(385-394)libs/sdk/src/remote/entries/proxy-resource.entry.ts (2)
ProxyResourceEntry(85-292)createProxyResourceEntry(346-355)libs/sdk/src/remote/entries/proxy-prompt.entry.ts (2)
ProxyPromptEntry(82-237)createProxyPromptEntry(278-287)libs/sdk/src/remote/mcp-client.types.ts (3)
McpConnectRequest(221-236)McpTransportType(71-71)McpRemoteAuthConfig(135-157)
libs/sdk/src/resource/resource.registry.ts (2)
libs/sdk/src/scope/scope.instance.ts (1)
apps(325-327)libs/sdk/src/app/instances/app.remote.instance.ts (1)
AppRemoteInstance(251-549)
libs/sdk/src/errors/remote.errors.ts (1)
libs/sdk/src/errors/mcp.error.ts (3)
InternalMcpError(133-146)PublicMcpError(113-127)MCP_ERROR_CODES(8-25)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (28)
libs/plugins/src/cache/cache.plugin.ts (5)
19-23: LGTM: Clear default bypass header.The constant follows the documented
x-frontmcp-*prefix pattern and is well-documented.
129-139: LGTM: Clear cacheability logic.The method correctly prioritizes explicit metadata over configured patterns, providing a sensible fallback hierarchy.
170-175: LGTM: Clean TTL resolution.The method provides a clear precedence: metadata TTL → configured default → hardcoded default (86400s).
177-249: LGTM: Properly addresses previous type safety concern.The updated logic correctly:
- Checks bypass headers before cache operations
- Uses both
fullNameandnamefor flexible pattern matching- Guards metadata augmentation with
isPlainObjectcheck (lines 221-237), resolving the previous review's concern about spreading primitivesThe fix appropriately returns primitives and arrays as-is when they cannot have
_metaattached.
251-275: LGTM: Consistent bypass and pattern logic.The write path correctly mirrors the read path with bypass checks and dual pattern matching for both
fullNameandname.apps/e2e/demo-e2e-remote/e2e/remote.e2e.test.ts (4)
1-16: Well-structured E2E test suite for remote MCP orchestration.The test file provides comprehensive coverage of remote MCP server functionality including connection, discovery, tool execution, resource access, prompts, caching, error handling, and concurrency.
75-85: Previousanytype issue has been properly addressed.The code now correctly uses
unknowntype with proper type narrowing viatypeofandinchecks, complying with the strict TypeScript guidelines.
238-261: Good error handling test coverage.Tests properly verify error behavior for non-existent tools, missing required parameters, and invalid parameter types. The assertions correctly check for
isError: truerather than expecting exceptions.
263-292: Comprehensive concurrency testing.Tests cover both concurrent local tool calls and mixed local/remote operations, which is essential for validating the gateway's ability to handle parallel requests.
libs/sdk/src/resource/resource.types.ts (1)
24-44: Type change toResourceEntryproperly supports both local and remote resources.The change from
ResourceInstancetoResourceEntryforIndexedResource.instancecorrectly reflects that the registry can now hold both localResourceInstanceobjects and remoteProxyResourceEntryobjects. The inline comment clarifies this dual purpose.libs/sdk/src/errors/mcp.error.ts (1)
8-25: New MCP error codes properly extend the error code surface.The addition of
UNAUTHORIZED(-32001) andFORBIDDEN(-32003) appropriately follows the JSON-RPC specification for server error codes (-32000 to -32099). These codes support the new remote authentication error handling paths.libs/sdk/src/resource/resource.events.ts (1)
1-22: Event payload type correctly updated toResourceEntry.The change to
ResourceEntry[]for the snapshot field aligns with the broader refactoring to support both local and remote resources. ThechangeScopeproperty is correctly used per coding guidelines to avoid confusion with the Scope class.libs/sdk/src/tool/tool.registry.ts (1)
131-140: Local app adoption path correctly preserved.The fallback for local apps properly continues using the existing adoption flow through child
ToolRegistryinstances, maintaining backward compatibility.libs/sdk/src/remote/mcp-client.service.ts (2)
514-525: Switch case properly uses block scope now.The
'forward'case body is now wrapped in braces, addressing the previous BiomenoSwitchDeclarationsissue.
725-748: Good protection against overlapping capability refreshes.The
refreshInProgressSet prevents concurrent refresh operations for the same app, which could cause race conditions or redundant network calls.libs/sdk/src/resource/resource.registry.ts (4)
132-166: Remote resource adoption properly implemented with type-safe approach.The implementation correctly:
- Uses
instanceof AppRemoteInstancefor type-safe remote app detection- Validates remote resource entries before processing
- Uses
ResourceEntrytype directly without unsafe casting- Includes proper error handling and logging
This approach should serve as the model for the similar logic in
ToolRegistry.
452-475:makeRowsignature correctly updated to acceptResourceEntry.The method now accepts
ResourceEntryinstead ofResourceInstance, eliminating the need for unsafe type casts when handling remote proxy entries. This is the correct fix that addresses the type safety concern from previous reviews.
500-521:providerIdOfupdated to work withResourceEntrybase type.The method now safely accesses
metadatavia the base entry interface and uses proper type narrowing withunknownand type guards. This is a good example of handling heterogeneous entry types safely.
225-243: Public API consistently usesResourceEntryreturn types.All public methods that expose resources now return
ResourceEntryor arrays thereof, providing a consistent interface that works with both localResourceInstanceand remoteProxyResourceEntryobjects.Also applies to: 255-258, 263-274, 279-288, 296-298, 301-303, 345-345, 421-424, 428-442
libs/sdk/src/remote/mcp-client.types.ts (2)
6-17: LGTM: Proper use of MCP protocol types.The imports correctly use strict MCP protocol types (
CallToolResult,ReadResourceResult,GetPromptResult) from the MCP SDK, aligning with coding guidelines and learnings for MCP response types.Based on learnings: MCP response types should use strict MCP protocol types.
135-157: LGTM: Well-structured authentication configuration.The
McpRemoteAuthConfigunion type provides a clean discriminated union with proper typing for each auth mode. Themapperfunction in the 'mapped' mode correctly acceptsAuthInfo | undefinedand returns credentials, supporting both authenticated and unauthenticated gateway contexts.libs/sdk/src/app/instances/app.remote.instance.ts (4)
39-42: LGTM: Proper typing replacesanytype.The new
ScopeWithMcpClientinterface provides proper typing for the scope parameter, addressing the previous review comment about avoidinganytypes.Based on coding guidelines: "Use strict TypeScript mode with no
anytypes without strong justification."
268-269: LGTM: Memory leak fixed with proper cleanup.The capability change subscription is now properly stored in
_unsubscribeCapabilityand cleaned up in thedisconnect()method, preventing the memory leak identified in previous reviews.Also applies to: 313-321, 380-383
410-426: LGTM: Proper typing for scope parameter.The method now uses
ScopeWithMcpClientinstead ofany, addressing the previous review comment about type safety.Based on coding guidelines: Avoid
anytypes without strong justification.
472-478: LGTM: Proper typing for remoteAuth parameter.The method now uses
RemoteAuthConfig(imported at line 14) instead ofany, addressing the previous review comment.Based on coding guidelines: Use strict TypeScript mode with no
anytypes.libs/sdk/src/errors/remote.errors.ts (3)
122-158: LGTM: Correct MCP authentication error codes.The authentication error classes now use the proper MCP-specific error codes:
RemoteAuthErrorusesMCP_ERROR_CODES.UNAUTHORIZED(-32001) for missing credentialsRemoteAuthorizationErrorusesMCP_ERROR_CODES.FORBIDDEN(-32003) for invalid/insufficient credentialsThis addresses the previous review comment and aligns with MCP JSON-RPC 2.0 authentication error conventions.
Based on learnings: Use specific error classes with MCP error codes instead of generic errors.
15-46: LGTM: Connection errors properly classified as internal.Both
RemoteConnectionErrorandRemoteDisconnectErrorcorrectly extendInternalMcpError, which is appropriate since connection failures are typically infrastructure/internal issues rather than client errors.
55-67: LGTM: Proper use of MCP error codes and HTTP status codes.The error classes correctly map to appropriate MCP error codes:
RemoteTimeoutError:INTERNAL_ERROR(-32603) with 504 Gateway TimeoutRemoteToolNotFoundError:METHOD_NOT_FOUND(-32601) with 404RemoteResourceNotFoundError:RESOURCE_NOT_FOUND(-32002) with 404RemotePromptNotFoundError:METHOD_NOT_FOUND(-32601) with 404- Execution errors:
INTERNAL_ERROR(-32603) with 500These mappings align with MCP protocol conventions and provide appropriate HTTP-level semantics.
Based on learnings: Use specific error classes with MCP error codes.
Also applies to: 76-116, 167-225
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @libs/sdk/src/app/instances/app.remote.instance.ts:
- Around line 402-409: reconnect() currently only re-establishes the client and
recreates proxies but does not re-subscribe to capability change notifications
after disconnect() unsubscribes; extract the capability subscription logic from
initialize() into a private helper (e.g., subscribeToCapabilityChanges or
restoreSubscriptions) and call that helper from both initialize() and
reconnect() (after discoverAndCreateProxies()) so capability change handlers are
re-registered when the instance reconnects.
♻️ Duplicate comments (1)
libs/sdk/src/prompt/prompt.registry.ts (1)
182-199: Fix unsafe type cast and Symbol handling in remote prompt adoption.The issues flagged in the previous review remain:
Unsafe cast (Line 192):
remotePrompt as PromptInstanceis incorrect.getPrompts()returnsPromptEntry[], butmakeRowexpectsPromptInstance. Unlikeresource.registry.tswhich updatedmakeRowto acceptResourceEntry, this file'smakeRowsignature (line 410-415) still requiresPromptInstance.Symbol uniqueness (Line 192):
Symbol(remotePrompt.name)creates a new unique symbol on each call. IfdiscoverAndCreateProxies()is called during capability refresh, prompts will be re-added with different symbols, creating duplicates sincelocalRowsis never cleared for remote prompts.🔎 Proposed fix
Update
makeRowsignature to acceptPromptEntry(aligning with resource.registry.ts pattern):private makeRow( token: Token, - instance: PromptInstance, + instance: PromptEntry, lineage: EntryLineage, source: PromptRegistry, ): IndexedPrompt {And remove the unsafe cast:
- const row = this.makeRow(Symbol(remotePrompt.name), remotePrompt as PromptInstance, prepend, this); + const row = this.makeRow(Symbol(remotePrompt.name), remotePrompt, prepend, this);Additionally, consider clearing remote prompt rows before re-adoption or using stable tokens via
Symbol.for().
🧹 Nitpick comments (2)
libs/sdk/src/app/instances/app.remote.instance.ts (1)
117-126: Avoid unsafe double cast throughunknown.The cast
snapshot as unknown as readonly ToolInstance[]bypasses type checking.ProxyToolEntryextendsToolEntry, notToolInstance. While they may be structurally compatible at runtime, this pattern masks potential type mismatches.Consider updating
ToolChangeEvent.snapshottype to useToolEntry[](the common base), or document why this cast is safe ifProxyToolEntryimplements all requiredToolInstancebehavior.🔎 Alternative approach
If
ToolChangeEventmust useToolInstance, consider adding a type assertion helper that validates compatibility:private createEvent(kind: ToolChangeKind, snapshot: ProxyToolEntry[]): ToolChangeEvent { this.version++; return { kind, changeScope: 'global' as ToolChangeScope, version: this.version, // ProxyToolEntry implements ToolEntry which is compatible with snapshot expectations snapshot: snapshot as readonly ToolEntry[], }; }Or update the event type definition to accept
ToolEntry[]for broader compatibility.libs/sdk/src/resource/resource.registry.ts (1)
419-429: Avoidanytype in disambiguate function.The
poolparameter is typed asMap<string, any>but should useResourceEntryto match the actual usage. Per coding guidelines,anytypes should be avoided.🔎 Proposed fix
- function disambiguate(candidate: string, pool: Map<string, any>, cfg: Required<ResourceExportOptions>): string { + function disambiguate(candidate: string, pool: Map<string, ResourceEntry>, cfg: Required<ResourceExportOptions>): string {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
libs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/entries/app.entry.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/tool/tool.registry.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/entries/app.entry.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.registry.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/entries/app.entry.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.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/common/entries/app.entry.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.registry.ts
🧠 Learnings (6)
📚 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/prompt/prompt.registry.tslibs/sdk/src/app/instances/app.remote.instance.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/common/records/**/*.ts : Centralize record types in common/records directory and import from there instead of module-specific files
Applied to files:
libs/sdk/src/prompt/prompt.registry.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 `getCapabilities()` method for dynamic capability exposure instead of hardcoding capabilities
Applied to files:
libs/sdk/src/app/instances/app.remote.instance.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 `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/app/instances/app.remote.instance.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/app/instances/app.remote.instance.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/app/instances/app.remote.instance.ts
🧬 Code graph analysis (3)
libs/sdk/src/common/entries/app.entry.ts (3)
libs/sdk/src/common/metadata/app.metadata.ts (1)
AppMetadata(350-350)libs/sdk/src/common/records/app.record.ts (1)
AppRecord(23-23)libs/sdk/src/common/interfaces/app.interface.ts (1)
AppInterface(6-6)
libs/sdk/src/app/instances/app.remote.instance.ts (9)
libs/sdk/src/remote/mcp-client.service.ts (1)
McpClientService(72-770)libs/sdk/src/remote/index.ts (8)
McpClientService(11-11)ProxyToolEntry(46-46)ProxyResourceEntry(53-53)ProxyPromptEntry(60-60)McpConnectRequest(34-34)McpTransportType(20-20)McpRemoteAuthConfig(27-27)createProxyToolEntry(48-48)libs/sdk/src/common/interfaces/internal/registry.interface.ts (3)
ToolRegistryInterface(115-129)ResourceRegistryInterface(131-146)PromptRegistryInterface(148-160)libs/sdk/src/remote/entries/proxy-tool.entry.ts (2)
ProxyToolEntry(108-339)createProxyToolEntry(385-394)libs/sdk/src/remote/entries/proxy-resource.entry.ts (1)
ProxyResourceEntry(85-292)libs/sdk/src/remote/entries/proxy-prompt.entry.ts (1)
ProxyPromptEntry(82-237)libs/sdk/src/common/metadata/app.metadata.ts (2)
RemoteAppMetadata(228-302)RemoteAuthConfig(193-223)libs/sdk/src/provider/provider.registry.ts (1)
ProviderRegistry(32-983)libs/sdk/src/remote/mcp-client.types.ts (3)
McpConnectRequest(221-236)McpTransportType(71-71)McpRemoteAuthConfig(135-157)
libs/sdk/src/resource/resource.registry.ts (2)
libs/sdk/src/scope/scope.instance.ts (1)
apps(325-327)libs/sdk/src/resource/resource.types.ts (1)
ResourceExportOptions(50-55)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (11)
libs/sdk/src/common/entries/app.entry.ts (1)
17-25: LGTM! Clean virtual method pattern for remote app detection.The
isRemotegetter with defaultfalseand clear documentation enables polymorphic adoption strategies in registries. This aligns well with theAppRemoteInstanceoverride returningtrue.libs/sdk/src/prompt/prompt.registry.ts (2)
118-131: LGTM! Clear branching for remote vs local app adoption.The conditional logic correctly distinguishes between remote and local apps, delegating to appropriate adoption methods.
201-212: LGTM! Correct delegation to child registry adoption.The method properly filters for app-owned registries and delegates to the existing
adoptFromChildmechanism.libs/sdk/src/app/instances/app.remote.instance.ts (5)
35-42: LGTM! Proper typing replaces previousanyusage.The
ScopeWithMcpClientinterface provides type safety for the scope parameter, addressing the previous review feedback about avoidinganytypes. Based on coding guidelines.
132-217: LGTM! Clean proxy registry implementations.Both
RemoteResourceRegistryandRemotePromptRegistrycorrectly implement their respective interfaces with proper storage and lookup mechanisms.
305-339: LGTM! Proper initialization with subscription cleanup.The initialization correctly:
- Builds connection request
- Connects to remote server
- Discovers and creates proxies
- Stores the capability change subscription for later cleanup (addressing previous memory leak concern)
415-457: LGTM! Proper typing and request construction.The
getOrCreateMcpClientServicemethod now uses proper typing viaScopeWithMcpClient, andbuildConnectRequestcorrectly maps metadata to the connection request format.
488-556: LGTM! Comprehensive proxy discovery and creation.The method properly:
- Clears existing proxies before refresh
- Discovers capabilities from remote server
- Creates proxy entries for tools, resources, templates, and prompts
- Awaits each proxy's ready state before adding to registries
libs/sdk/src/resource/resource.registry.ts (3)
128-141: LGTM! Consistent remote vs local app adoption pattern.The initialization branching mirrors the pattern in
prompt.registry.ts, maintaining consistency across registries.
194-217: Good implementation with proper typing.The method correctly:
- Validates remote resources before adding (lines 206-209)
- Uses
ResourceEntrytype inmakeRow(addressing previous review feedback)- Handles errors gracefully with logging
Note:
Symbol(remoteResource.name)creates unique symbols per call, which could theoretically cause duplicates if resources are refreshed. However, sincediscoverAndCreateProxiesinAppRemoteInstanceclears the remote registries before re-populating, this is mitigated at the source.
464-487: LGTM! Correct generalization to ResourceEntry.The
makeRowsignature now acceptsResourceEntryinstead ofResourceInstance, which:
- Addresses the previous review feedback about unsafe casts
- Properly supports both
ResourceInstanceandProxyResourceEntryinputs- Aligns with the broader API surface updates throughout the file
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
libs/sdk/src/app/app.utils.ts (2)
22-37: RFC 3986 URI validation properly implemented.The type guard now validates URLs using
isValidMcpUriat line 34, addressing the previous review comment about RFC 3986 compliance. The validation occurs both here in the type guard and in the schema (frontMcpRemoteAppMetadataSchema), providing defense in depth.
50-65: Type safety improvements addressed.The redundant cast was removed (line 53 now uses type guard narrowing directly), and
null as anywas changed tonull as unknown as AppEntry(line 62). The placeholder pattern is well-documented with comments explaining that actualAppRemoteInstanceis created duringAppRegistry.initialize.
🧹 Nitpick comments (2)
libs/sdk/src/common/metadata/app.metadata.ts (2)
313-317: Add non-empty validation for credential value.The
valuefield inremoteStaticCredentialsSchemaaccepts empty strings, which would cause silent auth failures at runtime. Consider adding.min(1)to ensure credentials are non-empty.🔎 Suggested fix
const remoteStaticCredentialsSchema = z.object({ type: z.enum(['bearer', 'basic', 'apiKey']), - value: z.string(), + value: z.string().min(1, 'Credential value cannot be empty'), headerName: z.string().optional(), });
305-311: Consider adding non-negative constraints for numeric options.Numeric transport options like
timeout,retryAttempts, andretryDelayMscurrently accept negative values which would be invalid at runtime.🔎 Suggested fix
const remoteTransportOptionsSchema = z.object({ - timeout: z.number().optional(), - retryAttempts: z.number().optional(), - retryDelayMs: z.number().optional(), + timeout: z.number().nonnegative().optional(), + retryAttempts: z.number().int().nonnegative().optional(), + retryDelayMs: z.number().nonnegative().optional(), fallbackToSSE: z.boolean().optional(), headers: z.record(z.string(), z.string()).optional(), });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/sdk/src/app/app.utils.tslibs/sdk/src/common/metadata/app.metadata.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/app/app.utils.tslibs/sdk/src/common/metadata/app.metadata.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/app/app.utils.tslibs/sdk/src/common/metadata/app.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/app/app.utils.tslibs/sdk/src/common/metadata/app.metadata.ts
🧠 Learnings (12)
📚 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 URIs per RFC 3986 at metadata level using proper validation
Applied to files:
libs/sdk/src/app/app.utils.tslibs/sdk/src/common/metadata/app.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/common/records/**/*.ts : Centralize record types in common/records directory and import from there instead of module-specific files
Applied to files:
libs/sdk/src/app/app.utils.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
libs/sdk/src/app/app.utils.tslibs/sdk/src/common/metadata/app.metadata.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/universal/**/*.{ts,tsx} : Universal app shell (UniversalApp, FrontMCPProvider) must provide platform-agnostic React context and initialization
Applied to files:
libs/sdk/src/app/app.utils.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Do not use `any` type without justification in TypeScript code
Applied to files:
libs/sdk/src/app/app.utils.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 **/*.ts : Use strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
libs/sdk/src/app/app.utils.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/app/app.utils.tslibs/sdk/src/common/metadata/app.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 **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/sdk/src/app/app.utils.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/**/*.{ts,tsx} : Avoid using any type without justification; all props, return types, and generics must be properly typed
Applied to files:
libs/sdk/src/app/app.utils.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Always validate component options before use
Applied to files:
libs/sdk/src/app/app.utils.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/app/app.utils.tslibs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{validation,**}/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validation
Applied to files:
libs/sdk/src/app/app.utils.ts
🧬 Code graph analysis (1)
libs/sdk/src/app/app.utils.ts (1)
libs/sdk/src/common/metadata/app.metadata.ts (1)
RemoteAppMetadata(229-303)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (4)
libs/sdk/src/common/metadata/app.metadata.ts (4)
142-175: Well-structured transport options interface.The
RemoteTransportOptionsinterface is well-documented with sensible defaults noted in JSDoc. The separation of transport-specific options from auth configuration follows good design principles.
319-332: Good use of discriminated union for auth configuration.The
remoteAuthConfigSchemacorrectly usesz.discriminatedUnionon themodefield, which enables proper type narrowing and provides clear error messages when validation fails.
338-346: **Schema correctly validates URL format per RFC 3986.**The use of.refine(isValidMcpUri, { message: ... })at line 339-341 correctly implements RFC 3986 URI validation at the schema level. "Refinements are a way to perform custom validation that Zod doesn't provide a native API for" and "Refinement functions should never throw. Instead they should return a falsy value to signal failure." TheisValidMcpUrifunction follows this pattern appropriately.
226-303: Well-documented remote app metadata interface.The expanded
RemoteAppMetadatainterface is comprehensive with clear JSDoc documentation for each transport type (url,worker,npm,esm) and their expected URL formats. The new fields (namespace,transportOptions,remoteAuth,refreshInterval) are well-designed to support the remote MCP orchestration feature.
There was a problem hiding this comment.
Actionable comments posted: 10
Fix all issues with AI Agents 🤖
In @libs/sdk/src/app/instances/app.remote.instance.ts:
- Around line 130-132: The code casts this.metadata to any to access cacheTTL
when initializing capabilityCache; instead extend the RemoteAppMetadata type to
include an optional cacheTTL?: number and update the metadata type import so you
can safely read (this.metadata).cacheTTL, or if you cannot change the type, add
a type guard that narrows this.metadata to a typed interface that includes
cacheTTL before using it in the CapabilityCache constructor (reference the
capabilityCache initialization and RemoteAppMetadata type).
- Around line 393-395: The code casts this.metadata to any to read cacheTTL
before calling this.capabilityCache.set; remove the unsafe any cast by adding
cacheTTL to the instance metadata type and then read it with proper typing
(e.g., const cacheTTL = this.metadata.cacheTTL ?? 60000) so
capabilityCache.set(this.id, capabilities, cacheTTL) uses a correctly typed
value; update the metadata interface where it’s defined (same location fixed for
the constructor issue) and remove the any cast here.
In @libs/sdk/src/prompt/prompt.registry.ts:
- Around line 182-199: The adoptPromptsFromRemoteApp method is unsafely casting
remotePrompt to PromptInstance and creating non-stable Symbols; change makeRow
to accept PromptEntry (not PromptInstance) and pass the prompt without casting,
and replace Symbol(remotePrompt.name) with a stable identifier (e.g.,
Symbol.for(remotePrompt.name)) or clear/cleanup stale entries in localRows
before re-adoption to avoid duplicate prompts; update references in makeRow,
localRows handling, and any callers to reflect the PromptEntry parameter type
change.
In @libs/sdk/src/remote-mcp/cache/capability-cache.ts:
- Around line 73-80: The set method in capability-cache currently overwrites the
capabilities' original fetchedAt with a new Date(); instead, preserve the
existing fetchedAt from the McpRemoteCapabilities passed in. Update the
set(appId: string, capabilities: McpRemoteCapabilities, ttl?: number)
implementation to compute effectiveTTL as before, but when calling
this.cache.set use capabilities.fetchedAt (or fall back to
capabilities.fetchedAt if nullable) for the stored fetchedAt field rather than
creating new Date(); keep expiresAt computed from Date.now() + effectiveTTL and
leave capabilities unchanged.
In @libs/sdk/src/remote-mcp/mcp-client.service.ts:
- Around line 469-538: The callTool operation lacks a per-request timeout:
create an AbortController at the start of the operation (use a configurable
timeout like this.options.requestTimeoutMs or add one), set a timer to call
controller.abort() when the timeout elapses, pass controller.signal to
connection.client.callTool instead of the current undefined second arg, clear
the timer on success, and ensure abort errors are mapped/handled (wrap as
RemoteToolExecutionError or treat as transient for withRetry and circuit/health
handling) so hung remote calls get cancelled deterministically.
- Around line 482-485: The methods callTool, readResource, and getPrompt accept
an authContext parameter but never forward it to the underlying MCP client;
update the calls (e.g., connection.client.callTool,
connection.client.readResource, connection.client.getPrompt) to pass authContext
into the client API (likely as a header or options argument) so the auth
information is propagated, or if the MCP client API does not support
authContext, remove the unused authContext parameter from those method
signatures and callers to avoid confusion; locate the methods in
mcp-client.service.ts (symbols: callTool, readResource, getPrompt,
connection.client.*) and either add the authContext argument to the client calls
or remove the parameter accordingly.
- Around line 734-748: The current try/catch around new
StreamableHTTPClientTransport inside the 'http' case is ineffective because the
constructor is synchronous and network errors happen on client.connect(); change
the flow so you always instantiate StreamableHTTPClientTransport(new URL(url))
(no constructor try/catch), then call transport.connect() inside a try/catch; on
connection failure and if httpOptions?.fallbackToSSE is true, log the fallback,
instantiate SSEClientTransport(new URL(url)), call its connect(), and return the
connected SSE transport; if fallback is disabled rethrow the original connection
error. Ensure references to StreamableHTTPClientTransport, SSEClientTransport,
and connect() are updated accordingly.
In @libs/sdk/src/remote-mcp/resilience/retry.ts:
- Around line 96-99: Remove the non-null assertion on lastError and make the
function type-safe: validate that maxAttempts is >= 1 at the start (throw a
RangeError if not) or ensure lastError is defined before throwing by replacing
"throw lastError!" with "throw lastError ?? new Error('No error captured during
retry')" (and keep the loop/assignment logic in the retry function and
references to lastError intact).
In @libs/sdk/src/tool/tool.registry.ts:
- Around line 166-187: adoptToolsFromRemoteApp currently casts ProxyToolEntry to
ToolInstance and generates transient symbols with Symbol(remoteTool.name);
instead widen the types so makeRow and IndexedTool.instance accept the base
ToolEntry (not only ToolInstance) and stop the unsafe cast (pass remoteTool as
ToolEntry), and replace Symbol(remoteTool.name) with a stable symbol lookup
(e.g., maintain a Map<string, symbol> on the registry/this to getOrCreate a
symbol for remoteTool.name) so repeated refreshes reuse the same symbol; update
signatures of makeRow and IndexedTool.instance to accept ToolEntry and propagate
that type change through creation of EntryLineage rows.
🧹 Nitpick comments (13)
libs/sdk/src/tool/tool.utils.ts (1)
80-87: LGTM - formatting simplification looks good.The single-line case statement is cleaner and the logic is preserved.
One optional consideration: adding an exhaustive check via a default case could guard against future
ToolKindvalues slipping through unhandled at runtime. This is a minor defensive coding suggestion.🔎 Optional: Add exhaustive check
export function toolDiscoveryDeps(rec: ToolRecord): Token[] { switch (rec.kind) { case ToolKind.FUNCTION: return depsOfFunc(rec.provide, 'discovery'); case ToolKind.CLASS_TOKEN: return depsOfClass(rec.provide, 'discovery'); + default: { + const _exhaustive: never = rec.kind; + throw new Error(`Unhandled ToolKind: ${_exhaustive}`); + } } }libs/sdk/src/prompt/prompt.registry.ts (1)
492-525: Consider using a type guard instead of casting with manual validation.The validation logic (lines 497-505) manually checks for required properties before the cast. This pattern works but could be improved with a type guard function for better type safety and reusability:
function isValidPromptInstance(entry: PromptEntry): entry is PromptInstance { const inst = entry as PromptInstance; return !!inst.record?.provide && !!inst.record?.kind && !!inst.record?.metadata && typeof inst.name === 'string'; }Also, as per coding guidelines, consider using specific error classes instead of generic
Errorfor the validation failures.libs/sdk/src/tool/tool.registry.ts (1)
457-471: Validation logic is sound; consider type guard for consistency.The validation at lines 463-471 properly checks required properties before use. The documentation accurately describes the scope binding behavior.
For consistency with TypeScript patterns and potential reuse, consider extracting the validation into a type guard:
function isValidToolInstance(entry: ToolEntry): entry is ToolInstance { const inst = entry as ToolInstance; return !!inst.record?.provide && !!inst.record?.kind && !!inst.record?.metadata && typeof inst.name === 'string'; }This would allow the cast to be type-safe after the guard check.
libs/sdk/src/remote-mcp/resilience/retry.ts (1)
104-143: Consider enhancing error detection with structured properties.The string-based error detection is practical but fragile—error messages vary across environments and can change. Consider supplementing with structured property checks:
export function isTransientError(error: Error): boolean { // Check for Node.js error codes const code = (error as NodeJS.ErrnoException).code; if (code && ['ECONNREFUSED', 'ECONNRESET', 'ETIMEDOUT', 'ENOTFOUND'].includes(code)) { return true; } // Check for HTTP status codes if available const status = (error as { status?: number }).status; if (status && (status === 429 || (status >= 500 && status <= 599))) { return true; } // Fall back to message matching const message = error.message?.toLowerCase() || ''; // ... existing checks }This would be more reliable for Node.js environments while maintaining the message-based fallback.
libs/sdk/src/remote-mcp/cache/capability-cache.ts (1)
143-154: Consider adding a cleanup method for expired entries.The cache uses lazy deletion (only cleaning up on
get()), butgetCachedAppIds()andgetStats()iterate over entries without removing expired ones. For long-running servers with many transient remote apps, expired entries could accumulate.Consider adding an explicit cleanup method:
/** * Remove all expired entries from the cache. * Call periodically or before getStats() for accurate counts. */ purgeExpired(): number { const now = Date.now(); let purged = 0; for (const [appId, cached] of this.cache.entries()) { if (cached.expiresAt <= now) { this.cache.delete(appId); purged++; } } return purged; }libs/sdk/src/remote-mcp/factories/record-builders.ts (1)
56-69: Consider adding remote annotations to resources and prompts for consistency.Tool records include helpful remote-tracking annotations (
frontmcp:remote,frontmcp:remoteAppId,frontmcp:remoteTool), butbuildRemoteResourceRecord,buildRemoteResourceTemplateRecord, andbuildRemotePromptRecorddon't add similar annotations. This inconsistency may complicate debugging and tracing of remote entity origins.🔎 Example annotation addition for resources
const metadata: ResourceMetadata = { name: resourceName, description: remoteResource.description || `Remote resource: ${remoteResource.name}`, uri: remoteResource.uri, mimeType: remoteResource.mimeType, + annotations: { + 'frontmcp:remote': true, + 'frontmcp:remoteAppId': remoteAppId, + 'frontmcp:remoteResource': remoteResource.name, + }, };libs/sdk/src/remote-mcp/resilience/health-check.ts (2)
106-113: Timeout timer continues running after race resolves.The
setTimeoutin the timeout promise isn't cleared whencheckFn()wins the race. While this doesn't cause functional issues, it leaves a dangling timer until it fires. Consider usingAbortControlleror clearing the timeout.🔎 Proposed fix using cleanup
try { - // Create a timeout promise - const timeoutPromise = new Promise<never>((_, reject) => { - setTimeout(() => reject(new Error('Health check timeout')), this.options.timeoutMs); - }); - - // Race check against timeout - await Promise.race([this.checkFn(), timeoutPromise]); + // Create a timeout promise with cleanup + let timeoutId: ReturnType<typeof setTimeout>; + const timeoutPromise = new Promise<never>((_, reject) => { + timeoutId = setTimeout(() => reject(new Error('Health check timeout')), this.options.timeoutMs); + }); + + try { + // Race check against timeout + await Promise.race([this.checkFn(), timeoutPromise]); + } finally { + clearTimeout(timeoutId!); + }
192-217:markHealthy()andmarkUnhealthy()don't updatelastResult.After calling these methods,
getLastResult()returns stale data from the previouscheck()call. This may confuse consumers who expectlastResultto reflect the current status. Consider updatinglastResultin these methods.🔎 Proposed fix for markHealthy
markHealthy(): void { const previousStatus = this.status; this.status = 'healthy'; this.consecutiveSuccesses++; this.consecutiveFailures = 0; + this.lastResult = { + status: this.status, + lastChecked: new Date(), + consecutiveFailures: this.consecutiveFailures, + consecutiveSuccesses: this.consecutiveSuccesses, + }; + if (previousStatus !== 'healthy') { this.options.onStatusChange(this.appId, 'healthy', previousStatus); } }libs/sdk/src/remote-mcp/resilience/circuit-breaker.ts (1)
233-240:getBreaker()ignores options if breaker already exists.When
getBreaker(appId, options)is called for an existing breaker, the providedoptionsare silently ignored. This could lead to confusion if callers expect their options to be applied. Consider either updating the existing breaker's options or logging a warning.🔎 Proposed warning addition
getBreaker(appId: string, options?: CircuitBreakerOptions): CircuitBreaker { let breaker = this.breakers.get(appId); - if (!breaker) { + if (breaker) { + // Existing breaker - options parameter is ignored + return breaker; + } else { breaker = new CircuitBreaker(appId, { ...this.defaultOptions, ...options }); this.breakers.set(appId, breaker); + return breaker; } - return breaker; }libs/sdk/src/remote-mcp/mcp-client.service.ts (2)
150-155: CircuitBreakerManageronStateChangecallback missingappId.The callback receives
(state, prev)but in a multi-app scenario, you need to know which app's circuit breaker changed state. TheCircuitBreakerOptions.onStateChangesignature takes(state, previousState)withoutappId, making this log message incomplete for debugging.🔎 Consider wrapping with per-app callbacks
The issue is in the
CircuitBreakerOptionsinterface which doesn't includeappId. Either:
- Update
CircuitBreakerOptions.onStateChangesignature to includeappId- Or set per-breaker callbacks when creating them via
getBreaker()
543-567:readResource()andgetPrompt()lack resilience features.Unlike
callTool(), these methods don't have circuit breaker checks, retry logic, or health status updates. For consistency and reliability, consider applying the same resilience patterns to all proxy execution methods.libs/sdk/src/app/instances/app.remote.instance.ts (1)
401-454: Consider parallel initialization for better performance.The sequential
await instance.readycalls for each tool, resource, and prompt could be slow with many capabilities. Consider batching withPromise.allif the instances don't have interdependencies.🔎 Suggested optimization
- // Register tools using standard ToolInstance with dynamic context class - for (const remoteTool of capabilities.tools) { - const toolInstance = createRemoteToolInstance(/* ... */); - await toolInstance.ready; - this._tools.registerToolInstance(toolInstance); - } + // Register tools in parallel + const toolInstances = capabilities.tools.map(remoteTool => + createRemoteToolInstance(remoteTool, this.mcpClient, this.id, this.scopeProviders, this.appOwner, namespace) + ); + await Promise.all(toolInstances.map(ti => ti.ready)); + toolInstances.forEach(ti => this._tools.registerToolInstance(ti));Apply similarly for resources, templates, and prompts.
libs/sdk/src/resource/resource.registry.ts (1)
569-602: Good validation but consider type refinement.The validation is thorough, checking for required properties before registration. However, the cast
resource as ResourceInstance(line 571) assumes the parameter is always aResourceInstance. Since the parameter type isResourceEntry, this could cause issues if aProxyResourceEntryor otherResourceEntrysubtype is passed.Consider either:
- Updating the parameter type to be more specific if only
ResourceInstanceis expected- Using type guards to safely access
recordproperties🔎 Suggested type guard approach
registerResourceInstance(resource: ResourceEntry): void { - // Validate that we have a proper instance with required properties - const instance = resource as ResourceInstance; - - // Check for required properties that make it a valid registerable instance - if (!instance.record || !instance.record.provide) { + // Validate using type guards + if (!('record' in resource) || !resource.record || !('provide' in resource.record) || !resource.record.provide) { throw new Error('Resource instance is missing required record.provide property'); } + const instance = resource as ResourceInstance;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
libs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/app/instances/index.tslibs/sdk/src/index.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.utils.tslibs/sdk/src/remote-mcp/cache/capability-cache.tslibs/sdk/src/remote-mcp/cache/index.tslibs/sdk/src/remote-mcp/factories/context-factories.tslibs/sdk/src/remote-mcp/factories/index.tslibs/sdk/src/remote-mcp/factories/instance-factories.tslibs/sdk/src/remote-mcp/factories/record-builders.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/remote-mcp/resilience/circuit-breaker.tslibs/sdk/src/remote-mcp/resilience/health-check.tslibs/sdk/src/remote-mcp/resilience/index.tslibs/sdk/src/remote-mcp/resilience/retry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/resource/resource.utils.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/tool/tool.utils.ts
💤 Files with no reviewable changes (2)
- libs/sdk/src/prompt/prompt.utils.ts
- libs/sdk/src/resource/resource.utils.ts
✅ Files skipped from review due to trivial changes (1)
- libs/sdk/src/app/instances/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/index.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/tool/tool.utils.tslibs/sdk/src/remote-mcp/cache/index.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/remote-mcp/resilience/health-check.tslibs/sdk/src/remote-mcp/resilience/retry.tslibs/sdk/src/remote-mcp/factories/instance-factories.tslibs/sdk/src/remote-mcp/resilience/circuit-breaker.tslibs/sdk/src/remote-mcp/cache/capability-cache.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/remote-mcp/factories/context-factories.tslibs/sdk/src/remote-mcp/resilience/index.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/factories/index.tslibs/sdk/src/remote-mcp/factories/record-builders.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.registry.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/tool/tool.utils.tslibs/sdk/src/remote-mcp/cache/index.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/remote-mcp/resilience/health-check.tslibs/sdk/src/remote-mcp/resilience/retry.tslibs/sdk/src/remote-mcp/factories/instance-factories.tslibs/sdk/src/remote-mcp/resilience/circuit-breaker.tslibs/sdk/src/remote-mcp/cache/capability-cache.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/remote-mcp/factories/context-factories.tslibs/sdk/src/remote-mcp/resilience/index.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/factories/index.tslibs/sdk/src/remote-mcp/factories/record-builders.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.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/tool/tool.utils.tslibs/sdk/src/remote-mcp/cache/index.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/remote-mcp/resilience/health-check.tslibs/sdk/src/remote-mcp/resilience/retry.tslibs/sdk/src/remote-mcp/factories/instance-factories.tslibs/sdk/src/remote-mcp/resilience/circuit-breaker.tslibs/sdk/src/remote-mcp/cache/capability-cache.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/remote-mcp/factories/context-factories.tslibs/sdk/src/remote-mcp/resilience/index.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/factories/index.tslibs/sdk/src/remote-mcp/factories/record-builders.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/resource/resource.registry.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/remote-mcp/cache/index.tslibs/sdk/src/remote-mcp/resilience/index.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/remote-mcp/factories/index.ts
🧠 Learnings (10)
📓 Common learnings
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
📚 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/tool/tool.utils.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/app/instances/app.remote.instance.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/remote-mcp/cache/index.tslibs/sdk/src/remote-mcp/resilience/index.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/remote-mcp/factories/index.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 `getCapabilities()` method for dynamic capability exposure instead of hardcoding capabilities
Applied to files:
libs/sdk/src/remote-mcp/cache/index.tslibs/sdk/src/remote-mcp/cache/capability-cache.tslibs/sdk/src/app/instances/app.remote.instance.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
libs/sdk/src/remote-mcp/cache/index.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/remote-mcp/factories/index.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/common/records/**/*.ts : Centralize record types in common/records directory and import from there instead of module-specific files
Applied to files:
libs/sdk/src/remote-mcp/cache/index.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/remote-mcp/factories/index.tslibs/sdk/src/remote-mcp/factories/record-builders.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/remote-mcp/resilience/health-check.tslibs/sdk/src/remote-mcp/resilience/retry.tslibs/sdk/src/remote-mcp/factories/instance-factories.tslibs/sdk/src/remote-mcp/resilience/circuit-breaker.tslibs/sdk/src/remote-mcp/factories/context-factories.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/factories/record-builders.tslibs/sdk/src/app/instances/app.remote.instance.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/remote-mcp/resilience/health-check.tslibs/sdk/src/remote-mcp/resilience/retry.tslibs/sdk/src/remote-mcp/resilience/circuit-breaker.tslibs/sdk/src/remote-mcp/factories/context-factories.tslibs/sdk/src/remote-mcp/resilience/index.tslibs/sdk/src/remote-mcp/index.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/app/instances/app.remote.instance.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/tool/tool.registry.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/app/instances/app.remote.instance.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/remote-mcp/index.tslibs/sdk/src/remote-mcp/mcp-client.types.tslibs/sdk/src/remote-mcp/mcp-client.service.ts
🧬 Code graph analysis (7)
libs/sdk/src/remote-mcp/resilience/health-check.ts (2)
libs/sdk/src/remote-mcp/index.ts (5)
HealthStatus(64-64)HealthCheckResult(65-65)HealthCheckOptions(66-66)HealthChecker(62-62)HealthCheckManager(63-63)libs/sdk/src/remote-mcp/resilience/index.ts (5)
HealthStatus(19-19)HealthCheckResult(20-20)HealthCheckOptions(21-21)HealthChecker(17-17)HealthCheckManager(18-18)
libs/sdk/src/remote-mcp/resilience/retry.ts (3)
libs/sdk/src/remote-mcp/index.ts (5)
RetryOptions(54-54)withRetry(50-50)isTransientError(51-51)isConnectionError(52-52)isAuthError(53-53)libs/sdk/src/remote-mcp/resilience/index.ts (5)
RetryOptions(6-6)withRetry(6-6)isTransientError(6-6)isConnectionError(6-6)isAuthError(6-6)libs/sdk/src/agent/adapters/base.adapter.ts (1)
sleep(196-198)
libs/sdk/src/remote-mcp/factories/instance-factories.ts (2)
libs/sdk/src/tool/tool.instance.ts (1)
ToolInstance(37-163)libs/sdk/src/remote-mcp/factories/record-builders.ts (4)
buildRemoteToolRecord(46-77)buildRemoteResourceRecord(87-110)buildRemoteResourceTemplateRecord(120-143)buildRemotePromptRecord(153-180)
libs/sdk/src/remote-mcp/cache/capability-cache.ts (1)
libs/sdk/src/remote-mcp/mcp-client.types.ts (1)
McpRemoteCapabilities(176-187)
libs/sdk/src/tool/tool.registry.ts (2)
libs/sdk/src/scope/scope.instance.ts (2)
apps(325-327)Scope(37-380)libs/sdk/src/common/entries/base.entry.ts (1)
EntryLineage(6-6)
libs/sdk/src/remote-mcp/factories/record-builders.ts (6)
libs/sdk/src/remote-mcp/factories/index.ts (6)
buildRemoteToolRecord(19-19)createRemoteToolContextClass(12-12)buildRemoteResourceRecord(20-20)createRemoteResourceContextClass(13-13)buildRemoteResourceTemplateRecord(21-21)createRemotePromptContextClass(14-14)libs/sdk/src/remote-mcp/index.ts (7)
buildRemoteToolRecord(76-76)McpClientService(11-11)createRemoteToolContextClass(72-72)buildRemoteResourceRecord(77-77)createRemoteResourceContextClass(73-73)buildRemoteResourceTemplateRecord(78-78)createRemotePromptContextClass(74-74)libs/sdk/src/common/records/tool.record.ts (1)
ToolClassTokenRecord(10-14)libs/sdk/src/remote-mcp/factories/context-factories.ts (3)
createRemoteToolContextClass(27-38)createRemoteResourceContextClass(47-57)createRemotePromptContextClass(67-78)libs/sdk/src/common/records/resource.record.ts (2)
ResourceClassTokenRecord(14-18)ResourceTemplateClassTokenRecord(39-43)libs/sdk/src/common/records/prompt.record.ts (1)
PromptClassTokenRecord(10-14)
libs/sdk/src/app/instances/app.remote.instance.ts (9)
libs/sdk/src/remote-mcp/index.ts (9)
McpClientService(11-11)CapabilityCache(45-45)McpConnectRequest(34-34)McpTransportType(20-20)McpRemoteAuthConfig(27-27)createRemoteToolInstance(81-81)createRemoteResourceInstance(82-82)createRemoteResourceTemplateInstance(83-83)createRemotePromptInstance(84-84)libs/sdk/src/common/interfaces/internal/registry.interface.ts (5)
PluginRegistryInterface(107-109)AdapterRegistryInterface(111-113)ToolRegistryInterface(115-129)ResourceRegistryInterface(131-146)PromptRegistryInterface(148-160)libs/sdk/src/common/metadata/app.metadata.ts (2)
RemoteAppMetadata(229-303)RemoteAuthConfig(194-224)libs/sdk/src/remote-mcp/cache/capability-cache.ts (1)
CapabilityCache(36-178)libs/sdk/src/resource/resource.registry.ts (1)
ResourceRegistry(36-649)libs/sdk/src/prompt/prompt.registry.ts (1)
PromptRegistry(30-541)libs/sdk/src/common/records/app.record.ts (1)
AppRecord(23-23)libs/sdk/src/remote-mcp/factories/index.ts (4)
createRemoteToolInstance(27-27)createRemoteResourceInstance(28-28)createRemoteResourceTemplateInstance(29-29)createRemotePromptInstance(30-30)libs/sdk/src/remote-mcp/factories/instance-factories.ts (4)
createRemoteToolInstance(40-50)createRemoteResourceInstance(67-77)createRemoteResourceTemplateInstance(94-104)createRemotePromptInstance(121-131)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (32)
libs/sdk/src/tool/tool.instance.ts (1)
128-131: Good addition for exhaustive switch handling.The default case ensures that any future or deprecated tool kinds are caught explicitly rather than falling through silently. The type cast for the error message is appropriate here since TypeScript narrows
this.record.kindtoneverin the default branch.Consider using a specific error class (e.g.,
InvalidToolKindError) for consistency with the codebase's error handling patterns, though a genericErroris acceptable for this internal exhaustive check. As per coding guidelines, specific error classes with MCP error codes are preferred.libs/sdk/src/remote-mcp/cache/index.ts (1)
1-6: LGTM!Clean barrel file that exports exactly the needed public API items (
CapabilityCacheandCapabilityCacheConfig) without legacy exports or aliases. Based on learnings, this follows the recommended pattern for barrel files.libs/sdk/src/remote-mcp/resilience/retry.ts (1)
55-60: LGTM!The
sleeputility is correctly implemented. While a similar implementation exists inbase.adapter.ts, the duplication is acceptable given the function's simplicity and the desire to keep the resilience module self-contained.libs/sdk/src/remote-mcp/resilience/index.ts (1)
1-22: LGTM!Well-organized barrel file that consolidates the resilience module's public API. Exports are properly categorized by source module, and the
typekeyword is correctly used for type-only exports. Based on learnings, this follows the recommended pattern for barrel files.libs/sdk/src/remote-mcp/factories/context-factories.ts (3)
27-38: LGTM - Factory correctly creates dynamic tool context class.The factory properly captures dependencies via closure and returns a typed context class that delegates to the MCP client. The use of strict MCP protocol type
CallToolResultaligns with the coding guidelines.
47-57: LGTM - Resource context factory with proper typing.The generic
Paramsconstraint andReadResourceResultreturn type follow MCP protocol conventions. The unused_paramsparameter is acceptable since the URI already contains the resolved parameters for remote resources.
33-35: The suggestion is incorrect.PromptContextdoes not extendExecutionContextBaseand therefore does not have agetAuthInfo()method—it directly has thereadonly authInfo: AuthInfo;property. The use ofthis.authInfoon line 74 is the correct approach for this class. The apparent inconsistency across the three factory functions reflects their different inheritance patterns:ToolContextandResourceContextboth extendExecutionContextBase(which providesgetAuthInfo()), whilePromptContextis standalone and directly accesses itsauthInfoproperty.Likely an incorrect or invalid review comment.
libs/sdk/src/remote-mcp/factories/instance-factories.ts (1)
40-50: LGTM - Clean factory pattern implementation.All four instance factories follow a consistent pattern: delegate to record builders, then instantiate the appropriate Instance class. The separation of concerns between record building and instance creation is well-designed, allowing remote entities to participate in the hook lifecycle like local entities.
Also applies to: 67-77, 94-104, 121-131
libs/sdk/src/remote-mcp/factories/record-builders.ts (1)
166-171: LGTM - Prompt arguments mapping is defensive.The optional chaining with fallback to empty array (
remotePrompt.arguments?.map(...) || []) correctly handles prompts that don't define arguments.libs/sdk/src/remote-mcp/resilience/health-check.ts (2)
44-163: Well-designed health checker with proper overlap prevention.The
isCheckingflag prevents overlapping checks (lines 92-101), and the status transition logic correctly handles thresholds for both healthy and unhealthy states. The callback invocation is properly guarded with status change detection.
223-293: LGTM - HealthCheckManager provides clean lifecycle management.The manager properly handles checker lifecycle with
addCheckerstopping existing checkers before creating new ones, anddisposecleanly stopping all checkers and clearing the map.libs/sdk/src/remote-mcp/factories/index.ts (1)
1-31: LGTM - Clean barrel exports.The barrel file properly organizes exports into logical groups (context factories, record builders, instance factories) with clear comments. This aligns with the coding guideline to "export users' needed items, no legacy exports with different names."
libs/sdk/src/remote-mcp/resilience/circuit-breaker.ts (3)
103-109: Consider if clearing all failures on success is the intended behavior.In closed state, a single success clears the entire
failuresarray (line 107). This aggressive reset may mask intermittent issues. An alternative is to let failures naturally expire via the sliding window (cleanOldFailures). However, this is a valid design choice for "self-healing" behavior.Is this aggressive failure clearing intentional for the self-healing use case?
42-201: Well-implemented circuit breaker with proper state machine.The state transitions are correct: closed → open (on threshold), open → half-open (on timeout), half-open → closed (on success threshold) or half-open → open (on failure). The sliding window cleanup and proper counter management ensure predictable behavior.
206-217: LGTM - CircuitOpenError provides useful context.The error includes
appId,nextAttemptTime, and a human-readable message with the wait time. This enables callers to implement appropriate retry-after behavior.libs/sdk/src/remote-mcp/mcp-client.service.ts (3)
173-249: Well-structured connection lifecycle with proper cleanup.The
connect()method properly handles existing connections, stores config for reconnection, updates status, and initializes health checking. The cleanup on successful connection (resetting reconnect attempts) is a good self-healing pattern.
685-716: LGTM - Comprehensive dispose method.The
dispose()method properly cleans up all resources: refresh timers, reconnect timers, health check manager, circuit breakers, connections, and callbacks. The parallel disconnect withPromise.allis appropriate.
920-960: Good exponential backoff implementation for auto-reconnect.The
scheduleAutoReconnect()correctly implements exponential backoff (delay * 2^attempts), respects max attempts, and cleans up on success. The recursive scheduling on failure is appropriate for self-healing behavior.libs/sdk/src/remote-mcp/mcp-client.types.ts (4)
1-18: Well-structured imports using strict MCP protocol types.The imports correctly use specific MCP protocol types (
CallToolResult,ReadResourceResult,GetPromptResult) from the SDK rather thanunknown, aligning with the TypeScript-first approach. Based on learnings, this follows the guideline to use strict MCP protocol types.
24-116: Connection and transport types are well-designed.The discriminated transport options (
McpHttpTransportOptions,McpWorkerTransportOptions,McpEsmTransportOptions) provide clear, type-safe configuration for different transport mechanisms. The optional properties with documented defaults are helpful.
135-157: Auth configuration uses proper discriminated unions.The
McpRemoteAuthConfigtype provides a clean discriminated union for different authentication strategies. Themappedmode's async mapper function provides flexibility for credential resolution.
311-348: Result types correctly wrap MCP protocol types with operational metadata.The
McpRemoteCallToolResult,McpRemoteReadResourceResult, andMcpRemoteGetPromptResulttypes properly wrap the strict MCP protocol types (CallToolResult,ReadResourceResult,GetPromptResult) while adding operational metadata likedurationMsandsuccess. This follows the guideline to use strict MCP protocol types. Based on learnings.libs/sdk/src/app/instances/app.remote.instance.ts (5)
40-65: Proper typing for scope with MCP client service.The
ScopeWithMcpClientinterface correctly types the scope object forgetOrCreateMcpClientService, avoiding theanytype issue flagged in previous reviews.
148-178: Initialization properly handles capability change subscription.The initialization correctly stores the unsubscribe function in
_unsubscribeCapability(line 164), addressing the previous memory leak concern. Error handling is appropriate with logging.
188-207: Lazy loading with proper concurrency handling.The
ensureCapabilitiesLoadedmethod correctly handles concurrent access by trackingloadingPromiseand ensuring it's cleared in thefinallyblock. This prevents duplicate capability fetches.
277-290: Disconnect properly cleans up subscription.The
disconnectmethod correctly unsubscribes from capability changes before disconnecting, and clears the unsubscribe function reference to prevent double-unsubscribe issues.
292-300: Missing capability change re-subscription after reconnect.After
disconnect()unsubscribes from capability changes (lines 279-282),reconnect()does not re-establish the subscription. This means capability change notifications will be lost after a reconnect cycle.🔎 Proposed fix
Extract subscription logic to a reusable helper and call it from both
initialize()andreconnect():+ /** + * Subscribe to capability changes for this remote app + */ + private subscribeToCapabilityChanges(): void { + const logger = this.scopeProviders.getActiveScope().logger; + this._unsubscribeCapability = this.mcpClient.onCapabilityChange((event) => { + if (event.appId === this.id) { + logger.info(`Remote capabilities changed for ${this.id}: ${event.kind}`); + this.capabilityCache.invalidate(this.id); + this.capabilitiesLoaded = false; + } + }); + } + async reconnect(): Promise<void> { await this.mcpClient.reconnect(this.id); this.isConnected = true; this.capabilitiesLoaded = false; this.capabilityCache.invalidate(this.id); + this.subscribeToCapabilityChanges(); }Then update
initialize()to use the helper as well.Likely an incorrect or invalid review comment.
libs/sdk/src/remote-mcp/index.ts (1)
1-85: Well-structured barrel exports for remote MCP module.The barrel file properly exports the public API surface with clear organization by category (service, types, cache, resilience, factories). Type exports use the
typekeyword appropriately. This follows the guideline to export users' needed items through barrel files. Based on learnings.libs/sdk/src/resource/resource.registry.ts (4)
127-141: Clean separation of remote vs local app resource adoption.The conditional logic properly distinguishes remote apps (which expose resources directly via proxies) from local apps (which use the hierarchical registry pattern). This aligns with the broader remote MCP architecture.
198-217: Robust remote resource adoption with validation.The method properly validates remote resources (checking for
nameproperty), handles errors gracefully with logging, and continues processing on invalid entries rather than failing entirely. The use ofSymbol(remoteResource.name)for tokens is appropriate for dynamically discovered resources.
219-230: Local app adoption reuses existing patterns.The
adoptResourcesFromLocalAppmethod properly reuses the existingadoptFromChildmechanism, maintaining consistency with the established registry hierarchy pattern.
464-487: Type signature correctly updated to accept ResourceEntry.The
makeRowmethod signature change fromResourceInstancetoResourceEntry(line 466) addresses the previous type safety concern. This allows bothResourceInstanceand proxy entries to be properly indexed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/sdk/src/tool/tool.events.ts (1)
32-34: Consider adding error handling to emit for resilience.Unlike
PromptEmitter.emit(which includes try-catch error handling), this implementation will stop notifying remaining listeners if any listener throws an error. Consider adding defensive error handling similar to the prompt emitter.🔎 Suggested defensive error handling
emit(e: ToolChangeEvent) { - for (const l of [...this.listeners]) l(e); + for (const l of [...this.listeners]) { + try { + l(e); + } catch (err) { + // Log error but continue notifying other listeners + // Use safe logging to avoid Node.js 24 util.inspect bug with Zod errors + console.error('ToolEmitter listener error:', err instanceof Error ? err.message : 'Unknown error'); + } + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
libs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/remote-mcp/cache/capability-cache.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/remote-mcp/resilience/retry.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/tool/tool.types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/remote-mcp/resilience/retry.ts
- libs/sdk/src/remote-mcp/cache/capability-cache.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Enable and use strict TypeScript settings - noanytypes without strong justification, useunknownfor generic type defaults instead
Avoid non-null assertions (!) - use proper error handling and throw specific errors when values are missing instead
Always use @frontmcp/utils for cryptographic operations - use hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode instead of node:crypto
Always use @frontmcp/utils for file system operations - use readFile, writeFile, mkdir, rename, unlink, stat, etc. instead of fs/promises or node:fs
Use constrained generic type parameters withRecord<string, string>or similar constraints instead of unconstrainedanydefaults
Files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/tool/tool.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts: MCP response types must use strictly typed MCP protocol definitions, notunknown- usePromise<GetPromptResult>,Promise<ReadResourceResult>, and similar MCP-defined types for execute() and read() methods
Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/tool/tool.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.registry.ts
**/libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Create shared base classes for common functionality - use ExecutionContextBase as parent class for ToolContext, ResourceContext, and similar context classes
Files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/tool/tool.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/tool/tool.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.registry.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Document known limitations clearly in code comments and README files
Files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/tool/tool.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.registry.ts
**/libs/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add backwards compatibility exports in new libraries - avoid legacy prefixes like PT-001 in test names
Files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/tool/tool.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.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/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/tokens/app.tokens.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/tool/tool.types.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters,plugins}/**/src/**/*event*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
changeScopeinstead ofscopeproperty name in change event types to avoid confusion with Scope class
Files:
libs/sdk/src/prompt/prompt.events.tslibs/sdk/src/tool/tool.events.ts
**/libs/{sdk,adapters,plugins}/**/src/**/*metadata*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Validate URIs per RFC 3986 at metadata level using Zod schema validation with
isValidMcpUrirefinement
Files:
libs/sdk/src/common/metadata/app.metadata.ts
🧠 Learnings (17)
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters}/**/src/**/*.ts : Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Applied to files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/remote-mcp/mcp-client.service.tslibs/sdk/src/tool/tool.registry.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*.ts : MCP response types must use strictly typed MCP protocol definitions, not `unknown` - use `Promise<GetPromptResult>`, `Promise<ReadResourceResult>`, and similar MCP-defined types for execute() and read() methods
Applied to files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/prompt/prompt.types.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/remote-mcp/mcp-client.service.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/sdk/src/common/records/**/*.ts : Centralize record types in common/records directory - import AnyResourceRecord and similar types from common/records, not from module-specific files
Applied to files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/tool/tool.types.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*event*.ts : Use `changeScope` instead of `scope` property name in change event types to avoid confusion with Scope class
Applied to files:
libs/sdk/src/prompt/prompt.events.tslibs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/tool/tool.events.tslibs/sdk/src/tool/tool.registry.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*error*.ts : Use specific MCP error classes with MCP error codes instead of generic errors - define error classes with `mcpErrorCode` property and `toJsonRpcError()` method
Applied to files:
libs/sdk/src/app/instances/app.remote.instance.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/remote-mcp/mcp-client.service.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/app/instances/app.remote.instance.tslibs/sdk/src/tool/tool.registry.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `any` type without justification in TypeScript files
Applied to files:
libs/sdk/src/app/instances/app.remote.instance.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{src}/**/*.{ts,tsx} : Do not use `any` type without justification in TypeScript code
Applied to files:
libs/sdk/src/app/instances/app.remote.instance.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/*.{ts,tsx} : Avoid non-null assertions (!) - use proper error handling and throw specific errors when values are missing instead
Applied to files:
libs/sdk/src/app/instances/app.remote.instance.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*metadata*.ts : Validate URIs per RFC 3986 at metadata level using Zod schema validation with `isValidMcpUri` refinement
Applied to files:
libs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Use proper ES module imports instead of `require()` for SDK imports; avoid dynamic require of `frontmcp/sdk` modules
Applied to files:
libs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/plugins/**/src/**/*.ts : Use module augmentation to extend ExecutionContextBase with new properties - declare module 'frontmcp/sdk' and define interface extensions for plugin context properties
Applied to files:
libs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Use module augmentation for context properties via `declare module 'frontmcp/sdk'` combined with runtime plugin metadata `contextExtensions`, not module-level side effects
Applied to files:
libs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*.ts : Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Applied to files:
libs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Extend tool metadata using `declare global` pattern to allow tools to specify plugin-specific options in their decorators
Applied to files:
libs/sdk/src/tool/tool.types.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/index.{ts,js} : Export all public APIs through appropriate entry points (frontmcp/uipack, frontmcp/uipack/adapters, frontmcp/uipack/theme, etc.)
Applied to files:
libs/sdk/src/tool/tool.types.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/remote-mcp/mcp-client.service.ts
🧬 Code graph analysis (3)
libs/sdk/src/prompt/prompt.registry.ts (4)
libs/sdk/src/common/interfaces/prompt.interface.ts (1)
scope(84-86)libs/sdk/src/scope/scope.instance.ts (2)
apps(325-327)Scope(37-380)libs/sdk/src/prompt/prompt.types.ts (1)
PromptExportOptions(34-39)libs/sdk/src/prompt/prompt.instance.ts (1)
PromptInstance(23-135)
libs/sdk/src/remote-mcp/mcp-client.service.ts (3)
libs/sdk/src/remote-mcp/resilience/retry.ts (3)
RetryOptions(6-21)withRetry(65-106)isTransientError(111-150)libs/sdk/src/remote-mcp/resilience/circuit-breaker.ts (3)
CircuitBreakerOptions(8-19)CircuitBreakerManager(222-268)CircuitBreaker(42-201)libs/sdk/src/remote-mcp/resilience/health-check.ts (4)
HealthCheckOptions(17-30)HealthCheckManager(223-293)HealthStatus(6-6)HealthChecker(44-218)
libs/sdk/src/tool/tool.registry.ts (5)
libs/sdk/src/scope/scope.instance.ts (2)
apps(325-327)Scope(37-380)libs/sdk/src/common/entries/base.entry.ts (1)
EntryLineage(6-6)libs/sdk/src/tool/tool.types.ts (1)
ExportNameOptions(20-25)libs/sdk/src/common/interfaces/index.ts (1)
Token(10-10)libs/sdk/src/tool/tool.instance.ts (2)
getMetadata(99-101)ToolInstance(37-163)
⏰ 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 (14)
libs/sdk/src/prompt/prompt.events.ts (2)
3-3: LGTM! Clean migration to PromptEntry.The update from
PromptInstancetoPromptEntryin both the import and thesnapshotfield aligns with the PR's goal of unifying local and remote prompt representations. ThechangeScopeproperty naming is correctly used as per coding guidelines.Also applies to: 13-13
26-36: Good defensive error handling in emit.The error handling with safe logging specifically addresses the Node.js 24 util.inspect bug with Zod errors, which is a thoughtful implementation detail.
libs/sdk/src/prompt/prompt.types.ts (1)
3-3: LGTM! Clear documentation of the PromptEntry migration.The type update from
PromptInstancetoPromptEntryis well-documented with inline JSDoc explaining that the instance field can now represent either local or remote prompt entries. This aligns with the PR's remote MCP orchestration objectives.Also applies to: 17-18
libs/sdk/src/tool/tool.events.ts (1)
1-1: LGTM! Consistent migration to ToolEntry.The update from
ToolInstancetoToolEntryaligns with the broader migration to unified local/remote tool representations. ThechangeScopeproperty naming is correctly used as per coding guidelines.Also applies to: 19-19
libs/sdk/src/tool/tool.types.ts (1)
1-1: LGTM! Consistent ToolEntry migration with clear documentation.The type update from
ToolInstancetoToolEntrymirrors the pattern used inprompt.types.ts, with clear inline documentation explaining that the instance field can represent either local or remote tool entries.Also applies to: 8-9
libs/sdk/src/common/tokens/app.tokens.ts (1)
30-34: LGTM! New remote app metadata fields added consistently.The five new metadata fields (
namespace,transportOptions,remoteAuth,refreshInterval,cacheTTL) are added using the consistenttokenFactory.meta()pattern and align with the PR's remote MCP server orchestration objectives. These additive changes expand the configuration surface for remote apps without breaking existing functionality.libs/sdk/src/tool/tool.registry.ts (1)
166-189: Excellent resolution of previous issues! LGTM.The critical issues from past reviews have been properly addressed:
✅ Stable token generation (Line 181): Now uses
Symbol.for()with app.id and tool name, ensuring the same symbol is returned across multiple calls. This prevents duplicate entries if capabilities are refreshed.✅ Type safety (Line 182):
remoteToolis passed directly tomakeRowwithout unsafe casting. The signature ofmakeRow(Line 379) has been correctly widened to acceptToolEntryinstead of requiringToolInstance.✅ Error handling: Try-catch block logs errors via scope logger, allowing graceful degradation if remote tool fetching fails.
✅ Validation: Checks for valid tool name before processing each remote tool.
The implementation follows the established pattern and maintains type safety throughout the adoption flow.
libs/sdk/src/prompt/prompt.registry.ts (1)
182-201: Previous critical issues successfully resolved! LGTM.The code properly addresses the concerns from past reviews:
✅ Stable token generation (Line 193): Uses
Symbol.for()with a composite key of app.id and prompt name. This ensures deterministic tokens across registry operations, preventing duplicate entries during capability refreshes.✅ Type safety (Line 194): The
remotePromptis passed directly tomakeRowwithout any unsafe casting. The method signature at Line 412 correctly acceptsPromptEntryas the base type.✅ Error handling (Lines 183, 198-200): Try-catch block with scope-aware error logging provides graceful failure handling.
✅ Validation (Lines 188-190): Validates prompt name before processing, with warning logs for invalid entries.
The implementation is consistent with the tool registry pattern and maintains proper type safety throughout.
libs/sdk/src/common/metadata/app.metadata.ts (2)
346-348: URI validation follows coding guidelines correctly.The implementation properly uses
isValidMcpUrirefinement for URL validation at the metadata schema level, as specified in the coding guidelines for metadata files.Based on learnings: URI validation should use Zod schema refinement with
isValidMcpUriat metadata level per RFC 3986.
145-340: Well-designed remote configuration types.The new remote app configuration structure is comprehensive and well thought out:
✅ Discriminated union pattern (Lines 326-339):
RemoteAuthConfiguses mode-based discrimination ('static' | 'forward' | 'oauth'), enabling type-safe access to mode-specific fields.✅ Resilience controls (Lines 145-175): Transport options include timeout, retries, delay, and SSE fallback—covering common failure scenarios.
✅ Flexible authentication (Lines 180-224): Supports static credentials, token forwarding, and OAuth, accommodating different security models.
✅ Orchestration features (Lines 265-301): Namespace isolation, capability caching, and refresh intervals enable robust multi-server coordination.
✅ Documentation: JSDoc comments clearly explain each option with defaults, making the API self-documenting.
The schemas correctly mirror the TypeScript interfaces, ensuring runtime validation matches compile-time types.
libs/sdk/src/remote-mcp/mcp-client.service.ts (2)
209-223: LGTM - SSE fallback properly implemented.The fallback logic now correctly handles connection failures during
client.connect()rather than during transport construction. The use ofshouldFallbackToSSE()andcreateFallbackSSETransport()provides a clean separation of concerns.
483-489: Authentication design is documented and consistent.The log message correctly explains that auth headers are configured at connection time via
transportOptions.headers(seecreateTransport()at lines 779-782). This connection-level authentication is a valid design choice for remote MCP servers.Note: If per-request authentication is needed in the future, the MCP SDK's client methods would need to support passing headers per-call.
libs/sdk/src/app/instances/app.remote.instance.ts (2)
116-116: LGTM - Capability subscription cleanup properly implemented.The memory leak identified in past reviews has been addressed:
- Unsubscribe function is stored (line 116, line 164)
- Properly cleaned up in
disconnect()(lines 279-282)This prevents memory leaks when remote app instances are disposed or disconnected.
Also applies to: 164-171, 279-282
44-47: LGTM - Proper typing for scope parameter.The
ScopeWithMcpClientinterface provides proper typing for the scope parameter ingetOrCreateMcpClientService(), addressing the past review comment about avoidinganytypes.Also applies to: 309-324
# Conflicts: # plugins/plugin-cache/src/__tests__/cache.plugin.test.ts
…and ensure remote capabilities loading
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/tool/tool.registry.ts (1)
362-370: Potential infinite loop in disambiguate function.Unlike
prompt.registry.tswhich hasMAX_DISAMBIGUATE_ATTEMPTS = 10000(line 28 and used in the disambiguate function), this implementation useswhile (true)without any upper bound. In pathological cases (e.g., maxLen too small to accommodate incrementing suffixes), this could loop indefinitely.🔎 Suggested fix to add max attempts
function disambiguate(candidate: string, pool: Map<string, ToolEntry>, cfg: Required<ExportNameOptions>): string { if (!pool.has(candidate)) return candidate; + const maxAttempts = 10000; let n = 2; - while (true) { + while (n <= maxAttempts) { const withN = ensureMaxLen(`${candidate}${sepFor(cfg.case)}${n}`, cfg.maxLen); if (!pool.has(withN)) return withN; n++; } + throw new Error(`Failed to disambiguate name "${candidate}" after ${maxAttempts} attempts`); }
🤖 Fix all issues with AI Agents
In @libs/sdk/src/auth/flows/session.verify.flow.ts:
- Around line 262-267: The anonymous user object for transparent-anon mode
includes a scope property while the public anonymous user object does not; make
the shapes consistent by adding a scope field to the public-anon user creation
too (e.g., add scope: 'public' or scope: scopes.join(' ') to the user object
created where the public anonymous user is constructed) so downstream code sees
a uniform user shape across both transparent and public anonymous flows.
- Around line 218-292: handleAnonymousFallback duplicates most of
handlePublicMode; extract the shared session creation into a private helper
(e.g., createAnonymousSession) that accepts options { issuer, authSig,
sessionIdHeader?, scopes? } and encapsulates logic using
this.state.sessionIdHeader, getMachineId(), decryptPublicSession, encryptJson,
detectPlatformFromUserAgent, and crypto.randomUUID() to build the user, payload
and session id; then replace the duplicated blocks in both handlePublicMode and
handleAnonymousFallback to call this.createAnonymousSession(...) and pass scopes
from (this.scope.auth?.options as TransparentAuthOptions)?.anonymousScopes when
handling transparent anon.
- Around line 273-280: The payload currently sets isPublic: true for
transparent-anon sessions (in session.verify.flow.ts), which conflates public
and transparent-anonymous modes; modify the payload construction (the object
assigned to payload containing uuid, nodeId, authSig, iat, isPublic, ...) to
distinguish modes explicitly: set isPublic only for true public sessions and add
either an authMode field (e.g. 'public' | 'transparent-anon' | 'anonymous') or a
boolean isAnonymous flag for transparent-anon sessions so downstream checks can
reliably differentiate; update the branch that builds authSig for
transparent-anon to include authMode: 'transparent-anon' (or isAnonymous: true)
and ensure public session construction sets authMode: 'public' (or isAnonymous:
false).
In @libs/sdk/src/transport/adapters/transport.local.adapter.ts:
- Around line 87-96: Replace the hardcoded remoteCapabilities object (symbol:
remoteCapabilities, conditional: hasRemoteApps) with capability retrieval via
getCapabilities() on the appropriate registry/registries; e.g., call
this.scope.remoteApps.getCapabilities() if a remote app registry exists, or
merge this.scope.tools.getCapabilities(),
this.scope.resources.getCapabilities(), and this.scope.prompts.getCapabilities()
into the server options capabilities so transport.local.adapter no longer
hardcodes tools/resources/prompts capabilities but delegates to each registry's
getCapabilities() method.
🧹 Nitpick comments (8)
libs/sdk/src/auth/flows/auth.verify.flow.ts (1)
211-230: Code duplication and inconsistent configuration handling.This method duplicates most of the logic from
handlePublicMode(lines 180-199) with subtle inconsistencies:
- TTL:
handlePublicModeusesthis.parseTtl(authOptions?.['sessionTtl'])allowing configuration, while this method hardcodes3600000.- Issuer:
handlePublicModerespectsauthOptions?.['issuer'], while this always usesbaseUrl.Consider extracting a shared helper to ensure consistent behavior and reduce duplication:
🔎 Proposed refactor
+ /** + * Create anonymous authorization from auth options + */ + private createAnonymousAuthorization(authOptions: Record<string, unknown> | undefined): PublicAuthorization { + const publicAccess = authOptions?.['publicAccess'] as Record<string, unknown> | undefined; + return PublicAuthorization.create({ + scopes: (authOptions?.['anonymousScopes'] as string[] | undefined) ?? ['anonymous'], + ttlMs: this.parseTtl(authOptions?.['sessionTtl'] as string | number | undefined), + issuer: (authOptions?.['issuer'] as string | undefined) ?? this.state.required.baseUrl, + allowedTools: (publicAccess?.['tools'] as string[] | 'all' | undefined) ?? 'all', + allowedPrompts: (publicAccess?.['prompts'] as string[] | 'all' | undefined) ?? 'all', + }); + }Then use in both stages:
async handlePublicMode() { const authOptions = this.scope.auth?.options as Record<string, unknown> | undefined; - const publicAccess = authOptions?.['publicAccess'] as Record<string, unknown> | undefined; - const authorization = PublicAuthorization.create({ - scopes: (authOptions?.['anonymousScopes'] as string[] | undefined) ?? ['anonymous'], - ttlMs: this.parseTtl(authOptions?.['sessionTtl'] as string | number | undefined), - issuer: (authOptions?.['issuer'] as string | undefined) ?? this.state.required.baseUrl, - allowedTools: (publicAccess?.['tools'] as string[] | 'all' | undefined) ?? 'all', - allowedPrompts: (publicAccess?.['prompts'] as string[] | 'all' | undefined) ?? 'all', - }); + const authorization = this.createAnonymousAuthorization(authOptions); // ... } async handleAnonymousFallback() { const authOptions = this.scope.auth?.options as Record<string, unknown> | undefined; - const publicAccess = authOptions?.['publicAccess'] as Record<string, unknown> | undefined; - const authorization = PublicAuthorization.create({ - scopes: (authOptions?.['anonymousScopes'] as string[] | undefined) ?? ['anonymous'], - ttlMs: 3600000, - issuer: this.state.required.baseUrl, - allowedTools: (publicAccess?.['tools'] as string[] | 'all' | undefined) ?? 'all', - allowedPrompts: (publicAccess?.['prompts'] as string[] | 'all' | undefined) ?? 'all', - }); + const authorization = this.createAnonymousAuthorization(authOptions); // ... }libs/sdk/src/tool/tool.utils.ts (1)
89-113: Consider validating all content items, not just the first.The
isCallToolResulthelper only validates the first item in thecontentarray. If subsequent items have invalid types, they'll pass through unchecked. While this is likely sufficient as a heuristic to detect remote tool results (since malformed remote results are the remote server's responsibility), it could cause unexpected behavior if a partially valid result is passed through.If this is intentional for performance or because remote servers are trusted, a clarifying comment would help.
🔎 Optional: Validate all content items
- if (content.length > 0) { - const firstItem = content[0] as Record<string, unknown>; - if (typeof firstItem !== 'object' || firstItem === null) return false; - if (typeof firstItem['type'] !== 'string') return false; - // Valid content types: 'text', 'image', 'audio', 'resource', 'resource_link' - const validTypes = ['text', 'image', 'audio', 'resource', 'resource_link']; - if (!validTypes.includes(firstItem['type'])) return false; - } + const validTypes = ['text', 'image', 'audio', 'resource', 'resource_link']; + for (const item of content) { + if (typeof item !== 'object' || item === null) return false; + const block = item as Record<string, unknown>; + if (typeof block['type'] !== 'string') return false; + if (!validTypes.includes(block['type'])) return false; + }libs/sdk/src/resource/flows/read-resource.flow.ts (1)
113-159: Consider extracting sharedensureRemoteCapabilitieslogic.This method is duplicated across multiple flows (ReadResourceFlow, ResourceTemplatesListFlow, GetPromptFlow, ResourcesListFlow, ToolsListFlow) with nearly identical implementation. Consider extracting this to a shared utility or base class method to reduce duplication.
The implementation itself is correct:
- Parallel execution with
Promise.allfor efficiency- Individual error handling prevents one failing app from blocking others
- Duck typing check is appropriate for the loosely typed app interface
🔎 Example: Extract to FlowBase or utility
// In FlowBase or a shared utility protected async loadRemoteAppCapabilities(): Promise<void> { const appRegistries = this.scope.providers.getRegistries('AppRegistry'); const remoteApps: Array<{ id: string; ensureCapabilitiesLoaded?: () => Promise<void> }> = []; for (const appRegistry of appRegistries) { for (const app of appRegistry.getApps()) { if (app.isRemote) remoteApps.push(app); } } if (remoteApps.length === 0) return; await Promise.all( remoteApps.map(async (app) => { if ('ensureCapabilitiesLoaded' in app && typeof app.ensureCapabilitiesLoaded === 'function') { try { await app.ensureCapabilitiesLoaded(); } catch (error) { this.logger.warn(`Failed to load capabilities for remote app ${app.id}: ${(error as Error).message}`); } } }) ); }libs/sdk/src/prompt/prompt.registry.ts (1)
502-547: Potential memory leak: subscription not cleaned up on registry disposal.The
adoptPromptsFromRemoteAppmethod subscribes to the remote registry (lines 221-224) but doesn't store the unsubscribe function. If this registry is disposed or the remote app disconnects, the subscription remains active, potentially causing:
- Memory leaks from retained closure references
- Unexpected
adoptRemotePrompts()calls after disposalConsider storing unsubscribe functions in a class field and calling them during cleanup.
Additionally, the cast on line 516 (
prompt as PromptInstance) is similar to the previous type safety concern, though the subsequent validation ensures required properties exist.🔎 Suggested approach for subscription cleanup
+ /** Subscriptions to remote app registries for cleanup */ + private remoteSubscriptions = new Map<string, () => void>(); + private adoptPromptsFromRemoteApp(app: AppEntry, scope: Scope): void { // ... existing code ... if (remoteRegistry && typeof remoteRegistry.subscribe === 'function') { - remoteRegistry.subscribe({ immediate: false }, () => { + // Clean up any existing subscription for this app + this.remoteSubscriptions.get(app.id)?.(); + + const unsubscribe = remoteRegistry.subscribe({ immediate: false }, () => { adoptRemotePrompts(); }); + this.remoteSubscriptions.set(app.id, unsubscribe); } }libs/sdk/src/resource/resource.registry.ts (1)
584-629: Validation is thorough but consider unifying with prompt registry.The validation pattern is identical to
registerPromptInstancein prompt.registry.ts. Consider extracting a shared validation helper to reduce duplication across registries.libs/sdk/src/auth/flows/session.verify.flow.ts (1)
295-306: Filter logic correctly coordinates auth modes.The updated filter properly handles all three scenarios:
- Authorization header already present
- Public mode (handled by
handlePublicMode)- Transparent mode with anonymous fallback (handled by
handleAnonymousFallback)Optional: Simplify for readability
Consider extracting the transparent anonymous check into a helper for clarity:
+const isTransparentAnonymous = (authOptions: AuthOptions | undefined): boolean => { + return authOptions !== undefined + && isTransparentMode(authOptions) + && (authOptions as TransparentAuthOptions).allowAnonymous === true; +}; @Stage('requireAuthorizationHeader', { filter: ({ state, scope }) => { if (state.authorizationHeader) return false; const authOptions = scope.auth?.options; if (authOptions && isPublicMode(authOptions)) return false; - if (authOptions && isTransparentMode(authOptions)) { - if ((authOptions as TransparentAuthOptions).allowAnonymous === true) return false; - } + if (isTransparentAnonymous(authOptions)) return false; return true; }, })libs/sdk/src/transport/adapters/transport.local.adapter.ts (1)
70-77: Strengthen type safety for remote app detection.The remote app detection uses runtime type checks (
typeof app === 'object'and'urlType' in app) without proper TypeScript type guards. This approach is not type-safe and could lead to runtime errors if the shape ofscope.metadata.appschanges.Consider defining a type predicate function or using a Zod schema to validate remote app structure, ensuring compile-time type safety and runtime validation.
🔎 Proposed type-safe approach using type predicate
+// Add helper function before connectServer method +private isRemoteApp(app: unknown): app is { urlType: string; standalone?: boolean } { + return ( + app !== null && + typeof app === 'object' && + 'urlType' in app && + typeof (app as { urlType?: unknown }).urlType === 'string' + ); +} + connectServer() { const { info, apps } = this.scope.metadata; - // Check if there are remote apps configured (they will have tools/resources/prompts that load later) - // Remote apps have 'urlType' property indicating they're external MCP servers - const hasRemoteApps = apps?.some( - (app) => - app && typeof app === 'object' && 'urlType' in app && (app as { standalone?: boolean }).standalone !== true, - ); + const hasRemoteApps = apps?.some((app) => this.isRemoteApp(app) && app.standalone !== true);Alternatively, if
AppMetadatatypes are available, use those directly with proper type guards.plugins/plugin-cache/src/cache.plugin.ts (1)
141-165: Consider parameter usage clarity.The
_flowCtxparameter is prefixed with an underscore (indicating it's intentionally unused), but the method doesn't use it—instead accessing context viathis.get(FrontMcpContextStorage). While the bypass header logic itself is correct (lines 158-160), the unused parameter might be confusing.If this parameter is required by the hook signature or for future extensibility, consider adding a comment explaining why it's present but unused. Otherwise, evaluate whether it can be removed for clarity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (23)
apps/e2e/demo-e2e-remote/src/main.tsdocs/draft/docs/plugins/cache-plugin.mdxlibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/flows/call-tool.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/uipack/package.jsonpackage.jsonplugins/plugin-cache/src/__tests__/cache.plugin.test.tsplugins/plugin-cache/src/cache.plugin.tsplugins/plugin-cache/src/cache.types.tsplugins/plugin-codecall/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/tool/flows/call-tool.flow.ts
- apps/e2e/demo-e2e-remote/src/main.ts
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Enable and use strict TypeScript settings - noanytypes without strong justification, useunknownfor generic type defaults instead
Avoid non-null assertions (!) - use proper error handling and throw specific errors when values are missing instead
Always use @frontmcp/utils for cryptographic operations - use hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode instead of node:crypto
Always use @frontmcp/utils for file system operations - use readFile, writeFile, mkdir, rename, unlink, stat, etc. instead of fs/promises or node:fs
Use constrained generic type parameters withRecord<string, string>or similar constraints instead of unconstrainedanydefaults
Files:
libs/sdk/src/resource/flows/read-resource.flow.tsplugins/plugin-cache/src/cache.types.tslibs/sdk/src/auth/flows/session.verify.flow.tsplugins/plugin-cache/src/cache.plugin.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tsplugins/plugin-cache/src/__tests__/cache.plugin.test.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts: MCP response types must use strictly typed MCP protocol definitions, notunknown- usePromise<GetPromptResult>,Promise<ReadResourceResult>, and similar MCP-defined types for execute() and read() methods
Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Files:
libs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
**/libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Create shared base classes for common functionality - use ExecutionContextBase as parent class for ToolContext, ResourceContext, and similar context classes
Files:
libs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Files:
libs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Document known limitations clearly in code comments and README files
Files:
libs/sdk/src/resource/flows/read-resource.flow.tsplugins/plugin-cache/src/cache.types.tslibs/sdk/src/auth/flows/session.verify.flow.tsplugins/plugin-cache/src/cache.plugin.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tsplugins/plugin-cache/src/__tests__/cache.plugin.test.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
**/libs/sdk/src/**/*flow*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Do not mutate rawInput in flows - use state.set() for managing flow state instead of modifying input parameters
Files:
libs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.ts
**/libs/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add backwards compatibility exports in new libraries - avoid legacy prefixes like PT-001 in test names
Files:
libs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.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/resource/flows/read-resource.flow.tslibs/uipack/package.jsonlibs/sdk/src/auth/flows/session.verify.flow.tslibs/sdk/src/tool/tool.instance.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/tool/tool.utils.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
libs/uipack/**/{package.json,*.ts,*.tsx,*.js,*.jsx}
📄 CodeRabbit inference engine (libs/uipack/CLAUDE.md)
Do not add React dependencies to @frontmcp/uipack - it must remain React-free. Use @frontmcp/ui for React components.
Files:
libs/uipack/package.json
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/plugins/cache-plugin.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/plugins/cache-plugin.mdx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Test all code paths including error conditions - include constructor validation tests, error class instanceof checks, and edge cases
Files:
plugins/plugin-cache/src/__tests__/cache.plugin.test.ts
🧠 Learnings (26)
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters}/**/src/**/*.ts : Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Applied to files:
libs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/transport/adapters/transport.local.adapter.tslibs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/prompt/flows/get-prompt.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Extend tool metadata using `declare global` pattern to allow tools to specify plugin-specific options in their decorators
Applied to files:
plugins/plugin-cache/src/cache.types.tsplugins/plugin-cache/src/cache.plugin.tsplugins/plugin-cache/src/__tests__/cache.plugin.test.tslibs/sdk/src/tool/tool.utils.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : RememberPlugin scopes include: `session` (default, current session only), `user` (persists across sessions), `tool` (tied to specific tool + session), and `global` (shared across all sessions/users)
Applied to files:
plugins/plugin-cache/src/cache.types.ts
📚 Learning: 2026-01-04T14:35:18.353Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2026-01-04T14:35:18.353Z
Learning: Applies to libs/uipack/**/{package.json,*.ts,*.tsx,*.js,*.jsx} : Do not add React dependencies to frontmcp/uipack - it must remain React-free. Use frontmcp/ui for React components.
Applied to files:
libs/uipack/package.json
📚 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/package.json : The frontmcp/ui package requires React as a peer dependency (^18.0.0 || ^19.0.0)
Applied to files:
libs/uipack/package.json
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Use proper ES module imports instead of `require()` for SDK imports; avoid dynamic require of `frontmcp/sdk` modules
Applied to files:
libs/uipack/package.json
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Always use `frontmcp/utils` for cryptographic operations (hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode) instead of `node:crypto`
Applied to files:
libs/uipack/package.json
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/*.{ts,tsx} : Always use frontmcp/utils for cryptographic operations - use hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode instead of node:crypto
Applied to files:
libs/uipack/package.json
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.ts : Avoid using `node:crypto` directly; always use `frontmcp/utils` for cross-platform cryptographic support
Applied to files:
libs/uipack/package.json
📚 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/package.json : Entry points must match the documented paths: frontmcp/ui/react, frontmcp/ui/renderers, frontmcp/ui/render, frontmcp/ui/universal, frontmcp/ui/bundler, frontmcp/ui/bridge, frontmcp/ui/components, frontmcp/ui/layouts, frontmcp/ui/web-components
Applied to files:
libs/uipack/package.json
📚 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/bundler/**/*.{ts,tsx} : The bundler module must re-export utilities from frontmcp/uipack/bundler and provide SSR component bundling functionality
Applied to files:
libs/uipack/package.json
📚 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/**/*.{ts,tsx} : Never import React-free utilities from frontmcp/ui; use frontmcp/uipack for bundling, build tools, platform adapters, and theme utilities
Applied to files:
libs/uipack/package.json
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/*.{ts,tsx} : Always use frontmcp/utils for file system operations - use readFile, writeFile, mkdir, rename, unlink, stat, etc. instead of fs/promises or node:fs
Applied to files:
libs/uipack/package.json
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Use module augmentation for context properties via `declare module 'frontmcp/sdk'` combined with runtime plugin metadata `contextExtensions`, not module-level side effects
Applied to files:
plugins/plugin-cache/src/cache.plugin.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/plugins/**/src/**/*.ts : Use module augmentation to extend ExecutionContextBase with new properties - declare module 'frontmcp/sdk' and define interface extensions for plugin context properties
Applied to files:
plugins/plugin-cache/src/cache.plugin.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Extend ExecutionContextBase with plugin-specific properties using module declaration (`declare module 'frontmcp/sdk'`) combined with `contextExtensions` in plugin metadata
Applied to files:
plugins/plugin-cache/src/cache.plugin.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*.ts : Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Applied to files:
libs/sdk/src/resource/flows/resource-templates-list.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/auth/flows/auth.verify.flow.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*plugin.ts : Plugins should extend `DynamicPlugin<Options, OptionsInput>` for configurable behavior, with `Plugin` decorator specifying name, description, and static providers
Applied to files:
plugins/plugin-cache/src/__tests__/cache.plugin.test.ts
📚 Learning: 2026-01-06T02:34:55.680Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/plugins/CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:55.680Z
Learning: Applies to libs/plugins/**/*.test.ts : Tests must achieve 95%+ code coverage across all metrics; use `MockStore` implementing `RememberStoreInterface` for RememberPlugin testing
Applied to files:
plugins/plugin-cache/src/__tests__/cache.plugin.test.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Test all code paths including error conditions - include constructor validation tests, error class instanceof checks, and edge cases
Applied to files:
plugins/plugin-cache/src/__tests__/cache.plugin.test.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/sdk/src/**/*flow*.ts : Do not mutate rawInput in flows - use state.set() for managing flow state instead of modifying input parameters
Applied to files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/flows/tools-list.flow.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/auth/flows/auth.verify.flow.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*.ts : MCP response types must use strictly typed MCP protocol definitions, not `unknown` - use `Promise<GetPromptResult>`, `Promise<ReadResourceResult>`, and similar MCP-defined types for execute() and read() methods
Applied to files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/sdk/src/common/records/**/*.ts : Centralize record types in common/records directory - import AnyResourceRecord and similar types from common/records, not from module-specific files
Applied to files:
libs/sdk/src/prompt/prompt.registry.tslibs/sdk/src/resource/resource.registry.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*event*.ts : Use `changeScope` instead of `scope` property name in change event types to avoid confusion with Scope class
Applied to files:
libs/sdk/src/resource/resource.registry.tslibs/sdk/src/tool/tool.registry.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/tool/tool.registry.ts
🧬 Code graph analysis (7)
libs/sdk/src/tool/tool.instance.ts (2)
libs/sdk/src/common/interfaces/tool.interface.ts (2)
input(68-70)input(72-79)libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (1)
inputSchema(12-17)
libs/sdk/src/transport/adapters/transport.local.adapter.ts (2)
libs/sdk/src/scope/scope.instance.ts (1)
apps(329-331)libs/sdk/src/agent/agent.scope.ts (2)
apps(245-247)apps(322-324)
libs/sdk/src/resource/flows/resource-templates-list.flow.ts (3)
libs/sdk/src/agent/hooks/agent.hooks.ts (1)
Stage(54-54)libs/sdk/src/scope/scope.instance.ts (1)
apps(329-331)libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
error(188-190)
libs/sdk/src/prompt/flows/get-prompt.flow.ts (3)
libs/sdk/src/agent/hooks/agent.hooks.ts (1)
Stage(54-54)libs/sdk/src/scope/scope.instance.ts (1)
apps(329-331)libs/sdk/src/common/interfaces/prompt.interface.ts (1)
error(118-120)
libs/sdk/src/resource/flows/resources-list.flow.ts (4)
libs/sdk/src/agent/hooks/agent.hooks.ts (1)
Stage(54-54)libs/sdk/src/scope/scope.instance.ts (2)
apps(329-331)resources(341-343)libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
error(188-190)libs/sdk/src/app/instances/app.local.instance.ts (1)
resources(85-87)
libs/sdk/src/auth/flows/auth.verify.flow.ts (1)
libs/sdk/src/agent/hooks/agent.hooks.ts (1)
Stage(54-54)
libs/sdk/src/tool/tool.utils.ts (1)
libs/sdk/src/common/entries/tool.entry.ts (1)
ParsedToolResult(28-28)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (46)
libs/sdk/src/auth/flows/auth.verify.flow.ts (3)
76-85: LGTM!The flow plan ordering is logical:
handleAnonymousFallbackis correctly positioned afterhandlePublicModeto handle the transparent mode fallback case beforerequireAuthorizationHeaderenforces the auth header requirement.
235-243: LGTM!The filter logic correctly handles all cases:
- Skips for public mode (handled by
handlePublicMode)- Skips when auth header exists (will proceed to
verifyToken)- Skips for transparent mode with
allowAnonymous(handled byhandleAnonymousFallback)The comment on line 239 accurately documents the relationship with
handleAnonymousFallback.
204-210: LGTM!The stage filter correctly gates
handleAnonymousFallbackto only execute when:
- In transparent mode
- No token is provided
allowAnonymousis explicitly enabledThis ensures the fallback only triggers under the intended conditions.
libs/sdk/src/tool/tool.instance.ts (2)
138-141: LGTM! Good exhaustive switch handling.Adding a default case that throws for unknown
ToolKindvalues ensures the switch is exhaustive and will fail fast if a new kind is added without handling it here.
144-158: LGTM! Passthrough for remote tools is appropriate.Using
.passthrough()for remote tools allows arguments to flow through without stripping unknown keys, since validation happens on the remote server. The annotation checkmetadata.annotations?.['frontmcp:remote'] === trueis correctly guarded with optional chaining.One minor observation: the
inputSchemavariable on line 151 shadows the class propertythis.inputSchemareferenced on the same line. While functionally correct, consider using a different variable name likepassthroughSchemafor clarity.libs/sdk/src/tool/tool.utils.ts (2)
82-87: LGTM! Switch relies on TypeScript exhaustiveness.The switch implicitly returns
undefinedfor any unhandled cases, which TypeScript will catch ifToolKindis extended without updating this function (assuming strict return type checking).
115-120: LGTM! Early return prevents double-wrapping.Short-circuiting when
rawis already a validCallToolResultprevents MCP-formatted responses from being wrapped again. This is correct behavior for remote tool results.libs/sdk/src/resource/flows/read-resource.flow.ts (1)
51-51: LGTM! Stage ordering is correct.Adding
ensureRemoteCapabilitiesbeforefindResourceensures remote app capabilities are loaded before resource lookup.libs/sdk/src/resource/flows/resource-templates-list.flow.ts (1)
40-40: LGTM! Stage added to pre phase.Remote capabilities are loaded before
findTemplates.libs/sdk/src/prompt/flows/get-prompt.flow.ts (1)
45-45: LGTM! Stage added beforefindPrompt.Ensures remote capabilities are available for prompt lookup.
libs/sdk/src/resource/flows/resources-list.flow.ts (2)
36-36: LGTM! Pre phase updated correctly.
149-167: LGTM! Resource deduplication by URI.Deduplicating resources by their URI (with fallback to
metadata.name) prevents duplicate entries when the same resource is registered from multiple sources (e.g., local and remote). The logging clearly shows the deduplication effect.libs/sdk/src/tool/flows/tools-list.flow.ts (2)
47-47: LGTM! Pre phase updated correctly.
182-196: LGTM! Tool deduplication logic.Deduplicating tools by
fullName || metadata.id || metadata.nameis appropriate and matches the key hierarchy used elsewhere in tool resolution. The logging clearly indicates deduplication occurred.libs/sdk/src/prompt/flows/prompts-list.flow.ts (3)
37-41: LGTM! Flow plan extended with remote capabilities stage.The
ensureRemoteCapabilitiesstage is correctly added to theprearray, ensuring remote app capabilities are loaded before prompt discovery begins.
97-143: Well-structured remote capability loading with proper error isolation.The implementation correctly:
- Scans all app registries to find remote apps
- Uses parallel execution with
Promise.allfor efficiency- Isolates failures per-app to prevent one failing remote from blocking others
- Uses duck-typing check for
ensureCapabilitiesLoadedmethod presenceOne minor observation: the type annotation on line 109 uses an inline object type with optional
ensureCapabilitiesLoaded. Consider extracting this to a named type or interface for better readability if this pattern is reused elsewhere.
145-177: Deduplication logic is sound but consider edge case.The deduplication using
fullName || metadata.nameworks well for preventing duplicates. However, if two different prompts legitimately have the samemetadata.namebut differentfullNamevalues (both truthy), they would be correctly distinguished. But if one hasfullNameand another has onlymetadata.namematching the first'sfullName, they'd be treated as duplicates.Given the logging at line 166 shows the deduplication count, this is likely intentional behavior for the remote MCP scenario where the same prompt may appear via multiple paths.
libs/sdk/src/prompt/prompt.registry.ts (5)
118-131: LGTM! Clear separation of remote vs local app adoption.The initialization flow now properly distinguishes between remote and local apps using the
isRemoteflag, addressing previous feedback about fragile duck-typing. Each path has its own dedicated adoption method.
178-226: Good implementation addressing past review concerns.This implementation correctly addresses the issues previously raised:
- ✅ Uses
Symbol.for()for stable, deterministic tokens (line 205)- ✅ Cleans up stale entries before re-adoption (lines 190-194)
- ✅ Validates remote prompt structure before processing
- ✅ Has try-catch with proper error logging
- ✅ Subscribes to remote registry for lazy-loaded prompts
The closure-based
adoptRemotePromptshelper enables clean re-adoption on subscription updates.
228-239: LGTM! Local app adoption delegates to existing mechanism.The local adoption path correctly filters by owner kind and delegates to the existing
adoptFromChildmethod, maintaining consistency with the hierarchical registry pattern.
437-452: Good type widening to PromptEntry.The
makeRowsignature now acceptsPromptEntryinstead ofPromptInstance, which correctly supports both localPromptInstanceand remoteProxyPromptEntrytypes. This addresses the type safety concerns from past reviews.
475-495: LGTM! Provider ID extraction handles PromptEntry safely.The method now accepts
PromptEntryand uses defensive access patterns with proper type guards. The cast throughunknownis appropriate for accessing potentially-dynamic metadata properties.libs/sdk/src/resource/resource.registry.ts (3)
128-141: LGTM! Consistent remote/local separation pattern.The initialization flow correctly separates remote and local app adoption, consistent with the prompt registry implementation.
246-257: LGTM! Local adoption delegates to existing mechanism.Consistent with the prompt registry pattern.
491-514: LGTM! makeRow correctly accepts ResourceEntry.The signature change allows both
ResourceInstanceandProxyResourceEntryto be used, addressing the past review concern about type safety.libs/sdk/src/tool/tool.registry.ts (4)
109-116: Good fix for duck-typing concern.The code now uses the explicit
isRemoteflag instead of checking forgetMcpClientmethod presence, addressing the previous review feedback about fragile duck-typing.
217-228: LGTM! Consistent local adoption pattern.Matches the pattern established in prompt and resource registries.
405-411: LGTM! makeRow correctly accepts ToolEntry.This addresses the past review concern about type safety. Both
ToolInstanceandProxyToolEntrycan now be passed without unsafe casting.
434-453: LGTM! Provider ID extraction updated for ToolEntry.The method now accepts
ToolEntryand uses defensive patterns consistent with the other registries.libs/sdk/src/auth/flows/session.verify.flow.ts (1)
61-61: LGTM: Stage ordering is correct.The new
handleAnonymousFallbackstage is correctly positioned afterhandlePublicModeand beforerequireAuthorizationHeader, ensuring proper precedence for different auth modes.libs/sdk/src/transport/adapters/transport.local.adapter.ts (3)
81-83: LGTM - Capability flags correctly include remote apps.The logic to enable prompts, resources, and tools capabilities when remote apps are present is correct. This ensures the MCP server advertises these capabilities even before remote app entries are fully loaded.
117-117: LGTM - Helpful observability enhancement.Adding
hasRemoteAppsto the logging output improves debugging and monitoring of remote app integration.
100-100: The capability spread order and override behavior is correct and intentional. Remote capabilities are pre-advertised as defaults when remote apps exist, and local registries either match or supersede these values with their own capability flags. No data loss occurs since local registries return the same capability structures when items are registered.plugins/plugin-cache/src/cache.types.ts (1)
23-60: Excellent documentation and API design.The new optional properties (
defaultTTL,toolPatterns,bypassHeader) are well-documented with comprehensive JSDoc, clear examples, and maintain backward compatibility. The distinction between tool-levelttl(inCachePluginToolOptions) and globaldefaultTTLis properly established.The security note about the
x-frontmcp-*prefix requirement for headers is a good practice that helps ensure headers are properly captured.plugins/plugin-cache/src/__tests__/cache.plugin.test.ts (3)
9-17: Good type safety improvements.The helper types
ValueProviderandFactoryProviderproperly formalize the provider shapes returned bydynamicProviders, improving type safety throughout the test suite while maintaining compatibility with the genericProviderFactoryTypereturn type.
259-273: LGTM - Constructor tests for new options.The tests properly verify that
toolPatternsandbypassHeaderoptions are accepted and stored correctly in the plugin configuration.
276-368: Excellent test coverage for pattern matching.The
isCacheabletest suite is comprehensive and covers all essential scenarios:
- No patterns configured
- Exact tool name matches
- Various wildcard patterns (namespace, prefix, suffix, middle)
- Multiple pattern combinations
- Special regex character escaping
This provides strong confidence in the pattern matching implementation.
libs/uipack/package.json (1)
62-62: Dependency is valid and secure.Version 2.3.0 of enclave-vm is available on npm (confirmed in yarn.lock) and is actively used throughout the codebase. No known security vulnerabilities were found in public CVE databases.
docs/draft/docs/plugins/cache-plugin.mdx (3)
210-220: LGTM! Configuration parameters are well-documented.The
toolPatternsandbypassHeaderconfiguration options are clearly documented with appropriate examples and defaults that match the implementation. The bypass header correctly uses the requiredx-frontmcp-*prefix.
274-340: Excellent guidance for remote tool caching.This section provides comprehensive coverage of pattern-based caching for remote tools, including clear examples, pattern syntax reference, and priority rules. The guidance aligns with the implementation and addresses the use case where tool metadata cannot be directly modified.
As per coding guidelines, this properly documents the remote integration patterns for the cache plugin.
343-364: Clear bypass documentation with correct examples.The cache bypass section provides practical examples and properly uses the
x-frontmcp-disable-cacheheader name. The custom header configuration example is clear and the use case guidance is helpful.plugins/plugin-cache/src/cache.plugin.ts (5)
19-23: LGTM! Well-documented default header.The default bypass header constant is clearly defined with a helpful comment explaining the prefix requirement for header extraction.
126-139: Correct priority logic for cache eligibility.The
shouldCacheToolmethod properly implements the documented priority rules: tool metadata takes precedence (line 134), then falls back to pattern matching (lines 137-138). This matches the behavior described in the documentation.
167-175: LGTM! Clean TTL resolution logic.The
getTtlmethod correctly implements the priority chain: tool-specific TTL (lines 171-173) takes precedence over the plugin default (line 174). The fallback to 86400 seconds matches the documented default.
177-249: Past type safety issue is properly resolved.The updated cache hit handling (lines 217-237) now correctly addresses the type safety concern from the previous review:
- Line 221 checks if the cached value is a plain object before attempting to augment it with
_meta- Lines 224-233 only spread object properties when safe to do so
- Lines 235-236 return primitives and arrays as-is without modification
Additionally, the dual checks for
tool.fullNameandtool.name(line 192) properly support pattern matching for both namespaced and non-namespaced tool identifiers.
25-42: Pattern matching logic is correct and well-tested.The
matchesToolPatternfunction properly handles both exact matches and glob-style wildcard patterns. The regex escaping correctly preserves special characters, and the wildcard conversion to.*is properly implemented with anchoring. The test suite comprehensively validates all documented pattern types: namespace wildcards (mintlify:*), prefix patterns (api:get-*), suffix patterns (*-readonly), middle patterns (api:*:list), and special character escaping (api.v1:get-users,api[2]:*). No changes needed.
…red logic for public and transparent modes
…pre-advertised capabilities
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/sdk/src/tool/tool.registry.ts (1)
489-523: Unsafe cast and type mismatch in validation logic.Three issues:
Line 492: Casts
tool: ToolEntrytoToolInstancewithout validation. If aProxyToolEntryor otherToolEntrysubclass is passed (which the method signature allows), this cast is unsafe and bypasses TypeScript's type checking.Lines 495-503: The validation accesses
instance.recordassumingToolInstancestructure, butToolEntrymay not guarantee this property exists. This creates a runtime risk if non-ToolInstanceentries are passed.Type signature vs. implementation mismatch: The signature accepts
ToolEntrybut the implementation assumesToolInstance. This violates the Liskov Substitution Principle.🔎 Recommended fix: narrow the parameter type or use type guards
Option 1 (preferred): Narrow the parameter type to only accept valid subclasses
- registerToolInstance(tool: ToolEntry): void { + registerToolInstance(tool: ToolInstance | RemoteToolInstance): void { // Validate that we have a proper instance with required properties - // Accept both ToolInstance and RemoteToolInstance (or any ToolEntry subclass with valid record) - const instance = tool as ToolInstance; + const instance = tool;Option 2: Add a type guard to validate before casting
registerToolInstance(tool: ToolEntry): void { + // Type guard to check if tool is a registerable instance + if (!isRegisterableToolInstance(tool)) { + throw new Error('Tool must be a ToolInstance or RemoteToolInstance with required properties'); + } const instance = tool as ToolInstance; + function isRegisterableToolInstance(t: ToolEntry): t is ToolInstance { + const candidate = t as ToolInstance; + return !!(candidate.record?.provide && candidate.record?.kind && candidate.record?.metadata); + }
🤖 Fix all issues with AI Agents
In @libs/sdk/src/tool/tool.registry.ts:
- Line 172: The direct cast const remoteRegistry = app.tools as ToolRegistry is
unsafe; add a runtime validation before using it: check that app.tools is
non-null and implements the ToolRegistry contract (e.g., has the expected
methods/properties used later such as registerTool, getTool, getAllTools or
whichever ToolRegistry-specific methods the code calls) and only then assign to
remoteRegistry or wrap it; if validation fails either adapt to the minimal
interface, create an adapter that implements ToolRegistry, or throw a
descriptive error. Ensure you update usages that assume ToolRegistry to handle
the validated/adapter type rather than relying on a blind cast.
🧹 Nitpick comments (1)
libs/sdk/src/tool/tool.registry.ts (1)
210-214: Duck-typing check forsubscribemethod.Checking
typeof remoteRegistry.subscribe === 'function'is defensive but fragile. If the object has asubscribeproperty that isn't callable or has a different signature, this could fail silently or cause runtime errors.Consider using a type guard or interface check to validate the registry implements the expected subscription pattern, or document that
ToolRegistryInterfaceguarantees thesubscribemethod.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
libs/sdk/src/auth/flows/auth.verify.flow.tslibs/sdk/src/tool/tool.registry.tslibs/sdk/src/transport/adapters/transport.local.adapter.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/transport/adapters/transport.local.adapter.ts
- libs/sdk/src/auth/flows/auth.verify.flow.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Enable and use strict TypeScript settings - noanytypes without strong justification, useunknownfor generic type defaults instead
Avoid non-null assertions (!) - use proper error handling and throw specific errors when values are missing instead
Always use @frontmcp/utils for cryptographic operations - use hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode instead of node:crypto
Always use @frontmcp/utils for file system operations - use readFile, writeFile, mkdir, rename, unlink, stat, etc. instead of fs/promises or node:fs
Use constrained generic type parameters withRecord<string, string>or similar constraints instead of unconstrainedanydefaults
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts: MCP response types must use strictly typed MCP protocol definitions, notunknown- usePromise<GetPromptResult>,Promise<ReadResourceResult>, and similar MCP-defined types for execute() and read() methods
Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Create shared base classes for common functionality - use ExecutionContextBase as parent class for ToolContext, ResourceContext, and similar context classes
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Files:
libs/sdk/src/tool/tool.registry.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Document known limitations clearly in code comments and README files
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add backwards compatibility exports in new libraries - avoid legacy prefixes like PT-001 in test names
Files:
libs/sdk/src/tool/tool.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/tool/tool.registry.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*error*.ts : Use specific MCP error classes with MCP error codes instead of generic errors - define error classes with `mcpErrorCode` property and `toJsonRpcError()` method
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*event*.ts : Use `changeScope` instead of `scope` property name in change event types to avoid confusion with Scope class
Applied to files:
libs/sdk/src/tool/tool.registry.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters}/**/src/**/*.ts : Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Applied to files:
libs/sdk/src/tool/tool.registry.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/tool/tool.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). (2)
- GitHub Check: Lint & Format Checks
- GitHub Check: Build Libraries
🔇 Additional comments (5)
libs/sdk/src/tool/tool.registry.ts (5)
109-116: Good improvement: usingapp.isRemoteinstead of duck-typing.The conditional logic now uses a proper property check instead of the fragile
getMcpClientmethod detection from earlier iterations. This is a more robust approach for discriminating remote vs. local apps.
282-289: Excellent: consistent ToolEntry widening across the API.The systematic change from
ToolInstancetoToolEntryacross return types and parameters (listAllInstances,listByOwner,exportResolvedNames,getExported,subscribe,makeRow) enables the registry to handle both local instances and remote proxy entries uniformly. This resolves the type cast issues flagged in previous reviews.Also applies to: 300-300, 376-376, 384-384, 407-407
364-371: Good defensive practice: disambiguation loop guard.Adding a
maxAttemptsguard prevents infinite loops during export name disambiguation. The 10,000 iteration limit is reasonable for practical scenarios.
436-455: LGTM: safe type casting with proper guards.The
providerIdOfimplementation correctly:
- Casts through
unknownbefore narrowing toRecord<string, unknown>- Validates types before property access
- Uses try-catch for defensive error handling
This follows the coding guideline: "Enable and use strict TypeScript settings."
193-193: Good fix: stable symbols with Symbol.for().Using
Symbol.for()with a composite key (tool:${app.id}:${remoteTool.name}) ensures stable, reusable symbols across registry operations and capability refreshes. This resolves the symbol duplication issue flagged in previous reviews.
…y incorporating owner identifiers
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @libs/sdk/src/tool/tool.registry.ts:
- Around line 299-304: Widen the public API types from ToolInstance to the
broader ToolEntry to support ProxyToolEntry and avoid unsafe casts: change the
return/type annotations for listAllInstances, listByOwner, exportResolvedNames,
getExported, subscribe, and makeRow to use ToolEntry (and readonly ToolEntry[]
where applicable), update any internal mappings (e.g., listAllIndexed().map(...)
usage) and call sites to treat results as ToolEntry (handling instance-specific
access via its instance property when needed), and remove previous casts to
ToolInstance so implementations like ProxyToolEntry are accepted.
- Around line 197-200: The current cleanup in adoptRemoteTools relies on
symbol.description string matching which is fragile; introduce a dedicated
Map<string, IndexedTool[]> named remoteAppTools as a class field to track rows
added per remote app, then in adoptRemoteTools use remoteAppTools.get(app.id) to
obtain previousTools and remove them from this.localRows (filtering out
previousTools.includes(row)); when creating new rows with makeRow and pushing
into this.localRows, also collect them into newRows and call
remoteAppTools.set(app.id, newRows). Ensure you reference localRows,
adoptRemoteTools, makeRow, and remoteAppTools when implementing these changes.
🧹 Nitpick comments (2)
libs/sdk/src/resource/flows/resources-list.flow.ts (1)
74-75: Avoidanytype forparams.Per coding guidelines, prefer
unknownoveranywhen the type is not fully known. Theparamsvariable can be typed more safely.🔎 Suggested fix
let method!: string; - let params: any; + let params: unknown;Then access
cursorwith proper type narrowing:- const cursor = params?.cursor; + const cursor = (params as { cursor?: string } | undefined)?.cursor;libs/sdk/src/tool/tool.registry.ts (1)
170-175: Consider using interface marker instead of duck-typing.While the duck-typing check for the
subscribemethod has safe fallback behavior (if not present, subscription is skipped), a more robust approach would be to use an interface marker property (similar toapp.isRemote) to explicitly indicate remote registry capabilities.This is optional since the current implementation safely handles cases where
subscribeis absent or has an unexpected signature.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/sdk/src/prompt/flows/prompts-list.flow.tslibs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/flows/tools-list.flow.tslibs/sdk/src/tool/tool.registry.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/prompt/flows/prompts-list.flow.ts
- libs/sdk/src/tool/flows/tools-list.flow.ts
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Enable and use strict TypeScript settings - noanytypes without strong justification, useunknownfor generic type defaults instead
Avoid non-null assertions (!) - use proper error handling and throw specific errors when values are missing instead
Always use @frontmcp/utils for cryptographic operations - use hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode instead of node:crypto
Always use @frontmcp/utils for file system operations - use readFile, writeFile, mkdir, rename, unlink, stat, etc. instead of fs/promises or node:fs
Use constrained generic type parameters withRecord<string, string>or similar constraints instead of unconstrainedanydefaults
Files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts: MCP response types must use strictly typed MCP protocol definitions, notunknown- usePromise<GetPromptResult>,Promise<ReadResourceResult>, and similar MCP-defined types for execute() and read() methods
Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
**/libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Create shared base classes for common functionality - use ExecutionContextBase as parent class for ToolContext, ResourceContext, and similar context classes
Files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Document known limitations clearly in code comments and README files
Files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
**/libs/sdk/src/**/*flow*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Do not mutate rawInput in flows - use state.set() for managing flow state instead of modifying input parameters
Files:
libs/sdk/src/resource/flows/resources-list.flow.ts
**/libs/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add backwards compatibility exports in new libraries - avoid legacy prefixes like PT-001 in test names
Files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.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/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*error*.ts : Use specific MCP error classes with MCP error codes instead of generic errors - define error classes with `mcpErrorCode` property and `toJsonRpcError()` method
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters}/**/src/**/*.ts : Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Applied to files:
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*.ts : Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Applied to files:
libs/sdk/src/resource/flows/resources-list.flow.ts
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*event*.ts : Use `changeScope` instead of `scope` property name in change event types to avoid confusion with Scope class
Applied to files:
libs/sdk/src/tool/tool.registry.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/tool/tool.registry.ts
🧬 Code graph analysis (1)
libs/sdk/src/resource/flows/resources-list.flow.ts (4)
libs/sdk/src/agent/hooks/agent.hooks.ts (1)
Stage(54-54)libs/sdk/src/scope/scope.instance.ts (2)
apps(329-331)resources(341-343)libs/sdk/src/common/interfaces/execution-context.interface.ts (1)
error(188-190)libs/sdk/src/app/instances/app.local.instance.ts (1)
resources(85-87)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (10)
libs/sdk/src/resource/flows/resources-list.flow.ts (3)
128-137: Verify silent error handling is intentional for resilience.Errors during capability loading are logged as warnings and swallowed. This provides resilience (partial failures don't block the entire list operation), but could mask systematic issues with remote server connectivity.
Consider whether aggregate failure tracking would be helpful (e.g., emit a single warning if all remote apps failed to load, vs. some succeeding).
149-168: LGTM on deduplication logic.The composite key
${resource.owner.id}:${resource.uri || resource.metadata.name}correctly handles:
- Same resource registered multiple times by the same owner (deduped)
- Different owners with resources of the same name (preserved for conflict resolution)
- Resources without URI (falls back to name)
The logging clearly reflects the deduplication outcome.
95-141: New stage integrates well with the flow pattern.The
ensureRemoteCapabilitiesmethod correctly:
- Discovers remote apps across all AppRegistry instances
- Loads capabilities in parallel with
Promise.all- Uses state management properly (no rawInput mutation)
- Provides clear logging for debugging
libs/sdk/src/tool/tool.registry.ts (7)
15-16: LGTM! Necessary imports for remote app support.The new imports for
ScopeandAppEntryenable the enhanced adoption logic that distinguishes remote from local apps.
105-116: Excellent improvement: metadata-based branching replaces duck-typing.The use of
app.isRemoteto distinguish remote from local apps is a robust replacement for the previous duck-typedgetMcpClientcheck. The clean separation of adoption paths improves maintainability.
210-211: Excellent fix: stable symbols and type-safe row creation.The use of
Symbol.for()creates stable, deterministic tokens that resolve the previous symbol duplication issue. Combined with the type widening ofmakeRowto acceptToolEntry, this eliminates the unsafe type cast that was previously flagged.
234-245: LGTM! Clean delegation to existing adoption mechanism.The local app adoption correctly filters for app-owned registries and delegates to the existing
adoptFromChildmethod, maintaining consistency with the established pattern.
379-389: Good defensive programming: bounded disambiguation loop.Adding a
maxAttemptslimit (10,000) with an explicit error when exceeded prevents potential infinite loops while providing clear diagnostics if an unexpected naming collision occurs.
483-499: Good defensive validation with type narrowing.The
isRegisterableTooltype guard validates that tool entries have the required properties (name,record.provide,record.kind,record.metadata) before registration. This provides runtime safety while narrowing the type for subsequent use.
529-566: Enhanced validation with detailed error diagnostics.The improved
registerToolInstancemethod validates tool structure using the type guard and provides specific error messages for each validation failure. This significantly improves debuggability when registering remote or agent-scoped tools.
…Map for efficient cleanup and tracking
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
libs/sdk/src/tool/tool.registry.ts (2)
185-242: Excellent improvements—validation and error handling are now robust.The method now includes:
- ✅ Validation of
app.toolsinterface before use- ✅ Error handling with logging
- ✅ Stable symbols using
Symbol.for()- ✅ Efficient cleanup via
remoteAppToolsMap- ✅ Conditional subscription with type guard
This addresses all critical concerns from previous reviews.
Optional: Validate getTools() return type
For additional robustness, validate that
getTools()returns an array:try { - const remoteTools = app.tools.getTools(); + const remoteTools = app.tools.getTools(); + if (!Array.isArray(remoteTools)) { + scope.logger.warn(`Remote app ${app.id} getTools() did not return an array`); + return; + } const newRows: IndexedTool[] = [];
309-424: Essential refactor—type widening enables remote tool support.The systematic widening of types from
ToolInstancetoToolEntryacross the public API is essential for supportingProxyToolEntryand remote tool instances. This eliminates unsafe type casts and provides a flexible, type-safe foundation for both local and remote tools.Methods updated:
listAllInstances,listByOwner,exportResolvedNames,getExported,subscribe, and internal helpers.Optional: Improve disambiguate error message
The error on line 398 could be more actionable:
- throw new Error(`Failed to disambiguate name "${candidate}" after ${maxAttempts} attempts`); + throw new Error(`Failed to disambiguate tool name "${candidate}" after ${maxAttempts} attempts. This indicates too many tools with similar names—consider adjusting naming or maxLen configuration.`);Also applies to: 327-327, 346-346, 389-399, 403-403, 411-411
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/sdk/src/resource/flows/resources-list.flow.tslibs/sdk/src/tool/tool.registry.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/sdk/src/resource/flows/resources-list.flow.ts
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Enable and use strict TypeScript settings - noanytypes without strong justification, useunknownfor generic type defaults instead
Avoid non-null assertions (!) - use proper error handling and throw specific errors when values are missing instead
Always use @frontmcp/utils for cryptographic operations - use hkdfSha256, encryptAesGcm, decryptAesGcm, randomBytes, sha256, sha256Hex, base64urlEncode, base64urlDecode instead of node:crypto
Always use @frontmcp/utils for file system operations - use readFile, writeFile, mkdir, rename, unlink, stat, etc. instead of fs/promises or node:fs
Use constrained generic type parameters withRecord<string, string>or similar constraints instead of unconstrainedanydefaults
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/libs/{sdk,adapters,plugins}/**/src/**/*.ts: MCP response types must use strictly typed MCP protocol definitions, notunknown- usePromise<GetPromptResult>,Promise<ReadResourceResult>, and similar MCP-defined types for execute() and read() methods
Validation flow pattern - execute/read methods return strictly typed MCP responses, parseOutput normalizes various input shapes, flows finalize using entry's parse methods
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/sdk/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Create shared base classes for common functionality - use ExecutionContextBase as parent class for ToolContext, ResourceContext, and similar context classes
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/{sdk,adapters}/**/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Files:
libs/sdk/src/tool/tool.registry.ts
**/*.{ts,tsx,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Document known limitations clearly in code comments and README files
Files:
libs/sdk/src/tool/tool.registry.ts
**/libs/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add backwards compatibility exports in new libraries - avoid legacy prefixes like PT-001 in test names
Files:
libs/sdk/src/tool/tool.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/tool/tool.registry.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters,plugins}/**/src/**/*error*.ts : Use specific MCP error classes with MCP error codes instead of generic errors - define error classes with `mcpErrorCode` property and `toJsonRpcError()` method
📚 Learning: 2026-01-06T02:34:41.820Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-06T02:34:41.820Z
Learning: Applies to **/libs/{sdk,adapters}/**/src/**/*.ts : Use getCapabilities() method in registries for dynamic capability exposure - do not hardcode capabilities in transport adapters
Applied to files:
libs/sdk/src/tool/tool.registry.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/tool/tool.registry.ts
🧬 Code graph analysis (1)
libs/sdk/src/tool/tool.registry.ts (7)
libs/sdk/src/tool/tool.types.ts (1)
IndexedTool(6-19)libs/sdk/src/scope/scope.instance.ts (2)
apps(329-331)Scope(37-384)libs/sdk/src/agent/agent.scope.ts (2)
apps(245-247)apps(322-324)libs/sdk/src/common/entries/base.entry.ts (1)
EntryLineage(6-6)libs/sdk/src/common/interfaces/index.ts (1)
Token(10-10)libs/sdk/src/auth/session/record/session.base.ts (1)
id(151-153)libs/sdk/src/tool/tool.instance.ts (2)
getMetadata(99-101)ToolInstance(37-184)
⏰ 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: Build Libraries
- GitHub Check: Lint & Format Checks
🔇 Additional comments (8)
libs/sdk/src/tool/tool.registry.ts (8)
38-39: Good solution for remote tool tracking.The dedicated
remoteAppToolsMap efficiently tracks tools by app ID, enabling robust cleanup during re-adoption. This addresses the fragile string-based filtering flagged in previous reviews.
173-178: LGTM—duck-typing is appropriate for remote registries.The type guard validates that
subscribeexists and is a function. While it doesn't verify the exact signature (options and callback parameters), this level of checking is reasonable for remote registries that may have varying implementations.
248-255: LGTM—clean delegation to existing adoption logic.The method correctly delegates local app tool adoption to the existing
adoptFromChildmechanism, maintaining consistency with the hierarchical registry pattern.
434-434: Type safety improvements align with ToolEntry widening.Both
makeRowandproviderIdOfnow acceptToolEntry, maintaining consistency with the broader refactoring. The defensive type narrowing inproviderIdOf(casting throughunknown) follows TypeScript best practices.Also applies to: 463-482
Also applies to: 463-482
582-591: Correct dynamic capability exposure.The method properly implements the registry pattern of exposing capabilities only when tools are registered, following coding guidelines. Based on learnings, this prevents hardcoding capabilities in transport adapters.
108-133: No changes needed. The code is already safe.
getActiveScope()has return typeScope(non-optional), notScope | null | undefined. The underlyingget()method throws an error ("Provider Scope is not available in local or parent registries") if Scope cannot be resolved, so there is no null/undefined case to defend against. The suggested defensive check would never execute.Likely an incorrect or invalid review comment.
493-509: Type guard correctly validates registration requirements, but fix incorrect reference.The
isRegisterableToolguard properly validates that entries have the requirednameandrecordproperties (withprovide,kind,metadata) before registration. However, the original comment referencesProxyToolEntry, which does not exist—remote tools use regularToolInstancecreated through thecreateRemoteToolInstance()factory. The type guard is correct for all ToolEntry subclasses.Likely an incorrect or invalid review comment.
538-576: The type cast is safe and justified.ProxyToolEntrydoes not exist in the codebase, and "RemoteToolInstance" is only mentioned in comments—remote tools are instantiated viacreateRemoteToolInstance(), which returns a standardToolInstanceobject.The pattern is correct:
registerToolInstance(tool: ToolEntry)accepts the abstract type for API flexibility- All actual values passed are
ToolInstanceobjects (either created directly or viacreateRemoteToolInstance)- The
instancesmap correctly stores onlyToolInstanceinstances- The type guard validates required properties exist
- The only usage of the map (
getInlineTools()) returns values asToolEntry[]anyway, so the cast poses no riskNo changes needed.
Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Plugins
Authentication
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.