From e1e981483ebe6849a16155c23a75867a0a789f5a Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 18:16:01 -0400 Subject: [PATCH 1/6] fix(launch): warn when noDebug disables the breakpoints the caller set (#710) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dapLaunchArgs.noDebug is a standard DAP flag every adapter honours by not enabling the debugger (JavaScript included since #706 forwards unknown launch keys): 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` with no warning at all. - launch-warnings.ts: buildNoDebugLaunchWarning, a pure builder over the breakpoint stores, the launch args and the caller's explicit breakOnExceptions. Fires only when the caller asked for a stop (line or function breakpoints, explicit breakOnExceptions other than 'none', stopOnEntry); the policy default does not count, so a deliberate plain run stays silent. - debug-launcher.ts: decided once per launch, before the dry-run branch, so a dryRunSpawn configuration check and a restart_debugging replay carry it too (restart bypasses handler intake, which is why this lives in the launcher). When it fires, the #308/#467/#469 breakpoint-shaped warnings are withheld — each diagnoses a symptom of this one cause. - tool-schemas.ts + docs/tool-reference.md: document the flag; the tool-list snapshot fence re-recorded for the new property. - Tests: builder + launcher/dry-run/restart wiring (mock proxy), and an e2e against real js-debug asserting the warning replaces the #467 text. Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/710.fixed.md | 1 + docs/tool-reference.md | 1 + src/server/tool-schemas.ts | 3 +- src/session/breakpoints/launch-warnings.ts | 49 +++++ src/session/launch/debug-launcher.ts | 28 ++- .../bp-assert.va-explicit.container-true.json | 4 + ...bp-assert.va-explicit.container-unset.json | 4 + .../bp-assert.va-open.container-true.json | 4 + .../bp-assert.va-open.container-unset.json | 4 + .../bp-assert.va-unset.container-true.json | 4 + .../bp-assert.va-unset.container-unset.json | 4 + ...bp-content.va-explicit.container-true.json | 4 + ...p-content.va-explicit.container-unset.json | 4 + .../bp-content.va-open.container-true.json | 4 + .../bp-content.va-open.container-unset.json | 4 + .../bp-content.va-unset.container-true.json | 4 + .../bp-content.va-unset.container-unset.json | 4 + .../bp-line.va-explicit.container-true.json | 4 + .../bp-line.va-explicit.container-unset.json | 4 + .../bp-line.va-open.container-true.json | 4 + .../bp-line.va-open.container-unset.json | 4 + .../bp-line.va-unset.container-true.json | 4 + .../bp-line.va-unset.container-unset.json | 4 + .../bp-unset.va-explicit.container-true.json | 4 + .../bp-unset.va-explicit.container-unset.json | 4 + .../bp-unset.va-open.container-true.json | 4 + .../bp-unset.va-open.container-unset.json | 4 + .../bp-unset.va-unset.container-true.json | 4 + .../bp-unset.va-unset.container-unset.json | 4 + .../session-manager-nodebug-warning.test.ts | 199 ++++++++++++++++++ .../mcp-server-break-on-exceptions.test.ts | 34 +++ 31 files changed, 406 insertions(+), 5 deletions(-) create mode 100644 changelog.d/710.fixed.md create mode 100644 tests/core/unit/session/session-manager-nodebug-warning.test.ts diff --git a/changelog.d/710.fixed.md b/changelog.d/710.fixed.md new file mode 100644 index 000000000..06a248e2b --- /dev/null +++ b/changelog.d/710.fixed.md @@ -0,0 +1 @@ +**A `noDebug` launch says the debugger is off instead of blaming your breakpoints** — `dapLaunchArgs.noDebug: true` is a standard DAP flag every adapter honours by not enabling the debugger (JavaScript included since #706 forwards unrecognised launch keys), so 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` with no warning while the caller waited for a stop that could not come. `start_debugging` now warns when `noDebug` 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, and withholds the breakpoint-shaped launch warnings (#308, #467, #469) that presuppose a debugger so they cannot contradict it. Decided 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/docs/tool-reference.md b/docs/tool-reference.md index 35ad151f2..01c5c2f25 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -366,6 +366,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): Run the program with the debugger disabled (the standard DAP flag; default `false`). Every adapter honours it: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives — a short script ends `stopped`, a server stays `running`. A bare `noDebug` run is legitimate and says nothing; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the response `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). The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. - 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/tool-schemas.ts b/src/server/tool-schemas.ts index 561fce649..26b2aa2af 100644 --- a/src/server/tool-schemas.ts +++ b/src/server/tool-schemas.ts @@ -148,7 +148,8 @@ export function buildToolDefinitions(options: BuildToolDefinitionsOptions): Tool type: 'object', 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' } + 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: 'Run the program with the debugger disabled (standard DAP flag, default false): 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' } }, additionalProperties: true }, diff --git a/src/session/breakpoints/launch-warnings.ts b/src/session/breakpoints/launch-warnings.ts index efdab35c0..9c494be0b 100644 --- a/src/session/breakpoints/launch-warnings.ts +++ b/src/session/breakpoints/launch-warnings.ts @@ -50,6 +50,55 @@ export function buildUnboundBreakpointExitWarning( ); } +/** + * noDebug launch warning (issue #710). `dapLaunchArgs.noDebug: true` is a + * standard DAP flag every adapter honours by not enabling the debugger: no + * breakpoint binds, no exception filter arms, no entry stop lands, and no + * `stopped` ever arrives. That is a legitimate way to just run the program, + * so the warning fires only when the caller also asked for a stop — line or + * function breakpoints in the store, an *explicit* breakOnExceptions other + * than 'none' (the caller's value, not the policy default the launcher fills + * in afterwards, which would make every bare noDebug run warn), or + * stopOnEntry — and names each thing that will not fire. The breakpoint-shaped + * launch warnings (#308, #467, #469) presuppose a debugger and are withheld + * when this one fires. + */ +export function buildNoDebugLaunchWarning( + session: Pick, + dapLaunchArgs: { noDebug?: boolean; stopOnEntry?: boolean } | undefined, + explicitBreakOnExceptions: string | undefined +): string | undefined { + if (dapLaunchArgs?.noDebug !== true) { + return undefined; + } + const expected: string[] = []; + const lineCount = session.breakpoints.size; + const functionCount = session.functionBreakpoints?.size ?? 0; + if (lineCount > 0) { + expected.push(`${lineCount} breakpoint(s)`); + } + if (functionCount > 0) { + expected.push(`${functionCount} function breakpoint(s)`); + } + if (explicitBreakOnExceptions !== undefined && explicitBreakOnExceptions !== 'none') { + expected.push(`breakOnExceptions='${explicitBreakOnExceptions}'`); + } + if (dapLaunchArgs.stopOnEntry === true) { + expected.push('stopOnEntry'); + } + if (expected.length === 0) { + return undefined; + } + const list = + expected.length === 1 + ? expected[0] + : `${expected.slice(0, -1).join(', ')} and ${expected[expected.length - 1]}`; + return ( + `dapLaunchArgs.noDebug is true, so the debugger is disabled for this launch and no stop can arrive: ` + + `${list} will not fire. Drop noDebug to debug, or ignore this if you only meant to run the program` + ); +} + /** What a launch reports when it ended STOPPED: a sentence for the message, and the fields behind it. */ export interface RunToCompletionSummary { /** Appended to the launch message after "Current state: stopped". */ diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index dcb302fe0..b4819ae94 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -26,6 +26,7 @@ import type { BreakpointController } from '../breakpoints/breakpoint-controller. import { reresolveAnchors } from '../breakpoints/anchor-resolution.js'; import { buildLogpointDowngradeLaunchWarning, + buildNoDebugLaunchWarning, buildUnboundBreakpointExitWarning, buildRunToCompletionSummary } from '../breakpoints/launch-warnings.js'; @@ -221,6 +222,13 @@ export class DebugLauncher { }; } + // noDebug disables the debugger, so nothing the caller asked to stop on + // can fire (issue #710). Decided here, before the dry-run branch, from the + // caller's own breakOnExceptions rather than the policy default resolved + // below: a dry run is a configuration check and a restart replays the + // same arguments, and both should say so. + const noDebugWarning = buildNoDebugLaunchWarning(session, dapLaunchArgs, breakOnExceptions); + try { // For dry run, start the proxy and wait for completion if (dryRunSpawn) { @@ -260,6 +268,7 @@ export class DebugLauncher { success: true, state: SessionState.STOPPED, data: { + ...(noDebugWarning ? { warning: noDebugWarning } : {}), dryRun: true, message: 'Dry run spawn command logged by proxy.', command: initialDryRunSnapshot?.command, @@ -296,6 +305,7 @@ export class DebugLauncher { success: true, state: SessionState.STOPPED, data: { + ...(noDebugWarning ? { warning: noDebugWarning } : {}), dryRun: true, message: 'Dry run spawn command logged by proxy.', command: latestSnapshot?.command, @@ -452,32 +462,42 @@ export class DebugLauncher { await this.breakpoints.resyncAll(finalSession); } + // The three breakpoint-shaped warnings below each diagnose a symptom + // ("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 = noDebugWarning === undefined; + // 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 // reported here instead of failing silently at "the program never // stopped". Suppressed for bind-late adapters (js/java), where // unverified-at-launch is the designed deferral path. - const fnBpWarning = this.breakpoints.functionBreakpointLaunchWarning(finalSession); + const fnBpWarning = debuggerOn + ? this.breakpoints.functionBreakpointLaunchWarning(finalSession) + : undefined; // Ran-to-completion with breakpoints that never bound (issue #467): // state "stopped" where the caller expected "paused" is only // explainable via list_breakpoints today — surface the stored // per-breakpoint diagnostics right here where the caller is looking. const unboundAtExitWarning = - finalState === SessionState.STOPPED + debuggerOn && finalState === SessionState.STOPPED ? buildUnboundBreakpointExitWarning(finalSession) : undefined; // Logpoint-downgrade verdict (issue #469): the deferred set_breakpoint // warning promised a launch-time answer — deliver it on this response. - const logpointWarning = buildLogpointDowngradeLaunchWarning(finalSession); + const logpointWarning = debuggerOn + ? buildLogpointDowngradeLaunchWarning(finalSession) + : undefined; // Adapter degradation notes (issue #441) accumulate on the session as // annotated output events arrive; joining here is best-effort — a note // arriving after this return still lands in the output buffer as an // attributed [mcp-debugger] Warning entry. const launchWarning = - [fnBpWarning, logpointWarning, unboundAtExitWarning, ...(finalSession.adapterNotices ?? [])] + [noDebugWarning, fnBpWarning, logpointWarning, unboundAtExitWarning, ...(finalSession.adapterNotices ?? [])] .filter(Boolean) .join('; ') || undefined; 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 f16a0cb05..78e369684 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 @@ -213,6 +213,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 e1266a5e3..32fbf3cd0 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 @@ -213,6 +213,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 f74ea12e1..f35fcf76e 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 @@ -213,6 +213,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 23cd205c4..b98e400b1 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 @@ -213,6 +213,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 f74ea12e1..f35fcf76e 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 @@ -213,6 +213,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 23cd205c4..b98e400b1 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 @@ -213,6 +213,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 f01f65a10..5058cecf9 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 b2e801fbf..ff93bcc41 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 5bbd83a62..4135bc3a4 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 fb526ba86..09f56829d 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 5bbd83a62..4135bc3a4 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 fb526ba86..09f56829d 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 dbe8d892d..df268f865 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 @@ -209,6 +209,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 60c38c184..0f934bf8e 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 @@ -209,6 +209,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 86f2313f6..e11e4bd5f 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 @@ -209,6 +209,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 e9278fdb2..f5982a60b 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 @@ -209,6 +209,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 86f2313f6..e11e4bd5f 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 @@ -209,6 +209,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 e9278fdb2..f5982a60b 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 @@ -209,6 +209,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 f01f65a10..5058cecf9 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 b2e801fbf..ff93bcc41 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 5bbd83a62..4135bc3a4 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 fb526ba86..09f56829d 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 5bbd83a62..4135bc3a4 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "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 fb526ba86..09f56829d 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 @@ -223,6 +223,10 @@ "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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" } }, "additionalProperties": 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 new file mode 100644 index 000000000..74c8f033d --- /dev/null +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -0,0 +1,199 @@ +/** + * buildNoDebugLaunchWarning (issue #710): `dapLaunchArgs.noDebug: true` reaches + * every adapter and disables the debugger, so nothing the caller asked to stop + * on can fire. The warning names what will not fire — and stays silent for a + * bare noDebug run, which is a legitimate "just run it". + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { buildNoDebugLaunchWarning } from '../../../../src/session/breakpoints/launch-warnings.js'; +import type { ManagedSession } from '../../../../src/session/session-store.js'; +import { SessionManager, type SessionManagerConfig } from '../../../../src/session/session-manager.js'; +import { DebugLanguage, SessionState } from '@debugmcp/shared'; +import { createMockDependencies } from './session-manager-test-utils.js'; +import type { MockProxyManager } from '../../../test-utils/mocks/mock-proxy-manager.js'; + +type BuilderSession = { + breakpoints: Map; + functionBreakpoints: Map; +}; + +const noBreakpoints = (): BuilderSession => ({ breakpoints: new Map(), functionBreakpoints: new Map() }); + +const withLineBreakpoints = (count: number): BuilderSession => ({ + breakpoints: new Map( + Array.from({ length: count }, (_, i) => [`bp${i}`, { file: '/proj/app.py', line: 10 + i, verified: false }]) + ), + functionBreakpoints: new Map() +}); + +function build( + session: BuilderSession, + dapLaunchArgs: Record | undefined, + explicitBreakOnExceptions?: string +): string | undefined { + // The builder is a free function over the two breakpoint stores plus the + // launch arguments and the caller's explicit exception mode. + return buildNoDebugLaunchWarning( + session as unknown as Pick, + dapLaunchArgs, + explicitBreakOnExceptions + ); +} + +describe('buildNoDebugLaunchWarning', () => { + it('names the line breakpoints that will not fire', () => { + const warning = build(withLineBreakpoints(2), { noDebug: true }); + expect(warning).toMatch(/noDebug is true/); + expect(warning).toMatch(/2 breakpoint\(s\)/); + expect(warning).toMatch(/will not fire/); + expect(warning).toMatch(/Drop noDebug/); + }); + + it('names function breakpoints separately from line breakpoints', () => { + const session = withLineBreakpoints(1); + session.functionBreakpoints.set('f', { functionName: 'main', verified: false }); + const warning = build(session, { noDebug: true }); + expect(warning).toMatch(/1 breakpoint\(s\)/); + expect(warning).toMatch(/1 function breakpoint\(s\)/); + }); + + it("names an explicit breakOnExceptions other than 'none'", () => { + const warning = build(noBreakpoints(), { noDebug: true }, 'uncaught'); + expect(warning).toMatch(/breakOnExceptions='uncaught'/); + expect(warning).not.toMatch(/breakpoint\(s\)/); + }); + + it("stays silent for an explicit breakOnExceptions of 'none' with nothing else set", () => { + expect(build(noBreakpoints(), { noDebug: true }, 'none')).toBeUndefined(); + }); + + it('does not treat the policy default as something the caller asked for', () => { + // The launcher passes the caller's value, undefined when unset; the + // 'uncaught' policy default must not make every bare noDebug run warn. + expect(build(noBreakpoints(), { noDebug: true }, undefined)).toBeUndefined(); + }); + + it('names stopOnEntry as a stop that will not come', () => { + const warning = build(noBreakpoints(), { noDebug: true, stopOnEntry: true }); + expect(warning).toMatch(/stopOnEntry/); + }); + + it('stays silent when noDebug is absent or false, whatever else is set', () => { + const session = withLineBreakpoints(3); + expect(build(session, undefined, 'all')).toBeUndefined(); + expect(build(session, { stopOnEntry: true }, 'all')).toBeUndefined(); + expect(build(session, { noDebug: false, stopOnEntry: true }, 'all')).toBeUndefined(); + }); + + it('lists every applicable clause in one sentence', () => { + const session = withLineBreakpoints(2); + session.functionBreakpoints.set('f', { functionName: 'main', verified: false }); + const warning = build(session, { noDebug: true, stopOnEntry: true }, 'all'); + expect(warning).toMatch( + /2 breakpoint\(s\), 1 function breakpoint\(s\), breakOnExceptions='all' and stopOnEntry will not fire/ + ); + }); +}); + +/** + * Wiring: the launcher computes the warning once per launch and puts it on + * data.warning for a real launch, a dry run and a restart; when it fires, the + * breakpoint-shaped launch warnings that presuppose a debugger are withheld. + */ +describe('SessionManager launches with noDebug (issue #710)', () => { + let sessionManager: SessionManager; + let dependencies: ReturnType; + let config: SessionManagerConfig; + + beforeEach(() => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + dependencies = createMockDependencies(); + config = { + logDirBase: '/tmp/test-sessions', + defaultDapLaunchArgs: { stopOnEntry: false, justMyCode: true } + }; + sessionManager = new SessionManager(config, dependencies); + }); + + afterEach(async () => { + await sessionManager.closeAllSessions(); + vi.useRealTimers(); + }); + + /** Replace the mock proxy's start with one that ends the program during startup. */ + function endDuringStartup(): void { + const proxy = dependencies.mockProxyManager; + proxy.start = vi.fn().mockImplementation(async (startConfig) => { + (proxy as unknown as { _isRunning: boolean })._isRunning = true; + proxy.startCalls.push(startConfig); + process.nextTick(() => proxy.emit('exited', 0)); + }) as MockProxyManager['start']; + } + + it('says which breakpoints will not fire instead of telling the caller to check their paths', async () => { + const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(session.id, { file: '/work/src/app.py', line: 7 }); + endDuringStartup(); + + const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { stopOnEntry: false, noDebug: true }); + await vi.runAllTimersAsync(); + const result = await startPromise; + + expect(result.success).toBe(true); + expect(result.state).toBe(SessionState.STOPPED); + const warning = (result.data as { warning?: string }).warning; + expect(warning).toMatch(/noDebug is true/); + expect(warning).toMatch(/1 breakpoint\(s\)/); + // The #467 diagnosis ("check the file path and line") would be wrong here. + expect(warning).not.toMatch(/never bound during this run/); + }); + + it('stays silent for a bare noDebug run with nothing to stop on', async () => { + const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + endDuringStartup(); + + const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { stopOnEntry: false, noDebug: true }); + await vi.runAllTimersAsync(); + const result = await startPromise; + + expect(result.success).toBe(true); + expect((result.data as { warning?: string }).warning).toBeUndefined(); + }); + + it('warns on a dry run too — it is a configuration check', async () => { + const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(session.id, { file: '/work/src/app.py', line: 7 }); + + const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { noDebug: true }, true); + await vi.runAllTimersAsync(); + const result = await startPromise; + + expect(result.success).toBe(true); + const data = result.data as { dryRun?: boolean; warning?: string }; + expect(data.dryRun).toBe(true); + expect(data.warning).toMatch(/noDebug is true/); + }); + + it('warns on restart for a breakpoint added after the noDebug launch', async () => { + const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { stopOnEntry: false, noDebug: true }); + await vi.runAllTimersAsync(); + const first = await startPromise; + expect(first.success).toBe(true); + expect((first.data as { warning?: string }).warning).toBeUndefined(); + + await sessionManager.setBreakpoint(session.id, { file: '/work/src/app.py', line: 7 }); + dependencies.mockProxyManager.simulateEvent('terminated'); + await vi.runAllTimersAsync(); + + const restartPromise = sessionManager.restartDebugging(session.id); + await vi.runAllTimersAsync(); + const restarted = await restartPromise; + + expect(restarted.success).toBe(true); + const data = restarted.data as { warning?: string; breakpointsReapplied?: number }; + expect(data.breakpointsReapplied).toBe(1); + expect(data.warning).toMatch(/noDebug is true/); + expect(data.warning).toMatch(/1 breakpoint\(s\)/); + }); +}); diff --git a/tests/e2e/mcp-server-break-on-exceptions.test.ts b/tests/e2e/mcp-server-break-on-exceptions.test.ts index cbcf270b7..11423cd07 100644 --- a/tests/e2e/mcp-server-break-on-exceptions.test.ts +++ b/tests/e2e/mcp-server-break-on-exceptions.test.ts @@ -456,6 +456,40 @@ describe('Break-on-exception (issue #220)', () => { expect(stopped!.exitCode).not.toBe(0); }, 60000); + it('warns that a noDebug launch cannot stop at its breakpoints instead of blaming their paths (issue #710)', async () => { + sessionId = await createSession('javascript', 'js-nodebug-launch'); + const bp = parseSdkToolResult(await mcpClient!.callTool({ + name: 'set_breakpoint', + arguments: { sessionId, file: JS_CLEAN_SCRIPT, line: 5 } + })); + expect(bp.success, JSON.stringify(bp)).toBe(true); + + // js-debug honours the standard DAP flag ("Running with noDebug, so + // debug domains are disabled"): no breakpoint binds, no stop comes. + const startRes = parseSdkToolResult(await mcpClient!.callTool({ + name: 'start_debugging', + arguments: { + sessionId, + scriptPath: JS_CLEAN_SCRIPT, + dapLaunchArgs: { stopOnEntry: false, noDebug: true } + } + })); + expect(startRes.success, JSON.stringify(startRes)).toBe(true); + const warning = (startRes as { warning?: string }).warning; + expect(warning).toMatch(/noDebug is true/); + expect(warning).toMatch(/1 breakpoint\(s\)/); + // The #467 diagnosis would send the caller to check a path that is fine. + expect(warning).not.toMatch(/check the file path and line/); + + const stopped = await pollUntil(async () => { + const snap = await getSessionSnapshot(mcpClient!, sessionId!); + return snap?.state === 'stopped' ? snap : undefined; + }, 20000); + expect(stopped, 'session should run to completion').toBeDefined(); + expect(stopped!.lastStop).toBeUndefined(); + expect(stopped!.exitCode).toBe(0); + }, 60000); + it('reports exit code 0 for a clean run (issue #247)', async () => { sessionId = await createSession('javascript', 'js-clean-exit-code'); From d5788ff3142330428e76287195365842ab70d1c1 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 18:25:26 -0400 Subject: [PATCH 2/6] docs: state which adapters complete a noDebug launch handshake; point at #746 for debugpy (#710) Co-Authored-By: Claude Opus 5 (1M context) --- docs/tool-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 01c5c2f25..9a6b1892a 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -366,7 +366,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): Run the program with the debugger disabled (the standard DAP flag; default `false`). Every adapter honours it: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives — a short script ends `stopped`, a server stays `running`. A bare `noDebug` run is legitimate and says nothing; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the response `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). The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. + - `noDebug` (boolean): Run the program with the debugger disabled (the standard DAP flag; default `false`). Every adapter honours it: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives. Where the adapter still completes the launch handshake (js-debug does), a short script ends `stopped` and a server stays `running`; debugpy sends no `initialized` event without a debugger and the launch currently fails init instead (issue #746). A bare `noDebug` run is legitimate and says nothing; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the response `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). The same warning comes back from `restart_debugging` and from a `dryRunSpawn` check. - 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 From 7f26589e752cf27954bd07d8c07c544affce254a Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 18:59:43 -0400 Subject: [PATCH 3/6] fix(launch): gate the noDebug warning on a measured per-policy capability; say "no effect" where the flag is ignored (#710) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #747: the issue's premise that every adapter honours noDebug was wrong, and the first version of this PR carried it into a warning that was false for half the languages while suppressing the accurate diagnostics. Measured per adapter (set a breakpoint, launch with noDebug): honours it: js-debug (runs, no stop); debugpy, Delve, CodeLLDB (they honour it too, but the launch currently fails under it — #746) ignores it: rdbg, netcoredbg, the JDI bridge, the mock adapter never sees it: rust (transformLaunchConfig builds from a fixed key set) - AdapterPolicy.honoursNoDebug (pinned per language in the policy contract test): true for javascript/python/go/cpp, unset elsewhere. - The launcher resolves noDebug the way the adapter will see it (adapterLaunchConfig over dapLaunchArgs over the server defaults — the proxy launcher's merge order) and decides debuggerOff = noDebug && honoursNoDebug. Only that gates the #308/#467/#469 suppression. - Where the adapter ignores the flag the warning says so ("noDebug has no effect with the ruby adapter: the debugger stays on") and the usual diagnostics stay. - With the debugger off, readiness no longer waits for an entry stop that cannot come (stopOnEntry + noDebug used to sit on the 30 s ceiling), and the session is projected RUNNING instead of staying INITIALIZING. - Builder takes ExceptionBreakMode; tests build real Breakpoint maps; the two dry-run payloads collapse into one helper; setMockProxyRunning moves to session-manager-test-utils (was copied in two suites). - Docs, schema and fragment state which adapters do what; e2e adds the mock (ignoring) case beside the js (honouring) one. Follow-ups: #746 (go/cpp findings added), #749 (downstream surfaces). Co-Authored-By: Claude Opus 5 (1M context) --- changelog.d/710.fixed.md | 2 +- docs/tool-reference.md | 2 +- .../src/interfaces/adapter-policy-cpp.ts | 3 + .../src/interfaces/adapter-policy-go.ts | 4 + .../src/interfaces/adapter-policy-js.ts | 3 + .../src/interfaces/adapter-policy-python.ts | 3 + .../shared/src/interfaces/adapter-policy.ts | 12 + src/server/tool-schemas.ts | 2 +- src/session/breakpoints/launch-warnings.ts | 48 +-- src/session/launch/debug-launcher.ts | 96 ++++-- src/session/operations-context.ts | 1 + .../bp-assert.va-explicit.container-true.json | 2 +- ...bp-assert.va-explicit.container-unset.json | 2 +- .../bp-assert.va-open.container-true.json | 2 +- .../bp-assert.va-open.container-unset.json | 2 +- .../bp-assert.va-unset.container-true.json | 2 +- .../bp-assert.va-unset.container-unset.json | 2 +- ...bp-content.va-explicit.container-true.json | 2 +- ...p-content.va-explicit.container-unset.json | 2 +- .../bp-content.va-open.container-true.json | 2 +- .../bp-content.va-open.container-unset.json | 2 +- .../bp-content.va-unset.container-true.json | 2 +- .../bp-content.va-unset.container-unset.json | 2 +- .../bp-line.va-explicit.container-true.json | 2 +- .../bp-line.va-explicit.container-unset.json | 2 +- .../bp-line.va-open.container-true.json | 2 +- .../bp-line.va-open.container-unset.json | 2 +- .../bp-line.va-unset.container-true.json | 2 +- .../bp-line.va-unset.container-unset.json | 2 +- .../bp-unset.va-explicit.container-true.json | 2 +- .../bp-unset.va-explicit.container-unset.json | 2 +- .../bp-unset.va-open.container-true.json | 2 +- .../bp-unset.va-open.container-unset.json | 2 +- .../bp-unset.va-unset.container-true.json | 2 +- .../bp-unset.va-unset.container-unset.json | 2 +- .../session-manager-nodebug-warning.test.ts | 281 ++++++++++++------ .../session-manager-run-to-completion.test.ts | 7 +- .../session/session-manager-test-utils.ts | 12 + .../session/session-manager-workflow.test.ts | 15 +- .../mcp-server-break-on-exceptions.test.ts | 27 ++ .../shared/adapter-policy-contract.test.ts | 18 +- 41 files changed, 388 insertions(+), 196 deletions(-) diff --git a/changelog.d/710.fixed.md b/changelog.d/710.fixed.md index 06a248e2b..5d95b0aa2 100644 --- a/changelog.d/710.fixed.md +++ b/changelog.d/710.fixed.md @@ -1 +1 @@ -**A `noDebug` launch says the debugger is off instead of blaming your breakpoints** — `dapLaunchArgs.noDebug: true` is a standard DAP flag every adapter honours by not enabling the debugger (JavaScript included since #706 forwards unrecognised launch keys), so 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` with no warning while the caller waited for a stop that could not come. `start_debugging` now warns when `noDebug` 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, and withholds the breakpoint-shaped launch warnings (#308, #467, #469) that presuppose a debugger so they cannot contradict it. Decided 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 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/docs/tool-reference.md b/docs/tool-reference.md index 9a6b1892a..d3d3f18ca 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -366,7 +366,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): Run the program with the debugger disabled (the standard DAP flag; default `false`). Every adapter honours it: no breakpoint binds, no exception filter arms, no entry stop lands, and no stop ever arrives. Where the adapter still completes the launch handshake (js-debug does), a short script ends `stopped` and a server stays `running`; debugpy sends no `initialized` event without a debugger and the launch currently fails init instead (issue #746). A bare `noDebug` run is legitimate and says nothing; when the session has breakpoints (line or function), an explicit `breakOnExceptions` other than `"none"`, or `stopOnEntry`, the response `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). 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. - 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/interfaces/adapter-policy-cpp.ts b/packages/shared/src/interfaces/adapter-policy-cpp.ts index d733758cc..2a58b4a8c 100644 --- a/packages/shared/src/interfaces/adapter-policy-cpp.ts +++ b/packages/shared/src/interfaces/adapter-policy-cpp.ts @@ -34,6 +34,9 @@ import { export const CppAdapterPolicy = { name: 'cpp', + // CodeLLDB honours noDebug by refusing debugger requests ("Not supported in + // noDebug mode"), which our launch sequence currently trips over (issue #746). + honoursNoDebug: true, supportsLogPoints: true, supportsFunctionBreakpoints: true, // Unlike Rust (issue #303), a bare 'main' in C/C++ IS the user's entry diff --git a/packages/shared/src/interfaces/adapter-policy-go.ts b/packages/shared/src/interfaces/adapter-policy-go.ts index 7d3cc2c56..355e2ca88 100644 --- a/packages/shared/src/interfaces/adapter-policy-go.ts +++ b/packages/shared/src/interfaces/adapter-policy-go.ts @@ -13,6 +13,10 @@ import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from ' export const GoAdapterPolicy = { name: 'go', + // Delve runs the program undebugged under noDebug (verified live: the + // breakpoint never fired, the process exited 0); the launch currently fails + // init because no `initialized` follows (issue #746). + honoursNoDebug: true, supportsLogPoints: true, supportsFunctionBreakpoints: true, /** diff --git a/packages/shared/src/interfaces/adapter-policy-js.ts b/packages/shared/src/interfaces/adapter-policy-js.ts index af54f44c1..54b196c6d 100644 --- a/packages/shared/src/interfaces/adapter-policy-js.ts +++ b/packages/shared/src/interfaces/adapter-policy-js.ts @@ -106,6 +106,9 @@ export const JsDebugAdapterPolicy = { // Names in late-loaded modules stay verified:false by design and bind at // the next pause — unverified-at-launch is not a failure here (issue #308). functionBreakpointsBindLate: true, + // Verified live: "Running with noDebug, so debug domains are disabled" — + // the program runs, nothing binds, no stop comes (issue #710). + honoursNoDebug: true, supportsReverseStartDebugging: true, childSessionStrategy: 'launchWithPendingTarget', buildChildStartArgs: (pendingId: string, parentConfig: Record) => { diff --git a/packages/shared/src/interfaces/adapter-policy-python.ts b/packages/shared/src/interfaces/adapter-policy-python.ts index dabf47106..33384bfd5 100644 --- a/packages/shared/src/interfaces/adapter-policy-python.ts +++ b/packages/shared/src/interfaces/adapter-policy-python.ts @@ -12,6 +12,9 @@ import type { DapClientBehavior, DapClientContext, ReverseRequestResult } from ' export const PythonAdapterPolicy = { name: 'python', + // debugpy runs the program undebugged under noDebug (verified live); the + // launch currently fails init because no `initialized` follows (issue #746). + honoursNoDebug: true, supportsLogPoints: true, supportsFunctionBreakpoints: true, supportsReverseStartDebugging: false, diff --git a/packages/shared/src/interfaces/adapter-policy.ts b/packages/shared/src/interfaces/adapter-policy.ts index 3a4e58da2..daee61e13 100644 --- a/packages/shared/src/interfaces/adapter-policy.ts +++ b/packages/shared/src/interfaces/adapter-policy.ts @@ -264,6 +264,18 @@ export interface AdapterPolicy { */ functionBreakpointsBindLate?: boolean; + /** + * True when the adapter honours DAP's `noDebug` launch flag by not enabling + * the debugger, so breakpoints, exception filters and an entry stop are + * inert on such a launch and the start_debugging response says so + * (issue #710). Measured per adapter, not assumed: js-debug, debugpy, Delve + * and CodeLLDB honour it; rdbg, netcoredbg, the JDI bridge and the mock + * adapter ignore it, and the rust launch transform never forwards it. Left + * undefined, the debugger is taken to stay on and a caller who set the + * flag is told it had no effect. + */ + honoursNoDebug?: boolean; + /** * Strategy for how to create/attach to the child session when reverse startDebugging occurs */ diff --git a/src/server/tool-schemas.ts b/src/server/tool-schemas.ts index 26b2aa2af..c1a0b846a 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: 'Run the program with the debugger disabled (standard DAP flag, default false): 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' } + noDebug: { type: 'boolean', description: 'DAP\'s launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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' } }, additionalProperties: true }, diff --git a/src/session/breakpoints/launch-warnings.ts b/src/session/breakpoints/launch-warnings.ts index 9c494be0b..e30fa6f58 100644 --- a/src/session/breakpoints/launch-warnings.ts +++ b/src/session/breakpoints/launch-warnings.ts @@ -8,7 +8,7 @@ * it); as free functions the purity is the signature rather than a convention. */ import path from 'path'; -import type { AdapterPolicy } from '@debugmcp/shared'; +import type { AdapterPolicy, ExceptionBreakMode } from '@debugmcp/shared'; import { normalizeBreakpointMessage } from '../../utils/breakpoint-message.js'; import type { ManagedSession } from '../session-store.js'; @@ -51,26 +51,40 @@ export function buildUnboundBreakpointExitWarning( } /** - * noDebug launch warning (issue #710). `dapLaunchArgs.noDebug: true` is a - * standard DAP flag every adapter honours by not enabling the debugger: no - * breakpoint binds, no exception filter arms, no entry stop lands, and no - * `stopped` ever arrives. That is a legitimate way to just run the program, - * so the warning fires only when the caller also asked for a stop — line or - * function breakpoints in the store, an *explicit* breakOnExceptions other - * than 'none' (the caller's value, not the policy default the launcher fills - * in afterwards, which would make every bare noDebug run warn), or - * stopOnEntry — and names each thing that will not fire. The breakpoint-shaped - * launch warnings (#308, #467, #469) presuppose a debugger and are withheld - * when this one fires. + * noDebug launch warning (issue #710). `noDebug: true` is DAP's "launch + * without enabling debugging". Where the adapter honours it (the policy's + * `honoursNoDebug`, measured per adapter), no breakpoint binds, no exception + * filter arms, no entry stop lands, and no `stopped` ever arrives. That is a + * legitimate way to just run the program, so the warning fires only when the + * caller also asked for a stop — line or function breakpoints in the store, + * an *explicit* breakOnExceptions other than 'none' (the caller's value, not + * the policy default the launcher fills in afterwards, which would make every + * bare noDebug run warn), or stopOnEntry — and names each thing that will not + * fire. The breakpoint-shaped launch warnings (#308, #467, #469) presuppose a + * debugger and are withheld by the launcher in that case. + * + * Where the adapter ignores the flag, the debugger stays on and the caller is + * told so instead — the flag they set changes nothing, which is worth a line. + * + * `noDebug` is the flag the adapter will actually see (adapterLaunchConfig + * over dapLaunchArgs over the server defaults, the launcher's merge order); + * `stopOnEntry` is the caller's own value. */ export function buildNoDebugLaunchWarning( - session: Pick, - dapLaunchArgs: { noDebug?: boolean; stopOnEntry?: boolean } | undefined, - explicitBreakOnExceptions: string | undefined + session: Pick, + launchArgs: { noDebug?: boolean; stopOnEntry?: boolean } | undefined, + explicitBreakOnExceptions: ExceptionBreakMode | undefined, + honoursNoDebug: boolean ): string | undefined { - if (dapLaunchArgs?.noDebug !== true) { + if (launchArgs?.noDebug !== true) { return undefined; } + if (!honoursNoDebug) { + return ( + `dapLaunchArgs.noDebug has no effect with the ${session.language} adapter: the debugger stays on, ` + + `and breakpoints, breakOnExceptions and stopOnEntry work as usual` + ); + } const expected: string[] = []; const lineCount = session.breakpoints.size; const functionCount = session.functionBreakpoints?.size ?? 0; @@ -83,7 +97,7 @@ export function buildNoDebugLaunchWarning( if (explicitBreakOnExceptions !== undefined && explicitBreakOnExceptions !== 'none') { expected.push(`breakOnExceptions='${explicitBreakOnExceptions}'`); } - if (dapLaunchArgs.stopOnEntry === true) { + if (launchArgs.stopOnEntry === true) { expected.push('stopOnEntry'); } if (expected.length === 0) { diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index b4819ae94..51dd76b4c 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -39,6 +39,24 @@ import { waitForLaunchReadiness } from './launch-readiness.js'; import type { ProxyLauncher } from './proxy-launcher.js'; import type { InFlightGuard } from '../in-flight-guard.js'; +/** + * The `noDebug` the adapter will see. The proxy launcher merges + * adapterLaunchConfig over dapLaunchArgs over the server defaults; a warning + * decided from dapLaunchArgs alone would miss a flag set (or unset) by either + * of the other two. + */ +function resolveNoDebug( + defaults: Partial, + dapLaunchArgs: Partial | undefined, + adapterLaunchConfig: Record | undefined +): boolean { + const fromAdapterConfig = adapterLaunchConfig?.noDebug; + if (typeof fromAdapterConfig === 'boolean') { + return fromAdapterConfig; + } + return (dapLaunchArgs?.noDebug ?? defaults.noDebug) === true; +} + export class DebugLauncher { constructor( private readonly ctx: LaunchContext, @@ -222,16 +240,43 @@ export class DebugLauncher { }; } - // noDebug disables the debugger, so nothing the caller asked to stop on - // can fire (issue #710). Decided here, before the dry-run branch, from the - // caller's own breakOnExceptions rather than the policy default resolved - // below: a dry run is a configuration check and a restart replays the - // same arguments, and both should say so. - const noDebugWarning = buildNoDebugLaunchWarning(session, dapLaunchArgs, breakOnExceptions); + // noDebug (issue #710). Whether the flag turns the debugger off is the + // policy's word, measured per adapter: where it does, nothing the caller + // asked to stop on can fire and the breakpoint-shaped launch warnings + // below are withheld; where the adapter ignores it, the warning says so + // instead. Decided here, before the dry-run branch, from the caller's own + // breakOnExceptions rather than the policy default resolved below: a dry + // run is a configuration check and a restart replays the same arguments, + // and both should say so. + const policy = this.ctx.selectPolicy(session.language); + const noDebug = resolveNoDebug(this.ctx.defaultDapLaunchArgs, dapLaunchArgs, adapterLaunchConfig); + const honoursNoDebug = policy.honoursNoDebug === true; + const debuggerOff = noDebug && honoursNoDebug; + const noDebugWarning = buildNoDebugLaunchWarning( + session, + { noDebug, stopOnEntry: dapLaunchArgs?.stopOnEntry }, + breakOnExceptions, + honoursNoDebug + ); + // With the debugger off an entry stop cannot come, so readiness must not + // wait for one — the launch would sit on the readiness ceiling instead. + const readinessArgs = debuggerOff ? { ...dapLaunchArgs, stopOnEntry: false } : dapLaunchArgs; try { // For dry run, start the proxy and wait for completion if (dryRunSpawn) { + const dryRunResult = (snapshot: { command?: string; script?: string } | undefined): DebugResult => ({ + success: true, + state: SessionState.STOPPED, + data: { + ...(noDebugWarning ? { warning: noDebugWarning } : {}), + dryRun: true, + message: 'Dry run spawn command logged by proxy.', + command: snapshot?.command, + script: snapshot?.script, + }, + }); + // Mark that we're setting up a dry run handler const sessionWithSetup = session as ManagedSession & { _dryRunHandlerSetup?: boolean }; sessionWithSetup._dryRunHandlerSetup = true; @@ -264,17 +309,7 @@ export class DebugLauncher { ); delete sessionWithSetup._dryRunHandlerSetup; - return { - success: true, - state: SessionState.STOPPED, - data: { - ...(noDebugWarning ? { warning: noDebugWarning } : {}), - dryRun: true, - message: 'Dry run spawn command logged by proxy.', - command: initialDryRunSnapshot?.command, - script: initialDryRunSnapshot?.script, - }, - }; + return dryRunResult(initialDryRunSnapshot); } // Wait for completion with timeout @@ -301,17 +336,7 @@ export class DebugLauncher { `[SessionManager] Dry run completed for session ${sessionId}, final state: ${latestSessionState.state}` ); - return { - success: true, - state: SessionState.STOPPED, - data: { - ...(noDebugWarning ? { warning: noDebugWarning } : {}), - dryRun: true, - message: 'Dry run spawn command logged by proxy.', - command: latestSnapshot?.command, - script: latestSnapshot?.script, - }, - }; + return dryRunResult(latestSnapshot); } else { // Timeout occurred. The state is read once: the log read below is // an await, and a late dry-run-complete/exit landing during it must @@ -381,7 +406,6 @@ export class DebugLauncher { this.ctx.logger.info(`[SessionManager] ProxyManager started for session ${sessionId}`); // Perform language-specific handshake if required - const policy = this.ctx.selectPolicy(session.language); if (policy.performHandshake) { try { await policy.performHandshake({ @@ -406,18 +430,26 @@ export class DebugLauncher { // Use policy-defined readiness criteria when available. const sessionStateAfterHandshake = this.ctx.getSession(sessionId).state; const alreadyReady = policy.isSessionReady - ? policy.isSessionReady(sessionStateAfterHandshake, { stopOnEntry: dapLaunchArgs?.stopOnEntry }) + ? policy.isSessionReady(sessionStateAfterHandshake, { stopOnEntry: readinessArgs?.stopOnEntry }) : sessionStateAfterHandshake === SessionState.PAUSED; if (!alreadyReady) { // Wait for adapter to be configured, first stop event, or termination - await waitForLaunchReadiness(this.ctx, { session, sessionId, policy, dapLaunchArgs }); + await waitForLaunchReadiness(this.ctx, { session, sessionId, policy, dapLaunchArgs: readinessArgs }); } else { this.ctx.logger.info( `[SessionManager] Session ${sessionId} already ${sessionStateAfterHandshake} after handshake - skipping adapter readiness wait` ); } + // With the debugger off, the entry stop that would normally carry the + // session out of INITIALIZING cannot come either (the core projects + // RUNNING on adapter-configured only when no entry stop is expected): + // the program is running, undebugged — say so. + if (debuggerOff && this.ctx.getSession(sessionId).state === SessionState.INITIALIZING) { + this.ctx.updateState(this.ctx.getSession(sessionId), SessionState.RUNNING); + } + // Re-fetch session to get the most up-to-date state const finalSession = this.ctx.getSession(sessionId); const finalState = finalSession.state; @@ -466,7 +498,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 = noDebugWarning === undefined; + const debuggerOn = !debuggerOff; // 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/session/operations-context.ts b/src/session/operations-context.ts index 161014637..6a4e354d5 100644 --- a/src/session/operations-context.ts +++ b/src/session/operations-context.ts @@ -141,6 +141,7 @@ export type LaunchContext = Pick< | 'adapterRegistry' | 'launchValidationCache' | 'dryRunTimeoutMs' + | 'defaultDapLaunchArgs' | 'getSession' | 'updateSession' | 'updateState' 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 78e369684..0ecfeb870 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 32fbf3cd0..00588b003 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 f35fcf76e..e31d553b4 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 b98e400b1..dcec8c0f2 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 f35fcf76e..e31d553b4 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 b98e400b1..dcec8c0f2 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 5058cecf9..0ae4e194d 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 ff93bcc41..8158b2220 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 4135bc3a4..5e6d1b400 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 09f56829d..cad1a168b 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 4135bc3a4..5e6d1b400 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 09f56829d..cad1a168b 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 df268f865..6634970ec 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 0f934bf8e..bf74904e2 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 e11e4bd5f..6f2c6552a 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 f5982a60b..32f0a9603 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 e11e4bd5f..6f2c6552a 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 f5982a60b..32f0a9603 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 5058cecf9..0ae4e194d 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 ff93bcc41..8158b2220 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 4135bc3a4..5e6d1b400 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 09f56829d..cad1a168b 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 4135bc3a4..5e6d1b400 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "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 09f56829d..cad1a168b 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": "Run the program with the debugger disabled (standard DAP flag, default false): 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" + "description": "DAP's launch-without-debugging flag (default false). Where the adapter honours it (js-debug does; debugpy, Delve and CodeLLDB do too, though their launches currently fail under it) 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" } }, "additionalProperties": 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 74c8f033d..4b68ad915 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -1,48 +1,44 @@ /** - * buildNoDebugLaunchWarning (issue #710): `dapLaunchArgs.noDebug: true` reaches - * every adapter and disables the debugger, so nothing the caller asked to stop - * on can fire. The warning names what will not fire — and stays silent for a - * bare noDebug run, which is a legitimate "just run it". + * buildNoDebugLaunchWarning (issue #710): where an adapter honours DAP's + * `noDebug` launch flag, the debugger is off and nothing the caller asked to + * stop on can fire — the warning names what will not fire, and stays silent + * for a bare noDebug run, which is a legitimate "just run it". Where the + * adapter ignores the flag, the caller is told it had no effect instead. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { buildNoDebugLaunchWarning } from '../../../../src/session/breakpoints/launch-warnings.js'; import type { ManagedSession } from '../../../../src/session/session-store.js'; import { SessionManager, type SessionManagerConfig } from '../../../../src/session/session-manager.js'; -import { DebugLanguage, SessionState } from '@debugmcp/shared'; -import { createMockDependencies } from './session-manager-test-utils.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 type { MockProxyManager } from '../../../test-utils/mocks/mock-proxy-manager.js'; -type BuilderSession = { - breakpoints: Map; - functionBreakpoints: Map; -}; +type BuilderSession = Pick; -const noBreakpoints = (): BuilderSession => ({ breakpoints: new Map(), functionBreakpoints: new Map() }); - -const withLineBreakpoints = (count: number): BuilderSession => ({ - breakpoints: new Map( - Array.from({ length: count }, (_, i) => [`bp${i}`, { file: '/proj/app.py', line: 10 + i, verified: false }]) - ), - functionBreakpoints: new Map() -}); +function session(lineBreakpoints = 0, functionBreakpoints = 0): BuilderSession { + const breakpoints = new Map(); + for (let i = 0; i < lineBreakpoints; i++) { + breakpoints.set(`bp${i}`, { id: `bp${i}`, file: '/proj/app.py', line: 10 + i, verified: false }); + } + const fnBreakpoints = new Map(); + for (let i = 0; i < functionBreakpoints; i++) { + fnBreakpoints.set(`fn${i}`, { id: `fn${i}`, functionName: `handler${i}`, verified: false }); + } + return { breakpoints, functionBreakpoints: fnBreakpoints, language: DebugLanguage.PYTHON }; +} function build( - session: BuilderSession, - dapLaunchArgs: Record | undefined, - explicitBreakOnExceptions?: string + s: BuilderSession, + launchArgs: { noDebug?: boolean; stopOnEntry?: boolean } | undefined, + explicitBreakOnExceptions?: ExceptionBreakMode, + honoursNoDebug = true ): string | undefined { - // The builder is a free function over the two breakpoint stores plus the - // launch arguments and the caller's explicit exception mode. - return buildNoDebugLaunchWarning( - session as unknown as Pick, - dapLaunchArgs, - explicitBreakOnExceptions - ); + return buildNoDebugLaunchWarning(s, launchArgs, explicitBreakOnExceptions, honoursNoDebug); } describe('buildNoDebugLaunchWarning', () => { it('names the line breakpoints that will not fire', () => { - const warning = build(withLineBreakpoints(2), { noDebug: true }); + const warning = build(session(2), { noDebug: true }); expect(warning).toMatch(/noDebug is true/); expect(warning).toMatch(/2 breakpoint\(s\)/); expect(warning).toMatch(/will not fire/); @@ -50,55 +46,63 @@ describe('buildNoDebugLaunchWarning', () => { }); it('names function breakpoints separately from line breakpoints', () => { - const session = withLineBreakpoints(1); - session.functionBreakpoints.set('f', { functionName: 'main', verified: false }); - const warning = build(session, { noDebug: true }); + const warning = build(session(1, 1), { noDebug: true }); expect(warning).toMatch(/1 breakpoint\(s\)/); expect(warning).toMatch(/1 function breakpoint\(s\)/); }); it("names an explicit breakOnExceptions other than 'none'", () => { - const warning = build(noBreakpoints(), { noDebug: true }, 'uncaught'); + const warning = build(session(), { noDebug: true }, 'uncaught'); expect(warning).toMatch(/breakOnExceptions='uncaught'/); expect(warning).not.toMatch(/breakpoint\(s\)/); }); it("stays silent for an explicit breakOnExceptions of 'none' with nothing else set", () => { - expect(build(noBreakpoints(), { noDebug: true }, 'none')).toBeUndefined(); + expect(build(session(), { noDebug: true }, 'none')).toBeUndefined(); }); it('does not treat the policy default as something the caller asked for', () => { // The launcher passes the caller's value, undefined when unset; the // 'uncaught' policy default must not make every bare noDebug run warn. - expect(build(noBreakpoints(), { noDebug: true }, undefined)).toBeUndefined(); + expect(build(session(), { noDebug: true }, undefined)).toBeUndefined(); }); it('names stopOnEntry as a stop that will not come', () => { - const warning = build(noBreakpoints(), { noDebug: true, stopOnEntry: true }); - expect(warning).toMatch(/stopOnEntry/); + expect(build(session(), { noDebug: true, stopOnEntry: true })).toMatch(/stopOnEntry/); }); it('stays silent when noDebug is absent or false, whatever else is set', () => { - const session = withLineBreakpoints(3); - expect(build(session, undefined, 'all')).toBeUndefined(); - expect(build(session, { stopOnEntry: true }, 'all')).toBeUndefined(); - expect(build(session, { noDebug: false, stopOnEntry: true }, 'all')).toBeUndefined(); + expect(build(session(3), undefined, 'all')).toBeUndefined(); + expect(build(session(3), { stopOnEntry: true }, 'all')).toBeUndefined(); + expect(build(session(3), { noDebug: false, stopOnEntry: true }, 'all')).toBeUndefined(); }); it('lists every applicable clause in one sentence', () => { - const session = withLineBreakpoints(2); - session.functionBreakpoints.set('f', { functionName: 'main', verified: false }); - const warning = build(session, { noDebug: true, stopOnEntry: true }, 'all'); + const warning = build(session(2, 1), { noDebug: true, stopOnEntry: true }, 'all'); expect(warning).toMatch( /2 breakpoint\(s\), 1 function breakpoint\(s\), breakOnExceptions='all' and stopOnEntry will not fire/ ); }); + + it('says the flag had no effect where the adapter ignores it — whatever the caller set', () => { + // rdbg, netcoredbg, the JDI bridge and the mock adapter ignore noDebug; + // the rust transform never forwards it. Their debugger stays on. + const warning = build({ ...session(2), language: DebugLanguage.RUBY }, { noDebug: true }, 'all', false); + expect(warning).toMatch(/noDebug has no effect with the ruby adapter/); + expect(warning).toMatch(/debugger stays on/); + expect(warning).not.toMatch(/will not fire/); + // ...and even with nothing to stop on: the caller set a flag that does nothing. + expect(build(session(), { noDebug: true }, undefined, false)).toMatch(/has no effect/); + expect(build(session(), { noDebug: false }, undefined, false)).toBeUndefined(); + }); }); /** - * Wiring: the launcher computes the warning once per launch and puts it on - * data.warning for a real launch, a dry run and a restart; when it fires, the - * breakpoint-shaped launch warnings that presuppose a debugger are withheld. + * Wiring: the launcher decides once per launch — from the merged noDebug the + * adapter will see and the policy's word on whether it honours the flag — and + * puts the warning on data.warning for a real launch, a dry run and a restart. + * Where the debugger is off, the breakpoint-shaped launch warnings are + * withheld and readiness does not wait for an entry stop that cannot come. */ describe('SessionManager launches with noDebug (issue #710)', () => { let sessionManager: SessionManager; @@ -124,76 +128,155 @@ describe('SessionManager launches with noDebug (issue #710)', () => { function endDuringStartup(): void { const proxy = dependencies.mockProxyManager; proxy.start = vi.fn().mockImplementation(async (startConfig) => { - (proxy as unknown as { _isRunning: boolean })._isRunning = true; + setMockProxyRunning(proxy, true); proxy.startCalls.push(startConfig); process.nextTick(() => proxy.emit('exited', 0)); }) as MockProxyManager['start']; } - it('says which breakpoints will not fire instead of telling the caller to check their paths', async () => { - const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); - await sessionManager.setBreakpoint(session.id, { file: '/work/src/app.py', line: 7 }); - endDuringStartup(); + /** Replace the mock proxy's start with one that configures the adapter and never stops. */ + function runWithoutStopping(): void { + const proxy = dependencies.mockProxyManager; + proxy.start = vi.fn().mockImplementation(async (startConfig) => { + setMockProxyRunning(proxy, true); + proxy.startCalls.push(startConfig); + process.nextTick(() => { + proxy.emit('adapter-configured'); + proxy.emit('initialized'); + }); + }) as MockProxyManager['start']; + } - const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { stopOnEntry: false, noDebug: true }); + async function launch(sessionId: string, dapLaunchArgs: Record, adapterLaunchConfig?: Record) { + const startPromise = sessionManager.startDebugging(sessionId, '/work/src/app.py', [], dapLaunchArgs, false, adapterLaunchConfig); await vi.runAllTimersAsync(); - const result = await startPromise; + return startPromise; + } - expect(result.success).toBe(true); - expect(result.state).toBe(SessionState.STOPPED); - const warning = (result.data as { warning?: string }).warning; - expect(warning).toMatch(/noDebug is true/); - expect(warning).toMatch(/1 breakpoint\(s\)/); - // The #467 diagnosis ("check the file path and line") would be wrong here. - expect(warning).not.toMatch(/never bound during this run/); - }); + const warningOf = (result: { data?: unknown }) => (result.data as { warning?: string } | undefined)?.warning; - it('stays silent for a bare noDebug run with nothing to stop on', async () => { - const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); - endDuringStartup(); + describe('where the adapter honours the flag', () => { + beforeEach(() => { + // The mock adapter ignores noDebug; the policy override says otherwise + // so the launcher takes the debugger-off path. The launcher reads the + // data layer's lookup (ctx.selectPolicy), which is the facade method — + // not the store's, which overridePolicy() targets. + const facade = sessionManager as unknown as { selectPolicy: (language: string) => AdapterPolicy }; + const original = facade.selectPolicy.bind(facade); + vi.spyOn(facade, 'selectPolicy').mockImplementation((language) => ({ ...original(language), honoursNoDebug: true })); + }); - const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { stopOnEntry: false, noDebug: true }); - await vi.runAllTimersAsync(); - const result = await startPromise; + it('says which breakpoints will not fire instead of telling the caller to check their paths', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + endDuringStartup(); - expect(result.success).toBe(true); - expect((result.data as { warning?: string }).warning).toBeUndefined(); - }); + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); - it('warns on a dry run too — it is a configuration check', async () => { - const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); - await sessionManager.setBreakpoint(session.id, { file: '/work/src/app.py', line: 7 }); + 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\)/); + // The #467 diagnosis ("check the file path and line") would be wrong here. + expect(warningOf(result)).not.toMatch(/never bound during this run/); + }); - const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { noDebug: true }, true); - await vi.runAllTimersAsync(); - const result = await startPromise; + it('stays silent for a bare noDebug run with nothing to stop on', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + endDuringStartup(); - expect(result.success).toBe(true); - const data = result.data as { dryRun?: boolean; warning?: string }; - expect(data.dryRun).toBe(true); - expect(data.warning).toMatch(/noDebug is true/); + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(result.success).toBe(true); + expect(warningOf(result)).toBeUndefined(); + }); + + it('reads the flag from adapterLaunchConfig, which wins over dapLaunchArgs (the merge order the adapter sees)', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + endDuringStartup(); + + const viaAdapterConfig = await launch(s.id, { stopOnEntry: false }, { noDebug: true }); + expect(warningOf(viaAdapterConfig)).toMatch(/noDebug is true/); + + endDuringStartup(); + const overridden = await launch(s.id, { stopOnEntry: false, noDebug: true }, { noDebug: false }); + expect(warningOf(overridden)).not.toMatch(/noDebug/); + // ...and with the debugger on, the #467 diagnosis is the right one again. + expect(warningOf(overridden)).toMatch(/never bound during this run/); + }); + + it('does not wait for the entry stop it just said cannot come', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + + const before = Date.now(); + const result = await launch(s.id, { stopOnEntry: true, noDebug: true }); + + expect(result.success).toBe(true); + expect(result.state).toBe(SessionState.RUNNING); + expect(warningOf(result)).toMatch(/stopOnEntry will not fire/); + // Readiness resolved on adapter-configured, not on the 30 s ceiling. + expect(Date.now() - before).toBeLessThan(30000); + }); + + 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 }); + + const startPromise = sessionManager.startDebugging(s.id, '/work/src/app.py', [], { noDebug: true }, true); + await vi.runAllTimersAsync(); + const result = await startPromise; + + expect(result.success).toBe(true); + const data = result.data as { dryRun?: boolean; warning?: string }; + expect(data.dryRun).toBe(true); + expect(data.warning).toMatch(/noDebug is true/); + }); + + it('warns on restart for a breakpoint added after the noDebug launch', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + const first = await launch(s.id, { stopOnEntry: false, noDebug: true }); + expect(first.success).toBe(true); + expect(warningOf(first)).toBeUndefined(); + + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + dependencies.mockProxyManager.simulateEvent('terminated'); + await vi.runAllTimersAsync(); + + const restartPromise = sessionManager.restartDebugging(s.id); + await vi.runAllTimersAsync(); + const restarted = await restartPromise; + + expect(restarted.success).toBe(true); + expect(warningOf(restarted)).toMatch(/noDebug is true/); + expect(warningOf(restarted)).toMatch(/1 breakpoint\(s\)/); + }); }); - it('warns on restart for a breakpoint added after the noDebug launch', async () => { - const session = await sessionManager.createSession({ language: DebugLanguage.MOCK }); - const startPromise = sessionManager.startDebugging(session.id, '/work/src/app.py', [], { stopOnEntry: false, noDebug: true }); - await vi.runAllTimersAsync(); - const first = await startPromise; - expect(first.success).toBe(true); - expect((first.data as { warning?: string }).warning).toBeUndefined(); + describe('where the adapter ignores the flag (the mock policy, like rdbg, netcoredbg and the JDI bridge)', () => { + it('says the flag had no effect and keeps the breakpoint diagnostics that still apply', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + endDuringStartup(); - await sessionManager.setBreakpoint(session.id, { file: '/work/src/app.py', line: 7 }); - dependencies.mockProxyManager.simulateEvent('terminated'); - await vi.runAllTimersAsync(); + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); - const restartPromise = sessionManager.restartDebugging(session.id); - await vi.runAllTimersAsync(); - const restarted = await restartPromise; + expect(result.success).toBe(true); + expect(warningOf(result)).toMatch(/noDebug has no effect with the mock adapter/); + expect(warningOf(result)).not.toMatch(/will not fire/); + // The debugger was on, so a breakpoint that never bound is still the + // #467 story — that diagnosis must survive. + expect(warningOf(result)).toMatch(/never bound during this run/); + }); + + it('says nothing when the flag was not set', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + endDuringStartup(); + + const result = await launch(s.id, { stopOnEntry: false }); - expect(restarted.success).toBe(true); - const data = restarted.data as { warning?: string; breakpointsReapplied?: number }; - expect(data.breakpointsReapplied).toBe(1); - expect(data.warning).toMatch(/noDebug is true/); - expect(data.warning).toMatch(/1 breakpoint\(s\)/); + expect(warningOf(result)).toBeUndefined(); + }); }); }); diff --git a/tests/core/unit/session/session-manager-run-to-completion.test.ts b/tests/core/unit/session/session-manager-run-to-completion.test.ts index 766f19719..d52936eb1 100644 --- a/tests/core/unit/session/session-manager-run-to-completion.test.ts +++ b/tests/core/unit/session/session-manager-run-to-completion.test.ts @@ -7,12 +7,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { SessionManager, SessionManagerConfig } from '../../../../src/session/session-manager.js'; import { DebugLanguage, SessionState } from '@debugmcp/shared'; -import { createMockDependencies } from './session-manager-test-utils.js'; -import type { MockProxyManager } from '../../../test-utils/mocks/mock-proxy-manager.js'; - -function setMockProxyRunning(proxyManager: MockProxyManager, running: boolean): void { - (proxyManager as unknown as { _isRunning: boolean })._isRunning = running; -} +import { createMockDependencies, setMockProxyRunning } from './session-manager-test-utils.js'; describe('SessionManager.startDebugging - run to completion (issue #701)', () => { let sessionManager: SessionManager; diff --git a/tests/core/unit/session/session-manager-test-utils.ts b/tests/core/unit/session/session-manager-test-utils.ts index 4e1e23bd7..ec2c7bd11 100644 --- a/tests/core/unit/session/session-manager-test-utils.ts +++ b/tests/core/unit/session/session-manager-test-utils.ts @@ -90,6 +90,18 @@ export function createMockDependencies(): SessionManagerDependencies & { * it — so a test that wants a policy behavior overrides it here rather than * standing up a real adapter. */ +/** + * Flip the mock proxy's private running flag. + * + * Tests that replace `start()` wholesale must reproduce its side effect: + * `sendDapRequest` reads `_isRunning` directly, so stubbing the public + * `isRunning()` would not be equivalent. One reach-in here rather than one + * per suite. + */ +export function setMockProxyRunning(proxyManager: MockProxyManager, running: boolean): void { + (proxyManager as unknown as { _isRunning: boolean })._isRunning = running; +} + export function overridePolicy( sessionManager: SessionManager, overrides: Partial diff --git a/tests/core/unit/session/session-manager-workflow.test.ts b/tests/core/unit/session/session-manager-workflow.test.ts index 89d074ebe..87a9d8476 100644 --- a/tests/core/unit/session/session-manager-workflow.test.ts +++ b/tests/core/unit/session/session-manager-workflow.test.ts @@ -4,20 +4,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { SessionManager, SessionManagerConfig } from '../../../../src/session/session-manager.js'; import { DebugLanguage, SessionState } from '@debugmcp/shared'; -import { createMockDependencies } from './session-manager-test-utils.js'; -import type { MockProxyManager } from '../../../test-utils/mocks/mock-proxy-manager.js'; - -/** - * Flip the mock proxy's private running flag. - * - * The tests below replace `start()` wholesale and must reproduce its side - * effect: `sendDapRequest` reads `_isRunning` directly, so stubbing the public - * `isRunning()` would not be equivalent. Keeping the reach-in here means one - * cast for the file rather than one per call site. - */ -function setMockProxyRunning(proxyManager: MockProxyManager, running: boolean): void { - (proxyManager as unknown as { _isRunning: boolean })._isRunning = running; -} +import { createMockDependencies, setMockProxyRunning } from './session-manager-test-utils.js'; describe('SessionManager - Debug Session Workflow', () => { let sessionManager: SessionManager; diff --git a/tests/e2e/mcp-server-break-on-exceptions.test.ts b/tests/e2e/mcp-server-break-on-exceptions.test.ts index 11423cd07..8e55e0852 100644 --- a/tests/e2e/mcp-server-break-on-exceptions.test.ts +++ b/tests/e2e/mcp-server-break-on-exceptions.test.ts @@ -223,6 +223,33 @@ describe('Break-on-exception (issue #220)', () => { })).rejects.toThrow(/breakOnExceptions/); }, 30000); + it('says noDebug had no effect on an adapter that ignores it, and still pauses at the breakpoint (issue #710)', async () => { + sessionId = await createSession('mock', 'mock-nodebug-ignored'); + const bp = parseSdkToolResult(await mcpClient!.callTool({ + name: 'set_breakpoint', + arguments: { sessionId, file: CRASHING_SCRIPT, line: 5 } + })); + expect(bp.success).toBe(true); + + // The mock adapter never reads noDebug (like rdbg, netcoredbg and the + // JDI bridge): the debugger stays on, so the breakpoint fires — and + // the caller is told the flag did nothing rather than that it disabled + // the debugger. + const startRes = parseSdkToolResult(await mcpClient!.callTool({ + name: 'start_debugging', + arguments: { + sessionId, + scriptPath: CRASHING_SCRIPT, + dapLaunchArgs: { stopOnEntry: false, noDebug: true } + } + })); + expect(startRes.success, JSON.stringify(startRes)).toBe(true); + expect(startRes.state).toBe('paused'); + const warning = (startRes as { warning?: string }).warning; + expect(warning).toMatch(/noDebug has no effect with the mock adapter/); + expect(warning).not.toMatch(/will not fire/); + }, 30000); + it("honors breakOnExceptions 'none' nested inside dapLaunchArgs with a warning (#305)", async () => { sessionId = await createSession('mock', 'mock-nested-break-on-exceptions'); diff --git a/tests/unit/shared/adapter-policy-contract.test.ts b/tests/unit/shared/adapter-policy-contract.test.ts index 764ab21c9..06efee822 100644 --- a/tests/unit/shared/adapter-policy-contract.test.ts +++ b/tests/unit/shared/adapter-policy-contract.test.ts @@ -44,6 +44,8 @@ interface PinnedCapabilities { functionBreakpointsVia: 'dap' | 'cdp' | undefined; /** Whether verified:false at launch is by design rather than a warning. */ functionBreakpointsBindLate: boolean | undefined; + /** Whether a `noDebug` launch actually turns the debugger off (issue #710). */ + honoursNoDebug: boolean | undefined; childSessionStrategy: ChildSessionStrategy; requiresCommandQueueing: boolean; /** @@ -62,7 +64,8 @@ interface PinnedCapabilities { * from its language, and the only policy delivering function breakpoints over CDP; ruby is the * only adapter pinning function breakpoints OFF (rdbg advertises the capability but ignores the * request, #636) and the only one declining a default exception mode; ruby/java/dotnet are the - * three that reject logpoints; js and java are the two that bind function breakpoints late. + * three that reject logpoints; js and java are the two that bind function breakpoints late; + * js, python, go and cpp are the four whose debugger a `noDebug` launch turns off (#710). */ const PINNED: Record = { [DebugLanguage.PYTHON]: { @@ -71,6 +74,7 @@ const PINNED: Record = { supportsLogPoints: true, functionBreakpointsVia: undefined, functionBreakpointsBindLate: undefined, + honoursNoDebug: true, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: 'uncaught' @@ -81,6 +85,7 @@ const PINNED: Record = { supportsLogPoints: false, functionBreakpointsVia: undefined, functionBreakpointsBindLate: undefined, + honoursNoDebug: undefined, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: undefined @@ -91,6 +96,7 @@ const PINNED: Record = { supportsLogPoints: true, functionBreakpointsVia: 'cdp', functionBreakpointsBindLate: true, + honoursNoDebug: true, childSessionStrategy: 'launchWithPendingTarget', requiresCommandQueueing: true, defaultExceptionBreakMode: 'uncaught' @@ -101,6 +107,7 @@ const PINNED: Record = { supportsLogPoints: true, functionBreakpointsVia: undefined, functionBreakpointsBindLate: undefined, + honoursNoDebug: undefined, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: 'uncaught' @@ -111,6 +118,7 @@ const PINNED: Record = { supportsLogPoints: true, functionBreakpointsVia: undefined, functionBreakpointsBindLate: undefined, + honoursNoDebug: true, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: 'uncaught' @@ -121,6 +129,7 @@ const PINNED: Record = { supportsLogPoints: false, functionBreakpointsVia: undefined, functionBreakpointsBindLate: true, + honoursNoDebug: undefined, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: 'uncaught' @@ -131,6 +140,7 @@ const PINNED: Record = { supportsLogPoints: false, functionBreakpointsVia: undefined, functionBreakpointsBindLate: undefined, + honoursNoDebug: undefined, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: 'uncaught' @@ -141,6 +151,7 @@ const PINNED: Record = { supportsLogPoints: true, functionBreakpointsVia: undefined, functionBreakpointsBindLate: undefined, + honoursNoDebug: true, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: 'uncaught' @@ -151,6 +162,7 @@ const PINNED: Record = { supportsLogPoints: true, functionBreakpointsVia: undefined, functionBreakpointsBindLate: undefined, + honoursNoDebug: undefined, childSessionStrategy: 'none', requiresCommandQueueing: false, defaultExceptionBreakMode: 'uncaught' @@ -227,6 +239,10 @@ describe.each(LANGUAGES)('AdapterPolicy contract — %s', (language) => { expect(policy.supportsLogPoints).toBe(pinned.supportsLogPoints); }); + it('pins whether a noDebug launch turns the debugger off', () => { + expect(policy.honoursNoDebug).toBe(pinned.honoursNoDebug); + }); + it('only claims a function-breakpoint delivery quirk when it supports them at all', () => { expect(policy.functionBreakpointsVia).toBe(pinned.functionBreakpointsVia); expect(policy.functionBreakpointsBindLate).toBe(pinned.functionBreakpointsBindLate); From f6d015b1f5333f501c888832da38d86701cabdd9 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 19:22:50 -0400 Subject: [PATCH 4/6] fix(launch): with the debugger off, neutralize stopOnEntry once for everything that reads it (#710) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second review of #747, on the readiness handling added last round: - The readiness-only stopOnEntry view was inert for python/go/cpp, whose isSessionReady is `state === PAUSED` (they always request an entry stop and auto-continue), and racy for js-debug, whose adapter-configured fires before the readiness listener exists while the core still withheld RUNNING under the caller's stopOnEntry; the post-hoc RUNNING projection then stamped a state nothing had observed. Replaced at the root: when debuggerOff, the launch args handed to the proxy, the handshake and the readiness wait carry stopOnEntry:false, so the core's projection, the readiness predicate and the adapter agree; the readiness policy drops its pause-only predicate (running is ready). session.lastLaunch and the warning keep the caller's value. Both band-aids are gone. - noDebug and stopOnEntry are resolved the way the adapter sees them (adapterLaunchConfig over dapLaunchArgs [over defaults for noDebug]) and by truthiness — the proxy forwards a string 'true' verbatim and js-debug gates on truthiness. - A failed launch under the flag (debugpy/Delve/CodeLLDB today, #746) carries the noDebug note in its data instead of an init failure with no pointer to the flag behind it. - A stop that lands before the launch reports outranks the policy pin: the ordinary diagnostics stay and the note says the flag had no effect. - Logpoints are counted apart from breakpoints, as the run-to-completion summary does; the redundant second policy lookup reads the hoisted one; overridePolicy's doc comment is back on overridePolicy and says which seam it targets. - Tests: adapter-configured is emitted synchronously inside start() (the real ordering); a pause-only readiness policy; stopOnEntry via adapterLaunchConfig; string-typed noDebug; the failed-launch note; the stop-that-arrived-anyway guard; logpoint counting. The two readiness tests fail with the root fix disabled. Co-Authored-By: Claude Opus 5 (1M context) --- src/session/breakpoints/launch-warnings.ts | 13 +- src/session/launch/debug-launcher.ts | 108 +++++++++++------ .../session-manager-nodebug-warning.test.ts | 113 ++++++++++++++++-- .../session/session-manager-test-utils.ts | 19 +-- 4 files changed, 195 insertions(+), 58 deletions(-) diff --git a/src/session/breakpoints/launch-warnings.ts b/src/session/breakpoints/launch-warnings.ts index e30fa6f58..47872d225 100644 --- a/src/session/breakpoints/launch-warnings.ts +++ b/src/session/breakpoints/launch-warnings.ts @@ -86,11 +86,22 @@ export function buildNoDebugLaunchWarning( ); } const expected: string[] = []; - const lineCount = session.breakpoints.size; + let lineCount = 0; + let logpointCount = 0; + for (const bp of session.breakpoints.values()) { + if (bp.logMessage !== undefined) { + logpointCount++; + } else { + lineCount++; + } + } const functionCount = session.functionBreakpoints?.size ?? 0; if (lineCount > 0) { expected.push(`${lineCount} breakpoint(s)`); } + if (logpointCount > 0) { + expected.push(`${logpointCount} logpoint(s)`); + } if (functionCount > 0) { expected.push(`${functionCount} function breakpoint(s)`); } diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index 51dd76b4c..2616ad33f 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -40,21 +40,44 @@ import type { ProxyLauncher } from './proxy-launcher.js'; import type { InFlightGuard } from '../in-flight-guard.js'; /** - * The `noDebug` the adapter will see. The proxy launcher merges - * adapterLaunchConfig over dapLaunchArgs over the server defaults; a warning - * decided from dapLaunchArgs alone would miss a flag set (or unset) by either - * of the other two. + * A launch flag the way the adapter will see it. The proxy launcher merges + * adapterLaunchConfig over dapLaunchArgs over the server defaults, so a value + * read from dapLaunchArgs alone would miss a flag set (or unset) by either of + * the other two. Truthiness, not `=== true`: the proxy forwards the value + * verbatim and the adapters gate on truthiness, so a string 'true' (the + * string-typed-args transport quirk) must count the way it will be counted. + * `defaults` is omitted for flags that must reflect only what the caller + * asked for. */ -function resolveNoDebug( - defaults: Partial, +function resolveLaunchFlag( + key: 'noDebug' | 'stopOnEntry', dapLaunchArgs: Partial | undefined, - adapterLaunchConfig: Record | undefined + adapterLaunchConfig: Record | undefined, + defaults?: Partial ): boolean { - const fromAdapterConfig = adapterLaunchConfig?.noDebug; - if (typeof fromAdapterConfig === 'boolean') { - return fromAdapterConfig; + const fromAdapterConfig = adapterLaunchConfig?.[key]; + if (fromAdapterConfig !== undefined) { + return Boolean(fromAdapterConfig); } - return (dapLaunchArgs?.noDebug ?? defaults.noDebug) === true; + const fromDapArgs = dapLaunchArgs?.[key]; + if (fromDapArgs !== undefined) { + return Boolean(fromDapArgs); + } + return Boolean(defaults?.[key]); +} + +/** + * The `data` of a failed launch: the failure record, plus the noDebug note + * when there is one — an adapter that honours the flag but cannot complete + * the launch under it (debugpy, Delve, CodeLLDB today: issue #746) would + * otherwise report an init failure with no pointer to the flag behind it. + */ +function failureData( + diagnosticData: T, + noDebugWarning: string | undefined +): { data?: T & { warning?: string } } { + const data = { ...diagnosticData, ...(noDebugWarning ? { warning: noDebugWarning } : {}) }; + return Object.keys(data).length > 0 ? { data } : {}; } export class DebugLauncher { @@ -249,18 +272,25 @@ export class DebugLauncher { // run is a configuration check and a restart replays the same arguments, // and both should say so. const policy = this.ctx.selectPolicy(session.language); - const noDebug = resolveNoDebug(this.ctx.defaultDapLaunchArgs, dapLaunchArgs, adapterLaunchConfig); + const noDebug = resolveLaunchFlag('noDebug', dapLaunchArgs, adapterLaunchConfig, this.ctx.defaultDapLaunchArgs); const honoursNoDebug = policy.honoursNoDebug === true; const debuggerOff = noDebug && honoursNoDebug; const noDebugWarning = buildNoDebugLaunchWarning( session, - { noDebug, stopOnEntry: dapLaunchArgs?.stopOnEntry }, + { noDebug, stopOnEntry: resolveLaunchFlag('stopOnEntry', dapLaunchArgs, adapterLaunchConfig) }, breakOnExceptions, honoursNoDebug ); - // With the debugger off an entry stop cannot come, so readiness must not - // wait for one — the launch would sit on the readiness ceiling instead. - const readinessArgs = debuggerOff ? { ...dapLaunchArgs, stopOnEntry: false } : dapLaunchArgs; + // With the debugger off an entry stop cannot come. Everything that reads + // stopOnEntry from here on — the proxy config, the core's projection to + // RUNNING on adapter-configured, the readiness wait — sees it off, so they + // agree and none of them waits for a stop that cannot arrive. The warning + // above and session.lastLaunch (recorded earlier) keep the caller's value. + const launchArgs = debuggerOff ? { ...dapLaunchArgs, stopOnEntry: false } : dapLaunchArgs; + // Likewise the policy's readiness predicate: python/go/cpp count only a + // pause as ready (they always request an entry stop and auto-continue), + // which cannot happen here — the generic rule, running is ready, applies. + const readinessPolicy = debuggerOff ? { ...policy, isSessionReady: undefined } : policy; try { // For dry run, start the proxy and wait for completion @@ -383,8 +413,7 @@ export class DebugLauncher { launchArgsShape?.request === 'attach' || launchArgsShape?.__attachMode === true; let effectiveBreakOnExceptions = breakOnExceptions; if (effectiveBreakOnExceptions === undefined && !isAttachShaped) { - const policyDefault = this.ctx.selectPolicy(session.language) - .getInitializationBehavior?.().defaultExceptionBreakMode; + const policyDefault = policy.getInitializationBehavior?.().defaultExceptionBreakMode; if (policyDefault) { effectiveBreakOnExceptions = policyDefault; this.ctx.logger.info( @@ -398,7 +427,7 @@ export class DebugLauncher { const launchConfigData = await this.proxyLauncher.start(session, { scriptPath, scriptArgs, - dapLaunchArgs, + dapLaunchArgs: launchArgs, dryRunSpawn, adapterLaunchConfig, breakOnExceptions: effectiveBreakOnExceptions, @@ -411,7 +440,7 @@ export class DebugLauncher { await policy.performHandshake({ proxyManager: session.proxyManager, sessionId: session.id, - dapLaunchArgs, + dapLaunchArgs: launchArgs, scriptPath, scriptArgs, breakpoints: session.breakpoints, @@ -429,27 +458,21 @@ export class DebugLauncher { // Use policy-defined readiness criteria when available. const sessionStateAfterHandshake = this.ctx.getSession(sessionId).state; - const alreadyReady = policy.isSessionReady - ? policy.isSessionReady(sessionStateAfterHandshake, { stopOnEntry: readinessArgs?.stopOnEntry }) - : sessionStateAfterHandshake === SessionState.PAUSED; + const alreadyReady = readinessPolicy.isSessionReady + ? readinessPolicy.isSessionReady(sessionStateAfterHandshake, { stopOnEntry: launchArgs?.stopOnEntry }) + : debuggerOff + ? sessionStateAfterHandshake === SessionState.RUNNING + : sessionStateAfterHandshake === SessionState.PAUSED; if (!alreadyReady) { // Wait for adapter to be configured, first stop event, or termination - await waitForLaunchReadiness(this.ctx, { session, sessionId, policy, dapLaunchArgs: readinessArgs }); + await waitForLaunchReadiness(this.ctx, { session, sessionId, policy: readinessPolicy, dapLaunchArgs: launchArgs }); } else { this.ctx.logger.info( `[SessionManager] Session ${sessionId} already ${sessionStateAfterHandshake} after handshake - skipping adapter readiness wait` ); } - // With the debugger off, the entry stop that would normally carry the - // session out of INITIALIZING cannot come either (the core projects - // RUNNING on adapter-configured only when no entry stop is expected): - // the program is running, undebugged — say so. - if (debuggerOff && this.ctx.getSession(sessionId).state === SessionState.INITIALIZING) { - this.ctx.updateState(this.ctx.getSession(sessionId), SessionState.RUNNING); - } - // Re-fetch session to get the most up-to-date state const finalSession = this.ctx.getSession(sessionId); const finalState = finalSession.state; @@ -480,7 +503,7 @@ export class DebugLauncher { success: false, state: SessionState.ERROR, error: errorMessage, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugWarning) }; } @@ -494,11 +517,20 @@ export class DebugLauncher { await this.breakpoints.resyncAll(finalSession); } + // 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). + // 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; + const noDebugNote = stoppedAnyway + ? buildNoDebugLaunchWarning(finalSession, { noDebug }, breakOnExceptions, false) + : noDebugWarning; + // The three breakpoint-shaped warnings below each diagnose a symptom // ("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; + const debuggerOn = !debuggerOff || stoppedAnyway; // 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 @@ -529,7 +561,7 @@ export class DebugLauncher { // arriving after this return still lands in the output buffer as an // attributed [mcp-debugger] Warning entry. const launchWarning = - [noDebugWarning, fnBpWarning, logpointWarning, unboundAtExitWarning, ...(finalSession.adapterNotices ?? [])] + [noDebugNote, fnBpWarning, logpointWarning, unboundAtExitWarning, ...(finalSession.adapterNotices ?? [])] .filter(Boolean) .join('; ') || undefined; @@ -563,9 +595,9 @@ export class DebugLauncher { reason: finalState === SessionState.PAUSED ? finalSession.lastStop?.reason ?? - (dapLaunchArgs?.stopOnEntry ? 'entry' : 'unknown') + (launchArgs?.stopOnEntry ? 'entry' : 'unknown') : undefined, - stopOnEntrySuccessful: !!dapLaunchArgs?.stopOnEntry && finalState === SessionState.PAUSED, + stopOnEntrySuccessful: !!launchArgs?.stopOnEntry && finalState === SessionState.PAUSED, }, }; } catch (error) { @@ -637,7 +669,7 @@ export class DebugLauncher { state: session.state, errorType, errorCode, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugWarning) }; } } 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 4b68ad915..e91bef630 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -77,6 +77,13 @@ describe('buildNoDebugLaunchWarning', () => { expect(build(session(3), { noDebug: false, stopOnEntry: true }, 'all')).toBeUndefined(); }); + it('counts logpoints apart from breakpoints, as the run-to-completion summary does', () => { + const s = session(1); + s.breakpoints.set('lp', { id: 'lp', file: '/proj/app.py', line: 30, verified: false, logMessage: 'x={x}' }); + const warning = build(s, { noDebug: true }); + expect(warning).toMatch(/1 breakpoint\(s\) and 1 logpoint\(s\) will not fire/); + }); + it('lists every applicable clause in one sentence', () => { const warning = build(session(2, 1), { noDebug: true, stopOnEntry: true }, 'all'); expect(warning).toMatch( @@ -134,16 +141,19 @@ describe('SessionManager launches with noDebug (issue #710)', () => { }) as MockProxyManager['start']; } - /** Replace the mock proxy's start with one that configures the adapter and never stops. */ + /** + * Replace the mock proxy's start with one that configures the adapter and + * never stops — emitting synchronously inside start(), the way the real + * worker reports adapter-configured before start() resolves, so the + * launcher's readiness listener is not yet registered when it fires. + */ function runWithoutStopping(): void { const proxy = dependencies.mockProxyManager; proxy.start = vi.fn().mockImplementation(async (startConfig) => { setMockProxyRunning(proxy, true); proxy.startCalls.push(startConfig); - process.nextTick(() => { - proxy.emit('adapter-configured'); - proxy.emit('initialized'); - }); + proxy.emit('adapter-configured'); + proxy.emit('initialized'); }) as MockProxyManager['start']; } @@ -155,15 +165,24 @@ describe('SessionManager launches with noDebug (issue #710)', () => { const warningOf = (result: { data?: unknown }) => (result.data as { warning?: string } | undefined)?.warning; + /** + * Overlay policy fields on the launcher's lookup. The launcher reads the + * data layer's `ctx.selectPolicy`, which is the facade method — not the + * store's, which overridePolicy() targets. The real method is taken from + * the prototype so a second overlay does not wrap the first spy. + */ + function pinPolicy(overrides: Partial): void { + const facade = sessionManager as unknown as { selectPolicy: (language: string) => AdapterPolicy }; + const proto = Object.getPrototypeOf(sessionManager) as { selectPolicy: (language: string) => AdapterPolicy }; + const original = proto.selectPolicy.bind(sessionManager); + vi.spyOn(facade, 'selectPolicy').mockImplementation((language) => ({ ...original(language), ...overrides })); + } + describe('where the adapter honours the flag', () => { beforeEach(() => { // The mock adapter ignores noDebug; the policy override says otherwise - // so the launcher takes the debugger-off path. The launcher reads the - // data layer's lookup (ctx.selectPolicy), which is the facade method — - // not the store's, which overridePolicy() targets. - const facade = sessionManager as unknown as { selectPolicy: (language: string) => AdapterPolicy }; - const original = facade.selectPolicy.bind(facade); - vi.spyOn(facade, 'selectPolicy').mockImplementation((language) => ({ ...original(language), honoursNoDebug: true })); + // so the launcher takes the debugger-off path. + pinPolicy({ honoursNoDebug: true }); }); it('says which breakpoints will not fire instead of telling the caller to check their paths', async () => { @@ -218,6 +237,78 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(warningOf(result)).toMatch(/stopOnEntry will not fire/); // Readiness resolved on adapter-configured, not on the 30 s ceiling. expect(Date.now() - before).toBeLessThan(30000); + // The adapter was asked for no entry stop either: one value everywhere. + const sent = dependencies.mockProxyManager.startCalls.at(-1) as { stopOnEntry?: boolean } | undefined; + expect(sent?.stopOnEntry).toBe(false); + // ...while the replayable launch spec keeps what the caller asked for. + expect(sessionManager.getSession(s.id)?.lastLaunch?.dapLaunchArgs?.stopOnEntry).toBe(true); + }); + + it('does not wait on a policy whose readiness is a pause (python/go/cpp always request an entry stop)', async () => { + pinPolicy({ honoursNoDebug: true, isSessionReady: (state: SessionState) => state === SessionState.PAUSED }); + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + + const before = Date.now(); + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(result.success).toBe(true); + expect(result.state).toBe(SessionState.RUNNING); + expect(Date.now() - before).toBeLessThan(30000); + }); + + it('names a stopOnEntry that came in through adapterLaunchConfig', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + + const result = await launch(s.id, { noDebug: true }, { stopOnEntry: true }); + + expect(warningOf(result)).toMatch(/stopOnEntry will not fire/); + }); + + it('counts a string-typed noDebug the way the adapter will (truthy)', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + endDuringStartup(); + + const result = await launch(s.id, { stopOnEntry: false, noDebug: 'true' }); + + expect(warningOf(result)).toMatch(/noDebug is true/); + expect(warningOf(result)).not.toMatch(/never bound during this run/); + }); + + it('carries the note on a launch that failed under the flag', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + dependencies.mockProxyManager.shouldFailStart = true; + + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(result.success).toBe(false); + expect(warningOf(result)).toMatch(/noDebug is 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 + // when the stop lands before the launch reports (a later stop is #749's + // territory: the launch has already answered). + 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, 'breakpoint', { reason: 'breakpoint', threadId: 1 }); + }) as MockProxyManager['start']; + + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(result.state).toBe(SessionState.PAUSED); + expect(warningOf(result)).toMatch(/noDebug has no effect/); + expect(warningOf(result)).not.toMatch(/will not fire/); }); it('warns on a dry run too — it is a configuration check', async () => { diff --git a/tests/core/unit/session/session-manager-test-utils.ts b/tests/core/unit/session/session-manager-test-utils.ts index ec2c7bd11..b3247be8b 100644 --- a/tests/core/unit/session/session-manager-test-utils.ts +++ b/tests/core/unit/session/session-manager-test-utils.ts @@ -82,14 +82,6 @@ export function createMockDependencies(): SessionManagerDependencies & { }; } -/** - * Overlay hooks on the session store's adapter policy. - * - * The store's lookup is the seam the session layer reads policy from — - * function-breakpoint name resolution and the launch warnings both go through - * it — so a test that wants a policy behavior overrides it here rather than - * standing up a real adapter. - */ /** * Flip the mock proxy's private running flag. * @@ -102,6 +94,17 @@ export function setMockProxyRunning(proxyManager: MockProxyManager, running: boo (proxyManager as unknown as { _isRunning: boolean })._isRunning = running; } +/** + * Overlay hooks on the session store's adapter policy. + * + * The store's lookup is the seam the breakpoint layer reads policy from — + * function-breakpoint name resolution and the launch-time function-breakpoint + * warning go through it — so a test that wants that policy behavior overrides + * it here rather than standing up a real adapter. The launcher reads the data + * layer's lookup instead (the facade's `selectPolicy`; see + * `OperationsContext.selectPolicy` vs `selectStorePolicy`), which a test + * overrides by spying on the SessionManager instance. + */ export function overridePolicy( sessionManager: SessionManager, overrides: Partial From c7e7224d365002e3bc75744a244afb8a47df7fe5 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 19:48:36 -0400 Subject: [PATCH 5/6] fix(launch): close the noDebug edges the third review found (#710) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A pause that came anyway counts as ready under debuggerOff (it fell to RUNNING-only and sat on the 30 s ceiling); an entry stop the core already resumed counts as evidence too (firstStopHandled is set on every stop of the launch), so a wrong policy pin plus stopOnEntry no longer reports "no stop can come" after one fired. - stopOnEntry is neutralized in adapterLaunchConfig as well as dapLaunchArgs — adapterLaunchConfig wins the adapter merge, so the worker and the js child adoption would still have waited on it. - The #295 worker force (stopOnEntry=true for pending CDP function breakpoints) yields to an honoured noDebug: the breakpoints cannot bind and the entry stop cannot come. - Attach-shaped start_debugging is left alone: noDebug is a launch request property. - A bare noDebug launch that fails under the flag carries a note too (the warning had nothing armed to say), and every failure return uses failureData — the dry-run timeout and the teardown branch dropped it. - resolveLaunchFlag reads 'true'/'false' the way the proxy parser coerces them and drops the defaults source (the server defaults are typed stopOnEntry/justMyCode only); LaunchContext no longer needs defaultDapLaunchArgs. The texts name the flag alone — either source may have carried it. Readiness is one explicit predicate. Co-Authored-By: Claude Opus 5 (1M context) --- src/proxy/dap-proxy-worker.ts | 7 +- src/session/breakpoints/launch-warnings.ts | 22 +++-- src/session/launch/debug-launcher.ts | 90 ++++++++++--------- src/session/operations-context.ts | 1 - .../session-manager-nodebug-warning.test.ts | 61 ++++++++++++- 5 files changed, 133 insertions(+), 48 deletions(-) diff --git a/src/proxy/dap-proxy-worker.ts b/src/proxy/dap-proxy-worker.ts index 8ba212baa..a70f3b9e9 100644 --- a/src/proxy/dap-proxy-worker.ts +++ b/src/proxy/dap-proxy-worker.ts @@ -1185,7 +1185,12 @@ export class DapProxyWorker { (this.currentInitPayload?.initialFunctionBreakpoints?.length ?? 0) > 0 ) { const launchArgs = (payload.dapArgs ?? {}) as Record; - if (launchArgs.stopOnEntry !== true) { + // Under an honoured noDebug the debugger is off (issue #710): no entry + // stop can come, and the function breakpoints will not bind either — + // the session layer has already told the caller so. + const noDebug = launchArgs.noDebug === true || launchArgs.noDebug === 'true'; + const debuggerOff = noDebug && this.adapterPolicy.honoursNoDebug === true; + if (launchArgs.stopOnEntry !== true && !debuggerOff) { payload.dapArgs = { ...launchArgs, stopOnEntry: true }; this.logger?.info('[Worker] Forcing stopOnEntry=true in the js-debug launch config (pending CDP function breakpoints, issue #295)'); } diff --git a/src/session/breakpoints/launch-warnings.ts b/src/session/breakpoints/launch-warnings.ts index 47872d225..c03855e18 100644 --- a/src/session/breakpoints/launch-warnings.ts +++ b/src/session/breakpoints/launch-warnings.ts @@ -66,9 +66,9 @@ export function buildUnboundBreakpointExitWarning( * Where the adapter ignores the flag, the debugger stays on and the caller is * told so instead — the flag they set changes nothing, which is worth a line. * - * `noDebug` is the flag the adapter will actually see (adapterLaunchConfig - * over dapLaunchArgs over the server defaults, the launcher's merge order); - * `stopOnEntry` is the caller's own value. + * `noDebug` and `stopOnEntry` are the flags the adapter will actually see + * (adapterLaunchConfig over dapLaunchArgs, the launcher's merge order); the + * text names the flag alone since either source may have carried it. */ export function buildNoDebugLaunchWarning( session: Pick, @@ -81,7 +81,7 @@ export function buildNoDebugLaunchWarning( } if (!honoursNoDebug) { return ( - `dapLaunchArgs.noDebug has no effect with the ${session.language} adapter: the debugger stays on, ` + + `noDebug has no effect with the ${session.language} adapter: the debugger stays on, ` + `and breakpoints, breakOnExceptions and stopOnEntry work as usual` ); } @@ -119,11 +119,23 @@ export function buildNoDebugLaunchWarning( ? expected[0] : `${expected.slice(0, -1).join(', ')} and ${expected[expected.length - 1]}`; return ( - `dapLaunchArgs.noDebug is true, so the debugger is disabled for this launch and no stop can arrive: ` + + `noDebug is true, so the debugger is disabled for this launch and no stop can arrive: ` + `${list} will not fire. Drop noDebug to debug, or ignore this if you only meant to run the program` ); } +/** + * The note a launch that failed under an honoured `noDebug` carries when the + * warning above had nothing to say (nothing was armed): the flag is the fact + * the caller needs before retrying paths and ports (issue #746). + */ +export function buildNoDebugFailureNote(): string { + return ( + 'noDebug is true, so this launch ran with the debugger disabled; the adapter accepted the flag ' + + 'but the launch did not complete under it. Drop noDebug to debug, or run the program without a debug session' + ); +} + /** What a launch reports when it ended STOPPED: a sentence for the message, and the fields behind it. */ export interface RunToCompletionSummary { /** Appended to the launch message after "Current state: stopped". */ diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index 2616ad33f..234352f23 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -26,6 +26,7 @@ import type { BreakpointController } from '../breakpoints/breakpoint-controller. import { reresolveAnchors } from '../breakpoints/anchor-resolution.js'; import { buildLogpointDowngradeLaunchWarning, + buildNoDebugFailureNote, buildNoDebugLaunchWarning, buildUnboundBreakpointExitWarning, buildRunToCompletionSummary @@ -41,29 +42,23 @@ import type { InFlightGuard } from '../in-flight-guard.js'; /** * A launch flag the way the adapter will see it. The proxy launcher merges - * adapterLaunchConfig over dapLaunchArgs over the server defaults, so a value - * read from dapLaunchArgs alone would miss a flag set (or unset) by either of - * the other two. Truthiness, not `=== true`: the proxy forwards the value - * verbatim and the adapters gate on truthiness, so a string 'true' (the - * string-typed-args transport quirk) must count the way it will be counted. - * `defaults` is omitted for flags that must reflect only what the caller - * asked for. + * adapterLaunchConfig over dapLaunchArgs (the server defaults carry neither + * of these keys), so a value read from dapLaunchArgs alone would miss a flag + * set — or unset — through adapterLaunchConfig. The string forms are read + * the way the proxy's message parser coerces them ('true'/'false', the + * string-typed-args transport quirk); anything else counts by truthiness, + * which is how the adapters read it. */ function resolveLaunchFlag( key: 'noDebug' | 'stopOnEntry', dapLaunchArgs: Partial | undefined, - adapterLaunchConfig: Record | undefined, - defaults?: Partial + adapterLaunchConfig: Record | undefined ): boolean { const fromAdapterConfig = adapterLaunchConfig?.[key]; - if (fromAdapterConfig !== undefined) { - return Boolean(fromAdapterConfig); - } - const fromDapArgs = dapLaunchArgs?.[key]; - if (fromDapArgs !== undefined) { - return Boolean(fromDapArgs); - } - return Boolean(defaults?.[key]); + const value = fromAdapterConfig !== undefined ? fromAdapterConfig : dapLaunchArgs?.[key]; + if (value === 'true') return true; + if (value === 'false') return false; + return Boolean(value); } /** @@ -272,7 +267,12 @@ export class DebugLauncher { // run is a configuration check and a restart replays the same arguments, // and both should say so. const policy = this.ctx.selectPolicy(session.language); - const noDebug = resolveLaunchFlag('noDebug', dapLaunchArgs, adapterLaunchConfig, this.ctx.defaultDapLaunchArgs); + // noDebug is a launch-request property; an attach-shaped start_debugging + // (request: 'attach' / __attachMode) attaches with the debugger on. + const launchArgsShape = dapLaunchArgs as Record | undefined; + const isAttachShaped = + launchArgsShape?.request === 'attach' || launchArgsShape?.__attachMode === true; + const noDebug = !isAttachShaped && resolveLaunchFlag('noDebug', dapLaunchArgs, adapterLaunchConfig); const honoursNoDebug = policy.honoursNoDebug === true; const debuggerOff = noDebug && honoursNoDebug; const noDebugWarning = buildNoDebugLaunchWarning( @@ -281,15 +281,29 @@ export class DebugLauncher { breakOnExceptions, honoursNoDebug ); + // A launch that fails under the flag still needs to point at it, whether + // or not there was anything armed to warn about. + const noDebugFailureNote = noDebugWarning ?? (debuggerOff ? buildNoDebugFailureNote() : undefined); // With the debugger off an entry stop cannot come. Everything that reads - // stopOnEntry from here on — the proxy config, the core's projection to - // RUNNING on adapter-configured, the readiness wait — sees it off, so they - // agree and none of them waits for a stop that cannot arrive. The warning - // above and session.lastLaunch (recorded earlier) keep the caller's value. + // stopOnEntry from here on — the proxy config (from either source the + // adapter merge reads), the core's projection to RUNNING on + // adapter-configured, the readiness wait — sees it off, so they agree and + // none of them waits for a stop that cannot arrive. The warning above and + // session.lastLaunch (recorded earlier) keep the caller's values. const launchArgs = debuggerOff ? { ...dapLaunchArgs, stopOnEntry: false } : dapLaunchArgs; - // Likewise the policy's readiness predicate: python/go/cpp count only a - // pause as ready (they always request an entry stop and auto-continue), - // which cannot happen here — the generic rule, running is ready, applies. + const launchAdapterConfig = + debuggerOff && adapterLaunchConfig?.stopOnEntry !== undefined + ? { ...adapterLaunchConfig, stopOnEntry: false } + : adapterLaunchConfig; + // Likewise readiness: python/go/cpp count only a pause as ready (they + // always request an entry stop and auto-continue), which cannot happen + // here — running is ready, and so is a pause that came anyway. + const isReady = (state: SessionState): boolean => + debuggerOff + ? state === SessionState.RUNNING || state === SessionState.PAUSED + : policy.isSessionReady + ? policy.isSessionReady(state, { stopOnEntry: launchArgs?.stopOnEntry }) + : state === SessionState.PAUSED; const readinessPolicy = debuggerOff ? { ...policy, isSessionReady: undefined } : policy; try { @@ -398,7 +412,7 @@ export class DebugLauncher { success: false, error: dryRunTimeoutError.message, state, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugFailureNote) }; } } @@ -408,9 +422,6 @@ export class DebugLauncher { // user did not specify one, launch sessions take the adapter policy's // default. Attach-shaped configs are excluded — pausing a process you // attached to on exceptions is surprising — as are dry runs (above). - const launchArgsShape = dapLaunchArgs as Record | undefined; - const isAttachShaped = - launchArgsShape?.request === 'attach' || launchArgsShape?.__attachMode === true; let effectiveBreakOnExceptions = breakOnExceptions; if (effectiveBreakOnExceptions === undefined && !isAttachShaped) { const policyDefault = policy.getInitializationBehavior?.().defaultExceptionBreakMode; @@ -429,7 +440,7 @@ export class DebugLauncher { scriptArgs, dapLaunchArgs: launchArgs, dryRunSpawn, - adapterLaunchConfig, + adapterLaunchConfig: launchAdapterConfig, breakOnExceptions: effectiveBreakOnExceptions, }); this.ctx.logger.info(`[SessionManager] ProxyManager started for session ${sessionId}`); @@ -458,11 +469,7 @@ export class DebugLauncher { // Use policy-defined readiness criteria when available. const sessionStateAfterHandshake = this.ctx.getSession(sessionId).state; - const alreadyReady = readinessPolicy.isSessionReady - ? readinessPolicy.isSessionReady(sessionStateAfterHandshake, { stopOnEntry: launchArgs?.stopOnEntry }) - : debuggerOff - ? sessionStateAfterHandshake === SessionState.RUNNING - : sessionStateAfterHandshake === SessionState.PAUSED; + const alreadyReady = isReady(sessionStateAfterHandshake); if (!alreadyReady) { // Wait for adapter to be configured, first stop event, or termination @@ -503,7 +510,7 @@ export class DebugLauncher { success: false, state: SessionState.ERROR, error: errorMessage, - ...failureData(diagnosticData, noDebugWarning) + ...failureData(diagnosticData, noDebugFailureNote) }; } @@ -518,10 +525,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). + // 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; + const stoppedAnyway = + debuggerOff && (finalState === SessionState.PAUSED || finalSession.firstStopHandled === true); const noDebugNote = stoppedAnyway ? buildNoDebugLaunchWarning(finalSession, { noDebug }, breakOnExceptions, false) : noDebugWarning; @@ -627,7 +637,7 @@ export class DebugLauncher { state: SessionState.STOPPED, errorType, errorCode, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugFailureNote) }; } @@ -669,7 +679,7 @@ export class DebugLauncher { state: session.state, errorType, errorCode, - ...failureData(diagnosticData, noDebugWarning) + ...failureData(diagnosticData, noDebugFailureNote) }; } } diff --git a/src/session/operations-context.ts b/src/session/operations-context.ts index 6a4e354d5..161014637 100644 --- a/src/session/operations-context.ts +++ b/src/session/operations-context.ts @@ -141,7 +141,6 @@ export type LaunchContext = Pick< | 'adapterRegistry' | 'launchValidationCache' | 'dryRunTimeoutMs' - | 'defaultDapLaunchArgs' | 'getSession' | 'updateSession' | 'updateState' 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 e91bef630..77e2e71bd 100644 --- a/tests/core/unit/session/session-manager-nodebug-warning.test.ts +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -257,13 +257,69 @@ describe('SessionManager launches with noDebug (issue #710)', () => { expect(Date.now() - before).toBeLessThan(30000); }); - it('names a stopOnEntry that came in through adapterLaunchConfig', async () => { + it('names a stopOnEntry that came in through adapterLaunchConfig, and neutralizes it there too', async () => { const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); runWithoutStopping(); const result = await launch(s.id, { noDebug: true }, { stopOnEntry: true }); expect(warningOf(result)).toMatch(/stopOnEntry will not fire/); + // adapterLaunchConfig wins the adapter merge, so it must carry false as well. + const sent = dependencies.mockProxyManager.startCalls.at(-1) as { stopOnEntry?: boolean; launchConfig?: { stopOnEntry?: boolean } } | undefined; + expect(sent?.stopOnEntry).toBe(false); + expect(sent?.launchConfig?.stopOnEntry).toBe(false); + }); + + it('reads the string forms the way the proxy parser coerces them', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + runWithoutStopping(); + + const result = await launch(s.id, { noDebug: 'true', stopOnEntry: 'false' }); + + // 'false' is false: no entry stop was asked for, so none is named. + expect(warningOf(result)).toBeUndefined(); + }); + + it('leaves an attach-shaped start_debugging alone — noDebug is a launch-request property', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + await sessionManager.setBreakpoint(s.id, { file: '/work/src/app.py', line: 7 }); + endDuringStartup(); + + const result = await launch(s.id, { request: 'attach', port: 9229, noDebug: true }); + + expect(warningOf(result)).not.toMatch(/noDebug/); + expect(warningOf(result)).toMatch(/never bound during this run/); + }); + + it('carries the note on a bare noDebug launch that failed, with nothing armed to warn about', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + dependencies.mockProxyManager.shouldFailStart = true; + + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + expect(result.success).toBe(false); + expect(warningOf(result)).toMatch(/noDebug is true, so this launch ran with the debugger disabled/); + }); + + it('believes an entry stop the core already resumed over the policy pin', async () => { + // A wrong pin plus stopOnEntry: the neutralized value makes the core + // auto-continue the entry stop, so no pause is left standing — but + // the stop happened, and the response must not say no stop can come. + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + 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, 'entry', { reason: 'entry', threadId: 1 }); + }) as MockProxyManager['start']; + + const result = await launch(s.id, { stopOnEntry: true, noDebug: true }); + + expect(result.success).toBe(true); + expect(warningOf(result)).toMatch(/noDebug has no effect/); + expect(warningOf(result)).not.toMatch(/will not fire/); }); it('counts a string-typed noDebug the way the adapter will (truthy)', async () => { @@ -304,11 +360,14 @@ describe('SessionManager launches with noDebug (issue #710)', () => { proxy.emit('stopped', 1, 'breakpoint', { reason: 'breakpoint', threadId: 1 }); }) as MockProxyManager['start']; + const before = Date.now(); const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); expect(result.state).toBe(SessionState.PAUSED); expect(warningOf(result)).toMatch(/noDebug has no effect/); expect(warningOf(result)).not.toMatch(/will not fire/); + // A pause that came anyway is ready too — not a 30 s wait for RUNNING. + expect(Date.now() - before).toBeLessThan(30000); }); it('warns on a dry run too — it is a configuration check', async () => { From a27e42ec7c7e7e17fb15870c1b5daaac70b63e34 Mon Sep 17 00:00:00 2001 From: JF Date: Wed, 16 Sep 2026 19:56:24 -0400 Subject: [PATCH 6/6] fix(launch): the noDebug failure note states the fact, not a cause it cannot know (#710) "the adapter accepted the flag but the launch did not complete under it" asserted causation on any failure under an honoured noDebug, including a bad scriptPath. The note now says the debugger was off and offers the remedy: drop the flag if the failure is unexpected, since some adapters cannot complete a launch under it (#746). Co-Authored-By: Claude Opus 5 (1M context) --- src/session/breakpoints/launch-warnings.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/session/breakpoints/launch-warnings.ts b/src/session/breakpoints/launch-warnings.ts index c03855e18..a76ace109 100644 --- a/src/session/breakpoints/launch-warnings.ts +++ b/src/session/breakpoints/launch-warnings.ts @@ -126,13 +126,16 @@ export function buildNoDebugLaunchWarning( /** * The note a launch that failed under an honoured `noDebug` carries when the - * warning above had nothing to say (nothing was armed): the flag is the fact - * the caller needs before retrying paths and ports (issue #746). + * warning above had nothing to say (nothing was armed). It states the fact + * and the remedy without blaming the flag: the failure may be unrelated (a + * bad path), or it may be an adapter that cannot complete a launch under + * the flag (issue #746) — either way, the flag is what the caller needs to + * know before retrying paths and ports. */ export function buildNoDebugFailureNote(): string { return ( - 'noDebug is true, so this launch ran with the debugger disabled; the adapter accepted the flag ' + - 'but the launch did not complete under it. Drop noDebug to debug, or run the program without a debug session' + 'noDebug is true, so this launch ran with the debugger disabled. If the failure is unexpected, drop noDebug ' + + 'and launch again — some adapters cannot complete a launch under the flag' ); }