Fix WAL mode set and sqlite connection pool setup for cursorcli#221
Open
bago2k4 wants to merge 2 commits into
Open
Fix WAL mode set and sqlite connection pool setup for cursorcli#221bago2k4 wants to merge 2 commits into
bago2k4 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts how the Cursor CLI provider opens SQLite databases to (1) reduce the chance of accumulating file descriptors via database/sql connection pooling, and (2) centralize enabling SQLite WAL mode into a one-time helper invoked when a session is first detected.
Changes:
- Constrain SQLite connection pools to 1 open / 1 idle connection and set a max lifetime for read-only DB handles used by the watcher and session reader.
- Remove per-read
PRAGMA journal_mode=WALcalls and introduceEnsureWALMode(dbPath)to set/verify WAL mode via a brief read-write connection. - Call
EnsureWALModeonce when a session is first observed by the watcher.
3 most important improvements to make:
- Update
EnsureWALModeto open SQLite using a safer/consistent DSN (e.g.,file:+filepath.ToSlash(...)) andmode=rwto avoid accidentally creating new DB files and to align with the repo’s established SQLite DSN pattern (seespecstory-cli/pkg/provenance/store.go:29). - Narrow the
CursorWatchermutex scope inhasSessionChangedso the lock only protects map state, not the DB I/O and WAL initialization work. - Add a focused unit test for
EnsureWALMode(this package already has tests) that creates a temp SQLite DB, checksPRAGMA journal_mode, runsEnsureWALMode, and asserts the mode becomes WAL.
Unit test opportunities (high-signal only):
- A single test covering
EnsureWALMode’s journal-mode transition and error behavior (especially if switching tomode=rw) would meaningfully reduce risk without adding tautological coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
specstory-cli/pkg/providers/cursorcli/watcher.go |
Uses a controlled read-only pool for polling counts and ensures WAL once when a session is first seen. |
specstory-cli/pkg/providers/cursorcli/sqlite_reader.go |
Applies controlled pooling for reads and adds centralized WAL-mode enforcement via EnsureWALMode. |
Comment on lines
225
to
+228
| // Always check record count - don't rely on file modification time | ||
| // SQLite with WAL mode may not update file mtime when records are added | ||
|
|
||
| // Open database to count records (with WAL mode) | ||
| // Open database to count records (read-only with controlled pool) |
Comment on lines
+357
to
+362
| // EnsureWALMode ensures the database is running in WAL journal mode. | ||
| // WAL mode is required so that the -wal file exists and file modification | ||
| // detection works reliably. This opens a brief read-write connection and | ||
| // is intended to be called once per session database, not on every read. | ||
| func EnsureWALMode(dbPath string) error { | ||
| db, err := sql.Open("sqlite", dbPath) |
| return fmt.Errorf("failed to enable WAL mode: got %q instead", newMode) | ||
| } | ||
|
|
||
| slog.Info("Enabled WAL mode on database", "path", dbPath) |
Comment on lines
+357
to
+361
| // EnsureWALMode ensures the database is running in WAL journal mode. | ||
| // WAL mode is required so that the -wal file exists and file modification | ||
| // detection works reliably. This opens a brief read-write connection and | ||
| // is intended to be called once per session database, not on every read. | ||
| func EnsureWALMode(dbPath string) error { |
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
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.
This pull request improves how SQLite databases are accessed and managed in the Cursor CLI provider, with a focus on connection pooling and ensuring reliable use of WAL (Write-Ahead Logging) mode. The changes prevent file descriptor leaks, ensure consistent file modification detection, and centralize the logic for enabling WAL mode.
Database connection management improvements:
sqlite_reader.goandwatcher.go. [1] [2] [3] [4]WAL mode handling enhancements:
EnsureWALModefunction insqlite_reader.gothat ensures the database is in WAL mode by opening a brief read-write connection. This function is now called once per session when a session is first seen in the watcher. [1] [2] [3]These changes make database access safer and more reliable, especially under heavy usage or when monitoring many sessions.