mcp: the execute timeout no longer severs an approved mutating run

[Shashankss1205]

Closes #113.

The issue names three defects on the MCP execute path. All three are fixed here, each with a test that is red without its fix.

1. One budget covered both the park and the work

timeout=approval_timeout + 120.0 if mutating else None bounded the whole subprocess, so a human who approved near the end of the park left roughly 120 seconds for the run itself — and a governed run's agent phases delegate to Claude Code, which reads files, edits them and verifies. The likely outcome was not a wedged process being cleaned up; it was a human-approved, tree-mutating run being SIGKILLed partway through mutating the tree. The non-mutating branch passed timeout=None and so was not bounded at all.

The park and the work now have separate budgets. DEFAULT_WORK_TIMEOUT is 1800s, matching GRAPHARC_SLACK_WORK_TIMEOUT — the Slack path carved exactly this case out of one shared timeout for the same reason, and its comment already said why. Overridable via GRAPHARC_MCP_WORK_TIMEOUT, which refuses a non-numeric or non-positive value rather than falling back to the default: an operator who sets a ceiling has one in mind, and silently substituting a different one is how a run gets killed at a limit nobody chose.

Mutating calls are bounded at approval_timeout + work; non-mutating ones at work, so that branch is bounded at all now.

2. plan.json could not catch the reissue

executed_run_id is stamped after loop.run() returns, so a run killed partway through never reaches the stamp. The record then said "never executed", the #100 guard waved a second go through, and the plan ran again over a tree the first run had half-changed — on one human approval, which is the property #100 was closed to protect.

Rather than pre-stamping (which would mark a plan as executed when it had merely parked and been denied), the guard reads the trace. The trace is written as the run proceeds, so it is the only place that evidence survives a kill — and it is already the audit trail this project treats as authoritative. go now refuses a plan whose trace holds a run that did node-level work and which the record does not name, pointing the reader at the trace and offering --again.

The phase vocabulary this depends on is pinned by a test, and deliberately so. A node did work iff its phase is outside the loop's own bookkeeping (plan, admission, round), the viewer's shape events (topology, approval_request, approval_response) and a bare stop. Using "not a loop phase" — the split observe.metrics makes for a different purpose — would have counted approval_response as an execution and so refused a plan a human had denied, forever. That case has its own test.

A torn or unreadable trace returns "no evidence" rather than refusing: a damaged file must not wedge every plan sitting beside it.

3. The kill left grandchildren

run_cli did not start a new session and process.kill() signals the direct child only, so a delegated Claude Code process could outlive the run that spawned it and keep holding the workspace. The child is now started with start_new_session=True and the timeout path SIGKILLs the whole process group, falling back to the single process where killpg is unavailable. An already-dead group is the success case arriving early, not an error.

What the agent is told

The docstring's "A timeout leaves the plan unexecuted and this call safe to reissue" was true of an unanswered park and false of everything else. It now distinguishes the two and sends the agent to show_graph before reissuing anything; the DriverError raised on a timeout says the same. Both are asserted by tests, because that docstring is the agent's whole briefing.

Verification

• Full suite green: 2171 selected, 13 deselected, ruff check . clean, on Python 3.14.7.
• Red without the fix, confirmed by reverting only grapharc/ and re-running: the reissue test fails assert 0 == 2 — the bug reproducing, go silently executing a second time.
• test_again_still_runs_an_unfinished_plan and test_planning_paperwork_is_not_mistaken_for_a_half_run pass both before and after, which is the point of them: they guard against the fix over-refusing.
• The grandchild test spawns a real process tree and asserts the grandchild is gone, rather than asserting a mock was called.
• docs/deep-dive.md's selected-count figure updated 2,151 → 2,171, as tests/test_deep_dive.py requires.

No CHANGELOG entry, matching #109 and #112 — those entries are written in the release commit.

🤖 Generated with Claude Code

---

Follow-up commits, from reviewing the above rather than running it

The phase vocabulary now has one owner. The first version of this PR added a fourth copy of "which phases are bookkeeping" — observe.metrics, observe.viewmodel (under a comment reading "mirrors metrics", which was true and was the problem), slack.live, and the new one in cli.plan. Four modules asking the same question with four answers, read for different purposes, is a bug in whichever is wrong with nothing in the tree to say which.

LOOP_PHASES, SHAPE_PHASES, DRIVER_PHASES and began_execution() now live in observe.trace, beside the TraceEvent they classify. All four callers defer to them; every local name aliases the shared object, so no drawing or feed behaviour changes.

Two guards, because one is not enough:

• Identity, not equality — four frozensets that happen to agree today is the state being ruled out.
• A source-level check that exactly one file may define these sets, since the identity check cannot notice a fifth copy under a new name. It asserts on files rather than file:line; my first attempt used line numbers, which fails on any unrelated edit to trace.py, and it also missed DRIVER_PHASES.

The direction the predicate errs in is now asserted, not assumed. An unlisted phase reads as an execution. That is the safe way round: a new bookkeeping phase makes go refuse a plan it could have run, which --again recovers; the opposite would re-run a half-finished mutating plan and spend a human approval given once.

Bare go has a test. grapharc plan … && grapharc go is the flow the README teaches, and bare go reaches its plan by a different route — find_unexecuted_plan passes over any record carrying an executed_run_id, and a killed run never wrote one, so a half-finished plan looks unexecuted to the selector and is the newest candidate. The refusal lands after the selection, so it holds; verified against main, where the same scenario exits 0 and puts a third run in the trace. It holds for a reason a refactor of the selector could undo without touching the guard, so it is pinned.

Figure now 2,175 selected.

One thing I considered and did not do

Short-circuiting _unfinished_execution so it stops at the first qualifying event instead of parsing the whole trace. These traces are tens to low hundreds of lines, so it would buy nothing measurable and would cost a hand-rolled JSONL parse where the module currently uses TraceRecorder like everything else.