[burtenshaw]

This PR adds optional image cleanup after validation and replay, preserving shared images and build caches.

[bot-ci-comment]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Owned build inspect can miss deadline
  • The owned-image verification now uses its own 10-second timeout after a successful build instead of the exhausted build deadline.

Or push these changes by commenting:

@cursor push a59aa16f30

Preview (a59aa16f30)

diff --git a/src/openenv/validation/providers/docker.py b/src/openenv/validation/providers/docker.py
--- a/src/openenv/validation/providers/docker.py
+++ b/src/openenv/validation/providers/docker.py
@@ -298,7 +298,7 @@
                         "{{.Id}}",
                         _image_tag(image_owner),
                     ],
-                    min(10, deadline - time.monotonic()),
+                    10,
                 )
                 if code:
                     raise ProviderError("Docker did not publish the owned image")

diff --git a/tests/test_validation/test_docker_provider.py b/tests/test_validation/test_docker_provider.py
--- a/tests/test_validation/test_docker_provider.py
+++ b/tests/test_validation/test_docker_provider.py
@@ -358,7 +358,7 @@
 
     def build(argv, timeout_s):
         if argv[1:3] == ["image", "inspect"]:
-            assert 0 < timeout_s <= 3
+            assert timeout_s == 10
             assert argv[-1] == f"openenv-validation:{owner}"
             return 0, IMAGE, ""
         assert argv[argv.index("--tag") + 1] == f"openenv-validation:{owner}"

_{You can send follow-ups to the cloud agent here.}

[cursor]

Alignment Review Report

Automated Checks

• Lint: PASS — uv run usort check, uv run ruff format --check, and uv run ruff check are all clean on every file this PR touches (src/openenv/cli/commands/validate.py, src/openenv/validation/providers/docker.py, src/openenv/validation/runner.py, and the touched test files). The repo-wide ruff format --check does flag ~55 files, but none are in this diff (pre-existing, unrelated formatting drift).
• Debug code: CLEAN — the repo-wide debug scan surfaces print(...)/TODO hits, but none fall on lines this PR added; the two print(...) matches inside touched test files are pre-existing lines untouched by this diff.
• Tests: PYTHONPATH=src:envs uv run pytest tests/test_validation/ tests/test_cli/test_validate.py -q → 713 passed, 37 skipped (skips are Docker-integration tests; no Docker daemon in this sandbox). The new validation_runtime/acceptance.json and README.md claims were cross-checked against the file directly: the "docker" suite is exactly 29 entries (5 lifecycle + 23 CLI + 1 echo canary), matching the updated README prose.

Open RFCs Context

• RFC 008 — Environment Auto-Validation (In Review, @zkwentz) is the directly relevant RFC: it defines the validation LaunchSpec/provider contract and requires "idempotent cleanup on success, failure, timeout and cancellation" — but that requirement is scoped to the run-owned subject (container), not the build image. This PR adds image cleanup as a new, explicitly opt-in (--cleanup-images) capability layered on top of the existing container-cleanup contract, so it doesn't change or contradict anything the RFC currently specifies — it's additive. No other Draft/In-Review RFC (000, 001, 002, 003, 005, 006, 010, 011, 012) touches the validation/Docker-provider surface this PR modifies.

Tier 1: Fixes Required

None — lint, debug-code, and test checks are all clean for the changed files.

Tier 2: Alignment Discussion

Principle Conflicts

None identified. This is confined to the local-validation CLI/provider internals (src/openenv/validation/*, src/openenv/cli/commands/validate.py); it doesn't touch the Gym-like/MCP API boundary, client-server separation, or reward computation, so none of the INVARIANTS.md/PRINCIPLES.md rules are implicated.

RFC Conflicts

None identified (see RFC 008 context above — additive, not conflicting).

Design/quality notes (non-blocking)

I verified the new ownership/safety logic carefully since it issues docker image rm:

• _image_tag strictly validates the owner against validation-[a-f0-9]{32} (always spec.run_id), and cleanup_image re-verifies both the image's immutable ID and its org.openenv.validation.image-owner label before removing anything, refusing cleanup on any mismatch (providers/docker.py:329-338). Good defense against removing a shared/retagged image.
• The race the code comment calls out — "cancellation can occur after Docker publishes a tag but before the client returns its immutable ID" (runner.py:176-177) — is handled correctly: image_owner is recorded before build() is called, so cleanup_image still runs (with expected_id=None) even if build() raises mid-way. Confirmed by test_image_cleanup_runs_after_containers_even_when_execution_fails[build]/[build_interrupt].
• cleanup_image's image_removed determination (via a post-rm docker image inspect on the immutable ID) correctly reflects whether a surviving external tag (the "shared" test case) keeps the underlying image alive — double-checked against both the real Docker integration test and the mocked unit test.
• One very minor UX gap (not a bug): passing --cleanup-images --skip-build together isn't explicitly rejected at the CLI layer — skip_build short-circuits before cleanup_images is ever consulted, so the flag is silently a no-op in that combination rather than erroring like the --url case does. Not worth blocking on, just flagging in case the author wants an explicit CLI-level guard for symmetry with the existing --cleanup-images + --url check.

Summary

• 0 mechanical issues to fix
• 1 non-blocking design note for the author's discretion (no alignment flag needed)
• 0 RFC conflicts to discuss

  

_{Sent by Cursor Automation: Pre-review}

[burtenshaw]

cursor review

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Cleanup parses engine-specific inspect errors
  • Image cleanup now verifies absence through portable image listings instead of parsing engine-specific inspect errors.

Or push these changes by commenting:

@cursor push 0434b0298a

Preview (0434b0298a)

diff --git a/src/openenv/validation/providers/docker.py b/src/openenv/validation/providers/docker.py
--- a/src/openenv/validation/providers/docker.py
+++ b/src/openenv/validation/providers/docker.py
@@ -135,6 +135,22 @@
     )
 
 
+def _image_ids(*, reference: str | None = None, owner: str | None = None) -> set[str]:
+    """List matching image IDs without relying on engine-specific inspect errors."""
+    argv = ["docker", "image", "ls", "--all", "--no-trunc", "--quiet"]
+    if owner is not None:
+        argv.extend(["--filter", f"label={_IMAGE_LABEL}={owner}"])
+    if reference is not None:
+        argv.append(reference)
+    code, output, _ = _command(argv, 10)
+    if code:
+        raise ProviderError("Docker could not list images")
+    ids = set(output.split())
+    if any(not re.fullmatch(r"sha256:[a-f0-9]{64}", image_id) for image_id in ids):
+        raise ProviderError("Docker returned an invalid image listing")
+    return ids
+
+
 def _contained(root: Path, relative: str) -> Path:
     path = root / relative
     try:
@@ -311,7 +327,9 @@
         """Release only this build's tag, preserving other tags and parent images."""
         tag = _image_tag(owner)
         evidence = {"owner": owner, "tag": tag, "required": True, "completed": False}
-        code, output, stderr = _command(
+        if not _image_ids(reference=tag):
+            return {**evidence, "completed": True, "reason": "owned tag absent"}
+        code, output, _ = _command(
             [
                 "docker",
                 "image",
@@ -323,8 +341,6 @@
             10,
         )
         if code:
-            if "No such image" in stderr or "No such object" in stderr:
-                return {**evidence, "completed": True, "reason": "owned tag absent"}
             raise ProviderError("Could not verify image ownership for cleanup")
         fields = output.split()
         if (
@@ -341,21 +357,14 @@
         code, _, _ = _command(["docker", "image", "rm", "--no-prune", tag], 10)
         if code:
             raise ProviderError("Could not remove validation image tag")
-        code, _, stderr = _command(
-            ["docker", "image", "inspect", "--format", "{{.Id}}", tag], 10
-        )
-        if not code or not ("No such image" in stderr or "No such object" in stderr):
+        if _image_ids(reference=tag):
             raise ProviderError("Image tag removal could not be independently verified")
-        code, _, stderr = _command(
-            ["docker", "image", "inspect", "--format", "{{.Id}}", image_id], 10
-        )
-        if code and not ("No such image" in stderr or "No such object" in stderr):
-            raise ProviderError("Image removal could not be independently verified")
+        image_removed = image_id not in _image_ids(owner=owner)
         return {
             **evidence,
             "image_id": image_id,
             "completed": True,
-            "image_removed": bool(code),
+            "image_removed": image_removed,
         }
 
     def start(self, spec: LaunchSpec) -> "DockerRunningSubject":

diff --git a/tests/test_validation/test_docker_provider.py b/tests/test_validation/test_docker_provider.py
--- a/tests/test_validation/test_docker_provider.py
+++ b/tests/test_validation/test_docker_provider.py
@@ -478,10 +478,18 @@
             assert argv == ["docker", "image", "rm", "--no-prune", tag]
             removed = True
             return 0, "", ""
-        if argv[-1] == IMAGE and shared:
-            return 0, IMAGE, ""
-        if removed:
-            return 1, "", "No such image"
+        if argv[2] == "ls":
+            assert argv[3:6] == ["--all", "--no-trunc", "--quiet"]
+            if "--filter" in argv:
+                assert argv[-2:] == [
+                    "--filter",
+                    f"label={docker._IMAGE_LABEL}={owner}",
+                ]
+                exists = shared
+            else:
+                assert argv[-1] == tag
+                exists = not removed
+            return 0, IMAGE if exists else "", ""
         return 0, f"{IMAGE} {owner}", ""
 
     monkeypatch.setattr(docker, "_command", command)
@@ -500,13 +508,15 @@
 
     def command(argv, timeout_s):
         calls.append(argv)
+        if argv[2] == "ls":
+            return 0, IMAGE, ""
         return 0, f"{IMAGE} {'other' if wrong_field == 'owner' else owner}", ""
 
     monkeypatch.setattr(docker, "_command", command)
     expected = "sha256:" + "c" * 64 if wrong_field == "identity" else IMAGE
     with pytest.raises(ProviderError, match="another identity or owner"):
         docker.DockerValidationProvider().cleanup_image(owner, expected)
-    assert len(calls) == 1 and calls[0][2] == "inspect"
+    assert [argv[2] for argv in calls] == ["ls", "inspect"]
 
 
 def test_build_rejects_symlink_escape_without_invoking_docker(tmp_path, commands):

_{You can send follow-ups to the cloud agent here.}

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Cleanup fails on large image inventories
  • Scoped the post-cleanup image existence check to the known image ID so large unrelated image inventories cannot overflow the bounded output.

Or push these changes by commenting:

@cursor push 62d4b0c8df

Preview (62d4b0c8df)

diff --git a/src/openenv/validation/providers/docker.py b/src/openenv/validation/providers/docker.py
--- a/src/openenv/validation/providers/docker.py
+++ b/src/openenv/validation/providers/docker.py
@@ -364,7 +364,7 @@
             **evidence,
             "image_id": image_id,
             "completed": True,
-            "image_removed": image_id not in _image_ids(),
+            "image_removed": image_id not in _image_ids(image_id),
         }
 
     def start(self, spec: LaunchSpec) -> "DockerRunningSubject":

_{You can send follow-ups to the cloud agent here.}

^{Reviewed by Cursor Bugbot for commit 6b01629. Configure here.}