diff --git a/src/mcqemu/tools/sandbox.py b/src/mcqemu/tools/sandbox.py index 5552fce..aa96850 100644 --- a/src/mcqemu/tools/sandbox.py +++ b/src/mcqemu/tools/sandbox.py @@ -7,6 +7,7 @@ import logging import os import shutil import signal +import sys from pathlib import Path from fastmcp import Context, FastMCP @@ -42,6 +43,24 @@ async def _await_exit(pid: int | None, seconds: float) -> bool: return not pid_alive(pid) +def rmtree_collecting(target: Path, errors: list[str]) -> None: + """shutil.rmtree that records failures instead of raising or hiding them. + + The callback keyword changed name in 3.12 (onerror -> onexc) and the two + are passed different third arguments, so normalize both. + """ + + def record(_func, path, exc): + if isinstance(exc, tuple): # 3.11's onerror passes sys.exc_info() + exc = exc[1] + errors.append(f"{path}: {exc}") + + if sys.version_info >= (3, 12): + shutil.rmtree(target, onexc=record) + else: + shutil.rmtree(target, onerror=record) + + def _tail_log(path: str | None, lines: int = 3) -> str: if not path: return "(no log)" @@ -227,7 +246,7 @@ async def sandbox_destroy(name: str, ctx: Context = None) -> dict: for target in targets: if not target.exists(): continue # nothing to remove is success, not a failure - shutil.rmtree(target, onexc=lambda _f, path, exc: errors.append(f"{path}: {exc}")) + rmtree_collecting(target, errors) overlay_gone = not Path(overlay).exists() log.info("sandbox_destroy: removed %s (overlay_deleted=%s)", name, overlay_gone) result = { diff --git a/tests/test_reliability.py b/tests/test_reliability.py index 67f874b..edfa74b 100644 --- a/tests/test_reliability.py +++ b/tests/test_reliability.py @@ -161,8 +161,9 @@ async def test_sandbox_destroy_reports_cleanup_failure_honestly(dirs, all_pids_d seeded_record(dirs, "sb", config={"disks": [], "sandbox_overlay": str(overlay)}), ) - def boom(target, onexc=None): - onexc(None, str(target), PermissionError("read-only filesystem")) + def boom(target, onexc=None, onerror=None): + # rmtree's callback keyword differs by Python version. + (onexc or onerror)(None, str(target), PermissionError("read-only filesystem")) monkeypatch.setattr("mcqemu.tools.sandbox.shutil.rmtree", boom) async with Client(mcp) as client: @@ -460,3 +461,21 @@ def test_attached_vm_identity_is_not_judged_by_name(dirs, monkeypatch): spawned = VMRecord(name="ours", source="spawned", qmp_socket="/tmp/q.sock", pid=1234) assert registry.is_process_alive(spawned) is False + + +def test_rmtree_helper_works_on_this_python(tmp_path): + """The rmtree callback keyword changed name in 3.12; support both.""" + from mcqemu.tools.sandbox import rmtree_collecting + + victim = tmp_path / "tree" + (victim / "sub").mkdir(parents=True) + (victim / "sub" / "f.txt").write_text("x") + errors: list[str] = [] + rmtree_collecting(victim, errors) + assert not victim.exists() + assert errors == [] + + # A path that cannot be removed must be recorded, not raised or swallowed. + missing = tmp_path / "not-there" + rmtree_collecting(missing, errors) + assert len(errors) == 1 and str(missing) in errors[0]