session: a session whose runner died can be reclaimed deliberately (#127)

Closes #19. A runner's claim is a compare-and-set, not a lease: it stops a
second runner from claiming a session and cannot notice one that died holding
it. The row stayed `running` with the dead `runner_pid` still on it -- so the
store knew who died and offered no way to act on it. Every later `run()` raised
`SessionBusy` forever, and the only remedy was a hand-written UPDATE, which
skips the lifecycle check and writes no transition row, corrupting the audit
trail the store exists to keep.

`SessionStore.release_dead_runner`, surfaced as `SessionManager.reclaim`.

**`failed`, not `interrupted`.** `interrupted` says a turn stopped somewhere it
can be picked up from; a runner that died left no such point. `failed` says the
turn did not settle, which is true, and is already resumable.

**Open holds survive.** `pending_approval` is left alone, so a session waiting
on a human is still waiting afterwards. Reclaiming a wedged session must not be
a way past an approval gate, and a test asserts the hold is still there.

**The liveness check runs inside `BEGIN IMMEDIATE`**, not before it, so two
operators reclaiming at once serialise on it. A check outside the write lock is
the same deferred-read race `_transaction`'s docstring already describes for
claiming.

Four refusals, each with a test: the session is not `running`; the pid is alive
(or belongs to another user, `PermissionError`, read as alive on purpose); the
pid is this process; a `running` row carries no `runner_pid` at all, which is
inconsistent in its own right and not something to paper over.

Pid reuse is not solved and is not pretended to be: a recycled pid reads as
alive, which is a refusal -- the safe direction. That is why nothing calls this
automatically. An automatic sweep would be a way for two live runners to fight
over one session, which is what the claim exists to prevent.

The crash is a real one in the tests: a child claims the session and `os._exit`s,
skipping every cleanup path, which is what a SIGKILL looks like from the store's
side. The alive-refusal test parks a real child on a file barrier rather than
inventing a pid, and the dead pid used elsewhere is spawned and reaped rather
than guessed -- a made-up number can belong to something, and the check under
test refuses a live pid.

The three honesty paragraphs this obsoletes are updated: `store.transition`'s
docstring, `session/runtime.py`'s limitations list, and the deep dive's. The
README no longer states it -- that sentence moved to the deep dive since the
issue was filed.

Verified: 2210 selected, 13 deselected, ruff clean.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Shashankss1205

docs/deep-dive.md

@@ -250,11 +250,11 @@ A stable system is not one that claims to have no edges — it is one whose edge
 - **A node's tokens are its own, not the run's movement while it ran.** Worth stating because it was the other way round: an `end` event carried the difference between two readings of the run's *shared* meter, so under fan-out the workers' windows overlapped and each was credited with its siblings' concurrent spend. Three workers costing 8 tokens each traced as 24/16/8, and `metrics` and `cost` agreed on 48 for 24 tokens of real work — doubling the estimated bill purely because the work ran in parallel. Attribution now comes from a per-node scope on the meter, so the same work costs the same serially and in parallel; a hand charge the usage callback never saw still lands on the node that made it.
 - **A planning round is an envelope, not a measurement.** A `round` event used to carry the planner's `tokens` and the round's `duration_ms`, both of which `metrics`, `cost` and `replay` add on top of node totals — and the planner's spend was already reported by its own `plan` event, so it was counted twice, and a round's duration encloses the plan plus every node it ran. Neither is on the event now; both are on its `state_delta` as `round_tokens` / `round_iterations` / `round_duration_ms`, where no reader sums them. `RoundRecord.iterations` also holds a figure now rather than always `0`.
 - **The Claude CLI backend is completion-only, and an agent node on it is *delegated* rather than governed.** The CLI has no tool-calling wire format, so GraphARC cannot run its own gated loop over it. Rather than refuse, `AgentNode` hands the whole loop to Claude Code's headless agent, in one of two named tiers. `allowlist`, the default, pre-approves exactly the Claude Code twins of the node's own registered tools (`read_file`→`Read`, … `run_command`→`Bash`), so one operator declaration governs both the governed loop and the delegated one — but the enforcement is Claude Code's own gating, not this graph's per-call policy, there are no per-tool trace events, and anything unlisted falls to headless default gating, which fails closed. `bypass`, explicit opt-in only, runs `bypassPermissions`: every tool Claude Code has, no checks at all. In either tier the calls are not confined by the sandbox executor and the token figure is the sub-agent's own rather than one GraphARC metered call by call; a `--max-tokens` the delegated path cannot enforce is refused rather than silently unapplied. The workspace boundary and the wall-clock ceiling still hold — the CLI runs in its own session, and the deadline kills the whole process group, not just the direct child. It warns on `DelegatedToolUseWarning` at construction, naming the tier, and marks every trace event `executor=delegated` with its `delegated_mode`; filter that warning to an error to get the old refusal back. Structured output still needs an OpenAI-wire backend: `openrouter`, `openai`, or a local `ollama`.
-- **A session turn is synchronous**, and a runner claim is a claim rather than a lease — nothing reclaims a session whose runner died holding it.
+- **A session turn is synchronous**, and a runner claim is a claim rather than a lease — a runner that dies holding a session does not release it by itself. `SessionManager.reclaim(session_id)` releases one deliberately, refusing unless the recorded runner pid is gone from this host; it is not automatic, because pid liveness is host-local and pids are recycled.
 - **`.env` and `grapharc.toml` follow the same discovery rule: the working directory, and nowhere else.** Neither searches parent directories — a run must not be governed by a file you did not know about, and must not be *billed* to one either. **This is a behaviour change:** the credential loader used to walk up to `/`, so a `.env` in an ancestor directory (a `$HOME` one on a shared box, a client project one above a demo checkout) was picked up silently. If you relied on that, move the file into the directory you run from, `export` the variable, or pass `env_file=` to name it explicitly. A real environment variable still beats any file.
 - **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost.
 
-**Verified this pass:** `pytest` → green, 2,205 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.8` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
+**Verified this pass:** `pytest` → green, 2,210 selected and 13 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all submodules in a clean virtualenv with `[all]`, and `0.1.8` on PyPI is that wheel. The counts are a snapshot, not a property of the project — `pytest` re-derives them in one command, which is the only reason they are quoted, and `tests/test_deep_dive.py` fails this line rather than letting it drift.
 
 [ROADMAP.md](../ROADMAP.md) tracks what is built and what is not, item by item.
 

grapharc/session/runtime.py

@@ -50,8 +50,12 @@
   stops. The kernel grew `astream` while this was being written; an async turn
   is now buildable and is simply not built yet.
 - **One runner at a time is a claim, not a lease.** `SessionStore.transition`
-  stops a second runner from claiming a session, and nothing reclaims one whose
-  runner died holding it — see that method's docstring.
+  stops a second runner from claiming a session; it cannot notice one that died
+  holding it. A session wedged in `running` after a crash is released
+  deliberately with `SessionManager.reclaim`, which refuses unless the recorded
+  `runner_pid` is gone from this host and records the release as a transition.
+  Deliberate rather than automatic: pid liveness is host-local and pids are
+  recycled, so an automatic sweep is a way for two live runners to fight.
 - **A hold names a node, not one particular task.** When a `Send` fan-out puts
   the *same* gated node on the boundary several times over, each of those tasks
   is held separately and each needs its own decision — so the count is exact
@@ -816,6 +820,25 @@ def resume(self, session_id: str) -> Session:
         record = self.store.require(session_id)
         return Session(record=record, manager=self, spec=self.registry.get(record.graph))
 
+    def reclaim(self, session_id: str, *, reason: str = "") -> SessionRecord:
+        """Release a session whose runner process died holding it.
+
+        A runner's claim is a compare-and-set rather than a lease, so a crash
+        mid-turn leaves the session `running` and every later `run()` raises
+        `SessionBusy`. This is the deliberate release: it refuses unless the
+        recorded `runner_pid` is gone from this host, moves the session to
+        `failed`, and records the reclaim as a transition naming the dead pid —
+        so the audit trail shows what happened instead of a hand-written UPDATE
+        hiding it.
+
+        Open approval holds are kept: a session waiting on a human is still
+        waiting afterwards. Nothing calls this automatically, because pid
+        liveness is host-local and pids are recycled — see
+        `SessionStore.release_dead_runner` for what it will and will not accept
+        as evidence that a runner is gone.
+        """
+        return self.store.release_dead_runner(session_id, reason=reason)
+
     def list(
         self, *, status: SessionStatus | Iterable[SessionStatus] | None = None
     ) -> list[SessionRecord]:

grapharc/session/store.py

@@ -41,6 +41,7 @@
 from grapharc.session.errors import (
     InvalidTransition,
     SessionBusy,
+    SessionError,
     SessionExistsError,
     SessionTerminated,
     ThreadInUseError,
@@ -113,6 +114,30 @@ class SessionStatus(StrEnum):
 )
 
 
+def _pid_alive(pid: int) -> bool:
+    """Whether `pid` names a live process *on this host*.
+
+    Signal 0 checks for existence without delivering anything. `PermissionError`
+    means the process exists and belongs to someone else, which counts as alive:
+    the conservative answer is the one that refuses a reclaim.
+
+    Host-local by construction, which is why the store does not try to be a
+    lease. A pid from another machine is meaningless here, and a recycled pid
+    reads as alive — a refusal, which is the direction to fail in.
+    """
+    try:
+        os.kill(pid, 0)
+    except ProcessLookupError:
+        return False
+    except PermissionError:
+        return True
+    except OSError:
+        # An unusable pid (negative, 0, out of range) is not a live runner, and
+        # 0 would signal our own process group rather than asking about a pid.
+        return False
+    return True
+
+
 class _Keep:
     """Sentinel: leave this column as it is (distinct from setting it to NULL)."""
 
@@ -417,8 +442,8 @@ def transition(
         was found instead. That is the whole of the "one runner at a time"
         guard — it stops a second runner from *claiming* a session, and it does
         not detect a runner that died holding one. A session stuck in `running`
-        after a crash has to be released deliberately, which is a legal
-        `running -> idle` transition.
+        after a crash is released by `release_dead_runner`, which checks the
+        recorded pid is gone and then makes a legal `running -> failed` move.
         """
         with self._transaction() as conn:
             row = conn.execute("SELECT * FROM sessions WHERE id = ?", (session_id,)).fetchone()
@@ -461,6 +486,93 @@ def transition(
             )
         return self.require(session_id)
 
+    def release_dead_runner(self, session_id: str, *, reason: str = "") -> SessionRecord:
+        """Release a `running` session whose runner process no longer exists.
+
+        A runner's claim is a compare-and-set, not a lease: it stops a second
+        runner from claiming a session and cannot notice one that died holding
+        it. Without this, a crash mid-turn left the row `running` forever and
+        every later `Session.run()` raised `SessionBusy`, with the only remedy
+        being a hand-written UPDATE — which skips the lifecycle check and writes
+        no transition row, corrupting the audit trail this store exists to keep.
+
+        Lands on `failed`, not `interrupted`. `interrupted` says a turn stopped
+        somewhere it can be picked up from, and a runner that died left no such
+        point; `failed` says the turn did not settle, which is true and is
+        already resumable.
+
+        **Open holds survive.** `pending_approval` is not touched, so a session
+        waiting on a human is still waiting after the reclaim. Reclaiming a
+        wedged session must not be a way past an approval gate.
+
+        The liveness check runs *inside* the write transaction, not before it,
+        so two operators reclaiming at once serialise on it — a check outside the
+        lock is the same deferred-read race `_transaction` already describes.
+
+        Refused, deliberately, when:
+
+        - the session is not `running` — there is no claim to release;
+        - the recorded pid is alive, or belongs to another user (`PermissionError`,
+          treated as alive). A live pid does not prove the *runner* lives, but it
+          is enough to prove it might;
+        - the pid is this process — that is a caller bug, not a crash;
+        - a `running` row carries no `runner_pid` at all, which is inconsistent
+          in its own right and not something to paper over.
+
+        Pid reuse is not solved here and is not pretended to be: a recycled pid
+        makes a dead runner look alive, which produces a refusal — the safe
+        direction. That is why this is a deliberate operator action and why
+        nothing calls it automatically.
+        """
+        with self._transaction() as conn:
+            row = conn.execute("SELECT * FROM sessions WHERE id = ?", (session_id,)).fetchone()
+            if row is None:
+                raise UnknownSessionError(f"no session {session_id!r} in {self.path}")
+            current = SessionStatus(row["status"])
+            if current is not SessionStatus.RUNNING:
+                raise SessionBusy(session_id, (SessionStatus.RUNNING.value,), current.value)
+
+            pid = row["runner_pid"]
+            if pid is None:
+                raise SessionError(
+                    f"session {session_id!r} is running but records no runner_pid, so "
+                    "there is no process to prove dead; this row is inconsistent and "
+                    "wants looking at rather than reclaiming"
+                )
+            pid = int(pid)
+            if pid == os.getpid():
+                raise SessionError(
+                    f"session {session_id!r} names this process ({pid}) as its runner; "
+                    "a runner does not reclaim its own session"
+                )
+            if _pid_alive(pid):
+                raise SessionError(
+                    f"session {session_id!r} is held by pid {pid}, which is still "
+                    "alive — refusing to reclaim a session that may be running. If "
+                    "that process is not a GraphARC runner, stop it first."
+                )
+
+            detail = f"runner pid {pid} no longer exists"
+            note = f"{reason} ({detail})" if reason else f"reclaimed: {detail}"
+            conn.execute(
+                "UPDATE sessions SET status = ?, updated_at = ?, runner_pid = NULL, "
+                "last_error = ? WHERE id = ?",
+                (SessionStatus.FAILED.value, _now(), detail, session_id),
+            )
+            conn.execute(
+                "INSERT INTO session_transitions (session_id, from_status, to_status, "
+                "reason, at, pid) VALUES (?, ?, ?, ?, ?, ?)",
+                (
+                    session_id,
+                    current.value,
+                    SessionStatus.FAILED.value,
+                    note,
+                    _now(),
+                    os.getpid(),
+                ),
+            )
+        return self.require(session_id)
+
     def history(self, session_id: str) -> list[StatusChange]:
         """Every status change this session has made, oldest first."""
         with self._lock: