diff --git a/Cargo.toml b/Cargo.toml index f0e05bf..c5368c7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -146,6 +146,7 @@ ctor = "1.0.13" pretty_assertions = "1.4.0" rmcp = { version = "3.1.2", features = ["server"] } serial_test = "3" +tokio = { version = "1.34.0", features = ["test-util"] } [[bin]] name = "coyote" diff --git a/assets/agents/adversary/README.md b/assets/agents/adversary/README.md index 807bae9..4b751d1 100644 --- a/assets/agents/adversary/README.md +++ b/assets/agents/adversary/README.md @@ -6,8 +6,9 @@ > is structurally impossible. Each verdict passes a deterministic contract gate (MET requires > evidence including a proving test; no test = at best PARTIAL), a holistic ADDITIVE-only pass hunts > absence/scope-drift/substitution/gamed-tests, and the CONFORMS/DIVERGES sentinel is computed by a -> script (missing plan = DIVERGES, fail-closed). The conformance doctrine below is unchanged — now -> structurally enforced. +> script (missing plan = DIVERGES, fail-closed). The conformance doctrine below is structurally +> enforced: CONFORMS requires ALL MET + zero holistic complaints + zero pipeline faults + +> zero red verification runs (exit≠0/timeout/skipped, or a runner error when commands were declared). An **adversarial plan-conformance reviewer**. Where [`code-reviewer`](../code-reviewer/README.md) @@ -41,6 +42,9 @@ ADVERSARIAL_REVIEW: CONFORMS Criteria: N/N met (all with tests). ``` +CONFORMS is deterministic: ALL MET + zero holistic complaints + zero pipeline faults + +zero red verification runs (exit≠0/timeout/skipped, or a runner error when commands were declared). + ``` ADVERSARIAL_REVIEW: DIVERGES Criteria: X/N met, Y partial, Z unmet/diverged. @@ -49,6 +53,12 @@ Complaints: 2. ... ``` +Every verdict — CONFORMS or DIVERGES — ends with a `Verification runs:` section: the recorded +PASS/FAIL results (command, exit code, duration, failure tail) of the caller-declared verification +commands, or an explicit marker when none were declared or the run never reached that stage. The +one exception is the verdict script's own crash guard, which emits a bare DIVERGES with a +"verdict computation error" line — and no section — when verdict computation itself dies. + A `DIVERGES` verdict **blocks** completion. The caller (sisyphus/architect) must reconcile it — resume the SAME coder/sisyphus session with the complaints pasted verbatim — or escalate. It mirrors the `oracle` + `plan-review` gate used before implementation, but applied *after* implementation. @@ -60,28 +70,41 @@ violation) and cites `file:line`. Vague complaints are not emitted. ```mermaid flowchart TD - A["parse (llm): extract acceptance criteria"] --> B["diff_facts (script): resolve + bound the diff"] - B --> C["map over criteria: one fresh-context branch each (parallel)"] + A["parse (llm): extract acceptance criteria"] --> B["facts (diff_facts.py): resolve + bound the diff"] + V["verification_commands (declared graph variable — never parsed from the prompt)"] --> R + B --> R["run_checks (script): run the declared verification commands ONCE, record results"] + R --> C["map over criteria: one fresh-context branch each (parallel)"] C --> D["check_criterion (llm + fs/ast tools)"] D --> E["crit_gate (script): MET needs change+test evidence"] E -. "reject-retry once; exhausted = PARTIAL" .-> D E --> F["holistic (llm): ADDITIVE-only hunt - absence, scope drift, substitution, gamed tests"] - F --> G["verdict (script): CONFORMS iff ALL MET + zero extras; no criteria = DIVERGES"] + F --> G["verdict (script): CONFORMS iff ALL MET + zero extras + zero faults + zero red runs; no criteria = DIVERGES"] G --> H(["ADVERSARIAL_REVIEW: CONFORMS | DIVERGES"]) + A -. "fallback" .-> P["pipeline_fault (script): record PIPELINE-FAULT marker"] + B -. "fallback" .-> P + R -. "fallback" .-> P + D -. "fallback" .-> CF["criterion_fault (script): UNMET + PIPELINE-FAULT evidence for that ONE criterion"] + CF -. "map join" .-> F + F -. "fallback" .-> G + P --> G ``` The conformance doctrine is baked into the graph's own nodes: 1. **Every** acceptance criterion gets its own fresh-context verification branch — MET / PARTIAL / UNMET / DIVERGED, machine-gated (MET requires the satisfying change AND the proving test, cited). No test ⇒ at best PARTIAL. Partial coverage of the criteria list is structurally impossible. 2. Ground-truth with read-only tools (`fs_grep`/`fs_read`/`ast_grep`): confirm required symbols exist as specified, changes land where they must, new behavior is actually reached, tests target behavior not implementation. -3. Hunt adversarially for the **absent**: skipped criteria, scope creep, interface/approach substitution, out-of-scope touches, downstream contract breakage. +3. Execution evidence is recorded **once, deterministically**: `run_checks` runs ONLY the caller-declared verification commands (never invented or auto-detected), sequentially under a 3300s total deadline — commands the deadline leaves no budget for are recorded as SKIPPED. Criterion branches have no execution tools by design: "tests pass"-style criteria are judged against the record — a recorded green run is MET citing it; no declared command ⇒ at best PARTIAL. Independently of the criterion judgments, the verdict script blocks CONFORMS on any red run (exit≠0/timeout/skipped) and on an ENVIRONMENT runner-error marker when commands were declared — the declared commands never ran, so the run is unproven. The commands arrive through the declared `verification_commands` graph variable (see [Declaring verification commands](#declaring-verification-commands)) — `parse` never extracts them from the prompt, so the prompt is no longer an extraction source for the commands. +4. Hunt adversarially for the **absent**: skipped criteria, scope creep, interface/approach substitution, out-of-scope touches, downstream contract breakage. +5. **Fail closed on pipeline faults**: if parse, diff resolution, or the verification runner dies, its fallback routes to `pipeline_fault`, which records a `PIPELINE-FAULT:` marker and continues to the verdict (a failed holistic pass falls back straight to the verdict, which synthesizes the same marker). Any recorded fault forces DIVERGES with the fault as complaint #1 — degraded is never passing, and the sentinel is always emitted: the graph never dies without one. +6. **Fail closed per criterion**: a criterion check that dies (max_iterations exhausted, API failure) falls back to `criterion_fault`, which records a schema-shaped UNMET verdict carrying a `PIPELINE-FAULT: criterion ` evidence marker for that one criterion. The other criteria still get verdicts, the report says the check DIED rather than judged, and the run is DIVERGES. That exact marker prefix (the one `criterion_fault` emits) forces the criterion into unmet regardless of the status it carries, so a verdict that echoes it with `MET` can never count toward CONFORMS; `check_criterion` is told never to begin its own evidence with `PIPELINE-FAULT:`. It is **read-only** — it produces a verdict, never a fix. ## Usage Typically spawned by `sisyphus` (or `architect`) alongside `code-reviewer`. The spawn prompt IS its -entire context, so it must include the diff (or a base ref to fetch) **and** the acceptance criteria: +entire context, so it must include the diff (or a base ref to fetch) **and** the acceptance criteria. +Verification commands travel separately, as a declared variable: ```sh agent__spawn --agent adversary --prompt " @@ -93,19 +116,41 @@ Run get_diff (or --base main), or: ## PLAN — acceptance criteria to check against -" +" --variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"} ``` Direct invocation for ad-hoc use: ```sh coyote -a adversary --agent-variable project_dir /path/to/repo \ + --agent-variable verification_commands '["cargo test --all"]' \ "Review staged changes against these criteria: " ``` +### Declaring verification commands + +`verification_commands` is a declared graph variable (default `'[]'`): a JSON array of shell +commands, passed as a string because agent variables always arrive as strings. `run_checks` runs +exactly those commands with a shell — so the declaration is a **trust boundary**. The prompt is the +wrong side of it: it also carries plan and diff text pasted from the repo under review, and an LLM +`parse` node extracting commands from that prose would let anything in the repo's text run under +the reviewer's shell. Hence the commands are never in the prompt and never in `parse`'s output +schema; `review-gauntlet` forwards its own `verification_commands` variable to the adversary lane +as a structured `inputs:` passthrough, not as prompt text. + +- `'[]'` (or omitted) ⇒ the "none declared" marker; execution criteria are at best PARTIAL. +- Anything that is not a JSON array of strings (unparsable JSON, an object, non-string items) is + an invalid declaration: `run_checks` executes **nothing** and records a + `PIPELINE-FAULT: verification_commands declaration invalid — …` marker, which forces DIVERGES: + it fails closed as a PIPELINE-FAULT (complaint #1) — it never degrades to an ENVIRONMENT marker. +- The engine merges only the keys declared under `parse`'s `output_schema.properties` into state, + so an injected instruction that makes the parse LLM emit an extra `verification_commands` key + cannot reach state: the undeclared key is dropped and the declared variable stands. + ### Tools - The graph's `diff_facts` script resolves the diff itself (staged → unstaged → `HEAD~1`, or an explicit ref/range named in the prompt) — there is no `get_diff` tool anymore. +- The `run_checks` script executes the commands declared via the `verification_commands` variable and records the results — criterion branches cite the record and never run anything themselves. - Criterion branches carry read-only `fs_read`/`fs_cat`/`fs_grep`/`ast_grep` for ground-truth checks. ## Related diff --git a/assets/agents/adversary/graph.yaml b/assets/agents/adversary/graph.yaml index 9ad6001..a1d63e4 100644 --- a/assets/agents/adversary/graph.yaml +++ b/assets/agents/adversary/graph.yaml @@ -11,8 +11,15 @@ description: | silently-overridden recorded decisions, out-of-scope touches, and gamed tests — ADDITIVE-only: it can add complaints, never flip a criterion verdict. The final CONFORMS/DIVERGES sentinel is computed by a script: CONFORMS - requires every criterion MET and zero holistic complaints. Missing plan = - DIVERGES (fail-closed) naming the missing spec. + requires every criterion MET, zero holistic complaints, zero pipeline faults, + and zero red verification runs (exit≠0/timeout/skipped, or a runner error + when commands were declared). Missing plan = DIVERGES (fail-closed) naming + the missing spec. Caller-declared verification + commands are executed ONCE by a deterministic stage and recorded — criterion + branches cite the record and never run anything themselves. Any pipeline + fault (parse, diff resolution, verification runner, or holistic failure) + lands a PIPELINE-FAULT marker and fails closed into DIVERGES: degraded is + never passing. version: '1.0' @@ -26,11 +33,22 @@ variables: - name: project_dir description: Project directory containing the changes under review default: '.' + - name: verification_commands + description: | + JSON array of shell commands the CALLER declares for the deterministic + run_checks stage — e.g. '["cargo test --all"]'. Never inferred from the + prompt; empty array = none declared. + default: '[]' settings: max_loop_iterations: 10 log_state_snapshots: false - timeout: 5400 + # 3600 (run_checks node timeout, worst case) + 5400 (the previous + # whole-graph budget, kept as the envelope for every stage other than + # run_checks: parse/facts before it, criteria map/holistic/verdict after) + # = 9000. A graph timeout kills from outside — no fallback fires and the + # sentinel is lost. + timeout: 9000 initial_state: project_dir_in: '' @@ -42,12 +60,16 @@ initial_state: changed_files: [] diff_text: '' diff_note: '' + exec_results: '' crit_verdicts: [] extra_complaints: [] observations: '' adv_report: '' gate_feedback: '' gate_attempts: 0 + parse_failure: '' + holistic_failure: '' + pipeline_faults: [] start: parse @@ -106,6 +128,9 @@ nodes: required: [criteria, diff_spec, plan_notes, scope_notes, extra_context, project_dir_in] max_iterations: 3 max_attempts: 2 + state_updates: + parse_failure: '{{output}}' + fallback: pipeline_fault next: facts facts: @@ -114,6 +139,16 @@ nodes: description: Deterministic diff resolution — changed files and the (bounded) diff text every criterion branch verifies against. script: scripts/diff_facts.py timeout: 120 + fallback: pipeline_fault + next: run_checks + + run_checks: + id: run_checks + type: script + description: Deterministic execution evidence — runs ONLY the commands declared via the verification_commands variable in the project dir and records exec_results; criterion branches cite the record and never execute anything. Undeclared commands degrade to a "none declared" marker, runner errors to an ENVIRONMENT marker (criterion branches judge it PARTIAL/unproven; the verdict additionally blocks CONFORMS when commands were declared); a declaration that is not a JSON array of strings executes nothing and records a PIPELINE-FAULT (fail-closed); a 3300s total deadline keeps the script under the node timeout (commands past it are recorded as skipped), and if the node is killed anyway the fallback records a PIPELINE-FAULT. + script: scripts/run_checks.py + timeout: 3600 + fallback: pipeline_fault next: map_criteria map_criteria: @@ -130,7 +165,7 @@ nodes: check_criterion: id: check_criterion type: llm - description: Fresh-context verification of ONE acceptance criterion against the diff and the working tree. + description: Fresh-context verification of ONE acceptance criterion against the diff and the working tree (ENVIRONMENT/SKIPPED runs judge PARTIAL here; the deterministic verdict blocks CONFORMS when declared commands did not run). instructions: | You verify ONE acceptance criterion against a code change. Fresh context, one criterion, full suspicion: assume the implementer drifted, cut a @@ -158,6 +193,25 @@ nodes: values shaped to the criterion's example, TODO/stub presented as done — those are PARTIAL or DIVERGED with the gaming named in the complaint. + Execution criteria ("tests pass", "lint clean", "build succeeds") are + judged ONLY against the recorded verification runs in your prompt — you + have no execution tools BY DESIGN and must never claim to have run + anything yourself: + - a recorded green run (exit 0) of a command covering the criterion is + the proving evidence — MET, citing that run (the command + exit 0). + - a recorded failing run covering the criterion ⇒ UNMET, citing the + recorded exit code and output tail. + - "none declared" or an ENVIRONMENT marker ⇒ at best PARTIAL, with an + ENVIRONMENT note in the complaint naming the exact command the caller + should declare via the `verification_commands` variable (structured + input, e.g. "cargo test --all") — commands are never declared in the + prompt. + - a SKIPPED record (the runner's total deadline expired before that + command ran) ⇒ at best PARTIAL for the criteria it covers, same + treatment as an ENVIRONMENT marker: unproven at the criterion level. + The deterministic verdict additionally blocks CONFORMS when declared + commands did not run — that is its job, not yours. + Output format — respond with ONLY this JSON object, no prose around it: {"id": "", "status": "MET" | "PARTIAL" | "UNMET" | "DIVERGED", @@ -169,6 +223,10 @@ nodes: Your response is machine-validated: a malformed response is rejected back to you exactly once with the reason, then recorded as PARTIAL (unproven). The id is stamped from the criterion itself — your echo is informative only. + You have a bounded tool budget — prefer fs_grep and line-ranged fs_read over + whole-file reads, and conclude as soon as the evidence is decisive. + Never begin `evidence` with the literal `PIPELINE-FAULT:` — that prefix is + reserved for the graph's own fault markers. prompt: | Repository: {{project_dir}} Changed files: {{changed_files}} @@ -181,16 +239,20 @@ nodes: The diff{{diff_note}}: {{diff_text}} + + Verification runs (recorded by the deterministic run_checks stage): + {{exec_results}} {{gate_feedback}} tools: - fs_read - fs_cat - fs_grep - ast_grep - max_iterations: 10 + max_iterations: 30 max_attempts: 2 state_updates: crit_verdict: '{{output}}' + fallback: criterion_fault next: crit_gate crit_gate: @@ -200,6 +262,13 @@ nodes: script: scripts/crit_gate.py timeout: 30 + criterion_fault: + id: criterion_fault + type: script + description: Fail-closed per-criterion fault marker (reached only via check_criterion's fallback) — converts the captured "LLM node failed" text into a schema-shaped UNMET verdict carrying a PIPELINE-FAULT evidence marker, so one dead criterion check can never sink the map or read as met. + script: scripts/criterion_fault.py + timeout: 30 + holistic: id: holistic type: llm @@ -271,14 +340,25 @@ nodes: observations: type: string required: [extra_complaints, observations] - max_iterations: 10 + max_iterations: 20 max_attempts: 2 + state_updates: + holistic_failure: '{{output}}' + fallback: verdict + next: verdict + + pipeline_fault: + id: pipeline_fault + type: script + description: Fail-closed fault marker (reached only via fallback routes from parse/facts/run_checks) — records a PIPELINE-FAULT in state and continues to verdict, which turns any fault into DIVERGES. The graph never dies without its sentinel. + script: scripts/pipeline_fault.py + timeout: 30 next: verdict verdict: id: verdict type: script - description: Deterministic verdict — CONFORMS requires every criterion MET and zero holistic complaints; no criteria at all fails closed. Assembles the exact sentinel format. + description: Deterministic verdict — CONFORMS requires every criterion MET, zero holistic complaints, zero pipeline faults, and zero recorded red verification runs (exit≠0/timeout/skipped, or an ENVIRONMENT runner-error marker when commands were declared); no criteria at all fails closed and any recorded PIPELINE-FAULT forces DIVERGES (degraded is never passing). Assembles the exact sentinel format. script: scripts/verdict.py timeout: 30 next: done diff --git a/assets/agents/adversary/scripts/criterion_fault.py b/assets/agents/adversary/scripts/criterion_fault.py new file mode 100644 index 0000000..952d1cb --- /dev/null +++ b/assets/agents/adversary/scripts/criterion_fault.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python3 +"""Fail-closed per-criterion fault marker for the adversary graph. + +Reached ONLY via check_criterion's `fallback:` route, after the engine has +written its "LLM node …failed: " text into `crit_verdict` through +state_updates. Converts that text into a schema-shaped UNMET verdict whose +evidence carries a "PIPELINE-FAULT:" marker, so one criterion check that dies +(max_iterations exhausted, API failure) is recorded as DIED rather than +judged, the other criteria still get verdicts, and the run is DIVERGES. The +map collects it like any other verdict — the branch never sinks the map. The +engine applies state_updates before routing to the fallback (src/graph/llm.rs: +`apply_state_updates_with_output`, then `outcome_from`), so the "no failure +text captured" branch is belt-and-braces rather than an expected path. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def criterion_fields(state): + c = state.get("criterion") + if isinstance(c, dict): + cid = c.get("id") + text = c.get("text") + return ( + cid.strip() if isinstance(cid, str) and cid.strip() else "unknown", + text.strip() if isinstance(text, str) else "", + ) + return "unknown", "" + + +def failure_detail(state): + captured = state.get("crit_verdict") + detail = captured.strip() if isinstance(captured, str) else "" + if not detail.startswith("LLM node"): + return "no failure text captured" + detail = detail.replace("\n", " ") + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + return detail + + +def main(): + state = load_state() + cid, ctext = criterion_fields(state) + detail = failure_detail(state) + print( + json.dumps( + { + "crit_verdict": { + "id": cid, + "text": ctext, + "status": "UNMET", + "evidence": f"PIPELINE-FAULT: criterion check failed — {detail}", + "complaint": ( + f"criterion check DIED (pipeline fault) — {detail}; the " + "criterion was NOT verified and is treated as unmet " + "(fail-closed)" + ), + } + } + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print( + json.dumps( + { + "crit_verdict": { + "id": "unknown", + "text": "", + "status": "UNMET", + "evidence": f"PIPELINE-FAULT: criterion fault-marker script error: {e}", + "complaint": ( + f"criterion check DIED (pipeline fault) — fault-marker script " + f"error: {e}; the criterion was NOT verified and is treated as " + "unmet (fail-closed)" + ), + } + } + ) + ) diff --git a/assets/agents/adversary/scripts/pipeline_fault.py b/assets/agents/adversary/scripts/pipeline_fault.py new file mode 100644 index 0000000..dddabfb --- /dev/null +++ b/assets/agents/adversary/scripts/pipeline_fault.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +"""Fail-closed fault marker for the adversary graph. + +Reached ONLY via `fallback:` routes (parse, facts, run_checks). Appends a +machine-readable "PIPELINE-FAULT:" marker to `pipeline_faults` and continues +to `verdict`, which turns any recorded fault into DIVERGES — the graph never +dies without its sentinel, and a degraded run can never read as CONFORMS. + +Stage attribution — grounded in engine behavior: a SCRIPT node's fallback +route writes NOTHING into state (src/graph/executor.rs script arm routes to +the fallback without recording the error), so the two script stages are +discriminated by which stage outputs are present: +- parse (llm): its `state_updates` wrote an "LLM node …failed: " + string into `parse_failure` — attribute to parse. +- run_checks vs facts (scripts): diff_facts.py unconditionally writes a + NONEMPTY `diff_text` (even its internal-failure path writes + "(diff unavailable)"), while the graph's initial value is ''. A nonempty + `diff_text` proves facts completed, so the fault is the verification + runner (run_checks killed at the node level, e.g. by the node timeout); + an empty `diff_text` means facts itself died. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + parse_failure = state.get("parse_failure") + detail = parse_failure.strip() if isinstance(parse_failure, str) else "" + diff_text = state.get("diff_text") + facts_completed = isinstance(diff_text, str) and bool(diff_text.strip()) + if detail.startswith("LLM node"): + detail = detail.replace("\n", " ") + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + faults.append(f"PIPELINE-FAULT: parse failed — cannot review: {detail}") + elif facts_completed: + faults.append( + "PIPELINE-FAULT: verification runner killed — the run_checks stage " + "died at the node level before recording exec_results; execution " + "criteria are unproven" + ) + else: + faults.append( + "PIPELINE-FAULT: diff resolution failed — changed files and diff text " + "could not be resolved; criterion verification never ran" + ) + print(json.dumps({"pipeline_faults": faults})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print( + json.dumps( + { + "pipeline_faults": [ + f"PIPELINE-FAULT: fault-marker script error: {e} — treat the " + "run as degraded" + ] + } + ) + ) diff --git a/assets/agents/adversary/scripts/run_checks.py b/assets/agents/adversary/scripts/run_checks.py new file mode 100644 index 0000000..299132b --- /dev/null +++ b/assets/agents/adversary/scripts/run_checks.py @@ -0,0 +1,207 @@ +#!/usr/bin/env python3 +"""Deterministic execution-evidence recorder for the adversary graph. + +Runs ONLY the caller-declared verification commands (verification_commands) +in the project dir and records the outcomes as `exec_results` — a list of +{cmd, exit, duration_s, tail} records. Criterion branches cite this record; +they never execute anything themselves. + +NO auto-detection: undeclared commands mean a "none declared" marker +(fail-visible, not fail-guessed). Any runner error degrades into an +ENVIRONMENT marker — this script never fails the node. + +Trust boundary: `verification_commands` is a DECLARED graph variable (or the +review-gauntlet's structured `inputs:` passthrough), never a field an LLM +extracts from the spawn prompt — that prose also carries plan/diff text pasted +from the repo under review, and whatever is declared here runs with +shell=True. Variables land in state as strings, so a JSON-encoded list is +accepted alongside a real list. A declaration that is not a JSON array of +strings is a caller contract violation, not a runner hiccup: it executes +NOTHING and fails closed as a PIPELINE-FAULT (complaint #1) — it never +degrades to an ENVIRONMENT marker. The engine merges only the keys an +llm node's `output_schema.properties` declares, so a parse LLM coaxed into +emitting an extra `verification_commands` key cannot overwrite the declared +value: the undeclared key is dropped before it reaches state. + +Budget: commands run sequentially, so TOTAL runtime is bounded by a deadline +(TOTAL_DEADLINE_SECS = 3300s, the run_checks node's 3600s timeout minus +margin). Without it, a handful of hanging commands (4 x 900s) would blow the +NODE timeout and the executor would kill this script from OUTSIDE — the +in-script degradation would never fire. Each command gets +min(PER_COMMAND_TIMEOUT_SECS, remaining budget); commands the deadline +leaves no budget for are recorded as {"cmd", "skipped": "deadline"} entries, +so the in-script handling stays authoritative. The env var +ADVERSARY_RUN_CHECKS_DEADLINE_SECS overrides the deadline (test seam). + +Process hygiene: commands run through the shell. On POSIX each command gets +its own session (start_new_session) and a timeout SIGKILLs the whole process +group, so grandchildren of a hung command cannot linger. The post-kill +reap/drain is itself bounded by DRAIN_TIMEOUT_SECS: a grandchild that escaped +the session (its own start_new_session) but still holds the stdout pipe +would otherwise block the drain until it exits. On non-POSIX platforms only +the direct shell child is killed — grandchildren of a timed-out command may +survive (documented limitation). + +Verification commands run with GRAPH_STATE* (GRAPH_STATE, GRAPH_STATE_FILE) +scrubbed from their environment: the reviewed repo's own tests may exec these very +scripts with their own GRAPH_STATE, and load_state() prefers +GRAPH_STATE_FILE, so an inherited live state file would silently win. +""" + +import json +import os +import signal +import subprocess +import time + +PER_COMMAND_TIMEOUT_SECS = 900 +TOTAL_DEADLINE_SECS = float(os.environ.get("ADVERSARY_RUN_CHECKS_DEADLINE_SECS") or 3300) +DRAIN_TIMEOUT_SECS = 10 +TAIL_LINES = 50 +MAX_TAIL_CHARS = 8000 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +class InvalidDeclaration(ValueError): + pass + + +def declared_commands(declared): + """Normalize the verification_commands declaration to a list of commands. + + Accepts a JSON list or a JSON-encoded string of one; missing/None means + none declared. Anything else raises InvalidDeclaration. + """ + if declared is None: + return [] + if isinstance(declared, str): + if not declared.strip(): + raise InvalidDeclaration("empty string (declare '[]' for none)") + try: + declared = json.loads(declared) + except json.JSONDecodeError as e: + raise InvalidDeclaration(f"not valid JSON: {e}") from e + if not isinstance(declared, list): + raise InvalidDeclaration( + f"expected a JSON array of strings, got {type(declared).__name__}" + ) + bad = [c for c in declared if not isinstance(c, str)] + if bad: + raise InvalidDeclaration(f"non-string item(s): {json.dumps(bad)}") + return [c.strip() for c in declared if c.strip()] + + +def tail_of(text): + tail = "\n".join(text.splitlines()[-TAIL_LINES:]) + if len(tail) > MAX_TAIL_CHARS: + tail = "…" + tail[-MAX_TAIL_CHARS:] + return tail + + +def run_one(cmd, proj, budget): + """Run one command, hard-bounded by `budget` seconds; return its record.""" + start = time.monotonic() + popen_kwargs = {"start_new_session": True} if os.name == "posix" else {} + env = {k: v for k, v in os.environ.items() if not k.startswith("GRAPH_STATE")} + p = subprocess.Popen( + cmd, + shell=True, + cwd=proj, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + encoding="utf-8", + errors="replace", + env=env, + **popen_kwargs, + ) + try: + out_s, err_s = p.communicate(timeout=budget) + except subprocess.TimeoutExpired: + if os.name == "posix": + try: + os.killpg(os.getpgid(p.pid), signal.SIGKILL) + except (ProcessLookupError, PermissionError): + pass + else: + p.kill() + try: + p.communicate(timeout=DRAIN_TIMEOUT_SECS) + except subprocess.TimeoutExpired: + pass + return { + "cmd": cmd, + "exit": -1, + "duration_s": round(time.monotonic() - start, 1), + "tail": f"TIMEOUT: no exit within {budget:.0f}s — recorded as a failing run", + } + combined = (out_s or "") + (("\n" + err_s) if err_s else "") + return { + "cmd": cmd, + "exit": p.returncode, + "duration_s": round(time.monotonic() - start, 1), + "tail": tail_of(combined), + } + + +def main(): + state = load_state() + try: + cmds = declared_commands(state.get("verification_commands")) + except InvalidDeclaration as e: + msg = f"PIPELINE-FAULT: verification_commands declaration invalid — {e}" + prior = state.get("pipeline_faults") + faults = [f for f in prior if isinstance(f, str)] if isinstance(prior, list) else [] + faults.append(msg) + print(json.dumps({"exec_results": msg, "pipeline_faults": faults})) + return + if not cmds: + print( + json.dumps( + { + "exec_results": ( + "none declared — the caller supplied no verification_commands; " + "execution criteria (build/test/lint) cannot be independently " + "confirmed. Declare the exact commands via the " + "verification_commands variable to get recorded runs." + ) + } + ) + ) + return + + proj = os.path.expanduser( + (state.get("project_dir") or "").strip() or "." + ) + deadline = time.monotonic() + TOTAL_DEADLINE_SECS + results = [] + for cmd in cmds: + remaining = deadline - time.monotonic() + if remaining <= 0: + results.append({"cmd": cmd, "skipped": "deadline"}) + continue + results.append(run_one(cmd, proj, min(PER_COMMAND_TIMEOUT_SECS, remaining))) + print(json.dumps({"exec_results": results})) + + +try: + main() +except Exception as e: # noqa: BLE001 — never fail the node; degrade visibly + print( + json.dumps( + { + "exec_results": ( + f"ENVIRONMENT: verification runner error: {e} — the declared " + "commands could not be executed; execution criteria are unproven " + "(PARTIAL at the criterion level; the verdict blocks CONFORMS when " + "commands were declared)" + ) + } + ) + ) diff --git a/assets/agents/adversary/scripts/verdict.py b/assets/agents/adversary/scripts/verdict.py index f604e0b..e1ec9e5 100644 --- a/assets/agents/adversary/scripts/verdict.py +++ b/assets/agents/adversary/scripts/verdict.py @@ -1,15 +1,35 @@ #!/usr/bin/env python3 """Deterministic conformance verdict for the adversary graph. -CONFORMS requires: at least one criterion, every criterion MET, and zero -holistic complaints. Everything else — including a missing plan — is -DIVERGES (fail-closed). Assembles the exact sentinel format the callers -route on. Never crashes into a silent verdict. +CONFORMS requires: at least one criterion, every criterion MET, zero +holistic complaints, zero pipeline faults, and zero recorded red +verification runs (exit≠0/timeout/skipped, or an ENVIRONMENT runner-error +marker when commands were declared — the declared commands never ran, so the +run is unproven). Everything else — including a missing plan or a degraded +(faulted) pipeline — is DIVERGES (fail-closed). +Any "PIPELINE-FAULT:" marker recorded in `pipeline_faults` (or a holistic +llm-node failure captured in `holistic_failure`) becomes complaint #1; +per-criterion results are still reported. A criterion whose check DIED +(criterion_fault's UNMET verdict, evidence prefixed DIED_EVIDENCE_PREFIX) is +rendered as died rather than judged and counted as unmet regardless of the +status it carries. Assembles the exact sentinel format the callers route on. +Never crashes into a silent verdict. """ import json import os +MAX_FAULT_DETAIL_CHARS = 300 +MAX_MARKER_CHARS = 200 +DIED_MARKER = "criterion check DIED (pipeline fault)" +DIED_EVIDENCE_PREFIX = "PIPELINE-FAULT: criterion " + + +def died(v): + evidence = v.get("evidence") + return isinstance(evidence, str) and evidence.startswith(DIED_EVIDENCE_PREFIX) + + def load_state(): if path := os.environ.get("GRAPH_STATE_FILE"): with open(path) as f: @@ -17,19 +37,194 @@ def load_state(): return json.loads(os.environ.get("GRAPH_STATE", "{}")) +def collect_pipeline_faults(state): + """Recorded fault markers, plus a holistic llm-node failure if one occurred. + + `holistic_failure` holds the "LLM node …failed: " string when the + holistic pass fell back to verdict; on a successful pass it holds the + rendered schema output (never that prefix). + """ + faults = [ + f.strip() + for f in (state.get("pipeline_faults") or []) + if isinstance(f, str) and f.strip() + ] + hf = state.get("holistic_failure") + if isinstance(hf, str) and hf.strip().startswith("LLM node"): + detail = hf.strip().replace("\n", " ") + if len(detail) > MAX_FAULT_DETAIL_CHARS: + detail = detail[: MAX_FAULT_DETAIL_CHARS - 1] + "…" + faults.append( + "PIPELINE-FAULT: holistic pass failed — criterion verdicts stand " + f"but cross-cutting hunt did not run ({detail})" + ) + return faults + + +def append_criterion_complaints(lines, verdicts, extra, start): + """Numbered complaints for non-MET verdicts and holistic extras.""" + i = start + for v in verdicts: + if v.get("status") == "MET" and not died(v): + continue + i += 1 + text = (v.get("text") or "").strip().replace("\n", " ") + if len(text) > 140: + text = text[:137] + "…" + complaint = (v.get("complaint") or "").strip() + if died(v): + if not complaint.startswith(DIED_MARKER): + complaint = f"{DIED_MARKER} — {complaint}" if complaint else DIED_MARKER + lines.append(f'{i}. Acceptance criterion "{text}" — {complaint}') + else: + lines.append( + f'{i}. Acceptance criterion "{text}" — {v.get("status", "?").title()} — ' + f"{complaint}" + ) + for c in extra: + i += 1 + lines.append( + f"{i}. {c.get('kind', 'violation')} — {c.get('location', '?')} — " + f"{c.get('violation', '')} — {c.get('fix', '')}" + ) + return i + + +def declared_nonempty(declared): + """True when verification_commands names at least one command. + + Accepts a list or a JSON-encoded string of one. None, unparseable, or + empty declarations count as not declared — run_checks already records + a PIPELINE-FAULT for the invalid ones. + """ + if isinstance(declared, str): + try: + declared = json.loads(declared) + except json.JSONDecodeError: + return False + if not isinstance(declared, list): + return False + return any(isinstance(c, str) and c.strip() for c in declared) + + +def exec_run_complaints(exec_results, declared): + """One complaint per red verification run recorded by run_checks.py. + + A red run is a record that was skipped (deadline) or exited nonzero + (including the -1 TIMEOUT sentinel). An "ENVIRONMENT: …" runner-error + marker with a non-empty declaration is one complaint: the declared + commands never ran. Other string markers ("none declared", + "PIPELINE-FAULT…") are not runs and yield nothing. + """ + if isinstance(exec_results, str): + marker = exec_results.strip().replace("\n", " ") + if not marker.startswith("ENVIRONMENT:") or not declared_nonempty(declared): + return [] + if len(marker) > MAX_MARKER_CHARS: + marker = marker[: MAX_MARKER_CHARS - 1] + "…" + return [ + f"Verification runner error — the declared command(s) never ran ({marker}); " + "unproven runs block CONFORMS" + ] + if not isinstance(exec_results, list): + return [] + out = [] + for r in exec_results: + if not isinstance(r, dict): + continue + cmd = r.get("cmd", "?") + if r.get("skipped"): + out.append(f"Verification run `{cmd}` — never ran: deadline expired (unproven)") + continue + code = r.get("exit") + if code == 0: + continue + outcome = "TIMEOUT (exit -1)" if code == -1 else f"exit {code}" + out.append( + f"Verification run `{cmd}` — {outcome} (recorded FAIL; a red declared " + "command blocks CONFORMS regardless of criterion judgments)" + ) + return out + + +def render_exec_results(exec_results): + """Lines for the "Verification runs:" report section. + + exec_results is a list of {cmd, exit, duration_s, tail} records (or + {cmd, skipped} for commands the runner's total deadline left no budget + for) from run_checks.py, or a string marker ("none declared —…" / + "ENVIRONMENT: …"), or empty when the run never reached the stage. + """ + if isinstance(exec_results, list): + out = [] + for r in exec_results: + if not isinstance(r, dict): + continue + if r.get("skipped"): + out.append( + f"- [SKIPPED] `{r.get('cmd', '?')}` — never ran: the runner's " + "total deadline expired first; covered criteria are unproven" + ) + continue + status = "PASS" if r.get("exit") == 0 else "FAIL" + out.append( + f"- [{status}] `{r.get('cmd', '?')}` — exit {r.get('exit', '?')}, " + f"{r.get('duration_s', '?')}s" + ) + if status == "FAIL": + tail = (r.get("tail") or "").strip() + out.extend(" " + t for t in tail.splitlines()[-15:]) + return out or ["- (no verification commands were run)"] + if isinstance(exec_results, str) and exec_results.strip(): + return [f"- {exec_results.strip()}"] + return ["- none recorded (the run did not reach the verification stage)"] + + def main(): state = load_state() verdicts = [v for v in (state.get("crit_verdicts") or []) if isinstance(v, dict)] extra = [c for c in (state.get("extra_complaints") or []) if isinstance(c, dict)] observations = (state.get("observations") or "").strip() + faults = collect_pipeline_faults(state) + reds = exec_run_complaints( + state.get("exec_results"), state.get("verification_commands") + ) n = len(verdicts) - met = [v for v in verdicts if v.get("status") == "MET"] - partial = [v for v in verdicts if v.get("status") == "PARTIAL"] - bad = [v for v in verdicts if v.get("status") in ("UNMET", "DIVERGED")] + met = [v for v in verdicts if v.get("status") == "MET" and not died(v)] + partial = [v for v in verdicts if v.get("status") == "PARTIAL" and not died(v)] + bad = [ + v for v in verdicts if died(v) or v.get("status") in ("UNMET", "DIVERGED") + ] lines = [] - if n == 0: + if faults: + lines.append("ADVERSARIAL_REVIEW: DIVERGES") + if n: + lines.append( + f"Criteria: {len(met)}/{n} met, {len(partial)} partial, " + f"{len(bad)} unmet/diverged — degraded run: pipeline fault recorded " + "(fail-closed)." + ) + else: + lines.append( + "Criteria: none verified — degraded run: pipeline fault recorded " + "(fail-closed)." + ) + lines.append("Complaints:") + i = 0 + for f in faults: + i += 1 + lines.append(f"{i}. {f}") + i = append_criterion_complaints(lines, verdicts, extra, i) + for c in reds: + i += 1 + lines.append(f"{i}. {c}") + if observations: + lines.append("") + lines.append("Non-blocking observations:") + lines.append(observations) + elif n == 0: lines.append("ADVERSARIAL_REVIEW: DIVERGES") lines.append("Criteria: none provided.") lines.append("Complaints:") @@ -38,7 +233,9 @@ def main(): "be judged without a spec (fail-closed). Supply the plan's Objective/Tasks/" "Acceptance criteria and re-run." ) - elif not partial and not bad and not extra: + for i, c in enumerate(reds, start=2): + lines.append(f"{i}. {c}") + elif not partial and not bad and not extra and not reds: lines.append("ADVERSARIAL_REVIEW: CONFORMS") lines.append(f"Criteria: {n}/{n} met (all with tests).") if observations: @@ -47,28 +244,18 @@ def main(): lines.append(observations) else: lines.append("ADVERSARIAL_REVIEW: DIVERGES") - lines.append( - f"Criteria: {len(met)}/{n} met, {len(partial)} partial, {len(bad)} unmet/diverged." - ) + header = f"Criteria: {len(met)}/{n} met, {len(partial)} partial, {len(bad)} unmet/diverged" + notes = [] + if died_count := sum(1 for v in verdicts if died(v)): + notes.append(f"degraded run: {died_count} criterion check(s) died (fail-closed)") + if reds: + notes.append(f"{len(reds)} red verification run(s) (fail-closed)") + lines.append(header + "".join(f" — {note}" for note in notes) + ".") lines.append("Complaints:") - i = 0 - for v in verdicts: - if v.get("status") == "MET": - continue + i = append_criterion_complaints(lines, verdicts, extra, 0) + for c in reds: i += 1 - text = (v.get("text") or "").strip().replace("\n", " ") - if len(text) > 140: - text = text[:137] + "…" - lines.append( - f'{i}. Acceptance criterion "{text}" — {v.get("status", "?").title()} — ' - f'{(v.get("complaint") or "").strip()}' - ) - for c in extra: - i += 1 - lines.append( - f"{i}. {c.get('kind', 'violation')} — {c.get('location', '?')} — " - f"{c.get('violation', '')} — {c.get('fix', '')}" - ) + lines.append(f"{i}. {c}") if observations: lines.append("") lines.append("Non-blocking observations:") @@ -84,6 +271,10 @@ def main(): text = text[:97] + "…" lines.append(f'- "{text}" — {(v.get("evidence") or "").strip()}') + lines.append("") + lines.append("Verification runs:") + lines.extend(render_exec_results(state.get("exec_results"))) + print(json.dumps({"adv_report": "\n".join(lines)})) diff --git a/assets/agents/architect/README.md b/assets/agents/architect/README.md index 245ac16..96df479 100644 --- a/assets/agents/architect/README.md +++ b/assets/agents/architect/README.md @@ -160,6 +160,9 @@ By default (`parallel_tasks: 0`) tasks run **sequentially** on the single run br - Architect integrates: completed task branches merge into the run branch **one at a time**, with a full build + test run after every merge. Conflicts go back to that task's Sisyphus session to rebase and re-verify. +- Optionally, the `agent__task_*` queue may mirror the on-disk `blocked_by` graph so completing a + task auto-dispatches its unblocked dependents. The queue is in-memory convenience only — disk task + files remain the durable source of truth. - Worktrees and task branches are cleaned up after each clean merge. Phase F (single draft PR + CI-check watch) is unchanged in both modes. diff --git a/assets/agents/architect/config.yaml b/assets/agents/architect/config.yaml index 0d5ca3c..4058e04 100644 --- a/assets/agents/architect/config.yaml +++ b/assets/agents/architect/config.yaml @@ -333,8 +333,14 @@ instructions: | Forced lanes: adversary<, probe — add probe when the task changed consumer-facing surface and the plan carries a local-run recipe> Plan / acceptance criteria: Local-run recipe / usage suites: " + --variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"} + <— the cargo commands above are only an EXAMPLE: replace them with THIS project's exact build/test/lint commands; never guess, and pass "[]" when none apply> ``` + Verification commands go in `--variables` (a JSON-array string), never in the prompt: the + prompt also carries pasted plan text from the repo under review, and whatever is declared + here is executed with a shell — a trust boundary the prose must not cross. + It returns `GAUNTLET: PASS/BLOCKED`. On BLOCKED apply the same fix-and-rerun rules as below (resume the SAME Sisyphus session with the findings verbatim, then a FRESH gauntlet run). The direct `adversary`/probe spawns below are the FALLBACK when the gauntlet is not installed. @@ -351,8 +357,13 @@ instructions: | .md section VERBATIM> " + --variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"} + <— the cargo commands above are only an EXAMPLE: replace them with THIS project's exact build/test/lint commands; never guess, and pass "[]" when none apply> ``` + Same trust boundary as the gauntlet spawn: verification commands are declared via + `--variables`, never in the prompt, because the prompt also carries untrusted pasted text. + - **`ADVERSARIAL_REVIEW: DIVERGES`** → treat it as a blocker: resume the SAME Sisyphus session (`agent__spawn --session_id --prompt "Fix these plan-conformance failures: "`) — do not spawn a fresh one. Re-run `adversary` ONCE after the fix to @@ -456,6 +467,20 @@ instructions: | the task (step 5), and clean up — `git worktree remove .worktrees/task-NNN` and delete the task branch (local + remote). + **Task-queue mirroring (optional).** For a parallel batch, the `agent__task_*` queue MAY mirror + the disk `blocked_by` graph — one queue task per TASK-NNN with `agent: sisyphus`, the delegation + prompt, and its variables — so completing a task auto-dispatches its unblocked dependents. This is + guidance, not a mandate; if you use it, these rules are HARD: + - Disk task files remain the durable store. The queue is in-memory and lost on + restart/compression — it is NEVER the source of truth. + - Root tasks (no blockers) are NOT auto-dispatched on create — spawn them yourself. + - Call `agent__task_complete` ONLY after this task's verification gate (adversary check, merge, + integration build+tests) passes — completing is what dispatches dependents. + - Collect every auto-dispatched agent via `agent__collect`; nothing is collected for you, and an + agent finishing does NOT complete its task. + - A failed spawn (capacity/whitelist) strands the queue task InProgress with no reset — recreate + the task rather than retrying it. + Phase F is UNCHANGED (same single draft PR from the run branch). Before opening it, verify no stale worktrees or task branches remain (`git worktree list`); clean up any leftovers. diff --git a/assets/agents/architecture-reviewer/config.yaml b/assets/agents/architecture-reviewer/config.yaml index 045a6ca..e0ff9b9 100644 --- a/assets/agents/architecture-reviewer/config.yaml +++ b/assets/agents/architecture-reviewer/config.yaml @@ -12,7 +12,11 @@ spawnable_agents: - explore - oracle max_concurrent_agents: 4 -max_agent_depth: 2 +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# architecture-reviewer(2) → {explore | oracle}(3). This agent spawns depth-3 +# children; 4 = one level of headroom. +max_agent_depth: 4 inject_spawn_instructions: true skills_enabled: true diff --git a/assets/agents/code-reviewer/README.md b/assets/agents/code-reviewer/README.md index 2a0e4f1..b294429 100644 --- a/assets/agents/code-reviewer/README.md +++ b/assets/agents/code-reviewer/README.md @@ -14,21 +14,44 @@ ```mermaid flowchart TD A["parse (llm)"] --> B["facts (script): diff, quality bar, ledger, history, trigger signals"] + A -. "fallback" .-> PF["parse_fault (script): PIPELINE-FAULT verdict"] + PF --> L B --> C["linters (script): tflint / hadolint / actionlint / buf breaking"] - C --> D["group_domains (script) --> refine_groups (llm) --> cover_gate (script: exact cover)"] - D --> E["map over domain slices (parallel): domain-reviewer leaders, teammates on"] - D --> F["aux_lanes (llm): org-context + prior-art spawns"] - D --> G["downstream_sweep (script)"] + C --> D1["group_propose (script): deterministic domain proposal"] + D1 --> D2["refine_groups (llm)"] + D2 --> D3["cover_gate (script: exact cover)"] + D2 -. "fallback" .-> D3 + D3 --> E["map over domain slices (parallel): domain-reviewer leaders, teammates on, max_attempts: 2"] + D3 --> F["aux_lanes (llm): org-context + prior-art spawns"] + D3 --> G["downstream_sweep (script)"] E --> H["completeness_gate (script): required sections even when clean"] + E -. "fallback" .-> DF["domain_fault (script): ⚠️ PIPELINE-FAULT slice banner"] H -. "reject-retry once" .-> E H --> I["synthesize (llm: dedup + prose, NO verdict authority)"] F --> I + F -. "fallback" .-> AF["aux_fault (script): PIPELINE-FAULT lane-skip note"] + AF --> I G --> I - I --> J["verify: finding-verifier (agent node, unskippable)"] - J --> K["verdict (script): FALSE-drops, rigor folding, MERGE-READY / NEEDS-HUMAN arithmetic"] + I --> J["verify: finding-verifier (agent node, unskippable, max_attempts: 2)"] + I -. "fallback" .-> K + J --> K["verdict (script): FALSE-drops, rigor folding, fault accounting, MERGE-READY / NEEDS-HUMAN arithmetic"] + J -. "fallback" .-> K K --> L["render (script): the standard report"] ``` +**Failure fails closed.** Every LLM/agent node retries once (`max_attempts: 2` — the +domain-reviewer lanes and the verifier included), then routes to a `fallback:` instead of killing +the graph, and every fallback that loses review coverage leaves a `PIPELINE-FAULT` marker: a dead +`parse` emits a complete fault verdict and jumps straight to render; a dead domain lane becomes a +`> ⚠️ PIPELINE-FAULT` banner where its slice report would be; a dead `refine_groups` falls +through to the cover gate, which keeps the deterministic proposal; a dead synthesis or verifier +falls through to the verdict script. The verdict script counts every fault and forces +`NEEDS-HUMAN` — a degraded run never reads `MERGE-READY`. The aux context lanes are the one +exception: they are enrichment, so their fault (`aux_fault`) is surfaced as a lane-skip note but +never blocks. Fault detection is prefix-anchored (a report that merely *quotes* a marker doesn't +trip it), and the report is always emitted: the verdict script's own crash guard emits a +`PIPELINE-FAULT` NEEDS-HUMAN, and render bypasses its "No changes to review." stub for fault +verdicts — a fault renders as a full report, never silence. A CodeRabbit-style code review orchestrator that coordinates per-file reviews and synthesizes findings into a unified report. diff --git a/assets/agents/code-reviewer/graph.yaml b/assets/agents/code-reviewer/graph.yaml index 6f4eb8a..39a6623 100644 --- a/assets/agents/code-reviewer/graph.yaml +++ b/assets/agents/code-reviewer/graph.yaml @@ -18,7 +18,12 @@ description: | version: '1.0' can_spawn_agents: true -max_agent_depth: 3 +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# review-gauntlet | step-runner(2) → code-reviewer(3) → +# {domain-reviewer(4) → file-reviewer(5) | finding-verifier(4)}. +# This agent spawns depth-4 children; 5 = one level of headroom. +max_agent_depth: 5 max_concurrent_agents: 16 variables: @@ -91,6 +96,10 @@ initial_state: final_report: '' gate_feedback: '' gate_attempts: 0 + parse_failure: '' + refine_failure: '' + aux_failure: '' + synth_failure: '' start: parse @@ -139,8 +148,19 @@ nodes: required: [diff_spec, intent, rigor_in, surfaces_in, project_dir_in, extra_context] max_iterations: 3 max_attempts: 2 + state_updates: + parse_failure: '{{output}}' + fallback: parse_fault next: facts + parse_fault: + id: parse_fault + type: script + description: Fail-closed marker for a dead parse stage (reached only via parse's fallback) — emits a complete PIPELINE-FAULT verdict_out and jumps straight to render; a review that could not parse its request never reads as MERGE-READY. + script: scripts/parse_fault.py + timeout: 30 + next: render + facts: id: facts type: script @@ -209,6 +229,9 @@ nodes: required: [groups] max_iterations: 3 max_attempts: 2 + state_updates: + refine_failure: '{{output}}' + fallback: cover_gate next: cover_gate cover_gate: @@ -272,8 +295,10 @@ nodes: Linter output (extract the lines for your files): {{linter_notes}} {{extra_context}} + max_attempts: 2 state_updates: domain_report: '{{output}}' + fallback: domain_fault next: completeness_gate completeness_gate: @@ -283,6 +308,13 @@ nodes: script: scripts/completeness_gate.py timeout: 30 + domain_fault: + id: domain_fault + type: script + description: Fail-closed domain lane fault marker (reached only via review_domain's fallback route inside the map branch). Normalizes the engine's "Agent node failed" text in domain_report into a > ⚠️ PIPELINE-FAULT banner the map collects; verdict.py counts any faulted lane and forces NEEDS-HUMAN — a dead domain lane never reads as a clean slice. + script: scripts/domain_fault.py + timeout: 30 + aux_lanes: id: aux_lanes type: llm @@ -338,6 +370,17 @@ nodes: max_iterations: 10 max_attempts: 2 timeout: 3600 + state_updates: + aux_failure: '{{output}}' + fallback: aux_fault + next: synthesize + + aux_fault: + id: aux_fault + type: script + description: Fail-closed marker for dead aux context lanes (reached only via aux_lanes' fallback) — records the failure as a PIPELINE-FAULT in aux_note, which synthesize surfaces as a lane skip. Aux lanes are enrichment; the degradation is visible but never blocks. + script: scripts/aux_fault.py + timeout: 30 next: synthesize downstream_sweep: @@ -456,6 +499,9 @@ nodes: required: [walkthrough, changes_rows, findings, cross_file, operational_history, org_context_section, attention_flags, placement_notes] max_iterations: 6 max_attempts: 2 + state_updates: + synth_failure: '{{output}}' + fallback: verdict next: verify verify: @@ -472,8 +518,10 @@ nodes: Findings (JSON; verify every 🔴 and 🟡 entry — id, severity, path, lines, claim are the fields; `title` + `block` together are the claim): {{findings}} + max_attempts: 2 state_updates: verifier_output: '{{output}}' + fallback: verdict next: verdict verdict: diff --git a/assets/agents/code-reviewer/scripts/aux_fault.py b/assets/agents/code-reviewer/scripts/aux_fault.py new file mode 100644 index 0000000..76edd09 --- /dev/null +++ b/assets/agents/code-reviewer/scripts/aux_fault.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for dead aux context lanes of the code-reviewer graph. + +Reached ONLY via aux_lanes' `fallback:` route. aux_lanes' state_updates wrote +an "LLM node …failed: " string into `aux_failure` before routing here; +this node records the failure as a PIPELINE-FAULT in `aux_note`, which flows +into synthesize's "lane skips" prompt line. Aux lanes are ENRICHMENT, not +review coverage: the degradation is made VISIBLE in the report's inputs but +never blocks — verdict.py deliberately does NOT scan aux_note for faults. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + raw = state.get("aux_failure") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print( + json.dumps( + {"aux_note": f"PIPELINE-FAULT: aux context lanes failed after retries — {detail}"} + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print( + json.dumps( + { + "aux_note": "PIPELINE-FAULT: aux context lanes failed after retries — " + f"fault-marker script error: {e}" + } + ) + ) diff --git a/assets/agents/code-reviewer/scripts/cover_gate.py b/assets/agents/code-reviewer/scripts/cover_gate.py index 1ebfc23..717a2f0 100644 --- a/assets/agents/code-reviewer/scripts/cover_gate.py +++ b/assets/agents/code-reviewer/scripts/cover_gate.py @@ -54,6 +54,11 @@ def main(): else: chosen = [{"domain": g["domain"], "files": g["files"]} for g in refined] if chosen is None: + # refine_groups' fallback lands here with `groups` never set; make the + # degradation visible in the synthesis prompt via groups_note. + refine_failure = state.get("refine_failure") + if not refined and isinstance(refine_failure, str) and refine_failure.startswith("LLM node"): + note = "refinement lane failed after retries — fell back to the deterministic grouping" chosen = [{"domain": g["domain"], "files": g["files"]} for g in proposed if isinstance(g, dict)] print(json.dumps({"group_items": chosen, "groups_note": note})) diff --git a/assets/agents/code-reviewer/scripts/domain_fault.py b/assets/agents/code-reviewer/scripts/domain_fault.py new file mode 100644 index 0000000..2b307d5 --- /dev/null +++ b/assets/agents/code-reviewer/scripts/domain_fault.py @@ -0,0 +1,59 @@ +#!/usr/bin/env python3 +"""Fail-closed domain lane fault marker for the code-reviewer graph. + +Reached ONLY via review_domain's `fallback:` route inside the map branch. +Grounded in engine behavior: on agent-node failure the engine writes +"Agent node failed: {e:#}" into `domain_report` via the node's state_updates +BEFORE routing to the fallback, so the raw failure text is already in state. +This node normalizes it into a "> ⚠️ PIPELINE-FAULT: domain review lane +failed after retries — …" banner the map collects as the lane's report; +verdict.py counts any banner-prefixed report as a fault and forces +NEEDS-HUMAN — a dead domain lane never reads as a clean slice. + +Slice identity comes from `domain_group` (the map item cover_gate emitted, +which carries `domain` and `files`); a missing/malformed item degrades to +the failure detail alone. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + +FAULT_PREFIX = "> ⚠️ PIPELINE-FAULT: domain review lane failed after retries — " + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + slice_desc = "" + group = state.get("domain_group") + if isinstance(group, dict): + domain = group.get("domain") + files = [f for f in (group.get("files") or []) if isinstance(f, str)] + if isinstance(domain, str) and domain.strip(): + slice_desc = f"domain '{domain.strip()}' (files: {', '.join(files)}): " + raw = state.get("domain_report") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print(json.dumps({"domain_report": f"{FAULT_PREFIX}{slice_desc}{detail}"})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print( + json.dumps( + { + "domain_report": "> ⚠️ PIPELINE-FAULT: domain review lane failed " + f"after retries — fault-marker script error: {e}" + } + ) + ) diff --git a/assets/agents/code-reviewer/scripts/parse_fault.py b/assets/agents/code-reviewer/scripts/parse_fault.py new file mode 100644 index 0000000..000815b --- /dev/null +++ b/assets/agents/code-reviewer/scripts/parse_fault.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for a dead parse stage of the code-reviewer graph. + +Reached ONLY via parse's `fallback:` route. parse's state_updates wrote an +"LLM node …failed: " string into `parse_failure` before routing here; +this node emits a complete PIPELINE-FAULT `verdict_out` and the graph jumps +straight to render — a review that could not even parse its request never +reads as MERGE-READY. render.py renders a PIPELINE-FAULT verdict despite the +empty changed-file list (its "No changes to review." early return skips +fault-reason verdicts). +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + +ATTENTION_LINE = ( + "PIPELINE-FAULT: parse failed — the review request could not be parsed; " + "the review did not run" +) + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def fault_verdict(detail): + return { + "verdict": "NEEDS-HUMAN", + "reason": f"PIPELINE-FAULT: parse failed — {detail}", + "counts": {}, + "deferred_count": 0, + "dropped_count": 0, + "dropped_titles": [], + "attention": [ATTENTION_LINE], + "findings_final": [], + } + + +def main(): + state = load_state() + raw = state.get("parse_failure") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("LLM node"): + detail = "the parse stage died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print(json.dumps({"verdict_out": fault_verdict(detail)})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print(json.dumps({"verdict_out": fault_verdict(f"fault-marker script error: {e}")})) diff --git a/assets/agents/code-reviewer/scripts/render.py b/assets/agents/code-reviewer/scripts/render.py index cf0ac54..77c4cf5 100644 --- a/assets/agents/code-reviewer/scripts/render.py +++ b/assets/agents/code-reviewer/scripts/render.py @@ -16,15 +16,28 @@ def load_state(): return json.loads(os.environ.get("GRAPH_STATE", "{}")) +# LLM-influenced values rendered after the summary line: a newline could +# forge a second summary line that review-gauntlet/verdict_gate.py parses. +def one_line(x): + return str(x).replace("\n", " ") + + def main(): state = load_state() v = state.get("verdict_out") or {} findings = v.get("findings_final") or [] counts = v.get("counts") or {} changed = state.get("changed_files") or [] - rigor = state.get("resolved_rigor") or "production" + rigor = one_line(state.get("resolved_rigor") or "production") - if not changed: + # A fault can leave changed_files empty (a parse fault dies before the + # diff is resolved; a verifier fault fires even on an empty diff) — a + # fault-degraded verdict must still render as a full report. parse_fault + # puts the marker in the reason; verdict.py prepends it to attention. + faulted = str(v.get("reason") or "").startswith("PIPELINE-FAULT:") or any( + isinstance(a, str) and a.startswith("PIPELINE-FAULT:") for a in v.get("attention") or [] + ) + if not changed and not faulted: print(json.dumps({"final_report": "# Code Review Summary\n\nNo changes to review."})) return @@ -32,7 +45,8 @@ def main(): lines = [ "# Code Review Summary", "", - f"**Verdict: {verdict}** — {v.get('reason', '')}", + # CONTRACT: review-gauntlet/verdict_gate.py anchors on this wording + f"**Verdict: {verdict}** — {one_line(v.get('reason', ''))}", "", "## Walkthrough", state.get("walkthrough") or "(no walkthrough provided)", @@ -89,15 +103,17 @@ def main(): m for x in findings for m in __import__("re").findall(r"MISS-\d+", (x.get("title") or "") + (x.get("block") or "")) }) + dropped = ", ".join(one_line(t) for t in v.get("dropped_titles", [])[:5]) lines += [ "---", + # CONTRACT: review-gauntlet/verdict_gate.py parses this line — keep the format in sync f"*Reviewed {len(changed)} files, found {counts.get('🔴', 0)} critical, " f"{counts.get('🟡', 0)} warnings, {counts.get('🟢', 0)} suggestions, " f"{counts.get('💡', 0)} nitpicks ({v.get('deferred_count', 0)} deferred by quality bar)*", - f"*Quality bar: {rigor} — provenance: {state.get('bar_provenance', '?')}; " - f"surfaces: {state.get('resolved_surfaces') or 'none'}*", + f"*Quality bar: {rigor} — provenance: {one_line(state.get('bar_provenance', '?'))}; " + f"surfaces: {one_line(state.get('resolved_surfaces') or 'none')}*", f"*Verdict: {verdict}; verifier: dropped {v.get('dropped_count', 0)} provably-false" - + (f" ({', '.join(v.get('dropped_titles', [])[:5])})" if v.get("dropped_count") else "") + + (f" ({dropped})" if v.get("dropped_count") else "") + f"; review-miss patterns applied: {', '.join(miss_ids) if miss_ids else 'none relevant'}*", ] print(json.dumps({"final_report": "\n".join(lines)})) diff --git a/assets/agents/code-reviewer/scripts/verdict.py b/assets/agents/code-reviewer/scripts/verdict.py index e992afa..1eeea61 100644 --- a/assets/agents/code-reviewer/scripts/verdict.py +++ b/assets/agents/code-reviewer/scripts/verdict.py @@ -8,6 +8,12 @@ The model that wrote the findings never grades its own homework: markers, exactly per the published rules; - MERGE-READY/NEEDS-HUMAN is computed from counts + always-human triggers (deterministic signals ∪ the synthesis's ADDITIVE-only flags). +Fail-closed: any PIPELINE-FAULT recorded upstream (dead domain lane, dead +verifier, verifier verdicts carrying a PIPELINE-FAULT note or the +`pipeline-fault` sentinel id, dead synthesis, missing domain reports) +forces NEEDS-HUMAN — a degraded run can never read MERGE-READY. Faults +always lead the attention list; the display cap applies only to the +non-fault entries so a degraded run can never hide a fault. The gate never crashes into a silent verdict: any internal error emits NEEDS-HUMAN naming the error. """ @@ -16,6 +22,9 @@ import json import os import re +MAX_DETAIL_CHARS = 500 + + def load_state(): if path := os.environ.get("GRAPH_STATE_FILE"): with open(path) as f: @@ -30,35 +39,43 @@ def sev_icon(severity): return "🟢" -def parse_verifier(text): - """Extract {id: {verdict, evidence, note}} from finding-verifier output.""" - verdicts = {} +def parse_verifier_entries(text): + """Extract the raw list of {id, verdict, evidence, note} dicts from + finding-verifier output. Order and duplicates are preserved so fault + accounting can count every entry (crash-guard entries all carry the + same "unknown" id).""" if not isinstance(text, str) or not text.strip(): - return verdicts + return [] start, end = text.find("["), text.rfind("]") if start != -1 and end > start: try: - for v in json.loads(text[start : end + 1]): - if isinstance(v, dict) and v.get("id"): - verdicts[str(v["id"])] = v - return verdicts + return [ + v for v in json.loads(text[start : end + 1]) if isinstance(v, dict) and v.get("id") + ] except (ValueError, TypeError): pass + entries = [] for m in re.finditer(r'\{[^{}]*"id"[^{}]*\}', text): try: v = json.loads(m.group(0)) if v.get("id"): - verdicts[str(v["id"])] = v + entries.append(v) except (ValueError, TypeError): continue - return verdicts + return entries + + +def parse_verifier(entries): + """Index verifier entries by finding id; the last entry per id wins.""" + return {str(v["id"]): v for v in entries} def main(): state = load_state() findings = [f for f in (state.get("findings") or []) if isinstance(f, dict)] rigor = (state.get("resolved_rigor") or "production").lower() - verdicts = parse_verifier(state.get("verifier_output")) + verifier_entries = parse_verifier_entries(state.get("verifier_output")) + verdicts = parse_verifier(verifier_entries) kept, dropped_titles = [], [] for f in findings: @@ -117,17 +134,73 @@ def main(): seen = set() attention = [a for a in attention if not (a.lower() in seen or seen.add(a.lower()))] - if (state.get("changed_files") or []) and not (state.get("domain_reports") or []): - attention.append( - "PIPELINE FAULT: the diff is non-empty but no domain reports were produced — " + # --- pipeline-fault accounting (fail closed) ----------------------------- + # PREFIX-anchored: a report that merely QUOTES a marker mid-text must not + # trip it. Any fault forces NEEDS-HUMAN. + faults = [] + changed = state.get("changed_files") or [] + reports = state.get("domain_reports") or [] + faulted = sum( + 1 for r in reports if isinstance(r, str) and r.lstrip().startswith("> ⚠️ PIPELINE-FAULT:") + ) + if faulted: + faults.append( + f"PIPELINE-FAULT: {faulted} of {len(reports)} domain review lane(s) " + "failed after retries — their slices were not reviewed" + ) + verifier_raw = state.get("verifier_output") + if isinstance(verifier_raw, str) and verifier_raw.lstrip().startswith("Agent node failed:"): + faults.append( + "PIPELINE-FAULT: finding verification failed — findings render as unverified" + ) + # finding-verifier completes "successfully" with faults INSIDE the payload: + # parse_fault's `pipeline-fault` sentinel entry, or a per-finding + # PIPELINE-FAULT note from verify_fault or verdict_gate's crash guard. + # Anchored on the sentinel id / note prefix only. Counted over the raw + # entries, not the id-indexed dict, so N crash-guard entries sharing the + # "unknown" id count as N. + fv_faults = [ + v + for v in verifier_entries + if v.get("id") == "pipeline-fault" + or (isinstance(v.get("note"), str) and v["note"].lstrip().startswith("PIPELINE-FAULT:")) + ] + if fv_faults: + first = fv_faults[0].get("note") + detail = first.strip().replace("\n", " ") if isinstance(first, str) else "" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + faults.append( + f"PIPELINE-FAULT: finding verification degraded — {len(fv_faults)} verifier " + f"verdict(s) carry a pipeline fault ({detail})" + ) + # Keyed off synth_failure + a non-empty diff, NEVER off findings == [] — + # a clean review legitimately has zero findings and stays MERGE-READY. + synth_failure = state.get("synth_failure") + if isinstance(synth_failure, str) and synth_failure.startswith("LLM node") and changed: + detail = synth_failure.strip().replace("\n", " ") + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + faults.append(f"PIPELINE-FAULT: synthesis failed — findings unavailable; {detail}") + if changed and not reports: + faults.append( + "PIPELINE-FAULT: the diff is non-empty but no domain reports were produced — " "the review did not actually run; do not trust this verdict." ) + # faults go FIRST and uncapped so the display cap can never hide one; + # the trigger reason keys off the human-facing triggers alone + has_triggers = bool(attention) + attention = faults + attention[:5] + reasons = [] if counts["🔴"]: reasons.append(f"{counts['🔴']} 🔴 CRITICAL finding(s)") if yellow_correctness: reasons.append(f"{yellow_correctness} 🟡 [correctness] finding(s) outside the deferred section") - if attention: + if faults: + # CONTRACT: review-gauntlet/verdict_gate.py anchors on this wording + reasons.append("pipeline fault(s) recorded — degraded run") + if has_triggers: reasons.append("always-human trigger(s) fired") verdict = "NEEDS-HUMAN" if reasons else "MERGE-READY" reason_line = "; ".join(reasons) if reasons else "no blocking findings and no always-human triggers" @@ -142,7 +215,7 @@ def main(): "deferred_count": len(deferred), "dropped_count": len(dropped_titles), "dropped_titles": dropped_titles, - "attention": attention[:5], + "attention": attention, "findings_final": final, } } @@ -158,9 +231,13 @@ except Exception as e: # noqa: BLE001 — never crash into a silent verdict { "verdict_out": { "verdict": "NEEDS-HUMAN", - "reason": f"verdict computation error: {e} — treat as needing human review", + # PIPELINE-FAULT prefix is load-bearing: render.py's + # empty-diff stub bypass anchors on it — without it a + # crashed gate on an empty diff collapses into the + # "No changes to review." stub. + "reason": f"PIPELINE-FAULT: verdict computation error: {e} — human review required", "counts": {}, "deferred_count": 0, "dropped_count": 0, - "dropped_titles": [], "attention": [f"verdict script error: {e}"], + "dropped_titles": [], "attention": [f"PIPELINE-FAULT: verdict script error: {e}"], "findings_final": [], } } diff --git a/assets/agents/deep-research/README.md b/assets/agents/deep-research/README.md index ea5f2ab..7d2864d 100644 --- a/assets/agents/deep-research/README.md +++ b/assets/agents/deep-research/README.md @@ -22,11 +22,13 @@ agent, this is the file to read alongside the ## Workflow -17 nodes. Solid arrows are static `next` / `routes` edges declared in +23 nodes. Solid arrows are static `next` / `routes` edges declared in `graph.yaml`; script nodes can also route dynamically via `_next` (shown as labeled branches out of the diamond). Dotted arrows show `map` fan-out — the `research_each_question` node spawns one `research_one_question` branch per -sub-question and joins them before continuing. +sub-question and joins them before continuing — and `fallback` edges +(labeled "fallback"), taken when a node fails outright (see *Fault paths* +below). ```mermaid flowchart TD @@ -38,18 +40,27 @@ flowchart TD bootstrap_research --> plan bootstrap_research --> knowledge_lookup plan["plan
llm + output_schema"] --> research_each_question + plan -. "fallback" .-> plan_fault{"plan_fault
script"} + plan_fault --> end_fault knowledge_lookup[("knowledge_lookup
rag")] --> research_each_question research_each_question[\research_each_question
map/] research_each_question -. "spawns × N" .-> research_one_question["research_one_question
llm + web tools"] + research_one_question -. "fallback" .-> question_fault{"question_fault
script"} research_each_question --> combine_findings combine_findings{"combine_findings
script"} --> vet_sources vet_sources["vet_sources
llm + classify_source"] --> critique + vet_sources -. "fallback" .-> vet_fault{"vet_fault
script"} + vet_fault --> critique critique["critique
llm"] --> reflexion_gate + critique -. "fallback" .-> critique_fault{"critique_fault
script"} + critique_fault --> reflexion_gate reflexion_gate{"reflexion_gate
script"} reflexion_gate -->|"PASS"| synthesize reflexion_gate -->|"REVISE (budget left)"| research_each_question reflexion_gate -->|"REVISE (budget spent)"| synthesize synthesize[["synthesize
agent → report-writer"]] --> verify_sources + synthesize -. "fallback" .-> synth_fault{"synth_fault
script"} + synth_fault --> end_fault verify_sources{"verify_sources
script"} --> approve approve{{"approve
approval"}} approve -->|"accept"| end_accepted @@ -59,13 +70,14 @@ flowchart TD end_accepted(["end_accepted
report"]) end_rejected(["end_rejected"]) + end_fault(["end_fault
DEEP_RESEARCH FAILED"]) ``` ### Node-type breakdown | Type | Nodes | |-----------------------------|-----------------------------------------------------------------------------------------------------------------------| -| `script` (Python) | `parse_request`, `bootstrap_research`, `combine_findings`, `reflexion_gate`, `verify_sources`, `incorporate_feedback` | +| `script` (Python) | `parse_request`, `bootstrap_research`, `combine_findings`, `reflexion_gate`, `verify_sources`, `incorporate_feedback`, plus the fault markers `plan_fault`, `question_fault`, `vet_fault`, `critique_fault`, `synth_fault` | | `llm` (tools: `[]`) | `plan`, `critique` | | `llm` (with tool whitelist) | `research_one_question`, `vet_sources` | | `rag` | `knowledge_lookup` — local corpus retrieval | @@ -73,7 +85,7 @@ flowchart TD | `agent` | `synthesize` — spawns the `report-writer` sub-agent | | `input` | `ask_topic` | | `approval` | `approve` | -| `end` | `end_accepted`, `end_rejected` | +| `end` | `end_accepted`, `end_rejected`, `end_fault` | ## Parallel execution @@ -217,6 +229,46 @@ folds that text into `research_feedback` and loops back to `settings.max_loop_iterations` (40) is the engine's infinite-loop backstop: it caps the total visits to any single node. +## Fault paths + +Every stage that can die outright has a `fallback:` edge to a small +fault-marker script — the dashed edges in the diagram. A fallback fires +after a node's retries are exhausted (`research_one_question` and +`synthesize` each get two attempts via `max_attempts: 2`); the engine +writes its failure text into the node's state key before routing, and +the marker normalizes that text into a `PIPELINE-FAULT: ...` entry in +`pipeline_faults`. The five routes: + +- `plan -> plan_fault -> end_fault` — fail-closed: no plan means + nothing to research. +- `research_one_question -> question_fault` — branch-local, inside the + map branch: one dead research lane becomes a fault finding instead of + sinking the whole map. +- `vet_sources -> vet_fault -> critique` — degrade-visibly: vetting is + a quality lane, not the product; `source_assessment` becomes a + neutral "unvetted" note. +- `critique -> critique_fault -> reflexion_gate` — degrade-visibly: the + rewritten critique deliberately has no `VERDICT:` line, so the gate's + malformed-critique PASS default proceeds to synthesis. +- `synthesize -> synth_fault -> end_fault` — fail-closed: no report + means no product. + +On the fail-closed routes, `end_fault` renders the +`DEEP_RESEARCH FAILED — PIPELINE-FAULT: ...` sentinel. On the +degrade-visibly routes, `verify_sources` folds the accumulated faults +into a `## Pipeline notes` section shown at the approval step and +appended to the accepted report (`{{report}}{{pipeline_notes}}`; +empty when there were no faults). Degraded output is never a silent +pass — every fault is visible to the reviewer or the caller — and some +end node always renders its sentinel, so the graph degrades instead of +dying. + +All 11 scripts also carry a top-level crash guard: a script that +throws records a fault and fails forward where the flow allows it (a +crashed `reflexion_gate` proceeds to `synthesize`; a crashed +`verify_sources` reports that sources were NOT checked) instead of +killing the run. + ## Running ```sh @@ -241,6 +293,9 @@ coyote -a deep-research # no prompt -> triggers ask_topic - `verify_sources` probes every cited URL / DOI with an HTTP HEAD request and reports which are unreachable, so the human reviewer sees broken citations before approving. +- Fault paths degrade visibly, never silently: a `PIPELINE-FAULT` + can only ever surface as a failure sentinel or a `## Pipeline + notes` section, never as a clean report. ## Customizing @@ -270,7 +325,7 @@ coyote -a deep-research # no prompt -> triggers ask_topic ``` assets/agents/deep-research/ - graph.yaml - agent config + 17-node workflow + graph.yaml - agent config + 23-node workflow tools.sh - classify_source custom tool README.md - this file knowledge/ @@ -279,9 +334,15 @@ assets/agents/deep-research/ scripts/ parse_request.py - _next: bootstrap_research, or ask_topic if no topic bootstrap_research.py - fan-out source: next [plan, knowledge_lookup] + plan_fault.py - fail-closed fault marker: dead plan -> end_fault + question_fault.py - branch-local fault marker for a dead research lane combine_findings.py - joins map output (question_findings) into findings + vet_fault.py - degrade-visibly fault marker: unvetted sources + critique_fault.py - degrade-visibly fault marker: skipped critique reflexion_gate.py - _next: research_each_question (revise) or synthesize - verify_sources.py - HTTP HEAD on cited URLs / DOIs + synth_fault.py - fail-closed fault marker: no report -> end_fault + verify_sources.py - HTTP HEAD on cited URLs / DOIs; folds pipeline_faults + into the report's "## Pipeline notes" section incorporate_feedback.py - _next: research_each_question, with user feedback ``` diff --git a/assets/agents/deep-research/graph.yaml b/assets/agents/deep-research/graph.yaml index 4aa2b25..d9d375e 100644 --- a/assets/agents/deep-research/graph.yaml +++ b/assets/agents/deep-research/graph.yaml @@ -38,6 +38,14 @@ initial_state: research_attempts: 0 local_context: '' local_sources: '' + # Fault-path defaults. `questions: []` also keeps the + # knowledge_lookup lane alive for one super-step when `plan` dies inside the + # static fan-out: research_each_question maps over zero items while the + # plan_fault → end_fault route renders the failure sentinel. + pipeline_faults: [] + pipeline_notes: '' + fault_text: '' + questions: [] start: parse_request @@ -91,8 +99,22 @@ nodes: maxItems: 6 description: 3-5 specific, self-contained sub-questions. required: [research_plan, questions] + # On success plan_failure holds the node's structured JSON output; on + # failure the engine writes "LLM node ...failed: " into it BEFORE + # routing to the fallback (schema properties are only merged on success). + state_updates: + plan_failure: '{{output}}' + fallback: plan_fault next: research_each_question + plan_fault: + id: plan_fault + type: script + description: Fail-closed marker for a dead planning stage (reached only via plan's fallback) — normalizes the failure text captured in plan_failure into a PIPELINE-FAULT and continues to end_fault; no plan means nothing to research, so the DEEP_RESEARCH FAILED sentinel always renders. The sibling knowledge_lookup lane survives one more super-step by mapping over the empty initial_state `questions` list. + script: scripts/plan_fault.py + fallback: end_fault + next: end_fault + knowledge_lookup: id: knowledge_lookup type: rag @@ -153,6 +175,16 @@ nodes: max_attempts: 2 state_updates: finding: '{{output}}' + # On failure the engine writes "LLM node ...failed: " into `finding` + # via the state_updates above BEFORE routing to the fallback. The marker + # is branch-local: a map branch's fallback must stay inside the branch. + fallback: question_fault + + question_fault: + id: question_fault + type: script + description: Fail-closed marker for a dead research lane (reached only via research_one_question's fallback route inside the map branch). Normalizes the engine's failure text in `finding` into a PIPELINE-FAULT finding the map collects — one dead lane never sinks the whole map. combine_findings later lifts PIPELINE-FAULT findings into pipeline_faults for the report's Pipeline notes. + script: scripts/question_fault.py combine_findings: id: combine_findings @@ -179,6 +211,15 @@ nodes: max_iterations: 15 state_updates: source_assessment: '{{output}}' + fallback: vet_fault + next: critique + + vet_fault: + id: vet_fault + type: script + description: Degrade-visibly marker for a dead vetting stage (reached only via vet_sources' fallback) — vetting is a quality lane, not the product. Records a PIPELINE-FAULT, rewrites source_assessment to a neutral "unvetted" note, and continues to critique, the stage vet_sources' own next pointed to. + script: scripts/vet_fault.py + fallback: critique next: critique critique: @@ -219,6 +260,15 @@ nodes: tools: [] state_updates: critique: '{{output}}' + fallback: critique_fault + next: reflexion_gate + + critique_fault: + id: critique_fault + type: script + description: Degrade-visibly marker for a dead critique stage (reached only via critique's fallback) — critique is a quality lane, not the product. Records a PIPELINE-FAULT and rewrites `critique` to a neutral note with deliberately NO "VERDICT:" line, so reflexion_gate's malformed-critique PASS default proceeds to synthesis; then continues to reflexion_gate, the stage critique's own next pointed to. + script: scripts/critique_fault.py + fallback: reflexion_gate next: reflexion_gate reflexion_gate: @@ -242,10 +292,22 @@ nodes: Produce the final report following your instructions. timeout: 300 + max_attempts: 2 state_updates: report: '{{output}}' + # On failure the engine writes "Agent node failed: " into `report` + # via the state_updates above BEFORE routing to the fallback. + fallback: synth_fault next: verify_sources + synth_fault: + id: synth_fault + type: script + description: Fail-closed marker for a dead synthesis stage (reached only via synthesize's fallback) — no report means no product. Normalizes the "Agent node failed:" text captured in `report` into a PIPELINE-FAULT and continues to end_fault, which renders the DEEP_RESEARCH FAILED sentinel. + script: scripts/synth_fault.py + fallback: end_fault + next: end_fault + verify_sources: id: verify_sources type: script @@ -261,7 +323,7 @@ nodes: {{report}} ---- - {{source_check}} + {{source_check}}{{pipeline_notes}} ---- Accept this report? Pick "accept" or "reject", or type specific @@ -284,9 +346,14 @@ nodes: end_accepted: id: end_accepted type: end - output: '{{report}}' + output: '{{report}}{{pipeline_notes}}' end_rejected: id: end_rejected type: end output: "Research on '{{topic}}' was rejected and discarded." + + end_fault: + id: end_fault + type: end + output: 'DEEP_RESEARCH FAILED — PIPELINE-FAULT: {{fault_text}}' diff --git a/assets/agents/deep-research/scripts/bootstrap_research.py b/assets/agents/deep-research/scripts/bootstrap_research.py index d230afc..36e9a59 100644 --- a/assets/agents/deep-research/scripts/bootstrap_research.py +++ b/assets/agents/deep-research/scripts/bootstrap_research.py @@ -6,6 +6,12 @@ list on this node fans out into two parallel branches (the LLM planner and the RAG knowledge lookup) as a single super-step. The validator requires declared parallel-branch script outputs, so we emit an empty JSON object explicitly here. + +Fail-safe: the node reads no state and computes nothing, so its +happy-path output IS the sane degraded output — the guard emits the same +empty object on any crash. No PIPELINE-FAULT is recorded: appending one +would require reading state (the very thing that crashed) or clobbering +faults recorded upstream by parse_request, and a crash here loses nothing. """ import json @@ -14,5 +20,7 @@ def main(): print(json.dumps({})) -if __name__ == "__main__": +try: main() +except Exception: # noqa: BLE001 — the fan-out source must never crash the graph + print(json.dumps({})) diff --git a/assets/agents/deep-research/scripts/combine_findings.py b/assets/agents/deep-research/scripts/combine_findings.py index b55d7aa..b5a2f09 100644 --- a/assets/agents/deep-research/scripts/combine_findings.py +++ b/assets/agents/deep-research/scripts/combine_findings.py @@ -6,6 +6,18 @@ one entry per sub-question, in input order). Downstream nodes (`vet_sources`, `critique`, `synthesize`) read `{{findings}}` as a single block, so this script renders the array as a Markdown document with one section per question. + +Fault visibility: a research lane that died lands here as a finding +starting with "PIPELINE-FAULT:" (written by question_fault inside the map +branch, where `pipeline_faults` itself is out of reach — map branches only +surface their collected output_key). This script lifts such findings into +`pipeline_faults` (deduplicated, since a reflexion or feedback loop can +re-run the map) so dead lanes surface in the report's Pipeline notes. + +Fail-safe: on a crash (e.g. malformed GRAPH_STATE), `findings` +becomes the fault text itself and a PIPELINE-FAULT entry is recorded — +the pipeline proceeds visibly degraded, and the human approval gate is +the backstop. Existing faults are preserved best-effort. """ import json import os @@ -19,21 +31,40 @@ def load_state(): return json.loads(os.environ.get("GRAPH_STATE", "{}")) +def safe_faults(): + try: + state = load_state() + return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + except Exception: # noqa: BLE001 — best-effort preservation only + return [] + + def main(): state = load_state() questions = state.get("questions") or [] per_question = state.get("question_findings") or [] + faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] sections = [] for idx, q in enumerate(questions): body = per_question[idx] if idx < len(per_question) else "" if isinstance(body, dict) or isinstance(body, list): body = json.dumps(body, indent=2) + if isinstance(body, str) and body.startswith("PIPELINE-FAULT:") and body not in faults: + faults.append(body) sections.append(f"## {q}\n\n{body}") findings = "\n\n".join(sections) if sections else "No findings gathered." - print(json.dumps({"findings": findings})) + print(json.dumps({"findings": findings, "pipeline_faults": faults})) -if __name__ == "__main__": +try: main() +except Exception as e: # noqa: BLE001 — a crashed join must degrade, not die + faults = safe_faults() + fault = ( + f"PIPELINE-FAULT: combining findings crashed — {e}; per-question " + "findings may exist in state but could not be joined" + ) + faults.append(fault) + print(json.dumps({"findings": fault, "pipeline_faults": faults})) diff --git a/assets/agents/deep-research/scripts/critique_fault.py b/assets/agents/deep-research/scripts/critique_fault.py new file mode 100644 index 0000000..5e88ba8 --- /dev/null +++ b/assets/agents/deep-research/scripts/critique_fault.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +"""Degrade-visibly marker for a dead critique stage of deep-research. + +Reached ONLY via critique's `fallback:` route. Grounded in engine behavior: +on llm-node failure the engine writes "LLM node …failed: " into +`critique` via the node's state_updates BEFORE routing to the fallback. +Critique is a quality lane, not the product, so this path degrades instead +of failing closed: it records a PIPELINE-FAULT (surfaced later in the +report's Pipeline notes) and rewrites `critique` to a neutral note, then +continues to `reflexion_gate` — the stage critique's own `next` pointed to. + +The rewritten note deliberately contains NO "VERDICT:" line: +reflexion_gate's parse_verdict defaults to PASS when no verdict line is +found (its documented malformed-critique behavior), so a dead critique +proceeds to synthesis instead of burning the revision budget on a fault. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + +CRITIQUE_NOTE = ( + "Critique unavailable — the critique stage failed (pipeline fault); " + "proceeding to synthesis without an automated review pass." +) + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def safe_faults(): + try: + state = load_state() + return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + except Exception: # noqa: BLE001 — best-effort preservation only + return [] + + +def main(): + state = load_state() + faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + raw = state.get("critique") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("LLM node"): + detail = "the critique stage died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + fault = f"PIPELINE-FAULT: critique failed — {detail}" + if fault not in faults: + faults.append(fault) + print(json.dumps({"pipeline_faults": faults, "critique": CRITIQUE_NOTE})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + faults = safe_faults() + faults.append(f"PIPELINE-FAULT: critique failed — fault-marker script error: {e}") + print( + json.dumps( + { + "pipeline_faults": faults, + "critique": CRITIQUE_NOTE, + } + ) + ) diff --git a/assets/agents/deep-research/scripts/incorporate_feedback.py b/assets/agents/deep-research/scripts/incorporate_feedback.py index 27c22f2..4926ba2 100644 --- a/assets/agents/deep-research/scripts/incorporate_feedback.py +++ b/assets/agents/deep-research/scripts/incorporate_feedback.py @@ -10,6 +10,13 @@ reflexion counter is reset so the user-driven pass gets a fresh revision budget. Routing (`_next`): always research_each_question. + +Fail-safe: a crash here means the user's feedback text cannot be +recovered, but their INTENT — another research pass — is unambiguous +(they chose neither accept nor reject). The degraded route therefore +still loops to `research_each_question`, with a generic feedback note +naming the loss and a PIPELINE-FAULT entry (best-effort preserving +existing faults) that surfaces in the next approval's Pipeline notes. """ import json import os @@ -23,6 +30,14 @@ def load_state(): return json.loads(os.environ.get("GRAPH_STATE", "{}")) +def safe_faults(): + try: + state = load_state() + return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + except Exception: # noqa: BLE001 — best-effort preservation only + return [] + + def main(): state = load_state() feedback = (state.get("decision") or "").strip() @@ -37,5 +52,26 @@ def main(): print(json.dumps(output)) -if __name__ == "__main__": +try: main() +except Exception as e: # noqa: BLE001 — a crashed feedback fold must degrade, not die + faults = safe_faults() + faults.append( + f"PIPELINE-FAULT: feedback incorporation crashed — {e}; the reviewer's " + "feedback text could not be recovered" + ) + print( + json.dumps( + { + "_next": "research_each_question", + "research_attempts": 0, + "research_feedback": ( + "The user reviewed the report and asked for changes, but " + "the feedback text could not be recovered (pipeline " + "fault). Re-run the research pass with extra care and " + "note this loss." + ), + "pipeline_faults": faults, + } + ) + ) diff --git a/assets/agents/deep-research/scripts/parse_request.py b/assets/agents/deep-research/scripts/parse_request.py index faae28c..f191811 100644 --- a/assets/agents/deep-research/scripts/parse_request.py +++ b/assets/agents/deep-research/scripts/parse_request.py @@ -9,6 +9,13 @@ supply one interactively. Routing (`_next`): - prompt present -> (no _next; static next: plan) - prompt empty -> ask_topic + +Fail-safe: a crash here (e.g. malformed GRAPH_STATE) means the +caller's prompt cannot be read at all, so proceeding silently is +meaningless — the degraded route is `ask_topic`, which asks the user for +the topic directly, plus a PIPELINE-FAULT entry so the loss is visible in +the final report's Pipeline notes. Clobbering `pipeline_faults` is safe +here: this is the start node, so the list can only hold its initial []. """ import json import os @@ -31,5 +38,17 @@ def main(): print(json.dumps({"_next": "ask_topic"})) -if __name__ == "__main__": +try: main() +except Exception as e: # noqa: BLE001 — a crashed entry router must degrade, not die + print( + json.dumps( + { + "_next": "ask_topic", + "pipeline_faults": [ + f"PIPELINE-FAULT: request parsing crashed — {e}; asked the " + "user for the research topic instead" + ], + } + ) + ) diff --git a/assets/agents/deep-research/scripts/plan_fault.py b/assets/agents/deep-research/scripts/plan_fault.py new file mode 100644 index 0000000..159fb04 --- /dev/null +++ b/assets/agents/deep-research/scripts/plan_fault.py @@ -0,0 +1,58 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for a dead planning stage of the deep-research graph. + +Reached ONLY via plan's `fallback:` route. Grounded in engine behavior: on +llm-node failure the engine writes "LLM node …failed: " into +`plan_failure` via the node's state_updates BEFORE routing to the fallback +(on success the same key holds the node's structured JSON output, which +never starts with the "LLM node" prefix). No plan means no sub-questions — +there is nothing to research — so this path is fail-closed: record the +PIPELINE-FAULT and continue to `end_fault`, which renders the +DEEP_RESEARCH FAILED sentinel. + +The planner runs inside the static fan-out [plan, knowledge_lookup]. While +this marker routes to end_fault, the sibling branch proceeds one more +super-step into `research_each_question`, whose `{{questions}}` resolves to +the initial_state [] — the doomed lane maps over zero items and the graph +returns end_fault's output at the next join. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + raw = state.get("plan_failure") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("LLM node"): + detail = "the planning stage died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + fault_text = f"planning failed — nothing to research: {detail}" + faults.append(f"PIPELINE-FAULT: {fault_text}") + print(json.dumps({"fault_text": fault_text, "pipeline_faults": faults})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + fault_text = f"planning failed — fault-marker script error: {e}" + print( + json.dumps( + { + "fault_text": fault_text, + "pipeline_faults": [f"PIPELINE-FAULT: {fault_text}"], + } + ) + ) diff --git a/assets/agents/deep-research/scripts/question_fault.py b/assets/agents/deep-research/scripts/question_fault.py new file mode 100644 index 0000000..2cb7ded --- /dev/null +++ b/assets/agents/deep-research/scripts/question_fault.py @@ -0,0 +1,70 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for a dead research lane of the deep-research graph. + +Reached ONLY via research_one_question's `fallback:` route inside the +`research_each_question` map branch (branch-local — a map branch's fallback +must stay inside the branch subgraph). Grounded in engine behavior: on +llm-node failure the engine writes "LLM node …failed: " into +`finding` via the node's state_updates BEFORE routing to the fallback (on +success the same key holds the researched findings, which never start with +the "LLM node" prefix). This node normalizes the failure text into a +PIPELINE-FAULT finding and the chain ends here — the map collects the fault +as the lane's `finding`, so one dead research lane never sinks the whole +map. combine_findings later lifts PIPELINE-FAULT findings into +`pipeline_faults` so the fault also surfaces in the report's Pipeline notes. +The question text is part of the fault so distinct dead lanes with identical +engine failure text do not collapse into one entry there. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 +MAX_QUESTION_CHARS = 200 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def fault_finding(question, detail): + return { + "finding": ( + "PIPELINE-FAULT: question research failed — finding unavailable — " + f'{detail} (question: "{question}")' + ) + } + + +def question_label(state): + raw = state.get("question") + question = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not question: + return "(unknown question)" + if len(question) > MAX_QUESTION_CHARS: + question = question[: MAX_QUESTION_CHARS - 1] + "…" + return question + + +def main(): + state = load_state() + raw = state.get("finding") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("LLM node"): + detail = "the research lane died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print(json.dumps(fault_finding(question_label(state), detail))) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print( + json.dumps( + fault_finding("(unknown question)", f"fault-marker script error: {e}") + ) + ) diff --git a/assets/agents/deep-research/scripts/reflexion_gate.py b/assets/agents/deep-research/scripts/reflexion_gate.py index 2dd1e6b..cf660e7 100644 --- a/assets/agents/deep-research/scripts/reflexion_gate.py +++ b/assets/agents/deep-research/scripts/reflexion_gate.py @@ -15,6 +15,12 @@ Routing (`_next`): Reflexion is a best-effort quality booster, not a hard gate: once the budget is spent the workflow proceeds anyway, and the human approval step is the final backstop. + +Fail-safe: a crash here fails FORWARD to `synthesize` — the same +direction as the existing malformed-critique PASS default — with a +PIPELINE-FAULT entry recorded (best-effort preserving existing faults), +so a broken gate costs the run its automated critique routing, never the +run itself. """ import json import os @@ -33,6 +39,14 @@ def load_state(): return json.loads(os.environ.get("GRAPH_STATE", "{}")) +def safe_faults(): + try: + state = load_state() + return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + except Exception: # noqa: BLE001 — best-effort preservation only + return [] + + def as_int(value, default=0): try: return int(value) @@ -72,5 +86,12 @@ def main(): print(json.dumps(output)) -if __name__ == "__main__": +try: main() +except Exception as e: # noqa: BLE001 — a crashed gate fails forward, never dies + faults = safe_faults() + faults.append( + f"PIPELINE-FAULT: reflexion gate crashed — {e}; proceeded to synthesis " + "without automated critique routing" + ) + print(json.dumps({"_next": "synthesize", "pipeline_faults": faults})) diff --git a/assets/agents/deep-research/scripts/synth_fault.py b/assets/agents/deep-research/scripts/synth_fault.py new file mode 100644 index 0000000..ea08c0f --- /dev/null +++ b/assets/agents/deep-research/scripts/synth_fault.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for a dead synthesis stage of the deep-research graph. + +Reached ONLY via synthesize's `fallback:` route. Grounded in engine +behavior: on agent-node failure the engine writes "Agent node failed: +{e:#}" into `report` via the node's state_updates BEFORE routing to the +fallback (on success the same key holds the report-writer's report, which +never starts with that prefix). No report means no product, so this path is +fail-closed: normalize the failure text, record the PIPELINE-FAULT, and +continue to `end_fault`, which renders the DEEP_RESEARCH FAILED sentinel. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + raw = state.get("report") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("Agent node failed:"): + detail = "the synthesis stage died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + fault_text = f"report synthesis failed — no report was produced: {detail}" + faults.append(f"PIPELINE-FAULT: {fault_text}") + print(json.dumps({"fault_text": fault_text, "pipeline_faults": faults})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + fault_text = f"report synthesis failed — fault-marker script error: {e}" + print( + json.dumps( + { + "fault_text": fault_text, + "pipeline_faults": [f"PIPELINE-FAULT: {fault_text}"], + } + ) + ) diff --git a/assets/agents/deep-research/scripts/verify_sources.py b/assets/agents/deep-research/scripts/verify_sources.py index 9828341..d68d515 100644 --- a/assets/agents/deep-research/scripts/verify_sources.py +++ b/assets/agents/deep-research/scripts/verify_sources.py @@ -6,6 +6,19 @@ request, and writes a `source_check` summary into state so the human reviewer sees broken citations at the approval step. Times out per request so a slow source cannot stall the graph. + +Pipeline-notes fold: end-node templates render unconditionally, so the +conditional "## Pipeline notes" section of the accepted report is built +HERE — the last stop before the approval gate on every accepted path, +including reflexion and feedback re-loops. `pipeline_notes` is the +rendered section when `pipeline_faults` is non-empty and the empty +string otherwise, keeping the happy-path `{{report}}{{pipeline_notes}}` +output byte-identical to the pre-hardening `{{report}}`. + +Fail-safe: per-URL probes were already guarded; the top level now +is too. On a crash the summary says sources were NOT checked, a +PIPELINE-FAULT is recorded (best-effort preserving existing faults), and +pipeline_notes still renders — the approval gate remains the backstop. """ import json import os @@ -25,6 +38,21 @@ def load_state(): return json.loads(os.environ.get("GRAPH_STATE", "{}")) +def safe_faults(): + try: + state = load_state() + return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + except Exception: # noqa: BLE001 — best-effort preservation only + return [] + + +def render_notes(faults): + entries = [f for f in (faults or []) if isinstance(f, str) and f.strip()] + if not entries: + return "" + return "\n\n## Pipeline notes\n\n" + "\n".join(f"- {e}" for e in entries) + + def reachable(url, timeout=5.0): req = urllib.request.Request(url, method="HEAD") try: @@ -62,8 +90,27 @@ def main(): + "\n".join(results) ) - print(json.dumps({"source_check": summary})) + notes = render_notes(state.get("pipeline_faults")) + print(json.dumps({"source_check": summary, "pipeline_notes": notes})) -if __name__ == "__main__": +try: main() +except Exception as e: # noqa: BLE001 — a crashed source check must degrade, not die + faults = safe_faults() + faults.append( + f"PIPELINE-FAULT: source verification crashed — {e}; cited sources " + "were not checked for reachability" + ) + print( + json.dumps( + { + "source_check": ( + "Source verification crashed (pipeline fault) — cited " + "sources were NOT checked for reachability." + ), + "pipeline_notes": render_notes(faults), + "pipeline_faults": faults, + } + ) + ) diff --git a/assets/agents/deep-research/scripts/vet_fault.py b/assets/agents/deep-research/scripts/vet_fault.py new file mode 100644 index 0000000..38b1586 --- /dev/null +++ b/assets/agents/deep-research/scripts/vet_fault.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +"""Degrade-visibly marker for a dead source-vetting stage of deep-research. + +Reached ONLY via vet_sources' `fallback:` route. Grounded in engine +behavior: on llm-node failure the engine writes "LLM node …failed: " +into `source_assessment` via the node's state_updates BEFORE routing to the +fallback. Vetting is a quality lane, not the product, so this path degrades +instead of failing closed: it records a PIPELINE-FAULT (surfaced later in +the report's Pipeline notes) and rewrites `source_assessment` to a neutral +"unvetted" note so downstream prompts (critique, synthesize) never carry +raw engine error text, then continues to `critique` — the stage +vet_sources' own `next` pointed to. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + +UNVETTED_NOTE = ( + "Source credibility assessment unavailable — the vetting stage failed " + "(pipeline fault); treat every cited source as unvetted and needing " + "corroboration. Do not request revision solely because this assessment " + "is unavailable — re-research cannot restore it; judge the findings on " + "their own evidence." +) + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def safe_faults(): + try: + state = load_state() + return [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + except Exception: # noqa: BLE001 — best-effort preservation only + return [] + + +def main(): + state = load_state() + faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] + raw = state.get("source_assessment") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("LLM node"): + detail = "the vetting stage died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + fault = f"PIPELINE-FAULT: source vetting failed — {detail}" + if fault not in faults: + faults.append(fault) + print(json.dumps({"pipeline_faults": faults, "source_assessment": UNVETTED_NOTE})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + faults = safe_faults() + faults.append(f"PIPELINE-FAULT: source vetting failed — fault-marker script error: {e}") + print( + json.dumps( + { + "pipeline_faults": faults, + "source_assessment": UNVETTED_NOTE, + } + ) + ) diff --git a/assets/agents/domain-reviewer/config.yaml b/assets/agents/domain-reviewer/config.yaml index 8ec24cd..3c6f548 100644 --- a/assets/agents/domain-reviewer/config.yaml +++ b/assets/agents/domain-reviewer/config.yaml @@ -23,7 +23,13 @@ enabled_skills: spawnable_agents: - file-reviewer max_concurrent_agents: 8 -max_agent_depth: 2 +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# review-gauntlet | step-runner(2) → code-reviewer(3) → domain-reviewer(4) → +# file-reviewer(5). This agent spawns depth-5 children; 6 = one level of headroom. +# NOTE: can_spawn_agents is not yet set, so the file-reviewer hop is documented +# but inert; 6 keeps the limit forward-compatible for when it is wired. +max_agent_depth: 6 variables: - name: project_dir diff --git a/assets/agents/finding-verifier/README.md b/assets/agents/finding-verifier/README.md index e86ae0f..f314e27 100644 --- a/assets/agents/finding-verifier/README.md +++ b/assets/agents/finding-verifier/README.md @@ -7,13 +7,23 @@ or confirmation-bias the check. A graph can't: the structure IS the discipline. ## How it works -``` -parse (llm) ──▶ verify_each (map, max_concurrency 4) ──▶ done (end) - └─▶ verify_one (llm + fs tools) ──▶ verdict_gate (script), one CHAIN per finding +```mermaid +flowchart TD + A["parse (llm, max_attempts 2)"] --> B["verify_each (map, max_concurrency 4): one branch per finding"] + A -. "fallback" .-> PF["parse_fault (script): verdicts = one UNVERIFIABLE PIPELINE-FAULT entry"] + PF --> D + B -. "spawns × N" .-> C["verify_one (llm + fs tools, max_attempts 2)"] + C --> G["verdict_gate (script): machine-validates the JSON contract"] + G -. "reject-retry once" .-> C + C -. "fallback" .-> VF["verify_fault (script): branch-local UNVERIFIABLE marker"] + B --> D(["done (end): FINDING_VERIFIER_RESULTS"]) ``` 1. **parse** — extracts every 🔴/🟡 finding from the spawn prompt into a structured list (id, severity verbatim, path, lines, self-contained claim). Extraction only — no judging. + Gets two attempts (`max_attempts: 2`); if both fail, its `fallback` routes to + `parse_fault`, which rewrites `verdicts` to a single UNVERIFIABLE entry naming the fault + and jumps straight to `done`. 2. **verify_each** — a `map` node fans out one `verify_one` branch per finding. Skipping a finding is structurally impossible; results collect in input order. 3. **verify_one** — reads the cited code (greps for moved code before concluding anything) @@ -28,12 +38,20 @@ parse (llm) ──▶ verify_each (map, max_concurrency 4) ──▶ done (end) contract (verdict enum; evidence required for VERIFIED/FALSE; note for UNVERIFIABLE), stamps the authoritative id FROM the finding item (the model's echo is never trusted), rejects back to `verify_one` exactly once with the reason, and on a second failure records - UNVERIFIABLE — a malformed branch never sinks the map. - Kept, marked unverified. + UNVERIFIABLE — a malformed branch never sinks the map. If `verify_one` itself dies (both + `max_attempts: 2` attempts fail), its `fallback` routes to `verify_fault`, which + normalizes the engine's failure text into a branch-local UNVERIFIABLE verdict the map + still collects. 4. **done** — a deterministic `end` node emits the raw verdict list plus the caller contract (VERIFIED = keep + paste evidence; FALSE = drop + tally; UNVERIFIABLE = keep, marked). No final LLM step that could editorialize. +The dashed edges are fault paths. All three fault emitters (`parse_fault`, `verify_fault`, +and `verdict_gate`'s own crash guard) surface as `PIPELINE-FAULT:` text in a verdict's +`note`, and a fault can only ever produce UNVERIFIABLE — degraded output is never a +passing verdict. Every path ends at `done`, so the `FINDING_VERIFIER_RESULTS` +sentinel is always emitted and the caller's verify lane degrades instead of dying. + ## What it is NOT - **Not a second opinion.** It never re-judges severity, style, or whether a finding @@ -50,3 +68,6 @@ installed, code-reviewer runs the same contract inline as a fallback. - `graph.yaml` — the agent (no tools.sh; the graph grants `fs_read`/`fs_cat`/`fs_grep` per node). +- `scripts/verdict_gate.py` — the deterministic contract gate. +- `scripts/parse_fault.py`, `scripts/verify_fault.py` — the fault markers behind the + dashed fallback edges. diff --git a/assets/agents/finding-verifier/graph.yaml b/assets/agents/finding-verifier/graph.yaml index 1adc582..cebc2c6 100644 --- a/assets/agents/finding-verifier/graph.yaml +++ b/assets/agents/finding-verifier/graph.yaml @@ -32,6 +32,7 @@ initial_state: verdicts: [] gate_feedback: '' gate_attempts: 0 + parse_failure: '' start: parse @@ -92,8 +93,20 @@ nodes: type: string description: Repository directory named in the request (echo the prompt default when none). required: [findings_list, project_dir] + max_attempts: 2 + state_updates: + parse_failure: '{{output}}' + fallback: parse_fault next: verify_each + parse_fault: + id: parse_fault + type: script + description: Fail-closed marker for a dead parse stage (reached only via parse's fallback) — rewrites `verdicts` to a single UNVERIFIABLE PIPELINE-FAULT entry and jumps straight to done, so the FINDING_VERIFIER_RESULTS sentinel still renders and the caller's verify lane degrades instead of dying. + script: scripts/parse_fault.py + timeout: 30 + next: done + verify_each: id: verify_each type: map @@ -165,6 +178,7 @@ nodes: max_attempts: 2 state_updates: verdict: '{{output}}' + fallback: verify_fault next: verdict_gate verdict_gate: @@ -174,6 +188,13 @@ nodes: script: scripts/verdict_gate.py timeout: 30 + verify_fault: + id: verify_fault + type: script + description: Fail-closed marker for a dead verify branch (reached only via verify_one's fallback; branch-local). Normalizes the engine's failure text in `verdict` into a schema-conformant UNVERIFIABLE verdict the map collects — a dead verifier lane never sinks the whole map or masquerades as verified. + script: scripts/verify_fault.py + timeout: 30 + done: id: done type: end diff --git a/assets/agents/finding-verifier/scripts/parse_fault.py b/assets/agents/finding-verifier/scripts/parse_fault.py new file mode 100644 index 0000000..7a6a8db --- /dev/null +++ b/assets/agents/finding-verifier/scripts/parse_fault.py @@ -0,0 +1,57 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for a dead parse stage of the finding-verifier graph. + +Reached ONLY via parse's `fallback:` route. parse's state_updates wrote an +"LLM node …failed: " string into `parse_failure` before routing here; +this node rewrites `verdicts` to a single schema-conformant UNVERIFIABLE +entry naming the fault and the graph jumps straight to `done` — the +FINDING_VERIFIER_RESULTS sentinel still renders, so the caller (e.g. +code-reviewer's verify lane) degrades instead of dying. Findings that were +never extracted cannot be verified; the caller's "UNVERIFIABLE = keep the +finding, marked unverified" semantics do the right thing with the entry. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def fault_verdicts(detail): + return { + "verdicts": [ + { + "id": "pipeline-fault", + "verdict": "UNVERIFIABLE", + "evidence": "", + "note": ( + "PIPELINE-FAULT: parse failed — findings could not be extracted " + f"from the verification request; no finding was verified: {detail}" + ), + } + ] + } + + +def main(): + state = load_state() + raw = state.get("parse_failure") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("LLM node"): + detail = "the parse stage died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print(json.dumps(fault_verdicts(detail))) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print(json.dumps(fault_verdicts(f"fault-marker script error: {e}"))) diff --git a/assets/agents/finding-verifier/scripts/verdict_gate.py b/assets/agents/finding-verifier/scripts/verdict_gate.py index 63085f2..451b43e 100644 --- a/assets/agents/finding-verifier/scripts/verdict_gate.py +++ b/assets/agents/finding-verifier/scripts/verdict_gate.py @@ -104,6 +104,7 @@ def main(): ) return + # Deliberately not PIPELINE-FAULT-prefixed: model misbehavior, not infrastructure. print( json.dumps( { @@ -128,7 +129,7 @@ except Exception as e: # noqa: BLE001 — the gate must never crash the chain "id": "unknown", "verdict": "UNVERIFIABLE", "evidence": "", - "note": f"verdict gate error: {e}", + "note": f"PIPELINE-FAULT: verdict gate error: {e}", } } ) diff --git a/assets/agents/finding-verifier/scripts/verify_fault.py b/assets/agents/finding-verifier/scripts/verify_fault.py new file mode 100644 index 0000000..4067c1f --- /dev/null +++ b/assets/agents/finding-verifier/scripts/verify_fault.py @@ -0,0 +1,63 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for a dead verify branch of the finding-verifier graph. + +Reached ONLY via verify_one's `fallback:` route inside the map branch. +Grounded in engine behavior: on llm-node failure the engine writes +"LLM node failed: " into `verdict` via the node's state_updates +BEFORE routing to the fallback, so the raw failure text is already in +state. This node normalizes it into a schema-conformant UNVERIFIABLE +verdict (authoritative id stamped from the finding item, mirroring +verdict_gate.py) and the chain ends there — the map collects a valid +verdict, so a dead verifier lane never sinks the whole map or masquerades +as verified. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def finding_id(state): + finding = state.get("finding") + if isinstance(finding, dict): + fid = finding.get("id") + if isinstance(fid, str) and fid.strip(): + return fid.strip() + return "unknown" + + +def fault_verdict(fid, detail): + return { + "verdict": { + "id": fid, + "verdict": "UNVERIFIABLE", + "evidence": "", + "note": f"PIPELINE-FAULT: verifier lane failed after retries — {detail}", + } + } + + +def main(): + state = load_state() + fid = finding_id(state) + raw = state.get("verdict") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if not detail.startswith("LLM node"): + detail = "the verifier lane died without recording a failure detail" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print(json.dumps(fault_verdict(fid, detail))) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print(json.dumps(fault_verdict("unknown", f"fault-marker script error: {e}"))) diff --git a/assets/agents/review-gauntlet/README.md b/assets/agents/review-gauntlet/README.md index c372131..39785f6 100644 --- a/assets/agents/review-gauntlet/README.md +++ b/assets/agents/review-gauntlet/README.md @@ -11,13 +11,21 @@ gets the chance to rationalize past it. ```mermaid flowchart TD parse["parse
(llm: extract request facts)"] --> signals["signals
(script: deterministic diff facts)"] + parse -. "fallback" .-> pfault["parse_fault
(script: PIPELINE-FAULT marker)"] + pfault --> gate signals --> select["select_lanes
(llm: ADDITIVE-only judgment)"] select --> build["build_items
(script: rules floor ∪ additions;
forced lanes override exactly)"] + select -. "fallback" .-> dlanes["default_lanes
(script: deterministic degraded
selection — wider, never narrower)"] + dlanes --> build build --> mcr["map: code-review"] & madv["map: adversary"] & msec["map: security"] & mpb["map: probe"] - mcr -->|"over: 0-or-1 items"| rcr[["code-reviewer"]] - madv --> radv[["adversary"]] - msec --> rsec[["security-reviewer"]] - mpb --> rpb[["probe"]] + mcr -->|"over: 0-or-1 items"| rcr[["code-reviewer
(max_attempts: 2)"]] + madv --> radv[["adversary
(max_attempts: 2)"]] + msec --> rsec[["security-reviewer
(max_attempts: 2)"]] + mpb --> rpb[["probe
(max_attempts: 2)"]] + rcr -. "fallback" .-> lfault["lane_fault
(script: PIPELINE-FAULT lane result,
collected by the map)"] + radv -. "fallback" .-> lfault + rsec -. "fallback" .-> lfault + rpb -. "fallback" .-> lfault mcr & madv & msec & mpb --> gate["verdict_gate
(script: sentinel regex — deterministic)"] gate --> done(["GAUNTLET: PASS | BLOCKED"]) ``` @@ -43,18 +51,37 @@ those rules are *structure*: (map-over-nothing), which *is* the skip mechanism. All four maps join at the verdict gate on equal-length paths. - **The verdict is a regex, not an opinion** (`verdict_gate.py`): a missing - sentinel is a lane FAILURE, never a pass; any 🔴 in the code-review report - blocks regardless of its verdict line; `NEEDS-HUMAN` without 🔴 passes but + sentinel is a lane FAILURE, never a pass; the critical count from the + code-review report's summary line (raw 🔴 count if absent) blocks + regardless of its verdict line; a code-review lane whose own verdict line + records a pipeline fault blocks (a degraded review is never a pass); + `NEEDS-HUMAN` without 🔴 passes but is surfaced under *Human attention required*; probe `INCONCLUSIVE` blocks with an environment note — it is never treated as PASS or FAIL. - **Verdict independence is structural**: lanes run as isolated sub-agents with no `teammates:` flag — no cross-lane messaging, by construction. +- **Failure fails closed, structurally** (`fallback:` routes + `PIPELINE-FAULT` + markers): every LLM node and lane retries once (`max_attempts: 2`), then a + fallback fires instead of killing the graph — and every fallback degrades + toward BLOCKED, never toward a pass. A dead lane becomes a `PIPELINE-FAULT:` + lane result (`lane_fault.py`) that the gate reports as BLOCKED *naming the + lane* — distinct from SKIPPED (not selected). A dead `parse` records a fault + and jumps straight to the gate (`parse_fault.py`). A dead `select_lanes` + degrades to a deterministic selection (`default_lanes.py`): code-review + + adversary, plus probe on consumer surface *with a local-run recipe*, plus + security on auth/deps/exec + signals or hardened posture, unioned with caller-forced lanes — wider, never + narrower. A crashed `build_items` records a fault so an all-SKIPPED gate + still blocks. Fault detection is prefix-anchored — a real review that merely + *quotes* "PIPELINE-FAULT:" flows to the normal sentinel rules. And the + `GAUNTLET:` sentinel is always emitted: even a crashed gate prints + `GAUNTLET: BLOCKED`, never silence. ## Sentinels parsed | Lane | Sentinel | Blocks on | |------|----------|-----------| -| code-reviewer | `**Verdict: MERGE-READY \| NEEDS-HUMAN**` | any 🔴 finding; missing sentinel | +| code-reviewer | `**Verdict: MERGE-READY \| NEEDS-HUMAN**` | critical count from the report's summary line (raw 🔴 count if absent); missing sentinel; own verdict line records a pipeline fault | | adversary | `ADVERSARIAL_REVIEW: CONFORMS \| DIVERGES` | DIVERGES; missing sentinel | | security-reviewer | `SECURITY_REVIEW: PASS \| FAIL` | FAIL; missing sentinel | | probe | `USAGE_PROBE: PASS \| FAIL \| INCONCLUSIVE` | FAIL; INCONCLUSIVE (environment note); missing sentinel | @@ -74,7 +101,18 @@ Plan / acceptance criteria: Local-run recipe / usage suites: -Forced lanes: (optional — e.g. 'adversary, probe' to run exactly those)" +Forced lanes: (optional — e.g. 'adversary, probe' to run exactly those)" \ + --variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"} +``` + +Verification commands are NOT part of the prompt. They are the declared +`verification_commands` graph variable (default `'[]'`; a JSON array of shell +commands as a string, since agent variables arrive as strings), also settable +from the CLI: + +```sh +coyote -a review-gauntlet --agent-variable project_dir /abs/path/to/repo \ + --agent-variable verification_commands '["cargo test --all"]' "" ``` The reply ends with `GAUNTLET: PASS` or `GAUNTLET: BLOCKED`, a lane status @@ -89,9 +127,24 @@ final report verbatim. - The caller decides *whether* review is warranted at all (the gauntlet is for non-trivial work — that's why code-review is unconditionally on); the gauntlet decides *which* of the other lanes apply and what the verdicts mean. -- Lane sub-agents get generous finite timeouts (2h each; 3h whole-gauntlet). - A lane that dies or times out produces no sentinel — which is a BLOCKED, - never a silent pass. +- Lane sub-agents get generous finite timeouts (2h each; 3h whole-gauntlet) + and one retry (`max_attempts: 2`). A lane that still dies or times out falls + back to the `lane_fault` marker — BLOCKED naming the lane, never a silent + pass. +- The `verification_commands` variable is forwarded to the adversary lane as a + structured `inputs:` passthrough on `run_adversary` (a lone `{{…}}` template + lands the raw value in the child's state) — never through the lane prompt + and never extracted by `parse`. The adversary's `run_checks` stage executes + those commands with a shell, which makes the declaration a trust boundary: + the prompt also carries plan text pasted from the repo under review, so + commands must never be inferred from it, and the engine merges only the keys + `parse`'s `output_schema.properties` declares, so an extra + `verification_commands` key emitted by the LLM is dropped before it reaches + state. + Undeclared stays undeclared + (`'[]'` ⇒ "none declared"); a declaration that is not a JSON array of + strings executes nothing and records a `PIPELINE-FAULT`, which the + adversary lane reports as DIVERGES and the gate as BLOCKED. - Signals failures degrade gracefully: if git can't compute the diff, lane selection falls back to caller context + forced lanes, and the report notes it. The auth-path regex deliberately over-fires (`auth(?!or)` matches diff --git a/assets/agents/review-gauntlet/graph.yaml b/assets/agents/review-gauntlet/graph.yaml index 54b3e74..0c08bb8 100644 --- a/assets/agents/review-gauntlet/graph.yaml +++ b/assets/agents/review-gauntlet/graph.yaml @@ -19,6 +19,11 @@ description: | version: '1.0' +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# review-gauntlet(2) → code-reviewer(3) → +# {domain-reviewer(4) → file-reviewer(5) | finding-verifier(4)}. +# This agent spawns depth-3 children; 4 = one level of headroom. max_agent_depth: 4 max_concurrent_agents: 12 @@ -26,11 +31,23 @@ variables: - name: project_dir description: Repository the change lives in; signals and every review lane run against it. default: '.' + - name: verification_commands + description: | + JSON array of shell commands the CALLER declares for the deterministic + run_checks stage — e.g. '["cargo test --all"]'. Never inferred from the + prompt; empty array = none declared. + default: '[]' settings: max_loop_iterations: 10 log_state_snapshots: true - timeout: 10800 + # max over agent lanes of (max_attempts × lane timeout) = 2 × 11400 = 22800 + # (agent-node retries each get a fresh per-attempt budget, + # src/graph/agent.rs retry_transient/bounded_attempt) + 1200 margin for + # the script stages (signals/select/build/gate) = 24000; the parse and + # select_lanes llm nodes carry no `timeout:` and are bounded by + # max_iterations. + timeout: 24000 initial_state: diff_spec: worktree @@ -56,6 +73,9 @@ initial_state: add_security: false add_probe: false lane_reasons: '' + parse_failure: '' + select_lanes_failure: '' + lanes_degraded: '' code_review_items: [] adversary_items: [] security_items: [] @@ -129,8 +149,19 @@ nodes: required: [diff_spec, plan_context, rigor, security_posture, probe_context, forced_lanes, extra_context, project_dir_in] max_iterations: 3 max_attempts: 2 + state_updates: + parse_failure: '{{output}}' + fallback: parse_fault next: signals + parse_fault: + id: parse_fault + type: script + description: Fail-closed marker for a dead parse stage (reached only via parse's fallback) — records a PIPELINE-FAULT in signals_error and jumps straight to the verdict gate, which blocks on it. Lane item lists stay their initial [] so every lane records SKIPPED. + script: scripts/parse_fault.py + timeout: 30 + next: verdict_gate + signals: id: signals type: script @@ -204,6 +235,17 @@ nodes: required: [add_code_review, add_adversary, add_security, add_probe, lane_reasons] max_iterations: 3 max_attempts: 2 + state_updates: + select_lanes_failure: '{{output}}' + fallback: default_lanes + next: build_items + + default_lanes: + id: default_lanes + type: script + description: Deterministic lane-selection fallback (reached only via select_lanes' fallback) — when the additive LLM judgment dies, forces code-review + adversary, plus probe when consumer_surface AND a probe_context (local-run recipe) is present, plus security on attack-surface signals (auth/deps/exec) or hardened posture, unioned with any caller-forced lanes. Selection degrades WIDER, never narrower; the degradation note flows into lanes_summary via build_items. + script: scripts/default_lanes.py + timeout: 30 next: build_items build_items: @@ -263,7 +305,12 @@ nodes: type: agent description: Independent code-quality review lane. agent: code-reviewer - timeout: 7200 + # Exceeds the code-reviewer graph's own settings.timeout (10800) by 600 so + # the child's graph timeout, not this lane timeout, is the binding bound. + # NOTE: the child's own internal retry envelope (its nodes' max_attempts × + # timeouts) is not derived here — deliberately out of scope for this + # change; tracked as a follow-up. + timeout: 11400 prompt: | Review the code changes in {{project_dir}} (diff: {{diff_spec}}) as an independent post-implementation review. @@ -276,15 +323,24 @@ nodes: {{extra_context}} Close your report with your Review Verdict line. + max_attempts: 2 state_updates: lane_out: '{{output}}' + fallback: lane_fault run_adversary: id: run_adversary type: agent description: Plan-conformance review lane. agent: adversary - timeout: 7200 + # Exceeds the adversary graph's own settings.timeout (9000) by 600 so the + # child's graph timeout, not this lane timeout, is the binding bound. + # NOTE: the child's own internal retry envelope (its nodes' max_attempts × + # timeouts) is not derived here — deliberately out of scope for this + # change; tracked as a follow-up. + timeout: 9600 + inputs: + verification_commands: '{{verification_commands}}' prompt: | Adversarial plan-conformance review in {{project_dir}}. @@ -293,11 +349,15 @@ nodes: The spec / plan / acceptance criteria it must satisfy: {{plan_context}} + Verification commands arrive as a structured input (the verification_commands variable), not in this prompt. + {{extra_context}} Return your ADVERSARIAL_REVIEW verdict. + max_attempts: 2 state_updates: lane_out: '{{output}}' + fallback: lane_fault run_security: id: run_security @@ -313,8 +373,10 @@ nodes: {{extra_context}} Return your SECURITY_REVIEW verdict. + max_attempts: 2 state_updates: lane_out: '{{output}}' + fallback: lane_fault run_probe: id: run_probe @@ -332,16 +394,28 @@ nodes: Plan / acceptance-criteria context: {{plan_context}} + The workspace may contain test files from an aborted earlier attempt of + this lane; reconcile rather than duplicate. + Derive expected behaviors from the plan/contract BEFORE reading implementation code, run existing suites first, then author spec-first tests for the uncovered delta. Return your USAGE_PROBE verdict. + max_attempts: 2 state_updates: lane_out: '{{output}}' + fallback: lane_fault + + lane_fault: + id: lane_fault + type: script + description: Fail-closed lane fault marker (reached only via the run_* lanes' fallback routes; shared across the four map branches). Normalizes the engine's "Agent node failed" text in lane_out into a PIPELINE-FAULT lane result the map collects; verdict_gate turns it into BLOCKED — a degraded lane is never a pass. + script: scripts/lane_fault.py + timeout: 30 verdict_gate: id: verdict_gate type: script - description: Deterministic sentinel parser and gate. A missing sentinel is a lane failure, never a pass; probe INCONCLUSIVE blocks with an environment note; 🔴 findings block regardless of the lane's own verdict line. + description: Deterministic sentinel parser and gate. A missing sentinel is a lane failure, never a pass; probe INCONCLUSIVE blocks with an environment note; the code-review critical count is read from render.py's summary line (raw 🔴 count if absent) and blocks regardless of the lane's own verdict line; a code-review verdict line that records the nested graph's own pipeline fault blocks as a degraded lane. script: scripts/verdict_gate.py timeout: 60 next: done diff --git a/assets/agents/review-gauntlet/scripts/build_items.py b/assets/agents/review-gauntlet/scripts/build_items.py index 4a31e58..f8198eb 100644 --- a/assets/agents/review-gauntlet/scripts/build_items.py +++ b/assets/agents/review-gauntlet/scripts/build_items.py @@ -8,20 +8,17 @@ skip mechanism — no dynamic routing anywhere. The LLM's add_* flags can only ADD lanes. A flag that is false never removes a lane the hard rules selected. + +Fail-closed: a builder crash emits all four lists empty plus a +PIPELINE-FAULT in signals_error — verdict_gate blocks on it and the crash +never kills the graph. """ import json import os -def load_state(): - if path := os.environ.get("GRAPH_STATE_FILE"): - with open(path) as f: - return json.load(f) - return json.loads(os.environ.get("GRAPH_STATE", "{}")) - - -state = load_state() - +# Keep in sync with default_lanes.ALIASES — a caller-forced lane must +# canonicalize the same way on the degraded path as it does here. ALIASES = { "code-review": "code-review", "code_review": "code-review", @@ -39,80 +36,113 @@ ALIASES = { } -def text(key): - v = state.get(key) - return v.strip() if isinstance(v, str) else "" +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) -forced_raw = state.get("forced_lanes") or [] -forced, unknown = [], [] -for lane in forced_raw: - canon = ALIASES.get(str(lane).strip().lower()) - (forced.append(canon) if canon else unknown.append(str(lane))) +def main(): + state = load_state() -reasons = [] -if forced: - lanes = set(forced) - reasons.append(f"forced lanes honored exactly: {sorted(lanes)}") -else: - lanes = {"code-review"} - reasons.append( - "code-review: always on — the gauntlet is only spawned for non-trivial work" - ) - if text("plan_context"): - lanes.add("adversary") - reasons.append("adversary: a spec/plan/acceptance-criteria context is present") - if ( - state.get("touches_auth") - or state.get("touches_deps") - or state.get("touches_exec") - or state.get("security_posture") == "hardened" - ): - lanes.add("security") + def text(key): + v = state.get(key) + return v.strip() if isinstance(v, str) else "" + + forced_raw = state.get("forced_lanes") or [] + forced, unknown = [], [] + for lane in forced_raw: + canon = ALIASES.get(str(lane).strip().lower()) + (forced.append(canon) if canon else unknown.append(str(lane))) + + reasons = [] + if forced: + lanes = set(forced) + reasons.append(f"forced lanes honored exactly: {sorted(lanes)}") + else: + lanes = {"code-review"} reasons.append( - "security: attack-surface signals (auth/deps/exec) or hardened posture" + "code-review: always on — the gauntlet is only spawned for non-trivial work" ) - if state.get("consumer_surface") and text("probe_context"): - lanes.add("probe") - reasons.append( - "probe: consumer-facing surface changed and a local-run recipe/suite pointer exists" + if text("plan_context"): + lanes.add("adversary") + reasons.append("adversary: a spec/plan/acceptance-criteria context is present") + if ( + state.get("touches_auth") + or state.get("touches_deps") + or state.get("touches_exec") + or state.get("security_posture") == "hardened" + ): + lanes.add("security") + reasons.append( + "security: attack-surface signals (auth/deps/exec) or hardened posture" + ) + if state.get("consumer_surface") and text("probe_context"): + lanes.add("probe") + reasons.append( + "probe: consumer-facing surface changed and a local-run recipe/suite pointer exists" + ) + + # LLM judgment is additive-only. + llm_why = text("lane_reasons")[:300] + for flag, lane in ( + ("add_code_review", "code-review"), + ("add_adversary", "adversary"), + ("add_security", "security"), + ("add_probe", "probe"), + ): + if state.get(flag) and lane not in lanes: + if lane == "probe" and not text("probe_context"): + reasons.append( + "probe: requested by lane judgment but skipped — no local-run " + "recipe/suite pointer (a probe without one is guaranteed INCONCLUSIVE)" + ) + continue + if lane == "adversary" and not text("plan_context"): + reasons.append( + "adversary: requested by lane judgment but skipped — no spec/plan " + "to check conformance against" + ) + continue + lanes.add(lane) + reasons.append(f"{lane}: added by lane-selection judgment — {llm_why}") + + if unknown: + reasons.append(f"ignored unknown forced lane name(s): {unknown}") + + # A degraded lane selection (default_lanes fallback) must survive into the + # final report; lanes_summary is the only channel this script doesn't + # overwrite silently. + degraded = text("lanes_degraded") + if degraded: + reasons.insert(0, degraded) + + print( + json.dumps( + { + "code_review_items": [{"lane": "code-review"}] if "code-review" in lanes else [], + "adversary_items": [{"lane": "adversary"}] if "adversary" in lanes else [], + "security_items": [{"lane": "security"}] if "security" in lanes else [], + "probe_items": [{"lane": "probe"}] if "probe" in lanes else [], + "lanes_summary": "\n".join(f"- {r}" for r in reasons), + } + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — a builder crash must never kill the graph + print( + json.dumps( + { + "code_review_items": [], + "adversary_items": [], + "security_items": [], + "probe_items": [], + "lanes_summary": "", + "signals_error": f"PIPELINE-FAULT: lane builder crashed — {e}; no lanes were run", + } ) - - # LLM judgment is additive-only. - llm_why = text("lane_reasons")[:300] - for flag, lane in ( - ("add_code_review", "code-review"), - ("add_adversary", "adversary"), - ("add_security", "security"), - ("add_probe", "probe"), - ): - if state.get(flag) and lane not in lanes: - if lane == "probe" and not text("probe_context"): - reasons.append( - "probe: requested by lane judgment but skipped — no local-run " - "recipe/suite pointer (a probe without one is guaranteed INCONCLUSIVE)" - ) - continue - if lane == "adversary" and not text("plan_context"): - reasons.append( - "adversary: requested by lane judgment but skipped — no spec/plan " - "to check conformance against" - ) - continue - lanes.add(lane) - reasons.append(f"{lane}: added by lane-selection judgment — {llm_why}") - -if unknown: - reasons.append(f"ignored unknown forced lane name(s): {unknown}") - -print( - json.dumps( - { - "code_review_items": [{"lane": "code-review"}] if "code-review" in lanes else [], - "adversary_items": [{"lane": "adversary"}] if "adversary" in lanes else [], - "security_items": [{"lane": "security"}] if "security" in lanes else [], - "probe_items": [{"lane": "probe"}] if "probe" in lanes else [], - "lanes_summary": "\n".join(f"- {r}" for r in reasons), - } ) -) diff --git a/assets/agents/review-gauntlet/scripts/default_lanes.py b/assets/agents/review-gauntlet/scripts/default_lanes.py new file mode 100644 index 0000000..ffaeabf --- /dev/null +++ b/assets/agents/review-gauntlet/scripts/default_lanes.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +"""Deterministic lane-selection fallback for the review gauntlet. + +Reached ONLY via select_lanes' `fallback:` route, when the additive LLM +judgment dies after retries. Emits `forced_lanes` — code-review + adversary, +plus probe when the deterministic signals flagged a consumer-facing surface +AND a local-run recipe exists (build_items gates probe the same way — a +probe without a recipe is INCONCLUSIVE by construction), +plus security when the attack-surface signals (auth/deps/exec) or a hardened +posture demand it, unioned with any lanes the caller forced (canonicalized +with the same aliases build_items uses) — which build_items honors exactly, +so selection degrades WIDER, never narrower. The degradation note goes into +`lanes_degraded`; build_items folds it into lanes_summary so the final +report names the degradation. + +Fail-closed: if the fallback itself crashes it emits the widest safe +selection (defaults ∪ security) plus a PIPELINE-FAULT in signals_error so +verdict_gate blocks — a crashed fallback never narrows selection silently. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + +# Keep in sync with build_items.ALIASES — a caller-forced lane must +# canonicalize the same way here as it would on the non-degraded path. +ALIASES = { + "code-review": "code-review", + "code_review": "code-review", + "code-reviewer": "code-review", + "adversary": "adversary", + "adversarial": "adversary", + "adversarial-review": "adversary", + "security": "security", + "security-review": "security", + "security-reviewer": "security", + "probe": "probe", + "usage": "probe", + "usage-pattern": "probe", + "usage-pattern-testing": "probe", +} + +# Stable emission order keeps the fallback deterministic. +LANE_ORDER = ["code-review", "adversary", "security", "probe"] + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + lanes = {"code-review", "adversary"} + probe_context = state.get("probe_context") + if ( + state.get("consumer_surface") + and isinstance(probe_context, str) + and probe_context.strip() + ): + lanes.add("probe") + # The deterministic security signals are already in state when this + # fallback runs; a degraded selection must never drop the security lane + # build_items' hard rules would have selected. + if ( + state.get("touches_auth") + or state.get("touches_deps") + or state.get("touches_exec") + or state.get("security_posture") == "hardened" + ): + lanes.add("security") + # Caller-forced lanes survive degradation — union, never overwrite. + for lane in state.get("forced_lanes") or []: + if canon := ALIASES.get(str(lane).strip().lower()): + lanes.add(canon) + forced = [lane for lane in LANE_ORDER if lane in lanes] + raw = state.get("select_lanes_failure") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + note = "lane selection degraded to deterministic defaults" + if detail: + note += f" — {detail}" + print(json.dumps({"forced_lanes": forced, "lanes_degraded": note})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fallback itself must never crash + print( + json.dumps( + { + "forced_lanes": ["code-review", "adversary", "security"], + "lanes_degraded": "lane selection degraded to deterministic " + f"defaults (fallback script error: {e})", + "signals_error": ( + f"PIPELINE-FAULT: lane-selection fallback crashed — {e}; " + "lanes widened to defaults ∪ security" + ), + } + ) + ) diff --git a/assets/agents/review-gauntlet/scripts/lane_fault.py b/assets/agents/review-gauntlet/scripts/lane_fault.py new file mode 100644 index 0000000..56a53ff --- /dev/null +++ b/assets/agents/review-gauntlet/scripts/lane_fault.py @@ -0,0 +1,59 @@ +#!/usr/bin/env python3 +"""Fail-closed lane fault marker for the review gauntlet. + +Reached ONLY via the run_* lane nodes' `fallback:` routes inside their map +branches. Grounded in engine behavior: on agent-node failure the engine +writes "Agent node failed: {e:#}" into `lane_out` via the node's +state_updates BEFORE routing to the fallback, so the raw failure text is +already in state. This node normalizes it into a +"PIPELINE-FAULT: lane failed after retries — …" marker that the map +collects as the lane result; verdict_gate turns any PIPELINE-FAULT lane +result into BLOCKED — a degraded lane can never read as a pass. + +Lane identity comes from `lane_ctx` (the map item build_items emitted, which +carries a `lane` field); a missing/malformed item degrades to "unknown". +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + lane = "unknown" + lane_ctx = state.get("lane_ctx") + if isinstance(lane_ctx, dict): + name = lane_ctx.get("lane") + if isinstance(name, str) and name.strip(): + lane = name.strip() + raw = state.get("lane_out") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print( + json.dumps( + {"lane_out": f"PIPELINE-FAULT: {lane} lane failed after retries — {detail}"} + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print( + json.dumps( + { + "lane_out": "PIPELINE-FAULT: lane failed after retries — " + f"fault-marker script error: {e}" + } + ) + ) diff --git a/assets/agents/review-gauntlet/scripts/parse_fault.py b/assets/agents/review-gauntlet/scripts/parse_fault.py new file mode 100644 index 0000000..7ace95d --- /dev/null +++ b/assets/agents/review-gauntlet/scripts/parse_fault.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 +"""Fail-closed marker for a dead parse stage of the review gauntlet. + +Reached ONLY via parse's `fallback:` route. parse's state_updates wrote an +"LLM node …failed: " string into `parse_failure` before routing here; +this node records the failure as a PIPELINE-FAULT in `signals_error` and the +graph jumps straight to the verdict gate. Every lane item list is still its +initial [] so every lane records SKIPPED, and verdict_gate's signals_error +rule makes the verdict BLOCKED — a gauntlet that could not even parse its +request never reads as a pass. +""" + +import json +import os + +MAX_DETAIL_CHARS = 500 + + +def load_state(): + if path := os.environ.get("GRAPH_STATE_FILE"): + with open(path) as f: + return json.load(f) + return json.loads(os.environ.get("GRAPH_STATE", "{}")) + + +def main(): + state = load_state() + raw = state.get("parse_failure") + detail = raw.strip().replace("\n", " ") if isinstance(raw, str) else "" + if len(detail) > MAX_DETAIL_CHARS: + detail = detail[: MAX_DETAIL_CHARS - 1] + "…" + print( + json.dumps( + { + "signals_error": "PIPELINE-FAULT: parse failed — cannot run the " + f"gauntlet: {detail}" + } + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — the fault marker itself must never crash + print( + json.dumps( + { + "signals_error": "PIPELINE-FAULT: parse failed — cannot run the " + f"gauntlet: fault-marker script error: {e}" + } + ) + ) diff --git a/assets/agents/review-gauntlet/scripts/verdict_gate.py b/assets/agents/review-gauntlet/scripts/verdict_gate.py index d1eea2d..f995a0d 100644 --- a/assets/agents/review-gauntlet/scripts/verdict_gate.py +++ b/assets/agents/review-gauntlet/scripts/verdict_gate.py @@ -5,8 +5,21 @@ Parses each configured lane's verdict SENTINEL with a regex — no LLM re-reads, no judgment. The gate's hard rules: - a missing sentinel is a LANE FAILURE and blocks — never a pass; - - any 🔴 finding in the code-review report blocks regardless of the - lane's own verdict line; + - a non-string lane result (the map collected an object instead of the + lane's rendered text) blocks — it cannot carry a sentinel; + - a PIPELINE-FAULT lane result (the lane's agent died after retries; + detected only when the report IS the lane_fault.py marker, i.e. it + STARTS with it — a real review merely quoting the marker flows to the + normal rules) or a PIPELINE-FAULT in signals_error (builder/parse + fault) blocks — a degraded pipeline is never a pass; + - a code-review lane whose OWN verdict line records a pipeline fault + (code-reviewer's verdict.py / render.py degraded-run wording) blocks — + the nested graph completed, but with a dead domain lane, verifier, or + synthesis behind it, so its critical count is not trustworthy; + - a non-zero critical count in the code-review report blocks regardless + of the lane's own verdict line — derived from render.py's deterministic + summary line (`*Reviewed N files, found X critical, …*`); the raw 🔴 + count is used only when that summary line is missing; - NEEDS-HUMAN without 🔴 passes but is surfaced as attention required; - probe INCONCLUSIVE blocks with an ENVIRONMENT note — it is never treated as PASS or FAIL (fix the environment per the plan's local-run @@ -29,29 +42,98 @@ def load_state(): def main(): state = load_state() blockers, attention, lane_lines, reports = [], [], [], [] + malformed = set() def lane_report(key): arr = state.get(key) or [] if not arr: return None item = arr[0] - return item if isinstance(item, str) else json.dumps(item) + if isinstance(item, str): + return item + malformed.add(key) + return json.dumps(item) def record(lane, status, detail=""): lane_lines.append(f"| {lane} | {status} | {detail} |") + def fault_excerpt(report): + line = next( + (ln for ln in report.splitlines() if "PIPELINE-FAULT:" in ln), report + ) + return line.strip()[:200] + + def is_lane_fault(report): + # A genuine lane fault is the ENTIRE report emitted by lane_fault.py, + # which always begins with the marker. Matching the marker anywhere + # would let a real review that merely QUOTES "PIPELINE-FAULT:" bypass + # 🔴 counting and sentinel parsing. + return report.strip().startswith("PIPELINE-FAULT:") + + def lane_fault_blocker(lane, report): + blockers.append( + f"{lane}: PIPELINE-FAULT — the lane failed after retries and produced " + f"no verdict; a degraded lane is never a pass ({fault_excerpt(report)})" + ) + record(lane, "BLOCKED", "PIPELINE-FAULT (lane failed)") + + def malformed_blocker(lane, report): + blockers.append(f"{lane}: malformed lane result (non-string item) — never a pass") + record(lane, "BLOCKED", "malformed lane result (non-string)") + reports.append((lane, report)) + # --- code-review ------------------------------------------------------ cr = lane_report("code_review_results") if cr is None: record("code-review", "SKIPPED", "not selected") + elif "code_review_results" in malformed: + malformed_blocker("code-review", cr) + elif is_lane_fault(cr): + lane_fault_blocker("code-review", cr) + reports.append(("code-review", cr)) else: reports.append(("code-review", cr)) m = re.search(r"Verdict:\**\s*\**\s*(MERGE-READY|NEEDS-HUMAN)", cr) - reds = cr.count("🔴") + # code-reviewer's verdict.py forces NEEDS-HUMAN on its own internal + # faults; render.py emits that as `**Verdict: NEEDS-HUMAN** — ` + # (CONTRACT comments at both emitters). Scoped to the FIRST rendered + # verdict line — render.py emits it before any finding body — so a + # finding that quotes the wording cannot trip it. + verdict_line = re.search(r"^\*\*Verdict: .*$", cr, re.MULTILINE) + m_fault = verdict_line and re.search( + r"^\*\*Verdict: NEEDS-HUMAN\*\* — .*" + r"(pipeline fault\(s\) recorded|PIPELINE-FAULT:|report rendering error)", + verdict_line.group(0), + ) + # Same quoted-marker class as the PIPELINE-FAULT prefix anchoring: a + # 🔴 quoted inside finding text or a Changes-table row is not a + # finding. render.py's summary line is the authoritative count; the + # raw count is the fail-closed fallback when that line is absent. The + # LAST match wins: render.py emits the line after every finding body, + # so a quoted look-alike earlier in the report cannot shadow it. The + # pattern is the FULL shape of render.py's line (CONTRACT comment + # there) so a partial look-alike in LLM-authored text never matches. + sums = re.findall( + r"^\*Reviewed \d+ files, found (\d+) critical, \d+ warnings, \d+ suggestions, " + r"\d+ nitpicks \(\d+ deferred by quality bar\)\*$", + cr, + re.MULTILINE, + ) + reds = int(sums[-1]) if sums else cr.count("🔴") + if m_fault: + blockers.append( + "code-review: the lane reported an internal PIPELINE-FAULT " + "(degraded review) — a degraded lane is never a pass" + ) if reds: blockers.append( f"code-review: {reds} 🔴 CRITICAL finding(s) — fix before claiming done" ) + if m_fault and reds: + record("code-review", "BLOCKED", f"PIPELINE-FAULT (degraded lane); {reds} 🔴") + elif m_fault: + record("code-review", "BLOCKED", "PIPELINE-FAULT (degraded lane)") + elif reds: record("code-review", "BLOCKED", f"{reds} 🔴 finding(s)") elif m and m.group(1) == "MERGE-READY": record("code-review", "GREEN", "MERGE-READY") @@ -72,6 +154,11 @@ def main(): adv = lane_report("adversary_results") if adv is None: record("adversary", "SKIPPED", "not selected") + elif "adversary_results" in malformed: + malformed_blocker("adversary", adv) + elif is_lane_fault(adv): + lane_fault_blocker("adversary", adv) + reports.append(("adversary", adv)) else: reports.append(("adversary", adv)) m = re.search(r"ADVERSARIAL_REVIEW:\s*(CONFORMS|DIVERGES)", adv) @@ -92,6 +179,11 @@ def main(): sec = lane_report("security_results") if sec is None: record("security", "SKIPPED", "not selected") + elif "security_results" in malformed: + malformed_blocker("security", sec) + elif is_lane_fault(sec): + lane_fault_blocker("security", sec) + reports.append(("security", sec)) else: reports.append(("security", sec)) m = re.search(r"SECURITY_REVIEW:\s*(PASS|FAIL)", sec) @@ -110,6 +202,11 @@ def main(): pb = lane_report("probe_results") if pb is None: record("probe", "SKIPPED", "not selected") + elif "probe_results" in malformed: + malformed_blocker("probe", pb) + elif is_lane_fault(pb): + lane_fault_blocker("probe", pb) + reports.append(("probe", pb)) else: reports.append(("probe", pb)) m = re.search(r"USAGE_PROBE:\s*(PASS|FAIL|INCONCLUSIVE)", pb) @@ -131,6 +228,12 @@ def main(): ) record("probe", "BLOCKED", "missing verdict sentinel") + # A gauntlet-level fault (build_items crash, dead parse stage) means the + # lanes above were never built — SKIPPED rows alone must not read as PASS. + signals_error = state.get("signals_error") + if isinstance(signals_error, str) and "PIPELINE-FAULT:" in signals_error: + blockers.append(f"pipeline: {signals_error}") + verdict = "BLOCKED" if blockers else "PASS" md = "\n".join( diff --git a/assets/agents/sisyphus/README.md b/assets/agents/sisyphus/README.md index 52af04b..8c91715 100644 --- a/assets/agents/sisyphus/README.md +++ b/assets/agents/sisyphus/README.md @@ -57,6 +57,7 @@ Spawnable sub-agents (from `config.yaml`): - 💻 **CLI Coding**: Provides a natural language interface for writing and editing code. - 🔄 **Task Management**: Tracks progress and context across complex operations. - 🛠️ **Tool Integration**: Seamlessly uses system tools for building, testing, and file manipulation. +- 🧵 **Task-queue bursts**: Dependency-shaped fan-outs (explore → code → verify) may be encoded as `agent__task_create` chains — dependents auto-dispatch when `agent__task_complete` unblocks them; todos remain the tracking source of truth. - 📋 **Plan-Driven Workflows**: Authors, reviews, and executes phased implementation plans with handoffs between steps. - 🚦 **Escalation policy**: `escalation_policy=autonomous` pre-delegates decision-making — standing rulings (`~/.config/coyote/share/standing-rulings.md`, recorded via `coyote --macro record-ruling`) and evidence-backed recommended options are taken without pausing and batched into a “Decisions taken autonomously” digest; secrets, money movement, and destructive actions always escalate. diff --git a/assets/agents/sisyphus/config.yaml b/assets/agents/sisyphus/config.yaml index 57ec5c0..d5c5f6f 100644 --- a/assets/agents/sisyphus/config.yaml +++ b/assets/agents/sisyphus/config.yaml @@ -234,6 +234,11 @@ instructions: | Never artificially cap at a small number. If there are 10 distinct things to find, spawn 10 agents. The system limit is the only ceiling that matters. + For dependency-shaped bursts (explore → code → verify), `agent__task_create` chains MAY encode the + ordering — dependents with `agent`+`prompt` auto-dispatch when `agent__task_complete` unblocks them. + Same rules as direct spawning: collect every auto-dispatched agent via `agent__collect`, and an agent + finishing does NOT complete its task — call `agent__task_complete` yourself. Todos remain the tracking source of truth. + If the slices are likely to FEED each other (adjacent modules; one lane's discovery redirects another's search), run the burst in team mode per `parallel-research`: after spawning, send every explorer the roster of its siblings (`agent__send_message --id --message "Teammate roster: =, ... Message a teammate directly when you find a fact in THEIR slice; check your inbox after each search round and before finalizing."`) so cross-slice facts move directly between lanes instead of waiting for your synthesis. Never do this with reviewer @@ -310,8 +315,14 @@ instructions: | Rigor: Security posture: Plan / acceptance criteria: Local-run recipe / usage suites: " + --variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"} + <— the cargo commands above are only an EXAMPLE: replace them with THIS project's exact build/test/lint commands; never guess, and pass "[]" when none apply> ``` + Verification commands go in `--variables` (a JSON-array string), never in the prompt: the prompt + also carries pasted plan text from the repo under review, and whatever is declared here is + executed with a shell — a trust boundary the prose must not cross. + It computes diff signals deterministically, selects lanes (code-review always; adversary when a spec/plan is present; security on attack-surface signals; probe on consumer-facing surface with a run recipe — judgment can only WIDEN the selection, never narrow it), runs the selected lanes in @@ -387,8 +398,13 @@ instructions: | PLAN — acceptance criteria to check against: " + --variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"} + <— the cargo commands above are only an EXAMPLE: replace them with THIS project's exact build/test/lint commands; never guess, and pass "[]" when none apply> ``` + Same trust boundary as the gauntlet spawn: verification commands are declared via `--variables`, + never in the prompt, because the prompt also carries untrusted pasted text. + ### Handling adversary findings - **`ADVERSARIAL_REVIEW: DIVERGES` blocks completion.** Do not mark the task done. Resume the SAME coder session (`agent__spawn --session_id --prompt "Fix these plan-conformance failures: "`) — do not spawn a fresh coder. After the fix, re-run `adversary` ONCE to confirm it now CONFORMS; if it still DIVERGES on the same criteria after one fix cycle, STOP and escalate to the user (the plan or the approach may be wrong — consider `oracle`). diff --git a/assets/agents/step-runner/README.md b/assets/agents/step-runner/README.md index 7514368..75611f0 100644 --- a/assets/agents/step-runner/README.md +++ b/assets/agents/step-runner/README.md @@ -27,6 +27,7 @@ flowchart TD gate_blocked -->|"yes"| orient gate_blocked -->|"no"| end_blocked orient["orient
llm, read-only"] --> route_staleness + orient -. "LLM fault" .-> note_llm_fault route_staleness{"route_staleness
script"} route_staleness -->|"major deviation"| gate_deviation route_staleness -->|"else"| implement @@ -35,6 +36,7 @@ flowchart TD gate_deviation -->|"abort"| end_rejected gate_deviation -->|"other (user guidance)"| implement implement[["implement
agent → coder"]] --> route_coder_result + implement -. "coder crashed
(no retries)" .-> end_failure route_coder_result{"route_coder_result
script"} route_coder_result -->|"CODER_COMPLETE"| verify_format_lint route_coder_result -->|"REJECTED / FAILED"| end_failure @@ -51,14 +53,18 @@ flowchart TD fix_loop_gate -->|"budget left"| implement fix_loop_gate -->|"budget spent"| end_failure edge_case_sweep["edge_case_sweep
llm"] --> route_sweep + edge_case_sweep -. "LLM fault" .-> write_handoff route_sweep{"route_sweep
script"} route_sweep -->|"5+ files or boundary"| independent_review route_sweep -->|"else"| write_handoff independent_review[["independent_review
agent → code-reviewer"]] --> route_review + independent_review -. "2 attempts failed" .-> write_handoff route_review{"route_review
script"} route_review -->|"🔴 critical findings"| implement route_review -->|"else"| write_handoff write_handoff["write_handoff
llm"] --> check_handoff + write_handoff -. "LLM fault" .-> note_llm_fault + note_llm_fault{"note_llm_fault
script"} --> end_failure check_handoff{"check_handoff
script"} check_handoff -->|"schema valid"| gate_user_review check_handoff -->|"one retry"| write_handoff @@ -81,8 +87,32 @@ End nodes emit sentinel outcomes for the caller: - `STEP_BLOCKED` — `depends_on` unsatisfied and the user declined to proceed. - `STEP_REJECTED` — user aborted at the deviation gate, or the coder's plan was rejected at its approval gate. -- `STEP_FAILED` — coder failed, the step-level fix budget was exhausted, or - the handoff failed validation twice. +- `STEP_FAILED` — the coder failed or crashed, the step-level fix budget was + exhausted, the handoff failed validation twice, or an orient/handoff LLM + fault was recorded (rendered as `Pipeline fault:` in the output). + +## Fault handling + +Node crashes degrade the run visibly instead of masquerading as success: + +- `implement` has **no retries** — rerunning a coder that died mid-edit + against an already-mutated tree is unsafe. Its failure text lands in + `coder_result` and the run falls straight to `end_failure`. +- `independent_review` retries once (`max_attempts: 2`); if both attempts + fail it falls back to `write_handoff`, which flags "PIPELINE-FAULT: + independent review did not run" in the handoff rather than presenting the + failure text as findings. `route_review` carries the same guard before its + 🔴 grep, so a reviewer fault never spends a fix-loop attempt. +- `edge_case_sweep` faults likewise fall through to `write_handoff` and are + flagged as "PIPELINE-FAULT: edge-case sweep did not run". +- `orient` and `write_handoff` faults route through `note_llm_fault`, which + distills the engine's failure text into a `fault_note` rendered in the + `STEP_FAILED` output — never an empty shell. + +Degraded is never passing (a skipped review or sweep is recorded as NOT RUN, +never paraphrased as covered), and the sentinel is always emitted — every +path terminates at one of the four end nodes, so the caller always gets a +parseable outcome. ## Usage diff --git a/assets/agents/step-runner/graph.yaml b/assets/agents/step-runner/graph.yaml index dc8cbe0..8e8c447 100644 --- a/assets/agents/step-runner/graph.yaml +++ b/assets/agents/step-runner/graph.yaml @@ -6,6 +6,12 @@ description: | review -> evidence-backed handoff -> user approval gate. Designed to be delegated to by sisyphus. version: '1.0' +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# step-runner(2) → code-reviewer(3) → +# {domain-reviewer(4) → file-reviewer(5) | finding-verifier(4)}. +# This agent spawns depth-3 children; 4 = one level of headroom (default 3 had none). +max_agent_depth: 4 global_tools: - ast_grep.sh - fs_cat.sh @@ -64,6 +70,7 @@ initial_state: plan_summary: '' implementation_brief: '' staleness_report: '' + orient_failure: '' has_major_deviation: false deviation_summary: '' user_feedback: '' @@ -80,12 +87,15 @@ initial_state: tests_output: '' edge_case_report: '' downstream_updates: '' + sweep_failure: '' needs_independent_review: false review_report: '' review_attempts: 0 max_review_attempts: 1 handoff_attempts: 0 handoff_fix: '' + handoff_failure: '' + fault_note: '' step_summary: '' start: resolve_step @@ -191,7 +201,9 @@ nodes: has_major_deviation, deviation_summary, ] - fallback: end_failure + state_updates: + orient_failure: '{{output}}' + fallback: note_llm_fault next: route_staleness route_staleness: @@ -270,6 +282,12 @@ nodes: timeout: 3600 state_updates: coder_result: '{{output}}' + # No max_attempts: rerunning a coder that died mid-edit against an + # already-mutated tree is unsafe. On failure the "Agent node failed:" + # text lands in coder_result (via state_updates above) and end_failure + # renders it; end_failure's other keys all have initial_state defaults, + # so the STEP_FAILED output degrades to empty build/tests sections. + fallback: end_failure next: route_coder_result route_coder_result: @@ -376,6 +394,8 @@ nodes: needs_independent_review: type: boolean required: [edge_case_report, downstream_updates, needs_independent_review] + state_updates: + sweep_failure: '{{output}}' fallback: write_handoff next: route_sweep @@ -405,8 +425,10 @@ nodes: Review the changed files against the step plan's acceptance criteria. Preserve severity tags in your findings. timeout: 1200 + max_attempts: 2 state_updates: review_report: '{{output}}' + fallback: write_handoff next: route_review route_review: @@ -450,6 +472,25 @@ nodes: If "Gate feedback" below is non-empty, a previous handoff attempt failed validation - fix exactly what it lists. + + PIPELINE-FAULT VISIBILITY: an "Independent review report" below that + starts with "Agent node failed:" means the independent review DID NOT + RUN - the graph routed here through the reviewer's failure fallback. + Never present that text as review findings. Flag it prominently + instead: add a Deviations entry "PIPELINE-FAULT: independent review + did not run - ", repeat it under Notes for next step, + and carry it into `step_summary` so the user sees it at the approval + gate. + + The same rule covers the "Edge-case sweep result" below: text starting + with "LLM node" means the edge-case sweep DID NOT RUN - the graph + routed here through the sweep's failure fallback. Never present it as + sweep findings. Add a Deviations entry "PIPELINE-FAULT: edge-case + sweep did not run - " and carry it into `step_summary` + so the user sees it at the approval gate. If it holds successful + structured output or is empty, ignore it - the sweep's real results + already flow through the "Edge cases discovered" / "Downstream plan + updates" sections above. prompt: | ## Step {{step_number}} ({{step_title}}) - plan at {{step_plan_path}} @@ -473,6 +514,9 @@ nodes: ## Downstream plan updates {{downstream_updates}} + ## Edge-case sweep result (fault detection) + {{sweep_failure}} + ## Independent review report (if any) {{review_report}} @@ -501,9 +545,24 @@ nodes: type: string description: 3-6 sentence summary of the step for the user's approval decision - what was done, deviations, anything needing their attention required: [step_summary] - fallback: end_failure + state_updates: + handoff_failure: '{{output}}' + fallback: note_llm_fault next: check_handoff + note_llm_fault: + id: note_llm_fault + type: script + description: | + Make LLM-node fallback captures visible: when orient or write_handoff + failed (their capture keys hold the engine's "LLM node..." failure + text), distill it into fault_note so end_failure can render WHY the + step died instead of an empty STEP_FAILED shell. + script: scripts/note_llm_fault.sh + timeout: 5 + fallback: end_failure + next: end_failure + check_handoff: id: check_handoff type: script @@ -601,6 +660,7 @@ nodes: Step: {{step_number}} ({{step_title}}) Fix attempts: {{fix_attempts}}/{{max_fix_attempts}} Blocking reason (if resolution failed): {{blocking_reason}} + Pipeline fault (if any): {{fault_note}} Coder result: {{coder_result}} diff --git a/assets/agents/step-runner/scripts/note_llm_fault.sh b/assets/agents/step-runner/scripts/note_llm_fault.sh new file mode 100755 index 0000000..85fd7b0 --- /dev/null +++ b/assets/agents/step-runner/scripts/note_llm_fault.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [[ -n "${GRAPH_STATE_FILE:-}" ]]; then + state=$(cat "$GRAPH_STATE_FILE") +elif [[ -n "${GRAPH_STATE:-}" ]]; then + state="$GRAPH_STATE" +else + state='{}' +fi + +orient_failure=$(echo "$state" | jq -r '.orient_failure // ""') +handoff_failure=$(echo "$state" | jq -r '.handoff_failure // ""') + +# Engine failure text is always prefix-anchored "LLM node" (llm.rs); on +# success these keys hold the node's structured JSON output, which never +# starts with that prefix. +if [[ "$orient_failure" == "LLM node"* ]]; then + fault_note="PIPELINE-FAULT: orient stage failed — ${orient_failure:0:300}" +elif [[ "$handoff_failure" == "LLM node"* ]]; then + fault_note="PIPELINE-FAULT: handoff stage failed — ${handoff_failure:0:300}" +else + fault_note="" +fi + +jq -nc --arg note "$fault_note" '{"fault_note": $note}' diff --git a/assets/agents/step-runner/scripts/route_review.sh b/assets/agents/step-runner/scripts/route_review.sh index eb6b2b9..c432920 100755 --- a/assets/agents/step-runner/scripts/route_review.sh +++ b/assets/agents/step-runner/scripts/route_review.sh @@ -13,6 +13,16 @@ review_report=$(echo "$state" | jq -r '.review_report // ""') review_attempts=$(echo "$state" | jq -r '.review_attempts // 0') max_review_attempts=$(echo "$state" | jq -r '.max_review_attempts // 1') +# Fail-visible guard: a review_report holding the engine's "Agent node +# failed:" text means the independent review DID NOT RUN (its fallback +# normally routes straight to write_handoff, bypassing this script). Never +# treat that text as findings or spend a fix-loop attempt on it - +# write_handoff flags the fault prominently in the handoff instead. +if [[ "$review_report" == "Agent node failed:"* ]]; then + jq -nc '{"_next": "write_handoff"}' + exit 0 +fi + if ! grep -qF "🔴" <<< "$review_report"; then jq -nc '{"_next": "write_handoff"}' exit 0 diff --git a/graph.example.yaml b/graph.example.yaml index fa018b9..7a29cbd 100644 --- a/graph.example.yaml +++ b/graph.example.yaml @@ -213,8 +213,10 @@ nodes: # agent/graph/global default when omitted. skill_instructions: null # Per-node skill-hint text override; uses the built-in default when omitted. output_schema: # Optional JSON Schema. The output is parsed to JSON - type: object # and its top-level object keys auto-merge into state - properties: # (so `topic` / `needs_deep_dive` become {{topic}} etc). + type: object # and only the keys declared under `properties` auto-merge + properties: # into state (so `topic` / `needs_deep_dive` become {{topic}} + # etc). Undeclared keys the model emits are dropped; lift + # one explicitly via state_updates: `{{output.}}`. topic: { type: string } needs_deep_dive: { type: boolean } required: [topic, needs_deep_dive] @@ -284,8 +286,9 @@ nodes: type: array items: { type: string } required: [context, sources] - # When `output_schema` is set, top-level keys auto-merge into state, so - # `context` and `sources` are produced without needing `state_updates`. + # When `output_schema` is set, the keys declared under `properties` + # auto-merge into state, so `context` and `sources` are produced without + # needing `state_updates`. Keys the schema does not declare are dropped. next: synthesize # Joins with retrieve at `synthesize`. # --- script node (the diamond's join; also dispatches) ----------------- @@ -369,7 +372,8 @@ nodes: # and is retired when the chain ends. A message delivered # after the chain's last flagged agent step is accepted # but never read. - output_schema: # Optional. Same extraction as llm nodes + output_schema: # Optional. Same extraction as llm nodes: only the keys + # declared under `properties` auto-merge into state. type: object properties: summary: { type: string } diff --git a/src/client/common.rs b/src/client/common.rs index 3eb1892..ac51ee6 100644 --- a/src/client/common.rs +++ b/src/client/common.rs @@ -523,14 +523,7 @@ pub async fn call_chat_completions( ctx.app.config.print_markdown(&text)?; } } - let mut tool_results = eval_tool_calls(ctx, tool_calls).await?; - if let Some(first) = tool_results.first_mut() { - first.thinking = thinking; - } - tool_results - .iter() - .for_each(|res| ctx.tool_scope.tool_tracker.record_call(res.call.clone())); - Ok((text, tool_results)) + finish_completion(ctx, text, tool_calls, thinking).await } Err(err) => Err(err), } @@ -592,14 +585,7 @@ pub async fn call_chat_completions_streaming( if !silent && !text.is_empty() && !text.ends_with('\n') { println!(); } - let mut tool_results = eval_tool_calls(ctx, tool_calls).await?; - if let Some(first) = tool_results.first_mut() { - first.thinking = thinking; - } - tool_results - .iter() - .for_each(|res| ctx.tool_scope.tool_tracker.record_call(res.call.clone())); - Ok((text, tool_results)) + finish_completion(ctx, text, tool_calls, thinking).await } Err(err) => { if !silent && !text.is_empty() { @@ -610,6 +596,72 @@ pub async fn call_chat_completions_streaming( } } +/// Streaming transport for graph `llm` nodes: accumulates the reply and the +/// transport itself renders nothing; no spinner, delta, or reasoning +/// (tool-call rendering inside `eval_tool_calls` is unchanged from the +/// non-streaming path). Because chunks arrive as they are generated, the +/// reqwest `read_timeout` bounds only stalls between chunks; the caller's +/// own deadline bounds the whole generation. Parity with the non-streaming +/// transport is kept deliberately: the handler's call-loop detection is off +/// (that path never had it), and a model the catalog marks `no_stream` +/// takes the non-streaming request, raced against the abort. An abort +/// observed after the provider call fails outright. Partial output is +/// never returned as success. +pub async fn call_chat_completions_streaming_quiet( + input: &Input, + client: &dyn Client, + ctx: &mut RequestContext, + abort_signal: AbortSignal, +) -> Result<(String, Vec)> { + if client.model().no_stream() { + let output = tokio::select! { + ret = client.chat_completions(input.clone()) => ret?, + _ = wait_abort_signal(&abort_signal) => bail!("Aborted."), + }; + if abort_signal.aborted() { + bail!("Aborted."); + } + let ChatCompletionsOutput { + text, + tool_calls, + thinking, + } = output; + return finish_completion(ctx, text, tool_calls, thinking).await; + } + + // `_rx` outlives the call so `handler.done()` still finds a receiver. + let (tx, _rx) = unbounded_channel(); + let mut handler = SseHandler::new(tx, abort_signal.clone()); + handler.set_silent(true); + handler.set_call_loop_detection(false); + + let send_ret = client.chat_completions_streaming(input, &mut handler).await; + + if abort_signal.aborted() { + bail!("Aborted."); + } + send_ret?; + + let (text, tool_calls, thinking) = handler.take(); + finish_completion(ctx, text, tool_calls, thinking).await +} + +async fn finish_completion( + ctx: &mut RequestContext, + text: String, + tool_calls: Vec, + thinking: Vec, +) -> Result<(String, Vec)> { + let mut tool_results = eval_tool_calls(ctx, tool_calls).await?; + if let Some(first) = tool_results.first_mut() { + first.thinking = thinking; + } + tool_results + .iter() + .for_each(|res| ctx.tool_scope.tool_tracker.record_call(res.call.clone())); + Ok((text, tool_results)) +} + pub fn noop_prepare_rerank(_client: &T, _data: &RerankData) -> Result { bail!("The client doesn't support rerank api") } @@ -848,6 +900,10 @@ mod tests { use super::*; use super::super::access_token::{is_rejected, set_access_token}; + use crate::config::{AppState, WorkingMode}; + use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; + use tokio::time::Instant; fn catch_error_message(data: &Value, status: u16) -> String { catch_error(data, status).unwrap_err().to_string() @@ -1010,4 +1066,283 @@ mod tests { assert!(!should_retry_auth(&api_status_error(401), client)); assert!(!is_rejected(client, "at-1")); } + + /// Fake provider whose streaming path emits `PACED_DELTAS` text chunks + /// 60s apart, then one thinking block and one tool call, and whose + /// non-streaming path returns that same reply in one piece when + /// `nonstream_replies` is set (and bails otherwise). Exercises the real + /// trait defaults (build_client, prepare_completion_data, abort + /// `select!`) with no network. + struct PacedStreamClient { + config: AppConfig, + model: Model, + stall_forever: bool, + nonstream_replies: bool, + streaming_calls: AtomicUsize, + } + + const PACED_DELTAS: usize = 10; + const PACED_GAP: Duration = Duration::from_secs(60); + + fn paced_text() -> String { + (0..PACED_DELTAS).map(|i| format!("chunk{i} ")).collect() + } + + fn paced_tool_call() -> ToolCall { + ToolCall::new( + "lookup".to_string(), + json!({"q": 1}), + Some("call-1".to_string()), + ) + } + + fn paced_thinking() -> ThinkingBlock { + ThinkingBlock::Thinking { + thinking: "hmm".to_string(), + signature: "sig".to_string(), + } + } + + #[async_trait::async_trait] + impl Client for PacedStreamClient { + fn app_config(&self) -> &AppConfig { + &self.config + } + + fn extra_config(&self) -> Option<&ExtraConfig> { + None + } + + fn patch_config(&self) -> Option<&RequestPatch> { + None + } + + fn name(&self) -> &str { + "paced-stream" + } + + fn model(&self) -> &Model { + &self.model + } + + async fn chat_completions_inner( + &self, + _client: &ReqwestClient, + data: ChatCompletionsData, + ) -> Result { + assert!(!data.stream, "non-streaming path must not request a stream"); + if !self.nonstream_replies { + bail!("the quiet transport must not take the non-streaming path"); + } + if self.stall_forever { + std::future::pending::<()>().await; + } + Ok(ChatCompletionsOutput { + text: paced_text(), + tool_calls: vec![paced_tool_call()], + thinking: vec![paced_thinking()], + }) + } + + async fn chat_completions_streaming_inner( + &self, + _client: &ReqwestClient, + handler: &mut SseHandler, + data: ChatCompletionsData, + ) -> Result<()> { + self.streaming_calls.fetch_add(1, Ordering::SeqCst); + assert!(data.stream, "trait default must request a stream"); + for i in 0..PACED_DELTAS { + tokio::time::sleep(PACED_GAP).await; + handler.text(&format!("chunk{i} "))?; + } + if self.stall_forever { + std::future::pending::<()>().await; + } + handler.thinking_block(paced_thinking()); + handler.tool_call(paced_tool_call()) + } + } + + fn paced_client(stall_forever: bool) -> PacedStreamClient { + PacedStreamClient { + config: AppConfig::default(), + model: Model::default(), + stall_forever, + nonstream_replies: false, + streaming_calls: AtomicUsize::new(0), + } + } + + fn no_stream_model() -> Model { + let mut data = ModelData::new(""); + data.no_stream = true; + Model::from_config("", &[data]).remove(0) + } + + fn assert_paced_reply(text: &str, results: &[ToolResult]) { + assert_eq!(text, paced_text()); + assert_eq!(results.len(), 1); + assert_eq!(results[0].call.name, "lookup"); + assert_eq!(results[0].call.id.as_deref(), Some("call-1")); + assert!( + matches!(&results[0].thinking[..], [ThinkingBlock::Thinking { thinking, .. }] if thinking == "hmm") + ); + } + + fn quiet_ctx() -> RequestContext { + RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd) + } + + /// 600s of virtual generation completes and yields the same shape the + /// non-streaming transport would: full text, tool results with the + /// thinking stapled onto the first, and the tracker fed each call. The + /// fake ignores the reqwest client, so reqwest's `read_timeout` is not in + /// the loop here; the real-socket tests in graph/mod.rs pin that it + /// fires on a stall (`sse_read_timeout_stall_is_transient`) and not on a + /// slow but continuous stream (`sse_slow_but_continuous_stream_outlives_read_timeout`). + #[tokio::test(start_paused = true)] + async fn quiet_streaming_accumulates_across_a_long_generation() { + let mut ctx = quiet_ctx(); + let input = Input::from_str(&ctx, "hi", None).unwrap(); + let client = paced_client(false); + let started = Instant::now(); + + let (text, results) = + call_chat_completions_streaming_quiet(&input, &client, &mut ctx, create_abort_signal()) + .await + .unwrap(); + + assert!(started.elapsed() >= PACED_GAP * PACED_DELTAS as u32); + assert_paced_reply(&text, &results); + assert!( + results[0].output["tool_call_error"].is_string(), + "an undeclared tool evaluates to a tool_call_error, not a panic: {}", + results[0].output + ); + } + + /// An abort mid-stream cancels the provider call promptly and fails the + /// call outright; the ten chunks already buffered are never returned. + #[tokio::test(start_paused = true)] + async fn quiet_streaming_abort_discards_partial_output() { + let mut ctx = quiet_ctx(); + let input = Input::from_str(&ctx, "hi", None).unwrap(); + let client = paced_client(true); + let abort = create_abort_signal(); + let trigger = abort.clone(); + tokio::spawn(async move { + tokio::time::sleep(PACED_GAP * PACED_DELTAS as u32 + Duration::from_secs(30)).await; + trigger.set_ctrlc(); + }); + let started = Instant::now(); + + let err = call_chat_completions_streaming_quiet(&input, &client, &mut ctx, abort) + .await + .expect_err("an aborted stream must not succeed"); + + assert_eq!(err.to_string(), "Aborted."); + assert!(started.elapsed() < PACED_GAP * (PACED_DELTAS as u32 + 1)); + } + + /// The quiet transport and the non-streaming transport hand the node the + /// same `(text, tool_results)` for the same provider reply. + #[tokio::test(start_paused = true)] + async fn quiet_streaming_matches_the_non_streaming_reply() { + let mut ctx = quiet_ctx(); + ctx.render_mode = RenderMode::Silent; + let input = Input::from_str(&ctx, "hi", None).unwrap(); + let mut client = paced_client(false); + client.nonstream_replies = true; + + let (streamed_text, streamed) = + call_chat_completions_streaming_quiet(&input, &client, &mut ctx, create_abort_signal()) + .await + .unwrap(); + let (plain_text, plain) = call_chat_completions( + &input, + false, + false, + &client, + &mut ctx, + create_abort_signal(), + ) + .await + .unwrap(); + + assert_eq!(streamed_text, plain_text); + assert_paced_reply(&streamed_text, &streamed); + assert_paced_reply(&plain_text, &plain); + assert_eq!(streamed[0].output, plain[0].output); + } + + /// A model the catalog marks `no_stream` never sees a streaming request; + /// the quiet transport takes the non-streaming path and returns the same + /// shape. + #[tokio::test] + async fn quiet_transport_honours_no_stream_models() { + let mut ctx = quiet_ctx(); + let input = Input::from_str(&ctx, "hi", None).unwrap(); + let mut client = paced_client(false); + client.model = no_stream_model(); + client.nonstream_replies = true; + + let (text, results) = + call_chat_completions_streaming_quiet(&input, &client, &mut ctx, create_abort_signal()) + .await + .unwrap(); + + assert_eq!(client.streaming_calls.load(Ordering::SeqCst), 0); + assert_paced_reply(&text, &results); + } + + /// The `no_stream` fallback is raced against the abort like the + /// streaming path: a hung non-streaming request is cancelled promptly. + #[tokio::test(start_paused = true)] + async fn quiet_transport_no_stream_fallback_honours_abort() { + let mut ctx = quiet_ctx(); + let input = Input::from_str(&ctx, "hi", None).unwrap(); + let mut client = paced_client(true); + client.model = no_stream_model(); + client.nonstream_replies = true; + let abort = create_abort_signal(); + let trigger = abort.clone(); + tokio::spawn(async move { + tokio::time::sleep(Duration::from_secs(5)).await; + trigger.set_ctrlc(); + }); + let started = Instant::now(); + + let err = call_chat_completions_streaming_quiet(&input, &client, &mut ctx, abort) + .await + .expect_err("an aborted non-streaming fallback must not succeed"); + + assert_eq!(err.to_string(), "Aborted."); + assert!(started.elapsed() < Duration::from_secs(10)); + } + + /// The user-level `stream` toggle is a rendering preference; the quiet + /// transport still streams with it off. + #[tokio::test(start_paused = true)] + async fn quiet_transport_ignores_the_user_stream_toggle() { + let app = AppState { + config: Arc::new(AppConfig { + stream: false, + ..AppConfig::default() + }), + ..AppState::test_default() + }; + let mut ctx = RequestContext::new(Arc::new(app), WorkingMode::Cmd); + let input = Input::from_str(&ctx, "hi", None).unwrap(); + assert!(!input.stream(), "fixture must have streaming disabled"); + let client = paced_client(false); + + let (text, results) = + call_chat_completions_streaming_quiet(&input, &client, &mut ctx, create_abort_signal()) + .await + .unwrap(); + + assert_eq!(client.streaming_calls.load(Ordering::SeqCst), 1); + assert_paced_reply(&text, &results); + } } diff --git a/src/client/stream.rs b/src/client/stream.rs index 79a2d87..8d37aa4 100644 --- a/src/client/stream.rs +++ b/src/client/stream.rs @@ -2,7 +2,7 @@ use super::{ApiStatusError, ThinkingBlock, ToolCall, catch_error}; use crate::utils::AbortSignal; use anyhow::{Context, Result, anyhow, bail}; -use eventsource_stream::Eventsource; +use eventsource_stream::{EventStreamError, Eventsource}; use futures_util::{Stream, StreamExt}; use reqwest::{RequestBuilder, header}; use serde_json::Value; @@ -18,6 +18,7 @@ pub struct SseHandler { max_call_repeats: usize, call_repeat_chain_len: usize, silent: bool, + call_loop_detection: bool, } impl SseHandler { @@ -32,6 +33,7 @@ impl SseHandler { max_call_repeats: 2, call_repeat_chain_len: 3, silent: false, + call_loop_detection: true, } } @@ -39,6 +41,13 @@ impl SseHandler { self.silent = silent; } + /// Loop detection guards an interactive stream; the non-streaming + /// transport never had it, so callers that must match that transport's + /// tool-call handling turn it off. + pub fn set_call_loop_detection(&mut self, enabled: bool) { + self.call_loop_detection = enabled; + } + pub fn text(&mut self, text: &str) -> Result<()> { if text.is_empty() { return Ok(()); @@ -73,7 +82,7 @@ impl SseHandler { } pub fn tool_call(&mut self, call: ToolCall) -> Result<()> { - if self.is_call_loop(&call) { + if self.call_loop_detection && self.is_call_loop(&call) { let loop_message = self.create_loop_detection_message(&call); return Err(anyhow!(loop_message)); } @@ -273,6 +282,11 @@ where break; } } + // Keep the typed reqwest error in the chain: a read_timeout stall + // is only recognised as transient via `reqwest::Error::is_timeout`. + Err(EventStreamError::Transport(err)) => { + return Err(err).context("Transport error"); + } Err(err) => { bail!("{err}"); } @@ -285,13 +299,12 @@ pub async fn json_stream(mut stream: S, mut handle: F) -> Result<()> where S: Stream> + Unpin, F: FnMut(&str) -> Result<()>, - E: std::error::Error, + E: std::error::Error + Send + Sync + 'static, { let mut parser = JsonStreamParser::default(); let mut unparsed_bytes = vec![]; while let Some(chunk_bytes) = stream.next().await { - let chunk_bytes = - chunk_bytes.map_err(|err| anyhow!("Failed to read json stream, {err}"))?; + let chunk_bytes = chunk_bytes.context("Failed to read json stream")?; unparsed_bytes.extend(chunk_bytes); match std::str::from_utf8(&unparsed_bytes) { Ok(text) => { @@ -430,6 +443,40 @@ mod tests { assert!(error_message.contains("test_function_loop")); } + /// With detection off, repeated identical calls all accumulate — the + /// non-streaming transport's behaviour, which the quiet transport mirrors. + #[test] + fn test_call_loop_detection_can_be_disabled() { + let (mut handler, _rx) = new_handler(); + handler.set_call_loop_detection(false); + let call = ToolCall::new("test_function_loop".to_string(), json!({"param": 1}), None); + + for _ in 0..5 { + handler.tool_call(call.clone()).unwrap(); + } + + let (_, calls, _) = handler.take(); + assert_eq!(calls.len(), 5); + } + + /// A transport failure keeps its typed error in the chain so callers can + /// classify it (a stall is only recognised as transient via the type). + #[tokio::test] + async fn json_stream_keeps_the_typed_transport_error() { + let stalled = std::io::Error::new(std::io::ErrorKind::TimedOut, "stall"); + let mut source = stream::iter(vec![Err::(stalled)]); + + let err = json_stream(&mut source, |_| Ok(())) + .await + .expect_err("a failed read must fail the stream"); + + assert!(err.chain().any(|c| { + c.downcast_ref::() + .is_some_and(|e| e.kind() == std::io::ErrorKind::TimedOut) + })); + assert!(format!("{err:#}").starts_with("Failed to read json stream")); + } + fn new_handler() -> (SseHandler, tokio::sync::mpsc::UnboundedReceiver) { let (sender, receiver) = tokio::sync::mpsc::unbounded_channel(); let abort_signal = crate::utils::create_abort_signal(); @@ -467,6 +514,43 @@ mod tests { assert!(handler.has_received_content()); } + /// A silent handler is the quiet transport's whole output gate: text, + /// tool calls, and thinking still accumulate for `take()`, but the + /// receiver sees no `Text` events, only the trailing `Done`. + #[test] + fn test_silent_handler_accumulates_without_emitting_text() { + let (mut handler, mut rx) = new_handler(); + handler.set_silent(true); + + handler.text("hel").unwrap(); + handler.text("lo").unwrap(); + handler.thinking_block(ThinkingBlock::Thinking { + thinking: "hmm".to_string(), + signature: "sig".to_string(), + }); + handler + .tool_call(ToolCall::new( + "lookup".to_string(), + json!({"q": 1}), + Some("call-1".to_string()), + )) + .unwrap(); + handler.done(); + + assert!(matches!(rx.try_recv(), Ok(SseEvent::Done))); + assert!(rx.try_recv().is_err(), "no events beyond Done"); + + let (text, calls, thinking) = handler.take(); + assert_eq!(text, "hello"); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0].name, "lookup"); + assert_eq!(calls[0].id.as_deref(), Some("call-1")); + assert_eq!(thinking.len(), 1); + assert!( + matches!(&thinking[0], ThinkingBlock::Thinking { thinking, .. } if thinking == "hmm") + ); + } + fn split_chunks(text: &str) -> Vec> { let len = text.len(); let cut1 = random_range(1..len - 1); diff --git a/src/config/prompts.rs b/src/config/prompts.rs index 4aeda0b..c37c13b 100644 --- a/src/config/prompts.rs +++ b/src/config/prompts.rs @@ -167,17 +167,26 @@ pub(in crate::config) const DEFAULT_SPAWN_INSTRUCTIONS: &str = indoc! {" ``` # Create tasks with dependencies (optional: auto-dispatch with --agent) agent__task_create --subject \"Explore existing patterns\" - agent__task_create --subject \"Implement feature\" --blocked_by [\"task_1\"] --agent coder --prompt \"Implement based on patterns found\" - agent__task_create --subject \"Write tests\" --blocked_by [\"task_2\"] + agent__task_create --subject \"Implement feature\" --blocked_by [\"1\"] --agent coder --prompt \"Implement based on patterns found\" + agent__task_create --subject \"Write tests\" --blocked_by [\"2\"] # Check what's runnable agent__task_list # After completing a task, mark it done to unblock dependents # If dependents have --agent set, they auto-dispatch - agent__task_complete --task_id task_1 + agent__task_complete --task_id 1 ``` + Task-queue rules (NOT obvious — read carefully): + - Task IDs are numeric strings (\"1\", \"2\", ...) assigned in creation order. + - A task created with `agent` but NO blockers is NEVER auto-dispatched — spawn it yourself. + - Auto-dispatch happens only inside `agent__task_complete`: completing a task is what + dispatches its newly-unblocked dependents. + - Auto-dispatched agents must still be collected via `agent__collect`, and an agent finishing + does NOT complete its task — call `agent__task_complete` yourself to advance the chain. + - A failed task permanently blocks its dependents — recreate the chain rather than retrying. + ### Escalation Handling Child agents may need user input but cannot prompt the user directly. When this happens, diff --git a/src/config/request_context.rs b/src/config/request_context.rs index ef04078..ef594d5 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -8157,6 +8157,280 @@ mod tests { assert!(paths::skill_file("frontend-ui-ux").exists()); } + #[test] + #[serial] + fn bundled_assets_pin_task_queue_guidance() { + let _guard = TestConfigDirGuard::new(); + Agent::install_builtin_agents(false).unwrap(); + + // 1. Architect config: the parallel-mode "Task-queue mirroring" + // section with all five HARD RULES. + let architect = read_to_string( + paths::agents_data_dir() + .join("architect") + .join("config.yaml"), + ) + .unwrap(); + for anchor in [ + "Task-queue mirroring (optional).", + "it is NEVER the source of truth", + "Root tasks (no blockers) are NOT auto-dispatched on create — spawn them yourself.", + "Call `agent__task_complete` ONLY after this task's verification gate", + "Collect every auto-dispatched agent via `agent__collect`", + "strands the queue task InProgress with no reset — recreate", + ] { + assert!( + architect.contains(anchor), + "architect config lost task-queue mirroring anchor: {anchor:?}" + ); + } + + // 2. Sisyphus config: the task-queue chain note in the + // parallel-research section. + let sisyphus = read_to_string( + paths::agents_data_dir() + .join("sisyphus") + .join("config.yaml"), + ) + .unwrap(); + for anchor in [ + "`agent__task_create` chains MAY encode the", + "Todos remain the tracking source of truth.", + ] { + assert!( + sisyphus.contains(anchor), + "sisyphus config lost task-queue chain anchor: {anchor:?}" + ); + } + + // 3. Spawn instructions: dispatch/collect/failure semantics plus the + // corrected numeric-string task IDs (no `task_1`-style remnants). + let spawn = crate::config::prompts::DEFAULT_SPAWN_INSTRUCTIONS; + for anchor in [ + "is NEVER auto-dispatched", + "agent__collect", + "recreate the chain rather than retrying", + "--blocked_by [\"1\"]", + ] { + assert!( + spawn.contains(anchor), + "DEFAULT_SPAWN_INSTRUCTIONS lost task-queue semantics anchor: {anchor:?}" + ); + } + assert!( + !spawn.contains("task_1"), + "DEFAULT_SPAWN_INSTRUCTIONS must not reference task_1-style IDs" + ); + } + + // Shell commands the adversary's run_checks stage executes must be + // declared as a variable, never as prompt prose an LLM would re-extract. + #[test] + #[serial] + fn bundled_assets_pin_verification_commands_as_declared_variable() { + let _guard = TestConfigDirGuard::new(); + Agent::install_builtin_agents(false).unwrap(); + + const VARIABLES_FORM: &str = + "--variables {\"verification_commands\": \"[\\\"cargo test --all\\\""; + const EXAMPLE_CUE: &str = + "only an EXAMPLE: replace them with THIS project's exact build/test/lint commands"; + for name in ["architect", "sisyphus"] { + let config = + read_to_string(paths::agents_data_dir().join(name).join("config.yaml")).unwrap(); + assert_eq!( + config.matches(VARIABLES_FORM).count(), + 2, + "{name} config must pass verification_commands via --variables on both the \ + review-gauntlet and adversary spawn templates" + ); + assert!( + !config.contains("Verification commands:"), + "{name} config must not declare verification commands as prompt prose" + ); + assert!( + config.matches("trust boundary").count() >= 2, + "{name} config must explain the trust boundary on both spawn templates" + ); + assert!( + config.contains("untrusted pasted text"), + "{name} config must name the untrusted prompt text as the reason" + ); + assert!( + config.matches(EXAMPLE_CUE).count() >= 2, + "{name} config must mark the cargo commands as an example to substitute on both \ + spawn templates, or a literal follower records `cargo: command not found` in non-Rust repos" + ); + } + + let readmes = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("assets/agents"); + let adversary = read_to_string(readmes.join("adversary/README.md")).unwrap(); + assert!( + !adversary.contains("## VERIFICATION"), + "adversary README must not show the prose ## VERIFICATION block" + ); + assert!(adversary.contains(VARIABLES_FORM)); + assert!( + adversary.contains("--agent-variable verification_commands '[\"cargo test --all\"]'"), + "adversary README must show the CLI form" + ); + assert!( + adversary.contains( + "merges only the keys declared under `parse`'s `output_schema.properties`" + ) && !adversary.contains("Known residual"), + "adversary README must state the schema-declared merge closes the extra-key channel" + ); + assert!( + adversary.contains("zero red verification runs"), + "adversary README must state the fail-closed CONFORMS doctrine" + ); + assert!( + !adversary.contains("soft ENVIRONMENT") + && adversary.contains("never degrades to an ENVIRONMENT marker"), + "adversary README must describe a malformed declaration as fail-closed, not a soft marker" + ); + let gauntlet = read_to_string(readmes.join("review-gauntlet/README.md")).unwrap(); + assert!( + !gauntlet.contains("Verification commands:"), + "review-gauntlet README must not show the prose Verification commands: line" + ); + assert!(gauntlet.contains(VARIABLES_FORM)); + assert!( + !gauntlet.contains("extracted verbatim by `parse`"), + "review-gauntlet README must not describe parse-extraction of the commands" + ); + assert!( + gauntlet.contains("--agent-variable verification_commands '[\"cargo test --all\"]'"), + "review-gauntlet README must show the CLI form" + ); + assert!(gauntlet.contains("plus probe on consumer surface *with a local-run recipe*")); + assert!(gauntlet.contains("critical count from the report's summary line")); + assert!(gauntlet.contains("raw 🔴 count if absent")); + assert!( + gauntlet + .contains("records a pipeline fault blocks (a degraded review is never a pass)") + ); + assert!( + !gauntlet.contains("any 🔴 in the code-review report"), + "review-gauntlet README must not describe the retired raw-🔴 gate" + ); + + // Normalize line endings: the assertion below spans a line break and + // Windows checkouts carry CRLF. + let runner = read_to_string(readmes.join("adversary/scripts/run_checks.py")) + .unwrap() + .replace("\r\n", "\n"); + assert!( + runner.contains("Trust boundary:") && runner.contains("never a field an LLM"), + "run_checks.py docstring must state the declared-variable trust boundary" + ); + assert!( + !runner.contains("soft ENVIRONMENT") + && runner.contains("never\ndegrades to an ENVIRONMENT marker"), + "run_checks.py docstring must describe a malformed declaration as fail-closed, not a soft marker" + ); + assert!( + !runner.contains("Known residual") && !gauntlet.contains("residual:"), + "run_checks.py and the review-gauntlet README must not disclose the closed residual" + ); + } + + // The spawner's own `max_agent_depth` is checked against the child's + // absolute depth (root = 0), so every hop along a chain needs its own + // limit >= child depth; +1 leaves one level of headroom. + #[test] + #[serial] + fn bundled_review_suite_depth_limits_cover_documented_chain() { + use crate::graph::GraphParser; + + fn bundled_agent_config(name: &str) -> AgentConfig { + let dir = paths::agents_data_dir().join(name); + let graph_path = dir.join("graph.yaml"); + if graph_path.exists() { + let graph = GraphParser::new(&dir) + .load_from_file(&graph_path) + .unwrap_or_else(|e| panic!("graph.yaml for '{name}' failed to parse: {e}")); + return AgentConfig::from_graph(name, &graph); + } + AgentConfig::load(&dir.join("config.yaml")) + .unwrap_or_else(|e| panic!("config.yaml for '{name}' failed to load: {e}")) + } + + let _guard = TestConfigDirGuard::new(); + Agent::install_builtin_agents(false).unwrap(); + + for (name, expected) in [ + ("architect", 10), + ("sisyphus", 5), + ("review-gauntlet", 4), + ("step-runner", 4), + ("code-reviewer", 5), + ("domain-reviewer", 6), + ("architecture-reviewer", 4), + ] { + assert_eq!( + bundled_agent_config(name).max_agent_depth, + expected, + "bundled '{name}' max_agent_depth drifted" + ); + } + + // User decision: domain-reviewer's file-reviewer fan-out stays unwired + // for now. Remove it from this list when `can_spawn_agents: true` is added. + const DOCUMENTED_BUT_UNWIRED: &[&str] = &["domain-reviewer"]; + + let chains: [&[&str]; 4] = [ + &[ + "architect", + "sisyphus", + "review-gauntlet", + "code-reviewer", + "domain-reviewer", + "file-reviewer", + ], + &[ + "architect", + "sisyphus", + "step-runner", + "code-reviewer", + "domain-reviewer", + "file-reviewer", + ], + &[ + "architect", + "sisyphus", + "review-gauntlet", + "code-reviewer", + "finding-verifier", + ], + &["architect", "sisyphus", "architecture-reviewer", "explore"], + ]; + for chain in chains { + for (i, pair) in chain.windows(2).enumerate() { + let (spawner, child) = (pair[0], pair[1]); + let cfg = bundled_agent_config(spawner); + assert!( + cfg.max_agent_depth >= i + 2, + "{spawner} (depth {i}) needs max_agent_depth >= {} to spawn {child} at depth {} with one level of headroom", + i + 2, + i + 1 + ); + if !DOCUMENTED_BUT_UNWIRED.contains(&spawner) { + assert!( + cfg.can_spawn_agents, + "{spawner} must have can_spawn_agents to reach {child}" + ); + } + if let Some(list) = &cfg.spawnable_agents { + assert!( + list.iter().any(|s| s == child), + "{spawner} spawnable_agents must include {child}, got {list:?}" + ); + } + } + } + } + #[test] #[serial] fn bundled_graph_agents_parse_and_validate() { @@ -8330,6 +8604,5292 @@ mod tests { ); } + // `state_updates::apply` merges only the keys an llm/agent node's + // `output_schema.properties` declares, so a bundled graph that relied on + // an undeclared model-emitted key reaching state would now break at + // runtime. Every key a graph consumes must have a declared writer, and a + // schema node's field list must not name a state key its schema omits. + #[test] + #[serial] + fn bundled_graph_output_schemas_declare_every_key_relied_on() { + use crate::graph::types::{ConcurrencyCap, Node}; + use crate::graph::{GraphParser, NodeType}; + use fancy_regex::Regex; + use std::collections::BTreeMap; + + fn idents(re: &Regex, text: &str) -> BTreeSet { + re.captures_iter(text) + .flatten() + .filter_map(|c| c.get(1).map(|m| m.as_str().to_string())) + .collect() + } + + fn state_updates_of(node: &Node) -> Option<&HashMap> { + match &node.node_type { + NodeType::Llm(n) => n.state_updates.as_ref(), + NodeType::Agent(n) => n.state_updates.as_ref(), + NodeType::Rag(n) => n.state_updates.as_ref(), + NodeType::Approval(n) => n.state_updates.as_ref(), + NodeType::Input(n) => n.state_updates.as_ref(), + NodeType::Script(n) => n.state_updates.as_ref(), + NodeType::End(n) => n.state_updates.as_ref(), + NodeType::Map(_) => None, + } + } + + fn templated_fields(node: &Node) -> Vec<&str> { + let mut fields: Vec<&str> = match &node.node_type { + NodeType::Llm(n) => { + let mut v = vec![n.prompt.as_str()]; + v.extend(n.instructions.as_deref()); + v + } + NodeType::Agent(n) => { + let mut v = vec![n.prompt.as_str()]; + v.extend(n.inputs.iter().flat_map(|m| m.values().map(String::as_str))); + v + } + NodeType::Rag(n) => { + let mut v: Vec<&str> = n.documents.iter().map(String::as_str).collect(); + v.extend(n.query.as_deref()); + v.extend(n.extractor_prompt.as_deref()); + v + } + NodeType::Approval(n) => vec![n.question.as_str()], + NodeType::Input(n) => { + let mut v = vec![n.question.as_str()]; + v.extend(n.default.as_deref()); + v + } + NodeType::End(n) => vec![n.output.as_str()], + NodeType::Map(n) => { + let mut v = vec![n.over.as_str()]; + if let Some(ConcurrencyCap::Template(t)) = &n.max_concurrency { + v.push(t); + } + v + } + NodeType::Script(_) => Vec::new(), + }; + fields.extend( + state_updates_of(node) + .into_iter() + .flat_map(|m| m.values().map(String::as_str)), + ); + fields + } + + fn schema_node_text(node: &Node) -> Option<(Option<&serde_json::Value>, String)> { + match &node.node_type { + NodeType::Llm(n) => Some(( + n.output_schema.as_ref(), + format!("{}\n{}", n.instructions.as_deref().unwrap_or(""), n.prompt), + )), + NodeType::Agent(n) => Some((n.output_schema.as_ref(), n.prompt.clone())), + _ => None, + } + } + + let template_root = Regex::new(r"\{\{\s*([A-Za-z_][A-Za-z0-9_]*)").unwrap(); + let script_read = + Regex::new(r#"state(?:\.get\(|\[)\s*["']([A-Za-z_][A-Za-z0-9_]*)["']"#).unwrap(); + let script_dict_key = Regex::new(r#"["']([A-Za-z_][A-Za-z0-9_]*)["']\s*:"#).unwrap(); + let script_subscript_assign = + Regex::new(r#"\[["']([A-Za-z_][A-Za-z0-9_]*)["']\]\s*=[^=]"#).unwrap(); + let backticked_field = Regex::new(r"(?m)^\s*-\s*`([a-z][a-z0-9_]*)`").unwrap(); + + // Seeded by the engine rather than any graph author: `initial_prompt` + // (dispatch) and the per-node scoped `output`/`choice`/`input` bindings. + let engine_seeded = ["initial_prompt", "output", "choice", "input"]; + + let _guard = TestConfigDirGuard::new(); + Agent::install_builtin_agents(false).unwrap(); + + let mut checked = Vec::new(); + for entry in std::fs::read_dir(paths::agents_data_dir()).unwrap() { + let dir = entry.unwrap().path(); + let graph_path = dir.join("graph.yaml"); + if !graph_path.exists() { + continue; + } + let name = dir.file_name().unwrap().to_string_lossy().to_string(); + let graph = GraphParser::new(&dir) + .load_from_file(&graph_path) + .unwrap_or_else(|e| panic!("graph.yaml for '{name}' failed to parse: {e}")); + + let mut scripts = String::new(); + if let Ok(entries) = std::fs::read_dir(dir.join("scripts")) { + for script in entries.map(|e| e.unwrap().path()) { + if script.extension().is_some_and(|ext| ext == "py") { + scripts.push_str(&read_to_string(&script).unwrap()); + scripts.push('\n'); + } + } + } + + let mut writers: BTreeSet = + engine_seeded.iter().map(|k| k.to_string()).collect(); + writers.extend(graph.initial_state.keys().cloned()); + writers.extend(graph.variables.iter().map(|v| v.name.clone())); + writers.extend(idents(&script_dict_key, &scripts)); + writers.extend(idents(&script_subscript_assign, &scripts)); + + let mut consumed = idents(&script_read, &scripts); + let mut declared: BTreeMap<&str, BTreeSet> = BTreeMap::new(); + for (id, node) in &graph.nodes { + for field in templated_fields(node) { + consumed.extend(idents(&template_root, field)); + } + writers.extend( + state_updates_of(node) + .into_iter() + .flat_map(|m| m.keys().cloned()), + ); + if let NodeType::Map(m) = &node.node_type { + writers.extend([ + m.as_name.clone(), + m.output_key.clone(), + m.collect_into.clone(), + ]); + } + if let Some((Some(schema), _)) = schema_node_text(node) { + let properties = schema + .get("properties") + .and_then(serde_json::Value::as_object) + .unwrap_or_else(|| { + panic!( + "{name}/{id}: output_schema has no object `properties`, so the \ + engine would merge nothing from its output: {schema}" + ) + }); + declared.insert(id, properties.keys().cloned().collect()); + } + } + let schema_keys: BTreeSet<&String> = declared.values().flatten().collect(); + + let unwritten: Vec<&String> = consumed + .iter() + .filter(|k| !writers.contains(*k) && !schema_keys.contains(k)) + .collect(); + assert!( + unwritten.is_empty(), + "{name}: state keys consumed with no declared writer (initial_state, variable, \ + state_updates target, map key, script-emitted key, or output_schema property): \ + {unwritten:?}" + ); + + for (id, properties) in &declared { + let (_, text) = schema_node_text(&graph.nodes[*id]).unwrap(); + let omitted: Vec = idents(&backticked_field, &text) + .into_iter() + .filter(|k| { + (consumed.contains(k) || schema_keys.contains(&k)) + && !properties.contains(k) + && !writers.contains(k) + }) + .collect(); + assert!( + omitted.is_empty(), + "{name}/{id}: the node's field list names state keys its output_schema does \ + not declare, so the engine would drop them when the model emits them: \ + {omitted:?}" + ); + } + checked.push(name); + } + for expected in ["adversary", "review-gauntlet"] { + assert!( + checked.iter().any(|n| n == expected), + "expected bundled graph agent '{expected}' to be checked; found {checked:?}" + ); + } + } + + // ---- adversary suite-script regression tests ---- + // + // Fault paths of the adversary's verdict/gate scripts, exercised by + // invoking `python3