From bd378960b26b33ea8406d45669a0e6eb4444b967 Mon Sep 17 00:00:00 2001 From: JB Date: Wed, 2 Sep 2026 20:50:21 -0400 Subject: [PATCH] fix(interactive-subagents): avoid Orca polling fan-out --- README.md | 4 +- bun.lock | 2 +- docs/npm-packages.md | 2 +- extensions/interactive-subagents/package.json | 2 +- .../pi-extension/subagents/orca.ts | 17 ++++- .../pi-extension/subagents/surface.ts | 38 +++++++---- extensions/interactive-subagents/test/test.ts | 66 ++++++++++++++++++- 7 files changed, 110 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index b63dbb9..38bc953 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ The currently published and deployed extension set is: - `@jbcode/pi-dictate@0.1.0`; - `@jbcode/pi-effort-controls@0.1.0`; - `@jbcode/pi-prompt-snippets@0.1.0`; -- `@jbcode/pi-interactive-subagents@3.7.2`; +- `@jbcode/pi-interactive-subagents@3.7.3`; - `@jbcode/pi-todo-herdr@0.2.5`. Configure the scoped Gitea npm registry and install only the packages you want. Keep the token in the user environment or user-level npm configuration; never commit it: @@ -143,7 +143,7 @@ Configure the scoped Gitea npm registry and install only the packages you want. pi install npm:@jbcode/pi-dictate@0.1.0 pi install npm:@jbcode/pi-effort-controls@0.1.0 pi install npm:@jbcode/pi-prompt-snippets@0.1.0 -pi install npm:@jbcode/pi-interactive-subagents@3.7.2 +pi install npm:@jbcode/pi-interactive-subagents@3.7.3 pi install npm:@jbcode/pi-todo-herdr@0.2.5 pi list ``` diff --git a/bun.lock b/bun.lock index 910845d..4d2ecb5 100644 --- a/bun.lock +++ b/bun.lock @@ -56,7 +56,7 @@ }, "extensions/interactive-subagents": { "name": "@jbcode/pi-interactive-subagents", - "version": "3.7.2", + "version": "3.7.3", "devDependencies": { "@earendil-works/pi-coding-agent": "catalog:", "@earendil-works/pi-tui": "catalog:", diff --git a/docs/npm-packages.md b/docs/npm-packages.md index 487237c..e77306a 100644 --- a/docs/npm-packages.md +++ b/docs/npm-packages.md @@ -52,7 +52,7 @@ After configuring the same scope and token in the npm configuration visible to P pi install npm:@jbcode/pi-dictate@0.1.0 pi install npm:@jbcode/pi-effort-controls@0.1.0 pi install npm:@jbcode/pi-prompt-snippets@0.1.0 -pi install npm:@jbcode/pi-interactive-subagents@3.7.2 +pi install npm:@jbcode/pi-interactive-subagents@3.7.3 pi install npm:@jbcode/pi-todo-herdr@0.2.5 ``` diff --git a/extensions/interactive-subagents/package.json b/extensions/interactive-subagents/package.json index cbb49bf..ad83d9c 100644 --- a/extensions/interactive-subagents/package.json +++ b/extensions/interactive-subagents/package.json @@ -1,6 +1,6 @@ { "name": "@jbcode/pi-interactive-subagents", - "version": "3.7.2", + "version": "3.7.3", "description": "Interactive subagents for Pi — spawn, orchestrate, and manage sub-agent sessions in Orca, Herdr or tmux.", "type": "module", "license": "MIT", diff --git a/extensions/interactive-subagents/pi-extension/subagents/orca.ts b/extensions/interactive-subagents/pi-extension/subagents/orca.ts index a82edd3..338631f 100644 --- a/extensions/interactive-subagents/pi-extension/subagents/orca.ts +++ b/extensions/interactive-subagents/pi-extension/subagents/orca.ts @@ -14,6 +14,7 @@ const execFileAsync = promisify(execFile); /** GNOME screen reader path — never a valid Orca CLI. */ const GNOME_SCREEN_READER = "/usr/bin/orca"; +const ASYNC_READ_SCREEN_TIMEOUT_MS = 5_000; let cachedCli: string | null | undefined; @@ -275,14 +276,26 @@ export function readScreen(surface: string, lines = 50): string { return parseReadTail(output); } -export async function readScreenAsync(surface: string, lines = 50): Promise { - const cli = requireOrca(); +async function readScreenWithCli( + cli: string, + surface: string, + lines: number, + timeout = ASYNC_READ_SCREEN_TIMEOUT_MS, +): Promise { const { stdout } = await execFileAsync(cli, readScreenArgs(surface, lines), { encoding: "utf8", + timeout, + killSignal: "SIGKILL", }); return parseReadTail(stdout); } +export async function readScreenAsync(surface: string, lines = 50): Promise { + return readScreenWithCli(requireOrca(), surface, lines); +} + +export const __orcaTest__ = { readScreenWithCli }; + export function closeSurface(surface: string): void { const cli = requireOrca(); const args = ["terminal", "close", "--terminal", surface]; diff --git a/extensions/interactive-subagents/pi-extension/subagents/surface.ts b/extensions/interactive-subagents/pi-extension/subagents/surface.ts index b96fe3f..c8c2d85 100644 --- a/extensions/interactive-subagents/pi-extension/subagents/surface.ts +++ b/extensions/interactive-subagents/pi-extension/subagents/surface.ts @@ -153,18 +153,28 @@ function readCompletionFile(path: string): PollResult | null { return { reason: "sentinel", exitCode }; } +type PollOptions = { + interval: number; + sessionFile?: string; + sentinelFile?: string; + completionFile?: string; + onTick?: (elapsed: number) => void; +}; + +/** Avoid launching a terminal CLI reader when a local completion signal is authoritative. */ +function shouldPollSurface( + options: Pick, +): boolean { + return !options.sessionFile && !options.sentinelFile && !options.completionFile; +} + export const __pollForExitTest__ = { interpretExitSidecar, readCompletionFile }; export async function pollForExit( surface: string, signal: AbortSignal, - options: { - interval: number; - sessionFile?: string; - sentinelFile?: string; - completionFile?: string; - onTick?: (elapsed: number) => void; - }, + options: PollOptions, + readSurface: (surface: string, lines?: number) => Promise = readScreenAsync, ): Promise { const start = Date.now(); @@ -197,12 +207,14 @@ export async function pollForExit( } catch {} } - try { - const screen = await readScreenAsync(surface, 5); - const match = screen.match(/__SUBAGENT_DONE_(\d+)__/); - if (match) return { reason: "sentinel", exitCode: Number.parseInt(match[1], 10) }; - } catch { - // Surface reads are best-effort. Filesystem completion remains authoritative. + if (shouldPollSurface(options)) { + try { + const screen = await readSurface(surface, 5); + const match = screen.match(/__SUBAGENT_DONE_(\d+)__/); + if (match) return { reason: "sentinel", exitCode: Number.parseInt(match[1], 10) }; + } catch { + // Surface reads are a best-effort fallback when no completion file is available. + } } const elapsed = Math.floor((Date.now() - start) / 1000); diff --git a/extensions/interactive-subagents/test/test.ts b/extensions/interactive-subagents/test/test.ts index b33020f..b6f8907 100644 --- a/extensions/interactive-subagents/test/test.ts +++ b/extensions/interactive-subagents/test/test.ts @@ -1,6 +1,14 @@ import { describe, it, before, after, beforeEach } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, writeFileSync, readFileSync, mkdirSync, rmSync, existsSync } from "node:fs"; +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; import { fileURLToPath } from "node:url"; @@ -32,11 +40,13 @@ import { import { __pollForExitTest__, + pollForExit, selectMuxBackend, shellEscape, } from "../pi-extension/subagents/surface.ts"; import { parseCreatedPane, parseCreatedTab } from "../pi-extension/subagents/herdr.ts"; import { + __orcaTest__, parseCreatedSplit, parseCreatedTerminal, parseReadTail, @@ -1812,6 +1822,60 @@ describe("subagent-done.ts", () => { describe("surface exit signals", () => { const { interpretExitSidecar, readCompletionFile } = __pollForExitTest__; + it("does not poll the terminal while waiting for a filesystem completion signal", async () => { + const dir = createTestDir(); + const completionFile = join(dir, "child.exit-code"); + let surfaceReads = 0; + + try { + const result = await pollForExit( + "unused-surface", + new AbortController().signal, + { + interval: 1, + completionFile, + onTick() { + writeFileSync(completionFile, "7\n"); + }, + }, + async () => { + surfaceReads += 1; + return "__SUBAGENT_DONE_0__"; + }, + ); + + assert.deepEqual(result, { reason: "sentinel", exitCode: 7 }); + assert.equal(surfaceReads, 0); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it("bounds an asynchronous Orca screen read and kills its CLI process", async () => { + const dir = createTestDir(); + const pidFile = join(dir, "reader.pid"); + const cli = join(dir, "fake-orca"); + writeFileSync( + cli, + `#!/usr/bin/env node\nrequire("node:fs").writeFileSync(${JSON.stringify(pidFile)}, String(process.pid));\nsetInterval(() => {}, 1000);\n`, + ); + chmodSync(cli, 0o755); + + try { + await assert.rejects(__orcaTest__.readScreenWithCli(cli, "unused-surface", 5, 500)); + const pid = Number.parseInt(readFileSync(pidFile, "utf8"), 10); + let running = true; + try { + process.kill(pid, 0); + } catch { + running = false; + } + assert.equal(running, false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + it("reads and removes a shell completion file", () => { withTempDir((dir) => { const completionFile = join(dir, "child.exit-code");