[malibio]

Summary

• conflictsStore: generation guard on load() / loadForNode() plus invalidateForDatabaseSwitch(), matching the schemas/ai-chats stores. Invalidate also clears records immediately, because node ids can repeat across databases (date nodes). Without that, the per-node indicator would flag the new database's nodes from the old journal until the reload lands. load() now resolves true/false for applied/discarded.
• databaseStore.evictAndReloadActiveDatabase() invalidates and reloads the conflicts store. This covers both switchTo and the non-default startup path.
• app-shell starts the startup "N unresolved conflicts" notice once databaseStore.activeDatabaseId is set, so list_conflicts is routed to the restored database. A load discarded by a switch shows no notice.

On a non-default startup list_conflicts runs twice: once from the evict and once from the notice. Both calls happen after routing is set, so both return the same database's journal.

Acceptance Criteria

• After launching into a non-default database, the startup conflicts notice counts that database's open conflicts
• After a database switch, conflictsStore.records describes the new database
• A conflicts load issued before a switch cannot overwrite the post-switch records

Tests

• conflicts.test.ts: new tests for invalidate clearing records, a stale load (success or failure) resolving after the reload, and a stale per-node load.
• database.test.ts: asserts invalidate runs before load on both the switch and non-default-startup paths.
• Frontend: 5378 passed. The pre-push gate passed.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review — PR #3069

Review Type: Initial review (no prior reviews/comments)

Summary

Small, well-scoped fix that brings conflictsStore in line with the #generation invalidation pattern already used by schemasData / aiChatsData, and moves the startup notice's load behind database routing. The design matches the existing architecture (evictAndReloadActiveDatabase is the one place per-database stores reset), and the tests pin the race behavior directly. Affected suites pass locally (conflicts.test.ts + database.test.ts: 56/56). Net positive, ready to merge.

Requirements Check

• ✅ After launching into a non-default database, the startup notice counts that database's open conflicts. The $effect in app-shell.svelte only fires once databaseStore.activeDatabaseId is non-null. DatabaseStore.load() assigns that only after await invoke('set_active_database'), so list_conflicts is routed to the restored database. On a non-default launch, evictAndReloadActiveDatabase() runs synchronously in the same tick, so both loads share the post-switch generation and neither is discarded.
• ✅ After a database switch, conflictsStore.records describes the new database. invalidateForDatabaseSwitch() clears the store and load() runs inside evictAndReloadActiveDatabase(). The switch test also checks the call order.
• ✅ A load issued before a switch cannot overwrite post-switch records. The generation check in load() and loadForNode() handles this, with tests for the success, failure and per-node paths.

Code Review Findings

Correctness checks (verified, no issue)

• Svelte 5 effect gating: conflictsNoticeStarted is a plain let, and it is read before activeDatabaseId. After the first run the effect returns early without reading any reactive value, so it has no dependencies and never runs again. Later switches correctly show no notice.
• Browser mode: the old call sat inside the __TAURI_INTERNALS__ block in onMount. The new effect keeps that gate, so behavior is unchanged.
• conflicts-pane.svelte {#if !conflictsStore.loaded}: the pane now shows "Loading…" between invalidate and reload instead of the previous database's list, which is the right UX. A pane load that gets discarded leaves loaded=false, but the reload that evictAndReloadActiveDatabase always issues sets it again. clearAllTabs() closes the pane on a switch anyway.
• load() return type void → boolean: every other caller (database.svelte.ts, conflicts-pane.svelte) uses void conflictsStore.load(), so none break. The error path now resolves true and still clears the records, and the updated test reflects that.
• Uses createLogger. No lint suppression, no issue numbers in comments, no backward-compat code.

🟡 Improvement (optional, can be a follow-up)

📁 packages/desktop-app/src/lib/stores/conflicts.svelte.ts — resolve() (and the loadForNode inside merge()) have no generation guard.
Conflict ids are deterministic. deterministic_conflict_id in packages/core/src/services/node_service/conflicts.rs is a UUIDv5 over kind | sorted node_ids | discriminator. Node ids can repeat across databases (date nodes, name-derived collection ids), so the same conflict id can exist in both databases. If a resolve_conflict is in flight when a switch lands, this.records.map(r => r.id === conflictId ? updated : r) would overwrite the new database's record with the old database's resolved copy. The same stale-write hazard this PR closes for load() stays open for the action path.
The window is narrow, and the pane is closed on switch, so this is not a blocker. Still, a two-line guard makes the store consistent: capture #generation before the invoke and skip the local patch if it changed. For merge(), capture it before merge_nodes and skip the loadForNode refresh if it moved. Principle: one invariant for every async write into per-database state, not most of them.

🟢 Nits

• Nit: 📁 app-shell.svelte:70-78 — the first paragraph of the comment still says "load on every startup, unconditionally". That now reads against the new "Started only once databaseStore has a selection" paragraph. Consider rewording to "unconditional on tier" so the two paragraphs agree.
• Nit: 📁 app-shell.svelte:85 — a fifth inline copy of the __TAURI_INTERNALS__ cast. isTauriBridgePresent (database store) and isTauri (tauri-commands) already exist but aren't exported. This PR doesn't need to fix that, but a shared $lib/utils helper would stop the copies (DRY).
• Nit: on a non-default launch, list_conflicts is sent twice (once from evictAndReloadActiveDatabase, once from the notice effect). Both results are correct and it only happens once per launch, so this is fine to leave. It is only worth noting if someone later wonders why the call is duplicated.
• Nit: the app-shell effect has no component-level test. The routing guarantee is covered indirectly by database.test.ts, which checks that invalidate and load are called on a non-default startup, and the gating is simple. Acceptable as-is.

Recommendation: APPROVE

All three acceptance criteria are met, the change follows the established generation-guard pattern, and the tests cover the real race paths. The resolve() guard is a worthwhile follow-up but does not block merge.

[malibio]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[malibio]

Review addressed (acff822)

✅ Addressed

• 🟡 resolve() / merge() generation guard: resolve() no longer patches local records if a database switch landed while it was in flight (conflict ids are deterministic, so the same id can exist in both databases). merge() skips its post-merge refresh after a switch. Two tests added.
• 🟢 Fixed the stale "unconditionally" comment in app-shell.svelte.

⏭️ Skipped

• 🟢 Shared Tauri-bridge helper: the inline check is an existing pattern across the file, and consolidating it is outside this PR's scope.
• 🟢 Double list_conflicts on a non-default startup: both calls happen after routing, return the same database's journal, and only happen once per launch.
• 🟢 Component test for the app-shell effect: the gating is trivial and the store-level tests cover the logic.

Re-review: not needed. The changes are small guards that follow the pattern the review already approved, and each has a test.