Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand Down
36 changes: 26 additions & 10 deletions src/omind/journal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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


Expand Down
7 changes: 6 additions & 1 deletion src/omind/merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
TEMPLATE_SECTIONS,
ActionItem,
NoteFields,
_atomic_write,
parse_note,
render_fields,
split_sections,
Expand Down Expand Up @@ -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
Expand Down
38 changes: 33 additions & 5 deletions src/omind/txn.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
}


Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions tests/test_journal.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
22 changes: 22 additions & 0 deletions tests/test_merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

from pathlib import Path

from omind import merge
from omind.merge import (
CONFLICT_TAG,
merge_fields,
Expand Down Expand Up @@ -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-*")] == []
28 changes: 28 additions & 0 deletions tests/test_txn.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]