fix: derive execute() input/output types from schemas in tool template#428
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR standardizes FrontMCP tool classes by hoisting ChangesSchema-Derived Typing Pattern Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-rate-limiting-and-progress.md (1)
88-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winImport
BatchProcessToolin the app snippet.The snippet references
BatchProcessToolbut does not import it.Suggested fix
// src/apps/main/index.ts import { App } from '`@frontmcp/sdk`'; +import { BatchProcessTool } from './tools/batch-process.tool'; `@App`({ name: 'main', tools: [BatchProcessTool], }) class MainApp {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-rate-limiting-and-progress.md` around lines 88 - 96, The snippet uses BatchProcessTool in the App decorator but never imports it; add an import for BatchProcessTool at the top of the file (before the `@App` usage), referencing the module that exports it (the example's BatchProcessTool module or package) and ensure the exported symbol name matches (BatchProcessTool) so MainApp can compile.libs/skills/catalog/frontmcp-development/examples/create-tool/basic-class-tool.md (1)
59-66:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd missing
GreetUserToolimport in app snippet.
tools: [GreetUserTool]is referenced without an import, so the example is not copy-paste runnable.Suggested fix
// src/apps/main/index.ts import { App } from '`@frontmcp/sdk`'; +import { GreetUserTool } from './tools/greet-user.tool'; `@App`({ name: 'main', tools: [GreetUserTool], }) class MainApp {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/skills/catalog/frontmcp-development/examples/create-tool/basic-class-tool.md` around lines 59 - 66, The example snippet references GreetUserTool but does not import it; update the top of the snippet (where imports are declared) to add an import for GreetUserTool so the code is runnable—e.g., import GreetUserTool from its module alongside the existing App import—ensuring the symbols GreetUserTool, App, and MainApp are all resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-di-and-errors.md`:
- Around line 22-30: The unlabeled fenced code block showing the project tree
triggers MD040; update the triple-backtick fence in
libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-di-and-errors.md
to include a language tag (e.g., ```text) so the file-tree block is labeled,
preserving the exact contents between the backticks; this will satisfy the
linter without changing the displayed tree.
In `@libs/skills/catalog/frontmcp-development/references/create-tool.md`:
- Around line 106-111: The markdown examples in create-tool.md are missing
language tags on their directory-tree code fences (triggering MD040); update
both code fences (the block shown at lines ~106-111 and the similar block at
~115-123) to include a language tag such as "text" (e.g., change ``` to ```text)
and apply the suggested content updates (use the alternative expanded tree
sample) so the fenced blocks are properly labeled and the examples match the
repo style.
---
Outside diff comments:
In
`@libs/skills/catalog/frontmcp-development/examples/create-tool/basic-class-tool.md`:
- Around line 59-66: The example snippet references GreetUserTool but does not
import it; update the top of the snippet (where imports are declared) to add an
import for GreetUserTool so the code is runnable—e.g., import GreetUserTool from
its module alongside the existing App import—ensuring the symbols GreetUserTool,
App, and MainApp are all resolved.
In
`@libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-rate-limiting-and-progress.md`:
- Around line 88-96: The snippet uses BatchProcessTool in the App decorator but
never imports it; add an import for BatchProcessTool at the top of the file
(before the `@App` usage), referencing the module that exports it (the example's
BatchProcessTool module or package) and ensure the exported symbol name matches
(BatchProcessTool) so MainApp can compile.
🪄 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: 6099cd94-13c8-455f-a809-131c37e20ad0
📒 Files selected for processing (7)
libs/nx-plugin/src/generators/tool/files/__fileName__.tool.ts__tmpl__libs/nx-plugin/src/generators/tool/tool.spec.tslibs/skills/catalog/frontmcp-development/examples/create-tool/basic-class-tool.mdlibs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-di-and-errors.mdlibs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-rate-limiting-and-progress.mdlibs/skills/catalog/frontmcp-development/references/create-tool.mdlibs/skills/catalog/skills-manifest.json
Contributor
Performance Test ResultsStatus: ✅ All tests passed Summary
Total: 101 tests across 21 projects 📊 View full report in workflow run Generated at: 2026-05-24T13:16:55.385Z |
Contributor
There was a problem hiding this comment.
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/examples/create-tool/tool-with-di-and-errors.md (1)
102-114:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd import or comment for
DatabaseProvider.Line 110 references
DatabaseProviderwithout showing its import. While line 128 notes that the provider implementation is covered in thecreate-providerreference, adding a brief comment or import statement here would improve example completeness and prevent copy-paste errors.📝 Suggested fix
// src/apps/main/index.ts import { App } from '`@frontmcp/sdk`'; import { DeleteRecordTool } from './tools/delete-record'; +// DatabaseProvider implementation shown in create-provider reference `@App`({ name: 'main', providers: [DatabaseProvider],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-di-and-errors.md` around lines 102 - 114, The example references DatabaseProvider in the App decorator but doesn't show its import; add an import for DatabaseProvider at the top (or add a one-line comment next to providers: [DatabaseProvider] stating "see create-provider example for implementation") so consumers copying MainApp (class MainApp) and the App decorator (App({ providers: [DatabaseProvider], tools: [DeleteRecordTool] })) won't miss the provider; ensure the import or comment references the same DatabaseProvider symbol used in the file and keep DeleteRecordTool and App imports unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In
`@libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-di-and-errors.md`:
- Around line 102-114: The example references DatabaseProvider in the App
decorator but doesn't show its import; add an import for DatabaseProvider at the
top (or add a one-line comment next to providers: [DatabaseProvider] stating
"see create-provider example for implementation") so consumers copying MainApp
(class MainApp) and the App decorator (App({ providers: [DatabaseProvider],
tools: [DeleteRecordTool] })) won't miss the provider; ensure the import or
comment references the same DatabaseProvider symbol used in the file and keep
DeleteRecordTool and App imports unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5147692b-abba-4f84-a078-edd22dc13ff4
📒 Files selected for processing (2)
libs/skills/catalog/frontmcp-development/examples/create-tool/tool-with-di-and-errors.mdlibs/skills/catalog/frontmcp-development/references/create-tool.md
✅ Files skipped from review due to trivial changes (1)
- libs/skills/catalog/frontmcp-development/references/create-tool.md
…_case consistency
…es' into fix/405-create-tool-inferred-types
…_case consistency
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
1 participant
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
Documentation
Tests
Chores