From cf6f681585563164e97ca6f66b2d5ee943ce712f Mon Sep 17 00:00:00 2001 From: JF Date: Sun, 30 Aug 2026 19:02:12 -0400 Subject: [PATCH 1/2] feat: retain proxy death diagnostics (#570) --- changelog.d/570.changed.md | 1 + docs/tool-reference.md | 3 ++ packages/shared/src/index.ts | 1 + packages/shared/src/models/index.ts | 10 ++++ src/server/handlers/inspection-tools.ts | 16 +++++-- src/server/handlers/session-tools.ts | 3 ++ src/session/attach/attach-controller.ts | 4 ++ src/session/launch/debug-launcher.ts | 10 ++++ .../launch/proxy-failure-diagnostics.ts | 6 +-- src/session/session-manager-core.ts | 47 ++++++++++++++++++- src/session/session-store.ts | 6 +++ .../server/handlers/session-tools.test.ts | 6 ++- .../server/server-inspection-tools.test.ts | 11 ++++- .../session-manager-exit-mapping.test.ts | 29 ++++++++++++ 14 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 changelog.d/570.changed.md diff --git a/changelog.d/570.changed.md b/changelog.d/570.changed.md new file mode 100644 index 000000000..c6352af83 --- /dev/null +++ b/changelog.d/570.changed.md @@ -0,0 +1 @@ +Persist structured proxy-failure diagnostics for mid-session adapter/proxy deaths and expose them on errored `list_debug_sessions` entries and failed or empty `get_stack_trace` responses. diff --git a/docs/tool-reference.md b/docs/tool-reference.md index b282269d0..1ce6c9375 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -108,6 +108,8 @@ Lists all active debugging sessions. - `"stopped"`: Session stopped (program terminated) - `"error"`: Session encountered an error +Errored sessions include optional `diagnostics` with the current launch attempt's `proxyLogPath`. The record is retained for proxy initialization failures and for proxy/adapter deaths after initialization, and is cleared when a new launch or attach attempt begins. + --- ### close_debug_session @@ -586,6 +588,7 @@ Gets the current call stack. - The filtered stack is never empty when the adapter reported frames: if *every* frame is internal (e.g. a goroutine paused inside the Go runtime), the top internal frame is kept so `get_scopes`/`evaluate_expression` still have a valid `frameId`, and the `note` says so. - When an explicit thread reports no frames, the response remains anchored to that thread and its `note` suggests a frame-bearing alternative when one is available. - When the implicit stopped thread is frameless, stack, locals, and default evaluation share one resolver. It scans siblings, prefers a thread whose frames the language policy recognizes as user code over runtime-only stacks, adopts it once, and discloses the switch in `note`/`anchorNote`. +- Failed or empty stack responses include the session's optional `diagnostics` when the proxy failed, matching `list_debug_sessions`. --- diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index a1bf47168..a0780a8cc 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -152,6 +152,7 @@ export type { SessionStopExceptionInfo, ExceptionBreakMode, SessionOutputEntry, + SessionFailureDiagnostics, // Debug info types Variable, diff --git a/packages/shared/src/models/index.ts b/packages/shared/src/models/index.ts index 58b0445b8..a9121666c 100644 --- a/packages/shared/src/models/index.ts +++ b/packages/shared/src/models/index.ts @@ -370,6 +370,14 @@ export interface SessionOutputEntry { redacted?: boolean; } +/** Pointers that make an errored session's proxy failure actionable. */ +export interface SessionFailureDiagnostics { + /** Server-host path to the current launch attempt's proxy log. */ + proxyLogPath?: string; + /** MCP resource URI for remote-safe access to the proxy log, when available. */ + proxyLogResource?: string; +} + /** * Public session snapshot returned by create/list operations. Unlike the * internal DebugSession, it carries no live handles and adds observed @@ -386,6 +394,8 @@ export interface DebugSessionInfo { lastStop?: SessionStopInfo; /** Debuggee exit code from the DAP 'exited' event, when the adapter reports one */ exitCode?: number; + /** Present when the session is in ERROR because its proxy failed. */ + diagnostics?: SessionFailureDiagnostics; /** * Live DAP mirror endpoint from expose_session (issue #217), host/port * only — the attach token is returned solely by the expose_session tool. diff --git a/src/server/handlers/inspection-tools.ts b/src/server/handlers/inspection-tools.ts index 46faadfcf..fa0c9ee1f 100644 --- a/src/server/handlers/inspection-tools.ts +++ b/src/server/handlers/inspection-tools.ts @@ -51,6 +51,9 @@ export const getVariablesTool: ToolHandler = async (ctx, args) => { export const getStackTraceTool: ToolHandler = async (ctx, args) => { requireSessionId(args); + const failureDiagnostics = () => + ctx.sessionManager.getSession(args.sessionId)?.failureDiagnostics; + try { // Default to false for cleaner output const includeInternals = args.includeInternals ?? false; @@ -63,7 +66,10 @@ export const getStackTraceTool: ToolHandler = async (ctx, args) => { ...(typeof stackTrace.threadId === 'number' ? { threadId: stackTrace.threadId } : {}), includeInternals, stopReason: lastStop?.reason, - lastStop + lastStop, + ...(stackTrace.frames.length === 0 && failureDiagnostics() + ? { diagnostics: failureDiagnostics() } + : {}) }; // Anything the result needs explaining (not paused, stack came // from a different thread, all threads frameless) plus the @@ -85,13 +91,17 @@ export const getStackTraceTool: ToolHandler = async (ctx, args) => { } catch (error) { const sessionResult = sessionErrorToResult(error); if (sessionResult) { - return sessionResult; + const diagnostics = failureDiagnostics(); + return diagnostics + ? sessionErrorToResult(error, { diagnostics })! + : sessionResult; } if (error instanceof Error && !(error instanceof McpError)) { // DAP-level failures (e.g. "Child session not ready ...") // must surface as errors, not as an empty-but-successful // stack trace (issue #124). - return failureResult(error.message); + const diagnostics = failureDiagnostics(); + return failureResult(error.message, diagnostics ? { diagnostics } : undefined); } // Re-throw unexpected errors throw error; diff --git a/src/server/handlers/session-tools.ts b/src/server/handlers/session-tools.ts index c086f2543..feed89278 100644 --- a/src/server/handlers/session-tools.ts +++ b/src/server/handlers/session-tools.ts @@ -168,6 +168,9 @@ export async function handleListDebugSessions(ctx: ToolContext): Promise 0 + ? diagnosticData + : undefined; const message = error instanceof Error ? error.message : String(error); // A close that landed during the teardown removed the session; the // state write would throw. Report the failure as-is. diff --git a/src/session/launch/debug-launcher.ts b/src/session/launch/debug-launcher.ts index 0581ae48b..5f39afd6a 100644 --- a/src/session/launch/debug-launcher.ts +++ b/src/session/launch/debug-launcher.ts @@ -169,6 +169,7 @@ export class DebugLauncher { session.exitCode = undefined; session.lastProxyExit = undefined; session.lastProxyError = undefined; + session.failureDiagnostics = undefined; this.ctx.logger.info(`[SessionManager] Session ${sessionId} lifecycle state set to ACTIVE`); // Record the launch spec for restart_debugging BEFORE attempting the @@ -289,6 +290,9 @@ export class DebugLauncher { dryRunTimeoutError, 'startDebugging' ); + session.failureDiagnostics = Object.keys(diagnosticData).length > 0 + ? diagnosticData + : undefined; return { success: false, @@ -392,6 +396,9 @@ export class DebugLauncher { new Error(errorMessage), 'startDebugging' ); + finalSession.failureDiagnostics = Object.keys(diagnosticData).length > 0 + ? diagnosticData + : undefined; return { success: false, state: SessionState.ERROR, @@ -466,6 +473,9 @@ export class DebugLauncher { }; } catch (error) { const diagnosticData = await failProxySetup(this.ctx, session, error, 'startDebugging'); + session.failureDiagnostics = Object.keys(diagnosticData).length > 0 + ? diagnosticData + : undefined; const errorMessage = error instanceof Error ? error.message : String(error); diff --git a/src/session/launch/proxy-failure-diagnostics.ts b/src/session/launch/proxy-failure-diagnostics.ts index b15c7ce1c..929418131 100644 --- a/src/session/launch/proxy-failure-diagnostics.ts +++ b/src/session/launch/proxy-failure-diagnostics.ts @@ -16,7 +16,7 @@ * Both paths now call `logProxyFailure`, which is why it lives here rather than * in either one. */ -import { sanitizeStderrTail } from '@debugmcp/shared'; +import { sanitizeStderrTail, type SessionFailureDiagnostics } from '@debugmcp/shared'; import type { ManagedSession } from '../session-store.js'; import type { IFileSystem, ILogger } from '../../interfaces/external-dependencies.js'; import type { ProxyInitProgress } from '../../utils/error-messages.js'; @@ -30,7 +30,7 @@ const PROXY_LOG_TAIL_LINES = 80; export const PROXY_LOG_TAIL_MAX_BYTES = 64 * 1024; /** The pointers a failed launch/attach returns to the caller (issue #493 / #551). */ - export interface ProxyFailureDiagnostics { + export interface ProxyFailureDiagnostics extends SessionFailureDiagnostics { initProgress?: ProxyInitProgress; proxyLogPath?: string; } @@ -45,7 +45,7 @@ export interface ProxyFailureLogDeps { } /** The two operations that can fail this way, named as they appear in the log. */ -export type ProxyFailureOperation = 'startDebugging' | 'attachToProcess'; +export type ProxyFailureOperation = 'startDebugging' | 'attachToProcess' | 'proxyExit'; /** * What `failProxySetup` needs on top of the log deps: the facade's diff --git a/src/session/session-manager-core.ts b/src/session/session-manager-core.ts index aefabcf56..0fa230f26 100644 --- a/src/session/session-manager-core.ts +++ b/src/session/session-manager-core.ts @@ -35,7 +35,11 @@ import { normalizeBreakpointMessage } from '../utils/breakpoint-message.js'; import { consumeChildOrigin } from '../utils/child-origin-events.js'; import { isPidAlive } from '../utils/jvm-orphan-reaper.js'; import { IAdapterRegistry } from '@debugmcp/shared'; -import type { ProxyFailureDiagnostics } from './launch/proxy-failure-diagnostics.js'; +import { + collectProxyFailureDiagnostics, + logProxyFailure, + type ProxyFailureDiagnostics +} from './launch/proxy-failure-diagnostics.js'; import type { AnchorResolution } from './breakpoints/anchor-resolution.js'; // Custom launch arguments interface extending DebugProtocol.LaunchRequestArguments @@ -392,6 +396,35 @@ export abstract class SessionManagerCore extends EventEmitter { this.logger.info('All debug sessions closed'); } + /** + * Persist the public failure pointers synchronously, then write the bounded + * structured log record in the background. EventEmitter does not await + * listeners, so the session projection must be complete before the handler + * returns. A generation guard prevents a slow log read from reviving an old + * launch's diagnostics after restart_debugging installs a new proxy. + */ + private recordProxyFailure(session: ManagedSession, error: unknown): void { + const generation = session.proxyGeneration; + session.failureDiagnostics = collectProxyFailureDiagnostics(session, error); + + void logProxyFailure( + { logger: this.logger, fileSystem: this.fileSystem }, + session, + error, + 'proxyExit' + ).then((diagnostics) => { + if (session.proxyGeneration === generation && session.state === SessionState.ERROR) { + session.failureDiagnostics = diagnostics; + } + }).catch((diagnosticsError: unknown) => { + // logProxyFailure is total by contract; retain the synchronously + // collected pointers even if a future implementation regresses that. + this.logger.warn( + `[SessionManager] Failed to finish proxy-exit diagnostics for session ${session.id}: ${diagnosticsError instanceof Error ? diagnosticsError.message : String(diagnosticsError)}` + ); + }); + } + protected setupProxyEventHandlers( session: ManagedSession, proxyManager: IProxyManager, @@ -406,6 +439,7 @@ export abstract class SessionManagerCore extends EventEmitter { session.lastStop = undefined; session.exitCode = undefined; session.adapterCapabilities = undefined; + session.failureDiagnostics = undefined; // Adapter degradation notes are per-launch (issue #441). session.adapterNotices = []; // Mandatory (issue #217): the relaunch's new proxyManager reports @@ -1068,6 +1102,7 @@ export abstract class SessionManagerCore extends EventEmitter { this.logger.error(`[ProxyManager ${sessionId}] Error:`, error); session.lastProxyError = error.message; this._updateSessionState(session, SessionState.ERROR); + this.recordProxyFailure(session, error); // Clean up listeners since proxy is in error state this.cleanupProxyEventHandlers(session, proxyManager); @@ -1091,6 +1126,7 @@ export abstract class SessionManagerCore extends EventEmitter { this.logger.debug(`[SessionManager] handleExit: session=${sessionId} currentState=${session.state} code=${code} signal=${signal} expected=${expected}`); this.logger.info(`[ProxyManager ${sessionId}] Exit: code=${code}, signal=${signal}, expected=${expected}`); session.lastProxyExit = { code, signal, expected }; + const stateBeforeExit = session.state; if (session.state !== SessionState.STOPPED && session.state !== SessionState.ERROR) { if (expected === true) { // Orderly debuggee termination (issue #258): the worker saw a @@ -1119,6 +1155,15 @@ export abstract class SessionManagerCore extends EventEmitter { } } + if (stateBeforeExit !== SessionState.ERROR && session.state === SessionState.ERROR) { + const exitDescription = `code=${code ?? 'null'}${signal ? `, signal=${signal}` : ''}`; + const exitError = Object.assign( + new Error(`Debug proxy exited unexpectedly (${exitDescription})`), + { code, signal, expected } + ); + this.recordProxyFailure(session, exitError); + } + // Clean up listeners since proxy is gone this.cleanupProxyEventHandlers(session, proxyManager); session.lastProxyPid = proxyManager.getProxyPid?.() ?? session.lastProxyPid; diff --git a/src/session/session-store.ts b/src/session/session-store.ts index d9afcd6ae..ac06b6edb 100644 --- a/src/session/session-store.ts +++ b/src/session/session-store.ts @@ -33,6 +33,7 @@ import type { DebugProtocol } from '@vscode/debugprotocol'; import { IProxyManager } from '../proxy/proxy-manager.js'; import { OutputRingBuffer } from './output-buffer.js'; import type { PauseIntent } from './execution/pause-intent.js'; +import type { ProxyFailureDiagnostics } from './launch/proxy-failure-diagnostics.js'; export interface ToolchainValidationState { compatible: boolean; @@ -106,6 +107,8 @@ export interface ManagedSession extends DebugSessionInfo { lastProxyExit?: ProxyExitState; /** Last proxy error-event message for the current launch. */ lastProxyError?: string; + /** Actionable pointers retained when the current launch's proxy fails. */ + failureDiagnostics?: ProxyFailureDiagnostics; // True once the first 'stopped' event after launch has been observed. // Used by the auto-continue trigger to identify the initial entry stop // even when the adapter reports a non-'entry' reason (e.g., js-debug @@ -273,6 +276,9 @@ export class SessionStore { updatedAt: s.updatedAt, lastStop: s.lastStop, exitCode: s.exitCode, + ...(s.state === SessionState.ERROR && s.failureDiagnostics + ? { diagnostics: s.failureDiagnostics } + : {}), // Mirror endpoint without the token (issue #217); the isRunning gate // keeps the projection honest on teardown paths that skip cleanup. ...(s.exposure && s.proxyManager?.isRunning() diff --git a/tests/core/unit/server/handlers/session-tools.test.ts b/tests/core/unit/server/handlers/session-tools.test.ts index fc44f192d..a16876960 100644 --- a/tests/core/unit/server/handlers/session-tools.test.ts +++ b/tests/core/unit/server/handlers/session-tools.test.ts @@ -32,7 +32,8 @@ describe('session tool handlers', () => { language: 'python', state: 'active', createdAt: now, - updatedAt: now + updatedAt: now, + diagnostics: { proxyLogPath: '/logs/proxy-session-1.log' } }]); const result = await handleListDebugSessions(ctx); @@ -45,6 +46,9 @@ describe('session tool handlers', () => { name: 'Test Session', language: 'python' }); + expect(payload.sessions[0].diagnostics).toEqual({ + proxyLogPath: '/logs/proxy-session-1.log' + }); }); }); }); diff --git a/tests/core/unit/server/server-inspection-tools.test.ts b/tests/core/unit/server/server-inspection-tools.test.ts index a15c3ffed..b23c63fe5 100644 --- a/tests/core/unit/server/server-inspection-tools.test.ts +++ b/tests/core/unit/server/server-inspection-tools.test.ts @@ -201,6 +201,7 @@ describe('Server Inspection Tools Tests', () => { it('echoes the inspected thread and frameless-thread note (issue #553)', async () => { const mockSession = { lastStop: { reason: 'pause', threadId: 1 }, + failureDiagnostics: { proxyLogPath: '/logs/proxy-test-session.log' }, proxyManager: { getCurrentThreadId: vi.fn().mockReturnValue(1), setCurrentThreadId: vi.fn() @@ -229,6 +230,7 @@ describe('Server Inspection Tools Tests', () => { expect(content.threadId).toBe(4); expect(content.lastStop.threadId).toBe(1); expect(content.note).toMatch(/Signal Dispatcher/); + expect(content.diagnostics).toEqual({ proxyLogPath: '/logs/proxy-test-session.log' }); expect(mockSessionManager.getStackTraceDetailed).toHaveBeenCalledWith( 'test-session', 4, false ); @@ -297,7 +299,10 @@ describe('Server Inspection Tools Tests', () => { }); it('should handle missing proxy manager', async () => { - const mockSession = { proxyManager: null }; + const mockSession = { + proxyManager: null, + failureDiagnostics: { proxyLogPath: '/logs/proxy-test-session.log' } + }; mockSessionManager.getSession.mockReturnValue(mockSession); const result = await callToolHandler({ @@ -312,10 +317,12 @@ describe('Server Inspection Tools Tests', () => { const content = JSON.parse(result.content[0].text); expect(content.success).toBe(false); expect(content.error).toContain('no active proxy for session test-session'); + expect(content.diagnostics).toEqual({ proxyLogPath: '/logs/proxy-test-session.log' }); }); it('should handle missing thread ID', async () => { const mockSession = { + failureDiagnostics: { proxyLogPath: '/logs/proxy-test-session.log' }, proxyManager: { getCurrentThreadId: vi.fn().mockReturnValue(null) } @@ -339,6 +346,7 @@ describe('Server Inspection Tools Tests', () => { it('should surface SessionManager errors as a truthful tool-level failure', async () => { const mockSession = { + failureDiagnostics: { proxyLogPath: '/logs/proxy-test-session.log' }, proxyManager: { getCurrentThreadId: vi.fn().mockReturnValue(1) } @@ -360,6 +368,7 @@ describe('Server Inspection Tools Tests', () => { const content = JSON.parse(result.content[0].text); expect(content.success).toBe(false); expect(content.error).toContain('Stack trace failed'); + expect(content.diagnostics).toEqual({ proxyLogPath: '/logs/proxy-test-session.log' }); }); }); diff --git a/tests/core/unit/session/session-manager-exit-mapping.test.ts b/tests/core/unit/session/session-manager-exit-mapping.test.ts index bca7eb93e..a86a3b9b3 100644 --- a/tests/core/unit/session/session-manager-exit-mapping.test.ts +++ b/tests/core/unit/session/session-manager-exit-mapping.test.ts @@ -11,6 +11,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 path from 'node:path'; import { createMockDependencies } from './session-manager-test-utils.js'; describe('SessionManager - proxy exit mapping (issue #258)', () => { @@ -93,6 +94,34 @@ describe('SessionManager - proxy exit mapping (issue #258)', () => { const session = sessionManager.getSession(sessionId); expect(session?.state).toBe(SessionState.ERROR); expect(session?.lastProxyExit).toEqual({ code: 134, signal: undefined, expected: false }); + expect(session?.failureDiagnostics).toEqual({ + proxyLogPath: path.join(session!.logDir!, `proxy-${sessionId}.log`) + }); + expect(sessionManager.getAllSessions().find(({ id }) => id === sessionId)?.diagnostics).toEqual( + session?.failureDiagnostics + ); + + await vi.waitFor(() => { + expect(dependencies.mockLogger.error).toHaveBeenCalledWith( + `[SessionManager] Detailed error in proxyExit for session ${sessionId}:`, + expect.objectContaining({ + message: 'Debug proxy exited unexpectedly (code=134)', + proxyLogPath: session?.failureDiagnostics?.proxyLogPath + }) + ); + }); + }); + + it('persists the same diagnostics for a proxy error event', async () => { + const sessionId = await startRunningSession(); + + dependencies.mockProxyManager.simulateEvent('error', new Error('adapter socket closed')); + + const session = sessionManager.getSession(sessionId); + expect(session?.state).toBe(SessionState.ERROR); + expect(session?.failureDiagnostics?.proxyLogPath).toBe( + path.join(session!.logDir!, `proxy-${sessionId}.log`) + ); }); it('keeps the legacy mapping when expected is absent: clean proxy exit → STOPPED', async () => { From 7685c939bd77b33e90765b1407f2134a32215b24 Mon Sep 17 00:00:00 2001 From: JF Date: Sun, 30 Aug 2026 23:55:41 -0400 Subject: [PATCH 2/2] test: include proxy log resource in result type ratchet --- tests/core/unit/session/debug-result-data.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/core/unit/session/debug-result-data.test.ts b/tests/core/unit/session/debug-result-data.test.ts index 7b76f4e36..1975e9b0e 100644 --- a/tests/core/unit/session/debug-result-data.test.ts +++ b/tests/core/unit/session/debug-result-data.test.ts @@ -6,6 +6,7 @@ describe('DebugResultData type contract (issue #590)', () => { expectTypeOf().toEqualTypeOf< | 'initProgress' | 'proxyLogPath' + | 'proxyLogResource' | 'message' | 'warning' | 'pending'