fix(server): snapshot service-update databases with VACUUM INTO - #8431
fix(server): snapshot service-update databases with VACUUM INTO#8431Adolanium wants to merge 2 commits into
Conversation
The launcher copied state.sqlite plus WAL plus shm with copyFile. That can be a torn snapshot. VACUUM INTO writes one consistent file even if the source is open. Restore still copies that file and drops leftover WAL and shm sidecars.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f021eae. Configure here.
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a contained service-update reliability fix that replaces potentially torn SQLite file copies with a consistent single-file snapshot while retaining the existing staging, durability, and restore flow. Production logic is limited to the rollback backup path, and the updated tests exercise snapshot integrity and launcher rollback behavior. You can add or adjust custom eligibility rules. Learn more. |
Windows still turns backslashes into slashes for SQLite. POSIX paths with a real backslash in the name stay unchanged. Launcher flow tests now seed a real sqlite file, so backup no longer fails with db-backup-failed.

What Changed
Service-update database backup now uses
VACUUM INTOinstead of copyingstate.sqliteplus WAL plus shm.That writes one consistent file. Restore still copies that file and drops leftover WAL and shm sidecars. A completed backup is still never overwritten.
Why
A live-style copy of sqlite plus WAL plus shm can be a torn snapshot. Agents.md already says
VACUUM INTOis the safe way. The launcher comment said the previous child is dead, so a quiet copy can work. Residual risk remains on Windows mapped shm and sequential copies.Checklist
Note
Medium Risk
Changes pre-update backup semantics for user SQLite state during service updates; incorrect snapshots or restore could lose data, though VACUUM INTO is intended to reduce torn-backup risk versus multi-file copy.
Overview
Service-update database backups no longer copy the live
state.sqlitefile plus-wal/-shmsidecars.backupDatabaseOncenow writes a single consistent snapshot via a newvacuumDatabaseIntohelper that opens the source read-only and runsVACUUM INTO, with Windows path normalization and quoted destination paths.Staging still uses a temp dir,
chmod 0o600, fsync, and atomic rename; restore behavior is unchanged in shape—it still copies the backed-up main file and strips leftover sidecars when rollback runs.Tests seed real SQLite databases instead of plain text, add direct coverage for
vacuumDatabaseInto(data integrity, no WAL/SHM on the snapshot, POSIX literal backslash in destinations), and assert rollback restores pre-migration row data and clears trial WAL/SHM files.Reviewed by Cursor Bugbot for commit 37b50d2. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Snapshot service-update databases with
VACUUM INTOinbackupDatabaseOncevacuumDatabaseInto, which opens the source DB read-only and runsVACUUM INTOto produce a single consistent snapshot filequoteSqliteLiteralto safely embed the destination path in the SQL statement; on Windows, backslashes are converted to forward slashes before quoting-wal/-shmsidecarsseedSqlitehelper, including new tests forVACUUM INTOsnapshotting and POSIX backslash handlingbackupDatabaseOnceno longer copies-wal/-shmfiles; any code expecting sidecar files in the backup directory will breakMacroscope summarized 37b50d2.