Merged
Conversation
Contributor
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a standalone MCP server package ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI (user / interactive)
participant MCPClient as mcp-init/status (local CLI)
participant MCPServer as ignix-mcp-server (stdio)
participant Tool as Tool Module (e.g., add_component)
participant FS as Filesystem (project dir)
participant NPM as npm / external registry
CLI->>MCPClient: user invokes "mcp init/status"
MCPClient->>MCPServer: send request over stdio (list_tools / call_tool)
MCPServer->>Tool: dispatch call_tool -> selected tool
Tool->>FS: read/write project files (package.json, src/)
Tool->>NPM: run npx/inspect package versions (install/check)
Tool-->>MCPServer: return structured JSON content
MCPServer-->>MCPClient: marshal MCP response
MCPClient-->>CLI: display status / output (human or --json)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Notes from a senior reviewer (concise, concrete):
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli-tool/src/commands/mcp.ts`:
- Around line 22-25: The fallback path in the possiblePaths array points to
path.resolve(__dirname, '../mcp.json') which ends up resolving into the compiled
dist directory instead of the package root; update the second entry in
possiblePaths to point two levels up (e.g., path.resolve(__dirname, '..', '..',
'mcp.json')) or otherwise resolve from the package root (or use require.resolve/
pkg-dir to locate package root) so that the code finds the published mcp.json at
package root; modify the possiblePaths array accordingly to reference the
updated path.
In `@packages/cli-tool/src/services/ComponentService.ts`:
- Around line 14-17: The getConfig() method currently calls loadConfig() on
every invocation causing repeated expensive I/O; modify ComponentService to
memoize the config at the instance level by adding a private field (e.g.,
configPromise?: Promise<ReturnType<typeof loadConfig>>) and change getConfig()
to return the cached promise if present or assign and return loadConfig() to
that field on first call; ensure install() and any recursive callers use
getConfig() unchanged so subsequent calls reuse the same cached promise.
In `@packages/cli-tool/src/services/TemplateService.ts`:
- Around line 16-19: The current getConfig() in TemplateService calls
loadConfig() on every use causing heavy I/O and duplicated behavior from
ComponentService; replace this by memoizing the config promise either at the
instance level or, preferably, extract the caching into a shared ConfigService
used by both services: create a module with a cached promise (e.g., getConfig()
that returns a single loadConfig() promise and resetConfig() for tests), update
TemplateService to call that shared getConfig instead of its private getConfig,
and ensure code paths that iterate componentDependencies and call
componentService.install() reuse the cached promise to avoid repeated loads.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 58d97362-b354-4f1f-aaa6-ff8450032bf9
📒 Files selected for processing (6)
packages/cli-tool/mcp.jsonpackages/cli-tool/package.jsonpackages/cli-tool/src/commands/mcp.tspackages/cli-tool/src/services/ComponentService.tspackages/cli-tool/src/services/TemplateService.tspackages/registry/templates/pages/dynamic-form/dynamic-form.test.tsx
💤 Files with no reviewable changes (1)
- packages/registry/templates/pages/dynamic-form/dynamic-form.test.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Build components with TypeScript for type-safety and rich IntelliSense support
Files:
packages/cli-tool/src/commands/mcp.tspackages/cli-tool/src/services/ComponentService.tspackages/cli-tool/src/services/TemplateService.ts
🔇 Additional comments (4)
packages/cli-tool/package.json (2)
14-18: Good packaging fix for runtime config availability.Adding
mcp.jsontofilesensures the config is actually shipped with the npm package, which aligns with the MCP lookup flow.
26-35: Keywords reformat is clean and non-functional.No behavioral change here; this looks fine.
packages/cli-tool/src/services/ComponentService.ts (1)
36-36: Usage looks correct, but inherits the uncached overhead.The call site is fine TypeScript—
await this.getConfig()properly awaits the async helper. The issue is upstream ingetConfig()lacking memoization. Once that's fixed, this line needs no change.packages/cli-tool/src/services/TemplateService.ts (1)
39-39: Call site is correct TypeScript.Properly awaits the config. Once upstream caching is applied, this line is good to go.
Comment on lines
+14
to
+17
|
|
||
| private async getConfig(): Promise<ReturnType<typeof loadConfig>> { | ||
| return await loadConfig(); | ||
| } |
Contributor
There was a problem hiding this comment.
Performance regression: config loaded on every install() call without caching.
The new getConfig() method calls loadConfig() on each invocation. Per config.ts, this involves fs.pathExists(), fs.readFile(), and dynamic imports with cache-busting timestamps. Since install() is called in loops (see add.ts:138-150) and recursively for componentDependencies (lines 59-63), this multiplies I/O operations from O(1) to O(N×M).
Consider memoizing at the instance level:
🔧 Proposed fix: cache the config promise
export class ComponentService {
private registryService = new RegistryService();
private dependencyService = new DependencyService();
+ private configPromise: Promise<ReturnType<typeof loadConfig>> | null = null;
- private async getConfig(): Promise<ReturnType<typeof loadConfig>> {
- return await loadConfig();
+ private getConfig(): Promise<ReturnType<typeof loadConfig>> {
+ if (!this.configPromise) {
+ this.configPromise = loadConfig();
+ }
+ return this.configPromise;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-tool/src/services/ComponentService.ts` around lines 14 - 17, The
getConfig() method currently calls loadConfig() on every invocation causing
repeated expensive I/O; modify ComponentService to memoize the config at the
instance level by adding a private field (e.g., configPromise?:
Promise<ReturnType<typeof loadConfig>>) and change getConfig() to return the
cached promise if present or assign and return loadConfig() to that field on
first call; ensure install() and any recursive callers use getConfig() unchanged
so subsequent calls reuse the same cached promise.
Comment on lines
+16
to
+19
|
|
||
| private async getConfig(): Promise<ReturnType<typeof loadConfig>> { | ||
| return await loadConfig(); | ||
| } |
Contributor
There was a problem hiding this comment.
Same uncached getConfig() pattern duplicated from ComponentService.
This has identical performance issues—loadConfig() triggers heavy I/O on every install() call. Since TemplateService also loops over componentDependencies (lines 66-69) and calls componentService.install(), you get compounded config reloads.
Two recommendations:
- Memoize the config promise at the instance level (same fix as ComponentService).
- Extract the caching logic to a shared utility or singleton to avoid DRY violations.
,
🔧 Option 1: Instance-level caching
export class TemplateService {
private registryService = new RegistryService();
private dependencyService = new DependencyService();
private componentService = new ComponentService();
+ private configPromise: Promise<ReturnType<typeof loadConfig>> | null = null;
- private async getConfig(): Promise<ReturnType<typeof loadConfig>> {
- return await loadConfig();
+ private getConfig(): Promise<ReturnType<typeof loadConfig>> {
+ if (!this.configPromise) {
+ this.configPromise = loadConfig();
+ }
+ return this.configPromise;
}♻️ Option 2: Shared singleton (better DRY)
Create a ConfigService that both services consume:
// packages/cli-tool/src/services/ConfigService.ts
import { loadConfig } from '../utils/config';
let configPromise: Promise<ReturnType<typeof loadConfig>> | null = null;
export function getConfig(): Promise<ReturnType<typeof loadConfig>> {
if (!configPromise) {
configPromise = loadConfig();
}
return configPromise;
}
export function resetConfig(): void {
configPromise = null; // useful for testing or hot-reload scenarios
}Then both services import getConfig from ConfigService instead of rolling their own.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-tool/src/services/TemplateService.ts` around lines 16 - 19, The
current getConfig() in TemplateService calls loadConfig() on every use causing
heavy I/O and duplicated behavior from ComponentService; replace this by
memoizing the config promise either at the instance level or, preferably,
extract the caching into a shared ConfigService used by both services: create a
module with a cached promise (e.g., getConfig() that returns a single
loadConfig() promise and resetConfig() for tests), update TemplateService to
call that shared getConfig instead of its private getConfig, and ensure code
paths that iterate componentDependencies and call componentService.install()
reuse the cached promise to avoid repeated loads.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 42
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/cli-tool/src/index.ts (1)
162-179:⚠️ Potential issue | 🟠 MajorFlatten the multiselect result before forwarding to
addCommand.
promptsreturns astring[]formultiselect. Wrapping it in[response.components]turns multiple selections into one argv entry, soignix add componentreceives a single malformed identifier instead of N component names.Suggested fix
- const identifiers = response.components ? [response.components] : []; + const identifiers = Array.isArray(response.components) ? response.components : [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli-tool/src/index.ts` around lines 162 - 179, The multiselect result from prompts is being wrapped into a nested array so addCommand.parseAsync receives a single combined argument; instead, use the array returned by response.components directly when calling addCommand.parseAsync. Replace the identifiers assignment that does const identifiers = response.components ? [response.components] : []; with one that preserves the array (e.g., const identifiers = response.components || [];), and pass identifiers into addCommand.parseAsync([...identifiers]) so addCommand.parseAsync('node','ignix','component', ...identifiers) receives each selected component as its own argv entry.packages/ignix-mcp-server/src/index.ts (1)
414-979: 🧹 Nitpick | 🔵 TrivialRemove the commented-out server copy.
Keeping a second full implementation commented out in the same file makes future fixes easy to miss and unnecessarily bloats the entrypoint. Delete it once the new path is the source of truth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ignix-mcp-server/src/index.ts` around lines 414 - 979, The file contains a full, commented-out duplicate of the MCP server implementation (starting at the commented shebang and including definitions like runCommand, MCP_CONFIG, handleGenericTool, server.setRequestHandler handlers for ListToolsRequestSchema/CallToolRequestSchema/ReadResourceRequestSchema, and the Server/StdioServerTransport startup block); remove this entire commented-out server copy so only the active implementation remains (delete the block that defines the duplicate Server, tools/resources loading, handlers, and the transport/connect log section).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@llms.txt`:
- Around line 31-34: The prompt incorrectly instructs calling init_project which
isn't part of the server API; update Scenario 2 to remove any mention of
init_project and instead rely solely on add_component with cwd to
auto-initialize when ignix.config.js is missing. Specifically, keep the check
for the presence of ignix.config.js, and if it's not present call add_component
with cwd: "." (do not attempt to call init_project), and avoid inventing steps
outside the documented server dispatch methods (create_react_project,
list_components, add_component, generate_template, validate_ignix_only,
get_component_docs).
In `@packages/cli-tool/src/commands/mcp-init.ts`:
- Around line 56-60: The action handler that sets the guard
process.env.MCP_INIT_RUNNING should ensure the flag is cleared after the action
finishes; wrap the body of the .action(async (opts) => { ... }) handler in a
try/finally so that regardless of success/error the env var is removed (use
delete process.env.MCP_INIT_RUNNING or set to '' in the finally). Apply the same
try/finally pattern to the other guarded blocks referenced (the ones around the
--dry-run branch and the other guarded sections) so the guard does not persist
across subsequent calls in the same interactive process.
- Around line 143-148: Update the Codex manual setup message in the mcp-init.ts
branch where you check if (!configPath) and client === 'codex' — replace the
outdated args = ["-y", "ignix", "mcp"] guidance with the current entrypoint (npx
-y `@mindfiredigital/ignix-mcp-server`@<version>) so the printed
[mcp_servers.ignix] example reflects the new package name and invocation; ensure
the console.log lines that emit the Codex block show the correct npx package and
args format consistent with the generated config.
- Around line 176-181: The current shallow merge that builds newConfig
(spreading existingConfig and getConfigContent(client, server)) overwrites the
entire servers/mcpServers map and can delete other MCP entries; instead perform
a nested merge for those maps: build newConfig by spreading existingConfig and
the new content but explicitly merge existingConfig.servers (and/or
existingConfig.mcpServers) with the servers/mcpServers returned by
getConfigContent(client, server) so you preserve existing keys while
adding/updating the ignix entry before calling fs.writeJSON(fullPath, newConfig,
{ spaces: 2 }).
- Around line 28-37: getConfigPath currently maps the 'claude' client to a local
".mcp.json", but mcp-status expects Claude Desktop's OS-specific
"claude_desktop_config.json"; update getConfigPath in getConfigPath(client:
Client) so that for 'claude' it returns the same OS-aware path that mcp-status
reads (the "claude_desktop_config.json" location), using the same helper or
logic used in packages/cli-tool/src/commands/mcp-status.ts to determine the
OS-specific config file path, so mcp init --client claude writes to the exact
file mcp status and Claude Desktop will find.
In `@packages/cli-tool/src/types/index.ts`:
- Around line 56-59: The ToolExecution interface currently pins type to the
literal 'shell', which prevents adding other execution kinds later; update the
ToolExecution definition (interface ToolExecution) so its type property is a
union of supported execution kinds (e.g., 'shell' | 'node' | 'http') and keep
command as-is, ensuring any usages or discriminated-union handling sites are
adjusted to accept the new union members.
- Around line 1-46: This file contains duplicate TypeScript interface
declarations causing TS2300 errors; remove the redundant second block and keep a
single definition for MCPServerConfig, CursorClaudeConfig, VSCodeConfig, and
PackageJson (ensure the existing MCPClientConfig still references the single
CursorClaudeConfig & VSCodeConfig), so delete the duplicate declarations (the
repeated MCPServerConfig, CursorClaudeConfig, VSCodeConfig, PackageJson) and
keep the original/exported versions to restore unique type identifiers.
In `@packages/cli-tool/src/utils/error-handler.d.ts`:
- Around line 1-21: The declaration files for createErrorResponse and
createSuccessResponse declare the response content.type as a string; change that
to the literal type 'text' to match the implementation and improve type safety
by updating the return types of createErrorResponse and createSuccessResponse so
their content array items have type: 'text' instead of type: string (refer to
functions createErrorResponse and createSuccessResponse and the content.type
property).
In `@packages/cli-tool/src/utils/error-handler.js`:
- Around line 35-51: The createSuccessResponse function treats null as an object
because typeof null === 'object'; update the guard before spreading by verifying
data is non-null and an object (e.g., data !== null && typeof data === 'object')
so you only spread when data is a real object and otherwise wrap the value as {
data } to avoid unexpected output from spreading null.
- Around line 2-13: The declaration file expects getErrorMessage to be exported
but the JS file doesn't export it; export the function to match
error-handler.d.ts by adding an export (e.g., export function
getErrorMessage(...) { ... } or append module.exports.getErrorMessage =
getErrorMessage) consistent with the repository's module system, ensuring the
symbol name getErrorMessage is publicly exported.
- Around line 1-2: The header comment above the getErrorMessage function
incorrectly references a different path; update that comment so it reflects the
actual file path for clarity by replacing
"packages/ignix-mcp-server/src/utils/error-handler.ts" with the correct path
"packages/cli-tool/src/utils/error-handler.js" in the comment immediately above
the getErrorMessage function.
In `@packages/cli-tool/src/utils/error-handler.ts`:
- Around line 37-53: createSuccessResponse can throw when data is null because
typeof null === 'object' but spreading null fails; update the conditional used
before spreading to ensure data is a non-null object (e.g., replace "typeof data
=== 'object'" with "data !== null && typeof data === 'object'") so you only
spread when safe, otherwise include data under a { data } key or similar,
keeping the same JSON structure and behavior of createSuccessResponse.
- Line 1: Update the top-of-file comment that currently contains the incorrect
package path string "packages/ignix-mcp-server/src/utils/error-handler.ts" so it
accurately reflects this file's package (or remove the stale header comment
entirely); locate the comment at the top of the file in error-handler.ts and
replace the wrong package path with the correct package identifier for this file
(or delete the comment) to avoid future confusion.
In `@packages/cli-tool/tsconfig.json`:
- Around line 5-7: Remove the three commented-out include paths
("../ignix-mcp-server/src/commands/mcp-init.ts",
"../ignix-mcp-server/src/commands/mcp-status.ts",
"../ignix-mcp-server/src/commands/mcp.ts") from the tsconfig.json to eliminate
dead configuration noise; simply delete those commented lines so the file only
contains active config entries and avoid adding such cross-package paths until
they're actually required.
In `@packages/ignix-mcp-server/CHANGELOG.md`:
- Around line 1-45: Remove the surrounding triple-backtick code fence and the
leading path comment so the file is plain Markdown (delete the opening
```markdown and the closing ``` and any leading filename line), and update the
release header "## [1.0.0] - 2024-01-XX" to use an actual release date (replace
2024-01-XX with the real YYYY-MM-DD).
In `@packages/ignix-mcp-server/mcp.json`:
- Around line 128-131: The inline 400+ character Node script in the "execution"
command is fragile and produces invalid JSX for the conditional ThemeProvider;
extract this logic into a dedicated script/CLI (e.g., a new script file invoked
by the command) that accepts page and theme args, constructs the component
string with correct conditional wrapping of ThemeProvider (ensure both opening
and closing tags are added only when theme is truthy), writes to src/pages via
fs.writeFileSync, and prints the JSON result; update the mcp.json "command" to
call that script (keeping references to page, theme, path.join and
fs.writeFileSync and the generated file path src/pages) so the template
generation is readable, testable, and fixes the JSX nesting bug.
- Around line 172-177: The "component_registry" resource in mcp.json uses a
relative "uri" ("file://../../packages/registry/registry.json") which can break
depending on working directory; change this by either (A) making the "uri" a
runtime-resolved absolute path (compute path.resolve(__dirname,
'../../packages/registry/registry.json') in your startup code and inject the
absolute file:// URI into the manifest before serving), or (B) remove the
file:// URI and instead load/parse registry.json from the server startup code
and embed its contents into the MCP manifest under the "component_registry"
resource; update the code that reads mcp.json to accept the injected/embedded
registry so "component_registry" no longer depends on a fragile relative path.
In `@packages/ignix-mcp-server/package.json`:
- Line 20: The "clean" npm script currently uses a Unix-only command ("rm -rf")
which breaks on Windows; update the "clean" script in package.json to a
cross-platform approach (either use the "rimraf" package and add it to
devDependencies, then set the "clean" script to use rimraf, or use a Node-based
removal command relying on fs.rmSync if targeting Node 18+); modify the "clean"
script value and, if choosing rimraf, add rimraf to devDependencies so CI and
developers on Windows can run npm run clean reliably.
In `@packages/ignix-mcp-server/src/index.ts`:
- Around line 293-304: The current interpolation replaces placeholders in
tool.execution.command into a single shell string and calls runCommand, which
risks command injection; change handleGenericTool to parse
tool.execution.command into an argv array (split by spaces or the tool's
declared arg template), substitute each {key} only inside individual argv tokens
(joining arrays into separate argv entries), and call runCommand (or a new
runCommandArgs) using an exec/spawn variant that accepts argv[] (no shell) so
arguments are passed as separate parameters; ensure you reference and update the
token-replacement logic around tool.execution.command and the call site that
uses runCommand to accept and use argv[] instead of a single shell string.
In `@packages/ignix-mcp-server/src/resources/docs-content.ts`:
- Around line 34-54: The file contains a large block of commented-out
alternative docs assigned to docsContent which is dead/commented code; remove
the commented-out block entirely so only the active documentation remains and
any real docsContent constant (e.g., docsContent) is left clean, using git
history to retrieve old versions if needed; update/commit the change by deleting
the commented block in packages/ignix-mcp-server/src/resources/docs-content.ts
and ensure no other references to that commented snippet remain.
- Line 28: The import path for the Button component is inconsistent:
docs-content.ts uses "import { Button } from '@/components/ui/button'" while
ignix-rules.ts uses "import { Button } from '@ignix-ui/button'"; pick the
project's canonical import pattern and update the other file so both import the
same symbol from the same module (i.e., change the import in docs-content.ts or
in ignix-rules.ts to match the chosen pattern), ensuring the named import Button
remains unchanged.
In `@packages/ignix-mcp-server/src/resources/ignix-rules.ts`:
- Around line 151-152: The import path usage is inconsistent: the file imports
UI components with "import { Button } from '@ignix-ui/button'; import { Card }
from '@ignix-ui/card';" but later validation expects "@/components/ui/". Choose
one pattern and make imports consistent; either update the top imports (Button,
Card) to "@/components/ui/button" and "@/components/ui/card" to match the
validator, or change the validation to accept '@ignix-ui/button' and
'@ignix-ui/card'. Update the references to the unique symbols Button and Card
(and any related imports) so all import statements and the validation string use
the same path pattern.
- Around line 1-57: Remove the 57-line commented-out previous version of
IGNIX_RULES at the top of the file: delete the entire commented block (all lines
starting with // that contain the old multi-scenario rules) so the source no
longer contains dead commented code; ensure the file still exports the intended
IGNIX_RULES constant (no leftover commented export or duplicate declarations)
and that the module compiles cleanly after removal.
In `@packages/ignix-mcp-server/src/resources/registry-summary.ts`:
- Around line 19-22: The code parses registryContent into registry and
immediately accesses registry.components, which can be undefined; update the
logic in registry-summary.ts (around registryContent, registry and the return
that uses Object.keys(registry.components)) to validate that registry and
registry.components exist and are objects before calling Object.keys; if
components is missing or not an object, treat totalComponents as 0 (or fallback
to an empty object) and proceed safely so the function never throws when
registry.json lacks a components key.
- Around line 33-38: The fallback summary currently returns a hardcoded
totalComponents: 90 which will drift; change the fallback to use an explicit
"unknown" sentinel (e.g., totalComponents: -1 or 0) instead of 90 and add a
short TODO comment explaining why the sentinel is used and how to replace it
with a real count later; update the return object in the fallback summary (the
returned object containing totalComponents, note, tip in registry-summary.ts) to
use the sentinel and include the TODO so maintainers know this is intentional.
In `@packages/ignix-mcp-server/src/tools/add-component.ts`:
- Around line 19-25: The code currently interpolates untrusted componentsList
into shell commands via runCommand (which uses execAsync), allowing shell
injection; replace runCommand calls that build commands with string
interpolation by using an argv-based spawn (e.g., execa or child_process.spawn)
that takes the program and argument array (no shell expansion), pass cwd and env
explicitly (preserve PATH), and remove shell:true usage; also validate and
sanitize each component name before using it (e.g., enforce a strict whitelist
regex like /^[A-Za-z0-9._-]+$/ and reject or normalize anything else) and apply
the same fix to the other block that constructs commands from componentsList
(the second occurrence in this file) so all uses of componentsList use
argv-based execution and validated names.
In `@packages/ignix-mcp-server/src/tools/create-project.ts`:
- Around line 26-38: The runCommand function calls execAsync without a timeout
which can hang indefinitely; update runCommand (the async function runCommand
that returns {stdout, stderr}) to pass a timeout option to execAsync (e.g.,
timeout in ms) and handle timeout errors by rejecting with a clear error/throw
so callers can recover; ensure the env, shell and maxBuffer options are
preserved and add a sensible default timeout (configurable via parameter or
constant) and wrap/annotate the thrown error so the caller can distinguish
timeout vs other execution failures.
- Around line 152-156: The current replacement in the block that checks
mainContent.includes('import "./index.css"') uses a regex /import React from
'react'/ that only matches single quotes while the replacement uses double
quotes; update the pattern used in mainContent.replace (the code referencing
mainContent and the regex) to match both single and double quote styles (e.g., a
regex that allows either quote around React) or match the import line more
generically before inserting import "./index.css", so the insert reliably runs
regardless of quote style.
- Around line 40-41: The function createProjectTool currently types its
parameter as any; define a proper interface (e.g., CreateProjectArgs) describing
the expected shape—projectName: string; template?: string; cwd?: string—export
it if needed, replace the parameter type with that interface
(createProjectTool(args: CreateProjectArgs)), and keep the existing
defaulting/destructuring ({ projectName, template = 'react-ts', cwd = '.' }) so
TypeScript can enforce correct usage and provide IDE/autocomplete benefits.
- Around line 131-137: The current string replacements on viteConfig (the two
viteConfig.replace(...) calls that add "import ... from '@tailwindcss/vite'" and
replace 'plugins: [react()]') are brittle and may silently fail; update them to
use robust regular expressions that tolerate different quotes/whitespace and
optional trailing commas, perform the replacements via RegExp (targeting the
import statement and the plugins array entry), and then validate that both regex
replacements actually matched and modified viteConfig (if not, throw or log an
error and abort) so you never silently write a corrupted Vite config.
In `@packages/ignix-mcp-server/src/tools/generate-template.ts`:
- Line 5: The generateTemplateTool function uses a loosely typed parameter
(args: any); define a specific interface (e.g., GenerateTemplateArgs with
optional fields page?: string, template?: string, cwd?: string) and update the
function signature to use it (export async function generateTemplateTool(args:
GenerateTemplateArgs)) so callers get proper type checking and IntelliSense; add
the interface near the top of the file and update any internal uses of args to
align with the new properties.
- Around line 5-9: The early call to page.includes('.css') in
generateTemplateTool can throw if page is undefined; move the null/undefined
validation for page (and template if needed) so it occurs before any .includes()
checks, i.e., check that page is a non-empty string (and template exists) at the
start of generateTemplateTool, then perform the ".includes('.css')" and
".includes('css')" checks; after reordering, remove the duplicate null/undefined
validation block that previously guarded page later in the function to avoid
redundant checks.
- Around line 92-107: The current logic at appPath replaces the entire App.tsx
when adding a new page (using appContent + newAppContent) which discards
existing imports, routing and custom logic; change generate-template.ts to
perform an incremental update: when appContent does not include
`./pages/${page}` insert an import line `import ${pageName}Page from
'./pages/${page}';` into the top import block (preserve existing imports) and
then locate the existing `function App()` (or default export component) and
either inject the new page component into its returned JSX or add a clear
comment/snippet inside the function for manual integration; use string
parsing/regex to find the import section and the `function App` body rather than
overwriting the whole file, and ensure you still create the file only if the
import is not already present.
In `@packages/ignix-mcp-server/src/tools/get-component-docs.ts`:
- Around line 29-30: The hard-coded REGISTRY_PATH constant points to an absolute
Windows path and must be replaced with a package-relative resolution: compute
the path to packages/registry/registry.json using import.meta.url (or URL + path
utilities) inside get-component-docs.ts instead of the literal 'D:\\...' string;
update the REGISTRY_PATH symbol (or replace usages) so it resolves
cross-platform and in CI, then use that resolved path when reading the registry.
In `@packages/ignix-mcp-server/src/tools/init-project.ts`:
- Line 23: Add a proper type for the args parameter of initProjectTool by
declaring an interface (e.g., InitProjectArgs { cwd?: string }) and use it in
the function signature: change export async function initProjectTool(args: any)
to export async function initProjectTool(args: InitProjectArgs). Also update any
internal references or call sites that pass args to satisfy the new type (e.g.,
ensure callers pass an object with optional cwd) so TypeScript type checking
passes.
- Line 80: Replace the stdout logging call so MCP server logs go to stderr: in
packages/ignix-mcp-server/src/tools/init-project.ts locate the
console.log(output) call (in the same scope as the earlier console.error usage)
and change it to emit to stderr (e.g., use console.error or
process.stderr.write) so all logging in the init-project tool is sent to stderr
and does not pollute the MCP stdout protocol stream.
In `@packages/ignix-mcp-server/src/tools/list-components.ts`:
- Around line 87-113: The catch block in list-components.ts currently returns
success: true on failure; change it to return a proper tool error response so
callers can detect failures: capture the thrown error (e.g., errorMessage =
error instanceof Error ? error.message : String(error)) and return the same JSON
structure but with success: false, isError: true, an error or message field
containing errorMessage (and keep components: [] and pagination), so downstream
logic (e.g., the caller that checks isError) correctly treats this as a failure.
In `@packages/ignix-mcp-server/src/tools/validate-ignix-only.ts`:
- Around line 11-13: The commented regex entries `/@radix-ui/` and
`/framer-motion/` in the patterns list are dead code—either remove them or add a
concise inline comment explaining why they are intentionally excluded/allowed
(for example, "Ignix uses `@radix-ui` internally; kept intentionally allowed") so
future readers understand the rationale; update the patterns array in
validate-ignix-only.ts (the commented lines near the other regex entries)
accordingly and ensure any explanation mentions whether they are intentionally
permitted or should be permanently removed.
- Line 64: The function validateIgnixOnlyTool currently types its parameter as
any; define and use a proper interface (e.g., ValidateIgnixOnlyArgs with
optional path?: string and cwd?: string) and change the function signature to
accept args: ValidateIgnixOnlyArgs so callers and IDEs get proper typing; update
any related imports/exports if necessary to expose the interface where required
and adjust internal usages of args.path/args.cwd to match the typed properties.
- Around line 70-87: The current early-return when !(await
fs.pathExists(fullPath)) incorrectly reports success and valid as true; update
the returned object in that check (the block referencing fullPath and scanPath)
to reflect a non-successful validation by setting success: false and valid:
false and include classification: 'PATH_NOT_FOUND' and an action string like
'Ensure the project has a src directory with React components.' so callers can
distinguish a missing-path condition from a successful validation.
- Around line 45-58: The for-await loop that processes files sequentially in
scanDirectory (iterating over files and calling scanDirectory and scanFile with
await) is slow for large repos; refactor to process directory entries in
parallel by mapping files to async worker tasks and awaiting Promise.all on that
array. Keep the same exclusion check for ['node_modules', '.next', 'dist',
'build', '.git', 'coverage'] and preserve calling scanDirectory(filePath,
basePath) for directories and scanFile(filePath) for files, but return or
flatten the combined results from Promise.all instead of pushing into a shared
violations array to avoid race conditions.
- Line 41: The function scanDirectory currently declares an unused parameter
basePath; remove basePath from the scanDirectory signature and from every call
site (including the recursive invocation inside scanDirectory and the initial
invocation elsewhere) so the function only accepts the necessary dir: string and
returns Promise<string[]>; update references to scanDirectory(...) accordingly
(look for the scanDirectory function and its recursive call and the initial call
that currently passes basePath) to eliminate the unused parameter.
---
Outside diff comments:
In `@packages/cli-tool/src/index.ts`:
- Around line 162-179: The multiselect result from prompts is being wrapped into
a nested array so addCommand.parseAsync receives a single combined argument;
instead, use the array returned by response.components directly when calling
addCommand.parseAsync. Replace the identifiers assignment that does const
identifiers = response.components ? [response.components] : []; with one that
preserves the array (e.g., const identifiers = response.components || [];), and
pass identifiers into addCommand.parseAsync([...identifiers]) so
addCommand.parseAsync('node','ignix','component', ...identifiers) receives each
selected component as its own argv entry.
In `@packages/ignix-mcp-server/src/index.ts`:
- Around line 414-979: The file contains a full, commented-out duplicate of the
MCP server implementation (starting at the commented shebang and including
definitions like runCommand, MCP_CONFIG, handleGenericTool,
server.setRequestHandler handlers for
ListToolsRequestSchema/CallToolRequestSchema/ReadResourceRequestSchema, and the
Server/StdioServerTransport startup block); remove this entire commented-out
server copy so only the active implementation remains (delete the block that
defines the duplicate Server, tools/resources loading, handlers, and the
transport/connect log section).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: dbd73c20-aacf-4cb5-a8a5-ba7eee2ea250
⛔ Files ignored due to path filters (3)
packages/cli-tool/src/utils/error-handler.d.ts.mapis excluded by!**/*.mappackages/cli-tool/src/utils/error-handler.js.mapis excluded by!**/*.mappnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (27)
llms.txtpackages/cli-tool/src/commands/mcp-init.tspackages/cli-tool/src/commands/mcp-status.tspackages/cli-tool/src/commands/mcp.tspackages/cli-tool/src/index.tspackages/cli-tool/src/types/index.tspackages/cli-tool/src/utils/error-handler.d.tspackages/cli-tool/src/utils/error-handler.jspackages/cli-tool/src/utils/error-handler.tspackages/cli-tool/tsconfig.jsonpackages/ignix-mcp-server/.gitignorepackages/ignix-mcp-server/CHANGELOG.mdpackages/ignix-mcp-server/mcp.jsonpackages/ignix-mcp-server/package.jsonpackages/ignix-mcp-server/src/index.tspackages/ignix-mcp-server/src/resources/docs-content.tspackages/ignix-mcp-server/src/resources/ignix-rules.tspackages/ignix-mcp-server/src/resources/registry-summary.tspackages/ignix-mcp-server/src/tools/add-component.tspackages/ignix-mcp-server/src/tools/create-project.tspackages/ignix-mcp-server/src/tools/generate-template.tspackages/ignix-mcp-server/src/tools/get-component-docs.tspackages/ignix-mcp-server/src/tools/init-project.tspackages/ignix-mcp-server/src/tools/list-components.tspackages/ignix-mcp-server/src/tools/validate-ignix-only.tspackages/ignix-mcp-server/tsconfig.jsonpackages/ignix-mcp-server/tsup.config.ts
💤 Files with no reviewable changes (1)
- packages/cli-tool/src/commands/mcp.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (README.md)
Build components with TypeScript for type-safety and rich IntelliSense support
Files:
packages/ignix-mcp-server/tsup.config.tspackages/ignix-mcp-server/src/resources/docs-content.tspackages/ignix-mcp-server/src/resources/registry-summary.tspackages/ignix-mcp-server/src/resources/ignix-rules.tspackages/cli-tool/src/utils/error-handler.d.tspackages/ignix-mcp-server/src/tools/validate-ignix-only.tspackages/cli-tool/src/utils/error-handler.tspackages/ignix-mcp-server/src/tools/add-component.tspackages/ignix-mcp-server/src/tools/init-project.tspackages/cli-tool/src/commands/mcp-status.tspackages/cli-tool/src/index.tspackages/cli-tool/src/commands/mcp-init.tspackages/ignix-mcp-server/src/index.tspackages/ignix-mcp-server/src/tools/get-component-docs.tspackages/ignix-mcp-server/src/tools/list-components.tspackages/ignix-mcp-server/src/tools/generate-template.tspackages/ignix-mcp-server/src/tools/create-project.tspackages/cli-tool/src/types/index.ts
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: - Code examples must use fenced code blocks with a language tag.
- Component prop tables should be Markdown tables listing type, default, and description.
- Verify relative links point to existing files.
Files:
packages/ignix-mcp-server/CHANGELOG.md
🧠 Learnings (8)
📚 Learning: 2026-04-02T07:32:04.251Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-04-02T07:32:04.251Z
Learning: Applies to **/*.{ts,tsx} : Build components with TypeScript for type-safety and rich IntelliSense support
Applied to files:
packages/cli-tool/tsconfig.jsonpackages/ignix-mcp-server/tsup.config.tspackages/ignix-mcp-server/tsconfig.jsonpackages/cli-tool/src/types/index.ts
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/docs/docs/components/**/*.mdx : Add MDX documentation files for components in `apps/docs/docs/components/` and link them in `apps/docs/sidebars.ts`
Applied to files:
packages/ignix-mcp-server/src/resources/docs-content.tspackages/ignix-mcp-server/src/tools/add-component.tspackages/ignix-mcp-server/src/tools/get-component-docs.ts
📚 Learning: 2026-03-31T05:22:54.128Z
Learnt from: PoonamKPatil
Repo: mindfiredigital/ignix-ui PR: 723
File: apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/contact-form.mdx:29-31
Timestamp: 2026-03-31T05:22:54.128Z
Learning: In the ignix-ui project (mindfiredigital/ignix-ui), the CLI command `ignix add component <name>` is the unified command used to import both components AND templates. Do not flag `ignix add component` as incorrect when used in template documentation — it is intentional.
Applied to files:
packages/ignix-mcp-server/src/resources/docs-content.tspackages/ignix-mcp-server/src/resources/ignix-rules.tspackages/ignix-mcp-server/src/tools/add-component.tsllms.txtpackages/ignix-mcp-server/src/tools/get-component-docs.tspackages/ignix-mcp-server/mcp.jsonpackages/ignix-mcp-server/src/tools/list-components.ts
📚 Learning: 2026-04-02T07:32:04.251Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-04-02T07:32:04.251Z
Learning: Applies to registry.json : registry.json must be updated when new components are added to the project
Applied to files:
packages/ignix-mcp-server/src/resources/registry-summary.ts
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to packages/registry/registry.json : Register components in `packages/registry/components/` directory by creating a component subdirectory and adding an entry to `packages/registry/registry.json`
Applied to files:
packages/ignix-mcp-server/src/resources/registry-summary.tspackages/ignix-mcp-server/src/tools/add-component.tspackages/ignix-mcp-server/src/tools/get-component-docs.ts
📚 Learning: 2026-04-02T07:32:04.251Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-04-02T07:32:04.251Z
Learning: Applies to **/index.tsx : Each component must have an index.tsx file for proper module exports
Applied to files:
packages/ignix-mcp-server/src/tools/validate-ignix-only.ts
📚 Learning: 2026-03-18T09:43:08.260Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2026-03-18T09:43:08.260Z
Learning: Applies to apps/storybook/src/components/**/*.{tsx,ts,jsx,js} : Create Component Files in `apps/storybook/src/components/` directory with a component subdirectory for each component
Applied to files:
packages/ignix-mcp-server/src/tools/add-component.tspackages/ignix-mcp-server/src/tools/generate-template.ts
📚 Learning: 2026-04-02T07:32:04.251Z
Learnt from: CR
Repo: mindfiredigital/ignix-ui PR: 0
File: README.md:0-0
Timestamp: 2026-04-02T07:32:04.251Z
Learning: Use the Component Generator script to create new components with consistent boilerplate structure
Applied to files:
packages/ignix-mcp-server/src/tools/add-component.tsllms.txtpackages/ignix-mcp-server/src/tools/generate-template.ts
🔇 Additional comments (21)
packages/cli-tool/tsconfig.json (1)
9-16: Formatting normalization looks good.Single-line arrays for
excludeandpathsimprove readability when entries are short. Clean change.packages/ignix-mcp-server/.gitignore (1)
1-36: Solid .gitignore coverage for a Node.js package.All expected artifacts are covered: build output, dependencies, logs, IDE files, env secrets, and test coverage. Good foundation for the new package.
packages/cli-tool/src/utils/error-handler.ts (1)
2-13: Well-structured error extraction with proper type guards.
getErrorMessagehandles the common error shapes cleanly:Errorinstances, plain strings, and duck-typed objects. Good defensive programming.packages/ignix-mcp-server/src/tools/create-project.ts (1)
64-83: Good input validation pattern.Lowercase-hyphen validation for project names prevents filesystem issues and follows npm package naming conventions. Clear error messages with actionable guidance.
packages/ignix-mcp-server/tsup.config.ts (2)
3-21: Well-configured tsup for MCP SDK requirements.ESM format, Node 18 target, treeshake enabled, and clean builds. The inline comments explaining rationale (
MCP SDK requires ESM,keep false for debugging) are helpful for future maintainers.
14-16: The build script usestsc, not tsup—this configuration is unused.The
package.jsonbuild script is"tsc", not tsup. Since TypeScript compiler doesn't invoketsup.config.ts, the banner configuration here has no effect. The shebang insrc/index.ts(line 1) is correctly placed and will be preserved in the compiled output.Consider removing this unused configuration file or clarifying if there's a planned migration to tsup.
> Likely an incorrect or invalid review comment.packages/ignix-mcp-server/src/resources/docs-content.ts (1)
1-32: Clean, concise reference doc.Good structure: constraints first, tool list, example workflow, then good/bad patterns. Appropriate density for quick LLM reference without token bloat.
packages/ignix-mcp-server/src/resources/registry-summary.ts (2)
5-6: Correct ESM__dirnamepolyfill.Standard pattern for getting directory path in ESM modules. Good use of
fileURLToPath+path.dirname.
11-15: Good multi-path fallback strategy.Covering monorepo dev, standalone dev, and published package contexts ensures the registry is found in different execution environments.
packages/ignix-mcp-server/tsconfig.json (1)
1-24: Solid TypeScript configuration.Good choices here:
strict: truefor full type safety,NodeNextmodule resolution for proper ESM support, and declaration/sourcemap generation for debugging and library consumers. The path alias setup aligns with the repo pattern.packages/ignix-mcp-server/src/tools/generate-template.ts (1)
60-88: Nice Tailwind-first template design.Good call using utility classes exclusively with dark mode support and responsive container. The gradient text effect and button styling are production-ready patterns.
packages/ignix-mcp-server/src/tools/init-project.ts (1)
26-45: Good idempotent design.Checking for existing
ignix.config.jsbefore running init prevents duplicate work and provides clear feedback. This is exactly what the changelog promises.packages/ignix-mcp-server/package.json (2)
36-43: Dependencies look appropriate.Good selection:
@modelcontextprotocol/sdkfor MCP,fs-extrafor file operations,semverfor version handling. The^ranges allow patch/minor updates while maintaining stability.
7-9: Good setup—shebang is already in place.The
binentry points todist/index.js, andsrc/index.tscorrectly includes the#!/usr/bin/env nodeshebang for CLI execution. This is properly configured for publishing as an executable package.packages/ignix-mcp-server/mcp.json (4)
56-59: Tailwind CSS directives need proper formatting.The command writes
@tailwind base;@tailwindcomponents;@tailwindutilities;as a single line with semicolons between directives. The semicolons after each directive are correct, but concatenating them on one line works fine for Tailwind. However, the bigger issue is that thetailwind.config.jsgenerated bynpx tailwindcss init -pwon't include thecontentpaths, so Tailwind won't scan any files.Consider adding the content configuration or using a more robust setup approach:
🛠️ Suggested improvement
- "command": "cd {cwd} && npm create vite@latest {projectName} -- --template react-ts && cd {projectName} && npm install && npm install -D tailwindcss postcss autoprefixer && npx tailwindcss init -p && echo '@tailwind base; `@tailwind` components; `@tailwind` utilities;' > src/index.css" + "command": "cd {cwd} && npm create vite@latest {projectName} -- --template react-ts && cd {projectName} && npm install && npm install -D tailwindcss postcss autoprefixer && npx tailwindcss init -p && node -e \"const fs=require('fs'); fs.writeFileSync('tailwind.config.js', fs.readFileSync('tailwind.config.js','utf8').replace('content: [],', 'content: [\\\"./index.html\\\", \\\"./src/**/*.{js,ts,jsx,tsx}\\\"],'));\" && echo '@tailwind base;\\n@tailwind components;\\n@tailwind utilities;' > src/index.css"[raise_major_issue, request_verification]
What is the default Tailwind CSS 4.0 installation process for Vite React projects?
166-169:fetch()requires Node.js 18+ (not available in older LTS versions).The
get_component_docscommand uses globalfetch(), which was only stabilized in Node.js 18. If users run on Node 16 (still common in enterprise environments), this will fail withReferenceError: fetch is not defined.Consider adding a Node version check or using a more portable approach:
🛠️ Alternative using https module
- "command": "node -e \"fetch(`https://mindfiredigital.github.io/ignix-ui/api/docs/{component}.json`).then(r=>r.ok?r.json():Promise.reject()).then(docs=>console.log(JSON.stringify({success:true,component:'{component}',docs:{description:docs.description,props:docs.props,examples:docs.examples,usage:docs.usage},tokenCount:Math.ceil(JSON.stringify(docs).length/4)}))).catch(()=>console.log(JSON.stringify({success:false,error:'Documentation not found',classification:'DOCS_NOT_FOUND',action:'Use list_components to verify component exists'})))\"" + "command": "node -e \"const https=require('https'); https.get('https://mindfiredigital.github.io/ignix-ui/api/docs/{component}.json',res=>{let d='';res.on('data',c=>d+=c);res.on('end',()=>{if(res.statusCode===200){const docs=JSON.parse(d);console.log(JSON.stringify({success:true,component:'{component}',docs:{description:docs.description,props:docs.props,examples:docs.examples,usage:docs.usage},tokenCount:Math.ceil(d.length/4)}));}else{console.log(JSON.stringify({success:false,error:'Documentation not found',classification:'DOCS_NOT_FOUND',action:'Use list_components to verify component exists'}));}});}).on('error',()=>console.log(JSON.stringify({success:false,error:'Documentation not found',classification:'DOCS_NOT_FOUND',action:'Use list_components to verify component exists'})));\""[raise_minor_issue, request_verification]
What Node.js version introduced stable global fetch?
1-15: Good structure overall—clear metadata and capability declarations.Nice work on the
aiNative,discoveryFirstinstructions, and theevergreencapability flag. The "DISCOVER → PLAN → INSTALL → BUILD" workflow is well-documented and helps AI agents understand the intended usage pattern. TheintentDetectionandtoolChainingcapabilities are appropriate for this use case.Also applies to: 195-203
104-107: No changes needed — array handling is correct.The
componentsarray is properly converted to a space-separated string in the TypeScript implementation. Inadd-component.ts(lines 130–136), the handler joins the array with spaces (componentsList.join(' ')) before passing the command to the shell, so["button", "card"]becomesignix add component button card --yes --json. Themcp.jsonshell template is metadata; actual execution routes through the TypeScript tool handler, which ensures proper array serialization.> Likely an incorrect or invalid review comment.packages/cli-tool/src/types/index.ts (3)
14-15: Good use ofPartial<>intersection for flexible config.The
MCPClientConfig = Partial<CursorClaudeConfig & VSCodeConfig>pattern elegantly handles configs that may havemcpServers(Cursor/Claude) orservers(VS Code) or both. This matches the usage inmcp-init.tswhere existing configs are loaded and merged. Well-designed type!
48-88: Clean interface definitions for MCP runtime types.
ClientStatus,ToolExecution,MCPTool,MCPResource,MCPConfig, andAuditLogare well-structured with appropriate optional fields. Thestatusunion type'ok' | 'not-found' | 'outdated' | 'error'provides good type narrowing for status handling. Nice TypeScript patterns here.
111-115:⚠️ Potential issue | 🟠 MajorType mismatch with mcp.json tool schema.
AddComponentArgsusesname: string, but theadd_componenttool inmcp.json(line 90-97) expectscomponents: string[](an array). This inconsistency will cause runtime issues when the types are used to validate tool inputs.🛠️ Proposed fix to align with mcp.json schema
export interface AddComponentArgs { - name: string; - destination?: string; + components: string[]; cwd?: string; }⛔ Skipped due to learnings
Learnt from: PoonamKPatil Repo: mindfiredigital/ignix-ui PR: 723 File: apps/docs/versioned_docs/version-1.0.3/templates/pages/forms/contact-form.mdx:29-31 Timestamp: 2026-03-31T05:22:54.128Z Learning: In the ignix-ui project (mindfiredigital/ignix-ui), the CLI command `ignix add component <name>` is the unified command used to import both components AND templates. Do not flag `ignix add component` as incorrect when used in template documentation — it is intentional.
Comment on lines
+31
to
+34
| **You MUST do this EXACT sequence:** | ||
| 1. Check if `ignix.config.js` exists in current directory | ||
| 2. If NOT, call `add_component` with `cwd: "."` - this will auto-initialize | ||
| 3. Or call `init_project` if available |
Contributor
There was a problem hiding this comment.
Scenario 2 references capabilities the server does not expose.
This prompt tells the model to check for ignix.config.js and optionally call init_project, but the server surface shown in packages/ignix-mcp-server/src/index.ts:225-243 only dispatches create_react_project, list_components, add_component, generate_template, validate_ignix_only, and get_component_docs. That will force the assistant to invent a step or skip the documented flow. Keep Scenario 2 on add_component(..., cwd) auto-init unless a filesystem/init tool is actually shipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@llms.txt` around lines 31 - 34, The prompt incorrectly instructs calling
init_project which isn't part of the server API; update Scenario 2 to remove any
mention of init_project and instead rely solely on add_component with cwd to
auto-initialize when ignix.config.js is missing. Specifically, keep the check
for the presence of ignix.config.js, and if it's not present call add_component
with cwd: "." (do not attempt to call init_project), and avoid inventing steps
outside the documented server dispatch methods (create_react_project,
list_components, add_component, generate_template, validate_ignix_only,
get_component_docs).
Comment on lines
+28
to
+37
| function getConfigPath(client: Client): string { | ||
| const paths: Record<Client, string> = { | ||
| cursor: '.cursor/mcp.json', | ||
| vscode: '.vscode/mcp.json', | ||
| claude: '.mcp.json', | ||
| windsurf: '.windsurf/mcp.json', | ||
| codex: '', // Codex requires manual setup | ||
| }; | ||
| return paths[client]; | ||
| } |
Contributor
There was a problem hiding this comment.
Claude init writes to a different file than status reads.
Line 32 writes .mcp.json in the current project, but packages/cli-tool/src/commands/mcp-status.ts:23-52 looks for Claude Desktop’s OS-specific claude_desktop_config.json. mcp init --client claude will not produce a config that either Claude or mcp status can see.
Suggested fix
+function getClaudeConfigPath(): string {
+ if (process.platform === 'win32') {
+ return path.join(process.env.APPDATA || '', 'Claude', 'claude_desktop_config.json');
+ }
+ if (process.platform === 'darwin') {
+ return path.join(
+ process.env.HOME || '',
+ 'Library',
+ 'Application Support',
+ 'Claude',
+ 'claude_desktop_config.json'
+ );
+ }
+ return path.join(process.env.HOME || '', '.config', 'Claude', 'claude_desktop_config.json');
+}
+
function getConfigPath(client: Client): string {
const paths: Record<Client, string> = {
cursor: '.cursor/mcp.json',
vscode: '.vscode/mcp.json',
- claude: '.mcp.json',
+ claude: getClaudeConfigPath(),
windsurf: '.windsurf/mcp.json',
codex: '', // Codex requires manual setup
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-tool/src/commands/mcp-init.ts` around lines 28 - 37,
getConfigPath currently maps the 'claude' client to a local ".mcp.json", but
mcp-status expects Claude Desktop's OS-specific "claude_desktop_config.json";
update getConfigPath in getConfigPath(client: Client) so that for 'claude' it
returns the same OS-aware path that mcp-status reads (the
"claude_desktop_config.json" location), using the same helper or logic used in
packages/cli-tool/src/commands/mcp-status.ts to determine the OS-specific config
file path, so mcp init --client claude writes to the exact file mcp status and
Claude Desktop will find.
Comment on lines
56
to
+60
| .action(async (opts) => { | ||
| const client: Client = opts.client; | ||
|
|
||
| let configPath = ''; | ||
|
|
||
| switch (client) { | ||
| case 'cursor': | ||
| configPath = path.resolve('.cursor/mcp.json'); | ||
| break; | ||
| case 'vscode': | ||
| configPath = path.resolve('.vscode/mcp.json'); | ||
| break; | ||
| case 'claude': | ||
| configPath = path.resolve('.mcp.json'); | ||
| break; | ||
| case 'codex': | ||
| console.log('\n⚠️ Codex requires manual setup.\n'); | ||
| console.log(`[mcp_servers.ignix] | ||
| command = "npx" | ||
| args = ["ignix", "mcp"]\n`); | ||
| return; | ||
| default: | ||
| console.error('❌ Unsupported MCP client:', client); | ||
| process.exit(1); | ||
| if (process.env.MCP_INIT_RUNNING) { | ||
| return; | ||
| } | ||
|
|
||
| // ========================= | ||
| // 📦 PACKAGE.JSON | ||
| // ========================= | ||
| const packageJsonPath = path.resolve('package.json'); | ||
|
|
||
| let packageJson: PackageJson = {}; | ||
| let shouldInstall = false; | ||
|
|
||
| const exists = await fs.exists(packageJsonPath); | ||
|
|
||
| if (!exists) { | ||
| packageJson = { | ||
| name: path.basename(process.cwd()), | ||
| private: true, | ||
| dependencies: {}, | ||
| }; | ||
| shouldInstall = true; | ||
| } else { | ||
| try { | ||
| packageJson = await fs.readJSON(packageJsonPath); | ||
| } catch { | ||
| packageJson = {}; | ||
| } | ||
| process.env.MCP_INIT_RUNNING = 'true'; |
Contributor
There was a problem hiding this comment.
Reset MCP_INIT_RUNNING after non-dry-run executions.
The guard is only cleared in the --dry-run branch. After one successful init in the same interactive process, later calls return immediately at Line 57 because MCP_INIT_RUNNING is still set. Wrap the action body in try/finally.
Suggested fix
.action(async (opts) => {
if (process.env.MCP_INIT_RUNNING) {
return;
}
process.env.MCP_INIT_RUNNING = 'true';
-
- // existing body...
+ try {
+ // existing body...
+ } finally {
+ delete process.env.MCP_INIT_RUNNING;
+ }
});Also applies to: 103-128, 159-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-tool/src/commands/mcp-init.ts` around lines 56 - 60, The action
handler that sets the guard process.env.MCP_INIT_RUNNING should ensure the flag
is cleared after the action finishes; wrap the body of the .action(async (opts)
=> { ... }) handler in a try/finally so that regardless of success/error the env
var is removed (use delete process.env.MCP_INIT_RUNNING or set to '' in the
finally). Apply the same try/finally pattern to the other guarded blocks
referenced (the ones around the --dry-run branch and the other guarded sections)
so the guard does not persist across subsequent calls in the same interactive
process.
Comment on lines
+143
to
+148
| if (!configPath) { | ||
| if (client === 'codex') { | ||
| console.log(chalk.yellow(`\n⚠️ Codex requires manual setup:`)); | ||
| console.log( | ||
| chalk.gray(`[mcp_servers.ignix]\ncommand = "npx"\nargs = ["-y", "ignix", "mcp"]\n`) | ||
| ); |
Contributor
There was a problem hiding this comment.
Codex instructions still print the removed ignix mcp entrypoint.
Every generated config now uses npx -y @mindfiredigital/ignix-mcp-server@..., but the Codex message still tells users to configure args = ["-y", "ignix", "mcp"]. Following that output produces the wrong server entry.
Suggested fix
if (client === 'codex') {
console.log(chalk.yellow(`\n⚠️ Codex requires manual setup:`));
console.log(
- chalk.gray(`[mcp_servers.ignix]\ncommand = "npx"\nargs = ["-y", "ignix", "mcp"]\n`)
+ chalk.gray(
+ `[mcp_servers.ignix]\ncommand = "npx"\nargs = ["-y", "@mindfiredigital/ignix-mcp-server@${versionPin}"]\n`
+ )
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!configPath) { | |
| if (client === 'codex') { | |
| console.log(chalk.yellow(`\n⚠️ Codex requires manual setup:`)); | |
| console.log( | |
| chalk.gray(`[mcp_servers.ignix]\ncommand = "npx"\nargs = ["-y", "ignix", "mcp"]\n`) | |
| ); | |
| if (!configPath) { | |
| if (client === 'codex') { | |
| console.log(chalk.yellow(`\n⚠️ Codex requires manual setup:`)); | |
| console.log( | |
| chalk.gray( | |
| `[mcp_servers.ignix]\ncommand = "npx"\nargs = ["-y", "@mindfiredigital/ignix-mcp-server@${versionPin}"]\n` | |
| ) | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-tool/src/commands/mcp-init.ts` around lines 143 - 148, Update
the Codex manual setup message in the mcp-init.ts branch where you check if
(!configPath) and client === 'codex' — replace the outdated args = ["-y",
"ignix", "mcp"] guidance with the current entrypoint (npx -y
`@mindfiredigital/ignix-mcp-server`@<version>) so the printed [mcp_servers.ignix]
example reflects the new package name and invocation; ensure the console.log
lines that emit the Codex block show the correct npx package and args format
consistent with the generated config.
Comment on lines
+176
to
+181
| const newConfig = { | ||
| ...existingConfig, | ||
| ...getConfigContent(client, server), | ||
| }; | ||
|
|
||
| await fs.writeJSON(fullPath, newConfig, { spaces: 2 }); |
Contributor
There was a problem hiding this comment.
Do not overwrite other MCP servers when updating the config.
This shallow spread replaces the entire servers/mcpServers object with { ignix: ... }. If a user already has other MCP servers configured, mcp init deletes them. Merge the nested map instead.
Suggested fix
- const newConfig = {
- ...existingConfig,
- ...getConfigContent(client, server),
- };
+ const newConfig =
+ client === 'vscode'
+ ? {
+ ...existingConfig,
+ servers: {
+ ...(existingConfig.servers || {}),
+ ignix: server,
+ },
+ }
+ : {
+ ...existingConfig,
+ mcpServers: {
+ ...(existingConfig.mcpServers || {}),
+ ignix: server,
+ },
+ };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-tool/src/commands/mcp-init.ts` around lines 176 - 181, The
current shallow merge that builds newConfig (spreading existingConfig and
getConfigContent(client, server)) overwrites the entire servers/mcpServers map
and can delete other MCP entries; instead perform a nested merge for those maps:
build newConfig by spreading existingConfig and the new content but explicitly
merge existingConfig.servers (and/or existingConfig.mcpServers) with the
servers/mcpServers returned by getConfigContent(client, server) so you preserve
existing keys while adding/updating the ignix entry before calling
fs.writeJSON(fullPath, newConfig, { spaces: 2 }).
Comment on lines
+11
to
+13
| // /@radix-ui/, | ||
| /@headlessui\/react/, | ||
| // /framer-motion/, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove or document commented-out patterns.
@radix-ui and framer-motion are commented out without explanation. If intentionally allowed (e.g., Ignix uses them internally), add a comment. Otherwise, remove the dead code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignix-mcp-server/src/tools/validate-ignix-only.ts` around lines 11 -
13, The commented regex entries `/@radix-ui/` and `/framer-motion/` in the
patterns list are dead code—either remove them or add a concise inline comment
explaining why they are intentionally excluded/allowed (for example, "Ignix uses
`@radix-ui` internally; kept intentionally allowed") so future readers understand
the rationale; update the patterns array in validate-ignix-only.ts (the
commented lines near the other regex entries) accordingly and ensure any
explanation mentions whether they are intentionally permitted or should be
permanently removed.
| return violations; | ||
| } | ||
|
|
||
| async function scanDirectory(dir: string, basePath: string): Promise<string[]> { |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Unused basePath parameter.
The basePath parameter is passed recursively but never used. Remove it to avoid confusion:
-async function scanDirectory(dir: string, basePath: string): Promise<string[]> {
+async function scanDirectory(dir: string): Promise<string[]> {And update the recursive call on line 52 and initial call on line 89.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignix-mcp-server/src/tools/validate-ignix-only.ts` at line 41, The
function scanDirectory currently declares an unused parameter basePath; remove
basePath from the scanDirectory signature and from every call site (including
the recursive invocation inside scanDirectory and the initial invocation
elsewhere) so the function only accepts the necessary dir: string and returns
Promise<string[]>; update references to scanDirectory(...) accordingly (look for
the scanDirectory function and its recursive call and the initial call that
currently passes basePath) to eliminate the unused parameter.
Comment on lines
+45
to
+58
| for (const file of files) { | ||
| const filePath = path.join(dir, file); | ||
| const stat = await fs.stat(filePath); | ||
|
|
||
| if (stat.isDirectory()) { | ||
| // Skip node_modules and other common directories | ||
| if (!['node_modules', '.next', 'dist', 'build', '.git', 'coverage'].includes(file)) { | ||
| const subViolations = await scanDirectory(filePath, basePath); | ||
| violations.push(...subViolations); | ||
| } | ||
| } else if (/\.(tsx?|jsx?)$/.test(file)) { | ||
| const fileViolations = await scanFile(filePath); | ||
| violations.push(...fileViolations); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider parallel file scanning for larger codebases.
Sequential await in the loop processes files one at a time. For large projects, Promise.all would improve performance:
const fileResults = await Promise.all(
files.map(async (file) => {
// ... process file
})
);Minor optimization, but worth considering for the "30 requests/minute" rate-limited context mentioned in the changelog.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignix-mcp-server/src/tools/validate-ignix-only.ts` around lines 45 -
58, The for-await loop that processes files sequentially in scanDirectory
(iterating over files and calling scanDirectory and scanFile with await) is slow
for large repos; refactor to process directory entries in parallel by mapping
files to async worker tasks and awaiting Promise.all on that array. Keep the
same exclusion check for ['node_modules', '.next', 'dist', 'build', '.git',
'coverage'] and preserve calling scanDirectory(filePath, basePath) for
directories and scanFile(filePath) for files, but return or flatten the combined
results from Promise.all instead of pushing into a shared violations array to
avoid race conditions.
| return violations; | ||
| } | ||
|
|
||
| export async function validateIgnixOnlyTool(args: any) { |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type the args parameter.
interface ValidateIgnixOnlyArgs {
path?: string;
cwd?: string;
}
export async function validateIgnixOnlyTool(args: ValidateIgnixOnlyArgs) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignix-mcp-server/src/tools/validate-ignix-only.ts` at line 64, The
function validateIgnixOnlyTool currently types its parameter as any; define and
use a proper interface (e.g., ValidateIgnixOnlyArgs with optional path?: string
and cwd?: string) and change the function signature to accept args:
ValidateIgnixOnlyArgs so callers and IDEs get proper typing; update any related
imports/exports if necessary to expose the interface where required and adjust
internal usages of args.path/args.cwd to match the typed properties.
Comment on lines
+70
to
+87
| if (!(await fs.pathExists(fullPath))) { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: JSON.stringify( | ||
| { | ||
| success: true, | ||
| valid: true, | ||
| message: `⚠️ Path "${scanPath}" not found. No validation performed.`, | ||
| }, | ||
| null, | ||
| 2 | ||
| ), | ||
| }, | ||
| ], | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Returning success when path doesn't exist is misleading.
A missing src directory means nothing was validated, yet the response shows success: true, valid: true. This could mask configuration errors. Consider returning a warning or error classification:
classification: 'PATH_NOT_FOUND',
action: 'Ensure the project has a src directory with React components.'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignix-mcp-server/src/tools/validate-ignix-only.ts` around lines 70 -
87, The current early-return when !(await fs.pathExists(fullPath)) incorrectly
reports success and valid as true; update the returned object in that check (the
block referencing fullPath and scanPath) to reflect a non-successful validation
by setting success: false and valid: false and include classification:
'PATH_NOT_FOUND' and an action string like 'Ensure the project has a src
directory with React components.' so callers can distinguish a missing-path
condition from a successful validation.
Comment on lines
+1
to
+46
| export interface MCPServerConfig { | ||
| command: string; | ||
| args: string[]; | ||
| } | ||
|
|
||
| export interface CursorClaudeConfig { | ||
| mcpServers: Record<string, MCPServerConfig>; | ||
| } | ||
|
|
||
| export interface VSCodeConfig { | ||
| servers: Record<string, MCPServerConfig>; | ||
| } | ||
|
|
||
| // Add this flexible config type | ||
| export type MCPClientConfig = Partial<CursorClaudeConfig & VSCodeConfig>; | ||
|
|
||
| export interface PackageJson { | ||
| name?: string; | ||
| private?: boolean; | ||
| dependencies?: Record<string, string>; | ||
| devDependencies?: Record<string, string>; | ||
| } | ||
|
|
||
| export interface NpmRegistryResponse { | ||
| version: string; | ||
| } | ||
|
|
||
| export interface MCPServerConfig { | ||
| command: string; | ||
| args: string[]; | ||
| } | ||
|
|
||
| export interface CursorClaudeConfig { | ||
| mcpServers: Record<string, MCPServerConfig>; | ||
| } | ||
|
|
||
| export interface VSCodeConfig { | ||
| servers: Record<string, MCPServerConfig>; | ||
| } | ||
|
|
||
| export interface PackageJson { | ||
| name?: string; | ||
| private?: boolean; | ||
| dependencies?: Record<string, string>; | ||
| devDependencies?: Record<string, string>; | ||
| } |
Contributor
There was a problem hiding this comment.
Critical: Duplicate interface definitions will cause TypeScript errors.
The following interfaces are defined twice in this file:
MCPServerConfig(lines 1-4 and 28-31)CursorClaudeConfig(lines 6-8 and 33-35)VSCodeConfig(lines 10-12 and 37-39)PackageJson(lines 17-22 and 41-46)
TypeScript will throw TS2300: Duplicate identifier errors. Remove the duplicate declarations (lines 28-46).
🐛 Proposed fix—remove duplicates
export interface NpmRegistryResponse {
version: string;
}
-export interface MCPServerConfig {
- command: string;
- args: string[];
-}
-
-export interface CursorClaudeConfig {
- mcpServers: Record<string, MCPServerConfig>;
-}
-
-export interface VSCodeConfig {
- servers: Record<string, MCPServerConfig>;
-}
-
-export interface PackageJson {
- name?: string;
- private?: boolean;
- dependencies?: Record<string, string>;
- devDependencies?: Record<string, string>;
-}
-
export interface ClientStatus {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli-tool/src/types/index.ts` around lines 1 - 46, This file contains
duplicate TypeScript interface declarations causing TS2300 errors; remove the
redundant second block and keep a single definition for MCPServerConfig,
CursorClaudeConfig, VSCodeConfig, and PackageJson (ensure the existing
MCPClientConfig still references the single CursorClaudeConfig & VSCodeConfig),
so delete the duplicate declarations (the repeated MCPServerConfig,
CursorClaudeConfig, VSCodeConfig, PackageJson) and keep the original/exported
versions to restore unique type identifiers.
Comment on lines
+172
to
+177
| "resources": [ | ||
| { | ||
| "name": "component_registry", | ||
| "uri": "file://../../packages/registry/registry.json", | ||
| "description": "Machine-readable component registry containing all available components" | ||
| }, |
Contributor
There was a problem hiding this comment.
Relative file URI may break depending on working directory.
The component_registry resource uses file://../../packages/registry/registry.json. This relative path assumes a specific directory structure and working directory context. If the MCP server is invoked from a different location (e.g., when installed globally via npm), this path won't resolve correctly.
Consider using an absolute path resolved at runtime or embedding the registry lookup in the server code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ignix-mcp-server/mcp.json` around lines 172 - 177, The
"component_registry" resource in mcp.json uses a relative "uri"
("file://../../packages/registry/registry.json") which can break depending on
working directory; change this by either (A) making the "uri" a runtime-resolved
absolute path (compute path.resolve(__dirname,
'../../packages/registry/registry.json') in your startup code and inject the
absolute file:// URI into the manifest before serving), or (B) remove the
file:// URI and instead load/parse registry.json from the server startup code
and embed its contents into the MCP manifest under the "component_registry"
resource; update the code that reads mcp.json to accept the injected/embedded
registry so "component_registry" no longer depends on a fragile relative path.
anandmindfire
approved these changes
Apr 17, 2026
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.
Pull Request Title
Fix MCP config loading and remove unnecessary test case
Description
What does this PR do?
This PR fixes issues related to loading the mcp.json configuration and improves the CLI setup. The changes include:
Type of Change
Checklist
npm run lint.New components or component API changes
Bug fixes
Tooling / config changes
Docs / Storybook updates
Test updates
Breaking changes