From 07019e6c61a1ccc4a47604c3ec7168cc6fa89dfe Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 21 Jul 2026 20:57:14 +0000 Subject: [PATCH] fix(dap): close tcp connect race and wake pending requests on transport end #spawnTcp reserved a port, released it, spawned the adapter, then immediately connected. On WSL2 with networkingMode=mirrored the Windows relay keeps accepting connections to the just-released reservation port for tens of ms, so the first connect bound a ghost socket instead of js-debug. Gate the connect on the adapter announcing its listening port on stdout (as vscode-js-debug does from inside its listen callback) so we only connect once the child owns the port; this also drains stdout, which nothing else consumes in tcp mode. Separately, the message reader only rejected pending requests from its catch, so a clean stream end (the ghost socket dropped after we wrote initialize) left requests and event waiters pending until their own timeout. Route both the reader end and adapter exit through a single #failConnection that rejects pending requests and wakes event waiters, turning every transport failure into an immediate "DAP connection closed" error instead of a silent 30s timeout. Fixes #6055 --- packages/coding-agent/CHANGELOG.md | 4 + packages/coding-agent/src/dap/client.ts | 91 +++++++++++++-- .../test/debug/dap-launch-failures.test.ts | 106 ++++++++++++++++++ 3 files changed, 194 insertions(+), 7 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6f32dc885..07cbba024 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed JS/TS `debug` launches timing out on WSL2 with `networkingMode=mirrored` by waiting for the adapter's listening banner before connecting (avoiding the ghost-accept window on a just-released reservation port) and rejecting pending DAP requests and event waiters the moment the transport closes, so any transport failure surfaces as an immediate `DAP connection closed` error instead of a silent 30s timeout ([#6055](https://github.com/can1357/oh-my-pi/issues/6055)). + ## [17.0.5] - 2026-07-18 ### Added diff --git a/packages/coding-agent/src/dap/client.ts b/packages/coding-agent/src/dap/client.ts index c75879eca..8cb6f3864 100644 --- a/packages/coding-agent/src/dap/client.ts +++ b/packages/coding-agent/src/dap/client.ts @@ -75,6 +75,10 @@ export class DapClient { #reverseRequestHandlers = new Map(); #adapterExited = false; #pendingWriteExitRejectors = new Set<() => void>(); + /** Rejectors for in-flight {@link waitForEvent} calls, woken when the + * transport closes so an event that can never arrive fails fast instead of + * waiting out its own timeout. */ + #eventWaiterRejectors = new Set<(error: Error) => void>(); constructor( adapter: DapResolvedAdapter, @@ -188,12 +192,17 @@ export class DapClient { }); try { - const { readable, writeSink, socket } = await waitForTcpTransport( - host, - port, - socketReadyTimeoutMs ?? SOCKET_READY_TIMEOUT_MS, - proc, - ); + // Wait for the adapter to announce it is listening on `port` before + // connecting. Without this gate the first connect can land in the + // window where a just-released reservation port still accepts + // connections (WSL2 mirrored networking, issue #6055): the transport + // then binds to a ghost of the reservation listener instead of the + // adapter. Draining stdout here also avoids a pipe-buffer deadlock — + // in tcp mode the DAP protocol flows over the socket, so nothing else + // consumes the adapter's stdout. + const readyTimeoutMs = socketReadyTimeoutMs ?? SOCKET_READY_TIMEOUT_MS; + await waitForTcpServerListening(proc, port, readyTimeoutMs); + const { readable, writeSink, socket } = await waitForTcpTransport(host, port, readyTimeoutMs, proc); const client = new DapClient(adapter, cwd, proc, { readable, writeSink, socket, port }); proc.exited.then(() => client.#handleProcessExit()); void client.#startMessageReader(); @@ -389,6 +398,7 @@ export class DapClient { let timeout: NodeJS.Timeout | undefined; const cleanup = () => { unsubscribe(); + this.#eventWaiterRejectors.delete(closeHandler); if (timeout) clearTimeout(timeout); if (signal) { signal.removeEventListener("abort", abortHandler); @@ -398,6 +408,10 @@ export class DapClient { cleanup(); reject(signal?.reason instanceof Error ? signal.reason : new ToolAbortError()); }; + const closeHandler = (error: Error) => { + cleanup(); + reject(error); + }; const unsubscribe = this.onEvent(event, body => { const typedBody = body as TBody; if (predicate && !predicate(typedBody)) { @@ -406,6 +420,7 @@ export class DapClient { cleanup(); resolve(typedBody); }); + this.#eventWaiterRejectors.add(closeHandler); if (signal) { signal.addEventListener("abort", abortHandler, { once: true }); } @@ -580,6 +595,7 @@ export class DapClient { const framer = new MessageFramer(this.#messageBuffer); + let closeError: Error | undefined; try { while (true) { const { done, value } = await reader.read(); @@ -619,13 +635,19 @@ export class DapClient { } } } catch (error) { - this.#rejectPendingRequests(new Error(`DAP connection closed: ${toErrorMessage(error)}`)); + closeError = new Error(`DAP connection closed: ${toErrorMessage(error)}`); } finally { // Persist any unparsed remainder so a restarted reader resumes mid-message. this.#messageBuffer = framer.remainder(); reader.releaseLock(); this.#isReading = false; } + // The transport is gone once the reader loop exits — on a thrown error + // or a clean stream end (a socket the peer dropped after we wrote, e.g. + // the WSL2-mirrored ghost-accept race in issue #6055). Fail every + // in-flight request and event waiter so callers see an immediate error + // instead of waiting out their own timeout. + this.#failConnection(closeError ?? new Error(`DAP connection closed: ${this.adapter.name} transport ended`)); } #handleResponse(message: DapResponseMessage): void { @@ -712,7 +734,19 @@ export class DapClient { ? `DAP adapter exited (code ${exitCode}): ${stderr}` : `DAP adapter exited unexpectedly (code ${exitCode})`, ); + this.#failConnection(error); + } + + /** Reject every in-flight request and wake every event waiter with `error`. + * Called when the transport dies (reader end, socket close, adapter exit) + * so nothing sits pending until its own timeout. */ + #failConnection(error: Error): void { this.#rejectPendingRequests(error); + const waiters = Array.from(this.#eventWaiterRejectors); + this.#eventWaiterRejectors.clear(); + for (const reject of waiters) { + reject(error); + } } #rejectPendingRequests(error: Error): void { @@ -826,6 +860,49 @@ async function waitForTcpTransport( throw new Error(`TCP port ${host}:${port} was not ready after ${timeoutMs}ms`); } +/** + * Give the adapter a chance to announce it is listening on `port` before the + * first connect. vscode-js-debug prints `Debug server listening at HOST:PORT` + * to stdout from inside its `listen()` callback; waiting for the port to appear + * there means we only connect once the child genuinely owns the reserved port, + * which closes the WSL2-mirrored ghost-accept window (issue #6055) at its root. + * + * Best-effort: resolves on the banner, on process exit, or on timeout — the + * subsequent connect loop and `proc.exitCode` checks surface real failures, so + * an adapter that never prints a banner still proceeds (just without the gate). + * Also drains stdout for the wait's duration: in tcp mode the DAP protocol + * flows over the socket, so nothing else consumes the adapter's stdout. + */ +async function waitForTcpServerListening( + proc: { stdout: ReadableStream; exitCode: number | null }, + port: number, + timeoutMs: number, +): Promise { + const ready = Promise.withResolvers(); + const portText = String(port); + void (async () => { + try { + const decoder = new TextDecoder(); + let buffered = ""; + for await (const chunk of proc.stdout) { + buffered += decoder.decode(chunk, { stream: true }); + if (buffered.includes(portText)) { + ready.resolve(); + } + // Keep only the tail relevant for banner matching so a chatty + // adapter cannot grow this buffer without bound. + if (buffered.length > 4096) { + buffered = buffered.slice(-1024); + } + } + } catch { + /* stdout errored — the connect loop surfaces the real failure */ + } + ready.resolve(); + })(); + await Promise.race([ready.promise, Bun.sleep(timeoutMs)]); +} + interface SocketTransport { readable: ReadableStream; writeSink: DapWriteSink; diff --git a/packages/coding-agent/test/debug/dap-launch-failures.test.ts b/packages/coding-agent/test/debug/dap-launch-failures.test.ts index 6e1f63622..063f168ec 100644 --- a/packages/coding-agent/test/debug/dap-launch-failures.test.ts +++ b/packages/coding-agent/test/debug/dap-launch-failures.test.ts @@ -487,6 +487,112 @@ describe("DAP launch failure handling", () => { }); }); +describe("DAP TCP transport resilience", () => { + const TCP_ADAPTER_BASE: DapResolvedAdapter = { + ...TEST_ADAPTER, + name: "js-debug-adapter", + command: process.execPath, + resolvedCommand: process.execPath, + connectMode: "tcp", + }; + + // Adapter that binds the reserved port, accepts the first connection, then + // drops it after 30ms without answering — the WSL2-mirrored ghost socket. + const GHOST_ADAPTER = ` +const port = Number(process.argv[2]); +const server = Bun.listen({ hostname: "127.0.0.1", port, socket: { + open(s){ setTimeout(() => { try { s.end(); } catch {} }, 30); }, + data(){}, close(){}, error(){}, +}}); +console.log("Debug server listening at 127.0.0.1:" + port); +await Bun.sleep(60_000); +`; + + // Adapter that binds only after a delay, prints the listening banner from + // inside its listen callback, then answers initialize over the socket. + const DELAYED_BANNER_ADAPTER = ` +const port = Number(process.argv[2]); +await Bun.sleep(150); +const server = Bun.listen({ hostname: "127.0.0.1", port, socket: { + open(){}, + data(s, data){ + const text = Buffer.from(data).toString(); + const m = /Content-Length: (\\d+)\\r\\n\\r\\n([\\s\\S]*)/.exec(text); + if (!m) return; + const req = JSON.parse(m[2].slice(0, Number(m[1]))); + const resp = JSON.stringify({ seq: 1, type: "response", request_seq: req.seq, success: true, command: req.command, body: { supportsConfigurationDoneRequest: true } }); + s.write(\`Content-Length: \${Buffer.byteLength(resp)}\\r\\n\\r\\n\${resp}\`); + }, + close(){}, error(){}, +}}); +console.log("Debug server listening at 127.0.0.1:" + port); +await Bun.sleep(60_000); +`; + + async function withTcpAdapter( + source: string, + run: (adapter: DapResolvedAdapter, cwd: string) => Promise, + ): Promise { + const cwd = await fs.mkdtemp(path.join(os.tmpdir(), "omp-debug-tcp-")); + const adapterPath = path.join(cwd, "tcp-adapter.mjs"); + await fs.writeFile(adapterPath, source); + const adapter: DapResolvedAdapter = { + ...TCP_ADAPTER_BASE, + args: [adapterPath, "${port}", "127.0.0.1"], + }; + try { + await run(adapter, cwd); + } finally { + await removeWithRetries(cwd); + } + } + + it("rejects a pending request fast when the transport closes cleanly without answering", async () => { + await withTcpAdapter(GHOST_ADAPTER, async (adapter, cwd) => { + const client = await DapClient.spawn({ adapter, cwd, socketReadyTimeoutMs: 5_000 }); + try { + const start = Date.now(); + // The ghost socket ends the read stream cleanly. Before the fix the + // request sat pending until its own 5s timeout; now the reader wakes + // it with a descriptive connection-closed error well before then. + await expect(client.sendRequest("initialize", {}, undefined, 5_000)).rejects.toThrow( + /DAP connection closed/, + ); + expect(Date.now() - start).toBeLessThan(2_000); + } finally { + await client.dispose(); + } + }); + }); + + it("wakes an event waiter when the transport closes instead of waiting out its timeout", async () => { + await withTcpAdapter(GHOST_ADAPTER, async (adapter, cwd) => { + const client = await DapClient.spawn({ adapter, cwd, socketReadyTimeoutMs: 5_000 }); + try { + const start = Date.now(); + await expect(client.waitForEvent("stopped", undefined, undefined, 5_000)).rejects.toThrow( + /DAP connection closed/, + ); + expect(Date.now() - start).toBeLessThan(2_000); + } finally { + await client.dispose(); + } + }); + }); + + it("defers the first connect until the adapter announces its listening port", async () => { + await withTcpAdapter(DELAYED_BANNER_ADAPTER, async (adapter, cwd) => { + const client = await DapClient.spawn({ adapter, cwd, socketReadyTimeoutMs: 5_000 }); + try { + const caps = await client.initialize({ clientID: "omp", adapterID: "js-debug-adapter" }, undefined, 5_000); + expect(caps).toMatchObject({ supportsConfigurationDoneRequest: true }); + } finally { + await client.dispose(); + } + }); + }); +}); + describe("DebugTool launch validation", () => { it("rejects directory programs when the selected adapter cannot debug a directory", async () => { const launchSpy = spyOn(dapModule, "selectLaunchAdapter").mockReturnValue({