A routing typo waited for a run to fail, and a reused run id merged two of them

CHANGELOG.md

@@ -11,6 +11,8 @@ Entries are newest-last within a release, matching the order they were written.
 
 - a run **stopped for overspending reported spending nothing**. Tokens were attributed from `end` events, and a node the budget interrupts emits `error` instead — so `grapharc metrics` answered `tokens: 0` for a run whose own enforcement message named the figure that stopped it (`max_tokens reached (51/5)`). The audit trail lost precisely the number the stop was about, and per-node attribution dropped the most expensive node in the run. Every `error` event is now stamped with what its node spent, exactly as `end` is, and both `summarize` and the cost report count it; sub-events inside a node remain a breakdown of its total rather than an addition, so the disjointness that kept `ends + orphans` from double-counting is unchanged, and `RunCost.tokens == RunMetrics.tokens` still holds.
 - the `.env` credential loader **walked up parent directories to `/`**, while the config layer next door refuses exactly that on principle — so the file that *spends money* was discovered more eagerly than the one that *constrains* a run. A run started in a scratch subdirectory picked up an `OPENROUTER_API_KEY` from any ancestor: a `.env` in `$HOME` billed every user's experiment on a shared box to that key, a demo checked out under a client project quietly used the client's key, and since `redact()` is the only thing that ever prints a key, nothing in normal operation said *which file paid*. The rationale `cli/config.py` wrote down for `grapharc.toml` — "a run must never be silently governed by a file in a directory you didn't know about" — applies with more force to the file that pays than to the file that restrains, so `find_env_file` now reads the start directory (default: the working directory) and no ancestor of it. **This is a behaviour change:** anyone relying on a parent-directory `.env` must move it into the directory they run from, `export` the variable, or pass `env_file=` naming the file. Neither escape hatch moved — a real environment variable still beats any file, and an explicit `env_file=` still reads a file anywhere on disk — and no "search boundary" was added in place of the walk, because stopping at a git root is still an upward search.
+- **the one edge-declaration path that still deferred its error.** `add_conditional_edge` passed the router and its mapping straight through to LangGraph, so a mapping pointing at a node nobody added was accepted, an empty mapping was accepted, and the first run to take that branch died on `self.ends[key]` — a bare `KeyError` raised from inside LangGraph's branch machinery, naming neither the graph, the source node, nor the router that produced the key. Everywhere else this kernel fails at declaration: an undeclared write raises at `add_node`, a write to a field the schema does not have raises at `add_node`, a cycle is refused at `compile()`. The mapping's targets were knowable all along. They are checked now, at `add_conditional_edge`, with an empty mapping refused and every unreachable target named alongside the key that leads to it; a router that annotates what it returns — a `Literal`, an `Enum` — has those members held against the mapping's keys, using the same hash lookup LangGraph will use, so the check predicts the failure rather than approximating it. A router that annotates nothing is still not second-guessed: predicting an arbitrary function's return value is not a check, and inventing a requirement would be worse than the gap. That last case is no longer a `KeyError`, though — the router is wrapped so an unmapped key raises `GraphRoutingError` naming the node, the key and the keys that were declared, which is what the rest of the kernel raises for a transition it cannot make. The wrapper keeps the router's name and annotations, because LangGraph names the branch after the one and infers the branch's input schema from the other.
+- a **reused `--run-id` silently welded two runs into one record.** Every executing command appends to its `--trace` file — by design, since `grapharc diff` reads two runs out of one file — and nothing checked whether the id the operator passed was already in there. Running the same `plan` twice with one `--trace`/`--run-id` pair produced a single "run" whose `metrics` summed both runs' tokens and node counts, whose `viz` drew the second path welded onto the end of the first, and whose `replay` reconstructed a chimera; the operator got no signal at any point, and the trace is documented as the record the metrics cannot disagree with. The file being appendable was never the defect — the id being reused was, so the guard sits at the start of the run rather than in the recorder: `plan`, `run` and `agent` (both executors) refuse an explicit `--run-id` that already has events in the target trace, with exit 2 naming the id, the count and the file, before a single event is written. Fail closed rather than auto-renaming, because a run id is the name an operator will look the run up under later and picking a different one silently is the same class of surprise. Generated ids are untouched — fresh by construction, so they pay for no scan — and different ids in one file stay exactly as they were.
 
 ## 0.1.3
 

README.md

@@ -269,7 +269,7 @@ Three of those need their edges stated, because the gap is where people get hurt
 
 **Budgets.** Tokens are charged without the node's cooperation: a LangChain callback is installed for the duration of every node, so any chat model invoked on that thread reports usage to the run's meter — including calls buried inside library code the node merely calls — and the ceiling is enforced at the node boundary. `max_seconds` is an interrupt, not a poll: SIGALRM on the main thread, an asynchronous exception otherwise, so a node parked in `time.sleep` or on a provider's socket is cut off at the deadline. Where it stops short: spend a provider never reports cannot be charged, a model invoked on a thread the node started itself is outside the callback's context, and an async exception cannot unwind a thread sitting inside a C call — it lands when that call returns. Even then the deadline holds at the node boundary: a node that overran does not get its writes into state.
 
-**Routing.** The routers are code, which is the property that matters: no model output is ever consulted to pick an edge. But `add_conditional_edge` passes the router and its mapping straight through to LangGraph — GraphARC does not verify that the router's return value is a key in the mapping, so a typo surfaces as a `KeyError` at run time rather than when the edge is added.
+**Routing.** The routers are code, which is the property that matters: no model output is ever consulted to pick an edge. `add_conditional_edge` checks the mapping where it is declared — an empty mapping is refused, every target must name a node the graph has or `END`, and a router annotated with what it returns (a `Literal`, an `Enum`) has those members held against the mapping's keys. Where it stops short: a router that declares nothing is not second-guessed, so the key it returns is only known when it returns one. That case is no longer a bare `KeyError` from inside LangGraph's branch machinery — it raises `GraphRoutingError` naming the node, the key and the keys there were — but it is still discovered by a run rather than by `add_conditional_edge`.
 
 **Typing.** Writes are checked in both directions: the dict a node returns is validated field by field against the state schema before it lands, and the state is validated again when the next node receives it. A value that doesn't fit raises `StateTypeError` naming the node, the field, the declared type and what arrived — and that includes the last node before `END`, so a bad type no longer escapes into the result. The validated value is what gets written, so a schema that says `int` means the result holds an `int`. The remaining gap is narrow and worth stating exactly: write-time validation is built from each field's *annotation*, so constraints carried in the annotation (`Annotated[int, Field(gt=0)]`) do bite, but a validator the state model declares for itself — `@field_validator`, `@model_validator` — is not run on a write. A node returning `{"slug": "NOT-LOWER"}` into a field whose validator demands lowercase is accepted, even though constructing the model directly with that value raises; the violation surfaces only when a later node receives the state and the whole model is rebuilt, which means one written by the last node before `END` still reaches the result. The write *allowlist* is GraphARC's; the *types* are Pydantic's.
 

docs/cookbook/01-basics.md

@@ -741,11 +741,15 @@ No model output is ever consulted to pick an edge, so no amount of prose in a mo
 reply can steer the graph — a model that writes `ROUTE TO: all_verified` into a
 state field is writing a string, not choosing a branch.
 
-One gap to know: `add_conditional_edge` passes your router and mapping straight
-through to LangGraph, and GraphARC does **not** check that the router's return
-values are keys of the mapping. A typo shows up as a `KeyError` at run time, not
-when the edge is added. `StopReason` is a `StrEnum`, so using its members as your
-mapping keys is a cheap way to make that typo impossible.
+What `add_conditional_edge` checks, and when: the mapping is read at declaration
+time, so an empty one is refused, a target naming a node you never added raises
+there and then, and a router annotated `-> Literal["again", "stop"]` (or with an
+`Enum` return type) has those members held against your mapping's keys. A router
+that declares nothing is left alone — the key it returns is only knowable when it
+returns one — but that case is no longer a bare `KeyError` from inside LangGraph:
+it raises `GraphRoutingError` naming the node, the key and the keys you declared.
+`StopReason` is a `StrEnum`, so annotating your router with it moves that last
+check to declaration time too.
 
 ---
 
@@ -1523,5 +1527,7 @@ but repeated here because they are the ones that surprise people:
    node writes — only the field's annotation is enforced, which does include a
    nested model's own validators. Rebuild the model at your program's boundary, or
    keep the invariant one level down.
-2. `add_conditional_edge` does not verify that your router's return values are keys
-   of your mapping. A typo is a run-time `KeyError`.
+2. `add_conditional_edge` checks its mapping when the edge is added — the targets,
+   and a router that annotates what it returns. A router that annotates nothing is
+   not second-guessed, so the key it returns is checked when it returns one; that
+   is a `GraphRoutingError` naming the router, not a bare `KeyError`.

grapharc/cli/agent.py

@@ -22,6 +22,7 @@
 
 from grapharc.cli import optional, style
 from grapharc.cli.output import EXIT_FAILED, EXIT_OK, emit, fail
+from grapharc.cli.runid import refuse_reused_run_id
 
 # Entry points accepted from `grapharc.tools`, in preference order: a registrar
 # that fills a registry, and a factory that returns specs. Both are supported
@@ -173,6 +174,11 @@ def run_agent(
     workspace = Path(workspace).expanduser().resolve()
     workspace.mkdir(parents=True, exist_ok=True)
     trace_path = Path(trace_path) if trace_path else workspace / "trace.jsonl"
+    # While `run_id` still says whether the operator chose one: the generated
+    # id below is fresh by construction and has nothing to collide with.
+    reused = refuse_reused_run_id(trace_path, run_id, command="agent", as_json=as_json, task=task)
+    if reused is not None:
+        return reused
     run_id = run_id or f"agent-{uuid.uuid4().hex[:8]}"
 
     try:

grapharc/cli/delegate.py

@@ -30,6 +30,7 @@
 
 from grapharc.cli import style
 from grapharc.cli.output import EXIT_FAILED, EXIT_OK, emit, fail
+from grapharc.cli.runid import refuse_reused_run_id
 
 #: What a bare run may use, mirroring the harness default of "the core tools,
 #: shell included". An explicit `--allow` replaces this outright.
@@ -209,6 +210,11 @@ def run_delegated(
     workspace = Path(workspace).expanduser().resolve()
     workspace.mkdir(parents=True, exist_ok=True)
     trace_path = Path(trace_path) if trace_path else workspace / "trace.jsonl"
+    # `--executor claude-cli` resolves its own trace path, so it owns the same
+    # guard `run_agent` applies to the sandboxed path.
+    reused = refuse_reused_run_id(trace_path, run_id, command="agent", as_json=as_json, task=task)
+    if reused is not None:
+        return reused
     run_id = run_id or f"agent-{uuid.uuid4().hex[:8]}"
 
     allowed = list(allow) if allow and allow != ["*"] else list(DEFAULT_DELEGATED_TOOLS)

grapharc/cli/graphrun.py

@@ -44,6 +44,7 @@
 from grapharc.cli.generate import resolve_or_generate_policy
 from grapharc.cli.output import EXIT_FAILED, EXIT_OK, emit, fail
 from grapharc.cli.plan import PlanSetupError, resolve_registry
+from grapharc.cli.runid import refuse_reused_run_id
 
 #: Stage names `demo` owns. Kept so `grapharc run stage0` — which worked before
 #: the split — fails with a redirection rather than an argparse complaint about
@@ -162,6 +163,14 @@ def run_graph(
 
     schema = state_schema or IncidentState
     trace_path = trace_path or Path(tempfile.mkdtemp(prefix="grapharc-run-")) / "trace.jsonl"
+    # Ahead of the recorder, and ahead of the admission check that writes the
+    # first event: an id already in this file would merge this run with an
+    # earlier one, and `--check-only` writes its verdict under that id too.
+    reused = refuse_reused_run_id(
+        trace_path, run_id, command="run", as_json=as_json, graph=graph_path
+    )
+    if reused is not None:
+        return reused
     trace = TraceRecorder(trace_path)
     checker = AdmissionChecker(
         registry=registry,

grapharc/cli/main.py

@@ -703,7 +703,9 @@ def build_parser() -> argparse.ArgumentParser:
         "--tenant", default=None, metavar="NAME", help="tenant to compile --policy for"
     )
     run.add_argument("--trace", type=Path, default=None, help="trace JSONL output path")
-    run.add_argument("--run-id", default=None, help="name this run")
+    run.add_argument(
+        "--run-id", default=None, help="name this run; refused if --trace already holds it"
+    )
     run.add_argument(
         "--max-tokens",
         type=int,
@@ -773,7 +775,9 @@ def build_parser() -> argparse.ArgumentParser:
         "--tenant", default=None, metavar="NAME", help="tenant to compile --policy for"
     )
     plan.add_argument("--trace", type=Path, default=None, help="trace JSONL output path")
-    plan.add_argument("--run-id", default=None, help="name this run")
+    plan.add_argument(
+        "--run-id", default=None, help="name this run; refused if --trace already holds it"
+    )
     plan.add_argument(
         "--max-rounds", type=int, default=None,
         help="planning rounds the loop may take (default: 8)",
@@ -822,7 +826,11 @@ def build_parser() -> argparse.ArgumentParser:
         help="directory the tools work in (default: a fresh temp dir)",
     )
     agent.add_argument("--trace", type=Path, default=None, help="default: <workspace>/trace.jsonl")
-    agent.add_argument("--run-id", default=None, help="name this run (default: agent-<random>)")
+    agent.add_argument(
+        "--run-id",
+        default=None,
+        help="name this run, refused if --trace already holds it (default: agent-<random>)",
+    )
     agent.add_argument(
         "--allow",
         action="append",

grapharc/cli/plan.py

@@ -39,6 +39,7 @@
 from grapharc.cli.config import load as load_settings
 from grapharc.cli.generate import resolve_or_generate_policy
 from grapharc.cli.output import EXIT_FAILED, EXIT_OK, emit, fail
+from grapharc.cli.runid import refuse_reused_run_id
 
 DEFAULT_REGISTRY = "grapharc.examples.plan_incident:build_registry"
 
@@ -261,6 +262,15 @@ def plan(
 
     trace_path = trace_path or Path(tempfile.mkdtemp(prefix="grapharc-plan-")) / "trace.jsonl"
 
+    # Before the setup, because this one is about the file the setup would start
+    # writing into: a run id already in that file merges this plan with an
+    # earlier one under a single name.
+    reused = refuse_reused_run_id(
+        trace_path, run_id, command="plan", as_json=as_json, goal=goal
+    )
+    if reused is not None:
+        return reused
+
     # Everything that can be wrong about the setup is decided before a model is
     # asked anything, so a bad flag cannot half-execute a plan.
     try:

grapharc/cli/runid.py

@@ -0,0 +1,88 @@
+"""One guard, shared by every command that writes a trace under a chosen id.
+
+`TraceRecorder` opens its file in append mode, and that is right: several runs
+in one file is the pattern `grapharc diff` is built on. What append mode cannot
+notice is that the id being appended under is already in the file. Two runs then
+merge under one name, and every reader downstream presents the blend as a single
+coherent run — `metrics` sums both runs' tokens and nodes, `viz` welds the second
+path onto the end of the first, `replay` reconstructs a chimera. The trace is
+documented as the one record everything else agrees with; a reused id is what
+makes that one record lie.
+
+So the file staying appendable is correct and the id being reused is the defect,
+which puts the refusal here, at the start of a run, rather than in the recorder.
+Only an id the operator actually typed is checked: a generated one is fresh by
+construction and must not make every run pay for a file scan.
+"""
+
+from __future__ import annotations
+
+import json
+from pathlib import Path
+from typing import Any
+
+from grapharc.cli.output import fail
+
+
+def count_events(path: Path, run_id: str) -> int:
+    """How many events in `path` already carry `run_id`.
+
+    Reads the lines itself instead of going through `TraceRecorder.read_events`:
+    this runs before a guarded command starts, so it reads one field per line
+    rather than validating a whole event model, and a line it cannot parse is
+    skipped rather than raised over. The guard's job is to spot a collision, not
+    to be the file's validator — the readers still report a torn trace, and an
+    unreadable file is a failure the run's own writer will report in its own
+    words.
+    """
+    if not path.is_file():
+        return 0
+    seen = 0
+    try:
+        with path.open(encoding="utf-8", errors="replace") as handle:
+            for line in handle:
+                if not line.strip():
+                    continue
+                try:
+                    record = json.loads(line)
+                except ValueError:
+                    continue
+                if isinstance(record, dict) and record.get("run_id") == run_id:
+                    seen += 1
+    except OSError:
+        return 0
+    return seen
+
+
+def refuse_reused_run_id(
+    trace_path: Path | str | None,
+    run_id: str | None,
+    *,
+    command: str,
+    as_json: bool,
+    **extra: Any,
+) -> int | None:
+    """The exit code to return, or None when this run may go ahead.
+
+    `run_id is None` means the caller will generate one, which cannot collide.
+    """
+    if run_id is None or trace_path is None:
+        return None
+    path = Path(trace_path)
+    events = count_events(path, run_id)
+    if not events:
+        return None
+    return fail(
+        f"run id {run_id!r} already has {events} event{'' if events == 1 else 's'} in "
+        f"{path}; pick a new --run-id or a new --trace file. Appending a second run "
+        f"under one id merges the two, and metrics, replay and viz would then report "
+        f"the blend as one run",
+        as_json=as_json,
+        command=command,
+        run_id=run_id,
+        trace=str(path),
+        **extra,
+    )
+
+
+__all__ = ["count_events", "refuse_reused_run_id"]