From 74c279c60bcbcbb557353fd620fdbc8f276ae30d Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 26 Jul 2026 03:40:51 +0000 Subject: [PATCH 1/3] fix(coding-agent): route js-debug commands to the stopped script child js-debug launches are session trees: a threadless root launcher spawns child sessions (main script, [worker N]) via reverse startDebugging. Every stateful command routed through a single #activeSessionId set on the last registration or stop, so a worker attaching after the script stopped stole focus. threads listed only the worker, post-launch breakpoints read back as unbound, and step/continue/evaluate could not target the script thread. Focus now follows stops, not registrations: a new session claims the active pointer only when no live, stopped session already holds it. threads aggregates every live thread across the tree, skipping the threadless launcher while real children are alive. Fixes #6663 --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/src/dap/session.ts | 80 ++++++++++++++++--- .../test/debug/dap-multi-session.test.ts | 77 +++++++++++++++++- 3 files changed, 145 insertions(+), 13 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c4777f2be..05854fdc1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ - Fixed the Docker `natives-builder` stage failing to build releases ≥ 17.1.1: the native audio stack added bindgen (miniaudio needs libclang) and a bundled-opus CMake build (needs cmake + make), none of which were installed in the slim builder image. - Fixed `omp usage` duplicating org-less legacy accounts as "no usage data" rows whenever any sibling report carried an organization (mixed pools of pre-org-capture rows and fresh org-scoped logins): an org-less account is now covered by its own org-less report, while org-attributed sibling reports still never count as its coverage. - `omp usage` revalidates the broker credential snapshot before rendering: live usage reports were previously paired with a disk-cached account list up to an hour old, so a just-completed re-login (org-less row upserted to org-scoped) rendered as a phantom duplicate until the cache expired. +- 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..91daa3eb9 100644 --- a/packages/coding-agent/src/dap/session.ts +++ b/packages/coding-agent/src/dap/session.ts @@ -960,16 +960,41 @@ 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 is a threadless launcher + // and each real thread lives in a child (main script, `[worker N]`, …). + // Querying only the active session would surface just one child's threads, + // so aggregate across every live thread-owning session in the tree. + const targets = this.#threadOwningSessions(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; + for (const thread of threads) { + const key = `${thread.id}\0${thread.name}`; + if (seen.has(key)) continue; + seen.add(key); + merged.push(thread); + } + } + return { snapshot: buildSummary(anchor), threads: merged }; } async stackTrace( @@ -1371,7 +1396,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 +1727,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 +1766,27 @@ export class DapSessionManager { return sessions; } + /** + * Live sessions in `session`'s tree that can own threads. The threadless + * root launcher (a js-debug coordinator with live children) is dropped when + * any real child is alive, but kept as a last resort so a collapsed tree + * still has a target. + */ + #threadOwningSessions(session: DapSession): DapSession[] { + const live = this.#getTreeSessions(session).filter( + candidate => candidate.status !== "terminated" && candidate.client.isAlive(), + ); + if (live.length === 0) return [session]; + const nonLauncher = live.filter(candidate => !this.#isLauncherSession(candidate, live)); + return nonLauncher.length > 0 ? nonLauncher : live; + } + + /** A root session that has spawned a still-live child is a threadless launcher. */ + #isLauncherSession(session: DapSession, live: DapSession[]): boolean { + if (session.parentSessionId) return false; + return live.some(candidate => candidate.parentSessionId === session.id); + } + #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..1e4fa19a5 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}`); @@ -246,4 +260,63 @@ 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", + }); + // 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).toEqual( + expect.arrayContaining([ + { id: 1, name: "script.mts" }, + { id: 1, name: "[worker 1]" }, + ]), + ); + // The threadless launcher is never queried while real children are live. + expect(root.requests.filter(request => request.command === "threads")).toHaveLength(0); + + await manager.terminate(undefined, 1_000); + }); }); From 04f61e2125e053a017ead8ad480a9a6ab22ce019 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 26 Jul 2026 03:48:08 +0000 Subject: [PATCH 2/3] fix(coding-agent): aggregate debug threads without assuming a threadless root Review feedback: classifying every root-with-a-live-child as a threadless launcher dropped the root from thread aggregation, so a custom TCP adapter whose root process owns threads and also issues startDebugging would have its threads omitted. Drop the launcher heuristic: threads now fans out across all live sessions in the tree and merges results. A genuinely threadless launcher simply returns no threads (or an error we skip), so no topology guess is made. Fixes #6663 --- packages/coding-agent/src/dap/session.ts | 32 ++++++++----------- .../test/debug/dap-multi-session.test.ts | 27 ++++++++++------ 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/packages/coding-agent/src/dap/session.ts b/packages/coding-agent/src/dap/session.ts index 91daa3eb9..4ac44fb92 100644 --- a/packages/coding-agent/src/dap/session.ts +++ b/packages/coding-agent/src/dap/session.ts @@ -961,11 +961,13 @@ export class DapSessionManager { timeoutMs: number = 30_000, ): Promise<{ snapshot: DapSessionSummary; threads: DapThread[] }> { const anchor = this.#touchActiveSession(); - // A js-debug launch is a session tree: the root is a threadless launcher - // and each real thread lives in a child (main script, `[worker N]`, …). - // Querying only the active session would surface just one child's threads, - // so aggregate across every live thread-owning session in the tree. - const targets = this.#threadOwningSessions(anchor); + // 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) { @@ -1767,24 +1769,16 @@ export class DapSessionManager { } /** - * Live sessions in `session`'s tree that can own threads. The threadless - * root launcher (a js-debug coordinator with live children) is dropped when - * any real child is alive, but kept as a last resort so a collapsed tree - * still has a target. + * 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. */ - #threadOwningSessions(session: DapSession): DapSession[] { + #liveTreeSessions(session: DapSession): DapSession[] { const live = this.#getTreeSessions(session).filter( candidate => candidate.status !== "terminated" && candidate.client.isAlive(), ); - if (live.length === 0) return [session]; - const nonLauncher = live.filter(candidate => !this.#isLauncherSession(candidate, live)); - return nonLauncher.length > 0 ? nonLauncher : live; - } - - /** A root session that has spawned a still-live child is a threadless launcher. */ - #isLauncherSession(session: DapSession, live: DapSession[]): boolean { - if (session.parentSessionId) return false; - return live.some(candidate => candidate.parentSessionId === session.id); + return live.length > 0 ? live : [session]; } #touchSessionAndAncestors(session: DapSession): void { 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 1e4fa19a5..655b0605a 100644 --- a/packages/coding-agent/test/debug/dap-multi-session.test.ts +++ b/packages/coding-agent/test/debug/dap-multi-session.test.ts @@ -223,7 +223,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); here it just echoes the + // same thread, which dedupes away. + expect(root.requests.filter(request => request.command === "threads")).toHaveLength(1); await manager.terminate(undefined, 100); }); @@ -262,12 +264,18 @@ describe("DAP multi-session debugging", () => { }); 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", - }); + 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, { @@ -308,14 +316,15 @@ describe("DAP multi-session debugging", () => { // `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 threadless launcher is never queried while real children are live. - expect(root.requests.filter(request => request.command === "threads")).toHaveLength(0); + // 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); }); From 80df85a84b59a8b1f1215290a7303e129a9a6e27 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 26 Jul 2026 03:50:50 +0000 Subject: [PATCH 3/3] fix(coding-agent): key debug thread aggregation by owning session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback: DAP thread IDs are scoped per client session, so deduping the tree-wide thread list by (id, name) collapsed distinct threads that different children happen to share — e.g. multiple identical worker scripts each exposing { id: 1, name: "worker.js" }. Key the dedupe set by the owning session id + thread id instead, so every live thread is preserved and only an exact repeat within a single session's response is dropped. Fixes #6663 --- packages/coding-agent/src/dap/session.ts | 6 ++- .../test/debug/dap-multi-session.test.ts | 49 ++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/src/dap/session.ts b/packages/coding-agent/src/dap/session.ts index 4ac44fb92..a5c6b0234 100644 --- a/packages/coding-agent/src/dap/session.ts +++ b/packages/coding-agent/src/dap/session.ts @@ -989,8 +989,12 @@ export class DapSessionManager { 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 = `${thread.id}\0${thread.name}`; + const key = `${target.id}\0${thread.id}`; if (seen.has(key)) continue; seen.add(key); merged.push(thread); 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 655b0605a..5046ab257 100644 --- a/packages/coding-agent/test/debug/dap-multi-session.test.ts +++ b/packages/coding-agent/test/debug/dap-multi-session.test.ts @@ -210,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); @@ -223,8 +226,8 @@ 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); - // The root is queried too (no topology guess); here it just echoes the - // same thread, which dedupes away. + // 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); @@ -328,4 +331,46 @@ describe("DAP multi-session debugging", () => { 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); + }); });