Fix folder workspace PR status icon from branch cache#7449
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change updates parent PR checks cache validation, row projection, and refresh inputs to account for worktree head and linked-review state. It extends the row and cache types with review number/status and a linked-review hint key, and reuses the shared parent PR checks projection in folder workspace card display. The refresh flow now includes the current head OID when fetching hosted reviews. Tests were updated to cover cache selection, stale-entry invalidation, and the folder card display path. Related PRs: None identified. Suggested labels: area:right-sidebar, area:sidebar, tests Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/renderer/src/components/sidebar/folder-workspace-card-pr-display.ts (1)
305-312: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAlign
hasNonGitHubLinkedReviewwith the canonical presence check.This helper diverges from the equivalent logic in
checks-panel-review.tsandWorktreeCard.tsx, which OR explicit presence checks. The??chain short-circuits on the first non-nullish value, so a legitimately-set later field can be masked by an earlier0(e.g.0 ?? 5→0), andBoolean(0)then yieldsfalse. While PR/MR numbers are 1-based today, this behavioral divergence undercuts this PR's goal of matching the child/right-sidebar selection exactly. A per-field!= nullcheck (which also coversundefinedoptional fields) is both correct and consistent.♻️ Proposed alignment
function hasNonGitHubLinkedReview(worktree: Worktree): boolean { - return Boolean( - worktree.linkedGitLabMR ?? - worktree.linkedBitbucketPR ?? - worktree.linkedAzureDevOpsPR ?? - worktree.linkedGiteaPR ?? - null - ) + return ( + worktree.linkedGitLabMR != null || + worktree.linkedBitbucketPR != null || + worktree.linkedAzureDevOpsPR != null || + worktree.linkedGiteaPR != null + ) }src/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.ts (1)
167-186: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the hosted-review-miss staleness path.
Both new tests pass
hostedReviewCache: null, so the newhostedReviewMissFetchedAtstaleness rejection branch (reject when(entry?.fetchedAt ?? 0) <= hostedReviewMissFetchedAtand PR isn't a current-head merged PR) is never exercised. Adding a case with a hosted-review miss entry whosefetchedAtis newer than the cached PR's would guard this new logic against regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5c3d823f-1226-4dbc-b252-1d3b5db73d7a
📒 Files selected for processing (2)
src/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.tssrc/renderer/src/components/sidebar/folder-workspace-card-pr-display.ts
brennanb2025
force-pushed
the
brennanb2025/fix-folder-pr-status-branch-cache
branch
from
c5c694a to
dab2f68
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/renderer/src/components/right-sidebar/parent-pr-checks-github-pr-cache.ts (1)
37-45: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
??chain only checks the first non-nullish field, not "any field set".
a ?? b ?? c ?? d ?? nullshort-circuits on the first field that isn'tnull/undefined, then only tests that value's truthiness — it never inspects the remaining fields once one is non-nullish. If an earlier field happened to be0(falsy but non-nullish), a genuinely linked later field (e.g.linkedGiteaPR) would be silently ignored. PR/MR numbers are 1-indexed in practice so this is unlikely to trigger, but the logic doesn't actually express "is any non-GitHub review linked".♻️ Proposed fix
function hasNonGitHubLinkedReview(worktree: Worktree): boolean { - return Boolean( - worktree.linkedGitLabMR ?? - worktree.linkedBitbucketPR ?? - worktree.linkedAzureDevOpsPR ?? - worktree.linkedGiteaPR ?? - null - ) + return ( + worktree.linkedGitLabMR != null || + worktree.linkedBitbucketPR != null || + worktree.linkedAzureDevOpsPR != null || + worktree.linkedGiteaPR != null + ) }src/renderer/src/components/sidebar/folder-workspace-card-pr-display.ts (1)
111-115: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnchecked type cast bypasses type safety for cache inputs.
coerceParentPrChecksCacheblindly castsRecord<string, unknown>toRecord<string, ParentPrChecksCacheEntry<T>>viaas, with no runtime shape validation. If any cache entry doesn't actually match{ data, fetchedAt }, downstream consumers (e.g. thefetchedAtcomparisons in the guard) will silently operate on malformed data rather than failing fast. Consider typinghostedReviewCache/prCache/checksCachemore precisely at the boundary where they're produced, so this function isn't needed as an escape hatch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cee6d0e0-fef2-4900-bb15-76ed0e6d5934
📒 Files selected for processing (8)
src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.test.tsxsrc/renderer/src/components/right-sidebar/parent-pr-checks-github-pr-cache.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-row-types.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-rows.tssrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.tssrc/renderer/src/components/sidebar/folder-workspace-card-pr-display.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.ts
brennanb2025
force-pushed
the
brennanb2025/fix-folder-pr-status-branch-cache
branch
from
dab2f68 to
000d94a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts (1)
158-197: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNon-obvious cache-precedence logic could use a short "why" comment.
The fallback order here (hosted-review cache → GitHub PR cache →
null/undefinedbased onhostedReviewEntry?.data === null) encodes a subtle precedence rule that isn't obvious from the code alone, similar to the pattern already flagged inparent-pr-checks-github-pr-cache.ts. A one-line comment on why hosted-review cache is checked before the GitHub PR cache, and why a confirmed-null hosted review yieldsnullwhile an absent/rejected entry yieldsundefined, would help future readers.As per coding guidelines: "When writing or modifying code driven by a design doc or non-obvious constraint, add a comment explaining why the code behaves the way it does. Keep comments short — one or two lines, capturing only the non-obvious reason."
Source: Coding guidelines
src/renderer/src/components/right-sidebar/parent-pr-checks-hosted-review-cache.ts (1)
59-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared linked-review helpers. This duplicates the provider switch in
src/renderer/src/components/sidebar/worktree-card-pr-display.tsand the linked-review guard insrc/renderer/src/components/right-sidebar/parent-pr-checks-github-pr-cache.ts; sharing that mapping would keep provider handling in one place and reduce drift when new providers are added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7e8f0136-3ac4-49b2-87dc-b2512b57d8f0
📒 Files selected for processing (10)
src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.test.tsxsrc/renderer/src/components/right-sidebar/parent-pr-checks-github-pr-cache.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-hosted-review-cache.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-refresh.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-row-types.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-rows.tssrc/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.tssrc/renderer/src/components/sidebar/folder-workspace-card-pr-display.ts
✅ Files skipped from review due to trivial changes (1)
- src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/src/components/right-sidebar/parent-pr-checks-row-types.ts
- src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.test.tsx
brennanb2025
force-pushed
the
brennanb2025/fix-folder-pr-status-branch-cache
branch
from
000d94a to
0c2a49c
Compare
brennanb2025
force-pushed
the
brennanb2025/fix-folder-pr-status-branch-cache
branch
from
0c2a49c to
812b2e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 829a6ba9-3288-4226-a303-cf7ff69e79e5
📒 Files selected for processing (11)
src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.test.tsxsrc/renderer/src/components/right-sidebar/parent-pr-checks-github-pr-cache.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-hosted-review-cache.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-refresh.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-row-types.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.tssrc/renderer/src/components/right-sidebar/parent-pr-checks-rows.tssrc/renderer/src/components/sidebar/WorktreeList.tsxsrc/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.tssrc/renderer/src/components/sidebar/folder-workspace-card-pr-display.ts
✅ Files skipped from review due to trivial changes (1)
- src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.ts
- src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.test.tsx
- src/renderer/src/components/sidebar/WorktreeList.tsx
- src/renderer/src/components/right-sidebar/parent-pr-checks-row-types.ts
- src/renderer/src/components/right-sidebar/parent-pr-checks-hosted-review-cache.ts
- src/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.ts
- src/renderer/src/components/right-sidebar/parent-pr-checks-github-pr-cache.ts
- src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts
- src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts
1 participant
Summary
Why
The right PR checks sidebar was usually correct because it projected hosted-review/PR cache state through
buildParentPrChecksProjection(...). The left folder workspace icon had a separate lookup path, so branch-discovered PRs and cache edge cases could show as a branch icon or stale status on the left while the right sidebar showed the PR.Consistency
Source of truth: parent PR-checks rows in the right sidebar.
Compared surfaces: folder workspace status icon, right sidebar folder PR checks panel, WorktreeCard PR display/cache guards, PR-status grouping legacy cache behavior, hosted-review refresh, and GitHub checks status sync.
Result: the left folder icon now follows the same review identity and classified status as the right sidebar, while deliberately omitting right-panel-only check detail names.
Perf / No-Regression Audit
repoMap; removed the left-sidebarchecksCachesubscription/fanout because the icon only needs review identity/classified status, not check-detail rows.WorktreeListchecksCache subscription, legacy/local cache tests, stale/mismatched cache tests, and refresh current-head propagation test.AI Review Report
Security Audit
Screenshots
Testing
pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/sidebar/folder-workspace-card-pr-display.test.ts src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.test.tsx src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.ts src/renderer/src/components/sidebar/WorktreeCard.pr-display.test.tsx src/renderer/src/components/sidebar/WorktreeCard.merged-pr-display.test.tsx src/renderer/src/components/sidebar/WorktreeCard.hosted-review-refresh.test.tsx src/renderer/src/store/slices/hosted-review.test.ts src/renderer/src/store/slices/github-checks.test.ts- 9 files / 91 tests passed.pnpm run typecheck:web- passed.pnpm exec oxlint ...changed files...- passed.git diff --check && git diff --cached --check- passed.Notes
.npmrcstill prints the existing${GITHUB_TOKEN}substitution warning during pnpm commands; commands completed successfully.