Snapshot live SQLite stores before querying

[jmcte]

Summary

• snapshot live SQLite databases with present WAL/SHM companions into private temporary storage before querying
• enforce read-only/query-only execution, bounded busy/process timeouts, structured failures, and deterministic cleanup
• migrate Messages and Safari history first and document the remaining provider migration queue

Governing Issue

Closes #90

Validation

• Relevant local checks passed
• Required PR checks are expected to satisfy CI Gate
• Skipped checks are explained below

swift test --disable-sandbox — 113 tests passed

bash scripts/ci/run-fast-checks.sh — passed; source line coverage 88.95%, mutation checks killed expected mutations, debug and release builds passed

WAL-backed rows, missing companions, query-only rejection, locked/busy stores, timeout, copy permission failure, schema drift, and cleanup are covered with synthetic tests.

Bootstrap Governance

• Changes are scoped to the linked issue
• Contributor or PR guidance changes are reflected in CONTRIBUTING.md, .github/PULL_REQUEST_TEMPLATE.md, and docs/bootstrap/onboarding.md when applicable
• PR author enabled auto-merge where GitHub allows it, or GitHub plan-limit evidence/unavailable reason is recorded and the fallback merge-readiness policy applies
• No real secrets, runtime auth, or machine-local env files are committed

This branch is stacked on provider-manifest PR #101. The PR remains draft until #101 merges and the diff collapses to the issue #90 commit; auto-merge will then be armed.

Flow Contract

• Owner lane: Hermes macOS/native storage safety
• Repair owner: jmcte
• Autonomy class: review-gated implementation from assigned issue
• Risk class: security; high-sensitivity private-store copy and process-lifecycle behavior

Flow Merge Readiness

• Every blocker has a next actor and next action
• No active blocking requested changes remain
• Non-author approval is present when required
• PR author enabled auto-merge where GitHub allows it, or recorded why it is unavailable/unsafe

Next actor: #101 reviewers, then security/architecture/macOS reviewers on this PR.

Merge Automation

• PR author enabled auto-merge with gh pr merge --auto --squash, or the reason it is unavailable/unsafe is noted below

Auto-merge is intentionally deferred while the PR is draft and stacked on #101.

Notes

• Private copies and query output use restrictive permissions and are deleted before return or error.
• The source database is never checkpointed, locked, vacuumed, or modified.

[jmcte]

@athena-omt @pheidon @ares-omt @Hermes-omt — this security-sensitive SQLite snapshot PR is stacked on #101 and intentionally draft until that dependency merges. Early review is welcome on WAL consistency, private temporary storage, query-only enforcement, timeout/termination behavior, cleanup, and source-path error mapping.

[Hermes-omt]

Review Summary

Core design is sound — 113 tests pass, CI green. However, four edge cases should be addressed:

1. No production timeout floor (Medium)

SQLiteSnapshotQueryEngine accepts timeout down to 0.001s and busyTimeoutMilliseconds can be zero. For production SQL providers under concurrent iCloud access, these values are too aggressive.
Action: Add a factory wrapper or static builder that enforces minimums for production use (e.g., timeout ≥ 5s, busyTimeout ≥ 500ms), or document the constraint clearly in sqlite-snapshots.md.

2. TerminationHandler race (Low)

The termination handler signals a semaphore but doesn't guard against process.terminate() racing with early completion. On some macOS versions terminate can fail silently.
Action: Check process.isRunning before calling terminate, or handle the error path explicitly in the timeout case.

3. SQLite -readonly WAL mode constraint (Low)

Pre-3.14 sqlite3 had a known issue with -readonly and WAL databases. macOS Monterey+ is safe, but document as a minimum version requirement.
Action: Add a note to sqlite-snapshots.md noting the minimum sqlite3 version (≥ 3.14) or confirm via runtime check.

4. Missing .tmp journal cleanup (Low)

If an external sqlite3 process crashes in /tmp before our snapshot dir is created, the orphaned .tmp file persists outside our cleanup scope.
Action: Very low risk; optionally note in docs that concurrent sqlite3 access to the same temp root requires coordination.

CI Status

• ✅ CI Gate: success
• ✅ PR Checks: success
• ⚠️ No non-author approvals yet (this is the first review)

Overall: Solid engineering. These are refinements, not blockers — but I'm requesting changes so the author can address them before auto-merge fires.

[Hermes-omt]

Request changes

SQLiteSnapshotQueryEngine.withSnapshot uses FileManager.copyItem(at: source, ...), which preserves symbolic links on macOS. Both migrated call paths accept caller-supplied --chat-db and --history-db; when either is a symlink, snapshot.sqlite remains a link to the live database. Companion lookup also uses the link pathname, so it can omit the target store's -wal/-shm files. This breaks the private, consistent-snapshot guarantee for a supported input.

Resolve the source with resolvingSymlinksInPath() before existence checks, copying, and companion derivation (while retaining the original path for reportedStore), or explicitly reject symlink inputs. Add a fixture test proving a symlinked WAL-backed source produces a regular snapshot and reads the target's committed WAL row.

Validated on the exact head: swift test --disable-sandbox passed 117 tests; GitHub PR Fast CI is successful.

[jmcte]

Addressed the current-head snapshot review in bf3bee6.

• Snapshot creation now resolves a store symlink before checking, copying, and locating its -wal/-shm companions.
• The original user-supplied path remains the reported store, so diagnostics do not expose a resolved private path unexpectedly.
• Added a regression that snapshots a symlinked WAL-mode store, verifies the copied database is not a symlink, verifies its WAL companion is present, and reads the committed WAL row.

Validation passed:

• swift test (118 tests)
• bash scripts/ci/run-fast-checks.sh
• git diff --check

[Hermes-omt]

Reviewed current head bf3bee6 on Hermes. The symlink/WAL snapshot repair resolves the prior blocking concern while preserving the original diagnostic path. Direct call paths now snapshot Messages and Safari History live stores; the new regression covers a symlinked WAL-backed store. git diff --check, targeted snapshot tests, and full swift test --disable-sandbox (118 tests) pass. Live PR checks are successful and auto-merge is enabled.

[athena-omt]

Request changes: withSnapshot copies snapshot.sqlite first and then copies -wal/-shm independently (SQLiteSnapshotQuery.swift:134-139), without establishing a read transaction or otherwise pinning the source. A live SQLite writer can checkpoint/truncate or append the WAL between those operations. The resulting private files can therefore combine different database generations: callers may get stale/missing committed rows or an invalid snapshot even though the source was valid. This violates the PR’s consistent-snapshot contract for the exact workload it targets. Please obtain the snapshot through SQLite’s backup API (or another SQLite-coordinated read transaction) and add a concurrent writer/checkpoint regression; copying static WAL fixtures alone does not exercise this race.\n\nEvidence: reviewed exact head bf3bee6; git diff --check clean; live PR checks SUCCESS. I could not run Swift tests on this Linux NAS because swift is not installed.

[Hermes-omt]

Reviewed exact head abcb2fb362e9435dd77229f71ee748e910e94771 on Hermes. The coherent VACUUM INTO snapshot implementation, symlink handling, production timeout-floor adoption, real inventory call path, and adjacent regressions are sound. git diff --check, CI policy/shell/privacy/secret checks, targeted snapshot tests, and full swift test --disable-sandbox passed locally on macOS. However, the fresh run for this exact SHA has PR Checks and dependent CI Gate failed. Public job-log access returns 403, so the failing step cannot be attributed from this isolated review lane. Please have the CI owner inspect run 30157825771 with authorized logs, repair or rerun the failing check, then request fresh review.

[athena-omt]

Exact-head re-review at abcb2fb362e9435dd77229f71ee748e910e94771 — APPROVE.

Prior blocker resolved. withSnapshot no longer copies the main db and its -wal/-shm files independently; it now creates the snapshot through SQLite with a single coordinated VACUUM INTO (SQLiteSnapshotQuery.createSQLiteSnapshot), which reads the source within one SQLite transaction and yields a self-contained, consistent snapshot — eliminating the cross-generation race that was flagged.

Verified on exact head:

• SQLiteSnapshotQuery.swift: createSQLiteSnapshot runs sqlite3 <source> "VACUUM INTO '...';" with a busy timeout, single-quote escaping, 0600 perms on the snapshot, terminate/kill timeout handling, and permission-failure mapping.
• LocalInventories.withSnapshot now uses SQLiteSnapshotQueryEngine.production (5s timeout / 500ms busy floors).
• New regression snapshotQueryFollowsSymlinkedStoreWithCoherentWALSnapshot holds a live concurrent WAL writer (wal_autocheckpoint=0) during snapshot creation and asserts a coherent WAL snapshot through a symlink — the requested race coverage.
• Required checks green on exact head (CI Gate, PR Checks). git diff --check clean.

The earlier CHANGES_REQUESTED review was against stale head bf3bee61; superseded by this approval.

[jmcte]

Hermes reconciliation — new head (review requested)

New head: 01c23902cb770a548bddc643a1be5e0f72231a9d (merge commit; parents 1589c65 (prior PR head) + 3469c2d (origin/main after #110/#107)) · pushed fast-forward, no force · author/committer Hermes <hermes@openclaw.local> (isolated agent identity).

What was reconciled

3 content conflicts, all in the SQLite-snapshot surface; resolved in favor of the VACUUM INTO snapshot repair:

• SQLiteSnapshotQuery.swift — kept resolvedSource (symlink resolution) feeding VACUUM INTO, so live/symlinked WAL stores snapshot coherently; production timeout floors (500ms busy / 5s process) retained.
• SQLiteSnapshotQueryTests.swift — kept both the PR symlink-coherent-WAL test and main’s single-file-WAL test, plus the production timeout-floor test (braces repaired after keep-both).
• docs/sqlite-snapshots.md — kept production timeout-minimum wording.

macOS-native verification (on Hermes)

• swift build → clean (Swift 6.3.3 / Xcode 26.6, arm64-apple-macosx26.0)
• swift test --filter SQLiteSnapshot → 10/10 passed (incl. snapshotQueryFollowsSymlinkedStoreWithCoherentWALSnapshot, snapshotQueryCreatesSingleFileSnapshotForLiveWALStore, productionSnapshotEngineEnforcesTimeoutFloors)

Head changed, so requesting fresh review. Per policy: not merging.

[Hermes-omt]

Reviewed exact head 01c23902cb770a548bddc643a1be5e0f72231a9d on Hermes (macOS, JMCTE-Macbook.local, Darwin). Approve.

• Freshness: refs/pull/102/head = 01c23902; head is the merge commit (parents 1589c656 + 3469c2d8) and origin/main (3469c2d8) is a full ancestor — fast-forward push, no force.
• The main reconciliation kept the VACUUM INTO snapshot repair; no conflict markers; git diff --check clean.
• Source verified at exact head: withSnapshot resolves symlinks via resolvingSymlinksInPath() before existence checks and snapshot creation; the snapshot is created with a single coordinated VACUUM INTO (source is read-only, never checkpointed/mutated); snapshot is mode 0600 in a 0700 temp dir; queries run -readonly with PRAGMA query_only=ON and a bounded busy timeout; timeout path escalates terminate → SIGKILL; all temp artifacts are removed before return/throw.
• LocalSQLiteInventoryReader.withSnapshot now uses SQLiteSnapshotQueryEngine.production, enforcing 5s process / 500ms busy floors; the remaining direct engine use (LocalInventories.swift:664) only queries the already-private snapshot, not a live store. Docs match the actual contract.
• Tests: full swift test --disable-sandbox on exact head on macOS — 166/166 passed (Swift 6.3.3, arm64-apple-macosx26.0), including snapshotQueryFollowsSymlinkedStoreWithCoherentWALSnapshot, snapshotQueryCreatesSingleFileSnapshotForLiveWALStore, and productionSnapshotEngineEnforcesTimeoutFloors.
• CI on exact head: CI Gate success, PR Checks success, Trusted Checks skipped. Auto-merge (squash) is enabled and no review threads are unresolved; prior symlink/WAL-race/timeout-floor change requests are all resolved in code.

No blockers.

[athena-omt]

Athena exact-head review at 01c23902cb770a548bddc643a1be5e0f72231a9d — APPROVE.

Surface: 4 changed files (+59/−4): SQLiteSnapshotQuery.swift, LocalInventories.swift, SQLiteSnapshotQueryTests.swift, docs/sqlite-snapshots.md. Full diff and real call paths reviewed in a fresh clone of codex/issue-90-sqlite-snapshots at exact head; head is a merge commit whose second parent is main @ 3469c2d8 — current with base, no force.

Prior Athena blocker is resolved. My REQUEST_CHANGES at bf3bee61f8 was that withSnapshot copied snapshot.sqlite and then copied -wal/-shm independently without a pinned read view, so a live writer could produce a mixed-generation snapshot. At this head the snapshot is materialized through one coordinated SQLite VACUUM INTO (SQLiteSnapshotQuery.swift:136-145): SQLite owns the read view, the Apple-owned source is never checkpointed or mutated, and the result is a single coherent private snapshot file.

Verified at exact head:

• Symlink handling: source.resolvingSymlinksInPath() precedes both the existence check and snapshot creation (SQLiteSnapshotQuery.swift:108-110,127); reportedStore keeps the caller-supplied path, so diagnostics do not expose resolved private paths.
• Timeout floors: SQLiteSnapshotQueryEngine.production enforces max(5, timeout) / max(500, busyTimeoutMilliseconds) (SQLiteSnapshotQuery.swift:25-39), and LocalSQLiteInventoryReader.withSnapshot — the live inventory path — now uses it (LocalInventories.swift:699). The remaining raw-engine use at LocalInventories.swift:664 only calls querySnapshot against the already-private snapshot, not a live store.
• Query confinement: snapshot is mode 0600 in a mode-0700 temp dir; queries run /usr/bin/sqlite3 -readonly with PRAGMA query_only=ON and a bounded busy timeout; the process timeout escalates terminate → SIGKILL; temp dir, query output, and error files are removed on return and throw.
• Tests: productionSnapshotEngineEnforcesTimeoutFloors pins the floors; snapshotQueryFollowsSymlinkedStoreWithCoherentWALSnapshot covers a symlinked WAL-backed live store and asserts a single-file coherent snapshot.

Checks on exact head: CI Gate success, PR Checks success, Trusted Checks skipped. Local swift test was not run from the Athena node (Linux host, no Swift toolchain); this approval relies on the green required checks plus Hermes' full local run (166/166 on macOS at this exact head).

Residual risk (minor, non-blocking): the public initializer still permits sub-floor timeouts by design for tests; MessagesArchive.swift uses the raw initializer, but its defaults (10s / 1000ms) already meet the floors. A later cleanup could migrate it to .production for consistency; nothing in this PR's diff regresses that path.

Not merging — leaving merge to the owner.