diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d75a84..7f349f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ here. Native format compatibility is documented separately in ## Unreleased +- Fix atomic writes on Windows in `write_private_atomic`: skip the Unix-only + `os.fchmod` call and the directory `fsync` where the platform cannot + perform them (Windows keeps the creating user's ACLs and NTFS journaling + covers directory durability), and make temporary-file cleanup best-effort + so a cleanup failure cannot mask the original error. + ## 0.11.0 - 2026-09-11 - Read root Codex rollouts written with `history_mode: "paginated"` by using diff --git a/src/session_migrate/jsonl.py b/src/session_migrate/jsonl.py index c86c932..164782b 100644 --- a/src/session_migrate/jsonl.py +++ b/src/session_migrate/jsonl.py @@ -2,6 +2,7 @@ from __future__ import annotations +import contextlib import hashlib import json import os @@ -160,7 +161,9 @@ def write_private_atomic(path: Path, data: bytes) -> tuple[int, int]: temporary_path = Path(temporary_name) published_identity: tuple[int, int] | None = None try: - os.fchmod(descriptor, 0o600) + # os.fchmod is Unix-only; Windows keeps the creating user's ACLs. + if hasattr(os, "fchmod"): + os.fchmod(descriptor, 0o600) with os.fdopen(descriptor, "wb") as stream: stream.write(data) stream.flush() @@ -178,7 +181,9 @@ def write_private_atomic(path: Path, data: bytes) -> tuple[int, int]: _fsync_directory(path.parent) return published_identity except BaseException: - temporary_path.unlink(missing_ok=True) + # A cleanup failure must not replace the error being reported. + with contextlib.suppress(OSError): + temporary_path.unlink(missing_ok=True) if published_identity is not None: _unlink_if_same_file(path, published_identity) raise @@ -205,6 +210,10 @@ def _mkdir_private(path: Path) -> None: def _fsync_directory(path: Path) -> None: + # Windows cannot os.open() a directory, and NTFS journaling keeps + # directory entries durable without an explicit fsync. + if os.name == "nt": + return descriptor = os.open(path, os.O_RDONLY | getattr(os, "O_DIRECTORY", 0)) try: os.fsync(descriptor) diff --git a/tests/test_jsonl.py b/tests/test_jsonl.py index 2141881..3f1ee89 100644 --- a/tests/test_jsonl.py +++ b/tests/test_jsonl.py @@ -99,3 +99,34 @@ def racing_link(source: object, target: object) -> None: write_private_atomic(path, b"migrator output\n") assert path.read_bytes() == b"racing winner" + + +def test_atomic_write_when_fchmod_is_unavailable( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + # Windows and WASI have no os.fchmod; writes must still succeed there. + monkeypatch.delattr(os, "fchmod", raising=False) + path = tmp_path / "session.jsonl" + + write_private_atomic(path, b"{}\n") + + assert path.read_bytes() == b"{}\n" + assert not list(tmp_path.glob(".*.tmp")) + + +def test_atomic_write_cleanup_does_not_mask_original_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + path = tmp_path / "session.jsonl" + + def failing_fchmod(descriptor: int, mode: int) -> None: + raise OSError("fchmod failed") + + def failing_unlink(target: object, **kwargs: object) -> None: + raise OSError("cleanup unlink failed") + + monkeypatch.setattr(os, "fchmod", failing_fchmod) + monkeypatch.setattr(os, "unlink", failing_unlink) + + with pytest.raises(JsonlError, match="fchmod failed"): + write_private_atomic(path, b"{}\n")