Skip to content

fix(15.1): prevent logout race in search index init#200

Merged
FSM1 merged 2 commits into
mainfrom
fix/search-index-stale-rebuild
Feb 24, 2026
Merged

fix(15.1): prevent logout race in search index init#200
FSM1 merged 2 commits into
mainfrom
fix/search-index-stale-rebuild

Conversation

@FSM1

@FSM1 FSM1 commented Feb 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • Fixes a race condition where clearIndex() (on logout) fires while init() is suspended at await loadEncrypted(), causing stale state writes and post-logout key material persistence to IndexedDB
  • Adds cancelledRef that clearIndex sets to abort in-flight init after each await point
  • Includes todo for async/incremental search index (long-term) and E2E learning doc

Addresses: #198 (comment)

Test plan

  • Verify logout during search palette open doesn't leave stale isIndexReady=true
  • Verify no encrypted index persisted to IndexedDB after logout
  • Search still works normally after re-login

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Added guide for running end-to-end tests locally before pushing changes, including setup instructions and debugging tips.
  • Chores

    • Updated planning documentation with new tasks for vault features and search performance optimization.
    • Improved search initialization to handle cancellation during async operations.

FSM1 and others added 2 commits February 24, 2026 23:47
When clearIndex fires (on logout) while init() is suspended at
await loadEncrypted(), the resumed init would overwrite isIndexReady
with true and persist encrypted key material to IndexedDB after
the user has logged out.

Fix: add cancelledRef that clearIndex sets to true. init() checks
this flag after each await and bails out before writing state or
persisting. clearIndex also resets buildingRef to prevent deadlock
if logout interrupts mid-build.

Addresses: #198 (comment)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: be0b498729fa
- Todo: async/incremental search index build for large vaults
- Learning: run E2E locally before push

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: 6f54eaf3475f
@coderabbitai

coderabbitai Bot commented Feb 24, 2026

Copy link
Copy Markdown

Walkthrough

The PR adds documentation on running E2E tests locally, updates the planning state with two new pending tasks (CRDT IPNS inbox sharing and async incremental search indexing), introduces a new planning document detailing strategies for async search index building, and modifies the useSearch hook to track and handle cancellation of in-flight async initialization operations.

Changes

Cohort / File(s) Summary
Documentation & Planning
.learnings/2026-02-24-run-e2e-locally-before-push.md, .planning/STATE.md, .planning/todos/pending/2026-02-24-async-incremental-search-index.md
New learnings document on running E2E tests locally; planning state updated with two new pending tasks; new planning document proposing three incremental strategies (batching with setTimeout, Web Worker offload, or incremental updates via folder store subscriptions) to make search index build async for large vaults.
Hook Cancellation Logic
apps/web/src/hooks/useSearch.ts
Added cancelledRef tracking to prevent stale state updates from in-flight async operations (loadEncrypted, buildFromFolderTree); cancellation checks guard state writes on init path completion, folder build completion, and initialization errors; clearIndex() sets cancelledRef to abort pending operations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main fix: preventing a logout race condition in search index initialization by introducing cancellation tracking with cancelledRef.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/search-index-stale-rebuild

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/src/hooks/useSearch.ts (1)

153-163: ⚠️ Potential issue | 🟡 Minor

rebuildIndex calls persistEncrypted without checking cancelledRef.

If triggerSearchIndexRebuild fires just before logout (e.g., from a sync callback), rebuildIndex holds a non-null vaultKeypair in its closure and calls persistEncrypted without any cancellation guard. clearIndex may run concurrently while persistEncrypted's async writes are in flight.

The if (!vaultKeypair) return guard partially mitigates this (a new rebuildIndex with vaultKeypair = null is created after logout), but does not close the narrow window where the old closure is already executing with a captured non-null keypair.

🛡️ Proposed fix
  const rebuildIndex = useCallback(() => {
    if (!vaultKeypair) return;
+   if (cancelledRef.current) return;
    const currentFolders = useFolderStore.getState().folders;
    searchIndexService.buildFromFolderTree(currentFolders);
+   if (cancelledRef.current) return;
    setIsIndexReady(true);

    // Re-persist (fire-and-forget)
-   searchIndexService.persistEncrypted(vaultKeypair.privateKey).catch((err) => {
-     console.warn('[search] Failed to persist index after rebuild:', err);
-   });
+   if (!cancelledRef.current) {
+     searchIndexService.persistEncrypted(vaultKeypair.privateKey).catch((err) => {
+       console.warn('[search] Failed to persist index after rebuild:', err);
+     });
+   }
  }, [vaultKeypair]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/hooks/useSearch.ts` around lines 153 - 163, rebuildIndex can
capture a non-null vaultKeypair and call searchIndexService.persistEncrypted
even after logout; fix by checking the cancellation flag before and immediately
after the async call: inside rebuildIndex read cancelledRef.current (or capture
it) and return if true before calling persistEncrypted, and in the
fire-and-forget promise chain check cancelledRef.current again before handling
success/failure (or skip calling persistEncrypted entirely if cancelled), so
that persistEncrypted is never invoked when clearIndex/logout has set
cancelledRef. Reference functions/vars: rebuildIndex, persistEncrypted,
cancelledRef, clearIndex, vaultKeypair, and ensure the check is performed inside
the useCallback closure to avoid the captured stale vaultKeypair window.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web/src/hooks/useSearch.ts`:
- Line 96: persistEncrypted can race with clearIndex because it's called
fire-and-forget and its async writes may execute after cancelledRef.current is
set true by clearIndex; update persistEncrypted to accept an AbortSignal (or
take cancelledRef) and check signal.aborted (or cancelledRef.current)
immediately before performing the final IndexedDB write so it aborts the write
when cancelled, then change callers (the places that call persistEncrypted from
useSearch: where cancelledRef is set and where rebuildIndex triggers
persistEncrypted) to pass an AbortSignal (or re-check cancelledRef after
awaiting any asynchronous steps) so rebuildIndex and the fire-and-forget
persistEncrypted won't write stale encrypted data after
searchIndexService.clear() runs; ensure clearIndex creates/uses the same
AbortController to signal cancellation to any in-flight persistEncrypted calls.

---

Outside diff comments:
In `@apps/web/src/hooks/useSearch.ts`:
- Around line 153-163: rebuildIndex can capture a non-null vaultKeypair and call
searchIndexService.persistEncrypted even after logout; fix by checking the
cancellation flag before and immediately after the async call: inside
rebuildIndex read cancelledRef.current (or capture it) and return if true before
calling persistEncrypted, and in the fire-and-forget promise chain check
cancelledRef.current again before handling success/failure (or skip calling
persistEncrypted entirely if cancelled), so that persistEncrypted is never
invoked when clearIndex/logout has set cancelledRef. Reference functions/vars:
rebuildIndex, persistEncrypted, cancelledRef, clearIndex, vaultKeypair, and
ensure the check is performed inside the useCallback closure to avoid the
captured stale vaultKeypair window.

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3236f4a and 8535ade.

📒 Files selected for processing (4)
  • .learnings/2026-02-24-run-e2e-locally-before-push.md
  • .planning/STATE.md
  • .planning/todos/pending/2026-02-24-async-incremental-search-index.md
  • apps/web/src/hooks/useSearch.ts

Comment thread apps/web/src/hooks/useSearch.ts
@FSM1 FSM1 merged commit 11abcfa into main Feb 24, 2026
21 checks passed
@FSM1 FSM1 deleted the fix/search-index-stale-rebuild branch March 3, 2026 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant