diff --git a/CHANGELOG.md b/CHANGELOG.md index d15c40c..571536b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [8.1.0] - 2026-08-02 +### Added +- **`Transaction.remove()` — journaled, recoverable deletion.** The journal + could only express writes, so a multi-note operation that *moves* notes could + journal half of what it did. (The `_Entry` docstring claimed `None` meant + "delete on rollback"; the field was typed `str` and nothing implemented it. + The docstring is now true rather than deleted.) + ### Fixed +- **`omind migrate` no longer loses a day of journal entries to a crash** + ([#194](https://github.com/CryptoJones/omind/issues/194) follow-up). + `migrate_journals` appended a stray journal's bullets to the relocated note + and then unlinked the stray. Between those two steps the entries existed in + exactly one place, and a crash there lost them outright with no way back. The + whole migration is now one journaled transaction. +- **The git merge driver writes atomically.** It used `write_text`, which + truncates in place, so an interrupted merge left a **torn note** *and* an exit + code git reads as success — the note is then committed and syncs to every + peer. It is deliberately *not* a transaction: git hands the driver exactly one + file (`%A`), so there is no multi-note window to roll back, and saying so in + the code is more useful than wrapping it in machinery it doesn't need. - **Notes are written with LF on every platform.** `store._atomic_write` and `paths.atomic_write_text` opened in text mode with the default newline translation, so the same note was written as LF on POSIX and CRLF on Windows — diff --git a/src/omind/journal.py b/src/omind/journal.py index e2c7693..b6cfbe6 100644 --- a/src/omind/journal.py +++ b/src/omind/journal.py @@ -26,7 +26,7 @@ from datetime import date, datetime, timedelta from pathlib import Path -from omind import paths +from omind import paths, txn from omind.hooks import JOURNAL_TAGS, action_bullets, journal_dir from omind.store import NoteFields, OmiStore, _atomic_write, render_fields, today @@ -106,20 +106,36 @@ def migrate_journals(omi_dir: Path | str) -> list[str]: # Same-package use of the store's private lock/index helpers: the move plus # index regeneration must be one critical section against other writers. with store._write_lock(): - for stray in find_stray_journals(store.omi_dir): + strays = find_stray_journals(store.omi_dir) + if not strays: + return moved + # Journaled: this appends to one note and deletes another, per stray. A + # crash between the two used to lose a day of journal entries outright, + # with no way back (#194). Now an interrupted run is undone by + # `omind recover`. + transaction = txn.Transaction(store.omi_dir) + target_dir.mkdir(parents=True, exist_ok=True) + for stray in strays: target = target_dir / stray.name + stray_text = stray.read_text(encoding="utf-8", errors="replace") if target.is_file(): - bullets = action_bullets(stray.read_text(encoding="utf-8", errors="replace")) + bullets = action_bullets(stray_text) + existing = target.read_text(encoding="utf-8", errors="replace") if bullets: - with target.open("a", encoding="utf-8") as fh: - fh.write("\n".join(bullets) + "\n") - stray.unlink() + merged = existing.rstrip("\n") + "\n" + "\n".join(bullets) + "\n" + transaction.write(target, merged) else: - target_dir.mkdir(parents=True, exist_ok=True) - stray.rename(target) + transaction.write(target, stray_text) + transaction.remove(stray) moved.append(stray.name) - if moved: - store._write_index() + transaction.prepare() + try: + transaction.apply(_atomic_write) + except BaseException: + transaction.rollback() + raise + transaction.commit() + store._write_index() return moved diff --git a/src/omind/merge.py b/src/omind/merge.py index c2c4f5b..add9ae5 100644 --- a/src/omind/merge.py +++ b/src/omind/merge.py @@ -40,6 +40,7 @@ TEMPLATE_SECTIONS, ActionItem, NoteFields, + _atomic_write, parse_note, render_fields, split_sections, @@ -384,7 +385,11 @@ def run_merge_driver( ours_md = ours_path.read_text(encoding="utf-8") theirs_md = theirs_path.read_text(encoding="utf-8") merged, clean, messages = merge_note_texts(base_md, ours_md, theirs_md) - ours_path.write_text(merged, encoding="utf-8") + # Atomic, not a journaled transaction: git hands the driver exactly one + # file (%A), so there is no multi-note window to roll back. The gap that + # DID exist here was `write_text` truncating in place — a crash mid-merge + # left a torn note *and* git believing the merge succeeded (#194). + _atomic_write(ours_path, merged) except (OSError, UnicodeError) as exc: print(f"omind merge-driver: {label}: {exc}", file=sys.stderr) return 1 diff --git a/src/omind/txn.py b/src/omind/txn.py index 77b5093..440a9a2 100644 --- a/src/omind/txn.py +++ b/src/omind/txn.py @@ -126,9 +126,10 @@ class _Entry: """One file this transaction will replace, and how to undo that.""" path: Path - #: Bytes to write. ``None`` means "this file should not exist afterwards", - #: which recovery reads as "delete it on rollback if we created it". - content: str + #: Text to write, or ``None`` to remove the file. A removal is journaled + #: exactly like a write — the pre-image is captured first — so rolling one + #: back puts the file back. + content: str | None #: sha256 of the file's bytes before we touched it; "" when it did not exist. prior_sha: str = "" existed: bool = False @@ -142,6 +143,7 @@ def to_json(self) -> dict[str, Any]: "prior_sha": self.prior_sha, "existed": self.existed, "new_sha": self.new_sha, + "removed": self.content is None, } @@ -199,6 +201,18 @@ def write(self, path: Path, content: str) -> None: raise TransactionError("cannot add writes after prepare()") self._entries.append(_Entry(path=Path(path), content=content)) + def remove(self, path: Path) -> None: + """Queue ``path`` for deletion, recoverably. + + The pre-image is captured like any other entry, so an interrupted run + that deleted a file gets it back. Without this, a multi-note operation + that *moves* notes — `omind migrate` relocating stray journals — could + only journal half of what it does. + """ + if self._prepared: + raise TransactionError("cannot add writes after prepare()") + self._entries.append(_Entry(path=Path(path), content=None)) + # -- storage ------------------------------------------------------------ def _dir(self) -> Path: @@ -239,7 +253,7 @@ def prepare(self) -> None: directory = self._dir() directory.mkdir(parents=True, exist_ok=True) for index, entry in enumerate(self._entries): - entry.new_sha = _sha(entry.content) + entry.new_sha = _sha(entry.content) if entry.content is not None else "" try: prior = entry.path.read_bytes() except FileNotFoundError: @@ -262,7 +276,11 @@ def apply(self, writer: Any) -> None: if not self._prepared: raise TransactionError("apply() before prepare()") for entry in self._entries: - writer(entry.path, entry.content) + if entry.content is None: + with contextlib.suppress(FileNotFoundError): + entry.path.unlink() + else: + writer(entry.path, entry.content) def commit(self) -> None: """Mark the transaction complete and drop the journal. @@ -338,6 +356,16 @@ def _rollback_journal(directory: Path) -> RecoveryReport: report.conflicts.append(path.name) continue + removal = not str(raw.get("new_sha") or "") and bool(raw.get("removed")) + if removal and existed and current_sha == "": + # We deleted it; put it back from the pre-image. + try: + _atomic_write_bytes(path, (directory / f"{index:04d}.pre").read_bytes()) + except OSError: + report.conflicts.append(path.name) + continue + report.restored.append(path.name) + continue if existed and current_sha == prior_sha: report.skipped.append(path.name) # never written, or already undone continue diff --git a/tests/test_journal.py b/tests/test_journal.py index 78eafdc..46f7418 100644 --- a/tests/test_journal.py +++ b/tests/test_journal.py @@ -213,3 +213,53 @@ def test_rollup_late_daily_keeps_archived_days(tmp_path: Path) -> None: journal.rollup_journals(tmp_path, week="2026-W23") text = (journal_dir / journal.rollup_name("2026-W23")).read_text(encoding="utf-8") assert "2026-06-01" in text and "2026-06-02" in text and "2026-06-03" in text + + +def test_interrupted_migration_loses_no_journal_entries(tmp_path: Path) -> None: + """A crash mid-migration used to lose a day of entries outright (#194). + + `migrate_journals` appends a stray's bullets to the relocated journal and + then unlinks the stray. Between those two steps the entries existed in one + place only; a crash after the unlink and before the append lost them with no + way back. It is journaled now, so an interrupted run is recoverable. + """ + from omind import txn + + _write_daily(tmp_path, "2026-06-01", _BULLETS[:1]) + stray = tmp_path / "Session Journal 2026-06-01.md" + original = stray.read_bytes() + + real_apply = txn.Transaction.apply + + def dying_apply(self: txn.Transaction, writer: object) -> None: + def half(path: Path, content: str) -> None: + raise KeyboardInterrupt("power loss") + + real_apply(self, half) + + import pytest as _pytest + + with _pytest.MonkeyPatch.context() as mp: + mp.setattr(txn.Transaction, "apply", dying_apply) + with _pytest.raises(KeyboardInterrupt): + journal.migrate_journals(tmp_path) + + # In-process rollback already put it back; nothing left for `omind recover`. + assert stray.read_bytes() == original + assert txn.pending(tmp_path) == [] + + +def test_migration_merges_a_same_day_journal_without_losing_either_trail( + tmp_path: Path, +) -> None: + """One session wrote before the layout change, another after.""" + _write_daily(tmp_path, "2026-06-01", _BULLETS[:1]) + _write_daily(tmp_path / "Journal", "2026-06-01", _BULLETS[1:2]) + + assert journal.migrate_journals(tmp_path) == ["Session Journal 2026-06-01.md"] + merged = (tmp_path / "Journal" / "Session Journal 2026-06-01.md").read_text( + encoding="utf-8" + ) + for bullet in _BULLETS[:2]: + assert bullet in merged + assert not (tmp_path / "Session Journal 2026-06-01.md").exists() diff --git a/tests/test_merge.py b/tests/test_merge.py index c3c8d40..54fe834 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -13,6 +13,7 @@ from pathlib import Path +from omind import merge from omind.merge import ( CONFLICT_TAG, merge_fields, @@ -348,3 +349,24 @@ def test_run_merge_driver_exits_one_on_unreadable_input(tmp_path: Path) -> None: bad = tmp_path / "bad.md" bad.write_bytes(b"\xff\xfe invalid utf-8 \xff") assert run_merge_driver(b, bad, t) == 1 + + +def test_merge_driver_writes_atomically(tmp_path: Path) -> None: + """A crash mid-merge must not leave a torn note git believes is merged. + + The driver used `write_text`, which truncates in place, so an interrupted + write left a half-written note *and* an exit code git read as success. It is + one file (git's %A), so this is an atomic write rather than a transaction — + there is no multi-note window to roll back. + """ + base = tmp_path / "base.md" + ours = tmp_path / "ours.md" + theirs = tmp_path / "theirs.md" + for path, summary in ((base, "base"), (ours, "ours"), (theirs, "theirs")): + path.write_text(f"# N\n\n## Summary\n{summary}\n", encoding="utf-8") + + assert merge.run_merge_driver(base, ours, theirs) == 0 + text = ours.read_text(encoding="utf-8") + assert "## Summary" in text + # No leftover temp files beside the target. + assert [p.name for p in tmp_path.glob(".tmp-*")] == [] diff --git a/tests/test_txn.py b/tests/test_txn.py index 44c20ca..e9e332a 100644 --- a/tests/test_txn.py +++ b/tests/test_txn.py @@ -317,3 +317,31 @@ def windows_like_text_writer(path: Path, text: str) -> None: monkeypatch.setattr(store, "_atomic_write", windows_like_text_writer) assert txn.recover(omi)[0].clean assert a.read_bytes() == original # byte-identical, not merely equivalent + + +def test_a_journaled_removal_is_rolled_back(omi: Path) -> None: + """Deleting is journaled like writing, so an interrupted move is undone.""" + doomed = omi / "Stray.md" + doomed.write_bytes(b"# Stray\n\n- entry one\n") + keeper = omi / "Target.md" + keeper.write_bytes(b"# Target\n") + + t = txn.Transaction(omi) + t.write(keeper, "# Target\n\n- entry one\n") + t.remove(doomed) + t.prepare() + t.apply(_atomic_write) + assert not doomed.exists() # the move happened + + assert txn.recover(omi)[0].clean # ...then the process died before commit + assert doomed.read_bytes() == b"# Stray\n\n- entry one\n" # back, byte-exact + assert keeper.read_bytes() == b"# Target\n" + + +def test_removing_a_file_that_never_existed_is_not_a_conflict(omi: Path) -> None: + t = txn.Transaction(omi) + t.remove(omi / "Ghost.md") + t.prepare() + t.apply(_atomic_write) + report = txn.recover(omi)[0] + assert report.clean and report.skipped == ["Ghost.md"]