Merge remote-tracking branch 'origin/main' into fix/live-auth-and-confinement

Author: Shashankss1205

CHANGELOG.md

@@ -16,5 +16,7 @@ Entries are newest-last within a release, matching the order they were written.
 - a bracket anywhere in a model's **prose** hijacked JSON extraction, because only the first `{`/`[` was ever tried. `Based on the context [lines 3-5]: {…}` was rejected as unparseable, and — worse — `Analysis (note [1]): {"supported": false}` returned a perfectly valid `[1]`, substituting a fabricated value for the verifier's actual answer. Every opener is tried now, and length alone turned out not to be a safe rank — a citation list like `[101, 205, 309, …]` *longer* than the verdict still won — so object spans are tried before array spans, each longest-first; junk still returns `None`, so fail-closed is unchanged.
 - a **bare backend name was read as a model name**, because `split_spec` only consulted the backend list when the spec contained a slash. `--model claude-cli` — the backend `models --check` reports as `usable` — shelled out to `claude -p --model claude-cli` and was refused by the CLI on *every* call, and `--model mock` named the paid subscription backend and spawned the real binary, so the double documented as "never reaches a provider" reached for one. A bare backend name now resolves to that backend (`claude-cli` to its own default model, `mock` to the scripted double, which ignores the model segment anyway); `openrouter`, `openai` and `ollama` front catalogues rather than a model, so those are refused with an example spelling instead of a guess about what to bill you for. The slash forms and bare *model* names are unchanged.
 - a failing `claude -p` **reported no reason at all**. The CLI exits non-zero with an empty stderr and puts its explanation in the JSON envelope on stdout; the gateway read only stderr, so the error was `claude -p exited 1: ` — a sentence that stops at the colon. Since the wrong-model bug above presented itself exactly that way, the two hid each other. stdout is read first now, and the recovered text also feeds the transient-vs-deterministic classifier, which was previously deciding from `""`.
+- **repeating `--registry` walked a Slack user straight past the agent opt-in.** The gate reads a flag's value to decide admission and argparse's `store` action then runs the *last* occurrence, but `_flag_value` returned the *first* — so `plan … --registry grapharc.examples.plan_docs:build_registry --registry grapharc.stdlib:build_registry` was judged against the demo registry and executed against the one that builds agent kinds on the host, with `GRAPHARC_SLACK_ALLOW_AGENT` never consulted and the forced `--approve` skipped in the same step. No privilege and no special knowledge needed: typing the flag twice was the whole exploit. Repeats of any admitted flag are refused outright now — the fail-closed reading, which retires the entire first-vs-last family rather than the one flag that exposed it — with a carve-out for the options the CLI itself accumulates (`agent --allow`/`--deny`, argparse `action="append"`), where every occurrence reaches the run and nothing can diverge. A duplicated `--model` is refused on the same rule, opted in or not, and `_flag_value` reads the last occurrence regardless, so the two readers can no longer disagree. A sweep over the whole allowlist asserts the duplicated form of every gated flag, so a future gate cannot reopen the gap.
+- a **NUL byte in a path came back as silence**, the worst answer a chat bot can give: `Path(raw).resolve()` raises `ValueError`, `handle_text_live` catches only `SlackCommandError`, so `trace a\x00b` escaped the bolt listener as an unhandled exception and the requester saw no reply at all — indistinguishable from the bot being down. A NUL anywhere in the request is now a refusal in the same voice the core tools already use ("cannot name a file"), and `_confined` turns any `ValueError`/`OSError` out of the filesystem into a refusal too, for callers of its own. Folded in from the same report: the flag allowlist tested `token.startswith("--")`, so a single-dash token slipped it and was spent as a positional — `trace -h` was admitted with `-h` as the path. Any leading dash is a flag now, and one not on the list is refused like any other.
 - the `/live` **token check crashed on the strangers it exists to refuse**. `secrets.compare_digest` rejects `str` outside ASCII, and `_authorized` handed it the raw query parameter, so `?token=café` raised `TypeError` through the handler: an unauthenticated 500 with a traceback in the log on all four `/live` routes, where every ASCII guess correctly got a 401. The 500-vs-401 split was itself an oracle about how the token is compared. Both sides are encoded to UTF-8 now, which drops the ASCII restriction and keeps the constant-time comparison that is the whole reason `compare_digest` is there. A NUL byte in `?trace=` was the same shape one function over — `resolve_trace` raises `ValueError`, not the `LivePathError` the route caught — and is a 404 like any other malformed path now.
 - the `/live` **index advertised traces the reader refuses to serve**. `scan_traces` walked the live root with `rglob("*.jsonl")`, which matches a symlinked file by name, then parsed it and published its name, size, mtime and **run ids** on `GET /live/api/runs` and the HTML index — for a file outside the root that `/live/api/stream` then 404s, the 404 being the proof of intent. One contract, two code paths, and only the reader enforced it; the live root is documented as the Slack bot's working directory, i.e. somewhere other things write. `scan_traces` routes every candidate through `resolve_trace` now and skips symlinks outright, so a refactor of either check cannot reopen the leak. The reader's confinement — `../`, `%2e%2e%2f`, absolute paths, `sub/../../`, symlinked directories — is unchanged.

README.md

@@ -482,7 +482,7 @@ Re-derived on 2026-07-28 by running each item, not by reading the commit log.
 - **Cost is recorded when a backend reports one, estimated when it does not.** Both gateways publish the provider's `cost_usd` through the same `llm_output` envelope, the runtime's usage callback writes it onto the node's `end` event, and an agent's `model` events carry the per-call breakdown. A backend that reports no price still falls back to a `RateCard` estimate, and the two figures stay apart — `recorded_cost_usd` is never a guess. Still missing: no tenant on a trace event, so per-tenant attribution is not offered.
 - **A node's tokens are its own, not the run's movement while it ran.** Worth stating because it was the other way round: an `end` event carried the difference between two readings of the run's *shared* meter, so under fan-out the workers' windows overlapped and each was credited with its siblings' concurrent spend. Three workers costing 8 tokens each traced as 24/16/8, and `metrics` and `cost` agreed on 48 for 24 tokens of real work — doubling the estimated bill purely because the work ran in parallel. Attribution now comes from a per-node scope on the meter, so the same work costs the same serially and in parallel; a hand charge the usage callback never saw still lands on the node that made it.
 - **A planning round is an envelope, not a measurement.** A `round` event used to carry the planner's `tokens` and the round's `duration_ms`, both of which `metrics`, `cost` and `replay` add on top of node totals — and the planner's spend was already reported by its own `plan` event, so it was counted twice, and a round's duration encloses the plan plus every node it ran. Neither is on the event now; both are on its `state_delta` as `round_tokens` / `round_iterations` / `round_duration_ms`, where no reader sums them. `RoundRecord.iterations` also holds a figure now rather than always `0`.
-- **The Claude CLI backend is completion-only.** Tool calling and structured output need one of the OpenAI-wire backends: `openrouter`, `openai`, or a local `ollama`.
+- **The Claude CLI backend is completion-only, and an agent node on it is *delegated* rather than governed.** The CLI has no tool-calling wire format, so GraphARC cannot run its own gated loop over it. Rather than refuse, `AgentNode` hands the whole loop to Claude Code's headless agent — which means every tool Claude Code has, under its `bypassPermissions` mode: those calls are not checked by this graph's permission policy, not confined by the sandbox executor, and the token figure is the sub-agent's own rather than one GraphARC metered call by call. The workspace boundary and the wall-clock ceiling still hold. It warns on `DelegatedToolUseWarning` at construction and marks every trace event `executor=delegated`, so a run stays auditable as delegated; filter that warning to an error to get the old refusal back. Structured output still needs an OpenAI-wire backend: `openrouter`, `openai`, or a local `ollama`.
 - **A session turn is synchronous**, and a runner claim is a claim rather than a lease — nothing reclaims a session whose runner died holding it.
 - **`.env` is found by walking up parent directories; `grapharc.toml` is not.** The config layer refuses an upward search on purpose — a run must not be governed by a file you did not know about. The credential loader predates that decision and still searches upward, so the thing that *spends money* is discovered more eagerly than the thing that *constrains* it.
 - **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost.

docs/cookbook/07-slack.md

@@ -27,6 +27,7 @@ an afterthought. The defaults:
 | The budget, policy and trace flags each command already has | `--registry` (imports an arbitrary module), `--config`, `--json`, `--no-color` |
 | `plan --registry`, for exactly the two registries the package ships | any other `--registry` value |
 | `agent`, only behind the double opt-in below | `--model` / `--reviewer-model`, unless the operator opts in |
+| Each admitted flag, once; `agent --allow`/`--deny` accumulate as the CLI does | The same flag twice (`--registry <demo> --registry <stdlib>`), because the gate would judge one occurrence and the CLI would run the other |
 
 With `--model` off, every reachable command runs the scripted, spend-free
 path. The default answer to "can someone in Slack cost me money?" is **no**;

grapharc/cli/config.py

@@ -161,8 +161,12 @@ def load(explicit: Path | None = None, *, cwd: Path | None = None) -> Settings:
             return Settings()
 
     try:
+        # `UnicodeDecodeError` is a `ValueError`, so it belongs in this tuple
+        # explicitly: without it a stray binary `grapharc.toml` in the working
+        # directory tracebacks out of every configurable command, because this
+        # file is picked up implicitly rather than named by the operator.
         document = tomllib.loads(path.read_text(encoding="utf-8"))
-    except (OSError, tomllib.TOMLDecodeError) as exc:
+    except (OSError, UnicodeDecodeError, tomllib.TOMLDecodeError) as exc:
         raise ConfigError(f"{path}: {exc}") from exc
 
     table = document.get(TABLE, document)

grapharc/cli/delegate.py

@@ -25,6 +25,7 @@
 import shutil
 import subprocess
 import uuid
+from dataclasses import dataclass
 from pathlib import Path
 
 from grapharc.cli import style
@@ -35,6 +36,130 @@
 DEFAULT_DELEGATED_TOOLS = ("Read", "Glob", "Grep", "LS", "Edit", "Write", "Bash")
 
 
+
+# ---------------------------------------------------------------------------
+# The reusable core. `run_delegated` below is the CLI's presentation of it, and
+# `grapharc.harness.agent.AgentNode` calls it directly when its backend is the
+# Claude CLI — one implementation, so the two paths cannot drift on what
+# actually gets spawned.
+
+
+class DelegationError(Exception):
+    """A delegated run could not be started, or came back unreadable.
+
+    `reason` is the machine-readable form, written to the trace so a run that
+    failed here is distinguishable afterwards from one that ran and refused.
+    """
+
+    def __init__(self, message: str, *, reason: str) -> None:
+        super().__init__(message)
+        self.reason = reason
+
+
+@dataclass(frozen=True)
+class DelegatedRun:
+    """What Claude Code reported back. Every number here is *its* figure.
+
+    `tokens_reported` is named for what it is: the sub-agent's own count, not
+    something GraphARC metered call by call. Nothing in this object was
+    observed by the permission engine.
+    """
+
+    ok: bool
+    answer: str
+    reason: str
+    turns: int
+    tokens_reported: int
+    cost_usd: float | None
+    session_id: str | None
+    allowed: list[str] | None  # None == no --allowedTools restriction at all
+    denied: list[str]
+
+
+def delegate_task(
+    task: str,
+    *,
+    workspace: Path,
+    model: str | None = None,
+    allow: list[str] | None = None,
+    deny: list[str] | None = None,
+    max_turns: int = 20,
+    max_seconds: float | None = None,
+    system_prompt: str | None = None,
+    permission_mode: str | None = None,
+) -> DelegatedRun:
+    """Run one headless `claude -p` agent loop in `workspace`.
+
+    Two separate axes, and conflating them is a trap worth naming. `allow`
+    controls *which* tools exist; `permission_mode` controls whether the ones
+    that mutate anything are allowed to run without a human answering a prompt.
+    Omitting `--allowedTools` does **not** mean "every tool": it means Claude
+    Code's own default gating, and headless there is nobody to approve a Write,
+    so the sub-agent reports back that it could not create the file. Measured,
+    not assumed. `permission_mode="bypassPermissions"` is what actually means
+    "everything", and it means it literally — no checks at all.
+
+    Either way the caller is responsible for having said so out loud;
+    `AgentNode` warns at construction and marks every trace event.
+    """
+    binary = shutil.which("claude")
+    if binary is None:
+        raise DelegationError(
+            "the delegated executor shells out to `claude`, which is not on PATH; "
+            "install Claude Code or use a tool-calling backend",
+            reason="claude_not_found",
+        )
+
+    workspace = Path(workspace).expanduser().resolve()
+    workspace.mkdir(parents=True, exist_ok=True)
+
+    argv = [binary, "-p", task, "--output-format", "json", "--max-turns", str(max_turns)]
+    if allow is not None:
+        argv += ["--allowedTools", ",".join(allow)]
+    if deny:
+        argv += ["--disallowedTools", ",".join(deny)]
+    if permission_mode:
+        argv += ["--permission-mode", permission_mode]
+    if model:
+        argv += ["--model", model]
+    if system_prompt:
+        argv += ["--append-system-prompt", system_prompt]
+
+    try:
+        completed = subprocess.run(
+            argv, cwd=workspace, capture_output=True, text=True, timeout=max_seconds
+        )
+    except subprocess.TimeoutExpired as exc:
+        raise DelegationError(
+            f"max_seconds ({max_seconds}) reached; the delegated run was stopped",
+            reason="deadline_exceeded",
+        ) from exc
+
+    try:
+        report = json.loads(completed.stdout)
+    except (json.JSONDecodeError, ValueError) as exc:
+        detail = (completed.stderr or completed.stdout or "").strip()[-500:]
+        raise DelegationError(
+            f"claude exited {completed.returncode} without a readable JSON report: {detail}",
+            reason="unreadable_report",
+        ) from exc
+
+    usage = report.get("usage") or {}
+    met = report.get("subtype") == "success" and not report.get("is_error", False)
+    return DelegatedRun(
+        ok=met,
+        answer=str(report.get("result") or "").strip(),
+        reason="target_met" if met else str(report.get("subtype") or "error"),
+        turns=int(report.get("num_turns") or 0),
+        tokens_reported=int(usage.get("input_tokens") or 0)
+        + int(usage.get("output_tokens") or 0),
+        cost_usd=report.get("total_cost_usd"),
+        session_id=report.get("session_id"),
+        allowed=list(allow) if allow is not None else None,
+        denied=list(deny or []),
+    )
+
+
 def run_delegated(
     task: str,
     *,
@@ -217,4 +342,10 @@ def run_delegated(
     return EXIT_OK if met else EXIT_FAILED
 
 
-__all__ = ["DEFAULT_DELEGATED_TOOLS", "run_delegated"]
+__all__ = [
+    "DEFAULT_DELEGATED_TOOLS",
+    "DelegatedRun",
+    "DelegationError",
+    "delegate_task",
+    "run_delegated",
+]

grapharc/cli/graphrun.py

@@ -70,7 +70,13 @@ def load_topology(path: Path) -> dict[str, Any]:
     """
     if not path.is_file():
         raise PlanSetupError(f"no such graph file: {path}")
-    text = path.read_text(encoding="utf-8")
+    try:
+        text = path.read_text(encoding="utf-8")
+    except (OSError, UnicodeDecodeError) as exc:
+        # A topology saved as UTF-16, or truncated in transit, is a file we
+        # cannot run — not a crash. `UnicodeDecodeError` is a `ValueError`, so
+        # neither decoder below would ever have caught it.
+        raise PlanSetupError(f"{path}: {exc}") from exc
     try:
         if path.suffix.lower() == ".toml":
             return tomllib.loads(text)

grapharc/cli/main.py

@@ -242,9 +242,23 @@ def _existing_trace(path: Path, *, command: str, as_json: bool) -> TraceRecorder
     Checked before constructing the recorder because `TraceRecorder.__init__`
     creates the parent directory: a typo in a read-only command should not leave
     a directory behind.
+
+    Existence is not enough. A directory, a file whose permissions forbid the
+    read, or any other `OSError` used to escape as a traceback with exit 1,
+    because the handlers below catch only `TraceReadError` — so the file is
+    opened here, where the failure is still reportable as the exit-2 document
+    the contract promises.
     """
     if not path.exists():
         return fail(f"no such trace file: {path}", as_json=as_json, command=command)
+    try:
+        path.open("rb").close()
+    except OSError as exc:
+        return fail(
+            f"unreadable trace file: {path}: {exc.strerror or exc}",
+            as_json=as_json,
+            command=command,
+        )
     return TraceRecorder(path)
 
 

grapharc/cli/plan.py

@@ -251,6 +251,10 @@ def plan(
         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.
+            # Silent in JSON mode: stdout there carries exactly one document, and
+            # a notice printed ahead of it makes the whole output unparseable.
+            if as_json:
+                return
             print(message, flush=True, file=sys.stdout)
 
         approval = file_approval(