Date: 2026-09-30. Light review, as the plan asks for chunk A: one reviewer
(an AI agent working in its own clone, running the code and trying to
break it) covered a1a6419, 34da00e, 69e8f9b and cb7a635. Claude
then acted as the skeptic and reproduced each finding before any fix.
Nothing touched the repo, /etc or the real store during the review.
| Id | Severity | Finding | Skeptic | Outcome |
|---|---|---|---|---|
| D1 | high | The M1 backup read changes.db with os.ReadFile inside the migration’s write transaction. Closing that descriptor drops the whole process’s POSIX locks, so other processes could write mid-migration and roll its journal back. |
Reproduced: 4 sc log processes on a 50-row M1 store, 13 of 20 failed with commit: disk I/O error (5898). The reviewer also showed a corrupt changes_path_seq index when the M1 binary wrote meanwhile. |
Fixed ee0ddea: the copy is read through the connection (sqlite3_serialize), byte for byte the file, no descriptor. After: 0 of 80 failed, integrity ok, every backup complete. |
| D2 | medium | A never-migrated M1 store that the user cannot write (read-only directory or file) can no longer be read: the migration fails, where sc-m1 still lists it. |
Reproduced. | Accepted by design, recorded as a known limit. The real store is migrated by scd at sign-off S2, long before the M4 read-only rescue path; the code only promises lock-free reads for up-to-date stores. |
| D3 | low | Any entry at changes.db.m1-backup, even a dangling symlink, counted as a backup, so the store was migrated with none. |
Reproduced. | Fixed 397877a: only a regular file counts; anything else stops the migration with one line. |
| T1 | test gap | The concurrency test used goroutines, which cannot see D1. | Confirmed: the old code passed it. | ee0ddea adds two tests with real processes; both fail 3 of 3 times on the old code. |
| T2 | test gap | Nothing tested the version re-read under the lock: without it, a store a newer sc moved to schema 2 meanwhile would be set back to 1. | Confirmed by mutation. | Test added b3d3f8e; fails with the re-read removed. |
Not defects (checked by the reviewer): no code path still orders by ts;
the M1 binary snapshots, lists, restores and inits on a migrated store with
integrity ok; the newer-schema and WAL refusals are one line with exit 1;
a store migrated by Open keeps M1’s 0644 database until Init or scd
runs, as plan 5.1 says.
changes.db outside SQLite while the process has a
connection to it. Any open plus close on the file drops every lock
the process holds. Init opens it once before any connection exists,
which is safe; scd (steps 12-14) must keep it that way, and the chunk D
review should check it.TestHelperProcess), since goroutines share SQLite’s lock bookkeeping.