Harden offline durability against short writes by Yaraslaut · Pull Request #548 · LASTRADA-Software/morph

Yaraslaut · 2026-09-15T15:12:28Z

Part of the S reliability-infra lane of the framework-review sprint (morph#518,
board: https://github.com/orgs/LASTRADA-Software/projects/2). This batch covers
the three offline-durability findings that share the FileIoOps fault-injection
seam: a short-write corruption bug, a NUL-truncation bug in the SQLite backend,
and missing directory fsync / SQLite PRAGMA durability settings.

Changes

FileOfflineQueue::writeLine and FileActionLog::append rolled back a short
fwrite() instead of leaving a truncated line in the append-mode file, which
the next successful write would silently merge onto — eventually bricking the
file's load() for good. Adds a shared rollBackShortWrite() helper, a
shared repairTornTail() helper (used by both classes, replacing two
identical copies), and a Windows-safe wideFtell() to avoid the ~2GiB
std::ftell overflow reviving the same bug on that platform.
SqliteOfflineQueue::bindText/textColumn measured strings to the first
NUL byte instead of their real length, silently truncating any payload or
idempotency key containing one and colliding two genuinely distinct keys
that only differed after a shared NUL prefix.
FileOfflineQueue::compact()'s rename and FileActionLog's constructor/
rotate() never fsynced the directory entries they mutate — a new
FileIoOps::syncPath primitive closes that gap for all three IOfflineQueue
backends, including SqliteOfflineQueue's own first-open file creation.
SqliteOfflineQueue's constructor also sets PRAGMA synchronous (default
NORMAL, SQLite's own recommendation under WAL; Synchronous::full is
available and costs ~18x per mutation — see docs/spec/offline/offline.md)
and PRAGMA busy_timeout, and verifies journal_mode=WAL actually took via
a read-back rather than trusting a silent fallback.

codecov · 2026-09-15T15:36:37Z

Codecov Report

❌ Patch coverage is 93.24324% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
include/morph/offline/sqlite_offline_queue.hpp	82.22%	3 Missing and 5 partials ⚠️
include/morph/core/file_io_ops.hpp	93.40%	3 Missing and 3 partials ⚠️
include/morph/journal/file_action_log.hpp	97.36%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

FileOfflineQueue::writeLine and FileActionLog::append both threw on a short fwrite() without rolling the file back. Both handles are opened in append mode, so the next successful write concatenated directly onto the truncated JSON with no separating newline, merging two records into one line. FileOfflineQueue::load() only tolerates a malformed *trailing* line; once a later write pushed the merged line into an interior position, the file could never be reopened again. Both now capture the file offset before writing and, on a short write, roll back to it via the existing FileIoOps::resizeFile seam before throwing, through a new shared morph::core::rollBackShortWrite() helper. The helper also re-syncs the stream's stdio position with fseek after the truncation, since resizeFile truncates by path rather than through the open FILE*'s own descriptor — without that, a second consecutive short write on the same long-lived handle would capture a stale offset and pad the file with NUL bytes instead of truncating it. FileOfflineQueue also gains a repairTornTail() (mirroring FileActionLog's own), run at construction before load(), as defense-in-depth against damage that predates this fix or a crash between a write and its rollback. Fixes #530 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018X6bKYZaa2rME5P9h1CqXQ Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>

bindText() bound payloads and idempotency keys with sqlite3_bind_text's length argument set to -1, which tells SQLite to measure the string by scanning for a NUL -- silently truncating any payload or key containing one. textColumn() had the same defect on the read side, constructing a std::string from the raw C string SQLite returns instead of using its real stored length. Two distinct idempotency keys differing only after a shared NUL prefix therefore collapsed to the same value, so the partial unique index treated a second, unrelated enqueue as a duplicate of the first and silently discarded it. bindText() now passes the string's actual length (guarded against exceeding INT_MAX before the narrowing cast) and textColumn() now sizes the returned string with sqlite3_column_bytes(). Adds a shared checkNulPayloadRoundTrip() conformance check, run against all three IOfflineQueue implementations, confirming FileOfflineQueue and InMemoryOfflineQueue already round-trip NUL correctly and pinning that SqliteOfflineQueue now does too. Fixes #531 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018X6bKYZaa2rME5P9h1CqXQ Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>

Two independent durability gaps. First, fsync on a file makes only that file's data durable, never the directory entry that names it -- so a freshly created queue/journal file, FileOfflineQueue::compact()'s rename, and both of FileActionLog::rotate()'s directory mutations (the seal rename and the fresh active-file creation) could all vanish on power loss even though their contents were already fsynced. Second, SqliteOfflineQueue set only journal_mode=WAL, despite its own docs crediting that pragma alone with the class's durability. Adds FileIoOps::syncPath (POSIX open+fsync+close on the directory, documented no-op on Windows) and wires it into the three sites above; a failure is surfaced rather than swallowed, matching how every other fsync failure in these classes is already handled. A bare relative filename's parent_path() is empty rather than ".", so syncPath resolves an empty directory to the current one instead of failing every caller that derives it from parent_path(). SqliteOfflineQueue's constructor now also sets PRAGMA synchronous=FULL and PRAGMA busy_timeout, and reads journal_mode back through a real prepared statement rather than trusting the pragma's exec to have succeeded -- a filesystem without shared-memory support (NFS, some containers) silently falls back to delete-mode instead of erroring. Fixes #532 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018X6bKYZaa2rME5P9h1CqXQ Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>

file_io_ops.md still described FileIoOps as six std::function members and said nothing about syncPath; journal.md still called repairTornTail() a private FileActionLog method after it became a shared morph::core:: free function. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018X6bKYZaa2rME5P9h1CqXQ Signed-off-by: Yaraslau Tamashevich <yaraslau.tamashevich@gmail.com>

Three failing checks on this branch. **Header ↔ spec sync.** include/morph/offline/** changed (short-write rollback, torn-tail repair, directory fsync, the WAL/synchronous/busy_timeout pragmas, and NUL-safe bind/read) while docs/spec/offline/** did not. offline.md now documents all of it under FileOfflineQueue and SqliteOfflineQueue, including why journal_mode is read back rather than trusted and why the directory fsync is separate from SQLite's own. **clang-tidy-diff.** Thirteen findings on changed lines: four misc-const-correctness, two readability-identifier-length (`io` -> `ioOps`), three modernize-use-ranges, and one each of pro-type-vararg, cert-err33-c and pro-type-reinterpret-cast. The three that cannot be fixed outright (POSIX ::open is variadic by design, the fseek return is deliberately unchecked on an already-failing path, sqlite3_column_text returns `const unsigned char*`) get NOLINTNEXTLINE with the reason *above* the marker, not wrapped after it -- a wrapped reason makes the comment itself the suppressed line and the marker silently misses. **Windows cl-debug / clangcl-debug.** The new directory-fsync test unlinked the queue file while the FileOfflineQueue still held it open in append mode; Windows refuses that, and the test threw "The process cannot access the file because it is being used by another process". The queue is now scoped so it closes first. Its SqliteOfflineQueue twin had the same shape -- harmless on Linux, where that suite runs today -- and is scoped too rather than left to become a Windows failure later. Verified locally: clang-tidy-diff.py over `git diff -U0 origin/master...HEAD` against a clang-configured compile database reports nothing on changed lines, and reproduces all thirteen findings when the fixes are stashed. morph_tests 1,447 cases green; morph_offline_sqlite_tests 26 cases green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The `io` -> `ioOps` rename pushed the parameter list past the 119-column limit, and my manual wrap did not match what clang-format produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The cl-debug/clangcl-debug presets set `stopOnFailure: true`, so ctest reports one failure per run and the next only surfaces after the previous is fixed -- two CI rounds found two instances of the same mistake. This audits the file instead of waiting for a third. Every FileOfflineQueue in tests/test_file_offline_queue.cpp that is still in scope when std::filesystem::remove() runs is now scoped so its handle closes first. Windows refuses to unlink a file another handle still has open; POSIX does not, which is why these passed locally and on every Linux leg. Two sites needed it: the short-write reopen (drain hoisted out so `pending` outlives the scope) and the directory-fsync-failure reopen. A third, in the unreadable-file test, is already inside this file's `#ifndef _WIN32` block and never runs there. The SqliteOfflineQueue suite has the same shape at six pre-existing sites but is Linux-only today (MORPH_BUILD_OFFLINE_SQLITE is off on the Windows legs), so those are left alone; the one new test this branch adds there was scoped in the previous commit so it does not become a failure the day that changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A /code-review pass over this branch found 16 issues. The headline one is that morph#530's own fix reproduces morph#530. **rollBackShortWrite grew the file instead of truncating it.** It resized to a *stream* offset, and callers buffer -- FileActionLog::append() is documented as buffered-until-flush(), so ftell routinely runs ahead of the on-disk size. std::filesystem::resize_file GROWS a file when asked for an offset past its end, padding with NULs, and the later flush then appended the record after the padding. Measured: ftell=30 against an on-disk size of 10, resize_file(30) yielding 30 bytes, and a final 50-byte file of data + 20 NULs + the record. That is a NUL-bearing *interior* line, which the reader rejects for the life of the file -- permanent bricking, which repairTornTail cannot heal because the file ends in a newline. Now: clearerr, flush *and check it*, truncate nothing if the flush failed (on a full disk that is the likely case), clamp to the real file size so it can only shrink, and reposition only after the buffer is empty -- the old trailing fseek re-flushed the bytes the resize had just removed. **wideFtell returns 0 on an append stream before the first I/O** on the Microsoft CRT and musl (C11 leaves it implementation-defined; glibc seeks to end, which is why Linux CI was green). The first short write after any open would therefore roll the file back to zero bytes, destroying a whole journal or queue backlog. positionAtEnd() now normalises the position after each append-mode fopen. **The rollback only covered a short fwrite.** A queue record is far under BUFSIZ, so fwrite is a memcpy and the write(2) fails inside the following fflush -- the common manifestation of ENOSPC had no rollback at all. Now wired to fwrite, fflush and fsync alike. **compact() leaked the FILE* and orphaned the temp file** whenever syncFile(out, tmp) threw -- a path an existing fault-injection test drives on every run, which had left 31 stray *.compact-tmp files in /tmp. Now an RAII guard; measured 0 after a run. **FileOfflineQueue's constructor-time repairTornTail is removed.** It was added to heal "an interior merge from a doubled-up short write", which it provably cannot do -- it only trims after the final newline, and its own doc says interior lines are left alone. It did break morph#494 (a failed construction must leave the file byte-identical: it is the only mutation that can run before load() throws) and it discards a complete final record whose only missing byte is the newline, wiping the file when that is the only line. The doc comment claiming parity with FileActionLog was false -- master's FileOfflineQueue has no such scan. **The directory fsync refused ordinary layouts.** It needs a *read* handle on the directory, strictly stronger than writing a file inside it: on a mode-0300 spool directory fopen(path,"a") succeeds while open(dir,O_RDONLY|O_DIRECTORY) gives EACCES, and several FUSE/WSL/overlay mounts return EINVAL/ENOSYS/ENOTSUP. All three classes became unconstructible there. syncPath now returns errno, classifyDirectorySync() splits unsupported (warn, continue) from failed (EIO and anything unrecognised -- still throws), and the fd gains O_CLOEXEC and EINTR retries. **SQLite pragmas were ordered backwards and over-strict.** busy_timeout was set *after* every statement that can return SQLITE_BUSY, including the journal_mode=WAL conversion that needs an exclusive lock -- so the multi-opener case it was added for failed exactly as before (12ms to throw without it; a full 1001ms wait with a 1000ms timeout set first). It is now the first statement after sqlite3_open, and configurable, with 0 restoring fail-fast -- the default 5s otherwise stalls under this class's own mutex, blocking a Qt GUI thread. synchronous=FULL is now a constructor parameter defaulting to NORMAL (measured 18x per mutation: 0.08ms -> 1.44ms; a 200-item drain 16ms -> 290ms), set before journal_mode so it holds whichever mode takes. A journal_mode that is not wal now warns instead of throwing: :memory:, NFS, CIFS and some overlay mounts are not less durable once synchronous is set, and kanban's enableOfflineQueue() builds one of these from a user-supplied path. **The morph#531 NUL test was vacuous for two of three backends.** enqueue() writes the record *and* keeps the item in memory, and drain() serves the map, so it compared the values it had just passed in. Verified: deleting escape_control_characters left it green. checkNulPayloadRoundTrip now takes a reopen factory; with it, that mutation fails two cases. **The busy_timeout test could call std::terminate**, unwinding past a joinable std::thread on a failed REQUIRE, and leaked the second connection on that path. Now jthread + a connection guard + catch(...), and it waits on the writer's own flag instead of a 200ms sleep that a loaded runner could outrun. Spec updated per AGENTS.md: journal.md gains a "Directory durability" section and corrects three statements; file_io_ops.md documents the free functions and the corrected signatures; offline.md rewrites both queues' durability sections. New tests cover the flush-path rollback, the unsupported-vs-failed directory fsync split, and a WAL database reporting journalMode()=="wal". Each was observed to fail against the unfixed code. Verified: morph_tests 1,450 cases green, morph_offline_sqlite_tests 27 green (and back to 0.23s from 5.89s), clang-tidy-diff and clang-format clean on changed lines, 0 stray temp files. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

classifyDirectorySync wrapped its errno switch in `#ifndef _WIN32`, so on Windows every nonzero result fell through to `failed`. The real syncPath is a documented no-op there and returns 0, so this only ever sees an injected value -- but the new unsupported-fsync test injects EACCES and expects `unsupported`, and got `failed` on the clangcl-debug leg alone. Classifying the value rather than the platform is what makes an injected errno mean the same thing everywhere. Every constant in the switch is in <cerrno> on the Microsoft CRT too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Third instance of the same mistake on this branch, and the last: the flush-rollback test added in the review-fix commit holds the reopened queue open across its std::filesystem::remove(). Windows refuses to unlink a file another handle still has open; POSIX does not, so it passes everywhere except the cl-debug and clangcl-debug legs. The earlier audit that caught the other two ran before this test existed. Re-run over the whole file now: this was the only remaining unguarded site, the one at :866 being inside the file's own `#ifndef _WIN32` block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Codecov reported 36 changed lines with no coverage. Measured locally with llvm-cov against the same changed-line set, three of the four touched headers are now at 100%: file_io_ops.hpp 90 covered, 0 uncovered (was 6 missing, 11 partial) file_action_log.hpp 28 covered, 0 uncovered (was 4 missing, 3 partial) file_offline_queue.hpp 49 covered, 0 uncovered sqlite_offline_queue.hpp 46 covered, 7 uncovered (was 6 missing, 6 partial) -> 213/220 = 96.82% of changed lines, up from 80.75% **tests/test_file_io_ops.cpp** is new: the free functions morph#530/#532 added were only ever reached through the three classes that call them, along whatever paths those happened to take -- which left every error classification and every degenerate argument untested, and those are exactly the parts whose whole job is to behave when something has already gone wrong. It covers classifyDirectorySync across the permission, unimplemented and real-I/O errno groups (getting a code onto the wrong side either bricks an ordinary deployment or downgrades a real durability failure to a warning), rollBackShortWrite's negative-offset, failing-flush, clamped and ordinary paths, positionAtEnd's null and append-stream cases, and repairTornTail's read-error path. **The EINTR retry is now testable rather than untestable.** It was inlined twice inside `syncPath`'s lambda, where no test can arrange for a signal to arrive mid-open. Extracted as `retryOnEintr`, it is ordinary code driven by a callable that fails once -- and the extraction also removes the duplicated loop. **SqliteOfflineQueue reads `PRAGMA synchronous` back** and exposes `synchronousLevel()`, for the same reason journal_mode is verified: exec discards the row, and `synchronous` is a per-connection property, so this is the only way a caller can confirm the level it asked for is in force. That turns the Synchronous test from "constructs without throwing" into an assertion against what SQLite actually applied (2 for full, 1 for normal). New behaviour tests, not line-touching: the unsupported-directory-fsync twins for FileActionLog and SqliteOfflineQueue (both assert the queue still *works*, which is the half that would be lost if it threw), writeLine's fsync rollback, and load()'s mid-read I/O error. The fsync rollback was verified to fail when the check is mutated away; a directory stands in for the read error, since opening one succeeds and the first read sets badbit. The seven lines that remain uncovered are two pairs of defensive throws for a PRAGMA returning no row (SQLite always returns one) and a sqlite3_bind_int64 failure, plus two `execOrThrow(` call lines that llvm-cov maps to zero while the argument line immediately below reports 42 -- an artifact, not a gap. Reaching any of them needs SQLite itself fault-injected. Verified: morph_tests 1,463 cases, morph_offline_sqlite_tests 29 cases, check_branch_coverage.py passes, clang-tidy-diff and clang-format clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot

🟡 Changes recommended

Rollback failure handling, SQLite defaults, and several durability paths remain inconsistent with the stated guarantees.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Hardens offline persistence against partial writes, embedded NULs, and incomplete filesystem durability.

Changes:

Adds rollback and torn-tail file I/O helpers.
Adds SQLite durability settings and binary-safe text handling.
Expands fault-injection, durability, and conformance tests.

File summaries

File	Description
`tests/test_offline_queue.cpp`	Tests NUL round-tripping in memory.
`tests/test_file_offline_queue.cpp`	Tests rollback and directory syncing.
`tests/test_file_io_ops.cpp`	Covers new file I/O helpers.
`tests/test_action_log_phase2.cpp`	Tests action-log durability behavior.
`tests/offline_sqlite/test_sqlite_offline_queue.cpp`	Tests SQLite durability and NUL handling.
`tests/offline_queue_conformance.hpp`	Adds shared NUL conformance checks.
`tests/CMakeLists.txt`	Registers file I/O tests.
`include/morph/offline/sqlite_offline_queue.hpp`	Adds PRAGMAs, directory sync, and length-safe text binding.
`include/morph/offline/file_offline_queue.hpp`	Rolls back failed writes and syncs compaction renames.
`include/morph/journal/file_action_log.hpp`	Adds rollback and directory durability handling.
`include/morph/core/file_io_ops.hpp`	Introduces shared recovery and sync primitives.
`docs/spec/offline/offline.md`	Documents queue durability changes.
`docs/spec/journal/journal.md`	Documents journal durability behavior.
`docs/spec/core/file_io_ops.md`	Specifies the expanded I/O abstraction.

Review details

Files reviewed: 14/14 changed files
Comments generated: 7
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    if (!errorCode) {
+        target = std::min(target, onDisk);  // only ever shrink
+    }
+    ioOps.resizeFile(path, target, errorCode);


+    if (ioOps.fflush(file) != 0) {
+        // Cannot reason about what reached disk, and cannot drop what did not.
+        // Leaving the file untouched is strictly safer than truncating to an
+        // offset that may exceed its real size -- see the note above.
+        return;


+        bool const dirSyncFailed = ::morph::core::classifyDirectorySync(_io.syncPath(_path.parent_path())) ==
+                                   ::morph::core::DirectorySync::failed;
+        bool const sealedDirSyncFailed =
+            sealedPath.parent_path() != _path.parent_path() &&
+            ::morph::core::classifyDirectorySync(_io.syncPath(sealedPath.parent_path())) ==
+                ::morph::core::DirectorySync::failed;


+    ///         `journal_mode` that does not end up as `wal` is **warned about,
+    ///         not thrown** — see the constructor body.
+    explicit SqliteOfflineQueue(std::filesystem::path path, std::optional<std::size_t> maxDepth = std::nullopt,
+                                ::morph::core::FileIoOps ioOps = {}, Synchronous synchronous = Synchronous::normal,


+            // `FileActionLog`/`FileOfflineQueue` (see `FileIoOps::syncPath`'s
+            // own docs). Unconditional: harmless when the file already
+            // existed, since syncing an unchanged directory is a cheap no-op.
+            auto const dirSync = ::morph::core::classifyDirectorySync(_io.syncPath(_path.parent_path()));


+    std::jthread writer{[&] {
+        writerEntered = true;
+        try {
+            (void)queue.enqueue("blocked-until-lock-released");


 | `LogEntry` is a plain aggregate | **No `glz::meta`** | Same automatic reflection `BRIDGE_REGISTER_ACTION` uses; no manual schema maintenance. |
 | Error path sharing | **`detail::throwOnGlazeError` for both `toJson`/`fromJson`** | `fromJson`'s failure is easy to test (malformed input); `toJson`'s is structurally unreachable for `LogEntry`. Routing both through one non-template function means the same compiled branch covers both, so `toJson`'s error path is exercised by `fromJson`'s tests. |
-| No entry-level deletion | **Append-only, no per-entry deletion API** | Permanent audit trail — unlike `IOfflineQueue` whose `markDone()` deletes retried items. `FileActionLog`'s `rotate()` and private `repairTornTail()` operate on the file, not on entries, and are not part of `IActionLog`. |
+| No entry-level deletion | **Append-only, no per-entry deletion API** | Permanent audit trail — unlike `IOfflineQueue` whose `markDone()` deletes retried items. `FileActionLog`'s `rotate()` and `morph::core::repairTornTail()` (shared with `FileOfflineQueue`; see `docs/spec/core/file_io_ops.md`) operate on the file, not on entries, and are not part of `IActionLog`. |


Five of the seven review comments held up. The first is a bricking bug the rollback itself could cause, reproduced before it was fixed. - core/file_io_ops.hpp, offline/file_offline_queue.hpp, journal/file_action_log.hpp: `rollBackShortWrite` ignored `resize_file`'s error code and told its callers nothing, so a rollback that could not truncate looked identical to one that had. It now returns `RollBack::clean` or `RollBack::torn`, and both callers latch `torn` and refuse every later write. Why that matters, measured: with the write short and the rollback's own flush failing (one full disk produces both), a partial record stays at the end of the file. `load()` tolerates that only while it is the *trailing* line. Let space free up and one more enqueue succeed on the same live object, and the new record concatenates onto the partial bytes with no separating newline -- the merged line is then interior, and reopening the queue throws a raw glaze parse error instead of loading, taking every record with it, including ones written long before the failure. That is the bricking morph#530 exists to prevent, reached through the rollback rather than around it. New regression test; it failed exactly that way before the fix. Not applied: fsyncing the file after a successful resize. An untruncated tail surviving a crash is always a *trailing* torn line, which `repairTornTail` (log) and `load()`/`compact()` (queue) already heal at the next open, so the extra fsync -- on a path that is already failing, and which can itself fail -- buys no guarantee that is not already there. - journal/file_action_log.hpp: `rotate()` collapsed both directory-sync results to `== failed`, which dropped the `unsupported` warning the constructor emits and the spec requires. Each distinct unsupported parent is now logged at warn; only `failed` still throws. - offline/sqlite_offline_queue.hpp: the directory to fsync is now asked of `sqlite3_db_filename(db, "main")` instead of derived from the constructor's path. `:memory:`, `""` and the `file::memory:` spellings have an empty `parent_path()`, which `syncPath` resolves to `"."` -- so this used to fsync the process's current working directory and report it as the queue's: a warning naming a database that is not on disk, or a refusal to construct an in-memory queue at all, over a directory it never touches. - tests/offline_sqlite/test_sqlite_offline_queue.cpp: the busy-timeout test published `writerEntered` *before* calling `enqueue()`, so the assertions that the call was still blocked ran before it had reached `sqlite3_step` and passed vacuously -- the test stayed green with the `busy_timeout` PRAGMA removed. It now requires the call to remain pending across a bounded window, which a missing timeout would end with SQLITE_BUSY well inside. - docs/spec/journal/journal.md: said `repairTornTail()` is "shared with `FileOfflineQueue`", which contradicts both that class and offline.md -- the queue deliberately does not call it. Not applied: defaulting `PRAGMA synchronous` to FULL. offline.md justifies NORMAL as SQLite's own recommendation under WAL, with the loss window absorbed by at-least-once delivery plus idempotencyKey dedup, and FULL measured at ~18x per mutation. The inconsistency the reviewer found was real but lived in the PR description, which claimed the constructor sets FULL; that has been corrected instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`include/morph/journal` branch coverage fell to 96.38%, below its 97% floor, because the previous commit added branches no test takes: `append()`'s refusal after a latched `RollBack::torn`, and `rotate()`'s per-parent `unsupported` warning. Both are reachable through `FileIoOps` injection, which is what the rest of this file's fault-injection block already does. - append: a short write whose rollback flush also fails (one full disk produces both) latches the torn tail; the next append -- with the injected failures cleared, so it would otherwise succeed -- must throw "refusing to append" rather than concatenate onto the partial record. - rotate: `syncPath` returning EACCES is `unsupported`, not `failed`. The sealed segment goes to a different directory so both parents are classified, and the test asserts *two* warnings -- one per distinct parent -- which is what the collapsed-to-bool version could not have produced. Measured locally after the change: include/morph/journal branch coverage 99.28% (file_action_log.hpp 98.61%), against the 97% floor. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Yaraslaut force-pushed the batch/530-531-532 branch from 294f9fd to aa24cbe Compare September 15, 2026 16:12

Yaraslaut and others added 5 commits September 15, 2026 22:57

Yaraslaut force-pushed the batch/530-531-532 branch from aa24cbe to b3fb1b0 Compare September 15, 2026 21:10

Yaraslaut and others added 5 commits September 15, 2026 23:29

clang-format: rewrap repairTornTail's signature

99db8f7

The `io` -> `ioOps` rename pushed the parameter list past the 119-column limit, and my manual wrap did not match what clang-format produces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Yaraslaut mentioned this pull request Sep 16, 2026

CI: self-hosted runners intermittently fail FetchContent clones with "could not read Username for 'https://github.com'" #552

Closed

Yaraslaut force-pushed the batch/530-531-532 branch from 1149ddf to 2e33084 Compare September 16, 2026 18:18

Yaraslaut marked this pull request as ready for review September 16, 2026 18:43

Yaraslaut requested a balanced review from Copilot September 16, 2026 18:48

Copilot started reviewing on behalf of Yaraslaut September 16, 2026 18:53 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Yaraslaut and others added 2 commits September 16, 2026 21:39

Yaraslaut merged commit fcef5f7 into master Sep 17, 2026
73 of 101 checks passed

Navigation Menu

Uh oh!

Harden offline durability against short writes - #548

Harden offline durability against short writes#548
Yaraslaut merged 13 commits into
masterfrom
batch/530-531-532

Yaraslaut commented Sep 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 15, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Yaraslaut commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

codecov Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Yaraslaut commented Sep 15, 2026 •

edited

Loading

codecov Bot commented Sep 15, 2026 •

edited

Loading