Repository navigation
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 1 minute. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
Priority: ⬆️ High Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change skips reading Cursor's desktop login database when its WAL is oversized or cannot be measured. This avoids main-thread freezes. Users whose only login is in an oversized WAL need to run cursor-agent login. No merge-blocking risk remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change avoids reading oversized or unreadable desktop login data without granting additional access or accepting skipped reads as successful logins. The assessed behavior is narrowly scoped, although the complete pre-change comparison remains limited. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, fix, scope, tradeoffs, linked issue, and test command. However, it does not use the required template and omits or does not explicitly complete several required sections, including Visual Proof with an N/A reason, Review, Agent skill upstream boundary, Notes, and the Checklist items. Resolution Update the description to follow the repository template. Add the required headings and complete the missing sections, including Visual Proof with
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ Minor suggestions only — one doc placement and two small robustness points inline. The mechanism and the 32 MiB cut are sound for the reported scenario.
Reviewed changes
- WAL size guard —
readCursorDesktopProfilenow statsstate.vscdb-waland returns an error result before opening when it exceedsCURSOR_DESKTOP_WAL_SKIP_BYTES(32 MiB), avoiding the main-thread wal-index recovery scan that froze the UI (#24360).cursor-auth.tsalready drops desktop read errors, so the usage alert is unaffected. - New test —
cursor-desktop-state-db.test.tsreads a seeded DB, reports a missing DB, and refuses a DB whose WAL is just over the threshold.
I confirmed against sqlite.org that the stall is the single-pass wal-index recovery, that it is linear in -wal bytes, that it runs on the first connector to a quiescent DB (Cursor not running means Orca is first), and that node:sqlite's timeout is only sqlite3_busy_timeout — so it cannot bound this. -wal (not .vscdb) is the right signal for the bounded ItemTable WHERE key IN (…) query, conditional on Cursor's ItemTable.key being uniquely indexed (true in the VS Code lineage). The threshold is a conservative heuristic, not a hard time bound, which is a reasonable call for a guard.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
The sqlite timeout only bounds a lock wait. Replaying a multi-GB WAL blocked the UI on focus. A normal database still supplies the login. Fixes stablyai#24360
A stat failure other than a missing file used to look like no WAL, so the main thread still opened the database. An oversized or unreadable WAL is now a skip, not a login error, and a small WAL still returns its token.
8596107 to
9638785
Compare
Sync update (
|
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions inline.
Reviewed changes
- Fail-closed WAL measurement —
cursorDesktopWalBytesnow returnsnullfor anystatSyncerror other thanENOENT, andreadCursorDesktopProfilereturns{ status: 'skipped', reason: 'wal-unreadable' }instead of opening the database. - Distinct skip result — a new
{ status: 'skipped'; reason: 'wal-too-large' | 'wal-unreadable' }union member replaces thestatus: 'error'overload for intentional skips;cursor-auth.tsstill only checksstatus === 'ok', so the intended fall-through is unchanged. - Doc placement — the "Reads the Cursor IDE's stored session" JSDoc was moved back onto
readCursorDesktopProfile, with a fresh one-liner oncursorDesktopWalBytes. - Tests — added a small-WAL read case plus a
node:fsmock that makesstatSyncthrowEACCES, asserting the unreadable-WAL skip.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
seedDatabase always writes state.vscdb. The extra parameter had no caller.
Sync update (
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Test-only cleanup —
seedDatabaseincursor-desktop-state-db.test.tsno longer takes anameparameter; it now always seedsstate.vscdb. This is a pure signature simplification with no production behavior change. - Prior feedback — this commit closes the last outstanding Pullfrog nit (unused
seedDatabaseparameter); the run-1 findings on doc placement, the distinctskippedresult, and fail-closed WAL measurement remain addressed.
The delta since the prior review (9638785) is test-only. I re-ran cursor-desktop-state-db.test.ts: 5/5 pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
|
Superseded by merged #24572, which credits @innocarpe and moves the desktop SQLite read to a bounded worker so large WALs cannot block the main process. Thank you for the original reproduction and focused fix; #24360 is now closed by the merged continuation. |
2 participants
Description
Switching back to Orca froze the UI for several seconds on machines whose Cursor
state.vscdbhas a huge uncheckpointed WAL. The desktop-login probe opens that database withnode:sqliteon the main thread. The 250 ms timeout only covers a lock wait, so SQLite still scans the whole WAL before the first query returns.Focused fix
In:
state.vscdb-walis larger than 32 MiB.Out:
Preserves
A missing database is still
missing. A normal database, with no WAL or a small one, still returns the stored access token, email, and membership fields. A busy lock on a small database still uses the existing timeout.Evidence
node node_modules/vitest/vitest.mjs run --config config/vitest.config.ts --cache false src/main/rate-limits/cursor-desktop-state-db.test.tsThe test reads a token from a small database, reports a missing file, and refuses a database whose WAL is just over 32 MiB without treating that file as a successful login.
User-regression-tradeoffs
Someone whose only Cursor login lives in a WAL larger than 32 MiB no longer gets that desktop session automatically. They can still sign in with
cursor-agent login. The freeze on every focus is the reported failure.Fixes #24360