feat: add examples directory support and update metadata schema in skill files#323
feat: add examples directory support and update metadata schema in skill files#323frontegg-david merged 4 commits intorelease/1.0.xfrom
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdded examples discovery, parsing, resolution, and consumption to the skills subsystem: new example metadata types and resolvers, examples/ detection during skill scanning, resolved Examples tables appended to SKILL.md output, a ReadSkillContentTool and CLI flags to list/read examples, interface/schema updates to carry resolved example info, updated loaders/instance behavior to resolve examples, tests and extensive catalog example docs, and Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (skills read)
participant Tool as ReadSkillContentTool
participant Reg as SkillRegistry
participant Inst as SkillInstance
participant FS as Filesystem
participant Parser as FrontmatterParser
CLI->>Tool: invoke(skillId, type, name)
Tool->>Reg: loadSkill(skillId)
Reg-->>Tool: SkillRecord / metadata
Tool->>Inst: locate SkillInstance (for baseDir/resources)
Inst-->>Tool: SkillInstance (baseDir, resources)
Tool->>FS: read file (references/ or examples/ path)
FS-->>Tool: file contents
Tool->>Parser: parse frontmatter + body
Parser-->>Tool: { frontmatter, body }
Tool-->>CLI: return content + metadata or available names
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-03-31T03:27:41.989Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/cli/src/commands/skills/read.ts (1)
183-199:⚠️ Potential issue | 🟡 MinorKeep the footer hint consistent with the example count.
The header only prints
Examples: ...whenexampleCount > 0, but the footer always printsfrontmcp skills read ... --examplesfor any skill that has references. That sends users to a guaranteed “No examples found” path for skills that have refs but no examples yet.Suggested fix
+ const exampleCount = entry.references?.reduce((sum, r) => sum + (r.examples?.length ?? 0), 0) ?? 0; + if (entry.references && entry.references.length > 0) { console.log(c('gray', ` References: ${entry.references.length} (use --refs to list)`)); - const exampleCount = entry.references.reduce((sum, r) => sum + (r.examples?.length ?? 0), 0); if (exampleCount > 0) { console.log(c('gray', ` Examples: ${exampleCount} (use --examples to list)`)); } } @@ if (entry.references && entry.references.length > 0) { console.log(c('gray', ` References: frontmcp skills read ${skillName} --refs`)); - console.log(c('gray', ` Examples: frontmcp skills read ${skillName} --examples`)); + if (exampleCount > 0) { + console.log(c('gray', ` Examples: frontmcp skills read ${skillName} --examples`)); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/commands/skills/read.ts` around lines 183 - 199, The footer always prints the examples hint even when no examples exist; change the logic to only print the "Examples: frontmcp skills read ${skillName} --examples" footer when exampleCount > 0 (the same metric computed by exampleCount = entry.references.reduce(...)); use the existing exampleCount variable and/or the same condition that guards the header examples line so the examples footer is emitted only if entry.references has examples.
♻️ Duplicate comments (1)
libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-rate-limiting-and-progress.md (1)
48-51:⚠️ Potential issue | 🔴 CriticalMissing explicit return after
this.fail()(duplicate concern).Similar to the issue in
tool-with-di-and-errors.md, line 50 callsthis.fail()but execution continues to the processing loop. Ifthis.fail()doesn't halt execution by throwing, the tool will proceed to process items even when validation fails.Add an explicit
returnifthis.fail()doesn't throw:🐛 Proposed fix
if (input.items.some((item) => item.trim() === '')) { this.fail(new Error('Items must not be empty strings')); + return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-rate-limiting-and-progress.md` around lines 48 - 51, The validation block calls this.fail(new Error(...)) but does not stop execution, so add an explicit early return (or throw) immediately after the this.fail(...) call to prevent continuing into the processing loop; update the code around this.mark('validation') / if (input.items.some(...)) { this.fail(...) } to include a return (e.g., this.fail(...); return;) so that the rest of the handler that iterates over input.items does not run when validation fails.
🟠 Major comments (35)
libs/skills/catalog/frontmcp-setup/examples/readme-guide/vercel-deployment-readme.md-61-68 (1)
61-68:⚠️ Potential issue | 🟠 MajorFix conflicting environment variable guidance for Vercel KV.
This example says Vercel KV vars are auto-injected, but then asks users to set
REDIS_URL. That conflict can cause incorrect setup instructions for this template.Suggested doc fix
-Set secrets via: `vercel env add REDIS_URL` +Vercel KV credentials (`KV_REST_API_URL`, `KV_REST_API_TOKEN`) are provided automatically when KV is attached to the project.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-setup/examples/readme-guide/vercel-deployment-readme.md` around lines 61 - 68, The README currently mixes Vercel KV (auto-injected vars KV_REST_API_URL and KV_REST_API_TOKEN) with a manual secret command for REDIS_URL; update the docs to remove the conflicting instruction or clarify intent: if the template uses Vercel KV, delete the "Set secrets via: `vercel env add REDIS_URL`" line and ensure the Environment Variables table only lists KV_REST_API_URL and KV_REST_API_TOKEN as auto-injected; if the template actually requires an external Redis instance, replace/instead document adding REDIS_URL with `vercel env add REDIS_URL` and mark KV_REST_API_URL/KV_REST_API_TOKEN as not required—refer to the variable names REDIS_URL, KV_REST_API_URL, and KV_REST_API_TOKEN to locate and update the text consistently.libs/skills/catalog/frontmcp-development/examples/create-skill-with-tools/directory-skill-with-tools.md-35-66 (1)
35-66:⚠️ Potential issue | 🟠 Major
SKILL.mdexample frontmatter is malformed and won’t parse as shown.The snippet is missing the opening
---, andparameters/exampleslist items are not nested correctly under their parent keys. This will break metadata parsing for readers following the sample.Proposed doc fix
```markdown ## <!-- skills/deploy-service/SKILL.md --> - +--- name: deploy-service description: Deploy a service through the full pipeline tags: [deploy, ci-cd, production] tools: - -- name: build_project + - name: build_project purpose: Compile the service required: true -- name: run_tests + - name: run_tests purpose: Execute test suite required: true -- name: deploy_to_env + - name: deploy_to_env purpose: Push build to target environment required: true parameters: -- name: environment - description: Target deployment environment - type: string - required: true - examples: -- scenario: Deploy to staging - expected-outcome: Service deployed and health check passes + - name: environment + description: Target deployment environment + type: string + required: true + examples: + - scenario: Deploy to staging + expected-outcome: Service deployed and health check passes ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-skill-with-tools/directory-skill-with-tools.md` around lines 35 - 66, The SKILL.md example frontmatter is malformed; add the missing opening '---' and fix YAML nesting so lists are indented under their parent keys: ensure 'tools:' contains properly indented items (e.g., '- name: build_project' etc.), and move 'parameters' and 'examples' to be nested under the 'deploy_to_env' tool (e.g., 'parameters:' with an indented '- name: environment' block that includes 'description', 'type', 'required', and an indented 'examples:' list with '- scenario' and 'expected-outcome'); update the SKILL.md frontmatter block accordingly so parsers can read the metadata.libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-binary/binary-build-config.md-57-59 (1)
57-59:⚠️ Potential issue | 🟠 MajorMove shebang to the first line of the CLI file.
The shebang
#!/usr/bin/env nodemust be the very first line. When preceded by comments, Unix systems fail to recognize the shebang directive, rendering the script non-executable.Suggested doc fix
-// src/cli.ts -#!/usr/bin/env node +#!/usr/bin/env node +// src/cli.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-binary/binary-build-config.md` around lines 57 - 59, Move the shebang so it is the very first line of the CLI file: ensure "#!/usr/bin/env node" appears before any comments or imports in src/cli.ts (i.e., before the import { FrontMcp } from '@frontmcp/sdk' line) so the OS recognizes the script as executable; simply cut the current shebang placement and place it as the file's first line.libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-binary/binary-build-config.md-44-46 (1)
44-46:⚠️ Potential issue | 🟠 MajorUpdate dependency version to match ecosystem and fix shebang placement in code example.
Line 45 uses
zodv3 (^3.23.0), but the entire codebase standardizes on v4 (^4.0.0). This example will cause users to pull an incompatible version.Additionally, line 58 places the shebang after a comment. For an executable script, the shebang must be the first line of the file. Move
#!/usr/bin/env nodeabove the// src/cli.tscomment so the example produces a valid, runnable script.Suggested fix
#!/usr/bin/env node // src/cli.ts import { FrontMcp } from '@frontmcp/sdk'; import { MyApp } from './my.app';And update line 45 to:
"zod": "^4.0.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-binary/binary-build-config.md` around lines 44 - 46, Update the example package JSON to use zod v4 by changing the dependency entry for "zod" to "^4.0.0", and fix the runnable script example by moving the shebang line (#!/usr/bin/env node) to be the very first line of the example file so it precedes the comment "// src/cli.ts"; ensure the example retains the imports for FrontMcp and MyApp (e.g., the top of src/cli.ts should start with the shebang then the imports for FrontMcp from '@frontmcp/sdk' and MyApp from './my.app').libs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.md-63-65 (1)
63-65:⚠️ Potential issue | 🟠 MajorExport the same provider token, not the implementation class.
Line 64 exports
AuditLogger(the class), but line 63 registers the provider underAuditLoggerToken. When the SDK normalizes exports, it will exposeAuditLoggeras the resolvable token—notAuditLoggerToken. Consumers requestingAuditLoggerTokenwill not find it.Suggested fix
`@Plugin`({ name: 'audit-log', description: 'Logs tool executions for audit compliance', providers: [{ provide: AuditLoggerToken, useClass: AuditLogger }], - exports: [AuditLogger], + exports: [AuditLoggerToken], contextExtensions: [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.md` around lines 63 - 65, The module exports the implementation class AuditLogger instead of the provider token AuditLoggerToken, causing consumers that request AuditLoggerToken to not resolve; update the exports array (and any contextExtensions if relevant) to export AuditLoggerToken (the provider token) rather than the AuditLogger class so the normalized exports match the provider registration (refer to providers: [{ provide: AuditLoggerToken, useClass: AuditLogger }], exports, and contextExtensions).libs/skills/catalog/frontmcp-config/examples/configure-http/entry-path-reverse-proxy.md-50-53 (1)
50-53:⚠️ Potential issue | 🟠 MajorFix CORS origin callback to use required callback-based API signature.
The example uses an incorrect return-based pattern. The FrontMCP SDK requires CORS origin callbacks to use a callback-based API where the second parameter is invoked with
(err, allow), not a direct return value.Corrected example
- origin: (origin: string) => { - // allow any *.myapp.com subdomain - return origin.endsWith('.myapp.com'); - }, + origin: (origin: string | undefined, callback) => { + if (!origin) { + callback(null, false); + return; + } + try { + const { hostname } = new URL(origin); + callback(null, hostname.endsWith('.myapp.com')); + } catch { + callback(null, false); + } + },The origin parameter can be
undefined, so direct.endsWith()calls will crash. Additionally, parse the origin as a URL to safely extract the hostname instead of comparing the full origin string.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-config/examples/configure-http/entry-path-reverse-proxy.md` around lines 50 - 53, Update the CORS origin handler to the callback-style API: replace the arrow function signature origin: (origin: string) => { ... } with a two-parameter form (origin, callback) and invoke callback(err, allow) instead of returning a boolean; guard against origin being undefined, and parse the origin as a URL to extract hostname before checking subdomain (e.g., allow any hostname that endsWith '.myapp.com'), then call callback(null, true) for allowed origins or callback(null, false) for disallowed ones (and callback(error) if parsing fails).libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/docker-multi-stage.md-44-45 (1)
44-45:⚠️ Potential issue | 🟠 Major
curlis not available innode:24-slimbase image.The
HEALTHCHECKcommand usescurl, which is not included in the minimalnode:24-slimimage. This will cause the health check to fail in production deployments.🐛 Proposed fixes
Option 1 (Recommended): Install curl in the runtime stage
FROM node:24-slim AS runtime +RUN apt-get update && apt-get install -y curl && rm -rf /var/lib/apt/lists/* WORKDIR /appOption 2: Use Node.js for the health check (no additional dependencies)
-HEALTHCHECK --interval=30s --timeout=5s --retries=3 \ - CMD curl -f http://localhost:3000/health || exit 1 +HEALTHCHECK --interval=30s --timeout=5s --retries=3 \ + CMD node -e "require('http').get('http://localhost:3000/health', (res) => process.exit(res.statusCode === 200 ? 0 : 1)).on('error', () => process.exit(1))"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/docker-multi-stage.md` around lines 44 - 45, The HEALTHCHECK currently uses "CMD curl -f http://localhost:3000/health || exit 1" but curl is not provided by the node:24-slim runtime; either install curl in the runtime stage (add a small apt-get install step in the runtime stage to install curl before setting HEALTHCHECK) or replace the HEALTHCHECK command with a Node.js-based check (e.g., a "node -e" one-liner that performs an HTTP GET against http://localhost:3000/health and exits nonzero on failure); update the Dockerfile's runtime stage where the HEALTHCHECK is declared to use one of these two options.libs/skills/catalog/frontmcp-development/examples/create-agent/basic-agent-with-tools.md-62-66 (1)
62-66:⚠️ Potential issue | 🟠 MajorInclude auth headers and response-status checks in GitHub API tool examples.
As written, this POST call can fail silently and usually won’t authenticate against GitHub. The docs should show
Authorization+Acceptheaders andresponse.okhandling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-agent/basic-agent-with-tools.md` around lines 62 - 66, The POST to GitHub using this.fetch in the example must include Authorization and Accept headers and check the response status; update the call in the example (the this.fetch(...) block that posts to /repos/.../pulls/.../reviews) to add headers: "Authorization: Bearer <token>" (pull token from input.token or env), "Accept: application/vnd.github+json", then inspect the returned Response (e.g., response.ok or response.status) and throw or return a clear error including status/text when the request fails; ensure the function still returns 'Comment posted' only on success.libs/skills/catalog/frontmcp-development/examples/create-skill/directory-based-skill.md-34-50 (1)
34-50:⚠️ Potential issue | 🟠 MajorThe SKILL.md frontmatter example is malformed.
This block is missing the opening
---, so it doesn’t represent valid YAML frontmatter as described.Proposed doc fix
-## <!-- skills/coding-standards/SKILL.md --> - -name: coding-standards +<!-- skills/coding-standards/SKILL.md --> +--- +name: coding-standards description: Project coding standards and patterns tags: [standards, conventions, quality] parameters: @@ ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-skill/directory-based-skill.md` around lines 34 - 50, The SKILL.md frontmatter block is malformed because it’s missing the opening YAML fence; add a leading '---' to the frontmatter so the block starting at "name: coding-standards" is valid YAML frontmatter (i.e., ensure the frontmatter around the SKILL.md example begins with --- and still ends with the existing ---), updating the block that contains "name: coding-standards" and the surrounding frontmatter example in directory-based-skill.md accordingly.libs/skills/catalog/frontmcp-config/examples/configure-auth-modes/remote-enterprise-oauth.md-29-29 (1)
29-29:⚠️ Potential issue | 🟠 MajorFix
z.record()usage for Zod 4 compatibility.
z.record(z.unknown())is invalid in Zod 4, which requires both key and value schemas to be specified.Proposed doc fix
- outputSchema: { rows: z.array(z.record(z.unknown())), rowCount: z.number() }, + outputSchema: { rows: z.array(z.record(z.string(), z.unknown())), rowCount: z.number() },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-config/examples/configure-auth-modes/remote-enterprise-oauth.md` at line 29, The z.record usage in the outputSchema is incompatible with Zod 4; replace z.record(z.unknown()) with a valid key/value form such as z.record(z.string(), z.unknown()) (or z.record(z.string(), z.any()) if preferred) so outputSchema remains: rows: z.array(z.record(...)), rowCount: z.number(); update the entry referencing outputSchema and the z.record call to use the two-argument signature to restore compatibility.libs/skills/catalog/frontmcp-production-readiness/examples/production-node-sdk/package-json-config.md-50-65 (1)
50-65:⚠️ Potential issue | 🟠 MajorUpdate Zod version to 4 in this production-readiness example.
The example pins
zodto^3.23.0, but the repository has migrated to Zod 4. The CHANGELOG states "Scaffold new projects withzod@^4.0.0so freshly generated apps match the runtime upgrade", and the SDK documentation explicitly referenceszod/v4. Using Zod 3 in a production-readiness guide contradicts both the upgrade path and SDK requirements.Proposed fix
"peerDependencies": { - "zod": "^3.23.0", + "zod": "^4.0.0", }, "devDependencies": { - "zod": "^3.23.0", + "zod": "^4.0.0", },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-node-sdk/package-json-config.md` around lines 50 - 65, Update the Zod version references in the example: replace "zod": "^3.23.0" with "zod": "^4.0.0" in both the "peerDependencies" and "devDependencies" blocks so the example aligns with the repository's Zod v4 migration and SDK docs (look for the "peerDependencies" and "devDependencies" entries referencing "zod" in this file).libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/redis-session-scaling.md-44-53 (1)
44-53:⚠️ Potential issue | 🟠 MajorSame fallback inconsistency in jobs.store.redis configuration.
The Redis configuration for the jobs store (lines 50-51) also provides fallback values that bypass the fail-fast validation in
EnvValidationProvider. This creates the same inconsistency noted in the main Redis config.♻️ Recommended fix: Remove fallbacks in jobs config
store: { redis: { provider: 'redis', - host: process.env.REDIS_HOST ?? 'localhost', - port: Number(process.env.REDIS_PORT ?? 6379), + host: process.env.REDIS_HOST!, + port: Number(process.env.REDIS_PORT!), }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/redis-session-scaling.md` around lines 44 - 53, The jobs store Redis config is using fallback values (process.env.REDIS_HOST ?? 'localhost' and Number(process.env.REDIS_PORT ?? 6379}) which bypass EnvValidationProvider; remove those fallbacks so jobs.store.redis reads the raw env vars (e.g., host: process.env.REDIS_HOST, port: Number(process.env.REDIS_PORT)) and let EnvValidationProvider enforce presence/validation for REDIS_HOST and REDIS_PORT.libs/skills/catalog/frontmcp-development/examples/create-provider/config-and-api-providers.md-63-66 (1)
63-66:⚠️ Potential issue | 🟠 MajorValidate environment variables in
onInit()instead of using non-null assertions.Lines 64-65 use non-null assertions (
!) forprocess.env.API_URLandprocess.env.API_KEY, which will silently convertundefinedto an empty string, causing runtime failures when the API client is used. For a provider example demonstrating async setup, validate that required environment variables are present and throw a descriptive error if they're missing.🛡️ Proposed fix with validation
async onInit() { - this.baseUrl = process.env.API_URL!; - this.apiKey = process.env.API_KEY!; + if (!process.env.API_URL || !process.env.API_KEY) { + throw new Error('Missing required environment variables: API_URL and API_KEY'); + } + this.baseUrl = process.env.API_URL; + this.apiKey = process.env.API_KEY; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-provider/config-and-api-providers.md` around lines 63 - 66, Replace the non-null assertions in the provider's async onInit() by explicitly validating process.env.API_URL and process.env.API_KEY: check that both process.env.API_URL and process.env.API_KEY are defined (and non-empty) before assigning to this.baseUrl and this.apiKey in onInit(), and if either is missing throw a descriptive Error (e.g., "Missing required env var API_URL" / "Missing required env var API_KEY") so the provider initialization fails fast with a clear message.libs/skills/catalog/frontmcp-development/examples/decorators-guide/multi-app-with-plugins-and-providers.md-28-33 (1)
28-33:⚠️ Potential issue | 🟠 MajorValidate
DB_URLenvironment variable in the factory.Line 31 passes
process.env.DB_URLtoDatabaseClientwithout validation. If the environment variable is missing, this will passundefined, likely causing a runtime error when the database client is first used. Add validation in the factory function to fail fast during provider initialization.🛡️ Proposed fix with validation
`@Provider`({ name: 'database', provide: DatabaseToken, - useFactory: () => new DatabaseClient(process.env.DB_URL), + useFactory: () => { + if (!process.env.DB_URL) { + throw new Error('Missing required environment variable: DB_URL'); + } + return new DatabaseClient(process.env.DB_URL); + }, }) class DatabaseProvider {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/decorators-guide/multi-app-with-plugins-and-providers.md` around lines 28 - 33, The provider factory for DatabaseToken in the `@Provider` decorator on class DatabaseProvider currently passes process.env.DB_URL directly to DatabaseClient; modify the useFactory function to validate that process.env.DB_URL is a non-empty string and throw a clear error (or otherwise fail-fast) if it's missing or invalid before constructing new DatabaseClient(process.env.DB_URL), so initialization fails early with an actionable message.libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/redis-session-scaling.md-30-37 (1)
30-37:⚠️ Potential issue | 🟠 MajorInconsistency between fail-fast validation and fallback defaults.
Lines 33-35 provide fallback values (
'localhost',6379) for Redis connection parameters, but theEnvValidationProvider(lines 73-78) marksREDIS_HOSTas required and throws if missing. This inconsistency undermines the fail-fast approach: the server will uselocalhostinstead of failing whenREDIS_HOSTis not set.For production deployments with fail-fast validation, remove the fallback defaults so missing environment variables are caught during startup.
♻️ Recommended fix: Remove fallbacks to enforce fail-fast
redis: { provider: 'redis', - host: process.env.REDIS_HOST ?? 'localhost', - port: Number(process.env.REDIS_PORT ?? 6379), + host: process.env.REDIS_HOST!, + port: Number(process.env.REDIS_PORT!), password: process.env.REDIS_PASSWORD, keyPrefix: 'mcp:', // Namespace keys to avoid collisions },Alternatively, update the
EnvValidationProviderto not requireREDIS_HOSTif fallbacks are intentional for development environments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/redis-session-scaling.md` around lines 30 - 37, The redis config block currently provides fallback defaults ('localhost' and 6379) which conflicts with the fail-fast EnvValidationProvider; update the redis config in the redis: { ... } object to stop supplying defaults so the code uses the raw environment variables (e.g. host: process.env.REDIS_HOST and port: Number(process.env.REDIS_PORT)) allowing EnvValidationProvider to catch missing REDIS_HOST/REDIS_PORT at startup; locate the redis config in the file and remove the ?? 'localhost' and ?? 6379 fallbacks.libs/skills/catalog/frontmcp-development/examples/create-provider/config-and-api-providers.md-68-82 (1)
68-82:⚠️ Potential issue | 🟠 MajorAdd HTTP response status validation and error handling.
The
get()andpost()methods don't checkres.okbefore parsing the response, and they lack error handling for network failures or JSON parsing errors. Since this is an example users will copy, it should demonstrate proper error handling patterns for HTTP API calls.🛡️ Proposed fix with response validation
async get(path: string) { const res = await fetch(`${this.baseUrl}${path}`, { headers: { Authorization: `Bearer ${this.apiKey}` }, }); + if (!res.ok) { + throw new Error(`API request failed: ${res.status} ${res.statusText}`); + } return res.json(); } async post(path: string, body: unknown) { const res = await fetch(`${this.baseUrl}${path}`, { method: 'POST', headers: { Authorization: `Bearer ${this.apiKey}`, 'Content-Type': 'application/json' }, body: JSON.stringify(body), }); + if (!res.ok) { + throw new Error(`API request failed: ${res.status} ${res.statusText}`); + } return res.json(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-provider/config-and-api-providers.md` around lines 68 - 82, The get() and post() methods call res.json() without checking res.ok or handling network/parse failures; update both methods (get(), post()) to: wrap the fetch and response parsing in try/catch, check if res.ok and if not read the response body (res.text() or safe res.json()) and throw an Error containing status and body, and on successful responses return parsed JSON; include handling for fetch/network errors and JSON parse errors so callers receive clear, typed errors and the methods still reference this.baseUrl and this.apiKey.libs/skills/catalog/frontmcp-development/examples/create-agent/custom-multi-pass-agent.md-69-69 (1)
69-69:⚠️ Potential issue | 🟠 MajorAdd error handling for JSON.parse to prevent runtime failures.
The LLM may return malformed JSON, causing
JSON.parse()to throw. Since this is an example users will copy, it should demonstrate proper error handling. Consider using a try-catch block or validating againstoutputSchemausing Zod'ssafeParse().🛡️ Proposed fix with try-catch
- return JSON.parse(finalReview.content); + try { + return JSON.parse(finalReview.content); + } catch (error) { + this.fail(new Error(`Failed to parse LLM response as JSON: ${error.message}`)); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-agent/custom-multi-pass-agent.md` at line 69, Wrap the JSON.parse(finalReview.content) call in error handling: catch JSON.parse throws and either attempt schema-safe parsing (e.g., using outputSchema.safeParse / Zod.safeParse) or return a clear error/result when parsing fails. Specifically, modify the place where finalReview.content is parsed (the JSON.parse call) to try-catch the parse error, log or surface the parse failure, and if using outputSchema, validate with outputSchema.safeParse to ensure the shape before returning the parsed value.libs/skills/catalog/frontmcp-deployment/examples/build-for-sdk/create-flat-config.md-29-34 (1)
29-34:⚠️ Potential issue | 🟠 MajorRemove
evalfrom the example tool implementation.Line 33 demonstrates executing untrusted input with
eval, which is unsafe in production-like patterns and easy to cargo-cult.🔧 Safer example replacement
- tool({ - name: 'calculate', - description: 'Perform calculation', - inputSchema: { expression: z.string() }, - outputSchema: { result: z.number() }, - })((input) => ({ result: eval(input.expression) })), + tool({ + name: 'calculate', + description: 'Perform calculation', + inputSchema: { + a: z.number(), + b: z.number(), + op: z.enum(['add', 'sub', 'mul', 'div']), + }, + outputSchema: { result: z.number() }, + })(({ a, b, op }) => { + const result = + op === 'add' ? a + b : + op === 'sub' ? a - b : + op === 'mul' ? a * b : + b === 0 ? NaN : a / b; + return { result }; + }), @@ - const result = await server.callTool('calculate', { expression: '2 + 2' }); + const result = await server.callTool('calculate', { a: 2, b: 2, op: 'add' });Also applies to: 39-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-deployment/examples/build-for-sdk/create-flat-config.md` around lines 29 - 34, The example tool named 'calculate' currently uses eval to compute input.expression (symbol: eval in the calculate tool, inputSchema/outputSchema/result), which is unsafe; replace eval with a safe evaluator (e.g., use a well-maintained math expression library such as mathjs or a small whitelist parser) and validate the expression against allowed characters/operators before evaluation, return errors on invalid input, and apply the same replacement to the other instance of eval referenced in the example (lines 39-40) so no untrusted code is executed.libs/skills/catalog/frontmcp-config/examples/configure-auth-modes/transparent-jwt-validation.md-25-33 (1)
25-33:⚠️ Potential issue | 🟠 Major
get_profileexample should not trust caller-supplieduserIdfor authenticated identity.At Line 27 and Line 31, the example accepts
userIdfrom input while describing an authenticated profile flow. This can teach an IDOR-prone pattern. Prefer deriving subject/user id from auth context and removinguserIdfrom input schema.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-config/examples/configure-auth-modes/transparent-jwt-validation.md` around lines 25 - 33, The example exposes an IDOR risk by accepting caller-supplied userId in GetProfileTool's inputSchema and execute signature; remove userId from inputSchema, stop trusting input.userId, and derive the authenticated subject from the request/auth context instead. Update the GetProfileTool.execute method to read the user id from the authentication/context API your framework provides (e.g., this.auth.user.id or a passed-in Context.getUser()), adjust the execute signature accordingly, and return that derived id/email rather than input.userId. Ensure inputSchema no longer declares userId and that any callers provide no user-supplied id.libs/skills/catalog/frontmcp-config/examples/configure-auth/multi-app-auth.md-63-66 (1)
63-66:⚠️ Potential issue | 🟠 MajorAvoid default fallback for admin OAuth client id in auth example.
At Line 65,
?? 'admin-client'can normalize unsafe defaults for a privileged app. Prefer requiring the env var (or throwing on missing) in this security-focused example.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-config/examples/configure-auth/multi-app-auth.md` around lines 63 - 66, The example currently uses a fallback default for the admin OAuth client id (clientId: process.env['ADMIN_OAUTH_CLIENT_ID'] ?? 'admin-client'), which is unsafe for a privileged app; change it to require the ADMIN_OAUTH_CLIENT_ID environment variable (or explicitly throw/validate if missing) so no implicit 'admin-client' default is used, updating the clientId assignment and adding a clear error/validation path referencing ADMIN_OAUTH_CLIENT_ID and clientId.libs/skills/catalog/frontmcp-development/examples/create-adapter/basic-api-adapter.md-34-41 (1)
34-41:⚠️ Potential issue | 🟠 MajorAdd defensive checks for fetch response and schema structure.
The example omits error handling before accessing
res.json()andschema.operations, which will fail with unclear errors on non-2xx responses or malformed payloads. The intermediatenamespaced-adapterexample explicitly demonstrates this pattern—compare its error handling at lines 48–57 with the basic example's current approach.Suggested defensive check
const res = await globalThis.fetch(this.options.endpoint, { headers: { Authorization: `Bearer ${this.options.apiKey}` }, }); + if (!res.ok) { + throw new Error(`Failed to fetch adapter schema: ${res.status} ${res.statusText}`); + } const schema = await res.json(); + if (!schema || !Array.isArray(schema.operations)) { + throw new Error('Invalid adapter schema: missing operations array'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-adapter/basic-api-adapter.md` around lines 34 - 41, Add defensive error handling around the fetch and schema parsing: after calling globalThis.fetch(this.options.endpoint, ...), check res.ok and throw a clear error including res.status and endpoint if not 2xx; wrap await res.json() in try/catch to surface JSON parse errors; validate that the parsed schema has an array at schema.operations before mapping (throw a descriptive error referencing schema.operations and this.options.endpoint if missing or invalid). Apply these checks in the function that returns the tools so mapping over schema.operations is only done when the response and schema are valid.libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/graceful-shutdown.md-44-65 (1)
44-65:⚠️ Potential issue | 🟠 MajorAdd error handling in the shutdown path.
The
shutdown()function awaitsserver.close()andserver.dispose()without error handling. If either promise rejects, the shutdown path becomes incomplete—cleanup may be skipped, and no error is logged. While the 30-seconddrainTimeoutacts as a safety net (forcing exit if shutdown hangs), it does not catch errors fromclose()ordispose()or prevent partial cleanup. For a production-readiness example, this should demonstrate proper error handling and resource cleanup in all cases.Suggested hardening
const shutdown = async (signal: string) => { if (isShuttingDown) return; isShuttingDown = true; - - console.log(`Received ${signal}. Starting graceful shutdown...`); - - // 1. Stop accepting new connections - await server.close(); - console.log('Server closed — no new connections accepted.'); - - // 2. Wait for in-flight requests to complete (with timeout) - const drainTimeout = setTimeout(() => { - console.error('Drain timeout reached. Forcing exit.'); - process.exit(1); - }, 30_000); // 30 second drain period - - // 3. Dispose all resources (Redis, DB connections, providers) - await server.dispose(); - clearTimeout(drainTimeout); - console.log('All resources disposed. Exiting.'); - - process.exit(0); + console.log(`Received ${signal}. Starting graceful shutdown...`); + const drainTimeout = setTimeout(() => { + console.error('Drain timeout reached. Forcing exit.'); + process.exit(1); + }, 30_000); + try { + await server.close(); + console.log('Server closed — no new connections accepted.'); + await server.dispose(); + console.log('All resources disposed. Exiting.'); + process.exit(0); + } catch (error) { + console.error('Graceful shutdown failed:', error); + process.exit(1); + } finally { + clearTimeout(drainTimeout); + } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/graceful-shutdown.md` around lines 44 - 65, The shutdown function currently awaits server.close() and server.dispose() without handling rejections; wrap the shutdown sequence in try/catch/finally so errors from server.close() or server.dispose() are caught and logged (using console.error), ensure server.dispose() is attempted even if server.close() fails (use finally or nested try/finally), always clear the drainTimeout before exiting, and call process.exit(1) on error vs process.exit(0) on clean shutdown; refer to the shutdown function and the server.close / server.dispose calls and the isShuttingDown and drainTimeout symbols when making these changes.libs/skills/catalog/frontmcp-development/examples/create-tool-annotations/readonly-query-tool.md-43-43 (1)
43-43:⚠️ Potential issue | 🟠 MajorAdd missing
DatabaseTokenimport to the example.Line 43 uses
this.get(DatabaseToken)without importing or definingDatabaseToken, making the example non-functional as written. Add the import statement:Suggested doc fix
import { Tool, ToolContext } from '@frontmcp/sdk'; import { z } from 'zod'; +import { DatabaseToken } from '../providers/database.provider';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-tool-annotations/readonly-query-tool.md` at line 43, The example uses this.get(DatabaseToken) but omits the DatabaseToken import; add an import for DatabaseToken at the top of the example (same module/namespace used elsewhere in your project) so the symbol DatabaseToken is defined and the call this.get(DatabaseToken) resolves correctly; update the example imports section to include DatabaseToken alongside the other imports referenced in the snippet.libs/skills/catalog/frontmcp-production-readiness/examples/production-cloudflare/durable-objects-state.md-96-110 (1)
96-110:⚠️ Potential issue | 🟠 MajorThis “upload” example never stores the file.
input.contentis never written anywhere; the snippet only fabricates a key/url and usesinput.content.length, which is the encoded string length rather than the uploaded byte size. For an example titled “Upload a file to R2 object storage”, this is misleading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-cloudflare/durable-objects-state.md` around lines 96 - 110, The example never writes the file to R2 and reports the wrong size; update FileUploadTool.execute to decode the base64 input.content to bytes, compute the byte length for size, and upload the decoded blob to R2 using the generated key (e.g., call the R2 binding's put with key and the decoded body). Ensure the returned object uses the same key and the correct byte size and that the R2 binding name you use matches the Worker binding (refer to key, size, execute, and FileUploadTool).libs/skills/catalog/frontmcp-development/examples/create-adapter/namespaced-adapter.md-43-79 (1)
43-79:⚠️ Potential issue | 🟠 MajorThe generated tools cannot handle GraphQL fields with arguments.
The introspection query requests only
nameanddescription(notargs),inputSchemais always empty, and the query string is hard-coded as{ ${queryName} }without any variable placeholders. This makes any field with required or optional arguments unusable. Fix by introspecting theargsfield, generatinginputSchemafrom it, and constructing parameterized queries that reference the variables.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-adapter/namespaced-adapter.md` around lines 43 - 79, The tools generation ignores field args and always uses an empty inputSchema and a non-parameterized query; update the introspection fetch to request fields[].args and, inside the fields.map in the method that returns tools, build a proper inputSchema from each field.args (including types and required flags) and set the tool execute to call executeQuery with both the queryName and a variables object; then change executeQuery(queryName, variables) to construct a parameterized GraphQL operation (e.g., a query/mutation signature with variable definitions and field argument placeholders) and send the variables in the request body so fields with required or optional arguments are supported.libs/skills/catalog/frontmcp-deployment/examples/deploy-to-cloudflare/worker-with-kv-storage.md-24-33 (1)
24-33:⚠️ Potential issue | 🟠 MajorThis example never actually stores anything in KV.
Lines 30-33 only echo the key, and Lines 40-46 never wire
FRONTMCP_KVinto the server or tool implementation. Right now this page documents a plain SSE Worker with an unused KV binding, not KV-backed session/state storage.Also applies to: 40-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-deployment/examples/deploy-to-cloudflare/worker-with-kv-storage.md` around lines 24 - 33, The example's StoreValueTool (Tool named 'store_value', class StoreValueTool extends ToolContext) never writes to the KV binding FRONTMCP_KV—its execute() only echoes the key and the FRONTMCP_KV binding is not passed into the tool/server—so update the example to use the FRONTMCP_KV binding inside execute() to store the value (e.g., await FRONTMCP_KV.put(key, value)) and ensure the server instantiation or tool wiring passes the Cloudflare binding into the ToolContext or exposes FRONTMCP_KV to the tool (adjust the server setup lines that register tools and bindings so FRONTMCP_KV is available to StoreValueTool and any session/state tools).libs/skills/catalog/frontmcp-development/examples/create-job/job-with-permissions.md-45-77 (1)
45-77:⚠️ Potential issue | 🟠 MajorExport the jobs and import them into
src/server.ts.
src/server.tsregistersDataExportJobandCleanupTempFiles, but the job snippet does not export either symbol and the server snippet never imports them. As written, the split example cannot compile.🐛 Proposed fix
- class DataExportJob extends JobContext { + export class DataExportJob extends JobContext { async execute(input: { dataset: string; destination: string }) { this.log(`Exporting dataset: ${input.dataset}`); const rows = await this.exportData(input.dataset, input.destination); return { exportedRows: rows, location: input.destination }; } @@ -const CleanupTempFiles = job({ +export const CleanupTempFiles = job({ name: 'cleanup-temp-files', description: 'Remove temporary files older than the specified age', inputSchema: { directory: z.string().describe('Directory to clean'), maxAgeDays: z.number().int().min(1).default(7),// src/server.ts import { FrontMcp, App } from '@frontmcp/sdk'; +import { DataExportJob, CleanupTempFiles } from './jobs/data-export.job'; `@App`({ name: 'data-app', jobs: [DataExportJob, CleanupTempFiles], })Also applies to: 82-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-job/job-with-permissions.md` around lines 45 - 77, The file fails to export the job symbols used by the server; export the DataExportJob class and the CleanupTempFiles job constant (e.g., add exports for DataExportJob and CleanupTempFiles) and then update the server import site to import those exact symbols so src/server.ts can register them; look for the class DataExportJob and the const CleanupTempFiles and ensure they are exported and imported where jobs are registered.libs/skills/catalog/frontmcp-development/examples/create-provider/basic-database-provider.md-33-59 (1)
33-59:⚠️ Potential issue | 🟠 MajorAdd the missing imports/exports across the split files.
Line 38 implements
DatabaseServicewithout importing it,DatabaseProviderandQueryUsersToolare not exported, andindex.tsreferences both classes without imports. The example cannot compile as written.🐛 Proposed fix
// src/apps/main/providers/database.provider.ts import { Provider } from '@frontmcp/sdk'; import { createPool, Pool } from 'your-db-driver'; +import { DatabaseService } from '../tokens'; `@Provider`({ name: 'DatabaseProvider' }) -class DatabaseProvider implements DatabaseService { +export class DatabaseProvider implements DatabaseService { private pool!: Pool;// src/apps/main/tools/query-users.tool.ts `@Tool`({ name: 'query_users', @@ }) -class QueryUsersTool extends ToolContext { +export class QueryUsersTool extends ToolContext { async execute(input: { filter?: string; limit: number }) { const db = this.get(DB_TOKEN);// src/apps/main/index.ts import { App } from '@frontmcp/sdk'; +import { DatabaseProvider } from './providers/database.provider'; +import { QueryUsersTool } from './tools/query-users.tool'; `@App`({ name: 'main',Also applies to: 63-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/examples/create-provider/basic-database-provider.md` around lines 33 - 59, The example is missing imports/exports: add an exported DatabaseService type/interface and ensure DatabaseProvider and QueryUsersTool are exported from their modules, and import them where index.ts references them; specifically, declare/export the DatabaseService interface used by DatabaseProvider, export the DatabaseProvider class and the QueryUsersTool class (or default-export if the example expects that), and update index.ts to import { DatabaseProvider, QueryUsersTool } (or the appropriate named/default imports) so all referenced symbols resolve.libs/skills/catalog/frontmcp-deployment/examples/deploy-to-lambda/cdk-deployment.md-42-42 (1)
42-42:⚠️ Potential issue | 🟠 MajorUse
ssm-securefor theSecureStringparameter reference.Line 42 uses the plaintext
ssmdynamic-reference form, but lines 72–75 create/frontmcp/auth-secretas aSecureString. AWS CloudFormation requires{{resolve:ssm-secure:...}}forSecureStringparameters;ssmis only for String/StringList types. (AWS docs)Proposed fix
- FRONTMCP_AUTH_SECRET: cdk.Fn.sub('{{resolve:ssm:/frontmcp/auth-secret}}'), + FRONTMCP_AUTH_SECRET: cdk.Fn.sub('{{resolve:ssm-secure:/frontmcp/auth-secret}}'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-deployment/examples/deploy-to-lambda/cdk-deployment.md` at line 42, The dynamic reference for FRONTMCP_AUTH_SECRET uses cdk.Fn.sub with '{{resolve:ssm:/frontmcp/auth-secret}}' but the parameter is created as a SecureString; update the reference to use the ssm-secure form (i.e., '{{resolve:ssm-secure:/frontmcp/auth-secret}}') in the FRONTMCP_AUTH_SECRET value so CloudFormation resolves the SecureString correctly while keeping the cdk.Fn.sub usage intact.libs/sdk/src/skill/__tests__/read-skill-content.spec.ts-154-252 (1)
154-252:⚠️ Potential issue | 🟠 MajorThese specs never exercise
ReadSkillContentToolitself.The lookup cases only prove
find()/map()on local arrays, and the file-reading cases only prove the mock/parser return what they were configured to return. A broken path-resolution, not-found, or response-shaping branch in the tool would still pass here, so the new feature still lacks direct success/error-path coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/sdk/src/skill/__tests__/read-skill-content.spec.ts` around lines 154 - 252, Tests currently never call ReadSkillContentTool itself; they only exercise array lookups and the parser mock. Update the spec to instantiate and call ReadSkillContentTool (e.g., new ReadSkillContentTool().run(...) or its actual entry method) for: a successful reference lookup (mock fs read to return expected file and assert tool output shape), a successful example lookup, a not-found path (assert tool returns undefined/error message and lists available names), and a path-resolution or read error (mock read failure and assert tool surfaces the error). Keep using parseSkillMdFrontmatter for frontmatter assertions but route input through ReadSkillContentTool so you cover its path-resolution, not-found, and error-shaping branches.libs/skills/__tests__/skills-validation.spec.ts-143-191 (1)
143-191:⚠️ Potential issue | 🟠 MajorThe new validators still skip legacy
category/skillcatalog entries.
findAllSkillDirs()already handles nested skill roots, but these new collectors and the examples-directory check only walkCATALOG_DIR/<entry>. Any nested skill’s references/examples are invisible to these assertions, so the suite can pass while missing part of the catalog.Also applies to: 419-426, 498-523
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/__tests__/skills-validation.spec.ts` around lines 143 - 191, The collectors getAllReferenceFiles and getAllExampleFiles only iterate CATALOG_DIR/<entry> and so miss nested skill roots; update them to iterate the full set of skill directories returned by findAllSkillDirs() (or reuse that helper) instead of re-reading CATALOG_DIR, and for each returned skillDir use path.join(skillDir, 'references'|'examples'| 'SKILL.md') to discover files; ensure the examples logic still walks reference subdirs under each skillDir and that SKILL.md detection uses the exact skillDir path so legacy category/skill entries are included.libs/skills/catalog/frontmcp-production-readiness/examples/production-lambda/scaling-and-monitoring.md-98-115 (1)
98-115:⚠️ Potential issue | 🟠 MajorImplement pagination to load all Parameter Store secrets.
GetParametersByPathpaginates withNextToken(default max 10 results per page). This code only processes the first page, so secrets beyond that will be silently omitted from the cache. Loop through all pages usingNextTokenuntil it's undefined. Also consider addingRecursive: trueto retrieve parameters from nested paths under/mcp/production/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-lambda/scaling-and-monitoring.md` around lines 98 - 115, The onInit method only processes the first page from SSM GetParametersByPath; update it to loop using the GetParametersByPathCommand's NextToken until undefined so all pages are retrieved, setting Recursive: true on the command to include nested paths and keeping WithDecryption: true; on each page iterate result.Parameters and call this.cache.set(key, param.Value ?? '') as before. Use the same SSMClient/ GetParametersByPathCommand symbols and preserve the path variable while passing NextToken on subsequent requests.libs/skills/catalog/frontmcp-production-readiness/examples/common-checklist/observability-setup.md-37-62 (1)
37-62:⚠️ Potential issue | 🟠 MajorHandle HTTP error statuses explicitly instead of collapsing them to a single message.
The example treats 404, 401, and 500 as the same failure (
response.ok === false→ "Operation not found"), while network and timeout errors still escape as uncaught exceptions. In a production-readiness guide, distinguish HTTP status codes explicitly and handle network failures separately.private async processOperation(id: string): Promise<boolean> { const response = await this.fetch(`https://api.example.com/operations/${id}`, { signal: AbortSignal.timeout(5000), }); if (!response.ok) { // Handle HTTP errors explicitly if (response.status === 404) { this.fail(new Error(`Operation not found: ${id}`)); } else if (response.status === 401) { this.fail(new Error(`Unauthorized access to operation: ${id}`)); } else { this.fail(new Error(`Server error: ${response.status}`)); } } return response.ok; }Network and timeout errors will still throw; wrap the call if you need to handle those separately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/common-checklist/observability-setup.md` around lines 37 - 62, The current processOperation collapses all non-OK responses and can throw unhandled network/timeout errors; update the private async processOperation(id: string) to explicitly branch on response.status (e.g., handle 404 -> call this.fail(new Error(`Operation not found: ${id}`)), 401 -> this.fail(new Error(`Unauthorized access to operation: ${id}`)), and other 5xx/4xx -> this.fail(new Error(`Server error: ${response.status}`))) and also wrap the fetch call in a try/catch to convert network or AbortSignal timeout errors into controlled failures via this.fail instead of letting exceptions escape; ensure execute() continues to use this.fail and only returns boolean/completed status when it’s safe to do so.libs/skills/catalog/frontmcp-deployment/examples/deploy-to-node/docker-compose-with-redis.md-70-75 (1)
70-75:⚠️ Potential issue | 🟠 MajorAdd
yarn.lockto the runtime stage before runningyarn install --frozen-lockfile.The production image runs
yarn install --frozen-lockfilebut only copiespackage.jsoninto that stage. This will fail during image build becauseyarn install --frozen-lockfilerequires the lockfile to be present and will not generate one.Suggested fix
FROM node:24-alpine AS production WORKDIR /app ENV NODE_ENV=production COPY --from=builder /app/dist ./dist COPY --from=builder /app/package.json ./ +COPY --from=builder /app/yarn.lock ./ RUN yarn install --frozen-lockfile --production && yarn cache clean🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-deployment/examples/deploy-to-node/docker-compose-with-redis.md` around lines 70 - 75, The production Docker stage (FROM node:24-alpine AS production) runs RUN yarn install --frozen-lockfile --production but only copies package.json into the stage, causing yarn to fail because yarn.lock is missing; fix by copying the lockfile into the production stage before running yarn install (e.g., COPY --from=builder /app/yarn.lock ./ or COPY yarn.lock ./) so that the RUN yarn install --frozen-lockfile command has the required yarn.lock present.libs/skills/catalog/frontmcp-production-readiness/examples/production-lambda/scaling-and-monitoring.md-35-43 (1)
35-43:⚠️ Potential issue | 🟠 MajorAdd
AutoPublishAliasto enable provisioned concurrency.
ProvisionedConcurrencyConfigon a SAMAWS::Serverless::Functionrequires a published version or alias to target. AddAutoPublishAliasto automatically manage this on each deployment.Suggested fix
# Reserved concurrency prevents downstream overload ReservedConcurrentExecutions: 100 + AutoPublishAlias: live # Provisioned concurrency for latency-sensitive endpoints ProvisionedConcurrencyConfig: ProvisionedConcurrentExecutions: 5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/examples/production-lambda/scaling-and-monitoring.md` around lines 35 - 43, The SAM function's ProvisionedConcurrencyConfig requires a published version or alias but the template lacks AutoPublishAlias; update the AWS::Serverless::Function resource that contains ProvisionedConcurrencyConfig to include an AutoPublishAlias (e.g., "live" or another stable alias name) so SAM will automatically publish versions and manage the alias on each deployment, ensuring provisioned concurrency can target that alias; keep the alias name consistent with any invocation/configuration referring to the function.
libs/skills/catalog/frontmcp-deployment/examples/deploy-to-cloudflare/basic-worker-deploy.md
Show resolved
Hide resolved
libs/skills/catalog/frontmcp-development/examples/create-plugin-hooks/basic-logging-plugin.md
Outdated
Show resolved
Hide resolved
...ontmcp-production-readiness/examples/production-cli-binary/stdio-transport-error-handling.md
Outdated
Show resolved
Hide resolved
...ontmcp-production-readiness/examples/production-cli-binary/stdio-transport-error-handling.md
Outdated
Show resolved
Hide resolved
...kills/catalog/frontmcp-production-readiness/examples/production-vercel/vercel-edge-config.md
Show resolved
Hide resolved
libs/skills/catalog/frontmcp-testing/examples/test-cli-binary/binary-startup-test.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/skills/__tests__/skills-validation.spec.ts`:
- Around line 111-149: parseExamplesTableRows currently discards the markdown
link target in the Example column so tests only validate the visible label;
update parseExamplesTableRows to parse and return the link href (e.g., add an
href/path field to each row alongside name, level, description by extracting the
markdown link target from exampleCell) and update the corresponding tests
(including the other test block around lines 676-707) to assert that the
returned href resolves to the expected example markdown path rather than only
checking the visible name.
- Around line 152-200: The reference and example file scanners
(getAllReferenceFiles and getAllExampleFiles) only iterate catalog/* and miss
nested skills; update both functions to use the existing findAllSkillDirs()
helper (or the same skill-root discovery used elsewhere) to enumerate skills so
nested paths like catalog/<category>/<skill> are included, and when pushing
results preserve the full relative skill path (the same string returned by
findAllSkillDirs) into the skill property so downstream validation uses the
canonical skill path; apply the same change pattern to the other similar blocks
mentioned (lines ~428-435 and ~507-533).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6b4bc4e9-2c0b-4aed-b1f9-3e1df991919f
📒 Files selected for processing (1)
libs/skills/__tests__/skills-validation.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
libs/skills/__tests__/skills-validation.spec.ts (2)
712-727:⚠️ Potential issue | 🟡 MinorFail when the Example cell has no link target.
The link/path assertions only run when
row.hrefis truthy. If the table cell degrades to plain text or a malformed markdown link, this test still passes as long as the visible label matches the manifest entry.🔗 Suggested assertion
- // Validate that the href resolves to the expected example file - if (row.href) { - const expectedHref = `../examples/${ref.name}/${example.name}.md`; - if (row.href !== expectedHref) { - mismatches.push( - `${entry.name}/${ref.name}/${example.name}: href "${row.href}" does not match expected "${expectedHref}"`, - ); - } - // Also verify the target file exists on disk - const resolvedPath = path.resolve(path.dirname(referencePath), row.href); - if (!fs.existsSync(resolvedPath)) { - mismatches.push( - `${entry.name}/${ref.name}/${example.name}: href target "${row.href}" does not exist on disk`, - ); - } - } + const expectedHref = `../examples/${ref.name}/${example.name}.md`; + if (!row.href) { + mismatches.push(`${entry.name}/${ref.name}/${example.name}: reference table is missing the example href`); + continue; + } + if (row.href !== expectedHref) { + mismatches.push( + `${entry.name}/${ref.name}/${example.name}: href "${row.href}" does not match expected "${expectedHref}"`, + ); + } + const resolvedPath = path.resolve(path.dirname(referencePath), row.href); + if (!fs.existsSync(resolvedPath)) { + mismatches.push( + `${entry.name}/${ref.name}/${example.name}: href target "${row.href}" does not exist on disk`, + ); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/__tests__/skills-validation.spec.ts` around lines 712 - 727, The test currently only validates href content when row.href is truthy, allowing missing or non-linked Example cells to pass; update the validation in the block that builds mismatches so it asserts presence of a link target: if row.href is falsy or empty, push a mismatch like `${entry.name}/${ref.name}/${example.name}: missing href link for example`, otherwise continue existing checks (compare to expectedHref and verify fs.existsSync on resolvedPath). Reference row.href, mismatches, expectedHref, resolvedPath, and the entry.name/ref.name/example.name context when adding the new assertion.
156-204:⚠️ Potential issue | 🟠 MajorUse
findAllSkillDirs()for every catalog traversal here.These scans still only walk one level under
CATALOG_DIR, so legacycatalog/<category>/<skill>entries never make it intodocumentationFiles,getAllExampleFiles(), or the examples/reference sync check. The downstream comparisons also need the full relative skill path, not just the top-level directory name.🧭 Suggested traversal pattern
function getAllReferenceFiles(): { skill: string; file: string; fullPath: string }[] { const results: { skill: string; file: string; fullPath: string }[] = []; - const entries = fs.readdirSync(CATALOG_DIR).filter((f) => { - const full = path.join(CATALOG_DIR, f); - return fs.statSync(full).isDirectory() && f !== 'node_modules'; - }); - for (const entry of entries) { - const refsDir = path.join(CATALOG_DIR, entry, 'references'); + for (const skill of findAllSkillDirs()) { + const refsDir = path.join(CATALOG_DIR, skill, 'references'); if (fs.existsSync(refsDir)) { const files = fs.readdirSync(refsDir).filter((f) => f.endsWith('.md')); for (const file of files) { - results.push({ skill: entry, file, fullPath: path.join(refsDir, file) }); + results.push({ skill, file, fullPath: path.join(refsDir, file) }); } } - const skillMd = path.join(CATALOG_DIR, entry, 'SKILL.md'); + const skillMd = path.join(CATALOG_DIR, skill, 'SKILL.md'); if (fs.existsSync(skillMd)) { - results.push({ skill: entry, file: 'SKILL.md', fullPath: skillMd }); + results.push({ skill, file: 'SKILL.md', fullPath: skillMd }); } } return results; }- const entries = fs.readdirSync(CATALOG_DIR).filter((f) => { - const full = path.join(CATALOG_DIR, f); - return fs.statSync(full).isDirectory() && f !== 'node_modules'; - }); - for (const entry of entries) { - const examplesDir = path.join(CATALOG_DIR, entry, 'examples'); + for (const skill of findAllSkillDirs()) { + const examplesDir = path.join(CATALOG_DIR, skill, 'examples'); if (!fs.existsSync(examplesDir)) continue; - const refsDir = path.join(CATALOG_DIR, entry, 'references'); + const refsDir = path.join(CATALOG_DIR, skill, 'references'); // ... for (const dir of exampleDirs) { if (!refNames.includes(dir)) { - mismatches.push(`${entry}/examples/${dir} has no matching reference file`); + mismatches.push(`${skill}/examples/${dir} has no matching reference file`); } } }Apply the same
for (const skill of findAllSkillDirs())pattern togetAllExampleFiles()as well.Also applies to: 432-439, 511-537
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/__tests__/skills-validation.spec.ts` around lines 156 - 204, getAllReferenceFiles and getAllExampleFiles only iterate one level under CATALOG_DIR and use the top-level folder name; change both to iterate over the full list returned by findAllSkillDirs() (e.g. for (const skill of findAllSkillDirs())) so nested paths like category/skill are included, replace usages of the local entry/refDir variables with the full relative skill path returned by findAllSkillDirs(), and ensure any downstream comparisons (e.g. documentationFiles and the examples/reference sync checks) use that full relative skill string rather than only the top-level directory name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/cli/src/commands/skills/read.ts`:
- Around line 65-67: The check treating options.examplesForRef as a truthy flag
allows an empty string to bypass examples mode; update the conditional logic
around options.listExamples and options.examplesForRef so that examples mode
only runs when examplesForRef is a non-empty string (e.g., typeof
options.examplesForRef === 'string' && options.examplesForRef.trim() !== '') or
when listExamples is true, and explicitly handle or reject a blank
examplesForRef (for example return an error message or usage hint). Modify the
block that defines refs and refFilter (the variables entry.references, const
refs and const refFilter) to use the stricter check and add explicit handling
for the blank-filter case instead of falling back to the default read path.
---
Duplicate comments:
In `@libs/skills/__tests__/skills-validation.spec.ts`:
- Around line 712-727: The test currently only validates href content when
row.href is truthy, allowing missing or non-linked Example cells to pass; update
the validation in the block that builds mismatches so it asserts presence of a
link target: if row.href is falsy or empty, push a mismatch like
`${entry.name}/${ref.name}/${example.name}: missing href link for example`,
otherwise continue existing checks (compare to expectedHref and verify
fs.existsSync on resolvedPath). Reference row.href, mismatches, expectedHref,
resolvedPath, and the entry.name/ref.name/example.name context when adding the
new assertion.
- Around line 156-204: getAllReferenceFiles and getAllExampleFiles only iterate
one level under CATALOG_DIR and use the top-level folder name; change both to
iterate over the full list returned by findAllSkillDirs() (e.g. for (const skill
of findAllSkillDirs())) so nested paths like category/skill are included,
replace usages of the local entry/refDir variables with the full relative skill
path returned by findAllSkillDirs(), and ensure any downstream comparisons (e.g.
documentationFiles and the examples/reference sync checks) use that full
relative skill string rather than only the top-level directory name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 22688480-544f-4b61-aa86-d8d3529db599
📒 Files selected for processing (11)
libs/cli/src/commands/skills/read.tslibs/skills/__tests__/skills-validation.spec.tslibs/skills/catalog/frontmcp-config/examples/configure-auth-modes/remote-enterprise-oauth.mdlibs/skills/catalog/frontmcp-deployment/examples/build-for-sdk/create-flat-config.mdlibs/skills/catalog/frontmcp-deployment/examples/deploy-to-lambda/cdk-deployment.mdlibs/skills/catalog/frontmcp-deployment/examples/deploy-to-node/docker-compose-with-redis.mdlibs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.mdlibs/skills/catalog/frontmcp-production-readiness/examples/production-cli-binary/binary-build-config.mdlibs/skills/catalog/frontmcp-production-readiness/examples/production-lambda/scaling-and-monitoring.mdlibs/skills/catalog/frontmcp-production-readiness/examples/production-node-sdk/package-json-config.mdlibs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/docker-multi-stage.md
✅ Files skipped from review due to trivial changes (6)
- libs/skills/catalog/frontmcp-deployment/examples/build-for-sdk/create-flat-config.md
- libs/skills/catalog/frontmcp-config/examples/configure-auth-modes/remote-enterprise-oauth.md
- libs/skills/catalog/frontmcp-production-readiness/examples/production-cli-binary/binary-build-config.md
- libs/skills/catalog/frontmcp-production-readiness/examples/production-node-server/docker-multi-stage.md
- libs/skills/catalog/frontmcp-deployment/examples/deploy-to-node/docker-compose-with-redis.md
- libs/skills/catalog/frontmcp-production-readiness/examples/production-node-sdk/package-json-config.md
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/skills/catalog/frontmcp-production-readiness/examples/production-lambda/scaling-and-monitoring.md
- libs/skills/catalog/frontmcp-development/examples/create-plugin/plugin-with-context-extension.md
- libs/skills/catalog/frontmcp-deployment/examples/deploy-to-lambda/cdk-deployment.md
Cherry-pick CreatedA cherry-pick PR to Please review and merge if this change should also be in If the cherry-pick is not needed, close the PR. |
1 participant
Summary by CodeRabbit
New Features
Documentation
Configuration