Harden reviewer runtime and profile output boundaries

.github/workflows/runtime.yml

@@ -0,0 +1,24 @@
+name: Runtime integration
+
+on:
+  pull_request:
+  push:
+    branches: [main, feat/adaptive-review-controller]
+
+permissions:
+  contents: read
+
+concurrency:
+  group: runtime-${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  docker:
+    runs-on: ubuntu-latest
+    timeout-minutes: 10
+    steps:
+      - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6
+      - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6
+        with:
+          python-version: '3.11'
+      - run: python3 scripts/test_runtime.py

README.md

@@ -51,6 +51,12 @@ 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 Docker, run `python3 scripts/test_runtime.py` for real container
+lifecycle checks using a bundled fake reviewer and synthetic authentication. The
+image build can download its pinned Python base image; no inference runs. These
+checks cover read-only source mounts, output capture, failures, timeouts, interruption,
+and container removal. They do not measure review quality or establish 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

codex/skills/code-review-c/references/onboarding.md

@@ -26,9 +26,10 @@ storage. Preparation, freezing, grading, comparison, and promotion launch no mod
    Do not substitute a model or infer reviewer identity from the controller.
 4. Set explicit campaign limits: maximum model calls, total elapsed trial seconds,
    and per-trial seconds. Zero defaults prohibit execution. All started calls,
-   including failed and interrupted attempts, consume the call allowance. Elapsed
-   time of finished attempts and reserved time for interrupted attempts consume
-   the frozen time allowance. These are call/time limits, not a guaranteed dollar
+   including failed and interrupted attempts, consume the call allowance. Started,
+   interrupted and cleanup-failed attempts spend their reserved time or their
+   recorded elapsed time, whichever is larger. Other attempts spend recorded
+   elapsed time. These consume the frozen time allowance, not a guaranteed dollar
    or token cap; no token price assumptions are made.
 5. Freeze with `HELPER onboard freeze --spec SPEC --out CAMPAIGN`. Frozen source
    trees, prompt hashes, identity, limits and gold keys are immutable. If a fixture,
@@ -63,13 +64,19 @@ keep all other evidence identical within each matched trial.
 
 Provide an immutable Docker image (`IMAGE@sha256:...`) containing the desired Codex
 version and its runtime dependencies, with `codex` on PATH and no custom entrypoint.
+An exact local image ID (`sha256:...`, as returned by `docker build --iidfile`)
+is also accepted. Tags alone are mutable and rejected.
 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
 is copied into an ephemeral private home and removed on exit. API-key or custom
 provider setups need an audited equivalent image/auth adapter; this runner supports
 normal Codex auth.json login. Do not mount the host's full Codex home.
 
+The version check runs in its own `crc-version-*` container. Timeout and SIGINT
+also attempt bounded removal; a removal failure makes the command fail and needs
+the recovery steps below.
+
 After explicitly authorizing model calls with limits, run:
 
 ```sh
@@ -90,8 +97,15 @@ with a structured calibration output schema. No child agents are permitted. The
 runner captures traces, stderr, structured findings, timing, usage, and input hashes.
 It executes serially to make matched latency easier to assess. Explicit limits stop
 new launches, timeouts remove containers, and existing attempts are never overwritten.
-An interrupted `started` receipt remains incomplete; preserve it. No automatic retry
-turns a failed attempt into a passing one. Start a fresh campaign for replacement trials.
+An interrupted receipt remains incomplete; preserve it. SIGINT records `interrupted`
+when removal succeeds. Removal errors or timeouts record `cleanup-failed` and can
+leave a container running; abrupt process loss can leave `started`.
+Use `docker ps -a --filter name=crc-` and `docker inspect NAME` to identify the
+affected container. Trial mounts identify its campaign. Version probes have
+`crc-version-*` names; compare their pinned image and any name in the command error.
+Remove only the affected container with
+`docker rm -f NAME` before continuing. Preserve receipts. No automatic retry turns
+a failed attempt into a passing one. Start a fresh campaign for replacement trials.
 
 ## Grade, compare, promote
 

codex/skills/code-review-c/scripts/reviewlib/onboarding.py

@@ -205,7 +205,7 @@ 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)):
@@ -214,7 +214,8 @@ def run(root, image, host_config, auth, max_calls, max_wall_seconds, trial_secon
     # A previously started trial still spends its call allowance after a crash.
     attempts = list((root / "attempts").glob("*/receipt.json"))
     used_calls = sum(bool(load(p).get("launched")) for p in attempts)
-    used_time = sum(load(p).get("reserved_seconds", trial_seconds) if load(p).get("status") == "started"
+    used_time = sum(max(load(p).get("reserved_seconds", trial_seconds), load(p).get("elapsed_seconds", trial_seconds))
+                    if load(p).get("status") in {"started", "interrupted", "cleanup-failed"}
                     else load(p).get("elapsed_seconds", trial_seconds) for p in attempts)
     config = __import__("tomllib").loads(Path(host_config).read_text())
     if set(config) != {"model", "model_reasoning_effort"}:
@@ -223,7 +224,11 @@ def run(root, image, host_config, auth, max_calls, max_wall_seconds, trial_secon
         raise ValueError("host-selected identity differs from frozen campaign")
     if not shutil.which("docker"):
         raise ValueError("Docker is required for isolated model trials")
-    version = subprocess.check_output(["docker", "run", "--rm", "--network=none", "--entrypoint", "codex", image, "--version"], text=True, timeout=30).strip()
+    probe = "crc-version-" + os.urandom(12).hex()
+    try:
+        version = subprocess.check_output(["docker", "run", "--rm", "--name", probe, "--network=none", "--entrypoint", "codex", image, "--version"], text=True, timeout=30).strip()
+    finally:
+        subprocess.run(["docker", "rm", "-f", probe], stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, check=True, timeout=15)
     if version != c["identity"]["codex_version"]:
         raise ValueError("container Codex version differs from frozen identity")
     schema = SKILL / "references" / "schemas" / "calibration-result.json"
@@ -259,7 +264,7 @@ def run(root, image, host_config, auth, max_calls, max_wall_seconds, trial_secon
                     shutil.copyfile(local_ref(root, prompt["ref"]), temp / "input" / "prompt.txt")
                     shutil.copyfile(schema, temp / "input" / "schema.json")
                     container = "crc-" + digest(str(trial))[:20]
-                    command = ["docker", "run", "--rm", "--name", container, "--entrypoint", "codex",
+                    command = ["docker", "run", "--rm", "--interactive", "--name", container, "--entrypoint", "codex",
                         "--cap-drop=ALL", "--security-opt=no-new-privileges", "--read-only",
                         "--tmpfs", "/tmp:rw,noexec,nosuid,size=256m", "--user", f"{os.getuid()}:{os.getgid()}",
                         "-e", "CODEX_HOME=/codex-home", "-e", "HOME=/tmp",
@@ -282,6 +287,9 @@ def run(root, image, host_config, auth, max_calls, max_wall_seconds, trial_secon
                         receipt["status"] = "finished" if result.returncode == 0 else "failed"
                     except subprocess.TimeoutExpired:
                         receipt["status"] = "timeout"
+                    except KeyboardInterrupt:
+                        receipt["status"] = "interrupted"
+                        raise
                     except OSError as error:
                         receipt["status"] = "failed"
                         receipt["error"] = str(error)
@@ -450,8 +458,10 @@ def compare(root):
 
 
 def promote(root, strategy, output):
-    output = external(output)
     c = read(root)
+    repositories = [inventory.read(Path(root) / "fixtures" / f["id"] / "bundle")["repo"]
+                    for f in c["fixtures"]]
+    output = external(output, *repositories)
     report = compare(root)
     selected = next(s for s in report["strategies"] if s["id"] == strategy)
     if not report["qualifying"] or not selected["passing"]:
@@ -468,8 +478,19 @@ def promote(root, strategy, output):
         limits=selected["measured_limits"], quality={k: selected["heldout"][k] for k in ("recall", "precision")},
         cost={"tokens": selected["heldout"]["tokens"], "latency_seconds": selected["heldout"]["latency_seconds"]},
         evidence={"report": report_ref, "sha256": digest(report), "campaign_id": c["id"]})
-    save(output / report_ref, report)
-    save(output / (profile["id"] + ".json"), profile)
+    report_path = local_ref(output, report_ref)
+    profile_path = local_ref(output, profile["id"] + ".json")
+    if report_path == profile_path:
+        raise ValueError("profile export destinations must be distinct")
+    for path in (report_path, profile_path):
+        external(path, *repositories)
+    output.mkdir(parents=True, exist_ok=True)
+    with tempfile.TemporaryDirectory(prefix=".profile-export-", dir=output) as temporary:
+        for name, path, value in (("report.json", report_path, report), ("profile.json", profile_path, profile)):
+            staged = Path(temporary) / name
+            save(staged, value)
+            path.parent.mkdir(parents=True, exist_ok=True)
+            staged.replace(path)
     return profile
 
 

scripts/test_runtime.py

@@ -0,0 +1,141 @@
+#!/usr/bin/env python3
+"""Run real Docker lifecycle checks with synthetic auth and a fake Codex executable."""
+import argparse
+import json
+import os
+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
+
+
+class RuntimeTests(GitCase):
+    image = None
+
+    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.trial = self.croot / "attempts/train0--focused--0"
+
+    def tearDown(self):
+        try:
+            for trial in (self.croot / "attempts").glob("*"):
+                with self.subTest(cleanup=trial.name):
+                    subprocess.run(["docker", "rm", "-f", "crc-" + digest(str(trial))[:20]],
+                                   capture_output=True, timeout=15, check=True)
+        finally:
+            super().tearDown()
+
+    def credentials(self, mode):
+        self.auth.write_text(json.dumps({"synthetic": True, "mode": mode}))
+        self.auth.chmod(0o600)
+
+    def container_absent(self, trial=None):
+        name = "crc-" + digest(str(trial or self.trial))[:20]
+        result = subprocess.run(["docker", "container", "inspect", name],
+                                capture_output=True, timeout=15)
+        self.assertNotEqual(result.returncode, 0, "Trial container survived cleanup")
+
+    def run_mode(self, mode, seconds=5):
+        self.credentials(mode)
+        result = onboarding.run(self.croot, self.image, self.config, self.auth, 1, 30, seconds)
+        self.assertEqual(result["launched"], 1)
+        receipt = load(self.trial / "receipt.json", "trial-receipt")
+        self.assertEqual(receipt["status"], mode)
+        self.assertTrue(receipt["launched"])
+        self.container_absent()
+        return receipt
+
+    def test_success_captures_output_usage_and_removes_container(self):
+        receipt = self.run_mode("finished")
+        self.assertEqual(receipt["tokens"], 120)
+        self.assertTrue(receipt["trace_hash"])
+        self.assertTrue(receipt["result_hash"])
+        result = load(self.trial / "result.json", "calibration-result")
+        self.assertEqual(result["status"], "complete")
+        self.assertTrue(result["inspected_hunks"])
+
+    def test_process_failure_is_retained_without_retry(self):
+        self.run_mode("failed")
+        before = (self.trial / "receipt.json").read_bytes()
+        result = onboarding.run(self.croot, self.image, self.config, self.auth, 1, 30, 5)
+        self.assertEqual(result["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.container_absent()
+        self.container_absent(second)
+
+    def test_timeout_removes_container_and_keeps_incomplete_evidence(self):
+        receipt = self.run_mode("timeout", seconds=2)
+        self.assertIsNone(receipt["result_hash"])
+        self.assertIsNone(receipt["tokens"])
+
+    def test_interrupt_removes_container_and_preserves_started_call(self):
+        self.credentials("interrupted")
+        command = [sys.executable, str(SKILL / "scripts/review.py"), "onboard", "run",
+                   "--campaign", str(self.croot), "--image", self.image,
+                   "--host-config", str(self.config), "--auth", str(self.auth),
+                   "--max-calls", "1", "--max-wall-seconds", "30", "--trial-seconds", "20"]
+        with subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE) as process:
+            try:
+                deadline = time.monotonic() + 15
+                trace = self.trial / "trace.jsonl"
+                while time.monotonic() < deadline:
+                    if trace.exists() and "runtime.fake.started" in trace.read_text():
+                        break
+                    if process.poll() is not None:
+                        self.fail("Runner exited before fake reviewer started")
+                    time.sleep(0.05)
+                else:
+                    self.fail("Fake reviewer never started")
+                process.send_signal(signal.SIGINT)
+                stdout, stderr = process.communicate(timeout=20)
+                self.assertNotEqual(process.returncode, 0, stdout + stderr)
+                self.container_absent()
+            finally:
+                if process.poll() is None:
+                    process.kill()
+                    process.communicate(timeout=10)
+        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.container_absent()
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--image", help="Existing fake-reviewer image, pinned by digest or local sha256 ID")
+    args = parser.parse_args()
+    if os.name != "posix":
+        parser.error("Docker runtime integration requires a POSIX host")
+    with tempfile.TemporaryDirectory(prefix="crc-runtime-build-") as temp:
+        if args.image:
+            RuntimeTests.image = args.image
+        else:
+            iid = Path(temp) / "image-id"
+            subprocess.run(["docker", "build", "--iidfile", str(iid), str(ROOT / "tests/runtime")],
+                           check=True, timeout=300)
+            RuntimeTests.image = iid.read_text().strip()
+        suite = unittest.defaultTestLoader.loadTestsFromTestCase(RuntimeTests)
+        return not unittest.TextTestRunner(verbosity=2).run(suite).wasSuccessful()
+
+
+if __name__ == "__main__":
+    sys.exit(main())

tests/runtime/Dockerfile

@@ -0,0 +1,2 @@
+FROM python:3.11-slim@sha256:b7a9e5f6a93d6d4a31f129bbd4bdee69efccb6ded0fb230d78aafaf37af98377
+COPY --chmod=0755 fake_codex.py /usr/local/bin/codex

tests/runtime/fake_codex.py

@@ -0,0 +1,39 @@
+#!/usr/bin/env python3
+"""Docker-only protocol fixture. No inference, network calls, or real credentials."""
+import json
+import re
+import sys
+import time
+from pathlib import Path
+
+if sys.argv[1:] == ["--version"]:
+    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:
+    pass
+else:
+    raise AssertionError("Pinned source mount must be read-only")
+
+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)
+mode = auth["mode"]
+if mode in {"timeout", "interrupted"}:
+    time.sleep(120)
+if 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)