diff --git a/changelog.d/1135.fixed.md b/changelog.d/1135.fixed.md new file mode 100644 index 000000000..9adcd5650 --- /dev/null +++ b/changelog.d/1135.fixed.md @@ -0,0 +1 @@ +**`get_local_variables` and `evaluate_expression` say when the frame they answer from is not your program's.** On a stop where no frame is program code — a pause on an idle Node program, which lands in the timer loop; a `step_out` of `main` in C or C++, which lands in the C runtime — `get_stack_trace` and the pause and step answers said `noProgramFrame` and "No program frame is active…", while `get_local_variables` returned the runtime frame's variables (`now = 4210`, `[return value] = 0`) as plain locals, and `evaluate_expression` without a `frameId` ran there without a word. Both now carry `noProgramFrame: true` and the same sentence, with one clause of their own: "The variables shown belong to '', not to your program." and "The expression was evaluated in that frame." The variables are still returned — they are what the adapter reported — and an explicit `frameId` adds nothing, since the caller chose the frame. A Rust `step_out` of `main` is not covered yet: its landing frame is not classified as runtime (#1131). (#1135) diff --git a/changelog.d/1139.fixed.md b/changelog.d/1139.fixed.md new file mode 100644 index 000000000..5f9056c6f --- /dev/null +++ b/changelog.d/1139.fixed.md @@ -0,0 +1 @@ +**A Python or Ruby attach to a port that does not speak DAP now says so, instead of describing an adapter that never existed.** These attaches start nothing: the server connects straight to the host and port given, so "the adapter" is whatever listens there. Attaching to a listener that accepts the connection and stays silent — the wrong port — waited 30 s and answered "…an adapter-side protocol stall, not a missing install — the adapter started and accepted the connection. Retrying usually succeeds", and retrying stalled identically. Attaching to a real HTTP port (the application's own, say), which hangs up on the DAP frame, answered "Critical initialization error: DAP client disconnected [Executable: python | adapter PID=none exitCode=n/a]", and a port that resets the connection (a forwarder with nothing behind it) answered the bare "DAP client error: read ECONNRESET". All three now name the endpoint and say that nothing was started for the attach; the stall, for one, reads: "127.0.0.1:5000 accepted the connection but never answered the DAP "initialize" request. Nothing was started for this attach: the server connected to what is already listening on that port, so check that it is a debugpy listener (python -m debugpy --listen 127.0.0.1:5000 …) and not another service. Retrying the same port stalls the same way." Once the endpoint has answered `initialize` it is a debug server, and a later stall is worded as its own; if it then closes or resets the connection mid-handshake, the answer is "Critical initialization error: DAP client disconnected [direct connect to host:port; no adapter process was started]" where it used to be "Proxy exited during initialization…" with lines of worker stderr, or "Error in DAP sequence: DAP client disconnected" (a server that first ends the session with a DAP `terminated` event is still answered as a proxy exit: #1152); a connection that was never made no longer says to check that the adapter is installed. The answer for a stall (the 30 s deadline) gains `data.initProgress.directConnect` (`host`, `port`, `listener`). The spawned-adapter messages are unchanged, and so is the 30 s deadline. (#1139) diff --git a/changelog.d/1140.fixed.md b/changelog.d/1140.fixed.md new file mode 100644 index 000000000..78f6c3c45 --- /dev/null +++ b/changelog.d/1140.fixed.md @@ -0,0 +1 @@ +**`snapshot.changedVariables` says when a variable is new or gone, instead of inventing a value for it.** A variable that came into scope between two stops was listed as a change from the string `""` — JavaScript's vocabulary for a Java, Rust, Dart or Python local that simply did not exist yet, indistinguishable from a real value, and for js-debug's `Return value` a "change" from `` to `undefined`. A variable that left scope silently dropped out, so "unchanged" and "gone" looked the same. Each entry now states which of three things happened, and the side that does not exist is left out: `{ "before": "1", "after": "2" }` for a changed value, `{ "after": "30", "new": true }` for a variable that appeared, `{ "before": "3", "gone": true }` for one that left scope. An empty `{}` still means nothing visible changed. The flags are only set when they are known: the locals a snapshot shows are capped at 20, so a name missing from a list that was cut may only not have fitted — a variable absent from a cut previous list is `{ "after": … }` with no `new`, and one absent from a cut current list is not listed. **Changed shape:** an entry for a new variable no longer has `before`; a client that read `before: ""` should read `new: true`. (#1140) diff --git a/changelog.d/1141.fixed.md b/changelog.d/1141.fixed.md new file mode 100644 index 000000000..dd81b9bda --- /dev/null +++ b/changelog.d/1141.fixed.md @@ -0,0 +1 @@ +**`get_stack_trace` no longer sends a caller round in a circle, and an ended session is explained by every inspection tool.** Two guidance gaps. On a stop where no frame is program code, `get_stack_trace` ended its note with "get_stack_trace with includeInternals: true lists the frames" — including for a caller who had just passed `includeInternals: true` and was looking at every frame. That clause is now added only by an answer that has not shown the thread's whole stack (a pause or step answer on a thread with more than one frame), and never by `get_stack_trace` itself. And after a program ran to completion, `get_stack_trace`, `get_scopes`, `get_variables` and `evaluate_expression` answered a bare `Session is terminated: ` where `get_local_variables` explained the state. All five now carry the same `message`, worded for what the session is: a launched program is run again with `restart_debugging`; an attach session, which `restart_debugging` refuses, is attached again with `attach_to_process` (it used to be told "Use restart_debugging" as well); a child session names its parent. The `error` text is unchanged. (#1141) diff --git a/changelog.d/1142.fixed.md b/changelog.d/1142.fixed.md new file mode 100644 index 000000000..99beca85c --- /dev/null +++ b/changelog.d/1142.fixed.md @@ -0,0 +1 @@ +**A `set_breakpoint` refusal no longer suggests a `statement` that cannot be found.** When `expectedContent` did not match the line, the refusal ended "Or address by content: set_breakpoint {statement: ""}" whatever that text was — including text that is nowhere in the file, where following the hint answered `statement … not found`. The sentence is now the statement resolver's own answer for the expected text, so the two cannot disagree: on one line it names it ("…that text is on line 11."); on several it adds `nearLine` and lists the lines; and when the resolver would not find it — the text is absent, or it is only in a comment or past a `//` or `#`, where statement matching does not look — no statement is offered and the refusal names the lines the same `expectedContent` would pass on ("That expectedContent matches line 12."), or says "That expectedContent matches no line of the file." An expectation that spans lines gets no such sentence. (#1142) diff --git a/changelog.d/1153-snapshot-completeness.fixed.md b/changelog.d/1153-snapshot-completeness.fixed.md new file mode 100644 index 000000000..010e43472 --- /dev/null +++ b/changelog.d/1153-snapshot-completeness.fixed.md @@ -0,0 +1 @@ +**Failed variable-scope reads no longer invent `gone` or `new` snapshot entries.** If one scope cannot be read while another supplies locals, the snapshot notes that its list is incomplete. A variable missing from that partial read is not reported as having left scope, and a variable that reappears when the read succeeds is not marked new. A successful empty scope still reports variables that actually left scope. (#1140) diff --git a/docs/architecture/api-reference.md b/docs/architecture/api-reference.md index 91f7ed5e8..2826d3ed0 100644 --- a/docs/architecture/api-reference.md +++ b/docs/architecture/api-reference.md @@ -338,7 +338,7 @@ Steps into a function call. #### `stepOut(sessionId: string): Promise>` Steps out of the current function. -The three step methods take only a `sessionId` and return a typed result, not `void`. `StepResultData` adds a required `message` plus the `StopAnswerFields` every stop answer shares (issue #904: an optional `location` — `{ file, line, column? }`, the first visible frame as the frame-anchor resolver reads it through `describeStopLocation` in `src/session/inspection/stop-location.ts` — and `threadId`/`note` only when the answer is anchored off the reported thread or no frame of the shown stack is program code; plus the stop `snapshot`, issues #982/#983 — composed once per answer after any same-line repeat, with `changedVariables` in place of `locals` on a repeated stop in the same frame, keyed off the session's `snapshotDiffBase`) and, when the stop that ended the step was recorded as something other than the step, `stopReason`/`rawStopReason` with the same semantics as `PauseResultData` (issue #678). If no `stopped` event arrives before the step grace window elapses, the call still succeeds with `data.pending = true` — that window is a reporting deadline, not a deadline on the debuggee. +The three step methods take only a `sessionId` and return a typed result, not `void`. `StepResultData` adds a required `message` plus the `StopAnswerFields` every stop answer shares (issue #904: an optional `location` — `{ file, line, column? }`, the first visible frame as the frame-anchor resolver reads it through `describeStopLocation` in `src/session/inspection/stop-location.ts` — and `threadId`/`note` only when the answer is anchored off the reported thread or no frame of the shown stack is program code; plus the stop `snapshot`, issues #982/#983 — composed once per answer after any same-line repeat, with `changedVariables` in place of `locals` on a repeated stop in the same frame, keyed off the session's `snapshotDiffBase` — each entry a `ChangedVariable`: `{ before, after }`, `{ after, new: true }` or `{ before, gone: true }`, issue #1140) and, when the stop that ended the step was recorded as something other than the step, `stopReason`/`rawStopReason` with the same semantics as `PauseResultData` (issue #678). If no `stopped` event arrives before the step grace window elapses, the call still succeeds with `data.pending = true` — that window is a reporting deadline, not a deadline on the debuggee. #### `pause(sessionId: string, threadId?: number): Promise>` Pauses execution. `PauseResultData` adds `message`, optional `stopReason`/`rawStopReason` (present only when this pause's own stop was recorded, never a stale earlier one) and the `StopAnswerFields` (`location`, `threadId`, `note`, `snapshot` — issue #982; the "Already paused" fast path carries none of them). A pause is read with the resolver's readiness option, so one reported on a thread with no program frame is anchored on the first sibling whose stack reaches the program (issue #898), and one where no thread has a program frame says so in `note` (issue #900). @@ -393,8 +393,10 @@ Closes the mirror endpoint via the `mirrorUnexpose` pseudo-command and disconnec #### `listThreads(sessionId: string): Promise>` Lists all threads in the debug session. -#### `getLocalVariables(sessionId: string, includeSpecial?: boolean, names?: string[]): Promise<{ variables: Variable[]; frame: { name: string; file: string; line: number } | null; scopeName: string | null; anchorNote?: string; truncation?: VariableTruncationSummary }>` -Orchestrates stack trace → scopes → variables, then delegates to the language adapter's policy to extract just the locals. If `includeSpecial` is false (default), internal/special variables are filtered out; `names` is an optional exact-match, case-sensitive filter applied to the extracted locals. The return type is the exported `LocalVariablesResult` (issue #982 named it; also what the protected `extractLocalsFromResolved` — the same pipeline behind an already-resolved stack, which the stop snapshot uses — returns). `anchorNote` explains any departure from "the top frame's local scope" (a lower frame was anchored, and/or a sibling scope supplied the variables); `truncation` appears when the size guards cut the response. +#### `getLocalVariables(sessionId: string, includeSpecial?: boolean, names?: string[]): Promise<{ variables: Variable[]; frame: { name: string; file: string; line: number } | null; scopeName: string | null; anchorNote?: string; noProgramFrame?: NoProgramFrameVerdict; incomplete?: true; truncation?: VariableTruncationSummary }>` +Orchestrates stack trace → scopes → variables, then delegates to the language adapter's policy to extract just the locals. If `includeSpecial` is false (default), internal/special variables are filtered out; `names` is an optional exact-match, case-sensitive filter applied to the extracted locals. The return type is the exported `LocalVariablesResult` (issue #982 named it; also what the protected `extractLocalsFromResolved` — the same pipeline behind an already-resolved stack, which the stop snapshot uses — returns). `anchorNote` explains any departure from "the top frame's local scope" (a lower frame was anchored, and/or a sibling scope supplied the variables); `noProgramFrame` (`{ threadId?, frameCount, top }`, issue #1135) is the resolver's verdict that no frame of the stack is program code — the variables are then a runtime frame's, and the tool answer says so; `truncation` appears when the size guards cut the response. + +`incomplete: true` records a failed scopes or variables request, separately from caps; `anchorNote` explains the partial read. The snapshot uses this internal field to avoid marking unread variables `gone` or recovered variables `new`. ### Session Management diff --git a/docs/javascript/README.md b/docs/javascript/README.md index 95597f3cb..25eb50944 100644 --- a/docs/javascript/README.md +++ b/docs/javascript/README.md @@ -410,7 +410,10 @@ open on its port (Node never closes it by itself) — a later attach by PID or b freezes on its next request; or `paused` at once on a sourceless `` frame between dispatches, with `note` saying no program frame is active and `get_local_variables` empty (issue #900) — continue and send a request, or set a - breakpoint. Attach to a live server you still need to use with `stopOnEntry: false` + breakpoint. A pause that lands in a named runtime frame instead (a launched + program idle between timers stops in `process.processTimers`) has variables of + its own: `get_local_variables` returns them with `noProgramFrame: true` and a + `note` saying they belong to that frame, not to your program (issue #1135). Attach to a live server you still need to use with `stopOnEntry: false` - Some advanced DAP features may not be exposed through MCP tools - **Source-mapped frames you cannot open.** A package that ships `.js.map` files whose `sources` point at `.ts` files it did not ship makes js-debug report those diff --git a/docs/tool-reference.md b/docs/tool-reference.md index d703aca58..2546ede27 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -245,9 +245,11 @@ Context: > 12 | return total 13 | 14 | def main(): -The file may have changed since you last read it. Pick the correct line from the context above. +The file may have changed since you last read it. Pick the correct line from the context above. Or address by content: set_breakpoint {statement: "total = sum(prices)"} — that text is on line 11. ``` +The last sentence is the statement resolver's own answer for the expected text (issue #1142), so following it always works: on one line it names the line; on several it adds `nearLine` (the line you asked for) and lists them — `{statement: "print(a, b)", nearLine: 5} — that text is on lines 4 and 6; nearLine picks the closest (line 4).`; when the resolver would not find the text — it is absent, or it is only where statement matching does not look (a comment line, or past a `//` or `#` on a line) — no statement is offered, and the sentence instead names the lines this same `expectedContent` would pass on (`That expectedContent matches line 12.`) or says `That expectedContent matches no line of the file.`; and text spanning lines gets no such sentence. It is only added where statement anchors are available (`DEBUG_MCP_BP_ADDRESSING=content`, the default). + Relatedly, when a debug adapter *accepts* a breakpoint but binds it to a different line (adapters snap requests on non-executable lines to the nearest valid one), the response reports it prominently instead of silently mutating the line: `message` and `warning` carry `"requested line 12, bound to line 13: \`...\`"`, and the response includes `requestedLine` alongside the bound `line`. Adapters that relocate breakpoints asynchronously (after the response) surface the move in `list_breakpoints`, where `line` ≠ `requestedLine` marks a snapped breakpoint. `expectedContent` requires a source file the server can read: it is rejected for Java FQCN breakpoints and for remote attach sessions (the debuggee's filesystem is the authority); a local attach — by `processId` or to a loopback endpoint — whose file exists on this host is accepted (issue #899). Both addressing aids can be restricted with the `DEBUG_MCP_BP_ADDRESSING` environment variable (`line` = pre-existing behavior, `assert` = + expectedContent/loud snapping, `content` = all features; default `content`) — useful for controlled comparisons of agent behavior. @@ -435,16 +437,28 @@ crashing state costs zero extra tool calls: (`DEBUG_MCP_SNAPSHOT_MAX_LOCALS` / `DEBUG_MCP_SNAPSHOT_MAX_VALUE_CHARS`); `truncation` says what was cut. The read is bounded (~2 s): if it expires or fails inside the composer, the answer ships without `locals` and `note` says so. A read that comes back empty is shown as no locals — and when - unprobed deeper frames exist (the snapshot probes the top 3), `note` says that too; an adapter - error the pipeline itself absorbs looks the same as a locals-free frame, and `get_local_variables` - on the same stop is the deeper read. The snapshot is orientation, not navigation — + unprobed deeper frames exist (the snapshot probes the top 3), `note` says that too; a + scopes or variables request that fails is noted as an incomplete locals list, and `get_local_variables` + on the same stop can retry the read. + The snapshot is orientation, not navigation — `get_local_variables` / `get_variables` remain the way to expand structures or fetch everything. - **`changedVariables`** replaces `locals` on a repeated stop in the same frame (issue #983) — stepping through a function, or a `continue_execution` → `wait_for_stop` loop over one - breakpoint: only what changed since the locals were last shown, as - `{ "name": { "before": "100.0", "after": "125.0" } }`. A variable new in scope appears with - `before: ""`; one that left scope simply drops out; an **empty `{}` means nothing - visible changed** — that is information. Frame identity is the locals anchor's name + file + + breakpoint: only what differs from the locals last shown, one entry per variable (issue #1140): + - a changed value — `{ "before": "100.0", "after": "125.0" }`; + - a variable that appeared — `{ "after": "30", "new": true }`, with no `before`; + - a variable that left scope — `{ "before": "3", "gone": true }`, with no `after`. + + No placeholder ever stands in for a missing side: any text can be a value. An **empty `{}` means + nothing visible changed** — that is information. The flags are only set when they are known. The + locals a snapshot shows are capped (see `locals` above), so a name missing from a list that was + cut may only not have fitted: a variable absent from a previous list that was cut is + `{ "after": "30" }` with no `new`, and one absent from a current list that was cut is not listed + at all (`truncation` says when a list was cut). A failed scopes or variables request also makes the list + incomplete (`note` explains it): missing variables are not called `gone`, and variables that + reappear after the failed read are not called `new`. An adapter's own return-value entry — js-debug's + `Return value` at a function's return point, debugpy's `(return) fn` after stepping over a call — + is a local like any other here and appears as `new`. Frame identity is the locals anchor's name + file + visible stack depth: any mismatch (a different function, a recursion depth change) re-emits full `locals` and rebases — never a wrong diff. The same stop answered twice (two `wait_for_stop` calls without resuming) re-emits full `locals` rather than diffing against @@ -565,7 +579,7 @@ See the [JavaScript configuration guide](javascript/README.md#custom-configurati **Exit code:** when the debuggee terminates, the exit code reported by the adapter is surfaced as `exitCode` in `list_debug_sessions` (and, for a launch that ended before reporting a pause, as `data.exitCode` above), so a crash (non-zero) is distinguishable from a clean exit. Adapters that never send `exited` still get one where the debugger states the code some other way: js-debug through the launch shim's recorded code (#247), rdbg through the adapter process's own exit status (#258), Delve before 1.27 through its `Process N has exited with status S` console line (#753 — in debug mode Delve prints it only in reply to `disconnect`, so the proxy sends the disconnect before forwarding `terminated`; Delve 1.27+ sends `exited` itself). No line, no code: nothing is guessed. -**Between launches:** a session whose program has exited is still present, with its breakpoints, its last real launch and, until the next launch, the previous run's output and exit code (a dry run leaves all of it in place). `start_debugging` (a new configuration), `restart_debugging` (the same one), `set_breakpoint`, `list_breakpoints`, `remove_breakpoint`, `clear_breakpoints` and `get_output` all work on it, so "ran to completion without hitting my breakpoint → set an earlier one → `restart_debugging`" needs no new session; each launch starts a fresh output buffer and re-applies the stored breakpoints with their verified state reset. Tools that need a live debuggee — stepping, `continue_execution`, `pause_execution`, `list_threads`, `get_stack_trace`/`get_scopes`/`get_variables`/`get_local_variables`/`evaluate_expression`, `get_source_context`, `expose_session` — answer `Session is terminated: ` until the next launch (issues #793, #806). +**Between launches:** a session whose program has exited is still present, with its breakpoints, its last real launch and, until the next launch, the previous run's output and exit code (a dry run leaves all of it in place). `start_debugging` (a new configuration), `restart_debugging` (the same one), `set_breakpoint`, `list_breakpoints`, `remove_breakpoint`, `clear_breakpoints` and `get_output` all work on it, so "ran to completion without hitting my breakpoint → set an earlier one → `restart_debugging`" needs no new session; each launch starts a fresh output buffer and re-applies the stored breakpoints with their verified state reset. Tools that need a live debuggee — stepping, `continue_execution`, `pause_execution`, `list_threads`, `get_stack_trace`/`get_scopes`/`get_variables`/`get_local_variables`/`evaluate_expression`, `get_source_context`, `expose_session` — answer `Session is terminated: ` until the next launch (issues #793, #806). The inspection tools (`get_stack_trace`, `get_scopes`, `get_variables`, `get_local_variables`, `evaluate_expression`) add a `message` beside that error saying what it means and what to do next, worded for what the session is (issue #1141): a launched program — `The program has terminated, so no frames or variables exist. Use restart_debugging to run it again.`; an attach session, which `restart_debugging` refuses — `The debugger is no longer attached to the target (it ended, or was detached), so no frames or variables exist. Use attach_to_process to attach again.`; a child session — that its process ended or was released, and the id of its parent. --- @@ -633,12 +647,25 @@ Steps over the current line, executing it without entering function calls. } ``` +A step past a declaration, and a step that leaves a block, in the same shape: + +```json +"changedVariables": { + "total": { "before": "0", "after": "2" }, + "sum": { "after": "30", "new": true }, + "doubled": { "before": "2", "gone": true } +} +``` + `location` and `context` are best-effort: they appear when the post-step stack trace and the source file could both be read. The [stop snapshot](#the-stop-snapshot) rides on every landed step (issues #982/#983). Stepping within one -function reports **`changedVariables`** — only what the executed line changed — instead of re-sending the +function reports **`changedVariables`** — only what the executed line changed: a value that changed, a +variable that appeared (`new: true`) or one that left scope (`gone: true`) — instead of re-sending the full `locals`; stepping into a new frame (or any frame-identity change) shows that frame's full `locals`. -An empty `changedVariables: {}` says the line changed nothing visible. On an adapter that steps once per +An empty `changedVariables: {}` says the line changed nothing visible. A stop whose frame shows no locals +at all carries neither `locals` nor `changedVariables`, so a frame whose every variable left scope says +nothing on that stop. On an adapter that steps once per token position (Dart), the snapshot is composed once, from the final landing of the repeated step. When the stop that ended the step was recorded as something other than the step — a breakpoint on the landing @@ -925,7 +952,7 @@ When the pause was reported on a thread with no program frame — on Windows, Co "note": "The stop was reported on thread 31208 (5: tid=31208), whose 4 frames are all internal/runtime (top: 'DbgBreakPoint'); showing thread 45324 (1: tid=45324), which is in 'main' (…/pause_test.cpp:20)." } ``` -When no thread has a program frame — an idle Node server's pause lands on a sourceless `` frame between event-loop dispatches (issue #900) — `location` is that frame and `note` reads `No program frame is active on thread 0: the only frame is internal/runtime (top: '' :1). The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; get_stack_trace with includeInternals: true lists the frames.` +When no thread has a program frame — an idle Node server's pause lands on a sourceless `` frame between event-loop dispatches (issue #900) — `location` is that frame and `note` reads `No program frame is active on thread 0: the only frame is internal/runtime (top: '' :1). The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint.` When the thread has more frames than the one shown, the sentence ends `…or set a breakpoint; get_stack_trace with includeInternals: true lists the frames.` instead (issue #1141); `get_stack_trace` itself never says that, since it only adds the sentence when it hid nothing. **Response** (the program had not stopped when the grace window elapsed): ```json @@ -1173,7 +1200,23 @@ Gets local variables for the session's shared inspection anchor. It walks down f **Size guards:** same caps and `truncation` advisory as [get_variables](#get_variables); additionally, the multi-frame scope fan-out stops issuing DAP requests once the per-call variable budget is spent (`truncation.scopesSkipped` reports scopes never fetched). Top-frame scopes are fetched first, so the locals that matter are unaffected. The `truncation` counts describe only the returned payload — values cut while fetching fan-out scopes the policy then discarded (e.g. Global/Closure) are not reported (issue #438). -If the originally stopped thread has no frames, this tool uses the same user-frame-preferring adopted anchor as `get_stack_trace`. The response's `anchorNote` always names an automatic thread or lower-frame switch. +If the originally stopped thread has no frames, this tool uses the same user-frame-preferring adopted anchor as `get_stack_trace`. The response's `note` always names an automatic thread or lower-frame switch. + +**When no frame is program code** (issue #1135) — a pause on a launched Node program idle between timers, which lands in the timer loop; a `step_out` of `main`, which lands in the C runtime — the answer still carries the variables the adapter reported for `frame`, and says whose they are: `noProgramFrame: true`, and a `note` with the sentence the pause or step answer gave for that stop (`No program frame is active…`) plus one clause naming the frame. + +```json +{ + "success": true, + "variables": [ { "name": "now", "value": "4210", "type": "number", "variablesReference": 0, "expandable": false } ], + "count": 1, + "frame": { "name": "process.processTimers", "file": "/internal/timers", "line": 527 }, + "scopeName": "Local: processTimers", + "noProgramFrame": true, + "note": "No program frame is active on thread 0: the only frame is internal/runtime (top: 'process.processTimers' /internal/timers:527). The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint. The variables shown belong to 'process.processTimers', not to your program." +} +``` + +The clause is left out when the list is empty. The [stop snapshot](#the-stop-snapshot) of such a stop carries no `locals` at all: it is orientation, and a runtime frame's variables are not; this tool is the explicit read. **Example - Python:** ```json @@ -1290,6 +1333,8 @@ Evaluates an expression in the context of the current debug session. `frame` is present when `frameId` was omitted and names the frame the expression ran in (the same shape `get_local_variables` reports). +When `frameId` was omitted and no frame of the stack is program code (issue #1135), the answer also carries `noProgramFrame: true`, and `anchorNote` ends with the sentence the pause or step answer gave for that stop (`No program frame is active…`) and `The expression was evaluated in that frame.` — the expression ran in a runtime frame (Node's timer loop, the C runtime's start-up), where your program's names do not resolve. An explicit `frameId` is the caller's choice and adds nothing. + **Example - Simple Variable:** ```json // Request (no frameId needed!) @@ -1572,7 +1617,7 @@ When the requested pause has not landed by the time the tool answers (an idle No **Notes:** - `state` is `"paused"` only once a stopped event has actually been observed; otherwise the attach reports `"running"`. When a requested post-attach pause is accepted but its stopped event has not arrived within the bounded wait, the response is successful with `state: "running"` and `pending: true` (at the top level and in `data`) and the `message` names the pending pause; the late stopped event is the only transition to `paused` — [`wait_for_stop`](#wait_for_stop) blocks until it arrives — and every paused session has a `lastStop`. -- An idle Node server may instead answer `paused` at once: js-debug lands the pause on a sourceless `` frame between event-loop dispatches. The attach answer then carries (at the top level and under `data`) `note: "No program frame is active on thread 0: the only frame is internal/runtime (top: '' :1). The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; …"`, `get_stack_trace` shows that one frame with `noProgramFrame: true` and the same note, and `get_local_variables` is empty (issue #900). Either way, attach to a live server you still need to use with `stopOnEntry: false`. +- An idle Node server may instead answer `paused` at once: js-debug lands the pause on a sourceless `` frame between event-loop dispatches. The attach answer then carries (at the top level and under `data`) `note: "No program frame is active on thread 0: the only frame is internal/runtime (top: '' :1). The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint."`, `get_stack_trace` shows that one frame with `noProgramFrame: true` and the same note, and `get_local_variables` is empty with the same flag and note (issues #900, #1135). Either way, attach to a live server you still need to use with `stopOnEntry: false`. - A `paused` attach answer reads where its stop landed through the same resolver read as `get_stack_trace` (issue #904): one `stackTrace` round trip, and on a thread whose stack has not materialised yet (a .NET post-attach pause) the bounded readiness wait `get_stack_trace` used to pay, now on the attach's own clock. On Windows, where CodeLLDB reported the attach stop on the injected break-in thread and the attach anchored on the program's thread instead, the answer's `note` says `The stop was reported on thread X; inspection is anchored on thread Y.` - The initial stop of an attach is reported as `lastStop.reason: "pause"` (`stopReason: "pause"` in `get_stack_trace`) by every adapter, whether the debugger paused the target on request or, as CodeLLDB does, stopped it itself. On Windows, CodeLLDB reports that stop as the `DebugBreakProcess` break-in — an `0x80000003` exception on a helper thread — so the adapter's own words stay beside the pause as `lastStop.rawReason: "exception"` and `description: "Exception 0x80000003 encountered at address …"` (issue #817). Only the first stop of an attach that did not pass `stopOnEntry: false` is read this way: a break-in met later, or first by an attach that opted out of the initial stop, is a real `__debugbreak()` and keeps `reason: "exception"`. - When `processId` was used, the message reads `Attached to process PID ` instead. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 0e847a0bc..634969c28 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -227,9 +227,18 @@ The message's second sentence tells you which stage stalled — it is derived fr - **"…failed to start or is not properly configured"** — nothing ever spawned or connected: verify the language's toolchain (`mcp-debugger doctor`). - **"…spawned (PID N) but the DAP connection was never established"** — the adapter process is up but its port never accepted within the whole 30 s: check for port conflicts or loopback firewalling. The proxy keeps trying to connect for as long as the adapter process is alive and the deadline has not passed (issue #963), so an adapter that is merely slow to start — rdbg on a loaded machine has taken past 13 s — connects when it is ready instead of failing early with `ECONNREFUSED`. -- **"Critical initialization error: Failed to connect DAP client: the adapter process exited (code N) before it started listening…"** — the adapter died while the proxy was waiting for it to listen; the message quotes its last stderr lines (a missing module, a refused toolchain). **"…nothing accepted a connection on host:port within 12.5s"** is the direct-connect form (a remote rdbg/debugpy attach): nothing listened on the address you gave within the attach's window. +- **"Critical initialization error: Failed to connect DAP client: the adapter process exited (code N) before it started listening…"** — the adapter died while the proxy was waiting for it to listen; the message quotes its last stderr lines (a missing module, a refused toolchain). **"…nothing accepted a connection on host:port within 12.5s"** is the direct-connect form (a remote rdbg/debugpy attach): nothing listened on the address you gave within the attach's window. It ends `[direct connect to host:port; no adapter process was started]`. - **"…the \"X\" request never received a response … an adapter-side protocol stall, not a missing install"** — the adapter connected and then went quiet mid-handshake. Retrying usually succeeds; if it recurs, capture a `DAP_TRACE=1` trace of the failing launch. The failed `start_debugging` result's `data` carries the same facts structurally (`initProgress`, `proxyLogPath`). +**A Python or Ruby attach is worded differently** (issue #1139), because the server starts nothing for it: it connects straight to the host and port you gave, and "the adapter" is whatever listens there. The usual cause of a failure is therefore the endpoint itself — most often the wrong port, such as the application's own HTTP port instead of the debugger's: + +- **"`host:port` accepted the connection but never answered the DAP \"initialize\" request. Nothing was started for this attach … check that it is a debugpy listener (`python -m debugpy --listen host:port …`) and not another service. Retrying the same port stalls the same way."** — something is listening, and it is silent to DAP. This one takes the full 30 s. +- **"Critical initialization error: `host:port` never answered the DAP \"initialize\" request (DAP client disconnected). Nothing was started for this attach …"** — the endpoint hung up on the DAP frame, which is what an HTTP server does, or reset the connection, which is what a forwarder with nothing behind it does (the reason in parentheses is then the socket's own, such as `read ECONNRESET`). This one answers at once. +- **"`host:port` answered the start of the DAP handshake, but the \"attach\" request never received a response …"** — the endpoint is a debug server and stalled later; capture a `DAP_TRACE=1` trace if it recurs. +- **"Critical initialization error: DAP client disconnected [direct connect to `host:port`; no adapter process was started]"** — the endpoint answered `initialize`, so it is a debug server, and then closed the connection before the attach completed (`read ECONNRESET` in place of `DAP client disconnected` when it reset it). Check that the debug server, or whatever forwards to it, is still up; a debugpy listener that already has a client refuses in its own words instead. One case is not worded this way yet (issue #1152): a debug server that ends the session itself during the handshake, with a DAP `terminated` event before it closes, is still answered `Proxy exited during initialization…`. + +The answer for a stall — the two that wait out the 30 s — also carries the endpoint structurally, as `data.initProgress.directConnect` (`{ host, port, listener }`). The answers that fail sooner name it in the message only. + For a worked example of diagnosing this class of failure (an rdbg launch that emitted the `initialized` event but never the `initialize` response), see the [rdbg initialize-response stall case study](./case-studies/rdbg-initialize-response-stall.md). ### `get_output` Returns Nothing diff --git a/packages/shared/src/interfaces/adapter-policy-python.ts b/packages/shared/src/interfaces/adapter-policy-python.ts index 205883d8a..01625223c 100644 --- a/packages/shared/src/interfaces/adapter-policy-python.ts +++ b/packages/shared/src/interfaces/adapter-policy-python.ts @@ -407,7 +407,9 @@ export const PythonAdapterPolicy = { mode: 'connect', host, port, - logDir: payload.logDir + logDir: payload.logDir, + // For the answer of an attach that reached something else (issue #1139). + listener: `a debugpy listener (python -m debugpy --listen ${host}:${port} …)` }; } diff --git a/packages/shared/src/interfaces/adapter-policy-ruby.ts b/packages/shared/src/interfaces/adapter-policy-ruby.ts index 6e8e87552..78836029d 100644 --- a/packages/shared/src/interfaces/adapter-policy-ruby.ts +++ b/packages/shared/src/interfaces/adapter-policy-ruby.ts @@ -247,7 +247,9 @@ export const RubyAdapterPolicy = { mode: 'connect', host, port, - logDir: payload.logDir + logDir: payload.logDir, + // For the answer of an attach that reached something else (issue #1139). + listener: `an rdbg debug server (rdbg --open --port ${port} …)` }; } diff --git a/packages/shared/src/interfaces/adapter-policy.ts b/packages/shared/src/interfaces/adapter-policy.ts index 177f2c8e9..4260ab246 100644 --- a/packages/shared/src/interfaces/adapter-policy.ts +++ b/packages/shared/src/interfaces/adapter-policy.ts @@ -1237,6 +1237,15 @@ export type AdapterSpawnConfig = host: string; port: number; logDir: string; + /** + * What should be listening at host:port, as a noun phrase that reads + * after "check that it is" (issue #1139): the server starts nothing + * for this connection, so when the endpoint does not speak DAP the + * likeliest cause is that it is something else — the application's own + * port, say — and the failed attach names what the caller meant to + * reach, with how such a listener is started. + */ + listener?: string; }; /** diff --git a/skills/debugging/SKILL.md b/skills/debugging/SKILL.md index 2d6b9306d..620df3a74 100644 --- a/skills/debugging/SKILL.md +++ b/skills/debugging/SKILL.md @@ -35,7 +35,9 @@ Do NOT reach for it when a single glance at the code or one log line would answe 7. step_over / step_into / step_out / continue_execution wait_for_stop {sessionId} -> blocks until the next stop or the exit (continue_execution does not wait) Each landed step's snapshot carries changedVariables {name: {before, after}} — what the executed - line changed — instead of re-sending full locals; an empty {} means nothing visible changed. + line changed — instead of re-sending full locals; a variable that appeared is {after, new: true}, + one that left scope is {before, gone: true}, and an empty {} means nothing visible changed. + ({after} with no flag: the earlier list was cut at its cap, so "new" is not known.) 8. get_output {sessionId} -> captured debuggee stdout/stderr 9. close_debug_session {sessionId} -> ALWAYS, even on failure ``` diff --git a/src/dap-core/types.ts b/src/dap-core/types.ts index 5c6310b01..0f408c5c9 100644 --- a/src/dap-core/types.ts +++ b/src/dap-core/types.ts @@ -54,6 +54,8 @@ export type ProxyStatusMessage = | { type: 'status'; sessionId: string; status: 'breakpoints_synced'; breakpoints: Array<{ id?: string; file: string; line: number; verified: boolean; adapterId?: number; boundLine?: number; message?: string; refused?: boolean }>; data?: unknown } | { type: 'status'; sessionId: string; status: 'adapter_notice'; note: string; data?: unknown } | { type: 'status'; sessionId: string; status: 'adapter_spawned'; pid?: number; data?: unknown } + /** The worker spawned no adapter: it dials a DAP server that is already listening (issue #1139). `listener` is the policy's phrase for what should be there. */ + | { type: 'status'; sessionId: string; status: 'direct_connect'; host: string; port: number; listener?: string; data?: unknown } | { type: 'status'; sessionId: string; status: 'dap_handshake_stage'; stage: 'transport_connected' | 'request_pending' | 'response_received'; command?: string; data?: unknown } /** The worker's pre-detach resume outcome (issue #763): a continue sent ahead of an attach-mode disconnect. */ | { type: 'status'; sessionId: string; status: 'pre_detach_resume'; trigger: 'detach' | 'close' | 'shutdown'; threadId?: number; resumed: boolean; error?: string; data?: unknown } diff --git a/src/proxy/dap-proxy-worker.ts b/src/proxy/dap-proxy-worker.ts index 2f09f9325..6adbd2d2f 100644 --- a/src/proxy/dap-proxy-worker.ts +++ b/src/proxy/dap-proxy-worker.ts @@ -44,6 +44,7 @@ import { validateProxyInitPayload } from '../utils/type-guards.js'; import { DapResponseError } from './dap-response-error.js'; +import { ErrorMessages, type DirectConnectEndpoint } from '../utils/error-messages.js'; import { SilentDapCommandPayload } from './dap-extensions.js'; // Import adapter policies from shared package import type { AdapterPolicy, AdapterSpecificState, BreakpointFields } from '@debugmcp/shared'; @@ -174,6 +175,15 @@ export class DapProxyWorker { private shutdownPromise: Promise | null = null; /** The init-failure report already sent to the parent, so it goes once. */ private initFailureReported: string | null = null; + /** + * The endpoint this worker dialed when it spawned no adapter (connect + * mode: a debugpy or rdbg attach), with the policy's phrase for what + * should be listening there; null when an adapter was spawned. Behind the + * `direct_connect` status and the wording of a failed init (issue #1139). + */ + private directConnect: DirectConnectEndpoint | null = null; + /** Whether the endpoint answered `initialize` with a success response (issue #1139). */ + private initializeAnswered = false; /** Once-per-session guard for the adapter_capabilities status message (issue #243). */ private adapterCapabilitiesSent: boolean = false; // Exit-code synthesis bookkeeping (issue #247): a real DAP exited event @@ -611,6 +621,9 @@ export class DapProxyWorker { if (!spawnConfig) { throw new Error(`Adapter policy ${this.adapterPolicy.name} does not provide spawn configuration`); } + // Facts of this attempt alone (issue #1139). + this.directConnect = null; + this.initializeAnswered = false; // Spawn mode's connect window (issue #963): the loop ends when the adapter // exits (this controller) or just after the parent's init deadline, whose @@ -702,6 +715,15 @@ export class DapProxyWorker { this.logger!.info( `[Worker] Connecting directly to DAP server at ${spawnConfig.host}:${spawnConfig.port} (no adapter process to monitor)` ); + // The counterpart of `adapter_spawned` (issue #1139): the parent's + // init-timeout diagnosis, and this worker's own failure report, name + // the endpoint instead of an adapter that was never started. + this.directConnect = { + host: spawnConfig.host, + port: spawnConfig.port, + ...(spawnConfig.listener ? { listener: spawnConfig.listener } : {}) + }; + this.sendStatus('direct_connect', { ...this.directConnect }); } // Connect to adapter @@ -927,10 +949,13 @@ export class DapProxyWorker { // the adapter's own exit reaches the parent as `adapter_exited`, which // fails the launch with a generic "Proxy exited during initialization" // and discards the reason sent afterwards — Delve refusing a too-new Go - // toolchain read as a bare exit code 0. - const report = this.describeInitFailure(payload, error); - this.initFailureReported = report; - this.sendError(report); + // toolchain read as a bare exit code 0. A direct connection whose + // socket died has reported already (reportDirectConnectDrop). + if (this.initFailureReported === null) { + const report = this.describeInitFailure(payload, error); + this.initFailureReported = report; + this.sendError(report); + } await this.shutdown(); throw error; } @@ -941,6 +966,11 @@ export class DapProxyWorker { * adapter spawn config (command + args only, NOT env) and its process state. */ private describeInitFailure(payload: ProxyInitPayload, error: unknown): string { + // A direct connection has no adapter command, executable or process to + // describe: what failed is the endpoint the caller named (issue #1139). + if (this.directConnect) { + return this.describeDirectConnectFailure(this.directConnect, error); + } const message = error instanceof Error ? error.message : String(error); const adapterCmd = payload.adapterCommand; const spawnInfo = adapterCmd @@ -952,6 +982,44 @@ export class DapProxyWorker { return `Critical initialization error: ${message} [${diagnostics}]`; } + /** A failed direct-connect init, worded by endpoint (issue #1139). */ + private describeDirectConnectFailure(endpoint: DirectConnectEndpoint, error: unknown): string { + const message = error instanceof Error ? error.message : String(error); + // The connection was made and `initialize` got no answer in DAP — the + // endpoint hung up or reset the connection, or the request died on the + // wire. A refusal in DAP (DapResponseError) is an answer: that endpoint + // is a debug server. + if (this.dapClient && !this.initializeAnswered && !(error instanceof DapResponseError)) { + return `Critical initialization error: ${ErrorMessages.directConnectInitializeUnanswered(endpoint, message)}`; + } + return `Critical initialization error: ${message} [direct connect to ${endpoint.host}:${endpoint.port}; no adapter process was started]`; + } + + /** A direct-connect attach whose handshake has not completed (issue #1139). */ + private inDirectConnectHandshake(): boolean { + return this.directConnect !== null && this.state === ProxyState.INITIALIZING; + } + + /** + * A direct connection that dies during the handshake is the init failure, + * and the socket says so before anything else does: a reset arrives as the + * client's `error` event and a hang-up as its `close` event, both ahead of + * the pending request's rejection reaching the init catch or the + * configuration phase. The parent fails the attach on the first thing it is + * sent — and what a close used to send first was the connection-closed + * status, read as "Proxy exited during initialization". So the first socket + * event sends the init-failure report and latches it; a later event, the + * init catch and the configuration phase then send nothing more. + */ + private reportDirectConnectDrop(reason: Error): void { + if (!this.directConnect || this.initFailureReported !== null) { + return; + } + const report = this.describeDirectConnectFailure(this.directConnect, reason); + this.initFailureReported = report; + this.sendError(report); + } + /** * Await the adapter's initialize response. For launch-mode policies that * declare initializeResponseOptional (rdbg, issue #492), the response is @@ -1056,6 +1124,7 @@ export class DapProxyWorker { payload.sessionId, this.adapterPolicy.getDapAdapterConfiguration().type )).then((caps) => { + this.initializeAnswered = true; this.sendStatus('dap_handshake_stage', { stage: 'response_received', command: 'initialize' }); return caps; }); @@ -1276,10 +1345,20 @@ export class DapProxyWorker { }, onError: (err) => { this.logger!.error('[Worker] DAP client error:', err); + if (this.inDirectConnectHandshake()) { + this.reportDirectConnectDrop(err); + return; + } this.sendError(`DAP client error: ${err.message}`); }, onClose: () => { this.logger!.info('[Worker] DAP client connection closed.'); + // Ahead of the terminal status below: the reason for a failed + // direct-connect handshake, in the words the client gives the + // requests it rejects on a close. + if (this.inDirectConnectHandshake()) { + this.reportDirectConnectDrop(new Error('DAP client disconnected')); + } // Adapters that close the DAP socket at debuggee exit (rdbg) race the // exit-time stdio flush exactly like terminated does — the parent // reacts to dap_connection_closed by tearing down its listeners, so @@ -1744,7 +1823,11 @@ export class DapProxyWorker { program: this.currentInitPayload?.scriptPath }) : undefined; - this.sendError(`Error in DAP sequence: ${message}${explanation ? `. ${explanation}` : ''}`); + // A direct connection that dropped under this phase has reported + // already, by endpoint (reportDirectConnectDrop). + if (this.initFailureReported === null) { + this.sendError(`Error in DAP sequence: ${message}${explanation ? `. ${explanation}` : ''}`); + } await this.shutdown(); } diff --git a/src/proxy/proxy-manager.ts b/src/proxy/proxy-manager.ts index aed6273f7..e61cce1a9 100644 --- a/src/proxy/proxy-manager.ts +++ b/src/proxy/proxy-manager.ts @@ -371,6 +371,8 @@ export class ProxyManager extends EventEmitter implements IProxyManager { this.isStopped = false; this.lastProxyPid = null; this.adapterStderrTail = []; + // The init progress describes the worker this start() launches. + this.initProgress = { transportConnected: false }; // The latch tracks the process this start() launches (issue #530) this.everInitialized = false; this.lastExitDetails = undefined; @@ -1576,6 +1578,21 @@ export class ProxyManager extends EventEmitter implements IProxyManager { this.logger.info(`[ProxyManager] Adapter process spawned (PID ${message.pid ?? 'unknown'})`); break; + case 'direct_connect': + // Progress fact for the init-timeout diagnosis (issue #1139): no + // adapter was spawned, the worker dials a listener that is already + // there. Recorded only when it names an endpoint the message can + // quote; anything else leaves the spawned-adapter wording in force. + if (typeof message.host === 'string' && message.host.length > 0 && typeof message.port === 'number') { + this.initProgress.directConnect = { + host: message.host, + port: message.port, + ...(typeof message.listener === 'string' && message.listener.length > 0 ? { listener: message.listener } : {}) + }; + } + this.logger.info(`[ProxyManager] Direct connect to ${message.host}:${message.port} (no adapter process)`); + break; + case 'pre_detach_resume': this.recordPreDetachResume(message); break; diff --git a/src/server.ts b/src/server.ts index 474296f52..d66e417ef 100644 --- a/src/server.ts +++ b/src/server.ts @@ -16,7 +16,7 @@ import { } from './errors/debug-errors.js'; import { SessionManager, SessionManagerConfig } from './session/session-manager.js'; import type { CloseSessionOutcome, DebugResult, LaunchResultData, StepResultData, WaitForStopResultData } from './session/session-manager-core.js'; -import { FrameSummary, StackTraceResult } from './session/session-manager-data.js'; +import { LocalVariablesResult, StackTraceResult } from './session/session-manager-data.js'; import { VariableTruncationSummary } from './session/variable-caps.js'; import { createProductionDependencies } from './container/dependencies.js'; import { ContainerConfig } from './container/types.js'; @@ -672,13 +672,7 @@ export class DebugMcpServer implements ToolContext { return this.sessionManager.getScopes(sessionId, frameId); } - public async getLocalVariables(sessionId: string, includeSpecial: boolean = false, names?: string[]): Promise<{ - variables: Variable[]; - frame: FrameSummary | null; - scopeName: string | null; - anchorNote?: string; - truncation?: VariableTruncationSummary; - }> { + public async getLocalVariables(sessionId: string, includeSpecial: boolean = false, names?: string[]): Promise { this.validateSession(sessionId); return this.sessionManager.getLocalVariables(sessionId, includeSpecial, names); } diff --git a/src/server/handlers/inspection-tools.ts b/src/server/handlers/inspection-tools.ts index 7140365aa..16e35f2a9 100644 --- a/src/server/handlers/inspection-tools.ts +++ b/src/server/handlers/inspection-tools.ts @@ -4,7 +4,6 @@ */ import { ProtocolError, ProtocolErrorCode } from '@modelcontextprotocol/server'; import { SessionState } from '@debugmcp/shared'; -import { SessionTerminatedError } from '../../errors/debug-errors.js'; import { ErrorMessages } from '../../utils/error-messages.js'; import type { ToolContext, ToolHandler } from '../tool-context.js'; import { enforceExplicitNames, requireSessionId } from '../tool-validation.js'; @@ -13,6 +12,7 @@ import { debuggerOffWhyFor, describeStaleBuild, emittableStopInfo, + endedSessionExtras, hasOpenableSourcePath, variablePayloadExtras } from './shared.js'; @@ -25,6 +25,18 @@ import { type ToolResult } from '../tool-result.js'; +/** + * Two notes as one text: the first is given its full stop when it lacks one + * (the locals anchor notes are clauses joined with '; ', a resolver note is a + * finished sentence). + */ +function joinSentences(first: string | undefined, second: string): string { + if (!first) { + return second; + } + return `${/[.!?]$/.test(first) ? first : `${first}.`} ${second}`; +} + export const getVariablesTool: ToolHandler = async (ctx, args) => { const sessionId = args.sessionId!; const scope = args.scope!; @@ -52,7 +64,8 @@ export const getVariablesTool: ToolHandler = async (ctx, args) => { return jsonResult({ success: true, variables, count: variables.length, variablesReference: scope, ...variablePayloadExtras(variables, args.names, truncation) }); } catch (error) { // Typed session errors report as {success: false}; anything else escapes. - return sessionErrorResultOrThrow(error); + // An ended session says what that means and what to do next (issue #1141). + return sessionErrorResultOrThrow(error, endedSessionExtras(ctx, error)); } }; @@ -122,7 +135,9 @@ export const getStackTraceTool: ToolHandler = async (ctx, args) => { } // No frame of the shown stack is program code and nothing was hidden to // say so (issue #904): a lone sourceless frame (an idle js-debug pause, - // issue #900), or a Java stack its filter handed back whole. + // issue #900), or a Java stack its filter handed back whole. Every frame + // is in this answer, so the sentence never points back at this tool + // (issue #1141). if (stackTrace.noProgramFrame && stackTrace.hiddenFrameCount === 0 && stackTrace.frames[0]) { notes.push(ErrorMessages.stopWithoutProgramFrame(stackTrace.threadId, stackTrace.totalFrameCount, stackTrace.frames[0])); } @@ -173,9 +188,12 @@ export const getStackTraceTool: ToolHandler = async (ctx, args) => { } catch (error) { const sessionResult = sessionErrorToResult(error); if (sessionResult) { + // An ended session gets the sentence get_local_variables always gave + // it, worded for what the session is (issue #1141). const diagnostics = failureDiagnostics(); - return diagnostics - ? sessionErrorToResult(error, { diagnostics })! + const ended = endedSessionExtras(ctx, error); + return diagnostics || ended + ? sessionErrorToResult(error, { ...(diagnostics ? { diagnostics } : {}), ...ended })! : sessionResult; } if (error instanceof Error && !(error instanceof ProtocolError)) { @@ -199,7 +217,7 @@ export const getScopesTool: ToolHandler = async (ctx, args) => { return jsonResult({ success: true, scopes }); } catch (error) { // Typed session errors report as {success: false}; anything else escapes. - return sessionErrorResultOrThrow(error); + return sessionErrorResultOrThrow(error, endedSessionExtras(ctx, error)); } }; @@ -246,7 +264,7 @@ export async function handleEvaluateExpression(ctx: ToolContext, args: { session timestamp: Date.now() }); - return sessionErrorToResult(error) ?? + return sessionErrorToResult(error, endedSessionExtras(ctx, error)) ?? rethrowAsMcpError(error, 'Failed to evaluate expression'); } } @@ -374,6 +392,24 @@ export async function handleGetLocalVariables(ctx: ToolContext, args: { sessionI response.note = result.anchorNote; } + // No frame of the stack is program code (issue #1135): the variables are + // real — the adapter reported them for `frame` — but they are a runtime + // frame's, and the stop's own answer already said so. The flag is the + // one get_stack_trace carries and the sentence the stop answer's; the last clause + // names the frame the variables came from, which is not the stack's top + // after a walk-down, and is left out when there is nothing shown to name. + if (result.noProgramFrame) { + const { threadId, frameCount, top } = result.noProgramFrame; + response.noProgramFrame = true; + const whose = result.variables.length > 0 && result.frame + ? ` The variables shown belong to '${result.frame.name}', not to your program.` + : ''; + response.note = joinSentences( + result.anchorNote, + ErrorMessages.stopWithoutProgramFrame(threadId, frameCount, top, { framesUnseen: frameCount > 1 }) + whose + ); + } + // Surface adapter warnings embedded in the scope name — e.g. Delve // reports "Locals (warning: optimized function)" when the debuggee was // built with optimizations, which typically means missing variables. @@ -418,11 +454,11 @@ export async function handleGetLocalVariables(ctx: ToolContext, args: { sessionI // Handle session state errors specifically. A terminated session is a // normal end state (e.g. a step_out ran the program to completion) — - // explain that instead of implying misuse. - const stateMessage = error instanceof SessionTerminatedError - ? 'The program has terminated, so no frames or variables exist. Use restart_debugging to run it again.' - : 'Cannot get local variables. The session must be paused at a breakpoint.'; - return sessionErrorToResult(error, { message: stateMessage }) ?? + // explain that instead of implying misuse, in the words every inspection + // tool uses for it (issue #1141). + const extras = endedSessionExtras(ctx, error) ?? + { message: 'Cannot get local variables. The session must be paused at a breakpoint.' }; + return sessionErrorToResult(error, extras) ?? rethrowAsMcpError(error, 'Failed to get local variables'); } } diff --git a/src/server/handlers/shared.ts b/src/server/handlers/shared.ts index 1b2187d58..449f12a0d 100644 --- a/src/server/handlers/shared.ts +++ b/src/server/handlers/shared.ts @@ -25,6 +25,7 @@ import type { DebugResult, StopAnswerFields, TreeAnswerFields } from '../../sess import type { AttachRequest } from '../../session/attach/attach-controller.js'; import type { ManagedSession } from '../../session/session-store.js'; import { ErrorMessages } from '../../utils/error-messages.js'; +import { SessionTerminatedError } from '../../errors/debug-errors.js'; import { failureResult, type ToolResult } from '../tool-result.js'; /** @@ -54,6 +55,26 @@ export function childSessionRefusal( ); } +/** + * What an inspection tool adds beside the bare `Session is terminated: ` + * error (issue #1141), or nothing for any other failure. An ended session is + * a normal state — the program finished, the debugger was detached — and + * the answer says so and names the next step, which depends on what the + * session is: the same three cases `restart_debugging` tells apart (a child + * first, then an attach, else a launch). A session row that cannot be read + * gets the launch wording, the one this sentence always had. + */ +export function endedSessionExtras(ctx: ToolContext, error: unknown): { message: string } | undefined { + if (!(error instanceof SessionTerminatedError)) { + return undefined; + } + const session = ctx.sessionManager.getSession(error.sessionId); + if (session?.parentSessionId) { + return { message: ErrorMessages.endedSessionNothingToInspect('child', session.parentSessionId) }; + } + return { message: ErrorMessages.endedSessionNothingToInspect(session?.attachMode === true ? 'attach' : 'launch') }; +} + /** * Whether a session's recorded `lastStop` describes where it is now. The * model keeps the record across continue/step (the run-to-completion summary diff --git a/src/server/tool-context.ts b/src/server/tool-context.ts index a931688a6..d6f7b13aa 100644 --- a/src/server/tool-context.ts +++ b/src/server/tool-context.ts @@ -25,7 +25,7 @@ import type { } from '@debugmcp/shared'; import type { SessionManager } from '../session/session-manager.js'; import type { CloseSessionOutcome, DebugResult, LaunchResultData, StepResultData, WaitForStopResultData } from '../session/session-manager-core.js'; -import type { FrameSummary, StackTraceResult } from '../session/session-manager-data.js'; +import type { LocalVariablesResult, StackTraceResult } from '../session/session-manager-data.js'; import type { VariableTruncationSummary } from '../session/variable-caps.js'; import type { SimpleFileChecker, FileExistenceResult } from '../utils/simple-file-checker.js'; import type { LineReader } from '../utils/line-reader.js'; @@ -114,13 +114,7 @@ export interface ToolContext { }>; getStackTrace(sessionId: string, includeInternals?: boolean, threadId?: number): Promise; getScopes(sessionId: string, frameId: number): Promise; - getLocalVariables(sessionId: string, includeSpecial?: boolean, names?: string[]): Promise<{ - variables: Variable[]; - frame: FrameSummary | null; - scopeName: string | null; - anchorNote?: string; - truncation?: VariableTruncationSummary; - }>; + getLocalVariables(sessionId: string, includeSpecial?: boolean, names?: string[]): Promise; continueExecution(sessionId: string): Promise; stepOver(sessionId: string): Promise>; stepInto(sessionId: string): Promise>; diff --git a/src/session/inspection/expression-evaluator.ts b/src/session/inspection/expression-evaluator.ts index 154debe4c..b61005777 100644 --- a/src/session/inspection/expression-evaluator.ts +++ b/src/session/inspection/expression-evaluator.ts @@ -52,10 +52,17 @@ export interface EvaluateResult { * frame answered without a second stack request. Present on failure too. */ frame?: FrameSummary; + /** + * True when the default anchor is a runtime frame because no frame of the + * stack is program code (issue #1135); `anchorNote` then carries the + * sentence the stop's own answer gave. Never set for an explicit + * frameId: the caller chose the frame. + */ + noProgramFrame?: true; } /** The disclosure fields every return of a default-anchored evaluation carries. */ -type AnchorFields = Pick; +type AnchorFields = Pick; /** * The "variable name" an evaluate expression stands for, for name-based @@ -160,12 +167,23 @@ export class ExpressionEvaluator { anchor.pausedFrame && !anchor.pausedFrame.kept ? ` Pass frameId: ${anchor.pausedFrame.frame.id} to evaluate there.` : ''; - const anchorNote = [anchor.note, anchor.pausedFrameNote && `${anchor.pausedFrameNote}${pausedAdvice}`] + // No frame of the stack is program code (issue #1135): the expression + // runs in a runtime frame, and the answer says so in the sentence the + // stack and the stop answers use for the same stop. + const top = anchor.frames[0]; + const outsideProgram = anchor.noProgramFrame && top + ? `${ErrorMessages.stopWithoutProgramFrame(anchor.threadId, anchor.totalFrameCount, top, { + framesUnseen: anchor.totalFrameCount > 1 + })} The expression was evaluated in that frame.` + : undefined; + const anchorNote = [anchor.note, anchor.pausedFrameNote && `${anchor.pausedFrameNote}${pausedAdvice}`, outsideProgram] .filter((part): part is string => !!part) .join(' '); - anchorFields = anchorNote ? { anchorNote } : {}; - if (anchor.frames.length > 0) { - const top = anchor.frames[0]; + anchorFields = { + ...(anchorNote ? { anchorNote } : {}), + ...(outsideProgram ? { noProgramFrame: true as const } : {}) + }; + if (top) { frameId = top.id; anchorFields = { ...anchorFields, frame: frameSummary(top) }; this.ctx.logger.info( diff --git a/src/session/inspection/stop-location.ts b/src/session/inspection/stop-location.ts index c21832a51..b33c91d40 100644 --- a/src/session/inspection/stop-location.ts +++ b/src/session/inspection/stop-location.ts @@ -75,7 +75,11 @@ export async function describeStopLocation( } if (result.noProgramFrame && top) { landing.noProgramFrame = true; - notes.push(ErrorMessages.stopWithoutProgramFrame(result.threadId, result.totalFrameCount, top)); + // The answer shows one frame, as `location`; the rest of the thread's + // stack, when it has one, is get_stack_trace's to list (issue #1141). + notes.push(ErrorMessages.stopWithoutProgramFrame(result.threadId, result.totalFrameCount, top, { + framesUnseen: result.totalFrameCount > 1 + })); } if (notes.length > 0) { landing.note = notes.join(' '); diff --git a/src/session/inspection/stop-snapshot.ts b/src/session/inspection/stop-snapshot.ts index 3f410b700..981adec69 100644 --- a/src/session/inspection/stop-snapshot.ts +++ b/src/session/inspection/stop-snapshot.ts @@ -17,7 +17,7 @@ */ import { SessionState, redactStopInfo } from '@debugmcp/shared'; import type { OperationsContext } from '../operations-context.js'; -import type { SnapshotException, StopSnapshot } from '../session-manager-core.js'; +import type { ChangedVariable, SnapshotException, StopSnapshot } from '../session-manager-core.js'; import type { StackTraceResult } from './frame-anchor-resolver.js'; import { frameSummary } from './frame-anchor-resolver.js'; import { applyVariableCaps, snapshotLocalsLimits } from '../variable-caps.js'; @@ -67,6 +67,43 @@ function setOwn(target: Record, key: string, value: T): void { Object.defineProperty(target, key, { value, enumerable: true, writable: true, configurable: true }); } +/** + * What differs between the locals last shown and the ones shown now (issue + * #1140): a changed value, a variable that appeared, a variable that left + * scope. No placeholder stands in for a missing side — any text can be a + * value — and a flag is only set when it is known: + * + * - `new` needs the BASE list to have been complete: a name missing from a + * list that was cut by the snapshot's cap may only not have fitted; + * - `gone` needs the CURRENT list to be complete, for the same reason turned + * round. Whether the base was cut does not matter for it: a variable that + * was shown and is absent from a complete list has left scope. + */ +function diffLocals( + base: { rendered: Record; complete: boolean }, + now: Record, + nowComplete: boolean +): Record { + const has = (map: Record, name: string): boolean => + Object.prototype.hasOwnProperty.call(map, name); + const changed: Record = {}; + for (const name of Object.keys(now)) { + if (!has(base.rendered, name)) { + setOwn(changed, name, { after: now[name], ...(base.complete ? { new: true as const } : {}) }); + } else if (base.rendered[name] !== now[name]) { + setOwn(changed, name, { before: base.rendered[name], after: now[name] }); + } + } + if (nowComplete) { + for (const name of Object.keys(base.rendered)) { + if (!has(now, name)) { + setOwn(changed, name, { before: base.rendered[name], gone: true as const }); + } + } + } + return changed; +} + /** * Compose the snapshot for one answer. Call it once per ANSWER — a step that * repeats on the same line (Dart) reads the landing per attempt but composes @@ -190,6 +227,20 @@ export async function composeStopSnapshot( return undefined; } else { const cappedLocals = applyVariableCaps(raced.variables, snapshotLocalsLimits()); + // Omissions merge from both passes; CUTS count only at the + // snapshot's own cap (a value cut upstream and re-cut here is one + // cut value, not two); unread scopes come from the pipeline. + const omitted = + (raced.truncation?.omittedCount ?? 0) + (cappedLocals.truncation?.omittedCount ?? 0); + const cut = cappedLocals.truncation?.valueTruncatedCount + ?? raced.truncation?.valueTruncatedCount ?? 0; + const scopesUnread = raced.truncation?.scopesSkipped ?? 0; + // Whether the list about to be shown is every local the frame has + // (issue #1140). A shortened value still names its variable, so + // cuts do not count; an omitted variable or an unread scope does. + // A failed scope request is unread too, even when another scope + // supplied enough locals to return a non-empty answer. + const complete = omitted === 0 && scopesUnread === 0 && !raced.incomplete; if (cappedLocals.variables.length === 0 && stackResult.frames.length > SNAPSHOT_FRAMES) { // The capped probe may simply not have reached the frames that // have locals; silence here would be indistinguishable from @@ -221,32 +272,15 @@ export async function composeStopSnapshot( if (stop && base && base.stopRef === stop) { snapshot.locals = flat; } else if (stop && base && base.frameKey === frameKey) { - const changed: Record = {}; - for (const name of Object.keys(flat)) { - const before = Object.prototype.hasOwnProperty.call(base.rendered, name) - ? base.rendered[name] - : ''; - if (before !== flat[name]) { - setOwn(changed, name, { before, after: flat[name] }); - } - } - snapshot.changedVariables = changed; - session.snapshotDiffBase = { stopRef: stop, frameKey, rendered: flat }; + snapshot.changedVariables = diffLocals(base, flat, complete); + session.snapshotDiffBase = { stopRef: stop, frameKey, rendered: flat, complete }; } else { snapshot.locals = flat; if (stop) { - session.snapshotDiffBase = { stopRef: stop, frameKey, rendered: flat }; + session.snapshotDiffBase = { stopRef: stop, frameKey, rendered: flat, complete }; } } } - // Omissions merge from both passes; CUTS count only at the - // snapshot's own cap (a value cut upstream and re-cut here is one - // cut value, not two); unread scopes come from the pipeline. - const omitted = - (raced.truncation?.omittedCount ?? 0) + (cappedLocals.truncation?.omittedCount ?? 0); - const cut = cappedLocals.truncation?.valueTruncatedCount - ?? raced.truncation?.valueTruncatedCount ?? 0; - const scopesUnread = raced.truncation?.scopesSkipped ?? 0; const parts: string[] = []; if (omitted > 0) parts.push(`${omitted} variable(s) omitted`); if (cut > 0) parts.push(`${cut} value(s) cut`); diff --git a/src/session/session-manager-core.ts b/src/session/session-manager-core.ts index 60d9d4034..628ff0755 100644 --- a/src/session/session-manager-core.ts +++ b/src/session/session-manager-core.ts @@ -160,6 +160,32 @@ export interface SnapshotException { details?: string; } +/** + * One entry of a snapshot's `changedVariables` (issue #1140). The side that + * does not exist is left out, and a flag says why — no placeholder stands in + * for a value, because any text can be a value: + * + * - `{ before, after }`: the value changed; + * - `{ after, new: true }`: the variable was not among the locals last shown; + * - `{ before, gone: true }`: it was, and is no longer among the locals. + * + * A flag is only set when it is known. The locals a snapshot shows are + * capped, so a name missing from a list that was cut may simply not have + * fitted, and a failed scope read also makes a list incomplete. A variable + * absent from an incomplete previous list is `{ after }` with no + * `new`, and one absent from an incomplete current list is not listed at all. + */ +export interface ChangedVariable { + /** The value last shown. Absent for a variable that was not among the locals last shown. */ + before?: string; + /** The value now. Absent for a variable that is no longer among the locals. */ + after?: string; + /** The variable was not there at the previous stop, whose list was complete. */ + new?: true; + /** The variable is no longer among the locals, whose list is complete. */ + gone?: true; +} + /** * The bounded state snapshot a PAUSED answer carries (issue #982), composed * from the landing read's own stack: top visible frames, flat redacted @@ -173,11 +199,12 @@ export interface StopSnapshot { locals?: Record; /** * In place of `locals` on a repeated stop in the same frame (issue #983): - * only what changed since the last shown locals (new variables appear with - * before ""; one that left scope simply drops out). An empty - * object means nothing visible changed — that is information. + * only what differs from the locals last shown, one entry per variable — + * a changed value, a variable that appeared, a variable that left scope + * (`ChangedVariable`, issue #1140). An empty object means nothing visible + * changed — that is information. */ - changedVariables?: Record; + changedVariables?: Record; /** What was cut, pointing at get_local_variables for the full list. */ truncation?: string; /** What expired or was withheld, and any locals anchor note. */ diff --git a/src/session/session-manager-data.ts b/src/session/session-manager-data.ts index b24cd5305..d589e07ac 100644 --- a/src/session/session-manager-data.ts +++ b/src/session/session-manager-data.ts @@ -44,9 +44,42 @@ export interface LocalVariablesResult { * Both notes may apply; they are joined with '; '. */ anchorNote?: string; + /** + * Present when no frame of the resolved stack is program code (issue + * #1135): the resolver's own verdict (`StackTraceResult.noProgramFrame`), + * with what the sentence about it needs — the thread, how many frames it + * reported, and its top frame. The variables are still the ones the + * adapter reported for `frame`; this says whose frame that is. + */ + noProgramFrame?: NoProgramFrameVerdict; + /** A scope read failed; an absent variable cannot be treated as absent from the frame. */ + incomplete?: true; truncation?: VariableTruncationSummary; } +/** The facts behind `ErrorMessages.stopWithoutProgramFrame` for a resolved stack. */ +export interface NoProgramFrameVerdict { + threadId?: number; + frameCount: number; + top: FrameSummary; +} + +/** + * The resolver's "no program frame" verdict as the inspection answers carry + * it, or nothing when the stack reaches the program (or has no frame at all). + */ +export function noProgramFrameVerdict(stackResult: StackTraceResult): NoProgramFrameVerdict | undefined { + const top = stackResult.frames[0]; + if (!stackResult.noProgramFrame || !top) { + return undefined; + } + return { + ...(typeof stackResult.threadId === 'number' ? { threadId: stackResult.threadId } : {}), + frameCount: stackResult.totalFrameCount, + top: frameSummary(top) + }; +} + /** * Stack trace frames plus filtering metadata (issue #346): how many frames the * adapter reported, how many the language policy hid, and whether the @@ -135,17 +168,19 @@ export abstract class SessionManagerData extends SessionManagerCore { async getVariablesDetailed(sessionId: string, variablesReference: number, names?: string[]): Promise<{ variables: Variable[]; truncation?: VariableTruncationSummary; + /** Internal observation status: a failed read is not an empty scope. */ + readFailed?: true; }> { const session = this._getSessionById(sessionId); this.logger.info(`[SM getVariables ${sessionId}] Entered. variablesReference: ${variablesReference}, Current state: ${session.state}`); if (!session.proxyManager || !session.proxyManager.isRunning()) { this.logger.warn(`[SM getVariables ${sessionId}] No active proxy.`); - return { variables: [] }; + return { variables: [], readFailed: true }; } if (session.state !== SessionState.PAUSED) { this.logger.warn(`[SM getVariables ${sessionId}] Session not paused. State: ${session.state}.`); - return { variables: [] }; + return { variables: [], readFailed: true }; } try { @@ -155,7 +190,7 @@ export abstract class SessionManagerData extends SessionManagerCore { // parsed (post-redaction) values are logged below. this.logger.info(`[SM getVariables ${sessionId}] DAP 'variables' response received. ${response?.body?.variables?.length ?? 0} variable(s).`); - if (response && response.body && response.body.variables) { + if (response?.success !== false && response?.body?.variables) { let vars = response.body.variables.map((v: DebugProtocol.Variable) => ({ name: v.name, value: v.value, type: v.type || "", variablesReference: v.variablesReference, @@ -189,10 +224,10 @@ export abstract class SessionManagerData extends SessionManagerCore { return capped; } this.logger.warn(`[SM getVariables ${sessionId}] No variables in response body for reference ${variablesReference}. Response:`, response); - return { variables: [] }; + return { variables: [], readFailed: true }; } catch (error) { this.logger.error(`[SM getVariables ${sessionId}] Error getting variables:`, error); - return { variables: [] }; + return { variables: [], readFailed: true }; } } @@ -226,11 +261,14 @@ export abstract class SessionManagerData extends SessionManagerCore { } async getScopes(sessionId: string, frameId: number): Promise { - return this.readScopes(sessionId, frameId, 'public'); + return (await this.readScopes(sessionId, frameId, 'public')).scopes; } /** Internal frame-walk probe: an unsupported/frameless candidate is expected. */ - private async probeScopes(sessionId: string, frameId: number): Promise { + private async probeScopes(sessionId: string, frameId: number): Promise<{ + scopes: DebugProtocol.Scope[]; + readFailed?: true; + }> { return this.readScopes(sessionId, frameId, 'probe'); } @@ -238,18 +276,18 @@ export abstract class SessionManagerData extends SessionManagerCore { sessionId: string, frameId: number, purpose: 'public' | 'probe' - ): Promise { + ): Promise<{ scopes: DebugProtocol.Scope[]; readFailed?: true }> { const session = this._getSessionById(sessionId); const prefix = purpose === 'public' ? 'getScopes' : 'scopeProbe'; this.logger.info(`[SM ${prefix} ${sessionId}] Entered. frameId: ${frameId}, Current state: ${session.state}`); if (!session.proxyManager || !session.proxyManager.isRunning()) { this.logger.warn(`[SM ${prefix} ${sessionId}] No active proxy.`); - return []; + return { scopes: [], readFailed: true }; } if (session.state !== SessionState.PAUSED) { this.logger.warn(`[SM ${prefix} ${sessionId}] Session not paused. State: ${session.state}.`); - return []; + return { scopes: [], readFailed: true }; } try { @@ -262,11 +300,11 @@ export abstract class SessionManagerData extends SessionManagerCore { if (response && response.body && response.body.scopes) { this.logger.info(`[SM getScopes ${sessionId}] Parsed scopes:`, response.body.scopes.map(s => ({name: s.name, ref: s.variablesReference, expensive: s.expensive }))); - return response.body.scopes; + return { scopes: response.body.scopes }; } const logMissing = purpose === 'public' ? this.logger.warn.bind(this.logger) : this.logger.debug.bind(this.logger); logMissing(`[SM ${prefix} ${sessionId}] No scopes in response body for frameId ${frameId}.`, response); - return []; + return { scopes: [], readFailed: true }; } catch (error) { if (purpose === 'public') { this.logger.error(`[SM getScopes ${sessionId}] Error getting scopes:`, error); @@ -275,7 +313,7 @@ export abstract class SessionManagerData extends SessionManagerCore { `[SM scopeProbe ${sessionId}] Frame ${frameId} did not provide scopes: ${error instanceof Error ? error.message : String(error)}` ); } - return []; + return { scopes: [], readFailed: true }; } } @@ -348,8 +386,11 @@ export abstract class SessionManagerData extends SessionManagerCore { // Step 2: Collect all scopes for all frames (may need multiple frames for closures) const scopesMap: Record = {}; + const failedScopeFrames = new Set(); for (const frame of stackFrames) { - const scopes = await this.probeScopes(sessionId, frame.id); + const scopeResult = await this.probeScopes(sessionId, frame.id); + if (scopeResult.readFailed) failedScopeFrames.add(frame.id); + const scopes = scopeResult.scopes; if (scopes && scopes.length > 0) { scopesMap[frame.id] = scopes; } @@ -366,10 +407,15 @@ export abstract class SessionManagerData extends SessionManagerCore { const truncationByScope = new Map(); let fetchedCount = 0; let scopeFetchesSkipped = 0; + let scopeReadFailed = false; const fetchBudget = maxVariablesPerCall(); const policy = this.selectPolicy(session.language); for (let frameIndex = 0; frameIndex < stackFrames.length; frameIndex++) { const frame = stackFrames[frameIndex]; + // Discovery probes lower frames up front, but their failures do not + // affect locals found above them. Only reads on the traversed path + // can make this answer incomplete. + scopeReadFailed ||= failedScopeFrames.has(frame.id); const scopes = scopesMap[frame.id]; if (!scopes) continue; for (const scope of scopes) { @@ -379,6 +425,10 @@ export abstract class SessionManagerData extends SessionManagerCore { continue; } const detailed = await this.getVariablesDetailed(sessionId, scope.variablesReference, names); + // An empty failed scope may be absent from the policy's selected + // refs (e.g. a JS Block beside a readable Local). Keep this status + // independently of truncation attributed to contributing scopes. + scopeReadFailed ||= detailed.readFailed === true; truncationByScope.set(scope.variablesReference, detailed.truncation); fetchedCount += detailed.variables.length; if (detailed.variables.length > 0) { @@ -596,12 +646,21 @@ export abstract class SessionManagerData extends SessionManagerCore { if (scopeNote) { anchorNotes.push(scopeNote); } + if (scopeReadFailed) { + anchorNotes.push('Some variable scopes could not be read; the locals list may be incomplete'); + } + + // No frame of this stack is program code (issue #1135): the variables + // are a runtime frame's, and the answer must be able to say so. + const noProgramFrame = noProgramFrameVerdict(stackResult); return { variables: cappedLocals.variables, frame: frameSummary(anchorFrame), scopeName, ...(anchorNotes.length > 0 ? { anchorNote: anchorNotes.join('; ') } : {}), + ...(noProgramFrame ? { noProgramFrame } : {}), + ...(scopeReadFailed ? { incomplete: true as const } : {}), ...(truncation ? { truncation } : {}) }; diff --git a/src/session/session-store.ts b/src/session/session-store.ts index b2c6238b6..82ad4b3df 100644 --- a/src/session/session-store.ts +++ b/src/session/session-store.ts @@ -164,6 +164,13 @@ export interface ManagedSession extends DebugSessionInfo { stopRef: SessionStopInfo; frameKey: string; rendered: Record; + /** + * Whether `rendered` is every local the frame had: no variable omitted by + * a cap, no scope left unread or failed to read (issue #1140). A name + * missing from a cut list may only not have fitted, so only a complete + * base can call a variable new. + */ + complete: boolean; }; // Monotonic identity for each proxy installed on this session. A late event // from an older worker cannot satisfy intent armed against a newer one. diff --git a/src/skill-content.ts b/src/skill-content.ts index 883f97171..85513a80d 100644 --- a/src/skill-content.ts +++ b/src/skill-content.ts @@ -50,7 +50,7 @@ Golden path: create_debug_session -> set_breakpoint (ABSOLUTE file path) -> star Failing test (Python): start_debugging {sessionId, scriptPath: ABSOLUTE test file, testRunner: "pytest", testTarget: "test_name"} — no breakpoint needed; the answer is paused at the failing assert (or the raise site of an exception that escapes the test) with snapshot.exception, frames and locals, and a passing test runs to completion (exit code 0). list_supported_languages shows testRunners per language. Key rules: -- Every paused answer (start/restart/wait_for_stop/pause/attach and the steps) already carries snapshot {exception?, frames, locals | changedVariables} — the exception message, top frames and the top frame's locals, redacted and capped. A repeated stop in the same frame (stepping, a continue+wait loop over one breakpoint) carries changedVariables {name: {before, after}} instead of locals — an empty {} means the line changed nothing visible. Read it before calling get_stack_trace/get_local_variables; call those (and get_scopes -> get_variables with a variablesReference) only to expand structures, see more frames, or fetch the full list (snapshot.truncation says when there is more). Entry stops carry no snapshot. +- Every paused answer (start/restart/wait_for_stop/pause/attach and the steps) already carries snapshot {exception?, frames, locals | changedVariables} — the exception message, top frames and the top frame's locals, redacted and capped. A repeated stop in the same frame (stepping, a continue+wait loop over one breakpoint) carries changedVariables instead of locals — {name: {before, after}} for a changed value, {after, new: true} for a variable that appeared, {before, gone: true} for one that left scope ({after} with no flag when the earlier list was cut at its cap, so "new" is not known); an empty {} means the line changed nothing visible. Read it before calling get_stack_trace/get_local_variables; call those (and get_scopes -> get_variables with a variablesReference) only to expand structures, see more frames, or fetch the full list (snapshot.truncation says when there is more). Entry stops carry no snapshot. - Stepping/evaluation/variable reads require the session to be PAUSED; the stop reason on each pause tells you why it stopped. - continue_execution returns at once, and a launch, step, pause or attach may answer pending:true when its stop has not arrived yet. Nothing is cancelled — breakpoints stay armed. Call wait_for_stop {sessionId} to block until the session pauses or ends (it reports the stop, or the exit code); a wait that runs out answers pending:true again, so call it again. A session whose program spawned child sessions (Python and JavaScript, on by default for launches; JavaScript attaches enable children only when local debugger access is verified; dapLaunchArgs subProcess: false or autoAttachChildProcesses: false opts a launch out) is waited on as a tree: the answer's sessionId names the session that stopped — use it to inspect — and children lists the rest with their states. - If a variable entry has a variablesReference, call get_variables with it to expand children. diff --git a/src/utils/breakpoint-resolver.ts b/src/utils/breakpoint-resolver.ts index 5e6064bef..76e1bbefe 100644 --- a/src/utils/breakpoint-resolver.ts +++ b/src/utils/breakpoint-resolver.ts @@ -8,6 +8,8 @@ const CONTEXT_LINES = 2; const MAX_LISTED_MATCHES = 20; +/** How many match lines the expectedContent hint names before "and N more". */ +const MAX_HINTED_MATCHES = 5; /** * How an expectedContent assertion matched (issue #379): 'exact' is trimmed @@ -81,44 +83,22 @@ function stripStatementTerminator(trimmedLine: string): string { } /** - * Check that `line` contains the expected content. Accepted, in order: - * exact trimmed-line equality, then exact equality against the - * comment-stripped actual line (the expectation is the line's entire code, - * issue #440), then the trimmed expectation as a substring of the actual - * line, then as a substring of the comment-stripped actual line — so - * trailing comments and partial-line anchors both work. - * On mismatch the message shows expected vs actual plus surrounding context - * so the agent can pick the correct line without another read. + * How `expectedContent` matches one source line, or null when it does not. + * Accepted, in order: exact trimmed-line equality, then exact equality + * against the comment-stripped line (the expectation is the line's entire + * code, issue #440), then the same without the line's statement terminator + * (issue #1038), then the trimmed expectation as a substring of the line, + * then as a substring of the comment-stripped line — so trailing comments + * and partial-line anchors both work. + * + * The one place these rules live: `assertLineContent` reads it for the line + * the caller named, and its mismatch message reads it for every other line + * (issue #1142), so what the message says about the file is this check's + * own verdict. */ -export function assertLineContent( - lines: string[], - line: number, - expectedContent: string, - filePath: string, - options?: AssertLineContentOptions -): LineContentAssertion { - if (expectedContent.trim() === '') { - return { - ok: false, - actual: null, - message: - 'Breakpoint not set: expectedContent is empty or whitespace-only. ' + - 'Provide a distinctive substring of the target line, or omit expectedContent to skip verification.', - }; - } - - if (line < 1 || line > lines.length) { - return { - ok: false, - actual: null, - message: - `Breakpoint not set: line ${line} of ${filePath} does not exist (file has ${lines.length} lines). ` + - `Re-read the file and pick a valid line.`, - }; - } - +function matchLineContent(rawLine: string, expectedContent: string): ContentMatchQuality | null { const expected = expectedContent.trim(); - const actual = lines[line - 1].trim(); + const actual = rawLine.trim(); // Exact trimmed match, then comment-stripped-exact (whole code of a // commented line, #440), then substring of the actual line. Then retry // substring with trailing comments stripped from BOTH sides: stripping @@ -130,7 +110,7 @@ export function assertLineContent( // collapses lines that differ only past a '//' or '#' — even inside // string literals. if (actual === expected) { - return { ok: true, matchQuality: 'exact', actual }; + return 'exact'; } // Whole-code match on a line that merely carries a trailing comment // (issue #440): exact-equivalent, no warning. The expectation-side guard @@ -139,9 +119,9 @@ export function assertLineContent( // literal ("http://a" vs "http://b") into a clean match. if ( stripTrailingComment(expectedContent).trim() === expected && - stripTrailingComment(lines[line - 1]).trim() === expected + stripTrailingComment(rawLine).trim() === expected ) { - return { ok: true, matchQuality: 'comment-stripped-exact', actual }; + return 'comment-stripped-exact'; } // The whole statement without the line's terminator (issue #1038): a COBOL // period, a C or JS semicolon — the one character an agent quoting a @@ -150,19 +130,61 @@ export function assertLineContent( if ( stripStatementTerminator(actual) === expected || (stripTrailingComment(expectedContent).trim() === expected && - stripStatementTerminator(stripTrailingComment(lines[line - 1]).trim()) === expected) + stripStatementTerminator(stripTrailingComment(rawLine).trim()) === expected) ) { - return { ok: true, matchQuality: 'terminator-stripped-exact', actual }; + return 'terminator-stripped-exact'; } if (actual.includes(expected)) { - return { ok: true, matchQuality: 'substring', actual }; + return 'substring'; } const strippedExpected = stripTrailingComment(expectedContent).trim(); if ( strippedExpected !== '' && - stripTrailingComment(lines[line - 1]).trim().includes(strippedExpected) + stripTrailingComment(rawLine).trim().includes(strippedExpected) ) { - return { ok: true, matchQuality: 'comment-stripped', actual }; + return 'comment-stripped'; + } + return null; +} + +/** + * Check that `line` contains the expected content, by the rules of + * `matchLineContent`. On mismatch the message shows expected vs actual plus + * surrounding context so the agent can pick the correct line without another + * read. + */ +export function assertLineContent( + lines: string[], + line: number, + expectedContent: string, + filePath: string, + options?: AssertLineContentOptions +): LineContentAssertion { + if (expectedContent.trim() === '') { + return { + ok: false, + actual: null, + message: + 'Breakpoint not set: expectedContent is empty or whitespace-only. ' + + 'Provide a distinctive substring of the target line, or omit expectedContent to skip verification.', + }; + } + + if (line < 1 || line > lines.length) { + return { + ok: false, + actual: null, + message: + `Breakpoint not set: line ${line} of ${filePath} does not exist (file has ${lines.length} lines). ` + + `Re-read the file and pick a valid line.`, + }; + } + + const expected = expectedContent.trim(); + const actual = lines[line - 1].trim(); + const matchQuality = matchLineContent(lines[line - 1], expectedContent); + if (matchQuality) { + return { ok: true, matchQuality, actual }; } let message = @@ -172,11 +194,78 @@ export function assertLineContent( `Context:\n${formatContext(lines, line)}\n` + `The file may have changed since you last read it. Pick the correct line from the context above.`; if (options?.statementHint) { - message += ` Or address by content: set_breakpoint {statement: "${expected}"}.`; + message += describeWhereItMatches(lines, line, expectedContent, filePath); } return { ok: false, actual, message }; } +/** "4", "4 and 6", "2, 3, 4, 5, 6 and 7 more". */ +function listLines(matches: number[]): string { + if (matches.length > MAX_HINTED_MATCHES) { + return `${matches.slice(0, MAX_HINTED_MATCHES).join(', ')} and ${matches.length - MAX_HINTED_MATCHES} more`; + } + return matches.length === 1 + ? String(matches[0]) + : `${matches.slice(0, -1).join(', ')} and ${matches[matches.length - 1]}`; +} + +/** + * What an expectedContent mismatch says about where the expected text is + * (issue #1142). The mismatched text used to be offered as a `statement` + * whatever it was, so an expectation that is nowhere in the file earned a + * hint whose own outcome is "statement … not found". Each claim now comes + * from the function that owns it, so the message and that function cannot + * disagree: + * + * - `resolveStatement` — what `set_breakpoint {statement}` runs — finds the + * text on one line: the hint offers the statement and names the line; + * - it finds it on several: the hint adds `nearLine` (the line the caller + * asked for) and says which lines, and which one that picks; + * - it does not (the text is absent, or it is only where the resolver does + * not look: a comment line, or past a `//` or `#`): no statement is + * offered, and the message names the lines this same expectedContent check + * passes on (`matchLineContent`), or says that it passes on none; + * - text spanning lines: nothing — no single line can match it. + */ +function describeWhereItMatches( + lines: string[], + line: number, + expectedContent: string, + filePath: string +): string { + const expected = expectedContent.trim(); + if (expected.includes('\n')) { + return ''; + } + // A comment or a blank is not a statement anchor: the resolver would + // refuse it for what it is. + if (!isCommentOrBlank(expected)) { + // With nearLine given the resolver cannot answer "ambiguous": its only + // refusal here is "not found". + const resolution = resolveStatement(lines, expected, filePath, line); + if (resolution.ok && resolution.candidates) { + return ( + ` Or address by content: set_breakpoint {statement: "${expected}", nearLine: ${line}} — ` + + `that text is on lines ${listLines(resolution.candidates)}; nearLine picks the closest (line ${resolution.line}).` + ); + } + if (resolution.ok) { + return ` Or address by content: set_breakpoint {statement: "${expected}"} — that text is on line ${resolution.line}.`; + } + } + // The line the caller named is not among them: it just failed this check. + const passing: number[] = []; + for (let i = 0; i < lines.length; i++) { + if (matchLineContent(lines[i], expectedContent) !== null) { + passing.push(i + 1); + } + } + if (passing.length === 0) { + return ' That expectedContent matches no line of the file.'; + } + return ` That expectedContent matches line${passing.length === 1 ? '' : 's'} ${listLines(passing)}.`; +} + /** * Whether a statement anchor is a blank line or a comment-only line under a * language-agnostic prefix heuristic (#, //, /*). Deliberately does NOT treat diff --git a/src/utils/error-messages.ts b/src/utils/error-messages.ts index ab1415df1..cafa7289a 100644 --- a/src/utils/error-messages.ts +++ b/src/utils/error-messages.ts @@ -16,8 +16,32 @@ export interface ProxyInitProgress { transportConnected: boolean; /** The DAP handshake request that was sent and has not been answered. */ pendingCommand?: string; + /** + * Present when the worker spawned no adapter and dialed a DAP server that + * was already listening — a debugpy or rdbg attach (issue #1139). The + * endpoint is the caller's own host and port, so a stall there is not "an + * adapter that started": the wording names the endpoint instead. + */ + directConnect?: DirectConnectEndpoint; +} + +/** The endpoint of a direct-connect attach, as the worker dialed it. */ +export interface DirectConnectEndpoint { + host: string; + port: number; + /** The policy's phrase for what should be listening there ("a debugpy listener (…)"). */ + listener?: string; } +/** + * The second sentence of every direct-connect failure where the endpoint + * never spoke DAP (issue #1139): nothing of ours was started, so the likeliest + * cause is the endpoint itself. + */ +const notADebugServer = (endpoint: DirectConnectEndpoint): string => + 'Nothing was started for this attach: the server connected to what is already listening on that port, ' + + `so check that it is ${endpoint.listener ?? "the debugger's own listener"} and not another service.`; + /** * Why no uncaught exception will stop a running program (issues #1009, #1032), * settled once per launch or attach by the session layer's exception arming: @@ -125,11 +149,39 @@ export const ErrorMessages = { * when the adapter demonstrably connected and answered events sent agents * to verify healthy toolchains with nowhere to go afterwards. * + * + * A direct-connect attach (issue #1139) has its own wording on every + * branch: the server started nothing, so "the adapter started", "retrying + * usually succeeds" and "check that the adapter is installed" are all + * false there. While `initialize` is unanswered the endpoint has not + * spoken DAP at all, and the likeliest cause is that it is not a debug + * server — the wrong port; once it has answered, it is one, and the stall + * is its own. + * * @param timeout - The timeout duration in seconds * @param progress - How far initialization got (issue #493); omit for the generic message */ proxyInitTimeout: (timeout: number, progress?: ProxyInitProgress) => { const base = `Debug proxy initialization did not complete within ${timeout}s.`; + const direct = progress?.directConnect; + if (direct) { + const endpoint = `${direct.host}:${direct.port}`; + if (!progress.transportConnected) { + return `${base} No connection to ${endpoint} was established. Nothing was started for this attach: ` + + `check the host and port, and that ${direct.listener ?? 'the debug server'} is listening there and reachable from this machine.`; + } + if (progress.pendingCommand === 'initialize') { + return `${base} ${endpoint} accepted the connection but never answered the DAP "initialize" request. ` + + `${notADebugServer(direct)} Retrying the same port stalls the same way.`; + } + if (progress.pendingCommand) { + return `${base} ${endpoint} answered the start of the DAP handshake, but the "${progress.pendingCommand}" request ` + + 'never received a response. Nothing was started for this attach: this is a stall in the debug server listening ' + + 'on that port, not a missing install. If it recurs, capture a DAP trace (DAP_TRACE=1) of the failing attach.'; + } + return `${base} Connected to ${endpoint} and the DAP handshake began, but initialization stalled before completing. ` + + 'Nothing was started for this attach: the debug server listening on that port is what stalled.'; + } if (progress?.transportConnected) { const pidNote = progress.adapterPid !== undefined ? ` The adapter process is running (PID ${progress.adapterPid}).` @@ -153,6 +205,20 @@ export const ErrorMessages = { `Check that the required debug adapter is installed and accessible.`; }, + /** + * A direct-connect attach whose endpoint never answered `initialize` and + * did not stall either (issue #1139): it hung up, or the request failed on + * the wire. What an HTTP server does with a DAP frame — the issue's own + * example, the application's port given instead of the debugger's. The + * worker's init-failure report used to read "DAP client disconnected + * [Executable: python | adapter PID=none exitCode=n/a]". + * Used in: src/proxy/dap-proxy-worker.ts (describeInitFailure) + * @param endpoint - What the worker dialed + * @param detail - The failure as the DAP client reported it + */ + directConnectInitializeUnanswered: (endpoint: DirectConnectEndpoint, detail: string) => + `${endpoint.host}:${endpoint.port} never answered the DAP "initialize" request (${detail}). ${notADebugServer(endpoint)}`, + /** * Informational message for step operations still executing after the grace window * Occurs when: A step operation (stepOver, stepInto, stepOut) doesn't receive a 'stopped' event @@ -243,21 +309,30 @@ export const ErrorMessages = { * `` frame between event-loop dispatches (issue #900); a Windows * pause on a native helper thread that no sibling could replace. Worded * for every adapter — what the frame is, the language guide says. - * Used in: src/session/inspection/stop-location.ts + * + * The pointer at the full stack is offered only when the thread has frames + * the answer has not shown (issue #1141): `get_stack_trace` says this + * sentence exactly when it hid nothing, where "includeInternals: true lists + * the frames" told a caller who had passed it to pass it; and a one-frame + * stack has no more to list than the frame the sentence already names. + * Used in: src/session/inspection/stop-location.ts, + * src/server/handlers/inspection-tools.ts * @param threadId - The thread the answer is anchored on, when known * @param frameCount - How many frames the thread reported * @param top - The frame shown + * @param opts.framesUnseen - The thread has frames this answer did not show */ stopWithoutProgramFrame: ( threadId: number | undefined, frameCount: number, - top: { name: string; file: string; line: number } + top: { name: string; file: string; line: number }, + opts?: { framesUnseen?: boolean } ) => `No program frame is active${typeof threadId === 'number' ? ` on thread ${threadId}` : ''}: ` + `${frameCount === 1 ? 'the only frame is' : `all ${frameCount} frames are`} internal/runtime ` + `(top: '${top.name}' ${top.file}:${top.line}). The stop landed outside your code: ` + - `continue_execution and trigger activity, or set a breakpoint; ` + - `get_stack_trace with includeInternals: true lists the frames.`, + `continue_execution and trigger activity, or set a breakpoint` + + (opts?.framesUnseen ? '; get_stack_trace with includeInternals: true lists the frames.' : '.'), /** * An explicit breakOnExceptions mode the adapter cannot honour (issue #1032): @@ -470,6 +545,30 @@ export const ErrorMessages = { stackTraceNotPaused: (state: string, why?: string) => `Session is not paused (state: ${state}); stack traces are only available while paused${why ? `; ${why}` : ''}.`, + /** + * The `message` an inspection tool puts beside the bare + * `Session is terminated: ` error (issue #1141), worded by what the + * session is, because the remedy differs: a launched program is run again + * with restart_debugging; an attach session has no launch to replay and + * restart_debugging refuses it, so it is attached again; a child session + * was started by its parent and can do neither. + * Used in: src/server/handlers/shared.ts (endedSessionExtras) + * @param kind - What the ended session is + * @param parentSessionId - The parent's id, for a child session + */ + endedSessionNothingToInspect: (kind: 'launch' | 'attach' | 'child', parentSessionId?: string) => { + switch (kind) { + case 'attach': + return 'The debugger is no longer attached to the target (it ended, or was detached), so no frames or variables exist. ' + + 'Use attach_to_process to attach again.'; + case 'child': + return 'The process this child session debugged has ended or was released, so no frames or variables exist. ' + + `A child session cannot be restarted on its own${parentSessionId ? `: its parent is ${parentSessionId}` : ''}.`; + case 'launch': + return 'The program has terminated, so no frames or variables exist. Use restart_debugging to run it again.'; + } + }, + /** * get_local_variables with no frame and the session not paused, with the * why when the launch runs with the debugger off (issue #749). diff --git a/tests/adapters/javascript/integration/javascript-no-program-frame.test.ts b/tests/adapters/javascript/integration/javascript-no-program-frame.test.ts new file mode 100644 index 000000000..fc6039ccc --- /dev/null +++ b/tests/adapters/javascript/integration/javascript-no-program-frame.test.ts @@ -0,0 +1,131 @@ +/** + * A stop where no frame is program code, read through every inspection tool + * of a real server and a real js-debug (issues #1135, #1141). + * + * `examples/javascript/pause_test.js` is a bare `setInterval`: between ticks + * the only JavaScript that runs is Node's own timer loop, so a pause lands on + * `process.processTimers` — one runtime frame, no program frame. That is the + * stop the 2026-11-11 sweep reported: get_stack_trace said `noProgramFrame`, + * get_local_variables answered the timer loop's `now` as a plain local, and + * get_stack_trace told a caller who had passed includeInternals: true to pass + * it. + * + * Where a pause lands is the runtime's call, so the suite states the rule + * rather than the landing: the three tools agree on whether the stack has a + * program frame, and when it has none each says so. The landing itself is + * asserted too, loudly, because a suite that only ever took the other branch + * would test nothing. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { startStdioClient, type StdioClientHandle, type ToolAnswer } from '../../../test-utils/helpers/mcp-stdio-client.js'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const examples = path.resolve(here, '../../../../examples/javascript'); +/** A bare setInterval: it never ends and runs no program code between ticks. */ +const IDLE_PROGRAM = path.join(examples, 'pause_test.js'); +/** Runs to completion in a few milliseconds. */ +const SHORT_PROGRAM = path.join(examples, 'simple_test.js'); + +interface Frame { name: string; file: string; line: number } +type StackAnswer = ToolAnswer & { stackFrames: Frame[]; count: number; includeInternals: boolean; noProgramFrame?: boolean; note?: string }; +type LocalsAnswer = ToolAnswer & { variables: Array<{ name: string; value: string }>; count: number; frame?: Frame; noProgramFrame?: boolean; note?: string }; +type EvaluateAnswer = ToolAnswer & { result?: string; frame?: Frame; noProgramFrame?: boolean; anchorNote?: string }; +type StopAnswer = ToolAnswer & { pending?: boolean; note?: string; location?: { file: string; line: number } }; + +describe('Inspection on a stop with no program frame (issues #1135, #1141) @requires-node', () => { + let server: StdioClientHandle | undefined; + + afterEach(async () => { + await server?.close(); + server = undefined; + }); + + it('get_stack_trace, get_local_variables and evaluate_expression agree that the frame is not program code', async () => { + server = await startStdioClient({ name: 'no-program-frame' }); + await server.createSession('javascript', 'no-program-frame'); + const launch = await server.call('start_debugging', { scriptPath: IDLE_PROGRAM }); + expect(launch, JSON.stringify(launch)).toMatchObject({ success: true, state: 'running' }); + + let pause = await server.call('pause_execution', {}); + if (pause.pending) { + // The pause is in force; the next timer tick lands it. + pause = await server.call('wait_for_stop', { timeout: 20_000 }); + } + expect(pause, JSON.stringify(pause)).toMatchObject({ success: true, state: 'paused' }); + + const stack = await server.call('get_stack_trace', { includeInternals: true }); + const locals = await server.call('get_local_variables', {}); + const evaluated = await server.call('evaluate_expression', { expression: '1 + 1' }); + const context = JSON.stringify({ stack, locals, evaluated }); + + // The rule, wherever the pause landed: one verdict, on all three. + expect(locals.noProgramFrame === true, context).toBe(stack.noProgramFrame === true); + expect(evaluated.noProgramFrame === true, context).toBe(stack.noProgramFrame === true); + + // The landing this fixture exists for. If a Node release ever lands the + // pause in the callback instead, this fails by name and the fixture needs + // another idle program — the assertions below must not go quietly unused. + expect(stack.noProgramFrame, `the pause on a bare setInterval landed in program code: ${context}`).toBe(true); + + // get_stack_trace was asked for every frame and shows every frame: it + // never tells the caller to ask for them (issue #1141). + expect(stack.includeInternals).toBe(true); + expect(stack.note, context).toContain('No program frame is active'); + expect(stack.note, context).not.toContain('includeInternals'); + + // get_local_variables keeps what the adapter reported and says whose it + // is (issue #1135). + expect(locals.note, context).toContain('No program frame is active'); + if (locals.count > 0) { + expect(locals.note, context).toContain(`The variables shown belong to '${locals.frame?.name}', not to your program.`); + } + // With one frame in all, there is no fuller stack to point at; with more, + // the pointer is there. + if (stack.count === 1) { + expect(locals.note, context).not.toContain('includeInternals'); + } else { + expect(locals.note, context).toContain('get_stack_trace with includeInternals: true lists the frames.'); + } + + // evaluate_expression without a frameId ran in that frame, and says so. + expect(evaluated, context).toMatchObject({ success: true, result: '2' }); + expect(evaluated.anchorNote, context).toContain('No program frame is active'); + expect(evaluated.anchorNote, context).toContain('The expression was evaluated in that frame.'); + + // An explicit frameId is the caller's choice: nothing is added. + const explicit = await server.call('evaluate_expression', { + expression: '1 + 1', + frameId: (stack.stackFrames[0] as Frame & { id: number }).id + }); + expect(explicit, JSON.stringify(explicit)).toMatchObject({ success: true, result: '2' }); + expect(explicit).not.toHaveProperty('noProgramFrame'); + expect(explicit).not.toHaveProperty('anchorNote'); + }, 90_000); + + it('explains an ended session from every inspection tool, with the error text unchanged', async () => { + server = await startStdioClient({ name: 'ended-session' }); + const sessionId = await server.createSession('javascript', 'ended-session'); + await server.call('start_debugging', { scriptPath: SHORT_PROGRAM }); + const ended = await server.call('wait_for_stop', { timeout: 30_000 }); + expect(ended, JSON.stringify(ended)).toMatchObject({ success: true, state: 'stopped' }); + + const sentence = 'The program has terminated, so no frames or variables exist. Use restart_debugging to run it again.'; + const calls: Array<[string, Record]> = [ + ['get_stack_trace', {}], + ['get_local_variables', {}], + ['get_scopes', { frameId: 1 }], + ['get_variables', { scope: 1 }], + ['evaluate_expression', { expression: '1 + 1' }] + ]; + for (const [tool, args] of calls) { + const answer = await server.call(tool, args); + expect(answer, `${tool}: ${JSON.stringify(answer)}`).toMatchObject({ + success: false, + error: `Session is terminated: ${sessionId}`, + message: sentence + }); + } + }, 90_000); +}); diff --git a/tests/adapters/javascript/integration/javascript-snapshot-diff.test.ts b/tests/adapters/javascript/integration/javascript-snapshot-diff.test.ts new file mode 100644 index 000000000..68093f82f --- /dev/null +++ b/tests/adapters/javascript/integration/javascript-snapshot-diff.test.ts @@ -0,0 +1,88 @@ +/** + * The stop snapshot's diff against a real js-debug (issue #1140): a variable + * that leaves scope is reported as gone, and no entry ever carries an + * invented value. + * + * The fixture's loop body has a block-scoped `doubled`; stepping from inside + * the block to the statement after the loop takes it (and the loop's `i`) + * out of the frame's locals while the function, and so the diff's frame + * identity, stays the same. How many steps V8 takes through a `for` header + * is its own business, so the suite steps until it is past the loop and + * reads every diff on the way. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { startStdioClient, type StdioClientHandle, type ToolAnswer } from '../../../test-utils/helpers/mcp-stdio-client.js'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const FIXTURE = path.resolve(here, '../../../fixtures/debug-scripts/js-block-scope.js'); +/** `total += doubled;` — inside the loop's block. */ +const LINE_IN_BLOCK = 10; +/** `const done = total > 0;` — the first statement after the loop. */ +const LINE_AFTER_LOOP = 12; + +interface ChangedVariable { before?: string; after?: string; new?: true; gone?: true } +type StopAnswer = ToolAnswer & { + location?: { file: string; line: number }; + snapshot?: { locals?: Record; changedVariables?: Record }; +}; + +describe('The snapshot diff against a real js-debug (issue #1140) @requires-node', () => { + let server: StdioClientHandle | undefined; + + afterEach(async () => { + await server?.close(); + server = undefined; + }); + + it('reports a block variable as gone once the block is left, and never invents a value', async () => { + server = await startStdioClient({ name: 'snapshot-diff-js' }); + await server.createSession('javascript', 'snapshot-diff-js'); + await server.call('set_breakpoint', { file: FIXTURE, line: LINE_IN_BLOCK }); + let launch = await server.call('start_debugging', { scriptPath: FIXTURE }); + expect(launch.success, JSON.stringify(launch)).toBe(true); + // The launch answers paused when the breakpoint is reached within its + // hold (issue #823); a slower start answers pending, and wait_for_stop + // lands the same stop with the same first snapshot. + if (launch.state !== 'paused') { + launch = await server.call('wait_for_stop', { timeout: 15000 }); + } + expect(launch, JSON.stringify(launch)).toMatchObject({ success: true, state: 'paused' }); + // The first stop shows the frame's locals in full: the block's and the function's. + expect(launch.snapshot?.locals, JSON.stringify(launch)).toMatchObject({ doubled: '2', i: '0', total: '0' }); + + const diffs: Array> = []; + let line = launch.location?.line; + for (let steps = 0; steps < 8 && line !== LINE_AFTER_LOOP; steps++) { + const step = await server.call('step_over', {}); + expect(step, JSON.stringify(step)).toMatchObject({ success: true, state: 'paused' }); + line = step.location?.line; + // Same function, same depth: every one of these stops answers a diff. + expect(step.snapshot?.changedVariables, JSON.stringify(step)).toBeDefined(); + diffs.push(step.snapshot!.changedVariables!); + } + const seen = JSON.stringify(diffs); + expect(line, `never reached the statement after the loop: ${seen}`).toBe(LINE_AFTER_LOOP); + + // The block's variable left scope on the way, with the value it last had. + const left = diffs.find((diff) => diff.doubled?.gone === true); + expect(left?.doubled, seen).toEqual({ before: '2', gone: true }); + // The loop counter went too, by the time the loop was over. + expect(diffs.some((diff) => diff.i?.gone === true), seen).toBe(true); + // The function's own variable is never reported gone. + expect(diffs.some((diff) => diff.total?.gone === true), seen).toBe(false); + + // The shape, on every entry of every diff: a side that does not exist is + // absent, a flag says why, and no placeholder stands in for a value. + for (const diff of diffs) { + for (const [name, entry] of Object.entries(diff)) { + const where = `${name} in ${seen}`; + if (entry.new) expect(entry, where).toEqual({ after: entry.after, new: true }); + if (entry.gone) expect(entry, where).toEqual({ before: entry.before, gone: true }); + expect(entry.before, where).not.toBe(''); + expect(typeof (entry.after ?? entry.before), where).toBe('string'); + } + } + }, 90_000); +}); diff --git a/tests/adapters/python/integration/python-attach-wrong-port.test.ts b/tests/adapters/python/integration/python-attach-wrong-port.test.ts new file mode 100644 index 000000000..b602c7df2 --- /dev/null +++ b/tests/adapters/python/integration/python-attach-wrong-port.test.ts @@ -0,0 +1,182 @@ +/** + * A Python attach to a port that is not a debugpy listener, against a real + * server (issue #1139). + * + * A Python attach starts nothing: the server connects straight to the host + * and port it was given. The issue's example is the application's own HTTP + * port given instead of the debugger's; an HTTP server answers a DAP frame by + * hanging up, and the attach used to answer + * "Critical initialization error: DAP client disconnected + * [Executable: python | adapter PID=none exitCode=n/a]" — an executable and + * an adapter process that never existed, and no word about the port. + * + * The listener here hangs up on the first bytes it receives, which is that + * path and answers within a second. A second listener resets the connection + * instead (what a forwarder with nothing behind it does): the reset reaches + * the worker as a socket error before the request fails, and that first + * report used to be the bare "DAP client error: read ECONNRESET". The other + * path — a listener that stays + * silent — waits out the 30 s initialization deadline before it answers, so + * its wording is covered at the unit level (error-messages, proxy-manager) + * and was checked by hand against this server. So is a port where nothing + * listens at all (the worker's own tests): it spends the 12.5 s connect + * window before it answers. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import net from 'node:net'; +import { startStdioClient, type StdioClientHandle, type ToolAnswer } from '../../../test-utils/helpers/mcp-stdio-client.js'; + +type AttachAnswer = ToolAnswer & { data?: { initProgress?: unknown; proxyLogPath?: string } }; + +/** + * A TCP listener that drops the client as soon as it says anything. `hangUp` + * closes the socket, which is what an HTTP server does with a DAP frame; + * `reset` aborts it (RST), which is what a forwarder with nothing behind it + * does. + */ +async function droppingListener(how: 'hangUp' | 'reset'): Promise<{ port: number; received: () => number; close: () => Promise }> { + let bytes = 0; + const sockets = new Set(); + const listener = net.createServer((socket) => { + sockets.add(socket); + socket.on('data', (chunk) => { + bytes += chunk.length; + if (how === 'reset') { + socket.resetAndDestroy(); + } else { + socket.destroy(); + } + }); + socket.on('error', () => undefined); + socket.on('close', () => sockets.delete(socket)); + }); + await new Promise((resolve) => listener.listen(0, '127.0.0.1', resolve)); + return { + port: (listener.address() as net.AddressInfo).port, + received: () => bytes, + close: async () => { + for (const socket of sockets) socket.destroy(); + await new Promise((resolve) => listener.close(() => resolve())); + } + }; +} + +/** + * A listener that speaks just enough DAP to answer `initialize`, and hangs up + * on the request after it: a debug server (or a forwarder to one) that goes + * away mid-handshake. + */ +async function answersInitializeThenHangsUp(): Promise<{ port: number; received: () => number; close: () => Promise }> { + let requests = 0; + const sockets = new Set(); + const listener = net.createServer((socket) => { + sockets.add(socket); + let buffer = Buffer.alloc(0); + socket.on('data', (chunk: Buffer) => { + buffer = Buffer.concat([buffer, chunk]); + for (;;) { + const headerEnd = buffer.indexOf('\r\n\r\n'); + const length = headerEnd === -1 ? null : /Content-Length: (\d+)/i.exec(buffer.subarray(0, headerEnd).toString('utf8')); + if (headerEnd === -1 || !length || buffer.length < headerEnd + 4 + Number(length[1])) return; + const request = JSON.parse(buffer.subarray(headerEnd + 4, headerEnd + 4 + Number(length[1])).toString('utf8')); + buffer = buffer.subarray(headerEnd + 4 + Number(length[1])); + requests++; + if (request.command !== 'initialize') { + socket.destroy(); + return; + } + const body = Buffer.from(JSON.stringify({ + seq: 1, type: 'response', request_seq: request.seq, command: 'initialize', success: true, + body: { supportsConfigurationDoneRequest: true } + }), 'utf8'); + socket.write(`Content-Length: ${body.length}\r\n\r\n`); + socket.write(body); + } + }); + socket.on('error', () => undefined); + socket.on('close', () => sockets.delete(socket)); + }); + await new Promise((resolve) => listener.listen(0, '127.0.0.1', resolve)); + return { + port: (listener.address() as net.AddressInfo).port, + received: () => requests, + close: async () => { + for (const socket of sockets) socket.destroy(); + await new Promise((resolve) => listener.close(() => resolve())); + } + }; +} + +describe('A Python attach to a port that is not a debug server (issue #1139)', () => { + let server: StdioClientHandle | undefined; + let target: Awaited> | undefined; + + afterEach(async () => { + await server?.close(); + server = undefined; + await target?.close(); + target = undefined; + }); + + it('names the endpoint and says nothing was started, instead of describing an adapter process', async () => { + target = await droppingListener('hangUp'); + server = await startStdioClient({ name: 'attach-wrong-port' }); + await server.createSession('python', 'attach-wrong-port'); + + const answer = await server.call('attach_to_process', { port: target.port }); + + expect(answer.success, JSON.stringify(answer)).toBe(false); + // The endpoint did receive the DAP frame, and hung up on it. + expect(target.received()).toBeGreaterThan(0); + expect(answer.message).toBe( + `Failed to attach: Critical initialization error: 127.0.0.1:${target.port} never answered the DAP "initialize" request ` + + '(DAP client disconnected). Nothing was started for this attach: the server connected to what is already ' + + `listening on that port, so check that it is a debugpy listener (python -m debugpy --listen 127.0.0.1:${target.port} …) ` + + 'and not another service.' + ); + // The pointers a failed attach always carries are still there. + expect(answer.data?.proxyLogPath, JSON.stringify(answer)).toEqual(expect.any(String)); + }, 60_000); + + it('says the same of an endpoint that resets the connection', async () => { + target = await droppingListener('reset'); + server = await startStdioClient({ name: 'attach-reset-port' }); + await server.createSession('python', 'attach-reset-port'); + + const answer = await server.call('attach_to_process', { port: target.port }); + + expect(answer.success, JSON.stringify(answer)).toBe(false); + expect(target.received()).toBeGreaterThan(0); + // The reason in parentheses is the socket's own (ECONNRESET, or a plain + // disconnect where the platform reports the reset as a close), so it is + // not pinned; everything the caller acts on is. + expect(answer.message, JSON.stringify(answer)).toMatch( + new RegExp( + `^Failed to attach: Critical initialization error: 127\\.0\\.0\\.1:${target.port} never answered the DAP "initialize" request \\(.+\\)\\. ` + + 'Nothing was started for this attach: the server connected to what is already listening on that port, ' + + 'so check that it is a debugpy listener .+ and not another service\\.$' + ) + ); + expect(answer.message).not.toContain('DAP client error'); + }, 60_000); + + it('names the endpoint when a debug server answers initialize and then hangs up, without doubting what it is', async () => { + // The close reaches the worker before the pending attach request fails, + // and the worker's connection-closed status used to reach the parent + // first: "Proxy exited during initialization. Code: null, Signal: + // undefined", followed by lines of worker stderr. + target = await answersInitializeThenHangsUp(); + server = await startStdioClient({ name: 'attach-dropped-port' }); + await server.createSession('python', 'attach-dropped-port'); + + const answer = await server.call('attach_to_process', { port: target.port }); + + expect(answer.success, JSON.stringify(answer)).toBe(false); + // initialize, then the attach it hung up on. + expect(target.received()).toBe(2); + expect(answer.message, JSON.stringify(answer)).toBe( + 'Failed to attach: Critical initialization error: DAP client disconnected ' + + `[direct connect to 127.0.0.1:${target.port}; no adapter process was started]` + ); + }, 60_000); +}); diff --git a/tests/adapters/python/integration/python-snapshot-diff.test.ts b/tests/adapters/python/integration/python-snapshot-diff.test.ts new file mode 100644 index 000000000..a93f8f618 --- /dev/null +++ b/tests/adapters/python/integration/python-snapshot-diff.test.ts @@ -0,0 +1,75 @@ +/** + * The stop snapshot's diff against a real debugpy (issue #1140): a variable + * that comes into scope is reported as new, with no invented "before". + * + * Before the fix a step past `b = 2` answered + * `"b": { "before": "", "after": "2" }` — a string that reads as a + * value, in JavaScript's vocabulary, for a Python local that simply did not + * exist yet. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { startStdioClient, type StdioClientHandle, type ToolAnswer } from '../../../test-utils/helpers/mcp-stdio-client.js'; +import { ensurePythonOnPath } from './env-utils.js'; + +const here = path.dirname(fileURLToPath(import.meta.url)); +const PROGRAM = path.resolve(here, '../../../../examples/python/simple_test.py'); +/** `b = 2` — `a` exists, `b` does not yet. */ +const LINE_BEFORE_B = 9; + +interface ChangedVariable { before?: string; after?: string; new?: true; gone?: true } +type StopAnswer = ToolAnswer & { + location?: { file: string; line: number }; + snapshot?: { locals?: Record; changedVariables?: Record }; +}; + +/** The PATH a Windows runner needs to find a Python with debugpy; nothing else of the runner's environment. */ +function pythonPath(): Record { + const env: Record = {}; + for (const [key, value] of Object.entries(process.env)) { + if (value !== undefined) env[key] = value; + } + ensurePythonOnPath(env); + return Object.fromEntries(Object.entries(env).filter(([key]) => key.toUpperCase() === 'PATH')); +} + +describe('The snapshot diff against a real debugpy (issue #1140) @requires-python', () => { + let server: StdioClientHandle | undefined; + + afterEach(async () => { + await server?.close(); + server = undefined; + }); + + it('reports a variable that came into scope as new, then a changed value as a change', async () => { + server = await startStdioClient({ name: 'snapshot-diff-py', env: pythonPath() }); + await server.createSession('python', 'snapshot-diff-py'); + await server.call('set_breakpoint', { file: PROGRAM, line: LINE_BEFORE_B }); + let launch = await server.call('start_debugging', { scriptPath: PROGRAM }); + expect(launch.success, JSON.stringify(launch)).toBe(true); + // The launch answers paused when the breakpoint is reached within its + // hold (issue #823); a slower start answers pending, and wait_for_stop + // lands the same stop with the same first snapshot. + if (launch.state !== 'paused') { + launch = await server.call('wait_for_stop', { timeout: 15000 }); + } + expect(launch, JSON.stringify(launch)).toMatchObject({ success: true, state: 'paused' }); + expect(launch.snapshot?.locals, JSON.stringify(launch)).toEqual({ a: '1' }); + + // b = 2 + const declared = await server.call('step_over', {}); + expect(declared.snapshot?.changedVariables, JSON.stringify(declared)).toEqual({ b: { after: '2', new: true } }); + + // print(...) — nothing the frame can see changes. + const printed = await server.call('step_over', {}); + expect(printed.snapshot?.changedVariables, JSON.stringify(printed)).toEqual({}); + + // a, b = b, a + const swapped = await server.call('step_over', {}); + expect(swapped.snapshot?.changedVariables, JSON.stringify(swapped)).toEqual({ + a: { before: '1', after: '2' }, + b: { before: '2', after: '1' } + }); + }, 90_000); +}); diff --git a/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts b/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts index 9d670b666..6af7d0e05 100644 --- a/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts +++ b/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts @@ -41,11 +41,14 @@ describe('RubyAdapterPolicy.getAdapterSpawnConfig', () => { launchConfig: { request: 'attach', host: '10.0.0.5', port: 12345 } }); + // With what should be listening there, for the answer of an attach that + // reaches something else (issue #1139). expect(config).toEqual({ mode: 'connect', host: '10.0.0.5', port: 12345, - logDir: '/tmp/logs' + logDir: '/tmp/logs', + listener: 'an rdbg debug server (rdbg --open --port 12345 …)' }); }); diff --git a/tests/core/unit/server/handlers/inspection-tools.test.ts b/tests/core/unit/server/handlers/inspection-tools.test.ts index 10654ef7c..4b1b3f059 100644 --- a/tests/core/unit/server/handlers/inspection-tools.test.ts +++ b/tests/core/unit/server/handlers/inspection-tools.test.ts @@ -8,12 +8,14 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ProtocolError, ProtocolErrorCode } from '@modelcontextprotocol/server'; import { DotnetAdapterPolicy, SessionLifecycleState } from '@debugmcp/shared'; import { + getScopesTool, getStackTraceTool, + getVariablesTool, handleEvaluateExpression, handleGetSourceContext, handleGetLocalVariables } from '../../../../../src/server/handlers/inspection-tools.js'; -import { SessionTerminatedError } from '../../../../../src/errors/debug-errors.js'; +import { SessionNotFoundError, SessionTerminatedError } from '../../../../../src/errors/debug-errors.js'; import { ErrorMessages } from '../../../../../src/utils/error-messages.js'; import { createMockToolContext } from '../server-test-helpers.js'; @@ -305,6 +307,97 @@ describe('inspection tool handlers', () => { expect(payload.frame.name).toBe('main'); expect(payload.scopeName).toBe('Locals'); expect(payload.message).toBeUndefined(); + expect(payload).not.toHaveProperty('noProgramFrame'); + expect(payload).not.toHaveProperty('note'); + }); + + describe('a stack with no program frame (issue #1135)', () => { + const TIMERS = { name: 'process.processTimers', file: '/internal/timers', line: 527 }; + const SENTENCE = + "No program frame is active on thread 0: the only frame is internal/runtime (top: 'process.processTimers' /internal/timers:527). " + + 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint.'; + + async function localsOf(result: Record) { + ctx.sessionManager.getLocalVariables.mockResolvedValue(result); + return JSON.parse((await handleGetLocalVariables(ctx, { sessionId: 'test-session' })).content[0].text); + } + + it('keeps the variables, flags the answer and says whose they are', async () => { + const payload = await localsOf({ + variables: [{ name: 'now', value: '4210', type: 'number' }], + frame: TIMERS, + scopeName: 'Local: processTimers', + noProgramFrame: { threadId: 0, frameCount: 1, top: TIMERS } + }); + + expect(payload.success).toBe(true); + expect(payload.variables).toEqual([{ name: 'now', value: '4210', type: 'number' }]); + expect(payload.noProgramFrame).toBe(true); + expect(payload.note).toBe(`${SENTENCE} The variables shown belong to 'process.processTimers', not to your program.`); + }); + + it('points at get_stack_trace when the thread has more frames than this answer names', async () => { + const crt = { name: '__tmainCRTStartup', file: '@__tmainCRTStartup', line: 51 }; + const payload = await localsOf({ + variables: [{ name: '[return value]', value: '0', type: 'int' }], + frame: crt, + scopeName: 'Local', + noProgramFrame: { threadId: 12952, frameCount: 4, top: crt } + }); + + expect(payload.note).toBe( + "No program frame is active on thread 12952: all 4 frames are internal/runtime (top: '__tmainCRTStartup' @__tmainCRTStartup:51). " + + 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; ' + + 'get_stack_trace with includeInternals: true lists the frames. ' + + "The variables shown belong to '__tmainCRTStartup', not to your program." + ); + }); + + it('names the frame the variables came from when the read walked down from the top', async () => { + const lower = { name: 'java.lang.Thread.run', file: 'java/lang/Thread.java', line: 1583 }; + const top = { name: 'jdk.internal.misc.Unsafe.park', file: 'jdk/internal/misc/Unsafe.java', line: 0 }; + const payload = await localsOf({ + variables: [{ name: 'target', value: 'Worker@1f', type: 'Runnable' }], + frame: lower, + scopeName: 'Local', + anchorNote: "Top frame 'jdk.internal.misc.Unsafe.park' has no local variables (runtime/stdlib frame); showing frame #2 'java.lang.Thread.run' instead", + noProgramFrame: { threadId: 3, frameCount: 5, top } + }); + + // The walk-down note first, then the verdict; the clause names the frame that answered, not the top. + expect(payload.note).toBe( + "Top frame 'jdk.internal.misc.Unsafe.park' has no local variables (runtime/stdlib frame); showing frame #2 'java.lang.Thread.run' instead. " + + "No program frame is active on thread 3: all 5 frames are internal/runtime (top: 'jdk.internal.misc.Unsafe.park' jdk/internal/misc/Unsafe.java:0). " + + 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; ' + + 'get_stack_trace with includeInternals: true lists the frames. ' + + "The variables shown belong to 'java.lang.Thread.run', not to your program." + ); + }); + + it('claims no variables when the list is empty', async () => { + const payload = await localsOf({ + variables: [], + frame: TIMERS, + scopeName: 'Local: processTimers', + noProgramFrame: { threadId: 0, frameCount: 1, top: TIMERS } + }); + + expect(payload.noProgramFrame).toBe(true); + expect(payload.note).toBe(SENTENCE); + expect(payload.message).toBe('The Local: processTimers scope is empty.'); + }); + + it('keeps a resolver note that already ends its sentence as it is', async () => { + const payload = await localsOf({ + variables: [], + frame: TIMERS, + scopeName: null, + anchorNote: 'The stop was reported on thread 4; inspection is anchored on thread 0.', + noProgramFrame: { threadId: 0, frameCount: 1, top: TIMERS } + }); + + expect(payload.note).toBe(`The stop was reported on thread 4; inspection is anchored on thread 0. ${SENTENCE}`); + }); }); it('shows "not paused" message when no frame available', async () => { @@ -523,5 +616,98 @@ describe('inspection tool handlers', () => { expect(stored.text).toContain(SECRET); }); }); + + describe('an ended session is explained by every inspection tool, in terms of what the session is (issue #1141)', () => { + const ENDED = 'Session is terminated: test-session'; + const LAUNCH_SENTENCE = + 'The program has terminated, so no frames or variables exist. Use restart_debugging to run it again.'; + + /** Each inspection tool, called on a session whose gate throws the terminated error. */ + const TOOLS: Array<[string, () => Promise<{ content: [{ text: string }] }>]> = [ + ['get_stack_trace', () => getStackTraceTool(ctx, { sessionId: 'test-session' }, 'get_stack_trace')], + ['get_scopes', () => getScopesTool(ctx, { sessionId: 'test-session', frameId: 1 }, 'get_scopes')], + ['get_variables', () => getVariablesTool(ctx, { sessionId: 'test-session', scope: 7 }, 'get_variables')], + ['evaluate_expression', () => handleEvaluateExpression(ctx, { sessionId: 'test-session', expression: 'x' })], + ['get_local_variables', () => handleGetLocalVariables(ctx, { sessionId: 'test-session' })] + ]; + + function endedSession(session: Record): void { + ctx.sessionManager.getSession.mockReturnValue({ + id: 'test-session', + sessionLifecycle: SessionLifecycleState.TERMINATED, + state: 'stopped', + ...session + }); + const terminated = () => { throw new SessionTerminatedError('test-session'); }; + ctx.validateSession = vi.fn().mockImplementation(terminated); + ctx.getStackTrace = vi.fn().mockImplementation(async () => terminated()); + ctx.getScopes = vi.fn().mockImplementation(async () => terminated()); + ctx.getVariablesDetailed = vi.fn().mockImplementation(async () => terminated()); + ctx.getLocalVariables = vi.fn().mockImplementation(async () => terminated()); + } + + it.each(TOOLS)('%s: a launched program that ended points at restart_debugging', async (_name, call) => { + endedSession({ lastLaunch: { scriptPath: '/app/main.py' } }); + + const payload = JSON.parse((await call()).content[0].text); + + expect(payload.success).toBe(false); + // The error text is unchanged: callers and tests match on it. + expect(payload.error).toBe(ENDED); + expect(payload.message).toBe(LAUNCH_SENTENCE); + }); + + it.each(TOOLS)('%s: an attach session points at attach_to_process, never at a restart it would be refused', async (_name, call) => { + endedSession({ attachMode: true }); + + const payload = JSON.parse((await call()).content[0].text); + + expect(payload.error).toBe(ENDED); + expect(payload.message).toBe( + 'The debugger is no longer attached to the target (it ended, or was detached), so no frames or variables exist. ' + + 'Use attach_to_process to attach again.' + ); + expect(payload.message).not.toContain('restart_debugging'); + }); + + it.each(TOOLS)('%s: a child session names its parent', async (_name, call) => { + // A child of an attach root: the child rule wins, whatever else the row carries. + endedSession({ parentSessionId: 'parent-1', attachMode: true }); + + const payload = JSON.parse((await call()).content[0].text); + + expect(payload.error).toBe(ENDED); + expect(payload.message).toBe( + 'The process this child session debugged has ended or was released, so no frames or variables exist. ' + + 'A child session cannot be restarted on its own: its parent is parent-1.' + ); + }); + + it('keeps the launch wording when the session row cannot be read', async () => { + endedSession({}); + ctx.sessionManager.getSession.mockReturnValue(undefined); + + const payload = JSON.parse((await getStackTraceTool(ctx, { sessionId: 'test-session' }, 'get_stack_trace')).content[0].text); + + expect(payload.message).toBe(LAUNCH_SENTENCE); + }); + + it('get_stack_trace keeps its failure diagnostics beside the sentence', async () => { + endedSession({ failureDiagnostics: { proxyLogPath: '/logs/proxy.log' } }); + + const payload = JSON.parse((await getStackTraceTool(ctx, { sessionId: 'test-session' }, 'get_stack_trace')).content[0].text); + + expect(payload.diagnostics).toEqual({ proxyLogPath: '/logs/proxy.log' }); + expect(payload.message).toBe(LAUNCH_SENTENCE); + }); + + it('adds nothing for a session that is merely unknown', async () => { + ctx.getStackTrace = vi.fn().mockRejectedValue(new SessionNotFoundError('nope')); + + const payload = JSON.parse((await getStackTraceTool(ctx, { sessionId: 'nope' }, 'get_stack_trace')).content[0].text); + + expect(payload).toEqual({ success: false, error: 'Session not found: nope' }); + }); + }); }); diff --git a/tests/core/unit/server/server-expected-content.test.ts b/tests/core/unit/server/server-expected-content.test.ts index 531ad9d5b..aa765ccd1 100644 --- a/tests/core/unit/server/server-expected-content.test.ts +++ b/tests/core/unit/server/server-expected-content.test.ts @@ -200,6 +200,40 @@ describe('set_breakpoint expectedContent (#271)', () => { expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); }); + describe('the statement hint on a mismatch can be followed (issue #1142)', () => { + async function refusalOf(args: Record): Promise { + try { + await callSetBreakpoint(args); + } catch (error) { + return (error as Error).message; + } + throw new Error('set_breakpoint did not refuse'); + } + + it('an off-by-one line names where the text is, and that statement then binds there', async () => { + const refusal = await refusalOf({ line: 4, expectedContent: 'total = sum(prices)' }); + + expect(refusal).toMatch(/Or address by content: set_breakpoint \{statement: "total = sum\(prices\)"\} — that text is on line 3\.$/); + + // Following the hint: the same text, as a statement. + const followed = JSON.parse((await callSetBreakpoint({ statement: 'total = sum(prices)' })).content[0].text); + expect(followed.success).toBe(true); + expect(mockSessionManager.setBreakpoint).toHaveBeenCalledWith('test-session', expect.objectContaining({ line: 3 })); + }); + + it('text that is nowhere in the file earns no statement hint, where following one used to fail', async () => { + const refusal = await refusalOf({ line: 3, expectedContent: 'THIS TEXT IS WRONG' }); + + expect(refusal).toContain('Pick the correct line from the context above.'); + expect(refusal).not.toContain('statement: "THIS TEXT IS WRONG"'); + expect(refusal).toMatch(/ That expectedContent matches no line of the file\.$/); + + // What the old hint led to. + await expect(callSetBreakpoint({ statement: 'THIS TEXT IS WRONG' })).rejects.toThrow(/not found/); + expect(mockSessionManager.setBreakpoint).not.toHaveBeenCalled(); + }); + }); + it('rejects expectedContent for attach sessions (no host-readable file)', async () => { mockSessionManager.getSession.mockReturnValue({ id: 'test-session', diff --git a/tests/core/unit/server/server-inspection-tools.test.ts b/tests/core/unit/server/server-inspection-tools.test.ts index fc306e912..c8d467773 100644 --- a/tests/core/unit/server/server-inspection-tools.test.ts +++ b/tests/core/unit/server/server-inspection-tools.test.ts @@ -477,6 +477,8 @@ describe('Server Inspection Tools Tests', () => { const content = JSON.parse(result.content[0].text); expect(content.noProgramFrame).toBe(true); expect(content.note).toContain("No program frame is active: the only frame is internal/runtime (top: '' :1)"); + // This answer IS the stack: pointing at get_stack_trace from it goes round in a circle (issue #1141). + expect(content.note).not.toContain('lists the frames'); // Absent, not false, on an ordinary stack. mockSessionManager.getStackTraceDetailed.mockResolvedValue({ @@ -490,6 +492,29 @@ describe('Server Inspection Tools Tests', () => { expect(plain).not.toHaveProperty('noProgramFrame'); }); + it('never tells a caller who passed includeInternals: true to pass it (issue #1141)', async () => { + mockSessionManager.getSession.mockReturnValue({ proxyManager: { getCurrentThreadId: vi.fn().mockReturnValue(1) } }); + const frames = [ + { id: 1, name: '__tmainCRTStartup', file: '@__tmainCRTStartup', line: 51 }, + { id: 2, name: 'mainCRTStartup', file: '@mainCRTStartup', line: 9 } + ]; + // With the flag the whole stack is shown: nothing hidden, no program frame. + mockSessionManager.getStackTraceDetailed.mockResolvedValue({ + frames, threadId: 7, totalFrameCount: 2, hiddenFrameCount: 0, allFramesInternal: false, noProgramFrame: true + }); + + const content = JSON.parse((await callToolHandler({ + method: 'tools/call', + params: { name: 'get_stack_trace', arguments: { sessionId: 'test-session', includeInternals: true } } + })).content[0].text); + + expect(content.includeInternals).toBe(true); + expect(content.stackFrames).toHaveLength(2); + expect(content.note).toContain('No program frame is active on thread 7: all 2 frames are internal/runtime'); + expect(content.note).toContain('continue_execution and trigger activity, or set a breakpoint.'); + expect(content.note).not.toContain('includeInternals'); + }); + it('should handle missing session', async () => { mockSessionManager.getSession.mockReturnValue(null); diff --git a/tests/core/unit/server/server-redefine-and-attach.test.ts b/tests/core/unit/server/server-redefine-and-attach.test.ts index e8b25e595..ef6d2f9b0 100644 --- a/tests/core/unit/server/server-redefine-and-attach.test.ts +++ b/tests/core/unit/server/server-redefine-and-attach.test.ts @@ -605,7 +605,7 @@ describe('hot_reload and attach stopOnEntry tests', () => { }); it('surfaces the attach note at the top level when the stop landed where no frame is program code (issues #904/#900)', async () => { - const note = "No program frame is active on thread 0: the only frame is internal/runtime (top: '' :1). The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; get_stack_trace with includeInternals: true lists the frames."; + const note = "No program frame is active on thread 0: the only frame is internal/runtime (top: '' :1). The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint."; mockSessionManager.attachToProcess.mockResolvedValue({ success: true, state: 'paused', diff --git a/tests/core/unit/session/inspection/stop-location.test.ts b/tests/core/unit/session/inspection/stop-location.test.ts index c2819e16d..374efbf7e 100644 --- a/tests/core/unit/session/inspection/stop-location.test.ts +++ b/tests/core/unit/session/inspection/stop-location.test.ts @@ -106,15 +106,32 @@ describe('describeStopLocation (issue #904)', () => { expect(landing.noProgramFrame).toBe(true); expect(landing.location).toEqual({ file: '', line: 1, column: 1 }); + // One frame, and the note names it: there is no fuller stack to point at (issue #1141). expect(landing.note).toBe( "No program frame is active on thread 0: the only frame is internal/runtime (top: '' :1). " + - 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; ' + - 'get_stack_trace with includeInternals: true lists the frames.' + 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint.' ); // The flag stays internal; the answer carries the sentence. expect(landingFields(landing)).toEqual({ location: { file: '', line: 1, column: 1 }, note: landing.note }); }); + it('points at get_stack_trace when the thread has more frames than the one the answer shows (issue #1141)', async () => { + const crt = { id: 1027, name: '__tmainCRTStartup', file: '@__tmainCRTStartup', line: 51, column: 0 }; + const { ctx } = context({ + reason: 'step', + stopThreadId: 12952, + read: result({ frames: [crt], threadId: 12952, totalFrameCount: 4, hiddenFrameCount: 3, allFramesInternal: true, noProgramFrame: true }) + }); + + const landing = await describeStopLocation(ctx, 'sid', 'stepOut'); + + expect(landing.note).toBe( + "No program frame is active on thread 12952: all 4 frames are internal/runtime (top: '__tmainCRTStartup' @__tmainCRTStartup:51). " + + 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; ' + + 'get_stack_trace with includeInternals: true lists the frames.' + ); + }); + it('keeps the resolver note for a frameless landing while the session is still paused', async () => { const note = 'The stopped thread reported no stack frames within 3000ms; the target may be paused in native code.'; const read = result({ frames: [], totalFrameCount: 0, threadId: 1, note }); diff --git a/tests/core/unit/session/inspection/stop-snapshot.test.ts b/tests/core/unit/session/inspection/stop-snapshot.test.ts index ebc4b393a..0949116d3 100644 --- a/tests/core/unit/session/inspection/stop-snapshot.test.ts +++ b/tests/core/unit/session/inspection/stop-snapshot.test.ts @@ -513,9 +513,11 @@ describe('composeStopSnapshot diff cache (issue #983)', () => { newStop(session); const second = await composeStopSnapshot(ctx, 'sid', stack(), 'test'); expect(second?.locals).toBeUndefined(); + // A variable that was not there is marked as such, with no invented + // "before" value (issue #1140). expect(second?.changedVariables).toEqual({ subtotal: { before: '100.0', after: '125.0' }, - discount: { before: '', after: '15.0' } + discount: { after: '15.0', new: true } }); }); @@ -601,6 +603,206 @@ describe('composeStopSnapshot diff cache (issue #983)', () => { }); }); +describe('composeStopSnapshot diff vocabulary: changed, new, gone (issue #1140)', () => { + type Extract = ReturnType; + const COMPUTE = { name: 'compute', file: '/proj/app.py', line: 44 }; + const variablesOf = (vals: Array<[string, string]>) => + vals.map(([name, value]) => ({ name, value, type: 'str', variablesReference: 0 })); + const localsResult = (vals: Array<[string, string]>, extra: Record = {}) => ({ + variables: variablesOf(vals), + frame: COMPUTE, + scopeName: 'Locals', + ...extra + }); + const numbered = (prefix: string, count: number): Array<[string, string]> => + Array.from({ length: count }, (_, i): [string, string] => [`${prefix}${String(i).padStart(2, '0')}`, `${i}`]); + const newStop = (session: Record) => { + session.lastStop = { reason: 'step', timestamp: Date.now(), threadId: 1 }; + }; + + /** Compose once per read, each on a fresh stop, and answer the last snapshot. */ + async function diffOf(...reads: Array>) { + const { ctx, extractLocalsFromResolved, session } = makeCtx(); + for (const read of reads) { + (extractLocalsFromResolved as Extract).mockResolvedValueOnce(read); + } + let last: Awaited>; + for (let i = 0; i < reads.length; i++) { + if (i > 0) newStop(session); + last = await composeStopSnapshot(ctx, 'sid', stack(), 'test'); + } + return last; + } + + it('says all three in one map: what changed, what appeared, what left scope', async () => { + const snapshot = await diffOf( + localsResult([['x', '1'], ['i', '3']]), + localsResult([['x', '2'], ['sum', '30']]) + ); + + expect(snapshot?.changedVariables).toEqual({ + x: { before: '1', after: '2' }, + sum: { after: '30', new: true }, + i: { before: '3', gone: true } + }); + expect(snapshot?.locals).toBeUndefined(); + }); + + it('a variable that left scope carries its last shown value', async () => { + const snapshot = await diffOf(localsResult([['x', '1'], ['i', '3']]), localsResult([['x', '1']])); + + expect(snapshot?.changedVariables).toEqual({ i: { before: '3', gone: true } }); + }); + + it('an empty map still means nothing visible changed', async () => { + const snapshot = await diffOf(localsResult([['x', '1']]), localsResult([['x', '1']])); + + expect(snapshot?.changedVariables).toEqual({}); + }); + + it('never calls a variable new when the previous list was cut: it may have been there, unseen', async () => { + // 25 locals, 20 shown: v20..v24 exist and were never shown. + const snapshot = await diffOf( + localsResult(numbered('v', 25)), + localsResult([['v24', '24'], ...numbered('v', 19)]) + ); + + // Shown now, absent from what was shown before — and that is all that is known. + expect(snapshot?.changedVariables?.v24).toEqual({ after: '24' }); + // v19 WAS shown and is absent from a list that is complete: that is known. + expect(snapshot?.changedVariables?.v19).toEqual({ before: '19', gone: true }); + expect(Object.keys(snapshot?.changedVariables ?? {}).sort()).toEqual(['v19', 'v24']); + }); + + it('never calls a variable gone when the current list was cut: it may still be there, unseen', async () => { + const snapshot = await diffOf( + localsResult([['a', '1'], ['b', '2']]), + localsResult([['a', '1'], ...numbered('c', 24)]) + ); + + // b is not among the 20 shown, and 5 were omitted: nothing can be said of b. + expect(snapshot?.changedVariables).not.toHaveProperty('b'); + // The previous list was complete, so what is shown now and was not there is new. + expect(snapshot?.changedVariables?.c00).toEqual({ after: '0', new: true }); + expect(snapshot?.truncation).toContain('omitted'); + }); + + it('never calls a variable gone when a scope went unread this time', async () => { + const snapshot = await diffOf( + localsResult([['a', '1'], ['b', '2']]), + localsResult([['a', '1']], { truncation: { omittedCount: 0, valueTruncatedCount: 0, scopesSkipped: 1 } }) + ); + + expect(snapshot?.changedVariables).toEqual({}); + }); + + it('never calls a variable new when a scope went unread last time', async () => { + const snapshot = await diffOf( + localsResult([['a', '1']], { truncation: { omittedCount: 0, valueTruncatedCount: 0, scopesSkipped: 1 } }), + localsResult([['a', '1'], ['b', '2']]) + ); + + expect(snapshot?.changedVariables).toEqual({ b: { after: '2' } }); + }); + + it('a list cut by its upstream caps counts as cut', async () => { + const snapshot = await diffOf( + localsResult([['a', '1']], { truncation: { omittedCount: 3, valueTruncatedCount: 0 } }), + localsResult([['a', '1'], ['b', '2']]) + ); + + expect(snapshot?.changedVariables).toEqual({ b: { after: '2' } }); + }); + + it('a value that was only shortened does not make its list incomplete', async () => { + const snapshot = await diffOf( + localsResult([['a', 'y'.repeat(500)]]), + localsResult([['a', 'y'.repeat(500)], ['b', '2']]) + ); + + expect(snapshot?.changedVariables).toEqual({ b: { after: '2', new: true } }); + }); + + it('carries completeness forward: the list a diff was built from is the next base', async () => { + const snapshot = await diffOf( + localsResult([['a', '1']]), + localsResult([['a', '1'], ...numbered('c', 24)]), // cut: becomes an incomplete base + localsResult([['a', '1'], ['z', '9']]) + ); + + // z was not among the 20 shown, but 5 were omitted then: not provably new. + expect(snapshot?.changedVariables?.z).toEqual({ after: '9' }); + // c00 was shown and is absent from a complete list: gone. + expect(snapshot?.changedVariables?.c00).toEqual({ before: '0', gone: true }); + }); + + it('treats the text "" as the value it is', async () => { + const snapshot = await diffOf( + localsResult([['x', '']]), + localsResult([['x', '5'], ['y', '']]) + ); + + expect(snapshot?.changedVariables).toEqual({ + x: { before: '', after: '5' }, + y: { after: '', new: true } + }); + }); + + it('keeps exotic names as own keys in both directions', async () => { + const appeared = await diffOf(localsResult([['x', '1']]), localsResult([['x', '1'], ['__proto__', '{}']])); + expect(Object.prototype.hasOwnProperty.call(appeared?.changedVariables, '__proto__')).toBe(true); + expect(JSON.parse(JSON.stringify(appeared?.changedVariables))['__proto__']).toEqual({ after: '{}', new: true }); + + const left = await diffOf(localsResult([['x', '1'], ['__proto__', '{}']]), localsResult([['x', '1']])); + expect(Object.prototype.hasOwnProperty.call(left?.changedVariables, '__proto__')).toBe(true); + expect(JSON.parse(JSON.stringify(left?.changedVariables))['__proto__']).toEqual({ before: '{}', gone: true }); + + // An inherited name is not a variable: "constructor" is new, not changed. + const inherited = await diffOf(localsResult([['x', '1']]), localsResult([['x', '1'], ['constructor', 'f']])); + expect(inherited?.changedVariables).toEqual({ constructor: { after: 'f', new: true } }); + }); + + it("reports an adapter's return-value entry as it happens: new when it appears", async () => { + // debugpy keeps "(return) add" in the frame once it appears (measured, + // 1.8.20); js-debug shows "Return value" at a function's return point. + const appeared = await diffOf( + localsResult([['a', '1']]), + localsResult([['a', '1'], ['(return) add', '3'], ['s', '3']]) + ); + expect(appeared?.changedVariables).toEqual({ + '(return) add': { after: '3', new: true }, + s: { after: '3', new: true } + }); + + // It stays, so the next stop says nothing more about it. + const snapshot = await diffOf( + localsResult([['a', '1']]), + localsResult([['a', '1'], ['(return) add', '3'], ['s', '3']]), + localsResult([['a', '1'], ['(return) add', '3'], ['s', '3'], ['t', '4']]) + ); + + expect(snapshot?.changedVariables).toEqual({ t: { after: '4', new: true } }); + }); + + it('an empty read emits no diff and leaves the base alone, so the next diff is against the last shown list', async () => { + const { ctx, extractLocalsFromResolved, session } = makeCtx(); + (extractLocalsFromResolved as Extract) + .mockResolvedValueOnce(localsResult([['x', '1']])) + .mockResolvedValueOnce(localsResult([])) + .mockResolvedValueOnce(localsResult([['x', '2']])); + + await composeStopSnapshot(ctx, 'sid', stack(), 'test'); + newStop(session); + const empty = await composeStopSnapshot(ctx, 'sid', stack(), 'test'); + expect(empty).not.toHaveProperty('changedVariables'); + expect(empty).not.toHaveProperty('locals'); + + newStop(session); + const third = await composeStopSnapshot(ctx, 'sid', stack(), 'test'); + expect(third?.changedVariables).toEqual({ x: { before: '1', after: '2' } }); + }); +}); + describe('review fixes: exception pair orientation (pre-merge heuristic)', () => { it('labels the identifier-shaped half as type when the raw pair arrives in spec order, without exceptionInfo', async () => { const { ctx } = makeCtx({ diff --git a/tests/core/unit/session/session-manager-dap.test.ts b/tests/core/unit/session/session-manager-dap.test.ts index 34492e60a..5cf3eb89e 100644 --- a/tests/core/unit/session/session-manager-dap.test.ts +++ b/tests/core/unit/session/session-manager-dap.test.ts @@ -3354,6 +3354,65 @@ describe('SessionManager - DAP Operations', () => { expect(result.note).toBeUndefined(); }); + describe('locals read on a stack with no program frame (issue #1135)', () => { + const TIMERS = { id: 1, name: 'process.processTimers', line: 527, column: 5, source: { path: '/internal/timers', sourceReference: 7 } }; + const NEXT_TICK = { id: 2, name: 'processTicksAndRejections', line: 103, column: 5, source: { path: '/internal/process/task_queues', sourceReference: 9 } }; + const USER = { id: 3, name: 'tick', line: 4, column: 3, source: { path: '/app/pause_test.js', sourceReference: 0 } }; + + /** A paused js session whose one thread reports `frames`, with a Local scope holding `now` on frame 1. */ + async function pausedWith(frames: unknown[], reason = 'pause') { + const session = await pausedInDependency(reason, { 1: frames }); + dependencies.mockProxyManager.setDapRequestHandler(async (command: string, args?: { frameId?: number; variablesReference?: number }) => { + if (command === 'stackTrace') { + return { success: true, body: { stackFrames: frames } }; + } + if (command === 'scopes') { + return { + success: true, + body: { scopes: args?.frameId === 1 ? [{ name: 'Local: processTimers', variablesReference: 11, expensive: false }] : [] } + }; + } + if (command === 'variables' && args?.variablesReference === 11) { + return { success: true, body: { variables: [{ name: 'now', value: '4210', type: 'number', variablesReference: 0 }] } }; + } + return { success: true, body: {} }; + }); + return session; + } + + it('still answers the runtime frame\'s variables, and says the stack has no program frame', async () => { + const session = await pausedWith([TIMERS]); + + const result = await sessionManager.getLocalVariables(session.id); + + // The adapter's answer is kept: it is what the frame holds. + expect(result.variables.map((variable) => [variable.name, variable.value])).toEqual([['now', '4210']]); + expect(result.frame).toEqual({ name: 'process.processTimers', file: '/internal/timers', line: 527 }); + // The resolver's verdict rides along, with what the sentence needs. + expect(result.noProgramFrame).toEqual({ + threadId: 1, + frameCount: 1, + top: { name: 'process.processTimers', file: '/internal/timers', line: 527 } + }); + }); + + it('counts every frame of the thread, not only the one kept visible', async () => { + const session = await pausedWith([TIMERS, NEXT_TICK]); + + const result = await sessionManager.getLocalVariables(session.id); + + expect(result.noProgramFrame).toMatchObject({ frameCount: 2, top: { name: 'process.processTimers' } }); + }); + + it('says nothing of the kind when the stack reaches the program', async () => { + const session = await pausedWith([TIMERS, USER], 'breakpoint'); + + const result = await sessionManager.getLocalVariables(session.id); + + expect(result).not.toHaveProperty('noProgramFrame'); + }); + }); + it('an all-internal stack on a breakpoint still takes the issue #346 path, not both', async () => { const session = await pausedInDependency('breakpoint', { 1: DEPENDENCY_PAUSE.slice(0, 4) }); diff --git a/tests/core/unit/session/session-manager-evaluate.test.ts b/tests/core/unit/session/session-manager-evaluate.test.ts index 7933000c9..01c78eb14 100644 --- a/tests/core/unit/session/session-manager-evaluate.test.ts +++ b/tests/core/unit/session/session-manager-evaluate.test.ts @@ -215,6 +215,80 @@ describe('SessionManager.evaluateExpression default-frame resolution', () => { expect(result.anchorNote).toMatch(/frameId: 30/); }); + describe('default evaluation on a stack with no program frame (issue #1135)', () => { + const TIMERS = { id: 7, name: 'process.processTimers', line: 527, column: 5, source: { path: '/internal/timers', sourceReference: 7 } }; + const NEXT_TICK = { id: 8, name: 'processTicksAndRejections', line: 103, column: 5, source: { path: '/internal/process/task_queues', sourceReference: 9 } }; + + async function pausedOutsideTheProgram(frames: unknown[]) { + const { sessionManager, dependencies } = makeManager({ stopOnEntry: false }); + const session = await createRunningSession(sessionManager, dependencies, { paused: false }); + dependencies.mockProxyManager.simulateStopped(1, 'pause'); + (sessionManager as unknown as { selectPolicy: () => unknown }).selectPolicy = () => JsDebugAdapterPolicy; + const evaluated: Array = []; + dependencies.mockProxyManager.setDapRequestHandler(async (command: string, args?: { frameId?: number }) => { + if (command === 'stackTrace') { + return { body: { stackFrames: frames } }; + } + if (command === 'evaluate') { + evaluated.push(args?.frameId); + return { body: { result: "'number'", type: 'string', variablesReference: 0 } }; + } + return { success: true, body: {} }; + }); + return { sessionManager, session, evaluated }; + } + + it('evaluates in the runtime frame and says that is what it is', async () => { + const { sessionManager, session, evaluated } = await pausedOutsideTheProgram([TIMERS]); + + const result = await sessionManager.evaluateExpression(session.id, 'typeof now'); + + expect(result.success).toBe(true); + expect(evaluated).toEqual([7]); + expect(result.frame).toEqual({ name: 'process.processTimers', file: '/internal/timers', line: 527 }); + expect(result.noProgramFrame).toBe(true); + // One frame, already named: nothing more for get_stack_trace to list (issue #1141). + expect(result.anchorNote).toBe( + "No program frame is active on thread 1: the only frame is internal/runtime (top: 'process.processTimers' /internal/timers:527). " + + 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint. ' + + 'The expression was evaluated in that frame.' + ); + }); + + it('points at the full stack when the thread has more runtime frames', async () => { + const { sessionManager, session } = await pausedOutsideTheProgram([TIMERS, NEXT_TICK]); + + const result = await sessionManager.evaluateExpression(session.id, 'typeof now'); + + expect(result.noProgramFrame).toBe(true); + expect(result.anchorNote).toContain('all 2 frames are internal/runtime'); + expect(result.anchorNote).toContain('get_stack_trace with includeInternals: true lists the frames.'); + }); + + it('says nothing for an explicit frameId: the caller chose the frame', async () => { + const { sessionManager, session, evaluated } = await pausedOutsideTheProgram([TIMERS]); + + const result = await sessionManager.evaluateExpression(session.id, 'typeof now', 7); + + expect(evaluated).toEqual([7]); + expect(result).not.toHaveProperty('noProgramFrame'); + expect(result).not.toHaveProperty('anchorNote'); + }); + + it('says nothing of the kind when the stack reaches the program, runtime frames below it or not', async () => { + const PROGRAM = { id: 3, name: 'tick', line: 12, column: 3, source: { path: '/work/app/server.js' } }; + const { sessionManager, session, evaluated } = await pausedOutsideTheProgram([PROGRAM, TIMERS, NEXT_TICK]); + + const result = await sessionManager.evaluateExpression(session.id, 'typeof now'); + + expect(result.success).toBe(true); + expect(evaluated).toEqual([3]); + expect(result.frame).toEqual({ name: 'tick', file: '/work/app/server.js', line: 12 }); + expect(result).not.toHaveProperty('noProgramFrame'); + expect(result).not.toHaveProperty('anchorNote'); + }); + }); + it('names the anchor frame when the evaluation itself fails (#672)', async () => { const { sessionManager, dependencies } = makeManager(); const session = await createRunningSession(sessionManager, dependencies); diff --git a/tests/core/unit/session/session-manager-snapshot-completeness.test.ts b/tests/core/unit/session/session-manager-snapshot-completeness.test.ts new file mode 100644 index 000000000..c4f503106 --- /dev/null +++ b/tests/core/unit/session/session-manager-snapshot-completeness.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { SessionManager } from '../../../../src/session/session-manager.js'; +import type { LocalVariablesResult } from '../../../../src/session/session-manager-data.js'; +import { DebugLanguage, JsDebugAdapterPolicy, type AdapterPolicy } from '@debugmcp/shared'; +import { createMockDependencies, createMockEnvironment } from './session-manager-test-utils.js'; + +describe('snapshot completeness after a failed local scope read', () => { + beforeEach(() => vi.useFakeTimers({ shouldAdvanceTime: true })); + afterEach(() => { vi.useRealTimers(); vi.clearAllMocks(); }); + + it.each(['throw', 'rejected response', 'missing body'])( + 'does not call an unread block variable gone and new again after %s', async (failure) => { + const dependencies = createMockDependencies(); + dependencies.environment = createMockEnvironment({ DEBUG_MCP_NO_REDACT: '' }); + const manager = new SessionManager({ + logDirBase: '/tmp/test-sessions', + defaultDapLaunchArgs: { stopOnEntry: true, justMyCode: true } + }, dependencies); + const session = await manager.createSession({ language: DebugLanguage.MOCK, executablePath: 'python' }); + await manager.startDebugging(session.id, 'test.py'); + await vi.runAllTimersAsync(); + dependencies.mockProxyManager.simulateStopped(1, 'entry'); + const internals = manager as unknown as { selectPolicy(language: string): AdapterPolicy }; + const policy = internals.selectPolicy('mock'); + internals.selectPolicy = () => ({ ...policy, extractLocalVariables: JsDebugAdapterPolicy.extractLocalVariables, getLocalScopeName: JsDebugAdapterPolicy.getLocalScopeName }); + let blockFails = false; + let blockEmpty = false; + let lowerScopesFail = false; + dependencies.mockProxyManager.setDapRequestHandler(async (command: string, args?: any) => { + if (command === 'stackTrace') return { success: true, body: { stackFrames: [1, 2].map(id => ({ + id, name: id === 1 ? 'main' : 'caller', source: { path: 'test.py' }, line: 10, column: 0 + })) } }; + if (command === 'scopes' && args.frameId === 2) { + if (lowerScopesFail) throw new Error('caller scopes unavailable'); + return { success: true, body: { scopes: [] } }; + } + if (command === 'scopes') return { success: true, body: { scopes: [ + { name: 'Block', variablesReference: 100, expensive: false }, + { name: 'Local', variablesReference: 101, expensive: false } + ] } }; + if (command === 'variables') { + if (args.variablesReference === 100 && blockFails) { + if (failure === 'throw') throw new Error('temporary scope read failure'); + if (failure === 'rejected response') return { success: false, message: 'scope unavailable' }; + return { success: true }; + } + if (args.variablesReference === 100 && blockEmpty) return { success: true, body: { variables: [] } }; + return { success: true, body: { variables: [{ name: args.variablesReference === 100 ? 'i' : 'x', value: '1', type: 'int', variablesReference: 0 }] } }; + } + return { success: true, args }; + }); + const step = async () => { + const pending = manager.stepOver(session.id); + await vi.advanceTimersByTimeAsync(20); + dependencies.mockProxyManager.simulateStopped(1, 'step'); + await vi.advanceTimersByTimeAsync(30); + return pending; + }; + const first = await step(); + expect(first.data?.snapshot?.locals).toEqual({ i: '1', x: '1' }); + blockFails = true; + const failedRead = await step(); + expect(failedRead.data?.snapshot?.changedVariables).toEqual({}); + expect(failedRead.data?.snapshot?.note).toContain('could not be read'); + blockFails = false; + const recovered = await step(); + expect(recovered.data?.snapshot?.changedVariables?.i).toEqual({ after: '1' }); + expect(recovered.data?.snapshot?.note).toBeUndefined(); + // A successful empty response is evidence of absence, unlike a failure. + // An unused caller's failed discovery must not obscure the top frame. + blockEmpty = true; + lowerScopesFail = true; + const leftScope = await step(); + expect(leftScope.data?.snapshot?.changedVariables).toEqual({ i: { before: '1', gone: true } }); + expect(leftScope.data?.snapshot?.note).toBeUndefined(); + }); + + it.each(['throw', 'rejected response', 'missing body'])( + 'does not treat failed scope discovery as an empty recursive frame after %s', async (failure) => { + const dependencies = createMockDependencies(); + dependencies.environment = createMockEnvironment({ DEBUG_MCP_NO_REDACT: '' }); + const manager = new SessionManager({ + logDirBase: '/tmp/test-sessions', + defaultDapLaunchArgs: { stopOnEntry: true, justMyCode: true } + }, dependencies); + const session = await manager.createSession({ language: DebugLanguage.MOCK, executablePath: 'python' }); + await manager.startDebugging(session.id, 'test.py'); + await vi.runAllTimersAsync(); + dependencies.mockProxyManager.simulateStopped(1, 'entry'); + let topScopesFail = false; + let topScopesEmpty = false; + dependencies.mockProxyManager.setDapRequestHandler(async (command: string, args?: any) => { + if (command === 'stackTrace') return { success: true, body: { stackFrames: [1, 2].map(id => ({ + id, name: 'recurse', source: { path: 'test.py' }, line: 10, column: 0 + })) } }; + if (command === 'scopes') { + if (args.frameId === 1 && topScopesFail) { + if (failure === 'throw') throw new Error('temporary scope discovery failure'); + if (failure === 'rejected response') return { success: false, message: 'scopes unavailable' }; + return { success: true }; + } + return { success: true, body: { scopes: args.frameId === 1 && topScopesEmpty ? [] : [ + { name: 'Locals', variablesReference: 100 + args.frameId, expensive: false } + ] } }; + } + if (command === 'variables') return { success: true, body: { variables: + (args.variablesReference === 101 ? ['i', 'x'] : ['x']).map(name => ({ + name, value: '1', type: 'int', variablesReference: 0 + })) + } }; + return { success: true, args }; + }); + const step = async () => { + const pending = manager.stepOver(session.id); + await vi.advanceTimersByTimeAsync(20); + dependencies.mockProxyManager.simulateStopped(1, 'step'); + await vi.advanceTimersByTimeAsync(30); + return pending; + }; + const first = await step(); + expect(first.data?.snapshot?.locals).toEqual({ i: '1', x: '1' }); + topScopesFail = true; + const failedRead = await step(); + expect(failedRead.data?.snapshot?.changedVariables).toEqual({}); + expect(failedRead.data?.snapshot?.note).toContain('could not be read'); + topScopesFail = false; + const recovered = await step(); + expect(recovered.data?.snapshot?.changedVariables?.i).toEqual({ after: '1' }); + expect(recovered.data?.snapshot?.note).toBeUndefined(); + topScopesEmpty = true; + expect(await manager.getScopes(session.id, 1)).toEqual([]); + const successfulEmpty = await manager.getLocalVariables(session.id) as LocalVariablesResult; + expect(successfulEmpty.incomplete).toBeUndefined(); + expect(successfulEmpty.anchorNote).not.toContain('could not be read'); + }); +}); diff --git a/tests/core/unit/session/session-manager-snapshot.test.ts b/tests/core/unit/session/session-manager-snapshot.test.ts index f951e026f..9a5d8d8fd 100644 --- a/tests/core/unit/session/session-manager-snapshot.test.ts +++ b/tests/core/unit/session/session-manager-snapshot.test.ts @@ -275,7 +275,8 @@ describe('step answers join the snapshot with changedVariables (issue #983)', () /** Deep-stack stub whose variable values and scopes-call count are mutable/observable. */ function stubSteppable(dependencies: ReturnType) { - const state = { x: 1, scopes: 0, autoStopOnNext: false }; + const state: { x: number; scopes: number; autoStopOnNext: boolean; extra: Array<{ name: string; value: string }> } = + { x: 1, scopes: 0, autoStopOnNext: false, extra: [] }; dependencies.mockProxyManager.setDapRequestHandler(async (command: string, args?: any) => { if (command === 'stackTrace') { return { @@ -301,7 +302,12 @@ describe('step answers join the snapshot with changedVariables (issue #983)', () if (command === 'variables') { return { success: true, - body: { variables: [{ name: 'x', value: String(state.x), type: 'int', variablesReference: 0 }] } + body: { + variables: [ + { name: 'x', value: String(state.x), type: 'int', variablesReference: 0 }, + ...state.extra.map((variable) => ({ ...variable, type: 'int', variablesReference: 0 })) + ] + } }; } if (command === 'next' && state.autoStopOnNext) { @@ -341,6 +347,30 @@ describe('step answers join the snapshot with changedVariables (issue #983)', () expect(second.data?.snapshot?.changedVariables).toEqual({ x: { before: '1', after: '2' } }); }); + it('a step past a declaration says the variable is new, and a step out of its block says it is gone (issue #1140)', async () => { + const { sessionManager, dependencies } = makeManager(); + const session = await createPausedSession(sessionManager, dependencies); + const state = stubSteppable(dependencies); + + await steppedOnce(sessionManager, dependencies, session.id); + + // `int sum = add(x, y);` — sum comes into scope. + state.extra = [{ name: 'sum', value: '30' }]; + const declared = await steppedOnce(sessionManager, dependencies, session.id); + expect(declared.data?.snapshot?.changedVariables).toEqual({ sum: { after: '30', new: true } }); + // No invented value stands in for "it was not there". + expect(JSON.stringify(declared.data?.snapshot)).not.toContain(''); + + // The block ends: sum is no longer among the locals. + state.extra = []; + const left = await steppedOnce(sessionManager, dependencies, session.id); + expect(left.data?.snapshot?.changedVariables).toEqual({ sum: { before: '30', gone: true } }); + + // And nothing more to say on the step after that. + const quiet = await steppedOnce(sessionManager, dependencies, session.id); + expect(quiet.data?.snapshot?.changedVariables).toEqual({}); + }); + it('a pending step carries no snapshot', async () => { const { sessionManager, dependencies } = makeManager(); const session = await createPausedSession(sessionManager, dependencies); diff --git a/tests/e2e/mcp-server-smoke-python.test.ts b/tests/e2e/mcp-server-smoke-python.test.ts index 54f0d7f8f..abced1d5e 100644 --- a/tests/e2e/mcp-server-smoke-python.test.ts +++ b/tests/e2e/mcp-server-smoke-python.test.ts @@ -255,10 +255,12 @@ describe('MCP Server Python Debugging Smoke Test', () => { if (secondStep.state === 'paused' && secondStep.snapshot) { const stepSnapshot = secondStep.snapshot as { locals?: Record; - changedVariables?: Record; + changedVariables?: Record; }; console.log('[Python Smoke Test] Second step changedVariables:', JSON.stringify(stepSnapshot.changedVariables)); expect(stepSnapshot.changedVariables, 'second step in the same frame carries the diff').toBeDefined(); + // No placeholder stands in for a missing value (issue #1140): a new variable says new. + expect(JSON.stringify(stepSnapshot.changedVariables)).not.toContain(''); expect(stepSnapshot.locals).toBeUndefined(); } } diff --git a/tests/fixtures/debug-scripts/js-block-scope.js b/tests/fixtures/debug-scripts/js-block-scope.js new file mode 100644 index 000000000..2a93cee24 --- /dev/null +++ b/tests/fixtures/debug-scripts/js-block-scope.js @@ -0,0 +1,16 @@ +// A block whose variables leave scope while the function goes on: the stop +// snapshot's diff reports them as gone (issue #1140). Line numbers are +// breakpoint targets for +// tests/adapters/javascript/integration/javascript-snapshot-diff.test.ts; +// do not shift them. +function main() { + let total = 0; + for (let i = 0; i < 1; i++) { + const doubled = (i + 1) * 2; + total += doubled; + } + const done = total > 0; + return done; +} + +main(); diff --git a/tests/proxy/dap-proxy-worker.test.ts b/tests/proxy/dap-proxy-worker.test.ts index 6ba672fa7..6e71b5bea 100644 --- a/tests/proxy/dap-proxy-worker.test.ts +++ b/tests/proxy/dap-proxy-worker.test.ts @@ -10,6 +10,7 @@ import { describe, it, expect, beforeEach, vi, type Mock } from 'vitest'; import { DapProxyWorker, MAX_QUEUED_COMMANDS } from '../../src/proxy/dap-proxy-worker.js'; import { GenericAdapterManager } from '../../src/proxy/dap-proxy-adapter-manager.js'; import { DapConnectionManager } from '../../src/proxy/dap-proxy-connection-manager.js'; +import { DapResponseError } from '../../src/proxy/dap-response-error.js'; import type { DapProxyDependencies, ILogger, @@ -1765,6 +1766,9 @@ describe('DapProxyWorker', () => { const capabilitiesStatuses = () => mockMessageSender.send.mock.calls.filter( ([message]) => message.type === 'status' && message.status === 'adapter_capabilities' ); + const directConnectStatuses = () => mockMessageSender.send.mock.calls + .map(([message]) => message) + .filter((message) => message.type === 'status' && message.status === 'direct_connect'); it('ruby launch proceeds when the initialize response never arrives but initialized did (issue #492)', async () => { vi.useFakeTimers(); @@ -1864,6 +1868,272 @@ describe('DapProxyWorker', () => { 'request_pending:launch', 'response_received:launch' ]); + // A spawned adapter is not a direct connection (issue #1139). + expect(directConnectStatuses()).toEqual([]); + }); + + describe('direct-connect attach: the worker dials a listener it did not start (issue #1139)', () => { + const LISTENER = 'an rdbg debug server (rdbg --open --port 12345 …)'; + const rubyAttachPayload = (): ProxyInitPayload => ({ + cmd: 'init', + sessionId: 'ruby-direct-session', + language: 'ruby', + executablePath: 'ruby', + adapterHost: '127.0.0.1', + adapterPort: 8124, + logDir: '/logs', + scriptPath: 'attach://remote', + launchConfig: { request: 'attach', host: '10.0.0.5', port: 12345 } + }); + const errorsSent = () => mockMessageSender.send.mock.calls + .map(([message]) => message as { type: string; message?: string }) + .filter((message) => message.type === 'error') + .map((message) => message.message); + + /** Run the worker's connect-and-handshake to its failure, with the teardown stubbed out. */ + async function failedAttach(stubs: { processStub: unknown; connectionStub: unknown }): Promise { + wireWorker(rubyAttachPayload(), stubs.processStub, stubs.connectionStub, RubyAdapterPolicy); + vi.spyOn(worker as unknown as { shutdown: () => Promise }, 'shutdown').mockResolvedValue(undefined); + await expect((worker as any).startAdapterAndConnect(rubyAttachPayload())).rejects.toThrow(); + return errorsSent() as string[]; + } + + it('says what it dialed, before the transport connects, and spawns nothing', async () => { + const stubs = makeStubs(async () => { throw new Error('DAP client disconnected'); }); + + await failedAttach(stubs); + + expect(stubs.processStub.spawn).not.toHaveBeenCalled(); + expect(directConnectStatuses()).toEqual([ + expect.objectContaining({ status: 'direct_connect', host: '10.0.0.5', port: 12345, listener: LISTENER }) + ]); + const order = mockMessageSender.send.mock.calls + .map(([m]) => m as StatusMessage & { stage?: string }) + .filter((m) => m.type === 'status') + .map((m) => (m.status === 'dap_handshake_stage' ? m.stage : m.status)); + expect(order.indexOf('direct_connect')).toBeGreaterThanOrEqual(0); + expect(order.indexOf('direct_connect')).toBeLessThan(order.indexOf('transport_connected')); + expect(order).not.toContain('adapter_spawned'); + }); + + it('an endpoint that hangs up before answering initialize is reported as probably not a debug server', async () => { + // What an HTTP server does with a DAP frame: it closes the socket. + const errors = await failedAttach(makeStubs(async () => { throw new Error('DAP client disconnected'); })); + + expect(errors).toEqual([ + 'Critical initialization error: 10.0.0.5:12345 never answered the DAP "initialize" request (DAP client disconnected). ' + + 'Nothing was started for this attach: the server connected to what is already listening on that port, ' + + `so check that it is ${LISTENER} and not another service.` + ]); + // Nothing about an executable or an adapter process: there is none. + expect(errors[0]).not.toContain('Executable:'); + expect(errors[0]).not.toContain('adapter PID'); + }); + + it('an endpoint that refuses initialize in DAP is a debug server: its own words, and the endpoint', async () => { + const refusal = new DapResponseError({ + seq: 2, type: 'response', request_seq: 1, command: 'initialize', success: false, message: 'client not supported' + }); + const errors = await failedAttach(makeStubs(async () => { throw refusal; })); + + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('client not supported'); + expect(errors[0]).toContain('[direct connect to 10.0.0.5:12345; no adapter process was started]'); + expect(errors[0]).not.toContain('not another service'); + expect(errors[0]).not.toContain('adapter PID'); + }); + + it('a connection that was never made names the endpoint and says nothing was started', async () => { + const stubs = makeStubs(async () => undefined); + stubs.connectionStub.connectWithRetry.mockRejectedValue( + new Error('Failed to connect DAP client: nothing accepted a connection on 10.0.0.5:12345 within 12.5s (12.6s elapsed, 61 attempts; last error: ECONNREFUSED)') + ); + + const errors = await failedAttach(stubs); + + expect(errors).toEqual([ + 'Critical initialization error: Failed to connect DAP client: nothing accepted a connection on 10.0.0.5:12345 within 12.5s ' + + '(12.6s elapsed, 61 attempts; last error: ECONNREFUSED) [direct connect to 10.0.0.5:12345; no adapter process was started]' + ]); + }); + + /** The socket-error handler the worker registered on the DAP client. */ + const socketError = (stubs: ReturnType) => + (stubs.connectionStub.setupEventHandlers.mock.calls[0][1] as unknown as { onError: (err: Error) => void }).onError; + /** An endpoint that answers `initialize` and sends `initialized`: a debug server. */ + const answersInitialize = async () => { + queueMicrotask(() => mockDapClient.emit('initialized')); + return {}; + }; + + it('an endpoint that resets the connection before answering initialize gets that same report, once', async () => { + // A reset reaches the worker twice: the socket's error event first, + // then the pending request's rejection. The parent fails the attach + // on the first error it is sent, and that one used to be the bare + // "DAP client error: read ECONNRESET" — no endpoint, no word that + // nothing was started. + const stubs = makeStubs(async () => { + socketError(stubs)(new Error('read ECONNRESET')); + throw new Error('DAP client disconnected'); + }); + + const errors = await failedAttach(stubs); + + expect(errors).toEqual([ + 'Critical initialization error: 10.0.0.5:12345 never answered the DAP "initialize" request (read ECONNRESET). ' + + 'Nothing was started for this attach: the server connected to what is already listening on that port, ' + + `so check that it is ${LISTENER} and not another service.` + ]); + }); + + it('once initialize was answered, a later failure does not doubt that the endpoint is a debug server', async () => { + // rdbg answered `initialize`; the connection then dropped under the attach request. + const stubs = makeStubs(answersInitialize); + stubs.connectionStub.sendAttachRequest.mockRejectedValue(new Error('DAP client disconnected')); + + const errors = await failedAttach(stubs); + + expect(errors).toEqual([ + 'Critical initialization error: DAP client disconnected [direct connect to 10.0.0.5:12345; no adapter process was started]' + ]); + }); + + it('a reset after initialize was answered names the endpoint, once, without that doubt', async () => { + const stubs = makeStubs(answersInitialize); + stubs.connectionStub.sendAttachRequest.mockImplementation(async () => { + socketError(stubs)(new Error('read ECONNRESET')); + throw new Error('DAP client disconnected'); + }); + + const errors = await failedAttach(stubs); + + expect(errors).toEqual([ + 'Critical initialization error: read ECONNRESET [direct connect to 10.0.0.5:12345; no adapter process was started]' + ]); + }); + + /** The socket-close handler the worker registered on the DAP client. */ + const socketClose = (stubs: ReturnType) => + (stubs.connectionStub.setupEventHandlers.mock.calls[0][1] as unknown as { onClose: () => unknown }).onClose; + const DROPPED_AFTER_INITIALIZE = + 'Critical initialization error: DAP client disconnected [direct connect to 10.0.0.5:12345; no adapter process was started]'; + + it('a hang-up after initialize was answered is reported by endpoint the moment the socket closes', async () => { + // The client emits `close` before it rejects what was pending, and + // the worker answers a close with the connection-closed status. + // With debugpy's attach order that status reached the parent first, + // and the attach failed as "Proxy exited during initialization" + // followed by worker stderr — the report composed afterwards went + // to nobody. + const stubs = makeStubs(answersInitialize); + let reportedAtClose: unknown[] = []; + stubs.connectionStub.sendAttachRequest.mockImplementation(async () => { + void socketClose(stubs)(); + reportedAtClose = errorsSent(); + throw new Error('DAP client disconnected'); + }); + + const errors = await failedAttach(stubs); + + expect(reportedAtClose).toEqual([DROPPED_AFTER_INITIALIZE]); + expect(errors).toEqual([DROPPED_AFTER_INITIALIZE]); + }); + + it('a hang-up before initialize is answered is reported at the close too, in the words it always had', async () => { + let reportedAtClose: unknown[] = []; + const stubs = makeStubs(async () => { + void socketClose(stubs)(); + reportedAtClose = errorsSent(); + throw new Error('DAP client disconnected'); + }); + + const errors = await failedAttach(stubs); + + const report = + 'Critical initialization error: 10.0.0.5:12345 never answered the DAP "initialize" request (DAP client disconnected). ' + + 'Nothing was started for this attach: the server connected to what is already listening on that port, ' + + `so check that it is ${LISTENER} and not another service.`; + expect(reportedAtClose).toEqual([report]); + expect(errors).toEqual([report]); + }); + + it('a hang-up during the configuration phase is reported by endpoint, not as an error in the DAP sequence', async () => { + // initialize answered, attach answered, then the connection drops + // under configurationDone: "Error in DAP sequence: DAP client + // disconnected" named neither the endpoint nor that nothing was started. + const stubs = makeStubs(answersInitialize); + stubs.connectionStub.sendConfigurationDone.mockImplementation(async () => { + void socketClose(stubs)(); + throw new Error('DAP client disconnected'); + }); + wireWorker(rubyAttachPayload(), stubs.processStub, stubs.connectionStub, RubyAdapterPolicy); + vi.spyOn(worker as unknown as { shutdown: () => Promise }, 'shutdown').mockResolvedValue(undefined); + + await (worker as any).startAdapterAndConnect(rubyAttachPayload()).catch(() => undefined); + + expect(stubs.connectionStub.sendConfigurationDone).toHaveBeenCalled(); + expect(errorsSent()).toEqual([DROPPED_AFTER_INITIALIZE]); + }); + + it('a close after a failure it has already reported adds nothing', async () => { + // The refusal is the report; the socket closing afterwards (the + // worker's own teardown, or the server hanging up) is not news. + const refusal = new DapResponseError({ + seq: 2, type: 'response', request_seq: 1, command: 'initialize', success: false, message: 'client not supported' + }); + const stubs = makeStubs(async () => { throw refusal; }); + + const errors = await failedAttach(stubs); + void socketClose(stubs)(); + + expect(errorsSent()).toEqual(errors); + expect(errors).toHaveLength(1); + expect(errors[0]).toContain('client not supported'); + }); + + it('a close on an established direct connection reports no error: the session ended', () => { + const stubs = makeStubs(async () => ({})); + wireWorker(rubyAttachPayload(), stubs.processStub, stubs.connectionStub, RubyAdapterPolicy); + (worker as any).dapClient = mockDapClient; + (worker as any).directConnect = { host: '10.0.0.5', port: 12345, listener: LISTENER }; + (worker as any).setupDapEventHandlers(); + (worker as any).state = ProxyState.CONNECTED; + + void socketClose(stubs)(); + + expect(errorsSent()).toEqual([]); + }); + + it('a socket error on an established direct connection keeps the plain report: it is not an init failure', () => { + const stubs = makeStubs(async () => ({})); + wireWorker(rubyAttachPayload(), stubs.processStub, stubs.connectionStub, RubyAdapterPolicy); + (worker as any).dapClient = mockDapClient; + (worker as any).directConnect = { host: '10.0.0.5', port: 12345, listener: LISTENER }; + (worker as any).setupDapEventHandlers(); + (worker as any).state = ProxyState.CONNECTED; + + socketError(stubs)(new Error('read ECONNRESET')); + + expect(errorsSent()).toEqual(['DAP client error: read ECONNRESET']); + }); + + it('a spawned adapter keeps its two reports for a reset during the handshake', async () => { + // Not a direct connection: the socket error and the init failure are + // reported as they always were. + const stubs = makeStubs(async () => { + socketError(stubs)(new Error('read ECONNRESET')); + throw new Error('DAP client disconnected'); + }); + wireWorker(rubyLaunchPayload(), stubs.processStub, stubs.connectionStub, RubyAdapterPolicy); + vi.spyOn(worker as unknown as { shutdown: () => Promise }, 'shutdown').mockResolvedValue(undefined); + + await expect((worker as any).startAdapterAndConnect(rubyLaunchPayload())).rejects.toThrow('DAP client disconnected'); + + const errors = errorsSent() as string[]; + expect(errors).toHaveLength(2); + expect(errors[0]).toBe('DAP client error: read ECONNRESET'); + expect(errors[1]).toMatch(/^Critical initialization error: DAP client disconnected \[Adapter command: rdbg /); + }); }); it('a policy without the opt-in still blocks on a missing initialize response', async () => { diff --git a/tests/unit/proxy/proxy-manager.start.test.ts b/tests/unit/proxy/proxy-manager.start.test.ts index 79fec0038..d6e776ea3 100644 --- a/tests/unit/proxy/proxy-manager.start.test.ts +++ b/tests/unit/proxy/proxy-manager.start.test.ts @@ -599,6 +599,83 @@ describe('ProxyManager.start', () => { } }); + it('names the endpoint of a direct-connect attach that never answered initialize (issue #1139)', async () => { + vi.useFakeTimers(); + const listener = 'a debugpy listener (python -m debugpy --listen 127.0.0.1:5000 …)'; + fakeProcess.sendCommand.mockImplementation((cmd: any) => { + if (cmd.cmd === 'init') { + setTimeout(() => { + const emitStatus = (extra: Record) => + fakeProcess.emit('message', { type: 'status', sessionId: cmd.sessionId, ...extra }); + emitStatus({ status: 'init_received' }); + // No adapter_spawned: nothing was spawned. The worker says what it dialed. + emitStatus({ status: 'direct_connect', host: '127.0.0.1', port: 5000, listener }); + emitStatus({ status: 'dap_handshake_stage', stage: 'transport_connected' }); + emitStatus({ status: 'dap_handshake_stage', stage: 'request_pending', command: 'initialize' }); + }, 0); + } + }); + + const startPromise = proxyManager.start(baseConfig); + + try { + const rejection = expect(startPromise).rejects.toThrow( + /Debug proxy initialization did not complete within 30s\. 127\.0\.0\.1:5000 accepted the connection but never answered the DAP "initialize" request\. Nothing was started for this attach/ + ); + await vi.advanceTimersByTimeAsync(30000); + await vi.runOnlyPendingTimersAsync(); + await Promise.resolve(); + await rejection; + + const error = await startPromise.catch((e: unknown) => e); + expect((error as Error).message).not.toContain('the adapter started'); + expect((error as Error).message).not.toContain('Retrying usually succeeds'); + // The structured facts ride on the error object for the tool result. + expect((error as { initProgress?: unknown }).initProgress).toEqual({ + transportConnected: true, + pendingCommand: 'initialize', + directConnect: { host: '127.0.0.1', port: 5000, listener } + }); + } finally { + vi.useRealTimers(); + } + }); + + it.each([ + ['no host', { host: '', port: 5000 }], + ['a host that is not text', { host: 5000, port: 5000 }], + ['a port that is not a number', { host: '127.0.0.1', port: 'nope' }], + ['neither', { host: '', port: 'nope' }] + ])('ignores a direct_connect status that names no usable endpoint: %s', async (_what, endpoint) => { + vi.useFakeTimers(); + fakeProcess.sendCommand.mockImplementation((cmd: any) => { + if (cmd.cmd === 'init') { + setTimeout(() => { + const emitStatus = (extra: Record) => + fakeProcess.emit('message', { type: 'status', sessionId: cmd.sessionId, ...extra }); + emitStatus({ status: 'init_received' }); + emitStatus({ status: 'direct_connect', ...endpoint }); + emitStatus({ status: 'dap_handshake_stage', stage: 'transport_connected' }); + emitStatus({ status: 'dap_handshake_stage', stage: 'request_pending', command: 'initialize' }); + }, 0); + } + }); + + const startPromise = proxyManager.start(baseConfig); + + try { + const rejection = expect(startPromise).rejects.toThrow(/Connected to the debug adapter, but the "initialize" request never received a response\./); + await vi.advanceTimersByTimeAsync(30000); + await vi.runOnlyPendingTimersAsync(); + await Promise.resolve(); + await rejection; + const error = await startPromise.catch((e: unknown) => e); + expect((error as { initProgress?: unknown }).initProgress).toEqual({ transportConnected: true, pendingCommand: 'initialize' }); + } finally { + vi.useRealTimers(); + } + }); + it('an answered handshake request no longer reads as pending when init later times out (issue #493)', async () => { vi.useFakeTimers(); fakeProcess.sendCommand.mockImplementation((cmd: any) => { diff --git a/tests/unit/shared/adapter-policy-python.test.ts b/tests/unit/shared/adapter-policy-python.test.ts index dd587f55b..7f227fe8f 100644 --- a/tests/unit/shared/adapter-policy-python.test.ts +++ b/tests/unit/shared/adapter-policy-python.test.ts @@ -106,7 +106,15 @@ describe('PythonAdapterPolicy', () => { launchConfig: { request: 'attach', connect: { host: '10.0.0.5', port: 5679 } } }); - expect(config).toEqual({ mode: 'connect', host: '10.0.0.5', port: 5679, logDir: '/logs' }); + // With what should be listening there, for the answer of an attach that + // reaches something else (issue #1139). + expect(config).toEqual({ + mode: 'connect', + host: '10.0.0.5', + port: 5679, + logDir: '/logs', + listener: 'a debugpy listener (python -m debugpy --listen 10.0.0.5:5679 …)' + }); }); it('returns connect mode for attach using top-level host/port', () => { @@ -115,7 +123,13 @@ describe('PythonAdapterPolicy', () => { launchConfig: { request: 'attach', host: '192.168.1.2', port: 5680 } }); - expect(config).toEqual({ mode: 'connect', host: '192.168.1.2', port: 5680, logDir: '/logs' }); + expect(config).toEqual({ + mode: 'connect', + host: '192.168.1.2', + port: 5680, + logDir: '/logs', + listener: 'a debugpy listener (python -m debugpy --listen 192.168.1.2:5680 …)' + }); }); it('defaults the attach host to 127.0.0.1', () => { diff --git a/tests/unit/utils/breakpoint-resolver.test.ts b/tests/unit/utils/breakpoint-resolver.test.ts index 3570ac2c9..a088cf34e 100644 --- a/tests/unit/utils/breakpoint-resolver.test.ts +++ b/tests/unit/utils/breakpoint-resolver.test.ts @@ -54,6 +54,186 @@ describe('assertLineContent', () => { expect(result.message).toContain('statement: "total = sum(prices)"'); }); + describe('the statement hint is what the resolver would answer (issue #1142)', () => { + const SWAP = [ + 'def main():', // 1 + ' a = 1', // 2 + ' b = 2', // 3 + ' print(a, b)', // 4 + ' a, b = b, a # the swap', // 5 + ' print(a, b)', // 6 + ' # a, b = b, a', // 7 + '', // 8 + 'main()', // 9 + ]; + const hintOf = (line: number, expected: string): string => { + const result = assertLineContent(SWAP, line, expected, FILE, { statementHint: true }); + expect(result.ok).toBe(false); + return result.ok ? '' : result.message; + }; + + it('offers no statement for text that is nowhere in the file, and says no line matches', () => { + const message = hintOf(5, 'THIS TEXT IS WRONG'); + + // Following the old hint answered "statement ... not found". + expect(message).not.toContain('statement: "THIS TEXT IS WRONG"'); + expect(message).not.toContain('Or address by content'); + expect(message).toMatch(/Pick the correct line from the context above\. That expectedContent matches no line of the file\.$/); + }); + + describe('with no statement form to offer, it says where the same expectedContent would pass', () => { + // The statement resolver ignores comment lines and whatever follows a + // `//` or `#`; expectedContent does not. "Matches no line" was said for + // the resolver's reasons while the text sat in plain sight a line away. + const afterTheContext = (lines: string[], line: number, expected: string): string => { + const result = assertLineContent(lines, line, expected, FILE, { statementHint: true }); + if (result.ok) throw new Error('expected a mismatch'); + expect(result.message).not.toContain('Or address by content'); + return result.message.slice(result.message.lastIndexOf('Pick the correct line')); + }; + const PICK = 'Pick the correct line from the context above.'; + + it('text that is only in a trailing comment', () => { + const lines = ['def main():', ' a = 1', ' total = a + 1 # BUG HERE', ' return total']; + + expect(afterTheContext(lines, 2, 'BUG HERE')).toBe(`${PICK} That expectedContent matches line 3.`); + }); + + it('part of a line that carries // inside a string', () => { + const lines = ['async function load() {', ' const base = 1;', ' return fetch("http://example.test/api/items");', '}']; + + expect(afterTheContext(lines, 2, 'example.test/api/items')).toBe(`${PICK} That expectedContent matches line 3.`); + }); + + it('part of a line that carries # inside a string', () => { + const lines = ['def label():', ' n = 1', ' return "issue #42 is open"']; + + expect(afterTheContext(lines, 2, '42 is open')).toBe(`${PICK} That expectedContent matches line 3.`); + }); + + it('an expectation that carries a trailing comment the file no longer has', () => { + const lines = ['def main():', ' a = 1', ' total = a + 1', ' return total']; + + expect(afterTheContext(lines, 2, 'total = a + 1 # running sum')).toBe(`${PICK} That expectedContent matches line 3.`); + }); + + it('caps a long list of such lines', () => { + const lines = ['x = 0', ...Array.from({ length: 8 }, (_, index) => `step_${index}() # TODO`)]; + + expect(afterTheContext(lines, 1, 'TODO')).toBe(`${PICK} That expectedContent matches lines 2, 3, 4, 5, 6 and 3 more.`); + }); + + it('lists every such line, a comment-only one included', () => { + const lines = ['x = 1 # TODO', 'y = 2', 'z = 3 # TODO', '# TODO']; + + expect(afterTheContext(lines, 2, 'TODO')).toBe(`${PICK} That expectedContent matches lines 1, 3 and 4.`); + }); + }); + + it('names the line when the text is on exactly one', () => { + const message = hintOf(4, 'a, b = b, a'); + + expect(message).toMatch(/Or address by content: set_breakpoint \{statement: "a, b = b, a"\} — that text is on line 5\.$/); + }); + + it('adds nearLine and lists the lines when the text is on several', () => { + const message = hintOf(5, 'print(a, b)'); + + expect(message).toMatch( + /Or address by content: set_breakpoint \{statement: "print\(a, b\)", nearLine: 5\} — that text is on lines 4 and 6; nearLine picks the closest \(line 4\)\.$/ + ); + }); + + it('offers no statement for an expectation that is a comment, and names the line the comment is on', () => { + const message = hintOf(5, '# a, b = b, a'); + + // A comment is not a statement anchor; the line it sits on still matches. + expect(message).not.toContain('statement'); + expect(message).toMatch(/Pick the correct line from the context above\. That expectedContent matches line 7\.$/); + }); + + it('offers nothing for an expectation that spans lines', () => { + const message = hintOf(5, 'a = 1\n b = 2'); + + expect(message).not.toContain('statement'); + expect(message).toMatch(/Pick the correct line from the context above\.$/); + }); + + it('never names a comment-only line as where the text is', () => { + // Line 7 carries the text inside a comment; line 5 is the statement. + expect(hintOf(2, 'a, b = b, a')).toContain('that text is on line 5.'); + }); + + it('caps a long list of matches', () => { + const many = ['x = 0', ...Array.from({ length: 12 }, () => 'tick()')]; + const result = assertLineContent(many, 1, 'tick()', FILE, { statementHint: true }); + if (result.ok) throw new Error('expected a mismatch'); + + expect(result.message).toContain('that text is on lines 2, 3, 4, 5, 6 and 7 more; nearLine picks the closest (line 2).'); + }); + + it('lists exactly as many matches as the cap in full', () => { + const five = ['x = 0', ...Array.from({ length: 5 }, () => 'tick()')]; + const result = assertLineContent(five, 1, 'tick()', FILE, { statementHint: true }); + if (result.ok) throw new Error('expected a mismatch'); + + expect(result.message).toContain('that text is on lines 2, 3, 4, 5 and 6; nearLine picks the closest (line 2).'); + }); + + it('every hint it gives resolves: fed back to resolveStatement, it lands on the line named', () => { + const expectations = [ + 'a, b = b, a', 'print(a, b)', 'a = 1', 'main()', 'b = 2', 'print', 'THIS TEXT IS WRONG', '# a, b = b, a', 'def', + // Seen by expectedContent and not by the statement resolver: only in + // a trailing comment, and carrying a comment the file does not have. + 'the swap', 'a, b = b, a # stale note', 'b = 2 # stale note' + ]; + let hintsChecked = 0; + let matchListsChecked = 0; + let noLineChecked = 0; + for (let line = 1; line <= SWAP.length; line++) { + for (const expected of expectations) { + const result = assertLineContent(SWAP, line, expected, FILE, { statementHint: true }); + if (result.ok) continue; + const hint = /set_breakpoint \{statement: "(.*?)"(?:, nearLine: (\d+))?\} — that text is on lines? .*?(?:\(line (\d+)\))?\.$/.exec(result.message); + if (!hint) { + // No hint: then the resolver must not find the text either. + const found = resolveStatement(SWAP, expected, FILE, line); + expect(found.ok, `no hint at line ${line} for ${JSON.stringify(expected)}, yet it resolves`).toBe(false); + // And what the message says instead is the check's own verdict on + // every line: the lines it names pass, and "no line" means none does. + const passing = SWAP.map((_, index) => index + 1).filter( + (candidate) => assertLineContent(SWAP, candidate, expected, FILE).ok + ); + const said = / That expectedContent matches (no line of the file|lines? [\d, and]+)\.$/.exec(result.message)?.[1]; + expect(said, result.message).toBeDefined(); + if (said === 'no line of the file') { + expect(passing, result.message).toEqual([]); + noLineChecked++; + } else { + expect((said ?? '').match(/\d+/g)?.map(Number), result.message).toEqual(passing); + expect(passing).not.toContain(line); + matchListsChecked++; + } + continue; + } + hintsChecked++; + const [, statement, nearLine, closest] = hint; + const followed = resolveStatement(SWAP, statement, FILE, nearLine ? Number(nearLine) : undefined); + expect(followed.ok, `${result.message}`).toBe(true); + if (followed.ok) { + const named = closest ?? /that text is on line (\d+)\./.exec(result.message)?.[1]; + expect(String(followed.line), result.message).toBe(named); + // The hint never sends the caller back to the line that just failed. + expect(followed.line).not.toBe(line); + } + } + } + expect(hintsChecked).toBeGreaterThan(10); + expect(matchListsChecked).toBeGreaterThan(10); + expect(noLineChecked).toBeGreaterThan(5); + }); + }); + it('passes when the expectation is a distinctive substring of the line (issue #367)', () => { const result = assertLineContent(LINES, 3, 'sum(prices)', FILE); expect(result.ok).toBe(true); diff --git a/tests/unit/utils/error-messages.test.ts b/tests/unit/utils/error-messages.test.ts index fee6e9245..12f9dcd4a 100644 --- a/tests/unit/utils/error-messages.test.ts +++ b/tests/unit/utils/error-messages.test.ts @@ -339,3 +339,129 @@ describe('the unarmed launch wording when the adapter cannot, or was not asked t expect(text).toMatch(/no pausing breakpoints/); }); }); + +describe('proxy initialization timeout for a direct-connect attach (issue #1139)', () => { + const BASE = 'Debug proxy initialization did not complete within 30s.'; + const LISTENER = 'a debugpy listener (python -m debugpy --listen 127.0.0.1:5000 …)'; + const directConnect = { host: '127.0.0.1', port: 5000, listener: LISTENER }; + + it('names the endpoint and the likely cause when "initialize" was never answered: nothing of ours started', () => { + const message = ErrorMessages.proxyInitTimeout(30, { transportConnected: true, pendingCommand: 'initialize', directConnect }); + + expect(message).toBe( + `${BASE} 127.0.0.1:5000 accepted the connection but never answered the DAP "initialize" request. ` + + 'Nothing was started for this attach: the server connected to what is already listening on that port, ' + + `so check that it is ${LISTENER} and not another service. Retrying the same port stalls the same way.` + ); + // What the spawned-adapter wording said, and what is false here. + expect(message).not.toContain('the adapter started'); + expect(message).not.toContain('Retrying usually succeeds'); + expect(message).not.toContain('missing install'); + }); + + it('stays general when the policy has no phrase for what should be listening', () => { + const message = ErrorMessages.proxyInitTimeout(30, { + transportConnected: true, + pendingCommand: 'initialize', + directConnect: { host: '10.0.0.5', port: 12345 } + }); + + expect(message).toContain('10.0.0.5:12345 accepted the connection but never answered the DAP "initialize" request.'); + expect(message).toContain("so check that it is the debugger's own listener and not another service."); + }); + + it('does not blame the port once the endpoint has answered "initialize": it does speak DAP', () => { + const message = ErrorMessages.proxyInitTimeout(30, { transportConnected: true, pendingCommand: 'attach', directConnect }); + + expect(message).toBe( + `${BASE} 127.0.0.1:5000 answered the start of the DAP handshake, but the "attach" request never received a response. ` + + 'Nothing was started for this attach: this is a stall in the debug server listening on that port, not a missing install. ' + + 'If it recurs, capture a DAP trace (DAP_TRACE=1) of the failing attach.' + ); + expect(message).not.toContain('not another service'); + expect(message).not.toContain('the adapter started'); + }); + + it('names the endpoint when the handshake began and nothing is pending', () => { + const message = ErrorMessages.proxyInitTimeout(30, { transportConnected: true, directConnect }); + + expect(message).toBe( + `${BASE} Connected to 127.0.0.1:5000 and the DAP handshake began, but initialization stalled before completing. ` + + 'Nothing was started for this attach: the debug server listening on that port is what stalled.' + ); + }); + + it('never tells a direct-connect caller to install an adapter when the connection was never made', () => { + const message = ErrorMessages.proxyInitTimeout(30, { transportConnected: false, directConnect }); + + expect(message).toBe( + `${BASE} No connection to 127.0.0.1:5000 was established. Nothing was started for this attach: ` + + `check the host and port, and that ${LISTENER} is listening there and reachable from this machine.` + ); + expect(message).not.toContain('installed and accessible'); + }); + + it('names a plain debug server there when the policy has no phrase for what should be listening', () => { + const message = ErrorMessages.proxyInitTimeout(30, { + transportConnected: false, + directConnect: { host: '10.0.0.5', port: 12345 } + }); + + expect(message).toBe( + `${BASE} No connection to 10.0.0.5:12345 was established. Nothing was started for this attach: ` + + 'check the host and port, and that the debug server is listening there and reachable from this machine.' + ); + }); + + it('leaves every spawned-adapter sentence as it was', () => { + expect(ErrorMessages.proxyInitTimeout(30, { transportConnected: true, pendingCommand: 'initialize', adapterPid: 52875 })).toBe( + `${BASE} Connected to the debug adapter, but the "initialize" request never received a response. ` + + 'The adapter process is running (PID 52875). This is an adapter-side protocol stall, not a missing install — ' + + 'the adapter started and accepted the connection. Retrying usually succeeds; ' + + 'if it recurs, capture a DAP trace (DAP_TRACE=1) of the failing launch.' + ); + expect(ErrorMessages.proxyInitTimeout(30, { transportConnected: true, adapterPid: 7 })).toBe( + `${BASE} Connected to the debug adapter and the DAP handshake began, but initialization stalled before completing. ` + + 'The adapter process is running (PID 7).' + ); + expect(ErrorMessages.proxyInitTimeout(30, { transportConnected: false, adapterPid: 7 })).toBe( + `${BASE} The adapter process spawned (PID 7) but the DAP connection was never established. ` + + "Check that the adapter's port is reachable and nothing is blocking loopback TCP connections." + ); + expect(ErrorMessages.proxyInitTimeout(30, { transportConnected: false })).toBe( + `${BASE} This may indicate that the debug adapter failed to start or is not properly configured. ` + + 'Check that the required debug adapter is installed and accessible.' + ); + }); +}); + +describe('stopWithoutProgramFrame offers the frames only when some are unseen (issue #1141)', () => { + const top = { name: 'process.processTimers', file: '/internal/timers', line: 527 }; + const HINT = 'get_stack_trace with includeInternals: true lists the frames'; + + it('ends at the remedy when the answer has shown every frame', () => { + const message = ErrorMessages.stopWithoutProgramFrame(0, 1, top); + expect(message).toBe( + "No program frame is active on thread 0: the only frame is internal/runtime (top: 'process.processTimers' /internal/timers:527). " + + 'The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint.' + ); + expect(message).not.toContain(HINT); + }); + + it('points at the full stack when the thread has frames the answer did not show', () => { + const message = ErrorMessages.stopWithoutProgramFrame( + 12952, + 4, + { name: '__tmainCRTStartup', file: '@__tmainCRTStartup', line: 51 }, + { framesUnseen: true } + ); + expect(message).toBe( + "No program frame is active on thread 12952: all 4 frames are internal/runtime (top: '__tmainCRTStartup' @__tmainCRTStartup:51). " + + `The stop landed outside your code: continue_execution and trigger activity, or set a breakpoint; ${HINT}.` + ); + }); + + it('names no thread when none is known', () => { + expect(ErrorMessages.stopWithoutProgramFrame(undefined, 1, top)).toMatch(/^No program frame is active: the only frame is/); + }); +});