Feat/multi location hosts sync readiness - #85
Conversation
Add host catalog merge/filter/search by logicalId, session inventory cache, and Keep/materialize restore path for provider hosts. Include unit tests and clearer jump-host missing guidance that is provider-neutral.
Wire the sidebar to the host catalog with All/Local/Remote filters, sticky search chrome, provider location chips, remote Keep / Keep-and-open rows, and session inventory refresh. Local-only chips stay hidden; multi-location badges remain for provider-backed hosts.
Introduce a single sync readiness store for Google OAuth and collection encryption so Sync and Backup and All Hosts stay consistent. Notify on collection unlock and setup, improve key-wrap recovery error handling, and refresh vault sidebar attention, theme-token warning banners, and recovery key presentation.
Document Unreleased host catalog, shared provider readiness, vault sidebar attention, and related fixes with commit hashes.
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis change adds provider-backed host cataloging, cached remote inventory, selective restore and collection-key recovery, shared sync readiness state, provider-aware sidebar actions, vault attention indicators, and updated provider icons and warning styles. ChangesHost synchronization and recovery
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
PR Summary by QodoMulti-location host catalog, Keep/materialize flow, and shared sync readiness
AI Description
Diagram
High-Level Assessment
Files changed (40)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/settings/tabs/vault/VaultSyncCard.tsx (1)
250-278: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winOAuth connect still blocks Upload/Restore
isCollectionActionBlockedstill includesisSyncing, andSyncDomainsGroupedusesisProviderDomainActionDisabledto disable both sync and restore actions. That keeps Upload/Restore blocked during Google OAuth connect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/settings/tabs/vault/VaultSyncCard.tsx` around lines 250 - 278, Remove isSyncing from isCollectionActionBlocked so Google OAuth connection does not disable Upload/Restore actions. Keep isSyncing included only in the provider-specific action gating, preserving blocking for collection setup, unlock, lock, recovery-key regeneration, and domain operations.
🧹 Nitpick comments (5)
src/components/layout/Sidebar.tsx (1)
253-257: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo-op ternary in
treeRootmemo — both branches returnsearchTerm.
hostFilter === 'all' || hostFilter === 'local' ? searchTerm : searchTermalways evaluates tosearchTermregardless ofhostFilter. This looks like leftover code from an incomplete refactor — please confirm whether a real per-filter distinction was intended (e.g. skipping search re-application forremote), otherwise simplify to remove the dead conditional and the now-unnecessaryhostFilterdependency.♻️ Suggested simplification (if no distinction is intended)
const treeRoot = useMemo( - () => buildTree(treeConnections, folders, hostFilter === 'all' || hostFilter === 'local' ? searchTerm : searchTerm), - [treeConnections, folders, searchTerm, hostFilter], + () => buildTree(treeConnections, folders, searchTerm), + [treeConnections, folders, searchTerm], );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/Sidebar.tsx` around lines 253 - 257, In the treeRoot useMemo, remove the no-op hostFilter ternary and pass searchTerm directly to buildTree. Remove hostFilter from that memo’s dependency array, unless a real per-filter search distinction is required; if one is intended, implement the distinct remote behavior explicitly.src/components/layout/sidebar/VaultNavSection.tsx (1)
19-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate branching logic between
resolveVaultAttentionandstatusDotClass.Both functions branch on the same
(status, error, securableCount)triple with parallel logic. Consider derivingstatusDotClassoutput fromresolveVaultAttention's result (or merging into one function) to avoid maintaining two copies of the same state machine.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/layout/sidebar/VaultNavSection.tsx` around lines 19 - 76, Consolidate the duplicated vault-state branching by deriving status-dot output from resolveVaultAttention, or by merging both results into a single shared resolver. Ensure the resulting logic still distinguishes in-use errors, unlocked vaults with securable credentials, locked vaults, and unconfigured vaults while preserving each existing className and title.src/features/connections/domain/hostMaterialize.ts (1)
59-72: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider suppressing the success toast when
loadConnectionsfails.If
loadConnectionsthrows, the user sees both an error toast ("local host list failed to refresh") and a success toast ("restore succeeded") simultaneously. While the comment on line 59 correctly notes the restore itself succeeded, surfacing both toasts at once may confuse users. Consider skipping the success toast (or downgrading it to a warning) when the reload fails.♻️ Suggested adjustment
// Restore succeeded — reload is best-effort and must not report as restore failure. + let reloadFailed = false; try { await options.loadConnections(); } catch (error) { + reloadFailed = true; const message = error instanceof Error ? error.message : String(error); options.showToast( 'error', message || 'Host was saved, but the local host list failed to refresh. Reload the app if hosts look stale.', ); } - if (!options.silentSuccess) { + if (!options.silentSuccess && !reloadFailed) { options.showToast('success', formatConnectionsRestoreSuccessMessage(result)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/connections/domain/hostMaterialize.ts` around lines 59 - 72, Track whether options.loadConnections() completes successfully in the restore flow around formatConnectionsRestoreSuccessMessage. When reloading fails, keep the error toast but suppress the subsequent success toast; retain the existing success toast when reload succeeds, respecting options.silentSuccess.src/store/connectionSlice.ts (1)
405-410: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
clearAllHostInventoryCaches()call.
clearAllHostInventoryCaches()is called directly here and again via theCONNECTIONS_CLEARED_EVENTlistener inuseHostCatalog.ts(line 242). The second call is a no-op since caches are already cleared. Not harmful, but consider removing the direct call here and relying solely on the event-driven cleanup to keep a single owner of cache lifecycle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/store/connectionSlice.ts` around lines 405 - 410, Remove the direct clearAllHostInventoryCaches() invocation from the connection-clearing flow and rely on the CONNECTIONS_CLEARED_EVENT dispatch to trigger the existing useHostCatalog.ts listener. Preserve the event dispatch and all other connection cleanup behavior.src/features/connections/presentation/useHostCatalog.ts (1)
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
PRIMARY_PROVIDERis hardcoded to'google'.The
HostLocationTagtype comment says "Extensible when more providers ship," but this hook only supports Google. When additional providers are added, this hook, the cache, and the readiness checks will need refactoring to iterate over multiple providers. Consider extracting the provider as a parameter or config now to avoid a larger refactor later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/connections/presentation/useHostCatalog.ts` at line 64, Make the provider used by the host catalog flow configurable instead of hardcoding PRIMARY_PROVIDER to 'google'. Thread the selected SyncProvider through the relevant hook, cache, and readiness-check logic, preserving Google as the default for current callers while allowing future providers without refactoring those paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/components/settings/tabs/vault/VaultSyncCard.tsx`:
- Around line 250-278: Remove isSyncing from isCollectionActionBlocked so Google
OAuth connection does not disable Upload/Restore actions. Keep isSyncing
included only in the provider-specific action gating, preserving blocking for
collection setup, unlock, lock, recovery-key regeneration, and domain
operations.
---
Nitpick comments:
In `@src/components/layout/Sidebar.tsx`:
- Around line 253-257: In the treeRoot useMemo, remove the no-op hostFilter
ternary and pass searchTerm directly to buildTree. Remove hostFilter from that
memo’s dependency array, unless a real per-filter search distinction is
required; if one is intended, implement the distinct remote behavior explicitly.
In `@src/components/layout/sidebar/VaultNavSection.tsx`:
- Around line 19-76: Consolidate the duplicated vault-state branching by
deriving status-dot output from resolveVaultAttention, or by merging both
results into a single shared resolver. Ensure the resulting logic still
distinguishes in-use errors, unlocked vaults with securable credentials, locked
vaults, and unconfigured vaults while preserving each existing className and
title.
In `@src/features/connections/domain/hostMaterialize.ts`:
- Around line 59-72: Track whether options.loadConnections() completes
successfully in the restore flow around formatConnectionsRestoreSuccessMessage.
When reloading fails, keep the error toast but suppress the subsequent success
toast; retain the existing success toast when reload succeeds, respecting
options.silentSuccess.
In `@src/features/connections/presentation/useHostCatalog.ts`:
- Line 64: Make the provider used by the host catalog flow configurable instead
of hardcoding PRIMARY_PROVIDER to 'google'. Thread the selected SyncProvider
through the relevant hook, cache, and readiness-check logic, preserving Google
as the default for current callers while allowing future providers without
refactoring those paths.
In `@src/store/connectionSlice.ts`:
- Around line 405-410: Remove the direct clearAllHostInventoryCaches()
invocation from the connection-clearing flow and rely on the
CONNECTIONS_CLEARED_EVENT dispatch to trigger the existing useHostCatalog.ts
listener. Preserve the event dispatch and all other connection cleanup behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73476610-30fd-4fd5-a131-47338733c1de
📒 Files selected for processing (40)
CHANGELOG.mdpackage.jsonsrc-tauri/src/sync/collection.rssrc-tauri/src/sync/commands.rssrc/components/icons/providerIcons.tsxsrc/components/layout/CommandPalette.tsxsrc/components/layout/Sidebar.tsxsrc/components/layout/TabBar.tsxsrc/components/layout/sidebar/ConnectionItem.tsxsrc/components/layout/sidebar/FolderItem.tsxsrc/components/layout/sidebar/HostLocationChips.tsxsrc/components/layout/sidebar/RemoteHostItem.tsxsrc/components/layout/sidebar/SidebarSection.tsxsrc/components/layout/sidebar/SplitSidebarActionButton.tsxsrc/components/layout/sidebar/VaultNavSection.tsxsrc/components/layout/sidebar/locationIcons.tsxsrc/components/layout/sidebar/types.tssrc/components/modals/AddConnectionModal.tsxsrc/components/settings/SettingsModal.tsxsrc/components/settings/tabs/VaultTab.tsxsrc/components/settings/tabs/vault/SyncCollectionSetupModal.tsxsrc/components/settings/tabs/vault/SyncDomainRow.tsxsrc/components/settings/tabs/vault/SyncProviderSetupSteps.tsxsrc/components/settings/tabs/vault/VaultStatusCard.tsxsrc/components/settings/tabs/vault/VaultSyncCard.tsxsrc/components/settings/tabs/vault/hooks/useConnectionsRestore.tssrc/components/settings/tabs/vault/hooks/useVaultPanelActions.tssrc/components/vault/RecoveryKeyModal.tsxsrc/features/connections/domain/hostCatalog.tssrc/features/connections/domain/hostInventoryCache.tssrc/features/connections/domain/hostMaterialize.tssrc/features/connections/domain/index.tssrc/features/connections/presentation/useHostCatalog.tssrc/store/connectionSlice.tssrc/vault/syncIpc.tssrc/vault/syncPassphrase.tssrc/vault/syncProviderGate.tssrc/vault/useSyncReadinessStore.tstests/hostCatalog.test.mjstsconfig.agent-tests.json
Strict missing-object handling for collection key-wrap download, validate wrap version and provider before apply, and normalize jump-chain host ids during filtered restore. Scope host inventory cache by account, clear it on disconnect, stop OAuth connect from blocking domain upload/restore, and tighten materialize toasts and vault nav attention resolution.
Record Unreleased fixes for key-wrap validation, jump-chain id normalize, account-scoped inventory cache, and domain action gating.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Style