From 23348926d4c14166cf07431ade3ca9dc36faa140 Mon Sep 17 00:00:00 2001 From: harishghasolia07 <100846446+harishghasolia07@users.noreply.github.com> Date: Wed, 26 Aug 2026 05:09:04 +0000 Subject: [PATCH] Give the board agent real write authority, behind a flag, governed by the gates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PRD §5 says the board agent holds "authority to write". Production means that literally — its board agent runs a create command and a guard layer decides whether the command lands. This repo shipped a board agent that could not write at all, and AGENTS.md justified that by claiming production's agent only proposed. That claim was wrong, and it was the sentence holding the divergence in place. BOARD_AGENT_WRITES (off by default) is the production shape. On, 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 — a write the gates refuse becomes a hold, not a card. Off, none of that code is in the process and Pass 2c writes as before. Off by default on purpose. Nothing in this layer has governed a real board the way the pipeline has, and 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. The injection guarantee differs per mode and SECURITY.md now states both rather than stating the stronger one twice: "cannot author a write" by default, and "cannot author a write the gates would not already have approved" with the flag on. The second is smaller. Saying so is the point. Evidence is stricter here than on the pipeline path. The gate wants comment history cited; on the pipeline that citation is the model's own claim parsed out of prose, and here it is a fact — readComments reports whether the agent actually called get_task_comments on that card. Two bugs found while building it, both kept fixed: - a subtask create hardcoded tier2Cited: false, so no agent-originated subtask could ever cite the parent it had just read, and every one was unwritable - the write loop swallowed a thrown error, so a missing cassette produced a tidy "0 created" and left Pass 2d to infer the problem from four mismatches. A loop that wrote nothing now rethrows; one that wrote something reports what landed Default paths are byte-identical: 1018 tests pass, and all five demo modes replay with zero cassette drift. The tool list is part of the prompt fingerprint, so that zero is what proves the flag is not leaking into the default path. --- .env.example | 12 +- AGENTS.md | 52 +++-- ARCHITECTURE.md | 9 +- CHANGELOG.md | 10 +- EXTRACTION.md | 6 +- LIMITATIONS.md | 7 +- README.md | 13 +- SECURITY.md | 28 ++- src/agents/agents.test.ts | 108 ++++++++- src/agents/boardAgent.ts | 190 +++++++++++++++- src/cli/demo.ts | 23 +- src/cli/runScenario.ts | 44 +++- src/config.ts | 22 +- src/pipeline/gates/governedTracker.test.ts | 246 +++++++++++++++++++++ src/pipeline/gates/governedTracker.ts | 203 +++++++++++++++++ src/pipeline/run.ts | 20 +- src/pipeline/toolLoop.ts | 160 +++++++++++++- 17 files changed, 1095 insertions(+), 58 deletions(-) create mode 100644 src/pipeline/gates/governedTracker.test.ts create mode 100644 src/pipeline/gates/governedTracker.ts 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