Dark-Alex-17/coyote
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(review-suite): pass verification_commands as a declared variable, never LLM-extracted from prose

Browse Source 
verification_commands — the shell commands the adversary's run_checks stage
executes — were extracted by the LLM parse node from {{initial_prompt}}, a
flat prompt that also carries untrusted plan/diff text pasted from the repo
under review. A planted '## VERIFICATION' block was indistinguishable from the
caller's declaration: prompt injection -> arbitrary command execution.

- adversary + review-gauntlet graphs declare verification_commands as a graph
  variable (default '[]'); the key leaves initial_state (or_insert seeding
  would otherwise shadow it) and leaves both parse nodes' instructions,
  output_schema.properties and required.
- review-gauntlet forwards it to run_adversary via a structured inputs:
  passthrough (lone {{verification_commands}} template), not lane-prompt prose.
- run_checks.py accepts a JSON list or a JSON-encoded string of one (variables
  arrive as strings); '[]'/[]/missing = none declared; any other shape fails
  closed with a PIPELINE-FAULT (nothing executed) that verdict.py turns into
  DIVERGES complaint #1.
- architect/sisyphus spawn templates and both READMEs document the
  --variables / --agent-variable form and the trust boundary. Known residual
  (llm-node auto-merge of undeclared output keys) is disclosed in the docs.
- Tests: list/JSON-string acceptance, fail-closed no-execution proof,
  prompt-prose injection proof, verdict wiring, asset pins for both graphs and
  orchestrator configs, dispatch-inputs > variable-override > default
  precedence.
This commit is contained in:
Dark-Alex-17 authored and Dark-Alex-17 committed 2026-09-14 10:50:34 -06:00
1 parent 00c3b60c2f
commit 300c7aeff0
9 files changed
+501 -47
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

assets/agents/adversary/README.md
+29 -8
View File
@@ -66,7 +66,8 @@ violation) and cites `file:line`. Vague complaints are not emitted.
```mermaid
flowchart TD
A["parse (llm): extract acceptance criteria + declared verification commands"] --> B["facts (diff_facts.py): resolve + bound the diff"]
A["parse (llm): extract acceptance criteria"] --> B["facts (diff_facts.py): resolve + bound the diff"]
V["verification_commands (declared graph variable — never parsed from the prompt)"] --> R
B --> R["run_checks (script): run the declared verification commands ONCE, record results"]
R --> C["map over criteria: one fresh-context branch each (parallel)"]
C --> D["check_criterion (llm + fs/ast tools)"]
@@ -88,7 +89,7 @@ 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. Execution evidence is recorded **once, deterministically**: `run_checks` runs ONLY the caller-declared verification commands (never invented or auto-detected), sequentially under a 3300s total deadline — commands the deadline leaves no budget for are recorded as SKIPPED. Criterion branches have no execution tools by design: "tests pass"-style criteria are judged against the record — a recorded green run is MET citing it; no declared command ⇒ at best PARTIAL.
3. Execution evidence is recorded **once, deterministically**: `run_checks` runs ONLY the caller-declared verification commands (never invented or auto-detected), sequentially under a 3300s total deadline — commands the deadline leaves no budget for are recorded as SKIPPED. Criterion branches have no execution tools by design: "tests pass"-style criteria are judged against the record — a recorded green run is MET citing it; no declared command ⇒ at best PARTIAL. The commands arrive through the declared `verification_commands` graph variable (see [Declaring verification commands](#declaring-verification-commands)) — `parse` never extracts them from the prompt, so the prompt is no longer an extraction source for the commands.
4. Hunt adversarially for the **absent**: skipped criteria, scope creep, interface/approach substitution, out-of-scope touches, downstream contract breakage.
5. **Fail closed on pipeline faults**: if parse, diff resolution, or the verification runner dies, its fallback routes to `pipeline_fault`, which records a `PIPELINE-FAULT:` marker and continues to the verdict (a failed holistic pass falls back straight to the verdict, which synthesizes the same marker). Any recorded fault forces DIVERGES with the fault as complaint #1 — degraded is never passing, and the sentinel is always emitted: the graph never dies without one.
6. **Fail closed per criterion**: a criterion check that dies (max_iterations exhausted, API failure) falls back to `criterion_fault`, which records a schema-shaped UNMET verdict carrying a `PIPELINE-FAULT: criterion ` evidence marker for that one criterion. The other criteria still get verdicts, the report says the check DIED rather than judged, and the run is DIVERGES. That exact marker prefix (the one `criterion_fault` emits) forces the criterion into unmet regardless of the status it carries, so a verdict that echoes it with `MET` can never count toward CONFORMS; `check_criterion` is told never to begin its own evidence with `PIPELINE-FAULT:`.
@@ -98,7 +99,8 @@ It is **read-only** — it produces a verdict, never a fix.
## Usage
Typically spawned by `sisyphus` (or `architect`) alongside `code-reviewer`. The spawn prompt IS its
entire context, so it must include the diff (or a base ref to fetch) **and** the acceptance criteria:
entire context, so it must include the diff (or a base ref to fetch) **and** the acceptance criteria.
Verification commands travel separately, as a declared variable:
```sh
agent__spawn --agent adversary --prompt "
@@ -110,23 +112,42 @@ Run get_diff (or --base main), or: <paste diff>
## PLAN — acceptance criteria to check against
<paste the task index.md body + the relevant PLAN-*.md section, verbatim>
## VERIFICATION (optional)
<exact build/test/lint commands to run, one per line — undeclared commands are never guessed>
"
" --variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"}
```
Direct invocation for ad-hoc use:
```sh
coyote -a adversary --agent-variable project_dir /path/to/repo \
--agent-variable verification_commands '["cargo test --all"]' \
"Review staged changes against these criteria: <paste criteria>"
```
### Declaring verification commands
`verification_commands` is a declared graph variable (default `'[]'`): a JSON array of shell
commands, passed as a string because agent variables always arrive as strings. `run_checks` runs
exactly those commands with a shell — so the declaration is a **trust boundary**. The prompt is the
wrong side of it: it also carries plan and diff text pasted from the repo under review, and an LLM
`parse` node extracting commands from that prose would let anything in the repo's text run under
the reviewer's shell. Hence the commands are never in the prompt and never in `parse`'s output
schema; `review-gauntlet` forwards its own `verification_commands` variable to the adversary lane
as a structured `inputs:` passthrough, not as prompt text.
- `'[]'` (or omitted) ⇒ the "none declared" marker; execution criteria are at best PARTIAL.
- Anything that is not a JSON array of strings (unparsable JSON, an object, non-string items) is
an invalid declaration: `run_checks` executes **nothing** and records a
`PIPELINE-FAULT: verification_commands declaration invalid — …` marker, which forces DIVERGES —
fail-closed, not degraded to the soft ENVIRONMENT marker.
- Known residual: the engine merges every top-level key of `parse`'s JSON output into state, not
only the keys in its `output_schema`, so an injected instruction that makes the parse LLM emit an
extra `verification_commands` key is not yet blocked at the engine level. Closing that requires
restricting the llm-node auto-merge to `output_schema.properties` (tracked as a follow-up).
### Tools
- The graph's `diff_facts` script resolves the diff itself (staged → unstaged → `HEAD~1`, or an explicit ref/range named in the prompt) — there is no `get_diff` tool anymore.
- The `run_checks` script executes the caller-declared verification commands and records the results — criterion branches cite the record and never run anything themselves.
- The `run_checks` script executes the commands declared via the `verification_commands` variable and records the results — criterion branches cite the record and never run anything themselves.
- Criterion branches carry read-only `fs_read`/`fs_cat`/`fs_grep`/`ast_grep` for ground-truth checks.
## Related
assets/agents/adversary/graph.yaml
+11 -12
View File
@@ -31,6 +31,12 @@ variables:
- name: project_dir
description: Project directory containing the changes under review
default: '.'
- name: verification_commands
description: |
JSON array of shell commands the CALLER declares for the deterministic
run_checks stage — e.g. '["cargo test --all"]'. Never inferred from the
prompt; empty array = none declared.
default: '[]'
settings:
max_loop_iterations: 10
@@ -47,7 +53,6 @@ initial_state:
changed_files: []
diff_text: ''
diff_note: ''
verification_commands: []
exec_results: ''
crit_verdicts: []
extra_complaints: []
@@ -85,10 +90,6 @@ nodes:
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.
- `verification_commands`: the exact build/test/lint shell commands the
caller explicitly declares for verification (verbatim strings, in the
order given). Empty array when the caller declares none — NEVER invent,
guess, or auto-detect commands; undeclared stays undeclared.
prompt: |
Extract the conformance-review facts from this request:
@@ -117,11 +118,7 @@ nodes:
type: string
project_dir_in:
type: string
verification_commands:
type: array
items:
type: string
required: [criteria, diff_spec, plan_notes, scope_notes, extra_context, project_dir_in, verification_commands]
required: [criteria, diff_spec, plan_notes, scope_notes, extra_context, project_dir_in]
max_iterations: 3
max_attempts: 2
state_updates:
@@ -141,7 +138,7 @@ nodes:
run_checks:
id: run_checks
type: script
description: Deterministic execution evidence — runs ONLY the caller-declared verification commands in the project dir and records exec_results; criterion branches cite the record and never execute anything. Undeclared commands degrade to a "none declared" marker, runner errors to an ENVIRONMENT marker; a 3300s total deadline keeps the script under the node timeout (commands past it are recorded as skipped), and if the node is killed anyway the fallback records a PIPELINE-FAULT.
description: Deterministic execution evidence — runs ONLY the commands declared via the verification_commands variable in the project dir and records exec_results; criterion branches cite the record and never execute anything. Undeclared commands degrade to a "none declared" marker, runner errors to an ENVIRONMENT marker; a declaration that is not a JSON array of strings executes nothing and records a PIPELINE-FAULT (fail-closed); a 3300s total deadline keeps the script under the node timeout (commands past it are recorded as skipped), and if the node is killed anyway the fallback records a PIPELINE-FAULT.
script: scripts/run_checks.py
timeout: 3600
fallback: pipeline_fault
@@ -199,7 +196,9 @@ nodes:
recorded exit code and output tail.
- "none declared" or an ENVIRONMENT marker ⇒ at best PARTIAL, with an
ENVIRONMENT note in the complaint naming the exact command the caller
should declare via verification_commands (e.g. "cargo test --all").
should declare via the `verification_commands` variable (structured
input, e.g. "cargo test --all") — commands are never declared in the
prompt.
- a SKIPPED record (the runner's total deadline expired before that
command ran) ⇒ at best PARTIAL for the criteria it covers, same
treatment as an ENVIRONMENT marker: unproven, never failed.
assets/agents/adversary/scripts/run_checks.py
+52 -8
View File
@@ -10,6 +10,19 @@ NO auto-detection: undeclared commands mean a "none declared" marker
(fail-visible, not fail-guessed). Any runner error degrades into an
ENVIRONMENT marker — this script never fails the node.
Trust boundary: `verification_commands` is a DECLARED graph variable (or the
review-gauntlet's structured `inputs:` passthrough), never a field an LLM
extracts from the spawn prompt — that prose also carries plan/diff text pasted
from the repo under review, and whatever is declared here runs with
shell=True. Variables land in state as strings, so a JSON-encoded list is
accepted alongside a real list. A declaration that is not a JSON array of
strings is a caller contract violation, not a runner hiccup: it executes
NOTHING and records a PIPELINE-FAULT (fail-closed into DIVERGES) instead of
degrading to the soft ENVIRONMENT marker. Known residual: the engine merges
every top-level key of an llm node's JSON output into state, so a parse LLM
coaxed into emitting an extra `verification_commands` key could still
overwrite the declared value before this script runs.
Budget: commands run sequentially, so TOTAL runtime is bounded by a deadline
(TOTAL_DEADLINE_SECS = 3300s, the run_checks node's 3600s timeout minus
margin). Without it, a handful of hanging commands (4 x 900s) would blow the
@@ -51,6 +64,35 @@ def load_state():
return json.loads(os.environ.get("GRAPH_STATE", "{}"))
class InvalidDeclaration(ValueError):
pass
def declared_commands(declared):
"""Normalize the verification_commands declaration to a list of commands.
Accepts a JSON list or a JSON-encoded string of one; missing/None means
none declared. Anything else raises InvalidDeclaration.
"""
if declared is None:
return []
if isinstance(declared, str):
if not declared.strip():
raise InvalidDeclaration("empty string (declare '[]' for none)")
try:
declared = json.loads(declared)
except json.JSONDecodeError as e:
raise InvalidDeclaration(f"not valid JSON: {e}") from e
if not isinstance(declared, list):
raise InvalidDeclaration(
f"expected a JSON array of strings, got {type(declared).__name__}"
)
bad = [c for c in declared if not isinstance(c, str)]
if bad:
raise InvalidDeclaration(f"non-string item(s): {json.dumps(bad)}")
return [c.strip() for c in declared if c.strip()]
def tail_of(text):
tail = "\n".join(text.splitlines()[-TAIL_LINES:])
if len(tail) > MAX_TAIL_CHARS:
@@ -101,12 +143,14 @@ def run_one(cmd, proj, budget):
def main():
state = load_state()
declared = state.get("verification_commands")
cmds = (
[c.strip() for c in declared if isinstance(c, str) and c.strip()]
if isinstance(declared, list)
else []
)
try:
cmds = declared_commands(state.get("verification_commands"))
except InvalidDeclaration as e:
msg = f"PIPELINE-FAULT: verification_commands declaration invalid — {e}"
faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)]
faults.append(msg)
print(json.dumps({"exec_results": msg, "pipeline_faults": faults}))
return
if not cmds:
print(
json.dumps(
@@ -114,8 +158,8 @@ def main():
"exec_results": (
"none declared — the caller supplied no verification_commands; "
"execution criteria (build/test/lint) cannot be independently "
"confirmed. Declare the exact commands in the spawn prompt to "
"get recorded runs."
"confirmed. Declare the exact commands via the "
"verification_commands variable to get recorded runs."
)
}
)