diff --git a/docs/superpowers/plans/2026-04-11-subagent-wrapper-semantic-completion.md b/docs/superpowers/plans/2026-04-11-subagent-wrapper-semantic-completion.md new file mode 100644 index 0000000..fa1829a --- /dev/null +++ b/docs/superpowers/plans/2026-04-11-subagent-wrapper-semantic-completion.md @@ -0,0 +1,220 @@ +# Subagent Wrapper Semantic Completion Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ensure the subagent tool completes once the wrapper has seen terminal assistant output, even if the spawned `pi` child process lingers instead of exiting promptly. + +**Architecture:** Keep the fix on the smallest shared surface: `src/wrapper/cli.mjs`. Add a short grace-and-cleanup path that starts only after a terminal normalized `assistant_text` event, preserve the semantic result data already captured from that event, and leave monitor, runner selection, schema, and tmux helpers unchanged. Cover the behavior with a regression test in `src/wrapper/cli.test.ts` that reproduces the current stuck state. + +**Tech Stack:** Node.js `child_process`, `fs/promises`, ESM `.mjs`, `tsx --test`, Pi JSON event normalization + +--- + +## Worktree and file structure + +This plan assumes work happens in a dedicated worktree already created during brainstorming. + +**Modify** +- `src/wrapper/cli.test.ts` — add regression coverage for terminal output followed by a lingering child process +- `src/wrapper/cli.mjs` — add wrapper-internal grace period and forced cleanup after semantic completion + +**Do not modify** +- `src/monitor.ts` +- `src/process-runner.ts` +- `src/tmux-runner.ts` +- `src/tmux.ts` +- model/schema/tool registration files + +### Task 1: Finish lingering wrapper runs after terminal assistant output + +**Files:** +- Modify: `src/wrapper/cli.test.ts` +- Modify: `src/wrapper/cli.mjs` +- Test: `src/wrapper/cli.test.ts` + +- [ ] **Step 1: Write the failing regression test** + +Add this test near the other wrapper regression tests in `src/wrapper/cli.test.ts`: + +```ts +test("wrapper exits and writes result.json after terminal output even if the pi child lingers", async () => { + const dir = await mkdtemp(join(tmpdir(), "pi-subagents-wrapper-")); + const metaPath = join(dir, "meta.json"); + const resultPath = join(dir, "result.json"); + const piPath = join(dir, "pi"); + + await writeFile( + piPath, + [ + `#!${process.execPath}`, + "console.log(JSON.stringify({type:'message_end',message:{role:'assistant',content:[{type:'text',text:'done'}],model:'openai/gpt-5',stopReason:'stop'}}));", + "setTimeout(() => {}, 10_000);", + ].join("\n"), + "utf8", + ); + await chmod(piPath, 0o755); + + await writeFile( + metaPath, + JSON.stringify( + { + runId: "run-1", + mode: "single", + agent: "scout", + agentSource: "builtin", + task: "inspect auth", + cwd: dir, + requestedModel: "openai/gpt-5", + resolvedModel: "openai/gpt-5", + startedAt: "2026-04-09T00:00:00.000Z", + sessionPath: join(dir, "child-session.jsonl"), + eventsPath: join(dir, "events.jsonl"), + resultPath, + stdoutPath: join(dir, "stdout.log"), + stderrPath: join(dir, "stderr.log"), + transcriptPath: join(dir, "transcript.log"), + systemPromptPath: join(dir, "system-prompt.md"), + }, + null, + 2, + ), + "utf8", + ); + + const wrapperPath = join(dirname(fileURLToPath(import.meta.url)), "cli.mjs"); + const child = spawn(process.execPath, [wrapperPath, metaPath], { + env: { ...process.env, PATH: dir }, + stdio: ["ignore", "pipe", "pipe"], + }); + + const exitCode = await waitForExit(child); + assert.equal(exitCode, 0); + + const result = JSON.parse(await readFile(resultPath, "utf8")); + assert.equal(result.exitCode, 0); + assert.equal(result.stopReason, "stop"); + assert.equal(result.finalText, "done"); + assert.equal(result.resolvedModel, "openai/gpt-5"); +}); +``` + +- [ ] **Step 2: Run the wrapper tests to verify the new test fails for the right reason** + +Run: + +```bash +npx tsx --test src/wrapper/cli.test.ts +``` + +Expected: FAIL in the new test with `wrapper did not exit within 1500ms` from `waitForExit`, proving the wrapper still waits forever after terminal output when the child process lingers. + +- [ ] **Step 3: Add shared timing helpers at the top of the wrapper** + +Insert these declarations near the top of `src/wrapper/cli.mjs`, below the imports: + +```js +const SEMANTIC_EXIT_GRACE_MS = 250; +const FORCED_EXIT_GRACE_MS = 250; + +async function sleep(ms) { + await new Promise((resolve) => setTimeout(resolve, ms)); +} +``` + +- [ ] **Step 4: Wire terminal-event cleanup into `runWrapper` with the smallest shared change** + +Make these exact edits inside `src/wrapper/cli.mjs`. + +Add the new wrapper state immediately after the existing local state declarations inside `runWrapper`: + +```js + let childClosed = false; + let completionCleanupStarted = false; + let child; + + const forceChildExitAfterSemanticCompletion = async () => { + if (completionCleanupStarted) return; + completionCleanupStarted = true; + + await sleep(SEMANTIC_EXIT_GRACE_MS); + if (childClosed) return; + + child.kill("SIGTERM"); + + await sleep(FORCED_EXIT_GRACE_MS); + if (childClosed) return; + + child.kill("SIGKILL"); + }; +``` + +Change the `assistant_text` branch in `handleStdoutLine` to preserve the captured semantic result and trigger cleanup only when a terminal stop reason is present: + +```js + if (normalized.type === "assistant_text") { + finalText = normalized.text; + resolvedModel = normalized.model ?? resolvedModel; + stopReason = normalized.stopReason ?? stopReason; + usage = normalized.usage ?? usage; + + if (normalized.stopReason) { + void forceChildExitAfterSemanticCompletion(); + } + } +``` + +Change the child spawn assignment from `const child = spawn(...)` to: + +```js + child = spawn("pi", args, { + cwd: meta.cwd, + env: childEnv, + stdio: ["ignore", "pipe", "pipe"], + }); +``` + +Update the exit handlers so the cleanup helper can see when the child has already finished naturally: + +```js + child.on("error", (error) => { + spawnError = error; + childClosed = true; + finish(1); + }); + + child.on("close", (code) => { + childClosed = true; + finish(code ?? (spawnError ? 1 : 0)); + }); +``` + +This keeps all existing result-shaping logic in place, preserves best-effort artifact logging, preserves `PI_SUBAGENTS_CHILD=1`, and makes normal completion depend on the semantic terminal event rather than an indefinitely lingering process. + +- [ ] **Step 5: Run the wrapper tests again and verify they all pass** + +Run: + +```bash +npx tsx --test src/wrapper/cli.test.ts +``` + +Expected: PASS. In particular, `wrapper exits and writes result.json after terminal output even if the pi child lingers` should now pass, and the existing initiator / child-env / artifact-write regression tests should remain green. + +- [ ] **Step 6: Run the full package test suite** + +Run: + +```bash +npm test +``` + +Expected: all package tests PASS, including wrapper, runner, extension, and monitor coverage. + +- [ ] **Step 7: Commit the finished bug fix** + +Run: + +```bash +git add src/wrapper/cli.mjs src/wrapper/cli.test.ts +git commit -m "fix: finish lingering subagent wrapper runs" +```