From e2e0002bab079a08a25c5fc719cb89dedf128581 Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 03:57:38 -0400 Subject: [PATCH 01/14] fix(chat-store): make group creation and chat moves atomic create_group read the highest position and inserted afterwards, so two groups made at the same moment (routes run on a thread pool) could take the same position. set_chat_group checked that the group existed and then updated the chat without holding the write lock, so a delete_group landing in between left the chat filed under a group that no longer exists, hidden from the sidebar until the next startup sweep. create_group is now one INSERT ... SELECT, and set_chat_group takes the write lock (BEGIN IMMEDIATE) before it looks at the group. Tests: eight threads behind a barrier must get positions 0-7; a second connection deleting the group exactly between the check and the move must not leave an orphaned chat. Both fail on the previous code. Plan item: BE-54 Co-Authored-By: Claude Sonnet 5.5 --- .../cortex_backend/repositories/storage.py | 16 +++- tests/test_chat_groups.py | 92 +++++++++++++++++++ 2 files changed, 103 insertions(+), 5 deletions(-) diff --git a/backend/cortex_backend/repositories/storage.py b/backend/cortex_backend/repositories/storage.py index c167036..eb9113c 100644 --- a/backend/cortex_backend/repositories/storage.py +++ b/backend/cortex_backend/repositories/storage.py @@ -1556,13 +1556,14 @@ def create_group(self, group_id: str, name: str) -> None: """Append a group after every existing one.""" try: with self.connect() as conn: - next_position = conn.execute( - "SELECT COALESCE(MAX(position), -1) + 1 FROM chat_groups" - ).fetchone()[0] + # One statement, so choosing the next position and inserting it + # cannot be split by another writer. Reading the maximum first + # and inserting afterwards let two groups made at the same + # moment (routes run on a thread pool) take the same position. conn.execute( "INSERT INTO chat_groups (id, name, position, collapsed, timestamp) " - "VALUES (?, ?, ?, 0, ?)", - (group_id, name, next_position, _utc_now_iso()), + "SELECT ?, ?, COALESCE(MAX(position), -1) + 1, 0, ? FROM chat_groups", + (group_id, name, _utc_now_iso()), ) except PersistenceError as exc: raise PersistenceError( @@ -1632,6 +1633,11 @@ def set_chat_group(self, thread_id: str, group_id: str | None) -> bool: """Move a chat into a group, or out of every group when ``group_id`` is None.""" try: with self.connect() as conn: + # The write lock is taken before the group is looked at. Without + # it, a delete_group landing between this check and the UPDATE + # left the chat filed under a group that no longer exists, and + # the sidebar hid it until the next startup sweep. + conn.execute("BEGIN IMMEDIATE") if group_id is not None and conn.execute( "SELECT 1 FROM chat_groups WHERE id = ?", (group_id,) ).fetchone() is None: diff --git a/tests/test_chat_groups.py b/tests/test_chat_groups.py index 0f9200f..d2e9812 100644 --- a/tests/test_chat_groups.py +++ b/tests/test_chat_groups.py @@ -10,6 +10,7 @@ import os import shutil import sqlite3 +import threading from pathlib import Path import pytest @@ -99,6 +100,97 @@ def test_moving_an_unknown_chat_reports_miss_without_raising(tmp_path: Path) -> assert repository.set_chat_group("missing-thread", "g1") is False +class _InterleavingConnection(sqlite3.Connection): + """A connection that lets a test run another writer at a chosen statement. + + ``before`` is called with each statement's text just before it executes, so + a test can land a second connection's write exactly between two statements + of the operation under test instead of hoping two threads collide. + """ + + before = None + + def execute(self, sql, *args): + hook = type(self).before + if hook is not None: + hook(sql) + return super().execute(sql, *args) + + +def test_concurrent_group_creation_never_produces_duplicate_positions(tmp_path: Path) -> None: + database = DatabaseManager(db_path=str(tmp_path / "chats.sqlite")) + workers = 8 + start = threading.Barrier(workers) + failures: list[BaseException] = [] + + def create(index: int) -> None: + try: + start.wait(timeout=10) + database.create_group(f"g{index}", f"Group {index}") + except BaseException as exc: # reported below, from the test thread + failures.append(exc) + + threads = [threading.Thread(target=create, args=(index,)) for index in range(workers)] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=30) + assert not thread.is_alive(), "a group-creating thread did not finish" + + assert failures == [] + assert sorted(group["position"] for group in database.list_groups()) == list(range(workers)) + + +def test_a_group_deleted_while_a_chat_is_being_moved_cannot_orphan_the_chat( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The check that the group exists and the move must be one atomic step. + + A second connection deletes the group at the moment the move is about to be + written. Whether that delete is held off until the move commits, or lands + first and refuses the move, the chat must never end up filed under a group + that is gone -- the sidebar hides such a chat until the next startup sweep. + """ + path = str(tmp_path / "chats.sqlite") + database = DatabaseManager(db_path=path) + database.create_chat("t1", "Alpha") + database.create_group("g1", "Research") + real_connect = sqlite3.connect + interleaved: list[str] = [] + + def delete_the_group_now(sql: str) -> None: + if interleaved or not sql.lstrip().upper().startswith("UPDATE THREADS SET GROUP_ID = ?"): + return + interleaved.append("tried") + other = real_connect(path, timeout=0.05) + try: + other.execute("UPDATE threads SET group_id = NULL WHERE group_id = 'g1'") + other.execute("DELETE FROM chat_groups WHERE id = 'g1'") + other.commit() + interleaved[0] = "deleted" + except sqlite3.OperationalError: + interleaved[0] = "held off" + finally: + other.close() + + def connect(*args, **kwargs): + kwargs.setdefault("factory", _InterleavingConnection) + return real_connect(*args, **kwargs) + + monkeypatch.setattr(sqlite3, "connect", connect) + monkeypatch.setattr(_InterleavingConnection, "before", staticmethod(delete_the_group_now)) + try: + database.set_chat_group("t1", "g1") + except PersistenceError: + pass # a refused move is acceptable; an orphaned chat is not + monkeypatch.undo() + + assert interleaved, "the second writer never got a chance to run" + known = {group["id"] for group in database.list_groups()} + filed_under = database.get_all_chats_summary()[0]["group_id"] + assert filed_under is None or filed_under in known + + # -- schema migration ------------------------------------------------------ From 3627420770e154e306ec9a0e66603ecdc4ccff5a Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:00:29 -0400 Subject: [PATCH 02/14] fix(persistence): retry short Windows sharing violations and fsync the memory backup os.replace failed on the first PermissionError, so a scanner or indexer holding the target open for a few milliseconds turned a routine save or backup into a PersistenceError; for permanent memory that rolled the in-memory list back and the user's edit was refused. The memory backup copy was also written without an fsync, unlike the primary file it protects. replace_with_retry (repositories/sqlite_backup.py, shared by both stores) tries at most four times, waiting 50, 100 and 200 ms, and only for PermissionError; any other error is raised at once and a lock that does not lift is reported after about a third of a second with nothing moved. It now publishes the chat database backup and its rotation, the settings database backup, the memory file, its backup copy and the set-aside of a corrupt memory file. The memory backup copy is fsynced before it replaces the backup. Recovery's quarantine moves and the sidecar moves are left as they were: they fail closed on purpose and their tests depend on it. Two existing tests simulated a lock by refusing the first rename only; they now refuse every attempt, since a lock that lifts is what succeeds by design. Plan item: BE-57 Co-Authored-By: Claude Sonnet 5.5 --- .../repositories/sqlite_backup.py | 35 ++++++ .../repositories/sqlite_settings.py | 3 +- .../cortex_backend/repositories/storage.py | 20 ++-- tests/test_chat_db_recovery.py | 40 ++++++- tests/test_persistence.py | 103 ++++++++++++++++++ tests/test_sqlite_backup.py | 67 ++++++++++++ tests/test_sqlite_settings_recovery.py | 28 +++++ 7 files changed, 286 insertions(+), 10 deletions(-) diff --git a/backend/cortex_backend/repositories/sqlite_backup.py b/backend/cortex_backend/repositories/sqlite_backup.py index a87c38d..2dbf54f 100644 --- a/backend/cortex_backend/repositories/sqlite_backup.py +++ b/backend/cortex_backend/repositories/sqlite_backup.py @@ -7,6 +7,8 @@ * ``snapshot_database`` copies a live database through SQLite's online backup API, so the copy includes commits that are still only in the ``-wal`` sidecar. +* ``replace_with_retry`` publishes a file over another one, waiting out the + short sharing violations Windows reports while a scanner has the target open. * ``open_at_rest`` / ``quick_check_at_rest`` read a backup that nothing is writing without leaving ``-wal`` and ``-shm`` files beside it, and bound the time a check may take. @@ -41,6 +43,10 @@ SIDECAR_SUFFIXES = ("-wal", "-shm") +# How often replace_with_retry tries, and how long it first waits between tries. +REPLACE_ATTEMPTS = 4 +REPLACE_RETRY_DELAY_SECONDS = 0.05 + # SQLite result codes (sqlite3.SQLITE_BUSY / SQLITE_LOCKED only exist from # Python 3.11, and this project supports 3.10). _SQLITE_BUSY = 5 @@ -100,6 +106,35 @@ def failure_detail(message: str, cause: BaseException | None) -> str: return f"{message} ({type(cause).__name__})" if cause is not None else message +def replace_with_retry( + source: str | os.PathLike[str], + destination: str | os.PathLike[str], + *, + attempts: int = REPLACE_ATTEMPTS, + delay: float = REPLACE_RETRY_DELAY_SECONDS, +) -> None: + """``os.replace`` that waits out a target another program is briefly holding. + + On Windows a virus scanner, search indexer or backup agent can have the + target open without ``FILE_SHARE_DELETE`` for a few tens of milliseconds, + and renaming over it then fails with ``PermissionError`` -- a sharing + violation that is almost always gone by the next try. Only that error is + retried, at most ``attempts`` times in all with the wait doubling from + ``delay`` (about a third of a second by default), so a lock that does not + lift is still reported promptly and unchanged: nothing has been moved when + ``os.replace`` raises. Any other ``OSError`` is raised at once. The retry + is not limited to Windows; where nothing holds files open it never fires. + """ + for attempt in range(1, attempts + 1): + try: + os.replace(source, destination) + return + except PermissionError: + if attempt == attempts: + raise + time.sleep(delay * 2 ** (attempt - 1)) + + def snapshot_database( source: str | os.PathLike[str], destination: str | os.PathLike[str], diff --git a/backend/cortex_backend/repositories/sqlite_settings.py b/backend/cortex_backend/repositories/sqlite_settings.py index bc30ba9..5d38007 100644 --- a/backend/cortex_backend/repositories/sqlite_settings.py +++ b/backend/cortex_backend/repositories/sqlite_settings.py @@ -27,6 +27,7 @@ find_interrupted_recovery, move_sidecars, put_sidecars_back, + replace_with_retry, snapshot_database, utc_now_iso, ) @@ -370,7 +371,7 @@ def _publish_verified_copy( populate(temporary_path) if not cls._database_is_valid(temporary_path): raise OSError("database copy failed integrity validation") - os.replace(temporary_path, destination) + replace_with_retry(temporary_path, destination) # Validating the copy opened it, so SQLite made "-wal" and # "-shm". os.replace moves only the file itself. for suffix in ("-wal", "-shm"): diff --git a/backend/cortex_backend/repositories/storage.py b/backend/cortex_backend/repositories/storage.py index eb9113c..07d97b0 100644 --- a/backend/cortex_backend/repositories/storage.py +++ b/backend/cortex_backend/repositories/storage.py @@ -38,6 +38,7 @@ open_at_rest, put_sidecars_back, quick_check_at_rest, + replace_with_retry, snapshot_database, utc_now_iso, ) @@ -457,17 +458,17 @@ def _publish_verified_copy( if displace_existing_to is not None and os.path.exists(destination): older_file_kept_as = cls._link_to_spare_name(displace_existing_to) try: - os.replace(destination, displace_existing_to) + replace_with_retry(destination, displace_existing_to) except OSError: cls._drop_spare_name(older_file_kept_as) raise displaced_to = displace_existing_to try: - os.replace(temporary_path, destination) + replace_with_retry(temporary_path, destination) except OSError: if displaced_to is not None: try: - os.replace(displaced_to, destination) + replace_with_retry(displaced_to, destination) except OSError: logging.warning("Could not return a displaced database file to its name.") else: @@ -536,7 +537,7 @@ def _restore_from_spare_name(spare: str | None, path: str) -> None: if spare is None: return try: - os.replace(spare, path) + replace_with_retry(spare, path) except OSError: logging.warning("Could not return an older database backup to its name.") @@ -1718,8 +1719,13 @@ def _atomic_copy_memos(cls, source: str, destination: str) -> None: ) os.close(fd) shutil.copy2(source, temporary_path) + # The copy is the recovery file for the next launch, so it gets the + # same flush to disk the primary gets in _save_memos before it is + # moved into place; copy2 only hands the bytes to the OS. + with open(temporary_path, "r+b") as copy: + os.fsync(copy.fileno()) cls._read_memos(temporary_path) - os.replace(temporary_path, destination) + replace_with_retry(temporary_path, destination) temporary_path = None except (OSError, shutil.Error, TypeError, ValueError, json.JSONDecodeError) as exc: raise PersistenceError( @@ -1829,7 +1835,7 @@ def _set_aside_corrupt_memory_file(self, cause: BaseException) -> None: """ damaged_path = f"{self.memory_file_path}.corrupt" try: - os.replace(self.memory_file_path, damaged_path) + replace_with_retry(self.memory_file_path, damaged_path) except OSError as exc: # Usually a lock. The save that follows would fail on the same # file anyway, so report the condition rather than pressing on. @@ -1866,7 +1872,7 @@ def _save_memos(self): self._prepare_primary_for_save() if os.path.exists(self.memory_file_path): self._atomic_copy_memos(self.memory_file_path, self.backup_file_path) - os.replace(temporary_path, self.memory_file_path) + replace_with_retry(temporary_path, self.memory_file_path) temporary_path = None except (OSError, TypeError, ValueError, json.JSONDecodeError) as exc: raise PersistenceError( diff --git a/tests/test_chat_db_recovery.py b/tests/test_chat_db_recovery.py index 2975c56..14039f9 100644 --- a/tests/test_chat_db_recovery.py +++ b/tests/test_chat_db_recovery.py @@ -719,8 +719,9 @@ def replace(source, destination, *args, **kwargs): if ( os.path.normcase(os.path.abspath(destination)) == backup and str(source).endswith(".tmp") - and not refused ): + # Refused on every try: a lock that lifts is retried and succeeds + # (see tests/test_persistence.py), so this one has to stay. refused.append(str(source)) raise PermissionError(errno.EACCES, "The process cannot access the file") return real_replace(source, destination, *args, **kwargs) @@ -738,6 +739,40 @@ def replace(source, destination, *args, **kwargs): assert _reopen(manager).backup_status == ("ok", None) +def test_a_backup_held_briefly_by_another_program_is_still_published( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A scanner holding the backup for a few milliseconds is not a failed backup.""" + manager, _ = _manager_with_data(tmp_path) + manager.update_chat_title("thread-1", "Changed after the last backup") + backup_before = Path(manager.backup_path).read_bytes() + real_replace = os.replace + backup = os.path.normcase(os.path.abspath(manager.backup_path)) + refused: list[str] = [] + monkeypatch.setattr("time.sleep", lambda _seconds: None) + + def replace(source, destination, *args, **kwargs): + if ( + os.path.normcase(os.path.abspath(destination)) == backup + and str(source).endswith(".tmp") + and len(refused) < 2 + ): + refused.append(str(source)) + raise PermissionError(errno.EACCES, "The process cannot access the file") + return real_replace(source, destination, *args, **kwargs) + + monkeypatch.setattr(os, "replace", replace) + + reopened = _reopen(manager) + monkeypatch.undo() + + assert len(refused) == 2 + assert reopened.backup_status == ("ok", None) + assert Path(manager.backup_path).read_bytes() != backup_before + assert reopened.load_chat("thread-1")["title"] == "Changed after the last backup" + assert DatabaseManager._database_is_valid(manager.backup_path) + + def _stray_files(directory: Path) -> list[str]: """Anything a rotation may set aside while it works and must not leave behind.""" return sorted(entry.name for entry in directory.iterdir() if entry.name.endswith((".old", ".tmp"))) @@ -762,8 +797,9 @@ def replace(source, destination, *args, **kwargs): if ( os.path.normcase(os.path.abspath(destination)) == backup and str(source).endswith(".tmp") - and not refused ): + # Refused on every try: a lock that lifts is retried and succeeds + # (see tests/test_persistence.py), so this one has to stay. refused.append(str(source)) raise PermissionError(errno.EACCES, "The process cannot access the file") return real_replace(source, destination, *args, **kwargs) diff --git a/tests/test_persistence.py b/tests/test_persistence.py index 4362fc1..d0a4196 100644 --- a/tests/test_persistence.py +++ b/tests/test_persistence.py @@ -1,5 +1,6 @@ """Persistence, migration, and recovery tests for local Cortex data.""" +import errno import json import os import shutil @@ -10,6 +11,8 @@ from pathlib import Path from unittest.mock import patch +import pytest + import cortex_backend.repositories.storage as storage from cortex_backend.repositories.storage import ( DatabaseManager, @@ -474,3 +477,103 @@ def test_forking_keeps_each_message_timestamp(self): [message["content"] for message in forked], ["first", "second"], ) + + +# -- Windows sharing violations and the memory backup copy --------------------------- + + +def _memory_manager(tmp_path: Path) -> tuple[PermanentMemoryManager, Path]: + memory_file = tmp_path / "memory.json" + manager = PermanentMemoryManager(memory_file_path=str(memory_file)) + manager.add_memo("first") # the next save has a primary to back up + return manager, memory_file + + +def _on_disk_memos(path: Path) -> list[str]: + return json.loads(path.read_text(encoding="utf-8"))["memos"] + + +def _leftover_temporaries(directory: Path) -> list[str]: + return sorted(entry.name for entry in directory.iterdir() if entry.name.endswith(".tmp")) + + +def _hold_the_memory_file(monkeypatch: pytest.MonkeyPatch, memory_file: Path, *, refusals: int): + """Refuse the first ``refusals`` renames onto the memory file, as a scanner holding it would.""" + real_replace = os.replace + target = os.path.normcase(os.path.abspath(memory_file)) + attempts: list[str] = [] + waits: list[float] = [] + + def replace(source, destination, *args, **kwargs): + if os.path.normcase(os.path.abspath(destination)) == target: + attempts.append(str(source)) + if len(attempts) <= refusals: + raise PermissionError(errno.EACCES, "The process cannot access the file") + return real_replace(source, destination, *args, **kwargs) + + monkeypatch.setattr(os, "replace", replace) + monkeypatch.setattr("time.sleep", waits.append) + return attempts, waits + + +def test_a_transient_sharing_violation_does_not_fail_a_memory_save( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + manager, memory_file = _memory_manager(tmp_path) + attempts, waits = _hold_the_memory_file(monkeypatch, memory_file, refusals=2) + + manager.add_memo("second") + monkeypatch.undo() + + assert len(attempts) == 3 and len(waits) == 2 + assert manager.get_memos() == ["first", "second"] + assert _on_disk_memos(memory_file) == ["first", "second"] + assert _on_disk_memos(Path(manager.backup_file_path)) == ["first"] + assert _leftover_temporaries(tmp_path) == [] + + +def test_a_sharing_violation_that_does_not_lift_fails_the_save_and_changes_nothing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + manager, memory_file = _memory_manager(tmp_path) + before = memory_file.read_bytes() + attempts, _ = _hold_the_memory_file(monkeypatch, memory_file, refusals=10_000) + + with pytest.raises(PersistenceError): + manager.add_memo("second") + monkeypatch.undo() + + assert len(attempts) == 4 # bounded, not a loop + assert manager.get_memos() == ["first"] # the in-memory list rolled back with the file + assert memory_file.read_bytes() == before + assert _leftover_temporaries(tmp_path) == [] + + +def test_the_memory_backup_copy_is_flushed_to_disk_before_it_replaces_the_backup( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The backup is what the next launch restores from, so it gets the same + fsync the primary gets: one for the new file and one for the backup copy.""" + manager, _ = _memory_manager(tmp_path) + backup = os.path.normcase(os.path.abspath(manager.backup_file_path)) + real_replace, real_fsync = os.replace, os.fsync + flushed_before_backup_replaced: list[int] = [] + flushes = 0 + + def fsync(descriptor): + nonlocal flushes + flushes += 1 + return real_fsync(descriptor) + + def replace(source, destination, *args, **kwargs): + if os.path.normcase(os.path.abspath(destination)) == backup: + flushed_before_backup_replaced.append(flushes) + return real_replace(source, destination, *args, **kwargs) + + monkeypatch.setattr(os, "fsync", fsync) + monkeypatch.setattr(os, "replace", replace) + + manager.add_memo("second") + monkeypatch.undo() + + assert flushed_before_backup_replaced == [2] diff --git a/tests/test_sqlite_backup.py b/tests/test_sqlite_backup.py index eddcb0b..7eeb17c 100644 --- a/tests/test_sqlite_backup.py +++ b/tests/test_sqlite_backup.py @@ -11,9 +11,11 @@ from cortex_backend.repositories import sqlite_backup from cortex_backend.repositories.sqlite_backup import ( + REPLACE_ATTEMPTS, failure_detail, move_sidecars, put_sidecars_back, + replace_with_retry, snapshot_database, ) @@ -182,3 +184,68 @@ def test_put_sidecars_back_never_overwrites_a_file_at_the_original_name(tmp_path assert Path(f"{destination}-wal").read_bytes() == b"write-ahead log" # The other sidecar had no such conflict and still went back. assert Path(f"{database}-shm").read_bytes() == b"shared memory" + + +# -- replace_with_retry -------------------------------------------------------- + + +def _refuse_replacing_until(monkeypatch: pytest.MonkeyPatch, *, refusals: int, error=PermissionError): + """Make os.replace fail ``refusals`` times, then work; record the waits between tries.""" + real_replace = os.replace + calls: list[str] = [] + waits: list[float] = [] + + def replace(source, destination, *args, **kwargs): + calls.append(str(destination)) + if len(calls) <= refusals: + raise error(13, "The process cannot access the file because it is being used") + return real_replace(source, destination, *args, **kwargs) + + monkeypatch.setattr(os, "replace", replace) + monkeypatch.setattr(time, "sleep", waits.append) + return calls, waits + + +def test_a_sharing_violation_that_lifts_is_waited_out(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + source, destination = tmp_path / "new", tmp_path / "current" + source.write_bytes(b"new") + destination.write_bytes(b"current") + calls, waits = _refuse_replacing_until(monkeypatch, refusals=2) + + replace_with_retry(source, destination) + + assert destination.read_bytes() == b"new" + assert not source.exists() + assert len(calls) == 3 + assert waits == [0.05, 0.1] # doubling, and only between tries + + +def test_a_sharing_violation_that_does_not_lift_is_raised_after_a_bounded_number_of_tries( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + source, destination = tmp_path / "new", tmp_path / "current" + source.write_bytes(b"new") + destination.write_bytes(b"current") + calls, waits = _refuse_replacing_until(monkeypatch, refusals=10_000) + + with pytest.raises(PermissionError): + replace_with_retry(source, destination) + + assert len(calls) == REPLACE_ATTEMPTS + assert len(waits) == REPLACE_ATTEMPTS - 1 + assert sum(waits) < 1.0 # a lock that stays is reported promptly + # Nothing was moved: the file that was there is intact and so is the new one. + assert destination.read_bytes() == b"current" + assert source.read_bytes() == b"new" + + +def test_an_error_other_than_a_sharing_violation_is_not_retried( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + calls, waits = _refuse_replacing_until(monkeypatch, refusals=10_000, error=FileNotFoundError) + + with pytest.raises(FileNotFoundError): + replace_with_retry(tmp_path / "missing", tmp_path / "current") + + assert len(calls) == 1 + assert waits == [] diff --git a/tests/test_sqlite_settings_recovery.py b/tests/test_sqlite_settings_recovery.py index a1ab16a..7c4bdb4 100644 --- a/tests/test_sqlite_settings_recovery.py +++ b/tests/test_sqlite_settings_recovery.py @@ -335,6 +335,34 @@ def replace(source, destination, *args, **kwargs): monkeypatch.setattr(os, "replace", replace) +def test_a_backup_held_briefly_by_another_program_is_still_published( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + """A scanner holding the backup for a few milliseconds is not a failed backup.""" + repository, _ = _repository_with_valid_backup(tmp_path) + backup_before = repository.backup_path.read_bytes() + real_replace = os.replace + backup = os.path.normcase(os.path.abspath(repository.backup_path)) + refused: list[str] = [] + monkeypatch.setattr("time.sleep", lambda _seconds: None) + + def replace(source, destination, *args, **kwargs): + if os.path.normcase(os.path.abspath(destination)) == backup and len(refused) < 2: + refused.append(str(source)) + raise PermissionError(errno.EACCES, "The process cannot access the file") + return real_replace(source, destination, *args, **kwargs) + + monkeypatch.setattr(os, "replace", replace) + + reopened = SQLiteSettingsRepository(repository.db_path) + monkeypatch.undo() + + assert len(refused) == 2 + assert reopened.backup_status == ("ok", None) + assert repository.backup_path.read_bytes() != backup_before + assert SQLiteSettingsRepository._database_is_valid(repository.backup_path) + + def _snapshot_that_fails_verification(monkeypatch: pytest.MonkeyPatch, repository) -> None: def torn(_source, destination, **_kwargs): Path(destination).write_bytes(b"torn snapshot") From c0d24069155fe7048cbf5ee88becbe53b37065bc Mon Sep 17 00:00:00 2001 From: Matthew Robert Wesney <157447210+dovvnloading@users.noreply.github.com> Date: Tue, 29 Sep 2026 04:01:45 -0400 Subject: [PATCH 03/14] fix(chat-store): stop re-announcing an already migrated legacy history directory The legacy JSON directory survives every migration pass: migrated files move to _migrated_