diff --git a/changelog.d/710.fixed.md b/changelog.d/710.fixed.md new file mode 100644 index 000000000..5d95b0aa2 --- /dev/null +++ b/changelog.d/710.fixed.md @@ -0,0 +1 @@ +**A `noDebug` launch says what it did to the debugger instead of blaming your breakpoints** — `dapLaunchArgs.noDebug: true` is DAP's "launch without enabling debugging", and where the adapter honours it no breakpoint binds and no stop ever arrives; the session then read as a mystery — a short script ended `stopped` with the #467 "check the file path and line" warning for breakpoints that were never going to bind, a server stayed `running` while the caller waited for a stop that could not come. Whether the flag turns the debugger off is now the adapter policy's word (`honoursNoDebug`), measured rather than assumed: js-debug honours it, and so do debugpy, Delve and CodeLLDB (their launches currently fail under it — #746); rdbg, netcoredbg and the Java bridge ignore it, and the rust launch transform never forwards it. Where the debugger is off, `start_debugging` warns when the flag is set alongside something the caller asked to stop on — line or function breakpoints, an *explicit* `breakOnExceptions` other than `none` (the policy default does not count, so a deliberate plain run stays silent), or `stopOnEntry` — naming each thing that will not fire, withholds the breakpoint-shaped launch warnings (#308, #467, #469) that presuppose a debugger, and no longer waits for an entry stop that cannot come. Where the adapter ignores the flag, the warning says it had no effect and the usual diagnostics stay. The flag is read the way the adapter will see it (`adapterLaunchConfig` over `dapLaunchArgs` over the server defaults), and the decision lives in the launcher, so `restart_debugging` and a `dryRunSpawn` configuration check carry the same warning; the `dapLaunchArgs` schema documents the flag. Found by the review of PR #706 (#710) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 35ad151f2..d3d3f18ca 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): 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/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/server/tool-schemas.ts b/src/server/tool-schemas.ts index 561fce649..c1a0b846a 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: '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 efdab35c0..a76ace109 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'; @@ -50,6 +50,95 @@ export function buildUnboundBreakpointExitWarning( ); } +/** + * 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` 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, + launchArgs: { noDebug?: boolean; stopOnEntry?: boolean } | undefined, + explicitBreakOnExceptions: ExceptionBreakMode | undefined, + honoursNoDebug: boolean +): string | undefined { + if (launchArgs?.noDebug !== true) { + return undefined; + } + if (!honoursNoDebug) { + return ( + `noDebug has no effect with the ${session.language} adapter: the debugger stays on, ` + + `and breakpoints, breakOnExceptions and stopOnEntry work as usual` + ); + } + const expected: string[] = []; + 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)`); + } + if (explicitBreakOnExceptions !== undefined && explicitBreakOnExceptions !== 'none') { + expected.push(`breakOnExceptions='${explicitBreakOnExceptions}'`); + } + if (launchArgs.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 ( + `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). 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. If the failure is unexpected, drop noDebug ' + + 'and launch again — some adapters cannot complete a launch under the flag' + ); +} + /** 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..234352f23 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -26,6 +26,8 @@ import type { BreakpointController } from '../breakpoints/breakpoint-controller. import { reresolveAnchors } from '../breakpoints/anchor-resolution.js'; import { buildLogpointDowngradeLaunchWarning, + buildNoDebugFailureNote, + buildNoDebugLaunchWarning, buildUnboundBreakpointExitWarning, buildRunToCompletionSummary } from '../breakpoints/launch-warnings.js'; @@ -38,6 +40,41 @@ import { waitForLaunchReadiness } from './launch-readiness.js'; import type { ProxyLauncher } from './proxy-launcher.js'; 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 (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 +): boolean { + const fromAdapterConfig = adapterLaunchConfig?.[key]; + const value = fromAdapterConfig !== undefined ? fromAdapterConfig : dapLaunchArgs?.[key]; + if (value === 'true') return true; + if (value === 'false') return false; + return Boolean(value); +} + +/** + * 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 { constructor( private readonly ctx: LaunchContext, @@ -221,9 +258,69 @@ export class DebugLauncher { }; } + // 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); + // 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( + session, + { noDebug, stopOnEntry: resolveLaunchFlag('stopOnEntry', dapLaunchArgs, adapterLaunchConfig) }, + 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 (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; + 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 { // 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; @@ -256,16 +353,7 @@ export class DebugLauncher { ); delete sessionWithSetup._dryRunHandlerSetup; - return { - success: true, - state: SessionState.STOPPED, - data: { - dryRun: true, - message: 'Dry run spawn command logged by proxy.', - command: initialDryRunSnapshot?.command, - script: initialDryRunSnapshot?.script, - }, - }; + return dryRunResult(initialDryRunSnapshot); } // Wait for completion with timeout @@ -292,16 +380,7 @@ export class DebugLauncher { `[SessionManager] Dry run completed for session ${sessionId}, final state: ${latestSessionState.state}` ); - return { - success: true, - state: SessionState.STOPPED, - data: { - 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 @@ -333,7 +412,7 @@ export class DebugLauncher { success: false, error: dryRunTimeoutError.message, state, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugFailureNote) }; } } @@ -343,13 +422,9 @@ 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 = this.ctx.selectPolicy(session.language) - .getInitializationBehavior?.().defaultExceptionBreakMode; + const policyDefault = policy.getInitializationBehavior?.().defaultExceptionBreakMode; if (policyDefault) { effectiveBreakOnExceptions = policyDefault; this.ctx.logger.info( @@ -363,21 +438,20 @@ export class DebugLauncher { const launchConfigData = await this.proxyLauncher.start(session, { scriptPath, scriptArgs, - dapLaunchArgs, + dapLaunchArgs: launchArgs, dryRunSpawn, - adapterLaunchConfig, + adapterLaunchConfig: launchAdapterConfig, breakOnExceptions: effectiveBreakOnExceptions, }); 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({ proxyManager: session.proxyManager, sessionId: session.id, - dapLaunchArgs, + dapLaunchArgs: launchArgs, scriptPath, scriptArgs, breakpoints: session.breakpoints, @@ -395,13 +469,11 @@ 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 }) - : sessionStateAfterHandshake === SessionState.PAUSED; + const alreadyReady = isReady(sessionStateAfterHandshake); 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: readinessPolicy, dapLaunchArgs: launchArgs }); } else { this.ctx.logger.info( `[SessionManager] Session ${sessionId} already ${sessionStateAfterHandshake} after handshake - skipping adapter readiness wait` @@ -438,7 +510,7 @@ export class DebugLauncher { success: false, state: SessionState.ERROR, error: errorMessage, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugFailureNote) }; } @@ -452,32 +524,54 @@ 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) — + // a pause still standing, or an entry stop the core already resumed + // (firstStopHandled is set on every stop of this launch, resumed or not). + // Then the debugger was on: keep the ordinary diagnostics and say the + // flag had no effect rather than that no stop can come. + const stoppedAnyway = + debuggerOff && (finalState === SessionState.PAUSED || finalSession.firstStopHandled === true); + 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 || 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 // 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 ?? [])] + [noDebugNote, fnBpWarning, logpointWarning, unboundAtExitWarning, ...(finalSession.adapterNotices ?? [])] .filter(Boolean) .join('; ') || undefined; @@ -511,9 +605,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) { @@ -543,7 +637,7 @@ export class DebugLauncher { state: SessionState.STOPPED, errorType, errorCode, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugFailureNote) }; } @@ -585,7 +679,7 @@ export class DebugLauncher { state: session.state, errorType, errorCode, - ...(Object.keys(diagnosticData).length > 0 ? { data: diagnosticData } : {}) + ...failureData(diagnosticData, noDebugFailureNote) }; } } 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..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 @@ -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": "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 e1266a5e3..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 @@ -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": "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 f74ea12e1..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 @@ -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": "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 23cd205c4..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 @@ -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": "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 f74ea12e1..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 @@ -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": "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 23cd205c4..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 @@ -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": "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 f01f65a10..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 @@ -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": "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 b2e801fbf..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 @@ -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": "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 5bbd83a62..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 @@ -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": "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 fb526ba86..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 @@ -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": "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 5bbd83a62..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 @@ -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": "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 fb526ba86..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 @@ -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": "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 dbe8d892d..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 @@ -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": "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 60c38c184..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 @@ -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": "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 86f2313f6..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 @@ -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": "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 e9278fdb2..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 @@ -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": "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 86f2313f6..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 @@ -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": "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 e9278fdb2..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 @@ -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": "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 f01f65a10..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 @@ -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": "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 b2e801fbf..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 @@ -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": "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 5bbd83a62..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 @@ -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": "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 fb526ba86..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 @@ -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": "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 5bbd83a62..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 @@ -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": "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 fb526ba86..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 @@ -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": "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 new file mode 100644 index 000000000..77e2e71bd --- /dev/null +++ b/tests/core/unit/session/session-manager-nodebug-warning.test.ts @@ -0,0 +1,432 @@ +/** + * 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, 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 = Pick; + +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( + s: BuilderSession, + launchArgs: { noDebug?: boolean; stopOnEntry?: boolean } | undefined, + explicitBreakOnExceptions?: ExceptionBreakMode, + honoursNoDebug = true +): string | undefined { + return buildNoDebugLaunchWarning(s, launchArgs, explicitBreakOnExceptions, honoursNoDebug); +} + +describe('buildNoDebugLaunchWarning', () => { + it('names the line breakpoints that will not fire', () => { + 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/); + expect(warning).toMatch(/Drop noDebug/); + }); + + it('names function breakpoints separately from line breakpoints', () => { + 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(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(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(session(), { noDebug: true }, undefined)).toBeUndefined(); + }); + + it('names stopOnEntry as a stop that will not come', () => { + expect(build(session(), { noDebug: true, stopOnEntry: true })).toMatch(/stopOnEntry/); + }); + + it('stays silent when noDebug is absent or false, whatever else is set', () => { + 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('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( + /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 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; + 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) => { + setMockProxyRunning(proxy, true); + proxy.startCalls.push(startConfig); + process.nextTick(() => proxy.emit('exited', 0)); + }) as MockProxyManager['start']; + } + + /** + * 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); + proxy.emit('adapter-configured'); + proxy.emit('initialized'); + }) as MockProxyManager['start']; + } + + async function launch(sessionId: string, dapLaunchArgs: Record, adapterLaunchConfig?: Record) { + const startPromise = sessionManager.startDebugging(sessionId, '/work/src/app.py', [], dapLaunchArgs, false, adapterLaunchConfig); + await vi.runAllTimersAsync(); + return startPromise; + } + + 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. + pinPolicy({ honoursNoDebug: true }); + }); + + 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(); + + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + 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/); + }); + + it('stays silent for a bare noDebug run with nothing to stop on', async () => { + const s = await sessionManager.createSession({ language: DebugLanguage.MOCK }); + endDuringStartup(); + + 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); + // 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, 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 () => { + 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 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 () => { + 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\)/); + }); + }); + + 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(); + + const result = await launch(s.id, { stopOnEntry: false, noDebug: true }); + + 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(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..b3247be8b 100644 --- a/tests/core/unit/session/session-manager-test-utils.ts +++ b/tests/core/unit/session/session-manager-test-utils.ts @@ -82,13 +82,28 @@ export function createMockDependencies(): SessionManagerDependencies & { }; } +/** + * 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; +} + /** * 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. + * 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, 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 cbcf270b7..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'); @@ -456,6 +483,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'); 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);