Configure proper nx mcp and make AGENTS.md less verbose#34154
Configure proper nx mcp and make AGENTS.md less verbose#34154valentinpalkovic wants to merge 3 commits into
Conversation
|
valentinpalkovic
force-pushed
the
valentin/configure-proper-nx-mcps
branch
from
a063ee4 to
981bc57
Compare
📝 WalkthroughWalkthroughRenames and reconfigures the NX MCP server to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
📝 Coding Plan
Comment Tip You can get early access to new features in CodeRabbit.Enable the |
valentinpalkovic
force-pushed
the
valentin/configure-proper-nx-mcps
branch
2 times, most recently
from
f31d40c to
36e73ac
Compare
valentinpalkovic
force-pushed
the
valentin/configure-proper-nx-mcps
branch
from
36e73ac to
381abcf
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 @.codex/config.toml:
- Line 3: Update the args array so the nx mcp invocation does not include the
unsupported flag: remove the "--project" token from the args value (currently
args = ["nx", "mcp", "--project"]) and leave it as ["nx", "mcp"] so the MCP
command matches the other configs and will start successfully.
In `@AGENTS.md`:
- Line 165: Update the /bin/* guidance in AGENTS.md so it is OS-aware: change
the blanket "always use `/bin/*`" rule to explicitly scope it to POSIX shells
(e.g., Linux/macOS) and add a Windows-safe fallback note explaining that
absolute POSIX paths may not exist in Windows shells and recommending Windows
alternatives (PowerShell copy cmdlets or fully-qualified executable paths) or a
cross-platform approach; edit the sentence containing `/bin/*` to mention
POSIX-only and append a short Windows fallback paragraph so readers on both
platforms get correct, actionable guidance.
In `@code/project.json`:
- Around line 13-20: The lint-js target currently sets "cache": true which can
cause --fix runs to be skipped on cache hits; update the lint-js target (the
object named "lint-js" that uses executor "nx:run-commands" and command "yarn
lint:js:cmd") to disable caching by setting "cache": false, or add a new target
(e.g., "lint-js-fix") that runs the same command but with "cache": false so fix
operations always execute file mutations while leaving the original cached lint
target intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7539c899-7856-4dc2-aa57-03c4a05ea609
📒 Files selected for processing (6)
.codex/config.toml.cursor/mcp.json.mcp.json.vscode/mcp.jsonAGENTS.mdcode/project.json
| @@ -0,0 +1,3 @@ | |||
| [mcp_servers."nx-mcp"] | |||
| command = "npx" | |||
| args = ["nx", "mcp", "--project"] No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the valid CLI options for nx mcpin current Nx docs, and does--project require an explicit value?
💡 Result:
From the current Nx docs, nx mcp supports these CLI options: [workspacePath] (alias -w), --transport <type>, --port <port> (alias -p), --tools <patterns...> (alias -t), --minimal / --no-minimal, --disableTelemetry, --debugLogs, and --help. [1]
--project is not a valid option for nx mcp in the current docs, so it doesn’t “require a value” — it’s simply unsupported for this command. [1]
Remove unsupported --project flag from nx mcp invocation.
The --project flag is not a valid option for nx mcp according to Nx documentation. This configuration will fail at startup. Remove --project to match the other MCP configs in this PR, leaving just ["nx", "mcp"].
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.codex/config.toml at line 3, Update the args array so the nx mcp invocation
does not include the unsupported flag: remove the "--project" token from the
args value (currently args = ["nx", "mcp", "--project"]) and leave it as ["nx",
"mcp"] so the MCP command matches the other configs and will start successfully.
| 3. **Always** delete the sandbox cache: `rm -rf ../storybook-sandboxes/<template>/node_modules/.cache` | ||
| 4. Restart the sandbox Storybook | ||
|
|
||
| **Important:** Use `/bin/cp` (not bare `cp`) to bypass shell aliases like `cp -i` that prompt for confirmation and block non-interactive execution. |
There was a problem hiding this comment.
Make shell-path guidance OS-aware.
Line 230’s “always use /bin/*” rule conflicts with the stated Linux+Windows environment (Line 14). These absolute POSIX paths can fail in Windows shells. Please scope this rule to POSIX shells and add a Windows-safe fallback note.
As per coding guidelines: **/AGENTS.md: Document agent capabilities and limitations.
Also applies to: 230-230
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 165, Update the /bin/* guidance in AGENTS.md so it is
OS-aware: change the blanket "always use `/bin/*`" rule to explicitly scope it
to POSIX shells (e.g., Linux/macOS) and add a Windows-safe fallback note
explaining that absolute POSIX paths may not exist in Windows shells and
recommending Windows alternatives (PowerShell copy cmdlets or fully-qualified
executable paths) or a cross-platform approach; edit the sentence containing
`/bin/*` to mention POSIX-only and append a short Windows fallback paragraph so
readers on both platforms get correct, actionable guidance.
| "lint-js": { | ||
| "dependsOn": [{ "projects": ["*"], "target": "compile" }], | ||
| "executor": "nx:run-commands", | ||
| "options": { "cwd": "{projectRoot}", "command": "yarn lint:js:cmd" }, | ||
| "cache": true, | ||
| "inputs": ["default"], | ||
| "configurations": { "production": {} } | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Nx run-commands, are file-mutating tasks (like lint with --fix) recommended to run with cache: false to avoid cache replay skipping side effects?
💡 Result:
Yes—Nx’s guidance is that only side‑effect‑free tasks should be cacheable, because a cached run can be replayed (restored from cache) instead of executing the command. For tasks that mutate source files (e.g., eslint --fix / nx lint --fix), replaying from cache would skip actually applying those edits, so you generally should disable Nx caching for that target (e.g., set the target’s "cache": false) or run it with --skipNxCache. [1][2]
Rule of thumb: caching is fine when the “side effects” are only writing declared outputs (e.g., dist/), but not when the task edits tracked inputs like *.ts files (which --fix can do). [1][3]
Sources
[1] Nx docs: “Cacheable operations need to be side effect free” — Cache Task Results https://nx.dev/docs/features/cache-task-results
[2] Nx docs: configure caching per-target via "cache": true|false — nx.json reference https://nx.dev/docs/reference/nx-json
[3] Nx docs: ESLint fix “may overwrite linted files” — @nx/eslint executor options https://nx.dev/docs/technologies/eslint/executors
Remove cache from lint-js target or split into separate cached/uncached targets.
Nx does not replay cached results for file-mutating tasks. With cache: true, runs using --fix will skip actual file rewrites on cache hits. The ESLint executor recommends disabling cache for fix operations. Either set "cache": false or create a separate lint-js-fix target with caching disabled.
Suggested safe split
"lint-js": {
"dependsOn": [{ "projects": ["*"], "target": "compile" }],
"executor": "nx:run-commands",
"options": { "cwd": "{projectRoot}", "command": "yarn lint:js:cmd" },
"cache": true,
"inputs": ["default"],
"configurations": { "production": {} }
},
+ "lint-js-fix": {
+ "dependsOn": [{ "projects": ["*"], "target": "compile" }],
+ "executor": "nx:run-commands",
+ "options": { "cwd": "{projectRoot}", "command": "yarn lint:js:cmd --fix" },
+ "cache": false,
+ "inputs": ["default"],
+ "configurations": { "production": {} }
+ },📝 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.
| "lint-js": { | |
| "dependsOn": [{ "projects": ["*"], "target": "compile" }], | |
| "executor": "nx:run-commands", | |
| "options": { "cwd": "{projectRoot}", "command": "yarn lint:js:cmd" }, | |
| "cache": true, | |
| "inputs": ["default"], | |
| "configurations": { "production": {} } | |
| }, | |
| "lint-js": { | |
| "dependsOn": [{ "projects": ["*"], "target": "compile" }], | |
| "executor": "nx:run-commands", | |
| "options": { "cwd": "{projectRoot}", "command": "yarn lint:js:cmd" }, | |
| "cache": true, | |
| "inputs": ["default"], | |
| "configurations": { "production": {} } | |
| }, | |
| "lint-js-fix": { | |
| "dependsOn": [{ "projects": ["*"], "target": "compile" }], | |
| "executor": "nx:run-commands", | |
| "options": { "cwd": "{projectRoot}", "command": "yarn lint:js:cmd --fix" }, | |
| "cache": false, | |
| "inputs": ["default"], | |
| "configurations": { "production": {} } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@code/project.json` around lines 13 - 20, The lint-js target currently sets
"cache": true which can cause --fix runs to be skipped on cache hits; update the
lint-js target (the object named "lint-js" that uses executor "nx:run-commands"
and command "yarn lint:js:cmd") to disable caching by setting "cache": false, or
add a new target (e.g., "lint-js-fix") that runs the same command but with
"cache": false so fix operations always execute file mutations while leaving the
original cached lint target intact.
…dbox-rebuild skill documentation in AGENTS.md
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
AGENTS.md (1)
222-222:⚠️ Potential issue | 🟡 MinorKeep shell guidance OS-aware (still unresolved).
This still reads as a universal rule, but
/bin/*paths are POSIX-specific and conflict with the Linux+Windows environment note. Please scope to POSIX shells and add Windows alternatives.As per coding guidelines:
**/AGENTS.md: Document agent capabilities and limitations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AGENTS.md` at line 222, Update the "Interactive prompts:" guidance to be OS-aware by scoping the `/bin/*` recommendation to POSIX shells only and adding Windows equivalents; specifically modify the paragraph that begins with "**Interactive prompts:**" to mention "POSIX shells (e.g., Linux/macOS): use full binary paths like /bin/cp, /bin/mv, /bin/rm to bypass aliases" and append a sentence for Windows explaining to use the full executable path or built-in commands (e.g., C:\Windows\System32\copy, move, del or PowerShell cmdlets) or run commands via cmd.exe/PowerShell to avoid interactive aliases/prompts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/sandbox-rebuild/SKILL.md:
- Around line 28-29: Update the guidance text that currently recommends using
“/bin/cp” and “/bin/rm” so it explicitly states those paths are POSIX-only
(Linux/macOS shells) and will not exist on Windows, and add Windows-safe
alternatives: for cmd.exe use “copy” and “del”, for PowerShell use “Copy-Item”
and “Remove-Item”, or recommend using cross-platform tooling (e.g., Node fs,
rsync, or a platform-aware script) as a fallback; change the literal occurrences
of “/bin/cp” and “/bin/rm” and the adjacent guidance lines to reflect this
POSIX-only note plus the Windows alternatives.
- Line 20: The instruction text in SKILL.md currently contains a
machine-specific absolute path ("/Users/valentinpalkovic/Projects/storybook");
replace that path with a generic phrase such as "run from the repository root"
and keep the existing placeholder guidance for "<package-name>" (examples:
"builder-vite", "core", "react-vite") so the line reads something like: run from
the repository root and replace "<package-name>" with the NX project name — this
removes the user-local path and makes the skill portable.
In `@AGENTS.md`:
- Around line 166-167: Update the documented commands in AGENTS.md to the
preferred yarn syntax and remove redundant caching flags: replace the test
command "yarn nx run code:test --changed" with "cd code && yarn test" and
replace the lint command "yarn nx run code:lint-js
<path-relative-to-code-folder-of-changed-files> --cache --fix" with "yarn --cwd
code lint:js:cmd <path-relative-to-code-folder-of-changed-files> --fix"; also
remove any other occurrences of the redundant "--cache" flag in these command
examples so they match the contributor guidelines.
---
Duplicate comments:
In `@AGENTS.md`:
- Line 222: Update the "Interactive prompts:" guidance to be OS-aware by scoping
the `/bin/*` recommendation to POSIX shells only and adding Windows equivalents;
specifically modify the paragraph that begins with "**Interactive prompts:**" to
mention "POSIX shells (e.g., Linux/macOS): use full binary paths like /bin/cp,
/bin/mv, /bin/rm to bypass aliases" and append a sentence for Windows explaining
to use the full executable path or built-in commands (e.g.,
C:\Windows\System32\copy, move, del or PowerShell cmdlets) or run commands via
cmd.exe/PowerShell to avoid interactive aliases/prompts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95a65bbb-9dc6-4850-b249-73f48905fb1e
📒 Files selected for processing (16)
.agents/skills/canary/SKILL.md.agents/skills/github-qa-labels/SKILL.md.agents/skills/pr/SKILL.md.agents/skills/sandbox-rebuild/SKILL.md.agents/skills/storybook-upgrade/SKILL.md.claude/skills/canary/SKILL.md.claude/skills/canary/SKILL.md.claude/skills/github-qa-labels/SKILL.md.claude/skills/github-qa-labels/SKILL.md.claude/skills/pr/SKILL.md.claude/skills/pr/SKILL.md.claude/skills/sandbox-rebuild/SKILL.md.claude/skills/storybook-upgrade/SKILL.md.claude/skills/storybook-upgrade/SKILL.md.gitignoreAGENTS.md
💤 Files with no reviewable changes (4)
- .agents/skills/pr/SKILL.md
- .agents/skills/storybook-upgrade/SKILL.md
- .agents/skills/github-qa-labels/SKILL.md
- .agents/skills/canary/SKILL.md
✅ Files skipped from review due to trivial changes (9)
- .claude/skills/sandbox-rebuild/SKILL.md
- .claude/skills/pr/SKILL.md
- .claude/skills/canary/SKILL.md
- .claude/skills/pr/SKILL.md
- .claude/skills/storybook-upgrade/SKILL.md
- .claude/skills/canary/SKILL.md
- .claude/skills/storybook-upgrade/SKILL.md
- .claude/skills/github-qa-labels/SKILL.md
- .claude/skills/github-qa-labels/SKILL.md
| yarn nx compile <package-name> | ||
| ``` | ||
|
|
||
| Run from the repo root (`/Users/valentinpalkovic/Projects/storybook`). Replace `<package-name>` with the NX project name (e.g. `builder-vite`, `core`, `react-vite`). |
There was a problem hiding this comment.
Remove machine-specific absolute path from instructions.
/Users/valentinpalkovic/Projects/storybook makes the skill non-portable. Please switch to a generic instruction like “run from the repository root” and avoid user-local paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/sandbox-rebuild/SKILL.md at line 20, The instruction text in
SKILL.md currently contains a machine-specific absolute path
("/Users/valentinpalkovic/Projects/storybook"); replace that path with a generic
phrase such as "run from the repository root" and keep the existing placeholder
guidance for "<package-name>" (examples: "builder-vite", "core", "react-vite")
so the line reads something like: run from the repository root and replace
"<package-name>" with the NX project name — this removes the user-local path and
makes the skill portable.
| Always use `/bin/cp` (not bare `cp`) to avoid shell alias prompts that block execution. | ||
|
|
There was a problem hiding this comment.
Make /bin/* guidance explicitly POSIX-only and add a Windows fallback.
The current wording implies /bin/cp and /bin/rm are universal, but these paths are not available in typical Windows shells. Please scope this to Linux/macOS shells and add Windows-safe alternatives.
Also applies to: 49-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/skills/sandbox-rebuild/SKILL.md around lines 28 - 29, Update the
guidance text that currently recommends using “/bin/cp” and “/bin/rm” so it
explicitly states those paths are POSIX-only (Linux/macOS shells) and will not
exist on Windows, and add Windows-safe alternatives: for cmd.exe use “copy” and
“del”, for PowerShell use “Copy-Item” and “Remove-Item”, or recommend using
cross-platform tooling (e.g., Node fs, rsync, or a platform-aware script) as a
fallback; change the literal occurrences of “/bin/cp” and “/bin/rm” and the
adjacent guidance lines to reflect this POSIX-only note plus the Windows
alternatives.
| 6. Run `yarn nx run code:test --changed` | ||
| 7. Run `yarn nx run code:lint-js <path-relative-to-code-folder-of-changed-files> --cache --fix` |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification of documented NX commands against workspace target definitions.
set -euo pipefail
echo "== Locate code/project.json (or equivalent) =="
fd -a 'project.json$|workspace.json$|nx.json$'
echo
echo "== Show `code` project target definitions for test and lint-js =="
fd -a 'project.json$' | xargs -I{} sh -c '
if rg -n "\"name\"\\s*:\\s*\"code\"|\"lint-js\"|\"test\"" "{}" >/dev/null 2>&1; then
echo "--- {} ---"
rg -n -C3 "\"lint-js\"|\"test\"|\"executor\"|\"options\"|\"configurations\"" "{}"
fi
'
echo
echo "== Search docs for the exact command patterns to keep guidance consistent =="
rg -n -C2 'nx run code:test --changed|nx run code:lint-js .*--cache --fix|run code:lint-js <' AGENTS.md .agents/skills/**/*.mdRepository: storybookjs/storybook
Length of output: 10565
🏁 Script executed:
cd code && cat -n project.json | head -30Repository: storybookjs/storybook
Length of output: 1282
Update documented commands to align with preferred yarn syntax and remove redundant flags.
The --cache flag in line 98 and 167 is redundant—caching is already configured in the target definitions. Per contributor guidelines, prefer the simpler approaches: use cd code && yarn test instead of yarn nx run code:test --changed, and yarn --cwd code lint:js:cmd <path> --fix instead of yarn nx run code:lint-js <path> --cache --fix for routine development work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 166 - 167, Update the documented commands in
AGENTS.md to the preferred yarn syntax and remove redundant caching flags:
replace the test command "yarn nx run code:test --changed" with "cd code && yarn
test" and replace the lint command "yarn nx run code:lint-js
<path-relative-to-code-folder-of-changed-files> --cache --fix" with "yarn --cwd
code lint:js:cmd <path-relative-to-code-folder-of-changed-files> --fix"; also
remove any other occurrences of the redundant "--cache" flag in these command
examples so they match the contributor guidelines.
2 participants
Closes #
What I did
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Caution
This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Documentation
New Features
Chores
Style