mcp: the execute timeout can SIGKILL an approved mutating run mid-mutation, then tells the agent it is safe to reissue

[Shashankss1205]

What happens

grapharc/mcp/server.py's execute tool bounds the CLI subprocess like this:

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,
)

and tells the calling agent, in the tool's own docstring:

A timeout leaves the plan unexecuted and this call safe to reissue.

That sentence is not true for a mutating plan, and the non-mutating branch has no timeout at all.

1. The mutating branch can SIGKILL a run mid-mutation

The outer budget is approval_timeout + 120s and it covers the whole subprocess, park and execution together. A human who approves near the end of the park leaves roughly 120 seconds for the work itself — and the work is a governed run whose agent phases delegate to Claude Code, which reads files, edits them and verifies. The Slack path budgets exactly this case at GRAPHARC_SLACK_WORK_TIMEOUT (default 1800s) for the same reason, and its comment says why: killing an approved run at 120s "does not protect anything, it just severs an approved run partway through its work."

So the likely outcome is not a wedged process being cleaned up. It is a human-approved, tree-mutating run being killed partway through mutating the tree.

Then run_cli raises DriverError, and the agent — which was told a timeout means the plan is unexecuted — reissues.

2. plan.json cannot catch the reissue either

#100 added a guard: go <dir> refuses a plan that already carries an executed_run_id. That guard does not fire here, because the stamp is written after loop.run() returns:

executed = any(r.executed for r in result.rounds)
if executed:
    record["executed_run_id"] = result.run_id

A SIGKILL partway through never reaches that line. So the record says "never executed", the guard waves the reissue through, and the plan runs a second time over a tree the first run had already half-changed — on one human approval, which is exactly the property #100 was closed to protect.

3. The kill leaves grandchildren

run_cli does not start a new session, and process.kill() signals the direct child only. The CLI's own deadline guard kills its process group deliberately (the deep dive says so), but that is the CLI's guard, not this outer one — so a delegated Claude Code process spawned by the killed run can outlive it, still holding the workspace.

4. The non-mutating branch never times out

timeout=None for a non-mutating plan. The comment says the bound exists to "catch a wedged process", and that is precisely the case where it is absent: a non-mutating run that wedges hangs the MCP tool call forever, with no way for the agent to recover.

Where in the code

• grapharc/mcp/server.py — the execute tool: the docstring's safety claim and the approval_timeout + 120.0 if mutating else None expression
• grapharc/mcp/driver.py — run_cli: asyncio.wait_for, process.kill(), and the DriverError text
• grapharc/cli/plan.py — where executed_run_id / executed_run_ids are stamped, after the run returns
• grapharc/slack/command.py — effective_timeout and WORK_COMMANDS, which already solved this shape for the Slack surface

What to consider

• Two budgets, not one, the way Slack already does it. The park gets approval_timeout; the work gets its own generous ceiling. One number cannot bound both, because they are bounded for opposite reasons.
• Do not claim more than the timeout can deliver. If a kill can land mid-execution, the docstring must say the plan's state is unknown and the run directory must be inspected — not that reissuing is safe. An agent acts on that sentence.
• Give the non-mutating branch a ceiling so the stated purpose of the bound is actually served.
• Kill the process group, so a delegated child does not outlive the run that spawned it.
• Worth deciding whether a killed run should leave a marker in plan.json (an interrupted_at, say) so the go <dir> re-executes an already-executed plan silently, and plan.json forgets the earlier runs #100 guard has something to refuse on. That is the durable fix for the reissue hazard, since no timeout tuning removes it.

Acceptance criteria

• No timeout can kill an approved mutating run within the budget a human was told the work would get
• If a kill can still land mid-execution, the tool says so rather than telling the agent it is safe to reissue, and a reissue is refused rather than silently re-running
• A wedged non-mutating run is bounded rather than hanging forever
• A killed run leaves no surviving delegated child
• uv run pytest green, uv run ruff check . clean