Repository navigation
feat(launch): one launch contract for every adapter — a short hold, then pending (#823, #826, #851) - #857
Conversation
…hen pending start_debugging and restart_debugging now answer by one rule in every language: once the program is launched the call holds its answer briefly and reports whichever comes first — the program stops (paused, with the reason), the program ends (stopped, with the exit code), or the hold elapses (running, pending: true, naming what is armed and pointing at wait_for_stop). - launch-readiness.ts is rebuilt on waitForSessionState as two waits on the session's state: until the program is launched (launchReadyCeilingMs, which also covers a stopOnEntry launch's entry stop), then the hold (launchHoldMs, 1000 ms — sized from measured launch-to-first-stop latency, at most 172 ms idle and 678 ms saturated across the nine language adapters). - js-debug no longer answers `running` at once (#823): AdapterPolicy .isSessionReady is removed from the interface and the nine policies. - The wait no longer depends on what is armed; describeLaunchArming is read when the answer is built and only words it (#826). - Breakpoints set, removed or cleared while the launch was starting are sent as soon as the program is launched, before the hold (#851). - The fresh-echo re-send after the launch is made only to a paused program: one still running when the hold elapses may already have ended with its exit not yet forwarded (#856), and the re-send then un-verified a logpoint that had fired. Closes #823. Closes #826. Closes #851. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…reaches the caller Review follow-up on the "re-send only when paused" rule. The worker's pre-launch setFunctionBreakpoints left a rejection to the parent's post-launch re-send to surface; a launch answered while running no longer makes that re-send, so the answer fell back to "could not resolve the name — check the symbol name", which is the wrong advice for a refusal. - The worker echoes any error answer to the pre-launch setFunctionBreakpoints (it did so only under noDebug); a transport failure or timeout is still not echoed. - buildFunctionBreakpointLaunchWarning reports a refusal-stamped record as a refusal, in the adapter's words and under every policy, and keeps the symbol-name sentence for names the adapter could not resolve. - Comments that described the re-send as what surfaces the state say what happens now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Code reviewFive reviewers (CLAUDE.md compliance, bug scan, git history, earlier PR feedback, code comments) went over 54cd41d. No finding reached the reporting threshold; three related ones scored 75 and are fixed in 212e9d6 because they share one cause, and one of them hid a real loss. The cause: the launch's fresh-echo re-send is now made only to a paused program. mcp-debugger/src/session/launch/debug-launcher.ts Lines 629 to 633 in 212e9d6
mcp-debugger/src/session/launch/debug-launcher.ts Lines 658 to 667 in 212e9d6
mcp-debugger/src/proxy/dap-proxy-worker.ts Lines 1432 to 1448 in 212e9d6 mcp-debugger/src/session/breakpoints/launch-warnings.ts Lines 271 to 326 in 212e9d6 Checked and not an issue: a reviewer asked whether a launch answered After the follow-up: unit 335 files / 6,657 tests, lint, 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Closes #823. Closes #826. Closes #851.
What changes
start_debugging(andrestart_debugging) now answer by one rule, the same in every language: once the program is launched, the call holds its answer for a short window and reports whichever comes first —pausedwith the reason;stoppedwith the exit code;runningwithpending: true, a message naming what is armed, and a pointer towait_for_stop(wait_for_stop: block until a session's next stop or end instead of polling list_debug_sessions #849).Before this PR the answer depended on the adapter and on what was armed:
paused(it landed inside a 500 ms window)pausedrunning, nopending, no mention of the breakpointrunning+pending: true, names the breakpointpausedpausedpendingpendingafter the hold;wait_for_stopcollects the stoppendingafter 5 s (JavaScript: barerunningat once)pendingafter the holdstopOnEntry: truepausedat the entry stopThe hold is not a limit on the program: nothing is cancelled when it elapses, and breakpoints and exception filters stay armed for the life of the process. It only decides whether the launch call reports the first stop itself. The
pendingmessage no longer states a duration, and the tool description does not either: the number is a tunable (launchHoldMs), not part of the contract.How the hold was sized
Measured, not picked. A nested mcp-debugger was launched under the debugger and instrumented with logpoints on the launcher's post-handshake line and on the core's user-visible-stop branch; each language's example was then launched through it with a breakpoint on an early line. "Latency" is the first stop minus the moment the hold begins.
Rule: 1000 ms if the slowest idle first stop is under 500 ms. It is 172 ms, so the hold is 1000 ms — 5.8× the slowest idle case and 1.5× the slowest saturated one. Toolchain work (
javacfor the JDI bridge,cobc, the g++ auto-compile) happens before the handshake and is outside the hold.One measurement looked like an outlier and was not: Java took a constant 2,020–2,030 ms because
examples/java/HelloWorld.javasleeps 2 s before the line the smoke tests break on. That is the "breakpoint reached after the hold" case: the launch answerspending, and the smoke tests, which poll for the pause, pass unchanged.The other way a launch is answered: the program ends
The same hold decides whether a short program's end is in the launch answer. Each language's hello-world example launched with nothing armed, three runs; the figure is the session's
running -> stoppedtransition minusinitializing -> running, from the server log:stoppedstoppedstoppedstoppedstoppedpendingpending;wait_for_stopreturnsstopped~1 s laterTwo things in that table are worth knowing before merging:
exitedevent, so a trivial script's end is reported ~850 ms after launch. On a loaded machine that launch will answerpendingandwait_for_stopwill collect the exit. The contract covers it; a 2 s hold would put it well inside.launchHoldMsis one number.exiteduntil the adapter's stdio pipes close, CodeLLDB never closes them while it lives, and the wait runs to its 2 s backstop every time. Filed as Windows: the end of a Rust/C++/COBOL program is reported 2 s late — the stdio drain waits for a pipe close CodeLLDB never produces #856 with the trace. It predates this PR (the old 5 s window absorbed it); with a 1 s hold a short Rust/C++/COBOL program on Windows is answeredpendingalthough it has ended. Not fixed here — it is the proxy's exit path, which deserves its own change.How
src/session/launch/launch-readiness.tsis rebuilt onwaitForSessionState(wait_for_stop: block until a session's next stop or end instead of polling list_debug_sessions #849) as two waits on the session's state: until the program is launched (the session leavesINITIALIZING; ceilinglaunchReadyCeilingMs, 30 s, which also covers astopOnEntrylaunch's entry stop), then the hold (launchHoldMs). Because the waits read state rather than the adapter'sstoppedevent, an entry stop the core auto-continues settles neither — which matters now that JavaScript waits too (a launch with function breakpoints forces such a stop).debug-launcher.tslosesisReady, thereadinessPolicyshim and the arming-based ceiling. AnoDebuglaunch needs no branch: it never stops, so it is answered by its exit or by the hold.describeLaunchArmingis read when the answer is built and only words it. That dissolves start_debugging arms its readiness window once, before the wait — a breakpoint set during the 5 s grace still gets the unarmed answer #826 rather than patching it: its suggested remedy was to extend to the armed ceiling, and there is no longer one to extend to.AdapterPolicy.isSessionReadyis removed from the interface and the nine policies. One cross-policy pin replaces nine per-policy tests.RUNNINGorPAUSED. A program still running when the hold elapses may already be gone (see Windows: the end of a Rust/C++/COBOL program is reported 2 s late — the stdio drain waits for a pipe close CodeLLDB never produces #856), and the re-send then comes back unverified: the logpoint e2e caught a logpoint that had fired readingverified: falseon Rust/C++/COBOL. A launch answeredrunningnow keeps the proxy's own send. Checked live in all nine languages (a breakpoint behind a 2 s sleep): thependinganswer carries no warning,list_breakpointsreportsverified: true, andwait_for_stopreturns the stop.set_breakpointracing a launch used to be stored and then not sent until the wait ended; a removal was never sent.Verified live
Against the built server, with the scripts from the issues:
runningfor a js-debug launch even when breakpoints are armed — the launch-response contract differs by adapter #823: a JavaScript breakpoint first reached 2 s in →running+pending: truenaming1 breakpoint(s), thenwait_for_stop→paused/breakpoint. The same script shape in Python answers identically. A module-load breakpoint still answerspausedin the one call.condition: "n >= 4"set 1 s into a Python launch (answeredverified: false, the launch still starting) stops the program atn = 4. On main the same breakpoint withn >= 9stopped it at 33 s.Tests
launch-readiness.test.tsrewritten (14 cases, including the auto-continued entry stop and thebeforeHoldhook).session-manager-launch-contract.test.ts(new, 16 cases) through the realSessionManager: the hold with and without anything armed, exit inside the hold, entry-stop launches waiting past the hold, start_debugging arms its readiness window once, before the wait — a breakpoint set during the 5 s grace still gets the unarmed answer #826, A breakpoint set while start_debugging is still starting is not sent to the adapter until the launch's readiness wait ends (30 s) #851 (set and remove), the re-send rule (none to a running launch, kept for a paused one), JavaScript following the same contract, restart.session-manager-operations-coverage.test.tsnow move the session's state the way the core does instead of hooking the old wait's proxy listeners.pending→wait_for_stop; a module-load breakpoint), Ruby (the unarmed answer; the first-breakpoint launch follows the contract).🤖 Generated with Claude Code