diff --git a/.gitignore b/.gitignore index c9bbffe..f07a2a2 100644 --- a/.gitignore +++ b/.gitignore @@ -232,8 +232,9 @@ __marimo__/ # A policy generated because no `--policy`, no `grapharc.toml` and no existing # one was found. Generated rather than authored: the design is that you *promote* # it — read it, edit it, move it, pass `--policy` — instead of committing it from -# where a tool dropped it. `grapharc.toml` itself is deliberately NOT ignored; -# that one is yours and belongs in version control. +# where a tool dropped it. In a project of your own, the scaffolded +# `grapharc.toml` is authored config and belongs in version control; in *this* +# repo the root one is dogfooding residue, ignored with the rest of it below. .grapharc/ # Durable claim stores from `grapharc demo --memory PATH`, and the two sidecars @@ -247,7 +248,30 @@ __marimo__/ /workspace/ /scratch/ +# Dogfooding residue at the repo root: the `grapharc init` scaffold pair, the +# registries that drive the demo films, and the `app/` directory a bench +# shakeout wrote its success condition into. None of it is project source, and +# none of it — the bench residue especially — should ride along on a push. +# Root-anchored, so `bench/fixture/app/` (a committed part of the bench) is +# untouched. +/registry.py +/grapharc.toml +/demo_registry.py +/sample_incident.py +/app/ + # Claude Code's per-machine settings. Already covered by many people's global # ignore; named here so a fresh clone does not depend on that being true. .claude/settings.local.json HANDOFF.md + +# Generated by `grapharc init --claude-code` when this repo dogfoods its own +# supervision. Generated rather than authored, like the policy above: promote +# a copy into a project you are governing instead of committing the one a +# smoke test dropped here. +/.mcp.json +/.claude/skills/ + +# The demo recording is regenerated by docs/demo/capture_supervised_slack.py; +# it embeds absolute paths from whichever machine made it. +docs/demo/session.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 70e7a7f..10fe7a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ and "used to be true" — the two things a reader most needs kept apart. Entries are newest-last within a release, matching the order they were written. +## Unreleased + +- **the supervised loop had no exit from Slack.** A Slack-launched `plan --approve` parked, showed a human the run, was approved — and then returned `stopped: planned (awaiting `grapharc go`)`, into a subcommand the bot's gate does not carry and cannot be talked into carrying. So the one exchange the bot exists for (ask for work, see the graph, let it run) stopped one step short of running anything, and the approval it collected authorised nothing. `plan --go` is admitted from Slack now, and admitting it is only safe because of the rule that came with it: **from Slack, `--go` forces `--approve`**, on every registry, whether or not the requester typed it. Anyone in a workspace can type into this bot; without that rule one message would take a model's proposal straight to execution on the host with the graph visible only afterwards. It is forced rather than refused so the useful command stays one message — propose it, show me, run it if I say yes. +- **the graph a human was asked to approve was not in the message they were asked to approve it from.** The parked status message said "planned graph is in the live view link" and left the reader to open a URL — on a phone, from a chat client, to see the thing they were being asked about. The proposal is rendered into the message now: the planner's rationale, every node, every edge, and the worst-case token estimate the gate computed. Two things ride with it. First, **kinds, not just names**: `ProposedNode.kind` is what admission governs and the name is a label the planner picked freely, so a node named `fix_it` of kind `apply_change` used to read as harmless — the kind is shown, and a kind the registry declares mutating is marked `✎`. Second, a registry that declares no `MUTATING_KINDS` marks *every* node and says so, matching the fail-closed reading `plan` already takes when it writes `mutating: true` into a plan file it cannot vouch for; the bot resolves the registry from the flag or the working directory's `grapharc.toml` and refuses to import anything outside the shipped set to find out. +- **the app manifest enabled interactivity and nothing used it.** Approving meant typing `/grapharc approve slack-runs/20260808-130940-1b43a32a/trace.jsonl` — a generated path, from a phone. A parked run's message now carries Approve and Deny buttons, and a click writes the same decision file the typed command writes. The click is **bound to the plan's fingerprint**: a parked run rewrites its request every round, so a button on a message scrolled back to names a proposal that is no longer the one waiting, and approving it would be approving a graph nobody read. That is refused at the handler and would be discarded by `file_approval` besides. The button's directory is re-confined inside the working directory exactly as a typed path is — a signed payload says the click is genuine, not that the button was drawn by a version of the bot that meant the same thing by it. The buttons leave on the same edit that reports the run started, because a finished run still showing a live Approve button is a lie a later click would act on. Who clicked is posted into the thread and **not** into the trace, which has no actor field; the bot will not imply an audit trail it does not have. +- **two minutes was the whole budget for work that takes minutes.** One timeout bounded every Slack command, defaulted to 120s so a runaway `metrics` could not hold a worker thread — and a delegated Claude Code phase reads files, runs things and writes a report. That is not a safety limit for such a run, it is a SIGKILL through the middle of one a human just approved. Commands that *execute* (`agent`, `plan --go`) draw on a separate `GRAPHARC_SLACK_WORK_TIMEOUT` (default 1800s); readers keep the short one. The human's share is carved out of whichever applies — a third for a parked `--go`, capped at 15 minutes, so saying yes leaves the run its budget; half for a plan-only park, which has nothing to do afterwards. A requester-supplied `--approval-timeout` larger than the budget is now refused rather than silently outliving the runner: a park that outlives its kill does not report `approval_timeout`, it dies mid-wait. +- **the stdlib registry could not use Claude Code at all** — the one combination its own module docstring describes. Given a backend with no tool-calling wire format, every agent-backed kind delegates its whole loop to Claude Code; delegation needs a directory to run in, and it looked for one on `harness.executor`, where only the sandboxing executors have it. The stdlib harness uses `LocalExecutor`, which has none, so every `investigate`/`verify`/`summarize` phase of every such run failed with *"the delegated executor needs a workspace directory"* before spawning anything. Underneath it was a quieter defect: `Harness(..., executor=…, workspace=…)` **silently discarded `workspace`**, which existed only to construct the default executor — three call sites in this repo pass both and got nothing for it. The harness records its workspace now, whichever executor is in play, delegation asks the harness before the executor, and the stdlib harness names the directory its tools are already confined to. +- a phase the budget curtailed **reported nothing at all**. `AgentResult.output` is empty by contract for every termination reason but `TARGET_MET` — the mid-work text lives in `partial_output` — and the stdlib phase body formatted `output` regardless, so a curtailed phase wrote `[budget_exhausted] ` with nothing after it. The work it did manage was dropped, and a downstream goal check read an empty note as a phase with nothing to report rather than one cut off. It reads `partial_output`, falling back to the reason line. +- a **delegated run was billed at zero**. `grapharc agent --executor claude-cli` drives an `AgentNode` with no enclosing graph, so every event it writes is an orphan, and it reports its spend on the `stop` event; `observe.cost` attributed orphan cost only from `model` events, so the recorded cost came out `$0.00` with `unpriced_tokens` at zero too — nothing said the figure was incomplete. Meanwhile `ReplayedRun.recorded_cost_usd`, which counts orphans by cost rather than by phase, reported the real number: two readers, one trace, two answers. Cost is attributed from any orphan carrying one now; only a `model` event still becomes a row in the model-call breakdown, because only that is a model call. +- the live page **closed the stream while the graph was still running**. "Finished" was any `stop` event in the run — but every `AgentNode` phase writes its own `stop` when its agent loop ends, so a three-phase graph was declared done the moment the first phase finished, and the SSE stream closed on a page with two nodes left to run. Only the *driver's* terminal stop ends a run, and `orphan_sub_events` is how that is read: a phase's stop falls inside its node span and is attributed there, while the driver's is an orphan — and a run with no graph at all (`grapharc agent`) has only orphans, so its stop still ends the stream exactly as before. +- policy generation **substituted another registry's dangerous kinds for a custom registry's silence**. `mutating or stdlib.MUTATING_KINDS` collapsed three states into two: a registry declaring `()` ("none of mine mutate") and a registry declaring nothing at all both became stdlib's `("apply_change",)`. So a registry whose real mutating kind is `deploy` or `publish` had that kind go unnamed while the model was told to guard a name the registry does not contain — and the resulting permissive policy is cached and governs every later run. A declared tuple is now used verbatim, empty included; `None` means nobody said, and every kind in the catalog is named dangerous. +- a delegated run whose deadline had already passed **spawned Claude Code in order to kill it**. `remaining_seconds()` is `max_seconds - elapsed` and goes negative once the budget is spent; `subprocess.run` accepts a negative timeout, starts the child, and kills it on the first wait — so an over-budget run launched the CLI, tore it down mid-startup, and reported `max_seconds (-3.2) reached`, a number no caller set. An exhausted deadline is refused before anything is spawned. +- the supervised agent's own status tool **crashed on the runs it was watching**. `mcp.driver.graph_status` read the trace with `TraceRecorder`, the strict reader, which raises `TraceReadError` on a half-written last line — and a half-written last line is the normal state of a file something is still appending to. An agent polls `show_graph` precisely *while* its run is in progress, so the one moment this had to answer was the one moment it raised, out of the MCP server, as a crash rather than a status. The rest of the codebase already had the answer: `TailRecorder` skips the torn line, which is how the live view and the Slack tailer read a running run. `graph_status` uses it now. Found and fixed on camera — `docs/demo/scenarios/fix_bug.py` is the recording, and the fix was then read and landed here deliberately with `tests/test_torn_trace_read.py`. +- **three CLI recordings, on the same terms as the Slack one.** `docs/demo/capture_cli.py` runs a scenario's commands in a pseudo-terminal — so the CLI takes its tty branch and emits the colour a person actually sees, rather than the byte-stable colourless form a pipe would give — and records each command's bytes, exit code and wall clock. `render_cli.py` draws them, parsing exactly `\x1b[…m` (including the `38;5;N` form the CLI emits under a 256-colour terminal) and dropping every other escape rather than half-interpreting it. The three: the admission gate refusing and then admitting, free and deterministic; GraphARC fixing the `graph_status` bug above in a copy of itself, where the interesting frame is the planner's own rationale under a deny — *"Since apply_change cannot be reached by an edge, this round investigates … for a human to act on"*, `mutating: false` — against the five-node `mutating: true` graph the same goal and model produce once a human amends the rule; and one trace file answering `trace`, `replay` and `diff`, including a run priced and refused during admission so that `diff` reports `path 3 -> 0 nodes`. +- **a demo you can re-make and check.** `docs/demo/capture_supervised_slack.py` drives the real Slack path — the real gate, a real planner, the real file handshake, a real fingerprint-checked click, a real delegated Claude Code phase — against a recording sink instead of a socket, and `render_demo.py` turns the recording into the film in the README. What is real and what is mocked is written down rather than implied, and the closing frame is computed from the trace file rather than from any message: `approval_request → approval_response → start`, with `start` last. The same property is asserted against a real CLI subprocess in `tests/test_slack_supervision.py`, so it does not depend on anyone re-recording a video. + ## 0.1.5 - a documentation and demo release; no runtime code changed between `0.1.4` and this wheel. The demo film was re-cut to open on the graph itself — frame one is the nine-node incident graph with its first node already running, then the question that built it, then the finished audited run — and the README now leads with it. The README and website stopped describing the project as early and unstable: the status line states the version and the testing discipline, and *Status and limits* became *Limits*, framed as edges that are documented and tested rather than confessed. A PyPI downloads badge joined the badge row. This release exists mostly so the PyPI project page, which renders the README frozen at publish time, catches up with all of it. diff --git a/README.md b/README.md index 1dab2d8..01c7404 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,10 @@ grapharc serve --live-root .grapharc/runs # live browser view of every run grapharc replay # reconstruct a run from its trace ``` +![A terminal running grapharc plan: round 1 is rejected with edge_denied and never executes, round 2 replans and runs, then viz draws the graph and metrics prints the per-node bill — all from the one trace file.](docs/media/grapharc-cli-gate.gif) + +*Free and deterministic — `--scripted` runs the registry's own stand-in planner, so this reproduces exactly on any checkout. Three more recordings, and what is and is not staged in each, in [docs/demo/](docs/demo/).* + Building a graph directly: ```python @@ -159,6 +163,31 @@ goal_met Both blocks are executed by `tests/test_readme.py` against every commit, so this page cannot drift from the code. +### What a policy actually does to an agent + +Not "blocks the bad call at the last moment" — it decides what the planner is *able to propose*. The planner is told the policy before it plans, so a denied edge changes the shape of the graph rather than producing a refusal to retry. + +![A terminal: a failing test, then a policy denying any edge into apply_change, then a plan under that policy proposing three read-only nodes whose rationale says it cannot reach apply_change, then the amended policy, then the same goal producing a five-node mutating graph, then the execution, the diff, and the test passing.](docs/media/grapharc-cli-fix-bug.gif) + +That is GraphARC fixing a bug in GraphARC — a real one, from this project's backlog, in a copy of this repository. Under `deny *->apply_change` the planner proposes three read-only nodes and says so itself: + +> Since `apply_change` cannot be reached by an edge, this round investigates the torn trace bug and writes findings to notes **for a human to act on**. + +`mutating: false`. A person then amends the rule, and the same goal on the same model returns a five-node graph containing `apply_change`, `mutating: true`. Claude Code executes it and the red test goes green. The recording, and exactly what in it is staged (the pacing) and what is not (everything else), is in [docs/demo/](docs/demo/). + +## Supervised Claude Code, from Slack + +One message asks for work; the answer is the *graph* it intends to run — nodes, their governed kinds, edges, worst-case cost — and two buttons. Nothing executes until a human presses one. + +![A Slack thread: the bot replies with the proposed three-node graph and Approve / Deny buttons, the plan is approved, the nodes then run one by one, and the closing frame reads the trace back — approval_request, approval_response, then the first node's start.](docs/media/grapharc-slack-supervised.gif) + +``` +/grapharc plan "explain what flaky.py does and why it is not reproducible" --go \ + --model claude-cli --registry grapharc.stdlib:build_registry +``` + +`--go` means plan *and* execute. From Slack the gate appends `--approve` to it unconditionally, so the run parks before its first node — a message from anyone in the workspace can propose work, and only a person can start it. The click is bound to the plan's fingerprint, so a button on a superseded proposal is refused rather than honoured. Setup and the full rule list are in the [Slack cookbook](docs/cookbook/07-slack.md); what is and is not mocked in that recording is in [docs/demo/](docs/demo/). + ## Where it sits | | GraphARC | Claude Code | OpenClaw | raw LangGraph | @@ -179,5 +208,6 @@ The edges are documented, not denied — the full list with mechanisms is in the - The HTTP API does not yet use the durable session layer. - On the Claude CLI backend an agent node is *delegated*, not governed. - Policy documents govern planning; the tool plane still reads CLI flags. +- The MCP gate binds the MCP surface, not the host: an agent with its own file tools in the run directory could forge the approval decision. The trust boundary is the working directory, as it is for the Slack workspace. Version `0.1.5` · [changelog](CHANGELOG.md) · [roadmap](ROADMAP.md) · [website](https://codegraphcontext.github.io/GraphARC/) · MIT diff --git a/ROADMAP.md b/ROADMAP.md index 9a91a7e..9cd69b5 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -407,6 +407,17 @@ Everything here works and nothing calls it. and the answer was negative, `2` could not run at all. - [x] **9.6 — Streaming output to clients** via SSE, with a `last-event-id` cursor so a reconnect skips what it already saw. +- [x] **9.7 — MCP supervision server** (`grapharc mcp`, behind the `mcp` + extra that had waited unimported since it was declared). Three stdio + tools — plan / show_graph / execute — and deliberately no approval + verb: a mutating plan parks on the file handshake for an out-of-band + human, a read-only one runs on the host's own prompt, and the verdict + fails closed (an undeclared registry and an old plan.json both read as + mutating). `grapharc init --claude-code` writes the `.mcp.json` and + the skill that adopt it. Distinct from §3.5, which is the MCP *client* + — GraphARC calling third-party tools — and remains open. The trust + boundary is stated in the deep dive: the gate binds the MCP surface, + not a co-located agent's own hands. - [ ] **9.3 — Cron schedules** and **9.4 — webhook triggers.** - [ ] **9.5 — Chat channels** (Slack / Discord). diff --git a/docs/cookbook/07-slack.md b/docs/cookbook/07-slack.md index c97169f..18a5691 100644 --- a/docs/cookbook/07-slack.md +++ b/docs/cookbook/07-slack.md @@ -22,7 +22,8 @@ an afterthought. The defaults: | Reachable from Slack | Refused from Slack | |---|---| -| `demo`, `run`, `plan`, `models`, `replay`, `diff`, `trace`, `metrics`, `viz` | `serve` | +| `demo`, `run`, `plan`, `models`, `replay`, `diff`, `trace`, `metrics`, `viz`, `approve` | `serve` | +| `plan --go`, which executes what it planned — **always after a human approves** | `plan --go` without a gate: `--go` forces `--approve`, and the flag cannot be dropped from Slack | | Paths that resolve inside the bot's working directory | Any path that escapes it (`trace ../../.env` is refused before a process spawns) | | The budget, policy and trace flags each command already has | `--registry` (imports an arbitrary module), `--config`, `--json`, `--no-color` | | `plan --registry`, for exactly the two registries the package ships | any other `--registry` value | @@ -106,7 +107,8 @@ Configuration is environment-only, read once at startup: |---|---|---| | `SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN` | — (required) | the two tokens from the app page | | `GRAPHARC_SLACK_WORKDIR` | the bot's cwd | the directory every path must resolve inside | -| `GRAPHARC_SLACK_TIMEOUT` | `120` | seconds one command may run before it is killed | +| `GRAPHARC_SLACK_TIMEOUT` | `120` | seconds a *reader* command may run before it is killed | +| `GRAPHARC_SLACK_WORK_TIMEOUT` | `1800` | seconds a command that **executes** (`agent`, `plan --go`) may run. Two minutes is a sensible limit for `metrics` and a SIGKILL through the middle of an approved run; raised below the reader timeout, the larger of the two wins | | `GRAPHARC_SLACK_ALLOW_MODEL` | off | `1` admits `--model`/`--reviewer-model` | | `GRAPHARC_SLACK_ALLOW_AGENT` | off | `1` admits `agent` — only together with `ALLOW_MODEL` | | `GRAPHARC_SLACK_COMMAND` | `/grapharc` | the slash command to answer to | @@ -211,6 +213,80 @@ real model rather than pretending. `--registry` from Slack accepts exactly these two shipped modules and nothing else — the flag's general form imports arbitrary code, which stays refused. +## Supervised work: the graph before the work + +![One Slack message drives Claude Code: the bot answers with the proposed graph — nodes, kinds, edges, worst-case cost — and two buttons; nothing runs until Approve is clicked, and the closing frame reads the trace back to show approval_request, then approval_response, then the first node's start.](../media/grapharc-slack-supervised.gif) + +*A real run: a real planner, a real approval handshake, a real delegated Claude +Code phase. The Slack transport is the one mocked part — see +[docs/demo/](../demo/) for exactly what is and is not real, and for the two +scripts that regenerate this. ([mp4](../media/grapharc-slack-supervised.mp4))* + +The whole point of the bot is this one exchange. You are away from your +machine, something needs doing, and you would like to see *what* is going to +be done before it is: + +``` +/grapharc plan "explain what flaky.py does and why it is not reproducible" --go \ + --model claude-cli --registry grapharc.stdlib:build_registry +``` + +`--go` means plan **and** execute. From Slack it never means that on its own: +the gate appends `--approve` to any `plan --go` it admits, whichever registry +was chosen, so the run parks before its first node. That rule is not a +convenience — anyone in the workspace can type into this bot, and without it +one message would take a model's proposal straight to execution on the host +with the graph visible only afterwards. + +What comes back is the proposal itself, in the message: + +``` +`grapharc plan '…' --go --model claude-cli …` — ⏸ waiting for approval (10s) · explain what flaky.py does… + why: Explore the workspace to locate flaky.py, investigate its behaviour and + source of non-determinism, then summarize the findings. + 3 nodes: + · collect_context + · investigate_flaky (investigate) + · summarize + 4 edges: + __start__ → collect_context + collect_context → investigate_flaky + investigate_flaky → summarize + summarize → __end__ + worst case: 5500 tok +4 events · 1860 tok +nothing above has run yet — it runs only if you approve. +[ Approve ] [ Deny ] +``` + +Three things about that message are load-bearing: + +- **The graph is in it.** Not behind a link. The person deciding is on a + phone, and "the graph is over there" asks them to approve something they + have not seen. +- **Kinds, not just names.** `investigate_flaky` is a name the planner chose; + `investigate` is what the gate actually governs. A node whose kind can + change files is marked `✎`, read from the registry's own `MUTATING_KINDS`. + A registry that declares nothing gets every node marked and says so — nobody + said it was safe, so it is not treated as safe. +- **Nothing has run.** The trace proves it afterwards: `approval_request`, + then `approval_response`, then the first node's `start`, in that order. + +The buttons need `interactivity` enabled in the app manifest (the one above +has it). A click writes the decision file the parked run is polling for, +quoting the request's fingerprint — so a button on a message you scrolled back +to, drawn for a plan the run has since replaced, is refused rather than +honoured. The typed form stays in the message for a workspace without +interactivity: `/grapharc approve `, or `--deny`. + +Who clicked is posted into the thread. It is not in the trace: the trace has +no actor field (see the architecture review), and the bot will not imply an +audit trail it does not have. + +Time is split rather than shared. A parked `plan --go` gives the human a third +of the work budget (capped at 15 minutes) and leaves the rest for the run they +authorise; a plan-only park may have half, since nothing follows it. + ## The `agent` opt-in `agent` is the command with file tools, which is exactly why it is off by diff --git a/docs/cookbook/09-supervised-agents.md b/docs/cookbook/09-supervised-agents.md new file mode 100644 index 0000000..93972d2 --- /dev/null +++ b/docs/cookbook/09-supervised-agents.md @@ -0,0 +1,65 @@ +# 09 — Supervised agents: GraphARC as an MCP server + +The agents people already use — Claude Code first among them — act alone: one +loop, its own judgment, edits landing as fast as it can type them. This +chapter turns that around without replacing the agent. The agent keeps its +intelligence; GraphARC supplies the gate. Work is proposed as a graph, the +human sees the graph, and execution happens under budgets, onto the trace, +with the mutating case parked until a human answers out of band. + +## The shape of it + +`grapharc mcp` is a stdio MCP server exposing exactly three tools: + +| tool | what it does | what it cannot do | +|---|---|---| +| `plan(goal, scripted?, max_rounds?)` | run the governed planning loop for the goal; return the admitted shape — nodes, edges, rationale, fingerprint — plus a `mutating` verdict | choose a registry, policy or model: those resolve from the operator's `grapharc.toml` in the server's root, never from the call | +| `show_graph(run_dir)` | read a run directory back: the proposal, whether a human is being asked right now, and — after execution — metrics and the Mermaid of what ran | see raw state: like the live view, it serves rendered summaries, never `state_delta` | +| `execute(run_dir, approval_timeout?)` | re-admit the saved plan through the gate and run it | approve itself: a mutating plan parks on the file handshake until a human answers `grapharc approve ` | + +There is no `approve` tool, and there never will be. The gate is between the +agent and the operator; a client that could call `approve()` would be +approving its own proposal, which is not approval. The supervised agent may +*request* (execute parks) and may *check* (`show_graph` says +`awaiting_approval`); the decision belongs to a human with a terminal or the +live view, outside the connection entirely. + +The tiering is deliberate: a plan whose admitted kinds are all read-only +executes on the host agent's own permission prompt — the human already said +yes to the tool call — while any plan containing a mutating kind waits for +the out-of-band answer. The verdict is computed at plan time against the +registry module's own `MUTATING_KINDS`, is stored in `plan.json` beside the +fingerprint, and fails closed twice over: a registry that declared nothing +reads as mutating, and so does a plan file without the field. + +## Adopting it in a Claude Code project + +```bash +cd your-project +grapharc init # once, if there is no registry.py/grapharc.toml yet +grapharc init --claude-code # writes .mcp.json and .claude/skills/grapharc/SKILL.md +``` + +`.mcp.json` registers `grapharc mcp` as a project server, so Claude Code +starts it on demand. The skill is the behavioural half: it routes multi-step +and state-changing work through plan → show → execute, tells the agent to +render the proposal and the `watch_url` to the user, and states the boundary +the server cannot enforce on the host's *other* hands — never run +`grapharc approve`, never touch the request or decision files, and a timeout +means ask, not retry. Neither file is ever overwritten; an existing one is +yours, and the command refuses by name. + +Watch a supervised run the same way as any other: `grapharc serve +--live-root .grapharc/runs`, and the parked proposal is drawn on the live +page with the approve command beside it. + +## The trust boundary, stated plainly + +The MCP gate binds the MCP surface, not the machine. A host agent holds its +own Write and Bash, and a process in the working directory can forge +`approval-decision.json` — the same posture as the Slack gate's workspace: +the trust boundary is the directory, and the skill's never-clauses are the +contract for hands the server cannot see. The park also lives inside one MCP +call: a host that times the tool out kills the wait, the plan stays +unexecuted, and the call is safe to reissue. And approval records the +decision, never the decider — the trace has no actor field. diff --git a/docs/deep-dive.md b/docs/deep-dive.md index ccb36cb..f309ff9 100644 --- a/docs/deep-dive.md +++ b/docs/deep-dive.md @@ -162,6 +162,10 @@ grapharc diff trace.jsonl # what changed between two runs grapharc metrics trace.jsonl # tokens, retries, termination reason ``` +![A terminal: two runs of the same goal written to one trace file — the second under a tighter token ceiling, priced and refused during admission so nothing ran — then trace, replay and diff reading both back out of that single file.](media/grapharc-cli-audit.gif) + +*Two runs, one file. The second was given a smaller `--max-tokens`, so admission priced the proposal, refused it, and no node started; `diff` reports that as `path 3 -> 0 nodes`. Free and deterministic — see [docs/demo/](demo/).* + **Replay is a reconstruction, not a re-execution.** It rebuilds the node sequence, the folded state, the timing and the failures from the JSONL and calls no model, no tool and no node. Two limits come from the recording side rather than this one, and both are in the signature rather than a comment: strings past 2,000 characters were truncated when they were written, so they replay truncated; and the trace does not record which state fields have reducers, so a field LangGraph appended to replays last-write-wins unless you pass the reducer. **Spans are optional by construction.** One root span per run, one child per node execution, with `AgentNode` sub-steps parented by inference — and a sub-step whose parent cannot be identified is parented to the run span rather than to a guess. The OpenTelemetry dependency is confined behind a Protocol, so importing the module needs no OTel installed. This was carried as unverified against the real SDK for a while; it has now been run against `opentelemetry-sdk` 1.44.0 with spans arriving at a real exporter. @@ -228,6 +232,12 @@ A stable system is not one that claims to have no edges — it is one whose edge - **The HTTP API does not use the durable session layer.** It has its own `InProcessRuntime`, whose sessions die with the process and whose approvals are recorded without being delivered. [ROADMAP.md](../ROADMAP.md) §12.3. +**The MCP supervision surface** + +- **The MCP gate binds the MCP surface, not the host.** The server exposes no approval verb, so a supervised agent cannot decide over its own connection — but the host agent holds its own Write and Bash, and a process in the run directory can forge `approval-decision.json`. The trust boundary is the working directory, the same posture the Slack gate documents for its workspace; the shipped skill states the never-clauses for the hands the server cannot see. +- **The host's tool-permission prompt is UX, not enforcement.** Allowlists and skip-permissions modes erase it, and GraphARC cannot observe it. The park on the file handshake is the gate; the prompt is a courtesy in front of it. +- **A parked `execute` lives inside one MCP call.** A host that times the tool out kills the wait; the plan stays unexecuted and the call is safe to reissue. Approval records the decision, never the decider — the trace has no actor field. + **Real limits of things that do work** - **Admission authorises a kind, not its arguments.** A proposal carrying `args={"path": "/etc/passwd"}` is admitted on the strength of its kind alone. diff --git a/docs/demo/README.md b/docs/demo/README.md new file mode 100644 index 0000000..ef5220b --- /dev/null +++ b/docs/demo/README.md @@ -0,0 +1,139 @@ +# The demos, and how to re-make them + +Four recordings, two pipelines. Everything with behaviour runs for real; what +the renderers add is a typeface, a background and a readable pace. Each section +below says exactly which is which, because a demo you cannot check is a claim, +not evidence. + +| recording | what it shows | needs a model? | +|---|---|---| +| `grapharc-cli-gate` | a proposal refused, replanned, admitted, executed | no — `--scripted` | +| `grapharc-cli-fix-bug` | GraphARC fixing a real bug in GraphARC, under its own policy | yes — Claude subscription | +| `grapharc-cli-audit` | one trace file answering every question about a run | no — `--scripted` | +| `grapharc-slack-supervised` | one Slack message, the graph, the buttons, the run | yes — Claude subscription | + +## The CLI recordings + +`capture_cli.py` runs a scenario's commands in a **pseudo-terminal**, so the +CLI takes its tty branch and emits the colour a person actually sees. Piping +would give the byte-stable colourless form instead — also real, also tested, +but not what a demo is about. What lands in the recording is each command's +bytes, its exit code and its wall-clock duration. + +```bash +python docs/demo/capture_cli.py docs/demo/scenarios/gate.py --out /tmp/gate.json +python docs/demo/render_cli.py /tmp/gate.json --out docs/media/grapharc-cli-gate.mp4 \ + --gif docs/media/grapharc-cli-gate.gif +``` + +`render_cli.py` parses SGR colour — including the `38;5;N` form the CLI emits +under a 256-colour terminal — and draws a terminal grid. Any other escape is +dropped rather than half-interpreted; the CLI emits none, so guessing at one +would be inventing behaviour to render. + +**Real:** the commands, in that order, with those exit codes and those +durations. Every character on screen is a byte the CLI wrote. +**Invented:** the pacing. A command is typed out, output is revealed a couple +of lines at a time, and each step holds long enough to read. The real duration +is not thrown away — it is on the frame, bottom right, so a 43-second agent +phase says so while still being watchable. + +### Demo 2 is the one worth checking closely + +`scenarios/fix_bug.py` fixes a bug that was really on this project's backlog: +`mcp.driver.graph_status` read a running trace with the strict reader, so the +supervised agent's own status tool raised `TraceReadError` on a half-written +line — at exactly the moment it is meant to be useful. +`tests/test_torn_trace_read.py` is that bug report in checkable form. + +The recording runs against a **copy** of the repository, made with `git init` +and one baseline commit, so a demo cannot damage the tree it is a demo of: + +```bash +SRC=$(pwd); DEMO=/tmp/grapharc-demo-repo +rm -rf "$DEMO" && mkdir -p "$DEMO" +tar --exclude=.git --exclude=.venv --exclude=__pycache__ -cf - -C "$SRC" . | tar xf - -C "$DEMO" +cd "$DEMO" && git init -q && git add -A && git commit -qm baseline && cd "$SRC" + +GRAPHARC_DEMO_WORKDIR=$DEMO python docs/demo/capture_cli.py \ + docs/demo/scenarios/fix_bug.py --out /tmp/fix.json +python docs/demo/render_cli.py /tmp/fix.json \ + --out docs/media/grapharc-cli-fix-bug.mp4 --gif docs/media/grapharc-cli-fix-bug.gif +``` + +The two policy documents in that copy (`policy.toml`, `policy-approved.toml`) +are written by hand, and the demo turns on the difference between them. That is +deliberate rather than incidental: left to itself, `plan` **generates** a policy +from the goal, and a goal containing the word "fix" gets a generated policy that +permits `apply_change` — which is the designed behaviour ("investigate the +outage" and "fix the outage" deserve different answers) but makes for a demo +whose rule changes under you. An explicit `--policy` is also the production +path, so the recording shows the mechanism a real operator would use. + +What the recording establishes, and none of it is staged: + +- under the deny, the planner proposes three read-only nodes and its own + rationale says why — *"Since apply_change cannot be reached by an edge, this + round investigates … for a human to act on"* — and the plan is `mutating: + false`; +- after the amendment, the same goal and the same model produce a five-node + graph containing `apply_change`, marked `mutating: true`; +- the execution is Claude Code, delegated to because the Claude CLI has no + tool-calling wire format, editing real files in the copy; +- the fix it produced was `TraceRecorder` → `TailRecorder`, which is the + answer the rest of the codebase already uses for reading a file mid-write; +- the test that opened the recording red closes it green. + +That fix was then read by a person and landed here deliberately, with the +regression test — the demo is where it was found working, not how it shipped. + +## The Slack recording + +`capture_supervised_slack.py` drives `grapharc.slack.bot`'s own +`handle_text_live` — the function the bolt listeners call — against a sink that +keeps every edit instead of posting it, and clicks Approve through +`handle_approval_action` exactly as a real click does. + +```bash +# free, no model, no key +python docs/demo/capture_supervised_slack.py --workdir /tmp/ws --scripted --out /tmp/s.json +# the real thing, on a Claude subscription +python docs/demo/capture_supervised_slack.py --workdir /tmp/ws \ + --model claude-cli --registry grapharc.stdlib:build_registry \ + --goal "explain what flaky.py does" --out /tmp/s.json +python docs/demo/render_demo.py /tmp/s.json --out docs/media/grapharc-slack-supervised.mp4 \ + --gif docs/media/grapharc-slack-supervised.gif +``` + +**Real:** the admission gate, its rule that `--go` implies `--approve`, the +planner, the proposed graph, the file handshake, the fingerprint check on the +click, the execution, the trace. Every character of message text is what +`handle_text_live` produced, byte for byte. +**Not real:** the Slack transport. No workspace, no token, no socket — a +recording sink stands in for `chat.postMessage`/`chat.update`, exactly as the +test suite's sink does. The chrome (channel name, avatars, colours) is drawn by +`render_demo.py` and is not Slack's. +**Selected, not fabricated:** a code fence longer than 20 lines is cut, and the +frame says how many lines it hid. + +The closing frame is computed from the trace file rather than from any message: + +``` +phase order approval_request → approval_response → start +``` + +`start` is the first node beginning work, and it comes last. The same property +is asserted against a real CLI subprocess in +`tests/test_slack_supervision.py::test_a_slack_go_parks_shows_the_graph_is_approved_by_button_and_then_runs`, +so it does not depend on anyone re-recording a video. + +## Requirements + +The capture scripts need nothing but grapharc itself. The renderers need +Pillow, `ffmpeg` on PATH, and the DejaVu fonts (`fonts-dejavu-core` on +Debian/Ubuntu). None of it is a dependency of the package — these are +documentation tools. + +Two glyphs the Slack path emits (`⏸`, `⬜`) have no outline in DejaVu and are +substituted (`‖`, `▫`) so they do not render as .notdef boxes; the table is at +the bottom of `render_demo.py`. diff --git a/docs/demo/capture_cli.py b/docs/demo/capture_cli.py new file mode 100644 index 0000000..46afefb --- /dev/null +++ b/docs/demo/capture_cli.py @@ -0,0 +1,172 @@ +"""Run a scripted terminal session for real and record what it printed. + +Each step is a shell command run in a **pseudo-terminal**, so the CLI takes its +tty branch and emits the colour a person actually sees — piping it would give +the byte-stable colourless form instead, which is a different (also real, also +tested) output and not the one a demo is about. + +Nothing is faked: the commands run, in order, in a working directory you name, +and what lands in the recording is their bytes and their wall-clock duration. +A step may declare `expect_exit` so a demo that is *supposed* to show a failure +(a crash, a refusal) does not read as the capture going wrong. + + python docs/demo/capture_cli.py docs/demo/scenarios/gate.py --out session.json + +A scenario module defines `WORKDIR`, `TITLE`, and `STEPS` — see +`docs/demo/scenarios/` for the three this repo ships. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import os +import pty +import selectors +import subprocess +import time +from pathlib import Path + +#: The terminal width every recording is made at, so frames line up with the +#: renderer's grid and the CLI wraps where the video shows it wrapping. +COLUMNS = 100 + + +def run_step( + command: str, *, cwd: Path, timeout: float, venv_bin: Path | None = None +) -> dict: + """One command in a pty. Returns its bytes, exit code and duration.""" + master, slave = pty.openpty() + started = time.monotonic() + environment = { + **os.environ, + # A real terminal width, so the CLI's rules and tables wrap the way + # they would on someone's screen rather than at whatever COLUMNS the + # capture process happened to inherit. + "COLUMNS": str(COLUMNS), + "TERM": "xterm-256color", + } + if venv_bin is not None: + # So the recording shows `grapharc …`, which is what a reader will + # type, rather than `python -m grapharc.cli.main …`, which is what a + # checkout without an activated environment would need. + environment["PATH"] = f"{venv_bin}{os.pathsep}{environment.get('PATH', '')}" + process = subprocess.Popen( + ["bash", "--noprofile", "--norc", "-c", command], + cwd=cwd, + stdout=slave, + stderr=slave, + stdin=subprocess.DEVNULL, + env=environment, + ) + os.close(slave) + chunks: list[bytes] = [] + selector = selectors.DefaultSelector() + selector.register(master, selectors.EVENT_READ) + deadline = started + timeout + while True: + if time.monotonic() > deadline: + process.kill() + chunks.append(b"\n[capture] timed out\n") + break + for _key, _mask in selector.select(timeout=0.2): + try: + data = os.read(master, 65536) + except OSError: + data = b"" + if data: + chunks.append(data) + if process.poll() is not None: + # Drain whatever the child wrote just before exiting. + while True: + try: + data = os.read(master, 65536) + except OSError: + break + if not data: + break + chunks.append(data) + break + selector.close() + os.close(master) + process.wait() + return { + "command": command, + "output": b"".join(chunks).decode("utf-8", errors="replace"), + "exit_code": process.returncode, + "seconds": round(time.monotonic() - started, 2), + } + + +def load_scenario(path: Path): + spec = importlib.util.spec_from_file_location("scenario", path) + module = importlib.util.module_from_spec(spec) + assert spec.loader is not None + spec.loader.exec_module(module) + return module + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("scenario", type=Path) + parser.add_argument("--out", type=Path, required=True) + parser.add_argument("--timeout", type=float, default=900.0) + parser.add_argument( + "--venv-bin", + type=Path, + default=Path(__file__).resolve().parents[2] / ".venv" / "bin", + help="prepended to PATH so `grapharc` resolves without an activated venv", + ) + args = parser.parse_args() + + scenario = load_scenario(args.scenario) + workdir = Path(scenario.WORKDIR).resolve() + setup = getattr(scenario, "setup", None) + if setup is not None: + setup(workdir) + workdir.mkdir(parents=True, exist_ok=True) + + steps = [] + for step in scenario.STEPS: + command = step["run"] + print(f"$ {command}", flush=True) + result = run_step( + command, + cwd=workdir, + timeout=step.get("timeout", args.timeout), + venv_bin=args.venv_bin, + ) + expected = step.get("expect_exit") + if expected is not None and result["exit_code"] != expected: + # Loud, not fatal: a demo whose refusal stopped refusing is a + # finding about the code, and the recording is the evidence. + print( + f" ! expected exit {expected}, got {result['exit_code']}", + flush=True, + ) + result["caption"] = step.get("caption", "") + result["expect_exit"] = expected + result["hold"] = step.get("hold") + steps.append(result) + print(f" exit {result['exit_code']} in {result['seconds']}s", flush=True) + + args.out.parent.mkdir(parents=True, exist_ok=True) + args.out.write_text( + json.dumps( + { + "title": scenario.TITLE, + "subtitle": getattr(scenario, "SUBTITLE", ""), + "workdir": str(workdir), + "steps": steps, + }, + indent=2, + ), + encoding="utf-8", + ) + print(f"\n{len(steps)} steps -> {args.out}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docs/demo/capture_supervised_slack.py b/docs/demo/capture_supervised_slack.py new file mode 100644 index 0000000..2b662f0 --- /dev/null +++ b/docs/demo/capture_supervised_slack.py @@ -0,0 +1,198 @@ +"""Drive the real Slack path end to end and record every message it produced. + +This is the capture half of the demo video. It runs `grapharc.slack.bot`'s own +`handle_text_live` — the same function the bolt listeners call — against a sink +that keeps every edit instead of posting it, clicks the Approve button through +`handle_approval_action` the way a real click would, and writes the result to +a JSON file for `render_demo.py` to turn into frames. + +What is real here: the admission gate, the planner (a real model with +`--model`), the proposed graph, the approval file handshake, the fingerprint +check, the execution, and the trace. Every line of message text in the output +is the text the bot would have sent to Slack, byte for byte. + +What is not real: the Slack transport. There is no workspace, no token and no +socket — the sink stands in for `chat.postMessage`/`chat.update`, exactly as +the test suite's sink does. That substitution is the point of the module +layout: everything with behaviour is testable, and demonstrable, without Slack. + + python docs/demo/capture_supervised_slack.py --workdir /tmp/ws --scripted + python docs/demo/capture_supervised_slack.py --workdir /tmp/ws \ + --model claude-cli --registry grapharc.stdlib:build_registry \ + --goal "summarise what flaky.py does" +""" + +from __future__ import annotations + +import argparse +import json +import threading +import time +from pathlib import Path + +from grapharc.slack.bot import handle_approval_action, handle_text_live +from grapharc.slack.config import SlackBotConfig + + +class RecordingSink: + """A `LiveSink` that timestamps every message instead of sending it.""" + + def __init__(self) -> None: + self.frames: list[dict] = [] + self.started = time.monotonic() + self._lock = threading.Lock() + + def _record(self, kind: str, text: str, blocks=None) -> None: + elements = [ + element + for block in (blocks or []) + if block.get("type") == "actions" + for element in block["elements"] + ] + with self._lock: + self.frames.append( + { + "at": round(time.monotonic() - self.started, 2), + "kind": kind, + "text": text, + # Labels for the renderer, values for the click. The value + # is what Slack would hand back on a real press, so the + # click below is the same call the bolt listener makes. + "buttons": [element["text"]["text"] for element in elements], + "values": [element["value"] for element in elements], + } + ) + + def post(self, text: str): + self._record("post", text) + return ("C_DEMO", "1.0") + + def update(self, handle, text: str, blocks=None) -> bool: + self._record("update", text, blocks) + return True + + def wait_for_buttons(self, timeout: float): + """Block until an edit carries buttons; return its value, or None.""" + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + with self._lock: + for frame in self.frames: + if frame["buttons"]: + return frame + time.sleep(0.1) + return None + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--workdir", required=True, type=Path) + parser.add_argument("--out", type=Path, default=Path("docs/demo/session.json")) + parser.add_argument("--goal", default="investigate the checkout outage") + parser.add_argument("--model", default=None, help="omit to use --scripted") + parser.add_argument("--registry", default=None) + parser.add_argument("--scripted", action="store_true") + parser.add_argument("--deny", action="store_true", help="click Deny instead") + parser.add_argument("--approve-after", type=float, default=3.0) + parser.add_argument("--timeout", type=float, default=900.0) + args = parser.parse_args() + + workdir = args.workdir.resolve() + workdir.mkdir(parents=True, exist_ok=True) + + request = f'plan "{args.goal}" --go' + if args.scripted: + request += " --scripted" + if args.model: + request += f" --model {args.model}" + if args.registry: + request += f" --registry {args.registry}" + + config = SlackBotConfig( + bot_token="xoxb-demo", + app_token="xapp-demo", + workdir=workdir, + timeout_seconds=120.0, + work_timeout_seconds=args.timeout, + live_interval_seconds=1.0, + allow_model=True, + allow_agent=True, + ) + + sink = RecordingSink() + decision: dict = {} + + def click() -> None: + frame = sink.wait_for_buttons(timeout=args.timeout) + if frame is None: + decision["error"] = "the run never offered a button" + return + # A human reads the plan before answering; the pause is the demo's way + # of showing that the run is genuinely waiting rather than racing. + time.sleep(args.approve_after) + value = frame["values"][1 if args.deny else 0] + decision["at"] = round(time.monotonic() - sink.started, 2) + decision["answer"] = handle_approval_action( + value, config, deny=args.deny, actor="U_DEMO" + ) + + clicker = threading.Thread(target=click, daemon=True) + clicker.start() + reply = handle_text_live(request, config, sink) + clicker.join(timeout=30.0) + + session = { + "request": request, + "workdir": str(workdir), + "frames": sink.frames, + "decision": decision, + "trailing_reply": reply, + "audit": _audit(workdir), + } + args.out.parent.mkdir(parents=True, exist_ok=True) + args.out.write_text(json.dumps(session, indent=2), encoding="utf-8") + print(f"{len(sink.frames)} frames -> {args.out}") + print(f"decision: {decision.get('answer') or decision.get('error')}") + return 0 + + +def _audit(workdir: Path) -> dict: + """What the trace says happened, read back the way any auditor would. + + This is the claim the whole design makes, checked against the file rather + than against the messages: the plan was asked about, a human answered, and + only then did a node start. The order of those three phases is the proof; + a run that executed before its `approval_response` would show it here. + """ + from grapharc.observe.replay import replay + from grapharc.observe.trace import TailRecorder + + traces = sorted(workdir.glob("slack-runs/*/trace.jsonl")) + if not traces: + return {} + recorder = TailRecorder(traces[-1]) + run_id = recorder.run_ids()[-1] + events = list(recorder.read_events(run_id)) + run = replay(recorder, run_id) + order = [ + event.phase + for event in events + if event.phase in ("approval_request", "approval_response", "start") + ] + return { + "trace": str(traces[-1].relative_to(workdir)), + "run_id": run_id, + "events": len(events), + "phase_order": list(dict.fromkeys(order)), + "executed": [execution.node for execution in run.executions if execution.completed], + "tokens": run.tokens, + "cost_usd": run.recorded_cost_usd, + "approved_before_first_node": ( + "approval_response" in order + and "start" in order + and order.index("approval_response") < order.index("start") + ), + } + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docs/demo/render_cli.py b/docs/demo/render_cli.py new file mode 100644 index 0000000..d0e0ad0 --- /dev/null +++ b/docs/demo/render_cli.py @@ -0,0 +1,316 @@ +"""Render a captured terminal session to an mp4 (and gif). + +The recording holds real bytes with real ANSI colour; this draws them into a +terminal-shaped frame sequence. The CLI emits SGR colour and nothing else — no +cursor addressing, no spinners — so a full terminal emulator is not needed and +would be a worse thing to trust: what is parsed here is exactly `\\x1b[…m`, and +any other escape is dropped rather than half-interpreted. + +Pacing is the one thing this invents. A command is typed out, its output is +revealed a few lines at a time, and each step holds long enough to read. Real +durations are not thrown away — each step's frame shows the wall clock the +command actually took, so a 43-second agent phase says so while still being +watchable. + + python docs/demo/render_cli.py session.json --out docs/media/x.mp4 --gif x.gif +""" + +from __future__ import annotations + +import argparse +import json +import re +import shutil +import subprocess +import tempfile +from pathlib import Path + +from PIL import Image, ImageDraw, ImageFont + +COLS, ROWS = 100, 30 +FONT_SIZE = 17 +#: Measured from the font rather than chosen, so a column lands exactly where +#: the glyph before it ends. A guessed cell width shows up as text that either +#: overlaps or drifts apart across a 100-column line. +CELL_W = 10.234375 # DejaVuSansMono advance at FONT_SIZE +CELL_H = 22 +PAD = 26 +HEADER = 44 +FOOTER = 46 + + +def _even(value: float) -> int: + """h264's yuv420p subsamples by two, so an odd dimension is an encode error.""" + return round(value / 2) * 2 + + +WIDTH = _even(COLS * CELL_W + 2 * PAD) +HEIGHT = _even(ROWS * CELL_H + 2 * PAD + HEADER + FOOTER) +FPS = 12 +#: A command is typed in this many frames whatever its length, over this many +#: seconds. Fixed frames rather than fixed characters-per-frame: a +#: `grapharc plan '' --model … --registry …` line runs past 250 +#: characters, and three-characters-a-frame spent eighty frames — seven +#: seconds — watching a prompt fill in. +TYPING_FRAMES = 10 +TYPING_SECONDS = 1.1 + +BG = (18, 20, 24) +CHROME = (30, 33, 38) +FG = (214, 219, 226) +MUTED = (132, 140, 152) +PROMPT = (110, 190, 130) + +#: The sixteen named ANSI colours. `cli/style.py` picks its depth from the +#: terminal and emits `38;5;N` under `TERM=xterm-256color` (which is what the +#: capture sets), so these are the fallback for a 16-colour recording. +ANSI_16 = [ + (40, 44, 52), (222, 95, 95), (110, 190, 130), (222, 176, 92), + (108, 158, 232), (186, 130, 220), (94, 190, 200), FG, + MUTED, (240, 130, 130), (140, 210, 155), (238, 200, 120), + (140, 180, 240), (208, 160, 235), (130, 210, 218), (245, 248, 252), +] + + +def xterm256(index: int) -> tuple[int, int, int]: + """Colour `N` of `38;5;N`, by the standard's own construction. + + Computed rather than tabulated: 0-15 are the named colours, 16-231 are a + 6x6x6 cube with the documented non-linear ramp, 232-255 are the grey + ramp. A hand-copied 256-entry table is exactly the kind of thing that ends + up subtly wrong and makes a demo misrepresent what the tool printed. + """ + if index < 16: + return ANSI_16[index] + if index < 232: + index -= 16 + levels = (0, 95, 135, 175, 215, 255) + return (levels[index // 36], levels[(index // 6) % 6], levels[index % 6]) + value = 8 + (index - 232) * 10 + return (value, value, value) + + +FONT_DIR = Path("/usr/share/fonts/truetype/dejavu") + +#: One pattern for every escape, classified by its final byte — not two +#: patterns racing each other. A separate "strip the ones I don't handle" regex +#: matched `\x1b[38;5;245m` too (its final byte is a letter like any other) and +#: silently removed every colour before the SGR parser ran, which renders a +#: perfectly colourful session in monochrome. +_CSI = re.compile(r"\x1b\[([0-9;?]*)([@-~])|\x1b\][^\x07]*\x07") + + +class Screen: + """A fixed grid of (char, colour, bold) cells that scrolls.""" + + def __init__(self) -> None: + self.rows: list[list[tuple[str, tuple, bool]]] = [[]] + self.colour = FG + self.bold = False + + def write(self, text: str) -> None: + position = 0 + for match in _CSI.finditer(text): + self._plain(text[position : match.start()]) + # Only `m` is colour. Every other CSI (and any OSC) is dropped + # rather than half-interpreted: the CLI emits none of them, so + # guessing at one would be inventing behaviour to render. + if match.group(2) == "m": + self._sgr(match.group(1)) + position = match.end() + self._plain(text[position:]) + + def _sgr(self, params: str) -> None: + codes = [int(raw or 0) for raw in (params or "0").split(";")] + index = 0 + while index < len(codes): + code = codes[index] + if code == 0: + self.colour, self.bold = FG, False + elif code == 1: + self.bold = True + elif code == 2: + self.colour = MUTED + elif code == 38 and index + 1 < len(codes): + # Extended foreground: `38;5;N` (256) or `38;2;R;G;B` (true + # colour). Both consume their arguments, so the loop has to + # skip them — reading them as further SGR codes would repaint + # the text with whatever the channel values happened to mean. + if codes[index + 1] == 5 and index + 2 < len(codes): + self.colour = xterm256(codes[index + 2]) + index += 2 + elif codes[index + 1] == 2 and index + 4 < len(codes): + self.colour = tuple(codes[index + 2 : index + 5]) + index += 4 + elif code == 39: + self.colour = FG + elif 30 <= code <= 37: + self.colour = ANSI_16[code - 30] + elif 90 <= code <= 97: + self.colour = ANSI_16[code - 90 + 8] + index += 1 + + def _plain(self, text: str) -> None: + for char in text: + if char == "\n": + self.rows.append([]) + elif char == "\r": + self.rows[-1] = [] + elif char == "\t": + self._plain(" " * (4 - len(self.rows[-1]) % 4)) + elif char >= " ": + if len(self.rows[-1]) >= COLS: + self.rows.append([]) + self.rows[-1].append((char, self.colour, self.bold)) + + def visible(self) -> list[list[tuple[str, tuple, bool]]]: + return self.rows[-ROWS:] + + def clone(self) -> Screen: + copy = Screen() + copy.rows = [list(row) for row in self.rows] + copy.colour, copy.bold = self.colour, self.bold + return copy + + +class Renderer: + def __init__(self, title: str, subtitle: str) -> None: + self.mono = ImageFont.truetype(str(FONT_DIR / "DejaVuSansMono.ttf"), FONT_SIZE) + self.mono_bold = ImageFont.truetype( + str(FONT_DIR / "DejaVuSansMono-Bold.ttf"), FONT_SIZE + ) + self.sans = ImageFont.truetype(str(FONT_DIR / "DejaVuSans.ttf"), 14) + self.sans_bold = ImageFont.truetype(str(FONT_DIR / "DejaVuSans-Bold.ttf"), 15) + self.title = title + self.subtitle = subtitle + + def draw(self, screen: Screen, *, caption: str, elapsed: str = "") -> Image.Image: + image = Image.new("RGB", (WIDTH, HEIGHT), BG) + draw = ImageDraw.Draw(image) + + draw.rectangle([0, 0, WIDTH, HEADER], fill=CHROME) + for index, colour in enumerate(((236, 106, 94), (238, 190, 82), (108, 198, 118))): + x = PAD + index * 20 + draw.ellipse([x, 16, x + 12, 28], fill=colour) + draw.text((PAD + 80, 14), self.title, font=self.sans_bold, fill=FG) + if self.subtitle: + offset = PAD + 92 + self.sans_bold.getlength(self.title) + draw.text((offset, 15), self.subtitle, font=self.sans, fill=MUTED) + + top = HEADER + PAD + for row_index, row in enumerate(screen.visible()): + y = top + row_index * CELL_H + for column, (char, colour, bold) in enumerate(row): + draw.text( + (PAD + column * CELL_W, y), + char, + font=self.mono_bold if bold else self.mono, + fill=colour, + ) + + draw.rectangle([0, HEIGHT - FOOTER, WIDTH, HEIGHT], fill=CHROME) + draw.text((PAD, HEIGHT - FOOTER + 15), caption, font=self.sans, fill=MUTED) + if elapsed: + draw.text( + (WIDTH - PAD - self.sans.getlength(elapsed), HEIGHT - FOOTER + 15), + elapsed, + font=self.sans, + fill=MUTED, + ) + return image + + +def _prompt_line(command: str, upto: int) -> str: + return f"\x1b[32m$\x1b[0m {command[:upto]}" + + +def build_frames(session: dict, args) -> list[tuple[Image.Image, float]]: + renderer = Renderer(session["title"], session.get("subtitle", "")) + screen = Screen() + frames: list[tuple[Image.Image, float]] = [] + + for step in session["steps"]: + command = step["command"] + caption = step.get("caption") or "" + elapsed = f"{step['seconds']:.1f}s · exit {step['exit_code']}" + + # Type the command in a fixed number of frames rather than a fixed + # number of characters per frame. A `grapharc plan '' --model …` + # line runs past 250 characters, and one frame per three characters + # made *typing it* eighty frames — seven seconds of watching a prompt + # fill in, and the single biggest contributor to the file size. + base = screen.clone() + steps = max(1, min(TYPING_FRAMES, len(command))) + for frame_index in range(1, steps + 1): + typing = base.clone() + typing.write(_prompt_line(command, round(len(command) * frame_index / steps))) + frames.append((renderer.draw(typing, caption=caption), TYPING_SECONDS / steps)) + screen.write(_prompt_line(command, len(command)) + "\n") + frames.append((renderer.draw(screen, caption=caption), 0.35)) + + # Reveal the output in blocks, so a long report scrolls rather than + # appearing whole. + lines = step["output"].splitlines() + block = max(1, args.lines_per_frame) + for index in range(0, len(lines), block): + screen.write("\n".join(lines[index : index + block]) + "\n") + frames.append((renderer.draw(screen, caption=caption, elapsed=elapsed), 1 / FPS * 2)) + hold = step.get("hold") or args.hold + frames.append((renderer.draw(screen, caption=caption, elapsed=elapsed), hold)) + screen.write("\n") + + return frames + + +def encode(frames, out: Path, gif: Path | None) -> None: + if shutil.which("ffmpeg") is None: + raise SystemExit("ffmpeg is required to encode the video") + with tempfile.TemporaryDirectory() as tmp: + listing = [] + for index, (image, seconds) in enumerate(frames): + path = Path(tmp) / f"{index:05d}.png" + image.save(path) + listing.append(f"file '{path}'\nduration {seconds}") + listing.append(f"file '{Path(tmp) / f'{len(frames) - 1:05d}.png'}'") + script = Path(tmp) / "frames.txt" + script.write_text("\n".join(listing), encoding="utf-8") + out.parent.mkdir(parents=True, exist_ok=True) + subprocess.run( + ["ffmpeg", "-y", "-f", "concat", "-safe", "0", "-i", str(script), + "-vf", f"fps={FPS},format=yuv420p", "-movflags", "+faststart", str(out)], + check=True, capture_output=True, + ) + if gif is not None: + palette = Path(tmp) / "palette.png" + subprocess.run( + ["ffmpeg", "-y", "-i", str(out), "-vf", + "fps=6,scale=900:-1:flags=lanczos,palettegen", str(palette)], + check=True, capture_output=True, + ) + subprocess.run( + ["ffmpeg", "-y", "-i", str(out), "-i", str(palette), "-lavfi", + "fps=6,scale=900:-1:flags=lanczos[x];[x][1:v]paletteuse", str(gif)], + check=True, capture_output=True, + ) + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("session", type=Path) + parser.add_argument("--out", type=Path, required=True) + parser.add_argument("--gif", type=Path, default=None) + parser.add_argument("--hold", type=float, default=2.6, help="seconds at the end of a step") + parser.add_argument("--lines-per-frame", type=int, default=2) + args = parser.parse_args() + + session = json.loads(args.session.read_text(encoding="utf-8")) + frames = build_frames(session, args) + encode(frames, args.out, args.gif) + total = sum(seconds for _image, seconds in frames) + where = f"{args.out}" + (f" and {args.gif}" if args.gif else "") + print(f"{len(frames)} frames, {total:.1f}s -> {where}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docs/demo/render_demo.py b/docs/demo/render_demo.py new file mode 100644 index 0000000..7fd4d0e --- /dev/null +++ b/docs/demo/render_demo.py @@ -0,0 +1,385 @@ +"""Turn a captured session into an mp4 (and gif) of the Slack conversation. + +Input is `capture_supervised_slack.py`'s JSON: one entry per message the bot +posted or edited, with the timestamp it happened at and the buttons it carried. +Every character rendered here came out of the bot; this module chooses a +typeface and a background and nothing else. + + python docs/demo/render_demo.py docs/demo/session.json --out docs/media/x.mp4 + +Frames are drawn with Pillow and encoded with ffmpeg, both of which must be +available — this is a documentation tool, not part of the package, and neither +is a dependency of `grapharc` itself. +""" + +from __future__ import annotations + +import argparse +import json +import re +import shutil +import subprocess +import tempfile +from pathlib import Path + +from PIL import Image, ImageDraw, ImageFont + +WIDTH, HEIGHT = 1280, 800 +MARGIN = 48 +FPS = 12 +#: Lines of a code fence one frame shows before it says how many it hid. +MAX_FENCE_LINES = 20 + +# Slack-ish dark palette. Not Slack's brand colours — this is a mock, and it +# should look like one rather than pass for a screenshot of the product. +BG = (26, 29, 33) +PANEL = (34, 37, 41) +FENCE_BG = (22, 24, 27) +TEXT = (222, 225, 230) +MUTED = (141, 148, 158) +ACCENT = (91, 140, 255) +GREEN = (46, 160, 106) +RED = (203, 68, 74) +AMBER = (222, 168, 62) + +FONT_DIR = Path("/usr/share/fonts/truetype/dejavu") + + +def _font(name: str, size: int) -> ImageFont.FreeTypeFont: + return ImageFont.truetype(str(FONT_DIR / name), size) + + +class Renderer: + def __init__(self) -> None: + self.mono = _font("DejaVuSansMono.ttf", 15) + self.mono_small = _font("DejaVuSansMono.ttf", 13) + self.sans = _font("DejaVuSans.ttf", 17) + self.sans_bold = _font("DejaVuSans-Bold.ttf", 17) + self.title = _font("DejaVuSans-Bold.ttf", 21) + self.small = _font("DejaVuSans.ttf", 13) + + # -- text helpers --------------------------------------------------------- + + def wrap(self, text: str, font, width: int) -> list[str]: + """Wrap on words, then hard-break anything still too wide.""" + out: list[str] = [] + for raw in text.split("\n"): + if not raw: + out.append("") + continue + line = "" + for word in raw.split(" "): + candidate = f"{line} {word}".strip() + if font.getlength(candidate) <= width or not line: + line = candidate + else: + out.append(line) + line = word + out.append(line) + broken: list[str] = [] + for line in out: + while font.getlength(line) > width and len(line) > 1: + cut = len(line) + while cut > 1 and font.getlength(line[:cut]) > width: + cut -= 1 + broken.append(line[:cut]) + line = line[cut:] + broken.append(line) + return broken + + # -- one frame ------------------------------------------------------------ + + def _chrome(self, draw: ImageDraw.ImageDraw) -> None: + draw.rectangle([0, 0, WIDTH, 64], fill=PANEL) + draw.text((MARGIN, 20), "# incidents", font=self.title, fill=TEXT) + draw.text( + (WIDTH - MARGIN - 330, 24), + "GraphARC — supervised Claude Code", + font=self.small, + fill=MUTED, + ) + + def _author(self, draw: ImageDraw.ImageDraw, y: int, *, app: bool) -> int: + initials, name, colour = ( + ("ga", "grapharc", ACCENT) if app else ("yo", "you", (140, 110, 190)) + ) + draw.ellipse([MARGIN, y, MARGIN + 34, y + 34], fill=colour) + draw.text((MARGIN + 9, y + 8), initials, font=self.small, fill=(255, 255, 255)) + draw.text((MARGIN + 48, y + 2), name, font=self.sans_bold, fill=TEXT) + if app: + draw.text((MARGIN + 140, y + 5), "APP", font=self.small, fill=MUTED) + return y + 38 + + def request_frame(self, request: str, caption: str) -> Image.Image: + """The message a person typed. The one frame the bot did not produce.""" + image = Image.new("RGB", (WIDTH, HEIGHT), BG) + draw = ImageDraw.Draw(image) + self._chrome(draw) + y = self._author(draw, 96, app=False) + for line in self.wrap(f"/grapharc {request}", self.mono, WIDTH - 2 * MARGIN - 48): + draw.text((MARGIN + 48, y), line, font=self.mono, fill=TEXT) + y += 22 + draw.rectangle([0, HEIGHT - 56, WIDTH, HEIGHT], fill=PANEL) + draw.text((MARGIN, HEIGHT - 38), caption, font=self.small, fill=MUTED) + return image + + def audit_frame(self, audit: dict, caption: str) -> Image.Image: + """The closing frame: what the trace file says, not what a message said.""" + image = Image.new("RGB", (WIDTH, HEIGHT), BG) + draw = ImageDraw.Draw(image) + self._chrome(draw) + y = 110 + draw.text((MARGIN, y), "the trace, read back", font=self.title, fill=TEXT) + y += 44 + order = " → ".join(audit.get("phase_order", [])) + rows = [ + ("trace", audit.get("trace", "—")), + ("run", audit.get("run_id", "—")), + ("events", str(audit.get("events", 0))), + ("phase order", order or "—"), + ("nodes that ran", ", ".join(audit.get("executed", [])) or "(none)"), + ("tokens", str(audit.get("tokens", 0))), + ( + "cost", + "—" if audit.get("cost_usd") in (None, 0) else f"${audit['cost_usd']:.4f}", + ), + ] + for label, value in rows: + draw.text((MARGIN, y), f"{label:>16}", font=self.mono, fill=MUTED) + lines = self.wrap(value, self.mono, WIDTH - 2 * MARGIN - 190) or [""] + for index, line in enumerate(lines): + draw.text((MARGIN + 180, y + index * 20), line, font=self.mono, fill=TEXT) + y += 22 + 20 * (len(lines) - 1) + y += 26 + verdict = audit.get("approved_before_first_node") + draw.text( + (MARGIN, y), + "✓ approved before the first node started" + if verdict + else "✗ a node started before the approval was answered", + font=self.sans_bold, + fill=GREEN if verdict else RED, + ) + draw.rectangle([0, HEIGHT - 56, WIDTH, HEIGHT], fill=PANEL) + draw.text((MARGIN, HEIGHT - 38), caption, font=self.small, fill=MUTED) + return image + + def frame(self, message: dict, *, caption: str, cursor: tuple | None = None) -> Image.Image: + image = Image.new("RGB", (WIDTH, HEIGHT), BG) + draw = ImageDraw.Draw(image) + self._chrome(draw) + y = self._author(draw, 96, app=True) + + body_width = WIDTH - 2 * MARGIN - 48 + left = MARGIN + 48 + for block, kind in _blocks(message["text"]): + if kind == "fence": + lines = [ + wrapped + for line in block.split("\n") + for wrapped in self.wrap(line, self.mono, body_width - 24) + ] + # Measured before the box is drawn, not while filling it: the + # final message carries the CLI's whole report, and a rectangle + # sized for every line of that runs off the bottom of the frame. + if len(lines) > MAX_FENCE_LINES: + hidden = len(lines) - MAX_FENCE_LINES + lines = lines[:MAX_FENCE_LINES] + [f"… {hidden} more lines"] + height = len(lines) * 20 + 20 + draw.rounded_rectangle( + [left, y, left + body_width, y + height], radius=6, fill=FENCE_BG + ) + inner = y + 10 + for line in lines: + draw.text((left + 12, inner), line, font=self.mono, fill=_line_colour(line)) + inner += 20 + y += height + 10 + else: + for line in self.wrap(block, self.sans, body_width): + draw.text((left, y), line, font=self.sans, fill=_prose_colour(line)) + y += 24 + y += 6 + if y > HEIGHT - 200: + break + + if message.get("buttons"): + y += 8 + x = left + for index, label in enumerate(message["buttons"]): + fill = GREEN if index == 0 else RED + width = 120 + draw.rounded_rectangle([x, y, x + width, y + 42], radius=6, fill=fill) + offset = (width - self.sans_bold.getlength(label)) / 2 + draw.text((x + offset, y + 10), label, font=self.sans_bold, fill=(255, 255, 255)) + x += width + 14 + if cursor is not None: + cx, cy = left + cursor[0], y + cursor[1] + draw.polygon( + [(cx, cy), (cx, cy + 20), (cx + 6, cy + 15), (cx + 13, cy + 26), + (cx + 18, cy + 23), (cx + 11, cy + 12), (cx + 18, cy + 11)], + fill=(255, 255, 255), + outline=(20, 20, 20), + ) + + draw.rectangle([0, HEIGHT - 56, WIDTH, HEIGHT], fill=PANEL) + draw.text((MARGIN, HEIGHT - 38), caption, font=self.small, fill=MUTED) + stamp = f"t+{message['at']:.1f}s" + draw.text( + (WIDTH - MARGIN - self.small.getlength(stamp), HEIGHT - 38), + stamp, + font=self.small, + fill=MUTED, + ) + return image + + +def _blocks(text: str) -> list[tuple[str, str]]: + """Split a Slack message into prose and code-fence blocks.""" + out: list[tuple[str, str]] = [] + for index, chunk in enumerate(text.split("```")): + if not chunk.strip(): + continue + out.append((chunk.strip("\n"), "fence" if index % 2 else "prose")) + return out + + +def _line_colour(line: str) -> tuple[int, int, int]: + stripped = line.strip() + if stripped.startswith("✎") or stripped.startswith("✗"): + return AMBER if stripped.startswith("✎") else RED + if stripped.startswith("✓"): + return GREEN + if stripped.startswith("▸"): + return AMBER + if "→" in stripped: + return (168, 190, 230) + return TEXT + + +def _prose_colour(line: str) -> tuple[int, int, int]: + if "waiting for approval" in line: + return AMBER + if "nothing above has run yet" in line: + return AMBER + if line.startswith(" str: + """Strip Slack link markup so a rendered frame reads as text.""" + for glyph, substitute in _SUBSTITUTES.items(): + text = text.replace(glyph, substitute) + text = re.sub(r"<(https?://[^|>]+)\|([^>]+)>", r"\2 →", text) + text = re.sub(r"<(https?://[^>]{0,60})[^>]*>", r"\1…", text) + # The CLI writes the final state dump with escaped newlines inside it. + text = text.replace("\\n", "\n") + return text.replace("*", "") + + +CAPTIONS = { + "start": "one Slack message — the gate has already forced --approve", + "parked": "the proposed graph, in the message · NOTHING has run yet", + "clicked": "a human clicks Approve — the decision is bound to the plan's fingerprint", + "done": "only now does it run — and the trace records asked, answered, executed", +} + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("session", type=Path) + parser.add_argument("--out", type=Path, required=True) + parser.add_argument("--gif", type=Path, default=None) + parser.add_argument("--hold", type=float, default=3.5, help="seconds per frame") + parser.add_argument("--park-hold", type=float, default=6.0) + args = parser.parse_args() + + if shutil.which("ffmpeg") is None: + raise SystemExit("ffmpeg is required to encode the video") + + session = json.loads(args.session.read_text(encoding="utf-8")) + frames = [dict(f, text=_clean(f["text"])) for f in session["frames"]] + if not frames: + raise SystemExit("the session recorded no messages") + + renderer = Renderer() + plan = [] # (image, seconds) + # `--go` is what the person typed; `--approve` is what the gate added. The + # video opens on the request precisely so that difference is visible. + plan.append( + ( + renderer.request_frame( + session["request"], "what a person types — note: no --approve" + ), + args.hold, + ) + ) + for index, message in enumerate(frames): + parked = bool(message.get("buttons")) + last = index == len(frames) - 1 + caption = CAPTIONS["start"] if index == 0 else ( + CAPTIONS["parked"] if parked else (CAPTIONS["done"] if last else "running…") + ) + hold = args.park_hold if parked else args.hold + plan.append((renderer.frame(message, caption=caption), hold)) + if parked: + # The click: same frame, cursor drawn onto the Approve button. + plan.append( + ( + renderer.frame(message, caption=CAPTIONS["clicked"], cursor=(52, 14)), + 2.0, + ) + ) + + if session.get("audit"): + plan.append( + ( + renderer.audit_frame( + session["audit"], "the audit trail and the dashboard are one file" + ), + args.hold + 1.5, + ) + ) + + with tempfile.TemporaryDirectory() as tmp: + listing = [] + for index, (image, seconds) in enumerate(plan): + path = Path(tmp) / f"{index:04d}.png" + image.save(path) + listing.append(f"file '{path}'\nduration {seconds}") + # ffmpeg's concat demuxer ignores the final entry's duration unless the + # last file is repeated, which is why it appears twice. + listing.append(f"file '{Path(tmp) / f'{len(plan) - 1:04d}.png'}'") + script = Path(tmp) / "frames.txt" + script.write_text("\n".join(listing), encoding="utf-8") + + args.out.parent.mkdir(parents=True, exist_ok=True) + subprocess.run( + ["ffmpeg", "-y", "-f", "concat", "-safe", "0", "-i", str(script), + "-vf", f"fps={FPS},format=yuv420p", "-movflags", "+faststart", str(args.out)], + check=True, capture_output=True, + ) + if args.gif is not None: + palette = Path(tmp) / "palette.png" + subprocess.run( + ["ffmpeg", "-y", "-i", str(args.out), "-vf", + "fps=6,scale=880:-1:flags=lanczos,palettegen", + str(palette)], check=True, capture_output=True, + ) + subprocess.run( + ["ffmpeg", "-y", "-i", str(args.out), "-i", str(palette), "-lavfi", + "fps=6,scale=880:-1:flags=lanczos[x];[x][1:v]paletteuse", str(args.gif)], + check=True, capture_output=True, + ) + print(f"{len(plan)} frames -> {args.out}" + (f" and {args.gif}" if args.gif else "")) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docs/demo/scenarios/audit.py b/docs/demo/scenarios/audit.py new file mode 100644 index 0000000..a84f30c --- /dev/null +++ b/docs/demo/scenarios/audit.py @@ -0,0 +1,68 @@ +"""Demo 3 — reading a finished run back out of its trace. + +The claim this checks is the one about the file: the dashboard cannot disagree +with the audit trail because they are the same file. So every command here is +pointed at one `trace.jsonl` and nothing else — the diagram, the bill, the +reconstruction of state at a chosen step, and the comparison of two runs all +come out of it. + +The run being audited is produced first, free and scripted, so this recording +needs no key and reproduces byte for byte. +""" + +from __future__ import annotations + +import os + +WORKDIR = os.environ.get("GRAPHARC_DEMO_WORKDIR", "/tmp/grapharc-demo-audit") +TITLE = "grapharc — the run, read back" +SUBTITLE = "one JSONL file answers every question about what happened" + +TRACE = "runs/audit.jsonl" + +STEPS = [ + { + "run": ( + "grapharc plan 'investigate the checkout outage' --scripted --go " + f"--trace {TRACE} --run-id run-a" + ), + "caption": "one governed run, written to one file", + "hold": 4.0, + }, + { + # Same goal, tighter ceiling. The worst case is priced during admission, + # so this is refused *before* a node runs rather than dying partway — + # which is the difference `diff` shows at the end. + "run": ( + "grapharc plan 'investigate the checkout outage' --scripted --go " + f"--trace {TRACE} --run-id run-b --max-tokens 1700" + ), + "caption": "the same goal under a tighter budget: priced, refused, nothing ran", + "expect_exit": 1, + "hold": 5.0, + }, + { + "run": f"grapharc trace {TRACE}", + "caption": "what is in this file: every run, every phase", + "hold": 4.5, + }, + { + "run": f"grapharc replay {TRACE} run-a", + "caption": "the run reconstructed — node by node, from the events alone", + "hold": 5.0, + }, + { + "run": f"grapharc diff {TRACE} run-a run-b", + "caption": "two runs compared — three nodes against none, and why", + # Exit 1 means "they differ", as `diff(1)` has always meant. + "expect_exit": 1, + "hold": 6.0, + }, +] + + +def setup(workdir) -> None: + import shutil + + shutil.rmtree(workdir, ignore_errors=True) + workdir.mkdir(parents=True, exist_ok=True) diff --git a/docs/demo/scenarios/fix_bug.py b/docs/demo/scenarios/fix_bug.py new file mode 100644 index 0000000..09457e4 --- /dev/null +++ b/docs/demo/scenarios/fix_bug.py @@ -0,0 +1,131 @@ +"""Demo 2 — GraphARC fixing a real bug in GraphARC, under its own gate. + +The bug is genuine and was on this project's backlog: `mcp.driver.graph_status` +read a running trace with the strict reader, so the supervised agent's own +status tool raised `TraceReadError` on a half-written line — at exactly the +moment it is meant to be useful, while the run it is reporting on is still +writing. `tests/test_torn_trace_read.py` is the bug report, in the form that +can be checked. + +What the recording shows, in order: the red test; this repository's standing +rule that an agent does not edit source unattended; a plan made under that rule +(which routes around `apply_change` and says so, and is `mutating: false`); a +human amending the rule for this one piece of work; the plan made *after* that, +which now contains `apply_change`; the execution; the green test; the diff. + +The agent is Claude Code, delegated to by every stdlib agent kind because the +Claude CLI has no tool-calling wire format. It is spending a real subscription +and editing real files — in a **copy** of the repository, so a demo cannot +damage the tree it is a demo of. The fix it produces is inspected afterwards +and landed deliberately, by a person, in a separate commit. +""" + +from __future__ import annotations + +import os +import subprocess + +WORKDIR = os.environ["GRAPHARC_DEMO_WORKDIR"] # a copy of the repo; see README +TITLE = "grapharc — fixing a bug in grapharc" +SUBTITLE = "the gate decides what the agent may even propose" + +TEST = "tests/test_torn_trace_read.py" +GOAL = ( + f"{TEST} fails: graph_status in grapharc/mcp/driver.py raises TraceReadError " + "on a half-written trace line. Fix the source so the test passes." +) +#: `--trace` is not decoration here: it puts `plan.json` in `runs/`, which is +#: what makes `grapharc go runs` resolve to *this* plan. Without it the plan +#: lands under `.grapharc/runs//` and `go runs` reads "runs" as a +#: goal string — planning something unrelated and executing that instead. +COMMON = ( + "--model claude-cli --registry grapharc.stdlib:build_registry " + "--trace runs/plan.jsonl" +) +#: The planner's own words for why it proposed what it proposed. Worth showing +#: rather than paraphrasing: under the deny it says out loud that it cannot +#: reach `apply_change`, which is the demo's whole point. +#: The delegation warning is long, correct, and printed once per agent node — +#: it belongs in a log, not across three frames of a video. Dropped from the +#: recording only; nothing suppresses it in a real run. +QUIET = "grep -viE 'warning|node = '" +RATIONALE = ( + "python -c \"import json;d=json.load(open('runs/plan.json'));" + "print('kinds :', [n['kind'] for n in d['proposal']['nodes']]);" + "print('mutating :', d['mutating']);" + "print('rationale :', d['proposal']['rationale'])\"" +) + +STEPS = [ + { + "run": f"python -m pytest {TEST} -q --no-header 2>&1 | tail -6", + "caption": "the bug, as a test: graph_status crashes on a trace being written", + "hold": 5.0, + }, + { + "run": "cat policy.toml", + "caption": "this repository's standing rule for automated planners", + "hold": 6.0, + }, + { + "run": ( + f"grapharc plan '{GOAL}' {COMMON} --policy policy.toml " + f"--max-rounds 2 --run-id under-deny 2>&1 | {QUIET}" + ), + "caption": "planned under the deny — admitted, but look at how many nodes", + "timeout": 600, + "hold": 6.0, + }, + { + "run": RATIONALE, + "caption": "the planner says it: it cannot reach apply_change, so it only investigates", + "hold": 7.0, + }, + { + "run": "cat policy-approved.toml", + "caption": "a human reads the plan and amends the rule — for this work, in writing", + "hold": 5.5, + }, + { + "run": ( + f"grapharc plan '{GOAL}' {COMMON} --policy policy-approved.toml " + f"--max-rounds 2 --run-id after-approval 2>&1 | {QUIET}" + ), + "caption": "same goal, same model, different rule: five nodes now", + "timeout": 600, + "hold": 5.0, + }, + { + "run": RATIONALE, + "caption": "apply_change is in the graph, and the plan is marked mutating", + "hold": 6.0, + }, + { + "run": ( + f"grapharc go runs {COMMON} --policy policy-approved.toml " + f"--max-rounds 2 2>&1 | {QUIET} | tail -18" + ), + "caption": "now it runs — Claude Code investigates, edits, verifies, reports", + "timeout": 1500, + "hold": 7.0, + }, + { + "run": "git --no-pager diff --stat && git --no-pager diff", + "caption": "the change it made: the strict reader swapped for the tolerant one", + "hold": 8.0, + }, + { + "run": f"python -m pytest {TEST} -q --no-header 2>&1 | tail -4", + "caption": "green — the same test that opened this recording", + "hold": 6.0, + }, +] + + +def setup(workdir) -> None: + """Back to the committed baseline, so a re-record starts where this one did.""" + import shutil + + subprocess.run(["git", "-C", str(workdir), "checkout", "--", "."], check=False) + for leftover in ("runs", ".grapharc"): + shutil.rmtree(workdir / leftover, ignore_errors=True) diff --git a/docs/demo/scenarios/gate.py b/docs/demo/scenarios/gate.py new file mode 100644 index 0000000..265db8a --- /dev/null +++ b/docs/demo/scenarios/gate.py @@ -0,0 +1,54 @@ +"""Demo 1 — the admission gate refusing, then admitting. + +The project's central claim, on the cheapest possible path: no model, no key, +no spend. The scripted planner's first proposal names a kind the policy denies, +so round 1 is refused *before anything runs*; the refusal is structured +feedback, the planner replans against it, and round 2 goes through the same +checker and executes. + +Everything here is deterministic, so this recording is reproducible byte for +byte by anyone who checks out the repo. +""" + +from __future__ import annotations + +import os + +WORKDIR = os.environ.get("GRAPHARC_DEMO_WORKDIR", "/tmp/grapharc-demo-gate") +TITLE = "grapharc — the admission gate" +SUBTITLE = "a model proposes; a deterministic checker decides" + +#: Named rather than generated, so `viz` and `metrics` below can be written +#: down. A reused id in one trace is refused by the CLI, which is why `setup` +#: clears the directory. +RUN = "gate-demo" +TRACE = "runs/gate.jsonl" + +STEPS = [ + { + "run": ( + "grapharc plan 'investigate the checkout outage' " + f"--scripted --go --trace {TRACE} --run-id {RUN}" + ), + "caption": "round 1 wanted to deploy and never ran; round 2 replanned and executed", + "hold": 6.0, + }, + { + "run": f"grapharc viz {TRACE} {RUN}", + "caption": "the graph that actually ran, drawn from the trace", + "hold": 4.5, + }, + { + "run": f"grapharc metrics {TRACE} {RUN}", + "caption": "the bill, per node — read out of the same file as the diagram", + "hold": 5.0, + }, +] + + +def setup(workdir) -> None: + """A clean directory: the refusal must not be a leftover from last time.""" + import shutil + + shutil.rmtree(workdir, ignore_errors=True) + workdir.mkdir(parents=True, exist_ok=True) diff --git a/docs/media/grapharc-cli-audit.gif b/docs/media/grapharc-cli-audit.gif new file mode 100644 index 0000000..ff6f40f Binary files /dev/null and b/docs/media/grapharc-cli-audit.gif differ diff --git a/docs/media/grapharc-cli-audit.mp4 b/docs/media/grapharc-cli-audit.mp4 new file mode 100644 index 0000000..961da86 Binary files /dev/null and b/docs/media/grapharc-cli-audit.mp4 differ diff --git a/docs/media/grapharc-cli-fix-bug.gif b/docs/media/grapharc-cli-fix-bug.gif new file mode 100644 index 0000000..39e6235 Binary files /dev/null and b/docs/media/grapharc-cli-fix-bug.gif differ diff --git a/docs/media/grapharc-cli-fix-bug.mp4 b/docs/media/grapharc-cli-fix-bug.mp4 new file mode 100644 index 0000000..f6242b2 Binary files /dev/null and b/docs/media/grapharc-cli-fix-bug.mp4 differ diff --git a/docs/media/grapharc-cli-gate.gif b/docs/media/grapharc-cli-gate.gif new file mode 100644 index 0000000..9b138d2 Binary files /dev/null and b/docs/media/grapharc-cli-gate.gif differ diff --git a/docs/media/grapharc-cli-gate.mp4 b/docs/media/grapharc-cli-gate.mp4 new file mode 100644 index 0000000..92f1f72 Binary files /dev/null and b/docs/media/grapharc-cli-gate.mp4 differ diff --git a/docs/media/grapharc-slack-supervised.gif b/docs/media/grapharc-slack-supervised.gif new file mode 100644 index 0000000..35d0f67 Binary files /dev/null and b/docs/media/grapharc-slack-supervised.gif differ diff --git a/docs/media/grapharc-slack-supervised.mp4 b/docs/media/grapharc-slack-supervised.mp4 new file mode 100644 index 0000000..fed2b89 Binary files /dev/null and b/docs/media/grapharc-slack-supervised.mp4 differ diff --git a/grapharc/cli/adopt.py b/grapharc/cli/adopt.py new file mode 100644 index 0000000..cca4e57 --- /dev/null +++ b/grapharc/cli/adopt.py @@ -0,0 +1,146 @@ +"""`grapharc init --claude-code` — adopt GraphARC as a supervision layer. + +Writes the two files that turn a Claude Code checkout into a supervised one: +`.mcp.json`, registering `grapharc mcp` as a project MCP server, and +`.claude/skills/grapharc/SKILL.md`, the contract that routes multi-step and +state-changing work through plan → show → execute instead of a stream of +unsupervised edits. + +Templates are string constants, the `init` convention: nothing here reads +packaged data files, and what a test asserts about the skill is asserted +against the exact bytes a user gets. Neither file is ever overwritten — +an existing one is the operator's, and the refusal names it. + +The skill's load-bearing clause is the last one: the agent must never answer +the approval itself. The MCP surface enforces that on its own connection (no +approve tool exists), but the host agent also holds Write and Bash — the +skill states the boundary for the hands the server cannot see, and the trust +boundary remains the working directory, as it is for the Slack workspace. +""" + +from __future__ import annotations + +from pathlib import Path + +from grapharc.cli import style +from grapharc.cli.output import EXIT_OK, EXIT_UNAVAILABLE, emit + +MCP_CONFIG_FILENAME = ".mcp.json" +SKILL_PATH = Path(".claude/skills/grapharc/SKILL.md") + +MCP_CONFIG_TEMPLATE = """\ +{ + "mcpServers": { + "grapharc": { + "command": "grapharc", + "args": ["mcp"] + } + } +} +""" + +SKILL_TEMPLATE = """\ +--- +name: grapharc +description: >- + Route multi-step or state-changing repository work through GraphARC + supervision: propose the work as a governed graph, show the user the graph, + execute only what the gate admits — and what a human approved, when the plan + can change files. Use for repo-wide fixes, refactors, migrations, or any + task that would otherwise be a long stream of unsupervised edits. +--- + +# GraphARC supervision + +This project routes substantial work through GraphARC's admission gate. The +`grapharc` MCP server exposes three tools — `plan`, `show_graph`, `execute` — +and deliberately nothing that decides an approval. + +## The flow + +1. **Propose, don't act.** For multi-step or state-changing work, call + `plan` with the user's goal instead of editing files directly. The + registry, policy and model come from this project's `grapharc.toml` — if + `plan` reports no registry or model, tell the user to run `grapharc init` + and set `model` in `grapharc.toml`; do not work around it. +2. **Show the user the graph.** Render the returned `proposal` — its nodes, + edges and rationale — in your reply, and give them `watch_url` when it is + set: the live page is where the graph draws itself while it runs. +3. **Execute under the gate.** Call `execute` with the returned `run_dir`. + A plan that can change files **parks**: tell the user it is waiting and + quote `approve_command` — they answer with `grapharc approve ` + (or `--deny`) in a terminal, or from the live view. A read-only plan runs + immediately. +4. **A timeout means ask, not retry.** If `execute` comes back + `approval_timeout`, the plan is unexecuted and the call is safe to + reissue — after the user says so. Never loop on `execute` waiting for a + yes that has not been given. +5. **Report from the record.** After execution, call `show_graph` for the + metrics and the Mermaid rendering of what actually ran, and cite those + rather than your recollection. + +## What you must never do + +- Never run `grapharc approve`, in any form, for any reason. +- Never create, edit or delete `approval-request.json` or + `approval-decision.json` — those files are the human's channel, not yours. +- Never edit `plan.json` to change what was admitted or whether it counts as + mutating; an edited plan is a new proposal, and the gate treats it as one. + +The decision belongs to the user. Your job is to make the question easy to +answer: show the graph, quote the command, and wait. +""" + + +def adopt_claude_code(*, as_json: bool = False) -> int: + """Write `.mcp.json` and the skill, or refuse naming what already exists.""" + mcp_config = Path(MCP_CONFIG_FILENAME) + skill = SKILL_PATH + + existing = [str(p) for p in (mcp_config, skill) if p.exists()] + if existing: + message = ( + f"refusing to overwrite: {', '.join(existing)} — these are yours " + "once written; move one aside if you want it regenerated" + ) + if as_json: + emit({"ok": False, "command": "init", "error": message}, [], as_json=True) + else: + import sys + + print(f"error: {message}", file=sys.stderr) + return EXIT_UNAVAILABLE + + skill.parent.mkdir(parents=True, exist_ok=True) + mcp_config.write_text(MCP_CONFIG_TEMPLATE, encoding="utf-8") + skill.write_text(SKILL_TEMPLATE, encoding="utf-8") + + payload = { + "ok": True, + "command": "init", + "mcp_config": str(mcp_config), + "skill": str(skill), + } + width = style.LABEL_WIDTH + lines = [ + style.kv("wrote", str(mcp_config), width=width, tint=style.accent), + style.kv("wrote", str(skill), width=width, tint=style.accent), + "", + style.kv( + "next", + "open Claude Code here; multi-step work now routes through " + "plan -> show -> execute, and approvals stay yours", + width=width, + ), + ] + emit(payload, lines, as_json=as_json) + return EXIT_OK + + +__all__ = [ + "MCP_CONFIG_FILENAME", + "MCP_CONFIG_TEMPLATE", + "SKILL_PATH", + "SKILL_TEMPLATE", + "adopt_claude_code", +] diff --git a/grapharc/cli/approve.py b/grapharc/cli/approve.py index d2fc9f2..ce7c837 100644 --- a/grapharc/cli/approve.py +++ b/grapharc/cli/approve.py @@ -6,17 +6,61 @@ run ignores a decision naming any other plan — and exits. Exit codes follow the CLI contract: 0 the decision was delivered, 1 nothing is waiting for one, 2 the path does not lead anywhere a request could be. + +Two flags exist because an approval nobody read is not an approval (issue #46): + +- `--show` prints the parked plan and exits **without deciding**, so a + reviewer can look before answering. It is the only mode that writes nothing. +- `--fingerprint FP` binds the decision to the plan the reviewer actually + read. A parked run rewrites its request every round, so between a `--show` + and the `approve` that follows it the plan may have been replaced; quoting + the fingerprint turns that race into a refusal (exit 2, both fingerprints + named) instead of a yes to a graph nobody saw. + +The deciding path prints the plan *before* it writes the decision, so even the +one-shot form leaves a record of what was approved rather than reporting it +after the fact. """ from __future__ import annotations from pathlib import Path +from typing import Any -from grapharc.cli.output import EXIT_FAILED, EXIT_OK, emit, fail +from grapharc.cli.output import EXIT_FAILED, EXIT_OK, EXIT_UNAVAILABLE, emit, fail from grapharc.planner.approval_file import read_request, write_decision -def approve(path: str | Path, *, deny: bool = False, as_json: bool = False) -> int: +def _plan_lines(request: dict[str, Any]) -> list[str]: + """The parked proposal as text: nodes, edges, rationale, fingerprint.""" + nodes = [str(node) for node in request.get("nodes", ())] + lines = [ + f"plan : {request.get('proposal_id', '?')}", + f"fingerprint : {request.get('fingerprint', '?')}", + f"nodes : {', '.join(nodes) if nodes else '(none)'}", + ] + edges = request.get("edges") or () + for index, edge in enumerate(edges): + try: + source, target = edge[0], edge[1] + except (TypeError, IndexError, KeyError): + continue + label = "edges : " if index == 0 else " " + lines.append(f"{label}{source} -> {target}") + rationale = str(request.get("rationale") or "").strip() + if rationale: + lines.append(f"rationale : {' '.join(rationale.split())}") + return lines + + +def approve( + path: str | Path, + *, + deny: bool = False, + show: bool = False, + fingerprint: str | None = None, + as_json: bool = False, +) -> int: """Deliver a decision to the approval request in `path`'s directory. `path` may be the trace file the run printed, or the directory holding it — @@ -38,23 +82,59 @@ def approve(path: str | Path, *, deny: bool = False, as_json: bool = False) -> i code=EXIT_FAILED, ) + parked = str(request.get("fingerprint", "")) + plan_lines = _plan_lines(request) + + if show: + # The one mode that decides nothing. Printing the fingerprint is what + # makes the follow-up command bindable. + emit( + { + "ok": True, + "command": "approve", + "decision": None, + "fingerprint": parked, + "proposal_id": request.get("proposal_id", ""), + "nodes": request.get("nodes", []), + "edges": request.get("edges", []), + "rationale": request.get("rationale", ""), + }, + [ + *plan_lines, + "", + f"approve : grapharc approve {directory} --fingerprint {parked}", + f"refuse : grapharc approve {directory} --fingerprint {parked} --deny", + ], + as_json=as_json, + ) + return EXIT_OK + + if fingerprint is not None and fingerprint != parked: + # Not a stale-file problem — `file_approval` already discards a decision + # naming the wrong plan. This is the reviewer being told *why* nothing + # was written: what they read is not what is waiting. + return fail( + "that is not the plan now waiting: you reviewed " + f"{fingerprint}, the parked plan is {parked}. " + f"Re-read it with `grapharc approve {directory} --show`.", + as_json=as_json, + command="approve", + # Exit 2: by this CLI's contract that is "the request you named + # is not there", which is exactly what a superseded plan is. + code=EXIT_UNAVAILABLE, + ) + decision = "denied" if deny else "approved" - fingerprint = str(request.get("fingerprint", "")) - write_decision(directory, fingerprint=fingerprint, decision=decision) + write_decision(directory, fingerprint=parked, decision=decision) payload = { "ok": True, "command": "approve", "decision": decision, - "fingerprint": fingerprint, + "fingerprint": parked, "proposal_id": request.get("proposal_id", ""), "nodes": request.get("nodes", []), } - nodes = ", ".join(str(n) for n in request.get("nodes", [])) or "(none)" - lines = [ - f"{decision}: plan {request.get('proposal_id', '?')} ({nodes})", - f"fingerprint {fingerprint}", - ] - emit(payload, lines, as_json=as_json) + emit(payload, [*plan_lines, "", f"decision : {decision}"], as_json=as_json) return EXIT_OK diff --git a/grapharc/cli/delegate.py b/grapharc/cli/delegate.py index 9178f97..c2dcb13 100644 --- a/grapharc/cli/delegate.py +++ b/grapharc/cli/delegate.py @@ -103,6 +103,17 @@ def delegate_task( Either way the caller is responsible for having said so out loud; `AgentNode` warns at construction and marks every trace event. """ + if max_seconds is not None and max_seconds <= 0: + # An exhausted budget must not spawn anything. `subprocess.run` accepts + # a negative timeout, starts the child, and kills it on the first wait — + # so the old behaviour launched Claude Code, tore it down mid-startup, + # and reported "max_seconds (-3.2) reached", a number no caller set. + # The deadline had already passed; say that, and spend nothing saying it. + raise DelegationError( + f"no time left for a delegated run ({max_seconds:.1f}s remaining); " + "the deadline passed before it could start", + reason="deadline_exceeded", + ) binary = shutil.which("claude") if binary is None: raise DelegationError( diff --git a/grapharc/cli/generate.py b/grapharc/cli/generate.py index 68d1cbf..ef6a06f 100644 --- a/grapharc/cli/generate.py +++ b/grapharc/cli/generate.py @@ -158,7 +158,7 @@ def resolve_or_generate_policy( workdir: Path | None = None, write: bool = True, catalog: dict[str, str] | None = None, - mutating: tuple[str, ...] = (), + mutating: tuple[str, ...] | None = (), fallback: Any = None, fallback_label: str = "", registry_target: str = "", @@ -232,12 +232,22 @@ def _settled() -> tuple[Any, str, str]: return _settled() try: - toml_text = build_policy_toml( - model, - goal, - catalog if catalog is not None else stdlib.catalog_for_prompt(model), - mutating or stdlib.MUTATING_KINDS, - ) + kinds = catalog if catalog is not None else stdlib.catalog_for_prompt(model) + # `mutating` has three states and they must not be collapsed. A tuple — + # empty included — is the registry declaring which of its kinds change + # things, and is used verbatim: `()` means it says none do. + # + # `None` means the registry declared *nothing*, which is not evidence of + # safety. Substituting stdlib's `MUTATING_KINDS` (what `mutating or …` + # did) told the model that the one dangerous kind is `apply_change` — a + # name that need not exist in this registry at all — while a custom + # registry's real `deploy`/`publish` kind went unnamed and the generated + # policy allowed edges into it. That policy is then cached and governs + # every later run. Undeclared therefore means every kind is dangerous: + # the same reading `plan` takes when it writes `mutating: true` into a + # plan file it cannot vouch for. + dangerous = tuple(sorted(kinds)) if mutating is None else mutating + toml_text = build_policy_toml(model, goal, kinds, dangerous) from grapharc.policy import PolicyEngine engine = PolicyEngine.from_toml(toml_text) diff --git a/grapharc/cli/main.py b/grapharc/cli/main.py index 3aed159..9470ac4 100644 --- a/grapharc/cli/main.py +++ b/grapharc/cli/main.py @@ -371,6 +371,10 @@ def _cmd_plan(args: argparse.Namespace) -> int: def _cmd_init(args: argparse.Namespace) -> int: + if getattr(args, "claude_code", False): + from grapharc.cli.adopt import adopt_claude_code + + return adopt_claude_code(as_json=args.json) from grapharc.cli.init_cmd import init return init(as_json=args.json) @@ -399,6 +403,8 @@ def _cmd_go(args: argparse.Namespace) -> int: run_id=args.run_id, max_tokens=args.max_tokens, config_path=args.config, + approve=args.approve, + approval_timeout=args.approval_timeout, as_json=args.json, ) candidate = Path(target) @@ -415,6 +421,8 @@ def _cmd_go(args: argparse.Namespace) -> int: run_id=args.run_id, max_tokens=args.max_tokens, config_path=args.config, + approve=args.approve, + approval_timeout=args.approval_timeout, as_json=args.json, ) return plan( @@ -443,7 +451,13 @@ def _cmd_go(args: argparse.Namespace) -> int: def _cmd_approve(args: argparse.Namespace) -> int: from grapharc.cli.approve import approve - return approve(args.path, deny=args.deny, as_json=args.json) + return approve( + args.path, + deny=args.deny, + show=args.show, + fingerprint=args.fingerprint, + as_json=args.json, + ) def _cmd_models(args: argparse.Namespace) -> int: @@ -551,6 +565,23 @@ def _cmd_agent(args: argparse.Namespace) -> int: ) +def _cmd_mcp(args: argparse.Namespace) -> int: + from grapharc.cli import optional + + try: + module = optional.load( + "grapharc.mcp", + needed_for="grapharc mcp", + hint="pip install 'grapharc[mcp]'", + ) + except optional.Unavailable as exc: + return fail(str(exc), as_json=args.json, command="mcp") + root = Path(args.root).resolve() if args.root else None + if root is not None and not root.is_dir(): + return fail(f"--root: not a directory: {root}", as_json=args.json, command="mcp") + return int(module.serve_stdio(root)) + + def _cmd_serve(args: argparse.Namespace) -> int: from grapharc.cli.serve import serve @@ -969,6 +1000,15 @@ def build_parser() -> argparse.ArgumentParser: parents=[common], help="scaffold a registry, a config and a runs directory in this directory", ) + ini.add_argument( + "--claude-code", + action="store_true", + help=( + "instead of the scaffold, write .mcp.json and the Claude Code " + "skill that route this project's multi-step work through " + "grapharc mcp supervision" + ), + ) ini.set_defaults(handler=_cmd_init) st = sub.add_parser( @@ -985,6 +1025,20 @@ def build_parser() -> argparse.ArgumentParser: ) ap.add_argument("path", type=Path, help="the paused run's trace file (or its directory)") ap.add_argument("--deny", action="store_true", help="refuse the plan instead of approving it") + ap.add_argument( + "--show", + action="store_true", + help="print the parked plan and exit without deciding anything", + ) + ap.add_argument( + "--fingerprint", + default=None, + metavar="FP", + help=( + "only decide if the parked plan is this one — the fingerprint " + "`--show` printed; refuses with exit 2 if it has been replaced" + ), + ) ap.set_defaults(handler=_cmd_approve) agent = sub.add_parser( @@ -1061,6 +1115,26 @@ def build_parser() -> argparse.ArgumentParser: agent.add_argument("--system-prompt", default=None) agent.set_defaults(handler=_cmd_agent) + mcp = sub.add_parser( + "mcp", + parents=[common], + help=( + "run the MCP supervision server on stdio (plan / show_graph / " + "execute; approval stays out of band)" + ), + ) + mcp.add_argument( + "--root", + default=None, + metavar="PATH", + help=( + "directory whose grapharc.toml and registry govern every plan, and " + "which confines every run_dir a client names (default: the working " + "directory)" + ), + ) + mcp.set_defaults(handler=_cmd_mcp) + serve = sub.add_parser("serve", parents=[common], help="run the HTTP API") serve.add_argument("--host", default="127.0.0.1") serve.add_argument("--port", type=int, default=8000) diff --git a/grapharc/cli/plan.py b/grapharc/cli/plan.py index a70ead4..dc86169 100644 --- a/grapharc/cli/plan.py +++ b/grapharc/cli/plan.py @@ -178,12 +178,17 @@ class PlanSetupError(Exception): """Raised before anything runs, so a bad flag never half-executes a plan.""" -def _write_plan_file(run_dir: Path, *, goal, registry_target, model_spec, result) -> None: +def _write_plan_file( + run_dir: Path, *, goal, registry_target, model_spec, result, mutating: bool = True +) -> None: """Persist the admitted-but-unexecuted plan next to its trace. What `grapharc go` reads. The proposal is stored whole and re-judged by admission at execution time — a hand-edited plan.json is a new proposal, - not a pre-approved one. + not a pre-approved one. `mutating` records the verdict the payload + carries, so a later driver deciding whether execution needs an approval + park does not have to re-import the registry; a plan file without the + field reads as mutating, never as safe. """ import json from datetime import UTC, datetime @@ -203,6 +208,7 @@ def _write_plan_file(run_dir: Path, *, goal, registry_target, model_spec, result "model": model_spec, "fingerprint": admitted.proposal.fingerprint(), "proposal": admitted.proposal.model_dump(mode="json"), + "mutating": mutating, "planned_at": datetime.now(UTC).isoformat(), }, indent=2, @@ -228,6 +234,52 @@ def find_unexecuted_plan(runs_root: Path | None = None) -> Path | None: return None +def _approval_gate( + trace_path: Path, + *, + run_id: str | None, + approval_timeout: float | None, + as_json: bool, +) -> Any: + """The file-handshake gate `--approve` configures, announce included. + + Shared by `plan` and `go` so the two commands cannot drift on how a + parked run asks its question. The announce is printed and flushed before + the run parks — a terminal user (or a log tailer) must learn how to + answer without waiting for the exit — and silent in JSON mode, where + stdout carries exactly one document; a JSON caller detects the park by + watching for `approval-request.json` in the run directory. + """ + import sys + + from grapharc.planner.approval_file import DEFAULT_TIMEOUT_SECONDS, file_approval + + watch_shown = False + + def _announce(message: str) -> None: + if as_json: + return + # The live link first, once: the page is where the parked proposal + # is drawn, and it should be open while the human decides. + nonlocal watch_shown + if not watch_shown: + watch_shown = True + url = watch_url(trace_path, run_id=run_id) + if url: + print( + style.kv("watch", url, width=style.LABEL_WIDTH, tint=style.accent), + flush=True, + file=sys.stdout, + ) + print(message, flush=True, file=sys.stdout) + + return file_approval( + trace_path.parent, + timeout_seconds=approval_timeout or DEFAULT_TIMEOUT_SECONDS, + announce=_announce, + ) + + def execute_plan( target: str | None, *, @@ -239,6 +291,8 @@ def execute_plan( run_id: str | None = None, max_tokens: int | None = None, config_path: Path | None = None, + approve: bool = False, + approval_timeout: float | None = None, as_json: bool = False, ) -> int: """`grapharc go []` — execute a plan `grapharc plan` saved. @@ -246,8 +300,11 @@ def execute_plan( The stored proposal is replayed through the full governed loop — a scripted planner whose one reply *is* the plan — so admission judges it again on the way in: what runs is what the gate admits now, not what a - file claims was admitted before. Executing is the human approval; there - is no second gate. + file claims was admitted before. Bare, executing is the human approval; + with `--approve` the run parks on the file handshake first and executes + only an answered yes — the gate an external driver relies on when the + plan can change things. These flags used to be accepted here and + silently dropped, which was worse than refusing them. """ import json @@ -292,6 +349,7 @@ def execute_plan( try: settings = load_settings(config_path) model_spec = settings.resolve("model", model_spec, record.get("model")) + policy_path = settings.resolve_path("policy", policy_path) tenant = settings.resolve("tenant", tenant, "default") max_tokens = settings.resolve("max_tokens", max_tokens, 100_000) model_args = _parse_model_args(model_arg_pairs) @@ -344,7 +402,16 @@ def execute_plan( registry=bundle.registry, state_schema=schema, writes=bundle.writes, - approval=None, + approval=( + _approval_gate( + trace_path, + run_id=run_id, + approval_timeout=approval_timeout, + as_json=as_json, + ) + if approve + else None + ), ) initial = schema(goal=goal) if "goal" in schema.model_fields else schema() result = loop.run(goal, initial, run_id=run_id) @@ -471,9 +538,11 @@ class RegistryBundle: #: that do not exist and permit ones that do. default_policy: Any = None #: Kinds the module considers dangerous, from its `MUTATING_KINDS`. Handed to - #: the policy generator so it knows what to deny; empty means it denies - #: nothing, which is why a module that can change things should say so. - mutating: tuple[str, ...] = () + #: the policy generator so it knows what to deny. `None` means the module + #: said nothing at all — which readers must treat as "assume mutating", + #: never as "declared safe" — while an explicit empty tuple is a + #: declaration that nothing here can change anything. + mutating: tuple[str, ...] | None = None #: The module's own `build_loop`, when it ships one. This is how a registry #: owns its goal check and observer instead of inheriting the incident #: demo's (`len(notes) >= 3`) — a registry whose state never accumulates @@ -526,12 +595,13 @@ def resolve_registry( registry(model, **kwargs) if _accepts_an_argument(registry) else registry(**kwargs) ) default_policy = getattr(module, "default_edge_policy", None) + declared_mutating = getattr(module, "MUTATING_KINDS", None) return RegistryBundle( registry=registry, state_schema=getattr(module, "STATE_SCHEMA", None), writes=getattr(module, "WRITES", None), default_policy=default_policy() if callable(default_policy) else default_policy, - mutating=tuple(getattr(module, "MUTATING_KINDS", ())), + mutating=None if declared_mutating is None else tuple(declared_mutating), build_loop=getattr(module, "build_loop", None), ) @@ -810,40 +880,13 @@ def plan( schema = state_schema or IncidentState trace = TraceRecorder(trace_path) - approval = None - if approve: - import sys - - from grapharc.planner.approval_file import DEFAULT_TIMEOUT_SECONDS, file_approval - - watch_shown = False - - def _announce(message: str) -> None: - # Printed *and flushed* before the run parks: a terminal user (or a - # log tailer) must learn how to answer without waiting for the exit. - # Silent in JSON mode: stdout there carries exactly one document, and - # a notice printed ahead of it makes the whole output unparseable. - if as_json: - return - # The live link first, once: the page is where the parked proposal - # is drawn, and it should be open while the human decides. - nonlocal watch_shown - if not watch_shown: - watch_shown = True - url = watch_url(trace_path, run_id=run_id) - if url: - print( - style.kv("watch", url, width=style.LABEL_WIDTH, tint=style.accent), - flush=True, - file=sys.stdout, - ) - print(message, flush=True, file=sys.stdout) - - approval = file_approval( - trace_path.parent, - timeout_seconds=approval_timeout or DEFAULT_TIMEOUT_SECONDS, - announce=_announce, + approval = ( + _approval_gate( + trace_path, run_id=run_id, approval_timeout=approval_timeout, as_json=as_json ) + if approve + else None + ) # The registry module's own loop builder wins; the incident demo's is the # fallback that keeps the default path byte-identical. build_loop = bundle.build_loop or incident_build_loop @@ -870,6 +913,33 @@ def _announce(message: str) -> None: loop.plan_only = command == "plan" and not go_after result = loop.run(goal, initial, run_id=run_id) + # The admitted shape as data, computed before the plan file so the file + # can carry the same `mutating` verdict the payload does. An external + # driver — the MCP server first among them — must not have to re-read + # plan.json for the shape, nor re-import the registry for the verdict. + admitted_record = next( + ( + record + for record in reversed(result.rounds) + if record.proposal is not None + and record.admission is not None + and record.admission.admitted + ), + None, + ) + if bundle.mutating is None: + # The registry module never said which kinds mutate. A reader must + # treat that as "assume mutating", so the payload says so rather than + # implying a safety nobody declared. + is_mutating = True + else: + admitted_kinds = ( + {node.kind for node in admitted_record.proposal.nodes} + if admitted_record + else set() + ) + is_mutating = bool(admitted_kinds & set(bundle.mutating)) + if result.stop is LoopStop.PLANNED: _write_plan_file( trace_path.parent, @@ -877,6 +947,7 @@ def _announce(message: str) -> None: registry_target=registry_target, model_spec=model_spec, result=result, + mutating=is_mutating, ) rounds = [ @@ -899,13 +970,25 @@ def _announce(message: str) -> None: "policy": policy_description, "policy_source": policy_source, "trace": str(trace_path), + "run_dir": str(trace_path.parent), **settings.provenance(policy_source=policy_source), "stop": result.stop.value, "detail": result.detail, "rounds": rounds, "rejections": [r.code for r in result.rejections()], + "mutating": is_mutating, "state": result.state.model_dump() if hasattr(result.state, "model_dump") else result.state, } + if admitted_record is not None: + payload["fingerprint"] = admitted_record.admission.fingerprint + payload["proposal"] = { + "nodes": [ + node.model_dump(mode="json", exclude={"subgraph"}) + for node in admitted_record.proposal.nodes + ], + "edges": [edge.model_dump(mode="json") for edge in admitted_record.proposal.edges], + "rationale": admitted_record.proposal.rationale, + } # The plain text of every line below is exactly what it was before colour # existed — the labels are still ten characters wide and the round rows still diff --git a/grapharc/harness/agent.py b/grapharc/harness/agent.py index 3afd123..7fe08cd 100644 --- a/grapharc/harness/agent.py +++ b/grapharc/harness/agent.py @@ -598,6 +598,12 @@ def _run_delegated(self, prompt: str, ctx: RunContext) -> AgentResult: from grapharc.cli.delegate import DelegationError, delegate_task remaining = ctx.meter.remaining_seconds() if ctx.meter else None + if remaining is not None and remaining <= 0: + # `remaining_seconds()` is `max_seconds - elapsed` and goes negative + # once the budget is spent. Delegating with that would spawn Claude + # Code only to kill it, so refuse here — before the trace claims a + # delegated phase started — and let the caller see a budget failure. + remaining = 0.0 step = 1 if self.trace is not None: self.trace.event( @@ -608,12 +614,19 @@ def _run_delegated(self, prompt: str, ctx: RunContext) -> AgentResult: "governed_by": "Claude Code, not this graph's policy"}, ) try: - workspace = getattr(self.harness.executor, "workspace", None) + # The harness first, its executor second. Only the sandboxing + # executors carry a workspace of their own, so asking the executor + # alone made every `LocalExecutor` harness undelegatable — which is + # the whole `stdlib` registry driven by the Claude CLI, the one + # combination whose documented behaviour *is* delegation. + workspace = getattr(self.harness, "workspace", None) or getattr( + self.harness.executor, "workspace", None + ) if workspace is None: raise DelegationError( - "the delegated executor needs a workspace directory, and this " - f"harness's executor ({type(self.harness.executor).__name__}) " - "does not expose one", + "a delegated run needs a workspace directory to run in, and " + f"this harness (executor: {type(self.harness.executor).__name__}) " + "does not name one; pass `workspace=` when building it", reason="no_workspace", ) run = delegate_task( diff --git a/grapharc/harness/core.py b/grapharc/harness/core.py index 4976b75..be3266a 100644 --- a/grapharc/harness/core.py +++ b/grapharc/harness/core.py @@ -36,6 +36,17 @@ def __init__( self.registry = registry self.policy = policy self.executor = executor or SandboxedExecutor(workspace) + # The directory this harness works in, kept on the harness itself. + # + # It used to exist only to build the default `SandboxedExecutor`, so + # `Harness(..., executor=LocalExecutor(), workspace=str(ws))` — which + # three call sites in this repo write — silently discarded `ws`. That + # was invisible until something asked where the harness was working: + # a delegated `AgentNode` reads the workspace to give Claude Code a + # cwd, found only `SandboxedExecutor` had one, and refused to run at + # all under `LocalExecutor`. Recording it here makes the argument mean + # what it reads as, whichever executor is in play. + self.workspace = workspace or getattr(self.executor, "workspace", None) self.pre_hooks = pre_hooks self.post_hooks = post_hooks self.approval = approval diff --git a/grapharc/mcp/__init__.py b/grapharc/mcp/__init__.py new file mode 100644 index 0000000..036995f --- /dev/null +++ b/grapharc/mcp/__init__.py @@ -0,0 +1,15 @@ +"""GraphARC as an MCP server — supervision for agents that already exist. + +Ships behind the `mcp` extra (`pip install 'grapharc[mcp]'`). The package +imports nothing from `grapharc.server`, which needs the `server` extra: the +read-side primitives here are the fastapi-free ones (`observe`, the approval +file handshake, `plan.json`). + +`grapharc mcp` is the entry point; `build_server` is the library surface a +test drives directly. +""" + +from grapharc.mcp.driver import DriverError +from grapharc.mcp.server import FORBIDDEN_TOOL_WORDS, build_server, serve_stdio + +__all__ = ["FORBIDDEN_TOOL_WORDS", "DriverError", "build_server", "serve_stdio"] diff --git a/grapharc/mcp/driver.py b/grapharc/mcp/driver.py new file mode 100644 index 0000000..8bd8f97 --- /dev/null +++ b/grapharc/mcp/driver.py @@ -0,0 +1,207 @@ +"""The MCP server's hands: build argv, spawn the CLI, read the record back. + +Everything here drives `grapharc` **as a subprocess**, never in process, for +three reasons the server module repeats: the stdio transport owns stdout and +`emit()` prints there; the `--json` payloads and exit codes are the tested +interface, so this stays a thin shim over a contract that already has a +suite; and an async subprocess keeps the event loop free, so `show_graph` +answers while an `execute` is parked. + +Argv is **built, never parsed**: no tool accepts a registry, policy or model +argument, because those resolve from the operator's `grapharc.toml` in the +root directory — the requester's call must not be able to widen what the +operator configured. Run directories are confined to the root the server was +started in, the same `is_relative_to` posture as the Slack gate's paths. + +Only fastapi-free modules are imported here (`observe.trace`, +`observe.metrics`, `planner.approval_file`): this package ships behind the +`mcp` extra and must not drag the `server` extra in with it. +""" + +from __future__ import annotations + +import asyncio +import json +import sys +from pathlib import Path +from typing import Any + +PLAN_FILENAME = "plan.json" + +#: A parked `execute` lives inside one MCP call, and hosts time tool calls +#: out. The default stays under typical host limits; a timeout leaves the +#: plan unexecuted and the call safe to reissue. +DEFAULT_APPROVAL_TIMEOUT = 240.0 + + +class DriverError(Exception): + """A tool call that cannot proceed, with the reason as the message.""" + + +def confine_run_dir(root: Path, run_dir: str) -> Path: + """Resolve `run_dir` and refuse anything outside the server's root. + + The server reads plan.json and trace.jsonl from whatever directory a + client names; without this, a client could point it at any readable + path on the machine. + """ + resolved = (root / run_dir).resolve() if not Path(run_dir).is_absolute() else Path( + run_dir + ).resolve() + if not resolved.is_relative_to(root.resolve()): + raise DriverError( + f"run_dir {run_dir!r} is outside the directory this server was " + f"started in ({root}); a run directory is always under it" + ) + return resolved + + +async def run_cli( + argv: list[str], *, cwd: Path, timeout: float | None = None +) -> tuple[int, str, str]: + """One `grapharc` subprocess, the Slack runner's spawn pattern made async.""" + process = await asyncio.create_subprocess_exec( + sys.executable, + "-m", + "grapharc.cli.main", + *argv, + cwd=cwd, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + try: + out, err = await asyncio.wait_for(process.communicate(), timeout=timeout) + except TimeoutError: + process.kill() + await process.wait() + raise DriverError( + f"grapharc {argv[0]} did not finish within {timeout}s and was stopped" + ) from None + return process.returncode or 0, out.decode(errors="replace"), err.decode(errors="replace") + + +def parse_document(stdout: str, *, command: str) -> dict[str, Any]: + """The one JSON document a `--json` command prints, or a named refusal.""" + try: + document = json.loads(stdout) + except (json.JSONDecodeError, ValueError) as exc: + raise DriverError( + f"grapharc {command} --json did not print a readable document: " + f"{stdout.strip()[-300:] or '(empty)'}" + ) from exc + if not isinstance(document, dict): + raise DriverError(f"grapharc {command} --json printed {type(document).__name__}") + return document + + +def read_plan_record(run_dir: Path) -> dict[str, Any]: + plan_file = run_dir / PLAN_FILENAME + if not plan_file.is_file(): + raise DriverError( + f"no {PLAN_FILENAME} in {run_dir} — call plan first; execute and " + "show_graph work on a directory plan created" + ) + try: + return json.loads(plan_file.read_text(encoding="utf-8")) + except (OSError, ValueError) as exc: + raise DriverError(f"unreadable {plan_file}: {exc}") from exc + + +def plan_is_mutating(record: dict[str, Any]) -> bool: + """The plan's own verdict — absent reads as mutating, never as safe.""" + value = record.get("mutating") + return True if not isinstance(value, bool) else value + + +def graph_status(run_dir: Path) -> dict[str, Any]: + """Compose the read-only view of one run directory. + + plan.json for the admitted shape, the trace for what has happened since, + the approval-request file for whether a human is being asked right now. + Rendered summaries only — the raw `state_delta` a node wrote is served by + nothing here, the same posture as the live view. + + `TailRecorder`, not `TraceRecorder`, for the same reason the live view and + the Slack tailer use it: this reads a file something else is still writing. + An agent polls `show_graph` precisely *while* its run is in progress, so a + read that lands mid-append is the normal case, not the edge case — and the + strict reader raises `TraceReadError` on the half-written last line, which + came out of the MCP server as a crash instead of an answer. + """ + from grapharc.observe.metrics import summarize, to_mermaid + from grapharc.observe.trace import TailRecorder + from grapharc.planner.approval_file import read_request + + record = read_plan_record(run_dir) + view: dict[str, Any] = { + "run_dir": str(run_dir), + "goal": record.get("goal", ""), + "fingerprint": record.get("fingerprint", ""), + "mutating": plan_is_mutating(record), + "proposal": { + "nodes": (record.get("proposal") or {}).get("nodes", []), + "edges": (record.get("proposal") or {}).get("edges", []), + "rationale": (record.get("proposal") or {}).get("rationale", ""), + }, + "executed_run_id": record.get("executed_run_id"), + } + + request = read_request(run_dir) + view["awaiting_approval"] = request is not None + view["approve_command"] = ( + f"grapharc approve {run_dir}" if request is not None else None + ) + + trace_path = run_dir / "trace.jsonl" + view["status"] = "planned" + if request is not None: + view["status"] = "awaiting_approval" + if record.get("executed_run_id") and trace_path.is_file(): + recorder = TailRecorder(trace_path) + run_id = str(record["executed_run_id"]) + metrics = summarize(recorder, run_id) + if metrics is not None: + view["status"] = "done" + view["metrics"] = metrics.model_dump(mode="json") + view["mermaid"] = to_mermaid(recorder, run_id) + return view + + +def build_plan_argv(goal: str, *, scripted: bool, max_rounds: int | None) -> list[str]: + argv = ["plan", goal, "--json"] + if scripted: + argv.append("--scripted") + if max_rounds is not None: + argv += ["--max-rounds", str(int(max_rounds))] + return argv + + +def build_execute_argv( + run_dir: Path, *, mutating: bool, approval_timeout: float +) -> list[str]: + """`go `, parked on the file handshake exactly when the plan mutates. + + The tiering is the maintainer's decision made mechanical: an all-read-only + plan executes on the host's own prompt; anything that can change files + parks for an out-of-band human. The verdict comes from the plan record, + where a missing field already read as mutating. + """ + argv = ["go", str(run_dir), "--json"] + if mutating: + argv += ["--approve", "--approval-timeout", str(float(approval_timeout))] + return argv + + +__all__ = [ + "DEFAULT_APPROVAL_TIMEOUT", + "PLAN_FILENAME", + "DriverError", + "build_execute_argv", + "build_plan_argv", + "confine_run_dir", + "graph_status", + "parse_document", + "plan_is_mutating", + "read_plan_record", + "run_cli", +] diff --git a/grapharc/mcp/server.py b/grapharc/mcp/server.py new file mode 100644 index 0000000..acb9bab --- /dev/null +++ b/grapharc/mcp/server.py @@ -0,0 +1,152 @@ +"""`grapharc mcp` — the supervision surface an external agent plugs into. + +Three tools, and deliberately not a fourth: + +- `plan(goal, ...)` proposes a graph for the goal through the governed loop + and returns the admitted shape as data — nodes, edges, rationale, + fingerprint, and whether executing it can change anything. +- `show_graph(run_dir)` is the read-only view: the proposal, whether a human + is being asked right now, and — after execution — the metrics and the + Mermaid rendering of what actually ran. +- `execute(run_dir, ...)` re-admits and runs the saved plan. A plan + containing a mutating kind parks on the file handshake until a human + answers `grapharc approve ` out of band; an all-read-only plan + runs on the host's own prompt. + +**There is no approve, deny, or decide tool, and there never will be.** The +gate is between the agent and the operator; a client that could call +`approve()` would be approving its own proposal, which is not approval. The +supervised agent may request (execute parks) and may check (show_graph says +`awaiting_approval`); the decision belongs to a human with a terminal or the +live view, outside this connection. + +The server drives the `grapharc` CLI as subprocesses and never prints to +stdout — the stdio transport owns it. Diagnostics go to stderr. +""" + +from __future__ import annotations + +import sys +from pathlib import Path +from typing import Any + +from mcp.server.fastmcp import FastMCP + +from grapharc.mcp import driver +from grapharc.mcp.driver import DEFAULT_APPROVAL_TIMEOUT, DriverError + +#: Verbs this surface refuses to grow. Checked by a gate test against the +#: registered tool names, so the refusal is a property, not a comment. +FORBIDDEN_TOOL_WORDS = ("approve", "deny", "decide") + + +def build_server(root: Path | None = None) -> FastMCP: + """The FastMCP server, rooted where it was started. + + `root` confines every `run_dir` a client names and is the working + directory of every spawned CLI — which is what makes the operator's + `grapharc.toml` there, and nothing the client says, decide the registry, + the policy and the model. + """ + base = (root or Path.cwd()).resolve() + server = FastMCP( + "grapharc", + instructions=( + "GraphARC supervision: plan proposes a governed graph for a goal " + "and returns the admitted shape; show_graph reads a run " + "directory back; execute runs a saved plan, parking for " + "out-of-band human approval when the plan can change files. " + "There is no approve tool: the decision belongs to the human, " + "via `grapharc approve ` or the live view." + ), + ) + + @server.tool() + async def plan( + goal: str, scripted: bool = False, max_rounds: int | None = None + ) -> dict[str, Any]: + """Propose a governed graph for `goal` and return the admitted shape. + + The registry, policy and model come from the operator's grapharc.toml + in the server's directory — they are not parameters, on purpose. + `scripted=True` rehearses with the registry's canned planner: free, + deterministic, no model call. + """ + code, out, err = await driver.run_cli( + driver.build_plan_argv(goal, scripted=scripted, max_rounds=max_rounds), + cwd=base, + ) + document = driver.parse_document(out, command="plan") + if code != 0 and not document.get("ok", False): + # The CLI's failure document is the answer — setup problems arrive + # with the CLI's own instructive text rather than a bare error. + return document + return { + "run_dir": document.get("run_dir"), + "goal": document.get("goal"), + "stop": document.get("stop"), + "proposal": document.get("proposal"), + "fingerprint": document.get("fingerprint"), + "mutating": document.get("mutating", True), + "kinds": document.get("kinds", []), + "policy": document.get("policy"), + "rejections": document.get("rejections", []), + "rounds": document.get("rounds", []), + "watch_url": document.get("watch_url"), + "plan_file": document.get("plan_file"), + "approve_command": ( + f"grapharc approve {document.get('run_dir')}" + if document.get("mutating", True) and document.get("run_dir") + else None + ), + } + + @server.tool() + async def show_graph(run_dir: str) -> dict[str, Any]: + """Read one run directory back: the admitted proposal, whether a human + is being asked right now, and the record of what ran. Read-only.""" + resolved = driver.confine_run_dir(base, run_dir) + return driver.graph_status(resolved) + + @server.tool() + async def execute( + run_dir: str, approval_timeout: float = DEFAULT_APPROVAL_TIMEOUT + ) -> dict[str, Any]: + """Execute the plan saved in `run_dir`, re-admitted through the gate. + + A plan with a mutating kind parks until a human answers + `grapharc approve ` out of band; tell the user, do not + answer it yourself. A timeout leaves the plan unexecuted and this + call safe to reissue. + """ + resolved = driver.confine_run_dir(base, run_dir) + record = driver.read_plan_record(resolved) + mutating = driver.plan_is_mutating(record) + code, out, err = await driver.run_cli( + driver.build_execute_argv( + resolved, mutating=mutating, approval_timeout=approval_timeout + ), + cwd=base, + # The subprocess bounds its own park via --approval-timeout; this + # outer bound only catches a wedged process, generously. + timeout=approval_timeout + 120.0 if mutating else None, + ) + document = driver.parse_document(out, command="go") + document["mutating"] = mutating + if mutating and not document.get("executed", False): + document["approve_command"] = f"grapharc approve {resolved}" + return document + + return server + + +def serve_stdio(root: Path | None = None) -> int: + """Run the server on stdio until the client hangs up. The CLI entry.""" + try: + build_server(root).run(transport="stdio") + except KeyboardInterrupt: # a Ctrl-C is a clean goodbye, not a stack trace + print("grapharc mcp: stopped", file=sys.stderr) + return 0 + + +__all__ = ["FORBIDDEN_TOOL_WORDS", "DriverError", "build_server", "serve_stdio"] diff --git a/grapharc/observe/cost.py b/grapharc/observe/cost.py index 214f8c9..690a28d 100644 --- a/grapharc/observe/cost.py +++ b/grapharc/observe/cost.py @@ -15,20 +15,20 @@ never added into `recorded_cost_usd`, so nobody can mistake an estimate for an invoice. -Tokens are counted from the `end` events of node executions, which is exactly -the rule `metrics.summarize` uses. That is not a coincidence and not a +Tokens are counted from the terminal events of node executions — `end` and +`error` alike, since the kernel stamps both with the node's spend — which is +exactly the rule `metrics.summarize` uses. That is not a coincidence and not a duplicate implementation: `RunCost.tokens` is asserted equal to `RunMetrics.tokens` by the test suite, because a cost report and an audit trail that disagree are worse than either alone. Two limits, stated rather than smoothed over: -- A node that raises never emits an `end` event, and the kernel's `error` event - carries no token count — so tokens spent inside a node that then failed are - invisible at node level. Where the node was an `AgentNode`, its per-call - `"model"` sub-events still hold them, and they are reported as - `tokens_before_error` rather than folded into the total that must match - `metrics`. +- A trace whose `error` events carry no token count (an older or hand-built + producer) leaves tokens spent inside a failed node invisible at node level. + Where the node was an `AgentNode`, its per-call `"model"` sub-events still + hold them, and they are reported as `tokens_before_error` rather than folded + into the total that must match `metrics`. - There is no tenant on a trace event, so tenant attribution is not offered here. Run, thread (session) and node are what the format supports today. """ @@ -142,10 +142,10 @@ class RunCost(BaseModel): unpriced_tokens: int = 0 per_node: list[NodeCost] = Field(default_factory=list) model_calls: list[ModelCallCost] = Field(default_factory=list) - # Tokens reported by sub-steps of nodes that ended in `error`. Held apart - # from `tokens` because the node-level total must keep matching - # `metrics.summarize`, and because "spend that bought nothing" is the - # number an incident review actually wants. + # Tokens reported by sub-steps of failed nodes whose `error` event carried + # no token count of its own. Held apart from `tokens` because the + # node-level total must keep matching `metrics.summarize`, and because + # "spend that bought nothing" is the number an incident review wants. tokens_before_error: int = 0 @property @@ -291,11 +291,34 @@ def _price_run(run: ReplayedRun, rates: RateCard | None) -> RunCost: ) if not execution.ok: - # No `end` event was written, so the node-level token count for this - # execution does not exist; whatever sub-steps it emitted before - # failing are the only record of what it spent. + # The kernel stamps a failed execution's terminal `error` event + # with what the node spent, exactly as it stamps `end`, and + # `metrics.summarize` counts it. Skipping it here made the cost + # report disagree with the audit trail by precisely the spend a + # budget stop was about. entry.errors += 1 - tokens_before_error += execution.sub_tokens + entry.tokens += execution.tokens + entry.duration_ms += execution.duration_ms or 0.0 + if execution.tokens: + estimate, unpriced_here, models = _estimate(execution, card) + entry.models = list(dict.fromkeys([*entry.models, *models])) + if execution.cost_usd is not None: + entry.recorded_cost_usd = _combine( + entry.recorded_cost_usd, execution.cost_usd + ) + recorded_total.append(execution.cost_usd) + else: + if estimate is not None: + entry.estimated_cost_usd = _combine( + entry.estimated_cost_usd, estimate + ) + estimated_total.append(estimate) + entry.unpriced_tokens += unpriced_here + unpriced += unpriced_here + # A trace whose `error` event carries no token count (an older or + # hand-built producer) still holds the sub-steps' record; that + # spend is not in the total and is reported apart. + tokens_before_error += max(0, execution.sub_tokens - execution.tokens) continue entry.executions += 1 @@ -329,7 +352,19 @@ def _price_run(run: ReplayedRun, rates: RateCard | None) -> RunCost: for sub in run.orphan_sub_events: orphan_tokens += sub.tokens or 0 orphan_ms += sub.duration_ms or 0.0 + # Cost is attributed from any orphan that carries one; only a `model` + # event becomes a `ModelCallCost`, because only that is a model call. + # Skipping the rest wholesale billed a delegated run at zero: an + # `--executor claude-cli` phase reports its spend on the `stop` event + # (`AgentNode._run_delegated`), which is not phase `model`, and with no + # enclosing graph every event it writes is an orphan. The bill came out + # $0.00 with no `unpriced_tokens` to say it was incomplete — while + # `ReplayedRun.recorded_cost_usd`, which counts orphans by cost rather + # than by phase, reported the real figure. Two readers, one trace, two + # answers. This is the rule that reconciles them. if sub.phase != "model": + if sub.cost_usd is not None: + recorded_total.append(sub.cost_usd) continue sub_estimate = ( None if sub.cost_usd is not None else card.price(sub.tokens or 0, sub.model) diff --git a/grapharc/observe/viewmodel.py b/grapharc/observe/viewmodel.py index 83bf951..bba6aa5 100644 --- a/grapharc/observe/viewmodel.py +++ b/grapharc/observe/viewmodel.py @@ -194,6 +194,7 @@ def _node_measures(node: NodeView, graph_events: list[TraceEvent], name: str) -> """ costs: list[float] = [] prefix = f"{name}:" + live = 0 for event in graph_events: if event.node == name and event.phase in ("end", "error"): node.tokens += event.tokens or 0 @@ -202,11 +203,16 @@ def _node_measures(node: NodeView, graph_events: list[TraceEvent], name: str) -> if event.cost_usd is not None: costs.append(event.cost_usd) node.executions += 1 + # Sub-steps before this terminal are a breakdown of the total just + # counted; carrying them forward would show a closed execution's + # spend twice on a node that runs again. + live = 0 elif node.status == "running" and ( event.node == name or event.node.startswith(prefix) ): if event.phase not in ("start",) and event.tokens: - node.live_tokens += event.tokens + live += event.tokens + node.live_tokens += live if costs: node.cost_usd = round(sum(costs), 6) if node.duration_ms is not None: diff --git a/grapharc/planner/approval_file.py b/grapharc/planner/approval_file.py index 6395ec9..9c8b945 100644 --- a/grapharc/planner/approval_file.py +++ b/grapharc/planner/approval_file.py @@ -60,6 +60,36 @@ def read_request(directory: Path) -> dict[str, Any] | None: return loaded if isinstance(loaded, dict) else None +def describe_request(request: dict[str, Any], directory: Path) -> str: + """The parked question as something a human can act on without prior knowledge. + + A run that parks and then says nothing but "waiting" depends on the reader + already knowing that `grapharc approve` exists, that `--deny` is the other + half, and that the wait expires. That is tribal knowledge, and the failure + mode is a run timing out unapproved while its operator watches a cursor — + which then reads as a planner failure rather than an unanswered question + (issue #52). So the block names the plan, its fingerprint, the deadline, + and both commands that end the wait. + + Deliberately plain text: this module is the headless half of the handshake + and must stay importable without the CLI's styling. The caller decorates. + """ + nodes = [str(node) for node in request.get("nodes", ())] + shape = f"{len(nodes)} node{'s' if len(nodes) != 1 else ''}" + if nodes: + shape += ": " + ", ".join(nodes) + lines = [ + f"awaiting approval — {shape}", + f" fingerprint : {request.get('fingerprint', '?')}", + f" waiting : up to {float(request.get('timeout_seconds') or 0):.0f}s," + " then the round counts as unapproved", + f" approve : grapharc approve {directory}", + f" refuse : grapharc approve {directory} --deny", + f" look first : grapharc approve {directory} --show", + ] + return "\n".join(lines) + + def write_decision(directory: Path, *, fingerprint: str, decision: str) -> Path: path = directory / DECISION_FILENAME _write_atomically(path, {"fingerprint": fingerprint, "decision": decision}) @@ -106,10 +136,7 @@ def gate( write_request(directory, request) if announce is not None: try: - announce( - f"waiting for approval (up to {timeout_seconds:.0f}s) — " - f"answer with: grapharc approve {directory}" - ) + announce(describe_request(request, directory)) except Exception: # noqa: BLE001 — narration must not decide pass while time.monotonic() < deadline: @@ -157,6 +184,7 @@ def _discard_own_stale_decision(path: Path, fingerprint: str) -> None: "DECISION_FILENAME", "DEFAULT_TIMEOUT_SECONDS", "REQUEST_FILENAME", + "describe_request", "file_approval", "read_request", "write_decision", diff --git a/grapharc/planner/loop.py b/grapharc/planner/loop.py index c7a8516..192599d 100644 --- a/grapharc/planner/loop.py +++ b/grapharc/planner/loop.py @@ -718,21 +718,11 @@ def _execute( }, ) - if self.plan_only: - # The graph is admitted, materialisable, and on the trace — which - # is exactly what "planned" means. Executing it is `grapharc go`'s - # job, in its own process, whenever the operator says. The - # approval gate is skipped on purpose: a plan that executes - # nothing has nothing to approve; the act of running `go` *is* - # the approval. - return _Execution( - state=state, - executed=False, - hard_stop=LoopStop.PLANNED, - execution_error="awaiting `grapharc go`", - ) - if self.approval is not None: + # Before the plan_only return, deliberately: `plan --approve` used + # to return PLANNED first, which made the flag inert — a gate that + # is configured has been asked for, and a parked plan is a real + # question whether or not this process will also execute it. parked = time.monotonic() decision = self._request_approval( proposal, verdict, ctx, round_number, goal=goal @@ -763,6 +753,19 @@ def _execute( hard_stop=stop, ) + if self.plan_only: + # The graph is admitted, materialisable, on the trace — and, when + # a gate was configured, approved above before being called a + # plan. Executing it is `grapharc go`'s job, in its own process, + # whenever the operator says; with no gate configured, the act of + # running `go` is the approval. + return _Execution( + state=state, + executed=False, + hard_stop=LoopStop.PLANNED, + execution_error="awaiting `grapharc go`", + ) + budget = self._round_budget(meter) try: raw = compiled.invoke( @@ -803,6 +806,14 @@ def _request_approval( so a reader showing "approve this?" needs nothing but the trace. The callback may block indefinitely from the loop's point of view — bounding the wait is the callback's job, reported as a "timeout" decision. + + `kinds` rides alongside `nodes` because only the kind is governed + (`ProposedNode`: the name is an instance label a planner chooses freely). + A request that names `fix_it` and omits `apply_change` asks a human to + approve a mutating plan while showing them a harmless word — the reader + cannot recover the kind from the name, so the event has to carry it. + `worst_case` is on the same event for the same reason: what this may + cost is part of the question, not something to look up elsewhere. """ self._emit( ctx, @@ -813,7 +824,12 @@ def _request_approval( "proposal_id": proposal.proposal_id, "fingerprint": proposal.fingerprint(), "nodes": [n.name for n in proposal.nodes], + "kinds": [n.kind for n in proposal.nodes], "edges": [[e.source, e.target] for e in proposal.edges], + "rationale": proposal.rationale, + "worst_case_tokens": verdict.worst_case.tokens, + "worst_case_seconds": verdict.worst_case.seconds, + "worst_case_complete": verdict.worst_case_complete, "goal": goal, }, ) diff --git a/grapharc/server/live.py b/grapharc/server/live.py index bb02a49..1d645c9 100644 --- a/grapharc/server/live.py +++ b/grapharc/server/live.py @@ -339,13 +339,22 @@ def compose_snapshot( mermaid = to_mermaid(recorder, chosen) run = replay(recorder, chosen) graph = layout_graph(build_graph_view(run)) - # Finished means: something wrote a termination reason, OR a driver wrote + # Finished means: something wrote a termination reason, OR a *driver* wrote # its terminal `stop` event — the planner writes the latter and never the - # former, and keying on `termination_reason` alone held the SSE stream - # open forever for every planner run. - done = any( - e.phase == "stop" or "termination_reason" in (e.state_delta or {}) - for e in run_events + # former, and keying on `termination_reason` alone held the SSE stream open + # forever for every planner run. + # + # "Driver" is the load-bearing word, and `orphan_sub_events` is how it is + # read. Every `AgentNode` phase writes its own `stop` when its agent loop + # ends, so on a multi-phase graph — investigate, then apply_change, then + # verify — any `stop` in `run_events` meant the whole run was declared + # finished the moment the *first* phase ended, and the SSE stream closed on + # a page with two nodes still to run. A phase's stop falls inside its node + # span and is attributed there; the driver's own is an orphan, and a run + # with no graph at all (`grapharc agent`) has only orphans, so its stop + # still ends the stream exactly as before. + done = any("termination_reason" in (e.state_delta or {}) for e in run_events) or any( + e.phase == "stop" for e in run.orphan_sub_events ) # A node that started and never ended means someone is inside it right now # — a delegated executor writes nothing between its start and its finish, diff --git a/grapharc/slack/bot.py b/grapharc/slack/bot.py index 85a5257..c658c01 100644 --- a/grapharc/slack/bot.py +++ b/grapharc/slack/bot.py @@ -24,18 +24,34 @@ from __future__ import annotations +import json import re import shlex from typing import Any from grapharc.observe.metrics import to_mermaid from grapharc.observe.trace import TailRecorder -from grapharc.slack.command import SlackCommandError, parse_command, trace_path, usage_text +from grapharc.planner.approval_file import read_request, write_decision +from grapharc.slack.command import ( + SlackCommandError, + confine_path, + effective_timeout, + mutating_kinds_for, + parse_command, + trace_path, + usage_text, +) from grapharc.slack.config import SlackBotConfig from grapharc.slack.format import format_result, live_view_url, mermaid_live_url -from grapharc.slack.live import LiveSettings, LiveSink, LiveTail +from grapharc.slack.live import ApprovalPrompt, LiveSettings, LiveSink, LiveTail from grapharc.slack.runner import run_command +#: `action_id`s for the two approval buttons. Stable strings, because a bolt +#: listener is registered against them and a message posted by an older bot +#: keeps whatever it was drawn with. +APPROVE_ACTION = "grapharc_approve" +DENY_ACTION = "grapharc_deny" + # An app_mention's text arrives as "<@U0BOTID> metrics t.jsonl r1". _MENTION = re.compile(r"<@[A-Z0-9]+>\s*") @@ -61,10 +77,18 @@ def handle_text_live(text: str, config: SlackBotConfig, sink: LiveSink | None) - allow_model=config.allow_model, allow_agent=config.allow_agent, timeout_seconds=config.timeout_seconds, + work_timeout_seconds=config.work_timeout_seconds, ) except SlackCommandError as exc: return str(exc) + # Recomputed from the admitted argv, so the runner's kill and the ceilings + # the gate injected into that same argv are derived from one number. + timeout = effective_timeout( + argv, + timeout_seconds=config.timeout_seconds, + work_timeout_seconds=config.work_timeout_seconds, + ) tpath = trace_path(argv, config.workdir) # Run ids already in the file, noted before the run: on a reused trace, # the final diagram must be *this* run's, and if this run wrote nothing @@ -84,29 +108,117 @@ def handle_text_live(text: str, config: SlackBotConfig, sink: LiveSink | None) - handle = None if handle is None: - result = run_command( - argv, workdir=config.workdir, timeout_seconds=config.timeout_seconds - ) + result = run_command(argv, workdir=config.workdir, timeout_seconds=timeout) return _with_final_links(format_result(result), argv, tpath, config, prior_runs) - def _update(message: str) -> bool: + def _update(message: str, prompt: ApprovalPrompt | None = None) -> bool: try: - return sink.update(handle, message) + return sink.update(handle, message, approval_blocks(message, prompt)) except Exception: return False settings = LiveSettings(update_interval=config.live_interval_seconds) view_url = live_view_url(argv, base=config.live_url_base, workdir=config.workdir) - with LiveTail(tpath, argv, _update, settings, view_url=view_url): - result = run_command( - argv, workdir=config.workdir, timeout_seconds=config.timeout_seconds - ) + with LiveTail( + tpath, + argv, + _update, + settings, + view_url=view_url, + workdir=config.workdir, + mutating_kinds=mutating_kinds_for(argv, config.workdir), + ): + result = run_command(argv, workdir=config.workdir, timeout_seconds=timeout) final = _with_final_links(format_result(result), argv, tpath, config, prior_runs) if _update(final): return "" return final +def approval_blocks(text: str, prompt: ApprovalPrompt | None) -> list[dict[str, Any]] | None: + """Block Kit for a parked run: the message, then Approve / Deny. + + None when nothing is pending — which is also how the buttons *leave*. The + status message is edited in place all run long, so the same edit that + reports "approved, running" has to drop the blocks; a finished run still + showing a live Approve button is a lie a later click would act on. + """ + if prompt is None: + return None + value = json.dumps({"dir": prompt.directory, "fp": prompt.fingerprint}) + return [ + {"type": "section", "text": {"type": "mrkdwn", "text": text}}, + { + "type": "actions", + "elements": [ + { + "type": "button", + "action_id": APPROVE_ACTION, + "style": "primary", + "text": {"type": "plain_text", "text": "Approve"}, + "value": value, + }, + { + "type": "button", + "action_id": DENY_ACTION, + "style": "danger", + "text": {"type": "plain_text", "text": "Deny"}, + "value": value, + }, + ], + }, + ] + + +def handle_approval_action( + raw_value: str, config: SlackBotConfig, *, deny: bool, actor: str = "" +) -> str: + """Answer a parked run from a button click. Returns what to say about it. + + Slack signs the payloads bolt hands us, so the value is not attacker-typed + the way a slash command is. It is still re-checked here, because "signed by + Slack" says the click is genuine, not that the button was drawn by a + version of this bot that meant the same thing by it: + + - the directory is re-confined inside the working directory, exactly as the + command gate confines a typed path; + - the fingerprint must match the request currently on disk. This is the + load-bearing check. A parked run rewrites its request every round, so a + button scrolled back to from round 1 names a proposal that is no longer + the one waiting — and approving *that* would be approving a graph nobody + read. Mismatch is refused here and, belt and braces, would be discarded + by `file_approval` too. + """ + try: + payload = json.loads(raw_value) + directory = str(payload["dir"]) + fingerprint = str(payload["fp"]) + except (ValueError, KeyError, TypeError): + return "that button carried nothing I can act on." + + try: + confine_path(directory, config.workdir) + except SlackCommandError as exc: + return str(exc) + + resolved = (config.workdir / directory).resolve() + request = read_request(resolved) + if request is None: + # The common, boring case: the run already moved on — approved by + # someone else, denied, or timed out while the message sat there. + return "nothing is waiting for approval there any more." + if str(request.get("fingerprint", "")) != fingerprint: + return ( + "that button was drawn for an earlier plan; the run has since " + "proposed a different one. Scroll down to the current message." + ) + + decision = "denied" if deny else "approved" + write_decision(resolved, fingerprint=fingerprint, decision=decision) + who = f" by <@{actor}>" if actor else "" + return f"{decision}{who} — plan `{fingerprint}`" + + def _with_final_links( final: str, argv: list[str], @@ -169,10 +281,18 @@ def post(self, text: str) -> object | None: except Exception: return None - def update(self, handle: object, text: str) -> bool: + def update( + self, handle: object, text: str, blocks: list[dict[str, Any]] | None = None + ) -> bool: try: channel, ts = handle # type: ignore[misc] - self._client.chat_update(channel=channel, ts=ts, text=text) + # `blocks=[]` rather than omitting the field: Slack keeps a + # message's existing blocks when `chat.update` does not mention + # them, so leaving it out would leave a finished run's Approve + # button on screen for someone to click. + self._client.chat_update( + channel=channel, ts=ts, text=text, blocks=blocks if blocks else [] + ) return True except Exception: return False @@ -210,6 +330,38 @@ def _mention(event: dict[str, Any], say: Any, client: Any) -> None: if reply: say(reply, thread_ts=thread_ts) + def _decide(ack: Any, body: dict[str, Any], client: Any, *, deny: bool) -> None: + """One Approve/Deny click. Ack first — Slack wants it within 3 seconds.""" + ack() + actions = body.get("actions") or [{}] + answer = handle_approval_action( + str(actions[0].get("value", "")), + config, + deny=deny, + actor=str((body.get("user") or {}).get("id", "")), + ) + # Posted as a new message in the run's thread rather than edited over + # the status message: the tailer owns that message and would overwrite + # this within the second. A reply is also the honest shape — who + # decided, and when, is a fact about the conversation, and the trace + # (which has no actor field) cannot record it. + channel = (body.get("channel") or {}).get("id", "") + message_ts = (body.get("message") or {}).get("ts") + if not channel: + return + try: + client.chat_postMessage(channel=channel, text=answer, thread_ts=message_ts) + except Exception: + pass + + @app.action(APPROVE_ACTION) + def _approve(ack: Any, body: dict[str, Any], client: Any) -> None: + _decide(ack, body, client, deny=False) + + @app.action(DENY_ACTION) + def _deny(ack: Any, body: dict[str, Any], client: Any) -> None: + _decide(ack, body, client, deny=True) + return app diff --git a/grapharc/slack/command.py b/grapharc/slack/command.py index 75f5ed2..5d47ee3 100644 --- a/grapharc/slack/command.py +++ b/grapharc/slack/command.py @@ -39,12 +39,20 @@ from __future__ import annotations +import importlib import shlex +import tomllib import uuid from dataclasses import dataclass, field from datetime import UTC, datetime from pathlib import Path +#: What the CLI plans with when neither a flag nor grapharc.toml says +#: otherwise. A second copy of `cli.plan.DEFAULT_REGISTRY`, kept here so this +#: module stays importable with nothing but the standard library — a test +#: asserts the two agree. +DEFAULT_PLAN_REGISTRY = "grapharc.examples.plan_incident:build_registry" + class SlackCommandError(Exception): """The text is not something the bot will run; the message says why.""" @@ -91,6 +99,17 @@ class CommandSpec: #: the human approval gate before anything executes. AGENT_PLAN_REGISTRY = "grapharc.stdlib:build_registry" +#: Subcommands whose Slack run *does* something rather than reading a file +#: back. They get `work_timeout_seconds` instead of the reader timeout: a +#: delegated Claude Code phase takes minutes, and the 120s that is generous for +#: `metrics` is a SIGKILL through the middle of an approved run. +WORK_COMMANDS = frozenset({"agent"}) + +#: The longest a parked plan may hold its wall clock waiting for a human. The +#: rest of the budget belongs to the work the human is approving; without a cap +#: a 30-minute work budget could be spent entirely on nobody answering. +APPROVAL_WAIT_CAP_SECONDS = 900.0 + #: Subcommands that write a trace while they run. The gate gives each of them #: a trace path it knows (unless the requester named one), so the bot can tail #: the file for live progress and readers (`metrics`, `viz`, `replay`) can be @@ -123,7 +142,13 @@ class CommandSpec: "--max-tokens": False, "--approval-timeout": False, }, - bool_flags=frozenset({"--approve", "--scripted"}), + # `--go` executes the admitted graph in the same run instead of + # stopping at PLANNED. Admitted from Slack because without it the + # supervised loop had nowhere to go: a parked plan was approved by a + # human and then returned "awaiting `grapharc go`" — into a subcommand + # this gate does not carry. It is safe to admit only because of the + # rule below, which makes `--go` imply `--approve`. + bool_flags=frozenset({"--approve", "--scripted", "--go"}), model_flags=frozenset({"--model"}), choice_flags={"--registry": PLAN_REGISTRIES}, ), @@ -148,7 +173,13 @@ class CommandSpec: repeatable_flags=frozenset({"--allow", "--deny"}), ), "approve": CommandSpec( - bool_flags=frozenset({"--deny"}), path_positionals=frozenset({0}) + # `--show` decides nothing and `--fingerprint` binds a decision to the + # plan that was read, so both make the Slack path safer rather than + # wider: from a phone, "look at it first" and "only if it is still + # this one" are exactly the two things a reviewer needs. + value_flags={"--fingerprint": False}, + bool_flags=frozenset({"--deny", "--show"}), + path_positionals=frozenset({0}), ), "replay": CommandSpec(path_positionals=frozenset({0})), "diff": CommandSpec(path_positionals=frozenset({0})), @@ -158,6 +189,83 @@ class CommandSpec: } +def effective_timeout( + argv: list[str], *, timeout_seconds: float, work_timeout_seconds: float +) -> float: + """The wall clock this argv gets before the runner kills it. + + Two budgets, because two very different things are being bounded. A reader + (`metrics`, `viz`, `trace`) that has not answered in two minutes is stuck, + and holding a bot worker thread longer helps nobody. A command that plans + and then executes is *supposed* to take minutes — a delegated Claude Code + phase reads files and edits them — and killing it at 120s does not protect + anything, it just severs an approved run partway through its work. + + `bot.py` calls this on the argv the gate returned, so the runner and the + gate's own `--approval-timeout` / `--max-seconds` injections agree on which + budget is in play. + """ + if not argv: + return timeout_seconds + if argv[0] in WORK_COMMANDS: + return work_timeout_seconds + if argv[0] == "plan" and _has_flag(argv, "--go"): + return work_timeout_seconds + return timeout_seconds + + +def mutating_kinds_for(argv: list[str], workdir: Path | None = None) -> frozenset[str] | None: + """Which kinds of this argv's registry can change things, for the approver. + + None means "assume every kind does" — the same fail-closed reading `plan` + takes when a registry declares no `MUTATING_KINDS`. It is returned whenever + this cannot *prove* which registry will be in play, because the mark exists + to warn and a mark that is wrong in the reassuring direction is worse than + no mark at all. + + Only modules in `PLAN_REGISTRIES` are ever imported. A `--registry` outside + that set was already refused by the gate; a `grapharc.toml` may point the + CLI at any module at all (including a `.py` file beside it), and that one is + reported as unknown rather than imported here — this runs inside the bot + process, and the bot does not execute the working directory's code. + """ + if not argv or argv[0] != "plan": + return frozenset() + target = _flag_value(argv, "--registry") + if target is None: + # No flag: the CLI will resolve `registry` from grapharc.toml in the + # working directory (which is the runner's cwd), else its own default. + target = _configured_registry(workdir) or DEFAULT_PLAN_REGISTRY + if target not in PLAN_REGISTRIES: + return None + module_name = target.partition(":")[0] + try: + module = importlib.import_module(module_name) + except Exception: + return None + declared = getattr(module, "MUTATING_KINDS", None) + if declared is None: + return None + return frozenset(str(kind) for kind in declared) + + +def _configured_registry(workdir: Path | None) -> str | None: + """`registry` from the working directory's grapharc.toml, if it sets one.""" + if workdir is None: + return None + path = workdir / "grapharc.toml" + try: + with path.open("rb") as handle: + loaded = tomllib.load(handle) + except (OSError, tomllib.TOMLDecodeError): + return None + section = loaded.get("grapharc") + if not isinstance(section, dict): + return None + value = section.get("registry") + return value if isinstance(value, str) else None + + def usage_text(*, allow_model: bool = False, allow_agent: bool = False) -> str: """One short message for an empty or unrecognised request.""" agent_on = allow_agent and allow_model @@ -179,7 +287,7 @@ def usage_text(*, allow_model: bool = False, allow_agent: bool = False) -> str: return "\n".join(lines) -def _confined(raw: str, workdir: Path) -> None: +def confine_path(raw: str, workdir: Path) -> None: """Refuse a path that escapes the working directory, before anything runs. Lexical resolution only — the target need not exist yet (`--trace` names a @@ -215,8 +323,15 @@ def parse_command( allow_model: bool = False, allow_agent: bool = False, timeout_seconds: float | None = None, + work_timeout_seconds: float | None = None, ) -> list[str]: - """Turn Slack text into the argv the bot may run, or raise with the reason.""" + """Turn Slack text into the argv the bot may run, or raise with the reason. + + `work_timeout_seconds` is the budget a command that *executes* gets (see + `effective_timeout`); the injected `--approval-timeout` and `--max-seconds` + are derived from whichever of the two will actually apply, so the CLI's own + graceful limits fire before the runner's kill rather than after it. + """ # People copy commands out of code-formatted Slack messages, and the # backticks come along for the ride: "`approve x/trace.jsonl`" arrives # with a backtick glued to the first and last token. No admissible @@ -310,11 +425,11 @@ def parse_command( allowed = ", ".join(f"`{v}`" for v in sorted(spec.choice_flags[flag])) raise SlackCommandError(f"`{flag}` from Slack accepts only: {allowed}") if is_path: - _confined(value, workdir) + confine_path(value, workdir) argv.extend([flag, value]) continue if positional_index in spec.path_positionals: - _confined(token, workdir) + confine_path(token, workdir) argv.append(token) positional_index += 1 index += 1 @@ -328,9 +443,12 @@ def parse_command( argv.extend(["--workspace", "agent"]) # The CLI's max_seconds interrupts the run cleanly and reports; the # bot's timeout kills the process mid-sentence. Default the ceiling - # to just under the timeout so the graceful mechanism fires first. - if "--max-seconds" not in argv and timeout_seconds is not None: - argv.extend(["--max-seconds", str(max(5.0, timeout_seconds - 10.0))]) + # to just under the timeout so the graceful mechanism fires first — + # `agent` is a work command, so that is the work budget, not the + # reader's two minutes. + budget = work_timeout_seconds if work_timeout_seconds is not None else timeout_seconds + if "--max-seconds" not in argv and budget is not None: + argv.extend(["--max-seconds", str(max(5.0, budget - 10.0))]) # A delegated run uses Claude Code's tools, and its Bash is a real # shell on the host with no grapharc sandbox around it. From Slack # that defaults off; a requester who set explicit globs made a @@ -359,20 +477,66 @@ def parse_command( if not _has_flag(argv, "--approve"): argv.append("--approve") - # ANY parked plan — stdlib-injected or requester-chosen `--approve` on a - # demo registry — must time its wait under the runner's kill: the CLI - # default (300s) exceeds the bot default (120s), and a park that outlives - # the runner is a hard kill mid-wait instead of a clean approval_timeout. - # Half the wall clock for the wait, capped to leave the run 10s to report, - # floored so a tiny ceiling still gives a human a moment. - if ( - name == "plan" - and _has_flag(argv, "--approve") - and not _has_flag(argv, "--approval-timeout") - and timeout_seconds is not None - ): - wait = max(10.0, min(timeout_seconds - 10.0, timeout_seconds / 2)) - argv.extend(["--approval-timeout", str(wait)]) + if name == "plan" and _has_flag(argv, "--go") and not _has_flag(argv, "--approve"): + # `--go` is the difference between proposing a graph and running one, + # and from Slack that difference always goes through a person. Any + # workspace member can type into this bot; without this rule a single + # message would take a model's proposal straight to execution on the + # host, with the graph visible only afterwards. + # + # It is forced rather than refused so that the useful command stays one + # message: `plan "…" --go` means propose it, show it to me, and run it + # if I say yes. The stdlib rule above already forces `--approve` for the + # agent registry; this extends the same reading to every registry, since + # what makes execution worth gating is that it executes. + argv.append("--approve") + + # ANY parked plan — stdlib-injected, `--go`-forced, or a requester's own + # `--approve` on a demo registry — must time its wait under the runner's + # kill: the CLI default (300s) exceeds the bot's reader default (120s), and + # a park that outlives the runner is a hard kill mid-wait instead of a clean + # approval_timeout. + # + # How much of the budget the human gets depends on what happens after they + # answer. A plan-only park has nothing left to do, so the wait may have half + # the clock. With `--go`, saying yes is the *start* of the work — give the + # human a third, capped, and leave the rest for the run they authorised. + if name == "plan" and _has_flag(argv, "--approve") and timeout_seconds is not None: + budget = effective_timeout( + argv, + timeout_seconds=timeout_seconds, + work_timeout_seconds=( + timeout_seconds if work_timeout_seconds is None else work_timeout_seconds + ), + ) + ceiling = max(10.0, budget - 10.0) + supplied = _flag_value(argv, "--approval-timeout") + if supplied is None: + share = budget / 3 if _has_flag(argv, "--go") else budget / 2 + wait = max(10.0, min(ceiling, share, APPROVAL_WAIT_CAP_SECONDS)) + argv.extend(["--approval-timeout", str(wait)]) + else: + # A requester may ask for a shorter wait, never a longer one. The + # injection above only fires when the flag is absent, so without + # this check `--approval-timeout 100000` sailed past it and the + # park outlived the runner — which is not an `approval_timeout` + # the run can report, it is a SIGKILL through the middle of the + # wait. Refused rather than clamped: silently running a different + # command from the one someone typed is how a gate loses its + # meaning. + try: + asked = float(supplied) + except ValueError: + raise SlackCommandError( + f"`--approval-timeout` wants a number of seconds, got {supplied!r}" + ) from None + if asked <= 0 or asked > ceiling: + raise SlackCommandError( + f"`--approval-timeout {supplied}` does not fit this command's " + f"budget: the wait must be between 1 and {ceiling:.0f} seconds, " + "so that a run nobody answers ends by reporting a timeout " + "rather than by being killed mid-wait" + ) # A tracing command whose trace the requester did not place gets one the # bot can find: unique per invocation (a reused file would make a tailer's diff --git a/grapharc/slack/config.py b/grapharc/slack/config.py index 9b149b9..91ba9e2 100644 --- a/grapharc/slack/config.py +++ b/grapharc/slack/config.py @@ -28,9 +28,16 @@ class SlackBotConfig: app_token: str # Every path a Slack user names must resolve inside this directory. workdir: Path = field(default_factory=Path.cwd) - # One command's wall clock. Slack acks immediately, so this bounds how - # long a runaway command can hold one of the bot's worker threads. + # One reader command's wall clock. Slack acks immediately, so this bounds + # how long a runaway command can hold one of the bot's worker threads. timeout_seconds: float = 120.0 + # The wall clock for a command that plans and then *executes* (`agent`, + # `plan --go`). Much larger on purpose: a delegated Claude Code phase reads + # and edits files, and two minutes is not a safety limit for that, it is a + # SIGKILL through the middle of a run a human just approved. The gate keeps + # the CLI's own graceful ceilings under this, so the run reports rather than + # dies. See `command.effective_timeout` for which budget applies to what. + work_timeout_seconds: float = 1800.0 # Opt-in: allow `--model` / `--reviewer-model`, which reach paid backends. allow_model: bool = False # Second opt-in: allow `agent`, which executes tools on the host. Only @@ -82,6 +89,21 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig: if timeout <= 0: raise SlackConfigError("GRAPHARC_SLACK_TIMEOUT must be positive") + raw_work_timeout = env.get("GRAPHARC_SLACK_WORK_TIMEOUT", "1800") + try: + work_timeout = float(raw_work_timeout) + except ValueError: + raise SlackConfigError( + "GRAPHARC_SLACK_WORK_TIMEOUT must be a number of seconds, " + f"got {raw_work_timeout!r}" + ) from None + if work_timeout <= 0: + raise SlackConfigError("GRAPHARC_SLACK_WORK_TIMEOUT must be positive") + # A work budget under the reader budget is almost certainly a typo, and + # the failure it produces is confusing: `plan --go` would be killed + # sooner than `metrics`. Take the larger rather than obeying literally. + work_timeout = max(work_timeout, timeout) + raw_interval = env.get("GRAPHARC_SLACK_LIVE_INTERVAL", "2.5") try: live_interval = float(raw_interval) @@ -105,6 +127,7 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig: app_token=app_token, workdir=workdir, timeout_seconds=timeout, + work_timeout_seconds=work_timeout, allow_model=env.get("GRAPHARC_SLACK_ALLOW_MODEL", "") == "1", allow_agent=env.get("GRAPHARC_SLACK_ALLOW_AGENT", "") == "1", slash_command=env.get("GRAPHARC_SLACK_COMMAND", "/grapharc"), diff --git a/grapharc/slack/live.py b/grapharc/slack/live.py index 941d574..00ad444 100644 --- a/grapharc/slack/live.py +++ b/grapharc/slack/live.py @@ -24,7 +24,7 @@ from collections.abc import Callable from dataclasses import dataclass from pathlib import Path -from typing import Protocol +from typing import Any, Protocol from grapharc.observe.metrics import to_mermaid from grapharc.observe.replay import NodeExecution, ReplayedRun, replay @@ -45,11 +45,18 @@ class LiveSink(Protocol): returns None (the caller falls back to the plain blocking reply), `update` returns False (the tailer goes quiet). A sink can be handed an unreliable network and the run must not care. + + `update` takes optional Block Kit `blocks` because a parked run's message + carries Approve/Deny buttons. `None` means "this message is text" — and, + for a sink editing in place, means *drop* any blocks it had: the edit that + says a plan was approved must also take its Approve button away. """ def post(self, text: str) -> object | None: ... - def update(self, handle: object, text: str) -> bool: ... + def update( + self, handle: object, text: str, blocks: list[dict[str, Any]] | None = None + ) -> bool: ... @dataclass(frozen=True) @@ -79,15 +86,19 @@ def __init__( self, trace_path: Path, argv: list[str], - update: Callable[[str], bool], + update: Callable[..., bool], settings: LiveSettings | None = None, *, view_url: str | None = None, + workdir: Path | None = None, + mutating_kinds: frozenset[str] | None = frozenset(), ) -> None: self._path = trace_path self._argv = list(argv) self._update = update self._view_url = view_url + self._workdir = workdir or Path.cwd() + self._mutating_kinds = mutating_kinds self._settings = settings or LiveSettings() try: self._offset = trace_path.stat().st_size @@ -122,14 +133,15 @@ def _loop(self) -> None: continue if time.monotonic() - last_update < self._settings.update_interval: continue - text = self._render(run_id) - if text is None: + rendered = self._render(run_id) + if rendered is None: continue + text, prompt = rendered dirty = False last_update = time.monotonic() if text == last_text: continue - if not self._update(text): + if not self._update(text, prompt): return # the sink is dead; go quiet, never raise last_text = text except Exception: @@ -137,7 +149,7 @@ def _loop(self) -> None: # do not depend on it. return - def _render(self, run_id: str) -> str | None: + def _render(self, run_id: str) -> tuple[str, ApprovalPrompt | None] | None: # TailRecorder, not TraceRecorder: a read can land mid-write, and the # plain reader raises on the torn line where this one skips it. recorder = TailRecorder(self._path) @@ -146,13 +158,15 @@ def _render(self, run_id: str) -> str | None: diagram = to_mermaid(recorder, run_id) except Exception: return None # e.g. no complete events yet; retry next tick - return render_progress( + text = render_progress( run, argv=self._argv, elapsed_s=time.monotonic() - self._started_at, diagram=diagram, view_url=self._view_url, + mutating_kinds=self._mutating_kinds, ) + return text, pending_approval_prompt(run, self._argv, self._workdir) def _read_new_run_id(self) -> str | None: """Fold newly appended complete lines; return the latest run_id seen. @@ -260,6 +274,70 @@ def _pending_approval(run: ReplayedRun) -> TraceEvent | None: return pending +def plan_lines( + approval: TraceEvent, *, mutating_kinds: frozenset[str] | None = frozenset() +) -> list[str]: + """The proposed graph as text: what a human is being asked to approve. + + Read from the `approval_request` event alone, which carries every field + this needs (`loop._request_approval`). A link is not an answer here — the + person deciding is on a phone, and "the graph is over there" asks them to + approve something they have not seen. So the shape goes in the message. + + Kinds, not just names: `fix_it` is a name a planner chose and `apply_change` + is the thing the gate actually governs. A node whose kind can change files + is marked ✎, because "will this touch my repo" is the question being asked. + + `mutating_kinds=None` means the registry declared nothing about which of + its kinds mutate. Every node is then marked, matching the reading `plan` + already takes for its own plan file: undeclared is not evidence of safety. + """ + delta = approval.state_delta or {} + names = [str(n) for n in delta.get("nodes", [])] + kinds = [str(k) for k in delta.get("kinds", [])] + lines: list[str] = [] + if mutating_kinds is None and names: + lines.append("⚠ this registry does not say which kinds change things — assume all do") + + rationale = str(delta.get("rationale") or "").strip() + if rationale: + lines.append(f"why: {' '.join(rationale.split())[:240]}") + + if names: + lines.append(f"{len(names)} node{'s' if len(names) != 1 else ''}:") + for index, name in enumerate(names): + kind = kinds[index] if index < len(kinds) else "" + # A kind equal to the name adds nothing — `ProposedNode` defaults + # `kind` to `name`, and printing "verify (verify)" is noise. + label = f"{name} ({kind})" if kind and kind != name else name + mark = "✎" if mutating_kinds is None or kind in mutating_kinds else "·" + lines.append(f" {mark} {label}") + + edges = delta.get("edges", []) + if edges: + lines.append(f"{len(edges)} edge{'s' if len(edges) != 1 else ''}:") + for edge in edges: + try: + source, target = edge[0], edge[1] + except (TypeError, IndexError, KeyError): + continue + lines.append(f" {source} → {target}") + + tokens = delta.get("worst_case_tokens") + if tokens: + estimate = f"worst case: {tokens} tok" + if not delta.get("worst_case_complete", True): + # The flag exists so nobody reads an incomplete number as complete. + estimate += " (lower bound — an unregistered kind has no price)" + lines.append(estimate) + + if not lines: + # An empty proposal is legal and means "no further work". Saying so is + # better than an empty box that reads as a rendering failure. + lines.append("(an empty plan — the planner proposes no further work)") + return lines + + def _planned_lines(run: ReplayedRun) -> list[str] | None: """One line per *declared* node, marked by status — or None without topology. @@ -318,13 +396,15 @@ def render_progress( elapsed_s: float, diagram: str | None = None, view_url: str | None = None, + mutating_kinds: frozenset[str] | None = frozenset(), ) -> str: """One Slack message describing the run so far. - Declared-graph node marks when the trace carries topology; node executions - when the run has them; otherwise the tail of the flat event feed — an - agent with no enclosing graph is all flat feed, and an empty box saying - nothing was the alternative. + A parked run shows the *plan* — the graph it is asking permission for. + Otherwise: declared-graph node marks when the trace carries topology; node + executions when the run has them; otherwise the tail of the flat event feed + — an agent with no enclosing graph is all flat feed, and an empty box + saying nothing was the alternative. """ approval = _pending_approval(run) if approval is not None: @@ -339,7 +419,12 @@ def render_progress( header += f" · {goal[:80]}" planned = _planned_lines(run) - if planned is not None: + if approval is not None: + # The proposed graph, not the node marks: every node of a parked plan + # is pending by definition, so a column of ⬜ says nothing about what + # is being authorised. The shape and the kinds are the question. + lines = plan_lines(approval, mutating_kinds=mutating_kinds) + elif planned is not None: lines = planned elif run.executions: lines = [_execution_line(e) for e in run.executions] @@ -372,12 +457,15 @@ def render_progress( parts.append(" · ".join(footer)) if approval is not None: + parts.append("*nothing above has run yet* — it runs only if you approve.") trace_arg = _trace_argument(argv) if trace_arg: - where = "live view" if view_url else "diagram" + # The typed form stays documented in the message even when the + # buttons render: a workspace without interactivity enabled, a + # client that will not draw blocks, and a stale message whose + # buttons the fingerprint check will refuse all end up here. parts.append( - f"planned graph is in the {where} link — approve with " - f"`/grapharc approve {trace_arg}`, refuse with " + f"buttons below, or: `/grapharc approve {trace_arg}` · " f"`/grapharc approve {trace_arg} --deny`" ) # The operator's own live view is the primary link; the mermaid.live @@ -389,6 +477,58 @@ def render_progress( return "\n".join(parts) +@dataclass(frozen=True) +class ApprovalPrompt: + """What a pending approval needs from the person deciding. + + `directory` is where the file handshake lives — relative to the bot's + working directory, because that is the only form safe to send to Slack and + back: an absolute path in a button's value is a path the round trip could + rewrite into somewhere else. `bot.py` re-confines it on the way in + regardless; this just keeps the value uninteresting to tamper with. + + `fingerprint` is what makes the button honest. It names the exact proposal + that was displayed, and `approve` refuses a decision quoting any other — so + a button on a scrolled-back message cannot approve the plan that replaced + the one it was drawn for. + """ + + directory: str + fingerprint: str + round_number: int = 0 + + +def pending_approval_prompt( + run: ReplayedRun, argv: list[str], workdir: Path +) -> ApprovalPrompt | None: + """The prompt for `run`'s unanswered approval request, or None.""" + approval = _pending_approval(run) + if approval is None: + return None + trace_arg = _trace_argument(argv) + if not trace_arg: + return None + delta = approval.state_delta or {} + fingerprint = str(delta.get("fingerprint") or "") + if not fingerprint: + # Without a fingerprint there is nothing to bind a decision to, and an + # unbound approve button is exactly the thing this design refuses to + # ship. Fall back to the typed command, which reads the request file. + return None + directory = Path(trace_arg).parent + try: + relative = directory.as_posix() if not directory.is_absolute() else str( + directory.relative_to(workdir) + ) + except ValueError: + return None + return ApprovalPrompt( + directory=relative, + fingerprint=fingerprint, + round_number=int(delta.get("round") or 0), + ) + + def _trace_argument(argv: list[str]) -> str | None: for index, token in enumerate(argv): if token == "--trace" and index + 1 < len(argv): diff --git a/grapharc/stdlib.py b/grapharc/stdlib.py index 2ac2476..5edc75d 100644 --- a/grapharc/stdlib.py +++ b/grapharc/stdlib.py @@ -231,7 +231,19 @@ def body(state: WorkState, ctx: Any) -> dict: # The stop reason is carried with the text, so a phase that gave up # is not indistinguishable from one that finished. reason = result.termination_reason.value - line = result.output if reason == "target_met" else f"[{reason}] {result.output}" + if reason == "target_met": + line = result.output + else: + # `AgentResult.output` is empty by contract for every reason but + # TARGET_MET — the mid-work text lives in `partial_output`. + # Reading `output` here formatted "[budget_exhausted] " with + # nothing after it: the work the phase did manage was dropped on + # the floor, and a downstream goal check could read the empty + # note as a phase that had nothing to report rather than one + # that was cut off. `note` is the last resort — it carries the + # budget line or the error repr when there is no prose at all. + text = result.partial_output or result.note or "(no output)" + line = curtailed_note(reason, text) # The reducer appends; returning the accumulated list would double it. return {field: [line]} @@ -264,8 +276,9 @@ def default_harness(tools: tuple[str, ...], workspace: Any = None) -> Any: ) from grapharc.tools import core_tools + root = Path(workspace or Path.cwd()) registry = ToolRegistry() - for spec in core_tools(Path(workspace or Path.cwd()), include=tools): + for spec in core_tools(root, include=tools): registry.register(spec) # `literal`, not a bare pattern: these names come from a registry, not from # an operator writing globs, and an ALLOW rule is the one tier where a name @@ -274,7 +287,14 @@ def default_harness(tools: tuple[str, ...], workspace: Any = None) -> Any: rules=[PermissionRule.literal(Decision.ALLOW, name) for name in tools], default=Decision.DENY, ) - return Harness(registry=registry, policy=policy, executor=LocalExecutor()) + # `workspace` is named even though `LocalExecutor` confines nothing: it is + # the directory a *delegated* phase is given as its cwd. Without it, this + # registry driven by the Claude CLI — the combination the module docstring + # describes, where the whole loop goes to Claude Code — failed at every + # agent node with "does not expose one". + return Harness( + registry=registry, policy=policy, executor=LocalExecutor(), workspace=str(root) + ) def build_registry( @@ -373,15 +393,54 @@ def catalog_for_prompt(model: Any = None) -> dict[str, str]: ) +def curtailed_note(reason: str, text: str) -> str: + """How a phase that did not finish cleanly writes itself into state. + + One function so that `goal_met` can recognise the shape it produces + rather than pattern-matching a format written down somewhere else. + """ + return f"[{reason}] {text}" + + +def _curtailed_reasons() -> frozenset[str]: + """Every `StopReason` except the one that means the phase finished. + + Read off the enum rather than hardcoded: a new stop reason must not + silently start reading as a successful outcome. Imported lazily to keep + this module importable without the harness, as the rest of it is. + """ + from grapharc.harness.agent import StopReason + + return frozenset( + reason.value for reason in StopReason if reason is not StopReason.TARGET_MET + ) + + +def is_curtailed(line: str) -> bool: + """True when `line` is a phase reporting that it stopped short.""" + if not line.startswith("["): + return False + marker, separator, _rest = line[1:].partition("]") + return bool(separator) and marker in _curtailed_reasons() + + def goal_met(state: Any) -> bool: - """Done when a report landed in `notes`. + """Done when a *report* landed in `notes` — not merely a line. `apply_change` and `summarize` are the kinds that write `notes`, so this reads as "the run produced its human-facing outcome". Deterministic code, never a model — and defensive about the schema, so a custom state supplied through `--registry` cannot turn "am I done" into an AttributeError. + + A curtailed phase writes a note too, and counting it stopped runs with + `goal_met` whose entire output was `['[error] ']` (issue #96): the agent + node had failed, the failure was folded into state as prose, and the length + check could not tell a report from an apology. A run whose only notes say + they did not finish has not met its goal — it has run out of ways to say + so, which the loop reports as its own reason. """ - return len(getattr(state, "notes", ()) or ()) >= 1 + notes = getattr(state, "notes", ()) or () + return any(not is_curtailed(str(note)) for note in notes) def _observe(state: Any) -> str: @@ -520,8 +579,10 @@ def build_loop( "build_loop", "build_registry", "catalog_for_prompt", + "curtailed_note", "default_edge_policy", "default_harness", "goal_met", + "is_curtailed", "scripted_planner_replies", ] diff --git a/pyproject.toml b/pyproject.toml index 3aebe11..5bdfafc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -116,6 +116,9 @@ all = [ [dependency-groups] dev = [ "httpx>=0.27", + # The MCP server ships behind the `mcp` extra; it is a dev dependency too + # so its gate tests always run in CI rather than silently skipping. + "mcp>=1.2", "pytest>=8.2", "pytest-asyncio>=0.24", "pytest-timeout>=2.3", diff --git a/tests/test_adopt.py b/tests/test_adopt.py new file mode 100644 index 0000000..da6deb6 --- /dev/null +++ b/tests/test_adopt.py @@ -0,0 +1,69 @@ +"""`grapharc init --claude-code` — the adoption files, and their contract. + +Two properties: the command never overwrites what an operator already owns, +and the skill it writes carries the never-self-approve clause in so many +words — the MCP surface refuses the approval verb on its own connection, and +the skill states the same boundary for the host agent's other hands. +""" + +from __future__ import annotations + +import json + +import pytest + +from grapharc.cli.adopt import MCP_CONFIG_FILENAME, SKILL_PATH, SKILL_TEMPLATE +from grapharc.cli.main import main + + +@pytest.fixture +def in_tmp(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + return tmp_path + + +def test_adopt_writes_the_server_config_and_the_skill(in_tmp, capsys): + assert main(["init", "--claude-code", "--json"]) == 0 + + payload = json.loads(capsys.readouterr().out) + assert payload["ok"] is True + config = json.loads((in_tmp / MCP_CONFIG_FILENAME).read_text()) + assert config["mcpServers"]["grapharc"]["command"] == "grapharc" + assert config["mcpServers"]["grapharc"]["args"] == ["mcp"] + assert (in_tmp / SKILL_PATH).is_file() + + +def test_adopt_refuses_to_overwrite_and_names_the_files(in_tmp, capsys): + (in_tmp / MCP_CONFIG_FILENAME).write_text("{}") + + assert main(["init", "--claude-code", "--json"]) == 2 + payload = json.loads(capsys.readouterr().out) + assert payload["ok"] is False + assert MCP_CONFIG_FILENAME in payload["error"] + # The refusal changed nothing: the operator's file is intact and the + # skill was not half-written beside it. + assert (in_tmp / MCP_CONFIG_FILENAME).read_text() == "{}" + assert not (in_tmp / SKILL_PATH).exists() + + +def test_the_skill_carries_the_never_self_approve_clause(in_tmp): + """The one sentence the packaging must not lose, asserted against the + exact bytes a user receives.""" + assert main(["init", "--claude-code"]) == 0 + text = (in_tmp / SKILL_PATH).read_text() + + assert text == SKILL_TEMPLATE + assert "Never run `grapharc approve`" in text + assert "approval-decision.json" in text + assert "A timeout means ask, not retry" in text + assert "The decision belongs to the user" in text + + +def test_plain_init_is_untouched_by_the_new_flag(in_tmp): + """`init` without the flag still scaffolds the registry pair, and the + adoption files are not part of that scaffold.""" + assert main(["init"]) == 0 + assert (in_tmp / "registry.py").is_file() + assert (in_tmp / "grapharc.toml").is_file() + assert not (in_tmp / MCP_CONFIG_FILENAME).exists() + assert not (in_tmp / SKILL_PATH).exists() diff --git a/tests/test_approval.py b/tests/test_approval.py index 59df539..a019ec0 100644 --- a/tests/test_approval.py +++ b/tests/test_approval.py @@ -340,3 +340,143 @@ def test_plan_approve_in_text_mode_still_announces_how_to_answer(tmp_path, capsy ]) assert "grapharc approve" in capsys.readouterr().out + + +# -- the gate reaches `plan` without `--go`, and `go` itself (PR B1) ------------ + + +def _answer(trace_dir: Path, argv_extra: list[str] | None = None) -> threading.Thread: + """Answer the file handshake from a background thread, via the real CLI. + + `--json` so the answer shares captured stdout with the parked command's + own document as two clean JSON values rather than interleaved prose. + """ + + def worker(): + deadline = time.monotonic() + 8 + while time.monotonic() < deadline: + if (trace_dir / REQUEST_FILENAME).exists(): + assert main(["approve", str(trace_dir), "--json", *(argv_extra or [])]) == 0 + return + time.sleep(0.02) + + thread = threading.Thread(target=worker) + thread.start() + return thread + + +def _last_document(text: str) -> dict: + """The final JSON document in a stream that may carry several.""" + decoder = json.JSONDecoder() + documents, index = [], 0 + while index < len(text): + if text[index] != "{": + index += 1 + continue + try: + document, index = decoder.raw_decode(text, index) + except json.JSONDecodeError: + index += 1 + continue + documents.append(document) + assert documents, f"no JSON document in: {text[:200]!r}" + return documents[-1] + + +def test_plan_approve_without_go_parks_and_a_denial_leaves_no_plan_file(tmp_path, capsys): + """`plan --approve` used to return PLANNED before the gate could fire — an + inert flag. Now the parked plan is a real question, and a denial is + fail-closed: nothing is saved for a later `go` to pick up.""" + trace = tmp_path / "run" / "trace.jsonl" + thread = _answer(trace.parent, ["--deny"]) + code = main( + ["plan", "investigate", "--scripted", "--approve", "--approval-timeout", "8", + "--trace", str(trace), "--json"] + ) + thread.join() + + assert code == 1 + payload = _last_document(capsys.readouterr().out) + assert payload["stop"] == "approval_denied" + assert not (trace.parent / "plan.json").exists() + + +def test_plan_approve_approved_still_stops_planned_and_saves_the_plan(tmp_path, capsys): + trace = tmp_path / "run" / "trace.jsonl" + thread = _answer(trace.parent) + code = main( + ["plan", "investigate", "--scripted", "--approve", "--approval-timeout", "8", + "--trace", str(trace), "--json"] + ) + thread.join() + + assert code == 0 + payload = _last_document(capsys.readouterr().out) + assert payload["stop"] == "planned" + assert (trace.parent / "plan.json").exists() + + +def _saved_plan(tmp_path, capsys) -> Path: + trace = tmp_path / "run" / "trace.jsonl" + assert main(["plan", "investigate", "--scripted", "--trace", str(trace), "--json"]) == 0 + capsys.readouterr() # drop the plan document; the tests below have their own + return trace.parent + + +def test_go_approve_denied_executes_nothing(tmp_path, capsys): + """`go --approve` was accepted and silently ignored — the flag now + parks the replayed plan, and a denial leaves it unexecuted.""" + run_dir = _saved_plan(tmp_path, capsys) + thread = _answer(run_dir, ["--deny"]) + code = main(["go", str(run_dir), "--approve", "--approval-timeout", "8", "--json"]) + thread.join() + + assert code == 1 + payload = _last_document(capsys.readouterr().out) + assert payload["stop"] == "approval_denied" + assert payload["executed"] is False + record = json.loads((run_dir / "plan.json").read_text()) + assert "executed_run_id" not in record + + +def test_go_approve_unanswered_times_out_with_the_plan_unexecuted(tmp_path, capsys): + run_dir = _saved_plan(tmp_path, capsys) + code = main(["go", str(run_dir), "--approve", "--approval-timeout", "0.6", "--json"]) + + assert code == 1 + payload = _last_document(capsys.readouterr().out) + assert payload["stop"] == "approval_timeout" + record = json.loads((run_dir / "plan.json").read_text()) + assert "executed_run_id" not in record + + +def test_go_approve_approved_executes_and_stamps_the_plan(tmp_path, capsys): + run_dir = _saved_plan(tmp_path, capsys) + thread = _answer(run_dir) + code = main(["go", str(run_dir), "--approve", "--approval-timeout", "8", "--json"]) + thread.join() + + assert code == 0 + payload = _last_document(capsys.readouterr().out) + assert payload["executed"] is True + record = json.loads((run_dir / "plan.json").read_text()) + assert record["executed_run_id"] + + +def test_the_plan_payload_carries_the_admitted_shape(tmp_path, capsys): + """An external driver must not have to re-read plan.json to learn what was + admitted, under what fingerprint, and whether it can change anything.""" + trace = tmp_path / "run" / "trace.jsonl" + assert main(["plan", "investigate", "--scripted", "--trace", str(trace), "--json"]) == 0 + + payload = _last_document(capsys.readouterr().out) + assert payload["run_dir"] == str(trace.parent) + assert payload["fingerprint"] + names = {node["name"] for node in payload["proposal"]["nodes"]} + assert names # the admitted round, as data + # The incident registry declares deploy as its mutating kind, and the + # admitted replan does not contain it. + assert payload["mutating"] is False + record = json.loads((trace.parent / "plan.json").read_text()) + assert payload["fingerprint"] == record["fingerprint"] + assert names == {node["name"] for node in record["proposal"]["nodes"]} diff --git a/tests/test_approval_review.py b/tests/test_approval_review.py new file mode 100644 index 0000000..babee78 --- /dev/null +++ b/tests/test_approval_review.py @@ -0,0 +1,146 @@ +"""Reviewing a parked plan before answering it — issues #46 and #52. + +The approval gate is the product's central trust claim, and two things were +missing from the human half of it. `grapharc approve` decided first and +described the plan afterwards, with no way to look without deciding (#46); and +a parked run announced that it was waiting without saying what would end the +wait (#52), so the failure mode was a run timing out unapproved while its +operator watched a cursor. +""" + +from __future__ import annotations + +import json + +import pytest + +from grapharc.cli.approve import approve +from grapharc.planner.approval_file import ( + DECISION_FILENAME, + describe_request, + write_request, +) + +REQUEST = { + "proposal_id": "p-1", + "fingerprint": "abc123def456", + "nodes": ["collect_context", "investigate", "apply_change"], + "edges": [["__start__", "collect_context"], ["collect_context", "investigate"]], + "rationale": "read the failing test, then repair it", + "timeout_seconds": 300.0, +} + + +@pytest.fixture +def parked(tmp_path): + write_request(tmp_path, REQUEST) + return tmp_path + + +# -- #46: look before you decide --------------------------------------------- + + +def test_show_prints_the_plan_and_decides_nothing(parked, capsys): + code = approve(parked, show=True) + out = capsys.readouterr().out + + assert code == 0 + assert "abc123def456" in out + assert "apply_change" in out + assert "collect_context -> investigate" in out + assert "read the failing test" in out + assert not (parked / DECISION_FILENAME).exists(), "--show wrote a decision" + + +def test_show_names_the_command_that_would_decide(parked, capsys): + approve(parked, show=True) + out = capsys.readouterr().out + assert "--fingerprint abc123def456" in out + assert "--deny" in out + + +def test_a_decision_bound_to_a_stale_fingerprint_is_refused(parked, capsys): + """Between the `--show` and the `approve`, the run may have re-proposed.""" + code = approve(parked, fingerprint="the-one-i-read") + out = capsys.readouterr().out + capsys.readouterr().err + + assert code == 2 + assert not (parked / DECISION_FILENAME).exists() + + +def test_the_refusal_names_both_fingerprints(parked, capsys): + approve(parked, fingerprint="the-one-i-read") + combined = capsys.readouterr() + text = combined.out + combined.err + assert "the-one-i-read" in text and "abc123def456" in text + + +def test_a_matching_fingerprint_decides_normally(parked): + assert approve(parked, fingerprint="abc123def456") == 0 + decision = json.loads((parked / DECISION_FILENAME).read_text()) + assert decision == {"fingerprint": "abc123def456", "decision": "approved"} + + +def test_the_one_shot_form_still_works_unchanged(parked): + """Automation, and the Slack button path, both rely on this.""" + assert approve(parked) == 0 + decision = json.loads((parked / DECISION_FILENAME).read_text()) + assert decision["decision"] == "approved" + + +def test_the_plan_is_printed_before_the_decision_line(parked, capsys): + """The complaint in #46: it decided, then described what it had decided.""" + approve(parked) + out = capsys.readouterr().out + assert out.index("abc123def456") < out.index("decision") + + +def test_show_as_json_carries_the_shape_and_no_decision(parked, capsys): + approve(parked, show=True, as_json=True) + payload = json.loads(capsys.readouterr().out) + assert payload["decision"] is None + assert payload["nodes"] == REQUEST["nodes"] + assert payload["fingerprint"] == "abc123def456" + + +def test_nothing_waiting_is_still_exit_one(tmp_path, capsys): + assert approve(tmp_path, show=True) == 1 + + +# -- #52: a parked run says what would end the wait --------------------------- + + +def test_the_park_announcement_is_actionable(tmp_path): + text = describe_request(REQUEST, tmp_path) + + assert "abc123def456" in text, "the fingerprint a reviewer must quote" + assert "3 nodes" in text and "apply_change" in text + assert "300s" in text, "the deadline" + assert f"grapharc approve {tmp_path}" in text + assert "--deny" in text + assert "--show" in text + + +def test_the_announcement_says_what_the_deadline_means(tmp_path): + """A silent expiry reads as a planner failure rather than an unanswered question.""" + assert "unapproved" in describe_request(REQUEST, tmp_path) + + +def test_an_empty_proposal_still_announces_something_answerable(tmp_path): + text = describe_request({"fingerprint": "f", "nodes": [], "timeout_seconds": 5}, tmp_path) + assert "0 nodes" in text + assert f"grapharc approve {tmp_path}" in text + + +def test_the_announcement_needs_no_cli_imports(tmp_path): + """`approval_file` is the headless half; the CLI decorates, it does not format. + + A styling import here would make the file handshake — which any process + that can reach the directory may drive — depend on the CLI package. + """ + import grapharc.planner.approval_file as module + + source = module.__file__ + assert source is not None + text = open(source, encoding="utf-8").read() + assert "grapharc.cli" not in text diff --git a/tests/test_config.py b/tests/test_config.py index 079ccac..4d10768 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -372,6 +372,23 @@ def test_an_explicit_policy_still_reports_its_own_layer(project, capsys): assert payload["sources"]["policy"] == "flag" +def test_go_is_governed_by_the_config_files_policy(project, capsys): + """`go` re-admits the saved plan through the gate, so the policy the file + names must govern there too. It resolved `model`, `tenant` and + `max_tokens` from the config but never `policy`: the plan was admitted + under the operator's document and then executed under the registry + default.""" + _, planned = _plan_payload(capsys) + run_dir = str(Path(planned["trace"]).parent) + + code = main(["go", run_dir, "--json"]) + payload = json.loads(capsys.readouterr().out) + + assert code == 0 + assert payload["policy_source"] == "flag-or-config" + assert "deny.toml" in payload["policy"] + + def test_the_demo_command_reads_the_config_too(tmp_path, monkeypatch, capsys): """`memory` and `reviewer_model` were declared in KEYS and read by nothing, so the same file made `plan` fail on a bad model and left `demo` scripted.""" diff --git a/tests/test_graph_viewmodel.py b/tests/test_graph_viewmodel.py index 6ddc552..7660335 100644 --- a/tests/test_graph_viewmodel.py +++ b/tests/test_graph_viewmodel.py @@ -160,6 +160,28 @@ def test_an_open_agent_node_reports_its_live_sub_step_tokens(tmp_path): assert node.live_tokens == 340 +def test_a_rerunning_node_does_not_recount_its_closed_executions(tmp_path): + """`live_tokens` is the still-open execution's spend. A node that ran + before has its earlier sub-steps already inside `tokens` via their `end` + event; scanning every sub-event again displayed that spend twice.""" + trace = TraceRecorder(tmp_path / "t.jsonl") + trace.event( + run_id="r1", graph="agent", node="topology", phase="topology", step=0, + state_delta={"nodes": ["worker"], "edges": [["__start__", "worker", "static"]]}, + ) + trace.event(run_id="r1", graph="agent", node="worker", phase="start", step=1) + trace.event(run_id="r1", graph="agent", node="worker:model", phase="model", step=2, tokens=100) + trace.event(run_id="r1", graph="agent", node="worker", phase="end", step=1, + duration_ms=3.0, tokens=100) + trace.event(run_id="r1", graph="agent", node="worker", phase="start", step=3) + trace.event(run_id="r1", graph="agent", node="worker:model", phase="model", step=4, tokens=30) + view = build_graph_view(replay(trace, "r1")) + node = next(n for n in view.nodes if n.label == "worker") + assert node.status == "running" + assert node.tokens == 100 + assert node.live_tokens == 30 + + def test_planless_planner_run_is_an_honest_empty(tmp_path): trace = TraceRecorder(tmp_path / "t.jsonl") trace.event(run_id="r1", graph="loop", node="planner", phase="plan", step=1) diff --git a/tests/test_mcp_gate.py b/tests/test_mcp_gate.py new file mode 100644 index 0000000..accffa7 --- /dev/null +++ b/tests/test_mcp_gate.py @@ -0,0 +1,170 @@ +"""The MCP supervision surface — `grapharc.mcp`. + +The property under test is the one the module refuses to compromise: a +supervised agent may request and may check, and can never decide. Everything +else is the thin-shim contract — the server drives the tested CLI, confines +what a client may name, honours the plan's own mutating verdict fail-closed, +and keeps stdout for the protocol. +""" + +from __future__ import annotations + +import json +import threading +import time +from pathlib import Path + +import pytest + +from grapharc.cli.main import main +from grapharc.mcp import FORBIDDEN_TOOL_WORDS, build_server +from grapharc.mcp.driver import ( + DriverError, + confine_run_dir, + plan_is_mutating, + read_plan_record, +) +from grapharc.planner.approval_file import REQUEST_FILENAME + + +def _unwrap(raw) -> dict: + """FastMCP's call_tool result as the tool's own dict, across SDK shapes.""" + if isinstance(raw, tuple) and len(raw) == 2 and isinstance(raw[1], dict): + structured = raw[1] + # Structured output arrives either as the dict itself or under "result". + return structured.get("result", structured) + blocks = raw[0] if isinstance(raw, tuple) else raw + text = "".join(getattr(block, "text", "") for block in blocks) + return json.loads(text) + + +@pytest.mark.asyncio +async def test_the_surface_is_three_tools_and_no_approval_verb(tmp_path): + """A client that could call approve() would be approving its own + proposal, which is not approval. The refusal is a property, not prose.""" + server = build_server(tmp_path) + tools = await server.list_tools() + + names = {tool.name for tool in tools} + assert names == {"plan", "show_graph", "execute"} + for name in names: + for word in FORBIDDEN_TOOL_WORDS: + assert word not in name.lower() + + +@pytest.mark.asyncio +async def test_the_plan_tool_offers_no_registry_policy_or_model_parameter(tmp_path): + """Those resolve from the operator's grapharc.toml in the server's root; + the requester's call must not be able to widen what the operator set.""" + server = build_server(tmp_path) + plan_tool = next(t for t in await server.list_tools() if t.name == "plan") + + parameters = set((plan_tool.inputSchema or {}).get("properties", {})) + assert parameters <= {"goal", "scripted", "max_rounds"} + + +@pytest.mark.asyncio +async def test_plan_show_execute_scripted_end_to_end(tmp_path, capsys): + """The whole supervised flow, spend-free: propose, read the shape back, + execute the read-only plan on the spot, and read the record of the run. + The server itself writes nothing to stdout — the protocol owns it.""" + server = build_server(tmp_path) + + planned = _unwrap(await server.call_tool("plan", {"goal": "investigate", "scripted": True})) + assert planned["stop"] == "planned" + assert planned["proposal"]["nodes"], "the admitted shape travels as data" + assert planned["fingerprint"] + assert planned["mutating"] is False # the incident replan admits no deploy + run_dir = planned["run_dir"] + + shown = _unwrap(await server.call_tool("show_graph", {"run_dir": run_dir})) + assert shown["status"] == "planned" + assert shown["awaiting_approval"] is False + + done = _unwrap(await server.call_tool("execute", {"run_dir": run_dir})) + assert done["executed"] is True # read-only: no park, the host prompt sufficed + + after = _unwrap(await server.call_tool("show_graph", {"run_dir": run_dir})) + assert after["status"] == "done" + assert after["executed_run_id"] + assert "mermaid" in after and after["metrics"]["nodes_executed"] + + assert capsys.readouterr().out == "" + + +def _mark_mutating(root: Path, run_dir: str) -> Path: + """Flip the record's verdict, resolving the CLI's root-relative run_dir.""" + resolved = Path(run_dir) if Path(run_dir).is_absolute() else root / run_dir + plan_file = resolved / "plan.json" + record = json.loads(plan_file.read_text()) + record["mutating"] = True + plan_file.write_text(json.dumps(record, indent=2) + "\n") + return resolved + + +@pytest.mark.asyncio +async def test_a_mutating_plan_parks_and_a_timeout_leaves_it_unexecuted(tmp_path): + server = build_server(tmp_path) + planned = _unwrap(await server.call_tool("plan", {"goal": "fix", "scripted": True})) + run_dir = _mark_mutating(tmp_path, planned["run_dir"]) + + outcome = _unwrap( + await server.call_tool("execute", {"run_dir": str(run_dir), "approval_timeout": 0.8}) + ) + + assert outcome["executed"] is False + assert outcome["stop"] == "approval_timeout" + assert "grapharc approve" in outcome["approve_command"] + assert "executed_run_id" not in json.loads((run_dir / "plan.json").read_text()) + + +@pytest.mark.asyncio +async def test_a_mutating_plan_executes_after_an_out_of_band_approval(tmp_path): + """The park is the request and the human's CLI is the decision — the MCP + connection never carries a yes.""" + server = build_server(tmp_path) + planned = _unwrap(await server.call_tool("plan", {"goal": "fix", "scripted": True})) + run_dir = _mark_mutating(tmp_path, planned["run_dir"]) + + def answer(): + deadline = time.monotonic() + 20 + while time.monotonic() < deadline: + if (run_dir / REQUEST_FILENAME).exists(): + assert main(["approve", str(run_dir), "--json"]) == 0 + return + time.sleep(0.05) + + thread = threading.Thread(target=answer) + thread.start() + outcome = _unwrap( + await server.call_tool("execute", {"run_dir": str(run_dir), "approval_timeout": 20}) + ) + thread.join() + + assert outcome["executed"] is True + + +def test_a_plan_record_without_the_verdict_reads_as_mutating(tmp_path): + """Absent is never safe: an old plan.json predates the field, and the + driver must park it rather than assume it read-only.""" + assert plan_is_mutating({}) is True + assert plan_is_mutating({"mutating": "false"}) is True # a string is not a verdict + assert plan_is_mutating({"mutating": False}) is False + + +def test_a_run_dir_outside_the_root_is_refused(tmp_path): + (tmp_path / "inside").mkdir() + assert confine_run_dir(tmp_path, "inside") == (tmp_path / "inside").resolve() + + with pytest.raises(DriverError) as refusal: + confine_run_dir(tmp_path, "/etc") + assert "outside" in str(refusal.value) + + with pytest.raises(DriverError): + confine_run_dir(tmp_path, "../elsewhere") + + +def test_a_directory_without_a_plan_names_the_missing_step(tmp_path): + with pytest.raises(DriverError) as refusal: + read_plan_record(tmp_path) + assert "call plan first" in str(refusal.value) diff --git a/tests/test_replay.py b/tests/test_replay.py index 69b2c21..71d935d 100644 --- a/tests/test_replay.py +++ b/tests/test_replay.py @@ -993,8 +993,34 @@ def test_rate_card_matches_the_longest_prefix(trace): assert RateCard().rate_for("anything") is None +def test_a_stamped_error_events_tokens_are_counted_and_priced(trace): + """The kernel stamps `error` terminals with the node's spend and + `metrics.summarize` counts them; the cost report skipped the whole + execution — a run stopped *for overspending* was billed as nearly free + while claiming to be complete.""" + trace.event(run_id="r1", graph="demo", node="a", phase="start", step=1) + trace.event(run_id="r1", graph="demo", node="a:model", phase="model", step=2, + tokens=100) + trace.event(run_id="r1", graph="demo", node="a", phase="error", step=1, + duration_ms=5.0, error="budget: tokens", tokens=150) + trace.event(run_id="r1", graph="demo", node="b", phase="start", step=3) + trace.event(run_id="r1", graph="demo", node="b", phase="end", step=3, + duration_ms=2.0, tokens=50) + + cost = attribute(trace, "r1", rates=RateCard(default=1.0)) + + assert cost.tokens == summarize(trace, "r1").tokens == 200 + assert cost.node("a").tokens == 150 + assert cost.errors == 1 + assert cost.estimated_cost_usd == pytest.approx(0.2) + assert cost.unpriced_tokens == 0 and cost.complete + assert cost.tokens_before_error == 0, "the stamp already includes the sub-steps" + + def test_tokens_spent_inside_a_failed_node_are_reported_separately(trace): - """The kernel's error event carries no tokens; the spend must not vanish.""" + """An error event with no token count (an older or hand-built producer): + the sub-steps' spend must not vanish, and must not be folded into the + total that matches `metrics`.""" trace.event(run_id="r1", graph="demo", node="agent", phase="start", step=1) trace.event(run_id="r1", graph="demo", node="agent:model", phase="model", step=2, tokens=700) diff --git a/tests/test_slack_gateway.py b/tests/test_slack_gateway.py index f8f3b82..70efc41 100644 --- a/tests/test_slack_gateway.py +++ b/tests/test_slack_gateway.py @@ -329,11 +329,11 @@ def test_a_nul_byte_is_a_refusal_not_an_exception(tmp_path): reply = handle_text("trace a\x00b", config) assert "NUL byte" in reply - # `_confined` keeps the guarantee for any other caller of its own. - from grapharc.slack.command import _confined + # `confine_path` keeps the guarantee for any other caller of its own. + from grapharc.slack.command import confine_path with pytest.raises(SlackCommandError, match="NUL byte"): - _confined("a\x00b", tmp_path) + confine_path("a\x00b", tmp_path) def test_a_single_dash_token_is_refused_as_a_flag_not_taken_as_a_path(tmp_path): diff --git a/tests/test_slack_live.py b/tests/test_slack_live.py index c0fd0c1..fc3536d 100644 --- a/tests/test_slack_live.py +++ b/tests/test_slack_live.py @@ -54,7 +54,7 @@ def test_only_events_after_the_start_offset_render(tmp_path): _write_run(TraceRecorder(path), "old-run") seen: list[str] = [] - with LiveTail(path, ["agent", "task"], lambda t: seen.append(t) or True, FAST): + with LiveTail(path, ["agent", "task"], lambda t, p=None: seen.append(t) or True, FAST): _write_run(TraceRecorder(path), "new-run") assert _wait_for(lambda: seen) assert all("old-run" not in t for t in seen), "the old run leaked into live progress" @@ -65,7 +65,7 @@ def test_no_update_without_new_bytes(tmp_path): TraceRecorder(path) # creates the parent; file itself absent seen: list[str] = [] - with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + with LiveTail(path, ["run", "g.toml"], lambda t, p=None: seen.append(t) or True, FAST): time.sleep(0.2) assert seen == [] @@ -73,7 +73,7 @@ def test_no_update_without_new_bytes(tmp_path): def test_identical_renders_are_not_reposted(tmp_path): path = tmp_path / "trace.jsonl" seen: list[str] = [] - with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + with LiveTail(path, ["run", "g.toml"], lambda t, p=None: seen.append(t) or True, FAST): _write_run(TraceRecorder(path), "r1") assert _wait_for(lambda: seen) count = len(seen) @@ -85,7 +85,7 @@ def test_a_dead_sink_silences_the_tail_without_raising(tmp_path): path = tmp_path / "trace.jsonl" calls: list[str] = [] - def dead(text: str) -> bool: + def dead(text: str, prompt=None) -> bool: calls.append(text) return False @@ -101,7 +101,7 @@ def dead(text: str) -> bool: def test_an_update_that_raises_never_escapes_the_thread(tmp_path): path = tmp_path / "trace.jsonl" - def explode(text: str) -> bool: + def explode(text: str, prompt=None) -> bool: raise RuntimeError("sink blew up") with LiveTail(path, ["run", "g.toml"], explode, FAST): @@ -113,7 +113,7 @@ def explode(text: str) -> bool: def test_a_torn_final_line_is_left_for_the_next_tick(tmp_path): path = tmp_path / "trace.jsonl" seen: list[str] = [] - with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + with LiveTail(path, ["run", "g.toml"], lambda t, p=None: seen.append(t) or True, FAST): _write_run(TraceRecorder(path), "r1") with path.open("a", encoding="utf-8") as f: f.write('{"ts": "2026-01-01T00:00:00.000+00:00", "run_id": "r1"') # no newline @@ -225,7 +225,7 @@ def post(self, text: str): self.posted.append(text) return ("C1", "171.1") - def update(self, handle, text: str) -> bool: + def update(self, handle, text: str, blocks=None) -> bool: if not self.updates_ok: return False assert handle == ("C1", "171.1") @@ -371,7 +371,7 @@ def test_a_replaced_trace_file_does_not_silence_the_tailer(tmp_path): _write_run(TraceRecorder(path), "big-old-run", nodes=6) # a large file seen: list[str] = [] - with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + with LiveTail(path, ["run", "g.toml"], lambda t, p=None: seen.append(t) or True, FAST): path.unlink() _write_run(TraceRecorder(path), "fresh-run", nodes=1) # smaller file assert _wait_for(lambda: any("fresh-run" not in t or True for t in seen) and seen) diff --git a/tests/test_slack_supervision.py b/tests/test_slack_supervision.py new file mode 100644 index 0000000..1efda27 --- /dev/null +++ b/tests/test_slack_supervision.py @@ -0,0 +1,553 @@ +"""Supervised work from Slack: propose a graph, show it, run it only if approved. + +The chain under test is the one a person actually uses from a phone: + + /grapharc plan "" --go + -> the gate forces --approve, so nothing executes unattended + -> the run parks and the bot renders the *proposed graph* into the message + -> Approve / Deny buttons answer the file handshake by fingerprint + -> the same run then executes what was approved, in one command + +Nothing here needs Slack, a token, or a model: the sink is a list, the buttons +are `handle_approval_action` called directly, and the planner is the shipped +scripted stand-in. The end-to-end tests do run the real CLI subprocess, because +the parking and the execution are the behaviour being claimed. +""" + +from __future__ import annotations + +import json +import threading +import time +from pathlib import Path + +import pytest + +from grapharc.observe.replay import replay +from grapharc.observe.trace import TailRecorder, TraceRecorder +from grapharc.planner.approval_file import write_request +from grapharc.slack.bot import ( + APPROVE_ACTION, + DENY_ACTION, + approval_blocks, + handle_approval_action, + handle_text_live, +) +from grapharc.slack.command import ( + APPROVAL_WAIT_CAP_SECONDS, + DEFAULT_PLAN_REGISTRY, + SlackCommandError, + effective_timeout, + mutating_kinds_for, + parse_command, +) +from grapharc.slack.config import SlackBotConfig +from grapharc.slack.live import ( + ApprovalPrompt, + pending_approval_prompt, + plan_lines, + render_progress, +) + + +def _config(tmp_path, **overrides) -> SlackBotConfig: + values = dict( + bot_token="xoxb-x", + app_token="xapp-x", + workdir=tmp_path, + timeout_seconds=60.0, + work_timeout_seconds=180.0, + live_interval_seconds=0.05, + allow_model=True, + allow_agent=True, + ) + values.update(overrides) + return SlackBotConfig(**values) + + +def _parse(text: str, tmp_path, **kwargs) -> list[str]: + values = dict( + workdir=tmp_path, + allow_model=True, + allow_agent=True, + timeout_seconds=60.0, + work_timeout_seconds=180.0, + ) + values.update(kwargs) + return parse_command(text, **values) + + +def _flag_value(argv: list[str], flag: str) -> str | None: + for index, token in enumerate(argv): + if token == flag and index + 1 < len(argv): + return argv[index + 1] + return None + + +# -- the gate: executing from Slack always goes through a human --------------- + + +def test_go_is_reachable_from_slack_at_all(tmp_path): + """Without this the supervised loop had no exit. + + A parked `plan --approve` was approved by a human and then returned + "awaiting `grapharc go`" — into a subcommand this gate does not carry. The + graph could be proposed and authorised from Slack and never run from there. + """ + argv = _parse('plan "fix the flaky test" --scripted --go', tmp_path) + assert "--go" in argv + + +def test_go_forces_approve_even_when_the_requester_did_not_ask_for_it(tmp_path): + """The safety rule the rest of this file rests on. + + Anyone in the workspace can type into the bot. `--go` is the difference + between proposing a graph and running one on the host, so from Slack it + always parks first — the requester does not get to skip that by omitting + the flag. + """ + argv = _parse('plan "do the thing" --scripted --go', tmp_path) + assert "--approve" in argv + + +def test_approve_is_forced_for_every_registry_not_only_the_agent_one(tmp_path): + for registry in ( + "grapharc.examples.plan_incident:build_registry", + "grapharc.examples.plan_docs:build_registry", + ): + argv = _parse(f'plan "g" --scripted --go --registry {registry}', tmp_path) + assert "--approve" in argv, registry + + +def test_plan_without_go_is_not_forced_to_park(tmp_path): + """Proposing is not executing; only `--go` earns the gate.""" + argv = _parse('plan "g" --scripted', tmp_path) + assert "--approve" not in argv + + +# -- two budgets: a reader's two minutes is a work run's SIGKILL -------------- + + +def test_a_reader_keeps_the_short_timeout(tmp_path): + argv = _parse("metrics t.jsonl r1", tmp_path) + assert effective_timeout(argv, timeout_seconds=60.0, work_timeout_seconds=180.0) == 60.0 + + +def test_a_command_that_executes_gets_the_work_budget(tmp_path): + for text in ('plan "g" --scripted --go', "agent do-something --model mock/x"): + argv = _parse(text, tmp_path) + assert ( + effective_timeout(argv, timeout_seconds=60.0, work_timeout_seconds=180.0) == 180.0 + ), text + + +def test_a_parked_go_leaves_most_of_the_clock_for_the_work_it_authorises(tmp_path): + """The human gets a third; the run they approve gets the rest. + + A plan-only park may have half the clock — nothing runs after it. With + `--go`, saying yes is the *start* of the work, and a wait that ate the + budget would leave an approved run to be killed before its first node. + """ + argv = _parse('plan "g" --scripted --go', tmp_path) + wait = float(_flag_value(argv, "--approval-timeout")) + assert wait == pytest.approx(60.0) # 180s work budget / 3 + assert wait < 180.0 - 10.0 + + +def test_a_plan_only_park_may_have_half_the_clock(tmp_path): + argv = _parse('plan "g" --scripted --approve', tmp_path) + assert float(_flag_value(argv, "--approval-timeout")) == pytest.approx(30.0) + + +def test_the_human_wait_is_capped_however_large_the_work_budget_is(tmp_path): + """A 30-minute work budget must not be spendable entirely on nobody answering.""" + argv = _parse('plan "g" --scripted --go', tmp_path, work_timeout_seconds=7200.0) + assert float(_flag_value(argv, "--approval-timeout")) == APPROVAL_WAIT_CAP_SECONDS + + +def test_a_wait_longer_than_the_budget_is_refused_not_run(tmp_path): + """The injection only fires when the flag is absent, so a supplied one must be checked. + + A park that outlives the runner is not an `approval_timeout` the run can + report — it is a kill through the middle of the wait, and the requester + gets a truncated message instead of an answer. + """ + with pytest.raises(SlackCommandError, match="does not fit this command's budget"): + _parse('plan "g" --scripted --go --approval-timeout 100000', tmp_path) + + +def test_a_shorter_wait_than_the_default_is_the_requesters_to_choose(tmp_path): + argv = _parse('plan "g" --scripted --go --approval-timeout 25', tmp_path) + assert _flag_value(argv, "--approval-timeout") == "25" + + +def test_a_wait_that_is_not_a_number_is_a_refusal(tmp_path): + with pytest.raises(SlackCommandError, match="wants a number of seconds"): + _parse('plan "g" --scripted --go --approval-timeout soon', tmp_path) + + +def test_a_delegated_agent_ceiling_comes_from_the_work_budget(tmp_path): + """`--max-seconds` reports cleanly; the runner's timeout kills. Order matters.""" + argv = _parse("agent do-something --model mock/x", tmp_path) + assert float(_flag_value(argv, "--max-seconds")) == pytest.approx(170.0) + + +# -- which kinds change things, for the person deciding ---------------------- + + +def test_the_shipped_registries_report_their_own_mutating_kinds(tmp_path): + incident = _parse( + 'plan "g" --scripted --registry grapharc.examples.plan_incident:build_registry', tmp_path + ) + assert mutating_kinds_for(incident, tmp_path) == frozenset({"deploy"}) + + stdlib = _parse('plan "g" --registry grapharc.stdlib:build_registry --model mock/x', tmp_path) + assert mutating_kinds_for(stdlib, tmp_path) == frozenset({"apply_change"}) + + +def test_a_registry_this_process_cannot_vouch_for_is_assumed_to_mutate(tmp_path): + """`grapharc.toml` may point the CLI at any module; the bot will not import it. + + None means "assume every kind changes things" — the same fail-closed + reading `plan` takes for its own plan file. Guessing the shipped default + here would put a reassuring `·` next to a node from a registry nobody in + this process has read. + """ + (tmp_path / "grapharc.toml").write_text( + '[grapharc]\nregistry = "registry.py:build_registry"\n', encoding="utf-8" + ) + argv = _parse('plan "g" --scripted', tmp_path) + assert mutating_kinds_for(argv, tmp_path) is None + + +def test_the_configured_registry_is_read_rather_than_assumed(tmp_path): + (tmp_path / "grapharc.toml").write_text( + '[grapharc]\nregistry = "grapharc.examples.plan_docs:build_registry"\n', + encoding="utf-8", + ) + argv = _parse('plan "g" --scripted', tmp_path) + assert mutating_kinds_for(argv, tmp_path) == frozenset() + + +def test_the_gates_default_registry_matches_the_clis(tmp_path): + """Two copies of one string; this is the test that keeps them one string.""" + from grapharc.cli.plan import DEFAULT_REGISTRY + + assert DEFAULT_PLAN_REGISTRY == DEFAULT_REGISTRY + + +# -- the plan a human is shown ------------------------------------------------ + + +def _approval_event(tmp_path, **delta): + recorder = TraceRecorder(tmp_path / "t.jsonl") + payload = { + "round": 1, + "proposal_id": "p1", + "fingerprint": "abc123", + "nodes": ["look", "fix_it", "check"], + "kinds": ["investigate", "apply_change", "verify"], + "edges": [["__start__", "look"], ["look", "fix_it"], ["fix_it", "check"]], + "rationale": "read the failing test, then repair it", + "worst_case_tokens": 4200, + "worst_case_complete": True, + "goal": "fix the flaky test", + } + payload.update(delta) + recorder.event( + run_id="r1", + graph="loop", + node="loop:approval", + phase="approval_request", + step=0, + state_delta=payload, + ) + return next( + e for e in recorder.read_events("r1") if e.phase == "approval_request" + ) + + +def test_the_proposed_graph_is_in_the_message_not_behind_a_link(tmp_path): + """The person deciding is on a phone. "The graph is over there" is not an answer.""" + lines = plan_lines(_approval_event(tmp_path), mutating_kinds=frozenset({"apply_change"})) + body = "\n".join(lines) + assert "look (investigate)" in body + assert "fix_it (apply_change)" in body + assert "look → fix_it" in body + assert "read the failing test" in body + assert "4200 tok" in body + + +def test_a_node_that_can_change_files_is_marked(tmp_path): + """`fix_it` is a name a planner chose; `apply_change` is what the gate governs.""" + lines = plan_lines(_approval_event(tmp_path), mutating_kinds=frozenset({"apply_change"})) + assert any(line.strip().startswith("✎") and "fix_it" in line for line in lines) + assert any(line.strip().startswith("·") and "look" in line for line in lines) + + +def test_an_undeclared_registry_marks_every_node_and_says_why(tmp_path): + lines = plan_lines(_approval_event(tmp_path), mutating_kinds=None) + body = "\n".join(lines) + assert "assume all do" in body + assert body.count("✎") == 3 + + +def test_a_kind_equal_to_its_name_is_not_repeated(tmp_path): + event = _approval_event(tmp_path, nodes=["verify"], kinds=["verify"], edges=[]) + assert any(line.strip() == "· verify" for line in plan_lines(event)) + + +def test_an_incomplete_worst_case_says_it_is_a_lower_bound(tmp_path): + event = _approval_event(tmp_path, worst_case_complete=False) + assert "lower bound" in "\n".join(plan_lines(event)) + + +def test_an_empty_plan_reads_as_an_empty_plan_not_a_broken_renderer(tmp_path): + event = _approval_event( + tmp_path, nodes=[], kinds=[], edges=[], rationale="", worst_case_tokens=0 + ) + assert "no further work" in "\n".join(plan_lines(event)) + + +def test_a_parked_run_shows_the_plan_and_says_nothing_has_run(tmp_path): + recorder = TraceRecorder(tmp_path / "t.jsonl") + recorder.event( + run_id="r1", graph="loop", node="loop:topology", phase="topology", step=0, + state_delta={"nodes": ["look", "fix_it"], "goal": "fix the flaky test", "round": 1}, + ) + recorder.event( + run_id="r1", graph="loop", node="loop:approval", phase="approval_request", step=0, + state_delta={ + "round": 1, "proposal_id": "p1", "fingerprint": "abc123", + "nodes": ["look", "fix_it"], "kinds": ["investigate", "apply_change"], + "edges": [["look", "fix_it"]], "rationale": "why not", + "goal": "fix the flaky test", + }, + ) + text = render_progress( + replay(recorder, "r1"), + argv=["plan", "g", "--trace", "runs/t.jsonl"], + elapsed_s=3.0, + mutating_kinds=frozenset({"apply_change"}), + ) + assert "waiting for approval" in text + assert "nothing above has run yet" in text + assert "look → fix_it" in text + # Not the pending-node column: every node of a parked plan is pending, so a + # stack of ⬜ says nothing about what is being authorised. + assert "⬜" not in text + + +# -- the buttons -------------------------------------------------------------- + + +def _park(tmp_path, fingerprint="abc123") -> Path: + directory = tmp_path / "slack-runs" / "one" + directory.mkdir(parents=True) + write_request( + directory, + {"proposal_id": "p1", "fingerprint": fingerprint, "nodes": ["a"], "edges": []}, + ) + return directory + + +def _value(directory: Path, tmp_path, fingerprint="abc123") -> str: + return json.dumps( + {"dir": str(directory.relative_to(tmp_path)), "fp": fingerprint} + ) + + +def test_a_pending_run_gets_buttons_and_a_finished_one_does_not(tmp_path): + prompt = ApprovalPrompt(directory="slack-runs/one", fingerprint="abc123") + blocks = approval_blocks("the plan", prompt) + action_ids = [ + element["action_id"] for element in blocks[1]["elements"] + ] + assert action_ids == [APPROVE_ACTION, DENY_ACTION] + assert approval_blocks("done", None) is None + + +def test_approving_writes_the_decision_the_parked_run_is_waiting_for(tmp_path): + directory = _park(tmp_path) + answer = handle_approval_action( + _value(directory, tmp_path), _config(tmp_path), deny=False, actor="U123" + ) + assert "approved" in answer and "U123" in answer + decision = json.loads((directory / "approval-decision.json").read_text()) + assert decision == {"fingerprint": "abc123", "decision": "approved"} + + +def test_denying_writes_a_denial(tmp_path): + directory = _park(tmp_path) + handle_approval_action(_value(directory, tmp_path), _config(tmp_path), deny=True) + decision = json.loads((directory / "approval-decision.json").read_text()) + assert decision["decision"] == "denied" + + +def test_a_button_from_an_earlier_round_cannot_approve_the_plan_that_replaced_it(tmp_path): + """The load-bearing check. + + A parked run rewrites its request every round. A message scrolled back to + from round 1 carries round 1's fingerprint, and clicking Approve on it + would authorise a graph nobody read. + """ + directory = _park(tmp_path, fingerprint="round-two") + answer = handle_approval_action( + _value(directory, tmp_path, fingerprint="round-one"), _config(tmp_path), deny=False + ) + assert "earlier plan" in answer + assert not (directory / "approval-decision.json").exists() + + +def test_a_button_naming_a_directory_outside_the_workdir_is_refused(tmp_path): + outside = tmp_path.parent / "elsewhere" + outside.mkdir(exist_ok=True) + write_request(outside, {"fingerprint": "abc123"}) + answer = handle_approval_action( + json.dumps({"dir": "../elsewhere", "fp": "abc123"}), _config(tmp_path), deny=False + ) + assert "escapes" in answer + assert not (outside / "approval-decision.json").exists() + + +def test_a_click_on_a_run_that_already_moved_on_says_so(tmp_path): + directory = tmp_path / "slack-runs" / "gone" + directory.mkdir(parents=True) + answer = handle_approval_action( + _value(directory, tmp_path), _config(tmp_path), deny=False + ) + assert "nothing is waiting" in answer + + +def test_a_malformed_button_value_is_an_answer_not_an_exception(tmp_path): + for raw in ("", "not json", "{}", '{"dir": "x"}'): + answer = handle_approval_action(raw, _config(tmp_path), deny=False) + assert "carried nothing" in answer, raw + + +def test_the_prompt_is_relative_and_disappears_once_answered(tmp_path): + recorder = TraceRecorder(tmp_path / "t.jsonl") + argv = ["plan", "g", "--trace", "slack-runs/one/trace.jsonl"] + delta = {"round": 1, "proposal_id": "p1", "fingerprint": "abc123", "nodes": [], "edges": []} + recorder.event( + run_id="r1", graph="loop", node="loop:approval", + phase="approval_request", step=0, state_delta=delta, + ) + prompt = pending_approval_prompt(replay(recorder, "r1"), argv, tmp_path) + assert prompt == ApprovalPrompt( + directory="slack-runs/one", fingerprint="abc123", round_number=1 + ) + + recorder.event( + run_id="r1", graph="loop", node="loop:approval", + phase="approval_response", step=0, + state_delta={"round": 1, "proposal_id": "p1", "decision": "approved"}, + ) + assert pending_approval_prompt(replay(recorder, "r1"), argv, tmp_path) is None + + +def test_a_request_without_a_fingerprint_draws_no_button(tmp_path): + """An unbound Approve button is exactly what this design refuses to ship.""" + recorder = TraceRecorder(tmp_path / "t.jsonl") + recorder.event( + run_id="r1", graph="loop", node="loop:approval", phase="approval_request", + step=0, state_delta={"round": 1, "nodes": [], "edges": []}, + ) + argv = ["plan", "g", "--trace", "slack-runs/one/trace.jsonl"] + assert pending_approval_prompt(replay(recorder, "r1"), argv, tmp_path) is None + + +# -- end to end: the whole supervised loop, one Slack message ---------------- + + +class RecordingSink: + """A `LiveSink` that keeps every edit and the blocks it was drawn with.""" + + def __init__(self) -> None: + self.posted: list[str] = [] + self.updated: list[tuple[str, list | None]] = [] + self._lock = threading.Lock() + + def post(self, text: str): + self.posted.append(text) + return ("C1", "1.0") + + def update(self, handle, text: str, blocks=None) -> bool: + with self._lock: + self.updated.append((text, blocks)) + return True + + def wait_for_buttons(self, timeout=60.0): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + with self._lock: + for _text, blocks in self.updated: + if blocks: + return blocks + time.sleep(0.05) + return None + + +def test_a_slack_go_parks_shows_the_graph_is_approved_by_button_and_then_runs(tmp_path): + """The whole claim, through the real CLI subprocess. + + One Slack message proposes a graph; the run parks without executing; the + message carries the graph and two buttons; a click answers the handshake by + fingerprint; the same run then executes what was approved. + """ + sink = RecordingSink() + config = _config(tmp_path) + approved: list[str] = [] + + def click_when_parked() -> None: + blocks = sink.wait_for_buttons() + assert blocks is not None, "the parked run never offered a button" + value = blocks[1]["elements"][0]["value"] + approved.append(handle_approval_action(value, config, deny=False, actor="U9")) + + clicker = threading.Thread(target=click_when_parked, daemon=True) + clicker.start() + handle_text_live('plan "investigate the checkout outage" --scripted --go', config, sink) + clicker.join(timeout=10.0) + + assert approved and "approved" in approved[0] + + parked = [text for text, _ in sink.updated if "waiting for approval" in text] + assert parked, "the run never parked" + assert "nothing above has run yet" in parked[0] + assert "→" in parked[0], "the proposed edges were not shown" + + final_text, final_blocks = sink.updated[-1] + assert final_blocks is None, "a finished run must not keep a live Approve button" + + # The trace is the record: asked, answered, then executed. + trace = next(tmp_path.glob("slack-runs/*/trace.jsonl")) + recorder = TailRecorder(trace) + run_id = recorder.run_ids()[-1] + phases = [e.phase for e in recorder.read_events(run_id)] + assert phases.index("approval_request") < phases.index("approval_response") + assert phases.index("approval_response") < phases.index("start"), ( + "a node started before the approval was answered" + ) + + +def test_a_denied_slack_go_never_starts_a_node(tmp_path): + sink = RecordingSink() + config = _config(tmp_path) + + def click_when_parked() -> None: + blocks = sink.wait_for_buttons() + assert blocks is not None + handle_approval_action(blocks[1]["elements"][1]["value"], config, deny=True) + + clicker = threading.Thread(target=click_when_parked, daemon=True) + clicker.start() + handle_text_live('plan "investigate the checkout outage" --scripted --go', config, sink) + clicker.join(timeout=10.0) + + trace = next(tmp_path.glob("slack-runs/*/trace.jsonl")) + recorder = TailRecorder(trace) + run_id = recorder.run_ids()[-1] + events = list(recorder.read_events(run_id)) + assert any(e.phase == "approval_response" for e in events) + assert not any(e.phase == "start" for e in events), "a denied plan executed" diff --git a/tests/test_supervised_path_fixes.py b/tests/test_supervised_path_fixes.py new file mode 100644 index 0000000..04572c6 --- /dev/null +++ b/tests/test_supervised_path_fixes.py @@ -0,0 +1,350 @@ +"""Regressions for the five bugs on the supervised-Claude-Code path. + +Each of these was reachable from one Slack message — propose a graph, approve +it, let Claude Code do the work — and each failed quietly rather than loudly: +a run billed at zero, a phase's output dropped, a live page that closed while +the graph was still running, a generated policy that permitted the one kind +worth denying, and a delegated agent spawned only to be killed. +""" + +from __future__ import annotations + +import pytest + +from grapharc.observe.cost import attribute +from grapharc.observe.replay import replay +from grapharc.observe.trace import TraceRecorder + +# -- a delegated run's bill --------------------------------------------------- + + +def test_a_delegated_runs_cost_is_billed_not_dropped(tmp_path): + """`--executor claude-cli` reports its spend on a `stop` event, not a `model` one. + + `grapharc agent` drives an `AgentNode` with no enclosing graph, so every + event it writes is an orphan. Attributing orphan cost only from `model` + events billed the whole run at $0.00 — and left `unpriced_tokens` at zero + too, so nothing said the figure was incomplete. + """ + recorder = TraceRecorder(tmp_path / "t.jsonl") + recorder.event( + run_id="r1", graph="agent", node="fix", phase="model", step=1, + state_delta={"executor": "delegated"}, + ) + recorder.event( + run_id="r1", graph="agent", node="fix", phase="stop", step=1, + tokens=4200, cost_usd=0.0731, + state_delta={"executor": "delegated", "turns": 6}, + ) + run = replay(recorder, "r1") + cost = attribute(run) + assert cost.recorded_cost_usd == pytest.approx(0.0731) + assert cost.tokens == 4200 + # And the two readers of one trace now agree. + assert cost.recorded_cost_usd == pytest.approx(run.recorded_cost_usd) + + +def test_a_stop_event_is_not_counted_as_a_model_call(tmp_path): + """Billing it must not turn it into a row in the model-call breakdown.""" + recorder = TraceRecorder(tmp_path / "t.jsonl") + recorder.event( + run_id="r1", graph="agent", node="fix", phase="stop", step=1, + tokens=10, cost_usd=0.5, + ) + cost = attribute(replay(recorder, "r1")) + assert cost.model_calls == [] + assert cost.recorded_cost_usd == pytest.approx(0.5) + + +# -- a curtailed phase's output ---------------------------------------------- + + +def test_a_curtailed_phase_reports_what_it_managed_to_say(tmp_path): + """`AgentResult.output` is empty by contract for every reason but TARGET_MET. + + Reading it for a curtailed phase formatted "[budget_exhausted] " with + nothing after it: the work the phase did manage was dropped, and a + downstream goal check saw an empty note rather than a truncated one. + """ + from grapharc.harness.agent import AgentResult, StopReason + from grapharc.stdlib import _agent_factory + + class _Node: + def run(self, _prompt, _ctx): + return AgentResult( + output="", + partial_output="I read tests/test_x.py and found the seed is unset", + termination_reason=StopReason.BUDGET_EXHAUSTED, + note="max_tokens reached (900/900)", + ) + + factory = _agent_factory(object(), lambda tools: object(), "investigate") + import grapharc.harness as harness_module + + original = harness_module.AgentNode + harness_module.AgentNode = lambda *a, **k: _Node() # type: ignore[assignment] + try: + body = factory(None) + from grapharc.stdlib import WorkState + + written = body(WorkState(goal="g"), None) + finally: + harness_module.AgentNode = original # type: ignore[assignment] + + line = written["findings"][0] + assert "budget_exhausted" in line + assert "the seed is unset" in line, "the phase's own text was dropped" + + +def test_a_curtailed_phase_with_no_prose_falls_back_to_its_reason(tmp_path): + from grapharc.harness.agent import AgentResult, StopReason + from grapharc.stdlib import WorkState, _agent_factory + + class _Node: + def run(self, _prompt, _ctx): + return AgentResult( + termination_reason=StopReason.ERROR, note="tool raised RuntimeError" + ) + + import grapharc.harness as harness_module + + original = harness_module.AgentNode + harness_module.AgentNode = lambda *a, **k: _Node() # type: ignore[assignment] + try: + written = _agent_factory(object(), lambda tools: object(), "verify")(None)( + WorkState(goal="g"), None + ) + finally: + harness_module.AgentNode = original # type: ignore[assignment] + assert "RuntimeError" in written["findings"][0] + + +def test_a_run_whose_only_note_is_an_error_has_not_met_its_goal(tmp_path): + """Issue #96, the half that made the other half invisible. + + `goal_met` counted notes, so a run whose entire output was `['[error] ']` + stopped `goal_met` — reporting success for a run that produced nothing. + An agent node's failure is folded into state as prose, and a length check + cannot tell a report from an apology. + """ + from grapharc.stdlib import WorkState, curtailed_note, goal_met + + failed = WorkState(goal="g", notes=[curtailed_note("error", "the tool exploded")]) + assert not goal_met(failed) + + # The empty-message form the issue actually observed. + assert not goal_met(WorkState(goal="g", notes=[curtailed_note("error", "")])) + + +def test_a_real_report_still_meets_the_goal(tmp_path): + from grapharc.stdlib import WorkState, goal_met + + assert goal_met(WorkState(goal="g", notes=["here is what happened"])) + # One good note among failures is still an outcome. + assert goal_met(WorkState(goal="g", notes=["[error] nope", "the real summary"])) + + +def test_curtailed_is_read_off_the_stop_reasons_not_guessed(tmp_path): + """Prose that merely starts with a bracket is not a stop reason.""" + from grapharc.stdlib import WorkState, goal_met, is_curtailed + + assert is_curtailed("[budget_exhausted] ran out") + assert not is_curtailed("[NOTE] the config is fine") + assert goal_met(WorkState(goal="g", notes=["[NOTE] the config is fine"])) + + +# -- the live page during a multi-phase run ---------------------------------- + + +def _phase(recorder, node, step, *, sub_stop=True): + """One graph node that internally drives an agent loop.""" + recorder.event(run_id="r1", graph="work", node=node, phase="start", step=step) + if sub_stop: + # The AgentNode's own terminal event, inside the node's span. + recorder.event( + run_id="r1", graph="work", node=node, phase="stop", step=step, + state_delta={"executor": "delegated"}, + ) + recorder.event( + run_id="r1", graph="work", node=node, phase="end", step=step, duration_ms=5.0 + ) + + +def test_the_live_page_stays_open_while_later_phases_still_have_to_run(tmp_path): + """Every agent phase writes a `stop`; only the driver's ends the run. + + Keying on any `stop` declared a three-phase graph finished the moment the + first phase ended, and closed the SSE stream on a page with two nodes left. + """ + from grapharc.server.live import build_snapshot + + recorder = TraceRecorder(tmp_path / "t.jsonl") + _phase(recorder, "investigate", 1) + recorder.event(run_id="r1", graph="work", node="apply_change", phase="start", step=2) + + snapshot = build_snapshot(tmp_path, "t.jsonl", "r1") + assert not snapshot.done, "the run was declared finished with a node still open" + + +def test_a_run_with_no_graph_still_finishes_on_its_own_stop(tmp_path): + """`grapharc agent` has no node spans, so its stop is an orphan — and terminal.""" + from grapharc.server.live import build_snapshot + + recorder = TraceRecorder(tmp_path / "t.jsonl") + recorder.event(run_id="r1", graph="agent", node="fix", phase="model", step=1) + recorder.event(run_id="r1", graph="agent", node="fix", phase="stop", step=1) + + snapshot = build_snapshot(tmp_path, "t.jsonl", "r1") + assert snapshot.done + + +def test_a_termination_reason_still_ends_the_run(tmp_path): + from grapharc.server.live import build_snapshot + + recorder = TraceRecorder(tmp_path / "t.jsonl") + recorder.event(run_id="r1", graph="work", node="n", phase="start", step=1) + recorder.event( + run_id="r1", graph="work", node="n", phase="end", step=1, + state_delta={"termination_reason": "goal_met"}, + ) + snapshot = build_snapshot(tmp_path, "t.jsonl", "r1") + assert snapshot.done + + +# -- the generated policy ---------------------------------------------------- + + +class _CapturingModel: + """Records the prompt it was asked, answers with a permissive document.""" + + def __init__(self) -> None: + self.prompt = "" + + def invoke(self, prompt): + self.prompt = prompt + return "[[rules]]\naction = 'allow'\ntarget = '*'\n" + + +def test_an_undeclared_registry_has_every_kind_treated_as_dangerous(tmp_path): + """`None` means nobody said, which is not evidence of safety. + + Substituting stdlib's `MUTATING_KINDS` named `apply_change` — a kind this + registry does not have — while its real mutating kind went unnamed, and the + generated policy was then cached and governed every later run. + """ + from grapharc.cli.generate import resolve_or_generate_policy + + model = _CapturingModel() + resolve_or_generate_policy( + None, + tenant="default", + model=model, + goal="ship it", + catalog={"survey": "read things", "publish": "push to prod"}, + mutating=None, + workdir=tmp_path, + write=False, + ) + dangerous = model.prompt.split("treat as dangerous):")[1].splitlines()[0] + assert "publish" in dangerous and "survey" in dangerous + assert "apply_change" not in dangerous, "a kind from some other registry was named" + + +def test_a_registry_that_declares_nothing_mutates_is_believed(tmp_path): + """`()` is a declaration, not an absence — it must not be overwritten.""" + from grapharc.cli.generate import resolve_or_generate_policy + + model = _CapturingModel() + resolve_or_generate_policy( + None, + tenant="default", + model=model, + goal="read the docs", + catalog={"survey": "read things", "summarise": "write a summary"}, + mutating=(), + workdir=tmp_path, + write=False, + ) + dangerous = model.prompt.split("treat as dangerous):")[1].splitlines()[0] + assert "(none)" in dangerous + + +def test_a_declared_mutating_tuple_is_passed_through(tmp_path): + from grapharc.cli.generate import resolve_or_generate_policy + + model = _CapturingModel() + resolve_or_generate_policy( + None, + tenant="default", + model=model, + goal="fix it", + catalog={"survey": "read", "deploy": "ship"}, + mutating=("deploy",), + workdir=tmp_path, + write=False, + ) + dangerous = model.prompt.split("treat as dangerous):")[1].splitlines()[0] + assert "deploy" in dangerous and "survey" not in dangerous + + +# -- delegating with no time left -------------------------------------------- + + +# -- the workspace a delegated run is given ---------------------------------- + + +def test_a_harness_keeps_the_workspace_it_was_given(tmp_path): + """`workspace=` used to be discarded whenever an explicit executor was passed. + + It existed only to construct the default `SandboxedExecutor`, so + `Harness(..., executor=LocalExecutor(), workspace=ws)` — which several call + sites in this repo write — silently dropped `ws`. + """ + from grapharc.harness import Harness, LocalExecutor, PermissionPolicy, ToolRegistry + + harness = Harness( + ToolRegistry(), + PermissionPolicy(rules=[]), + executor=LocalExecutor(), + workspace=str(tmp_path), + ) + assert harness.workspace == str(tmp_path) + + +def test_a_sandboxed_harness_still_reports_its_executors_workspace(tmp_path): + from grapharc.harness import Harness, PermissionPolicy, ToolRegistry + + harness = Harness(ToolRegistry(), PermissionPolicy(rules=[]), workspace=str(tmp_path)) + assert harness.workspace == str(tmp_path) + assert harness.executor.workspace == str(tmp_path) + + +def test_the_stdlib_harness_names_a_workspace_so_claude_code_can_be_delegated_to(tmp_path): + """The stdlib registry driven by the Claude CLI *is* a delegated run. + + Every agent kind in this registry delegates its whole loop to Claude Code + when the backend has no tool-calling wire format — that is what the module + docstring describes. Its harness used `LocalExecutor`, which has no + workspace, so `_run_delegated` refused before spawning anything and every + agent phase of every such run failed with "does not expose one". + """ + from grapharc.stdlib import default_harness + + harness = default_harness(("read_file",), workspace=tmp_path) + assert harness.workspace == str(tmp_path) + + +def test_an_exhausted_deadline_does_not_spawn_claude_code(tmp_path): + """`remaining_seconds()` goes negative once the budget is spent. + + `subprocess.run` accepts a negative timeout: it starts the child and kills + it on the first wait. So an over-budget run launched Claude Code, tore it + down mid-startup, and reported a max_seconds nobody had set. + """ + from grapharc.cli.delegate import DelegationError, delegate_task + + for remaining in (0.0, -3.2): + with pytest.raises(DelegationError) as caught: + delegate_task("do a thing", workspace=tmp_path, max_seconds=remaining) + assert caught.value.reason == "deadline_exceeded" + assert "no time left" in str(caught.value) diff --git a/tests/test_torn_trace_read.py b/tests/test_torn_trace_read.py new file mode 100644 index 0000000..330d6bb --- /dev/null +++ b/tests/test_torn_trace_read.py @@ -0,0 +1,76 @@ +"""A trace being written must stay readable to the tools that read it. + +`TraceRecorder` appends one JSON line per event, so a reader that opens the +file while a run is in progress can land on a half-written last line. That is +not a corrupt file — it is a file with one more byte coming — and the codebase +already has the answer: `TailRecorder` skips the torn line where the strict +reader raises, which is how the live view and the Slack tailer read a running +run without crashing. + +`mcp/driver.graph_status` did not use it. So the supervised agent's own status +tool — the one it calls to find out whether a human has approved yet, while +the run it is asking about is still writing — raised `TraceReadError` out of +the MCP server instead of answering. +""" + +from __future__ import annotations + +import json + +from grapharc.observe.trace import TraceRecorder + + +def _run_dir(tmp_path, *, torn: bool): + """A finished plan whose trace has (or has not) a half-written last line.""" + directory = tmp_path / "run" + directory.mkdir() + (directory / "plan.json").write_text( + json.dumps( + { + "goal": "investigate", + "fingerprint": "f1", + "executed_run_id": "r1", + "mutating": False, + "proposal": {"nodes": ["triage"], "edges": [], "rationale": "why"}, + } + ), + encoding="utf-8", + ) + trace = directory / "trace.jsonl" + recorder = TraceRecorder(trace) + recorder.event(run_id="r1", graph="g", node="triage", phase="start", step=1) + recorder.event( + run_id="r1", graph="g", node="triage", phase="end", step=1, duration_ms=4.0, tokens=11 + ) + if torn: + # Exactly what a reader sees mid-append: the writer got half a line out. + with trace.open("a", encoding="utf-8") as handle: + handle.write('{"run_id": "r1", "graph": "g", "node": "verify", "pha') + return directory + + +def test_a_torn_last_line_does_not_crash_the_status_tool(tmp_path): + """The failure this file exists for. + + A supervised agent polls `show_graph` to learn whether its plan has been + approved. Polling *while the run writes* is the whole point of polling, so + the one moment this must not raise is the one moment it did. + """ + from grapharc.mcp.driver import graph_status + + view = graph_status(_run_dir(tmp_path, torn=True)) + + assert view["status"] == "done" + assert view["metrics"]["tokens"] == 11 + assert "triage" in view["mermaid"] + + +def test_an_intact_trace_reads_exactly_as_before(tmp_path): + """The fix must not be "skip lines and hope" — a whole file is unchanged.""" + from grapharc.mcp.driver import graph_status + + view = graph_status(_run_dir(tmp_path, torn=False)) + + assert view["status"] == "done" + assert view["metrics"]["tokens"] == 11 + assert view["metrics"]["nodes_executed"] == 1 diff --git a/uv.lock b/uv.lock index 7ffa97c..d7db3bf 100644 --- a/uv.lock +++ b/uv.lock @@ -406,6 +406,7 @@ slack = [ [package.dev-dependencies] dev = [ { name = "httpx" }, + { name = "mcp" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-timeout" }, @@ -437,6 +438,7 @@ provides-extras = ["openrouter", "openai", "ollama", "server", "mcp", "ladybug", [package.metadata.requires-dev] dev = [ { name = "httpx", specifier = ">=0.27" }, + { name = "mcp", specifier = ">=1.2" }, { name = "pytest", specifier = ">=8.2" }, { name = "pytest-asyncio", specifier = ">=0.24" }, { name = "pytest-timeout", specifier = ">=2.3" },