fix(canary): keep a passing smoke's verdict when a Windows fixture holds read-only git objects

.github/workflows/python-tests.yml

@@ -819,6 +819,16 @@ jobs:
       - name: Check Windows collaboration lock sidecars
         run: python -m pytest -q --basetemp .t tests/test_collaboration_sidecar_scan.py
 
+      - name: Qualify the read-only fixture teardown on native Windows
+        # Windows refuses to unlink a read-only file and git marks every loose
+        # object read-only, so these suites fail on the pre-fix teardown with
+        # PermissionError [WinError 5] and cannot discriminate on POSIX.
+        run: >-
+          python -m pytest -q
+          tests/canary/test_smoke_fixture_cleanup.py
+          tests/extensions/test_process_runtime.py
+          tests/capabilities/test_issue_fix_temporary_workspace.py
+
       - name: Qualify native Windows peer reads and long-path inputs
         run: >-
           python -m pytest -q

examples/issue-fix-reviewer-recommendation-smoke.py

@@ -7,7 +7,6 @@
 import json
 import os
 import re
-import shutil
 import subprocess
 import sys
 import tempfile
@@ -26,6 +25,7 @@
     ISSUE_FIX_REVIEWER_SOURCES_INPUT_SCHEMA_VERSION,
     build_issue_fix_reviewer_recommendation_packet,
 )
+from loopx.extensions.process_runtime import remove_owned_tree  # noqa: E402
 
 
 PRIVATE_PATTERNS = [
@@ -44,7 +44,7 @@ def temporary_git_repo() -> Iterator[Path]:
     finally:
         for attempt in range(10):
             try:
-                shutil.rmtree(path)
+                remove_owned_tree(path)
                 break
             except FileNotFoundError:
                 break

examples/issue-fix-reviewer-request-smoke.py

@@ -9,7 +9,6 @@
 import json
 import os
 import re
-import shutil
 import subprocess
 import sys
 import tempfile
@@ -40,6 +39,7 @@
 from loopx.capabilities.issue_fix.reviewer_notification_drain import (  # noqa: E402
     drain_issue_fix_reviewer_notification_queue,
 )
+from loopx.extensions.process_runtime import remove_owned_tree  # noqa: E402
 from loopx.extensions.lark.reviewer_notification import (  # noqa: E402
     lark_reviewer_notification_sink,
 )
@@ -2095,7 +2095,7 @@ def drain_metadata_loader(
     finally:
         for attempt in range(10):
             try:
-                shutil.rmtree(path)
+                remove_owned_tree(path)
                 break
             except FileNotFoundError:
                 break

loopx/canary/runner.py

@@ -3,7 +3,6 @@
 import os
 import re
 import shlex
-import shutil
 import subprocess
 import sys
 import tempfile
@@ -12,7 +11,7 @@
 from pathlib import Path
 from typing import Any, Callable
 
-from ..extensions.process_runtime import terminate_process_tree
+from ..extensions.process_runtime import remove_owned_tree, terminate_process_tree
 from .planner import (
     REPO_ROOT,
     build_catalog_canary_plan,
@@ -288,6 +287,17 @@ def _retire_fixture_runtimes(root: Path, *, timeout: float = 15) -> None:
                 raise RuntimeError(f"managed runtime still exiting: {path.name}")
 
 
+def _remove_fixture_root(root: Path) -> None:
+    """Stop every managed runtime in an owned smoke root, then remove the root.
+
+    The root belongs to one check and holds host fixtures, git object stores
+    included, so removal tolerates the read-only attributes a host leaves behind.
+    A failure still raises, and the caller retains the tree for diagnosis.
+    """
+    _retire_fixture_runtimes(root)
+    remove_owned_tree(root)
+
+
 def _run_check(
     check: dict[str, Any],
     *,
@@ -347,8 +357,7 @@ def _run_check(
                 )
         finally:
             try:
-                _retire_fixture_runtimes(root)
-                shutil.rmtree(root)
+                _remove_fixture_root(root)
             except (OSError, RuntimeError, ValueError) as error:
                 # Retain the owned namespace if shutdown cannot be proved.
                 # Do not mask the check with unconditional second deletion.

loopx/capabilities/issue_fix/acceptance_loop.py

@@ -16,6 +16,7 @@
     run_caller_validation as _run_caller_validation,
     require_validation_passed as _require_passed,
 )
+from ...extensions.process_runtime import remove_owned_tree
 
 
 ISSUE_FIX_ACCEPTANCE_LOOP_SCHEMA_VERSION = "issue_fix_acceptance_loop_v0"
@@ -256,9 +257,16 @@ def add_lines(lines: list[str]) -> None:
 
 
 def _remove_temporary_git_workspace(workspace: Path) -> None:
+    """Drop this packet's own temporary repository, read-only objects included.
+
+    Git marks every loose object read-only, which Windows refuses to unlink, so
+    the retry below only ever helped a transient lock; the host attribute is
+    cleared by the shared owner before each attempt.
+    """
+
     for attempt in range(5):
         try:
-            shutil.rmtree(workspace)
+            remove_owned_tree(workspace)
             return
         except OSError:
             if attempt == 4:

loopx/extensions/process_runtime.py

@@ -3,7 +3,9 @@
 from collections.abc import Callable, Mapping, Sequence
 from dataclasses import dataclass
 import os
+import shutil
 import signal
+import stat
 import subprocess
 import sys
 import threading
@@ -239,6 +241,98 @@ def cleanup() -> None:
             closed = True
     return cleanup
 
+_WINDOWS_REPARSE_POINT = getattr(stat, "FILE_ATTRIBUTE_REPARSE_POINT", 0x400)
+
+
+def _is_link_or_junction(target: str) -> bool:
+    """Whether ``target`` is a link this teardown must not traverse.
+
+    ``os.path.islink`` covers POSIX symlinks and Windows file or directory
+    symlinks. A Windows junction is also a reparse point but is not reported as
+    a symlink, and ``os.walk`` still descends into it, so the reparse attribute
+    is checked explicitly; ``os.path.isjunction`` does not exist before 3.12.
+    """
+    if os.path.islink(target):
+        return True
+    if os.name != "nt":
+        return False
+    try:
+        status = os.lstat(target)
+    except OSError:
+        return False
+    # Only a directory reparse point (junction or mount point) can redirect the
+    # walk; file reparse points such as cloud placeholders are not links, so
+    # their read-only attribute is still cleared normally. ``st_file_attributes``
+    # exists only on Windows, so read it dynamically instead of through the
+    # cross-platform ``stat_result`` type.
+    attributes = getattr(status, "st_file_attributes", 0)
+    return stat.S_ISDIR(status.st_mode) and bool(
+        attributes & _WINDOWS_REPARSE_POINT
+    )
+
+
+def _clear_windows_read_only_files(root: str) -> None:
+    """Clear the read-only attribute of every file git wrote under ``root``.
+
+    Windows refuses to unlink a read-only file even though its directory is
+    writable, and git marks each loose object and pack read-only, so a tree that
+    holds a repository cannot be removed. Only Windows has that rule: POSIX
+    unlinks a read-only member of a writable directory. Links and junctions are
+    skipped so this walk cannot reach a target outside the owned tree, and a
+    file with more than one link is left untouched because a hard link shares
+    the file (and its read-only attribute) with another directory entry that may
+    belong to another owner; ``rmtree`` then fails closed.
+    """
+    for directory, subdirectories, files in os.walk(root):
+        subdirectories[:] = [
+            name
+            for name in subdirectories
+            if not _is_link_or_junction(os.path.join(directory, name))
+        ]
+        for name in files:
+            target = os.path.join(directory, name)
+            if _is_link_or_junction(target):
+                continue
+            try:
+                status = os.stat(target)
+                if status.st_nlink > 1:
+                    # A hard link is another directory entry for the same file,
+                    # and git uses them for a local clone. Clearing the
+                    # read-only attribute here would also change the source
+                    # repository object outside the owned tree, so leave it for
+                    # rmtree to fail closed instead.
+                    continue
+                if not status.st_mode & stat.S_IWRITE:
+                    os.chmod(target, status.st_mode | stat.S_IWRITE)
+            except OSError:
+                continue
+
+
+def remove_owned_tree(path: str | Path) -> None:
+    """Delete an owned, previously isolated tree while tolerating host attributes.
+
+    The filesystem sibling of :func:`terminate_process_tree`: a smoke fixture
+    root or frozen workspace is created by one owner, filled with host fixtures,
+    and removed by the same owner. A fixture that contains a git repository
+    cannot be removed by a plain ``shutil.rmtree`` on Windows, where every loose
+    object git wrote carries the read-only attribute, so teardown fails with
+    ``PermissionError`` [WinError 5] and the whole tree is left behind. The
+    attribute is cleared first on Windows only; every other host already removes
+    a read-only member of a writable directory unchanged. A root that is itself
+    a link or junction is refused before any attribute is cleared: ``os.walk``
+    would start at the link target, and ``shutil.rmtree`` only refuses the link
+    after that walk has already changed permissions outside the owned tree.
+    Ownership still stays with the caller: a removal that cannot complete raises,
+    so a caller that must retain the tree for diagnosis can keep observing it.
+    """
+    root = os.fspath(path)
+    if _is_link_or_junction(root):
+        raise OSError(f"refusing to remove linked tree root: {root}")
+    if os.name == "nt":
+        _clear_windows_read_only_files(root)
+    shutil.rmtree(root)
+
+
 def run_capped_process(
     argv: Sequence[str],
     *,