move to using a global database by Broderick-Westrope · Pull Request #20 · Broderick-Westrope/anvil

Broderick-Westrope · 2026-06-02T11:41:40Z

PR Type

Enhancement

Description

Migrate from per-project SQLite databases to a single global database
Add working_dir column to sessions for project association
Implement migration engine with sync and batched modes using ATTACH DATABASE
Thread working_dir through session service, workspace, backend, and UI layers
Add --there, --session/-s, --skip-migration, and --all CLI flags
Rename DataDirectory to ProjectDirectory; remove dead queries

Diagram Walkthrough

flowchart LR
  PerProject["Per-project SQLite DBs"]
  MigEngine["Migration Engine"]
  GlobalDB["Global SQLite DB (~/.local/share/anvil/anvil.db)"]
  WorkDir["working_dir column"]
  CLI["CLI flags (--there, --skip-migration)"]
  UI["TUI sessions dialog"]
  PerProject -- "ATTACH DATABASE" --> MigEngine
  MigEngine -- "sync/batched copy" --> GlobalDB
  GlobalDB -- "scoped queries" --> WorkDir
  WorkDir -- "filter sessions" --> UI
  CLI -- "resolve session dir" --> GlobalDB

File Walkthrough

Relevant files

Enhancement

14 files

migrate.go `Migration engine with sync and batched modes via ATTACH DATABASE`	+534/-0
migrate_startup.go `Startup migration helpers for current and all projects`	+91/-0
db.go `Update prepared statements for renamed and new queries`	+134/-134
sessions.sql.go `Add working_dir to session queries and new list/get variants`	+93/-12
files.sql.go `Replace dead queries with session-scoped ListSessionFilesByPath`	+9/-85
messages.sql.go `Add ListUserMessagesByWorkingDir query`	+18/-17
models.go `Add WorkingDir field to Session model`	+6/-0
connect.go `Add ConnectGlobal/ReleaseGlobal with shared connection pool`	+41/-5
20260601000000_add_working_dir.sql `Migration adding working_dir column and migrations_completed table`	+16/-0
session.go `Thread working_dir through session service interface and methods`	+42/-10
config.go `Rename DataDirectory to ProjectDirectory with deprecation alias`	+8/-7
load.go `Update config loading for ProjectDirectory rename`	+21/-9
root.go `Add --there, --skip-migration flags and global DB setup`	+105/-9
session.go `Add --all flag to session list; filter by working dir`	+20/-11

Tests

6 files

migrate_test.go `Comprehensive tests for project DB migration scenarios`	+450/-0
connect_test.go `Tests for global database connection management`	+62/-0
session_test.go `Tests for working_dir persistence, filtering, and inheritance`	+138/-2
load_test.go `Update tests for DataDirectory to ProjectDirectory rename`	+18/-18
agent_test.go `Pass working_dir to session Create calls in tests`	+16/-16
coordinator_test.go `Pass working_dir to session Create calls in coordinator tests`	+12/-12

Additional files

52 files

AGENTS.md	+6/-2
scratchpad.md	+2/-1
agentic_fetch_tool.go	+1/-1
coordinator.go	+1/-1
coordinator_providers.go	+1/-1
anvil_info.go	+1/-1
anvil_info_test.go	+2/-2
multiedit_test.go	+0/-4
app.go	+5/-4
resolve_session_test.go	+23/-12
backend.go	+4/-4
session.go	+7/-6
proto.go	+13/-5
logs.go	+1/-1
mcp.go	+3/-3
run.go	+4/-4
stats.go	+5/-11
index.html	+1/-1
commands.go	+1/-1
commands_test.go	+2/-2
store.go	+3/-3
querier.go	+6/-6
files.sql	+2/-18
messages.sql	+6/-5
sessions.sql	+19/-3
service.go	+11/-17
service_test.go	+3/-2
file.go	+4/-14
message.go	+3/-3
message_test.go	+4/-4
proto.go	+4/-2
SKILL.md	+1/-1
docs.go	+2/-2
swagger.json	+2/-2
swagger.yaml	+3/-3
branch.go	+0/-2
sessions.go	+34/-2
tree.go	+10/-12
diffview_test.go	+0/-2
history.go	+1/-1
ui.go	+1/-1
app_workspace.go	+11/-5
client_workspace.go	+4/-4
workspace.go	+2/-2
design-2025-05-30-global-database.md	+100/-17
README.md	+60/-0
phase-1-schema-db-cleanup.md	+113/-0
phase-2-query-scoping.md	+125/-0
phase-3-interface-callsites.md	+130/-0
phase-4-migration-engine.md	+95/-0
phase-5-cli-tui-stats.md	+111/-0
schema.json	+1/-1

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Replace per-project SQLite databases with a single global database at ~/.local/share/anvil/anvil.db. Sessions are now discoverable from any directory, MCP OAuth tokens are shared across projects, and cross-project search becomes possible. Key changes: - Add working_dir column to sessions for project association - Add ConnectGlobal/ReleaseGlobal with shared connection pool - Migration engine with sync (trigger-drop) and batched (trigger-tolerant) modes, OAuth newest-wins conflict resolution, ATTACH DATABASE for cross-DB copy - Thread working_dir through 11 interface layers (session.Service -> Workspace -> Backend -> HTTP -> UI) - Add --session/-s, --there, --skip-migration CLI flags - TUI sessions dialog defaults to current directory with ctrl+a toggle - Stats show global totals - Rename DataDirectory to ProjectDirectory - Remove dead ListLatestSessionFiles/ListNewFiles queries - Switch filetracker to absolute paths Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

- Document OAuth token concurrent-migration race condition as accepted behavior in migrate.go - Improve --there error message when working directory no longer exists to suggest alternatives (--cwd or omitting --there) Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

- Add Windows absolute path detection (drive letter patterns like C:\ and C:/) alongside Unix (/) in read_files relative-to-absolute conversion - Update AGENTS.md persistence section to reflect global database architecture Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

- Use context.Background() for DETACH DATABASE in defer to prevent connection pool pollution when parent context is canceled - Add UNC path detection (\server\share) in read_files migration - Add backwards compatibility for deprecated data_directory config key; automatically migrates to project_directory during config loading Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

github-actions · 2026-06-02T11:43:16Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Infinite Loop Risk In `copyInBatches`, when using `INSERT OR IGNORE` with `LIMIT ? OFFSET ?`, if all rows in a batch are duplicates (already exist), `RowsAffected()` returns 0 even though there are more rows to process. This causes the loop to break prematurely, leaving uncopied rows. For example, if the first batch of 100 rows are all duplicates but rows 101+ are new, they will never be copied. Conversely, this is not an infinite loop but a silent data loss issue during batched migration. func copyInBatches(ctx context.Context, conn sql.Conn, query string, batchSize int, table string) error { offset := 0 for { if err := ctx.Err(); err != nil { return err } result, err := conn.ExecContext(ctx, query, batchSize, offset) if err != nil { return fmt.Errorf("failed to copy %s at offset %d: %w", table, offset, err) } affected, err := result.RowsAffected() if err != nil { return fmt.Errorf("failed to get rows affected for %s: %w", table, err) } if affected == 0 { break } offset += batchSize } return nil SQL Injection* In `migrateSynchronous`, trigger names are interpolated into SQL via `fmt.Sprintf("DROP TRIGGER IF EXISTS %s", t)` without parameterization. While the trigger names are hardcoded string literals in this function, this pattern is fragile — if the `triggers` slice is ever populated from external input, it becomes a SQL injection vector. Consider using quoted identifiers or keeping the full DDL statements as constants. for _, t := range triggers { if _, err := tx.ExecContext(ctx, fmt.Sprintf("DROP TRIGGER IF EXISTS %s", t)); err != nil { return fmt.Errorf("failed to drop trigger %s: %w", t, err) } } Resource Leak In `resolveThereSession`, `db.ConnectGlobal` is called and then `db.ReleaseGlobal()` is deferred. Later in `setupLocalWorkspace`, `db.ConnectGlobal` is called again. If `ConnectGlobal` uses a singleton/pool pattern, the `ReleaseGlobal` in `resolveThereSession` may close the connection before `setupLocalWorkspace` uses it. If it opens a new connection each time, the first release is fine but the pattern is confusing. Verify that the early connect-then-release doesn't interfere with the subsequent connect in the normal startup path. conn, err := db.ConnectGlobal(ctx) if err != nil { return session.Session{}, err } defer func() { _ = db.ReleaseGlobal() }()

github-actions · 2026-06-02T11:44:13Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Session not loaded after --there resolution When `--there` is used, `sessionID` is set to the resolved session's ID and `continueLast` is set to false. However, the condition `sessionID != "" && !there` skips the `resolveWorkspaceSessionID` block when `--there` was used. This means the session is never loaded into the workspace's session context via `ws.GetSession`. The `there` flag variable is a local bool, but after the `--there` block, the code should still call `ws.GetSession` with the resolved `sessionID`. The condition should be `sessionID != ""` without the `!there` guard, since the session still needs to be fetched from the workspace. internal/cmd/root.go [143] - if sessionID != "" && !there { + if sessionID != "" { Suggestion importance[1-10]: 8 __ Why: This is a significant logic bug. When `--there` is used, `sessionID` is set and `there` remains `true`, so the condition `sessionID != "" && !there` evaluates to `false`, skipping the `resolveWorkspaceSessionID` call. This means the session won't be properly resolved within the workspace context. The session was resolved against the global DB in `resolveThereSession`, but the workspace-level resolution (which may handle hash prefixes differently or set up workspace state) is skipped entirely. Removing the `!there` guard would fix this.	Medium
	Batch loop may stop prematurely on duplicates The batch loop breaks when `RowsAffected() == 0`, but with `INSERT OR IGNORE`, rows that already exist are silently ignored and not counted as affected. If a batch contains only duplicates (e.g., from a partial prior migration), `affected` will be 0 and the loop terminates prematurely, skipping remaining un-migrated rows at higher offsets. You should count the source rows to determine when to stop, or always advance the offset until the SELECT returns fewer rows than the batch size. internal/db/migrate.go [393-402] affected, err := result.RowsAffected() if err != nil { return fmt.Errorf("failed to get rows affected for %s: %w", table, err) } - if affected == 0 { + // Always advance offset. With INSERT OR IGNORE, affected may be 0 + // if all rows in this batch were duplicates, but there may still be + // new rows at higher offsets. Stop only when the source SELECT + // returned fewer rows than batchSize (i.e., we've exhausted the source). + // Since we can't distinguish "0 source rows" from "all duplicates" + // using RowsAffected alone, we need a separate count query. + _ = affected + var sourceCount int64 + countRow := conn.QueryRowContext(ctx, fmt.Sprintf("SELECT COUNT(*) FROM source.%s WHERE rowid > ? AND rowid <= ?", table), offset, offset+batchSize) + if err := countRow.Scan(&sourceCount); err != nil \|\| sourceCount == 0 { break } offset += batchSize Suggestion importance[1-10]: 7 __ Why: The concern is valid — `INSERT OR IGNORE` with `RowsAffected() == 0` could cause premature termination if an entire batch consists of duplicates from a partial prior migration. However, the `improved_code` is problematic: it uses `fmt.Sprintf` to inject a table name into SQL (potential injection risk), and the `rowid > ? AND rowid <= ?` logic doesn't match the `LIMIT ? OFFSET ?` pattern used in the query. A better fix would be to check if the SELECT returned fewer rows than `batchSize`. Despite the flawed fix, the identified issue is real and could cause data loss in edge cases.	Medium
	Missing file existence check before migration `MigrateCurrentProject` does not check whether the source database file actually exists before attempting migration. If the project directory has no `anvil.db` (e.g., a new project), `MigrateProjectDB` will likely fail trying to open a non-existent file. Add an existence check after computing `sourcePath` to return early if the file doesn't exist. internal/db/migrate_startup.go [19-21] sourcePath := filepath.Join(projectDir, "anvil.db") + + if _, err := os.Stat(sourcePath); os.IsNotExist(err) { + return nil + } migrated, err := IsMigrated(ctx, globalDB, sourcePath) Suggestion importance[1-10]: 7 __ Why: The suggestion is valid — `MigrateCurrentProject` computes `sourcePath` and immediately calls `IsMigrated` and then `MigrateProjectDB` without checking if the source file exists. For new projects without a legacy `.anvil/anvil.db`, this would cause unnecessary errors. The companion function `MigrateAllProjects` (line 62) also doesn't check file existence before calling `IsMigrated`. However, `IsMigrated` only checks the `migrations_completed` table in the global DB, so it wouldn't fail — the failure would come from `MigrateProjectDB` trying to open a non-existent source DB. This is a real defensive programming improvement.	Medium
General	Skip already-migrated databases early The function does not check `IsMigrated` before performing the migration. While `INSERT` `OR IGNORE` prevents duplicate rows, the entire migration (ATTACH, goose migrations on source, all the INSERT/UPDATE queries) still runs unnecessarily on every call. The `IsMigrated` function exists but is never called inside `MigrateProjectDB`, wasting significant I/O for already-migrated databases. internal/db/migrate.go [89-93] + // Check if already migrated to avoid redundant work. + migrated, err := IsMigrated(ctx, globalDB, sourcePath) + if err != nil { + return fmt.Errorf("failed to check migration status: %w", err) + } + if migrated { + slog.Info("Source DB already migrated, skipping", "path", sourcePath) + return nil + } + if batchSize <= 0 { return migrateSynchronous(ctx, conn, workingDir, sourcePath) } return migrateBatched(ctx, conn, workingDir, sourcePath, batchSize) Suggestion importance[1-10]: 7 __ Why: The suggestion is valid — `IsMigrated` exists but is never called within `MigrateProjectDB`, meaning already-migrated databases will go through the full ATTACH/goose/INSERT cycle unnecessarily. Adding an early check would save significant I/O. However, the `INSERT OR IGNORE` and `migrations_completed` table ensure correctness regardless, so this is a performance optimization rather than a bug fix. It's also possible the caller is expected to check `IsMigrated` externally (as seen in `MigrateCurrentProject`), which would reduce the impact.	Medium
	Silent error swallowing in reload `reloadSessions` silently swallows errors, which makes debugging impossible if session listing fails. At minimum, log the error so users/developers can diagnose issues. internal/ui/dialog/sessions.go [431-444] func (s *Session) reloadSessions() { var workingDir string if !s.showAll { workingDir = s.com.Workspace.WorkingDir() } sessions, err := s.com.Workspace.ListSessions(context.TODO(), workingDir) if err != nil { + slog.Error("Failed to reload sessions", "error", err) return } s.sessions = sessions s.list.SetItems(sessionItems(s.com.Styles, sessionsModeNormal, s.sessions...)...) s.list.SelectFirst() s.list.ScrollToSelected() } Suggestion importance[1-10]: 4 __ Why: The suggestion to add error logging in `reloadSessions` is reasonable for debuggability. However, silently returning on error is a common pattern in TUI code where there's no good way to surface errors to the user, and the impact is minor — the session list simply doesn't update. Adding `slog.Error` is a modest improvement.	Low
	Relative path fallback causes lookup mismatches When `os.Getwd()` fails, `abspath` returns a relative path. This relative path will be stored in the database and won't match future absolute-path lookups, causing `LastReadTime` to always return zero and `RecordRead` to create duplicate entries. Consider returning an error or at least cleaning the joined result, but the real risk is the silent data inconsistency from storing a relative path in a column that callers assume is always absolute. internal/filetracker/service.go [62-73] func abspath(path string) string { path = filepath.Clean(path) if filepath.IsAbs(path) { return path } wd, err := os.Getwd() if err != nil { - slog.Warn("Error getting working directory", "error", err) + slog.Error("Cannot make path absolute without working directory; storing relative path which may cause lookup mismatches", "error", err, "path", path) return path } - return filepath.Join(wd, path) + return filepath.Clean(filepath.Join(wd, path)) } Suggestion importance[1-10]: 4 __ Why: The concern about storing a relative path when `os.Getwd()` fails is technically valid, but `os.Getwd()` failing is extremely rare in practice (requires the working directory to have been deleted). The suggested fix only changes the log level from `Warn` to `Error` and adds `filepath.Clean` to the final join (which is redundant since `filepath.Join` already cleans). The core issue — returning a relative path — remains unfixed in both versions. The improvement is marginal.	Low

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

…migrate Eliminates the db → projects layering violation by moving application-level data migration logic out of the infrastructure package. internal/db stays focused on connections, sqlc queries, and schema migrations. The new internal/migrate package owns the ATTACH-based copy engine, OAuth conflict resolution, startup orchestration, and project discovery dependency. Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

The migration engine moved from internal/db/migrate.go to internal/migrate/ in fe774ab. Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Add ResetMigration/ResetAllMigrations to clear completion markers so partially-failed migrations can be re-run. Add --force-migration CLI flag that clears all markers before migrating. Add per-stage logging to migrateBatched for observability. Document re-run safety guarantees (INSERT OR IGNORE ensures idempotent replay without duplicates). Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Broderick-Westrope added 11 commits June 2, 2026 07:56

tweak

009be90

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

tweak data dir name

deb26cc

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

mig default

a67ec31

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

query changes

0767643

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

ded

7fe86b3

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

da review fixes

011cc39

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

update scratchpad

7f36d38

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Broderick-Westrope self-assigned this Jun 2, 2026

github-actions Bot added the Review effort 4/5 label Jun 2, 2026

Broderick-Westrope added 10 commits June 3, 2026 21:30

docs: add session UX and stats TUI ideas to scratchpad

36df97d

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Merge branch 'main' into global-database

58b3ad8

docs: update AGENTS.md migration path after package extraction

9d40d82

The migration engine moved from internal/db/migrate.go to internal/migrate/ in fe774ab. Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

refactor: read trigger DDL from sqlite_master instead of hardcoding

8c739c8

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

fix: quote trigger identifiers and guard against IF NOT EXISTS stripping

74af3fb

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Merge branch 'main' into global-database

db20120

fix: update VCR cassettes and remove ResetPool from parallel OAuth tests

9cb569c

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

fix: release pooled DB connections in migrate tests for Windows cleanup

2c3c8c9

Co-authored-by: Brodie Westrope brodie.westrope@gmail.com

Broderick-Westrope merged commit 62f9762 into main Jun 3, 2026
11 of 12 checks passed

Broderick-Westrope deleted the global-database branch June 3, 2026 12:23

Navigation Menu

move to using a global database - #20

move to using a global database#20
Broderick-Westrope merged 21 commits into
mainfrom
global-database

Broderick-Westrope commented Jun 2, 2026 •

edited by github-actions Bot

Loading

Uh oh!

github-actions Bot commented Jun 2, 2026

Uh oh!

github-actions Bot commented Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Broderick-Westrope commented Jun 2, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

github-actions Bot commented Jun 2, 2026

PR Reviewer Guide 🔍

Uh oh!

github-actions Bot commented Jun 2, 2026

PR Code Suggestions ✨

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Broderick-Westrope commented Jun 2, 2026 •

edited by github-actions Bot

Loading