diff --git a/changelog.d/571.added.md b/changelog.d/571.added.md new file mode 100644 index 00000000..a3156871 --- /dev/null +++ b/changelog.d/571.added.md @@ -0,0 +1 @@ +Expose a bounded, sanitized `debug://sessions/{id}/proxy-log` MCP resource for every session launch attempt and include its URI beside `proxyLogPath` in failure diagnostics. diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 1ce6c937..3ca9f6d6 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -108,7 +108,7 @@ 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. +Errored sessions include optional `diagnostics` with the current launch attempt's server-host `proxyLogPath` and remote-safe `proxyLogResource`. The record is retained for proxy initialization failures and for proxy/adapter deaths after initialization, and is cleared when a new launch or attach attempt begins. --- @@ -989,6 +989,8 @@ Each session also exposes its captured output as an MCP resource: - **`resources/subscribe`** to a session's URI to receive `notifications/resources/updated` pings as output arrives. Pings are coalesced (~150 ms), so notification volume is independent of how fast the debuggee prints — on a ping, re-read the resource or call `get_output` with your cursor. - Subscriptions are tracked per server instance and cleaned up when the session closes. +After a launch or attach creates its run directory, `resources/list` also includes `debug://sessions/{sessionId}/proxy-log`. Reading it returns at most the final 64 KiB, sanitized and trimmed to 80 lines. It is a point-in-time diagnostic resource and is intentionally not subscribable; output remains the only resource that emits update notifications. + --- ## Additional Tools diff --git a/src/server.ts b/src/server.ts index c5efe06e..e75b703a 100644 --- a/src/server.ts +++ b/src/server.ts @@ -651,7 +651,12 @@ export class DebugMcpServer implements ToolContext { // `environment` here instead. Both fields are readonly, so production never // observes the difference — but a test that swaps `sessionManager` after // construction sees the swap in tool handlers only, not in resource handlers. - registerResourceHandlers(this.server, this.sessionManager, this.outputResources); + registerResourceHandlers( + this.server, + this.sessionManager, + this.outputResources, + dependencies.fileSystem + ); registerPromptHandlers(this.server, this.environment); this.sessionManager.on('output-captured', this.outputResources.handleOutputCaptured); this.server.onerror = (error) => { diff --git a/src/server/output-resources.ts b/src/server/output-resources.ts index b44f908f..34922c85 100644 --- a/src/server/output-resources.ts +++ b/src/server/output-resources.ts @@ -1,5 +1,5 @@ /** - * Debuggee-output resources (issue #218). + * Per-session debug resources (issues #218 / #571). * * Each debug session exposes its captured debuggee output as * debug://sessions/{id}/output — a verbatim console transcript (all categories @@ -16,18 +16,23 @@ import { ErrorCode as McpErrorCode, McpError } from '@modelcontextprotocol/sdk/types.js'; -import { ILogger } from '@debugmcp/shared'; +import { ILogger, type IFileSystem } from '@debugmcp/shared'; import type { SessionManager } from '../session/session-manager.js'; - -export function outputResourceUri(sessionId: string): string { - return `debug://sessions/${sessionId}/output`; -} - -/** Returns the sessionId encoded in a debug output resource URI, or undefined. */ -export function parseOutputResourceUri(uri: string): string | undefined { - const match = /^debug:\/\/sessions\/([^/]+)\/output$/.exec(uri); - return match?.[1]; -} +import { proxyLogPathFor } from '../proxy/session-log-layout.js'; +import { readProxyLogTail } from '../session/launch/proxy-failure-diagnostics.js'; +import { + outputResourceUri, + parseOutputResourceUri, + parseProxyLogResourceUri, + proxyLogResourceUri +} from '../session/session-resource-uris.js'; + +export { + outputResourceUri, + parseOutputResourceUri, + parseProxyLogResourceUri, + proxyLogResourceUri +} from '../session/session-resource-uris.js'; /** Debounce window for resources/updated pings. */ export const OUTPUT_UPDATE_DEBOUNCE_MS = 150; @@ -118,33 +123,59 @@ export class OutputResourceNotifier { } /** - * Registers the MCP resource handlers (resources/list, resources/read, - * resources/subscribe, resources/unsubscribe) for the debuggee-output resources. + * Registers the MCP resource handlers. Only debuggee output is subscribable; + * the bounded proxy-log tail is an on-demand diagnostic snapshot. */ export function registerResourceHandlers( server: Server, sessionManager: SessionManager, - notifier: OutputResourceNotifier + notifier: OutputResourceNotifier, + fileSystem: Pick ): void { server.setRequestHandler(ListResourcesRequestSchema, async () => { const sessions = sessionManager.getAllSessions(); return { - resources: sessions.map(session => ({ - uri: outputResourceUri(session.id), - name: `Debuggee output — ${session.name}`, - description: `stdout/stderr/console output captured for ${session.language} debug session '${session.name}'`, - mimeType: 'text/plain' - })) + resources: sessions.flatMap(session => { + const resources = [{ + uri: outputResourceUri(session.id), + name: `Debuggee output — ${session.name}`, + description: `stdout/stderr/console output captured for ${session.language} debug session '${session.name}'`, + mimeType: 'text/plain' + }]; + if (sessionManager.getSession(session.id)?.logDir) { + resources.push({ + uri: proxyLogResourceUri(session.id), + name: `Debug proxy log — ${session.name}`, + description: `Sanitized tail of the debug proxy log for ${session.language} debug session '${session.name}'`, + mimeType: 'text/plain' + }); + } + return resources; + }) }; }); server.setRequestHandler(ReadResourceRequestSchema, async (request) => { const uri = request.params.uri; - const sessionId = parseOutputResourceUri(uri); + const outputSessionId = parseOutputResourceUri(uri); + const proxyLogSessionId = parseProxyLogResourceUri(uri); + const sessionId = outputSessionId ?? proxyLogSessionId; const session = sessionId ? sessionManager.getSession(sessionId) : undefined; if (!session) { throw new McpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`); } + if (proxyLogSessionId) { + if (!session.logDir) { + throw new McpError(McpErrorCode.InvalidParams, `Unknown resource: ${uri}`); + } + const text = await readProxyLogTail( + fileSystem, + proxyLogPathFor(session.logDir, session.id) + ); + return { + contents: [{ uri, mimeType: 'text/plain', text: text ?? '' }] + }; + } return { contents: [{ uri, diff --git a/src/session/launch/proxy-failure-diagnostics.ts b/src/session/launch/proxy-failure-diagnostics.ts index 92941813..626dedff 100644 --- a/src/session/launch/proxy-failure-diagnostics.ts +++ b/src/session/launch/proxy-failure-diagnostics.ts @@ -22,6 +22,7 @@ import type { IFileSystem, ILogger } from '../../interfaces/external-dependencie import type { ProxyInitProgress } from '../../utils/error-messages.js'; import { getErrorMessage, SessionNotFoundError } from '../../errors/debug-errors.js'; import { proxyLogPathFor } from '../../proxy/session-log-layout.js'; +import { proxyLogResourceUri } from '../session-resource-uris.js'; /** How many trailing proxy-log lines are worth reporting after a failure. */ const PROXY_LOG_TAIL_LINES = 80; @@ -71,6 +72,7 @@ export function collectProxyFailureDiagnostics( // it would drop `data` from the tool result entirely. if (session.logDir) { diagnostics.proxyLogPath = proxyLogPathFor(session.logDir, session.id); + diagnostics.proxyLogResource = proxyLogResourceUri(session.id); } // The error, by contrast, is a value that has already misbehaved once — a @@ -148,6 +150,7 @@ export function buildProxyFailureErrorDetails( toString: error?.toString ? error.toString() : 'No toString', initProgress: diagnostics.initProgress, proxyLogPath: diagnostics.proxyLogPath, + proxyLogResource: diagnostics.proxyLogResource, proxyLogTail }; diff --git a/src/session/session-resource-uris.ts b/src/session/session-resource-uris.ts new file mode 100644 index 00000000..ee16b39a --- /dev/null +++ b/src/session/session-resource-uris.ts @@ -0,0 +1,21 @@ +/** Pure URI composition/parsing shared by session diagnostics and MCP resources. */ + +export function outputResourceUri(sessionId: string): string { + return `debug://sessions/${sessionId}/output`; +} + +export function proxyLogResourceUri(sessionId: string): string { + return `debug://sessions/${sessionId}/proxy-log`; +} + +/** Returns the sessionId encoded in a debug output resource URI, or undefined. */ +export function parseOutputResourceUri(uri: string): string | undefined { + const match = /^debug:\/\/sessions\/([^/]+)\/output$/.exec(uri); + return match?.[1]; +} + +/** Returns the sessionId encoded in a debug proxy-log resource URI, or undefined. */ +export function parseProxyLogResourceUri(uri: string): string | undefined { + const match = /^debug:\/\/sessions\/([^/]+)\/proxy-log$/.exec(uri); + return match?.[1]; +} diff --git a/tests/core/unit/server/server-resources.test.ts b/tests/core/unit/server/server-resources.test.ts index 8920c366..8bc5455a 100644 --- a/tests/core/unit/server/server-resources.test.ts +++ b/tests/core/unit/server/server-resources.test.ts @@ -4,6 +4,7 @@ * and debounced resources/updated pings driven by 'output-captured'. */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import path from 'node:path'; import { Server } from '@modelcontextprotocol/sdk/server/index.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { McpError } from '@modelcontextprotocol/sdk/types.js'; @@ -95,6 +96,27 @@ describe('Server Output Resources Tests', () => { }); expect(result.resources[0].name).toContain('alpha'); }); + + it('lists a proxy-log resource only after a launch creates a run directory', async () => { + mockSessionManager.getAllSessions.mockReturnValue([ + { id: 'sess-1', name: 'not launched', language: 'python' }, + { id: 'sess-2', name: 'launched', language: 'mock' } + ]); + mockSessionManager.getSession.mockImplementation((sessionId: string) => + sessionId === 'sess-2' + ? mockSession({ id: 'sess-2', logDir: path.join('/logs', 'sess-2', 'run-123') }) + : mockSession({ id: 'sess-1', logDir: undefined }) + ); + + const { listResourcesHandler } = getResourceHandlers(mockServer); + const result = await listResourcesHandler({ method: 'resources/list', params: {} }); + + expect(result.resources.map((resource: { uri: string }) => resource.uri)).toEqual([ + 'debug://sessions/sess-1/output', + 'debug://sessions/sess-2/output', + 'debug://sessions/sess-2/proxy-log' + ]); + }); }); describe('resources/read', () => { @@ -129,6 +151,42 @@ describe('Server Output Resources Tests', () => { expect(result.contents[0].text).toBe(''); }); + it('routes proxy-log reads through the bounded sanitizer', async () => { + const logDir = path.join('/logs', 'sess-1', 'run-123'); + mockSessionManager.getSession.mockReturnValue(mockSession({ logDir })); + const lines = Array.from({ length: 100 }, (_, index) => `line ${index + 1}`); + lines[98] = '[Worker] argv: --token=super-secret-value'; + mockDependencies.fileSystem.readTail.mockResolvedValue(lines.join('\n')); + + const { readResourceHandler } = getResourceHandlers(mockServer); + const result = await readResourceHandler({ + method: 'resources/read', + params: { uri: 'debug://sessions/sess-1/proxy-log' } + }); + + expect(mockDependencies.fileSystem.readTail).toHaveBeenCalledWith( + path.join(logDir, 'proxy-sess-1.log'), + 64 * 1024 + ); + expect(result.contents[0]).toMatchObject({ + uri: 'debug://sessions/sess-1/proxy-log', + mimeType: 'text/plain' + }); + expect(result.contents[0].text).toContain('line 100'); + expect(result.contents[0].text).toContain('[REDACTED'); + expect(result.contents[0].text).not.toContain('super-secret-value'); + }); + + it('rejects a proxy-log URI before the session has a run directory', async () => { + mockSessionManager.getSession.mockReturnValue(mockSession({ logDir: undefined })); + const { readResourceHandler } = getResourceHandlers(mockServer); + + await expect(readResourceHandler({ + method: 'resources/read', + params: { uri: 'debug://sessions/sess-1/proxy-log' } + })).rejects.toBeInstanceOf(McpError); + }); + it('rejects unknown URIs and unknown sessions', async () => { mockSessionManager.getSession.mockReturnValue(undefined); const { readResourceHandler } = getResourceHandlers(mockServer); @@ -180,6 +238,14 @@ describe('Server Output Resources Tests', () => { await expect(subscribe('debug://sessions/ghost/output')).rejects.toBeInstanceOf(McpError); }); + it('keeps subscriptions output-resource-only', async () => { + mockSessionManager.getSession.mockReturnValue( + mockSession({ logDir: path.join('/logs', 'sess-1', 'run-123') }) + ); + + await expect(subscribe('debug://sessions/sess-1/proxy-log')).rejects.toBeInstanceOf(McpError); + }); + it('stops pinging after unsubscribe, cancelling any pending timer', async () => { mockSessionManager.getSession.mockReturnValue(mockSession()); await subscribe('debug://sessions/sess-1/output'); diff --git a/tests/core/unit/server/server-test-helpers.ts b/tests/core/unit/server/server-test-helpers.ts index a7bc6b4c..2a1924e0 100644 --- a/tests/core/unit/server/server-test-helpers.ts +++ b/tests/core/unit/server/server-test-helpers.ts @@ -29,6 +29,7 @@ export function createMockDependencies() { ensureDir: vi.fn().mockResolvedValue(undefined), pathExists: vi.fn().mockResolvedValue(true), readFile: vi.fn().mockResolvedValue('{}'), + readTail: vi.fn().mockResolvedValue(''), writeFile: vi.fn().mockResolvedValue(undefined), exists: vi.fn().mockResolvedValue(true), mkdir: vi.fn().mockResolvedValue(undefined), diff --git a/tests/core/unit/session/launch/proxy-failure-diagnostics.test.ts b/tests/core/unit/session/launch/proxy-failure-diagnostics.test.ts index 662e27aa..240983ec 100644 --- a/tests/core/unit/session/launch/proxy-failure-diagnostics.test.ts +++ b/tests/core/unit/session/launch/proxy-failure-diagnostics.test.ts @@ -23,6 +23,7 @@ const initProgress: ProxyInitProgress = { transportConnected: true, pendingComma const runDir = path.join('/tmp', 'logs', 'sess-1', 'run-1'); const proxyLogPath = path.join(runDir, 'proxy-sess-1.log'); +const proxyLogResource = 'debug://sessions/sess-1/proxy-log'; /** A winston log file: every line newline-TERMINATED, so the text ends in a newline. */ function logFile(lines: string[]): string { @@ -39,14 +40,15 @@ describe('collectProxyFailureDiagnostics', () => { expect(collectProxyFailureDiagnostics({ id: 'sess-1', logDir: runDir }, error)).toEqual({ initProgress, - proxyLogPath + proxyLogPath, + proxyLogResource }); }); it('still points at the proxy log when the error carries no init progress', () => { expect( collectProxyFailureDiagnostics({ id: 'sess-1', logDir: runDir }, new Error('adapter exited')) - ).toEqual({ proxyLogPath }); + ).toEqual({ proxyLogPath, proxyLogResource }); }); it('keeps the session-derived pointer when the error refuses to be read', () => { @@ -60,7 +62,8 @@ describe('collectProxyFailureDiagnostics', () => { }; expect(collectProxyFailureDiagnostics({ id: 'sess-1', logDir: runDir }, hostile)).toEqual({ - proxyLogPath + proxyLogPath, + proxyLogResource }); }); @@ -241,7 +244,7 @@ describe('logProxyFailure', () => { ); // The pointers still reach the caller, and the log still names the failure. - expect(diagnostics).toEqual({ proxyLogPath }); + expect(diagnostics).toEqual({ proxyLogPath, proxyLogResource }); expect(logger.error).toHaveBeenCalledWith( '[SessionManager] Detailed error in attachToProcess for session sess-1:', expect.objectContaining({ @@ -270,7 +273,7 @@ describe('logProxyFailure', () => { // The hostile field costs itself and nothing else: the record is the full // one, not the degraded fallback. - expect(diagnostics).toEqual({ proxyLogPath }); + expect(diagnostics).toEqual({ proxyLogPath, proxyLogResource }); expect(logger.error).toHaveBeenCalledWith( expect.stringContaining('Detailed error in attachToProcess'), expect.objectContaining({ @@ -295,7 +298,7 @@ describe('logProxyFailure', () => { new Error('attach failed'), 'attachToProcess' ) - ).resolves.toEqual({ proxyLogPath }); + ).resolves.toEqual({ proxyLogPath, proxyLogResource }); }); it('logs the proxy log tail but returns only the pointers', async () => { @@ -311,7 +314,7 @@ describe('logProxyFailure', () => { 'attachToProcess' ); - expect(diagnostics).toEqual({ initProgress, proxyLogPath }); + expect(diagnostics).toEqual({ initProgress, proxyLogPath, proxyLogResource }); expect(logger.error).toHaveBeenCalledWith( expect.any(String), expect.objectContaining({ proxyLogTail: 'adapter said: could not open port' }) diff --git a/tests/core/unit/session/session-manager-attach-diagnostics.test.ts b/tests/core/unit/session/session-manager-attach-diagnostics.test.ts index 0d33f83d..13482793 100644 --- a/tests/core/unit/session/session-manager-attach-diagnostics.test.ts +++ b/tests/core/unit/session/session-manager-attach-diagnostics.test.ts @@ -76,7 +76,8 @@ describe('SessionManager - attach failure diagnostics (issue #561)', () => { expect(result.data).toEqual({ initProgress, - proxyLogPath: expect.stringContaining('proxy-') + proxyLogPath: expect.stringContaining('proxy-'), + proxyLogResource: expect.stringMatching(/^debug:\/\/sessions\/.+\/proxy-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 a86a3b9b..e4758d25 100644 --- a/tests/core/unit/session/session-manager-exit-mapping.test.ts +++ b/tests/core/unit/session/session-manager-exit-mapping.test.ts @@ -95,7 +95,8 @@ describe('SessionManager - proxy exit mapping (issue #258)', () => { 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`) + proxyLogPath: path.join(session!.logDir!, `proxy-${sessionId}.log`), + proxyLogResource: `debug://sessions/${sessionId}/proxy-log` }); expect(sessionManager.getAllSessions().find(({ id }) => id === sessionId)?.diagnostics).toEqual( session?.failureDiagnostics diff --git a/tests/unit/session-manager-operations-coverage.test.ts b/tests/unit/session-manager-operations-coverage.test.ts index 66626c8c..aa991c58 100644 --- a/tests/unit/session-manager-operations-coverage.test.ts +++ b/tests/unit/session-manager-operations-coverage.test.ts @@ -993,7 +993,8 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () // itself tears nothing down: the one stop() is the pre-launch teardown // of the proxy this session already had. expect(result!.data).toEqual({ - proxyLogPath: path.join('/tmp/session-logs', 'proxy-test-session.log') + proxyLogPath: path.join('/tmp/session-logs', 'proxy-test-session.log'), + proxyLogResource: 'debug://sessions/test-session/proxy-log' }); expect(mockLogger.error).toHaveBeenCalledWith( expect.stringContaining('Detailed error in startDebugging'), @@ -1266,7 +1267,8 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () error: expect.stringContaining('code=134') })); expect(result.data).toEqual({ - proxyLogPath: path.join('/tmp/session-logs', 'proxy-test-session.log') + proxyLogPath: path.join('/tmp/session-logs', 'proxy-test-session.log'), + proxyLogResource: 'debug://sessions/test-session/proxy-log' }); } finally { startProxySpy.mockRestore(); @@ -2058,7 +2060,8 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () expect(result.success).toBe(false); expect(result.data).toEqual({ initProgress, - proxyLogPath: path.join('/tmp', 'logs', 'test-session', 'run-123', 'proxy-test-session.log') + proxyLogPath: path.join('/tmp', 'logs', 'test-session', 'run-123', 'proxy-test-session.log'), + proxyLogResource: 'debug://sessions/test-session/proxy-log' }); expect(stopSpy).toHaveBeenCalled(); }); @@ -2074,7 +2077,8 @@ describe('Session Manager Operations Coverage - Error Paths and Edge Cases', () }); expect(result.data).toEqual({ - proxyLogPath: path.join('/tmp', 'logs', 'test-session', 'run-456', 'proxy-test-session.log') + proxyLogPath: path.join('/tmp', 'logs', 'test-session', 'run-456', 'proxy-test-session.log'), + proxyLogResource: 'debug://sessions/test-session/proxy-log' }); });