From 9047d939041f2d4a46ca891868210acedce105a9 Mon Sep 17 00:00:00 2001 From: Harsh23Kashyap <55448981+Harsh23Kashyap@users.noreply.github.com> Date: Sat, 1 Aug 2026 12:48:49 +0530 Subject: [PATCH 1/3] fix(web): scope file-search recents to (repo, revision) in the browse dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #1387. --- .../browse/components/fileSearchCommandDialog.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx index ac743d231..761fe2ee5 100644 --- a/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx +++ b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx @@ -35,7 +35,16 @@ export const FileSearchCommandDialog = () => { const [searchQuery, setSearchQuery] = useState(''); const { navigateToPath } = useBrowseNavigation(); - const [recentlyOpened, setRecentlyOpened] = useLocalStorage(`recentlyOpenedFiles-${repoName}`, []); + // Scope the recents to the (repo, revision) tuple so a switch + // branches doesn't carry over suggestions from another revision + // that may not exist on the current one. 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. Issue #1387. + const [recentlyOpened, setRecentlyOpened] = useLocalStorage( + `recentlyOpenedFiles-${repoName}-${revisionName ?? 'HEAD'}`, + [], + ); useHotkeys("mod+p", (event) => { event.preventDefault(); From 3d1938dd1733c90b8ed9b25cae5f6b08aad25cb9 Mon Sep 17 00:00:00 2001 From: Harsh23Kashyap <55448981+Harsh23Kashyap@users.noreply.github.com> Date: Sat, 1 Aug 2026 12:48:49 +0530 Subject: [PATCH 2/3] test(web): cover the per-revision recents key in the browse file-search 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 #1387. --- CHANGELOG.md | 1 + .../fileSearchCommandDialog.test.tsx | 122 ++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index d487b5b8c..820fd7514 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Upgraded `brace-expansion` to `^1.1.17`/`^2.1.3`/`^5.0.8`. [#1527](https://github.com/sourcebot-dev/sourcebot/pull/1527) +- The browse file-search dialog's recently-opened files are now scoped to the `(repo, revision)` tuple, so switching branches in the same repo no longer carries over suggestions from a different revision that may not exist on the current one. [#1387](https://github.com/sourcebot-dev/sourcebot/issues/1387) ## [5.1.5] - 2026-07-31 diff --git a/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx new file mode 100644 index 000000000..36984625c --- /dev/null +++ b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx @@ -0,0 +1,122 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render } from '@testing-library/react'; + +// Stub `useLocalStorage` so the test can capture the key the component +// passes and assert on it. Without this, we'd be asserting on real +// localStorage (which jsdom does provide, but the key changes per +// revision and the test only wants to verify the key format). +const capturedKeys: string[] = []; +vi.mock('usehooks-ts', () => ({ + useLocalStorage: (key: string, initial: T) => { + capturedKeys.push(key); + return [initial, vi.fn()] as [T, (v: T) => void]; + }, +})); + +// Make the params mock swappable per-test. We use a single mutable +// holder so each `it` can set its own (repoName, revisionName) tuple +// before rendering. +const mockParams: { repoName: string; revisionName: string | undefined } = { + repoName: 'github.com/foo/bar', + revisionName: 'main', +}; +vi.mock('@/app/(app)/browse/hooks/useBrowseParams', () => ({ + useBrowseParams: () => ({ repoName: mockParams.repoName, revisionName: mockParams.revisionName }), +})); +vi.mock('@/app/(app)/browse/hooks/useBrowseState', () => ({ + useBrowseState: () => ({ state: { isFileSearchOpen: false }, updateBrowseState: vi.fn() }), +})); +vi.mock('@/app/(app)/browse/hooks/useBrowseNavigation', () => ({ + useBrowseNavigation: () => ({ navigateToPath: vi.fn() }), +})); + +vi.mock('next/navigation', () => ({ + usePathname: () => '/browse/github.com/foo/bar@main/-/tree/src', +})); + +vi.mock('@tanstack/react-query', () => ({ + useQuery: () => ({ data: [], isLoading: false, isError: false }), +})); + +vi.mock('@/app/api/(client)/client', () => ({ + getFiles: vi.fn(), +})); + +vi.mock('react-hotkeys-hook', () => ({ + useHotkeys: vi.fn(), +})); + +vi.mock('@/app/(app)/browse/components/fileTreeItemIcon', () => ({ + FileTreeItemIcon: () => null, +})); + +vi.mock('@/components/ui/command', () => ({ + Command: ({ children }: { children: React.ReactNode }) => <>{children}, + CommandEmpty: ({ children }: { children: React.ReactNode }) => <>{children}, + CommandGroup: ({ children }: { children: React.ReactNode }) => <>{children}, + CommandInput: () => null, + CommandItem: ({ children }: { children: React.ReactNode }) => <>{children}, + CommandList: ({ children }: { children: React.ReactNode }) => <>{children}, +})); + +vi.mock('@/components/ui/dialog', () => ({ + Dialog: ({ children }: { children: React.ReactNode }) => <>{children}, + DialogContent: ({ children }: { children: React.ReactNode }) => <>{children}, + DialogDescription: () => null, + DialogTitle: () => null, +})); + +const { FileSearchCommandDialog } = await import('./fileSearchCommandDialog'); + +const renderWithParams = () => { + capturedKeys.length = 0; + render(); + return capturedKeys[0]; +}; + +describe('FileSearchCommandDialog recents key (issue #1387)', () => { + afterEach(() => { + // Reset the mock params to the default between tests so a + // mutation in one test doesn't leak into the next. + mockParams.repoName = 'github.com/foo/bar'; + mockParams.revisionName = 'main'; + }); + + test('scopes the recents localStorage key to the (repo, revision) tuple', () => { + // The previous key was `recentlyOpenedFiles-${repoName}` — same + // across revisions. After the fix, the key is scoped per + // revision so switching branches doesn't carry over suggestions + // from another revision that may not exist on the current one. + mockParams.repoName = 'github.com/foo/bar'; + mockParams.revisionName = 'main'; + const key = renderWithParams(); + expect(key).toBe('recentlyOpenedFiles-github.com/foo/bar-main'); + }); + + test('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. + mockParams.repoName = 'github.com/foo/bar'; + mockParams.revisionName = undefined; + const key = renderWithParams(); + expect(key).toBe('recentlyOpenedFiles-github.com/foo/bar-HEAD'); + }); + + test('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. This test asserts that the + // keys differ for the two revisions. + mockParams.repoName = 'github.com/foo/bar'; + mockParams.revisionName = 'main'; + const keyMain = renderWithParams(); + + mockParams.revisionName = 'feature/foo'; + const keyFeature = renderWithParams(); + + expect(keyMain).toBe('recentlyOpenedFiles-github.com/foo/bar-main'); + expect(keyFeature).toBe('recentlyOpenedFiles-github.com/foo/bar-feature/foo'); + expect(keyMain).not.toBe(keyFeature); + }); +}); From 4b0eb9aeeb8ef4f71c007c12faa8ad3c91c7d572 Mon Sep 17 00:00:00 2001 From: Harsh23Kashyap <55448981+Harsh23Kashyap@users.noreply.github.com> Date: Sun, 2 Aug 2026 12:26:22 +0530 Subject: [PATCH 3/3] fix(web): encode the recents key with JSON to avoid boundary ambiguity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit finding on PR #1529: the previous fix used a `-` 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 #1529. --- CHANGELOG.md | 2 +- .../fileSearchCommandDialog.test.tsx | 24 +++++++++++++++---- .../components/fileSearchCommandDialog.tsx | 6 +++-- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 820fd7514..30f98e193 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Upgraded `brace-expansion` to `^1.1.17`/`^2.1.3`/`^5.0.8`. [#1527](https://github.com/sourcebot-dev/sourcebot/pull/1527) -- The browse file-search dialog's recently-opened files are now scoped to the `(repo, revision)` tuple, so switching branches in the same repo no longer carries over suggestions from a different revision that may not exist on the current one. [#1387](https://github.com/sourcebot-dev/sourcebot/issues/1387) +- The browse file-search dialog's recently-opened files are now scoped to the `(repo, revision)` tuple, so switching branches in the same repo no longer carries over suggestions from a different revision that may not exist on the current one. [#1529](https://github.com/sourcebot-dev/sourcebot/pull/1529) ## [5.1.5] - 2026-07-31 diff --git a/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx index 36984625c..a5d6a1b30 100644 --- a/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx +++ b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.test.tsx @@ -90,7 +90,7 @@ describe('FileSearchCommandDialog recents key (issue #1387)', () => { mockParams.repoName = 'github.com/foo/bar'; mockParams.revisionName = 'main'; const key = renderWithParams(); - expect(key).toBe('recentlyOpenedFiles-github.com/foo/bar-main'); + expect(key).toBe('recentlyOpenedFiles::["github.com/foo/bar","main"]'); }); test('uses the HEAD fallback for the "no revision in URL" case', () => { @@ -100,7 +100,7 @@ describe('FileSearchCommandDialog recents key (issue #1387)', () => { mockParams.repoName = 'github.com/foo/bar'; mockParams.revisionName = undefined; const key = renderWithParams(); - expect(key).toBe('recentlyOpenedFiles-github.com/foo/bar-HEAD'); + expect(key).toBe('recentlyOpenedFiles::["github.com/foo/bar","HEAD"]'); }); test('produces a different key for a different revision in the same repo', () => { @@ -115,8 +115,24 @@ describe('FileSearchCommandDialog recents key (issue #1387)', () => { mockParams.revisionName = 'feature/foo'; const keyFeature = renderWithParams(); - expect(keyMain).toBe('recentlyOpenedFiles-github.com/foo/bar-main'); - expect(keyFeature).toBe('recentlyOpenedFiles-github.com/foo/bar-feature/foo'); + expect(keyMain).toBe('recentlyOpenedFiles::["github.com/foo/bar","main"]'); + expect(keyFeature).toBe('recentlyOpenedFiles::["github.com/foo/bar","feature/foo"]'); expect(keyMain).not.toBe(keyFeature); }); + + test('does not collide for tuples whose naive-dash concatenation would be ambiguous', () => { + // CodeRabbit finding: a naive `-` format + // collides for tuples like (`foo-bar`, `baz`) vs (`foo`, `bar-baz`). + // The JSON-encoded format is collision-free by construction, so + // this test asserts that two such tuples produce different keys. + mockParams.repoName = 'foo-bar'; + mockParams.revisionName = 'baz'; + const keyA = renderWithParams(); + + mockParams.repoName = 'foo'; + mockParams.revisionName = 'bar-baz'; + const keyB = renderWithParams(); + + expect(keyA).not.toBe(keyB); + }); }); diff --git a/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx index 761fe2ee5..8809a331b 100644 --- a/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx +++ b/packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx @@ -40,9 +40,11 @@ export const FileSearchCommandDialog = () => { // that may not exist on the current one. 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. Issue #1387. + // in URL" case. The components are JSON-encoded to avoid + // boundary ambiguity (e.g. `foo-bar` + `baz` vs `foo` + `bar-baz` + // would otherwise produce the same key). Issue #1387. const [recentlyOpened, setRecentlyOpened] = useLocalStorage( - `recentlyOpenedFiles-${repoName}-${revisionName ?? 'HEAD'}`, + `recentlyOpenedFiles::${JSON.stringify([repoName, revisionName ?? 'HEAD'])}`, [], );