Skip to content

feat: surface js debuggee exit code (exitCode) via preload shim synthesis - #252

Merged
debugmcpdev merged 2 commits into
mainfrom
fix/js-exit-code
Aug 4, 2026
Merged

feat: surface js debuggee exit code (exitCode) via preload shim synthesis#252
debugmcpdev merged 2 commits into
mainfrom
fix/js-exit-code

Conversation

@debugmcpdev

Copy link
Copy Markdown
Collaborator

Fixes #247

Second of two PRs for the cluster:js-child-session backlog chunk (#248/#249/#250 are in #251).

Root cause

Not a forwarding bug: vscode-js-debug never emits a DAP exited event at all (verified against the vendored bundle — the only exited occurrence is a thread event reason; the only exit-code text is a stderr line gated behind outputCapture: 'console', which we can't use without regressing #218 stdout capture). The child-event forwarding path is complete and unfiltered; the event source was missing. So JavaScript sessions could never distinguish a crash (exit 1) from a clean exit (exit 0) — the contract #220 added for Python and mock.

Mechanism: the debuggee records, the worker replays

  1. assets/exitcode-shim.cjs — a tiny CJS preload registered via NODE_OPTIONS --require. Writes process.on('exit')'s code to a per-session temp file. Root-process-only via an inherited env claim (MCP_DEBUGGER_EXITCODE_CLAIMED), so spawned children / worker_threads / cluster workers skip while tsx-style wrappers still record the propagated code at the outermost process. Fully try/caught — can never break the debuggee.
  2. transformLaunchConfig injects the shim (double-quoted, forward-slashed for Windows paths with spaces; idempotent; multi-location resolution mirroring the vendor lookup) and MCP_DEBUGGER_EXITCODE_FILE (per-session UUID temp path). Launch mode only; a missing asset degrades gracefully to today's behavior with a warning.
  3. Worker onTerminated reads the file and synthesizes exited {exitCode} before forwarding terminated — ordering is load-bearing: whichever of exited/terminated reaches the SessionManager first strips the other's handler, and the worker shuts down inside onTerminated. Real exited events stay authoritative (flag), parent+child double-terminated synthesizes once, and non-js sessions skip instantly (no env marker). No new config plumbing: the worker already holds the launch config env.
  4. IFileSystem gains readFile/remove (fs-extra-wired) so the worker stays unit-testable.

Documented gaps (never a guessed value): signal-killed debuggees (exit handler never runs) and attach mode (target env not ours) still report no exitCode.

Testing

  • Shim: 5 tests spawning real node children (exit 0 / 7 / uncaught throw / root-claim with spawned child / unset marker no-op).
  • Transform: 5 tests (injection + quoting, user NODE_OPTIONS preserved, no double-append, graceful skip, attach untouched).
  • Worker: 6 tests (exited-precedes-terminated ordering, file absent, real-event authority, once-across-double-terminated, garbage content, non-js skip).
  • E2E against real js-debug: the [BUG] start_debugging hangs past 60s for a fast-crashing JavaScript script #242 regression guard now asserts exitCode defined and non-zero exactly like its python twin (closing the loop [BUG] js-debug sessions never surface the debuggee exit code (exitCode missing) #247 requested), plus a new clean-exit exitCode === 0 case — the first such assertion for any js session.
  • Full gate: 159 unit test files, 18 e2e smoke files, lint clean.

🤖 Generated with Claude Code

…247)

vscode-js-debug never emits a DAP 'exited' event (verified against the
vendored bundle), so JavaScript sessions could not distinguish a crash
from a clean exit - the exact contract #220 added for other adapters.
The forwarding path was already complete; the event source was missing.

The debuggee now records its own exit code and the worker replays it:

- New assets/exitcode-shim.cjs preload: writes process exit code to a
  per-session temp file, root-process-only via an inherited env claim so
  spawned children / worker_threads / cluster workers skip while
  tsx-style wrappers still record the propagated code. Fully try/caught,
  never breaks the debuggee.
- transformLaunchConfig injects the shim (NODE_OPTIONS --require,
  quoted + forward slashes for Windows paths, idempotent) plus the
  MCP_DEBUGGER_EXITCODE_FILE env marker. Launch mode only; missing shim
  asset degrades gracefully to today's behavior.
- The worker's onTerminated synthesizes 'exited' from the recorded file
  BEFORE forwarding 'terminated' (ordering matters: whichever event
  arrives first strips the other's session handler). Real exited events
  stay authoritative; parent+child double-terminated synthesizes once.
  Self-gating via the env marker, so non-js sessions skip instantly.
- IFileSystem gains readFile/remove (fs-extra wired, worker stays
  unit-testable).

Signal-killed debuggees and attach mode still report no exitCode (exit
handler never runs / env not ours) - documented; never a guessed value.

The #242 e2e regression guard now asserts exitCode like its python twin,
plus a new clean-exit exitCode=0 case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.23404% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/proxy/dap-proxy-dependencies.ts 0.00% 3 Missing ⚠️
src/proxy/dap-proxy-worker.ts 92.85% 2 Missing ⚠️
...adapter-javascript/src/javascript-debug-adapter.ts 93.75% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@debugmcpdev
debugmcpdev merged commit d09b7a9 into main Aug 4, 2026
9 of 10 checks passed
@debugmcpdev
debugmcpdev deleted the fix/js-exit-code branch August 4, 2026 18:49
debugmcpdev added a commit that referenced this pull request Aug 4, 2026
… fixtures (issue #247 e2e) (#256)

The 'reports exit code 0 for a clean run' e2e test added in PR #252
referenced tests/fixtures/debug-scripts/js-clean-exit.js, but the
fixture never made it into the commit: the blanket tests/**/*.js
ignore (meant for compiled TS artifacts) made 'git add' silently skip
it, so it lived on as an untracked local file until branch cleanup
removed it. Any fresh checkout then failed the test with 'Script file
not found'. CI never caught it because only the container e2e subset
runs there. The committed sibling js-throws.js only exists because it
was force-added, which is why the trap was invisible.

Fix both layers: recreate the fixture (timer-callback shape like
js-throws.js, natural exit 0) and add a narrow gitignore negation for
tests/fixtures/debug-scripts/*.js — hand-written debuggee fixtures,
not TS output — so future fixtures can't be silently dropped. Compiled
artifacts elsewhere under tests/ stay ignored.

Verified: all 9 tests in mcp-server-break-on-exceptions.test.ts pass;
every debug-scripts fixture referenced from tests/ exists.

Co-authored-by: JF <john.franklin@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
debugmcpdev added a commit that referenced this pull request Aug 5, 2026
…h sessions end 'stopped', not 'error' (fixes #258) (#263)

Two defects, both exposed by the #254 stdio drain barrier:

1. Ordering: onExited/onTerminated/onClose all await the same drain
   barrier, and onTerminated's extra await (#252 synthesis) let onClose
   win by a microtask - the codeless dap_connection_closed status reached
   the parent before the terminated DAP event, stripping the handlers
   that would have marked the session stopped. A FIFO terminal-signal
   queue in the proxy worker now preserves arrival order, and the
   adapter process 'exit' status rides the same queue.

2. Fabricated exit code: ProxyManager emitted 'exit' with
   `message.code ?? 1` (and dap-core with `message.code || 1`, mangling
   a real 0), so every codeless closure became exit code 1 and
   SessionManager mapped it to ERROR - even for a clean run.

Terminal statuses now carry an explicit `expected` flag (terminal DAP
event already forwarded, or shutdown underway) and pass the code through
untouched. SessionManager maps expected teardowns to STOPPED (recording
the debuggee exit code), unexpected closures to ERROR, and keeps the
legacy rule for real proxy-process exits. The duplicate unlatched 'exit'
emit from the functional core is suppressed.

Ruby also gains adapterExitCodeIsDebuggeeExitCode: rdbg -c propagates
the debuggee's exit status but never sends a DAP exited event, so the
worker now synthesizes one - clean run records exitCode 0, unhandled
raise records exitCode 1, matching Python/js.

Deferred (cosmetic): Ruby's isSessionReady still resolves the
start_debugging ready-wait via the exit path for quick scripts, logging
'proxy exited during startup'; the reported state is now accurate.

Co-authored-by: JF <john.franklin@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] js-debug sessions never surface the debuggee exit code (exitCode missing)

2 participants