mirror of
https://github.com/stablyai/orca.git
synced 2026-10-16 16:03:26 +00:00
Show a merged branch PR when the worktree head is one of its own commits
A worktree can end up behind its own merged PR's final head — update-branch
merges or web-committed suggestions land on the remote branch and merge
without a local pull. The merged-implicit visibility rule required exact
head equality, so the Checks panel dropped to 'No pull request found —
Create a pull request…' and the sidebar merged badge vanished right after
a successful merge.
Keep the reused-branch guard, but before hiding a merged branch-matched PR
ask GitHub whether the worktree head is one of the PR's own commits
(commits/{sha}/pulls, bounded immutable-friendly cache, core rate-limit
guard, fail-safe to the old hide). Confirmed heads are stamped on PRInfo/
HostedReviewInfo as confirmedContainedHeadOid so the renderer agreement
points (hosted-review staleness, refresh fallback number, no-PR preserve,
sidebar merged badge) accept the same worktrees without refetch loops.
Old main commits carry no association with the PR, so reused branch names
stay hidden; commits never pushed to GitHub fail the probe and stay hidden.
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -33,7 +33,9 @@ const {
|
||||
getRemoteUrlForRepoMock: vi.fn(),
|
||||
gitExecFileAsyncMock: vi.fn(),
|
||||
getRateLimitMock: vi.fn(),
|
||||
rateLimitGuardMock: vi.fn<() => RateLimitGuardResult>(() => ({ blocked: false })),
|
||||
rateLimitGuardMock: vi.fn<(bucket?: string) => RateLimitGuardResult>(() => ({
|
||||
blocked: false
|
||||
})),
|
||||
noteRateLimitSpendMock: vi.fn(),
|
||||
ghRepoExecOptionsMock: vi.fn((context) =>
|
||||
context.connectionId
|
||||
@@ -121,6 +123,7 @@ import {
|
||||
__resetTrackedUpstreamBranchCacheForTests
|
||||
} from './client'
|
||||
import { __resetPRConflictSummaryGitCapabilityCacheForTests } from './conflict-summary'
|
||||
import { resetMergedPRCommitMembershipCacheForTest } from './merged-pr-commit-membership'
|
||||
|
||||
describe('checkOrcaStarred', () => {
|
||||
beforeEach(() => {
|
||||
@@ -192,6 +195,7 @@ describe('getPRForBranch', () => {
|
||||
_resetMergeQueueCacheForTests()
|
||||
__resetTrackedUpstreamBranchCacheForTests()
|
||||
__resetPRConflictSummaryGitCapabilityCacheForTests()
|
||||
resetMergedPRCommitMembershipCacheForTest()
|
||||
})
|
||||
|
||||
it('queries GitHub by head branch when the remote is on github.com', async () => {
|
||||
@@ -746,6 +750,176 @@ describe('getPRForBranch', () => {
|
||||
})
|
||||
})
|
||||
|
||||
const mockMergedBranchPRLookupBehindHead = (prNumber = 6011): void => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
number: prNumber,
|
||||
title: 'Merged PR with unpulled final head',
|
||||
state: 'closed',
|
||||
merged_at: '2026-07-03T21:27:36Z',
|
||||
html_url: `https://github.com/acme/widgets/pull/${prNumber}`,
|
||||
updated_at: '2026-07-03T21:27:36Z',
|
||||
draft: false,
|
||||
mergeable_state: 'clean',
|
||||
head: { ref: 'fix-hibernation-wake', sha: 'aaaa1111aaaa1111' },
|
||||
base: { ref: 'main', sha: 'base-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
number: prNumber,
|
||||
title: 'Merged PR with unpulled final head',
|
||||
state: 'MERGED',
|
||||
url: `https://github.com/acme/widgets/pull/${prNumber}`,
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-07-03T21:27:36Z',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'fix-hibernation-wake',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'aaaa1111aaaa1111'
|
||||
})
|
||||
})
|
||||
gitExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: 'bbbb2222bbbb2222\n',
|
||||
stderr: ''
|
||||
})
|
||||
}
|
||||
|
||||
it('shows a merged branch PR when the worktree head is one of its own commits', async () => {
|
||||
mockMergedBranchPRLookupBehindHead()
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ number: 6011 }, { number: 42 }])
|
||||
})
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
|
||||
expect(ghExecFileAsyncMock).toHaveBeenNthCalledWith(
|
||||
3,
|
||||
['api', 'repos/acme/widgets/commits/bbbb2222bbbb2222/pulls?per_page=100'],
|
||||
expect.anything()
|
||||
)
|
||||
expect(pr).toMatchObject({
|
||||
number: 6011,
|
||||
state: 'merged',
|
||||
headSha: 'aaaa1111aaaa1111',
|
||||
confirmedContainedHeadOid: 'bbbb2222bbbb2222'
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps hiding a merged branch PR when the head belongs to a different PR (reused branch)', async () => {
|
||||
mockMergedBranchPRLookupBehindHead()
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ number: 42 }])
|
||||
})
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
|
||||
expect(pr).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps hiding a merged branch PR when the commit membership probe fails', async () => {
|
||||
mockMergedBranchPRLookupBehindHead()
|
||||
ghExecFileAsyncMock.mockRejectedValueOnce(new Error('HTTP 422: No commit found'))
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
|
||||
expect(pr).toBeNull()
|
||||
})
|
||||
|
||||
it('skips the membership probe while the core rate-limit budget is blocked', async () => {
|
||||
mockMergedBranchPRLookupBehindHead()
|
||||
rateLimitGuardMock.mockImplementation((bucket?: string) =>
|
||||
bucket === 'core'
|
||||
? { blocked: true, remaining: 0, limit: 5000, resetAt: 0 }
|
||||
: { blocked: false }
|
||||
)
|
||||
|
||||
const pr = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
|
||||
expect(pr).toBeNull()
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('reuses the cached membership answer instead of re-querying per poll', async () => {
|
||||
mockMergedBranchPRLookupBehindHead()
|
||||
ghExecFileAsyncMock.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ number: 6011 }])
|
||||
})
|
||||
const first = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
expect(first).toMatchObject({ number: 6011, confirmedContainedHeadOid: 'bbbb2222bbbb2222' })
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(3)
|
||||
|
||||
mockMergedBranchPRLookupBehindHead()
|
||||
const second = await getPRForBranch('/repo-root', 'fix-hibernation-wake')
|
||||
|
||||
expect(second).toMatchObject({ number: 6011, confirmedContainedHeadOid: 'bbbb2222bbbb2222' })
|
||||
// No fourth membership call: the confirmed answer is immutable and cached.
|
||||
expect(ghExecFileAsyncMock).toHaveBeenCalledTimes(5)
|
||||
})
|
||||
|
||||
it('uses the caller-supplied worktree head for the membership probe without shelling out', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([
|
||||
{
|
||||
number: 6012,
|
||||
title: 'Merged PR queried by checks panel',
|
||||
state: 'closed',
|
||||
merged_at: '2026-07-03T21:27:36Z',
|
||||
html_url: 'https://github.com/acme/widgets/pull/6012',
|
||||
updated_at: '2026-07-03T21:27:36Z',
|
||||
draft: false,
|
||||
mergeable_state: 'clean',
|
||||
head: { ref: 'fix-hibernation-wake', sha: 'aaaa1111aaaa1111' },
|
||||
base: { ref: 'main', sha: 'base-oid' }
|
||||
}
|
||||
])
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify({
|
||||
number: 6012,
|
||||
title: 'Merged PR queried by checks panel',
|
||||
state: 'MERGED',
|
||||
url: 'https://github.com/acme/widgets/pull/6012',
|
||||
statusCheckRollup: [],
|
||||
updatedAt: '2026-07-03T21:27:36Z',
|
||||
isDraft: false,
|
||||
mergeable: 'MERGEABLE',
|
||||
baseRefName: 'main',
|
||||
headRefName: 'fix-hibernation-wake',
|
||||
baseRefOid: 'base-oid',
|
||||
headRefOid: 'aaaa1111aaaa1111'
|
||||
})
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
stdout: JSON.stringify([{ number: 6012 }])
|
||||
})
|
||||
|
||||
const outcome = await getPRForBranchOutcome(
|
||||
'/repo-root',
|
||||
'fix-hibernation-wake',
|
||||
null,
|
||||
null,
|
||||
null,
|
||||
{
|
||||
currentHeadOid: 'cccc3333cccc3333'
|
||||
}
|
||||
)
|
||||
|
||||
expect(gitExecFileAsyncMock).not.toHaveBeenCalled()
|
||||
expect(outcome).toMatchObject({
|
||||
kind: 'found',
|
||||
pr: { number: 6012, confirmedContainedHeadOid: 'cccc3333cccc3333' }
|
||||
})
|
||||
})
|
||||
|
||||
it('prefers branch lookup over a fallback PR number', async () => {
|
||||
getOwnerRepoMock.mockResolvedValueOnce({ owner: 'acme', repo: 'widgets' })
|
||||
ghExecFileAsyncMock
|
||||
|
||||
@@ -62,6 +62,7 @@ import {
|
||||
type LocalGitExecOptions,
|
||||
type OwnerRepo
|
||||
} from './gh-utils'
|
||||
import { isCommitPartOfMergedPR } from './merged-pr-commit-membership'
|
||||
import { getSshGitProvider } from '../providers/ssh-git-dispatch'
|
||||
import {
|
||||
hasHostedReviewLocalGitOptions,
|
||||
@@ -2808,7 +2809,30 @@ export async function getPRForBranchOutcome(
|
||||
typeof options.currentHeadOid === 'string' && options.currentHeadOid.trim().length > 0
|
||||
? options.currentHeadOid.trim()
|
||||
: null
|
||||
const hideMergedImplicitPR = async (candidate: PullRequestLookupData | null) => {
|
||||
let confirmedContainedHeadOid: string | null = null
|
||||
const mergedPRContainsHead = async (
|
||||
candidate: PullRequestLookupData,
|
||||
candidateRepo: OwnerRepo | null,
|
||||
headOid: string | null
|
||||
): Promise<boolean> => {
|
||||
if (!candidateRepo || !headOid) {
|
||||
return false
|
||||
}
|
||||
const contained = await isCommitPartOfMergedPR({
|
||||
ownerRepo: candidateRepo,
|
||||
prNumber: candidate.number,
|
||||
commitOid: headOid,
|
||||
ghOptions
|
||||
})
|
||||
if (contained) {
|
||||
confirmedContainedHeadOid = headOid
|
||||
}
|
||||
return contained
|
||||
}
|
||||
const hideMergedImplicitPR = async (
|
||||
candidate: PullRequestLookupData | null,
|
||||
candidateRepo: OwnerRepo | null
|
||||
) => {
|
||||
if (!candidate || !isMergedImplicitPR(candidate, linkedPRNumber)) {
|
||||
return false
|
||||
}
|
||||
@@ -2819,7 +2843,18 @@ export async function getPRForBranchOutcome(
|
||||
explicitCurrentHeadOid !== null
|
||||
? explicitCurrentHeadOid
|
||||
: await getCurrentHeadOid(repoPath, connectionId, localGitOptions)
|
||||
return shouldHideMergedImplicitPR(candidate, linkedPRNumber, currentHeadOidForMergedImplicit)
|
||||
if (!shouldHideMergedImplicitPR(candidate, linkedPRNumber, currentHeadOidForMergedImplicit)) {
|
||||
return false
|
||||
}
|
||||
// Why: a worktree can sit behind its own PR's final head (update-branch
|
||||
// merges, web-committed suggestions). A head that is one of the PR's own
|
||||
// commits is the same line of work, not a reused branch name — keep the
|
||||
// merged PR visible instead of offering "create a pull request".
|
||||
return !(await mergedPRContainsHead(
|
||||
candidate,
|
||||
candidateRepo,
|
||||
currentHeadOidForMergedImplicit
|
||||
))
|
||||
}
|
||||
|
||||
if (typeof linkedPRNumber === 'number') {
|
||||
@@ -2886,7 +2921,7 @@ export async function getPRForBranchOutcome(
|
||||
}
|
||||
}
|
||||
let mergedBranchLookupNumber: number | null = null
|
||||
if (await hideMergedImplicitPR(data)) {
|
||||
if (await hideMergedImplicitPR(data, dataRepo)) {
|
||||
mergedBranchLookupNumber = data?.number ?? null
|
||||
data = null
|
||||
dataRepo = null
|
||||
@@ -2913,14 +2948,15 @@ export async function getPRForBranchOutcome(
|
||||
data.number === fallbackPRNumber
|
||||
const explicitHeadHidesMergedImplicitPR =
|
||||
explicitCurrentHeadOid !== null &&
|
||||
shouldHideMergedImplicitPR(data, linkedPRNumber, explicitCurrentHeadOid)
|
||||
shouldHideMergedImplicitPR(data, linkedPRNumber, explicitCurrentHeadOid) &&
|
||||
!(await mergedPRContainsHead(data, dataRepo, explicitCurrentHeadOid))
|
||||
const shouldPreserveMergedFallback =
|
||||
!explicitHeadHidesMergedImplicitPR &&
|
||||
(fallbackConfirmedMergedBranch || options.acceptMergedFallbackPR === true)
|
||||
// Why: a currently visible PR can be merged outside Orca; when the caller
|
||||
// marks the fallback as visible review state, keep its lifecycle fresh even
|
||||
// if GitHub no longer reports it by branch (for example deleted heads).
|
||||
if ((await hideMergedImplicitPR(data)) && !shouldPreserveMergedFallback) {
|
||||
if ((await hideMergedImplicitPR(data, dataRepo)) && !shouldPreserveMergedFallback) {
|
||||
return { kind: 'no-pr', fetchedAt: Date.now() }
|
||||
}
|
||||
|
||||
@@ -2962,6 +2998,7 @@ export async function getPRForBranchOutcome(
|
||||
: {}),
|
||||
...(data.mergeStateStatus !== undefined ? { mergeStateStatus: data.mergeStateStatus } : {}),
|
||||
headSha: data.headRefOid,
|
||||
...(confirmedContainedHeadOid ? { confirmedContainedHeadOid } : {}),
|
||||
...(data.baseRefName ? { baseRefName: data.baseRefName } : {}),
|
||||
prRepo: dataRepo ?? undefined,
|
||||
headRepo: dataHeadRepo ?? undefined,
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
import { ghExecFileAsync } from './gh-utils'
|
||||
import { noteRateLimitSpend, rateLimitGuard } from './rate-limit'
|
||||
import type { OwnerRepo } from './github-repository-identity'
|
||||
|
||||
type GhExecOptions = Parameters<typeof ghExecFileAsync>[1]
|
||||
|
||||
// Why: a merged PR's commit set is immutable, so confirmations never expire in
|
||||
// practice; the TTLs only bound memory and let "commit not on GitHub yet"
|
||||
// answers retry after a future push.
|
||||
const MEMBERSHIP_CACHE_MAX_ENTRIES = 200
|
||||
const MEMBERSHIP_CONFIRMED_TTL_MS = 6 * 60 * 60 * 1000
|
||||
const MEMBERSHIP_NEGATIVE_TTL_MS = 5 * 60 * 1000
|
||||
|
||||
const membershipCache = new Map<string, { value: boolean; expiresAt: number }>()
|
||||
|
||||
function pruneMergedPRCommitMembershipCache(now = Date.now()): void {
|
||||
for (const [cacheKey, cached] of membershipCache) {
|
||||
if (cached.expiresAt <= now) {
|
||||
membershipCache.delete(cacheKey)
|
||||
}
|
||||
}
|
||||
while (membershipCache.size > MEMBERSHIP_CACHE_MAX_ENTRIES) {
|
||||
const oldestKey = membershipCache.keys().next().value
|
||||
if (oldestKey === undefined) {
|
||||
break
|
||||
}
|
||||
membershipCache.delete(oldestKey)
|
||||
}
|
||||
}
|
||||
|
||||
export function resetMergedPRCommitMembershipCacheForTest(): void {
|
||||
membershipCache.clear()
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether `commitOid` is part of pull request `prNumber` on GitHub — i.e. the
|
||||
* commit belongs to that PR's history rather than merely sharing a branch name.
|
||||
* A worktree sitting on such a commit is on the PR's own line of work (for
|
||||
* example behind web-committed suggestions or an update-branch merge), not a
|
||||
* reused branch name. Conservative on any failure: returns false.
|
||||
*/
|
||||
export async function isCommitPartOfMergedPR(args: {
|
||||
ownerRepo: OwnerRepo
|
||||
prNumber: number
|
||||
commitOid: string
|
||||
ghOptions: GhExecOptions
|
||||
}): Promise<boolean> {
|
||||
const oid = args.commitOid.trim().toLowerCase()
|
||||
if (!/^[0-9a-f]{4,64}$/.test(oid) || !Number.isInteger(args.prNumber)) {
|
||||
return false
|
||||
}
|
||||
const owner = args.ownerRepo.owner
|
||||
const repo = args.ownerRepo.repo
|
||||
const cacheKey = `${owner.toLowerCase()}/${repo.toLowerCase()}#${args.prNumber}@${oid}`
|
||||
const now = Date.now()
|
||||
pruneMergedPRCommitMembershipCache(now)
|
||||
const cached = membershipCache.get(cacheKey)
|
||||
if (cached && cached.expiresAt > now) {
|
||||
return cached.value
|
||||
}
|
||||
// Why blocked → uncached false: keep the merged PR hidden without burning
|
||||
// budget; the next poll re-asks once the rate-limit window recovers.
|
||||
if (rateLimitGuard('core').blocked) {
|
||||
return false
|
||||
}
|
||||
try {
|
||||
noteRateLimitSpend('core')
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
['api', `repos/${owner}/${repo}/commits/${oid}/pulls?per_page=100`],
|
||||
args.ghOptions
|
||||
)
|
||||
const parsed = JSON.parse(stdout) as unknown
|
||||
const value =
|
||||
Array.isArray(parsed) &&
|
||||
parsed.some(
|
||||
(entry) =>
|
||||
typeof entry === 'object' &&
|
||||
entry !== null &&
|
||||
(entry as { number?: unknown }).number === args.prNumber
|
||||
)
|
||||
membershipCache.set(cacheKey, {
|
||||
value,
|
||||
expiresAt: now + (value ? MEMBERSHIP_CONFIRMED_TTL_MS : MEMBERSHIP_NEGATIVE_TTL_MS)
|
||||
})
|
||||
return value
|
||||
} catch {
|
||||
// Why: the common failure is 422 for a commit never pushed to GitHub —
|
||||
// definitive "new local work" today, but a push can change it; retry later.
|
||||
membershipCache.set(cacheKey, {
|
||||
value: false,
|
||||
expiresAt: now + MEMBERSHIP_NEGATIVE_TTL_MS
|
||||
})
|
||||
return false
|
||||
}
|
||||
}
|
||||
@@ -36,8 +36,7 @@ import type {
|
||||
Worktree,
|
||||
Repo,
|
||||
IssueInfo,
|
||||
LinearIssue,
|
||||
PRInfo
|
||||
LinearIssue
|
||||
} from '../../../../shared/types'
|
||||
import { CONFLICT_OPERATION_LABELS } from './WorktreeCardHelpers'
|
||||
import {
|
||||
@@ -48,7 +47,10 @@ import {
|
||||
} from './WorktreeCardMeta'
|
||||
import { WorktreeCardPortsDetails, WorktreeCardPortsTrigger } from './WorktreeCardPorts'
|
||||
import { writeWorkspaceDragData } from './workspace-status'
|
||||
import { getWorktreeCardPrDisplay } from './worktree-card-pr-display'
|
||||
import {
|
||||
getWorktreeCardPrDisplay,
|
||||
isCachedMergedBranchPRCurrentForWorktree
|
||||
} from './worktree-card-pr-display'
|
||||
import type { WorktreeCardPrDisplay } from './worktree-card-pr-display'
|
||||
import {
|
||||
coerceWorktreeCardVisibleTitle,
|
||||
@@ -153,20 +155,6 @@ function isWebClient(): boolean {
|
||||
return Boolean((window as unknown as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__)
|
||||
}
|
||||
|
||||
function isCachedMergedBranchPRCurrentForWorktree(
|
||||
cachedPR: PRInfo | null | undefined,
|
||||
worktree: Worktree
|
||||
): boolean {
|
||||
return (
|
||||
cachedPR?.state === 'merged' &&
|
||||
typeof cachedPR.headSha === 'string' &&
|
||||
cachedPR.headSha.length > 0 &&
|
||||
typeof worktree.head === 'string' &&
|
||||
worktree.head.length > 0 &&
|
||||
cachedPR.headSha === worktree.head
|
||||
)
|
||||
}
|
||||
|
||||
function getDirectoryName(folderPath: string): string {
|
||||
const normalized = folderPath.replace(/[\\/]+$/, '')
|
||||
const parts = normalized.split(/[\\/]+/)
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { HostedReviewInfo } from '../../../../shared/hosted-review'
|
||||
import { getWorktreeCardPrDisplay } from './worktree-card-pr-display'
|
||||
import type { PRInfo } from '../../../../shared/types'
|
||||
import {
|
||||
getWorktreeCardPrDisplay,
|
||||
isCachedMergedBranchPRCurrentForWorktree
|
||||
} from './worktree-card-pr-display'
|
||||
|
||||
const pr: HostedReviewInfo = {
|
||||
provider: 'github',
|
||||
@@ -104,3 +108,38 @@ describe('getWorktreeCardPrDisplay', () => {
|
||||
expect(getWorktreeCardPrDisplay(bitbucketReview, null)).toBe(bitbucketReview)
|
||||
})
|
||||
})
|
||||
|
||||
describe('isCachedMergedBranchPRCurrentForWorktree', () => {
|
||||
const mergedPR: PRInfo = {
|
||||
number: 55,
|
||||
title: 'Merged PR',
|
||||
state: 'merged',
|
||||
url: 'https://github.com/stablyai/orca/pull/55',
|
||||
checksStatus: 'success',
|
||||
updatedAt: '2026-07-03T00:00:00.000Z',
|
||||
mergeable: 'MERGEABLE',
|
||||
headSha: 'final-head'
|
||||
}
|
||||
|
||||
it('matches when the worktree sits exactly on the merged head', () => {
|
||||
expect(isCachedMergedBranchPRCurrentForWorktree(mergedPR, { head: 'final-head' })).toBe(true)
|
||||
})
|
||||
|
||||
it('matches when the worktree head is a confirmed commit of the merged PR', () => {
|
||||
expect(
|
||||
isCachedMergedBranchPRCurrentForWorktree(
|
||||
{ ...mergedPR, confirmedContainedHeadOid: 'behind-head' },
|
||||
{ head: 'behind-head' }
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects a merged PR when the worktree head is neither the final head nor confirmed', () => {
|
||||
expect(
|
||||
isCachedMergedBranchPRCurrentForWorktree(
|
||||
{ ...mergedPR, confirmedContainedHeadOid: 'other-head' },
|
||||
{ head: 'reused-branch-head' }
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,7 +1,24 @@
|
||||
import type { HostedReviewInfo } from '../../../../shared/hosted-review'
|
||||
import type { PRInfo, Worktree } from '../../../../shared/types'
|
||||
|
||||
type LinkedReviewMetadataProvider = Exclude<HostedReviewInfo['provider'], 'unsupported'>
|
||||
|
||||
export function isCachedMergedBranchPRCurrentForWorktree(
|
||||
cachedPR: PRInfo | null | undefined,
|
||||
worktree: Pick<Worktree, 'head'>
|
||||
): boolean {
|
||||
return (
|
||||
cachedPR?.state === 'merged' &&
|
||||
typeof cachedPR.headSha === 'string' &&
|
||||
cachedPR.headSha.length > 0 &&
|
||||
typeof worktree.head === 'string' &&
|
||||
worktree.head.length > 0 &&
|
||||
// Why: a worktree behind its own merged PR (update-branch/web commits) is
|
||||
// still that PR's line of work; match the main-process visibility rule.
|
||||
(cachedPR.headSha === worktree.head || cachedPR.confirmedContainedHeadOid === worktree.head)
|
||||
)
|
||||
}
|
||||
|
||||
type LinkedReviewNumbers = {
|
||||
linkedPR: number | null
|
||||
linkedGitLabMR: number | null
|
||||
|
||||
@@ -2264,6 +2264,55 @@ describe('createGitHubSlice.fetchPRForBranch', () => {
|
||||
expect(mockApi.cache.setGitHub).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('preserves cached merged PR data when the worktree head is a confirmed PR commit', async () => {
|
||||
vi.useFakeTimers()
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const repoId = 'repo-1'
|
||||
const branch = 'feature/merged-pr-behind-head'
|
||||
const worktreeId = 'wt-merged-behind-head'
|
||||
const cachedPR = makePR({
|
||||
number: 13,
|
||||
title: 'Merged PR with unpulled final head',
|
||||
state: 'merged',
|
||||
headSha: 'merged-final-head',
|
||||
confirmedContainedHeadOid: 'behind-head'
|
||||
})
|
||||
|
||||
store.setState({
|
||||
repos: [{ id: repoId, path: repoPath, name: 'repo', kind: 'git' }],
|
||||
worktreesByRepo: {
|
||||
[repoId]: [
|
||||
makePRRefreshWorktree({
|
||||
id: worktreeId,
|
||||
repoId,
|
||||
branch,
|
||||
head: 'behind-head'
|
||||
})
|
||||
]
|
||||
},
|
||||
prCache: {
|
||||
[`${repoId}::${branch}`]: {
|
||||
data: cachedPR,
|
||||
fetchedAt: 1
|
||||
}
|
||||
}
|
||||
} as unknown as Partial<AppState>)
|
||||
mockApi.gh.refreshPRNow.mockResolvedValueOnce({ kind: 'no-pr', fetchedAt: 2 })
|
||||
|
||||
await expect(
|
||||
store.getState().fetchPRForBranch(repoPath, branch, {
|
||||
force: true,
|
||||
repoId,
|
||||
worktreeId
|
||||
})
|
||||
).resolves.toEqual(cachedPR)
|
||||
expect(store.getState().prCache[`${repoId}::${branch}`]).toEqual({
|
||||
data: cachedPR,
|
||||
fetchedAt: 1
|
||||
})
|
||||
})
|
||||
|
||||
it.each([
|
||||
{
|
||||
name: 'worktree id is missing',
|
||||
@@ -3482,6 +3531,53 @@ describe('createGitHubSlice.refreshGitHubForWorktreeIfStale', () => {
|
||||
expect(mockApi.gh.prForBranch).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('keeps a confirmed behind-head merged PR as the refresh fallback number', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
const repoId = 'repo-1'
|
||||
const branch = 'feature/merged-pr-behind-head'
|
||||
const worktreeId = 'wt-merged-behind-fallback'
|
||||
mockApi.gh.enqueuePRRefresh.mockResolvedValueOnce({ kind: 'queued' })
|
||||
|
||||
store.setState({
|
||||
repos: [{ id: repoId, path: repoPath, name: 'repo', kind: 'git' }],
|
||||
worktreesByRepo: {
|
||||
[repoId]: [
|
||||
makePRRefreshWorktree({
|
||||
id: worktreeId,
|
||||
repoId,
|
||||
branch,
|
||||
head: 'behind-head'
|
||||
})
|
||||
]
|
||||
},
|
||||
prCache: {
|
||||
[`${repoId}::${branch}`]: {
|
||||
data: makePR({
|
||||
number: 13,
|
||||
title: 'Merged PR with unpulled final head',
|
||||
state: 'merged',
|
||||
headSha: 'merged-final-head',
|
||||
confirmedContainedHeadOid: 'behind-head'
|
||||
}),
|
||||
fetchedAt: 1
|
||||
}
|
||||
}
|
||||
} as unknown as Partial<AppState>)
|
||||
|
||||
store.getState().enqueueGitHubPRRefresh(worktreeId, 'active', 80)
|
||||
await Promise.resolve()
|
||||
|
||||
// Why 13: a merged PR confirmed to contain this worktree head is still the
|
||||
// branch's PR; losing the fallback number would blank the panel whenever
|
||||
// GitHub stops reporting the deleted head by branch name.
|
||||
expect(mockApi.gh.enqueuePRRefresh).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
candidate: expect.objectContaining({ fallbackPRNumber: 13 })
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
it('direct-fetches when enqueue returns an explicit fallback result', async () => {
|
||||
const store = createTestStore()
|
||||
const repoPath = '/repo'
|
||||
|
||||
@@ -1039,7 +1039,10 @@ function buildPRRefreshCandidate(
|
||||
)
|
||||
const cachedFallbackPRNumber = cachedPR?.number ?? null
|
||||
const cachedMergedPRMovedPastHead =
|
||||
worktree.linkedPR == null && cachedPR?.state === 'merged' && cachedPR.headSha !== worktree.head
|
||||
worktree.linkedPR == null &&
|
||||
cachedPR?.state === 'merged' &&
|
||||
cachedPR.headSha !== worktree.head &&
|
||||
cachedPR.confirmedContainedHeadOid !== worktree.head
|
||||
const fallbackPRNumber =
|
||||
worktree.linkedPR == null && !cachedMergedPRMovedPastHead
|
||||
? (cachedFallbackPRNumber ?? hostedReviewFallbackPRNumber)
|
||||
@@ -1264,7 +1267,8 @@ function shouldPreserveExistingPRForFallbackMiss(args: {
|
||||
const worktree = args.worktreeId ? findWorktreeById(args.state, args.worktreeId) : null
|
||||
const worktreeHead = worktree?.head
|
||||
// Why: merged branch PRs are only safe to keep when cached PR metadata still
|
||||
// matches the commit this stored worktree is actually on.
|
||||
// matches the commit this stored worktree is actually on — exactly, or via a
|
||||
// head main confirmed to be part of the merged PR.
|
||||
const preservesMergedPRForCurrentHead =
|
||||
args.nextPR === null &&
|
||||
args.linkedPRNumber == null &&
|
||||
@@ -1273,7 +1277,8 @@ function shouldPreserveExistingPRForFallbackMiss(args: {
|
||||
args.currentPR.headSha.length > 0 &&
|
||||
typeof worktreeHead === 'string' &&
|
||||
worktreeHead.length > 0 &&
|
||||
args.currentPR.headSha === worktreeHead
|
||||
(args.currentPR.headSha === worktreeHead ||
|
||||
args.currentPR.confirmedContainedHeadOid === worktreeHead)
|
||||
|
||||
// Why: fallback PR numbers come from already-visible cache, not durable
|
||||
// worktree metadata. A branch/fallback miss is weaker than the current exact
|
||||
|
||||
@@ -547,6 +547,39 @@ describe('hosted review slice', () => {
|
||||
expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(2)
|
||||
})
|
||||
|
||||
it('serves a cached merged review whose confirmed contained head matches the worktree', async () => {
|
||||
const mergedBehindHead: HostedReviewInfo = {
|
||||
provider: 'github',
|
||||
number: 7,
|
||||
title: 'Merged with unpulled final head',
|
||||
state: 'merged',
|
||||
url: 'https://github.com/acme/orca/pull/7',
|
||||
status: 'success',
|
||||
updatedAt: '2026-05-10T00:00:00.000Z',
|
||||
mergeable: 'MERGEABLE',
|
||||
headSha: 'aaaaaaa',
|
||||
confirmedContainedHeadOid: 'bbbbbbb'
|
||||
}
|
||||
mockApi.hostedReview.forBranch.mockResolvedValueOnce(mergedBehindHead)
|
||||
const store = makeStore()
|
||||
|
||||
await expect(
|
||||
store.getState().fetchHostedReviewForBranch('/repo', 'feature/merged', {
|
||||
currentHeadOid: 'bbbbbbb'
|
||||
})
|
||||
).resolves.toEqual(mergedBehindHead)
|
||||
|
||||
// Why one call: a merged review confirmed for this worktree head is not
|
||||
// stale, so the second read must reuse the branch-scoped cache instead of
|
||||
// refetching every poll.
|
||||
await expect(
|
||||
store.getState().fetchHostedReviewForBranch('/repo', 'feature/merged', {
|
||||
currentHeadOid: 'bbbbbbb'
|
||||
})
|
||||
).resolves.toEqual(mergedBehindHead)
|
||||
expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('does not preserve a merged GitHub review after the worktree moves off its head', async () => {
|
||||
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const cacheKey = getHostedReviewCacheKey(
|
||||
|
||||
@@ -100,9 +100,10 @@ function isStaleMergedGitHubReviewForHead(
|
||||
cached: CacheEntry<HostedReviewInfo> | undefined,
|
||||
currentHeadOid: string | null | undefined
|
||||
): boolean {
|
||||
// Why: a merged GitHub PR is only shown when the worktree sits on its head.
|
||||
// The cache key is branch-scoped, so a worktree that advanced off the merged
|
||||
// head must not reuse (or, on failure, preserve) the now-stale merged review.
|
||||
// Why: a merged GitHub PR is only shown when the worktree sits on its head
|
||||
// or on a commit confirmed to be part of the PR. The cache key is
|
||||
// branch-scoped, so a worktree that advanced off the merged line of work
|
||||
// must not reuse (or, on failure, preserve) the now-stale merged review.
|
||||
const head = typeof currentHeadOid === 'string' ? currentHeadOid.trim() : ''
|
||||
if (head.length === 0) {
|
||||
return false
|
||||
@@ -113,7 +114,8 @@ function isStaleMergedGitHubReviewForHead(
|
||||
data.state === 'merged' &&
|
||||
typeof data.headSha === 'string' &&
|
||||
data.headSha.length > 0 &&
|
||||
data.headSha !== head
|
||||
data.headSha !== head &&
|
||||
data.confirmedContainedHeadOid !== head
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -120,6 +120,9 @@ export function hostedReviewInfoFromGitHubPRInfo(pr: PRInfo): HostedReviewInfo {
|
||||
...(pr.mergeQueueRequired !== undefined ? { mergeQueueRequired: pr.mergeQueueRequired } : {}),
|
||||
...(pr.mergeStateStatus !== undefined ? { mergeStateStatus: pr.mergeStateStatus } : {}),
|
||||
...(pr.headSha ? { headSha: pr.headSha } : {}),
|
||||
...(pr.confirmedContainedHeadOid
|
||||
? { confirmedContainedHeadOid: pr.confirmedContainedHeadOid }
|
||||
: {}),
|
||||
...(pr.conflictSummary ? { conflictSummary: pr.conflictSummary } : {})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -25,6 +25,9 @@ export type HostedReviewInfo = {
|
||||
mergeQueueRequired?: boolean | null
|
||||
mergeStateStatus?: string | null
|
||||
headSha?: string
|
||||
// Why: mirrors PRInfo.confirmedContainedHeadOid so merged-review staleness
|
||||
// checks accept a worktree head confirmed to be part of the merged PR.
|
||||
confirmedContainedHeadOid?: string
|
||||
/** Target branch name for review-created worktree compare-base repair. */
|
||||
baseRefName?: string
|
||||
conflictSummary?: PRConflictSummary
|
||||
|
||||
@@ -1133,6 +1133,10 @@ export type PRInfo = {
|
||||
// Keeping the head SHA in cached PR metadata lets the checks panel poll the
|
||||
// correct commit without re-querying GitHub or guessing from local branch refs.
|
||||
headSha?: string
|
||||
// Why: a merged branch-matched PR stays visible when the worktree head is one
|
||||
// of the PR's own commits (behind update-branch/web commits). Cache staleness
|
||||
// checks must honor that confirmation without re-querying GitHub.
|
||||
confirmedContainedHeadOid?: string
|
||||
/** Target branch name for PR-created worktree compare-base repair. */
|
||||
baseRefName?: string
|
||||
prRepo?: GitHubRepositoryIdentity
|
||||
|
||||
Reference in New Issue
Block a user