feat: Add end-to-end tests for various applications and tools #122
Conversation
WalkthroughAdds many new end-to-end demo applications, extensive E2E tests and utilities (mock servers), multiple tools/resources/prompts/providers/plugins across apps, SDK type/export tweaks, hook and session-key updates, numerous per-app build/test configs, and an Nx toolchain upgrade. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
apps/e2e/demo-e2e-redis/src/apps/sessions/tools/set-session-data.tool.ts (1)
24-28: Session ID generation issue (duplicate).This tool has the same critical session ID generation issue as
GetSessionDataTool(already flagged in get-session-data.tool.ts review). Each call generates a new session ID, breaking session persistence.
🟡 Minor comments (6)
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts-163-177 (1)
163-177: Add assertion or fail when content type is unexpected.The type guard at line 170 allows the test to silently pass if
content.type !== 'text', potentially masking issues where the prompt returns unexpected content types.const message = result.messages[0]; if (message.content.type === 'text') { expect(message.content.text).toContain('UI Tools Showcase'); expect(message.content.text).toContain('HTML Type'); expect(message.content.text).toContain('React Type'); expect(message.content.text).toContain('MDX Type'); expect(message.content.text).toContain('Markdown Type'); + } else { + throw new Error(`Expected text content type, got: ${message.content.type}`); }apps/e2e/demo-e2e-redis/src/apps/sessions/data/session.store.ts-50-56 (1)
50-56: Pattern matching only replaces the first wildcard and doesn't filter expired keys.Two issues:
pattern.replace('*', '.*')only replaces the first*. Patterns likeuser:*:data:*won't work correctly.- The method returns keys of potentially expired entries since it doesn't check
expiresAt.keys(pattern?: string): string[] { - const allKeys = Array.from(this.data.keys()); + // Filter out expired keys first + const allKeys = Array.from(this.data.keys()).filter((key) => { + const entry = this.data.get(key); + if (entry?.expiresAt && Date.now() > entry.expiresAt) { + this.data.delete(key); + return false; + } + return true; + }); if (!pattern) return allKeys; - const regex = new RegExp(pattern.replace('*', '.*')); + const regex = new RegExp('^' + pattern.replace(/\*/g, '.*') + '$'); return allKeys.filter((k) => regex.test(k)); }apps/e2e/demo-e2e-public/src/apps/notes/prompts/summarize-notes.prompt.ts-17-33 (1)
17-33:systemContentis computed but never used.The
systemContentvariable containing format-specific instructions is built but not included in the returned result. This appears to be dead code, or it was intended to be part of the prompt messages.If the system instructions should be included, consider adding a system message:
return { messages: [ + { + role: 'assistant', + content: { + type: 'text', + text: systemContent, + }, + }, { role: 'user', content: { type: 'text', text: `Please summarize the following notes:\n\n${notesList}`, }, }, ], description: `Summarize ${notes.length} notes in ${format} format`, };Alternatively, if
systemContentis intentionally unused, remove the dead code.Committable suggestion skipped: line range outside the PR's diff.
apps/e2e/demo-e2e-providers/e2e/providers.e2e.test.ts-34-44 (1)
34-44: Tighten regex-based assertions and avoid non-null (!) assertionsLines 34–44, 50–60, and 78–89 use non-null assertions on regex match results (
instanceIdMatch1![1],startedAtMatch1![1]). This violates the TypeScript guideline to avoid non-null assertions and use proper error handling instead:- expect(instanceIdMatch1).not.toBeNull(); - expect(instanceIdMatch2).not.toBeNull(); - expect(instanceIdMatch1![1]).toBe(instanceIdMatch2![1]); + if (!instanceIdMatch1 || !instanceIdMatch2) { + throw new Error('Expected instanceId in both get-app-info responses'); + } + expect(instanceIdMatch1[1]).toBe(instanceIdMatch2[1]);Similarly at lines 50–60 for
startedAtMatch, and at lines 78–89 forinstanceIdMatch.Lines 131–138 and 155–160 use optional guards that allow tests to silently skip assertions if the expected fields are missing. This masks contract violations:
- if (uptimeMatch1 && uptimeMatch2) { - const uptime1 = parseInt(uptimeMatch1[1], 10); - const uptime2 = parseInt(uptimeMatch2[1], 10); - expect(uptime2).toBeGreaterThanOrEqual(uptime1); - } + if (!uptimeMatch1 || !uptimeMatch2) { + throw new Error('Expected uptime in both responses'); + } + const uptime1 = parseInt(uptimeMatch1[1], 10); + const uptime2 = parseInt(uptimeMatch2[1], 10); + expect(uptime2).toBeGreaterThanOrEqual(uptime1);- const message = result.messages[0]; - if (message.content.type === 'text') { - expect(message.content.text).toContain('GLOBAL Scope Provider'); - expect(message.content.text).toContain('CONTEXT Scope Provider'); - expect(message.content.text).toContain('Demo E2E Providers'); - } + const message = result.messages[0]; + expect(message.content.type).toBe('text'); + expect(message.content.text).toContain('GLOBAL Scope Provider'); + expect(message.content.text).toContain('CONTEXT Scope Provider'); + expect(message.content.text).toContain('Demo E2E Providers');These changes enforce the expected contract explicitly and fail loudly if the response shape changes.
apps/e2e/demo-e2e-multiapp/src/apps/calendar/resources/events-all.resource.ts-10-10 (1)
10-10: Potential schema validation failure for optional description.The output schema defines
descriptionas a requiredz.string(), but based on the related prompt file (schedule-overview.prompt.ts), event descriptions appear to be optional (e.description || 'No description'). Ife.descriptionisundefined, Zod validation will fail.Consider either making the schema field optional or providing a default value:
Option 1 - Make optional in schema:
z.object({ id: z.string(), title: z.string(), - description: z.string(), + description: z.string().optional(), startTime: z.number(),Option 2 - Provide default in mapping:
events: events.map((e) => ({ id: e.id, title: e.title, - description: e.description, + description: e.description ?? '', startTime: e.startTime,Also applies to: 37-45
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts-34-34 (1)
34-34: Remove the double type assertion.The double type assertion
as unknown as Outputis a code smell that bypasses TypeScript's type safety. If the SDK's context properly typesctx.output, this assertion shouldn't be necessary.Apply this diff to remove the unsafe assertion:
- const { title, sectionCount, mdxContent } = ctx.output as unknown as Output; + const { title, sectionCount, mdxContent } = ctx.output as Output;If this causes type errors, the root cause should be addressed in the SDK's type definitions rather than bypassing with double assertions.
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts
Outdated
Show resolved
Hide resolved
… enhance error handling in tools
There was a problem hiding this comment.
Actionable comments posted: 20
♻️ Duplicate comments (5)
apps/e2e/demo-e2e-redis/src/apps/sessions/tools/get-session-data.tool.ts (1)
26-27: Critical: Session ID generation still breaks session management.This is the same critical issue flagged in the previous review. The code was changed from
Date.now()torandomUUID(), but the fundamental problem remains: eachexecuteinvocation generates a completely new session ID, making it impossible to retrieve data previously stored viaSetSessionDataTool.The session ID must be derived from a shared context (e.g.,
this.scope.context.sessionIdor a session token) rather than generated anew on each call. Both get and set operations need to use the same persistent session ID.apps/e2e/demo-e2e-serverless/src/apps/serverless/tools/serverless-info.tool.ts (1)
5-9: Remove the unused_dummyparameter.This was previously flagged. If no input is required, use an empty object schema instead:
-const inputSchema = z - .object({ - _dummy: z.string().optional().describe('Unused parameter'), - }) - .strict(); +const inputSchema = z.object({}).strict();apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts (1)
185-194: Replace non-null assertion with proper guard.Per coding guidelines, avoid non-null assertions (
!). Use proper error handling with an explicit guard after thetoBeDefined()check.expect(execResult.status).toBe('ok'); expect(execResult.result).toBeDefined(); - const data = execResult.result!; + if (!execResult.result) throw new Error('Expected result to be defined'); + const data = execResult.result; expect(data.count).toBe(3); expect(data.users).toHaveLength(3);apps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.ts (1)
17-18: AlignInput/Outputtyping with simpler Zod infer patternSame note as in
clear-audit-log.tool.ts: instead ofz.infer<z.ZodObject<typeof inputSchema>>, you can follow the template style:type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;for consistency across tools and clearer types.
apps/e2e/demo-e2e-hooks/src/apps/audit/tools/get-audit-log.tool.ts (1)
32-33: Reuse the simpler Zod infer pattern forInput/OutputAs with the other audit tools, you can simplify:
type Input = z.infer<z.ZodObject<typeof inputSchema>>; type Output = z.infer<typeof outputSchema>;to:
type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;to match the template style and reduce type noise.
🧹 Nitpick comments (47)
apps/e2e/demo-e2e-notifications/webpack.config.js (1)
7-12: Consider simplifying the conditional logic.The
devtoolModuleFilenameTemplateis conditionally set based onNODE_ENV !== 'production', butmodeis hardcoded to'development'on line 11. If the mode will always be development for this e2e app, the conditional could be simplified or removed. Alternatively, if you expect to support production builds in the future, consider makingmodeconfigurable.apps/e2e/demo-e2e-redis/webpack.config.js (1)
1-28: Config looks correct; consider a shared base for e2e webpack configsThis matches the other e2e app configs (Node target, tsc, main/tsconfig/output path all look right) and should build the Redis demo as expected.
Given how many nearly identical
apps/e2e/demo-e2e-*/webpack.config.jsfiles there are, you might later factor out a small shared helper (e.g.,createE2EWebpackConfig('demo-e2e-redis')) to reduce duplication, but it’s fine to keep as-is for now.apps/e2e/demo-e2e-cache/webpack.config.js (1)
1-28: Cache webpack config is consistent; same optional DRY opportunityThis config is structurally identical to the other e2e app configs and the output path for
demo-e2e-cacheis correct, so it should integrate cleanly with the Nx targets.As with the Redis config, you could later introduce a shared factory/utility for these Node e2e webpack configs to avoid repeating the same plugin/options block in every app, but it isn’t required for this PR.
apps/e2e/demo-e2e-redis/src/apps/sessions/tools/get-session-data.tool.ts (1)
18-25: Class structure follows the pattern correctly.The tool decorator and class definition are properly configured. The method signature on line 25, while correct, uses a verbose type annotation
z.infer<z.ZodObject<typeof inputSchema>>that could be simplified if you extract the inferred type as a separate type alias for better readability.apps/e2e/demo-e2e-errors/webpack.config.js (1)
11-12: Consider tyingmode/devtoolto environment or Nx configurationHard‑coding
mode: 'development'anddevtool: 'eval-cheap-module-source-map'means even “production” builds for this app will behave like dev builds (larger bundles, source maps, eval). If you ever run this target in a prod‑like configuration, consider deriving these fromprocess.env.NODE_ENVor letting Nx drive them and only overriding when necessary.apps/e2e/demo-e2e-multiapp/webpack.config.js (1)
7-11: Consider aligning the mode with NODE_ENV for consistency.The
modeis hardcoded to'development'on line 11, but the conditional on lines 7-9 checksprocess.env.NODE_ENV !== 'production'. While this works, it creates a minor inconsistency—webpack's mode remains'development'regardless ofNODE_ENV.If you want the webpack mode to be configurable, consider:
+ mode: process.env.NODE_ENV === 'production' ? 'production' : 'development', - mode: 'development', devtool: 'eval-cheap-module-source-map',Alternatively, if the mode should always be
'development'for e2e tests, you could simplify the conditional:output: { path: join(__dirname, '../../../dist/apps/e2e/demo-e2e-multiapp'), - ...(process.env.NODE_ENV !== 'production' && { - devtoolModuleFilenameTemplate: '[absolute-resource-path]', - }), + devtoolModuleFilenameTemplate: '[absolute-resource-path]', },libs/testing/src/server/test-server.ts (1)
199-224: Consider consolidating polling logic.
waitForReady()andwaitForReadyWithExitDetection()share similar polling logic. You could extract the common health-check polling into a shared helper with an optional exit-check callback, reducing duplication.- async waitForReady(timeout?: number): Promise<void> { - const timeoutMs = timeout ?? this.options.startupTimeout; - const deadline = Date.now() + timeoutMs; - const checkInterval = 100; - - while (Date.now() < deadline) { - try { - const response = await fetch(`${this._info.baseUrl}${this.options.healthCheckPath}`, { - method: 'GET', - signal: AbortSignal.timeout(1000), - }); - - if (response.ok || response.status === 404) { - this.log('Server is ready'); - return; - } - } catch { - // Server not ready yet - } - - await sleep(checkInterval); - } - - throw new Error(`Server did not become ready within ${timeoutMs}ms`); - } + async waitForReady(timeout?: number): Promise<void> { + await this.waitForReadyWithExitDetection(() => ({ exited: false }), timeout); + }apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts (2)
25-26: Simplify type inference.Line 25 wraps the input schema with
z.ZodObject<typeof inputSchema>, but sinceinputSchemais already a ZodObject, this is redundant. For consistency with line 26, use direct inference instead.Apply this diff:
-type Input = z.infer<z.ZodObject<typeof inputSchema>>; +type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;
35-50: Consider minor refactoring for cleaner code.Several optional improvements:
Line 36: The local
storevariable is unnecessary since it's only used once. Consider accessingtaskStoredirectly.Line 37: The type assertion
as TaskPriorityis unnecessary because the zod enum validation already ensures the value is of the correct type when it's not'all'.Apply this diff for cleaner code:
async execute(input: Input): Promise<Output> { - const store = taskStore; - let tasks = input.priority === 'all' ? store.getAll() : store.getByPriority(input.priority as TaskPriority); + const tasks = input.priority === 'all' ? taskStore.getAll() : taskStore.getByPriority(input.priority); return { tasks: tasks.map((t) => ({apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts (3)
4-16: Consider adding.strict()to the nested sections object for consistency.The outer object uses
.strict()(line 16), but the nested object defining section structure (lines 9-12) does not. For consistency and to ensure unknown properties are rejected at all levels, consider applying.strict()to the nested schema as well.Apply this diff:
sections: z .array( z.object({ heading: z.string(), content: z.string(), - }), + }).strict(), ) .describe('Document sections'),
18-23: Add.strict()to outputSchema for consistency.The
inputSchemauses.strict()mode, butoutputSchemadoes not. For consistency and to ensure the output validation rejects unknown properties, consider adding.strict()to the output schema as well.Apply this diff:
-const outputSchema = z.object({ +const outputSchema = z.object({ uiType: z.literal('mdx'), title: z.string(), sectionCount: z.number(), mdxContent: z.string(), -}); +}).strict();
25-25: Simplify the type inference pattern.The type inference pattern
z.infer<z.ZodObject<typeof inputSchema>>is unnecessarily complex and semantically incorrect. SinceinputSchemais already a Zod schema, you should infer the type directly usingz.infer<typeof inputSchema>, as demonstrated correctly on line 26 forOutput.Apply this diff:
-type Input = z.infer<z.ZodObject<typeof inputSchema>>; +type Input = z.infer<typeof inputSchema>;apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.ts (4)
8-8: Remove redundant.optional()before.default().In Zod v4,
.default()already makes the field optional with a default value, so the.optional()modifier is redundant.Apply this diff to simplify the schema:
- description: z.string().optional().default('').describe('Event description'), + description: z.string().default('').describe('Event description'),
25-26: Simplify type definitions.The
z.ZodObject<...>wrapper is unnecessary. Zod'sz.infercan infer types directly from the schema.Apply this diff to simplify:
-type Input = z.infer<z.ZodObject<typeof inputSchema>>; +type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;
36-36: Remove unnecessary intermediate variable.The
storevariable on line 36 is unnecessary and can be removed for cleaner code.Apply this diff:
async execute(input: Input): Promise<Output> { - const store = eventStore; - const event = store.create(input.title, input.description, input.startTime, input.endTime, input.location); + const event = eventStore.create(input.title, input.description, input.startTime, input.endTime, input.location);
5-13: Consider adding timestamp validation.The schema accepts any numeric timestamps for
startTimeandendTimewithout validating thatendTimeis afterstartTime. While this might be intentional for e2e testing flexibility, adding validation could prevent invalid event creation.If validation is desired, you could add a
.refine()check:const inputSchema = z .object({ title: z.string().min(1).describe('Event title'), description: z.string().default('').describe('Event description'), startTime: z.number().describe('Event start time (Unix timestamp in ms)'), endTime: z.number().describe('Event end time (Unix timestamp in ms)'), location: z.string().optional().describe('Event location'), }) .strict() .refine((data) => data.endTime > data.startTime, { message: 'End time must be after start time', path: ['endTime'], });apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (2)
4-10: Tighten statusCode validation to integer HTTP codesSince this represents an HTTP status, consider constraining it to integers for stricter validation:
- statusCode: z.number().optional().default(400).describe('HTTP status code'), + statusCode: z.number().int().optional().default(400).describe('HTTP status code'),
25-28: Good use of specific MCP error class for custom error emissionUsing
PublicMcpErrorwith an expliciterrorCodeand optionalstatusCodematches the guideline to emit typed MCP errors rather than generic exceptions. Given this tool is explicitly for error-path testing, an always-throwexecuteis reasonable here. Based on learnings, this aligns well with the MCP error-handling approach.If you ever need a non-error path for this tool, you could add a boolean trigger in
inputSchema(mirroringthrow-internal-error) and return{ success: true }when not throwing.apps/e2e/demo-e2e-serverless/webpack.config.js (1)
7-12: Optionally alignmode/devtoolwithNODE_ENVRight now,
modeis always'development'anddevtoolis always'eval-cheap-module-source-map', whileNODE_ENVonly affectsdevtoolModuleFilenameTemplate. If you ever want this config to better mimic production behavior, consider tyingmode/devtooltoprocess.env.NODE_ENV, e.g.:- mode: 'development', - devtool: 'eval-cheap-module-source-map', + mode: process.env.NODE_ENV === 'production' ? 'production' : 'development', + devtool: + process.env.NODE_ENV === 'production' + ? false + : 'eval-cheap-module-source-map',(Or similar logic, depending on how you run these e2e builds.)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.ts (2)
4-18: Clarify the semantics of the optionalrequiredfield.The
requiredfield (line 12) is defined asz.boolean().optional(), which creates ambiguity: if a field'srequiredproperty is undefined, should it be treated as required or not required? Consider using.default(false)to make the behavior explicit.Apply this diff to make the default behavior explicit:
- required: z.boolean().optional(), + required: z.boolean().default(false),
33-34: Simplify type aliases.The type aliases use unnecessarily verbose syntax. Since
inputSchemaandoutputSchemaare already Zod objects, you can simplify the type inference.Apply this diff:
-type Input = z.infer<z.ZodObject<typeof inputSchema>>; +type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-list.tool.ts (2)
4-28: Simplify and correct Zod inference forInputThe
Inputalias is usingz.infer<z.ZodObject<typeof inputSchema>>, which is non‑idiomatic and likely mismatches Zod’s generic expectations (it expects the schema type directly, not wrapped again inZodObject). You can rely on the schema type directly and keep the pattern consistent with other usage:-const inputSchema = z - .object({ +const inputSchema = z + .object({ title: z.string().describe('List title'), items: z .array( z.object({ text: z.string(), completed: z.boolean().optional(), }), ) .describe('List items'), ordered: z.boolean().optional().describe('Use ordered list'), }) .strict(); -const outputSchema = z.object({ +const outputSchema = z.object({ uiType: z.literal('markdown'), title: z.string(), itemCount: z.number(), completedCount: z.number(), markdown: z.string(), }); -type Input = z.infer<z.ZodObject<typeof inputSchema>>; +type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;This keeps strict typing without introducing unnecessary generics and aligns with Zod 4’s typical usage.
35-48: Tighten typing forctx.outputinstead of double casting
ctx.output as unknown as Outputworks but loses type safety twice and can hide mismatches between the tool’s runtime output and the UI template’s expectations.If
@frontmcp/sdkexposes a typed UI context (e.g., via generics or a dedicated interface), it would be better to typectxso thatctx.outputis alreadyOutput(or a compatible type) and eliminate the cast entirely. For example, conceptually:// Pseudocode – adjust to actual SDK types ui: { uiType: 'markdown', template: (ctx: { output: Output }) => { const { title, itemCount, completedCount, markdown } = ctx.output; ... }, }If the SDK doesn’t currently expose such typing, consider a small helper around
ToolContext/UI templates to centralize this cast instead of repeatingunknown as Outputin each tool.apps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.ts (1)
31-51: LGTM: Solid implementation with good immutability practices.The
RequestLoggerImplclass correctly implements the interface with proper type safety. The spread operator on line 48 ensures immutability when returning logs.For production use, consider using a more robust ID generation method at line 33 instead of
Math.random()to avoid potential collisions (e.g., UUID or nanoid). However, this is perfectly acceptable for E2E demo purposes.apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts (1)
5-34: Tool wiring looks good; simplify Zod typings and consider enum reuseThe overall implementation is consistent with the other CRM tools and looks functionally correct. Two small, optional refinements:
- Simplify the
executeinput typeYou can lean on Zod’s standard pattern here and avoid constructing a
ZodObjecttype manually:- async execute(input: z.infer<z.ZodObject<typeof inputSchema>>): Promise<z.infer<typeof outputSchema>> { + async execute(input: z.infer<typeof inputSchema>): Promise<z.infer<typeof outputSchema>> {This is the usual, clearer way to infer the input type from the schema and avoids potential mismatches with
ZodObject’s generic parameters. Please verify withtscthat this matches your project’s Zod typings.
- (Optional) Extract the activity type enum for reuse
Since the same enum literal list appears in both
inputSchemaandoutputSchema(and likely other CRM tools), you might want to centralize it to keep types in sync:+const activityTypeEnum = z.enum(['call', 'email', 'meeting', 'note']); + const inputSchema = z .object({ userId: z.string().describe('User ID for the activity'), - type: z.enum(['call', 'email', 'meeting', 'note']).describe('Activity type'), + type: activityTypeEnum.describe('Activity type'), description: z.string().describe('Activity description'), }) .strict(); const outputSchema = z.object({ activity: z.object({ id: z.string(), userId: z.string(), - type: z.enum(['call', 'email', 'meeting', 'note']), + type: activityTypeEnum, description: z.string(), timestamp: z.string(), }), });libs/sdk/src/hooks/hooks.utils.ts (1)
10-34: Merging legacy and TC39 hooks is solid; consider tighteninginstancetypingThe combined
allHooks = [...legacyHooks, ...tc39Hooks]plus resolvingmetadata.targettoitem/instancepreserves existing behavior while adding TC39 support; the mapping overallHookslooks correct.Given the SDK guideline to avoid
anyin strict mode, you could optionally tighten the signature to use a generic object constraint without affecting call sites:-export function normalizeHooksFromCls(instance: any): HookRecord[] { +export function normalizeHooksFromCls<T extends object>(instance: T): HookRecord[] {This keeps the implementation identical at runtime but improves type safety at the boundary of this helper. As per coding guidelines, this moves the helper closer to strict TypeScript without introducing breaking changes.
apps/e2e/demo-e2e-public/webpack.config.js (1)
1-28: Config looks correct for an Nx Node e2e app; consider making mode/devtool env‑awareThis mirrors standard Nx Node e2e configs: the output path, entry, and
NxAppWebpackPluginoptions all look coherent for a debug‑friendly test app. If you ever need a production‑style build for this entrypoint, you might want to derivemodeanddevtoolfromNODE_ENVinstead of hard‑coding development, e.g.:- mode: 'development', - devtool: 'eval-cheap-module-source-map', + mode: + process.env.NODE_ENV === 'production' ? 'production' : 'development', + devtool: + process.env.NODE_ENV === 'production' + ? 'source-map' + : 'eval-cheap-module-source-map',apps/e2e/demo-e2e-transparent/src/apps/tasks/tools/list-tasks.tool.ts (2)
11-24: Optional: tightencreatedAtvalidation to a datetime format
createdAtis currently a free-form string. If the tasks store always uses a specific timestamp format (e.g., ISO 8601), consider using zod’s datetime helpers (such as an ISO‑datetime schema) so invalid timestamps are rejected at the boundary and the contract is clearer.Please double‑check the zod 4 docs for the recommended datetime helper (e.g., ISO datetime) and confirm it matches the format used by your tasks store.
26-27: Simplify and correct Zod inference typesThe alias for
ListTasksInputis more complex than needed and can be fragile due to thez.ZodObject<typeof inputSchema>generic. You can rely directly on the schema type:type ListTasksInput = z.infer<typeof inputSchema>; type ListTasksOutput = z.infer<typeof outputSchema>;This is the idiomatic pattern with zod, improves readability, and avoids potential generic mismatches. If other tools still use the older pattern, consider aligning them with this style for consistency.
Please verify against the zod 4 type definitions/examples that
z.infer<typeof inputSchema>is the preferred pattern for inferring input types from object schemas.apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-stats.tool.ts (1)
5-9: Consider using an empty object schema instead of a dummy field.The
_dummyfield with.optional()serves as a placeholder for a no-input tool. A cleaner approach would be to use an empty strict object schema, which better conveys the intent and avoids documenting a field that callers should ignore.const inputSchema = z .object({ - _dummy: z.string().optional().describe('Unused'), }) .strict();libs/testing/src/matchers/mcp-matchers.ts (1)
118-137: Strengthen shape check for wrappers intoHaveTextContentThe extended matcher looks good and the union handling is sound, but the current guard only checks for
'text' in resultand then unconditionally callsresult.text(). If a non-wrapper object with a non-functiontextproperty ever reaches this matcher, it will throw at runtime instead of producing a clean “received wrong type” failure.You can make the matcher a bit more robust by also checking that
text(and optionallyhasTextContent) are functions before using them:- // Check if it's a valid wrapper object with text() method - if (typeof result !== 'object' || result === null || !('text' in result)) { + // Check if it's a valid wrapper object with text() method + if ( + typeof result !== 'object' || + result === null || + typeof (result as { text?: () => string | undefined }).text !== 'function' + ) { return { pass: false, message: () => `Expected a ToolResultWrapper or ResourceContentWrapper object with text method`, }; } - // Get text content - works for both wrapper types - const text = result.text(); + // Get text content - works for both wrapper types + const text = (result as { text: () => string | undefined }).text(); - // Check if has text content - ToolResultWrapper has hasTextContent, ResourceContentWrapper uses text() !== undefined - const hasText = 'hasTextContent' in result ? result.hasTextContent() : text !== undefined; + // Check if has text content - ToolResultWrapper has hasTextContent, ResourceContentWrapper uses text() !== undefined + const hasText = + 'hasTextContent' in result && typeof (result as { hasTextContent?: () => boolean }).hasTextContent === 'function' + ? (result as { hasTextContent: () => boolean }).hasTextContent() + : text !== undefined;This keeps the intended behavior while making the matcher more defensive against incorrect inputs.
apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/create-note.tool.ts (1)
19-20: Make type inference consistent and simpler.Line 19 wraps
inputSchemawithz.ZodObject<...>while line 20 uses the simpler form. SinceinputSchemais already aZodObject(created withz.object()), the wrapper is redundant.Apply this diff to make both type aliases consistent and simpler:
-type Input = z.infer<z.ZodObject<typeof inputSchema>>; +type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;apps/e2e/demo-e2e-cache/src/apps/compute/tools/reset-stats.tool.ts (1)
5-9: Optional: simplify “no-input” schema instead of dummy fieldUsing an optional
_dummyfield works, but it adds an unused property to the input contract. If the tooling supports tools with no fields, you could simplify this to an explicit empty object schema for clarity:-const inputSchema = z - .object({ - _dummy: z.string().optional().describe('Unused'), - }) - .strict(); +const inputSchema = z.object({}).strict();This keeps the tool’s intent (“no meaningful input”) clear and avoids placeholder properties leaking into schemas.
apps/e2e/demo-e2e-notifications/src/apps/notify/tools/trigger-progress.tool.ts (1)
19-20: Simplify Zod inference for clearer typingYou can simplify these aliases and avoid awkward generics by inferring directly from the schema:
type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;This matches common Zod patterns and avoids relying on
z.ZodObjectgenerics explicitly.apps/e2e/demo-e2e-notifications/src/apps/notify/tools/long-running-task.tool.ts (1)
19-20: Align Zod inference with schema for cleaner typesAs in the other tool, you can infer directly from the schemas instead of going through
z.ZodObject:type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;This keeps the types simpler and more idiomatic Zod.
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts (1)
58-64: Consider returning the actual table HTML (or clarify output contract)
executecurrently returns anhtmlfield with only a summary string:html: `<table>${input.headers.length} columns, ${input.rows.length} rows</table>`,while the UI template builds the real table markup separately. If any consumers rely on
Output['html']directly (e.g., logs, non‑UI clients, other renderers), they’ll see only the summary, not the actual table.If the contract is “
htmlcontains the rendered table,” consider changing this to reuse the same markup as the template (or generate it once and share), or clearly document thathtmlis intentionally a summary.apps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-chart.tool.ts (2)
32-73: Reduce reliance onunknowndouble‑casts intemplatefor stronger typingThe
templateimplementation currently does:const { data, maxValue } = ctx.output as unknown as Output; const title = (ctx.input as unknown as Input).title;This bypasses type safety and makes it easy to drift from the declared schemas.
If the SDK exposes a typed context for the
ui.templatehandler (e.g., via generics on@Toolor aToolTemplateContext<Input, Output>helper), prefer annotatingctxwith that type and removing theunknown asbridges. If that’s not available yet, consider at least narrowingctx.input/ctx.outputvia small runtime guards before asserting the types.This keeps you aligned with the strict TypeScript/no‑
anyguidelines and makes future refactors safer.
76-83: Execute logic is fine; consider stricter numeric validation for chart valuesThe
executemethod is clear and avoids division‑by‑zero via:const maxValue = Math.max(...input.data.map((d) => d.value), 1);To avoid odd rendering from negative or non‑finite values (e.g., negative bar heights or
NaNpx), you could tighten the input schema, for example:- value: z.number(), + value: z.number().nonnegative().finite(),This keeps chart semantics clean while still supporting the current UI logic.
apps/e2e/demo-e2e-hooks/src/apps/audit/tools/clear-audit-log.tool.ts (2)
16-17: Simplify ZodInput/Outputtype inferenceThe aliases
type Input = z.infer<z.ZodObject<typeof inputSchema>>; type Output = z.infer<typeof outputSchema>;are more complex than needed and diverge from the template pattern. Consider:
type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>;as used in
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.tsfor consistency and clearer intent.
19-33: Clear‑audit tool implementation looks good;_dummyinput can likely be droppedThe
executeimplementation is clear and side‑effect is well encapsulated inauditLog.clear()with a simple success payload. If the framework allows a truly empty input, consider using an empty input schema instead of the_dummyfield so the tool’s contract more accurately reflects “no input required”.apps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.ts (1)
27-37: Execution flow and delay handling are clear; minor conditional simplification possibleThe tool behavior (optional delay then processed response with timestamp) matches the description and is easy to follow. Since
delayhas a default of0, you could simplify:if (input.delay && input.delay > 0) {to
if (input.delay > 0) {to avoid the redundant truthiness check.
apps/e2e/demo-e2e-hooks/src/apps/audit/tools/get-audit-log.tool.ts (1)
42-58: Audit‑log retrieval and shaping logic look solidThe conditional selection of entries (all vs filtered by
toolName) and the explicit mapping into the output schema, along withexecutionOrderandstatsfromauditLog, is clear and consistent with the tool description. Only nuance: an empty stringtoolNameis treated as “no filter”; if that’s ever user‑supplied input and you need stricter behavior, you might want to validate against empty strings at the schema level.apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/create-task.tool.ts (1)
5-20: Reduce duplication aroundpriorityand simplify typings / castThe implementation looks correct; a few small cleanups could tighten the type story:
priority’s enum is defined twice (input and output schemas) and you also have aTaskPrioritytype from the store. You can DRY this up by introducing a shared Zod enum and reusing it across input, output, and the store type.- Once
Inputis inferred directly frominputSchema,input.priorityshould already be'low' | 'medium' | 'high'(given the.default('medium')), making theas TaskPrioritycast typically unnecessary ifTaskPriorityis the same union.Example refactor:
-const inputSchema = z - .object({ - title: z.string().min(1).describe('Task title'), - description: z.string().optional().default('').describe('Task description'), - priority: z.enum(['low', 'medium', 'high']).optional().default('medium').describe('Task priority'), - }) - .strict(); +const priorityEnum = z.enum(['low', 'medium', 'high']); + +const inputSchema = z + .object({ + title: z.string().min(1).describe('Task title'), + description: z.string().optional().default('').describe('Task description'), + priority: priorityEnum.optional().default('medium').describe('Task priority'), + }) + .strict(); -const outputSchema = z.object({ +const outputSchema = z.object({ id: z.string(), title: z.string(), description: z.string(), - priority: z.enum(['low', 'medium', 'high']), + priority: priorityEnum, status: z.enum(['pending', 'in_progress', 'completed']), createdAt: z.number(), }); -type Input = z.infer<z.ZodObject<typeof inputSchema>>; +type Input = z.infer<typeof inputSchema>; type Output = z.infer<typeof outputSchema>; async execute(input: Input): Promise<Output> { const store = taskStore; - const task = store.create(input.title, input.description, input.priority as TaskPriority); + const task = store.create(input.title, input.description, input.priority);Please double‑check that this aligns with your
taskStore.createandTaskPrioritytypings in../data/task.store.Also applies to: 34-41
apps/e2e/demo-e2e-transparent/src/apps/tasks/tools/create-task.tool.ts (1)
5-21: Consider pushing defaults/constraints into the schema and reusing the priority enumFunctionally this is fine for an e2e demo; a few tweaks could make the contract clearer and more consistent with other task tools:
- Right now
prioritydefaults to'medium'only inexecute, not ininputSchema, so the schema doesn’t advertise the default. If you move the default into the schema, callers and tooling will see it.titleaccepts empty strings; if you want to match the multiapp demo behavior, you could add.min(1)to prevent empty titles.- Description is required here but optional with a default in the multiapp demo; decide whether you want that difference or align them.
- As in the multiapp file, you can reuse a single
priorityEnumin both input and output schemas.Example (if you do want to align with the multiapp behavior):
-const inputSchema = z - .object({ - title: z.string().describe('Title of the task'), - description: z.string().describe('Description of the task'), - priority: z.enum(['low', 'medium', 'high']).optional().describe('Task priority'), - }) - .strict(); +const priorityEnum = z.enum(['low', 'medium', 'high']); + +const inputSchema = z + .object({ + title: z.string().min(1).describe('Title of the task'), + description: z.string().optional().default('').describe('Description of the task'), + priority: priorityEnum.optional().default('medium').describe('Task priority'), + }) + .strict(); -const outputSchema = z.object({ +const outputSchema = z.object({ id: z.string(), title: z.string(), description: z.string(), - priority: z.enum(['low', 'medium', 'high']), + priority: priorityEnum, completed: z.boolean(), createdBy: z.string(), - createdAt: z.string(), + createdAt: z.string(), }); -type CreateTaskInput = z.infer<z.ZodObject<typeof inputSchema>>; +type CreateTaskInput = z.infer<typeof inputSchema>; @@ - priority: input.priority || 'medium', + priority: input.priority,If you’re on Zod 4 as indicated, you could also tighten
createdAtto an ISO datetime withz.iso.datetime()if you want the schema to matchtoISOString()more closely.Please confirm that any changes stay compatible with
tasksStore.addand with how you intend these two demos to differ (or not) in their task contracts.Also applies to: 37-45
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts (2)
29-32: Avoidas unknown ascasts in theui.templatecontextThe double casts on
ctx.outputandctx.inputeffectively bypass type safety and can hide mismatches between the ToolContext and the Zod schemas, which goes against the strict TS guideline.If the
@frontmcp/sdktypes allow it, consider typing the template context directly:- template: (ctx) => { - const { topic, points, hasCode } = ctx.output as unknown as Output; - const codeExample = (ctx.input as unknown as Input).codeExample; + template: (ctx: { input: Input; output: Output }) => { + const { topic, points, hasCode } = ctx.output; + const { codeExample } = ctx.input;If the SDK’s callback type is stricter, a compromise is to do a one-time cast with a clearly named variable (e.g.,
const typedCtx = ctx as { input: Input; output: Output };) instead of repeatingas unknown ason each access.
33-56: Optionally base code-block rendering oncodeExamplepresenceRight now the MDX code block is rendered whenever
hasCodeis true, regardless of whethercodeExampleis actually present. If the two ever get out of sync (e.g., future changes or a malformed caller), you could end up rendering a fenced block withundefined.A small guard makes this more robust:
- const pointsList = points.map((p) => `- ${p}`).join('\n'); + const pointsList = points.map((p) => `- ${p}`).join('\n'); + const showCode = !!codeExample; ... -${ - hasCode +${ + showCode ? ` ## Code Example \`\`\`typescript ${codeExample} \`\`\` `Not critical for the current E2E/demo usage, but it tightens the contract between input and rendered MDX.
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts (1)
35-52:ctx.output as unknown as Outputweakens type-safety in the UI templateThe double cast
ctx.output as unknown as Outputbypasses the SDK’s typing guarantees and can hide mismatches between the actualoutputshape andOutput. Ifctx.outputis already typed asunknownor a compatible tool-output type, a single, direct cast is sufficient and clearer.If the current SDK typings allow it, consider simplifying to:
- template: (ctx) => { - const { title, findingCount, markdown } = ctx.output as unknown as Output; + template: (ctx) => { + const { title, findingCount, markdown } = ctx.output as Output;If that still doesn’t type-check, it’s a hint that the SDK’s UI context type might be too generic; tightening it there (so each tool’s
ui.templatesees its ownOutputtype) would let you drop the cast entirely.
apps/e2e/demo-e2e-cache/src/apps/compute/tools/reset-stats.tool.ts
Outdated
Show resolved
Hide resolved
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts (2)
19-20: Previous issue resolved - type aliases now correct!The type aliases correctly use
z.infer<typeof schema>without unnecessary wrappers. The previous issue with the redundantz.ZodObjectwrapper on line 19 has been fixed.
22-56: Previous issue resolved - template now correctly usesctx.input!The template correctly accesses input fields from
ctx.input(line 30) and properly usesescapeHtml()for all user-provided content (lines 34, 40, 47), preventing XSS vulnerabilities.However, the type assertion on line 30 is unnecessary:
const { headers, rows, title } = ctx.input as unknown as { headers: string[]; rows: string[][]; title?: string };Since
ToolContext<typeof inputSchema, typeof outputSchema>already provides proper typing forctx.input, this assertion can be removed:- const { headers, rows, title } = ctx.input as unknown as { headers: string[]; rows: string[][]; title?: string }; + const { headers, rows, title } = ctx.input;apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (1)
16-17: Past review comment is resolved.The type aliases correctly use
z.infer<typeof inputSchema>andz.infer<typeof outputSchema>. The issue mentioned in the past review comment (incorrectz.infer<z.ZodObject<typeof inputSchema>>usage) is not present in the current code.
🧹 Nitpick comments (20)
apps/e2e/demo-e2e-hooks/src/apps/audit/tools/get-audit-log.tool.ts (1)
11-30: Optionally tighten zod schemas for timestamp and stage to better match the audit modelIf the underlying
AuditEntryuses:
- ISO timestamps, you may want
timestamp: z.iso.datetime()instead of a plainz.string().- A fixed set of stages, you could replace
stage: z.string()withz.enum([...])(similar tohookType) to catch drift between the data model and the tool output at runtime.This is not required for correctness, but would give stronger validation guarantees for the e2e surface.
If you confirm the actual
AuditEntryshape, you can validate the above against the zod 4 docs forz.iso.datetime()and enums.apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts (1)
1-16: Schemas and typing are clean and align with strict TS/Zod usageImports, Zod schemas, and the
Input/Outputtypings are all strongly typed with noany/non‑null assertions, and the strict input object is appropriate here. As an optional tweak, you could also makeoutputSchemastrict for symmetry with other tools:-const outputSchema = z.object({ - success: z.boolean(), -}); +const outputSchema = z + .object({ + success: z.boolean(), + }) + .strict();As per coding guidelines, ...
apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/list-notes.tool.ts (1)
5-17: Small cleanup: inlinenoteStoreand consider future‑proofing input schemaTwo minor, optional tweaks for consistency and clarity:
- The
const store = noteStore;indirection is unnecessary; you can inlinenoteStore.getAll()to match other list tools:- async execute(_input: Input): Promise<Output> { - const store = noteStore; - const notes = store.getAll(); + async execute(_input: Input): Promise<Output> { + const notes = noteStore.getAll();
- If you anticipate filters/pagination (as in the example list tool using
limit/query), you could extendinputSchemaaccordingly to keep list tools consistent across apps; otherwise the current empty strict object is fine for a simple demo.Also applies to: 28-41
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts (2)
18-23: Consider adding.strict()for consistency.While the input schema uses
.strict()mode, the output schema does not. For consistency and to prevent accidental property additions, consider applying.strict()to the output schema as well.Apply this diff:
-const outputSchema = z.object({ +const outputSchema = z.object({ uiType: z.literal('mdx'), title: z.string(), sectionCount: z.number(), mdxContent: z.string(), -}); +}).strict();
36-36: Avoid double type assertion - verify framework typing.The double type assertion
as unknown as Outputbypasses TypeScript's type checking completely, which reduces type safety. This pattern suggests either a framework typing limitation or a type mismatch that should be addressed.Please verify:
- Whether the
@frontmcp/sdkframework provides proper typing forctx.outputin UI templates- If the framework typing should be improved to avoid the need for type assertions
- If
ctx.outputis guaranteed to match theoutputSchematype at runtimeAs per coding guidelines: Avoid bypassing TypeScript's type system without strong justification. If the framework guarantees
ctx.outputmatchesOutput, consider requesting better type inference from the framework authors.apps/e2e/demo-e2e-notifications/src/apps/notify/tools/trigger-resource-change.tool.ts (1)
5-41: Well-structured tool; minor optional consistency nitThis tool is cleanly implemented: Zod schemas are explicit, types are inferred correctly, and the
ToolContextgenerics, decorator metadata, and side effects (broadcast + log) are wired in a consistent way with the rest of the Tool pattern.One small, purely cosmetic nit you may consider (no need to block on it): the fallback URI value differs between the log payload (
'all-resources'on Line 33) and the human-readable message ('all resources'on Line 39). Aligning these (either both hyphenated or both spaced) would make debugging/log inspection slightly clearer.apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts (2)
20-25: Add.strict()to outputSchema for consistency.The
outputSchemashould use.strict()mode to match the pattern used ininputSchema(line 18) and reject unknown properties, ensuring consistent validation behavior across the tool.Based on learnings, every schema should use
.strict()mode.Apply this diff:
const outputSchema = z.object({ uiType: z.literal('markdown'), title: z.string(), findingCount: z.number(), markdown: z.string(), -}); +}).strict();
37-51: Consider avoiding the double type assertion in the template.Line 38 uses
as unknown as Outputwhich bypasses TypeScript's type safety. While this may be necessary due to SDK typing limitations, consider either:
- Adding runtime validation before the assertion
- Requesting improved typing for
ctx.outputin the SDK's template contextExample with runtime validation:
template: (ctx) => { const output = ctx.output; if (!output || typeof output !== 'object') { throw new Error('Invalid output'); } const { title, findingCount, markdown } = output as Output; // ... rest of template }libs/sdk/src/common/metadata/tool.metadata.ts (1)
158-158: Consider using a more specific type constraint instead ofany.The use of
z.ZodObject<any>violates the coding guideline to avoidanytypes without strong justification. Consider usingz.ZodObject<RawZodShape>orz.ZodObject<z.ZodRawShape>for better type safety.Note that line 141 has the same pattern in
StructuredOutputType, so if you address this, please also update that line for consistency.Apply this diff to improve type safety:
-export type ToolInputType = z.ZodRawShape | z.ZodObject<any>; +export type ToolInputType = z.ZodRawShape | z.ZodObject<RawZodShape>;As per coding guidelines, strict TypeScript mode should avoid
anytypes and use more specific constraints.apps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.ts (2)
4-9: Consider tighteningdelayvalidation for clearer semantics
delayis currentlyz.number().optional().default(0). Given it’s used as a millisecond timeout, you might want to constrain it, e.g.int().min(0)(and possibly a reasonable.max(...)) to avoid negative or non‑integer delays and make the intent clearer. This is non‑blocking but can reduce surprising inputs.If you confirm you’re on Zod 4 as indicated, this shape:
delay: z.number().int().min(0).optional().default(0)will still infer a non‑optional
numberfordelayin the parsedInput.
27-31: Simplify delay condition based on Zod defaulted output typeBecause
delayis defined asz.number().optional().default(0), the inferredInput['delay']seen byexecuteshould always be anumber(default applied when omitted). That makes the first truthiness check redundant; you can simplify to:if (input.delay > 0) { await new Promise((resolve) => setTimeout(resolve, input.delay)); }This keeps the runtime behavior the same while tightening the typing/intent.
Please double‑check your generated
Inputtype (e.g. via your editor ortsc) to confirmdelayis indeed inferred asnumberwith your current Zod version.apps/e2e/demo-e2e-transparent/src/apps/tasks/tools/create-task.tool.ts (1)
32-50: Consider more precise defaults and slightly stronger IDs / type exportsNothing blocking, but a few small robustness tweaks you might consider:
- Use nullish coalescing for defaults so future schema changes don’t accidentally treat other falsy values as “missing”:
const userId = authInfo?.user?.sub ?? 'anonymous';priority: input.priority ?? 'medium',- For IDs,
task-${Date.now()}is fine for a demo but can collide under very fast or parallel creation. If it’s easy, you could switch to something likecrypto.randomUUID()or add a random suffix for better uniqueness.- If these task types are meant to be reused elsewhere in the app (e.g., in resources or prompts), consider
export type CreateTaskInput/Outputso other modules can import them instead of re-declaring task shapes.All of these are optional polish; current behavior is correct for an e2e demo.
apps/e2e/demo-e2e-redis/src/apps/sessions/tools/set-session-data.tool.ts (3)
5-11: Input schema is solid; consider tighteningttlSecondsvalidation.The schema is clear and strict, which is good. If negative or non-integer TTLs are not meaningful for your session store, consider constraining it (e.g. to positive integers) to catch bad inputs earlier:
- ttlSeconds: z.number().optional().describe('Time to live in seconds'), + ttlSeconds: z.number().int().positive().optional().describe('Time to live in seconds'),You could also add
.default(<seconds>)if most callers use a common TTL.
13-17: Optional: make the output schema strict for symmetry with input.Input uses
.strict()while output currently allows extra properties. For consistency and to prevent accidental extra fields leaking out of the tool, you could make the output strict as well:-const outputSchema = z.object({ - success: z.boolean(), - key: z.string(), - message: z.string(), -}); +const outputSchema = z + .object({ + success: z.boolean(), + key: z.string(), + message: z.string(), + }) + .strict();
25-37: Implementation looks correct; double‑checksessionIdfallback behavior.The tool wiring and return shape align with other tools and the session store pattern. One nuance:
const sessionId = this.authInfo.sessionId ?? 'mock-session-default';only falls back whensessionIdisnull/undefined. IfsessionIdcan be an empty string (the test client getter inlibs/testing/src/client/mcp-test-client.tsreturns''when unset), you might prefer:const sessionId = this.authInfo.sessionId || 'mock-session-default';if you want a non-empty, explicit default session ID for all such cases. If
''is intentionally a valid session ID, current code is fine; just confirm the intended behavior.apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts (1)
24-42: Consider importing types instead of duplicating them.The
CodeCallExecuteResultandCodeCallSearchResultinterfaces are defined locally, but these types already exist in the codebase atlibs/plugins/src/codecall/tools/execute.schema.tsandlibs/plugins/src/codecall/codecall.types.ts. Importing the canonical types would reduce duplication and ensure consistency if the source types evolve.Apply this pattern to import the existing types:
-// Type for codecall:execute result -interface CodeCallExecuteResult<T> { - status: 'ok' | 'error' | 'timeout' | 'runtime_error' | 'syntax_error' | 'tool_error' | 'illegal_access'; - result?: T; - error?: unknown; - logs?: string[]; -} - -// Type for codecall:search result -interface CodeCallSearchResult { - tools: Array<{ - name: string; - appId: string; - description: string; - relevanceScore: number; - matchedQueries: string[]; - }>; - warnings?: Array<{ type: string; message: string }>; - totalAvailableTools: number; -} +import type { CodeCallExecuteResult } from '@frontmcp/plugins/codecall'; +import type { CodeCallSearchResult } from '@frontmcp/plugins/codecall';apps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.ts (1)
62-65: Avoid the type assertion; type the factory parameter directly.The
ctx as FrontMcpContextassertion circumvents TypeScript's type inference. Sinceinjectdeclares[FRONTMCP_CONTEXT], the factory parameter should be typed directly. Per coding guidelines, avoid type assertions when proper typing is possible.- useFactory: async (ctx): Promise<RequestLogger> => { - const frontMcpContext = ctx as FrontMcpContext; - const requestId = frontMcpContext.requestId || 'unknown'; - const sessionId = frontMcpContext.sessionId || 'unknown'; + useFactory: async (ctx: FrontMcpContext): Promise<RequestLogger> => { + const requestId = ctx.requestId || 'unknown'; + const sessionId = ctx.sessionId || 'unknown';apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts (1)
46-56: Consider defensive coding for codeExample access.While runtime-safe (because
hasCodeis derived from!!input.codeExample), TypeScript may not infer thatcodeExampleis defined whenhasCodeis true. Consider using optional chaining or a default value for extra type safety.\`\`\`typescript -${codeExample} +${codeExample ?? ''} \`\`\`Or add a type assertion comment explaining the invariant:
// hasCode guarantees codeExample is defined ${codeExample}apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (1)
8-8: Consider simplifying.optional().default(400)to just.default(400).The
.optional().default(400)pattern works but is redundant. Using.default(400)alone achieves the same effect: the field is optional in the input and always present in the output with the default value when omitted.Apply this diff to simplify:
- statusCode: z.number().optional().default(400).describe('HTTP status code'), + statusCode: z.number().default(400).describe('HTTP status code'),apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts (1)
5-49: Tightenprioritytyping to remove redundant optional and unsafe cast
priorityis declared asoptional().default('all')but is treated as always defined inexecute, and you need anas TaskPrioritycast. You can rely ondefault('all')alone to handle missing/undefined values and make the inferred type required, which then lets TS narrow correctly and removes the need for the cast and thestorealias.Suggested refactor:
-const inputSchema = z - .object({ - priority: z.enum(['low', 'medium', 'high', 'all']).optional().default('all').describe('Filter by priority'), - }) - .strict(); +const inputSchema = z + .object({ + // `default('all')` is enough to handle missing/undefined and keeps the type non-optional + priority: z.enum(['low', 'medium', 'high', 'all']).default('all').describe('Filter by priority'), + }) + .strict(); @@ export default class ListTasksTool extends ToolContext<typeof inputSchema, typeof outputSchema> { async execute(input: Input): Promise<Output> { - const store = taskStore; - let tasks = input.priority === 'all' ? store.getAll() : store.getByPriority(input.priority as TaskPriority); + const tasks = + input.priority === 'all' + ? taskStore.getAll() + : taskStore.getByPriority(input.priority); @@ - tasks: tasks.map((t) => ({ + tasks: tasks.map((t) => ({ id: t.id, title: t.title, description: t.description, priority: t.priority, status: t.status, createdAt: t.createdAt, })), count: tasks.length, }; } }This removes the unnecessary optional, improves the inferred
Input['priority']type, and avoids theas TaskPriorityassertion, aligning better with the strict typing guidelines. As per coding guidelines, this keeps runtime behaviour and types in sync while avoiding casts.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (51)
apps/e2e/demo-e2e-cache/src/apps/compute/tools/expensive-operation.tool.ts(1 hunks)apps/e2e/demo-e2e-cache/src/apps/compute/tools/get-cache-stats.tool.ts(1 hunks)apps/e2e/demo-e2e-cache/src/apps/compute/tools/non-cached.tool.ts(1 hunks)apps/e2e/demo-e2e-cache/src/apps/compute/tools/reset-stats.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-list.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-stats.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-delete.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts(1 hunks)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-update.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts(1 hunks)apps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.ts(1 hunks)apps/e2e/demo-e2e-hooks/src/apps/audit/tools/clear-audit-log.tool.ts(1 hunks)apps/e2e/demo-e2e-hooks/src/apps/audit/tools/get-audit-log.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/list-events.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/create-note.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/list-notes.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/create-task.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts(1 hunks)apps/e2e/demo-e2e-notifications/src/apps/notify/tools/long-running-task.tool.ts(1 hunks)apps/e2e/demo-e2e-notifications/src/apps/notify/tools/trigger-progress.tool.ts(1 hunks)apps/e2e/demo-e2e-notifications/src/apps/notify/tools/trigger-resource-change.tool.ts(1 hunks)apps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.ts(1 hunks)apps/e2e/demo-e2e-providers/src/apps/config/tools/get-app-info.tool.ts(1 hunks)apps/e2e/demo-e2e-providers/src/apps/config/tools/get-request-info.tool.ts(1 hunks)apps/e2e/demo-e2e-public/src/apps/notes/tools/create-note.tool.ts(1 hunks)apps/e2e/demo-e2e-public/src/apps/notes/tools/list-notes.tool.ts(1 hunks)apps/e2e/demo-e2e-redis/src/apps/sessions/tools/get-session-data.tool.ts(1 hunks)apps/e2e/demo-e2e-redis/src/apps/sessions/tools/set-session-data.tool.ts(1 hunks)apps/e2e/demo-e2e-serverless/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-serverless/src/apps/serverless/tools/cold-start-test.tool.ts(1 hunks)apps/e2e/demo-e2e-serverless/src/apps/serverless/tools/serverless-info.tool.ts(1 hunks)apps/e2e/demo-e2e-transparent/src/apps/tasks/tools/create-task.tool.ts(1 hunks)apps/e2e/demo-e2e-transparent/src/apps/tasks/tools/list-tasks.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-card.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-list.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-chart.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.ts(1 hunks)libs/sdk/src/common/metadata/tool.metadata.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts
🚧 Files skipped from review as they are similar to previous changes (23)
- apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.ts
- apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-delete.tool.ts
- apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-list.tool.ts
- apps/e2e/demo-e2e-cache/src/apps/compute/tools/non-cached.tool.ts
- apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/list-events.tool.ts
- apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts
- apps/e2e/demo-e2e-public/src/apps/notes/tools/list-notes.tool.ts
- apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/create-task.tool.ts
- apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts
- apps/e2e/demo-e2e-notifications/src/apps/notify/tools/trigger-progress.tool.ts
- apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-card.tool.ts
- apps/e2e/demo-e2e-serverless/src/apps/serverless/tools/serverless-info.tool.ts
- apps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-chart.tool.ts
- apps/e2e/demo-e2e-serverless/src/apps/serverless/tools/cold-start-test.tool.ts
- apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/create-note.tool.ts
- apps/e2e/demo-e2e-cache/src/apps/compute/tools/reset-stats.tool.ts
- apps/e2e/demo-e2e-redis/src/apps/sessions/tools/get-session-data.tool.ts
- apps/e2e/demo-e2e-cache/src/apps/compute/tools/get-cache-stats.tool.ts
- apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-stats.tool.ts
- apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-update.tool.ts
- apps/e2e/demo-e2e-hooks/src/apps/audit/tools/clear-audit-log.tool.ts
- apps/e2e/demo-e2e-transparent/src/apps/tasks/tools/list-tasks.tool.ts
- apps/e2e/demo-e2e-providers/src/apps/config/tools/get-app-info.tool.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.tsapps/e2e/demo-e2e-notifications/src/apps/notify/tools/long-running-task.tool.tslibs/sdk/src/common/metadata/tool.metadata.tsapps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-list.tool.tsapps/e2e/demo-e2e-providers/src/apps/config/tools/get-request-info.tool.tsapps/e2e/demo-e2e-multiapp/src/apps/notes/tools/list-notes.tool.tsapps/e2e/demo-e2e-hooks/src/apps/audit/tools/get-audit-log.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.tsapps/e2e/demo-e2e-redis/src/apps/sessions/tools/set-session-data.tool.tsapps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.tsapps/e2e/demo-e2e-transparent/src/apps/tasks/tools/create-task.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.tsapps/e2e/demo-e2e-public/src/apps/notes/tools/create-note.tool.tsapps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.tsapps/e2e/demo-e2e-cache/src/apps/compute/tools/expensive-operation.tool.tsapps/e2e/demo-e2e-serverless/jest.e2e.config.tsapps/e2e/demo-e2e-notifications/src/apps/notify/tools/trigger-resource-change.tool.tsapps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/common/metadata/tool.metadata.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/common/metadata/tool.metadata.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
🧠 Learnings (21)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
Applied to files:
libs/sdk/src/common/metadata/tool.metadata.tsapps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/sdk/src/common/metadata/tool.metadata.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.tsapps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/metadata/tool.metadata.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/list-notes.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.tsapps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.tsapps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/react-form.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Use Zod schema validation for all component inputs as a core validation strategy
Applied to files:
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.tsapps/e2e/demo-e2e-serverless/jest.e2e.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Enable strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
apps/e2e/demo-e2e-serverless/jest.e2e.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use type parameters with constraints instead of unconstrained generics, and prefer `unknown` over `any` for generic type defaults
Applied to files:
apps/e2e/demo-e2e-serverless/jest.e2e.config.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Follow the preset pattern for hierarchical configurations across the codebase
Applied to files:
apps/e2e/demo-e2e-serverless/jest.e2e.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts
🧬 Code graph analysis (19)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts (4)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (1)
Tool(19-29)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts (1)
Tool(21-38)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts (1)
Tool(19-40)
apps/e2e/demo-e2e-notifications/src/apps/notify/tools/long-running-task.tool.ts (3)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (3)
inputSchema(12-17)Input(19-19)Output(41-41)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)
apps/e2e/demo-e2e-providers/src/apps/config/tools/get-request-info.tool.ts (1)
apps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.ts (2)
RequestLogger(11-18)REQUEST_LOGGER_TOKEN(6-6)
apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/list-notes.tool.ts (3)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (1)
inputSchema(12-17)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts (1)
Tool(21-32)libs/sdk/src/common/decorators/decorator-utils.ts (1)
store(184-188)
apps/e2e/demo-e2e-hooks/src/apps/audit/tools/get-audit-log.tool.ts (3)
apps/e2e/demo-e2e-cache/src/apps/compute/tools/get-cache-stats.tool.ts (1)
Tool(15-33)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts (1)
Tool(23-34)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts (1)
Tool(21-32)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts (7)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (3)
inputSchema(12-17)Input(19-19)Output(41-41)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-list.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts (1)
Tool(23-34)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-update.tool.ts (1)
Tool(29-41)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)
apps/e2e/demo-e2e-redis/src/apps/sessions/tools/set-session-data.tool.ts (3)
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)libs/testing/src/client/mcp-test-client.ts (1)
sessionId(206-208)
apps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.ts (4)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (3)
inputSchema(12-17)Input(19-19)Output(41-41)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts (1)
Tool(23-34)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)
apps/e2e/demo-e2e-transparent/src/apps/tasks/tools/create-task.tool.ts (6)
apps/e2e/demo-e2e-cache/src/apps/compute/tools/expensive-operation.tool.ts (1)
Tool(22-46)apps/e2e/demo-e2e-cache/src/apps/compute/tools/get-cache-stats.tool.ts (1)
Tool(15-33)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts (1)
Tool(23-34)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-update.tool.ts (1)
Tool(29-41)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (3)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts (1)
Tool(17-33)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts (1)
Tool(21-38)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts (1)
Tool(19-40)
apps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.ts (2)
libs/testing/src/client/mcp-test-client.ts (1)
sessionId(206-208)libs/sdk/src/index.ts (2)
FRONTMCP_CONTEXT(27-27)FrontMcpContext(21-21)
apps/e2e/demo-e2e-public/src/apps/notes/tools/create-note.tool.ts (11)
apps/e2e/demo-e2e-cache/src/apps/compute/tools/expensive-operation.tool.ts (1)
Tool(22-46)apps/e2e/demo-e2e-cache/src/apps/compute/tools/non-cached.tool.ts (1)
Tool(21-43)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts (1)
Tool(23-34)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts (1)
Tool(21-32)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-update.tool.ts (1)
Tool(29-41)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (1)
Tool(19-29)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts (1)
Tool(17-33)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts (1)
Tool(21-38)
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts (3)
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-codecall/src/apps/crm/data/crm.store.ts (1)
crmStore(152-152)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts (5)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (3)
inputSchema(12-17)Input(19-19)Output(41-41)apps/e2e/demo-e2e-cache/src/apps/compute/tools/expensive-operation.tool.ts (1)
Tool(22-46)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (1)
Tool(19-29)
apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts (4)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (1)
inputSchema(12-17)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts (1)
Tool(21-32)libs/sdk/src/common/decorators/decorator-utils.ts (1)
store(184-188)
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts (2)
libs/plugins/src/codecall/tools/execute.schema.ts (1)
CodeCallExecuteResult(146-152)libs/plugins/src/codecall/codecall.types.ts (1)
CodeCallSearchResult(396-404)
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-log.tool.ts (8)
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-list.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/activities-stats.tool.ts (1)
Tool(13-23)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-delete.tool.ts (1)
Tool(16-30)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-list.tool.ts (1)
Tool(21-32)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-update.tool.ts (1)
Tool(29-41)apps/e2e/demo-e2e-codecall/src/apps/crm/data/crm.store.ts (1)
crmStore(152-152)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts (4)
apps/e2e/demo-e2e-cache/src/apps/compute/tools/non-cached.tool.ts (1)
Tool(21-43)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts (3)
apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-create.tool.ts (1)
Tool(25-36)apps/e2e/demo-e2e-codecall/src/apps/crm/tools/users-get.tool.ts (1)
Tool(24-35)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(18-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts
Outdated
Show resolved
Hide resolved
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts
Outdated
Show resolved
Hide resolved
… enhance error handling in tools
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts (1)
32-33: Avoid double type assertions - use proper typing instead.These lines use
as unknown as Typeassertions to bypass TypeScript's type checking, which violates the strict TypeScript coding guidelines. This issue was previously flagged but remains unaddressed.As per coding guidelines, avoid type assertions and use proper error handling instead. Consider:
- Using type guards to validate the structure at runtime
- Adding proper type constraints to the framework's template function signature
- Filing an issue with the framework to improve type definitions
Example with type guards:
template: (ctx) => { - const { topic, points, hasCode } = ctx.output as unknown as Output; - const codeExample = (ctx.input as unknown as Input).codeExample; + if (!ctx.output || typeof ctx.output !== 'object') { + throw new Error('Invalid output context'); + } + if (!ctx.input || typeof ctx.input !== 'object') { + throw new Error('Invalid input context'); + } + const output = ctx.output as Output; + const input = ctx.input as Input; + const { topic, points, hasCode } = output; + const codeExample = input.codeExample;
🧹 Nitpick comments (3)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts (1)
1-24: Strict schemas and Tool metadata look solid and type‑safe
- Good use of
z.object(...).strict()for both input and output, andz.infer‑basedInput/Outputtypes—noanyor non‑null assertions and aligns with the strict TS guidance.- The defaulted
triggerflag plus description make the intent clear and keep the runtime contract simple.@Toolmetadata is minimal and accurate for this error‑demo tool.If you want extra polish, you could also add a
.describe(...)on thesuccessfield to improve generated docs/UX, but that’s purely optional. Based on learnings, this matches the preset pattern of using explicit schemas and documentation.apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts (1)
6-9: Optionally constrainstatusCodeto valid HTTP status rangeThe tool behavior and typing look good, and using
PublicMcpErrormatches the guidelines. To make the schema stricter and self-documenting, you could constrainstatusCodeto integer HTTP codes:- statusCode: z.number().default(400).describe('HTTP status code'), + statusCode: z.number().int().min(100).max(599).default(400).describe('HTTP status code'),This keeps the same default while preventing non-integer or out-of-range values from slipping through.
apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts (1)
30-30: Consider clarifying the description.The description says "List all tasks" but the tool supports filtering by priority. Consider updating to something like "List tasks with optional priority filtering" for clarity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts(1 hunks)apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts(1 hunks)apps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/list-notes.tool.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts(1 hunks)apps/e2e/demo-e2e-redis/src/apps/sessions/tools/set-session-data.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts
- apps/e2e/demo-e2e-redis/src/apps/sessions/tools/set-session-data.tool.ts
- apps/e2e/demo-e2e-multiapp/src/apps/notes/tools/list-notes.tool.ts
- apps/e2e/demo-e2e-hooks/src/apps/audit/tools/audited.tool.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.tsapps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts
🧠 Learnings (20)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-custom-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use HTMX schema with strict mode including get, post, put, delete, target, swap, and trigger properties
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Use Zod schema validation for all component inputs as a core validation strategy
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Use consistent enum naming for variants ('primary', 'secondary', 'outline', 'ghost', 'danger', 'success') and sizes ('xs', 'sm', 'md', 'lg', 'xl')
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Enable strict TypeScript mode with no `any` types without strong justification - use `unknown` instead for generic type defaults
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use type parameters with constraints instead of unconstrained generics, and prefer `unknown` over `any` for generic type defaults
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Ensure all TypeScript builds complete without warnings and all tests pass with 100% pass rate before committing code
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.tsapps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.ts : Always use `escapeHtml()` utility for all user-provided content to prevent XSS vulnerabilities
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts
🧬 Code graph analysis (3)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts (3)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts (1)
Tool(24-68)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts (1)
Tool(32-89)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts (1)
Tool(30-65)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts (3)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(20-33)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts (1)
Tool(32-89)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts (1)
Tool(30-65)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts (3)
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts (1)
Tool(24-68)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-doc.tool.ts (1)
Tool(30-65)apps/e2e/demo-e2e-ui/src/apps/widgets/tools/mdx-interactive.tool.ts (1)
Tool(24-76)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-internal-error.tool.ts (1)
25-35: Correct use ofInternalMcpErrorwith explicit MCP error codeThe
executeimplementation is concise and follows the MCP error guidelines:
- Throws the specific
InternalMcpErrorclass instead of a genericError.- Uses a clear, stable MCP error code string (
SIMULATED_INTERNAL_ERROR) suitable for tests and client handling.- Return type
Promise<Output>is consistent with the non‑error path, and there are no unsafe casts oranyusage.This is exactly the kind of internal-error demo tool we want for the E2E errors suite. As per coding guidelines, the error handling pattern here looks correct.
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-not-found.tool.ts (1)
1-40: LGTM! Clean implementation for error testing.The tool implementation follows all coding guidelines:
- Strict Zod schemas with proper type inference
- Specific error class (
ResourceNotFoundError) with appropriate URI format- No
anytypes or non-null assertions- Proper use of the
ToolContextpattern and decorator systemThe past type inference issue was successfully addressed. The simulated resource Set is appropriate for e2e testing purposes.
apps/e2e/demo-e2e-ui/src/apps/widgets/tools/html-table.tool.ts (1)
1-68: LGTM! Previous issues resolved.The type inference and template implementation have been corrected. The code now properly:
- Uses
z.infer<typeof inputSchema>without the redundant wrapper (line 21)- Accesses input data from
ctx.inputin the template (line 32)- Applies
escapeHtmlto all user-provided content (lines 36, 42, 49)- Uses
.strict()mode on schemas (lines 10, 19)The pattern of returning minimal metadata in
execute()while the UI template handles full rendering is consistent with the demo app's design.apps/e2e/demo-e2e-ui/src/apps/widgets/tools/markdown-report.tool.ts (1)
1-89: LGTM! Previous issues resolved.The type inference has been corrected. The code now properly:
- Uses
z.infer<typeof inputSchema>without the redundant wrapper (line 29)- Uses
.strict()mode on schemas (lines 18, 27)- Correctly accesses
ctx.outputin the template for processed data (line 40)- Implements a clean separation:
execute()processes findings into markdown, while the UI template wraps it with additional formattingThe severity emoji mapping and markdown generation logic is well-structured and consistent with similar markdown/MDX tools in the codebase.
apps/e2e/demo-e2e-errors/src/apps/errors/tools/throw-validation-error.tool.ts (1)
4-40: Validation tool implementation looks correct and consistentInput/output schemas, inferred types, and the execute logic (conditional
InvalidInputErrorwith useful metadata, otherwise returning a typed success payload) are all coherent and align with the intended “validation error” demo behavior. No changes needed.apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts (1)
1-9: LGTM! Clean imports and input schema.The input schema is well-structured with strict validation and proper default handling.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (1)
11-183: Error-path, invalid-input, and TTL-expiry scenarios still missingThe suite currently exercises only happy-path cache behavior. Per the repo’s test guidelines, we also want coverage for error and edge paths to push overall coverage to ~95%+ and ensure robust behavior, e.g.:
- Invalid or out-of-range tool inputs (e.g., negative
complexity, missingoperationId) returning error responses.- Behavior when the underlying tool fails (ensuring failures aren’t cached).
- TTL expiry/invalidation behavior, if the CachePlugin is configured with a TTL (e.g., call, advance time/wait, call again and verify re-execution).
- Malformed/invalid cache resources (bad
cache://URIs) and prompt invocations with invalid args returning appropriate errors.These gaps were noted in a previous review; adding a dedicated
"Error Handling"describe with such cases would align this file with the global**/*.test.tscoverage requirements.
Based on learnings and coding guidelines.
🧹 Nitpick comments (15)
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts (1)
24-42: Import types from the source package instead of redefining them.The local
CodeCallExecuteResultandCodeCallSearchResultinterfaces duplicate types that already exist in the codebase (libs/plugins/src/codecall/tools/execute.schema.tsandlibs/plugins/src/codecall/codecall.types.ts). This violates the DRY principle and risks type drift if the source definitions evolve.Replace the local type definitions with imports:
import { test, expect } from '@frontmcp/testing'; +import type { CodeCallExecuteResult, CodeCallSearchResult } from '@frontmcp/plugins'; // Expected seed data from crm.store.ts const SEED_USERS = [ { id: 'user-1', name: 'John Doe', email: 'john@acme.com', company: 'Acme Inc', role: 'CEO' }, { id: 'user-2', name: 'Jane Smith', email: 'jane@globex.com', company: 'Globex Corp', role: 'CTO' }, { id: 'user-3', name: 'Bob Johnson', email: 'bob@initech.com', company: 'Initech', role: 'Manager' }, ]; const SEED_ACTIVITY_STATS = { total: 3, byType: { call: 1, email: 1, meeting: 1 }, byUser: { 'user-1': 2, 'user-2': 1 }, }; - -// Type for codecall:execute result -interface CodeCallExecuteResult<T> { - status: 'ok' | 'error' | 'timeout' | 'runtime_error' | 'syntax_error' | 'tool_error' | 'illegal_access'; - result?: T; - error?: unknown; - logs?: string[]; -} - -// Type for codecall:search result -interface CodeCallSearchResult { - tools: Array<{ - name: string; - appId: string; - description: string; - relevanceScore: number; - matchedQueries: string[]; - }>; - warnings?: Array<{ type: string; message: string }>; - totalAvailableTools: number; -}apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts (6)
17-57: Consider adding at least one negative path for Notes toolsThe Notes tests cover core happy paths (create, list, resource read, prompt) but never exercise validation or error behavior. To better catch regressions in schema/validation, consider adding a case such as calling
create-notewith missing/invalid fields and asserting the expected MCP error code and message.As per coding guidelines, **/*.test.ts should include error-path coverage where feasible.
72-93: Strengthen Tasks list/filter assertionsRight now the Tasks tests mainly assert that the response contains generic markers (
"tasks","high"). They won’t catch issues like incorrect filtering or missing items. Consider tightening them by, for example:
- Asserting that
list-tasksafter creatingList Task Testincludes that title.- In the priority filter test, asserting that only
high-priority tasks are present (or thatlow-priority tasks are absent) if the payload shape makes this practical.This would make the E2E checks more diagnostic without adding much complexity.
141-146: Upcoming-only events semantics are only weakly validatedThe
upcomingOnlytest currently just asserts that the response contains"events", which doesn’t prove that past events are actually filtered. If the calendar app exposes event timestamps, it would be more robust to:
- Create at least one past and one future event within the test.
- Call
list-eventswith{ upcomingOnly: true }.- Assert that only future events appear (or at least that past events do not).
This would better validate the multi-app calendar behavior you’re targeting.
164-191: Optionally assert per-app metadata during tool discoveryThe cross-app tool discovery tests nicely verify that all expected tool names are present and that your three apps contribute 2 tools each. If the tool metadata includes app/namespace information (e.g.,
app,namespace, or similar), it could be useful to also assert that:
- Each tool from Notes/Tasks/Calendar is tagged with the correct app/namespace.
- No tool appears under the wrong app.
This would more directly exercise the “namespace handling” behavior mentioned in the file header, but it’s an optional enhancement.
246-258: Make the “independent storage” test more meaningfulThe
each app should have independent storagetest currently only checks for the presence of acountfield, which won’t catch regressions where counts are wrong or storage is shared. If the server state is reset per test (or can be reset via a helper/tool), consider:
- Asserting that notes count is at least
2and tasks count is at least1(or exactly those values when you control state).- Or capturing baseline counts, creating new items, and asserting that only the relevant app’s count increased.
This would give the test real signal about per-app storage isolation rather than just response shape.
11-260: Add at least one explicit cross-app error/isolation regression checkAcross the suite you exercise happy-path isolation and discovery thoroughly, which is great. One thing that’s missing, per the test guidelines, is an explicit error-path case that proves the server rejects cross-app misuse. For example:
- Attempting to read
tasks://allvia a Notes-only context if such a distinction exists, or- Calling a non-existent tool/prompt and asserting the correct MCP error code and message.
Even a single such E2E test would round out this multi-app suite and better guard against future changes in routing or namespace resolution.
As per coding guidelines, **/*.test.ts should include error-path and error-class coverage where practical.
apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts (1)
17-126: Consider documenting session isolation behavior.The tests store data with various keys across different test cases without explicit cleanup. While this may be intentional for session testing, it's worth clarifying whether:
- Each test case runs with an isolated session
- Session state is shared across tests within the same
describeblock- The test framework automatically cleans up session state between tests
If session isolation is not guaranteed, consider adding explicit cleanup:
test.afterEach(async ({ mcp }) => { // Clean up session data after each test // This ensures tests don't interfere with each other });Or add a comment documenting the isolation behavior:
test.describe('Redis Session E2E (Mocked)', () => { test.use({ server: 'apps/e2e/demo-e2e-redis/src/main.ts', publicMode: true, }); + // Note: Each test runs with an isolated session provided by the test frameworkapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
139-155: Add XSS and error scenario tests for resource access.Resource access tests only verify successful operations. Based on learnings, UI tests should test HTML escaping for user-provided content to prevent XSS attacks.
Consider adding:
test('should handle invalid resource URIs', async ({ mcp }) => { const content = await mcp.resources.read('ui://invalid-resource'); expect(content).toBeErrorResult(); }); test('should escape HTML in templates to prevent XSS', async ({ mcp }) => { const content = await mcp.resources.read('ui://templates'); expect(content).toBeSuccessful(); // Verify that any user-provided content is properly escaped // This prevents XSS attacks when templates contain user input const htmlContent = content.contents[0]?.text || ''; expect(htmlContent).not.toContain('<script>'); // Add more specific XSS vector tests based on template structure });Based on learnings, HTML escaping tests are important for preventing XSS attacks in UI components.
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (3)
54-61: Prefer structured assertions over string-based JSON matching for statsRight now the execution-count checks rely on:
const statsContent = JSON.stringify(stats);expect(statsContent).toContain('"expensive-operation":1');(and similar)This is a bit brittle (formatting, extra fields, or wrapper changes could break these assertions while behavior is still correct). If the tool response exposes a structured stats object (e.g., an
executionCountsmap or similar), consider asserting against the numeric property values instead of string search. That will make failures more precise and less tied to serialization details.Also applies to: 76-82, 96-102, 121-127
131-147: Optional: add explicit state reset around resource tests for determinismThe resource tests rely only on the presence of the
cache://statsresource and some generic keys in the returned content, which is fine. For extra determinism (and in case you later assert specific counts), you might want to reset stats in abeforeEachand explicitly drive the state (similar to other describes) rather than inheriting any execution history from prior tests.
149-171: Make prompt content-type expectations strict to avoid silent passesIn
should generate cache report, assertions are guarded by:const message = result.messages[0]; if (message.content.type === 'text') { // expectations... }If the content type ever changes, the test will silently skip the assertions and still pass. To keep this test meaningful, assert the type explicitly or fail otherwise, e.g.:
const message = result.messages[0]; expect(message.content.type).toBe('text'); expect(message.content.text).toContain('Execution Statistics'); expect(message.content.text).toContain('expensive-operation');(or throw in an
elsebranch). This ensures the report shape is actually validated.apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts (3)
62-72: Align internal error assertions with the “error ID” intentThe comment says internal errors should have an error ID for support, but the test only checks that the text contains
'error'. If the testing API exposes the underlying error payload, consider also asserting that the support error ID is present (e.g., someerrorIdor equivalent field) so the test actually enforces that contract.Based on learnings, this would better validate the MCP error schema for internal errors.
83-94: Consider asserting custom error metadata (code / status) if exposedFor the custom error case you verify it’s an MCP error and that the message contains
'custom error', which is good. If@frontmcp/testinglets you inspect the structured error (e.g.,data.code,data.statusCode, etc.), it would be worth asserting that:
- the custom error code matches
'CUSTOM_ERROR', and- the status code matches
400.That would fully exercise the custom error surface, not just the message text.
Based on learnings, this would strengthen coverage of custom MCP error types.
130-153: Tighten prompt result checks by assertingcontent.type === 'text'The prompt tests only check the message text inside an
if (message.content.type === 'text')block. If the type isn’t'text', the test still passes, which weakens the contract.You can assert the content type explicitly and then always check the text content:
- const message = result.messages[0]; - if (message.content.type === 'text') { - expect(message.content.text).toContain('MCP Error Codes Reference'); - } + const message = result.messages[0]; + expect(message.content.type).toBe('text'); + expect(message.content.text).toContain('MCP Error Codes Reference'); @@ - const message = result.messages[0]; - if (message.content.type === 'text') { - expect(message.content.text).toContain('Internal Error'); - } + const message = result.messages[0]; + expect(message.content.type).toBe('text'); + expect(message.content.text).toContain('Internal Error');This makes the tests fail if the prompt result stops returning text content, which better enforces the intended MCP schema.
Based on learnings, this matches the schema‑validation‑first testing approach.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
apps/demo/jest.config.ts(1 hunks)apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-hooks/e2e/hooks.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-multiapp/project.json(1 hunks)apps/e2e/demo-e2e-notifications/e2e/notifications.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-notifications/project.json(1 hunks)apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-providers/e2e/providers.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.ts(1 hunks)apps/e2e/demo-e2e-public/e2e/public-auth.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-serverless/project.json(1 hunks)apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts(1 hunks)apps/ui/mdx-demo/jest.config.ts(1 hunks)apps/ui/react-demo/jest.config.ts(1 hunks)jest.config.ts(1 hunks)libs/adapters/jest.config.ts(1 hunks)libs/cli/jest.config.ts(1 hunks)libs/json-schema-to-zod-v3/jest.config.ts(1 hunks)libs/mcp-from-openapi/jest.config.ts(1 hunks)libs/plugins/jest.config.ts(1 hunks)libs/sdk/jest.config.ts(1 hunks)libs/testing/jest.config.ts(1 hunks)libs/testing/src/expect.ts(1 hunks)libs/ui/jest.config.ts(1 hunks)package.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- jest.config.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
- apps/e2e/demo-e2e-providers/src/apps/config/providers/request-logger.provider.ts
- apps/e2e/demo-e2e-public/e2e/public-auth.e2e.test.ts
- apps/e2e/demo-e2e-notifications/e2e/notifications.e2e.test.ts
- apps/e2e/demo-e2e-serverless/project.json
- apps/e2e/demo-e2e-providers/e2e/providers.e2e.test.ts
- apps/e2e/demo-e2e-hooks/e2e/hooks.e2e.test.ts
- apps/e2e/demo-e2e-notifications/project.json
- apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.ts
- apps/e2e/demo-e2e-multiapp/project.json
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/json-schema-to-zod-v3/jest.config.tslibs/cli/jest.config.tslibs/mcp-from-openapi/jest.config.tsapps/ui/react-demo/jest.config.tslibs/plugins/jest.config.tsapps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/demo/jest.config.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tslibs/sdk/jest.config.tslibs/ui/jest.config.tsapps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/ui/mdx-demo/jest.config.tslibs/testing/src/expect.tslibs/adapters/jest.config.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.tslibs/testing/jest.config.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/json-schema-to-zod-v3/jest.config.tslibs/cli/jest.config.tslibs/mcp-from-openapi/jest.config.tslibs/plugins/jest.config.tslibs/sdk/jest.config.tslibs/ui/jest.config.tslibs/testing/src/expect.tslibs/adapters/jest.config.tslibs/testing/jest.config.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.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/jest.config.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/json-schema-to-zod-v3/jest.config.tslibs/cli/jest.config.tslibs/mcp-from-openapi/jest.config.tsapps/ui/react-demo/jest.config.tslibs/plugins/jest.config.tsapps/demo/jest.config.tslibs/sdk/jest.config.tslibs/ui/jest.config.tsapps/ui/mdx-demo/jest.config.tslibs/adapters/jest.config.tslibs/testing/jest.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
libs/json-schema-to-zod-v3/jest.config.tslibs/cli/jest.config.tslibs/mcp-from-openapi/jest.config.tslibs/plugins/jest.config.tsapps/demo/jest.config.tslibs/sdk/jest.config.tslibs/adapters/jest.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.schema.ts : Every component must have a Zod schema with `.strict()` mode to reject unknown properties
Applied to files:
libs/json-schema-to-zod-v3/jest.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/cli/jest.config.tslibs/mcp-from-openapi/jest.config.tsapps/ui/react-demo/jest.config.tslibs/plugins/jest.config.tsapps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tslibs/sdk/jest.config.tslibs/ui/jest.config.tsapps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/ui/mdx-demo/jest.config.tslibs/testing/src/expect.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.tslibs/testing/jest.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tslibs/testing/src/expect.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tslibs/testing/src/expect.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use type parameters with constraints instead of unconstrained generics, and prefer `unknown` over `any` for generic type defaults
Applied to files:
libs/testing/src/expect.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `getCapabilities()` for dynamic capability exposure instead of hardcoding capabilities in adapters
Applied to files:
libs/adapters/jest.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.tsapps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
package.json
🧬 Code graph analysis (7)
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.ts (1)
libs/testing/src/expect.ts (1)
expect(80-80)
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts (1)
libs/testing/src/expect.ts (1)
expect(80-80)
libs/testing/src/expect.ts (1)
libs/testing/src/matchers/matcher-types.ts (1)
McpMatchers(15-301)
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
libs/testing/src/expect.ts (1)
expect(80-80)
apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts (1)
libs/testing/src/expect.ts (1)
expect(80-80)
apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts (1)
libs/testing/src/expect.ts (1)
expect(80-80)
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts (4)
libs/plugins/src/codecall/tools/execute.schema.ts (1)
CodeCallExecuteResult(146-152)libs/plugins/src/codecall/codecall.types.ts (1)
CodeCallSearchResult(396-404)libs/testing/src/expect.ts (1)
expect(80-80)scripts/bump-synchronized-versions.mjs (1)
result(130-130)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (18)
package.json (1)
33-42: Verify Nx 22.2.3 compatibility with Node 22+ and confirm no breaking changes.This is a significant toolchain upgrade affecting the entire monorepo. While the version bump is applied consistently across all Nx packages, Nx 22.x may introduce breaking changes to workspace configuration, plugin behavior, or build outputs.
Please verify the following before merging:
- Confirm Nx 22.2.3 officially supports Node 22+ (your engine requirement on line 6).
- Review the Nx 22.x release notes and migration guide for any breaking changes.
- Verify all build and test scripts (
nx build,nx test,nx run-many) execute successfully with the new version.Also applies to: 58-58
apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts (1)
44-411: Well-structured and comprehensive test suite.The test suite provides excellent coverage of the CodeCall plugin functionality:
- Meta-tools exposure and CRM tool discovery
- Tool search with multiple query patterns and topK limiting
- Tool description with schema validation
- Tool execution across various scenarios (list, stats, aggregation, specific user, creation, filtering)
- Error handling for nonexistent tools and script errors
- Resource and prompt access patterns
The guard pattern
if (!execResult.result) throw new Error(...)is correctly applied throughout, and type narrowing is properly implemented. All previous non-null assertion issues have been properly addressed.libs/plugins/jest.config.ts (1)
2-39: LGTM: Standard Jest config migration to CommonJS exports.The switch from ES module default export to CommonJS
module.exportsis a standard and well-supported approach for Jest configuration files. This change, applied consistently across all 8 Jest config files in the PR (libs/plugins, libs/mcp-from-openapi, apps/demo, apps/ui/react-demo, apps/ui/mdx-demo, libs/adapters, libs/testing, libs/ui), is purely mechanical and preserves all configuration values.libs/json-schema-to-zod-v3/jest.config.ts (1)
1-34: LGTM: Jest config now uses CommonJS export with unchanged settings.Switching to
module.exportskeeps the Jest configuration behavior the same while aligning with the repo‑wide CommonJS pattern for config files.libs/sdk/jest.config.ts (1)
2-20: LGTM: CommonJS Jest config with SWC spec loading looks correct.Using
require/module.exportshere is appropriate for Jest, and reading.spec.swcrcintoswcJestConfigwhile disablingswcrclookup is a clean pattern. No functional changes to the Jest settings themselves.libs/cli/jest.config.ts (1)
1-36: LGTM: CLI Jest config now exported via CommonJS with same behavior.The move to
module.exportsmatches the project‑wide pattern and preserves the existing Jest configuration for the CLI tests.apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts (1)
11-16: Test harness configuration looks solidThe
test.usesetup cleanly binds this suite to the multiapp server in public mode; no issues here, and it matches the intended E2E surface.apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (2)
10-16: Test setup looks good, but consider platform configurations.The test configuration is correct. However, based on learnings, UI tests should ideally test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable to ensure UI tools render correctly across different MCP clients.
Based on learnings, consider adding platform-specific test variants if UI rendering differs across platforms.
180-200: LGTM - Tool discovery test is comprehensive.The tool discovery test properly verifies all 8 UI tools across the 4 types (HTML, React, MDX, Markdown). The structure is clear and maintainable.
apps/e2e/demo-e2e-serverless/e2e/serverless.e2e.test.ts (3)
9-15: LGTM! Clean test setup.The imports and test configuration are properly structured, with appropriate use of the testing framework and clear server configuration.
11-165: Test structure is well-organized.The test suite has clear organization with logical groupings (Serverless Info, Cold Start Testing, Environment Resource, etc.) and descriptive test names that document the expected behavior.
Note: The past review comment comprehensively addresses the missing error path testing required by coding guidelines for 95%+ coverage.
131-136: No changes needed. Theverbose: 'true'parameter is correct. The prompt framework enforcesRecord<string, string>for all arguments per the MCP protocol specification, requiring all values to be passed as strings. The prompt implementation explicitly handles this withargs['verbose'] === 'true'.apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (2)
11-22: Solid end-to-end coverage of core cache behaviorsThe suite cleanly exercises cached vs non-cached tools, execution counts, and basic stats flows with clear setup via
test.useandreset-statsin the main cache-focused describes. Nothing blocking here; this is a good foundation for the cache E2E story.Also applies to: 23-33, 34-83, 85-103, 105-129
173-183: Tool discovery checks look goodThe discovery test cleanly verifies that all expected cache-related tools are exposed from the server. This is a useful sanity check for the wiring and complements the behavior-focused tests.
libs/testing/src/expect.ts (3)
27-43: Well-structured generic type parameters for sync/async matcher support.The addition of the
R extends void | Promise<void>type parameter correctly models synchronous and asynchronous matcher returns, and the use ofunknownas the default forTaligns with the coding guidelines. The nestednot,resolves, andrejectsproperties are properly typed with appropriate constraints.
49-49: Correct: Base expect returns synchronous matchers.Specifying
McpExpectMatchers<void, T>for the base expect call correctly indicates synchronous matchers, with async matchers accessible via.resolvesand.rejects.
80-80: Type assertion bypasses type checking but follows established patterns.The double type assertion
as unknown as McpExpectbypasses TypeScript's type safety checks. While this is necessary for the Playwright-style typing approach (as documented in the comments), be aware that this means any runtime incompatibilities betweenjestExpectand theMcpExpectinterface won't be caught at compile time.As a best practice for SDK libraries, consider adding runtime validation or integration tests to confirm the runtime behavior matches the type definitions, especially for the custom matcher extensions.
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts (1)
12-167: Strong, well‑structured E2E coverage for MCP error flowsThis suite does a solid job exercising both error and success paths for all error tools, resources, prompts, and tool discovery, with clear assertions on MCP error codes and text content. It aligns well with the “test all code paths including errors” guidance for
**/*.test.ts.Based on learnings, this is a good application of the TypeScript‑first, protocol‑level testing philosophy.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (3)
18-161: Add error-path and validation tests for UI tools.All tool tests (HTML/React/MDX/Markdown) cover only happy paths with valid inputs. Per test guidelines, you should also cover invalid parameters, validation failures, and edge cases to reach required coverage.
Consider adding, for each tool family, tests that:
- Call tools with missing required fields, wrong types, or out-of-range values.
- Trigger validation failures (unknown properties, invalid variants/options).
- Exercise edge cases (empty/huge arrays, null/undefined fields) and assert error results and codes.
This will also help maintain the 95%+ coverage target across branches and error paths. As per coding guidelines, comprehensive error-path testing is required.
181-189: Add prompt error-path tests (invalid name / missing args).Prompt tests only verify successful access to
ui-showcase. To align with the test guidelines and prior feedback, please add cases such as:
- Calling
mcp.prompts.getwith a non-existent prompt name and asserting an error result.- If
ui-showcasedefines required arguments, calling it without those args and asserting the appropriate validation/error response.These will ensure prompt handling is robust and help satisfy the 95%+ coverage requirement for error branches. As per coding guidelines, all code paths—including failures—should be tested.
187-200: Ensure prompt content type is asserted so tests fail on unexpected types.Because assertions are wrapped in
if (message.content.type === 'text'), the test can silently pass without checking content when the type is wrong. Add an explicit assertion on the type before narrowing.Suggested change:
- const message = result.messages[0]; - if (message.content.type === 'text') { - expect(message.content.text).toContain('UI Tools Showcase'); - expect(message.content.text).toContain('HTML Type'); - expect(message.content.text).toContain('React Type'); - expect(message.content.text).toContain('MDX Type'); - expect(message.content.text).toContain('Markdown Type'); - } + const message = result.messages[0]; + expect(message.content.type).toBe('text'); + if (message.content.type === 'text') { + expect(message.content.text).toContain('UI Tools Showcase'); + expect(message.content.text).toContain('HTML Type'); + expect(message.content.text).toContain('React Type'); + expect(message.content.text).toContain('MDX Type'); + expect(message.content.text).toContain('Markdown Type'); + }This guarantees the test fails if the prompt stops returning text content or changes type.
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (1)
173-184: Good addition of error scenario coverage.This addresses part of the previous review feedback by testing validation errors for missing required fields. The test correctly expects error code -32602 (INVALID_PARAMS) for Zod validation failures.
Consider extending error coverage in a follow-up with tests for:
- Invalid resource URI reads (e.g.,
cache://invalid)- Out-of-range complexity values (if validated)
apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts (2)
83-97: TTL test name/behavior misaligned with actual expiration semantics.This test only verifies immediate retrieval with a TTL parameter; it doesn’t assert that the value actually expires, despite the file header mentioning “TTL expiration handling”. This can give a misleading sense of coverage around expiry logic.
Consider either:
- Option A: Actually test expiration with a short TTL (if the environment supports time control or a short sleep), or
- Option B: Rename the test to reflect what it validates and document that expiration itself isn’t being asserted.
For Option B, something like:
- test('should store data with TTL', async ({ mcp }) => { + test('should store and retrieve data with TTL parameter (no expiration verified)', async ({ mcp }) => { @@ - // Should be accessible immediately + // Should be accessible immediately; TTL expiration is not validated in this environment const getResult = await mcp.tools.call('get-session-data', { key: 'ttl-key' });
11-133: Add focused error-path E2E tests to meet coverage and guidelines.All tests here exercise happy paths only; there are no assertions for error responses or failure branches (invalid/missing params, invalid prompt/resource IDs, backend errors). As per coding guidelines for
**/*.test.ts, we need coverage of error paths as well, not just success flows.Consider adding a small error-focused describe block, e.g.:
test.describe('Redis Session E2E Errors', () => { test('should error on missing key parameter for set-session-data', async ({ mcp }) => { const result = await mcp.tools.call('set-session-data', { // key missing on purpose value: 'test-value', }); expect(result).toBeError(); }); test('should error on negative TTL', async ({ mcp }) => { const result = await mcp.tools.call('set-session-data', { key: 'bad-ttl', value: 'value', ttlSeconds: -1, }); expect(result).toBeError(); }); test('should error on non-existent session resource', async ({ mcp }) => { const content = await mcp.resources.read('session://non-existent'); expect(content).toBeError(); }); test('should error on non-existent session prompt', async ({ mcp }) => { const result = await mcp.prompts.get('non-existent-prompt', {}); expect(result).toBeError(); }); });Adjust matcher names to whatever
@frontmcp/testingactually exposes, but the key point is: explicitly cover the failure branches for tools, resources, and prompts to align with the 95%+ coverage and error-path requirements. As per coding guidelines, comprehensive error-path coverage is required for this test file.
🧹 Nitpick comments (6)
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
57-68: Tighten XSS assertion to use result content matchers instead ofJSON.stringify.
JSON.stringify(result)may not reflect the rendered payload accurately (and could break ifresultgains non-serializable fields). To more directly verify escaping, prefer asserting against the rendered content via the existing matchers.For example:
- const content = JSON.stringify(result); - expect(content).not.toContain('<script>alert'); + expect(result).not.toHaveTextContent('<script>alert');This keeps the test aligned with the MCP testing surface and focused on the actual output content.
apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts (2)
17-57: Consider adding error path tests for Notes App.The Notes App tests cover happy paths well (create, list, read resource, get prompt), but there are no tests for error scenarios such as:
- Creating a note with missing required fields
- Reading a non-existent resource
Example addition:
test('should reject note with missing title', async ({ mcp }) => { const result = await mcp.tools.call('create-note', { content: 'Content without title', }); expect(result).toBeError(); });As per coding guidelines: Test all code paths including errors.
121-172: Consider adding error path tests for Calendar App.The Calendar App tests cover CRUD operations and resource/prompt access, but there are no tests for validation errors such as:
- Invalid time ranges (endTime before startTime)
- Missing required fields (startTime, endTime)
Example addition:
test('should reject event with invalid time range', async ({ mcp }) => { const now = Date.now(); const result = await mcp.tools.call('create-event', { title: 'Invalid Event', startTime: now + 3600000, // Start after end endTime: now, }); expect(result).toBeError(); });As per coding guidelines: Test all code paths including errors.
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts (1)
140-146: Add assertion for unexpected content types to avoid silent test failures.The type narrowing with
if (message.content.type === 'text')could cause assertions to be silently skipped if the content type is unexpectedly not 'text'. Consider adding an else clause or a fail assertion.if (message.content.type === 'text') { expect(message.content.text).toContain('MCP Error Codes Reference'); // Verify all standard error codes are documented expect(message.content.text).toContain('INVALID_PARAMS'); expect(message.content.text).toContain('RESOURCE_NOT_FOUND'); expect(message.content.text).toContain('INTERNAL_ERROR'); + } else { + throw new Error(`Expected text content type but got: ${message.content.type}`); }Also applies to: 159-162
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (1)
166-170: Add assertion for unexpected content types to avoid silent test failures.Similar to the errors test file, the type narrowing could cause assertions to be silently skipped.
if (message.content.type === 'text') { expect(message.content.text).toContain('Execution Statistics'); expect(message.content.text).toContain('expensive-operation'); + } else { + throw new Error(`Expected text content type but got: ${message.content.type}`); }apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts (1)
99-133: Strengthen prompt summary assertions by enforcing expected message type.Right now, the test only checks content when
message.content.type === 'text'; if the type is wrong, the test silently skips the assertion and still passes. You can make this stricter by asserting the type before narrowing:- const message = result.messages[0]; - if (message.content.type === 'text') { - expect(message.content.text).toContain('session'); - } + const message = result.messages[0]; + expect(message.content.type).toBe('text'); + expect(message.content.text).toContain('session');This ensures the test fails if the prompt response shape changes unexpectedly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/e2e/demo-e2e-codecall/e2e/codecall.e2e.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
🧬 Code graph analysis (2)
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts (2)
scripts/bump-synchronized-versions.mjs (1)
result(130-130)libs/testing/src/expect.ts (1)
expect(80-80)
apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts (2)
scripts/bump-synchronized-versions.mjs (1)
result(130-130)libs/testing/src/expect.ts (1)
expect(80-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (16)
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
163-179: Resource and tool discovery coverage looks solid.Listing/reading
ui://templatesand asserting on type markers, plus verifying all UI tools appear inmcp.tools.list(), provides good e2e coverage of discovery and resource access for this demo app. No issues from my side here.Also applies to: 204-223
apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts (4)
1-16: LGTM - Clean test setup with appropriate configuration.The file header clearly documents the test scope, and the test fixture configuration with
publicMode: trueis appropriate for E2E testing.
96-103: Good validation error test coverage for Tasks App.This test properly validates that invalid priority values are rejected with an error, which aligns with the coding guidelines for testing error paths.
174-222: LGTM - Comprehensive cross-app discovery tests.The cross-app discovery tests thoroughly verify that all tools, resources, and prompts from all three apps are discoverable through the MCP interface. The explicit count verification (6 tools total) provides a good regression safeguard.
224-270: LGTM - Well-structured app isolation tests.The isolation tests effectively verify that data from one app doesn't leak into another app's resources. The verification of independent storage per app with count checks provides good coverage for multi-app scenarios.
apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts (4)
1-16: LGTM - Well-documented error handling test suite.The file header clearly documents the MCP error types being tested, and the test configuration is appropriate for E2E testing.
18-39: Excellent error path coverage with specific MCP error codes.The validation error tests properly verify both the failure case (with specific error code -32602/INVALID_PARAMS) and the success case. This aligns well with the coding guidelines for testing all code paths.
41-81: LGTM - Good coverage for Not Found and Internal errors.The tests properly verify:
- Not Found errors with MCP code -32002 (RESOURCE_NOT_FOUND)
- Internal errors with MCP code -32603 (INTERNAL_ERROR)
- Both failure and success paths for each error type
166-176: LGTM - Comprehensive tool discovery verification.The test verifies all expected error demo tools are discoverable, ensuring the error handling infrastructure is properly registered.
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (4)
1-16: LGTM - Clear test documentation and setup.The file header clearly documents the cache behavior being tested, and the test configuration is appropriate for E2E testing.
17-83: LGTM - Thorough cache behavior verification.The cached tool behavior tests effectively verify:
- First call execution
- Cache hit on subsequent identical calls (execution count = 1)
- Cache miss with different inputs (execution count = 2)
The use of
beforeEachto reset stats ensures test isolation.
105-129: LGTM - Effective cache vs non-cache comparison.This test clearly demonstrates the difference in execution counts between cached (1) and non-cached (3) tools with identical inputs, providing good regression coverage for cache functionality.
186-195: LGTM - Complete tool discovery verification.All expected cache-related tools are verified to be discoverable.
apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts (3)
1-16: Harness configuration and suite structure look solid.Nice use of
test.usewith the demo server entrypoint andpublicMode: true, and the high-level describe blocks mirror the features under test clearly.
17-52: Good baseline coverage for basic session storage semantics.These tests cover storing, retrieving, and the “missing key → null” behavior in a clear, readable way and validate both success and content.
54-81: Session persistence and resource read coverage look good.The multi-key persistence and
session://currentresource checks cover important cross-call/session behavior and validate both success and expected content.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.ts (2)
5-13: Confirm intended semantics ofdescriptiondefault + optional
descriptionis defined asz.string().optional().default(''). With Zod 4, this means missing/undefinedvalues will be defaulted to'', and the inferred output type is effectively a non‑undefined string. If you want the field to truly remain optional (possiblyundefined) you might drop the.default(''), or if you want it always present, you can remove.optional()for clarity.
41-52: Minor refactor: inlineeventStoreand object mapping
const store = eventStore;is unnecessary indirection; you can calleventStore.createdirectly. Also, since you return a 1:1 mapping ofevent, a small refactor can reduce repetition:- const store = eventStore; - const event = store.create(input.title, input.description, input.startTime, input.endTime, input.location); - - return { - id: event.id, - title: event.title, - description: event.description, - startTime: event.startTime, - endTime: event.endTime, - location: event.location, - createdAt: event.createdAt, - }; + const event = eventStore.create( + input.title, + input.description, + input.startTime, + input.endTime, + input.location, + ); + + const { id, title, description, startTime, endTime, location, createdAt } = event; + return { id, title, description, startTime, endTime, location, createdAt };Purely a readability/DRY improvement; behavior stays the same.
apps/e2e/demo-e2e-codecall/jest.e2e.config.ts (1)
1-33: Configuration follows the established pattern correctly.The Jest e2e configuration is well-structured and consistent with the preset pattern used across the codebase. All referenced files exist, and the settings are appropriate for end-to-end tests (60s timeout, sequential execution with maxWorkers: 1).
Consider adding a type annotation for better type safety, following the pattern used in similar e2e configs like
apps/e2e/demo-e2e-serverless/jest.e2e.config.ts:+import type { Config } from '@jest/types'; + -export default { +const config: Config.InitialOptions = { displayName: 'demo-e2e-codecall', preset: '../../../jest.preset.js', // ... -}; +}; + +export default config;apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
204-217: Tighten error assertions for stronger guarantees.The error‑path tests for missing HTML headers and non‑existent prompts are valuable, but
expect(result).toBeError()is fairly generic. If your matchers support it, consider also asserting on the specific error code and/or a key part of the message (for example, invalid params vs not found) so regressions in error mapping can’t slip through unnoticed.As per coding guidelines, more specific error assertions help keep error-path coverage robust.
apps/e2e/demo-e2e-cache/project.json (1)
1-51: Config mirrors other e2e apps; verify Jest coverage and consider simplifying targets.This project.json is consistent with the other e2e apps and should work fine. As with
demo-e2e-errors, just ensurejest.e2e.config.tswrites coverage where theoutputsfields expect it fortestvstest:e2e. If this app will never have separate unit tests, you could also consider dropping or aliasing the plaintesttarget to reduce duplication.apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (1)
57-82: Consider more structured assertions for stats and a named error code.Two small polish ideas:
- For stats,
JSON.stringify(stats)plus substring checks works but is a bit brittle; if the test helpers or result shape expose the underlying JSON (e.g., via acontentpayload or custom matcher), prefer asserting on parsed data or a higher‑level matcher instead of string contains.- If your SDK exposes a constant for the JSON‑RPC
INVALID_PARAMScode, using that instead of the raw-32602literal will make the intent clearer and reduce magic numbers.These are optional, the current tests are still readable and effective.
Also applies to: 99-102, 121-127, 176-186
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-cache/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-cache/project.json(1 hunks)apps/e2e/demo-e2e-codecall/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-codecall/project.json(1 hunks)apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-errors/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-errors/project.json(1 hunks)apps/e2e/demo-e2e-hooks/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-hooks/project.json(1 hunks)apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.ts(1 hunks)apps/e2e/demo-e2e-openapi/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-openapi/project.json(1 hunks)apps/e2e/demo-e2e-providers/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-providers/project.json(1 hunks)apps/e2e/demo-e2e-public/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-public/project.json(1 hunks)apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-redis/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-redis/project.json(1 hunks)apps/e2e/demo-e2e-transparent/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-transparent/project.json(1 hunks)apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-ui/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-ui/project.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (17)
- apps/e2e/demo-e2e-redis/project.json
- apps/e2e/demo-e2e-providers/jest.e2e.config.ts
- apps/e2e/demo-e2e-multiapp/e2e/multiapp.e2e.test.ts
- apps/e2e/demo-e2e-hooks/jest.e2e.config.ts
- apps/e2e/demo-e2e-redis/e2e/redis-session.e2e.test.ts
- apps/e2e/demo-e2e-public/jest.e2e.config.ts
- apps/e2e/demo-e2e-codecall/project.json
- apps/e2e/demo-e2e-providers/project.json
- apps/e2e/demo-e2e-openapi/project.json
- apps/e2e/demo-e2e-transparent/jest.e2e.config.ts
- apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts
- apps/e2e/demo-e2e-public/project.json
- apps/e2e/demo-e2e-redis/jest.e2e.config.ts
- apps/e2e/demo-e2e-ui/jest.e2e.config.ts
- apps/e2e/demo-e2e-errors/jest.e2e.config.ts
- apps/e2e/demo-e2e-hooks/project.json
- apps/e2e/demo-e2e-cache/jest.e2e.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-openapi/jest.e2e.config.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-codecall/jest.e2e.config.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-ui/project.jsonapps/e2e/demo-e2e-openapi/jest.e2e.config.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-errors/project.jsonapps/e2e/demo-e2e-transparent/project.jsonapps/e2e/demo-e2e-codecall/jest.e2e.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
apps/e2e/demo-e2e-ui/project.json
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Follow the preset pattern for hierarchical configurations across the codebase
Applied to files:
apps/e2e/demo-e2e-openapi/jest.e2e.config.tsapps/e2e/demo-e2e-codecall/jest.e2e.config.ts
🧬 Code graph analysis (2)
apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.ts (3)
libs/cli/src/templates/3rd-party-integration/src/tools/example.list.ts (3)
inputSchema(12-17)Input(19-19)Output(41-41)apps/e2e/demo-e2e-errors/src/apps/errors/tools/successful.tool.ts (1)
Tool(20-33)apps/e2e/demo-e2e-multiapp/src/apps/tasks/tools/list-tasks.tool.ts (1)
Tool(28-51)
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (2)
scripts/bump-synchronized-versions.mjs (1)
result(130-130)libs/testing/src/expect.ts (1)
expect(80-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (9)
apps/e2e/demo-e2e-ui/project.json (2)
34-50: Verify intent of shared Jest config betweentestandtest:e2etargets.Both the
testandtest:e2etargets reference the same Jest config file (jest.e2e.config.ts). This is atypical—normally:
testtarget usesjest.config.tsfor unit teststest:e2etarget usesjest.e2e.config.tsfor end-to-end testsConfirm whether this is intentional or if the
testtarget should reference a different Jest config. The distinction matters for test setup, timeout handling, and test environment isolation.
1-27: Project configuration structure looks solid.The build, serve, and target setup follows Nx best practices. Webpack-based build for Node target, development/production configurations, and proper output paths are well-defined. Ensure the referenced files (
webpack.config.js,tsconfig.app.json,jest.e2e.config.ts,src/main.ts) exist and are properly configured.apps/e2e/demo-e2e-openapi/jest.e2e.config.ts (1)
1-33: LGTM! Configuration follows established e2e testing patterns.The Jest configuration is well-structured and consistent with the hierarchical preset pattern. The settings are appropriate for e2e testing: serial execution prevents race conditions, the 60-second timeout accommodates longer-running integration tests, and the SWC transform properly handles TypeScript with decorator metadata.
apps/e2e/demo-e2e-multiapp/src/apps/calendar/tools/create-event.tool.ts (2)
15-23: Output schema is consistent and minimal
outputSchemamirrors the event shape cleanly (including optionallocationandcreatedAt), which should make the tool’s contract predictable for callers and aligns with how similar tools structure their responses.
35-39: Good use of explicit time-range validation and typed errorThe
startTime >= endTimeguard withInvalidInputErroris clear, uses a specific error type from the SDK instead of a genericError, and the message matches the condition (“before end time” excludes equality).apps/e2e/demo-e2e-errors/project.json (1)
1-51: Nx project config looks solid; just double‑check Jest coverage paths.Targets and options follow the same pattern as the other e2e apps and look correct. The only thing to verify is that
apps/e2e/demo-e2e-errors/jest.e2e.config.tswrites coverage to the directories you declare inoutputsfortestvstest:e2e; if Jest’scoverageDirectoryis shared, Nx caching may not match the actual coverage location.Based on learnings, the Nx usage itself aligns with the repo’s build guidelines.
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
18-201: Comprehensive UI E2E coverage and good use of content assertions.The suite does a nice job exercising all UI tool types (HTML/React/MDX/Markdown), resource listing/reading, prompts, and tool discovery, and the assertions on content tokens (e.g.
rowCount,maxValue,sectionCount,completedCount) make the checks meaningful. The explicitexpect(message.content.type).toBe('text')before inspectingmessage.content.textand the XSS-focused table test (html-tablewith<script>input) align well with the testing and security guidelines.As per coding guidelines, this hits both happy paths and key validation/XSS behaviors.
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (1)
11-197: Good end‑to‑end coverage of cache behavior and integration points.The suite thoroughly covers cached vs non‑cached execution, comparative stats, resource reads (
cache://stats), prompt generation (cache-report), and tool discovery, withreset-statsensuring isolation between tests. The explicit validation check for missingoperationIdusingtoBeError(-32602)is a nice alignment with the JSON‑RPC invalid‑params semantic and the testing guidelines for error paths.As per coding guidelines, this gives solid coverage of both happy paths and a key validation failure.
apps/e2e/demo-e2e-transparent/project.json (1)
1-51: Transparent e2e project config is consistent with the other demos.The targets and options match the established pattern and should integrate cleanly with Nx. As with the other e2e configs, just confirm that
jest.e2e.config.tsuses coverage directories compatible with theoutputsfortestandtest:e2eso Nx caching metadata stays accurate.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (1)
177-189: Previous review comment not fully addressed - expand error coverage.The "Error Scenarios" section contains only one error test (missing operationId), but the previous review comment requested comprehensive error testing across multiple categories. The following scenarios remain untested:
- Input validation: negative complexity, out-of-range values
- Cache failures: behavior when cache operations fail or throw errors
- TTL behavior: cache expiration and invalidation after TTL
- Resource errors: malformed URIs (e.g.,
cache://invalid), read failures- Prompt errors: invalid arguments to
cache-reportpromptWithout these tests, the suite will not achieve the 95%+ coverage requirement mandated by coding guidelines.
Add comprehensive error tests:
test.describe('Error Scenarios', () => { test('should handle missing required operationId', async ({ mcp }) => { const result = await mcp.tools.call('expensive-operation', { complexity: 5, }); expect(result).toBeError(); expect(result).toHaveTextContent('operationId'); }); test('should validate complexity constraints', async ({ mcp }) => { const result = await mcp.tools.call('expensive-operation', { operationId: 'test', complexity: -1, // Invalid negative value }); expect(result).toBeError(); }); test('should handle resource read with malformed URI', async ({ mcp }) => { const result = await mcp.resources.read('cache://invalid-resource'); expect(result).toBeError(); }); test('should verify cache invalidation after TTL', async ({ mcp }) => { await mcp.tools.call('reset-stats', {}); // First call await mcp.tools.call('expensive-operation', { operationId: 'ttl-test', complexity: 1, }); // Wait for TTL expiration (30s default + buffer) await new Promise(resolve => setTimeout(resolve, 31000)); // Second call should re-execute await mcp.tools.call('expensive-operation', { operationId: 'ttl-test', complexity: 1, }); const stats = await mcp.tools.call('get-cache-stats', {}); const statsContent = JSON.stringify(stats); expect(statsContent).toContain('"expensive-operation":2'); }); });As per coding guidelines: Test all code paths including errors and achieve 95%+ coverage.
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (2)
70-99: Extend error coverage for React tools.React tool tests currently only cover happy paths. To achieve 95% coverage per coding guidelines, add error cases such as invalid data types (e.g.,
data: 'not-an-array'for react-chart) and missing required fields.Based on learnings, test all code paths including errors.
101-160: Add error path tests for MDX and Markdown tools.Similar to React tools, MDX and Markdown tool tests only cover successful operations. Consider adding tests for missing required fields, invalid types, and validation failures to meet the 95% coverage requirement.
Based on learnings, comprehensive error path testing is essential.
🧹 Nitpick comments (12)
apps/e2e/demo-e2e-redis/src/apps/sessions/resources/session-current.resource.ts (1)
21-33: Error handling is a recommended refactor for robustness, not a critical issue for e2e demo code.The code uses safe null-coalescing (
??) rather than non-null assertions, which aligns with coding guidelines. For a straightforward e2e demo resource, the simple implementation is acceptable. If error handling is desired, wrapgetSessionStore()andstore.getAll()calls in try-catch and return appropriate error responses rather than throwing, which is the MCP pattern for application-level errors in resource operations.Verify the return type of
store.getAll()matches the schema'sdatafield type independently from this resource's implementation.apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
162-178: Consider adding invalid resource URI test.Resource tests only verify successful access. Add a test for reading a non-existent or invalid resource URI (e.g.,
widgets://invalid-resource) to verify error handling.apps/e2e/demo-e2e-ui/src/apps/widgets/resources/ui-templates.resource.ts (1)
22-48: Calculate totalTools dynamically to prevent drift.The hardcoded
totalTools: 8could become incorrect if the tools arrays are modified. Calculate it dynamically to ensure accuracy and reduce maintenance burden.Apply this diff:
async execute(): Promise<z.infer<typeof outputSchema>> { - return { - uiTypes: [ + const uiTypes = [ { type: 'html', tools: ['html-table', 'html-card'], description: 'Plain HTML templates with optional Handlebars enhancement', }, { type: 'react', tools: ['react-chart', 'react-form'], description: 'React component templates with SSR support', }, { type: 'mdx', tools: ['mdx-doc', 'mdx-interactive'], description: 'MDX templates combining Markdown and JSX', }, { type: 'markdown', tools: ['markdown-report', 'markdown-list'], description: 'Pure Markdown templates for documentation', }, - ], - totalTools: 8, + ]; + + return { + uiTypes, + totalTools: uiTypes.reduce((sum, t) => sum + t.tools.length, 0), }; }libs/testing/src/auth/mock-api-server.ts (2)
114-118: Non-null assertion onthis.servercan be avoided.Per coding guidelines, avoid non-null assertions (
!). Although the server is just assigned on line 107, the callback could theoretically execute after an error clears it.this.server.listen(port, () => { - const address = this.server!.address(); + const address = this.server?.address(); if (!address || typeof address === 'string') { reject(new Error('Failed to get server address')); return; }
143-154: Consider using optional chaining instead of non-null assertion.The guard at line 139 ensures
this.serveris not null, but using the non-null assertion still violates the coding guideline. Using optional chaining or assigning to a local variable is safer.return new Promise((resolve, reject) => { - this.server!.close((err) => { + const server = this.server; + if (!server) { + resolve(); + return; + } + server.close((err) => { if (err) { reject(err); } else { this.server = null; this._info = null; this.log('Mock API server stopped'); resolve(); } }); });apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.ts (2)
138-196: Consider extracting the session initialization pattern into a helper.The initialize → get session ID → send notification sequence is repeated in all three test suites. While acceptable for e2e tests, extracting this to a helper function would reduce duplication and make tests more maintainable.
// Helper function to initialize MCP session async function initializeSession(baseUrl: string): Promise<string | null> { const response = await fetch(`${baseUrl}/`, { method: 'POST', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/event-stream', }, body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'initialize', params: { capabilities: {}, clientInfo: { name: 'test-client', version: '1.0.0' }, protocolVersion: '2024-11-05', }, }), }); const sessionId = response.headers.get('mcp-session-id'); // Send initialized notification await fetch(`${baseUrl}/`, { method: 'POST', headers: { 'Content-Type': 'application/json', Accept: 'application/json, text/event-stream', 'mcp-session-id': sessionId ?? '', }, body: JSON.stringify({ jsonrpc: '2.0', method: 'notifications/initialized', }), }); return sessionId; }
191-195: Assertions could be more specific.The test asserts
toolsText.toContain('tools')which is a weak assertion - it only checks for the presence of the word "tools" in the response. Consider parsing the JSON and validating the structure.expect(toolsResponse.ok).toBe(true); const toolsText = await toolsResponse.text(); - // Should have generated tools from OpenAPI spec - expect(toolsText).toContain('tools'); + // Should have generated tools from OpenAPI spec + const toolsResult = JSON.parse(toolsText); + expect(toolsResult.result).toHaveProperty('tools'); + expect(Array.isArray(toolsResult.result.tools)).toBe(true); });apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts (2)
22-51: Simplify the token factory and mock OAuth server setup.The current setup creates the token factory and mock server twice. The pattern of starting, stopping, and restarting the mock server is confusing and may lead to resource leaks if the first
mockOAuthinstance isn't properly cleaned up.beforeAll(async () => { - // Create token factory and mock OAuth server - tokenFactory = new TestTokenFactory({ - issuer: 'http://localhost', // Will be updated after mock server starts - audience: 'frontmcp-test', - }); - - mockOAuth = new MockOAuthServer(tokenFactory, { debug: false }); - const oauthInfo = await mockOAuth.start(); - - // Update token factory with actual issuer URL + // Start mock OAuth server first to get the actual port + const tempTokenFactory = new TestTokenFactory({ + issuer: 'http://localhost', + audience: 'frontmcp-test', + }); + const tempMockOAuth = new MockOAuthServer(tempTokenFactory, { debug: false }); + const tempOauthInfo = await tempMockOAuth.start(); + await tempMockOAuth.stop(); + + // Create final token factory and mock server with correct issuer tokenFactory = new TestTokenFactory({ - issuer: oauthInfo.issuer, - audience: oauthInfo.issuer, + issuer: `http://localhost:${tempOauthInfo.port}`, + audience: `http://localhost:${tempOauthInfo.port}`, }); - mockOAuth = new MockOAuthServer(tokenFactory, { debug: false }); - await mockOAuth.stop(); - const newOauthInfo = await mockOAuth.start(); + mockOAuth = new MockOAuthServer(tokenFactory, { port: tempOauthInfo.port, debug: false }); + const oauthInfo = await mockOAuth.start(); // Start MCP server pointing to mock OAuth server = await TestServer.start({ command: 'npx tsx apps/e2e/demo-e2e-transparent/src/main.ts', env: { - IDP_PROVIDER_URL: newOauthInfo.baseUrl, - IDP_EXPECTED_AUDIENCE: newOauthInfo.issuer, + IDP_PROVIDER_URL: oauthInfo.baseUrl, + IDP_EXPECTED_AUDIENCE: oauthInfo.issuer, }, startupTimeout: 30000, debug: false, }); }, 60000);Alternatively, consider updating
TestTokenFactoryto allow changing the issuer after construction, which would simplify this pattern across tests.
176-192: Protected resource metadata test has overly permissive assertions.The test accepts any of
[200, 301, 302, 307, 308, 404]as valid statuses, which makes it difficult to catch regressions. Consider narrowing the expected behavior or documenting why such flexibility is needed.- // Should return metadata, 404 if not configured, or redirect - expect([200, 301, 302, 307, 308, 404]).toContain(response.status); + // OAuth protected resource metadata should be available + // Note: Server may redirect to canonical URL or return 404 if not configured + expect([200, 404]).toContain(response.status);libs/testing/src/auth/mock-oauth-server.ts (3)
97-102: Non-null assertion onthis.servershould be avoided.Same issue as in
mock-api-server.ts. Per coding guidelines, avoid non-null assertions.this.server.listen(port, () => { - const address = this.server!.address(); + const address = this.server?.address(); if (!address || typeof address === 'string') { reject(new Error('Failed to get server address')); return; }
128-139: Same non-null assertion pattern as mock-api-server.Consider extracting this server lifecycle pattern to a shared utility since both mock servers use identical start/stop logic.
273-280: Body reading lacks size limit.The
readBodymethod accumulates all incoming chunks without a size limit, which could lead to memory exhaustion if a client sends a large payload. For a test utility this is low risk, but consider adding a reasonable limit.private readBody(req: IncomingMessage): Promise<string> { return new Promise((resolve, reject) => { const chunks: Buffer[] = []; + let totalSize = 0; + const maxSize = 1024 * 1024; // 1MB limit req.on('data', (chunk) => chunks.push(chunk)); + totalSize += chunk.length; + if (totalSize > maxSize) { + req.destroy(); + reject(new Error('Request body too large')); + return; + } + chunks.push(chunk); + }); req.on('end', () => resolve(Buffer.concat(chunks).toString())); req.on('error', reject); }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-openapi/src/apps/ecommerce/index.ts(1 hunks)apps/e2e/demo-e2e-providers/e2e/providers.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-redis/src/apps/sessions/resources/session-current.resource.ts(1 hunks)apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-transparent/src/main.ts(1 hunks)apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts(1 hunks)apps/e2e/demo-e2e-ui/src/apps/widgets/resources/ui-templates.resource.ts(1 hunks)libs/testing/src/auth/mock-api-server.ts(1 hunks)libs/testing/src/auth/mock-oauth-server.ts(1 hunks)libs/testing/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/e2e/demo-e2e-errors/e2e/errors.e2e.test.ts
- apps/e2e/demo-e2e-providers/e2e/providers.e2e.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/testing/src/index.tsapps/e2e/demo-e2e-redis/src/apps/sessions/resources/session-current.resource.tsapps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tsapps/e2e/demo-e2e-transparent/src/main.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tslibs/testing/src/auth/mock-oauth-server.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.tslibs/testing/src/auth/mock-api-server.tsapps/e2e/demo-e2e-ui/src/apps/widgets/resources/ui-templates.resource.tsapps/e2e/demo-e2e-openapi/src/apps/ecommerce/index.ts
libs/*/src/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Files:
libs/testing/src/index.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/testing/src/index.tslibs/testing/src/auth/mock-oauth-server.tslibs/testing/src/auth/mock-api-server.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
🧠 Learnings (15)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/*/src/index.ts : Use barrel exports (index.ts) to export all public API surface without legacy exports or aliases
Applied to files:
libs/testing/src/index.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
libs/testing/src/index.tsapps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.tsapps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
apps/e2e/demo-e2e-transparent/src/main.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Maintain 95%+ test coverage across statements, branches, functions, and lines
Applied to files:
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tsapps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-11-05T15:00:47.800Z
Learnt from: frontegg-david
Repo: agentfront/frontmcp PR: 11
File: libs/core/src/auth/jwks/jwks.service.ts:62-0
Timestamp: 2025-11-05T15:00:47.800Z
Learning: In the FrontMCP codebase (libs/core/src/auth/jwks/jwks.service.ts), the verifyGatewayToken method intentionally skips signature verification when running in development-only no-auth mode, using decodeJwtPayloadSafe instead of jwtVerify. This is a deliberate design for local development convenience and should not be flagged as a security issue when the PR or code context indicates development/no-auth mode.
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test HTML escaping for user-provided content to prevent XSS attacks
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Add JSDoc examples with example tags showing basic usage and options patterns for all components
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.ts : Validate component options using `validateOptions()` helper and return error box on validation failure
Applied to files:
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/** : Organize components into schema.ts (definitions) and implementation .ts files with matching names
Applied to files:
apps/e2e/demo-e2e-ui/src/apps/widgets/resources/ui-templates.resource.ts
🧬 Code graph analysis (4)
apps/e2e/demo-e2e-transparent/src/main.ts (1)
libs/testing/src/index.ts (1)
LogLevel(58-58)
apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (1)
libs/testing/src/expect.ts (1)
expect(80-80)
libs/testing/src/auth/mock-api-server.ts (1)
libs/testing/src/index.ts (5)
MockRoute(85-85)MockResponse(85-85)MockAPIServerOptions(85-85)MockAPIServerInfo(85-85)MockAPIServer(84-84)
apps/e2e/demo-e2e-ui/src/apps/widgets/resources/ui-templates.resource.ts (1)
apps/e2e/demo-e2e-redis/src/apps/sessions/resources/session-current.resource.ts (1)
Resource(11-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (29)
apps/e2e/demo-e2e-openapi/src/apps/ecommerce/index.ts (3)
1-4: LGTM!The imports are clean and follow standard conventions.
6-9: LGTM!The environment variable configuration with fallbacks is appropriate for E2E testing. The use of the
||operator correctly handles bothundefinedand empty string values.
11-27: LGTM!The app configuration follows the framework's decorator pattern correctly. The
includeSecurityInInput: trueoption is appropriate for E2E testing scenarios where security parameters need to be exposed as inputs. The empty class body is the expected pattern when using decorator-based configuration.apps/e2e/demo-e2e-cache/e2e/cache.e2e.test.ts (1)
168-173: Good type narrowing and error handling.The type guard properly narrows
message.content.typeto'text'and throws a descriptive error for unexpected types. This follows the guideline to use proper error handling instead of silent failures.apps/e2e/demo-e2e-transparent/src/main.ts (4)
1-4: AI summary inconsistency detected.The AI-generated summary mentions
ComputeAppand a default port of3016, but the actual code importsTasksAppand uses default port3011. Please verify this is the intended configuration for the transparent auth demo.
6-10: LGTM!Good documentation explaining the IdP provider URL configuration and appropriate use of environment variables with sensible defaults for e2e testing.
37-37: LGTM!Empty class as a bootstrap target for the decorator is an appropriate pattern for this framework.
17-35: The transport configuration placement underauthis consistent with the established pattern across all e2e demos in the codebase. This is the correct structure.apps/e2e/demo-e2e-redis/src/apps/sessions/resources/session-current.resource.ts (4)
1-3: LGTM!The imports are appropriate and necessary for the resource implementation.
5-9: Well-structured output schema.The zod schema provides clear type safety for the resource output. The use of
z.record(z.string(), z.string())indicates all session data values are expected to be strings.
11-16: LGTM!The resource decorator is properly configured with appropriate URI, name, description, and MIME type.
17-20: LGTM!The class declaration follows TypeScript best practices with properly constrained generic type parameters. The use of
Record<string, never>for input clearly indicates no input is expected, and the output type is safely inferred from the zod schema.apps/e2e/demo-e2e-ui/e2e/ui.e2e.test.ts (4)
1-17: LGTM!The test setup and imports are correctly configured for e2e testing.
46-67: Good edge case and security coverage.The empty rows test and XSS escaping test are excellent additions that follow security best practices for user-provided content.
Based on learnings, HTML escaping tests are important for preventing XSS attacks.
186-202: Type narrowing fix applied correctly.The explicit type assertion at line 193 ensures the test fails if content type is not
'text', addressing the concern from the previous review about potentially skipped assertions.
219-239: LGTM!Comprehensive tool discovery verification covering all 8 UI tools across the 4 types.
apps/e2e/demo-e2e-ui/src/apps/widgets/resources/ui-templates.resource.ts (2)
1-13: LGTM!Schema definition is well-structured with appropriate zod validation for the UI templates resource.
15-21: LGTM!Resource decorator and class definition follow the established patterns in the codebase and correctly indicate no input parameters.
libs/testing/src/auth/mock-api-server.ts (4)
1-29: Well-documented module with clear examples.The file header and usage example are comprehensive and follow good documentation practices.
37-73: Type definitions are well-structured.The interfaces provide clear, typed contracts for the mock server configuration and responses. Using
unknownforbodyinMockResponseis appropriate per coding guidelines.
185-232: Request handler is well-implemented with proper error handling.The handler correctly:
- Sets CORS headers for cross-origin testing
- Handles OPTIONS preflight requests
- Serves OpenAPI spec at expected endpoints
- Returns appropriate error responses (404, 500)
- Catches and logs errors
234-255: Route matching logic is functional but consider edge cases.The path parameter matching works for simple cases like
/products/:id, but note that it doesn't extract parameter values or handle optional segments. For a mock server used in testing, this is acceptable.libs/testing/src/index.ts (1)
82-85: Clean barrel exports for new mock servers.The exports follow the established pattern in this file: class exports followed by type-only exports. This aligns with the coding guideline for barrel exports.
apps/e2e/demo-e2e-openapi/e2e/openapi.e2e.test.ts (1)
77-136: Test setup and teardown are properly implemented.The
beforeAllcorrectly starts the mock API server before the MCP server (dependency order) andafterAllproperly cleans up both servers. The 60000ms timeout is appropriate for server startup.apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts (3)
62-100: Unauthorized access tests are well-structured.These tests properly verify 401 responses for missing and invalid tokens at the HTTP level. Using HTTP status codes (rather than parsed error objects) is appropriate for e2e tests that validate the transport layer.
102-174: Token validation tests provide good coverage.The tests cover:
- Valid token acceptance (lines 103-131)
- Expired token rejection (lines 133-152)
- Invalid signature rejection (lines 154-173)
This addresses the core authentication scenarios.
194-266: Skipped tests are properly documented.The
describe.skipblocks include clear documentation about why they're skipped (SDK session handling issue) and have a TODO for re-enabling. This is good practice for temporarily disabled tests.libs/testing/src/auth/mock-oauth-server.ts (2)
1-29: Well-documented module with clear usage example.The documentation clearly explains the purpose and provides a practical example.
244-271: Token endpoint handles only anonymous grant type.The implementation is appropriate for testing purposes. The error response for unsupported grant types follows OAuth 2.0 error format correctly.
…onger session IDs
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
libs/sdk/src/flows/flow.instance.ts (1)
113-118: Good addition for error visibility.The structured error logging improves debugging of unhandled errors while safely avoiding Node.js util.inspect issues with Zod errors.
Consider including the stack trace for more comprehensive debugging context:
- console.error('[FlowInstance] Unhandled error:', { - name: e instanceof Error ? e.name : 'UnknownError', - message: e instanceof Error ? e.message : String(e), - }); + console.error('[FlowInstance] Unhandled error:', { + name: e instanceof Error ? e.name : 'UnknownError', + message: e instanceof Error ? e.message : String(e), + stack: e instanceof Error ? e.stack : undefined, + });apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts (3)
108-180: Consider additional test cases for comprehensive coverage.While the current token validation tests cover the core scenarios well, consider adding tests for edge cases to achieve the 95%+ coverage guideline:
- Malformed JWT structure (not three dot-separated segments)
- Token with missing required claims (sub, iat, exp)
- Token with invalid audience claim
- Token with future
iat(issued in the future)- Token with correct signature but from different issuer
As per coding guidelines, achieve and maintain 95%+ test coverage across all metrics.
237-237: Reconsider the sessionId fallback strategy.Using
sessionId ?? ''provides an empty string fallback whensessionIdis null or undefined. While this avoids non-null assertions (which is good per coding guidelines), sending an empty header value might not be the intended behavior if sessionId is actually missing.Consider one of these approaches:
Option 1: Assert sessionId exists before proceeding:
const sessionId = initResponse.headers.get('mcp-session-id'); expect(sessionId).toBeDefined(); expect(sessionId).not.toBe(''); // Later use: 'mcp-session-id': sessionId,Option 2: Fail fast if sessionId is missing:
const sessionId = initResponse.headers.get('mcp-session-id'); if (!sessionId) { throw new Error('Expected mcp-session-id header in init response'); } // Later use: 'mcp-session-id': sessionId,As per learnings, avoid non-null assertions but ensure proper error handling.
Also applies to: 252-252, 309-309, 324-324, 383-383, 398-398, 450-450, 465-465
17-480: Consider additional test scenarios for comprehensive auth coverage.While the current test suite covers the core authentication flows well, consider adding tests for these scenarios to approach the 95% coverage guideline and ensure robust auth behavior:
- Scope-based authorization: Test that users with insufficient scopes cannot access certain operations
- Concurrent sessions: Verify that multiple simultaneous authenticated sessions work correctly
- Session lifecycle: Test session timeout or invalidation scenarios
- Malformed requests: Test authentication with malformed JSON-RPC requests
- Token with extra/unknown claims: Ensure server handles non-standard claims gracefully
Example test structure for scope-based authorization:
it('should reject tool execution with insufficient scopes', async () => { const token = await tokenFactory.createTestToken({ sub: 'user-readonly', scopes: ['read'], // No 'write' scope }); const sessionId = await initializeAuthenticatedSession(server.info.baseUrl, token); const callResponse = await fetch(`${server.info.baseUrl}/`, { method: 'POST', headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}`, 'mcp-session-id': sessionId, }, body: JSON.stringify({ jsonrpc: '2.0', id: 2, method: 'tools/call', params: { name: 'create-task', arguments: { title: 'Test' } }, }), }); expect(callResponse.status).toBe(403); });As per coding guidelines, achieve and maintain 95%+ test coverage.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts(1 hunks)libs/sdk/src/common/schemas/session-header.schema.ts(1 hunks)libs/sdk/src/context/__tests__/session-key.test.ts(3 hunks)libs/sdk/src/context/frontmcp-context.ts(1 hunks)libs/sdk/src/context/session-key.provider.ts(1 hunks)libs/sdk/src/flows/flow.instance.ts(1 hunks)libs/sdk/src/transport/flows/handle.streamable-http.flow.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/sdk/src/transport/flows/handle.streamable-http.flow.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/context/session-key.provider.tslibs/sdk/src/common/schemas/session-header.schema.tslibs/sdk/src/flows/flow.instance.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tslibs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/context/frontmcp-context.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/context/session-key.provider.tslibs/sdk/src/common/schemas/session-header.schema.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/context/frontmcp-context.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/context/session-key.provider.tslibs/sdk/src/common/schemas/session-header.schema.tslibs/sdk/src/flows/flow.instance.tslibs/sdk/src/context/__tests__/session-key.test.tslibs/sdk/src/context/frontmcp-context.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.tslibs/sdk/src/context/__tests__/session-key.test.ts
🧠 Learnings (11)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use FrontMCP's TypeScript-first schema validation framework philosophy - all types should align with MCP protocol definitions
Applied to files:
libs/sdk/src/common/schemas/session-header.schema.tsapps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
Applied to files:
libs/sdk/src/common/schemas/session-header.schema.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Don't mutate rawInput in flows - use state.set() for managing flow state instead
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Applied to files:
libs/sdk/src/flows/flow.instance.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Use specific error classes with MCP error codes instead of generic errors
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead of `unknown` for `execute()` and `read()` methods
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.test.ts : Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Applied to files:
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts
🧬 Code graph analysis (1)
libs/sdk/src/context/__tests__/session-key.test.ts (1)
libs/sdk/src/context/session-key.provider.ts (1)
SessionKey(27-64)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
libs/sdk/src/context/session-key.provider.ts (1)
29-29: LGTM! Constant update aligns with encrypted session ID requirements.The increase from 256 to 2048 characters accommodates encrypted session IDs (AES-256-GCM base64url format) which can reach 460-700+ characters. The change is consistent across the codebase and properly tested.
libs/sdk/src/context/frontmcp-context.ts (1)
99-99: LGTM! Internal validation constant consistent with SessionKey.MAX_LENGTH.The local
SESSION_KEY_MAX_LENGTHconstant is updated to match theSessionKey.MAX_LENGTHchange, maintaining consistency across validation boundaries.libs/sdk/src/common/schemas/session-header.schema.ts (1)
7-8: LGTM! Schema validation updated with clear rationale.The Zod schema's maximum length constraint is updated to 2048, with documentation explaining the need for encrypted session IDs. The validation logic correctly uses Zod 4 syntax and maintains strict character constraints (printable ASCII only).
Also applies to: 15-15
libs/sdk/src/context/__tests__/session-key.test.ts (1)
22-27: LGTM! Comprehensive test coverage for new boundary.All test cases are properly updated to validate the new 2048-character maximum:
- Boundary tests cover exactly 2048 (valid) and 2049 (invalid)
- Error message assertions reflect the new limit
- Static constant assertion confirms
MAX_LENGTH === 2048The tests maintain thorough coverage of edge cases and validation scenarios.
Also applies to: 29-32, 96-99, 111-112
apps/e2e/demo-e2e-transparent/e2e/transparent-auth.e2e.test.ts (3)
68-106: LGTM! Appropriate HTTP-level validation.The unauthorized access tests correctly validate HTTP status codes and headers for authentication failures. Using raw
fetch()with status code assertions is the appropriate approach for these HTTP transport-level tests.
182-198: LGTM! Pragmatic handling of endpoint variations.The test appropriately handles multiple valid responses for the well-known endpoint, as the behavior may vary based on server configuration. The conditional metadata validation for 200 responses is a sensible approach.
18-21: Add explicit type annotations for strict TypeScript.The test variables lack explicit type annotations. While TypeScript infers the types, explicit annotations improve type safety and align with strict TypeScript mode guidelines.
Apply this diff to add explicit type annotations:
- let mockOAuth: MockOAuthServer; - let tokenFactory: TestTokenFactory; - let server: TestServer; + let mockOAuth!: MockOAuthServer; + let tokenFactory!: TestTokenFactory; + let server!: TestServer;Note: The definite assignment assertions (
!) are appropriate here becausebeforeAllguarantees initialization before tests run.As per coding guidelines, enable strict TypeScript mode with explicit types.
⛔ Skipped due to learnings
Learnt from: CR Repo: agentfront/frontmcp PR: 0 File: libs/ui/CLAUDE.md:0-0 Timestamp: 2025-11-26T15:23:04.965Z Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicableLearnt from: CR Repo: agentfront/frontmcp PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-01T00:33:33.644Z Learning: Applies to **/*.test.ts : Test all code paths including errors, constructor validation, and error class `instanceof` checks
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts (1)
52-60: Consider adding a positive boundary test for the maximum valid length.The test correctly validates that 2049 characters is rejected. However, comprehensive boundary testing should also verify that exactly 2048 characters (the maximum valid length) is accepted.
Consider adding a test case like this after line 60:
it('should accept sessionKey at maximum length', async () => { const registry = new ProviderRegistry([createValueProvider(TEST_TOKEN, { name: 'test' })]); await registry.ready; const maxLengthKey = 'a'.repeat(2048); await expect(registry.buildViews(maxLengthKey)).resolves.toBeDefined(); const views = await registry.buildViews(maxLengthKey); const sessionKey = views.context.get(SessionKey) as SessionKey; expect(sessionKey.value).toBe(maxLengthKey); });.github/workflows/push.yml (3)
19-21: Consider trimming whitespace from .nvmrc content.If
.nvmrccontains a trailing newline or whitespace, it could cause issues withactions/setup-node. Consider usingtrto strip whitespace:- name: Get Node version id: node-version - run: echo "version=$(cat .nvmrc)" >> $GITHUB_OUTPUT + run: echo "version=$(cat .nvmrc | tr -d '[:space:]')" >> $GITHUB_OUTPUT
147-152: Consider error handling strategy for sequential E2E tests.With multiple Jest commands in sequence, if the first test suite fails, subsequent tests won't run. This could hide failures in other test suites.
Options to consider:
- Use
|| trueto continue on failure but track exit codes- Run with a wrapper script that captures all results
- Accept current behavior if early exit on failure is desired
If you want all tests to run regardless of individual failures:
- name: Run E2E tests (batch 1) run: | + set +e + exit_code=0 - npx jest apps/e2e/demo-e2e-public --runInBand --no-cache - npx jest apps/e2e/demo-e2e-transparent --runInBand --no-cache - npx jest apps/e2e/demo-e2e-openapi --runInBand --no-cache - npx jest apps/e2e/demo-e2e-codecall --runInBand --no-cache + npx jest apps/e2e/demo-e2e-public --runInBand --no-cache || exit_code=1 + npx jest apps/e2e/demo-e2e-transparent --runInBand --no-cache || exit_code=1 + npx jest apps/e2e/demo-e2e-openapi --runInBand --no-cache || exit_code=1 + npx jest apps/e2e/demo-e2e-codecall --runInBand --no-cache || exit_code=1 + exit $exit_code
121-217: Consider using Nx affected for E2E tests.The E2E batch jobs bypass Nx and run Jest directly on all test suites, meaning E2E tests always run regardless of what changed. This differs from the unit tests approach which uses
nx affected.If this is intentional (E2E tests should always run on push), this is fine. However, for efficiency on large PRs, consider:
run: npx nx affected -t e2e --projects='demo-e2e-public,demo-e2e-transparent,...'Based on learnings, the project uses Nx build system for all build commands.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/push.yml(2 hunks)libs/sdk/src/provider/__tests__/provider-session-scope.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts: Achieve and maintain 95%+ test coverage across all metrics (statements, branches, functions, lines)
Test all code paths including errors, constructor validation, and error classinstanceofchecks
Files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
libs/{sdk,adapters,plugins,cli}/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
libs/{sdk,adapters,plugins,cli}/src/**/*.ts: Return strictly typed MCP protocol responses (GetPromptResult, ReadResourceResult, etc.) instead ofunknownforexecute()andread()methods
Validate URIs per RFC 3986 at metadata level using Zod validation with custom refinements
UsegetCapabilities()for dynamic capability exposure instead of hardcoding capabilities in adapters
UsechangeScopeinstead ofscopefor change event properties to avoid confusion with the Scope class
Validate hooks match their entry type and fail fast with InvalidHookFlowError for unsupported flows
Don't mutate rawInput in flows - use state.set() for managing flow state instead
Files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
libs/**
⚙️ CodeRabbit configuration file
libs/**: Contains publishable SDK libraries. Review for API correctness, breaking changes, and consistency with docs. When public APIs change, ensure there is a matching docs/draft/docs/** update (not direct edits under docs/docs/**).
Files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
🧠 Learnings (3)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
libs/sdk/src/provider/__tests__/provider-session-scope.test.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
.github/workflows/push.yml
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Ensure all TypeScript builds complete without warnings and all tests pass with 100% pass rate before committing code
Applied to files:
.github/workflows/push.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
.github/workflows/push.yml (4)
5-7: LGTM!Good use of concurrency control to cancel in-progress workflows when new commits are pushed to the same branch.
23-51: LGTM!Good approach removing
continue-on-errorto make lint/prettier failures explicit. The job structure is clean and the dependency on the shared setup job for node version is well implemented.
89-119: LGTM!Good separation of concerns by excluding E2E tests from unit test runs. The
--passWithNoTestsflag is appropriate for affected-based test runs where no tests may be impacted.
80-87: The download-artifact@v4 action preserves directory structure by default—the concern about flattening is not applicable here. The uploaded paths (libs/*/distanddist/) are extracted to the same structure in downstream jobs, where tests locate them via Nx build resolution or explicit webpack output paths. No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
.github/workflows/push.yml (2)
147-152: Consider removing--forceExitflag to detect async cleanup issues.The
--forceExitflag forces Jest to exit immediately after tests complete, which can mask issues with async operations that haven't properly cleaned up (e.g., open database connections, timers, file handles). While convenient for CI, it prevents detecting resource leaks that could become problems in production.If the flag is necessary due to hanging operations in the current test suite, consider tracking and fixing the underlying issues causing tests to hang rather than masking them with
--forceExit.Also applies to: 179-185, 211-217
147-152: Add error handling to prevent silent test skipping.Multi-command run blocks fail-fast by default—if the first Jest command fails, subsequent commands won't execute, leading to incomplete test coverage reports. Consider adding error handling or using a test framework that aggregates results.
Option 1: Continue on error and capture exit codes:
- name: Run E2E tests (batch 1) run: | + set +e + EXIT_CODE=0 npx jest apps/e2e/demo-e2e-public --runInBand --no-cache --forceExit + EXIT_CODE=$((EXIT_CODE + $?)) npx jest apps/e2e/demo-e2e-transparent --runInBand --no-cache --forceExit + EXIT_CODE=$((EXIT_CODE + $?)) npx jest apps/e2e/demo-e2e-openapi --runInBand --no-cache --forceExit + EXIT_CODE=$((EXIT_CODE + $?)) npx jest apps/e2e/demo-e2e-codecall --runInBand --no-cache --forceExit + EXIT_CODE=$((EXIT_CODE + $?)) + exit $EXIT_CODEOption 2: If switching to Nx (as suggested in the previous comment), Nx handles this automatically with
--continue.Also applies to: 179-185, 211-217
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/push.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
.github/workflows/push.yml
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Ensure all TypeScript builds complete without warnings and all tests pass with 100% pass rate before committing code
Applied to files:
.github/workflows/push.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (3)
.github/workflows/push.yml (3)
5-7: LGTM! Improved concurrency configuration.The concurrency group rename and
cancel-in-progress: truesetting will prevent resource waste by canceling outdated workflow runs when new commits are pushed.
10-21: LGTM! Excellent DRY refactor.Centralizing the Node version extraction in a shared setup job eliminates duplication and ensures all jobs use the same Node version from
.nvmrc.
89-120: LGTM! Proper artifact reuse and Nx integration.The unit-tests job correctly downloads build artifacts, sets Nx SHAs, and uses Nx's test orchestration with appropriate exclusions for e2e tests.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
.github/workflows/push.yml (3)
36-36: Upgrade actions/setup-node from v4 to v6.All jobs still use
actions/setup-node@v4. The latest stable version is v6, which includes security improvements and new features.Also applies to: 68-68, 104-104, 136-136, 168-168, 200-200
123-155: Add missing "Set Nx SHAs" step to E2E batch jobs.All three E2E batch jobs are missing the "Set Nx SHAs" step that's present in lint, build, and unit-tests jobs. This step is necessary for Nx's affected command logic and caching to work correctly.
Also applies to: 156-187, 188-219
149-154: Use Nx test orchestration instead of direct Jest invocation.The E2E tests bypass Nx by calling
npx jestdirectly, contradicting the project's standard practice and the learning that states "Use the Nx build system for all build commands."Based on learnings, using Nx ensures proper caching, dependency tracking, and consistency with other CI jobs.
Also applies to: 181-186, 213-219
🧹 Nitpick comments (2)
apps/e2e/demo-e2e-codecall/jest.e2e.config.ts (1)
1-34: LGTM! Well-configured for E2E testing.The configuration appropriately addresses E2E test requirements:
forceExit: trueandmaxWorkers: 1prevent race conditions and handle cleanup for tests with external dependencies- 60-second timeout accommodates potentially slow E2E operations
- Module name mapper enables testing against source files
- Preset pattern usage aligns with the hierarchical configuration approach
Optional: Add explicit TypeScript type annotation.
For improved type safety, consider importing and using Jest's
Configtype:+import type { Config } from '@jest/types'; + -export default { +const config: Config.InitialOptions = { displayName: 'demo-e2e-codecall', preset: '../../../jest.preset.js', // ... rest of config -}; +}; + +export default config;However, this is purely a nice-to-have enhancement, as Jest config files are commonly written without explicit typing.
apps/e2e/demo-e2e-ui/jest.e2e.config.ts (1)
1-34: Consider adding explicit type annotation for type safety.The exported configuration object lacks an explicit type annotation. Adding a type from
@jest/typeswould provide better compile-time validation.Apply this diff to add type safety:
+import type { Config } from '@jest/types'; + -export default { +const config: Config.InitialOptions = { displayName: 'demo-e2e-ui', preset: '../../../jest.preset.js', testEnvironment: 'node', testMatch: ['<rootDir>/e2e/**/*.e2e.test.ts'], testTimeout: 60000, forceExit: true, maxWorkers: 1, setupFilesAfterEnv: ['<rootDir>/../../../libs/testing/src/setup.ts'], transformIgnorePatterns: ['node_modules/(?!(jose)/)'], transform: { '^.+\\.[tj]s$': [ '@swc/jest', { jsc: { parser: { syntax: 'typescript', decorators: true, }, transform: { decoratorMetadata: true, }, target: 'es2022', }, }, ], }, moduleNameMapper: { '^@frontmcp/testing$': '<rootDir>/../../../libs/testing/src/index.ts', '^@frontmcp/sdk$': '<rootDir>/../../../libs/sdk/src/index.ts', '^@frontmcp/adapters$': '<rootDir>/../../../libs/adapters/src/index.ts', '^@frontmcp/plugins$': '<rootDir>/../../../libs/plugins/src/index.ts', }, }; + +export default config;As per coding guidelines.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.github/workflows/push.yml(2 hunks)apps/e2e/demo-e2e-cache/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-codecall/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-errors/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-hooks/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-multiapp/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-notifications/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-openapi/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-providers/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-public/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-redis/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-serverless/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-transparent/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-ui/jest.e2e.config.ts(1 hunks)libs/testing/src/auth/mock-oauth-server.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/e2e/demo-e2e-errors/jest.e2e.config.ts
- apps/e2e/demo-e2e-cache/jest.e2e.config.ts
- apps/e2e/demo-e2e-notifications/jest.e2e.config.ts
- apps/e2e/demo-e2e-providers/jest.e2e.config.ts
- apps/e2e/demo-e2e-redis/jest.e2e.config.ts
- apps/e2e/demo-e2e-public/jest.e2e.config.ts
- apps/e2e/demo-e2e-multiapp/jest.e2e.config.ts
- apps/e2e/demo-e2e-serverless/jest.e2e.config.ts
- apps/e2e/demo-e2e-openapi/jest.e2e.config.ts
- apps/e2e/demo-e2e-hooks/jest.e2e.config.ts
- apps/e2e/demo-e2e-transparent/jest.e2e.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
libs/testing/src/auth/mock-oauth-server.tsapps/e2e/demo-e2e-ui/jest.e2e.config.tsapps/e2e/demo-e2e-codecall/jest.e2e.config.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/testing/src/auth/mock-oauth-server.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
.github/workflows/push.yml
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
.github/workflows/push.ymlapps/e2e/demo-e2e-ui/jest.e2e.config.tsapps/e2e/demo-e2e-codecall/jest.e2e.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Ensure all TypeScript builds complete without warnings and all tests pass with 100% pass rate before committing code
Applied to files:
.github/workflows/push.yml
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Follow the preset pattern for hierarchical configurations across the codebase
Applied to files:
apps/e2e/demo-e2e-ui/jest.e2e.config.tsapps/e2e/demo-e2e-codecall/jest.e2e.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
apps/e2e/demo-e2e-ui/jest.e2e.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-ui/jest.e2e.config.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: lint
🔇 Additional comments (11)
.github/workflows/push.yml (5)
5-7: LGTM: Improved concurrency configuration.The concurrency group rename and enabling
cancel-in-progressare appropriate for CI workflows, preventing redundant builds when new commits are pushed.
10-21: LGTM: Well-structured shared setup job.The shared setup job properly extracts the Node version from
.nvmrcand makes it available to downstream jobs, reducing duplication.
47-53: Verify continue-on-error for lint and format steps.Both linting and formatting steps use
continue-on-error: true, which prevents failures in these steps from blocking the workflow. This contradicts the learning that "all TypeScript builds complete without warnings" before committing. Verify whether allowing lint/format failures to pass is intentional.
82-89: LGTM: Appropriate artifact handling.The artifact upload configuration correctly captures build outputs with a reasonable 1-day retention period for CI.
91-121: LGTM: Well-structured unit test job.The unit test job correctly depends on setup and build, downloads artifacts, sets Nx SHAs, and uses Nx for test orchestration. The exclusion of E2E tests is appropriate since they run in separate batch jobs.
libs/testing/src/auth/mock-oauth-server.ts (6)
1-33: LGTM!Clear module documentation with a practical usage example that demonstrates the typical workflow: creating a token factory, starting the server, using the server URL for configuration, creating tokens, and cleanup.
38-56: LGTM!Interfaces are well-documented with JSDoc comments and follow proper typing conventions.
68-78: LGTM!Clean class structure with proper null initialization for mutable state. The connection tracking Set enables clean server shutdown.
155-170: LGTM!Proper error handling in the
infogetter when the server isn't running, and clean accessor for the token factory.
176-255: LGTM!Request handling is well-structured with proper CORS support, routing, and error handling. The
*origin is appropriate for a test mock server.
257-299: LGTM!Token endpoint follows OAuth 2.0 response format with proper error handling for unsupported grant types. The
readBodyutility is appropriate for a controlled test environment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/push.yml (1)
47-53: Consider making lint and format checks blocking.Both the linter and prettier checks have
continue-on-error: true, which means failures won't block the pipeline. If code quality enforcement is desired, consider removing these flags to make the checks blocking.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/push.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
.github/workflows/push.yml
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
.github/workflows/push.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
.github/workflows/push.yml (5)
6-7: LGTM!The concurrency configuration is appropriate for CI workflows. Using
ci-${{ github.ref }}withcancel-in-progress: trueprevents redundant pipeline runs when pushing multiple commits in quick succession.
10-21: LGTM!The shared setup job is a good pattern for centralizing configuration. Reading the Node version from
.nvmrcand exposing it via job outputs eliminates duplication across downstream jobs.
55-89: LGTM!The build job is well-structured with proper artifact handling. Uploading build artifacts with 1-day retention enables downstream jobs to reuse build outputs without rebuilding, improving pipeline efficiency.
91-121: LGTM!The unit-tests job properly depends on build artifacts and uses Nx affected to run only impacted tests. The
--exclude='demo-e2e-*'pattern correctly separates unit tests from E2E tests for parallel execution.
123-154: E2E batch jest configuration is properly configured.The E2E tests are correctly set up with Jest's
forceExitoption already configured in each project'sjest.e2e.config.tsfile (e.g.,apps/e2e/demo-e2e-public/jest.e2e.config.ts). This ensures Jest will exit cleanly after tests complete, preventing CI hangs from lingering resources.The workflow correctly:
- ✅ Uses Nx orchestration (
nx run-many -t test)- ✅ Includes the "Set Nx SHAs" step for proper caching
- ✅ Has
forceExit: trueconfigured in jest.e2e.config.ts for all E2E projectsNo CLI overrides needed—the Jest executor respects the forceExit setting from the referenced jest config file.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/e2e/demo-e2e-ui/jest.e2e.config.ts (2)
9-9:forceExitcan mask hanging processes and resource leaks.This issue has already been flagged in previous review comments. Setting
forceExit: trueforces Jest to terminate without waiting for open handles to close, which can hide issues like unclosed database connections, pending timers, or incomplete cleanup.
12-12: Remove unnecessary capture group fromtransformIgnorePatternsregex.This issue has already been flagged in previous review comments. The pattern
'node_modules/(?!(jose)/)'includes an unnecessary capture group. For a single package, the pattern should be'node_modules/(?!jose/)'.
🧹 Nitpick comments (2)
libs/testing/src/auth/mock-oauth-server.ts (1)
73-73: Consider importing Socket type at the top.The inline
import('net').Sockettype reference works but could be cleaner with a top-level import for better readability.Apply this diff to add a top-level import:
-import { createServer, Server, IncomingMessage, ServerResponse } from 'http'; +import { createServer, Server, IncomingMessage, ServerResponse } from 'http'; +import type { Socket } from 'net'; import type { TestTokenFactory } from './token-factory';Then update line 73:
- private connections: Set<import('net').Socket> = new Set(); + private connections: Set<Socket> = new Set();apps/e2e/demo-e2e-cache/jest.e2e.config.ts (1)
9-9: TheforceExitflag is unnecessary—the testing framework already implements proper cleanup.The E2E tests use the
@frontmcp/testingfixture framework, which automatically manages resource cleanup: theforceExitfeature is an escape-hatch, and it is advised to tear down external resources after each test to make sure Jest can shut down cleanly. The testing framework satisfies this requirement through automatic lifecycle management:
- Per-test cleanup: Client disconnect after each test (
cleanupTestFixtures())- File-level cleanup: Server shutdown via
afterAll()hook (cleanupSharedResources())These cleanup operations run properly, so
forceExit: trueis not required. Consider removing the flag to allow Jest to exit cleanly once the framework's automatic cleanup completes, which also enables--detectOpenHandlesfor debugging if needed.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.github/workflows/push.yml(3 hunks)apps/e2e/demo-e2e-cache/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-codecall/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-errors/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-hooks/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-multiapp/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-notifications/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-openapi/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-providers/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-public/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-redis/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-transparent/jest.e2e.config.ts(1 hunks)apps/e2e/demo-e2e-ui/jest.e2e.config.ts(1 hunks)libs/testing/src/auth/mock-oauth-server.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/e2e/demo-e2e-transparent/jest.e2e.config.ts
- apps/e2e/demo-e2e-redis/jest.e2e.config.ts
- apps/e2e/demo-e2e-openapi/jest.e2e.config.ts
- apps/e2e/demo-e2e-errors/jest.e2e.config.ts
- apps/e2e/demo-e2e-hooks/jest.e2e.config.ts
- apps/e2e/demo-e2e-providers/jest.e2e.config.ts
- apps/e2e/demo-e2e-multiapp/jest.e2e.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Enable 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 specific error classes with MCP error codes instead of generic errors
Use type parameters with constraints instead of unconstrained generics, and preferunknownoveranyfor generic type defaults
Follow the preset pattern for hierarchical configurations across the codebase
Files:
apps/e2e/demo-e2e-ui/jest.e2e.config.tsapps/e2e/demo-e2e-codecall/jest.e2e.config.tsapps/e2e/demo-e2e-cache/jest.e2e.config.tsapps/e2e/demo-e2e-notifications/jest.e2e.config.tslibs/testing/src/auth/mock-oauth-server.tsapps/e2e/demo-e2e-public/jest.e2e.config.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/testing/src/auth/mock-oauth-server.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/**/*.test.ts : Test behavior across platform configurations (OpenAI, Claude, Gemini, ngrok) where applicable
Applied to files:
apps/e2e/demo-e2e-ui/jest.e2e.config.ts.github/workflows/push.ymlapps/e2e/demo-e2e-codecall/jest.e2e.config.tsapps/e2e/demo-e2e-cache/jest.e2e.config.tsapps/e2e/demo-e2e-notifications/jest.e2e.config.tsapps/e2e/demo-e2e-public/jest.e2e.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Follow the preset pattern for hierarchical configurations across the codebase
Applied to files:
apps/e2e/demo-e2e-ui/jest.e2e.config.tsapps/e2e/demo-e2e-codecall/jest.e2e.config.tsapps/e2e/demo-e2e-cache/jest.e2e.config.tsapps/e2e/demo-e2e-notifications/jest.e2e.config.tsapps/e2e/demo-e2e-public/jest.e2e.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to libs/{sdk,adapters,plugins,cli}/src/**/*.ts : Use `changeScope` instead of `scope` for change event properties to avoid confusion with the Scope class
Applied to files:
apps/e2e/demo-e2e-ui/jest.e2e.config.ts
📚 Learning: 2025-11-26T15:23:04.965Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-11-26T15:23:04.965Z
Learning: Applies to libs/ui/src/components/**/*.test.ts : Write validation tests covering invalid variant/options, unknown properties, and valid option acceptance
Applied to files:
apps/e2e/demo-e2e-ui/jest.e2e.config.ts
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Use the Nx build system for all build commands (nx build sdk, nx test ast-guard, nx run-many -t test)
Applied to files:
.github/workflows/push.yml
📚 Learning: 2025-12-01T00:33:33.644Z
Learnt from: CR
Repo: agentfront/frontmcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T00:33:33.644Z
Learning: Applies to **/*.ts : Avoid non-null assertions (`!`) - use proper error handling and throw specific errors instead
Applied to files:
libs/testing/src/auth/mock-oauth-server.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (15)
apps/e2e/demo-e2e-notifications/jest.e2e.config.ts (1)
1-38: LGTM! Configuration follows the established E2E pattern.The Jest E2E configuration is well-structured and consistent with other E2E configs in this PR. The previous setup file path issue has been resolved, proper TypeScript types are used, and all settings are appropriate for E2E testing (60-second timeout, sequential execution, decorator support).
apps/e2e/demo-e2e-codecall/jest.e2e.config.ts (1)
1-38: LGTM! Well-structured E2E configuration.The Jest configuration is properly set up and consistent with other e2e test configurations in the repository. Key aspects:
- Follows the preset pattern as required by coding guidelines
- Appropriate settings for E2E tests (60s timeout, serial execution with
maxWorkers: 1)- Correct TypeScript transform configuration with decorator support
- Proper module path mappings for internal packages
Note:
forceExit: trueis standard for E2E tests but be aware it can mask resource leaks (unclosed handles, pending timers). This is typically acceptable for tests interacting with external systems.apps/e2e/demo-e2e-ui/jest.e2e.config.ts (3)
1-1: LGTM!Correctly uses type-only import for the Jest configuration type.
3-11: Configuration follows standard e2e test patterns.The configuration appropriately sets up the e2e environment with:
- Preset pattern following hierarchical configuration guidelines
- Single worker (
maxWorkers: 1) to prevent race conditions in e2e tests- Reasonable 60-second timeout for e2e operations
- Proper SWC transformer setup with TypeScript decorators support
- Correct module name mappings to source files
Also applies to: 13-36
38-38: LGTM!Standard default export pattern for Jest configuration.
apps/e2e/demo-e2e-public/jest.e2e.config.ts (1)
1-38: LGTM! Well-structured Jest e2e configuration.The configuration follows the standard pattern for e2e tests in this codebase with appropriate settings:
- Serial execution (
maxWorkers: 1) prevents race conditions in e2e tests- 60-second timeout is reasonable for end-to-end scenarios
- SWC transformation with decorator support aligns with the codebase's use of decorators
- Module name mappings correctly reference internal libraries
- The
joselibrary is properly included intransformIgnorePatternsfor ESM transformationlibs/testing/src/auth/mock-oauth-server.ts (2)
83-126: Past review comment addressed - LGTM!The non-null assertion concern from the previous review has been properly addressed. The server reference is now captured in a local variable before the async boundary, eliminating the need for
!assertions.
131-155: Past review comment addressed - LGTM!The non-null assertion concern from the previous review has been properly addressed. The server reference is captured locally before the async boundary, avoiding the need for
!assertions.apps/e2e/demo-e2e-cache/jest.e2e.config.ts (3)
1-3: LGTM: Proper type annotations.The type-only import and explicit
Config.InitialOptionstype annotation follow TypeScript best practices with noanytypes.
4-7: LGTM: Configuration follows preset pattern.The configuration correctly uses the hierarchical preset pattern and appropriate E2E-specific settings for test environment and file matching.
12-35: LGTM: Transform and module mappings are well-configured.The SWC transform configuration properly enables decorator support for the cache app, and the module name mapper correctly resolves internal monorepo packages.
.github/workflows/push.yml (4)
10-21: LGTM! Clean centralized setup.The shared setup job properly centralizes Node version management by reading from
.nvmrcand exposing it as an output for downstream jobs. This ensures consistency across all workflow jobs.
24-38: LGTM! Node setup properly integrated.The lint job correctly depends on the setup job and uses
actions/setup-node@v6with the shared node-version output. The previous comment about upgrading from v4 to v6 has been addressed.
55-89: LGTM! Build job structure is sound.The build job properly uses Nx affected builds and uploads artifacts for downstream consumption. The artifact paths cover typical build outputs, and 1-day retention is appropriate for CI artifacts.
91-121: LGTM! Unit test exclusion pattern is correct.The unit-tests job properly depends on build, downloads artifacts, and excludes E2E tests using the pattern
--exclude='demo-e2e-*', which correctly matches all E2E projects in the batches.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.