diff --git a/assets/agents/adversary/scripts/run_checks.py b/assets/agents/adversary/scripts/run_checks.py index f39d597..fa6dd45 100644 --- a/assets/agents/adversary/scripts/run_checks.py +++ b/assets/agents/adversary/scripts/run_checks.py @@ -25,6 +25,11 @@ 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). + +Verification commands run with GRAPH_STATE* (GRAPH_STATE, GRAPH_STATE_FILE) +scrubbed from their environment: the reviewed repo's own tests may exec these very +scripts with their own GRAPH_STATE, and load_state() prefers +GRAPH_STATE_FILE, so an inherited live state file would silently win. """ import json @@ -57,6 +62,7 @@ def run_one(cmd, proj, budget): """Run one command, hard-bounded by `budget` seconds; return its record.""" start = time.monotonic() popen_kwargs = {"start_new_session": True} if os.name == "posix" else {} + env = {k: v for k, v in os.environ.items() if not k.startswith("GRAPH_STATE")} p = subprocess.Popen( cmd, shell=True, @@ -64,6 +70,7 @@ def run_one(cmd, proj, budget): stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + env=env, **popen_kwargs, ) try: diff --git a/assets/agents/architecture-reviewer/config.yaml b/assets/agents/architecture-reviewer/config.yaml index 045a6ca..e0ff9b9 100644 --- a/assets/agents/architecture-reviewer/config.yaml +++ b/assets/agents/architecture-reviewer/config.yaml @@ -12,7 +12,11 @@ spawnable_agents: - explore - oracle max_concurrent_agents: 4 -max_agent_depth: 2 +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# architecture-reviewer(2) → {explore | oracle}(3). This agent spawns depth-3 +# children; 4 = one level of headroom. +max_agent_depth: 4 inject_spawn_instructions: true skills_enabled: true diff --git a/assets/agents/code-reviewer/graph.yaml b/assets/agents/code-reviewer/graph.yaml index e81516e..39a6623 100644 --- a/assets/agents/code-reviewer/graph.yaml +++ b/assets/agents/code-reviewer/graph.yaml @@ -18,7 +18,12 @@ description: | version: '1.0' can_spawn_agents: true -max_agent_depth: 3 +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# review-gauntlet | step-runner(2) → code-reviewer(3) → +# {domain-reviewer(4) → file-reviewer(5) | finding-verifier(4)}. +# This agent spawns depth-4 children; 5 = one level of headroom. +max_agent_depth: 5 max_concurrent_agents: 16 variables: diff --git a/assets/agents/domain-reviewer/config.yaml b/assets/agents/domain-reviewer/config.yaml index 8ec24cd..3c6f548 100644 --- a/assets/agents/domain-reviewer/config.yaml +++ b/assets/agents/domain-reviewer/config.yaml @@ -23,7 +23,13 @@ enabled_skills: spawnable_agents: - file-reviewer max_concurrent_agents: 8 -max_agent_depth: 2 +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# review-gauntlet | step-runner(2) → code-reviewer(3) → domain-reviewer(4) → +# file-reviewer(5). This agent spawns depth-5 children; 6 = one level of headroom. +# NOTE: can_spawn_agents is not yet set, so the file-reviewer hop is documented +# but inert; 6 keeps the limit forward-compatible for when it is wired. +max_agent_depth: 6 variables: - name: project_dir diff --git a/assets/agents/review-gauntlet/graph.yaml b/assets/agents/review-gauntlet/graph.yaml index a606d62..6960eb0 100644 --- a/assets/agents/review-gauntlet/graph.yaml +++ b/assets/agents/review-gauntlet/graph.yaml @@ -19,6 +19,11 @@ description: | version: '1.0' +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# review-gauntlet(2) → code-reviewer(3) → +# {domain-reviewer(4) → file-reviewer(5) | finding-verifier(4)}. +# This agent spawns depth-3 children; 4 = one level of headroom. max_agent_depth: 4 max_concurrent_agents: 12 diff --git a/assets/agents/step-runner/graph.yaml b/assets/agents/step-runner/graph.yaml index d7ae2bf..8e8c447 100644 --- a/assets/agents/step-runner/graph.yaml +++ b/assets/agents/step-runner/graph.yaml @@ -6,6 +6,12 @@ description: | review -> evidence-backed handoff -> user approval gate. Designed to be delegated to by sisyphus. version: '1.0' +# The limit is consulted by the SPAWNER against the child's absolute depth +# (root = 0). Deepest documented chain: architect(0) → sisyphus(1) → +# step-runner(2) → code-reviewer(3) → +# {domain-reviewer(4) → file-reviewer(5) | finding-verifier(4)}. +# This agent spawns depth-3 children; 4 = one level of headroom (default 3 had none). +max_agent_depth: 4 global_tools: - ast_grep.sh - fs_cat.sh diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 13cb9b3..c7e45a2 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -8223,6 +8223,102 @@ mod tests { ); } + // The spawner's own `max_agent_depth` is checked against the child's + // absolute depth (root = 0), so every hop along a chain needs its own + // limit >= child depth; +1 leaves one level of headroom. + #[test] + #[serial] + fn bundled_review_suite_depth_limits_cover_documented_chain() { + use crate::graph::GraphParser; + + fn bundled_agent_config(name: &str) -> AgentConfig { + let dir = paths::agents_data_dir().join(name); + let graph_path = dir.join("graph.yaml"); + if graph_path.exists() { + let graph = GraphParser::new(&dir) + .load_from_file(&graph_path) + .unwrap_or_else(|e| panic!("graph.yaml for '{name}' failed to parse: {e}")); + return AgentConfig::from_graph(name, &graph); + } + AgentConfig::load(&dir.join("config.yaml")) + .unwrap_or_else(|e| panic!("config.yaml for '{name}' failed to load: {e}")) + } + + let _guard = TestConfigDirGuard::new(); + Agent::install_builtin_agents(false).unwrap(); + + for (name, expected) in [ + ("architect", 10), + ("sisyphus", 5), + ("review-gauntlet", 4), + ("step-runner", 4), + ("code-reviewer", 5), + ("domain-reviewer", 6), + ("architecture-reviewer", 4), + ] { + assert_eq!( + bundled_agent_config(name).max_agent_depth, + expected, + "bundled '{name}' max_agent_depth drifted" + ); + } + + // User decision: domain-reviewer's file-reviewer fan-out stays unwired + // for now. Remove it from this list when `can_spawn_agents: true` is added. + const DOCUMENTED_BUT_UNWIRED: &[&str] = &["domain-reviewer"]; + + let chains: [&[&str]; 4] = [ + &[ + "architect", + "sisyphus", + "review-gauntlet", + "code-reviewer", + "domain-reviewer", + "file-reviewer", + ], + &[ + "architect", + "sisyphus", + "step-runner", + "code-reviewer", + "domain-reviewer", + "file-reviewer", + ], + &[ + "architect", + "sisyphus", + "review-gauntlet", + "code-reviewer", + "finding-verifier", + ], + &["architect", "sisyphus", "architecture-reviewer", "explore"], + ]; + for chain in chains { + for (i, pair) in chain.windows(2).enumerate() { + let (spawner, child) = (pair[0], pair[1]); + let cfg = bundled_agent_config(spawner); + assert!( + cfg.max_agent_depth >= i + 2, + "{spawner} (depth {i}) needs max_agent_depth >= {} to spawn {child} at depth {} with one level of headroom", + i + 2, + i + 1 + ); + if !DOCUMENTED_BUT_UNWIRED.contains(&spawner) { + assert!( + cfg.can_spawn_agents, + "{spawner} must have can_spawn_agents to reach {child}" + ); + } + if let Some(list) = &cfg.spawnable_agents { + assert!( + list.iter().any(|s| s == child), + "{spawner} spawnable_agents must include {child}, got {list:?}" + ); + } + } + } + } + #[test] #[serial] fn bundled_graph_agents_parse_and_validate() { @@ -8592,6 +8688,53 @@ mod tests { ); } + #[test] + fn adversary_run_checks_scrubs_graph_state_from_verification_commands() { + if !cmd_available("python3") { + eprintln!("skipping: python3 not available"); + return; + } + let unique = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(); + let state_file = + env::temp_dir().join(format!("coyote-adversary-run-checks-state-{unique}.json")); + // python3 probe rather than `echo "${VAR:-UNSET}"`: run_checks.py uses + // shell=True, which is cmd.exe on the Windows CI leg. + let state = json!({ + "verification_commands": [ + "python3 -c \"import os; print('file=' + os.environ.get('GRAPH_STATE_FILE', 'UNSET') + ' inline=' + os.environ.get('GRAPH_STATE', 'UNSET'))\"" + ] + }); + write(&state_file, state.to_string()).unwrap(); + + // The script itself must load from GRAPH_STATE_FILE (the envs are + // applied after the helper's env_remove), so an inline dummy state + // proves the file-preferred path is the one exercised. + let out = run_adversary_script_env( + "run_checks.py", + &json!({}), + &[("GRAPH_STATE_FILE", state_file.to_str().unwrap())], + ); + let _ = std::fs::remove_file(&state_file); + + let results = out["exec_results"].as_array().unwrap(); + assert_eq!( + results.len(), + 1, + "state must come from the file: {results:?}" + ); + assert_eq!(results[0]["exit"], 0, "{results:?}"); + assert!( + results[0]["tail"] + .as_str() + .unwrap() + .contains("file=UNSET inline=UNSET"), + "verification command must not inherit GRAPH_STATE*: {results:?}" + ); + } + #[test] fn adversary_run_checks_runner_error_degrades_to_environment_marker() { if !cmd_available("python3") { @@ -9297,6 +9440,7 @@ mod tests { let out = std::process::Command::new("python3") .arg(&path) .env("GRAPH_STATE", state.to_string()) + .env_remove("GRAPH_STATE_FILE") .output() .unwrap_or_else(|e| panic!("failed to invoke python3 {script}: {e}")); assert!( @@ -9787,6 +9931,7 @@ mod tests { let out = std::process::Command::new("python3") .arg(&path) .env("GRAPH_STATE", raw_state) + .env_remove("GRAPH_STATE_FILE") .output() .unwrap_or_else(|e| panic!("failed to invoke python3 {script}: {e}")); assert!( @@ -10375,6 +10520,7 @@ mod tests { let out = std::process::Command::new("python3") .arg(&path) .env("GRAPH_STATE", raw_state) + .env_remove("GRAPH_STATE_FILE") .output() .unwrap_or_else(|e| panic!("failed to invoke python3 {script}: {e}")); assert!( @@ -10606,6 +10752,7 @@ mod tests { let out = std::process::Command::new("bash") .arg(&path) .env("GRAPH_STATE", state.to_string()) + .env_remove("GRAPH_STATE_FILE") .output() .unwrap_or_else(|e| panic!("failed to invoke bash {script}: {e}")); assert!( @@ -10908,6 +11055,7 @@ mod tests { let out = std::process::Command::new("python3") .arg(&path) .env("GRAPH_STATE", raw_state) + .env_remove("GRAPH_STATE_FILE") .output() .unwrap_or_else(|e| panic!("failed to invoke python3 {script}: {e}")); assert!(