Skip to content

fix(web): scope file-search recents to (repo, revision) in the browse dialog - #1529

Open
Harsh23Kashyap wants to merge 3 commits into
sourcebot-dev:mainfrom
Harsh23Kashyap:fix/file-search-recents-revision-scoped
Open

fix(web): scope file-search recents to (repo, revision) in the browse dialog#1529
Harsh23Kashyap wants to merge 3 commits into
sourcebot-dev:mainfrom
Harsh23Kashyap:fix/file-search-recents-revision-scoped

Conversation

@Harsh23Kashyap

@Harsh23Kashyap Harsh23Kashyap commented Aug 1, 2026

Copy link
Copy Markdown

Summary

The browse file-search dialog (mod+p) stored recently opened files in localStorage under a key scoped only by repoName. A user who opened a file on main, switched to feature/foo, and re-opened the dialog would see recents from main — paths that may not exist on the new revision. Selecting one navigated with the new revision and landed on a 404.

Fix: scope the localStorage key to the (repoName, revisionName) tuple, so a branch switch in the same repo yields a fresh recents list scoped to the new revision.

Fixes #1387.

What changes

packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx:38 — the useLocalStorage key changes from `recentlyOpenedFiles-${repoName}` to `recentlyOpenedFiles-${repoName}-${revisionName ?? 'HEAD'}`. The 'HEAD' default matches the file-fetch fallback on the next line, so the recents key and the file list key agree on the "no revision in URL" case.

The setRecentlyOpened callback does not need to change — useLocalStorage from usehooks-ts writes to whatever key the current key string resolves to. Switching revisions automatically re-renders the dialog with the new key, which starts empty (or with whatever recents the user previously stored under that exact (repo, revision) tuple).

Old keys (`recentlyOpenedFiles-<repo>`) become orphaned entries in localStorage and are ignored. No migration is required — the worst case is the user loses the recents accumulated under the old key, which is the bug being fixed.

Files

  • packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx — the key-derivation change (one line, plus a 4-line comment explaining the rationale).
  • packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx — 3 new vitest cases.
  • CHANGELOG.md — one-sentence entry under [Unreleased] → Fixed.

Why this is in scope

The component already reads revisionName from useBrowseParams() on the line above and uses it to fetch the file list (getFiles({ repoName, revisionName: revisionName ?? 'HEAD' })), so the data was already revision-scoped. The localStorage key was the only thing that wasn't. The fix is a single key-derivation change that brings the persistence layer in line with the data layer that's already there.

Test coverage

3 vitest cases in fileSearchCommandDialog.test.tsx:

  • Scopes the recents localStorage key to the (repo, revision) tuple. Asserts the key is recentlyOpenedFiles-github.com/foo/bar-main for (github.com/foo/bar, main). The regression assertion for the bug.
  • Uses the HEAD fallback for the "no revision in URL" case. The file-fetch on the next line uses revisionName ?? 'HEAD' as the default; the recents key needs to agree so the user sees a consistent recents list for the "default branch" view. Asserts the key is recentlyOpenedFiles-github.com/foo/bar-HEAD when revisionName is undefined.
  • Produces a different key for a different revision in the same repo. The bug was: switching from main to feature/foo showed recents from main. After the fix, the keys differ, so the recents are naturally scoped. Asserts main and feature/foo produce different keys for the same repo.

The test stubs useLocalStorage from usehooks-ts to capture the key the component passes (without this, we'd be asserting on real localStorage, which jsdom does provide but is per-test mutable state that's harder to reason about). The other hooks the dialog uses (useBrowseParams, useBrowseState, useQuery, etc.) are stubbed so the test only exercises the key derivation.

3/3 tests pass; 7 pre-existing OpenTelemetry-setup failures in ee/askmcp/... and ee/permissionSyncStatus/... are unchanged by this PR (they fail to load due to the OTel SDK version mismatch, before any of my code runs).

Backward compatibility

Pure behaviour change. Old keys' data is left in localStorage and ignored, so a user upgrading from a previous version simply starts with an empty recents list on their next branch switch. No data loss on the server side.

Risks

Minimal. The default 'HEAD' matches the file-fetch default on the next line, so the recents key and the file list key agree on the "no revision in URL" case. If a future change splits 'HEAD' and undefined semantics, both sides will need to be updated in lockstep — but that's already true for the file fetch.

Future work

  • A "clear recents" UI button next to the recents list. Not in scope for this PR; the bug was about cross-revision leakage, not about retention control.
  • Migrating recents to the server (Prisma userRecentFile table scoped by (userId, repoId, revisionName)). Punted: a localStorage key is the lowest-cost fix and matches the existing pattern. If a user clears their browser data they lose their recents, but they also lose their session, so this is fine.

Note

Low Risk
Client-only localStorage key change in the browse file-search dialog; no auth, API, or server impact beyond users starting fresh recents per revision.

Overview
Fixes browse mod+p file search showing recently opened paths from another branch after a revision switch (issue #1387).

FileSearchCommandDialog now persists recents under a localStorage key derived from [repoName, revisionName ?? 'HEAD'] via recentlyOpenedFiles::${JSON.stringify(...)}, instead of repo name only. That aligns recents with the same revision default used for getFiles, and avoids ambiguous keys that a simple repo-revision string could collide on. Prior repo-only keys are left unused (no migration).

Adds fileSearchCommandDialog.test.tsx (Vitest) to lock the key format, HEAD when revision is missing, per-revision separation, and collision safety. CHANGELOG documents the fix under [Unreleased].

Reviewed by Cursor Bugbot for commit 4b0eb9a. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes
    • Browse file-search suggestions and recently opened files are now scoped to the selected repository and revision.
    • Suggestions no longer carry over when switching branches or revisions.
    • Searches without a specified revision consistently use HEAD.

… dialog

The browse file-search dialog (mod+p) stored recently opened files in
localStorage under a key scoped only by repoName. A user who opened
a file on `main`, switched to `feature/foo`, and re-opened the dialog
would see recents from `main` — paths that may not exist on the new
revision. Selecting one navigated with the new revision and landed on
a 404.

Change the localStorage key to `recentlyOpenedFiles-${repoName}-${revisionName ?? 'HEAD'}` so the recents are naturally scoped per (repo, revision) tuple. The 'HEAD' default matches the file-fetch fallback on the next line, so the recents key and the file list key agree on the "no revision in URL" case.

Old keys become orphaned entries in localStorage and are ignored. No migration is required.

Fixes sourcebot-dev#1387.
…ch dialog

Three vitest cases in fileSearchCommandDialog.test.tsx:

- Scopes the recents localStorage key to the (repo, revision) tuple
  (e.g. `recentlyOpenedFiles-github.com/foo/bar-main`).
- Uses the `HEAD` fallback for the "no revision in URL" case, so the
  recents key and the file-list key agree.
- Produces a different key for a different revision in the same
  repo, which is the regression assertion for the bug.

The test stubs `useLocalStorage` from `usehooks-ts` to capture the
key the component passes (without this, we'd be asserting on real
localStorage, which jsdom does provide but is per-test mutable state
that's harder to reason about). The other hooks the dialog uses
(useBrowseParams, useBrowseState, useQuery, etc.) are stubbed so the
test only exercises the key derivation.

Plus a one-line CHANGELOG entry under [Unreleased] -> Fixed.

Refs sourcebot-dev#1387.
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The file-search dialog now scopes recently opened files by repository and revision. It uses HEAD when no revision is set. Tests verify key generation and revision separation. The changelog documents the fix.

Changes

File search recents

Layer / File(s) Summary
Scope recents by repository and revision
packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx
The localStorage key now uses a JSON-encoded repository and revision pair. The revision defaults to HEAD.
Validate storage-key scoping
packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx, CHANGELOG.md
Tests verify repository and revision scoping, the HEAD fallback, distinct revision keys, and collision-resistant encoding. The changelog records the fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes scope file-search recents by repository and revision, including a HEAD fallback, which satisfies issue #1387.
Out of Scope Changes check ✅ Passed The changelog update and focused tests directly support the revision-scoped file-search recents change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes scoping file-search recents by repository and revision in the browse dialog.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@CHANGELOG.md`:
- Line 12: Update the changelog entry’s trailing GitHub reference to use the
current pull request ID and the repository’s /pull/<id> URL instead of the
existing issue URL, preserving the required
[#<id>](https://github.com/sourcebot-dev/sourcebot/pull/<id>) format.

In `@packages/web/src/app/`(app)/browse/components/fileSearchCommandDialog.tsx:
- Around line 44-46: Update the recentlyOpened useLocalStorage key in the file
search command dialog to encode the repoName and revisionName tuple without
ambiguity, using serialization or another collision-free format while preserving
the HEAD fallback. Add a regression test covering the specified
repository/revision pairs and verify they produce distinct storage keys.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0f06252b-d561-44c8-a88a-37ed022e0f22

📥 Commits

Reviewing files that changed from the base of the PR and between 39bf1a0 and 3d1938d.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx
  • packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx

Comment thread CHANGELOG.md Outdated
CodeRabbit finding on PR sourcebot-dev#1529: the previous fix used a
`<repoName>-<revisionName>` template, which collides for tuples like
(`foo-bar`, `baz`) and (`foo`, `bar-baz`) — both produce the key
`recentlyOpenedFiles-foo-bar-baz`. Both components can contain `-`
(GitHub branch names, repo paths), so the boundary is ambiguous.

Switch to a JSON-encoded tuple: `recentlyOpenedFiles::${JSON.stringify([repoName, revisionName ?? 'HEAD'])}`. JSON.stringify of a 2-element array is a unique representation of the tuple, so the key is collision-free regardless of which characters appear in the components.

The `'HEAD'` default and the `::` prefix are preserved from the previous fix. The CHANGELOG link is also corrected to point at the PR (was the issue URL, per CodeRabbit).

Adds a 4th test case that asserts two ambiguous tuples produce different keys.

Refs sourcebot-dev#1529.
@Harsh23Kashyap

Copy link
Copy Markdown
Author

Both CodeRabbit inline comments addressed in 4b0eb9ae:

  • CHANGELOG link — fixed; was the issue URL (#1387), now points at the PR (#1529).
  • Collision-free key encoding — the previous <repoName>-<revisionName> template collides for tuples like (foo-bar, baz) vs (foo, bar-baz) since both components can contain -. Switched to recentlyOpenedFiles::${JSON.stringify([repoName, revisionName ?? 'HEAD'])}. JSON.stringify of a 2-element array is a unique representation of the tuple, so the key is collision-free regardless of which characters appear in the components. The :: prefix and HEAD default are preserved.

Added a 4th test case that asserts the two ambiguous tuples above produce different keys.

4/4 tests pass.

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.

File search recents are shared across browse revisions

1 participant