From 8005720a8149c30e9bc0645d024432a5942eacaa Mon Sep 17 00:00:00 2001 From: Toni Nowak Date: Sun, 27 Sep 2026 14:26:49 +0200 Subject: [PATCH] fix(pilot): require symmetric observed Git validation --- PILOT.md | 4 + TASKS.md | 4 + scripts/pilot_contract_triage_pair.py | 106 +++++++++++++- tests/test_pilot_contract_case_profile.py | 4 + tests/test_pilot_profile_git_validation.py | 158 +++++++++++++++++++++ 5 files changed, 270 insertions(+), 6 deletions(-) create mode 100644 tests/test_pilot_profile_git_validation.py diff --git a/PILOT.md b/PILOT.md index 3b86a86..47ef5a1 100644 --- a/PILOT.md +++ b/PILOT.md @@ -1731,3 +1731,7 @@ This integration is an offline measurement capability, not delivery or efficacy ## VCR391 — Profile advice acknowledgment timing The profile event parser now observes acknowledgment after both legacy and configured valid triage results. A real synthetic event stream verifies acknowledgment before the next tool and distinguishes missing or late acknowledgment. This fixes delivery measurement only; it does not establish native delivery or efficacy. + +## VCR392 — Symmetric repair-arm Git validation + +Repair-profile arms now receive isolated Git baselines before execution. Setup failure prevents launch. The runner records the agent’s exact diff-check invocation and exit codes; missing or unsuccessful checks prevent task acceptance in both arms. Protected fixture digests exclude only internal Git metadata. Four offline tests cover setup failure, metadata handling, whitespace errors and missing/nonzero checks; existing profile tests remain green. No live delivery or efficacy result is established. diff --git a/TASKS.md b/TASKS.md index 1525bc3..cd5211d 100644 --- a/TASKS.md +++ b/TASKS.md @@ -1135,3 +1135,7 @@ The runner now writes the bounded redacted measurement to its private output dir ## VCR391 — Profile advice acknowledgment timing The profile event parser now observes acknowledgment after both legacy and configured valid triage results. A real synthetic event stream verifies acknowledgment before the next tool and distinguishes missing or late acknowledgment. This fixes delivery measurement only; it does not establish native delivery or efficacy. + +## VCR392 — Symmetric repair-arm Git validation + +Repair-profile arms now receive isolated Git baselines before execution. Setup failure prevents launch. The runner records the agent’s exact diff-check invocation and exit codes; missing or unsuccessful checks prevent task acceptance in both arms. Protected fixture digests exclude only internal Git metadata. Four offline tests cover setup failure, metadata handling, whitespace errors and missing/nonzero checks; existing profile tests remain green. No live delivery or efficacy result is established. diff --git a/scripts/pilot_contract_triage_pair.py b/scripts/pilot_contract_triage_pair.py index 5dccdaf..e768c4e 100644 --- a/scripts/pilot_contract_triage_pair.py +++ b/scripts/pilot_contract_triage_pair.py @@ -361,6 +361,7 @@ def _case_treatment_prompt(profile: CaseProfile, advice_policy: str) -> str: guidance += ( f"Complete the requested behavior change in {profile.source_file}. " f"Do not modify {profile.focused_test_file} or configured evidence files. " + "Run git diff --check and require exit code zero after the change. " "Run the focused test again and the required full test suite after the change. " "The supervisor runs a separate immutable oracle." ) @@ -537,6 +538,8 @@ def _event_receipts( evidence: set[str] = set() focused_exits: list[int] = [] full_exits: list[int] = [] + git_diff_check_exits: list[int] = [] + git_diff_check_invoked = False useful_failure_ms: float | None = None useful_failure_observed = False triage_seen = False @@ -624,6 +627,10 @@ def _event_receipts( if kind: pending[event_id] = (kind, None) argv = common._command_argv(item) + if (profile is not None and profile.outcome_mode == "repair" + and argv == ["git", "diff", "--check"]): + git_diff_check_invoked = True + pending[event_id] = ("git-diff-check", None) if argv and "jevcompass" in argv: triage_seen = True exact = _is_triage(item, focused_exits[0] if focused_exits else None, profile) @@ -643,7 +650,9 @@ def _event_receipts( elif event_type == "item.completed" and event_id in pending: kind, _ = pending.pop(event_id) code = item.get("exit_code") - if kind == "focused" and isinstance(code, int) and not isinstance(code, bool): + if kind == "git-diff-check" and isinstance(code, int) and not isinstance(code, bool): + git_diff_check_exits.append(code) + elif kind == "focused" and isinstance(code, int) and not isinstance(code, bool): focused_exits.append(code) output = item.get("aggregated_output") if not isinstance(output, str): @@ -718,6 +727,16 @@ def _event_receipts( "codex_billing_estimate": None, "event_count": len(list(lines)) if isinstance(lines, list) else None, } + if profile is not None and profile.outcome_mode == "repair": + result["agent_git_diff_check_invocation_observed"] = git_diff_check_invoked + result["agent_git_diff_check_exit_codes"] = git_diff_check_exits + result["agent_git_diff_check_exit_code"] = ( + git_diff_check_exits[-1] if git_diff_check_exits else None + ) + result["agent_git_diff_check_passed"] = bool( + git_diff_check_invoked and git_diff_check_exits + and all(code == 0 for code in git_diff_check_exits) + ) if advice_policy == "nonbinding": result["workflow_acknowledgment"] = workflow_acknowledgment result["triage_result_acknowledgment"] = triage_result_acknowledgment @@ -807,8 +826,58 @@ def _run_arm( return result, answer -def _fixture_digest(root: Path, *, exclude_relative: str | None = None) -> str | None: - """Hash fixture files while ignoring generated Python bytecode.""" +def _initialize_private_git_baseline(root: Path) -> bool: + """Initialize an isolated Git baseline for one copied repair fixture.""" + git = shutil.which("git") + if not git: + return False + env = { + "PATH": os.environ.get("PATH", ""), + "HOME": str(root), + "GIT_CONFIG_NOSYSTEM": "1", + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_TERMINAL_PROMPT": "0", + "GIT_AUTHOR_NAME": "JevCompass Fixture", + "GIT_AUTHOR_EMAIL": "fixture@example.invalid", + "GIT_COMMITTER_NAME": "JevCompass Fixture", + "GIT_COMMITTER_EMAIL": "fixture@example.invalid", + } + commands = ( + [git, "init", "--quiet"], + [git, "add", "-A"], + [git, "-c", "commit.gpgsign=false", "-c", "core.hooksPath=/dev/null", + "commit", "--quiet", "-m", "Immutable synthetic fixture baseline"], + ) + try: + for argv in commands: + completed = subprocess.run( + argv, cwd=root, env=env, stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, timeout=10, check=False, + ) + if completed.returncode != 0: + return False + except (OSError, subprocess.TimeoutExpired): + return False + return True + + +def _repair_git_diff_check_valid(arm: dict[str, Any]) -> bool: + """Require an observed, successful diff check for repair-profile completion.""" + exits = arm.get("agent_git_diff_check_exit_codes") + return bool( + arm.get("agent_git_diff_check_invocation_observed") is True + and isinstance(exits, list) and exits + and all(isinstance(code, int) and not isinstance(code, bool) and code == 0 + for code in exits) + and arm.get("agent_git_diff_check_passed") is True + ) + + +def _fixture_digest( + root: Path, *, exclude_relative: str | None = None, + exclude_internal_git: bool = False, +) -> str | None: + """Hash fixture files while ignoring bytecode and optional private .git metadata.""" digest = hashlib.sha256() try: for path in sorted(root.rglob("*")): @@ -817,6 +886,8 @@ def _fixture_digest(root: Path, *, exclude_relative: str | None = None) -> str | relative_text = path.relative_to(root).as_posix() if relative_text == exclude_relative: continue + if exclude_internal_git and (relative_text == ".git" or relative_text.startswith(".git/")): + continue relative = relative_text.encode("utf-8") digest.update(len(relative).to_bytes(4, "big")) digest.update(relative) @@ -963,9 +1034,11 @@ def run_pair( for name in (source_file_rel, test_file_rel) } initial_fixture_digest = _fixture_digest(fixture_source) + repair_profile = bool(case_profile and case_profile.outcome_mode == "repair") initial_protected_digest = _fixture_digest( fixture_source, exclude_relative=source_file_rel, - ) if case_profile and case_profile.outcome_mode == "repair" else initial_fixture_digest + exclude_internal_git=repair_profile, + ) if repair_profile else initial_fixture_digest for true_arm in ("baseline", "treatment"): fixtures[true_arm] = private_root / f"{true_arm}-fixture" homes[true_arm] = private_root / f"{true_arm}-home" @@ -982,6 +1055,19 @@ def run_pair( return receipt if digests["baseline"] != digests["treatment"]: raise RuntimeError("fixture parity verification failed") + if repair_profile and not all( + _initialize_private_git_baseline(fixtures[name]) + for name in ("baseline", "treatment") + ): + receipt = { + "schema_version": 1, "run_id": run_id, "status": "failed", + "failure": "repair_git_setup_failed", "arms": {}, + } + common._private_write( + output_dir / "receipt.json", + (json.dumps(receipt, sort_keys=True, indent=2) + "\n").encode("utf-8"), + ) + return receipt arms: dict[str, dict[str, Any]] = {} answers: dict[str, str | None] = {} @@ -1003,6 +1089,11 @@ def run_pair( measurement_path=output_dir / f"{label}-agent-measurement.json", profile=case_profile, ) + if repair_profile: + arms[label].setdefault("agent_git_diff_check_invocation_observed", False) + arms[label].setdefault("agent_git_diff_check_exit_codes", []) + arms[label].setdefault("agent_git_diff_check_exit_code", None) + arms[label].setdefault("agent_git_diff_check_passed", False) arms[label]["fixture_sha256_before"] = digests[true_arm] source_unchanged: dict[str, bool] = {} @@ -1026,8 +1117,10 @@ def run_pair( ) protected_fixture_unchanged[label] = ( initial_protected_digest is not None - and _fixture_digest(fixtures[true_arm], exclude_relative=source_file_rel) - == initial_protected_digest + and _fixture_digest( + fixtures[true_arm], exclude_relative=source_file_rel, + exclude_internal_git=repair_profile, + ) == initial_protected_digest ) source_bytes = _safe_artifact(fixtures[true_arm] / source_file_rel) if source_bytes is not None: @@ -1070,6 +1163,7 @@ def run_pair( and source_changed.get(label) is True and tests_unchanged.get(label) is True and protected_fixture_unchanged.get(label) is True + and _repair_git_diff_check_valid(arm) ) arm["validated_completion_ms"] = endpoint_ms if completion_valid else None arm["task_outcome_status"] = ( diff --git a/tests/test_pilot_contract_case_profile.py b/tests/test_pilot_contract_case_profile.py index ae4a850..c5077be 100644 --- a/tests/test_pilot_contract_case_profile.py +++ b/tests/test_pilot_contract_case_profile.py @@ -160,6 +160,10 @@ def fake_arm(**kwargs): "first_useful_failure_observed": True, "full_suite_invocation_observed": True, "full_suite_exit": 0 if repair_succeeds else 1, + "agent_git_diff_check_invocation_observed": repair_succeeds, + "agent_git_diff_check_exit_codes": [0] if repair_succeeds else [], + "agent_git_diff_check_exit_code": 0 if repair_succeeds else None, + "agent_git_diff_check_passed": repair_succeeds, } if advice_policy == "nonbinding" and kwargs["treatment"]: result.update({ diff --git a/tests/test_pilot_profile_git_validation.py b/tests/test_pilot_profile_git_validation.py new file mode 100644 index 0000000..90d1cac --- /dev/null +++ b/tests/test_pilot_profile_git_validation.py @@ -0,0 +1,158 @@ +from __future__ import annotations + +import hashlib +import json +import shlex +import subprocess +import sys +import tempfile +import types +import unittest +from pathlib import Path +from unittest.mock import patch + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "scripts")) + +import pilot_contract_triage_pair as runner + + +def _event(kind: str, event_id: str, argv: list[str], **extra: object) -> str: + item = { + "id": event_id, + "type": "command_execution", + "command": shlex.join(argv), + **extra, + } + return json.dumps({"type": kind, "item": item}) + + +def _repair_diff_receipt(check_argv: list[str] | None, exit_code: int | None): + lines = [] + if check_argv is not None: + lines = [ + _event("item.started", "diff", check_argv), + _event("item.completed", "diff", check_argv, exit_code=exit_code), + ] + result, _ = runner._event_receipts( + lines, [1.0, 2.0], 0.0, + profile=types.SimpleNamespace(outcome_mode="repair", focused_command=(), evidence_files={}), + ) + return result + + +class PilotProfileGitValidationTests(unittest.TestCase): + def _profile(self, fixture: Path, oracle: Path) -> runner.CaseProfile: + oracle.write_text("pass\n", encoding="utf-8") + return runner.CaseProfile( + case_id="synthetic-git-validation", + fixture_source=fixture, + task_prompt="Repair the copied source.", + source_file="src/service.py", + focused_test_file="tests/test_service.py", + focused_command=("python", "-m", "unittest", "tests.test_service"), + evidence_files={}, + evidence_markers={}, + failure_markers=("AssertionError",), + triage_kinds=(), + triage_hypotheses=(), + triage_accepted_ids=(), + triage_accepted_statuses=(), + triage_observations={}, + rank_hypotheses=False, + outcome_mode="repair", + oracle_script=oracle, + oracle_sha256=hashlib.sha256(oracle.read_bytes()).hexdigest(), + ) + + def test_repair_pair_aborts_before_agent_when_git_is_unavailable(self): + with tempfile.TemporaryDirectory() as temp: + root = Path(temp) + fixture = root / "fixture" + (fixture / "src").mkdir(parents=True) + (fixture / "tests").mkdir() + (fixture / "src/service.py").write_text("VALUE = 1\n", encoding="utf-8") + (fixture / "tests/test_service.py").write_text("assert True\n", encoding="utf-8") + profile = self._profile(fixture, root / "oracle.py") + output = root / "output" + with ( + patch.object(runner, "_verify_codex_version", return_value=True), + patch.object(runner.core, "_copy_auth", return_value=True), + patch.object(runner.shutil, "which", return_value=None), + patch.object(runner, "_run_arm") as run_arm, + ): + receipt = runner.run_pair( + codex="codex", model="test-model", reasoning_effort="low", + timeout=2, seed=4, output_dir=output, case_profile=profile, + ) + self.assertEqual(receipt["failure"], "repair_git_setup_failed") + self.assertEqual(receipt["status"], "failed") + run_arm.assert_not_called() + self.assertEqual( + json.loads((output / "receipt.json").read_text())["failure"], + "repair_git_setup_failed", + ) + + def test_private_git_baseline_and_protected_digest_ignore_only_git_metadata(self): + with tempfile.TemporaryDirectory() as temp: + fixture = Path(temp) + (fixture / "src").mkdir() + (fixture / "src/service.py").write_text("VALUE = 1\n", encoding="utf-8") + before = runner._fixture_digest( + fixture, exclude_relative="src/service.py", exclude_internal_git=True, + ) + self.assertTrue(runner._initialize_private_git_baseline(fixture)) + after = runner._fixture_digest( + fixture, exclude_relative="src/service.py", exclude_internal_git=True, + ) + self.assertEqual(before, after) + self.assertNotEqual( + runner._fixture_digest(fixture, exclude_relative="src/service.py"), + before, + ) + + def test_whitespace_patch_fails_real_diff_check_and_repair_acceptance(self): + with tempfile.TemporaryDirectory() as temp: + fixture = Path(temp) + (fixture / "src").mkdir() + source = fixture / "src/service.py" + source.write_text("VALUE = 1\n", encoding="utf-8") + self.assertTrue(runner._initialize_private_git_baseline(fixture)) + source.write_text("VALUE = 2 \n", encoding="utf-8") + check = subprocess.run( + ["git", "diff", "--check"], cwd=fixture, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=False, + ) + self.assertNotEqual(check.returncode, 0) + receipt, _ = runner._event_receipts( + [ + _event("item.started", "diff", ["git", "diff", "--check"]), + _event("item.completed", "diff", ["git", "diff", "--check"], + exit_code=check.returncode, aggregated_output=check.stderr.decode()), + ], + [1.0, 2.0], 0.0, + profile=types.SimpleNamespace(outcome_mode="repair", focused_command=(), evidence_files={}), + ) + self.assertTrue(receipt["agent_git_diff_check_invocation_observed"]) + self.assertEqual(receipt["agent_git_diff_check_exit_code"], check.returncode) + self.assertFalse(receipt["agent_git_diff_check_passed"]) + self.assertFalse(runner._repair_git_diff_check_valid(receipt)) + + def test_missing_or_nonzero_observed_diff_check_cannot_accept_repair(self): + missing = _repair_diff_receipt(None, None) + self.assertFalse(missing["agent_git_diff_check_invocation_observed"]) + self.assertEqual(missing["agent_git_diff_check_exit_codes"], []) + self.assertFalse(runner._repair_git_diff_check_valid(missing)) + + nonzero = _repair_diff_receipt(["git", "diff", "--check"], 2) + self.assertTrue(nonzero["agent_git_diff_check_invocation_observed"]) + self.assertEqual(nonzero["agent_git_diff_check_exit_codes"], [2]) + self.assertFalse(nonzero["agent_git_diff_check_passed"]) + self.assertFalse(runner._repair_git_diff_check_valid(nonzero)) + + accepted = _repair_diff_receipt(["git", "diff", "--check"], 0) + self.assertTrue(runner._repair_git_diff_check_valid(accepted)) + + +if __name__ == "__main__": + unittest.main()