Skip to content

fix(proxy): a noDebug launch completes without waiting on a configuration phase the adapter never opens (#746) - #750

Merged
debugmcpdev merged 5 commits into
mainfrom
fix/746-nodebug-launch-completes
Sep 17, 2026
Merged

debugmcpdev merged 5 commits into
mainfrom
fix/746-nodebug-launch-completes

Conversation

@debugmcpdev

@debugmcpdev debugmcpdev commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Closes #746.

What was wrong

A noDebug launch on an adapter that honours the flag (#710: python, go, cpp — js was the one working case) could never succeed. The proxy worker's configuration phase (setBreakpointssetFunctionBreakpointssetExceptionBreakpointsconfigurationDoneadapter_configured_and_launched) runs off the adapter's initialized event, and:

  • debugpy and Delve — correctly, per DAP — send no initialized when they are not debugging. The worker never reported configured, ProxyManager.start() never resolved, and the launch died as Proxy exited during initialization. Code: 0 for a short script (it had run and exited 0) or the 30 s init deadline for a server. On go the two-phase path burned its 2 s + 10 s waits first.
  • CodeLLDB does send initialized, then refuses what is sent in the phase (Internal debugger error: Not supported in noDebug mode.) → Error in DAP sequence → shutdown.

Measured before writing code, with DAP_TRACE=1 and a raw DAP probe against CodeLLDB: it withholds the launch response until configurationDone; it refuses setBreakpoints but accepts configurationDone and then launches; without configurationDone nothing ever happens (12 s silence).

What changes

Under an honoured noDebug — the launcher's decision (debuggerOff = noDebug && honoursNoDebug, #747), now stamped once on the init payload (ProxyConfig.debuggerOffProxyInitPayload.debuggerOff, parser-validated) so the worker never re-derives it from a launch config an adapter transform may have reshaped; attach never carries it — the worker stops waiting on and stops dying on the configuration phase, but does not pre-empt the adapter:

  • the launch is reported configured on the launch response (completeDebuggerOffLaunch), and the go/java two-phase waits are skipped — both are pure delays when no initialized is coming;
  • if the adapter opens a phase anyway, the normal phase runs unchanged (breakpoints, exception filters, configurationDone, breakpoints_synced echo): the adapter's own answers stand, and a stale pin for some adapter build self-corrects (its breakpoints bind, its stops arrive) instead of the program running free;
  • only the failure policy changes, and only for a request the adapter answered with an error (DapResponseError, which MinimalDapClient now rejects with — a transport failure, timeout or shutdown is still a broken session): the refusal is forwarded, configurationDone is still sent so the phase the adapter opened is closed (CodeLLDB releases the launch response on it), and the session carries on — unless it is already shutting down;
  • markConfiguredAndLaunched makes the CONNECTED transition + readiness status fire once, from INITIALIZING only, since under the flag the launch response and the end of a phase both reach it — a phase pending from the handshake, or one in flight when the launch response lands (CodeLLDB answers launch before its configurationDone response), closes first, as in debug mode;
  • a terminal DAP event (terminated, exited, adapter exit, socket close — all four go through enqueueTerminalSignal) waits, bounded by the slot's 2 s backstop and contained, for the launch outcome (settleDebuggerOffLaunch) and then only marks configured: a phase still pending from the handshake is dropped, one in flight is left to finish, and no phase is ever opened from a terminal slot against a program that has ended. The handshake's own completion checks a synchronous terminalSignalArrived flag before opening one. This closes the tick-count race (initialized and terminated in one socket read) that could regress to "Proxy exited during initialization";
  • every refusal — setBreakpoints, setFunctionBreakpoints, setExceptionBreakpoints, configurationDone — is forwarded to the caller in the adapter's own words: worker adapter_notice status → ProxyManager adapter-notice event → recorded like a Docker: Rust type summaries unavailable (no rustc in image) — &str/String render as raw LLDB internals #441 policy annotation (launch warning + [mcp-debugger] Warning output entry), through the same [FEATURE] Secret redaction + least-privilege variable inspection #237 write-time redaction as every other buffer entry, with the listener contained like the output path. A refused setBreakpoints is also echoed per breakpoint: the file groups the adapter already answered are echoed as they came (verified, adapterId), only the unanswered ones are stamped verified: false with the refusal as message, so list_breakpoints shows what the adapter actually said. A refused setFunctionBreakpoints gets the same function_breakpoints_synced echo;
  • the launcher's post-launch resyncAll is gated on !debuggerOff: CodeLLDB has already answered in the phase, debugpy/Delve open no phase, and resyncAll discards the answer anyway (measured: debugpy says Server is not available to a post-launch setBreakpoints, and nothing kept it) — the round trips bought nothing. Follow-up breakpoints: a failed live setBreakpoints re-send never stamps the adapter's answer onto the records #754 asks for live-sync refusals to be stamped onto the records the way the worker echo now does;
  • the js-debug function breakpoints via the CDP proxy (revives #282 by another route) #295 stopOnEntry-force site (js launch command args) reads the same stamp — nothing in the worker re-derives the decision from a flag any more.

The parent needs no change: adapter_configured_and_launched resolves start(), #747's neutralized stopOnEntry projects RUNNING, readiness short-circuits, and the program's exit lands as STOPPED + exit code with the #467/#701 summary and the #710 warning. When the launch fails under the flag, the failure note (buildNoDebugFailureNote(language)) says the debugger was off and — for go only — that Delve resolves the program through exec.LookPath (see the quirk below).

Stamp, not derive — the first version derived the decision a second time in the worker from the post-transform launchConfig; both review rounds pointed out the agreement rested on every honouring transform spreading the key through, and a divergence produces the exact #746 symptom with a misleading "debugger is off" warning attached. One decision, decided once in the launcher, read by the worker.

Measured matrix (this branch's dist, set a breakpoint, launch { stopOnEntry: false, noDebug: true })

adapter before after
python (debugpy) success: false, Proxy exited during initialization. Code: 0 runningstopped, exit 0, Result: 30 in output, #710 warning
go (Delve) 30 s init ceiling runningstopped, Hello, World! in output, #710 warning (no exitCode: Delve reports the status as a console line only, in debug mode too — #753)
cpp (CodeLLDB) Error in DAP sequence: … Not supported in noDebug mode. runningstopped, exit 0, program output; CodeLLDB's own refusal in get_output, in the launch warning (…; setBreakpoints refused under noDebug: Internal debugger error: Not supported in noDebug mode.) and on each pre-launch breakpoint's message in list_breakpoints
javascript (js-debug) stopped, exit 0 unchanged
rust, ruby, dotnet, java, mock paused at the breakpoint, "no effect" warning unchanged

One adapter quirk surfaced truthfully through the new path: under the flag Delve runs the binary through Go's exec.LookPath, so on Windows an extensionless binary fails with the adapter's own executable file not found in %PATH% (debug mode spawns it fine). Documented; the go e2e builds a .exe on win32.

Tests

Docs

docs/tool-reference.md noDebug paragraph, the honoursNoDebug doc comment and the python/go/cpp policy comments no longer say the launch fails; changelog.d/746.fixed.md.

Out of scope

Rust forwarding the flag (its transformLaunchConfig drops it, so noDebug silently does nothing there) is a separate decision. The later surfaces of a debugger-off session (set_breakpoint, list_breakpoints, pause_execution, inspection still answering in debugger terms) are #749 (PR #751). Go never reporting exitCode is #753; stamping live-sync refusals onto breakpoint records is #754.

🤖 Generated with Claude Code

…tion phase the adapter never opens (#746)

Under an honoured noDebug (#710) the worker's configuration phase ran off
the adapter's `initialized` event, and an adapter that is not debugging —
debugpy, Delve — correctly never sends one: the launch died as "Proxy
exited during initialization. Code: 0" (a short script that had run and
exited 0) or the 30 s init deadline (a server), after go burned its 2 s +
10 s two-phase waits. CodeLLDB does open the phase and then refuses what
is sent in it ("Not supported in noDebug mode"), which read as "Error in
DAP sequence" and tore the session down; measured with a raw DAP probe, it
withholds the launch response until configurationDone and accepts that
request after refusing setBreakpoints.

The worker now stops waiting on the phase and stops dying on it, without
pre-empting the adapter:

- `isDebuggerOff(launchArgs)`: noDebug as the adapter will see it (true or
  the parser-surviving 'true'), gated by the policy's `honoursNoDebug`;
  attach never counts. The #295 stopOnEntry force reads through it too.
- Plain launch mode reports the launch configured on the launch response
  (`completeDebuggerOffLaunch`); the go/java two-phase branch skips both
  waits — pure delays when no `initialized` is coming.
- A configuration phase the adapter opens anyway still runs unchanged, so
  its own answers stand and a stale pin self-corrects; only the failure
  policy changes: a refused request is logged, configurationDone is still
  sent best-effort to close the phase, no shutdown.
- `markConfiguredAndLaunched` makes the CONNECTED transition + readiness
  status idempotent: under the flag the launch response and the end of a
  phase both reach it, in either order.

Measured after: python/go/cpp go from init failure to running → stopped
with the exit code, the program's output and the #710 warning; js is
unchanged; rust/ruby/dotnet/java/mock still pause at their breakpoints.
One Delve quirk surfaced truthfully: under the flag it runs the binary
through Go's exec.LookPath, so on Windows it needs its .exe extension.

Tests: tests/proxy/dap-proxy-worker-nodebug.test.ts (11); python, go and
cpp noDebug e2e cases (run locally — CI does not run the e2e project).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.90576% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/proxy/dap-proxy-worker.ts 97.26% 4 Missing ⚠️

📢 Thoughts on this report? Let us know!

…nfiguration request reaches the caller (#746 review)

Review of #750 (ten findings, all taken):

- The worker read noDebug as `true`/`'true'` while the launcher's
  resolveLaunchFlag took anything else by truthiness, so `noDebug: 'True'`
  (or 1) had the launcher neutralise stopOnEntry and warn "debugger off"
  while the worker waited for an `initialized` that debugpy — reading the
  same value as truthy — never sends: the exact #746 failure, with a
  warning claiming it was deliberate. Both now read through one
  `coerceLaunchFlag` (src/utils/launch-flags.ts).
- A refused configuration request under the flag went to the worker log
  only; a stale-pin build failing for a real reason reported success with
  no mention. The worker now forwards the adapter's own words as an
  `adapter_notice` status (naming the request), which ProxyManager emits as
  `adapter-notice` and the core records exactly like a #441 policy
  annotation: on session.adapterNotices (joined into the launch warning)
  and as a `[mcp-debugger] Warning` output entry. CodeLLDB's "Not supported
  in noDebug mode" now reads in the start_debugging warning and get_output.
- completeDebuggerOffLaunch reported configured before running a phase that
  had arrived during the launch request (a stale pin on a two-phase
  adapter), and its log line claimed no phase was coming right before one
  ran. The pending phase runs first — it marks configured itself — and the
  launch-response mark only fires from INITIALIZING.
- markConfiguredAndLaunched could flip a session that shutdown had already
  moved to SHUTTING_DOWN (the adapter went away mid-phase; the rejection
  reached the tolerant catch) back to CONNECTED and emit configured after
  dap_connection_closed. Guarded on INITIALIZING.
- The LLM-facing tool-schema text, the still-unreleased #710 fragment, the
  failure note ("some adapters cannot complete a launch under the flag")
  and the failureData comment all described the pre-#746 world; reworded,
  and the 24 tool-list fences re-recorded.
- Delve reports no exit code — in debug mode either; the status is a
  console line — so the changelog and docs no longer claim one for go and
  the go e2e asserts the console line instead.
- The cpp e2e asserts the refusal it claimed to check, in get_output and in
  the warning.
- The worker-test scaffolding (typed message sender, dependency bag) moved
  into tests/test-utils/mocks/dap-proxy-doubles.ts and both the go fallback
  suite and the new one build from it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@debugmcpdev

Copy link
Copy Markdown
Collaborator Author

Review round 1 (/code-review 750 high) — ten findings, all taken in 7907887

# Finding Resolution
1 isDebuggerOff() read only true/'true'; the launcher's resolveLaunchFlag takes anything else by truthiness, so noDebug: 'True'/1 had the launcher say "debugger off" while the worker waited for initialized — the exact #746 failure with a warning calling it deliberate One coerceLaunchFlag (src/utils/launch-flags.ts) read by both; worker test covers 'True', 1, 'yes' and 'false'
2 LLM-facing tool-schemas.ts noDebug text (and the unreleased 710.fixed.md) still said the launches "currently fail" Reworded; 24 tool-list fences re-recorded
3 completeDebuggerOffLaunch reported configured before running a phase that had arrived during the launch request (stale pin on a two-phase adapter) Pending phase runs first (it marks configured itself); the launch-response mark fires only from INITIALIZING. New go test pins the order launch-response → setBreakpoints → configurationDone → configured
4 The tolerant catch downgraded any phase rejection to a log line; a stale-pin build failing for a real reason reported success with no mention The adapter's own words now go to the caller: worker adapter_notice status (naming the request) → ProxyManager adapter-notice → core records it exactly like a #441 annotation (launch warning + [mcp-debugger] Warning output entry). cpp's warning now ends ; setBreakpoints refused under noDebug: Internal debugger error: Not supported in noDebug mode.
5 markConfiguredAndLaunched could flip SHUTTING_DOWN back to CONNECTED from the new catch path Guarded on INITIALIZING; test simulates shutdown rejecting a pending setBreakpoints
6 go e2e asserted no exitCode while the changelog/docs claimed "with the exit code" Measured: Delve reports no exitCode in debug mode either (console line only). Docs/changelog say so; the go e2e asserts the console line. Filing a dogfood ticket for exit-code synthesis on go
7 buildNoDebugFailureNote / failureData comment described the pre-#746 world Reworded: the failure is most likely unrelated to the flag; drop it to compare
8 cpp e2e comment claimed the refusal is surfaced but never asserted it Asserts the refusal in get_output and in the launch warning
9 Third copy of the worker-test scaffolding createMockMessageSender + createMockWorkerDependencies hoisted into tests/test-utils/mocks/dap-proxy-doubles.ts; the go fallback suite and the new suite use them
10 The "no configuration phase to wait for" log line preceded a phase run Distinct log for the pending case; the reorder in #3 removes the contradiction

Re-verified: proxy suite 323, unit+integration 5376 green; python/go/cpp noDebug e2e and the 9-adapter probe matrix unchanged (python/go/cpp complete, js unchanged, the five ignoring adapters pause).

…rate only the adapter's own refusals; order configured before a same-read exit (#746 review 2)

Re-review of #750 (seven findings, all taken):

- The worker re-derived debuggerOff from the post-transform launchConfig;
  agreement with the launcher rested on every honouring transform
  spreading the key through, and a divergence is the exact #746 symptom
  with a misleading "debugger is off" warning. The launcher now stamps
  its decision on the init payload (ProxyConfig.debuggerOff →
  ProxyInitPayload.debuggerOff, parser-coerced and validated) and the
  worker reads the stamp. The #295 site keeps reading its own launch
  command's args through coerceLaunchFlag.
- The tolerant catch relabelled any rejection — 'DAP client
  disconnected', a timeout, 'worker shutdown' — as a refusal, sent
  configurationDone at a client shutdown may have nulled, and forwarded a
  bogus notice. MinimalDapClient now rejects a DAP error response with a
  typed DapResponseError; only that is tolerated, only while the session
  is not shutting down; everything else is the broken session it is.
- Plain launch mode handles `initialized` as it arrives, so the phase can
  be mid-flight when the launch response lands (CodeLLDB answers launch
  before its configurationDone response) and configured was reported
  before the phase closed. The phase in flight is tracked and awaited.
- A terminal DAP event dispatched from the same socket read as the launch
  response could outrun it by a tick or two, shut the worker down before
  it reported configured, and regress to "Proxy exited during
  initialization". The exited/terminated tasks now wait for the launch
  outcome first (settleDebuggerOffLaunch): configured, then the exit.
- Refused setExceptionBreakpoints / setFunctionBreakpoints were swallowed
  by their inner catches and never became notices; they do now, so the
  docs' "the refusal reaches the warning" holds for every request.
- A forwarded notice bypassed the #237 write-time redaction; it goes
  through the same pass, flagged like any other redacted entry.
- configurationDoneAttempted folded into `stage`; one sendStatusSafely
  for the breakpoint echo, the notice and the child-adoption report.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@debugmcpdev

Copy link
Copy Markdown
Collaborator Author

Review round 2 (/code-review 750 medium, scoped to round 1's fixes) — seven findings, all taken

# Finding Resolution
1 The tolerant catch was cause- and state-agnostic: 'DAP client disconnected', a timeout or 'worker shutdown' all became "refused under noDebug", configurationDone was fired at a possibly-null client, and configured could be reported anyway MinimalDapClient now rejects a DAP error response with a typed DapResponseError (carrying the response); only that is tolerated, and only while the session is not SHUTTING_DOWN/TERMINATED. A transport failure is fatal as in debug mode. Tests: transport error → Error in DAP sequence, no notice; a refusal landing during shutdown → no notice, no configurationDone, no mark
2 Plain launch mode handles initialized on arrival, so the phase could be mid-flight when the launch response landed and configured was reported before the phase closed (CodeLLDB answers launch before its configurationDone response) The in-flight phase is tracked (initializedPhase) and awaited by completeDebuggerOffLaunch. Test pins launch-response → configurationDone → configured
3 Derive-vs-stamp, second time, with a concrete desync scenario Stamped. debuggerOff rides ProxyLaunchRequestProxyConfig → the init command → ProxyInitPayload (parser coerces/validates it); the worker reads the stamp. Tests: the launcher stamps it (and false where the flag is ignored); the worker's "stamp without the key reports early, key without the stamp does not"
4 Refused setExceptionBreakpoints / setFunctionBreakpoints never became notices noticeRefusalUnderNoDebug(request, err) in both inner catches (typed refusals only). Test asserts both notices
5 recordAdapterNotice bypassed #237 redaction Same write-time pass as every other entry; flagged redacted. Test with a GitHub PAT in the note
6 Same-read terminated vs launch-response race sendTrackedLaunch records the launch outcome; the exited/terminated terminal tasks first settleDebuggerOffLaunch() — configured is reported before the exit goes out, deterministically. The test models the terminal path starting first with the response delayed eight ticks; without the hook it yields ['terminated'] alone (the mark is refused after shutdown)
7 configurationDoneAttempted redundant with stage; three copies of the safe-status guard stage alone; one sendStatusSafely used by the breakpoint echo, the notice and the child-adoption report

Re-verified: proxy/parser/launcher/core suites, unit + integration, typecheck:all, lint; python/go/cpp noDebug e2e green against the rebuilt dist.

…e after a refusal is still fatal; bounded, contained terminal-slot completion (#746 review 3)

Third pass on #750 (nine findings, all taken):

- Round 2 overwrote tests/unit/proxy/dap-response-error.test.ts — the
  whole #663 suite — with the one new DapResponseError test. Restored from
  main with the new describe appended.
- The configurationDone the tolerant branch sends after a refusal swallowed
  every error and still marked configured: a socket drop there read as a
  running session on a dead adapter. Only a DapResponseError is tolerated;
  anything else takes the fatal path (failConfigurationPhase).
- initializedEventPending was never consumed, so a second completion (the
  launch response and a terminal signal both completing the launch) took
  the pending branch, hit the duplicate guard without waiting, and reported
  configured mid-phase. Pending is consumed once, and the duplicate guard
  awaits the phase in flight.
- settleDebuggerOffLaunch ran an unbounded wait (the launch request's 30 s
  timeout) inside a terminal-signal slot, and a throw there aborted the
  slot before terminated was forwarded. Bounded by the slot's 2 s backstop
  (now one named constant shared with the stdio drain) and contained.
- A refused setBreakpoints skipped the breakpoints_synced echo, so the
  store never saw the adapter's answer; the refusal is echoed onto every
  pre-launch breakpoint (list_breakpoints shows CodeLLDB's own words).
- The #295 stopOnEntry-force site still re-derived the decision from its
  own args; it reads the stamp. With no worker consumer left, the
  coercion helper folded back into the launcher's resolveLaunchFlag.
- The failure note said the failure was "most likely unrelated" to the
  flag while the PR documents a flag-caused one (Delve + .exe); it names
  both possibilities.
- One redactForBuffer() for output entries and forwarded notices; the
  notice handler is the recorder itself; the child-adoption report calls
  the one readiness reporter again.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@debugmcpdev

Copy link
Copy Markdown
Collaborator Author

Review round 3 (/code-review 750 low, scoped to round 2) — nine findings, all taken

# Finding Resolution
1 Round 2 overwrote tests/unit/proxy/dap-response-error.test.ts (the #663 suite) with the one new test Restored from main, new describe appended — 7 tests
2 The configurationDone sent after a refusal swallowed every error and still marked configured Only a DapResponseError is tolerated there; a transport failure takes the fatal path (failConfigurationPhase). Test: refusal, then 'DAP client disconnected'Error in DAP sequence, no configured
3 initializedEventPending never consumed → a second completion took the pending branch, hit the duplicate guard without waiting, reported configured mid-phase Consumed once; the duplicate guard awaits the in-flight phase. Test: go two-phase, pending phase + terminated with the launch response → setBreakpoints → configurationDone → configured → terminated
4 settleDebuggerOffLaunch ran an unbounded wait inside a terminal slot, and a throw aborted the forward Bounded by the slot's 2 s backstop (TERMINAL_SIGNAL_BACKSTOP_MS, shared with the stdio drain) and wrapped. Tests: launch never answered → terminated forwarded after the backstop; the readiness status throwing → terminated still forwarded
5 The failure note said "most likely unrelated to the flag" while the PR documents the Delve .exe case Names both: possibly unrelated (see the error), or an adapter behaving differently under the flag
6 The #295 site still re-derived the decision from its own args Reads the stamp. coerceLaunchFlag had no worker consumer left → folded back into resolveLaunchFlag; the helper module and its test removed
7 A refused setBreakpoints skipped the breakpoints_synced echo The refusal is echoed onto every pre-launch breakpoint (verified: false, message = the adapter's words). Live: cpp list_breakpoints"Internal debugger error: Not supported in noDebug mode." on the pre-launch breakpoint
8 Duplicated redact-at-write block; redundant notice lambda One redactForBuffer() for output entries and notices; recordAdapterNotice registered directly
9 Child-adoption report inlined the readiness payload Calls reportConfiguredAndLaunched() again inside its try/catch

Re-verified: proxy/parser/session/server suites (1838), unit + integration, typecheck:all, lint, check:docs; python/go/cpp noDebug e2e green against the rebuilt dist.

…fusal echo keeps what the adapter accepted; every terminal path settles the launch (#746 review 4)

Fourth pass on #750 (nine findings, all taken):

- settleDebuggerOffLaunch could open a whole configuration phase from a
  terminal-signal slot (a stale-pin two-phase adapter, `initialized` and
  `terminated` in one read) — outside the backstop, against a program that
  had ended. The slot now only marks configured; a pending phase is
  dropped, one in flight is left to finish. The handshake's own completion
  checks a synchronous terminalSignalArrived flag before opening one.
- The setBreakpoints refusal echo stamped every pre-launch breakpoint,
  overwriting groups the adapter had accepted (verified, adapterId). The
  echo now carries the answered groups as they came and stamps only the
  rest.
- The failure note put Delve's exec/.exe advice on every adapter's failed
  noDebug launch; it takes the language and says it for go only.
- A refused setFunctionBreakpoints forwarded a notice but no
  function_breakpoints_synced echo; it echoes verified:false + the refusal
  onto each function breakpoint, as the line breakpoints get.
- resyncAll re-asked an adapter whose debugger is off (CodeLLDB had
  already answered in the phase; debugpy/Delve open no phase, and
  resyncAll discards the answer anyway): gated on !debuggerOff.
- The adapter-notice listener had no guard around the output-captured
  emit; a throwing subscriber would have escaped into the IPC listener.
  Contained like the output path.
- The settle ran in two of four terminal slots; hoisted into
  enqueueTerminalSignal so adapter_exited and dap_connection_closed share
  it.
- DapResponseError.command names the notice; one isNoDebugRefusal()
  predicate; the child-flush backstop uses the shared constant; the
  sendStatusSafely comment says what it covers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@debugmcpdev

Copy link
Copy Markdown
Collaborator Author

Review round 4 (/code-review 750 low, scoped to round 3) — nine findings, all taken

# Finding Resolution
1 settleDebuggerOffLaunch could open a whole configuration phase from a terminal slot, outside the backstop, against a program that had ended The slot only marks configured (a pending phase is dropped, one in flight finishes on its own); the handshake's completion checks a synchronous terminalSignalArrived flag before opening a phase. Test: stale-pin go, initialized + terminated in one read → launch-response → configured → terminated, no setBreakpoints
2 The refusal echo stamped every pre-launch breakpoint, overwriting groups the adapter had accepted Answered groups are echoed as they came; only the unanswered ones are stamped. Test: file A accepted (verified: true, adapterId), file B refused
3 Delve's .exe advice on every adapter's failed noDebug launch buildNoDebugFailureNote(language); the sentence is go's only
4 Refused setFunctionBreakpoints → notice but no echo function_breakpoints_synced with verified: false + the refusal per function breakpoint
5 resyncAll re-asked an adapter whose debugger is off Gated on !debuggerOff. Measured first: debugpy answers a post-launch setBreakpoints with "Server is not available", but resyncAll discards that answer — so the round trips bought nothing. Follow-up #754 filed to stamp live-sync refusals onto the records
6 Notice listener registered without the output path's guard Contained; test with a throwing output-captured subscriber
7 Settle in two of four terminal slots Hoisted into enqueueTerminalSignal; test asserts configured before dap_connection_closed
8 DapResponseError.command unused; duplicated predicate/strings isNoDebugRefusal(), notices named by error.command
9 Stale comments; hardcoded child-flush backstop Constant shared; sendStatusSafely's comment says what it covers

Re-verified: proxy/parser/session/server suites (1843), unit + integration, typecheck:all, lint, check:docs; python/go/cpp noDebug e2e green against the rebuilt dist.

@debugmcpdev
debugmcpdev merged commit 3fca2d8 into main Sep 17, 2026
10 checks passed
@debugmcpdev
debugmcpdev deleted the fix/746-nodebug-launch-completes branch September 17, 2026 18:44
debugmcpdev pushed a commit that referenced this pull request Sep 17, 2026
…ers not-paused with the why, not "no active proxy" (#749)

Measured on the merged tree (#750 + #751) with debugpy under noDebug:
nothing ever stops, so no thread is current, and debugpy refuses the
`threads` discovery ("Server is not available") — the facade then threw
ProxyNotRunningError, "Cannot get stack trace: no active proxy", for a
session whose proxy was alive. js-debug hid this: its inspector answers
`threads`, so the js e2e reached the resolver's not-paused note.

On a session that is not paused, no thread is expected; the session
layer's not-paused answer (with the debugger-off why) needs none, so the
facade hands it there. A paused session with no thread to name keeps
the error — that is the anomaly it describes. The two existing throw
tests now say "paused".

The python surfaces e2e (the clean case: debugpy attaches no debugger
at all) covers set_breakpoint, list_breakpoints, get_stack_trace,
get_local_variables, evaluate_expression, step_over and pause_execution;
watched it fail on exactly this answer against a dist without the fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
debugmcpdev pushed a commit that referenced this pull request Sep 17, 2026
…stack_trace never claims "no active proxy"; pause on an initializing launch says why (#749 review 4)

Fourth pass, on the merged tree (#750 + #751), seven findings, all taken:

- get_stack_trace asked the adapter for `threads` before the state check
  that made the answer irrelevant: a session that is not paused gets the
  session layer's not-paused note (with the why) with no round trip — on
  a wedged adapter that request blocked for the DAP timeout first.
- The PAUSED + no-thread branch still threw "no active proxy" for a proxy
  that was alive; the resolver's "No stopped thread is known for this
  session." is the truthful answer, so the throw is gone (the error stays
  for a session with no proxy at all).
- A breakpoint the adapter verified is proof this build debugs after all,
  and it arrives before any hit. Measured first: every honouring adapter
  refuses or unbinds a live breakpoint under the flag (js-debug "Unbound
  breakpoint", debugpy "Server is not available", Delve "noDebug mode:
  unable to process 'setBreakpoints'", CodeLLDB "Not supported in noDebug
  mode"), so a verified record can only come from a build that ignores
  it. Consulted on read (adapterVerifiedABreakpoint, inside isDebuggerOff)
  since bindings are per-launch state; the launch response reads the same
  evidence, so "will not fire" is never said of a breakpoint
  list_breakpoints shows bound.
- pause_execution on an INITIALIZING debugger-off session was the one
  surface without the why.
- withDebuggerOffWhy now passes the rejected value itself as the cause,
  not a synthesized Error, when the bridge threw a non-Error.
- The "Used in" map on debuggerOffForLaunch named five files that never
  reference it; it names the one gate that does.
- The list_breakpoints doc note quotes the paused variant too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
debugmcpdev added a commit that referenced this pull request Sep 17, 2026
…so on every later surface (#749) (#751)

* fix(session): a session whose launch runs with the debugger off says so on every later surface (#749)

#747 decided "this noDebug launch runs with the debugger off" once, for the
start_debugging response, and forgot it. Nothing on the session recorded
it, so set_breakpoint on the running session came back verified:false with
no reason, list_breakpoints showed everything unbound with none,
pause_execution ended in the #678 "may be blocked in native code" guess,
and stepping, get_stack_trace, get_local_variables and evaluate_expression
answered "not paused" in debugger terms.

Record: `DebugSessionInfo.debuggerDisabled` (inherited by ManagedSession),
written in the launcher where debuggerOff is decided (real launches, not
dry runs), reset in the launcher's and the attach controller's per-attempt
blocks — not in the core's setupProxyEventHandlers, which runs inside
proxyLauncher.start after the launcher wrote it — and cleared in
handleStopped: a real stop is stronger evidence than the policy's pin.
Projected by SessionStore.getAll() and list_debug_sessions.

Consult, never pre-empting the adapter: every request still goes to the
adapter and its own answer is kept; one sentence
(ErrorMessages.debuggerOffForLaunch) is added beside it — set_breakpoint
(unverified only), list_breakpoints (top-level warning), pause pending
(instead of the policy's guess) and pause refused (appended), "Not paused:"
on step/continue, and the stack-trace note, evaluate error and locals
message. Measured: js-debug still lands a pause under noDebug, so the
sentence says "no stop is expected" rather than "cannot come", and that
stop clears the flag.

Tests: field lifecycle in session-manager-nodebug-warning.test.ts, both
projections, every consumer, and a js e2e over pause_test.js (run locally
— CI does not run the e2e project).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(session): a pause does not clear the debugger-off decision; one gated why for every surface (#749 review)

Review of #751 (nine findings, all taken):

- handleStopped cleared debuggerDisabled on ANY stop, but the PR's own
  measurement says js-debug lands a user pause under noDebug with its
  breakpoints still off — so after the first pause every surface lost its
  why again. Only a stop a live debugger produces clears it now
  (DEBUGGER_ON_STOP_REASONS: the breakpoint family, exception, entry,
  step); the launcher's stoppedAnyway reads the same record instead of its
  own firstStopHandled/PAUSED discriminator, so the launch response and
  the record cannot disagree.
- The record was never cleared on STOPPED/ERROR, so a breakpoint queued
  after the run said "cannot bind" and list_debug_sessions kept reporting
  a stopped session as debugger-off. isDebuggerOff() gates the decision on
  the session being running or paused; the projection and every consumer
  read through it.
- One home for the concern: src/session/debugger-off.ts (the reason set,
  isDebuggerOff, debuggerOffWhy) replaces five hand-spliced reads.
- The pending-pause message appended "no stop is expected" to a base text
  that promises one; ErrorMessages.pausePendingDebuggerOff is one message.
- A refused pause threw a fresh Error, dropping the adapter's own error
  object (stack, props); the original is rethrown with the why appended,
  and the no-debug-target return carries the why too.
- list_breakpoints warned even when every record was verified, contradicting
  set_breakpoint's own gate; now only with an unverified record.
- The e2e's refusal branch was unreachable (callTool throws on an MCP
  error); callToolSafely, and the landed-pause branch asserts the decision
  is kept, as measured.
- docs: list_breakpoints documents the top-level warning; the noDebug
  paragraph, the list_debug_sessions field note and the fragment describe
  which stops clear the decision and that it is not reported once the
  session is over.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(session): a step from a noDebug pause proves nothing either; the decision is consulted only while running or paused (#749 review 2)

Re-review of #751 (eight findings, all taken):

- 'step' was in the set of stops that clear the decision, but the js
  pause the PR measured lands because the inspector is attached, and so
  does a step taken from it (measured: reason 'step', breakpoints still
  unbound) — one step_over after the pause put every surface back in the
  pre-#749 state. The set is now the user-asked stops (breakpoint family,
  exception) plus 'entry'; the e2e steps after the pause and checks the
  record survives.
- A launch refused before the proxy existed (the MSVC-toolchain branch)
  moves the session back to CREATED with the record intact, and the gate
  only excluded terminal states. isDebuggerOff() now means running or
  paused, exactly as the docs said.
- stoppedAnyway had dropped the PAUSED clause, so a launch ending paused on
  a 'pause' stop could say no stop can arrive. Restored: a paused launch
  never claims that, whatever the record says about breakpoints.
- withDebuggerOffWhy appends to the adapter's own error; when the message
  will not take the append (getter-only), it wraps with the original as
  the cause instead of throwing from inside the pause path.
- The tool-reference bullet said "no stop ever arrives" and, four sentences
  on, that js-debug lands a pause: "no breakpoint, exception or entry stop
  ever arrives".
- The e2e comment said the pause makes the session forget the decision
  while the assertion checked the opposite; the comment matches now.
- handleListDebugSessions re-gated a field SessionStore.getAll() had
  already gated; it mirrors the field.
- USER_BREAK_REASONS moved next to BREAKPOINT_STOP_REASONS in
  @debugmcp/shared (the core re-exports it); DEBUGGER_ON_STOP_REASONS is
  that set plus 'entry' rather than a third hand-built union.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(session): proof the debugger is on is the adapter's own breakpoint reason, a hit, an exception or an entry — and the launch warning stops claiming no stop can arrive (#749 review 3)

Third pass on #751 (twelve findings; eleven taken, one declined below):

- js-debug relabels a `debugger;` statement stop ('pause', "Paused on
  debugger statement") to 'breakpoint', and under noDebug that pause
  lands (measured) — so the clear must key on what the adapter itself
  reported: stopProvesDebuggerOn() = hitBreakpointIds present, or an
  exception or entry stop, or a breakpoint reason the adapter called one
  (raw and normalized). An uncaught throw under js noDebug was measured
  not to stop, so 'exception' stays as proof.
- The restored PAUSED clause in stoppedAnyway made the launch response say
  "no effect … breakpoints work as usual" while the record said they cannot
  bind. The root was #747's wording — "the debugger is disabled … and no
  stop can arrive" — refuted for js by a pause, a step and a `debugger;`
  statement. The warning now says the debugger is off for this launch and
  names what will not fire, so the record and the response agree, and the
  PAUSED clause goes again.
- The raw record is now `launchDebuggerOff` on ManagedSession, distinct
  from the projected `debuggerDisabled` on DebugSessionInfo, so no handler
  can read the ungated value by accident.
- isDebuggerOff() includes INITIALIZING: the proxy is up and a breakpoint
  set in that window still goes to the adapter, whose "Unbound breakpoint"
  needs the why.
- A paused session gets only the clause still true of it — breakpoints
  cannot bind — not "no stop is expected".
- The debugger-off pending-pause message keeps the policy's own
  explanation: on js-debug the pause can land under the flag, and #678's
  smart-stepper advice is what makes it.
- withDebuggerOffWhy no longer mutates the adapter's error: a new Error
  with the why, the original untouched as the cause.
- The composed "not paused" texts live in error-messages.ts (notPaused,
  cannotEvaluateNotPaused, stackTraceNotPaused, noStackFramesNotPaused,
  withDebuggerOffWhy, pausePendingDebuggerOff); one debuggerOffWhyFor(ctx,
  id) in handlers/shared.ts replaces three ctx→session→why copies; the
  USER_BREAK_REASONS re-export shim is gone.
- Declined: treating an auto-continued 'pause'-reason entry stop as proof
  the flag was ignored (a stale-pin js-debug build stopping at entry as
  'pause' while the launcher neutralised stopOnEntry) — the record
  self-corrects on the first breakpoint the adapter reports hit, and
  reading a pause as proof is the bug the round fixed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs(schema): the dapLaunchArgs.noDebug description stops claiming no stop ever arrives (#749)

The schema is the one noDebug text the model reads before choosing the
flag, and it still said "no stop ever arrives" after #751 retired that
claim in the warning, the fragment and the docs: js-debug lands a user
pause under the flag. It now says what is true of every honouring
adapter — no breakpoint, exception or entry stop — and that the session
reports debuggerDisabled while such a launch runs. Fences re-recorded.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(server): get_stack_trace on a running session with no thread answers not-paused with the why, not "no active proxy" (#749)

Measured on the merged tree (#750 + #751) with debugpy under noDebug:
nothing ever stops, so no thread is current, and debugpy refuses the
`threads` discovery ("Server is not available") — the facade then threw
ProxyNotRunningError, "Cannot get stack trace: no active proxy", for a
session whose proxy was alive. js-debug hid this: its inspector answers
`threads`, so the js e2e reached the resolver's not-paused note.

On a session that is not paused, no thread is expected; the session
layer's not-paused answer (with the debugger-off why) needs none, so the
facade hands it there. A paused session with no thread to name keeps
the error — that is the anomaly it describes. The two existing throw
tests now say "paused".

The python surfaces e2e (the clean case: debugpy attaches no debugger
at all) covers set_breakpoint, list_breakpoints, get_stack_trace,
get_local_variables, evaluate_expression, step_over and pause_execution;
watched it fail on exactly this answer against a dist without the fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(session): a verified breakpoint is proof the debugger is on; get_stack_trace never claims "no active proxy"; pause on an initializing launch says why (#749 review 4)

Fourth pass, on the merged tree (#750 + #751), seven findings, all taken:

- get_stack_trace asked the adapter for `threads` before the state check
  that made the answer irrelevant: a session that is not paused gets the
  session layer's not-paused note (with the why) with no round trip — on
  a wedged adapter that request blocked for the DAP timeout first.
- The PAUSED + no-thread branch still threw "no active proxy" for a proxy
  that was alive; the resolver's "No stopped thread is known for this
  session." is the truthful answer, so the throw is gone (the error stays
  for a session with no proxy at all).
- A breakpoint the adapter verified is proof this build debugs after all,
  and it arrives before any hit. Measured first: every honouring adapter
  refuses or unbinds a live breakpoint under the flag (js-debug "Unbound
  breakpoint", debugpy "Server is not available", Delve "noDebug mode:
  unable to process 'setBreakpoints'", CodeLLDB "Not supported in noDebug
  mode"), so a verified record can only come from a build that ignores
  it. Consulted on read (adapterVerifiedABreakpoint, inside isDebuggerOff)
  since bindings are per-launch state; the launch response reads the same
  evidence, so "will not fire" is never said of a breakpoint
  list_breakpoints shows bound.
- pause_execution on an INITIALIZING debugger-off session was the one
  surface without the why.
- withDebuggerOffWhy now passes the rejected value itself as the cause,
  not a synthesized Error, when the bridge threw a non-Error.
- The "Used in" map on debuggerOffForLaunch named five files that never
  reference it; it names the one gate that does.
- The list_breakpoints doc note quotes the paused variant too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: JF <john.franklin@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <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.

noDebug launches never complete proxy init on python and go (no initialized event) and error on cpp (CodeLLDB refuses configuration in noDebug mode)

2 participants