ADR-009 stages 9.1-9.4: migrations, backups, secret scrubbing, .graphlink archive - #303
Merged
Merged
Conversation
…covery chats.db had no way to evolve and no way to recover. Schema was re-probed with CREATE TABLE IF NOT EXISTS / conditional ALTER on every single connection, with no PRAGMA user_version anywhere - so there was no supported path to change the on-disk shape in a release. And autosave overwrites the one and only copy every 30 seconds with no backup, so a corrupt write loses the user's entire chat history with nothing to restore from. 9.1 - graphlink_migrations.py adds an ordered migration runner for both SQLite and the plain-dict session.dat state. The SQLite runner applies steps in order inside one real transaction and bumps user_version only after every step succeeds. This needs manual BEGIN/COMMIT with isolation_level=None: Python's sqlite3 default implicitly commits before DDL and PRAGMA, so `with conn:` would leave a failed chain half-applied. chats.db's per-connection schema probing becomes one versioned migration that also adds the missing FK indexes on notes.chat_id/pins.chat_id, plus a busy_timeout. session.dat's scattered `if field not in state` backfills become an explicit chain, behavior-preserving. 9.2 - backend/db_backup.py snapshots chats.db via SQLite's online backup API rather than a file copy, since the live DB can be mid-write and a raw copy of a WAL database can be torn. Retention keeps the 10 most recent plus one per calendar day. On corruption the live file is quarantined to .corrupted-<ts> and the newest backup is restored in its place; if no backup exists the user is told so rather than being silently handed an empty library. Saves become UPDATE ... WHERE id = ? AND updated_at = ?, so a lost race raises instead of clobbering the other writer. Timestamps carry microseconds. At second resolution two saves inside the same wall-clock second share a token, which makes a stale value indistinguishable from a fresh one and silently defeats the concurrency check entirely. Restore deletes the corrupt file's -wal/-shm sidecars. They describe writes against the quarantined file's page layout; left in place, the next connection would try to replay them onto the restored database. Known limitation: a save against a chat another writer has deleted also reports "modified elsewhere", since both cases surface as rowcount 0. It refuses the write either way, so nothing is clobbered - only the wording is imprecise. Test plan: full pytest from repo root - 2114 passed, 16 skipped. The new tests pass with #302's real-data guard active, confirming they are tmp_path-isolated. Covers migration ordering and rollback-on-failure, upgrading a pre-existing v0 database with real rows in it, retention math, kill-9-mid-save recovery from a truncated database, and a two- session lost-write race. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
dovvnloading
changed the title
dovvnloading
changed the title
dovvnloading
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Local storage had no schema-migration mechanism, no backups, and no
recovery path from a corrupt database. Two saves of the same chat could
silently clobber each other. There was no way to get data out of the app,
and no single place deciding what counts as a secret.
Change
9.1 — schema migrations. A shared
run_sqlite_migrationsdriver keyedon
PRAGMA user_version, with ordered steps and a gap check. Both storesmove onto it:
chats.dbgets an explicit initial-schema step, and thesettings store's scattered backfills become an ordered chain. Migrations
run under a manual
BEGIN/COMMIT/ROLLBACK—sqlite3's legacytransaction control implicitly commits before DDL, so
with conn:is notatomic here.
9.2 — backups, corruption recovery, concurrent-save safety. Backups
via SQLite's online backup API, pruned to the ten most recent plus one
per day. A corrupt database is quarantined (including its
-wal/-shmsidecars, which would otherwise replay onto the restored file) and
restored from the newest good backup. Saves are optimistically
concurrency-checked on
updated_atand raiseConcurrentSaveConflictrather than overwriting, using microsecond timestamps — second resolution
defeats the check.
9.3 — secret scrubbing. One chokepoint,
backend/secret_scrub.py,matching both on key name and on credential-shaped values, plus absolute
paths in all three forms (Windows, UNC, POSIX).
9.4 — the
.graphlinkarchive. A plain zip: chats as JSON, assets asreal files, so it is readable without this app. Every payload is scrubbed
on the way out. Import validates before writing anything, refuses zip-slip
and absolute member names, caps member size against decompression bombs,
refuses newer format versions rather than guessing, and drops assets whose
bytes do not hash to their own ref.
What is not here
Stages 9.5 (asset externalization) and 9.6 (flat edge format) — the two
on-disk format changes. They ship separately.
Test plan
backend/tests/test_migrations.py,test_db_backup.py,test_chat_library.py— migration ordering and gap detection,transaction rollback, backup/prune/restore, quarantine including
sidecars, concurrent-save conflict.
backend/tests/test_secret_scrub.py(12) — written as attempts to sneaka secret past the scrubber, not as a happy-path walk.
backend/tests/test_workspace_archive.py(16) — round-trip into adifferent asset store (the second-machine case), zero secrets in the
archive, and the hostile-input set.