fix(core): fix tool listing #4
Conversation
WalkthroughRefactors the invoker/flow hook system to plan-based metadata, restructures tool ownership and registry lineage, adds new tool flows, removes the session.create flow and flow routers, makes MCP handler guards/schemas optional, and introduces several SDK/type surface changes and scope initialization updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Scope
participant ToolsListFlow
participant ToolRegistry
participant App
Client->>Scope: runFlowForOutput('tools:list-tools', {request, ctx})
Scope->>ToolsListFlow: invoke flow (pre → execute → post)
rect rgb(230,245,255)
ToolsListFlow->>ToolsListFlow: parseInput (validate method)
ToolsListFlow->>ToolRegistry: findTools (discover per-scope / per-app)
end
rect rgb(235,255,230)
ToolsListFlow->>ToolsListFlow: resolveConflicts (detect name collisions)
ToolsListFlow->>App: collect tool metadata
ToolsListFlow->>ToolsListFlow: parseTools (build descriptors)
end
ToolsListFlow-->>Client: return ListToolsResult (tools[])
sequenceDiagram
participant ToolRegistry
participant ToolInstance
participant OwnerRef
Note over ToolRegistry: Old construction
ToolRegistry->>ToolInstance: new ToolInstance(record, providers)
ToolInstance->>ToolInstance: providedBy: Token[] (old)
Note over ToolRegistry: New construction (lineage/owner)
ToolRegistry->>ToolInstance: new ToolInstance(record, providers, owner: EntryOwnerRef)
ToolInstance->>ToolInstance: owner = EntryOwnerRef
ToolRegistry->>ToolRegistry: track EntryLineage for ownership resolution
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ UTG Post-Process Complete No new issues were detected in the generated code and all check runs have completed. The unit test generation process has completed successfully. |
|
Creating a PR to put the unit tests in... The changes have been created in this pull request: View PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (4)
libs/core/src/transport/adapters/transport.local.adapter.ts (2)
117-123: Unsafe non-null assertions and hardcoded protocol value.Two issues:
Lines 120 and 122 use non-null assertions (
session!.id,session!.payload) without validating thatsessionexists. Ifsessionis undefined in the auth object, this will cause a runtime error.Line 123 hardcodes
protocol: 'sse', but this class supports bothSSEServerTransportandStreamableHTTPServerTransport(line 14). This may be incorrect for HTTP-based transports.Consider this fix:
- const {token, user, session} = req[ServerRequestTokens.auth]; + const {token, user, session} = req[ServerRequestTokens.auth]; + if (!session) { + throw new Error('Session is required for authentication'); + } req.auth = { token, - sessionId: session!.id, + sessionId: session.id, user, - sessionIdPayload: session!.payload, - protocol: 'sse', + sessionIdPayload: session.payload, + protocol: this.transport instanceof SSEServerTransport ? 'sse' : 'http', scopes: [], clientId: user.sub ?? '', transport, } satisfies AuthInfo;
136-145: Inconsistent null safety checks.Line 136 accesses
req.body.errorwithout checking ifreq.bodyexists, while line 145 correctly uses optional chaining (req.body?.result). Ifreq.bodyis undefined, line 136 will throw a runtime error.Apply this fix:
- if (req.body.error) { - const {code, message} = req.body.error; + if (req.body?.error) { + const {code, message} = req.body.error; if (code === -32600 && message === 'Elicitation not supported') {libs/core/src/transport/mcp-handlers/call-tool-request.handler.ts (1)
20-25: Restore real tool execution in call handler.
Line 23 now returns a stub"tesintg"response, so everycallToolrequest will ignore the requested tool and always reply with that placeholder text. That completely breaks MCP tool invocation. Please wire this handler back to the tool execution path (e.g., resolve the tool via the scope registry or run the newtools:call-toolflow) and remove the stub before shipping.libs/core/src/tool/flows/tool.execute.flow.ts (1)
274-282: Duplicate Stage handlers double-fire parse/validateThere are already
@Stage('parseInput')and@Stage('validateInput')methods earlier in this class. Adding_parseInputStageand_validateInputStagewith the same stage names causes the pipeline to invoke those phases twice, doubling hook execution and mutating state twice. Please drop the duplicates (or give them distinct stage ids).- @Stage('parseInput') - async _parseInputStage() { - await this.runToolStage(ToolHookStage.willParseInput); - } - - @Stage('validateInput') - async _validateInputStage() { - await this.runToolStage(ToolHookStage.willValidateInput); - }
🧹 Nitpick comments (5)
libs/core/src/transport/flows/handle.sse.flow.ts (2)
62-62: Useconstfortokensince it's never reassigned.Only
sessionneeds to be mutable. Destructure immutable values withconstfor clarity.Apply this diff:
- let {token, session} = request[ServerRequestTokens.auth] as Authorization; + const {token} = request[ServerRequestTokens.auth] as Authorization; + let {session} = request[ServerRequestTokens.auth] as Authorization;Or use a more concise pattern:
- let {token, session} = request[ServerRequestTokens.auth] as Authorization; + const auth = request[ServerRequestTokens.auth] as Authorization; + const {token} = auth; + let {session} = auth;
98-109: Remove commented-out code.The explicit "Not implemented" error is appropriate, but the commented code should be removed to improve readability.
Apply this diff:
async onElicitResult() { - // const transport = await transportService.getTransporter('sse', token, session.id); - // if (!transport) { - // this.respond(httpRespond.rpcError('session not initialized')); - // return; - // } - // await transport.handleRequest(request, response); this.fail(new Error('Not implemented')); }libs/core/src/transport/flows/handle.streamable-http.flow.ts (1)
62-62: Useconstfortokensince it's never reassigned.Only
sessionneeds to be mutable. Destructure immutable values withconstfor clarity.Apply this diff:
- let {token, session} = request[ServerRequestTokens.auth] as Authorization; + const {token} = request[ServerRequestTokens.auth] as Authorization; + let {session} = request[ServerRequestTokens.auth] as Authorization;Or use a more concise pattern:
- let {token, session} = request[ServerRequestTokens.auth] as Authorization; + const auth = request[ServerRequestTokens.auth] as Authorization; + const {token} = auth; + let {session} = auth;libs/core/src/transport/adapters/transport.local.adapter.ts (1)
77-96: Remove commented code and avoid empty catch blocks.Several code quality concerns:
- Lines 81-83 contain commented-out code without explanation. Either remove it or add a comment explaining why it's disabled.
- Lines 84-85 and 92-94 use empty catch blocks that silently suppress errors, making debugging difficult.
- Lines 78 and 88 use
console.log/console.warninstead ofthis.logger(which is available and used elsewhere).Consider this refactor:
async destroy(reason?: string): Promise<void> { - console.log('destroying transporter, reason:', reason); + this.logger.info('Destroying transporter', { reason }); try { - // if(!this.transport.closed){ - // this.transport.close(); - // } - } catch { - /* empty */ + if (!this.transport.closed) { + this.transport.close(); + } + } catch (error) { + this.logger.warn('Failed to close transport', { error }); } if (reason) { - console.warn(`Destroying transporter: ${reason}`); + this.logger.warn('Destroying transporter with reason', { reason }); } else { try { this.onDispose?.(); - } catch { - /* empty */ + } catch (error) { + this.logger.error('onDispose callback failed', { error }); } } }README.md (1)
42-68: Fix nested list indentation to align with markdown standards.The nested items in the table of contents use 4-space indentation, but the markdown standard (MD007) expects 2 spaces per nesting level.
As per coding guidelines
Apply this diff to fix the indentation:
- [Why FrontMCP?](#why-frontmcp) - [Installation](#installation) - [Quickstart](#quickstart) - - [Minimal Server & App](#minimal-server--app) - - [Function and Class Tools](#function-and-class-tools) - - [Scripts & tsconfig](#scripts--tsconfig) - - [MCP Inspector](#mcp-inspector) + - [Minimal Server & App](#minimal-server--app) + - [Function and Class Tools](#function-and-class-tools) + - [Scripts & tsconfig](#scripts--tsconfig) + - [MCP Inspector](#mcp-inspector) - [Core Concepts](#core-concepts) - - [Servers](#servers) - - [Apps](#apps) - - [Tools](#tools) - - [Resources](#resources) - - [Prompts](#prompts) - - [Providers](#providers) - - [Adapters](#adapters) - - [Plugins](#plugins) + - [Servers](#servers) + - [Apps](#apps) + - [Tools](#tools) + - [Resources](#resources) + - [Prompts](#prompts) + - [Providers](#providers) + - [Adapters](#adapters) + - [Plugins](#plugins) - [Authentication](#authentication) - - [Remote OAuth](#remote-oauth) - - [Local OAuth](#local-oauth) + - [Remote OAuth](#remote-oauth) + - [Local OAuth](#local-oauth) - [Sessions & Transport](#sessions--transport) - [Logging Transports](#logging-transports) - [Deployment](#deployment) - - [Local Dev](#local-dev) - - [Production](#production) + - [Local Dev](#local-dev) + - [Production](#production) - [Version Alignment](#version-alignment) - [Contributing](#contributing) - [License](#license)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (55)
README.md(3 hunks)apps/demo/src/apps/expenses/tools/create-expense.tool.ts(1 hunks)docs/getting-started/quickstart.mdx(4 hunks)docs/index.mdx(0 hunks)docs/servers/extensibility/logging.mdx(1 hunks)libs/adapters/src/openapi/openapi.types.ts(1 hunks)libs/core/src/adapter/adapter.instance.ts(1 hunks)libs/core/src/app/app.registry.ts(2 hunks)libs/core/src/app/instances/app.local.instance.ts(1 hunks)libs/core/src/auth/flows/session.verify.flow.ts(0 hunks)libs/core/src/auth/flows/well-known.jwks.flow.ts(0 hunks)libs/core/src/auth/flows/well-known.oauth-authorization-server.flow.ts(0 hunks)libs/core/src/auth/flows/well-known.prm.flow.ts(0 hunks)libs/core/src/auth/session/flows/session.create.flow.ts(0 hunks)libs/core/src/common/flow.router.ts(0 hunks)libs/core/src/exceptions/tool-not-found.exception.ts(0 hunks)libs/core/src/invoker/index.ts(0 hunks)libs/core/src/invoker/invoker.decorators.ts(2 hunks)libs/core/src/invoker/invoker.factory.ts(2 hunks)libs/core/src/invoker/invoker.flow.ts(4 hunks)libs/core/src/invoker/invoker.helpers.ts(0 hunks)libs/core/src/invoker/invoker.types.ts(1 hunks)libs/core/src/plugin/plugin.registry.ts(4 hunks)libs/core/src/plugin/plugin.types.ts(0 hunks)libs/core/src/provider/provider.registry.ts(8 hunks)libs/core/src/scope/scope.instance.ts(5 hunks)libs/core/src/scope/scope.registry.ts(1 hunks)libs/core/src/scope/scope.utils.ts(3 hunks)libs/core/src/tool/flows/call-tool.flow.ts(1 hunks)libs/core/src/tool/flows/tool.execute.flow.ts(4 hunks)libs/core/src/tool/flows/tools-list.flow.ts(1 hunks)libs/core/src/tool/tool.instance.ts(2 hunks)libs/core/src/tool/tool.registry.ts(10 hunks)libs/core/src/tool/tool.types.ts(3 hunks)libs/core/src/tool/tool.utils.ts(3 hunks)libs/core/src/transport/adapters/transport.local.adapter.ts(4 hunks)libs/core/src/transport/flows/handle.sse.flow.ts(4 hunks)libs/core/src/transport/flows/handle.streamable-http.flow.ts(4 hunks)libs/core/src/transport/mcp-handlers/Initialized-notification.hanlder.ts(1 hunks)libs/core/src/transport/mcp-handlers/call-tool-request.handler.ts(1 hunks)libs/core/src/transport/mcp-handlers/index.ts(1 hunks)libs/core/src/transport/mcp-handlers/initialize-request.handler.ts(0 hunks)libs/core/src/transport/mcp-handlers/list-tools-request.handler.ts(1 hunks)libs/core/src/transport/mcp-handlers/mcp-handlers.types.ts(1 hunks)libs/core/src/transport/transport.registry.ts(2 hunks)libs/plugins/src/cache/cache.plugin.ts(0 hunks)libs/sdk/src/dynamic/dynamic.adapter.ts(2 hunks)libs/sdk/src/entries/index.ts(1 hunks)libs/sdk/src/entries/scope.entry.ts(2 hunks)libs/sdk/src/entries/tool.entry.ts(1 hunks)libs/sdk/src/interfaces/adapter.interface.ts(1 hunks)libs/sdk/src/interfaces/flow.interface.ts(2 hunks)libs/sdk/src/interfaces/internal/registry.interface.ts(1 hunks)libs/sdk/src/metadata/flow.metadata.ts(1 hunks)libs/sdk/src/metadata/front-mcp.metadata.ts(3 hunks)
💤 Files with no reviewable changes (13)
- libs/core/src/auth/flows/well-known.jwks.flow.ts
- libs/plugins/src/cache/cache.plugin.ts
- libs/core/src/invoker/index.ts
- libs/core/src/auth/flows/well-known.oauth-authorization-server.flow.ts
- libs/core/src/exceptions/tool-not-found.exception.ts
- docs/index.mdx
- libs/core/src/auth/flows/well-known.prm.flow.ts
- libs/core/src/common/flow.router.ts
- libs/core/src/invoker/invoker.helpers.ts
- libs/core/src/plugin/plugin.types.ts
- libs/core/src/auth/flows/session.verify.flow.ts
- libs/core/src/transport/mcp-handlers/initialize-request.handler.ts
- libs/core/src/auth/session/flows/session.create.flow.ts
🧰 Additional context used
🧬 Code graph analysis (18)
libs/core/src/transport/mcp-handlers/index.ts (1)
libs/core/src/transport/mcp-handlers/mcp-handlers.types.ts (1)
McpHandlerOptions(43-46)
libs/core/src/transport/mcp-handlers/Initialized-notification.hanlder.ts (1)
libs/core/src/transport/mcp-handlers/mcp-handlers.types.ts (1)
McpHandler(26-41)
libs/sdk/src/interfaces/flow.interface.ts (1)
libs/sdk/src/metadata/flow.metadata.ts (1)
FlowMetadata(35-44)
libs/core/src/tool/flows/tools-list.flow.ts (6)
libs/core/src/auth/flows/well-known.oauth-authorization-server.flow.ts (3)
outputSchema(47-47)ExtendFlows(76-78)Flow(85-144)libs/sdk/src/metadata/flow.metadata.ts (3)
FlowPlan(54-56)ExtendFlows(8-10)FlowRunOptions(23-30)libs/core/src/tool/flows/call-tool.flow.ts (2)
ExtendFlows(42-50)Flow(56-130)libs/core/src/auth/flows/well-known.jwks.flow.ts (2)
ExtendFlows(27-35)Flow(42-91)libs/core/src/scope/scope.instance.ts (1)
tools(116-118)libs/core/src/app/instances/app.local.instance.ts (1)
tools(73-75)
libs/core/src/tool/tool.instance.ts (1)
libs/core/src/provider/provider.registry.ts (1)
ProviderRegistry(24-890)
libs/core/src/transport/mcp-handlers/call-tool-request.handler.ts (1)
libs/core/src/transport/mcp-handlers/mcp-handlers.types.ts (2)
McpHandlerOptions(43-46)McpHandler(26-41)
libs/core/src/provider/provider.registry.ts (1)
libs/core/src/scope/scope.instance.ts (2)
providers(108-110)Scope(25-136)
libs/core/src/tool/flows/call-tool.flow.ts (2)
libs/sdk/src/metadata/flow.metadata.ts (3)
FlowPlan(54-56)ExtendFlows(8-10)FlowRunOptions(23-30)libs/core/src/tool/flows/tools-list.flow.ts (2)
ExtendFlows(44-52)Flow(58-170)
libs/sdk/src/entries/scope.entry.ts (1)
libs/sdk/src/interfaces/internal/registry.interface.ts (1)
ToolRegistryInterface(87-95)
libs/core/src/invoker/invoker.flow.ts (4)
libs/sdk/src/metadata/flow.metadata.ts (1)
FlowName(13-13)libs/core/src/invoker/invoker.types.ts (1)
CreateOptions(215-218)libs/core/src/scope/scope.instance.ts (1)
Scope(25-136)libs/core/src/invoker/invoker.factory.ts (1)
makeRouteInvoker(115-147)
libs/core/src/transport/flows/handle.streamable-http.flow.ts (2)
libs/core/src/transport/flows/handle.sse.flow.ts (1)
stateSchema(27-31)libs/core/src/scope/scope.instance.ts (1)
Scope(25-136)
libs/core/src/transport/flows/handle.sse.flow.ts (1)
libs/core/src/transport/flows/handle.streamable-http.flow.ts (1)
stateSchema(27-31)
libs/core/src/tool/tool.registry.ts (3)
libs/sdk/src/interfaces/internal/registry.interface.ts (1)
ToolRegistryInterface(87-95)libs/core/src/tool/tool.types.ts (2)
DEFAULT_EXPORT_OPTS(88-93)IndexedTool(64-76)libs/core/src/tool/tool.utils.ts (3)
normalizeSegment(93-98)normalizeProviderId(100-106)normalizeOwnerPath(108-119)
libs/core/src/tool/tool.types.ts (2)
libs/core/src/tool/tool.instance.ts (1)
ToolInstance(5-44)libs/core/src/tool/tool.registry.ts (1)
ToolRegistry(28-399)
libs/core/src/scope/scope.utils.ts (1)
libs/sdk/src/metadata/front-mcp.metadata.ts (1)
AppScopeMetadata(82-86)
libs/core/src/scope/scope.instance.ts (5)
libs/core/src/app/app.registry.ts (1)
AppRegistry(9-72)libs/core/src/tool/tool.registry.ts (1)
ToolRegistry(28-399)libs/core/src/transport/transport.registry.ts (1)
TransportService(20-205)libs/sdk/src/metadata/flow.metadata.ts (1)
FlowName(13-13)libs/sdk/src/interfaces/flow.interface.ts (2)
FlowInputOf(8-8)FlowOutputOf(9-9)
libs/core/src/transport/mcp-handlers/list-tools-request.handler.ts (1)
libs/core/src/transport/mcp-handlers/mcp-handlers.types.ts (2)
McpHandlerOptions(43-46)McpHandler(26-41)
libs/core/src/tool/tool.utils.ts (1)
libs/core/src/tool/tool.types.ts (1)
NameCase(80-80)
🪛 ESLint
libs/core/src/provider/provider.registry.ts
[error] 865-865: Unexpected aliasing of 'this' to local variable.
(@typescript-eslint/no-this-alias)
libs/core/src/tool/flows/call-tool.flow.ts
[error] 87-89: Unexpected empty async method 'findTool'.
(@typescript-eslint/no-empty-function)
[error] 92-94: Unexpected empty async method 'acquireQuota'.
(@typescript-eslint/no-empty-function)
[error] 97-99: Unexpected empty async method 'acquireSemaphore'.
(@typescript-eslint/no-empty-function)
[error] 102-104: Unexpected empty async method 'createToolCallContext'.
(@typescript-eslint/no-empty-function)
[error] 107-109: Unexpected empty async method 'validateInput'.
(@typescript-eslint/no-empty-function)
[error] 112-114: Unexpected empty async method 'execute'.
(@typescript-eslint/no-empty-function)
[error] 117-119: Unexpected empty async method 'validateOutput'.
(@typescript-eslint/no-empty-function)
[error] 122-124: Unexpected empty async method 'releaseSemaphore'.
(@typescript-eslint/no-empty-function)
[error] 127-129: Unexpected empty async method 'releaseQuota'.
(@typescript-eslint/no-empty-function)
libs/core/src/transport/flows/handle.streamable-http.flow.ts
[error] 62-62: 'token' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 102-104: Unexpected empty async method 'onElicitResult'.
(@typescript-eslint/no-empty-function)
libs/core/src/transport/flows/handle.sse.flow.ts
[error] 62-62: 'token' is never reassigned. Use 'const' instead.
(prefer-const)
libs/core/src/tool/tool.registry.ts
[error] 169-169: Type boolean trivially inferred from a boolean literal, remove type annotation.
(@typescript-eslint/no-inferrable-types)
libs/core/src/tool/tool.utils.ts
[error] 51-51: Unnecessary escape character: ..
(no-useless-escape)
[error] 51-51: Unnecessary escape character: /.
(no-useless-escape)
🪛 markdownlint-cli2 (0.18.1)
README.md
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
47-47: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
48-48: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
52-52: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
53-53: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
54-54: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
60-60: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
64-64: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
65-65: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
65-65: Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (18)
libs/core/src/app/instances/app.local.instance.ts (1)
1-2: LGTM: Formatting-only changes.The import statements have been reformatted to remove spaces inside braces. No functional changes.
docs/servers/extensibility/logging.mdx (1)
6-6: LGTM! Front matter delimiter added correctly.The closing
---properly delimits the YAML front matter section, which is standard MDX/markdown format.README.md (1)
431-432: LGTM! Paragraph reflowed without semantic changes.The version alignment explanation remains accurate and clear.
docs/getting-started/quickstart.mdx (2)
30-30: LGTM! Path references updated consistently.The file path references have been simplified from
apps/hello/src/...tosrc/..., reflecting a flatter and more beginner-friendly project structure. All three locations (app comment, tool comment, and dev script) are updated consistently.Also applies to: 45-45, 68-68
83-84: LGTM! Tip content reformatted for readability.The JSON payload example has been moved to a separate line, improving readability without changing the content.
apps/demo/src/apps/expenses/tools/create-expense.tool.ts (1)
17-17: All references to the old tool name have been successfully updated.Verification confirms no string references to
'create-expense'exist in the codebase. The new tool name'createExpense'is correctly set in the tool definition, and the import/usage inindex.tsproperly references the updated tool. The breaking change has been successfully addressed.libs/sdk/src/entries/index.ts (1)
1-1: LGTM!Exposing
base.entryis necessary sinceEntryOwnerRefis now used as a public property type inToolEntry(line 9 oftool.entry.ts). This properly extends the public API surface.libs/core/src/app/app.registry.ts (1)
47-66: LGTM! Parallel initialization improves performance.Collecting all
app.readypromises and awaiting them withPromise.allenables concurrent app initialization instead of sequential. If any app fails to initialize,Promise.allwill reject immediately, which is appropriate for the initialization phase.libs/core/src/transport/mcp-handlers/index.ts (1)
7-14: LGTM! Inferred return type is sufficient.Removing the explicit return type allows TypeScript to infer it from the implementation. This is a clean simplification with no functional impact.
libs/core/src/scope/scope.utils.ts (2)
49-54: LGTM! Hardcoded 'root' ID is safe.The multi-app scope uses a hardcoded
'root'ID, which is always defined and appropriate for the root scope context.
36-42: No issues found; review comment is invalid.Both
LocalAppMetadataandRemoteAppMetadatadefinename: stringas a required field (not optional). SinceAppMetadatais a union of these two types,appMetadata.nameis guaranteed to exist. Therefore,appMetadata.id ?? appMetadata.namewill always resolve to a defined string and poses no type safety risk.Likely an incorrect or invalid review comment.
libs/core/src/transport/transport.registry.ts (1)
1-19: LGTM! Import formatting cleanup with no functional changes.The changes are limited to import formatting and organization. No logic or behavior is affected.
libs/sdk/src/interfaces/flow.interface.ts (1)
48-57: LGTM! Logger exposure improves flow observability.Adding
scopeLoggertoFlowBaseallows flow implementations to access structured logging viathis.scopeLogger. The initialization fromscope.loggeris straightforward and safe.libs/sdk/src/entries/tool.entry.ts (1)
9-9: Theownerproperty is correctly initialized across all instantiation sites.The verification confirms that:
ToolRegistryacceptsowner: EntryOwnerRefin its constructor (line 56) and assigns it asthis.owner(line 59)ToolInstanceis instantiated only atToolRegistryline 103 withnew ToolInstance(rec, this.providers, this.owner)ToolInstanceconstructor (line 13) properly receivesowner: EntryOwnerRefand assigns it (line 15)- No direct
ToolEntryinstantiations or factory methods bypass this initializationThe change is implemented correctly with no uninitialized
ownerproperties.libs/core/src/adapter/adapter.instance.ts (1)
55-59: The change is type-safe; no action required.The AdapterInterface explicitly requires
options: {name:string} & any, guaranteeing thatadapter.options.nameis always defined as a string. All adapters must implement AdapterInterface, so the TypeScript compiler prevents any adapter from omitting the name field. The change from the fallback approach to direct access is actually more type-safe, as it leverages the type system rather than metadata fields.libs/core/src/invoker/invoker.flow.ts (3)
12-20: LGTM! Clean import consolidation.The import restructuring aligns well with the plan-based refactoring approach. Moving
FlowNameto@frontmcp/sdkcentralizes type definitions appropriately.
95-95: LGTM! Cleaner constructor reference.The direct
instance.constructorusage is more concise than the previous approach while maintaining type safety.
141-141: LGTM! Idiomatic ES6 shorthand.The object shorthand syntax changes improve code clarity without altering behavior. The spread operation order at line 154 correctly allows
baseExtrasto override properties fromextrasFactory().Also applies to: 149-150, 154-154, 194-194
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
310-322: Remote OAuth formatting still contains spacing issues around optional markers.While the Local OAuth block (lines 332–339) has been correctly formatted with no spaces around
?:markers (e.g.,scopes?: string[]), the Remote OAuth block still retains the malformed spacing from the earlier review concern. Lines 310–322 showdcrEnabled ? : boolean,,clientId ? : string ..., etc., which should match the corrected Local OAuth pattern.Apply this diff to align Remote OAuth formatting with the corrected Local OAuth block:
auth: { type: 'remote', name: 'frontegg', baseUrl: 'https://idp.example.com', - dcrEnabled ? : boolean, - clientId ? : string | ((info: { clientId: string }) => string), - mode ? : 'orchestrated' | 'transparent', - allowAnonymous ? : boolean, - consent ? : boolean, - scopes ? : string[], - grantTypes ? : ('authorization_code' | 'refresh_token')[], - authEndpoint ? : string, - tokenEndpoint ? : string, - registrationEndpoint ? : string, - userInfoEndpoint ? : string, - jwks ? : JSONWebKeySet, - jwksUri ? : string + dcrEnabled?: boolean, + clientId?: string | ((info: { clientId: string }) => string), + mode?: 'orchestrated' | 'transparent', + allowAnonymous?: boolean, + consent?: boolean, + scopes?: string[], + grantTypes?: ('authorization_code' | 'refresh_token')[], + authEndpoint?: string, + tokenEndpoint?: string, + registrationEndpoint?: string, + userInfoEndpoint?: string, + jwks?: JSONWebKeySet, + jwksUri?: string }
🧹 Nitpick comments (1)
README.md (1)
42-68: Fix table of contents indentation to conform to markdown style guide.The nested list items (sub-topics under main categories) use 4-space indentation, but
markdownlintexpects 2-space indentation per the MD007 rule. This inconsistency should be standardized.Apply this diff to fix the indentation:
- [Why FrontMCP?](#why-frontmcp) - [Installation](#installation) - [Quickstart](#quickstart) - - [Minimal Server & App](#minimal-server--app) - - [Function and Class Tools](#function-and-class-tools) - - [Scripts & tsconfig](#scripts--tsconfig) - - [MCP Inspector](#mcp-inspector) +- [Why FrontMCP?](#why-frontmcp) +- [Installation](#installation) +- [Quickstart](#quickstart) + - [Minimal Server & App](#minimal-server--app) + - [Function and Class Tools](#function-and-class-tools) + - [Scripts & tsconfig](#scripts--tsconfig) + - [MCP Inspector](#mcp-inspector) - [Core Concepts](#core-concepts) - - [Servers](#servers) - - [Apps](#apps) - - [Tools](#tools) - - [Resources](#resources) - - [Prompts](#prompts) - - [Providers](#providers) - - [Adapters](#adapters) - - [Plugins](#plugins) + - [Servers](#servers) + - [Apps](#apps) + - [Tools](#tools) + - [Resources](#resources) + - [Prompts](#prompts) + - [Providers](#providers) + - [Adapters](#adapters) + - [Plugins](#plugins) - [Authentication](#authentication) - - [Remote OAuth](#remote-oauth) - - [Local OAuth](#local-oauth) + - [Remote OAuth](#remote-oauth) + - [Local OAuth](#local-oauth) - [Sessions & Transport](#sessions--transport) - [Logging Transports](#logging-transports) - [Deployment](#deployment) - - [Local Dev](#local-dev) - - [Production](#production) + - [Local Dev](#local-dev) + - [Production](#production) - [Version Alignment](#version-alignment) - [Contributing](#contributing) - [License](#license)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(5 hunks)docs/servers/authentication/overview.mdx(1 hunks)docs/servers/extensibility/adapters.mdx(1 hunks)docs/servers/extensibility/logging.mdx(1 hunks)docs/servers/extensibility/plugins.mdx(1 hunks)docs/servers/extensibility/providers.mdx(1 hunks)libs/sdk/src/interfaces/adapter.interface.ts(2 hunks)
✅ Files skipped from review due to trivial changes (3)
- docs/servers/authentication/overview.mdx
- docs/servers/extensibility/adapters.mdx
- docs/servers/extensibility/providers.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/interfaces/adapter.interface.ts
- docs/servers/extensibility/logging.mdx
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
45-45: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
46-46: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
47-47: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
48-48: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
52-52: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
53-53: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
54-54: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
55-55: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
56-56: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
57-57: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
59-59: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
60-60: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
64-64: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
65-65: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
65-65: Link fragments should be valid
(MD051, link-fragments)
🔇 Additional comments (1)
docs/servers/extensibility/plugins.mdx (1)
2-3: Documentation metadata updates look good.The title change from "Plugins" to "Plugin Extensions" improves clarity and specificity, while the new
sidebarTitlekeeps navigation concise. This is consistent with the pattern applied to other doc pages (adapters, providers) as part of the site reorganization.
Summary by CodeRabbit
New Features
Improvements
Refactoring