diff --git a/.env.example b/.env.example index 4c8ecc4..bb38ab5 100644 --- a/.env.example +++ b/.env.example @@ -76,11 +76,19 @@ OTEL_CAPTURE_IO_MAX_CHARS=8000 # ── Agent layer (optional; PRD §5) ─────────────────────────────────────────── # A board agent above eight role agents. OFF by default — it is the one part of # this repo written for it rather than extracted from production, so it does not -# stand in front of a first-time reader. It cannot write: role agents get a -# read-only adapter wrapper, and Pass 2c remains the only writer. +# stand in front of a first-time reader. Role agents never write: they get a +# read-only adapter wrapper. With BOARD_AGENT_WRITES off — the default — Pass 2c +# remains the only writer. AGENTS_ENABLED=false # Items handed to a role agent per run. Each is a model call. AGENT_MAX_DELEGATIONS=8 +# Let the BOARD agent perform the writes instead of Pass 2c. OFF by default. +# Off, no model is in the write path at all — no write tool exists to reach. +# On, the board agent writes the already-gated plan through a governed adapter, +# which is what PRD §5 means by "authority to write" and what production runs. +# Every write it originates is re-run through the same deterministic gates, so a +# write those gates refuse becomes a hold. Requires AGENTS_ENABLED. +BOARD_AGENT_WRITES=false # ── Dispute arbiter (optional) ──────────────────────────────────────────────── # Resolve a Pass 2a-vs-blind-read write-level dispute against live tracker state diff --git a/AGENTS.md b/AGENTS.md index 6578638..b81ecd4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,8 +18,8 @@ npm run demo -- --agents # replays the agent recording, offline |---|---|---| | How many | one per run | one per archetype | | Decides | which items need a closer look | how an item reads to its owner | -| Tools | none — it orchestrates | `get_task`, `get_task_comments`, `search_tasks` | -| Can write | **no** | **no** | +| Tools | none by default; the read tools **plus writes** under `BOARD_AGENT_WRITES` | `get_task`, `get_task_comments`, `search_tasks` | +| Can write | **no** by default · **yes, through the gates** under `BOARD_AGENT_WRITES` | **no**, in every configuration | | Built from | `boardAgent.ts` | the loop + its profile + its state | A role agent is the existing tool loop given three things that already existed: its **profile** @@ -29,7 +29,8 @@ memory, and `readOnlyTracker` as its tools. See [ROLES.md](ROLES.md). ## Where it sits ``` -… 2a categorization → 2b contract check → [ AGENT LAYER ] → 2c execute → 2d audit +default … 2a → 2b → [ AGENT LAYER ] → 2c execute ───────────→ 2d audit +BOARD_AGENT_WRITES=1 … 2a → 2b → [ AGENT LAYER ] → board agent writes ──→ 2d audit ``` **After every gate, before the writer.** Both halves of that matter: @@ -40,16 +41,23 @@ memory, and `readOnlyTracker` as its tools. See [ROLES.md](ROLES.md). ## Two guarantees, both structural -### 1. An agent cannot write +### 1. A role agent cannot write Not because the prompt asks it not to — because `readOnlyTracker` wraps the adapter and refuses every `apply()`, and no write tool is offered in the first place. Prompt text is a request; a wrapper is a guarantee. A model that has been jailbroken, confused, or fed a malicious transcript still has no -code path to a mutation. +code path to a mutation. This holds in every configuration; there is no flag that gives a role agent +a write tool. -**Pass 2c remains the only writer, and it has no model in it.** The agent decides; deterministic code -executes. That is also what production does: its board agent *proposes*, and a script enforces the -protected-status guard, the duplicate check and read-only mode. +**By default, Pass 2c is the only writer and it has no model in it.** The agent decides; +deterministic code executes. + +**`BOARD_AGENT_WRITES` changes who performs the write, and only for the board agent.** On, the board +agent is handed the already-gated plan plus write tools, and writes it through `governedTracker` — +which re-runs every deterministic gate over anything it originates, so a write the gates refuse +becomes a hold rather than a card. That is the shape PRD §5 describes and the shape production runs. +Off — the default — none of that code is in the process at all. The guarantee in each mode is stated +exactly in `SECURITY.md`, including how the second one is smaller than the first. ### 2. An agent cannot claim a write that did not happen @@ -131,16 +139,24 @@ test, not by recording"** — and a reader who wants to see it fire should run t ### This is what "authority to write" means The internal spec this repo was built from describes the Board agent as *"the orchestrator above the -role agents, holding board state and authority to write."* Read literally that sounds like a write -handle, and building it that way would put a model in the write path and cost the guarantee the -README leads with. (That spec is private and not shipped in this repo — the quote is given here in -full so the argument stands on its own without it.) - -**Production does not work that way either.** Its board agent proposes, and one script enforces the -protected-status guard, the duplicate check and read-only mode. "Authority to write" there means *its -decisions result in writes* — not that it performs them. Pass 2c is this repo's equivalent of that -script. Proposing into the gates is the faithful port: the agent genuinely decides, and something -deterministic and auditable is still the only thing that writes. +role agents, holding board state and authority to write."* (That spec is private and not shipped +here — the quote is given in full so the argument stands without it.) + +**Production means that literally.** Its board agent runs a create command, and a guard layer decides +whether the command lands: the protected-status guard, the duplicate check, read-only mode. The agent +performs the write; the guards govern it. An earlier version of this file claimed production's board +agent only *proposed* and never performed writes. That was wrong, and it mattered — it was used here +to argue that a read-only board agent was the faithful port when it was actually the divergent one. + +`BOARD_AGENT_WRITES` is that shape, ported. On, the board agent writes through `governedTracker`, +which is this repo's equivalent of that guard layer: every write it originates is rebuilt into a +manifest item and re-run through the same gates the pipeline's own answer faced. + +**The default is still off, and that is a deliberate smaller claim rather than the faithful one.** +Nothing here has governed a real board for months, the way the pipeline has. Defaulting a model into +the write path of a repo people clone and point at their own tracker is not a claim this repo has +earned. Off, no model reaches the tracker at all and the README's headline property is literal; on, +it is the production shape and `SECURITY.md` states precisely what narrows. An earlier version of this layer could change one prose field. That was safe, and it was not orchestration. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 4bff3e4..97059f9 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -35,11 +35,14 @@ means. | — | evidence prefetch | Fetch card history for the candidates 2a will need. Host-side, so 2a is a plain completion. | | **2a** | categorization | `NEW_TASK` / `DUPLICATE` / `SUBTASK` / `UPDATE` / `RELATE`, against the live board. | | **2b** | contract check | An independent **blind** re-derivation. Disagreement becomes a human hold. | -| **2c** | execute | The only writer. Deterministic — **no model in the write path.** | +| **2c** | execute | The writer. Deterministic — **no model in the write path.** `BOARD_AGENT_WRITES` substitutes the board agent here; see AGENTS.md. | | **2d** | audit | Did the board end up how 2c said it would? | -Passes 0–1.7 read; 2a–2b decide; 2c writes; 2d verifies. A model never touches the write itself — 2c -takes a plan and applies it, which is why a wrong write requires a wrong *plan*, not a stray token. +Passes 0–1.7 read; 2a–2b decide; 2c writes; 2d verifies. By default a model never touches the write +itself — 2c takes a plan and applies it, which is why a wrong write requires a wrong *plan*, not a +stray token. Under `BOARD_AGENT_WRITES` the board agent performs the write instead, and the +equivalent statement is that a wrong write needs a wrong plan that *also survives every gate a second +time*. ## Pass 2b is blind, and that is the headline claim diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d7c981..ac06e0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,7 +59,7 @@ to what runs internally, the tuned few-shot examples are replaced with generic o [EXTRACTION.md](EXTRACTION.md)). **Pipeline.** Eight passes, 0 through 2d: cleanup, inventory, critic, consolidator, categorization, -a **blind** contract check that never sees the categorization answer, the only writer, and a +a **blind** contract check that never sees the categorization answer, the writer, and a post-write audit. Eight offline scenarios replay real recorded model responses through the real prompts, parsers and gates — `npm run demo`. @@ -89,6 +89,14 @@ live source to running the pipeline over it, planning by default, writing only w It may propose a different category, list, assignee or description; every proposal is re-run through the same gates, so a proposal the gates refuse becomes a hold, never a write. +**Board-agent write authority (`BOARD_AGENT_WRITES`), also off by default.** PRD §5 gives the board +agent "authority to write", and production means that literally — its board agent runs a create +command behind a guard layer. This flag is that shape: the board agent gets write tools behind +`governedTracker`, which rebuilds every write it originates into a manifest item and re-runs the full +deterministic gate set over it, so a write the gates refuse becomes a hold. Off, Pass 2c writes and no +model reaches the tracker at all. The prompt-injection guarantee differs between the two modes, and +`SECURITY.md` now states each one exactly rather than stating the stronger one twice. + **No accuracy claimed.** Volume and hold rate are reported from 711 real items across 49 production runs; precision and recall are not, because the only alternative to a hand-labelled ground truth that doesn't exist is a model grading a model. See [LIMITATIONS.md](LIMITATIONS.md). diff --git a/EXTRACTION.md b/EXTRACTION.md index d8f1c14..28b6cf8 100644 --- a/EXTRACTION.md +++ b/EXTRACTION.md @@ -14,14 +14,14 @@ this line is how you work out whether the other already has it. | | Production | Here | Why | |---|---|---|---| | Passes 2a/2b | Tool-using agents that can fetch extra card history on demand | Plain completions; all evidence pre-fetched host-side. An optional agent layer sits *above* them, off by default | The default path costs duplicate recall on semantically-worded matches; the whole-board Jaccard backstop and the evidence-citation hold gate catch the fallout as human holds, not silent creates. | -| The agent loop itself | Delegated to a separate agent runtime — the pipeline is a *client* of it, over CLI and HTTP | **Written for this repo**, on the existing read-only tool loop | The only component here that is not an extraction. The production loop is a different product whose prompts read internal workspace files; porting it was neither possible nor in scope. Stated plainly in `LIMITATIONS.md`, and the reason `AGENTS_ENABLED` defaults to false. | -| Read-only enforcement | Structural, via a wrapper script that refuses write subcommands | Enforced at the adapter | Same guarantee, fewer moving parts. | +| The agent loop itself | Delegated to a separate agent runtime — the pipeline is a *client* of it, over CLI and HTTP | **Written for this repo**, on the existing tool loop (read-only by default; the board agent gets governed write tools under `BOARD_AGENT_WRITES`) | The only component here that is not an extraction. The production loop is a different product whose prompts read internal workspace files; porting it was neither possible nor in scope. Stated plainly in `LIMITATIONS.md`, and the reason `AGENTS_ENABLED` defaults to false. | +| Write enforcement | The board agent runs a write command; a wrapper script enforces the protected-status guard, the duplicate check and read-only mode as it does | Two adapter wrappers: `readOnlyTracker` refuses every write (role agents, always), `governedTracker` re-runs the full deterministic gate set over each write (the board agent, under `BOARD_AGENT_WRITES`) | Same shape, one layer closer to the thing it guards. An earlier version of this row said production's agent only *proposed*; that was wrong, and it was the sentence used to justify shipping a board agent that could not write at all. | | Ingestion — transport | 8 webhooks, 14 cron routes, an Express app | A reference wiring, not a port: `npm run poll` (cron-able) and `npm run serve` (signature-verified GitHub/Slack webhooks, re-pulling rather than parsing the delivery payload) | Neither is the production infra — no TLS termination, process supervision, queue durability or horizontal scale. Built to prove `runPipeline(source, deps)` reaches from a real trigger, not to be deployed as-is. | | Ingestion — reads and shapes | Slack, Gmail, GitHub, Drive and meeting transcripts | Read clients for GitHub, Gmail, Drive and Slack; normalizers for five payload shapes | Reading a service is not the same concern as scheduling the read, and conflating them cost this repo three sources — see below. | | Which sources the pipeline *accepts* | A closed union — `kind: 'meeting' \| 'channel_sweep'` — on a source struct shaped for those two: `transcript`, `rawTranscript`, `participantLine`, `channelId`. GitHub, Gmail and Drive activity reaches the board through the separate agent runtime, not this pipeline | Five kinds behind one `IngestedSource`, all running the identical Pass 0→2d chain | **This repo generalized the contract; production did not.** Two meeting-only gates are the reason production's is closed — ASR speaker-confidence provenance feeding a Pass 2b legitimacy check, and visual grounding over video frames. Neither was extracted (see below), and with them gone nothing in the pass logic reads source kind except to pick a noun for a prompt. The generalization is real and code-verified, but it is **this repo's**, not a description of what production runs today. | | Per-person agents | 12 live agent runtimes with their own state and tool access | 8 role *archetypes* — a profile, routing keywords and a state file each, drivable as read-only agents | Archetypes de-identify by construction: there is no real name to strip, because the concept is generic. They are load-bearing either way — the profile shapes the prompt even with agents off. | | Per-role state | A `STATE.md` and journal per agent, rewritten on a schedule | One JSON file per archetype: what that role currently has open, plus human-maintained context | Same idea, scoped to what a pipeline can honestly maintain. Production's version is an agent's working memory; here it is a memo the pipeline writes after each run and reads back into the next one's prompt. No journal — nothing here would read one. | -| Read-only enforcement in agent passes | An environment variable read by a shell script | A wrapper around the adapter whose `apply()` refuses | Same intent, fewer moving parts, and the guarantee sits next to the thing it guards. | +| Read-only enforcement in agent passes | An environment variable read by a shell script | A wrapper around the adapter whose `apply()` refuses | Same intent, fewer moving parts, and the guarantee sits next to the thing it guards. Unchanged by `BOARD_AGENT_WRITES` — that flag reaches the board agent only; no role agent gets a write tool in any configuration. | | Tracker client | A 2,034-line bash script shelling out from TypeScript | Typed HTTP adapters for ClickUp and Linear | Most of that script was `jq` shaping. Three pieces were real logic and were carried across; see below. | | Retrieval | A live vector substrate | A declared `Retriever` interface, wired into 2a/2b; ships `nullRetriever` (default) and `localRetriever` (opt-in, flat-file Jaccard ranking) | Retrieval quality has never been measured for either implementation, so no claim about it would be falsifiable. The interface ships so the architecture visibly accommodates a knowledge layer; the live vector substrate does not, because nothing could be said about it honestly. | diff --git a/LIMITATIONS.md b/LIMITATIONS.md index 84f3ff7..98c99c2 100644 --- a/LIMITATIONS.md +++ b/LIMITATIONS.md @@ -297,8 +297,11 @@ That is why `AGENTS_ENABLED` defaults to **false**. What turning it on can and c - It **can** propose a description, a category, a list or an assignee, and raise an ownership doubt. - Every proposal is re-run through `applyGates` — the same gates Pass 2b uses, not a copy — so one the gates refuse becomes a human hold rather than a write. -- It **cannot** write anything, and **cannot un-hold**: agents only ever see items that already - passed the gates, so there is no path from an agent to an item a gate stopped. +- It **cannot un-hold**: agents only ever see items that already + passed the gates, so there is no path from an agent to an item a gate stopped. A **role** agent + cannot write in any configuration. The **board** agent cannot either, unless `BOARD_AGENT_WRITES` + is on — with it on, it performs the write, and every write it originates is re-gated first. See + AGENTS.md, and SECURITY.md for how the injection guarantee narrows in that mode. Measured across both recordings and all eight scenarios, **no proposal has changed a final category** — `agentReplay.test.ts` compares each item's final category against Pass 2a's and fails diff --git a/README.md b/README.md index af3e318..cc29640 100644 --- a/README.md +++ b/README.md @@ -75,7 +75,8 @@ source (transcript | channel | github | gmail | drive) Pass 1.7 consolidator ─ merge, dedupe, anchor Pass 2a categorization ─ NEW_TASK | DUPLICATE | SUBTASK | UPDATE, against the live board Pass 2b contract check ─ a BLIND re-derivation; a genuinely different WRITE holds - Pass 2c execute ─ the only writer. Deterministic. No model in the write path. + Pass 2c execute ─ the writer. Deterministic. No model in the write path. + (BOARD_AGENT_WRITES hands this to the board agent instead.) Pass 2d audit ─ did the board end up how 2c said it would? ``` @@ -213,8 +214,14 @@ and demo stays offline because they start from a recorded payload rather than a **An optional agent layer** sits between the gates and the writer: a board agent that delegates to eight role agents with **read-only** tools. It is off by default. It may **propose** a different category, list, assignee or description — and every proposal is re-run through the same -gates, so one the gates refuse becomes a hold rather than a write. **The agent never writes, and -never un-holds.** See [AGENTS.md](AGENTS.md). +gates, so one the gates refuse becomes a hold rather than a write. **Role agents never write, and no +agent ever un-holds.** + +A second flag, `BOARD_AGENT_WRITES`, hands the write itself to the board agent — the "authority to +write" PRD §5 describes, and the shape production runs. It is also off by default. On, the agent gets +write tools behind `governedTracker`, which re-runs every deterministic gate over anything it +originates; a write the gates refuse becomes a hold. Off, no model reaches the tracker at all. See +[AGENTS.md](AGENTS.md), and [SECURITY.md](SECURITY.md) for exactly which guarantee each mode buys. **The rule that makes the tracker seam real:** the pipeline speaks canonical member names and list keys; only an adapter ever sees a tracker id. Every gate, prompt, parser and the whole categorization diff --git a/SECURITY.md b/SECURITY.md index 98e64ab..62f6c00 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -25,9 +25,31 @@ documents, and every tool result: copies the content it flagged is a second store of the thing worth protecting. **This is not a sandbox.** The pattern list is regexes; a rephrased attack walks past it. What bounds -the damage is structural and downstream: the writer is deterministic, every write passes the gates, -and Pass 2b re-derives the categorization blind. A successful injection can mislead a -categorization. It cannot author a write. +the damage is structural and downstream: every write passes the gates, and Pass 2b re-derives the +categorization blind. + +**In the default configuration, the writer is deterministic and the guarantee is absolute:** + +> A successful injection can mislead a categorization. It cannot author a write. + +There is no code path from a model turn to a mutation, because no write tool exists for one to reach. + +**With `BOARD_AGENT_WRITES` on, the guarantee narrows, and the narrower version is the one to hold us +to:** + +> A successful injection cannot author a write **the deterministic gates would not already have +> approved.** + +That mode hands the board agent write tools, so a model does reach the tracker. Every write it +originates is rebuilt into a manifest item and re-run through the same gates the pipeline's own +answer faced — routing, roster, evidence, duplicate, critical — and a write those gates refuse +becomes a hold. What an injection could still do is steer a write that passes every gate: a +plausible card, on a real list, for a real person. What it cannot do is rotate a credential, reach an +off-roster assignee, or invent a list. The gate list in `ARCHITECTURE.md` is the actual boundary, and +it is worth reading before turning the flag on rather than after. + +The flag is off by default, and the paragraph above is the reason it is a flag rather than the +default. ## Which commands touch a live service diff --git a/src/agents/agents.test.ts b/src/agents/agents.test.ts index 8f330ea..9e6beba 100644 --- a/src/agents/agents.test.ts +++ b/src/agents/agents.test.ts @@ -14,7 +14,7 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { type DelegationResult, MAX_DELEGATIONS, delegateToRoleAgents, selectForDelegation, summariseRun } from './boardAgent'; +import { type DelegationResult, MAX_DELEGATIONS, delegateToRoleAgents, selectForDelegation, summariseRun, writeBoard } from './boardAgent'; import { type RoleEnrichment, buildRoleAgentPrompt, parseRoleReply, roleOf, runRoleAgent } from './roleAgent'; import { applyProposals } from '../pipeline/run'; import { applyGates } from '../pipeline/passes/contractCheck'; @@ -500,3 +500,109 @@ describe('the run summary reports the executor, never the intention', () => { expect(out).toMatch(/ownership doubts raised: Avery Chen — looks like design work/); }); }); + +/** + * The board agent as writer (`BOARD_AGENT_WRITES`). + * + * The read-only guarantee above describes the default and still holds there. These cover the one + * configuration where a model reaches the tracker, and the property that matters most in it is not + * "the write worked" — it is that the *count* of writes comes from the tracker rather than from the + * model's account of itself. A model narrating its own run will describe what it meant to do. + */ +describe('writeBoard', () => { + const board = () => memoryTracker({ tasks: [] }); + + const createCall = (title: string, id = 'c1') => ({ + toolCalls: [ + { + id, + name: 'create_task', + arguments: { title, list_key: 'backend', assignee: 'Avery Chen', description: 'a real description' }, + }, + ], + }); + + it('writes the approved plan and counts what the tracker actually did', async () => { + const tracker = board(); + const model = scripted([createCall('Add rate limiting'), { text: 'done' }]); + + const out = await writeBoard([item()], { model, tracker, snapshot: new Map() }); + + expect(out.created).toBe(1); + expect(await tracker.listTasks()).toHaveLength(1); + }); + + /** + * The anti-fabrication rule, at the one place it could actually be violated. The model claims two + * creates in prose and performs one; the tally must follow the tracker. + */ + it('counts the write, not the claim', async () => { + const tracker = board(); + const model = scripted([ + createCall('Add rate limiting'), + { text: 'Created 5 tasks: rate limiting, dashboards, alerts, docs, and the runbook.' }, + ]); + + const out = await writeBoard([item()], { model, tracker, snapshot: new Map() }); + + expect(out.created).toBe(1); + expect(await tracker.listTasks()).toHaveLength(1); + }); + + /** A write the gates refuse is counted as refused and never reaches the board. */ + it('refuses an off-roster write and reports it as refused', async () => { + const tracker = board(); + const model = scripted([ + { + toolCalls: [ + { + id: 'c1', + name: 'create_task', + arguments: { title: 'Ship it', list_key: 'backend', assignee: 'Mallory Stranger', description: 'x' }, + }, + ], + }, + { text: 'done' }, + ]); + + const out = await writeBoard([], { model, tracker, snapshot: new Map() }); + + expect(out.refused).toBe(1); + expect(out.created).toBe(0); + expect(await tracker.listTasks()).toHaveLength(0); + }); + + /** + * The refusal has to reach the model as a refusal. Told nothing, it retries the same rejected + * write until the turn cap; told why, it can do something else. + */ + it('tells the model why a write was refused', async () => { + const model = scripted([ + { + toolCalls: [ + { + id: 'c1', + name: 'create_task', + arguments: { title: 'Ship it', list_key: 'backend', assignee: 'Mallory Stranger', description: 'x' }, + }, + ], + }, + { text: 'understood' }, + ]); + + await writeBoard([], { model, tracker: board(), snapshot: new Map() }); + + const toolReply = model.seen[1]?.messages.find((m) => m.role === 'tool'); + expect(String(toolReply?.content)).toMatch(/REFUSED/); + expect(String(toolReply?.content)).toMatch(/roster/i); + }); + + it('offers the write tools alongside the read ones', async () => { + const model = scripted([{ text: 'nothing to do' }]); + await writeBoard([], { model, tracker: board(), snapshot: new Map() }); + + const offered = (model.seen[0]?.tools ?? []).map((t) => t.name); + expect(offered).toContain('create_task'); + expect(offered).toContain('get_task'); + }); +}); diff --git a/src/agents/boardAgent.ts b/src/agents/boardAgent.ts index bf45429..f910177 100644 --- a/src/agents/boardAgent.ts +++ b/src/agents/boardAgent.ts @@ -23,9 +23,14 @@ * agent is handed the outcome rather than asked what happened. The rule is enforced by construction * rather than by asking, which is the same reasoning as `readOnlyTracker`. */ -import type { ExecuteResult } from '../pipeline/passes/execute'; +import { type ExecuteResult, type ExecutedAction, type PlanContext, planOperations } from '../pipeline/passes/execute'; import type { CategorizationItem } from '../pipeline/parsing/categorizationManifest'; import type { HeldItem } from '../pipeline/gates/contractGates'; +import { approvedOpSet, governedTracker } from '../pipeline/gates/governedTracker'; +import type { DeterministicGateOptions } from '../pipeline/passes/contractCheck'; +import { READ_ONLY_TOOLS, WRITE_TOOLS, makeToolLoopRunner } from '../pipeline/toolLoop'; +import { screenedPrimary } from '../utils/security'; +import type { BoardTask, OpOutcome, TrackerAdapter, TrackerOperation } from '../trackers'; import { type RoleEnrichment, type RoleAgentDeps, roleOf, runRoleAgent } from './roleAgent'; export interface BoardAgentDeps extends RoleAgentDeps { @@ -157,3 +162,186 @@ export function summariseRun(exec: ExecuteResult, held: HeldItem[], delegations: return lines.join('\n'); } + +// ── The board agent as writer (BOARD_AGENT_WRITES) ─────────────────────────── +// +// Everything above this line runs whenever the agent layer is on. Everything below runs only when +// `BOARD_AGENT_WRITES` is also on, and it is the one configuration in which a model reaches the +// tracker at all. +// +// **Why this exists.** PRD §5 gives the board agent "authority to write", and production means that +// literally: its board agent runs a create command and a guard layer decides whether the command +// lands. This is that shape. The default — Pass 2c, no model — is the smaller claim, and it stays +// the default for the reason `AGENTS_ENABLED` is off: it is the claim this repo can make without +// asking anyone to trust a model with a mutation. +// +// **What the agent adds over Pass 2c.** Pass 2c applies a plan exactly. The agent can look at the +// board first and change its mind: comment on the card that already covers this instead of creating +// a second one, or merge two items that turned out to be the same work. That is judgement Pass 2c +// structurally cannot exercise, and it is what production's board agent spends its turns on. +// +// **What it cannot do.** Originate a write the gates refuse. See `gates/governedTracker.ts`. + +export interface BoardWriteDeps { + model: RoleAgentDeps['model']; + tracker: TrackerAdapter; + snapshot: Map; + gateOpts?: DeterministicGateOptions; + planCtx?: PlanContext; + maxIterations?: number; + onHold?: (held: HeldItem, op: TrackerOperation) => void; + onEvent?: RoleAgentDeps['onEvent']; +} + +const OUTCOME_ORDER: Array = ['failed', 'refused', 'unsupported', 'applied', 'unchanged']; + +/** + * Have the board agent write the approved plan. + * + * Returns the same `ExecuteResult` Pass 2c returns, so Pass 2d, the `executed` event, role-state and + * the run summary all work unchanged — none of them should need to know which writer ran. + * + * **The tally is built from outcomes, never from the model's account of itself.** That is the + * anti-fabrication rule from `summariseRun` above, and it matters more here than anywhere else in the + * repo: this is the one place a model could claim a card it never created. + */ +export async function writeBoard(items: CategorizationItem[], deps: BoardWriteDeps): Promise { + const plan = planOperations(items, deps.planCtx ?? {}); + + // Which cards the agent actually opened the history of. Becomes `tier2Cited` at the gate, so the + // evidence check here is a fact about what it read rather than a claim it made in prose. + const commentsRead = new Set(); + + const performed: Array<{ op: TrackerOperation; outcome: OpOutcome }> = []; + const governed = governedTracker(deps.tracker, { + approvedOps: approvedOpSet(plan.flatMap((a) => a.ops)), + snapshot: deps.snapshot, + ...(deps.gateOpts ? { gateOpts: deps.gateOpts } : {}), + readComments: (id) => commentsRead.has(id), + ...(deps.onHold ? { onHold: deps.onHold } : {}), + }); + + // Record-and-forward, so the tally below counts what the tracker did rather than what the loop + // believes it asked for. + const recording: TrackerAdapter = { + ...governed, + async apply(op) { + const outcome = await governed.apply(op); + performed.push({ op, outcome }); + return outcome; + }, + }; + + const run = makeToolLoopRunner({ + model: deps.model, + tracker: recording, + tools: [...READ_ONLY_TOOLS, ...WRITE_TOOLS], + writable: true, // `recording` is already governed; wrapping it read-only would refuse everything + ...(deps.maxIterations != null ? { maxIterations: deps.maxIterations } : {}), + onEvent: (e) => { + if (e.kind === 'tool' && e.name === 'get_task_comments') commentsRead.add(String(e.args.task_id ?? '')); + deps.onEvent?.(e); + }, + }); + + try { + await run(buildBoardWritePrompt(items, deps.tracker.renderSnapshot([...deps.snapshot.values()])), 'board/write'); + } catch (err) { + // **A loop that wrote nothing rethrows.** It reached the tracker zero times, so there is no + // partial result to report and every number below would be a zero that looks like a decision. + // A missing cassette lands here, and it has to be as loud as it is everywhere else in this repo: + // swallowing it produced a run that printed a tidy "0 created" and left Pass 2d to infer the + // problem from four mismatches, which is precisely the quiet-wrong-number failure the cassette + // client refuses to ship. + if (performed.length === 0) throw err; + + // Some writes did land. Those are real, they are on the board, and the honest move is to report + // them and let Pass 2d name the gap — a half-written run that counts itself correctly is + // recoverable in a way one that claims success is not. + deps.onEvent?.({ kind: 'cap-hit', iterations: performed.length }); + } + + return tally(plan, performed); +} + +/** + * The prompt. Deliberately short on encouragement and specific about the one thing that differs from + * Pass 2c: it may disagree with the plan, and the interesting case is when it should. + */ +export function buildBoardWritePrompt(items: CategorizationItem[], boardText: string): string { + return [ + 'You are the board agent. The pipeline has already decided what should happen to each item below,', + 'and every one of them has passed every deterministic gate. Your job is to write them to the board.', + '', + 'THE BOARD RIGHT NOW:', + screenedPrimary(boardText, 'board-write-snapshot'), + '', + 'WHAT THE PIPELINE DECIDED:', + ...items.map((it) => { + const target = it.existingTaskId ?? it.parentTaskId; + return ` [${it.item}] ${it.category}: ${screenedPrimary(it.title, `board-write-item-${it.item}`)}` + + `${target ? ` → ${target}` : ''}${it.assignee ? ` · ${it.assignee}` : ''}${it.list ? ` · ${it.list}` : ''}`; + }), + '', + 'Write each one. Applying the plan as given is the right answer for almost all of them.', + '', + 'You may depart from it where looking at the board tells you something the pipeline could not:', + 'if a card already covers an item, comment on that card instead of creating a duplicate; if two', + 'items are the same work, write one. Read a card before you claim it covers something — an', + 'update whose history you have not opened will be refused.', + '', + 'Anything you write that the pipeline did not plan is re-checked by the same gates it passed.', + 'A refusal comes back with the reason. Respond to it — do not retry the same write.', + '', + 'Stop when every item is written or accounted for. Do not summarise; the summary is generated', + 'from what the board actually did, not from what you say here.', + ].join('\n'); +} + +/** Fold the operations the tracker actually performed back into per-action results. */ +function tally(plan: ReturnType, performed: Array<{ op: TrackerOperation; outcome: OpOutcome }>): ExecuteResult { + const counts = { created: 0, commented: 0, skipped: 0, refused: 0, failed: 0, unsupported: 0 }; + const left = [...performed]; + + const actions: ExecutedAction[] = plan.map((action) => { + // An op belongs to the action that planned it. Agent-originated ops match nothing here and are + // gathered into their own action below rather than being silently attributed to a planned item. + const results = action.ops.flatMap((planned) => { + const i = left.findIndex((p) => p.op === planned || JSON.stringify(p.op) === JSON.stringify(planned)); + return i === -1 ? [] : left.splice(i, 1); + }); + return { ...action, results, ok: results.length > 0 && results.every((r) => r.outcome.status === 'applied' || r.outcome.status === 'unchanged') }; + }); + + if (left.length) { + actions.push({ + item: -1, + category: 'UNKNOWN', + title: 'agent-originated writes', + ops: left.map((p) => p.op), + outcome: 'planned', + results: left, + ok: left.every((r) => r.outcome.status === 'applied' || r.outcome.status === 'unchanged'), + }); + } + + for (const { op, outcome } of performed) { + if (outcome.status === 'applied') { + if (op.kind === 'createTask') counts.created++; + else if (op.kind === 'addComment') counts.commented++; + } else if (outcome.status === 'refused') counts.refused++; + else if (outcome.status === 'unsupported') counts.unsupported++; + else if (outcome.status === 'failed') counts.failed++; + } + + // A planned action whose ops never reached the tracker is not a success. The agent chose not to + // write it, and that is exactly the kind of quiet omission Pass 2d exists to catch — so it is + // counted as skipped and left visible rather than folded into a total. + counts.skipped += plan.filter((a) => a.outcome === 'skipped_duplicate').length; + + return { actions, ...counts }; +} + +/** Ordering helper for traces: worst outcome first, so a refusal is never buried under successes. */ +export const worstOutcomeFirst = (a: OpOutcome['status'], b: OpOutcome['status']): number => + OUTCOME_ORDER.indexOf(a) - OUTCOME_ORDER.indexOf(b); diff --git a/src/cli/demo.ts b/src/cli/demo.ts index aacfea7..fd13040 100644 --- a/src/cli/demo.ts +++ b/src/cli/demo.ts @@ -15,7 +15,7 @@ */ import { existsSync, rmSync } from 'node:fs'; import { join } from 'node:path'; -import { AGENTS_ENABLED, CASSETTE_DIR, CASSETTE_DIR_AGENTS, CASSETTE_DIR_AGENTS_ANTHROPIC, CASSETTE_DIR_ANTHROPIC } from '../config'; +import { AGENTS_ENABLED, BOARD_AGENT_WRITES, CASSETTE_DIR, CASSETTE_DIR_AGENTS, CASSETTE_DIR_AGENTS_ANTHROPIC, CASSETTE_DIR_ANTHROPIC } from '../config'; import { listScenarios, loadScenario } from '../fixtures'; import { cassetteClient } from '../providers/cassette'; import { runScenario } from './runScenario'; @@ -28,6 +28,7 @@ async function main(): Promise { // you get an agent run replaying a non-agent recording, which fails as a missing cassette // rather than as the configuration mistake it actually is. const agents = args.includes('--agents') || AGENTS_ENABLED; + const boardWrites = args.includes('--board-writes') || BOARD_AGENT_WRITES; const providerIdx = args.indexOf('--provider'); const provider = providerIdx !== -1 ? args[providerIdx + 1] : 'deepseek'; const only = args.filter((a) => !a.startsWith('--')).find((a) => a !== provider); @@ -45,12 +46,22 @@ async function main(): Promise { if (agents) { console.log( - '\nAgent layer ON (PRD §5). A board agent delegates to role agents, which have READ-ONLY tools —\n' + - 'Pass 2c is still the only writer. This is the one part of the repo built rather than extracted;\n' + - 'see AGENTS.md and LIMITATIONS.md.' + boardWrites + ? '\nAgent layer ON, and the BOARD AGENT IS THE WRITER (BOARD_AGENT_WRITES) — this is PRD §5\'s\n' + + '"authority to write" and the shape production runs. Role agents are still read-only; every\n' + + 'write the board agent originates is re-run through the same deterministic gates first, so a\n' + + 'write the gates refuse becomes a hold. See AGENTS.md.' + : '\nAgent layer ON (PRD §5). A board agent delegates to role agents, which have READ-ONLY tools —\n' + + 'Pass 2c is still the only writer. This is the one part of the repo built rather than extracted;\n' + + 'see AGENTS.md and LIMITATIONS.md.' ); } + if (boardWrites && !agents) { + console.error('--board-writes needs the agent layer: add --agents (the board agent is the writer).'); + process.exit(1); + } + if (comparing) { console.log( '\nReplaying the Claude recording. The goldens describe the DeepSeek run, so differences below\n' + @@ -91,7 +102,7 @@ async function main(): Promise { console.log(`\n▶ ${name} — ${scenario.expected.description}`); const model = cassetteClient(join(cassettes, name)); - const run = await runScenario(scenario, { model, idempotencyPath: statePath, agents }); + const run = await runScenario(scenario, { model, idempotencyPath: statePath, agents, boardWrites }); if (run.mismatches.length && (comparing || agents)) { // Not counted as a failure, for the same reason in both cases: the goldens describe ONE @@ -130,7 +141,7 @@ async function main(): Promise { if (twice) { // `agents` is threaded through deliberately: without it the second pass ran the non-agent // path, so `--twice --agents` silently proved nothing about the agent path's idempotency. - const second = await runScenario(scenario, { model, idempotencyPath: statePath, quiet: true, agents }); + const second = await runScenario(scenario, { model, idempotencyPath: statePath, quiet: true, agents, boardWrites }); const ok = second.result.status === 'skipped' && second.modelCalls === 0; if (!ok) failures++; // The layer is READ from the run, never hardcoded. It used to print 'source' unconditionally diff --git a/src/cli/runScenario.ts b/src/cli/runScenario.ts index eed771b..f6767fa 100644 --- a/src/cli/runScenario.ts +++ b/src/cli/runScenario.ts @@ -14,16 +14,16 @@ import { pendingHumanStore } from '../state/pendingHuman'; import { type Scenario, diffExpected } from '../fixtures'; import { traceEvents, traceModelClient } from '../observability/otel'; import { PipelineEvents, type PipelineEvent } from '../pipeline/events'; -import { setTaskUrlBuilder } from '../pipeline/gates/clarify'; +import { indexTasks, setTaskUrlBuilder } from '../pipeline/gates/clarify'; import { type PipelineResult, runPipeline } from '../pipeline/run'; import type { ModelClient } from '../providers'; import { setOpsRegistryPath } from '../registry/opsRegistry'; import { setCorrectionsPath } from '../state/corrections'; import { fileRoleStateStore, setRoleStateDir } from '../state/roleState'; -import { delegateToRoleAgents } from '../agents/boardAgent'; -import { AGENT_MAX_DELEGATIONS, AGENTS_ENABLED } from '../config'; +import { delegateToRoleAgents, writeBoard } from '../agents/boardAgent'; +import { AGENT_MAX_DELEGATIONS, AGENTS_ENABLED, BOARD_AGENT_WRITES } from '../config'; import { memoryTracker } from '../trackers/memory'; -import { categoryBreakdown } from '../pipeline/parsing/categorizationManifest'; +import { type CategorizationItem, categoryBreakdown } from '../pipeline/parsing/categorizationManifest'; export type RunScenarioOptions = { model: ModelClient; @@ -46,6 +46,14 @@ export type RunScenarioOptions = { * a developer left in their `.env`. */ agents?: boolean; + /** + * Let the board agent perform the writes (`BOARD_AGENT_WRITES`). Defaults to the env flag, false. + * + * Requires `agents`. On without it is a configuration error rather than a silent no-op, because + * "I turned the write flag on and nothing changed" is the kind of quiet nothing this repo tries + * hard not to ship. + */ + boardWrites?: boolean; }; export type ScenarioRun = { @@ -121,6 +129,15 @@ export async function runScenario(scenario: Scenario, opts: RunScenarioOptions): return r.text; }; + const agentsOn = opts.agents ?? AGENTS_ENABLED; + const boardWrites = opts.boardWrites ?? BOARD_AGENT_WRITES; + if (boardWrites && !agentsOn) { + throw new Error( + 'BOARD_AGENT_WRITES is on but the agent layer is off. The board agent is the writer in that ' + + 'mode, so this combination would silently do nothing — set AGENTS_ENABLED=1 or pass --agents.' + ); + } + const tracker = memoryTracker({ tasks: scenario.board }); const result = await runPipeline(scenario.source, { @@ -130,7 +147,7 @@ export async function runScenario(scenario: Scenario, opts: RunScenarioOptions): ...(opts.pendingHumanPath ? { pendingHuman: pendingHumanStore(opts.pendingHumanPath) } : {}), // Same `model` and `tracker` the pipeline uses, so the agent path is the real thing behind the // same seams rather than a parallel implementation that could drift from it. - ...(opts.agents ?? AGENTS_ENABLED + ...(agentsOn ? { agents: { delegate: (items) => @@ -147,6 +164,23 @@ export async function runScenario(scenario: Scenario, opts: RunScenarioOptions): }, } : {}), + ...(boardWrites + ? { + writeBoard: (items: CategorizationItem[]) => + writeBoard(items, { + model, + tracker, + snapshot: indexTasks(scenario.board), + ...(scenario.source.todayIso ? { planCtx: { todayIso: scenario.source.todayIso } } : {}), + onHold: (h, op) => + emitter.emit({ type: 'alert', detail: `board agent write refused (${op.kind}): ${h.gate}` }), + onEvent: (e) => { + if (e.kind === 'tool') emitter.emit({ type: 'agent:tool', name: e.name, args: e.args }); + else emitter.emit({ type: 'alert', detail: `board agent hit its ${e.iterations}-turn cap` }); + }, + }), + } + : {}), events: emitter, runPass: async ({ prompt, label }) => ({ text: await complete(passKey(label), prompt) }), runCategorization: (prompt, label, system) => complete(`2a/${itemKey(label)}`, prompt, system), diff --git a/src/config.ts b/src/config.ts index 42a0e27..bf8959e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -153,13 +153,33 @@ export const TOOL_LOOP_MAX_ITERATIONS = int('TOOL_LOOP_MAX_ITERATIONS', 6); * front of every reader. * * On, it costs one model call per delegated item and reads card history the deterministic path never - * fetches. It cannot write: role agents get `readOnlyTracker`, and Pass 2c remains the only writer. + * fetches. Role agents still cannot write: they get `readOnlyTracker`, and with `BOARD_AGENT_WRITES` + * off — the default — Pass 2c remains the only writer. */ export const AGENTS_ENABLED = bool('AGENTS_ENABLED', false); /** Items handed to a role agent in one run. Each is a model call, so a bad batch cannot run away. */ export const AGENT_MAX_DELEGATIONS = int('AGENT_MAX_DELEGATIONS', 8); +/** + * Let the **board agent** perform the writes, instead of Pass 2c. + * + * **Off by default.** Off, this repo's headline property holds literally: no model is in the write + * path at all, because no write tool exists for one to reach. On, the board agent is handed the + * already-gated plan and writes it through `governedTracker`, which is the shape PRD §5 describes + * ("authority to write") and the shape production actually runs — its board agent calls a write + * command, and a guard layer decides whether that command lands. + * + * **What stays true with it on.** Every write the agent originates is rebuilt into a + * `CategorizationItem` and re-run through the same deterministic gates the pipeline's own answer + * faced. A write those gates refuse becomes a hold, exactly as it would have upstream. So the + * guarantee narrows honestly — from *an injection cannot author a write* to *an injection cannot + * author a write the gates would not already have approved* — rather than disappearing. + * + * Requires `AGENTS_ENABLED`; on without it is a configuration error, not a silent no-op. + */ +export const BOARD_AGENT_WRITES = bool('BOARD_AGENT_WRITES', false); + // ── Dispute arbiter (optional) ─────────────────────────────────────────────── /** * Resolve a Pass 2a-vs-blind-read write-level dispute against live tracker state instead of holding diff --git a/src/pipeline/gates/governedTracker.test.ts b/src/pipeline/gates/governedTracker.test.ts new file mode 100644 index 0000000..8e970ba --- /dev/null +++ b/src/pipeline/gates/governedTracker.test.ts @@ -0,0 +1,246 @@ +/** + * The governed write boundary — the wrapper that decides whether a board-agent write lands. + * + * This file exists to pin one sentence, because it is the sentence that replaces a stronger one: + * + * > A successful injection cannot author a write **the deterministic gates would not already have + * > approved.** + * + * With `BOARD_AGENT_WRITES` off, the repo's claim is the stronger "cannot author a write", and + * `toolLoop.contract.test.ts` pins that. These tests cover the mode where a model genuinely can reach + * the tracker, so every one of them is a test that the *gates* stopped something, not that the + * absence of a code path did. + */ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { approvedOpSet, governedTracker, opSignature } from './governedTracker'; +import { indexTasks } from './clarify'; +import { type OpsRegistry, setOpsRegistryPath } from '../../registry/opsRegistry'; +import type { BoardTask, TrackerAdapter, TrackerOperation } from '../../trackers'; +import { memoryTracker } from '../../trackers/memory'; + +let dir: string; + +const REGISTRY: OpsRegistry = { + version: 1, + updatedAt: '2026-01-01T00:00:00.000Z', + members: [ + { name: 'Avery Chen', externalIds: {}, email: 'avery@example.com', role: 'engineer', defaultProjects: ['backend'] }, + ], + routes: [ + { + key: 'backend', + externalIds: {}, + pattern: 'api|backend|rate', + defaultAssignee: 'Avery Chen', + validAssignees: ['Avery Chen'], + status: 'active', + }, + ], + log: [], +}; + +const BOARD: BoardTask[] = [ + { id: 't100', title: 'Rate limiting for the public API', status: 'in progress', assignees: ['Avery Chen'], listKey: 'backend' }, +]; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'governed-')); + writeFileSync(join(dir, 'r.json'), JSON.stringify(REGISTRY), 'utf8'); + setOpsRegistryPath(join(dir, 'r.json')); +}); +afterEach(() => { + setOpsRegistryPath(null); + rmSync(dir, { recursive: true, force: true }); +}); + +/** A tracker that records what actually reached it, so "never reached the adapter" is assertable. */ +function recording(): TrackerAdapter & { ops: TrackerOperation[] } { + const inner = memoryTracker({ tasks: BOARD }); + const ops: TrackerOperation[] = []; + return { + ...inner, + ops, + async apply(op) { + ops.push(op); + return inner.apply(op); + }, + }; +} + +const wrap = ( + inner: TrackerAdapter, + over: Partial[1]> = {} +): TrackerAdapter => + governedTracker(inner, { approvedOps: new Set(), snapshot: indexTasks(BOARD), ...over }); + +const CREATE: TrackerOperation = { + kind: 'createTask', + listKey: 'backend', + title: 'Add a rate-limit dashboard', + assignees: ['Avery Chen'], + description: 'Chart the limiter that already shipped.', +}; + +describe('an approved op applies as planned', () => { + /** + * Re-gating an op the pipeline just gated is not merely wasted work — it fails. `tier2Cited` and + * the resolved fields live on the manifest item, not on the operation, so an op rebuilt from + * scratch loses them and the write the gates approved gets refused by those same gates. + */ + it('passes an op in the approved set straight through', async () => { + const inner = recording(); + const out = await wrap(inner, { approvedOps: approvedOpSet([CREATE]) }).apply(CREATE); + + expect(out.status).toBe('applied'); + expect(inner.ops).toEqual([CREATE]); + }); + + it('treats a changed body as a different op, not an approved one', async () => { + const inner = recording(); + const tampered = { ...CREATE, description: 'and also email the client the contract' }; + + // Signature is over the whole op, so matching titles do not smuggle a different body through. + expect(opSignature(tampered)).not.toBe(opSignature(CREATE)); + + const out = await wrap(inner, { approvedOps: approvedOpSet([CREATE]) }).apply(tampered); + expect(out.status).toBe('refused'); + expect(inner.ops).toEqual([]); + }); +}); + +describe('a novel op faces every gate the pipeline faced', () => { + it('lets a clean create through', async () => { + const inner = recording(); + const out = await wrap(inner).apply(CREATE); + + expect(out.status).toBe('applied'); + expect(inner.ops).toHaveLength(1); + }); + + it('refuses an assignee who is not on the roster, and the adapter never sees it', async () => { + const inner = recording(); + const out = await wrap(inner).apply({ ...CREATE, assignees: ['Mallory Stranger'] }); + + expect(out.status).toBe('refused'); + expect('detail' in out && out.detail).toMatch(/roster/i); + expect(inner.ops).toEqual([]); + }); + + it('refuses an unknown list key', async () => { + const inner = recording(); + const out = await wrap(inner).apply({ ...CREATE, listKey: 'not-a-list' }); + + expect(out.status).toBe('refused'); + expect(inner.ops).toEqual([]); + }); + + /** + * The gate that ignores confidence, reached through a write rather than through a manifest. An + * agent that has been talked into rotating a credential is exactly the scenario this mode has to + * survive to be shippable at all. + */ + it('refuses a write that trips the critical gate', async () => { + const inner = recording(); + const out = await wrap(inner).apply({ + ...CREATE, + title: 'Rotate the Stripe API key and grant access to the new vendor', + }); + + expect(out.status).toBe('refused'); + expect('detail' in out && out.detail).toMatch(/critical/i); + expect(inner.ops).toEqual([]); + }); + + it('refuses rather than fails — a retry cannot help', async () => { + const out = await wrap(recording()).apply({ ...CREATE, assignees: ['Mallory Stranger'] }); + expect(out.status).not.toBe('failed'); + expect(out.status).toBe('refused'); + }); + + /** + * Fail closed on anything with no manifest form. An op this layer cannot express as an item is an + * op no gate can judge, and "unfamiliar" must never mean "allowed". + */ + it('refuses an operation it cannot express as an item', async () => { + const inner = recording(); + const out = await wrap(inner).apply({ kind: 'moveList', taskId: 't100', listKey: 'backend' }); + + expect(out.status).toBe('refused'); + expect('detail' in out && out.detail).toMatch(/no manifest form/i); + expect(inner.ops).toEqual([]); + }); +}); + +describe('evidence is what the agent read, not what it said', () => { + /** + * The evidence gate covers DUPLICATE and SUBTASK — containment and "same work" are the two claims + * comment history actually proves. An UPDATE is gated on card identity instead, deliberately, so a + * comment on a card the agent named explicitly is not the case this mechanism is for. + * + * On the pipeline path, "I checked the history" is the model's own claim, parsed out of its prose. + * Here it is a fact the loop recorded: a model that says it looked and did not gets held. + */ + const SUBTASK: TrackerOperation = { + kind: 'createTask', + listKey: 'backend', + title: 'Chart p99 latency under the limiter', + assignees: ['Avery Chen'], + description: 'Add a p99 panel to the limiter dashboard.', + parentId: 't100', + }; + + it('holds a subtask whose parent history was never fetched', async () => { + const inner = recording(); + const out = await wrap(inner, { readComments: () => false }).apply(SUBTASK); + + expect(out.status).toBe('refused'); + expect('detail' in out && out.detail).toMatch(/evidence not cited/i); + expect(inner.ops).toEqual([]); + }); + + it('lets the same subtask through once the parent history has actually been read', async () => { + const inner = recording(); + const out = await wrap(inner, { readComments: (id) => id === 't100' }).apply(SUBTASK); + + expect(out.status).toBe('applied'); + expect(inner.ops).toHaveLength(1); + }); + + /** An UPDATE naming a real card is settled by identity, and needs no history to be written. */ + it('does not ask a comment on a named, existing card for history it does not need', async () => { + const inner = recording(); + const out = await wrap(inner, { readComments: () => false }).apply({ + kind: 'addComment', + taskId: 't100', + body: 'Shipped the limiter.', + }); + + expect(out.status).toBe('applied'); + }); +}); + +describe('reads are untouched', () => { + it('passes every read through and names itself in the trace', async () => { + const g = wrap(recording()); + + expect(g.name).toMatch(/:governed$/); + expect(await g.getTask('t100')).not.toBeNull(); + expect(await g.listTasks()).toHaveLength(1); + }); +}); + +describe('a refused write is surfaced, not swallowed', () => { + it('reports the hold to the caller so it reaches the same place any other hold does', async () => { + const seen: string[] = []; + await wrap(recording(), { onHold: (h) => seen.push(h.gate) }).apply({ + ...CREATE, + assignees: ['Mallory Stranger'], + }); + + expect(seen).toHaveLength(1); + expect(seen[0]).toMatch(/roster/i); + }); +}); diff --git a/src/pipeline/gates/governedTracker.ts b/src/pipeline/gates/governedTracker.ts new file mode 100644 index 0000000..ad8241e --- /dev/null +++ b/src/pipeline/gates/governedTracker.ts @@ -0,0 +1,203 @@ +/** + * A governed write boundary — the adapter wrapper that lets the **board agent** write. + * + * This is the second of two adapter wrappers, and the pair is the whole write-authority design: + * + * `readOnlyTracker` refuses every write. What role agents get, always. + * `governedTracker` allows a write **that the deterministic gates accept**. What the board agent + * gets, and only when `BOARD_AGENT_WRITES` is on. + * + * **Why a wrapper and not prompt text.** Same reason as `readOnlyTracker`: a prompt is a request and + * a wrapper is a guarantee. A model that has been jailbroken, confused, or fed a malicious transcript + * cannot argue its way past this, because the argument never reaches the tracker — `applyGates` does. + * + * ## What this does and does not guarantee + * + * With `BOARD_AGENT_WRITES` off, this file is not in the process and the repo's claim is literal: no + * model is in the write path, because no write tool exists for one to reach. + * + * With it on, the claim narrows, and it is worth stating the narrowed version exactly rather than + * restating the old one: + * + * > A successful injection cannot author a write **the deterministic gates would not already have + * > approved.** + * + * That is smaller than "cannot author a write". It is not nothing: every gate the pipeline's own + * answer faced — routing, roster, evidence, duplicate, critical — runs again here, over an item + * rebuilt from the operation the model actually asked for rather than from anything it claimed. + * + * ## Two paths through `apply` + * + * **An op that matches the approved plan exactly applies as planned.** It was gated moments ago; + * re-gating it would be theatre, and worse, would fail — `tier2Cited` and the resolved fields live on + * the manifest item, not on the operation, so a naive rebuild loses them and the write the pipeline + * approved gets refused by its own gates. Matching is on the whole serialised op, so a title that + * matches with a body that does not is a novel op, not an approved one. + * + * **Anything else is rebuilt into a `CategorizationItem` and re-gated.** That covers the case this + * mode exists for: the agent looked at the board, decided the plan was wrong, and wants to comment on + * an existing card instead of creating a second one. + * + * ## Evidence, and why this is stricter than the pipeline + * + * The evidence gate holds a DUPLICATE or SUBTASK whose rationale does not cite comment history — + * "same work" and containment are the two claims that history actually proves. (An UPDATE is gated on + * card *identity* instead, deliberately; see the note in `contractGates.ts`.) + * + * On the pipeline path that citation is the model's own claim, checked by parsing its prose. Here it + * is a fact: `readComments` reports whether this agent actually called `get_task_comments` on that + * card during this run. A model that says it checked but did not gets held, which makes this strictly + * stricter than the pipeline's own version of the same gate. + */ +import type { BoardTask, OpOutcome, TrackerAdapter, TrackerOperation } from '../../trackers'; +import type { CategorizationItem } from '../parsing/categorizationManifest'; +import { type DeterministicGateOptions, applyGates } from '../passes/contractCheck'; +import type { HeldItem } from './contractGates'; + +export interface GovernedTrackerContext { + /** + * The operations the pipeline already gated and approved, serialised. An op in this set applies + * without re-gating; see the header for why re-gating an approved op is both pointless and wrong. + */ + approvedOps: ReadonlySet; + /** The board as the run read it, for the gates that compare against existing cards. */ + snapshot: Map; + gateOpts?: DeterministicGateOptions; + /** Did the agent actually fetch this card's comments this run? Becomes `tier2Cited`. */ + readComments?: (taskId: string) => boolean; + /** Called for every write the gates refuse, so it reaches the same place any other hold does. */ + onHold?: (held: HeldItem, op: TrackerOperation) => void; +} + +/** + * Serialise an operation to a stable string. + * + * Key-sorted rather than `JSON.stringify(op)` directly: object key order is insertion order in V8, so + * the same logical op built by two different code paths would otherwise produce two different + * signatures and an approved write would be treated as novel. + */ +export function opSignature(op: TrackerOperation): string { + return JSON.stringify(op, Object.keys(op).sort()); +} + +/** Build the approved-op set from a plan's operations. */ +export function approvedOpSet(ops: readonly TrackerOperation[]): Set { + return new Set(ops.map(opSignature)); +} + +/** + * Rebuild the manifest item an operation implies, so the gates can judge it. + * + * Returns `null` for an operation shape this layer cannot express as an item. That is a refusal, not + * a pass — an op nothing can gate must not reach the tracker just because it is unfamiliar. + */ +function itemForOp( + op: TrackerOperation, + n: number, + ctx: GovernedTrackerContext +): CategorizationItem | null { + const cited = (taskId: string): boolean => ctx.readComments?.(taskId) ?? false; + const base = { item: n, confidence: 'med' as const, raw: `agent-originated ${op.kind}` }; + + switch (op.kind) { + case 'createTask': + return { + ...base, + title: op.title, + category: 'NEW_TASK', + ...(op.listKey ? { list: op.listKey } : {}), + ...(op.assignees[0] ? { assignee: op.assignees[0] } : {}), + ...(op.description ? { finalDesc: op.description } : {}), + ...(op.dueDate ? { dueDate: op.dueDate } : {}), + ...(op.status ? { status: op.status } : {}), + ...(op.parentId ? { category: 'SUBTASK' as const, parentTaskId: op.parentId } : {}), + // A plain create cites nothing by construction — there is no prior card to have read. A + // subtask create does have one, and the evidence gate asks for exactly that card's history, + // so it cites the parent it actually opened. Hardcoding `false` here made every agent- + // originated subtask unwritable, which would have read as "the gate is strict" rather than + // as the bug it was. + tier2Cited: op.parentId ? cited(op.parentId) : false, + }; + + case 'addComment': + return { + ...base, + title: ctx.snapshot.get(op.taskId)?.title ?? op.taskId, + category: 'UPDATE', + existingTaskId: op.taskId, + finalDesc: op.body, + tier2Cited: cited(op.taskId), + }; + + case 'setStatus': + return { + ...base, + title: ctx.snapshot.get(op.taskId)?.title ?? op.taskId, + category: 'UPDATE', + existingTaskId: op.taskId, + status: op.status, + tier2Cited: cited(op.taskId), + }; + + case 'setAssignees': + return { + ...base, + title: ctx.snapshot.get(op.taskId)?.title ?? op.taskId, + category: 'UPDATE', + existingTaskId: op.taskId, + ...(op.assignees[0] ? { assignee: op.assignees[0] } : {}), + tier2Cited: cited(op.taskId), + }; + + default: + return null; + } +} + +/** + * Wrap an adapter so it accepts a write only when the deterministic gates do. + * + * `refused` rather than `failed`, matching `readOnlyTracker` and everything else in this repo: the + * operation is well-formed and the tracker could perform it. This layer declined, and a retry will + * decline identically. + */ +export function governedTracker(inner: TrackerAdapter, ctx: GovernedTrackerContext): TrackerAdapter { + // Agent-originated items are numbered below zero so a held one is instantly distinguishable from a + // held inventory item in a trace, and can never collide with a real inventory line number. + let originated = 0; + + return { + name: `${inner.name}:governed`, + capabilities: inner.capabilities, + getTask: (id) => inner.getTask(id), + getComments: (id, limit) => inner.getComments(id, limit), + listTasks: (opts) => inner.listTasks(opts), + renderSnapshot: (tasks) => inner.renderSnapshot(tasks), + + async apply(op): Promise { + if (ctx.approvedOps.has(opSignature(op))) return inner.apply(op); + + const item = itemForOp(op, --originated, ctx); + if (!item) { + return { + status: 'refused', + detail: + `governed: "${op.kind}" has no manifest form, so no gate can judge it — refusing rather ` + + `than writing something unchecked`, + }; + } + + const { held } = applyGates([item], ctx.snapshot, ctx.gateOpts ?? {}); + const stop = held[0]; + if (stop) { + ctx.onHold?.(stop, op); + return { + status: 'refused', + detail: `governed: ${stop.gate} — ${stop.question}`, + }; + } + + return inner.apply(op); + }, + }; +} diff --git a/src/pipeline/run.ts b/src/pipeline/run.ts index 49102b9..a635333 100644 --- a/src/pipeline/run.ts +++ b/src/pipeline/run.ts @@ -69,6 +69,15 @@ export type PipelineDeps = { * refuse becomes a human hold rather than a write. */ agents?: { delegate(items: CategorizationItem[]): Promise }; + /** + * The board agent as writer (`BOARD_AGENT_WRITES`). Omit and Pass 2c writes, which is the default + * and the configuration every claim about "no model in the write path" describes. + * + * Supplied, it replaces Pass 2c and must return the same `ExecuteResult`, because everything after + * the write — the `executed` event, the audit, role memory — is shared and should not know which + * writer ran. What it may write is bounded by `governedTracker`, not by this seam. + */ + writeBoard?: (items: CategorizationItem[]) => Promise; /** * The retrieval seam (PRD §8) — an external knowledge layer feeding extra context to passes 2a/2b. * @@ -352,9 +361,14 @@ export async function runPipeline(source: IngestedSource, deps: PipelineDeps): P if (regated.flags.length) emit({ type: 'flags', flags: regated.flags }); } - // ── Pass 2c — the only writer ──────────────────────────────────────────── - const exec = await timed('2c-execute', () => - executeOperations(planOperations(writable, { ...(source.todayIso ? { todayIso: source.todayIso } : {}) }), deps.tracker) + // ── The write ──────────────────────────────────────────────────────────── + // Pass 2c by default — deterministic, no model. `deps.writeBoard` swaps in the board agent, which + // is PRD §5's "authority to write" and what production runs; either way the result shape is the + // same, so nothing downstream branches on which one ran. + const exec = await timed(deps.writeBoard ? '2c-execute (board agent)' : '2c-execute', () => + deps.writeBoard + ? deps.writeBoard(writable) + : executeOperations(planOperations(writable, { ...(source.todayIso ? { todayIso: source.todayIso } : {}) }), deps.tracker) ); emit({ type: 'executed', ...exec }); diff --git a/src/pipeline/toolLoop.ts b/src/pipeline/toolLoop.ts index b196959..289503b 100644 --- a/src/pipeline/toolLoop.ts +++ b/src/pipeline/toolLoop.ts @@ -84,10 +84,83 @@ export const READ_ONLY_TOOLS: ToolSpec[] = [ }, ]; +/** + * The write half, offered **only** to the board agent and **only** when `BOARD_AGENT_WRITES` is on. + * + * These reach a `governedTracker`, never a raw adapter: every write is rebuilt into a manifest item + * and re-run through the deterministic gates before it lands. See `gates/governedTracker.ts` for the + * exact guarantee that buys, which is narrower than the read-only path's and stated as such. + * + * Deliberately four, not the adapter's full surface. `linkTasks` and `moveList` have no manifest form + * this layer can gate, so offering them would mean either an ungated write or a tool that always + * refuses — and a tool that always refuses is worse than no tool, because the model spends turns + * discovering it. + */ +export const WRITE_TOOLS: ToolSpec[] = [ + { + name: 'create_task', + description: + 'Create a new board card. Only for work that is genuinely not on the board yet — check first.', + parameters: { + type: 'object', + properties: { + title: { type: 'string' }, + list_key: { type: 'string', description: 'Which list the card belongs on.' }, + assignee: { type: 'string', description: 'Canonical name of the owner.' }, + description: { type: 'string' }, + parent_id: { type: 'string', description: 'Set to make this a subtask of an existing card.' }, + }, + required: ['title', 'list_key', 'assignee'], + }, + }, + { + name: 'add_comment', + description: + 'Comment on an existing card. The right call when the work is already tracked and this is news about it.', + parameters: { + type: 'object', + properties: { task_id: { type: 'string' }, body: { type: 'string' } }, + required: ['task_id', 'body'], + }, + }, + { + name: 'set_status', + description: "Move an existing card to a different status.", + parameters: { + type: 'object', + properties: { task_id: { type: 'string' }, status: { type: 'string' } }, + required: ['task_id', 'status'], + }, + }, + { + name: 'set_assignees', + description: 'Replace the owners of an existing card. Replaces, never appends.', + parameters: { + type: 'object', + properties: { + task_id: { type: 'string' }, + assignees: { type: 'array', items: { type: 'string' } }, + }, + required: ['task_id', 'assignees'], + }, + }, +]; + export interface ToolLoopOptions { model: ModelClient; - /** Wrapped in `readOnlyTracker` internally — passing a writable adapter is safe. */ + /** + * Wrapped in `readOnlyTracker` internally **unless** `tools` is given — passing a writable adapter + * to the default loop is safe. A caller that supplies its own tool list is responsible for the + * wrapper too, which in practice means `governedTracker`. + */ tracker: TrackerAdapter; + /** + * Defaults to `READ_ONLY_TOOLS`, and that default is load-bearing: the tool list is part of the + * prompt fingerprint, so every existing caller keeps replaying its recorded cassettes byte for byte. + */ + tools?: ToolSpec[]; + /** Skip the read-only wrapper. Only meaningful with `tools`; the caller has already governed writes. */ + writable?: boolean; maxIterations?: number; onEvent?: (e: { kind: 'tool'; name: string; args: Record } | { kind: 'cap-hit'; iterations: number }) => void; } @@ -97,7 +170,8 @@ export interface ToolLoopOptions { * so nothing downstream knows or cares whether tools were used. */ export function makeToolLoopRunner(opts: ToolLoopOptions): (prompt: string, label: string) => Promise { - const tracker = readOnlyTracker(opts.tracker); + const tracker = opts.writable ? opts.tracker : readOnlyTracker(opts.tracker); + const tools = opts.tools ?? READ_ONLY_TOOLS; const maxIterations = opts.maxIterations ?? TOOL_LOOP_MAX_ITERATIONS; return async function run(prompt: string, label: string): Promise { @@ -112,7 +186,7 @@ export function makeToolLoopRunner(opts: ToolLoopOptions): (prompt: string, labe key: `${label}/turn-${i + 1}`, messages, determinism: 'strict', - tools: READ_ONLY_TOOLS, + tools, }); if (!res.toolCalls?.length) return res.text; @@ -124,7 +198,7 @@ export function makeToolLoopRunner(opts: ToolLoopOptions): (prompt: string, labe messages.push({ role: 'tool', toolCallId: call.id, - content: await dispatch(tracker, call.name, call.arguments), + content: await dispatch(tracker, call.name, call.arguments, tools), }); } } @@ -143,10 +217,34 @@ export function makeToolLoopRunner(opts: ToolLoopOptions): (prompt: string, labe }; } +/** + * Turn a write outcome into the sentence the model reads next. + * + * Every status is reported, including the ones that are not success. A model told nothing about a + * refusal will retry the same rejected write until the turn cap; a model told *why* can comment + * instead of creating, or stop. `refused` is deliberately worded as a decision rather than an error, + * because that is what it is — the gate did its job. + */ +function renderOutcome(tool: string, outcome: OpOutcome): string { + switch (outcome.status) { + case 'applied': + return `${tool}: applied${outcome.resultId ? ` (id ${outcome.resultId})` : ''}`; + case 'unchanged': + return `${tool}: already in that state — nothing to do`; + case 'refused': + return `${tool}: REFUSED — ${'detail' in outcome ? outcome.detail : 'a guard declined it'}`; + case 'unsupported': + return `${tool}: this tracker cannot express that operation`; + default: + return `${tool}: failed — ${'detail' in outcome ? outcome.detail : 'unknown error'}`; + } +} + async function dispatch( tracker: TrackerAdapter, name: string, - args: Record + args: Record, + available: ToolSpec[] = READ_ONLY_TOOLS ): Promise { try { switch (name) { @@ -198,10 +296,60 @@ async function dispatch( : `no open task title contains "${q}"`; } + // ── Writes. Present only when the caller offered WRITE_TOOLS, and reaching a governed + // adapter in that case — `apply` here is `governedTracker.apply`, not a raw one. + // + // Every outcome is reported back verbatim, refusals included. A refused write must read to the + // model as a refusal it can respond to, not as a silence it retries: the gate's question is the + // most useful thing it could be told, and hiding it would produce a loop that writes the same + // rejected card until the turn cap. + case 'create_task': { + const assignee = String(args.assignee ?? '').trim(); + const outcome = await tracker.apply({ + kind: 'createTask', + listKey: String(args.list_key ?? ''), + title: String(args.title ?? ''), + assignees: assignee ? [assignee] : [], + ...(args.description ? { description: String(args.description) } : {}), + ...(args.parent_id ? { parentId: String(args.parent_id) } : {}), + }); + return renderOutcome('create_task', outcome); + } + + case 'add_comment': + return renderOutcome( + 'add_comment', + await tracker.apply({ + kind: 'addComment', + taskId: String(args.task_id ?? ''), + body: String(args.body ?? ''), + }) + ); + + case 'set_status': + return renderOutcome( + 'set_status', + await tracker.apply({ + kind: 'setStatus', + taskId: String(args.task_id ?? ''), + status: String(args.status ?? ''), + }) + ); + + case 'set_assignees': + return renderOutcome( + 'set_assignees', + await tracker.apply({ + kind: 'setAssignees', + taskId: String(args.task_id ?? ''), + assignees: Array.isArray(args.assignees) ? args.assignees.map(String) : [], + }) + ); + default: // Naming what IS available turns a hallucinated tool into a corrected next turn rather than // a dead end the model tries to work around. - return `no tool named "${name}". Available: ${READ_ONLY_TOOLS.map((t) => t.name).join(', ')}`; + return `no tool named "${name}". Available: ${available.map((t) => t.name).join(', ')}`; } } catch (err) { // **The error message is screened too.** It is the one path out of `dispatch` that skipped