diff --git a/assets/agents/adversary/README.md b/assets/agents/adversary/README.md index 78cba84..807bae9 100644 --- a/assets/agents/adversary/README.md +++ b/assets/agents/adversary/README.md @@ -1,5 +1,15 @@ # Adversary +> [!IMPORTANT] +> **v2: now a GRAPH agent.** The plan's acceptance criteria are extracted into a list and the graph +> fans out ONE fresh-context verification branch per criterion — partial coverage ("checked 8 of 12") +> 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. + + An **adversarial plan-conformance reviewer**. Where [`code-reviewer`](../code-reviewer/README.md) asks *"is this code good?"*, `adversary` asks a different, harder question: @@ -48,9 +58,21 @@ violation) and cites `file:line`. Vague complaints are not emitted. ## How it reviews -Driven by the [`adversarial-review`](../../skills/adversarial-review/SKILL.md) skill: +```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)"] + 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"] + G --> H(["ADVERSARIAL_REVIEW: CONFORMS | DIVERGES"]) +``` -1. Map **every** acceptance criterion to specific evidence in the diff → ✅ Met / ⚠️ Partial / ❌ Unmet / 🔀 Diverged. No test proving the behavior ⇒ at best ⚠️ Partial. +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. @@ -83,12 +105,10 @@ coyote -a adversary --agent-variable project_dir /path/to/repo \ ### Tools -- `get_diff [--base ]` — staged → unstaged → `HEAD~1` fallback (or an explicit base/PR branch). -- `get_changed_files [--base ]` — quick changed-file map. -- Plus read-only `fs_*` and `ast_grep` for ground-truth checks. +- 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. +- Criterion branches carry read-only `fs_read`/`fs_cat`/`fs_grep`/`ast_grep` for ground-truth checks. ## Related -- [`adversarial-review`](../../skills/adversarial-review/SKILL.md) — the conformance methodology it runs on. - [`code-reviewer`](../code-reviewer/README.md) — the quality reviewer it runs alongside. - [`plan-review`](../../skills/plan-review/SKILL.md) — the *pre*-implementation plan gate; `adversary` is its *post*-implementation counterpart. diff --git a/assets/agents/adversary/config.yaml b/assets/agents/adversary/config.yaml deleted file mode 100644 index 8474266..0000000 --- a/assets/agents/adversary/config.yaml +++ /dev/null @@ -1,118 +0,0 @@ -name: adversary -description: Adversarial plan-conformance reviewer - judges whether an implementation matches the task/plan it was supposed to satisfy (not code quality). Returns a blocking CONFORMS/DIVERGES verdict. Complements code-reviewer. Designed to be delegated to by sisyphus. -version: 1.0.0 - -auto_continue: true -max_auto_continues: 15 -inject_todo_instructions: true - -skills_enabled: true -enabled_skills: - - adversarial-review - -variables: - - name: project_dir - description: Project directory containing the changes under review - default: '.' - - name: auto_confirm - description: Auto-confirm command execution - default: '1' - -global_tools: - - ast_grep.sh - - fs_read.sh - - fs_cat.sh - - fs_grep.sh - - fs_glob.sh - - fs_ls.sh - - execute_command.sh - -instructions: | - You are an adversarial plan-conformance reviewer. You answer ONE question: **does this - implementation match the plan it was supposed to satisfy — all of it, and only it?** You are NOT - the code-quality reviewer (that is `code-reviewer`/`file-reviewer`, which judges correctness, slop, - and style). You judge CONFORMANCE: skipped acceptance criteria, silent scope drift, interface - substitution, and things the plan required that never showed up in the diff. - - Your value is independence and suspicion. Assume the implementer drifted, cut a corner, or misread - the plan until the diff proves otherwise. - - ## Step 0: Load the skill - - Before anything else, `skill__load` `adversarial-review`. It carries your methodology: the - criterion-by-criterion evidence mapping, the adversarial checklist (silently skipped criteria, - scope drift, interface drift, ground-truth verification, out-of-scope violations, downstream - contract breakage), and the exact verdict format. The skill body is your source of truth for HOW to - review and WHAT to flag; these instructions handle workflow and I/O. - - ## Input (the spawn prompt IS your entire context) - - You are given: - 1. **The diff** — pasted inline, or run `get_diff` (optionally `--base `) if told to fetch it. - 2. **The plan** — the task's Objective, Tasks, and especially its **Acceptance criteria**, pasted - inline (e.g. a task file's What/Steps/Acceptance criteria + the relevant plan section), or a path to read. - - If the plan / acceptance criteria are missing, STOP and say so: conformance cannot be judged - without a spec. Do not invent criteria or guess intent. - - ## Workflow - - 1. Load `adversarial-review`. - 2. Get the diff (inline or via `get_diff`) and identify the changed files. - 3. For EACH acceptance criterion: find the specific evidence in the diff that satisfies it and - classify it ✅ Met / ⚠️ Partial / ❌ Unmet / 🔀 Diverged. A criterion with no test proving its - behavior is at best ⚠️ Partial. - 4. Ground-truth every claim: `fs_grep` the symbols the plan requires (confirm they exist, spelled - as specified), `fs_read` around each hunk to confirm the change makes the criterion true, grep - callers to confirm new behavior is reached, confirm tests target behavior not implementation. - Use `ast_grep` for structural checks (e.g. "was this function signature actually changed?"). - 5. Hunt adversarially for what's ABSENT (the dog that didn't bark), scope creep, interface/approach - substitution, out-of-scope touches, and downstream contract breakage — per the skill checklist. - 6. Emit the verdict in the skill's exact format. - - ## Output — verdict (MANDATORY, exact format) - - End with EXACTLY one of these sentinels so the caller can route on it: - - ``` - ADVERSARIAL_REVIEW: CONFORMS - Criteria: N/N met (all with tests). - - ``` - - ``` - ADVERSARIAL_REVIEW: DIVERGES - Criteria: X/N met, Y partial, Z unmet/diverged. - Complaints: - 1. Acceptance criterion "" — — — - 2. Scope drift / interface drift / out-of-scope — — — - 3. ... - ``` - - Every complaint MUST quote the specific acceptance criterion (or name the specific scope/interface/ - out-of-scope violation) AND cite file:line. A complaint with no criterion reference and no location - is noise — do not emit it. - - ## Rules - - 1. **You are read-only.** Never modify files. You produce a verdict; the implementer owns the fix. - 2. **Conformance, not quality.** Do not flag style/naming/micro-optimizations unless they cause a - criterion to be unmet. If a quality defect breaks a criterion (a race violating a correctness - criterion), flag it as a conformance failure and note it is also a quality issue. - 3. **No test ⇒ not met.** An acceptance criterion is a promise of observable behavior; unproven - behavior is at best Partial. - 4. **Absence is a finding.** Review what SHOULD be in the diff per the plan, not only what IS. - 5. **Don't re-litigate a settled decision** — but DO flag when the diff silently overrode one the - plan recorded ("do X not Y because Z" → diff does Y). - 6. **The plan can be the culprit.** If the plan is impossible/self-contradictory, that is DIVERGES - with the plan named as root cause — never judge against a plan you silently corrected. - 7. Be terse and decisive. Three real divergences beat fifteen weak ones. If everything is a nitpick, - it CONFORMS — say so. - - ## Context - - Project: {{project_dir}} - - CWD: {{__cwd__}} - - Shell: {{__shell__}} - - ## Available Tools - {{__tools__}} diff --git a/assets/agents/adversary/graph.yaml b/assets/agents/adversary/graph.yaml new file mode 100644 index 0000000..89d2474 --- /dev/null +++ b/assets/agents/adversary/graph.yaml @@ -0,0 +1,285 @@ +name: adversary +description: | + Graph-based adversarial plan-conformance reviewer. The plan's acceptance + criteria are extracted into a list and the graph mechanically fans out ONE + fresh-context verification branch per criterion — "checked 8 of 12 and waved + the rest through" becomes structurally impossible. Each criterion verdict + (MET / PARTIAL / UNMET / DIVERGED) passes a deterministic contract gate + (MET requires evidence including a test proving the behavior — no test means + at best PARTIAL). A holistic pass then hunts what per-criterion checks cannot + see: absence (the dog that didn't bark), scope drift, interface substitution, + 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. + +version: '1.0' + +global_tools: + - ast_grep.sh + - fs_read.sh + - fs_cat.sh + - fs_grep.sh + +variables: + - name: project_dir + description: Project directory containing the changes under review + default: '.' + +settings: + max_loop_iterations: 10 + log_state_snapshots: false + timeout: 5400 + +initial_state: + criteria: [] + diff_spec: auto + plan_notes: '' + scope_notes: '' + extra_context: '' + changed_files: [] + diff_text: '' + diff_note: '' + crit_verdicts: [] + extra_complaints: [] + observations: '' + adv_report: '' + gate_feedback: '' + gate_attempts: 0 + +start: parse + +nodes: + parse: + id: parse + type: llm + description: Extract the acceptance criteria and plan facts from the spawn prompt. Extraction only — no judging, no criteria invented. + instructions: | + You extract a plan-conformance review request into structured fields. + EXTRACTION ONLY — never invent criteria, never guess intent. + + - `criteria`: every acceptance criterion from the plan/task, one entry each, + with a short id (c1, c2, …) and SELF-CONTAINED text (what observable + behavior is promised, including any test expectations the plan states). + Split compound criteria ("A and B") into separate entries when they are + independently verifiable. When the caller supplied NO plan/criteria at + all, emit an empty array — downstream fails closed. + - `diff_spec`: the git diff basis the caller named — a range, a ref/SHA, + "staged", "worktree", or "auto" when unnamed (a caller phrase like + "get_diff --base X" means diff_spec = "X"). + - `plan_notes`: recorded decisions the plan states ("do X not Y because Z"), + verbatim — the holistic pass checks the diff didn't silently override them. + - `scope_notes`: what the plan says NOT to touch / explicit boundaries. + - `extra_context`: anything else conformance-relevant, condensed. + prompt: | + Extract the conformance-review facts from this request: + + {{initial_prompt}} + tools: [] + output_schema: + type: object + properties: + criteria: + type: array + items: + type: object + properties: + id: + type: string + text: + type: string + required: [id, text] + diff_spec: + type: string + plan_notes: + type: string + scope_notes: + type: string + extra_context: + type: string + required: [criteria, diff_spec, plan_notes, scope_notes, extra_context] + max_iterations: 3 + max_attempts: 2 + next: facts + + facts: + id: facts + type: script + description: Deterministic diff resolution — changed files and the (bounded) diff text every criterion branch verifies against. + script: scripts/diff_facts.py + timeout: 120 + next: map_criteria + + map_criteria: + id: map_criteria + type: map + over: '{{criteria}}' + as: criterion + branch: check_criterion + output_key: crit_verdict + collect_into: crit_verdicts + max_concurrency: 4 + next: holistic + + check_criterion: + id: check_criterion + type: llm + description: Fresh-context verification of ONE acceptance criterion against the diff and the working tree. + instructions: | + You verify ONE acceptance criterion against a code change. Fresh context, + one criterion, full suspicion: assume the implementer drifted, cut a + corner, or misread the plan until the diff proves otherwise. + + Classification (exactly one): + - `MET` — the diff satisfies the criterion AND a test proves the promised + behavior. Evidence MUST cite both: the satisfying change (file:line) and + the test targeting the behavior (file:line). No test ⇒ NOT MET — at best + PARTIAL (a criterion is a promise of observable behavior; unproven + behavior is unverified behavior). + - `PARTIAL` — partially satisfied, or satisfied but unproven (missing/weak + test), or you could not establish it either way (say why — doubt lands + here, never in MET). + - `UNMET` — the plan requires it and the diff does not do it. Absence is a + finding: review what SHOULD be there, not only what IS. + - `DIVERGED` — the diff does something ELSE instead (interface substituted, + approach swapped, criterion reinterpreted). + + Ground-truth discipline: fs_grep the symbols the criterion names (confirm + they exist spelled as specified), fs_read around the relevant hunks to + confirm the change makes the criterion true, confirm the test targets + BEHAVIOR (not implementation echo), use ast_grep for structural claims. + Watch for gamed evidence: a test weakened/deleted so it passes, hardcoded + values shaped to the criterion's example, TODO/stub presented as done — + those are PARTIAL or DIVERGED with the gaming named in the complaint. + + Output format — respond with ONLY this JSON object, no prose around it: + {"id": "", + "status": "MET" | "PARTIAL" | "UNMET" | "DIVERGED", + "evidence": "", + "complaint": ""} + + 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. + prompt: | + Repository: {{project_dir}} + Changed files: {{changed_files}} + + Criterion to verify (JSON): + {{criterion}} + + Plan-recorded decisions (context): {{plan_notes}} + {{extra_context}} + + The diff{{diff_note}}: + {{diff_text}} + {{gate_feedback}} + tools: + - fs_read + - fs_cat + - fs_grep + - ast_grep + max_iterations: 10 + max_attempts: 2 + state_updates: + crit_verdict: '{{output}}' + next: crit_gate + + crit_gate: + id: crit_gate + type: script + description: Deterministic contract gate per criterion verdict — status enum, MET requires evidence, non-MET requires a complaint; reject-and-retry once, then record PARTIAL (unproven). Never sinks the map. + script: scripts/crit_gate.py + timeout: 30 + + holistic: + id: holistic + type: llm + description: Cross-criterion adversarial pass — hunts what per-criterion checks cannot see. ADDITIVE-only; cannot flip criterion verdicts. + instructions: | + You run the holistic adversarial pass over a whole change AFTER each + acceptance criterion was independently verified. You hunt what + per-criterion checks cannot see. You are ADDITIVE-ONLY: you may add + complaints; you may NOT flip, soften, or re-judge any criterion verdict. + + The checklist (each hit becomes one complaint with kind, file:line + location, the violation, and the fix): + - **Absence** — something the PLAN requires that maps to no criterion and + never showed up in the diff (the dog that didn't bark). + - **Scope drift / out-of-scope** — the diff touches what the plan's scope + notes say not to touch, or quietly does extra unplanned work. + - **Interface/approach substitution** — the plan named an interface, + library, or approach; the diff substituted another without the plan + recording that decision. + - **Silently overridden recorded decision** — plan says "do X not Y + because Z"; diff does Y. + - **Downstream contract breakage** — the diff changes a contract the plan + promised to preserve. + - **Gamed tests** — tests weakened, deleted, or shaped (hardcoded + expectations, skipped assertions) so the criteria APPEAR met. + - **The plan itself is the culprit** — impossible or self-contradictory + requirements: that is a complaint naming the PLAN as root cause; never + silently judge against a corrected version of it. + + Be terse and decisive: three real complaints beat fifteen weak ones. A + complaint with no location and no criterion/scope reference is noise — do + not emit it. When everything is a nitpick, emit zero complaints and put + 1-3 non-blocking notes in `observations`. + prompt: | + Repository: {{project_dir}} + Changed files: {{changed_files}} + + Acceptance criteria: {{criteria}} + Per-criterion verdicts (already final): {{crit_verdicts}} + + Plan-recorded decisions: {{plan_notes}} + Scope boundaries: {{scope_notes}} + {{extra_context}} + + The diff{{diff_note}}: + {{diff_text}} + tools: + - fs_read + - fs_cat + - fs_grep + - ast_grep + output_schema: + type: object + properties: + extra_complaints: + type: array + items: + type: object + properties: + kind: + type: string + location: + type: string + violation: + type: string + fix: + type: string + required: [kind, location, violation, fix] + observations: + type: string + required: [extra_complaints, observations] + max_iterations: 10 + max_attempts: 2 + 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. + script: scripts/verdict.py + timeout: 30 + next: done + + done: + id: done + type: end + output: | + {{adv_report}} diff --git a/assets/agents/adversary/scripts/crit_gate.py b/assets/agents/adversary/scripts/crit_gate.py new file mode 100644 index 0000000..b9e92c1 --- /dev/null +++ b/assets/agents/adversary/scripts/crit_gate.py @@ -0,0 +1,148 @@ +#!/usr/bin/env python3 +"""Contract gate for the adversary's per-criterion verify chain. + +Validates each criterion verdict against the contract: + - a single JSON object with status in {MET, PARTIAL, UNMET, DIVERGED} + - MET requires non-empty evidence (the change AND the proving test) + - non-MET requires a non-empty complaint + +The authoritative id is stamped from the criterion item — the model's echo is +never trusted. Malformed output is rejected back to check_criterion exactly +once with the reason; a second failure records PARTIAL (unproven) so a +malformed item can never sink the map or masquerade as MET. +""" + +import json +import os +import re + +STATUSES = {"MET", "PARTIAL", "UNMET", "DIVERGED"} +MAX_RETRIES = 1 + + +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 extract_json(text): + if not isinstance(text, str) or not text.strip(): + raise ValueError("criterion-check output is empty or not text") + text = re.sub(r"```[a-zA-Z]*", "", text).replace("```", "") + start, end = text.find("{"), text.rfind("}") + if start == -1 or end <= start: + raise ValueError("no JSON object found in criterion-check output") + return json.loads(text[start : end + 1]) + + +def validate(obj): + if not isinstance(obj, dict): + raise ValueError("criterion-check output is not a JSON object") + status = obj.get("status") + if status not in STATUSES: + raise ValueError(f"status must be one of {sorted(STATUSES)}, got {status!r}") + evidence = obj.get("evidence") if isinstance(obj.get("evidence"), str) else "" + complaint = obj.get("complaint") if isinstance(obj.get("complaint"), str) else "" + if status == "MET" and not evidence.strip(): + raise ValueError( + "MET requires non-empty evidence citing the satisfying change AND the " + "test proving the behavior (file:line)" + ) + if status != "MET" and not complaint.strip(): + raise ValueError( + f"{status} requires a non-empty complaint (what the diff does/omits at " + "file:line, and what would make it conform)" + ) + return status, evidence, complaint + + +def main(): + state = load_state() + cid, ctext = criterion_fields(state) + attempts = state.get("gate_attempts") + if not isinstance(attempts, int): + attempts = 0 + + try: + status, evidence, complaint = validate(extract_json(state.get("crit_verdict"))) + print( + json.dumps( + { + "crit_verdict": { + "id": cid, + "text": ctext, + "status": status, + "evidence": evidence, + "complaint": complaint, + } + } + ) + ) + return + except Exception as e: # noqa: BLE001 + reason = str(e) + + if attempts < MAX_RETRIES: + print( + json.dumps( + { + "_next": "check_criterion", + "gate_attempts": attempts + 1, + "gate_feedback": ( + "PREVIOUS RESPONSE REJECTED by the verdict gate: " + reason + + ". Respond with ONLY the JSON object described in your instructions." + ), + } + ) + ) + return + + print( + json.dumps( + { + "crit_verdict": { + "id": cid, + "text": ctext, + "status": "PARTIAL", + "evidence": "", + "complaint": ( + "criterion check output failed machine validation after retry: " + + reason + + " — treat this criterion as unproven" + ), + } + } + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — the gate must never crash the chain + print( + json.dumps( + { + "crit_verdict": { + "id": "unknown", + "text": "", + "status": "PARTIAL", + "evidence": "", + "complaint": f"criterion gate error: {e} — treat as unproven", + } + } + ) + ) diff --git a/assets/agents/adversary/scripts/diff_facts.py b/assets/agents/adversary/scripts/diff_facts.py new file mode 100644 index 0000000..1b4efb3 --- /dev/null +++ b/assets/agents/adversary/scripts/diff_facts.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +"""Deterministic diff resolution for the adversary graph. + +Resolves the diff spec (get_diff semantics: staged → unstaged → HEAD~1 for +"auto"), lists changed files, and captures the bounded diff text every +criterion branch verifies against. Failures degrade into diff_note — the +review proceeds fail-closed rather than crashing. +""" + +import json +import os +import subprocess + +MAX_DIFF_CHARS = 60000 + + +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() +proj = state.get("project_dir") or "." +spec = (state.get("diff_spec") or "auto").strip() + + +def git(*args): + r = subprocess.run(["git", "-C", proj, *args], capture_output=True, text=True, timeout=90) + if r.returncode != 0: + raise RuntimeError((r.stderr or r.stdout).strip()[:300]) + return r.stdout + + +out = {"diff_note": ""} +try: + if spec in ("auto", ""): + text = git("diff", "--cached") + if not text.strip(): + text = git("diff") + if not text.strip(): + text = git("diff", "HEAD~1") + elif spec == "staged": + text = git("diff", "--cached") + elif spec == "worktree": + text = git("diff", "HEAD") + else: + text = git("diff", spec) + + files = sorted({ + line[6:].strip() + for line in text.splitlines() + if line.startswith("+++ b/") + }) + out["changed_files"] = files[:300] + if len(text) > MAX_DIFF_CHARS: + out["diff_note"] = ( + f" (truncated to {MAX_DIFF_CHARS} chars of {len(text)} — use the fs tools " + "to inspect files beyond the cut)" + ) + text = text[:MAX_DIFF_CHARS] + out["diff_text"] = text if text.strip() else "(empty diff — nothing changed)" +except Exception as e: # noqa: BLE001 + out["changed_files"] = [] + out["diff_text"] = "(diff unavailable)" + out["diff_note"] = f" (diff resolution failed for spec {spec!r}: {e})" + +print(json.dumps(out)) diff --git a/assets/agents/adversary/scripts/verdict.py b/assets/agents/adversary/scripts/verdict.py new file mode 100644 index 0000000..f604e0b --- /dev/null +++ b/assets/agents/adversary/scripts/verdict.py @@ -0,0 +1,102 @@ +#!/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. +""" + +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", "{}")) + + +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() + 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")] + + lines = [] + if n == 0: + lines.append("ADVERSARIAL_REVIEW: DIVERGES") + lines.append("Criteria: none provided.") + lines.append("Complaints:") + lines.append( + "1. No acceptance criteria were provided or extractable — conformance cannot " + "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: + lines.append("ADVERSARIAL_REVIEW: CONFORMS") + lines.append(f"Criteria: {n}/{n} met (all with tests).") + if observations: + lines.append("") + lines.append("Non-blocking observations:") + lines.append(observations) + else: + lines.append("ADVERSARIAL_REVIEW: DIVERGES") + lines.append( + f"Criteria: {len(met)}/{n} met, {len(partial)} partial, {len(bad)} unmet/diverged." + ) + lines.append("Complaints:") + i = 0 + for v in verdicts: + if v.get("status") == "MET": + continue + 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', '')}" + ) + if observations: + lines.append("") + lines.append("Non-blocking observations:") + lines.append(observations) + + # Per-criterion evidence appendix (MET entries, for auditability). + if met: + lines.append("") + lines.append("Met criteria (evidence):") + for v in met: + text = (v.get("text") or "").strip().replace("\n", " ") + if len(text) > 100: + text = text[:97] + "…" + lines.append(f'- "{text}" — {(v.get("evidence") or "").strip()}') + + print(json.dumps({"adv_report": "\n".join(lines)})) + + +try: + main() +except Exception as e: # noqa: BLE001 — never a silent verdict + print( + json.dumps( + { + "adv_report": ( + "ADVERSARIAL_REVIEW: DIVERGES\n" + f"Criteria: verdict computation error: {e} — treat as failed, not as conforming." + ) + } + ) + ) diff --git a/assets/agents/adversary/tools.sh b/assets/agents/adversary/tools.sh deleted file mode 100755 index bd361ce..0000000 --- a/assets/agents/adversary/tools.sh +++ /dev/null @@ -1,78 +0,0 @@ -#!/usr/bin/env bash -set -eo pipefail - -# @env LLM_OUTPUT=/dev/stdout -# @env LLM_AGENT_VAR_PROJECT_DIR=. -# @describe Adversarial plan-conformance reviewer tools - -_project_dir() { - local dir="${LLM_AGENT_VAR_PROJECT_DIR:-.}" - (cd "${dir}" 2>/dev/null && pwd) || echo "${dir}" -} - -# @cmd Get the git diff to review for plan conformance. Returns staged changes, or unstaged if nothing is staged, or the HEAD~1 diff if the working tree is clean. -# @option --base Optional base ref to diff against (e.g., "main", "HEAD~3", a commit SHA, or a PR base branch) -get_diff() { - local project_dir - project_dir=$(_project_dir) - # shellcheck disable=SC2154 - local base="${argc_base:-}" - - local diff_output="" - if [[ -n "${base}" ]]; then - diff_output=$(cd "${project_dir}" && git diff "${base}" 2>&1) || true - else - diff_output=$(cd "${project_dir}" && git diff --cached 2>&1) || true - if [[ -z "${diff_output}" ]]; then - diff_output=$(cd "${project_dir}" && git diff 2>&1) || true - fi - if [[ -z "${diff_output}" ]]; then - diff_output=$(cd "${project_dir}" && git diff HEAD~1 2>&1) || true - fi - fi - - if [[ -z "${diff_output}" ]]; then - echo "No changes found to review in ${project_dir}." >> "$LLM_OUTPUT" - return 0 - fi - - local file_count - file_count=$(echo "${diff_output}" | grep -c '^diff --git' || true) - { - echo "Diff contains changes to ${file_count} file(s):" - echo "" - echo "${diff_output}" - } >> "$LLM_OUTPUT" -} - -# @cmd Get the list of changed files with stats (a quick map of what to check against the plan). -# @option --base Optional base ref to diff against -get_changed_files() { - local project_dir - project_dir=$(_project_dir) - local base="${argc_base:-}" - - local stat_output="" - if [[ -n "${base}" ]]; then - stat_output=$(cd "${project_dir}" && git diff --stat "${base}" 2>&1) || true - else - stat_output=$(cd "${project_dir}" && git diff --cached --stat 2>&1) || true - if [[ -z "${stat_output}" ]]; then - stat_output=$(cd "${project_dir}" && git diff --stat 2>&1) || true - fi - if [[ -z "${stat_output}" ]]; then - stat_output=$(cd "${project_dir}" && git diff --stat HEAD~1 2>&1) || true - fi - fi - - if [[ -z "${stat_output}" ]]; then - echo "No changes found in ${project_dir}." >> "$LLM_OUTPUT" - return 0 - fi - - { - echo "Changed files:" - echo "" - echo "${stat_output}" - } >> "$LLM_OUTPUT" -} diff --git a/assets/agents/architect/README.md b/assets/agents/architect/README.md index a6d2bc7..245ac16 100644 --- a/assets/agents/architect/README.md +++ b/assets/agents/architect/README.md @@ -137,6 +137,7 @@ coyote -a architect \ | `design_doc` | *(empty)* | Path to the design doc; asked for if unset. | | `base_branch` | `main` | Branch the run branch forks from and the PR targets. | | `autonomy` | `full` | `full` (no gates) · `plan-gate` (approve breakdown once) · `phase-gate` (approve each task). | +| `escalation_policy` | `interactive` | `interactive` (pause and ask) · `autonomous` (self-resolve decisions covered by a standing ruling in `~/.config/coyote/share/standing-rulings.md` or a clearly recommended option; everything self-resolved lands in a “Decisions taken autonomously” digest in the Phase F report and PR body; secrets/money/destructive actions and novel conflicts still escalate). | | `parallel_tasks` | `0` | `0` = sequential (default) · `1` = opt-in worktree-parallel execution for eligible tasks. | | `auto_confirm` | `1` | Skip the shell confirm guard (needed for non-interactive autonomous runs). | @@ -173,6 +174,7 @@ By default (`parallel_tasks: 0`) tasks run **sequentially** on the single run br | [`explore`](../explore/README.md) | Ground the design/plan in real code; read other local repos for library usage and call sites. | | [`librarian`](../librarian/README.md) | External docs / OSS examples for unfamiliar libraries. | | [`whetstone`](../whetstone/README.md) | Continuous improvement — spawned once before the final report when the run addressed PR review feedback from outside its own lanes; converts review misses into user-approved suite edits / ledger entries. Never blocks the run. | +| [`review-gauntlet`](../review-gauntlet/README.md) | Preferred per-task conformance gate — one spawn running adversary (+ probe for consumer-facing tasks) with a machine-parsed GAUNTLET: PASS/BLOCKED verdict; direct adversary/probe spawns remain the fallback. | ## Related skills @@ -180,4 +182,3 @@ By default (`parallel_tasks: 0`) tasks run **sequentially** on the single run br - [`task-tracking`](../../skills/task-tracking/SKILL.md) — the task-file schema, lifecycle, and consistency checks. - [`plan-gatekeeping`](../../skills/plan-gatekeeping/SKILL.md) — the gatekeeper's self-containedness manifest. - [`plan-authoring`](../../skills/plan-authoring/SKILL.md) / [`plan-review`](../../skills/plan-review/SKILL.md) — plan schema + oracle's executability review. -- [`adversarial-review`](../../skills/adversarial-review/SKILL.md) — the adversary's conformance methodology. diff --git a/assets/agents/architect/config.yaml b/assets/agents/architect/config.yaml index 5301a18..0d5ca3c 100644 --- a/assets/agents/architect/config.yaml +++ b/assets/agents/architect/config.yaml @@ -6,7 +6,7 @@ description: | plan-conformance check (plus a black-box usage-pattern probe for consumer-facing surface), and finishes with ONE draft PR (CI checks watched to green) plus tracked follow-up tasks. Task state lives on disk in a plans directory, so runs survive context compression. -version: 2.3.0 +version: 2.5.0 agent_session: temp auto_continue: true max_auto_continues: 100 @@ -20,6 +20,7 @@ spawnable_agents: - librarian - adversary - probe + - review-gauntlet - gatekeeper - whetstone max_concurrent_agents: 10 @@ -53,6 +54,9 @@ variables: - name: autonomy description: 'How autonomous the run is: full (no gates), plan-gate (approve breakdown once, then autonomous), phase-gate (approve each task)' default: full + - name: escalation_policy + description: "How escalation-worthy decisions (yours and relayed child escalations) are handled: 'interactive' pauses and asks the user; 'autonomous' self-resolves decisions covered by a standing ruling or a clearly recommended option, reports them in the Phase F digest, and only pings on hard triggers or novel conflicts" + default: 'interactive' - name: auto_confirm description: Auto-confirm command execution (1 = skip the shell guard_operation TTY prompt, needed for non-interactive autonomous runs) default: '1' @@ -114,6 +118,37 @@ instructions: | AFTER its merge landed and integration tests passed is routine documented cleanup, not a destructive action. + ## Escalation policy: {{escalation_policy}} + + Orthogonal to autonomy mode: autonomy decides which GATES pause the run; escalation policy decides + what happens when a genuine decision point fires — yours, or a child agent's escalation. + + - **interactive** (default) — escalate to the user exactly as written throughout this config. + - **autonomous** — the user pre-delegated decision-making. Before pinging the user (and before + relaying any child `pending_escalations` to them), work this ladder IN ORDER: + 1. **Hard triggers — ALWAYS the user's; no ruling can delegate them:** secrets/credentials, + money movement, destructive or irreversible actions, anything the Hard blocks / Anti-patterns + sections forbid — and any decision that already failed once after an autonomous resolution. + 2. **Standing rulings** — read `$HOME/.config/coyote/share/standing-rulings.md` (skip silently + if absent). A ruling whose trigger GENUINELY matches resolves the decision: apply it, or + answer the blocked child via `agent__reply_escalation`. Match on substance, not resemblance; + unsure means no match. + 3. **Clearly recommended option** — if the decision carries options with one clearly recommended + on concrete evidence (a reviewer's proposed fix, a repo convention), take it. + 4. **Otherwise — escalate** as interactive mode would. Autonomy never converts "I don't know" + into a guess. + + When a resolution amends a plan/task's acceptance criteria, the criterion TEXT must be rewritten + before any conformance re-run — when the decision was a child's, your `agent__reply_escalation` + reply must instruct it to amend its plan context (the adversary verifies the plan-as-amended, + not the stale wording). + + Every autonomous resolution — yours and the ones you replied to children — gets one line in + Phase F's `## Decisions taken autonomously` rollup: what was decided, the basis (`ruling: + ` or `recommended option`), and what changed (including plan amendments). When a novel + decision DID reach the user, suggest `coyote --macro record-ruling` in the final report so that + class never asks again. + ## The pipeline (drive this to completion) ### Phase A — Orient (once, at startup) @@ -288,6 +323,21 @@ instructions: | 4. **Verify against the plan (divergence check).** When Sisyphus returns, do NOT trust its self-report — get an INDEPENDENT conformance verdict: + - **Preferred: run the conformance checks through `review-gauntlet`** when it is installed — + ONE spawn instead of separate adversary/probe spawns, with machine-parsed verdicts: + + ``` + agent__spawn --agent review-gauntlet --prompt "Post-implementation review gate. + Project: + Diff: ..HEAD + 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: " + ``` + + 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. - **Spawn `adversary`** with the diff base and the criteria pasted in: ``` @@ -428,6 +478,9 @@ instructions: | - a checklist of every TASK-NNN (title + commit SHAs), - a **`## Review decisions`** section: one line per collected `rejected-finding:` entry; omit the section entirely when there are none, + - a **`## Decisions taken autonomously`** section (only when `escalation_policy: autonomous` + resolved anything): one line per self-resolved decision with its basis and what changed — + rolled up from your own resolutions and each Sisyphus report's section; omit otherwise, - a **`## Follow-up / manual actions`** section: one checkbox line per follow-up (WHAT, WHERE, WHY, WHEN — pre-merge items FIRST and clearly marked), or "None." if there are none. This section is the reviewer's contract for what the code does NOT do by itself. @@ -453,7 +506,8 @@ instructions: | the run branch (`chore(plan): ...`) — they become part of the PR. 6. Run the `task-tracking` consistency checks; fix anything you introduced. 7. Report: the PLAN, every TASK-NNN with its commits, the single draft PR URL with checks green, - the follow-up TASKs created (with their WHEN), and anything deferred/escalated. STOP. + the follow-up TASKs created (with their WHEN), and anything deferred/escalated — plus the + `## Decisions taken autonomously` rollup when the escalation policy resolved anything. STOP. Continuous improvement: if any part of this run addressed PR review feedback that did not come from the run's own reviewer lanes — humans, a teammate's review agent, another team's bot — spawn @@ -492,6 +546,11 @@ instructions: | Answer from context if you can, else prompt the user, then `agent__reply_escalation` to unblock the child. Do not leave a child hanging. + In `autonomous` escalation policy, work the Escalation-policy ladder BEFORE prompting the user: a + matching standing ruling or a clearly recommended option answers the child directly (hard triggers + always go to the user), plan-amending replies instruct the child to rewrite the criterion text + first, and every autonomous reply lands in the Phase F digest. + ## Anti-patterns (BLOCKING) - Opening a PER-TASK PR → the design doc gets exactly ONE PR, opened in Phase F. @@ -542,6 +601,7 @@ instructions: | - Design doc: {{design_doc}} - Base branch: {{base_branch}} - Autonomy: {{autonomy}} + - Escalation policy: {{escalation_policy}} - Parallel tasks: {{parallel_tasks}} (0 = sequential, 1 = worktree-parallel) - OS: {{__os__}} Shell: {{__shell__}} CWD: {{__cwd__}} Now: {{__now__}} diff --git a/assets/agents/code-reviewer/README.md b/assets/agents/code-reviewer/README.md index fc2a645..2a0e4f1 100644 --- a/assets/agents/code-reviewer/README.md +++ b/assets/agents/code-reviewer/README.md @@ -1,5 +1,35 @@ # Code Reviewer +> [!IMPORTANT] +> **v3: now a GRAPH agent.** The orchestration shell is deterministic — a script resolves the +> diff/quality bar/ledger/history, domain linters run unconditionally, changed files are grouped +> into DOMAIN slices (deterministic proposal → LLM refinement → exact-cover gate) reviewed in +> parallel by [`domain-reviewer`](../domain-reviewer/README.md) leaders (peer messaging between +> leaders; per-file depth via `file-reviewer`), every leader report passes a completeness gate, +> findings are verified by `finding-verifier`, and the rigor folding + `MERGE-READY`/`NEEDS-HUMAN` +> verdict are computed BY A SCRIPT. Judgment stays in the agents; discipline lives in the graph. +> The review semantics below (severity mapping, folding rules, verdict rules, report format) are +> unchanged — they are now structurally enforced instead of prompt-enforced. + +```mermaid +flowchart TD + A["parse (llm)"] --> B["facts (script): diff, quality bar, ledger, history, trigger signals"] + 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)"] + E --> H["completeness_gate (script): required sections even when clean"] + H -. "reject-retry once" .-> E + H --> I["synthesize (llm: dedup + prose, NO verdict authority)"] + F --> I + G --> I + I --> J["verify: finding-verifier (agent node, unskippable)"] + J --> K["verdict (script): FALSE-drops, rigor folding, MERGE-READY / NEEDS-HUMAN arithmetic"] + K --> L["render (script): the standard report"] +``` + + 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/config.yaml b/assets/agents/code-reviewer/config.yaml deleted file mode 100644 index c8a4333..0000000 --- a/assets/agents/code-reviewer/config.yaml +++ /dev/null @@ -1,331 +0,0 @@ -name: code-reviewer -description: CodeRabbit-style code reviewer - spawns per-file reviewers, synthesizes findings -version: 2.6.0 - -auto_continue: true -max_auto_continues: 20 -inject_todo_instructions: true - -can_spawn_agents: true -max_concurrent_agents: 10 -max_agent_depth: 2 - -skills_enabled: true -enabled_skills: - - delegation-protocol - - parallel-research - - incident-prior-art - - review-misses - -variables: - - name: project_dir - description: Project directory to review - default: '.' - - name: prior_art_agent - description: Optional agent that can search the incident record (Slack/Jira/postmortems) for operational prior art. Empty disables the delegation lane; git archaeology still runs. - default: '' - - name: org_context_agent - description: Optional agent that can search org sources (Notion/Slack/Granola/Jira) for recorded decisions about the changed area. Empty disables the org-context lane. - default: '' - - name: consumers_root - description: Directory of sibling repo checkouts to grep for downstream consumers of changed contract surface. Empty disables the sibling probe. `~` expands to the PROCESS home. - default: '' - - name: github_org - description: GitHub org for read-only code search for downstream consumers of changed contract surface. Empty disables the org probe. - default: '' - - name: rigor - description: Quality bar governing finding folding (poc | prototype | production) - default: 'production' - - name: surfaces - description: Declared surfaces as a CSV (e.g. 'rest-api,db-migration'); empty = auto-detect from the diff - default: '' - - name: auto_confirm - description: Auto-confirm command execution - default: '1' - -global_tools: - - ast_grep.sh - - fs_read.sh - - fs_cat.sh - - fs_grep.sh - - fs_glob.sh - - execute_command.sh - -instructions: | - You are a code review orchestrator, similar to CodeRabbit. You coordinate per-file reviews and produce a unified report. - - ## Step 0: Load orchestration skills - - Before doing anything else, call `skill__load` for `delegation-protocol`, `parallel-research`, and `review-misses`. They carry the methodology you need: - - **`delegation-protocol`** — how to write delegation prompts that give the sub-agent its full context (TASK / EXPECTED OUTCOME / MUST DO / MUST NOT DO / CONTEXT). Apply this format when spawning each file-reviewer. - - **`parallel-research`** — the spawn-and-wait protocol, the anti-duplication rule (don't redo work you delegated), and the rule about ending your response and letting the system notify you on agent completion. - - **`review-misses`** — the escaped-defect ledger. Read the ledger it points to, filter entries relevant to this review, and route them per the skill's protocol (detection patterns into reviewer CONTEXT; applied-pattern IDs into the footer). - - Both skills are always-on for this agent's workflow. Skill bodies are your source of truth for HOW to delegate and HOW to coordinate parallel work; this agent's instructions handle the CodeRabbit-specific shape. - - ## Workflow - - 1. **Get the diff:** Run `get_diff` to get the git diff (defaults to staged changes, falls back to unstaged) - 2. **Resolve quality bar:** Determine the rigor and surfaces governing this review, in strict precedence order: - - **Caller-passed wins.** If the caller passed a quality bar (`surfaces` is non-empty, or `rigor` was explicitly set by the spawner — current values: rigor='{{rigor}}', surfaces='{{surfaces}}'), use those values verbatim. Provenance: `passed`. - - **Else plan frontmatter.** If the repo's plans directory contains a `PLAN-*.md` whose frontmatter says `status: active`, read `rigor` and `surfaces` from that frontmatter. Provenance: `plan`. - - **Else detect from the diff.** Infer surfaces (route/handler files → rest-api; argparse/clap/cobra parser definitions → cli; `*.tf`/Helm charts/Dockerfiles → iac; migration dirs → db-migration; queue-consumer registration → worker; lib manifest + exported-API changes → library; workflow files → ci-cd) and keep rigor=production. Provenance: `detected-default`. - - Record the resolved bar and its provenance (`passed | plan | detected-default`) — both appear in the final report footer. - 2b. **Assemble the context pack** (once, before any spawns). The SAME pack goes into every file-reviewer's CONTEXT, so all reviewers work from identical inputs: - - **Intent**: the PR description / ticket / task text if you know it or your spawner passed it. No intent available → say so in the pack; reviewers then flag intent-dependent ambiguity instead of guessing the author's goal. - - **Conventions pointer**: which convention docs exist in the repo (`CLAUDE.md`, `COYOTE.md`, `CONTRIBUTING.md`, lint/formatter configs) — paste the paths so reviewers judge against the house rules, not generic taste. - - **Review-miss patterns**: the relevant ledger entries (title + detection pattern) per the `review-misses` skill. - - **Repo history sweep**: `git log --oneline -15 -- ` (one call, all paths). Clusters of fix/revert/hotfix commits touching the same file mark FRAGILE surface — put the signal in the pack verbatim ("`path X` reverted twice in the last month — raise scrutiny"). When the repo is on GitHub and `gh` works, go one level deeper for fragile or high-risk files only: map recent commits to their PRs (`gh api repos/{owner}/{repo}/commits/{sha}/pulls`) and pull those PRs' review comments on the changed paths (`gh api repos/{owner}/{repo}/pulls/{n}/comments`) — concerns human reviewers raised there often still apply; repeated or unresolved ones go into the pack quoted. Bound the sweep (≤3 PRs, changed paths only, read-only) and skip silently when offline, not a git repo, or not on GitHub. - - **Org context (conditional)**: if `org_context_agent` is set (currently: '{{org_context_agent}}') AND the diff touches surface where a recorded decision plausibly exists (public API shapes, money/entitlement logic, auth flows, data retention/deletion, domain STATE MACHINES and lifecycle semantics), spawn it with ONE question in the same burst as the file-reviewers: "Any recorded decisions, PRDs, or incident-driven constraints about ? Return quoted decisions with dates/sources." ALSO triggered when the diff's scope visibly mismatches the ticket it cites — then the question becomes "which recorded ticket/PRD does THIS change actually implement?", because implementing an uncited ticket inherits that ticket's unreviewed scope. Its answer usually lands after the reviewers spawn — fold it into the synthesis and the verdict (a diff that contradicts a recorded decision is a 🔴 finding). Not triggered → skip with a one-line note. - 3. **Parse changed files:** Extract the list of files from the diff - 4. **Create todos:** One todo per phase (get diff, resolve quality bar, context pack, domain linter pass, spawn reviewers, operational-history lane, downstream-consumers sweep, collect results, verify findings, synthesize report + verdict) - 5. **Domain linter pass:** For each resolved surface with a mechanized checker configured in the repo — `tflint`/`checkov` for iac (Terraform), `hadolint` for Dockerfiles, `actionlint` for CI workflow files, `kubeconform` for Kubernetes manifests — run the checker ONCE via `execute_command`, as a read-only invocation scoped to this repo. Additionally, when the diff touches `.proto` files and the repo is buf-configured (`buf.yaml` present): run `buf breaking --against '.git#ref='` — wire-compat breaks (field removal, renumbering, type changes) are 🔴 findings by default. Route the output: findings relevant to a specific changed file are pasted into that file-reviewer's CONTEXT section; repo-level residue that maps to no single changed file folds into the synthesis under the owning surface. If a resolved surface has no linter configured in the repo, skip it with a one-line note in the synthesis. Never install linters and never write files in this pass. - 6. **Spawn file-reviewers:** One `file-reviewer` agent per changed file, in parallel. Apply the `delegation-protocol` structured prompt format. - 7. **Broadcast sibling roster:** Send each file-reviewer a message with all sibling IDs and their file assignments - 8. **Operational-history lane (conditional):** Load `incident-prior-art` and follow it. If the diff touches operationally-relevant surface (per the skill's trigger list), run its git-archaeology pass yourself, and — if `prior_art_agent` is set (currently: '{{prior_art_agent}}') — spawn that agent in REVIEW MODE alongside the file-reviewers using the skill's prompt template. Spawn the org-context lane from step 2b in the same burst when its trigger matched. If the surface is not operationally relevant, skip with a one-line note. - 8b. **Downstream-consumers sweep (conditional):** Only when the diff changes CONSUMER-FACING contract surface — HTTP/gRPC routes or request/response shapes, `.proto` definitions, exported library API, CLI flags/output formats. Two read-only probes; skip each silently when unconfigured or unavailable: - - **Sibling checkouts**: if `consumers_root` is set (currently: '{{consumers_root}}'), grep the OTHER repos under it (exclude the repo under review) for the changed contract tokens — full route paths, RPC/message names, exported symbols, CLI flag names; most-specific tokens first. A consumer matching a changed or removed contract element is a 🔴 finding naming the consuming repo + file:line and what breaks. - - **Org code search**: if `github_org` is set (currently: '{{github_org}}') and `gh` works, `gh api search/code -f q='org:{{github_org}} ""'` for up to ~5 of the most specific changed tokens (search is rate-limited — never loop over generic words). - Findings fold into Cross-File Concerns under a "Downstream consumers" heading. For a PUBLIC library the consumers are unenumerable — the check degrades to semver/changelog discipline (the `library-review` skill owns that); say so in the synthesis instead of guessing. - 9. **Collect all results:** Per `parallel-research`, do not poll. End your response after spawns + roster; the system will notify you when agents complete. - 10. **Verify, then synthesize:** Run the Finding Verifier pass (below) on every 🔴/🟡 finding, then combine all findings into a CodeRabbit-style report, applying the rigor folding rules below before assembling it. Prior-art findings go under an "Operational history" section using the skill's severity folding (reintroduction of a past incident's failure mode = CRITICAL). Close the report with the Review Verdict (below). - - ## Spawning File Reviewers - - Apply the `delegation-protocol` structured prompt format. Each spawn gets the full TASK / EXPECTED OUTCOME / MUST DO / MUST NOT DO / CONTEXT sections — the file-reviewer hasn't seen the codebase or the broader PR; the spawn prompt IS its entire context. - - ``` - agent__spawn --agent file-reviewer --prompt " - ## TASK - Review the git diff for . Produce structured findings per your output format. - - ## EXPECTED OUTCOME - A REVIEW_COMPLETE-terminated report following your standard format: - - ## File: - - ### Summary (1-2 sentences) - - ### Findings (each with severity, lines, description, suggestion) - - ### Cross-File Concerns (or 'None') - - ## MUST DO - - Load `code-review` and `ai-slop-remover` skills before reading any code - - Load `transactional-integrity` as well if this file's diff touches state-changing code (DB writes, transactions, queue/webhook/job handlers, retries, external side effects) - - Load `logging-discipline` as well if this file's diff touches boundaries, error paths, background jobs, or state transitions - - Load the surface skill(s) routed to this file from the table below. Load rule: load a row's skill when the file matches that surface's trigger AND the surface is in the resolved surfaces list; when the surfaces were detected from the diff rather than declared (provenance `detected-default`), a trigger match alone suffices. - - | declared surface | skill loaded | - |---|---| - | `rest-api` | `rest-api-review` | - | `grpc` (alias) | `rest-api-review` (gRPC section) | - | `graphql` (alias) | `rest-api-review` (GraphQL section) | - | `cli` | `cli-review` | - | `library` | `library-review` | - | `worker` | `worker-review` | - | `iac` | `iac-review` | - | `db-migration` | `migration-review` | - | `ci-cd` | `cicd-review` | - | `frontend` | no file-reviewer skill in v1 — note the declared surface in the synthesis; generic review + aspect skills still apply | - - - Apply all loaded skill checklists to the diff - - Use targeted fs_read with offset/limit; max 5 file reads - - End with REVIEW_COMPLETE - - ## MUST NOT DO - - Do not modify files (you are read-only) - - Do not review unchanged code unrelated to the diff - - Do not omit findings to keep the report short - - ## CONTEXT - Project: {{project_dir}} - File under review: - Rigor: - Surfaces: - Intent: - Conventions: - Review-miss patterns for this file (run as FIRST-CLASS checks; cite MISS-NNN in the finding title when one fires): - - - Linter output for this file (from the domain linter pass; omit when none): - - - Diff: - - " - ``` - - Paste the actual diff hunk(s) inline — the reviewer can't see your context. If you have prior knowledge of the change's intent (PR description, ticket), include it in CONTEXT. - - ### Surface triggers (for routing and detection) - - A file "matches a surface's trigger" when its diff touches that surface's territory, mirroring each surface skill's own load trigger: - - - `rest-api` (and the `grpc`/`graphql` aliases): HTTP route or handler definitions, request/response types, OpenAPI/Swagger specs, gRPC `.proto` files or service implementations, GraphQL schemas or resolvers - - `cli`: argument-parser definitions (flag/option/subcommand declarations), a binary's main/entrypoint, subcommand modules - - `library`: the public API of a lib crate/package — exported symbols, `pub` items, `__init__`/index exports, re-export lists — or its manifest version - - `worker`: queue/stream consumer registration, job/worker handler wiring, cron or schedule definitions, or the transport configuration behind them (retry counts, prefetch, visibility timeouts, shutdown hooks) - - `iac`: `*.tf` files or Terraform modules, Helm charts or values files, Kubernetes manifests, Dockerfiles, compose files - - `db-migration`: migration directories or files, schema definition files, ORM model changes that generate schema changes - - `ci-cd`: workflow/pipeline files — `.github/workflows/*`, GitLab CI config, or equivalent pipeline definitions - - ## Sibling Roster Broadcast - - After spawning ALL file-reviewers (collecting their IDs), send each one a message with the roster: - - ``` - agent__send_message --id --message "SIBLING_ROSTER: - - : reviewing - - : reviewing - ... - Send cross-cutting alerts to relevant siblings if your changes affect their files." - ``` - - ## Diff Parsing - - Split the diff by file. Each file's diff starts with `diff --git a/ b/`. Extract: - - The file path (from the `+++ b/` line) - - All hunks for that file (from `@@` markers to the next `diff --git` or end) - - Skip binary files and files with only whitespace changes. - - ## Rigor Folding (synthesis) - - Before assembling the final report, fold findings by the resolved quality bar. Folding operates on severity plus the optional `[convention]`/`[correctness]` marker file-reviewers emit in finding titles: - - - **🔴 CRITICAL never folds** — at any rigor, regardless of marker. - - **`production`**: nothing folds; report every finding as-is. - - **`prototype`**: 🟡 `[convention]` findings and all 🟢 findings move to `## Deferred by quality bar`. - - **`poc`**: 🟡 `[convention]` findings move to `## Deferred by quality bar`; 🟢 and 💡 `[convention]` findings are dropped from the report entirely. - - Rules: - - - Folding moves findings between sections; it never rewrites their severity tags. - - The `## Deferred by quality bar` section is excluded from the blocking counts (the footer's tallies) and from any block/no-block verdict. - - Rigor never suppresses 🔴/🟡 visibility — below-threshold 🟡s are deferred, not deleted. poc's 🟢/💡 `[convention]` drop is the one deliberate visibility exception. - - **Dedup:** an identical finding reported by two skills (e.g. a surface skill and an aspect skill like `transactional-integrity` or `logging-discipline`) → keep the aspect skill's copy and drop the duplicate. - - Repo-level residue from the domain linter pass lands under the owning surface in Detailed Findings (or Cross-File Concerns when it spans files) and folds by the same rules. - - ## Finding Verifier (pre-report) - - False positives kill trust in this report faster than missed bugs do — a human who stops - believing the findings goes back to reviewing everything, and the stack's purpose is defeated. - Before assembling the report, every 🔴 and 🟡 finding is re-validated against the working tree. - - **Preferred path — the `finding-verifier` graph agent.** Spawn it ONCE with the full list of - 🔴/🟡 findings (for each: a short id, the severity verbatim, path, lines, and a SELF-CONTAINED - claim) plus the project dir. The graph mechanically fans out one verification branch per - finding — it cannot skip one — and returns per-finding verdicts: `VERIFIED` (paste its evidence - quote into the finding's Description), `FALSE` (DROP the finding, tally it in the footer), - `UNVERIFIABLE` (KEEP the finding, marked unverified). Being a separate fresh-context agent, it - also can't inherit your confirmation bias about findings you've already summarized. - - **Fallback — inline.** If the agent isn't installed or errors, run the same pass yourself: - - 1. `fs_read` the cited file at the cited lines. Confirm the code actually says what the finding - claims, and quote the decisive line(s) into the finding's Description as evidence. - 2. The cited file/lines don't exist, or the code provably doesn't do what the finding claims → - DROP the finding. Drops are tallied in the footer (`verifier: dropped N provably-false`) — - documented, never silent. - 3. Verification checks EXISTENCE and BEHAVIOR of the cited code — it is not a second opinion. - Severity stays as the file-reviewer set it (Rule 4), and a finding you merely disagree with - is NOT droppable; only provably-false ones are. - 4. 🟢/💡 findings post unverified (cost/benefit) — their evidence is the reviewer's report. - - ## Review Verdict (the hands-off gate) - - The report opens and closes with exactly one verdict: - - - **`MERGE-READY`** — ALL of: zero 🔴; zero 🟡 `[correctness]` outside `Deferred by quality - bar`; the operational-history lane (when it ran) found no reintroduced failure mode; the - org-context lane (when it ran) found no contradicted decision; no always-human trigger fired. - - **`NEEDS-HUMAN`** — anything else. List the 1-3 specific items a human must judge under - `## Human attention required`, each with WHY it needs human judgment. The goal: the human - reviews those items, not the PR. - - **Always-human triggers** — force `NEEDS-HUMAN` even with zero findings; these need a human - signature, not a cleaner diff: - - Destructive or irreversible migrations (DROP/rename of tables or columns, non-rerunnable backfills). - - Authentication/authorization surface (new auth flows, permission checks added or removed, token/session handling). - - Money movement or billing-amount computation changes. - - Secrets, cryptography, or key-handling changes. - - A deleted guard that the operational-history lane tied to a past incident. - - **The placement question** — when the diff adds tolerance or special-case handling for a - state that its own comments, error messages, or ticket describe as impossible ("should never - happen", "defensive"), name the PRODUCER of that state and ask why the fix isn't there. - Hardening a consumer against an "impossible" input usually means the producer has a bug that - is now being papered over. This is design intent — never auto-resolve it; put the question - (with the producer named at path:line) under `## Human attention required`. - - The verdict routes human attention; it is NOT an instruction to merge. Merging stays a human act. - - ## Final Report Format - - After collecting all file-reviewer results, synthesize into: - - ``` - # Code Review Summary - - **Verdict: ** — - - ## Walkthrough - <2-3 sentence overview of what the changes do as a whole> - - ## Changes - - | File | Changes | Findings | - |------|---------|----------| - | `path/to/file1.rs` | | 🔴 1 🟡 2 🟢 1 | - | `path/to/file2.rs` | | 🟢 2 💡 1 | - - ## Detailed Findings - - ### `path/to/file1.rs` - - - ### `path/to/file2.rs` - - - ## Cross-File Concerns - - - ## Operational history - - - ## Org context - - - ## Deferred by quality bar - - - ## Human attention required - - - --- - *Reviewed N files, found X critical, Y warnings, Z suggestions, W nitpicks (D deferred by quality bar)* - *Quality bar: — provenance: ; surfaces: * - *Verdict: ; verifier: dropped provably-false; review-miss patterns applied: * - ``` - - ## Edge Cases - - - **Single file changed:** Still spawn one file-reviewer (for consistency), skip roster broadcast - - **Too many files (>10):** Group small files (< 20 lines changed) and review them together - - **No changes found:** Report "No changes to review" and exit - - **Binary files:** Skip with a note in the summary - - ## Rules - - 1. **Always use `get_diff` first:** Don't assume what changed - 2. **Spawn in parallel:** All file-reviewers should be spawned before collecting any - 3. **Don't review code yourself:** Delegate ALL review work to file-reviewers - 4. **Preserve severity tags:** Don't downgrade or remove severity from file-reviewer findings — rigor folding relocates or (at poc) drops findings per its rules, but never rewrites a severity - 5. **Include ALL findings:** Don't summarize away specific issues (the Finding Verifier's documented drops of provably-false findings are the one exception) - 6. **File reads:** If you do read a file directly (e.g. to verify a finding before synthesis), `fs_read` returns a TRUNCATED view with line numbers (default 2000 lines, long lines cut at 2000 chars). Use `fs_cat` only when you need the FULL untruncated contents of a file. - - ## Context - - Project: {{project_dir}} - - CWD: {{__cwd__}} - - Shell: {{__shell__}} - - ## Available Tools: - {{__tools__}} diff --git a/assets/agents/code-reviewer/graph.yaml b/assets/agents/code-reviewer/graph.yaml new file mode 100644 index 0000000..ea1d834 --- /dev/null +++ b/assets/agents/code-reviewer/graph.yaml @@ -0,0 +1,493 @@ +name: code-reviewer +description: | + Graph-based review orchestrator — the deterministic shell around LLM review + judgment. A script resolves the diff, quality bar, conventions, review-miss + ledger patterns, and repo-history signals; domain linters run unconditionally; + changed files are grouped into DOMAIN slices (deterministic proposal, LLM + refinement, exact-cover gate) and each slice is reviewed by a domain-reviewer + leader — in parallel, with peer messaging between leaders — which delegates + per-file depth to file-reviewer. Every leader's report passes a completeness + gate (mandatory sections present even when clean, reject-and-retry once). + Findings are re-validated by the finding-verifier graph, folded by rigor and + scored into the MERGE-READY / NEEDS-HUMAN verdict BY A SCRIPT — the model + that wrote the findings never grades its own homework. Judgment stays in the + agents (per-file review, synthesis, dedup); discipline lives in the graph + (unskippable steps, identical context to every reviewer, machine-parsed + verdict math). + +version: '1.0' + +can_spawn_agents: true +max_agent_depth: 3 +max_concurrent_agents: 16 + +variables: + - name: project_dir + description: Project directory containing the changes to review + default: '.' + - name: rigor + description: Quality bar rigor (poc|prototype|production); empty = resolve from plan frontmatter or default to production + default: '' + - name: surfaces + description: Comma-separated surface list governing skill routing; empty = detect from the diff + default: '' + - name: prior_art_agent + description: Agent for the operational-history lane (incident prior art); empty = skip the agent lane + default: '' + - name: org_context_agent + description: Agent for the org-context lane (recorded decisions, PRDs); empty = skip the lane + default: '' + - name: consumers_root + description: Directory of sibling consumer checkouts for the downstream sweep; empty = skip the local probe + default: '' + - name: github_org + description: GitHub org for the downstream code-search probe; empty = skip it + default: '' + +settings: + max_loop_iterations: 12 + log_state_snapshots: false + timeout: 10800 + +initial_state: + diff_spec: auto + intent: '' + rigor_in: '' + surfaces_in: '' + extra_context: '' + resolved_rigor: production + resolved_surfaces: '' + bar_provenance: detected-default + resolved_diff_spec: '' + changed_files: [] + file_count: 0 + facts_note: '' + conventions: '' + ledger_matches: '' + history_notes: '' + touches_auth: false + destructive_migration: false + consumer_surface: false + linter_notes: '' + proposed_groups: [] + groups_note: '' + group_items: [] + domain_reports: [] + org_context_notes: '' + prior_art_notes: '' + aux_note: '' + downstream_notes: '' + walkthrough: '' + changes_rows: [] + findings: [] + cross_file: '' + operational_history: '' + org_context_section: '' + attention_flags: [] + placement_notes: '' + verifier_output: '' + verdict_out: {} + final_report: '' + gate_feedback: '' + gate_attempts: 0 + +start: parse + +nodes: + parse: + id: parse + type: llm + description: Extract the review request's facts from the spawn prompt. Extraction only — no judging. + instructions: | + You extract the facts of a code-review request. EXTRACTION ONLY. + + - `diff_spec`: the git diff basis the caller named — a range ("main...HEAD"), + a ref/SHA, "staged", "worktree" (uncommitted vs HEAD), or "auto" when the + caller did not name one (auto = staged, falling back to unstaged, falling + back to HEAD~1 — the classic get_diff semantics; a caller phrase like + "run get_diff --base X" means diff_spec = "X"). + - `intent`: the PR description / ticket / task intent text, condensed but + specific. Empty string when the caller gave none. + - `rigor_in`: ONLY when the caller explicitly set a rigor (poc|prototype|production), + e.g. via a "Quality bar:" line; else empty string. + - `surfaces_in`: ONLY when the caller explicitly declared surfaces; else empty. + - `extra_context`: any remaining reviewer-relevant caller context, condensed. + prompt: | + Extract the review-request facts from this request: + + {{initial_prompt}} + tools: [] + output_schema: + type: object + properties: + diff_spec: + type: string + intent: + type: string + rigor_in: + type: string + surfaces_in: + type: string + extra_context: + type: string + required: [diff_spec, intent, rigor_in, surfaces_in, extra_context] + max_iterations: 3 + max_attempts: 2 + next: facts + + facts: + id: facts + type: script + description: Deterministic facts — diff resolution, changed files, quality-bar resolution (passed → plan frontmatter → detected), conventions pointer, review-miss ledger matches, repo-history sweep, verdict-trigger signals. Pure git/gh/filesystem; no LLM. + script: scripts/facts.py + timeout: 240 + next: linters + + linters: + id: linters + type: script + description: Domain linter pass — tflint/hadolint/actionlint on matching changed files, buf breaking when buf-configured protos changed. Only tools already on PATH; never installs; degrades silently per tool. + script: scripts/linters.py + timeout: 420 + next: group_propose + + group_propose: + id: group_propose + type: script + description: Deterministic domain grouping of changed files by surface-trigger path patterns. + script: scripts/group_domains.py + timeout: 30 + next: refine_groups + + refine_groups: + id: refine_groups + type: llm + description: Judgment refinement of the proposed domain groups — may rebalance, never drop or invent files (the cover gate enforces it). + instructions: | + You refine a deterministic domain grouping of changed files. You may: + - move a file whose CONTENT belongs to a different domain than its path suggests; + - split a group larger than ~8 files into coherent subgroups (name them + "-1", "-2" or by subsystem); + - merge groups that are really one change (a migration and its models); + - rename a domain for clarity. + + You MUST include every changed file in exactly one group — no drops, no + inventions, no duplicates. A downstream gate validates the cover and falls + back to the deterministic proposal if you violate it. + prompt: | + Proposed groups (deterministic): + {{proposed_groups}} + + All changed files (every one must appear in exactly one group): + {{changed_files}} + + Change intent: {{intent}} + tools: [] + output_schema: + type: object + properties: + groups: + type: array + items: + type: object + properties: + domain: + type: string + files: + type: array + items: + type: string + rationale: + type: string + required: [domain, files] + required: [groups] + max_iterations: 3 + max_attempts: 2 + next: cover_gate + + cover_gate: + id: cover_gate + type: script + description: Exact-cover validation of the refined groups — every changed file in exactly one group, no unknown files. Violations fall back to the deterministic proposal (noted). Emits the map items. + script: scripts/cover_gate.py + timeout: 30 + next: [map_domains, aux_lanes, downstream_sweep] + + map_domains: + id: map_domains + type: map + over: '{{group_items}}' + as: domain_group + branch: review_domain + output_key: domain_report + collect_into: domain_reports + max_concurrency: 4 + next: synthesize + + review_domain: + id: review_domain + type: agent + description: One domain-reviewer leader per domain slice — reviews the slice as a unit, delegates per-file depth to file-reviewer, messages the other leaders. + agent: domain-reviewer + teammates: true + timeout: 7200 + prompt: | + ## TASK + Review the changes in your assigned domain slice as ONE coherent unit and + produce a DOMAIN_REVIEW_COMPLETE-terminated report per your output format. + + ## ASSIGNMENT (JSON) + {{domain_group}} + + ## MUST DO + - Fetch your slice's diff yourself: `get_diff --files ` + (diff spec: {{resolved_diff_spec}} — pass it as `--base` when it is a ref or range; + omit `--base` for staged/unstaged specs) + - Apply your domain charter: per-file review depth per your sizing rules, PLUS the + domain-level pass (guard symmetry across the slice, literal/enum drift, + schema↔usage mismatch, slice-level blast radius / test adequacy / removed guards) + - Report the mandatory checks EVEN WHEN CLEAN + - Message the other domain leaders when your slice changes something their domain consumes + {{gate_feedback}} + + ## MUST NOT DO + - No file modification; no findings omitted; never rewrite severities + + ## CONTEXT + Project: {{project_dir}} + Rigor: {{resolved_rigor}} (provenance: {{bar_provenance}}) + Surfaces: {{resolved_surfaces}} + Intent: {{intent}} + Conventions: {{conventions}} + Review-miss patterns (run as FIRST-CLASS checks; cite MISS-NNN in the finding title when one fires): + {{ledger_matches}} + Repo-history signals: + {{history_notes}} + Linter output (extract the lines for your files): + {{linter_notes}} + {{extra_context}} + state_updates: + domain_report: '{{output}}' + next: completeness_gate + + completeness_gate: + id: completeness_gate + type: script + description: Deterministic report-contract gate — required sections present (even when clean) and the completion sentinel. Rejects back to the leader exactly once with the missing sections, then keeps the report banner-flagged. Never sinks the map. + script: scripts/completeness_gate.py + timeout: 30 + + aux_lanes: + id: aux_lanes + type: llm + description: Conditional context lanes — org-context and operational-history/prior-art agents, spawned only when configured AND triggered. Additive context; never blocks. + instructions: | + You run the review's conditional context lanes by spawning agents (you have the + agent__spawn / agent__collect tools). Evaluate each lane's trigger against the + change facts; spawn the applicable ones IN ONE BURST, collect, and distill. + + **Org-context lane** — only if `{{org_context_agent}}` is a non-empty agent name AND + the diff touches surface where a recorded decision plausibly exists (public API + shapes, money/entitlement logic, auth flows, data retention/deletion, domain state + machines and lifecycle semantics) OR the diff's scope visibly mismatches the + ticket/intent it cites. Spawn it with ONE question: "Any recorded decisions, PRDs, + or incident-driven constraints about ? Return quoted decisions with + dates/sources." (For a ticket mismatch: "which recorded ticket/PRD does THIS change + actually implement?") Distill the answer into `org_context_notes` — quoted decisions + with dates, and whether the diff appears to honor them. + + **Operational-history lane** — only if `{{prior_art_agent}}` is a non-empty agent + name AND the diff touches operationally relevant surface (workers/queues, + migrations, infra, auth, retry/timeout logic, external calls, feature flags). + Spawn it in REVIEW MODE: "REVIEW MODE: incident prior art for . Return incidents, postmortems, alert history, and prior-PR + lessons tied to these paths, with dates and links — especially any GUARD present + in the past that this diff deletes or weakens." Distill into `prior_art_notes`; + quote anything tying a DELETED guard to a past incident verbatim (the verdict + treats reintroduced failure modes as blocking). + + A lane whose trigger does not match, whose agent name is empty, or whose agent + fails: skip it and say why in `aux_note` (one line each). Never invent lane + content; empty results are valid results. + prompt: | + Change facts: + - files: {{changed_files}} + - intent: {{intent}} + - auth surface: {{touches_auth}}; migrations: {{destructive_migration}}; consumer surface: {{consumer_surface}} + - resolved surfaces: {{resolved_surfaces}} + - extra: {{extra_context}} + + Configured lane agents: org-context='{{org_context_agent}}', prior-art='{{prior_art_agent}}'. + Project: {{project_dir}} + output_schema: + type: object + properties: + org_context_notes: + type: string + prior_art_notes: + type: string + aux_note: + type: string + required: [org_context_notes, prior_art_notes, aux_note] + max_iterations: 10 + max_attempts: 2 + timeout: 3600 + next: synthesize + + downstream_sweep: + id: downstream_sweep + type: script + description: Downstream-consumers sweep — deterministic contract-token extraction from the diff, grep of sibling checkouts, bounded org code search. Runs only when consumer-facing surface changed; degrades silently. + script: scripts/downstream_sweep.py + timeout: 240 + state_updates: + downstream_notes: '{{downstream_notes}}' + next: synthesize + + synthesize: + id: synthesize + type: llm + description: The judgment core — dedup, cross-file/cross-domain reasoning, prose. Emits STRUCTURED findings for the deterministic verdict; has NO verdict authority and cannot drop findings. + instructions: | + You synthesize domain review reports into the structured inputs of a code-review + report. You have NO verdict authority: the verdict, rigor folding, and severity + math happen downstream in a script, and finding verification happens in a separate + fresh-context agent. Your job is judgment, not grading. + + Rules (non-negotiable): + - **Include ALL findings** from every domain report — never summarize one away. + Preserve each severity tag and any `[convention]`/`[correctness]` title marker + VERBATIM (marker goes in the `marker` field: "", "convention", or "correctness"). + - **Dedup**: an identical finding reported twice (e.g. a surface skill and an + aspect skill like transactional-integrity) → keep the aspect skill's copy, drop + the duplicate. Dedup is the ONLY permitted removal. + - **Lane-derived findings you MUST add** when present in the lane notes: + - a diff that contradicts a recorded org decision (org_context_notes) → a + 🔴 CRITICAL finding quoting the decision; + - a downstream consumer hit on a changed/removed contract element + (downstream_notes) → a 🔴 CRITICAL finding naming the consuming repo + file:line; + - repo-level linter residue that maps to no single file → a finding under the + owning surface (severity per the linter's own level; buf breaking = 🔴). + - **Every finding needs**: id (f1, f2, …), severity (icon + word verbatim, e.g. + "🔴 CRITICAL"), marker, file, lines, title, and `block` = the finding's full + markdown block exactly as it will appear in the report (#### title + Severity / + Lines / Description / Suggestion bullets). + - **attention_flags** (ADDITIVE-ONLY human-attention triggers; scripts add the + deterministic ones — migrations, auth — separately): money movement or + billing-amount computation changes; secrets/cryptography/key handling; a deleted + guard the operational-history lane tied to a past incident; a reintroduced + incident failure mode; a contradicted recorded decision. One short line each. + - **The placement question**: when the diff adds tolerance for a state its own + comments/errors call impossible ("should never happen", "defensive"), name the + PRODUCER of that state at path:line and put the question in `placement_notes` — + it is design intent, never auto-resolved. + - Prose sections: `walkthrough` (2-3 sentences on the change as a whole), + `changes_rows` (one {file, desc} per changed file, brief), `cross_file` + (cross-domain concerns from the leaders' reports + teammate alerts, or "None"), + `operational_history` (from prior_art_notes; "" when the lane didn't run), + `org_context_section` (from org_context_notes; "" when the lane didn't run). + prompt: | + Domain reports ({{file_count}} changed files, grouped into the domain slices below): + {{domain_reports}} + + Lane notes: + - org context: {{org_context_notes}} + - operational history / prior art: {{prior_art_notes}} + - downstream consumers: {{downstream_notes}} + - lane skips: {{aux_note}} + - linter summary: {{linter_notes}} + - facts notes: {{facts_note}} {{groups_note}} + + Intent: {{intent}} + Rigor: {{resolved_rigor}}; surfaces: {{resolved_surfaces}} + output_schema: + type: object + properties: + walkthrough: + type: string + changes_rows: + type: array + items: + type: object + properties: + file: + type: string + desc: + type: string + required: [file, desc] + findings: + type: array + items: + type: object + properties: + id: + type: string + severity: + type: string + marker: + type: string + file: + type: string + lines: + type: string + title: + type: string + block: + type: string + required: [id, severity, marker, file, lines, title, block] + cross_file: + type: string + operational_history: + type: string + org_context_section: + type: string + attention_flags: + type: array + items: + type: string + placement_notes: + type: string + required: [walkthrough, changes_rows, findings, cross_file, operational_history, org_context_section, attention_flags, placement_notes] + max_iterations: 6 + max_attempts: 2 + next: verify + + verify: + id: verify + type: agent + description: Independent fresh-context verification of every 🔴/🟡 finding — structurally on the path to the report; cannot be skipped. + agent: finding-verifier + timeout: 3600 + prompt: | + Verify these code-review findings against the working tree. + + Project: {{project_dir}} + + Findings (JSON; verify every 🔴 and 🟡 entry — id, severity, path, lines, claim + are the fields; `title` + `block` together are the claim): + {{findings}} + state_updates: + verifier_output: '{{output}}' + next: verdict + + verdict: + id: verdict + type: script + description: Deterministic verdict math — applies verifier drops (FALSE, tallied), attaches evidence, folds by rigor, computes MERGE-READY/NEEDS-HUMAN from severity counts + always-human triggers. The model never grades its own homework. + script: scripts/verdict.py + timeout: 60 + next: render + + render: + id: render + type: script + description: Deterministic report assembly in the standard format from the structured findings and prose sections. + script: scripts/render.py + timeout: 30 + next: done + + done: + id: done + type: end + output: | + {{final_report}} diff --git a/assets/agents/code-reviewer/scripts/completeness_gate.py b/assets/agents/code-reviewer/scripts/completeness_gate.py new file mode 100644 index 0000000..599b5c2 --- /dev/null +++ b/assets/agents/code-reviewer/scripts/completeness_gate.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +"""Report-contract gate for each domain-reviewer's output. + +"Silence reads as not done": the mandatory sections must be PRESENT even when +clean. A report missing required sections is rejected back to the leader +exactly once with the missing list; a second failure keeps the report, +banner-flagged, so the synthesis (and the human) can see the gap — a +malformed report never sinks the whole review and never silently passes as +complete. +""" + +import json +import os +import re + +REQUIRED = [ + ("## Domain:", "the `## Domain:` header"), + ("### Findings", "the `### Findings` section"), + ("### Mandatory checks", "the `### Mandatory checks` section (report blast radius / test adequacy / guard symmetry even when clean)"), + ("DOMAIN_REVIEW_COMPLETE", "the `DOMAIN_REVIEW_COMPLETE` sentinel"), +] +MAX_RETRIES = 1 + + +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() + report = state.get("domain_report") + report = report if isinstance(report, str) else "" + attempts = state.get("gate_attempts") + if not isinstance(attempts, int): + attempts = 0 + + missing = [desc for marker, desc in REQUIRED if marker not in report] + if not missing: + print(json.dumps({})) + return + + if attempts < MAX_RETRIES: + print( + json.dumps( + { + "_next": "review_domain", + "gate_attempts": attempts + 1, + "gate_feedback": ( + "- COMPLETENESS GATE REJECTED your previous report — missing: " + + "; ".join(missing) + + ". Re-emit the FULL report with every required section " + "(mandatory checks are reported even when clean) and end with " + "DOMAIN_REVIEW_COMPLETE." + ), + } + ) + ) + return + + group = state.get("domain_group") + domain = group.get("domain", "?") if isinstance(group, dict) else "?" + banner = ( + f"> ⚠️ COMPLETENESS GATE: this domain report ({domain}) is missing required " + f"sections after one retry: {'; '.join(missing)}. Treat the missing checks as " + "NOT DONE, not as clean.\n\n" + ) + print(json.dumps({"domain_report": banner + report})) + + +try: + main() +except Exception as e: # noqa: BLE001 — a gate crash must never sink the map + print(json.dumps({"domain_report": f"> ⚠️ COMPLETENESS GATE ERROR: {e}\n\n(report unavailable)"})) diff --git a/assets/agents/code-reviewer/scripts/cover_gate.py b/assets/agents/code-reviewer/scripts/cover_gate.py new file mode 100644 index 0000000..1ebfc23 --- /dev/null +++ b/assets/agents/code-reviewer/scripts/cover_gate.py @@ -0,0 +1,76 @@ +#!/usr/bin/env python3 +"""Exact-cover gate for the refined domain groups. + +The LLM refinement may rebalance the deterministic proposal, but it may not +lose, invent, or duplicate files. This gate verifies the refined groups are an +EXACT COVER of the changed-file set; any violation falls back to the +deterministic proposal with a note. The LLM can shape the grouping, never +shrink it. Emits `group_items` — the map node's item list. + +Crash-safe: this node has a `next:` edge, so an unguarded crash would +tolerant-fail ONWARD with an empty `group_items` — an empty review that could +masquerade as MERGE-READY. On any internal error it therefore emits one +catch-all group so the review still runs (and verdict.py's pipeline tripwire +catches the empty-report case independently). +""" + +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", "{}")) + + +def main(): + state = load_state() + changed = state.get("changed_files") or [] + proposed = state.get("proposed_groups") or [] + refined = state.get("groups") or [] + + note = "" + chosen = None + if isinstance(refined, list) and refined: + seen = [] + valid = True + for g in refined: + if not isinstance(g, dict) or not isinstance(g.get("files"), list) or not g.get("domain"): + valid = False + note = "refined groups malformed — fell back to the deterministic grouping" + break + seen.extend(g["files"]) + if valid: + if sorted(seen) != sorted(changed): + missing = sorted(set(changed) - set(seen)) + extra = sorted(set(seen) - set(changed)) + dupes = sorted({f for f in seen if seen.count(f) > 1}) + note = ( + "refined groups failed the exact-cover check " + f"(missing={missing[:5]} extra={extra[:5]} duplicated={dupes[:5]}) — " + "fell back to the deterministic grouping" + ) + else: + chosen = [{"domain": g["domain"], "files": g["files"]} for g in refined] + if chosen is None: + chosen = [{"domain": g["domain"], "files": g["files"]} for g in proposed if isinstance(g, dict)] + + print(json.dumps({"group_items": chosen, "groups_note": note})) + + +try: + main() +except Exception as e: # noqa: BLE001 — a crash must never tolerant-fail into an empty review + try: + files = load_state().get("changed_files") or [] + except Exception: # noqa: BLE001 + files = [] + print( + json.dumps( + { + "group_items": [{"domain": "all-changes", "files": files}], + "groups_note": f"cover gate error: {e} — fell back to a single catch-all group", + } + ) + ) diff --git a/assets/agents/code-reviewer/scripts/downstream_sweep.py b/assets/agents/code-reviewer/scripts/downstream_sweep.py new file mode 100644 index 0000000..85b6c60 --- /dev/null +++ b/assets/agents/code-reviewer/scripts/downstream_sweep.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +"""Downstream-consumers sweep for the code-reviewer graph. + +Runs only when consumer-facing contract surface changed. Extracts the most +specific changed contract tokens from the diff deterministically (route path +literals, proto message/service/rpc names, exported symbols), greps sibling +checkouts under `consumers_root`, and runs a bounded org code search via gh. +Hits are reported for the synthesis to raise as 🔴 findings naming the +consuming repo + file:line. Degrades silently when unconfigured/offline; +public-library caveats are noted, not guessed at. +""" + +import json +import os +import re +import shutil +import subprocess + +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() +proj = os.path.abspath(state.get("project_dir") or ".") +spec = state.get("resolved_diff_spec") or "worktree" +consumers_root = os.path.expanduser((state.get("consumers_root") or "").strip()) +github_org = (state.get("github_org") or "").strip() + +if not state.get("consumer_surface"): + print(json.dumps({"downstream_notes": "(skipped — no consumer-facing contract surface in the diff)"})) + raise SystemExit(0) + + +def diff_text(): + args = {"staged": ["--cached"], "worktree": ["HEAD"], "HEAD~1": ["HEAD~1"]}.get(spec, [spec]) + r = subprocess.run(["git", "-C", proj, "diff", *args], capture_output=True, text=True, timeout=90) + return r.stdout if r.returncode == 0 else "" + + +try: + changed = [ + l[1:] for l in diff_text().splitlines() + if (l.startswith("+") or l.startswith("-")) and not l.startswith(("+++", "---")) + ] + text = "\n".join(changed) + tokens = set() + tokens.update(re.findall(r'["\'](/[a-z0-9][a-z0-9/_{}.:-]{5,})["\']', text)) # route paths + tokens.update(re.findall(r"^\s*(?:message|service|rpc)\s+([A-Z][A-Za-z0-9_]{5,})", text, re.M)) # proto names + tokens.update(re.findall(r"^\s*(?:pub\s+fn|pub\s+struct|func|export\s+(?:function|const|class))\s+([A-Z][A-Za-z0-9_]{5,})", text, re.M)) + tokens = sorted(tokens, key=len, reverse=True)[:5] + + notes = [] + if not tokens: + notes.append("(no specific contract tokens extractable from the diff — sweep skipped)") + else: + notes.append(f"contract tokens probed: {tokens}") + if consumers_root and os.path.isdir(consumers_root): + self_name = os.path.basename(proj) + for tok in tokens: + r = subprocess.run( + ["grep", "-rIl", "--exclude-dir=.git", "--exclude-dir=node_modules", + "--exclude-dir=target", "--exclude-dir=vendor", tok, consumers_root], + capture_output=True, text=True, timeout=60, + ) + hits = [ + h for h in r.stdout.splitlines() + if h.strip() and f"/{self_name}/" not in h and not h.startswith(proj) + ][:4] + for h in hits: + notes.append(f"CONSUMER HIT (sibling checkout): {h} references '{tok}' — " + "a changed/removed contract element with a live consumer is a 🔴 finding") + elif consumers_root: + notes.append(f"(consumers_root '{consumers_root}' not found — local probe skipped)") + else: + notes.append("(consumers_root unset — local probe skipped)") + + if github_org and shutil.which("gh"): + env = dict(os.environ) + env.pop("CLICOLOR_FORCE", None) + env.pop("FORCE_COLOR", None) + env["GH_NO_UPDATE_NOTIFIER"] = "1" + for tok in tokens: + r = subprocess.run( + ["gh", "api", "search/code", "-f", f'q=org:{github_org} "{tok}"', + "--jq", r'.items[:4][] | "\(.repository.full_name): \(.path)"'], + capture_output=True, text=True, timeout=30, env=env, + ) + if r.returncode == 0 and r.stdout.strip(): + for line in r.stdout.strip().splitlines(): + notes.append(f"CONSUMER HIT (org search): {line} references '{tok}'") + elif github_org: + notes.append("(gh unavailable — org code search skipped)") + else: + notes.append("(github_org unset — org code search skipped)") + notes.append("Public-library caveat: consumers of a PUBLIC library are unenumerable — " + "for exported-API changes the check degrades to semver/changelog discipline.") + print(json.dumps({"downstream_notes": "\n".join(notes)})) +except Exception as e: # noqa: BLE001 + print(json.dumps({"downstream_notes": f"(sweep degraded: {e})"})) diff --git a/assets/agents/code-reviewer/scripts/facts.py b/assets/agents/code-reviewer/scripts/facts.py new file mode 100644 index 0000000..f52d2ad --- /dev/null +++ b/assets/agents/code-reviewer/scripts/facts.py @@ -0,0 +1,281 @@ +#!/usr/bin/env python3 +"""Deterministic review facts for the code-reviewer graph. + +Resolves the diff, the changed-file list, the quality bar (caller-passed → +plan frontmatter → detected default), the conventions pointer, review-miss +ledger matches, the repo-history sweep (fix/revert clusters, bounded gh +PR-comment pull), and the deterministic verdict-trigger signals. Pure +git/gh/filesystem — no LLM. Failures degrade into `facts_note`; they never +sink the review. +""" + +import glob +import json +import os +import re +import subprocess + +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() +proj = state.get("project_dir") or "." +spec = (state.get("diff_spec") or "auto").strip() +notes = [] +out = {} + + +def git(*args, timeout=60): + r = subprocess.run(["git", "-C", proj, *args], capture_output=True, text=True, timeout=timeout) + if r.returncode != 0: + raise RuntimeError((r.stderr or r.stdout).strip()[:300]) + return r.stdout + + +def scrubbed_env(): + env = dict(os.environ) + env.pop("CLICOLOR_FORCE", None) + env.pop("FORCE_COLOR", None) + env["GH_NO_UPDATE_NOTIFIER"] = "1" + env["NO_COLOR"] = "1" + return env + + +# --- diff resolution (get_diff semantics) ----------------------------------- +resolved = spec +files = [] +try: + if spec in ("auto", ""): + names = git("diff", "--cached", "--numstat") + resolved = "staged" + if not names.strip(): + names = git("diff", "--numstat") + resolved = "worktree" + if not names.strip(): + names = git("diff", "HEAD~1", "--numstat") + resolved = "HEAD~1" + elif spec == "staged": + names = git("diff", "--cached", "--numstat") + elif spec == "worktree": + names = git("diff", "HEAD", "--numstat") + else: + names = git("diff", spec, "--numstat") + for line in names.splitlines(): + parts = line.split("\t") + if len(parts) == 3: + added, deleted, path = parts + if added == "-" and deleted == "-": + notes.append(f"binary file skipped: {path}") + continue + files.append(path.strip()) +except Exception as e: # noqa: BLE001 + notes.append(f"diff resolution failed for spec {spec!r}: {e}") + +out["resolved_diff_spec"] = resolved +out["changed_files"] = files[:300] +out["file_count"] = len(files) + + +def diff_text(): + try: + if resolved == "staged": + return git("diff", "--cached", timeout=90) + if resolved == "worktree": + return git("diff", "HEAD", timeout=90) if spec == "worktree" else git("diff", timeout=90) + if resolved == "HEAD~1": + return git("diff", "HEAD~1", timeout=90) + return git("diff", resolved, timeout=90) + except Exception: # noqa: BLE001 + return "" + + +dtext = diff_text() +added_lines = "\n".join( + l for l in dtext.splitlines() if l.startswith("+") and not l.startswith("+++") +) + +# --- quality bar resolution -------------------------------------------------- +rigor_in = (state.get("rigor_in") or state.get("rigor") or "").strip() +surfaces_in = (state.get("surfaces_in") or state.get("surfaces") or "").strip() +resolved_rigor, resolved_surfaces, provenance = "production", "", "detected-default" +if rigor_in or surfaces_in: + resolved_rigor = rigor_in or "production" + resolved_surfaces = surfaces_in + provenance = "passed" +else: + for plan in sorted(glob.glob(os.path.join(proj, "plans", "PLAN-*.md"))): + try: + with open(plan) as f: + head = f.read(4000) + m = re.match(r"\A---\n(.*?)\n---", head, re.S) + if m and re.search(r"^status:\s*active\s*$", m.group(1), re.M): + rig = re.search(r"^rigor:\s*(\S+)", m.group(1), re.M) + sur = re.search(r"^surfaces:\s*(.+)$", m.group(1), re.M) + if rig: + resolved_rigor = rig.group(1).strip() + if sur: + resolved_surfaces = sur.group(1).strip("[] \n") + provenance = "plan" + break + except OSError: + continue + +SURFACE_PATTERNS = [ + ("db-migration", r"migrations?/|\.sql$|schema\.(rb|prisma|sql)"), + ("iac", r"\.tf$|helm|charts?/|Dockerfile|docker-compose|k8s/|manifests?/"), + ("ci-cd", r"\.github/workflows/|\.gitlab-ci|Jenkinsfile|\.circleci/"), + ("worker", r"workers?/|jobs?/|consumers?/|cron|celery|sidekiq|river"), + ("rest-api", r"routes?|handlers?|controllers?|endpoints?|\bapi\b|\.proto$|openapi|swagger|graphql|resolvers?"), + ("cli", r"\bcmd/|\bcli\b"), + ("library", r"go\.mod$|package\.json$|Cargo\.toml$|pyproject\.toml$|setup\.py$|__init__\.py$"), +] +if provenance == "detected-default" and files: + detected = [] + for surface, pat in SURFACE_PATTERNS: + if any(re.search(pat, f, re.I) for f in files): + detected.append(surface) + resolved_surfaces = ", ".join(detected) + +out["resolved_rigor"] = resolved_rigor +out["resolved_surfaces"] = resolved_surfaces +out["bar_provenance"] = provenance + +# --- conventions pointer ------------------------------------------------------ +conv = [] +for pat in ("CLAUDE.md", "COYOTE.md", "CONTRIBUTING.md", ".golangci*", ".eslintrc*", + "rustfmt.toml", ".rubocop.yml", "ruff.toml", ".editorconfig", "Makefile"): + conv.extend(sorted(glob.glob(os.path.join(proj, pat)))[:2]) +out["conventions"] = ", ".join(os.path.relpath(p, proj) for p in conv) or "none found" + +# --- review-miss ledger ------------------------------------------------------- +ledger_text = "" +for cand in ( + os.path.join(os.path.expanduser("~"), ".config", "coyote", "share", "review-misses.md"), + os.path.join(os.path.expanduser("~"), ".config", "coyote", "review-misses.md"), +): + if os.path.isfile(cand): + try: + with open(cand) as f: + ledger_text = f.read() + except OSError: + pass + break + +matches = [] +if ledger_text: + try: + repo_name = os.path.basename(git("rev-parse", "--show-toplevel").strip()) + except Exception: # noqa: BLE001 + repo_name = os.path.basename(os.path.abspath(proj)) + keywords = {repo_name.lower()} + keywords.update(s.strip().lower() for s in resolved_surfaces.split(",") if s.strip()) + for f in files: + keywords.update(p.lower() for p in re.split(r"[/_.-]", f) if len(p) >= 4) + entries = re.split(r"(?=^## MISS-)", ledger_text, flags=re.M) + for entry in entries: + if not entry.startswith("## MISS-"): + continue + body = entry.lower() + if any(k in body for k in keywords): + lines = entry.strip().splitlines() + matches.append("\n".join(lines[:8])) + if len(matches) >= 8: + break +out["ledger_matches"] = "\n\n".join(matches) if matches else "(no matching ledger entries)" + +# --- repo-history sweep ------------------------------------------------------- +history = [] +fragile = [] +try: + if files: + log = git("log", "--oneline", "-15", "--", *files[:100]) + history.append(log.strip()[:2000]) + per_file = {} + for f in files[:50]: + try: + fl = git("log", "--oneline", "-10", "--", f) + except Exception: # noqa: BLE001 + continue + hits = [l for l in fl.splitlines() if re.search(r"\b(fix|revert|hotfix)\b", l, re.I)] + if len(hits) >= 2: + fragile.append(f) + per_file[f] = hits[:2] + for f, hits in per_file.items(): + history.append(f"FRAGILE: {f} — {len(hits)}+ fix/revert commits recently — raise scrutiny") +except Exception as e: # noqa: BLE001 + notes.append(f"history sweep failed: {e}") + +# gh one-level-deeper: PR review comments for fragile files (bounded, silent skip) +try: + if fragile: + remote = git("remote", "get-url", "origin").strip() + m = re.search(r"github\.com[:/]([^/]+/[^/.]+)", remote) + if m: + repo_slug = m.group(1) + env = scrubbed_env() + shas = [] + for f in fragile[:2]: + lg = git("log", "--format=%H", "-2", "--", f) + shas.extend(lg.split()) + pr_nums = [] + for sha in shas[:3]: + r = subprocess.run( + ["gh", "api", f"repos/{repo_slug}/commits/{sha}/pulls", "--jq", ".[].number"], + capture_output=True, text=True, timeout=30, env=env, + ) + if r.returncode == 0: + pr_nums.extend(n for n in r.stdout.split() if n.isdigit()) + seen = [] + for n in pr_nums: + if n not in seen: + seen.append(n) + comments = [] + for n in seen[:3]: + r = subprocess.run( + ["gh", "api", f"repos/{repo_slug}/pulls/{n}/comments", + "--jq", r'.[] | "\(.path): \(.body)"'], + capture_output=True, text=True, timeout=30, env=env, + ) + if r.returncode == 0: + for line in r.stdout.splitlines(): + path = line.split(":", 1)[0] + if any(path in f or f in path for f in files): + comments.append(line[:200]) + if len(comments) >= 6: + break + if comments: + history.append("Prior PR review comments on fragile paths:\n" + "\n".join(comments[:6])) +except Exception: # noqa: BLE001 + pass # offline / not GitHub / no gh — silent by design + +out["history_notes"] = "\n".join(h for h in history if h) or "(no notable history)" + +# --- deterministic verdict-trigger signals ----------------------------------- +def any_path(pattern): + return any(re.search(pattern, f, re.I) for f in files) + + +migration_files = [f for f in files if re.search(r"migrations?/|\.sql$", f, re.I)] +mig_added = "\n".join( + l for l in dtext.splitlines() + if l.startswith("+") and not l.startswith("+++") +) if migration_files else "" +out["destructive_migration"] = bool( + migration_files + and re.search(r"(?i)drop\s+(table|column)|rename\s+(table|column|to)|alter\s+\S+\s+drop", mig_added) +) +out["touches_auth"] = bool( + any_path(r"auth(?!or)|login|logout|token|session|permission|rbac|\bacl\b|credential") + or re.search(r"auth(?!or)|password|jwt|session|credential", added_lines, re.I) +) +out["consumer_surface"] = any_path( + r"routes?|handlers?|\bapi\b|endpoints?|\bcmd/|\bcli\b|\.proto$|openapi|swagger|controllers?|graphql" +) + +if notes: + out["facts_note"] = "; ".join(notes)[:800] +print(json.dumps(out)) diff --git a/assets/agents/code-reviewer/scripts/group_domains.py b/assets/agents/code-reviewer/scripts/group_domains.py new file mode 100644 index 0000000..7be22cf --- /dev/null +++ b/assets/agents/code-reviewer/scripts/group_domains.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +"""Deterministic domain grouping of changed files for the code-reviewer graph. + +Each file lands in the FIRST matching domain (patterns ordered most-specific +first); unmatched files land in `general`. When the grouping is very wide, +single-file groups merge into `general` so the fan-out stays sane. The LLM +refinement node may rebalance this proposal; the cover gate guarantees no +file is lost either way. Crash-safe: an error emits one catch-all proposal +rather than tolerant-failing into an empty grouping. +""" + +import json +import os +import re + +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", "{}")) + + +DOMAINS = [ + ("db", r"migrations?/|\.sql$|schema\.(rb|prisma|sql)|models?/"), + ("iac", r"\.tf$|helm|charts?/|(^|/)Dockerfile|docker-compose|k8s/|manifests?/"), + ("ci-cd", r"\.github/workflows/|\.gitlab-ci|Jenkinsfile|\.circleci/"), + ("workers", r"workers?/|jobs?/|consumers?/|cron|tasks?/"), + ("api", r"routes?|handlers?|controllers?|endpoints?|\bapi\b|\.proto$|openapi|swagger|graphql|resolvers?"), + ("cli", r"\bcmd/|\bcli\b"), + ("deps", r"go\.(mod|sum)$|package(-lock)?\.json$|yarn\.lock$|Cargo\.(toml|lock)$|requirements[^/]*\.txt$|pyproject\.toml$|Gemfile|pom\.xml$|composer\.json$"), + ("tests", r"(^|/)tests?/|_test\.|\.test\.|spec\."), + ("docs", r"\.(md|rst|adoc|txt)$"), +] + + +def main(): + state = load_state() + files = state.get("changed_files") or [] + + groups = {} + for f in files: + domain = "general" + for name, pat in DOMAINS: + if re.search(pat, f, re.I): + domain = name + break + groups.setdefault(domain, []).append(f) + + # Wide fan-out: merge single-file groups into general. + if len(groups) > 6: + for name in list(groups): + if name != "general" and len(groups[name]) == 1: + groups.setdefault("general", []).extend(groups.pop(name)) + + proposed = [{"domain": k, "files": v} for k, v in sorted(groups.items())] + print(json.dumps({"proposed_groups": proposed})) + + +try: + main() +except Exception as e: # noqa: BLE001 — never tolerant-fail into an empty grouping + try: + files = load_state().get("changed_files") or [] + except Exception: # noqa: BLE001 + files = [] + print(json.dumps({"proposed_groups": [{"domain": "all-changes", "files": files}]})) diff --git a/assets/agents/code-reviewer/scripts/linters.py b/assets/agents/code-reviewer/scripts/linters.py new file mode 100644 index 0000000..f497e7b --- /dev/null +++ b/assets/agents/code-reviewer/scripts/linters.py @@ -0,0 +1,87 @@ +#!/usr/bin/env python3 +"""Domain linter pass for the code-reviewer graph. + +Runs whichever mechanized checkers exist on PATH against the matching changed +files: tflint (Terraform), hadolint (Dockerfiles), actionlint (workflow +files), and `buf breaking` when buf-configured protos changed. Read-only, +never installs anything, and each tool degrades silently to a skip note. +Output is routed per-file where possible so the domain leaders can extract +their slice's lines. +""" + +import json +import os +import re +import shutil +import subprocess + +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() +proj = state.get("project_dir") or "." +files = state.get("changed_files") or [] +spec = state.get("resolved_diff_spec") or "worktree" +notes = [] + + +def run(cmd, timeout=60): + r = subprocess.run(cmd, capture_output=True, text=True, timeout=timeout, cwd=proj) + return (r.stdout + r.stderr).strip() + + +def capped(text, n=40): + lines = text.splitlines() + return "\n".join(lines[:n]) + (f"\n… ({len(lines) - n} more lines)" if len(lines) > n else "") + + +try: + tf = [f for f in files if f.endswith(".tf")] + if tf and shutil.which("tflint"): + notes.append("[tflint]\n" + capped(run(["tflint", "--no-color"], 120) or "clean")) + elif tf: + notes.append("[tflint] skipped — not on PATH") + + dockers = [f for f in files if re.search(r"(^|/)Dockerfile", f)] + if dockers and shutil.which("hadolint"): + notes.append("[hadolint]\n" + capped(run(["hadolint", "--no-color", *dockers[:10]], 60) or "clean")) + elif dockers: + notes.append("[hadolint] skipped — not on PATH") + + workflows = [f for f in files if ".github/workflows/" in f] + if workflows and shutil.which("actionlint"): + notes.append("[actionlint]\n" + capped(run(["actionlint", "-no-color", *workflows[:10]], 60) or "clean")) + elif workflows: + notes.append("[actionlint] skipped — not on PATH") + + protos = [f for f in files if f.endswith(".proto")] + if protos and os.path.isfile(os.path.join(proj, "buf.yaml")) and shutil.which("buf"): + base = None + if "..." in spec: + base = spec.split("...")[0] + elif ".." in spec: + base = spec.split("..")[0] + elif spec not in ("staged", "worktree", "HEAD~1"): + base = spec + else: + try: + r = subprocess.run(["git", "-C", proj, "rev-parse", "HEAD"], + capture_output=True, text=True, timeout=15) + base = r.stdout.strip() if r.returncode == 0 else None + except Exception: # noqa: BLE001 + base = None + if base: + out = run(["buf", "breaking", "--against", f".git#ref={base}"], 120) + notes.append("[buf breaking] (wire-compat breaks are 🔴 by default)\n" + capped(out or "clean")) + else: + notes.append("[buf breaking] skipped — no base ref resolvable") + elif protos: + notes.append("[buf breaking] skipped — no buf.yaml or buf not on PATH") +except Exception as e: # noqa: BLE001 + notes.append(f"[linters] pass degraded: {e}") + +print(json.dumps({"linter_notes": "\n\n".join(notes) if notes else "(no applicable linters)"})) diff --git a/assets/agents/code-reviewer/scripts/render.py b/assets/agents/code-reviewer/scripts/render.py new file mode 100644 index 0000000..cf0ac54 --- /dev/null +++ b/assets/agents/code-reviewer/scripts/render.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +"""Deterministic report assembly for the code-reviewer graph. + +Builds the standard report format from the structured findings and prose +sections. Pure templating — no judgment, no re-summarization, no finding +ever added or removed here. +""" + +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", "{}")) + + +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" + + if not changed: + print(json.dumps({"final_report": "# Code Review Summary\n\nNo changes to review."})) + return + + verdict = v.get("verdict", "NEEDS-HUMAN") + lines = [ + "# Code Review Summary", + "", + f"**Verdict: {verdict}** — {v.get('reason', '')}", + "", + "## Walkthrough", + state.get("walkthrough") or "(no walkthrough provided)", + "", + "## Changes", + "", + "| File | Changes | Findings |", + "|------|---------|----------|", + ] + + desc_by_file = {} + for row in state.get("changes_rows") or []: + if isinstance(row, dict) and row.get("file"): + desc_by_file[row["file"]] = row.get("desc", "") + blocking = [f for f in findings if f.get("section") == "blocking"] + deferred = [f for f in findings if f.get("section") == "deferred"] + for f in changed: + per = [x for x in blocking if x.get("file") == f] + tally = " ".join( + f"{icon} {n}" for icon, n in + ((i, sum(1 for x in per if x.get("icon") == i)) for i in ("🔴", "🟡", "🟢", "💡")) + if n + ) or "—" + lines.append(f"| `{f}` | {desc_by_file.get(f, '')} | {tally} |") + + lines += ["", "## Detailed Findings", ""] + by_file = {} + for x in blocking: + by_file.setdefault(x.get("file", "(unattributed)"), []).append(x) + if by_file: + for f in sorted(by_file): + lines += [f"### `{f}`", ""] + for x in by_file[f]: + lines += [x.get("block", ""), ""] + else: + lines += ["No blocking findings.", ""] + + lines += ["## Cross-File Concerns", state.get("cross_file") or "None", ""] + if (state.get("operational_history") or "").strip(): + lines += ["## Operational history", state["operational_history"], ""] + if (state.get("org_context_section") or "").strip(): + lines += ["## Org context", state["org_context_section"], ""] + if deferred and rigor != "production": + lines += ["## Deferred by quality bar", ""] + for x in deferred: + lines += [x.get("block", ""), ""] + attention = v.get("attention") or [] + if verdict == "NEEDS-HUMAN" and attention: + lines += ["## Human attention required", ""] + lines += [f"- {a}" for a in attention] + lines += [""] + + miss_ids = sorted({ + m for x in findings for m in + __import__("re").findall(r"MISS-\d+", (x.get("title") or "") + (x.get("block") or "")) + }) + lines += [ + "---", + 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"*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"; review-miss patterns applied: {', '.join(miss_ids) if miss_ids else 'none relevant'}*", + ] + print(json.dumps({"final_report": "\n".join(lines)})) + + +try: + main() +except Exception as e: # noqa: BLE001 + print(json.dumps({"final_report": f"# Code Review Summary\n\n**Verdict: NEEDS-HUMAN** — report rendering error: {e}"})) diff --git a/assets/agents/code-reviewer/scripts/verdict.py b/assets/agents/code-reviewer/scripts/verdict.py new file mode 100644 index 0000000..e992afa --- /dev/null +++ b/assets/agents/code-reviewer/scripts/verdict.py @@ -0,0 +1,168 @@ +#!/usr/bin/env python3 +"""Deterministic verdict math for the code-reviewer graph. + +The model that wrote the findings never grades its own homework: +- verifier verdicts are applied mechanically (FALSE → drop, tallied; + VERIFIED → evidence attached; UNVERIFIABLE → kept, marked); +- rigor folding is arithmetic over severity + [convention]/[correctness] + 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). +The gate never crashes into a silent verdict: any internal error emits +NEEDS-HUMAN naming the error. +""" + +import json +import os +import re + +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 sev_icon(severity): + for icon in ("🔴", "🟡", "🟢", "💡"): + if icon in (severity or ""): + return icon + return "🟢" + + +def parse_verifier(text): + """Extract {id: {verdict, evidence, note}} from finding-verifier output.""" + verdicts = {} + if not isinstance(text, str) or not text.strip(): + return verdicts + 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 + except (ValueError, TypeError): + pass + for m in re.finditer(r'\{[^{}]*"id"[^{}]*\}', text): + try: + v = json.loads(m.group(0)) + if v.get("id"): + verdicts[str(v["id"])] = v + except (ValueError, TypeError): + continue + return verdicts + + +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")) + + kept, dropped_titles = [], [] + for f in findings: + icon = sev_icon(f.get("severity")) + v = verdicts.get(str(f.get("id"))) + block = f.get("block") or f"#### {f.get('title', '(untitled)')}" + if v and icon in ("🔴", "🟡"): + if v.get("verdict") == "FALSE": + dropped_titles.append(f.get("title", f.get("id", "?"))) + continue + if v.get("verdict") == "VERIFIED" and v.get("evidence"): + block += f"\n- **Evidence** (verifier): {v['evidence']}" + elif v.get("verdict") == "UNVERIFIABLE": + block += "\n- *(unverified: " + (v.get("note") or "could not be established") + ")*" + elif icon in ("🔴", "🟡") and not v: + block += "\n- *(unverified: no verifier verdict returned for this finding)*" + kept.append({**f, "icon": icon, "block": block}) + + # --- rigor folding (arithmetic, never rewrites severities) -------------- + for f in kept: + marker = (f.get("marker") or "").lower() + icon = f["icon"] + section = "blocking" + if icon == "🔴": + section = "blocking" + elif rigor == "prototype" and ((icon == "🟡" and marker == "convention") or icon == "🟢"): + section = "deferred" + elif rigor == "poc": + if icon == "🟡" and marker == "convention": + section = "deferred" + elif icon in ("🟢", "💡") and marker == "convention": + section = "dropped-by-poc" + f["section"] = section + final = [f for f in kept if f["section"] != "dropped-by-poc"] + blocking = [f for f in final if f["section"] == "blocking"] + deferred = [f for f in final if f["section"] == "deferred"] + + counts = {i: sum(1 for f in blocking if f["icon"] == i) for i in ("🔴", "🟡", "🟢", "💡")} + yellow_correctness = sum( + 1 for f in blocking if f["icon"] == "🟡" and (f.get("marker") or "").lower() == "correctness" + ) + + # --- always-human triggers ---------------------------------------------- + attention = [] + if state.get("destructive_migration"): + attention.append("Destructive/irreversible migration in the diff (DROP/RENAME) — needs a human signature.") + if state.get("touches_auth"): + attention.append("Authentication/authorization surface changed — needs a human signature.") + for flag in state.get("attention_flags") or []: + if isinstance(flag, str) and flag.strip(): + attention.append(flag.strip()) + placement = (state.get("placement_notes") or "").strip() + if placement: + attention.append("Placement question (design intent, never auto-resolved): " + placement) + # dedup, preserve order + 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 — " + "the review did not actually run; do not trust this verdict." + ) + 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: + 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" + + print( + json.dumps( + { + "verdict_out": { + "verdict": verdict, + "reason": reason_line, + "counts": counts, + "deferred_count": len(deferred), + "dropped_count": len(dropped_titles), + "dropped_titles": dropped_titles, + "attention": attention[:5], + "findings_final": final, + } + } + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — never crash into a silent verdict + print( + json.dumps( + { + "verdict_out": { + "verdict": "NEEDS-HUMAN", + "reason": f"verdict computation error: {e} — treat as needing human review", + "counts": {}, "deferred_count": 0, "dropped_count": 0, + "dropped_titles": [], "attention": [f"verdict script error: {e}"], + "findings_final": [], + } + } + ) + ) diff --git a/assets/agents/domain-reviewer/README.md b/assets/agents/domain-reviewer/README.md new file mode 100644 index 0000000..3ca9e08 --- /dev/null +++ b/assets/agents/domain-reviewer/README.md @@ -0,0 +1,43 @@ +# Domain Reviewer + +A **domain-slice review leader**. Where `file-reviewer` reviews one file's diff +in isolation, domain-reviewer reviews a group of *related* changed files — the +DB migration plus the queries that use it, the routes plus their handlers — as +ONE coherent unit. It runs as a map branch of the `code-reviewer` graph: one +leader per domain, in parallel, with peer messaging between leaders. + +## Why it exists + +The strongest recurring review misses are **cross-file-within-domain** defects: +seed data drifting from canonical enums, a guard fixed in one of N twin paths, +a migration renaming a column the query layer still uses. Per-file reviewers +each hold one fragment of those bugs and only connect them if sibling messaging +happens to fire. A domain leader holds the whole slice in one context — the +connection is the default, not a lucky message. + +## How it works + +1. Fetches its slice's diff itself (`get_diff --files `). +2. **Sizing**: a small slice (≤3 files, <400 changed lines) is reviewed + directly with the full `code-review` skill stack; a larger one fans out one + `file-reviewer` per file (classic delegation-protocol prompts + sibling + roster) and the leader synthesizes. +3. **The domain pass** (always, its unique value): guard symmetry across the + slice, literal/constant drift between files, schema↔usage mismatch, and the + mandatory blast-radius / test-adequacy / removed-guards checks at slice + level — reported even when clean. +4. **Leader-to-leader messaging**: the code-reviewer graph runs leaders with + `teammates: true`, so the DB leader can tell the API leader "the migration + renames `status` → `state`, check your serializers" mid-review. +5. Emits a machine-checkable report (`## Domain:` / `### Findings` with a + `File` field per finding / `### Mandatory checks` / `### Cross-Domain + Concerns` / `DOMAIN_REVIEW_COMPLETE`) — the graph's completeness gate + rejects a report missing required sections exactly once, then flags it. + +## Notes + +- Read-only; severities per the standard 🔴/🟡/🟢/💡 mapping; never rewrites a + file-reviewer's severity; dedups identical findings (aspect skill's copy wins). +- Normally spawned by the `code-reviewer` graph, not directly — but a direct + spawn works: give it a domain name, a file list, a diff spec, and the quality + bar in the prompt. diff --git a/assets/agents/domain-reviewer/config.yaml b/assets/agents/domain-reviewer/config.yaml new file mode 100644 index 0000000..8ec24cd --- /dev/null +++ b/assets/agents/domain-reviewer/config.yaml @@ -0,0 +1,160 @@ +name: domain-reviewer +description: Domain-slice review leader - reviews a group of related changed files as ONE coherent unit (cross-file-within-domain checks are first-class), delegating per-file depth to file-reviewer when the slice is large. Runs as a map branch of the code-reviewer graph with teammate messaging to the other domain leaders. +version: 1.0.0 + +auto_continue: true +max_auto_continues: 20 +inject_todo_instructions: false + +skills_enabled: true +enabled_skills: + - code-review + - ai-slop-remover + - transactional-integrity + - logging-discipline + - rest-api-review + - cli-review + - library-review + - worker-review + - iac-review + - migration-review + - cicd-review + +spawnable_agents: + - file-reviewer +max_concurrent_agents: 8 +max_agent_depth: 2 + +variables: + - name: project_dir + description: Project directory containing the changes under review + default: '.' + - name: auto_confirm + description: Auto-confirm command execution + default: '1' + +global_tools: + - fs_read.sh + - fs_grep.sh + - fs_glob.sh + - fs_cat.sh + - fs_ls.sh + +instructions: | + You are a DOMAIN REVIEW LEADER. You review one domain's slice of a code change — a group of + related files (the DB migration plus the queries that use it; the routes plus their handlers) — + as ONE coherent unit. Your unique value over per-file review is the space BETWEEN the files: + cross-file-within-domain defects that no single-file reviewer can see. You either review the + slice directly or lead a team of `file-reviewer` agents through it, and you always add the + domain-level pass on top. + + Your spawn prompt IS your entire context: an ASSIGNMENT (JSON with `domain` and `files`), a diff + spec, the quality bar (rigor/surfaces + provenance), and the shared context pack (intent, + conventions, review-miss patterns, repo-history signals, linter output). + + ## Step 0: Load review skills + + Before reading any code, `skill__load` `code-review` and `ai-slop-remover`. Additionally load + `transactional-integrity` when the slice touches state-changing code (DB writes, transactions, + queue/webhook/job handlers, retries, external side effects) and `logging-discipline` when it + touches boundaries, error paths, background jobs, or state transitions. Load the surface skill(s) + for your domain per this table (load a row's skill when your files match that surface's trigger + AND the surface is in the resolved surfaces list; when provenance is `detected-default`, a + trigger match alone suffices): + + | declared surface | skill loaded | + |---|---| + | `rest-api` (and `grpc`/`graphql` aliases) | `rest-api-review` | + | `cli` | `cli-review` | + | `library` | `library-review` | + | `worker` | `worker-review` | + | `iac` | `iac-review` | + | `db-migration` | `migration-review` | + | `ci-cd` | `cicd-review` | + + ## Workflow + + 1. **Fetch your slice's diff**: `get_diff --files ` (add + `--base ` when the diff spec names one; omit it for staged/unstaged review). + 2. **Choose depth**: + - **Direct** (slice is ≤3 files AND <400 changed lines): review each file's diff yourself, + applying the loaded skill checklists exactly as a file-reviewer would. + - **Delegate** (bigger): spawn one `file-reviewer` per file IN PARALLEL, each with the full + TASK / EXPECTED OUTCOME / MUST DO / MUST NOT DO / CONTEXT structure — pass through the + context pack fields from YOUR prompt verbatim (intent, conventions, review-miss patterns, + rigor/surfaces + provenance, this file's linter output) and paste each file's diff hunks + inline (the reviewer cannot see your context). Group tiny files (<20 changed lines) into a + shared reviewer when the slice exceeds ~8 files. After spawning ALL of them, send each the + sibling roster (`SIBLING_ROSTER:` with ids + file assignments) so they can exchange + cross-cutting alerts. Then collect results — do not poll. + 3. **The domain pass** (ALWAYS yours, direct or delegated — this is why you exist). Check the + SLICE AS A WHOLE for: + - **Guard symmetry**: a guard/validation/limit changed in one file — grep the slice (and the + repo) for its siblings (same interface, same job kind, same naming family); a fix applied + to one of N twins is N-1 latent bugs. + - **Literal/constant drift**: values that must agree across the slice's files (enum literals + vs seed data, config keys vs readers, error codes vs handlers) — quote both sides when they + disagree. + - **Schema ↔ usage mismatch**: migrations vs the queries/models that use the schema; proto + definitions vs marshaling code; API shapes vs client calls within the slice. + - **Blast radius, test adequacy, and removed guards at slice level** (the mandatory checks + from `code-review`) — REPORT them under `### Mandatory checks` even when clean; silence + reads as not done. + 4. **Talk to the other domain leaders** (you run alongside them with peer messaging armed): + check your inbox before finalizing, and send a leader a 1-2 sentence alert when your slice + changes something their domain consumes (a renamed column, a changed route shape, a moved + constant). Messages move facts sideways; they never replace your report. + 5. **Synthesize the domain report** in the exact output format below. + + ## Output Format + + Structure your final response EXACTLY as: + + ``` + ## Domain: + + ### Summary + <2-3 sentences: what this slice changes as a unit> + + ### Findings + + #### + - **Severity**: 🔴 CRITICAL | 🟡 WARNING | 🟢 SUGGESTION | 💡 NITPICK + - **File**: + - **Lines**: - + - **Description**: + - **Suggestion**: + + #### + ... + + ### Mandatory checks + - Blast radius: "> + - Test adequacy: "> + - Guard symmetry: "> + + ### Cross-Domain Concerns + + + DOMAIN_REVIEW_COMPLETE + ``` + + ## Rules + + 1. **You are read-only.** Never modify files. + 2. **Every finding carries a File field** — your report covers multiple files and the synthesis + is machine-assisted; a finding without a file is unroutable. + 3. **Severity tags per the standard mapping** (🔴 correctness/security/data-loss; 🟡 logic + errors, races, missing error handling; 🟢 clarity/coupling/missing tests; 💡 style/nitpicks). + Preserve your file-reviewers' severities verbatim; dedup identical findings (keep the aspect + skill's copy). + 4. **Include ALL findings** — yours and your file-reviewers' — never summarize any away. + 5. **Mandatory checks report even when clean.** + 6. **End with DOMAIN_REVIEW_COMPLETE** on its own line. + + ## Context + - Project: {{project_dir}} + - CWD: {{__cwd__}} + + ## Available Tools + {{__tools__}} diff --git a/assets/agents/domain-reviewer/tools.sh b/assets/agents/domain-reviewer/tools.sh new file mode 100755 index 0000000..94dbecc --- /dev/null +++ b/assets/agents/domain-reviewer/tools.sh @@ -0,0 +1,63 @@ +#!/usr/bin/env bash +set -eo pipefail + +# @env LLM_OUTPUT=/dev/stdout +# @env LLM_AGENT_VAR_PROJECT_DIR=. +# @describe Domain-slice reviewer tools + +_project_dir() { + local dir="${LLM_AGENT_VAR_PROJECT_DIR:-.}" + (cd "${dir}" 2>/dev/null && pwd) || echo "${dir}" +} + +# @cmd Get the git diff for this domain's file slice. Defaults to staged changes, falls back to unstaged, then HEAD~1. Pass --base for a ref or range (e.g. "main...HEAD"). +# @option --base Optional base ref or range to diff against (e.g. "main...HEAD", "HEAD~3", a SHA) +# @option --files Comma-separated list of file paths to limit the diff to (your assignment's files) +get_diff() { + local project_dir + project_dir=$(_project_dir) + # shellcheck disable=SC2154 + local base="${argc_base:-}" + # shellcheck disable=SC2154 + local files_csv="${argc_files:-}" + + local -a path_args=() + if [[ -n "${files_csv}" ]]; then + local IFS=',' + local f + for f in ${files_csv}; do + f="$(echo "${f}" | sed -e 's/^ *//' -e 's/ *$//')" + [[ -n "${f}" ]] && path_args+=("${f}") + done + fi + + _diff() { + if ((${#path_args[@]})); then + (cd "${project_dir}" && git diff "$@" -- "${path_args[@]}" 2>&1) || true + else + (cd "${project_dir}" && git diff "$@" 2>&1) || true + fi + } + + local diff_output="" + if [[ -n "${base}" ]]; then + diff_output=$(_diff "${base}") + else + diff_output=$(_diff --cached) + [[ -z "${diff_output}" ]] && diff_output=$(_diff) + [[ -z "${diff_output}" ]] && diff_output=$(_diff HEAD~1) + fi + + if [[ -z "${diff_output}" ]]; then + echo "No changes found for the requested files in ${project_dir}." >> "$LLM_OUTPUT" + return 0 + fi + + local file_count + file_count=$(echo "${diff_output}" | grep -c '^diff --git' || true) + { + echo "Diff contains changes to ${file_count} file(s):" + echo "" + echo "${diff_output}" + } >> "$LLM_OUTPUT" +} diff --git a/assets/agents/finding-verifier/README.md b/assets/agents/finding-verifier/README.md index d9fd3bf..e86ae0f 100644 --- a/assets/agents/finding-verifier/README.md +++ b/assets/agents/finding-verifier/README.md @@ -9,7 +9,7 @@ or confirmation-bias the check. A graph can't: the structure IS the discipline. ``` parse (llm) ──▶ verify_each (map, max_concurrency 4) ──▶ done (end) - └─▶ verify_one (llm + fs_read/fs_cat/fs_grep), one branch PER finding + └─▶ verify_one (llm + fs tools) ──▶ verdict_gate (script), one CHAIN per finding ``` 1. **parse** — extracts every 🔴/🟡 finding from the spawn prompt into a structured list @@ -23,6 +23,12 @@ parse (llm) ──▶ verify_each (map, max_concurrency 4) ──▶ done (end) - `FALSE` — the code doesn't exist anywhere or provably doesn't do what the claim says. Requires PROOF, not disagreement — doubt never lands here. - `UNVERIFIABLE` — couldn't establish either way (runtime behavior, external systems). + + Each branch is a chain: `verify_one → verdict_gate`. The gate machine-validates the JSON + 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. 4. **done** — a deterministic `end` node emits the raw verdict list plus the caller contract (VERIFIED = keep + paste evidence; FALSE = drop + tally; UNVERIFIABLE = keep, diff --git a/assets/agents/finding-verifier/graph.yaml b/assets/agents/finding-verifier/graph.yaml index 5e91756..e7fcba3 100644 --- a/assets/agents/finding-verifier/graph.yaml +++ b/assets/agents/finding-verifier/graph.yaml @@ -30,6 +30,8 @@ settings: initial_state: findings_list: [] verdicts: [] + gate_feedback: '' + gate_attempts: 0 start: parse @@ -136,11 +138,18 @@ nodes: VERIFIED and FALSE, empty string for UNVERIFIABLE>", "note": ""} + + Your response is machine-validated by a deterministic gate: a malformed + response (wrong shape, unknown verdict value, missing required + evidence/note) is rejected back to you exactly once with the reason, + then recorded as UNVERIFIABLE. The `id` you echo is informative only — + the gate stamps the authoritative id from the finding itself. prompt: | Repository: {{project_dir}} Verify this single finding: {{finding}} + {{gate_feedback}} tools: - fs_read - fs_cat @@ -149,6 +158,14 @@ nodes: max_attempts: 2 state_updates: verdict: '{{output}}' + next: verdict_gate + + verdict_gate: + id: verdict_gate + type: script + description: Deterministic contract gate for each verdict — validates shape, verdict enum, and evidence/note rules; rejects back to verify_one exactly once with the reason, then records UNVERIFIABLE. No LLM judgment. + script: scripts/verdict_gate.py + timeout: 30 done: id: done diff --git a/assets/agents/finding-verifier/scripts/verdict_gate.py b/assets/agents/finding-verifier/scripts/verdict_gate.py new file mode 100644 index 0000000..63085f2 --- /dev/null +++ b/assets/agents/finding-verifier/scripts/verdict_gate.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +"""Contract gate for finding-verifier's per-finding verify chain. + +Validates the raw verifier output against the verdict contract: + - a single JSON object + - verdict in {VERIFIED, FALSE, UNVERIFIABLE} + - VERIFIED / FALSE require non-empty evidence + - UNVERIFIABLE requires a non-empty note + +On a valid response it normalizes the object (the authoritative `id` is +stamped from the finding item itself — the model's echo is never trusted) +and writes it to the chain's output key. On an invalid response it routes +back to `verify_one` exactly once with the rejection reason; if the retry +is also invalid, it records an UNVERIFIABLE verdict naming the failure so +a malformed item can never sink the whole map or masquerade as a pass. + +Deterministic by design: no LLM judgment happens here, and the script +always exits 0 with a valid JSON object on stdout. +""" + +import json +import os +import re + +VERDICTS = {"VERIFIED", "FALSE", "UNVERIFIABLE"} +MAX_RETRIES = 1 + + +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 extract_json(text): + if not isinstance(text, str) or not text.strip(): + raise ValueError("verifier output is empty or not text") + text = re.sub(r"```[a-zA-Z]*", "", text).replace("```", "") + start, end = text.find("{"), text.rfind("}") + if start == -1 or end <= start: + raise ValueError("no JSON object found in verifier output") + return json.loads(text[start : end + 1]) + + +def validate(obj): + if not isinstance(obj, dict): + raise ValueError("verifier output is not a JSON object") + verdict = obj.get("verdict") + if verdict not in VERDICTS: + raise ValueError(f"verdict must be one of {sorted(VERDICTS)}, got {verdict!r}") + evidence = obj.get("evidence") if isinstance(obj.get("evidence"), str) else "" + note = obj.get("note") if isinstance(obj.get("note"), str) else "" + if verdict in ("VERIFIED", "FALSE") and not evidence.strip(): + raise ValueError( + f"{verdict} requires non-empty evidence quoting the decisive line(s) with path:line" + ) + if verdict == "UNVERIFIABLE" and not note.strip(): + raise ValueError("UNVERIFIABLE requires a note explaining why it could not be established") + return verdict, evidence, note + + +def main(): + state = load_state() + fid = finding_id(state) + attempts = state.get("gate_attempts") + if not isinstance(attempts, int): + attempts = 0 + + try: + verdict, evidence, note = validate(extract_json(state.get("verdict"))) + print( + json.dumps( + {"verdict": {"id": fid, "verdict": verdict, "evidence": evidence, "note": note}} + ) + ) + return + except Exception as e: # noqa: BLE001 — every failure is a rejection reason + reason = str(e) + + if attempts < MAX_RETRIES: + print( + json.dumps( + { + "_next": "verify_one", + "gate_attempts": attempts + 1, + "gate_feedback": ( + "PREVIOUS RESPONSE REJECTED by the verdict gate: " + + reason + + ". Respond with ONLY the JSON object described under" + " 'Output format' — no prose, no code fences." + ), + } + ) + ) + return + + print( + json.dumps( + { + "verdict": { + "id": fid, + "verdict": "UNVERIFIABLE", + "evidence": "", + "note": f"verifier output failed machine validation after retry: {reason}", + } + } + ) + ) + + +try: + main() +except Exception as e: # noqa: BLE001 — the gate must never crash the chain + print( + json.dumps( + { + "verdict": { + "id": "unknown", + "verdict": "UNVERIFIABLE", + "evidence": "", + "note": f"verdict gate error: {e}", + } + } + ) + ) diff --git a/assets/agents/review-gauntlet/README.md b/assets/agents/review-gauntlet/README.md new file mode 100644 index 0000000..c372131 --- /dev/null +++ b/assets/agents/review-gauntlet/README.md @@ -0,0 +1,101 @@ +# Review Gauntlet + +A graph-based post-implementation review gate — **"the done gate"**. An +orchestrator that must not claim *done* without green reviews spawns this agent +once instead of judging trigger rules and parsing verdict sentinels itself. The +graph structure makes skipping a lane or misreading a verdict impossible: +trigger signals are computed by a script, lanes run as isolated parallel +sub-agents, and the final verdict is a deterministic regex gate — an LLM never +gets the chance to rationalize past it. + +```mermaid +flowchart TD + parse["parse
(llm: extract request facts)"] --> signals["signals
(script: deterministic diff facts)"] + signals --> select["select_lanes
(llm: ADDITIVE-only judgment)"] + select --> build["build_items
(script: rules floor ∪ additions;
forced lanes override exactly)"] + 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 & madv & msec & mpb --> gate["verdict_gate
(script: sentinel regex — deterministic)"] + gate --> done(["GAUNTLET: PASS | BLOCKED"]) +``` + +## Why a graph + +The four independent review lanes existed before this agent — as **prompt +discipline** inside the calling orchestrator: "spawn code-reviewer when the +change is broad", "🔴 always blocks", "INCONCLUSIVE is never PASS". Prompt +discipline degrades under context pressure: a lane gets skipped, a verdict +line gets paraphrased, a missing report reads as "nothing to report". Here +those rules are *structure*: + +- **Lane selection floor is deterministic** (`build_items.py`): code-review + always runs; adversary runs when a spec/plan is present; security runs on + attack-surface signals (auth/deps/exec paths or diff content) or hardened + posture; probe runs when consumer-facing surface changed *and* a local-run + recipe exists. The `select_lanes` LLM can only **widen** the set — a false + flag never removes a rule-selected lane. `forced_lanes` from the caller + override everything exactly. +- **Conditional lanes without dynamic routing**: each lane is a `map` node + over a 0-or-1-item list — an empty list means the map runs zero branches + (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 + 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. + +## Sentinels parsed + +| Lane | Sentinel | Blocks on | +|------|----------|-----------| +| code-reviewer | `**Verdict: MERGE-READY \| NEEDS-HUMAN**` | any 🔴 finding; missing sentinel | +| 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 | + +## Spawning it + +``` +agent__spawn --agent review-gauntlet --prompt "Post-implementation review gate. + +Project: /abs/path/to/repo +Diff: main...HEAD (a git range, a ref, or 'worktree' for uncommitted changes) +Rigor: production Security posture: standard + +Plan / acceptance criteria: + + +Local-run recipe / usage suites: + + +Forced lanes: (optional — e.g. 'adversary, probe' to run exactly those)" +``` + +The reply ends with `GAUNTLET: PASS` or `GAUNTLET: BLOCKED`, a lane status +table, the lane-selection reasons, blockers, attention items, and each lane's +full report in a collapsible section. On BLOCKED: fix the findings per your +own findings-handling rules, then spawn a **fresh** gauntlet run. On PASS with +*Human attention required*: complete the work but carry those items into your +final report verbatim. + +## Notes + +- 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. +- 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 + auth/authn/authz/oauth but not "author") — the safe direction, since signals + only ever ADD the security lane. +- Requires the `code-reviewer`, `adversary`, `security-reviewer`, and `probe` + agents to be installed (they ship with Coyote). diff --git a/assets/agents/review-gauntlet/graph.yaml b/assets/agents/review-gauntlet/graph.yaml new file mode 100644 index 0000000..c62a5f7 --- /dev/null +++ b/assets/agents/review-gauntlet/graph.yaml @@ -0,0 +1,350 @@ +name: review-gauntlet +description: | + Graph-based post-implementation review gate — "the done gate". Given a diff + spec and the work's context (plan/acceptance criteria, rigor, security + posture), it deterministically computes diff signals, selects the applicable + independent review lanes — code-reviewer (always), adversary (when a + spec/plan exists), security-reviewer (attack-surface signals), probe + (consumer-facing surface with a local-run recipe) — runs the selected lanes + IN PARALLEL as isolated sub-agents, and machine-parses their verdict + sentinels into one deterministic verdict: GAUNTLET: PASS or + GAUNTLET: BLOCKED. An orchestrator that must not claim "done" without green + reviews spawns this once instead of judging trigger rules and parsing + verdicts itself — the graph structure makes skipping a lane or misreading a + verdict impossible. Invariants: lane selection can only be WIDENED by LLM + judgment, never narrowed (deterministic rules are the floor); a missing + verdict sentinel is a lane failure, never a pass; probe INCONCLUSIVE blocks + with an environment note (it is never PASS or FAIL); reviewer lanes run + verdict-independent — no cross-lane messaging. + +version: '1.0' + +max_agent_depth: 4 +max_concurrent_agents: 12 + +variables: + - name: project_dir + description: Repository the change lives in; signals and every review lane run against it. + default: '.' + +settings: + max_loop_iterations: 10 + log_state_snapshots: true + timeout: 10800 + +initial_state: + diff_spec: worktree + plan_context: '' + rigor: production + security_posture: standard + probe_context: '' + extra_context: '' + forced_lanes: [] + changed_files: [] + file_count: 0 + signals_summary: '' + signals_error: '' + touches_auth: false + touches_deps: false + touches_exec: false + touches_proto: false + touches_migrations: false + consumer_surface: false + docs_only: false + add_code_review: false + add_adversary: false + add_security: false + add_probe: false + lane_reasons: '' + code_review_items: [] + adversary_items: [] + security_items: [] + probe_items: [] + code_review_results: [] + adversary_results: [] + security_results: [] + probe_results: [] + lanes_summary: '' + gauntlet_verdict: '' + gauntlet_report: '' + +start: parse + +nodes: + parse: + id: parse + type: llm + description: Extract the review request's facts from the spawn prompt. Extraction only — no judging, no lane selection. + instructions: | + You extract the facts of a review request into structured fields. + EXTRACTION ONLY — do not decide which review lanes should run. + + - `diff_spec`: the git diff specification the caller named — a ref + range ("main...HEAD", "HEAD~3..HEAD"), a single ref, or the literal + "worktree" for uncommitted changes against HEAD. When the caller + did not name one, use "worktree". + - `plan_context`: the plan / spec / acceptance-criteria text the + implementation must satisfy, verbatim (or a file path the caller + pointed at). Empty string when the caller supplied none. + - `rigor`: poc | prototype | production. Default "production" when + not stated. + - `security_posture`: prototype | standard | hardened. Default + "standard" when not stated. + - `probe_context`: the local-run recipe and/or existing usage-suite + pointers for black-box probing (how to boot the service, where the + suites live). Empty string when the caller supplied none. + - `forced_lanes`: lanes the caller explicitly demanded (verbatim + names). Empty array = automatic selection. + - `extra_context`: any remaining caller context reviewers should see, + condensed. + prompt: | + Extract the review-request facts from this request: + + {{initial_prompt}} + tools: [] + output_schema: + type: object + properties: + diff_spec: + type: string + plan_context: + type: string + rigor: + type: string + security_posture: + type: string + probe_context: + type: string + forced_lanes: + type: array + items: + type: string + extra_context: + type: string + required: [diff_spec, plan_context, rigor, security_posture, probe_context, forced_lanes, extra_context] + max_iterations: 3 + max_attempts: 2 + next: signals + + signals: + id: signals + type: script + description: Deterministic diff signals — file list, attack-surface and consumer-surface indicators. Pure git; no LLM judgment. + script: scripts/signals.py + timeout: 120 + next: select_lanes + + select_lanes: + id: select_lanes + type: llm + description: Additive-only lane judgment on top of the deterministic rules. It can WIDEN the gauntlet, never narrow it. + instructions: | + You look at a change's deterministic diff signals and decide whether any + review lane the hard rules might have missed should ALSO run. You can + only ADD lanes — the deterministic floor (enforced downstream, not by + you) is: code-review always runs; adversary runs when a plan/spec is + present; security runs on attack-surface signals or hardened posture; + probe runs when consumer-facing surface changed and a local-run recipe + exists. Setting a flag to false NEVER removes a lane the rules selected. + + Reasons to add: + - `add_security`: the diff handles external/untrusted input, touches + crypto/secrets handling, or changes trust boundaries in ways the + path-based signals missed. + - `add_adversary`: the request references behavior it must preserve or + criteria scattered in prose that amount to a spec, even without a + formal plan. + - `add_probe`: the change alters observable API/CLI behavior even + though the path signals missed it (only useful when a local-run + recipe exists — the builder skips it otherwise, noting why). + - `add_code_review`: always selected by the rules; flag exists only + for forced-lane configurations. + + Put one line per added lane into `lane_reasons`. When nothing needs + adding, all flags false and empty reasons is the correct answer. + prompt: | + Change signals (deterministic): + - diff spec: {{diff_spec}} + - files changed: {{file_count}} + - changed files (first 200): {{changed_files}} + - touches auth paths/content: {{touches_auth}} + - touches dependency manifests: {{touches_deps}} + - adds exec/shell/subprocess calls: {{touches_exec}} + - touches protobuf/contracts: {{touches_proto}} + - touches migrations: {{touches_migrations}} + - consumer-facing surface paths: {{consumer_surface}} + - docs-only diff: {{docs_only}} + - {{signals_summary}} + {{signals_error}} + + Caller context: + - rigor: {{rigor}}; security posture: {{security_posture}} + - plan/spec present: {{plan_context}} + - probe recipe present: {{probe_context}} + - extra: {{extra_context}} + tools: [] + output_schema: + type: object + properties: + add_code_review: + type: boolean + add_adversary: + type: boolean + add_security: + type: boolean + add_probe: + type: boolean + lane_reasons: + type: string + required: [add_code_review, add_adversary, add_security, add_probe, lane_reasons] + max_iterations: 3 + max_attempts: 2 + next: build_items + + build_items: + id: build_items + type: script + description: Deterministic lane builder — hard rules are the floor, LLM flags may only add, forced lanes override exactly. Emits one 0-or-1-item list per lane; an empty list skips that lane's map. + script: scripts/build_items.py + timeout: 30 + next: [map_code_review, map_adversary, map_security, map_probe] + + map_code_review: + id: map_code_review + type: map + over: '{{code_review_items}}' + as: lane_ctx + branch: run_code_review + output_key: lane_out + collect_into: code_review_results + max_concurrency: 1 + next: verdict_gate + + map_adversary: + id: map_adversary + type: map + over: '{{adversary_items}}' + as: lane_ctx + branch: run_adversary + output_key: lane_out + collect_into: adversary_results + max_concurrency: 1 + next: verdict_gate + + map_security: + id: map_security + type: map + over: '{{security_items}}' + as: lane_ctx + branch: run_security + output_key: lane_out + collect_into: security_results + max_concurrency: 1 + next: verdict_gate + + map_probe: + id: map_probe + type: map + over: '{{probe_items}}' + as: lane_ctx + branch: run_probe + output_key: lane_out + collect_into: probe_results + max_concurrency: 1 + next: verdict_gate + + run_code_review: + id: run_code_review + type: agent + description: Independent code-quality review lane. + agent: code-reviewer + timeout: 7200 + prompt: | + Review the code changes in {{project_dir}} (diff: {{diff_spec}}) as an + independent post-implementation review. + + Quality bar: rigor={{rigor}}. + + Plan / acceptance-criteria context (may be empty): + {{plan_context}} + + {{extra_context}} + + Close your report with your Review Verdict line. + state_updates: + lane_out: '{{output}}' + + run_adversary: + id: run_adversary + type: agent + description: Plan-conformance review lane. + agent: adversary + timeout: 7200 + prompt: | + Adversarial plan-conformance review in {{project_dir}}. + + The implementation to check: diff {{diff_spec}}. + + The spec / plan / acceptance criteria it must satisfy: + {{plan_context}} + + {{extra_context}} + + Return your ADVERSARIAL_REVIEW verdict. + state_updates: + lane_out: '{{output}}' + + run_security: + id: run_security + type: agent + description: Security review lane. + agent: security-reviewer + timeout: 7200 + prompt: | + Security review of the code changes in {{project_dir}} (diff: {{diff_spec}}). + + Security posture: {{security_posture}}. + + {{extra_context}} + + Return your SECURITY_REVIEW verdict. + state_updates: + lane_out: '{{output}}' + + run_probe: + id: run_probe + type: agent + description: Black-box usage-pattern verification lane. + agent: probe + timeout: 7200 + prompt: | + Usage-pattern verification (black-box probe) of the changes in + {{project_dir}} (diff: {{diff_spec}}). + + Local-run recipe / existing usage suites: + {{probe_context}} + + Plan / acceptance-criteria context: + {{plan_context}} + + 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. + state_updates: + lane_out: '{{output}}' + + 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. + script: scripts/verdict_gate.py + timeout: 60 + next: done + + done: + id: done + type: end + output: | + GAUNTLET: {{gauntlet_verdict}} + + {{gauntlet_report}} diff --git a/assets/agents/review-gauntlet/scripts/build_items.py b/assets/agents/review-gauntlet/scripts/build_items.py new file mode 100644 index 0000000..4a31e58 --- /dev/null +++ b/assets/agents/review-gauntlet/scripts/build_items.py @@ -0,0 +1,118 @@ +#!/usr/bin/env python3 +"""Deterministic lane builder for the review gauntlet. + +Selection = hard rules (the floor) ∪ LLM additions; forced lanes override +everything exactly. Emits one 0-or-1-item list per lane: an empty list makes +that lane's map node run zero branches (map-over-nothing), which IS the +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. +""" + +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() + +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", +} + + +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( + "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") + 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}") + +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/signals.py b/assets/agents/review-gauntlet/scripts/signals.py new file mode 100644 index 0000000..f36369f --- /dev/null +++ b/assets/agents/review-gauntlet/scripts/signals.py @@ -0,0 +1,99 @@ +#!/usr/bin/env python3 +"""Deterministic diff signals for the review gauntlet. + +Pure git — no LLM judgment. Every signal is computed from the changed-file +list and the added lines of the diff. Failures never sink the gauntlet: +on any error the script reports `signals_error` and leaves the defaults +(false/empty) in place, which biases lane selection toward the caller's +context and forced lanes. +""" + +import json +import os +import re +import subprocess + +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() +proj = state.get("project_dir") or "." +spec = (state.get("diff_spec") or "worktree").strip() + + +def git(*args): + r = subprocess.run( + ["git", "-C", proj, *args], capture_output=True, text=True, timeout=90 + ) + if r.returncode != 0: + raise RuntimeError((r.stderr or r.stdout).strip()[:400]) + return r.stdout + + +out = {} +try: + if spec in ("", "worktree"): + names = git("diff", "--name-only", "HEAD") + difftext = git("diff", "HEAD") + else: + names = git("diff", "--name-only", spec) + difftext = git("diff", spec) + files = [f.strip() for f in names.splitlines() if f.strip()] + added = "\n".join( + l for l in difftext.splitlines() if l.startswith("+") and not l.startswith("+++") + ) + + def any_path(pattern): + return any(re.search(pattern, f, re.I) for f in files) + + out.update( + { + "changed_files": files[:200], + "file_count": len(files), + "touches_proto": any_path(r"\.proto$|buf\.(yaml|gen\.yaml)$"), + "touches_migrations": any_path(r"migrations?/|\.sql$"), + # auth(?!or) admits auth/authn/authz/oauth but not "author". + # Over-firing here is the safe direction: it only ADDS a lane. + "touches_auth": any_path( + r"auth(?!or)|login|logout|token|session|permission|rbac|\bacl\b|credential|secret" + ) + or bool( + re.search( + r"auth(?!or)|password|token|secret|credential|jwt|session", + added, + re.I, + ) + ), + "touches_deps": any_path( + r"go\.(mod|sum)$|package(-lock)?\.json$|yarn\.lock$|pnpm-lock\.yaml$" + r"|Cargo\.(toml|lock)$|requirements[^/]*\.txt$|pyproject\.toml$" + r"|Gemfile(\.lock)?$|pom\.xml$|build\.gradle|composer\.(json|lock)$" + ), + "touches_exec": bool( + re.search( + r"exec\.Command|subprocess|os/exec|Runtime\.getRuntime" + r"|shell_exec|\beval\(|\bsystem\(|child_process", + added, + ) + ), + "consumer_surface": any_path( + r"routes?|handlers?|\bapi\b|endpoints?|\bcmd/|\bcli\b|\.proto$" + r"|openapi|swagger|controllers?|graphql" + ), + "docs_only": bool(files) + and all(re.search(r"\.(md|rst|txt|adoc)$", f, re.I) for f in files), + "signals_summary": f"{len(files)} file(s) changed (diff: {spec})", + } + ) +except Exception as e: # noqa: BLE001 — degraded signals must not sink the gauntlet + out["signals_error"] = ( + f"NOTE: diff signals could not be computed ({e}); " + "lane selection falls back to caller context and forced lanes." + ) + out["signals_summary"] = "signals unavailable" + +print(json.dumps(out)) diff --git a/assets/agents/review-gauntlet/scripts/verdict_gate.py b/assets/agents/review-gauntlet/scripts/verdict_gate.py new file mode 100644 index 0000000..d1eea2d --- /dev/null +++ b/assets/agents/review-gauntlet/scripts/verdict_gate.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +"""Deterministic verdict gate for the review gauntlet. + +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; + - 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 + recipe, then re-run); + - the gate itself must never crash into a silent pass: any internal + error emits GAUNTLET: BLOCKED naming the error. +""" + +import json +import os +import re + +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() + blockers, attention, lane_lines, reports = [], [], [], [] + + 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) + + def record(lane, status, detail=""): + lane_lines.append(f"| {lane} | {status} | {detail} |") + + # --- code-review ------------------------------------------------------ + cr = lane_report("code_review_results") + if cr is None: + record("code-review", "SKIPPED", "not selected") + else: + reports.append(("code-review", cr)) + m = re.search(r"Verdict:\**\s*\**\s*(MERGE-READY|NEEDS-HUMAN)", cr) + reds = cr.count("🔴") + if reds: + blockers.append( + f"code-review: {reds} 🔴 CRITICAL finding(s) — fix before claiming done" + ) + record("code-review", "BLOCKED", f"{reds} 🔴 finding(s)") + elif m and m.group(1) == "MERGE-READY": + record("code-review", "GREEN", "MERGE-READY") + elif m: + attention.append( + "code-review: NEEDS-HUMAN — see the lane report's " + "'Human attention required' section" + ) + record("code-review", "GREEN (attention)", "NEEDS-HUMAN, no 🔴") + else: + blockers.append( + "code-review: no Review Verdict sentinel found — the lane failed; " + "a missing verdict is never a pass" + ) + record("code-review", "BLOCKED", "missing verdict sentinel") + + # --- adversary -------------------------------------------------------- + adv = lane_report("adversary_results") + if adv is None: + record("adversary", "SKIPPED", "not selected") + else: + reports.append(("adversary", adv)) + m = re.search(r"ADVERSARIAL_REVIEW:\s*(CONFORMS|DIVERGES)", adv) + if m and m.group(1) == "CONFORMS": + record("adversary", "GREEN", "CONFORMS") + elif m: + blockers.append( + "adversary: DIVERGES — the implementation does not conform to the plan" + ) + record("adversary", "BLOCKED", "DIVERGES") + else: + blockers.append( + "adversary: no ADVERSARIAL_REVIEW sentinel found — lane failed; never a pass" + ) + record("adversary", "BLOCKED", "missing verdict sentinel") + + # --- security --------------------------------------------------------- + sec = lane_report("security_results") + if sec is None: + record("security", "SKIPPED", "not selected") + else: + reports.append(("security", sec)) + m = re.search(r"SECURITY_REVIEW:\s*(PASS|FAIL)", sec) + if m and m.group(1) == "PASS": + record("security", "GREEN", "PASS") + elif m: + blockers.append("security: SECURITY_REVIEW: FAIL — exploitable finding(s)") + record("security", "BLOCKED", "FAIL") + else: + blockers.append( + "security: no SECURITY_REVIEW sentinel found — lane failed; never a pass" + ) + record("security", "BLOCKED", "missing verdict sentinel") + + # --- probe ------------------------------------------------------------ + pb = lane_report("probe_results") + if pb is None: + record("probe", "SKIPPED", "not selected") + else: + reports.append(("probe", pb)) + m = re.search(r"USAGE_PROBE:\s*(PASS|FAIL|INCONCLUSIVE)", pb) + if m and m.group(1) == "PASS": + record("probe", "GREEN", "PASS") + elif m and m.group(1) == "FAIL": + blockers.append("probe: USAGE_PROBE: FAIL — observed behavior contradicts the contract") + record("probe", "BLOCKED", "FAIL") + elif m: + blockers.append( + "probe: INCONCLUSIVE — the ENVIRONMENT could not be established; this is " + "not a code verdict. Follow the plan's local-run recipe to fix the " + "environment and re-run. INCONCLUSIVE is never treated as PASS or FAIL." + ) + record("probe", "BLOCKED", "INCONCLUSIVE (environment)") + else: + blockers.append( + "probe: no USAGE_PROBE sentinel found — lane failed; never a pass" + ) + record("probe", "BLOCKED", "missing verdict sentinel") + + verdict = "BLOCKED" if blockers else "PASS" + + md = "\n".join( + ["| lane | status | detail |", "|------|--------|--------|"] + lane_lines + ) + md += "\n\n## Lane selection\n" + (state.get("lanes_summary") or "(none recorded)") + if blockers: + md += "\n\n## Blockers\n" + "\n".join(f"- {b}" for b in blockers) + if attention: + md += "\n\n## Human attention required\n" + "\n".join(f"- {a}" for a in attention) + if state.get("signals_error"): + md += f"\n\n> {state['signals_error']}" + md += "\n\n## Full lane reports\n" + for lane, rep in reports: + md += f"\n
{lane}\n\n{rep}\n\n
\n" + + print(json.dumps({"gauntlet_verdict": verdict, "gauntlet_report": md})) + + +try: + main() +except Exception as e: # noqa: BLE001 — the gate must never crash into a silent pass + print( + json.dumps( + { + "gauntlet_verdict": "BLOCKED", + "gauntlet_report": f"verdict gate error: {e} — treat as a failed gate, not a pass.", + } + ) + ) diff --git a/assets/agents/security-reviewer/config.yaml b/assets/agents/security-reviewer/config.yaml index 79ff4e8..60149bd 100644 --- a/assets/agents/security-reviewer/config.yaml +++ b/assets/agents/security-reviewer/config.yaml @@ -1,6 +1,6 @@ name: security-reviewer description: Security analyst - hunts exploitable flaws in a code change (injection, secrets, authz gaps, SSRF, supply chain) by tracing untrusted data to dangerous sinks. Returns a posture-gated PASS/FAIL verdict so POCs aren't held to production strictness. Complements code-reviewer (quality) and adversary (plan conformance). Designed to be delegated to by sisyphus. -version: 1.0.0 +version: 1.1.0 auto_continue: true max_auto_continues: 15 @@ -61,6 +61,17 @@ instructions: | 1. Load `security-review`. 2. Get the diff (inline or via `get_diff`) and identify the changed files. + 2b. **Mechanized pre-pass (deterministic, read-only).** Before manual tracing, run whichever of + these exist on PATH and fit the repo — never install anything, skip each silently when absent, + time-box each to ~60s via `execute_command`: + - `gitleaks` on the changed paths (committed-secret candidates); + - `semgrep --config auto --error --quiet` scoped to the changed files (or the repo's own + `.semgrep.yml` when present); + - a dependency audit when a manifest/lockfile changed: `osv-scanner --lockfile `, + `npm audit --omit dev`, `cargo audit`, or `pip-audit` — whichever matches. + Treat every mechanized hit as a LEAD, not a finding: trace it per steps 3-5 and report it only + with the confirmed source→sink path (or secret location) and your own severity calibration. + Note which tools ran/were skipped in the report. Raw scanner output is never pasted as findings. 3. For EACH hunk: identify untrusted-data sources, dangerous sinks, and the mediation (or lack of it) between them. Apply the skill's hunt checklist (secrets, injection, paths, authn/authz, deserialization, network, supply chain, crypto, data exposure, resource abuse). diff --git a/assets/agents/sisyphus/README.md b/assets/agents/sisyphus/README.md index 717b3c8..52af04b 100644 --- a/assets/agents/sisyphus/README.md +++ b/assets/agents/sisyphus/README.md @@ -47,6 +47,7 @@ Spawnable sub-agents (from `config.yaml`): - **[code-reviewer](../code-reviewer/README.md)** — independent post-implementation review; fires when the change is broad (2+ coders, 5+ files) or crosses architectural boundaries. - **[adversary](../adversary/README.md)** — plan-conformance review; fires whenever the change implements a written spec, plan step, or acceptance-criteria list. Orthogonal to `code-reviewer` — both can run. - **[security-reviewer](../security-reviewer/README.md)** — security analysis; fires when the change touches attack surface (external input, auth/secrets, shell/file-path sinks, new dependencies). Verdict is posture-gated (`prototype`/`standard`/`hardened`) so POCs aren't held to production strictness, but Critical findings (committed secrets, host-endangering code) block in every posture. Orthogonal to both other reviewers — all three can run. +- **[review-gauntlet](../review-gauntlet/README.md)** — preferred single entry point for the post-implementation review passes: a graph that computes diff signals deterministically, runs the applicable lanes (code-reviewer / adversary / security-reviewer / probe) in parallel, and machine-parses their verdict sentinels into GAUNTLET: PASS/BLOCKED — skipping a lane or misreading a verdict becomes structurally impossible. The reviewer bullets above describe the lanes it runs and remain the spawn fallback when it is not installed. - **[step-runner](../step-runner/README.md)** — graph agent that executes one step of a phased plan repo. Internally delegates to `coder` for implementation and optionally to `code-reviewer` for review. - **[whetstone](../whetstone/README.md)** — continuous improvement; spawned ONCE at the end of a run that addressed review feedback from outside Sisyphus's own lanes (humans, or a teammate's independent review agent — never its own reviewers' output). Classifies each piece of feedback CATCH/PARTIAL/MISS against what the run's own reviewers flagged and converts the misses into user-approved suite improvements (skill edits, ledger entries). Never blocks run completion. @@ -57,6 +58,7 @@ Spawnable sub-agents (from `config.yaml`): - 🔄 **Task Management**: Tracks progress and context across complex operations. - 🛠️ **Tool Integration**: Seamlessly uses system tools for building, testing, and file manipulation. - 📋 **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. ## Plan-Driven Workflows diff --git a/assets/agents/sisyphus/config.yaml b/assets/agents/sisyphus/config.yaml index e1f38fb..57ec5c0 100644 --- a/assets/agents/sisyphus/config.yaml +++ b/assets/agents/sisyphus/config.yaml @@ -1,6 +1,6 @@ name: sisyphus description: OpenCode-style orchestrator - classifies intent, delegates to specialists, tracks progress with todos, enforces OMO-grade verification discipline -version: 3.11.0 +version: 3.13.0 agent_session: temp auto_continue: true @@ -17,11 +17,12 @@ spawnable_agents: - adversary - security-reviewer - probe + - review-gauntlet - architecture-reviewer - step-runner - whetstone max_concurrent_agents: 40 -max_agent_depth: 3 +max_agent_depth: 5 inject_spawn_instructions: true summarization_threshold: 80000 @@ -55,6 +56,9 @@ variables: - name: auto_confirm description: Auto-confirm command execution default: '1' + - name: escalation_policy + description: "How escalation-worthy decisions are handled mid-run: 'interactive' pauses and asks the user; 'autonomous' self-resolves decisions covered by a standing ruling or a clearly recommended option, reports them in a final-report digest, and only pings on hard triggers or novel conflicts" + default: 'interactive' mcp_servers: - ddg-search @@ -292,6 +296,38 @@ instructions: | - Never reuse a verification claim from an earlier report (yours or another agent's) without re-running the command yourself. Prior reports are unverified context, not evidence. - An honest failure — "gate X failed / could not run, here is the verbatim error" — is an acceptable, preferable deliverable. A success-shaped report with missing evidence poisons every downstream consumer. + ### The review gauntlet (preferred): one spawn, every review lane, machine-parsed verdicts + + When the `review-gauntlet` agent is installed, run the post-implementation review passes through + it instead of spawning the individual reviewers yourself. Spawn it ONCE whenever the work was + non-trivial (any trigger in the reviewer sections below would fire, or you are unsure whether one + would): + + ``` + agent__spawn --agent review-gauntlet --prompt "Post-implementation review gate. + Project: + Diff: + Rigor: Security posture: + Plan / acceptance criteria: + Local-run recipe / usage suites: " + ``` + + 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 + parallel verdict-independent, and returns a machine-parsed `GAUNTLET: PASS` or `GAUNTLET: BLOCKED` + with a lane table, blockers, attention items, and every lane's full report. + + - `GAUNTLET: BLOCKED` blocks completion. Route each blocker through the findings-handling rules in + the reviewer sections below (🔴 → coder fix in the SAME session; DIVERGES → the adversary rules; + security FAIL → the security rules; probe INCONCLUSIVE → fix the ENVIRONMENT per the plan's + local-run recipe first — it is never a code verdict). Then spawn a FRESH gauntlet run; lane + selection re-runs from the new diff. + - `GAUNTLET: PASS` with a "Human attention required" section: finish the work and carry those + items into your final report verbatim. + - The reviewer sections below remain authoritative for HANDLING each lane's findings, and are the + spawn fallback when `review-gauntlet` is not installed. + ### Independent code review (post-coder, non-trivial work) After completing delegated `coder` work, spawn `code-reviewer` for an independent review pass if ANY of these are true: @@ -587,9 +623,49 @@ instructions: | - Writing non-trivial code → always `coder` (graph agent runs verification internally) - Multi-angle exploration → fan out `explore` agents + ## Escalation policy: {{escalation_policy}} + + Every "ASK the user" / "STOP and escalate" instruction in this config is subject to this policy. + + - **interactive** (default) — pause and ask exactly as written throughout this config. + - **autonomous** — the user pre-delegated decision-making for this run ("here's the spec — go"). + When an escalation-worthy decision arises, work this ladder IN ORDER: + + 1. **Hard triggers — ALWAYS escalate; no ruling or policy can delegate these:** decisions about + secrets/credentials, money movement, destructive or irreversible actions (data loss, + force-push, dropping data or public API without a migration path), anything the Hard Blocks + section forbids — and any decision that already failed once after an autonomous resolution + (a ruling that did not survive one fix cycle goes to a human, never into a retry loop). + 2. **Standing rulings** — read `$HOME/.config/coyote/share/standing-rulings.md` (skip silently + if absent). If a ruling's trigger GENUINELY matches the decision at hand, apply its + resolution. Match on substance, not resemblance — a ruling about security-vs-spec conflicts + does not authorize scope changes. Unsure whether it matches? Then it does not. + 3. **Clearly recommended option** — if you would have presented options with one marked + `(Recommended)` backed by concrete evidence (a reviewer's proposed fix, a failing test that + names the cause, a repo convention), take the recommended option. + 4. **Otherwise — escalate** exactly as interactive mode would. A genuinely novel conflict or a + decision with no defensible recommendation still reaches the user; autonomy never converts + "I don't know" into a guess. + + **Plan amendments taken autonomously.** When a resolution amends the spec/plan/acceptance + criteria (e.g. a security fix conflicts with a criterion's wording), REWRITE the affected + criterion text in your working plan context BEFORE re-running any reviewer or the gauntlet — + the adversary must verify the plan-as-amended, not the stale wording. The amendment is always + a visible artifact, never implied. + + **The digest (MANDATORY when any autonomous decision was taken).** Your final report — and the + PR body, when you open one — MUST include a `## Decisions taken autonomously` section: one line + per decision stating what was decided, the basis (`ruling: ` or `recommended option`), + and what changed (including any plan amendment). Zero autonomous decisions → omit the section. + This converts interrupt-driven review into digest-driven review — never into silence. + + **Close the loop.** When a decision DID reach the user (step 4), suggest in your final report + that they record the resolution with `coyote --macro record-ruling` so that class of decision + never asks again. + ## User Interaction (get buy-in before major decisions) - Use `user__ask`, `user__confirm`, `user__checkbox`, `user__input` to clarify ambiguities interactively. **Do NOT guess when you can ask.** + Use `user__ask`, `user__confirm`, `user__checkbox`, `user__input` to clarify ambiguities interactively. **Do NOT guess when you can ask.** When `escalation_policy` is `autonomous`, the Escalation policy section above governs which of these actually pause the run — autonomy substitutes standing rulings and evidence-backed recommendations for the pause; it never substitutes guessing. | Situation | Tool | |-----------|------| @@ -620,6 +696,11 @@ instructions: | If you see `pending_escalations` in tool results, a child agent needs user input and is blocked. Reply promptly via `agent__reply_escalation`. You can answer from context, or prompt the user yourself first and relay the answer. + In `autonomous` escalation policy, work the Escalation-policy ladder before prompting the user: a + matching standing ruling or a clearly recommended option in the child's escalation answers it + directly via `agent__reply_escalation` (hard triggers always go to the user). Replies that amend a + plan/spec follow the amendment rule above, and every autonomous reply lands in the digest. + ## Anti-Patterns (BLOCKING) - Skipping intent verbalization → unclear routing, wasted turns @@ -647,6 +728,7 @@ instructions: | ## Context - Project: {{project_dir}} + - Escalation policy: {{escalation_policy}} - OS: {{__os__}} - Shell: {{__shell__}} - CWD: {{__cwd__}} diff --git a/assets/agents/whetstone/README.md b/assets/agents/whetstone/README.md index 9dc31f2..4401d6e 100644 --- a/assets/agents/whetstone/README.md +++ b/assets/agents/whetstone/README.md @@ -15,8 +15,7 @@ cluster into learnable classes — and that a miss, once *recorded*, converts to the next review. The study was a one-time manual harvest; whetstone is the same loop run continuously, at the moment the signal is strongest: when human feedback on a PR is being addressed, the spawner (sisyphus/architect) knows BOTH what the humans said and what the -suite's own lanes flagged — ground truth the periodic `mine-review-misses` macro has to -re-infer from comment formats. +suite's own lanes flagged — ground truth that offline comment mining could only re-infer from formats. ## How it fits @@ -50,12 +49,13 @@ re-infer from comment formats. | Tool | Sees | When | |------|------|------| | `whetstone` | Human feedback on PRs YOUR runs addressed, with suite ground truth | Real-time, per run | -| `mine-review-misses` | All human comments across an org/repo set, historical | Periodic batch / bootstrap | +| Review-round reconciliation | Human-caught, panel-missed defects on any PR the panel re-reviews → candidates inbox (`share/review-miss-candidates.jsonl`) | Automatic, every round | | `record-miss` | Defects that escaped review entirely (prod, incidents) | One-off, when it bites | -Whetstone narrows what the miner finds on your own PRs, but doesn't replace it: the miner -harvests teammates' PRs, repos you don't code in, and years of history in one run — and -`record-miss` covers the misses no one ever commented on. +Collection is automatic end to end: whetstone captures misses the moment feedback is +addressed, round reconciliation captures them on any PR the panel re-reviews (teammates' +PRs included), and `record-miss` covers the misses no one ever commented on. Nothing +auto-appends to the LEDGER itself — candidates wait in the inbox until a human promotes them. ## Rules that keep it safe diff --git a/assets/macros/mine-review-misses.yaml b/assets/macros/mine-review-misses.yaml deleted file mode 100644 index 4d00f52..0000000 --- a/assets/macros/mine-review-misses.yaml +++ /dev/null @@ -1,77 +0,0 @@ -# Mine PR review history for review-miss ledger candidates — the portable version of -# the gap-study methodology: pull real review -# comments from the repos you care about, find the defect CLASSES human reviewers keep -# enforcing that automated review would miss (org conventions, domain semantics, live -# org state, missing mechanical rules), and seed the ledger with the ones you approve. -# Use it when joining a team / moving jobs to bootstrap the ledger from the org's -# actual review culture instead of waiting for misses to bite one at a time. -# -# The first step is deterministic shell: it resolves the scope, pulls the corpus via -# gh into ~/.cache/coyote/mine-misses/, strips ANSI (CLICOLOR_FORCE breaks piped gh -# JSON), and normalizes to JSONL. The agent step then splits reviewer lanes, mines -# defect classes, proposes candidates, and appends ONLY what you approve — -# idempotently, in record-miss's entry format. -# -# Run as: -# coyote --macro mine-review-misses "myorg" # org: 12 most recently pushed repos -# coyote --macro mine-review-misses "owner/repo1,owner/repo2" # explicit repo list -# coyote --macro mine-review-misses "." # the repo in the current directory -# -# Re-run it periodically: each run stores a per-scope watermark under -# ~/.config/coyote/share/mine-misses-state/, and later runs pull only comments updated -# since the previous run (falling back to the newest-400 window on first run). Appends -# are idempotent and candidates are deduped against the ledger, so re-running is always -# safe. If you ABORT a run before reviewing its candidates, delete that scope's -# watermark file to re-widen the next window. Note the miner only sees misses a human -# eventually commented on — defects that escaped review entirely (caught in prod or an -# incident) are record-miss's job; the two are complementary. -# -# Notes: an org-wide pull is a few hundred GitHub API calls (a few minutes; well under -# authenticated rate limits). Mining quality depends on humans having reviewed there — -# a repo reviewed only by bots yields nothing, and that itself is worth knowing. -variables: - - name: scope - description: GitHub org name, comma-separated owner/repo list, or "." for the current directory's repo -isolated: false -steps: - - |- - !export GH_NO_UPDATE_NOTIFIER=1 NO_COLOR=1; unset CLICOLOR_FORCE GH_FORCE_TTY 2>/dev/null; d="$HOME/.cache/coyote/mine-misses"; rm -rf "$d"; mkdir -p "$d"; scope="{{scope}}" - case "$scope" in - .|"") repos="$(git remote get-url origin 2>/dev/null | sed -E 's#^(git@|https?://)([^/:]+)[:/]##; s#\.git$##')" ;; - */*) repos="$(printf '%s' "$scope" | tr ',' ' ')" ;; - *) repos="$(gh repo list "$scope" --no-archived --limit 12 --json nameWithOwner --jq '.[].nameWithOwner' | tr '\n' ' ')" ;; - esac - repos=$(echo $repos); [ -n "$repos" ] || { echo "mine-review-misses: could not resolve any repos from scope '$scope' (pass an org name, an owner/repo list, or run inside a git repo)"; exit 1; } - state="$HOME/.config/coyote/share/mine-misses-state"; mkdir -p "$state"; skey="$state/$(printf '%s' "$scope" | tr '/,: ' '____')" - sincearg=""; window="newest 400 comments per repo (first run for this scope)" - if [ -f "$skey" ]; then w=$(cat "$skey"); sincearg="&since=$w"; window="comments updated since $w (incremental)"; fi - now=$(date -u +%Y-%m-%dT%H:%M:%SZ) - echo "scope: $scope" > "$d/MANIFEST"; echo "repos: $repos" >> "$d/MANIFEST"; echo "window: $window" >> "$d/MANIFEST" - for r in $repos; do - slug=$(printf '%s' "$r" | tr '/' '_') - : > "$d/$slug-comments.raw" - for p in 1 2 3 4; do out=$(gh api "repos/$r/pulls/comments?sort=created&direction=desc&per_page=100&page=$p$sincearg" --jq '.[] | {pr: (.pull_request_url|split("/")|last), user: .user.login, path: .path, line: (.line // .original_line), created: .created_at, body: (.body|.[0:600])}' 2>/dev/null) || break; [ -n "$out" ] || break; printf '%s\n' "$out" >> "$d/$slug-comments.raw"; done - gh pr list --repo "$r" --state all --limit 300 --json number,author,title --jq '.[] | {n: .number, author: .author.login, title: .title}' > "$d/$slug-prs.raw" 2>/dev/null - : > "$d/$slug-reviews.raw" - for n in $(gh api "search/issues?q=repo:$r+is:pr+review:changes_requested&sort=updated&order=desc&per_page=30" --jq '.items[].number' 2>/dev/null); do - gh api "repos/$r/pulls/$n/reviews" --jq '.[] | select(.state=="CHANGES_REQUESTED" or (.state=="COMMENTED" and (.body|length)>60)) | {pr: '"$n"', user: .user.login, state: .state, body: (.body|.[0:600])}' >> "$d/$slug-reviews.raw" 2>/dev/null - done - for f in "$d/$slug-comments" "$d/$slug-prs" "$d/$slug-reviews"; do - sed 's/\x1b\[[0-9;]*m//g' "$f.raw" 2>/dev/null | jq -c . > "$f.jsonl" 2>/dev/null || : > "$f.jsonl"; rm -f "$f.raw" - done - echo "$r: comments=$(wc -l < "$d/$slug-comments.jsonl" | tr -d ' ') prs=$(wc -l < "$d/$slug-prs.jsonl" | tr -d ' ') review_bodies=$(wc -l < "$d/$slug-reviews.jsonl" | tr -d ' ')" >> "$d/MANIFEST" - done - echo "$now" > "$skey" - cat "$d/MANIFEST" - - |- - A PR-review corpus was just pulled into `$HOME/.cache/coyote/mine-misses/` — read `MANIFEST` there first (scope, repos, the time window, per-repo counts). Per repo the files are `_-comments.jsonl` (inline review comments: {pr,user,path,line,created,body}), `_-prs.jsonl` (PR metadata: {n,author,title}), and `_-reviews.jsonl` (CHANGES_REQUESTED / substantive review bodies: {pr,user,state,body}). On an INCREMENTAL window (see MANIFEST) the comment corpus covers only activity since the previous run — near-zero counts mean little new review activity, which you should simply report; the review-bodies file is not windowed, so on incremental runs ignore review bodies from PRs you'd expect were already mined. Load the `review-misses` skill, then mine this corpus for ledger candidates. Follow the method in order — each step corrects a specific bias: - - 1. **Lane hygiene first** (skipping this makes the analysis circular). Per repo: (a) drop PR-author self-replies — join comments to `-prs.jsonl` by PR number (jq) and drop records where the commenter is that PR's author; (b) exclude non-human reviewers — `[bot]`-suffixed accounts AND any account whose comments are recognizably automated-review output (severity-emoji conventions like 🔴/🟠/🟡/🟢, lane tags like `[adversary]`/`[code-review]`, "Panel review" tables, templated verdict blocks — automated reviews are sometimes posted from a personal account, so judge by format, not just username). Name the accounts you excluded and why. What remains is the organic human corpus — report its size per repo; if a repo has almost none, say so and move on. - - 2. **Mine defect CLASSES, not comments.** Read the organic human comments and review bodies looking for RECURRING things humans enforce, in two families: (a) knowledge not derivable from the repo — org conventions (especially ones the repo's own prior art CONTRADICTS: those poison neighbor-convention heuristics and make the strongest entries), domain/product semantics, live org state (client registries, environment facts, safe test tenants); (b) objectively checkable patterns reviewers keep flagging by hand (grep-shaped rules). EXCLUDE taste — naming preferences, style debates, "I'd have structured this differently" — a ledger polluted with taste gets ignored. One candidate per CLASS: merge repeat occurrences into a single entry citing multiple PRs. - - 3. **Dedup against the existing ledger.** Read `$HOME/.config/coyote/share/review-misses.md` (fall back to the legacy `$HOME/.config/coyote/review-misses.md` if share/ has none). A candidate matching an existing entry's defect class becomes a proposal to SHARPEN that entry (name its MISS id), not a new one. - - 4. **Propose, don't write.** Present at most 10 candidates, strongest first (recurrence × consequence), each formatted exactly as a ledger entry: title / repo(s) / surface (rest-api|worker|db-migration|cli|iac|library|ci-cd|any) / what escaped (real PR numbers + a short verbatim quote as evidence — never invent citations) / why review missed it / **detection pattern** (the payload: a concrete check or grep a reviewer can run, not advice). Then ask me which to record — use a checkbox selection. - - 5. **Append only the approved entries**, idempotently, one at a time with exactly this shell shape (title-match skip; numbering continues from the ledger's current count): `f="$HOME/.config/coyote/share/review-misses.md"; mkdir -p "$(dirname "$f")"; [ -f "$f" ] || printf '# Review-miss ledger\n\nFormat and usage: the review-misses skill.\n' > "$f"; grep -qF "" "$f" || { n=$(grep -c '^## MISS-' "$f"); printf '\n## MISS-%03d — <title>\n- date: %s\n- repo: <repos>\n- surface: <surface>\n- what escaped: <what>\n- why review missed it: <why>\n- detection pattern: <detection>\n' "$((n+1))" "$(date +%F)" >> "$f"; }` — for sharpen-proposals, edit the existing entry in place instead. Finish by showing me the ledger's full `## MISS-` title list so I see the final state. diff --git a/assets/macros/record-ruling.yaml b/assets/macros/record-ruling.yaml new file mode 100644 index 0000000..9264b73 --- /dev/null +++ b/assets/macros/record-ruling.yaml @@ -0,0 +1,18 @@ +# Record a standing ruling — a pre-delegated decision that agents running with +# escalation_policy: autonomous may apply INSTEAD of escalating — in +# ~/.config/coyote/share/standing-rulings.md. The first step appends a numbered +# skeleton entry deterministically (zero tokens); the second has the agent interview +# you to fill in the <fill> fields — the trigger is the payload: it must be narrow +# enough that a matching decision is unmistakable. +# Run as: +# coyote --macro record-ruling "<one line: when X happens, resolve by Y>" +# (avoid double quotes inside the description; it is spliced into a shell string) +variables: + - name: description + description: One line stating the ruling — when <situation>, resolve by <action> +isolated: false +steps: + - |- + !f="$HOME/.config/coyote/share/standing-rulings.md"; mkdir -p "$(dirname "$f")"; [ -f "$f" ] || printf '# Standing rulings\n\nPre-delegated decisions for escalation_policy: autonomous — an agent applies a ruling\nINSTEAD of escalating when its trigger genuinely matches. Hard triggers (secrets, money\nmovement, destructive/irreversible actions) can never be delegated here. Every autonomous\napplication is reported in the run digest.\n' > "$f"; n=$(grep -c '^## R-' "$f" 2>/dev/null || true); n=$((${n:-0}+1)); id=$(printf 'R-%03d' "$n"); printf '\n## %s — {{description}}\n- date: %s\n- trigger: <fill the narrow situation this covers — matching must be unmistakable>\n- resolution: <fill exactly what to do, including whether plan/spec text gets amended>\n- scope: <fill any|repo/surface limits>\n- provenance: <fill the escalation/PR/incident this came from>\n' "$id" "$(date +%F)" >> "$f"; echo "Appended $id to $f" + - |- + Open `$HOME/.config/coyote/share/standing-rulings.md` and find the newest `## R-` entry (just appended: "{{description}}"). Interview me briefly — one focused question at a time — to fill in the `<fill …>` fields, then write the completed entry back. Push hardest on the **trigger**: it must be narrow and unambiguous — a ruling that could plausibly match unrelated decisions is worse than no ruling, because agents apply these unsupervised. Confirm the resolution states whether plan/spec text gets amended. REFUSE to record a ruling that would delegate a hard trigger (secrets/credentials, money movement, destructive or irreversible actions) — those always escalate, by design. If the ruling matches an existing entry's class, propose sharpening that entry instead of keeping a duplicate. diff --git a/assets/skills/adversarial-review/SKILL.md b/assets/skills/adversarial-review/SKILL.md deleted file mode 100644 index 3222260..0000000 --- a/assets/skills/adversarial-review/SKILL.md +++ /dev/null @@ -1,79 +0,0 @@ ---- -description: Adversarial plan-conformance review of an implementation against the task/plan it was supposed to satisfy. Verdict is CONFORMS or DIVERGES with acceptance-criterion-referenced complaints. Grants read-only filesystem access for ground-truth checks. Complements code-review (which judges code quality); this judges whether the code is the RIGHT code per the plan. -enabled_tools: fs_read, fs_grep, fs_glob, fs_cat, fs_ls ---- -You are an adversarial plan-conformance reviewer. A code-quality reviewer already asks "is this code good?" — you ask a different, harder question: **"is this the code the plan asked for, and ONLY that?"** You are hunting for the gap between what was specified and what was built. Assume the implementer drifted, cut a corner, or misread the plan until the diff proves otherwise. Your independence is the value: you have no stake in the implementation decisions and no reason to rationalize them. - -You review THE CHANGE against THE PLAN. You are given (a) the diff, (b) the task/plan it implements — its Objective, Tasks, and above all its **Acceptance criteria**. If the plan is missing, say so and stop: you cannot judge conformance without a spec. - -## The core discipline: map every acceptance criterion to evidence - -For EACH acceptance criterion in the plan, find the specific evidence in the diff that satisfies it, and classify: - -| Verdict per criterion | Meaning | -|---|---| -| ✅ **Met** | The diff contains code that observably satisfies this criterion, AND a test that will fail if it regresses. Cite the file:line. | -| ⚠️ **Partial** | Some of the criterion is implemented but a case, path, or sub-requirement is missing. Name what's missing. | -| ❌ **Unmet** | No code in the diff satisfies this criterion. The "dog that didn't bark." | -| 🔀 **Diverged** | The diff implements something ADJACENT to the criterion but not it — different interface, different behavior, different data shape than specified. | - -A criterion with no corresponding test is at best ⚠️ Partial — "implemented but unverifiable" is not "met." An acceptance criterion is a promise of observable behavior; if nothing proves the behavior, the promise is unkept. - -## What to hunt for (adversarial checklist) - -### 1. Silently skipped criteria (the dog that didn't bark) -Read the acceptance criteria list, then the diff. Every criterion with no matching change is a finding. Implementers under-deliver far more often by *omission* than by writing wrong code. The absent migration, the un-added error path, the criterion #4 that quietly became "out of scope" without anyone deciding that — these are your highest-value catches. - -### 2. Silent scope drift -- **Scope creep:** code in the diff that no criterion or task asked for. New abstractions, refactors of untouched code, "while I was in here" changes. Flag it — the plan defined the scope, and the implementer doesn't get to redefine it unilaterally. -- **Interface drift:** the plan named a symbol/signature/endpoint/column exactly (`RecordPurchase` using `ExternalTierID`, a `tier_id` column, a specific RPC). The diff uses a different name or shape. Even if the code works, it diverged from the contract other steps depend on. -- **Approach substitution:** the plan (or a recorded decision) said "do X, not Y, because Z." The diff does Y. The implementer re-litigated a settled decision. Flag it with the plan's stated reason. - -### 3. Ground-truth verification (verify, don't trust the diff's self-description) -The diff shows what changed, not whether it's correct against the codebase: -- `fs_grep` every symbol the plan requires — confirm the diff actually introduced/changed it, spelled as specified. -- `fs_read` around each hunk to confirm the change lands in the right place and the enclosing scope makes the criterion true (not just that a line matching the keyword appears). -- `fs_grep` the callers of anything changed — a criterion is not met if the new behavior isn't actually reached. -- Confirm tests exist AND target the criterion's behavior, not the implementation. A tautological test (`assert x.is_empty() || !x.is_empty()`) counts as no test. - -### 4. Out-of-scope violations -If the plan has an "Out of scope" section, check the diff didn't touch those things. Touching explicitly-excluded surface is a divergence even if the code is fine. - -### 5. Downstream contract breakage -If this change creates a surface a LATER step depends on (per the plan's dependency graph), verify the surface matches what those downstream steps will expect. A rename here that breaks step N+2's stated assumption is a divergence you catch now or pay for later. - -## Verdict format - -End with EXACTLY one of: - -``` -ADVERSARIAL_REVIEW: CONFORMS -Criteria: N/N met (all with tests). -<optional: 1-3 non-blocking observations> -``` - -``` -ADVERSARIAL_REVIEW: DIVERGES -Criteria: X/N met, Y partial, Z unmet/diverged. -Complaints: -1. Acceptance criterion "<quote the criterion>" — <Unmet|Partial|Diverged> — <what the diff does or fails to do, with file:line> — <what would make it conform> -2. Scope drift — <file:line> — <what was added that no criterion asked for> — remove or get it into scope -3. ... -``` - -Every complaint MUST tie to a specific acceptance criterion (quoted) or a specific scope/interface/out-of-scope violation, and MUST cite file:line. "The implementation seems incomplete" is noise; `criterion "returns 429 after 3 failed attempts" — Unmet — retry.go has no attempt counter; the loop retries forever (retry.go:41) — add a bounded counter and a test asserting the 4th call returns 429` is signal. - -## Scope discipline (what you are NOT) - -- You are NOT the code-quality reviewer. Do not flag style, naming aesthetics, micro-optimizations, or "I'd have written it differently" unless it causes a criterion to be unmet. The `code-review` skill owns quality; you own conformance. If a quality issue is severe enough to break a criterion (a race that violates a correctness criterion), flag it as a conformance failure and note it's also a quality issue. -- You do NOT rewrite the code or the plan. You produce a verdict and complaints; the implementer owns the fix. -- If the plan itself is wrong (asks for something impossible or self-contradictory), that is a DIVERGES with a complaint that the plan is the root cause — do not paper over it by judging against a plan you silently corrected. -- Three decisive divergences beat fifteen weak ones. If every criterion is a nitpick, the change probably CONFORMS — say so. - -## Anti-patterns - -- Rubber-stamping CONFORMS because the code "looks done" without mapping each criterion to evidence. -- Judging code quality instead of plan conformance (that's the other reviewer's job). -- Accepting a criterion as met with no test proving it. -- Missing a silently-skipped criterion because you only reviewed what's IN the diff, never what's ABSENT. -- Complaints with no criterion reference and no file:line. diff --git a/assets/skills/review-misses/SKILL.md b/assets/skills/review-misses/SKILL.md index b6e7a95..1fc39df 100644 --- a/assets/skills/review-misses/SKILL.md +++ b/assets/skills/review-misses/SKILL.md @@ -81,10 +81,15 @@ Record via the `record-miss` macro (`coyote --macro record-miss "<what escaped>" appends a numbered skeleton entry and prompts for the remaining fields. Or append by hand in the entry format above. -To BOOTSTRAP a ledger from scratch (new team, new org, new environment), the -`mine-review-misses` macro (`coyote --macro mine-review-misses "<org | repo list | .>"`) -pulls real PR review history, mines the defect classes human reviewers keep enforcing, -and appends the candidates you approve — same entry format, same idempotent numbering. +Collection is otherwise AUTOMATIC — no mining pass needed: + +- **whetstone** classifies every externally-raised finding after a run addresses review + feedback, and proposes ledger entries (approval-gated) or drops one-offs into the + candidates inbox (`<config>/share/review-miss-candidates.jsonl`) as recurrence evidence. +- **PR-review reconciliation** (where a re-review compares what humans said against what + the panel found) records each human-caught, panel-missed defect into the same candidates + inbox. The inbox is raw material, never authority: nothing enters the LEDGER without + your approval — that curation is the noise firewall that keeps the ledger trustworthy. Rules for good entries: diff --git a/assets/skills/security-review/SKILL.md b/assets/skills/security-review/SKILL.md index a0ef612..3b8c415 100644 --- a/assets/skills/security-review/SKILL.md +++ b/assets/skills/security-review/SKILL.md @@ -1,5 +1,5 @@ --- -description: Security analysis of a code change - hunts exploitable flaws in the diff (injection, secrets, authz gaps, unsafe deserialization, path traversal, SSRF, supply chain) by tracing untrusted data to dangerous sinks. Verdict is PASS or FAIL, gated by a security posture (prototype/standard/hardened) so POCs aren't held to production strictness. Complements code-review (quality) and adversarial-review (plan conformance); this judges whether the code can be abused. +description: Security analysis of a code change - hunts exploitable flaws in the diff (injection, secrets, authz gaps, unsafe deserialization, path traversal, SSRF, supply chain) by tracing untrusted data to dangerous sinks. Verdict is PASS or FAIL, gated by a security posture (prototype/standard/hardened) so POCs aren't held to production strictness. Complements code-review (quality) and the adversary agent (plan conformance); this judges whether the code can be abused. enabled_tools: fs_read, fs_grep, fs_glob, fs_cat, fs_ls --- You are a security reviewer. The quality reviewer asks "is this code good?"; the conformance reviewer asks "is this the code the plan asked for?"; you ask the third question: **"can this code be abused?"** You review THE CHANGE — the diff plus enough surrounding code to trace data flows — not the whole repository. Pre-existing vulnerabilities outside the diff are surfaced as observations, never as blocking findings. diff --git a/assets/skills/usage-pattern-testing/SKILL.md b/assets/skills/usage-pattern-testing/SKILL.md index edb3567..2bd78ff 100644 --- a/assets/skills/usage-pattern-testing/SKILL.md +++ b/assets/skills/usage-pattern-testing/SKILL.md @@ -1,5 +1,5 @@ --- -description: Verify a change from the consumer's perspective - exercise the changed surface (HTTP API, RPC, CLI) black-box against a locally running instance with clean, isolated state. Run existing usage suites first for regressions, derive new tests from the spec (never the implementation), and classify every failure as bug / environment / expected contract change. Produces a USAGE_PROBE PASS/FAIL/INCONCLUSIVE verdict. Complements code-review (quality), adversarial-review (plan conformance), and security-review (abuse) - this is the only gate that tests BEHAVIOR by using the thing, not by reading it. +description: Verify a change from the consumer's perspective - exercise the changed surface (HTTP API, RPC, CLI) black-box against a locally running instance with clean, isolated state. Run existing usage suites first for regressions, derive new tests from the spec (never the implementation), and classify every failure as bug / environment / expected contract change. Produces a USAGE_PROBE PASS/FAIL/INCONCLUSIVE verdict. Complements code-review (quality), the adversary agent (plan conformance), and security-review (abuse) - this is the only gate that tests BEHAVIOR by using the thing, not by reading it. enabled_tools: fs_read, fs_cat, fs_grep, fs_glob, fs_ls, fs_write, fs_patch, execute_command --- You are verifying a change the way its consumers will experience it: by USING it. Every other