diff --git a/.github/workflows/runtime.yml b/.github/workflows/runtime.yml new file mode 100644 index 0000000..49422e8 --- /dev/null +++ b/.github/workflows/runtime.yml @@ -0,0 +1,27 @@ +name: Docker lifecycle + +on: + push: + pull_request: + +permissions: + contents: read + +concurrency: + group: runtime-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + runtime: + name: Real Docker lifecycle + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6 + with: + persist-credentials: false + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6 + with: + python-version: '3.11' + - name: Exercise fake reviewer in real Docker + run: python scripts/test_runtime.py diff --git a/README.md b/README.md index 8bfa96b..c8dc6c1 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,13 @@ see only pinned source/evidence and temporary Codex authentication, keeping answ keys outside the reviewer filesystem. Three matched repetitions, held-out fixtures, critical-defect recall, precision and completion gates precede profile promotion. +On a POSIX host with Linux Docker, run `python3 scripts/test_runtime.py` for the +same lifecycle checks run by Docker CI. The tests build an image with a fake Codex +executable and synthetic authentication; they run no inference. They cover stdin, +read-only source isolation, output capture, retained failures, trial timeout and +interruption, and version-probe cleanup. The pinned Python base image may need to +be downloaded. These checks do not measure review quality or model capacity. + Keep review artifacts, campaign data, local profiles, credentials, private rules, caches and benchmark results outside the package and reviewed repository. Provenance for adapted review guidance is pinned in [PROVENANCE.json](PROVENANCE.json), with diff --git a/codex/skills/code-review-c/references/onboarding.md b/codex/skills/code-review-c/references/onboarding.md index 25d952a..9b7555c 100644 --- a/codex/skills/code-review-c/references/onboarding.md +++ b/codex/skills/code-review-c/references/onboarding.md @@ -61,8 +61,11 @@ keep all other evidence identical within each matched trial. ## Run isolated trials -Provide an immutable Docker image (`IMAGE@sha256:...`) containing the desired Codex +Provide an immutable Docker image (`IMAGE@sha256:...` or a local `sha256:IMAGE_ID`) containing the desired Codex version and its runtime dependencies, with `codex` on PATH and no custom entrypoint. +Local image IDs must already exist in the selected Docker engine; mutable tags +are rejected. The Docker lifecycle test builds its own fake reviewer and uses its +immutable local ID without a registry adapter or model calls. Provide a sanitized host config containing **only** `model` and `model_reasoning_effort`, matching the frozen host-selected identity. The harness does not invent candidate overrides. Provide the existing host's `auth.json`; it diff --git a/codex/skills/code-review-c/references/operations.md b/codex/skills/code-review-c/references/operations.md index 9a65393..e3d352a 100644 --- a/codex/skills/code-review-c/references/operations.md +++ b/codex/skills/code-review-c/references/operations.md @@ -6,6 +6,9 @@ The first-release support target is Linux with CPython 3.11–3.15, Git, and a C host providing native agents. GitHub PR pinning additionally requires authenticated `gh`. Container trials require a working Linux Docker engine and an immutable image containing the selected Codex version. Review and preview do not need Docker. +Use a registry reference pinned as `NAME@sha256:DIGEST`, or an immutable local +`sha256:IMAGE_ID` from `docker image inspect`. Local IDs are specific to the Docker +engine and must be loaded on another host before use; mutable tags are rejected. Other host platforms and Python versions have not been validated for this release. Install the `v0.1.0` tag as described in the repository README. Keep an old installed diff --git a/codex/skills/code-review-c/scripts/reviewlib/onboarding.py b/codex/skills/code-review-c/scripts/reviewlib/onboarding.py index b98e2f1..0849809 100644 --- a/codex/skills/code-review-c/scripts/reviewlib/onboarding.py +++ b/codex/skills/code-review-c/scripts/reviewlib/onboarding.py @@ -235,14 +235,14 @@ def run(root, image, host_config, auth, max_calls, max_wall_seconds, trial_secon """ root = external(root) c = read(root) - if not re.fullmatch(r"[\w./:-]+@sha256:[0-9a-f]{64}", image): + if not re.fullmatch(r"(?:[\w./:-]+@)?sha256:[0-9a-f]{64}", image): raise ValueError("trial image must be pinned by sha256 digest") limits = c["limits"] for key, value in (("max_calls", max_calls), ("max_wall_seconds", max_wall_seconds), ("trial_seconds", trial_seconds)): if type(value) is not int or value <= 0 or value > limits[key]: raise ValueError("explicit positive run limits must fit the frozen campaign") # A previously started trial still spends its call allowance after a crash. - attempts = [load(p) for p in (root / "attempts").glob("*/receipt.json")] + attempts = [load(p, "trial-receipt") for p in (root / "attempts").glob("*/receipt.json")] used_calls = sum(bool(r.get("launched")) for r in attempts) used_time = sum(max(r.get("reserved_seconds", trial_seconds), r.get("elapsed_seconds", trial_seconds)) if r.get("status") in {"started", "interrupted", "cleanup-failed"} diff --git a/codex/skills/code-review-c/scripts/reviewlib/records.py b/codex/skills/code-review-c/scripts/reviewlib/records.py index c84e436..112b707 100644 --- a/codex/skills/code-review-c/scripts/reviewlib/records.py +++ b/codex/skills/code-review-c/scripts/reviewlib/records.py @@ -19,7 +19,8 @@ def record(kind, **values): def load(path, kind=None): value = json.loads(Path(path).read_text()) - if kind and (not isinstance(value, dict) or value.get("kind") != kind or value.get("version") != VERSION): + if kind and (not isinstance(value, dict) or value.get("kind") != kind + or type(value.get("version")) is not int or value["version"] != VERSION): raise ValueError(f"expected {kind} version {VERSION}: {path}") return value diff --git a/scripts/test_runtime.py b/scripts/test_runtime.py new file mode 100644 index 0000000..24d2928 --- /dev/null +++ b/scripts/test_runtime.py @@ -0,0 +1,184 @@ +#!/usr/bin/env python3 +"""Exercise the real Docker/CLI lifecycle with a fake reviewer; never calls a model.""" +import json +import os +import re +import signal +import subprocess +import sys +import tempfile +import time +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(ROOT / "tests")) +from support import GitCase, SKILL +from reviewlib import onboarding +from reviewlib.records import digest, load, validate_output + + +class RuntimeTests(GitCase): + images = {} + + def setUp(self): + super().setUp() + self.campaign_spec() + self.croot = self.root / "campaign" + onboarding.freeze(self.root / "spec.json", self.croot) + self.config = self.root / "host.toml" + self.config.write_text('model = "synthetic-exact-model"\nmodel_reasoning_effort = "high"\n') + self.auth = self.root / "auth.json" + self.auth.write_text('{"synthetic": true, "mode": "finished"}') + self.auth.chmod(0o600) + self.trial = self.croot / "attempts/train0--focused--0" + self.containers = set() + + def tearDown(self): + try: + # Clean only this test's named containers, after outcome assertions. + for trial in (self.croot / "attempts").glob("*"): + self.containers.add("crc-" + digest(str(trial))[:20]) + for name in sorted(self.containers): + subprocess.run(["docker", "rm", "-f", name], capture_output=True, timeout=15, check=True) + finally: + super().tearDown() + + def command(self, image="finished", seconds=10): + return [sys.executable, str(SKILL / "scripts/review.py"), "onboard", "run", + "--campaign", str(self.croot), "--image", self.images[image], + "--host-config", str(self.config), "--auth", str(self.auth), + "--max-calls", "1", "--max-wall-seconds", "60", "--trial-seconds", str(seconds)] + + def container_absent(self, name): + # A daemon error must not masquerade as successful container removal. + names = subprocess.check_output(["docker", "ps", "-a", "--format", "{{.Names}}"], + text=True, timeout=15).splitlines() + self.assertNotIn(name, names, "Container survived production cleanup") + + def assert_trial_removed(self, trial=None): + self.container_absent("crc-" + digest(str(trial or self.trial))[:20]) + + def run_mode(self, mode, seconds=10): + self.auth.write_text(json.dumps({"synthetic": True, "mode": mode})) + result = subprocess.run(self.command(seconds=seconds), capture_output=True, text=True, timeout=60) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(json.loads(result.stdout)["launched"], 1) + receipt = load(self.trial / "receipt.json", "trial-receipt") + self.assertEqual(receipt["status"], mode) + self.assertTrue(receipt["launched"]) + self.assert_trial_removed() + return receipt + + def test_success_captures_stdin_output_usage_and_removes_container(self): + receipt = self.run_mode("finished") + self.assertEqual(receipt["tokens"], 120) + self.assertEqual(receipt["trace_hash"], digest((self.trial / "trace.jsonl").read_bytes())) + self.assertEqual(receipt["result_hash"], digest((self.trial / "result.json").read_bytes())) + result = load(self.trial / "result.json", "calibration-result") + validate_output(result, "calibration-result") + self.assertEqual(result["status"], "complete") + self.assertTrue(result["inspected_hunks"]) + + def test_process_failure_is_retained_on_resume(self): + self.run_mode("failed") + before = (self.trial / "receipt.json").read_bytes() + result = subprocess.run(self.command(), capture_output=True, text=True, timeout=60) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(json.loads(result.stdout)["launched"], 1) + self.assertEqual((self.trial / "receipt.json").read_bytes(), before) + second = self.croot / "attempts/train0--focused--1" + self.assertEqual(load(second / "receipt.json", "trial-receipt")["status"], "failed") + self.assert_trial_removed(second) + + def test_timeout_retains_incomplete_evidence_and_removes_container(self): + receipt = self.run_mode("timeout", seconds=3) + self.assertIsNone(receipt["result_hash"]) + self.assertIsNone(receipt["tokens"]) + self.assertTrue(receipt["trace_hash"]) + + def interrupted_run(self, image, ready): + stderr_path = self.root / "runner-stderr.txt" + with (self.root / "runner-stdout.txt").open("wb") as stdout, stderr_path.open("wb") as stderr: + with subprocess.Popen(self.command(image, seconds=30), stdout=stdout, stderr=stderr) as process: + try: + deadline = time.monotonic() + 20 + while time.monotonic() < deadline: + if ready(stderr_path): + break + if process.poll() is not None: + self.fail("Runner exited before fixture started: " + stderr_path.read_text()) + time.sleep(0.05) + else: + self.fail("Fixture did not start: " + stderr_path.read_text()) + process.send_signal(signal.SIGINT) + process.wait(timeout=25) + self.assertNotEqual(process.returncode, 0) + finally: + if process.poll() is None: + process.kill() + process.wait(timeout=10) + return stderr_path.read_text() + + def test_interrupt_retains_attempt_and_removes_container(self): + self.auth.write_text('{"synthetic": true, "mode": "interrupted"}') + trace = self.trial / "trace.jsonl" + self.interrupted_run("finished", lambda _: trace.exists() and "runtime.fake.started" in trace.read_text()) + receipt = load(self.trial / "receipt.json", "trial-receipt") + self.assertEqual(receipt["status"], "interrupted") + self.assertTrue(receipt["launched"]) + self.assertGreater(receipt["elapsed_seconds"], 0) + self.assertIsNone(receipt["result_hash"]) + self.assert_trial_removed() + + def probe_name(self, stderr): + match = re.search(r"Version probe container: (crc-version-[0-9a-f]{32})", stderr) + self.assertIsNotNone(match, stderr) + name = match.group(1) + self.containers.add(name) + return name + + def test_version_probe_timeout_removes_container_without_starting_trial(self): + result = subprocess.run(self.command("timeout"), capture_output=True, text=True, timeout=60) + self.assertNotEqual(result.returncode, 0) + name = self.probe_name(result.stderr) + self.assertIn("timed out", result.stderr) + self.assertFalse((self.croot / "attempts").exists()) + self.container_absent(name) + + def test_version_probe_interrupt_removes_container_without_starting_trial(self): + def running(path): + stderr = path.read_text() + if "Version probe container:" not in stderr: + return False + name = self.probe_name(stderr) + result = subprocess.run(["docker", "inspect", "--format", "{{.State.Running}}", name], + capture_output=True, text=True, timeout=15) + return result.returncode == 0 and result.stdout.strip() == "true" + + stderr = self.interrupted_run("interrupted", running) + self.assertFalse((self.croot / "attempts").exists()) + self.container_absent(self.probe_name(stderr)) + + +def main(): + if os.name != "posix": + raise SystemExit("Docker lifecycle tests require a POSIX host") + engine = subprocess.check_output(["docker", "info", "--format", "{{.OSType}}"], text=True, timeout=30).strip() + if engine != "linux": + raise SystemExit("Docker lifecycle tests require Linux containers") + with tempfile.TemporaryDirectory(prefix="crc-runtime-build-") as temp: + for mode in ("finished", "timeout", "interrupted"): + iid = Path(temp) / mode + subprocess.run(["docker", "build", "--iidfile", str(iid), "--build-arg", "PROBE_MODE=" + mode, + str(ROOT / "tests/runtime")], check=True, timeout=300) + image = iid.read_text().strip() + if not re.fullmatch(r"sha256:[0-9a-f]{64}", image): + raise ValueError("Docker did not return an immutable image ID") + RuntimeTests.images[mode] = image + suite = unittest.defaultTestLoader.loadTestsFromTestCase(RuntimeTests) + return not unittest.TextTestRunner(verbosity=2).run(suite).wasSuccessful() + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/runtime/Dockerfile b/tests/runtime/Dockerfile new file mode 100644 index 0000000..6651887 --- /dev/null +++ b/tests/runtime/Dockerfile @@ -0,0 +1,4 @@ +FROM python:3.11-slim@sha256:b7a9e5f6a93d6d4a31f129bbd4bdee69efccb6ded0fb230d78aafaf37af98377 +ARG PROBE_MODE=finished +ENV CRC_TEST_PROBE_MODE=${PROBE_MODE} +COPY --chmod=0755 fake_codex.py /usr/local/bin/codex diff --git a/tests/runtime/fake_codex.py b/tests/runtime/fake_codex.py new file mode 100644 index 0000000..2ee0d7f --- /dev/null +++ b/tests/runtime/fake_codex.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python3 +"""Docker protocol fixture: synthetic credentials, no inference or network calls.""" +import errno +import json +import os +import re +import sys +import time +from pathlib import Path + +if sys.argv[1:] == ["--version"]: + if os.environ["CRC_TEST_PROBE_MODE"] != "finished": + time.sleep(120) + print("codex-cli synthetic") + sys.exit(0) + +assert "--ephemeral" in sys.argv and "read-only" in sys.argv +auth = json.loads(Path("/codex-home/auth.json").read_text()) +assert auth["synthetic"] is True +assert Path("/input/schema.json").is_file() +assert {p.name for p in Path("/workspace").iterdir()} == {"base", "head"} +assert not Path("/grading/keys.json").exists() +try: + Path("/workspace/head/a.py").write_text("mutated") +except OSError as error: + assert error.errno == errno.EROFS, "Pinned workspace must use a read-only mount" +else: + raise AssertionError("Pinned workspace is writable") + +prompt = sys.stdin.read() +assert "OWNED CHANGED HUNKS" in prompt, "Docker must forward the task prompt on stdin" +print(json.dumps({"type": "runtime.fake.started", "synthetic": True}), flush=True) +if auth["mode"] in {"timeout", "interrupted"}: + time.sleep(120) +if auth["mode"] == "failed": + sys.exit(23) + +result = {"kind": "calibration-result", "version": 1, "status": "complete", + "inspected_hunks": re.findall(r"^HUNK ([0-9a-f]+)$", prompt, flags=re.M), + "extra_visible_bytes": 0, "findings": [], "gaps": []} +Path(sys.argv[sys.argv.index("-o") + 1]).write_text(json.dumps(result)) +print(json.dumps({"type": "turn.completed", "usage": {"input_tokens": 100, "output_tokens": 20}}), flush=True) diff --git a/tests/test_installation.py b/tests/test_installation.py new file mode 100644 index 0000000..69464a8 --- /dev/null +++ b/tests/test_installation.py @@ -0,0 +1,82 @@ +"""Fresh-install CLI integration with synthetic receipts, without inference.""" +import json +import shutil +import subprocess +import sys + +from support import GitCase, ROOT, SKILL, receipts +from reviewlib.records import save + + +class InstallationTests(GitCase): + def exercise_installation(self, plugin): + install = self.root / "fresh install" + if plugin: + skill = install / "codex/skills/code-review-c" + for relative in (".codex-plugin/plugin.json", ".agents/plugins/marketplace.json"): + target = install / relative + target.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(ROOT / relative, target) + else: + skill = install / "skills/code-review-c" + shutil.copytree(SKILL, skill, ignore=shutil.ignore_patterns("__pycache__")) + helper = skill / "scripts/review.py" + cwd = self.root / "unrelated working directory" + cwd.mkdir() + + def cli(*args, success=True): + result = subprocess.run([sys.executable, str(helper), *map(str, args)], cwd=cwd, + capture_output=True, text=True, timeout=30) + if success: + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + return json.loads(result.stdout) + self.assertNotEqual(result.returncode, 0) + return json.loads(result.stderr) + + bundle, plan_path = self.root / "bundle", self.root / "plan.json" + cli("inventory", "--repo", self.repo, "--base", self.base, "--head", self.head, "--out", bundle) + plan = cli("plan", "--bundle", bundle, "--out", plan_path) + common = ("--bundle", bundle, "--plan", plan_path) + self.assertTrue(cli("validate", *common)["valid"]) + results = self.root / "results" + results.mkdir() + schedule_args = (*common, "--results", results, "--capacity", "2") + initial = cli("next", *schedule_args) + self.assertEqual(initial["gate"], "pending") + self.assertEqual(len(initial["launch"]), 2) + self.assertTrue(initial["queued"]) + + for task, receipt in zip(plan["tasks"], receipts(plan)): + prompt = self.root / (task["id"] + ".txt") + cli("prompt", *common, "--task", task["id"], "--out", prompt) + self.assertIn("OWNED CHANGED HUNKS", prompt.read_text()) + self.assertTrue(set(task["hunks"])) + for hunk in task["hunks"]: + self.assertIn(hunk, prompt.read_text()) + save(results / (task["id"] + ".json"), receipt) + report = cli("report", *common, "--results", results) + self.assertEqual(report["gate"], "clean") + self.assertEqual(report["coverage_gaps"], []) + self.assertEqual(report["findings"], []) + + # Resume retained work, but reject receipts for stale task inputs. + receipt_path = results / (plan["tasks"][0]["id"] + ".json") + original = json.loads(receipt_path.read_text()) + receipt_path.unlink() + resumed = cli("next", *schedule_args) + self.assertEqual(resumed["launch"], [original["task_id"]]) + self.assertEqual(resumed["gate"], "pending") + save(receipt_path, {**original, "input_hash": "0" * 64}) + error = cli("report", *common, "--results", results, success=False) + self.assertIn("stale task receipt", error["error"]) + save(receipt_path, original) + self.assertEqual(cli("report", *common, "--results", results)["gate"], "clean") + + for forbidden in (skill / "state", skill / "profiles", self.repo / "results"): + self.assertFalse(forbidden.exists()) + + def test_copied_skill_runs_from_unrelated_directory(self): + self.exercise_installation(plugin=False) + + def test_plugin_layout_runs_from_unrelated_directory(self): + self.exercise_installation(plugin=True) diff --git a/tests/test_onboarding.py b/tests/test_onboarding.py index b47091b..1db31c8 100644 --- a/tests/test_onboarding.py +++ b/tests/test_onboarding.py @@ -472,7 +472,7 @@ def test_combined_layout_cannot_be_promoted(self): with self.assertRaisesRegex(ValueError, "combined controls"): onboarding.promote(self.croot, "combined", self.root / "profiles") - def fake_runner(self, timeout=False, interrupt=False, expected_launches=1): + def fake_runner(self, timeout=False, interrupt=False, expected_launches=1, image=None): config = self.root / "host.toml" config.write_text('model = "synthetic-exact-model"\nmodel_reasoning_effort = "high"\n') auth = self.root / "auth.json" @@ -507,7 +507,7 @@ def run(command, **kwargs): patch.object(onboarding.subprocess, "check_output", return_value="codex-cli synthetic\n"), \ patch.object(onboarding.subprocess, "run", side_effect=run): try: - result = onboarding.run(self.croot, "example/codex@sha256:" + "a" * 64, config, auth, 1, 10, 5) + result = onboarding.run(self.croot, image or "example/codex@sha256:" + "a" * 64, config, auth, 1, 10, 5) finally: self.assertEqual(len(cleanups), expected_launches + 1) self.assertEqual(result["launched"], expected_launches) @@ -522,6 +522,34 @@ def test_runner_captures_trace_and_enforces_call_limit_without_models(self): self.assertTrue(receipt["result_hash"]) self.assertEqual(receipt["status"], "finished") + def test_runner_accepts_immutable_local_image_id(self): + self.freeze() + receipt = self.fake_runner(image="sha256:" + "b" * 64) + self.assertEqual(receipt["status"], "finished") + + def test_resume_rejects_invalid_receipts_before_docker_or_new_attempts(self): + self.freeze() + receipt = self.fake_runner() + path = self.croot / "attempts/train0--focused--0/receipt.json" + cases = [("kind", "task-result"), ("version", 2), ("version", True), ("version", 1.0)] + for field, value in cases: + with self.subTest(field=field, value=value): + save(path, {**receipt, field: value}) + before = path.read_bytes() + with patch.object(onboarding.subprocess, "check_output", return_value="codex-cli synthetic\n") as probe, \ + patch.object(onboarding.subprocess, "run", side_effect=AssertionError("Invalid receipt reached Docker")) as runner: + with self.assertRaisesRegex(ValueError, "expected trial-receipt version 1"): + onboarding.run(self.croot, "example/codex@sha256:" + "b" * 64, + self.root / "host.toml", self.root / "auth.json", 1, 10, 5) + probe.assert_not_called() + runner.assert_not_called() + self.assertEqual(path.read_bytes(), before) + self.assertEqual(list((self.croot / "attempts").iterdir()), [path.parent]) + save(path, receipt) + self.fake_runner() + self.assertEqual(load(path), receipt) + self.assertTrue((self.croot / "attempts/train0--focused--1/receipt.json").is_file()) + def test_runner_timeout_retains_failed_attempt(self): self.freeze() receipt = self.fake_runner(timeout=True)