Repository navigation
feat(launch): start_debugging answers a stop with where it is, and get_stack_trace carries the stale-build note (#951) - #960
Merged
Conversation
…t_stack_trace carries the stale-build note (#951) A launch (or restart) that is paused when it answers now carries `location`, `context`, and `threadId`/`note` the way wait_for_stop does. DebugLauncher.launch reads the landing through describeStopLocation on the PAUSED arm — only for a stop the core kept, since PAUSED without lastStop is the auto-continue transient — LaunchContext gains getStackTraceDetailed, and LaunchResultData (DebugResultData & StopLandingFields) threads through the five launch/restart signatures. The launch message is unchanged; the structured location is the addition. The three hoist-plus-stale-check blocks in execution-tools.ts (step, wait_for_stop, pause) become one helper, hoistStopLanding, which the start_debugging and restart_debugging handlers now call too. get_stack_trace appends the same sentence when the file of its top frame is newer than the compiled file the debugger binds it through; a `<…>` label or an unresolvedSource top frame is not probed. Measured on the dev proxy with an aged .NET DLL: before, the launch answer and get_stack_trace carried neither location nor note while wait_for_stop carried the sentence; after, both carry it, a rebuild clears both, and a Python stopOnEntry launch answers with location and no note. The start_debugging and get_stack_trace tool descriptions name the new fields (24 tool-list snapshots regenerated); the seven doc pages that said "(#951)" now name the carriers. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3ULnoVSoCZ5t2Y8d98Ctv
…eal checker; CLAUDE.md names the launch as a landing-read caller The plan named full-server .NET cases for the launch answer and get_stack_trace beside the handler-level ones; they are in server-stale-build.test.ts now (aged DLL, transformedLaunch on the session, the real BuildStalenessChecker over the fake fs). The hoistStopLanding doc says the launch handlers call it on every answer and why that is a no-op without a location. CLAUDE.md's landing-read sentence lists the launch as its fifth caller and the handler helper. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3ULnoVSoCZ5t2Y8d98Ctv
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…e vacuous lastStop gate and the false comments behind it (#951 review) Own-PR review findings, all seven addressed: - debug-launcher.ts: "PAUSED without lastStop is the auto-continue transient" was false — _updateSessionState has refused PAUSED without lastStop since #598 and the auto-continue path records the stop before entering PAUSED. The gate on finalSession.lastStop was therefore vacuous and is gone; the new comment names the real protection (the readiness wait's microtask re-read, #849), and the pre-existing `reason` fallback comment that said the same false thing is corrected. - shared.ts: hasOpenableSourcePath(file) — not a `<…>` label and absolute. hoistStopLanding now skips the line read and the stale probe for any other file, so a step landing on a JDI package-relative label such as java/io/PrintStream.java no longer runs the Java candidate walk and a stat against the server's cwd (pre-existing since #938); get_stack_trace applies the same rule beside its unresolvedSource check. Two cases per stop tool in execution-tools.test.ts. - The helper's doc no longer claims nothing happens without a location (threadId/note are copied first; the launch's non-stop arms carry none). - Carrier lists that omitted the launch: stop-location.ts header, StopLandingFields doc, docs/architecture/component-design.md. - docs/tool-reference.md: data.note lacks the appended stale sentence, so the placement sentence says so; the launch response example now shows the real swap_vars.py lines with the five-entry surrounding the line reader produces. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y3ULnoVSoCZ5t2Y8d98Ctv
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
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.
Closes #951. Follow-up to #950 (#952) and #938 (#949).
What was wrong
For the compiled languages the stale-build artefact is known only once the launch has been transformed, so the first chance to say anything is a stop answer — and the most common agent flow for a fast program never read one that carried it:
Measured again on the dev proxy before this change (aged
dotnet.dll, breakpoint inProgram.cs):start_debuggingansweredpausedwithdata.reason: "breakpoint"and nothing else;get_stack_tracecarried nonote; onlywait_for_stopon the already-paused session carried the sentence. The launch answer carried nolocation,threadId,noteorlastStopon any arm.What this changes
start_debuggingandrestart_debugginganswer a stop the waywait_for_stopdoes.DebugLauncher.launchreads the landing through the one landing read behind pause, step, wait_for_stop and attach (describeStopLocation, #914) on its PAUSED arm. An entry stop the core auto-continues never reaches that arm: the readiness wait reads the state throughwaitForSessionState, whose microtask re-read (#849) sees the RUNNING the continue sets in the same call stack — and a PAUSED session always carrieslastStop(the state writer enforces it since #598), which the review round found two comments in this file claiming otherwise; both are corrected.LaunchContextgainsgetStackTraceDetailed(type-only; the launcher is built with the full context), andLaunchResultData = DebugResultData & StopLandingFieldsthreads through the five launch/restart signatures;DebugResultDatastays closed (its key-set test is unchanged, a sibling pins the new type). The launchmessageis unchanged; the structuredlocationis the addition. Dry runs, the pending arm and the run-to-completion arm read no stack (asserted).One handler helper for every stop answer. The three identical hoist-plus-stale-check blocks in
execution-tools.ts(step,wait_for_stop,pause_execution) becomehoistStopLandinginhandlers/shared.ts:threadId/noteto the top level, thenlocation, the linecontextand the stale-build sentence (readLineContext+describeStaleBuild, overlapped), appended last. The step/wait/pause suite ran unchanged against the extraction before the two launch handlers became its fourth and fifth callers.get_stack_tracecarries the sentence for its top frame. After the notes the stack itself needed (hidden frames, adopted thread, unresolved sources), the samedescribeStaleBuildprobe onframes[0]— skipped for a<…>label, a relative spelling or anunresolvedSourceframe, which name no file on this host, and for an empty stack. The same rule (hasOpenableSourcePath) now gates the line read and the stale probe inhoistStopLandingfor every stop answer: before, a step landing on a JDI package-relative label such asjava/io/PrintStream.javahad the Java candidate walk and astatrun against the server's cwd (pre-existing since #938; the review round surfaced it as the one carrier asymmetry). Cost note:get_stack_traceis called more often than the stop carriers and the checker does not cache map-proved misses (a.tstop frame with no compiled file is a stat per candidate per call) — pre-existing for the other carriers; every decline is pure path arithmetic.Descriptions and docs. The
start_debuggingandget_stack_tracetool descriptions name the new fields (24 tool-list snapshots regenerated, as #914/#857/#855 did).docs/tool-reference.md(set_breakpoint's carrier list, the launch response example and "The program stops" bullet, aget_stack_tracebullet), the six language pages that said "(#951)",docs/javascript/README.md(carrier list; and a claim thatlastStop.textappears in the launch answer — it never did, onlyreason),docs/architecture/api-reference.md,changelog.d/951.added.md.Measured after (dev proxy, Windows, aged
dotnet.dll)start_debugging→pausedwith top-levellocation(Program.cs:8),context, and thenotenaming the DLL and the source with both mtimes;datakeeps its copy oflocation.get_stack_trace→ the samenote.dotnet build, relaunch →locationandcontextpresent, nonote, on both.stopOnEntrylaunch →location(hello.py:1) andcontext, nonote: no false positive on an interpreted language.Verification
npx vitest run --project unit: 372 files, 7434 passed, 4 skipped. One pre-existing case needed a change:session-manager-operations-coverage.test.ts"answers with the stop when it lands inside the hold" advanced the fake clock by 0 ms after the stop, and the landing read's 10 ms settle never fired (15 s timeout); it now advances 50 ms, with the reason recorded.session-manager-launch-contract.test.ts(paused launch carrieslocation; entry stop too; no stack read on the pending and run-to-completion arms; restart carries it),debug-result-data.test.ts(LaunchResultDatakeys), newhandlers/debuggee-tools.test.ts(both launch handlers hoistlocation/threadId/note, readcontext, append the stale sentence, hoist nothing without a location),handlers/inspection-tools.test.ts(get_stack_traceappends the sentence last, skips labels/unresolved/empty),handlers/execution-tools.test.tsunchanged and green across the extraction, andserver-stale-build.test.tswith two cases over the full server and the real checker on the fake fs (aged DLL,transformedLaunchon the session): the launch answer andget_stack_traceboth carry the note.npx vitest run tests/integration/javascript-launch-config.test.ts: 16 passed. The start_debugging answersrunningfor a js-debug launch even when breakpoints are armed — the launch-response contract differs by adapter #823 case ("a breakpoint reached as the program starts from the launch itself") now assertslocation.line === 9and the line'scontexton the answer that reported the stop — the CI-run proof with a real adapter.mcp-server-smoke-java,-dotnet,-cpp,-javascript— 4 files, 34 passed. The .NET and JavaScript aging cases now assert thelocationand the stalenoteon the launch answer itself (the two fixtures that are paused when the launch answers) and thenoteonget_stack_trace; Java (its fixture sleeps past the hold) and C++ assert theget_stack_tracecarrier, C++ the launch carrier when it answered paused. The new assertions encode exactly what the pre-change measurement on the dev proxy showed missing; they were not run against the old build.npm run lint,npm run typecheck:all(ratchet baseline 0 across 0),node scripts/check-docs.mjs: clean.Side finding while measuring, filed as #959: a .NET launch without
dapLaunchArgs.programpasses the.csto netcoredbg and fails with a bare0x800700c1atconfigurationDone.Review round (own-PR review at high effort)
Seven findings, all addressed in the follow-up commit: the two false "PAUSED without
lastStop" comments indebug-launcher.ts(mine and the pre-existing one it copied; the gate they justified was vacuous and is gone); the carrier asymmetry above (hasOpenableSourcePath, with two cases per stop tool inexecution-tools.test.ts); the helper's doc overstating "nothing happens without a location" (threadId/noteare copied first; the launch's non-stop arms carry neither); three carrier lists that still omitted the launch (stop-location.ts,StopLandingFields,component-design.md); the field-placement sentence intool-reference.md(data.notelacks the appended stale sentence); and the launch response example, now the realswap_vars.pylines with the five-entrysurroundingthe line reader produces.Out of scope
lastStopon the launch answer (the issue asks forlocationandnote;reasonis there); the launchmessagewording (pinned by an existing test, andlocationis the contract); anoteonget_local_variables/get_variables; anunresolvedSourceflag onlocationitself (the handler-side path rule covers the label and relative cases; an absolute path behind asourceReferenceis still probed, and a probe of a file that exists here is a fair one); #812.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y3ULnoVSoCZ5t2Y8d98Ctv