fix(dev): open inspector only in the process serving the app - #1388
Conversation
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nuxt-cli/src/commands/dev.ts (1)
160-166: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winClose the inspector on every terminal close path.
--no-fork/profile returnsclosedirectly, andcloseAllcan run before the first fork handoff; both leave the parent inspector listening after the dev server closes. Wrap terminal closes withcloseInspector()in afinallyblock.Also applies to: 223-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nuxt-cli/src/commands/dev.ts` around lines 160 - 166, Update the terminal close paths in the dev command, including the direct return around setupShortcuts and closeAll, so they always invoke closeInspector() in a finally block after closing the dev server. Ensure the inspector is closed when --no-fork/profile exits directly and when closeAll runs before the first fork handoff, while preserving the existing close behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nuxt-cli/src/commands/dev.ts`:
- Around line 36-43: Update the dev command option handling around the inspect
and inspect-brk definitions so inline Node inspector targets such as
--inspect=host:port remain available to resolveInspectOptions(ctx.rawArgs).
Either exclude these Node-style flags from citty parsing and read them from
unprocessed CLI/exec arguments, or configure the command to preserve unknown
options, while retaining normal boolean flag behavior when no inline value is
supplied.
In `@packages/nuxt-cli/src/dev/inspect.ts`:
- Around line 95-100: Update openInspector so it preserves an existing inspector
session when inspector.url() reports port, host, and wait settings compatible
with the requested options; do not call inspector.close() or reopen in that
case. Only close and invoke inspector.open when an explicit option override
changes the target, while retaining the existing native process.execArgv
configuration behavior.
---
Outside diff comments:
In `@packages/nuxt-cli/src/commands/dev.ts`:
- Around line 160-166: Update the terminal close paths in the dev command,
including the direct return around setupShortcuts and closeAll, so they always
invoke closeInspector() in a finally block after closing the dev server. Ensure
the inspector is closed when --no-fork/profile exits directly and when closeAll
runs before the first fork handoff, while preserving the existing close
behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: acb9e3db-334f-4ce4-b417-0dc5cb15fbce
📒 Files selected for processing (7)
packages/nuxt-cli/src/commands/dev.tspackages/nuxt-cli/src/dev/index.tspackages/nuxt-cli/src/dev/inspect.tspackages/nuxt-cli/src/dev/pool.tspackages/nuxt-cli/src/dev/utils.tspackages/nuxt-cli/test/unit/help.spec.tspackages/nuxt-cli/test/unit/inspect.spec.ts
| try { | ||
| if (inspector.url()) { | ||
| inspector.close() | ||
| } | ||
| // Node itself logs `Debugger listening on …` when the inspector opens. | ||
| inspector.open(options.port, options.host, options.wait) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
For supported Node.js versions, what occurs when inspector.close()followed byinspector.open(port, host, true)is called after the process was launched with--inspect-brk and execution has resumed?
💡 Result:
When a Node.js process is launched with the --inspect-brk flag, the V8 inspector is activated automatically, and the process pauses execution on the first line of code to wait for a debugger client to connect [1][2]. If you call inspector.close while the process is running, the inspector server is deactivated, and all active inspector connections are forcibly terminated [3][4]. Subsequently calling inspector.open(port, host, true) will restart the inspector on the specified port and host [3][5]. Because the wait parameter is set to true, the Node.js process will block execution at that point until a debugger client connects to the new inspector instance [3][6]. Once a client connects, the process will resume execution [3][5]. In summary, the sequence effectively re-initializes the inspector and forces the application to pause and wait for a new debugging session, effectively re-entering the "wait-for-debugger" state despite the process having already been running [3]. Top results: [3][5][4]
Citations:
- 1: https://nodejs.org/dist/latest/docs/api/debugger.html
- 2: https://github.com/nodejs/node/blob/main/doc/api/debugger.md
- 3: http://nodejs.org/api/inspector.html
- 4: https://nodejs.org/download/release/v18.20.7/docs/api/inspector.html
- 5: https://nodejs.org/docs/latest/api/inspector.html
- 6: https://nodejs.org/docs/latest-v24.x/api/inspector.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant file/module without executing repository code.
if [ -f packages/nuxt-cli/src/dev/inspect.ts ]; then
echo "== file exists =="
wc -l packages/nuxt-cli/src/dev/inspect.ts
echo
sed -n '1,140p' packages/nuxt-cli/src/dev/inspect.ts | cat -n
else
echo "== file missing =="
fd -a 'inspect\.ts$' .
fi
echo
echo "== related inspect argument references =="
rg -n "openInspector|resolveInspectOptions|process\.execArgv|\\-\\-inspect(-brk)?|inspect\.open|inspector\.close|wait" packages/nuxt-cli/src packages 2>/dev/null || true
echo
echo "== changed file stat =="
git diff --stat
echo
git diff -- packages/nuxt-cli/src/dev/inspect.ts | sed -n '1,240p'Repository: nuxt/cli
Length of output: 50365
🌐 Web query:
Node.js inspect.close deactivates inspector server inspector.clientClosedEvent on connections closed
💡 Result:
The inspector.close() method in Node.js is responsible for deactivating the inspector [1][2]. When called, it forcibly terminates any active connections and blocks execution until the inspector server has fully stopped [1][3]. Regarding your query about inspector.clientClosedEvent, there is no documented event by this exact name in the standard Node.js inspector module API [1][3]. The module's primary events are managed through the inspector.Session class, which emits close events when a session is disconnected [4]. In summary, inspector.close() deactivates the inspector and ensures all connections are terminated [1][3]. If you are looking for information on connection lifecycle events, you should refer to the inspector.Session class documentation, which handles connection-specific events such as inspectorNotification and session termination [4][5].
Citations:
- 1: http://nodejs.org/api/inspector.html
- 2: https://beta.docs.nodejs.org/inspector.html
- 3: https://nodejs.org/api/inspector.html
- 4: https://nodejs.org/download/release/v18.20.7/docs/api/inspector.html
- 5: https://docs.deno.com/api/node/inspector/
Preserve an existing native inspector session.
When the native process.execArgv inspection configuration wins, Node opens the inspector before openInspector() runs. Closing it inlines inspector.close() and then calling inspector.open(..., options.wait) deactivates existing connections and re-waits if options.wait is true. Preserve an existing inspect URL when its port, host, and wait settings already satisfy the requested options, and only reopen when an explicit override changes the target.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nuxt-cli/src/dev/inspect.ts` around lines 95 - 100, Update
openInspector so it preserves an existing inspector session when inspector.url()
reports port, host, and wait settings compatible with the requested options; do
not call inspector.close() or reopen in that case. Only close and invoke
inspector.open when an explicit option override changes the target, while
retaining the existing native process.execArgv configuration behavior.
a945895 to
51c2b8d
Compare
1 participant
🔗 Linked issue
resolves #1195
resolves #1369
📚 Description
this bundles some fixes for native
--inspectusage