Skip to content

fix(#501): stop stranding forked children of js attach targets - #514

Merged
debugmcpdev merged 1 commit into
mainfrom
fix/501-js-attach-fork-release
Aug 27, 2026
Merged

fix(#501): stop stranding forked children of js attach targets#514
debugmcpdev merged 1 commit into
mainfrom
fix/501-js-attach-fork-release

Conversation

@debugmcpdev

Copy link
Copy Markdown
Collaborator

Fixes #501.

What

Attaching a javascript debug session to a Node process that fork()s children left those children permanently wedged: js-debug's pwa-node attach defaults inherit autoAttachChildProcesses: true, so the auto-attach bootloader was injected into the target; every fork parked under waitForDebugger and sent a startDebugging reverse request that the single-child ChildSessionManager silently dropped. The child's main module never ran.

Two complementary fixes:

  1. Default autoAttachChildProcesses: false on the js attach path (transformAttachConfig, mirroring launch mode; defensive normalization in performHandshake for embedders). Caller-overridable via adapterConfig — a supplied boolean is respected, never silently overwritten (cf. ruby attach across a container boundary can bind no breakpoints: localfs is silently overridden, localfsMap is inert #499) — and the key is now in supportedAttachKeys so opting in doesn't trip the unrecognized-key warning (adapterConfig: unknown keys are accepted silently — a typo of a supported key (pathMapping vs pathMappings) gets no warning #466 mechanism).
  2. Release instead of drop. An unadoptable startDebugging target is now released to run undebugged: a throwaway MinimalDapClient performs the minimal unpark sequence verified against the vendored bundle (initializeconfigurationDoneattach {__pendingTargetId, continueOnAttach: true} → ready-signal grace → disconnect {terminateDebuggee: false}), bounded at ~20s, never touching adoption state, with a loud warning. createChildSession now reports 'adopted' | 'duplicate' | 'released' | 'release-failed'; MinimalDapClient rolls back its adoptedTargets only on 'release-failed' (parity with the js-debug: failed child adoption is unretryable (pendingTargetId never removed from adoptedTargets) #249 rollback) so a re-sent request can retry — released targets stay recorded, since their server-side deferred has settled and can never be adopted.

Two findings from validating against the real stack:

  • js-debug delivers fork auto-attach requests on the adopted child's connection, not the parent's (server log: 23 startDebugging received, 1 adoption, 0 releases). The child-safe policy now forwards unadoptable targets back to the owning manager for release instead of squashing them.
  • The cycle-breaking dynamic import('./minimal-dap.js') is now cached in a module-level promise: an adoption racing a release issued two concurrent import() calls, which vitest's module mocker can resolve inconsistently (one caller received the real module, the other the mock).

Testing

🤖 Generated with Claude Code

https://claude.ai/code/session_01NgA5kHzkhFZLqbWNZRTsVG

js-debug's pwa-node attach defaults autoAttachChildProcesses to true, so
attaching to a Node process that fork()s children injected the auto-attach
bootloader into it; every fork then parked under waitForDebugger and sent a
startDebugging reverse request that the single-child ChildSessionManager
silently dropped — the child never ran (its main module never executed).

Two complementary fixes:

- Default autoAttachChildProcesses to false on the js attach path (mirroring
  launch mode), caller-overridable via adapterConfig and now listed in
  supportedAttachKeys. Forks of an attach target run untouched by default.
- When a startDebugging target cannot be adopted (adoption in progress or a
  child already active), release it instead of dropping it: a throwaway DAP
  connection attaches with its __pendingTargetId (which makes js-debug run
  the parked target) and detaches without terminating. The child runs
  undebugged and a loud warning is logged. js-debug delivers fork auto-attach
  requests on the adopted child's connection, so the child-safe policy now
  forwards unadoptable targets back to the manager instead of squashing them.

createChildSession now reports an outcome ('adopted' | 'duplicate' |
'released' | 'release-failed'); MinimalDapClient rolls its adoptedTargets
back on 'release-failed' so a re-sent startDebugging can retry (parity with
the #249 rollback), and keeps released targets recorded — their server-side
pending deferred has settled and can never be adopted.

The dynamic MinimalDapClient import in ChildSessionManager is now cached in
a module-level promise: an adoption racing a release issued two concurrent
import() calls for the same module, which vitest's module mocker can resolve
inconsistently (one caller got the real module, the other the mock).

New fork_attach_target.js fixture (fork + IPC-ack pattern, mirroring
ProxyManager's worker handshake) and e2e coverage: forks keep completing
their IPC handshake while attached with the default config, and with
autoAttachChildProcesses:true the release path frees every unadoptable fork.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NgA5kHzkhFZLqbWNZRTsVG
@debugmcpdev
debugmcpdev enabled auto-merge (squash) August 27, 2026 14:11
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.76471% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/proxy/child-session-manager.ts 94.52% 4 Missing ⚠️
src/proxy/minimal-dap.ts 50.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

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.

js attach to a Node process that forks children strands those children (second startDebugging target ignored, child waits forever)

1 participant