Repository navigation
fix(settings): return current preferences after slow updates - #27199
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The handler now returns current preferences after delayed updates. No merge-blocking issue was identified; the reported platform validation remains for CI. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation 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. Title check The title clearly and concisely describes the main change: returning current preferences after slow settings updates. Description check The description covers the required sections, explains the stale-response problem and implementation, references issue #26853, documents the backend-only visual impact, testing results, limitations, r…
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
-
📝 Generate docstrings 💡
-
-
- Commit to this branch
- Create a new PR
-
-
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
1 participant
ELI5
Settings writes now reply with the latest preferences after a slower language, proxy, or agent-hook update finishes.
What Changed
Previously, a desktop settings write captured a full settings snapshot before waiting for those updates. Another write could finish during that wait, leaving the older request to return outdated preferences. The handler now reads the current Store when it returns. Side effects and telemetry still use the original write's snapshot.
Why
The existing paired-runtime settings writer already reads current settings after reconciliation. Using the same response boundary on desktop fixes the stale reply without adding settings versions, changing persistence, or serializing unrelated writes.
Linked Issue
Related to the settings-concurrency verification for #26853; this regression also reproduces on main without that PR. No separate existing issue found in the bounded search.
Visual Proof
N/A — this changes the backend reply boundary. No rendering control or layout changes. The table shows recorded registered-handler results with real Store and SQLite persistence; it is not an application screenshot.
Testing
On macOS with Node 24.20.0, 44 tests pass across the new delayed-reply suite, existing settings-handler suite, and durable preference suite. The language and proxy controls fail on unchanged main: the reply says dark while both the Store and independent SQLite read say light. The hook control also verifies that its side-effect input retains the original dark snapshot while the reply contains light.
Head
924dc8be5734f965ad1d156eebc08400bfc789ecadds the new SQLite IPC suite to the existing explicit Node-runtime test list. Its production code and test assertions are byte-identical to the previously verified head964cc4d858e9915678769cefb7152fcebda3a8f4. The canonical runner now dispatches the same three cases to Node; all three plus the existing runtime/SQLite boundary controls pass, 16 tests across three files. Before registration, the same three assertions also passed in Bun. This follows the existing SQLite worker-IPC runtime policy; it is not a product fix or a weakened assertion.Full Node typecheck, full-file root/native/type-aware/anti-slop/React Doctor checks, test casting check, changed-code quality gate, formatting, and diff whitespace checks pass. Commands ran directly with the required background-launch environment, using existing dependencies without installation or native rebuilding. Full application build, rendered Electron validation, Linux, Windows, and SSH execution were not run for this backend-only change; CI remains required.
AI Disclosure
Review
Reviewed write authority, asynchronous response timing, real persisted state, retained side-effect inputs, paired-runtime behavior, and renderer publication fencing. The response shape and all persisted fields remain compatible with existing clients.
Agent skill upstream boundary
Notes
The bounded related-code search found the paired runtime already returns a fresh read. Open #26826 extracts this same desktop handler for settings backup/import and should retain this correction when rebased. No Git commands, paths, shortcuts, platform checks, or remote wire fields change.
The prior head
964cc4dcompleted CI with 14 successful and 16 skipped checks. Fresh CI is required for the additive test-registration head924dc8be; prior results do not represent that new run.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)