smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): aggregate debug threads without assuming a threadless root

Browse Source 
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
This commit is contained in:
roboomp
2026-07-26 03:48:08 +00:00
parent 74c279c60b
commit 04f61e2125
2 changed files with 31 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/dap/session.ts
+13 -19
View File
@@ -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<string>();
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 {
packages/coding-agent/test/debug/dap-multi-session.test.ts
+18 -9
View File
@@ -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);
});