Fix vault sync loading state and stale closure bug#65
Conversation
Moved to done/, beginning implementation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, returning users saw a misleading "EMPTY DIRECTORY" for 30+ seconds while IPNS resolved. Three issues contributed: 1. Stale closure in handleSync captured folders from render cycle — root folder wasn't in the closure yet on initial load. Fixed by reading useFolderStore.getState() directly inside the async callback. 2. useInterval doesn't fire immediately — added useEffect to trigger sync on first mount instead of waiting 30s for setInterval. 3. No distinction between "syncing" and "empty vault" — added initialSyncComplete tracking to sync store and isNewVault flag to vault store. Shows terminal-style syncing animation until IPNS resolves; new vaults skip straight to empty state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request addresses vault synchronization loading state issues on login by introducing stale closure fixes, initial sync tracking, and an "isNewVault" flag to differentiate new vaults from returning ones. It adds UI feedback with styling, state management enhancements, and immediate sync triggering on mount. Changes
Sequence DiagramsequenceDiagram
actor User
participant Auth as useAuth Hook
participant VaultStore as Vault Store
participant SyncStore as Sync Store
participant Polling as useSyncPolling
participant FileBrowser as FileBrowser UI
User->>Auth: Login / Create Vault
alt New Vault
Auth->>VaultStore: setVaultKeys({isNewVault: true})
VaultStore->>VaultStore: isNewVault = true
else Returning Vault
Auth->>VaultStore: setVaultKeys({isNewVault: false})
VaultStore->>VaultStore: isNewVault = false
end
Auth->>SyncStore: reset()
SyncStore->>SyncStore: initialSyncComplete = false
Auth->>Polling: Hook mounted with new vault state
Polling->>Polling: Trigger immediate sync on mount
alt New Vault Path
Polling->>SyncStore: completeInitialSync()
SyncStore->>SyncStore: initialSyncComplete = true
else Returning Vault Path
Polling->>Polling: Poll until sync succeeds
Polling->>Polling: Then completeInitialSync()
Polling->>SyncStore: completeInitialSync()
SyncStore->>SyncStore: initialSyncComplete = true
end
SyncStore-->>FileBrowser: initialSyncComplete = true
FileBrowser->>FileBrowser: Hide vault syncing UI
FileBrowser->>FileBrowser: Show folder contents or empty state
FileBrowser-->>User: Vault ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/components/file-browser/FileBrowser.tsx`:
- Around line 353-368: The animated loader `.vault-syncing-bar-fill` uses
`animation: vault-syncing-slide ... infinite` but lacks a prefers-reduced-motion
rule; add a `@media (prefers-reduced-motion: reduce)` block in the component's
stylesheet (the CSS module / styles where `.vault-syncing-bar-fill` is defined)
that sets `.vault-syncing-bar-fill { animation: none; }` so users who prefer
reduced motion won't see the continuous `vault-syncing-slide` animation.
🧹 Nitpick comments (2)
apps/web/src/hooks/useSyncPolling.ts (1)
76-81: Consider potential double-sync on mount.The immediate-on-mount effect fires
doSync()once via theinitialSyncFiredref guard. However,doSyncis in the dependency array and is recreated on every render where its own deps change. Since the ref guard prevents re-firing, this is safe in practice. But note that ifuseIntervalfires its first tick very close to mount (unlikely with 30s), there's no issue sinceisSyncingRefguards concurrency.One minor observation: because
doSyncis a dependency, React's exhaustive-deps lint is satisfied, but the intent is clearly "run once on mount." A comment noting that the ref makes this effectively a one-shot despitedoSyncbeing in deps would help future readers — though this is optional.apps/web/src/components/file-browser/FileBrowser.tsx (1)
354-358: Minor a11y: consider adding an accessible status announcement for screen readers.The vault syncing block uses
aria-hidden="true"on the<pre>ASCII art (good), but there's norole="status"oraria-live="polite"region to announce the syncing state to screen reader users. The<p>elements with the status text could benefit from being in a live region.♿ Proposed a11y improvement
- <div className="vault-syncing" data-testid="vault-syncing"> + <div className="vault-syncing" data-testid="vault-syncing" role="status" aria-live="polite">
There was a problem hiding this comment.
Pull request overview
Improves the web vault’s initial synchronization experience and fixes a React/Zustand stale-closure issue that delayed session-restore file loading, while ensuring sync state is correctly cleared on logout/token-refresh failure.
Changes:
- Fixes stale closure in
handleSyncby reading the latest folder state viauseFolderStore.getState(), and triggers an immediate sync on mount (not after the first 30s interval). - Adds an initial vault-sync loading UI driven by new
initialSyncComplete(sync store) andisNewVault(vault store) flags. - Resets sync store state on logout and on refresh-token failure, alongside existing vault/folder store clears.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/styles/file-browser.css | Adds styles for the new “vault syncing” loading state UI. |
| apps/web/src/stores/vault.store.ts | Introduces isNewVault flag to distinguish first-time vaults from returning users. |
| apps/web/src/stores/sync.store.ts | Adds initialSyncComplete tracking and action to control initial-sync UX. |
| apps/web/src/lib/api/client.ts | Resets sync store on refresh-token failure before logging out. |
| apps/web/src/hooks/useSyncPolling.ts | Fires initial sync immediately; completes initial sync for new vaults. |
| apps/web/src/hooks/useAuth.ts | Sets isNewVault on vault creation; resets sync store on logout paths. |
| apps/web/src/components/file-browser/FileBrowser.tsx | Uses fresh store state in sync callback; renders initial-sync loading UI. |
| .planning/todos/done/2026-02-08-vault-sync-loading-state-ux.md | Adds planning record for the UX change. |
| .planning/STATE.md | Updates pending/completed todo counts. |
| .learnings/2026-02-09-vault-sync-loading-state.md | Captures the root-cause analysis and lessons learned. |
- Add prefers-reduced-motion media query to disable vault syncing animation - Propagate errors during initial sync so loading UI stays visible on failure - Skip unnecessary IPNS resolve for new vaults on mount - Add role="status" aria-live="polite" for screen reader a11y - Add sync store unit tests for initialSyncComplete lifecycle Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
handleSynccapturedfoldersfrom the React render cycle; root folder wasn't in the closure on initial load. Now readsuseFolderStore.getState()directly.setIntervalto tick.initialSyncCompletetracking in sync store andisNewVaultflag in vault store. Shows terminal-style animated progress bar while IPNS resolves; new vaults skip straight to empty state.Test plan
pnpm --filter web build)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes