Fix linked reviews on detached workspaces (#28228)

Fix linked reviews on detached workspaces
This commit is contained in:
Neil
2026-10-13 17:15:21 -07:00
committed by GitHub
parent 565e34081a
commit 16efc33a91
9 changed files with 307 additions and 20 deletions
@@ -10,14 +10,15 @@ import {
referenceRepo,
referenceLinearIssue
} from './workspace-reference-fixtures.test-support'
import { getWorkspaceReferenceRequest } from './workspace-reference-details'
import { normalizeTaskSourceContext } from '../../../../shared/task-source-context'
import { getLinearReadScope, scopedLinearCacheKey } from '@/store/slices/linear/linear-slice-scope'
const cache = vi.hoisted(() => ({ entries: {} }))
const cache = vi.hoisted(() => ({ entries: {}, hosted: {} }))
vi.mock('@/store', () => ({
useAppStore: (selector: (state: unknown) => unknown) =>
selector({
hostedReviewCache: {},
hostedReviewCache: cache.hosted,
prCache: {},
issueCache: {},
linearIssueCache: cache.entries
@@ -42,6 +43,62 @@ vi.mock('./workspace-delete-quick-action', () => ({
afterEach(cleanup)
describe('single linked reference rich hover details', () => {
it.each([
{
data: {
provider: 'gitlab' as const,
number: 8,
title: 'Fix detached review',
state: 'open' as const,
status: 'success' as const,
updatedAt: '',
mergeable: 'MERGEABLE' as const,
url: 'https://gitlab.example.com/team/repo/-/merge_requests/8'
},
title: 'Fix detached review'
},
{ data: null, title: 'MR details unavailable' }
])(
'reads detached MR details from the same exact cache as Linked work: $title',
({ data, title }) => {
const repo = {
...referenceRepo,
gitRemoteIdentity: {
canonicalKey: 'gitlab.example.com/team/repo',
remoteName: 'origin',
remoteUrl: 'https://gitlab.example.com/team/repo.git'
}
}
const item = {
provider: 'gitlab' as const,
type: 'mr' as const,
number: 8,
url: 'https://gitlab.example.com/team/repo/-/merge_requests/8'
}
const workspace = {
...referenceWorkspace,
branch: '',
linkedPR: null,
linkedGitLabMR: 8,
linkedItems: [item]
}
const key = getWorkspaceReferenceRequest(item, workspace, repo).key
cache.hosted = { [key]: { data, fetchedAt: Date.now(), linkedReviewHintKey: 'gitlab:8' } }
const { result } = renderHook(() =>
useWorktreeCardReviewDetails({
worktree: workspace,
repo,
settings: null,
projectGroups: [],
cardProps: ['pr'],
newCardStyle: false
})
)
expect(result.current.hostedReviewCacheKey).toBe(key)
expect(result.current.prDisplay?.title).toBe(title)
}
)
it('reuses source-bound Linear details when an exact workspace read is outside the legacy all-workspaces cache slot', () => {
const source = normalizeTaskSourceContext({
provider: 'linear',
@@ -1,4 +1,5 @@
import { useEffect } from 'react'
import { getRepoExecutionHostId } from '../../../../shared/execution-host'
import { isMacAppDataPath } from '@/lib/passive-macos-app-data-access'
import { installWindowVisibilityInterval, isWindowVisible } from '@/lib/window-visibility-interval'
@@ -55,6 +56,7 @@ export function useWorktreeCardLifecycleEffects({
showIssue: boolean
showLinearIssue: boolean
}): void {
const hostedReviewExactKey = !branch || branch === 'HEAD' ? hostedReviewCacheKey : undefined
useEffect(() => {
if (
!newCardStyle ||
@@ -73,11 +75,13 @@ export function useWorktreeCardLifecycleEffects({
// Why: hidden card metadata is revealed on whole-card hover, so fetch lazily instead of always-on polling.
void fetchHostedReviewForBranch(repo.path, branch, {
repoId: repo.id,
repoOwnerExecutionHostId: getRepoExecutionHostId(repo),
...(hostedReviewExactKey ? { exactReviewKey: hostedReviewExactKey } : {}),
linkedGitHubPR: worktree.linkedPR ?? null,
...(cachedBranchFallbackGitHubPRNumber !== null
? { fallbackGitHubPR: cachedBranchFallbackGitHubPRNumber }
: {}),
currentHeadOid: worktree.head ?? null,
currentHeadOid: hostedReviewExactKey ? null : (worktree.head ?? null),
linkedGitLabMR,
linkedBitbucketPR,
linkedAzureDevOpsPR,
@@ -100,7 +104,8 @@ export function useWorktreeCardLifecycleEffects({
linkedGiteaPR,
fetchHostedReviewForBranch,
branch,
hostedReviewCacheKey
hostedReviewCacheKey,
hostedReviewExactKey
])
// Why: same as above for issues — hidden-surface polling only burns GitHub calls for invisible data.
@@ -1,4 +1,5 @@
import { useMemo } from 'react'
import { getDetachedWorkspaceReviewRequest } from './workspace-detached-review-request'
import { getLinearReadScope, scopedLinearCacheKey } from '@/store/slices/linear/linear-slice-scope'
import { getWorkspaceAttachmentSourceContext } from './workspace-attachment-source-result'
import { getWorkspaceReferenceLinearWorkspaceId } from './workspace-reference-details'
@@ -47,8 +48,10 @@ export function useWorktreeCardReviewDetails({
const folderMetaRowContent = newCardStyle
? hasPathIdentityEnabled && Boolean(folderPathIdentityDisplay)
: isFolder
const hostedReviewExactKey = getDetachedWorkspaceReviewRequest(worktree, repo)?.key
const hostedReviewCacheKey =
repo && branch
hostedReviewExactKey ??
(repo && branch
? getHostedReviewCacheKey(
repo.path,
branch,
@@ -58,7 +61,7 @@ export function useWorktreeCardReviewDetails({
repo.executionHostId,
true
)
: ''
: '')
const prCacheKey =
repo && branch
? getGitHubPRCacheKey(
@@ -0,0 +1,23 @@
import type { Repo } from '../../../../shared/repo-types'
import type { Worktree } from '../../../../shared/worktree/types'
import { getWorkspaceAttachments } from '../../../../shared/workspace-attachments'
import { getWorkspaceAttachmentKey } from '../../../../shared/workspace-attachment-normalization'
import { getActiveWorkspaceReviewKey } from './worktree-attachment-editing'
import { getWorkspaceReferenceRequest } from './workspace-reference-details'
import { canReadWorkspaceReferenceReview } from './workspace-reference-review-source'
export function getDetachedWorkspaceReviewRequest(workspace: Worktree, repo?: Repo) {
const branch = workspace.branch.replace(/^refs\/heads\//, '').trim()
if (!repo || (branch && branch !== 'HEAD')) {
return null
}
const selectedKey = getActiveWorkspaceReviewKey(workspace)
const item = getWorkspaceAttachments(workspace).find(
(attachment) => getWorkspaceAttachmentKey(attachment) === selectedKey
)
if (!item) {
return null
}
const request = getWorkspaceReferenceRequest(item, workspace, repo)
return canReadWorkspaceReferenceReview(request) ? request : null
}
@@ -63,6 +63,24 @@ beforeEach(() => {
afterEach(() => {
vi.useRealTimers()
})
it('leaves the detached primary review to its visible target while retaining sibling reviews', () => {
useAppStore.setState({
worktreesByRepo: {
repo: [
{
...workspace,
branch: '',
linkedPR: 7,
linkedItems: [referenceAttachment(7), referenceAttachment(8)]
}
]
}
})
const targets = update()
expect(targets).toHaveLength(1)
expect(targets[0]?.key).toContain('8')
})
it('pauses disconnected SSH owners and only admits the owning reconnect', () => {
useAppStore.setState({ repos: [{ ...referenceRepo, connectionId: 'ssh-a' }] })
expect(update()).toEqual([])
@@ -13,6 +13,7 @@ import {
} from './workspace-reference-details'
import { canReadWorkspaceReferenceReview } from './workspace-reference-review-source'
import { readWorkspaceReferenceDetails } from './workspace-reference-detail-read'
import { getDetachedWorkspaceReviewRequest } from './workspace-detached-review-request'
import { selectIsSleepingWorktree } from './use-worktree-sleep-state'
export function getWorkspaceReferenceRefreshTargets(
@@ -39,6 +40,7 @@ export function getWorkspaceReferenceRefreshTargets(
const knownProvider =
state.hostedReviewCache[branchKey]?.data?.provider ??
(state.prCache[candidate.cacheKey]?.data ? 'github' : undefined)
const primaryExactKey = getDetachedWorkspaceReviewRequest(worktree, repo)?.key
const runtimeVersion = getWorkspaceReferenceRuntimeVersion(worktree, repo)
for (const item of getWorkspaceAttachments(worktree)) {
if (item.type === 'issue') {
@@ -52,6 +54,10 @@ export function getWorkspaceReferenceRefreshTargets(
continue
}
const key = request.key
// The primary detached review already has a visible refresh target.
if (key === primaryExactKey) {
continue
}
if (targets.get(key)?.selected) {
continue
}
@@ -0,0 +1,126 @@
import { beforeEach, expect, it, vi } from 'vitest'
import type { Repo } from '../../../../shared/repo-types'
import { createGlobalSettingsFixture } from '../../../../shared/global-settings-test-fixture'
import { getWorkspaceReferenceRequest } from '@/components/sidebar/workspace-reference-details'
import {
createTestStore,
makePRRefreshWorktree,
mockApi,
resetRemoteRuntimeMocks,
runtimeEnvironmentCall
} from '../slices/github-slice-test-harness'
import { getVisibleHostedReviewRefreshTargets } from './visible-hosted-review-refresh-targets'
const owner: Repo = { id: 'repo-1', path: '/repo', displayName: 'repo', badgeColor: '', addedAt: 0 }
const providers = [
['github', 'linkedPR', 'github.com/team/repo', 'pull'],
['gitlab', 'linkedGitLabMR', 'gitlab.example.com/team/repo', '-/merge_requests'],
['bitbucket', 'linkedBitbucketPR', 'bitbucket.org/team/repo', 'pull-requests'],
['azure-devops', 'linkedAzureDevOpsPR', 'dev.azure.com/team/project/_git/repo', 'pullrequest'],
['gitea', 'linkedGiteaPR', 'codeberg.org/team/repo', 'pulls']
] as const
beforeEach(() => {
vi.clearAllMocks()
resetRemoteRuntimeMocks()
mockApi.hostedReview.forBranch.mockResolvedValue(null)
})
it.each(providers)(
'loads exact detached %s reviews without branch discovery or cache collisions',
async (provider, slot, canonicalKey, route) => {
const repo: Repo = {
...owner,
gitRemoteIdentity: {
canonicalKey,
remoteName: 'origin',
remoteUrl: `https://${canonicalKey}.git`
}
}
const items = [8, 9].map((number) => ({
provider,
type: provider === 'gitlab' ? ('mr' as const) : ('pr' as const),
number,
url: `https://${canonicalKey}/${route}/${number}`
}))
const workspaces = items.map((item) =>
makePRRefreshWorktree({
id: `detached-${item.number}`,
branch: '',
[slot]: item.number,
linkedItems: [item]
})
)
const store = createTestStore()
store.setState({
repos: [repo],
settings: null,
worktreesByRepo: { [repo.id]: workspaces },
activeWorktreeId: null,
visibleReviewWorktreeIds: workspaces.map((workspace) => workspace.id),
sshConnectionStates: new Map()
})
const targets = getVisibleHostedReviewRefreshTargets(store.getState(), store.getState)
expect(targets).toHaveLength(2)
expect(new Set(targets.map((target) => target.key)).size).toBe(2)
await Promise.all(targets.map((target) => target.refresh()))
for (const [index, workspace] of workspaces.entries()) {
const key = getWorkspaceReferenceRequest(items[index], workspace, repo).key
expect(store.getState().hostedReviewCache[key]?.data).toBeNull()
}
expect(mockApi.hostedReview.forBranch).toHaveBeenCalledTimes(2)
expect(mockApi.gh.prForBranch).not.toHaveBeenCalled()
}
)
it.each(['runtime:owner-server', 'ssh:owner-server'] as const)(
'retains %s ownership while focus names another server',
async (executionHostId) => {
const repo: Repo = {
...owner,
executionHostId,
...(executionHostId.startsWith('ssh:') ? { connectionId: 'owner-server' } : {}),
gitRemoteIdentity: {
canonicalKey: 'gitlab.com/team/repo',
remoteName: 'origin',
remoteUrl: 'https://gitlab.com/team/repo.git'
}
}
const workspace = makePRRefreshWorktree({ branch: '', linkedGitLabMR: 8 })
const store = createTestStore()
store.setState({
repos: [repo],
settings: createGlobalSettingsFixture({ activeRuntimeEnvironmentId: 'other-server' }),
worktreesByRepo: { [repo.id]: [workspace] },
visibleReviewWorktreeIds: [workspace.id],
activeWorktreeId: workspace.id,
sshConnectionStates: new Map([
[
'owner-server',
{ targetId: 'owner-server', status: 'connected', error: null, reconnectAttempt: 0 }
]
])
})
runtimeEnvironmentCall.mockResolvedValue({ id: 'review', ok: true, result: null })
const targets = getVisibleHostedReviewRefreshTargets(store.getState(), store.getState, {
selectedOnly: true
})
expect(targets).toHaveLength(1)
await targets[0].refresh()
if (executionHostId.startsWith('runtime:')) {
expect(runtimeEnvironmentCall).toHaveBeenCalledWith(
expect.objectContaining({
selector: 'owner-server',
method: 'hostedReview.forBranch',
params: expect.objectContaining({ branch: '', linkedGitLabMR: 8 })
})
)
expect(mockApi.hostedReview.forBranch).not.toHaveBeenCalled()
} else {
expect(mockApi.hostedReview.forBranch).toHaveBeenCalledWith(
expect.objectContaining({ repoOwnerExecutionHostId: executionHostId })
)
expect(runtimeEnvironmentCall).not.toHaveBeenCalled()
}
}
)
@@ -16,6 +16,7 @@ import {
getVisibleHostedReviewRefreshTargets,
visibleHostedReviewRefreshInputsChanged
} from './visible-hosted-review-refresh-targets'
import { getWorkspaceReferenceRequest } from '@/components/sidebar/workspace-reference-details'
import { createVisibleHostedReviewRefreshScheduler } from './visible-hosted-review-refresh-scheduler'
const repo: Repo = { id: 'repo-1', path: '/repo', displayName: 'repo', badgeColor: '', addedAt: 0 }
@@ -103,6 +104,47 @@ describe('visible hosted review refresh targets', () => {
)
})
it.each([review, null])(
'resolves a detached linked MR to a title or a settled miss: %j',
async (result) => {
const owner: Repo = {
...repo,
gitRemoteIdentity: {
canonicalKey: 'gitlab.example.com/team/repo',
remoteName: 'origin',
remoteUrl: 'https://gitlab.example.com/team/repo.git'
}
}
const item = {
provider: 'gitlab' as const,
type: 'mr' as const,
number: 8,
url: 'https://gitlab.example.com/team/repo/-/merge_requests/8'
}
const detached = { ...worktree, branch: '', linkedGitLabMR: 8, linkedItems: [item] }
const store = setup(owner)
store.setState({ worktreesByRepo: { [repo.id]: [detached] }, activeWorktreeId: detached.id })
mockApi.hostedReview.forBranch.mockResolvedValue(result)
const exactKey = getWorkspaceReferenceRequest(item, detached, owner).key
const rows = getVisibleHostedReviewRefreshTargets(store.getState(), store.getState, {
selectedOnly: true
})
expect(rows).toHaveLength(1)
expect(rows[0].key).toBe(exactKey)
expect(await rows[0].refresh()).toBe(true)
expect(mockApi.hostedReview.forBranch).toHaveBeenCalledWith(
expect.objectContaining({
branch: '',
linkedGitLabMR: 8,
repoOwnerExecutionHostId: 'local',
currentHeadOid: null
})
)
expect(store.getState().hostedReviewCache[exactKey]?.data).toEqual(result)
expect(mockApi.gh.prForBranch).not.toHaveBeenCalled()
}
)
it('uses the repo runtime owner rather than the focused server and fills both caches', async () => {
const owner: Repo = { ...repo, executionHostId: 'runtime:owner-server' }
const store = setup(owner)
@@ -1,3 +1,4 @@
import { getDetachedWorkspaceReviewRequest } from '@/components/sidebar/workspace-detached-review-request'
import type { AppState } from '../types'
import {
getHostedReviewCacheKey,
@@ -87,18 +88,21 @@ export function getVisibleHostedReviewRefreshTargets(
options
)) {
const id = worktree.id
if (!candidate.branch || candidate.branch === 'HEAD') {
const exactRequest = getDetachedWorkspaceReviewRequest(worktree, repo)
if ((!candidate.branch || candidate.branch === 'HEAD') && !exactRequest) {
continue
}
const key = getHostedReviewCacheKey(
candidate.repoPath,
candidate.branch,
state.settings,
candidate.repoId,
candidate.connectionId,
candidate.executionHostId,
true
)
const key =
exactRequest?.key ??
getHostedReviewCacheKey(
candidate.repoPath,
candidate.branch,
state.settings,
candidate.repoId,
candidate.connectionId,
candidate.executionHostId,
true
)
const hostedEntry = state.hostedReviewCache[key]
const prEntry = state.prCache[candidate.cacheKey]
const hints = {
@@ -116,12 +120,14 @@ export function getVisibleHostedReviewRefreshTargets(
(hostedEntry?.data != null && hostedEntry.data.provider !== 'github')
// The legacy GitHub action still resolves bare IDs; never pass it another owner's candidate.
if (
getGitHubRepoLookupIndex(state.repos).findById(repo.id) !== repo ||
findWorktreeById(state, worktree.id) !== worktree
!exactRequest &&
(getGitHubRepoLookupIndex(state.repos).findById(repo.id) !== repo ||
findWorktreeById(state, worktree.id) !== worktree)
) {
continue
}
const knownGitHub = shouldCoordinateVisibleGitHubReview(state, worktree, candidate)
const knownGitHub =
!exactRequest && shouldCoordinateVisibleGitHubReview(state, worktree, candidate)
const runtime = knownGitHub ? getPRRefreshRuntimeRepoTarget(state, candidate) : null
if (knownGitHub && !runtime) {
continue
@@ -166,11 +172,12 @@ export function getVisibleHostedReviewRefreshTargets(
})
: before.fetchHostedReviewForBranch(candidate.repoPath, candidate.branch, {
...hints,
...(exactRequest ? { exactReviewKey: key } : {}),
force,
repoId: candidate.repoId,
repoOwnerExecutionHostId: getRepoExecutionHostId(candidate),
fallbackGitHubPR: nonGitHub ? null : candidate.fallbackPRNumber,
currentHeadOid: candidate.currentHeadOid,
currentHeadOid: exactRequest ? null : candidate.currentHeadOid,
active: selected,
admissionTier: selected ? 'interactive' : 'background'
}))