From 71cfb97456fa032d502efc694d0d0e7fc6d31312 Mon Sep 17 00:00:00 2001 From: pi Date: Sun, 12 Apr 2026 14:24:00 +0100 Subject: [PATCH] fix: register background tools, session replay, background persistence, and tests/docs for Task 5 spec-review --- README.md | 26 ++++++++++++++++- index.ts | 45 ++++++++++++++++++++---------- src/background-status-tool.test.ts | 4 +++ src/extension.test.ts | 33 ++++++++++++++-------- 4 files changed, 82 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 99de13a..0d47fdd 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,31 @@ pi install https://gitea.rwiesner.com/pi/pi-subagents - selected model - optional working directory -Child runs are normal Pi sessions. This package does not add built-in role behavior, markdown-discovered subagents, per-agent tool restrictions, or appended role prompts. +Child runs are normal Pi sessions. + +## Presets + +Subagent presets are discovered from markdown files. Locations: + +- global: `~/.pi/agent/subagents/*.md` +- nearest project: `.pi/subagents/*.md` (nearest ancestor directory) + +Project presets override global presets by name. No built-in presets or roles are bundled with this package. + +Preset frontmatter fields: + +- `name` (required) +- `description` (required) +- `model` (optional; provider/id) +- `tools` (optional; comma-separated list or array) + +Preset body becomes the child session system prompt. The `tools` field maps directly to Pi CLI `--tools` and limits which built-in tools are available to the child run. + +## Tools provided + +- `subagent` — foreground single or parallel runs using named presets +- `background_agent` — detached process-backed run that returns immediately with a handle +- `background_agent_status` — query background run counts and status ## Requirements diff --git a/index.ts b/index.ts index 7d841e2..6adaf1b 100644 --- a/index.ts +++ b/index.ts @@ -3,7 +3,7 @@ import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { createRunArtifacts } from "./src/artifacts.ts"; import { loadSubagentsConfig } from "./src/config.ts"; -import { monitorRun } from "./src/monitor.ts"; +import { monitorRun as defaultMonitorRun } from "./src/monitor.ts"; import { listAvailableModelReferences } from "./src/models.ts"; import { createProcessSingleRunner } from "./src/process-runner.ts"; import { createConfiguredRunSingleTask } from "./src/runner.ts"; @@ -18,10 +18,17 @@ import { isInsideTmux, } from "./src/tmux.ts"; +// background imports +import { createBackgroundRegistry } from "./src/background-registry.ts"; +import { createBackgroundAgentTool } from "./src/background-tool.ts"; +import { createBackgroundStatusTool } from "./src/background-status-tool.ts"; +import { createBackgroundAgentSchema } from "./src/background-schema.ts"; +import { discoverSubagentPresets } from "./src/presets.ts"; + const packageRoot = dirname(fileURLToPath(import.meta.url)); const wrapperPath = join(packageRoot, "src", "wrapper", "cli.mjs"); -export default function subagentsExtension(pi: ExtensionAPI) { +export default function subagentsExtension(pi: ExtensionAPI, deps: any = {}) { if (process.env.PI_SUBAGENTS_GITHUB_COPILOT_INITIATOR === "agent") { pi.registerProvider("github-copilot", { headers: { "X-Initiator": "agent" }, @@ -37,6 +44,8 @@ export default function subagentsExtension(pi: ExtensionAPI) { let lastRegisteredModelsKey: string | undefined; + const monitorRun = deps.monitorRun ?? defaultMonitorRun; + const tmuxRunner = createTmuxSingleRunner({ assertInsideTmux() { if (!isInsideTmux()) throw new Error('tmux runner requires pi to be running inside tmux.'); @@ -68,11 +77,10 @@ export default function subagentsExtension(pi: ExtensionAPI) { }); // background registry and helpers - const registry = (typeof createBackgroundRegistry === 'function') ? createBackgroundRegistry() : undefined; + const registry = createBackgroundRegistry(); let latestUi: { notify(message: string, type?: "info" | "warning" | "error"): void; setStatus(key: string, text: string | undefined): void } | undefined; function renderCounts() { - if (!registry) return undefined; const counts = registry.getCounts(); return counts.total === 0 ? undefined : `bg: ${counts.running} running / ${counts.total} total`; } @@ -82,7 +90,6 @@ export default function subagentsExtension(pi: ExtensionAPI) { } async function watchBackgroundRun(runId: string) { - if (!registry) return; const run = registry.getRun(runId); if (!run || !run.paths || !run.paths.eventsPath || !run.paths.resultPath) return; try { @@ -147,8 +154,12 @@ export default function subagentsExtension(pi: ExtensionAPI) { discoverSubagentPresets, launchDetachedTask: processRunner.launchDetachedTask, registerBackgroundRun(entry: any) { - if (!registry) return { running: 0, completed: 0, failed: 0, aborted: 0, total: 0 }; registry.recordLaunch({ runId: entry.runId, preset: entry.preset, task: entry.task, requestedModel: entry.requestedModel, resolvedModel: entry.resolvedModel, paths: entry.paths, meta: entry.meta }); + const run = registry.getRun(entry.runId); + try { + // persist initial run record so session manager can rebuild later + pi.appendEntry("pi-subagents:bg-run", { run }); + } catch (e) {} return registry.getCounts(); }, watchBackgroundRun(runId: string) { @@ -167,20 +178,26 @@ export default function subagentsExtension(pi: ExtensionAPI) { // replay persisted runs if session manager provides entries try { const entries = ctx.sessionManager?.getEntries?.() ?? []; - const bgEntries = entries.filter((e: any) => e.type === "pi-subagents:bg-run" || e.type === "pi-subagents:bg-update"); - // convert to registry runs if possible - const runs = bgEntries.map((be: any) => be.data?.run).filter(Boolean); - if (registry && runs.length) registry.replay(runs); + // clear existing registry and rebuild from session entries + registry.replay([]); + for (const e of entries) { + const type = (e.type ?? e.customType) as string | undefined; + if (type === "pi-subagents:bg-run") { + const run = e.data?.run ?? e.data; + if (run && run.runId) registry.recordLaunch(run as any); + } else if (type === "pi-subagents:bg-update") { + const data = e.data; + if (data && data.runId) registry.recordUpdate(data.runId, data as any); + } + } } catch (e) {} updateStatus(); // reattach watchers for running runs try { - if (registry) { - for (const r of registry.getSnapshot({ includeCompleted: true })) { - if (r.status === "running") void watchBackgroundRun(r.runId); - } + for (const r of registry.getSnapshot({ includeCompleted: true })) { + if (r.status === "running") void watchBackgroundRun(r.runId); } } catch (e) {} diff --git a/src/background-status-tool.test.ts b/src/background-status-tool.test.ts index d760eef..bb4a28d 100644 --- a/src/background-status-tool.test.ts +++ b/src/background-status-tool.test.ts @@ -15,11 +15,15 @@ test("status tool shows active runs by default and can query single run", async assert.equal(resDefault.details.runs.length, 1); assert.equal(resDefault.details.runs[0].runId, "r1"); assert.match(resDefault.content[0].text, /Active runs: 1/); + // assert counts appear in details and text + assert.deepEqual(resDefault.details.counts, { running: 1, completed: 1, failed: 0, aborted: 0, total: 2 }); + assert.match(resDefault.content[0].text, /running=1\s+completed=1\s+failed=0\s+aborted=0\s+total=2/); const resSingle: any = await tool.execute("id", { runId: "r2" }, undefined, undefined, undefined); assert.equal(resSingle.details.runs.length, 1); assert.equal(resSingle.details.runs[0].runId, "r2"); assert.match(resSingle.content[0].text, /status=completed/); + assert.deepEqual(resSingle.details.counts, { running: 1, completed: 1, failed: 0, aborted: 0, total: 2 }); const resNotFound: any = await tool.execute("id", { runId: "nope" }, undefined, undefined, undefined); assert.equal(resNotFound.details.runs.length, 0); diff --git a/src/extension.test.ts b/src/extension.test.ts index 48e3abd..2bc01ef 100644 --- a/src/extension.test.ts +++ b/src/extension.test.ts @@ -34,8 +34,13 @@ test("the extension entrypoint registers the subagent tool with the currently av }, ); - assert.equal(registeredTools.length, 1); + // expect three tools: subagent, background_agent_status, background_agent + assert.equal(registeredTools.length, 3); assert.equal(registeredTools[0]?.name, "subagent"); + assert.equal(registeredTools[1]?.name, "background_agent_status"); + assert.equal(registeredTools[2]?.name, "background_agent"); + + // subagent schema checks (same as before) const params = registeredTools[0]?.parameters; const branches = params?.anyOf ?? params?.oneOf ?? [params]; // ensure union branches exist: single-mode and parallel-mode @@ -57,6 +62,13 @@ test("the extension entrypoint registers the subagent tool with the currently av "anthropic/claude-sonnet-4-5", "openai/gpt-5", ]); + + // background_agent schema should include dynamic model enum + const bgAgent = registeredTools.find((t: any) => t.name === "background_agent"); + assert(bgAgent, "background_agent registered"); + const bgParams = bgAgent.parameters; + assert.equal(bgParams.properties.model.enum[0], "anthropic/claude-sonnet-4-5"); + assert.equal(bgParams.properties.model.enum[1], "openai/gpt-5"); } finally { if (original === undefined) delete process.env.PI_SUBAGENTS_CHILD; else process.env.PI_SUBAGENTS_CHILD = original; @@ -97,7 +109,7 @@ test("before_agent_start re-applies subagent registration when available models }, ); - assert.equal(registeredTools.length, 1); + assert.equal(registeredTools.length, 3); { const params = registeredTools[0]?.parameters; const branches = params?.anyOf ?? params?.oneOf ?? [params]; @@ -121,9 +133,9 @@ test("before_agent_start re-applies subagent registration when available models }, ); - assert.equal(registeredTools.length, 2); + assert.equal(registeredTools.length, 6); { - const params = registeredTools[1]?.parameters; + const params = registeredTools[3]?.parameters; const branches = params?.anyOf ?? params?.oneOf ?? [params]; const parallelBranch = branches.find((b: any) => b.properties && "tasks" in b.properties); assert(parallelBranch, "parallel branch present"); @@ -270,7 +282,7 @@ test("does not re-register the subagent tool when models list unchanged, but re- }, ); - assert.equal(registerToolCalls, 1); + assert.equal(registerToolCalls, 3); // Second registration with the same models — should not increase count await handlers.before_agent_start?.( @@ -285,7 +297,7 @@ test("does not re-register the subagent tool when models list unchanged, but re- }, ); - assert.equal(registerToolCalls, 1); + assert.equal(registerToolCalls, 3); // Third call with changed model list — should re-register await handlers.session_start?.( @@ -299,7 +311,7 @@ test("does not re-register the subagent tool when models list unchanged, but re- }, ); - assert.equal(registerToolCalls, 2); + assert.equal(registerToolCalls, 6); } finally { if (original === undefined) delete process.env.PI_SUBAGENTS_CHILD; else process.env.PI_SUBAGENTS_CHILD = original; @@ -343,7 +355,7 @@ test("same model set in different orders should NOT trigger re-registration", as }, ); - assert.equal(registerToolCalls, 1); + assert.equal(registerToolCalls, 3); // Same models but reversed order — should NOT re-register await handlers.before_agent_start?.( @@ -358,7 +370,7 @@ test("same model set in different orders should NOT trigger re-registration", as }, ); - assert.equal(registerToolCalls, 1); + assert.equal(registerToolCalls, 3); } finally { if (original === undefined) delete process.env.PI_SUBAGENTS_CHILD; else process.env.PI_SUBAGENTS_CHILD = original; @@ -411,10 +423,9 @@ test("empty model list should NOT register the tool, but a later non-empty list }, ); - assert.equal(registerToolCalls, 1); + assert.equal(registerToolCalls, 3); } finally { if (original === undefined) delete process.env.PI_SUBAGENTS_CHILD; else process.env.PI_SUBAGENTS_CHILD = original; } }); -