← Back to PRs

#15620: fix(memory): await sync in search to prevent database closure race

by superlowburn open 2026-02-13 17:28 View on GitHub →
stale size: XS
## Summary Fixes #14716 - MemorySearch `search()` was firing off `warmSession()` and `sync()` without awaiting them - During sync, `runSafeReindex` closes the database to swap files — if a concurrent search was running, it would hit "database is not open" - Fix: `search()` now awaits both `warmSession()` and `sync()` before querying, preventing the race condition ## Changes - `src/memory/manager.ts`: Changed `void this.warmSession()` → `await this.warmSession()` and `void this.sync()` → `await this.sync()` in the `search()` method. Added clarifying comment in `runSafeReindex` about the database closure window. - `src/memory/manager.async-search.test.ts`: Updated test to reflect new blocking behavior (search now waits for sync instead of racing ahead) ## Trade-off Search now blocks on sync completion, which may slightly increase latency for the first search after dirty state. This is acceptable because the previous fire-and-forget behavior caused data corruption (empty sqlite index) and persistent "database is not open" errors. ## Test plan - [x] Updated async search test passes - [x] All 96 memory module tests pass (17 test files) - [x] Existing test assertion change is intentional — old test asserted buggy fire-and-forget behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR changes `MemoryIndexManager.search()` to await `warmSession()` and (when `sync.onSearch` and the index is dirty) await `sync({ reason: "search" })` before running queries. It also adds a clarifying comment in `runSafeReindex()` about the window where the SQLite DB handle is closed during file swapping, and updates the async-search test to assert that `search()` blocks behind a pending sync. The intent is to prevent the “database is not open” race caused by `runSafeReindex()` closing/swapping the index while concurrent searches still use `this.db`. Two issues to address before merge: - `await this.warmSession()` doesn’t actually wait for the session-start sync, because `warmSession()` still fire-and-forgets `sync()`. - The new “sync lock” wording in `runSafeReindex()` doesn’t match the current implementation: `sync()` is deduped but there’s no mutual exclusion that blocks concurrent DB reads during the close/swap window, so non-search sync triggers can still race with `search()`. <h3>Confidence Score: 2/5</h3> - This PR reduces one race but still leaves paths where searches can hit a closed SQLite handle. - Awaiting search-triggered sync helps, but `warmSession()` still doesn’t await its sync and there’s no mutual exclusion preventing non-search sync triggers (watch/interval/session-delta) from closing/swapping the DB during concurrent reads; the new comment also overstates the protection provided. - src/memory/manager.ts <sub>Last reviewed commit: e249028</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs