diff --git a/CHANGELOG.md b/CHANGELOG.md index 2550554a0..c44d8734f 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 f4dad44f6..327678e47 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 a2b655059..9a29ba0a9 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 9862f4c5d..768f9620b 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/mcp-debugger/package.json b/packages/mcp-debugger/package.json index dd83893b9..368c18691 100644 --- a/packages/mcp-debugger/package.json +++ b/packages/mcp-debugger/package.json @@ -42,4 +42,4 @@ "@debugmcp/shared": "workspace:*" }, "author": "Sycamore LLC (https://github.com/debugmcp)" -} +} \ No newline at end of file diff --git a/packages/shared/src/interfaces/adapter-policy-ruby.ts b/packages/shared/src/interfaces/adapter-policy-ruby.ts index 078f0b7be..4bc2d0155 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 ce618cf04..a3e43efcb 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 65311bbaa..4d15a918f 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 53f270f6e..b16e0ac08 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 4f880214d..5483f191f 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 8efb0e176..a8c2e8b2a 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 811e88503..ec1dea78d 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 35a1a115c..d6fda4aab 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 d6ded29ba..c4828ce93 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 dfdf9db79..eb83cdb7b 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 bff29431d..5f807f960 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 5a530fe38..fa4874b34 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 d8bd08092..dfca9627f 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 605a99d4f..37727ce54 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);