diff --git a/hermes_cli/macos_tcc_anchor.py b/hermes_cli/macos_tcc_anchor.py index cd35be1a74..99b0d76c16 100644 --- a/hermes_cli/macos_tcc_anchor.py +++ b/hermes_cli/macos_tcc_anchor.py @@ -29,8 +29,9 @@ stays on the stable venv path) and closes both holes: points at ``@executable_path/../lib`` — no rewrite. 3. A pre-install boot gate actually launches the staged copy and demands ``import encodings`` plus ``sys.prefix == ``. Failure rolls the - staging file back and leaves the live venv untouched, so a bad anchor - can never brick update/doctor again. + staging file back and leaves the live interpreter untouched (a surplus + provisioned dylib in ``venv/lib/`` may remain — harmless), so a bad + anchor can never brick update/doctor again. All functions are no-ops on non-macOS and for interpreters that are not uv-managed. Best-effort: never raises to callers. @@ -49,13 +50,16 @@ import tempfile from pathlib import Path from hermes_constants import venv_python_path +from hermes_cli.managed_uv import _RUNTIME_DIR_NAME logger = logging.getLogger(__name__) _MARKER_NAME = ".tcc-anchor-source" _STORE_COMMON_MARKERS = ("cpython-", "-macos-") -_STORE_ROOT_MARKERS = ("/uv/python/", "/.hermes-runtime/python/") +# The runtime-store marker is derived from managed_uv so a rename of the +# repair-generation directory cannot silently stop the anchor from matching. +_STORE_ROOT_MARKERS = ("/uv/python/", f"/{_RUNTIME_DIR_NAME}/python/") class _BootGateFailed(Exception): @@ -159,6 +163,26 @@ def _anchor_marker(venv_bin: Path) -> Path: return venv_bin / _MARKER_NAME +def _write_marker(venv_bin: Path, source_file: Path) -> None: + """Write the anchor marker atomically (write-then-rename). + + A concurrent ensure (update + doctor --fix) must never observe a + partially-written marker: a torn read would compare unequal and trigger + a spurious reinstall, and ``write_text`` alone is not atomic. + """ + fd, tmp_name = tempfile.mkstemp(prefix=f"{_MARKER_NAME}.", dir=str(venv_bin)) + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fh.write(_marker_value(source_file)) + os.replace(tmp_name, _anchor_marker(venv_bin)) + except OSError: + try: + os.unlink(tmp_name) + except OSError: + pass + raise + + def _store_root(source_file: Path) -> Path: # .../cpython--macos-*/bin/python3.N → store root return source_file.resolve(strict=False).parent.parent @@ -200,24 +224,41 @@ def _provision_libpython( logger.debug("libpython provision skipped", exc_info=True) -def _copy_alias(venv_bin: Path, name: str, anchor: Path) -> None: - """Materialize *name* as a real-file copy of *anchor* (atomic rename).""" - tmp = venv_bin / f".{name}.tcc-tmp" +def _copy_alias(venv_bin: Path, name: str, anchor: Path) -> bool: + """Materialize *name* as a real-file copy of *anchor* (atomic rename). + + Returns False (and warns) on failure: a leftover alias *symlink* to the + anchor is the exact #95541 crash shape, so callers must know when the + alias set is incomplete. The staging name is unique (mkstemp) so a + concurrent ensure (update + doctor --fix) cannot promote a truncated + interim copy. + """ + tmp_path: Path | None = None try: - shutil.copy2(anchor, tmp) - os.chmod(tmp, anchor.stat().st_mode | 0o111) - os.replace(tmp, venv_bin / name) - except OSError: - try: - tmp.unlink(missing_ok=True) - except OSError: - pass + fd, tmp_name = tempfile.mkstemp(prefix=f".{name}.tcc-", dir=str(venv_bin)) + os.close(fd) + tmp_path = Path(tmp_name) + shutil.copy2(anchor, tmp_path) + os.chmod(tmp_path, anchor.stat().st_mode | 0o111) + os.replace(tmp_path, venv_bin / name) + return True + except OSError as exc: + logger.warning("TCC anchor alias %s not materialized: %s", name, exc) + if tmp_path is not None: + try: + tmp_path.unlink(missing_ok=True) + except OSError: + pass + return False def _materialize_aliases( venv_bin: Path, anchor: Path, *, refresh: bool = False -) -> None: - """Materialize uv alias names as real-file copies of the anchor.""" +) -> bool: + """Materialize uv alias names as real-file copies of the anchor. + + Returns True only when every alias that needed materializing succeeded. + """ names = set(_sibling_names()) try: names.update( @@ -227,13 +268,16 @@ def _materialize_aliases( ) except OSError: pass + ok = True for name in sorted(names): alias = venv_bin / name try: if refresh or alias.is_symlink() or not alias.exists(): - _copy_alias(venv_bin, name, anchor) + ok = _copy_alias(venv_bin, name, anchor) and ok except OSError: + ok = False continue + return ok def _passes_boot_gate(staged: Path, venv_dir: Path) -> bool: @@ -305,8 +349,19 @@ def _install_anchor(venv_dir: Path, source_file: Path) -> None: f"staged copy at {tmp_path} failed encodings/prefix probe" ) os.replace(tmp_path, venv_py) - _anchor_marker(venv_bin).write_text(_marker_value(source_file), encoding="utf-8") - _materialize_aliases(venv_bin, venv_py, refresh=True) + aliases_ok = _materialize_aliases(venv_bin, venv_py, refresh=True) + if aliases_ok: + # Marker last, atomically: it asserts the WHOLE layout (anchor + + # aliases) is complete. A partially-materialized alias set (the + # #95541 crash shape when an alias stays a symlink) must not read + # "active" in doctor — leaving the marker absent makes the next + # ensure retry the install. + _write_marker(venv_bin, source_file) + else: + logger.warning( + "TCC anchor installed but alias materialization was " + "incomplete; leaving anchor unmarked so the next run retries" + ) except Exception: try: tmp_path.unlink(missing_ok=True) diff --git a/tests/hermes_cli/test_macos_tcc_anchor.py b/tests/hermes_cli/test_macos_tcc_anchor.py index 818b2e6d2d..f07eff9f4c 100644 --- a/tests/hermes_cli/test_macos_tcc_anchor.py +++ b/tests/hermes_cli/test_macos_tcc_anchor.py @@ -297,6 +297,75 @@ class TestEnsureTccAnchor: assert venv_py.is_symlink() assert not (venv_py.parent / ".tcc-anchor-source").exists() + def test_alias_failure_leaves_anchor_unmarked(self, tmp_path, monkeypatch, caplog): + # If an alias copy fails (e.g. ENOSPC), the marker must NOT be + # written: a symlink alias to the anchored copy is the #95541 crash + # shape, and a marker would make doctor report "active" over it. + # The next ensure retries the whole install. + _darwin(monkeypatch) + store_bin = _build_store(tmp_path) + root = _build_checkout(tmp_path, store_bin=store_bin) + venv_py = venv_python_path(root / ".venv") + monkeypatch.setattr(tcc, "_copy_alias", lambda *a, **k: False) + + import logging + + with caplog.at_level(logging.WARNING, logger=tcc.__name__): + tcc.ensure_tcc_anchor(root) + + assert not (venv_py.parent / ".tcc-anchor-source").exists() + status, _ = tcc.tcc_anchor_state(root) + assert status != "active" + assert any("alias" in r.message for r in caplog.records) + + # Recovery: with alias copies working again the retry completes. + monkeypatch.undo() + monkeypatch.setattr(tcc.platform, "system", lambda: "Darwin") + assert tcc.ensure_tcc_anchor(root) is not None + assert tcc.tcc_anchor_state(root)[0] == "active" + + def test_copy_alias_failure_warns_and_cleans_staging(self, tmp_path, monkeypatch, caplog): + # _copy_alias must log the failure (silent skips hid the #95541 + # shape) and never leave a staging file behind. + anchor = tmp_path / "anchor" + anchor.write_bytes(b"#!anchor") + anchor.chmod(0o755) + + def boom(src, dst, **kw): + raise OSError(28, "No space left on device") + + monkeypatch.setattr(tcc.shutil, "copy2", boom) + + import logging + + with caplog.at_level(logging.WARNING, logger=tcc.__name__): + assert tcc._copy_alias(tmp_path, "python3", anchor) is False + + assert any("python3" in r.message for r in caplog.records) + assert not list(tmp_path.glob(".python3.tcc-*")) + + def test_marker_written_atomically(self, tmp_path): + # Marker goes through write-then-rename; no torn intermediate name + # survives and the content matches the resolved source path. + source = tmp_path / "store" / "python3.11" + source.parent.mkdir(parents=True) + source.write_bytes(b"#!store") + venv_bin = tmp_path / "bin" + venv_bin.mkdir() + + tcc._write_marker(venv_bin, source) + + marker = venv_bin / ".tcc-anchor-source" + assert marker.read_text(encoding="utf-8") == tcc._marker_value(source) + assert not list(venv_bin.glob(".tcc-anchor-source.*")) + + def test_store_root_marker_tracks_managed_uv_constant(self): + # The repair-generation store marker must stay derived from + # managed_uv's directory constant, not drift as a hardcoded string. + from hermes_cli.managed_uv import _RUNTIME_DIR_NAME + + assert f"/{_RUNTIME_DIR_NAME}/python/" in tcc._STORE_ROOT_MARKERS + class TestBootGate: """Direct branch coverage for _passes_boot_gate.