From 030c6372d1d5a7896e4871b4194ce897416c0e0c Mon Sep 17 00:00:00 2001 From: dr mike Date: Thu, 3 Sep 2026 09:28:15 +0330 Subject: [PATCH] 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 --- graphify/install.py | 26 ++++++++++++- tests/test_atomic_version_stamp.py | 62 ++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 tests/test_atomic_version_stamp.py diff --git a/graphify/install.py b/graphify/install.py index 048589476..7a2dcf8d8 100644 --- a/graphify/install.py +++ b/graphify/install.py @@ -32,6 +32,28 @@ 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" diff --git a/tests/test_atomic_version_stamp.py b/tests/test_atomic_version_stamp.py new file mode 100644 index 000000000..08c000228 --- /dev/null +++ b/tests/test_atomic_version_stamp.py @@ -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"