diff --git a/AGENTS.md b/AGENTS.md index 3efbd31..aa49032 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,9 @@ extensions/ ext-toggle.ts # /ext slash command — list & toggle extensions at runtime todo.ts # `todo` tool for the agent + /todos for the user (copy of upstream example) mcp-loader.ts # Generic MCP server loader + /mcp slash command + task.ts # `task` tool — pi-task (isolated child, PASS/FAIL envelope, boundary diff) as a tool + fork-gate.ts # tool_call hook: block forks whose brief needs `task`; reason = the task(...) to make +test/ # node --test 'test/**/*.test.mjs' — two-sided tests for the pure parts (classifier, root validation) install.sh # Idempotent installer — symlinks extensions/ into ~/.pi/agent/extensions/ package.json # pi package manifest — enables `pi install /path` as an alternative README.md # User-facing docs. diff --git a/README.md b/README.md index 58f936c..f73bc9c 100644 --- a/README.md +++ b/README.md @@ -222,6 +222,63 @@ A footer line shows pending changes (e.g. `pending: notify→off, foo→on`) so 3. In a running pi session, `/reload` is enough; no restart needed 4. (or, with `ext-toggle` installed: `/ext` to disable noisy ones at runtime) +### `task.ts` + +Registers the [`pi-task`](https://gitea.jordbo.se/joakimp/pi-toolkit) runner as +the `task` tool: a delegated task runs in an **isolated** child agent that sees +only the spec (context ladder L0–L2), returns a machine-checked PASS/FAIL +envelope, has every `roots[]` entry diffed before and after (a change outside +`write_allowed` FAILS), and leaves an audit dir under `~/.pi/agent/pi-task/`. +Compare `fork`, whose child inherits the *entire* parent branch (L4) and returns +prose. + +Why a tool and not just the CLI: `fork` is a tool with a self-recommending +description in the model's face every turn; the CLI had to be remembered, and the +prose rule that said "use pi-task for briefs with prohibitions" lost to the tool +list for months. The rule now lives in the tool description and in +`promptGuidelines`, which pi appends to the system prompt — the one place +compaction cannot remove it from. + +Beyond wrapping the CLI the tool: +- rejects, **before** a model run, the two spec errors that make a boundary + violation certain — `write_allowed` not an exact subset of `roots`, and a + writable root nested inside a watched-only root (the parent's porcelain would + change every time); +- serialises sibling `task` calls whose roots overlap (parallel siblings see each + other's writes as violations — measured 2026-09-17); +- returns the CLI's parent-facing report as the tool result (verdict, problems, + deliverable, evidence pointers, audit dir) and the parsed `result.json` as + `details`. A FAIL verdict is a *result*; only the CLI refusing to run is an + error. + +Finds the runner at `$PI_TASK_BIN`, `/opt/pi-toolkit/bin/pi-task`, +`~/src/pi-toolkit/bin/pi-task`, `~/src/src_local/pi-toolkit/bin/pi-task`, +`/workspace/pi-toolkit/bin/pi-task`, then `$PATH`. Effort tiers resolve through +`pi-fork.effortProfiles` in `settings.json`, same as `fork`. + +### `fork-gate.ts` + +A `tool_call` hook that **blocks** a `fork` whose brief contains a prohibition +(*do not / must not / never / only …*), a write boundary (*only touch, nothing +else, read-only, stay within …*) or a clause-initial file-changing imperative +(*Edit …, Commit …, Fix …, Implement …*), and returns — as the block reason the +model reads — the `task(...)` call to make instead, plus the CLI fallback. + +Measured motivation: on 2026-09-17 all five fork briefs in one session carried +"do not"; one returned confident verbatim quotes that did not exist in the +source, and the four migration briefs had disjoint write boundaries that fork +cannot enforce (they were re-run as pi-task and passed). Running the shipped +classifier over those five real briefs redirects 5/5. + +The gate matches **wording, not intent**, and says so: a genuinely read-only +exploration brief that trips it is rephrased without the prohibition; one that +cannot be rephrased needed `task`. Two-sided tests in +`test/fork-gate.test.mjs` pin both the must-block and must-pass sets (the latter +includes "Write a summary of…", "Report which files were modified…", "Give me an +update on…" — verbs a naive list would misfire on). + +`PI_FORK_GATE=off` makes it log to stderr instead of blocking; `/ext` disables it. + ### `todo.ts` Gives the agent a `todo` tool (actions: `list` / `add` / `toggle` / `clear`) so it can externalize a multi-step plan and tick items off as it works. Also registers `/todos` so you can inspect the current list at any time. diff --git a/extensions/fork-gate.ts b/extensions/fork-gate.ts new file mode 100644 index 0000000..6b96dba --- /dev/null +++ b/extensions/fork-gate.ts @@ -0,0 +1,139 @@ +/** + * fork-gate — put the "pi-task, not fork" rule where the decision is made. + * + * WHY. `fork` is a registered tool: its description is in the model's face on + * every turn, and it recommends itself for "implementation". `pi-task` is a + * CLI with no tool description, reached through bash after remembering it + * exists. The prose rule that says which to use (global AGENTS.md, the + * pi-extensions skill) was correct and lost anyway — the skill is gone after + * the first compaction, the tool description never is. Measured on this fleet + * (2026-09-01/06 mbp-m1-2020, 2026-09-17 tor-ms22): forks with prohibitions in + * the brief ignored them, invented quotes, answered in the user's voice; the + * same briefs as pi-task runs passed their envelope. Root cause is by design: + * a fork child receives the WHOLE parent branch with the brief as the last + * message (pi-fork src/index.ts), so in a long session the narrative outweighs + * the instruction. + * + * WHAT. A `tool_call` hook on `fork` that BLOCKS when the brief carries the + * three things a fork cannot be trusted with — a prohibition, a write + * boundary, or an imperative to change files — and returns, as the block + * reason, the exact `task(...)` call to make instead. Fork stays available for + * what it is good at: read-only exploration that needs THIS conversation, and + * N parallel opinions on one question. A false positive costs one turn and + * teaches the rule in-context, which is the point; the message says how to + * rephrase a genuinely read-only brief. + * + * The classifier matches WORDING, not intent, and says so. + * + * OFF SWITCH. `PI_FORK_GATE=off` in the environment disables blocking for a + * session (the match is still logged to stderr); `/ext` (ext-toggle) disables + * the extension entirely. Neither is needed in normal operation. + * + * Companion: `task.ts` registers the `task` tool this gate points at. The + * gate does not require it — the reason text also gives the CLI form. + */ + +import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; + +// ── Classifier (pure; exported so test/fork-gate.test.mjs can drive it) ────── + +export interface GateRule { + /** Class of hazard, used in the block reason. */ + kind: "prohibition" | "boundary" | "mutation"; + pattern: RegExp; + /** One line: why a fork cannot be trusted with this. */ + why: string; +} + +export const GATE_RULES: GateRule[] = [ + { + kind: "prohibition", + pattern: + /\b(do not|don'?t|must not|mustn'?t|never|not allowed|forbidden|prohibited|refrain from|under no circumstances|avoid (touching|editing|modifying|changing|writing))\b/i, + why: "a fork inherits your entire branch; the parent narrative outweighs a prohibition placed at the end of it", + }, + { + kind: "boundary", + pattern: + /\b(only (touch|edit|modify|change|write|create|alter)|(touch|edit|modify|change|write) only|nothing else|no other (files?|dirs?|directories|paths?)|outside (of )?(this|these|that|the|its|your) (dir|directory|directories|file|files|folder|path|root|scope|repo|tree)|write[- ]?boundar(y|ies)|read[- ]only|stay (within|inside)|confined? to|limited to (the |these |those )?(\S+ )?(files?|dir|directory|directories|paths?|tree|repo))\b/i, + why: "fork has no boundary diff — a write outside the named set is invisible; pi-task diffs roots[] before and after", + }, + { + // Clause-initial imperatives that change files. Deliberately excludes + // "write" and "create" (too often "write a summary"); the boundary and + // prohibition rules catch write tasks that phrase themselves carefully. + kind: "mutation", + pattern: + /(^|[.!?:;\n]\s*|\b(then|and|also|please|now)\s+)(commit|push|implement|refactor|rewrite|rename|delete|edit|modify|update|fix|apply|migrate|convert|replace|install|patch)\b/im, + why: "work that changes files needs a checkable PASS/FAIL and an audit trail; fork returns prose and deletes its own temp dir on exit", + }, +]; + +export interface GateVerdict { + block: boolean; + kind?: GateRule["kind"]; + matched?: string; + why?: string; +} + +/** Decide whether a fork brief must be redirected to pi-task. */ +export function classifyForkBrief(brief: unknown): GateVerdict { + if (typeof brief !== "string" || brief.length === 0) return { block: false }; + for (const rule of GATE_RULES) { + const m = rule.pattern.exec(brief); + if (m) { + // Report the whole match trimmed, so the agent sees which words fired. + return { block: true, kind: rule.kind, matched: m[0].trim(), why: rule.why }; + } + } + return { block: false }; +} + +/** The block reason the model reads. Exported so the test can pin its content. */ +export function blockReason(v: GateVerdict): string { + return [ + `fork BLOCKED by fork-gate — the brief contains a ${v.kind}: "${v.matched}".`, + `Why: ${v.why}.`, + "", + "Use the `task` tool instead (pi-task, L0–L2: isolated child that sees ONLY your spec,", + "immutable spec, PASS/FAIL envelope, write-boundary diff, audit trail):", + "", + ' task(id="short-slug", goal="", deliverable="",', + ' effort="fast|balanced|deep", read_only=false,', + ' roots=["/abs/repo/docs", "/abs/repo/src"], # WATCHED, each diffed on its own', + ' write_allowed=["/abs/repo/docs"], # CHANGEABLE: exact subset of roots; never nested in another root', + ' facts=[""], files=["/abs/path/to/read"])', + "", + " CLI form if the tool is absent: /opt/pi-toolkit/bin/pi-task run (`pi-task schema` lists fields)", + " Tasks whose roots overlap run one after another, never in parallel (the tool serialises them).", + "", + "If this really is READ-ONLY exploration that needs this conversation's context, or N parallel", + "opinions on one question, drop the prohibition/mutation wording and call fork again — the gate", + "matches wording, not intent. If the brief needs the prohibition, it needs pi-task.", + ].join("\n"); +} + +// ── Extension ──────────────────────────────────────────────────────────────── + +export default function (pi: ExtensionAPI) { + const mode = (process.env.PI_FORK_GATE ?? "block").toLowerCase(); + + pi.on("tool_call", (event, ctx) => { + if (event.toolName !== "fork") return; + const input = event.input as { task?: unknown } | undefined; + const verdict = classifyForkBrief(input?.task); + if (!verdict.block) return; + + const reason = blockReason(verdict); + if (mode === "off") { + process.stderr.write( + `[fork-gate] PI_FORK_GATE=off — would have blocked fork (${verdict.kind}: "${verdict.matched}")\n`, + ); + return; + } + if (ctx.hasUI) { + ctx.ui.notify(`fork-gate: blocked a fork (${verdict.kind}: "${verdict.matched}") → use task`, "warning"); + } + return { block: true, reason }; + }); +} diff --git a/extensions/task.ts b/extensions/task.ts new file mode 100644 index 0000000..eb79a54 --- /dev/null +++ b/extensions/task.ts @@ -0,0 +1,245 @@ +/** + * task — `pi-task` as a first-class tool, so the isolated rung of the context + * ladder competes with `fork` on equal footing. + * + * WHY. `fork` (L4: child inherits the ENTIRE parent branch) is a tool and is + * therefore in the model's face every turn; `pi-task` (L0–L2: child sees only + * an immutable spec) was a CLI reached through bash after remembering that it + * exists and hand-writing a JSON file. Models choose among the affordances in + * front of them, so the prose rule "prefer pi-task for work with prohibitions + * or write boundaries" lost to the tool list — repeatedly, and by the agent + * that had just read the rule. This registers the same CLI as `task`, with the + * decision rule in the description AND in `promptGuidelines` (which pi appends + * to the system prompt, so it survives compaction; the skill text does not). + * + * WHAT IT WRAPS. `/opt/pi-toolkit/bin/pi-task run ` (pi-toolkit). + * The wrapper adds nothing to the contract; it just builds the spec from flat + * parameters, validates the two things the CLI cannot catch until the run is + * over (write_allowed ⊆ roots; no writable root nested in a watched one — both + * make a violation certain, i.e. a guaranteed false FAIL), serialises tasks + * whose roots overlap (parallel siblings see each other's writes as violations; + * measured 2026-09-17), and returns the CLI's own parent-facing report plus the + * parsed result.json as details. + * + * WHAT IT DOES NOT FIX. Isolation removes the *narrative* failures (parent + * voice, invented continuity, ignored prohibitions). It does not remove + * confabulation: the envelope proves shape, not truth. Spot-check the evidence + * pointers — the report says so at the top of every result. + * + * Companion: `fork-gate.ts` blocks forks whose brief needs this tool. + */ + +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { homedir, tmpdir } from "node:os"; +import path from "node:path"; +import { StringEnum } from "@earendil-works/pi-ai"; +import type { ExtensionAPI } from "@earendil-works/pi-coding-agent"; +import { Type } from "typebox"; + +// ── Locating the CLI ───────────────────────────────────────────────────────── + +function findPiTask(): string { + const env = process.env.PI_TASK_BIN; + if (env && existsSync(env)) return env; + const home = homedir(); + const candidates = [ + "/opt/pi-toolkit/bin/pi-task", + path.join(home, "src/pi-toolkit/bin/pi-task"), + path.join(home, "src/src_local/pi-toolkit/bin/pi-task"), + "/workspace/pi-toolkit/bin/pi-task", + ]; + for (const c of candidates) if (existsSync(c)) return c; + for (const dir of (process.env.PATH ?? "").split(path.delimiter)) { + const c = path.join(dir, "pi-task"); + if (dir && existsSync(c)) return c; + } + throw new Error( + "pi-task CLI not found (looked at $PI_TASK_BIN, /opt/pi-toolkit/bin, ~/src/pi-toolkit/bin, /workspace/pi-toolkit/bin, $PATH). " + + "Install pi-toolkit or set PI_TASK_BIN.", + ); +} + +// ── Root overlap (exported for test/task.test.mjs) ─────────────────────────── + +/** True when one path is the other or lies inside it. */ +export function rootsOverlap(a: string, b: string): boolean { + const na = path.resolve(a); + const nb = path.resolve(b); + return na === nb || na.startsWith(nb + path.sep) || nb.startsWith(na + path.sep); +} + +export function anyOverlap(as: string[], bs: string[]): boolean { + return as.some((a) => bs.some((b) => rootsOverlap(a, b))); +} + +/** + * The two spec errors that make a boundary violation CERTAIN, checked before + * spending a model run on them. Mirrors pi-task's own check: a delta line is + * keyed by the root it was observed in, and is allowed only if that exact root + * string is in write_allowed. So a writable subdirectory of a watched repo root + * trips the repo root's porcelain diff every time. + */ +export function validateRoots(roots: string[], writeAllowed: string[] | undefined, readOnly: boolean): string[] { + const problems: string[] = []; + if (roots.length === 0) problems.push("roots must name at least one absolute path to watch"); + for (const r of roots) { + if (!path.isAbsolute(r)) problems.push(`root is not absolute: ${r}`); + else if (!existsSync(r)) problems.push(`root does not exist: ${r}`); + } + if (readOnly) { + if (writeAllowed && writeAllowed.length) problems.push("read_only=true but write_allowed is set — pick one"); + return problems; + } + if (!writeAllowed || writeAllowed.length === 0) { + problems.push( + "read_only=false requires write_allowed (an exact subset of roots). Without it every root is writable and a violation is undetectable.", + ); + return problems; + } + for (const w of writeAllowed) { + if (!roots.includes(w)) { + problems.push(`write_allowed entry is not one of roots (must match a root string exactly): ${w}`); + continue; + } + for (const r of roots) { + if (r !== w && !writeAllowed.includes(r) && rootsOverlap(w, r)) { + problems.push( + `writable root ${w} is nested in (or contains) watched-only root ${r}: any write would register as a violation there. ` + + "List the writable part as its own root and drop the enclosing one.", + ); + } + } + } + return problems; +} + +// ── Extension ──────────────────────────────────────────────────────────────── + +const ID_RE = /^[a-z0-9][a-z0-9-]{1,48}$/; +const MAX_REPORT_BYTES = 24_000; + +export default function (pi: ExtensionAPI) { + /** Tasks in flight, in registration order; a newcomer waits for earlier overlapping ones. */ + const running: { id: string; roots: string[]; done: Promise }[] = []; + + pi.registerTool({ + name: "task", + label: "Task (pi-task, isolated child)", + description: + "Run a delegated task in an ISOLATED child agent (pi-task; context ladder L0–L2). Unlike `fork`, the child sees ONLY this spec — " + + "not this conversation — so prohibitions and boundaries in the brief are actually obeyed. Returns a machine-checked PASS/FAIL: " + + "the envelope must parse; every root is diffed before/after and a change outside write_allowed FAILS the task; cost and wall-clock " + + "budgets apply; a full audit dir is kept under ~/.pi/agent/pi-task/. Use `task` for any delegated work that changes files, any " + + "brief containing do-not/only/never, or whenever you want a checkable result rather than prose. Use `fork` only for read-only " + + "exploration that needs this conversation's context, or N parallel opinions on one question. Roots are watched individually: " + + "write_allowed must be an exact subset of roots, and a writable root must not sit inside another listed root (list the writable " + + "part on its own). Tasks whose roots overlap are run one at a time automatically. Spot-check the evidence pointers in the result.", + promptSnippet: + "Delegate work to an isolated child agent with a PASS/FAIL envelope and write-boundary diff (prefer over fork for anything that writes or has prohibitions)", + promptGuidelines: [ + "Use task, not fork, when a delegated brief will change files, contains a prohibition (do not / only / never), or when a machine-checked PASS/FAIL is wanted; fork inherits this entire conversation and does not reliably obey such briefs.", + "Treat a task result's envelope as shape, not truth: spot-check its evidence pointers before building on them.", + ], + parameters: Type.Object({ + id: Type.String({ description: "Short slug (a-z, 0-9, -). Used in the session id and the audit dir name." }), + goal: Type.String({ description: "Verbatim task statement. The child has NO other context — say everything." }), + deliverable: Type.String({ description: "The exact shape of answer wanted (e.g. 'a table of file:line pointers', 'the edited files plus a one-line summary')." }), + effort: Type.Optional( + StringEnum(["fast", "balanced", "deep"] as const, { + description: "Model tier from settings pi-fork.effortProfiles: fast=mechanical/lookups, balanced=default, deep=architecture/security/ambiguity.", + }), + ), + read_only: Type.Optional(Type.Boolean({ description: "Default true. Any change under roots then FAILS the task." })), + roots: Type.Array(Type.String(), { + description: "Absolute paths WATCHED for changes (git porcelain --ignored, or a file manifest for non-git dirs). Each is diffed on its own.", + }), + write_allowed: Type.Optional( + Type.Array(Type.String(), { + description: "Exact subset of roots the child may CHANGE. Required when read_only=false. Must not be nested inside a watched-only root.", + }), + ), + facts: Type.Optional(Type.Array(Type.String(), { description: "Verified facts the child may rely on without re-deriving (L2 context)." })), + files: Type.Optional(Type.Array(Type.String(), { description: "Absolute paths the child should read itself (L1 context)." })), + commands: Type.Optional(Type.Array(Type.String(), { description: "Exact commands the child may run." })), + wall_s: Type.Optional(Type.Integer({ description: "Wall-clock budget in seconds (default 600). The child is killed at this point and the task FAILS." })), + usd: Type.Optional(Type.Number({ description: "Cost budget in USD (default 1.0). Exceeding it FAILS the task after the fact." })), + }), + + async execute(_toolCallId, params, signal, onUpdate) { + const bin = findPiTask(); + if (!ID_RE.test(params.id)) throw new Error(`id must match ${ID_RE} (got ${JSON.stringify(params.id)})`); + const readOnly = params.read_only ?? true; + const roots = params.roots.map((r) => path.resolve(r)); + const writeAllowed = params.write_allowed?.map((r) => path.resolve(r)); + const problems = validateRoots(roots, writeAllowed, readOnly); + if (problems.length) throw new Error(`task spec rejected before launch:\n- ${problems.join("\n- ")}`); + + const wall = params.wall_s ?? 600; + const spec = { + id: params.id, + goal: params.goal, + deliverable: params.deliverable, + effort: params.effort ?? "balanced", + read_only: readOnly, + roots, + ...(writeAllowed ? { write_allowed: writeAllowed } : {}), + context: { + ...(params.facts?.length ? { facts: params.facts } : {}), + ...(params.files?.length ? { files: params.files } : {}), + ...(params.commands?.length ? { commands: params.commands } : {}), + }, + budget: { wall_s: wall, usd: params.usd ?? 1.0 }, + }; + + // Serialise against earlier in-flight tasks whose roots overlap ours. + const blockers = running.filter((t) => anyOverlap(t.roots, roots)); + let release!: () => void; + const me = { id: params.id, roots, done: new Promise((res) => (release = res)) }; + running.push(me); + try { + if (blockers.length) { + onUpdate?.({ + content: [{ type: "text", text: `waiting for overlapping task(s) ${blockers.map((b) => b.id).join(", ")} — overlapping roots run sequentially` }], + }); + await Promise.all(blockers.map((b) => b.done)); + } + if (signal?.aborted) return { content: [{ type: "text", text: "Cancelled before launch" }], details: { cancelled: true } }; + + const dir = mkdtempSync(path.join(tmpdir(), "pi-task-spec-")); + const specPath = path.join(dir, `${params.id}.json`); + writeFileSync(specPath, JSON.stringify(spec, null, 2)); + onUpdate?.({ content: [{ type: "text", text: `pi-task run ${params.id} (${spec.effort}, ${readOnly ? "read-only" : `writes: ${writeAllowed!.join(", ")}`}, ≤${wall}s)` }] }); + let res: { stdout: string; stderr: string; code: number | null; killed?: boolean }; + try { + res = await pi.exec(bin, ["run", specPath], { signal, timeout: (wall + 90) * 1000 }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + + // 0 = PASS, 1 = FAIL verdict — both are results. Anything else is the CLI refusing. + if (res.code !== 0 && res.code !== 1) { + throw new Error( + `pi-task did not produce a verdict (exit ${res.code}${res.killed ? ", killed" : ""}):\n${(res.stderr || res.stdout).slice(-4000)}`, + ); + } + const auditDir = /^\s*audit:\s*(\S+)\s*$/m.exec(res.stdout)?.[1]; + let result: Record | undefined; + if (auditDir && existsSync(path.join(auditDir, "result.json"))) { + try { + result = JSON.parse(readFileSync(path.join(auditDir, "result.json"), "utf8")); + } catch {} + } + let report = res.stdout.trimEnd(); + if (report.length > MAX_REPORT_BYTES) report = `${report.slice(0, MAX_REPORT_BYTES)}\n… (report truncated; full result at ${auditDir ?? "audit dir"}/result.json)`; + return { + content: [{ type: "text", text: report }], + details: { verdict: res.code === 0 ? "PASS" : "FAIL", audit_dir: auditDir, result }, + }; + } finally { + release(); + const i = running.indexOf(me); + if (i >= 0) running.splice(i, 1); + } + }, + }); +} diff --git a/package.json b/package.json index c995631..87dbede 100644 --- a/package.json +++ b/package.json @@ -2,9 +2,17 @@ "name": "pi-extensions", "version": "0.1.0", "description": "Custom and modified pi coding-agent extensions", - "keywords": ["pi-package"], + "keywords": [ + "pi-package" + ], "license": "MIT", "pi": { - "extensions": ["./extensions"] + "extensions": [ + "./extensions" + ] + }, + "type": "module", + "scripts": { + "test": "node --test 'test/**/*.test.mjs'" } } diff --git a/skill/SKILL.md b/skill/SKILL.md index e33b08f..7ec1750 100644 --- a/skill/SKILL.md +++ b/skill/SKILL.md @@ -1,17 +1,17 @@ --- name: pi-extensions description: >- - Use the pi extensions (pi-fork, pi-observational-memory, ssh-controlmaster) effectively in the pi coding agent harness. Load this skill only when running inside pi (detection - `fork` and `recall` are present in your tool list, or `pi --ssh` was used to start the session). pi-fork dispatches focused subtasks to forked agents at fast/balanced/deep effort tiers; pi-observational-memory compacts long sessions into recallable observations + reflections; ssh-controlmaster rewires pi's read/write/edit/bash tools to execute on a remote host over a multiplexed SSH connection. Also covers the context ladder L0-L4 and when to reach for the separate `pi-task` CLI instead of `fork` - isolated child, immutable spec, machine-checked envelope, write-boundary diff. This skill covers tier selection, task design, boundary discipline, when to use recall, and remote-pi mechanics. + Use the pi extensions (pi-fork, pi-observational-memory, ssh-controlmaster) effectively in the pi coding agent harness. Load this skill only when running inside pi (detection - `fork` and `recall` are present in your tool list, or `pi --ssh` was used to start the session). pi-fork dispatches focused subtasks to forked agents at fast/balanced/deep effort tiers; pi-observational-memory compacts long sessions into recallable observations + reflections; ssh-controlmaster rewires pi's read/write/edit/bash tools to execute on a remote host over a multiplexed SSH connection. Also covers the context ladder L0-L4 and the `task` tool (pi-task: isolated child, immutable spec, machine-checked envelope, write-boundary diff) that is the DEFAULT for delegated work, with `fork` reserved for read-only exploration and parallel opinions, plus the `fork-gate` hook that enforces the split. This skill covers rung and tier selection, task design, boundary discipline, when to use recall, and remote-pi mechanics. --- -# Pi Extensions: pi-fork, pi-observational-memory, ssh-controlmaster +# Pi Extensions: pi-fork + task/fork-gate, pi-observational-memory, ssh-controlmaster ## When to Load This Skill Load only when **both** of these are true: 1. You are running inside the **pi coding agent harness** (not Claude Code, not opencode, not any other harness). -2. The `fork` and/or `recall` tools appear in your available tool list, **or** the session was started with `pi --ssh ...`. +2. The `fork`, `task` and/or `recall` tools appear in your available tool list, **or** the session was started with `pi --ssh ...`. If you do not see those tools, this skill does not apply — skip it. Other harnesses do not have these extensions and the patterns below will not work there. @@ -71,7 +71,78 @@ ssh-controlmaster is orthogonal but composes cleanly: when pi is operating remot --- -## Part 1: pi-fork +## Part 1: delegating work — `task` and `fork` + +### Decide the rung BEFORE the brief (read this first) + +Two tools run a child agent. They differ in one thing, and it decides the +quality of what comes back: **what the child sees.** + +| tool | child sees | rung | gives you | use for | +|---|---|---|---|---| +| **`task`** (pi-extensions `task.ts`, wraps `pi-task`) | **only your spec** — goal, named files, curated facts | L0–L2 | immutable spec, PASS/FAIL envelope, per-root boundary diff, audit dir, budgets | **any delegated work that changes files or must obey a rule** — the default | +| **`fork`** (pi-fork) | **your entire branch**, brief appended last | L4 | prose report, effort tiers, parallel dispatch from one message | read-only exploration that needs this conversation; N independent opinions | + +**Pre-flight before any `fork(...)` — one *yes* makes it a `task`:** +1. Does the brief say *do not / only / never / must not*? +2. Will the child write, edit, commit or push anything? +3. Do I want a PASS/FAIL I can check, rather than prose? + +Why the text alone did not work (and why this is now enforced): the rule above +lived in this skill and in the global AGENTS.md for months and was still +violated by agents that had just read it — five fork briefs in one session on +2026-09-17, all carrying "do not", one of which returned confident verbatim +quotes that did not exist. `fork` is a *tool*: its self-recommending +description ("implementation, testing, review…") is in the tool list every turn +and survives compaction; this skill is gone after the first compaction, and +`pi-task` was a CLI to be remembered. Two structural fixes shipped 2026-09-19: + +- **`task` is a tool** (`extensions/task.ts`), so both rungs sit in the tool + list with the decision rule in their descriptions, and `promptGuidelines` + puts the rule in the system prompt where compaction cannot remove it. It + rejects, before spending a model run, the two spec errors that make a + violation certain (see "roots" below) and serialises overlapping tasks. +- **`fork-gate`** (`extensions/fork-gate.ts`) is a `tool_call` hook that BLOCKS + a fork whose brief contains a prohibition, a write boundary or a clause-initial + file-changing imperative, and returns the `task(...)` call to make instead. + It matches wording, not intent: a genuinely read-only exploration brief that + trips it is rephrased, and one that cannot be rephrased without its + prohibition needed `task` all along. `PI_FORK_GATE=off` logs instead of + blocking; `/ext` disables it entirely. + +Minimal call: + +``` +task(id="slug", goal="…verbatim; the child has NO other context…", + deliverable="…exact shape wanted…", effort="fast|balanced|deep", + read_only=false, + roots=["/abs/repo/docs", "/abs/repo/src"], # WATCHED, each diffed alone + write_allowed=["/abs/repo/docs"], # exact subset of roots + facts=["verified fact"], files=["/abs/path/to/read"]) +``` + +**Roots — the two errors the tool refuses up front.** Every root is diffed on +its own and a delta is allowed only if that *exact root string* is in +`write_allowed`. So (a) `write_allowed` must be a subset of `roots`, not a +subdirectory of one, and (b) a writable root must not lie inside a watched-only +root — the parent's porcelain would change and register a violation every time. +List the writable part as its own root and leave the enclosing repo out. This is +the shape the 2026-09-17 migration tasks used (sibling roots, `write_allowed` +naming four of them) and it passed cleanly. + +**Overlap.** Sibling `task` calls whose roots overlap would see each other's +writes as violations; the tool runs them one after another automatically. Do +not rely on that for ordering *semantics* — if B needs A's output, call B after +A returns. + +**What isolation does not fix.** L0 removes the *narrative* failures (parent +voice, invented continuity, ignored prohibitions). It does not remove +confabulation: an under-specified spec still gets a confident deliverable. The +report prints the evidence pointers under a "SPOT-CHECK THESE" heading for a +reason. + +Everything below about tiers, brief design and boundary discipline applies to +**both** tools — a `task` spec is a brief too. ### Effort tier mapping @@ -85,15 +156,15 @@ Configured in `~/.pi/agent/settings.json` under `pi-fork.effortProfiles`. The co **Rule of thumb:** start at `balanced` unless you have a specific reason to go up or down. Going too cheap on a deep task wastes a fork; going too expensive on a mechanical task is just slow. -### When to fork vs. do it yourself +### When to delegate vs. do it yourself -Fork when **any** of: +Having chosen the rung above, delegate (either tool) when **any** of: - The task requires reading many files whose contents you don't need to keep in your main context afterwards (the fork returns a dense summary; raw file contents stay in the fork's context and are discarded). - You want to run multiple analyses in **parallel** (especially: comparing N options, where independent reasoning is itself a signal — see "parallel forks" below). - The task is well-scoped enough to specify completely up front and well-bounded enough that returning a dense report is more useful than continuing the dialogue. - You are about to do something that would burn a lot of tokens on tool calls (long file reads, many bash invocations) whose output you will mostly discard. -Don't fork when: +Don't delegate when: - The work fits in your current context budget without crowding out what comes next. - The task is exploratory and you'll need to iterate based on what you find (forking turns iteration into round-trips with full task-spec rewrites). - You need to make decisions during the work that depend on context only the main thread has. @@ -175,11 +246,12 @@ sits at one extreme of it. Five rungs: | **L3** | a **truncated tail** of the parent branch | *nothing implements this* — would need a new spec key plus `--session ` | **no** | | **L4** | the **entire** parent branch | `fork(task=…)` — `getHeader()+getBranch()`, no offset or limit anywhere in the call chain | yes | -**`pi-task` is a CLI, not an extension — it will never appear in your tool list.** -Invoke it with `bash`: `/opt/pi-toolkit/bin/pi-task run ` (source at -`/workspace/pi-toolkit/bin/pi-task`, `schema` subcommand prints the spec fields). -It reads an immutable JSON spec, and "inherit the session" is not expressible in -that schema — the isolation is structural, not a request. +**`pi-task` is a CLI (`/opt/pi-toolkit/bin/pi-task`, `schema` prints the spec +fields); the `task` tool from pi-extensions wraps it** so it appears in your tool +list next to `fork`. If the tool is absent, invoke the CLI with `bash`: +`/opt/pi-toolkit/bin/pi-task run `. Either way it reads an immutable +JSON spec, and "inherit the session" is not expressible in that schema — the +isolation is structural, not a request. **Choose the lowest rung that can do the job:** @@ -292,7 +364,17 @@ When entries conflict, **the most recent observation reflects the latest known s ## Quick Reference ``` +task(id, goal, deliverable, effort, read_only, roots, write_allowed, facts, files, commands, wall_s, usd) + - L0-L2: isolated child sees ONLY the spec — DEFAULT for work that writes or has rules + - roots[] = WATCHED (each diffed alone); write_allowed[] = exact subset of roots, + never nested inside a watched-only root (the tool rejects both errors up front) + - envelope must parse or the run FAILED; spot-check evidence pointers + - overlapping-root tasks are serialised; audit: ~/.pi/agent/pi-task/-/result.json + - CLI fallback: bash /opt/pi-toolkit/bin/pi-task run (schema | selftest | run --dry-run) + fork(task=..., effort=fast|balanced|deep) # L4: child inherits your WHOLE branch + - ONLY for read-only exploration needing this conversation, or N parallel opinions + - fork-gate BLOCKS briefs with do-not/only/never, write boundaries, or "edit/commit/fix …" - state decision authority explicitly - pass verified context up front - specify deliverable shape @@ -302,13 +384,6 @@ fork(task=..., effort=fast|balanced|deep) # L4: child inherits your WHOLE b - write-capable? demand "What I did NOT do", then verify from git/fs, not the report - prohibition in the brief => not a `fast` task -bash: /opt/pi-toolkit/bin/pi-task run # L0-L2: isolated child, NOT a tool - - schema | selftest | run [--dry-run] - - context.facts (pasted) / .files (names only) / .commands - - roots[] = WATCHED, write_allowed[] = CHANGEABLE subset - - envelope must parse or the run FAILED - - audit + cost: ~/.pi/agent/pi-task/-/result.json - recall(id=<12-char-hex>) - only when stakes justify the cost - id must already be visible in your context @@ -317,8 +392,9 @@ recall(id=<12-char-hex>) ``` ~/.pi/agent/settings.json - pi-fork.effortProfiles — model + thinking-depth per tier + pi-fork.effortProfiles — model + thinking-depth per tier (used by BOTH fork and task) pi-fork.defaultEffort — usually "balanced" + env PI_FORK_GATE=off — fork-gate logs instead of blocking (default: block) observational-memory.* — token thresholds, model, agentMaxTurns observational-memory.debugLog: true — opt-in NDJSON telemetry at ~/.pi/agent/observational-memory/debug/.ndjson (off by default) diff --git a/test/fork-gate.test.mjs b/test/fork-gate.test.mjs new file mode 100644 index 0000000..bff600e --- /dev/null +++ b/test/fork-gate.test.mjs @@ -0,0 +1,76 @@ +// Two-sided test of the fork-gate classifier: briefs that MUST be redirected +// to pi-task and briefs that MUST stay forkable. A gate that only ever blocks +// has not been shown to discriminate. Runs the shipped .ts directly (node >= 23 +// strips types natively; the only import is type-only). +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { classifyForkBrief, blockReason, GATE_RULES } from "../extensions/fork-gate.ts"; + +// Real shapes from the sessions that motivated the gate (2026-09-17, tor-ms22). +const MUST_BLOCK = [ + ["prohibition", "Migrate docs/level-01-basics. Do NOT modify files outside docs/level-01-basics/ or tutorials/01-basics/."], + ["prohibition", "Read the repo and report. Don't change anything."], + ["prohibition", "You must not commit."], + ["prohibition", "Never touch the inventory."], + ["prohibition", "Please refrain from editing mkdocs.yml"], + ["boundary", "Only touch the three files listed above."], + ["boundary", "Edit only the README; nothing else."], + ["boundary", "This is a read-only review of the role."], + ["boundary", "Stay within roles/static_site/ for all changes."], + ["boundary", "Changes are limited to the docs directory"], + ["mutation", "Implement the FQCN sweep across docs/ and run mkdocs --strict."], + ["mutation", "Fix the eleven failing playbooks under solutions/."], + ["mutation", "Review the playbooks, then commit the result on develop."], + ["mutation", "Rewrite Exercise 5 to use the inventory plugin."], + ["mutation", "Please update docs/getting-started/structure.md from the real tree."], +]; + +// Fork's legitimate uses: read-only exploration against this conversation, and +// opinions. Includes phrasings that a naive verb list would misfire on. +const MUST_PASS = [ + "Find where the runner config is written to disk and report file:line.", + "Which function creates the temp files for the session? Report the call chain.", + "Compare the three staging options we discussed and give an independent opinion.", + "Summarize the architecture of the static_site role in 200 words.", + "Write a summary of what the last five commits changed.", + "Give me an update on how the mailbox derivation handles ready-status events.", + "Report which files were modified by CI run 23 according to the logs.", + "Explain the difference between compose and keyed_groups in the openstack inventory plugin.", + "List every place the 60000 ms default is documented; return paths and line numbers.", + "Is the fixed deadline applied in the feed path? Quote the code with a pointer.", +]; + +test("every hazardous brief is blocked with the expected class", () => { + for (const [kind, brief] of MUST_BLOCK) { + const v = classifyForkBrief(brief); + assert.equal(v.block, true, `should block: ${brief}`); + assert.equal(v.kind, kind, `wrong class for: ${brief} (matched "${v.matched}")`); + assert.ok(v.matched && v.matched.length > 0, "reports what matched"); + } +}); + +test("every legitimate exploration brief passes", () => { + for (const brief of MUST_PASS) { + const v = classifyForkBrief(brief); + assert.equal(v.block, false, `false positive on: ${brief} (${v.kind}: "${v.matched}")`); + } +}); + +test("non-string / empty briefs never block (gate must not break fork on odd input)", () => { + for (const x of [undefined, null, 42, "", {}, []]) { + assert.equal(classifyForkBrief(x).block, false); + } +}); + +test("block reason carries the redirect: task(...) call, CLI fallback, sequential-roots rule", () => { + const r = blockReason(classifyForkBrief(MUST_BLOCK[0][1])); + for (const needle of ["task(id=", "write_allowed=", "/opt/pi-toolkit/bin/pi-task run", "overlap", "matches wording, not intent"]) { + assert.ok(r.includes(needle), `reason lacks "${needle}"`); + } + assert.ok(r.includes('"Do NOT"'), "reason names the words that fired"); +}); + +test("rules are ordered prohibition → boundary → mutation and each has a why", () => { + assert.deepEqual(GATE_RULES.map((r) => r.kind), ["prohibition", "boundary", "mutation"]); + for (const r of GATE_RULES) assert.ok(r.why.length > 20); +}); diff --git a/test/task.test.mjs b/test/task.test.mjs new file mode 100644 index 0000000..3aa85cb --- /dev/null +++ b/test/task.test.mjs @@ -0,0 +1,73 @@ +// Tests for the pure parts of task.ts: root overlap and the pre-launch spec +// validation that turns a guaranteed false FAIL into an immediate error. +// task.ts imports typebox / @earendil-works/pi-ai at runtime (only resolvable +// inside pi), so the functions are cut out of the shipped source by brace +// matching and type-stripped — the same approach as mempalace-toolkit's tests. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync, writeFileSync, mkdtempSync, mkdirSync } from "node:fs"; +import { stripTypeScriptTypes } from "node:module"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +const src = readFileSync(new URL("../extensions/task.ts", import.meta.url), "utf8"); +function fn(name) { + const start = src.indexOf(`export function ${name}(`); + if (start < 0) throw new Error(`not found: ${name}`); + let depth = 0, seen = false; + for (let i = start; i < src.length; i++) { + if (src[i] === "{") { depth++; seen = true; } + else if (src[i] === "}") { depth--; if (seen && depth === 0) return src.slice(start, i + 1); } + } + throw new Error(`unterminated: ${name}`); +} +const work = mkdtempSync(path.join(tmpdir(), "task-test-")); +const modPath = path.join(work, "pure.mjs"); +writeFileSync(modPath, stripTypeScriptTypes( + `import { existsSync } from "node:fs";\nimport path from "node:path";\n` + + [fn("rootsOverlap"), fn("anyOverlap"), fn("validateRoots")].join("\n\n"), { mode: "strip" })); +const { rootsOverlap, anyOverlap, validateRoots } = await import(modPath); + +// Real directories, since validateRoots checks existence. +const repo = path.join(work, "repo"); const docs = path.join(repo, "docs"); const srcd = path.join(repo, "src"); const other = path.join(work, "other"); +for (const d of [docs, srcd, other]) mkdirSync(d, { recursive: true }); + +test("rootsOverlap: identity, containment both ways, siblings, prefix-but-not-parent", () => { + assert.equal(rootsOverlap(repo, repo), true); + assert.equal(rootsOverlap(repo, docs), true); + assert.equal(rootsOverlap(docs, repo), true); + assert.equal(rootsOverlap(docs, srcd), false); + assert.equal(rootsOverlap(path.join(work, "repo"), path.join(work, "repo2")), false, "'/x/repo' is not a parent of '/x/repo2'"); + assert.equal(anyOverlap([docs], [srcd, other]), false); + assert.equal(anyOverlap([docs, other], [srcd, other]), true); +}); + +test("validateRoots: read-only spec with existing absolute roots is accepted", () => { + assert.deepEqual(validateRoots([docs, srcd], undefined, true), []); +}); + +test("validateRoots: the migrate-0N shape (sibling roots, write_allowed subset) is accepted", () => { + assert.deepEqual(validateRoots([docs, srcd, other], [docs], false), []); +}); + +test("validateRoots: write task without write_allowed is rejected (violations undetectable)", () => { + const p = validateRoots([docs], undefined, false); + assert.equal(p.length, 1); assert.match(p[0], /requires write_allowed/); +}); + +test("validateRoots: write_allowed must be an exact root string", () => { + const p = validateRoots([repo], [docs], false); + assert.ok(p.some((x) => /not one of roots/.test(x)), p.join("\n")); +}); + +test("validateRoots: writable root nested in a watched-only root is rejected (certain false FAIL)", () => { + const p = validateRoots([repo, docs], [docs], false); + assert.ok(p.some((x) => /nested in/.test(x)), p.join("\n")); +}); + +test("validateRoots: read_only with write_allowed, relative root, missing root are each named", () => { + const p = validateRoots(["relative/path", path.join(work, "nope"), docs], [docs], true); + assert.ok(p.some((x) => /not absolute/.test(x))); + assert.ok(p.some((x) => /does not exist/.test(x))); + assert.ok(p.some((x) => /pick one/.test(x))); +});