From 9427ad8f9462435073ff87640e470ac1969b6fc6 Mon Sep 17 00:00:00 2001 From: Roger Navelsaker Date: Fri, 20 Mar 2026 14:29:48 +0100 Subject: [PATCH] feat: add pi session metadata and ready marker --- src/agents/session-env.ts | 30 ++ src/commands/coordinator.ts | 22 +- src/commands/monitor.ts | 19 +- src/commands/sling.ts | 45 +-- src/commands/supervisor.ts | 22 +- src/runtimes/pi-guards.test.ts | 486 --------------------------------- src/runtimes/pi-guards.ts | 366 ------------------------- src/runtimes/pi.test.ts | 97 ++----- src/runtimes/pi.ts | 54 ++-- src/runtimes/sapling.ts | 2 +- src/runtimes/types.ts | 12 +- 11 files changed, 133 insertions(+), 1022 deletions(-) create mode 100644 src/agents/session-env.ts delete mode 100644 src/runtimes/pi-guards.test.ts delete mode 100644 src/runtimes/pi-guards.ts diff --git a/src/agents/session-env.ts b/src/agents/session-env.ts new file mode 100644 index 00000000..e65929b6 --- /dev/null +++ b/src/agents/session-env.ts @@ -0,0 +1,30 @@ +export type OverstorySessionKind = + | "standalone" + | "orchestrator" + | "coordinator" + | "monitor" + | "worker"; + +export interface OverstorySessionEnvOpts { + baseEnv?: Record; + sessionKind: OverstorySessionKind; + agentName: string; + capability: string; + worktreePath: string; + projectRoot: string; + taskId?: string; + profile?: string; +} + +export function buildOverstorySessionEnv(opts: OverstorySessionEnvOpts): Record { + return { + ...(opts.baseEnv ?? {}), + OVERSTORY_SESSION_KIND: opts.sessionKind, + OVERSTORY_AGENT_NAME: opts.agentName, + OVERSTORY_CAPABILITY: opts.capability, + OVERSTORY_WORKTREE_PATH: opts.worktreePath, + OVERSTORY_PROJECT_ROOT: opts.projectRoot, + ...(opts.taskId !== undefined ? { OVERSTORY_TASK_ID: opts.taskId } : {}), + ...(opts.profile !== undefined ? { OVERSTORY_PROFILE: opts.profile } : {}), + }; +} diff --git a/src/commands/coordinator.ts b/src/commands/coordinator.ts index a3e5fc1c..4ce86b7b 100644 --- a/src/commands/coordinator.ts +++ b/src/commands/coordinator.ts @@ -17,6 +17,7 @@ import { join } from "node:path"; import { Command } from "commander"; import { createIdentity, loadIdentity } from "../agents/identity.ts"; import { createManifestLoader, resolveModel } from "../agents/manifest.ts"; +import { buildOverstorySessionEnv } from "../agents/session-env.ts"; import { loadConfig } from "../config.ts"; import { AgentError, ValidationError } from "../errors.ts"; import { jsonOutput } from "../json.ts"; @@ -441,22 +442,23 @@ export async function startCoordinatorSession( if (await agentDefFile.exists()) { appendSystemPromptFile = agentDefPath; } + const sessionEnv = buildOverstorySessionEnv({ + baseEnv: runtime.buildEnv(resolvedModel), + sessionKind: "coordinator", + agentName: coordinatorName, + capability: "coordinator", + worktreePath: projectRoot, + projectRoot, + ...(profileFlag ? { profile: profileFlag } : {}), + }); const spawnCmd = runtime.buildSpawnCommand({ model: resolvedModel.model, permissionMode: "bypass", cwd: projectRoot, appendSystemPromptFile, - env: { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: coordinatorName, - ...(profileFlag ? { OVERSTORY_PROFILE: profileFlag } : {}), - }, - }); - const pid = await tmux.createSession(tmuxSession, projectRoot, spawnCmd, { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: coordinatorName, - ...(profileFlag ? { OVERSTORY_PROFILE: profileFlag } : {}), + env: sessionEnv, }); + const pid = await tmux.createSession(tmuxSession, projectRoot, spawnCmd, sessionEnv); // Create a run for this coordinator session BEFORE recording the session, // so the session can reference the run ID from the start. diff --git a/src/commands/monitor.ts b/src/commands/monitor.ts index bff2bbd0..85642218 100644 --- a/src/commands/monitor.ts +++ b/src/commands/monitor.ts @@ -18,6 +18,7 @@ import { join } from "node:path"; import { Command } from "commander"; import { createIdentity, loadIdentity } from "../agents/identity.ts"; import { createManifestLoader, resolveModel } from "../agents/manifest.ts"; +import { buildOverstorySessionEnv } from "../agents/session-env.ts"; import { loadConfig } from "../config.ts"; import { AgentError, ValidationError } from "../errors.ts"; import { jsonOutput } from "../json.ts"; @@ -149,20 +150,22 @@ async function startMonitor(opts: { json: boolean; attach: boolean }): Promiseworking. diff --git a/src/commands/sling.ts b/src/commands/sling.ts index 023b4b89..2967dbe3 100644 --- a/src/commands/sling.ts +++ b/src/commands/sling.ts @@ -24,6 +24,7 @@ import { join, resolve } from "node:path"; import { createIdentity, loadIdentity } from "../agents/identity.ts"; import { createManifestLoader, resolveModel } from "../agents/manifest.ts"; import { writeOverlay } from "../agents/overlay.ts"; +import { buildOverstorySessionEnv } from "../agents/session-env.ts"; import { createCanopyClient } from "../canopy/client.ts"; import { loadConfig } from "../config.ts"; import { AgentError, HierarchyError, ValidationError } from "../errors.ts"; @@ -914,12 +915,15 @@ export async function slingCommand(taskId: string, opts: SlingOptions): Promise< // 11c. Spawn: headless runtimes bypass tmux entirely; tmux path is unchanged. if (runtime.headless === true && runtime.buildDirectSpawn) { - const directEnv = { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: name, - OVERSTORY_WORKTREE_PATH: worktreePath, - OVERSTORY_TASK_ID: taskId, - }; + const directEnv = buildOverstorySessionEnv({ + baseEnv: runtime.buildEnv(resolvedModel), + sessionKind: "worker", + agentName: name, + capability, + worktreePath, + projectRoot: config.project.root, + taskId, + }); const argv = runtime.buildDirectSpawn({ cwd: worktreePath, env: directEnv, @@ -1000,24 +1004,23 @@ export async function slingCommand(taskId: string, opts: SlingOptions): Promise< // 12. Create tmux session running claude in interactive mode const tmuxSessionName = `overstory-${config.project.name}-${name}`; + const sessionEnv = buildOverstorySessionEnv({ + baseEnv: runtime.buildEnv(resolvedModel), + sessionKind: "worker", + agentName: name, + capability, + worktreePath, + projectRoot: config.project.root, + taskId, + }); const spawnCmd = runtime.buildSpawnCommand({ model: resolvedModel.model, permissionMode: "bypass", cwd: worktreePath, sharedWritableDirs: getSharedWritableDirs(config.project.root, capability), - env: { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: name, - OVERSTORY_WORKTREE_PATH: worktreePath, - OVERSTORY_TASK_ID: taskId, - }, - }); - const pid = await createSession(tmuxSessionName, worktreePath, spawnCmd, { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: name, - OVERSTORY_WORKTREE_PATH: worktreePath, - OVERSTORY_TASK_ID: taskId, + env: sessionEnv, }); + const pid = await createSession(tmuxSessionName, worktreePath, spawnCmd, sessionEnv); // 13. Record session BEFORE sending the beacon so that hook-triggered // updateLastActivity() can find the entry and transition booting->working. @@ -1114,10 +1117,8 @@ export async function slingCommand(taskId: string, opts: SlingOptions): Promise< // the beacon text entirely (overstory-3271). // // Skipped for runtimes that return false from requiresBeaconVerification(). - // Pi's TUI idle and processing states are indistinguishable via detectReady - // (both show "pi v..." header and the token-usage status bar), so the loop - // would incorrectly conclude the beacon was not received and spam duplicate - // startup messages. + // Pi uses an explicit extension-owned ready marker instead, so this loop + // would only resend duplicate startup messages there. const needsVerification = !runtime.requiresBeaconVerification || runtime.requiresBeaconVerification(); if (needsVerification) { diff --git a/src/commands/supervisor.ts b/src/commands/supervisor.ts index b1a6d7c5..e8f35740 100644 --- a/src/commands/supervisor.ts +++ b/src/commands/supervisor.ts @@ -17,6 +17,7 @@ import { join } from "node:path"; import { Command } from "commander"; import { createIdentity, loadIdentity } from "../agents/identity.ts"; import { createManifestLoader, resolveModel } from "../agents/manifest.ts"; +import { buildOverstorySessionEnv } from "../agents/session-env.ts"; import { loadConfig } from "../config.ts"; import { AgentError, ValidationError } from "../errors.ts"; import { jsonOutput } from "../json.ts"; @@ -177,22 +178,23 @@ async function startSupervisor(opts: { if (await agentDefFile.exists()) { appendSystemPromptFile = agentDefPath; } + const sessionEnv = buildOverstorySessionEnv({ + baseEnv: runtime.buildEnv(resolvedModel), + sessionKind: "coordinator", + agentName: opts.name, + capability: "supervisor", + worktreePath: projectRoot, + projectRoot, + taskId: opts.task, + }); const spawnCmd = runtime.buildSpawnCommand({ model: resolvedModel.model, permissionMode: "bypass", cwd: projectRoot, appendSystemPromptFile, - env: { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: opts.name, - OVERSTORY_TASK_ID: opts.task, - }, - }); - const pid = await createSession(tmuxSession, projectRoot, spawnCmd, { - ...runtime.buildEnv(resolvedModel), - OVERSTORY_AGENT_NAME: opts.name, - OVERSTORY_TASK_ID: opts.task, + env: sessionEnv, }); + const pid = await createSession(tmuxSession, projectRoot, spawnCmd, sessionEnv); // Wait for Claude Code TUI to render before sending input await waitForTuiReady(tmuxSession, (content) => runtime.detectReady(content)); diff --git a/src/runtimes/pi-guards.test.ts b/src/runtimes/pi-guards.test.ts deleted file mode 100644 index 67a25139..00000000 --- a/src/runtimes/pi-guards.test.ts +++ /dev/null @@ -1,486 +0,0 @@ -import { describe, expect, test } from "bun:test"; -import { INTERACTIVE_TOOLS, NATIVE_TEAM_TOOLS } from "../agents/guard-rules.ts"; -import { PiRuntime } from "./pi.ts"; -import { generatePiGuardExtension } from "./pi-guards.ts"; -import type { HooksDef } from "./types.ts"; - -const WORKTREE = "/project/.overstory/worktrees/test-agent"; - -function builderHooks(name = "test-builder"): HooksDef { - return { agentName: name, capability: "builder", worktreePath: WORKTREE }; -} - -function scoutHooks(name = "test-scout"): HooksDef { - return { agentName: name, capability: "scout", worktreePath: WORKTREE }; -} - -function coordinatorHooks(name = "test-coordinator"): HooksDef { - return { agentName: name, capability: "coordinator", worktreePath: WORKTREE }; -} - -describe("generatePiGuardExtension", () => { - describe("header and identity", () => { - test("embeds agent name in generated code", () => { - const generated = generatePiGuardExtension(builderHooks("my-builder")); - expect(generated).toContain('const AGENT_NAME = "my-builder";'); - }); - - test("embeds worktree path in generated code", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain(`const WORKTREE_PATH = "${WORKTREE}";`); - }); - - test("embeds capability in file header comment", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("Capability: builder"); - }); - - test("imports Pi Extension type", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - 'import type { ExtensionAPI } from "@mariozechner/pi-coding-agent";', - ); - }); - - test("exports a default Pi Extension factory", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("export default function (pi: ExtensionAPI) {"); - expect(generated).toContain('pi.on("tool_call", async (event) => {'); - }); - }); - - describe("TEAM_BLOCKED / INTERACTIVE_BLOCKED — separate sets per category (all capabilities)", () => { - test("all NATIVE_TEAM_TOOLS appear in TEAM_BLOCKED for builder", () => { - const generated = generatePiGuardExtension(builderHooks()); - const teamSection = extractTeamBlockedSection(generated); - for (const tool of NATIVE_TEAM_TOOLS) { - expect(teamSection).toContain(`"${tool}"`); - } - }); - - test("all INTERACTIVE_TOOLS appear in INTERACTIVE_BLOCKED for builder", () => { - const generated = generatePiGuardExtension(builderHooks()); - const interactiveSection = extractInteractiveBlockedSection(generated); - for (const tool of INTERACTIVE_TOOLS) { - expect(interactiveSection).toContain(`"${tool}"`); - } - }); - - test("TEAM_BLOCKED and INTERACTIVE_BLOCKED checks use has() for efficiency", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("TEAM_BLOCKED.has(event.toolName)"); - expect(generated).toContain("INTERACTIVE_BLOCKED.has(event.toolName)"); - }); - - test("team tools use delegation block reason", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("Overstory agents must use 'ov sling' for delegation"); - }); - - test("interactive tools use human-interaction block reason", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - "requires human interaction — use ov mail (--type question) to escalate", - ); - }); - }); - - describe("Builder — implementation capability", () => { - test("write tools are NOT in TEAM_BLOCKED or INTERACTIVE_BLOCKED for builder", () => { - const generated = generatePiGuardExtension(builderHooks()); - const teamSection = extractTeamBlockedSection(generated); - const interactiveSection = extractInteractiveBlockedSection(generated); - expect(teamSection).not.toContain('"Write"'); - expect(teamSection).not.toContain('"Edit"'); - expect(teamSection).not.toContain('"NotebookEdit"'); - expect(interactiveSection).not.toContain('"Write"'); - expect(interactiveSection).not.toContain('"Edit"'); - expect(interactiveSection).not.toContain('"NotebookEdit"'); - }); - - test("WRITE_BLOCKED constant is absent for builder", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).not.toContain("const WRITE_BLOCKED ="); - }); - - test("has FILE_MODIFYING_PATTERNS section", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("FILE_MODIFYING_PATTERNS.some"); - }); - - test("has SAFE_PREFIXES array", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("const SAFE_PREFIXES ="); - }); - - test("does NOT have DANGEROUS_PATTERNS blocklist guard", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).not.toContain("DANGEROUS_PATTERNS.some"); - }); - - test("Bash path boundary check for file-modifying commands", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("FILE_MODIFYING_PATTERNS.some((re) => re.test(cmd))"); - expect(generated).toContain("Bash path boundary violation"); - }); - - test("builder Bash path boundary uses + '/' and exact match", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('!p.startsWith(WORKTREE_PATH + "/")'); - expect(generated).toContain("p !== WORKTREE_PATH"); - }); - - test("builder does NOT use cmd.trimStart() (no safe prefix check)", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).not.toContain("cmd.trimStart()"); - }); - }); - - describe("Scout — non-implementation capability", () => { - test("write tools ARE in WRITE_BLOCKED for scout", () => { - const generated = generatePiGuardExtension(scoutHooks()); - const writeSection = extractWriteBlockedSection(generated); - expect(writeSection).toContain('"Write"'); - expect(writeSection).toContain('"Edit"'); - expect(writeSection).toContain('"NotebookEdit"'); - }); - - test("WRITE_BLOCKED uses capability-specific block reason", () => { - const generated = generatePiGuardExtension(scoutHooks()); - expect(generated).toContain("scout agents cannot modify files"); - }); - - test("has whitelist+blocklist pattern (SAFE_PREFIXES then DANGEROUS_PATTERNS)", () => { - const generated = generatePiGuardExtension(scoutHooks()); - expect(generated).toContain("SAFE_PREFIXES.some((p) => trimmed.startsWith(p))"); - expect(generated).toContain("DANGEROUS_PATTERNS.some((re) => re.test(cmd))"); - }); - - test("safe prefix check uses cmd.trimStart() for leading whitespace tolerance", () => { - const generated = generatePiGuardExtension(scoutHooks()); - expect(generated).toContain("const trimmed = cmd.trimStart();"); - expect(generated).toContain("trimmed.startsWith(p)"); - }); - - test("SAFE_PREFIXES check comes before DANGEROUS_PATTERNS check", () => { - const generated = generatePiGuardExtension(scoutHooks()); - const safeIdx = generated.indexOf("SAFE_PREFIXES.some"); - const dangerIdx = generated.indexOf("DANGEROUS_PATTERNS.some"); - expect(safeIdx).toBeGreaterThan(-1); - expect(dangerIdx).toBeGreaterThan(-1); - expect(safeIdx).toBeLessThan(dangerIdx); - }); - - test("does NOT have FILE_MODIFYING_PATTERNS guard", () => { - const generated = generatePiGuardExtension(scoutHooks()); - expect(generated).not.toContain("FILE_MODIFYING_PATTERNS.some"); - }); - - test("block reason references capability name", () => { - const generated = generatePiGuardExtension(scoutHooks()); - expect(generated).toContain("scout agents cannot modify files"); - }); - }); - - describe("Coordinator — coordination capability", () => { - test("safe prefixes include git add and git commit", () => { - const generated = generatePiGuardExtension(coordinatorHooks()); - const safePrefixesSection = extractSafePrefixesSection(generated); - expect(safePrefixesSection).toContain('"git add"'); - expect(safePrefixesSection).toContain('"git commit"'); - }); - - test("write tools are in WRITE_BLOCKED (coordination is non-implementation)", () => { - const generated = generatePiGuardExtension(coordinatorHooks()); - const writeSection = extractWriteBlockedSection(generated); - expect(writeSection).toContain('"Write"'); - }); - - test("builder does NOT have git add/commit in safe prefixes", () => { - const generated = generatePiGuardExtension(builderHooks()); - const safePrefixesSection = extractSafePrefixesSection(generated); - expect(safePrefixesSection).not.toContain('"git add"'); - expect(safePrefixesSection).not.toContain('"git commit"'); - }); - }); - - describe("path boundary guards (all capabilities)", () => { - test("WRITE_SCOPE_TOOLS constant is always present", () => { - for (const hooks of [builderHooks(), scoutHooks(), coordinatorHooks()]) { - const generated = generatePiGuardExtension(hooks); - expect(generated).toContain( - 'const WRITE_SCOPE_TOOLS = new Set(["write", "edit", "Write", "Edit", "NotebookEdit"]);', - ); - } - }); - - test("path boundary check uses WORKTREE_PATH + '/' for subpath safety", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('filePath.startsWith(WORKTREE_PATH + "/")'); - }); - - test("path boundary allows exact worktree path match", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("filePath !== WORKTREE_PATH"); - }); - - test("path boundary checks file_path and notebook_path fields", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("file_path"); - expect(generated).toContain("notebook_path"); - }); - - test("path boundary block reason is clear", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - "Path boundary violation: file is outside your assigned worktree", - ); - }); - }); - - describe("universal Bash danger guards (all capabilities)", () => { - test("blocks git push for builder", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("git push is blocked"); - }); - - test("blocks git push for scout", () => { - const generated = generatePiGuardExtension(scoutHooks()); - expect(generated).toContain("git push is blocked"); - }); - - test("blocks git reset --hard", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("git reset --hard is not allowed"); - }); - - test("enforces branch naming convention using AGENT_NAME", () => { - const generated = generatePiGuardExtension(builderHooks("my-agent")); - // These strings intentionally contain literal ${...} — they appear in the generated code - // as template literal expressions, not as interpolations in this test file. - expect(generated).toContain("overstory/$" + "{AGENT_NAME}/"); - expect(generated).toContain( - "Branch must follow overstory/$" + "{AGENT_NAME}/{task-id} convention", - ); - }); - - test("bash guard matches both Bash and bash tool names", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('event.toolName === "Bash"'); - expect(generated).toContain('event.toolName === "bash"'); - }); - }); - - describe("quality gate prefixes", () => { - test("custom quality gate commands appear in SAFE_PREFIXES", () => { - const hooks: HooksDef = { - agentName: "test-reviewer", - capability: "reviewer", - worktreePath: WORKTREE, - qualityGates: [ - { name: "Tests", command: "bun test", description: "all tests must pass" }, - { name: "Lint", command: "bun run lint", description: "lint clean" }, - ], - }; - const generated = generatePiGuardExtension(hooks); - const safePrefixesSection = extractSafePrefixesSection(generated); - expect(safePrefixesSection).toContain('"bun test"'); - expect(safePrefixesSection).toContain('"bun run lint"'); - }); - - test("default quality gates provide SAFE_PREFIXES entries", () => { - // Without custom gates, DEFAULT_QUALITY_GATES are used - const generated = generatePiGuardExtension(scoutHooks()); - expect(generated).toContain("const SAFE_PREFIXES ="); - // bun test is the default quality gate command - const safePrefixesSection = extractSafePrefixesSection(generated); - expect(safePrefixesSection).toContain('"bun test"'); - }); - }); - - describe("generated code is self-contained", () => { - test("output is non-empty TypeScript string", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(typeof generated).toBe("string"); - expect(generated.length).toBeGreaterThan(500); - }); - - test("output ends with newline", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated.endsWith("\n")).toBe(true); - }); - - test("DANGEROUS_PATTERNS constant is always present", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("const DANGEROUS_PATTERNS ="); - }); - - test("FILE_MODIFYING_PATTERNS constant is always present", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("const FILE_MODIFYING_PATTERNS ="); - }); - - test("returns { type: 'allow' } as default", () => { - const generated = generatePiGuardExtension(builderHooks()); - // Pi's ExtensionAPI uses implicit undefined return for allow (no explicit { type: "allow" } needed). - // The generated code uses a comment marker "// Default: allow." instead. - expect(generated).toContain("// Default: allow."); - }); - - test("uses String() for safe property access on event.input", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain("String("); - expect(generated).toContain("event.input as Record"); - }); - - test("deterministic output for same inputs", () => { - const hooks = builderHooks("consistent-builder"); - const g1 = generatePiGuardExtension(hooks); - const g2 = generatePiGuardExtension(hooks); - expect(g1).toBe(g2); - }); - }); - - describe("activity tracking events", () => { - test('generated code contains pi.on("tool_call", ...)', () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('pi.on("tool_call",'); - }); - - test("generated code contains pi.exec ov log tool-start in tool_call handler", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - 'pi.exec("ov", ["log", "tool-start", "--agent", AGENT_NAME, "--tool-name", event.toolName])', - ); - }); - - test('generated code contains pi.on("tool_execution_end", ...)', () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('pi.on("tool_execution_end",'); - }); - - test("generated code contains pi.exec ov log tool-end in tool_execution_end handler", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - 'pi.exec("ov", ["log", "tool-end", "--agent", AGENT_NAME, "--tool-name", event.toolName])', - ); - }); - - test('generated code contains pi.on("session_shutdown", ...)', () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('pi.on("session_shutdown",'); - }); - - test("generated code awaits pi.exec ov log session-end in session_shutdown handler", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - 'await pi.exec("ov", ["log", "session-end", "--agent", AGENT_NAME])', - ); - }); - - test("tool_call handler passes --tool-name event.toolName to tool-start", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - 'pi.exec("ov", ["log", "tool-start", "--agent", AGENT_NAME, "--tool-name", event.toolName])', - ); - }); - - test("tool_execution_end handler passes --tool-name event.toolName to tool-end", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain( - 'pi.exec("ov", ["log", "tool-end", "--agent", AGENT_NAME, "--tool-name", event.toolName])', - ); - }); - - test("tool_execution_end handler uses named event parameter (not _event)", () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('pi.on("tool_execution_end", async (event) => {'); - expect(generated).not.toContain('pi.on("tool_execution_end", async (_event) => {'); - }); - - test('generated code contains pi.on("agent_end", ...)', () => { - const generated = generatePiGuardExtension(builderHooks()); - expect(generated).toContain('pi.on("agent_end",'); - }); - - test("generated code awaits pi.exec ov log session-end in agent_end handler", () => { - const generated = generatePiGuardExtension(builderHooks()); - // agent_end handler must await (not fire-and-forget) so it completes - // before Pi moves on, ensuring the SessionStore is updated. - const agentEndIdx = generated.indexOf('pi.on("agent_end"'); - const sessionShutdownIdx = generated.indexOf('pi.on("session_shutdown"'); - expect(agentEndIdx).toBeGreaterThan(-1); - expect(sessionShutdownIdx).toBeGreaterThan(-1); - // agent_end must come before session_shutdown - expect(agentEndIdx).toBeLessThan(sessionShutdownIdx); - // Extract the agent_end handler body - const handlerBody = generated.slice(agentEndIdx, sessionShutdownIdx); - expect(handlerBody).toContain( - 'await pi.exec("ov", ["log", "session-end", "--agent", AGENT_NAME])', - ); - }); - - test("agent_end handler is present for all capabilities", () => { - for (const hooks of [builderHooks(), scoutHooks(), coordinatorHooks()]) { - const generated = generatePiGuardExtension(hooks); - expect(generated).toContain('pi.on("agent_end",'); - } - }); - }); - - describe("PiRuntime integration", () => { - test("PiRuntime.requiresBeaconVerification() returns false", () => { - const runtime = new PiRuntime(); - expect(runtime.requiresBeaconVerification()).toBe(false); - }); - }); -}); - -// --- Helpers --- - -/** - * Extract the TEAM_BLOCKED Set literal section from generated code. - * Returns the text between "TEAM_BLOCKED = new Set" and the first "]);" - * after that point. - */ -function extractTeamBlockedSection(generated: string): string { - const start = generated.indexOf("TEAM_BLOCKED = new Set"); - const end = generated.indexOf("]);", start); - if (start === -1 || end === -1) return ""; - return generated.slice(start, end + 3); -} - -/** - * Extract the INTERACTIVE_BLOCKED Set literal section from generated code. - * Returns the text between "INTERACTIVE_BLOCKED = new Set" and the first "]);" - * after that point. - */ -function extractInteractiveBlockedSection(generated: string): string { - const start = generated.indexOf("INTERACTIVE_BLOCKED = new Set"); - const end = generated.indexOf("]);", start); - if (start === -1 || end === -1) return ""; - return generated.slice(start, end + 3); -} - -/** - * Extract the WRITE_BLOCKED Set literal section from generated code. - * Returns the text between "WRITE_BLOCKED = new Set" and the first "]);" - * after that point. - */ -function extractWriteBlockedSection(generated: string): string { - const start = generated.indexOf("WRITE_BLOCKED = new Set"); - const end = generated.indexOf("]);", start); - if (start === -1 || end === -1) return ""; - return generated.slice(start, end + 3); -} - -/** - * Extract the SAFE_PREFIXES array literal section from generated code. - * Returns the text between "SAFE_PREFIXES =" and the next "];" - */ -function extractSafePrefixesSection(generated: string): string { - const start = generated.indexOf("const SAFE_PREFIXES ="); - const end = generated.indexOf("];", start); - if (start === -1 || end === -1) return ""; - return generated.slice(start, end + 2); -} diff --git a/src/runtimes/pi-guards.ts b/src/runtimes/pi-guards.ts deleted file mode 100644 index 6679eee8..00000000 --- a/src/runtimes/pi-guards.ts +++ /dev/null @@ -1,366 +0,0 @@ -// Pi runtime guard extension generator. -// Generates self-contained TypeScript code for .pi/extensions/overstory-guard.ts. -// -// Pi's extension system uses the ExtensionAPI factory style: -// export default function(pi: ExtensionAPI) { pi.on("event", handler) } -// -// Guards fire via pi.on("tool_call", ...) and return { block: true, reason } -// to prevent tool execution — equivalent to Claude Code's PreToolUse hooks. -// -// Activity tracking fires via pi.exec("ov log ...") on tool_call, -// tool_execution_end, agent_end, and session_shutdown events so the SessionStore -// lastActivity stays fresh and the watchdog does not zombie-classify agents. - -import { - DANGEROUS_BASH_PATTERNS, - INTERACTIVE_TOOLS, - NATIVE_TEAM_TOOLS, - SAFE_BASH_PREFIXES, - WRITE_TOOLS, -} from "../agents/guard-rules.ts"; -import { extractQualityGatePrefixes } from "../agents/hooks-deployer.ts"; -import { DEFAULT_QUALITY_GATES } from "../config.ts"; -import type { HooksDef } from "./types.ts"; - -/** Capabilities that must not modify project files. */ -const NON_IMPLEMENTATION_CAPABILITIES = new Set([ - "scout", - "reviewer", - "lead", - "coordinator", - "supervisor", - "monitor", -]); - -/** Coordination capabilities that get git add/commit whitelisted for metadata sync. */ -const COORDINATION_CAPABILITIES = new Set(["coordinator", "supervisor", "monitor"]); - -/** - * Bash patterns that modify files and require path boundary validation. - * Mirrors FILE_MODIFYING_BASH_PATTERNS in hooks-deployer.ts (not exported, duplicated here). - * Applied to implementation agents (builder/merger) only. - */ -const FILE_MODIFYING_BASH_PATTERNS = [ - "sed\\s+-i", - "sed\\s+--in-place", - "echo\\s+.*>", - "printf\\s+.*>", - "cat\\s+.*>", - "tee\\s", - "\\bmv\\s", - "\\bcp\\s", - "\\brm\\s", - "\\bmkdir\\s", - "\\btouch\\s", - "\\bchmod\\s", - "\\bchown\\s", - ">>", - "\\binstall\\s", - "\\brsync\\s", -]; - -/** Serialize a string array as a TypeScript Set literal (tab-indented entries). */ -function toSetLiteral(items: string[]): string { - if (items.length === 0) return "new Set([])"; - const entries = items.map((s) => `\t"${s}",`).join("\n"); - return `new Set([\n${entries}\n])`; -} - -/** Serialize a string array as a TypeScript string[] literal (tab-indented entries). */ -function toStringArrayLiteral(items: string[]): string { - if (items.length === 0) return "[]"; - const entries = items.map((s) => `\t"${s}",`).join("\n"); - return `[\n${entries}\n]`; -} - -/** - * Serialize grep -qE pattern strings as a TypeScript RegExp[] literal. - * Pattern strings use \\b/\\s double-escaping: their string values (\b/\s) map - * directly to JavaScript regex word boundary/whitespace tokens. - */ -function toRegExpArrayLiteral(patterns: string[]): string { - if (patterns.length === 0) return "[]"; - const entries = patterns.map((p) => `\t/${p}/,`).join("\n"); - return `[\n${entries}\n]`; -} - -/** - * Generate a self-contained TypeScript guard extension for Pi's extension system. - * - * The returned string is ready to write as `.pi/extensions/overstory-guard.ts`. - * Pi loads this file and calls the default export with an ExtensionAPI instance. - * - * Extension uses the correct Pi factory style: - * export default function(pi: ExtensionAPI) { pi.on("event", handler); } - * - * Guard order (per AgentRuntime spec): - * 1. Block NATIVE_TEAM_TOOLS (all agents) — use ov sling for delegation. - * (Safety net: Pi does not use Claude Code's native team tools, so these - * are no-ops unless a future Pi version adds similar tool names.) - * 2. Block INTERACTIVE_TOOLS (all agents) — escalate via ov mail instead. - * (Safety net: Pi does not have AskUserQuestion/EnterPlanMode natively.) - * 3. Block write tools for non-implementation capabilities. - * (Pi uses lowercase tool names: "write", "edit" — checked in addition to - * the original mixed-case Claude Code names for forward compatibility.) - * 4. Path boundary on write/edit tools (all agents, defense-in-depth). - * (Pi uses event.input.path, not file_path.) - * 5. Universal Bash danger guards: git push, reset --hard, wrong branch naming. - * (Pi bash tool is named "bash" in lowercase.) - * 6a. Non-implementation agents: safe prefix whitelist then dangerous pattern blocklist. - * 6b. Implementation agents (builder/merger): file-modifying bash path boundary. - * 7. Default allow. - * - * Activity tracking: - * - tool_call handler: fire-and-forget "ov log tool-start" to update lastActivity. - * - tool_execution_end handler: fire-and-forget "ov log tool-end". - * - agent_end handler: awaited "ov log session-end" — fires when the agentic loop - * completes (task done). Without this, completed Pi agents get watchdog-escalated - * through stalled → nudge → triage → terminate. - * - session_shutdown handler: awaited "ov log session-end" — fires on Ctrl+C/SIGTERM. - * Kept as a safety net in case agent_end does not fire (e.g., crash, force-kill). - * - * These tracking calls prevent the watchdog from zombie-classifying Pi agents due - * to stale lastActivity timestamps (the root cause of the zombie state bug). - * - * @param hooks - Agent identity, capability, worktree path, and optional quality gates. - * @returns Self-contained TypeScript source code for the Pi guard extension file. - */ -export function generatePiGuardExtension(hooks: HooksDef): string { - const { agentName, capability, worktreePath, qualityGates } = hooks; - const gates = qualityGates ?? DEFAULT_QUALITY_GATES; - const gatePrefixes = extractQualityGatePrefixes(gates); - - const isNonImpl = NON_IMPLEMENTATION_CAPABILITIES.has(capability); - const isCoordination = COORDINATION_CAPABILITIES.has(capability); - - // Build safe Bash prefixes: base set + coordination extras + quality gate commands. - const safePrefixes: string[] = [ - ...SAFE_BASH_PREFIXES, - ...(isCoordination ? ["git add", "git commit"] : []), - ...gatePrefixes, - ]; - - // Pi uses lowercase tool names; also include the original mixed-case names - // from WRITE_TOOLS as a safety net for any future Pi version that adopts them. - const piWriteToolsBlocked = ["write", "edit", ...WRITE_TOOLS]; - - const teamBlockedCode = toSetLiteral([...NATIVE_TEAM_TOOLS]); - const interactiveBlockedCode = toSetLiteral([...INTERACTIVE_TOOLS]); - const writeBlockedCode = isNonImpl ? toSetLiteral(piWriteToolsBlocked) : null; - const safePrefixesCode = toStringArrayLiteral(safePrefixes); - const dangerousPatternsCode = toRegExpArrayLiteral(DANGEROUS_BASH_PATTERNS); - const fileModifyingPatternsCode = toRegExpArrayLiteral(FILE_MODIFYING_BASH_PATTERNS); - - // Capability-specific Bash guard block (mutually exclusive). - // Indented for insertion inside the "bash" tool_call branch. - const capabilityBashBlock = isNonImpl - ? [ - "", - `\t\t\t// Non-implementation agents: whitelist safe prefixes, block dangerous patterns.`, - `\t\t\tconst trimmed = cmd.trimStart();`, - `\t\t\tif (SAFE_PREFIXES.some((p) => trimmed.startsWith(p))) {`, - `\t\t\t\treturn; // Safe command — allow through.`, - `\t\t\t}`, - `\t\t\tif (DANGEROUS_PATTERNS.some((re) => re.test(cmd))) {`, - `\t\t\t\treturn {`, - `\t\t\t\t\tblock: true,`, - `\t\t\t\t\treason: "${capability} agents cannot modify files — this command is not allowed",`, - `\t\t\t\t};`, - `\t\t\t}`, - ].join("\n") - : [ - "", - `\t\t\t// Implementation agents: path boundary on file-modifying Bash commands.`, - `\t\t\tif (FILE_MODIFYING_PATTERNS.some((re) => re.test(cmd))) {`, - `\t\t\t\tconst tokens = cmd.split(/\\s+/);`, - `\t\t\t\tconst paths = tokens`, - `\t\t\t\t\t.filter((t) => t.startsWith("/"))`, - `\t\t\t\t\t.map((t) => t.replace(/[";>]*$/, ""));`, - `\t\t\t\tfor (const p of paths) {`, - `\t\t\t\t\tif (!p.startsWith("/dev/") && !p.startsWith("/tmp/") && !p.startsWith(WORKTREE_PATH + "/") && p !== WORKTREE_PATH) {`, - `\t\t\t\t\t\treturn {`, - `\t\t\t\t\t\t\tblock: true,`, - `\t\t\t\t\t\t\treason: "Bash path boundary violation: command targets a path outside your worktree. All file modifications must stay within your assigned worktree.",`, - `\t\t\t\t\t\t};`, - `\t\t\t\t\t}`, - `\t\t\t\t}`, - `\t\t\t}`, - ].join("\n"); - - const lines = [ - `// .pi/extensions/overstory-guard.ts`, - `// Generated by overstory — do not edit manually.`, - `// Agent: ${agentName} | Capability: ${capability}`, - `//`, - `// Uses Pi's ExtensionAPI factory style: export default function(pi: ExtensionAPI) { ... }`, - `// pi.on("tool_call", ...) returns { block: true, reason } to prevent tool execution.`, - `// pi.exec("ov", [...]) calls the overstory CLI for activity tracking and lifecycle.`, - `import type { ExtensionAPI } from "@mariozechner/pi-coding-agent";`, - ``, - `const AGENT_NAME = "${agentName}";`, - `const WORKTREE_PATH = "${worktreePath}";`, - ``, - `// Native team/task tools blocked (all agents) — use ov sling for delegation.`, - `// Safety net: Pi does not use Claude Code team tool names natively.`, - `const TEAM_BLOCKED = ${teamBlockedCode};`, - ``, - `// Interactive tools blocked (all agents) — escalate via ov mail instead.`, - `// Safety net: Pi does not use these Claude Code tool names natively.`, - `const INTERACTIVE_BLOCKED = ${interactiveBlockedCode};`, - ``, - ...(isNonImpl && writeBlockedCode !== null - ? [ - `// Write tools blocked for non-implementation capabilities.`, - `// Includes Pi lowercase names ("write", "edit") and Claude Code names for compat.`, - `const WRITE_BLOCKED = ${writeBlockedCode};`, - ``, - ] - : []), - `// Write-scope tools where path boundary is enforced (all agents, defense-in-depth).`, - `// Pi uses lowercase tool names; also include Claude Code names for forward compat.`, - `const WRITE_SCOPE_TOOLS = new Set(["write", "edit", "Write", "Edit", "NotebookEdit"]);`, - ``, - `// Safe Bash command prefixes — checked before the dangerous pattern blocklist.`, - `const SAFE_PREFIXES = ${safePrefixesCode};`, - ``, - `// Dangerous Bash patterns blocked for non-implementation agents.`, - `const DANGEROUS_PATTERNS = ${dangerousPatternsCode};`, - ``, - `// File-modifying Bash patterns requiring path boundary validation (implementation agents).`, - `const FILE_MODIFYING_PATTERNS = ${fileModifyingPatternsCode};`, - ``, - `export default function (pi: ExtensionAPI) {`, - `\t/**`, - `\t * Tool call guard + activity tracking.`, - `\t *`, - `\t * Fires before each tool executes. Returns { block: true, reason } to block.`, - `\t * Fire-and-forgets "ov log tool-start" to update lastActivity in the SessionStore,`, - `\t * preventing the Tier 0 watchdog from zombie-classifying this agent due to stale`, - `\t * lastActivity timestamps (the root cause of the Pi zombie state bug).`, - `\t *`, - `\t * NOTE: Pi tool names are lowercase ("bash", "write", "edit").`, - `\t * event.toolName is used (not event.name — that field does not exist on ToolCallEvent).`, - `\t * Path boundary uses event.input.path (not file_path — that is Claude Code's field name).`, - `\t */`, - `\tpi.on("tool_call", async (event) => {`, - `\t\t// Activity tracking: update lastActivity so watchdog knows agent is alive.`, - `\t\t// Fire-and-forget — do not await (avoids latency on every tool call).`, - `\t\tpi.exec("ov", ["log", "tool-start", "--agent", AGENT_NAME, "--tool-name", event.toolName]).catch(() => {});`, - ``, - `\t\t// 1. Block native team/task tools (all agents).`, - `\t\tif (TEAM_BLOCKED.has(event.toolName)) {`, - `\t\t\treturn {`, - `\t\t\t\tblock: true,`, - `\t\t\t\treason: \`Overstory agents must use 'ov sling' for delegation — \${event.toolName} is not allowed\`,`, - `\t\t\t};`, - `\t\t}`, - ``, - `\t\t// 2. Block interactive tools (all agents).`, - `\t\tif (INTERACTIVE_BLOCKED.has(event.toolName)) {`, - `\t\t\treturn {`, - `\t\t\t\tblock: true,`, - `\t\t\t\treason: \`\${event.toolName} requires human interaction — use ov mail (--type question) to escalate\`,`, - `\t\t\t};`, - `\t\t}`, - ``, - ...(isNonImpl - ? [ - `\t\t// 3. Block write tools for non-implementation capabilities.`, - `\t\tif (WRITE_BLOCKED.has(event.toolName)) {`, - `\t\t\treturn {`, - `\t\t\t\tblock: true,`, - `\t\t\t\treason: \`${capability} agents cannot modify files — \${event.toolName} is not allowed\`,`, - `\t\t\t};`, - `\t\t}`, - ``, - ] - : []), - `\t\t// ${isNonImpl ? "4" : "3"}. Path boundary enforcement for write/edit tools (all agents).`, - `\t\t// Pi uses event.input.path (not file_path — that is Claude Code's field name).`, - `\t\tif (WRITE_SCOPE_TOOLS.has(event.toolName)) {`, - `\t\t\tconst filePath = String(`, - `\t\t\t\t(event.input as Record)?.path ??`, - `\t\t\t\t(event.input as Record)?.file_path ??`, - `\t\t\t\t(event.input as Record)?.notebook_path ??`, - `\t\t\t\t"",`, - `\t\t\t);`, - `\t\t\tif (filePath && !filePath.startsWith(WORKTREE_PATH + "/") && filePath !== WORKTREE_PATH) {`, - `\t\t\t\treturn {`, - `\t\t\t\t\tblock: true,`, - `\t\t\t\t\treason: "Path boundary violation: file is outside your assigned worktree. All writes must target files within your worktree.",`, - `\t\t\t\t};`, - `\t\t\t}`, - `\t\t}`, - ``, - `\t\t// ${isNonImpl ? "5" : "4"}. Bash command guards.`, - `\t\t// Pi's bash tool is named "bash" (lowercase).`, - `\t\tif (event.toolName === "bash" || event.toolName === "Bash") {`, - `\t\t\tconst cmd = String((event.input as Record)?.command ?? "");`, - ``, - `\t\t\t// Universal danger guards (all agents).`, - `\t\t\tif (/\\bgit\\s+push\\b/.test(cmd)) {`, - `\t\t\t\treturn {`, - `\t\t\t\t\tblock: true,`, - `\t\t\t\t\treason: "git push is blocked — use ov merge to integrate changes, push manually when ready",`, - `\t\t\t\t};`, - `\t\t\t}`, - `\t\t\tif (/git\\s+reset\\s+--hard/.test(cmd)) {`, - `\t\t\t\treturn {`, - `\t\t\t\t\tblock: true,`, - `\t\t\t\t\treason: "git reset --hard is not allowed — it destroys uncommitted work",`, - `\t\t\t\t};`, - `\t\t\t}`, - `\t\t\tconst branchMatch = /git\\s+checkout\\s+-b\\s+(\\S+)/.exec(cmd);`, - `\t\t\tif (branchMatch) {`, - `\t\t\t\tconst branch = branchMatch[1] ?? "";`, - `\t\t\t\tif (!branch.startsWith(\`overstory/\${AGENT_NAME}/\`)) {`, - `\t\t\t\t\treturn {`, - `\t\t\t\t\t\tblock: true,`, - `\t\t\t\t\t\treason: \`Branch must follow overstory/\${AGENT_NAME}/{task-id} convention\`,`, - `\t\t\t\t\t};`, - `\t\t\t\t}`, - `\t\t\t}`, - capabilityBashBlock, - `\t\t}`, - ``, - `\t\t// Default: allow.`, - `\t});`, - ``, - `\t/**`, - `\t * Tool execution end: fire-and-forget "ov log tool-end" for event tracking.`, - `\t * Paired with tool_call's tool-start fire for proper begin/end event logging.`, - `\t */`, - `\tpi.on("tool_execution_end", async (event) => {`, - `\t\tpi.exec("ov", ["log", "tool-end", "--agent", AGENT_NAME, "--tool-name", event.toolName]).catch(() => {});`, - `\t});`, - ``, - `\t/**`, - `\t * Agent end: log session-end when the agentic loop completes (task done).`, - `\t *`, - `\t * Awaited so it completes before Pi moves on. Without this handler, completed`, - `\t * Pi agents never transition to "completed" state in the SessionStore, causing`, - `\t * the watchdog to escalate them through stalled → nudge → triage → terminate.`, - `\t *`, - `\t * Fires when the agent finishes its work — before session_shutdown.`, - `\t */`, - `\tpi.on("agent_end", async (_event) => {`, - `\t\tawait pi.exec("ov", ["log", "session-end", "--agent", AGENT_NAME]).catch(() => {});`, - `\t});`, - ``, - `\t/**`, - `\t * Session shutdown: safety-net session-end log for non-graceful exits.`, - `\t *`, - `\t * Awaited so it completes before Pi exits. Kept as a fallback in case`, - `\t * agent_end does not fire (e.g., crash, force-kill, Ctrl+C before task completes).`, - `\t *`, - `\t * Fires on Ctrl+C, Ctrl+D, or SIGTERM.`, - `\t */`, - `\tpi.on("session_shutdown", async (_event) => {`, - `\t\tawait pi.exec("ov", ["log", "session-end", "--agent", AGENT_NAME]).catch(() => {});`, - `\t});`, - `}`, - ``, - ]; - - return lines.join("\n"); -} diff --git a/src/runtimes/pi.test.ts b/src/runtimes/pi.test.ts index dfb11c6d..2e97429d 100644 --- a/src/runtimes/pi.test.ts +++ b/src/runtimes/pi.test.ts @@ -260,40 +260,29 @@ describe("PiRuntime", () => { expect(state).toEqual({ phase: "loading" }); }); - test("returns loading when only 'pi v' header present (no status bar)", () => { + test("returns loading when only header content is present", () => { const state = runtime.detectReady(" pi v0.55.1\n escape to interrupt"); expect(state).toEqual({ phase: "loading" }); }); - test("returns loading when only status bar present (no header)", () => { + test("returns loading when old status-bar content is present without marker", () => { const state = runtime.detectReady("0.0%/200k (auto) (anthropic) claude-opus-4-6"); expect(state).toEqual({ phase: "loading" }); }); - test("returns ready for real Pi TUI pane content", () => { + test("returns ready when the explicit Overstory ready marker is present", () => { const pane = [ " pi v0.55.1", " escape to interrupt", - " ctrl+c to clear", - "", - "[Context]", - " ~/Projects/os-eco/CLAUDE.md", - "", - "[Extensions]", - " project", - " overstory-guard.ts", - "", - "────────────────────────────────", - "~/Projects/os-eco/overstory (main)", - "0.0%/200k (auto) (anthropic) claude-opus-4-6 • high", + "[OVERSTORY:READY] agent=builder-1 runtime=pi", ].join("\n"); const state = runtime.detectReady(pane); expect(state).toEqual({ phase: "ready" }); }); - test("returns ready for minimal header + status bar", () => { - const state = runtime.detectReady("pi v1.0\n\n42.5%/200k done"); - expect(state).toEqual({ phase: "ready" }); + test("returns loading for the marker prefix without the runtime token", () => { + const state = runtime.detectReady("[OVERSTORY:READY] agent=builder-1"); + expect(state).toEqual({ phase: "loading" }); }); test("returns loading for random pane content", () => { @@ -307,11 +296,11 @@ describe("PiRuntime", () => { expect(state.phase).not.toBe("dialog"); }); - test("handles bedrock model provider in status bar", () => { + test("ignores old Pi footer content and waits for the marker", () => { const pane = " pi v0.55.1\n\n0.0%/200k (auto) (amazon-bedrock) us.anthropic.claude-opus-4-6-v1 • high"; const state = runtime.detectReady(pane); - expect(state).toEqual({ phase: "ready" }); + expect(state).toEqual({ phase: "loading" }); }); }); @@ -370,7 +359,7 @@ describe("PiRuntime", () => { expect(content).toBe("# Pi Agent Overlay\nThis is the overlay content."); }); - test("deploys guard extension to .pi/extensions/overstory-guard.ts", async () => { + test("does not deploy the legacy guard extension file", async () => { const worktreePath = join(tempDir, "worktree"); await runtime.deployConfig( @@ -381,10 +370,10 @@ describe("PiRuntime", () => { const guardPath = join(worktreePath, ".pi", "extensions", "overstory-guard.ts"); const exists = await Bun.file(guardPath).exists(); - expect(exists).toBe(true); + expect(exists).toBe(false); }); - test("guard extension contains agent name and worktree path", async () => { + test("does not deploy Pi settings.json", async () => { const worktreePath = join(tempDir, "my-worktree"); await runtime.deployConfig( @@ -393,57 +382,9 @@ describe("PiRuntime", () => { { agentName: "my-pi-agent", capability: "builder", worktreePath }, ); - const guardPath = join(worktreePath, ".pi", "extensions", "overstory-guard.ts"); - const content = await Bun.file(guardPath).text(); - expect(content).toContain("my-pi-agent"); - expect(content).toContain(worktreePath); - }); - - test("deploys Pi settings.json with extensions config", async () => { - const worktreePath = join(tempDir, "worktree"); - - await runtime.deployConfig( - worktreePath, - { content: "# Overlay" }, - { agentName: "test-builder", capability: "builder", worktreePath }, - ); - const settingsPath = join(worktreePath, ".pi", "settings.json"); const exists = await Bun.file(settingsPath).exists(); - expect(exists).toBe(true); - - const content = await Bun.file(settingsPath).text(); - const parsed = JSON.parse(content) as Record; - expect(parsed.extensions).toEqual(["./extensions"]); - }); - - test("settings.json has trailing newline", async () => { - const worktreePath = join(tempDir, "worktree"); - - await runtime.deployConfig(worktreePath, undefined, { - agentName: "test-builder", - capability: "builder", - worktreePath, - }); - - const settingsPath = join(worktreePath, ".pi", "settings.json"); - const content = await Bun.file(settingsPath).text(); - expect(content.endsWith("\n")).toBe(true); - }); - - test("settings.json uses tab indentation", async () => { - const worktreePath = join(tempDir, "worktree"); - - await runtime.deployConfig(worktreePath, undefined, { - agentName: "test-builder", - capability: "builder", - worktreePath, - }); - - const settingsPath = join(worktreePath, ".pi", "settings.json"); - const content = await Bun.file(settingsPath).text(); - // Tab-indented JSON has \t before array entries - expect(content).toContain("\t"); + expect(exists).toBe(false); }); test("skips CLAUDE.md when overlay is undefined", async () => { @@ -460,7 +401,7 @@ describe("PiRuntime", () => { expect(overlayExists).toBe(false); }); - test("still deploys guard and settings when overlay is undefined", async () => { + test("does not deploy Pi-specific files when overlay is undefined", async () => { const worktreePath = join(tempDir, "worktree"); await runtime.deployConfig(worktreePath, undefined, { @@ -472,11 +413,11 @@ describe("PiRuntime", () => { const guardPath = join(worktreePath, ".pi", "extensions", "overstory-guard.ts"); const settingsPath = join(worktreePath, ".pi", "settings.json"); - expect(await Bun.file(guardPath).exists()).toBe(true); - expect(await Bun.file(settingsPath).exists()).toBe(true); + expect(await Bun.file(guardPath).exists()).toBe(false); + expect(await Bun.file(settingsPath).exists()).toBe(false); }); - test("all three files present when overlay is provided", async () => { + test("only CLAUDE.md is present when overlay is provided", async () => { const worktreePath = join(tempDir, "worktree"); await runtime.deployConfig( @@ -492,8 +433,8 @@ describe("PiRuntime", () => { const settingsExists = await Bun.file(join(worktreePath, ".pi", "settings.json")).exists(); expect(claudeMdExists).toBe(true); - expect(guardExists).toBe(true); - expect(settingsExists).toBe(true); + expect(guardExists).toBe(false); + expect(settingsExists).toBe(false); }); }); diff --git a/src/runtimes/pi.ts b/src/runtimes/pi.ts index 34bd48d6..79ec6f2a 100644 --- a/src/runtimes/pi.ts +++ b/src/runtimes/pi.ts @@ -4,7 +4,6 @@ import { mkdir } from "node:fs/promises"; import { join } from "node:path"; import type { PiRuntimeConfig, ResolvedModel } from "../types.ts"; -import { generatePiGuardExtension } from "./pi-guards.ts"; import type { AgentRuntime, HooksDef, @@ -24,12 +23,14 @@ const DEFAULT_PI_CONFIG: PiRuntimeConfig = { }, }; +const PI_READY_MARKER_PREFIX = "[OVERSTORY:READY]"; + /** * Pi runtime adapter. * * Implements AgentRuntime for the `pi` CLI (Mario Zechner's Pi coding agent). - * Security is enforced via Pi guard extensions rather than permission-mode flags — - * Pi has no --permission-mode equivalent. + * Pi has no --permission-mode flag. Session-scoped policy and readiness are + * enforced by the companion os-eco Pi extension package instead. */ export class PiRuntime implements AgentRuntime { /** Unique identifier for this runtime. */ @@ -67,7 +68,6 @@ export class PiRuntime implements AgentRuntime { * Maps SpawnOpts to the `pi` CLI flags: * - `model` → `--model ` * - `permissionMode` is accepted but NOT mapped — Pi has no permission-mode flag. - * Security is enforced via guard extensions deployed by deployConfig(). * - `appendSystemPrompt` → `--append-system-prompt ''` (POSIX single-quote escaping) * * The `cwd` and `env` fields are handled by the tmux session creator, not embedded here. @@ -112,68 +112,52 @@ export class PiRuntime implements AgentRuntime { } /** - * Deploy per-agent instructions and guards to a worktree. + * Deploy per-agent instructions to a worktree. * - * Writes up to three files: - * 1. `.claude/CLAUDE.md` — agent's task-specific overlay. Skipped when overlay is undefined. - * 2. `.pi/extensions/overstory-guard.ts` — Pi guard extension (always deployed). - * 3. `.pi/settings.json` — Pi settings enabling the extensions directory (always deployed). + * Pi session policy and readiness signaling now live in the external + * os-eco Pi extension package. Overstory only writes the task-specific + * overlay file here. * * @param worktreePath - Absolute path to the agent's git worktree - * @param overlay - Overlay content to write as CLAUDE.md, or undefined for guard-only deployment - * @param hooks - Agent identity, capability, worktree path, and optional quality gates + * @param overlay - Overlay content to write as CLAUDE.md, or undefined to skip instruction output + * @param _hooks - Reserved for interface compatibility; Pi policy now lives in the extension package */ async deployConfig( worktreePath: string, overlay: OverlayContent | undefined, - hooks: HooksDef, + _hooks: HooksDef, ): Promise { if (overlay) { const claudeDir = join(worktreePath, ".claude"); await mkdir(claudeDir, { recursive: true }); await Bun.write(join(claudeDir, "CLAUDE.md"), overlay.content); } - - // Always deploy Pi guard extension. - const piExtDir = join(worktreePath, ".pi", "extensions"); - await mkdir(piExtDir, { recursive: true }); - await Bun.write(join(piExtDir, "overstory-guard.ts"), generatePiGuardExtension(hooks)); - - // Always deploy Pi settings pointing at the extensions directory. - const piDir = join(worktreePath, ".pi"); - const settings = { extensions: ["./extensions"] }; - await Bun.write(join(piDir, "settings.json"), `${JSON.stringify(settings, null, "\t")}\n`); } /** * Pi does not require beacon verification/resend. * * Claude Code's TUI sometimes swallows Enter during late initialization, so the - * orchestrator resends the beacon until the pane leaves the "idle" state. Pi's TUI - * does not have this issue AND its idle vs. processing states are indistinguishable - * via detectReady (the header "pi v..." and status bar token counter are visible in - * both states). Enabling the resend loop would spam Pi with duplicate beacon messages. + * orchestrator resends the beacon until the pane leaves the "idle" state. Pi uses + * an explicit ready marker emitted by the companion extension package, so it does + * not need the resend loop either. */ requiresBeaconVerification(): boolean { return false; } /** - * Detect Pi TUI readiness from a tmux pane content snapshot. + * Detect Pi readiness from the explicit extension-owned ready marker. * - * Pi shows a header containing "pi" and "model:" when the TUI has fully rendered. - * Pi has no trust dialog phase. + * The marker is rendered into the Pi UI by the os-eco Pi extension only when the + * managed session is ready for work: + * [OVERSTORY:READY] agent= runtime=pi * * @param paneContent - Captured tmux pane content to analyze * @returns Current readiness phase */ detectReady(paneContent: string): ReadyState { - // Pi's TUI shows "pi v" in the header and a status bar with - // a token usage indicator like "0.0%/200k" when fully rendered. - // Earlier detection checked for "model:" which Pi's TUI never contains. - const hasHeader = paneContent.includes("pi v"); - const hasStatusBar = /\d+\.\d+%\/\d+k/.test(paneContent); - if (hasHeader && hasStatusBar) { + if (paneContent.includes(PI_READY_MARKER_PREFIX) && paneContent.includes("runtime=pi")) { return { phase: "ready" }; } return { phase: "loading" }; diff --git a/src/runtimes/sapling.ts b/src/runtimes/sapling.ts index 448259fb..c8f30aee 100644 --- a/src/runtimes/sapling.ts +++ b/src/runtimes/sapling.ts @@ -45,7 +45,7 @@ const SAPLING_ALIAS_FALLBACKS: Record = { /** * Bash patterns that modify files and require path boundary validation - * for implementation agents (builder/merger). Mirrors the constant in pi-guards.ts. + * for implementation agents (builder/merger). */ const FILE_MODIFYING_BASH_PATTERNS = [ "sed\\s+-i", diff --git a/src/runtimes/types.ts b/src/runtimes/types.ts index 9b9a64c3..0ea97053 100644 --- a/src/runtimes/types.ts +++ b/src/runtimes/types.ts @@ -47,7 +47,7 @@ export interface OverlayContent { /** * Runtime-agnostic hook/guard configuration for deployment to a worktree. * Each runtime adapter translates this into its native guard mechanism - * (e.g., settings.local.json hooks for Claude Code, guard extensions for Pi). + * (e.g., settings.local.json hooks for Claude Code, guard files for Sapling). */ export interface HooksDef { /** Agent name injected into hook commands. */ @@ -152,7 +152,7 @@ export interface AgentRuntime { /** Stability level of this runtime adapter. */ readonly stability: "stable" | "beta" | "experimental"; - /** Relative path to the instruction file within a worktree (e.g. ".claude/CLAUDE.md"). */ + /** Relative path to the instruction file within a worktree (for example "AGENTS.md"). */ readonly instructionPath: string; /** Build the shell command string to spawn an interactive agent in a tmux pane. */ @@ -167,8 +167,9 @@ export interface AgentRuntime { /** * Deploy per-agent instructions and guards to a worktree. * Claude Code writes .claude/CLAUDE.md + settings.local.json hooks. + * Pi writes its runtime-specific instruction file and relies on the external + * os-eco Pi extension package for session-scoped policy and readiness signaling. * Codex writes AGENTS.md (no hook deployment needed). - * Pi writes .claude/CLAUDE.md + a guard extension in .pi/extensions/. * When overlay is undefined, only hooks are deployed (no instruction file written). */ deployConfig( @@ -210,9 +211,8 @@ export interface AgentRuntime { * * Claude Code's TUI sometimes swallows Enter during late initialization, so the * orchestrator resends the beacon if the pane still appears idle (overstory-3271). - * Pi's TUI does not exhibit this behavior AND its idle/processing states are - * indistinguishable via detectReady (both show the header and status bar), so - * the resend loop would spam Pi with duplicate startup messages. + * Pi's companion extension emits an explicit ready marker instead, so the resend + * loop would just spam duplicate startup messages there as well. * * Runtimes that omit this method (or return true) get the resend loop. * Pi returns false to skip it.