[malibio]

Summary

• databaseStore.load() now awaits set_active_database(resolved) before committing the selection, so a remembered or tray-picked non-default database becomes the gRPC routing target (and node-event watch target), not only the window pin and switcher.
• When the restored database is not the daemon default, the sidebar stores' boot-time loads (which went out unrouted and were answered by the default) are evicted and reloaded. The evict/reload block is extracted from switchTo into evictAndReloadActiveDatabase(); only switchTo resets tabs, so restored tabs survive a restart.
• app-shell.svelte mounts PaneManager only once activeDatabaseId is set, so restored tabs never read from the default database first.
• A tray pick that commits while load() is routing wins (its set_active_database is sent later, and load() re-checks the selection after the await).

Pin and routing stay separate commands: pinning is per-window, routing is process-wide.

Tests

• 4 new database.test.ts cases: routing before commit, reload on non-default restore, no reload on default restore, tray pick during load wins.
• Frontend: 5359 passed; pre-push gate passed.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review: PR #3066

Review Type: Initial review. The PR had no earlier reviews or comments.

Requirements Check (#3058)

Requirement: at startup, the restored database must also be the gRPC routing target, not only the window pin and the switcher.

• ✅ load() now awaits set_active_database(resolved) before it commits activeDatabaseId. The node, import, embeddings, agent-session and local-agent clients, and the node-event watcher (GrpcClient::set_active_database re-subscribes it), all point at the restored database.
• ✅ When the restore is not the default, the sidebar loads that ran before routing was set are dropped and reloaded. Each of those stores captures a generation at the start of a load and discards the result if the generation changed. The evict step (forgetLocallyCreated, invalidateForDatabaseSwitch) runs only after the routing call resolves. So a boot-time load, a pro:tier-detected reload or an onDaemonReconnect reload that went out unrouted is discarded, and anything issued later is routed. Every interleaving I traced is safe.
• ✅ PaneManager is mounted only once a database is selected, so restored tabs never read from the default database first.
• ✅ Restored tabs are kept on a non-default restore. Only switchTo resets tabs, and a test covers this.
• ✅ The Rust set_active_database never fails (it returns Ok(()), and it is a no-op when the id is unchanged). The new await therefore adds no new way for load() to fail beyond the IPC call itself.

Ran locally: bun run test src/tests/stores/database.test.ts: 40/40 passed.

Code Review Findings

🟡 [Improvement] The tray-pick ordering the comment relies on is not guaranteed
📁 packages/desktop-app/src/lib/stores/database.svelte.ts:204-214
The comment says a tray switchTo that commits during load()'s await "wins: its set_active_database was sent after this one". That covers only one interleaving. switchTo sets activeDatabaseId only after its own set_active_database resolves. So this sequence is possible:

1. switchTo('a') sends set_active_database(a) and is waiting on it. activeDatabaseId is still null.
2. load() passes its re-check (null) and sends set_active_database(b).
3. Rust applies a, then b. switchTo commits a and reloads the sidebar. load() re-checks, sees a, and returns.

Result: the UI shows a and routing points at b, which is the same split this issue fixes, and it lasts until a manual switch. The window is small. It needs a pending or live tray pick (applyPendingTraySelection, or the tray:select-database event during a webview boot) for a different id than initial_database_id, overlapping load()'s IPC round-trips. It is still reachable. The existing test "lets a tray switch… win" covers only the case where load() sends first.
Recommendation: Capture switchSeq at the start of load(), without bumping it, and return if it has changed, both before and after the set_active_database await:

const seqAtStart = this.switchSeq; // read, never bump: must not abort an in-flight switchTo
...
if (this.activeDatabaseId !== null || this.switchSeq !== seqAtStart) return;
await invoke('set_active_database', { id: resolved });
if (this.activeDatabaseId !== null || this.switchSeq !== seqAtStart) return;

Reading the counter keeps the deliberate choice not to abort a mid-flush switchTo, and it closes the reverse ordering. Add the mirror test: switchTo's set_active_database is held open while load() reaches its routing step; assert that set_active_database(b) is never sent, or that the last routed id equals activeDatabaseId. Update the comment to match.
Principle: correctness under concurrency. An invariant stated in a comment should be enforced by the code, not by timing.

🟢 [Nit] A failed load() now leaves the workspace blank instead of showing a degraded pane
📁 packages/desktop-app/src/lib/components/layout/app-shell.svelte:674-676
When the daemon is unreachable, the banner explains the blank pane and onDaemonReconnect retries load(). But if the daemon is reachable and list_databases (or the routing IPC) returns an error, activeDatabaseId stays null and the content area is empty. The only error is databaseStore.error in the switcher, and nothing retries until the next reconnect. The same happens if a tray switchTo fails during boot after load() has bailed (this also applies once the fix above is in). This is acceptable for now. Consider a small empty-state that shows databaseStore.error with a Retry button calling databaseStore.load().

🟢 [Nit / follow-up, pre-existing] The startup conflict nudge reads before routing is set
📁 packages/desktop-app/src/lib/components/layout/app-shell.svelte:252 (loadConflicts)
list_conflicts goes through the routed node client (client.client()). At boot it runs before routing is set, so on a non-default restore the "N unresolved conflicts" nudge counts the default database's journal. evictAndReloadActiveDatabase does not reload conflictsStore, and neither did switchTo before this PR. The Conflicts pane reloads when it mounts, so only the one-time nudge and the cached records are wrong. Out of scope; consider a follow-up that calls conflictsStore.load() from evictAndReloadActiveDatabase, or starts loadConflicts after databaseStore.load() resolves.

🟢 Positive notes

• Extracting evictAndReloadActiveDatabase() is a clean DRY win. The shared eviction list can no longer drift between switch and restore, and keeping the tab reset in switchTo is the right boundary.
• Gating PaneManager on the selection is simpler and more robust than adding routing guards to each viewer.
• The tests check behaviour that matters: routing happens before commit (activeWhenRouted is null), a non-default restore reloads, a default restore does not, and tabs are preserved.
• Pin (per window) and routing (per process) stay separate commands, which matches their different scopes.

Recommendation: APPROVE

The change fixes #3058 correctly. The boot-time unrouted reads are handled well by the existing generation guards. The one 🟡 is a narrow reverse-order race with the tray pick and a comment that overstates the guarantee. It is a small fix (read switchSeq, add the mirror test), and I recommend it before merge, but it is not a blocker.

[malibio]

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

[malibio]

Review addressed (a6e9f12)

✅ 🟡 Tray-pick race (reverse ordering) — load() now captures switchSeq at the start of first-selection resolution and also tracks in-flight switchTo calls (switchesInFlight). It skips its own set_active_database when a switch is in flight or started since it began, and re-checks after the await. Bumping switchSeq from load() was avoided on purpose (it would abort a mid-flush tray switch). Added the mirror test (switchTo sends first, load resolves while it is pending); verified it fails with the guard removed.

⏭️ 🟢 Blank content area on failed load() — skipped: the switcher already surfaces databaseStore.error and onDaemonReconnect retries. An empty-state UI is beyond this fix's scope.

⏭️ 🟢 Startup conflicts notice counts default DB — pre-existing (switchTo never reloaded conflicts either), and conflictsStore.load() has no generation guard, so just reloading it would still race. Filed as #3067.

Frontend: 5360 passed; pre-push gate passed.

Re-Review Decision

NO RE-REVIEW NEEDED — the change is a small, test-covered guard implementing the reviewer's suggested approach; no other logic changed.