Fix detached PR sidebar display

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
brennanb2025
2026-06-26 23:13:24 -07:00
co-authored by Orca
parent 1b3961128a
commit a4e3ebd835
4 changed files with 180 additions and 12 deletions
@@ -1,6 +1,7 @@
import { renderToStaticMarkup } from 'react-dom/server'
import type { ReactNode } from 'react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import type { HostedReviewInfo } from '../../../../shared/hosted-review'
import type {
GlobalSettings,
PRInfo,
@@ -125,6 +126,21 @@ function makePRInfo(overrides: Partial<PRInfo> = {}): PRInfo {
}
}
function makeHostedReview(overrides: Partial<HostedReviewInfo> = {}): HostedReviewInfo {
return {
provider: 'github',
number: 6340,
title: 'Merged PR still checked out',
state: 'merged',
url: 'https://github.com/acme/orca/pull/6340',
status: 'success',
updatedAt: '2026-05-17T00:00:00.000Z',
mergeable: 'MERGEABLE',
headSha: 'abc123',
...overrides
}
}
function renderWorktreeCardMarkup(element: ReactNode): string {
return renderToStaticMarkup(<>{element}</>)
}
@@ -251,6 +267,70 @@ describe('WorktreeCard merged PR fallback display', () => {
expect(markup).not.toContain('Detached HEAD @ merge-c')
})
it('shows detached hosted PRs even when the cache entry was warmed from a linked hint', async () => {
hostedReviewCache = {
'local::repo-1::__detached_head__:merge-commit': {
data: makeHostedReview({
number: 24,
title: 'Validate Orca PR status fallback',
headSha: 'pr-head-before-merge',
status: 'failure'
}),
fetchedAt: 200,
linkedReviewHintKey: 'github:24'
}
}
prCache = {
'repo-1::__detached_head__:merge-commit': {
data: makePRInfo({
number: 24,
title: 'Validate Orca PR status fallback',
headSha: 'pr-head-before-merge',
checksStatus: 'failure'
}),
fetchedAt: 100
}
}
const { default: WorktreeCard } = await import('./WorktreeCard')
const markup = renderWorktreeCardMarkup(
<WorktreeCard
worktree={makeWorktree({ branch: '', linkedPR: null, head: 'merge-commit' })}
repo={makeRepo()}
isActive={false}
/>
)
expect(markup).toContain('Linked PR #24')
expect(markup).not.toContain('Detached HEAD @ merge-c')
})
it('keeps linked-hint detached hosted PRs hidden without HEAD-scoped PR proof', async () => {
hostedReviewCache = {
'local::repo-1::__detached_head__:merge-commit': {
data: makeHostedReview({
number: 24,
title: 'Stale manually linked PR',
headSha: 'pr-head-before-merge'
}),
fetchedAt: 200,
linkedReviewHintKey: 'github:24'
}
}
const { default: WorktreeCard } = await import('./WorktreeCard')
const markup = renderWorktreeCardMarkup(
<WorktreeCard
worktree={makeWorktree({ branch: '', linkedPR: null, head: 'merge-commit' })}
repo={makeRepo()}
isActive={false}
/>
)
expect(markup).not.toContain('Linked PR #24')
expect(markup).toContain('Detached HEAD @ merge-c')
})
it('hides detached HEAD badge when the parent list supplies PR status', async () => {
const { default: WorktreeCard } = await import('./WorktreeCard')
@@ -136,6 +136,11 @@ type WorktreeCardProps = {
const EMPTY_WORKSPACE_PORTS = []
const HOSTED_REVIEW_CARD_REFRESH_INTERVAL_MS = 60_000
type WorktreeCardHostedReviewCacheEntry = {
data?: HostedReviewInfo | null
fetchedAt?: number
linkedReviewHintKey?: string
}
type WorktreeCardPRCacheEntry = { data?: PRInfo | null; fetchedAt?: number }
export function shouldBeginWorktreeRename(
@@ -178,11 +183,30 @@ function isCachedMergedBranchPRCurrentForWorktree(
)
}
function isDetachedHeadCacheBranch(cacheBranch: string): boolean {
return cacheBranch.startsWith('__detached_head__:')
}
function findDetachedHostedReviewCacheEntry(
hostedReviewCache: Record<string, WorktreeCardHostedReviewCacheEntry> | undefined,
cacheBranch: string
): WorktreeCardHostedReviewCacheEntry | undefined {
if (!hostedReviewCache || !isDetachedHeadCacheBranch(cacheBranch)) {
return undefined
}
// Why: imported/folder workspaces can observe the same detached review under a
// different repo owner key; the HEAD-scoped suffix is still exact.
const suffix = `::${cacheBranch}`
return Object.entries(hostedReviewCache).find(
([key, entry]) => key.endsWith(suffix) && entry?.data
)?.[1]
}
function findDetachedPRCacheEntry(
prCache: Record<string, WorktreeCardPRCacheEntry> | undefined,
cacheBranch: string
): WorktreeCardPRCacheEntry | undefined {
if (!prCache || !cacheBranch.startsWith('__detached_head__:')) {
if (!prCache || !isDetachedHeadCacheBranch(cacheBranch)) {
return undefined
}
// Why: imported/folder workspaces can observe the same detached PR under a
@@ -479,8 +503,10 @@ const WorktreeCard = React.memo(function WorktreeCard({
const linearIssueCacheKey = worktree.linkedLinearIssue ? `all::${worktree.linkedLinearIssue}` : ''
// Subscribe to ONLY the specific cache entry, not entire review/issue caches.
const hostedReviewEntry = useAppStore((s) =>
hostedReviewCacheKey ? s.hostedReviewCache[hostedReviewCacheKey] : undefined
const hostedReviewEntry = useAppStore(
(s) =>
(hostedReviewCacheKey ? s.hostedReviewCache[hostedReviewCacheKey] : undefined) ??
findDetachedHostedReviewCacheEntry(s.hostedReviewCache, prCacheBranch)
)
const prCacheEntry = useAppStore(
(s) =>
@@ -522,7 +548,7 @@ const WorktreeCard = React.memo(function WorktreeCard({
// cache entries are already scoped by the checked-out HEAD merge commit.
const cachedBranchPR = prCacheEntry?.data
const cachedBranchPRFetchedAt = prCacheEntry?.fetchedAt
const cachedBranchPRIsHeadScoped = branch.length === 0 && prCacheBranch.length > 0
const cachedBranchPRIsHeadScoped = branch.length === 0 && isDetachedHeadCacheBranch(prCacheBranch)
const cachedMergedBranchPRMatchesCurrentHead = isCachedMergedBranchPRCurrentForWorktree(
cachedBranchPR,
worktree
@@ -540,6 +566,17 @@ const WorktreeCard = React.memo(function WorktreeCard({
const cachedBranchReview = useCachedBranchReview
? hostedReviewInfoFromGitHubPRInfo(cachedBranchPR)
: hostedReview
const detachedHostedReviewMatchesCachedHeadPR =
cachedBranchPRIsHeadScoped &&
cachedBranchPR !== undefined &&
cachedBranchPR !== null &&
hostedReview?.provider === 'github' &&
hostedReview.number === cachedBranchPR.number &&
!hasNonGitHubLinkedReview
const reviewHintKey =
(useCachedBranchReview || detachedHostedReviewMatchesCachedHeadPR) && !hasLinkedReview
? ''
: hostedReviewEntry?.linkedReviewHintKey
const prDisplay = getWorktreeCardPrDisplay(
cachedBranchReview,
linkedGitHubPR,
@@ -548,8 +585,7 @@ const WorktreeCard = React.memo(function WorktreeCard({
linkedAzureDevOpsPR,
linkedGiteaPR,
{
reviewHintKey:
useCachedBranchReview && !hasLinkedReview ? '' : hostedReviewEntry?.linkedReviewHintKey
reviewHintKey
}
)
const issue: IssueInfo | null | undefined = worktree.linkedIssue
@@ -3521,22 +3521,21 @@ const VirtualizedWorktreeViewport = React.memo(function VirtualizedWorktreeViewp
.filter((item) => item.start < viewportBottom && item.end > viewportTop)
.map((item) => renderRows[item.index])
.filter((row): row is WorktreeItemRow => row?.type === 'item')
.filter((row) => row.repo?.kind === 'git' && !row.worktree.isBare && row.worktree.branch)
.filter((row) => row.repo?.kind === 'git' && hasGitHubPRRefreshIdentity(row.worktree))
const visibleWorktreeIds = new Set(visibleRows.map((row) => row.worktree.id))
if (
shouldTrackSidebarWorktree &&
currentWorktree &&
!currentWorktree.isBare &&
currentWorktree.branch
hasGitHubPRRefreshIdentity(currentWorktree)
) {
visibleWorktreeIds.add(currentWorktree.id)
}
const visibleIdentity = visibleRows
.map((row) => `${row.worktree.id}:${row.worktree.branch}:${row.worktree.linkedPR ?? ''}`)
.map((row) => getGitHubPRRefreshIdentity(row.worktree))
.join('|')
const sidebarIdentity =
shouldTrackSidebarWorktree && currentWorktree
? `${currentWorktree.id}:${currentWorktree.branch}:${currentWorktree.linkedPR ?? ''}`
? getGitHubPRRefreshIdentity(currentWorktree)
: ''
const key = `${visibleIdentity}:${sidebarIdentity}:${sshConnectedGeneration}:${prVisibleRefreshGeneration}:${cardProps.join(',')}`
if (!key || key === lastVisibleRefreshKeyRef.current) {
@@ -4899,6 +4898,19 @@ export function installWorktreeVisibleRefreshVisibilityListener(onChange: () =>
return () => document.removeEventListener('visibilitychange', onChange)
}
export function hasGitHubPRRefreshIdentity(
worktree: Pick<Worktree, 'isBare' | 'branch' | 'head'>
): boolean {
return !worktree.isBare && Boolean(worktree.branch || worktree.head)
}
export function getGitHubPRRefreshIdentity(
worktree: Pick<Worktree, 'id' | 'branch' | 'head' | 'linkedPR'>
): string {
const gitIdentity = worktree.branch || worktree.head || ''
return `${worktree.id}:${gitIdentity}:${worktree.linkedPR ?? ''}`
}
const WorktreeList = React.memo(function WorktreeList({
scrollOffsetRef,
scrollAnchorRef,
@@ -1,5 +1,9 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { installWorktreeVisibleRefreshVisibilityListener } from './WorktreeList'
import {
getGitHubPRRefreshIdentity,
hasGitHubPRRefreshIdentity,
installWorktreeVisibleRefreshVisibilityListener
} from './WorktreeList'
describe('installWorktreeVisibleRefreshVisibilityListener', () => {
beforeEach(() => {
@@ -29,4 +33,40 @@ describe('installWorktreeVisibleRefreshVisibilityListener', () => {
cleanup()
expect(removeEventListener).toHaveBeenCalledWith('visibilitychange', onChange)
})
it('tracks detached worktrees by head for visible PR refreshes', () => {
expect(
hasGitHubPRRefreshIdentity({
isBare: false,
branch: '',
head: '67c9b9565d1596f40afbb5d4acb1ee71e56e1c4e'
})
).toBe(true)
expect(
getGitHubPRRefreshIdentity({
id: 'repo-1::C:/Users/neil/orca/workspaces/demo-project-6470-detached-24',
branch: '',
head: '67c9b9565d1596f40afbb5d4acb1ee71e56e1c4e',
linkedPR: null
})
).toBe(
'repo-1::C:/Users/neil/orca/workspaces/demo-project-6470-detached-24:67c9b9565d1596f40afbb5d4acb1ee71e56e1c4e:'
)
})
it('keeps branch worktree refresh identity branch-scoped', () => {
expect(
getGitHubPRRefreshIdentity({
id: 'repo-1::/repo/worktrees/feature',
branch: 'feature/local-branch',
head: 'abc123',
linkedPR: 42
})
).toBe('repo-1::/repo/worktrees/feature:feature/local-branch:42')
})
it('does not track worktrees without branch or head identity', () => {
expect(hasGitHubPRRefreshIdentity({ isBare: false, branch: '', head: '' })).toBe(false)
expect(hasGitHubPRRefreshIdentity({ isBare: true, branch: 'main', head: 'abc123' })).toBe(false)
})
})