-
Notifications
You must be signed in to change notification settings - Fork 4
feat: Add dashboard plugin for built-in Frontmcp visualization tools #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds a Dashboard plugin (UI, graph provider, tools), SDK introspection APIs, E2E standalone demo apps/tests, legacy SSE sessionId-from-query handling, PluginRegistry hookScope support, TypeScript/declaration map updates, and various build/config and CLI scaffolding changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Client
participant Server as FrontMcp Server
participant Provider as GraphDataProvider
participant Registry as ScopeRegistry
participant SSE as SSE/Transport
Browser->>Server: GET /dashboard
Server->>Server: generateDashboardHtml(options)
Server-->>Browser: HTML (inline or CDN entry)
Browser->>SSE: Open SSE /message (MCP)
SSE->>Server: MCP request dashboard:graph
Server->>Provider: getGraphData()
Provider->>Registry: getScopes()
Registry-->>Provider: [ScopeEntry...]
Provider-->>Server: GraphData
Server-->>SSE: MCP response
SSE-->>Browser: SSE event (graph result)
sequenceDiagram
participant Client as Legacy SSE Client
participant Req as HTTP Handler
participant Transport as Transport Layer
participant SessionMgr as SessionManager
Client->>Req: POST /message?sessionId=abc123
Req->>Req: getQuerySessionId() and read header
alt header present
Req->>Transport: use header session id
else header absent & query present
Req->>Transport: use query session id
end
Transport->>SessionMgr: resolve or create session
SessionMgr-->>Transport: session object
Transport-->>Req: attach session -> 200 OK
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (8)
libs/cli/src/graph/loader.ts (3)
14-16: Unused interface export.
LoadResultis defined and exported but never used —loadMcpForGraphreturnsGraphDatadirectly. Consider removing this interface to reduce API surface noise.🔎 Proposed fix
-/** - * Result of loading an MCP server for graph extraction. - */ -export interface LoadResult { - graphData: GraphData; -} -
137-165: Silent error suppression may hide extraction issues.Multiple empty
catch {}blocks (here and at lines 199, 226, 268, 292, 315) silently swallow extraction errors. While defensive extraction is reasonable, completely suppressing errors makes debugging difficult when graph data is incomplete.Consider logging to stderr so errors appear in the subprocess output:
🔎 Example fix for one catch block
- } catch {} + } catch (e) { + console.error('Warning: failed to extract auth info:', e); + }
440-450: Fallback JSON parsing heuristic is fragile.The fallback logic searching for lines starting with
{and containing"nodes"could match unintended output (e.g., debug logs containing JSON). Since the marker-based extraction should work in normal cases, consider either:
- Removing the fallback and failing with a clear error, or
- Adding a more specific pattern (e.g., checking for complete GraphData structure)
libs/cli/src/graph/extractor.ts (2)
253-255: Empty catch blocks may hide unexpected errors.The catch blocks swallow all exceptions, not just "registry unavailable" errors. Unexpected runtime errors (type mismatches, null dereferences) will be silently ignored, making debugging difficult.
Consider at least logging at debug level, or narrow the catch to specific expected conditions.
🔎 Example approach
- } catch { - // Apps registry may not be available - } + } catch (error) { + // Apps registry may not be available - only expected for optional registries + if (process.env.DEBUG) { + console.debug('[graph] Apps registry unavailable:', error); + } + }
137-146: Edge duplicates are not prevented.Unlike
addNode,addEdgedoes not check for duplicates. If an edge is added from both app-level and scope-level extraction, duplicates will accumulate, potentially inflatingedgeCountand causing visual issues.🔎 Proposed fix
const addedNodeIds = new Set<string>(); +const addedEdgeIds = new Set<string>(); // Helper to add edge const addEdge = (source: string, target: string, type: GraphEdge['type'] = 'contains'): void => { const edgeId = `${source}->${target}`; + if (addedEdgeIds.has(edgeId)) return; + addedEdgeIds.add(edgeId); edges.push({ id: edgeId, source, target, type, }); };libs/cli/src/graph/server.ts (3)
69-76: SIGINT listener accumulates on repeated calls and Promise settlement is unclear.Each call to
startGraphServeradds a newSIGINTlistener without cleanup. Additionally,resolve()is called immediately beforeprocess.exit(0), making the settlement behavior unclear since the process terminates.Consider using
process.once('SIGINT', ...)and removing the redundantresolve().🔎 Proposed fix
// Handle graceful shutdown - process.on('SIGINT', () => { + process.once('SIGINT', () => { console.log(`\n${c('gray', '[graph]')} shutting down...`); server.close(); - resolve(); process.exit(0); });
82-101: Consider handling spawn errors for robustness.If
xdg-openis not installed on Linux or the command fails, the error is silently ignored. For a dev tool this is acceptable, but a brief catch could improve user feedback.🔎 Example improvement
- spawn(command, args, { stdio: 'ignore', detached: true }).unref(); + const child = spawn(command, args, { stdio: 'ignore', detached: true }); + child.on('error', () => { + // Silently ignore - browser opening is best-effort + }); + child.unref();
424-434:nodeSizesis missing entries forresource-templateandadapternode types.While the fallback
{ width: 200, height: 70 }on line 444 handles this gracefully, adding explicit entries would be more consistent with thenodeConfigdefinitions above.🔎 Proposed addition
const nodeSizes = { server: { width: 220, height: 70 }, scope: { width: 220, height: 70 }, auth: { width: 220, height: 70 }, app: { width: 200, height: 70 }, plugin: { width: 200, height: 70 }, + adapter: { width: 200, height: 70 }, tool: { width: 200, height: 80 }, resource: { width: 200, height: 70 }, + 'resource-template': { width: 200, height: 70 }, prompt: { width: 200, height: 70 }, };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/demo/src/main.tslibs/cli/src/args.tslibs/cli/src/cli.tslibs/cli/src/commands/graph.tslibs/cli/src/graph/extractor.tslibs/cli/src/graph/index.tslibs/cli/src/graph/json-exporter.tslibs/cli/src/graph/loader.tslibs/cli/src/graph/server.tslibs/cli/src/graph/types.tslibs/sdk/src/front-mcp/front-mcp.tslibs/sdk/src/scope/scope.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/cli/src/graph/json-exporter.tslibs/cli/src/graph/index.tslibs/cli/src/args.tslibs/cli/src/graph/loader.tslibs/cli/src/commands/graph.tslibs/cli/src/cli.tslibs/cli/src/graph/server.tsapps/demo/src/main.tslibs/sdk/src/scope/scope.registry.tslibs/cli/src/graph/types.tslibs/cli/src/graph/extractor.tslibs/sdk/src/front-mcp/front-mcp.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/cli/src/graph/json-exporter.tslibs/cli/src/graph/index.tslibs/cli/src/args.tslibs/cli/src/graph/loader.tslibs/cli/src/commands/graph.tslibs/cli/src/cli.tslibs/cli/src/graph/server.tslibs/sdk/src/scope/scope.registry.tslibs/cli/src/graph/types.tslibs/cli/src/graph/extractor.tslibs/sdk/src/front-mcp/front-mcp.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/main.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/scope/scope.registry.tslibs/sdk/src/front-mcp/front-mcp.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/**/index.ts : Export public API through barrel files - export users' needed items, no legacy exports with different names
Applied to files:
libs/cli/src/graph/json-exporter.tslibs/cli/src/graph/index.tslibs/cli/src/graph/extractor.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/*/index.ts : Implement entry point re-exports in index.ts files for each module (adapters, build, bundler, etc.)
Applied to files:
libs/cli/src/graph/index.tslibs/cli/src/graph/loader.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/cli/src/graph/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/cli/src/graph/index.tslibs/cli/src/graph/loader.tslibs/sdk/src/front-mcp/front-mcp.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{adapters,theme,build}/**/*.test.{ts,tsx,js,jsx} : Test component behavior across all platform configurations (OpenAI, Claude, etc.)
Applied to files:
libs/cli/src/args.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:
libs/cli/src/cli.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/cli/src/cli.tslibs/cli/src/graph/types.tslibs/cli/src/graph/extractor.tslibs/sdk/src/front-mcp/front-mcp.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/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:
apps/demo/src/main.tslibs/sdk/src/front-mcp/front-mcp.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/scope/scope.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/universal/**/*.{ts,tsx} : Universal app shell (UniversalApp, FrontMCPProvider) must provide platform-agnostic React context and initialization
Applied to files:
libs/sdk/src/front-mcp/front-mcp.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/front-mcp/front-mcp.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/*/package.json : Each library in /libs/* is independent and publishable under frontmcp/* scope
Applied to files:
libs/sdk/src/front-mcp/front-mcp.ts
🧬 Code graph analysis (7)
libs/cli/src/graph/json-exporter.ts (2)
libs/cli/src/graph/types.ts (1)
GraphData(90-94)libs/cli/src/colors.ts (1)
c(13-13)
libs/cli/src/graph/loader.ts (1)
libs/cli/src/graph/types.ts (1)
GraphData(90-94)
libs/cli/src/commands/graph.ts (6)
libs/cli/src/args.ts (1)
ParsedArgs(17-36)libs/cli/src/colors.ts (1)
c(13-13)libs/cli/src/utils/fs.ts (1)
resolveEntry(34-72)libs/cli/src/graph/loader.ts (1)
loadMcpForGraph(22-45)libs/cli/src/graph/json-exporter.ts (1)
exportGraphJson(14-25)libs/cli/src/graph/server.ts (1)
startGraphServer(18-77)
libs/cli/src/cli.ts (2)
libs/cli/src/colors.ts (1)
c(13-13)libs/cli/src/commands/graph.ts (1)
runGraph(23-69)
apps/demo/src/main.ts (1)
libs/sdk/src/common/decorators/front-mcp.decorator.ts (1)
FrontMcp(45-106)
libs/cli/src/graph/extractor.ts (1)
libs/cli/src/graph/types.ts (3)
GraphNode(31-57)GraphEdge(67-73)GraphOwnerRef(23-26)
libs/sdk/src/front-mcp/front-mcp.ts (1)
libs/sdk/src/common/metadata/front-mcp.metadata.ts (1)
FrontMcpConfigType(209-209)
🔇 Additional comments (18)
apps/demo/src/main.ts (2)
8-8: LGTM! Import follows existing pattern.The import statement correctly follows the same pattern as the WeatherMcpApp import and aligns with the PR objective of enriching the demo for graph visualization testing.
12-12: Addition of CrmMcpApp to the apps array is correct. The CrmMcpApp module is properly exported as a default class and correctly imported. This enhancement to the demo application for testing the graph visualization feature is valid.libs/sdk/src/scope/scope.registry.ts (1)
103-109: LGTM!The
getScopes()method is well-implemented with clear documentation. Returning a shallow copy via spread operator protects the internalinstancesmap from external mutation.libs/sdk/src/front-mcp/front-mcp.ts (2)
46-52: LGTM with a minor consideration.The delegation to
this.scopes.getScopes()is clean. Note that callinggetScopes()beforereadyresolves will throw sincethis.scopesis uninitialized. This is acceptable sincecreateForGraph()awaitsreadybefore returning the instance, but callers using the constructor directly should be aware.
84-98: LGTM!The
createForGraph()factory method correctly separates initialization from server startup, enabling introspection use cases. The pattern is consistent withcreateHandler()and the JSDoc example clearly demonstrates the intended usage.libs/cli/src/args.ts (1)
59-71: LGTM!The argument parsing for graph flags is implemented correctly:
--jsonproperly handles both boolean (no value) and string (file path) cases--portparsing is consistent with the existing--timeoutpattern- Edge case handling for
--jsonfollowed by another flag is correctlibs/cli/src/cli.ts (1)
123-125: LGTM!The graph command integration follows established CLI patterns. Help text is comprehensive with clear examples covering all options.
libs/cli/src/graph/index.ts (1)
1-9: LGTM!Clean barrel file consolidating graph module exports. This follows the established pattern for public API exposure through index files. Based on learnings, this aligns with SDK guidelines for barrel file exports.
libs/cli/src/graph/json-exporter.ts (1)
14-25: LGTM!Clean implementation of the JSON exporter. The dual behavior (file vs stdout) is well-handled, and letting file write errors bubble up to the command handler's catch block is a reasonable approach.
libs/cli/src/commands/graph.ts (1)
23-69: LGTM!Well-structured command implementation with:
- Clear error messages guiding users on entry file resolution
- Proper early return after JSON export
- Consistent error handling pattern with process.exit(1)
- Default values matching documentation (port 4200, open false)
libs/cli/src/graph/loader.ts (1)
400-411: [rewritten review comment]
[classification tag]libs/cli/src/graph/extractor.ts (2)
8-119: Well-designed decoupling interfaces.The minimal interface definitions effectively decouple the CLI from the SDK's concrete types, allowing runtime duck-typing while maintaining type safety during development. This approach supports flexibility for future SDK changes.
369-430: Clean helper function implementations.The node ID generators and node factory functions follow a consistent pattern and properly construct
GraphNodeobjects with the correct types and data mappings.libs/cli/src/graph/server.ts (2)
116-127: CDN dependency versions are pinned appropriately.Using pinned versions (React 19, @xyflow/react@12, dagre 0.8.5) via esm.sh ensures consistent behavior without requiring a build step. This is well-suited for a dev visualization tool.
18-77: Solid dev server implementation.The HTTP server correctly handles routes, provides helpful error messages for port conflicts, and includes graceful shutdown. The Promise-based API integrates well with the CLI command pattern.
libs/cli/src/graph/types.ts (3)
5-18: Comprehensive node type enumeration.The
GraphNodeTypeunion covers all necessary node variants for the server structure visualization. The literal string union provides strong type safety.
31-57: Well-typedGraphNodeinterface following TypeScript best practices.The use of
unknownfor schema types (lines 41-42) andRecord<string, unknown>for annotations aligns with the coding guidelines requirement to avoidany. The inline data type effectively captures the type-specific variations.
104-126: UseReactFlowNodeandReactFlowEdgetypes in thelayoutGraphfunction or document their purpose.The
layoutGraphfunction (line 437 inserver.ts) constructs React Flow nodes and edges inline without referencing the exportedReactFlowNode,ReactFlowEdge, andReactFlowGraphDatatypes. Either apply these types to annotate the return value and improve type safety, or add a comment if these types are intentionally reserved for external consumers of the module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 20
🧹 Nitpick comments (9)
libs/sdk/src/common/utils/decide-request-intent.utils.ts (1)
148-158: Consider usingunknownwith type guards instead ofany.Per coding guidelines, prefer
unknownoverany. The cast toanyon line 150 bypasses type safety.🔎 Suggested refinement
function getQuerySessionId(req: ServerRequest): string | undefined { - const anyReq = req as any; - const raw = anyReq.url ?? anyReq.path ?? '/'; + const raw = (req as { url?: string }).url ?? (req as { path?: string }).path ?? '/'; try { const u = new URL(String(raw), 'http://local'); return u.searchParams.get('sessionId') ?? undefined; } catch { return undefined; } }libs/plugins/src/dashboard/tools/list-tools.tool.ts (1)
48-48: Consider adding type parameters to ToolContext for better type safety.The class extends
ToolContextwithout specifying the input and output type parameters, which reduces type safety for theexecutemethod.🔎 Proposed enhancement with type parameters
-export default class ListToolsTool extends ToolContext { +export default class ListToolsTool extends ToolContext<ListToolsInput, ListToolsOutput> { async execute(input: ListToolsInput): Promise<ListToolsOutput> {apps/e2e/demo-e2e-standalone/src/apps/isolated/resources/isolated-info.resource.ts (1)
12-17: Consider addingoutputSchemato the decorator for consistency.The
outputSchemais defined but not passed to the@Resourcedecorator. While this may work for the E2E fixture, including it in the decorator (similar to howTooldecorators includeoutputSchema) would enable runtime validation and improve consistency with other resource definitions.🔎 Proposed enhancement
@Resource({ uri: 'isolated://info', name: 'Isolated Info', description: 'Information about the isolated standalone app', mimeType: 'application/json', + outputSchema, })apps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.ts (1)
51-62: Consider usingafterEachor try/finally for client cleanup.Multiple tests create
isolatedClientinstances and calldisconnect()at the end. If a test fails mid-execution, the client won't be properly disconnected. Consider extracting client creation to a shared setup with proper cleanup hooks to ensure resources are released even on test failures.libs/sdk/src/transport/flows/handle.sse.flow.ts (1)
106-109: Type cast for URL access is pragmatic but consider defining a proper interface.The
anyReqcast allows access tourlorpathproperties that may not be in the base type. While functional, this could hide type mismatches. Consider defining a minimal interface for the expected request shape or adding a type guard.🔎 Alternative approach
- const anyReq = request as { url?: string; path?: string }; - const querySessionId = getQuerySessionId(anyReq.url ?? anyReq.path); + const requestUrl = (request as { url?: string }).url ?? request.path; + const querySessionId = getQuerySessionId(requestUrl);Or define an interface at the top of the file if this pattern is reused:
interface RequestWithUrl { url?: string; path?: string; }libs/plugins/src/dashboard/dashboard.plugin.ts (1)
71-78: Consider extracting duplicated options parsing logic.Both the constructor and
dynamicProvidersperform identical options parsing with the same merge-and-parse pattern. While this duplication may be intentional for static method independence, consider extracting to a private static helper for maintainability.🔎 Proposed refactor
+ private static parseOptions(options: DashboardPluginOptionsInput): DashboardPluginOptions { + return dashboardPluginOptionsSchema.parse({ + ...defaultDashboardPluginOptions, + ...options, + }); + } + constructor(options: DashboardPluginOptionsInput = {}) { super(); - // Parse options with Zod schema to apply all defaults - this.options = dashboardPluginOptionsSchema.parse({ - ...defaultDashboardPluginOptions, - ...options, - }); + this.options = DashboardPlugin.parseOptions(options); } static override dynamicProviders(options: DashboardPluginOptionsInput): ProviderType[] { - // Parse options with Zod schema to apply all defaults - const parsedOptions = dashboardPluginOptionsSchema.parse({ - ...defaultDashboardPluginOptions, - ...options, - }); + const parsedOptions = DashboardPlugin.parseOptions(options); return [ { name: 'dashboard:config', provide: DashboardConfigToken, useValue: parsedOptions, }, ]; }Also applies to: 85-99
libs/plugins/src/dashboard/providers/graph-data.provider.ts (1)
31-44: Cache implementation lacks concurrency safety.If
getGraphData()is called concurrently while cache is stale, multiple extractions may run simultaneously. Consider adding a mutex or pending-promise pattern to avoid redundant work.🔎 Proposed fix with pending promise
private cachedData: GraphData | null = null; private cacheTimestamp = 0; private readonly cacheTTL = 5000; // 5 seconds cache + private pendingExtraction: Promise<GraphData> | null = null; async getGraphData(): Promise<GraphData> { const now = Date.now(); // Return cached if still valid if (this.cachedData && now - this.cacheTimestamp < this.cacheTTL) { return this.cachedData; } + // If extraction is already in progress, wait for it + if (this.pendingExtraction) { + return this.pendingExtraction; + } + // Extract fresh graph data - this.cachedData = this.extractGraphData(); - this.cacheTimestamp = now; + this.pendingExtraction = Promise.resolve().then(() => { + this.cachedData = this.extractGraphData(); + this.cacheTimestamp = Date.now(); + this.pendingExtraction = null; + return this.cachedData; + }); - return this.cachedData; + return this.pendingExtraction; }libs/plugins/src/dashboard/html/html.generator.ts (1)
364-370: Request timeout doesn't clear on successful response.The timeout at lines 365-370 will fire even after a successful response, causing a redundant
pendingRequests.delete()call. While harmless, it's wasteful.🔎 Proposed fix with timeout cleanup
return new Promise((resolve, reject) => { - this.pendingRequests.set(id, { resolve, reject }); + const timeoutId = setTimeout(() => { + if (this.pendingRequests.has(id)) { + this.pendingRequests.delete(id); + reject(new Error('Request timeout')); + } + }, 30000); + + this.pendingRequests.set(id, { + resolve: (value) => { + clearTimeout(timeoutId); + resolve(value); + }, + reject: (err) => { + clearTimeout(timeoutId); + reject(err); + }, + }); fetch(this.messageEndpoint, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body, }).catch(reject); - - // Timeout after 30s - setTimeout(() => { - if (this.pendingRequests.has(id)) { - this.pendingRequests.delete(id); - reject(new Error('Request timeout')); - } - }, 30000); });libs/plugins/src/dashboard/app/dashboard.app.ts (1)
174-174: Clean up comment formatting.The comment on line 174 starts with a dash (
-) which appears to be a typo or formatting inconsistency.🔎 Suggested fix
- standalone: true, // - dashboard is part of root scope so GraphDataProvider can access all tools/resources + standalone: true, // Dashboard is part of root scope so GraphDataProvider can access all tools/resources
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
libs/uipack/src/renderers/mdx.renderer.d.ts.mapis excluded by!**/*.map,!**/*.maplibs/uipack/src/renderers/utils/detect.d.ts.mapis excluded by!**/*.map,!**/*.maplibs/uipack/src/renderers/utils/hash.d.ts.mapis excluded by!**/*.map,!**/*.maplibs/uipack/src/renderers/utils/index.d.ts.mapis excluded by!**/*.map,!**/*.maplibs/uipack/src/renderers/utils/transpiler.d.ts.mapis excluded by!**/*.map,!**/*.mapyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (55)
.gitignoreapps/demo/src/main.tsapps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tsapps/e2e/demo-e2e-standalone/jest.e2e.config.tsapps/e2e/demo-e2e-standalone/project.jsonapps/e2e/demo-e2e-standalone/src/apps/isolated/index.tsapps/e2e/demo-e2e-standalone/src/apps/isolated/resources/isolated-info.resource.tsapps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.tsapps/e2e/demo-e2e-standalone/src/apps/parent/index.tsapps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.tsapps/e2e/demo-e2e-standalone/src/main.tsapps/e2e/demo-e2e-standalone/tsconfig.app.jsonapps/e2e/demo-e2e-standalone/tsconfig.e2e.jsonapps/e2e/demo-e2e-standalone/tsconfig.jsonapps/e2e/demo-e2e-standalone/webpack.config.jslibs/adapters/project.jsonlibs/adapters/tsconfig.lib.jsonlibs/cli/src/commands/__tests__/create.spec.tslibs/cli/src/commands/create.tslibs/mcp-from-openapi/project.jsonlibs/mcp-from-openapi/tsconfig.lib.jsonlibs/plugins/project.jsonlibs/plugins/src/dashboard/app/dashboard.app.tslibs/plugins/src/dashboard/app/index.tslibs/plugins/src/dashboard/dashboard.plugin.tslibs/plugins/src/dashboard/dashboard.symbol.tslibs/plugins/src/dashboard/dashboard.types.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/html/index.tslibs/plugins/src/dashboard/index.tslibs/plugins/src/dashboard/providers/graph-data.provider.tslibs/plugins/src/dashboard/providers/index.tslibs/plugins/src/dashboard/shared/index.tslibs/plugins/src/dashboard/shared/types.tslibs/plugins/src/dashboard/tools/graph.tool.tslibs/plugins/src/dashboard/tools/index.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/tools/list-tools.tool.tslibs/plugins/src/index.tslibs/plugins/tsconfig.lib.jsonlibs/sdk/project.jsonlibs/sdk/src/app/instances/app.local.instance.tslibs/sdk/src/common/metadata/app.metadata.tslibs/sdk/src/common/utils/decide-request-intent.utils.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/tsconfig.lib.jsonlibs/uipack/project.jsonlibs/uipack/src/renderers/utils/detect.d.tslibs/uipack/src/renderers/utils/hash.d.tslibs/uipack/src/renderers/utils/index.d.tslibs/uipack/src/renderers/utils/transpiler.d.tslibs/uipack/tsconfig.lib.jsonpackage.jsontsconfig.base.json
💤 Files with no reviewable changes (4)
- libs/uipack/src/renderers/utils/hash.d.ts
- libs/uipack/src/renderers/utils/detect.d.ts
- libs/uipack/src/renderers/utils/index.d.ts
- libs/uipack/src/renderers/utils/transpiler.d.ts
✅ Files skipped from review due to trivial changes (1)
- apps/e2e/demo-e2e-standalone/project.json
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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/plugins/src/dashboard/shared/index.tslibs/plugins/src/dashboard/providers/index.tslibs/plugins/src/dashboard/html/index.tsapps/e2e/demo-e2e-standalone/src/apps/isolated/index.tslibs/plugins/src/dashboard/tools/index.tslibs/cli/src/commands/__tests__/create.spec.tsapps/e2e/demo-e2e-standalone/src/main.tslibs/sdk/src/app/instances/app.local.instance.tsapps/e2e/demo-e2e-standalone/src/apps/parent/index.tslibs/cli/src/commands/create.tslibs/plugins/src/dashboard/dashboard.types.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/plugins/src/dashboard/dashboard.plugin.tslibs/plugins/src/index.tsapps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.tslibs/plugins/src/dashboard/app/index.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/dashboard.symbol.tsapps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.tslibs/plugins/src/dashboard/tools/graph.tool.tslibs/plugins/src/dashboard/providers/graph-data.provider.tslibs/sdk/src/common/utils/decide-request-intent.utils.tslibs/plugins/src/dashboard/app/dashboard.app.tsapps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tsapps/e2e/demo-e2e-standalone/jest.e2e.config.tslibs/plugins/src/dashboard/index.tslibs/sdk/src/plugin/plugin.registry.tsapps/e2e/demo-e2e-standalone/src/apps/isolated/resources/isolated-info.resource.tslibs/plugins/src/dashboard/shared/types.tsapps/demo/src/main.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/plugins/src/dashboard/shared/index.tslibs/plugins/src/dashboard/providers/index.tslibs/plugins/src/dashboard/html/index.tslibs/plugins/src/dashboard/tools/index.tslibs/adapters/project.jsonlibs/cli/src/commands/__tests__/create.spec.tslibs/sdk/src/app/instances/app.local.instance.tslibs/cli/src/commands/create.tslibs/plugins/src/dashboard/dashboard.types.tslibs/mcp-from-openapi/tsconfig.lib.jsonlibs/mcp-from-openapi/project.jsonlibs/sdk/src/transport/flows/handle.sse.flow.tslibs/plugins/src/dashboard/dashboard.plugin.tslibs/plugins/src/index.tslibs/sdk/project.jsonlibs/plugins/src/dashboard/app/index.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/dashboard.symbol.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.tslibs/plugins/src/dashboard/tools/graph.tool.tslibs/plugins/src/dashboard/providers/graph-data.provider.tslibs/sdk/src/common/utils/decide-request-intent.utils.tslibs/plugins/src/dashboard/app/dashboard.app.tslibs/plugins/project.jsonlibs/plugins/tsconfig.lib.jsonlibs/uipack/project.jsonlibs/sdk/tsconfig.lib.jsonlibs/plugins/src/dashboard/index.tslibs/sdk/src/plugin/plugin.registry.tslibs/plugins/src/dashboard/shared/types.tslibs/adapters/tsconfig.lib.jsonlibs/sdk/src/common/metadata/app.metadata.tslibs/uipack/tsconfig.lib.json
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/instances/app.local.instance.tslibs/sdk/src/transport/flows/handle.sse.flow.tslibs/sdk/src/common/utils/decide-request-intent.utils.tslibs/sdk/src/plugin/plugin.registry.tslibs/sdk/src/common/metadata/app.metadata.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Achieve 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including error cases and constructor validation
Include error classinstanceofchecks in tests to validate error handling
Files:
apps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.ts
apps/demo/**
⚙️ CodeRabbit configuration file
apps/demo/**: apps/demo directory contains a demo application for testing purposes. It can be used as a reference for SDK usage examples.
Files:
apps/demo/src/main.ts
🧠 Learnings (31)
📓 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-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/*/index.ts : Implement entry point re-exports in index.ts files for each module (adapters, build, bundler, etc.)
Applied to files:
libs/plugins/src/dashboard/shared/index.tslibs/plugins/src/dashboard/providers/index.tslibs/plugins/src/dashboard/html/index.tslibs/plugins/src/dashboard/tools/index.tslibs/adapters/project.jsonapps/e2e/demo-e2e-standalone/src/main.tslibs/plugins/src/index.tslibs/plugins/src/dashboard/app/index.tsapps/e2e/demo-e2e-standalone/webpack.config.jslibs/uipack/project.jsonlibs/plugins/src/dashboard/index.tslibs/uipack/tsconfig.lib.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 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/plugins/src/dashboard/shared/index.tslibs/plugins/src/dashboard/providers/index.tslibs/plugins/src/dashboard/html/index.tslibs/plugins/src/dashboard/tools/index.tslibs/plugins/src/index.tslibs/sdk/project.jsonlibs/plugins/src/dashboard/app/index.ts.gitignoreapps/e2e/demo-e2e-standalone/webpack.config.jslibs/plugins/tsconfig.lib.jsonlibs/uipack/project.jsonlibs/sdk/tsconfig.lib.jsonlibs/plugins/src/dashboard/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/plugins/src/dashboard/shared/index.tslibs/plugins/src/dashboard/app/index.ts.gitignoreapps/e2e/demo-e2e-standalone/tsconfig.jsonlibs/plugins/tsconfig.lib.jsonlibs/uipack/project.jsonlibs/plugins/src/dashboard/index.tslibs/plugins/src/dashboard/shared/types.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/plugins/src/dashboard/shared/index.tslibs/plugins/src/dashboard/html/index.tslibs/plugins/src/dashboard/app/index.tsapps/e2e/demo-e2e-standalone/webpack.config.jslibs/plugins/tsconfig.lib.jsonlibs/uipack/project.jsonlibs/adapters/tsconfig.lib.jsonlibs/uipack/tsconfig.lib.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/renderers/**/*.renderer.ts : Server-side renderers must export both named exports (ReactRenderer, MdxRenderer) and default instances (reactRenderer, mdxRenderer) with consistent interface patterns
Applied to files:
libs/plugins/src/dashboard/shared/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/**/*.{ts,tsx} : Avoid using any type without justification; all props, return types, and generics must be properly typed
Applied to files:
libs/plugins/src/dashboard/shared/index.ts.gitignore
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validation
Applied to files:
libs/plugins/src/dashboard/shared/index.ts.gitignorelibs/uipack/project.jsonlibs/uipack/tsconfig.lib.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Avoid using `any` type without justification in TypeScript
Applied to files:
libs/plugins/src/dashboard/shared/index.ts.gitignorelibs/uipack/project.jsonlibs/uipack/tsconfig.lib.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{adapters,theme,build}/**/*.test.{ts,tsx,js,jsx} : Test component behavior across all platform configurations (OpenAI, Claude, etc.)
Applied to files:
apps/e2e/demo-e2e-standalone/tsconfig.e2e.jsonlibs/adapters/project.jsonlibs/cli/src/commands/__tests__/create.spec.tslibs/cli/src/commands/create.tsapps/e2e/demo-e2e-standalone/tsconfig.app.jsonapps/e2e/demo-e2e-standalone/tsconfig.jsonapps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tslibs/uipack/project.jsonapps/e2e/demo-e2e-standalone/jest.e2e.config.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-standalone/tsconfig.e2e.jsonlibs/cli/src/commands/__tests__/create.spec.tslibs/cli/src/commands/create.tsapps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tsapps/e2e/demo-e2e-standalone/jest.e2e.config.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} : Include error class `instanceof` checks in tests to validate error handling
Applied to files:
apps/e2e/demo-e2e-standalone/tsconfig.e2e.jsonlibs/cli/src/commands/__tests__/create.spec.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:
apps/e2e/demo-e2e-standalone/src/apps/isolated/index.tsapps/e2e/demo-e2e-standalone/src/apps/parent/index.tsapps/demo/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: Use Nx for build system with commands like `nx build sdk`, `nx test`, `nx run-many -t test`
Applied to files:
libs/adapters/project.jsonlibs/mcp-from-openapi/project.jsonlibs/sdk/project.jsonlibs/plugins/project.jsonpackage.jsonlibs/uipack/project.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{build,base-template,adapters}/**/*.{ts,tsx,js,jsx} : Use buildCdnScriptsFromTheme with appropriate options (inline: true for blocked-network platforms like Claude)
Applied to files:
libs/adapters/project.jsonlibs/plugins/src/dashboard/dashboard.types.tslibs/mcp-from-openapi/project.jsonlibs/sdk/project.json.gitignoreapps/e2e/demo-e2e-standalone/webpack.config.jslibs/plugins/project.jsonlibs/uipack/project.jsonlibs/uipack/tsconfig.lib.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} : SSR components must be tested for correct HTML output and client-side hydration must be tested separately
Applied to files:
libs/cli/src/commands/__tests__/create.spec.tsapps/e2e/demo-e2e-standalone/e2e/standalone.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/**/*.test.{ts,tsx} : Use React Testing Library for component tests and include SSR/hydration tests for all interactive components
Applied to files:
libs/cli/src/commands/__tests__/create.spec.tsapps/e2e/demo-e2e-standalone/tsconfig.app.jsonapps/e2e/demo-e2e-standalone/e2e/standalone.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/**/*.test.{ts,tsx} : Do not skip SSR/hydration testing for components; all interactive widgets must be tested in both rendering contexts
Applied to files:
libs/cli/src/commands/__tests__/create.spec.tsapps/e2e/demo-e2e-standalone/e2e/standalone.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 : Use `changeScope` property instead of `scope` in change events to avoid confusion with Scope class
Applied to files:
libs/sdk/src/app/instances/app.local.instance.tslibs/plugins/tsconfig.lib.jsonlibs/sdk/tsconfig.lib.jsonlibs/sdk/src/plugin/plugin.registry.tslibs/adapters/tsconfig.lib.jsonlibs/uipack/tsconfig.lib.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{theme,build,bundler,base-template}/**/*.{ts,tsx,js,jsx} : Use theme.cdn configuration instead of hard-coding CDN URLs
Applied to files:
libs/plugins/src/dashboard/dashboard.types.ts.gitignoreapps/e2e/demo-e2e-standalone/webpack.config.jslibs/uipack/project.jsonlibs/uipack/tsconfig.lib.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 libs/sdk/src/**/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
libs/mcp-from-openapi/tsconfig.lib.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 libs/sdk/src/**/*.ts : MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead of `unknown`
Applied to files:
libs/mcp-from-openapi/tsconfig.lib.jsonlibs/plugins/src/dashboard/shared/types.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 : Use strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
libs/mcp-from-openapi/tsconfig.lib.json.gitignoreapps/e2e/demo-e2e-standalone/tsconfig.jsonlibs/plugins/tsconfig.lib.jsonlibs/adapters/tsconfig.lib.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: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/plugins/src/dashboard/dashboard.plugin.tslibs/plugins/src/dashboard/app/dashboard.app.tslibs/plugins/src/dashboard/shared/types.tslibs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.test.{ts,tsx,js,jsx} : Test HTML escaping for user-provided content in security-critical paths
Applied to files:
.gitignoreapps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tslibs/uipack/project.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:
.gitignorelibs/plugins/tsconfig.lib.jsonlibs/adapters/tsconfig.lib.jsonlibs/uipack/tsconfig.lib.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : No React dependency in frontmcp/uipack - use frontmcp/ui for React components and hooks
Applied to files:
libs/plugins/tsconfig.lib.jsonlibs/uipack/project.jsonlibs/adapters/tsconfig.lib.jsonlibs/uipack/tsconfig.lib.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 libs/sdk/src/**/*.ts : Use `getCapabilities()` method for dynamic capability exposure instead of hardcoding capabilities
Applied to files:
libs/sdk/tsconfig.lib.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:
apps/demo/src/main.tslibs/adapters/tsconfig.lib.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/react/hooks/**/*.{ts,tsx} : MCP bridge hooks (useMcpBridge, useCallTool, useToolInput, useToolOutput, useTheme) must be properly typed and handle loading/error states
Applied to files:
libs/sdk/src/common/metadata/app.metadata.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : Do not expose internal error details to users - sanitize error messages
Applied to files:
libs/uipack/tsconfig.lib.json
🧬 Code graph analysis (15)
apps/e2e/demo-e2e-standalone/src/apps/isolated/index.ts (2)
apps/e2e/demo-e2e-standalone/src/apps/parent/index.ts (1)
App(10-15)libs/plugins/src/dashboard/app/dashboard.app.ts (1)
App(144-176)
libs/cli/src/commands/__tests__/create.spec.ts (1)
libs/cli/src/commands/create.ts (1)
runCreate(1558-1594)
apps/e2e/demo-e2e-standalone/src/main.ts (1)
apps/demo/src/main.ts (1)
FrontMcp(11-34)
libs/sdk/src/app/instances/app.local.instance.ts (1)
libs/sdk/src/plugin/plugin.registry.ts (1)
PluginRegistry(17-181)
apps/e2e/demo-e2e-standalone/src/apps/parent/index.ts (2)
apps/e2e/demo-e2e-standalone/src/apps/isolated/index.ts (1)
App(14-24)libs/plugins/src/dashboard/app/dashboard.app.ts (1)
App(144-176)
libs/sdk/src/transport/flows/handle.sse.flow.ts (2)
libs/sdk/src/common/schemas/session-header.schema.ts (1)
validateMcpSessionHeader(37-41)libs/sdk/src/common/schemas/http-output.schema.ts (1)
httpRespond(256-370)
apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.ts (4)
apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (1)
Tool(4-23)libs/plugins/src/dashboard/tools/graph.tool.ts (1)
Tool(19-56)libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
Tool(36-99)libs/plugins/src/dashboard/tools/list-tools.tool.ts (1)
Tool(38-94)
libs/plugins/src/dashboard/tools/list-resources.tool.ts (7)
apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.ts (1)
Tool(4-23)apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (1)
Tool(4-23)libs/plugins/src/dashboard/tools/graph.tool.ts (1)
Tool(19-56)libs/plugins/src/dashboard/tools/list-tools.tool.ts (1)
Tool(38-94)libs/plugins/src/dashboard/dashboard.symbol.ts (1)
ParentScopeToken(12-12)libs/plugins/src/dashboard/index.ts (1)
ParentScopeToken(17-17)libs/sdk/src/app/instances/app.local.instance.ts (1)
resources(72-74)
apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (3)
libs/plugins/src/dashboard/tools/graph.tool.ts (1)
Tool(19-56)libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
Tool(36-99)libs/plugins/src/dashboard/tools/list-tools.tool.ts (1)
Tool(38-94)
libs/plugins/src/dashboard/html/html.generator.ts (1)
libs/plugins/src/dashboard/dashboard.types.ts (1)
DashboardPluginOptions(58-58)
libs/plugins/src/dashboard/tools/list-tools.tool.ts (5)
apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (1)
Tool(4-23)libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
Tool(36-99)libs/plugins/src/dashboard/dashboard.symbol.ts (1)
ParentScopeToken(12-12)libs/plugins/src/dashboard/index.ts (1)
ParentScopeToken(17-17)libs/sdk/src/app/instances/app.local.instance.ts (1)
tools(68-70)
libs/plugins/src/dashboard/tools/graph.tool.ts (1)
libs/plugins/src/dashboard/shared/types.ts (1)
GraphData(100-104)
libs/sdk/src/common/utils/decide-request-intent.utils.ts (1)
libs/sdk/src/common/interfaces/server.interface.ts (1)
ServerRequest(6-14)
libs/sdk/src/plugin/plugin.registry.ts (1)
libs/sdk/src/scope/scope.instance.ts (1)
Scope(34-349)
libs/sdk/src/common/metadata/app.metadata.ts (1)
libs/sdk/src/common/schemas/annotated-class.schema.ts (7)
annotatedFrontMcpProvidersSchema(27-48)annotatedFrontMcpAuthProvidersSchema(50-71)annotatedFrontMcpPluginsSchema(73-94)annotatedFrontMcpAdaptersSchema(96-117)annotatedFrontMcpToolsSchema(119-127)annotatedFrontMcpResourcesSchema(129-147)annotatedFrontMcpPromptsSchema(149-160)
🪛 ast-grep (0.40.3)
libs/plugins/src/dashboard/tools/list-resources.tool.ts
[warning] 89-89: 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(input.filter, 'i')
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)
libs/plugins/src/dashboard/tools/list-tools.tool.ts
[warning] 70-70: 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(input.filter, 'i')
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)
⏰ 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 (50)
.gitignore (1)
70-74: Clear and appropriate handling of generated TypeScript declaration files.The patterns correctly prevent TypeScript declaration files and their source maps from being committed when they're generated in source directories. The comment clearly explains the intent, and the scope is precise enough to avoid false positives.
libs/cli/src/commands/create.ts (4)
280-288: LGTM! Clean E2E tsconfig template.The template correctly extends the base config and adds necessary types for Jest E2E tests. Including
jest.e2e.config.tsin the include array provides type-checking for the config file itself, which is a good practice.
482-484: Documentation updated correctly.The README template now includes
tsconfig.e2e.jsonin the project structure, which is consistent with the new E2E scaffolding functionality.
1165-1166: Dynamic README generation updated correctly.The generated README correctly reflects the new
tsconfig.e2e.jsonfile in the project structure.
1377-1377: Scaffolding implementation looks good.The call to
scaffoldFileIfMissingfollows the same pattern as other scaffolded files and correctly places the E2E tsconfig at the project root alongside the E2E test directory and Jest config.libs/cli/src/commands/__tests__/create.spec.ts (1)
146-150: Test coverage for E2E tsconfig scaffolding is adequate.The test verifies that
tsconfig.e2e.jsonis created during project scaffolding, following the same pattern as other file creation tests. This provides appropriate coverage for the new E2E configuration scaffolding feature.libs/sdk/src/plugin/plugin.registry.ts (4)
35-35: LGTM: Clean field declaration.The
hookScopefield is properly encapsulated and immutable, consistent with the existingscopefield.
42-46: LGTM: Clear documentation.The JSDoc effectively explains the purpose and usage of the
hookScopeparameter, particularly the gateway-level hook triggering behavior for non-standalone apps.
50-50: LGTM: Safe default with backward compatibility.The fallback to
this.scopeensures existing code continues to work unchanged while enabling the new parent-scope hook registration for non-standalone apps.
170-171: LGTM: Correct implementation of hookScope feature.The change to use
this.hookScopeinstead ofthis.scopeproperly implements the gateway-level hook registration for non-standalone apps. The updated comment clearly explains the behavior.libs/sdk/src/app/instances/app.local.instance.ts (2)
40-42: LGTM: Correct conditional logic for hookScope derivation.The strict equality check (
standalone === false) is intentional and correct:
trueorundefined: Treats the app as standalone → hooks use the app's own scope (default behavior)false: Treats the app as non-standalone → hooks use the parent (gateway) scopeThis design ensures apps are standalone by default unless explicitly configured otherwise, which is a safe default.
43-43: LGTM: Correct usage of updated PluginRegistry constructor.The hookScope is properly passed as the fourth argument, aligning with the updated constructor signature. This enables gateway-level HTTP hook triggering for non-standalone apps.
libs/sdk/src/common/metadata/app.metadata.ts (1)
108-126: Schema construction refactoring looks good.The restructuring from object literal +
satisfiesto.object(...) satisfies ...pattern is a stylistic improvement that maintains the same validation logic while being more explicit. TheRawZodShape<T>type constraint ensures type safety is preserved.Also applies to: 175-188
package.json (3)
87-87: Tailwind CSS v4 introduces breaking changes.Tailwind CSS v4.1.18 is being added, which includes significant breaking changes:
- Important modifier now comes at the END:
text-xl!instead of!text-xl- Opacity utilities removed; use slash syntax:
bg-blue-500/20instead ofbg-opacity-20- CSS variables use parentheses:
bg-(--brand-color)instead ofbg-[--brand-color]- New
@import "tailwindcss"syntax replaces separate@tailwinddirectives- Default ring width is now 1px (use
ring-3for old 3px)Ensure all Tailwind usage in the codebase is updated to v4 syntax, especially if migrating from v3.
Based on learnings: Tailwind v4 has architectural changes requiring CSS-first configuration.
55-55: No changes needed. Bothvite: "^7.3.0"andpostcss: "^8.5.6"are valid and represent current stable versions as of January 2026.Likely an incorrect or invalid review comment.
55-55: Security audit confirms newly added dependencies are secure.Verified that none of the 13 newly added packages have known security vulnerabilities:
- @nx/vite@22.3.3
- @tailwindcss/postcss@4.1.18
- @testing-library/jest-dom@6.9.1
- @testing-library/react@16.3.1
- @types/dagre@0.7.53
- @vitejs/plugin-react@5.1.2
- autoprefixer@10.4.23
- identity-obj-proxy@3.0.0
- jest-environment-jsdom@30.2.0
- nyc@17.1.0
- postcss@8.5.6
- tailwindcss@4.1.18
- vite@7.3.0
apps/e2e/demo-e2e-standalone/webpack.config.js (1)
14-26: Well-configured E2E test build setup.The NxAppWebpackPlugin configuration is appropriate for an E2E test application:
target: 'node'correctly targets Node.js runtimeexternalDependencies: 'all'avoids bundling dependencies (faster builds, smaller bundles)buildLibsFromSource: trueensures latest library code is used during testingoptimization: falseandoutputHashing: 'none'are reasonable for test buildsgeneratePackageJson: falseis appropriate since this is a test harnesslibs/sdk/tsconfig.lib.json (1)
6-17: LGTM!The configuration correctly enables declaration emission with source maps. The path mappings are appropriately set:
@frontmcp/sdkpoints to source for self-referencing, while sibling packages (@frontmcp/uipack,@frontmcp/ui) reference their compiled.d.tsfiles indist/, ensuring correct type resolution during builds.libs/uipack/tsconfig.lib.json (1)
6-6: LGTM!Setting
noEmit: falseenables declaration file output, aligning with the monorepo's declaration emission workflow.tsconfig.base.json (1)
6-13: LGTM!The base configuration establishes sensible defaults:
noEmit: trueprevents accidental emission at the root level, whiledeclarationMap: falseand a definedoutDirprovide a baseline that individual library configs can override as needed for their specific build requirements.libs/plugins/tsconfig.lib.json (1)
6-12: LGTM!The configuration correctly enables declaration emission and maps
@frontmcp/sdkto the SDK's compiled type definitions indist/. This ensures proper type resolution when building the plugins library.libs/sdk/src/common/utils/decide-request-intent.utils.ts (1)
163-173: LGTM!The session ID resolution correctly prioritizes the
Mcp-Session-Idheader while falling back to the query parameter for legacy SSE/messageendpoints. The conditional extraction (postToMessage ? getQuerySessionId(req) : undefined) ensures the query parameter fallback only applies to the legacy path, maintaining backward compatibility without affecting other endpoints.apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.ts (1)
1-23: LGTM!Clean implementation following the established pattern from
ParentHelloTool. The input/output schemas are properly defined, type parameters correctly match the schema shapes, and theas constassertion ensures the literal type forscopeis preserved.libs/mcp-from-openapi/tsconfig.lib.json (1)
6-8: LGTM! Configuration correctly enables declaration emission.These changes properly configure TypeScript to emit declaration files and declaration maps, which are required for the new
build-typestarget. SettingnoEmit: falseallowstscto generate.d.tsfiles, anddeclarationMap: trueimproves IDE navigation and debugging support.apps/e2e/demo-e2e-standalone/jest.e2e.config.ts (1)
1-44: LGTM! Well-configured E2E test setup.This Jest configuration is appropriately structured for E2E testing:
- ESM compatibility using
createRequirefor loading the coverage preset- SWC-based transformation with TypeScript decorator support (required for the SDK)
- Single worker (
maxWorkers: 1) ensures proper test isolation for E2E scenarios- Reasonable 60-second timeout for E2E operations
forceExit: trueprevents hanging processes after test completion- Transform ignore pattern correctly handles the
joseESM module- Module name mapper provides clean path aliases for internal libraries
libs/sdk/project.json (1)
79-86: Correct dependency pattern for SDK with external lib dependencies.The
build-typestarget correctly includesdependsOn: ["^build"]to ensure that dependent libraries (like@frontmcp/uipackreferenced in the external array at line 27) are fully built before generating SDK type declarations. This ensures type resolution works correctly.This differs from
libs/uipack/project.jsonandlibs/mcp-from-openapi/project.jsonwherebuild-typeshas no dependencies, which is appropriate since those libraries don't depend on other internal packages.libs/plugins/src/dashboard/shared/types.ts (1)
1-186: LGTM! Well-structured type definitions.The type definitions are comprehensive and follow TypeScript best practices:
- Proper use of
unknownfor flexible schema and metadata fields (lines 46-47, 49, 148)- Clear domain modeling for graph structures, events, and telemetry
- No use of
anytypes- Appropriate union types for discriminated unions (GraphNodeType, GraphEdgeType, DashboardEvent)
libs/plugins/src/dashboard/shared/index.ts (1)
1-1: LGTM! Proper barrel export pattern.The barrel export follows the established pattern for centralizing type exports.
Based on learnings, barrel files should export public API items for user consumption.
libs/adapters/tsconfig.lib.json (1)
6-8: LGTM! Appropriate configuration for declaration generation.The changes enable declaration file emission with source maps:
noEmit: falseallows TypeScript to emit declaration filesdeclarationMap: truegenerates source maps for better IDE navigation and debuggingThis aligns with the PR's build-types target for generating TypeScript declarations across libraries.
libs/plugins/src/dashboard/app/index.ts (1)
1-1: LGTM! Proper barrel export for dashboard app.The barrel export correctly re-exports the dashboard app components for public consumption.
Based on learnings, entry point re-exports should expose public API items through index.ts files.
libs/plugins/src/dashboard/html/index.ts (1)
1-1: LGTM: Clean barrel export following SDK patterns.The barrel export correctly exposes
generateDashboardHtmlfrom the internal module, following the established pattern for libs/plugins public API exports.Based on learnings: "Export public API through barrel files - export users' needed items, no legacy exports with different names"
apps/e2e/demo-e2e-standalone/tsconfig.json (1)
1-16: LGTM: Standard TypeScript project configuration.The configuration correctly uses TypeScript project references to delegate to separate app and E2E test configs, which is the recommended pattern for composite projects.
apps/e2e/demo-e2e-standalone/tsconfig.e2e.json (1)
1-12: LGTM: Appropriate E2E test configuration.The configuration correctly enables decorator support (required for
@Appdecorators), uses NodeNext module resolution, and includes Jest types for the E2E testing environment.apps/e2e/demo-e2e-standalone/tsconfig.app.json (1)
1-14: LGTM: Well-configured app TypeScript settings.The configuration correctly:
- Uses NodeNext module resolution for modern Node.js compatibility
- Enables React JSX transform with
react-jsx- Supports decorators required by the
@Appdecorator pattern- Appropriately excludes test files while including app source
apps/e2e/demo-e2e-standalone/src/apps/parent/index.ts (1)
10-14: LGTM: Standalone default behavior is correctly implemented.The ParentApp configuration correctly omits the
standaloneflag. The@Appdecorator's metadata schema explicitly defaultsstandalonetofalsewhen not specified, which ensures the app is included in the parent/root scope as documented in the codebase.libs/plugins/src/dashboard/providers/index.ts (1)
1-1: LGTM: Clean barrel export following best practices.The re-export pattern is correct and aligns with the project's barrel file conventions.
libs/plugins/src/index.ts (1)
7-8: LGTM: Dashboard exports follow established plugin patterns.The export structure is consistent with existing plugins and correctly exposes both the plugin class and app class along with their configuration types.
libs/plugins/src/dashboard/tools/index.ts (1)
1-3: LGTM: Clean barrel export establishing the tools public API.The re-export pattern correctly exposes the three dashboard tools with clear, descriptive names.
apps/e2e/demo-e2e-standalone/src/apps/isolated/index.ts (1)
1-24: LGTM: Well-documented standalone app for E2E testing.The app configuration correctly implements scope isolation with
standalone: true, and the inline documentation clearly explains the routing behavior and purpose. The structure follows established patterns from other apps in the codebase.apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (1)
1-23: LGTM!The implementation correctly follows the SDK patterns and mirrors the
IsolatedHelloToolstructure. Zod v4 schema definitions are valid, type parameters align with schemas, and theas constassertion ensures proper literal type inference.apps/demo/src/main.ts (1)
11-20: LGTM!The demo app configuration is properly updated:
DashboardAppintegration from@frontmcp/plugins- Expanded app registration for comprehensive demo coverage
- Transport configuration restructured with
enableLegacySSEat the appropriate levellibs/sdk/src/transport/flows/handle.sse.flow.ts (2)
67-76: LGTM!The
getQuerySessionIdhelper is well-implemented with proper error handling. UsingURLconstructor with a dummy base is a clean approach for parsing query parameters without requiring a full URL.
117-124: LGTM!The validation and prioritization logic correctly:
- Rejects invalid query param sessionId with 404 (consistent with header validation)
- Prioritizes header over query param via
mcpSessionHeader ?? validatedQuerySessionIdThis maintains backward compatibility while enabling legacy SSE
/message?sessionId=xxxpattern.apps/e2e/demo-e2e-standalone/src/main.ts (1)
7-39: LGTM!Excellent documentation of expected routing behavior and a comprehensive transport configuration for E2E testing. The server properly configures both root and isolated scopes with appropriate SSE and message endpoints.
libs/plugins/src/dashboard/dashboard.symbol.ts (1)
1-12: LGTM!Clean DI token declarations following standard Symbol-based patterns. The tokens are well-documented and correctly named with the
Tokensuffix convention.libs/plugins/src/dashboard/tools/graph.tool.ts (1)
40-51: LGTM - Schema stripping implementation is correct.The schema stripping logic properly spreads the original data and sets schema fields to
undefined. This approach maintains immutability of the cached graph data.libs/plugins/src/dashboard/dashboard.types.ts (2)
52-54: Nested schema transforms ensure defaults are applied correctly.The
.transform()calls with inner.parse()ensure that even whenauthorcdnisundefined, the nested defaults are still applied. This is a good pattern for deeply nested configuration.
36-37: The dashboard token authentication is not implemented - add server-side token validation to the middleware.The
createDashboardMiddlewarefunction (lines 44-66 ofdashboard.app.ts) only checks if the dashboard is enabled, but does not validate the token parameter from incoming requests. The token is embedded in the generated HTML and passed as a query parameter (?token=xxx), but the middleware does not extract, compare, or authenticate against the configuredoptions.auth.token.Additionally, ensure:
- Use constant-time comparison (e.g.,
timingSafeEqualfrom Node'scryptomodule, already used elsewhere in the SDK) when comparing tokens- Do not log token values in any debug or error messages
- Enforce HTTPS in production before exposing query-parameter-based token authentication
⛔ Skipped due to learnings
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.libs/plugins/src/dashboard/index.ts (1)
1-28: Well-organized barrel file with clean public API surface.The exports are logically grouped (main plugin, app, types, symbols, providers, tools) and use type-only exports where appropriate (line 20). This aligns with the learning about barrel files exporting public API cleanly.
libs/plugins/src/dashboard/app/dashboard.app.ts (1)
171-173: Verify public authentication mode is intentional.The dashboard is configured with
mode: 'public', meaning no authentication is required to access it. This exposes server structure, tools, and resources without any access control.Please confirm this is intentional and consider:
- If the dashboard should have authentication in production environments
- Whether there should be configuration options to enable auth
- If this poses any security risks for the data exposed through the dashboard tools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
libs/plugins/src/dashboard/app/dashboard.app.ts (2)
125-182: Public API requires documentation update.
DashboardAppis exported from@frontmcp/plugins(a publishable SDK library). Per coding guidelines, public API changes require matching documentation indocs/draft/docs/**.The README.md provides good documentation, but official docs should be added to maintain consistency with other plugins (cache-plugin.mdx, codecall-plugin.mdx).
#!/bin/bash # Check if dashboard plugin documentation was added fd -e mdx -e md dashboard docs/draft/docs/ ls -la docs/draft/docs/plugins/ 2>/dev/null || echo "plugins dir not found"
54-55: Remove unnecessary type assertions on lines 54–55.The
ServerRequestinterface already typespathasstring,urlasstring, andmethodasHttpMethod(a string literal union). Theas stringcasts are redundant and should be removed.Fix
- const urlPath = (req.path || req.url || '/') as string; - const method = ((req.method as string) || 'GET').toUpperCase(); + const urlPath = req.path || req.url || '/'; + const method = (req.method || 'GET').toUpperCase();
🧹 Nitpick comments (7)
apps/e2e/demo-e2e-standalone/webpack.config.js (1)
12-12: Consider conditional devtool for production mode.The devtool is hardcoded to
'eval-cheap-module-source-map', which is development-optimized. Since line 11 allows production mode, consider using a production-appropriate devtool like'source-map'whenNODE_ENV === 'production'.However, if this E2E configuration is strictly for development/testing and production builds are not actually used, the current setting is acceptable.
🔎 Proposed conditional devtool configuration
- devtool: 'eval-cheap-module-source-map', + devtool: process.env.NODE_ENV === 'production' ? 'source-map' : 'eval-cheap-module-source-map',libs/mcp-from-openapi/project.json (1)
50-56: LGTM! Dedicated declaration build resolves the previous redundancy issue.The new
build-typestarget correctly handles TypeScript declaration emission separately from the main builds, addressing the race condition concern from the previous review.Note: The
--declarationflag on line 54 is redundant when using--emitDeclarationOnly, as the latter already implies declaration emission.🔎 Optional: Remove redundant flag
"options": { - "command": "tsc -p libs/mcp-from-openapi/tsconfig.lib.json --emitDeclarationOnly --declaration --declarationMap --outDir libs/mcp-from-openapi/dist" + "command": "tsc -p libs/mcp-from-openapi/tsconfig.lib.json --emitDeclarationOnly --declarationMap --outDir libs/mcp-from-openapi/dist" }apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.ts (1)
7-9: Simplify Zod schema:.optional()is redundant with.default()in Zod 4.In Zod 4,
.default()automatically handlesundefinedvalues and makes object fields optional. The.optional()modifier is unnecessary and can be removed for cleaner schema definitions.🔎 Proposed simplification
inputSchema: z.object({ - name: z.string().optional().default('World'), + name: z.string().default('World'), }),Based on Zod 4 library documentation provided.
apps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.ts (1)
45-125: LGTM! Comprehensive isolated scope testing with proper resource cleanup.The isolated scope tests thoroughly verify connection, tool/resource visibility, and operations within the isolated scope. Each test properly creates and disconnects the isolated client, ensuring no resource leaks.
Optional refactor: The client creation pattern is duplicated across five tests (lines 53-56, 65-68, 83-86, 98-101, 112-115). Consider extracting to a helper function or test fixture to reduce duplication:
🔎 Example helper function
async function createIsolatedClient(baseUrl: string): Promise<McpTestClient> { return await McpTestClient.create({ baseUrl: baseUrl + '/isolated', publicMode: true, }).buildAndConnect(); }However, the current approach maintains test independence and readability, which may be preferred for E2E tests.
libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
9-23:safeRegexhelper addresses ReDoS concern adequately.The implementation provides reasonable protection with the 100-character limit and try-catch. The static analysis warning on line 15 is a false positive since this wrapper specifically mitigates the ReDoS risk.
However, this same helper is duplicated in
list-tools.tool.ts. Consider extracting it to a shared utility to avoid duplication.🔎 Suggested shared utility extraction
Create a shared utility file:
// libs/plugins/src/dashboard/shared/safe-regex.ts export function safeRegex(pattern: string): RegExp | null { if (pattern.length > 100) { return null; } try { const regex = new RegExp(pattern, 'i'); regex.test('test'); return regex; } catch { return null; } }Then import in both tool files:
-function safeRegex(pattern: string): RegExp | null { - // ... implementation -} +import { safeRegex } from '../shared/safe-regex';libs/plugins/src/dashboard/providers/graph-data.provider.ts (1)
84-94: Improvement over previousas anycast.The code now uses
unknownwith a typed interfaceProvidersWithParent, which is safer than the previousas anyapproach. This addresses the past review comment appropriately.However, accessing private
parentProvidersfield via runtime cast remains fragile. Consider adding a TODO to track exposing this in the SDK's public API.🔎 Add TODO for SDK improvement
// If not found locally, traverse up the provider hierarchy // This is needed when dashboard is standalone (has its own isolated scope) // Access private parentProviders field via runtime cast - no public API exists + // TODO: Consider adding getParentProviders() to ProviderRegistryInterface in @frontmcp/sdk if (!scopeRegistries || scopeRegistries.length === 0) {libs/plugins/src/dashboard/tools/list-tools.tool.ts (1)
9-23: DuplicatesafeRegeximplementation.This is identical to the implementation in
list-resources.tool.ts. As noted in that file's review, consider extracting to a shared utility.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tsapps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.tsapps/e2e/demo-e2e-standalone/webpack.config.jslibs/adapters/project.jsonlibs/mcp-from-openapi/project.jsonlibs/plugins/project.jsonlibs/plugins/src/dashboard/README.mdlibs/plugins/src/dashboard/app/dashboard.app.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/providers/graph-data.provider.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/tools/list-tools.tool.tslibs/sdk/project.jsonlibs/sdk/src/common/metadata/app.metadata.tslibs/uipack/project.json
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/sdk/src/common/metadata/app.metadata.ts
- libs/uipack/project.json
- libs/sdk/project.json
- libs/plugins/project.json
🧰 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/plugins/src/dashboard/providers/graph-data.provider.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/app/dashboard.app.tslibs/plugins/src/dashboard/tools/list-tools.tool.tsapps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tsapps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.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/plugins/src/dashboard/providers/graph-data.provider.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/app/dashboard.app.tslibs/plugins/src/dashboard/tools/list-tools.tool.tslibs/adapters/project.jsonlibs/mcp-from-openapi/project.jsonlibs/plugins/src/dashboard/README.md
**/*.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-standalone/e2e/standalone.e2e.test.ts
🧠 Learnings (25)
📓 Common learnings
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
📚 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/plugins/src/dashboard/providers/graph-data.provider.tslibs/plugins/src/dashboard/tools/list-tools.tool.tslibs/adapters/project.jsonlibs/mcp-from-openapi/project.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.{ts,tsx,js,jsx} : Properly escape user-provided content to prevent XSS attacks in HTML generation
Applied to files:
libs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.test.{ts,tsx,js,jsx} : Test HTML escaping for user-provided content in security-critical paths
Applied to files:
libs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.tsapps/e2e/demo-e2e-standalone/e2e/standalone.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 : MCP response types should use strict MCP protocol types (GetPromptResult, ReadResourceResult) instead of `unknown`
Applied to files:
libs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/app/dashboard.app.tslibs/mcp-from-openapi/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 : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/plugins/src/dashboard/app/dashboard.app.tslibs/plugins/src/dashboard/tools/list-tools.tool.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Avoid using `any` type without justification in TypeScript
Applied to files:
libs/plugins/src/dashboard/app/dashboard.app.tslibs/adapters/project.jsonlibs/mcp-from-openapi/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: FrontMCP is a TypeScript-first schema validation framework - all types should align with MCP protocol definitions
Applied to files:
libs/plugins/src/dashboard/app/dashboard.app.tslibs/mcp-from-openapi/project.jsonlibs/plugins/src/dashboard/README.md
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validation
Applied to files:
libs/plugins/src/dashboard/tools/list-tools.tool.tslibs/mcp-from-openapi/project.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/*/index.ts : Implement entry point re-exports in index.ts files for each module (adapters, build, bundler, etc.)
Applied to files:
libs/adapters/project.jsonlibs/mcp-from-openapi/project.jsonapps/e2e/demo-e2e-standalone/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: Use Nx for build system with commands like `nx build sdk`, `nx test`, `nx run-many -t test`
Applied to files:
libs/adapters/project.jsonlibs/mcp-from-openapi/project.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{adapters,theme,build}/**/*.test.{ts,tsx,js,jsx} : Test component behavior across all platform configurations (OpenAI, Claude, etc.)
Applied to files:
libs/adapters/project.jsonapps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.tsapps/e2e/demo-e2e-standalone/webpack.config.js
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{build,base-template,adapters}/**/*.{ts,tsx,js,jsx} : Use buildCdnScriptsFromTheme with appropriate options (inline: true for blocked-network platforms like Claude)
Applied to files:
libs/adapters/project.jsonlibs/mcp-from-openapi/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 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/adapters/project.jsonlibs/mcp-from-openapi/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 : Use strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
libs/adapters/project.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} : Avoid using any type without justification; all props, return types, and generics must be properly typed
Applied to files:
libs/adapters/project.jsonlibs/mcp-from-openapi/project.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/adapters/project.jsonlibs/mcp-from-openapi/project.jsonapps/e2e/demo-e2e-standalone/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/**/index.ts : Export public API through barrel files - export users' needed items, no legacy exports with different names
Applied to files:
libs/adapters/project.jsonlibs/mcp-from-openapi/project.jsonapps/e2e/demo-e2e-standalone/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 **/*.test.{ts,tsx} : Test all code paths including error cases and constructor validation
Applied to files:
apps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : No React dependency in frontmcp/uipack - use frontmcp/ui for React components and hooks
Applied to files:
libs/mcp-from-openapi/project.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/mcp-from-openapi/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 libs/*/package.json : Each library in /libs/* is independent and publishable under frontmcp/* scope
Applied to files:
libs/mcp-from-openapi/project.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/mcp-from-openapi/project.json
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{theme,build,bundler,base-template}/**/*.{ts,tsx,js,jsx} : Use theme.cdn configuration instead of hard-coding CDN URLs
Applied to files:
apps/e2e/demo-e2e-standalone/webpack.config.js
📚 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:
apps/e2e/demo-e2e-standalone/webpack.config.js
🧬 Code graph analysis (5)
libs/plugins/src/dashboard/providers/graph-data.provider.ts (3)
libs/plugins/src/dashboard/shared/types.ts (4)
GraphData(100-104)GraphNode(62-67)GraphEdge(77-83)GraphMetadata(88-95)libs/sdk/src/app/instances/app.local.instance.ts (3)
tools(68-70)resources(72-74)prompts(76-78)libs/sdk/src/scope/scope.instance.ts (1)
apps(302-304)
libs/plugins/src/dashboard/html/html.generator.ts (3)
libs/plugins/src/dashboard/index.ts (1)
DashboardPluginOptions(9-9)libs/plugins/src/dashboard/dashboard.types.ts (1)
DashboardPluginOptions(58-58)libs/plugins/src/index.ts (1)
DashboardPluginOptions(8-8)
libs/plugins/src/dashboard/tools/list-resources.tool.ts (3)
libs/plugins/src/dashboard/tools/list-tools.tool.ts (1)
Tool(58-117)libs/plugins/src/dashboard/dashboard.symbol.ts (1)
ParentScopeToken(12-12)libs/sdk/src/app/instances/app.local.instance.ts (1)
resources(72-74)
libs/plugins/src/dashboard/tools/list-tools.tool.ts (4)
libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
Tool(56-122)apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (1)
Tool(4-23)libs/plugins/src/dashboard/dashboard.symbol.ts (1)
ParentScopeToken(12-12)libs/sdk/src/app/instances/app.local.instance.ts (1)
tools(68-70)
apps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.ts (1)
libs/sdk/src/app/instances/app.local.instance.ts (2)
tools(68-70)resources(72-74)
🪛 ast-grep (0.40.3)
libs/plugins/src/dashboard/tools/list-resources.tool.ts
[warning] 14-14: 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(pattern, 'i')
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)
libs/plugins/src/dashboard/tools/list-tools.tool.ts
[warning] 14-14: 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(pattern, 'i')
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)
🪛 markdownlint-cli2 (0.18.1)
libs/plugins/src/dashboard/README.md
105-105: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
172-172: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (25)
apps/e2e/demo-e2e-standalone/webpack.config.js (4)
1-3: LGTM!Standard imports for Nx webpack configuration.
5-10: LGTM!Output configuration is appropriate with conditional development-specific settings.
11-11: Good fix! Mode now respects NODE_ENV.The previous inconsistency where mode was hardcoded to 'development' has been resolved. The mode now correctly respects the NODE_ENV environment variable.
14-26: LGTM!The NxAppWebpackPlugin configuration is appropriate for a Node.js E2E test application:
target: 'node'andexternalDependencies: 'all'correctly configure for Node.js runtimeoptimization: falseis standard for test/development buildsbuildLibsFromSource: trueensures building from TypeScript sourcelibs/adapters/project.json (2)
73-80: LGTM! Build-types target is correctly configured.The dedicated
build-typestarget properly usestsc --emitDeclarationOnlyto generate TypeScript declarations separately from the bundling process. The flags (--declaration,--declarationMap,--outDir) are appropriate, and the dependency on["^build"]ensures parent libraries are built first.
83-83: LGTM! Build sequence correctly resolves previous declaration conflict.The updated dependency sequence
["build-types", "build-cjs", "build-esm"]ensures that TypeScript declarations are generated first bytsc, then the JavaScript bundles are created by esbuild. Since the declaration-related options have been removed from thebuild-cjstarget (as noted in the AI summary), this eliminates the declaration file conflict that was flagged in previous reviews.libs/mcp-from-openapi/project.json (1)
59-59: LGTM! Correct build dependency ordering.Adding
build-typesto the dependency chain ensures declarations are emitted before the main builds, preventing potential race conditions and ensuring clean artifact generation.apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.ts (1)
1-22: LGTM! Clean and type-safe tool implementation.The tool definition follows the framework conventions correctly with proper decorator usage, Zod schema validation, and type-safe execution. The implementation is straightforward and appropriate for a demonstration tool.
apps/e2e/demo-e2e-standalone/e2e/standalone.e2e.test.ts (5)
1-43: LGTM! Well-structured root scope tests.The root scope test suite properly verifies connection, tool visibility, and tool execution. The assertions correctly validate that only parent tools are visible in the root scope and that isolated tools remain hidden, ensuring proper scope isolation.
127-173: LGTM! Critical scope isolation properly verified.These tests ensure the fundamental isolation guarantees: tools are scoped correctly, and each scope maintains independent sessions. The assertions comprehensively validate that root and isolated scopes operate independently without cross-contamination.
175-203: LGTM! Cross-scope security boundaries properly enforced.These tests validate that tools cannot be invoked across scope boundaries, which is critical for security and isolation. Both tests correctly expect errors when attempting unauthorized cross-scope tool calls.
205-286: LGTM! Comprehensive path routing security validation.The path routing security tests thoroughly verify that:
- Invalid nested paths return 404
- Only valid scope endpoints are accessible
- Root and isolated message endpoints are properly separated
The comment at lines 269-271 has been updated from the previous review and now correctly clarifies that 404 is the expected response for missing sessions, which is consistent with the assertion.
1-287: Excellent E2E test coverage for standalone app functionality.This test suite comprehensively validates the standalone app infrastructure with well-organized test cases covering all critical scenarios: scope isolation, path routing security, tool/resource operations, and error handling. The tests follow best practices with proper resource cleanup and clear assertions.
libs/plugins/src/dashboard/tools/list-resources.tool.ts (2)
28-51: LGTM!Schema definitions are well-structured with appropriate types, optionals, and defaults. Using
z.inputandz.outputcorrectly differentiates between input (before defaults) and output (after parsing) types.
65-121: LGTM!The tool implementation is robust:
- Defensive try-catch blocks handle unavailable registries gracefully
- Safe fallbacks for missing names/URIs
- Filter application correctly uses
safeRegexand skips filtering on invalid patternsThe type assertion on line 68 is consistent with the pattern used in
list-tools.tool.tsand is acceptable giventryGetreturnsunknown.libs/plugins/src/dashboard/providers/graph-data.provider.ts (2)
49-62: LGTM!Simple and effective caching implementation. The 5-second TTL balances freshness with performance for dashboard use cases.
185-223: LGTM!The extraction method correctly:
- Uses defensive try-catch for unavailable registries
- Filters out dashboard tools to prevent self-reference in the graph
- Creates meaningful node IDs and edges
The pattern is consistently applied across all extraction methods.
libs/plugins/src/dashboard/app/dashboard.app.ts (1)
59-66: Acceptable workaround for SDK interface gap.Using
unknownwith a typed interface is safer thanas any. The optional chaining handles environments wheresetHeadermay not exist. This addresses the past review comment adequately.Consider opening an SDK issue to add
setHeaderto theServerResponseinterface.libs/plugins/src/dashboard/README.md (1)
1-100: LGTM!Comprehensive documentation covering features, installation, configuration, authentication, and architecture. The examples align with the implementation.
libs/plugins/src/dashboard/tools/list-tools.tool.ts (2)
28-53: LGTM!Schema definitions now correctly use
z.object(), addressing the past review comment about the inconsistent schema pattern. Type derivation withz.inputandz.outputis appropriate.
68-116: LGTM!The tool implementation is well-structured:
- Defensive try-catch for unavailable registries
- Safe regex filtering with graceful fallback
- Conditional schema inclusion based on
includeSchemasflag- Consistent pattern with
ListResourcesToollibs/plugins/src/dashboard/html/html.generator.ts (4)
7-16: XSS mitigation implemented correctly.The
escapeForJshelper addresses the past review comment about token interpolation. It properly escapes:
- Backslashes and quotes (prevents string breakout)
- Angle brackets as
\x3c/\x3e(prevents</script>injection)- Newlines (prevents multi-line injection)
This is a solid implementation for safe JavaScript string interpolation.
368-475: DuplicateMcpClientclass removed.There is now a single
McpClientimplementation, addressing the past review comment about the duplicate class definition that shadowed the first.The implementation handles:
- SSE connection establishment with endpoint discovery
- JSON-RPC message routing with pending request tracking
- Session ID extraction for legacy SSE transport
- Proper cleanup on close
477-588: LGTM!The
DashboardAppReact component is well-structured with:
- Proper state management for connection status, errors, and graph data
- Effect cleanup to close the MCP client on unmount
- Clear loading, error, and connected states
- Integration with React Flow for graph visualization
25-36: LGTM!Clean public API with a single
generateDashboardHtmlexport that delegates to internal implementations based on CDN configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
libs/plugins/src/dashboard/tools/list-tools.tool.ts (2)
9-23: DuplicatesafeRegeximplementation.This is identical to the implementation in
list-resources.tool.ts. As noted in that file's review, consider extracting to a shared utility.
71-72: Type assertion violates coding guidelines.Same issue as in
list-resources.tool.ts. Per coding guidelines, avoid type assertions and use proper type guards instead.🔎 Proposed fix
- const parentScope = this.tryGet(ParentScopeToken) as ScopeEntry | undefined; - const targetScope = parentScope || this.scope; + const parentScope = this.tryGet(ParentScopeToken); + const targetScope = isScopeEntry(parentScope) ? parentScope : this.scope;If extracting
safeRegexto a shared utility, also extract the type guard there.libs/plugins/src/dashboard/html/html.generator.ts (1)
112-124: Same CDN URL escaping concerns apply here.Similar to the external entrypoint version, CDN URLs in the import map (lines 115-120) and CSS href (line 124) are not escaped. Consider the same defense-in-depth validation as suggested for the external version.
🧹 Nitpick comments (7)
libs/plugins/src/dashboard/tools/list-resources.tool.ts (3)
9-23: Consider extractingsafeRegexto a shared utility.This identical function is duplicated in
list-tools.tool.ts. Extract it to a shared module (e.g.,../utils/safe-regex.ts) to maintain DRY principles.Additionally, the current mitigation (testing against
'test') doesn't actually detect catastrophic backtracking patterns—it only validates that the regex compiles and runs on a trivial input. For stronger protection, consider using a library likere2orsafe-regexthat performs static analysis on the pattern.🔎 Proposed extraction
Create a shared utility:
// libs/plugins/src/dashboard/utils/safe-regex.ts export function safeRegex(pattern: string, maxLength = 100): RegExp | null { if (pattern.length > maxLength) { return null; } try { const regex = new RegExp(pattern, 'i'); regex.test('test'); return regex; } catch { return null; } }
68-69: Type assertion violates coding guidelines.Per coding guidelines, avoid type assertions (
as). ThetryGetreturn type should be properly narrowed using type guards rather than casting.🔎 Proposed fix using type guard
- const parentScope = this.tryGet(ParentScopeToken) as ScopeEntry | undefined; - const targetScope = parentScope || this.scope; + const parentScope = this.tryGet(ParentScopeToken); + const targetScope = isScopeEntry(parentScope) ? parentScope : this.scope;Add a type guard function:
function isScopeEntry(value: unknown): value is ScopeEntry { return value !== null && typeof value === 'object' && 'resources' in value; }Alternatively, if
@frontmcp/sdkprovides typed token resolution, use that pattern instead.
74-87: Silent catch blocks hide potential issues.Empty catch blocks make debugging difficult. Consider logging at debug level or at least adding a comment explaining why errors are expected and safe to ignore.
🔎 Proposed improvement
try { const resources = targetScope.resources?.getResources?.(true) || []; for (const resource of resources) { results.push({ name: resource.name || 'unnamed', uri: resource.uri || 'unknown', description: resource.metadata?.description, mimeType: resource.metadata?.mimeType, isTemplate: false, }); } - } catch { - // Resources registry may not be available + } catch (error) { + // Resources registry may not be available in all scope configurations + // This is expected when the parent scope doesn't expose resources }libs/plugins/src/dashboard/tools/list-tools.tool.ts (2)
46-47: Consider usingz.record(z.unknown())for schema fields.Using
z.unknown()forinputSchemaandoutputSchemais acceptable, but if these are expected to be JSON Schema objects, consider a more specific type or usingz.record(z.string(), z.unknown())for better documentation.
83-87: Silent catch block hides potential issues.Same pattern as in
list-resources.tool.ts. Consider adding more descriptive comments or debug logging.libs/plugins/src/dashboard/html/html.generator.ts (2)
82-82: Consider escaping CDN entrypoint for safety.The
cdn.entrypointis interpolated directly into a dynamicimport()call. If this value contains quotes or special characters, it could break the JavaScript string. Consider escaping it withescapeForJs()for consistency with howtokenandbasePathare handled.🔎 Proposed fix
function generateExternalEntrypointHtml(options: DashboardPluginOptions): string { const { cdn, auth } = options; const safeBasePath = escapeForJs(options.basePath); const token = escapeForJs(auth?.token || ''); + const safeEntrypoint = cdn.entrypoint ? escapeForJs(cdn.entrypoint) : ''; return `<!DOCTYPE html> ... // Load external dashboard UI - import('${cdn.entrypoint}').then(mod => { + import('${safeEntrypoint}').then(mod => { if (mod.mount) {
51-51: Minor: Stray semicolon after HTML tag.Line 51 has a semicolon after
</title>which is not standard HTML syntax. While browsers typically handle this gracefully, consider removing it for cleaner markup.Also applies to: 64-64, 91-91
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.tsapps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.tslibs/plugins/src/dashboard/README.mdlibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/tools/list-tools.tool.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/plugins/src/dashboard/README.md
- apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.tsapps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.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/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.ts
🧠 Learnings (8)
📚 Learning: 2025-12-19T02:04:46.475Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T02:04:46.475Z
Learning: Applies to libs/sdk/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using proper validation
Applied to files:
libs/plugins/src/dashboard/tools/list-resources.tool.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.{ts,tsx,js,jsx} : Properly escape user-provided content to prevent XSS attacks in HTML generation
Applied to files:
libs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.test.{ts,tsx,js,jsx} : Test HTML escaping for user-provided content in security-critical paths
Applied to files:
libs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/tools/list-tools.tool.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/plugins/src/dashboard/html/html.generator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : Do not expose internal error details to users - sanitize error messages
Applied to files:
libs/plugins/src/dashboard/html/html.generator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx} : Use `.strict()` on all Zod schemas for validation
Applied to files:
libs/plugins/src/dashboard/tools/list-tools.tool.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/plugins/src/dashboard/tools/list-tools.tool.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/plugins/src/dashboard/tools/list-tools.tool.ts
🧬 Code graph analysis (3)
libs/plugins/src/dashboard/html/html.generator.ts (4)
libs/plugins/src/dashboard/html/index.ts (1)
generateDashboardHtml(1-1)libs/plugins/src/dashboard/dashboard.types.ts (1)
DashboardPluginOptions(58-58)libs/plugins/src/dashboard/index.ts (1)
DashboardPluginOptions(9-9)libs/plugins/src/index.ts (1)
DashboardPluginOptions(8-8)
libs/plugins/src/dashboard/tools/list-tools.tool.ts (5)
apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (1)
Tool(16-30)libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
Tool(56-122)libs/plugins/src/dashboard/dashboard.symbol.ts (1)
ParentScopeToken(12-12)libs/plugins/src/dashboard/index.ts (1)
ParentScopeToken(17-17)libs/sdk/src/app/instances/app.local.instance.ts (1)
tools(68-70)
apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (1)
apps/e2e/demo-e2e-standalone/src/apps/isolated/tools/isolated-hello.tool.ts (1)
Tool(16-30)
🪛 ast-grep (0.40.3)
libs/plugins/src/dashboard/tools/list-resources.tool.ts
[warning] 14-14: 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(pattern, 'i')
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)
libs/plugins/src/dashboard/tools/list-tools.tool.ts
[warning] 14-14: 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(pattern, 'i')
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)
⏰ 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 (15)
libs/plugins/src/dashboard/tools/list-resources.tool.ts (2)
28-31: LGTM on Zod schema definitions.The schemas correctly use
z.object()with appropriate field types, defaults, and descriptions. This aligns with Zod 4 patterns and provides proper type inference viaz.inputandz.output.Also applies to: 38-49
107-120: LGTM on filter application and return logic.The filtering logic properly uses
safeRegexfor ReDoS protection and gracefully falls back to returning all resources when the pattern is invalid. The return structure correctly matches the output schema.libs/plugins/src/dashboard/tools/list-tools.tool.ts (3)
28-32: LGTM on Zod schema definitions.The schemas use
z.object()correctly with proper defaults via.default(). Usingz.inputfor the input type correctly captures the pre-default types, andz.outputfor the output type is appropriate.Also applies to: 39-51
98-110: LGTM on conditional schema inclusion.The conditional spread pattern
...(condition ? { ... } : {})correctly includes schemas only whenincludeSchemasis true. This avoids unnecessary data transfer when schemas aren't needed.
58-67: LGTM on Tool decorator configuration.The tool is properly configured with name, description, schemas, and
readOnlyHintannotation indicating it doesn't modify state.apps/e2e/demo-e2e-standalone/src/apps/parent/tools/parent-hello.tool.ts (4)
1-2: LGTM!The imports are clean and necessary for the tool definition.
4-14: Excellent use of Zod 4 schemas!The schemas are well-defined and follow Zod 4 conventions correctly:
.default('World')provides a sensible default for optional inputz.outputcorrectly extracts output types after transformationsz.literal('parent')ensures type safety for the scope field
16-21: LGTM!The Tool decorator is properly configured with clear metadata and the correct schemas.
22-30: LGTM!The implementation is clean, type-safe, and consistent with the similar
IsolatedHelloToolpattern. The execute method correctly uses the input schema types and returns a properly typed output.libs/plugins/src/dashboard/html/html.generator.ts (6)
7-16: LGTM: JS string escaping implementation is correct.The
escapeForJsfunction properly handles escaping for JavaScript string literals, including quotes, backslashes, script-closing sequences, and control characters.
25-36: LGTM: Clean routing logic.The main function correctly routes to external or inline HTML generation based on the CDN entrypoint configuration.
43-44: Good: Past XSS vulnerabilities have been addressed.The token and basePath values are now properly escaped using
escapeForJs()before interpolation into JavaScript strings, addressing the previously flagged XSS concerns.
88-88: Good: Error message XSS vulnerability has been addressed.The error message is now escaped using
escapeHtml()before being inserted intoinnerHTML, preventing HTML/script injection through error messages.
101-103: Good: Consistent use of escaping for sensitive values.The inline version also properly escapes
basePathandtokenbefore interpolation, maintaining security consistency with the external version.
379-482: LGTM: Well-structured MCP client implementation.The
McpClientclass properly implements SSE transport with MCP protocol:
- Connects via EventSource and waits for endpoint discovery
- Handles JSON-RPC message routing with promise-based request tracking
- Includes appropriate error handling and connection timeouts
- Supports legacy sessionId parameter extraction
The duplicate class definition issue from the previous review has been resolved—only one
McpClientimplementation is present.
…egex utility for input validation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
libs/plugins/src/dashboard/html/html.generator.ts (1)
41-103: CDN URL escaping in HTML attribute context - existing concern.The implementation now uses
escapeForJs()for all values including CDN URLs. While this addresses XSS risks in JavaScript string contexts, a previous review noted that HTML attributes (likehrefon line 74) technically require HTML entity escaping (",<, etc.) rather than JavaScript escaping (\",\x3c).Given that CDN URLs are developer-configured and schema-validated (not user input), the risk is minimal. The current implementation provides defense-in-depth for the JavaScript template literal context.
Based on learnings from past reviews regarding HTML attribute escaping in HTML generation.
🧹 Nitpick comments (2)
libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
88-96: Consider providing feedback when filter regex is invalid.The code correctly uses
safeRegexto prevent ReDoS attacks, addressing the previous review concern. However, when a user-provided filter is invalid (null pattern), the function silently returns all resources without indication. Users might expect an error or warning when their regex is malformed or too complex.Consider either:
- Adding a field to the output schema to indicate filter was skipped
- Throwing a descriptive error when the pattern is invalid
- Documenting this silent-fallback behavior in the tool description
The current implementation is safe and functional, so this is an optional enhancement for better UX.
🔎 Optional: Add filter status to output
export const listResourcesOutputSchema = z.object({ resources: z.array( z.object({ name: z.string(), uri: z.string(), description: z.string().optional(), mimeType: z.string().optional(), isTemplate: z.boolean(), }), ), count: z.number(), + filterApplied: z.boolean().optional().describe('Whether the filter was successfully applied'), });Then in the execute method:
// Apply filter if provided (with ReDoS protection) let filtered = results; + let filterApplied = false; if (input.filter) { const pattern = safeRegex(input.filter); if (pattern) { filtered = results.filter((r) => pattern.test(r.name) || pattern.test(r.uri)); + filterApplied = true; } // If pattern is invalid, skip filtering and return all resources } return { resources: filtered, count: filtered.length, + filterApplied: input.filter ? filterApplied : undefined, };libs/plugins/src/dashboard/html/html.generator.ts (1)
109-626: LGTM! Comprehensive inline dashboard implementation.The inline dashboard generation properly handles:
- Security: All user-controlled values (
basePath,token) are escaped withescapeForJs()- SSE transport: The
McpClientclass (lines 398-501) correctly implements SSE connection establishment, endpoint discovery via the 'endpoint' event, JSON-RPC message routing, and cleanup- MCP protocol: Tool calling follows the JSON-RPC structure with proper request/response correlation using the
pendingRequestsMap- React rendering: The React application safely renders error messages since
React.createElementautomatically escapes text content- Graph visualization: Proper integration with Dagre for layout and React Flow for interactive rendering
The large inline HTML (500+ lines) is acceptable for a code generation scenario where the entire UI must be self-contained.
Optional: Consider extracting configuration objects
For improved readability, you could extract large configuration objects like
nodeConfig(lines 319-330) into module-level constants, though this is purely a stylistic suggestion for such a code generation context.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/plugins/src/dashboard/html/html.generator.tslibs/plugins/src/dashboard/shared/safe-regex.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/tools/list-tools.tool.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/plugins/src/dashboard/tools/list-tools.tool.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/plugins/src/dashboard/shared/safe-regex.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/html/html.generator.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/plugins/src/dashboard/shared/safe-regex.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/html/html.generator.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{build,base-template,adapters}/**/*.{ts,tsx,js,jsx} : Use buildCdnScriptsFromTheme with appropriate options (inline: true for blocked-network platforms like Claude)
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/*/index.ts : Implement entry point re-exports in index.ts files for each module (adapters, build, bundler, etc.)
📚 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/plugins/src/dashboard/shared/safe-regex.tslibs/plugins/src/dashboard/tools/list-resources.tool.tslibs/plugins/src/dashboard/html/html.generator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.{ts,tsx,js,jsx} : Properly escape user-provided content to prevent XSS attacks in HTML generation
Applied to files:
libs/plugins/src/dashboard/html/html.generator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{renderers,utils,base-template,tool-template,runtime}/**/*.test.{ts,tsx,js,jsx} : Test HTML escaping for user-provided content in security-critical paths
Applied to files:
libs/plugins/src/dashboard/html/html.generator.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/plugins/src/dashboard/html/html.generator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/**/*.{ts,tsx,js,jsx} : Do not expose internal error details to users - sanitize error messages
Applied to files:
libs/plugins/src/dashboard/html/html.generator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{theme,build,bundler,base-template}/**/*.{ts,tsx,js,jsx} : Use theme.cdn configuration instead of hard-coding CDN URLs
Applied to files:
libs/plugins/src/dashboard/html/html.generator.ts
📚 Learning: 2025-12-24T00:41:55.397Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/uipack/CLAUDE.md:0-0
Timestamp: 2025-12-24T00:41:55.397Z
Learning: Applies to libs/uipack/src/{build,base-template,adapters}/**/*.{ts,tsx,js,jsx} : Use buildCdnScriptsFromTheme with appropriate options (inline: true for blocked-network platforms like Claude)
Applied to files:
libs/plugins/src/dashboard/html/html.generator.ts
🧬 Code graph analysis (1)
libs/plugins/src/dashboard/html/html.generator.ts (1)
libs/plugins/src/dashboard/dashboard.types.ts (1)
DashboardPluginOptions(58-58)
🪛 ast-grep (0.40.3)
libs/plugins/src/dashboard/shared/safe-regex.ts
[warning] 13-13: 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(pattern, 'i')
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)
🔇 Additional comments (4)
libs/plugins/src/dashboard/shared/safe-regex.ts (1)
8-22: The ReDoS protection implementation is pragmatic and reasonable.The static analysis warning about ReDoS on line 14 is a false positive—this function is the protection mechanism. The combination of a 100-character length limit, try-catch block, and quick test provides pragmatic defense against most dangerous patterns.
Note: The quick test on line 16 with
'test'won't catch all malicious patterns (e.g.,(a+)+$won't hang on short strings but will on longer inputs). However, the 100-character limit significantly constrains pattern complexity, making this a reasonable trade-off for the use case.libs/plugins/src/dashboard/tools/list-resources.tool.ts (1)
47-68: Excellent error handling and type safety.The scope fallback logic, optional chaining, and try-catch blocks provide robust handling for cases where resources may not be available. The implementation follows TypeScript strict mode guidelines with no
anytypes or non-null assertions.libs/plugins/src/dashboard/html/html.generator.ts (2)
7-16: LGTM! Robust JavaScript string escaping utility.The
escapeForJsfunction properly escapes all characters that could break out of JavaScript string literals, including quotes, backslashes, and script-closing sequences. Using hex escapes (\x3c,\x3e) for angle brackets effectively prevents injection attacks. This addresses the XSS vulnerabilities flagged in previous reviews.
25-36: LGTM! Clean routing logic.The entry point function provides a clear decision point between external CDN-based UI and inline self-contained dashboard, with proper type safety.
…ng and path resolution
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.