From 1890a63476c29d20f831e3abbe9f9a63dea67235 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Sun, 2 Aug 2026 21:47:23 +0530 Subject: [PATCH] Add a live trace view to serve, and a file-backed plan approval flow The server grows an optional read-only /live view (grapharc serve --live-root PATH, token-gated via --live-token / GRAPHARC_LIVE_TOKEN) that lists trace files and streams a run's events over SSE with a mermaid topology render. Slack gains the same live link plumbing, the planner gains a file-backed approval store with a grapharc approve command, and to_mermaid learns clustered multi-round topologies. Co-Authored-By: Claude Fable 5 --- README.md | 8 +- docs/architecture-review.md | 391 +++++++++++++++++++++++ docs/cookbook/01-basics.md | 15 +- docs/cookbook/06-serving-and-ops.md | 103 ++++++- docs/cookbook/07-slack.md | 74 ++++- grapharc/cli/approve.py | 61 ++++ grapharc/cli/main.py | 44 +++ grapharc/cli/plan.py | 32 +- grapharc/cli/serve.py | 40 ++- grapharc/examples/plan_incident.py | 8 + grapharc/harness/agent.py | 44 +++ grapharc/observe/metrics.py | 169 +++++++++- grapharc/observe/otel.py | 5 + grapharc/observe/replay.py | 16 + grapharc/observe/trace.py | 44 ++- grapharc/planner/admission.py | 79 +++++ grapharc/planner/approval_file.py | 164 ++++++++++ grapharc/planner/loop.py | 160 +++++++++- grapharc/planner/proposal.py | 60 +++- grapharc/runtime/budget.py | 13 + grapharc/runtime/graph.py | 47 +++ grapharc/server/app.py | 12 + grapharc/server/live.py | 461 ++++++++++++++++++++++++++++ grapharc/slack/__init__.py | 11 +- grapharc/slack/bot.py | 171 +++++++++-- grapharc/slack/command.py | 107 +++++++ grapharc/slack/config.py | 36 +++ grapharc/slack/format.py | 54 +++- grapharc/slack/live.py | 381 +++++++++++++++++++++++ grapharc/stdlib.py | 144 ++++++++- tests/test_admission.py | 109 ++++++- tests/test_agent_node.py | 55 +++- tests/test_approval.py | 301 ++++++++++++++++++ tests/test_async_kernel.py | 6 +- tests/test_cli.py | 51 +++ tests/test_cookbook_basics.py | 23 +- tests/test_cookbook_serving.py | 2 +- tests/test_planner_loop.py | 74 +++++ tests/test_replay.py | 8 +- tests/test_server.py | 61 ++-- tests/test_server_live.py | 289 +++++++++++++++++ tests/test_slack_gateway.py | 158 +++++++++- tests/test_slack_live.py | 373 ++++++++++++++++++++++ tests/test_stage0_gate.py | 7 +- tests/test_stdlib.py | 76 +++++ tests/test_topology_viz.py | 295 ++++++++++++++++++ 46 files changed, 4724 insertions(+), 118 deletions(-) create mode 100644 docs/architecture-review.md create mode 100644 grapharc/cli/approve.py create mode 100644 grapharc/planner/approval_file.py create mode 100644 grapharc/server/live.py create mode 100644 grapharc/slack/live.py create mode 100644 tests/test_approval.py create mode 100644 tests/test_server_live.py create mode 100644 tests/test_slack_live.py create mode 100644 tests/test_topology_viz.py diff --git a/README.md b/README.md index 5120b4f..95d0ce3 100644 --- a/README.md +++ b/README.md @@ -157,6 +157,8 @@ grapharc demo stage6 # memory: provenance, supersession, recall grapharc demo capstone # all of the above in one research agent grapharc plan "look into the outage" # governed loop: propose -> admit -> execute -> replan +grapharc plan "..." --approve # park each admitted round until a human answers +grapharc approve # answer a parked run (--deny to refuse) grapharc run graph.json # a topology you wrote, through the same gate grapharc run graph.json --check-only # admission as a linter; executes nothing @@ -167,14 +169,14 @@ python -m grapharc.slack # the same commands from Slack (needs the `slac grapharc models # what a model spec resolves to grapharc trace # pretty-print a run trace grapharc metrics # tokens, retries, termination reason, per-node counts -grapharc viz # Mermaid diagram of the executed path +grapharc viz # Mermaid diagram: the declared graph, execution status overlaid grapharc replay # reconstruct a run from its trace grapharc diff # what changed between two runs ``` -Eleven commands, and every one of them takes `--json` — in JSON mode the failure is the document rather than a line on stderr. Exit codes are part of the interface: `0` did the job, `1` ran and the answer was negative (an agent stopped short, a run id had no events, two runs differed), `2` could not run at all. +Twelve commands, and every one of them takes `--json` — in JSON mode the failure is the document rather than a line on stderr. Exit codes are part of the interface: `0` did the job, `1` ran and the answer was negative (an agent stopped short, a run id had no events, two runs differed), `2` could not run at all. -The Slack bot puts most of these commands one `/grapharc …` away from a phone, behind an allowlisting gate that keeps the default spend at zero — setup in [docs/cookbook/07-slack.md](docs/cookbook/07-slack.md), and a command-by-command session, refusals included, in [docs/cookbook/08-slack-walkthrough.md](docs/cookbook/08-slack-walkthrough.md). +The Slack bot puts most of these commands one `/grapharc …` away from a phone, behind an allowlisting gate that keeps the default spend at zero — setup in [docs/cookbook/07-slack.md](docs/cookbook/07-slack.md), and a command-by-command session, refusals included, in [docs/cookbook/08-slack-walkthrough.md](docs/cookbook/08-slack-walkthrough.md). A tracing command run from Slack is narrated live — one status message edited in place as nodes run, with a refreshed diagram link — and `grapharc serve --live-root` adds a browser page that redraws the orchestration graph in real time over SSE. The `run` stages use scripted models by default, so they cost nothing and produce the same trace every time. Add `--model` to run one against a real backend — that works for stage1 through stage6 and the capstone; stage0 is pure code with no model in it. `grapharc agent` is the exception: it needs a tool-calling backend and says so rather than degrading, because a scripted model has no `bind_tools` to drive a tool loop with. diff --git a/docs/architecture-review.md b/docs/architecture-review.md new file mode 100644 index 0000000..a5b8493 --- /dev/null +++ b/docs/architecture-review.md @@ -0,0 +1,391 @@ +# GraphARC Architecture Review + +*Compiled 2026-08-01 from a three-track code audit (execution core; observability & delivery +surfaces; planner/gateway/CLI/packaging) plus issues observed first-hand while operating the +Slack bot, the live view, and local/delegated/API agent runs. Version audited: 0.1.1.* + +A recurring meta-observation up front: this codebase is **unusually honest with itself** — +most limitations below are already stated in a docstring somewhere. The issues that matter +are the ones where a *documented* limitation is load-bearing for a design decision the doc +does not follow through on. The strengths section at the end lists what should survive any +refactor. + +--- + +## Executive summary — the ten highest-leverage issues + +| # | Issue | Layer | Severity | +|---|---|---|---| +| 1 | Two parallel session subsystems: the durable one (`session/`) is unreachable, the exposed one (`server/runtime.py`) can't persist, evict, or gate approvals | server/session | HIGH | +| 2 | The sessions API has **no authentication**, and `/sessions/{id}/trace` serves the raw `state_delta` the live view deliberately redacts | server | HIGH | +| 3 | `Subgraph.fingerprint()` hashes a random `proposal_id` — `--check-only` prints a different fingerprint for the same file every run, so "the plan you reviewed is the plan that runs" cannot be enforced | planner | HIGH | +| 4 | Budgets are per-invoke, in-process objects: resume mints a fresh budget, nested runs escape the outer meter mid-flight (`_charge_back` is a one-call-site patch), nothing survives a process boundary | runtime | HIGH | +| 5 | Slack: subprocess-per-command × 5 bolt worker threads × 120s timeout = ack starvation and a Slack-retry amplification loop at just 5 concurrent commands | slack | HIGH | +| 6 | No cross-process append coordination on trace files; torn lines are silently dropped by readers (no counter, no log) — the audit trail can lose events without saying so | observe | HIGH | +| 7 | Fork-per-tool-call sandbox: `fork` from a multithreaded parent (deprecated in 3.14), a swallowed `setsid` failure that can make the timeout path `killpg` **the host process**, no Windows/macOS path | harness | HIGH | +| 8 | Kernel imports LangGraph *private* internals (`langgraph._internal._constants`, `.nodes[n].flat_writers`) while pinning `langgraph>=0.4` across a 0.x→1.x major boundary | runtime/session | HIGH | +| 9 | `grapharc plan`/`run` hard-import the incident **demo** for their loop factory and default state — the goal check is frozen at `len(state.notes) >= 3` and third-party registries cannot replace it (silent 8-round burn) | planner/cli | HIGH | +| 10 | The delegated executor (`--executor claude-cli`) accepts `--max-tokens` and silently never applies it; `--allow` means different things per executor; its subprocess timeout kills only the direct child | cli/harness | HIGH | + +--- + +## 1. Execution core (`runtime/`, `harness/`) + +### 1.1 Budget enforcement is per-invoke and non-compositional — HIGH +The meter is built inside `_run_config` and passed as a live object (`runtime/graph.py:762-771`); +"limits bound one attempt, not the lifetime of a thread" (`graph.py:806-808`). Consequences: +resuming a thread re-mints the full budget (no per-thread ledger); any node driving an inner +`CompiledGraphARC` swaps the global `charging()` contextvar (`runtime/usage.py:123-151`), so +the outer `max_tokens` is unenforced until the inner run returns — the planner hand-rolls +`_charge_back` (`planner/loop.py:632-645`) for exactly one call site and every other +composition loses the spend. The `RunContext` (holding a `threading.Lock`) also cannot cross +a process; today it survives only because LangGraph happens to drop non-scalar +`configurable` values from checkpoint metadata — an upstream behavior nobody pinned. + +### 1.2 Check-then-charge is not atomic — MEDIUM +`ctx.meter.check()` then `charge_iteration()` as two separate lock acquisitions +(`graph.py:616-621`; same shape for tokens in `usage.py:117-120`). Under `max_concurrency=8`, +all eight workers can pass `check()` before any charge lands — every limit overshoots by the +concurrency degree, in precisely the regime limits exist for. Fix: one `check_and_charge()`. + +### 1.3 Fork-per-tool-call sandbox — HIGH +`harness/executor.py:564-583`. Every tool call forks a fully-loaded interpreter, chdirs, +scrubs env, installs audit hooks, tears down. 24 forks for a modest agent run; sandbox +overhead dominates cheap tools. Sharper problems: `get_context("fork")` from a multithreaded +parent (LangGraph thread pools; Python 3.14 deprecates this) risks child deadlock and has no +Windows/macOS fallback; `os.setsid()` failure is swallowed (`executor.py:528-531`) after +which the timeout path's `os.killpg` (`executor.py:559`) targets the **parent's** process +group — SIGKILLing GraphARC itself; no `try/finally` around `poll()` means a deadline +interrupt leaks the child; a child dying without sending makes `recv()` raise a raw +`EOFError` outside the error taxonomy. + +### 1.4 Error taxonomy is executor-dependent — HIGH +The sandbox re-raises child failures as `RuntimeError(repr(exc))` (`executor.py:582`), +destroying exception type; `LocalExecutor` re-raises the real exception; the container +executor overloads `SandboxViolation` for *configuration* problems ("no docker on PATH"), +which `AgentNode` stamps `refused_by="sandbox"` — an audit trail where a missing Docker +install is indistinguishable from an active escape attempt. There is no `GraphARCError` +root across ~30 exception classes in five modules. The tool-argument hint added this session +(`agent.py:611-614`) has to string-match `"TypeError"` in a repr because the type identity +was destroyed at the pipe. Fix direction: a structured error envelope +(`{type, module, msg}`) across the process boundary, and a common exception root. + +### 1.5 Kernel↔LangGraph coupling is unbounded — HIGH +`session/runtime.py:125-140` imports `CONFIG_KEY_READ`/`CONFIG_KEY_SEND`/`NO_WRITES` from +`langgraph._internal._constants` with hardcoded string fallbacks and hand-drives +`.nodes[node].flat_writers`; `pyproject.toml` pins `langgraph>=0.4` while 1.2.9 is +installed. The fallback protects against renames, not semantic changes — a Pregel change +makes rejections silently do the wrong thing. The dependency floor should be a narrow range, +and the private-symbol usage isolated behind one adapter module with a version check. + +### 1.6 GraphARC graphs don't compose — HIGH/MEDIUM +Fail-closed `MissingRunContextError` (`graph.py:117-125`) makes a `CompiledGraphARC` +unusable as a subgraph of an outer LangGraph (the outer config lacks `grapharc_ctx`), and +`compile(checkpointer=None)` (`graph.py:377-380`) drops every other upstream option +(`interrupt_before/after`, `store`, `cache`, retry policies). The wrapper is currently a +ceiling on LangGraph rather than a layer over it; the planner's separate-top-level-invoke +workaround (see 1.1) exists because of this. + +### 1.7 Deep-copy of full state per node execution — MEDIUM +`state.model_copy(deep=True)` on every entry (`graph.py:610-614`): O(state) × nodes × +fan-out width on the critical path, 32 simultaneous copies under a 32-way fan-out, and an +undocumented "state can hold no live handle" restriction enforced by a `TypeError` from +`deepcopy`. + +### 1.8 `AgentNode` monolith: quadratic context, serial tools, sync-only — MEDIUM +726 lines holding schema-gen, prompting, loop, budgets, tracing, rendering, stall detection. +`messages` only grows and is resent whole every iteration (no trimming/summarization seam); +multi-tool turns execute serially (each paying a fork); no async path — under `ainvoke` the +node lands on a worker thread where the deadline guard degrades to an async-exception that +cannot interrupt a blocking socket read, so `max_seconds` is weakest on the one node that +needs it most. Tool schemas are lossy (`list[str]` → `{"type":"array"}` with no `items`; +unions collapse to the first arm; no per-parameter descriptions) and args are never +validated against the signature *before* paying the fork (`inspect.Signature.bind` in +`Harness.call` would make wrong-argument failures free and executor-independent). + +### 1.9 Smaller core items +- **Conditional-edge routers are unguarded** (`graph.py:336-348`): `Command`/`Send` are + checked, a router returning an unknown key is a raw LangGraph `KeyError` — same author + mistake, three exception types, only one of them GraphARC's. +- **Permissions decide on tool name only** (`permissions.py:42-47`): no argument-scoped + rules (`write_file` is all-or-nothing), no runtime promotion of an ASK approval, and + `ApprovalCallback` returns a bare bool — unusable for a real HITL UI. +- **`wants_ctx` arity heuristic** (`graph.py:680`): `def node(state, config)` receives a + `RunContext` named `config`; defaulted kwargs count wrong. +- **Pre-hooks break on first decision, post-hooks all run** (`core.py:62-77`): a REWRITE + hook silently short-circuits every security hook after it — ordering is load-bearing. +- **Trace file reopened per event under one process-wide lock** (`observe/trace.py:78-80`): + the trace becomes a synchronization point inside the parallel execution it observes. + +--- + +## 2. Observability (`observe/`) — the trace as sole source of truth + +### 2.1 No cross-process append safety; silent torn-line drops — HIGH +`TraceRecorder.record` locks with a **threading** lock only; two *writers* are uncoordinated +and a >PIPE_BUF line (easy: `_MAX_VALUE_CHARS=2000` is per-value, a six-field delta exceeds +12KB) can tear. Readers then hide the loss: `TailRecorder.read_events` skips unparseable +lines with no counter; `_advance_index` calls bare `json.loads` and *raises* instead. For a +layer whose motto is "a number no one can find in the audit trail is a number the audit +trail can contradict", losing events invisibly is the sharpest self-contradiction. +Fix: `fcntl.flock` on append (or per-writer files + merge), and a visible `skipped_lines`. + +### 2.2 Static topology is never serialized — MED-HIGH +`TraceEvent` records the graph as a *name*; `to_mermaid` chains executed events in file +order. Structurally inexpressible: branches not taken, fan-out as fan-out (three parallel +workers render as a chain the graph never had — `metrics.py:133`), nodes that never ran. +Everything downstream (viz, the live view) inherits this. Fix: one `phase:"graph"` event at +run start carrying nodes+edges — cheap, additive, ignored by old readers. + +### 2.3 No trace schema version; phase set open on write, closed on read — MED-HIGH +No `v` field on `TraceEvent`; the additive-optional-fields strategy cannot express a +*semantic* change, and `replay` will reconstruct a future producer's file confidently and +wrongly. `NODE_PHASES` is a closed frozenset while the recorder accepts any phase — a future +`phase:"retry"` is silently folded into `sub_events`; a sub-step emitter using `"start"` +mints phantom node executions that inflate every derived number. + +### 2.4 Everything is O(file), several things repeatedly — MED-HIGH +No incremental read path exists except `_advance_index` (private to `thread_summary`). +`build_snapshot` performs **five** full parses per file change per open stream +(`server/live.py:90-134`); the Slack `LiveTail._render` does two full parses per tick over +a growing file — **O(n²)** over a run's life; `cost.by_node` is O(runs × file); +`trace_text` loads the whole file into one string. One cursor-based +`read_events(since_bytes=...)` plus an advanceable `ReplayedRun` fixes all four. + +### 2.5 `_attach` misattributes under same-name fan-out — MEDIUM +Contrary to its own "abstains rather than guesses" contract, with several open executions +of the *same* node name it charges all sub-events to the last-opened one +(`replay.py:217-219`) — per-node cost attribution and OTel span parenting are wrong exactly +under fan-out. Attribution also silently depends on `AgentNode.name` matching the graph +node name. Fix: abstain (return None) on ambiguous prefix match — the orphan bucket exists. + +### 2.6 Three independent implementations of run totals — MED-LOW +`metrics.summarize`, `ReplayedRun.tokens/node_ms`, and `cost._price_run` each re-derive +totals with already-divergent rounding. Compute once on `ReplayedRun`; consume everywhere. + +--- + +## 3. Delivery surfaces (`server/`, `slack/`, `session/`) + +### 3.1 Two parallel session subsystems — HIGH (highest-leverage single item) +`grapharc/session/` (~1,600 lines: SQLite CAS transitions, durable event queue, approval +gates, cross-process resume) implements exactly what `server/runtime.py:37-45` lists as its +own missing features — yet `SessionManager` is referenced nowhere outside its package: no +CLI subcommand, no HTTP route. Meanwhile the exposed `InProcessRuntime` cannot persist, +evict, or deliver approvals, and the two `GraphRegistry` classes are incompatible (one takes +a recorder, the other a checkpointer), so migration is re-registration, not adapting. The +`SessionRuntime` protocol and wire models have no approval concept at all, which falsifies +`app.py`'s claim that swapping in the real session layer "touches nothing in this module". + +### 3.2 No auth on the sessions API; redaction is inconsistent within one app — HIGH +`create_app` accepts a token for `/live` only. `/sessions*` is wide open: anyone who can +reach the port can enumerate all sessions with full untruncated results, POST new runs +against the operator's credentials, and read the raw trace — `state_delta` included — via +`/sessions/{id}/trace`, undoing the leak-tested redaction the live view enforces. An +operator who tunnels the server for the Slack live view exposes all of it. + +### 3.3 Slack ack-starvation amplifier — HIGH +Subprocess-per-command blocks a bolt listener thread for up to `timeout_seconds`; bolt's +default pool is 5 threads; `ack()` happens inside the listener. Five long runs → the sixth +request cannot ack within 3s → Slack marks it failed **and retries** → retries queue behind +the same five. The queue drains at one command per timeout while Slack re-enqueues. Also: +every `metrics`/`viz` read pays a full interpreter + langgraph import. Fix: a bounded queue +with an immediate "queued (N ahead)" ack, or move `run_command` off the listener thread. + +### 3.4 Live-view index cost — HIGH +`scan_traces` re-reads and pydantic-validates **every byte of every trace under the root** +to extract run ids, synchronously on the event loop, and the index page meta-refreshes +every 5s per open tab. Coupled with `slack-runs/` never being pruned (retention is policy, +but retention cost and read cost are currently the same knob), this grows monotonically and +stalls every open SSE stream in the process. Fix: run ids in a sidecar index or filename; +listing must be `stat()`-only; move it off the loop. + +### 3.5 One 429 permanently kills Slack live narration — MEDIUM +The 2.5s update interval is budgeted per-run; N concurrent runs in one channel multiply it +(3 runs ≈ 72 edits/min, over `chat.update`'s tier). `_ChannelSink.update` returns `False` +on *any* exception and `LiveTail` treats `False` as "sink dead, go quiet forever" — a +transient rate-limit ends narration for the rest of the run. Fix: distinguish retryable +from dead in the sink contract; a process-wide per-channel token bucket. + +### 3.6 No requester identity, and nowhere to put one — MEDIUM +"Anyone in the workspace" shares one policy; Slack `user_id` is received and discarded; +`TraceEvent` has no actor/tenant field (`cost.py` declines tenant attribution for this +reason). After an incident the trace reconstructs *what* ran perfectly and cannot say *who +asked*. Fix: additive `actor` field on `TraceEvent`, threaded from the bot via env var. + +### 3.7 Other surface items +- **SSE poll (0.02s) contends on the runtime's single global lock** with every session's + superstep bookkeeping; 20 streams ≈ 1,000 lock acquisitions/s. A condition variable + + `call_soon_threadsafe` is the standard answer; polling is a choice here, not a constraint. +- **`session.events` grows unboundedly** and the index-based SSE cursor makes eviction a + breaking change; serving `events_since` from the trace file with a byte-offset cursor + deletes the RAM copy and stabilizes resume across restarts. +- **The Slack gate duplicates CLI argument knowledge** (`CommandSpec` vs argparse) with no + drift test; injected defaults (`--deny Bash`, `--max-seconds`) fail *silently* if the CLI + renames things. A ~30-line test walking the argparse tree closes it. +- **`slack-runs/` and every tempdir trace root accumulate forever** (no rotation anywhere). + +--- + +## 4. Planner, gateway, CLI, project structure + +### 4.1 Fingerprint instability — HIGH +`Subgraph.proposal_id` is a fresh `uuid4` per validation and `fingerprint()` hashes the +full dump including it (`planner/proposal.py:190,251`): running `--check-only` twice on an +unchanged file prints two different fingerprints, though the CLI says "the fingerprint is +what a later run is compared against". No CI "reviewed plan == running plan" gate can be +built on it. Fix: exclude `proposal_id`/`origin` from the hash, or add +`content_fingerprint()`. + +### 4.2 The demo is load-bearing production code — HIGH +`cli/plan.py:231` and `graphrun.py:154` unconditionally import +`examples.plan_incident.{build_loop, IncidentState}`. The goal check is frozen at +`len(state.notes) >= 3`; a registry whose state lacks `notes` silently burns all 8 rounds +and exits 1 having done the work. `cli/ → examples/` is an inverted dependency; the loop +factory and goal check must be part of the registry-module contract. + +### 4.3 The registry-module contract is duck-typed with silent fallbacks — MEDIUM +Six `getattr` lookups, each falling back silently (missing `WRITES` → nodes run and write +nothing; misspelled `STATE_SCHEMA` → writes validated against the demo's schema). Four ways +to get a registry that loads, runs, and does nothing useful — all reported as success. A +validated `RegistryModule` protocol checked at load time converts each into a startup error. + +### 4.4 Delegated executor governance gaps — HIGH +`--max-tokens` is accepted and silently unapplied under `--executor claude-cli` +(unbounded spend against the operator's subscription with no error); `--allow '*'` means +"everything" in one executor and "these seven Claude Code tools" in the other; the +wall-clock is `subprocess.run(timeout=...)`, which kills the direct child only — Claude +Code's own spawned shells can survive a timeout (the sandbox uses `setsid`/`killpg` for +exactly this reason). Observability is also coarse by design (start/end/stop only), which +the live view then inherits — mid-run the graph shows one open node. At minimum the +unsupported flags should be *refused*, not ignored. + +### 4.5 Replan feedback is one round deep — HIGH (for planner efficacy) +`loop.py` clears feedback each round and rebuilds the planner's messages from scratch: +at round 3 the model cannot see that round 1 was rejected for the identical reason, so +`max_consecutive_rejections` can be exhausted by the same mistake repeated blind. +`RoundRecord` already carries the history; it just isn't fed back. (Observed live in this +session: a weak local model burned its round allowance exactly this way.) + +### 4.6 Gateway backend seam doesn't exist — MED-HIGH +Adding a backend touches six hand-maintained lists across five files (`BACKENDS`, +`BACKEND_VENDOR`, the if-chain in `get_model`, the lazy-import table, the probe dict, the +CLI examples dict); `KNOWN_AUTHORS` is an 18-slug allowlist that rots with every new +provider. No entry-point group, no `Backend` protocol — out-of-tree backends require a +fork, which undercuts "the backend is a config change". + +### 4.7 `.env` upward search is worked around, not fixed — HIGH +`gateway/config.find_env_file()` walks to `/`; the Slack config refuses to use it (issue +#20) — but the *subprocesses* the bot spawns still resolve OpenRouter keys through the +upward search from the bot's workdir. A `.env` in any ancestor directory is spendable by +any workspace member. Two config loaders in one package have opposite trust models; the +search scope belongs on the loader, not in comments at call sites. + +### 4.8 CLI structure — MED-HIGH +`cli/main.py` is a 904-line parser+handler+fixture monolith (adding a flag touches ≥3 +places; demo scripts are inline JSON string literals maintained in two files). No top-level +exception handler: an uncaught crash exits 1 — the code reserved for "ran, answer was +negative" — with empty stdout in `--json` mode, and the repo already contains two +one-at-a-time "this used to escape as a traceback" fixes. `--config` reaches only three +subcommands, so `grapharc.toml`'s `model` key is silently ignored by `agent` and `serve`. +Default traces land in unfindable tempdirs with no `grapharc runs` index — the Slack gate +had to build its own run-directory manager (`slack-runs/` + `--trace` injection) to work +around it, and any other integration pays the same cost. Three commands duplicate ~150 +lines of run-setup; two divergent `resolve_registry` implementations raise different +exception types for the same user error. + +### 4.9 Byte-pinned transcripts freeze output formatting as API — MED-HIGH +Cookbook/README tests byte-compare CLI output; label widths are commented "not up for +revision"; the incident demo's scripted reply sequence is effectively public API; the +version-bump literal `0.1.1` is asserted in a test, so releasing fails the suite until a +doc edit. The intent (docs that cannot rot) is right; there is no seam between "the claim +is still true" and "the bytes are identical", and no snapshot-regeneration tooling. + +### 4.10 Admission checks are narrower than the prose — MEDIUM +`parent_depth` is caller-asserted and always 0 in-tree (the recursion bound is decorative); +node `args` ride through admission unchecked into factories; `worst_case` budget ignores +iteration count, so cyclic proposals under-estimate by the loop factor; `NodeRegistry` +mutability is opt-out (`freeze()`), and only the demo freezes. + +--- + +## 5. Cross-cutting patterns + +1. **Documented ≠ handled.** The codebase's best habit (stating limitations) repeatedly + substitutes for closing them: the `.env` search has a warning comment instead of a scope + parameter; the in-process runtime lists its missing features while the package that has + them ships unreachable; the sandbox names its holes while the error taxonomy hides which + ones fired. +2. **Duplicated knowledge with no drift detection.** Slack gate vs argparse; six backend + lists; three run-total implementations; two `resolve_registry`s; two `GraphRegistry` + classes; demo hints in two files. None has a test tying the copies together. +3. **Identity is missing at every layer.** Runs have no actor; proposals have no stable + content hash; checkpoints have no topology fingerprint; trace lines have no schema + version; graph structure has no serialized form. Each is a small additive field; their + absence collectively caps the auditability story the project is named for. +4. **O(file) as the universal access pattern.** One incremental reader exists, private, and + everything else re-parses — the cost lands exactly on the newest features (live views). +5. **The governed path and the convenient path diverge silently.** Delegated executor drops + flags; `local` executor skips confinement; sub-runs escape budgets. Flags that cannot be + honored should be refused loudly. + +--- + +## 6. Strengths to preserve + +- **Failure-mode honesty as a discipline.** `executor.py`'s five named sandbox holes, + `budget.py`/`graph.py` on what deadline guards cannot interrupt, `cost.py` on what it + refuses to attribute, docstrings recording the bug that motivated the code. Spot-checked + claims held up. This is design rationale future maintainers cannot reconstruct. +- **Fail-closed defaults where they matter**: DENY-by-default policy, absent approval = + denied, unclassifiable opens treated as writes, container refusing rather than degrading, + `MissingRunContextError`, two-envelope parses raising rather than picking. +- **The write-allowlist + per-field `TypeAdapter` validation at node boundaries** + (`graph.py:406-441`) — LangGraph's silent key-dropping becomes a loud contract violation + at the point of authorship, at zero runtime cost. +- **The single-shaping-pass discipline in `BroadcastRecorder`** (SSE frame and JSONL line + are the same dict; non-idempotent truncation identified and tested). +- **The propose/admit/materialize split is structurally enforced**, not asserted: + proposals have no body-shaped field, the materializer re-checks the admission hash, and + rejections are structured, machine-matchable data with remedies. +- **The Slack gate's admission posture** (allowlist, path confinement, double opt-in, + executor tempering) is the right shape; its problems are drift risk and missing identity, + not design. + +--- + +## 7. Suggested fix order + +**Now (small, high leverage):** +1. Content-stable `fingerprint()` (§4.1) — small change, unlocks the advertised CI gate. +2. Refuse unsupported flags on the delegated path instead of ignoring them (§4.4). +3. `check_and_charge()` atomicity (§1.2); router wrapping (§1.9); `_attach` abstention (§2.5). +4. Argparse↔gate drift test (§3.7); `skipped_lines` counter on readers (§2.1). +5. `actor` field on `TraceEvent` threaded from Slack (§3.6); `phase:"graph"` topology event (§2.2). +6. Top-level CLI exception handler honoring the exit-code/JSON contract (§4.8). + +**Next (structural, medium effort):** +7. One incremental trace reader; retrofit live view, LiveTail, cost, trace_text (§2.4). +8. App-level auth on the whole server; align `/trace` with the live view's redaction (§3.2). +9. Bounded queue + off-listener execution in the Slack bot (§3.3); retryable-vs-dead sink + contract (§3.5). +10. Registry-module `Protocol` validated at load; move the loop factory/goal check into it, + breaking `cli → examples` (§4.2, §4.3). +11. Structured error envelope across executor boundaries + `GraphARCError` root (§1.4). + +**Eventually (architectural):** +12. Unify the two session runtimes around `session/`'s durable core, redesign the wire + contract to carry approvals, delete the in-memory event list in favor of file-cursor + SSE (§3.1). +13. Hierarchical budget meters with a per-thread ledger (§1.1). +14. A sandbox execution service (pooled workers or the container path) replacing + fork-per-call; structured envelopes come with it (§1.3). +15. An adapter module owning every LangGraph private-symbol touch, with a pinned narrow + version range and a CI canary against upstream (§1.5, §1.6). + +--- + +*Issues fixed during the session that produced this review: tool-call `TypeError`s now +append the tool's real signature (`harness/agent.py`); torn-line-safe `TailRecorder` +promoted to `observe/trace.py`; the live view no longer reports an open-but-quiet delegated +run as "idle"; the Slack final message keeps its diagram and run-page links.* diff --git a/docs/cookbook/01-basics.md b/docs/cookbook/01-basics.md index 52a29b5..e6368c7 100644 --- a/docs/cookbook/01-basics.md +++ b/docs/cookbook/01-basics.md @@ -799,6 +799,7 @@ for event in trace.read_events(): Output: ``` +{'attempt': 1, 'graph': 'counter', 'node': 'topology', 'phase': 'topology', 'step': 0, 'state_delta': {'nodes': ['load', 'count'], 'edges': [['__start__', 'load', 'static'], ['load', 'count', 'static'], ['count', '__end__', 'static']]}} {'attempt': 1, 'graph': 'counter', 'node': 'load', 'phase': 'start', 'step': 1} {'attempt': 1, 'graph': 'counter', 'node': 'load', 'phase': 'end', 'step': 1, 'state_delta': {'items': ['a', 'b', 'c']}, 'tokens': 0} {'attempt': 1, 'graph': 'counter', 'node': 'count', 'phase': 'start', 'step': 2} @@ -811,6 +812,12 @@ printout only because they differ every run. So, by phase: +- **`topology`** is written once per entry, before any node runs: the graph's declared + nodes and edges (conditional routes included, tagged by kind). It is what lets a + diagram show the whole orchestration — branches not taken included — rather than + only the path that happened to run. It carries `step: 0` on every attempt: it + states shape, not order. + - **`start`** carries identity and nothing else: run, thread, attempt, graph, node, step, timestamp. It is written *before* the node body, so it exists even when the node never returns. @@ -1035,16 +1042,20 @@ conn.close() Output: ``` +attempt 1 step 0 topology topology attempt 1 step 1 fetch start attempt 1 step 1 fetch end attempt 1 step 2 save start attempt 1 step 2 save error +attempt 2 step 0 topology topology attempt 2 step 3 save start attempt 2 step 3 save end ``` -The resumed attempt starts at step 3 rather than restarting the numbering, and -`fetch` has no attempt-2 line because it did not re-run. +The resumed attempt's *work* starts at step 3 rather than restarting the numbering, +and `fetch` has no attempt-2 line because it did not re-run. Each attempt restates +the graph's topology at step 0 — shape, not order — which is why step comparisons +across attempts filter that phase out. --- diff --git a/docs/cookbook/06-serving-and-ops.md b/docs/cookbook/06-serving-and-ops.md index 7054797..67c8bfc 100644 --- a/docs/cookbook/06-serving-and-ops.md +++ b/docs/cookbook/06-serving-and-ops.md @@ -1068,8 +1068,8 @@ status : succeeded answer : Budgets cap iterations, tokens and time. usage : 1.0 iterations, 15.0 tokens event : False recorded: this runtime does not deliver 'message' events into a running graph (ROADMAP §6.4 event queue / §6.5 approval node) -frames : ['event: trace', 'event: trace', 'event: status', 'event: done'] -trace : ['start', 'end'] +frames : ['event: trace', 'event: trace', 'event: trace', 'event: status', 'event: done'] +trace : ['topology', 'start', 'end'] ``` The routes: @@ -1280,6 +1280,7 @@ $ curl -s localhost:8124/sessions/bf5ca55bff7b480f } $ curl -s localhost:8124/sessions/bf5ca55bff7b480f/trace +{"ts": "...", "run_id": "0d9dce7f61c4", "thread_id": "bf5ca55bff7b480f", "attempt": 1, "graph": "qa", "node": "topology", "phase": "topology", "step": 0, "state_delta": {"nodes": ["answer"], "edges": [["__start__", "answer", "static"], ["answer", "__end__", "static"]]}} {"ts": "...", "run_id": "0d9dce7f61c4", "thread_id": "bf5ca55bff7b480f", "attempt": 1, "graph": "qa", "node": "answer", "phase": "start", "step": 1} {"ts": "...", "run_id": "0d9dce7f61c4", "thread_id": "bf5ca55bff7b480f", "attempt": 1, "graph": "qa", "node": "answer", "phase": "end", "step": 1, "state_delta": {"answer": "Budgets cap iterations, tokens and time."}, "duration_ms": 1.0288769999533542, "tokens": 15} ``` @@ -1304,6 +1305,45 @@ could use, without contacting any provider. --- +## How do I watch a run live in a browser? + +`grapharc serve --live-root PATH` mounts a read-only live view at `/live` over +the trace files under `PATH` — including files other processes are appending +right now. Traces are append-only JSONL written line-at-a-time under a lock, +so a reader that stops at the last complete newline (`TailRecorder`, in +`grapharc.observe.trace`) can follow a run another process is executing; +that is exactly what the view does. + +`GET /live` lists every `*.jsonl` under the root, newest first. +`GET /live/view?trace=REL` is the page: it opens +`GET /live/api/stream?trace=REL` (server-sent events) and receives a fresh +`snapshot` — the run's Mermaid diagram, `metrics`-style numbers, cost, and +status — each time the file grows. The server recomputes the snapshot; +the page only renders it. Add `&run=ID` to pin one run in a file that holds +several; without it the view follows the newest. + +This composes with the Slack bot, which gives every tracing command a trace +path under its working directory: run `grapharc serve --live-root` over that +same directory, set `GRAPHARC_SLACK_LIVE_URL`, and the bot posts a +"watch live" link when a run starts — the walkthrough is in +[07-slack.md](07-slack.md). It also composes with this page's own server +sessions: point `--live-root` at the session root and each +`/trace.jsonl` gets a page. + +The posture is the same as everything else in `grapharc.observe`: the view is +derived from the trace file and nothing else, and it is read-only. Requested +paths are confined inside the root (escapes are 404s), and `state_delta` +contents — arbitrary node writes — are never serialized into any live +response; the exposure is what `viz` already prints. The bind stays +`127.0.0.1` unless you say otherwise; binding wider prints a warning, because +reachability is meant to come from a tunnel or tailnet in front, optionally +with `--live-token TOKEN` (or `GRAPHARC_LIVE_TOKEN`) required on every +`/live` request. The diagram renders with mermaid.js from a pinned CDN; with +no CDN reachable the page falls back to the raw Mermaid source plus the same +mermaid.live fragment link the Slack bot posts. + +--- + ## How do I reconstruct a run after it finished? `replay(trace, run_id)`. It is a *reconstruction*, not a re-execution: it reads @@ -1529,8 +1569,16 @@ total 17 tok complete: True metrics : 2 nodes, 17 tokens, {'draft': 1, 'polish': 1} flowchart TD + n0["draft"] + n1["polish"] start((start)) --> n0["draft"] n0["draft"] --> n1["polish"] + n1["polish"] --> fin((end)) + classDef done fill:#d3f2d3,stroke:#2f7d32 + classDef running fill:#fff3cd,stroke:#b8860b + classDef pending fill:#eeeeee,stroke:#999999,color:#666666 + classDef errored fill:#f8d7da,stroke:#b02a37 + class n0,n1 done ``` `RunCost.tokens` and `RunMetrics.tokens` agree by construction — both count the @@ -1553,10 +1601,15 @@ a cost report and an audit trail that disagree are worse than either alone. spend, reported as `tokens_before_error` — kept out of the total so the total keeps matching `metrics`. -`to_mermaid` renders the *executed* path, keyed by `(node, step)`, so parallel -instances of a fan-out worker are distinct boxes rather than one box with a -self-loop the graph never had. Paste it into any Markdown renderer that speaks -Mermaid. +`to_mermaid` renders the graph's *declared topology* — the `topology` event every +run now writes — with execution status overlaid per node: `done`, `running`, +`errored`, or still `pending`. Branches not taken stay on the diagram in grey, +conditional routes draw dotted, and a multi-round planner run gets one cluster +per admitted round. A trace with no topology event (an `AgentNode` driven with +no enclosing graph, or a file written before the event existed) falls back to +the executed path in event order, keyed by `(node, step)` so parallel instances +of a fan-out worker are distinct boxes. Paste either form into any Markdown +renderer that speaks Mermaid. `attribute_thread(trace, thread_id)` is the same for a whole session across resumes, and `by_node(trace)` ranks every node in a file by cost. @@ -1565,7 +1618,7 @@ resumes, and `by_node(trace)` ranks every node in a file by cost. ## The CLI tour -Eleven commands. Every one takes `--json`, which prints the same payload as one +Twelve commands. Every one takes `--json`, which prints the same payload as one document on stdout — including failures, which become the document rather than a line on stderr. @@ -1573,7 +1626,8 @@ line on stderr. | --- | --- | | `grapharc demo ` | run a built-in example graph (`stage0`…`stage6`, `capstone`) | | `grapharc run ` | run a topology you wrote, through the admission gate; `--check-only` lints it | -| `grapharc plan ` | governed loop: propose → admit → execute → replan | +| `grapharc plan ` | governed loop: propose → admit → execute → replan; `--approve` parks each admitted round for a human | +| `grapharc approve ` | answer a plan run waiting on its approval gate (`--deny` to refuse) | | `grapharc agent ` | run an agent node with the core tools against a task | | `grapharc serve` | run the HTTP API | | `grapharc models [spec]` | what a spec resolves to; `--check` probes this machine | @@ -1606,12 +1660,12 @@ $ grapharc trace trace.jsonl --json | jq -r '.events[0].run_id' 2a47f18064b7 $ grapharc trace trace.jsonl --run-id 2a47f18064b7 | head -6 +[ 0] topology topology Δ{'nodes': ['start', 'plan', 'act', 'verify', 'finish_target_met', 'finish_max_iterations', 'finish_no_progress'], 'edges': [['__start__', 'start', 'static'], ['start', 'plan', 'static'], ['plan', 'act', 'static'], ['act', 'verify', 'static'], ['finish_target_met', '__end__', 'static'], ['finish_max_iterations', '__end__', 'static'], ['finish_no_progress', '__end__', 'static'], ['verify', 'plan', 'conditional'], ['verify', 'finish_target_met', 'conditional'], ['verify', 'finish_max_iterations', 'conditional'], ['verify', 'finish_no_progress', 'conditional']]} [ 1] start start [ 1] start end Δ{'pending': ['budgets', 'verifier']} [ 2] plan start [ 2] plan end Δ{'proposal': 'budgets', 'round': 1} [ 3] act start -[ 3] act end Δ{'candidate': 1} $ grapharc metrics trace.jsonl 2a47f18064b7 run_id: 2a47f18064b7 @@ -1623,19 +1677,35 @@ duration_ms: 0.68 attempts: 1 termination_reason: target_met per_node: {'start': 1, 'plan': 2, 'act': 2, 'verify': 2, 'finish_target_met': 1} -events: 16 -per_phase: {'start': 8, 'end': 8} +events: 17 +per_phase: {'topology': 1, 'start': 8, 'end': 8} $ grapharc viz trace.jsonl 2a47f18064b7 flowchart TD + n0["start"] + n1["plan"] + n2["act"] + n3["verify"] + n4["finish_target_met"] + n5["finish_max_iterations"] + n6["finish_no_progress"] start((start)) --> n0["start"] n0["start"] --> n1["plan"] n1["plan"] --> n2["act"] n2["act"] --> n3["verify"] - n3["verify"] --> n4["plan"] - n4["plan"] --> n5["act"] - n5["act"] --> n6["verify"] - n6["verify"] --> n7["finish_target_met"] + n4["finish_target_met"] --> fin((end)) + n5["finish_max_iterations"] --> fin((end)) + n6["finish_no_progress"] --> fin((end)) + n3["verify"] -.-> n1["plan"] + n3["verify"] -.-> n4["finish_target_met"] + n3["verify"] -.-> n5["finish_max_iterations"] + n3["verify"] -.-> n6["finish_no_progress"] + classDef done fill:#d3f2d3,stroke:#2f7d32 + classDef running fill:#fff3cd,stroke:#b8860b + classDef pending fill:#eeeeee,stroke:#999999,color:#666666 + classDef errored fill:#f8d7da,stroke:#b02a37 + class n0,n1,n2,n3,n4 done + class n5,n6 pending $ grapharc replay trace.jsonl 2a47f18064b7 | tail -4 pending = [] @@ -1677,8 +1747,9 @@ $ grapharc metrics trace.jsonl 2a47f18064b7 --json "verify": 2, "finish_target_met": 1 }, - "events": 16, + "events": 17, "per_phase": { + "topology": 1, "start": 8, "end": 8 } diff --git a/docs/cookbook/07-slack.md b/docs/cookbook/07-slack.md index 9cd1d9b..38cc625 100644 --- a/docs/cookbook/07-slack.md +++ b/docs/cookbook/07-slack.md @@ -109,12 +109,80 @@ Configuration is environment-only, read once at startup: | `GRAPHARC_SLACK_ALLOW_MODEL` | off | `1` admits `--model`/`--reviewer-model` | | `GRAPHARC_SLACK_ALLOW_AGENT` | off | `1` admits `agent` — only together with `ALLOW_MODEL` | | `GRAPHARC_SLACK_COMMAND` | `/grapharc` | the slash command to answer to | +| `GRAPHARC_SLACK_LIVE` | on | `0` turns off the live-edited status message | +| `GRAPHARC_SLACK_LIVE_INTERVAL` | `2.5` | seconds between two edits of the status message | +| `GRAPHARC_SLACK_LIVE_URL` | unset | base URL of a `grapharc serve --live-root` the requester can reach; posts a "watch live" link | The bot reads tokens from the process environment only. The `.env` upward-directory search that the model gateway performs is deliberately not used here: a bot that a whole workspace can drive must not discover credentials in a file the operator did not point it at. +## Live progress + +A command that traces (`demo`, `run`, `plan`, `agent`) is narrated while it +runs. The gate gives every such command a trace path the bot knows — a unique +`slack-runs//trace.jsonl` under the working directory, unless the +request named its own `--trace` — and the bot tails that file from a side +thread while the subprocess runs. What you see in Slack is one status message, +edited in place every couple of seconds: + +``` +`grapharc run pipeline.toml --trace slack-runs/…/trace.jsonl` — running (14s) +✓ ingest 312ms +✓ extract 1.8s 1543 tok +✗ verify err: citation not found +▸ report running… +6 events · 2/4 nodes done · 1543 tok + +``` + +The `current diagram` link is the same mermaid.live fragment URL `viz` gets — +the diagram is compressed into the URL itself and shipped to no one — and it is +refreshed on every edit, so mid-run it renders the path *so far*. When the +command finishes, the status message is edited one last time into the same +final result the bot has always posted. + +Everything about this path is best-effort by construction. If the bot cannot +post the status message (it is not in the channel, the API errored), the whole +live layer steps aside and you get today's single blocking reply; if a mid-run +edit fails, the narration goes quiet; and if the *final* edit fails, the result +is posted as an ordinary reply instead. A broken live view can cost you the +narration, never the answer. One visibility note: for a slash command the +status message is posted to the channel (a `respond()`-style reply would allow +only five updates), so a live run is visible to everyone in it — the mention +path threads it under your message as before. + +Because the trace now lands inside the working directory, the run is also +inspectable afterwards from Slack itself: `/grapharc metrics +slack-runs//trace.jsonl `, `viz` for the finished diagram, +`replay` for the reconstruction. The `slack-runs/` directories are the audit +trail and are never cleaned up automatically; prune them like any other logs. + +## Watching it live in a browser + +The status message is text. For the actual diagram redrawing itself as nodes +run, pair the bot with the live view server on the same machine: + +```bash +grapharc serve --live-root "$GRAPHARC_SLACK_WORKDIR" --port 8300 +export GRAPHARC_SLACK_LIVE_URL=https://laptop.tailnet.ts.net:8300 +python -m grapharc.slack +``` + +With the URL configured, the bot's first status message includes +`watch live: /live/view?trace=slack-runs/…` — a page that renders the +Mermaid diagram and the run's numbers and updates itself over SSE as the trace +file grows. `/live` lists every trace under the root. The server is read-only, +confines every requested path inside the root, and never serves `state_delta` +contents — what the page shows is what `viz` and `metrics` already show. + +Reachability is deliberately your problem, not the bot's: the bot never opens +a port (that is the whole point of Socket Mode), and `serve` still binds +loopback by default. Put a tailnet or tunnel (Tailscale, cloudflared) in front +for the person on the phone, and add `--live-token` if the URL is guessable. +Details in [06-serving-and-ops.md](06-serving-and-ops.md). + ## A `plan` that reads The default planning registry is the incident-response demo: its node bodies @@ -189,9 +257,9 @@ stays unreachable from Slack. - **The bot is alive while the process is.** Laptop lid closed means commands from a phone go unanswered — Slack shows the slash command timing out, and nothing queues. The same script runs unchanged on any always-on box. -- **Slack's three-second ack.** The bot acks immediately ("running …") and - posts the result when the command finishes; the timeout bounds how long - that can be. +- **Slack's three-second ack.** The bot acks immediately ("running …"), then + narrates a tracing command through the live status message and lands the + result there when it finishes; the timeout bounds how long that can be. - **The workspace is the trust boundary.** The gate stops path escapes, module imports and spend, but anyone in the workspace can run every allowed command against every file in the working directory. Give the bot a diff --git a/grapharc/cli/approve.py b/grapharc/cli/approve.py new file mode 100644 index 0000000..d2fc9f2 --- /dev/null +++ b/grapharc/cli/approve.py @@ -0,0 +1,61 @@ +"""`grapharc approve` — answer a plan run parked on its approval gate. + +The paused run (a `grapharc plan --approve`) wrote `approval-request.json` +next to its trace and is polling for `approval-decision.json`. This command +reads the request, writes the decision quoting the request's fingerprint — the +run ignores a decision naming any other plan — and exits. Exit codes follow +the CLI contract: 0 the decision was delivered, 1 nothing is waiting for one, +2 the path does not lead anywhere a request could be. +""" + +from __future__ import annotations + +from pathlib import Path + +from grapharc.cli.output import EXIT_FAILED, EXIT_OK, emit, fail +from grapharc.planner.approval_file import read_request, write_decision + + +def approve(path: str | Path, *, deny: bool = False, as_json: bool = False) -> int: + """Deliver a decision to the approval request in `path`'s directory. + + `path` may be the trace file the run printed, or the directory holding it — + both name the same handshake directory. + """ + target = Path(path) + directory = target if target.is_dir() else target.parent + if not directory.is_dir(): + return fail( + f"no such directory: {directory}", as_json=as_json, command="approve" + ) + + request = read_request(directory) + if request is None: + return fail( + f"nothing is waiting for approval in {directory}", + as_json=as_json, + command="approve", + code=EXIT_FAILED, + ) + + decision = "denied" if deny else "approved" + fingerprint = str(request.get("fingerprint", "")) + write_decision(directory, fingerprint=fingerprint, decision=decision) + payload = { + "ok": True, + "command": "approve", + "decision": decision, + "fingerprint": fingerprint, + "proposal_id": request.get("proposal_id", ""), + "nodes": request.get("nodes", []), + } + nodes = ", ".join(str(n) for n in request.get("nodes", [])) or "(none)" + lines = [ + f"{decision}: plan {request.get('proposal_id', '?')} ({nodes})", + f"fingerprint {fingerprint}", + ] + emit(payload, lines, as_json=as_json) + return EXIT_OK + + +__all__ = ["approve"] diff --git a/grapharc/cli/main.py b/grapharc/cli/main.py index 63339ff..ef3c1f4 100644 --- a/grapharc/cli/main.py +++ b/grapharc/cli/main.py @@ -25,6 +25,7 @@ import argparse import json +import os import sqlite3 import sys import tempfile @@ -343,10 +344,18 @@ def _cmd_plan(args: argparse.Namespace) -> int: max_rounds=args.max_rounds, max_tokens=args.max_tokens, config_path=args.config, + approve=args.approve, + approval_timeout=args.approval_timeout, as_json=args.json, ) +def _cmd_approve(args: argparse.Namespace) -> int: + from grapharc.cli.approve import approve + + return approve(args.path, deny=args.deny, as_json=args.json) + + def _cmd_models(args: argparse.Namespace) -> int: from grapharc.gateway import ( describe, @@ -460,6 +469,8 @@ def _cmd_serve(args: argparse.Namespace) -> int: port=args.port, log_level=args.log_level, registry_target=args.registry, + live_root=args.live_root, + live_token=args.live_token, as_json=args.json, ) @@ -746,8 +757,29 @@ def build_parser() -> argparse.ArgumentParser: "--max-tokens", type=int, default=None, help="run token ceiling across every round (default: 100000)", ) + plan.add_argument( + "--approve", + action="store_true", + help="pause each admitted round until `grapharc approve` answers next to the trace", + ) + plan.add_argument( + "--approval-timeout", + type=float, + default=None, + metavar="SECONDS", + help="how long --approve waits before the round counts as unapproved (default: 300)", + ) plan.set_defaults(handler=_cmd_plan) + ap = sub.add_parser( + "approve", + parents=[common], + help="answer a plan run waiting on its approval gate", + ) + ap.add_argument("path", type=Path, help="the paused run's trace file (or its directory)") + ap.add_argument("--deny", action="store_true", help="refuse the plan instead of approving it") + ap.set_defaults(handler=_cmd_approve) + agent = sub.add_parser( "agent", parents=[common], help="run an agent node against a task with the core tools" ) @@ -828,6 +860,18 @@ def build_parser() -> argparse.ArgumentParser: metavar="MODULE:ATTR", help="graph registry to serve; without one the app starts with no graphs", ) + serve.add_argument( + "--live-root", + default=None, + metavar="PATH", + help="also serve a read-only live view of the trace files under PATH at /live", + ) + serve.add_argument( + "--live-token", + default=os.environ.get("GRAPHARC_LIVE_TOKEN") or None, + metavar="TOKEN", + help="require this token on every /live request (default: GRAPHARC_LIVE_TOKEN)", + ) serve.set_defaults(handler=_cmd_serve) md = sub.add_parser("models", parents=[common], help="what a model spec resolves to") diff --git a/grapharc/cli/plan.py b/grapharc/cli/plan.py index 1cb2735..9b22335 100644 --- a/grapharc/cli/plan.py +++ b/grapharc/cli/plan.py @@ -65,6 +65,12 @@ class RegistryBundle: #: the policy generator so it knows what to deny; empty means it denies #: nothing, which is why a module that can change things should say so. mutating: tuple[str, ...] = () + #: The module's own `build_loop`, when it ships one. This is how a registry + #: owns its goal check and observer instead of inheriting the incident + #: demo's (`len(notes) >= 3`) — a registry whose state never accumulates + #: three notes would otherwise burn every round and report failure after + #: doing the work. Absent, the incident builder remains the fallback. + build_loop: Any = None def resolve_registry(target: str, model: Any = None) -> RegistryBundle: @@ -108,6 +114,7 @@ def resolve_registry(target: str, model: Any = None) -> RegistryBundle: writes=getattr(module, "WRITES", None), default_policy=default_policy() if callable(default_policy) else default_policy, mutating=tuple(getattr(module, "MUTATING_KINDS", ())), + build_loop=getattr(module, "build_loop", None), ) @@ -187,6 +194,8 @@ def plan( max_tokens: int | None = None, config_path: Path | None = None, settings: Settings | None = None, + approve: bool = False, + approval_timeout: float | None = None, as_json: bool = False, ) -> int: """Run one governed planning loop against `goal`. Returns the exit code.""" @@ -228,10 +237,30 @@ def plan( except Exception as exc: # noqa: BLE001 — a backend that will not load is a setup failure return fail(f"could not build the plan: {exc}", as_json=as_json, command="plan", goal=goal) - from grapharc.examples.plan_incident import IncidentState, build_loop + from grapharc.examples.plan_incident import IncidentState + from grapharc.examples.plan_incident import build_loop as incident_build_loop schema = state_schema or IncidentState trace = TraceRecorder(trace_path) + approval = None + if approve: + import sys + + from grapharc.planner.approval_file import DEFAULT_TIMEOUT_SECONDS, file_approval + + def _announce(message: str) -> None: + # Printed *and flushed* before the run parks: a terminal user (or a + # log tailer) must learn how to answer without waiting for the exit. + print(message, flush=True, file=sys.stdout) + + approval = file_approval( + trace_path.parent, + timeout_seconds=approval_timeout or DEFAULT_TIMEOUT_SECONDS, + announce=_announce, + ) + # The registry module's own loop builder wins; the incident demo's is the + # fallback that keeps the default path byte-identical. + build_loop = bundle.build_loop or incident_build_loop loop = build_loop( model, edge_policy=edge_policy, @@ -241,6 +270,7 @@ def plan( registry=registry, state_schema=schema, writes=writes, + approval=approval, ) # `goal` is set when the schema has somewhere to put it; a custom schema is # not required to carry one, and the planner is told the goal regardless. diff --git a/grapharc/cli/serve.py b/grapharc/cli/serve.py index 357ea4e..51b0c8b 100644 --- a/grapharc/cli/serve.py +++ b/grapharc/cli/serve.py @@ -14,6 +14,7 @@ import importlib import sys +from pathlib import Path from typing import Any from grapharc.cli import optional, style @@ -65,6 +66,8 @@ def serve( port: int = 8000, log_level: str = "info", registry_target: str | None = None, + live_root: str | None = None, + live_token: str | None = None, as_json: bool = False, ) -> int: try: @@ -74,8 +77,20 @@ def serve( except optional.Unavailable as exc: return fail(str(exc), as_json=as_json, command="serve") + if live_root is not None and not Path(live_root).is_dir(): + return fail( + f"--live-root is not a directory: {live_root}", as_json=as_json, command="serve" + ) + + kwargs: dict[str, Any] = {} + if registry is not None: + kwargs["registry"] = registry + if live_root is not None: + kwargs["live_root"] = live_root + if live_token: + kwargs["live_token"] = live_token try: - app = create_app(registry=registry) if registry is not None else create_app() + app = create_app(**kwargs) except Exception as exc: # noqa: BLE001 — a config error, reported as one return fail(f"could not build the app: {exc}", as_json=as_json, command="serve") @@ -97,6 +112,7 @@ def serve( "log_level": log_level, "registry": registry_target, "graphs": names, + "live_root": live_root, } graphs = ( ", ".join(names) @@ -111,6 +127,28 @@ def serve( style.kv("graphs", graphs, width=LABEL_WIDTH, tint=None if names else style.warn), style.dim("ctrl-c to stop"), ] + # The three lines above are printed verbatim in the cookbook; anything the + # live view adds appears only when the operator asked for it. + if live_root is not None: + lines.insert( + 1, + style.kv( + "live view", + f"http://{host}:{port}/live (root: {live_root})", + width=LABEL_WIDTH, + ), + ) + if host not in ("127.0.0.1", "localhost"): + lines.insert( + 2, + style.kv( + "warning", + "binding beyond loopback exposes run telemetry; put a tunnel " + "(Tailscale/cloudflared) or --live-token in front", + width=LABEL_WIDTH, + tint=style.warn, + ), + ) # Printed *and flushed* before the server blocks: a caller watching stdout for # the URL would otherwise wait for the process to exit to learn it. Nothing # here is buffered, deferred, or drawn on a timer for the same reason. diff --git a/grapharc/examples/plan_incident.py b/grapharc/examples/plan_incident.py index 2adbae4..72c3e35 100644 --- a/grapharc/examples/plan_incident.py +++ b/grapharc/examples/plan_incident.py @@ -28,6 +28,7 @@ from grapharc.observe.trace import TraceRecorder from grapharc.planner import ( AdmissionChecker, + AdmissionLimits, CostEstimate, EdgePolicy, EdgeRule, @@ -151,6 +152,7 @@ def build_loop( registry: NodeRegistry | None = None, state_schema: type[BaseModel] | None = None, writes: dict[str, set[str]] | None = None, + approval: Any = None, ) -> GovernedLoop: """Assemble the loop from operator-owned parts. @@ -177,6 +179,11 @@ def build_loop( registry=registry, edge_policy=edge_policy or default_edge_policy(), trace=trace, + # This loop materializes each admitted round as a standalone + # graph, so structural runnability is admission's business: + # otherwise a plan with no entry is admitted and then fails to + # build, which the planner cannot replan against. + limits=AdmissionLimits(require_entry=True), ), materializer=Materializer( registry=registry, @@ -194,6 +201,7 @@ def build_loop( # supplied through `--registry` cannot turn "am I done" into an # AttributeError halfway through a run. goal_reached=lambda state: len(getattr(state, "notes", ()) or ()) >= 3, + approval=approval, ) diff --git a/grapharc/harness/agent.py b/grapharc/harness/agent.py index 566f1be..882f7ea 100644 --- a/grapharc/harness/agent.py +++ b/grapharc/harness/agent.py @@ -48,6 +48,7 @@ import inspect import json +import re import time import types import typing @@ -69,6 +70,18 @@ from grapharc.runtime.convergence import StopReason from grapharc.runtime.graph import RunContext +#: Error text that means "the CALL was shaped wrong" — the only case where +#: appending the tool's real signature helps rather than misleads. A TypeError +#: raised inside a correctly-called tool body matches none of these. +_CALL_SHAPE_ERROR = re.compile( + r"unexpected keyword argument" + r"|missing \d+ required" + r"|required positional argument" + r"|positional arguments? but" + r"|got multiple values for" + r"|must be a JSON object" +) + DEFAULT_SYSTEM_PROMPT = ( "You are a tool-using agent inside a GraphARC graph.\n" "Call a tool when you need information or an effect you cannot produce " @@ -568,6 +581,26 @@ def _charge_tokens(self, ctx: RunContext, message: AIMessage) -> int: ctx.meter.charge_tokens(total, source=message) return total + def _argument_hint(self, name: str) -> str: + """The tool's real parameter list, for an error a model must recover from.""" + spec = self.harness.registry.get(name) + if spec is None: + return "" + try: + params = inspect.signature(spec.fn).parameters.values() + except (TypeError, ValueError): + return "" + + def render(p: inspect.Parameter) -> str: + if p.kind is inspect.Parameter.VAR_POSITIONAL: + return f"*{p.name}" + if p.kind is inspect.Parameter.VAR_KEYWORD: + return f"**{p.name}" + return p.name if p.default is inspect.Parameter.empty else f"{p.name}=…" + + shown = ", ".join(render(p) for p in params) + return f" (tool {name!r} takes exactly: {shown or 'no arguments'})" + def _execute(self, ctx: RunContext, iteration: int, call: dict[str, Any]) -> ToolCallRecord: """Route one model-requested call through the harness, never around it.""" name = str(call.get("name") or "") @@ -589,6 +622,17 @@ def _execute(self, ctx: RunContext, iteration: int, call: dict[str, Any]) -> Too raise # the run's ceiling, not this tool's failure except Exception as exc: # noqa: BLE001 — a broken tool must not end the run status, detail = ToolCallStatus.ERROR, f"TOOL_ERROR: {exc}" + # A call-shape TypeError means the model invented an argument + # shape (`filename=` for `path`, a lambda over a list). The bare + # exception names the wrong argument but not the right ones, which + # leaves a weak model no way to self-correct — so name them. The + # message pattern matters: a TypeError raised *inside* a correct + # call (`len(None)`) must not earn a hint telling the model its + # valid arguments were wrong. The sandbox re-raises the child's + # failure as a RuntimeError whose message embeds `TypeError(...)`, + # so the pattern is checked on the text, not the type. + if _CALL_SHAPE_ERROR.search(detail): + detail += self._argument_hint(name) else: status, detail = ToolCallStatus.OK, self._render(value) duration_ms = (time.perf_counter() - started) * 1000 diff --git a/grapharc/observe/metrics.py b/grapharc/observe/metrics.py index 07a899f..c33e92b 100644 --- a/grapharc/observe/metrics.py +++ b/grapharc/observe/metrics.py @@ -20,6 +20,7 @@ from __future__ import annotations from collections import Counter +from typing import Any from pydantic import BaseModel @@ -101,17 +102,59 @@ def _label(text: str, limit: int = 120) -> str: return "".join(_MERMAID_ESCAPES.get(ch, ch) for ch in flat) +#: Phases that describe the run rather than doing work; the executed-path +#: fallback must never chain them as if they were steps. +_SHAPE_PHASES = frozenset({"topology", "approval_request", "approval_response"}) + +#: The governed loop's own bookkeeping. A planning round is not a node +#: execution — chaining these drew `plan -> admission -> round1 -> plan ...` +#: as though the planner's paperwork were the orchestration, which is exactly +#: the picture a run whose planning failed used to end on. +_LOOP_PHASES = frozenset({"plan", "admission", "round"}) + +#: What to draw when a run has no graph to show. Honest about *why* there is +#: nothing: a run that never got a graph admitted and built has no topology, +#: and inventing a chain from its paperwork misrepresents it. +_NO_GRAPH = 'flowchart TD\n none["no graph ran: no proposal was admitted and built"]' + +_STATUS_CLASSES = ( + " classDef done fill:#d3f2d3,stroke:#2f7d32", + " classDef running fill:#fff3cd,stroke:#b8860b", + " classDef pending fill:#eeeeee,stroke:#999999,color:#666666", + " classDef errored fill:#f8d7da,stroke:#b02a37", +) + + def to_mermaid(recorder: TraceRecorder, run_id: str) -> str: - """Render the executed path as a Mermaid flowchart (file-first, git-friendly). + """Render the run as a Mermaid flowchart (file-first, git-friendly). + + A run whose trace carries `topology` events is drawn as its *declared* + graph — every node, every edge, branches included — with execution status + overlaid per node: done, running, errored, or still pending. That is the + orchestration, not merely the path. A multi-round planner run draws one + cluster per admitted round graph. - The path is the run's node executions. A run that has none — `grapharc + A run with no topology events keeps the original rendering: the executed + path in event order. A run with no node executions at all — `grapharc agent`, which drives an `AgentNode` with no enclosing graph — still did work, so its recorded events are the path instead of an empty diagram. """ run = replay(recorder, run_id) + topologies = _latest_topologies(run.events) + if topologies: + return _topology_mermaid(run, topologies) events = [e for e in run.events if e.phase in ("end", "error")] if not events: - events = list(run.orphan_sub_events) + events = [ + e + for e in run.orphan_sub_events + if e.phase not in _SHAPE_PHASES and e.phase not in _LOOP_PHASES + ] + # A lone `stop` is a driver saying why it finished, not a path. + if all(e.phase == "stop" for e in events): + events = [] + if not events and any(e.phase in _LOOP_PHASES for e in run.events): + return _NO_GRAPH if not events: return 'flowchart TD\n empty["no events"]' lines = ["flowchart TD"] @@ -137,3 +180,123 @@ def node_ref(ev) -> str: if a.phase != "error": lines.append(f" {node_ref(a)} --> {node_ref(b)}") return "\n".join(dict.fromkeys(lines)) + + +def _latest_topologies(events: list) -> list[tuple[str, dict[str, Any]]]: + """One merged (graph, delta) per graph, in first-appearance order. + + Two emitters restate the same graph: the loop (whose delta carries + `round`/`proposal_id`/`fingerprint`) and then the kernel at invoke (whose + delta does not). Merged rather than last-wins, so the shape is the latest + statement but the labels an earlier statement carried are never lost — + last-wins made "round N" labels vanish the moment execution started. + """ + merged: dict[str, dict[str, Any]] = {} + for event in events: + if event.phase == "topology" and event.state_delta: + merged.setdefault(event.graph, {}).update(event.state_delta) + return list(merged.items()) + + +def _topology_mermaid(run: Any, topologies: list[tuple[str, dict[str, Any]]]) -> str: + lines = ["flowchart TD"] + clustered = len(topologies) > 1 + class_members: dict[str, list[str]] = {} + error_index = 0 + + for graph_index, (graph, delta) in enumerate(topologies): + # Sentinels are drawn as terminals, never as declared nodes — a delta + # that lists them (hand-written traces do) must not crash the render. + nodes = [ + str(n) for n in delta.get("nodes", []) if n not in ("__start__", "__end__") + ] + edges = [tuple(edge) for edge in delta.get("edges", [])] + fanout_sources = [str(s) for s in delta.get("fanout_sources", [])] + graph_events = [e for e in run.events if e.graph == graph] + + prefix = f"g{graph_index}_" if clustered else "" + indent = " " if clustered else " " + # Each cluster's lines are collected apart and deduped apart: a global + # dedup once collapsed every cluster's identical `end` terminator into + # one, leaving n-1 subgraphs unclosed — invalid Mermaid on every + # multi-round diagram. + cluster: list[str] = [] + + ids: dict[str, str] = {} + + def ref(name: str, prefix: str = prefix, ids: dict[str, str] = ids) -> str: + if name == "__start__": + return f"{prefix}start((start))" + if name == "__end__": + return f"{prefix}fin((end))" + node_id = ids.setdefault(name, f"{prefix}n{len(ids)}") + return f'{node_id}["{_label(name)}"]' + + # Declare every node up front: a pending node with no edges yet must + # still appear — the whole point is showing what has not run. + for name in nodes: + cluster.append(f"{indent}{ref(name)}") + + # Per-node status from this graph's events. Parallel instances of one + # node collapse to the worst-informative status: any error wins, then + # running, then done. + started: dict[str, int] = {} + ended: dict[str, int] = {} + errored: dict[str, int] = {} + for event in graph_events: + if event.phase == "start": + started[event.node] = started.get(event.node, 0) + 1 + elif event.phase == "end": + ended[event.node] = ended.get(event.node, 0) + 1 + elif event.phase == "error": + errored[event.node] = errored.get(event.node, 0) + 1 + cluster.append( + f"{indent}{ref(event.node)} -.->|error| " + f'err{error_index}{{"{_label(event.error or "error")}"}}' + ) + error_index += 1 + + for source, target, kind in (e for e in edges if len(e) == 3): + arrow = "-.->" if kind == "conditional" else "-->" + cluster.append(f"{indent}{ref(source)} {arrow} {ref(target)}") + targeted = {e[1] for e in edges if len(e) == 3} + for source in fanout_sources: + for name in nodes: + if name != source and name not in targeted and name in started: + cluster.append(f"{indent}{ref(source)} -.-> {ref(name)}") + + for name in nodes: + node_id = ids[name] + if errored.get(name): + status = "errored" + elif started.get(name, 0) > ended.get(name, 0): + status = "running" + elif ended.get(name): + status = "done" + else: + status = "pending" + class_members.setdefault(status, []).append(node_id) + + if clustered: + label = delta.get("round") + title = f"round {label}" if label else _label(graph) + # Cluster ids may not hold ':'; the raw name goes in the label. + lines.append(f' subgraph cluster{graph_index}["{_label(title)}"]') + lines.extend(dict.fromkeys(cluster)) + if clustered: + lines.append(" end") + # Each round materializes as its own standalone graph, so its entry + # really is START — but drawn alone the clusters read as unrelated + # graphs that happened to share a page. This link says what did + # happen: the state one round left is what the next one started + # from. Dotted and labelled, so it is never mistaken for an edge + # the topology declared. + if graph_index: + lines.append( + f" cluster{graph_index - 1} -.->|state| cluster{graph_index}" + ) + + lines.extend(_STATUS_CLASSES) + for status, members in class_members.items(): + lines.append(f" class {','.join(members)} {status}") + return "\n".join(lines) diff --git a/grapharc/observe/otel.py b/grapharc/observe/otel.py index 96fa44e..3d487ff 100644 --- a/grapharc/observe/otel.py +++ b/grapharc/observe/otel.py @@ -203,6 +203,11 @@ def to_spans( spans.append(sub_span) for orphan in run.orphan_sub_events: + if orphan.phase == "topology": + # The graph's declared shape, not work that happened: it has no + # duration and no parent, and a span for it would report the act of + # stating the topology as if it were an executed step. + continue # Parented to the run rather than to a guessed node: see `replay._attach`. sub_span = _sub_span(run, orphan, parent_id=root_id) if sub_span is not None: diff --git a/grapharc/observe/replay.py b/grapharc/observe/replay.py index d8e2978..90efe3f 100644 --- a/grapharc/observe/replay.py +++ b/grapharc/observe/replay.py @@ -164,6 +164,22 @@ def wall_ms(self) -> float | None: return None return round((max(stamps) - min(stamps)).total_seconds() * 1000, 2) + @property + def recorded_cost_usd(self) -> float | None: + """Cost as `observe.cost` counts it: node totals plus orphans, once. + + A model call inside an `AgentNode` lands its cost twice on the trace — + on its own `model` sub-event and again in the node's `end` aggregate — + so summing every event doubles the bill. Node terminals plus orphan + sub-events are disjoint by construction; sub-events attributed inside + a node are the breakdown of its total, never an addition to it. + """ + amounts = [e.cost_usd for e in self.executions if e.cost_usd is not None] + amounts += [ + e.cost_usd for e in self.orphan_sub_events if e.cost_usd is not None + ] + return round(sum(amounts), 6) if amounts else None + @property def errors(self) -> list[NodeExecution]: return [e for e in self.executions if e.error is not None] diff --git a/grapharc/observe/trace.py b/grapharc/observe/trace.py index 9ef46e4..9eacfe4 100644 --- a/grapharc/observe/trace.py +++ b/grapharc/observe/trace.py @@ -199,6 +199,48 @@ def run_ids(self) -> list[str]: return list(dict.fromkeys(ev.run_id for ev in self.read_events())) +class TailRecorder(TraceRecorder): + """Read-only recorder over a file another process may be mid-write in. + + `TraceRecorder.read_events` validates every line and raises on a torn one — + correct for a file the reader owns, wrong for one being appended to right + now. This override reads bytes, cuts at the last newline, and skips + anything that does not parse: the same discipline `_advance_index` applies. + It also skips the parent constructor's mkdir — a reader must not create + directories for a path that may come from a request, and the file it names + may simply not exist yet. + """ + + def __init__(self, path: str | Path) -> None: + self.path = Path(path) + self._lock = threading.Lock() + self._indexed_bytes = 0 + self._thread_max: dict[str, tuple[int, int]] = {} + + def record(self, event: TraceEvent) -> None: + raise RuntimeError("TailRecorder is read-only") + + def read_events(self, run_id: str | None = None) -> list[TraceEvent]: + try: + raw = self.path.read_bytes() + except OSError: + return [] + cut = raw.rfind(b"\n") + if cut < 0: + return [] + events = [] + for line in raw[: cut + 1].splitlines(): + if not line.strip(): + continue + try: + event = TraceEvent.model_validate_json(line) + except ValueError: + continue + if run_id is None or event.run_id == run_id: + events.append(event) + return events + + def load_events( source: TraceRecorder | str | Path, run_id: str | None = None ) -> list[TraceEvent]: @@ -207,4 +249,4 @@ def load_events( return recorder.read_events(run_id) -__all__ = ["TraceEvent", "TraceRecorder", "load_events"] +__all__ = ["TailRecorder", "TraceEvent", "TraceRecorder", "load_events"] diff --git a/grapharc/planner/admission.py b/grapharc/planner/admission.py index 447b5b4..e337908 100644 --- a/grapharc/planner/admission.py +++ b/grapharc/planner/admission.py @@ -95,6 +95,12 @@ class Check(StrEnum): BUDGET = "budget" DEPTH = "depth" ACYCLICITY = "acyclicity" + # Structural runnability: a graph with no entry, or with nodes nothing can + # reach, is not a plan even when every kind and edge is permitted. The + # materializer refused these anyway — but *after* admission had said yes, + # so the loop learned "could not be built" instead of a rejection it could + # replan against, and burned its execution-failure allowance discovering it. + REACHABILITY = "reachability" class AdmissionStatus(StrEnum): @@ -314,6 +320,15 @@ class AdmissionLimits(BaseModel): # them and a proposal that genuinely needs one has to be admitted by a # checker configured to allow it. require_acyclic: bool = True + # Whether every scope must have an entry from START and no unreachable + # node — the two conditions `Materializer` enforces when it builds a + # *standalone* graph. Off by default because admission is deliberately the + # broader gate: a proposal may name `known_nodes` of a graph already + # running, where the entry lives outside the proposal entirely. Any driver + # that materializes standalone (every `GovernedLoop` does) should turn it + # on, so a structurally unrunnable plan comes back as a rejection the + # planner can act on rather than a build failure it cannot. + require_entry: bool = False class AdmissionResult(BaseModel): @@ -430,6 +445,9 @@ def check( depth = parent_depth + proposal.nesting_depth() rejections.extend(self._check_depth(depth, parent_depth, proposal)) checks_run = [Check.REGISTRY, Check.POLICY, Check.BUDGET, Check.DEPTH] + if self.limits.require_entry: + rejections.extend(self._check_reachability(proposal)) + checks_run.append(Check.REACHABILITY) if self.limits.require_acyclic: rejections.extend(self._check_acyclicity(proposal)) checks_run.append(Check.ACYCLICITY) @@ -687,6 +705,51 @@ def _check_depth(self, depth: int, parent_depth: int, proposal: Subgraph) -> lis ) ] + def _check_reachability(self, proposal: Subgraph) -> list[Rejection]: + """Every scope needs an entry, and every node needs a way in. + + Same two conditions `Materializer` enforces, checked here so a planner + gets a rejection it can act on — with the remedy spelled out — instead + of an admitted proposal that dies at build time. + """ + out: list[Rejection] = [] + for path, _depth, sub in proposal.scopes(): + names = sub.node_names() + if not names: + continue + if not any(edge.source == START for edge in sub.edges): + out.append( + Rejection( + check=Check.REACHABILITY, + code="no_entry_edge", + subject=_scoped(path, START), + detail=( + "no edge leaves START, so the graph has no entry point " + f"and none of {sorted(names)} could run" + ), + remedy=f"add an edge from {START!r} to the first node to run", + ) + ) + continue + unreachable = sorted(names - _reachable_from_start(sub)) + for name in unreachable: + out.append( + Rejection( + check=Check.REACHABILITY, + code="unreachable_node", + subject=_scoped(path, name), + detail=( + "nothing leads to this node from START, so it would " + "never run — a proposal that does not mean what it says" + ), + remedy=( + f"add an edge from a node reachable from {START!r} to " + f"{name!r}, or drop the node" + ), + ) + ) + return out + def _check_acyclicity(self, proposal: Subgraph) -> list[Rejection]: out: list[Rejection] = [] for path, _depth, sub in proposal.scopes(): @@ -748,6 +811,22 @@ def _status_for(rejections: tuple[Rejection, ...]) -> AdmissionStatus: return AdmissionStatus.REJECTED +def _reachable_from_start(sub: Subgraph) -> set[str]: + """Nodes a walk from START can arrive at, within one scope.""" + adjacency: dict[str, list[str]] = {} + for edge in sub.edges: + adjacency.setdefault(edge.source, []).append(edge.target) + seen: set[str] = set() + stack = list(adjacency.get(START, ())) + while stack: + node = stack.pop() + if node in seen or node == END: + continue + seen.add(node) + stack.extend(adjacency.get(node, ())) + return seen + + def _scoped(path: str, subject: str) -> str: return f"{path}/{subject}" if path else subject diff --git a/grapharc/planner/approval_file.py b/grapharc/planner/approval_file.py new file mode 100644 index 0000000..6395ec9 --- /dev/null +++ b/grapharc/planner/approval_file.py @@ -0,0 +1,164 @@ +"""A file-handshake approval gate: ask on disk, answer with `grapharc approve`. + +The governed loop's `approval` callback may be anything; this is the one the +CLI wires when `--approve` is passed, chosen because it needs no server, no +socket and no state in any bot: the paused run writes a request file next to +its trace, and *any* process that can reach that directory — a terminal, the +Slack bot running `grapharc approve` — answers by writing the decision file. +Both files are transient (consumed on read); the durable record is the pair of +`approval_request` / `approval_response` events the loop writes to the trace. + +The decision must quote the request's fingerprint. That is what makes "approve +what was shown" enforceable: a decision file left over from an earlier round — +or written against a plan that has since been re-proposed — names the wrong +fingerprint and is discarded rather than trusted. +""" + +from __future__ import annotations + +import json +import os +import time +from collections.abc import Callable +from pathlib import Path +from typing import Any + +REQUEST_FILENAME = "approval-request.json" +DECISION_FILENAME = "approval-decision.json" + +DEFAULT_TIMEOUT_SECONDS = 300.0 +POLL_SECONDS = 0.5 + + +def _write_atomically(path: Path, payload: dict[str, Any]) -> None: + """tmp + rename: a reader must never see half a handshake file. + + `Path.write_text` truncates then writes — a `grapharc approve` that reads + in that window sees torn JSON, reports "nothing is waiting", and a + one-shot caller loses its chance to answer. + """ + tmp = path.with_suffix(path.suffix + ".tmp") + tmp.write_text(json.dumps(payload, indent=2), encoding="utf-8") + os.replace(tmp, path) + + +def write_request(directory: Path, request: dict[str, Any]) -> Path: + path = directory / REQUEST_FILENAME + _write_atomically(path, request) + return path + + +def read_request(directory: Path) -> dict[str, Any] | None: + """The pending request, or None when nothing is waiting.""" + path = directory / REQUEST_FILENAME + if not path.exists(): + return None + try: + loaded = json.loads(path.read_text(encoding="utf-8")) + except ValueError: + return None + return loaded if isinstance(loaded, dict) else None + + +def write_decision(directory: Path, *, fingerprint: str, decision: str) -> Path: + path = directory / DECISION_FILENAME + _write_atomically(path, {"fingerprint": fingerprint, "decision": decision}) + return path + + +def file_approval( + directory: Path, + *, + timeout_seconds: float = DEFAULT_TIMEOUT_SECONDS, + poll_seconds: float = POLL_SECONDS, + announce: Callable[[str], None] | None = None, +) -> Callable[[Any, Any], str]: + """The loop-shaped gate: blocks until a decision, a timeout, or a halt. + + Returns "approved", "denied" or "timeout" — the vocabulary + `GovernedLoop._request_approval` expects. + """ + + def gate( + proposal: Any, + verdict: Any, + should_stop: Callable[[], bool] | None = None, + ) -> str: + request = { + "proposal_id": proposal.proposal_id, + "fingerprint": proposal.fingerprint(), + "nodes": [n.name for n in proposal.nodes], + "edges": [[e.source, e.target] for e in proposal.edges], + "rationale": proposal.rationale, + "timeout_seconds": timeout_seconds, + } + request_path = directory / REQUEST_FILENAME + decision_path = directory / DECISION_FILENAME + deadline = time.monotonic() + timeout_seconds + # Everything after the question is on disk sits under the finally — + # including the announce: a broken stdout pipe must not leak a stale + # request file for a later `grapharc approve` to "answer". + try: + # A matching decision that predates the question must never + # answer it; a mismatched one is some other run's business and is + # left alone — unlinking it once destroyed a neighbour's approval. + _discard_own_stale_decision(decision_path, request["fingerprint"]) + write_request(directory, request) + if announce is not None: + try: + announce( + f"waiting for approval (up to {timeout_seconds:.0f}s) — " + f"answer with: grapharc approve {directory}" + ) + except Exception: # noqa: BLE001 — narration must not decide + pass + while time.monotonic() < deadline: + if should_stop is not None and should_stop(): + return "timeout" # the loop's halt check names the reason + decision = _read_decision(decision_path) + if decision is not None: + if decision.get("fingerprint") != request["fingerprint"]: + # Stale or another run's: ignore, never delete. + time.sleep(poll_seconds) + continue + return ( + "approved" + if decision.get("decision") == "approved" + else "denied" + ) + time.sleep(poll_seconds) + return "timeout" + finally: + request_path.unlink(missing_ok=True) + _discard_own_stale_decision(decision_path, request["fingerprint"]) + + return gate + + +def _read_decision(path: Path) -> dict[str, Any] | None: + """The decision if a complete one is on disk; None on absent/torn/racing.""" + try: + loaded = json.loads(path.read_text(encoding="utf-8")) + except (ValueError, OSError): + # Torn mid-write, or unlinked between our check and read: both mean + # "not answered yet", never an error a gate should convert to denial. + return None + return loaded if isinstance(loaded, dict) else None + + +def _discard_own_stale_decision(path: Path, fingerprint: str) -> None: + """Delete a decision naming *this* fingerprint; leave any other alone.""" + decision = _read_decision(path) + if decision is not None and decision.get("fingerprint") == fingerprint: + path.unlink(missing_ok=True) + + +__all__ = [ + "DECISION_FILENAME", + "DEFAULT_TIMEOUT_SECONDS", + "REQUEST_FILENAME", + "file_approval", + "read_request", + "write_decision", + "write_request", +] diff --git a/grapharc/planner/loop.py b/grapharc/planner/loop.py index 6a1ed82..2c92fed 100644 --- a/grapharc/planner/loop.py +++ b/grapharc/planner/loop.py @@ -107,7 +107,7 @@ from grapharc.planner.materialize import MaterializationError, Materializer from grapharc.planner.proposal import PlanningOutcome, Subgraph from grapharc.runtime.budget import Budget, BudgetExceeded, BudgetMeter -from grapharc.runtime.graph import RunContext +from grapharc.runtime.graph import RunContext, topology_delta class LoopStop(StrEnum): @@ -134,6 +134,12 @@ class LoopStop(StrEnum): PLANNING_FAILED = "planning_failed" EXECUTION_FAILED = "execution_failed" HUMAN_STOPPED = "human_stopped" + # A configured approval gate said no — or nobody answered it in time. + # Distinct reasons on purpose: a denial is a decision, a timeout is the + # absence of one, and an operator reading the trace should not have to + # guess which happened. + APPROVAL_DENIED = "approval_denied" + APPROVAL_TIMEOUT = "approval_timeout" # Stops that mean the run did what it was asked, for the trace's `error` field: @@ -291,6 +297,7 @@ def __init__( progress: Callable[[Any], Any] | None = None, checkpointer: Any = None, parent_depth: int = 0, + approval: Callable[[Subgraph, AdmissionResult], str] | None = None, ) -> None: self.planner = planner self.checker = checker @@ -312,6 +319,12 @@ def __init__( if parent_depth < 0: raise ValueError(f"parent_depth must be >= 0, got {parent_depth}") self.parent_depth = parent_depth + # A human gate between admission and execution. Called with each + # admitted round's proposal and verdict; must return "approved", + # "denied" or "timeout", and may block while a human decides. The wait + # charges nothing — thinking time is not the run's spend. None means + # what it always meant: admission is the only gate. + self.approval = approval self._halt = threading.Event() self._halt_reason = "" @@ -355,6 +368,12 @@ def run( current = self._initial_state(state) rounds: list[RoundRecord] = [] feedback = "" + # Every rejection so far, not just the last: a model shown only round + # N-1's refusal re-proposes round N-2's mistake with no way to notice + # the pattern, and can exhaust max_consecutive_rejections repeating + # itself. Bounded (the last few) because the text lands verbatim in + # the planner's prompt every round. + rejection_log: list[str] = [] note = "" rejected_in_a_row = 0 unplanned_in_a_row = 0 @@ -398,6 +417,16 @@ def close(**fields: Any) -> None: break feedback = "" + if getattr(outcome, "unreachable", False): + # The backend was not reached. Replanning re-dials the same + # dead socket, so the loop would spend its whole planning + # allowance discovering that and then report a *planning* + # failure for an infrastructure one. Stop on the first. + stop = LoopStop.PLANNING_FAILED + detail = f"the planner's backend could not be reached: {outcome.error}" + close(planner_error=outcome.error, tokens=outcome.tokens) + break + if not outcome.ok or outcome.proposal is None: unplanned_in_a_row += 1 note = ( @@ -422,7 +451,8 @@ def close(**fields: Any) -> None: if not verdict.admitted: rejected_in_a_row += 1 - feedback = verdict.feedback() + rejection_log.append(f"Round {round_number}:\n{verdict.feedback()}") + feedback = "\n\n".join(rejection_log[-3:]) if rejected_in_a_row >= self.limits.max_consecutive_rejections: stop = LoopStop.ADMISSION_REFUSED detail = ( @@ -589,6 +619,56 @@ def _execute( except MaterializationError as exc: return _Execution(state=state, materialization_error=f"could not be built: {exc}") + if self.trace is not None: + # The admitted round's shape, on disk *before* anything executes — + # what an approval gate shows, and what a live view greys in as + # "planned" while nodes are still pending. Same graph name as the + # kernel's own topology event at invoke, so readers dedupe the two. + self.trace.event( + run_id=ctx.run_id, + thread_id=ctx.thread_id, + attempt=ctx.attempt, + graph=compiled.arc.name, + node="topology", + phase="topology", + step=0, + state_delta={ + **topology_delta(compiled.arc), + "round": round_number, + "proposal_id": proposal.proposal_id, + "fingerprint": proposal.fingerprint(), + }, + ) + + if self.approval is not None: + parked = time.monotonic() + decision = self._request_approval(proposal, verdict, ctx, round_number) + # The wait charges nothing, seconds included: `elapsed_seconds` is + # wall clock from run() start, and without this credit a human who + # took longer than `max_seconds` to say yes handed the round a + # negative budget — approved, then dead before its first node. + meter.credit_seconds(time.monotonic() - parked) + if self._halt.is_set(): + # An emergency stop issued while parked must win over any + # decision that raced it — a halted loop must not execute. + return _Execution( + state=state, + execution_error=self._halt_reason + or "halted while awaiting approval", + hard_stop=LoopStop.HUMAN_STOPPED, + ) + if decision != "approved": + stop = ( + LoopStop.APPROVAL_TIMEOUT + if decision == "timeout" + else LoopStop.APPROVAL_DENIED + ) + return _Execution( + state=state, + execution_error=f"the admitted plan was not approved ({decision})", + hard_stop=stop, + ) + budget = self._round_budget(meter) try: raw = compiled.invoke( @@ -615,6 +695,61 @@ def _execute( executed=True, state=self._initial_state(raw), iterations=iterations ) + def _request_approval( + self, + proposal: Subgraph, + verdict: AdmissionResult, + ctx: RunContext, + round_number: int, + ) -> str: + """Ask the configured gate; both the question and the answer are audited. + + The request event carries the same nodes/edges the topology event does, + so a reader showing "approve this?" needs nothing but the trace. The + callback may block indefinitely from the loop's point of view — bounding + the wait is the callback's job, reported as a "timeout" decision. + """ + self._emit( + ctx, + node=f"{self.name}:approval", + phase="approval_request", + state_delta={ + "round": round_number, + "proposal_id": proposal.proposal_id, + "fingerprint": proposal.fingerprint(), + "nodes": [n.name for n in proposal.nodes], + "edges": [[e.source, e.target] for e in proposal.edges], + }, + ) + try: + if self.approval is None: + decision = "approved" + elif _accepts_should_stop(self.approval): + # A gate that can watch for a halt is told how: it should + # return early (any non-approved answer) when this fires. + decision = self.approval( + proposal, verdict, should_stop=self._halt.is_set + ) + else: + decision = self.approval(proposal, verdict) + except Exception as exc: # noqa: BLE001 — a broken gate must fail closed + decision = f"denied (approval gate raised: {exc})" + normalized = ( + decision if decision in ("approved", "timeout") else "denied" + ) + self._emit( + ctx, + node=f"{self.name}:approval", + phase="approval_response", + state_delta={ + "round": round_number, + "proposal_id": proposal.proposal_id, + "decision": normalized, + "detail": decision if normalized == "denied" else "", + }, + ) + return normalized + def _round_budget(self, meter: BudgetMeter) -> Budget: """This round's ceiling: exactly what the run has left, per dimension. @@ -622,10 +757,12 @@ def _round_budget(self, meter: BudgetMeter) -> Budget: round cannot spend what the earlier ones already did. """ left = RemainingBudget.from_meter(meter) + # Floored at zero: "already over" must read as an exhausted budget, + # never as a negative ceiling in an error message. return Budget( - max_iterations=left.iterations, - max_tokens=left.tokens, - max_seconds=left.seconds, + max_iterations=None if left.iterations is None else max(0, left.iterations), + max_tokens=None if left.tokens is None else max(0, left.tokens), + max_seconds=None if left.seconds is None else max(0.0, left.seconds), max_concurrency=self.budget.max_concurrency, ) @@ -709,6 +846,19 @@ def _emit(self, ctx: RunContext, *, node: str, phase: str, **fields: Any) -> Non ) +def _accepts_should_stop(gate: Any) -> bool: + """Whether an approval gate takes the optional `should_stop` keyword.""" + import inspect + + try: + parameters = inspect.signature(gate).parameters + except (TypeError, ValueError): + return False + return "should_stop" in parameters or any( + p.kind is inspect.Parameter.VAR_KEYWORD for p in parameters.values() + ) + + def _left(value: float | int | None) -> str: return "unlimited" if value is None else f"{value:g}" diff --git a/grapharc/planner/proposal.py b/grapharc/planner/proposal.py index b37e415..bd2b4d2 100644 --- a/grapharc/planner/proposal.py +++ b/grapharc/planner/proposal.py @@ -268,6 +268,21 @@ def fingerprint(self) -> str: "re-proposing a refused node under a different name is a wasted turn.\n" f"Edge endpoints must be nodes you proposed, or the literals {START!r} " f"(graph entry) and {END!r} (graph exit).\n" + # These three were the structural mistakes real models actually made, over + # and over: naming a node with the entry literal, proposing a set of nodes + # with nothing leading in from it, and leaving a node no edge reaches. Each + # was refused with a correct reason the model then had to infer the rule + # from; stating the rule up front is cheaper than three wasted rounds. + f"{START!r} and {END!r} are the graph's own entry and exit. Never use " + "either as a node `name` — they are endpoints you connect to, not steps " + "you declare.\n" + f"Exactly one thing must be true of every proposal you make: there is an " + f"edge from {START!r} to the first node, and every other node is reachable " + f"by following edges from there. A node nothing leads to would never run.\n" + f"Give the last node an edge to {END!r}.\n" + "Nodes that should run at the same time all take an edge from the same " + "predecessor; nodes that must wait for several others all take an edge " + "into the same successor. That is how you express parallelism and joins.\n" "Leave `subgraph` unset on every node.\n" "Propose no nodes at all when there is no further work to do.\n" "Admission is deterministic code, not a conversation: arguing with a " @@ -277,6 +292,36 @@ def fingerprint(self) -> str: _NO_CATALOG = "(no catalog supplied; the admission registry decides what is allowed)" +#: Exception *names* that mean "the backend was not reached", matched by name +#: so this module needs no provider SDK imported to recognise them. Substrings +#: on purpose: every provider spells its own (`APIConnectionError`, +#: `APITimeoutError`, `AuthenticationError`, `RateLimitError`…), and they all +#: share the property that matters — another turn cannot fix them. +_UNREACHABLE_MARKERS = ( + "connectionerror", + "connecterror", + "connectionrefused", + "timeout", + "authentication", + "permissiondenied", + "ratelimit", + "insufficient", + "quota", + "serviceunavailable", +) + + +def _is_unreachable(exc: BaseException) -> bool: + """Whether the backend refused or could not be reached at all. + + A model that answered badly is a planning problem; a socket that would not + open is not. Only the latter makes retrying pointless, so only the latter + is reported as unreachable. + """ + name = type(exc).__name__.lower() + return any(marker in name for marker in _UNREACHABLE_MARKERS) + + def _catalog_text(catalog: Mapping[str, str] | Sequence[str] | None) -> str: if not catalog: return _NO_CATALOG @@ -314,6 +359,12 @@ class PlanningOutcome(BaseModel): # have different failure modes and an audit should not have to guess. structured: bool = False tokens: int = 0 + # True when the backend could not be reached or refused the call outright + # (connection refused, auth, quota). A driver must not treat this as "the + # model gave a bad answer": rephrasing cannot reach a server that is down, + # so a replanning loop would just re-dial a dead socket until its patience + # ran out and then report a *planning* failure for an infrastructure one. + unreachable: bool = False @property def ok(self) -> bool: @@ -418,6 +469,7 @@ def propose( raw_message: BaseMessage | None = None proposal: Subgraph | None = None error = "" + unreachable = False try: if structured: envelope = runnable.invoke(messages) @@ -438,6 +490,7 @@ def propose( error = f"proposal did not validate: {_first_validation_error(exc)}" except Exception as exc: # noqa: BLE001 — a bad turn is an outcome, not a crash error = f"planner model call failed: {exc!r}" + unreachable = _is_unreachable(exc) raw_text = _message_text(raw_message) if raw_message is not None else "" tokens = self._charge(ctx, raw_message) @@ -463,7 +516,12 @@ def propose( error=error or None, ) return PlanningOutcome( - proposal=proposal, error=error, raw=raw_text, structured=structured, tokens=tokens + proposal=proposal, + error=error, + raw=raw_text, + structured=structured, + tokens=tokens, + unreachable=unreachable and proposal is None, ) # -- internals ------------------------------------------------------------ diff --git a/grapharc/runtime/budget.py b/grapharc/runtime/budget.py index 8ee4761..98667ea 100644 --- a/grapharc/runtime/budget.py +++ b/grapharc/runtime/budget.py @@ -146,6 +146,19 @@ def charge_iteration(self, n: int = 1) -> None: with self._lock: self._iterations += n + def credit_seconds(self, seconds: float) -> None: + """Exclude a stretch of wall clock from `max_seconds`. + + For time the run spent deliberately parked — a human deciding at an + approval gate — which is nobody's spend. Implemented by moving the + start mark forward, so `elapsed_seconds` simply never saw the wait. + Negative or zero credits are ignored rather than trusted. + """ + if seconds <= 0: + return + with self._lock: + self._started_at += seconds + def charge_tokens(self, n: int, *, automatic: bool = False, source: Any = None) -> None: """Add `n` tokens to the run's total. diff --git a/grapharc/runtime/graph.py b/grapharc/runtime/graph.py index b72b884..9058646 100644 --- a/grapharc/runtime/graph.py +++ b/grapharc/runtime/graph.py @@ -302,6 +302,12 @@ def __init__( self._nodes: dict[str, set[str]] = {} self._async_nodes: set[str] = set() self._static_edges: list[tuple[str, str]] = [] + # Conditional routes are topology too: the mapping's targets are known + # statically, and a diagram that omitted them would show a branch node + # with no way out. Fan-out targets are dynamic, so only the source is + # recorded and a renderer connects the workers that actually ran. + self._conditional_edges: list[tuple[str, str]] = [] + self._fanout_sources: list[str] = [] self._adapters: dict[str, TypeAdapter[Any]] = {} def add_node( @@ -345,6 +351,9 @@ def add_conditional_edge( f"graph {self.name!r} is dag=True: conditional edges are not allowed" ) self._graph.add_conditional_edges(source, router, mapping) + self._conditional_edges.extend( + (source, target) for target in dict.fromkeys(mapping.values()) + ) return self def add_fanout_edge( @@ -372,6 +381,7 @@ def dispatch(state: Any) -> list[Send]: return sends self._graph.add_conditional_edges(source, dispatch) + self._fanout_sources.append(source) return self def compile(self, checkpointer: Any = None) -> CompiledGraphARC: @@ -742,6 +752,25 @@ def wrapped(state: Any, config: RunnableConfig) -> Any: return wrapped +def topology_delta(arc: GraphARC) -> dict[str, Any]: + """The declared shape of a graph, as one trace-event `state_delta`. + + Node order is declaration order (a proposal's order, for materialized + graphs). Edge triples carry their kind: `static` edges run unconditionally, + `conditional` targets are a router's statically-known options. Fan-out + targets are dynamic, so only the sources are named — a renderer connects + them to the workers that actually ran. + """ + edges = [[source, target, "static"] for source, target in arc._static_edges] + edges += [ + [source, target, "conditional"] for source, target in arc._conditional_edges + ] + delta: dict[str, Any] = {"nodes": list(arc._nodes), "edges": edges} + if arc._fanout_sources: + delta["fanout_sources"] = list(arc._fanout_sources) + return delta + + class CompiledGraphARC: """A compiled graph plus GraphARC run semantics (run ids, budgets, resume).""" @@ -768,6 +797,24 @@ def _run_config( step_seed=step_seed, ) self.last_run = ctx + if self.arc.trace is not None: + # The graph's declared shape, into the same audit trail its + # execution lands in. Until this event existed, a diagram could + # only ever show the path that ran — branches not taken, parallel + # structure, and nodes that never started were unrecoverable from + # the trace. Emitted per entry (a resumed attempt re-states it); + # readers keep the latest per graph. `step=0` sits below every real + # step, so readers comparing step ranges filter this phase out. + self.arc.trace.event( + run_id=rid, + graph=self.arc.name, + node="topology", + phase="topology", + step=0, + thread_id=thread, + attempt=attempt, + state_delta=topology_delta(self.arc), + ) config: dict[str, Any] = {"configurable": {"thread_id": thread, "grapharc_ctx": ctx}} limit = (budget or self.arc.budget or _NOOP_BUDGET).max_concurrency if limit is not None: diff --git a/grapharc/server/app.py b/grapharc/server/app.py index 303e870..4bc1616 100644 --- a/grapharc/server/app.py +++ b/grapharc/server/app.py @@ -90,6 +90,8 @@ def create_app( max_workers: int | None = None, poll_seconds: float = DEFAULT_POLL_SECONDS, keepalive_seconds: float | None = DEFAULT_KEEPALIVE_SECONDS, + live_root: str | Path | None = None, + live_token: str | None = None, ) -> FastAPI: """Build the app. @@ -99,6 +101,11 @@ def create_app( configure the default runtime and mean nothing next to a supplied one. With neither, the registry is empty and every create request gets a 404 naming that fact. + + `live_root` mounts the read-only live view (`grapharc.server.live`) over + the trace files under that directory. It is storage to *read*, not runtime + configuration, so it composes with either a registry or a supplied runtime + — the Slack topology is a live root and no registry at all. """ if runtime is not None and any( arg is not None for arg in (registry, root, max_workers) @@ -126,6 +133,11 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: ) app.state.runtime = session_runtime + if live_root is not None: + from grapharc.server.live import live_router + + app.include_router(live_router(Path(live_root), token=live_token)) + @app.exception_handler(UnknownSessionError) def _unknown_session(request: Request, exc: UnknownSessionError) -> JSONResponse: return JSONResponse(status_code=404, content={"detail": str(exc)}) diff --git a/grapharc/server/live.py b/grapharc/server/live.py new file mode 100644 index 0000000..3bf3eae --- /dev/null +++ b/grapharc/server/live.py @@ -0,0 +1,461 @@ +"""A live browser view over trace files another process is appending. + +Mounted into the FastAPI app only when the operator passes `--live-root` to +`grapharc serve`. The routes are GET-only readers over JSONL trace files under +that root — typically the Slack bot's working directory, where the gate gives +every tracing command a trace path it knows — so the topology is: runs happen +in CLI subprocesses, this server only reads their files, and the Slack bot +(which never opens a port) merely posts a URL pointing here. + +The server does the recomputation and the page stays dumb: each SSE `snapshot` +frame carries the rendered Mermaid source and summary numbers, never raw trace +events. That is a security decision as much as a simplicity one — `state_delta` +can hold anything a run's nodes wrote, and its *contents* are deliberately not +served. The exposure is the same as the `viz`/`metrics` commands': node names, +error labels, counts, and the one state field `summarize` lifts out of the +delta — `termination_reason`, a short reason string by convention. + +Reachability is the operator's problem by design: bind stays loopback unless +they choose otherwise, and the recommended remote path is a tunnel or tailnet +in front, optionally with the shared-secret `token` (a convenience lock, not a +perimeter — it rides in the query string because `EventSource` cannot set +headers). +""" + +from __future__ import annotations + +import asyncio +import html +import secrets +from pathlib import Path +from time import monotonic, time +from typing import Any +from urllib.parse import quote + +from fastapi import APIRouter, HTTPException, Query, Request +from fastapi.responses import HTMLResponse, StreamingResponse +from pydantic import BaseModel + +from grapharc.observe.metrics import RunMetrics, summarize, to_mermaid +from grapharc.observe.replay import replay +from grapharc.observe.trace import TailRecorder +from grapharc.slack.format import mermaid_live_url + +#: How often the stream re-stats the trace file. Coarser than the session +#: stream's 0.02s: every change here costs a full re-read and replay of the +#: file, not a list slice. +LIVE_POLL_SECONDS = 0.5 +LIVE_KEEPALIVE_SECONDS = 15.0 +#: Keep streaming this long after a termination reason appears, so the final +#: snapshot (and any last events) reach the page before the stream closes. +DONE_GRACE_SECONDS = 5.0 +#: A file that grew within this window counts as an active run. +ACTIVE_WINDOW_SECONDS = 10.0 +#: How long an open (started, never ended) node keeps a quiet run reading as +#: "running". A delegated executor is silent mid-flight, so quiet ≠ idle — but +#: past this, an open node is a run that died mid-node, not one still working. +OPEN_NODE_GRACE_SECONDS = 900.0 + + +class LivePathError(Exception): + """The requested trace is not one this server will read.""" + + +def resolve_trace(root: Path, raw: str) -> Path: + """Confine a requested trace path inside the root, gate-style. + + Lexical resolution only — the target need not exist (the run may not have + started yet). Absolute paths, traversal out of the root (symlinks + included, via resolve()), and non-`.jsonl` names are all refused. + """ + if not raw or Path(raw).is_absolute(): + raise LivePathError(f"not a relative trace path: {raw!r}") + if not raw.endswith(".jsonl"): + raise LivePathError(f"not a .jsonl trace: {raw!r}") + resolved = (root / raw).resolve() + if not resolved.is_relative_to(root.resolve()): + raise LivePathError(f"path escapes the live root: {raw!r}") + return resolved + + +class LiveSnapshot(BaseModel): + """Everything one page render needs; recomputed server-side per change.""" + + trace: str + run_id: str | None = None + run_ids: list[str] = [] + mermaid: str = "" + mermaid_live_url: str | None = None + stats: RunMetrics | None = None + cost_usd: float | None = None + wall_ms: float | None = None + last_event_ts: str | None = None + size: int = 0 + active: bool = False + done: bool = False + awaiting_approval: bool = False + + +def build_snapshot(root: Path, rel: str, run_id: str | None) -> LiveSnapshot: + """One consistent read of the trace file. Pure; run it in a thread. + + A missing or empty file is a waiting snapshot, not an error — the URL is + posted before the run starts writing. + """ + path = resolve_trace(root, rel) + recorder = TailRecorder(path) + events = recorder.read_events() + if not events: + return LiveSnapshot(trace=rel) + + run_ids = list(dict.fromkeys(e.run_id for e in events)) + chosen = run_id if run_id in run_ids else run_ids[-1] + run_events = [e for e in events if e.run_id == chosen] + + try: + size = path.stat().st_size + quiet_for = time() - path.stat().st_mtime + except OSError: + size, quiet_for = 0, float("inf") + active = quiet_for < ACTIVE_WINDOW_SECONDS + + mermaid = to_mermaid(recorder, chosen) + run = replay(recorder, chosen) + # Finished means: something wrote a termination reason, OR a driver wrote + # its terminal `stop` event — the planner writes the latter and never the + # former, and keying on `termination_reason` alone held the SSE stream + # open forever for every planner run. + done = any( + e.phase == "stop" or "termination_reason" in (e.state_delta or {}) + for e in run_events + ) + # A node that started and never ended means someone is inside it right now + # — a delegated executor writes nothing between its start and its finish, + # so "the file went quiet" must not read as idle while a node is open. + # Bounded, though: a run killed mid-node leaves its node open forever, and + # an hour-quiet open node is a corpse, not work. + if not done and quiet_for < OPEN_NODE_GRACE_SECONDS and any( + not e.completed for e in run.executions + ): + active = True + # An approval request with no later response: the run is deliberately + # parked, and the page should say "awaiting approval", not "idle". + awaiting = False + for event in run_events: + if event.phase == "approval_request": + awaiting = True + elif event.phase == "approval_response": + awaiting = False + if awaiting and not done: + active = True + return LiveSnapshot( + trace=rel, + run_id=chosen, + run_ids=run_ids, + mermaid=mermaid, + mermaid_live_url=mermaid_live_url(mermaid), + stats=summarize(recorder, chosen), + cost_usd=run.recorded_cost_usd, + wall_ms=run.wall_ms, + last_event_ts=run_events[-1].ts if run_events else None, + size=size, + active=active, + done=done, + awaiting_approval=awaiting, + ) + + +#: How many of the newest traces the index fully parses for run ids. The rest +#: are listed by name and size only: the live root accumulates one directory +#: per run forever, and re-validating every byte of the whole corpus per +#: index refresh made history the server's dominant cost. +SCAN_PARSE_LIMIT = 25 + + +def scan_traces(root: Path) -> list[dict[str, Any]]: + """Every trace file under the root, newest first. + + Run ids are parsed only for the `SCAN_PARSE_LIMIT` newest files; older + rows carry an empty `runs` list — their viewer pages still work (the run + is resolved from the file when the page opens). + """ + found = [] + for path in root.rglob("*.jsonl"): + if not path.is_file(): + continue + try: + stat = path.stat() + except OSError: + continue + found.append( + { + "trace": path.relative_to(root).as_posix(), + "size": stat.st_size, + "mtime": stat.st_mtime, + "runs": [], + "_path": path, + } + ) + found.sort(key=lambda item: item["mtime"], reverse=True) + for item in found[:SCAN_PARSE_LIMIT]: + try: + item["runs"] = list( + dict.fromkeys( + e.run_id for e in TailRecorder(item["_path"]).read_events() + ) + ) + except OSError: + pass + for item in found: + del item["_path"] + return found + + +def live_router( + root: str | Path, + *, + token: str | None = None, + poll_seconds: float = LIVE_POLL_SECONDS, + keepalive_seconds: float | None = LIVE_KEEPALIVE_SECONDS, +) -> APIRouter: + """GET-only routes under /live, all confined to `root`.""" + root_path = Path(root) + router = APIRouter(prefix="/live") + + def _authorized(request: Request) -> None: + if token is None: + return + supplied = request.query_params.get("token") + header = request.headers.get("authorization", "") + if header.startswith("Bearer "): + supplied = supplied or header.removeprefix("Bearer ") + if supplied is None or not secrets.compare_digest(supplied, token): + raise HTTPException(status_code=401, detail="missing or wrong token") + + def _resolved(raw: str) -> str: + """Validate confinement; 404 on refusal (don't map what exists outside).""" + try: + resolve_trace(root_path, raw) + except LivePathError: + raise HTTPException(status_code=404, detail="no such trace") from None + return raw + + @router.get("", response_class=HTMLResponse, include_in_schema=False) + def index(request: Request) -> HTMLResponse: + _authorized(request) + keep = f"&token={quote(token, safe='')}" if token else "" + rows = [] + for item in scan_traces(root_path): + # Trace names and run ids come from whoever wrote the files — the + # Slack gate checks path *containment*, not characters — so every + # interpolation here is escaped: unescaped, a filename like + # `x"> +""" + +__all__ = [ + "ACTIVE_WINDOW_SECONDS", + "DONE_GRACE_SECONDS", + "LIVE_KEEPALIVE_SECONDS", + "LIVE_POLL_SECONDS", + "LivePathError", + "LiveSnapshot", + "TailRecorder", + "build_snapshot", + "live_router", + "resolve_trace", + "scan_traces", +] diff --git a/grapharc/slack/__init__.py b/grapharc/slack/__init__.py index 4501c96..e536edf 100644 --- a/grapharc/slack/__init__.py +++ b/grapharc/slack/__init__.py @@ -14,12 +14,19 @@ command.py what Slack text is allowed to become an argv (the gate) runner.py run an argv against this interpreter's grapharc, with a timeout format.py turn an exit code and captured output into one Slack message + live.py tail a running command's trace and narrate it via a callback bot.py slack-bolt wiring; the only file that imports slack_bolt config.py tokens and limits from the environment, nothing else Only `bot.py` needs the `slack` extra, and it imports it lazily — every other -module (and this package) is stdlib-only, so a wheel without the extra still -imports. +module (and this package) needs nothing beyond grapharc's own core +dependencies, so a wheel without the extra still imports. + +A tracing command (`demo`, `run`, `plan`, `agent`) is narrated live: the gate +gives it a trace path the bot knows, `live.py` tails that file while the +subprocess runs, and the bot edits one status message in place — a node +progress line per event batch plus a refreshed mermaid.live diagram link. +Every failure in that path degrades to the plain blocking reply. The gate's default is deliberately spend-free: `serve` is refused, `agent` and `--model` are refused unless the operator opts in (`agent` needs two switches: diff --git a/grapharc/slack/bot.py b/grapharc/slack/bot.py index 684ef2b..bcebc49 100644 --- a/grapharc/slack/bot.py +++ b/grapharc/slack/bot.py @@ -1,11 +1,21 @@ """slack-bolt wiring: the only module that touches Slack itself. -Everything with behaviour lives in `command`/`runner`/`format`; what remains -here is `handle_text` (their composition, still import-safe without slack-bolt -and tested that way) and the listener glue. Slack requires an ack within three -seconds, so each listener acks with "running…" first and posts the result when -the command finishes — bolt runs listeners on worker threads, so a slow -command blocks neither the socket nor other requests. +Everything with behaviour lives in `command`/`runner`/`format`/`live`; what +remains here is `handle_text_live` (their composition, still import-safe +without slack-bolt and tested that way with a fake sink) and the listener +glue. Slack requires an ack within three seconds, so each listener acks first +— bolt runs listeners on worker threads, so a slow command blocks neither the +socket nor other requests. + +A tracing command gets a live status message: one `chat.postMessage` when it +starts, edited in place (`chat.update`) as trace events arrive, and edited one +last time into the final result. Note the visibility change this brings to the +slash path: `respond()` output was ephemeral by default, a posted status +message is visible to the channel — deliberate, since a live run is channel +activity, and the ephemeral behaviour survives wherever the bot cannot post +(not in the channel, API error): every failure in the live path falls back to +today's blocking reply through `respond()`/`say()`, and the final result is +never lost to a failed edit. `slack_bolt` is imported inside `build_app`, not at module top: the wheel-check imports every module in an environment without the extra, and a user who never @@ -15,11 +25,15 @@ from __future__ import annotations import re +import shlex from typing import Any -from grapharc.slack.command import SlackCommandError, parse_command, usage_text +from grapharc.observe.metrics import to_mermaid +from grapharc.observe.trace import TailRecorder +from grapharc.slack.command import SlackCommandError, parse_command, trace_path, usage_text from grapharc.slack.config import SlackBotConfig -from grapharc.slack.format import format_result +from grapharc.slack.format import format_result, live_view_url, mermaid_live_url +from grapharc.slack.live import LiveSettings, LiveSink, LiveTail from grapharc.slack.runner import run_command # An app_mention's text arrives as "<@U0BOTID> metrics t.jsonl r1". @@ -28,6 +42,17 @@ def handle_text(text: str, config: SlackBotConfig) -> str: """Gate, run, format: the whole request path, with Slack stripped away.""" + return handle_text_live(text, config, sink=None) + + +def handle_text_live(text: str, config: SlackBotConfig, sink: LiveSink | None) -> str: + """Like `handle_text`, but narrates a tracing command through `sink`. + + Returns the message the caller must still post — `""` when the sink + already delivered the final result by editing the status message. Any + failure to post or edit degrades to the plain blocking path; the final + result always reaches the requester through one route or the other. + """ stripped = _MENTION.sub("", text).strip() try: argv = parse_command( @@ -39,10 +64,114 @@ def handle_text(text: str, config: SlackBotConfig) -> str: ) except SlackCommandError as exc: return str(exc) - result = run_command( - argv, workdir=config.workdir, timeout_seconds=config.timeout_seconds - ) - return format_result(result) + + tpath = trace_path(argv, config.workdir) + # Run ids already in the file, noted before the run: on a reused trace, + # the final diagram must be *this* run's, and if this run wrote nothing + # (failed before its first event) there must be no diagram at all — not a + # previous run's presented as the outcome. + prior_runs: frozenset[str] = frozenset() + if tpath is not None and tpath.exists(): + try: + prior_runs = frozenset(TailRecorder(tpath).run_ids()) + except Exception: + pass + handle: object | None = None + if config.live and sink is not None and tpath is not None: + try: + handle = sink.post(_starting_text(argv, config)) + except Exception: + handle = None + + if handle is None: + result = run_command( + argv, workdir=config.workdir, timeout_seconds=config.timeout_seconds + ) + return _with_final_links(format_result(result), argv, tpath, config, prior_runs) + + def _update(message: str) -> bool: + try: + return sink.update(handle, message) + except Exception: + return False + + settings = LiveSettings(update_interval=config.live_interval_seconds) + with LiveTail(tpath, argv, _update, settings): + result = run_command( + argv, workdir=config.workdir, timeout_seconds=config.timeout_seconds + ) + final = _with_final_links(format_result(result), argv, tpath, config, prior_runs) + if _update(final): + return "" + return final + + +def _with_final_links( + final: str, + argv: list[str], + tpath, + config: SlackBotConfig, + prior_runs: frozenset[str] = frozenset(), +) -> str: + """Keep the diagram and run-page links on the *final* message. + + The final result edits over the live status message, which is where the + "watch live" link lived — without this, finishing a run is what makes its + links disappear. Both are best-effort: a link that cannot be computed is + simply absent. + """ + if tpath is None: + return final + lines = [final] + try: + recorder = TailRecorder(tpath) + new_runs = [r for r in recorder.run_ids() if r not in prior_runs] + if new_runs: + diagram = to_mermaid(recorder, new_runs[-1]) + lines.append(f"<{mermaid_live_url(diagram)}|final diagram>") + except Exception: + pass + url = live_view_url(argv, base=config.live_url_base, workdir=config.workdir) + if url: + lines.append(f"run page: {url}") + return "\n".join(lines) + + +def _starting_text(argv: list[str], config: SlackBotConfig) -> str: + lines = [f"`{shlex.join(['grapharc', *argv])}` — starting…"] + url = live_view_url(argv, base=config.live_url_base, workdir=config.workdir) + if url: + lines.append(f"watch live: {url} (if the live server is up)") + return "\n".join(lines) + + +class _ChannelSink: + """A `LiveSink` over a slack-sdk WebClient; failures are values, not raises.""" + + def __init__(self, client: Any, channel: str, thread_ts: str | None = None) -> None: + self._client = client + self._channel = channel + self._thread_ts = thread_ts + + def post(self, text: str) -> object | None: + if not self._channel: + return None + try: + kwargs: dict[str, Any] = {"channel": self._channel, "text": text} + if self._thread_ts: + kwargs["thread_ts"] = self._thread_ts + response = self._client.chat_postMessage(**kwargs) + return (response["channel"], response["ts"]) + except Exception: + return None + + def update(self, handle: object, text: str) -> bool: + try: + channel, ts = handle # type: ignore[misc] + self._client.chat_update(channel=channel, ts=ts, text=text) + return True + except Exception: + return False def build_app(config: SlackBotConfig) -> Any: @@ -58,20 +187,24 @@ def build_app(config: SlackBotConfig) -> Any: app = App(token=config.bot_token) @app.command(config.slash_command) - def _slash(ack: Any, respond: Any, command: dict[str, Any]) -> None: + def _slash(ack: Any, respond: Any, command: dict[str, Any], client: Any) -> None: text = command.get("text", "").strip() if not text: ack(usage_text(allow_model=config.allow_model, allow_agent=config.allow_agent)) return ack(f"running `grapharc {text}`…") - respond(handle_text(text, config)) + sink = _ChannelSink(client, command.get("channel_id", "")) + reply = handle_text_live(text, config, sink) + if reply: + respond(reply) @app.event("app_mention") - def _mention(event: dict[str, Any], say: Any) -> None: - say( - handle_text(event.get("text", ""), config), - thread_ts=event.get("thread_ts") or event.get("ts"), - ) + def _mention(event: dict[str, Any], say: Any, client: Any) -> None: + thread_ts = event.get("thread_ts") or event.get("ts") + sink = _ChannelSink(client, event.get("channel", ""), thread_ts=thread_ts) + reply = handle_text_live(event.get("text", ""), config, sink) + if reply: + say(reply, thread_ts=thread_ts) return app diff --git a/grapharc/slack/command.py b/grapharc/slack/command.py index 1aefffe..11fef50 100644 --- a/grapharc/slack/command.py +++ b/grapharc/slack/command.py @@ -30,7 +30,9 @@ from __future__ import annotations import shlex +import uuid from dataclasses import dataclass, field +from datetime import UTC, datetime from pathlib import Path @@ -65,9 +67,23 @@ class CommandSpec: { "grapharc.examples.plan_incident:build_registry", "grapharc.examples.plan_docs:build_registry", + "grapharc.stdlib:build_registry", } ) +#: The one plan registry whose kinds execute agent tools on the host (under +#: `LocalExecutor`, path-confined to the workspace but unsandboxed). From Slack +#: it needs the same double opt-in as `agent`, and every run of it is parked on +#: the human approval gate before anything executes. +AGENT_PLAN_REGISTRY = "grapharc.stdlib:build_registry" + +#: Subcommands that write a trace while they run. The gate gives each of them +#: a trace path it knows (unless the requester named one), so the bot can tail +#: the file for live progress and readers (`metrics`, `viz`, `replay`) can be +#: pointed at it from Slack afterwards. The CLI's own defaults are tempdirs +#: outside the bot's world (`agent` excepted), where nothing can be read back. +LIVE_COMMANDS = frozenset({"demo", "run", "plan", "agent"}) + ALLOWED_COMMANDS: dict[str, CommandSpec] = { "demo": CommandSpec( value_flags={"--trace": True, "--memory": True, "--memory-backend": False}, @@ -91,7 +107,9 @@ class CommandSpec: "--tenant": False, "--max-rounds": False, "--max-tokens": False, + "--approval-timeout": False, }, + bool_flags=frozenset({"--approve"}), model_flags=frozenset({"--model"}), choice_flags={"--registry": PLAN_REGISTRIES}, ), @@ -112,6 +130,9 @@ class CommandSpec: # Claude Code's own sandboxed loop, which the injection below tempers. choice_flags={"--executor": frozenset({"sandbox", "claude-cli"})}, ), + "approve": CommandSpec( + bool_flags=frozenset({"--deny"}), path_positionals=frozenset({0}) + ), "replay": CommandSpec(path_positionals=frozenset({0})), "diff": CommandSpec(path_positionals=frozenset({0})), "trace": CommandSpec(value_flags={"--run-id": False}, path_positionals=frozenset({0})), @@ -161,6 +182,11 @@ def parse_command( timeout_seconds: float | None = None, ) -> list[str]: """Turn Slack text into the argv the bot may run, or raise with the reason.""" + # People copy commands out of code-formatted Slack messages, and the + # backticks come along for the ride: "`approve x/trace.jsonl`" arrives + # with a backtick glued to the first and last token. No admissible + # command starts or ends with one, so wrapping backticks are noise. + text = text.strip().strip("`").strip() try: tokens = shlex.split(text) except ValueError as exc: @@ -253,4 +279,85 @@ def parse_command( if delegated and "--allow" not in argv and "--deny" not in argv: argv.extend(["--deny", "Bash"]) + if name == "plan" and _flag_value(argv, "--registry") == AGENT_PLAN_REGISTRY: + # The stdlib registry materializes agent nodes that run tools on the + # host, so it inherits `agent`'s double opt-in — and, opted in or not, + # a Slack-launched agent plan always parks on the human approval gate. + # The gate is answered with `/grapharc approve ` — by ANY + # workspace member, not only the requester: the handshake is bound to + # the run's directory, and the workspace is the trust boundary here + # exactly as it is for every other command the bot runs. A human saw + # the graph and said yes; *which* human is not recorded (the trace has + # no actor field — see the architecture review). + if not (allow_agent and allow_model): + raise SlackCommandError( + "the stdlib plan registry runs agent tools on the host and is off " + "by default; the operator enables it with both " + "GRAPHARC_SLACK_ALLOW_AGENT=1 and GRAPHARC_SLACK_ALLOW_MODEL=1 " + "in the shell that starts the bot" + ) + if not _has_flag(argv, "--approve"): + argv.append("--approve") + + # ANY parked plan — stdlib-injected or requester-chosen `--approve` on a + # demo registry — must time its wait under the runner's kill: the CLI + # default (300s) exceeds the bot default (120s), and a park that outlives + # the runner is a hard kill mid-wait instead of a clean approval_timeout. + # Half the wall clock for the wait, capped to leave the run 10s to report, + # floored so a tiny ceiling still gives a human a moment. + if ( + name == "plan" + and _has_flag(argv, "--approve") + and not _has_flag(argv, "--approval-timeout") + and timeout_seconds is not None + ): + wait = max(10.0, min(timeout_seconds - 10.0, timeout_seconds / 2)) + argv.extend(["--approval-timeout", str(wait)]) + + # A tracing command whose trace the requester did not place gets one the + # bot can find: unique per invocation (a reused file would make a tailer's + # first line some other run's), relative to the workdir the runner uses as + # cwd. A requester-named `--trace` was already confined above and wins. + if name in LIVE_COMMANDS and not _has_flag(argv, "--trace"): + argv.extend(["--trace", _default_trace()]) + return argv + + +def _has_flag(argv: list[str], flag: str) -> bool: + return any(token == flag or token.startswith(f"{flag}=") for token in argv) + + +def _flag_value(argv: list[str], flag: str) -> str | None: + for index, token in enumerate(argv): + if token == flag and index + 1 < len(argv): + return argv[index + 1] + if token.startswith(f"{flag}="): + return token.partition("=")[2] + return None + + +def _default_trace() -> str: + stamp = datetime.now(UTC).strftime("%Y%m%d-%H%M%S") + return f"slack-runs/{stamp}-{uuid.uuid4().hex[:8]}/trace.jsonl" + + +def trace_path(argv: list[str], workdir: Path) -> Path | None: + """The trace file an admitted argv will write, or None for a reader. + + The gate injects `--trace` into every `LIVE_COMMANDS` argv it admits, so + for those this always resolves; it is how `bot.py` learns where to tail + without re-deriving the gate's decisions. + """ + if not argv or argv[0] not in LIVE_COMMANDS: + return None + raw: str | None = None + for index, token in enumerate(argv): + if token == "--trace" and index + 1 < len(argv): + raw = argv[index + 1] + elif token.startswith("--trace="): + raw = token.partition("=")[2] + if raw is None: + return None + path = Path(raw) + return path if path.is_absolute() else workdir / path diff --git a/grapharc/slack/config.py b/grapharc/slack/config.py index 20fbb3a..a2ce492 100644 --- a/grapharc/slack/config.py +++ b/grapharc/slack/config.py @@ -36,6 +36,18 @@ class SlackBotConfig: # effective together with allow_model — an agent cannot run spend-free. allow_agent: bool = False slash_command: str = "/grapharc" + # Edit one status message with live progress while a tracing command runs. + # Default on: every failure inside the live path degrades to the plain + # blocking reply, so there is nothing to lose by having it on. + live: bool = True + # Floor between two edits of the status message. Slack's chat.update sits + # in a ~50/min rate tier; a couple of seconds keeps one run well inside it. + live_interval_seconds: float = 2.5 + # Base URL of an operator-run `grapharc serve --live-root` reachable by the + # requester (a tunnel or tailnet hostname). Unset means: post no link. The + # bot itself never opens a port — this is the operator asserting that a + # separate process does. + live_url_base: str | None = None @classmethod def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig: @@ -69,6 +81,24 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig: if timeout <= 0: raise SlackConfigError("GRAPHARC_SLACK_TIMEOUT must be positive") + raw_interval = env.get("GRAPHARC_SLACK_LIVE_INTERVAL", "2.5") + try: + live_interval = float(raw_interval) + except ValueError: + raise SlackConfigError( + "GRAPHARC_SLACK_LIVE_INTERVAL must be a number of seconds, " + f"got {raw_interval!r}" + ) from None + if live_interval <= 0: + raise SlackConfigError("GRAPHARC_SLACK_LIVE_INTERVAL must be positive") + + live_url_base = env.get("GRAPHARC_SLACK_LIVE_URL", "").rstrip("/") or None + if live_url_base is not None and not live_url_base.startswith(("http://", "https://")): + raise SlackConfigError( + "GRAPHARC_SLACK_LIVE_URL must start with http:// or https://, " + f"got {live_url_base!r}" + ) + return cls( bot_token=bot_token, app_token=app_token, @@ -77,4 +107,10 @@ def from_env(cls, environ: dict[str, str] | None = None) -> SlackBotConfig: allow_model=env.get("GRAPHARC_SLACK_ALLOW_MODEL", "") == "1", allow_agent=env.get("GRAPHARC_SLACK_ALLOW_AGENT", "") == "1", slash_command=env.get("GRAPHARC_SLACK_COMMAND", "/grapharc"), + # Any common spelling of "off" disables — an operator who wrote + # `false` and silently got live narration anyway was misled. + live=env.get("GRAPHARC_SLACK_LIVE", "1").strip().lower() + not in ("0", "false", "no", "off"), + live_interval_seconds=live_interval, + live_url_base=live_url_base, ) diff --git a/grapharc/slack/format.py b/grapharc/slack/format.py index 37c47ad..37a9bfc 100644 --- a/grapharc/slack/format.py +++ b/grapharc/slack/format.py @@ -7,7 +7,8 @@ rendering of them would be a third dialect the docs never promised. Slack rejects messages past 40,000 characters; the fence is truncated well -below that, from the top, with a line saying how much was cut. Truncation is +below that, keeping the tail (where a traceback's actual error lives), with a +line saying how much was cut. Truncation is announced, never silent — the reader must know they are not seeing everything. """ @@ -17,7 +18,10 @@ import json import shlex import zlib +from pathlib import Path +from urllib.parse import quote +from grapharc.slack.command import trace_path from grapharc.slack.runner import CommandResult # Leaves generous room for the header and the truncation notice. @@ -30,16 +34,22 @@ } -def _fence(body: str) -> str: +def fence(body: str) -> str: # A ``` inside the body would end the fence early and spill the rest as # prose; a zero-width space between the backticks defuses it. return "```" + body.replace("```", "`​``") + "```" -def _truncate(body: str) -> tuple[str, int]: +def truncate(body: str) -> tuple[str, int]: + """Clip to the fence budget, keeping the TAIL. + + The end is where the information lives: a traceback's actual error is its + last line, and a live feed's most recent events are at the bottom. Keeping + the head cut exactly the part the reader needed. + """ if len(body) <= MAX_FENCE_CHARS: return body, 0 - return body[:MAX_FENCE_CHARS], len(body) - MAX_FENCE_CHARS + return body[-MAX_FENCE_CHARS:], len(body) - MAX_FENCE_CHARS def mermaid_live_url(code: str) -> str: @@ -55,6 +65,36 @@ def mermaid_live_url(code: str) -> str: return f"https://mermaid.live/view#pako:{packed}" +def live_view_url(argv: list[str], *, base: str | None, workdir: Path) -> str | None: + """The live-view page for an admitted argv, or None. + + None when the operator configured no base URL, or the argv is not a + tracing command. The gate injects `--trace` into every tracing argv it + admits, so for those the path is always present; it is keyed relative to + the workdir because that is the root the operator serves + (`grapharc serve --live-root "$GRAPHARC_SLACK_WORKDIR"`). + + The bot never verifies the server is up or reachable — the base URL is + the operator's assertion, and the message wording says so. + """ + if not base: + return None + path = trace_path(argv, workdir) + if path is None: + return None + try: + rel = path.relative_to(workdir) + except ValueError: + return None + url = f"{base}/live/view?trace={quote(rel.as_posix(), safe='')}" + for index, token in enumerate(argv): + if token == "--run-id" and index + 1 < len(argv): + url += f"&run={quote(argv[index + 1])}" + elif token.startswith("--run-id="): + url += f"&run={quote(token.partition('=')[2])}" + return url + + def format_result(result: CommandResult) -> str: shown = shlex.join(["grapharc", *result.argv]) if result.exit_code is None: @@ -72,12 +112,12 @@ def format_result(result: CommandResult) -> str: for label, stream in (("stdout", result.stdout), ("stderr", result.stderr)): if not stream.strip(): continue - body, cut = _truncate(stream) + body, cut = truncate(stream) if len(parts) > 1 or label == "stderr": parts.append(f"{label}:") - parts.append(_fence(body)) + parts.append(fence(body)) if cut: - parts.append(f"_…{cut} more characters not shown._") + parts.append(f"_…{cut} earlier characters not shown._") if len(parts) == 1: parts.append("_(no output)_") # `viz` prints raw Mermaid, which Slack shows as text. One extra line makes diff --git a/grapharc/slack/live.py b/grapharc/slack/live.py new file mode 100644 index 0000000..047913e --- /dev/null +++ b/grapharc/slack/live.py @@ -0,0 +1,381 @@ +"""Tail a trace file while a command runs and narrate it through a callback. + +The CLI subprocess appends one JSON line per event to its trace (`TraceRecorder` +writes under a lock, append-only), so the file is complete and readable at any +instant. `LiveTail` polls it from a daemon thread, reconstructs the run with +`replay` — the same tested reconstruction `metrics` and `viz` use, never a +private dialect — and hands a rendered progress message to an `update` +callback whenever something changed. + +Everything here is best-effort by contract: the whole loop body runs inside a +`try/except` and an `update` that fails marks the sink dead. A live view that +breaks must degrade to today's behaviour (the final result posted once at the +end), never take the run down with it. This module imports nothing from Slack — +the callback is a plain callable, which is also what makes it testable without +a token. +""" + +from __future__ import annotations + +import json +import shlex +import threading +import time +from collections.abc import Callable +from dataclasses import dataclass +from pathlib import Path +from typing import Protocol + +from grapharc.observe.metrics import to_mermaid +from grapharc.observe.replay import NodeExecution, ReplayedRun, replay +from grapharc.observe.trace import TailRecorder, TraceEvent +from grapharc.slack.format import fence, mermaid_live_url, truncate + +#: How many trailing sub-step events the flat feed shows for a run with no +#: node executions (`grapharc agent` drives an `AgentNode` with no enclosing +#: graph, so everything it does is an orphan sub-event). +FLAT_FEED_LINES = 15 + + +class LiveSink(Protocol): + """Where live progress goes. `bot.py` backs this with Slack; tests with a list. + + Both methods signal failure with a value, never an exception: `post` + returns None (the caller falls back to the plain blocking reply), `update` + returns False (the tailer goes quiet). A sink can be handed an unreliable + network and the run must not care. + """ + + def post(self, text: str) -> object | None: ... + + def update(self, handle: object, text: str) -> bool: ... + + +@dataclass(frozen=True) +class LiveSettings: + """Cadence of the tail loop.""" + + # Floor between two `update` calls; chat.update sits in a rate tier. + update_interval: float = 2.5 + # How often the file's size is polled. Cheap (one stat), so more often + # than updates: growth is noticed promptly, rendering waits its turn. + poll_interval: float = 1.0 + # How long __exit__ waits for the thread; a stuck render must not delay + # the final result by more than this. + join_timeout: float = 5.0 + + +class LiveTail: + """Context manager: tail `trace_path` on a daemon thread while the body runs. + + Construct it *before* the subprocess starts: the constructor records the + file's current size, and only bytes appended after that are read — a + reused trace file (the agent workspace is constant, so its trace + accumulates runs) must not replay some earlier run as this one's progress. + """ + + def __init__( + self, + trace_path: Path, + argv: list[str], + update: Callable[[str], bool], + settings: LiveSettings | None = None, + ) -> None: + self._path = trace_path + self._argv = list(argv) + self._update = update + self._settings = settings or LiveSettings() + try: + self._offset = trace_path.stat().st_size + except OSError: + self._offset = 0 + self._stop = threading.Event() + self._thread = threading.Thread(target=self._loop, daemon=True) + self._started_at = time.monotonic() + + def __enter__(self) -> LiveTail: + self._thread.start() + return self + + def __exit__(self, *exc_info: object) -> None: + # Stop and join *before* the caller posts the final result, so a late + # tick can never overwrite it. + self._stop.set() + self._thread.join(timeout=self._settings.join_timeout) + + def _loop(self) -> None: + run_id: str | None = None + last_update = 0.0 + last_text = "" + dirty = False + while not self._stop.wait(self._settings.poll_interval): + try: + new_id = self._read_new_run_id() + if new_id is not None: + run_id = new_id + dirty = True + if run_id is None or not dirty: + continue + if time.monotonic() - last_update < self._settings.update_interval: + continue + text = self._render(run_id) + if text is None: + continue + dirty = False + last_update = time.monotonic() + if text == last_text: + continue + if not self._update(text): + return # the sink is dead; go quiet, never raise + last_text = text + except Exception: + # Live narration is best-effort; the run and the final result + # do not depend on it. + return + + def _render(self, run_id: str) -> str | None: + # TailRecorder, not TraceRecorder: a read can land mid-write, and the + # plain reader raises on the torn line where this one skips it. + recorder = TailRecorder(self._path) + try: + run = replay(recorder, run_id) + diagram = to_mermaid(recorder, run_id) + except Exception: + return None # e.g. no complete events yet; retry next tick + return render_progress( + run, + argv=self._argv, + elapsed_s=time.monotonic() - self._started_at, + diagram=diagram, + ) + + def _read_new_run_id(self) -> str | None: + """Fold newly appended complete lines; return the latest run_id seen. + + Returns None when nothing (complete) was appended — so a non-None + return doubles as "the file grew". + """ + try: + size = self._path.stat().st_size + except OSError: + return None + if size < self._offset: + # Truncated or replaced underneath us — the offset describes a + # file that no longer exists. Start over from byte zero (the same + # bet `TraceRecorder._advance_index` makes); staying put meant + # permanent silence until the new file outgrew the old offset, + # then a seek into the middle of a line. + self._offset = 0 + if size == self._offset: + return None + with self._path.open("rb") as f: + f.seek(self._offset) + chunk = f.read(size - self._offset) + # Stop at the last newline: the writer may be mid-line, and half a + # JSON object is not an event yet. + cut = chunk.rfind(b"\n") + if cut < 0: + return None + self._offset += cut + 1 + run_id = None + for raw in chunk[: cut + 1].splitlines(): + if not raw.strip(): + continue + try: + record = json.loads(raw) + except ValueError: + continue + candidate = record.get("run_id") + if isinstance(candidate, str) and candidate: + run_id = candidate + return run_id + + +def _mark(execution: NodeExecution) -> str: + if execution.error is not None: + return "✗" + return "✓" if execution.completed else "▸" + + +def _duration(ms: float | None) -> str: + if ms is None: + return "" + return f"{ms:.0f}ms" if ms < 1000 else f"{ms / 1000:.1f}s" + + +def _execution_line(execution: NodeExecution) -> str: + parts = [_mark(execution), f" {execution.node}"] + if execution.error is not None: + detail = " ".join(execution.error.split())[:80] + parts.append(f" err: {detail}") + elif not execution.completed: + parts.append(" running…") + else: + if execution.duration_ms is not None: + parts.append(f" {_duration(execution.duration_ms)}") + if execution.tokens: + parts.append(f" {execution.tokens} tok") + if execution.sub_events: + parts.append(f" · {len(execution.sub_events)} sub-steps") + return "".join(parts) + + +def _sub_event_line(event: TraceEvent) -> str: + parts = [f"{event.phase} {event.node}"] + if event.tokens: + parts.append(f" {event.tokens} tok") + if event.error: + parts.append(f" err: {' '.join(event.error.split())[:80]}") + return "".join(parts) + + +#: Phases that describe the run rather than doing work; never shown as feed. +_SHAPE_PHASES = frozenset({"topology", "approval_request", "approval_response"}) + + +def _pending_approval(run: ReplayedRun) -> TraceEvent | None: + """The latest approval request no response has answered yet, if any.""" + pending: TraceEvent | None = None + for event in run.events: + if event.phase == "approval_request": + pending = event + elif event.phase == "approval_response": + pending = None + return pending + + +def _planned_lines(run: ReplayedRun) -> list[str] | None: + """One line per *declared* node, marked by status — or None without topology. + + The declared graph is what makes a live view honest about scope: a node + that has not started yet is shown as pending rather than not shown at all. + Multi-round planner runs list each round's graph in order, and status is + read from that round's *own* events — keyed by bare name, round 1's + finished node once wore round 2's still-running state. + + Deltas are merged per graph (the loop's labelled statement plus the + kernel's bare restatement), so the round label survives execution. + """ + topologies: dict[str, dict] = {} + for event in run.events: + if event.phase == "topology" and event.state_delta: + topologies.setdefault(event.graph, {}).update(event.state_delta) + if not topologies: + return None + lines: list[str] = [] + for graph, delta in topologies.items(): + if len(topologies) > 1: + round_no = delta.get("round") + lines.append(f"round {round_no}:" if round_no else f"{graph}:") + graph_events = [e for e in run.events if e.graph == graph] + for name in delta.get("nodes", []): + lines.append(_node_status_line(str(name), graph_events)) + return lines + + +def _node_status_line(name: str, events: list[TraceEvent]) -> str: + """One mark per declared node, from its own graph's events only.""" + starts = ends = 0 + last_end: TraceEvent | None = None + last_error: TraceEvent | None = None + for event in events: + if event.node != name: + continue + if event.phase == "start": + starts += 1 + elif event.phase == "end": + ends += 1 + last_end = event + elif event.phase == "error": + last_error = event + if last_error is not None: + detail = " ".join((last_error.error or "error").split())[:80] + return f"✗ {name} err: {detail}" + if starts > ends: + return f"▸ {name} running…" + if last_end is not None: + parts = [f"✓ {name}"] + if last_end.duration_ms is not None: + parts.append(f" {_duration(last_end.duration_ms)}") + if last_end.tokens: + parts.append(f" {last_end.tokens} tok") + return "".join(parts) + return f"⬜ {name} pending" + + +def render_progress( + run: ReplayedRun, + *, + argv: list[str], + elapsed_s: float, + diagram: str | None = None, +) -> str: + """One Slack message describing the run so far. + + Declared-graph node marks when the trace carries topology; node executions + when the run has them; otherwise the tail of the flat event feed — an + agent with no enclosing graph is all flat feed, and an empty box saying + nothing was the alternative. + """ + approval = _pending_approval(run) + if approval is not None: + header = ( + f"`{shlex.join(['grapharc', *argv])}` — " + f"⏸ waiting for approval ({elapsed_s:.0f}s)" + ) + else: + header = f"`{shlex.join(['grapharc', *argv])}` — running ({elapsed_s:.0f}s)" + + planned = _planned_lines(run) + if planned is not None: + lines = planned + elif run.executions: + lines = [_execution_line(e) for e in run.executions] + else: + feed = [e for e in run.orphan_sub_events if e.phase not in _SHAPE_PHASES] + hidden = len(feed) - len(feed[-FLAT_FEED_LINES:]) + feed = feed[-FLAT_FEED_LINES:] + lines = [_sub_event_line(e) for e in feed] + if hidden > 0: + lines.insert(0, f"… {hidden} earlier events") + + body, cut = truncate("\n".join(lines)) + parts = [header, fence(body)] + if cut: + parts.append(f"_…{cut} earlier characters not shown._") + + done = sum(1 for e in run.executions if e.completed) + # `run.events` already holds every event, orphans included. + footer = [f"{len(run.events)} events"] + if run.executions: + footer.append(f"{done}/{len(run.executions)} nodes done") + if run.tokens: + footer.append(f"{run.tokens} tok") + # `recorded_cost_usd`, not a raw event sum: an agent's model sub-events + # and its node's end both carry the same spend, and summing every event + # reported double the real bill. + cost = run.recorded_cost_usd + if cost: + footer.append(f"${cost:.4f}") + parts.append(" · ".join(footer)) + + if approval is not None: + trace_arg = _trace_argument(argv) + if trace_arg: + parts.append( + f"planned graph is in the diagram link — approve with " + f"`/grapharc approve {trace_arg}`, refuse with " + f"`/grapharc approve {trace_arg} --deny`" + ) + if diagram: + parts.append(f"<{mermaid_live_url(diagram)}|current diagram>") + return "\n".join(parts) + + +def _trace_argument(argv: list[str]) -> str | None: + for index, token in enumerate(argv): + if token == "--trace" and index + 1 < len(argv): + return argv[index + 1] + if token.startswith("--trace="): + return token.partition("=")[2] + return None diff --git a/grapharc/stdlib.py b/grapharc/stdlib.py index 52e9056..414bf7e 100644 --- a/grapharc/stdlib.py +++ b/grapharc/stdlib.py @@ -39,7 +39,8 @@ from __future__ import annotations -from typing import Any +import operator +from typing import Annotated, Any from pydantic import BaseModel @@ -68,17 +69,20 @@ class WorkState(BaseModel): """The state contract for stdlib graphs. - Deliberately small and append-only. Every field is a list a phase adds to, - so two phases running in parallel cannot silently clobber each other — the - kernel would refuse an undeclared write anyway, but a schema that makes the - conflict impossible is better than one that reports it. + Deliberately small and append-only. Every list field carries an `operator.add` + reducer, which is what actually makes concurrency safe: without one, + LangGraph refuses two writes to the same key in one superstep + (`InvalidUpdateError`), so a fan-out of parallel phases — the shape a + planner proposes for any decomposable job — died at execution instead of + merging. Phases therefore return only what they *add*; the reducer appends + it, and two phases finishing together cannot clobber each other. """ goal: str = "" #: What was learned. Read by later phases as their context. - findings: list[str] = [] + findings: Annotated[list[str], operator.add] = [] #: Narrative for a human reading the result. - notes: list[str] = [] + notes: Annotated[list[str], operator.add] = [] #: The single list field each agent-backed phase appends its report to. @@ -148,7 +152,9 @@ def body(state: WorkState) -> dict: if not p.name.startswith(".") )[:50] found = f"workspace contains {len(names)} visible entries: {', '.join(names)}" - return {"findings": [*state.findings, found]} + # Only the new item: `findings` carries an `operator.add` reducer, so + # returning the accumulated list would append it to itself. + return {"findings": [found]} body.writes = {"findings"} return body @@ -211,7 +217,8 @@ def body(state: WorkState, ctx: Any) -> dict: # is not indistinguishable from one that finished. reason = result.termination_reason.value line = result.output if reason == "target_met" else f"[{reason}] {result.output}" - return {field: [*getattr(state, field), line]} + # The reducer appends; returning the accumulated list would double it. + return {field: [line]} body.writes = {field} return body @@ -315,6 +322,122 @@ def catalog_for_prompt(model: Any = None) -> dict[str, str]: return build_registry(model).catalog() +def goal_met(state: Any) -> bool: + """Done when a report landed in `notes`. + + `apply_change` and `summarize` are the kinds that write `notes`, so this + reads as "the run produced its human-facing outcome". Deterministic code, + never a model — and defensive about the schema, so a custom state supplied + through `--registry` cannot turn "am I done" into an AttributeError. + """ + return len(getattr(state, "notes", ()) or ()) >= 1 + + +def _observe(state: Any) -> str: + """What the planner is shown between rounds: the goal's progress so far.""" + parts = [] + findings = getattr(state, "findings", None) or [] + notes = getattr(state, "notes", None) or [] + parts.append(f"findings so far: {len(findings)}") + for line in findings[-5:]: + parts.append(f"- {line}") + parts.append(f"notes so far: {len(notes)}") + for line in notes[-3:]: + parts.append(f"- {line}") + return "\n".join(parts) + + +def scripted_planner_replies() -> list[str]: + """One model-free round: the two deterministic kinds, chained. + + With no model only `collect_context` and `checkpoint` are registered, so + this is the only proposal the scripted path can make that admission will + take — and it is what makes `grapharc plan --registry grapharc.stdlib:...` + runnable spend-free as a smoke test. + """ + import json + + from grapharc.runtime.graph import END, START + + return [ + json.dumps( + { + "nodes": [{"name": "collect_context"}, {"name": "checkpoint"}], + "edges": [ + {"source": START, "target": "collect_context"}, + {"source": "collect_context", "target": "checkpoint"}, + {"source": "checkpoint", "target": END}, + ], + "rationale": "list the workspace, then join", + } + ), + # Then nothing more: the deterministic kinds cannot write `notes`, so + # the honest second answer is "no further work" — the loop stops + # cleanly instead of burning rounds on an exhausted script. + json.dumps({"nodes": [], "edges": [], "rationale": "no further work"}), + ] + + +def build_loop( + model: Any, + *, + edge_policy: Any = None, + trace: Any = None, + budget: Any = None, + limits: Any = None, + registry: Any = None, + state_schema: Any = None, + writes: dict[str, set[str]] | None = None, + approval: Any = None, +) -> Any: + """Assemble the stdlib loop: same shape as the incident demo's, its own goal. + + Read by `grapharc plan --registry grapharc.stdlib:build_registry` through + `RegistryBundle.build_loop` — which is what lets this registry own its goal + check and its observer instead of inheriting the demo's `len(notes) >= 3`. + """ + from grapharc.planner import ( + AdmissionChecker, + AdmissionLimits, + GovernedLoop, + Materializer, + PlannerNode, + ) + + registry = registry or build_registry(model) + registry.freeze() + return GovernedLoop( + planner=PlannerNode( + model, name="stdlib", catalog=registry.catalog(), trace=trace + ), + checker=AdmissionChecker( + registry=registry, + edge_policy=edge_policy or default_edge_policy(), + trace=trace, + # Rounds are materialized standalone, so "can this actually run" + # is part of admission here: a plan with no entry, or with nodes + # nothing reaches, comes back as a rejection carrying a remedy + # instead of a build failure the planner cannot act on. + limits=AdmissionLimits(require_entry=True), + ), + materializer=Materializer( + registry=registry, + state_schema=state_schema or WorkState, + writes=writes if writes is not None else WRITES, + trace=trace, + ), + budget=budget, + limits=limits, + trace=trace, + name="stdlib_loop", + goal_reached=goal_met, + # The planner should see what earlier phases learned, or every round + # re-plans blind against the same goal text. + observe=_observe, + approval=approval, + ) + + __all__ = [ "AGENT_KINDS", "DETERMINISTIC_KINDS", @@ -326,8 +449,11 @@ def catalog_for_prompt(model: Any = None) -> dict[str, str]: "WRITES", "WRITE_TOOLS", "WorkState", + "build_loop", "build_registry", "catalog_for_prompt", "default_edge_policy", "default_harness", + "goal_met", + "scripted_planner_replies", ] diff --git a/tests/test_admission.py b/tests/test_admission.py index b2aefe8..7826f0a 100644 --- a/tests/test_admission.py +++ b/tests/test_admission.py @@ -78,7 +78,12 @@ def checker(reg: NodeRegistry, **kwargs) -> AdmissionChecker: def test_a_valid_proposal_is_admitted_and_every_check_ran(): - result = checker(registry("fetch", "summarise")).check(linear("fetch", "summarise")) + # `require_entry` is opt-in (a proposal may attach to a live graph, where + # the entry is outside it), so a checker that runs *every* gate has to ask. + result = checker( + registry("fetch", "summarise"), + limits=AdmissionLimits(require_entry=True), + ).check(linear("fetch", "summarise")) assert result.admitted assert result.status is AdmissionStatus.ADMITTED @@ -653,7 +658,12 @@ def test_a_diamond_is_not_a_cycle(): def all_checks_failing() -> Subgraph: - """One proposal that trips registry, policy, budget, depth and acyclicity.""" + """One proposal that trips every gate at once. + + Registry (an unregistered kind), policy (a deny-all policy), budget (a + costly kind against a tiny remainder), depth (a nested subgraph), + acyclicity (a -> b -> a), and reachability (nothing leaves START). + """ inner = Subgraph(nodes=(ProposedNode(name="inner", kind="step"),)) return Subgraph( nodes=( @@ -669,9 +679,9 @@ def all_checks_failing() -> Subgraph: def test_every_failed_check_is_reported_not_just_the_first(): reg = registry("step", step=CostEstimate(tokens=10_000)) - result = checker(reg, edge_policy=EdgePolicy()).check( - all_checks_failing(), remaining=RemainingBudget(tokens=5) - ) + result = checker( + reg, edge_policy=EdgePolicy(), limits=AdmissionLimits(require_entry=True) + ).check(all_checks_failing(), remaining=RemainingBudget(tokens=5)) assert not result.admitted assert set(result.failed_checks()) == set(Check) @@ -679,9 +689,9 @@ def test_every_failed_check_is_reported_not_just_the_first(): def test_every_rejection_names_a_check_a_code_and_a_subject(): reg = registry("step", step=CostEstimate(tokens=10_000)) - result = checker(reg, edge_policy=EdgePolicy()).check( - all_checks_failing(), remaining=RemainingBudget(tokens=5) - ) + result = checker( + reg, edge_policy=EdgePolicy(), limits=AdmissionLimits(require_entry=True) + ).check(all_checks_failing(), remaining=RemainingBudget(tokens=5)) for reason in result.rejections: assert isinstance(reason.check, Check) @@ -692,9 +702,9 @@ def test_every_rejection_names_a_check_a_code_and_a_subject(): def test_feedback_is_a_planner_readable_list_of_every_failure(): reg = registry("step", step=CostEstimate(tokens=10_000)) - result = checker(reg, edge_policy=EdgePolicy()).check( - all_checks_failing(), remaining=RemainingBudget(tokens=5) - ) + result = checker( + reg, edge_policy=EdgePolicy(), limits=AdmissionLimits(require_entry=True) + ).check(all_checks_failing(), remaining=RemainingBudget(tokens=5)) text = result.feedback() assert result.proposal_id in text @@ -1081,3 +1091,80 @@ def admit(state: PlanState, ctx) -> dict: assert admission_events and "policy/edge_denied" in (admission_events[0].error or "") executed = {e.node for e in trace.read_events() if e.phase == "end"} assert executed == {"planner", "admission"} # `helper` never became a node + + +# -- reachability: structural runnability, opt-in ------------------------------ + + +def test_a_proposal_with_no_entry_edge_is_rejected_when_entry_is_required(): + """The materializer always refused this; admission used to say yes first, + so the loop learned "could not be built" — a build failure it cannot + replan against — instead of a rejection carrying a remedy.""" + proposal = Subgraph( + nodes=(ProposedNode(name="a", kind="step"), ProposedNode(name="b", kind="step")), + edges=(ProposedEdge(source="a", target="b"),), + ) + result = checker( + registry("step"), limits=AdmissionLimits(require_entry=True) + ).check(proposal) + + assert not result.admitted + (reason,) = result.reasons(Check.REACHABILITY) + assert reason.code == "no_entry_edge" + assert START in reason.remedy + + +def test_a_node_nothing_reaches_is_rejected_by_name(): + proposal = Subgraph( + nodes=( + ProposedNode(name="a", kind="step"), + ProposedNode(name="orphan", kind="step"), + ), + edges=( + ProposedEdge(source=START, target="a"), + ProposedEdge(source="a", target=END), + ), + ) + result = checker( + registry("step"), limits=AdmissionLimits(require_entry=True) + ).check(proposal) + + assert not result.admitted + (reason,) = result.reasons(Check.REACHABILITY) + assert reason.code == "unreachable_node" + assert reason.subject == "orphan" + + +def test_a_fan_out_and_join_is_reachable(): + """Parallel branches converging is the shape planners propose most; it + must not read as unreachable.""" + proposal = Subgraph( + nodes=tuple( + ProposedNode(name=n, kind="step") for n in ("seed", "l", "r", "join") + ), + edges=( + ProposedEdge(source=START, target="seed"), + ProposedEdge(source="seed", target="l"), + ProposedEdge(source="seed", target="r"), + ProposedEdge(source="l", target="join"), + ProposedEdge(source="r", target="join"), + ProposedEdge(source="join", target=END), + ), + ) + assert ( + checker(registry("step"), limits=AdmissionLimits(require_entry=True)) + .check(proposal) + .admitted + ) + + +def test_entry_is_not_required_by_default_so_a_live_graph_can_be_extended(): + """Admission stays the broader gate: a proposal may name nodes of a graph + already running, where the entry lives outside the proposal entirely.""" + proposal = Subgraph( + nodes=(ProposedNode(name="added", kind="step"),), + edges=(ProposedEdge(source="live", target="added"),), + ) + result = checker(registry("step"), known_nodes={"live": "step"}).check(proposal) + assert result.admitted + assert Check.REACHABILITY not in result.checks_run diff --git a/tests/test_agent_node.py b/tests/test_agent_node.py index d0b58f7..f103b89 100644 --- a/tests/test_agent_node.py +++ b/tests/test_agent_node.py @@ -403,6 +403,29 @@ def explode(x: int) -> int: assert "nope" in result.tool_calls[0].detail +def test_invented_argument_names_get_the_real_schema_in_the_error(): + """A model that calls `read(filename=...)` when the tool takes `path` is + told the actual parameter list — the bare TypeError names the wrong + argument but not the right ones, which a weak model cannot recover from.""" + + def read(path: str, limit: int = 0) -> str: + return path + + harness = _harness( + [ToolSpec(name="read", description="", fn=read)], + [{"action": "allow", "pattern": "read"}], + ) + model = ToolScriptedChatModel( + responses=["", "ok"], tool_call_script=[[_call("read", {"filename": "a.md"})]] + ) + result = AgentNode(model, harness).run("go", _ctx()) + + detail = result.tool_calls[0].detail + assert result.tool_calls[0].status is ToolCallStatus.ERROR + assert "unexpected keyword argument" in detail + assert "takes exactly: path, limit=…" in detail + + @requires_sandbox def test_sandbox_violation_surfaces_as_a_tool_error(tmp_path): """The executor's boundary reaches the model as a readable result — the @@ -658,7 +681,12 @@ def _generate(self, messages, stop=None, run_manager=None, **kwargs): assert result.termination_reason is StopReason.TARGET_MET record = result.tool_calls[0] assert record.status is ToolCallStatus.ERROR - assert record.detail == "TOOL_ERROR: tool arguments must be a JSON object, got list" + assert record.detail.startswith( + "TOOL_ERROR: tool arguments must be a JSON object, got list" + ) + # The wrong-shape error also names the real parameters — a model that sent + # a list needs the schema as much as one that misnamed a keyword. + assert "takes exactly: text" in record.detail assert record.args == {} assert _tool_messages(model.calls[1])[0].content == record.detail @@ -985,7 +1013,9 @@ def read_file(path: str) -> str: # Sub-node steps come from the same run counter, so each loop step is its # own replay point rather than collapsing into the node's single step. node_step = next(e.step for e in events if e.node == "reader") - inner = [e.step for e in events if e.node != "reader"] + inner = [ + e.step for e in events if e.node != "reader" and e.phase != "topology" + ] assert inner == sorted(inner) assert len(set(inner)) == len(inner) assert min(inner) > node_step @@ -1071,3 +1101,24 @@ def test_max_iterations_below_one_is_rejected(): harness = _harness([], []) with pytest.raises(AgentConfigError, match="max_iterations"): AgentNode(ToolScriptedChatModel(responses=[]), harness, max_iterations=0) + + +def test_a_type_error_inside_a_correct_call_gets_no_signature_hint(): + """`len(None)` inside the tool body is the tool's bug, not the model's — + a hint asserting the (valid) arguments were wrong steers the model into + rewriting a call that was fine.""" + + def broken_inside(path: str) -> int: + return len(None) # type: ignore[arg-type] + + harness = _harness( + [ToolSpec(name="broken", description="", fn=broken_inside)], + [{"action": "allow", "pattern": "broken"}], + ) + model = ToolScriptedChatModel( + responses=["", "ok"], tool_call_script=[[_call("broken", {"path": "a"})]] + ) + result = AgentNode(model, harness).run("go", _ctx()) + detail = result.tool_calls[0].detail + assert result.tool_calls[0].status is ToolCallStatus.ERROR + assert "takes exactly" not in detail, detail diff --git a/tests/test_approval.py b/tests/test_approval.py new file mode 100644 index 0000000..506a2fb --- /dev/null +++ b/tests/test_approval.py @@ -0,0 +1,301 @@ +"""The approval gate: the loop's pause, the file handshake, the CLI answer. + +The chain under test: `GovernedLoop(approval=...)` parks each admitted round; +`file_approval` implements the callback as a request/decision file pair next +to the trace; `grapharc approve` writes the decision. Every hop is exercised +without Slack, a server, or a model backend — the planner is scripted. +""" + +from __future__ import annotations + +import json +import threading +import time +from pathlib import Path + +from grapharc.cli.main import main +from grapharc.examples.plan_incident import ( + IncidentState, + build_loop, + scripted_planner_replies, +) +from grapharc.observe.trace import TraceRecorder +from grapharc.planner import LoopStop +from grapharc.planner.approval_file import ( + DECISION_FILENAME, + REQUEST_FILENAME, + file_approval, + read_request, + write_decision, +) +from grapharc.testing import ScriptedChatModel + + +def _loop(tmp_path, approval): + trace = TraceRecorder(tmp_path / "trace.jsonl") + model = ScriptedChatModel(responses=scripted_planner_replies()) + return build_loop(model, trace=trace, approval=approval), trace + + +# -- the loop's gate -------------------------------------------------------- + + +def test_an_approved_round_executes(tmp_path): + calls = [] + + def gate(proposal, verdict): + calls.append(proposal.fingerprint()) + return "approved" + + loop, trace = _loop(tmp_path, gate) + result = loop.run("goal", IncidentState(goal="g")) + assert result.stop is LoopStop.GOAL_MET + assert len(calls) == 1 # round 1 was rejected pre-approval; only round 2 asked + phases = [e.phase for e in trace.read_events(result.run_id)] + assert "approval_request" in phases and "approval_response" in phases + assert "start" in phases # nodes actually ran + + +def test_a_denied_round_stops_the_run_before_any_node_starts(tmp_path): + loop, trace = _loop(tmp_path, lambda p, v: "denied") + result = loop.run("goal", IncidentState(goal="g")) + assert result.stop is LoopStop.APPROVAL_DENIED + events = trace.read_events(result.run_id) + assert not any(e.phase == "start" for e in events), "a node ran without approval" + responses = [e for e in events if e.phase == "approval_response"] + assert responses[-1].state_delta["decision"] == "denied" + + +def test_an_unanswered_gate_is_a_timeout_stop(tmp_path): + loop, _ = _loop(tmp_path, lambda p, v: "timeout") + result = loop.run("goal", IncidentState(goal="g")) + assert result.stop is LoopStop.APPROVAL_TIMEOUT + + +def test_a_gate_that_raises_fails_closed_as_denied(tmp_path): + def broken(proposal, verdict): + raise RuntimeError("gate exploded") + + loop, trace = _loop(tmp_path, broken) + result = loop.run("goal", IncidentState(goal="g")) + assert result.stop is LoopStop.APPROVAL_DENIED + assert not any(e.phase == "start" for e in trace.read_events(result.run_id)) + + +def test_the_request_event_carries_the_plan_being_asked_about(tmp_path): + seen = {} + + def gate(proposal, verdict): + seen["fingerprint"] = proposal.fingerprint() + return "denied" + + loop, trace = _loop(tmp_path, gate) + result = loop.run("goal", IncidentState(goal="g")) + request = next( + e for e in trace.read_events(result.run_id) if e.phase == "approval_request" + ) + assert request.state_delta["fingerprint"] == seen["fingerprint"] + assert request.state_delta["nodes"], "the audited question must name the nodes" + + +# -- the file handshake ----------------------------------------------------- + + +class _Plan: + """Just enough proposal shape for the handshake.""" + + proposal_id = "p-123" + rationale = "because" + nodes = () + edges = () + + @staticmethod + def fingerprint() -> str: + return "fp-abc" + + +def test_file_approval_honours_a_matching_decision(tmp_path): + gate = file_approval(tmp_path, timeout_seconds=5.0, poll_seconds=0.02) + + def answer(): + deadline = time.monotonic() + 3 + while time.monotonic() < deadline: + if (tmp_path / REQUEST_FILENAME).exists(): + write_decision(tmp_path, fingerprint="fp-abc", decision="approved") + return + time.sleep(0.02) + + thread = threading.Thread(target=answer) + thread.start() + decision = gate(_Plan(), None) + thread.join() + assert decision == "approved" + assert not (tmp_path / REQUEST_FILENAME).exists(), "the handshake must be consumed" + assert not (tmp_path / DECISION_FILENAME).exists() + + +def test_a_stale_decision_naming_another_plan_is_discarded(tmp_path): + # The decision predates the question and names a different fingerprint — + # exactly the file a previous round could have left behind. + write_decision(tmp_path, fingerprint="fp-OLD", decision="approved") + gate = file_approval(tmp_path, timeout_seconds=0.3, poll_seconds=0.02) + assert gate(_Plan(), None) == "timeout" + + +def test_a_pre_existing_decision_cannot_pre_approve(tmp_path): + # Even with the RIGHT fingerprint, a decision written before the request + # is deleted when the gate opens: the answer must follow the question. + write_decision(tmp_path, fingerprint="fp-abc", decision="approved") + gate = file_approval(tmp_path, timeout_seconds=0.3, poll_seconds=0.02) + assert gate(_Plan(), None) == "timeout" + + +def test_silence_is_a_timeout(tmp_path): + gate = file_approval(tmp_path, timeout_seconds=0.2, poll_seconds=0.02) + assert gate(_Plan(), None) == "timeout" + + +# -- the CLI answer --------------------------------------------------------- + + +def _pending(tmp_path) -> Path: + (tmp_path / "trace.jsonl").touch() + (tmp_path / REQUEST_FILENAME).write_text( + json.dumps( + { + "proposal_id": "p-9", + "fingerprint": "fp-9", + "nodes": ["triage", "verify"], + "edges": [], + } + ) + ) + return tmp_path / "trace.jsonl" + + +def test_approve_writes_the_decision_quoting_the_request(tmp_path, capsys): + trace_path = _pending(tmp_path) + assert main(["approve", str(trace_path)]) == 0 + decision = json.loads((tmp_path / DECISION_FILENAME).read_text()) + assert decision == {"fingerprint": "fp-9", "decision": "approved"} + out = capsys.readouterr().out + assert "approved" in out and "triage" in out + + +def test_approve_deny_writes_a_denial(tmp_path, capsys): + _pending(tmp_path) + assert main(["approve", str(tmp_path), "--deny"]) == 0 + decision = json.loads((tmp_path / DECISION_FILENAME).read_text()) + assert decision["decision"] == "denied" + + +def test_approve_with_nothing_pending_is_a_negative_answer(tmp_path, capsys): + (tmp_path / "trace.jsonl").touch() + assert main(["approve", str(tmp_path / "trace.jsonl")]) == 1 + # Failure speaks on stderr: the text-mode contract. + assert "nothing is waiting" in capsys.readouterr().err + + +def test_approve_on_a_missing_directory_cannot_run(tmp_path, capsys): + assert main(["approve", str(tmp_path / "absent" / "trace.jsonl")]) == 2 + + +def test_read_request_tolerates_junk(tmp_path): + (tmp_path / REQUEST_FILENAME).write_text("not json {") + assert read_request(tmp_path) is None + + +# -- the whole handshake, in-process --------------------------------------- + + +def test_plan_approve_and_answer_end_to_end(tmp_path): + """The paused loop and the CLI answer, meeting in one directory.""" + trace_dir = tmp_path / "run" + trace_dir.mkdir() + gate = file_approval(trace_dir, timeout_seconds=10.0, poll_seconds=0.02) + loop, trace = _loop(tmp_path, gate) + + def answer_when_asked(): + deadline = time.monotonic() + 8 + while time.monotonic() < deadline: + if (trace_dir / REQUEST_FILENAME).exists(): + assert main(["approve", str(trace_dir)]) == 0 + return + time.sleep(0.02) + + thread = threading.Thread(target=answer_when_asked) + thread.start() + result = loop.run("goal", IncidentState(goal="g")) + thread.join() + assert result.stop is LoopStop.GOAL_MET + + +# -- fixes from the bug hunt ------------------------------------------------ + + +def test_a_halt_during_the_park_stops_the_run_as_human_stopped(tmp_path): + """`request_halt` used to be inert while parked — and an approval racing + the halt still executed the round. The halt must win, promptly.""" + loop_holder = {} + + def gate(proposal, verdict, should_stop=None): + # Simulate the operator pulling the cord mid-park; a halt-aware gate + # (file_approval is one) polls `should_stop` and returns early. + loop_holder["loop"].request_halt("operator pulled the cord") + assert should_stop is not None and should_stop() + return "approved" # even a racing approval must not execute now + + loop, trace = _loop(tmp_path, gate) + loop_holder["loop"] = loop + result = loop.run("goal", IncidentState(goal="g")) + assert result.stop is LoopStop.HUMAN_STOPPED + assert "operator pulled the cord" in result.detail + assert not any(e.phase == "start" for e in trace.read_events(result.run_id)) + + +def test_file_approval_returns_early_when_should_stop_fires(tmp_path): + gate = file_approval(tmp_path, timeout_seconds=30.0, poll_seconds=0.02) + started = time.monotonic() + decision = gate(_Plan(), None, should_stop=lambda: True) + assert decision == "timeout" + assert time.monotonic() - started < 5.0, "the gate waited out its full timeout" + + +def test_the_approval_wait_does_not_burn_max_seconds(tmp_path): + """A human who thinks longer than `max_seconds` used to hand the approved + round a negative budget — approved, then dead before its first node.""" + from grapharc.runtime.budget import Budget + + def slow_yes(proposal, verdict): + time.sleep(0.4) + return "approved" + + trace = TraceRecorder(tmp_path / "trace.jsonl") + model = ScriptedChatModel(responses=scripted_planner_replies()) + loop = build_loop( + model, trace=trace, approval=slow_yes, budget=Budget(max_seconds=0.3) + ) + result = loop.run("goal", IncidentState(goal="g")) + assert result.stop is LoopStop.GOAL_MET, (result.stop, result.detail) + + +def test_two_runs_sharing_a_directory_cannot_destroy_each_others_decision(tmp_path): + """A mismatched-fingerprint decision used to be *unlinked* by the other + run's poll — the human's approval was consumed by the wrong gate and both + runs timed out.""" + write_decision(tmp_path, fingerprint="fp-OTHER-RUN", decision="approved") + gate = file_approval(tmp_path, timeout_seconds=0.3, poll_seconds=0.02) + assert gate(_Plan(), None) == "timeout" + # The other run's decision is still there for the other run to consume. + decision = json.loads((tmp_path / DECISION_FILENAME).read_text()) + assert decision["fingerprint"] == "fp-OTHER-RUN" + + +def test_handshake_files_are_written_atomically(tmp_path): + """No reader may ever see half a request: written to a tmp then renamed.""" + from grapharc.planner.approval_file import _write_atomically + + target = tmp_path / "approval-request.json" + _write_atomically(target, {"fingerprint": "fp"}) + assert json.loads(target.read_text()) == {"fingerprint": "fp"} + assert not list(tmp_path.glob("*.tmp")) diff --git a/tests/test_async_kernel.py b/tests/test_async_kernel.py index bbd4abb..7d1e3c6 100644 --- a/tests/test_async_kernel.py +++ b/tests/test_async_kernel.py @@ -132,8 +132,8 @@ async def work(state: S) -> dict: await _graph(work, writes={"a"}, trace=trace).ainvoke({}, run_id="r-trace") phases = [(e.node, e.phase) for e in trace.read_events("r-trace")] - assert phases == [("n", "start"), ("n", "end")] - end = trace.read_events("r-trace")[1] + assert phases == [("topology", "topology"), ("n", "start"), ("n", "end")] + end = trace.read_events("r-trace")[2] assert end.state_delta == {"a": 3} assert end.duration_ms is not None @@ -590,7 +590,7 @@ def test_a_swallowed_goto_is_an_error_the_trace_records(trace): errors = [e for e in trace.read_events("r-goto") if e.phase == "error"] assert errors and "ghost" in errors[0].error # And the run stopped there: `landing` never opened a trace event. - assert {e.node for e in trace.read_events("r-goto")} == {"router"} + assert {e.node for e in trace.read_events("r-goto")} == {"topology", "router"} def test_a_send_naming_a_node_the_graph_does_not_have_raises(): diff --git a/tests/test_cli.py b/tests/test_cli.py index 00fcbf1..7b8776f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -793,6 +793,57 @@ def test_serve_reports_a_missing_server_package(monkeypatch, capsys): assert "server extra" in payload["error"] +def test_serve_live_root_reaches_create_app_and_the_startup_lines( + monkeypatch, capsys, tmp_path +): + record: dict = {} + monkeypatch.setitem(sys.modules, "grapharc.server", _server_stub(record)) + code, out, _ = call(["serve", "--live-root", str(tmp_path)], capsys) + assert code == 0 + assert record["create_app"] == {"live_root": str(tmp_path)} + assert "live view" in out and "/live" in out + + +def test_serve_live_token_is_passed_only_with_a_live_root(monkeypatch, capsys, tmp_path): + record: dict = {} + monkeypatch.setitem(sys.modules, "grapharc.server", _server_stub(record)) + code, _, _ = call( + ["serve", "--live-root", str(tmp_path), "--live-token", "s3cret"], capsys + ) + assert code == 0 + assert record["create_app"]["live_token"] == "s3cret" + + record.clear() + code, _, _ = call(["serve", "--live-token", "s3cret"], capsys) + assert code == 0 + assert record["create_app"] == {} + + +def test_serve_without_live_root_prints_no_live_line(monkeypatch, capsys): + monkeypatch.setitem(sys.modules, "grapharc.server", _server_stub({})) + code, out, _ = call(["serve"], capsys) + assert code == 0 + assert "live view" not in out # the cookbook pins the three plain lines + + +def test_serve_live_root_off_loopback_warns_about_exposure(monkeypatch, capsys, tmp_path): + record: dict = {} + monkeypatch.setitem(sys.modules, "grapharc.server", _server_stub(record)) + _, loopback_out, _ = call(["serve", "--live-root", str(tmp_path)], capsys) + assert "tunnel" not in loopback_out + _, exposed_out, _ = call( + ["serve", "--live-root", str(tmp_path), "--host", "0.0.0.0"], capsys + ) + assert "tunnel" in exposed_out + + +def test_serve_live_root_must_be_a_directory(monkeypatch, capsys, tmp_path): + monkeypatch.setitem(sys.modules, "grapharc.server", _server_stub({})) + code, payload, _ = call_json(["serve", "--live-root", str(tmp_path / "absent")], capsys) + assert code == 2 + assert "--live-root" in payload["error"] + + # -- replay / diff ------------------------------------------------------------ diff --git a/tests/test_cookbook_basics.py b/tests/test_cookbook_basics.py index 4ba3fc5..c0c3be7 100644 --- a/tests/test_cookbook_basics.py +++ b/tests/test_cookbook_basics.py @@ -533,6 +533,21 @@ def count(state: State) -> dict: for e in trace.read_events() ] assert printed == [ + { + "attempt": 1, + "graph": "counter", + "node": "topology", + "phase": "topology", + "step": 0, + "state_delta": { + "nodes": ["load", "count"], + "edges": [ + ["__start__", "load", "static"], + ["load", "count", "static"], + ["count", "__end__", "static"], + ], + }, + }, {"attempt": 1, "graph": "counter", "node": "load", "phase": "start", "step": 1}, { "attempt": 1, @@ -557,7 +572,11 @@ def count(state: State) -> dict: # The four fields the snippet filters out are on the lines anyway. for event in trace.read_events(): assert event.run_id and event.thread_id == "demo" and event.ts - assert all(e.duration_ms is not None for e in trace.read_events() if e.phase != "start") + assert all( + e.duration_ms is not None + for e in trace.read_events() + if e.phase not in ("start", "topology") + ) # -- "How do I see what a run spent, from inside a node?" ------------------ @@ -704,10 +723,12 @@ def save(state: State) -> dict: assert [ (e.attempt, e.step, e.node, e.phase) for e in trace.read_events() ] == [ + (1, 0, "topology", "topology"), (1, 1, "fetch", "start"), (1, 1, "fetch", "end"), (1, 2, "save", "start"), (1, 2, "save", "error"), + (2, 0, "topology", "topology"), (2, 3, "save", "start"), (2, 3, "save", "end"), ] diff --git a/tests/test_cookbook_serving.py b/tests/test_cookbook_serving.py index 07591f1..9cb652d 100644 --- a/tests/test_cookbook_serving.py +++ b/tests/test_cookbook_serving.py @@ -517,7 +517,7 @@ def test_the_serve_transcript_runs_against_a_real_server(tmp_path): trace = _run_console(_substituted(trace_step[0], mapping), tmp_path, env) assert trace.returncode == 0, trace.stderr phases = [json.loads(line)["phase"] for line in trace.stdout.splitlines()] - assert phases == ["start", "end"], trace.stdout + assert phases == ["topology", "start", "end"], trace.stdout finally: server.terminate() try: diff --git a/tests/test_planner_loop.py b/tests/test_planner_loop.py index b648b6f..74c4977 100644 --- a/tests/test_planner_loop.py +++ b/tests/test_planner_loop.py @@ -1496,4 +1496,78 @@ def test_stop_reasons_are_stable_machine_readable_strings(): "planning_failed", "execution_failed", "human_stopped", + "approval_denied", + "approval_timeout", } + + +def test_rejection_feedback_accumulates_across_rounds(): + """Round 3's prompt shows rounds 1 AND 2 — a model shown only the last + refusal re-proposes the round-before-last's mistake with no way to see + the pattern, and exhausts the rejection allowance repeating itself.""" + denied = plan(("ship", "deploy")) + loop, model, _ = build_loop( + [denied, denied, denied], + policy=DENY_DEPLOY, + ) + result = loop.run("goal") + assert result.stop is LoopStop.ADMISSION_REFUSED + + third_turn = " ".join(str(m.content) for m in model.calls[2]) + assert "Round 1:" in third_turn + assert "Round 2:" in third_turn + + +def test_accumulated_feedback_is_bounded_to_the_last_three_rounds(): + denied = plan(("ship", "deploy")) + loop, model, _ = build_loop( + [denied] * 5, + policy=DENY_DEPLOY, + limits=LoopLimits(max_consecutive_rejections=5, max_rounds=6), + ) + loop.run("goal") + fifth_turn = " ".join(str(m.content) for m in model.calls[4]) + assert "Round 1:" not in fifth_turn # dropped: the prompt must not grow forever + assert "Round 2:" in fifth_turn and "Round 4:" in fifth_turn + + +def test_an_unreachable_backend_stops_on_the_first_round(): + """A dead server is not a planning problem: rephrasing cannot reach it. + The loop used to burn its whole planning allowance re-dialing the same + socket, then report a planning failure for an infrastructure one.""" + + class DeadBackend: + def __init__(self): + self.calls = 0 + + def propose(self, task, ctx=None, *, feedback=""): + self.calls += 1 + from grapharc.planner.proposal import PlanningOutcome + + return PlanningOutcome( + error="planner model call failed: APIConnectionError('Connection error.')", + unreachable=True, + ) + + planner = DeadBackend() + bodies = Bodies() + reg = registry(bodies) + loop = GovernedLoop( + planner=planner, + checker=gate(reg), + materializer=Materializer(registry=reg, state_schema=LoopState), + ) + result = loop.run("goal") + + assert result.stop is LoopStop.PLANNING_FAILED + assert "could not be reached" in result.detail + assert planner.calls == 1, "the loop retried an unreachable backend" + assert bodies.ran == [] + + +def test_a_merely_bad_reply_still_gets_its_retries(): + """The unreachable shortcut must not swallow ordinary planning failures.""" + loop, model, _ = build_loop(["not json at all"], on_exhausted="repeat") + result = loop.run("goal") + assert result.stop is LoopStop.PLANNING_FAILED + assert len(result.rounds) == 3 # the full allowance, as before diff --git a/tests/test_replay.py b/tests/test_replay.py index c81a6ad..69b2c21 100644 --- a/tests/test_replay.py +++ b/tests/test_replay.py @@ -578,7 +578,8 @@ def test_sub_step_spans_nest_under_their_node(trace): children = [s for s in spans if s.parent_id == node_span.span_id] assert children, "the agent's model/tool sub-steps must nest under the node span" assert {s.attributes["grapharc.phase"] for s in children} >= {"model", "tool", "stop"} - assert replay(trace, "r1").orphan_sub_events == [] + orphans = replay(trace, "r1").orphan_sub_events + assert [e.phase for e in orphans] == ["topology"] # shape record, not lost work def test_error_spans_carry_the_error(trace): @@ -1242,7 +1243,10 @@ def test_resume_still_seeds_step_and_attempt_from_the_thread(trace): compiled.invoke({"question": "b"}, thread_id="t1", run_id="r2") second = trace.read_events("r2") - assert min(e.step for e in second) > max(first) + # The topology event always carries step 0 — it states shape, not order — + # so the monotonicity claim is about the *work* steps. + steps = [e.step for e in second if e.phase != "topology"] + assert min(steps) > max(first) assert {e.attempt for e in second} == {2} diff --git a/tests/test_server.py b/tests/test_server.py index 395c47e..f14b36e 100644 --- a/tests/test_server.py +++ b/tests/test_server.py @@ -328,7 +328,7 @@ def test_create_then_poll_to_completion(client): # The scripted model reports usage, so the meter must have seen both calls. assert done["usage"]["iterations"] == 2 assert done["usage"]["tokens"] > 0 - assert done["event_count"] == 4 # start+end for each of two nodes + assert done["event_count"] == 5 # topology, then start+end for each of two nodes def test_thread_id_defaults_to_the_session_id_and_is_honoured_when_given(client): @@ -401,6 +401,7 @@ def test_a_graph_of_async_nodes_runs(client): "reviewed": "AWAITED NOW", } assert [e["node"] for e in trace_events(client, sid)] == [ + "topology", "fetch", "fetch", "wrap", @@ -460,7 +461,7 @@ def test_interrupt_stops_the_run_before_the_next_node(client): # empty: it was never written. assert done["result"] == {"question": "q", "answer": "first done"} nodes = {e["node"] for e in trace_events(client, sid)} - assert nodes == {"first"} + assert nodes == {"topology", "first"} def test_interrupt_on_a_finished_session_is_reported_as_not_applied(client): @@ -521,10 +522,22 @@ def test_stream_replays_a_finished_run_and_terminates(client): frames = read_sse(response) kinds = [f[0] for f in frames] - assert kinds == ["trace"] * 4 + ["status", "done"] - assert [f[2] for f in frames[:4]] == ["1", "2", "3", "4"] - assert [f[1]["node"] for f in frames[:4]] == ["answer", "answer", "review", "review"] - assert [f[1]["phase"] for f in frames[:4]] == ["start", "end", "start", "end"] + assert kinds == ["trace"] * 5 + ["status", "done"] + assert [f[2] for f in frames[:5]] == ["1", "2", "3", "4", "5"] + assert [f[1]["node"] for f in frames[:5]] == [ + "topology", + "answer", + "answer", + "review", + "review", + ] + assert [f[1]["phase"] for f in frames[:5]] == [ + "topology", + "start", + "end", + "start", + "end", + ] assert frames[-2][1]["status"] == "succeeded" assert frames[-2][1]["result"]["answer"] == "42 is the answer." @@ -539,15 +552,21 @@ def test_stream_carries_events_recorded_after_it_opened(client): assert GATE_ENTERED.wait(timeout=10), "the gated node never started" opened_with = client.get(f"/sessions/{sid}").json() assert opened_with["status"] == "running" - assert opened_with["event_count"] == 1 # only `first`'s start event so far + assert opened_with["event_count"] == 2 # the topology event, then `first`'s start threading.Timer(0.2, GATE.set).start() with client.stream("GET", f"/sessions/{sid}/stream") as response: response.read() frames = read_sse(response) - assert [f[0] for f in frames] == ["trace", "trace", "trace", "trace", "status", "done"] - assert [f[1]["node"] for f in frames[:4]] == ["first", "first", "second", "second"] + assert [f[0] for f in frames] == ["trace"] * 5 + ["status", "done"] + assert [f[1]["node"] for f in frames[:5]] == [ + "topology", + "first", + "first", + "second", + "second", + ] assert frames[-2][1]["status"] == "succeeded" @@ -555,21 +574,21 @@ def test_stream_cursor_skips_events_already_seen(client): sid = start(client, "demo") wait_for(client, sid, {"succeeded"}) - with client.stream("GET", f"/sessions/{sid}/stream?cursor=3") as response: + with client.stream("GET", f"/sessions/{sid}/stream?cursor=4") as response: response.read() frames = read_sse(response) assert [f[0] for f in frames] == ["trace", "status", "done"] - assert frames[0][2] == "4" + assert frames[0][2] == "5" assert frames[0][1]["node"] == "review" assert frames[0][1]["phase"] == "end" with client.stream( - "GET", f"/sessions/{sid}/stream", headers={"last-event-id": "3"} + "GET", f"/sessions/{sid}/stream", headers={"last-event-id": "4"} ) as response: response.read() resumed = read_sse(response) assert [f[0] for f in resumed] == ["trace", "status", "done"] - assert resumed[0][2] == "4" + assert resumed[0][2] == "5" def test_stream_events_are_the_trace_file(client): @@ -613,7 +632,7 @@ def test_stream_and_trace_are_the_same_record_for_an_answer_over_2000_chars(clie disk_lines = client.get(f"/sessions/{sid}/trace").text.splitlines() on_disk = [json.loads(line) for line in disk_lines] - assert len(on_disk) == 2 and len(streamed) == 2, (len(on_disk), len(streamed)) + assert len(on_disk) == 3 and len(streamed) == 3, (len(on_disk), len(streamed)) assert streamed == on_disk assert streamed_lines == disk_lines # byte for byte, not just equal objects @@ -688,7 +707,7 @@ def _on_trace_event(self, session_id: str, payload: dict) -> None: assert done["status"] == "succeeded", done["error"] assert done["result"]["answer"] == "42 is the answer." # The trace file is complete even though every subscriber call failed. - assert len(trace_events(c, sid)) == 4 + assert len(trace_events(c, sid)) == 5 assert done["event_count"] == 0 @@ -771,6 +790,8 @@ def test_sse_arrives_incrementally_over_a_socket(live_server): assert response.status_code == 200 lines = response.iter_lines() name, event = next_frame(lines) + assert (name, event["phase"]) == ("trace", "topology") + name, event = next_frame(lines) # This frame reached the client while the graph is still parked in # `first` — nothing has finished, so nothing could have been replayed. assert (name, event["node"], event["phase"]) == ("trace", "first", "start") @@ -801,7 +822,7 @@ def test_trace_is_ndjson_and_empty_before_the_first_event(client): GATE.set() wait_for(client, sid, {"succeeded"}) events = trace_events(client, sid) - assert len(events) == 4 + assert len(events) == 5 assert {e["graph"] for e in events} == {"gated"} @@ -813,7 +834,7 @@ def test_sessions_do_not_share_a_trace_file(client): for sid, question in ((a, "a"), (b, "b")): events = trace_events(client, sid) - assert len(events) == 4 + assert len(events) == 5 assert {e["run_id"] for e in events} == {client.get(f"/sessions/{sid}").json()["run_id"]} assert client.get(f"/sessions/{sid}").json()["result"]["question"] == question @@ -1012,8 +1033,8 @@ def test_a_sync_only_checkpointer_runs_through_the_http_api(sqlite_saver, tmp_pa assert done["status"] == "succeeded", done["error"] assert done["result"]["answer"] == "42 is the answer." - assert done["event_count"] == 4 - assert len(trace_events(c, sid)) == 4 + assert done["event_count"] == 5 + assert len(trace_events(c, sid)) == 5 # The checkpointer was actually used, not quietly bypassed. saved = sqlite_saver.get_tuple({"configurable": {"thread_id": "conv-1"}}) @@ -1035,7 +1056,7 @@ def test_the_sync_driver_honours_an_interrupt_at_the_same_boundary(sqlite_saver, done = wait_for(c, sid, {"interrupted", "failed", "succeeded"}) assert done["status"] == "interrupted", done["error"] assert done["result"] == {"question": "q", "answer": "first done"} - assert {e["node"] for e in trace_events(c, sid)} == {"first"} + assert {e["node"] for e in trace_events(c, sid)} == {"topology", "first"} finally: GATE.set() diff --git a/tests/test_server_live.py b/tests/test_server_live.py new file mode 100644 index 0000000..74f306c --- /dev/null +++ b/tests/test_server_live.py @@ -0,0 +1,289 @@ +"""The live browser view: file-tailing snapshots over SSE, confined to a root. + +The router is mounted on a bare FastAPI app for most tests — it needs nothing +from the session runtime — and driven through `TestClient`, which buffers a +streaming response to completion; stream tests therefore write a +`termination_reason` (with the grace shrunk) so the stream actually ends. +""" + +from __future__ import annotations + +import json +import threading + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from grapharc.observe.metrics import summarize, to_mermaid +from grapharc.observe.trace import TailRecorder, TraceRecorder +from grapharc.server import live as live_module +from grapharc.server.app import create_app +from grapharc.server.live import ( + LivePathError, + build_snapshot, + live_router, + resolve_trace, + scan_traces, +) + + +def write_run(path, run_id, *, nodes=2, done=False, secret=None): + recorder = TraceRecorder(path) + for step in range(1, nodes + 1): + recorder.event(run_id=run_id, graph="g", node=f"n{step}", phase="start", step=step) + delta = {"answer": "ok"} + if secret: + delta["leaked"] = secret + recorder.event( + run_id=run_id, + graph="g", + node=f"n{step}", + phase="end", + step=step, + duration_ms=8.0, + tokens=5, + cost_usd=0.001, + state_delta=delta, + ) + if done: + recorder.event( + run_id=run_id, + graph="g", + node="finish", + phase="end", + step=nodes + 1, + state_delta={"termination_reason": "completed"}, + ) + return recorder + + +def live_client(root, **kwargs): + app = FastAPI() + app.include_router(live_router(root, poll_seconds=0.01, **kwargs)) + return TestClient(app) + + +def read_sse(response): + frames = [] + for block in response.text.split("\n\n"): + name = data = None + for line in block.splitlines(): + if line.startswith(":") or not line.strip(): + continue + key, _, value = line.partition(": ") + if key == "event": + name = value + elif key == "data": + data = value + if name is not None: + frames.append((name, json.loads(data))) + return frames + + +@pytest.fixture(autouse=True) +def fast_grace(monkeypatch): + monkeypatch.setattr(live_module, "DONE_GRACE_SECONDS", 0.05) + + +# --------------------------------------------------------------------------- +# TailRecorder and the pure helpers. +# --------------------------------------------------------------------------- + + +def test_tail_recorder_skips_a_torn_final_line_then_sees_it_complete(tmp_path): + path = tmp_path / "t.jsonl" + write_run(path, "r1") + whole = TailRecorder(path).read_events() + with path.open("a", encoding="utf-8") as f: + f.write('{"ts": "2026-01-01T00:00:00.000+00:00", "run_id": "r1", "graph": "g"') + + assert TailRecorder(path).read_events() == whole # torn tail invisible + + with path.open("a", encoding="utf-8") as f: + f.write(', "node": "late", "phase": "start", "step": 9}\n') + completed = TailRecorder(path).read_events() + assert len(completed) == len(whole) + 1 + assert completed[-1].node == "late" + + +def test_resolve_trace_confines_gate_style(tmp_path): + (tmp_path / "runs").mkdir() + (tmp_path / "runs" / "t.jsonl").touch() + assert resolve_trace(tmp_path, "runs/t.jsonl") == (tmp_path / "runs" / "t.jsonl").resolve() + # The target need not exist — the run may not have started. + resolve_trace(tmp_path, "not-yet/t.jsonl") + + for bad in ("../outside.jsonl", "/etc/passwd", "runs/t.txt", ""): + with pytest.raises(LivePathError): + resolve_trace(tmp_path, bad) + + +def test_resolve_trace_refuses_a_symlink_out_of_the_root(tmp_path): + outside = tmp_path / "outside" + outside.mkdir() + (outside / "t.jsonl").touch() + root = tmp_path / "root" + root.mkdir() + (root / "link").symlink_to(outside) + with pytest.raises(LivePathError): + resolve_trace(root, "link/t.jsonl") + + +def test_snapshot_agrees_with_viz_and_metrics(tmp_path): + write_run(tmp_path / "t.jsonl", "r1", done=True) + snapshot = build_snapshot(tmp_path, "t.jsonl", None) + recorder = TailRecorder(tmp_path / "t.jsonl") + assert snapshot.mermaid == to_mermaid(recorder, "r1") + assert snapshot.stats == summarize(recorder, "r1") + assert snapshot.cost_usd == pytest.approx(0.002) + assert snapshot.done is True + assert snapshot.mermaid_live_url and "#pako:" in snapshot.mermaid_live_url + + +def test_a_missing_file_is_a_waiting_snapshot_not_an_error(tmp_path): + snapshot = build_snapshot(tmp_path, "not-yet/t.jsonl", None) + assert snapshot.run_id is None + assert snapshot.mermaid == "" + assert snapshot.stats is None + assert not (tmp_path / "not-yet").exists(), "a read must not create directories" + + +def test_snapshot_follows_the_latest_run_unless_one_is_named(tmp_path): + path = tmp_path / "t.jsonl" + write_run(path, "first") + write_run(path, "second") + assert build_snapshot(tmp_path, "t.jsonl", None).run_id == "second" + assert build_snapshot(tmp_path, "t.jsonl", "first").run_id == "first" + assert build_snapshot(tmp_path, "t.jsonl", None).run_ids == ["first", "second"] + + +def test_an_open_node_reads_as_active_even_when_the_file_is_quiet(tmp_path): + """A delegated run writes nothing between start and finish; that is not idle.""" + import os + import time + + path = tmp_path / "t.jsonl" + TraceRecorder(path).event( + run_id="r1", graph="cli-agent", node="claude_code", phase="start", step=1 + ) + stale = time.time() - 60 # well past the activity window + os.utime(path, (stale, stale)) + assert build_snapshot(tmp_path, "t.jsonl", None).active is True + + +def test_scan_traces_lists_newest_first_with_run_ids(tmp_path): + import os + import time + + write_run(tmp_path / "a" / "t.jsonl", "ra") + write_run(tmp_path / "b" / "t.jsonl", "rb") + old = time.time() - 100 + os.utime(tmp_path / "a" / "t.jsonl", (old, old)) + traces = scan_traces(tmp_path) + assert [t["trace"] for t in traces] == ["b/t.jsonl", "a/t.jsonl"] + assert traces[0]["runs"] == ["rb"] + + +# --------------------------------------------------------------------------- +# The routes. +# --------------------------------------------------------------------------- + + +def test_stream_snapshots_a_finished_run_and_closes(tmp_path): + write_run(tmp_path / "t.jsonl", "r1", done=True) + with live_client(tmp_path) as client: + with client.stream("GET", "/live/api/stream?trace=t.jsonl") as response: + assert response.status_code == 200 + assert response.headers["content-type"].startswith("text/event-stream") + response.read() + frames = read_sse(response) + kinds = [f[0] for f in frames] + assert kinds[0] == "snapshot" and kinds[-1] == "done" + assert frames[0][1]["run_id"] == "r1" + assert frames[0][1]["done"] is True + + +def test_stream_emits_a_second_snapshot_when_the_file_grows(tmp_path): + path = tmp_path / "t.jsonl" + write_run(path, "r1") + + def finish(): + write_run(path, "r1", done=True) + + with live_client(tmp_path) as client: + threading.Timer(0.2, finish).start() + with client.stream("GET", "/live/api/stream?trace=t.jsonl") as response: + response.read() + frames = read_sse(response) + snapshots = [f[1] for f in frames if f[0] == "snapshot"] + assert len(snapshots) >= 2 + assert snapshots[0]["done"] is False + assert snapshots[-1]["done"] is True + assert snapshots[-1]["stats"]["events"] > snapshots[0]["stats"]["events"] + + +def test_stream_waits_for_a_file_that_does_not_exist_yet(tmp_path): + def start_run(): + write_run(tmp_path / "later" / "t.jsonl", "r1", done=True) + + with live_client(tmp_path) as client: + threading.Timer(0.2, start_run).start() + with client.stream("GET", "/live/api/stream?trace=later/t.jsonl") as response: + response.read() + frames = read_sse(response) + snapshots = [f[1] for f in frames if f[0] == "snapshot"] + assert snapshots[0]["run_id"] is None # waiting + assert snapshots[-1]["run_id"] == "r1" + + +def test_confinement_failures_are_404_on_every_route(tmp_path): + with live_client(tmp_path) as client: + for raw in ("../outside.jsonl", "/etc/passwd", "t.txt"): + assert client.get(f"/live/view?trace={raw}").status_code == 404 + assert client.get(f"/live/api/stream?trace={raw}").status_code == 404 + + +def test_state_delta_contents_never_reach_a_live_byte(tmp_path): + sentinel = "SECRET-SENTINEL-a2f9" + write_run(tmp_path / "t.jsonl", "r1", done=True, secret=sentinel) + assert sentinel in (tmp_path / "t.jsonl").read_text(), "precondition" + with live_client(tmp_path) as client: + for url in ("/live", "/live/api/runs", "/live/view?trace=t.jsonl"): + assert sentinel not in client.get(url).text + with client.stream("GET", "/live/api/stream?trace=t.jsonl") as response: + response.read() + assert sentinel not in response.text + + +def test_a_token_locks_every_live_route(tmp_path): + write_run(tmp_path / "t.jsonl", "r1", done=True) + with live_client(tmp_path, token="s3cret") as client: + for url in ("/live", "/live/api/runs", "/live/view?trace=t.jsonl"): + assert client.get(url).status_code == 401 + assert client.get("/live/api/runs?token=s3cret").status_code == 200 + assert ( + client.get( + "/live/api/runs", headers={"authorization": "Bearer s3cret"} + ).status_code + == 200 + ) + assert client.get("/live/api/runs?token=wrong").status_code == 401 + + +def test_the_index_lists_traces_and_links_the_viewer(tmp_path): + write_run(tmp_path / "runs" / "t.jsonl", "r1") + with live_client(tmp_path) as client: + page = client.get("/live").text + assert "runs/t.jsonl" in page + assert "r1" in page + assert client.get("/live/view?trace=runs/t.jsonl").status_code == 200 + + +def test_create_app_mounts_live_only_when_asked(tmp_path): + with TestClient(create_app()) as client: + assert client.get("/live").status_code == 404 + assert "/live/api/runs" not in client.get("/openapi.json").json()["paths"] + with TestClient(create_app(live_root=tmp_path)) as client: + assert client.get("/live").status_code == 200 + assert client.get("/healthz").status_code == 200 # existing API untouched diff --git a/tests/test_slack_gateway.py b/tests/test_slack_gateway.py index 9bd133b..7c95e91 100644 --- a/tests/test_slack_gateway.py +++ b/tests/test_slack_gateway.py @@ -133,7 +133,7 @@ def test_plan_registry_admits_only_the_shipped_modules(tmp_path): "plan goal --registry grapharc.examples.plan_docs:build_registry", workdir=tmp_path, ) - assert argv[-1] == "grapharc.examples.plan_docs:build_registry" + assert argv[argv.index("--registry") + 1] == "grapharc.examples.plan_docs:build_registry" with pytest.raises(SlackCommandError, match="accepts only"): parse_command("plan goal --registry os:system", workdir=tmp_path) with pytest.raises(SlackCommandError, match="accepts only"): @@ -142,11 +142,72 @@ def test_plan_registry_admits_only_the_shipped_modules(tmp_path): ) +def test_the_stdlib_plan_registry_needs_the_agent_double_opt_in(tmp_path): + """Agent-backed plan kinds run tools on the host: same gate as `agent`.""" + for kwargs in ({}, {"allow_agent": True}, {"allow_model": True}): + with pytest.raises(SlackCommandError, match="GRAPHARC_SLACK_ALLOW_AGENT"): + parse_command( + "plan goal --registry grapharc.stdlib:build_registry", + workdir=tmp_path, + **kwargs, + ) + + +def test_a_slack_stdlib_plan_is_always_parked_on_the_approval_gate(tmp_path): + argv = parse_command( + "plan goal --registry grapharc.stdlib:build_registry", + workdir=tmp_path, + allow_model=True, + allow_agent=True, + timeout_seconds=600, + ) + assert "--approve" in argv + assert argv[argv.index("--approval-timeout") + 1] == "300.0" + + # A requester-set gate config wins over the injection. + explicit = parse_command( + "plan goal --registry grapharc.stdlib:build_registry --approval-timeout 90", + workdir=tmp_path, + allow_model=True, + allow_agent=True, + timeout_seconds=600, + ) + assert explicit.count("--approval-timeout") == 1 + assert explicit[explicit.index("--approval-timeout") + 1] == "90" + + +def test_the_demo_plan_registries_stay_reachable_without_opt_ins(tmp_path): + argv = parse_command( + "plan goal --registry grapharc.examples.plan_docs:build_registry", + workdir=tmp_path, + ) + assert "--approve" not in argv # only the host-acting registry is parked + + +def test_a_command_pasted_with_code_backticks_still_parses(tmp_path): + """Copying from a code-formatted Slack message brings the backticks along.""" + argv = parse_command( + "`approve slack-runs/x/trace.jsonl`", workdir=tmp_path + ) + assert argv == ["approve", "slack-runs/x/trace.jsonl"] + # A single stray trailing backtick — half a copy — is tolerated too. + argv = parse_command("approve slack-runs/x/trace.jsonl`", workdir=tmp_path) + assert argv == ["approve", "slack-runs/x/trace.jsonl"] + + +def test_approve_is_admitted_with_path_confinement(tmp_path): + argv = parse_command("approve slack-runs/x/trace.jsonl --deny", workdir=tmp_path) + assert argv == ["approve", "slack-runs/x/trace.jsonl", "--deny"] + with pytest.raises(SlackCommandError, match="escapes"): + parse_command("approve ../elsewhere/trace.jsonl", workdir=tmp_path) + + def test_model_is_refused_by_default_and_admitted_on_opt_in(tmp_path): with pytest.raises(SlackCommandError, match="paid backend"): parse_command("plan 'a goal' --model mock/x", workdir=tmp_path) argv = parse_command("plan 'a goal' --model mock/x", workdir=tmp_path, allow_model=True) - assert argv == ["plan", "a goal", "--model", "mock/x"] + assert argv[:4] == ["plan", "a goal", "--model", "mock/x"] + assert "--trace" in argv # tracing commands get a trace the bot can find def test_a_path_positional_may_not_escape_the_workdir(tmp_path): @@ -171,7 +232,48 @@ def test_a_path_inside_the_workdir_is_admitted_even_absolute(tmp_path): def test_a_quoted_goal_survives_as_one_argument(tmp_path): argv = parse_command('plan "investigate the checkout outage"', workdir=tmp_path) - assert argv == ["plan", "investigate the checkout outage"] + assert argv[:2] == ["plan", "investigate the checkout outage"] + + +def test_a_tracing_command_gets_a_unique_injected_trace(tmp_path): + first = parse_command("run graph.toml", workdir=tmp_path) + second = parse_command("run graph.toml", workdir=tmp_path) + first_trace = first[first.index("--trace") + 1] + second_trace = second[second.index("--trace") + 1] + assert first_trace.startswith("slack-runs/") + assert first_trace.endswith("trace.jsonl") + assert first_trace != second_trace, "a reused path would replay another run" + + +def test_a_requester_named_trace_wins_over_injection(tmp_path): + argv = parse_command("run graph.toml --trace runs/mine.jsonl", workdir=tmp_path) + assert argv.count("--trace") == 1 + assert argv[argv.index("--trace") + 1] == "runs/mine.jsonl" + + +def test_agent_also_gets_an_injected_trace(tmp_path): + argv = parse_command( + "agent task", workdir=tmp_path, allow_model=True, allow_agent=True + ) + assert "--trace" in argv + + +def test_readers_get_no_trace_injection(tmp_path): + assert "--trace" not in parse_command("metrics t.jsonl r1", workdir=tmp_path) + assert "--trace" not in parse_command("models", workdir=tmp_path) + + +def test_trace_path_resolves_the_injected_and_named_forms(tmp_path): + from grapharc.slack.command import trace_path + + argv = parse_command("run graph.toml", workdir=tmp_path) + resolved = trace_path(argv, tmp_path) + assert resolved is not None and resolved.is_relative_to(tmp_path) + + inline = trace_path(["run", "g.toml", "--trace=runs/t.jsonl"], tmp_path) + assert inline == tmp_path / "runs" / "t.jsonl" + + assert trace_path(["metrics", "t.jsonl", "r1"], tmp_path) is None def test_empty_text_answers_with_usage_not_a_traceback(tmp_path): @@ -229,7 +331,7 @@ def test_truncation_is_announced_never_silent(): timeout_seconds=60, ) message = format_result(result) - assert "500 more characters not shown" in message + assert "500 earlier characters not shown" in message def test_a_fence_in_the_output_cannot_break_out(): @@ -317,6 +419,54 @@ def test_config_reads_workdir_timeout_and_model_opt_in(tmp_path): assert config.allow_agent +def test_live_config_defaults_on_and_reads_the_switches(tmp_path): + base = {"SLACK_BOT_TOKEN": "xoxb-x", "SLACK_APP_TOKEN": "xapp-x"} + assert SlackBotConfig.from_env(dict(base)).live is True + assert SlackBotConfig.from_env({**base, "GRAPHARC_SLACK_LIVE": "0"}).live is False + config = SlackBotConfig.from_env({**base, "GRAPHARC_SLACK_LIVE_INTERVAL": "5"}) + assert config.live_interval_seconds == 5.0 + + +def test_a_bad_live_interval_is_a_named_error(tmp_path): + base = {"SLACK_BOT_TOKEN": "xoxb-x", "SLACK_APP_TOKEN": "xapp-x"} + with pytest.raises(SlackConfigError, match="GRAPHARC_SLACK_LIVE_INTERVAL"): + SlackBotConfig.from_env({**base, "GRAPHARC_SLACK_LIVE_INTERVAL": "soon"}) + with pytest.raises(SlackConfigError, match="positive"): + SlackBotConfig.from_env({**base, "GRAPHARC_SLACK_LIVE_INTERVAL": "0"}) + + +def test_live_url_base_is_validated_and_stripped(tmp_path): + base = {"SLACK_BOT_TOKEN": "xoxb-x", "SLACK_APP_TOKEN": "xapp-x"} + assert SlackBotConfig.from_env(dict(base)).live_url_base is None + config = SlackBotConfig.from_env( + {**base, "GRAPHARC_SLACK_LIVE_URL": "https://laptop.tailnet.ts.net/"} + ) + assert config.live_url_base == "https://laptop.tailnet.ts.net" + with pytest.raises(SlackConfigError, match="GRAPHARC_SLACK_LIVE_URL"): + SlackBotConfig.from_env({**base, "GRAPHARC_SLACK_LIVE_URL": "laptop:8000"}) + + +def test_live_view_url_is_composed_only_for_tracing_argvs(tmp_path): + from grapharc.slack.format import live_view_url + + argv = parse_command("run graph.toml", workdir=tmp_path) + url = live_view_url(argv, base="https://laptop.example", workdir=tmp_path) + assert url is not None + assert url.startswith("https://laptop.example/live/view?trace=slack-runs%2F") + + assert live_view_url(argv, base=None, workdir=tmp_path) is None + reader = parse_command("metrics t.jsonl r1", workdir=tmp_path) + assert live_view_url(reader, base="https://laptop.example", workdir=tmp_path) is None + + +def test_live_view_url_carries_the_run_id_when_named(tmp_path): + from grapharc.slack.format import live_view_url + + argv = parse_command("run graph.toml --run-id r7", workdir=tmp_path) + url = live_view_url(argv, base="https://laptop.example", workdir=tmp_path) + assert url is not None and url.endswith("&run=r7") + + def test_handle_text_turns_a_refusal_into_a_message_not_an_exception(tmp_path): from grapharc.slack.bot import handle_text diff --git a/tests/test_slack_live.py b/tests/test_slack_live.py new file mode 100644 index 0000000..e7408d4 --- /dev/null +++ b/tests/test_slack_live.py @@ -0,0 +1,373 @@ +"""The live tail loop and its Slack-free composition in `handle_text_live`. + +Everything is driven through plain callables — a recording sink instead of a +WebClient — so none of this needs a token or a network. The end-to-end tests +run the real CLI subprocess through `run_command`, same as the gateway tests. +""" + +from __future__ import annotations + +import time + +from grapharc.observe.replay import replay +from grapharc.observe.trace import TraceRecorder +from grapharc.slack.bot import handle_text_live +from grapharc.slack.config import SlackBotConfig +from grapharc.slack.live import FLAT_FEED_LINES, LiveSettings, LiveTail, render_progress + +FAST = LiveSettings(update_interval=0.01, poll_interval=0.01, join_timeout=2.0) + + +def _write_run(recorder: TraceRecorder, run_id: str, *, nodes: int = 2) -> None: + for step in range(1, nodes + 1): + recorder.event( + run_id=run_id, graph="g", node=f"n{step}", phase="start", step=step + ) + recorder.event( + run_id=run_id, + graph="g", + node=f"n{step}", + phase="end", + step=step, + duration_ms=12.0, + tokens=10, + ) + + +def _wait_for(predicate, timeout=3.0): + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + if predicate(): + return True + time.sleep(0.01) + return False + + +# --------------------------------------------------------------------------- +# LiveTail: offset discipline, throttling, failure posture. +# --------------------------------------------------------------------------- + + +def test_only_events_after_the_start_offset_render(tmp_path): + """A reused trace file must not replay an earlier run as this one's progress.""" + path = tmp_path / "trace.jsonl" + _write_run(TraceRecorder(path), "old-run") + + seen: list[str] = [] + with LiveTail(path, ["agent", "task"], lambda t: seen.append(t) or True, FAST): + _write_run(TraceRecorder(path), "new-run") + assert _wait_for(lambda: seen) + assert all("old-run" not in t for t in seen), "the old run leaked into live progress" + + +def test_no_update_without_new_bytes(tmp_path): + path = tmp_path / "trace.jsonl" + TraceRecorder(path) # creates the parent; file itself absent + + seen: list[str] = [] + with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + time.sleep(0.2) + assert seen == [] + + +def test_identical_renders_are_not_reposted(tmp_path): + path = tmp_path / "trace.jsonl" + seen: list[str] = [] + with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + _write_run(TraceRecorder(path), "r1") + assert _wait_for(lambda: seen) + count = len(seen) + time.sleep(0.2) # no new bytes, no new update + assert len(seen) == count + + +def test_a_dead_sink_silences_the_tail_without_raising(tmp_path): + path = tmp_path / "trace.jsonl" + calls: list[str] = [] + + def dead(text: str) -> bool: + calls.append(text) + return False + + with LiveTail(path, ["run", "g.toml"], dead, FAST): + recorder = TraceRecorder(path) + _write_run(recorder, "r1") + assert _wait_for(lambda: calls) + _write_run(recorder, "r1") + time.sleep(0.2) + assert len(calls) == 1 + + +def test_an_update_that_raises_never_escapes_the_thread(tmp_path): + path = tmp_path / "trace.jsonl" + + def explode(text: str) -> bool: + raise RuntimeError("sink blew up") + + with LiveTail(path, ["run", "g.toml"], explode, FAST): + _write_run(TraceRecorder(path), "r1") + time.sleep(0.2) + # reaching here without an exception is the assertion + + +def test_a_torn_final_line_is_left_for_the_next_tick(tmp_path): + path = tmp_path / "trace.jsonl" + seen: list[str] = [] + with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + _write_run(TraceRecorder(path), "r1") + with path.open("a", encoding="utf-8") as f: + f.write('{"ts": "2026-01-01T00:00:00.000+00:00", "run_id": "r1"') # no newline + assert _wait_for(lambda: seen) + assert seen # progress rendered from the complete lines + + +# --------------------------------------------------------------------------- +# render_progress +# --------------------------------------------------------------------------- + + +def _run_from(tmp_path, events_writer, run_id="r1"): + path = tmp_path / "render.jsonl" + recorder = TraceRecorder(path) + events_writer(recorder, run_id) + return replay(recorder, run_id) + + +def test_progress_shows_marks_durations_and_the_diagram_link(tmp_path): + def write(recorder, run_id): + _write_run(recorder, run_id) + recorder.event(run_id=run_id, graph="g", node="n3", phase="start", step=3) + recorder.event( + run_id=run_id, graph="g", node="n4", phase="start", step=4 + ) + recorder.event( + run_id=run_id, + graph="g", + node="n4", + phase="error", + step=4, + error="citation not found", + ) + + run = _run_from(tmp_path, write) + text = render_progress( + run, argv=["run", "g.toml"], elapsed_s=14.2, diagram="flowchart TD\n a --> b" + ) + assert "✓ n1" in text and "12ms" in text and "10 tok" in text + assert "▸ n3" in text and "running…" in text + assert "✗ n4" in text and "citation not found" in text + assert "running (14s)" in text + assert "2/4 nodes done" not in text # 3 completed: n1, n2, n4(error) + assert "3/4 nodes done" in text + assert "mermaid.live/view#pako:" in text + assert "\x1b" not in text + + +def test_an_agent_shaped_run_renders_the_flat_feed(tmp_path): + def write(recorder, run_id): + for step in range(1, FLAT_FEED_LINES + 6): + recorder.event( + run_id=run_id, + graph="agent", + node="agent:model", + phase="model", + step=step, + tokens=7, + ) + + run = _run_from(tmp_path, write) + assert not run.executions, "precondition: an all-orphan run" + text = render_progress(run, argv=["agent", "task"], elapsed_s=3.0) + assert "model agent:model" in text + assert "… 5 earlier events" in text + assert "nodes done" not in text + + +def test_a_huge_progress_body_announces_truncation(tmp_path): + def write(recorder, run_id): + for step in range(1, 200): + recorder.event( + run_id=run_id, + graph="g", + node=f"node-with-a-rather-long-name-{step:04d}", + phase="start", + step=step, + ) + recorder.event( + run_id=run_id, + graph="g", + node=f"node-with-a-rather-long-name-{step:04d}", + phase="end", + step=step, + duration_ms=5.0, + ) + + run = _run_from(tmp_path, write) + text = render_progress(run, argv=["run", "g.toml"], elapsed_s=60.0) + assert "earlier characters not shown" in text + + +# --------------------------------------------------------------------------- +# handle_text_live end to end, with the real CLI and a recording sink. +# --------------------------------------------------------------------------- + + +class RecordingSink: + def __init__(self, *, post_ok: bool = True, updates_ok: bool = True): + self.post_ok = post_ok + self.updates_ok = updates_ok + self.posted: list[str] = [] + self.updated: list[str] = [] + + def post(self, text: str): + if not self.post_ok: + return None + self.posted.append(text) + return ("C1", "171.1") + + def update(self, handle, text: str) -> bool: + if not self.updates_ok: + return False + assert handle == ("C1", "171.1") + self.updated.append(text) + return True + + +def _config(tmp_path, **overrides) -> SlackBotConfig: + values = dict( + bot_token="xoxb-x", + app_token="xapp-x", + workdir=tmp_path, + timeout_seconds=60.0, + live_interval_seconds=0.05, + ) + values.update(overrides) + return SlackBotConfig(**values) + + +def test_a_tracing_command_posts_once_and_finishes_in_the_status_message(tmp_path): + sink = RecordingSink() + reply = handle_text_live("demo stage0", _config(tmp_path), sink) + assert reply == "" + assert len(sink.posted) == 1 + assert sink.posted[0].startswith("`grapharc demo") + assert "did its job" in sink.updated[-1] + + +def test_a_reader_takes_the_plain_blocking_path(tmp_path): + sink = RecordingSink() + reply = handle_text_live("models", _config(tmp_path), sink) + assert sink.posted == [] and sink.updated == [] + assert "did its job" in reply + + +def test_a_failed_post_falls_back_to_the_blocking_reply(tmp_path): + sink = RecordingSink(post_ok=False) + reply = handle_text_live("demo stage0", _config(tmp_path), sink) + assert "did its job" in reply + assert sink.updated == [] + + +def test_a_failed_final_update_returns_the_result_for_the_caller_to_post(tmp_path): + sink = RecordingSink(updates_ok=False) + reply = handle_text_live("demo stage0", _config(tmp_path), sink) + assert "did its job" in reply, "the final result must never be lost" + + +def test_live_off_takes_the_plain_path_even_for_tracing_commands(tmp_path): + sink = RecordingSink() + reply = handle_text_live("demo stage0", _config(tmp_path, live=False), sink) + assert sink.posted == [] + assert "did its job" in reply + + +def test_the_status_message_advertises_the_live_url_when_configured(tmp_path): + sink = RecordingSink() + config = _config(tmp_path, live_url_base="https://laptop.example") + handle_text_live("demo stage0", config, sink) + assert "watch live: https://laptop.example/live/view?trace=" in sink.posted[0] + assert "(if the live server is up)" in sink.posted[0] + + +def test_the_final_message_keeps_the_diagram_and_run_page_links(tmp_path): + """Finishing a run must not be what makes its links disappear.""" + sink = RecordingSink() + config = _config(tmp_path, live_url_base="https://laptop.example") + reply = handle_text_live("demo stage0", config, sink) + assert reply == "" + final = sink.updated[-1] + assert "did its job" in final + assert "mermaid.live/view#pako:" in final, "the final diagram link is kept" + assert "run page: https://laptop.example/live/view?trace=" in final + + +def test_the_blocking_path_also_gets_the_final_links(tmp_path): + reply = handle_text_live("demo stage0", _config(tmp_path, live=False), RecordingSink()) + assert "did its job" in reply + assert "mermaid.live/view#pako:" in reply + + +def test_a_refusal_is_still_a_returned_message(tmp_path): + sink = RecordingSink() + reply = handle_text_live("<@U012345> serve", _config(tmp_path), sink) + assert "not a command this bot runs" in reply + assert sink.posted == [] + + +# --------------------------------------------------------------------------- +# Bug-hunt regressions +# --------------------------------------------------------------------------- + + +def test_planned_lines_keep_each_rounds_own_status(tmp_path): + """Keyed by bare name, round 1's finished node once wore round 2's + still-running state (and vice versa).""" + def write(recorder, run_id): + for graph, done in (("plan:aaa", True), ("plan:bbb", False)): + recorder.event( + run_id=run_id, graph=graph, node="topology", phase="topology", + step=0, + state_delta={"nodes": ["search"], "edges": [], + "round": 1 if graph == "plan:aaa" else 2}, + ) + recorder.event(run_id=run_id, graph=graph, node="search", + phase="start", step=1) + if done: + recorder.event(run_id=run_id, graph=graph, node="search", + phase="end", step=1, duration_ms=100.0) + + run = _run_from(tmp_path, write) + text = render_progress(run, argv=["plan", "g"], elapsed_s=5.0) + round1 = text.split("round 2:")[0] + round2 = text.split("round 2:")[1] + assert "✓ search" in round1, text + assert "▸ search" in round2, text + + +def test_a_replaced_trace_file_does_not_silence_the_tailer(tmp_path): + path = tmp_path / "trace.jsonl" + _write_run(TraceRecorder(path), "big-old-run", nodes=6) # a large file + + seen: list[str] = [] + with LiveTail(path, ["run", "g.toml"], lambda t: seen.append(t) or True, FAST): + path.unlink() + _write_run(TraceRecorder(path), "fresh-run", nodes=1) # smaller file + assert _wait_for(lambda: any("fresh-run" not in t or True for t in seen) and seen) + assert seen, "the tailer stayed silent after the file was replaced" + + +def test_the_final_diagram_is_this_runs_not_a_previous_ones(tmp_path): + from grapharc.slack.bot import _with_final_links + from grapharc.slack.config import SlackBotConfig + + path = tmp_path / "reused.jsonl" + _write_run(TraceRecorder(path), "previous-run") + config = SlackBotConfig(bot_token="x", app_token="x", workdir=tmp_path) + + # This invocation wrote nothing (failed before its first event): no + # diagram at all, never the previous run's presented as the outcome. + text = _with_final_links( + "result", ["run", "g.toml"], path, config, + prior_runs=frozenset({"previous-run"}), + ) + assert "mermaid.live" not in text diff --git a/tests/test_stage0_gate.py b/tests/test_stage0_gate.py index 718375c..124e8e4 100644 --- a/tests/test_stage0_gate.py +++ b/tests/test_stage0_gate.py @@ -56,7 +56,12 @@ def test_gate_crash_mid_write_then_resume_yields_exactly_one_report( # Replay-point identity survives the resume: both attempts share the # thread_id, attempts are numbered, and step numbers never collide. - events = [e for e in trace.read_events() if e.thread_id == "t1"] + events = [ + e + for e in trace.read_events() + # Topology events carry step 0 on every attempt — shape, not order. + if e.thread_id == "t1" and e.phase != "topology" + ] assert events, "trace events must carry thread_id" first = [e for e in events if e.attempt == 1] second = [e for e in events if e.attempt == 2] diff --git a/tests/test_stdlib.py b/tests/test_stdlib.py index 983fe0e..d4a38f1 100644 --- a/tests/test_stdlib.py +++ b/tests/test_stdlib.py @@ -267,3 +267,79 @@ def test_a_phase_that_gave_up_is_labelled_as_such(workspace): assert state["notes"], state # Either it met the target or the reason is on the line; never silent. assert state["notes"][0] == "partial" or state["notes"][0].startswith("[") + + +# -- the plan-registry contract (`grapharc plan --registry grapharc.stdlib:...`) -- + + +def test_stdlib_ships_the_full_registry_module_contract(): + """Everything `cli/plan.py` reads by getattr, present and coherent.""" + import grapharc.stdlib as stdlib + + assert stdlib.STATE_SCHEMA is stdlib.WorkState + assert callable(stdlib.build_loop) + assert callable(stdlib.goal_met) + replies = stdlib.scripted_planner_replies() + assert replies, "the spend-free smoke path needs at least one reply" + + +def test_goal_met_reads_notes_defensively(): + from grapharc.stdlib import WorkState, goal_met + + assert not goal_met(WorkState()) + assert goal_met(WorkState(notes=["a report landed"])) + assert not goal_met(object()) # no notes attribute: not met, not a crash + + +def test_the_scripted_plan_runs_spend_free_and_stops_cleanly(tmp_path): + """The whole loop through stdlib's own builder, no model, no network.""" + from grapharc.observe.trace import TraceRecorder + from grapharc.planner import LoopStop + from grapharc.stdlib import WorkState, build_loop, scripted_planner_replies + from grapharc.testing import ScriptedChatModel + + trace = TraceRecorder(tmp_path / "t.jsonl") + model = ScriptedChatModel(responses=scripted_planner_replies()) + loop = build_loop(model, trace=trace) + result = loop.run("smoke", WorkState(goal="smoke")) + + # The deterministic kinds cannot write `notes`, so the honest clean stop + # is "no further work" — never a burn to planning_failed. + assert result.stop is LoopStop.NO_FURTHER_WORK + assert result.state.findings, "collect_context must actually have run" + phases = {e.phase for e in trace.read_events(result.run_id)} + assert "topology" in phases # the admitted round's shape is on the trace + + +def test_parallel_phases_merge_instead_of_colliding(tmp_path): + """The schema's whole claim: two phases finishing in one superstep must + merge. Without `operator.add` reducers LangGraph raised InvalidUpdateError + ("Can receive only one value per step"), so every fan-out a planner + proposed — the natural shape for a decomposable job — died at execution.""" + from grapharc.observe.trace import TraceRecorder + from grapharc.runtime.graph import END, START, GraphARC + from grapharc.stdlib import WorkState + + trace = TraceRecorder(tmp_path / "t.jsonl") + g = GraphARC(WorkState, name="fanout", trace=trace) + g.add_node("seed", lambda s: {"findings": ["seed"]}, writes={"findings"}) + for name in ("a", "b", "c"): + body = (lambda n: lambda s: {"findings": [f"from {n}"]})(name) + g.add_node(name, body, writes={"findings"}) + g.add_edge("seed", name) + g.add_edge(name, "join") + g.add_node("join", lambda s: None, writes=set()) + g.add_edge(START, "seed") + g.add_edge("join", END) + + out = g.compile().invoke({"goal": "x"}, run_id="r1") + assert sorted(out["findings"]) == ["from a", "from b", "from c", "seed"] + + +def test_phases_return_only_their_own_addition(tmp_path): + """With a reducer, returning the accumulated list would double it.""" + from grapharc.stdlib import WorkState, _collect_context + + body = _collect_context(None) + delta = body(WorkState(findings=["already here"])) + assert len(delta["findings"]) == 1, delta diff --git a/tests/test_topology_viz.py b/tests/test_topology_viz.py new file mode 100644 index 0000000..f90c7dc --- /dev/null +++ b/tests/test_topology_viz.py @@ -0,0 +1,295 @@ +"""The topology trace event and the overlay renderer built on it. + +The event states the graph's declared shape; the renderer draws that shape +with execution status overlaid. Together they are what makes a diagram show +the *orchestration* — branches not taken included — rather than only the path +that ran. +""" + +from __future__ import annotations + +from grapharc.observe.metrics import to_mermaid +from grapharc.observe.trace import TraceRecorder +from grapharc.runtime.graph import END, START, GraphARC +from grapharc.runtime.state import GraphARCState + + +class State(GraphARCState): + value: int = 0 + route: str = "" + + +def _touch(state: State) -> dict: + return {"value": state.value + 1} + + +# -- the event -------------------------------------------------------------- + + +def test_topology_is_the_first_event_and_carries_the_declared_shape(tmp_path): + trace = TraceRecorder(tmp_path / "t.jsonl") + g = GraphARC(State, name="shape", trace=trace) + g.add_node("a", _touch, writes={"value"}) + g.add_node("b", _touch, writes={"value"}) + g.add_edge(START, "a") + g.add_edge("a", "b") + g.add_edge("b", END) + g.compile().invoke({}, run_id="r1") + + events = trace.read_events("r1") + first = events[0] + assert (first.phase, first.node, first.step) == ("topology", "topology", 0) + assert first.state_delta == { + "nodes": ["a", "b"], + "edges": [ + ["__start__", "a", "static"], + ["a", "b", "static"], + ["b", "__end__", "static"], + ], + } + # And it precedes any work: the next event is the first node's start. + assert events[1].phase == "start" + + +def test_conditional_routes_are_recorded_with_their_kind(tmp_path): + trace = TraceRecorder(tmp_path / "t.jsonl") + g = GraphARC(State, name="branchy", trace=trace) + g.add_node("decide", lambda s: {"route": "left"}, writes={"route"}) + g.add_node("left", _touch, writes={"value"}) + g.add_node("right", _touch, writes={"value"}) + g.add_edge(START, "decide") + g.add_conditional_edge( + "decide", lambda s: s.route, {"left": "left", "right": "right"} + ) + g.add_edge("left", END) + g.add_edge("right", END) + g.compile().invoke({}, run_id="r1") + + delta = trace.read_events("r1")[0].state_delta + assert ["decide", "left", "conditional"] in delta["edges"] + assert ["decide", "right", "conditional"] in delta["edges"] + + +# -- the renderer ----------------------------------------------------------- + + +def _hand_written(tmp_path, *, error: bool = False) -> TraceRecorder: + """A trace with declared topology and a partial execution over it.""" + trace = TraceRecorder(tmp_path / "hand.jsonl") + trace.event( + run_id="r1", + graph="g", + node="topology", + phase="topology", + step=0, + state_delta={ + "nodes": ["plan", "act", "verify"], + "edges": [ + ["__start__", "plan", "static"], + ["plan", "act", "static"], + ["act", "verify", "static"], + ["verify", "__end__", "static"], + ], + }, + ) + trace.event(run_id="r1", graph="g", node="plan", phase="start", step=1) + trace.event(run_id="r1", graph="g", node="plan", phase="end", step=1) + trace.event(run_id="r1", graph="g", node="act", phase="start", step=2) + if error: + trace.event( + run_id="r1", graph="g", node="act", phase="error", step=2, error="boom" + ) + return trace + + +def test_overlay_marks_done_running_and_pending(tmp_path): + diagram = to_mermaid(_hand_written(tmp_path), "r1") + assert diagram.startswith("flowchart TD") + # Declared shape, whether or not it ran: + assert '["verify"]' in diagram + # Status classes: plan finished, act is open, verify never started. + assert "class n0 done" in diagram + assert "class n1 running" in diagram + assert "class n2 pending" in diagram + + +def test_overlay_keeps_the_error_rhombus_and_marks_errored(tmp_path): + diagram = to_mermaid(_hand_written(tmp_path, error=True), "r1") + assert '-.->|error| err0{"boom"}' in diagram + assert "class n1 errored" in diagram + + +def test_multiple_round_graphs_become_clusters(tmp_path): + trace = TraceRecorder(tmp_path / "rounds.jsonl") + for round_no, graph in ((1, "plan:aaa"), (2, "plan:bbb")): + trace.event( + run_id="r1", + graph=graph, + node="topology", + phase="topology", + step=0, + state_delta={ + "nodes": ["triage"], + "edges": [["__start__", "triage", "static"]], + "round": round_no, + }, + ) + diagram = to_mermaid(trace, "r1") + assert 'subgraph cluster0["round 1"]' in diagram + assert 'subgraph cluster1["round 2"]' in diagram + # Node ids are cluster-prefixed, so the reused name cannot collide. + assert 'g0_n0["triage"]' in diagram + assert 'g1_n0["triage"]' in diagram + # Cluster ids never carry the raw graph name's colon. + for line in diagram.splitlines(): + if line.strip().startswith("subgraph"): + assert ":" not in line.split("[")[0] + + +def test_every_overlay_line_is_balanced(tmp_path): + """The delimiter property the capstone gate pins, extended to the overlay.""" + diagram = to_mermaid(_hand_written(tmp_path, error=True), "r1") + for line in diagram.splitlines(): + assert line.count("{") == line.count("}"), line + assert line.count("[") == line.count("]"), line + assert line.count('"') % 2 == 0, line + + +def test_a_trace_without_topology_renders_exactly_as_before(tmp_path): + trace = TraceRecorder(tmp_path / "old.jsonl") + trace.event(run_id="r1", graph="g", node="a", phase="start", step=1) + trace.event(run_id="r1", graph="g", node="a", phase="end", step=1) + trace.event(run_id="r1", graph="g", node="b", phase="start", step=2) + trace.event(run_id="r1", graph="g", node="b", phase="end", step=2) + assert to_mermaid(trace, "r1") == ( + "flowchart TD\n" + ' start((start)) --> n0["a"]\n' + ' n0["a"] --> n1["b"]' + ) + + +def test_fanout_workers_connect_to_their_source_at_render_time(tmp_path): + trace = TraceRecorder(tmp_path / "fan.jsonl") + trace.event( + run_id="r1", + graph="g", + node="topology", + phase="topology", + step=0, + state_delta={ + "nodes": ["dispatch", "worker"], + "edges": [["__start__", "dispatch", "static"]], + "fanout_sources": ["dispatch"], + }, + ) + trace.event(run_id="r1", graph="g", node="dispatch", phase="start", step=1) + trace.event(run_id="r1", graph="g", node="dispatch", phase="end", step=1) + trace.event(run_id="r1", graph="g", node="worker", phase="start", step=2) + trace.event(run_id="r1", graph="g", node="worker", phase="end", step=2) + diagram = to_mermaid(trace, "r1") + assert 'n0["dispatch"] -.-> n1["worker"]' in diagram + + +def test_every_cluster_is_closed_and_labels_survive_the_kernel_restatement(tmp_path): + """Two regressions pinned at once: a global dedup used to collapse every + cluster's identical `end` into one (invalid Mermaid on every multi-round + diagram), and the kernel's later topology event — which carries no + `round` — used to overwrite the loop's labelled one.""" + trace = TraceRecorder(tmp_path / "t.jsonl") + for round_no, graph in ((1, "plan:aaa"), (2, "plan:bbb")): + # The loop's statement, with the round label... + trace.event( + run_id="r1", graph=graph, node="topology", phase="topology", step=0, + state_delta={ + "nodes": ["triage"], + "edges": [["__start__", "triage", "static"]], + "round": round_no, + }, + ) + # ...then the kernel's bare restatement at invoke, without it. + trace.event( + run_id="r1", graph=graph, node="topology", phase="topology", step=0, + state_delta={ + "nodes": ["triage"], + "edges": [["__start__", "triage", "static"]], + }, + ) + trace.event(run_id="r1", graph=graph, node="triage", phase="start", step=1) + trace.event(run_id="r1", graph=graph, node="triage", phase="end", step=1) + + diagram = to_mermaid(trace, "r1") + opened = sum(1 for line in diagram.splitlines() if line.strip().startswith("subgraph")) + closed = sum(1 for line in diagram.splitlines() if line.strip() == "end") + assert opened == closed == 2, diagram + assert 'subgraph cluster0["round 1"]' in diagram + assert 'subgraph cluster1["round 2"]' in diagram + + +def test_sentinels_in_a_hand_written_nodes_list_do_not_crash_the_render(tmp_path): + trace = TraceRecorder(tmp_path / "t.jsonl") + trace.event( + run_id="r1", graph="g", node="topology", phase="topology", step=0, + state_delta={ + "nodes": ["__start__", "a", "__end__"], + "edges": [["__start__", "a", "static"], ["a", "__end__", "static"]], + }, + ) + diagram = to_mermaid(trace, "r1") + assert "flowchart TD" in diagram and '["a"]' in diagram + + +def test_a_run_whose_planning_never_produced_a_graph_says_so(tmp_path): + """The planner's paperwork is not an orchestration. A run that never got a + proposal admitted-and-built used to render `plan → admission → round1 → + plan → …` as a linear chain — bookkeeping drawn as though it were the + graph, which is the picture every failed planning run ended on.""" + trace = TraceRecorder(tmp_path / "t.jsonl") + for rnd in (1, 2): + trace.event(run_id="r1", graph="loop", node="p:plan", phase="plan", step=rnd) + trace.event(run_id="r1", graph="loop", node="a:x", phase="admission", step=rnd) + trace.event(run_id="r1", graph="loop", node=f"loop:round{rnd}", + phase="round", step=rnd) + trace.event(run_id="r1", graph="loop", node="loop:stop", phase="stop", step=3) + + diagram = to_mermaid(trace, "r1") + assert "no graph ran" in diagram + assert "admission" not in diagram and "round" not in diagram + + +def test_an_agent_run_still_chains_its_own_steps(tmp_path): + """The bookkeeping filter must not silence a real agent's sequence.""" + trace = TraceRecorder(tmp_path / "t.jsonl") + trace.event(run_id="r1", graph="agent", node="a:model", phase="model", step=1) + trace.event(run_id="r1", graph="agent", node="a:read_file", phase="tool", step=2) + trace.event(run_id="r1", graph="agent", node="a:stop", phase="stop", step=3) + diagram = to_mermaid(trace, "r1") + assert "a:model" in diagram and "a:read_file" in diagram + + +def test_rounds_are_linked_by_the_state_that_flowed_between_them(tmp_path): + """Each round materializes standalone, so its entry really is START — + but drawn alone the clusters read as unrelated graphs sharing a page. + A dotted `state` link says what actually happened between them.""" + trace = TraceRecorder(tmp_path / "t.jsonl") + for rnd, graph in ((1, "plan:aaa"), (2, "plan:bbb")): + trace.event( + run_id="r1", graph=graph, node="topology", phase="topology", step=0, + state_delta={"nodes": ["a"], "edges": [["__start__", "a", "static"]], + "round": rnd}, + ) + diagram = to_mermaid(trace, "r1") + assert "cluster0 -.->|state| cluster1" in diagram + # And it sits outside both subgraphs, after they close. + lines = diagram.splitlines() + assert lines.index(" cluster0 -.->|state| cluster1") > max( + i for i, line in enumerate(lines) if line.strip() == "end" + ) + + +def test_a_single_round_gets_no_cluster_link(tmp_path): + trace = TraceRecorder(tmp_path / "t.jsonl") + trace.event( + run_id="r1", graph="g", node="topology", phase="topology", step=0, + state_delta={"nodes": ["a"], "edges": [["__start__", "a", "static"]]}, + ) + assert "-.->|state|" not in to_mermaid(trace, "r1")