[advaitpaliwal]

Databases migrated from V1 never return freed space to disk. This is a small fix for an obvious bug, so I'm submitting it without a prior issue.

The bug

initializeV2Database creates statev2.sqlite with NodeSqlite.backup. A page-level backup keeps the V1 file's auto_vacuum = NONE. The check in Sqlite.ts that sets auto_vacuum = INCREMENTAL only runs on an empty database, so it never runs on a migrated one. Because of that, the existing incremental_vacuum loop in DatabaseStorage does nothing on every install that migrated from V1. Space freed later, for example by migration 063 dropping the V1 projections or by history compaction, stays in the file.

Evidence

I saw this on a macOS install running 0.1.2:

• statev2.sqlite had grown to 14.4 GB, with 2,111,248 of its 3,526,806 pages (60%) on the freelist.
• PRAGMA auto_vacuum returned 0.
• The server stalled for up to 18 s at a time, and T3 Connect showed "reconnecting".
• After VACUUM INTO, the file was 5.6 GB. After t3 doctor --fix, auto_vacuum was 2.

The fix

The import now runs PRAGMA auto_vacuum = INCREMENTAL followed by VACUUM INTO on a read-only connection to state.sqlite. This happens in a worker thread, so the event loop stays free. The copy comes out compacted and already INCREMENTAL, so the existing reclaim loop works from the first start.

• VACUUM INTO can renumber rowids. That is safe here because nothing in the V1 tables depends on rowids, and the message search index that is keyed on them is created later, by migration 69.
• Databases that were already migrated as NONE still need t3 doctor. This PR doesn't change that design.

Verification

• Regression test: I extended "includes committed WAL data" in initializeV2Database.test.ts to assert that the copy has auto_vacuum = 2 and freelist_count = 0. It fails on main (auto_vacuum: 0) and passes with this change.
• Cost, measured on a 1 GB V1 database: VACUUM INTO took 7.8 s, against 4.5 s for backup. It runs once and off the event loop. The output was 414 MB instead of 1,034 MB, and integrity_check returned ok.
• Not checked: a real V1 → V2 upgrade of a multi-GB database through the desktop app.

[macroscopeapp]

Approvability

Verdict: Approved at bd21af0

Macroscope's review found this PR approvable — First-time V1 imports now create compacted copies that support the existing disk-space reclamation mechanism, with copying moved off the event loop. The original database remains read-only, and existing V2 databases are not replaced.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Snapshot creation now runs in a worker thread using SQLite VACUUM INTO after setting incremental auto-vacuum. The test checks that committed data is copied and that the snapshot has the expected auto-vacuum mode and freelist count.

Changes: SQLite snapshot creation

Layer / File(s)	Summary
Create the snapshot in the worker apps/server/src/persistence/initializeV2Database.ts	The worker opens the legacy database read-only, sets auto_vacuum to INCREMENTAL, creates the snapshot with VACUUM INTO, and closes the database.
Handle worker completion and verify snapshot apps/server/src/persistence/initializeV2Database.ts, apps/server/src/persistence/initializeV2Database.test.ts	The caller terminates the worker on abort and rejects on worker errors or nonzero exit. The test adds freed pages to the fixture and checks committed data, auto_vacuum, and freelist_count.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant initializeV2Database
  participant Worker
  participant LegacyDatabase
  participant SnapshotFile
  initializeV2Database->>Worker: Start with source and snapshot paths
  Worker->>LegacyDatabase: Open read-only
  Worker->>LegacyDatabase: Set auto_vacuum to INCREMENTAL
  Worker->>SnapshotFile: Write snapshot with VACUUM INTO
  Worker->>LegacyDatabase: Close database
  Worker-->>initializeV2Database: Exit status

Loading

Possibly related issues

• [Bug]: Claude workflow progress re-stores the whole workflow.agents array on every subagent.updated (1.4 GB for one 486-agent workflow, never compacted) #27598: This issue reports unreclaimed SQLite space when auto_vacuum is disabled. This PR sets incremental auto-vacuum on the imported snapshot, but does not address event payload growth or databases already migrated with auto-vacuum disabled.

Possibly related PRs

• pingdotgg/t3code#29811: Both changes enable incremental auto-vacuum through different database-opening paths.
• pingdotgg/t3code#31217: Both changes update initializeV2Database snapshot creation and preserve typed error handling.

Suggested reviewers: maria-rcks

Merge Risk: Minimal · up to 68e95

The snapshot migration can be merged after normal checks; cancellation cleanup now waits for the worker.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: allowing migrated databases to return freed space to disk.
Description check		The description includes the required Problem, Change, Scope and approval rationale, and Verification sections. It explains the bug, implementation, test results, measured performance, and remaining v…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description includes the required Problem, Change, Scope and approval rationale, and Verification sections. It explains the bug, implementation, test results, measured performance, and remaining verification limits.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Usage-based review receipt

• Mode: Continue automatically
• Reviewed files: 2
• Charged: $0.50
• View usage details

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 67eebac
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: c640df53-418c-4901-bf0d-7d70ded0bbf1

Commits

Reviewing files that changed from the base of the PR and between bd21af0 and 68e959c.

Files selected for processing: 2

• apps/server/src/persistence/initializeV2Database.test.ts
• apps/server/src/persistence/initializeV2Database.ts

Limit details: You’ve used all 10 included reviews currently available.

[coderabbitai]

• Fix comments: Fix CodeRabbit comments on this PR

[maria-rcks]

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head bd21af04329eb260e62b154147843ccd02c2acf4 on a linux boat vm with the real web client, playwright from this repo, /usr/bin/google-chrome, and codex gpt-6.1-sol through the local provider proxy.

created a genuine v1 database using the last v1 app (024d49520e): added a real git project, started a thread, completed an acknowledgment turn and a read-only repository-inventory turn. stopped that server and imported identical copies through normal startup on github/main (cefb544af3) and this pr. no fixture sql, mocked provider, manual vacuum, or accelerated maintenance timers.

read-only queries against each running server database showed:

                         auto_vacuum  page_count  freelist_count  integrity_check
v1 source                    0            606          0             ok
main after import            0            746        256             ok
main after maintenance       0            746        524             ok
pr after import              2            747        255             ok
pr after maintenance         2            247          0             ok

the pr returned 500 pages (2,048,000 bytes) through normal background maintenance. main retained its freed pages. the source v1 database and both input copies retained the same sha256 throughout.

nearby behavior also passed: both clients displayed the migrated messages; the pr completed a real continuation (PR_CONTINUE_OK); after a server restart with both v1 and v2 files present, that response persisted, the v2 inode was unchanged, and another real codex turn completed (PR_RESTART_OK). sqlite integrity remained ok.

blacksmith passed all 6 focused import/storage tests and vp run check:changed cefb544af3c98224874419198c0359b652ca3c94 (format, lint, knip, graphql, affected typechecks and tests). all 31 reported ci checks were successful at inspection. no source changes or pushes were needed.

command transcript

Not verified

macos/windows desktop, native ios/android, multi-gb imports, worker cancellation on windows, and simulated crash/corruption scenarios were not exercised on this linux vm. the existing windows cancellation review finding remains outside this verification result.

Takeover update (68e959c226, written by claude-opus-5-5)

The verification above covers bd21af0432.

• Merged current main (merge commit, no rebase). The conflict was with refactor: Effect.try calls map failures to typed errors #31217's typed catch; the worker copy now fails with V2DatabaseImportError like main's backup did.
• Addressed CodeRabbit's finding: interruption now waits for the snapshot worker to exit before the scope removes the temp directory (68e959c).
• Checks: vp run check:changed passes (format, lint, knip, typecheck, initializeV2Database.test.ts).
• Runtime check on a 1.5 GB V1 file: interrupting mid-copy left only state.sqlite (temp dir removed); a full run then produced statev2.sqlite with auto_vacuum = 2 and all rows.