Stop the test suite from reading and rewriting real user data - #302
Merged
Conversation
Several test files constructed the app with create_app() but without settings_state_file= or chat_db_path=, so those fell through to the production defaults - ~/.graphlink/session.dat and ~/.graphlink/chats.db. Every pytest run therefore read and rewrote the live settings and chat history of whoever ran the suite. Confirmed live, not theoretical: running test_auth.py alone changes the real session.dat's mtime. It was benign only by luck - create_app()'s bootstrap_provider_state() reads the persisted mode and immediately writes that same value back, so the bytes happened to match. Its except branch writes MODE_OLLAMA_LOCAL unconditionally, which would have silently overwritten a real configured mode. Fixes the five offending call sites (four helpers plus one test that builds its own TestClient) to pass tmp_path/TemporaryDirectory-derived overrides, matching the convention test_assets.py and test_app_ws.py already follow. Fixing the instances is not enough on its own - this regressed silently once and would again, since nothing made a bare create_app() wrong at the point of use. backend/tests/conftest.py now carries an autouse guard that raises the moment any test resolves a path under the real ~/.graphlink, naming the offending path, and test_real_data_guard.py proves the guard actually fires on each access path rather than passing vacuously. Scope note: the guard covers backend/tests/ (where every offender was and where app-construction tests live). tests/ at the repo root was verified clean by an equivalent one-off run but has no conftest of its own. Test plan: full pytest from repo root with the guard active - 2045 passed, 14 skipped. Each of the four fixed files also verified in isolation with a before/after mtime check on the real session.dat and chats.db: neither changes. Before the fix, the same check showed session.dat changing on every run.
dovvnloading
added a commit
that referenced
this pull request
Aug 9, 2026
…covery (#303) 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>
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
Several test files constructed the app with
create_app()but withoutsettings_state_file=orchat_db_path=, so those fell through to the production defaults —~/.graphlink/session.datand~/.graphlink/chats.db. Everypytestrun therefore read and rewrote the live settings and chat history of whoever ran the suite.Confirmed live, not theoretical: running
test_auth.pyalone changes the realsession.dat's mtime.It was benign only by luck.
create_app()'sbootstrap_provider_state()reads the persisted mode and immediately writes that same value back, so the bytes happened to match. Itsexceptbranch writesMODE_OLLAMA_LOCALunconditionally — on a config whose persisted mode failed validation, that would have silently overwritten a real configured mode.Change
Five call sites now pass
tmp_path/TemporaryDirectory-derived overrides, matching the conventiontest_assets.pyandtest_app_ws.pyalready follow:test_auth.pyauthed_clientfixture + 3 directcreate_app()callstest_http_trust_boundary.py_make_client()helper (~15 callers)test_security_invariants.py_make_client()helper (~9 callers)test_session_lifecycle.py_make_client()helpertest_http_trust_boundary.py:280TestClient, bypassing the helperFixing the instances isn't enough on its own — this regressed silently once and would again, because nothing made a bare
create_app()wrong at the point of use. Sobackend/tests/conftest.pygains an autouse guard that raises the moment any test resolves a path under the real~/.graphlink, naming the offending path.test_real_data_guard.pyproves the guard actually fires on each access path (SettingsManagerexplicit,SettingsManagervia default arg,chat_library._connect, barecreate_app()) and still permits correcttmp_pathusage — a guard that passed vacuously would be worse than none.Scope note: the guard covers
backend/tests/, where every offender was and where the app-construction tests live.tests/at the repo root was verified clean by an equivalent one-off instrumented run but has no conftest of its own.Test plan
pytest -qfrom repo root with the guard active: 2045 passed, 14 skipped.session.datandchats.db: neither changes. Before the fix, the same check showedsession.datchanging on every run.chats.dbintegrityok, 41 chats,user_versionunchanged;session.datparses,schema_version4, stored API key still DPAPI-encrypted.