fix(install): write .graphify_version atomically via os.replace

Match the SKILL.md / references/ crash-safety pattern so an interrupted install cannot leave a truncated stamp, and replace managed symlinks instead of writing through them.

Fixes #3286

(cherry picked from commit 030c6372d1)
This commit is contained in:
dr mike authored and safishamsi committed 2026-09-05 01:18:59 +01:00
1 parent 6377f1609d
commit c488c0e07d
2 files changed
+86 -2

No files matched your search

+24 -2
View File
@@ -32,6 +32,28 @@ except Exception:
from graphify.paths import GRAPHIFY_OUT as _GRAPHIFY_OUT
def _write_version_stamp(skill_dst: Path, version: str) -> None:
"""Atomically write ``.graphify_version`` beside ``skill_dst``.
Matches the SKILL.md install path: temp file in the same directory, then
``os.replace``. Unlike ``Path.write_text`` (and unlike
``paths.write_text_atomic``, which resolves through symlinks), ``os.replace``
replaces a managed symlink in place — consistent with SKILL.md /
``references/`` and crash-safe against a half-written stamp (#3286).
"""
version_file = skill_dst.parent / ".graphify_version"
tmp = version_file.with_name(".graphify_version.tmp")
try:
tmp.write_text(version, encoding="utf-8")
os.replace(tmp, version_file)
except Exception:
try:
tmp.unlink(missing_ok=True)
except OSError:
pass
raise
@functools.lru_cache(maxsize=None)
def _always_on(basename: str) -> str:
"""Read a packaged always-on instruction block from graphify/always_on/.
@@ -236,7 +258,7 @@ def _copy_skill_file(platform_name: str, *, project: bool = False, project_dir:
pass
raise
(skill_dst.parent / ".graphify_version").write_text(__version__, encoding="utf-8")
_write_version_stamp(skill_dst, __version__)
print(f" skill installed -> {skill_dst}")
return skill_dst
def _remove_skill_file(platform_name: str, *, project: bool = False, project_dir: Path | None = None) -> bool:
@@ -881,7 +903,7 @@ def vscode_install(project_dir: Path | None = None) -> None:
orphan_refs = skill_dst.parent / "references"
if orphan_refs.exists():
shutil.rmtree(orphan_refs)
(skill_dst.parent / ".graphify_version").write_text(__version__, encoding="utf-8")
_write_version_stamp(skill_dst, __version__)
print(f" skill installed -> {skill_dst}")
instructions = (project_dir or Path(".")) / ".github" / "copilot-instructions.md"
+62
View File
@@ -0,0 +1,62 @@
"""Regression tests for atomic `.graphify_version` stamp writes (#3286)."""
from __future__ import annotations
import os
import pytest
import graphify.install as install
def test_write_version_stamp_is_atomic_and_cleans_tmp(tmp_path):
skill_dst = tmp_path / "skills" / "graphify" / "SKILL.md"
skill_dst.parent.mkdir(parents=True)
skill_dst.write_text("skill", encoding="utf-8")
install._write_version_stamp(skill_dst, "1.2.3")
stamp = skill_dst.parent / ".graphify_version"
assert stamp.read_text(encoding="utf-8") == "1.2.3"
assert {p.name for p in skill_dst.parent.iterdir()} == {"SKILL.md", ".graphify_version"}
def test_write_version_stamp_preserves_existing_on_replace_failure(tmp_path, monkeypatch):
skill_dst = tmp_path / "skills" / "graphify" / "SKILL.md"
skill_dst.parent.mkdir(parents=True)
skill_dst.write_text("skill", encoding="utf-8")
stamp = skill_dst.parent / ".graphify_version"
stamp.write_text("old", encoding="utf-8")
def boom(src, dst):
raise OSError("simulated failure")
monkeypatch.setattr(os, "replace", boom)
with pytest.raises(OSError):
install._write_version_stamp(skill_dst, "new")
assert stamp.read_text(encoding="utf-8") == "old"
assert not (skill_dst.parent / ".graphify_version.tmp").exists()
def test_write_version_stamp_replaces_symlink_instead_of_following(tmp_path):
"""Managed-dotfile case: os.replace must replace the symlink, not write through."""
real_store = tmp_path / "dotfiles" / ".graphify_version"
real_store.parent.mkdir(parents=True)
real_store.write_text("from-dotfiles", encoding="utf-8")
skill_dir = tmp_path / "skills" / "graphify"
skill_dir.mkdir(parents=True)
skill_dst = skill_dir / "SKILL.md"
skill_dst.write_text("skill", encoding="utf-8")
link = skill_dir / ".graphify_version"
try:
link.symlink_to(real_store)
except OSError as exc:
pytest.skip(f"symlink creation not permitted: {exc}")
install._write_version_stamp(skill_dst, "installed")
assert link.is_symlink() is False
assert link.read_text(encoding="utf-8") == "installed"
assert real_store.read_text(encoding="utf-8") == "from-dotfiles"