fix: shared cache for PR status consistency #498
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implement prStatusStore with subscription pattern similar to containerRuns to ensure consistent PR status across sidebar and main view components.
Updates all remaining direct getPrStatus() calls to use refreshPrStatus() from the shared prStatusStore for complete consistency across the app.
- Remove unused getPrStatus() sync getter from prStatusStore - Fix bug where additions/deletions/changedFiles were lost due to casting to PrInfo instead of preserving full PR metadata - Restore PrStatus type in usePrStatus hook to maintain compatibility with PrPreviewTooltip component which needs stats fields
Remove redundant PrData type from prStatusStore and use PrStatus directly from usePrStatus hook, eliminating duplicate type definition while maintaining clear separation between PrInfo (logic) and PrStatus (display).
- Move PrStatus type to lib/prStatus.ts alongside PrInfo - Remove unused error state from usePrStatus hook - Remove loading state (subscription pattern emits immediately with cache) - Update prStatusStore to import PrStatus from correct location - Remove skeleton loader from FileChangesPanel (not needed with instant cache) - Update PrPreviewTooltip import to use new type location
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| additions?: number; | ||
| deletions?: number; | ||
| changedFiles?: number; | ||
| }; |
There was a problem hiding this comment.
Bug: Type change makes number/url optional, breaking UI display
The PrStatus type definition changed from having required fields (number: number, url: string, state: string) to inheriting from PrInfo where these are optional (number?: number, url?: string, state?: string | null). Multiple UI components use pr.number in template strings without null checks (e.g., (#${pr.number})), which would display (#undefined) if the field is missing. The old type guaranteed these fields existed; the new type does not, creating a type safety regression that could produce incorrect UI text.
Additional Locations (1)
When taskPath changes from Task A to Task B, clear pr state immediately to prevent briefly showing Task A's PR button while Task B's data loads.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
PR status displayed inconsistently between sidebar and main view. One component could show stale data (e.g., "View PR") while another showed current state (e.g., "PR Merged").
Root cause: Each component independently fetched PR status via IPC with local state, creating cache inconsistency when PRs were merged/closed externally.
Solution
Implement shared cache pattern following existing
containerRuns.tsarchitecture:New:
lib/prStatusStore.tsUpdated:
hooks/usePrStatus.tserrorstateloadingstate (subscription emits cached values immediately)Updated:
lib/prStatus.tsPrStatustype (extendsPrInfowith display stats)PrInfofor logic/filtering,PrStatusfor displayUpdated: Direct IPC calls replaced with store
components/ProjectMainView.tsx- Delete dialog PR checkscomponents/kanban/KanbanBoard.tsx- Auto-complete detectionhooks/useDeleteRisks.ts- Risk assessmentBenefits
containerRuns.ts,activityStore.ts)Testing
Note
Centralizes PR status via a shared store with deduplicated IPC and subscriptions, updating hooks and components to use it for consistent, up-to-date UI.
lib/prStatusStore.tswith cached PR status, concurrent request deduplication, andsubscribeToPrStatus/refreshPrStatusAPIs.PrStatusinlib/prStatus.ts(extendsPrInfowith diff stats) and reuseisActivePr.hooks/usePrStatus.tsto subscribe to the store; removeloading/errorand return{ pr, refresh }.components/ProjectMainView.tsx: Replace direct IPC PR checks withrefreshPrStatus; useisActivePrin delete flow.components/kanban/KanbanBoard.tsx: UserefreshPrStatusto auto-mark tasks “Ready for review”.hooks/useDeleteRisks.ts: Switch torefreshPrStatusandisActivePrfor risk assessment.components/PrPreviewTooltip.tsx: ImportPrStatusfromlib/prStatus.components/FileChangesPanel.tsx: Remove PR loading skeleton/prop; rely on sharedusePrStatus.Written by Cursor Bugbot for commit 4de604a. This will update automatically on new commits. Configure here.