diff --git a/assets/agents/adversary/graph.yaml b/assets/agents/adversary/graph.yaml index e267aee..8b962d9 100644 --- a/assets/agents/adversary/graph.yaml +++ b/assets/agents/adversary/graph.yaml @@ -15,8 +15,9 @@ description: | DIVERGES (fail-closed) naming the missing spec. Caller-declared verification commands are executed ONCE by a deterministic stage and recorded — criterion branches cite the record and never run anything themselves. Any pipeline - fault (parse, diff resolution, or holistic failure) lands a PIPELINE-FAULT - marker and fails closed into DIVERGES: degraded is never passing. + fault (parse, diff resolution, verification runner, or holistic failure) + lands a PIPELINE-FAULT marker and fails closed into DIVERGES: degraded is + never passing. version: '1.0' @@ -140,9 +141,10 @@ 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; never fails the node. + 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. script: scripts/run_checks.py timeout: 3600 + fallback: pipeline_fault next: map_criteria map_criteria: @@ -198,6 +200,9 @@ nodes: - "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"). + - 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. Output format — respond with ONLY this JSON object, no prose around it: {"id": "", @@ -325,7 +330,7 @@ nodes: pipeline_fault: id: pipeline_fault type: script - description: Fail-closed fault marker (reached only via fallback routes from parse/facts) — records a PIPELINE-FAULT in state and continues to verdict, which turns any fault into DIVERGES. The graph never dies without its sentinel. + description: Fail-closed fault marker (reached only via fallback routes from parse/facts/run_checks) — records a PIPELINE-FAULT in state and continues to verdict, which turns any fault into DIVERGES. The graph never dies without its sentinel. script: scripts/pipeline_fault.py timeout: 30 next: verdict diff --git a/assets/agents/adversary/scripts/pipeline_fault.py b/assets/agents/adversary/scripts/pipeline_fault.py index a16bfc7..dddabfb 100644 --- a/assets/agents/adversary/scripts/pipeline_fault.py +++ b/assets/agents/adversary/scripts/pipeline_fault.py @@ -1,15 +1,23 @@ #!/usr/bin/env python3 """Fail-closed fault marker for the adversary graph. -Reached ONLY via `fallback:` routes (parse, facts). Appends a machine-readable -"PIPELINE-FAULT:" marker to `pipeline_faults` and continues to `verdict`, -which turns any recorded fault into DIVERGES — the graph never dies without -its sentinel, and a degraded run can never read as CONFORMS. +Reached ONLY via `fallback:` routes (parse, facts, run_checks). Appends a +machine-readable "PIPELINE-FAULT:" marker to `pipeline_faults` and continues +to `verdict`, which turns any recorded fault into DIVERGES — the graph never +dies without its sentinel, and a degraded run can never read as CONFORMS. -Stage attribution: an llm-node failure writes an "LLM node …failed: " -string into `parse_failure` via parse's `state_updates`; anything else means -the deterministic facts (diff resolution) script failed, which leaves no -failure text in state by design. +Stage attribution — grounded in engine behavior: a SCRIPT node's fallback +route writes NOTHING into state (src/graph/executor.rs script arm routes to +the fallback without recording the error), so the two script stages are +discriminated by which stage outputs are present: +- parse (llm): its `state_updates` wrote an "LLM node …failed: " + string into `parse_failure` — attribute to parse. +- run_checks vs facts (scripts): diff_facts.py unconditionally writes a + NONEMPTY `diff_text` (even its internal-failure path writes + "(diff unavailable)"), while the graph's initial value is ''. A nonempty + `diff_text` proves facts completed, so the fault is the verification + runner (run_checks killed at the node level, e.g. by the node timeout); + an empty `diff_text` means facts itself died. """ import json @@ -30,11 +38,19 @@ def main(): faults = [f for f in (state.get("pipeline_faults") or []) if isinstance(f, str)] parse_failure = state.get("parse_failure") detail = parse_failure.strip() if isinstance(parse_failure, str) else "" + diff_text = state.get("diff_text") + facts_completed = isinstance(diff_text, str) and bool(diff_text.strip()) if detail.startswith("LLM node"): detail = detail.replace("\n", " ") if len(detail) > MAX_DETAIL_CHARS: detail = detail[: MAX_DETAIL_CHARS - 1] + "…" faults.append(f"PIPELINE-FAULT: parse failed — cannot review: {detail}") + elif facts_completed: + faults.append( + "PIPELINE-FAULT: verification runner killed — the run_checks stage " + "died at the node level before recording exec_results; execution " + "criteria are unproven" + ) else: faults.append( "PIPELINE-FAULT: diff resolution failed — changed files and diff text " diff --git a/assets/agents/adversary/scripts/run_checks.py b/assets/agents/adversary/scripts/run_checks.py index c82e225..f39d597 100644 --- a/assets/agents/adversary/scripts/run_checks.py +++ b/assets/agents/adversary/scripts/run_checks.py @@ -9,14 +9,32 @@ they never execute anything themselves. NO auto-detection: undeclared commands mean a "none declared" marker (fail-visible, not fail-guessed). Any runner error degrades into an ENVIRONMENT marker — this script never fails the node. + +Budget: commands run sequentially, so TOTAL runtime is bounded by a deadline +(TOTAL_DEADLINE_SECS = 3300s, the run_checks node's 3600s timeout minus +margin). Without it, a handful of hanging commands (4 x 900s) would blow the +NODE timeout and the executor would kill this script from OUTSIDE — the +in-script degradation would never fire. Each command gets +min(PER_COMMAND_TIMEOUT_SECS, remaining budget); commands the deadline +leaves no budget for are recorded as {"cmd", "skipped": "deadline"} entries, +so the in-script handling stays authoritative. The env var +ADVERSARY_RUN_CHECKS_DEADLINE_SECS overrides the deadline (test seam). + +Process hygiene: commands run through the shell. On POSIX each command gets +its own session (start_new_session) and a timeout SIGKILLs the whole process +group, so grandchildren of a hung command cannot linger. On non-POSIX +platforms only the direct shell child is killed — grandchildren of a +timed-out command may survive (documented limitation). """ import json import os +import signal import subprocess import time PER_COMMAND_TIMEOUT_SECS = 900 +TOTAL_DEADLINE_SECS = float(os.environ.get("ADVERSARY_RUN_CHECKS_DEADLINE_SECS") or 3300) TAIL_LINES = 50 MAX_TAIL_CHARS = 8000 @@ -35,6 +53,45 @@ def tail_of(text): return tail +def run_one(cmd, proj, budget): + """Run one command, hard-bounded by `budget` seconds; return its record.""" + start = time.monotonic() + popen_kwargs = {"start_new_session": True} if os.name == "posix" else {} + p = subprocess.Popen( + cmd, + shell=True, + cwd=proj, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + **popen_kwargs, + ) + try: + out_s, err_s = p.communicate(timeout=budget) + except subprocess.TimeoutExpired: + if os.name == "posix": + try: + os.killpg(os.getpgid(p.pid), signal.SIGKILL) + except (ProcessLookupError, PermissionError): + pass + else: + p.kill() + p.communicate() # reap the child and drain the pipes + return { + "cmd": cmd, + "exit": -1, + "duration_s": round(time.monotonic() - start, 1), + "tail": f"TIMEOUT: no exit within {budget:.0f}s — recorded as a failing run", + } + combined = (out_s or "") + (("\n" + err_s) if err_s else "") + return { + "cmd": cmd, + "exit": p.returncode, + "duration_s": round(time.monotonic() - start, 1), + "tail": tail_of(combined), + } + + def main(): state = load_state() declared = state.get("verification_commands") @@ -61,39 +118,14 @@ def main(): proj = os.path.expanduser( (state.get("project_dir") or "").strip() or "." ) + deadline = time.monotonic() + TOTAL_DEADLINE_SECS results = [] for cmd in cmds: - start = time.monotonic() - try: - r = subprocess.run( - cmd, - shell=True, - cwd=proj, - capture_output=True, - text=True, - timeout=PER_COMMAND_TIMEOUT_SECS, - ) - combined = (r.stdout or "") + (("\n" + r.stderr) if r.stderr else "") - results.append( - { - "cmd": cmd, - "exit": r.returncode, - "duration_s": round(time.monotonic() - start, 1), - "tail": tail_of(combined), - } - ) - except subprocess.TimeoutExpired: - results.append( - { - "cmd": cmd, - "exit": -1, - "duration_s": round(time.monotonic() - start, 1), - "tail": ( - f"TIMEOUT: no exit within {PER_COMMAND_TIMEOUT_SECS}s — " - "recorded as a failing run" - ), - } - ) + remaining = deadline - time.monotonic() + if remaining <= 0: + results.append({"cmd": cmd, "skipped": "deadline"}) + continue + results.append(run_one(cmd, proj, min(PER_COMMAND_TIMEOUT_SECS, remaining))) print(json.dumps({"exec_results": results})) diff --git a/assets/agents/adversary/scripts/verdict.py b/assets/agents/adversary/scripts/verdict.py index 524baca..127ae28 100644 --- a/assets/agents/adversary/scripts/verdict.py +++ b/assets/agents/adversary/scripts/verdict.py @@ -73,15 +73,22 @@ def append_criterion_complaints(lines, verdicts, extra, start): def render_exec_results(exec_results): """Lines for the "Verification runs:" report section. - exec_results is a list of {cmd, exit, duration_s, tail} records from - run_checks.py, or a string marker ("none declared —…" / "ENVIRONMENT: …"), - or empty when the run never reached the verification stage. + exec_results is a list of {cmd, exit, duration_s, tail} records (or + {cmd, skipped} for commands the runner's total deadline left no budget + for) from run_checks.py, or a string marker ("none declared —…" / + "ENVIRONMENT: …"), or empty when the run never reached the stage. """ if isinstance(exec_results, list): out = [] for r in exec_results: if not isinstance(r, dict): continue + if r.get("skipped"): + out.append( + f"- [SKIPPED] `{r.get('cmd', '?')}` — never ran: the runner's " + "total deadline expired first; covered criteria are unproven" + ) + continue status = "PASS" if r.get("exit") == 0 else "FAIL" out.append( f"- [{status}] `{r.get('cmd', '?')}` — exit {r.get('exit', '?')}, " diff --git a/src/config/request_context.rs b/src/config/request_context.rs index be469cd..ee87eea 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -8360,12 +8360,23 @@ mod tests { } fn run_adversary_script(script: &str, state: &serde_json::Value) -> serde_json::Value { + run_adversary_script_env(script, state, &[]) + } + + fn run_adversary_script_env( + script: &str, + state: &serde_json::Value, + envs: &[(&str, &str)], + ) -> serde_json::Value { let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) .join("assets/agents/adversary/scripts") .join(script); - let out = std::process::Command::new("python3") - .arg(&path) - .env("GRAPH_STATE", state.to_string()) + let mut cmd = std::process::Command::new("python3"); + cmd.arg(&path).env("GRAPH_STATE", state.to_string()); + for (k, v) in envs { + cmd.env(k, v); + } + let out = cmd .output() .unwrap_or_else(|e| panic!("failed to invoke python3 {script}: {e}")); assert!( @@ -8506,7 +8517,11 @@ mod tests { let state = json!({"verification_commands": ["echo ok", "false"]}); let out = run_adversary_script("run_checks.py", &state); let results = out["exec_results"].as_array().unwrap(); - assert_eq!(results.len(), 2, "one record per declared command: {results:?}"); + assert_eq!( + results.len(), + 2, + "one record per declared command: {results:?}" + ); assert_eq!(results[0]["cmd"], "echo ok"); assert_eq!(results[0]["exit"], 0, "green command must record exit 0"); assert!( @@ -8520,6 +8535,180 @@ mod tests { ); } + #[test] + fn adversary_run_checks_runner_error_degrades_to_environment_marker() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + // A dead project_dir makes every spawn raise — the outer except must + // still exit 0 (asserted inside the helper) and degrade the whole run + // to the ENVIRONMENT marker instead of failing the node. + let state = json!({ + "verification_commands": ["echo hi"], + "project_dir": "/nonexistent/xyz" + }); + let out = run_adversary_script("run_checks.py", &state); + let marker = out["exec_results"].as_str().unwrap(); + assert!( + marker.starts_with("ENVIRONMENT"), + "a runner error must degrade to an ENVIRONMENT marker: {marker}" + ); + } + + #[test] + fn adversary_run_checks_deadline_skips_remaining_commands() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + // Deadline seam: a 1s total budget. The hung first command must be cut + // off at min(per-command, remaining)≈1s and recorded as a timeout; the + // second must be recorded as skipped — the in-script handling stays + // authoritative instead of the NODE timeout killing the script from + // outside (which would bypass the ENVIRONMENT degradation entirely). + let state = json!({"verification_commands": ["sleep 5", "echo never"]}); + let out = run_adversary_script_env( + "run_checks.py", + &state, + &[("ADVERSARY_RUN_CHECKS_DEADLINE_SECS", "1")], + ); + let results = out["exec_results"].as_array().unwrap(); + assert_eq!( + results.len(), + 2, + "every declared command gets a record: {results:?}" + ); + assert_eq!( + results[0]["exit"], -1, + "a command outliving its budget must record a timeout: {results:?}" + ); + assert!( + results[0]["tail"].as_str().unwrap().contains("TIMEOUT"), + "the timeout must be named in the tail: {results:?}" + ); + assert_eq!( + results[1]["skipped"], "deadline", + "commands past the deadline must be recorded as skipped: {results:?}" + ); + } + + #[test] + fn adversary_pipeline_fault_parse_stage_attribution() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + let out = run_adversary_script( + "pipeline_fault.py", + &json!({"parse_failure": "LLM node 'parse' failed: provider exploded"}), + ); + let faults = out["pipeline_faults"].as_array().unwrap(); + let fault = faults[0].as_str().unwrap(); + assert!( + fault.contains("PIPELINE-FAULT: parse failed — cannot review"), + "an llm-node failure string must attribute to the parse stage: {fault}" + ); + assert!( + fault.contains("provider exploded"), + "the failure detail must be carried into the fault: {fault}" + ); + } + + #[test] + fn adversary_pipeline_fault_facts_and_run_checks_stage_attribution() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + // A script node's fallback writes NOTHING into state (executor.rs + // script arm), so attribution keys off which stage outputs are + // present. facts stage: parse succeeded (schema JSON in + // parse_failure) but diff_text is still the initial '' — diff + // resolution itself died. + let out = run_adversary_script( + "pipeline_fault.py", + &json!({"parse_failure": "{\"criteria\": []}", "diff_text": ""}), + ); + let faults = out["pipeline_faults"].as_array().unwrap(); + assert!( + faults[0] + .as_str() + .unwrap() + .contains("PIPELINE-FAULT: diff resolution failed"), + "empty diff_text must attribute to the facts stage: {faults:?}" + ); + + // run_checks stage: facts completed (diff_facts.py unconditionally + // writes a nonempty diff_text) but the runner died at the node level + // before recording exec_results. + let out = run_adversary_script( + "pipeline_fault.py", + &json!({ + "parse_failure": "{\"criteria\": []}", + "diff_text": "diff --git a/x b/x", + "exec_results": "" + }), + ); + let faults = out["pipeline_faults"].as_array().unwrap(); + assert!( + faults[0] + .as_str() + .unwrap() + .contains("PIPELINE-FAULT: verification runner killed"), + "nonempty diff_text must attribute to the run_checks stage: {faults:?}" + ); + } + + #[test] + fn adversary_check_criterion_prompt_cites_exec_results() { + use crate::graph::{GraphParser, NodeType}; + let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("assets/agents/adversary"); + let graph = GraphParser::new(&dir) + .load_from_file(dir.join("graph.yaml")) + .expect("adversary graph.yaml must parse"); + let node = graph + .nodes + .get("check_criterion") + .expect("adversary graph must have a check_criterion node"); + let NodeType::Llm(llm) = &node.node_type else { + panic!("check_criterion must be an llm node"); + }; + assert!( + llm.prompt.contains("{{exec_results}}"), + "check_criterion's prompt must cite the recorded verification runs: {}", + llm.prompt + ); + } + + #[test] + fn adversary_verdict_renders_skipped_runs() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + let state = json!({ + "pipeline_faults": [], + "crit_verdicts": [ + {"id": "c1", "text": "does X", "status": "MET", + "evidence": "src/x.rs:1 + test src/x.rs:99", "complaint": ""} + ], + "extra_complaints": [], + "observations": "", + "exec_results": [{"cmd": "cargo test --all", "skipped": "deadline"}] + }); + let out = run_adversary_script("verdict.py", &state); + let report = out["adv_report"].as_str().unwrap(); + assert!( + report.contains("- [SKIPPED] `cargo test --all`"), + "a skipped record must render as SKIPPED: {report}" + ); + assert!( + !report.contains("[FAIL] `cargo test --all`"), + "a skipped record must not render as FAIL: {report}" + ); + } + #[test] fn adversary_crit_gate_contract_regression() { if !cmd_available("python3") { @@ -8548,7 +8737,10 @@ mod tests { "crit_verdict": "not json at all" }), ); - assert_eq!(out["_next"], "check_criterion", "first failure must retry: {out}"); + assert_eq!( + out["_next"], "check_criterion", + "first failure must retry: {out}" + ); assert_eq!(out["gate_attempts"], 1); // Malformed output with the retry exhausted → recorded PARTIAL (unproven).