Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/deep-dive.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
27 changes: 25 additions & 2 deletions grapharc/session/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]:
Expand Down
116 changes: 114 additions & 2 deletions grapharc/session/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
from grapharc.session.errors import (
InvalidTransition,
SessionBusy,
SessionError,
SessionExistsError,
SessionTerminated,
ThreadInUseError,
Expand Down Expand Up @@ -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)."""

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading