diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index fd2fe0217..3e3e68806 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -49,6 +49,7 @@ ### Fixed - Corrected Windows shell resolution errors to identify the active global, project, overlay, or runtime source for `shellPath`, including profile and custom configuration directories, instead of directing every user to the retired `settings.json` file ([#6579](https://github.com/can1357/oh-my-pi/issues/6579)). +- Fixed `debug` (js-debug/`pwa-node`) stateful commands misrouting after launch: a lazily-attached `[worker N]` child session (or the threadless root launcher) would steal the active-session focus from the stopped script child, so `threads` listed only the worker thread, post-launch breakpoints read back as pending/unbound, and there was no way to step/continue/evaluate the script's thread. Focus now follows stops rather than registrations, and `threads` aggregates every live thread across the session tree ([#6663](https://github.com/can1357/oh-my-pi/issues/6663)). ## [17.1.3] - 2026-07-24 diff --git a/packages/coding-agent/src/dap/session.ts b/packages/coding-agent/src/dap/session.ts index 02f7b72e9..a5c6b0234 100644 --- a/packages/coding-agent/src/dap/session.ts +++ b/packages/coding-agent/src/dap/session.ts @@ -960,16 +960,47 @@ export class DapSessionManager { signal?: AbortSignal, timeoutMs: number = 30_000, ): Promise<{ snapshot: DapSessionSummary; threads: DapThread[] }> { - const session = this.#touchActiveSession(); - const response = await this.#sendRequestWithConfig( - session, - "threads", - undefined, - signal, - timeoutMs, - ); - session.threads = response?.threads ?? []; - return { snapshot: buildSummary(session), threads: session.threads }; + const anchor = this.#touchActiveSession(); + // A js-debug launch is a session tree: the root may be a threadless + // launcher while each real thread lives in a child (main script, + // `[worker N]`, …), and other adapters keep every thread on the root. + // Querying only the active session would surface just one session's + // threads, so aggregate across the whole live tree. No topology guess: + // a threadless launcher simply returns no threads (or an error we skip). + const targets = this.#liveTreeSessions(anchor); + const merged: DapThread[] = []; + const seen = new Set(); + for (const target of targets) { + let threads: DapThread[]; + try { + const response = await this.#sendRequestWithConfig( + target, + "threads", + undefined, + signal, + timeoutMs, + ); + threads = response?.threads ?? []; + } catch (error) { + logger.warn("Failed to list threads for debug session", { + sessionId: target.id, + error: toErrorMessage(error), + }); + continue; + } + target.threads = threads; + // DAP thread IDs are scoped per client session, so identical IDs from + // different sessions (e.g. two identical worker scripts) are distinct + // live threads and MUST be preserved; only collapse an exact repeat + // within a single session's response. + for (const thread of threads) { + const key = `${target.id}\0${thread.id}`; + if (seen.has(key)) continue; + seen.add(key); + merged.push(thread); + } + } + return { snapshot: buildSummary(anchor), threads: merged }; } async stackTrace( @@ -1371,7 +1402,13 @@ export class DapSessionManager { if (parentSessionId) { this.#sessions.get(parentSessionId)?.childSessionIds.add(session.id); } - this.#activeSessionId = session.id; + // Focus follows stops, not registrations: a lazily-attached child (e.g. a + // js-debug `[worker N]` session) must not steal focus from a sibling that + // is already stopped at a breakpoint / entry. Only claim focus when no + // live, stopped session currently holds it. + if (!this.#hasLiveStoppedActiveSession()) { + this.#activeSessionId = session.id; + } const heartbeat = setInterval(() => { if (!client.isAlive()) { session.status = "terminated"; @@ -1696,6 +1733,12 @@ export class DapSessionManager { return session; } + /** True when the current active session is live and paused at a stop. */ + #hasLiveStoppedActiveSession(): boolean { + const active = this.#getActiveSessionOrNull(); + return active !== null && active.status === "stopped" && active.client.isAlive(); + } + #getActiveSessionOrThrow(): DapSession { const session = this.#getActiveSessionOrNull(); if (!session) { @@ -1729,6 +1772,19 @@ export class DapSessionManager { return sessions; } + /** + * Live (non-terminated, connected) sessions in `session`'s tree, or the + * session itself when the tree has collapsed. Used to fan `threads` out + * across the whole tree; a threadless session just reports no threads, so + * this makes no assumption about which node owns them. + */ + #liveTreeSessions(session: DapSession): DapSession[] { + const live = this.#getTreeSessions(session).filter( + candidate => candidate.status !== "terminated" && candidate.client.isAlive(), + ); + return live.length > 0 ? live : [session]; + } + #touchSessionAndAncestors(session: DapSession): void { const now = Date.now(); let current: DapSession | undefined = session; diff --git a/packages/coding-agent/test/debug/dap-multi-session.test.ts b/packages/coding-agent/test/debug/dap-multi-session.test.ts index 25ca1c1eb..5046ab257 100644 --- a/packages/coding-agent/test/debug/dap-multi-session.test.ts +++ b/packages/coding-agent/test/debug/dap-multi-session.test.ts @@ -6,6 +6,7 @@ import type { DapClientState, DapEventMessage, DapResolvedAdapter, + DapThread, } from "@oh-my-pi/pi-coding-agent/dap/types"; const TEST_ADAPTER: DapResolvedAdapter = { @@ -25,6 +26,13 @@ const TEST_ADAPTER: DapResolvedAdapter = { type EventHandler = (body: unknown, event: DapEventMessage) => void | Promise; type ReverseHandler = (args: unknown) => unknown | Promise; +interface FakeOptions { + /** Threads returned by this session's `threads` request. */ + threads?: DapThread[]; + /** Thread id reported by the synthetic `stopped` event (defaults to 7). */ + stopThreadId?: number; +} + class FakeDapClient { readonly proc: DapClientState["proc"]; readonly port = 8123; @@ -39,6 +47,7 @@ class FakeDapClient { readonly childConfiguration?: Record, readonly childRequest: "launch" | "attach" = "launch", readonly stopOnStart = true, + readonly options: FakeOptions = {}, ) { this.proc = { exited: this.#exited.promise, @@ -71,10 +80,10 @@ class FakeDapClient { }); }); } else if (this.stopOnStart) { - queueMicrotask(() => this.#emit("stopped", { reason: "entry", threadId: 7 })); + queueMicrotask(() => this.#emit("stopped", { reason: "entry", threadId: this.options.stopThreadId ?? 7 })); } } - if (command === "threads") return { threads: [{ id: 7, name: "target.js" }] }; + if (command === "threads") return { threads: this.options.threads ?? [{ id: 7, name: "target.js" }] }; if (command === "stackTrace") { return { stackFrames: [{ id: 70, name: "main", line: 2, column: 1, source: { path: "/tmp/target.js" } }], @@ -127,6 +136,11 @@ class FakeDapClient { this.#emit(event, body); } + /** Drive an adapter-initiated reverse request (e.g. a late `startDebugging`). */ + async triggerReverse(command: string, args: unknown): Promise { + await this.#emitReverse(command, args); + } + async #emitReverse(command: string, args: unknown): Promise { const handler = this.#reverseHandlers.get(command); if (!handler) throw new Error(`Missing reverse handler for ${command}`); @@ -196,6 +210,9 @@ describe("DAP multi-session debugging", () => { __pendingTargetId: "attached-child", }, "attach", + true, + // Threadless launcher: answers `threads` with an empty list. + { threads: [] }, ); const child = new FakeDapClient(undefined, "launch", false); spyOn(DapClient, "spawn").mockResolvedValue(root as unknown as DapClient); @@ -209,7 +226,9 @@ describe("DAP multi-session debugging", () => { expect(active?.parentSessionId).toBeDefined(); expect(threads.threads).toEqual([{ id: 7, name: "target.js" }]); expect(child.requests.filter(request => request.command === "threads")).toHaveLength(1); - expect(root.requests.filter(request => request.command === "threads")).toHaveLength(0); + // The root is queried too (no topology guess), but being threadless it + // contributes nothing. + expect(root.requests.filter(request => request.command === "threads")).toHaveLength(1); await manager.terminate(undefined, 100); }); @@ -246,4 +265,112 @@ describe("DAP multi-session debugging", () => { await manager.terminate(undefined, 100); }); + + it("keeps focus on the stopped script child when a worker attaches later", async () => { + const root = new FakeDapClient( + { + name: "script.mts", + type: "pwa-node", + __pendingTargetId: "main", + program: "/tmp/script.mts", + }, + "launch", + true, + // Threadless launcher: it answers `threads` with an empty list. + { threads: [] }, + ); + // The script child stops on entry (thread 1), then a worker session + // attaches afterwards via a late reverse `startDebugging`. + const main = new FakeDapClient(undefined, "launch", true, { + threads: [{ id: 1, name: "script.mts" }], + stopThreadId: 1, + }); + const worker = new FakeDapClient(undefined, "launch", false, { + threads: [{ id: 1, name: "[worker 1]" }], + }); + const children = [main, worker]; + spyOn(DapClient, "spawn").mockResolvedValue(root as unknown as DapClient); + spyOn(DapClient, "connect").mockImplementation(async () => { + const next = children.shift(); + if (!next) throw new Error("Unexpected child DAP connection"); + return next as unknown as DapClient; + }); + const manager = new DapSessionManager(); + + const launched = await manager.launch( + { adapter: TEST_ADAPTER, program: "/tmp/script.mts", cwd: "/tmp" }, + undefined, + 1_000, + ); + expect(launched.status).toBe("stopped"); + const scriptSessionId = launched.id; + + // A worker_threads spawn triggers a late child attach on the launcher. + await root.triggerReverse("startDebugging", { + request: "launch", + configuration: { name: "[worker 1]", type: "pwa-node" }, + }); + expect(manager.listSessions()).toHaveLength(3); + + // Focus must stay on the stopped script child, not jump to the worker. + const active = manager.getActiveSession(); + expect(active?.id).toBe(scriptSessionId); + expect(active?.threadId).toBe(1); + + // `threads` must surface every live thread across the tree, not just one. + const threads = await manager.threads(undefined, 1_000); + expect(threads.threads).toHaveLength(2); + expect(threads.threads).toEqual( + expect.arrayContaining([ + { id: 1, name: "script.mts" }, + { id: 1, name: "[worker 1]" }, + ]), + ); + // The launcher is still queried, but being threadless it contributes none. + expect(root.requests.filter(request => request.command === "threads")).toHaveLength(1); + + await manager.terminate(undefined, 1_000); + }); + + it("preserves per-session threads that share an id and name across children", async () => { + const root = new FakeDapClient( + { name: "pool.mjs", type: "pwa-node", __pendingTargetId: "main", program: "/tmp/pool.mjs" }, + "launch", + true, + { threads: [] }, + ); + // Two identical worker scripts each expose the same session-local thread + // id and name; DAP scopes ids per session, so both are distinct threads. + const main = new FakeDapClient(undefined, "launch", true, { + threads: [{ id: 1, name: "worker.js" }], + stopThreadId: 1, + }); + const worker = new FakeDapClient(undefined, "launch", false, { + threads: [{ id: 1, name: "worker.js" }], + }); + const children = [main, worker]; + spyOn(DapClient, "spawn").mockResolvedValue(root as unknown as DapClient); + spyOn(DapClient, "connect").mockImplementation(async () => { + const next = children.shift(); + if (!next) throw new Error("Unexpected child DAP connection"); + return next as unknown as DapClient; + }); + const manager = new DapSessionManager(); + + await manager.launch({ adapter: TEST_ADAPTER, program: "/tmp/pool.mjs", cwd: "/tmp" }, undefined, 1_000); + await root.triggerReverse("startDebugging", { + request: "launch", + configuration: { name: "worker #2", type: "pwa-node" }, + }); + expect(manager.listSessions()).toHaveLength(3); + + const threads = await manager.threads(undefined, 1_000); + // Both identical threads survive aggregation \u2014 not collapsed into one. + expect(threads.threads).toEqual([ + { id: 1, name: "worker.js" }, + { id: 1, name: "worker.js" }, + ]); + + await manager.terminate(undefined, 1_000); + }); });