From efd0f2daca659b67730237e19242154a3084e089 Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 4 Aug 2026 15:37:54 -0400 Subject: [PATCH 1/2] fix: capture Go debuggee output via Delve outputMode:remote; emit CodeLLDB's canonical terminal attribute (#225, #223) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go (#225): the launch transform now sets outputMode: 'remote' (user override wins), so Delve forwards the target's stdout/stderr as DAP output events instead of writing them to dlv's own stdio where they never reached get_output. Verified against dlv 1.26.3: the exec-mode e2e smoke test now asserts 'Hello, World!' arrives with category stdout. Rust (#223): the launch transform emitted a debugpy-style 'console' key; CodeLLDB deserializes that only as a legacy alias of its real 'terminal' attribute. Emit terminal: 'console' (translating legacy console values, explicit terminal wins). Investigation with a DAP trace showed the key rename does not by itself fix output capture on Windows: CodeLLDB's TerminalKind::Console performs no stdio redirection (launch.rs:460), so the debuggee inherits the adapter process's pipes — same topology as Ruby (#222). That gap is fixed by the adapter-stdio forwarding follow-up; docs updated to scope the known issue to Windows. Also: comprehensive-mcp-tools gains an outputMarker per-language check (enabled for go), and the go/rust skill references reflect the new state. Fixes #225 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 2 + docs/tool-reference.md | 2 +- packages/adapter-go/src/go-debug-adapter.ts | 8 +++- .../unit/transform-launch-config.test.ts | 17 +++++++++ .../adapter-rust/src/rust-debug-adapter.ts | 26 +++++++++++-- .../adapter-rust/tests/rust-adapter.test.ts | 37 ++++++++++++++++++- skills/debugging/references/go.md | 3 +- skills/debugging/references/rust.md | 4 +- .../go/integration/go-session-smoke.test.ts | 2 + .../integration/rust-session-smoke.test.ts | 5 ++- tests/e2e/comprehensive-mcp-tools.test.ts | 24 +++++++++--- tests/e2e/mcp-server-smoke-go.test.ts | 14 ++++++- 12 files changed, 126 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 141b0a0d..2550554a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Debuggee exit code surfaced** — the exit code from the DAP `exited` event is stored on the session and returned as `exitCode` in `list_debug_sessions`, making a crash (non-zero) distinguishable from a clean exit (#220) ### Fixed +- **Go debuggee output captured** — the Go adapter now launches Delve with `outputMode: 'remote'`, so the target program's stdout/stderr arrives as DAP output events and shows up in `get_output` instead of vanishing into dlv's own stdio (fixes #225) +- **Rust launch config uses CodeLLDB's canonical `terminal` attribute** — the adapter emitted a debugpy-style `console` key that CodeLLDB only honors as a legacy alias; it now emits `terminal: 'console'`, translating any user-supplied legacy `console` values (#223 — the Windows output-capture gap the issue uncovered is tracked separately: LLDB's console mode inherits the debuggee's stdio rather than emitting DAP output events) - **js-debug launch no longer hangs on fast-exiting scripts** — the launch barrier now settles when the debuggee emits `terminated`/`exited` during the launch window, and `dispose()` rejects a still-pending wait as a structural backstop; `start_debugging` for a JavaScript script that crashes (or completes) within seconds of launch now returns promptly with state `stopped` instead of hanging past the MCP client timeout. Also removes an intermittent ~10s stall when js-debug's `initialized` event raced the handshake listener (fixes #242) - Mock adapter now answers `setExceptionBreakpoints` (previously an unhandled-command error) and emits `exited` before `terminated`, matching real adapter ordering (#220) - Corrected the JavaScript adapter's declared `exceptionBreakpointFilters` to the IDs js-debug actually reports (`all`, `uncaught`) (#220) diff --git a/docs/tool-reference.md b/docs/tool-reference.md index 947284d7..a2b65505 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -729,7 +729,7 @@ Gets the debuggee's output (stdout/stderr/console) captured for a session. Outpu - Works while the program is running and after it finishes — output stays readable until `close_debug_session`. Re-launching a session starts a fresh buffer (seq restarts at 1). - `hasMore: true` means more entries matched than `limit` allowed; call again with `since: nextSince`. - Incremental polling recipe: call once, remember `nextSince`, and pass it as `since` on the next call — you'll only ever see new output. -- Adapter support: Python (`redirectOutput`), JavaScript (`outputCapture: 'std'`), and Java forward debuggee stdio as output events; Go and .NET typically do as well. Ruby currently routes debuggee stdio to the adapter process, so no entries are captured (tracked upstream). +- Adapter support: Python (`redirectOutput`), JavaScript (`outputCapture: 'std'`), Go (`outputMode: 'remote'`), and Java forward debuggee stdio as output events; .NET typically does as well. Ruby routes debuggee stdio to the adapter process, so no entries are captured (#222); Rust does the same on Windows, where LLDB's console mode inherits stdio instead of emitting output events (#223). #### Output resources & subscriptions diff --git a/packages/adapter-go/src/go-debug-adapter.ts b/packages/adapter-go/src/go-debug-adapter.ts index 455a0240..30494304 100644 --- a/packages/adapter-go/src/go-debug-adapter.ts +++ b/packages/adapter-go/src/go-debug-adapter.ts @@ -61,6 +61,7 @@ interface GoLaunchConfig extends LanguageSpecificLaunchConfig { hideSystemGoroutines?: boolean; goroutineFilters?: string[]; substitutePath?: Array<{ from: string; to: string }>; + outputMode?: 'local' | 'remote'; [key: string]: unknown; } @@ -345,7 +346,12 @@ export class GoDebugAdapter extends EventEmitter implements IDebugAdapter { goConfig.stackTraceDepth = 50; goConfig.showGlobalVariables = false; goConfig.hideSystemGoroutines = true; - + + // Delve's default outputMode 'local' writes the target's stdout/stderr to + // the dlv process's own stdio, which never reaches get_output; 'remote' + // forwards it as DAP output events (issue #225). + goConfig.outputMode = (rawConfig.outputMode as GoLaunchConfig['outputMode']) ?? 'remote'; + return goConfig; } diff --git a/packages/adapter-go/tests/unit/transform-launch-config.test.ts b/packages/adapter-go/tests/unit/transform-launch-config.test.ts index 0b638081..120a3e5d 100644 --- a/packages/adapter-go/tests/unit/transform-launch-config.test.ts +++ b/packages/adapter-go/tests/unit/transform-launch-config.test.ts @@ -112,4 +112,21 @@ describe('GoDebugAdapter.transformLaunchConfig — mode inference', () => { expect(cfg.stopOnEntry).toBe(false); }); + + it('defaults outputMode to "remote" so target stdio arrives as DAP output events (issue #225)', async () => { + const cfg = await adapter.transformLaunchConfig({ + program: '/proj/main.go' + } as unknown as GenericLaunchConfig); + + expect(cfg.outputMode).toBe('remote'); + }); + + it('preserves a user-supplied outputMode override', async () => { + const cfg = await adapter.transformLaunchConfig({ + program: '/proj/main.go', + outputMode: 'local' + } as unknown as GenericLaunchConfig); + + expect(cfg.outputMode).toBe('local'); + }); }); diff --git a/packages/adapter-rust/src/rust-debug-adapter.ts b/packages/adapter-rust/src/rust-debug-adapter.ts index a33f6bf1..71d8bc51 100644 --- a/packages/adapter-rust/src/rust-debug-adapter.ts +++ b/packages/adapter-rust/src/rust-debug-adapter.ts @@ -86,10 +86,30 @@ interface RustLaunchConfig extends LanguageSpecificLaunchConfig { initCommands?: string[]; // LLDB commands to run on init preRunCommands?: string[]; // LLDB commands before running postRunCommands?: string[]; // LLDB commands after running - console?: 'internalConsole' | 'integratedTerminal' | 'externalTerminal'; + terminal?: 'console' | 'integrated' | 'external'; // CodeLLDB's canonical attribute + console?: 'internalConsole' | 'integratedTerminal' | 'externalTerminal'; // legacy alias, accepted as input [key: string]: unknown; // Required by LanguageSpecificLaunchConfig } +/** + * CodeLLDB's launch schema takes `terminal`; `console` is a legacy alias + * carried over from the debugpy/js-debug convention (issue #223). Accept + * either as input but always emit the canonical `terminal` key. + */ +function resolveTerminalKind(rustConfig: RustLaunchConfig): NonNullable { + if (rustConfig.terminal) { + return rustConfig.terminal; + } + switch (rustConfig.console) { + case 'integratedTerminal': return 'integrated'; + case 'externalTerminal': return 'external'; + case 'internalConsole': + default: + // 'console' captures the debuggee's stdio as DAP output events + return 'console'; + } +} + /** * Rust Debug Adapter implementation */ @@ -803,8 +823,8 @@ export class RustDebugAdapter extends EventEmitter implements IDebugAdapter { // Critical: Enable Rust language support for proper pretty-printing sourceLanguages: ['rust'], - // Console configuration - console: rustConfig.console || 'internalConsole', + // Console configuration — CodeLLDB's key is `terminal`, not `console` + terminal: resolveTerminalKind(rustConfig), // Source mapping for debugging std library (optional) sourceMap: rustConfig.sourceMap || {}, diff --git a/packages/adapter-rust/tests/rust-adapter.test.ts b/packages/adapter-rust/tests/rust-adapter.test.ts index bde37086..24f9167c 100644 --- a/packages/adapter-rust/tests/rust-adapter.test.ts +++ b/packages/adapter-rust/tests/rust-adapter.test.ts @@ -227,9 +227,44 @@ describe('RustDebugAdapter', () => { const config = { args: ['--verbose'] }; - + await expect(adapter.transformLaunchConfig(config)).rejects.toThrow('No program specified'); }); + + it('should default to terminal "console" so debuggee stdio arrives as DAP output events (issue #223)', async () => { + const transformed = await adapter.transformLaunchConfig({ + program: './target/debug/myapp' + }); + + expect(transformed.terminal).toBe('console'); + expect(transformed.console).toBeUndefined(); + }); + + it('should translate legacy console values to CodeLLDB terminal values', async () => { + const cases: Array<[string, string]> = [ + ['internalConsole', 'console'], + ['integratedTerminal', 'integrated'], + ['externalTerminal', 'external'] + ]; + for (const [legacy, expected] of cases) { + const transformed = await adapter.transformLaunchConfig({ + program: './target/debug/myapp', + console: legacy + }); + expect(transformed.terminal).toBe(expected); + expect(transformed.console).toBeUndefined(); + } + }); + + it('should let an explicit terminal value win over a legacy console value', async () => { + const transformed = await adapter.transformLaunchConfig({ + program: './target/debug/myapp', + terminal: 'integrated', + console: 'externalTerminal' + }); + + expect(transformed.terminal).toBe('integrated'); + }); }); describe('Connection Management', () => { diff --git a/skills/debugging/references/go.md b/skills/debugging/references/go.md index c246a990..17519922 100644 --- a/skills/debugging/references/go.md +++ b/skills/debugging/references/go.md @@ -46,7 +46,7 @@ Not supported. The Go adapter implements launch mode only — `attach_to_process ## Quirks -- **KNOWN ISSUE — debuggee stdout not forwarded (issue #225):** `get_output` does not capture the Go program's stdout. Do not debug by adding `fmt.Println` calls — set breakpoints and inspect state with `evaluate_expression`, `get_local_variables`, and `get_variables` instead. +- **Debuggee output is captured:** the adapter launches with Delve's `outputMode: 'remote'`, so the program's stdout/stderr arrives as `get_output` entries (categories `stdout`/`stderr`). - **`stopOnEntry` is forced to `false`** by the Go adapter policy (unless you explicitly set it) to dodge Delve's "unknown goroutine 1" quirk. If you force `stopOnEntry: true` and see that error, it is harmless — execution continues. Set a breakpoint on the first line of `main` if you need an entry stop. - Goroutine-aware, with limits: stack traces show the current goroutine's frames; Go runtime and testing frames (paths with `/runtime/` or `/testing/`) are filtered out by default — pass `includeInternals: true` to `get_stack_trace` to see them. There are no MCP tools to list or switch goroutines. - Exception breakpoints `panic` and `fatal` are enabled by default — panics stop the debugger without any setup (`get_stack_trace` reports `stopReason`). @@ -61,5 +61,4 @@ Not supported. The Go adapter implements launch mode only — `attach_to_process | "Go executable not found" | `go` not on PATH | Install Go 1.18+; verify `go version` | | Breakpoints not hit | Optimized binary (exec mode) or wrong path/line | Rebuild with `-gcflags="all=-N -l"`; absolute paths; line must be an executable statement | | "unknown goroutine 1" error | `stopOnEntry: true` with Delve | Leave `stopOnEntry` unset/false; the error is harmless if it appears | -| `get_output` empty despite prints | Issue #225 — Go stdout not forwarded | Inspect state via `evaluate_expression` / breakpoints, not stdout | | Stack full of runtime frames | `includeInternals: true` set, or panic inside runtime | Omit `includeInternals` for user frames only | diff --git a/skills/debugging/references/rust.md b/skills/debugging/references/rust.md index e95ab16f..8efb0e17 100644 --- a/skills/debugging/references/rust.md +++ b/skills/debugging/references/rust.md @@ -53,7 +53,7 @@ Not supported. The Rust adapter implements launch mode only — `attach_to_proce - **Windows toolchain (critical):** MSVC-built binaries give control flow only — breakpoints/stepping work, but strings, Vecs, and structs show `` or corrupted values. `RUST_MSVC_BEHAVIOR` controls what happens when an MSVC binary is detected: `warn` (default — log and proceed), `error` (fail with `ENVIRONMENT_INVALID`), `continue` (silent). Check any binary first with `mcp-debugger check-rust-binary target/debug/app.exe` — it reports `Toolchain: GNU` or `MSVC`. - **Windows initial stop:** debugging may first stop in system functions (not user code). Issue one `continue_execution` to reach your breakpoint; auto-continue through these system stops is not yet implemented. -- **KNOWN ISSUE — `get_output` may be empty (issue #223):** debuggee stdout may not appear in `get_output` due to a launch-config console/terminal mismatch. Do not rely on print debugging — inspect state with `evaluate_expression`, `get_local_variables`, and breakpoints instead. +- **KNOWN ISSUE — `get_output` may be empty on Windows (issue #223):** in CodeLLDB's console mode the debuggee inherits the adapter process's stdio, and on Windows LLDB does not convert it to DAP output events. Do not rely on print debugging there — inspect state with `evaluate_expression`, `get_local_variables`, and breakpoints instead. - Expression evaluation goes through LLDB: simple field access and method calls like `my_vec.len()` work, but Rust-specific syntax (closures, trait methods) may not. - Debug builds only: release builds need `debug = true` in `[profile.release]` and still inline/optimize away variables. Prefer `opt-level = 0`. - GNU builds of crates that import Windows DLLs (`tokio`, `windows-sys`, `parking_lot_core`, ...) need full MinGW binutils — rustup's self-contained toolchain lacks `as.exe`, so `dlltool` fails. Install via MSYS2 (`mingw-w64-x86_64-binutils`, `-gcc`) and prepend `C:\msys64\mingw64\bin` to PATH. @@ -68,4 +68,4 @@ Not supported. The Rust adapter implements launch mode only — `attach_to_proce | "Can't find CodeLLDB" | Adapter not vendored / npx package on non-Linux | Run `pnpm --filter @debugmcp/adapter-rust run build:adapter`, or set `CODELLDB_PATH` | | First stop is in system/ntdll frames | Windows initial system breakpoint | `continue_execution` once, then you land on your breakpoint | | `dlltool ... CreateProcess` build error | rustup GNU toolchain missing `as.exe` | Install MSYS2 mingw-w64 binutils/gcc; prepend `C:\msys64\mingw64\bin` to PATH | -| `get_output` returns no stdout | Issue #223 (console/terminal launch mismatch) | Use `evaluate_expression` / variables at breakpoints instead | +| `get_output` returns no stdout (Windows) | Issue #223 — debuggee stdio inherited by adapter process | Use `evaluate_expression` / variables at breakpoints instead | diff --git a/tests/adapters/go/integration/go-session-smoke.test.ts b/tests/adapters/go/integration/go-session-smoke.test.ts index d44c202f..cb54b370 100644 --- a/tests/adapters/go/integration/go-session-smoke.test.ts +++ b/tests/adapters/go/integration/go-session-smoke.test.ts @@ -98,6 +98,8 @@ describe('Go adapter - session smoke (integration)', () => { expect(transformed.program).toBe(path.join(projectRoot, 'main.go')); expect(transformed.cwd).toBe(projectRoot); expect(transformed.args).toEqual(['--sample']); + // Route the target's stdio through DAP output events (issue #225) + expect(transformed.outputMode).toBe('remote'); }); it('handles test mode configuration', async () => { diff --git a/tests/adapters/rust/integration/rust-session-smoke.test.ts b/tests/adapters/rust/integration/rust-session-smoke.test.ts index 8247b932..a3475dd0 100644 --- a/tests/adapters/rust/integration/rust-session-smoke.test.ts +++ b/tests/adapters/rust/integration/rust-session-smoke.test.ts @@ -116,6 +116,9 @@ describe('Rust adapter - session smoke (integration)', () => { expect(transformed.cwd).toBe(projectRoot); expect(transformed.args).toEqual(['--sample']); expect(transformed.sourceLanguages).toEqual(['rust']); - expect(transformed.console).toBe('internalConsole'); + // CodeLLDB's schema key is `terminal`; 'console' captures the debuggee's + // stdio as DAP output events (issue #223). + expect(transformed.terminal).toBe('console'); + expect(transformed.console).toBeUndefined(); }); }); diff --git a/tests/e2e/comprehensive-mcp-tools.test.ts b/tests/e2e/comprehensive-mcp-tools.test.ts index e35ab78a..dfdf9db7 100644 --- a/tests/e2e/comprehensive-mcp-tools.test.ts +++ b/tests/e2e/comprehensive-mcp-tools.test.ts @@ -129,6 +129,7 @@ interface LangDef { available: boolean; skipReason?: string; dapLaunchArgs?: Record; // language-specific DAP launch args + outputMarker?: string; // text the script prints — get_output must capture it (issues #223/#225) } const LANGUAGES: LangDef[] = [ @@ -138,7 +139,7 @@ const LANGUAGES: LangDef[] = [ { language: 'rust', script: RUST_SCRIPT, bpLine: RUST_BP_LINE, available: hasRust, skipReason: hasRust ? undefined : 'Rust toolchain not installed' }, { language: 'ruby', script: RUBY_SCRIPT, bpLine: RUBY_BP_LINE, available: hasRuby, skipReason: hasRuby ? undefined : 'Ruby/rdbg not installed' }, { language: 'go', script: GO_SCRIPT, bpLine: GO_BP_LINE, available: hasGo, skipReason: hasGo ? undefined : 'Go/Delve not installed', - dapLaunchArgs: { mode: 'exec' } }, // launchScript set in beforeAll after build + dapLaunchArgs: { mode: 'exec' }, outputMarker: 'Hello, World!' }, // launchScript set in beforeAll after build { language: 'dotnet', script: DOTNET_SCRIPT, bpLine: DOTNET_BP_LINE, available: hasDotnet, skipReason: hasDotnet ? undefined : '.NET/netcoredbg not installed', dapLaunchArgs: { justMyCode: true } }, // launchScript set in beforeAll after build { language: 'java', script: JAVA_SCRIPT, bpLine: JAVA_BP_LINE, available: hasJava, skipReason: hasJava ? undefined : 'JDK not installed', @@ -592,15 +593,26 @@ describe(`Comprehensive MCP Debugger Test — 21 Tools × ${LANGUAGES.length} La } /* ---- Tool 16: get_output (issue #218) ---- */ - // Lenient: entries may legitimately be empty (Ruby routes debuggee - // stdio to the adapter process; the adapter may emit no DAP output - // events) — only the tool contract is asserted per-language. + // Languages with an outputMarker must capture the script's own + // output (issues #223/#225). Others stay lenient: entries may + // legitimately be empty (e.g. Ruby routes debuggee stdio to the + // adapter process) — only the tool contract is asserted. t0 = Date.now(); try { const outRes = await callToolSafely(mcpClient!, 'get_output', { sessionId: currentSessionId }); if (outRes.success === true) { - const count = Array.isArray(outRes.entries) ? outRes.entries.length : 0; - record('get_output', lang.language, 'PASS', `entries=${count}`, Date.now() - t0); + const entries = (Array.isArray(outRes.entries) ? outRes.entries : []) as Array<{ category: string; output: string }>; + const markerEntry = lang.outputMarker + ? entries.find(e => e.output.includes(lang.outputMarker!)) + : undefined; + if (lang.outputMarker && !markerEntry) { + record('get_output', lang.language, 'FAIL', + `marker "${lang.outputMarker}" not captured (entries=${entries.length})`, Date.now() - t0); + } else { + record('get_output', lang.language, 'PASS', + `entries=${entries.length}${markerEntry ? `, marker category=${markerEntry.category}` : ''}`, + Date.now() - t0); + } } else { record('get_output', lang.language, 'FAIL', `success=${outRes.success}: ${outRes.error ?? outRes.message ?? ''}`, Date.now() - t0); } diff --git a/tests/e2e/mcp-server-smoke-go.test.ts b/tests/e2e/mcp-server-smoke-go.test.ts index e5ca1c5d..e0bfafec 100644 --- a/tests/e2e/mcp-server-smoke-go.test.ts +++ b/tests/e2e/mcp-server-smoke-go.test.ts @@ -248,10 +248,22 @@ describe('MCP Server Go Debugging Smoke Test @requires-go', () => { // 5. Continue execution console.log('[Go Smoke Test] Continuing execution...'); await callToolSafely(mcpClient!, 'continue_execution', { sessionId }); - + // Wait for script to complete await new Promise(resolve => setTimeout(resolve, 1000)); + // 6. Debuggee output must be retrievable (issue #225) — outputMode + // 'remote' makes Delve forward the target's stdio as DAP output events + // instead of writing it to dlv's own stdout. + console.log('[Go Smoke Test] Fetching debuggee output...'); + const outputResult = await callToolSafely(mcpClient!, 'get_output', { sessionId }); + expect(outputResult.success).toBe(true); + const outputEntries = outputResult.entries as Array<{ category: string; output: string }>; + console.log(`[Go Smoke Test] Captured ${outputEntries.length} output entries`); + const helloEntry = outputEntries.find(e => e.output.includes('Hello, World!')); + expect(helloEntry).toBeDefined(); + expect(helloEntry!.category).toBe('stdout'); + } finally { // Clean up test binary try { From 466b36486b049bade162325c3475d003d0c7f90d Mon Sep 17 00:00:00 2001 From: JF Date: Tue, 4 Aug 2026 15:58:52 -0400 Subject: [PATCH 2/2] feat: forward adapter-process stdio as debuggee output events (fixes #222, fixes #223) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some adapters hand the debuggee their own stdio instead of converting it to DAP output events: rdbg -c runs the target as a child of the adapter with inherited pipes on every platform, and CodeLLDB's console mode performs no stdio redirection at all — on Windows (where LLDB cannot intercept via its own pipes) the target inherits the adapter's handles. In both cases the program's output only ever reached the proxy log, never get_output. Mechanism (mirrors the #247 worker-side synthesis pattern): - AdapterSpawnConfig's spawn variant gains an opt-in forwardStdio field; RubyAdapterPolicy sets it for launch (attach has no adapter process by construction) with a /^DEBUGGER: / stderr exclusion for rdbg's banners; RustAdapterPolicy sets it on win32 only (POSIX gets output via CodeLLDB's own DAP events from LLDB's STDOUT/STDERR broadcasts — the channels are mutually exclusive). - GenericAdapterManager fans each stream's LineBuffer out to the new raw callback (all lines, blank included, unsanitized — the client must see the program's real output, parity with debugpy/js-debug) while the persisted-log path keeps its blank-drop + sanitizeStderr redaction byte-for-byte. - DapProxyWorker synthesizes sendDapEvent('output', {category, output}) from the callback, so entries land in the existing per-session ring buffer with no changes above the proxy. Exit-flush race (found in live verification): a debuggee printing to a block-buffered pipe flushes everything at exit, milliseconds AFTER the adapter's terminated event and socket close — and the SessionManager reacts to either by stripping listeners and stopping the proxy, so the flushed output was dropped. When forwarding is active the worker now holds exited/terminated forwarding and the dap_connection_closed status behind a stdio-drain barrier (streams' close events, 2s backstop). Stream data fires before close and IPC is FIFO, so the output deterministically wins the race. Verified live on Windows: Ruby fizzbuzz yields all 15 stdout lines + the stderr marker with rdbg banners excluded; Rust hello_world yields the full program output including the exit-time tail. Also: fizzbuzz.rb gains a warn marker (appended — all breakpoint line numbers preserved), Ruby/Rust e2e smoke tests assert get_output, the comprehensive matrix gains outputMarkers for ruby+rust, and docs/skill references now describe launch-works/attach-doesn't for Ruby. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 1 + docs/ruby/README.md | 13 ++ docs/tool-reference.md | 2 +- examples/ruby/fizzbuzz.rb | 1 + .../src/interfaces/adapter-policy-ruby.ts | 8 +- .../src/interfaces/adapter-policy-rust.ts | 14 +- .../shared/src/interfaces/adapter-policy.ts | 16 ++ .../tests/unit/adapter-policy-rust.test.ts | 27 +++ skills/debugging/references/ruby.md | 4 +- skills/debugging/references/rust.md | 3 +- src/proxy/dap-proxy-adapter-manager.ts | 44 +++- src/proxy/dap-proxy-worker.ts | 102 ++++++++- .../ruby/unit/adapter-policy-ruby.test.ts | 32 ++- tests/e2e/comprehensive-mcp-tools.test.ts | 6 +- tests/e2e/mcp-server-smoke-ruby.test.ts | 16 ++ tests/e2e/mcp-server-smoke-rust.test.ts | 12 + tests/proxy/dap-proxy-worker.test.ts | 207 ++++++++++++++++++ .../proxy/dap-proxy-adapter-manager.test.ts | 72 ++++++ 18 files changed, 557 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2550554a..c44d8734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Debuggee exit code surfaced** — the exit code from the DAP `exited` event is stored on the session and returned as `exitCode` in `list_debug_sessions`, making a crash (non-zero) distinguishable from a clean exit (#220) ### Fixed +- **Ruby and Rust-on-Windows debuggee output captured** — adapters whose debuggee inherits the adapter process's stdio (rdbg `-c` on all platforms, CodeLLDB's console mode on Windows) now opt into proxy-side forwarding: the adapter's stdout/stderr lines are synthesized into DAP `output` events and land in the same per-session buffer `get_output` reads. rdbg's own `DEBUGGER:` stderr banners are excluded (they still go to the session log, sanitized, as before); the forwarded copy is raw, matching how debugpy/js-debug output reaches the buffer. Ruby attach mode is unchanged — no adapter process exists there, so output stays on the target's own terminal (fixes #222, fixes #223) - **Go debuggee output captured** — the Go adapter now launches Delve with `outputMode: 'remote'`, so the target program's stdout/stderr arrives as DAP output events and shows up in `get_output` instead of vanishing into dlv's own stdio (fixes #225) - **Rust launch config uses CodeLLDB's canonical `terminal` attribute** — the adapter emitted a debugpy-style `console` key that CodeLLDB only honors as a legacy alias; it now emits `terminal: 'console'`, translating any user-supplied legacy `console` values (#223 — the Windows output-capture gap the issue uncovered is tracked separately: LLDB's console mode inherits the debuggee's stdio rather than emitting DAP output events) - **js-debug launch no longer hangs on fast-exiting scripts** — the launch barrier now settles when the debuggee emits `terminated`/`exited` during the launch window, and `dispose()` rejects a still-pending wait as a structural backstop; `start_debugging` for a JavaScript script that crashes (or completes) within seconds of launch now returns promptly with state `stopped` instead of hanging past the MCP client timeout. Also removes an intermittent ~10s stall when js-debug's `initialized` event raced the handshake listener (fixes #242) diff --git a/docs/ruby/README.md b/docs/ruby/README.md index f4dad44f..327678e4 100644 --- a/docs/ruby/README.md +++ b/docs/ruby/README.md @@ -65,6 +65,15 @@ reports a `Local variables` scope), `get_local_variables`, `evaluate_expression` (evaluated in rdbg's `repl` context — expressions can read and modify program state), `step_over` / `step_into` / `step_out`, and `continue_execution`. +### Program output + +`rdbg -c` runs the script as a child of the adapter process with inherited stdio, so the +program's `puts`/`warn` output lands on the adapter's pipes rather than in DAP output +events. The proxy forwards those lines as synthesized `stdout`/`stderr` entries, so +`get_output` (and the `debug://sessions/{id}/output` resource) returns the script's +output as usual; rdbg's own `DEBUGGER:` stderr banners are excluded and only appear in +the session log. + ### Bundler projects Pass `useBundler` through the launch configuration to run the target via `bundle exec`: @@ -106,6 +115,10 @@ detach_from_process { "sessionId": "...", "terminateProcess": false } The target keeps running after detach, and `rdbg` keeps listening — you can re-attach later. Pass `terminateProcess: true` to kill the target instead. +Note: in attach mode there is no adapter process between mcp-debugger and the target, so +`get_output` captures nothing — the program's stdio stays on whatever terminal (or pod +log) the process was started in. + ## Remote attach (containers and Kubernetes) Because attach connects directly to rdbg's TCP socket, anything that forwards a TCP port diff --git a/docs/tool-reference.md b/docs/tool-reference.md index a2b65505..9a29ba0a 100644 --- a/docs/tool-reference.md +++ b/docs/tool-reference.md @@ -729,7 +729,7 @@ Gets the debuggee's output (stdout/stderr/console) captured for a session. Outpu - Works while the program is running and after it finishes — output stays readable until `close_debug_session`. Re-launching a session starts a fresh buffer (seq restarts at 1). - `hasMore: true` means more entries matched than `limit` allowed; call again with `since: nextSince`. - Incremental polling recipe: call once, remember `nextSince`, and pass it as `since` on the next call — you'll only ever see new output. -- Adapter support: Python (`redirectOutput`), JavaScript (`outputCapture: 'std'`), Go (`outputMode: 'remote'`), and Java forward debuggee stdio as output events; .NET typically does as well. Ruby routes debuggee stdio to the adapter process, so no entries are captured (#222); Rust does the same on Windows, where LLDB's console mode inherits stdio instead of emitting output events (#223). +- Adapter support: Python (`redirectOutput`), JavaScript (`outputCapture: 'std'`), Go (`outputMode: 'remote'`), and Java forward debuggee stdio as output events; .NET typically does as well. Ruby launch mode and Rust on Windows route debuggee stdio to the adapter process; the proxy forwards those lines as synthesized `stdout`/`stderr` events (#222/#223), excluding rdbg's `DEBUGGER:` banners. Ruby **attach** captures nothing — the target's stdio stays wherever the process was started. #### Output resources & subscriptions diff --git a/examples/ruby/fizzbuzz.rb b/examples/ruby/fizzbuzz.rb index 9862f4c5..768f9620 100644 --- a/examples/ruby/fizzbuzz.rb +++ b/examples/ruby/fizzbuzz.rb @@ -20,3 +20,4 @@ def main end main +warn 'fizzbuzz complete' diff --git a/packages/shared/src/interfaces/adapter-policy-ruby.ts b/packages/shared/src/interfaces/adapter-policy-ruby.ts index 078f0b7b..4bc2d015 100644 --- a/packages/shared/src/interfaces/adapter-policy-ruby.ts +++ b/packages/shared/src/interfaces/adapter-policy-ruby.ts @@ -209,7 +209,13 @@ export const RubyAdapterPolicy: AdapterPolicy = { host: payload.adapterHost, port: payload.adapterPort, logDir: payload.logDir, - env: payload.adapterCommand.env + env: payload.adapterCommand.env, + // rdbg -c runs the debuggee as a child of the adapter process with + // inherited stdio while DAP travels over TCP — the program's output + // only ever appears on the adapter's pipes (issue #222). rdbg's own + // stderr banners ("DEBUGGER: wait for debugger connection...") are + // excluded from forwarding but still logged. + forwardStdio: { excludeStderrLinePattern: /^DEBUGGER: / } }; } }; diff --git a/packages/shared/src/interfaces/adapter-policy-rust.ts b/packages/shared/src/interfaces/adapter-policy-rust.ts index ce618cf0..a3e43efc 100644 --- a/packages/shared/src/interfaces/adapter-policy-rust.ts +++ b/packages/shared/src/interfaces/adapter-policy-rust.ts @@ -269,6 +269,14 @@ export const RustAdapterPolicy: AdapterPolicy = { * Get the configuration for spawning the Rust debug adapter (CodeLLDB) */ getAdapterSpawnConfig: (payload, platform: NodeJS.Platform = process.platform, arch: NodeJS.Architecture = process.arch) => { + // Windows only (issue #223): CodeLLDB's console mode performs no stdio + // redirection, and unlike POSIX (where LLDB holds the debuggee's stdio + // pipes and CodeLLDB emits DAP output events from the STDOUT/STDERR + // process broadcasts), LLDB on Windows lets the debuggee inherit the + // adapter process's pipes. Forward those as output events there; on + // POSIX the channels are exclusive, so this stays off to avoid noise. + const forwardStdio = platform === 'win32' ? {} : undefined; + // If a custom adapter command was provided, use it directly if (payload.adapterCommand) { return { @@ -278,7 +286,8 @@ export const RustAdapterPolicy: AdapterPolicy = { host: payload.adapterHost, port: payload.adapterPort, logDir: payload.logDir, - env: payload.adapterCommand.env + env: payload.adapterCommand.env, + forwardStdio }; } @@ -320,7 +329,8 @@ export const RustAdapterPolicy: AdapterPolicy = { ...process.env, // Windows specific: enable native PDB reader ...(platform === 'win32' ? { LLDB_USE_NATIVE_PDB_READER: '1' } : {}) - } + }, + forwardStdio }; } }; diff --git a/packages/shared/src/interfaces/adapter-policy.ts b/packages/shared/src/interfaces/adapter-policy.ts index 65311bba..4d15a918 100644 --- a/packages/shared/src/interfaces/adapter-policy.ts +++ b/packages/shared/src/interfaces/adapter-policy.ts @@ -383,6 +383,22 @@ export type AdapterSpawnConfig = logDir: string; cwd?: string; env?: NodeJS.ProcessEnv; + /** + * Forward the adapter process's stdio as synthesized DAP 'output' + * events (issue #222). Opt-in for adapters whose launch-mode debuggee + * inherits the adapter process's stdio instead of having it converted + * to DAP output events — e.g. rdbg -c (all platforms) and CodeLLDB's + * console mode on Windows. Policies that leave this unset keep the + * existing behavior: adapter stdio is drained to logs only. + */ + forwardStdio?: { + /** + * stderr lines matching this pattern are adapter diagnostics (e.g. + * rdbg's `DEBUGGER: ` banners): still logged, never forwarded as + * debuggee output. + */ + excludeStderrLinePattern?: RegExp; + }; } | { mode: 'connect'; diff --git a/packages/shared/tests/unit/adapter-policy-rust.test.ts b/packages/shared/tests/unit/adapter-policy-rust.test.ts index 53f270f6..b16e0ac0 100644 --- a/packages/shared/tests/unit/adapter-policy-rust.test.ts +++ b/packages/shared/tests/unit/adapter-policy-rust.test.ts @@ -175,6 +175,33 @@ describe('RustAdapterPolicy', () => { expect(config.args).toEqual(['--port', '9000']); expect(config.env?.LLDB_USE_NATIVE_PDB_READER).toBe('1'); }); + + it('opts into adapter-stdio forwarding on win32 only (issue #223)', () => { + // Windows: LLDB's console mode lets the debuggee inherit the adapter + // process's stdio — forwarding is the only way output reaches get_output. + const win = RustAdapterPolicy.getAdapterSpawnConfig!({ + adapterHost: '127.0.0.1', + adapterPort: 9000, + logDir: '/tmp/logs' + }, 'win32', 'x64'); + expect(win.forwardStdio).toEqual({}); + + const winCustom = RustAdapterPolicy.getAdapterSpawnConfig!({ + adapterCommand: { command: 'custom', args: [] }, + adapterHost: '127.0.0.1', + adapterPort: 9000, + logDir: '/tmp/logs' + }, 'win32', 'x64'); + expect(winCustom.forwardStdio).toEqual({}); + + // POSIX: CodeLLDB emits DAP output events itself (LLDB holds the pipes) + const linux = RustAdapterPolicy.getAdapterSpawnConfig!({ + adapterHost: '127.0.0.1', + adapterPort: 9000, + logDir: '/tmp/logs' + }, 'linux', 'x64'); + expect(linux.forwardStdio).toBeUndefined(); + }); }); it('handles reverse requests via DAP client behavior', async () => { diff --git a/skills/debugging/references/ruby.md b/skills/debugging/references/ruby.md index 4f880214..5483f191 100644 --- a/skills/debugging/references/ruby.md +++ b/skills/debugging/references/ruby.md @@ -70,7 +70,7 @@ For containers/pods, use the **debuggee's** filesystem paths in `set_breakpoint` ## Quirks - **Launch always stops at load.** rdbg suspends the script before the first line so breakpoints bind even for scripts that finish in milliseconds. With `stopOnEntry: false` (default) that entry pause is released automatically; with `dapLaunchArgs: { "stopOnEntry": true }` you get control at the first line. -- **KNOWN ISSUE — `get_output` captures nothing for Ruby (issue #222).** rdbg routes debuggee stdout/stderr to the adapter process, so no output entries are recorded. Do not diagnose by reading stdout. Instead: have the script write results to a file you can read, or pause at a breakpoint and use `evaluate_expression` / `get_local_variables` to inspect state directly. +- **Debuggee output is captured in launch mode.** rdbg hands the debuggee the adapter process's stdio; the proxy forwards it as `get_output` entries (categories `stdout`/`stderr`, rdbg's own `DEBUGGER:` banners excluded). **Attach mode captures nothing** — the target's stdio stays wherever the process was started; inspect state via `evaluate_expression` / `get_local_variables` there instead. - **evaluate_expression runs in rdbg's `repl` context** — expressions can read *and modify* program state (`x = 5` works). Useful for testing fixes live. - **Scope name is `Local variables`** (not "Locals"); `get_local_variables` handles this for you. Locals are only reported while stopped. - **Windows `.bat` shim bypass is automatic** — if spawn fails anyway, set `RDBG_PATH` to the rdbg script inside your Ruby installation's `bin` directory. @@ -85,4 +85,4 @@ For containers/pods, use the **debuggee's** filesystem paths in `set_breakpoint` | Connect refused on attach | Target not listening | Start it with `rdbg --open --host --port

`; rdbg prints `Debugger can attach via TCP/IP` when ready; verify port-forwarding | | Breakpoint not verified on attach | Host path used for a remote/container target | Use the path as the debuggee sees it (e.g. `/app/app.rb` from `get_stack_trace`) | | Locals empty | Session not paused | Hit a breakpoint or `pause_execution` first — rdbg reports locals only while stopped | -| `get_output` returns no entries | Issue #222 — Ruby debuggee stdio is not captured | Write to a file from the script, or inspect via `evaluate_expression` at a breakpoint | +| `get_output` returns no entries on attach | Attached target's stdio stays on its own terminal/pod | Read the target's own logs, or inspect via `evaluate_expression` at a breakpoint | diff --git a/skills/debugging/references/rust.md b/skills/debugging/references/rust.md index 8efb0e17..a8c2e8b2 100644 --- a/skills/debugging/references/rust.md +++ b/skills/debugging/references/rust.md @@ -53,7 +53,7 @@ Not supported. The Rust adapter implements launch mode only — `attach_to_proce - **Windows toolchain (critical):** MSVC-built binaries give control flow only — breakpoints/stepping work, but strings, Vecs, and structs show `` or corrupted values. `RUST_MSVC_BEHAVIOR` controls what happens when an MSVC binary is detected: `warn` (default — log and proceed), `error` (fail with `ENVIRONMENT_INVALID`), `continue` (silent). Check any binary first with `mcp-debugger check-rust-binary target/debug/app.exe` — it reports `Toolchain: GNU` or `MSVC`. - **Windows initial stop:** debugging may first stop in system functions (not user code). Issue one `continue_execution` to reach your breakpoint; auto-continue through these system stops is not yet implemented. -- **KNOWN ISSUE — `get_output` may be empty on Windows (issue #223):** in CodeLLDB's console mode the debuggee inherits the adapter process's stdio, and on Windows LLDB does not convert it to DAP output events. Do not rely on print debugging there — inspect state with `evaluate_expression`, `get_local_variables`, and breakpoints instead. +- **Debuggee output is captured:** on POSIX CodeLLDB forwards the program's stdio as DAP output events; on Windows (where LLDB's console mode makes the debuggee inherit the adapter process's stdio) the proxy forwards the adapter's stdio instead. Either way the program's stdout/stderr arrives as `get_output` entries. - Expression evaluation goes through LLDB: simple field access and method calls like `my_vec.len()` work, but Rust-specific syntax (closures, trait methods) may not. - Debug builds only: release builds need `debug = true` in `[profile.release]` and still inline/optimize away variables. Prefer `opt-level = 0`. - GNU builds of crates that import Windows DLLs (`tokio`, `windows-sys`, `parking_lot_core`, ...) need full MinGW binutils — rustup's self-contained toolchain lacks `as.exe`, so `dlltool` fails. Install via MSYS2 (`mingw-w64-x86_64-binutils`, `-gcc`) and prepend `C:\msys64\mingw64\bin` to PATH. @@ -68,4 +68,3 @@ Not supported. The Rust adapter implements launch mode only — `attach_to_proce | "Can't find CodeLLDB" | Adapter not vendored / npx package on non-Linux | Run `pnpm --filter @debugmcp/adapter-rust run build:adapter`, or set `CODELLDB_PATH` | | First stop is in system/ntdll frames | Windows initial system breakpoint | `continue_execution` once, then you land on your breakpoint | | `dlltool ... CreateProcess` build error | rustup GNU toolchain missing `as.exe` | Install MSYS2 mingw-w64 binutils/gcc; prepend `C:\msys64\mingw64\bin` to PATH | -| `get_output` returns no stdout (Windows) | Issue #223 — debuggee stdio inherited by adapter process | Use `evaluate_expression` / variables at breakpoints instead | diff --git a/src/proxy/dap-proxy-adapter-manager.ts b/src/proxy/dap-proxy-adapter-manager.ts index 811e8850..ec1dea78 100644 --- a/src/proxy/dap-proxy-adapter-manager.ts +++ b/src/proxy/dap-proxy-adapter-manager.ts @@ -13,6 +13,9 @@ import { AdapterSpawnResult } from './dap-proxy-interfaces.js'; +/** Which adapter-process stream a forwarded line came from. */ +export type AdapterStdioSource = 'stdout' | 'stderr'; + /** * Configuration for spawning any debug adapter */ @@ -22,6 +25,15 @@ export interface GenericAdapterConfig { logDir: string; cwd?: string; env?: NodeJS.ProcessEnv; + /** + * When set, every raw line read from the adapter process's stdout/stderr — + * including blank lines, unsanitized — is also delivered here (issue #222: + * some adapters hand the debuggee their own stdio, so these lines ARE the + * program's output). The sanitized, blank-dropping log path is unchanged; + * the redaction that protects persisted logs must not rewrite what the + * debugging client sees, matching debugpy/js-debug output-event behavior. + */ + onStdioLine?: (source: AdapterStdioSource, line: string) => void; } /** @@ -129,7 +141,7 @@ export class GenericAdapterManager { this.logger.info(`[AdapterManager] Spawned adapter process PID: ${adapterProcess.pid} (windowsHide=${!!spawnOptions.windowsHide}, detached=${!!spawnOptions.detached})`); // Set up error handlers and stderr capture - this.setupProcessHandlers(adapterProcess); + this.setupProcessHandlers(adapterProcess, config.onStdioLine); return { process: adapterProcess, @@ -140,7 +152,10 @@ export class GenericAdapterManager { /** * Set up process event handlers */ - private setupProcessHandlers(adapterProcess: ChildProcess): void { + private setupProcessHandlers( + adapterProcess: ChildProcess, + onStdioLine?: (source: AdapterStdioSource, line: string) => void + ): void { adapterProcess.on('error', (err: Error) => { this.logger.error('[AdapterManager] Adapter process spawn error:', err); }); @@ -150,8 +165,10 @@ export class GenericAdapterManager { // assignment split across two chunks would otherwise leak its tail past // the key/value redaction patterns (issues #151/#153). if (adapterProcess.stderr) { - this.consumeStream(adapterProcess.stderr, line => - this.logger.error(`[AdapterManager STDERR] ${line}`) + this.consumeStream( + adapterProcess.stderr, + line => this.logger.error(`[AdapterManager STDERR] ${line}`), + onStdioLine && (line => onStdioLine('stderr', line)) ); } @@ -159,8 +176,10 @@ export class GenericAdapterManager { // it through the same sanitized path so a chatty adapter cannot fill the // pipe buffer and stall, and its diagnostics land in the log at debug. if (adapterProcess.stdout) { - this.consumeStream(adapterProcess.stdout, line => - this.logger.debug(`[AdapterManager STDOUT] ${line}`) + this.consumeStream( + adapterProcess.stdout, + line => this.logger.debug(`[AdapterManager STDOUT] ${line}`), + onStdioLine && (line => onStdioLine('stdout', line)) ); } @@ -175,9 +194,20 @@ export class GenericAdapterManager { * process 'exit' — the pipe can still deliver the rest of a split line * after exit, which would re-create the straddle leak (issue #151). */ - private consumeStream(stream: Readable, logLine: (line: string) => void): void { + private consumeStream( + stream: Readable, + logLine: (line: string) => void, + forwardLine?: (line: string) => void + ): void { const buffer = new LineBuffer(); const record = (lines: string[]) => { + if (forwardLine) { + // Debuggee-output fan-out (issue #222): raw lines, blank lines + // included — they are program output, not log noise. + for (const line of lines) { + forwardLine(line); + } + } for (const line of sanitizeStderr(lines.filter(l => l.trim().length > 0))) { logLine(line); } diff --git a/src/proxy/dap-proxy-worker.ts b/src/proxy/dap-proxy-worker.ts index 35a1a115..d6fda4aa 100644 --- a/src/proxy/dap-proxy-worker.ts +++ b/src/proxy/dap-proxy-worker.ts @@ -20,7 +20,7 @@ import { ErrorMessage } from './dap-proxy-interfaces.js'; import { CallbackRequestTracker } from './dap-proxy-request-tracker.js'; -import { GenericAdapterManager } from './dap-proxy-adapter-manager.js'; +import { GenericAdapterManager, AdapterStdioSource } from './dap-proxy-adapter-manager.js'; import { DapConnectionManager } from './dap-proxy-connection-manager.js'; import { validateProxyInitPayload @@ -68,6 +68,8 @@ export class DapProxyWorker { // wins over synthesis, and parent+child terminated events synthesize once private exitedEventSeen: boolean = false; private exitSynthesisAttempted: boolean = false; + /** Armed when adapter stdio forwarding is active (issue #222): resolves once both stdio streams close. */ + private adapterStdioDrained: Promise | null = null; private initializedEventPending: boolean = false; private deferInitializedHandling: boolean = false; private initializedEventHandled: boolean = false; @@ -373,9 +375,15 @@ export class DapProxyWorker { spawnConfig.cwd = process.env.MCP_WORKSPACE_ROOT; } - const spawnResult = await this.processManager!.spawn(spawnConfig); + const spawnResult = await this.processManager!.spawn({ + ...spawnConfig, + onStdioLine: this.buildStdioForwarder(spawnConfig.forwardStdio) + }); this.adapterProcess = spawnResult.process; + if (spawnConfig.forwardStdio) { + this.adapterStdioDrained = this.createStdioDrainBarrier(spawnResult.process); + } this.logger!.info(`[Worker] Adapter spawned with PID: ${spawnResult.pid}`); this.adapterProcess.on('error', (err) => { @@ -637,14 +645,22 @@ export class DapProxyWorker { this.logger!.debug('[Worker] DAP event: thread', body); this.sendDapEvent('thread', body); }, - onExited: (body) => { + onExited: async (body) => { this.logger!.info(`[Worker] DAP event: exited exitCode=${body.exitCode}`); - // A real exited event stays authoritative - suppress synthesis (issue #247) + // A real exited event stays authoritative - suppress synthesis (issue #247). + // Set synchronously, before any await, so a racing terminated sees it. this.exitedEventSeen = true; + await this.waitForAdapterStdioDrain(); this.sendDapEvent('exited', body); }, onTerminated: async (body) => { this.logger!.info(`[Worker] DAP event: terminated body=${JSON.stringify(body)}`); + // When adapter stdio is forwarded as debuggee output, the exit-time + // flush of a block-buffered pipe arrives milliseconds AFTER the DAP + // terminated event — but the SessionManager reacts to terminated by + // stopping the proxy, which drops late messages. Hold terminated + // until the streams have drained so the output wins the race. + await this.waitForAdapterStdioDrain(); // Must complete before terminated is forwarded: whichever of // exited/terminated reaches the SessionManager first strips the // other's handler, and shutdown() below tears down the client @@ -656,8 +672,13 @@ export class DapProxyWorker { this.logger!.error('[Worker] DAP client error:', err); this.sendError(`DAP client error: ${err.message}`); }, - onClose: () => { + onClose: async () => { this.logger!.info('[Worker] DAP client connection closed.'); + // Adapters that close the DAP socket at debuggee exit (rdbg) race the + // exit-time stdio flush exactly like terminated does — the parent + // reacts to dap_connection_closed by tearing down its listeners, so + // hold this path behind the same drain barrier (issue #222). + await this.waitForAdapterStdioDrain(); this.sendStatus('dap_connection_closed'); this.shutdown(); } @@ -1138,6 +1159,77 @@ export class DapProxyWorker { } } + /** + * Resolves when both adapter stdio streams have closed — i.e. every byte + * the debuggee flushed on exit has been read and forwarded (issue #222). + * Only armed when stdio forwarding is active. + */ + private createStdioDrainBarrier(adapterProcess: ChildProcess): Promise { + const streamClosed = (stream: NodeJS.ReadableStream | null): Promise => + !stream || (stream as unknown as { closed?: boolean }).closed + ? Promise.resolve() + : new Promise(resolve => stream.once('close', () => resolve())); + return Promise.all([ + streamClosed(adapterProcess.stdout), + streamClosed(adapterProcess.stderr) + ]).then(() => undefined); + } + + /** + * Hold exited/terminated forwarding until adapter stdio has drained: a + * debuggee printing to a block-buffered pipe flushes everything at exit, + * milliseconds after the adapter's terminated event, and the SessionManager + * stops the proxy on terminated — dropping late messages. Stream 'data' + * fires before 'close' and IPC is FIFO, so waiting here guarantees the + * forwarded output reaches the session buffer first. 2s backstop for + * adapters that never close their pipes. No-op when forwarding is off. + */ + private async waitForAdapterStdioDrain(): Promise { + if (!this.adapterStdioDrained) { + return; + } + let timer: NodeJS.Timeout | undefined; + const backstop = new Promise(resolve => { + timer = setTimeout(resolve, 2000); + }); + try { + await Promise.race([this.adapterStdioDrained, backstop]); + } finally { + if (timer) { + clearTimeout(timer); + } + } + } + + /** + * Build the raw-stdio → DAP 'output' forwarder for adapters whose debuggee + * inherits the adapter process's stdio (issue #222: rdbg -c on all + * platforms, CodeLLDB's console mode on Windows). Returns undefined when + * the policy did not opt in via spawnConfig.forwardStdio — the adapter + * manager then drains stdio to logs only, exactly as before. + */ + private buildStdioForwarder( + forwardConfig: { excludeStderrLinePattern?: RegExp } | undefined + ): ((source: AdapterStdioSource, line: string) => void) | undefined { + if (!forwardConfig) { + return undefined; + } + const exclude = forwardConfig.excludeStderrLinePattern; + return (source, line) => { + if (source === 'stderr' && exclude?.test(line)) { + return; // adapter diagnostic banner: log path only + } + try { + // '\n' restores the line ending LineBuffer stripped, and keeps blank + // lines past handleOutput's empty-output drop. + this.sendDapEvent('output', { category: source, output: line + '\n' }); + } catch (err) { + // IPC gone during teardown; a stream 'data' handler must never throw. + this.logger?.debug?.('[Worker] Failed to forward adapter stdio line', err); + } + }; + } + private sendDapEvent(event: string, body: unknown): void { const message: DapEventMessage = { type: 'dapEvent', diff --git a/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts b/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts index d6ded29b..c4828ce9 100644 --- a/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts +++ b/tests/adapters/ruby/unit/adapter-policy-ruby.test.ts @@ -93,10 +93,40 @@ describe('RubyAdapterPolicy.getAdapterSpawnConfig', () => { host: '127.0.0.1', port: 4711, logDir: '/tmp/logs', - env: { FOO: 'bar' } + env: { FOO: 'bar' }, + forwardStdio: { excludeStderrLinePattern: /^DEBUGGER: / } }); }); + it('opts launch mode into stdio forwarding with an rdbg banner exclusion (issue #222)', () => { + const config = RubyAdapterPolicy.getAdapterSpawnConfig!({ + ...basePayload, + launchConfig: { request: 'launch' }, + adapterCommand: { command: '/usr/bin/rdbg', args: ['--open'] } + }); + + expect(config.mode).toBe('spawn'); + const pattern = (config as { forwardStdio?: { excludeStderrLinePattern?: RegExp } }) + .forwardStdio?.excludeStderrLinePattern; + expect(pattern).toBeInstanceOf(RegExp); + // rdbg's own banners are excluded... + expect(pattern!.test('DEBUGGER: Debugger can attach via TCP/IP (127.0.0.1:4711)')).toBe(true); + expect(pattern!.test('DEBUGGER: wait for debugger connection...')).toBe(true); + // ...but program output is not, even when it mentions the prefix mid-line + expect(pattern!.test('6: Fizz')).toBe(false); + expect(pattern!.test('log: DEBUGGER: mentioned mid-line')).toBe(false); + }); + + it('does not enable stdio forwarding for attach (no adapter process exists)', () => { + const config = RubyAdapterPolicy.getAdapterSpawnConfig!({ + ...basePayload, + launchConfig: { request: 'attach', port: 9229 } + }); + + expect(config.mode).toBe('connect'); + expect('forwardStdio' in config).toBe(false); + }); + it('throws for launch without an adapter command (no silent fallback)', () => { expect(() => RubyAdapterPolicy.getAdapterSpawnConfig!({ diff --git a/tests/e2e/comprehensive-mcp-tools.test.ts b/tests/e2e/comprehensive-mcp-tools.test.ts index dfdf9db7..eb83cdb7 100644 --- a/tests/e2e/comprehensive-mcp-tools.test.ts +++ b/tests/e2e/comprehensive-mcp-tools.test.ts @@ -136,8 +136,10 @@ const LANGUAGES: LangDef[] = [ { language: 'python', script: PYTHON_SCRIPT, bpLine: PYTHON_BP_LINE, available: true }, { language: 'javascript', script: JS_SCRIPT, bpLine: JS_BP_LINE, available: true }, { language: 'mock', script: PYTHON_SCRIPT, bpLine: PYTHON_BP_LINE, available: true }, - { language: 'rust', script: RUST_SCRIPT, bpLine: RUST_BP_LINE, available: hasRust, skipReason: hasRust ? undefined : 'Rust toolchain not installed' }, - { language: 'ruby', script: RUBY_SCRIPT, bpLine: RUBY_BP_LINE, available: hasRuby, skipReason: hasRuby ? undefined : 'Ruby/rdbg not installed' }, + { language: 'rust', script: RUST_SCRIPT, bpLine: RUST_BP_LINE, available: hasRust, skipReason: hasRust ? undefined : 'Rust toolchain not installed', + outputMarker: 'Hello, MCP Debugger!' }, + { language: 'ruby', script: RUBY_SCRIPT, bpLine: RUBY_BP_LINE, available: hasRuby, skipReason: hasRuby ? undefined : 'Ruby/rdbg not installed', + outputMarker: '1: 1' }, // iteration 1's puts — the loop breakpoint re-arms, so later output isn't guaranteed { language: 'go', script: GO_SCRIPT, bpLine: GO_BP_LINE, available: hasGo, skipReason: hasGo ? undefined : 'Go/Delve not installed', dapLaunchArgs: { mode: 'exec' }, outputMarker: 'Hello, World!' }, // launchScript set in beforeAll after build { language: 'dotnet', script: DOTNET_SCRIPT, bpLine: DOTNET_BP_LINE, available: hasDotnet, skipReason: hasDotnet ? undefined : '.NET/netcoredbg not installed', diff --git a/tests/e2e/mcp-server-smoke-ruby.test.ts b/tests/e2e/mcp-server-smoke-ruby.test.ts index bff29431..5f807f96 100644 --- a/tests/e2e/mcp-server-smoke-ruby.test.ts +++ b/tests/e2e/mcp-server-smoke-ruby.test.ts @@ -197,5 +197,21 @@ describe('MCP Server Ruby Debugging Smoke Test @requires-ruby', () => { // 8. Continue to completion (no further matching breakpoints) const contResult = await callToolSafely(mcpClient!, 'continue_execution', { sessionId }); expect(contResult.success).toBe(true); + + // 9. Debuggee output must be retrievable (issue #222): rdbg -c gives the + // debuggee the adapter process's stdio, which the proxy forwards as + // synthesized output events. Wait for the script to finish and flush. + await new Promise(resolve => setTimeout(resolve, 3000)); + const outputResult = await callToolSafely(mcpClient!, 'get_output', { sessionId }); + expect(outputResult.success).toBe(true); + const outputEntries = outputResult.entries as Array<{ category: string; output: string }>; + const fizzEntry = outputEntries.find(e => e.output.includes('6: Fizz')); + expect(fizzEntry).toBeDefined(); + expect(fizzEntry!.category).toBe('stdout'); + const warnEntry = outputEntries.find(e => e.output.includes('fizzbuzz complete')); + expect(warnEntry).toBeDefined(); + expect(warnEntry!.category).toBe('stderr'); + // rdbg's own banners must stay out of the output buffer + expect(outputEntries.some(e => e.output.startsWith('DEBUGGER:'))).toBe(false); }, 90000); // Ruby interpreter startup under rdbg takes several seconds }); diff --git a/tests/e2e/mcp-server-smoke-rust.test.ts b/tests/e2e/mcp-server-smoke-rust.test.ts index 5a530fe3..fa4874b3 100644 --- a/tests/e2e/mcp-server-smoke-rust.test.ts +++ b/tests/e2e/mcp-server-smoke-rust.test.ts @@ -191,6 +191,18 @@ describe('MCP Server Rust Debugging Smoke Test', () => { if (versionValue) { expect(versionValue).toContain('1.75'); } + + // Debuggee output must be retrievable (issue #223): POSIX gets it via + // CodeLLDB's own DAP output events, Windows via the proxy's adapter-stdio + // forwarding. Both markers print before the line-26 breakpoint, so no + // continue is needed (Windows re-hits the breakpoint on continue). + const outputResult = await callToolSafely(mcpClient!, 'get_output', { sessionId }); + expect(outputResult.success).toBe(true); + const outputEntries = outputResult.entries as Array<{ category: string; output: string }>; + const helloEntry = outputEntries.find(e => e.output.includes('Hello, MCP Debugger!')); + expect(helloEntry).toBeDefined(); + expect(helloEntry!.category).toBe('stdout'); + expect(outputEntries.some(e => e.output.includes('Sum of 5 and 10 is: 15'))).toBe(true); }, 60000 ); diff --git a/tests/proxy/dap-proxy-worker.test.ts b/tests/proxy/dap-proxy-worker.test.ts index d8bd0809..dfca9627 100644 --- a/tests/proxy/dap-proxy-worker.test.ts +++ b/tests/proxy/dap-proxy-worker.test.ts @@ -688,6 +688,213 @@ describe('DapProxyWorker', () => { ); }); + it('startAdapterAndConnect wires adapter stdio forwarding for Ruby launch sessions (issue #222)', async () => { + const payload: ProxyInitPayload = { + cmd: 'init', + sessionId: 'ruby-launch-session', + language: 'ruby', + executablePath: 'ruby', + adapterHost: '127.0.0.1', + adapterPort: 8124, + logDir: '/logs', + scriptPath: '/path/to/fizzbuzz.rb', + launchConfig: { request: 'launch', type: 'rdbg' }, + adapterCommand: { + command: 'rdbg', + args: ['--open', '--host', '127.0.0.1', '--port', '8124', '-c', '--', 'ruby', '/path/to/fizzbuzz.rb'] + } + }; + + const processStub = { + spawn: vi.fn().mockResolvedValue({ + process: new EventEmitter() as unknown as ChildProcess, + pid: 4242 + }), + shutdown: vi.fn().mockResolvedValue(undefined) + }; + + const connectionStub = { + connectWithRetry: vi.fn().mockResolvedValue(mockDapClient), + setAdapterPolicy: vi.fn(), + setupEventHandlers: vi.fn((client: EventEmitter, handlers: Record void>) => { + if (handlers.onInitialized) client.on('initialized', handlers.onInitialized); + }), + initializeSession: vi.fn().mockImplementation(async () => { + setImmediate(() => (mockDapClient as EventEmitter).emit('initialized')); + }), + sendLaunchRequest: vi.fn().mockResolvedValue(undefined), + setBreakpoints: vi.fn().mockResolvedValue(undefined), + sendConfigurationDone: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn().mockResolvedValue(undefined) + }; + + (worker as any).logger = mockLogger; + (worker as any).processManager = processStub; + (worker as any).connectionManager = connectionStub; + (worker as any).adapterPolicy = RubyAdapterPolicy; + (worker as any).adapterState = RubyAdapterPolicy.createInitialState(); + (worker as any).currentInitPayload = payload; + (worker as any).currentSessionId = payload.sessionId; + (worker as any).state = ProxyState.INITIALIZING; + + await (worker as any).startAdapterAndConnect(payload); + + expect(processStub.spawn).toHaveBeenCalledTimes(1); + const spawnConfig = processStub.spawn.mock.calls[0][0]; + expect(typeof spawnConfig.onStdioLine).toBe('function'); + + const outputMessages = () => mockMessageSender.send.mock.calls + .map(([message]) => message) + .filter((message) => message.type === 'dapEvent' && message.event === 'output'); + + spawnConfig.onStdioLine('stdout', '6: Fizz'); + expect(outputMessages().at(-1)?.body).toEqual({ category: 'stdout', output: '6: Fizz\n' }); + + spawnConfig.onStdioLine('stderr', 'some warning'); + expect(outputMessages().at(-1)?.body).toEqual({ category: 'stderr', output: 'some warning\n' }); + + // Blank lines are program output and survive as bare newlines + spawnConfig.onStdioLine('stdout', ''); + expect(outputMessages().at(-1)?.body).toEqual({ category: 'stdout', output: '\n' }); + + // rdbg's own stderr banners are excluded from forwarding + const before = outputMessages().length; + spawnConfig.onStdioLine('stderr', 'DEBUGGER: wait for debugger connection...'); + expect(outputMessages().length).toBe(before); + }); + + it('holds terminated and dap_connection_closed until adapter stdio drains (issue #222)', async () => { + // A debuggee printing to a block-buffered pipe flushes everything at + // exit, milliseconds AFTER the adapter's terminated event / socket + // close. The worker must forward the flushed output before either + // teardown signal reaches the parent, which reacts by dropping late + // messages. + const payload: ProxyInitPayload = { + cmd: 'init', + sessionId: 'ruby-drain-session', + language: 'ruby', + executablePath: 'ruby', + adapterHost: '127.0.0.1', + adapterPort: 8125, + logDir: '/logs', + scriptPath: '/path/to/fizzbuzz.rb', + launchConfig: { request: 'launch', type: 'rdbg' }, + adapterCommand: { command: 'rdbg', args: ['--open', '-c', '--', 'ruby', '/path/to/fizzbuzz.rb'] } + }; + + const adapterProcess = new EventEmitter() as any; + adapterProcess.pid = 4244; + adapterProcess.stdout = new EventEmitter(); + adapterProcess.stderr = new EventEmitter(); + adapterProcess.kill = vi.fn(); + adapterProcess.unref = vi.fn(); + + const processStub = { + spawn: vi.fn().mockResolvedValue({ process: adapterProcess as ChildProcess, pid: 4244 }), + shutdown: vi.fn().mockResolvedValue(undefined) + }; + + const connectionStub = { + connectWithRetry: vi.fn().mockResolvedValue(mockDapClient), + setAdapterPolicy: vi.fn(), + setupEventHandlers: vi.fn((client: EventEmitter, handlers: Record void>) => { + if (handlers.onInitialized) client.on('initialized', handlers.onInitialized); + if (handlers.onTerminated) client.on('terminated', handlers.onTerminated); + if (handlers.onClose) client.on('close', handlers.onClose); + }), + initializeSession: vi.fn().mockImplementation(async () => { + setImmediate(() => (mockDapClient as EventEmitter).emit('initialized')); + }), + sendLaunchRequest: vi.fn().mockResolvedValue(undefined), + setBreakpoints: vi.fn().mockResolvedValue(undefined), + sendConfigurationDone: vi.fn().mockResolvedValue(undefined), + disconnect: vi.fn().mockResolvedValue(undefined) + }; + + (worker as any).logger = mockLogger; + (worker as any).processManager = processStub; + (worker as any).connectionManager = connectionStub; + (worker as any).adapterPolicy = RubyAdapterPolicy; + (worker as any).adapterState = RubyAdapterPolicy.createInitialState(); + (worker as any).currentInitPayload = payload; + (worker as any).currentSessionId = payload.sessionId; + (worker as any).state = ProxyState.INITIALIZING; + + await (worker as any).startAdapterAndConnect(payload); + const spawnConfig = processStub.spawn.mock.calls[0][0]; + + const sentMessages = () => mockMessageSender.send.mock.calls.map(([m]) => m); + const flush = () => new Promise(resolve => setImmediate(resolve)); + + // Adapter announces termination and closes the DAP socket... + (mockDapClient as EventEmitter).emit('terminated', {}); + (mockDapClient as EventEmitter).emit('close'); + await flush(); + + // ...but neither signal is forwarded while the stdio streams are open + expect(sentMessages().some(m => m.type === 'dapEvent' && m.event === 'terminated')).toBe(false); + expect(sentMessages().some(m => m.type === 'status' && m.status === 'dap_connection_closed')).toBe(false); + + // The exit-time flush arrives, then the streams close + spawnConfig.onStdioLine('stdout', '15: FizzBuzz'); + adapterProcess.stdout.emit('close'); + adapterProcess.stderr.emit('close'); + await flush(); + + const messages = sentMessages(); + const outputIdx = messages.findIndex(m => + m.type === 'dapEvent' && m.event === 'output' && m.body?.category === 'stdout' && m.body?.output === '15: FizzBuzz\n'); + const terminatedIdx = messages.findIndex(m => m.type === 'dapEvent' && m.event === 'terminated'); + expect(outputIdx).toBeGreaterThanOrEqual(0); + expect(terminatedIdx).toBeGreaterThanOrEqual(0); + // The flushed output must reach the parent before the teardown signal + expect(outputIdx).toBeLessThan(terminatedIdx); + }); + + it('startAdapterAndConnect passes no stdio forwarder for policies that do not opt in', async () => { + const payload: ProxyInitPayload = { + cmd: 'init', + sessionId: 'py-no-forward-session', + executablePath: 'python', + adapterHost: 'localhost', + adapterPort: 5679, + logDir: '/logs', + scriptPath: '/path/to/script.py' + }; + + const processStub = { + spawn: vi.fn().mockResolvedValue({ + process: new EventEmitter() as unknown as ChildProcess, + pid: 4243 + }), + shutdown: vi.fn().mockResolvedValue(undefined) + }; + + const connectionStub = { + connectWithRetry: vi.fn().mockResolvedValue(mockDapClient), + setAdapterPolicy: vi.fn(), + setupEventHandlers: vi.fn(), + initializeSession: vi.fn(), + sendLaunchRequest: vi.fn(), + setBreakpoints: vi.fn(), + sendConfigurationDone: vi.fn(), + disconnect: vi.fn() + }; + + (worker as any).logger = mockLogger; + (worker as any).processManager = processStub; + (worker as any).connectionManager = connectionStub; + (worker as any).adapterPolicy = PythonAdapterPolicy; + (worker as any).adapterState = PythonAdapterPolicy.createInitialState(); + (worker as any).currentInitPayload = payload; + (worker as any).state = ProxyState.INITIALIZING; + + await (worker as any).startAdapterAndConnect(payload); + + expect(processStub.spawn).toHaveBeenCalledTimes(1); + expect(processStub.spawn.mock.calls[0][0].onStdioLine).toBeUndefined(); + }); + it('startAdapterAndConnect should defer initialized and send launch before configurationDone when sendLaunchBeforeConfig is true', async () => { const payload: ProxyInitPayload = { cmd: 'init', diff --git a/tests/unit/proxy/dap-proxy-adapter-manager.test.ts b/tests/unit/proxy/dap-proxy-adapter-manager.test.ts index 605a99d4..37727ce5 100644 --- a/tests/unit/proxy/dap-proxy-adapter-manager.test.ts +++ b/tests/unit/proxy/dap-proxy-adapter-manager.test.ts @@ -227,6 +227,78 @@ describe('GenericAdapterManager', () => { }); }); + describe('stdio forwarding to onStdioLine (issue #222)', () => { + const REDACTED = '[REDACTED — line contained sensitive data]'; + let onStdioLine: ReturnType; + + beforeEach(async () => { + onStdioLine = vi.fn(); + await manager.spawn({ command: 'rdbg', args: ['--open'], logDir: '/logs', onStdioLine }); + }); + + it('forwards stdout lines to the callback and still logs them', () => { + mockProcess.stdout.emit('data', Buffer.from('6: Fizz\n')); + + expect(onStdioLine).toHaveBeenCalledWith('stdout', '6: Fizz'); + expect(logger.debug).toHaveBeenCalledWith('[AdapterManager STDOUT] 6: Fizz'); + }); + + it('forwards stderr lines to the callback and still logs them at error level', () => { + mockProcess.stderr.emit('data', Buffer.from('some warning\n')); + + expect(onStdioLine).toHaveBeenCalledWith('stderr', 'some warning'); + expect(logger.error).toHaveBeenCalledWith('[AdapterManager STDERR] some warning'); + }); + + it('forwards blank lines (program output) without logging them', () => { + mockProcess.stdout.emit('data', Buffer.from('\n\n')); + + expect(onStdioLine).toHaveBeenCalledTimes(2); + expect(onStdioLine).toHaveBeenCalledWith('stdout', ''); + expect(logger.debug).not.toHaveBeenCalledWith('[AdapterManager STDOUT] '); + }); + + it('forwards secret-bearing lines raw while the log copy is redacted', () => { + // The forwarded copy is the debuggee's own output as the debugging + // client must see it (parity with debugpy redirectOutput); only the + // persisted log line goes through whole-line redaction. + mockProcess.stdout.emit('data', Buffer.from('API_KEY=zzz-secret-value\n')); + + expect(onStdioLine).toHaveBeenCalledWith('stdout', 'API_KEY=zzz-secret-value'); + expect(logger.debug).toHaveBeenCalledWith(`[AdapterManager STDOUT] ${REDACTED}`); + }); + + it('joins a line that straddles two chunks into a single callback call', () => { + mockProcess.stdout.emit('data', Buffer.from('Hello, ')); + mockProcess.stdout.emit('data', Buffer.from('World!\n')); + + expect(onStdioLine).toHaveBeenCalledTimes(1); + expect(onStdioLine).toHaveBeenCalledWith('stdout', 'Hello, World!'); + }); + + it('flushes a trailing partial line exactly once when end is followed by close', () => { + mockProcess.stdout.emit('data', Buffer.from('no newline yet')); + expect(onStdioLine).not.toHaveBeenCalled(); + + mockProcess.stdout.emit('end'); + mockProcess.stdout.emit('close'); + + const matches = onStdioLine.mock.calls.filter(call => call[1] === 'no newline yet'); + expect(matches).toHaveLength(1); + }); + + it('changes nothing when no callback is configured', async () => { + const plainProcess = createMockProcess(777); + (spawner.spawn as any).mockReturnValue(plainProcess); + await manager.spawn({ command: 'python', args: [], logDir: '/logs' }); + + plainProcess.stdout.emit('data', Buffer.from('diag line\n')); + + expect(onStdioLine).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith('[AdapterManager STDOUT] diag line'); + }); + }); + describe('shutdown', () => { it('returns early for null process', async () => { await manager.shutdown(null);