diff --git a/CHANGELOG.md b/CHANGELOG.md index 77118917..fc2b38fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed +- **Attach verification window default raised 5s → 20s** — the `attach_to_process` thread-poll deadline only ever bites when the debug adapter is alive but the target is slow to become debuggable (js-debug child-session adoption on a loaded host, a warming JVM); adapter death still fails fast via the proxy-gone latch, and the poll returns the moment threads appear, so healthy attaches never pay for the headroom. A false "attach failed" on a healthy target is strictly worse for an agent than a slower genuine failure. Pass a small `verifyTimeout` for fast failure-by-design probes + +### Fixed +- **A paused session can no longer answer `get_stack_trace` with a silent empty success** — some adapters report a stop before the stack is materialized (netcoredbg right after the post-attach pause — the milder sibling of #353's `0x80131302`), and the tracked thread can be a frameless runtime thread. The agent-facing stack path now retries an empty-but-successful stackTrace within a bounded window (~3s, exits on first frame), falls back to scanning the other stopped threads and adopts the first one with frames (annotated via a `note` in the response, so scopes/evaluate anchor correctly), and — if everything stays frameless — returns the empty result with a `note` saying what to try instead. Not-paused and no-known-thread empty results now carry an explanatory `note` too + ### Added - **CodeLLDB ships as per-platform npm packages (esbuild pattern)** — five new packages `@debugmcp/codelldb-{win32-x64,darwin-x64,darwin-arm64,linux-x64,linux-arm64}` (versioned by the CodeLLDB release, currently 1.11.8, payload staged from the digest-pinned VSIXs) are `optionalDependencies` of `@debugmcp/mcp-debugger`, so npm installs exactly the one matching your os/cpu. Rust and C/C++ debugging now work out of the box on every platform npm serves — previously the CLI tarball bundled linux-x64 only — and the core tarball shrinks from ~54 MB to a few MB. The resolver probes the installed platform package last — after the vendor tree and after `CODELLDB_PATH` — so an explicit `CODELLDB_PATH` still overrides the auto-installed package, and installs with `--omit=optional` keep working via `CODELLDB_PATH` (#383) diff --git a/docs/patterns/error-handling.md b/docs/patterns/error-handling.md index 10ac8e39..abf43999 100644 --- a/docs/patterns/error-handling.md +++ b/docs/patterns/error-handling.md @@ -236,11 +236,15 @@ target (e.g. a busy or warming JVM) can legitimately need longer than the default window, the window is caller-configurable (issue #143): - The default lives in the protected, test-shrinkable field - `attachVerifyTimeoutMs` (5s), following the `stepGraceMs`/`pauseGraceMs` - field pattern. + `attachVerifyTimeoutMs` (20s), following the `stepGraceMs`/`pauseGraceMs` + field pattern. It is deliberately generous: adapter death fails fast + regardless (the proxy-gone latch), so the deadline only bites when the + adapter is alive but the target is slow to report threads — where a false + "attach failed" is worse than a slow genuine failure. - Callers override it per attach via the `verifyTimeout` (ms) argument on `attach_to_process` / `create_debug_session`; the value is validated - (positive finite number) and clamped to 10 minutes. + (positive finite number) and clamped to 10 minutes. Pass a small value for + fast failure-by-design probes. - The failure text comes from `ErrorMessages.attachVerifyFailed(timeoutMs, lastFailure)`, which names the `verifyTimeout` knob so a caller that hit the window on a slow target knows how to retry. diff --git a/skills/debugging/references/java.md b/skills/debugging/references/java.md index 6f24e97a..bb1b7acc 100644 --- a/skills/debugging/references/java.md +++ b/skills/debugging/references/java.md @@ -50,7 +50,7 @@ continue_execution {"sessionId": ""} // required with suspend=y to let ``` - **Deferred breakpoints work natively:** for classes not yet loaded, the JDI bridge registers a `ClassPrepareRequest` and binds the breakpoint when the JVM loads the class, then reports `verified: true`. Set breakpoints freely before or after attach — no re-sends needed. -- For a busy or warming JVM, raise `verifyTimeout` (ms) on `attach_to_process` — attach fails if no thread is reported within ~5 s by default. Attach sessions skip host-side file checks, so remote paths are fine. +- For a busy or warming JVM, raise `verifyTimeout` (ms) on `attach_to_process` — attach fails if no thread is reported within ~20 s by default. Attach sessions skip host-side file checks, so remote paths are fine. ## Quirks diff --git a/skills/debugging/references/javascript.md b/skills/debugging/references/javascript.md index 8201d3e7..da229313 100644 --- a/skills/debugging/references/javascript.md +++ b/skills/debugging/references/javascript.md @@ -53,7 +53,7 @@ set_breakpoint { "sessionId": "", "file": "/abs/path/server.js", "line continue_execution { "sessionId": "" } ``` -Shorthand: `create_debug_session { "language": "javascript", "host": "127.0.0.1", "port": 9229 }` attaches in one call. Attach is verified by polling for threads (`verifyTimeout`, default ~5000 ms). Detach with `detach_from_process { "sessionId": "", "terminateProcess": false }`; remote debugging beyond a reachable host/port requires manual configuration (e.g. an SSH tunnel). +Shorthand: `create_debug_session { "language": "javascript", "host": "127.0.0.1", "port": 9229 }` attaches in one call. Attach is verified by polling for threads (`verifyTimeout`, default ~20000 ms). Detach with `detach_from_process { "sessionId": "", "terminateProcess": false }`; remote debugging beyond a reachable host/port requires manual configuration (e.g. an SSH tunnel). ## Quirks diff --git a/skills/debugging/references/python.md b/skills/debugging/references/python.md index 7f945057..65b3bb6a 100644 --- a/skills/debugging/references/python.md +++ b/skills/debugging/references/python.md @@ -49,7 +49,7 @@ set_breakpoint { "sessionId": "", "file": "/abs/path/script.py", "line continue_execution { "sessionId": "" } ``` -Shorthand: `create_debug_session { "language": "python", "host": "127.0.0.1", "port": 5678 }` creates the session and attaches in one call. After the handshake the attach is verified by polling for threads; raise `verifyTimeout` (default ~5000 ms) for slow targets. Detach with `detach_from_process { "sessionId": "", "terminateProcess": false }`. +Shorthand: `create_debug_session { "language": "python", "host": "127.0.0.1", "port": 5678 }` creates the session and attaches in one call. After the handshake the attach is verified by polling for threads; raise `verifyTimeout` (default ~20000 ms) for slow targets. Detach with `detach_from_process { "sessionId": "", "terminateProcess": false }`. ## Quirks diff --git a/src/server.ts b/src/server.ts index 43707235..2066f257 100644 --- a/src/server.ts +++ b/src/server.ts @@ -872,7 +872,11 @@ export class DebugMcpServer { if (typeof currentThreadId !== 'number') { throw new ProxyNotRunningError(sessionId || 'unknown', 'get stack trace'); } - return this.sessionManager.getStackTraceDetailed(sessionId, currentThreadId, includeInternals); + // ensureStackReady: the thread above was resolved implicitly (the MCP tool + // has no threadId argument), so a paused session answering with zero + // frames gets the bounded readiness retry + thread scan instead of a + // confusing empty success. + return this.sessionManager.getStackTraceDetailed(sessionId, currentThreadId, includeInternals, { ensureStackReady: true }); } public async getScopes(sessionId: string, frameId: number): Promise { @@ -1080,7 +1084,7 @@ export class DebugMcpServer { return { tools: [ - { name: 'create_debug_session', description: 'Create a new debugging session. Provide host and port to attach to a running process; omit them for launch mode', inputSchema: { type: 'object', properties: { language: { type: 'string', enum: supportedLanguages, description: 'Programming language for debugging' }, name: { type: 'string', description: 'Optional session name' }, executablePath: {type: 'string', description: 'Path to language executable (optional, will auto-detect if not provided)'}, host: { type: 'string', description: 'Host to attach to for remote debugging (optional, triggers attach mode)' }, port: { type: 'number', description: 'Debug port to attach to for remote debugging (optional, triggers attach mode)' }, timeout: { type: 'number', description: 'Connection timeout in milliseconds for attach mode (default: 30000)' }, verifyTimeout: { type: 'number', description: 'Attach mode only: how long to wait (ms) for the debugger to report at least one thread after attaching before failing the attach (default: 5000, max: 600000)' }, adapterConfig: { type: 'object', description: 'Attach mode only: adapter-specific attach configuration merged into the attach config (see attach_to_process)', additionalProperties: true } }, required: ['language'] } }, + { name: 'create_debug_session', description: 'Create a new debugging session. Provide host and port to attach to a running process; omit them for launch mode', inputSchema: { type: 'object', properties: { language: { type: 'string', enum: supportedLanguages, description: 'Programming language for debugging' }, name: { type: 'string', description: 'Optional session name' }, executablePath: {type: 'string', description: 'Path to language executable (optional, will auto-detect if not provided)'}, host: { type: 'string', description: 'Host to attach to for remote debugging (optional, triggers attach mode)' }, port: { type: 'number', description: 'Debug port to attach to for remote debugging (optional, triggers attach mode)' }, timeout: { type: 'number', description: 'Connection timeout in milliseconds for attach mode (default: 30000)' }, verifyTimeout: { type: 'number', description: 'Attach mode only: how long to wait (ms) for the debugger to report at least one thread after attaching before failing the attach (default: 20000, max: 600000)' }, adapterConfig: { type: 'object', description: 'Attach mode only: adapter-specific attach configuration merged into the attach config (see attach_to_process)', additionalProperties: true } }, required: ['language'] } }, { name: 'list_supported_languages', description: 'List all supported debugging languages with metadata', inputSchema: { type: 'object', properties: {} } }, { name: 'list_debug_sessions', description: 'List all active debugging sessions. Paused sessions include lastStop with the reason for the most recent stop (e.g. "breakpoint" vs "exception")', inputSchema: { type: 'object', properties: {} } }, { name: 'set_breakpoint', description: 'Set a breakpoint. Setting breakpoints on non-executable lines (structural, declarative) may lead to unexpected behavior', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' }, file: { type: 'string', description: 'Path to the source file or Java FQCN. For Java, passing a fully-qualified class name (e.g. "com.example.MyClass" or "com.example.Outer$Inner") is preferred — it works reliably with all classloaders including custom classloaders. Alternatively, use absolute file paths.' }, line: { type: 'number', description: 'Line number where to set breakpoint. Executable statements (assignments, function calls, conditionals, returns) work best. Structural lines (function/class definitions), declarative lines (imports), or non-executable lines (comments, blank lines) may cause unexpected stepping behavior' }, ...setBreakpointExtraProps, condition: { type: 'string', description: 'Optional expression: only break (or log) when it evaluates truthy' }, logMessage: { type: 'string', description: 'Create a logpoint: instead of pausing, log this message when the line is hit. Expressions in {curly braces} are interpolated (e.g. "order={orderId} total={total}"). Messages arrive in get_output while the program runs at full speed. Supported by Python, JavaScript, Go, and Rust adapters; not by Java or .NET' }, suspendPolicy: { type: 'string', enum: ['all', 'thread'], description: 'Suspend policy when breakpoint is hit: "all" suspends all threads (default), "thread" only suspends the event thread. Only supported by the Java/JDI adapter.' } }, required: setBreakpointRequired } }, @@ -1113,7 +1117,7 @@ export class DebugMcpServer { } }, { name: 'restart_debugging', description: 'Restart the debuggee: terminate the current program (if still running) and relaunch it with the same configuration as the last start_debugging. All current breakpoints are re-applied automatically. The get_output buffer starts fresh — read from since=0 after a restart. Works while running, paused, or after the program has exited. Not available for attach sessions or sessions never launched', inputSchema: { type: 'object', properties: { sessionId: { type: 'string' } }, required: ['sessionId'] } }, - { name: 'attach_to_process', description: 'Attach to a running process for debugging. After the attach handshake the target is verified by polling for threads; if none are reported within verifyTimeout (~5s default) the attach fails and the debug proxy is torn down', inputSchema: { + { name: 'attach_to_process', description: 'Attach to a running process for debugging. After the attach handshake the target is verified by polling for threads; if none are reported within verifyTimeout (~20s default) the attach fails and the debug proxy is torn down', inputSchema: { type: 'object', properties: { sessionId: { type: 'string', description: 'Debug session ID' }, @@ -1121,7 +1125,7 @@ export class DebugMcpServer { host: { type: 'string', description: 'Host to attach to (default: localhost)' }, processId: { type: ['number', 'string'], description: 'Process ID (for local attach, language-specific)' }, timeout: { type: 'number', description: 'Connection timeout in milliseconds (default: 30000)' }, - verifyTimeout: { type: 'number', description: 'How long to wait (ms) for the debugger to report at least one thread after attaching before failing the attach (default: 5000, max: 600000). Increase for targets that are slow to become debuggable, e.g. a busy or warming JVM' }, + verifyTimeout: { type: 'number', description: 'How long to wait (ms) for the debugger to report at least one thread after attaching before failing the attach (default: 20000, max: 600000). Decrease for fast failure-by-design probes; increase for targets that are exceptionally slow to become debuggable' }, sourcePaths: { type: 'array', items: { type: 'string' }, description: 'Source paths for code mapping' }, stopOnEntry: { type: 'boolean', description: 'Stop on entry after attaching' }, justMyCode: { type: 'boolean', description: 'Only debug user code (skip library code)' }, @@ -2055,13 +2059,21 @@ export class DebugMcpServer { stopReason: lastStop?.reason, lastStop }; - // Issue #346: when the language policy hid frames, say so in the - // response instead of relying on the agent knowing filtering exists. + // Anything the result needs explaining (not paused, stack came + // from a different thread, all threads frameless) plus the + // issue #346 hidden-frames disclosure share the note field. + const notes: string[] = []; + if (stackTrace.note) { + notes.push(stackTrace.note); + } if (stackTrace.hiddenFrameCount > 0) { payload.hiddenFrames = stackTrace.hiddenFrameCount; - payload.note = stackTrace.allFramesInternal + notes.push(stackTrace.allFramesInternal ? `All ${stackTrace.totalFrameCount} frames are internal/runtime frames; showing the top internal frame so scopes and evaluate still work. Pass includeInternals: true to see the full stack.` - : `${stackTrace.hiddenFrameCount} internal frame(s) hidden — pass includeInternals: true to see them.`; + : `${stackTrace.hiddenFrameCount} internal frame(s) hidden — pass includeInternals: true to see them.`); + } + if (notes.length > 0) { + payload.note = notes.join(' '); } result = { content: [{ type: 'text', text: JSON.stringify(payload) }] }; } catch (error) { diff --git a/src/session/session-manager-data.ts b/src/session/session-manager-data.ts index ca82d299..a91ef684 100644 --- a/src/session/session-manager-data.ts +++ b/src/session/session-manager-data.ts @@ -14,6 +14,7 @@ import { redactVariableValue } from '@debugmcp/shared'; import { SessionManagerCore } from './session-manager-core.js'; +import { IProxyManager } from '../proxy/proxy-manager.js'; import { DebugProtocol } from '@vscode/debugprotocol'; import { applyVariableCaps, @@ -32,16 +33,36 @@ export interface StackTraceResult { totalFrameCount: number; hiddenFrameCount: number; allFramesInternal: boolean; + /** + * Present when the result needs explaining: the session was not paused, no + * stopped thread was known, the stack came from a different thread than the + * tracked one, or every thread stayed frameless. Surfaced to the agent in + * the get_stack_trace tool payload. + */ + note?: string; } -function emptyStackTraceResult(): StackTraceResult { - return { frames: [], totalFrameCount: 0, hiddenFrameCount: 0, allFramesInternal: false }; +function emptyStackTraceResult(note?: string): StackTraceResult { + return { frames: [], totalFrameCount: 0, hiddenFrameCount: 0, allFramesInternal: false, ...(note ? { note } : {}) }; } /** * Data retrieval functionality for session management */ export abstract class SessionManagerData extends SessionManagerCore { + /** + * How long the agent-facing stack-trace path keeps polling a PAUSED session + * whose adapter answers stackTrace with success + zero frames, before + * returning the honest empty result. Some adapters report the stop before + * the stack is materialized (netcoredbg after the post-attach pause — the + * milder sibling of issue #353's 0x80131302), so success-with-0-frames + * while paused is nearly always a transient race, not truth. The poll exits + * on the first non-empty answer, so the window costs nothing when the + * adapter is ready. Follows the attachVerifyTimeoutMs pattern. + */ + protected pausedStackReadyTimeoutMs = 3000; + protected pausedStackReadyIntervalMs = 250; + /** * Selects the appropriate adapter policy based on language */ @@ -167,75 +188,184 @@ export abstract class SessionManagerData extends SessionManagerCore { * has a frameId to anchor scopes/evaluate, and `hiddenFrameCount` + * `allFramesInternal` let the response say what was hidden. */ - async getStackTraceDetailed(sessionId: string, threadId?: number, includeInternals: boolean = false): Promise { + async getStackTraceDetailed( + sessionId: string, + threadId?: number, + includeInternals: boolean = false, + opts?: { ensureStackReady?: boolean } + ): Promise { const session = this._getSessionById(sessionId); const currentThreadId = session.proxyManager?.getCurrentThreadId(); this.logger.info(`[SM getStackTrace ${sessionId}] Entered. Requested threadId: ${threadId}, Current state: ${session.state}, Actual currentThreadId: ${currentThreadId}, includeInternals: ${includeInternals}`); - + if (!session.proxyManager || !session.proxyManager.isRunning()) { this.logger.warn(`[SM getStackTrace ${sessionId}] No active proxy.`); - return emptyStackTraceResult(); + return emptyStackTraceResult('No active debug process for this session.'); } if (session.state !== SessionState.PAUSED) { this.logger.warn(`[SM getStackTrace ${sessionId}] Session not paused. State: ${session.state}.`); - return emptyStackTraceResult(); + return emptyStackTraceResult(`Session is not paused (state: ${session.state}); stack traces are only available while paused.`); } const currentThreadForRequest = threadId || currentThreadId; if (typeof currentThreadForRequest !== 'number') { this.logger.warn(`[SM getStackTrace ${sessionId}] No effective thread ID to use.`); - return emptyStackTraceResult(); + return emptyStackTraceResult('No stopped thread is known for this session.'); } + const proxyManager = session.proxyManager; try { - this.logger.info(`[SM getStackTrace ${sessionId}] Sending DAP 'stackTrace' for threadId ${currentThreadForRequest}.`); - const response = await session.proxyManager.sendDapRequest('stackTrace', { threadId: currentThreadForRequest }); - this.logger.info(`[SM getStackTrace ${sessionId}] DAP 'stackTrace' response received. Body:`, response?.body); + let rawFrames = await this.requestRawStackFrames(sessionId, proxyManager, currentThreadForRequest); + let note: string | undefined; - // A failed DAP response (e.g. "Child session not ready ...") must not - // be flattened into an empty-but-successful stack trace (issue #124): - // propagate the failure to the caller. - if (response?.success === false) { - throw new Error(response.message || `DAP 'stackTrace' request failed`); + // A PAUSED session answering with zero frames is nearly always a + // transient adapter race (netcoredbg materializes the managed stack a + // beat after the stop event) or a frameless runtime thread being + // tracked as current. On the agent-facing path, chase the real stack + // instead of handing back a confusing empty success. + if (rawFrames.length === 0 && opts?.ensureStackReady) { + const ready = await this.waitForReadyStack(sessionId, session, proxyManager, currentThreadForRequest); + rawFrames = ready.frames; + note = ready.note; } - if (response && response.body && response.body.stackFrames) { - let frames: StackFrame[] = response.body.stackFrames.map((sf: DebugProtocol.StackFrame) => ({ - id: sf.id, name: sf.name, - file: sf.source?.path || sf.source?.name || "", - line: sf.line, column: sf.column - })); - - // Apply filtering using the language's policy - const totalFrameCount = frames.length; - let allFramesInternal = false; - const policy = this.selectPolicy(session.language); - if (policy.filterStackFrames) { - this.logger.info(`[SM getStackTrace ${sessionId}] Applying stack frame filtering for ${session.language}. Original count: ${frames.length}`); - const filtered = policy.filterStackFrames(frames, includeInternals); - // Central guarantee (issue #346): a policy filter must never leave the - // agent with zero frames when the adapter reported some — keep the top - // unfiltered frame so scopes/evaluate still have an anchor. - if (filtered.length === 0 && frames.length > 0) { - allFramesInternal = true; - frames = [frames[0]]; - } else { - frames = filtered; - } - this.logger.info(`[SM getStackTrace ${sessionId}] After filtering: ${frames.length} frames (hidden: ${totalFrameCount - frames.length}, allFramesInternal: ${allFramesInternal})`); - } + let frames: StackFrame[] = rawFrames.map((sf: DebugProtocol.StackFrame) => ({ + id: sf.id, name: sf.name, + file: sf.source?.path || sf.source?.name || "", + line: sf.line, column: sf.column + })); - this.logger.info(`[SM getStackTrace ${sessionId}] Parsed stack frames (top 3):`, frames.slice(0,3).map(f => ({name:f.name, file:f.file, line:f.line}))); - return { frames, totalFrameCount, hiddenFrameCount: totalFrameCount - frames.length, allFramesInternal }; + // Apply filtering using the language's policy + const totalFrameCount = frames.length; + let allFramesInternal = false; + const policy = this.selectPolicy(session.language); + if (policy.filterStackFrames) { + this.logger.info(`[SM getStackTrace ${sessionId}] Applying stack frame filtering for ${session.language}. Original count: ${frames.length}`); + const filtered = policy.filterStackFrames(frames, includeInternals); + // Central guarantee (issue #346): a policy filter must never leave the + // agent with zero frames when the adapter reported some — keep the top + // unfiltered frame so scopes/evaluate still have an anchor. + if (filtered.length === 0 && frames.length > 0) { + allFramesInternal = true; + frames = [frames[0]]; + } else { + frames = filtered; + } + this.logger.info(`[SM getStackTrace ${sessionId}] After filtering: ${frames.length} frames (hidden: ${totalFrameCount - frames.length}, allFramesInternal: ${allFramesInternal})`); } - this.logger.warn(`[SM getStackTrace ${sessionId}] No stackFrames in response body. Response:`, response); - throw new Error(`DAP 'stackTrace' response did not include stack frames`); + + this.logger.info(`[SM getStackTrace ${sessionId}] Parsed stack frames (top 3):`, frames.slice(0,3).map(f => ({name:f.name, file:f.file, line:f.line}))); + return { frames, totalFrameCount, hiddenFrameCount: totalFrameCount - frames.length, allFramesInternal, ...(note ? { note } : {}) }; } catch (error) { this.logger.error(`[SM getStackTrace ${sessionId}] Error getting stack trace:`, error); throw error instanceof Error ? error : new Error(String(error)); } } + /** + * Send one DAP stackTrace request and return the raw frames. A failed DAP + * response (e.g. "Child session not ready ...") must not be flattened into + * an empty-but-successful stack trace (issue #124): propagate the failure. + */ + private async requestRawStackFrames( + sessionId: string, + proxyManager: IProxyManager, + threadId: number + ): Promise { + this.logger.info(`[SM getStackTrace ${sessionId}] Sending DAP 'stackTrace' for threadId ${threadId}.`); + const response = await proxyManager.sendDapRequest('stackTrace', { threadId }); + this.logger.info(`[SM getStackTrace ${sessionId}] DAP 'stackTrace' response received. Body:`, response?.body); + + if (response?.success === false) { + throw new Error(response.message || `DAP 'stackTrace' request failed`); + } + if (!response || !response.body || !response.body.stackFrames) { + this.logger.warn(`[SM getStackTrace ${sessionId}] No stackFrames in response body. Response:`, response); + throw new Error(`DAP 'stackTrace' response did not include stack frames`); + } + return response.body.stackFrames; + } + + /** + * Bounded readiness loop for a PAUSED session whose stackTrace succeeded + * with zero frames: re-poll the same thread (the stack may not be + * materialized yet), and each round also scan the other stopped threads — + * the tracked thread may be a frameless runtime thread (finalizer, JIT) + * while the real stack lives elsewhere. A frame-bearing thread found by the + * scan is adopted as current so scopes/evaluate anchor to it. If nothing + * reports frames within the window, return the honest empty answer with a + * note telling the agent what to try instead. + */ + private async waitForReadyStack( + sessionId: string, + session: { state: SessionState }, + proxyManager: IProxyManager, + threadId: number + ): Promise<{ frames: DebugProtocol.StackFrame[]; note?: string }> { + const deadline = Date.now() + this.pausedStackReadyTimeoutMs; + while (Date.now() < deadline) { + await new Promise(resolve => setTimeout(resolve, this.pausedStackReadyIntervalMs)); + if (session.state !== SessionState.PAUSED) { + return { frames: [], note: 'The session left the paused state while waiting for the stack; it is no longer paused.' }; + } + const frames = await this.requestRawStackFrames(sessionId, proxyManager, threadId); + if (frames.length > 0) { + return { frames }; + } + const scanned = await this.scanThreadsForFrames(sessionId, proxyManager, threadId); + if (scanned) { + proxyManager.setCurrentThreadId(scanned.threadId); + this.logger.info(`[SM getStackTrace ${sessionId}] Thread ${threadId} stayed frameless; adopted thread ${scanned.threadId} which has a stack.`); + return { + frames: scanned.frames, + note: `The stopped thread ${threadId} reported no stack frames; switched to thread ${scanned.threadId}, which has one.` + }; + } + } + this.logger.warn(`[SM getStackTrace ${sessionId}] Stack stayed empty for ${this.pausedStackReadyTimeoutMs}ms while paused (threadId ${threadId}).`); + return { + frames: [], + note: `The stopped thread reported no stack frames within ${this.pausedStackReadyTimeoutMs}ms; the target may be paused in native code. Retry get_stack_trace, or use list_threads to inspect other threads.` + }; + } + + /** + * Probe the other stopped threads for one that reports stack frames. + * Probe failures on individual threads are not fatal — runtime threads may + * reject stackTrace outright. + */ + private async scanThreadsForFrames( + sessionId: string, + proxyManager: IProxyManager, + excludeThreadId: number + ): Promise<{ threadId: number; frames: DebugProtocol.StackFrame[] } | null> { + let threads: DebugProtocol.Thread[] | undefined; + try { + const response = await proxyManager.sendDapRequest('threads', {}); + threads = response?.body?.threads; + } catch (err) { + this.logger.warn(`[SM getStackTrace ${sessionId}] Thread scan could not list threads: ${err instanceof Error ? err.message : String(err)}`); + return null; + } + if (!Array.isArray(threads)) { + return null; + } + for (const thread of threads) { + if (!thread || typeof thread.id !== 'number' || thread.id === excludeThreadId) { + continue; + } + try { + const frames = await this.requestRawStackFrames(sessionId, proxyManager, thread.id); + if (frames.length > 0) { + return { threadId: thread.id, frames }; + } + } catch { + // This thread rejected stackTrace — keep scanning. + } + } + return null; + } + async getScopes(sessionId: string, frameId: number): Promise { const session = this._getSessionById(sessionId); this.logger.info(`[SM getScopes ${sessionId}] Entered. frameId: ${frameId}, Current state: ${session.state}`); diff --git a/src/session/session-manager-operations.ts b/src/session/session-manager-operations.ts index b39cc07a..efa06831 100644 --- a/src/session/session-manager-operations.ts +++ b/src/session/session-manager-operations.ts @@ -100,11 +100,20 @@ export abstract class SessionManagerOperations extends SessionManagerData { * 'threads' is polled until the debugger reports at least one thread. * If the window elapses without any threads, the attach is reported as a * failure instead of a false "paused" success (issue #124). - * Callers can widen the window per attach via the 'verifyTimeout' tool - * argument for targets that are slow to become debuggable (issue #143). + * Callers can adjust the window per attach via the 'verifyTimeout' tool + * argument (issue #143) — smaller for fast failure-by-design probes, larger + * for targets that are exceptionally slow to become debuggable. + * + * The default is deliberately generous: an adapter that dies mid-verify + * fails fast regardless (the proxyGone latch), so the deadline only ever + * bites when the adapter is alive but the target is slow to report threads + * — e.g. js-debug child-session adoption on a heavily loaded host, or a + * warming JVM — where a false "attach failed" is far worse than a slow + * genuine failure. The poll exits as soon as threads appear, so healthy + * attaches never pay for the headroom. * Protected so tests can shrink the window. */ - protected attachVerifyTimeoutMs = 5000; + protected attachVerifyTimeoutMs = 20000; protected attachVerifyIntervalMs = 250; /** diff --git a/src/utils/error-messages.ts b/src/utils/error-messages.ts index 2d224686..1986da0a 100644 --- a/src/utils/error-messages.ts +++ b/src/utils/error-messages.ts @@ -74,7 +74,7 @@ export const ErrorMessages = { * threads within the verification window — either the attach is dead * (issue #124) or the target is slow to become debuggable (issue #143) * Used in: src/session/session-manager-operations.ts - * Default window: 5 seconds, overridable per call via 'verifyTimeout' + * Default window: 20 seconds, overridable per call via 'verifyTimeout' * @param timeoutMs - The verification window in milliseconds * @param lastFailure - The last observed failure while polling 'threads' */ diff --git a/tests/core/unit/session/session-manager-dap.test.ts b/tests/core/unit/session/session-manager-dap.test.ts index 0ef13cc5..01755e90 100644 --- a/tests/core/unit/session/session-manager-dap.test.ts +++ b/tests/core/unit/session/session-manager-dap.test.ts @@ -1252,6 +1252,139 @@ describe('SessionManager - DAP Operations', () => { }); }); + describe('Paused stack readiness (empty-stack hardening)', () => { + // A PAUSED session answering stackTrace with success + zero frames is + // (nearly always) a transient adapter race — netcoredbg does this right + // after the post-attach pause. With ensureStackReady the agent-facing + // path retries within a bounded window and falls back to scanning other + // stopped threads; internal callers keep the single-shot behavior. + const READY_FRAME = { id: 7, name: 'Program.Main', source: { path: '/work/Program.cs' }, line: 12, column: 1 }; + + function setShortReadyWindow() { + (sessionManager as unknown as { pausedStackReadyTimeoutMs: number; pausedStackReadyIntervalMs: number }) + .pausedStackReadyTimeoutMs = 400; + (sessionManager as unknown as { pausedStackReadyIntervalMs: number }) + .pausedStackReadyIntervalMs = 25; + } + + it('retries an empty-but-successful stack until frames appear', async () => { + const session = await createPausedSession(); + setShortReadyWindow(); + let stackTraceCalls = 0; + dependencies.mockProxyManager.setDapRequestHandler(async (command: string) => { + if (command === 'stackTrace') { + stackTraceCalls++; + return { success: true, body: { stackFrames: stackTraceCalls < 3 ? [] : [READY_FRAME] } }; + } + return { success: true, body: {} }; + }); + + const result = await sessionManager.getStackTraceDetailed(session.id, undefined, false, { ensureStackReady: true }); + + expect(result.frames.map(f => f.name)).toEqual(['Program.Main']); + expect(stackTraceCalls).toBeGreaterThanOrEqual(3); + expect(result.note).toBeUndefined(); + }); + + it('falls back to another stopped thread when the current one stays frameless, and annotates', async () => { + const session = await createPausedSession(); // currentThreadId = 1 + setShortReadyWindow(); + dependencies.mockProxyManager.setDapRequestHandler(async (command: string, args?: { threadId?: number }) => { + if (command === 'stackTrace') { + return { success: true, body: { stackFrames: args?.threadId === 2 ? [READY_FRAME] : [] } }; + } + if (command === 'threads') { + return { success: true, body: { threads: [{ id: 1, name: '' }, { id: 2, name: 'worker' }] } }; + } + return { success: true, body: {} }; + }); + + const result = await sessionManager.getStackTraceDetailed(session.id, undefined, false, { ensureStackReady: true }); + + expect(result.frames.map(f => f.name)).toEqual(['Program.Main']); + expect(result.note).toMatch(/thread 2/); + // The frame-bearing thread is adopted so scopes/evaluate anchor to it. + expect(dependencies.mockProxyManager.getCurrentThreadId()).toBe(2); + }); + + it('returns an honest empty result with a note when no thread reports frames', async () => { + const session = await createPausedSession(); + setShortReadyWindow(); + dependencies.mockProxyManager.setDapRequestHandler(async (command: string) => { + if (command === 'stackTrace') { + return { success: true, body: { stackFrames: [] } }; + } + if (command === 'threads') { + return { success: true, body: { threads: [{ id: 1, name: '' }] } }; + } + return { success: true, body: {} }; + }); + + const result = await sessionManager.getStackTraceDetailed(session.id, undefined, false, { ensureStackReady: true }); + + expect(result.frames).toEqual([]); + expect(result.note).toMatch(/no stack frames/i); + }); + + it('does not retry or scan without ensureStackReady (internal-caller behavior unchanged)', async () => { + const session = await createPausedSession(); + dependencies.mockProxyManager.setDapRequestHandler(async (command: string) => { + if (command === 'stackTrace') { + return { success: true, body: { stackFrames: [] } }; + } + return { success: true, body: {} }; + }); + + const result = await sessionManager.getStackTraceDetailed(session.id); + + expect(result.frames).toEqual([]); + expect(dependencies.mockProxyManager.dapRequestCalls.filter(c => c.command === 'stackTrace')).toHaveLength(1); + }); + + // Raw 'continued' events are deliberately ignored while PAUSED (the + // stale-event guard in session-manager-core), so these tests leave the + // paused state the way the continue/step operations do: through + // _updateSessionState. + function forceRunning(sessionId: string) { + const managed = sessionManager.getSession(sessionId); + (sessionManager as unknown as { _updateSessionState: (s: unknown, st: SessionState) => void }) + ._updateSessionState(managed, SessionState.RUNNING); + } + + it('stops retrying when the session leaves PAUSED mid-wait', async () => { + const session = await createPausedSession(); + setShortReadyWindow(); + let stackTraceCalls = 0; + dependencies.mockProxyManager.setDapRequestHandler(async (command: string) => { + if (command === 'stackTrace') { + stackTraceCalls++; + if (stackTraceCalls === 1) { + // A continue races in between readiness polls. + forceRunning(session.id); + } + return { success: true, body: { stackFrames: [] } }; + } + return { success: true, body: {} }; + }); + + const result = await sessionManager.getStackTraceDetailed(session.id, undefined, false, { ensureStackReady: true }); + + expect(result.frames).toEqual([]); + expect(stackTraceCalls).toBe(1); + expect(result.note).toMatch(/paused/i); + }); + + it('annotates the not-paused empty result', async () => { + const session = await createPausedSession(); + forceRunning(session.id); + + const result = await sessionManager.getStackTraceDetailed(session.id); + + expect(result.frames).toEqual([]); + expect(result.note).toMatch(/not paused/i); + }); + }); + describe('Exception Breakpoints and Stop Detail (issue #220)', () => { it('threads breakOnExceptions into the ProxyConfig', async () => { const session = await sessionManager.createSession({ diff --git a/tests/e2e/comprehensive-mcp-tools.test.ts b/tests/e2e/comprehensive-mcp-tools.test.ts index 583fcb81..85ab421e 100644 --- a/tests/e2e/comprehensive-mcp-tools.test.ts +++ b/tests/e2e/comprehensive-mcp-tools.test.ts @@ -748,6 +748,9 @@ describe(`Comprehensive MCP Debugger Test — ${ALL_TOOLS.length} Tools × ${LAN sessionId: currentSessionId, port: 5678, host: 'localhost', + // Nothing is listening — failure is the expected outcome, so opt + // out of the generous default verify window to keep the sweep fast. + verifyTimeout: 2000, }); // Attach will likely fail because no process is actually listening diff --git a/tests/e2e/mcp-server-smoke-dotnet-attach.test.ts b/tests/e2e/mcp-server-smoke-dotnet-attach.test.ts index b3312e36..76393de5 100644 --- a/tests/e2e/mcp-server-smoke-dotnet-attach.test.ts +++ b/tests/e2e/mcp-server-smoke-dotnet-attach.test.ts @@ -173,8 +173,8 @@ describe.skipIf(SKIP_DOTNET)('MCP Server .NET Attach Smoke Test @requires-dotnet name: 'get_stack_trace', arguments: { sessionId } })) as { success?: boolean; stackFrames?: Array<{ name?: string; file?: string }> }; - expect(stackResponse.success).toBe(true); - expect((stackResponse.stackFrames ?? []).length).toBeGreaterThan(0); + expect(stackResponse.success, JSON.stringify(stackResponse)).toBe(true); + expect((stackResponse.stackFrames ?? []).length, JSON.stringify(stackResponse)).toBeGreaterThan(0); // Breakpoints must bind against the attached process ("No symbols have // been loaded" was the second #353 symptom). diff --git a/tests/e2e/mcp-server-smoke-js-function-bp.test.ts b/tests/e2e/mcp-server-smoke-js-function-bp.test.ts index fd5ad89e..39ce2890 100644 --- a/tests/e2e/mcp-server-smoke-js-function-bp.test.ts +++ b/tests/e2e/mcp-server-smoke-js-function-bp.test.ts @@ -359,7 +359,7 @@ describe('MCP Server JavaScript Function Breakpoints', () => { name: 'attach_to_process', arguments: { sessionId: sid, host: '127.0.0.1', port } })); - expect(attachResponse.success).toBe(true); + expect(attachResponse.success, JSON.stringify(attachResponse)).toBe(true); // Ensure the target is running (attach may leave it paused), then set the // function breakpoint while it runs: no pause frames exist, so the bridge diff --git a/tests/test-utils/mocks/mock-proxy-manager.ts b/tests/test-utils/mocks/mock-proxy-manager.ts index 5d4f6a28..88d97d0f 100644 --- a/tests/test-utils/mocks/mock-proxy-manager.ts +++ b/tests/test-utils/mocks/mock-proxy-manager.ts @@ -204,6 +204,10 @@ export class MockProxyManager extends EventEmitter implements IProxyManager { return this._currentThreadId; } + setCurrentThreadId(threadId: number): void { + this._currentThreadId = threadId; + } + // Test helpers setDapRequestHandler(handler: (command: string, args?: any) => Promise): void { this._dapRequestHandler = handler; diff --git a/tests/unit/server-coverage.test.ts b/tests/unit/server-coverage.test.ts index 5fda75e5..5f59c126 100644 --- a/tests/unit/server-coverage.test.ts +++ b/tests/unit/server-coverage.test.ts @@ -181,7 +181,7 @@ describe('Server Coverage - Error Paths and Edge Cases', () => { const result = await server.getStackTrace('test-session'); expect(mockProxy.sendDapRequest).toHaveBeenCalledWith('threads', {}); - expect(mockSessionManager.getStackTraceDetailed).toHaveBeenCalledWith('test-session', 5, false); + expect(mockSessionManager.getStackTraceDetailed).toHaveBeenCalledWith('test-session', 5, false, { ensureStackReady: true }); expect(result.frames).toHaveLength(1); });