feat: improve error handling and validation in skills manifest and related files#316
Merged
frontegg-david merged 6 commits intorelease/1.0.xfrom Mar 29, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdated CLI color handling and help layout; replaced Changes
Sequence Diagram(s)sequenceDiagram
participant FrontMcp as FrontMcpInstance
participant Server as FrontMcpServer
participant Scope as ScopeEntry
FrontMcp->>Server: await server.start()
Server-->>FrontMcp: started
FrontMcp->>Scope: for each scope -> scope.emitServerStarted()
Scope->>Scope: invoke registered callbacks sequentially (await each)
Scope-->>FrontMcp: completed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-03-29T01:57:52.918Z |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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/core/__tests__/help.spec.ts (1)
54-83:⚠️ Potential issue | 🟡 MinorMake the command-presence check line-aware.
Line 69's
listand Line 72'sinstallchecks can now pass via the newskills list/skills installentries on Line 76 and Line 77, so this test no longer proves the top-level commands are rendered. Match against normalized help lines instead of raw substring containment.Suggested test tightening
it('should list all commands in help output', () => { const help = getHelpOutput(); + const helpLines = help + .split('\n') + .map((line) => line.replace(/\x1B\[[0-9;]*m/g, '').trimStart()); + const expectedCommands = [ 'dev', 'build', @@ 'skills show', ]; for (const cmd of expectedCommands) { - expect(help).toContain(cmd); + expect(helpLines.some((line) => line.startsWith(cmd))).toBe(true); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/core/__tests__/help.spec.ts` around lines 54 - 83, The test currently checks command presence with raw substring matches which falsely passes when subcommands like "skills list" include substrings "list"/"install"; update the test in "should list all commands in help output" to perform line-aware checks: call getHelpOutput(), split it into lines (e.g., .split(/\r?\n/)), normalize each line with trim(), then for each entry in expectedCommands ensure there exists a normalized help line that either equals the command or startsWith the command followed by whitespace (to allow descriptions). Locate the expectedCommands array and replace the for-loop that uses expect(help).toContain(cmd) with this line-based existence check (you can extract a small helper inside the test to find the matching line).
🧹 Nitpick comments (2)
libs/cli/src/core/__tests__/colors.spec.ts (1)
100-142: Consider adding a test forNO_COLOR=""(empty string).The test suite covers the main scenarios well, but missing a test for the edge case where
NO_COLORis set to an empty string. According to the NO_COLOR standard, this should also disable colors.✅ Proposed additional test
it('should return plain text when NO_COLOR is set', () => { process.env['NO_COLOR'] = '1'; const result = c('red', 'error text'); expect(result).toBe('error text'); }); + + it('should return plain text when NO_COLOR is empty string', () => { + process.env['NO_COLOR'] = ''; + const result = c('red', 'error text'); + expect(result).toBe('error text'); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/core/__tests__/colors.spec.ts` around lines 100 - 142, Add a test in the existing "NO_COLOR support" describe block that sets process.env['NO_COLOR'] = '' and asserts the color helper c('red', 'error text') returns plain text; specifically, create an it(...) case (e.g. "should treat NO_COLOR empty string as set") that sets process.env['NO_COLOR'] = '' then calls c('red', 'error text') and expects 'error text' (the afterEach cleanup will remove the env var).libs/cli/src/core/help.ts (1)
63-104: Align command visibility with Commander's helper methods for consistency.Line 64, Line 84, and Line 97 access
cmd.commandsandskillsCmd.commandsdirectly, while line 106 useshelper.visibleOptions(cmd). This inconsistency means if hidden commands are ever added (via.hidden), they would still appear in help output. Since Commander'shelper.visibleCommands(cmd)filters hidden commands just likehelper.visibleOptions(cmd)does, use it instead of raw.commandsarrays to stay aligned with Commander's visibility model and prevent future drift.Replace direct command array access with:
helper.visibleCommands(cmd)instead ofcmd.commandshelper.visibleCommands(skillsCmd)instead ofskillsCmd.commands- Track rendered command names to show any ungrouped visible commands under an "Other" section
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/core/help.ts` around lines 63 - 104, The help generator is using raw cmd.commands and skillsCmd.commands which can include hidden commands; replace those usages with helper.visibleCommands(cmd) and helper.visibleCommands(skillsCmd) so visibility matches commander’s helper.visibleOptions(cmd). Update the sections that build termWidth, the grouped commands loop (using GROUPS and formatCommandLine), and the Skills inline section (formatSkillsLine) to iterate over helper.visibleCommands(...) instead of .commands; additionally track the names of rendered visible commands and add an "Other" section that lists any remaining visible commands not covered by GROUPS to ensure nothing visible is omitted.
🤖 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/core/colors.ts`:
- Around line 13-18: The colorsEnabled function currently treats NO_COLOR as
truthy so an empty string doesn't disable colors; update colorsEnabled to treat
any presence of NO_COLOR (i.e., process.env['NO_COLOR'] !== undefined) as
disabling colors, leaving the existing FORCE_COLOR (fc handling) and isTTY
fallback unchanged; locate the colorsEnabled function and replace the truthy
check with an undefined-check to ensure NO_COLOR="" disables colors.
---
Outside diff comments:
In `@libs/cli/src/core/__tests__/help.spec.ts`:
- Around line 54-83: The test currently checks command presence with raw
substring matches which falsely passes when subcommands like "skills list"
include substrings "list"/"install"; update the test in "should list all
commands in help output" to perform line-aware checks: call getHelpOutput(),
split it into lines (e.g., .split(/\r?\n/)), normalize each line with trim(),
then for each entry in expectedCommands ensure there exists a normalized help
line that either equals the command or startsWith the command followed by
whitespace (to allow descriptions). Locate the expectedCommands array and
replace the for-loop that uses expect(help).toContain(cmd) with this line-based
existence check (you can extract a small helper inside the test to find the
matching line).
---
Nitpick comments:
In `@libs/cli/src/core/__tests__/colors.spec.ts`:
- Around line 100-142: Add a test in the existing "NO_COLOR support" describe
block that sets process.env['NO_COLOR'] = '' and asserts the color helper
c('red', 'error text') returns plain text; specifically, create an it(...) case
(e.g. "should treat NO_COLOR empty string as set") that sets
process.env['NO_COLOR'] = '' then calls c('red', 'error text') and expects
'error text' (the afterEach cleanup will remove the env var).
In `@libs/cli/src/core/help.ts`:
- Around line 63-104: The help generator is using raw cmd.commands and
skillsCmd.commands which can include hidden commands; replace those usages with
helper.visibleCommands(cmd) and helper.visibleCommands(skillsCmd) so visibility
matches commander’s helper.visibleOptions(cmd). Update the sections that build
termWidth, the grouped commands loop (using GROUPS and formatCommandLine), and
the Skills inline section (formatSkillsLine) to iterate over
helper.visibleCommands(...) instead of .commands; additionally track the names
of rendered visible commands and add an "Other" section that lists any remaining
visible commands not covered by GROUPS to ensure nothing visible is omitted.
🪄 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: 80648167-0ce1-43be-9d62-0149363ffcab
📒 Files selected for processing (5)
libs/cli/src/core/__tests__/colors.spec.tslibs/cli/src/core/__tests__/help.spec.tslibs/cli/src/core/colors.tslibs/cli/src/core/help.tslibs/cli/src/core/program.ts
…roved organization and clarity
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (7)
libs/skills/catalog/frontmcp-testing/SKILL.md (1)
52-57: Add new testing skills to the “Related skills” section for discoverability.You introduced dedicated routes (
test-auth,test-browser-build,test-cli-binary,test-direct-client,test-e2e-handler,test-tool-unit), but they are not reflected in the related-skills list. Please add them so navigation stays consistent.As per coding guidelines,
libs/**content should remain consistent with surrounding documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-testing/SKILL.md` around lines 52 - 57, The "Related skills" list in SKILL.md is missing entries for the new testing routes; update the Related skills section in libs/skills/catalog/frontmcp-testing/SKILL.md to include rows for `test-auth`, `test-browser-build`, `test-cli-binary`, `test-direct-client`, `test-e2e-handler`, and `test-tool-unit` matching the table format used for other skills (name column, backticked route, short description) so discoverability and consistency with the other entries is preserved.libs/cli/src/commands/skills/install.ts (1)
42-60: Consider adding error handling for file operations.The function has two potential issues:
- TOCTOU gap: Between
fileExists()andreadFile(), the file could theoretically be deleted, causingreadFileto throw.- Uncaught errors: Neither
readFilenorwriteFileis wrapped in try-catch, so permission errors or other I/O failures will propagate as unhandled exceptions with raw stack traces.For a CLI tool, graceful error messages improve UX.
♻️ Proposed fix with error handling
async function ensureClaudeMdSkillsInstructions(cwd: string): Promise<void> { const claudeMdPath = path.join(cwd, 'CLAUDE.md'); - if (await fileExists(claudeMdPath)) { - const content = await readFile(claudeMdPath); - if (content.includes(SKILLS_MARKER)) { - // Already has skills instructions — nothing to do - return; + try { + if (await fileExists(claudeMdPath)) { + const content = await readFile(claudeMdPath); + if (content.includes(SKILLS_MARKER)) { + // Already has skills instructions — nothing to do + return; + } + // Exists but missing skills section — prepend it + const updated = CLAUDE_MD_SKILLS_SECTION + '\n' + content; + await writeFile(claudeMdPath, updated); + console.log(`${c('green', '✓')} Updated ${c('cyan', 'CLAUDE.md')} with skills usage instructions`); + } else { + // Create new CLAUDE.md with skills section + await writeFile(claudeMdPath, CLAUDE_MD_SKILLS_SECTION); + console.log(`${c('green', '✓')} Created ${c('cyan', 'CLAUDE.md')} with skills usage instructions`); } - // Exists but missing skills section — prepend it - const updated = CLAUDE_MD_SKILLS_SECTION + '\n' + content; - await writeFile(claudeMdPath, updated); - console.log(`${c('green', '✓')} Updated ${c('cyan', 'CLAUDE.md')} with skills usage instructions`); - } else { - // Create new CLAUDE.md with skills section - await writeFile(claudeMdPath, CLAUDE_MD_SKILLS_SECTION); - console.log(`${c('green', '✓')} Created ${c('cyan', 'CLAUDE.md')} with skills usage instructions`); + } catch (err) { + console.error(c('yellow', `⚠ Could not update CLAUDE.md: ${(err as Error).message}`)); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/commands/skills/install.ts` around lines 42 - 60, In ensureClaudeMdSkillsInstructions, wrap the file operations (fileExists, readFile, writeFile) in a single try-catch and handle potential TOCTOU by attempting to read the file directly and treating ENOENT as "file missing" rather than relying on fileExists; specifically, use readFile inside the try, if it throws with code 'ENOENT' proceed to create CLAUDE.md with CLAUDE_MD_SKILLS_SECTION, otherwise on other read/write errors log a clear, user-friendly message (mentioning the operation and filepath) and return/throw a controlled error instead of letting raw stack traces surface; update the console messages to reflect success only after writeFile completes and reference SKILLS_MARKER when deciding whether to prepend.libs/skills/catalog/frontmcp-extensibility/SKILL.md (1)
35-38: Consider varying the phrasing for readability.The "Skip When" items all start with "You need to", which reads repetitively. Consider varying the sentence structure for better flow.
✏️ Suggested rephrasing
### Skip When -- You need to build core MCP components (see `frontmcp-development`) -- You need to configure auth, transport, or CORS (see `frontmcp-config`) -- You need to write a plugin with hooks and context extensions (see `create-plugin`) +- Building core MCP components (see `frontmcp-development`) +- Configuring auth, transport, or CORS (see `frontmcp-config`) +- Writing a plugin with hooks and context extensions (see `create-plugin`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-extensibility/SKILL.md` around lines 35 - 38, The three bullet lines in SKILL.md ("You need to build core MCP components", "You need to configure auth, transport, or CORS", "You need to write a plugin with hooks and context extensions") are repetitive; change them to varied phrasing for readability — e.g. make one an imperative ("Build core MCP components — see `frontmcp-development`"), another passive/descriptor ("Configure auth, transport, or CORS — see `frontmcp-config`"), and the third a gerund/continuation ("Writing a plugin with hooks and context extensions — see `create-plugin`"); preserve the links/tokens (`frontmcp-development`, `frontmcp-config`, `create-plugin`) and keep the three items parallel in intent but not identical in sentence start.libs/sdk/src/completion/flows/complete.flow.ts (1)
177-188: Improved type safety for resource argument completion.The direct call to
resourceMatch.instance.getArgumentCompleter(argName)is cleaner than the previousanycast approach and aligns with the typedResourceInstanceinterface. The null-check and error handling are correctly preserved.Consider applying the same pattern to prompt completion (lines 135-148) for consistency. Currently, prompts still use
const instance = prompt as anywhile resources now use the typed interface directly.♻️ Potential follow-up to align prompt completion
// Check if the prompt instance has a completer for this argument // Completion support is optional - prompts can implement getArgumentCompleter to provide suggestions - const instance = prompt as any; // PromptInstance may have completer method - if (typeof instance.getArgumentCompleter === 'function') { - const completer = instance.getArgumentCompleter(argName); + const completer = prompt.getArgumentCompleter(argName); + if (completer) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/sdk/src/completion/flows/complete.flow.ts` around lines 177 - 188, In the prompt completion block (where the code currently does "const instance = prompt as any" and calls its completer), replace the unsafe any cast by using the typed prompt instance and call prompt.getArgumentCompleter(argName) directly (mirroring the resource path); keep the existing null-check and try/catch around the completer invocation and assign result.values/total/hasMore to the same local variables (values, total, hasMore). Ensure the prompt type implements the same getArgumentCompleter signature (import/adjust the interface if needed) so the call is type-safe and consistent with the resource completion code that uses resourceMatch.instance.getArgumentCompleter.libs/cli/src/commands/skills/catalog.ts (1)
12-27: Avoid duplicating manifest contract types in CLI.
SkillReferenceEntry/SkillEntryhere now mirror manifest schema locally; this can drift fromlibs/skills/src/manifest.tsand silently desync parsing and search behavior.Consider importing a shared exported manifest type (or moving the shared interfaces to a common package) so catalog loading and manifest generation stay schema-locked.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/commands/skills/catalog.ts` around lines 12 - 27, The local SkillReferenceEntry and SkillEntry interfaces duplicate the manifest schema and should be replaced with the shared manifest types to avoid drift; remove the local declarations and import the exported types (e.g., ManifestSkill or SkillManifest/SkillReference from the shared manifest module in libs/skills or the common package) and update any references in this file (catalog loading/parsing and search logic) to use those imported types so CLI parsing remains schema-locked to the canonical manifest definitions.libs/cli/src/commands/skills/register.ts (1)
66-73: Rename the positional to<nameOrPath>in the CLI surface.
commanderwill still parse this, but--helpnow rendersskills read <name> [reference], which hides the newskill:filepathform.✏️ Suggested rename
- .argument('<name>', 'Skill name or skill:filepath (e.g., frontmcp-dev:references/create-tool.md)') + .argument('<nameOrPath>', 'Skill name or skill:filepath (e.g., frontmcp-dev:references/create-tool.md)') .argument('[reference]', 'Reference name to read (e.g., create-tool)') .option('--refs', 'List all available references for the skill') - .action(async (name: string, reference: string | undefined, options: { refs?: boolean }) => { + .action(async (nameOrPath: string, reference: string | undefined, options: { refs?: boolean }) => { const { readSkill } = await import('./read.js'); - await readSkill(name, { reference, listRefs: options.refs }); + await readSkill(nameOrPath, { reference, listRefs: options.refs }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/commands/skills/register.ts` around lines 66 - 73, Update the CLI positional argument name so help shows the new form: change the first .argument call in the command registration from '<name>' to '<nameOrPath>' (in the file where the command is built and .action imports readSkill), keeping the description and behavior the same so commander still passes the value to readSkill(nameOrPath, { reference, listRefs: options.refs }); this will make --help display "skills read <nameOrPath> [reference]" and surface the skill:filepath form.libs/cli/src/core/__tests__/help.spec.ts (1)
56-61: ANSI escape code stripping is correctly implemented.The
\x1bcontrol character in the regex is the standard ANSI escape sequence prefix. ESLint'sno-control-regexrule flags this as a false positive—this is the canonical pattern for stripping terminal color codes in test assertions. Consider adding an inline disable comment for clarity:- const stripAnsi = (s: string) => s.replace(/\x1b\[[0-9;]*m/g, ''); + // eslint-disable-next-line no-control-regex -- Intentional ANSI escape sequence matching + const stripAnsi = (s: string) => s.replace(/\x1b\[[0-9;]*m/g, '');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/core/__tests__/help.spec.ts` around lines 56 - 61, The regex /\x1b\[[0-9;]*m/g inside the stripAnsi function is intentionally using the control character \x1b to strip ANSI codes and should be marked as an exception to ESLint's no-control-regex; add an inline ESLint disable for that rule immediately above the regex (e.g., a single-line disable like eslint-disable-next-line no-control-regex) or wrap just that expression with a disable/enable pair so the stripAnsi function (and the pattern /\x1b\[[0-9;]*m/g) remains unchanged but ESLint no-control-regex is suppressed only for this line.
🤖 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/core/__tests__/colors.spec.ts`:
- Around line 45-53: Save the original environment values for FORCE_COLOR and
NO_COLOR (e.g., const originalForceColor and originalNoColor) before mutating
them, then in beforeEach set process.env.FORCE_COLOR = '1' as you currently do,
and in afterEach restore them by checking the saved originals: if an original
was undefined delete the key, otherwise set it back to the original value; apply
the same save/restore pattern to the other beforeEach/afterEach pair referenced
later (the one around lines 101-104).
In `@libs/sdk/src/common/entries/resource.entry.ts`:
- Around line 62-66: The new abstract getter ResourceEntry.providers is a
breaking API—restore backwards compatibility by removing the abstract
requirement: change providers to a non-abstract getter with a safe default
implementation (e.g., return an empty ProviderRegistry or a shared no-op
registry) or make it a protected/optional getter so existing subclasses don’t
have to implement it; update the ResourceEntry class (the providers getter) to
provide that default behavior and keep the public API stable.
In `@libs/sdk/src/common/entries/scope.entry.ts`:
- Around line 74-83: Rename the private field `_lifecycleCallbacks` to
`lifecycleCallbacks` (remove the underscore) and update all usages (e.g., in
`onServerStarted`) to push into `this.lifecycleCallbacks`; then add a public
getter method (e.g., `getLifecycleCallbacks(): Array<() => void |
Promise<void>>`) that returns `this.lifecycleCallbacks` so the entry class
follows the SDK convention of non-underscored private fields exposed via a
getter.
In `@libs/sdk/src/resource/flows/read-resource.flow.ts`:
- Around line 237-242: The current fallback uses a constant 'anonymous' for
sessionKey which causes CONTEXT-scoped providers cached by buildViews to be
shared across unauthenticated requests; change the session key logic in
read-resource.flow.ts to prefer this.state.sessionId when present (e.g.,
this.state.sessionId ?? ctx.authInfo?.sessionId) and, if neither exists,
generate a unique request-scoped key (e.g., a short random/UUID per request)
instead of the literal 'anonymous' before calling
resource.providers.buildViews(sessionKey, new Map(this.deps)); ensure you only
change how sessionKey is derived and pass the new key to buildViews so cached
providers are isolated per request.
In `@libs/sdk/src/resource/resource.instance.ts`:
- Around line 107-120: getArgumentCompleter currently invokes prototype-bound
completers with this === cls.prototype which prevents access to
ResourceContext/instance state; change the call so the completer runs with a
real resource instance or context instead of the raw prototype. Locate
getArgumentCompleter and the branch that uses this.record.provide and
cls.prototype.getArgumentCompleter; if the completer is intended to access
DI/providers or this.params create/obtain a proper ResourceContext or
instantiate the resource (or resolve it from DI) and call its
getArgumentCompleter with that instance, otherwise enforce/validate that the
method is static/pure and return null or throw a clear error directing callers
to implement an instance-level override.
In `@libs/sdk/src/skill/skill.instance.ts`:
- Around line 126-151: Replace manual '/' parsing/concatenation in getBaseDir
and load with the path utilities from `@frontmcp/utils`: use the provided
isAbsolute/dirname/join/normalize functions instead of lastIndexOf('/') and
string interpolation. In getBaseDir(), derive the directory for SkillKind.FILE
via dirname(this.record.filePath) and return callerDir for SkillKind.VALUE
unchanged. In load(), compute refsPath only when metadata.resources?.references
exists and resolveReferences for both absolute and relative paths by checking
isAbsolute(refsPath) — if absolute use normalize/refPath directly, otherwise
join(baseDir, refsPath); ensure you still call resolveReferences(refsDir) and
handle the case where baseDir is undefined by allowing resolution of absolute
refs even when callerDir is missing. Use the symbols getBaseDir, load,
SkillKind, this.record.filePath, this.record.callerDir,
metadata.resources?.references, and resolveReferences to locate places to
change.
In `@libs/skills/catalog/frontmcp-development/references/create-plugin.md`:
- Around line 428-448: The fenced folder-structure code block starting with the
"plugins/" tree (containing entries like "my-plugin/", "index.ts",
"my-plugin.plugin.ts", etc.) is missing a language specifier; update the opening
fence from ``` to ```text so the block reads as a text code block (e.g.,
```text) to satisfy markdownlint and enable proper syntax highlighting.
In `@libs/skills/catalog/frontmcp-development/references/decorators-guide.md`:
- Line 168: Update the documentation row for `authProviders?` so the provider
name uses correct branding: change the lowercase example `'github'` to
`'GitHub'` (and any descriptive text that references github) in the table cell
that currently reads "`Per-tool auth providers: ['github'] or [{ name, scopes,
alias }]`", ensuring the example and description consistently use "GitHub".
In
`@libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md`:
- Line 23: Update the product name casing in the checklist item that currently
reads "`serve: false` is set in the FrontMcp config (or uses direct API)`" to
use "FrontMCP" so it matches the title; change "FrontMcp" -> "FrontMCP" and scan
for any other occurrences of "FrontMcp" in this document to make them consistent
as well.
- Around line 12-18: The checklist currently only references a generic dispose()
but the example shows both client.close() and server.dispose(); update the API
Surface section to explicitly document the client cleanup method by adding an
item for client.close() (or clearly state that dispose() refers to
server.dispose()), and clarify which symbol is responsible for cleaning
connections/resources (e.g., client.close() vs server.dispose()); reference the
exported create(), connect() (which returns the typed client with
listTools/callTool), and ensure dispose()/close() semantics are unambiguous in
the checklist.
In `@libs/skills/catalog/frontmcp-testing/SKILL.md`:
- Around line 50-53: The SKILL.md contains duplicate routing entries for "Test
authenticated endpoints" mapped to two routes (`setup-testing` +
`configure-auth` and `test-auth`); pick one canonical route and remove or mark
the other as legacy/alternate to avoid ambiguity—update the table row so only a
single entry remains (either keep the composite `setup-testing` +
`configure-auth` or keep `test-auth`) and, if you must retain the second, label
it clearly as "legacy" or "alternate" in the Description column to preserve
correctness and consistency.
---
Nitpick comments:
In `@libs/cli/src/commands/skills/catalog.ts`:
- Around line 12-27: The local SkillReferenceEntry and SkillEntry interfaces
duplicate the manifest schema and should be replaced with the shared manifest
types to avoid drift; remove the local declarations and import the exported
types (e.g., ManifestSkill or SkillManifest/SkillReference from the shared
manifest module in libs/skills or the common package) and update any references
in this file (catalog loading/parsing and search logic) to use those imported
types so CLI parsing remains schema-locked to the canonical manifest
definitions.
In `@libs/cli/src/commands/skills/install.ts`:
- Around line 42-60: In ensureClaudeMdSkillsInstructions, wrap the file
operations (fileExists, readFile, writeFile) in a single try-catch and handle
potential TOCTOU by attempting to read the file directly and treating ENOENT as
"file missing" rather than relying on fileExists; specifically, use readFile
inside the try, if it throws with code 'ENOENT' proceed to create CLAUDE.md with
CLAUDE_MD_SKILLS_SECTION, otherwise on other read/write errors log a clear,
user-friendly message (mentioning the operation and filepath) and return/throw a
controlled error instead of letting raw stack traces surface; update the console
messages to reflect success only after writeFile completes and reference
SKILLS_MARKER when deciding whether to prepend.
In `@libs/cli/src/commands/skills/register.ts`:
- Around line 66-73: Update the CLI positional argument name so help shows the
new form: change the first .argument call in the command registration from
'<name>' to '<nameOrPath>' (in the file where the command is built and .action
imports readSkill), keeping the description and behavior the same so commander
still passes the value to readSkill(nameOrPath, { reference, listRefs:
options.refs }); this will make --help display "skills read <nameOrPath>
[reference]" and surface the skill:filepath form.
In `@libs/cli/src/core/__tests__/help.spec.ts`:
- Around line 56-61: The regex /\x1b\[[0-9;]*m/g inside the stripAnsi function
is intentionally using the control character \x1b to strip ANSI codes and should
be marked as an exception to ESLint's no-control-regex; add an inline ESLint
disable for that rule immediately above the regex (e.g., a single-line disable
like eslint-disable-next-line no-control-regex) or wrap just that expression
with a disable/enable pair so the stripAnsi function (and the pattern
/\x1b\[[0-9;]*m/g) remains unchanged but ESLint no-control-regex is suppressed
only for this line.
In `@libs/sdk/src/completion/flows/complete.flow.ts`:
- Around line 177-188: In the prompt completion block (where the code currently
does "const instance = prompt as any" and calls its completer), replace the
unsafe any cast by using the typed prompt instance and call
prompt.getArgumentCompleter(argName) directly (mirroring the resource path);
keep the existing null-check and try/catch around the completer invocation and
assign result.values/total/hasMore to the same local variables (values, total,
hasMore). Ensure the prompt type implements the same getArgumentCompleter
signature (import/adjust the interface if needed) so the call is type-safe and
consistent with the resource completion code that uses
resourceMatch.instance.getArgumentCompleter.
In `@libs/skills/catalog/frontmcp-extensibility/SKILL.md`:
- Around line 35-38: The three bullet lines in SKILL.md ("You need to build core
MCP components", "You need to configure auth, transport, or CORS", "You need to
write a plugin with hooks and context extensions") are repetitive; change them
to varied phrasing for readability — e.g. make one an imperative ("Build core
MCP components — see `frontmcp-development`"), another passive/descriptor
("Configure auth, transport, or CORS — see `frontmcp-config`"), and the third a
gerund/continuation ("Writing a plugin with hooks and context extensions — see
`create-plugin`"); preserve the links/tokens (`frontmcp-development`,
`frontmcp-config`, `create-plugin`) and keep the three items parallel in intent
but not identical in sentence start.
In `@libs/skills/catalog/frontmcp-testing/SKILL.md`:
- Around line 52-57: The "Related skills" list in SKILL.md is missing entries
for the new testing routes; update the Related skills section in
libs/skills/catalog/frontmcp-testing/SKILL.md to include rows for `test-auth`,
`test-browser-build`, `test-cli-binary`, `test-direct-client`,
`test-e2e-handler`, and `test-tool-unit` matching the table format used for
other skills (name column, backticked route, short description) so
discoverability and consistency with the other entries is preserved.
🪄 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: cfed63ff-9563-4dba-8421-443e53f73944
📒 Files selected for processing (99)
libs/cli/src/commands/skills/catalog.tslibs/cli/src/commands/skills/install.tslibs/cli/src/commands/skills/read.tslibs/cli/src/commands/skills/register.tslibs/cli/src/commands/skills/search.tslibs/cli/src/commands/skills/show.tslibs/cli/src/core/__tests__/colors.spec.tslibs/cli/src/core/__tests__/help.spec.tslibs/cli/src/core/colors.tslibs/cli/src/core/help.tslibs/sdk/src/common/entries/resource.entry.tslibs/sdk/src/common/entries/scope.entry.tslibs/sdk/src/common/interfaces/resource.interface.tslibs/sdk/src/common/interfaces/skill.interface.tslibs/sdk/src/completion/flows/complete.flow.tslibs/sdk/src/front-mcp/front-mcp.tslibs/sdk/src/provider/provider.registry.tslibs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/resource/resource.instance.tslibs/sdk/src/skill/skill.instance.tslibs/sdk/src/skill/skill.utils.tslibs/skills/__tests__/manifest.spec.tslibs/skills/catalog/frontmcp-config/SKILL.mdlibs/skills/catalog/frontmcp-config/references/configure-auth-modes.mdlibs/skills/catalog/frontmcp-config/references/configure-auth.mdlibs/skills/catalog/frontmcp-config/references/configure-elicitation.mdlibs/skills/catalog/frontmcp-config/references/configure-http.mdlibs/skills/catalog/frontmcp-config/references/configure-session.mdlibs/skills/catalog/frontmcp-config/references/configure-throttle-guard-config.mdlibs/skills/catalog/frontmcp-config/references/configure-throttle.mdlibs/skills/catalog/frontmcp-config/references/configure-transport-protocol-presets.mdlibs/skills/catalog/frontmcp-config/references/configure-transport.mdlibs/skills/catalog/frontmcp-config/references/setup-redis.mdlibs/skills/catalog/frontmcp-config/references/setup-sqlite.mdlibs/skills/catalog/frontmcp-deployment/SKILL.mdlibs/skills/catalog/frontmcp-deployment/references/build-for-browser.mdlibs/skills/catalog/frontmcp-deployment/references/build-for-cli.mdlibs/skills/catalog/frontmcp-deployment/references/build-for-sdk.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-lambda.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-node-dockerfile.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-node.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-vercel-config.mdlibs/skills/catalog/frontmcp-deployment/references/deploy-to-vercel.mdlibs/skills/catalog/frontmcp-development/SKILL.mdlibs/skills/catalog/frontmcp-development/references/create-adapter.mdlibs/skills/catalog/frontmcp-development/references/create-agent-llm-config.mdlibs/skills/catalog/frontmcp-development/references/create-agent.mdlibs/skills/catalog/frontmcp-development/references/create-job.mdlibs/skills/catalog/frontmcp-development/references/create-plugin-hooks.mdlibs/skills/catalog/frontmcp-development/references/create-plugin.mdlibs/skills/catalog/frontmcp-development/references/create-prompt.mdlibs/skills/catalog/frontmcp-development/references/create-provider.mdlibs/skills/catalog/frontmcp-development/references/create-resource.mdlibs/skills/catalog/frontmcp-development/references/create-skill-with-tools.mdlibs/skills/catalog/frontmcp-development/references/create-skill.mdlibs/skills/catalog/frontmcp-development/references/create-tool-annotations.mdlibs/skills/catalog/frontmcp-development/references/create-tool-output-schema-types.mdlibs/skills/catalog/frontmcp-development/references/create-tool.mdlibs/skills/catalog/frontmcp-development/references/create-workflow.mdlibs/skills/catalog/frontmcp-development/references/decorators-guide.mdlibs/skills/catalog/frontmcp-development/references/official-adapters.mdlibs/skills/catalog/frontmcp-development/references/official-plugins.mdlibs/skills/catalog/frontmcp-extensibility/SKILL.mdlibs/skills/catalog/frontmcp-extensibility/references/vectoriadb.mdlibs/skills/catalog/frontmcp-guides/references/example-knowledge-base.mdlibs/skills/catalog/frontmcp-guides/references/example-task-manager.mdlibs/skills/catalog/frontmcp-guides/references/example-weather-api.mdlibs/skills/catalog/frontmcp-production-readiness/SKILL.mdlibs/skills/catalog/frontmcp-production-readiness/references/common-checklist.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-browser.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-cli-binary.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-cli-daemon.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-cloudflare.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-lambda.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-node-server.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-vercel.mdlibs/skills/catalog/frontmcp-setup/SKILL.mdlibs/skills/catalog/frontmcp-setup/references/frontmcp-skills-usage.mdlibs/skills/catalog/frontmcp-setup/references/multi-app-composition.mdlibs/skills/catalog/frontmcp-setup/references/nx-workflow.mdlibs/skills/catalog/frontmcp-setup/references/project-structure-nx.mdlibs/skills/catalog/frontmcp-setup/references/project-structure-standalone.mdlibs/skills/catalog/frontmcp-setup/references/readme-guide.mdlibs/skills/catalog/frontmcp-setup/references/setup-project.mdlibs/skills/catalog/frontmcp-setup/references/setup-redis.mdlibs/skills/catalog/frontmcp-setup/references/setup-sqlite.mdlibs/skills/catalog/frontmcp-testing/SKILL.mdlibs/skills/catalog/frontmcp-testing/references/setup-testing.mdlibs/skills/catalog/frontmcp-testing/references/test-auth.mdlibs/skills/catalog/frontmcp-testing/references/test-browser-build.mdlibs/skills/catalog/frontmcp-testing/references/test-cli-binary.mdlibs/skills/catalog/frontmcp-testing/references/test-direct-client.mdlibs/skills/catalog/frontmcp-testing/references/test-e2e-handler.mdlibs/skills/catalog/frontmcp-testing/references/test-tool-unit.mdlibs/skills/catalog/skills-manifest.jsonlibs/skills/scripts/generate-manifest.mjslibs/skills/src/manifest.ts
💤 Files with no reviewable changes (1)
- libs/cli/src/commands/skills/show.ts
✅ Files skipped from review due to trivial changes (64)
- libs/skills/catalog/frontmcp-deployment/references/build-for-browser.md
- libs/skills/catalog/frontmcp-config/references/configure-throttle.md
- libs/skills/catalog/frontmcp-guides/references/example-weather-api.md
- libs/skills/catalog/frontmcp-setup/references/nx-workflow.md
- libs/skills/catalog/frontmcp-deployment/references/build-for-sdk.md
- libs/skills/catalog/frontmcp-config/references/configure-elicitation.md
- libs/skills/catalog/frontmcp-guides/references/example-task-manager.md
- libs/skills/catalog/frontmcp-testing/references/test-e2e-handler.md
- libs/skills/catalog/frontmcp-development/references/create-provider.md
- libs/skills/catalog/frontmcp-setup/references/setup-redis.md
- libs/skills/catalog/frontmcp-config/references/configure-transport-protocol-presets.md
- libs/cli/src/commands/skills/search.ts
- libs/skills/catalog/frontmcp-development/references/create-adapter.md
- libs/skills/catalog/frontmcp-testing/references/test-direct-client.md
- libs/skills/catalog/frontmcp-testing/references/test-cli-binary.md
- libs/skills/catalog/frontmcp-testing/references/test-tool-unit.md
- libs/skills/catalog/frontmcp-testing/references/setup-testing.md
- libs/skills/catalog/frontmcp-config/references/setup-sqlite.md
- libs/skills/catalog/frontmcp-development/references/create-tool-output-schema-types.md
- libs/skills/catalog/frontmcp-setup/references/project-structure-nx.md
- libs/skills/catalog/frontmcp-setup/references/project-structure-standalone.md
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-vercel-config.md
- libs/skills/catalog/frontmcp-config/references/configure-auth-modes.md
- libs/skills/catalog/frontmcp-config/references/configure-http.md
- libs/skills/catalog/frontmcp-testing/references/test-auth.md
- libs/skills/catalog/frontmcp-setup/references/setup-project.md
- libs/skills/catalog/frontmcp-development/references/official-plugins.md
- libs/skills/catalog/frontmcp-guides/references/example-knowledge-base.md
- libs/skills/catalog/frontmcp-setup/references/multi-app-composition.md
- libs/skills/catalog/frontmcp-testing/references/test-browser-build.md
- libs/skills/catalog/frontmcp-config/references/configure-auth.md
- libs/skills/catalog/frontmcp-config/references/configure-throttle-guard-config.md
- libs/skills/tests/manifest.spec.ts
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-node-dockerfile.md
- libs/skills/catalog/frontmcp-config/references/setup-redis.md
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-node.md
- libs/skills/catalog/frontmcp-development/references/official-adapters.md
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-lambda.md
- libs/skills/catalog/frontmcp-setup/references/setup-sqlite.md
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-vercel.md
- libs/skills/catalog/frontmcp-config/references/configure-session.md
- libs/skills/catalog/frontmcp-config/references/configure-transport.md
- libs/skills/catalog/frontmcp-development/references/create-prompt.md
- libs/skills/catalog/frontmcp-development/references/create-agent-llm-config.md
- libs/skills/catalog/frontmcp-development/references/create-agent.md
- libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md
- libs/skills/catalog/frontmcp-development/references/create-workflow.md
- libs/skills/catalog/frontmcp-production-readiness/references/common-checklist.md
- libs/skills/catalog/frontmcp-development/references/create-tool-annotations.md
- libs/skills/catalog/frontmcp-development/references/create-plugin-hooks.md
- libs/skills/catalog/frontmcp-setup/references/frontmcp-skills-usage.md
- libs/skills/catalog/frontmcp-development/references/create-skill.md
- libs/skills/catalog/frontmcp-development/references/create-resource.md
- libs/skills/catalog/frontmcp-production-readiness/references/production-browser.md
- libs/skills/catalog/frontmcp-setup/references/readme-guide.md
- libs/skills/catalog/frontmcp-development/references/create-tool.md
- libs/skills/catalog/frontmcp-production-readiness/references/production-cloudflare.md
- libs/skills/catalog/frontmcp-production-readiness/references/production-cli-daemon.md
- libs/skills/catalog/frontmcp-production-readiness/references/production-cli-binary.md
- libs/skills/catalog/frontmcp-production-readiness/SKILL.md
- libs/skills/catalog/frontmcp-production-readiness/references/production-node-server.md
- libs/skills/catalog/frontmcp-production-readiness/references/production-vercel.md
- libs/skills/catalog/frontmcp-production-readiness/references/production-lambda.md
- libs/skills/catalog/frontmcp-development/references/create-job.md
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/cli/src/core/help.ts
libs/skills/catalog/frontmcp-development/references/decorators-guide.md
Outdated
Show resolved
Hide resolved
libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md
Outdated
Show resolved
Hide resolved
libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md
Outdated
Show resolved
Hide resolved
…g variable names for clarity
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/skills/catalog/frontmcp-development/references/decorators-guide.md (1)
524-591:⚠️ Potential issue | 🔴 CriticalFix documentation for
@Joband@Workflowdecorators — several field names and completeness issues found.The
@Jobtable documents aschedule?field (cron expressions) that does not exist in theJobMetadatainterface. Remove this field from the documentation.The
@Workflowtable documentswebhookConfig?but the actual interface field iswebhook?— update the field name. Additionally, the@Workflowtable is incomplete and missing these fields that exist inWorkflowMetadata:
maxConcurrency?— Maximum parallel step concurrency (Default: 5)tags?— Categorization tagslabels?— Key-value labelshideFromDiscovery?— Hide from workflow listingpermissions?— Access control rulesinputSchema?— Workflow-level input schemaoutputSchema?— Workflow-level output schemaThe
@WorkflowSteptable and all documented@Jobfields (exceptschedule) are correct.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-development/references/decorators-guide.md` around lines 524 - 591, Update the docs so the `@Job` table matches JobMetadata by removing the non-existent "schedule?" field, and update the `@Workflow` table to use the actual field name "webhook?" (not "webhookConfig?") and add the missing WorkflowMetadata fields: "maxConcurrency? (Maximum parallel step concurrency, Default: 5)", "tags?", "labels?", "hideFromDiscovery?", "permissions?", "inputSchema?", and "outputSchema?"; keep the `@WorkflowStep` table and all other `@Job` fields unchanged.
🧹 Nitpick comments (3)
libs/cli/src/commands/skills/catalog.ts (2)
293-306: Well-implemented reference indexing.The logic correctly guards against undefined
references, and the tokenization/filtering approach mirrors the existing patterns for skill name and description processing. The 1x weighting for references is consistent with the documented approach.Minor nit:
namePartson line 296 shadows the outer-scope variable on line 270. Consider renaming torefNamePartsfor clarity during maintenance.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/commands/skills/catalog.ts` around lines 293 - 306, The variable name nameParts declared inside the references loop in the skills catalog indexing (the for (const ref of skill.references) block) shadows the outer nameParts used earlier for skill names; rename the inner variable to refNameParts (and update all uses inside that block, e.g., where you push parts.push(...refNameParts)) to avoid shadowing and improve clarity while preserving the same splitting and description-tokenizing logic.
12-15: Consider importingSkillReferenceEntryfrom@frontmcp/skillsto reduce duplication.This interface mirrors the canonical definition in
libs/skills/src/manifest.ts. While local definitions work for CLI decoupling, importing from the source would prevent potential drift if the manifest schema evolves.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/commands/skills/catalog.ts` around lines 12 - 15, This file defines a local SkillReferenceEntry interface that duplicates the canonical type; remove the local declaration and import the canonical type from the shared package (use import { SkillReferenceEntry } from "@frontmcp/skills") and update any references/usages in this module to use the imported type; ensure the package exports that symbol and adjust any local names if needed so the code compiles.libs/cli/src/core/__tests__/help.spec.ts (1)
89-91: Consider clearer failure messages for command checks.When
hasCommandLine(cmd)returnsfalse, the failure message will be generic ("Expected false to be true"). A custom matcher orexpect.extendcould provide clearer diagnostics, but a simpler approach is to use a descriptive error string.♻️ Optional improvement for better failure messages
for (const cmd of expectedCommands) { - expect(hasCommandLine(cmd)).toBe(true); + if (!hasCommandLine(cmd)) { + throw new Error(`Expected command "${cmd}" in help output`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/core/__tests__/help.spec.ts` around lines 89 - 91, Replace the generic assertions in the loop that checks expectedCommands so failures show which command is missing: iterate over expectedCommands and for each cmd use hasCommandLine(cmd) and throw a descriptive Error (e.g., throw new Error(`Help output missing command '${cmd}'`)) or call fail with that message when hasCommandLine(cmd) is false; update the loop containing expectedCommands and hasCommandLine to perform this explicit check so test failures include the specific command 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/core/__tests__/help.spec.ts`:
- Around line 38-44: The test in help.spec.ts checks ordering but doesn't assert
that "socket" actually exists; update the test that uses getHelpOutput(), pmIdx
and socketIdx to include an explicit assertion that socketIdx is found (e.g.,
expect(socketIdx).toBeGreaterThan(-1) or expect(socketIdx).not.toBe(-1)) before
the existing expect(socketIdx).toBeGreaterThan(pmIdx) so failures clearly report
a missing "socket" entry.
In `@libs/skills/catalog/frontmcp-development/references/create-plugin.md`:
- Around line 81-121: The Quick Start mixes static provider registration and
dynamic provider creation; remove the redundant static providers array from the
`@Plugin` decorator on GreeterPlugin and instead register GreeterService only via
the static override dynamicProviders(opts: { prefix: string }) method so
configuration flows through dynamicProviders; update the `@Plugin`(...) call to
omit providers/exports (leave name only or exports if desired) and ensure
GreeterPlugin.init({ prefix: '...' }) remains the single source of provider
instantiation for GreeterService.
In `@libs/skills/catalog/frontmcp-development/references/decorators-guide.md`:
- Around line 541-558: The example for the `@Job` decorator uses raw Zod shapes
for inputSchema and outputSchema which is inconsistent with importing z from
'zod' and may be mistaken for needing full Zod schemas; update the example on
the SyncDataJob (class name) to use full Zod objects for both inputSchema and
outputSchema (e.g., inputSchema: z.object({ source: z.string().describe('Data
source to sync') }) and outputSchema: z.object({ synced: z.number() })) so it
matches the z import and removes ambiguity about whether Jobs accept raw shapes
or Zod schemas.
In
`@libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md`:
- Around line 49-51: Add client.close() to the integration-test cleanup flow so
tests perform create() → connect() → callTool() → dispose() and then
client.close(); update any test teardown/shared cleanup helpers to invoke
client.close() (in addition to dispose()) to match the public API and example
usage and ensure connection cleanup; reference the test flow helpers that call
create()/connect()/dispose() and modify them to call client.close() (or add a
new closeClient helper) so every integration test runs client.close() in its
afterEach/teardown.
---
Outside diff comments:
In `@libs/skills/catalog/frontmcp-development/references/decorators-guide.md`:
- Around line 524-591: Update the docs so the `@Job` table matches JobMetadata by
removing the non-existent "schedule?" field, and update the `@Workflow` table to
use the actual field name "webhook?" (not "webhookConfig?") and add the missing
WorkflowMetadata fields: "maxConcurrency? (Maximum parallel step concurrency,
Default: 5)", "tags?", "labels?", "hideFromDiscovery?", "permissions?",
"inputSchema?", and "outputSchema?"; keep the `@WorkflowStep` table and all other
`@Job` fields unchanged.
---
Nitpick comments:
In `@libs/cli/src/commands/skills/catalog.ts`:
- Around line 293-306: The variable name nameParts declared inside the
references loop in the skills catalog indexing (the for (const ref of
skill.references) block) shadows the outer nameParts used earlier for skill
names; rename the inner variable to refNameParts (and update all uses inside
that block, e.g., where you push parts.push(...refNameParts)) to avoid shadowing
and improve clarity while preserving the same splitting and
description-tokenizing logic.
- Around line 12-15: This file defines a local SkillReferenceEntry interface
that duplicates the canonical type; remove the local declaration and import the
canonical type from the shared package (use import { SkillReferenceEntry } from
"@frontmcp/skills") and update any references/usages in this module to use the
imported type; ensure the package exports that symbol and adjust any local names
if needed so the code compiles.
In `@libs/cli/src/core/__tests__/help.spec.ts`:
- Around line 89-91: Replace the generic assertions in the loop that checks
expectedCommands so failures show which command is missing: iterate over
expectedCommands and for each cmd use hasCommandLine(cmd) and throw a
descriptive Error (e.g., throw new Error(`Help output missing command
'${cmd}'`)) or call fail with that message when hasCommandLine(cmd) is false;
update the loop containing expectedCommands and hasCommandLine to perform this
explicit check so test failures include the specific command 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: 1379f83f-fd3b-41e7-87e6-bc5cd68a09ab
📒 Files selected for processing (11)
libs/cli/src/commands/skills/catalog.tslibs/cli/src/commands/skills/register.tslibs/cli/src/core/__tests__/colors.spec.tslibs/cli/src/core/__tests__/help.spec.tslibs/sdk/src/common/entries/scope.entry.tslibs/sdk/src/resource/flows/read-resource.flow.tslibs/sdk/src/skill/skill.instance.tslibs/skills/catalog/frontmcp-development/references/create-plugin.mdlibs/skills/catalog/frontmcp-development/references/decorators-guide.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.mdlibs/skills/catalog/frontmcp-testing/SKILL.md
✅ Files skipped from review due to trivial changes (2)
- libs/cli/src/core/tests/colors.spec.ts
- libs/sdk/src/skill/skill.instance.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- libs/sdk/src/common/entries/scope.entry.ts
- libs/cli/src/commands/skills/register.ts
- libs/skills/catalog/frontmcp-testing/SKILL.md
libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md
Outdated
Show resolved
Hide resolved
…g variable names for clarity
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
libs/cli/src/core/__tests__/help.spec.ts (1)
47-53: Optional: make inline skills assertions ANSI-normalized too.These checks currently use raw
toContain(...), while Line 57+ already normalizes ANSI and whitespace. Reusing that normalized path here would make this block equally resilient to formatting/styling changes.♻️ Suggested refactor
it('should show skills subcommands inline', () => { const help = getHelpOutput(); - expect(help).toContain('skills search'); - expect(help).toContain('skills list'); - expect(help).toContain('skills install'); - expect(help).toContain('skills read'); + // eslint-disable-next-line no-control-regex + const stripAnsi = (s: string) => s.replace(/\x1b\[[0-9;]*m/g, ''); + const helpLines = help.split(/\r?\n/).map((l) => stripAnsi(l).trim()); + const hasCommandLine = (cmd: string) => helpLines.some((line) => line === cmd || line.startsWith(cmd + ' ')); + + expect(hasCommandLine('skills search')).toBe(true); + expect(hasCommandLine('skills list')).toBe(true); + expect(hasCommandLine('skills install')).toBe(true); + expect(hasCommandLine('skills read')).toBe(true); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/cli/src/core/__tests__/help.spec.ts` around lines 47 - 53, The test uses raw toContain checks against getHelpOutput() which can fail with ANSI/whitespace differences; call normalizeAnsi(getHelpOutput()) (the same normalizer used later in the file) and assert against that normalized string (e.g., check normalizedHelp.includes('skills search'), 'skills list', 'skills install', 'skills read') so the assertions become resilient to styling/formatting changes; update the block that currently references getHelpOutput() to first create normalizedHelp = normalizeAnsi(getHelpOutput()) and use normalizedHelp.includes(...) for each skill check.libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md (4)
31-31: Specify both client and server cleanup methods.This checklist item only mentions
dispose(), but the API Surface section (line 17) and example usage (lines 64-65) show that proper cleanup requires bothclient.close()andserver.dispose(). The checklist should explicitly mention both to match the documented API.📝 Proposed clarification
-- [ ] `dispose()` is called in the host app's shutdown handler +- [ ] Both `client.close()` and `server.dispose()` are called in the host app's shutdown handler🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md` at line 31, Update the checklist item that currently says "`dispose()` is called in the host app's shutdown handler" to explicitly require both the client and server cleanup methods by mentioning "ensure client.close() is called and server.dispose() is called in the host app's shutdown handler" (matching the API Surface and example usage which reference client.close() and server.dispose()).
15-15: Clarify thatconnect()is a server method, not a standalone function.The current wording "
connect()returns a typed client" could be misinterpreted as a standalone exported function. The example usage (lines 58-59) shows thatcreate()returns a server object, andconnect()is a method on that server.📝 Proposed clarification
-- [ ] `connect()` returns a typed client with `listTools`, `callTool`, etc. +- [ ] Server provides `connect()` method that returns a typed client with `listTools`, `callTool`, etc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md` at line 15, The docs currently imply connect() is a standalone function; update the wording to clarify connect() is a method on the server object returned by create() (e.g., "the server's connect() method returns a typed client..."), and adjust the example and any references so they call connect as a method on the server instance (referencing create() and the server object) rather than as a top-level function.
19-19: Consider documenting theexportsfield for modern package resolution.Modern npm packages increasingly use the
exportsfield to control module resolution, especially for dual ESM/CJS packages. Since this is an SDK that will be embedded in various Node.js environments, documenting theexportsfield as a checklist item could improve package compatibility.📦 Optional enhancement
-- [ ] `package.json` has correct `main`, `module`, `types` fields +- [ ] `package.json` has correct `main`, `module`, `types`, and `exports` fields🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md` at line 19, Update the checklist line that currently reads "`package.json` has correct `main`, `module`, `types` fields" to also include an `exports` entry; specifically add a new checkbox like "`package.json` has correct `main`, `module`, `types`, and `exports` fields" and include a short sub-note to verify the `exports` field covers CJS and ESM entry points (or maps conditional exports) to ensure compatibility for dual ESM/CJS SDK consumers.
17-17: Consider stronger wording for cleanup guarantee.The static analysis tool suggests that "properly" is weaker than alternatives. Consider "thoroughly" or "completely" to strengthen the cleanup requirement, or rephrase to be more direct.
📝 Suggested alternatives
-- [ ] Server `dispose()` and client `close()` properly clean up all resources (connections, timers, listeners) +- [ ] Server `dispose()` and client `close()` completely clean up all resources (connections, timers, listeners)Or:
-- [ ] Server `dispose()` and client `close()` properly clean up all resources (connections, timers, listeners) +- [ ] Server `dispose()` and client `close()` clean up all resources (connections, timers, listeners)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md` at line 17, Replace the weak wording "properly" with a stronger, explicit guarantee: update the checklist item for server.dispose() and client.close() to read something like "Server dispose() and client close() must completely release all resources (connections, timers, listeners)" or "Server dispose() and client close() must thoroughly clean up and release all resources (connections, timers, listeners)"; reference the exact symbols dispose() and close() so reviewers know to verify those methods perform the complete resource teardown.apps/e2e/demo-e2e-resource-providers/jest.e2e.config.ts (1)
40-41: Prefer spreading preset before explicit overrides for deterministic precedence.
coverageDirectoryis currently set before...e2eCoveragePreset; if the preset also definescoverageDirectory, your local value is silently overridden.Suggested diff
- coverageDirectory: '../../../coverage/e2e/demo-e2e-resource-providers', - ...e2eCoveragePreset, + ...e2eCoveragePreset, + coverageDirectory: '../../../coverage/e2e/demo-e2e-resource-providers',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/demo-e2e-resource-providers/jest.e2e.config.ts` around lines 40 - 41, The config spreads e2eCoveragePreset after setting coverageDirectory so the preset can overwrite your explicit value; change the order so ...e2eCoveragePreset is spread first and then your explicit settings (e.g., coverageDirectory) come after to ensure your overrides take precedence—update the object where coverageDirectory and ...e2eCoveragePreset are combined so e2eCoveragePreset appears before coverageDirectory.apps/e2e/demo-e2e-resource-providers/src/plugins/counter/counter.plugin.ts (1)
6-6: UnusedCOUNTER_TOKENexport.
COUNTER_TOKENis defined but never used — the plugin usesCounterServiceclass directly as the DI token in bothprovidersandcontextExtensions. Consider removing the unused token or using it consistently if external code needs to reference the token explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/demo-e2e-resource-providers/src/plugins/counter/counter.plugin.ts` at line 6, The exported symbol COUNTER_TOKEN is declared but never used; either remove the export or switch the plugin to use that token consistently. If you choose removal: delete the COUNTER_TOKEN declaration and any unused imports so the plugin uses the CounterService class directly in providers and contextExtensions. If you prefer using the token: replace occurrences of CounterService in the providers array and contextExtensions mapping with COUNTER_TOKEN and ensure CounterService is provided under that token (e.g., { provide: COUNTER_TOKEN, useClass: CounterService }) so external code can import COUNTER_TOKEN as the DI key.apps/e2e/demo-e2e-resource-providers/e2e/resource-providers.e2e.spec.ts (1)
16-32: Consider adding type guards for safer extraction.The helper functions use
astype assertions on unknown response structures. While acceptable in test utilities, the error messages could be more descriptive when extraction fails.🔧 Optional: More descriptive error messages
function extractResourceJson<T>(result: unknown): T { const raw = result as { raw?: { contents?: Array<{ text?: string }> } }; const text = raw?.raw?.contents?.[0]?.text; - if (!text) throw new Error('No text content in resource result'); + if (!text) throw new Error(`No text content in resource result: ${JSON.stringify(result)}`); return JSON.parse(text) as T; } function extractToolJson<T>(result: unknown): T { const raw = result as { raw?: { structuredContent?: T; content?: Array<{ text?: string }> } }; if (raw?.raw?.structuredContent) return raw.raw.structuredContent; const text = raw?.raw?.content?.[0]?.text; - if (!text) throw new Error('No content in tool result'); + if (!text) throw new Error(`No content in tool result: ${JSON.stringify(result)}`); return JSON.parse(text) as T; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/demo-e2e-resource-providers/e2e/resource-providers.e2e.spec.ts` around lines 16 - 32, The helpers extractResourceJson and extractToolJson currently cast unknowns with "as" and throw generic errors; add lightweight type guards that verify raw.raw.contents is an array and contains text (for extractResourceJson) and that raw.raw.structuredContent or raw.raw.content[0].text exists (for extractToolJson), use those guards before parsing, wrap JSON.parse in a try/catch to throw a descriptive Error that includes which function failed and a stringified snippet of the problematic result (e.g., include the first 200 chars of JSON.stringify(result)), and replace the generic messages ("No text content in resource result" / "No content in tool result") with these more descriptive errors to aid debugging.apps/e2e/demo-e2e-resource-providers/webpack.config.js (1)
11-12: Apply NODE_ENV check to mode and devtool to prevent development settings from leaking into production builds.Lines 11–12 hard-code development settings (
mode: 'development'anddevtool: 'eval-cheap-module-source-map') without respecting build configurations. Line 7 already establishes theprocess.env.NODE_ENVpattern in this file for conditional output settings; the same pattern should extend to webpack's mode and devtool to ensure production builds use appropriate settings when invoked with production configuration.Suggested refactor
const { NxAppWebpackPlugin } = require('@nx/webpack/app-plugin'); const { join } = require('path'); +const isProd = process.env.NODE_ENV === 'production'; module.exports = { @@ - mode: 'development', - devtool: 'eval-cheap-module-source-map', + mode: isProd ? 'production' : 'development', + devtool: isProd ? false : 'eval-cheap-module-source-map',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/demo-e2e-resource-providers/webpack.config.js` around lines 11 - 12, Replace the hard-coded development values for webpack config properties by reading process.env.NODE_ENV: set the exported config's mode to 'development' when process.env.NODE_ENV === 'development' and to 'production' otherwise, and set devtool to a development-friendly source map (e.g., 'eval-cheap-module-source-map') only for development while using a production-appropriate value (e.g., false or 'source-map') for non-development; update the config object properties 'mode' and 'devtool' accordingly so builds invoked with NODE_ENV=production do not leak development settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/e2e/demo-e2e-resource-providers/webpack.config.js`:
- Line 24: The webpack build config's generatePackageJson option is inconsistent
with the project.json setting; update the generatePackageJson property in the
webpack configuration (the object containing generatePackageJson: false) to true
so it matches project.json's build target and uses a single source of truth.
---
Nitpick comments:
In `@apps/e2e/demo-e2e-resource-providers/e2e/resource-providers.e2e.spec.ts`:
- Around line 16-32: The helpers extractResourceJson and extractToolJson
currently cast unknowns with "as" and throw generic errors; add lightweight type
guards that verify raw.raw.contents is an array and contains text (for
extractResourceJson) and that raw.raw.structuredContent or
raw.raw.content[0].text exists (for extractToolJson), use those guards before
parsing, wrap JSON.parse in a try/catch to throw a descriptive Error that
includes which function failed and a stringified snippet of the problematic
result (e.g., include the first 200 chars of JSON.stringify(result)), and
replace the generic messages ("No text content in resource result" / "No content
in tool result") with these more descriptive errors to aid debugging.
In `@apps/e2e/demo-e2e-resource-providers/jest.e2e.config.ts`:
- Around line 40-41: The config spreads e2eCoveragePreset after setting
coverageDirectory so the preset can overwrite your explicit value; change the
order so ...e2eCoveragePreset is spread first and then your explicit settings
(e.g., coverageDirectory) come after to ensure your overrides take
precedence—update the object where coverageDirectory and ...e2eCoveragePreset
are combined so e2eCoveragePreset appears before coverageDirectory.
In `@apps/e2e/demo-e2e-resource-providers/src/plugins/counter/counter.plugin.ts`:
- Line 6: The exported symbol COUNTER_TOKEN is declared but never used; either
remove the export or switch the plugin to use that token consistently. If you
choose removal: delete the COUNTER_TOKEN declaration and any unused imports so
the plugin uses the CounterService class directly in providers and
contextExtensions. If you prefer using the token: replace occurrences of
CounterService in the providers array and contextExtensions mapping with
COUNTER_TOKEN and ensure CounterService is provided under that token (e.g., {
provide: COUNTER_TOKEN, useClass: CounterService }) so external code can import
COUNTER_TOKEN as the DI key.
In `@apps/e2e/demo-e2e-resource-providers/webpack.config.js`:
- Around line 11-12: Replace the hard-coded development values for webpack
config properties by reading process.env.NODE_ENV: set the exported config's
mode to 'development' when process.env.NODE_ENV === 'development' and to
'production' otherwise, and set devtool to a development-friendly source map
(e.g., 'eval-cheap-module-source-map') only for development while using a
production-appropriate value (e.g., false or 'source-map') for non-development;
update the config object properties 'mode' and 'devtool' accordingly so builds
invoked with NODE_ENV=production do not leak development settings.
In `@libs/cli/src/core/__tests__/help.spec.ts`:
- Around line 47-53: The test uses raw toContain checks against getHelpOutput()
which can fail with ANSI/whitespace differences; call
normalizeAnsi(getHelpOutput()) (the same normalizer used later in the file) and
assert against that normalized string (e.g., check
normalizedHelp.includes('skills search'), 'skills list', 'skills install',
'skills read') so the assertions become resilient to styling/formatting changes;
update the block that currently references getHelpOutput() to first create
normalizedHelp = normalizeAnsi(getHelpOutput()) and use
normalizedHelp.includes(...) for each skill check.
In
`@libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md`:
- Line 31: Update the checklist item that currently says "`dispose()` is called
in the host app's shutdown handler" to explicitly require both the client and
server cleanup methods by mentioning "ensure client.close() is called and
server.dispose() is called in the host app's shutdown handler" (matching the API
Surface and example usage which reference client.close() and server.dispose()).
- Line 15: The docs currently imply connect() is a standalone function; update
the wording to clarify connect() is a method on the server object returned by
create() (e.g., "the server's connect() method returns a typed client..."), and
adjust the example and any references so they call connect as a method on the
server instance (referencing create() and the server object) rather than as a
top-level function.
- Line 19: Update the checklist line that currently reads "`package.json` has
correct `main`, `module`, `types` fields" to also include an `exports` entry;
specifically add a new checkbox like "`package.json` has correct `main`,
`module`, `types`, and `exports` fields" and include a short sub-note to verify
the `exports` field covers CJS and ESM entry points (or maps conditional
exports) to ensure compatibility for dual ESM/CJS SDK consumers.
- Line 17: Replace the weak wording "properly" with a stronger, explicit
guarantee: update the checklist item for server.dispose() and client.close() to
read something like "Server dispose() and client close() must completely release
all resources (connections, timers, listeners)" or "Server dispose() and client
close() must thoroughly clean up and release all resources (connections, timers,
listeners)"; reference the exact symbols dispose() and close() so reviewers know
to verify those methods perform the complete resource teardown.
🪄 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: 578d7d0a-2afb-4c64-aabc-44aa8317c874
📒 Files selected for processing (24)
.github/workflows/codeql.ymlapps/e2e/demo-e2e-resource-providers/e2e/resource-providers.e2e.spec.tsapps/e2e/demo-e2e-resource-providers/jest.e2e.config.tsapps/e2e/demo-e2e-resource-providers/project.jsonapps/e2e/demo-e2e-resource-providers/src/apps/main/index.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/providers/data-store.provider.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/resources/counter-status.resource.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/resources/debug-providers.resource.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/resources/store-contents.resource.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/tools/counter-increment.tool.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/tools/debug-providers.tool.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/tools/store-get.tool.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/tools/store-set.tool.tsapps/e2e/demo-e2e-resource-providers/src/main.tsapps/e2e/demo-e2e-resource-providers/src/plugins/counter/counter.plugin.tsapps/e2e/demo-e2e-resource-providers/tsconfig.app.jsonapps/e2e/demo-e2e-resource-providers/tsconfig.jsonapps/e2e/demo-e2e-resource-providers/webpack.config.jslibs/cli/src/commands/skills/catalog.tslibs/cli/src/core/__tests__/help.spec.tslibs/skills/catalog/frontmcp-development/references/create-plugin.mdlibs/skills/catalog/frontmcp-development/references/decorators-guide.mdlibs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.mdlibs/skills/src/index.ts
✅ Files skipped from review due to trivial changes (6)
- .github/workflows/codeql.yml
- libs/skills/src/index.ts
- apps/e2e/demo-e2e-resource-providers/tsconfig.json
- apps/e2e/demo-e2e-resource-providers/tsconfig.app.json
- apps/e2e/demo-e2e-resource-providers/project.json
- libs/skills/catalog/frontmcp-development/references/create-plugin.md
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/cli/src/commands/skills/catalog.ts
- libs/skills/catalog/frontmcp-development/references/decorators-guide.md
…ties from responses
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/e2e/demo-e2e-resource-providers/e2e/resource-providers.e2e.spec.ts`:
- Around line 44-55: The tests claiming to list "all" tools/resources need to
include assertions for the debug endpoints; update the test cases that call
mcp.tools.list() and mcp.resources.list() to also assert presence of the debug
tool and debug resource (e.g., add
expect(tools).toContainTool('debug_providers') in the "should list all tools"
test and expect(resources).toContainResource('debug://providers') in the "should
list all resources" test) so discovery regressions for the debug endpoints will
be caught; locate the assertions in the tests that reference mcp.tools.list and
mcp.resources.list and append the new expect checks.
🪄 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: 455f44d5-96bf-487e-a903-318036f26027
📒 Files selected for processing (7)
apps/e2e/demo-e2e-resource-providers/e2e/resource-providers.e2e.spec.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/resources/debug-providers.resource.tsapps/e2e/demo-e2e-resource-providers/src/apps/main/tools/debug-providers.tool.tsapps/e2e/demo-e2e-resource-providers/src/plugins/counter/counter.plugin.tsapps/e2e/demo-e2e-resource-providers/webpack.config.jslibs/sdk/src/resource/flows/read-resource.flow.tslibs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md
✅ Files skipped from review due to trivial changes (2)
- apps/e2e/demo-e2e-resource-providers/webpack.config.js
- libs/skills/catalog/frontmcp-production-readiness/references/production-node-sdk.md
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/sdk/src/resource/flows/read-resource.flow.ts
- apps/e2e/demo-e2e-resource-providers/src/apps/main/tools/debug-providers.tool.ts
…ties from responses
Contributor
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. |
This was referenced Mar 31, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
skills readreplacesshow; installer adds provider-specific skills README section; demo e2e app showcases resource/provider interoperability.Help / UX
Bug Fixes
Documentation