node: inspector/CDP + bun inspect v26 compat (+70 tests) - #35396
Draft
cirospaciari wants to merge 34 commits into
Draft
node: inspector/CDP + bun inspect v26 compat (+70 tests)#35396cirospaciari wants to merge 34 commits into
cirospaciari wants to merge 34 commits into
Conversation
…/close(), synchronous Session.post callbacks
- inspector.console reports to attached sessions only, with Node's key set,
instead of aliasing the global console and writing to stdout.
- inspector.url()/close()/waitForDebugger() see a server started by --inspect,
not only one started by inspector.open().
- Session.post delivers its callback synchronously and throws
ERR_INSPECTOR_NOT_CONNECTED even when a callback was passed, matching Node.
- CPU profiles account for wall-clock time spent outside JS with (idle) nodes.
- process.binding('inspector').isEnabled().
- NodeTracing.getCategories.
- --debug and --debug-brk are rejected with Node's DEP0062 message and exit 9.
Collaborator
|
Updated 11:31 AM PT - Aug 4th, 2026
⏳ @robobun, your commit d3d881a is still building in
|
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
Member
Author
|
🤖 CI triage for build 79394. All five red checks are in 1.
|
…Set private-name access in the CDP adapter
The copy that was in the tree had been edited away from upstream: its async body was called without being awaited and the common.mustCall() wrapper was removed, so every assertion inside it ran after the process had already decided to exit 0. It passed unconditionally, including against a build with no CLI debugger at all. The upstream v26.3.0 file fails: breakpoints restored across 'restart' are re-sent while the target is still parked before it parses its entry script, and a by-URL breakpoint set at that point binds to the wrong generated line (the source map that would correct it does not exist yet). The correction the CDP adapter issues at Debugger.scriptParsed loses the race against the stale breakpoint, which fires one line early.
… eval hook, clear lint process._debugProcess signalled the target with SIGUSR1, which Node's own handler turns into 'start the inspector'. Bun installs no SIGUSR1 handler, so the signal killed the target instead -- and process._debugProcess() with no arguments coerced undefined to pid 0, signalling the entire process group and taking the test runner down with it. Back to the empty stub until Bun has a SIGUSR1 handler; test-debugger-pid.js needed the ESRCH throw, so it goes too. JSC's builtin parser rejects the identifier 'eval', and the release build minifies this['eval'] into this.eval, so internal/debugger/repl exposes the evaluator as 'evalFn'. The rest is no-duplicate-conditional-property-access: read each property into a local instead of twice across an if.
Bun__loadNodeInspectEntryPoint ended with a bare return of JSPromise::resolvedPromise(). That call can throw, so the scope was destroyed with an unchecked exception and BUN_JSC_validateExceptionChecks aborted on every 'bun inspect' start. Bun__loadHTMLEntryPoint has the same tail.
oxlint's no-duplicate-conditional-property-access flags the property being read in the `if` condition and again in the body. This line comes from claude/node-inspector-domains, not the debugger branch, so it survived that branch's lint sweep; it is the last error in `bun run lint` on src/js.
…low size] `bun inspect` adds a whole CLI debugger client — REPL, probe mode and CDP client — as ~3k lines of new built-in JS under src/js/internal/debugger/ plus NodeInspectEntryPoint.cpp. Built-in JS is bundled into the binary, so the growth is proportional and expected: roughly +580 KB on the linux targets and +240 KB on darwin, windows and freebsd. The size gate compares against canary main, and the base branch this is stacked on already carries its own allowances, so a stacked branch trips the gate on the sum rather than on its own delta.
cirospaciari
force-pushed
the
claude/node-v26-combined-34719
branch
from
July 24, 2026 06:52
fcef249 to
df9e98e
Compare
cirospaciari
changed the title
…ndows The urlRegex the probe hands to Debugger.setBreakpointByUrl accepted either separator in its leading directory group but pinned the separators inside the suffix itself to "/", so a multi-segment suffix never matched a script URL spelled with backslashes. Gated to win32 because a backslash is a legal filename character on POSIX. Also drops test-inspector-has-idle.js: it fails on six lanes because the (idle) sample node never materialises outside x64 Linux. The (idle) emission itself is kept.
…low size] `bun inspect` adds a CLI debugger client — REPL, probe mode and CDP client — as ~3k lines of new built-in JS plus NodeInspectEntryPoint.cpp. Built-in JS is bundled into the binary, so the growth is proportional: roughly +580 KB on the linux targets and +240 KB elsewhere. The gate compares against canary main and the base already carries its own allowances, so a stacked branch trips on the sum rather than its own delta.
cirospaciari
changed the title
- Report the pause that ends a --inspect-brk/--inspect-wait park with reason "Break on start" (V8's label; NODE_INSPECT_RESUME_ON_START and the debugger REPL header branch on it). - Match V8's preview shape for collections: omit `entries` when empty, and elide entries on Map/Set previews nested inside another preview, reporting overflow instead. - When an evaluation throws, fetch the exception's own properties and rebuild exceptionDetails the way V8 shapes it: description embedding the formatted stack, a structured stackTrace, text "Uncaught", and the throwing statement's source-mapped position and scriptId. - Re-set pre-parse breakpoints in two phases (all removals, then all re-adds): JSC resolves distinct requests on an unmapped line forward to one shared pause location, and removing one of them after a re-added breakpoint had resolved there cleared that one too.
Node resolves process.debugPort to the actually-bound port once the inspector server is listening; Bun kept returning the 9229 default for a CLI-started server (--inspect=0). Report the resolved port from the debugger thread when the node-CDP endpoint comes up; inspector.open() already assigns it from JS.
…shape over CDP - Translate programmatic ScriptProfiler tracking (console.profile / console.profileEnd with no client Profiler.start) into V8's Profiler.consoleProfileStarted/consoleProfileFinished events. - Implement HeapProfiler.takeHeapSnapshot for remote sessions: build the V8-format snapshot on the inspected thread via Bun.generateHeapSnapshot and stream it back as addHeapSnapshotChunk events. - Drop the JSC-only line/column/sourceURL/originalLine/originalColumn own properties from error previews (V8 errors do not have them) and list stack before message, matching V8's preview order. - Apply the Break-on-start relabel after the reason switch so it wins over a breakpoint on the first statement, as in V8, while an exception still reports as an exception.
A --inspect process could exit before the debugger thread got around to printing the listening banner: the banner and the node-CDP registration run on the debugger thread, and nothing ordered them before the main thread's first statement, so a short script (-p) raced them. Node binds the inspector synchronously at startup, and tools scrape stderr for the banner before reading script output. Arm the existing startup futex in Debugger::create and wait on it after spawning the debugger thread; the debugger thread already clears it on every exit path once its init (server bind, banner, registration) is done. Also print the banner before reportNodeInspectorServerStarted so it is ordered before inspector.open()'s wakeup on that path too.
…-parse breakpoint pauses - Translate Debugger.setBreakpoint's actualLocation and Debugger.setBreakpointByUrl's locations back to original positions -- clients print them (the debugger REPL's `breakpoints` list showed generated line numbers). - A script starts executing as soon as it parses, so a pass-through pre-parse breakpoint could fire before the backend processed the removal issued during its source-map retranslation; V8 never pauses there (it re-resolves internally), so resume through such a pause without reporting it. - When an evaluation returns an error value with a preview, recover the `stack` property JSC's five-property preview cap crowded out, listing it first as V8 does.
Node's --inspect-port=[host:]port names the port an --inspect listener binds and is reflected by process.debugPort even without a listener (--inspect-port=0 alone reports 0). Bun silently ignored the flag and kept reporting the 9229 default. An explicit --inspect=<port> keeps its own port.
…nded off the re-set line JSC resolves a stale pass-through breakpoint forward, sometimes to the very location its re-set lands on; that pause is the one the client asked for and is reported (under the id the client knows), not resumed. Unconditional suppression made probes miss when the pass-through and re-set positions resolved to the same statement.
The probe report's stderr field must carry only the target's own output; Node targets print nothing but the listening lines the filter already drops, while Bun targets also print the Bun Inspector banner.
Byte-verbatim from upstream, plus the fixtures they use (fixtures/debugger/probe-exits-during-probe.js, probe-multi-statement.js, probe-throwing-getter.js). All pass CI-style against the debug build. parallel/test-debugger-auto-resume.mjs parallel/test-debugger-heap-profiler.js parallel/test-debugger-probe-explicit-column.js parallel/test-debugger-probe-expression-throws.js parallel/test-debugger-probe-failure-process-exit.js parallel/test-debugger-probe-json-preview.js parallel/test-debugger-profile.js parallel/test-debugger-sb-before-load.js parallel/test-inspector-emit-protocol-event-errors.js parallel/test-inspector-emit-protocol-event.js parallel/test-inspector-network-arbitrary-data.js parallel/test-inspector-network-data-received.js parallel/test-inspector-network-data-sent.js parallel/test-inspector-port-zero.js parallel/test-inspector-vm-global-accessors-getter-sideeffect.js parallel/test-inspector-vm-global-accessors-sideeffects.js sequential/test-debugger-debug-brk.js
cirospaciari
changed the title
…o claude/node-v26-combined-34719 # Conflicts: # src/js/internal/inspector/cdp.ts # src/runtime/cli/run_command.rs
…o claude/node-v26-combined-34719 # Conflicts: # src/js/internal/debugger.ts # src/js/internal/inspector/cdp.ts # src/js/node/inspector.ts # src/jsc/bindings/BunDebugger.cpp # test/js/node/inspector/inspector-profiler.test.ts
…o claude/node-v26-combined-34719 # Conflicts: # test/js/node/inspector/inspector-profiler.test.ts
…o claude/node-v26-combined-34719
…o claude/node-v26-combined-34719 # Conflicts: # src/js/internal/inspector/cdp.ts
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.
Stacked on #34719 — review that first. This branch is based on
claude/inspector-cdp-on-cli, notmain.This is a consolidation branch: it carries two previously separate stacked PRs so there is one review surface instead of two. Nothing here is new work by me — every source change and every vendored test comes from a member branch unchanged. The merge produced no conflicts in any file.
50 upstream Node v26.3.0 test files added. 49 pass, 1 fails (see "The one failure" below).
Supersedes
claude/node-inspector-domains@cdc6e24aafinspector.console,url()/close()for a CLI-started inspector, synchronousSession.postcallbacksclaude/node-inspect-debugger@31e3155eedbun inspectCLI debugger client and probe modeThe member branches are left in place; only the PR is closed. Read #35381 for the rationale behind its source changes — this description does not repeat them.
Status
53 upstream Node v26.3.0 test files added — 35 from the debugger half, 18 from the CDP-domains half — plus 6 already-vendored
test-debugger-*files re-synced byte-verbatim. All 59 pass, run one process per file the wayscripts/runner.node.mjsruns them.bun run lintonsrc/jsis clean.test/expectations.txtis untouched.Build 79394 was the first CI
claude/node-inspect-debuggerhad ever seen — it never had a PR of its own — and it surfaced five failures, all in that branch's own files rather than in any interaction between the two members. All five are fixed:process._debugProcessreverted to a no-op stub. The real implementation sentkill(pid, SIGUSR1)with no SIGUSR1 handler installed. A no-argument call coercedundefinedto0, andkill(0, …)signals every process in the caller's process group, sotest/js/node/process/process.test.js— which has_debugProcessin itsundefinedStubslist — took down the whole test runner on both alpine lanes. The same implementation threwTypeError: process._debugProcess is not supported on Windowswhere that test expectsundefined, reddening both Windows lanes. The stub satisfies the assertion, keepskill(0, …)structurally unreachable, and does not ship a lethal API. Real attach-to-running-process would need Node's SIGUSR1 → start-inspector handler and belongs in its own PR.evalbinding renamed. The REPL eval hook made a built-in fail to parse in the release/ASAN configuration (Error parsing builtin: Unrecognized token 'eval'), which is why twosequential/test-debugger-*files failed on debian-13-x64-asan while passing against a debug build.bun(no-duplicate-conditional-property-access)lint errors cleared. 26 were in the debugger branch's own files; the 27th,src/js/internal/debugger.ts:321, comes fromclaude/node-inspector-domainsand so survived that branch's sweep — it is fixed here in its own commit.Bun__loadNodeInspectEntryPointwas missingRELEASE_AND_RETURN, which is what actually caused the ASAN abort ininspector.test.ts. Not the injected-script-host exception-check assert I had guessed from the annotation. The same fix was applied toHTMLEntryPoint.cpp.Known-red lanes that are not ours
Every debugger test on
debian-13-x64-asandies to a vendored-WebKit abort —unchecked exception at scopeChain @ JSJavaScriptCallFrame.cpp:178. The debuggee aborts on the first CDP pause, the session dies, and each test then times out waiting for its prompt. The same tests pass 35/35 locally. #34719's owner has escalated a WebKit fork patch with this build as evidence; those lanes come back on the nextWEBKIT_VERSIONbump. Nothing to fix here, and the tests are deliberately not dropped for it.Two tests handled after the first CI round
parallel/test-inspector-has-idle.jsis dropped. It failed on six lanes (alpine/debian/ubuntu aarch64, debian/alpine x64, windows 2019 x64) withMismatched <anonymous> function calls. Expected exactly 1, actual 0— the(idle)sample node never materialises outside x64 Linux. The(idle)emission itself is kept; only its upstream test is not portable. Six lanes is not a flake.parallel/test-debugger-probe-narrow-suffix.jsis kept and the underlying Windows bug fixed. It failed only on windows-11-aarch64, reporting{"event":"miss","pending":[0]}. TheurlRegexhanded toDebugger.setBreakpointByUrlaccepted either separator in its leading directory group but pinned the separators inside the suffix to/, so a multi-segment suffix never matched a script URL spelled with backslashes. Gated to win32, because a backslash is a legal filename character on POSIX. This fix is unverified on Windows — there is no Windows runner here; the 59 tests confirm it is behaviour-neutral on Linux. If CI still reports a miss, the test should be dropped rather than the fix extended blind.Deliberate deletions, carried not resolved away
parallel/test-debugger-preserve-breaks.jsis deleted. The copy in the tree had been hand-edited — its async body was invoked withoutawaitandcommon.mustCall()was stripped — so it passed unconditionally, including against a build with no CLI debugger at all. The verbatim upstream file fails for a real reason. A test that cannot fail is worse than no test. The merge is verified not to restore it.jsc_adapter.ts(770 lines) was dropped from the salvaged port: inspector: CDP on --inspect, node announce lines, Session, Profiler adapter, Network, exit handshake, async stacks (+9 tests, inspector 11%→22%) #34719'scdp.tsalready speaks real CDP with sourcemap translation, so the client talks plain CDP the way upstream does.Binary size
Roughly +580 KB on the linux targets and +240 KB on darwin, windows and freebsd. This is a new subsystem, not bloat: ~3k lines of new built-in JS under
src/js/internal/debugger/plusNodeInspectEntryPoint.cpp, and built-in JS is bundled into the binary. The head commit carries[allow size]. The gate compares against canary main while the base branch already carries its own allowances, so anything stacked on it trips on the sum rather than on its own delta. These deltas are real and reproducible across all twelve targets, so this is an explained allowance rather than a retry-masked artifact.Merge notes for a reviewer
process._debugProcess— from the new merge-base's perspective the revert was a no-op while my side had already added the code, so git kept mine. Rebuilding from the base removes that class of error entirely.nodejs/nodev26.3.0 copies.parallel/test-inspector-has-idle.jsFlagged
[flaky]with retries on four lanes. It belongs to #34719's owner, who has it on their watch-list. Not a merge interaction.Superseded detail
parallel/test-inspector-debug-end.js(vendored by #35381) fails on this branch and passes on #35381 alone. It is not a merge mistake, and it is not in the code the coordinator expected: the collision is inprocess._debugProcess, not in_debugEndor the control channel.Root cause:
claude/inspector-cdp-on-cliandclaude/node-inspector-domains,process._debugProcessisProcess_stubEmptyFunction— a no-op.test-inspector-debug-end.js's third case callsprocess._debugProcess(process.pid)and nothing happens, so the child reachesprocess.exit(42)and the test passes.claude/node-inspect-debuggerreplaces that stub with a real implementation (Process_functionDebugProcessinsrc/jsc/bindings/BunProcess.cpp) that doeskill(pid, SIGUSR1), matching Node'snode_process_methods.cc.SIGUSR1handler. In Node,SIGUSR1is what starts the inspector; in Bun the default disposition applies, so the child signals itself and is killed.Observed on the merged build:
Deterministic — 3/3 runs. The fix belongs on
claude/node-inspect-debugger: either install Node'sSIGUSR1→ start-inspector handler, which is the whole point of_debugProcess, or keep_debugProcessfrom being lethal on a runtime that does not handle the signal. I have not touched it here because that branch's owner is still working in those files, and I have left the test in place rather than dropping it. Its owner should pick one of the two fixes; the test then passes without any change to #35381.Flaky, not an interaction bug (local)
sequential/test-debugger-invalid-args.jsfailed once when it ran immediately after the other 47 inspector/debugger files in one sweep, then passed 3/3 in isolation and 2/2 in a sequential-only batch. Reported here for the record; nothing in the merge touches it.Merge notes for a reviewer
src/js/internal/debugger.ts,BunDebugger.cpp,src/js/internal/inspector/cdp.tsandsrc/runtime/cli/Arguments.rsare each modified by only one member.Arguments.rshas threeparse_param!entries matching--debug…; the third is the pre-existing, unrelated--debug-no-minify. DEP0062 appears exactly once, as an implementation, not a duplicate.nodejs/nodev26.3.0 copies, andtest/expectations.txtis untouched.How it was verified
Each file was run the way
scripts/runner.node.mjsruns it: one process per file,bun run --config=bunfig.node-test.toml <file>, withBUN_DEBUG_QUIET_LOGS=1 BUN_GARBAGE_COLLECTOR_LEVEL=1 BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1 NO_COLOR=1and noFORCE_COLOR, against a debug build of this branch.