From 8296f7c03c98b1ea33a640e57235d3de9e8841b0 Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 17 Sep 2026 11:22:42 -0400 Subject: [PATCH 1/7] fix(session): a session whose launch runs with the debugger off says so on every later surface (#749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #747 decided "this noDebug launch runs with the debugger off" once, for the start_debugging response, and forgot it. Nothing on the session recorded it, so set_breakpoint on the running session came back verified:false with no reason, list_breakpoints showed everything unbound with none, pause_execution ended in the #678 "may be blocked in native code" guess, and stepping, get_stack_trace, get_local_variables and evaluate_expression answered "not paused" in debugger terms. Record: `DebugSessionInfo.debuggerDisabled` (inherited by ManagedSession), written in the launcher where debuggerOff is decided (real launches, not dry runs), reset in the launcher's and the attach controller's per-attempt blocks — not in the core's setupProxyEventHandlers, which runs inside proxyLauncher.start after the launcher wrote it — and cleared in handleStopped: a real stop is stronger evidence than the policy's pin. Projected by SessionStore.getAll() and list_debug_sessions. Consult, never pre-empting the adapter: every request still goes to the adapter and its own answer is kept; one sentence (ErrorMessages.debuggerOffForLaunch) is added beside it — set_breakpoint (unverified only), list_breakpoints (top-level warning), pause pending (instead of the policy's guess) and pause refused (appended), "Not paused:" on step/continue, and the stack-trace note, evaluate error and locals message. Measured: js-debug still lands a pause under noDebug, so the sentence says "no stop is expected" rather than "cannot come", and that stop clears the flag. Tests: field lifecycle in session-manager-nodebug-warning.test.ts, both projections, every consumer, and a js e2e over pause_test.js (run locally — CI does not run the e2e project). Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/749.fixed.md | 1 + docs/tool-reference.md | 4 +- packages/shared/src/models/index.ts | 10 ++ src/server/handlers/breakpoint-tools.ts | 28 +++++- src/server/handlers/inspection-tools.ts | 10 +- src/server/handlers/session-tools.ts | 4 + src/session/attach/attach-controller.ts | 2 + src/session/execution/execution-controller.ts | 27 +++++- .../inspection/expression-evaluator.ts | 6 +- .../inspection/frame-anchor-resolver.ts | 5 +- src/session/launch/debug-launcher.ts | 11 +++ src/session/session-manager-core.ts | 4 + src/session/session-store.ts | 2 + src/utils/error-messages.ts | 16 +++ .../server/handlers/inspection-tools.test.ts | 22 +++++ .../server/handlers/session-tools.test.ts | 15 +++ ...server-breakpoint-management-tools.test.ts | 26 +++++ .../unit/server/server-control-tools.test.ts | 46 +++++++++ .../session-manager-nodebug-warning.test.ts | 95 ++++++++++++++++++ .../session/session-store-projection.test.ts | 13 +++ .../mcp-server-break-on-exceptions.test.ts | 80 +++++++++++++++ ...ession-manager-operations-coverage.test.ts | 97 +++++++++++++++++++ 22 files changed, 511 insertions(+), 13 deletions(-) create mode 100644 changelog.d/749.fixed.md diff --git a/changelog.d/749.fixed.md b/changelog.d/749.fixed.md new file mode 100644 index 000000000..fed508c5b --- /dev/null +++ b/changelog.d/749.fixed.md @@ -0,0 +1 @@ +**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` — reset per launch and attach, and cleared by any `stopped` event that arrives anyway (js-debug lands a pause under the flag; a real stop proves the debugger on). Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal); the recorded fact adds the why beside it: "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" (#749) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index d3d3f18ca..9aa4d3b1d 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -120,6 +120,8 @@ Lists all active debugging sessions. **`lastStop`:** present while the session is `paused` (the stop it is at: `reason`, `threadId`, `timestamp`, the adapter's `description`/`text`, and `exceptionInfo` for exception stops) and after it reaches `stopped`/`error` (the last stop before it ended). A `running` session never carries one, so a poller that calls this after `continue_execution` or a step sees `state: "running"` with no stop record until the next stop lands — the record of the stop it just left is not repeated as if the program were still paused. +**`debuggerDisabled`:** present (`true`) while the current launch runs with the debugger off — `dapLaunchArgs.noDebug: true` on an adapter that honours it (see `start_debugging`). Cleared by the next launch or attach, and by any `stopped` event that arrives anyway (issue #749). + Errored sessions include optional `diagnostics` with the current launch attempt's server-host `proxyLogPath` and remote-safe `proxyLogResource`. The record is retained for proxy initialization failures and for proxy/adapter deaths after initialization, and is cleared when a new launch or attach attempt begins. --- @@ -366,7 +368,7 @@ Starts debugging a script. - `dapLaunchArgs` (object, optional): Standard DAP launch arguments: - `stopOnEntry` (boolean): Stop at first line (default `false` — the opposite of attach, which pauses unless `stopOnEntry` is `false`) - `justMyCode` (boolean): Debug only user code (default `true`). JavaScript launch: `true` blackboxes `node_modules` through js-debug's `skipFiles` and keeps js-debug's smart-stepper on, so a pause or step that lands in skipped code (Node internals, `node_modules`) is stepped through and may never land (`pending: true`, with an explanation); `false` drops `node_modules` from the skip list and turns the stepper off, so steps land inside dependencies and `pause_execution` lands as soon as any JavaScript runs (issue #678). A caller `skipFiles` replaces the default list. Source maps are on for every JavaScript launch, `.js` programs included — stops report `src/*.ts` when maps and sources are present; `adapterLaunchConfig: { sourceMaps: false }` opts out (issue #684) - - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off and names what will not fire, in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. + - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off and names what will not fire, in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint") and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event that arrives anyway — js-debug lands a pause under the flag — proves the debugger on and clears it (issue #749). - Additional DAP launch keys (`program`, `cwd`, `env`, language-specific options) pass through to the adapter. Top-level parameters do **not** belong here: a nested `breakOnExceptions` is honored as an alias (the top-level value wins if both are given) and reported via a `warning` in the response; other misplaced top-level keys (`dryRunSpawn`, `sessionId`, `scriptPath`, `adapterLaunchConfig`) are stripped with a warning instead of silently riding into the launch config. - `adapterLaunchConfig` (object, optional): Adapter-specific launch configuration overrides. Use this for language-specific settings that go beyond standard DAP arguments (e.g., `mainClass` and `classpath` for Java, `buildCommand` for Rust). For Rust, `_adapterSettings` passes through to CodeLLDB (issue #441) — e.g. `{"_adapterSettings": {"scriptConfig": {"lang": {"rust": {"sysroot": "/path"}}}}}` points the Rust formatter lookup at an explicit sysroot; the `CODELLDB_RUST_SYSROOT` env var does the same without per-launch config (a user-supplied `_adapterSettings` value wins over the env var). - `dryRunSpawn` (boolean, optional): Test spawn without actually starting diff --git a/packages/shared/src/models/index.ts b/packages/shared/src/models/index.ts index 2899fad29..8f2d1034d 100644 --- a/packages/shared/src/models/index.ts +++ b/packages/shared/src/models/index.ts @@ -432,6 +432,16 @@ export interface DebugSessionInfo { exitCode?: number; /** Present when the session is in ERROR because its proxy failed. */ diagnostics?: SessionFailureDiagnostics; + /** + * True while the current launch runs with the debugger off: the launch + * carried `noDebug: true` and the adapter honours it (issue #710), so no + * breakpoint can bind and no stop is expected. Set by the launcher, reset + * per launch and attach, and cleared by the first `stopped` event that + * arrives anyway — a real stop proves the debugger on for this adapter + * build. Later surfaces (set_breakpoint, list_breakpoints, pause, + * stepping, inspection) read it to say why (issue #749). + */ + debuggerDisabled?: boolean; /** * Live DAP mirror endpoint from expose_session (issue #217), host/port * only — the attach token is returned solely by the expose_session tool. diff --git a/src/server/handlers/breakpoint-tools.ts b/src/server/handlers/breakpoint-tools.ts index 5ea66a110..4f73b23fc 100644 --- a/src/server/handlers/breakpoint-tools.ts +++ b/src/server/handlers/breakpoint-tools.ts @@ -14,8 +14,20 @@ import type { FunctionBreakpointRemoval } from '../../session/session-manager-op import type { ToolContext, ToolHandler } from '../tool-context.js'; import { requireSessionId, type WithSessionId } from '../tool-validation.js'; import { readLineContext } from './shared.js'; +import { ErrorMessages } from '../../utils/error-messages.js'; import { failureResult, jsonResult, sessionErrorResultOrThrow, type ToolResult } from '../tool-result.js'; +/** + * The why beside an unverified answer while the session's launch runs with + * the debugger off (issue #749). The request still went to the adapter and + * its own answer is kept; a breakpoint it verified anyway needs no note. + */ +function debuggerOffNote(ctx: ToolContext, sessionId: string, verified: boolean): string | undefined { + return !verified && ctx.sessionManager.getSession(sessionId)?.debuggerDisabled + ? ErrorMessages.debuggerOffForLaunch + : undefined; +} + export const setBreakpointTool: ToolHandler = async (ctx, args) => { const isFunctionBp = args.function !== undefined; if (!isFunctionBp && (!args.file || (args.line === undefined && args.statement === undefined))) { @@ -105,7 +117,10 @@ async function setFunctionBreakpointBranch(ctx: ToolContext, args: WithSessionId timestamp: Date.now() }); - const warnings = [breakpoint.message, fnGate.warning, normalized?.note, nameHint, syncWarning].filter(Boolean); + const warnings = [ + breakpoint.message, fnGate.warning, normalized?.note, nameHint, syncWarning, + debuggerOffNote(ctx, args.sessionId, breakpoint.verified) + ].filter(Boolean); return jsonResult({ success: true, breakpointId: breakpoint.id, @@ -177,7 +192,10 @@ async function setLineBreakpointBranch(ctx: ToolContext, args: WithSessionId): P }` : undefined; - const warnings = [breakpoint.message, logPointGate.warning, syncWarning, snapWarning].filter(Boolean); + const warnings = [ + breakpoint.message, logPointGate.warning, syncWarning, snapWarning, + debuggerOffNote(ctx, args.sessionId, breakpoint.verified) + ].filter(Boolean); const result: ToolResult = jsonResult({ success: true, breakpointId: breakpoint.id, @@ -226,13 +244,17 @@ export const listBreakpointsTool: ToolHandler = async (ctx, args) => { const functionBreakpoints = args.file === undefined ? ctx.sessionManager.listFunctionBreakpoints(args.sessionId) : []; + // Per-breakpoint records carry the adapter's own answers; the one reason + // none of them can bind right now goes on the response (issue #749). + const debuggerOff = ctx.sessionManager.getSession(args.sessionId)?.debuggerDisabled === true; return jsonResult({ success: true, breakpoints, count: breakpoints.length, ...(args.file === undefined ? { functionBreakpoints, functionCount: functionBreakpoints.length } - : {}) + : {}), + ...(debuggerOff ? { warning: ErrorMessages.debuggerOffForLaunch } : {}) }); } catch (error) { return sessionErrorResultOrThrow(error); diff --git a/src/server/handlers/inspection-tools.ts b/src/server/handlers/inspection-tools.ts index 6173d84fe..0a1d347c4 100644 --- a/src/server/handlers/inspection-tools.ts +++ b/src/server/handlers/inspection-tools.ts @@ -5,6 +5,7 @@ import { ErrorCode as McpErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; 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'; import { carriesLastStop, variablePayloadExtras } from './shared.js'; @@ -334,12 +335,15 @@ export async function handleGetLocalVariables(ctx: ToolContext, args: { sessionI // Distinguish "not paused" from "paused but the anchored thread has // no frames" — the latter used to claim the debugger may not be // paused while list_debug_sessions said paused (issue #465). - const sessionState = ctx.sessionManager.getSession(args.sessionId)?.state; - response.message = sessionState === SessionState.PAUSED + const sessionNow = ctx.sessionManager.getSession(args.sessionId); + response.message = sessionNow?.state === SessionState.PAUSED ? 'The session is paused, but the anchored thread reported no stack frames. ' + 'Try get_stack_trace with a threadId from list_threads, or continue_execution ' + 'followed by pause_execution to re-anchor on a reportable thread.' - : 'No stack frames available. The debugger may not be paused.'; + : sessionNow?.debuggerDisabled + // The why, when the launch runs with the debugger off (issue #749). + ? `No stack frames available; ${ErrorMessages.debuggerOffForLaunch}.` + : 'No stack frames available. The debugger may not be paused.'; } else if (!result.scopeName) { response.message = 'No local scope found in the current frame.'; } else { diff --git a/src/server/handlers/session-tools.ts b/src/server/handlers/session-tools.ts index b691475a2..0fbfdb9ca 100644 --- a/src/server/handlers/session-tools.ts +++ b/src/server/handlers/session-tools.ts @@ -175,6 +175,10 @@ export async function handleListDebugSessions(ctx: ToolContext): Promise): string { + return session.debuggerDisabled + ? `Not paused: ${ErrorMessages.debuggerOffForLaunch}` + : 'Not paused'; +} + export class ExecutionController { constructor( private readonly ctx: ExecutionContext, @@ -177,7 +187,7 @@ export class ExecutionController { } if (session.state !== SessionState.PAUSED) { this.ctx.logger.warn(`[SM ${logTag} ${sessionId}] Not paused. State: ${session.state}`); - return { success: false, error: 'Not paused', state: session.state }; + return { success: false, error: notPausedError(session), state: session.state }; } if (typeof threadId !== 'number') { this.ctx.logger.warn(`[SM ${logTag} ${sessionId}] No current thread ID.`); @@ -201,7 +211,7 @@ export class ExecutionController { } if (session.state !== SessionState.PAUSED) { this.ctx.logger.warn(`[SM ${logTag} ${sessionId}] No longer paused after the origin read. State: ${session.state}`); - return { success: false, error: 'Not paused', state: session.state }; + return { success: false, error: notPausedError(session), state: session.state }; } this.ctx.logger.info(`[SM ${logTag} ${sessionId}] Sending DAP '${command}' for threadId ${threadId}`); @@ -429,7 +439,7 @@ export class ExecutionController { this.ctx.logger.warn( `[SessionManager continue] Session ${sessionId} not paused. State: ${session.state}.` ); - return { success: false, error: 'Not paused', state: session.state }; + return { success: false, error: notPausedError(session), state: session.state }; } if (typeof threadId !== 'number') { this.ctx.logger.warn( @@ -622,7 +632,11 @@ export class ExecutionController { `[SessionManager pause] No stopped event within ${this.ctx.tunables.pauseGraceMs}ms grace window in session ${sessionId}; completing asynchronously` ); const pausePending = ErrorMessages.pausePending(this.ctx.tunables.pauseGraceMs / 1000); - const hint = await this.describePendingStop(session, sessionId, 'pause'); + // With the debugger off for this launch the why is known (issue #749); + // the policy's guess at native code or a syscall would be wrong. + const hint = session.debuggerDisabled + ? ErrorMessages.debuggerOffForLaunch + : await this.describePendingStop(session, sessionId, 'pause'); return { success: true, state: session.state, @@ -648,6 +662,11 @@ export class ExecutionController { if (errorMessage.includes(NO_DEBUG_TARGET_MARKER)) { return { success: false, error: errorMessage, state: session.state }; } + if (session.debuggerDisabled) { + // The adapter refused the pause (CodeLLDB under noDebug): its answer + // stands, with the why beside it (issue #749). + throw new Error(`${errorMessage} (${ErrorMessages.debuggerOffForLaunch})`); + } throw outcome.error instanceof Error ? outcome.error : new Error(errorMessage); } diff --git a/src/session/inspection/expression-evaluator.ts b/src/session/inspection/expression-evaluator.ts index d85126ed0..f83ef295d 100644 --- a/src/session/inspection/expression-evaluator.ts +++ b/src/session/inspection/expression-evaluator.ts @@ -8,6 +8,7 @@ * hook (issue #237) runs before anything, including the logs, sees the result. */ import { getErrorMessage } from '../../errors/debug-errors.js'; +import { ErrorMessages } from '../../utils/error-messages.js'; import { buildRedactionNotice, isSensitiveName, @@ -133,7 +134,10 @@ export class ExpressionEvaluator { ); return { success: false, - error: 'Cannot evaluate: debugger not paused. Ensure the debugger is stopped at a breakpoint.', + error: session.debuggerDisabled + // The why, when the launch runs with the debugger off (issue #749). + ? `Cannot evaluate: debugger not paused (${ErrorMessages.debuggerOffForLaunch})` + : 'Cannot evaluate: debugger not paused. Ensure the debugger is stopped at a breakpoint.', }; } diff --git a/src/session/inspection/frame-anchor-resolver.ts b/src/session/inspection/frame-anchor-resolver.ts index 2ffc6c1b4..e546eef33 100644 --- a/src/session/inspection/frame-anchor-resolver.ts +++ b/src/session/inspection/frame-anchor-resolver.ts @@ -18,6 +18,7 @@ import type { DebugProtocol } from '@vscode/debugprotocol'; import path from 'path'; import type { IProxyManager } from '../../proxy/proxy-manager.js'; import type { ManagedSession } from '../session-store.js'; +import { ErrorMessages } from '../../utils/error-messages.js'; /** The frame fields a tool response names when it says which frame answered. */ export type FrameSummary = Pick; @@ -135,8 +136,10 @@ export class FrameAnchorResolver { } if (session.state !== SessionState.PAUSED) { this.ctx.logger.warn(`[FrameAnchor ${sessionId}] Session not paused: ${session.state}.`); + // The why, when the launch runs with the debugger off (issue #749). + const why = session.debuggerDisabled ? `; ${ErrorMessages.debuggerOffForLaunch}` : ''; return emptyResult( - `Session is not paused (state: ${session.state}); stack traces are only available while paused.`, + `Session is not paused (state: ${session.state}); stack traces are only available while paused${why}.`, threadId ); } diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index 234352f23..ce617ccb0 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -242,6 +242,9 @@ export class DebugLauncher { session.lastProxyError = undefined; session.failureDiagnostics = undefined; session.lastStop = undefined; + // The previous launch's debugger-off decision does not carry over + // (issue #749); this attempt decides again below. + session.debuggerDisabled = undefined; this.ctx.logger.info(`[SessionManager] Session ${sessionId} lifecycle state set to ACTIVE`); // Record the launch spec for restart_debugging BEFORE attempting the @@ -275,6 +278,14 @@ export class DebugLauncher { const noDebug = !isAttachShaped && resolveLaunchFlag('noDebug', dapLaunchArgs, adapterLaunchConfig); const honoursNoDebug = policy.honoursNoDebug === true; const debuggerOff = noDebug && honoursNoDebug; + // Recorded on the session so the surfaces after this response can say + // why they answer in non-debugger terms (issue #749). Not for a dry run: + // nothing launches. Written before the proxy starts, so the core's + // per-launch reset (which runs inside proxyLauncher.start) is not the + // place to clear it — the block above is. + if (debuggerOff && !dryRunSpawn) { + session.debuggerDisabled = true; + } const noDebugWarning = buildNoDebugLaunchWarning( session, { noDebug, stopOnEntry: resolveLaunchFlag('stopOnEntry', dapLaunchArgs, adapterLaunchConfig) }, diff --git a/src/session/session-manager-core.ts b/src/session/session-manager-core.ts index ef97aa1b2..ed30e1424 100644 --- a/src/session/session-manager-core.ts +++ b/src/session/session-manager-core.ts @@ -736,6 +736,10 @@ export abstract class SessionManagerCore extends EventEmitter { } session.firstStopHandled = true; + // A stop is stronger evidence than the policy's noDebug pin: this + // adapter build debugs after all, so the later surfaces must stop + // explaining themselves in debugger-off terms (issue #749). + session.debuggerDisabled = undefined; }; proxyManager.on('stopped', handleStopped); handlers.set('stopped', handleStopped); diff --git a/src/session/session-store.ts b/src/session/session-store.ts index d76eba49b..bae5ccc34 100644 --- a/src/session/session-store.ts +++ b/src/session/session-store.ts @@ -289,6 +289,8 @@ export class SessionStore { ...(s.state === SessionState.ERROR && s.failureDiagnostics ? { diagnostics: s.failureDiagnostics } : {}), + // The current launch runs with the debugger off (issue #749). + ...(s.debuggerDisabled ? { debuggerDisabled: true } : {}), // Mirror endpoint without the token (issue #217); the isRunning gate // keeps the projection honest on teardown paths that skip cleanup. ...(s.exposure && s.proxyManager?.isRunning() diff --git a/src/utils/error-messages.ts b/src/utils/error-messages.ts index 0e6b52168..505987a4b 100644 --- a/src/utils/error-messages.ts +++ b/src/utils/error-messages.ts @@ -151,6 +151,22 @@ export const ErrorMessages = { `'paused' once the stop lands. Check the session state to confirm.`, + /** + * The why a session whose current launch runs with the debugger off appends + * to every answer that would otherwise read in debugger terms (issue #749): + * `dapLaunchArgs.noDebug` on an adapter that honours it (issue #710) — + * an unverified breakpoint, a pause that never lands, "not paused" from + * stepping and inspection. Names the fact and the remedy, never the + * adapter's own answer, which stays as it came. "No stop is expected" + * rather than "cannot come": js-debug still lands a pause under the flag. + * Used in: src/server/handlers/breakpoint-tools.ts, src/server/handlers/inspection-tools.ts, + * src/session/execution/execution-controller.ts, src/session/inspection/frame-anchor-resolver.ts, + * src/session/inspection/expression-evaluator.ts + */ + debuggerOffForLaunch: + 'the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; ' + + 'drop noDebug and launch again to debug', + /** * Suffix appended to the attach message when the post-attach pause was * requested (explicitly or by the default) but no 'stopped' event arrived diff --git a/tests/core/unit/server/handlers/inspection-tools.test.ts b/tests/core/unit/server/handlers/inspection-tools.test.ts index cb862f926..8ec038333 100644 --- a/tests/core/unit/server/handlers/inspection-tools.test.ts +++ b/tests/core/unit/server/handlers/inspection-tools.test.ts @@ -13,6 +13,7 @@ import { handleGetLocalVariables } from '../../../../../src/server/handlers/inspection-tools.js'; import { SessionTerminatedError } from '../../../../../src/errors/debug-errors.js'; +import { ErrorMessages } from '../../../../../src/utils/error-messages.js'; import { createMockToolContext } from '../server-test-helpers.js'; // DebugMcpServer builds its dependencies in the constructor; mock the container @@ -221,6 +222,27 @@ describe('inspection tool handlers', () => { expect(payload.message).toContain('No stack frames available'); }); + it('says why no frame is available while the launch runs with the debugger off (issue #749)', async () => { + ctx.sessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'running', + sessionLifecycle: 'ACTIVE', + debuggerDisabled: true + }); + ctx.sessionManager.getLocalVariables.mockResolvedValue({ + variables: [], + frame: null, + scopeName: null + }); + + const result = await handleGetLocalVariables(ctx, { sessionId: 'test-session' }); + const payload = JSON.parse(result.content[0].text); + + expect(payload.success).toBe(true); + expect(payload.message).toContain('No stack frames available'); + expect(payload.message).toContain(ErrorMessages.debuggerOffForLaunch); + }); + it('shows "no local scope" message when frame exists but no scope', async () => { ctx.sessionManager.getLocalVariables.mockResolvedValue({ variables: [], diff --git a/tests/core/unit/server/handlers/session-tools.test.ts b/tests/core/unit/server/handlers/session-tools.test.ts index 28f9c57c6..2bd3e821b 100644 --- a/tests/core/unit/server/handlers/session-tools.test.ts +++ b/tests/core/unit/server/handlers/session-tools.test.ts @@ -76,5 +76,20 @@ describe('session tool handlers', () => { expect(byId.stopped.lastStop).toMatchObject({ reason: 'breakpoint' }); expect(byId.errored.lastStop).toMatchObject({ reason: 'exception' }); }); + + it('reports debuggerDisabled for a launch running with the debugger off (issue #749)', async () => { + const now = new Date(); + ctx.sessionManager.getAllSessions.mockReturnValue([ + { id: 'off', name: 'o', language: 'python', state: 'running', createdAt: now, debuggerDisabled: true }, + { id: 'on', name: 'n', language: 'python', state: 'running', createdAt: now } + ]); + + const result = await handleListDebugSessions(ctx); + const payload = JSON.parse(result.content[0].text); + const byId = Object.fromEntries(payload.sessions.map((s: { id: string }) => [s.id, s])); + + expect(byId.off.debuggerDisabled).toBe(true); + expect(byId.on).not.toHaveProperty('debuggerDisabled'); + }); }); }); diff --git a/tests/core/unit/server/server-breakpoint-management-tools.test.ts b/tests/core/unit/server/server-breakpoint-management-tools.test.ts index 3ddcca4c7..06f62236b 100644 --- a/tests/core/unit/server/server-breakpoint-management-tools.test.ts +++ b/tests/core/unit/server/server-breakpoint-management-tools.test.ts @@ -3,6 +3,7 @@ * list_breakpoints / remove_breakpoint / clear_breakpoints */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { ErrorMessages } from '../../../../src/utils/error-messages.js'; import { Server } from '@modelcontextprotocol/sdk/server/index.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { DebugMcpServer } from '../../../../src/server.js'; @@ -93,6 +94,31 @@ describe('Server Breakpoint Management Tools', () => { expect(mockSessionManager.listBreakpoints).toHaveBeenCalledWith('test-session', undefined); }); + it('says why none of the breakpoints can bind while the launch runs with the debugger off (issue #749)', async () => { + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'running', + sessionLifecycle: 'ACTIVE', + debuggerDisabled: true + }); + mockSessionManager.listBreakpoints.mockReturnValue([ + { id: 'bp-1', file: '/a.py', line: 10, verified: false, message: 'Unbound breakpoint' } + ]); + mockSessionManager.listFunctionBreakpoints.mockReturnValue([]); + + const result = await callToolHandler({ + method: 'tools/call', + params: { name: 'list_breakpoints', arguments: { sessionId: 'test-session' } } + }); + + const content = JSON.parse(result.content[0].text); + expect(content.success).toBe(true); + // The adapter's own per-breakpoint answer is untouched. + expect(content.breakpoints[0].message).toBe('Unbound breakpoint'); + expect(content.warning).toMatch(/debugger is off for this launch/); + expect(content.warning).toBe(ErrorMessages.debuggerOffForLaunch); + }); + it('always includes empty function-breakpoint fields in the unfiltered response (#306)', async () => { mockSessionManager.listBreakpoints.mockReturnValue([]); mockSessionManager.listFunctionBreakpoints.mockReturnValue([]); diff --git a/tests/core/unit/server/server-control-tools.test.ts b/tests/core/unit/server/server-control-tools.test.ts index 3ceb9016b..9c40d9641 100644 --- a/tests/core/unit/server/server-control-tools.test.ts +++ b/tests/core/unit/server/server-control-tools.test.ts @@ -54,6 +54,52 @@ describe('Server Control Tools Tests', () => { }); describe('set_breakpoint', () => { + it('appends why an unverified breakpoint cannot bind while the launch runs with the debugger off (issue #749)', async () => { + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'running', + sessionLifecycle: 'ACTIVE', + debuggerDisabled: true + }); + // The request still went to the adapter; its own answer is kept. + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 10, verified: false, message: 'Unbound breakpoint' } + }); + + const result = await callToolHandler({ + method: 'tools/call', + params: { name: 'set_breakpoint', arguments: { sessionId: 'test-session', file: '/path/to/test.py', line: 10 } } + }); + + expect(mockSessionManager.setBreakpoint).toHaveBeenCalled(); + const content = JSON.parse(result.content[0].text); + expect(content.success).toBe(true); + expect(content.verified).toBe(false); + expect(content.warning).toContain('Unbound breakpoint'); + expect(content.warning).toContain(ErrorMessages.debuggerOffForLaunch); + }); + + it('adds no debugger-off note to a breakpoint the adapter verified anyway (issue #749)', async () => { + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'running', + sessionLifecycle: 'ACTIVE', + debuggerDisabled: true + }); + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 10, verified: true } + }); + + const result = await callToolHandler({ + method: 'tools/call', + params: { name: 'set_breakpoint', arguments: { sessionId: 'test-session', file: '/path/to/test.py', line: 10 } } + }); + + const content = JSON.parse(result.content[0].text); + expect(content.verified).toBe(true); + expect(content.warning).toBeUndefined(); + }); + it('should set breakpoint successfully', async () => { const mockBreakpoint: Breakpoint = { id: 'bp-1', diff --git a/tests/core/unit/session/session-manager-nodebug-warning.test.ts b/tests/core/unit/session/session-manager-nodebug-warning.test.ts index 77e2e71bd..455d0a4d7 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -429,4 +429,99 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(warningOf(result)).toBeUndefined(); }); }); + + /** + * The decision outlives the launch response (issue #749): later surfaces — + * set_breakpoint, list_breakpoints, pause, inspection — read it off the + * session to say why they answer the way they do. + */ + describe('records the decision on the session (issue #749)', () => { + it('sets debuggerDisabled on the session for an honoured noDebug launch', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + + await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + }); + + it('leaves it unset where the adapter ignores the flag, and for a launch without it', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + + await launch(s.id, { stopOnEntry: false, noDebug: true }); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + + pinPolicy({ honoursNoDebug: true }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false }); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + }); + + it('clears it on the next launch without the flag', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + + dependencies.mockProxyManager.simulateEvent('terminated'); + await vi.runAllTimersAsync(); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false }); + + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + }); + + it('does not set it for a dry run — nothing launched', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + const proxy = dependencies.mockProxyManager; + proxy.start = vi.fn().mockImplementation(async (startConfig) => { + proxy.startCalls.push(startConfig); + process.nextTick(() => proxy.emit('dry-run-complete', 'python app.py', '/work/src/app.py')); + }) as MockProxyManager['start']; + + const startPromise = sessionManager.startDebugging(s.id, '/work/src/app.py', [], { stopOnEntry: false, noDebug: true }, true); + await vi.runAllTimersAsync(); + const result = await startPromise; + + expect((result.data as { dryRun?: boolean }).dryRun).toBe(true); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + }); + + it('is cleared by a stop that arrives anyway — the adapter proved the debugger on', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + + // A later pause lands (js-debug does this under noDebug: measured). + dependencies.mockProxyManager.simulateEvent('stopped', 1, 'pause', { reason: 'pause', threadId: 1 }); + await vi.runAllTimersAsync(); + + expect(sessionManager.getSession(s.id)?.state).toBe(SessionState.PAUSED); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + }); + + it('recomputes it on restart_debugging, which replays the same arguments', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + dependencies.mockProxyManager.simulateEvent('terminated'); + await vi.runAllTimersAsync(); + // A stop-free termination leaves the flag; restart resets and re-decides. + runWithoutStopping(); + + const restartPromise = sessionManager.restartDebugging(s.id); + await vi.runAllTimersAsync(); + const result = await restartPromise; + + expect(result.success).toBe(true); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + }); + }); }); diff --git a/tests/core/unit/session/session-store-projection.test.ts b/tests/core/unit/session/session-store-projection.test.ts index d2f4142da..17676c3bf 100644 --- a/tests/core/unit/session/session-store-projection.test.ts +++ b/tests/core/unit/session/session-store-projection.test.ts @@ -52,3 +52,16 @@ describe('SessionStore.getAll() lastStop projection (issue #720)', () => { } }); }); + +describe('SessionStore.getAll() debuggerDisabled projection (issue #749)', () => { + it('projects debuggerDisabled only while the launch runs with the debugger off', () => { + const { store, id } = storeWith(SessionState.RUNNING); + expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); + + store.get(id)!.debuggerDisabled = true; + expect(store.getAll().find((s) => s.id === id)!.debuggerDisabled).toBe(true); + + store.get(id)!.debuggerDisabled = undefined; + expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); + }); +}); diff --git a/tests/e2e/mcp-server-break-on-exceptions.test.ts b/tests/e2e/mcp-server-break-on-exceptions.test.ts index 8e55e0852..caf95a1bc 100644 --- a/tests/e2e/mcp-server-break-on-exceptions.test.ts +++ b/tests/e2e/mcp-server-break-on-exceptions.test.ts @@ -38,6 +38,7 @@ const ROOT = path.resolve(__dirname, '../..'); const CRASHING_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-scripts', 'with-errors.py'); const JS_CRASHING_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-scripts', 'js-throws.js'); const JS_CLEAN_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-scripts', 'js-clean-exit.js'); +const JS_PAUSE_SCRIPT = path.resolve(ROOT, 'examples', 'javascript', 'pause_test.js'); const ATTACH_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'python', 'attach_then_raise.py'); const PYTHON = process.platform === 'win32' ? 'python' : 'python3'; @@ -55,6 +56,7 @@ interface SessionSnapshot { }; }; exitCode?: number; + debuggerDisabled?: boolean; } async function getSessionSnapshot(client: Client, sessionId: string): Promise { @@ -517,6 +519,84 @@ describe('Break-on-exception (issue #220)', () => { expect(stopped!.exitCode).toBe(0); }, 60000); + it('says the debugger is off on every later surface of a noDebug session (issue #749)', async () => { + sessionId = await createSession('javascript', 'js-nodebug-surfaces'); + const bp = parseSdkToolResult(await mcpClient!.callTool({ + name: 'set_breakpoint', + arguments: { sessionId, file: JS_PAUSE_SCRIPT, line: 4 } + })); + expect(bp.success, JSON.stringify(bp)).toBe(true); + + // A program that stays up, so the surfaces are asked while it runs. + const startRes = parseSdkToolResult(await mcpClient!.callTool({ + name: 'start_debugging', + arguments: { + sessionId, + scriptPath: JS_PAUSE_SCRIPT, + dapLaunchArgs: { stopOnEntry: false, noDebug: true } + } + })); + expect(startRes.success, JSON.stringify(startRes)).toBe(true); + expect(startRes.state).toBe('running'); + expect((startRes as { warning?: string }).warning).toMatch(/noDebug is true/); + + const why = /the debugger is off for this launch/; + + // The session remembers the decision... + const running = await getSessionSnapshot(mcpClient!, sessionId); + expect(running?.debuggerDisabled).toBe(true); + + // ...a live breakpoint still goes to js-debug, whose own answer is + // kept ("Unbound breakpoint") with the why beside it... + const live = parseSdkToolResult(await mcpClient!.callTool({ + name: 'set_breakpoint', + arguments: { sessionId, file: JS_PAUSE_SCRIPT, line: 5 } + })) as { success?: boolean; verified?: boolean; warning?: string }; + expect(live.success).toBe(true); + expect(live.verified).toBe(false); + expect(live.warning).toMatch(/Unbound breakpoint/); + expect(live.warning).toMatch(why); + + const listed = parseSdkToolResult(await mcpClient!.callTool({ + name: 'list_breakpoints', + arguments: { sessionId } + })) as { warning?: string; breakpoints?: Array<{ verified?: boolean }> }; + expect(listed.warning).toMatch(why); + expect(listed.breakpoints?.every(b => b.verified === false)).toBe(true); + + // ...inspection and stepping say why there is nothing paused... + const stack = parseSdkToolResult(await mcpClient!.callTool({ + name: 'get_stack_trace', + arguments: { sessionId } + })) as { note?: string }; + expect(stack.note).toMatch(why); + + const step = parseSdkToolResult(await mcpClient!.callTool({ + name: 'step_over', + arguments: { sessionId } + })) as { success?: boolean; error?: string }; + expect(step.success).toBe(false); + expect(step.error).toMatch(/^Not paused: /); + expect(step.error).toMatch(why); + + // ...and a pause is still sent to js-debug. Measured: js-debug lands + // it even under noDebug (the inspector is attached; only the debug + // domains are off), and that stop proves the debugger on — the session + // forgets the decision. Should a js-debug build refuse or never land + // it, the why rides on that answer instead. + const pause = parseSdkToolResult(await mcpClient!.callTool({ + name: 'pause_execution', + arguments: { sessionId } + })) as { success?: boolean; state?: string; error?: string; data?: { message?: string } }; + if (pause.success && pause.state === 'paused') { + const paused = await getSessionSnapshot(mcpClient!, sessionId); + expect(paused?.state).toBe('paused'); + expect(paused).not.toHaveProperty('debuggerDisabled'); + } else { + expect(`${pause.error ?? ''} ${pause.data?.message ?? ''}`).toMatch(why); + } + }, 60000); + it('reports exit code 0 for a clean run (issue #247)', async () => { sessionId = await createSession('javascript', 'js-clean-exit-code'); diff --git a/tests/unit/session-manager-operations-coverage.test.ts b/tests/unit/session-manager-operations-coverage.test.ts index 51342ea65..5b8e06797 100644 --- a/tests/unit/session-manager-operations-coverage.test.ts +++ b/tests/unit/session-manager-operations-coverage.test.ts @@ -4432,4 +4432,101 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () expect(bpArg).not.toHaveProperty('suspendPolicy'); }); }); + + /** + * A session whose launch runs with the debugger off (noDebug honoured, + * issue #710) still sends every request to the adapter and surfaces the + * adapter's own answer; the recorded fact supplies the why beside it + * (issue #749). + */ + describe('a launch running with the debugger off says so on every later surface (issue #749)', () => { + const why = ErrorMessages.debuggerOffForLaunch; + + it('has one sentence for the why, naming the fact and the remedy', () => { + expect(why).toMatch(/debugger is off for this launch/); + expect(why).toMatch(/noDebug/); + }); + + it('explains a pause that the adapter accepted but that never lands, instead of guessing at native code', async () => { + vi.useFakeTimers(); + try { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = true; + mockProxyManager.sendDapRequest.mockResolvedValue({}); + const describePendingStop = vi.fn().mockReturnValue('Explained by the policy.'); + vi.spyOn(operations as any, 'selectPolicy').mockReturnValue({ describePendingStop } as any); + + const promise = operations.pause('test-session', 1); + await vi.advanceTimersByTimeAsync(5000); + const result = await promise; + + // The pause was still sent: the adapter's answer is the ground truth. + expect(mockProxyManager.sendDapRequest).toHaveBeenCalledWith('pause', expect.objectContaining({ threadId: 1 })); + expect(result.success).toBe(true); + expect(result.data?.pending).toBe(true); + expect(result.data?.message).toBe(`${ErrorMessages.pausePending(5)} ${why}`); + expect(describePendingStop).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it("keeps the adapter's refusal of a pause and appends the why", async () => { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = true; + mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { + if (command === 'pause') { + throw new Error('Internal debugger error: Not supported in noDebug mode.'); + } + return {}; + }); + + await expect(operations.pause('test-session', 1)).rejects.toThrow( + `Internal debugger error: Not supported in noDebug mode. (${why})` + ); + }); + + it('says why stepping and continuing find nothing paused', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = true; + + const step = await operations.stepOver('test-session'); + expect(step.success).toBe(false); + expect(step.error).toBe(`Not paused: ${why}`); + + const cont = await operations.continue('test-session'); + expect(cont.success).toBe(false); + expect(cont.error).toBe(`Not paused: ${why}`); + }); + + it('leaves the plain "Not paused" alone when the debugger is on', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = undefined; + + const step = await operations.stepOver('test-session'); + expect(step.error).toBe('Not paused'); + }); + + it('says why an expression cannot be evaluated', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = true; + + const result = await operations.evaluateExpression('test-session', '1 + 1'); + + expect(result.success).toBe(false); + expect(result.error).toContain('not paused'); + expect(result.error).toContain(why); + }); + + it('says why the stack trace is empty', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = true; + + const result = await operations.getStackTraceDetailed('test-session'); + + expect(result.frames).toEqual([]); + expect(result.note).toMatch(/not paused/i); + expect(result.note).toContain(why); + }); + }); }); From 5e2a3d1e546e9e77a0420eb9a644ef3218b36229 Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 17 Sep 2026 12:31:38 -0400 Subject: [PATCH 2/7] fix(session): a pause does not clear the debugger-off decision; one gated why for every surface (#749 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #751 (nine findings, all taken): - handleStopped cleared debuggerDisabled on ANY stop, but the PR's own measurement says js-debug lands a user pause under noDebug with its breakpoints still off — so after the first pause every surface lost its why again. Only a stop a live debugger produces clears it now (DEBUGGER_ON_STOP_REASONS: the breakpoint family, exception, entry, step); the launcher's stoppedAnyway reads the same record instead of its own firstStopHandled/PAUSED discriminator, so the launch response and the record cannot disagree. - The record was never cleared on STOPPED/ERROR, so a breakpoint queued after the run said "cannot bind" and list_debug_sessions kept reporting a stopped session as debugger-off. isDebuggerOff() gates the decision on the session being running or paused; the projection and every consumer read through it. - One home for the concern: src/session/debugger-off.ts (the reason set, isDebuggerOff, debuggerOffWhy) replaces five hand-spliced reads. - The pending-pause message appended "no stop is expected" to a base text that promises one; ErrorMessages.pausePendingDebuggerOff is one message. - A refused pause threw a fresh Error, dropping the adapter's own error object (stack, props); the original is rethrown with the why appended, and the no-debug-target return carries the why too. - list_breakpoints warned even when every record was verified, contradicting set_breakpoint's own gate; now only with an unverified record. - The e2e's refusal branch was unreachable (callTool throws on an MCP error); callToolSafely, and the landed-pause branch asserts the decision is kept, as measured. - docs: list_breakpoints documents the top-level warning; the noDebug paragraph, the list_debug_sessions field note and the fragment describe which stops clear the decision and that it is not reported once the session is over. Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/749.fixed.md | 2 +- docs/tool-reference.md | 5 +- packages/shared/src/models/index.ts | 12 ++-- src/server/handlers/breakpoint-tools.ts | 21 ++++--- src/server/handlers/inspection-tools.ts | 9 +-- src/server/handlers/session-tools.ts | 3 +- src/session/debugger-off.ts | 44 +++++++++++++++ src/session/execution/execution-controller.ts | 55 +++++++++++++------ .../inspection/expression-evaluator.ts | 9 +-- .../inspection/frame-anchor-resolver.ts | 6 +- src/session/launch/debug-launcher.ts | 14 ++--- src/session/session-manager-core.ts | 13 +++-- src/session/session-store.ts | 6 +- src/utils/error-messages.ts | 26 ++++++++- .../server/handlers/session-tools.test.ts | 4 +- ...server-breakpoint-management-tools.test.ts | 34 ++++++++++++ .../unit/server/server-control-tools.test.ts | 20 +++++++ .../session-manager-nodebug-warning.test.ts | 34 +++++++++++- .../session/session-store-projection.test.ts | 5 ++ .../mcp-server-break-on-exceptions.test.ts | 15 +++-- ...ession-manager-operations-coverage.test.ts | 49 ++++++++++++++--- 21 files changed, 311 insertions(+), 75 deletions(-) create mode 100644 src/session/debugger-off.ts diff --git a/changelog.d/749.fixed.md b/changelog.d/749.fixed.md index fed508c5b..7bc22251b 100644 --- a/changelog.d/749.fixed.md +++ b/changelog.d/749.fixed.md @@ -1 +1 @@ -**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` — reset per launch and attach, and cleared by any `stopped` event that arrives anyway (js-debug lands a pause under the flag; a real stop proves the debugger on). Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal); the recorded fact adds the why beside it: "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" (#749) +**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while it is running or paused — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (breakpoint, exception, entry, step): that proves the debugger on for the adapter build. A pause does not clear it — js-debug lands a user pause under the flag with its breakpoints still unbound. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 9aa4d3b1d..96c50a2b1 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -120,7 +120,7 @@ Lists all active debugging sessions. **`lastStop`:** present while the session is `paused` (the stop it is at: `reason`, `threadId`, `timestamp`, the adapter's `description`/`text`, and `exceptionInfo` for exception stops) and after it reaches `stopped`/`error` (the last stop before it ended). A `running` session never carries one, so a poller that calls this after `continue_execution` or a step sees `state: "running"` with no stop record until the next stop lands — the record of the stop it just left is not repeated as if the program were still paused. -**`debuggerDisabled`:** present (`true`) while the current launch runs with the debugger off — `dapLaunchArgs.noDebug: true` on an adapter that honours it (see `start_debugging`). Cleared by the next launch or attach, and by any `stopped` event that arrives anyway (issue #749). +**`debuggerDisabled`:** present (`true`) while the current launch runs with the debugger off — `dapLaunchArgs.noDebug: true` on an adapter that honours it (see `start_debugging`) — and the session is `running` or `paused`. Cleared by the next launch or attach, and by a `stopped` event only a live debugger produces (breakpoint, exception, entry, step; not a pause). Omitted once the session is `stopped` or in `error` (issue #749). Errored sessions include optional `diagnostics` with the current launch attempt's server-host `proxyLogPath` and remote-safe `proxyLogResource`. The record is retained for proxy initialization failures and for proxy/adapter deaths after initialization, and is cleared when a new launch or attach attempt begins. @@ -302,6 +302,7 @@ Lists all breakpoints in a session with their current verified state and adapter - `functionBreakpoints`/`functionCount` are always present in the unfiltered response (empty arrays when none exist). When filtering by `file` they are omitted — function breakpoints are session-global, not file-scoped. - `adapterId` is the debug adapter's own numeric id for the breakpoint, captured from setBreakpoints responses and breakpoint events. It is absent until the adapter has seen the breakpoint. - Verification is eventually consistent: some adapters (js-debug, JDI, netcoredbg) bind breakpoints asynchronously and confirm via DAP breakpoint events shortly after launch or class load. +- `warning` (top level) appears only while the session's current launch runs with the debugger off (`noDebug: true` on an adapter that honours it, see `start_debugging`) and at least one listed breakpoint is unverified: "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug". The per-breakpoint records keep the adapter's own answers (issue #749). - A breakpoint the program has stopped on is reported `verified: true` from that stop onward, even if the adapter never confirmed it (issue #673), for adapters whose `stopped` event names the breakpoints it hit (`hitBreakpointIds`: js-debug, debugpy, Delve, CodeLLDB — netcoredbg, the JDI bridge and rdbg omit the field). Such a record carries `verifiedBy: "hit"` until the adapter itself confirms it (`"adapter"`); an adapter answer of "unbound" does not downgrade it. A provisional "Unbound breakpoint" `message` is dropped by the hit; any other note is kept. - On entries of the `breakpoints` array, `boundFile`/`boundLine` appear when the adapter answers under a *different* file from the request. For a source-mapped `.ts` request on a JavaScript launch with maps on (the default) js-debug verifies the request under the `.ts` path, `get_stack_trace` frames show `.ts`, and the pair is absent; it appears when js-debug answers under the generated `dist/*.js` instead — measured with `adapterLaunchConfig: { sourceMaps: false }` (the second entry in the example above was captured that way, and the frames then show the generated file too), and possible whenever the `.ts` source cannot be resolved through the map (issues #673, #700). `file` and `line` keep describing the request; the bound pair is where it landed. (Entries of `functionBreakpoints` use the same names for the bound location of the symbol, present whenever it is bound.) @@ -368,7 +369,7 @@ Starts debugging a script. - `dapLaunchArgs` (object, optional): Standard DAP launch arguments: - `stopOnEntry` (boolean): Stop at first line (default `false` — the opposite of attach, which pauses unless `stopOnEntry` is `false`) - `justMyCode` (boolean): Debug only user code (default `true`). JavaScript launch: `true` blackboxes `node_modules` through js-debug's `skipFiles` and keeps js-debug's smart-stepper on, so a pause or step that lands in skipped code (Node internals, `node_modules`) is stepped through and may never land (`pending: true`, with an explanation); `false` drops `node_modules` from the skip list and turns the stepper off, so steps land inside dependencies and `pause_execution` lands as soon as any JavaScript runs (issue #678). A caller `skipFiles` replaces the default list. Source maps are on for every JavaScript launch, `.js` programs included — stops report `src/*.ts` when maps and sources are present; `adapterLaunchConfig: { sourceMaps: false }` opts out (issue #684) - - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off and names what will not fire, in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint") and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event that arrives anyway — js-debug lands a pause under the flag — proves the debugger on and clears it (issue #749). + - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off and names what will not fire, in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint") and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event only a live debugger produces (breakpoint, exception, entry, step) proves the debugger on for that adapter build and clears the decision; a pause does not — js-debug lands a user pause under the flag while its breakpoints stay unbound — and once the session is `stopped` or in `error` the decision is no longer reported or consulted (issue #749). - Additional DAP launch keys (`program`, `cwd`, `env`, language-specific options) pass through to the adapter. Top-level parameters do **not** belong here: a nested `breakOnExceptions` is honored as an alias (the top-level value wins if both are given) and reported via a `warning` in the response; other misplaced top-level keys (`dryRunSpawn`, `sessionId`, `scriptPath`, `adapterLaunchConfig`) are stripped with a warning instead of silently riding into the launch config. - `adapterLaunchConfig` (object, optional): Adapter-specific launch configuration overrides. Use this for language-specific settings that go beyond standard DAP arguments (e.g., `mainClass` and `classpath` for Java, `buildCommand` for Rust). For Rust, `_adapterSettings` passes through to CodeLLDB (issue #441) — e.g. `{"_adapterSettings": {"scriptConfig": {"lang": {"rust": {"sysroot": "/path"}}}}}` points the Rust formatter lookup at an explicit sysroot; the `CODELLDB_RUST_SYSROOT` env var does the same without per-launch config (a user-supplied `_adapterSettings` value wins over the env var). - `dryRunSpawn` (boolean, optional): Test spawn without actually starting diff --git a/packages/shared/src/models/index.ts b/packages/shared/src/models/index.ts index 8f2d1034d..bf5523c0f 100644 --- a/packages/shared/src/models/index.ts +++ b/packages/shared/src/models/index.ts @@ -436,10 +436,14 @@ export interface DebugSessionInfo { * True while the current launch runs with the debugger off: the launch * carried `noDebug: true` and the adapter honours it (issue #710), so no * breakpoint can bind and no stop is expected. Set by the launcher, reset - * per launch and attach, and cleared by the first `stopped` event that - * arrives anyway — a real stop proves the debugger on for this adapter - * build. Later surfaces (set_breakpoint, list_breakpoints, pause, - * stepping, inspection) read it to say why (issue #749). + * per launch and attach, and cleared by a `stopped` event only a live + * debugger produces (breakpoint, exception, entry, step — not a pause, + * which js-debug lands under the flag with breakpoints still off): such a + * stop proves the debugger on for this adapter build. Projected, and + * consulted by the later surfaces (set_breakpoint, list_breakpoints, + * pause, stepping, inspection), only while the session is running or + * paused — once it is stopped or in error the record describes nothing + * that is running (issue #749). */ debuggerDisabled?: boolean; /** diff --git a/src/server/handlers/breakpoint-tools.ts b/src/server/handlers/breakpoint-tools.ts index 4f73b23fc..109534e7e 100644 --- a/src/server/handlers/breakpoint-tools.ts +++ b/src/server/handlers/breakpoint-tools.ts @@ -14,7 +14,7 @@ import type { FunctionBreakpointRemoval } from '../../session/session-manager-op import type { ToolContext, ToolHandler } from '../tool-context.js'; import { requireSessionId, type WithSessionId } from '../tool-validation.js'; import { readLineContext } from './shared.js'; -import { ErrorMessages } from '../../utils/error-messages.js'; +import { debuggerOffWhy } from '../../session/debugger-off.js'; import { failureResult, jsonResult, sessionErrorResultOrThrow, type ToolResult } from '../tool-result.js'; /** @@ -23,9 +23,11 @@ import { failureResult, jsonResult, sessionErrorResultOrThrow, type ToolResult } * its own answer is kept; a breakpoint it verified anyway needs no note. */ function debuggerOffNote(ctx: ToolContext, sessionId: string, verified: boolean): string | undefined { - return !verified && ctx.sessionManager.getSession(sessionId)?.debuggerDisabled - ? ErrorMessages.debuggerOffForLaunch - : undefined; + if (verified) { + return undefined; + } + const session = ctx.sessionManager.getSession(sessionId); + return session ? debuggerOffWhy(session) : undefined; } export const setBreakpointTool: ToolHandler = async (ctx, args) => { @@ -245,8 +247,13 @@ export const listBreakpointsTool: ToolHandler = async (ctx, args) => { ? ctx.sessionManager.listFunctionBreakpoints(args.sessionId) : []; // Per-breakpoint records carry the adapter's own answers; the one reason - // none of them can bind right now goes on the response (issue #749). - const debuggerOff = ctx.sessionManager.getSession(args.sessionId)?.debuggerDisabled === true; + // the unverified ones cannot bind right now goes on the response (issue + // #749) — like set_breakpoint's note, a breakpoint the adapter verified + // anyway is not contradicted. + const anyUnverified = + breakpoints.some((bp) => !bp.verified) || functionBreakpoints.some((bp) => !bp.verified); + const session = ctx.sessionManager.getSession(args.sessionId); + const why = anyUnverified && session ? debuggerOffWhy(session) : undefined; return jsonResult({ success: true, breakpoints, @@ -254,7 +261,7 @@ export const listBreakpointsTool: ToolHandler = async (ctx, args) => { ...(args.file === undefined ? { functionBreakpoints, functionCount: functionBreakpoints.length } : {}), - ...(debuggerOff ? { warning: ErrorMessages.debuggerOffForLaunch } : {}) + ...(why ? { warning: why } : {}) }); } catch (error) { return sessionErrorResultOrThrow(error); diff --git a/src/server/handlers/inspection-tools.ts b/src/server/handlers/inspection-tools.ts index 0a1d347c4..d884360b6 100644 --- a/src/server/handlers/inspection-tools.ts +++ b/src/server/handlers/inspection-tools.ts @@ -5,7 +5,7 @@ import { ErrorCode as McpErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import { SessionState } from '@debugmcp/shared'; import { SessionTerminatedError } from '../../errors/debug-errors.js'; -import { ErrorMessages } from '../../utils/error-messages.js'; +import { debuggerOffWhy } from '../../session/debugger-off.js'; import type { ToolContext, ToolHandler } from '../tool-context.js'; import { enforceExplicitNames, requireSessionId } from '../tool-validation.js'; import { carriesLastStop, variablePayloadExtras } from './shared.js'; @@ -336,13 +336,14 @@ export async function handleGetLocalVariables(ctx: ToolContext, args: { sessionI // no frames" — the latter used to claim the debugger may not be // paused while list_debug_sessions said paused (issue #465). const sessionNow = ctx.sessionManager.getSession(args.sessionId); + // The why, when the launch runs with the debugger off (issue #749). + const why = sessionNow ? debuggerOffWhy(sessionNow) : undefined; response.message = sessionNow?.state === SessionState.PAUSED ? 'The session is paused, but the anchored thread reported no stack frames. ' + 'Try get_stack_trace with a threadId from list_threads, or continue_execution ' + 'followed by pause_execution to re-anchor on a reportable thread.' - : sessionNow?.debuggerDisabled - // The why, when the launch runs with the debugger off (issue #749). - ? `No stack frames available; ${ErrorMessages.debuggerOffForLaunch}.` + : why + ? `No stack frames available; ${why}.` : 'No stack frames available. The debugger may not be paused.'; } else if (!result.scopeName) { response.message = 'No local scope found in the current frame.'; diff --git a/src/server/handlers/session-tools.ts b/src/server/handlers/session-tools.ts index 0fbfdb9ca..125594a51 100644 --- a/src/server/handlers/session-tools.ts +++ b/src/server/handlers/session-tools.ts @@ -10,6 +10,7 @@ import { isContainerRuntime } from '../../utils/container-path-utils.js'; import type { ToolContext, ToolHandler } from '../tool-context.js'; import { assertPlainObjectArg, requireSessionId } from '../tool-validation.js'; import { carriesLastStop, successWarning } from './shared.js'; +import { isDebuggerOff } from '../../session/debugger-off.js'; import { failureResult, jsonResult, rethrowAsMcpError, type ToolResult } from '../tool-result.js'; export const createDebugSessionTool: ToolHandler = async (ctx, args) => { @@ -175,7 +176,7 @@ export async function handleListDebugSessions(ctx: ToolContext): Promise = new Set([ + ...BREAKPOINT_STOP_REASONS, + 'exception', + 'entry', + 'step' +]); + +/** + * Whether the decision applies now: recorded for this launch, and the + * launch is still running or paused. Once the session is stopped or in + * error the record describes nothing that is running — a breakpoint set + * then is an ordinary queued one for the next launch. + */ +export function isDebuggerOff(session: DebuggerOffView): boolean { + return session.debuggerDisabled === true && !isTerminalSessionState(session.state); +} + +/** The why to place beside an answer, when the decision applies; else nothing. */ +export function debuggerOffWhy(session: DebuggerOffView): string | undefined { + return isDebuggerOff(session) ? ErrorMessages.debuggerOffForLaunch : undefined; +} diff --git a/src/session/execution/execution-controller.ts b/src/session/execution/execution-controller.ts index 203de38b4..3c926c547 100644 --- a/src/session/execution/execution-controller.ts +++ b/src/session/execution/execution-controller.ts @@ -51,6 +51,7 @@ import type { StopLocation } from '../session-manager-core.js'; import { USER_BREAK_REASONS } from '../session-manager-core.js'; +import { debuggerOffWhy, isDebuggerOff, type DebuggerOffView } from '../debugger-off.js'; import { samePath } from '../breakpoints/hit-verification.js'; import type { ExecutionContext } from '../operations-context.js'; import type { PauseCoordinator } from './pause-coordinator.js'; @@ -140,10 +141,24 @@ function isSameLine(a: StopLocation, b: StopLocation): boolean { * The step/continue refusal for a session that is not paused, with the why * when the session's launch runs with the debugger off (issue #749). */ -function notPausedError(session: Pick): string { - return session.debuggerDisabled - ? `Not paused: ${ErrorMessages.debuggerOffForLaunch}` - : 'Not paused'; +function notPausedError(session: DebuggerOffView): string { + const why = debuggerOffWhy(session); + return why ? `Not paused: ${why}` : 'Not paused'; +} + +/** + * A pause the adapter answered with an error, with the why beside the + * adapter's own words when the launch runs with the debugger off (issue + * #749). The original error object is kept — its stack and any structured + * properties are the adapter's answer too. + */ +function withDebuggerOffWhy(session: DebuggerOffView, error: unknown, message: string): { error: Error; message: string } { + const err = error instanceof Error ? error : new Error(message); + const why = debuggerOffWhy(session); + if (why) { + err.message = `${err.message} (${why})`; + } + return { error: err, message: err.message }; } export class ExecutionController { @@ -631,12 +646,21 @@ export class ExecutionController { this.ctx.logger.info( `[SessionManager pause] No stopped event within ${this.ctx.tunables.pauseGraceMs}ms grace window in session ${sessionId}; completing asynchronously` ); + // With the debugger off for this launch the why is known (issue #749): + // one message that promises no stop, instead of the policy's guess at + // native code or a syscall. + if (isDebuggerOff(session)) { + return { + success: true, + state: session.state, + data: { + message: ErrorMessages.pausePendingDebuggerOff(this.ctx.tunables.pauseGraceMs / 1000), + pending: true + } + }; + } const pausePending = ErrorMessages.pausePending(this.ctx.tunables.pauseGraceMs / 1000); - // With the debugger off for this launch the why is known (issue #749); - // the policy's guess at native code or a syscall would be wrong. - const hint = session.debuggerDisabled - ? ErrorMessages.debuggerOffForLaunch - : await this.describePendingStop(session, sessionId, 'pause'); + const hint = await this.describePendingStop(session, sessionId, 'pause'); return { success: true, state: session.state, @@ -659,15 +683,14 @@ export class ExecutionController { this.ctx.logger.error( `[SessionManager pause] Error sending 'pause' for session ${sessionId}: ${errorMessage}` ); + // The adapter's answer stands — a refusal (CodeLLDB under noDebug), or + // no debug target yet (js-debug before the child adopts) — with the why + // beside it when the launch runs with the debugger off (issue #749). + const answered = withDebuggerOffWhy(session, outcome.error, errorMessage); if (errorMessage.includes(NO_DEBUG_TARGET_MARKER)) { - return { success: false, error: errorMessage, state: session.state }; - } - if (session.debuggerDisabled) { - // The adapter refused the pause (CodeLLDB under noDebug): its answer - // stands, with the why beside it (issue #749). - throw new Error(`${errorMessage} (${ErrorMessages.debuggerOffForLaunch})`); + return { success: false, error: answered.message, state: session.state }; } - throw outcome.error instanceof Error ? outcome.error : new Error(errorMessage); + throw answered.error; } async listThreads(sessionId: string): Promise> { diff --git a/src/session/inspection/expression-evaluator.ts b/src/session/inspection/expression-evaluator.ts index f83ef295d..a2ca9a26c 100644 --- a/src/session/inspection/expression-evaluator.ts +++ b/src/session/inspection/expression-evaluator.ts @@ -8,7 +8,7 @@ * hook (issue #237) runs before anything, including the logs, sees the result. */ import { getErrorMessage } from '../../errors/debug-errors.js'; -import { ErrorMessages } from '../../utils/error-messages.js'; +import { debuggerOffWhy } from '../debugger-off.js'; import { buildRedactionNotice, isSensitiveName, @@ -132,11 +132,12 @@ export class ExpressionEvaluator { this.ctx.logger.warn( `[SM evaluateExpression ${sessionId}] Cannot evaluate: session not paused. State: ${session.state}` ); + // The why, when the launch runs with the debugger off (issue #749). + const why = debuggerOffWhy(session); return { success: false, - error: session.debuggerDisabled - // The why, when the launch runs with the debugger off (issue #749). - ? `Cannot evaluate: debugger not paused (${ErrorMessages.debuggerOffForLaunch})` + error: why + ? `Cannot evaluate: debugger not paused (${why})` : 'Cannot evaluate: debugger not paused. Ensure the debugger is stopped at a breakpoint.', }; } diff --git a/src/session/inspection/frame-anchor-resolver.ts b/src/session/inspection/frame-anchor-resolver.ts index e546eef33..c44fc2186 100644 --- a/src/session/inspection/frame-anchor-resolver.ts +++ b/src/session/inspection/frame-anchor-resolver.ts @@ -18,7 +18,7 @@ import type { DebugProtocol } from '@vscode/debugprotocol'; import path from 'path'; import type { IProxyManager } from '../../proxy/proxy-manager.js'; import type { ManagedSession } from '../session-store.js'; -import { ErrorMessages } from '../../utils/error-messages.js'; +import { debuggerOffWhy } from '../debugger-off.js'; /** The frame fields a tool response names when it says which frame answered. */ export type FrameSummary = Pick; @@ -137,9 +137,9 @@ export class FrameAnchorResolver { if (session.state !== SessionState.PAUSED) { this.ctx.logger.warn(`[FrameAnchor ${sessionId}] Session not paused: ${session.state}.`); // The why, when the launch runs with the debugger off (issue #749). - const why = session.debuggerDisabled ? `; ${ErrorMessages.debuggerOffForLaunch}` : ''; + const why = debuggerOffWhy(session); return emptyResult( - `Session is not paused (state: ${session.state}); stack traces are only available while paused${why}.`, + `Session is not paused (state: ${session.state}); stack traces are only available while paused${why ? `; ${why}` : ''}.`, threadId ); } diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index ce617ccb0..66f08461b 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -536,13 +536,13 @@ export class DebugLauncher { } // The policy's word is a static pin; a stop that arrived anyway is the - // stronger evidence (an adapter build that ignores the flag after all) — - // a pause still standing, or an entry stop the core already resumed - // (firstStopHandled is set on every stop of this launch, resumed or not). - // Then the debugger was on: keep the ordinary diagnostics and say the - // flag had no effect rather than that no stop can come. - const stoppedAnyway = - debuggerOff && (finalState === SessionState.PAUSED || finalSession.firstStopHandled === true); + // stronger evidence (an adapter build that ignores the flag after all). + // The core's stopped handler is the one judge of that — it clears the + // recorded decision on a stop only a live debugger produces (issue + // #749), so the launch response and the record cannot disagree. Then + // the debugger was on: keep the ordinary diagnostics and say the flag + // had no effect rather than that no stop can come. + const stoppedAnyway = debuggerOff && finalSession.debuggerDisabled !== true; const noDebugNote = stoppedAnyway ? buildNoDebugLaunchWarning(finalSession, { noDebug }, breakOnExceptions, false) : noDebugWarning; diff --git a/src/session/session-manager-core.ts b/src/session/session-manager-core.ts index ed30e1424..587384b8a 100644 --- a/src/session/session-manager-core.ts +++ b/src/session/session-manager-core.ts @@ -9,6 +9,7 @@ import { } from '@debugmcp/shared'; import type { Breakpoint, FunctionBreakpoint, StackFrame } from '@debugmcp/shared'; import { BREAKPOINT_STOP_REASONS } from '@debugmcp/shared'; +import { DEBUGGER_ON_STOP_REASONS } from './debugger-off.js'; import { isRedactionEnabled } from '../utils/redaction-mode.js'; import { ValidationResultCache } from '../utils/language-availability.js'; import { SessionStore, ManagedSession } from './session-store.js'; @@ -736,10 +737,14 @@ export abstract class SessionManagerCore extends EventEmitter { } session.firstStopHandled = true; - // A stop is stronger evidence than the policy's noDebug pin: this - // adapter build debugs after all, so the later surfaces must stop - // explaining themselves in debugger-off terms (issue #749). - session.debuggerDisabled = undefined; + // A stop only a live debugger produces is stronger evidence than the + // policy's noDebug pin: this adapter build debugs after all, so the + // later surfaces must stop explaining themselves in debugger-off + // terms (issue #749). A pause is not such a stop — js-debug lands one + // under noDebug with breakpoints still off. + if (DEBUGGER_ON_STOP_REASONS.has(reason)) { + session.debuggerDisabled = undefined; + } }; proxyManager.on('stopped', handleStopped); handlers.set('stopped', handleStopped); diff --git a/src/session/session-store.ts b/src/session/session-store.ts index bae5ccc34..882b5435a 100644 --- a/src/session/session-store.ts +++ b/src/session/session-store.ts @@ -33,6 +33,7 @@ export interface CreateSessionParams { import type { DebugProtocol } from '@vscode/debugprotocol'; import { IProxyManager } from '../proxy/proxy-manager.js'; import { OutputRingBuffer } from './output-buffer.js'; +import { isDebuggerOff } from './debugger-off.js'; import type { PauseIntent } from './execution/pause-intent.js'; import type { ProxyFailureDiagnostics } from './launch/proxy-failure-diagnostics.js'; @@ -289,8 +290,9 @@ export class SessionStore { ...(s.state === SessionState.ERROR && s.failureDiagnostics ? { diagnostics: s.failureDiagnostics } : {}), - // The current launch runs with the debugger off (issue #749). - ...(s.debuggerDisabled ? { debuggerDisabled: true } : {}), + // The current launch runs with the debugger off (issue #749) — while + // it runs; a stopped or errored session describes nothing running. + ...(isDebuggerOff(s) ? { debuggerDisabled: true } : {}), // Mirror endpoint without the token (issue #217); the isRunning gate // keeps the projection honest on teardown paths that skip cleanup. ...(s.exposure && s.proxyManager?.isRunning() diff --git a/src/utils/error-messages.ts b/src/utils/error-messages.ts index 505987a4b..425f7aea0 100644 --- a/src/utils/error-messages.ts +++ b/src/utils/error-messages.ts @@ -35,6 +35,15 @@ const IN_FLIGHT = { /** The launch-shaped operations one session may hold a claim for (issue #711). */ export type InFlightOperation = keyof typeof IN_FLIGHT; +/** + * The why a session whose current launch runs with the debugger off appends + * to every answer that would otherwise read in debugger terms (issue #749) — + * a module constant so the composed messages below can build on it. + */ +const DEBUGGER_OFF_FOR_LAUNCH = + 'the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; ' + + 'drop noDebug and launch again to debug'; + export const ErrorMessages = { /** * Error message for DAP request timeouts @@ -163,9 +172,20 @@ export const ErrorMessages = { * src/session/execution/execution-controller.ts, src/session/inspection/frame-anchor-resolver.ts, * src/session/inspection/expression-evaluator.ts */ - debuggerOffForLaunch: - 'the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; ' + - 'drop noDebug and launch again to debug', + debuggerOffForLaunch: DEBUGGER_OFF_FOR_LAUNCH, + + /** + * The pending-pause message for a session whose launch runs with the + * debugger off (issue #749): the pause was sent and accepted, no stop came + * within the grace window, and — unlike `pausePending` — no stop is + * promised, since none is expected. The session still reports 'paused' + * should one land anyway (js-debug does this under noDebug). + * Used in: src/session/execution/execution-controller.ts + * @param graceSeconds - The grace window duration in seconds + */ + pausePendingDebuggerOff: (graceSeconds: number) => + `Pause requested; no 'stopped' event within ${graceSeconds}s — ${DEBUGGER_OFF_FOR_LAUNCH}. ` + + `Check the session state in case a stop lands anyway.`, /** * Suffix appended to the attach message when the post-attach pause was diff --git a/tests/core/unit/server/handlers/session-tools.test.ts b/tests/core/unit/server/handlers/session-tools.test.ts index 2bd3e821b..321aebe18 100644 --- a/tests/core/unit/server/handlers/session-tools.test.ts +++ b/tests/core/unit/server/handlers/session-tools.test.ts @@ -81,7 +81,8 @@ describe('session tool handlers', () => { const now = new Date(); ctx.sessionManager.getAllSessions.mockReturnValue([ { id: 'off', name: 'o', language: 'python', state: 'running', createdAt: now, debuggerDisabled: true }, - { id: 'on', name: 'n', language: 'python', state: 'running', createdAt: now } + { id: 'on', name: 'n', language: 'python', state: 'running', createdAt: now }, + { id: 'over', name: 'v', language: 'python', state: 'stopped', createdAt: now, debuggerDisabled: true } ]); const result = await handleListDebugSessions(ctx); @@ -90,6 +91,7 @@ describe('session tool handlers', () => { expect(byId.off.debuggerDisabled).toBe(true); expect(byId.on).not.toHaveProperty('debuggerDisabled'); + expect(byId.over).not.toHaveProperty('debuggerDisabled'); }); }); }); diff --git a/tests/core/unit/server/server-breakpoint-management-tools.test.ts b/tests/core/unit/server/server-breakpoint-management-tools.test.ts index 06f62236b..84f47c35a 100644 --- a/tests/core/unit/server/server-breakpoint-management-tools.test.ts +++ b/tests/core/unit/server/server-breakpoint-management-tools.test.ts @@ -119,6 +119,40 @@ describe('Server Breakpoint Management Tools', () => { expect(content.warning).toBe(ErrorMessages.debuggerOffForLaunch); }); + it('adds no debugger-off warning when every breakpoint is verified anyway, or once the launch is over (issue #749)', async () => { + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'running', + sessionLifecycle: 'ACTIVE', + debuggerDisabled: true + }); + mockSessionManager.listBreakpoints.mockReturnValue([ + { id: 'bp-1', file: '/a.py', line: 10, verified: true } + ]); + mockSessionManager.listFunctionBreakpoints.mockReturnValue([]); + + let result = await callToolHandler({ + method: 'tools/call', + params: { name: 'list_breakpoints', arguments: { sessionId: 'test-session' } } + }); + expect(JSON.parse(result.content[0].text).warning).toBeUndefined(); + + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'stopped', + sessionLifecycle: 'ACTIVE', + debuggerDisabled: true + }); + mockSessionManager.listBreakpoints.mockReturnValue([ + { id: 'bp-1', file: '/a.py', line: 10, verified: false } + ]); + result = await callToolHandler({ + method: 'tools/call', + params: { name: 'list_breakpoints', arguments: { sessionId: 'test-session' } } + }); + expect(JSON.parse(result.content[0].text).warning).toBeUndefined(); + }); + it('always includes empty function-breakpoint fields in the unfiltered response (#306)', async () => { mockSessionManager.listBreakpoints.mockReturnValue([]); mockSessionManager.listFunctionBreakpoints.mockReturnValue([]); diff --git a/tests/core/unit/server/server-control-tools.test.ts b/tests/core/unit/server/server-control-tools.test.ts index 9c40d9641..d0de66c02 100644 --- a/tests/core/unit/server/server-control-tools.test.ts +++ b/tests/core/unit/server/server-control-tools.test.ts @@ -79,6 +79,26 @@ describe('Server Control Tools Tests', () => { expect(content.warning).toContain(ErrorMessages.debuggerOffForLaunch); }); + it('adds no debugger-off note once the launch is over — a queued breakpoint is an ordinary one (issue #749)', async () => { + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'stopped', + sessionLifecycle: 'ACTIVE', + debuggerDisabled: true + }); + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 10, verified: false } + }); + + const result = await callToolHandler({ + method: 'tools/call', + params: { name: 'set_breakpoint', arguments: { sessionId: 'test-session', file: '/path/to/test.py', line: 10 } } + }); + + const content = JSON.parse(result.content[0].text); + expect(content.warning).toBeUndefined(); + }); + it('adds no debugger-off note to a breakpoint the adapter verified anyway (issue #749)', async () => { mockSessionManager.getSession.mockReturnValue({ id: 'test-session', diff --git a/tests/core/unit/session/session-manager-nodebug-warning.test.ts b/tests/core/unit/session/session-manager-nodebug-warning.test.ts index 455d0a4d7..d8fc39ca3 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -491,19 +491,47 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); }); - it('is cleared by a stop that arrives anyway — the adapter proved the debugger on', async () => { + it('survives a pause that lands — js-debug pauses under noDebug while its breakpoints stay unbound', async () => { pinPolicy({ honoursNoDebug: true }); const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); runWithoutStopping(); await launch(s.id, { stopOnEntry: false, noDebug: true }); expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); - // A later pause lands (js-debug does this under noDebug: measured). + // Measured: the inspector is attached and a user pause lands, but the + // debug domains — breakpoints — are off. A pause proves nothing. dependencies.mockProxyManager.simulateEvent('stopped', 1, 'pause', { reason: 'pause', threadId: 1 }); await vi.runAllTimersAsync(); expect(sessionManager.getSession(s.id)?.state).toBe(SessionState.PAUSED); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + }); + + it.each(['breakpoint', 'function breakpoint', 'exception', 'entry', 'step'])( + "is cleared by a '%s' stop — one a disabled debugger cannot produce", + async (reason) => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + + dependencies.mockProxyManager.simulateEvent('stopped', 1, reason, { reason, threadId: 1 }); + await vi.runAllTimersAsync(); + + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + } + ); + + it('is not projected once the launch is over — the next set_breakpoint is an ordinary queued one', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + endDuringStartup(); + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + expect(result.state).toBe(SessionState.STOPPED); + + const listed = sessionManager.getAllSessions().find((x) => x.id === s.id); + expect(listed).not.toHaveProperty('debuggerDisabled'); }); it('recomputes it on restart_debugging, which replays the same arguments', async () => { diff --git a/tests/core/unit/session/session-store-projection.test.ts b/tests/core/unit/session/session-store-projection.test.ts index 17676c3bf..12ec8d1d1 100644 --- a/tests/core/unit/session/session-store-projection.test.ts +++ b/tests/core/unit/session/session-store-projection.test.ts @@ -63,5 +63,10 @@ describe('SessionStore.getAll() debuggerDisabled projection (issue #749)', () => store.get(id)!.debuggerDisabled = undefined; expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); + + // Once the launch is over the flag describes nothing that is running. + store.get(id)!.debuggerDisabled = true; + store.get(id)!.state = SessionState.STOPPED; + expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); }); }); diff --git a/tests/e2e/mcp-server-break-on-exceptions.test.ts b/tests/e2e/mcp-server-break-on-exceptions.test.ts index caf95a1bc..0bce4b43b 100644 --- a/tests/e2e/mcp-server-break-on-exceptions.test.ts +++ b/tests/e2e/mcp-server-break-on-exceptions.test.ts @@ -584,16 +584,19 @@ describe('Break-on-exception (issue #220)', () => { // domains are off), and that stop proves the debugger on — the session // forgets the decision. Should a js-debug build refuse or never land // it, the why rides on that answer instead. - const pause = parseSdkToolResult(await mcpClient!.callTool({ - name: 'pause_execution', - arguments: { sessionId } - })) as { success?: boolean; state?: string; error?: string; data?: { message?: string } }; + // callToolSafely: a refusal reaches the wire as an MCP error, which + // callTool would throw rather than return. + const pause = await callToolSafely(mcpClient!, 'pause_execution', { sessionId }) as { + success?: boolean; state?: string; error?: unknown; message?: string; data?: { message?: string }; + }; if (pause.success && pause.state === 'paused') { + // A pause proves nothing about breakpoints: the session keeps the + // decision, and the surfaces keep explaining. const paused = await getSessionSnapshot(mcpClient!, sessionId); expect(paused?.state).toBe('paused'); - expect(paused).not.toHaveProperty('debuggerDisabled'); + expect(paused?.debuggerDisabled).toBe(true); } else { - expect(`${pause.error ?? ''} ${pause.data?.message ?? ''}`).toMatch(why); + expect(`${String(pause.error ?? '')} ${pause.message ?? ''} ${pause.data?.message ?? ''}`).toMatch(why); } }, 60000); diff --git a/tests/unit/session-manager-operations-coverage.test.ts b/tests/unit/session-manager-operations-coverage.test.ts index 5b8e06797..7d4d2c801 100644 --- a/tests/unit/session-manager-operations-coverage.test.ts +++ b/tests/unit/session-manager-operations-coverage.test.ts @@ -18,8 +18,7 @@ import { type Breakpoint, type CustomLaunchRequestArguments, type FunctionBreakpoint, - type ILogger -} from '@debugmcp/shared'; + type ILogger, NO_DEBUG_TARGET_MARKER } from '@debugmcp/shared'; /** Concrete subclass for testing the abstract SessionManagerOperations */ class TestableSessionManagerOperations extends SessionManagerOperations { @@ -4464,26 +4463,62 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () expect(mockProxyManager.sendDapRequest).toHaveBeenCalledWith('pause', expect.objectContaining({ threadId: 1 })); expect(result.success).toBe(true); expect(result.data?.pending).toBe(true); - expect(result.data?.message).toBe(`${ErrorMessages.pausePending(5)} ${why}`); + // One message that does not promise the stop the base text promises. + expect(result.data?.message).toBe(ErrorMessages.pausePendingDebuggerOff(5)); + expect(result.data?.message).toContain(why); + expect(result.data?.message).not.toMatch(/blocked in native code/); + expect(result.data?.message).not.toMatch(/will report 'paused' once the stop lands/); expect(describePendingStop).not.toHaveBeenCalled(); } finally { vi.useRealTimers(); } }); - it("keeps the adapter's refusal of a pause and appends the why", async () => { + it("keeps the adapter's refusal of a pause — the same error object — and appends the why", async () => { mockSession.state = SessionState.RUNNING; mockSession.debuggerDisabled = true; + const refusal = Object.assign(new Error('Internal debugger error: Not supported in noDebug mode.'), { code: 'E_NODEBUG' }); mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { if (command === 'pause') { - throw new Error('Internal debugger error: Not supported in noDebug mode.'); + throw refusal; } return {}; }); - await expect(operations.pause('test-session', 1)).rejects.toThrow( - `Internal debugger error: Not supported in noDebug mode. (${why})` + const thrown = await operations.pause('test-session', 1).then( + () => { throw new Error('expected a rejection'); }, + (err: unknown) => err ); + expect(thrown).toBe(refusal); + expect((thrown as Error).message).toBe(`Internal debugger error: Not supported in noDebug mode. (${why})`); + expect((thrown as Error & { code?: string }).code).toBe('E_NODEBUG'); + }); + + it('carries the why on a pause that found no debug target yet (js-debug before the child adopts)', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = true; + mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { + if (command === 'pause') { + throw new Error(`pause failed: ${NO_DEBUG_TARGET_MARKER}`); + } + return {}; + }); + + const result = await operations.pause('test-session', 1); + + expect(result.success).toBe(false); + expect(result.error).toContain(NO_DEBUG_TARGET_MARKER); + expect(result.error).toContain(why); + }); + + it('drops the why once the launch is over — the flag describes a launch that is no longer running', async () => { + mockSession.state = SessionState.STOPPED; + mockSession.debuggerDisabled = true; + + const result = await operations.getStackTraceDetailed('test-session'); + + expect(result.frames).toEqual([]); + expect(result.note ?? '').not.toContain(why); }); it('says why stepping and continuing find nothing paused', async () => { From 523d71eef934cd8409f733381277d2098be3af7f Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 17 Sep 2026 13:14:01 -0400 Subject: [PATCH 3/7] fix(session): a step from a noDebug pause proves nothing either; the decision is consulted only while running or paused (#749 review 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-review of #751 (eight findings, all taken): - 'step' was in the set of stops that clear the decision, but the js pause the PR measured lands because the inspector is attached, and so does a step taken from it (measured: reason 'step', breakpoints still unbound) — one step_over after the pause put every surface back in the pre-#749 state. The set is now the user-asked stops (breakpoint family, exception) plus 'entry'; the e2e steps after the pause and checks the record survives. - A launch refused before the proxy existed (the MSVC-toolchain branch) moves the session back to CREATED with the record intact, and the gate only excluded terminal states. isDebuggerOff() now means running or paused, exactly as the docs said. - stoppedAnyway had dropped the PAUSED clause, so a launch ending paused on a 'pause' stop could say no stop can arrive. Restored: a paused launch never claims that, whatever the record says about breakpoints. - withDebuggerOffWhy appends to the adapter's own error; when the message will not take the append (getter-only), it wraps with the original as the cause instead of throwing from inside the pause path. - The tool-reference bullet said "no stop ever arrives" and, four sentences on, that js-debug lands a pause: "no breakpoint, exception or entry stop ever arrives". - The e2e comment said the pause makes the session forget the decision while the assertion checked the opposite; the comment matches now. - handleListDebugSessions re-gated a field SessionStore.getAll() had already gated; it mirrors the field. - USER_BREAK_REASONS moved next to BREAKPOINT_STOP_REASONS in @debugmcp/shared (the core re-exports it); DEBUGGER_ON_STOP_REASONS is that set plus 'entry' rather than a third hand-built union. Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/749.fixed.md | 2 +- docs/tool-reference.md | 4 +- packages/shared/src/index.ts | 2 +- .../shared/src/interfaces/adapter-policy.ts | 7 +++ packages/shared/src/models/index.ts | 12 ++--- src/server/handlers/session-tools.ts | 6 +-- src/session/debugger-off.ts | 30 ++++++----- src/session/execution/execution-controller.ts | 15 ++++-- src/session/launch/debug-launcher.ts | 7 ++- src/session/session-manager-core.ts | 9 ++-- .../server/handlers/session-tools.test.ts | 6 +-- .../session-manager-nodebug-warning.test.ts | 54 ++++++++++++++++++- .../session/session-store-projection.test.ts | 11 ++-- .../mcp-server-break-on-exceptions.test.ts | 18 ++++--- ...ession-manager-operations-coverage.test.ts | 20 +++++++ 15 files changed, 153 insertions(+), 50 deletions(-) diff --git a/changelog.d/749.fixed.md b/changelog.d/749.fixed.md index 7bc22251b..6af375e0c 100644 --- a/changelog.d/749.fixed.md +++ b/changelog.d/749.fixed.md @@ -1 +1 @@ -**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while it is running or paused — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (breakpoint, exception, entry, step): that proves the debugger on for the adapter build. A pause does not clear it — js-debug lands a user pause under the flag with its breakpoints still unbound. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) +**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while it is running or paused — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (breakpoint, exception, entry): that proves the debugger on for the adapter build. A pause, or a step taken from one, does not clear it — js-debug lands both under the flag with its breakpoints still unbound. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 96c50a2b1..0cbb352e8 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -120,7 +120,7 @@ Lists all active debugging sessions. **`lastStop`:** present while the session is `paused` (the stop it is at: `reason`, `threadId`, `timestamp`, the adapter's `description`/`text`, and `exceptionInfo` for exception stops) and after it reaches `stopped`/`error` (the last stop before it ended). A `running` session never carries one, so a poller that calls this after `continue_execution` or a step sees `state: "running"` with no stop record until the next stop lands — the record of the stop it just left is not repeated as if the program were still paused. -**`debuggerDisabled`:** present (`true`) while the current launch runs with the debugger off — `dapLaunchArgs.noDebug: true` on an adapter that honours it (see `start_debugging`) — and the session is `running` or `paused`. Cleared by the next launch or attach, and by a `stopped` event only a live debugger produces (breakpoint, exception, entry, step; not a pause). Omitted once the session is `stopped` or in `error` (issue #749). +**`debuggerDisabled`:** present (`true`) while the current launch runs with the debugger off — `dapLaunchArgs.noDebug: true` on an adapter that honours it (see `start_debugging`) — and the session is `running` or `paused`. Cleared by the next launch or attach, and by a `stopped` event only a live debugger produces (breakpoint, exception, entry; not a pause or a step). Omitted outside `running`/`paused` (issue #749). Errored sessions include optional `diagnostics` with the current launch attempt's server-host `proxyLogPath` and remote-safe `proxyLogResource`. The record is retained for proxy initialization failures and for proxy/adapter deaths after initialization, and is cleared when a new launch or attach attempt begins. @@ -369,7 +369,7 @@ Starts debugging a script. - `dapLaunchArgs` (object, optional): Standard DAP launch arguments: - `stopOnEntry` (boolean): Stop at first line (default `false` — the opposite of attach, which pauses unless `stopOnEntry` is `false`) - `justMyCode` (boolean): Debug only user code (default `true`). JavaScript launch: `true` blackboxes `node_modules` through js-debug's `skipFiles` and keeps js-debug's smart-stepper on, so a pause or step that lands in skipped code (Node internals, `node_modules`) is stepped through and may never land (`pending: true`, with an explanation); `false` drops `node_modules` from the skip list and turns the stepper off, so steps land inside dependencies and `pause_execution` lands as soon as any JavaScript runs (issue #678). A caller `skipFiles` replaces the default list. Source maps are on for every JavaScript launch, `.js` programs included — stops report `src/*.ts` when maps and sources are present; `adapterLaunchConfig: { sourceMaps: false }` opts out (issue #684) - - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off and names what will not fire, in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint") and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event only a live debugger produces (breakpoint, exception, entry, step) proves the debugger on for that adapter build and clears the decision; a pause does not — js-debug lands a user pause under the flag while its breakpoints stay unbound — and once the session is `stopped` or in `error` the decision is no longer reported or consulted (issue #749). + - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no breakpoint, exception or entry stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off and names what will not fire, in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint") and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event only a live debugger produces (breakpoint, exception, entry) proves the debugger on for that adapter build and clears the decision; a pause, or a step taken from one, does not — js-debug lands both under the flag while its breakpoints stay unbound — and outside `running`/`paused` (over, or never launched) the decision is neither reported nor consulted (issue #749). - Additional DAP launch keys (`program`, `cwd`, `env`, language-specific options) pass through to the adapter. Top-level parameters do **not** belong here: a nested `breakOnExceptions` is honored as an alias (the top-level value wins if both are given) and reported via a `warning` in the response; other misplaced top-level keys (`dryRunSpawn`, `sessionId`, `scriptPath`, `adapterLaunchConfig`) are stripped with a warning instead of silently riding into the launch config. - `adapterLaunchConfig` (object, optional): Adapter-specific launch configuration overrides. Use this for language-specific settings that go beyond standard DAP arguments (e.g., `mainClass` and `classpath` for Java, `buildCommand` for Rust). For Rust, `_adapterSettings` passes through to CodeLLDB (issue #441) — e.g. `{"_adapterSettings": {"scriptConfig": {"lang": {"rust": {"sysroot": "/path"}}}}}` points the Rust formatter lookup at an explicit sysroot; the `CODELLDB_RUST_SYSROOT` env var does the same without per-launch config (a user-supplied `_adapterSettings` value wins over the env var). - `dryRunSpawn` (boolean, optional): Test spawn without actually starting diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 9cad7b34e..f67c43825 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -193,7 +193,7 @@ export type { QueuedDapCommand, PendingStopContext } from './interfaces/adapter-policy.js'; -export { BREAKPOINT_STOP_REASONS } from './interfaces/adapter-policy.js'; +export { BREAKPOINT_STOP_REASONS, USER_BREAK_REASONS } from './interfaces/adapter-policy.js'; export { DefaultAdapterPolicy, resolveExceptionFilters, diff --git a/packages/shared/src/interfaces/adapter-policy.ts b/packages/shared/src/interfaces/adapter-policy.ts index daee61e13..0b9a62de7 100644 --- a/packages/shared/src/interfaces/adapter-policy.ts +++ b/packages/shared/src/interfaces/adapter-policy.ts @@ -57,6 +57,13 @@ export const BREAKPOINT_STOP_REASONS: ReadonlySet = new Set([ 'instruction breakpoint' ]); +/** + * Stop reasons the user asked for: the breakpoint family plus an exception + * the user asked to break on. The first-stop auto-continue must never + * swallow one, and one is proof a debugger is live (issues #749, #746). + */ +export const USER_BREAK_REASONS: ReadonlySet = new Set([...BREAKPOINT_STOP_REASONS, 'exception']); + /** * Context passed to AdapterPolicy.normalizeStopReason (issues #260/#302). * See that method's doc comment for the completeness rules. diff --git a/packages/shared/src/models/index.ts b/packages/shared/src/models/index.ts index bf5523c0f..2d7a80a37 100644 --- a/packages/shared/src/models/index.ts +++ b/packages/shared/src/models/index.ts @@ -437,13 +437,13 @@ export interface DebugSessionInfo { * carried `noDebug: true` and the adapter honours it (issue #710), so no * breakpoint can bind and no stop is expected. Set by the launcher, reset * per launch and attach, and cleared by a `stopped` event only a live - * debugger produces (breakpoint, exception, entry, step — not a pause, - * which js-debug lands under the flag with breakpoints still off): such a - * stop proves the debugger on for this adapter build. Projected, and - * consulted by the later surfaces (set_breakpoint, list_breakpoints, + * debugger produces (breakpoint, exception, entry — not a pause or a + * step, which js-debug lands under the flag with breakpoints still off): + * such a stop proves the debugger on for this adapter build. Projected, + * and consulted by the later surfaces (set_breakpoint, list_breakpoints, * pause, stepping, inspection), only while the session is running or - * paused — once it is stopped or in error the record describes nothing - * that is running (issue #749). + * paused — over, or not yet launched, the record describes nothing that + * is running (issue #749). */ debuggerDisabled?: boolean; /** diff --git a/src/server/handlers/session-tools.ts b/src/server/handlers/session-tools.ts index 125594a51..4a583f970 100644 --- a/src/server/handlers/session-tools.ts +++ b/src/server/handlers/session-tools.ts @@ -10,7 +10,6 @@ import { isContainerRuntime } from '../../utils/container-path-utils.js'; import type { ToolContext, ToolHandler } from '../tool-context.js'; import { assertPlainObjectArg, requireSessionId } from '../tool-validation.js'; import { carriesLastStop, successWarning } from './shared.js'; -import { isDebuggerOff } from '../../session/debugger-off.js'; import { failureResult, jsonResult, rethrowAsMcpError, type ToolResult } from '../tool-result.js'; export const createDebugSessionTool: ToolHandler = async (ctx, args) => { @@ -176,8 +175,9 @@ export async function handleListDebugSessions(ctx: ToolContext): Promise = new Set([ - ...BREAKPOINT_STOP_REASONS, - 'exception', - 'entry', - 'step' + ...USER_BREAK_REASONS, + 'entry' ]); /** * Whether the decision applies now: recorded for this launch, and the - * launch is still running or paused. Once the session is stopped or in - * error the record describes nothing that is running — a breakpoint set - * then is an ordinary queued one for the next launch. + * launch is running or paused. Over (stopped, error) or not yet launched + * (created — a launch refused before the proxy existed leaves the record + * behind — initializing, ready), it describes nothing that is running: a + * breakpoint set then is an ordinary queued one for the next launch. */ export function isDebuggerOff(session: DebuggerOffView): boolean { - return session.debuggerDisabled === true && !isTerminalSessionState(session.state); + return ( + session.debuggerDisabled === true && + (session.state === SessionState.RUNNING || session.state === SessionState.PAUSED) + ); } /** The why to place beside an answer, when the decision applies; else nothing. */ diff --git a/src/session/execution/execution-controller.ts b/src/session/execution/execution-controller.ts index 3c926c547..085496877 100644 --- a/src/session/execution/execution-controller.ts +++ b/src/session/execution/execution-controller.ts @@ -155,10 +155,19 @@ function notPausedError(session: DebuggerOffView): string { function withDebuggerOffWhy(session: DebuggerOffView, error: unknown, message: string): { error: Error; message: string } { const err = error instanceof Error ? error : new Error(message); const why = debuggerOffWhy(session); - if (why) { - err.message = `${err.message} (${why})`; + if (!why) { + return { error: err, message: err.message }; + } + const appended = `${err.message} (${why})`; + try { + // The adapter's own object, so its stack and any structured props + // travel; the stack's first line keeps the original message. + err.message = appended; + return { error: err, message: appended }; + } catch { + // A getter-only message: wrap instead, keeping the original as the cause. + return { error: new Error(appended, { cause: err }), message: appended }; } - return { error: err, message: err.message }; } export class ExecutionController { diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index 66f08461b..099e3dd60 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -541,8 +541,11 @@ export class DebugLauncher { // recorded decision on a stop only a live debugger produces (issue // #749), so the launch response and the record cannot disagree. Then // the debugger was on: keep the ordinary diagnostics and say the flag - // had no effect rather than that no stop can come. - const stoppedAnyway = debuggerOff && finalSession.debuggerDisabled !== true; + // had no effect rather than that no stop can come. A launch that ends + // paused on any stop at all may not claim no stop can come either, + // whatever the record says about breakpoints. + const stoppedAnyway = + debuggerOff && (finalSession.debuggerDisabled !== true || finalState === SessionState.PAUSED); const noDebugNote = stoppedAnyway ? buildNoDebugLaunchWarning(finalSession, { noDebug }, breakOnExceptions, false) : noDebugWarning; diff --git a/src/session/session-manager-core.ts b/src/session/session-manager-core.ts index 587384b8a..4f6c482d4 100644 --- a/src/session/session-manager-core.ts +++ b/src/session/session-manager-core.ts @@ -8,7 +8,7 @@ import { isTerminalSessionState, AdapterPolicy, SessionOutputEntry, redactSecretsInString } from '@debugmcp/shared'; import type { Breakpoint, FunctionBreakpoint, StackFrame } from '@debugmcp/shared'; -import { BREAKPOINT_STOP_REASONS } from '@debugmcp/shared'; +import { USER_BREAK_REASONS } from '@debugmcp/shared'; import { DEBUGGER_ON_STOP_REASONS } from './debugger-off.js'; import { isRedactionEnabled } from '../utils/redaction-mode.js'; import { ValidationResultCache } from '../utils/language-availability.js'; @@ -51,11 +51,8 @@ import { samePath } from './breakpoints/hit-verification.js'; -/** - * Stop reasons the first-stop auto-continue must never swallow: the shared - * breakpoint family plus an exception the user asked to break on. - */ -export const USER_BREAK_REASONS: ReadonlySet = new Set([...BREAKPOINT_STOP_REASONS, 'exception']); +/** Re-exported for the execution controller; defined beside the breakpoint family in `@debugmcp/shared`. */ +export { USER_BREAK_REASONS }; // Custom launch arguments interface extending DebugProtocol.LaunchRequestArguments export interface CustomLaunchRequestArguments extends DebugProtocol.LaunchRequestArguments { diff --git a/tests/core/unit/server/handlers/session-tools.test.ts b/tests/core/unit/server/handlers/session-tools.test.ts index 321aebe18..034f14c1d 100644 --- a/tests/core/unit/server/handlers/session-tools.test.ts +++ b/tests/core/unit/server/handlers/session-tools.test.ts @@ -79,10 +79,11 @@ describe('session tool handlers', () => { it('reports debuggerDisabled for a launch running with the debugger off (issue #749)', async () => { const now = new Date(); + // SessionStore.getAll() is the one gate on the field (running or paused + // only); the handler mirrors what it was given. ctx.sessionManager.getAllSessions.mockReturnValue([ { id: 'off', name: 'o', language: 'python', state: 'running', createdAt: now, debuggerDisabled: true }, - { id: 'on', name: 'n', language: 'python', state: 'running', createdAt: now }, - { id: 'over', name: 'v', language: 'python', state: 'stopped', createdAt: now, debuggerDisabled: true } + { id: 'on', name: 'n', language: 'python', state: 'running', createdAt: now } ]); const result = await handleListDebugSessions(ctx); @@ -91,7 +92,6 @@ describe('session tool handlers', () => { expect(byId.off.debuggerDisabled).toBe(true); expect(byId.on).not.toHaveProperty('debuggerDisabled'); - expect(byId.over).not.toHaveProperty('debuggerDisabled'); }); }); }); diff --git a/tests/core/unit/session/session-manager-nodebug-warning.test.ts b/tests/core/unit/session/session-manager-nodebug-warning.test.ts index d8fc39ca3..3e98cad33 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -344,6 +344,26 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(warningOf(result)).toMatch(/noDebug is true/); }); + it("never claims no stop can come from a launch that ended paused — even on a 'pause' the record survives", async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + const proxy = dependencies.mockProxyManager; + proxy.start = vi.fn().mockImplementation(async (startConfig) => { + setMockProxyRunning(proxy, true); + proxy.startCalls.push(startConfig); + proxy.emit('adapter-configured'); + proxy.emit('initialized'); + proxy.emit('stopped', 1, 'pause', { reason: 'pause', threadId: 1 }); + }) as MockProxyManager['start']; + + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(result.state).toBe(SessionState.PAUSED); + expect(warningOf(result)).not.toMatch(/no stop can arrive/); + // ...while the record — breakpoints still cannot bind — is kept. + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + }); + it('believes a stop that arrived anyway over the policy pin', async () => { // The mock adapter stops at its breakpoint whatever the flag says — the // override pinned honoursNoDebug, so this is what a wrong pin looks like @@ -507,7 +527,39 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); }); - it.each(['breakpoint', 'function breakpoint', 'exception', 'entry', 'step'])( + it('survives a step taken from that pause — it proves exactly as much as the pause did', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + + // Measured on js-debug: pause lands, step_over from it lands with + // reason 'step', and line breakpoints still cannot bind. + dependencies.mockProxyManager.simulateEvent('stopped', 1, 'pause', { reason: 'pause', threadId: 1 }); + await vi.runAllTimersAsync(); + dependencies.mockProxyManager.simulateEvent('continued'); + dependencies.mockProxyManager.simulateEvent('stopped', 1, 'step', { reason: 'step', threadId: 1 }); + await vi.runAllTimersAsync(); + + expect(sessionManager.getSession(s.id)?.state).toBe(SessionState.PAUSED); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + }); + + it('is not consulted while the session is merely created — a launch that failed before the proxy leaves it CREATED', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + + // The MSVC-toolchain refusal path: back to CREATED with the record intact. + sessionManager.getSession(s.id)!.state = SessionState.CREATED; + + const listed = sessionManager.getAllSessions().find((x) => x.id === s.id); + expect(listed).not.toHaveProperty('debuggerDisabled'); + }); + + it.each(['breakpoint', 'function breakpoint', 'exception', 'entry'])( "is cleared by a '%s' stop — one a disabled debugger cannot produce", async (reason) => { pinPolicy({ honoursNoDebug: true }); diff --git a/tests/core/unit/session/session-store-projection.test.ts b/tests/core/unit/session/session-store-projection.test.ts index 12ec8d1d1..aceacacde 100644 --- a/tests/core/unit/session/session-store-projection.test.ts +++ b/tests/core/unit/session/session-store-projection.test.ts @@ -64,9 +64,14 @@ describe('SessionStore.getAll() debuggerDisabled projection (issue #749)', () => store.get(id)!.debuggerDisabled = undefined; expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); - // Once the launch is over the flag describes nothing that is running. + // Only a running or paused launch: over (stopped/error), or never + // launched (created/initializing), the record describes nothing running. store.get(id)!.debuggerDisabled = true; - store.get(id)!.state = SessionState.STOPPED; - expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); + for (const state of [SessionState.STOPPED, SessionState.ERROR, SessionState.CREATED, SessionState.INITIALIZING]) { + store.get(id)!.state = state; + expect(store.getAll().find((s) => s.id === id)!, state).not.toHaveProperty('debuggerDisabled'); + } + store.get(id)!.state = SessionState.PAUSED; + expect(store.getAll().find((s) => s.id === id)!.debuggerDisabled).toBe(true); }); }); diff --git a/tests/e2e/mcp-server-break-on-exceptions.test.ts b/tests/e2e/mcp-server-break-on-exceptions.test.ts index 0bce4b43b..8b0e5a87d 100644 --- a/tests/e2e/mcp-server-break-on-exceptions.test.ts +++ b/tests/e2e/mcp-server-break-on-exceptions.test.ts @@ -580,21 +580,27 @@ describe('Break-on-exception (issue #220)', () => { expect(step.error).toMatch(why); // ...and a pause is still sent to js-debug. Measured: js-debug lands - // it even under noDebug (the inspector is attached; only the debug - // domains are off), and that stop proves the debugger on — the session - // forgets the decision. Should a js-debug build refuse or never land - // it, the why rides on that answer instead. + // it even under noDebug (the inspector is attached; line breakpoints + // still cannot bind), and so does a step taken from it — neither + // proves anything about breakpoints, so the session keeps the + // decision. Should a js-debug build refuse or never land the pause, + // the why rides on that answer instead. // callToolSafely: a refusal reaches the wire as an MCP error, which // callTool would throw rather than return. const pause = await callToolSafely(mcpClient!, 'pause_execution', { sessionId }) as { success?: boolean; state?: string; error?: unknown; message?: string; data?: { message?: string }; }; if (pause.success && pause.state === 'paused') { - // A pause proves nothing about breakpoints: the session keeps the - // decision, and the surfaces keep explaining. const paused = await getSessionSnapshot(mcpClient!, sessionId); expect(paused?.state).toBe('paused'); expect(paused?.debuggerDisabled).toBe(true); + + const stepped = await callToolSafely(mcpClient!, 'step_over', { sessionId }) as { success?: boolean; state?: string }; + if (stepped.success && stepped.state === 'paused') { + const afterStep = await getSessionSnapshot(mcpClient!, sessionId); + expect(afterStep?.lastStop?.reason).toBe('step'); + expect(afterStep?.debuggerDisabled).toBe(true); + } } else { expect(`${String(pause.error ?? '')} ${pause.message ?? ''} ${pause.data?.message ?? ''}`).toMatch(why); } diff --git a/tests/unit/session-manager-operations-coverage.test.ts b/tests/unit/session-manager-operations-coverage.test.ts index 7d4d2c801..ac1ac6a9f 100644 --- a/tests/unit/session-manager-operations-coverage.test.ts +++ b/tests/unit/session-manager-operations-coverage.test.ts @@ -4494,6 +4494,26 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () expect((thrown as Error & { code?: string }).code).toBe('E_NODEBUG'); }); + it('wraps a refusal whose message cannot be appended to, keeping it as the cause', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.debuggerDisabled = true; + const frozen = new Error('Not supported in noDebug mode.'); + Object.defineProperty(frozen, 'message', { get: () => 'Not supported in noDebug mode.' }); + mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { + if (command === 'pause') { + throw frozen; + } + return {}; + }); + + const thrown = await operations.pause('test-session', 1).then( + () => { throw new Error('expected a rejection'); }, + (err: unknown) => err as Error & { cause?: unknown } + ); + expect(thrown.message).toBe(`Not supported in noDebug mode. (${why})`); + expect(thrown.cause).toBe(frozen); + }); + it('carries the why on a pause that found no debug target yet (js-debug before the child adopts)', async () => { mockSession.state = SessionState.RUNNING; mockSession.debuggerDisabled = true; From d7855b30e39c6953432be720a0e691fac4916a1d Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 17 Sep 2026 13:58:42 -0400 Subject: [PATCH 4/7] =?UTF-8?q?fix(session):=20proof=20the=20debugger=20is?= =?UTF-8?q?=20on=20is=20the=20adapter's=20own=20breakpoint=20reason,=20a?= =?UTF-8?q?=20hit,=20an=20exception=20or=20an=20entry=20=E2=80=94=20and=20?= =?UTF-8?q?the=20launch=20warning=20stops=20claiming=20no=20stop=20can=20a?= =?UTF-8?q?rrive=20(#749=20review=203)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third pass on #751 (twelve findings; eleven taken, one declined below): - js-debug relabels a `debugger;` statement stop ('pause', "Paused on debugger statement") to 'breakpoint', and under noDebug that pause lands (measured) — so the clear must key on what the adapter itself reported: stopProvesDebuggerOn() = hitBreakpointIds present, or an exception or entry stop, or a breakpoint reason the adapter called one (raw and normalized). An uncaught throw under js noDebug was measured not to stop, so 'exception' stays as proof. - The restored PAUSED clause in stoppedAnyway made the launch response say "no effect … breakpoints work as usual" while the record said they cannot bind. The root was #747's wording — "the debugger is disabled … and no stop can arrive" — refuted for js by a pause, a step and a `debugger;` statement. The warning now says the debugger is off for this launch and names what will not fire, so the record and the response agree, and the PAUSED clause goes again. - The raw record is now `launchDebuggerOff` on ManagedSession, distinct from the projected `debuggerDisabled` on DebugSessionInfo, so no handler can read the ungated value by accident. - isDebuggerOff() includes INITIALIZING: the proxy is up and a breakpoint set in that window still goes to the adapter, whose "Unbound breakpoint" needs the why. - A paused session gets only the clause still true of it — breakpoints cannot bind — not "no stop is expected". - The debugger-off pending-pause message keeps the policy's own explanation: on js-debug the pause can land under the flag, and #678's smart-stepper advice is what makes it. - withDebuggerOffWhy no longer mutates the adapter's error: a new Error with the why, the original untouched as the cause. - The composed "not paused" texts live in error-messages.ts (notPaused, cannotEvaluateNotPaused, stackTraceNotPaused, noStackFramesNotPaused, withDebuggerOffWhy, pausePendingDebuggerOff); one debuggerOffWhyFor(ctx, id) in handlers/shared.ts replaces three ctx→session→why copies; the USER_BREAK_REASONS re-export shim is gone. - Declined: treating an auto-continued 'pause'-reason entry stop as proof the flag was ignored (a stale-pin js-debug build stopping at entry as 'pause' while the launcher neutralised stopOnEntry) — the record self-corrects on the first breakpoint the adapter reports hit, and reading a pause as proof is the bug the round fixed. Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/710.fixed.md | 2 +- changelog.d/749.fixed.md | 2 +- docs/tool-reference.md | 4 +- packages/shared/src/models/index.ts | 17 ++-- src/server/handlers/breakpoint-tools.ts | 12 +-- src/server/handlers/inspection-tools.ts | 15 ++-- src/server/handlers/shared.ts | 11 +++ src/session/attach/attach-controller.ts | 2 +- src/session/breakpoints/launch-warnings.ts | 4 +- src/session/debugger-off.ts | 75 ++++++++++++------ src/session/execution/execution-controller.ts | 29 +++---- .../inspection/expression-evaluator.ts | 8 +- .../inspection/frame-anchor-resolver.ts | 9 +-- src/session/launch/debug-launcher.ts | 13 ++- src/session/session-manager-core.ts | 14 ++-- src/session/session-store.ts | 8 ++ src/utils/error-messages.ts | 61 +++++++++++++- .../server/handlers/inspection-tools.test.ts | 2 +- ...server-breakpoint-management-tools.test.ts | 6 +- .../unit/server/server-control-tools.test.ts | 27 ++++++- .../session-manager-nodebug-warning.test.ts | 79 ++++++++++++++----- .../session/session-store-projection.test.ts | 20 +++-- ...ession-manager-operations-coverage.test.ts | 70 ++++++++-------- 23 files changed, 310 insertions(+), 180 deletions(-) diff --git a/changelog.d/710.fixed.md b/changelog.d/710.fixed.md index 5d95b0aa2..6cee12878 100644 --- a/changelog.d/710.fixed.md +++ b/changelog.d/710.fixed.md @@ -1 +1 @@ -**A `noDebug` launch says what it did to the debugger instead of blaming your breakpoints** — `dapLaunchArgs.noDebug: true` is DAP's "launch without enabling debugging", and where the adapter honours it no breakpoint binds and no stop ever arrives; the session then read as a mystery — a short script ended `stopped` with the #467 "check the file path and line" warning for breakpoints that were never going to bind, a server stayed `running` while the caller waited for a stop that could not come. Whether the flag turns the debugger off is now the adapter policy's word (`honoursNoDebug`), measured rather than assumed: js-debug honours it, and so do debugpy, Delve and CodeLLDB (their launches currently fail under it — #746); rdbg, netcoredbg and the Java bridge ignore it, and the rust launch transform never forwards it. Where the debugger is off, `start_debugging` warns when the flag is set alongside something the caller asked to stop on — line or function breakpoints, an *explicit* `breakOnExceptions` other than `none` (the policy default does not count, so a deliberate plain run stays silent), or `stopOnEntry` — naming each thing that will not fire, withholds the breakpoint-shaped launch warnings (#308, #467, #469) that presuppose a debugger, and no longer waits for an entry stop that cannot come. Where the adapter ignores the flag, the warning says it had no effect and the usual diagnostics stay. The flag is read the way the adapter will see it (`adapterLaunchConfig` over `dapLaunchArgs` over the server defaults), and the decision lives in the launcher, so `restart_debugging` and a `dryRunSpawn` configuration check carry the same warning; the `dapLaunchArgs` schema documents the flag. Found by the review of PR #706 (#710) +**A `noDebug` launch says what it did to the debugger instead of blaming your breakpoints** — `dapLaunchArgs.noDebug: true` is DAP's "launch without enabling debugging", and where the adapter honours it no breakpoint binds and no breakpoint, exception or entry stop ever arrives; the session then read as a mystery — a short script ended `stopped` with the #467 "check the file path and line" warning for breakpoints that were never going to bind, a server stayed `running` while the caller waited for a stop that could not come. Whether the flag turns the debugger off is now the adapter policy's word (`honoursNoDebug`), measured rather than assumed: js-debug honours it, and so do debugpy, Delve and CodeLLDB (their launches currently fail under it — #746); rdbg, netcoredbg and the Java bridge ignore it, and the rust launch transform never forwards it. Where the debugger is off, `start_debugging` warns when the flag is set alongside something the caller asked to stop on — line or function breakpoints, an *explicit* `breakOnExceptions` other than `none` (the policy default does not count, so a deliberate plain run stays silent), or `stopOnEntry` — naming each thing that will not fire, withholds the breakpoint-shaped launch warnings (#308, #467, #469) that presuppose a debugger, and no longer waits for an entry stop that cannot come. Where the adapter ignores the flag, the warning says it had no effect and the usual diagnostics stay. The flag is read the way the adapter will see it (`adapterLaunchConfig` over `dapLaunchArgs` over the server defaults), and the decision lives in the launcher, so `restart_debugging` and a `dryRunSpawn` configuration check carry the same warning; the `dapLaunchArgs` schema documents the flag. Found by the review of PR #706 (#710) diff --git a/changelog.d/749.fixed.md b/changelog.d/749.fixed.md index 6af375e0c..9eb7e821d 100644 --- a/changelog.d/749.fixed.md +++ b/changelog.d/749.fixed.md @@ -1 +1 @@ -**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while it is running or paused — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (breakpoint, exception, entry): that proves the debugger on for the adapter build. A pause, or a step taken from one, does not clear it — js-debug lands both under the flag with its breakpoints still unbound. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) +**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while the launch is live — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (a breakpoint the adapter itself reports or names in `hitBreakpointIds`, an exception, an entry stop): that proves the debugger on for the adapter build. A pause, a step taken from one, or a `debugger;` statement does not clear it — js-debug lands all three under the flag with its breakpoints still unbound — and a paused session is told only that breakpoints cannot bind. The `start_debugging` warning itself no longer claims "no stop can arrive": it says the debugger is off for this launch and names what will not fire. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 0cbb352e8..068c2dd24 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -120,7 +120,7 @@ Lists all active debugging sessions. **`lastStop`:** present while the session is `paused` (the stop it is at: `reason`, `threadId`, `timestamp`, the adapter's `description`/`text`, and `exceptionInfo` for exception stops) and after it reaches `stopped`/`error` (the last stop before it ended). A `running` session never carries one, so a poller that calls this after `continue_execution` or a step sees `state: "running"` with no stop record until the next stop lands — the record of the stop it just left is not repeated as if the program were still paused. -**`debuggerDisabled`:** present (`true`) while the current launch runs with the debugger off — `dapLaunchArgs.noDebug: true` on an adapter that honours it (see `start_debugging`) — and the session is `running` or `paused`. Cleared by the next launch or attach, and by a `stopped` event only a live debugger produces (breakpoint, exception, entry; not a pause or a step). Omitted outside `running`/`paused` (issue #749). +**`debuggerDisabled`:** present (`true`) while the current launch runs with the debugger off — `dapLaunchArgs.noDebug: true` on an adapter that honours it (see `start_debugging`) — and the launch is live (`initializing`, `running` or `paused`). Cleared by the next launch or attach, and by a `stopped` event only a live debugger produces: a breakpoint the adapter itself reports (or names in `hitBreakpointIds`), an exception stop, or an entry stop — not a pause, a step, or a `debugger;` statement js-debug relabels as a breakpoint. Omitted otherwise (issue #749). Errored sessions include optional `diagnostics` with the current launch attempt's server-host `proxyLogPath` and remote-safe `proxyLogResource`. The record is retained for proxy initialization failures and for proxy/adapter deaths after initialization, and is cleared when a new launch or attach attempt begins. @@ -369,7 +369,7 @@ Starts debugging a script. - `dapLaunchArgs` (object, optional): Standard DAP launch arguments: - `stopOnEntry` (boolean): Stop at first line (default `false` — the opposite of attach, which pauses unless `stopOnEntry` is `false`) - `justMyCode` (boolean): Debug only user code (default `true`). JavaScript launch: `true` blackboxes `node_modules` through js-debug's `skipFiles` and keeps js-debug's smart-stepper on, so a pause or step that lands in skipped code (Node internals, `node_modules`) is stepped through and may never land (`pending: true`, with an explanation); `false` drops `node_modules` from the skip list and turns the stepper off, so steps land inside dependencies and `pause_execution` lands as soon as any JavaScript runs (issue #678). A caller `skipFiles` replaces the default list. Source maps are on for every JavaScript launch, `.js` programs included — stops report `src/*.ts` when maps and sources are present; `adapterLaunchConfig: { sourceMaps: false }` opts out (issue #684) - - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no breakpoint, exception or entry stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off and names what will not fire, in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint") and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event only a live debugger produces (breakpoint, exception, entry) proves the debugger on for that adapter build and clears the decision; a pause, or a step taken from one, does not — js-debug lands both under the flag while its breakpoints stay unbound — and outside `running`/`paused` (over, or never launched) the decision is neither reported nor consulted (issue #749). + - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no breakpoint, exception or entry stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off for this launch and names what will not fire — not that no stop of any kind can come — in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug honours it today; debugpy, Delve and CodeLLDB honour it as well but their launches currently fail under it (issue #746). **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint") and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event only a live debugger produces — a breakpoint the adapter itself reports (or names in `hitBreakpointIds`), an exception stop, an entry stop — proves the debugger on for that adapter build and clears the decision; a pause, a step taken from one, or a `debugger;` statement do not (js-debug lands all three under the flag while its breakpoints stay unbound; an uncaught throw does not stop). On a paused session the sentence keeps only the clause still true of it — breakpoints cannot bind — and outside a live launch (`initializing`/`running`/`paused`) the decision is neither reported nor consulted (issue #749). - Additional DAP launch keys (`program`, `cwd`, `env`, language-specific options) pass through to the adapter. Top-level parameters do **not** belong here: a nested `breakOnExceptions` is honored as an alias (the top-level value wins if both are given) and reported via a `warning` in the response; other misplaced top-level keys (`dryRunSpawn`, `sessionId`, `scriptPath`, `adapterLaunchConfig`) are stripped with a warning instead of silently riding into the launch config. - `adapterLaunchConfig` (object, optional): Adapter-specific launch configuration overrides. Use this for language-specific settings that go beyond standard DAP arguments (e.g., `mainClass` and `classpath` for Java, `buildCommand` for Rust). For Rust, `_adapterSettings` passes through to CodeLLDB (issue #441) — e.g. `{"_adapterSettings": {"scriptConfig": {"lang": {"rust": {"sysroot": "/path"}}}}}` points the Rust formatter lookup at an explicit sysroot; the `CODELLDB_RUST_SYSROOT` env var does the same without per-launch config (a user-supplied `_adapterSettings` value wins over the env var). - `dryRunSpawn` (boolean, optional): Test spawn without actually starting diff --git a/packages/shared/src/models/index.ts b/packages/shared/src/models/index.ts index 2d7a80a37..552e9cad8 100644 --- a/packages/shared/src/models/index.ts +++ b/packages/shared/src/models/index.ts @@ -433,17 +433,12 @@ export interface DebugSessionInfo { /** Present when the session is in ERROR because its proxy failed. */ diagnostics?: SessionFailureDiagnostics; /** - * True while the current launch runs with the debugger off: the launch - * carried `noDebug: true` and the adapter honours it (issue #710), so no - * breakpoint can bind and no stop is expected. Set by the launcher, reset - * per launch and attach, and cleared by a `stopped` event only a live - * debugger produces (breakpoint, exception, entry — not a pause or a - * step, which js-debug lands under the flag with breakpoints still off): - * such a stop proves the debugger on for this adapter build. Projected, - * and consulted by the later surfaces (set_breakpoint, list_breakpoints, - * pause, stepping, inspection), only while the session is running or - * paused — over, or not yet launched, the record describes nothing that - * is running (issue #749). + * Present (`true`) while the current launch runs with the debugger off: + * the launch carried `noDebug: true` and the adapter honours it (issue + * #710), so no breakpoint can bind. A projection of the session's + * `launchDebuggerOff` record, made only while the launch is live + * (initializing, running or paused) — see `isDebuggerOff` in + * `src/session/debugger-off.ts` (issue #749). */ debuggerDisabled?: boolean; /** diff --git a/src/server/handlers/breakpoint-tools.ts b/src/server/handlers/breakpoint-tools.ts index 109534e7e..af1865e74 100644 --- a/src/server/handlers/breakpoint-tools.ts +++ b/src/server/handlers/breakpoint-tools.ts @@ -13,8 +13,7 @@ import { import type { FunctionBreakpointRemoval } from '../../session/session-manager-operations.js'; import type { ToolContext, ToolHandler } from '../tool-context.js'; import { requireSessionId, type WithSessionId } from '../tool-validation.js'; -import { readLineContext } from './shared.js'; -import { debuggerOffWhy } from '../../session/debugger-off.js'; +import { debuggerOffWhyFor, readLineContext } from './shared.js'; import { failureResult, jsonResult, sessionErrorResultOrThrow, type ToolResult } from '../tool-result.js'; /** @@ -23,11 +22,7 @@ import { failureResult, jsonResult, sessionErrorResultOrThrow, type ToolResult } * its own answer is kept; a breakpoint it verified anyway needs no note. */ function debuggerOffNote(ctx: ToolContext, sessionId: string, verified: boolean): string | undefined { - if (verified) { - return undefined; - } - const session = ctx.sessionManager.getSession(sessionId); - return session ? debuggerOffWhy(session) : undefined; + return verified ? undefined : debuggerOffWhyFor(ctx, sessionId); } export const setBreakpointTool: ToolHandler = async (ctx, args) => { @@ -252,8 +247,7 @@ export const listBreakpointsTool: ToolHandler = async (ctx, args) => { // anyway is not contradicted. const anyUnverified = breakpoints.some((bp) => !bp.verified) || functionBreakpoints.some((bp) => !bp.verified); - const session = ctx.sessionManager.getSession(args.sessionId); - const why = anyUnverified && session ? debuggerOffWhy(session) : undefined; + const why = debuggerOffNote(ctx, args.sessionId, !anyUnverified); return jsonResult({ success: true, breakpoints, diff --git a/src/server/handlers/inspection-tools.ts b/src/server/handlers/inspection-tools.ts index d884360b6..484ab4293 100644 --- a/src/server/handlers/inspection-tools.ts +++ b/src/server/handlers/inspection-tools.ts @@ -5,10 +5,10 @@ import { ErrorCode as McpErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; import { SessionState } from '@debugmcp/shared'; import { SessionTerminatedError } from '../../errors/debug-errors.js'; -import { debuggerOffWhy } from '../../session/debugger-off.js'; +import { ErrorMessages } from '../../utils/error-messages.js'; import type { ToolContext, ToolHandler } from '../tool-context.js'; import { enforceExplicitNames, requireSessionId } from '../tool-validation.js'; -import { carriesLastStop, variablePayloadExtras } from './shared.js'; +import { carriesLastStop, debuggerOffWhyFor, variablePayloadExtras } from './shared.js'; import { failureResult, jsonResult, @@ -335,16 +335,13 @@ export async function handleGetLocalVariables(ctx: ToolContext, args: { sessionI // Distinguish "not paused" from "paused but the anchored thread has // no frames" — the latter used to claim the debugger may not be // paused while list_debug_sessions said paused (issue #465). - const sessionNow = ctx.sessionManager.getSession(args.sessionId); - // The why, when the launch runs with the debugger off (issue #749). - const why = sessionNow ? debuggerOffWhy(sessionNow) : undefined; - response.message = sessionNow?.state === SessionState.PAUSED + const sessionState = ctx.sessionManager.getSession(args.sessionId)?.state; + response.message = sessionState === SessionState.PAUSED ? 'The session is paused, but the anchored thread reported no stack frames. ' + 'Try get_stack_trace with a threadId from list_threads, or continue_execution ' + 'followed by pause_execution to re-anchor on a reportable thread.' - : why - ? `No stack frames available; ${why}.` - : 'No stack frames available. The debugger may not be paused.'; + // With the why, when the launch runs with the debugger off (issue #749). + : ErrorMessages.noStackFramesNotPaused(debuggerOffWhyFor(ctx, args.sessionId)); } else if (!result.scopeName) { response.message = 'No local scope found in the current frame.'; } else { diff --git a/src/server/handlers/shared.ts b/src/server/handlers/shared.ts index 63392ee1e..df5b1430c 100644 --- a/src/server/handlers/shared.ts +++ b/src/server/handlers/shared.ts @@ -7,6 +7,7 @@ import { isTerminalSessionState, REDACTION_NOTICE, SessionState, Variable } from import type { LineContext } from '../../utils/line-reader.js'; import { buildTruncationNotice, VariableTruncationSummary } from '../../session/variable-caps.js'; import type { ToolContext } from '../tool-context.js'; +import { debuggerOffWhy } from '../../session/debugger-off.js'; import type { DebugResult } from '../../session/session-manager-core.js'; /** @@ -27,6 +28,16 @@ export function carriesLastStop(state: SessionState | undefined): boolean { return state === SessionState.PAUSED || isTerminalSessionState(state); } +/** + * The debugger-off why for a session the handler names by id (issue #749): + * the gated sentence from `debuggerOffWhy`, or nothing when the session is + * unknown or the decision does not apply. + */ +export function debuggerOffWhyFor(ctx: ToolContext, sessionId: string): string | undefined { + const session = ctx.sessionManager.getSession(sessionId); + return session ? debuggerOffWhy(session) : undefined; +} + /** The line-context slice the breakpoint and step payloads embed. */ export type EmbeddedLineContext = Pick; diff --git a/src/session/attach/attach-controller.ts b/src/session/attach/attach-controller.ts index 6e349e229..9ba9b8cda 100644 --- a/src/session/attach/attach-controller.ts +++ b/src/session/attach/attach-controller.ts @@ -152,7 +152,7 @@ export class AttachController { // be reported as this attempt's once the session lands in ERROR (#720). session.lastStop = undefined; // An attach debugs regardless of any earlier noDebug launch (issue #749). - session.debuggerDisabled = undefined; + session.launchDebuggerOff = undefined; try { // For attach mode, we use a placeholder scriptPath diff --git a/src/session/breakpoints/launch-warnings.ts b/src/session/breakpoints/launch-warnings.ts index a76ace109..1a1c6ea65 100644 --- a/src/session/breakpoints/launch-warnings.ts +++ b/src/session/breakpoints/launch-warnings.ts @@ -118,8 +118,10 @@ export function buildNoDebugLaunchWarning( expected.length === 1 ? expected[0] : `${expected.slice(0, -1).join(', ')} and ${expected[expected.length - 1]}`; + // What is known: the flag, and what it keeps from firing. Not "no stop + // can arrive" — js-debug lands a user pause under the flag (issue #749). return ( - `noDebug is true, so the debugger is disabled for this launch and no stop can arrive: ` + + `noDebug is true, so the debugger is off for this launch: ` + `${list} will not fire. Drop noDebug to debug, or ignore this if you only meant to run the program` ); } diff --git a/src/session/debugger-off.ts b/src/session/debugger-off.ts index 94a69cdca..1d5428b0d 100644 --- a/src/session/debugger-off.ts +++ b/src/session/debugger-off.ts @@ -1,48 +1,73 @@ /** * The debugger-off decision a session carries for one launch (issue #749): - * `noDebug: true` on an adapter that honours it (issue #710). One gate and - * one sentence, read by every surface that would otherwise answer in - * debugger terms — set_breakpoint, list_breakpoints, pause, stepping, - * inspection, list_debug_sessions — so the wording and the gate cannot - * drift between them. + * `noDebug: true` on an adapter that honours it (issue #710). One gate, one + * rule for what clears it, and one sentence per state, read by every + * surface that would otherwise answer in debugger terms — set_breakpoint, + * list_breakpoints, pause, stepping, inspection, list_debug_sessions — so + * the wording and the gate cannot drift between them. */ -import { SessionState, USER_BREAK_REASONS } from '@debugmcp/shared'; +import type { DebugProtocol } from '@vscode/debugprotocol'; +import { BREAKPOINT_STOP_REASONS, SessionState } from '@debugmcp/shared'; import { ErrorMessages } from '../utils/error-messages.js'; -/** The slice of a session (or its public projection) the decision is read from. */ +/** The slice of a session the decision is read from. */ export interface DebuggerOffView { state: SessionState; - debuggerDisabled?: boolean; + launchDebuggerOff?: boolean; } /** - * Stops only a live debugger produces: a breakpoint or exception the user - * asked for, or an entry stop. A `stopped` with one of these reasons clears - * the decision — the adapter build debugs after all. Neither `pause` nor - * `step` is among them: measured on js-debug under `noDebug`, a user pause - * lands (the inspector is attached) and so does a step taken from it, while - * line breakpoints still cannot bind — they prove nothing about binding. + * Whether a stop proves the debugger is live after all — this adapter + * build ignores the flag, or a stale pin — and the record must go. Judged + * on what the adapter itself reported, not the policy's relabel. Measured + * on js-debug under `noDebug`: the inspector is attached, so a user pause + * lands, a step from it lands, and a `debugger;` statement pauses with the + * adapter's reason 'pause' (relabelled 'breakpoint' by the policy) — while + * line breakpoints still cannot bind and an uncaught throw does not stop. + * Proof, then, is: a breakpoint the adapter itself called one, or named in + * `hitBreakpointIds`; an exception stop; an entry stop. */ -export const DEBUGGER_ON_STOP_REASONS: ReadonlySet = new Set([ - ...USER_BREAK_REASONS, - 'entry' -]); +export function stopProvesDebuggerOn( + reason: string, + rawReason: string, + body: DebugProtocol.StoppedEvent['body'] | undefined +): boolean { + if ((body?.hitBreakpointIds?.length ?? 0) > 0) { + return true; + } + if (reason === 'entry' || reason === 'exception') { + return true; + } + return BREAKPOINT_STOP_REASONS.has(reason) && BREAKPOINT_STOP_REASONS.has(rawReason); +} /** * Whether the decision applies now: recorded for this launch, and the - * launch is running or paused. Over (stopped, error) or not yet launched - * (created — a launch refused before the proxy existed leaves the record - * behind — initializing, ready), it describes nothing that is running: a + * launch is live — initializing (the proxy is up; a breakpoint set now + * still goes to the adapter), running, or paused. Over (stopped, error) or + * never launched (created — a launch refused before the proxy existed + * leaves the record behind), it describes nothing that is running: a * breakpoint set then is an ordinary queued one for the next launch. */ export function isDebuggerOff(session: DebuggerOffView): boolean { return ( - session.debuggerDisabled === true && - (session.state === SessionState.RUNNING || session.state === SessionState.PAUSED) + session.launchDebuggerOff === true && + (session.state === SessionState.INITIALIZING || + session.state === SessionState.RUNNING || + session.state === SessionState.PAUSED) ); } -/** The why to place beside an answer, when the decision applies; else nothing. */ +/** + * The why to place beside an answer, when the decision applies; else + * nothing. A paused session gets the clause that is still true of it — + * breakpoints cannot bind — not "no stop is expected". + */ export function debuggerOffWhy(session: DebuggerOffView): string | undefined { - return isDebuggerOff(session) ? ErrorMessages.debuggerOffForLaunch : undefined; + if (!isDebuggerOff(session)) { + return undefined; + } + return session.state === SessionState.PAUSED + ? ErrorMessages.debuggerOffForLaunchPaused + : ErrorMessages.debuggerOffForLaunch; } diff --git a/src/session/execution/execution-controller.ts b/src/session/execution/execution-controller.ts index 085496877..cdac0f36f 100644 --- a/src/session/execution/execution-controller.ts +++ b/src/session/execution/execution-controller.ts @@ -38,6 +38,7 @@ import { NO_DEBUG_TARGET_MARKER, SessionLifecycleState, SessionState, + USER_BREAK_REASONS, type StackFrame } from '@debugmcp/shared'; import { DebugProtocol } from '@vscode/debugprotocol'; @@ -50,7 +51,6 @@ import type { StepResultData, StopLocation } from '../session-manager-core.js'; -import { USER_BREAK_REASONS } from '../session-manager-core.js'; import { debuggerOffWhy, isDebuggerOff, type DebuggerOffView } from '../debugger-off.js'; import { samePath } from '../breakpoints/hit-verification.js'; import type { ExecutionContext } from '../operations-context.js'; @@ -142,15 +142,13 @@ function isSameLine(a: StopLocation, b: StopLocation): boolean { * when the session's launch runs with the debugger off (issue #749). */ function notPausedError(session: DebuggerOffView): string { - const why = debuggerOffWhy(session); - return why ? `Not paused: ${why}` : 'Not paused'; + return ErrorMessages.notPaused(debuggerOffWhy(session)); } /** * A pause the adapter answered with an error, with the why beside the * adapter's own words when the launch runs with the debugger off (issue - * #749). The original error object is kept — its stack and any structured - * properties are the adapter's answer too. + * #749). The adapter's error is untouched and travels as the cause. */ function withDebuggerOffWhy(session: DebuggerOffView, error: unknown, message: string): { error: Error; message: string } { const err = error instanceof Error ? error : new Error(message); @@ -158,16 +156,8 @@ function withDebuggerOffWhy(session: DebuggerOffView, error: unknown, message: s if (!why) { return { error: err, message: err.message }; } - const appended = `${err.message} (${why})`; - try { - // The adapter's own object, so its stack and any structured props - // travel; the stack's first line keeps the original message. - err.message = appended; - return { error: err, message: appended }; - } catch { - // A getter-only message: wrap instead, keeping the original as the cause. - return { error: new Error(appended, { cause: err }), message: appended }; - } + const composed = ErrorMessages.withDebuggerOffWhy(err.message, why); + return { error: new Error(composed, { cause: err }), message: composed }; } export class ExecutionController { @@ -655,21 +645,22 @@ export class ExecutionController { this.ctx.logger.info( `[SessionManager pause] No stopped event within ${this.ctx.tunables.pauseGraceMs}ms grace window in session ${sessionId}; completing asynchronously` ); + const hint = await this.describePendingStop(session, sessionId, 'pause'); // With the debugger off for this launch the why is known (issue #749): - // one message that promises no stop, instead of the policy's guess at - // native code or a syscall. + // one message that promises no stop, with the policy's own explanation + // kept — on js-debug a pause can land under the flag, and #678's + // advice is what makes it. if (isDebuggerOff(session)) { return { success: true, state: session.state, data: { - message: ErrorMessages.pausePendingDebuggerOff(this.ctx.tunables.pauseGraceMs / 1000), + message: ErrorMessages.pausePendingDebuggerOff(this.ctx.tunables.pauseGraceMs / 1000, hint), pending: true } }; } const pausePending = ErrorMessages.pausePending(this.ctx.tunables.pauseGraceMs / 1000); - const hint = await this.describePendingStop(session, sessionId, 'pause'); return { success: true, state: session.state, diff --git a/src/session/inspection/expression-evaluator.ts b/src/session/inspection/expression-evaluator.ts index a2ca9a26c..154debe4c 100644 --- a/src/session/inspection/expression-evaluator.ts +++ b/src/session/inspection/expression-evaluator.ts @@ -9,6 +9,7 @@ */ import { getErrorMessage } from '../../errors/debug-errors.js'; import { debuggerOffWhy } from '../debugger-off.js'; +import { ErrorMessages } from '../../utils/error-messages.js'; import { buildRedactionNotice, isSensitiveName, @@ -132,13 +133,10 @@ export class ExpressionEvaluator { this.ctx.logger.warn( `[SM evaluateExpression ${sessionId}] Cannot evaluate: session not paused. State: ${session.state}` ); - // The why, when the launch runs with the debugger off (issue #749). - const why = debuggerOffWhy(session); return { success: false, - error: why - ? `Cannot evaluate: debugger not paused (${why})` - : 'Cannot evaluate: debugger not paused. Ensure the debugger is stopped at a breakpoint.', + // With the why, when the launch runs with the debugger off (issue #749). + error: ErrorMessages.cannotEvaluateNotPaused(debuggerOffWhy(session)), }; } diff --git a/src/session/inspection/frame-anchor-resolver.ts b/src/session/inspection/frame-anchor-resolver.ts index c44fc2186..66ba5a9d3 100644 --- a/src/session/inspection/frame-anchor-resolver.ts +++ b/src/session/inspection/frame-anchor-resolver.ts @@ -19,6 +19,7 @@ import path from 'path'; import type { IProxyManager } from '../../proxy/proxy-manager.js'; import type { ManagedSession } from '../session-store.js'; import { debuggerOffWhy } from '../debugger-off.js'; +import { ErrorMessages } from '../../utils/error-messages.js'; /** The frame fields a tool response names when it says which frame answered. */ export type FrameSummary = Pick; @@ -136,12 +137,8 @@ export class FrameAnchorResolver { } if (session.state !== SessionState.PAUSED) { this.ctx.logger.warn(`[FrameAnchor ${sessionId}] Session not paused: ${session.state}.`); - // The why, when the launch runs with the debugger off (issue #749). - const why = debuggerOffWhy(session); - return emptyResult( - `Session is not paused (state: ${session.state}); stack traces are only available while paused${why ? `; ${why}` : ''}.`, - threadId - ); + // With the why, when the launch runs with the debugger off (issue #749). + return emptyResult(ErrorMessages.stackTraceNotPaused(session.state, debuggerOffWhy(session)), threadId); } const effectiveThreadId = threadId ?? currentThreadId; diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index 099e3dd60..029fa91b4 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -244,7 +244,7 @@ export class DebugLauncher { session.lastStop = undefined; // The previous launch's debugger-off decision does not carry over // (issue #749); this attempt decides again below. - session.debuggerDisabled = undefined; + session.launchDebuggerOff = undefined; this.ctx.logger.info(`[SessionManager] Session ${sessionId} lifecycle state set to ACTIVE`); // Record the launch spec for restart_debugging BEFORE attempting the @@ -284,7 +284,7 @@ export class DebugLauncher { // per-launch reset (which runs inside proxyLauncher.start) is not the // place to clear it — the block above is. if (debuggerOff && !dryRunSpawn) { - session.debuggerDisabled = true; + session.launchDebuggerOff = true; } const noDebugWarning = buildNoDebugLaunchWarning( session, @@ -541,11 +541,10 @@ export class DebugLauncher { // recorded decision on a stop only a live debugger produces (issue // #749), so the launch response and the record cannot disagree. Then // the debugger was on: keep the ordinary diagnostics and say the flag - // had no effect rather than that no stop can come. A launch that ends - // paused on any stop at all may not claim no stop can come either, - // whatever the record says about breakpoints. - const stoppedAnyway = - debuggerOff && (finalSession.debuggerDisabled !== true || finalState === SessionState.PAUSED); + // had no effect rather than that the breakpoints will not fire. (A + // launch that ends paused on a pause or a step keeps the record and + // the warning: the flag still keeps its breakpoints from binding.) + const stoppedAnyway = debuggerOff && finalSession.launchDebuggerOff !== true; const noDebugNote = stoppedAnyway ? buildNoDebugLaunchWarning(finalSession, { noDebug }, breakOnExceptions, false) : noDebugWarning; diff --git a/src/session/session-manager-core.ts b/src/session/session-manager-core.ts index 4f6c482d4..4f3fc788e 100644 --- a/src/session/session-manager-core.ts +++ b/src/session/session-manager-core.ts @@ -9,7 +9,7 @@ import { } from '@debugmcp/shared'; import type { Breakpoint, FunctionBreakpoint, StackFrame } from '@debugmcp/shared'; import { USER_BREAK_REASONS } from '@debugmcp/shared'; -import { DEBUGGER_ON_STOP_REASONS } from './debugger-off.js'; +import { stopProvesDebuggerOn } from './debugger-off.js'; import { isRedactionEnabled } from '../utils/redaction-mode.js'; import { ValidationResultCache } from '../utils/language-availability.js'; import { SessionStore, ManagedSession } from './session-store.js'; @@ -51,8 +51,6 @@ import { samePath } from './breakpoints/hit-verification.js'; -/** Re-exported for the execution controller; defined beside the breakpoint family in `@debugmcp/shared`. */ -export { USER_BREAK_REASONS }; // Custom launch arguments interface extending DebugProtocol.LaunchRequestArguments export interface CustomLaunchRequestArguments extends DebugProtocol.LaunchRequestArguments { @@ -737,10 +735,12 @@ export abstract class SessionManagerCore extends EventEmitter { // A stop only a live debugger produces is stronger evidence than the // policy's noDebug pin: this adapter build debugs after all, so the // later surfaces must stop explaining themselves in debugger-off - // terms (issue #749). A pause is not such a stop — js-debug lands one - // under noDebug with breakpoints still off. - if (DEBUGGER_ON_STOP_REASONS.has(reason)) { - session.debuggerDisabled = undefined; + // terms (issue #749). Judged on what the adapter itself reported, not + // the policy's relabel: js-debug lands a pause, a step and a + // `debugger;` statement (relabelled 'breakpoint') under noDebug with + // its breakpoints still off. + if (stopProvesDebuggerOn(reason, rawReason, body)) { + session.launchDebuggerOff = undefined; } }; proxyManager.on('stopped', handleStopped); diff --git a/src/session/session-store.ts b/src/session/session-store.ts index 882b5435a..70312ca93 100644 --- a/src/session/session-store.ts +++ b/src/session/session-store.ts @@ -140,6 +140,14 @@ export interface ManagedSession extends DebugSessionInfo { // attach — the user's value, or the policy's launch default when unset // (issue #244). Previously write-only pass-through; recorded for read-back. effectiveBreakOnExceptions?: ExceptionBreakMode; + // The launcher's decision that the current launch runs with the debugger + // off — noDebug on an adapter that honours it (issue #710). The raw + // record: written per launch, reset per launch and attach, cleared by a + // stop only a live debugger produces. Never read directly for a + // caller-facing answer — isDebuggerOff/debuggerOffWhy (debugger-off.ts) + // gate it on the launch being live, and DebugSessionInfo.debuggerDisabled + // is its projection (issue #749). + launchDebuggerOff?: boolean; // Caller-provided adapterConfig keys the adapter's attach transform did not // carry into the DAP attach request (issue #450). Recorded per attach by // ProxyLauncher.start; consumed by attachToProcess for the response warning. diff --git a/src/utils/error-messages.ts b/src/utils/error-messages.ts index 425f7aea0..1170f9edb 100644 --- a/src/utils/error-messages.ts +++ b/src/utils/error-messages.ts @@ -174,18 +174,71 @@ export const ErrorMessages = { */ debuggerOffForLaunch: DEBUGGER_OFF_FOR_LAUNCH, + /** + * The same why for a session that is paused (issue #749): a stop did land + * — js-debug lands a pause under noDebug — so only the binding clause is + * still true of it. + * Used in: src/session/debugger-off.ts + */ + debuggerOffForLaunchPaused: + 'the debugger is off for this launch (noDebug is true): breakpoints cannot bind; ' + + 'drop noDebug and launch again to debug', + /** * The pending-pause message for a session whose launch runs with the * debugger off (issue #749): the pause was sent and accepted, no stop came * within the grace window, and — unlike `pausePending` — no stop is - * promised, since none is expected. The session still reports 'paused' - * should one land anyway (js-debug does this under noDebug). + * promised, since none is expected. The policy's own explanation (#678: + * js-debug's smart-stepper, which can also keep a pause from landing + * under the flag) rides along when there is one. * Used in: src/session/execution/execution-controller.ts * @param graceSeconds - The grace window duration in seconds + * @param policyHint - The adapter policy's explanation, when it has one */ - pausePendingDebuggerOff: (graceSeconds: number) => + pausePendingDebuggerOff: (graceSeconds: number, policyHint?: string) => `Pause requested; no 'stopped' event within ${graceSeconds}s — ${DEBUGGER_OFF_FOR_LAUNCH}. ` + - `Check the session state in case a stop lands anyway.`, + `Check the session state in case a stop lands anyway.` + + (policyHint ? ` ${policyHint}` : ''), + + /** + * An adapter's own answer with the debugger-off why beside it (issue + * #749): a refused pause, or one that found no debug target yet. + * Used in: src/session/execution/execution-controller.ts + */ + withDebuggerOffWhy: (adapterMessage: string, why: string) => `${adapterMessage} (${why})`, + + /** + * The step/continue refusal for a session that is not paused, with the + * why when the launch runs with the debugger off (issue #749). + * Used in: src/session/execution/execution-controller.ts + */ + notPaused: (why?: string) => (why ? `Not paused: ${why}` : 'Not paused'), + + /** + * The evaluate refusal for a session that is not paused, with the why + * when the launch runs with the debugger off (issue #749). + * Used in: src/session/inspection/expression-evaluator.ts + */ + cannotEvaluateNotPaused: (why?: string) => + why + ? `Cannot evaluate: debugger not paused (${why})` + : 'Cannot evaluate: debugger not paused. Ensure the debugger is stopped at a breakpoint.', + + /** + * The empty stack trace's note for a session that is not paused, with the + * why when the launch runs with the debugger off (issue #749). + * Used in: src/session/inspection/frame-anchor-resolver.ts + */ + stackTraceNotPaused: (state: string, why?: string) => + `Session is not paused (state: ${state}); stack traces are only available while paused${why ? `; ${why}` : ''}.`, + + /** + * get_local_variables with no frame and the session not paused, with the + * why when the launch runs with the debugger off (issue #749). + * Used in: src/server/handlers/inspection-tools.ts + */ + noStackFramesNotPaused: (why?: string) => + why ? `No stack frames available; ${why}.` : 'No stack frames available. The debugger may not be paused.', /** * Suffix appended to the attach message when the post-attach pause was diff --git a/tests/core/unit/server/handlers/inspection-tools.test.ts b/tests/core/unit/server/handlers/inspection-tools.test.ts index 8ec038333..ca4b62610 100644 --- a/tests/core/unit/server/handlers/inspection-tools.test.ts +++ b/tests/core/unit/server/handlers/inspection-tools.test.ts @@ -227,7 +227,7 @@ describe('inspection tool handlers', () => { id: 'test-session', state: 'running', sessionLifecycle: 'ACTIVE', - debuggerDisabled: true + launchDebuggerOff: true }); ctx.sessionManager.getLocalVariables.mockResolvedValue({ variables: [], diff --git a/tests/core/unit/server/server-breakpoint-management-tools.test.ts b/tests/core/unit/server/server-breakpoint-management-tools.test.ts index 84f47c35a..ccc8eec6f 100644 --- a/tests/core/unit/server/server-breakpoint-management-tools.test.ts +++ b/tests/core/unit/server/server-breakpoint-management-tools.test.ts @@ -99,7 +99,7 @@ describe('Server Breakpoint Management Tools', () => { id: 'test-session', state: 'running', sessionLifecycle: 'ACTIVE', - debuggerDisabled: true + launchDebuggerOff: true }); mockSessionManager.listBreakpoints.mockReturnValue([ { id: 'bp-1', file: '/a.py', line: 10, verified: false, message: 'Unbound breakpoint' } @@ -124,7 +124,7 @@ describe('Server Breakpoint Management Tools', () => { id: 'test-session', state: 'running', sessionLifecycle: 'ACTIVE', - debuggerDisabled: true + launchDebuggerOff: true }); mockSessionManager.listBreakpoints.mockReturnValue([ { id: 'bp-1', file: '/a.py', line: 10, verified: true } @@ -141,7 +141,7 @@ describe('Server Breakpoint Management Tools', () => { id: 'test-session', state: 'stopped', sessionLifecycle: 'ACTIVE', - debuggerDisabled: true + launchDebuggerOff: true }); mockSessionManager.listBreakpoints.mockReturnValue([ { id: 'bp-1', file: '/a.py', line: 10, verified: false } diff --git a/tests/core/unit/server/server-control-tools.test.ts b/tests/core/unit/server/server-control-tools.test.ts index d0de66c02..31516459d 100644 --- a/tests/core/unit/server/server-control-tools.test.ts +++ b/tests/core/unit/server/server-control-tools.test.ts @@ -59,7 +59,7 @@ describe('Server Control Tools Tests', () => { id: 'test-session', state: 'running', sessionLifecycle: 'ACTIVE', - debuggerDisabled: true + launchDebuggerOff: true }); // The request still went to the adapter; its own answer is kept. mockSessionManager.setBreakpoint.mockResolvedValue({ @@ -79,12 +79,33 @@ describe('Server Control Tools Tests', () => { expect(content.warning).toContain(ErrorMessages.debuggerOffForLaunch); }); + it('says only that breakpoints cannot bind on a session that is paused — a stop did land (issue #749)', async () => { + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: 'paused', + sessionLifecycle: 'ACTIVE', + launchDebuggerOff: true + }); + mockSessionManager.setBreakpoint.mockResolvedValue({ + breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 10, verified: false, message: 'Unbound breakpoint' } + }); + + const result = await callToolHandler({ + method: 'tools/call', + params: { name: 'set_breakpoint', arguments: { sessionId: 'test-session', file: '/path/to/test.py', line: 10 } } + }); + + const content = JSON.parse(result.content[0].text); + expect(content.warning).toContain(ErrorMessages.debuggerOffForLaunchPaused); + expect(content.warning).not.toMatch(/no stop is expected/); + }); + it('adds no debugger-off note once the launch is over — a queued breakpoint is an ordinary one (issue #749)', async () => { mockSessionManager.getSession.mockReturnValue({ id: 'test-session', state: 'stopped', sessionLifecycle: 'ACTIVE', - debuggerDisabled: true + launchDebuggerOff: true }); mockSessionManager.setBreakpoint.mockResolvedValue({ breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 10, verified: false } @@ -104,7 +125,7 @@ describe('Server Control Tools Tests', () => { id: 'test-session', state: 'running', sessionLifecycle: 'ACTIVE', - debuggerDisabled: true + launchDebuggerOff: true }); mockSessionManager.setBreakpoint.mockResolvedValue({ breakpoint: { id: 'bp-1', file: '/path/to/test.py', line: 10, verified: true } diff --git a/tests/core/unit/session/session-manager-nodebug-warning.test.ts b/tests/core/unit/session/session-manager-nodebug-warning.test.ts index 3e98cad33..5545c5be9 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -10,7 +10,7 @@ import { buildNoDebugLaunchWarning } from '../../../../src/session/breakpoints/l import type { ManagedSession } from '../../../../src/session/session-store.js'; import { SessionManager, type SessionManagerConfig } from '../../../../src/session/session-manager.js'; import { DebugLanguage, SessionState, type AdapterPolicy, type Breakpoint, type ExceptionBreakMode, type FunctionBreakpoint } from '@debugmcp/shared'; -import { createMockDependencies, setMockProxyRunning } from './session-manager-test-utils.js'; +import { createMockDependencies, overridePolicy, setMockProxyRunning } from './session-manager-test-utils.js'; import type { MockProxyManager } from '../../../test-utils/mocks/mock-proxy-manager.js'; type BuilderSession = Pick; @@ -194,8 +194,10 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(result.success).toBe(true); expect(result.state).toBe(SessionState.STOPPED); - expect(warningOf(result)).toMatch(/noDebug is true/); - expect(warningOf(result)).toMatch(/1 breakpoint\(s\)/); + // What is known: the flag, and what it keeps from firing. Not a claim + // that no stop of any kind can come — js-debug lands a pause under it. + expect(warningOf(result)).toMatch(/^noDebug is true, so the debugger is off for this launch: 1 breakpoint\(s\) will not fire\./); + expect(warningOf(result)).not.toMatch(/no stop can arrive/); // The #467 diagnosis ("check the file path and line") would be wrong here. expect(warningOf(result)).not.toMatch(/never bound during this run/); }); @@ -359,9 +361,11 @@ describe('SessionManager launches with noDebug (issue #710)', () => { const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); expect(result.state).toBe(SessionState.PAUSED); - expect(warningOf(result)).not.toMatch(/no stop can arrive/); - // ...while the record — breakpoints still cannot bind — is kept. - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + // The response and the record agree: the debugger is off for the + // breakpoints, whatever paused — no "no effect", no "no stop can come". + expect(warningOf(result)).toMatch(/the debugger is off for this launch: 1 breakpoint\(s\) will not fire/); + expect(warningOf(result)).not.toMatch(/has no effect/); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); }); it('believes a stop that arrived anyway over the policy pin', async () => { @@ -463,7 +467,7 @@ describe('SessionManager launches with noDebug (issue #710)', () => { await launch(s.id, { stopOnEntry: false, noDebug: true }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); }); it('leaves it unset where the adapter ignores the flag, and for a launch without it', async () => { @@ -471,12 +475,12 @@ describe('SessionManager launches with noDebug (issue #710)', () => { runWithoutStopping(); await launch(s.id, { stopOnEntry: false, noDebug: true }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBeUndefined(); pinPolicy({ honoursNoDebug: true }); runWithoutStopping(); await launch(s.id, { stopOnEntry: false }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBeUndefined(); }); it('clears it on the next launch without the flag', async () => { @@ -484,14 +488,14 @@ describe('SessionManager launches with noDebug (issue #710)', () => { const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); runWithoutStopping(); await launch(s.id, { stopOnEntry: false, noDebug: true }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); dependencies.mockProxyManager.simulateEvent('terminated'); await vi.runAllTimersAsync(); runWithoutStopping(); await launch(s.id, { stopOnEntry: false }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBeUndefined(); }); it('does not set it for a dry run — nothing launched', async () => { @@ -508,7 +512,7 @@ describe('SessionManager launches with noDebug (issue #710)', () => { const result = await startPromise; expect((result.data as { dryRun?: boolean }).dryRun).toBe(true); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBeUndefined(); }); it('survives a pause that lands — js-debug pauses under noDebug while its breakpoints stay unbound', async () => { @@ -516,7 +520,7 @@ describe('SessionManager launches with noDebug (issue #710)', () => { const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); runWithoutStopping(); await launch(s.id, { stopOnEntry: false, noDebug: true }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); // Measured: the inspector is attached and a user pause lands, but the // debug domains — breakpoints — are off. A pause proves nothing. @@ -524,7 +528,7 @@ describe('SessionManager launches with noDebug (issue #710)', () => { await vi.runAllTimersAsync(); expect(sessionManager.getSession(s.id)?.state).toBe(SessionState.PAUSED); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); }); it('survives a step taken from that pause — it proves exactly as much as the pause did', async () => { @@ -542,7 +546,7 @@ describe('SessionManager launches with noDebug (issue #710)', () => { await vi.runAllTimersAsync(); expect(sessionManager.getSession(s.id)?.state).toBe(SessionState.PAUSED); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); }); it('is not consulted while the session is merely created — a launch that failed before the proxy leaves it CREATED', async () => { @@ -550,7 +554,7 @@ describe('SessionManager launches with noDebug (issue #710)', () => { const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); runWithoutStopping(); await launch(s.id, { stopOnEntry: false, noDebug: true }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); // The MSVC-toolchain refusal path: back to CREATED with the record intact. sessionManager.getSession(s.id)!.state = SessionState.CREATED; @@ -559,19 +563,54 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(listed).not.toHaveProperty('debuggerDisabled'); }); + it("survives a `debugger;` statement js-debug relabels 'breakpoint' — the adapter itself said 'pause'", async () => { + pinPolicy({ honoursNoDebug: true }); + // The relabel lives in the store's policy (the core's handleStopped reads it). + overridePolicy(sessionManager, { + normalizeStopReason: (raw: string, body?: { description?: string }) => + raw === 'pause' && body?.description === 'Paused on debugger statement' ? 'breakpoint' : raw + }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + + dependencies.mockProxyManager.simulateEvent('stopped', 1, 'pause', { + reason: 'pause', threadId: 1, description: 'Paused on debugger statement' + }); + await vi.runAllTimersAsync(); + + expect(sessionManager.getSession(s.id)?.lastStop?.reason).toBe('breakpoint'); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); + }); + + it('is cleared by a stop that names the breakpoints it hit, whatever the reason was called', async () => { + pinPolicy({ honoursNoDebug: true }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + runWithoutStopping(); + await launch(s.id, { stopOnEntry: false, noDebug: true }); + + dependencies.mockProxyManager.simulateEvent('stopped', 1, 'pause', { + reason: 'pause', threadId: 1, hitBreakpointIds: [1] + }); + await vi.runAllTimersAsync(); + + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBeUndefined(); + }); + it.each(['breakpoint', 'function breakpoint', 'exception', 'entry'])( - "is cleared by a '%s' stop — one a disabled debugger cannot produce", + "is cleared by a '%s' stop the adapter itself reported — one a disabled debugger cannot produce", async (reason) => { pinPolicy({ honoursNoDebug: true }); const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); runWithoutStopping(); await launch(s.id, { stopOnEntry: false, noDebug: true }); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); dependencies.mockProxyManager.simulateEvent('stopped', 1, reason, { reason, threadId: 1 }); await vi.runAllTimersAsync(); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBeUndefined(); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBeUndefined(); } ); @@ -601,7 +640,7 @@ describe('SessionManager launches with noDebug (issue #710)', () => { const result = await restartPromise; expect(result.success).toBe(true); - expect(sessionManager.getSession(s.id)?.debuggerDisabled).toBe(true); + expect(sessionManager.getSession(s.id)?.launchDebuggerOff).toBe(true); }); }); }); diff --git a/tests/core/unit/session/session-store-projection.test.ts b/tests/core/unit/session/session-store-projection.test.ts index aceacacde..28f4814cc 100644 --- a/tests/core/unit/session/session-store-projection.test.ts +++ b/tests/core/unit/session/session-store-projection.test.ts @@ -58,20 +58,24 @@ describe('SessionStore.getAll() debuggerDisabled projection (issue #749)', () => const { store, id } = storeWith(SessionState.RUNNING); expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); - store.get(id)!.debuggerDisabled = true; + store.get(id)!.launchDebuggerOff = true; expect(store.getAll().find((s) => s.id === id)!.debuggerDisabled).toBe(true); - store.get(id)!.debuggerDisabled = undefined; + store.get(id)!.launchDebuggerOff = undefined; expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); - // Only a running or paused launch: over (stopped/error), or never - // launched (created/initializing), the record describes nothing running. - store.get(id)!.debuggerDisabled = true; - for (const state of [SessionState.STOPPED, SessionState.ERROR, SessionState.CREATED, SessionState.INITIALIZING]) { + // Only a live launch — initializing (the proxy is up and a breakpoint + // set now still goes to the adapter), running or paused: over + // (stopped/error) or never launched (created), the record describes + // nothing running. + store.get(id)!.launchDebuggerOff = true; + for (const state of [SessionState.STOPPED, SessionState.ERROR, SessionState.CREATED]) { store.get(id)!.state = state; expect(store.getAll().find((s) => s.id === id)!, state).not.toHaveProperty('debuggerDisabled'); } - store.get(id)!.state = SessionState.PAUSED; - expect(store.getAll().find((s) => s.id === id)!.debuggerDisabled).toBe(true); + for (const state of [SessionState.INITIALIZING, SessionState.PAUSED]) { + store.get(id)!.state = state; + expect(store.getAll().find((s) => s.id === id)!.debuggerDisabled, state).toBe(true); + } }); }); diff --git a/tests/unit/session-manager-operations-coverage.test.ts b/tests/unit/session-manager-operations-coverage.test.ts index ac1ac6a9f..1a476aa59 100644 --- a/tests/unit/session-manager-operations-coverage.test.ts +++ b/tests/unit/session-manager-operations-coverage.test.ts @@ -4450,7 +4450,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () vi.useFakeTimers(); try { mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = true; + mockSession.launchDebuggerOff = true; mockProxyManager.sendDapRequest.mockResolvedValue({}); const describePendingStop = vi.fn().mockReturnValue('Explained by the policy.'); vi.spyOn(operations as any, 'selectPolicy').mockReturnValue({ describePendingStop } as any); @@ -4463,20 +4463,23 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () expect(mockProxyManager.sendDapRequest).toHaveBeenCalledWith('pause', expect.objectContaining({ threadId: 1 })); expect(result.success).toBe(true); expect(result.data?.pending).toBe(true); - // One message that does not promise the stop the base text promises. - expect(result.data?.message).toBe(ErrorMessages.pausePendingDebuggerOff(5)); + // One message that does not promise the stop the base text promises, + // with the policy's own explanation kept: on js-debug the pause can + // land under the flag, and #678's advice is what makes it. + expect(result.data?.message).toBe(ErrorMessages.pausePendingDebuggerOff(5, 'Explained by the policy.')); expect(result.data?.message).toContain(why); + expect(result.data?.message).toContain('Explained by the policy.'); expect(result.data?.message).not.toMatch(/blocked in native code/); expect(result.data?.message).not.toMatch(/will report 'paused' once the stop lands/); - expect(describePendingStop).not.toHaveBeenCalled(); + expect(describePendingStop).toHaveBeenCalled(); } finally { vi.useRealTimers(); } }); - it("keeps the adapter's refusal of a pause — the same error object — and appends the why", async () => { + it("keeps the adapter's refusal of a pause as the cause, untouched, and appends the why", async () => { mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = true; + mockSession.launchDebuggerOff = true; const refusal = Object.assign(new Error('Internal debugger error: Not supported in noDebug mode.'), { code: 'E_NODEBUG' }); mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { if (command === 'pause') { @@ -4485,38 +4488,19 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () return {}; }); - const thrown = await operations.pause('test-session', 1).then( - () => { throw new Error('expected a rejection'); }, - (err: unknown) => err - ); - expect(thrown).toBe(refusal); - expect((thrown as Error).message).toBe(`Internal debugger error: Not supported in noDebug mode. (${why})`); - expect((thrown as Error & { code?: string }).code).toBe('E_NODEBUG'); - }); - - it('wraps a refusal whose message cannot be appended to, keeping it as the cause', async () => { - mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = true; - const frozen = new Error('Not supported in noDebug mode.'); - Object.defineProperty(frozen, 'message', { get: () => 'Not supported in noDebug mode.' }); - mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { - if (command === 'pause') { - throw frozen; - } - return {}; - }); - const thrown = await operations.pause('test-session', 1).then( () => { throw new Error('expected a rejection'); }, (err: unknown) => err as Error & { cause?: unknown } ); - expect(thrown.message).toBe(`Not supported in noDebug mode. (${why})`); - expect(thrown.cause).toBe(frozen); + expect(thrown.message).toBe(ErrorMessages.withDebuggerOffWhy('Internal debugger error: Not supported in noDebug mode.', why)); + expect(thrown.cause).toBe(refusal); + expect(refusal.message).toBe('Internal debugger error: Not supported in noDebug mode.'); + expect((thrown.cause as { code?: string }).code).toBe('E_NODEBUG'); }); it('carries the why on a pause that found no debug target yet (js-debug before the child adopts)', async () => { mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = true; + mockSession.launchDebuggerOff = true; mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { if (command === 'pause') { throw new Error(`pause failed: ${NO_DEBUG_TARGET_MARKER}`); @@ -4531,9 +4515,19 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () expect(result.error).toContain(why); }); + it('carries the why while the launch is still initializing — the proxy is up and answering', async () => { + mockSession.state = SessionState.INITIALIZING; + mockSession.launchDebuggerOff = true; + + const result = await operations.getStackTraceDetailed('test-session'); + + expect(result.frames).toEqual([]); + expect(result.note).toContain(why); + }); + it('drops the why once the launch is over — the flag describes a launch that is no longer running', async () => { mockSession.state = SessionState.STOPPED; - mockSession.debuggerDisabled = true; + mockSession.launchDebuggerOff = true; const result = await operations.getStackTraceDetailed('test-session'); @@ -4543,28 +4537,30 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () it('says why stepping and continuing find nothing paused', async () => { mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = true; + mockSession.launchDebuggerOff = true; const step = await operations.stepOver('test-session'); expect(step.success).toBe(false); - expect(step.error).toBe(`Not paused: ${why}`); + expect(step.error).toBe(ErrorMessages.notPaused(why)); + expect(step.error).toContain(why); const cont = await operations.continue('test-session'); expect(cont.success).toBe(false); - expect(cont.error).toBe(`Not paused: ${why}`); + expect(cont.error).toBe(ErrorMessages.notPaused(why)); }); it('leaves the plain "Not paused" alone when the debugger is on', async () => { mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = undefined; + mockSession.launchDebuggerOff = undefined; const step = await operations.stepOver('test-session'); + expect(step.error).toBe(ErrorMessages.notPaused()); expect(step.error).toBe('Not paused'); }); it('says why an expression cannot be evaluated', async () => { mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = true; + mockSession.launchDebuggerOff = true; const result = await operations.evaluateExpression('test-session', '1 + 1'); @@ -4575,7 +4571,7 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () it('says why the stack trace is empty', async () => { mockSession.state = SessionState.RUNNING; - mockSession.debuggerDisabled = true; + mockSession.launchDebuggerOff = true; const result = await operations.getStackTraceDetailed('test-session'); From d65ad56984bad1b71664596e0319eee6c6832a90 Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 17 Sep 2026 14:55:06 -0400 Subject: [PATCH 5/7] docs(schema): the dapLaunchArgs.noDebug description stops claiming no stop ever arrives (#749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The schema is the one noDebug text the model reads before choosing the flag, and it still said "no stop ever arrives" after #751 retired that claim in the warning, the fragment and the docs: js-debug lands a user pause under the flag. It now says what is true of every honouring adapter — no breakpoint, exception or entry stop — and that the session reports debuggerDisabled while such a launch runs. Fences re-recorded. Co-Authored-By: Claude Opus 5 (1M context) --- src/server/tool-schemas.ts | 2 +- .../tool-list/bp-assert.va-explicit.container-true.json | 2 +- .../tool-list/bp-assert.va-explicit.container-unset.json | 2 +- .../tool-list/bp-assert.va-open.container-true.json | 2 +- .../tool-list/bp-assert.va-open.container-unset.json | 2 +- .../tool-list/bp-assert.va-unset.container-true.json | 2 +- .../tool-list/bp-assert.va-unset.container-unset.json | 2 +- .../tool-list/bp-content.va-explicit.container-true.json | 2 +- .../tool-list/bp-content.va-explicit.container-unset.json | 2 +- .../tool-list/bp-content.va-open.container-true.json | 2 +- .../tool-list/bp-content.va-open.container-unset.json | 2 +- .../tool-list/bp-content.va-unset.container-true.json | 2 +- .../tool-list/bp-content.va-unset.container-unset.json | 2 +- .../tool-list/bp-line.va-explicit.container-true.json | 2 +- .../tool-list/bp-line.va-explicit.container-unset.json | 2 +- .../__snapshots__/tool-list/bp-line.va-open.container-true.json | 2 +- .../tool-list/bp-line.va-open.container-unset.json | 2 +- .../tool-list/bp-line.va-unset.container-true.json | 2 +- .../tool-list/bp-line.va-unset.container-unset.json | 2 +- .../tool-list/bp-unset.va-explicit.container-true.json | 2 +- .../tool-list/bp-unset.va-explicit.container-unset.json | 2 +- .../tool-list/bp-unset.va-open.container-true.json | 2 +- .../tool-list/bp-unset.va-open.container-unset.json | 2 +- .../tool-list/bp-unset.va-unset.container-true.json | 2 +- .../tool-list/bp-unset.va-unset.container-unset.json | 2 +- 25 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/server/tool-schemas.ts b/src/server/tool-schemas.ts index e170a4813..25552c61e 100644 --- a/src/server/tool-schemas.ts +++ b/src/server/tool-schemas.ts @@ -149,7 +149,7 @@ export function buildToolDefinitions(options: BuildToolDefinitionsOptions): Tool properties: { stopOnEntry: { type: 'boolean', description: 'Pause at the first line before running. Default false — the opposite of attach, which pauses unless stopOnEntry is false' }, justMyCode: { type: 'boolean', description: 'Only debug user code (default true). JavaScript launch: true blackboxes node_modules via skipFiles and keeps js-debug\'s smart-stepper on, so a pause or step that lands in skipped code (Node internals, node_modules) is stepped through and may never land; false drops node_modules from skipFiles and turns the stepper off, so pauses and steps land inside dependencies' }, - noDebug: { type: 'boolean', description: 'DAP\'s launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so' } + noDebug: { type: 'boolean', description: 'DAP\'s launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so' } }, additionalProperties: true }, diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-true.json index dcf58c25a..3cbaa3596 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-true.json @@ -216,7 +216,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-unset.json index d4c98ade1..20f274f09 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-explicit.container-unset.json @@ -216,7 +216,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-true.json index d6a330d28..a8649c2ce 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-true.json @@ -216,7 +216,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-unset.json index ac53ef4a9..368a1cf0e 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-open.container-unset.json @@ -216,7 +216,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-true.json index d6a330d28..a8649c2ce 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-true.json @@ -216,7 +216,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-unset.json index ac53ef4a9..368a1cf0e 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-assert.va-unset.container-unset.json @@ -216,7 +216,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-true.json index a111b7172..838213055 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-true.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-unset.json index 3c91f90a5..4b9eeb751 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-explicit.container-unset.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-true.json index d7f0e2765..29d5724ac 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-true.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-unset.json index 4d93b17cf..1e7c24152 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-open.container-unset.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-true.json index d7f0e2765..29d5724ac 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-true.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-unset.json index 4d93b17cf..1e7c24152 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-content.va-unset.container-unset.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-true.json index ff96afe58..8aae9a0de 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-true.json @@ -212,7 +212,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-unset.json index cd8436dac..61362ac7a 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-explicit.container-unset.json @@ -212,7 +212,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-true.json index c6a376813..761c8f46a 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-true.json @@ -212,7 +212,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-unset.json index ea3b3ab8a..ad23833b5 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-open.container-unset.json @@ -212,7 +212,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-true.json index c6a376813..761c8f46a 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-true.json @@ -212,7 +212,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-unset.json index ea3b3ab8a..ad23833b5 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-line.va-unset.container-unset.json @@ -212,7 +212,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-true.json index a111b7172..838213055 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-true.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-unset.json index 3c91f90a5..4b9eeb751 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-explicit.container-unset.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-true.json index d7f0e2765..29d5724ac 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-true.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-unset.json index 4d93b17cf..1e7c24152 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-open.container-unset.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-true.json b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-true.json index d7f0e2765..29d5724ac 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-true.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-true.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true diff --git a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-unset.json b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-unset.json index 4d93b17cf..1e7c24152 100644 --- a/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-unset.json +++ b/tests/core/unit/server/__snapshots__/tool-list/bp-unset.va-unset.container-unset.json @@ -226,7 +226,7 @@ }, "noDebug": { "type": "boolean", - "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no stop ever arrives — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug, debugpy, Delve and CodeLLDB) no breakpoint binds, no exception filter arms, no entry stop lands and no breakpoint, exception or entry stop ever arrives (a pause may still land where the runtime allows one) — the response warns when breakpoints, an explicit breakOnExceptions or stopOnEntry are set alongside it, and while the launch runs the session reports debuggerDisabled and every later surface says the debugger is off. rdbg, netcoredbg and the Java bridge ignore the flag, and the rust launch never forwards it: their debugger stays on and the response says so" } }, "additionalProperties": true From e839b4bc23108d6de4d001e10d4977e749fe5f8c Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 17 Sep 2026 14:55:07 -0400 Subject: [PATCH 6/7] fix(server): get_stack_trace on a running session with no thread answers not-paused with the why, not "no active proxy" (#749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured on the merged tree (#750 + #751) with debugpy under noDebug: nothing ever stops, so no thread is current, and debugpy refuses the `threads` discovery ("Server is not available") — the facade then threw ProxyNotRunningError, "Cannot get stack trace: no active proxy", for a session whose proxy was alive. js-debug hid this: its inspector answers `threads`, so the js e2e reached the resolver's not-paused note. On a session that is not paused, no thread is expected; the session layer's not-paused answer (with the debugger-off why) needs none, so the facade hands it there. A paused session with no thread to name keeps the error — that is the anomaly it describes. The two existing throw tests now say "paused". The python surfaces e2e (the clean case: debugpy attaches no debugger at all) covers set_breakpoint, list_breakpoints, get_stack_trace, get_local_variables, evaluate_expression, step_over and pause_execution; watched it fail on exactly this answer against a dist without the fix. Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/749.fixed.md | 2 +- src/server.ts | 16 ++- .../server/server-inspection-tools.test.ts | 4 +- .../mcp-server-break-on-exceptions.test.ts | 98 +++++++++++++++++++ tests/unit/server-coverage.test.ts | 38 ++++++- 5 files changed, 151 insertions(+), 7 deletions(-) diff --git a/changelog.d/749.fixed.md b/changelog.d/749.fixed.md index 9eb7e821d..7e1acd532 100644 --- a/changelog.d/749.fixed.md +++ b/changelog.d/749.fixed.md @@ -1 +1 @@ -**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while the launch is live — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (a breakpoint the adapter itself reports or names in `hitBreakpointIds`, an exception, an entry stop): that proves the debugger on for the adapter build. A pause, a step taken from one, or a `debugger;` statement does not clear it — js-debug lands all three under the flag with its breakpoints still unbound — and a paused session is told only that breakpoints cannot bind. The `start_debugging` warning itself no longer claims "no stop can arrive": it says the debugger is off for this launch and names what will not fire. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) +**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms — or worse: with no thread ever current and debugpy refusing the `threads` discovery, `get_stack_trace` claimed "no active proxy" for a session whose proxy was alive. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while the launch is live — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (a breakpoint the adapter itself reports or names in `hitBreakpointIds`, an exception, an entry stop): that proves the debugger on for the adapter build. A pause, a step taken from one, or a `debugger;` statement does not clear it — js-debug lands all three under the flag with its breakpoints still unbound — and a paused session is told only that breakpoints cannot bind. The `start_debugging` warning itself no longer claims "no stop can arrive": it says the debugger is off for this launch and names what will not fire. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) diff --git a/src/server.ts b/src/server.ts index a7c5077fc..36449c5e0 100644 --- a/src/server.ts +++ b/src/server.ts @@ -31,6 +31,7 @@ import { Breakpoint, FunctionBreakpoint, SessionLifecycleState, + SessionState, IEnvironment, ILogger, ExceptionBreakMode @@ -540,11 +541,22 @@ export class DebugMcpServer implements ToolContext { currentThreadId = threads[0].id; } } catch { - // threads request failed — fall through to error + // threads request failed — fall through } } if (typeof currentThreadId !== 'number') { - throw new ProxyNotRunningError(sessionId || 'unknown', 'get stack trace'); + // No thread is known and the adapter named none. On a session that + // is not paused that is expected — nothing has stopped yet, or the + // launch runs with the debugger off and the adapter refuses even + // `threads` (debugpy: "Server is not available") — and the proxy is + // alive, so "no active proxy" would be false; the session layer's + // not-paused answer (with the why, issue #749) needs no thread. A + // paused session with no thread to name is the anomaly the error + // is for. + if (session.state !== SessionState.PAUSED) { + return this.sessionManager.getStackTraceDetailed(sessionId, undefined, includeInternals); + } + throw new ProxyNotRunningError(sessionId || 'unknown', 'get stack trace'); } // ensureStackReady: the thread above was resolved implicitly (the MCP tool // has no threadId argument), so a paused session answering with zero diff --git a/tests/core/unit/server/server-inspection-tools.test.ts b/tests/core/unit/server/server-inspection-tools.test.ts index e029dbf81..ea6dc8972 100644 --- a/tests/core/unit/server/server-inspection-tools.test.ts +++ b/tests/core/unit/server/server-inspection-tools.test.ts @@ -19,6 +19,7 @@ import { type MockSessionManager } from './server-test-helpers.js'; import { OutputRingBuffer } from '../../../../src/session/output-buffer.js'; +import { SessionState } from '@debugmcp/shared'; // Mock dependencies vi.mock('@modelcontextprotocol/sdk/server/index.js'); @@ -436,8 +437,9 @@ describe('Server Inspection Tools Tests', () => { expect(content.diagnostics).toEqual({ proxyLogPath: '/logs/proxy-test-session.log' }); }); - it('should handle missing thread ID', async () => { + it('should handle missing thread ID on a paused session', async () => { const mockSession = { + state: SessionState.PAUSED, failureDiagnostics: { proxyLogPath: '/logs/proxy-test-session.log' }, proxyManager: { getCurrentThreadId: vi.fn().mockReturnValue(null) diff --git a/tests/e2e/mcp-server-break-on-exceptions.test.ts b/tests/e2e/mcp-server-break-on-exceptions.test.ts index 085d054fd..fb4ccdc2f 100644 --- a/tests/e2e/mcp-server-break-on-exceptions.test.ts +++ b/tests/e2e/mcp-server-break-on-exceptions.test.ts @@ -40,6 +40,7 @@ const JS_CRASHING_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-script const JS_CLEAN_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-scripts', 'js-clean-exit.js'); const SIMPLE_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'debug-scripts', 'simple.py'); const JS_PAUSE_SCRIPT = path.resolve(ROOT, 'examples', 'javascript', 'pause_test.js'); +const PY_PAUSE_SCRIPT = path.resolve(ROOT, 'examples', 'python', 'pause_test.py'); const ATTACH_SCRIPT = path.resolve(ROOT, 'tests', 'fixtures', 'python', 'attach_then_raise.py'); const PYTHON = process.platform === 'win32' ? 'python' : 'python3'; @@ -433,6 +434,103 @@ describe('Break-on-exception (issue #220)', () => { expect(stopped!.lastStop).toBeUndefined(); expect(stopped!.exitCode).toBe(0); }, 60000); + + it('says the debugger is off on every later surface of a noDebug session (issue #749)', async () => { + // The clean case: debugpy attaches no debugger at all under the flag + // (js-debug keeps its inspector, so a pause still lands there). Every + // request still goes to debugpy; its own answer — "Server is not + // available" — is kept, and the session's recorded decision adds why. + sessionId = await createSession('python', 'py-nodebug-surfaces'); + const bp = parseSdkToolResult(await mcpClient!.callTool({ + name: 'set_breakpoint', + arguments: { sessionId, file: PY_PAUSE_SCRIPT, line: 7 } + })); + expect(bp.success, JSON.stringify(bp)).toBe(true); + + const startRes = parseSdkToolResult(await mcpClient!.callTool({ + name: 'start_debugging', + arguments: { + sessionId, + scriptPath: PY_PAUSE_SCRIPT, + dapLaunchArgs: { stopOnEntry: false, noDebug: true } + } + })); + expect(startRes.success, JSON.stringify(startRes)).toBe(true); + expect(startRes.state).toBe('running'); + expect((startRes as { warning?: string }).warning).toMatch(/noDebug is true/); + + const why = /the debugger is off for this launch/; + const debugpySaid = /Server is not available/; + + const running = await getSessionSnapshot(mcpClient!, sessionId); + expect(running?.debuggerDisabled).toBe(true); + + const live = parseSdkToolResult(await mcpClient!.callTool({ + name: 'set_breakpoint', + arguments: { sessionId, file: PY_PAUSE_SCRIPT, line: 8 } + })) as { success?: boolean; verified?: boolean; warning?: string }; + expect(live.success).toBe(true); + expect(live.verified).toBe(false); + expect(live.warning).toMatch(debugpySaid); + expect(live.warning).toMatch(why); + + const listed = parseSdkToolResult(await mcpClient!.callTool({ + name: 'list_breakpoints', + arguments: { sessionId } + })) as { warning?: string; breakpoints?: Array<{ verified?: boolean }> }; + expect(listed.warning).toMatch(why); + expect(listed.breakpoints?.every(b => b.verified === false)).toBe(true); + + // No thread is ever current (nothing stops), and debugpy refuses the + // `threads` discovery too: the answer is the not-paused note with the + // why, not "no active proxy" — the proxy is alive. + const stack = parseSdkToolResult(await mcpClient!.callTool({ + name: 'get_stack_trace', + arguments: { sessionId } + })) as { success?: boolean; note?: string; error?: string }; + expect(stack.success, JSON.stringify(stack)).toBe(true); + expect(stack.note).toMatch(why); + + const locals = parseSdkToolResult(await mcpClient!.callTool({ + name: 'get_local_variables', + arguments: { sessionId } + })) as { success?: boolean; count?: number; message?: string }; + expect(locals.success).toBe(true); + expect(locals.count).toBe(0); + expect(locals.message).toMatch(why); + + const evaluated = parseSdkToolResult(await mcpClient!.callTool({ + name: 'evaluate_expression', + arguments: { sessionId, expression: 'counter' } + })) as { success?: boolean; error?: string }; + expect(evaluated.success).toBe(false); + expect(evaluated.error).toMatch(why); + + const step = parseSdkToolResult(await mcpClient!.callTool({ + name: 'step_over', + arguments: { sessionId } + })) as { success?: boolean; error?: string }; + expect(step.success).toBe(false); + expect(step.error).toMatch(/^Not paused: /); + expect(step.error).toMatch(why); + + // The pause is still sent. Measured: debugpy refuses it under the flag + // ("Server is not available"), and the refusal reaches the wire as an + // MCP error with the why beside it; should a debugpy build accept and + // land it instead, the decision stays (a pause proves nothing about + // breakpoints). + const pause = await callToolSafely(mcpClient!, 'pause_execution', { sessionId }) as { + success?: boolean; state?: string; error?: unknown; message?: string; data?: { message?: string }; + }; + if (pause.success && pause.state === 'paused') { + const paused = await getSessionSnapshot(mcpClient!, sessionId); + expect(paused?.debuggerDisabled).toBe(true); + } else { + const answer = `${String(pause.error ?? '')} ${pause.message ?? ''} ${pause.data?.message ?? ''}`; + expect(answer).toMatch(why); + expect(answer).toMatch(debugpySaid); + } + }, 60000); }); describe('JavaScript launch (js-debug child session)', () => { diff --git a/tests/unit/server-coverage.test.ts b/tests/unit/server-coverage.test.ts index 2ab2068fa..728af3d35 100644 --- a/tests/unit/server-coverage.test.ts +++ b/tests/unit/server-coverage.test.ts @@ -7,7 +7,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { Server } from '@modelcontextprotocol/sdk/server/index.js'; import { DebugMcpServer } from '../../src/server'; import { McpError } from '@modelcontextprotocol/sdk/types.js'; -import { SessionLifecycleState } from '@debugmcp/shared'; +import { SessionLifecycleState, SessionState } from '@debugmcp/shared'; import { createProductionDependencies } from '../../src/container/dependencies.js'; import { SessionManager } from '../../src/session/session-manager.js'; import { @@ -215,7 +215,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { expect(result.frames).toHaveLength(1); }); - it('should throw when getStackTrace has no thread and threads request fails', async () => { + it('should throw when a paused getStackTrace has no thread and the threads request fails', async () => { const mockProxy = { getCurrentThreadId: () => null, isRunning: () => true, @@ -223,6 +223,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { }; mockSessionManager.getSession.mockReturnValue({ id: 'test-session', + state: SessionState.PAUSED, sessionLifecycle: SessionLifecycleState.ACTIVE, proxyManager: mockProxy }); @@ -231,7 +232,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { .rejects.toThrow('Cannot get stack trace: no active proxy'); }); - it('should throw when getStackTrace has no thread and threads response is empty', async () => { + it('should throw when a paused getStackTrace has no thread and the threads response is empty', async () => { const mockProxy = { getCurrentThreadId: () => null, isRunning: () => true, @@ -239,6 +240,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { }; mockSessionManager.getSession.mockReturnValue({ id: 'test-session', + state: SessionState.PAUSED, sessionLifecycle: SessionLifecycleState.ACTIVE, proxyManager: mockProxy }); @@ -246,6 +248,36 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { await expect(server.getStackTrace('test-session')) .rejects.toThrow('Cannot get stack trace: no active proxy'); }); + + it('hands a running session with no thread to the session layer instead of claiming no active proxy (issue #749)', async () => { + // A launch that runs with the debugger off never stops, so no thread + // is ever current, and debugpy refuses the `threads` discovery + // ("Server is not available"). The proxy is alive; the honest answer + // is the resolver's not-paused note with the why — the same answer a + // debug-mode running session gets once its thread is known. + const mockProxy = { + getCurrentThreadId: () => null, + isRunning: () => true, + sendDapRequest: vi.fn().mockRejectedValue(new Error('Server is not available')) + }; + mockSessionManager.getSession.mockReturnValue({ + id: 'test-session', + state: SessionState.RUNNING, + launchDebuggerOff: true, + sessionLifecycle: SessionLifecycleState.ACTIVE, + proxyManager: mockProxy + }); + const notPaused = { + frames: [], totalFrameCount: 0, hiddenFrameCount: 0, allFramesInternal: false, + note: 'Session is running, not paused: the debugger is off for this launch (noDebug is true)' + }; + mockSessionManager.getStackTraceDetailed.mockResolvedValue(notPaused); + + await expect(server.getStackTrace('test-session')).resolves.toBe(notPaused); + + expect(mockProxy.sendDapRequest).toHaveBeenCalledWith('threads', {}); + expect(mockSessionManager.getStackTraceDetailed).toHaveBeenCalledWith('test-session', undefined, false); + }); }); describe('Create Debug Session Edge Cases', () => { From 6e74e812bc65413f2efd3a1971b3de815ae13f93 Mon Sep 17 00:00:00 2001 From: JF Date: Thu, 17 Sep 2026 15:24:10 -0400 Subject: [PATCH 7/7] fix(session): a verified breakpoint is proof the debugger is on; get_stack_trace never claims "no active proxy"; pause on an initializing launch says why (#749 review 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth pass, on the merged tree (#750 + #751), seven findings, all taken: - get_stack_trace asked the adapter for `threads` before the state check that made the answer irrelevant: a session that is not paused gets the session layer's not-paused note (with the why) with no round trip — on a wedged adapter that request blocked for the DAP timeout first. - The PAUSED + no-thread branch still threw "no active proxy" for a proxy that was alive; the resolver's "No stopped thread is known for this session." is the truthful answer, so the throw is gone (the error stays for a session with no proxy at all). - A breakpoint the adapter verified is proof this build debugs after all, and it arrives before any hit. Measured first: every honouring adapter refuses or unbinds a live breakpoint under the flag (js-debug "Unbound breakpoint", debugpy "Server is not available", Delve "noDebug mode: unable to process 'setBreakpoints'", CodeLLDB "Not supported in noDebug mode"), so a verified record can only come from a build that ignores it. Consulted on read (adapterVerifiedABreakpoint, inside isDebuggerOff) since bindings are per-launch state; the launch response reads the same evidence, so "will not fire" is never said of a breakpoint list_breakpoints shows bound. - pause_execution on an INITIALIZING debugger-off session was the one surface without the why. - withDebuggerOffWhy now passes the rejected value itself as the cause, not a synthesized Error, when the bridge threw a non-Error. - The "Used in" map on debuggerOffForLaunch named five files that never reference it; it names the one gate that does. - The list_breakpoints doc note quotes the paused variant too. Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/749.fixed.md | 2 +- docs/tool-reference.md | 4 +- src/server.ts | 26 +++++------ src/session/debugger-off.ts | 33 +++++++++++++- src/session/execution/execution-controller.ts | 16 +++++-- src/session/launch/debug-launcher.ts | 27 +++++++----- src/utils/error-messages.ts | 5 +-- .../server/server-inspection-tools.test.ts | 21 +++++---- .../session-manager-nodebug-warning.test.ts | 30 +++++++++++++ .../session/session-store-projection.test.ts | 23 ++++++++++ tests/unit/server-coverage.test.ts | 42 +++++++++++++----- ...ession-manager-operations-coverage.test.ts | 43 +++++++++++++++++++ 12 files changed, 219 insertions(+), 53 deletions(-) diff --git a/changelog.d/749.fixed.md b/changelog.d/749.fixed.md index 7e1acd532..6eaea4176 100644 --- a/changelog.d/749.fixed.md +++ b/changelog.d/749.fixed.md @@ -1 +1 @@ -**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms — or worse: with no thread ever current and debugpy refusing the `threads` discovery, `get_stack_trace` claimed "no active proxy" for a session whose proxy was alive. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while the launch is live — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (a breakpoint the adapter itself reports or names in `hitBreakpointIds`, an exception, an entry stop): that proves the debugger on for the adapter build. A pause, a step taken from one, or a `debugger;` statement does not clear it — js-debug lands all three under the flag with its breakpoints still unbound — and a paused session is told only that breakpoints cannot bind. The `start_debugging` warning itself no longer claims "no stop can arrive": it says the debugger is off for this launch and names what will not fire. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) +**A session whose launch runs with the debugger off says so on every later surface** — `start_debugging` decided "this `noDebug` launch runs with the debugger off" (#710) and then forgot it: `set_breakpoint` on the running session came back `verified: false` with no reason, `list_breakpoints` showed everything unbound with none, `pause_execution` ended in the "may be blocked in native code" guess, and stepping, `get_stack_trace`, `get_local_variables` and `evaluate_expression` answered "not paused" in debugger terms — or worse: with no thread ever current and debugpy refusing the `threads` discovery, `get_stack_trace` claimed "no active proxy" for a session whose proxy was alive. The decision is now recorded on the session — `list_debug_sessions` reports `debuggerDisabled: true` while the launch is live — reset per launch and attach, and cleared by a `stopped` event only a live debugger produces (a breakpoint the adapter itself reports or names in `hitBreakpointIds`, an exception, an entry stop): that proves the debugger on for the adapter build — as does a breakpoint the adapter verified, since every honouring adapter refuses or unbinds one under the flag. A pause, a step taken from one, or a `debugger;` statement does not clear it — js-debug lands all three under the flag with its breakpoints still unbound — and a paused session is told only that breakpoints cannot bind. The `start_debugging` warning itself no longer claims "no stop can arrive": it says the debugger is off for this launch and names what will not fire. Every request still goes to the adapter and its own answer is kept ("Unbound breakpoint", a refusal in the adapter's words); the recorded fact adds the why beside it — "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — and a pause that is accepted but never lands gets a message that promises no stop instead of the native-code guess (#749) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 482c340c0..1987ceedf 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -302,7 +302,7 @@ Lists all breakpoints in a session with their current verified state and adapter - `functionBreakpoints`/`functionCount` are always present in the unfiltered response (empty arrays when none exist). When filtering by `file` they are omitted — function breakpoints are session-global, not file-scoped. - `adapterId` is the debug adapter's own numeric id for the breakpoint, captured from setBreakpoints responses and breakpoint events. It is absent until the adapter has seen the breakpoint. - Verification is eventually consistent: some adapters (js-debug, JDI, netcoredbg) bind breakpoints asynchronously and confirm via DAP breakpoint events shortly after launch or class load. -- `warning` (top level) appears only while the session's current launch runs with the debugger off (`noDebug: true` on an adapter that honours it, see `start_debugging`) and at least one listed breakpoint is unverified: "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug". The per-breakpoint records keep the adapter's own answers (issue #749). +- `warning` (top level) appears only while the session's current launch runs with the debugger off (`noDebug: true` on an adapter that honours it, see `start_debugging`) and at least one listed breakpoint is unverified: "the debugger is off for this launch (noDebug is true): breakpoints cannot bind and no stop is expected; drop noDebug and launch again to debug" — on a paused session (js-debug lands a pause under the flag) only the clause still true of it, "…: breakpoints cannot bind; drop noDebug and launch again to debug". The per-breakpoint records keep the adapter's own answers; a breakpoint the adapter *did* verify is proof this build debugs after all, and the warning (with `debuggerDisabled` and every other debugger-off note) drops (issue #749). - A breakpoint the program has stopped on is reported `verified: true` from that stop onward, even if the adapter never confirmed it (issue #673), for adapters whose `stopped` event names the breakpoints it hit (`hitBreakpointIds`: js-debug, debugpy, Delve, CodeLLDB — netcoredbg, the JDI bridge and rdbg omit the field). Such a record carries `verifiedBy: "hit"` until the adapter itself confirms it (`"adapter"`); an adapter answer of "unbound" does not downgrade it. A provisional "Unbound breakpoint" `message` is dropped by the hit; any other note is kept. - On entries of the `breakpoints` array, `boundFile`/`boundLine` appear when the adapter answers under a *different* file from the request. For a source-mapped `.ts` request on a JavaScript launch with maps on (the default) js-debug verifies the request under the `.ts` path, `get_stack_trace` frames show `.ts`, and the pair is absent; it appears when js-debug answers under the generated `dist/*.js` instead — measured with `adapterLaunchConfig: { sourceMaps: false }` (the second entry in the example above was captured that way, and the frames then show the generated file too), and possible whenever the `.ts` source cannot be resolved through the map (issues #673, #700). `file` and `line` keep describing the request; the bound pair is where it landed. (Entries of `functionBreakpoints` use the same names for the bound location of the symbol, present whenever it is bound.) @@ -369,7 +369,7 @@ Starts debugging a script. - `dapLaunchArgs` (object, optional): Standard DAP launch arguments: - `stopOnEntry` (boolean): Stop at first line (default `false` — the opposite of attach, which pauses unless `stopOnEntry` is `false`) - `justMyCode` (boolean): Debug only user code (default `true`). JavaScript launch: `true` blackboxes `node_modules` through js-debug's `skipFiles` and keeps js-debug's smart-stepper on, so a pause or step that lands in skipped code (Node internals, `node_modules`) is stepped through and may never land (`pending: true`, with an explanation); `false` drops `node_modules` from the skip list and turns the stepper off, so steps land inside dependencies and `pause_execution` lands as soon as any JavaScript runs (issue #678). A caller `skipFiles` replaces the default list. Source maps are on for every JavaScript launch, `.js` programs included — stops report `src/*.ts` when maps and sources are present; `adapterLaunchConfig: { sourceMaps: false }` opts out (issue #684) - - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no breakpoint, exception or entry stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off for this launch and names what will not fire — not that no stop of any kind can come — in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug, debugpy, Delve and CodeLLDB honour it (debugpy and Delve open no configuration phase at all, so the launch is complete on the launch response; CodeLLDB refuses the configuration requests, and the refusal reaches the `warning` and `get_output` — issue #746). The run ends `stopped` with the program's output and, where the adapter reports one, the exit code (Delve prints the status to the console only). Under the flag Delve runs the program through Go's `exec`, so on Windows the binary needs its `.exe` extension. **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint", or CodeLLDB's refusal echoed onto each pre-launch breakpoint) and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event only a live debugger produces — a breakpoint the adapter itself reports (or names in `hitBreakpointIds`), an exception stop, an entry stop — proves the debugger on for that adapter build and clears the decision; a pause, a step taken from one, or a `debugger;` statement do not (js-debug lands all three under the flag while its breakpoints stay unbound; an uncaught throw does not stop). On a paused session the sentence keeps only the clause still true of it — breakpoints cannot bind — and outside a live launch (`initializing`/`running`/`paused`) the decision is neither reported nor consulted (issue #749). + - `noDebug` (boolean): DAP's "launch without enabling debugging" flag (default `false`). What it does depends on the adapter, and the response says which case you are in. **Honoured** — the debugger is off: no breakpoint binds, no exception filter arms, no entry stop lands, and no breakpoint, exception or entry stop ever arrives, so a short script ends `stopped` and a server stays `running`. That is a legitimate plain run and says nothing by itself; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the `warning` says the debugger is off for this launch and names what will not fire — not that no stop of any kind can come — in place of the unbound-breakpoint diagnostics that would otherwise send you to check paths that are fine (issue #710). js-debug, debugpy, Delve and CodeLLDB honour it (debugpy and Delve open no configuration phase at all, so the launch is complete on the launch response; CodeLLDB refuses the configuration requests, and the refusal reaches the `warning` and `get_output` — issue #746). The run ends `stopped` with the program's output and, where the adapter reports one, the exit code (Delve prints the status to the console only). Under the flag Delve runs the program through Go's `exec`, so on Windows the binary needs its `.exe` extension. **Ignored** — rdbg, netcoredbg and the Java bridge run the debugger regardless, and the rust launch transform never forwards the flag: the `warning` says the flag had no effect and everything works as usual. The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. While such a launch runs, the session remembers the decision (`list_debug_sessions` reports `debuggerDisabled: true`) and every later surface says why: `set_breakpoint` and `list_breakpoints` keep the adapter's own answer (still sent — e.g. js-debug's "Unbound breakpoint", or CodeLLDB's refusal echoed onto each pre-launch breakpoint) and add "the debugger is off for this launch (noDebug is true)…", `pause_execution` is still sent and the same sentence explains a pause that never lands or one the adapter refused, and `step_*`/`continue_execution`/`get_stack_trace`/`get_local_variables`/`evaluate_expression` append it to their "not paused" answers. A `stopped` event only a live debugger produces — a breakpoint the adapter itself reports (or names in `hitBreakpointIds`), an exception stop, an entry stop — proves the debugger on for that adapter build and clears the decision, and so does a breakpoint the adapter verified (every honouring adapter refuses or unbinds one under the flag, so a bound one can only come from a build that ignores it); a pause, a step taken from one, or a `debugger;` statement do not (js-debug lands all three under the flag while its breakpoints stay unbound; an uncaught throw does not stop). On a paused session the sentence keeps only the clause still true of it — breakpoints cannot bind — and outside a live launch (`initializing`/`running`/`paused`) the decision is neither reported nor consulted (issue #749). - Additional DAP launch keys (`program`, `cwd`, `env`, language-specific options) pass through to the adapter. Top-level parameters do **not** belong here: a nested `breakOnExceptions` is honored as an alias (the top-level value wins if both are given) and reported via a `warning` in the response; other misplaced top-level keys (`dryRunSpawn`, `sessionId`, `scriptPath`, `adapterLaunchConfig`) are stripped with a warning instead of silently riding into the launch config. - `adapterLaunchConfig` (object, optional): Adapter-specific launch configuration overrides. Use this for language-specific settings that go beyond standard DAP arguments (e.g., `mainClass` and `classpath` for Java, `buildCommand` for Rust). For Rust, `_adapterSettings` passes through to CodeLLDB (issue #441) — e.g. `{"_adapterSettings": {"scriptConfig": {"lang": {"rust": {"sysroot": "/path"}}}}}` points the Rust formatter lookup at an explicit sysroot; the `CODELLDB_RUST_SYSROOT` env var does the same without per-launch config (a user-supplied `_adapterSettings` value wins over the env var). - `dryRunSpawn` (boolean, optional): Test spawn without actually starting diff --git a/src/server.ts b/src/server.ts index 36449c5e0..565454e85 100644 --- a/src/server.ts +++ b/src/server.ts @@ -530,6 +530,16 @@ export class DebugMcpServer implements ToolContext { } return result; } + // A session that is not paused has no stack to read, and the session + // layer's answer — "not paused", with the why when the launch runs with + // the debugger off (issue #749) — needs no thread. Say so before asking + // the adapter for one: a launch under noDebug never stops, so no thread + // is ever current, and debugpy refuses the `threads` discovery ("Server + // is not available") — a wasted round trip at best, the DAP timeout on + // a wedged adapter at worst. + if (session.state !== SessionState.PAUSED) { + return this.sessionManager.getStackTraceDetailed(sessionId, undefined, includeInternals); + } let currentThreadId = session.proxyManager.getCurrentThreadId(); // If no thread ID is known (e.g. adapter omitted threadId from stopped event), // try to discover one via a 'threads' DAP request. @@ -545,18 +555,10 @@ export class DebugMcpServer implements ToolContext { } } if (typeof currentThreadId !== 'number') { - // No thread is known and the adapter named none. On a session that - // is not paused that is expected — nothing has stopped yet, or the - // launch runs with the debugger off and the adapter refuses even - // `threads` (debugpy: "Server is not available") — and the proxy is - // alive, so "no active proxy" would be false; the session layer's - // not-paused answer (with the why, issue #749) needs no thread. A - // paused session with no thread to name is the anomaly the error - // is for. - if (session.state !== SessionState.PAUSED) { - return this.sessionManager.getStackTraceDetailed(sessionId, undefined, includeInternals); - } - throw new ProxyNotRunningError(sessionId || 'unknown', 'get stack trace'); + // Paused, no thread known, and the adapter named none: the proxy is + // alive, so "no active proxy" would be false. The session layer says + // what is true — no stopped thread is known for this session. + return this.sessionManager.getStackTraceDetailed(sessionId, undefined, includeInternals); } // ensureStackReady: the thread above was resolved implicitly (the MCP tool // has no threadId argument), so a paused session answering with zero diff --git a/src/session/debugger-off.ts b/src/session/debugger-off.ts index 1d5428b0d..dcc6c8795 100644 --- a/src/session/debugger-off.ts +++ b/src/session/debugger-off.ts @@ -14,6 +14,33 @@ import { ErrorMessages } from '../utils/error-messages.js'; export interface DebuggerOffView { state: SessionState; launchDebuggerOff?: boolean; + breakpoints?: ReadonlyMap; + functionBreakpoints?: ReadonlyMap; +} + +/** + * Whether the adapter has verified a breakpoint of this launch — proof as + * strong as a stop that this build debugs after all, and it arrives before + * any hit. Measured: every adapter that honours the flag refuses or unbinds + * a breakpoint under it (js-debug "Unbound breakpoint", debugpy "Server is + * not available", Delve "noDebug mode: unable to process 'setBreakpoints'", + * CodeLLDB "Not supported in noDebug mode"), so a verified record can only + * come from a build that ignores the flag. Per-launch state (bindings are + * reset at each launch), so it is consulted on read rather than clearing + * the record the way a stop does. + */ +export function adapterVerifiedABreakpoint(session: DebuggerOffView): boolean { + for (const bp of session.breakpoints?.values() ?? []) { + if (bp.verified) { + return true; + } + } + for (const bp of session.functionBreakpoints?.values() ?? []) { + if (bp.verified) { + return true; + } + } + return false; } /** @@ -47,14 +74,16 @@ export function stopProvesDebuggerOn( * still goes to the adapter), running, or paused. Over (stopped, error) or * never launched (created — a launch refused before the proxy existed * leaves the record behind), it describes nothing that is running: a - * breakpoint set then is an ordinary queued one for the next launch. + * breakpoint set then is an ordinary queued one for the next launch. And + * not once the adapter has verified a breakpoint (see above). */ export function isDebuggerOff(session: DebuggerOffView): boolean { return ( session.launchDebuggerOff === true && (session.state === SessionState.INITIALIZING || session.state === SessionState.RUNNING || - session.state === SessionState.PAUSED) + session.state === SessionState.PAUSED) && + !adapterVerifiedABreakpoint(session) ); } diff --git a/src/session/execution/execution-controller.ts b/src/session/execution/execution-controller.ts index cdac0f36f..387bb65c9 100644 --- a/src/session/execution/execution-controller.ts +++ b/src/session/execution/execution-controller.ts @@ -148,7 +148,8 @@ function notPausedError(session: DebuggerOffView): string { /** * A pause the adapter answered with an error, with the why beside the * adapter's own words when the launch runs with the debugger off (issue - * #749). The adapter's error is untouched and travels as the cause. + * #749). The adapter's error travels as the cause exactly as it was + * thrown — an Error, or whatever else the bridge rejected with. */ function withDebuggerOffWhy(session: DebuggerOffView, error: unknown, message: string): { error: Error; message: string } { const err = error instanceof Error ? error : new Error(message); @@ -157,7 +158,7 @@ function withDebuggerOffWhy(session: DebuggerOffView, error: unknown, message: s return { error: err, message: err.message }; } const composed = ErrorMessages.withDebuggerOffWhy(err.message, why); - return { error: new Error(composed, { cause: err }), message: composed }; + return { error: new Error(composed, { cause: error }), message: composed }; } export class ExecutionController { @@ -529,7 +530,16 @@ export class ExecutionController { } if (session.state !== SessionState.RUNNING) { - return { success: false, error: `Cannot pause in state: ${session.state}`, state: session.state }; + // With the why while the launch is still initializing with the + // debugger off (issue #749) — every other surface carries it in that + // state. + const why = debuggerOffWhy(session); + const refusal = `Cannot pause in state: ${session.state}`; + return { + success: false, + error: why ? ErrorMessages.withDebuggerOffWhy(refusal, why) : refusal, + state: session.state + }; } this.ctx.logger.debug(`[SessionManager] pauseExecution: sending DAP pause for session=${sessionId} currentState=${session.state}`); diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index 9c7c7ed1a..04373cc61 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -39,6 +39,7 @@ import { import { waitForLaunchReadiness } from './launch-readiness.js'; import type { ProxyLauncher } from './proxy-launcher.js'; import type { InFlightGuard } from '../in-flight-guard.js'; +import { adapterVerifiedABreakpoint } from '../debugger-off.js'; /** * A launch flag the way the adapter will see it. The proxy launcher merges @@ -542,16 +543,20 @@ export class DebugLauncher { } // The policy's word is a static pin; a stop that arrived anyway is the - // stronger evidence (an adapter build that ignores the flag after all). - // The core's stopped handler is the one judge of that — it clears the - // recorded decision on a stop only a live debugger produces (issue - // #749), so the launch response and the record cannot disagree. Then - // the debugger was on: keep the ordinary diagnostics and say the flag - // had no effect rather than that the breakpoints will not fire. (A - // launch that ends paused on a pause or a step keeps the record and - // the warning: the flag still keeps its breakpoints from binding.) - const stoppedAnyway = debuggerOff && finalSession.launchDebuggerOff !== true; - const noDebugNote = stoppedAnyway + // stronger evidence (an adapter build that ignores the flag after all): + // a stop only a live debugger produces — the core's stopped handler + // is the one judge of that and clears the recorded decision (issue + // #749) — or a breakpoint the adapter verified in its configuration + // phase, which is read from the same evidence every later surface + // reads. Either way the launch response and the record cannot + // disagree. Then the debugger was on: keep the ordinary diagnostics + // and say the flag had no effect rather than that the breakpoints + // will not fire. (A launch that ends paused on a pause or a step + // keeps the record and the warning: the flag still keeps its + // breakpoints from binding.) + const debuggerOnAnyway = + debuggerOff && (finalSession.launchDebuggerOff !== true || adapterVerifiedABreakpoint(finalSession)); + const noDebugNote = debuggerOnAnyway ? buildNoDebugLaunchWarning(finalSession, { noDebug }, breakOnExceptions, false) : noDebugWarning; @@ -559,7 +564,7 @@ export class DebugLauncher { // ("check the file path", "check the symbol name", "will PAUSE") that // has one cause when the debugger is off — the noDebug warning names // it, and they are withheld so they cannot contradict it (issue #710). - const debuggerOn = !debuggerOff || stoppedAnyway; + const debuggerOn = !debuggerOff || debuggerOnAnyway; // Unbound-at-launch warning (issue #308): the verified state is fresh // after the re-sync above, so a name the adapter could not resolve is diff --git a/src/utils/error-messages.ts b/src/utils/error-messages.ts index 1170f9edb..c320ddc84 100644 --- a/src/utils/error-messages.ts +++ b/src/utils/error-messages.ts @@ -168,9 +168,8 @@ export const ErrorMessages = { * stepping and inspection. Names the fact and the remedy, never the * adapter's own answer, which stays as it came. "No stop is expected" * rather than "cannot come": js-debug still lands a pause under the flag. - * Used in: src/server/handlers/breakpoint-tools.ts, src/server/handlers/inspection-tools.ts, - * src/session/execution/execution-controller.ts, src/session/inspection/frame-anchor-resolver.ts, - * src/session/inspection/expression-evaluator.ts + * Used in: src/session/debugger-off.ts (`debuggerOffWhy`, the one gate every + * surface reads it through — the handlers and controllers never name it) */ debuggerOffForLaunch: DEBUGGER_OFF_FOR_LAUNCH, diff --git a/tests/core/unit/server/server-inspection-tools.test.ts b/tests/core/unit/server/server-inspection-tools.test.ts index ea6dc8972..26614bc78 100644 --- a/tests/core/unit/server/server-inspection-tools.test.ts +++ b/tests/core/unit/server/server-inspection-tools.test.ts @@ -437,17 +437,21 @@ describe('Server Inspection Tools Tests', () => { expect(content.diagnostics).toEqual({ proxyLogPath: '/logs/proxy-test-session.log' }); }); - it('should handle missing thread ID on a paused session', async () => { + it('answers a paused session with no thread to name with the session layer\'s note, not "no active proxy"', async () => { const mockSession = { state: SessionState.PAUSED, failureDiagnostics: { proxyLogPath: '/logs/proxy-test-session.log' }, proxyManager: { - getCurrentThreadId: vi.fn().mockReturnValue(null) + getCurrentThreadId: vi.fn().mockReturnValue(null), + sendDapRequest: vi.fn().mockResolvedValue({ body: { threads: [] } }) } }; - mockSessionManager.getSession.mockReturnValue(mockSession); - + mockSessionManager.getStackTraceDetailed.mockResolvedValue({ + frames: [], totalFrameCount: 0, hiddenFrameCount: 0, allFramesInternal: false, + note: 'No stopped thread is known for this session.' + }); + const result = await callToolHandler({ method: 'tools/call', params: { @@ -455,11 +459,12 @@ describe('Server Inspection Tools Tests', () => { arguments: { sessionId: 'test-session' } } }); - - // The server returns a structured failure result (success: false) with an error message + + // The proxy is alive; the resolver's answer is the truthful one. const content = JSON.parse(result.content[0].text); - expect(content.success).toBe(false); - expect(content.error).toContain('no active proxy for session test-session'); + expect(content.success).toBe(true); + expect(content.stackFrames).toEqual([]); + expect(content.note).toContain('No stopped thread is known for this session.'); }); it('should surface SessionManager errors as a truthful tool-level failure', async () => { diff --git a/tests/core/unit/session/session-manager-nodebug-warning.test.ts b/tests/core/unit/session/session-manager-nodebug-warning.test.ts index 52795ae1f..0e251a094 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -416,6 +416,36 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(Date.now() - before).toBeLessThan(30000); }); + it('believes a breakpoint the adapter verified anyway over the policy pin — before any stop', async () => { + // A wrong pin seen from the other side: the adapter binds the + // breakpoint (its configuration-phase echo says verified) and the + // program keeps running. The launch must not say "will not fire" of + // a breakpoint list_breakpoints shows bound; the record stays (a stop + // is what clears it) but is consulted with the evidence. + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + const [queued] = sessionManager.listBreakpoints(s.id); + const proxy = dependencies.mockProxyManager; + proxy.start = vi.fn().mockImplementation(async (startConfig) => { + setMockProxyRunning(proxy, true); + proxy.startCalls.push(startConfig); + proxy.emit('adapter-configured'); + proxy.emit('initialized'); + proxy.simulateEvent('breakpoints-synced', [ + { id: queued.id, file: '/work/src/app.py', line: 7, verified: true, adapterId: 3 } + ]); + }) as MockProxyManager['start']; + + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(result.state).toBe(SessionState.RUNNING); + expect(warningOf(result)).toMatch(/noDebug has no effect/); + expect(warningOf(result)).not.toMatch(/will not fire/); + expect(sessionManager.listBreakpoints(s.id)[0].verified).toBe(true); + const listed = sessionManager.getAllSessions().find((x) => x.id === s.id); + expect(listed).not.toHaveProperty('debuggerDisabled'); + }); + it('warns on a dry run too — it is a configuration check', async () => { const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); diff --git a/tests/core/unit/session/session-store-projection.test.ts b/tests/core/unit/session/session-store-projection.test.ts index 28f4814cc..2df723b83 100644 --- a/tests/core/unit/session/session-store-projection.test.ts +++ b/tests/core/unit/session/session-store-projection.test.ts @@ -78,4 +78,27 @@ describe('SessionStore.getAll() debuggerDisabled projection (issue #749)', () => expect(store.getAll().find((s) => s.id === id)!.debuggerDisabled, state).toBe(true); } }); + + it('drops the projection once the adapter has verified a breakpoint — proof this build debugs after all', () => { + // Measured: every adapter that honours the flag refuses or unbinds a + // breakpoint under it (js-debug "Unbound breakpoint", debugpy "Server is + // not available", Delve "noDebug mode: unable to process + // 'setBreakpoints'", CodeLLDB "Not supported in noDebug mode"), so a + // record the adapter verified can only come from a build that ignores + // the flag — as strong as a stop, and it arrives before any hit. + const { store, id } = storeWith(SessionState.RUNNING); + const managed = store.get(id)!; + managed.launchDebuggerOff = true; + const bp = { id: 'bp-1', file: 'a.py', line: 3, verified: false }; + managed.breakpoints.set(bp.id, bp as never); + expect(store.getAll().find((s) => s.id === id)!.debuggerDisabled).toBe(true); + + bp.verified = true; + expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); + + // A verified function breakpoint counts the same way. + bp.verified = false; + managed.functionBreakpoints.set('fbp-1', { id: 'fbp-1', name: 'main', verified: true } as never); + expect(store.getAll().find((s) => s.id === id)!).not.toHaveProperty('debuggerDisabled'); + }); }); diff --git a/tests/unit/server-coverage.test.ts b/tests/unit/server-coverage.test.ts index 728af3d35..e99aad57d 100644 --- a/tests/unit/server-coverage.test.ts +++ b/tests/unit/server-coverage.test.ts @@ -200,6 +200,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { }; mockSessionManager.getSession.mockReturnValue({ id: 'test-session', + state: SessionState.PAUSED, sessionLifecycle: SessionLifecycleState.ACTIVE, proxyManager: mockProxy }); @@ -215,7 +216,12 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { expect(result.frames).toHaveLength(1); }); - it('should throw when a paused getStackTrace has no thread and the threads request fails', async () => { + // A paused session with no current thread asks the adapter (`threads`); + // when that names none too, the session layer answers — "No stopped + // thread is known for this session." — because the proxy is alive and + // "no active proxy" would be false (issue #749). The error is reserved + // for a session with no proxy at all. + it('hands a paused session with no thread to the session layer when the threads request fails', async () => { const mockProxy = { getCurrentThreadId: () => null, isRunning: () => true, @@ -227,12 +233,19 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { sessionLifecycle: SessionLifecycleState.ACTIVE, proxyManager: mockProxy }); + const noThread = { + frames: [], totalFrameCount: 0, hiddenFrameCount: 0, allFramesInternal: false, + note: 'No stopped thread is known for this session.' + }; + mockSessionManager.getStackTraceDetailed.mockResolvedValue(noThread); - await expect(server.getStackTrace('test-session')) - .rejects.toThrow('Cannot get stack trace: no active proxy'); + await expect(server.getStackTrace('test-session')).resolves.toBe(noThread); + + expect(mockProxy.sendDapRequest).toHaveBeenCalledWith('threads', {}); + expect(mockSessionManager.getStackTraceDetailed).toHaveBeenCalledWith('test-session', undefined, false); }); - it('should throw when a paused getStackTrace has no thread and the threads response is empty', async () => { + it('hands a paused session with no thread to the session layer when the threads response is empty', async () => { const mockProxy = { getCurrentThreadId: () => null, isRunning: () => true, @@ -244,17 +257,24 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { sessionLifecycle: SessionLifecycleState.ACTIVE, proxyManager: mockProxy }); + const noThread = { + frames: [], totalFrameCount: 0, hiddenFrameCount: 0, allFramesInternal: false, + note: 'No stopped thread is known for this session.' + }; + mockSessionManager.getStackTraceDetailed.mockResolvedValue(noThread); - await expect(server.getStackTrace('test-session')) - .rejects.toThrow('Cannot get stack trace: no active proxy'); + await expect(server.getStackTrace('test-session')).resolves.toBe(noThread); + + expect(mockSessionManager.getStackTraceDetailed).toHaveBeenCalledWith('test-session', undefined, false); }); - it('hands a running session with no thread to the session layer instead of claiming no active proxy (issue #749)', async () => { + it('answers a session that is not paused without asking the adapter for threads (issue #749)', async () => { // A launch that runs with the debugger off never stops, so no thread // is ever current, and debugpy refuses the `threads` discovery - // ("Server is not available"). The proxy is alive; the honest answer - // is the resolver's not-paused note with the why — the same answer a - // debug-mode running session gets once its thread is known. + // ("Server is not available") — and on a wedged adapter that request + // would block for the DAP timeout. The resolver's not-paused answer + // (with the why) needs no thread, so the discovery is skipped: the + // same answer a debug-mode running session gets, one round trip cheaper. const mockProxy = { getCurrentThreadId: () => null, isRunning: () => true, @@ -275,7 +295,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { await expect(server.getStackTrace('test-session')).resolves.toBe(notPaused); - expect(mockProxy.sendDapRequest).toHaveBeenCalledWith('threads', {}); + expect(mockProxy.sendDapRequest).not.toHaveBeenCalled(); expect(mockSessionManager.getStackTraceDetailed).toHaveBeenCalledWith('test-session', undefined, false); }); }); diff --git a/tests/unit/session-manager-operations-coverage.test.ts b/tests/unit/session-manager-operations-coverage.test.ts index 1a476aa59..d3d236fd2 100644 --- a/tests/unit/session-manager-operations-coverage.test.ts +++ b/tests/unit/session-manager-operations-coverage.test.ts @@ -4558,6 +4558,49 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () expect(step.error).toBe('Not paused'); }); + it('drops the why once the adapter has verified a breakpoint — the build debugs after all', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.launchDebuggerOff = true; + mockSession.breakpoints.set('bp-1', { id: 'bp-1', file: 'a.py', line: 3, verified: true } as never); + + const step = await operations.stepOver('test-session'); + expect(step.error).toBe('Not paused'); + + const stack = await operations.getStackTraceDetailed('test-session'); + expect(stack.note ?? '').not.toContain(why); + }); + + it('says why a pause is refused while the launch is still initializing (issue #749)', async () => { + mockSession.state = SessionState.INITIALIZING; + mockSession.launchDebuggerOff = true; + + const result = await operations.pause('test-session', 1); + + expect(result.success).toBe(false); + expect(result.error).toContain('Cannot pause in state: initializing'); + expect(result.error).toContain(why); + expect(mockProxyManager.sendDapRequest).not.toHaveBeenCalledWith('pause', expect.anything()); + }); + + it('keeps a non-Error refusal of a pause as the cause, as it was thrown', async () => { + mockSession.state = SessionState.RUNNING; + mockSession.launchDebuggerOff = true; + const refusal = { code: 'E_NODEBUG', text: 'not an Error instance' }; + mockProxyManager.sendDapRequest.mockImplementation(async (command: string) => { + if (command === 'pause') { + throw refusal; + } + return {}; + }); + + const thrown = await operations.pause('test-session', 1).then( + () => { throw new Error('expected a rejection'); }, + (err: unknown) => err as Error & { cause?: unknown } + ); + expect(thrown.message).toContain(why); + expect(thrown.cause).toBe(refusal); + }); + it('says why an expression cannot be evaluated', async () => { mockSession.state = SessionState.RUNNING; mockSession.launchDebuggerOff = true;