--- a/ops/autonomy/server-loop/server_loop.py +++ b/ops/autonomy/server-loop/server_loop.py @@ -609,29 +609,47 @@ def verify_commands(cfg, workspace: Path): + """Run checks fail-fast, but always run bounded, explicit artifact cleanup.""" rows = [] timeout = int(cfg.get("verification_timeout_seconds", 900)) - for raw in cfg.get("verification_commands", []): - try: - p = run(["bash", "-lc", raw], cwd=workspace, timeout=timeout) - row = { - "command": raw, - "returncode": p.returncode, - "stdout": text_tail(p.stdout), - "stderr": text_tail(p.stderr), - "timed_out": False, - } - except subprocess.TimeoutExpired as exc: - row = { - "command": raw, - "returncode": 124, - "stdout": text_tail(exc.stdout), - "stderr": text_tail(exc.stderr), - "timed_out": True, - } - rows.append(row) - if row["returncode"] != 0: - break + + def run_group(commands, group_timeout, group_name, fail_fast=True): + group_rows = [] + for raw in commands: + try: + proc = run(["bash", "-lc", raw], cwd=workspace, timeout=group_timeout) + row = { + "command": raw, + "returncode": proc.returncode, + "stdout": text_tail(proc.stdout), + "stderr": text_tail(proc.stderr), + "timed_out": False, + "phase": group_name, + } + except subprocess.TimeoutExpired as exc: + row = { + "command": raw, + "returncode": 124, + "stdout": text_tail(exc.stdout), + "stderr": text_tail(exc.stderr), + "timed_out": True, + "phase": group_name, + } + group_rows.append(row) + if fail_fast and row["returncode"] != 0: + break + return group_rows + + rows.extend(run_group(cfg.get("verification_commands", []), timeout, "verification")) + cleanup_timeout = max(1, min(int(cfg.get("verification_cleanup_timeout_seconds", 120)), 300)) + # This group runs even when a verifier fails or times out. Any cleanup failure + # remains a failed gate; the cleanup never converts a failed check into success. + rows.extend(run_group( + cfg.get("verification_cleanup_commands", []), + cleanup_timeout, + "cleanup", + fail_fast=False, + )) return rows --- a/ops/autonomy/server-loop/test_server_loop.py +++ b/ops/autonomy/server-loop/test_server_loop.py @@ -271,6 +271,109 @@ self.assertEqual(rows[0]["returncode"], 124) self.assertTrue(rows[0]["timed_out"]) self.assertEqual(rows[0]["stdout"], "partial") + + def test_verification_cleanup_runs_after_failure_and_preserves_failure(self): + original_run = loop.run + called = [] + + def fake_run(cmd, cwd=None, timeout=30): + raw = cmd[-1] + called.append(raw) + if raw == "failing-check": + return subprocess.CompletedProcess(cmd, 2, "", "failure") + if raw == "cleanup-generated": + return subprocess.CompletedProcess(cmd, 0, "cleaned", "") + if raw == "must-not-run": + return subprocess.CompletedProcess(cmd, 0, "unexpected", "") + raise AssertionError("unexpected command") + + loop.run = fake_run + try: + rows = loop.verify_commands( + { + "verification_commands": ["failing-check", "must-not-run"], + "verification_cleanup_commands": ["cleanup-generated"], + "verification_timeout_seconds": 5, + "verification_cleanup_timeout_seconds": 5, + }, + Path("/tmp"), + ) + finally: + loop.run = original_run + + self.assertEqual(called, ["failing-check", "cleanup-generated"]) + self.assertEqual([row["returncode"] for row in rows], [2, 0]) + self.assertEqual([row["phase"] for row in rows], ["verification", "cleanup"]) + self.assertEqual(rows[0]["stderr"], "failure") + self.assertEqual(rows[1]["stdout"], "cleaned") + + def test_verification_cleanup_timeout_remains_a_failure(self): + original_run = loop.run + called = [] + + def fake_run(cmd, cwd=None, timeout=30): + raw = cmd[-1] + called.append(raw) + if raw == "slow-check": + raise subprocess.TimeoutExpired(cmd, timeout, output="partial", stderr="late") + if raw == "cleanup-generated": + return subprocess.CompletedProcess(cmd, 0, "cleaned", "") + raise AssertionError("unexpected command") + + loop.run = fake_run + try: + rows = loop.verify_commands( + { + "verification_commands": ["slow-check"], + "verification_cleanup_commands": ["cleanup-generated"], + "verification_timeout_seconds": 1, + "verification_cleanup_timeout_seconds": 5, + }, + Path("/tmp"), + ) + finally: + loop.run = original_run + + self.assertEqual(called, ["slow-check", "cleanup-generated"]) + self.assertEqual(rows[0]["returncode"], 124) + self.assertTrue(rows[0]["timed_out"]) + self.assertEqual(rows[1]["returncode"], 0) + self.assertEqual(rows[1]["phase"], "cleanup") + + def test_verification_cleanup_removes_real_generated_artifacts_after_failure(self): + import tempfile + + with tempfile.TemporaryDirectory() as td: + root = Path(td) + artifact = root / "api" / "example_pkg.egg-info" / "SOURCES.txt" + + def fail_after_generating_artifact(): + artifact.parent.mkdir(parents=True) + artifact.write_text("generated metadata") + return 2 + + command = ( + "mkdir -p api/example_pkg.egg-info && " + "printf generated > api/example_pkg.egg-info/SOURCES.txt && exit 2" + ) + cleanup = ( + "find api -type d -name '*.egg-info' -prune " + "-exec rm -rf -- {} +" + ) + rows = loop.verify_commands( + { + "verification_commands": [command], + "verification_cleanup_commands": [cleanup], + "verification_timeout_seconds": 5, + "verification_cleanup_timeout_seconds": 5, + }, + root, + ) + self.assertEqual(rows[0]["returncode"], 2) + self.assertEqual(rows[1]["returncode"], 0) + self.assertEqual(rows[1]["phase"], "cleanup") + self.assertFalse(artifact.exists()) + self.assertFalse(artifact.parent.exists()) def test_verified_candidate_path_prefixes_allow_api_and_server_loop(self): prefixes = ["api/", "ops/autonomy/server-loop/"] --- a/ops/autonomy/server-loop/config.example.json +++ b/ops/autonomy/server-loop/config.example.json @@ -10,7 +10,10 @@ "codex_model": null, "codex_timeout_seconds": 1800, "verification_timeout_seconds": 900, - "verified_candidate_path_prefixes": ["api/", "ops/autonomy/server-loop/"], + "verified_candidate_path_prefixes": [ + "api/", + "ops/autonomy/server-loop/" + ], "verification_commands": [ "git diff --check", "python3 -m py_compile ops/autonomy/server-loop/server_loop.py ops/autonomy/server-loop/migrate_config.py", @@ -21,5 +24,9 @@ "uv lock --project api --check", "uv run --project api ruff check api/controllers/web/completion.py api/controllers/web/darkoza.py api/controllers/web/passport.py api/services/darkoza_account_identity_service.py api/services/darkoza_default_assistant_service.py api/services/darkoza_payment_service.py api/tests/unit_tests/controllers/web/test_darkoza.py api/tests/unit_tests/controllers/web/test_passport.py api/tests/unit_tests/services/test_darkoza_account_identity_service.py api/tests/unit_tests/services/test_darkoza_default_assistant_service.py api/tests/unit_tests/services/test_darkoza_payment_service.py", "uv run --project api pytest --no-cov --timeout 30 api/tests/unit_tests/controllers/web/test_darkoza.py api/tests/unit_tests/controllers/web/test_passport.py api/tests/unit_tests/services/test_darkoza_account_identity_service.py api/tests/unit_tests/services/test_darkoza_default_assistant_service.py api/tests/unit_tests/services/test_darkoza_payment_service.py" + ], + "verification_cleanup_timeout_seconds": 120, + "verification_cleanup_commands": [ + "rm -rf -- .ruff_cache; for root in api dify-agent ops/autonomy/server-loop; do [ -d \"$root\" ] || continue; while IFS= read -r -d '' path; do rm -rf -- \"$path\"; done < <(find \"$root\" -type d \\( -name .venv -o -name build -o -name '*.egg-info' -o -name __pycache__ -o -name .pytest_cache -o -name .ruff_cache -o -name .mypy_cache \\) -prune -print0); done" ] }