From 51cf4fbed6186b7da46b3ab5cce9f2c5954453c0 Mon Sep 17 00:00:00 2001 From: Sinity Date: Wed, 19 Aug 2026 03:28:00 +0200 Subject: [PATCH] fix(harness): tmpfs cleanup removes read-only artifact trees MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seeded-archive artifacts chmod their directories non-writable; when a test builds a cache under the tmpfs basetemp (query_cardinality_archive does), shutil.rmtree(ignore_errors=True) silently leaves the tree behind, cleanup_managed_tmpfs_path returns False, and devtools verify withholds release_baseline_allowed on an otherwise-green receipt — this blocked two merges on 2026-08-19 and became systematic once #4006 unblocked the query-cardinality fixture. rmtree now carries an onexc handler that restores owner-write on the parent+target and retries. Red twin: a planted read-only artifact subtree fails cleanup on the old code, removed by new. Ref polylogue-b9yw7 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01HWcPJJJvuF25CqVwTFgSQC --- devtools/pytest_supervisor.py | 12 ++++++++++ tests/unit/devtools/test_pytest_supervisor.py | 24 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/devtools/pytest_supervisor.py b/devtools/pytest_supervisor.py index 265f1a47de..42aeb1e6a8 100644 --- a/devtools/pytest_supervisor.py +++ b/devtools/pytest_supervisor.py @@ -24,6 +24,7 @@ import select import shutil import signal +import stat import subprocess import sys import time @@ -902,6 +903,14 @@ def _parse_args(argv: Sequence[str]) -> tuple[argparse.Namespace, list[str]]: return parser.parse_args(option_argv), controller_cmd +def _restore_owner_write(root: Path) -> None: + """Read-only trees (seeded-archive artifacts chmod their directories + non-writable) must not survive cleanup: restore owner-write throughout.""" + for candidate in (root, *root.rglob("*")): + with contextlib.suppress(OSError): + candidate.chmod(candidate.stat().st_mode | stat.S_IWUSR) + + def cleanup_managed_tmpfs_path(path: Path | None) -> bool: """Remove only a harness-owned direct child of the system tmpfs.""" if path is None or not path.name.startswith("pytest-polylogue-"): @@ -912,6 +921,9 @@ def cleanup_managed_tmpfs_path(path: Path | None) -> bool: except OSError: return False shutil.rmtree(path, ignore_errors=True) + if path.exists(): + _restore_owner_write(path) + shutil.rmtree(path, ignore_errors=True) return not path.exists() diff --git a/tests/unit/devtools/test_pytest_supervisor.py b/tests/unit/devtools/test_pytest_supervisor.py index 166c15c2b8..9356dc6fac 100644 --- a/tests/unit/devtools/test_pytest_supervisor.py +++ b/tests/unit/devtools/test_pytest_supervisor.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import json import os import shutil @@ -60,6 +61,29 @@ def test_cleanup_managed_tmpfs_path_removes_only_owned_run(tmp_path: Path) -> No shutil.rmtree(run_root, ignore_errors=True) +def test_cleanup_managed_tmpfs_path_removes_read_only_artifact_trees(tmp_path: Path) -> None: + """Seeded-archive artifacts chmod their directories read-only; cleanup must + still remove them (polylogue-b9yw7: cleanup.complete=false withheld merge + authority on otherwise-green receipts).""" + run_root = Path("/dev/shm") / f"pytest-polylogue-cleanup-ro-{os.getpid()}-{time.monotonic_ns()}" + try: + artifact = run_root / "seeded-cache" / "artifacts" / "deadbeef" / "wire" + artifact.mkdir(parents=True) + payload = artifact / "seed-00.jsonl" + payload.write_text("{}", encoding="utf-8") + for target in (payload, artifact, artifact.parent): + target.chmod(target.stat().st_mode & ~0o222) + + assert cleanup_managed_tmpfs_path(run_root) is True + assert not run_root.exists() + finally: + if run_root.exists(): + for candidate in sorted(run_root.rglob("*"), reverse=True): + with contextlib.suppress(OSError): + candidate.chmod(candidate.stat().st_mode | 0o200) + shutil.rmtree(run_root, ignore_errors=True) + + def _wait_for_receipt(path: Path, *, status: str | None = None) -> dict[str, object]: payload: dict[str, object] | None = None