From b81b876a24aecdbf08409d5bb99b7cbe2ee9b54a Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 10:50:28 +1000 Subject: [PATCH 01/12] Add locked shell environment and command dialect guard --- tests/test_shell_command_guard.py | 63 ++++++++++++ villani_code/project_memory.py | 1 + villani_code/shells.py | 166 ++++++++++++++++++++++++++++++ villani_code/state.py | 2 + villani_code/state_runtime.py | 26 +++-- villani_code/state_tooling.py | 1 + villani_code/tools.py | 56 ++++++++-- 7 files changed, 299 insertions(+), 16 deletions(-) create mode 100644 tests/test_shell_command_guard.py diff --git a/tests/test_shell_command_guard.py b/tests/test_shell_command_guard.py new file mode 100644 index 00000000..51d5727b --- /dev/null +++ b/tests/test_shell_command_guard.py @@ -0,0 +1,63 @@ +from __future__ import annotations + +import json +from pathlib import Path + +from villani_code.shells import classify_and_rewrite_command, detect_shell_environment +from villani_code.tools import execute_tool + + +def test_env_detection_returns_shell_family(tmp_path: Path) -> None: + env = detect_shell_environment(str(tmp_path)) + assert env.shell_family in {"cmd", "powershell", "bash", "zsh", "unknown"} + + +def test_cmd_rm_rewrites_to_del(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("rm foo.txt", "cmd") + assert decision.classification == "needs_rewrite" + assert decision.command == "del /q foo.txt" + + +def test_cmd_grep_rewrites_to_findstr(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("grep hello notes.txt", "cmd") + assert decision.classification == "needs_rewrite" + assert decision.command == 'findstr /n /c:"hello" notes.txt' + + +def test_cmd_tail_rewrites_to_powershell_get_content(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("tail -n 20 app.log", "cmd") + assert decision.classification == "needs_rewrite" + assert decision.command == 'powershell -NoProfile -Command "Get-Content \'app.log\' -Tail 20"' + + +def test_cmd_heredoc_blocked(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("cat < None: + decision = classify_and_rewrite_command("grep hello notes.txt", "powershell") + assert decision.classification == "needs_rewrite" + assert decision.command == 'Select-String -Pattern "hello" notes.txt' + + +def test_bash_native_commands_are_unchanged(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("tail -n 2 app.log", "bash") + assert decision.classification == "allowed" + assert decision.command == "tail -n 2 app.log" + + +def test_blocked_commands_produce_compact_structured_feedback(tmp_path: Path) -> None: + result = execute_tool( + "Bash", + {"command": "cat < dict[str, Any]: return asdict(self) diff --git a/villani_code/shells.py b/villani_code/shells.py index 2ba1c7d3..9d6647f0 100644 --- a/villani_code/shells.py +++ b/villani_code/shells.py @@ -1,5 +1,8 @@ from __future__ import annotations +import os +import re +from dataclasses import asdict, dataclass from enum import Enum from typing import Iterable @@ -11,6 +14,42 @@ class ShellFamily(str, Enum): _BASH_ONLY_PATTERNS = ("$?", "tail -", "2>/dev/null", "&& echo", "; echo") +OSFamily = str +LockedShellFamily = str + + +@dataclass(slots=True) +class ShellEnvironment: + os_family: OSFamily + shell_family: LockedShellFamily + shell_exe: str + cwd: str + + def to_dict(self) -> dict[str, str]: + return asdict(self) + + +@dataclass(slots=True) +class ShellCommandDecision: + classification: str + command: str + offending_pattern: str = "" + short_reason: str = "" + suggested_equivalent: str = "" + + def to_dict(self) -> dict[str, str]: + payload = { + "classification": self.classification, + "command": self.command, + } + if self.offending_pattern: + payload["offending_pattern"] = self.offending_pattern + if self.short_reason: + payload["short_reason"] = self.short_reason + if self.suggested_equivalent: + payload["suggested_equivalent"] = self.suggested_equivalent + return payload + def shell_family_for_platform(platform_name: str) -> ShellFamily: name = platform_name.lower() @@ -19,6 +58,133 @@ def shell_family_for_platform(platform_name: str) -> ShellFamily: return ShellFamily.POSIX +def detect_shell_environment(cwd: str) -> ShellEnvironment: + platform_name = os.sys.platform.lower() + if platform_name.startswith("win"): + os_family = "windows" + elif platform_name == "darwin": + os_family = "mac" + else: + os_family = "linux" + + shell_exe = os.environ.get("SHELL") or os.environ.get("COMSPEC") or os.environ.get("PSModulePath", "") + shell_exe = str(shell_exe or "").strip() + lowered_shell = shell_exe.lower() + if "powershell" in lowered_shell or "pwsh" in lowered_shell: + family = "powershell" + elif lowered_shell.endswith("cmd.exe") or "\\cmd.exe" in lowered_shell: + family = "cmd" + elif lowered_shell.endswith("zsh") or "/zsh" in lowered_shell: + family = "zsh" + elif lowered_shell.endswith("bash") or "/bash" in lowered_shell: + family = "bash" + elif os_family == "windows": + family = "cmd" + if not shell_exe: + shell_exe = "cmd.exe" + else: + family = "unknown" + return ShellEnvironment( + os_family=os_family, + shell_family=family, + shell_exe=shell_exe, + cwd=str(cwd), + ) + + +def classify_and_rewrite_command(command: str, shell_family: str) -> ShellCommandDecision: + raw = str(command or "").strip() + if not raw: + return ShellCommandDecision(classification="allowed", command=raw) + + if shell_family == "cmd": + if _has_bash_heredoc(raw): + return ShellCommandDecision( + classification="blocked", + command=raw, + offending_pattern="< bool: + return bool(re.search(r"<<\s*'?EOF'?", command, re.IGNORECASE)) + + +def _rewrite_for_cmd(command: str) -> str: + rm_match = re.match(r"^\s*rm\s+(.+)$", command) + if rm_match: + return f"del /q {rm_match.group(1).strip()}" + grep_match = re.match(r'^\s*grep\s+("([^"]+)"|\'([^\']+)\'|(\S+))\s+(.+)$', command) + if grep_match: + pattern = grep_match.group(2) or grep_match.group(3) or grep_match.group(4) or "" + file_part = grep_match.group(5).strip() + return f'findstr /n /c:"{pattern}" {file_part}' + tail_match = re.match(r"^\s*tail\s+-n\s+(\d+)\s+(.+)$", command) + if tail_match: + num = tail_match.group(1) + file_part = tail_match.group(2).strip().strip('"').strip("'") + return f'powershell -NoProfile -Command "Get-Content \'{file_part}\' -Tail {num}"' + return command + + +def _rewrite_for_powershell(command: str) -> str: + grep_match = re.match(r'^\s*grep\s+("([^"]+)"|\'([^\']+)\'|(\S+))\s+(.+)$', command) + if grep_match: + pattern = grep_match.group(2) or grep_match.group(3) or grep_match.group(4) or "" + file_part = grep_match.group(5).strip() + return f'Select-String -Pattern "{pattern}" {file_part}' + tail_match = re.match(r"^\s*tail\s+-n\s+(\d+)\s+(.+)$", command) + if tail_match: + num = tail_match.group(1) + file_part = tail_match.group(2).strip() + return f"Get-Content {file_part} -Tail {num}" + return command + + def normalize_command_for_shell(command: str, family: ShellFamily) -> str: normalized = command.strip() if family == ShellFamily.WINDOWS: diff --git a/villani_code/state.py b/villani_code/state.py index 1a2c80d4..fe4ab3c1 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -35,6 +35,7 @@ from villani_code.runtime_safety import ensure_runtime_dependencies_not_shadowed from villani_code.retrieval import Retriever from villani_code.skills import discover_skills +from villani_code.shells import detect_shell_environment from villani_code.streaming import StreamCoalescer, assemble_anthropic_stream from villani_code.tools import tool_specs from villani_code.transcripts import save_transcript @@ -538,6 +539,7 @@ def __init__( self._mission_state: MissionState | None = None self._event_recorder: RuntimeEventRecorder | None = None self._current_turn_index: int | None = None + self._shell_environment = detect_shell_environment(str(self.repo)).to_dict() if self.small_model: self._init_small_model_support() diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index 9e03824d..c7cc02ca 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -1204,6 +1204,12 @@ def _build_session_state_from_plan(instruction: str, plan: Any) -> SessionState: ) +def _with_shell_environment(runner: Any, session: SessionState) -> SessionState: + shell_env = dict(getattr(runner, "_shell_environment", {}) or {}) + session.shell_environment = {str(k): str(v) for k, v in shell_env.items() if str(k).strip()} + return session + + def ensure_project_memory_and_plan(runner: Any, instruction: str) -> None: if getattr(runner, "_planning_read_only", False): return @@ -1256,12 +1262,12 @@ def ensure_project_memory_and_plan(runner: Any, instruction: str) -> None: if runner.plan_mode == "off" or not plan.non_trivial: runner.event_callback({"type": "plan_auto_approved", "risk": plan.risk_level.value}) - update_session_state(runner.repo, session) + update_session_state(runner.repo, _with_shell_environment(runner, session)) return if runner.villani_mode: runner.event_callback({"type": "plan_auto_approved", "risk": plan.risk_level.value}) - update_session_state(runner.repo, session) + update_session_state(runner.repo, _with_shell_environment(runner, session)) return runner.event_callback({"type": "plan_approval_required", "risk": plan.risk_level.value}) @@ -1270,10 +1276,10 @@ def ensure_project_memory_and_plan(runner: Any, instruction: str) -> None: runner.event_callback({"type": "plan_rejected"}) session.outcome_status = "rejected" session.next_step_hints = ["Revise plan scope or lower risk before retrying"] - update_session_state(runner.repo, session) + update_session_state(runner.repo, _with_shell_environment(runner, session)) raise RuntimeError("Execution plan rejected by user.") runner.event_callback({"type": "plan_approved", "risk": plan.risk_level.value}) - update_session_state(runner.repo, session) + update_session_state(runner.repo, _with_shell_environment(runner, session)) @@ -1318,14 +1324,14 @@ def run_post_execution_validation(runner: Any, changed_files: list[str]) -> str: if result.passed: checkpoint = runner._context_governance.create_checkpoint(inventory, str(getattr(plan, "task_goal", "")), ["validation passed"]) runner.event_callback({"type": "context_checkpoint_created", "checkpoint_id": checkpoint.checkpoint_id, "reason": "validation_passed"}) - update_session_state(runner.repo, SessionState( + update_session_state(runner.repo, _with_shell_environment(runner, SessionState( affected_files=changed_files, validation_plan_summary=[s.step.name for s in result.plan.selected_steps], validation_summary="passed", outcome_status="success", next_step_hints=["Finalize and report output"], handoff_checkpoint="validation_passed", - )) + ))) if getattr(runner, "_mission_state", None) is not None: runner._mission_state.validation_failures = [] runner._mission_state.last_checkpoint_id = checkpoint.checkpoint_id @@ -1345,7 +1351,7 @@ def run_post_execution_validation(runner: Any, changed_files: list[str]) -> str: if outcome.recovered: checkpoint = runner._context_governance.create_checkpoint(inventory, str(getattr(plan, "task_goal", "")), ["validation passed after repair"]) runner.event_callback({"type": "context_checkpoint_created", "checkpoint_id": checkpoint.checkpoint_id, "reason": "repair_recovered"}) - update_session_state(runner.repo, SessionState( + update_session_state(runner.repo, _with_shell_environment(runner, SessionState( affected_files=changed_files, validation_plan_summary=[s.step.name for s in result.plan.selected_steps], validation_summary="passed after repair", @@ -1353,7 +1359,7 @@ def run_post_execution_validation(runner: Any, changed_files: list[str]) -> str: outcome_status="recovered", next_step_hints=["Report repaired validation and summarize edits"], handoff_checkpoint="repair_recovered", - )) + ))) if getattr(runner, "_mission_state", None) is not None: runner._mission_state.validation_failures = [] runner._mission_state.last_checkpoint_id = checkpoint.checkpoint_id @@ -1362,7 +1368,7 @@ def run_post_execution_validation(runner: Any, changed_files: list[str]) -> str: checkpoint = runner._context_governance.create_checkpoint(inventory, str(getattr(plan, "task_goal", "")), ["repair attempts exhausted"]) runner.event_callback({"type": "context_checkpoint_created", "checkpoint_id": checkpoint.checkpoint_id, "reason": "repair_exhausted"}) - update_session_state(runner.repo, SessionState( + update_session_state(runner.repo, _with_shell_environment(runner, SessionState( affected_files=changed_files, validation_plan_summary=[s.step.name for s in result.plan.selected_steps], validation_summary="failed", @@ -1371,7 +1377,7 @@ def run_post_execution_validation(runner: Any, changed_files: list[str]) -> str: outcome_status="failed", next_step_hints=["Inspect failing step and rerun with interactive guidance"], handoff_checkpoint="repair_exhausted", - )) + ))) if getattr(runner, "_mission_state", None) is not None: runner._mission_state.validation_failures = [result.failure_summary] runner._mission_state.last_failed_summary = outcome.message diff --git a/villani_code/state_tooling.py b/villani_code/state_tooling.py index c84c64ca..8c15710a 100644 --- a/villani_code/state_tooling.py +++ b/villani_code/state_tooling.py @@ -665,6 +665,7 @@ def _debug_callback_with_turn(event_type: str, payload: dict[str, Any]) -> None: unsafe=runner.unsafe, debug_callback=_debug_callback_with_turn, tool_call_id=stable_tool_use_id, + runtime_state={"shell_environment": dict(getattr(runner, "_shell_environment", {}))}, ) runner.event_callback( { diff --git a/villani_code/tools.py b/villani_code/tools.py index 423807ed..5c840980 100644 --- a/villani_code/tools.py +++ b/villani_code/tools.py @@ -17,6 +17,7 @@ extract_unified_diff_targets, parse_unified_diff, ) +from villani_code.shells import classify_and_rewrite_command, detect_shell_environment class LsInput(BaseModel): @@ -143,6 +144,7 @@ def execute_tool( unsafe: bool = False, debug_callback: Any | None = None, tool_call_id: str = "", + runtime_state: dict[str, Any] | None = None, ) -> dict[str, Any]: model = TOOL_MODELS.get(name) if not model: @@ -164,7 +166,14 @@ def execute_tool( if name == "Search": return _ok(_run_search(parsed, repo)) if name == "Bash": - return _ok(_run_bash(parsed, repo, unsafe=unsafe, debug_callback=debug_callback, tool_call_id=tool_call_id)) + return _run_bash( + parsed, + repo, + unsafe=unsafe, + debug_callback=debug_callback, + tool_call_id=tool_call_id, + runtime_state=runtime_state, + ) if name == "Write": return _ok(_run_write(parsed, repo, debug_callback=debug_callback, tool_call_id=tool_call_id)) if name == "Patch": @@ -235,21 +244,44 @@ def _run_search(data: SearchInput, repo: Path) -> str: return proc.stdout -def _run_bash(data: BashInput, repo: Path, unsafe: bool, debug_callback: Any | None = None, tool_call_id: str = "") -> str: +def _run_bash( + data: BashInput, + repo: Path, + unsafe: bool, + debug_callback: Any | None = None, + tool_call_id: str = "", + runtime_state: dict[str, Any] | None = None, +) -> dict[str, Any]: lowered = data.command.lower() if not unsafe: for bad in DENYLIST: if bad in lowered: raise ValueError(f"Refusing command: {bad.strip()}") + shell_state = (runtime_state or {}).get("shell_environment", {}) if isinstance(runtime_state, dict) else {} + shell_family = str(shell_state.get("shell_family", "")).strip() if isinstance(shell_state, dict) else "" + if not shell_family: + shell_family = detect_shell_environment(cwd=str(repo)).shell_family + decision = classify_and_rewrite_command(data.command, shell_family) + if decision.classification == "blocked": + blocked_payload = { + "shell_family": shell_family, + "classification": "blocked", + "offending_pattern": decision.offending_pattern, + "short_reason": decision.short_reason, + } + if decision.suggested_equivalent: + blocked_payload["suggested_equivalent"] = decision.suggested_equivalent + return {"content": json.dumps(blocked_payload, separators=(",", ":")), "is_error": True} + command_to_run = decision.command cwd = _safe_path(repo, data.cwd) if callable(debug_callback): - debug_callback("command_started", {"command": data.command, "cwd": data.cwd, "tool_call_id": tool_call_id}) - proc = subprocess.run(data.command, shell=True, cwd=str(cwd), capture_output=True, text=True, timeout=data.timeout_sec) + debug_callback("command_started", {"command": command_to_run, "cwd": data.cwd, "tool_call_id": tool_call_id}) + proc = subprocess.run(command_to_run, shell=True, cwd=str(cwd), capture_output=True, text=True, timeout=data.timeout_sec) if callable(debug_callback): debug_callback( "command_finished", { - "command": data.command, + "command": command_to_run, "cwd": data.cwd, "exit_code": proc.returncode, "stdout": proc.stdout, @@ -258,7 +290,19 @@ def _run_bash(data: BashInput, repo: Path, unsafe: bool, debug_callback: Any | N "tool_call_id": tool_call_id, }, ) - return json.dumps({"command": data.command, "exit_code": proc.returncode, "stdout": proc.stdout, "stderr": proc.stderr}, indent=2) + return _ok( + json.dumps( + { + "command": command_to_run, + "original_command": data.command, + "classification": decision.classification, + "exit_code": proc.returncode, + "stdout": proc.stdout, + "stderr": proc.stderr, + }, + indent=2, + ) + ) def _run_write(data: WriteInput, repo: Path, debug_callback: Any | None = None, tool_call_id: str = "") -> str: From ad2cb00b1cd44224c13544556be5fbe4401f0083 Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 11:09:25 +1000 Subject: [PATCH 02/12] Tighten cmd shell guard and add shell reminder --- tests/test_shell_command_guard.py | 30 ++++++++++++++++++++++++++ tests/test_state_runtime.py | 30 ++++++++++++++++++++++++++ villani_code/shells.py | 36 ++++++++++++++++++++++++++++++- villani_code/state_runtime.py | 27 +++++++++++++++++++++++ villani_code/tools.py | 5 ++++- 5 files changed, 126 insertions(+), 2 deletions(-) diff --git a/tests/test_shell_command_guard.py b/tests/test_shell_command_guard.py index 51d5727b..f94f2e76 100644 --- a/tests/test_shell_command_guard.py +++ b/tests/test_shell_command_guard.py @@ -36,6 +36,35 @@ def test_cmd_heredoc_blocked(tmp_path: Path) -> None: assert decision.offending_pattern == "< None: + decision = classify_and_rewrite_command("echo ok && type app.log | head -50", "cmd") + assert decision.classification == "blocked" + assert decision.offending_token == "head" + + +def test_cmd_head_n_rewrites_to_powershell_totalcount(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("head -n 20 app.log", "cmd") + assert decision.classification == "needs_rewrite" + assert decision.command == 'powershell -NoProfile -Command "Get-Content \'app.log\' -TotalCount 20"' + + +def test_cmd_head_default_rewrites_to_powershell_totalcount_10(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("head app.log", "cmd") + assert decision.classification == "needs_rewrite" + assert decision.command == 'powershell -NoProfile -Command "Get-Content \'app.log\' -TotalCount 10"' + + +def test_cmd_heredoc_anywhere_is_blocked(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("echo start && python - <<'EOF'\nprint(1)\nEOF", "cmd") + assert decision.classification == "blocked" + assert decision.offending_token == "<<" + + +def test_cmd_classifier_scans_full_command_string(tmp_path: Path) -> None: + decision = classify_and_rewrite_command("echo ok && dir | head -30", "cmd") + assert decision.classification == "blocked" + + def test_powershell_grep_rewrites_to_select_string(tmp_path: Path) -> None: decision = classify_and_rewrite_command("grep hello notes.txt", "powershell") assert decision.classification == "needs_rewrite" @@ -59,5 +88,6 @@ def test_blocked_commands_produce_compact_structured_feedback(tmp_path: Path) -> payload = json.loads(result["content"]) assert payload["shell_family"] == "cmd" assert payload["classification"] == "blocked" + assert payload["offending_token"] == "<<" assert payload["offending_pattern"] == "< None: self._retriever = _RetrieverStub() +class _ContextGovernanceStub: + def load_inventory(self): + return SimpleNamespace(task_id="") + + def register_item(self, *_args, **_kwargs) -> None: + return None + + def prune_for_budget(self, _inventory) -> None: + return None + + def save_inventory(self, _inventory) -> None: + return None + + def test_inject_retrieval_briefing_skips_tool_result_user_turn() -> None: runner = _RunnerStub() messages = [ @@ -103,6 +117,22 @@ def test_inject_retrieval_briefing_inserts_for_plain_text_user_turn() -> None: assert messages[0]["content"][1] == {"type": "text", "text": "Need context on runtime."} +def test_prepare_messages_for_model_injects_cmd_shell_reminder() -> None: + runner = SimpleNamespace( + small_model=False, + _context_budget=None, + _context_governance=_ContextGovernanceStub(), + _execution_plan=None, + _shell_environment={"shell_family": "cmd"}, + ) + messages = [{"role": "user", "content": [{"type": "text", "text": "check logs"}]}] + + prepared = state_runtime.prepare_messages_for_model(runner, messages) + + assert prepared[-1]["content"][0]["type"] == "text" + assert prepared[-1]["content"][0]["text"].startswith("Shell: Windows cmd.") + + def test_validate_anthropic_tool_sequence_rejects_text_after_tool_result() -> None: messages = [ { diff --git a/villani_code/shells.py b/villani_code/shells.py index 9d6647f0..066cc241 100644 --- a/villani_code/shells.py +++ b/villani_code/shells.py @@ -33,6 +33,7 @@ def to_dict(self) -> dict[str, str]: class ShellCommandDecision: classification: str command: str + offending_token: str = "" offending_pattern: str = "" short_reason: str = "" suggested_equivalent: str = "" @@ -42,6 +43,8 @@ def to_dict(self) -> dict[str, str]: "classification": self.classification, "command": self.command, } + if self.offending_token: + payload["offending_token"] = self.offending_token if self.offending_pattern: payload["offending_pattern"] = self.offending_pattern if self.short_reason: @@ -96,15 +99,35 @@ def classify_and_rewrite_command(command: str, shell_family: str) -> ShellComman raw = str(command or "").strip() if not raw: return ShellCommandDecision(classification="allowed", command=raw) + lowered = raw.lower() if shell_family == "cmd": if _has_bash_heredoc(raw): return ShellCommandDecision( classification="blocked", command=raw, + offending_token="<<", offending_pattern="< ShellComman def _has_bash_heredoc(command: str) -> bool: - return bool(re.search(r"<<\s*'?EOF'?", command, re.IGNORECASE)) + if "<<" in command: + return True + return bool(re.search(r"<<\s*'?\w+'?", command, re.IGNORECASE)) def _rewrite_for_cmd(command: str) -> str: @@ -168,6 +193,15 @@ def _rewrite_for_cmd(command: str) -> str: num = tail_match.group(1) file_part = tail_match.group(2).strip().strip('"').strip("'") return f'powershell -NoProfile -Command "Get-Content \'{file_part}\' -Tail {num}"' + head_n_match = re.match(r"^\s*head\s+-n\s+(\d+)\s+(.+)$", command) + if head_n_match: + num = head_n_match.group(1) + file_part = head_n_match.group(2).strip().strip('"').strip("'") + return f'powershell -NoProfile -Command "Get-Content \'{file_part}\' -TotalCount {num}"' + head_match = re.match(r"^\s*head\s+(.+)$", command) + if head_match: + file_part = head_match.group(1).strip().strip('"').strip("'") + return f'powershell -NoProfile -Command "Get-Content \'{file_part}\' -TotalCount 10"' return command diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index c7cc02ca..22d63c12 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -371,6 +371,7 @@ def inject_diagnosis_hint(messages: list[dict[str, Any]], diagnosis: dict[str, s def prepare_messages_for_model(runner: Any, messages: list[dict[str, Any]]) -> list[dict[str, Any]]: prepared = deepcopy(messages) + _inject_shell_reminder(runner, prepared) if runner.small_model: inject_retrieval_briefing(runner, prepared) if runner._context_budget: @@ -393,6 +394,32 @@ def prepare_messages_for_model(runner: Any, messages: list[dict[str, Any]]) -> l return prepared +def _inject_shell_reminder(runner: Any, messages: list[dict[str, Any]]) -> None: + if not messages: + return + shell_env = dict(getattr(runner, "_shell_environment", {}) or {}) + shell_family = str(shell_env.get("shell_family", "")).strip().lower() + if shell_family == "cmd": + reminder = "Shell: Windows cmd. Do not use Unix tools like head, grep, tail, rm, or heredocs." + elif shell_family == "powershell": + reminder = "Shell: Windows PowerShell. Avoid bash heredocs and bash-only syntax." + elif shell_family in {"bash", "zsh"}: + reminder = f"Shell: {shell_family}. Use POSIX shell syntax." + else: + return + last = messages[-1] + if last.get("role") != "user": + return + content = last.get("content", []) + if not isinstance(content, list): + return + if any(isinstance(block, dict) and block.get("type") == "tool_result" for block in content): + return + if any(isinstance(block, dict) and block.get("type") == "text" and str(block.get("text", "")).startswith("Shell: ") for block in content): + return + content.insert(0, {"type": "text", "text": reminder}) + + def validate_anthropic_tool_sequence(messages: list[dict[str, Any]]) -> None: for index, message in enumerate(messages): if message.get("role") != "assistant": diff --git a/villani_code/tools.py b/villani_code/tools.py index 5c840980..f0e74f36 100644 --- a/villani_code/tools.py +++ b/villani_code/tools.py @@ -266,9 +266,12 @@ def _run_bash( blocked_payload = { "shell_family": shell_family, "classification": "blocked", - "offending_pattern": decision.offending_pattern, "short_reason": decision.short_reason, } + if decision.offending_token: + blocked_payload["offending_token"] = decision.offending_token + if decision.offending_pattern: + blocked_payload["offending_pattern"] = decision.offending_pattern if decision.suggested_equivalent: blocked_payload["suggested_equivalent"] = decision.suggested_equivalent return {"content": json.dumps(blocked_payload, separators=(",", ":")), "is_error": True} From f49ed9de4ed3653472af02b84e62580e9e4b02ef Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 11:27:56 +1000 Subject: [PATCH 03/12] Enforce bounded recovery after hard validation failures --- tests/test_state_runtime.py | 103 +++++++++++++- tests/test_state_tooling_mutation_policy.py | 60 ++++++++ villani_code/state.py | 25 +++- villani_code/state_runtime.py | 149 ++++++++++++++++++++ villani_code/state_tooling.py | 14 +- 5 files changed, 339 insertions(+), 12 deletions(-) diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index 83ff45e0..c9b61cee 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -278,14 +278,103 @@ def test_repeated_validation_updates_compact_state_without_repeated_prose( first = runner._run_verification("edit") second = runner._run_verification("edit") - third = runner._run_verification("edit") - fourth = runner._run_verification("edit") - assert "validation_repeated_without_new_evidence: false" in first - assert "validation_repeated_without_new_evidence: true" in second - assert "status repeated without new validation evidence" in third - assert fourth == "" - assert runner._validation_repeated_without_new_evidence is True + +def test_syntax_error_does_not_mark_verification_pass(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + target = tmp_path / "legal_review_app.py" + target.write_text("print('x')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._current_verification_targets = {"legal_review_app.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["legal_review_app.py"]) + + def fake_run(cmd, **kwargs): + class P: + returncode = 1 + stdout = "" + stderr = 'Traceback (most recent call last):\n File "legal_review_app.py", line 2\nSyntaxError: invalid syntax\n' + return P() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert "status=fail" in runner._last_validation_summary + + +def test_name_error_does_not_mark_verification_pass(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + target = tmp_path / "legal_review_app.py" + target.write_text("print('x')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._current_verification_targets = {"legal_review_app.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["legal_review_app.py"]) + + def fake_run(cmd, **kwargs): + class P: + returncode = 1 + stdout = "" + stderr = 'Traceback (most recent call last):\n File "legal_review_app.py", line 4\nNameError: name x is not defined\n' + return P() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert "status=fail" in runner._last_validation_summary + + +def test_hard_failure_sets_recovery_mode_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + (tmp_path / "legal_review_app.py").write_text("print('x')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._current_verification_targets = {"legal_review_app.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["legal_review_app.py"]) + + def fake_run(cmd, **kwargs): + class P: + returncode = 1 + stdout = "" + stderr = 'Traceback (most recent call last):\n File "legal_review_app.py", line 7\nModuleNotFoundError: No module named y\n' + return P() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert runner._recovery_mode is True + assert runner._failing_file == "legal_review_app.py" + assert runner._failing_error_summary + + +def test_successful_rerun_clears_recovery_mode(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + (tmp_path / "legal_review_app.py").write_text("print('x')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._current_verification_targets = {"legal_review_app.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["legal_review_app.py"]) + + class FailProc: + returncode = 1 + stdout = "" + stderr = 'Traceback (most recent call last):\n File "legal_review_app.py", line 7\nImportError: bad import\n' + + class PassProc: + returncode = 0 + stdout = "ok" + stderr = "" + + runs = {"count": 0} + def fake_run(cmd, **kwargs): + rendered = " ".join(cmd) if isinstance(cmd, list) else str(cmd) + if rendered.startswith("git "): + return PassProc() + runs["count"] += 1 + return FailProc() if runs["count"] == 1 else PassProc() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert runner._recovery_mode is True + runner._run_verification("edit") + assert runner._recovery_mode is False def test_changed_validation_state_emits_new_compact_summary( diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index ad18ea3e..8d62979a 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -126,3 +126,63 @@ def test_patch_existing_file_rewrite_heavy_is_rejected(tmp_path: Path) -> None: result = execute_tool_with_policy(runner, "Patch", {"unified_diff": "\n".join(diff_lines)}, "1", 0) assert result["is_error"] is True assert "Rewrite-heavy mutation rejected" in str(result["content"]) + + +def test_recovery_mode_blocks_delete_of_active_failing_file(tmp_path: Path) -> None: + runner = _runner(tmp_path) + target = tmp_path / "legal_review_app.py" + target.write_text("print('x')\n", encoding="utf-8") + runner._recovery_mode = True + runner._failing_file = "legal_review_app.py" + runner._failing_error_summary = "SyntaxError" + runner._file_was_read_since_failure = True + + result = execute_tool_with_policy( + runner, + "Patch", + {"unified_diff": "--- a/legal_review_app.py\n+++ /dev/null\n@@ -1 +0,0 @@\n-print('x')\n"}, + "1", + 0, + ) + assert result["is_error"] is True + assert '"recovery_blocked": true' in str(result["content"]) + assert "delete_blocked_for_failing_file" in str(result["content"]) + + +def test_recovery_mode_blocks_edit_without_read_first(tmp_path: Path) -> None: + runner = _runner(tmp_path) + target = tmp_path / "legal_review_app.py" + target.write_text("print('x')\n", encoding="utf-8") + runner._recovery_mode = True + runner._failing_file = "legal_review_app.py" + runner._failing_error_summary = "NameError" + runner._file_was_read_since_failure = False + + result = execute_tool_with_policy( + runner, + "Write", + {"file_path": "legal_review_app.py", "content": "print('fixed')\n"}, + "1", + 0, + ) + assert result["is_error"] is True + assert "read_required_before_edit" in str(result["content"]) + + +def test_read_failing_file_flips_recovery_read_flag(tmp_path: Path) -> None: + runner = _runner(tmp_path) + (tmp_path / "legal_review_app.py").write_text("print('x')\n", encoding="utf-8") + runner._recovery_mode = True + runner._failing_file = "legal_review_app.py" + runner._failing_error_summary = "ImportError" + runner._file_was_read_since_failure = False + + result = execute_tool_with_policy( + runner, + "Read", + {"file_path": "legal_review_app.py", "max_bytes": 1000}, + "1", + 0, + ) + assert result["is_error"] is False + assert runner._file_was_read_since_failure is True diff --git a/villani_code/state.py b/villani_code/state.py index fe4ab3c1..958a6efc 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -529,6 +529,11 @@ def __init__( self._patch_sanity_retry_pending = False self._first_attempt_write_lock_active = False self._first_attempt_locked_target = "" + self._recovery_mode = False + self._failing_file = "" + self._failing_error_summary = "" + self._failing_command = "" + self._file_was_read_since_failure = False self._context_governance = ContextGovernanceManager(self.repo) self._planning_read_only = False self._runtime_mode: Literal["execution", "planning"] = "execution" @@ -903,6 +908,11 @@ def run( self._scope_expansion_used = False self._first_attempt_write_lock_active = bool(required_initial_read) self._first_attempt_locked_target = required_initial_read + self._recovery_mode = False + self._failing_file = "" + self._failing_error_summary = "" + self._failing_command = "" + self._file_was_read_since_failure = False if self._first_attempt_write_lock_active: self.event_callback( { @@ -1396,10 +1406,21 @@ def _budget_reason( tool_calls_used += 1 if self.small_model: result = self._truncate_tool_result(tool_name, result) - if tool_name == "Read" and not result.get("is_error"): - self._files_read.add(str(tool_input.get("file_path", ""))) + if tool_name == "Read" and not result.get("is_error"): + read_path = str(tool_input.get("file_path", "")).replace("\\", "/").lstrip("./") + self._files_read.add(read_path) + if ( + getattr(self, "_recovery_mode", False) + and read_path + and read_path == str(getattr(self, "_failing_file", "")) + ): + self._file_was_read_since_failure = True if tool_name in {"Write", "Patch"} and not result.get("is_error"): + if getattr(self, "_recovery_mode", False): + target_path = str(tool_input.get("file_path", "")).replace("\\", "/").lstrip("./") + if target_path and target_path == str(getattr(self, "_failing_file", "")): + self._file_was_read_since_failure = False self._pending_verification = self._run_post_edit_verification( trigger=f"{tool_name} execution" ) diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index 22d63c12..6dc2ea6c 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -12,6 +12,7 @@ from typing import Any from villani_code.autonomy import VerificationStatus +from villani_code.autonomy import FindingCategory, VerificationFinding from villani_code.indexing import DEFAULT_IGNORE, RepoIndex from villani_code.live_display import apply_live_display_delta from villani_code.planning import TaskMode, generate_execution_plan @@ -20,6 +21,7 @@ from villani_code.validation_loop import run_validation from villani_code.shells import baseline_import_validation_command, shell_family_for_platform from villani_code.repair import execute_repair_loop +from villani_code.patch_apply import PatchApplyError, extract_unified_diff_targets, parse_unified_diff from villani_code.repo_map import build_repo_map from villani_code.repo_rules import classify_repo_path, is_ignored_repo_path from villani_code.retrieval import Retriever @@ -532,7 +534,114 @@ def _is_strongly_adjacent_path(candidate: str, locked_paths: set[str]) -> bool: return False +def _recovery_blocked_feedback(reason: str, tool: str, path: str, detail: str = "") -> str: + payload = { + "recovery_blocked": True, + "reason": reason, + "tool": tool, + "path": path, + } + if detail: + payload["detail"] = detail + return json.dumps(payload, sort_keys=True) + + +def _patch_deletes_target_file(unified_diff: str, failing_file: str) -> bool: + try: + parsed = parse_unified_diff(unified_diff) + except PatchApplyError: + return False + for file_patch in parsed: + old_path = str(file_patch.old_path).replace("\\", "/").lstrip("./") + new_path = str(file_patch.new_path).replace("\\", "/").lstrip("./") + if old_path == failing_file and file_patch.new_path == "/dev/null": + return True + if new_path == failing_file and file_patch.old_path == "/dev/null": + continue + return False + + +def _detect_hard_failure(cmd_results: list[dict[str, Any]]) -> dict[str, str] | None: + for result in cmd_results: + command = str(result.get("command", "")).strip() + exit_code = int(result.get("exit", 0)) + stdout = str(result.get("stdout", "") or "") + stderr = str(result.get("stderr", "") or "") + combined = f"{stdout}\n{stderr}".strip() + lower = combined.lower() + has_crash_signal = any( + token in combined + for token in [ + "SyntaxError", + "NameError", + "ImportError", + "ModuleNotFoundError", + ] + ) or ("traceback (most recent call last)" in lower) or ("unhandled exception" in lower) + if exit_code != 0 or has_crash_signal: + evidence = parse_failure_signal(stdout, stderr) + summary = str(evidence.get("error_summary", "")).strip() + if not summary: + summary = (stderr or stdout or f"Command failed with exit {exit_code}").splitlines()[0][:200] + failing_file = str(evidence.get("traceback_file", "")).strip() + return { + "failing_command": command[:200], + "failing_file": failing_file, + "error_summary": summary[:220], + } + return None + + def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, Any]) -> str | None: + if tool_name in {"Write", "Patch"} and bool(getattr(runner, "_recovery_mode", False)): + failing_file = str(getattr(runner, "_failing_file", "")).replace("\\", "/").lstrip("./") + if failing_file: + targets: set[str] = set() + if tool_name == "Write": + target = str(tool_input.get("file_path", "")).replace("\\", "/").lstrip("./") + if target: + targets.add(target) + else: + diff_text = str(tool_input.get("unified_diff", "")) + default_path = str(tool_input.get("file_path", "") or "") or None + try: + targets = { + str(path).replace("\\", "/").lstrip("./") + for path in extract_unified_diff_targets(diff_text, default_file_path=default_path) + } + except PatchApplyError: + if default_path: + targets = {str(default_path).replace("\\", "/").lstrip("./")} + if targets and failing_file not in targets: + return _recovery_blocked_feedback( + "must_fix_active_failing_file", + tool_name, + ",".join(sorted(targets)), + f"active_failing_file={failing_file}", + ) + if failing_file in targets and not bool(getattr(runner, "_file_was_read_since_failure", False)): + return _recovery_blocked_feedback( + "read_required_before_edit", + tool_name, + failing_file, + "read the failing file before editing", + ) + if tool_name == "Write" and failing_file in targets: + content = str(tool_input.get("content", "")) + existing_path = (runner.repo / failing_file).resolve() + before_text = existing_path.read_text(encoding="utf-8", errors="replace") if existing_path.exists() and existing_path.is_file() else "" + if not content.strip(): + return _recovery_blocked_feedback("delete_blocked_for_failing_file", tool_name, failing_file) + before_lines = before_text.splitlines() + after_lines = content.splitlines() + changed_lines = sum(1 for a, b in zip(before_lines, after_lines) if a != b) + abs(len(before_lines) - len(after_lines)) + if before_lines and changed_lines > max(120, int(len(before_lines) * 0.8)): + return _recovery_blocked_feedback("rewrite_blocked_for_failing_file", tool_name, failing_file) + if tool_name == "Patch" and failing_file in targets: + diff_text = str(tool_input.get("unified_diff", "")) + if _patch_deletes_target_file(diff_text, failing_file): + return _recovery_blocked_feedback("delete_blocked_for_failing_file", tool_name, failing_file) + constrained = runner.small_model or runner.villani_mode or runner.benchmark_config.enabled if not constrained: return None @@ -1004,6 +1113,46 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: intended_targets=sorted(runner._current_verification_targets), before_contents=dict(runner._current_verification_before_contents), ) + hard_failure = _detect_hard_failure(cmd_results) + if hard_failure: + summary = hard_failure.get("error_summary", "") or "runtime/validation command failed" + failing_file = ( + hard_failure.get("failing_file", "") + or _single_clear_file(attributed_intentional) + or _single_clear_file(sorted(runner._current_verification_targets)) + or "" + ) + runner._recovery_mode = True + runner._failing_file = failing_file + runner._failing_error_summary = summary + runner._failing_command = hard_failure.get("failing_command", "") + runner._file_was_read_since_failure = False + verification.status = VerificationStatus.FAIL + verification.confidence_score = min(float(getattr(verification, "confidence_score", 0.5)), 0.05) + verification.summary = f"Hard failure detected: {summary}" + verification.findings.append( + VerificationFinding( + category=FindingCategory.REGRESSION, + message=f"Hard failure: {summary}", + file_path=failing_file or None, + severity="high", + ) + ) + runner.event_callback( + { + "type": "recovery_mode_activated", + "failing_file": failing_file, + "failing_error_summary": summary, + "failing_command": runner._failing_command, + } + ) + elif bool(getattr(runner, "_recovery_mode", False)) and verification.status in {VerificationStatus.PASS, VerificationStatus.REPAIRED}: + runner._recovery_mode = False + runner._failing_file = "" + runner._failing_error_summary = "" + runner._failing_command = "" + runner._file_was_read_since_failure = False + runner.event_callback({"type": "recovery_mode_cleared"}) finding_fingerprints = sorted( "|".join( [ diff --git a/villani_code/state_tooling.py b/villani_code/state_tooling.py index 8c15710a..986d007a 100644 --- a/villani_code/state_tooling.py +++ b/villani_code/state_tooling.py @@ -485,10 +485,10 @@ def execute_tool_with_policy( if not any(command.startswith(prefix) for prefix in readonly_prefixes): return {"content": "Planning mode is read-only: shell command is not on read-only allowlist", "is_error": True} + policy_error = runner._small_model_tool_guard(tool_name, tool_input) + if policy_error: + return {"content": policy_error, "is_error": True} if runner.small_model or runner.villani_mode or runner.benchmark_config.enabled: - policy_error = runner._small_model_tool_guard(tool_name, tool_input) - if policy_error: - return {"content": policy_error, "is_error": True} if runner.small_model: runner._tighten_tool_input(tool_name, tool_input) if tool_name in {"Write", "Patch"}: @@ -679,4 +679,12 @@ def _debug_callback_with_turn(event_type: str, payload: dict[str, Any]) -> None: **({"forced": True} if forced else {}), } ) + if ( + tool_name == "Read" + and not result.get("is_error") + and bool(getattr(runner, "_recovery_mode", False)) + ): + read_path = str(tool_input.get("file_path", "")).replace("\\", "/").lstrip("./") + if read_path and read_path == str(getattr(runner, "_failing_file", "")): + runner._file_was_read_since_failure = True return result From d8b2bde112c0996c3759bd99c82ecd7eb7ccaa24 Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 12:12:59 +1000 Subject: [PATCH 04/12] Lock recovery to active solution file and harden validation commands --- tests/test_state_execution.py | 29 ++++++ tests/test_state_runtime.py | 27 ++++++ tests/test_state_tooling_mutation_policy.py | 58 +++++++++++ villani_code/state.py | 2 + villani_code/state_execution.py | 7 ++ villani_code/state_runtime.py | 101 ++++++++++++++++---- villani_code/state_tooling.py | 4 + 7 files changed, 210 insertions(+), 18 deletions(-) create mode 100644 tests/test_state_execution.py diff --git a/tests/test_state_execution.py b/tests/test_state_execution.py new file mode 100644 index 00000000..1b2fc048 --- /dev/null +++ b/tests/test_state_execution.py @@ -0,0 +1,29 @@ +from __future__ import annotations + +from villani_code.state_execution import collect_validation_artifacts + + +def test_collect_validation_artifacts_ignores_masked_failure_patterns() -> None: + transcript = { + "tool_results": [ + { + "content": '{"command":"python web_app.py || echo ok","exit_code":0}', + "is_error": False, + } + ] + } + artifacts = collect_validation_artifacts(transcript) + assert artifacts == [] + + +def test_collect_validation_artifacts_keeps_clean_command() -> None: + transcript = { + "tool_results": [ + { + "content": '{"command":"python web_app.py","exit_code":0}', + "is_error": False, + } + ] + } + artifacts = collect_validation_artifacts(transcript) + assert artifacts == ["python web_app.py (exit=0)"] diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index c9b61cee..446efaef 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -341,6 +341,7 @@ class P: runner._run_verification("edit") assert runner._recovery_mode is True assert runner._failing_file == "legal_review_app.py" + assert runner._active_solution_file == "legal_review_app.py" assert runner._failing_error_summary @@ -373,8 +374,34 @@ def fake_run(cmd, **kwargs): monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) runner._run_verification("edit") assert runner._recovery_mode is True + assert runner._active_solution_file == "legal_review_app.py" runner._run_verification("edit") assert runner._recovery_mode is False + assert runner._active_solution_file == "legal_review_app.py" + + +def test_helper_failure_does_not_switch_active_solution_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") + (tmp_path / "helper.py").write_text("raise RuntimeError('x')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._active_solution_file = "web_app.py" + runner._current_verification_targets = {"web_app.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["helper.py"]) + + def fake_run(cmd, **kwargs): + class P: + returncode = 1 + stdout = "" + stderr = 'Traceback (most recent call last):\n File "helper.py", line 1\nRuntimeError: x\n' + return P() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert runner._recovery_mode is True + assert runner._failing_file == "helper.py" + assert runner._active_solution_file == "web_app.py" def test_changed_validation_state_emits_new_compact_summary( diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index 8d62979a..cdad514d 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -149,6 +149,27 @@ def test_recovery_mode_blocks_delete_of_active_failing_file(tmp_path: Path) -> N assert "delete_blocked_for_failing_file" in str(result["content"]) +def test_recovery_mode_blocks_delete_of_active_solution_file(tmp_path: Path) -> None: + runner = _runner(tmp_path) + target = tmp_path / "web_app.py" + target.write_text("print('x')\n", encoding="utf-8") + runner._recovery_mode = True + runner._active_solution_file = "web_app.py" + runner._failing_file = "helper.py" + runner._file_was_read_since_failure = True + + result = execute_tool_with_policy( + runner, + "Patch", + {"unified_diff": "--- a/web_app.py\n+++ /dev/null\n@@ -1 +0,0 @@\n-print('x')\n"}, + "1", + 0, + ) + assert result["is_error"] is True + assert "delete_blocked_for_failing_file" in str(result["content"]) + assert '"recovery_blocked": true' in str(result["content"]) + + def test_recovery_mode_blocks_edit_without_read_first(tmp_path: Path) -> None: runner = _runner(tmp_path) target = tmp_path / "legal_review_app.py" @@ -169,6 +190,43 @@ def test_recovery_mode_blocks_edit_without_read_first(tmp_path: Path) -> None: assert "read_required_before_edit" in str(result["content"]) +def test_recovery_mode_blocks_parallel_entrypoint_launch(tmp_path: Path) -> None: + runner = _runner(tmp_path) + (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") + (tmp_path / "web_server.py").write_text("print('x')\n", encoding="utf-8") + runner._recovery_mode = True + runner._active_solution_file = "web_app.py" + + result = execute_tool_with_policy( + runner, + "Bash", + {"command": "python web_server.py", "timeout_sec": 30}, + "1", + 0, + ) + assert result["is_error"] is True + assert "recovery_entrypoint_switch_blocked" in str(result["content"]) + + +def test_recovery_mode_allows_helper_file_edit_with_active_solution(tmp_path: Path) -> None: + runner = _runner(tmp_path) + (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") + helper = tmp_path / "utils.py" + helper.write_text("x=1\n", encoding="utf-8") + runner._recovery_mode = True + runner._active_solution_file = "web_app.py" + runner._failing_file = "web_app.py" + + result = execute_tool_with_policy( + runner, + "Patch", + {"unified_diff": "--- a/utils.py\n+++ b/utils.py\n@@ -1 +1 @@\n-x=1\n+x=2\n"}, + "1", + 0, + ) + assert result["is_error"] is False + + def test_read_failing_file_flips_recovery_read_flag(tmp_path: Path) -> None: runner = _runner(tmp_path) (tmp_path / "legal_review_app.py").write_text("print('x')\n", encoding="utf-8") diff --git a/villani_code/state.py b/villani_code/state.py index 958a6efc..065847f3 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -531,6 +531,7 @@ def __init__( self._first_attempt_locked_target = "" self._recovery_mode = False self._failing_file = "" + self._active_solution_file = "" self._failing_error_summary = "" self._failing_command = "" self._file_was_read_since_failure = False @@ -910,6 +911,7 @@ def run( self._first_attempt_locked_target = required_initial_read self._recovery_mode = False self._failing_file = "" + self._active_solution_file = "" self._failing_error_summary = "" self._failing_command = "" self._file_was_read_since_failure = False diff --git a/villani_code/state_execution.py b/villani_code/state_execution.py index 7df5e695..36b038d1 100644 --- a/villani_code/state_execution.py +++ b/villani_code/state_execution.py @@ -7,6 +7,11 @@ from villani_code.repo_rules import classify_repo_path, is_ignored_repo_path +def _has_masking_pattern(command: str) -> bool: + lowered = str(command or "").lower() + return ("||" in lowered) or ("&& echo" in lowered) or ("2>&1 |" in lowered) or ("timeout" in lowered and "echo" in lowered) + + @dataclass(frozen=True, slots=True) class ChangeSummary: intentional: list[str] @@ -34,6 +39,8 @@ def collect_validation_artifacts(transcript: dict[str, Any]) -> list[str]: artifacts: list[str] = [] for tool_result in transcript.get("tool_results", []): for record in parse_command_evidence(str(tool_result.get("content", ""))): + if _has_masking_pattern(str(record.get("command", ""))): + continue artifact = normalize_artifact(record) if artifact: artifacts.append(artifact) diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index 6dc2ea6c..7e6d46c4 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -561,6 +561,38 @@ def _patch_deletes_target_file(unified_diff: str, failing_file: str) -> bool: return False +def _command_has_masking_patterns(command: str) -> bool: + cmd = str(command or "").strip().lower() + if not cmd: + return False + if "||" in cmd: + return True + if "&& echo" in cmd: + return True + if "2>&1 |" in cmd: + return True + if "timeout" in cmd and "echo" in cmd: + return True + return False + + +def _is_validation_or_launch_command(command: str) -> bool: + lowered = str(command or "").strip().lower() + if not lowered: + return False + markers = ("pytest", "unittest", "test", "validate", "verification", "python ", "python3 ", "uv run", "poetry run") + return any(marker in lowered for marker in markers) + + +def _extract_command_python_target(command: str) -> str: + matches = re.findall(r"([\w./-]+\.py)\b", str(command or "")) + for match in matches: + normalized = _normalize_repo_path(match) + if normalized and not normalized.startswith("tests/"): + return normalized + return "" + + def _detect_hard_failure(cmd_results: list[dict[str, Any]]) -> dict[str, str] | None: for result in cmd_results: command = str(result.get("command", "")).strip() @@ -595,7 +627,9 @@ def _detect_hard_failure(cmd_results: list[dict[str, Any]]) -> dict[str, str] | def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, Any]) -> str | None: if tool_name in {"Write", "Patch"} and bool(getattr(runner, "_recovery_mode", False)): failing_file = str(getattr(runner, "_failing_file", "")).replace("\\", "/").lstrip("./") - if failing_file: + active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") + locked_file = active_solution_file or failing_file + if locked_file: targets: set[str] = set() if tool_name == "Write": target = str(tool_input.get("file_path", "")).replace("\\", "/").lstrip("./") @@ -612,35 +646,49 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An except PatchApplyError: if default_path: targets = {str(default_path).replace("\\", "/").lstrip("./")} - if targets and failing_file not in targets: - return _recovery_blocked_feedback( - "must_fix_active_failing_file", - tool_name, - ",".join(sorted(targets)), - f"active_failing_file={failing_file}", - ) - if failing_file in targets and not bool(getattr(runner, "_file_was_read_since_failure", False)): + if locked_file in targets and not bool(getattr(runner, "_file_was_read_since_failure", False)): return _recovery_blocked_feedback( "read_required_before_edit", tool_name, - failing_file, + locked_file, "read the failing file before editing", ) - if tool_name == "Write" and failing_file in targets: + if tool_name == "Write" and locked_file in targets: content = str(tool_input.get("content", "")) - existing_path = (runner.repo / failing_file).resolve() + existing_path = (runner.repo / locked_file).resolve() before_text = existing_path.read_text(encoding="utf-8", errors="replace") if existing_path.exists() and existing_path.is_file() else "" if not content.strip(): - return _recovery_blocked_feedback("delete_blocked_for_failing_file", tool_name, failing_file) + return _recovery_blocked_feedback("delete_blocked_for_failing_file", tool_name, locked_file) before_lines = before_text.splitlines() after_lines = content.splitlines() changed_lines = sum(1 for a, b in zip(before_lines, after_lines) if a != b) + abs(len(before_lines) - len(after_lines)) if before_lines and changed_lines > max(120, int(len(before_lines) * 0.8)): - return _recovery_blocked_feedback("rewrite_blocked_for_failing_file", tool_name, failing_file) - if tool_name == "Patch" and failing_file in targets: + return _recovery_blocked_feedback("rewrite_blocked_for_failing_file", tool_name, locked_file) + if tool_name == "Patch" and locked_file in targets: diff_text = str(tool_input.get("unified_diff", "")) - if _patch_deletes_target_file(diff_text, failing_file): - return _recovery_blocked_feedback("delete_blocked_for_failing_file", tool_name, failing_file) + if _patch_deletes_target_file(diff_text, locked_file): + return _recovery_blocked_feedback("delete_blocked_for_failing_file", tool_name, locked_file) + if tool_name == "Bash": + command = str(tool_input.get("command", "")).strip() + if _is_validation_or_launch_command(command) and _command_has_masking_patterns(command): + return _recovery_blocked_feedback("suspicious_validation_command", tool_name, "", "masked failure patterns detected") + target = _extract_command_python_target(command) + active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") + if _is_validation_or_launch_command(command) and target and not active_solution_file: + runner._active_solution_file = target + if ( + bool(getattr(runner, "_recovery_mode", False)) + and _is_validation_or_launch_command(command) + and target + and active_solution_file + and target != active_solution_file + ): + return _recovery_blocked_feedback( + "recovery_entrypoint_switch_blocked", + tool_name, + target, + f"active_solution_file={active_solution_file}", + ) constrained = runner.small_model or runner.villani_mode or runner.benchmark_config.enabled if not constrained: @@ -1095,8 +1143,18 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: if stderr_lines: lines.append(f"key stderr:\n{stderr_lines}") + if not str(getattr(runner, "_active_solution_file", "")).strip(): + seed_active = ( + _single_clear_file(sorted(runner._current_verification_targets)) + or _single_clear_file(attributed_intentional) + ) + if seed_active: + runner._active_solution_file = seed_active + verification_artifacts = [ - r.get("command", "") for r in cmd_results if int(r.get("exit", 1)) == 0 + r.get("command", "") + for r in cmd_results + if int(r.get("exit", 1)) == 0 and not _command_has_masking_patterns(str(r.get("command", ""))) ] validation_target_paths = sorted(set(runner._current_verification_targets) or set(attributed_intentional)) validation_target = json.dumps(validation_target_paths) @@ -1116,12 +1174,18 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: hard_failure = _detect_hard_failure(cmd_results) if hard_failure: summary = hard_failure.get("error_summary", "") or "runtime/validation command failed" + active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") failing_file = ( hard_failure.get("failing_file", "") or _single_clear_file(attributed_intentional) or _single_clear_file(sorted(runner._current_verification_targets)) or "" ) + failing_file = _normalize_repo_path(failing_file) + if active_solution_file and failing_file == active_solution_file: + failing_file = active_solution_file + elif not active_solution_file and failing_file: + runner._active_solution_file = failing_file runner._recovery_mode = True runner._failing_file = failing_file runner._failing_error_summary = summary @@ -1142,6 +1206,7 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: { "type": "recovery_mode_activated", "failing_file": failing_file, + "active_solution_file": str(getattr(runner, "_active_solution_file", "")), "failing_error_summary": summary, "failing_command": runner._failing_command, } diff --git a/villani_code/state_tooling.py b/villani_code/state_tooling.py index 986d007a..1719aff4 100644 --- a/villani_code/state_tooling.py +++ b/villani_code/state_tooling.py @@ -604,6 +604,10 @@ def execute_tool_with_policy( if normalized_targets: runner._intended_targets.update(normalized_targets) runner._current_verification_targets = set(normalized_targets) + if not str(getattr(runner, "_active_solution_file", "")).strip(): + seed_target = normalized_targets[0] if len(normalized_targets) == 1 else "" + if seed_target: + runner._active_solution_file = seed_target runner._current_verification_before_contents = {} checkpoint_paths: list[Path] = [] for normalized_target in normalized_targets: From 29ab71a130977c0f9b87ef5d7cf92cf26fb6f943 Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 13:00:41 +1000 Subject: [PATCH 05/12] Gate completion on active solution validation state --- tests/test_bounded_execution.py | 40 +++++++++++ tests/test_state_runtime.py | 6 ++ tests/test_state_tooling_mutation_policy.py | 42 +++++++++++ villani_code/state.py | 53 +++----------- villani_code/state_runtime.py | 78 ++++++++++++++++++++- 5 files changed, 176 insertions(+), 43 deletions(-) diff --git a/tests/test_bounded_execution.py b/tests/test_bounded_execution.py index ed08c70f..5f0fc6b3 100644 --- a/tests/test_bounded_execution.py +++ b/tests/test_bounded_execution.py @@ -110,3 +110,43 @@ def test_villani_task_reports_completed_when_done(tmp_path: Path) -> None: assert result["execution"]["completed"] is True assert result["execution"]["terminated_reason"] == "completed" + + +def test_active_solution_failed_validation_blocks_completed_status(tmp_path: Path, monkeypatch) -> None: + runner = _runner( + tmp_path, + [ + {"role": "assistant", "content": [{"type": "tool_use", "id": "1", "name": "Bash", "input": {"command": "python web_server.py"}}]}, + {"role": "assistant", "content": [{"type": "text", "text": "all done"}]}, + ], + ) + def fake_verify(*_args, **_kwargs) -> str: + runner._active_solution_file = "web_server.py" + runner._active_solution_last_validation_ok = False + return "" + monkeypatch.setattr(runner, "_run_verification", fake_verify) + + result = runner.run("work") + + assert result["execution"]["completed"] is False + assert result["execution"]["terminated_reason"] == "active_solution_validation_failed" + + +def test_active_solution_successful_validation_allows_completed_status(tmp_path: Path, monkeypatch) -> None: + runner = _runner( + tmp_path, + [ + {"role": "assistant", "content": [{"type": "tool_use", "id": "1", "name": "Bash", "input": {"command": "python web_server.py"}}]}, + {"role": "assistant", "content": [{"type": "text", "text": "all done"}]}, + ], + ) + def fake_verify(*_args, **_kwargs) -> str: + runner._active_solution_file = "web_server.py" + runner._active_solution_last_validation_ok = True + return "" + monkeypatch.setattr(runner, "_run_verification", fake_verify) + + result = runner.run("work") + + assert result["execution"]["completed"] is True + assert result["execution"]["terminated_reason"] == "completed" diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index 446efaef..8bbab2c1 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -299,6 +299,8 @@ class P: monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) runner._run_verification("edit") assert "status=fail" in runner._last_validation_summary + assert runner._active_solution_last_validation_ok is False + assert runner._active_solution_last_validation_summary def test_name_error_does_not_mark_verification_pass(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -320,6 +322,8 @@ class P: monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) runner._run_verification("edit") assert "status=fail" in runner._last_validation_summary + assert runner._active_solution_last_validation_ok is False + assert runner._active_solution_last_validation_summary def test_hard_failure_sets_recovery_mode_state(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -375,9 +379,11 @@ def fake_run(cmd, **kwargs): runner._run_verification("edit") assert runner._recovery_mode is True assert runner._active_solution_file == "legal_review_app.py" + assert runner._active_solution_last_validation_ok is False runner._run_verification("edit") assert runner._recovery_mode is False assert runner._active_solution_file == "legal_review_app.py" + assert runner._active_solution_last_validation_ok is True def test_helper_failure_does_not_switch_active_solution_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index cdad514d..a6bec2d0 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -227,6 +227,48 @@ def test_recovery_mode_allows_helper_file_edit_with_active_solution(tmp_path: Pa assert result["is_error"] is False +def test_recovery_mode_blocks_full_write_of_active_solution_after_hard_failure(tmp_path: Path) -> None: + runner = _runner(tmp_path) + target = tmp_path / "web_server.py" + target.write_text("print('x')\n", encoding="utf-8") + runner._recovery_mode = True + runner._active_solution_file = "web_server.py" + runner._failing_file = "web_server.py" + runner._active_solution_last_validation_ok = False + runner._file_was_read_since_failure = True + + result = execute_tool_with_policy( + runner, + "Write", + {"file_path": "web_server.py", "content": "print('rewritten')\n"}, + "1", + 0, + ) + assert result["is_error"] is True + assert "full_write_blocked_for_active_solution" in str(result["content"]) + assert '"recovery_blocked": true' in str(result["content"]) + + +def test_recovery_mode_allows_bounded_patch_of_active_solution_after_hard_failure(tmp_path: Path) -> None: + runner = _runner(tmp_path) + target = tmp_path / "web_server.py" + target.write_text("x=1\n", encoding="utf-8") + runner._recovery_mode = True + runner._active_solution_file = "web_server.py" + runner._failing_file = "web_server.py" + runner._active_solution_last_validation_ok = False + runner._file_was_read_since_failure = True + + result = execute_tool_with_policy( + runner, + "Patch", + {"unified_diff": "--- a/web_server.py\n+++ b/web_server.py\n@@ -1 +1 @@\n-x=1\n+x=2\n"}, + "1", + 0, + ) + assert result["is_error"] is False + + def test_read_failing_file_flips_recovery_read_flag(tmp_path: Path) -> None: runner = _runner(tmp_path) (tmp_path / "legal_review_app.py").write_text("print('x')\n", encoding="utf-8") diff --git a/villani_code/state.py b/villani_code/state.py index 065847f3..502b6af6 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -532,6 +532,8 @@ def __init__( self._recovery_mode = False self._failing_file = "" self._active_solution_file = "" + self._active_solution_last_validation_ok: bool | None = None + self._active_solution_last_validation_summary = "" self._failing_error_summary = "" self._failing_command = "" self._file_was_read_since_failure = False @@ -912,6 +914,8 @@ def run( self._recovery_mode = False self._failing_file = "" self._active_solution_file = "" + self._active_solution_last_validation_ok = None + self._active_solution_last_validation_summary = "" self._failing_error_summary = "" self._failing_command = "" self._file_was_read_since_failure = False @@ -1060,6 +1064,11 @@ def _finish_bounded( def _budget_reason( completed: bool = False, model_idle: bool = False ) -> str | None: + if completed: + active_solution_file = str(getattr(self, "_active_solution_file", "")).strip() + active_validation_ok = getattr(self, "_active_solution_last_validation_ok", None) + if active_solution_file and active_validation_ok is not True: + return "active_solution_validation_missing" if active_validation_ok is None else "active_solution_validation_failed" if execution_budget is None: return None elapsed = time.monotonic() - start @@ -1218,27 +1227,7 @@ def _budget_reason( reason = _budget_reason(completed=True) if reason: return _finish_bounded(response, reason, reason == "completed") - transcript["final_assistant_content"] = response.get("content", []) - transcript_path = self._save_transcript_and_link(transcript) - post = self._run_post_execution_validation(_change_summary()[2]) - if post: - response.setdefault("content", []).append({"type": "text", "text": post}) - self._save_session_snapshot(messages) - if self._event_recorder is not None: - self._event_recorder.write_digest() - if self._debug_recorder is not None: - self._debug_recorder.write_final_summary( - status="completed", - termination_reason="completed", - total_turns=turns_used, - mission_id=self._mission_id, - ) - return { - "response": response, - "messages": messages, - "transcript_path": str(transcript_path), - "transcript": transcript, - } + return _finish_bounded(response, "completed", True) proposal = self._capture_edit_proposal(response) if proposal: self.event_callback( @@ -1346,27 +1335,7 @@ def _budget_reason( reason = _budget_reason(completed=True) if reason: return _finish_bounded(response, reason, reason == "completed") - transcript["final_assistant_content"] = response.get("content", []) - transcript_path = None - if not self._planning_read_only: - transcript_path = self._save_transcript_and_link(transcript) - self._save_session_snapshot(messages) - self._update_mission_state(status="completed", compact_summary=summarize_mission_state(self._mission_state) if self._mission_state else "") - if self._event_recorder is not None: - self._event_recorder.write_digest() - if self._debug_recorder is not None: - self._debug_recorder.write_final_summary( - status="completed", - termination_reason="completed", - total_turns=turns_used, - mission_id=self._mission_id, - ) - return { - "response": response, - "messages": messages, - "transcript_path": str(transcript_path) if transcript_path is not None else "", - "transcript": transcript, - } + return _finish_bounded(response, "completed", True) tool_results: list[dict[str, Any]] = [] for block in tool_uses: diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index 7e6d46c4..dd343904 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -624,6 +624,33 @@ def _detect_hard_failure(cmd_results: list[dict[str, Any]]) -> dict[str, str] | return None +def _command_targets_active_solution(command: str, active_solution_file: str, validation_targets: set[str]) -> bool: + active = str(active_solution_file or "").replace("\\", "/").lstrip("./") + if not active: + return False + if not _is_validation_or_launch_command(command): + return False + normalized_targets = {str(t).replace("\\", "/").lstrip("./") for t in validation_targets if str(t).strip()} + explicit_target = _extract_command_python_target(command) + if explicit_target: + return explicit_target == active + cmd = str(command or "") + if active in cmd: + return True + return active in normalized_targets + + +def _has_hard_failure_signal(stdout: str, stderr: str, exit_code: int) -> bool: + combined = f"{stdout}\n{stderr}".strip() + lower = combined.lower() + return ( + exit_code != 0 + or any(token in combined for token in ("SyntaxError", "NameError", "ImportError", "ModuleNotFoundError")) + or ("traceback (most recent call last)" in lower) + or ("unhandled exception" in lower) + ) + + def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, Any]) -> str | None: if tool_name in {"Write", "Patch"} and bool(getattr(runner, "_recovery_mode", False)): failing_file = str(getattr(runner, "_failing_file", "")).replace("\\", "/").lstrip("./") @@ -655,6 +682,18 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An ) if tool_name == "Write" and locked_file in targets: content = str(tool_input.get("content", "")) + active_is_failed = ( + locked_file == active_solution_file + and bool(getattr(runner, "_recovery_mode", False)) + and getattr(runner, "_active_solution_last_validation_ok", None) is False + ) + if active_is_failed and content.strip(): + return _recovery_blocked_feedback( + "full_write_blocked_for_active_solution", + tool_name, + locked_file, + "use a bounded patch/edit while recovering from hard failure", + ) existing_path = (runner.repo / locked_file).resolve() before_text = existing_path.read_text(encoding="utf-8", errors="replace") if existing_path.exists() and existing_path.is_file() else "" if not content.strip(): @@ -1171,10 +1210,43 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: intended_targets=sorted(runner._current_verification_targets), before_contents=dict(runner._current_verification_before_contents), ) + active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") + if not active_solution_file: + for result in cmd_results: + seeded_target = _extract_command_python_target(str(result.get("command", ""))) + if seeded_target: + runner._active_solution_file = seeded_target + active_solution_file = seeded_target + break + validation_targets = set(validation_target_paths) + active_cmd_results = [ + result + for result in cmd_results + if _command_targets_active_solution( + str(result.get("command", "")), + active_solution_file, + validation_targets, + ) + ] + if active_cmd_results: + latest = active_cmd_results[-1] + exit_code = int(latest.get("exit", 0)) + stdout = str(latest.get("stdout", "") or "") + stderr = str(latest.get("stderr", "") or "") + has_failure = _has_hard_failure_signal(stdout, stderr, exit_code) + if has_failure: + runner._active_solution_last_validation_ok = False + evidence_lines = [ln.strip() for ln in (stderr or stdout or f"exit={exit_code}").splitlines() if ln.strip()] + summary_line = (evidence_lines[-1] if evidence_lines else f"exit={exit_code}")[:220] + runner._active_solution_last_validation_summary = summary_line + verification.status = VerificationStatus.FAIL + else: + runner._active_solution_last_validation_ok = True + runner._active_solution_last_validation_summary = "validation passed" + hard_failure = _detect_hard_failure(cmd_results) if hard_failure: summary = hard_failure.get("error_summary", "") or "runtime/validation command failed" - active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") failing_file = ( hard_failure.get("failing_file", "") or _single_clear_file(attributed_intentional) @@ -1186,11 +1258,15 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: failing_file = active_solution_file elif not active_solution_file and failing_file: runner._active_solution_file = failing_file + active_solution_file = failing_file runner._recovery_mode = True runner._failing_file = failing_file runner._failing_error_summary = summary runner._failing_command = hard_failure.get("failing_command", "") runner._file_was_read_since_failure = False + if active_solution_file and failing_file == active_solution_file: + runner._active_solution_last_validation_ok = False + runner._active_solution_last_validation_summary = summary[:220] verification.status = VerificationStatus.FAIL verification.confidence_score = min(float(getattr(verification, "confidence_score", 0.5)), 0.05) verification.summary = f"Hard failure detected: {summary}" From 8c9c558afc62b0994f4182fcd5d166cb086efd5d Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 13:38:29 +1000 Subject: [PATCH 06/12] Pin recovery to primary execution target after hard failure --- tests/test_state_runtime.py | 50 +++++++++++++++++++ tests/test_state_tooling_mutation_policy.py | 19 ++++++++ villani_code/state.py | 6 ++- villani_code/state_runtime.py | 53 +++++++++++++++++---- 4 files changed, 118 insertions(+), 10 deletions(-) diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index 8bbab2c1..4d688ac5 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -346,6 +346,7 @@ class P: assert runner._recovery_mode is True assert runner._failing_file == "legal_review_app.py" assert runner._active_solution_file == "legal_review_app.py" + assert runner._primary_execution_target == "legal_review_app.py" assert runner._failing_error_summary @@ -379,10 +380,12 @@ def fake_run(cmd, **kwargs): runner._run_verification("edit") assert runner._recovery_mode is True assert runner._active_solution_file == "legal_review_app.py" + assert runner._primary_execution_target == "legal_review_app.py" assert runner._active_solution_last_validation_ok is False runner._run_verification("edit") assert runner._recovery_mode is False assert runner._active_solution_file == "legal_review_app.py" + assert runner._primary_execution_target == "legal_review_app.py" assert runner._active_solution_last_validation_ok is True @@ -410,6 +413,53 @@ class P: assert runner._active_solution_file == "web_app.py" +def test_first_meaningful_validation_sets_primary_execution_target(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + (tmp_path / "service.py").write_text("print('ok')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._current_verification_targets = {"service.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["service.py"]) + + def fake_run(cmd, **kwargs): + class P: + returncode = 0 + stdout = "ok" + stderr = "" + + return P() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert runner._primary_execution_target == "service.py" + + +def test_recovery_does_not_clear_until_primary_target_validates(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") + (tmp_path / "helper.py").write_text("print('helper')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._current_verification_targets = {"helper.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["helper.py"]) + runner._recovery_mode = True + runner._active_solution_file = "web_app.py" + runner._primary_execution_target = "web_app.py" + runner._active_solution_last_validation_ok = False + + def fake_run(cmd, **kwargs): + class P: + returncode = 0 + stdout = "helper ok" + stderr = "" + + return P() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert runner._recovery_mode is True + + def test_changed_validation_state_emits_new_compact_summary( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index a6bec2d0..9ad52199 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -196,6 +196,7 @@ def test_recovery_mode_blocks_parallel_entrypoint_launch(tmp_path: Path) -> None (tmp_path / "web_server.py").write_text("print('x')\n", encoding="utf-8") runner._recovery_mode = True runner._active_solution_file = "web_app.py" + runner._primary_execution_target = "web_app.py" result = execute_tool_with_policy( runner, @@ -206,6 +207,24 @@ def test_recovery_mode_blocks_parallel_entrypoint_launch(tmp_path: Path) -> None ) assert result["is_error"] is True assert "recovery_entrypoint_switch_blocked" in str(result["content"]) + assert "primary_execution_target=web_app.py" in str(result["content"]) + + +def test_recovery_mode_allows_rerun_of_same_primary_target(tmp_path: Path) -> None: + runner = _runner(tmp_path) + (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") + runner._recovery_mode = True + runner._primary_execution_target = "web_app.py" + runner._active_solution_file = "web_app.py" + + result = execute_tool_with_policy( + runner, + "Bash", + {"command": "python web_app.py", "timeout_sec": 30}, + "1", + 0, + ) + assert result["is_error"] is False def test_recovery_mode_allows_helper_file_edit_with_active_solution(tmp_path: Path) -> None: diff --git a/villani_code/state.py b/villani_code/state.py index 502b6af6..8af2233d 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -532,6 +532,7 @@ def __init__( self._recovery_mode = False self._failing_file = "" self._active_solution_file = "" + self._primary_execution_target = "" self._active_solution_last_validation_ok: bool | None = None self._active_solution_last_validation_summary = "" self._failing_error_summary = "" @@ -914,6 +915,7 @@ def run( self._recovery_mode = False self._failing_file = "" self._active_solution_file = "" + self._primary_execution_target = "" self._active_solution_last_validation_ok = None self._active_solution_last_validation_summary = "" self._failing_error_summary = "" @@ -1065,7 +1067,9 @@ def _budget_reason( completed: bool = False, model_idle: bool = False ) -> str | None: if completed: - active_solution_file = str(getattr(self, "_active_solution_file", "")).strip() + active_solution_file = str( + getattr(self, "_primary_execution_target", "") or getattr(self, "_active_solution_file", "") + ).strip() active_validation_ok = getattr(self, "_active_solution_last_validation_ok", None) if active_solution_file and active_validation_ok is not True: return "active_solution_validation_missing" if active_validation_ok is None else "active_solution_validation_failed" diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index dd343904..64462a0c 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -593,6 +593,21 @@ def _extract_command_python_target(command: str) -> str: return "" +def _primary_execution_target(runner: Any) -> str: + return str(getattr(runner, "_primary_execution_target", "")).replace("\\", "/").lstrip("./") + + +def _seed_primary_execution_target(runner: Any, target: str) -> str: + normalized = _normalize_repo_path(target) + if not normalized: + return _primary_execution_target(runner) + current = _primary_execution_target(runner) + if current: + return current + runner._primary_execution_target = normalized + return normalized + + def _detect_hard_failure(cmd_results: list[dict[str, Any]]) -> dict[str, str] | None: for result in cmd_results: command = str(result.get("command", "")).strip() @@ -713,20 +728,23 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An return _recovery_blocked_feedback("suspicious_validation_command", tool_name, "", "masked failure patterns detected") target = _extract_command_python_target(command) active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") + primary_target = _primary_execution_target(runner) if _is_validation_or_launch_command(command) and target and not active_solution_file: runner._active_solution_file = target + if _is_validation_or_launch_command(command) and target: + _seed_primary_execution_target(runner, target) if ( bool(getattr(runner, "_recovery_mode", False)) and _is_validation_or_launch_command(command) and target - and active_solution_file - and target != active_solution_file + and primary_target + and target != primary_target ): return _recovery_blocked_feedback( "recovery_entrypoint_switch_blocked", tool_name, target, - f"active_solution_file={active_solution_file}", + f"primary_execution_target={primary_target}", ) constrained = runner.small_model or runner.villani_mode or runner.benchmark_config.enabled @@ -1189,6 +1207,8 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: ) if seed_active: runner._active_solution_file = seed_active + if str(getattr(runner, "_active_solution_file", "")).strip(): + _seed_primary_execution_target(runner, str(getattr(runner, "_active_solution_file", ""))) verification_artifacts = [ r.get("command", "") @@ -1218,6 +1238,7 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: runner._active_solution_file = seeded_target active_solution_file = seeded_target break + primary_target = _seed_primary_execution_target(runner, active_solution_file) validation_targets = set(validation_target_paths) active_cmd_results = [ result @@ -1259,6 +1280,7 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: elif not active_solution_file and failing_file: runner._active_solution_file = failing_file active_solution_file = failing_file + primary_target = _seed_primary_execution_target(runner, active_solution_file or failing_file) runner._recovery_mode = True runner._failing_file = failing_file runner._failing_error_summary = summary @@ -1283,17 +1305,30 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: "type": "recovery_mode_activated", "failing_file": failing_file, "active_solution_file": str(getattr(runner, "_active_solution_file", "")), + "primary_execution_target": primary_target, "failing_error_summary": summary, "failing_command": runner._failing_command, } ) elif bool(getattr(runner, "_recovery_mode", False)) and verification.status in {VerificationStatus.PASS, VerificationStatus.REPAIRED}: - runner._recovery_mode = False - runner._failing_file = "" - runner._failing_error_summary = "" - runner._failing_command = "" - runner._file_was_read_since_failure = False - runner.event_callback({"type": "recovery_mode_cleared"}) + primary_target = _primary_execution_target(runner) + validated_primary = False + if primary_target: + validated_primary = any( + _command_targets_active_solution( + str(result.get("command", "")), + primary_target, + validation_targets, + ) + for result in cmd_results + ) + if not primary_target or validated_primary: + runner._recovery_mode = False + runner._failing_file = "" + runner._failing_error_summary = "" + runner._failing_command = "" + runner._file_was_read_since_failure = False + runner.event_callback({"type": "recovery_mode_cleared"}) finding_fingerprints = sorted( "|".join( [ From 2311c2489b62eb5c019296f0ed851236ba72068e Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 14:09:21 +1000 Subject: [PATCH 07/12] Add recovery gate for new validation artifacts after hard failure --- tests/test_mission_state_runtime.py | 5 ++ tests/test_state_runtime.py | 25 ++++++++ tests/test_state_tooling_mutation_policy.py | 15 ++++- villani_code/mission_state.py | 6 ++ villani_code/state.py | 9 +++ villani_code/state_runtime.py | 68 ++++++++++++++++++--- villani_code/state_tooling.py | 16 +++++ 7 files changed, 132 insertions(+), 12 deletions(-) diff --git a/tests/test_mission_state_runtime.py b/tests/test_mission_state_runtime.py index ef3651c3..0eb60599 100644 --- a/tests/test_mission_state_runtime.py +++ b/tests/test_mission_state_runtime.py @@ -34,6 +34,9 @@ def test_mission_state_roundtrip(tmp_path: Path) -> None: mode="execution", repo_root=str(tmp_path), status="active", + recovery_mode=True, + primary_execution_target="app.py", + primary_target_minimally_valid=False, verified_facts=[VerifiedFact(kind="k", value="v", source="s")], open_hypotheses=[OpenHypothesis(hypothesis_id="h1", statement="maybe", confidence=0.4, status="open")], ) @@ -41,6 +44,8 @@ def test_mission_state_roundtrip(tmp_path: Path) -> None: loaded = load_mission_state(tmp_path, "m1") assert loaded.verified_facts[0].value == "v" assert loaded.open_hypotheses[0].hypothesis_id == "h1" + assert loaded.recovery_mode is True + assert loaded.primary_execution_target == "app.py" def test_mission_directory_and_current_pointer(tmp_path: Path) -> None: diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index 4d688ac5..3580b81d 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -344,6 +344,7 @@ class P: monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) runner._run_verification("edit") assert runner._recovery_mode is True + assert runner._primary_target_minimally_valid is False assert runner._failing_file == "legal_review_app.py" assert runner._active_solution_file == "legal_review_app.py" assert runner._primary_execution_target == "legal_review_app.py" @@ -432,6 +433,7 @@ class P: monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) runner._run_verification("edit") assert runner._primary_execution_target == "service.py" + assert runner._primary_target_minimally_valid is True def test_recovery_does_not_clear_until_primary_target_validates(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -460,6 +462,29 @@ class P: assert runner._recovery_mode is True +def test_mission_state_tracks_live_primary_target_and_recovery_mode(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + (tmp_path / "service.py").write_text("print('x')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._ensure_mission("fix service") + runner._verification_baseline_changed = set() + runner._current_verification_targets = {"service.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["service.py"]) + + class FailProc: + returncode = 1 + stdout = "" + stderr = 'Traceback (most recent call last):\n File "service.py", line 1\nRuntimeError: boom\n' + + monkeypatch.setattr(state_runtime.subprocess, "run", lambda cmd, **kwargs: FailProc()) + runner._run_verification("edit") + + assert runner._mission_state is not None + assert runner._mission_state.primary_execution_target == "service.py" + assert runner._mission_state.recovery_mode is True + assert runner._mission_state.primary_target_minimally_valid is False + + def test_changed_validation_state_emits_new_compact_summary( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index 9ad52199..c475655d 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -190,13 +190,22 @@ def test_recovery_mode_blocks_edit_without_read_first(tmp_path: Path) -> None: assert "read_required_before_edit" in str(result["content"]) -def test_recovery_mode_blocks_parallel_entrypoint_launch(tmp_path: Path) -> None: +def test_recovery_mode_blocks_new_validation_artifact_entrypoint_launch(tmp_path: Path) -> None: runner = _runner(tmp_path) (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") - (tmp_path / "web_server.py").write_text("print('x')\n", encoding="utf-8") runner._recovery_mode = True runner._active_solution_file = "web_app.py" runner._primary_execution_target = "web_app.py" + runner._recovery_files_at_failure = {"web_app.py"} + + create = execute_tool_with_policy( + runner, + "Write", + {"file_path": "web_server.py", "content": "print('x')\n"}, + "1a", + 0, + ) + assert create["is_error"] is False result = execute_tool_with_policy( runner, @@ -206,7 +215,7 @@ def test_recovery_mode_blocks_parallel_entrypoint_launch(tmp_path: Path) -> None 0, ) assert result["is_error"] is True - assert "recovery_entrypoint_switch_blocked" in str(result["content"]) + assert "recovery_new_validation_artifact_blocked" in str(result["content"]) assert "primary_execution_target=web_app.py" in str(result["content"]) diff --git a/villani_code/mission_state.py b/villani_code/mission_state.py index 8552f94d..41e08d37 100644 --- a/villani_code/mission_state.py +++ b/villani_code/mission_state.py @@ -52,6 +52,9 @@ class MissionState: validation_failures: list[str] = field(default_factory=list) last_failed_command: str = "" last_failed_summary: str = "" + recovery_mode: bool = False + primary_execution_target: str = "" + primary_target_minimally_valid: bool = False last_checkpoint_id: str = "" last_transcript_path: str = "" compact_summary: str = "" @@ -83,6 +86,9 @@ def from_dict(cls, payload: dict[str, Any]) -> "MissionState": validation_failures=[str(v) for v in payload.get("validation_failures", [])], last_failed_command=str(payload.get("last_failed_command", "")), last_failed_summary=str(payload.get("last_failed_summary", "")), + recovery_mode=bool(payload.get("recovery_mode", False)), + primary_execution_target=str(payload.get("primary_execution_target", "")), + primary_target_minimally_valid=bool(payload.get("primary_target_minimally_valid", False)), last_checkpoint_id=str(payload.get("last_checkpoint_id", "")), last_transcript_path=str(payload.get("last_transcript_path", "")), compact_summary=str(payload.get("compact_summary", "")), diff --git a/villani_code/state.py b/villani_code/state.py index 8af2233d..895107ac 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -533,11 +533,14 @@ def __init__( self._failing_file = "" self._active_solution_file = "" self._primary_execution_target = "" + self._primary_target_minimally_valid = False self._active_solution_last_validation_ok: bool | None = None self._active_solution_last_validation_summary = "" self._failing_error_summary = "" self._failing_command = "" self._file_was_read_since_failure = False + self._recovery_files_at_failure: set[str] = set() + self._recovery_created_artifacts: set[str] = set() self._context_governance = ContextGovernanceManager(self.repo) self._planning_read_only = False self._runtime_mode: Literal["execution", "planning"] = "execution" @@ -916,11 +919,14 @@ def run( self._failing_file = "" self._active_solution_file = "" self._primary_execution_target = "" + self._primary_target_minimally_valid = False self._active_solution_last_validation_ok = None self._active_solution_last_validation_summary = "" self._failing_error_summary = "" self._failing_command = "" self._file_was_read_since_failure = False + self._recovery_files_at_failure = set() + self._recovery_created_artifacts = set() if self._first_attempt_write_lock_active: self.event_callback( { @@ -1588,6 +1594,9 @@ def _update_mission_state(self, **fields: Any) -> None: if hasattr(self._mission_state, key): setattr(self._mission_state, key, value) self._mission_state.intended_targets = sorted(self._intended_targets) + self._mission_state.recovery_mode = bool(getattr(self, "_recovery_mode", False)) + self._mission_state.primary_execution_target = str(getattr(self, "_primary_execution_target", "")) + self._mission_state.primary_target_minimally_valid = bool(getattr(self, "_primary_target_minimally_valid", False)) if self._mission_state.status == "active": self._mission_state.changed_files = self._git_changed_files() save_mission_state(self.repo, self._mission_state) diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index 64462a0c..a4a7a5db 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -597,6 +597,28 @@ def _primary_execution_target(runner: Any) -> str: return str(getattr(runner, "_primary_execution_target", "")).replace("\\", "/").lstrip("./") +def _sync_recovery_state_to_mission(runner: Any) -> None: + mission_state = getattr(runner, "_mission_state", None) + if mission_state is None: + return + mission_state.recovery_mode = bool(getattr(runner, "_recovery_mode", False)) + mission_state.primary_execution_target = _primary_execution_target(runner) + mission_state.primary_target_minimally_valid = bool(getattr(runner, "_primary_target_minimally_valid", False)) + save_mission_state(runner.repo, mission_state) + + +def _snapshot_repo_files(repo: Path) -> set[str]: + collected: set[str] = set() + for path in repo.rglob("*"): + if not path.is_file(): + continue + rel = str(path.relative_to(repo)).replace("\\", "/").lstrip("./") + if rel.startswith(".git/") or rel.startswith(".villani_code/"): + continue + collected.add(rel) + return collected + + def _seed_primary_execution_target(runner: Any, target: str) -> str: normalized = _normalize_repo_path(target) if not normalized: @@ -605,6 +627,8 @@ def _seed_primary_execution_target(runner: Any, target: str) -> str: if current: return current runner._primary_execution_target = normalized + runner._primary_target_minimally_valid = bool(getattr(runner, "_active_solution_last_validation_ok", False)) + _sync_recovery_state_to_mission(runner) return normalized @@ -740,12 +764,18 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An and primary_target and target != primary_target ): - return _recovery_blocked_feedback( - "recovery_entrypoint_switch_blocked", - tool_name, - target, - f"primary_execution_target={primary_target}", + recovery_created_artifacts = set(getattr(runner, "_recovery_created_artifacts", set())) + recovery_files_at_failure = set(getattr(runner, "_recovery_files_at_failure", set())) + confidently_new_target = target in recovery_created_artifacts or ( + bool(recovery_files_at_failure) and target not in recovery_files_at_failure ) + if confidently_new_target: + return _recovery_blocked_feedback( + "recovery_new_validation_artifact_blocked", + tool_name, + target, + f"primary_execution_target={primary_target}", + ) constrained = runner.small_model or runner.villani_mode or runner.benchmark_config.enabled if not constrained: @@ -1264,6 +1294,9 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: else: runner._active_solution_last_validation_ok = True runner._active_solution_last_validation_summary = "validation passed" + if primary_target and active_solution_file and primary_target == active_solution_file: + runner._primary_target_minimally_valid = True + _sync_recovery_state_to_mission(runner) hard_failure = _detect_hard_failure(cmd_results) if hard_failure: @@ -1282,10 +1315,13 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: active_solution_file = failing_file primary_target = _seed_primary_execution_target(runner, active_solution_file or failing_file) runner._recovery_mode = True + runner._primary_target_minimally_valid = False runner._failing_file = failing_file runner._failing_error_summary = summary runner._failing_command = hard_failure.get("failing_command", "") runner._file_was_read_since_failure = False + runner._recovery_files_at_failure = _snapshot_repo_files(runner.repo) + runner._recovery_created_artifacts = set() if active_solution_file and failing_file == active_solution_file: runner._active_solution_last_validation_ok = False runner._active_solution_last_validation_summary = summary[:220] @@ -1310,25 +1346,39 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: "failing_command": runner._failing_command, } ) + _sync_recovery_state_to_mission(runner) elif bool(getattr(runner, "_recovery_mode", False)) and verification.status in {VerificationStatus.PASS, VerificationStatus.REPAIRED}: primary_target = _primary_execution_target(runner) validated_primary = False if primary_target: - validated_primary = any( - _command_targets_active_solution( + targeted_results = [ + result + for result in cmd_results + if _command_targets_active_solution( str(result.get("command", "")), primary_target, validation_targets, ) - for result in cmd_results + ] + validated_primary = bool(targeted_results) and all( + not _has_hard_failure_signal( + str(result.get("stdout", "") or ""), + str(result.get("stderr", "") or ""), + int(result.get("exit", 0)), + ) + for result in targeted_results ) - if not primary_target or validated_primary: + if validated_primary: + runner._primary_target_minimally_valid = True runner._recovery_mode = False runner._failing_file = "" runner._failing_error_summary = "" runner._failing_command = "" runner._file_was_read_since_failure = False + runner._recovery_files_at_failure = set() + runner._recovery_created_artifacts = set() runner.event_callback({"type": "recovery_mode_cleared"}) + _sync_recovery_state_to_mission(runner) finding_fingerprints = sorted( "|".join( [ diff --git a/villani_code/state_tooling.py b/villani_code/state_tooling.py index 1719aff4..48474cdc 100644 --- a/villani_code/state_tooling.py +++ b/villani_code/state_tooling.py @@ -662,6 +662,12 @@ def _debug_callback_with_turn(event_type: str, payload: dict[str, Any]) -> None: if callable(debug_callback): debug_callback(event_type, callback_payload) + write_target = "" + write_target_existed = False + if tool_name == "Write": + write_target = str(tool_input.get("file_path", "")).replace("\\", "/").lstrip("./") + if write_target: + write_target_existed = (runner.repo / write_target).exists() result = execute_tool( tool_name, tool_input, @@ -691,4 +697,14 @@ def _debug_callback_with_turn(event_type: str, payload: dict[str, Any]) -> None: read_path = str(tool_input.get("file_path", "")).replace("\\", "/").lstrip("./") if read_path and read_path == str(getattr(runner, "_failing_file", "")): runner._file_was_read_since_failure = True + if ( + tool_name == "Write" + and not result.get("is_error") + and bool(getattr(runner, "_recovery_mode", False)) + and write_target + and not write_target_existed + ): + created_artifacts = set(getattr(runner, "_recovery_created_artifacts", set())) + created_artifacts.add(write_target) + runner._recovery_created_artifacts = created_artifacts return result From 6dfade9af116d2a3d948493512980ca4664e849c Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 14:43:28 +1000 Subject: [PATCH 08/12] Arm live recovery on primary hard failures and block sibling target pivots --- tests/test_state_tooling_mutation_policy.py | 29 ++++++++- villani_code/state.py | 17 +++++ villani_code/state_runtime.py | 70 +++++++++++++++++++-- villani_code/state_tooling.py | 18 ++++++ 4 files changed, 125 insertions(+), 9 deletions(-) diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index c475655d..b9f5d077 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -3,7 +3,7 @@ from pathlib import Path from villani_code.state import Runner -from villani_code.state_tooling import execute_tool_with_policy +from villani_code.state_tooling import execute_tool_with_lifecycle, execute_tool_with_policy class _Client: @@ -215,8 +215,10 @@ def test_recovery_mode_blocks_new_validation_artifact_entrypoint_launch(tmp_path 0, ) assert result["is_error"] is True - assert "recovery_new_validation_artifact_blocked" in str(result["content"]) - assert "primary_execution_target=web_app.py" in str(result["content"]) + assert '"classification": "blocked"' in str(result["content"]) + assert '"short_reason": "recovery_target_switch_blocked"' in str(result["content"]) + assert '"primary_execution_target": "web_app.py"' in str(result["content"]) + assert '"attempted_target": "web_server.py"' in str(result["content"]) def test_recovery_mode_allows_rerun_of_same_primary_target(tmp_path: Path) -> None: @@ -236,6 +238,27 @@ def test_recovery_mode_allows_rerun_of_same_primary_target(tmp_path: Path) -> No assert result["is_error"] is False +def test_bash_hard_failure_on_primary_sets_live_recovery_state(tmp_path: Path) -> None: + runner = _runner(tmp_path) + (tmp_path / "web_app.py").write_text("def broken(:\n pass\n", encoding="utf-8") + runner._primary_execution_target = "web_app.py" + runner._active_solution_file = "web_app.py" + runner._recovery_mode = False + + result = execute_tool_with_lifecycle( + runner=runner, + tool_name="Bash", + tool_input={"command": "python web_app.py", "timeout_sec": 30}, + tool_use_id="fail-1", + turn_index=0, + ) + assert result["is_error"] is False + assert runner._recovery_mode is True + assert runner._failing_file == "web_app.py" + assert runner._primary_execution_target == "web_app.py" + assert runner._primary_target_minimally_valid is False + + def test_recovery_mode_allows_helper_file_edit_with_active_solution(tmp_path: Path) -> None: runner = _runner(tmp_path) (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") diff --git a/villani_code/state.py b/villani_code/state.py index 895107ac..c5b38006 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -1406,6 +1406,23 @@ def _budget_reason( trigger=f"{tool_name} execution" ) elif tool_name == "Bash": + try: + decoded = json.loads(str(result.get("content", ""))) + except Exception: + decoded = {} + if isinstance(decoded, dict): + from villani_code import state_runtime + + state_runtime.activate_live_recovery_on_primary_failure( + self, + command=str(decoded.get("command", "") or str(tool_input.get("command", ""))), + exit_code=int(decoded.get("exit_code", 0)), + stdout=str(decoded.get("stdout", "") or ""), + stderr=str(decoded.get("stderr", "") or ""), + attempted_target=state_runtime._extract_command_python_target( + str(decoded.get("command", "") or str(tool_input.get("command", ""))) + ), + ) self._pending_verification = self._run_verification( trigger=f"{tool_name} execution" ) diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index a4a7a5db..c504cc79 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -546,6 +546,17 @@ def _recovery_blocked_feedback(reason: str, tool: str, path: str, detail: str = return json.dumps(payload, sort_keys=True) +def _recovery_target_switch_blocked_feedback(primary_target: str, attempted_target: str) -> str: + payload = { + "classification": "blocked", + "short_reason": "recovery_target_switch_blocked", + "primary_execution_target": primary_target, + "attempted_target": attempted_target, + "recovery_blocked": True, + } + return json.dumps(payload, sort_keys=True) + + def _patch_deletes_target_file(unified_diff: str, failing_file: str) -> bool: try: parsed = parse_unified_diff(unified_diff) @@ -690,6 +701,57 @@ def _has_hard_failure_signal(stdout: str, stderr: str, exit_code: int) -> bool: ) +def activate_live_recovery_on_primary_failure( + runner: Any, + *, + command: str, + exit_code: int, + stdout: str, + stderr: str, + attempted_target: str = "", +) -> bool: + normalized_target = _normalize_repo_path(attempted_target) + primary_target = _seed_primary_execution_target(runner, normalized_target) if normalized_target else _primary_execution_target(runner) + if not normalized_target or not primary_target or normalized_target != primary_target: + return False + if not _is_validation_or_launch_command(command): + return False + if not _has_hard_failure_signal(stdout, stderr, exit_code): + return False + evidence = parse_failure_signal(stdout, stderr) + summary = str(evidence.get("error_summary", "")).strip() + if not summary: + summary = (stderr or stdout or f"Command failed with exit {exit_code}").splitlines()[0][:220] + traceback_file = _normalize_repo_path(str(evidence.get("traceback_file", "")).strip()) + if traceback_file and (traceback_file == primary_target or traceback_file.endswith(f"/{primary_target}")): + failing_file = primary_target + else: + failing_file = primary_target + runner._recovery_mode = True + runner._primary_target_minimally_valid = False + runner._failing_file = failing_file + runner._failing_error_summary = summary[:220] + runner._failing_command = str(command or "")[:200] + runner._file_was_read_since_failure = False + runner._recovery_files_at_failure = _snapshot_repo_files(runner.repo) + runner._recovery_created_artifacts = set() + runner._active_solution_file = primary_target + runner._active_solution_last_validation_ok = False + runner._active_solution_last_validation_summary = summary[:220] + runner.event_callback( + { + "type": "recovery_mode_activated", + "failing_file": failing_file, + "active_solution_file": primary_target, + "primary_execution_target": primary_target, + "failing_error_summary": summary[:220], + "failing_command": runner._failing_command, + } + ) + _sync_recovery_state_to_mission(runner) + return True + + def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, Any]) -> str | None: if tool_name in {"Write", "Patch"} and bool(getattr(runner, "_recovery_mode", False)): failing_file = str(getattr(runner, "_failing_file", "")).replace("\\", "/").lstrip("./") @@ -763,6 +825,7 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An and target and primary_target and target != primary_target + and not bool(getattr(runner, "_primary_target_minimally_valid", False)) ): recovery_created_artifacts = set(getattr(runner, "_recovery_created_artifacts", set())) recovery_files_at_failure = set(getattr(runner, "_recovery_files_at_failure", set())) @@ -770,12 +833,7 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An bool(recovery_files_at_failure) and target not in recovery_files_at_failure ) if confidently_new_target: - return _recovery_blocked_feedback( - "recovery_new_validation_artifact_blocked", - tool_name, - target, - f"primary_execution_target={primary_target}", - ) + return _recovery_target_switch_blocked_feedback(primary_target, target) constrained = runner.small_model or runner.villani_mode or runner.benchmark_config.enabled if not constrained: diff --git a/villani_code/state_tooling.py b/villani_code/state_tooling.py index 48474cdc..f7e5404a 100644 --- a/villani_code/state_tooling.py +++ b/villani_code/state_tooling.py @@ -1,6 +1,7 @@ from __future__ import annotations import difflib +import json import py_compile import re from dataclasses import dataclass @@ -689,6 +690,23 @@ def _debug_callback_with_turn(event_type: str, payload: dict[str, Any]) -> None: **({"forced": True} if forced else {}), } ) + if tool_name == "Bash" and not result.get("is_error"): + try: + decoded = json.loads(str(result.get("content", ""))) + except Exception: + decoded = {} + if isinstance(decoded, dict): + from villani_code import state_runtime + + command = str(decoded.get("command", "") or str(tool_input.get("command", ""))) + state_runtime.activate_live_recovery_on_primary_failure( + runner, + command=command, + exit_code=int(decoded.get("exit_code", 0)), + stdout=str(decoded.get("stdout", "") or ""), + stderr=str(decoded.get("stderr", "") or ""), + attempted_target=state_runtime._extract_command_python_target(command), + ) if ( tool_name == "Read" and not result.get("is_error") From 0e7d9707107eff518b1a89f586fbd603fa18c775 Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 14:58:00 +1000 Subject: [PATCH 09/12] Block unsafe cmd detached launch patterns for long-running commands --- tests/test_shell_command_guard.py | 47 +++++++++++++++++++++++++++++++ tests/test_state_execution.py | 13 +++++++++ villani_code/shells.py | 28 ++++++++++++++++++ villani_code/state_execution.py | 2 ++ 4 files changed, 90 insertions(+) diff --git a/tests/test_shell_command_guard.py b/tests/test_shell_command_guard.py index f94f2e76..ceea2111 100644 --- a/tests/test_shell_command_guard.py +++ b/tests/test_shell_command_guard.py @@ -91,3 +91,50 @@ def test_blocked_commands_produce_compact_structured_feedback(tmp_path: Path) -> assert payload["offending_token"] == "<<" assert payload["offending_pattern"] == "< None: + decision = classify_and_rewrite_command("python app.py > out.txt 2>&1 & echo STARTED", "cmd") + assert decision.classification == "blocked" + assert decision.offending_pattern + assert "detached" in decision.short_reason + + +def test_cmd_invalid_detached_launch_feedback_is_compact_structured(tmp_path: Path) -> None: + result = execute_tool( + "Bash", + {"command": "python app.py > out.txt 2>&1 & echo STARTED", "cwd": ".", "timeout_sec": 5}, + tmp_path, + runtime_state={"shell_environment": {"shell_family": "cmd"}}, + ) + assert result["is_error"] is True + payload = json.loads(result["content"]) + assert payload["shell_family"] == "cmd" + assert payload["classification"] == "blocked" + assert payload["short_reason"] == "detached/background launch syntax is not safe in cmd" + assert payload["offending_pattern"] + assert set(payload) == {"shell_family", "classification", "short_reason", "offending_pattern"} + + +def test_cmd_ordinary_command_unaffected() -> None: + decision = classify_and_rewrite_command("echo hello", "cmd") + assert decision.classification == "allowed" + assert decision.command == "echo hello" + + +def test_cmd_blocked_detached_launch_does_not_execute(tmp_path: Path, monkeypatch) -> None: + called = {"value": False} + + def _fail_if_called(*_args, **_kwargs): + called["value"] = True + raise AssertionError("subprocess.run should not be called for blocked detached cmd launch") + + monkeypatch.setattr("villani_code.tools.subprocess.run", _fail_if_called) + result = execute_tool( + "Bash", + {"command": "python app.py > out.txt 2>&1 & echo STARTED", "cwd": ".", "timeout_sec": 5}, + tmp_path, + runtime_state={"shell_environment": {"shell_family": "cmd"}}, + ) + assert result["is_error"] is True + assert called["value"] is False diff --git a/tests/test_state_execution.py b/tests/test_state_execution.py index 1b2fc048..d1a697c3 100644 --- a/tests/test_state_execution.py +++ b/tests/test_state_execution.py @@ -27,3 +27,16 @@ def test_collect_validation_artifacts_keeps_clean_command() -> None: } artifacts = collect_validation_artifacts(transcript) assert artifacts == ["python web_app.py (exit=0)"] + + +def test_collect_validation_artifacts_skips_error_results_even_with_command_shape() -> None: + transcript = { + "tool_results": [ + { + "content": '{"command":"python app.py > out.txt 2>&1 & echo STARTED","exit_code":0}', + "is_error": True, + } + ] + } + artifacts = collect_validation_artifacts(transcript) + assert artifacts == [] diff --git a/villani_code/shells.py b/villani_code/shells.py index 066cc241..db4edf48 100644 --- a/villani_code/shells.py +++ b/villani_code/shells.py @@ -102,6 +102,14 @@ def classify_and_rewrite_command(command: str, shell_family: str) -> ShellComman lowered = raw.lower() if shell_family == "cmd": + detached_pattern = _detect_invalid_cmd_detached_launch(raw) + if detached_pattern: + return ShellCommandDecision( + classification="blocked", + command=raw, + offending_pattern=detached_pattern, + short_reason="detached/background launch syntax is not safe in cmd", + ) if _has_bash_heredoc(raw): return ShellCommandDecision( classification="blocked", @@ -179,6 +187,26 @@ def _has_bash_heredoc(command: str) -> bool: return bool(re.search(r"<<\s*'?\w+'?", command, re.IGNORECASE)) +def _detect_invalid_cmd_detached_launch(command: str) -> str: + raw = str(command or "").strip() + if not raw: + return "" + lowered = raw.lower() + if not re.search(r"\s&\s", raw): + return "" + if re.search(r"\s&\s*echo\b", lowered): + if re.search(r"\b(start\s+\"[^\"]*\"\s+/b|start\s+/b)\b", lowered): + return "" + if re.search(r"\s*>\s*\S+\s+2>&1\s+&\s*", lowered) or re.search(r"\s*>\s*\S+\s+&\s*", lowered): + return "& echo after redirection" + return "& echo launch marker" + if re.search(r"\s*>\s*\S+\s+2>&1\s+&\s*", lowered): + return "2>&1 & command chaining" + if lowered.endswith("&"): + return "trailing & background assumption" + return "" + + def _rewrite_for_cmd(command: str) -> str: rm_match = re.match(r"^\s*rm\s+(.+)$", command) if rm_match: diff --git a/villani_code/state_execution.py b/villani_code/state_execution.py index 36b038d1..4d4f9215 100644 --- a/villani_code/state_execution.py +++ b/villani_code/state_execution.py @@ -38,6 +38,8 @@ def summarize_changes(changed_files: list[str]) -> ChangeSummary: def collect_validation_artifacts(transcript: dict[str, Any]) -> list[str]: artifacts: list[str] = [] for tool_result in transcript.get("tool_results", []): + if bool(tool_result.get("is_error")): + continue for record in parse_command_evidence(str(tool_result.get("content", ""))): if _has_masking_pattern(str(record.get("command", ""))): continue From a8562cd0f58b86fa66646725447682f7d9929a5d Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 15:27:13 +1000 Subject: [PATCH 10/12] Align runtime target/recovery state with direct execution evidence --- tests/test_shell_command_guard.py | 12 ++ tests/test_state_runtime.py | 74 +++++++++ tests/test_state_tooling_mutation_policy.py | 45 +++++- villani_code/context_projection.py | 22 +++ villani_code/mission_state.py | 4 + villani_code/shells.py | 27 +++- villani_code/state.py | 17 +++ villani_code/state_runtime.py | 158 +++++++++++++++----- villani_code/state_tooling.py | 10 +- 9 files changed, 321 insertions(+), 48 deletions(-) diff --git a/tests/test_shell_command_guard.py b/tests/test_shell_command_guard.py index ceea2111..738a81fd 100644 --- a/tests/test_shell_command_guard.py +++ b/tests/test_shell_command_guard.py @@ -65,6 +65,13 @@ def test_cmd_classifier_scans_full_command_string(tmp_path: Path) -> None: assert decision.classification == "blocked" +def test_cmd_embedded_tail_or_wc_fragments_are_blocked(tmp_path: Path) -> None: + tail_decision = classify_and_rewrite_command("echo ok && type app.log | tail -20", "cmd") + wc_decision = classify_and_rewrite_command("echo ok && type app.log | wc -l", "cmd") + assert tail_decision.classification == "blocked" + assert wc_decision.classification == "blocked" + + def test_powershell_grep_rewrites_to_select_string(tmp_path: Path) -> None: decision = classify_and_rewrite_command("grep hello notes.txt", "powershell") assert decision.classification == "needs_rewrite" @@ -100,6 +107,11 @@ def test_cmd_invalid_detached_launch_pattern_is_blocked() -> None: assert "detached" in decision.short_reason +def test_cmd_ampersand_detached_assumption_is_blocked() -> None: + decision = classify_and_rewrite_command("python app.py & dir", "cmd") + assert decision.classification == "blocked" + + def test_cmd_invalid_detached_launch_feedback_is_compact_structured(tmp_path: Path) -> None: result = execute_tool( "Bash", diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index 3580b81d..55177200 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -7,6 +7,7 @@ from villani_code.state import Runner from villani_code import state_runtime +from villani_code.context_projection import build_model_context_packet class DummyClient: @@ -133,6 +134,37 @@ def test_prepare_messages_for_model_injects_cmd_shell_reminder() -> None: assert prepared[-1]["content"][0]["text"].startswith("Shell: Windows cmd.") +def test_model_context_packet_includes_recovery_block() -> None: + runner = SimpleNamespace( + _mission_state=SimpleNamespace( + objective="o", + mode="execution", + current_step_id="", + plan_summary="", + verified_facts=[], + open_hypotheses=[], + intended_targets=[], + changed_files=[], + last_failed_command="", + validation_failures=[], + compact_summary="", + ), + _task_contract={}, + skills=[], + repo=Path("."), + _runtime_mode="execution", + _primary_execution_target="service.py", + _primary_execution_target_cwd=".", + _recovery_mode=True, + _failing_target_contract_summary="service.py @ .", + _primary_target_minimally_valid=False, + _recovery_target_switch_blocked=True, + ) + packet = build_model_context_packet(runner) + assert packet["recovery"]["recovery_mode"] is True + assert packet["recovery"]["primary_target_contract"]["target"] == "service.py" + + def test_validate_anthropic_tool_sequence_rejects_text_after_tool_result() -> None: messages = [ { @@ -436,6 +468,48 @@ class P: assert runner._primary_target_minimally_valid is True +def test_single_file_write_seed_is_replaceable_by_stronger_direct_signal(tmp_path: Path) -> None: + _seed_repo(tmp_path) + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + state_runtime._seed_primary_execution_target(runner, "__init__.py", cwd=str(tmp_path), evidence="write_only") + state_runtime._seed_primary_execution_target(runner, "service.py", cwd=str(tmp_path), evidence="direct_run") + assert runner._primary_execution_target == "service.py" + assert runner._primary_execution_target_evidence == "direct_run" + + +def test_primary_target_contract_distinguishes_same_file_different_cwd(tmp_path: Path) -> None: + _seed_repo(tmp_path) + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + state_runtime._seed_primary_execution_target(runner, "service.py", cwd=str(tmp_path / "a"), evidence="direct_validation") + state_runtime._seed_primary_execution_target(runner, "service.py", cwd=str(tmp_path / "b"), evidence="direct_validation") + assert runner._primary_execution_target == "service.py" + assert runner._primary_execution_target_cwd == "a" + + +def test_hard_failure_activation_requires_same_primary_contract(tmp_path: Path) -> None: + _seed_repo(tmp_path) + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + state_runtime._seed_primary_execution_target(runner, "service.py", cwd=str(tmp_path / "svc"), evidence="direct_run") + armed = state_runtime.activate_live_recovery_on_primary_failure( + runner, + command="python service.py", + exit_code=1, + stdout="", + stderr="Traceback (most recent call last):\n File \"service.py\", line 1\nNameError: x\n", + attempted_target="service.py", + attempted_cwd=str(tmp_path / "other"), + ) + assert armed is False + + +def test_live_validation_candidates_refresh_after_material_changes(tmp_path: Path) -> None: + _seed_repo(tmp_path) + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._current_verification_targets = {"old.py"} + state_runtime.refresh_live_validation_candidates(runner, ["new_script.py"]) + assert runner._current_verification_targets == {"old.py", "new_script.py"} + + def test_recovery_does_not_clear_until_primary_target_validates(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: _seed_repo(tmp_path) (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index b9f5d077..79038d46 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -216,7 +216,10 @@ def test_recovery_mode_blocks_new_validation_artifact_entrypoint_launch(tmp_path ) assert result["is_error"] is True assert '"classification": "blocked"' in str(result["content"]) - assert '"short_reason": "recovery_target_switch_blocked"' in str(result["content"]) + assert ( + '"short_reason": "recovery_target_switch_blocked"' in str(result["content"]) + or '"short_reason": "recovery_new_wrapper_target_blocked"' in str(result["content"]) + ) assert '"primary_execution_target": "web_app.py"' in str(result["content"]) assert '"attempted_target": "web_server.py"' in str(result["content"]) @@ -238,6 +241,46 @@ def test_recovery_mode_allows_rerun_of_same_primary_target(tmp_path: Path) -> No assert result["is_error"] is False +def test_recovery_e2e_blocks_drift_after_scaffold_then_real_target_failure(tmp_path: Path) -> None: + runner = _runner(tmp_path) + scaffold = execute_tool_with_policy( + runner, + "Write", + {"file_path": "app/__init__.py", "content": ""}, + "s1", + 0, + ) + assert scaffold["is_error"] is False + assert runner._primary_execution_target == "" + + (tmp_path / "service.py").write_text("def broken(:\n pass\n", encoding="utf-8") + runner._primary_execution_target = "service.py" + runner._primary_execution_target_cwd = "." + runner._primary_execution_target_evidence = "direct_run" + failed = execute_tool_with_lifecycle( + runner=runner, + tool_name="Bash", + tool_input={"command": "python service.py", "timeout_sec": 30}, + tool_use_id="b1", + turn_index=0, + ) + assert failed["is_error"] is False + assert runner._recovery_mode is True + + blocked = execute_tool_with_policy( + runner, + "Bash", + {"command": "python helper.py", "timeout_sec": 30}, + "b2", + 1, + ) + assert blocked["is_error"] is True + assert ( + "recovery_target_switch_blocked" in str(blocked["content"]) + or "recovery_new_wrapper_target_blocked" in str(blocked["content"]) + ) + + def test_bash_hard_failure_on_primary_sets_live_recovery_state(tmp_path: Path) -> None: runner = _runner(tmp_path) (tmp_path / "web_app.py").write_text("def broken(:\n pass\n", encoding="utf-8") diff --git a/villani_code/context_projection.py b/villani_code/context_projection.py index 20cec6d7..e7d9867e 100644 --- a/villani_code/context_projection.py +++ b/villani_code/context_projection.py @@ -28,6 +28,16 @@ def build_model_context_packet(runner: "Runner") -> dict[str, Any]: constraints.append(f"Success predicate: {contract.get('success_predicate', '')}") constraints.extend([f"No-go: {p}" for p in contract.get("no_go_paths", [])[:4]]) skill_guidance = [getattr(skill, "guidance", "") for skill in getattr(runner, "skills", []) if getattr(skill, "guidance", "")] + recovery_block = { + "primary_target_contract": { + "target": str(getattr(runner, "_primary_execution_target", "")), + "cwd": str(getattr(runner, "_primary_execution_target_cwd", "") or "."), + }, + "recovery_mode": bool(getattr(runner, "_recovery_mode", False)), + "failing_target_summary": str(getattr(runner, "_failing_target_contract_summary", "") or getattr(runner, "_failing_error_summary", "")), + "primary_target_minimally_valid": bool(getattr(runner, "_primary_target_minimally_valid", False)), + "switch_blocked": bool(getattr(runner, "_recovery_target_switch_blocked", False)), + } return { "objective": getattr(mission, "objective", ""), "runtime_mode": getattr(mission, "mode", getattr(runner, "_runtime_mode", "execution")), @@ -43,6 +53,7 @@ def build_model_context_packet(runner: "Runner") -> dict[str, Any]: "constraints": constraints, "repo_root": str(getattr(runner, "repo", "")), "skill_guidance": [s for s in skill_guidance if s][:8], + "recovery": recovery_block, } @@ -67,4 +78,15 @@ def render_model_context_packet(packet: dict[str, Any]) -> str: if guidance: lines.append("Skill guidance:") lines.extend(f"- {g}" for g in guidance[:6]) + recovery = packet.get("recovery", {}) + if isinstance(recovery, dict): + contract = recovery.get("primary_target_contract", {}) + lines.append( + "Recovery: " + f"mode={bool(recovery.get('recovery_mode', False))}; " + f"primary={contract.get('target', '')}@{contract.get('cwd', '.')}; " + f"min_valid={bool(recovery.get('primary_target_minimally_valid', False))}; " + f"switch_blocked={bool(recovery.get('switch_blocked', False))}; " + f"failure={recovery.get('failing_target_summary', '')}" + ) return "\n".join(lines) diff --git a/villani_code/mission_state.py b/villani_code/mission_state.py index 41e08d37..fc6d6044 100644 --- a/villani_code/mission_state.py +++ b/villani_code/mission_state.py @@ -54,6 +54,8 @@ class MissionState: last_failed_summary: str = "" recovery_mode: bool = False primary_execution_target: str = "" + primary_execution_cwd: str = "" + primary_execution_evidence: str = "none" primary_target_minimally_valid: bool = False last_checkpoint_id: str = "" last_transcript_path: str = "" @@ -88,6 +90,8 @@ def from_dict(cls, payload: dict[str, Any]) -> "MissionState": last_failed_summary=str(payload.get("last_failed_summary", "")), recovery_mode=bool(payload.get("recovery_mode", False)), primary_execution_target=str(payload.get("primary_execution_target", "")), + primary_execution_cwd=str(payload.get("primary_execution_cwd", "")), + primary_execution_evidence=str(payload.get("primary_execution_evidence", "none")), primary_target_minimally_valid=bool(payload.get("primary_target_minimally_valid", False)), last_checkpoint_id=str(payload.get("last_checkpoint_id", "")), last_transcript_path=str(payload.get("last_transcript_path", "")), diff --git a/villani_code/shells.py b/villani_code/shells.py index db4edf48..735f422e 100644 --- a/villani_code/shells.py +++ b/villani_code/shells.py @@ -136,6 +136,23 @@ def classify_and_rewrite_command(command: str, shell_family: str) -> ShellComman offending_pattern=" embedded head ", short_reason="unix head token is not valid in cmd", ) + embedded_tail_match = re.search(r"(^|[|;&()\n\t ])tail(?:\s|$)", lowered) + if embedded_tail_match and not re.match(r"^\s*tail(?:\s|$)", lowered): + return ShellCommandDecision( + classification="blocked", + command=raw, + offending_token="tail", + offending_pattern=" embedded tail ", + short_reason="unix tail token is not valid in cmd", + ) + if re.search(r"(^|[|;&()\n\t ])wc(?:\s|$)", lowered): + return ShellCommandDecision( + classification="blocked", + command=raw, + offending_token="wc", + offending_pattern=" embedded wc ", + short_reason="unix wc token is not valid in cmd", + ) if re.search(r"\|\s*(ForEach-Object|Where-Object|Select-Object)\b", raw, re.IGNORECASE): return ShellCommandDecision( classification="blocked", @@ -192,7 +209,13 @@ def _detect_invalid_cmd_detached_launch(command: str) -> str: if not raw: return "" lowered = raw.lower() - if not re.search(r"\s&\s", raw): + if "&" not in raw: + return "" + if "&&" in raw: + cleaned = raw.replace("&&", "") + else: + cleaned = raw + if "&" not in cleaned: return "" if re.search(r"\s&\s*echo\b", lowered): if re.search(r"\b(start\s+\"[^\"]*\"\s+/b|start\s+/b)\b", lowered): @@ -202,7 +225,7 @@ def _detect_invalid_cmd_detached_launch(command: str) -> str: return "& echo launch marker" if re.search(r"\s*>\s*\S+\s+2>&1\s+&\s*", lowered): return "2>&1 & command chaining" - if lowered.endswith("&"): + if lowered.endswith("&") or re.search(r"\s&\s+\S", raw): return "trailing & background assumption" return "" diff --git a/villani_code/state.py b/villani_code/state.py index c5b38006..3883ce68 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -533,6 +533,8 @@ def __init__( self._failing_file = "" self._active_solution_file = "" self._primary_execution_target = "" + self._primary_execution_target_cwd = "" + self._primary_execution_target_evidence = "none" self._primary_target_minimally_valid = False self._active_solution_last_validation_ok: bool | None = None self._active_solution_last_validation_summary = "" @@ -541,6 +543,8 @@ def __init__( self._file_was_read_since_failure = False self._recovery_files_at_failure: set[str] = set() self._recovery_created_artifacts: set[str] = set() + self._recovery_target_switch_blocked = False + self._failing_target_contract_summary = "" self._context_governance = ContextGovernanceManager(self.repo) self._planning_read_only = False self._runtime_mode: Literal["execution", "planning"] = "execution" @@ -919,6 +923,8 @@ def run( self._failing_file = "" self._active_solution_file = "" self._primary_execution_target = "" + self._primary_execution_target_cwd = "" + self._primary_execution_target_evidence = "none" self._primary_target_minimally_valid = False self._active_solution_last_validation_ok = None self._active_solution_last_validation_summary = "" @@ -927,6 +933,8 @@ def run( self._file_was_read_since_failure = False self._recovery_files_at_failure = set() self._recovery_created_artifacts = set() + self._recovery_target_switch_blocked = False + self._failing_target_contract_summary = "" if self._first_attempt_write_lock_active: self.event_callback( { @@ -1024,6 +1032,13 @@ def _finish_bounded( for block in response.get("content", []) if block.get("type") == "text" ) + primary_target = str(getattr(self, "_primary_execution_target", "")).strip() + primary_ok = getattr(self, "_primary_target_minimally_valid", False) + if primary_target and not primary_ok: + final_text = ( + "[Execution status] Incomplete: primary execution target does not yet have clean direct validation.\n\n" + + final_text + ).strip() execution = ExecutionResult( final_text=final_text, turns_used=turns_used, @@ -1613,6 +1628,8 @@ def _update_mission_state(self, **fields: Any) -> None: self._mission_state.intended_targets = sorted(self._intended_targets) self._mission_state.recovery_mode = bool(getattr(self, "_recovery_mode", False)) self._mission_state.primary_execution_target = str(getattr(self, "_primary_execution_target", "")) + self._mission_state.primary_execution_cwd = str(getattr(self, "_primary_execution_target_cwd", "")) + self._mission_state.primary_execution_evidence = str(getattr(self, "_primary_execution_target_evidence", "none")) self._mission_state.primary_target_minimally_valid = bool(getattr(self, "_primary_target_minimally_valid", False)) if self._mission_state.status == "active": self._mission_state.changed_files = self._git_changed_files() diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index c504cc79..f89f94c8 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -546,10 +546,10 @@ def _recovery_blocked_feedback(reason: str, tool: str, path: str, detail: str = return json.dumps(payload, sort_keys=True) -def _recovery_target_switch_blocked_feedback(primary_target: str, attempted_target: str) -> str: +def _recovery_target_switch_blocked_feedback(primary_target: str, attempted_target: str, *, reason: str = "recovery_target_switch_blocked") -> str: payload = { "classification": "blocked", - "short_reason": "recovery_target_switch_blocked", + "short_reason": reason, "primary_execution_target": primary_target, "attempted_target": attempted_target, "recovery_blocked": True, @@ -608,12 +608,48 @@ def _primary_execution_target(runner: Any) -> str: return str(getattr(runner, "_primary_execution_target", "")).replace("\\", "/").lstrip("./") +_TARGET_EVIDENCE_ORDER = {"none": 0, "write_only": 1, "indirect_validation": 2, "direct_validation": 3, "direct_run": 4} + + +def _normalize_contract_cwd(runner: Any, cwd: str | None) -> str: + raw = str(cwd or "").strip() + if not raw: + return "." + try: + repo_root = Path(getattr(runner, "repo", ".")).resolve() + resolved = Path(raw).resolve() + return str(resolved.relative_to(repo_root)).replace("\\", "/").lstrip("./") or "." + except Exception: + return _normalize_repo_path(raw) or "." + + +def _target_contract(runner: Any, target: str, cwd: str | None = None) -> dict[str, str]: + return {"target": _normalize_repo_path(target), "cwd": _normalize_contract_cwd(runner, cwd)} + + +def _primary_execution_contract(runner: Any) -> dict[str, str]: + return { + "target": _primary_execution_target(runner), + "cwd": _normalize_contract_cwd(runner, str(getattr(runner, "_primary_execution_target_cwd", "") or ".")), + } + + +def _same_target_contract(a: dict[str, str], b: dict[str, str]) -> bool: + return str(a.get("target", "")) == str(b.get("target", "")) and str(a.get("cwd", ".")) == str(b.get("cwd", ".")) + + +def _target_contract_summary(contract: dict[str, str]) -> str: + return f"{contract.get('target', '') or 'none'} @ {contract.get('cwd', '.') or '.'}" + + def _sync_recovery_state_to_mission(runner: Any) -> None: mission_state = getattr(runner, "_mission_state", None) if mission_state is None: return mission_state.recovery_mode = bool(getattr(runner, "_recovery_mode", False)) mission_state.primary_execution_target = _primary_execution_target(runner) + mission_state.primary_execution_cwd = str(getattr(runner, "_primary_execution_target_cwd", "") or ".") + mission_state.primary_execution_evidence = str(getattr(runner, "_primary_execution_target_evidence", "none")) mission_state.primary_target_minimally_valid = bool(getattr(runner, "_primary_target_minimally_valid", False)) save_mission_state(runner.repo, mission_state) @@ -630,14 +666,23 @@ def _snapshot_repo_files(repo: Path) -> set[str]: return collected -def _seed_primary_execution_target(runner: Any, target: str) -> str: - normalized = _normalize_repo_path(target) +def _seed_primary_execution_target(runner: Any, target: str, *, cwd: str | None = None, evidence: str = "write_only") -> str: + contract = _target_contract(runner, target, cwd) + normalized = contract["target"] if not normalized: return _primary_execution_target(runner) - current = _primary_execution_target(runner) - if current: - return current + current = _primary_execution_contract(runner) + current_evidence = str(getattr(runner, "_primary_execution_target_evidence", "none")) + current_strength = _TARGET_EVIDENCE_ORDER.get(current_evidence, 0) + incoming_strength = _TARGET_EVIDENCE_ORDER.get(str(evidence or "none"), 0) + if current["target"]: + if incoming_strength < current_strength: + return current["target"] + if incoming_strength == current_strength and not _same_target_contract(current, contract): + return current["target"] runner._primary_execution_target = normalized + runner._primary_execution_target_cwd = contract["cwd"] + runner._primary_execution_target_evidence = str(evidence or "none") runner._primary_target_minimally_valid = bool(getattr(runner, "_active_solution_last_validation_ok", False)) _sync_recovery_state_to_mission(runner) return normalized @@ -687,7 +732,7 @@ def _command_targets_active_solution(command: str, active_solution_file: str, va cmd = str(command or "") if active in cmd: return True - return active in normalized_targets + return len(normalized_targets) == 1 and active in normalized_targets def _has_hard_failure_signal(stdout: str, stderr: str, exit_code: int) -> bool: @@ -709,10 +754,11 @@ def activate_live_recovery_on_primary_failure( stdout: str, stderr: str, attempted_target: str = "", + attempted_cwd: str | None = None, ) -> bool: - normalized_target = _normalize_repo_path(attempted_target) - primary_target = _seed_primary_execution_target(runner, normalized_target) if normalized_target else _primary_execution_target(runner) - if not normalized_target or not primary_target or normalized_target != primary_target: + attempted_contract = _target_contract(runner, attempted_target, attempted_cwd) + primary_contract = _primary_execution_contract(runner) + if not attempted_contract["target"] or not primary_contract["target"] or not _same_target_contract(attempted_contract, primary_contract): return False if not _is_validation_or_launch_command(command): return False @@ -723,27 +769,30 @@ def activate_live_recovery_on_primary_failure( if not summary: summary = (stderr or stdout or f"Command failed with exit {exit_code}").splitlines()[0][:220] traceback_file = _normalize_repo_path(str(evidence.get("traceback_file", "")).strip()) - if traceback_file and (traceback_file == primary_target or traceback_file.endswith(f"/{primary_target}")): - failing_file = primary_target + if traceback_file and (traceback_file == primary_contract["target"] or traceback_file.endswith(f"/{primary_contract['target']}")): + failing_file = primary_contract["target"] else: - failing_file = primary_target + failing_file = primary_contract["target"] runner._recovery_mode = True + runner._recovery_target_switch_blocked = False runner._primary_target_minimally_valid = False runner._failing_file = failing_file + runner._failing_target_contract_summary = _target_contract_summary(primary_contract) runner._failing_error_summary = summary[:220] runner._failing_command = str(command or "")[:200] runner._file_was_read_since_failure = False runner._recovery_files_at_failure = _snapshot_repo_files(runner.repo) runner._recovery_created_artifacts = set() - runner._active_solution_file = primary_target + runner._active_solution_file = primary_contract["target"] runner._active_solution_last_validation_ok = False runner._active_solution_last_validation_summary = summary[:220] runner.event_callback( { "type": "recovery_mode_activated", "failing_file": failing_file, - "active_solution_file": primary_target, - "primary_execution_target": primary_target, + "active_solution_file": primary_contract["target"], + "primary_execution_target": primary_contract["target"], + "primary_execution_cwd": primary_contract["cwd"], "failing_error_summary": summary[:220], "failing_command": runner._failing_command, } @@ -813,27 +862,31 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An if _is_validation_or_launch_command(command) and _command_has_masking_patterns(command): return _recovery_blocked_feedback("suspicious_validation_command", tool_name, "", "masked failure patterns detected") target = _extract_command_python_target(command) + attempted_contract = _target_contract(runner, target, tool_input.get("cwd")) active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") - primary_target = _primary_execution_target(runner) + primary_contract = _primary_execution_contract(runner) + primary_target = primary_contract["target"] if _is_validation_or_launch_command(command) and target and not active_solution_file: runner._active_solution_file = target if _is_validation_or_launch_command(command) and target: - _seed_primary_execution_target(runner, target) + _seed_primary_execution_target(runner, target, cwd=tool_input.get("cwd"), evidence="direct_run") if ( bool(getattr(runner, "_recovery_mode", False)) and _is_validation_or_launch_command(command) and target and primary_target - and target != primary_target + and not _same_target_contract(attempted_contract, primary_contract) and not bool(getattr(runner, "_primary_target_minimally_valid", False)) ): + runner._recovery_target_switch_blocked = True recovery_created_artifacts = set(getattr(runner, "_recovery_created_artifacts", set())) recovery_files_at_failure = set(getattr(runner, "_recovery_files_at_failure", set())) confidently_new_target = target in recovery_created_artifacts or ( bool(recovery_files_at_failure) and target not in recovery_files_at_failure ) if confidently_new_target: - return _recovery_target_switch_blocked_feedback(primary_target, target) + return _recovery_target_switch_blocked_feedback(primary_target, target, reason="recovery_new_wrapper_target_blocked") + return _recovery_target_switch_blocked_feedback(primary_target, target) constrained = runner.small_model or runner.villani_mode or runner.benchmark_config.enabled if not constrained: @@ -1254,6 +1307,8 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: touched_tests = [p for p in attributed_intentional if p.startswith("tests/") and p.endswith(".py")] touched_sources = [p for p in attributed_intentional if p.endswith(".py") and not p.startswith("tests/")] + if attributed_intentional: + refresh_live_validation_candidates(runner, attributed_intentional) task_mode = getattr(runner, "_task_mode", TaskMode.GENERAL) if touched_tests: commands.append(["pytest", "-q", *touched_tests]) @@ -1288,16 +1343,6 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: if stderr_lines: lines.append(f"key stderr:\n{stderr_lines}") - if not str(getattr(runner, "_active_solution_file", "")).strip(): - seed_active = ( - _single_clear_file(sorted(runner._current_verification_targets)) - or _single_clear_file(attributed_intentional) - ) - if seed_active: - runner._active_solution_file = seed_active - if str(getattr(runner, "_active_solution_file", "")).strip(): - _seed_primary_execution_target(runner, str(getattr(runner, "_active_solution_file", ""))) - verification_artifacts = [ r.get("command", "") for r in cmd_results @@ -1319,14 +1364,25 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: before_contents=dict(runner._current_verification_before_contents), ) active_solution_file = str(getattr(runner, "_active_solution_file", "")).replace("\\", "/").lstrip("./") - if not active_solution_file: - for result in cmd_results: - seeded_target = _extract_command_python_target(str(result.get("command", ""))) - if seeded_target: - runner._active_solution_file = seeded_target - active_solution_file = seeded_target - break - primary_target = _seed_primary_execution_target(runner, active_solution_file) + direct_target = "" + for result in cmd_results: + rendered_command = str(result.get("command", "")) + if not _is_validation_or_launch_command(rendered_command): + continue + seeded_target = _extract_command_python_target(rendered_command) + if seeded_target: + direct_target = seeded_target + break + if direct_target and not active_solution_file: + runner._active_solution_file = direct_target + active_solution_file = direct_target + if not active_solution_file and not direct_target: + fallback_target = _single_clear_file(validation_target_paths) + if fallback_target: + active_solution_file = fallback_target + runner._active_solution_file = fallback_target + evidence_kind = "direct_run" if direct_target else "indirect_validation" + primary_target = _seed_primary_execution_target(runner, active_solution_file or direct_target, cwd=str(runner.repo), evidence=evidence_kind) validation_targets = set(validation_target_paths) active_cmd_results = [ result @@ -1371,10 +1427,14 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: elif not active_solution_file and failing_file: runner._active_solution_file = failing_file active_solution_file = failing_file - primary_target = _seed_primary_execution_target(runner, active_solution_file or failing_file) + primary_target = _seed_primary_execution_target( + runner, active_solution_file or failing_file, cwd=str(runner.repo), evidence="direct_validation" + ) runner._recovery_mode = True + runner._recovery_target_switch_blocked = False runner._primary_target_minimally_valid = False runner._failing_file = failing_file + runner._failing_target_contract_summary = _target_contract_summary(_primary_execution_contract(runner)) runner._failing_error_summary = summary runner._failing_command = hard_failure.get("failing_command", "") runner._file_was_read_since_failure = False @@ -1400,6 +1460,7 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: "failing_file": failing_file, "active_solution_file": str(getattr(runner, "_active_solution_file", "")), "primary_execution_target": primary_target, + "primary_execution_cwd": str(getattr(runner, "_primary_execution_target_cwd", "") or "."), "failing_error_summary": summary, "failing_command": runner._failing_command, } @@ -1429,7 +1490,9 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: if validated_primary: runner._primary_target_minimally_valid = True runner._recovery_mode = False + runner._recovery_target_switch_blocked = False runner._failing_file = "" + runner._failing_target_contract_summary = "" runner._failing_error_summary = "" runner._failing_command = "" runner._file_was_read_since_failure = False @@ -1844,3 +1907,18 @@ def run_post_execution_validation(runner: Any, changed_files: list[str]) -> str: runner._mission_state.last_checkpoint_id = checkpoint.checkpoint_id save_mission_state(runner.repo, runner._mission_state) return outcome.message + + +def refresh_live_validation_candidates(runner: Any, changed_or_created_targets: list[str]) -> None: + normalized = { + _normalize_repo_path(path) + for path in changed_or_created_targets + if _normalize_repo_path(path) + } + if not normalized: + return + current = set(getattr(runner, "_current_verification_targets", set())) + primary_target = _primary_execution_target(runner) + if primary_target: + normalized.add(primary_target) + runner._current_verification_targets = current | normalized diff --git a/villani_code/state_tooling.py b/villani_code/state_tooling.py index f7e5404a..e8b13ff0 100644 --- a/villani_code/state_tooling.py +++ b/villani_code/state_tooling.py @@ -604,11 +604,7 @@ def execute_tool_with_policy( ) if normalized_targets: runner._intended_targets.update(normalized_targets) - runner._current_verification_targets = set(normalized_targets) - if not str(getattr(runner, "_active_solution_file", "")).strip(): - seed_target = normalized_targets[0] if len(normalized_targets) == 1 else "" - if seed_target: - runner._active_solution_file = seed_target + runner._current_verification_targets.update(normalized_targets) runner._current_verification_before_contents = {} checkpoint_paths: list[Path] = [] for normalized_target in normalized_targets: @@ -619,6 +615,9 @@ def execute_tool_with_policy( runner._before_contents[normalized_target] = before_text runner._current_verification_before_contents[normalized_target] = before_text runner.checkpoints.create(checkpoint_paths, message_index=message_count) + from villani_code import state_runtime + + state_runtime.refresh_live_validation_candidates(runner, normalized_targets) result = execute_tool_with_lifecycle( runner=runner, tool_name=tool_name, @@ -706,6 +705,7 @@ def _debug_callback_with_turn(event_type: str, payload: dict[str, Any]) -> None: stdout=str(decoded.get("stdout", "") or ""), stderr=str(decoded.get("stderr", "") or ""), attempted_target=state_runtime._extract_command_python_target(command), + attempted_cwd=str(tool_input.get("cwd", "") or runner.repo), ) if ( tool_name == "Read" From 5341dbc2514a21d66d6ab7917ff34b7429ad87d6 Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 15:50:57 +1000 Subject: [PATCH 11/12] Align live validation, recovery context, and primary target contracts --- tests/test_state_runtime.py | 57 ++++++++++++++++++--- tests/test_state_tooling_mutation_policy.py | 43 ++++++++++++++++ villani_code/context_projection.py | 34 +++++++----- villani_code/project_memory.py | 52 +++++++++++++++++++ villani_code/state_runtime.py | 51 ++++++++++++++++-- villani_code/state_tooling.py | 27 +++++++++- 6 files changed, 239 insertions(+), 25 deletions(-) diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index 55177200..4985efbc 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -7,7 +7,8 @@ from villani_code.state import Runner from villani_code import state_runtime -from villani_code.context_projection import build_model_context_packet +from villani_code.context_projection import build_model_context_packet, render_model_context_packet +from villani_code.project_memory import load_validation_config class DummyClient: @@ -134,7 +135,7 @@ def test_prepare_messages_for_model_injects_cmd_shell_reminder() -> None: assert prepared[-1]["content"][0]["text"].startswith("Shell: Windows cmd.") -def test_model_context_packet_includes_recovery_block() -> None: +def test_model_context_packet_omits_recovery_block_when_not_recovering() -> None: runner = SimpleNamespace( _mission_state=SimpleNamespace( objective="o", @@ -155,14 +156,52 @@ def test_model_context_packet_includes_recovery_block() -> None: _runtime_mode="execution", _primary_execution_target="service.py", _primary_execution_target_cwd=".", - _recovery_mode=True, + _primary_execution_target_evidence="direct_run", + _recovery_mode=False, _failing_target_contract_summary="service.py @ .", _primary_target_minimally_valid=False, _recovery_target_switch_blocked=True, ) packet = build_model_context_packet(runner) - assert packet["recovery"]["recovery_mode"] is True - assert packet["recovery"]["primary_target_contract"]["target"] == "service.py" + assert packet["recovery"] is None + + +def test_model_context_packet_includes_compact_live_recovery_contract() -> None: + runner = SimpleNamespace( + _mission_state=SimpleNamespace( + objective="o", + mode="execution", + current_step_id="", + plan_summary="", + verified_facts=[], + open_hypotheses=[], + intended_targets=[], + changed_files=[], + last_failed_command="", + validation_failures=[], + compact_summary="", + primary_execution_target="stale.py", + primary_execution_cwd="stale-cwd", + ), + _task_contract={}, + skills=[], + repo=Path("."), + _runtime_mode="execution", + _primary_execution_target="service.py", + _primary_execution_target_cwd="services", + _primary_execution_target_evidence="direct_validation", + _recovery_mode=True, + _failing_target_contract_summary="service.py @ services", + _primary_target_minimally_valid=False, + _recovery_target_switch_blocked=True, + ) + packet = build_model_context_packet(runner) + rendered = render_model_context_packet(packet) + assert packet["recovery"]["primary_execution_target"] == "service.py" + assert packet["recovery"]["primary_execution_cwd"] == "services" + assert "Recovery contract:" in rendered + assert "target=service.py" in rendered + assert "cwd=services" in rendered def test_validate_anthropic_tool_sequence_rejects_text_after_tool_result() -> None: @@ -506,8 +545,14 @@ def test_live_validation_candidates_refresh_after_material_changes(tmp_path: Pat _seed_repo(tmp_path) runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) runner._current_verification_targets = {"old.py"} - state_runtime.refresh_live_validation_candidates(runner, ["new_script.py"]) + state_runtime.refresh_live_validation_candidates( + runner, + ["new_script.py"], + observed_commands=["python new_script.py"], + ) assert runner._current_verification_targets == {"old.py", "new_script.py"} + cfg = load_validation_config(tmp_path) + assert any(step.command == "python new_script.py" for step in cfg.steps) def test_recovery_does_not_clear_until_primary_target_validates(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: diff --git a/tests/test_state_tooling_mutation_policy.py b/tests/test_state_tooling_mutation_policy.py index 79038d46..0a0e595c 100644 --- a/tests/test_state_tooling_mutation_policy.py +++ b/tests/test_state_tooling_mutation_policy.py @@ -2,6 +2,8 @@ from pathlib import Path +from villani_code.context_projection import build_model_context_packet, render_model_context_packet +from villani_code.project_memory import load_validation_config from villani_code.state import Runner from villani_code.state_tooling import execute_tool_with_lifecycle, execute_tool_with_policy @@ -363,6 +365,47 @@ def test_recovery_mode_allows_bounded_patch_of_active_solution_after_hard_failur assert result["is_error"] is False +def test_integration_scaffold_then_direct_run_reseeds_primary_and_live_validation(tmp_path: Path) -> None: + runner = _runner(tmp_path) + (tmp_path / ".villani").mkdir(parents=True, exist_ok=True) + (tmp_path / ".villani" / "validation.json").write_text( + '{"steps":[{"name":"git-diff","command":"git diff --stat","kind":"inspection","cost_level":1,"is_mutating":false}]}', + encoding="utf-8", + ) + scaffold = execute_tool_with_policy( + runner, + "Write", + {"file_path": "app/__init__.py", "content": ""}, + "s1", + 0, + ) + assert scaffold["is_error"] is False + assert runner._primary_execution_target == "" + + (tmp_path / "service.py").write_text("def broken(:\n pass\n", encoding="utf-8") + run = execute_tool_with_policy( + runner, + "Bash", + {"command": "python service.py", "timeout_sec": 30, "cwd": str(tmp_path)}, + "b1", + 1, + ) + assert run["is_error"] is False + assert runner._primary_execution_target == "service.py" + assert runner._primary_execution_target_evidence == "direct_run" + assert runner._primary_execution_target_cwd == "." + assert runner._recovery_mode is True + + packet = build_model_context_packet(runner) + rendered = render_model_context_packet(packet) + assert "Recovery contract:" in rendered + assert "target=service.py" in rendered + assert "cwd=." in rendered + + cfg = load_validation_config(tmp_path) + assert any(step.command == "python service.py" for step in cfg.steps) + + def test_read_failing_file_flips_recovery_read_flag(tmp_path: Path) -> None: runner = _runner(tmp_path) (tmp_path / "legal_review_app.py").write_text("print('x')\n", encoding="utf-8") diff --git a/villani_code/context_projection.py b/villani_code/context_projection.py index e7d9867e..b0f9c9e3 100644 --- a/villani_code/context_projection.py +++ b/villani_code/context_projection.py @@ -28,16 +28,21 @@ def build_model_context_packet(runner: "Runner") -> dict[str, Any]: constraints.append(f"Success predicate: {contract.get('success_predicate', '')}") constraints.extend([f"No-go: {p}" for p in contract.get("no_go_paths", [])[:4]]) skill_guidance = [getattr(skill, "guidance", "") for skill in getattr(runner, "skills", []) if getattr(skill, "guidance", "")] - recovery_block = { - "primary_target_contract": { - "target": str(getattr(runner, "_primary_execution_target", "")), - "cwd": str(getattr(runner, "_primary_execution_target_cwd", "") or "."), - }, - "recovery_mode": bool(getattr(runner, "_recovery_mode", False)), - "failing_target_summary": str(getattr(runner, "_failing_target_contract_summary", "") or getattr(runner, "_failing_error_summary", "")), - "primary_target_minimally_valid": bool(getattr(runner, "_primary_target_minimally_valid", False)), - "switch_blocked": bool(getattr(runner, "_recovery_target_switch_blocked", False)), - } + recovery_mode = bool(getattr(runner, "_recovery_mode", False)) + primary_target = str(getattr(runner, "_primary_execution_target", "")).replace("\\", "/").lstrip("./") + recovery_block: dict[str, Any] | None = None + if recovery_mode and primary_target: + recovery_block = { + "primary_execution_target": primary_target, + "primary_execution_cwd": str(getattr(runner, "_primary_execution_target_cwd", "") or "."), + "primary_execution_evidence": str(getattr(runner, "_primary_execution_target_evidence", "none")), + "recovery_mode": True, + "failing_target_summary": str( + getattr(runner, "_failing_target_contract_summary", "") or getattr(runner, "_failing_error_summary", "") + ), + "primary_target_minimally_valid": bool(getattr(runner, "_primary_target_minimally_valid", False)), + "switch_blocked": bool(getattr(runner, "_recovery_target_switch_blocked", False)), + } return { "objective": getattr(mission, "objective", ""), "runtime_mode": getattr(mission, "mode", getattr(runner, "_runtime_mode", "execution")), @@ -79,12 +84,13 @@ def render_model_context_packet(packet: dict[str, Any]) -> str: lines.append("Skill guidance:") lines.extend(f"- {g}" for g in guidance[:6]) recovery = packet.get("recovery", {}) - if isinstance(recovery, dict): - contract = recovery.get("primary_target_contract", {}) + if isinstance(recovery, dict) and recovery: lines.append( - "Recovery: " + "Recovery contract: " + f"target={recovery.get('primary_execution_target', '')}; " + f"cwd={recovery.get('primary_execution_cwd', '.')}; " + f"evidence={recovery.get('primary_execution_evidence', 'none')}; " f"mode={bool(recovery.get('recovery_mode', False))}; " - f"primary={contract.get('target', '')}@{contract.get('cwd', '.')}; " f"min_valid={bool(recovery.get('primary_target_minimally_valid', False))}; " f"switch_blocked={bool(recovery.get('switch_blocked', False))}; " f"failure={recovery.get('failing_target_summary', '')}" diff --git a/villani_code/project_memory.py b/villani_code/project_memory.py index 1dadea62..96266b43 100644 --- a/villani_code/project_memory.py +++ b/villani_code/project_memory.py @@ -403,6 +403,58 @@ def load_validation_config(repo: Path) -> ValidationConfig: return ValidationConfig.from_dict(json.loads(path.read_text(encoding="utf-8"))) +def augment_validation_config_with_live_commands(repo: Path, commands: list[str]) -> ValidationConfig: + """Persist a small set of live-observed validation commands into validation.json. + + This keeps startup fallback configs (for example, only git diff) from remaining stale + after the runtime observes stronger, direct validation evidence. + """ + normalized_commands = [str(cmd).strip() for cmd in commands if str(cmd).strip()] + if not normalized_commands: + return load_validation_config(repo) + cfg = load_validation_config(repo) + existing_commands = {str(step.command).strip() for step in cfg.steps if str(step.command).strip()} + added = False + for command in normalized_commands: + if command in existing_commands: + continue + lowered = command.lower() + if "pytest" in lowered: + kind = "test" + strategy = "related_tests" + elif any(token in lowered for token in ("ruff", "mypy", "lint", "typecheck")): + kind = "lint" + strategy = "changed_files" + elif lowered.startswith("python ") or lowered.startswith("python3 "): + kind = "inspection" + strategy = "targeted" + else: + kind = "inspection" + strategy = "none" + name = f"live-{kind}-{len(cfg.steps) + 1}" + cfg.steps.append( + ValidationStep( + name=name, + command=command, + kind=kind, + cost_level=2, + is_mutating=False, + scope_hint="targeted", + language_family="generic", + target_strategy=strategy, + escalation_role="early_signal", + typical_trigger_conditions=["live_observed"], + ) + ) + existing_commands.add(command) + added = True + if added: + path = repo / VILLANI_DIR / "validation.json" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(cfg.to_dict(), indent=2, sort_keys=True) + "\n", encoding="utf-8") + return cfg + + def update_session_state(repo: Path, state: SessionState) -> None: path = repo / VILLANI_DIR / "session_state.json" path.parent.mkdir(parents=True, exist_ok=True) diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index f89f94c8..bd658ba7 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -16,7 +16,13 @@ from villani_code.indexing import DEFAULT_IGNORE, RepoIndex from villani_code.live_display import apply_live_display_delta from villani_code.planning import TaskMode, generate_execution_plan -from villani_code.project_memory import SessionState, ensure_project_memory, load_repo_map, update_session_state +from villani_code.project_memory import ( + SessionState, + augment_validation_config_with_live_commands, + ensure_project_memory, + load_repo_map, + update_session_state, +) from villani_code.context_governance import ContextCompactor, ContextInclusionReason, ContextExclusionReason from villani_code.validation_loop import run_validation from villani_code.shells import baseline_import_validation_command, shell_family_for_platform @@ -735,6 +741,28 @@ def _command_targets_active_solution(command: str, active_solution_file: str, va return len(normalized_targets) == 1 and active in normalized_targets +def _suggest_live_validation_commands(targets: set[str], observed_commands: list[str]) -> list[str]: + suggestions: list[str] = [] + seen: set[str] = set() + for command in observed_commands: + trimmed = str(command or "").strip() + if not trimmed or trimmed in seen: + continue + if _is_validation_or_launch_command(trimmed): + seen.add(trimmed) + suggestions.append(trimmed) + for target in sorted(targets): + normalized = _normalize_repo_path(target) + if not normalized: + continue + if normalized.endswith(".py"): + candidate = f"python {normalized}" + if candidate not in seen: + seen.add(candidate) + suggestions.append(candidate) + return suggestions[:8] + + def _has_hard_failure_signal(stdout: str, stderr: str, exit_code: int) -> bool: combined = f"{stdout}\n{stderr}".strip() lower = combined.lower() @@ -870,6 +898,7 @@ def small_model_tool_guard(runner: Any, tool_name: str, tool_input: dict[str, An runner._active_solution_file = target if _is_validation_or_launch_command(command) and target: _seed_primary_execution_target(runner, target, cwd=tool_input.get("cwd"), evidence="direct_run") + refresh_live_validation_candidates(runner, [target], observed_commands=[command]) if ( bool(getattr(runner, "_recovery_mode", False)) and _is_validation_or_launch_command(command) @@ -1348,6 +1377,13 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: for r in cmd_results if int(r.get("exit", 1)) == 0 and not _command_has_masking_patterns(str(r.get("command", ""))) ] + if verification_artifacts: + live_targets = sorted(set(getattr(runner, "_current_verification_targets", set())) or set(attributed_intentional)) + refresh_live_validation_candidates( + runner, + live_targets, + observed_commands=[str(command) for command in verification_artifacts if str(command).strip()], + ) validation_target_paths = sorted(set(runner._current_verification_targets) or set(attributed_intentional)) validation_target = json.dumps(validation_target_paths) artifact_signature = json.dumps(sorted(verification_artifacts)) @@ -1909,7 +1945,12 @@ def run_post_execution_validation(runner: Any, changed_files: list[str]) -> str: return outcome.message -def refresh_live_validation_candidates(runner: Any, changed_or_created_targets: list[str]) -> None: +def refresh_live_validation_candidates( + runner: Any, + changed_or_created_targets: list[str], + *, + observed_commands: list[str] | None = None, +) -> None: normalized = { _normalize_repo_path(path) for path in changed_or_created_targets @@ -1921,4 +1962,8 @@ def refresh_live_validation_candidates(runner: Any, changed_or_created_targets: primary_target = _primary_execution_target(runner) if primary_target: normalized.add(primary_target) - runner._current_verification_targets = current | normalized + combined = current | normalized + runner._current_verification_targets = combined + live_suggestions = _suggest_live_validation_commands(combined, list(observed_commands or [])) + if live_suggestions: + augment_validation_config_with_live_commands(runner.repo, live_suggestions) diff --git a/villani_code/state_tooling.py b/villani_code/state_tooling.py index e8b13ff0..3ac45f11 100644 --- a/villani_code/state_tooling.py +++ b/villani_code/state_tooling.py @@ -220,6 +220,28 @@ def _benchmark_mutation_targets(tool_name: str, tool_input: dict[str, Any]) -> l return [] +def _is_validation_candidate_target(path: str) -> bool: + normalized = str(path or "").replace("\\", "/").lstrip("./") + if not normalized: + return False + suffix = Path(normalized).suffix.lower() + return suffix in { + ".py", + ".ts", + ".tsx", + ".js", + ".jsx", + ".rs", + ".go", + ".toml", + ".json", + ".yaml", + ".yml", + ".ini", + ".cfg", + } + + def _normalize_mutation_payload(tool_name: str, tool_input: dict[str, Any]) -> None: if tool_name == "Write": content = str(tool_input.get("content", "")) @@ -604,7 +626,8 @@ def execute_tool_with_policy( ) if normalized_targets: runner._intended_targets.update(normalized_targets) - runner._current_verification_targets.update(normalized_targets) + verification_candidates = [target for target in normalized_targets if _is_validation_candidate_target(target)] + runner._current_verification_targets.update(verification_candidates) runner._current_verification_before_contents = {} checkpoint_paths: list[Path] = [] for normalized_target in normalized_targets: @@ -617,7 +640,7 @@ def execute_tool_with_policy( runner.checkpoints.create(checkpoint_paths, message_index=message_count) from villani_code import state_runtime - state_runtime.refresh_live_validation_candidates(runner, normalized_targets) + state_runtime.refresh_live_validation_candidates(runner, verification_candidates or normalized_targets) result = execute_tool_with_lifecycle( runner=runner, tool_name=tool_name, From a0654ac7ed6a170d84ea5540d79f9e525a0c6967 Mon Sep 17 00:00:00 2001 From: mmprotest Date: Sat, 11 Apr 2026 16:22:19 +1000 Subject: [PATCH 12/12] Strengthen validation evidence quality and recovery/final-state honesty --- tests/test_bounded_execution.py | 46 +++++++++++++ tests/test_hardening_pass.py | 32 ++++++++- tests/test_state_runtime.py | 73 +++++++++++++++++++- villani_code/context_projection.py | 14 ++-- villani_code/project_memory.py | 24 +++++-- villani_code/state.py | 22 ++++-- villani_code/state_runtime.py | 104 +++++++++++++++++++++++++---- villani_code/validation_loop.py | 40 ++++++++++- 8 files changed, 322 insertions(+), 33 deletions(-) diff --git a/tests/test_bounded_execution.py b/tests/test_bounded_execution.py index 5f0fc6b3..c4c252d4 100644 --- a/tests/test_bounded_execution.py +++ b/tests/test_bounded_execution.py @@ -150,3 +150,49 @@ def fake_verify(*_args, **_kwargs) -> str: assert result["execution"]["completed"] is True assert result["execution"]["terminated_reason"] == "completed" + + +def test_final_summary_is_honest_when_primary_validation_missing(tmp_path: Path, monkeypatch) -> None: + runner = _runner( + tmp_path, + [ + {"role": "assistant", "content": [{"type": "tool_use", "id": "1", "name": "Bash", "input": {"command": "python web_server.py"}}]}, + {"role": "assistant", "content": [{"type": "text", "text": "Fully functional and thoroughly tested."}]}, + ], + ) + def fake_verify(*_args, **_kwargs) -> str: + runner._primary_execution_target = "web_server.py" + runner._primary_target_minimally_valid = False + runner._active_solution_file = "web_server.py" + runner._active_solution_last_validation_ok = False + return "" + monkeypatch.setattr(runner, "_run_verification", fake_verify) + monkeypatch.setattr(runner, "_run_post_execution_validation", lambda _changed: "") + + result = runner.run("work") + final_text = result["execution"]["final_text"] + assert "Incomplete: primary execution target does not yet have clean direct validation" in final_text + assert "Fully functional and thoroughly tested." in final_text + + +def test_final_summary_can_remain_positive_when_primary_validation_is_strong(tmp_path: Path, monkeypatch) -> None: + runner = _runner( + tmp_path, + [ + {"role": "assistant", "content": [{"type": "tool_use", "id": "1", "name": "Bash", "input": {"command": "python web_server.py"}}]}, + {"role": "assistant", "content": [{"type": "text", "text": "Fully functional and thoroughly tested."}]}, + ], + ) + def fake_verify(*_args, **_kwargs) -> str: + runner._primary_execution_target = "web_server.py" + runner._primary_target_minimally_valid = True + runner._active_solution_file = "web_server.py" + runner._active_solution_last_validation_ok = True + return "" + monkeypatch.setattr(runner, "_run_verification", fake_verify) + monkeypatch.setattr(runner, "_run_post_execution_validation", lambda _changed: "") + + result = runner.run("work") + final_text = result["execution"]["final_text"] + assert "Incomplete: primary execution target does not yet have clean direct validation" not in final_text + assert "Fully functional and thoroughly tested." in final_text diff --git a/tests/test_hardening_pass.py b/tests/test_hardening_pass.py index bd9522f8..8e516d60 100644 --- a/tests/test_hardening_pass.py +++ b/tests/test_hardening_pass.py @@ -5,12 +5,14 @@ from villani_code.planning import PlanRiskLevel, generate_execution_plan -from villani_code.project_memory import SessionState, init_project_memory, load_repo_map, load_validation_config +from villani_code.project_memory import SessionState, ValidationStep, init_project_memory, load_repo_map, load_validation_config from villani_code.state import Runner from villani_code import state_runtime from villani_code.validation_loop import ( + classify_validation_step_strength, infer_targeted_command, plan_validation, + select_validation_steps, summarize_validation_failure, ) @@ -116,6 +118,34 @@ def test_cost_aware_ordering_and_failure_summary(tmp_path: Path) -> None: assert len(failure.compact_output) < 1500 +def test_validation_step_strength_prefers_direct_over_helper() -> None: + direct = ValidationStep( + name="direct", + command="python service.py", + kind="test", + cost_level=2, + is_mutating=False, + ) + helper = ValidationStep( + name="helper", + command="python helper_wrapper.py", + kind="inspection", + cost_level=2, + is_mutating=False, + ) + assert classify_validation_step_strength(direct) == "direct_run_or_task_validation" + assert classify_validation_step_strength(helper) == "helper_wrapper" + + +def test_select_validation_steps_keeps_stronger_steps_available(tmp_path: Path) -> None: + _seed_repo(tmp_path) + init_project_memory(tmp_path) + cfg = load_validation_config(tmp_path) + selected = select_validation_steps(cfg, ["villani_code/mod.py"]) + kinds = [step.kind for step in selected] + assert "test" in kinds + + def test_dedicated_repair_executor_is_bounded(tmp_path: Path) -> None: _seed_repo(tmp_path) init_project_memory(tmp_path) diff --git a/tests/test_state_runtime.py b/tests/test_state_runtime.py index 4985efbc..7af17f99 100644 --- a/tests/test_state_runtime.py +++ b/tests/test_state_runtime.py @@ -192,8 +192,10 @@ def test_model_context_packet_includes_compact_live_recovery_contract() -> None: _primary_execution_target_evidence="direct_validation", _recovery_mode=True, _failing_target_contract_summary="service.py @ services", + _failing_error_summary="NameError: x", _primary_target_minimally_valid=False, _recovery_target_switch_blocked=True, + _active_solution_last_validation_ok=False, ) packet = build_model_context_packet(runner) rendered = render_model_context_packet(packet) @@ -202,6 +204,8 @@ def test_model_context_packet_includes_compact_live_recovery_contract() -> None: assert "Recovery contract:" in rendered assert "target=service.py" in rendered assert "cwd=services" in rendered + assert "active_validation_ok=False" in rendered + assert "failure=NameError: x" in rendered def test_validate_anthropic_tool_sequence_rejects_text_after_tool_result() -> None: @@ -455,10 +459,10 @@ def fake_run(cmd, **kwargs): assert runner._primary_execution_target == "legal_review_app.py" assert runner._active_solution_last_validation_ok is False runner._run_verification("edit") - assert runner._recovery_mode is False + assert runner._recovery_mode is True assert runner._active_solution_file == "legal_review_app.py" assert runner._primary_execution_target == "legal_review_app.py" - assert runner._active_solution_last_validation_ok is True + assert runner._active_solution_last_validation_ok is False def test_helper_failure_does_not_switch_active_solution_file(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: @@ -504,7 +508,8 @@ class P: monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) runner._run_verification("edit") assert runner._primary_execution_target == "service.py" - assert runner._primary_target_minimally_valid is True + assert runner._primary_target_minimally_valid is False + assert runner._active_solution_last_validation_ok is False def test_single_file_write_seed_is_replaceable_by_stronger_direct_signal(tmp_path: Path) -> None: @@ -555,6 +560,21 @@ def test_live_validation_candidates_refresh_after_material_changes(tmp_path: Pat assert any(step.command == "python new_script.py" for step in cfg.steps) +def test_direct_live_validation_candidate_is_ranked_ahead_of_helper_wrapper(tmp_path: Path) -> None: + _seed_repo(tmp_path) + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._primary_execution_target = "service.py" + state_runtime.refresh_live_validation_candidates( + runner, + ["service.py"], + observed_commands=["python helper_wrapper.py", "python service.py"], + ) + cfg = load_validation_config(tmp_path) + live_steps = [step for step in cfg.steps if step.name.startswith("live-")] + live_commands = [step.command for step in live_steps] + assert live_commands.index("python service.py") < live_commands.index("python helper_wrapper.py") + + def test_recovery_does_not_clear_until_primary_target_validates(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: _seed_repo(tmp_path) (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") @@ -581,6 +601,53 @@ class P: assert runner._recovery_mode is True +def test_recovery_requires_strong_primary_validation_to_clear(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + _seed_repo(tmp_path) + (tmp_path / "web_app.py").write_text("print('x')\n", encoding="utf-8") + (tmp_path / "helper_wrapper.py").write_text("print('helper')\n", encoding="utf-8") + runner = Runner(client=DummyClient(), repo=tmp_path, model="m", stream=False, small_model=True) + runner._verification_baseline_changed = set() + runner._recovery_mode = True + runner._primary_execution_target = "web_app.py" + runner._active_solution_file = "web_app.py" + runner._current_verification_targets = {"web_app.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["web_app.py"]) + original_verify = runner._verification_engine.verify + + def force_pass(*args, **kwargs): + verified = original_verify(*args, **kwargs) + verified.status = state_runtime.VerificationStatus.PASS + return verified + + monkeypatch.setattr(runner._verification_engine, "verify", force_pass) + + class P: + returncode = 0 + stdout = "ok" + stderr = "" + + calls = {"n": 0} + + def fake_run(cmd, **kwargs): + rendered = " ".join(cmd) if isinstance(cmd, list) else str(cmd) + if rendered.startswith("git "): + return P() + calls["n"] += 1 + if calls["n"] == 1: + return P() + return P() + + monkeypatch.setattr(state_runtime.subprocess, "run", fake_run) + runner._run_verification("edit") + assert runner._recovery_mode is True + assert runner._primary_target_minimally_valid is False + + runner._current_verification_targets = {"web_app.py"} + monkeypatch.setattr(state_runtime, "git_changed_files", lambda _repo: ["tests/test_web_app.py"]) + runner._run_verification("edit") + assert runner._active_solution_last_validation_ok is True + + def test_mission_state_tracks_live_primary_target_and_recovery_mode(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: _seed_repo(tmp_path) (tmp_path / "service.py").write_text("print('x')\n", encoding="utf-8") diff --git a/villani_code/context_projection.py b/villani_code/context_projection.py index b0f9c9e3..98e62294 100644 --- a/villani_code/context_projection.py +++ b/villani_code/context_projection.py @@ -32,16 +32,19 @@ def build_model_context_packet(runner: "Runner") -> dict[str, Any]: primary_target = str(getattr(runner, "_primary_execution_target", "")).replace("\\", "/").lstrip("./") recovery_block: dict[str, Any] | None = None if recovery_mode and primary_target: + failing_summary = str(getattr(runner, "_failing_error_summary", "") or "") + failing_target_summary = str(getattr(runner, "_failing_target_contract_summary", "") or "") recovery_block = { "primary_execution_target": primary_target, "primary_execution_cwd": str(getattr(runner, "_primary_execution_target_cwd", "") or "."), "primary_execution_evidence": str(getattr(runner, "_primary_execution_target_evidence", "none")), "recovery_mode": True, - "failing_target_summary": str( - getattr(runner, "_failing_target_contract_summary", "") or getattr(runner, "_failing_error_summary", "") - ), + "failing_target_summary": failing_target_summary, + "failing_error_summary": failing_summary, "primary_target_minimally_valid": bool(getattr(runner, "_primary_target_minimally_valid", False)), "switch_blocked": bool(getattr(runner, "_recovery_target_switch_blocked", False)), + "active_solution_last_validation_ok": getattr(runner, "_active_solution_last_validation_ok", None), + "needs_direct_validation": not bool(getattr(runner, "_primary_target_minimally_valid", False)), } return { "objective": getattr(mission, "objective", ""), @@ -85,6 +88,7 @@ def render_model_context_packet(packet: dict[str, Any]) -> str: lines.extend(f"- {g}" for g in guidance[:6]) recovery = packet.get("recovery", {}) if isinstance(recovery, dict) and recovery: + failure_summary = str(recovery.get("failing_error_summary", "") or recovery.get("failing_target_summary", "")) lines.append( "Recovery contract: " f"target={recovery.get('primary_execution_target', '')}; " @@ -93,6 +97,8 @@ def render_model_context_packet(packet: dict[str, Any]) -> str: f"mode={bool(recovery.get('recovery_mode', False))}; " f"min_valid={bool(recovery.get('primary_target_minimally_valid', False))}; " f"switch_blocked={bool(recovery.get('switch_blocked', False))}; " - f"failure={recovery.get('failing_target_summary', '')}" + f"active_validation_ok={recovery.get('active_solution_last_validation_ok', None)}; " + f"needs_direct_validation={bool(recovery.get('needs_direct_validation', True))}; " + f"failure={failure_summary}" ) return "\n".join(lines) diff --git a/villani_code/project_memory.py b/villani_code/project_memory.py index 96266b43..c187ad43 100644 --- a/villani_code/project_memory.py +++ b/villani_code/project_memory.py @@ -398,12 +398,19 @@ def load_repo_map(repo: Path) -> dict[str, Any]: def load_validation_config(repo: Path) -> ValidationConfig: path = repo / VILLANI_DIR / "validation.json" - if not path.exists(): + if path.exists(): + cfg = ValidationConfig.from_dict(json.loads(path.read_text(encoding="utf-8"))) + if cfg.steps: + return cfg + repo_map, generated, _rules = scan_repo(repo) + if not repo_map.languages and not generated.steps: return ValidationConfig(steps=[]) - return ValidationConfig.from_dict(json.loads(path.read_text(encoding="utf-8"))) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(generated.to_dict(), indent=2, sort_keys=True) + "\n", encoding="utf-8") + return generated -def augment_validation_config_with_live_commands(repo: Path, commands: list[str]) -> ValidationConfig: +def augment_validation_config_with_live_commands(repo: Path, commands: list[str], *, primary_target: str = "") -> ValidationConfig: """Persist a small set of live-observed validation commands into validation.json. This keeps startup fallback configs (for example, only git diff) from remaining stale @@ -419,12 +426,21 @@ def augment_validation_config_with_live_commands(repo: Path, commands: list[str] if command in existing_commands: continue lowered = command.lower() - if "pytest" in lowered: + if "pytest" in lowered and primary_target and primary_target in lowered: + kind = "test" + strategy = "primary_target" + elif "pytest" in lowered: kind = "test" strategy = "related_tests" + elif any(token in lowered for token in ("helper", "wrapper", "probe", "verify_", "smoke", "sanity")): + kind = "inspection" + strategy = "helper_wrapper" elif any(token in lowered for token in ("ruff", "mypy", "lint", "typecheck")): kind = "lint" strategy = "changed_files" + elif (lowered.startswith("python ") or lowered.startswith("python3 ")) and primary_target and primary_target in lowered: + kind = "test" + strategy = "primary_target" elif lowered.startswith("python ") or lowered.startswith("python3 "): kind = "inspection" strategy = "targeted" diff --git a/villani_code/state.py b/villani_code/state.py index 3883ce68..56195170 100644 --- a/villani_code/state.py +++ b/villani_code/state.py @@ -1025,6 +1025,16 @@ def _has_meaningful_benchmark_edit() -> bool: def _finish_bounded( response: dict[str, Any], reason: str, completed: bool ) -> dict[str, Any]: + def _evidence_guard_prefix() -> str: + primary_target = str(getattr(self, "_primary_execution_target", "")).strip() + primary_ok = bool(getattr(self, "_primary_target_minimally_valid", False)) + if not primary_target or primary_ok: + return "" + return ( + "[Execution status] Incomplete: primary execution target does not yet have clean direct validation. " + "Treat results as partial until direct validation succeeds." + ) + elapsed = time.monotonic() - start intentional_changes, incidental_changes, all_changes = _change_summary() final_text = "\n".join( @@ -1032,13 +1042,11 @@ def _finish_bounded( for block in response.get("content", []) if block.get("type") == "text" ) - primary_target = str(getattr(self, "_primary_execution_target", "")).strip() - primary_ok = getattr(self, "_primary_target_minimally_valid", False) - if primary_target and not primary_ok: - final_text = ( - "[Execution status] Incomplete: primary execution target does not yet have clean direct validation.\n\n" - + final_text - ).strip() + guard_prefix = _evidence_guard_prefix() + if guard_prefix: + final_text = f"{guard_prefix}\n\n{final_text}".strip() + response.setdefault("content", []) + response["content"].insert(0, {"type": "text", "text": guard_prefix}) execution = ExecutionResult( final_text=final_text, turns_used=turns_used, diff --git a/villani_code/state_runtime.py b/villani_code/state_runtime.py index bd658ba7..6fdc7d18 100644 --- a/villani_code/state_runtime.py +++ b/villani_code/state_runtime.py @@ -610,11 +610,43 @@ def _extract_command_python_target(command: str) -> str: return "" +def _classify_validation_strength(command: str, *, primary_contract: dict[str, str] | None = None) -> str: + lowered = str(command or "").strip().lower() + if not lowered: + return "structural_only" + if lowered.startswith("git diff") or lowered.startswith("ls ") or lowered.startswith("find "): + return "structural_only" + if any(tok in lowered for tok in ("helper", "wrapper", "probe", "verify_", "smoke", "sanity")): + return "helper_wrapper" + target = _extract_command_python_target(command) + primary_target = str((primary_contract or {}).get("target", "")).strip() + if target and primary_target and target == primary_target: + return "direct_run_or_task_validation" + if "pytest" in lowered and primary_target and (primary_target in lowered or "tests/" in lowered): + return "direct_run_or_task_validation" + if any(tok in lowered for tok in ("py_compile", "compileall", " import ")): + return "direct_import_or_compile" + if any(tok in lowered for tok in ("pytest", "unittest", "python ", "python3 ", "uv run", "poetry run")): + return "indirect_probe" + return "structural_only" + + +def _validation_strength_value(kind: str) -> int: + return _VALIDATION_STRENGTH_ORDER.get(str(kind), 0) + + def _primary_execution_target(runner: Any) -> str: return str(getattr(runner, "_primary_execution_target", "")).replace("\\", "/").lstrip("./") _TARGET_EVIDENCE_ORDER = {"none": 0, "write_only": 1, "indirect_validation": 2, "direct_validation": 3, "direct_run": 4} +_VALIDATION_STRENGTH_ORDER = { + "structural_only": 0, + "indirect_probe": 1, + "helper_wrapper": 2, + "direct_import_or_compile": 3, + "direct_run_or_task_validation": 4, +} def _normalize_contract_cwd(runner: Any, cwd: str | None) -> str: @@ -1372,11 +1404,17 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: if stderr_lines: lines.append(f"key stderr:\n{stderr_lines}") - verification_artifacts = [ - r.get("command", "") - for r in cmd_results - if int(r.get("exit", 1)) == 0 and not _command_has_masking_patterns(str(r.get("command", ""))) - ] + primary_contract = _primary_execution_contract(runner) + verification_artifacts: list[str] = [] + strongest_artifact_strength = "structural_only" + for result in cmd_results: + rendered = str(result.get("command", "")) + if int(result.get("exit", 1)) != 0 or _command_has_masking_patterns(rendered): + continue + strength = _classify_validation_strength(rendered, primary_contract=primary_contract) + if _validation_strength_value(strength) > _validation_strength_value(strongest_artifact_strength): + strongest_artifact_strength = strength + verification_artifacts.append(rendered) if verification_artifacts: live_targets = sorted(set(getattr(runner, "_current_verification_targets", set())) or set(attributed_intentional)) refresh_live_validation_candidates( @@ -1417,7 +1455,12 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: if fallback_target: active_solution_file = fallback_target runner._active_solution_file = fallback_target - evidence_kind = "direct_run" if direct_target else "indirect_validation" + evidence_kind = ( + "direct_run" + if _validation_strength_value(strongest_artifact_strength) + >= _validation_strength_value("direct_run_or_task_validation") + else "indirect_validation" + ) primary_target = _seed_primary_execution_target(runner, active_solution_file or direct_target, cwd=str(runner.repo), evidence=evidence_kind) validation_targets = set(validation_target_paths) active_cmd_results = [ @@ -1429,12 +1472,22 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: validation_targets, ) ] + strong_primary_proof = False if active_cmd_results: latest = active_cmd_results[-1] exit_code = int(latest.get("exit", 0)) stdout = str(latest.get("stdout", "") or "") stderr = str(latest.get("stderr", "") or "") has_failure = _has_hard_failure_signal(stdout, stderr, exit_code) + latest_strength = _classify_validation_strength( + str(latest.get("command", "")), + primary_contract=_primary_execution_contract(runner), + ) + strong_primary_proof = ( + not has_failure + and _validation_strength_value(latest_strength) + >= _validation_strength_value("direct_run_or_task_validation") + ) if has_failure: runner._active_solution_last_validation_ok = False evidence_lines = [ln.strip() for ln in (stderr or stdout or f"exit={exit_code}").splitlines() if ln.strip()] @@ -1442,9 +1495,13 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: runner._active_solution_last_validation_summary = summary_line verification.status = VerificationStatus.FAIL else: - runner._active_solution_last_validation_ok = True - runner._active_solution_last_validation_summary = "validation passed" - if primary_target and active_solution_file and primary_target == active_solution_file: + runner._active_solution_last_validation_ok = strong_primary_proof + runner._active_solution_last_validation_summary = ( + "validation passed (direct)" + if strong_primary_proof + else "validation passed (weak evidence)" + ) + if strong_primary_proof and primary_target and active_solution_file and primary_target == active_solution_file: runner._primary_target_minimally_valid = True _sync_recovery_state_to_mission(runner) @@ -1522,6 +1579,15 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: int(result.get("exit", 0)), ) for result in targeted_results + ) and all( + _validation_strength_value( + _classify_validation_strength( + str(result.get("command", "")), + primary_contract=_primary_execution_contract(runner), + ) + ) + >= _validation_strength_value("direct_run_or_task_validation") + for result in targeted_results ) if validated_primary: runner._primary_target_minimally_valid = True @@ -1609,7 +1675,10 @@ def run_verification(runner: Any, trigger: str = "edit") -> str: for finding in verification.findings[:6]: lines.append(f"- {finding.category.value}: {finding.message}") lines.append("") - summary = f"status={verification.status.value}; confidence={verification.confidence_score}" + summary = ( + f"status={verification.status.value}; confidence={verification.confidence_score}; " + f"strongest_validation={strongest_artifact_strength}" + ) if verification.findings: summary += f"; findings={len(verification.findings)}" runner.event_callback( @@ -1964,6 +2033,17 @@ def refresh_live_validation_candidates( normalized.add(primary_target) combined = current | normalized runner._current_verification_targets = combined - live_suggestions = _suggest_live_validation_commands(combined, list(observed_commands or [])) + observed = list(observed_commands or []) + contract = _primary_execution_contract(runner) + ordered_observed = sorted( + [cmd for cmd in observed if str(cmd).strip()], + key=lambda cmd: _validation_strength_value(_classify_validation_strength(str(cmd), primary_contract=contract)), + reverse=True, + ) + live_suggestions = _suggest_live_validation_commands(combined, ordered_observed) if live_suggestions: - augment_validation_config_with_live_commands(runner.repo, live_suggestions) + augment_validation_config_with_live_commands( + runner.repo, + live_suggestions, + primary_target=contract.get("target", ""), + ) diff --git a/villani_code/validation_loop.py b/villani_code/validation_loop.py index 5661b150..d939d870 100644 --- a/villani_code/validation_loop.py +++ b/villani_code/validation_loop.py @@ -10,6 +10,14 @@ from villani_code.planning import ActionClass, ChangeImpact, TaskMode, compact_failure_output from villani_code.project_memory import ValidationConfig, ValidationStep, load_repo_map, load_validation_config +VALIDATION_STRENGTH_ORDER = { + "structural_only": 0, + "indirect_probe": 1, + "helper_wrapper": 2, + "direct_import_or_compile": 3, + "direct_run_or_task_validation": 4, +} + @dataclass(slots=True) class ValidationTarget: @@ -164,6 +172,27 @@ def infer_targeted_command(step: ValidationStep, changed_files: list[str], repo_ return f"python -m pytest -q {quoted}".strip() +def classify_validation_step_strength(step: ValidationStep, *, changed_files: list[str] | None = None) -> str: + command = str(step.command or "").strip().lower() + if step.kind == "inspection" and command.startswith("git diff"): + return "structural_only" + if any(tok in command for tok in ("helper", "wrapper", "probe", "verify_", "smoke", "sanity")): + return "helper_wrapper" + if step.kind in {"build", "test"}: + return "direct_run_or_task_validation" + if step.kind in {"typecheck", "lint"} or any(tok in command for tok in ("py_compile", "compileall", " import ")): + return "direct_import_or_compile" + if step.kind == "inspection": + return "indirect_probe" + if changed_files and step.scope_hint == "targeted": + return "indirect_probe" + return "structural_only" + + +def validation_step_strength_value(step: ValidationStep, *, changed_files: list[str] | None = None) -> int: + return VALIDATION_STRENGTH_ORDER.get(classify_validation_step_strength(step, changed_files=changed_files), 0) + + def _step_order(step: ValidationStep) -> tuple[int, int, str]: kind_order = {"format": 0, "lint": 1, "typecheck": 2, "test": 3, "build": 4, "inspection": 5} return (step.cost_level, kind_order.get(step.kind, 9), step.name) @@ -189,7 +218,10 @@ def _impact_from_inputs(scope: ValidationScope, change_impact: str | None, actio def plan_validation(config: ValidationConfig, changed_files: list[str], repo_map: dict[str, Any] | None = None, change_impact: str | None = None, action_classes: list[str] | None = None, task_mode: str = TaskMode.GENERAL.value) -> ValidationPlan: scope = infer_validation_scope(changed_files) - enabled = sorted([s for s in config.steps if s.enabled], key=_step_order) + enabled = sorted( + [s for s in config.steps if s.enabled], + key=lambda step: (_step_order(step), -validation_step_strength_value(step, changed_files=changed_files)), + ) impact = _impact_from_inputs(scope, change_impact, action_classes) targets = infer_validation_targets(changed_files, repo_map) @@ -330,4 +362,8 @@ def run_validation(repo: Path, changed_files: list[str], event_callback: Any | N def select_validation_steps(config: ValidationConfig, changed_files: list[str]) -> list[ValidationStep]: - return [row.step for row in plan_validation(config, changed_files).selected_steps] + selected = [row.step for row in plan_validation(config, changed_files).selected_steps] + return sorted( + selected, + key=lambda step: (_step_order(step), -validation_step_strength_value(step, changed_files=changed_files)), + )