fix(web): scope recent files to the browse revision - #1686
dipeshbabu wants to merge 3 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. WalkthroughFile search recents are now stored by repository and revision. A missing revision uses ChangesFile search recents
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to Recent files are scoped to the browse revision, and the supplied evidence identifies no issue requiring a fix before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx">
<violation number="1" location="packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx:39">
P3: Each distinct (repo, revision) pair — including every commit SHA the user browses at — permanently creates a new localStorage entry that is never evicted, and legacy `recentlyOpenedFiles-<repo>` keys are intentionally left in place too. Storage clutter grows without bound over time for active browsing. Consider capping the recents array length and pruning keys for revisions no longer reachable, e.g. store one entry per repo holding a `Map`/object of revision → files instead of one key per revision.</violation>
<violation number="2" location="packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx:39">
P2: Scoping the key by revision closes the cross-revision case, but the "Recently opened" list is still rendered as a stored snapshot without validating any entry against the files that currently exist at that revision (`recentlyOpened.map(...)` runs with no cross-check against `files`). If the revision's content moved since the file was opened — a branch advanced and deleted/renamed the path — selecting the entry calls `navigateToPath` to a blob that no longer exists, which is the same end-user failure mode #1387 describes, just narrowed to the matching key. Filter `recentlyOpened` against the current `files` (or remove missing paths on read), e.g. `recentlyOpened.filter(r => files.some(f => f.path === r.path))`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(`recentlyOpenedFiles-${repoName}`, []); | ||
| const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>( | ||
| `recentlyOpenedFiles-${JSON.stringify([repoName, revisionName ?? 'HEAD'])}`, |
There was a problem hiding this comment.
P2: Scoping the key by revision closes the cross-revision case, but the "Recently opened" list is still rendered as a stored snapshot without validating any entry against the files that currently exist at that revision (recentlyOpened.map(...) runs with no cross-check against files). If the revision's content moved since the file was opened — a branch advanced and deleted/renamed the path — selecting the entry calls navigateToPath to a blob that no longer exists, which is the same end-user failure mode #1387 describes, just narrowed to the matching key. Filter recentlyOpened against the current files (or remove missing paths on read), e.g. recentlyOpened.filter(r => files.some(f => f.path === r.path)).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx, line 39:
<comment>Scoping the key by revision closes the cross-revision case, but the "Recently opened" list is still rendered as a stored snapshot without validating any entry against the files that currently exist at that revision (`recentlyOpened.map(...)` runs with no cross-check against `files`). If the revision's content moved since the file was opened — a branch advanced and deleted/renamed the path — selecting the entry calls `navigateToPath` to a blob that no longer exists, which is the same end-user failure mode #1387 describes, just narrowed to the matching key. Filter `recentlyOpened` against the current `files` (or remove missing paths on read), e.g. `recentlyOpened.filter(r => files.some(f => f.path === r.path))`.</comment>
<file context>
@@ -35,7 +35,10 @@ export const FileSearchCommandDialog = () => {
- const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(`recentlyOpenedFiles-${repoName}`, []);
+ const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(
+ `recentlyOpenedFiles-${JSON.stringify([repoName, revisionName ?? 'HEAD'])}`,
+ [],
+ );
</file context>
|
|
||
| const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(`recentlyOpenedFiles-${repoName}`, []); | ||
| const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>( | ||
| `recentlyOpenedFiles-${JSON.stringify([repoName, revisionName ?? 'HEAD'])}`, |
There was a problem hiding this comment.
P3: Each distinct (repo, revision) pair — including every commit SHA the user browses at — permanently creates a new localStorage entry that is never evicted, and legacy recentlyOpenedFiles-<repo> keys are intentionally left in place too. Storage clutter grows without bound over time for active browsing. Consider capping the recents array length and pruning keys for revisions no longer reachable, e.g. store one entry per repo holding a Map/object of revision → files instead of one key per revision.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/web/src/app/(app)/browse/components/fileSearchCommandDialog.tsx, line 39:
<comment>Each distinct (repo, revision) pair — including every commit SHA the user browses at — permanently creates a new localStorage entry that is never evicted, and legacy `recentlyOpenedFiles-<repo>` keys are intentionally left in place too. Storage clutter grows without bound over time for active browsing. Consider capping the recents array length and pruning keys for revisions no longer reachable, e.g. store one entry per repo holding a `Map`/object of revision → files instead of one key per revision.</comment>
<file context>
@@ -35,7 +35,10 @@ export const FileSearchCommandDialog = () => {
- const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(`recentlyOpenedFiles-${repoName}`, []);
+ const [recentlyOpened, setRecentlyOpened] = useLocalStorage<FileTreeItem[]>(
+ `recentlyOpenedFiles-${JSON.stringify([repoName, revisionName ?? 'HEAD'])}`,
+ [],
+ );
</file context>
This branch has not been deployed
1 participant
Fixes #1387
Opening file search after changing branches could show recently opened files from another revision and navigate to paths that do not exist on the current branch. Store recent files by repository and revision, with omitted revisions sharing the explicit HEAD history.
The storage key encodes the pair without separator collisions. Existing repository-only history is left untouched and is not imported because its revision is unknown. Regression tests cover switching revisions, restoring history after remounting, default HEAD, legacy storage, and distinct repository/revision pairs.
Validation completed before opening this PR:
Note
Low Risk
Client-only browse UI localStorage key change with no auth or server impact; users may see empty recents until they reopen files on each revision.
Overview
Fixes browse file search showing recently opened entries from another branch/revision after you switch refs, which could send navigation to paths that do not exist on the current revision.
Recent history is now stored in
localStorageunder a key that includes both repository and revision (revisionName, orHEADwhen omitted so default and explicit HEAD share one list). Legacy keys that only keyed by repo are not migrated because their revision is unknown. New Vitest coverage exercises revision switching, remount persistence, HEAD aliasing, legacy storage, and repo/revision names that could collide with naive string joining.Reviewed by Cursor Bugbot for commit ab293d7. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes #1387 by scoping recently opened files to the current repository and revision, so switching branches no longer surfaces or navigates to files from another revision.
repoNameandrevisionName, with omitted revisions mapped toHEADso they share history.Written for commit ab293d7. Summary will update on new commits.
Summary by CodeRabbit