diff --git a/src/renderer/src/components/GitHubItemDialog.tsx b/src/renderer/src/components/GitHubItemDialog.tsx index 0e50c3bbc50..fc1f9505114 100644 --- a/src/renderer/src/components/GitHubItemDialog.tsx +++ b/src/renderer/src/components/GitHubItemDialog.tsx @@ -141,6 +141,7 @@ import { buildPRCommentConversationReplyBody } from '@/components/right-sidebar/ import { useAppStore } from '@/store' import { useAllWorktrees } from '@/store/selectors' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' +import { withGitHubCheckDetailsTimeout } from '@/runtime/github-check-details-timeout' import { useRepoLabels, useRepoAssignees, useImmediateMutation } from '@/hooks/useIssueMetadata' import { useRepoLabelsBySlug, useRepoAssigneesBySlug } from '@/hooks/useGitHubSlugMetadata' import { GitHubMarkdownComposer } from '@/components/github/GitHubMarkdownComposer' @@ -3473,16 +3474,18 @@ function ChecksTab({ }, { timeoutMs: 30_000 } ) - : window.api.gh.prCheckDetails({ - repoPath: repoPath ?? '', - repoId: repoId ?? undefined, - sourceContext, - checkRunId: check.checkRunId, - workflowRunId: check.workflowRunId, - checkName: check.name, - url: check.url, - prRepo - }) + : withGitHubCheckDetailsTimeout( + window.api.gh.prCheckDetails({ + repoPath: repoPath ?? '', + repoId: repoId ?? undefined, + sourceContext, + checkRunId: check.checkRunId, + workflowRunId: check.workflowRunId, + checkName: check.name, + url: check.url, + prRepo + }) + ) void detailsRequest .then((details) => { if (!mountedRef.current) { diff --git a/src/renderer/src/components/PullRequestPage.tsx b/src/renderer/src/components/PullRequestPage.tsx index 4b46d0148e5..4ef21776947 100644 --- a/src/renderer/src/components/PullRequestPage.tsx +++ b/src/renderer/src/components/PullRequestPage.tsx @@ -134,6 +134,7 @@ import { buildPRCommentConversationReplyBody } from '@/components/right-sidebar/ import { useAppStore } from '@/store' import { useAllWorktrees } from '@/store/selectors' import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' +import { withGitHubCheckDetailsTimeout } from '@/runtime/github-check-details-timeout' import { useRepoLabels, useRepoAssignees, useImmediateMutation } from '@/hooks/useIssueMetadata' import { useRepoLabelsBySlug, useRepoAssigneesBySlug } from '@/hooks/useGitHubSlugMetadata' import { @@ -3557,16 +3558,18 @@ function ChecksTab({ }, { timeoutMs: 30_000 } ) - : window.api.gh.prCheckDetails({ - repoPath: repoPath ?? '', - repoId: repoId ?? undefined, - sourceContext, - checkRunId: check.checkRunId, - workflowRunId: check.workflowRunId, - checkName: check.name, - url: check.url, - prRepo - }) + : withGitHubCheckDetailsTimeout( + window.api.gh.prCheckDetails({ + repoPath: repoPath ?? '', + repoId: repoId ?? undefined, + sourceContext, + checkRunId: check.checkRunId, + workflowRunId: check.workflowRunId, + checkName: check.name, + url: check.url, + prRepo + }) + ) void detailsRequest .then((details) => { if (!mountedRef.current) { diff --git a/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx b/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx index 2a9a8c4ecd5..50350fb1b47 100644 --- a/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx +++ b/src/renderer/src/components/editor/CheckRunDetailsPanel.tsx @@ -247,7 +247,11 @@ export function CheckRunDetailsPanel({
{loading ? ( -
+
{translate( 'auto.components.editor.CheckRunDetailsPanel.1f2b980522', @@ -256,7 +260,11 @@ export function CheckRunDetailsPanel({
) : (
- {error &&
{error}
} + {error && ( +
+ {error} +
+ )} {hasOutput && (
diff --git a/src/renderer/src/components/editor/check-run-details-tab.ts b/src/renderer/src/components/editor/check-run-details-tab.ts index bbc8fc1180b..02e833b61b6 100644 --- a/src/renderer/src/components/editor/check-run-details-tab.ts +++ b/src/renderer/src/components/editor/check-run-details-tab.ts @@ -1,21 +1,34 @@ import type { GitLabProjectRef } from '../../../../shared/gitlab-types' -import type { PRCheckDetail, PRCheckRunDetails } from '../../../../shared/types' +import type { + GitHubRepositoryIdentity, + PRCheckDetail, + PRCheckRunDetails +} from '../../../../shared/types' export type OpenCheckRunDetailsState = { contextKey: string check: PRCheckDetail + requestId?: number details: PRCheckRunDetails | null loading: boolean error: string | null + githubRepository?: GitHubRepositoryIdentity | null /** Why: fork/cross-project MR jobs live outside the repo's own project, so reloads need the pipeline's project. */ gitlabProjectRef?: GitLabProjectRef | null } export type CheckRunDetailsTabPatch = Pick< OpenCheckRunDetailsState, - 'details' | 'loading' | 'error' | 'gitlabProjectRef' + 'requestId' | 'details' | 'loading' | 'error' | 'githubRepository' | 'gitlabProjectRef' > +let nextCheckRunDetailsRequestId = 0 + +export function createCheckRunDetailsRequestId(): number { + nextCheckRunDetailsRequestId += 1 + return nextCheckRunDetailsRequestId +} + export function isSameGitLabProjectRef( a: GitLabProjectRef | null, b: GitLabProjectRef | null @@ -23,6 +36,13 @@ export function isSameGitLabProjectRef( return a === b || (a?.host === b?.host && a?.path === b?.path) } +export function isSameGitHubRepository( + a: GitHubRepositoryIdentity | null, + b: GitHubRepositoryIdentity | null +): boolean { + return a === b || (a?.owner === b?.owner && a?.repo === b?.repo && a?.host === b?.host) +} + export function getCheckRunTabIdentity(check: PRCheckDetail): string { if (check.checkRunId) { return `check-run:${check.checkRunId}` diff --git a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts index a773910d332..265aeaf9417 100644 --- a/src/renderer/src/components/github-item-dialog-source-boundary.test.ts +++ b/src/renderer/src/components/github-item-dialog-source-boundary.test.ts @@ -230,6 +230,9 @@ describe('GitHubItemDialog source host boundaries', () => { expect(checksSection).toContain('window.api.gh.prChecks({') expect(checksSection).toContain('window.api.gh.rerunPRChecks({') expect(checksSection).toContain('prCheckDetails({') + expect(checksSection).toMatch( + /withGitHubCheckDetailsTimeout\(\s*window\.api\.gh\.prCheckDetails\(\{/ + ) }) it('uses hydrated work item details for the page checks tab', () => { diff --git a/src/renderer/src/components/pull-request-page-host-boundary.test.ts b/src/renderer/src/components/pull-request-page-host-boundary.test.ts index 10407760e6d..84c391b92f0 100644 --- a/src/renderer/src/components/pull-request-page-host-boundary.test.ts +++ b/src/renderer/src/components/pull-request-page-host-boundary.test.ts @@ -249,6 +249,9 @@ describe('PullRequestPage host boundaries', () => { expect(checksSection).toContain('window.api.gh.prChecks({') expect(checksSection).toContain('window.api.gh.rerunPRChecks({') expect(checksSection).toContain('prCheckDetails({') + expect(checksSection).toMatch( + /withGitHubCheckDetailsTimeout\(\s*window\.api\.gh\.prCheckDetails\(\{/ + ) }) it('routes edit metadata and mutations through the PR source context', () => { diff --git a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx index 9fc66a78819..d41233815d1 100644 --- a/src/renderer/src/components/right-sidebar/ChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/ChecksPanel.tsx @@ -4532,6 +4532,7 @@ export default function ChecksPanel(): React.JSX.Element { checksLoading={checksLoading} checkDetailsContextKey={stateRequestKey} onLoadCheckDetails={handleLoadCheckDetails} + githubRepository={pr?.prRepo ?? null} getGitLabProjectRef={getGitLabProjectRef} /> )} diff --git a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx index 0c64561edec..f2e3bdda640 100644 --- a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx +++ b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksPanel.tsx @@ -181,7 +181,7 @@ export default function FolderWorkspacePrChecksPanel({ workflowRunId: check.workflowRunId, checkName: check.name, url: check.url, - prRepo: null + prRepo: row.githubRepository ?? null }, { repoId: row.repo.id } ) diff --git a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx index 29d82748467..cd07701bde7 100644 --- a/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx +++ b/src/renderer/src/components/right-sidebar/FolderWorkspacePrChecksRow.tsx @@ -128,6 +128,7 @@ export function FolderWorkspacePrChecksRow({ checksLoading={row.isRefreshing} checkDetailsContextKey={row.refreshIdentity} onLoadCheckDetails={onLoadCheckDetails} + githubRepository={row.githubRepository ?? null} worktreeId={row.worktree.id} detailsStickySurface="card" /> diff --git a/src/renderer/src/components/right-sidebar/checks-list-expanded-details.test.tsx b/src/renderer/src/components/right-sidebar/checks-list-expanded-details.test.tsx index 1a4bfdd8b58..f3b61948121 100644 --- a/src/renderer/src/components/right-sidebar/checks-list-expanded-details.test.tsx +++ b/src/renderer/src/components/right-sidebar/checks-list-expanded-details.test.tsx @@ -7,6 +7,8 @@ import { TooltipProvider } from '@/components/ui/tooltip' import type { PRCheckDetail, PRCheckRunDetails } from '../../../../shared/types' import { ChecksList } from './checks-panel-content' +globalThis.IS_REACT_ACT_ENVIRONMENT = true + const openCheckRunDetails = vi.fn() const patchOpenCheckRunDetails = vi.fn() const activeWorktreeState = vi.hoisted(() => ({ @@ -84,6 +86,7 @@ function renderChecksList( props: Partial<{ worktreeId: string detailsStickySurface: 'sidebar' | 'card' + checkDetailsContextKey: string checks: PRCheckDetail[] onLoadCheckDetails: (check: PRCheckDetail) => Promise }> = {} @@ -94,7 +97,7 @@ function renderChecksList( { expect(stickyBar?.textContent).not.toContain('View full logs') }) + it('finishes the full-details tab when its sidebar unmounts during loading', async () => { + let resolveDetails: (details: PRCheckRunDetails) => void = () => {} + const request = new Promise((resolve) => { + resolveDetails = resolve + }) + renderChecksList({ worktreeId: 'wt-child-1', onLoadCheckDetails: () => request }) + + await act(async () => { + await Promise.resolve() + }) + patchOpenCheckRunDetails.mockClear() + const button = [...container.querySelectorAll('button')].find((candidate) => + candidate.textContent?.includes('View full details') + ) + + act(() => { + button!.dispatchEvent(new MouseEvent('click', { bubbles: true })) + root.render(
) + }) + await act(async () => { + resolveDetails(checkDetails) + await request + }) + + expect(patchOpenCheckRunDetails).toHaveBeenCalledWith( + 'wt-child-1', + 'repo:42', + failingCheck, + expect.objectContaining({ details: checkDetails, loading: false, error: null }) + ) + }) + + it('shows a load error in the full-details tab after its sidebar unmounts', async () => { + let rejectDetails: (error: Error) => void = () => {} + const request = new Promise((_resolve, reject) => { + rejectDetails = reject + }) + renderChecksList({ worktreeId: 'wt-child-1', onLoadCheckDetails: () => request }) + + await act(async () => { + await Promise.resolve() + }) + patchOpenCheckRunDetails.mockClear() + const button = [...container.querySelectorAll('button')].find((candidate) => + candidate.textContent?.includes('View full details') + ) + + act(() => { + button!.dispatchEvent(new MouseEvent('click', { bubbles: true })) + root.render(
) + }) + await act(async () => { + rejectDetails(new Error('GitHub request failed')) + await request.catch(() => undefined) + }) + + expect(patchOpenCheckRunDetails).toHaveBeenCalledWith( + 'wt-child-1', + 'repo:42', + failingCheck, + expect.objectContaining({ details: null, loading: false, error: 'GitHub request failed' }) + ) + }) + + it('retries an inline details error', async () => { + const onLoadCheckDetails = vi + .fn<() => Promise>() + .mockRejectedValueOnce(new Error('GitHub request failed')) + .mockResolvedValueOnce(checkDetails) + renderChecksList({ onLoadCheckDetails }) + + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + const retry = [...container.querySelectorAll('button')].find( + (candidate) => candidate.textContent?.trim() === 'Retry' + ) + + await act(async () => { + retry!.dispatchEvent(new MouseEvent('click', { bubbles: true })) + await Promise.resolve() + await Promise.resolve() + }) + + expect(onLoadCheckDetails).toHaveBeenCalledTimes(2) + expect(container.textContent).toContain('Verify failed') + }) + + it('ignores a stale inline result after returning to the same context', async () => { + const requests: { + resolve: (details: PRCheckRunDetails) => void + reject: (error: Error) => void + }[] = [] + const onLoadCheckDetails = vi.fn( + () => + new Promise((resolve, reject) => { + requests.push({ resolve, reject }) + }) + ) + + renderChecksList({ checkDetailsContextKey: 'repo:A', onLoadCheckDetails }) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + renderChecksList({ checkDetailsContextKey: 'repo:B', onLoadCheckDetails }) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + renderChecksList({ checkDetailsContextKey: 'repo:A', onLoadCheckDetails }) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + + expect(requests).toHaveLength(3) + await act(async () => { + requests[2]!.resolve(checkDetails) + await Promise.resolve() + }) + await act(async () => { + requests[0]!.reject(new Error('stale request failed')) + await Promise.resolve() + }) + + expect(container.textContent).toContain('Verify failed') + expect(container.textContent).not.toContain('stale request failed') + }) + it('uses resolved details when showing the action-required fallback hint', async () => { renderChecksList({ onLoadCheckDetails: async () => ({ diff --git a/src/renderer/src/components/right-sidebar/checks-panel-content.tsx b/src/renderer/src/components/right-sidebar/checks-panel-content.tsx index 2b2b695b189..d4fd6ece571 100644 --- a/src/renderer/src/components/right-sidebar/checks-panel-content.tsx +++ b/src/renderer/src/components/right-sidebar/checks-panel-content.tsx @@ -88,6 +88,7 @@ import type { PRCheckRunDetails, PRComment, GitHubReactionContent, + GitHubRepositoryIdentity, PRConflictSummary, PRMergeableState } from '../../../../shared/types' @@ -105,6 +106,7 @@ import { useActiveWorktree } from '@/store/selectors' import { useAppStore } from '@/store' import { sortChecksBySeverity } from '../../../../shared/pr-check-severity-order' import { summarizeProviderChecks } from '../../../../shared/provider-check-summary' +import { createCheckRunDetailsRequestId } from '@/components/editor/check-run-details-tab' export const PullRequestIcon = GitPullRequest @@ -542,6 +544,7 @@ export function ConflictTriageStrip({ } type CheckDetailsLoadState = { + requestId?: number loading: boolean details: PRCheckRunDetails | null error: string | null @@ -673,7 +676,9 @@ function CheckRunDetails({ checkDetailsContextKey, worktreeId, detailsStickySurface = 'sidebar', - getGitLabProjectRef + getGitLabProjectRef, + githubRepository, + onRetry }: { check: PRCheckDetail state: CheckDetailsLoadState | undefined @@ -682,6 +687,8 @@ function CheckRunDetails({ detailsStickySurface?: CheckDetailsStickySurface /** Why: a getter, not a value — the source ref is filled by an async fetch and would read stale during render. */ getGitLabProjectRef?: () => GitLabProjectRef | null + githubRepository?: GitHubRepositoryIdentity | null + onRetry: () => void }): React.JSX.Element { const openCheckRunDetails = useAppStore((s) => s.openCheckRunDetails) const details = state?.details @@ -718,9 +725,11 @@ function CheckRunDetails({ return } openCheckRunDetails(worktreeId, checkDetailsContextKey, check, { + requestId: state?.requestId, details: state?.details ?? null, loading: state?.loading ?? false, error: state?.error ?? null, + githubRepository: githubRepository ?? null, gitlabProjectRef: getGitLabProjectRef?.() ?? null }) } @@ -748,7 +757,7 @@ function CheckRunDetails({
)} {state?.loading ? ( -
+
{translate( @@ -805,7 +814,26 @@ function CheckRunDetails({ )}
- {state?.error &&
{state.error}
} + {state?.error && ( +
+ + {state.error} + + +
+ )} {hasOutput && (
@@ -973,7 +1001,8 @@ export function ChecksList({ onLoadCheckDetails, worktreeId: worktreeIdOverride, detailsStickySurface = 'sidebar', - getGitLabProjectRef + getGitLabProjectRef, + githubRepository }: { checks: PRCheckDetail[] checksLoading: boolean @@ -984,6 +1013,7 @@ export function ChecksList({ detailsStickySurface?: CheckDetailsStickySurface /** Why: a getter, not a value — the source ref is filled by an async fetch and would read stale during render. */ getGitLabProjectRef?: () => GitLabProjectRef | null + githubRepository?: GitHubRepositoryIdentity | null }): React.JSX.Element { const activeWorktree = useActiveWorktree() const resolvedWorktreeId = worktreeIdOverride ?? activeWorktree?.id ?? null @@ -1112,55 +1142,113 @@ export function ChecksList({ return } const requestContextKey = checkDetailsContextKey + const requestId = createCheckRunDetailsRequestId() setDetailsByCheckKey((current) => ({ ...current, - [row.key]: { loading: true, details: null, error: null } + [row.key]: { requestId, loading: true, details: null, error: null } })) - void onLoadCheckDetails(row.check) + if (resolvedWorktreeId) { + patchOpenCheckRunDetails(resolvedWorktreeId, requestContextKey, row.check, { + requestId, + details: null, + loading: true, + error: null, + githubRepository: githubRepository ?? null, + gitlabProjectRef: getGitLabProjectRef?.() ?? null + }) + } + const request = Promise.resolve().then(() => onLoadCheckDetails(row.check)) + void request .then((details) => { - if (detailsContextRef.current !== requestContextKey) { - return - } - setDetailsByCheckKey((current) => ({ - ...current, - [row.key]: { - loading: false, + if (resolvedWorktreeId) { + patchOpenCheckRunDetails(resolvedWorktreeId, requestContextKey, row.check, { + requestId, details, + loading: false, error: details ? null : translate( 'auto.components.right.sidebar.checks.panel.content.e15a8b77ef', 'No inline details are available for this check.' ), - // Why: a detail-less result is only final for this status — re-arm the retry once the job moves on. - errorAt: details - ? undefined - : { status: row.check.status, conclusion: row.check.conclusion } - } - })) - }) - .catch((err) => { + githubRepository: githubRepository ?? null, + gitlabProjectRef: getGitLabProjectRef?.() ?? null + }) + } if (detailsContextRef.current !== requestContextKey) { return } - setDetailsByCheckKey((current) => ({ - ...current, - [row.key]: { - loading: false, - details: null, - error: - err instanceof Error - ? err.message - : translate( - 'auto.components.right.sidebar.checks.panel.content.checkDetailsLoadFailed', - 'Failed to load check details.' - ), - errorAt: { status: row.check.status, conclusion: row.check.conclusion } + setDetailsByCheckKey((current) => { + if (current[row.key]?.requestId !== requestId) { + return current } - })) + return { + ...current, + [row.key]: { + requestId, + loading: false, + details, + error: details + ? null + : translate( + 'auto.components.right.sidebar.checks.panel.content.e15a8b77ef', + 'No inline details are available for this check.' + ), + // Why: a detail-less result is only final for this status — re-arm the retry once the job moves on. + errorAt: details + ? undefined + : { status: row.check.status, conclusion: row.check.conclusion } + } + } + }) + }) + .catch((err) => { + const error = + err instanceof Error + ? err.message + : translate( + 'auto.components.right.sidebar.checks.panel.content.checkDetailsLoadFailed', + 'Failed to load check details.' + ) + if (resolvedWorktreeId) { + patchOpenCheckRunDetails(resolvedWorktreeId, requestContextKey, row.check, { + requestId, + details: null, + loading: false, + error, + githubRepository: githubRepository ?? null, + gitlabProjectRef: getGitLabProjectRef?.() ?? null + }) + } + if (detailsContextRef.current !== requestContextKey) { + return + } + setDetailsByCheckKey((current) => { + if (current[row.key]?.requestId !== requestId) { + return current + } + return { + ...current, + [row.key]: { + requestId, + loading: false, + details: null, + error, + errorAt: { status: row.check.status, conclusion: row.check.conclusion } + } + } + }) }) }, - [checkDetailsContextKey, detailsByCheckKey, onLoadCheckDetails] + [ + checkDetailsContextKey, + detailsByCheckKey, + getGitLabProjectRef, + githubRepository, + onLoadCheckDetails, + patchOpenCheckRunDetails, + resolvedWorktreeId + ] ) useEffect(() => { @@ -1184,9 +1272,11 @@ export function ChecksList({ continue } patchOpenCheckRunDetails(resolvedWorktreeId, checkDetailsContextKey, row.check, { + requestId: detailsState.requestId, details: detailsState.details ?? null, loading: detailsState.loading ?? false, error: detailsState.error ?? null, + githubRepository: githubRepository ?? null, gitlabProjectRef: getGitLabProjectRef?.() ?? null }) } @@ -1194,6 +1284,7 @@ export function ChecksList({ checkDetailsContextKey, detailsByCheckKey, getGitLabProjectRef, + githubRepository, patchOpenCheckRunDetails, resolvedWorktreeId, rows @@ -1370,6 +1461,8 @@ export function ChecksList({ worktreeId={resolvedWorktreeId} detailsStickySurface={detailsStickySurface} getGitLabProjectRef={getGitLabProjectRef} + githubRepository={githubRepository} + onRetry={() => requestCheckDetails(row)} /> )}
diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.ts index 2b9e5dcbe33..9cf77aa2565 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.test.ts @@ -126,7 +126,8 @@ describe('parent PR checks refresh', () => { worktrees: [unlinked, linked], repos: [repo] }) - const fetchHostedReviewForBranch = vi.fn(async () => makeReview()) + const githubRepository = { owner: 'upstream', repo: 'project' } + const fetchHostedReviewForBranch = vi.fn(async () => makeReview({ githubRepository })) const fetchPRChecks = vi.fn(async () => []) await runLimitedParentPrChecksRefreshes({ @@ -151,7 +152,7 @@ describe('parent PR checks refresh', () => { staleWhileRevalidate: true } ]) - expect(fetchPRChecks).toHaveBeenCalledWith('/repo', 7, 'feature', 'abc123', null, { + expect(fetchPRChecks).toHaveBeenCalledWith('/repo', 7, 'feature', 'abc123', githubRepository, { repoId: 'repo-1', force: false }) diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.ts index 3d5ea1bbc8b..2404950b422 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-refresh.ts @@ -151,7 +151,7 @@ async function refreshParentPrChecksCandidate( review.number, candidate.branch, review.headSha, - null, + review.githubRepository ?? null, { repoId: candidate.repo.id, force } ) } diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-row-types.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-row-types.ts index 9e6f3c2f784..18fc4459da6 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-row-types.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-row-types.ts @@ -1,4 +1,11 @@ -import type { CheckStatus, PRCheckDetail, PRInfo, Repo, Worktree } from '../../../../shared/types' +import type { + CheckStatus, + GitHubRepositoryIdentity, + PRCheckDetail, + PRInfo, + Repo, + Worktree +} from '../../../../shared/types' import type { HostedReviewInfo } from '../../../../shared/hosted-review' import type { AppState } from '@/store' import { translate } from '@/i18n/i18n' @@ -59,6 +66,7 @@ export type ParentPrChecksRow = { reviewState: HostedReviewInfo['state'] | null reviewStatus: HostedReviewInfo['status'] | null provider: HostedReviewInfo['provider'] | null + githubRepository?: GitHubRepositoryIdentity | null summary: string detailNames: string[] checks: PRCheckDetail[] diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts index d261df5851e..c9e2ebc18cc 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.test.ts @@ -590,7 +590,8 @@ describe('buildParentPrChecksProjection', () => { it('reads scoped GitHub checks detail names without using details as aggregate truth', () => { const repo = makeRepo({ connectionId: 'ssh-1' }) const worktree = makeWorktree({ id: 'repo-1::/feature' }) - const review = makeReview({ status: 'failure', headSha: 'abc123' }) + const githubRepository = { owner: 'upstream', repo: 'project' } + const review = makeReview({ status: 'failure', headSha: 'abc123', githubRepository }) const hostedKey = getHostedReviewCacheKey( repo.path, 'feature', @@ -601,7 +602,7 @@ describe('buildParentPrChecksProjection', () => { const checksKey = getGitHubRepoCacheKey( repo.path, repo.id, - prChecksCacheSuffix(12, null, 'abc123'), + prChecksCacheSuffix(12, githubRepository, 'abc123'), settings, repo.connectionId ) @@ -628,6 +629,7 @@ describe('buildParentPrChecksProjection', () => { expect(projection.rows[0]?.detailNames).toEqual(['build']) expect(projection.rows[0]?.status).toBe('failing') + expect(projection.rows[0]?.githubRepository).toEqual(githubRepository) }) it('prioritizes action-required check detail names before truncating the preview', () => { diff --git a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts index 6330c3bbee9..72f4ab921bc 100644 --- a/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts +++ b/src/renderer/src/components/right-sidebar/parent-pr-checks-rows.ts @@ -147,6 +147,7 @@ function buildParentPrChecksRow( reviewState: review?.state ?? fallbackDisplay?.state ?? null, reviewStatus: review?.status ?? fallbackDisplay?.status ?? null, provider: review?.provider ?? fallbackDisplay?.provider ?? null, + githubRepository: review?.provider === 'github' ? (review.githubRepository ?? null) : null, summary: getRowSummary(status, review, detailNames), detailNames, checks: checkDetails, @@ -251,7 +252,7 @@ function getGitHubChecksEntry( args: ParentPrChecksRowSourceArgs & { repo: Repo }, review: HostedReviewInfo ): ParentPrChecksCacheEntry | undefined { - const prRepo = null + const prRepo = review.githubRepository ?? null const withHead = getGitHubRepoCacheKey( args.repo.path, args.repo.id, diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 1aaead6e8f0..f36d61e25ff 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -251,7 +251,8 @@ "51f15c37d3": "Cannot open directory: {{value0}}", "f2e00db373": "File not found: {{value0}}", "checkRunDetailsUnavailable": "No details are available for this check.", - "checkRunDetailsLoadFailed": "Failed to load check details." + "checkRunDetailsLoadFailed": "Failed to load check details.", + "checkRunDetailsRepoUnavailable": "Repository details are unavailable for this check." }, "github": { "f129c42773": "GitHub did not return the new comment.", @@ -11129,6 +11130,7 @@ "checksUnresolvedChip": "unresolved", "checksUnresolvedStripHint": "These checks finished without a pass or fail verdict.", "e15a8b77ef": "No inline details are available for this check.", + "retryCheckDetails": "Retry", "679bf2093c": "Copy log excerpt", "d713f500b2": "Log excerpt", "a916648574": "Open details", @@ -15126,6 +15128,9 @@ "loadFailed": "Failed to load the GitLab job log.", "emptyTrace": "No log is available for this GitLab job.", "timedOut": "Timed out loading the GitLab job log." + }, + "githubCheckDetailsTimeout": { + "timedOut": "Timed out loading check details." } }, "ssh": { diff --git a/src/renderer/src/runtime/github-check-details-timeout.test.ts b/src/renderer/src/runtime/github-check-details-timeout.test.ts new file mode 100644 index 00000000000..1031331ca55 --- /dev/null +++ b/src/renderer/src/runtime/github-check-details-timeout.test.ts @@ -0,0 +1,33 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + GITHUB_CHECK_DETAILS_TIMEOUT_MS, + withGitHubCheckDetailsTimeout +} from './github-check-details-timeout' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string) => fallback +})) + +describe('withGitHubCheckDetailsTimeout', () => { + beforeEach(() => { + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it('passes through a result and clears its deadline', async () => { + await expect(withGitHubCheckDetailsTimeout(Promise.resolve('details'))).resolves.toBe('details') + expect(vi.getTimerCount()).toBe(0) + }) + + it('rejects a stalled local request after the remote RPC budget', async () => { + const stalled = withGitHubCheckDetailsTimeout(new Promise(() => {})) + const assertion = expect(stalled).rejects.toThrow('Timed out loading check details.') + + await vi.advanceTimersByTimeAsync(GITHUB_CHECK_DETAILS_TIMEOUT_MS) + + await assertion + }) +}) diff --git a/src/renderer/src/runtime/github-check-details-timeout.ts b/src/renderer/src/runtime/github-check-details-timeout.ts new file mode 100644 index 00000000000..9e077fb995c --- /dev/null +++ b/src/renderer/src/runtime/github-check-details-timeout.ts @@ -0,0 +1,25 @@ +import { translate } from '@/i18n/i18n' + +export const GITHUB_CHECK_DETAILS_TIMEOUT_MS = 30_000 + +/** Bound local IPC the same way runtime RPC bounds remote check-detail requests. */ +export async function withGitHubCheckDetailsTimeout(pending: Promise): Promise { + let timer: ReturnType | undefined + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => { + reject( + new Error( + translate( + 'auto.runtime.githubCheckDetailsTimeout.timedOut', + 'Timed out loading check details.' + ) + ) + ) + }, GITHUB_CHECK_DETAILS_TIMEOUT_MS) + }) + try { + return await Promise.race([pending, timeout]) + } finally { + clearTimeout(timer) + } +} diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index 8b87ee976c7..c02b660e8d1 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -3080,18 +3080,24 @@ describe('createEditorSlice conflict status reconciliation', () => { url: null, checkRunId: 42 } + const githubRepository = { owner: 'upstream', repo: 'project' } store.getState().openCheckRunDetails('wt-1', 'repo:99', check, { details: null, loading: false, - error: null + error: null, + githubRepository }) await store.getState().reloadOpenCheckRunDetailsTab('wt-1::check-details::check-run:42') expect(fetchPRCheckDetails).toHaveBeenCalledWith( '/repo', - expect.objectContaining({ checkRunId: 42, checkName: 'verify' }), + expect.objectContaining({ + checkRunId: 42, + checkName: 'verify', + prRepo: githubRepository + }), { repoId: 'repo-1' } ) expect(store.getState().openFiles).toContainEqual( @@ -3105,6 +3111,41 @@ describe('createEditorSlice conflict status reconciliation', () => { ) }) + it('stops loading when an open check-details tab loses its repository', async () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const store = createStore()((...args: any[]) => ({ + activeWorktreeId: 'wt-1', + repos: [], + worktreesByRepo: {}, + fetchPRCheckDetails: vi.fn(), + ...createEditorSlice(...(args as Parameters)) + })) as unknown as StoreApi + const check = { + name: 'verify', + status: 'completed' as const, + conclusion: 'failure' as const, + url: null, + checkRunId: 42 + } + + store.getState().openCheckRunDetails('wt-1', 'repo:99', check, { + details: null, + loading: true, + error: null + }) + await store.getState().reloadOpenCheckRunDetailsTab('wt-1::check-details::check-run:42') + + expect(store.getState().openFiles).toContainEqual( + expect.objectContaining({ + id: 'wt-1::check-details::check-run:42', + checkRunDetails: expect.objectContaining({ + loading: false, + error: 'Repository details are unavailable for this check.' + }) + }) + ) + }) + // Regression for #7732: refreshing a GitLab job tab through the GitHub check-runs // API returns null and blanks the tab the user just asked to reload. it('reloads an open GitLab job tab through the job trace client', async () => { @@ -3304,6 +3345,112 @@ describe('createEditorSlice conflict status reconciliation', () => { ) }) + it('ignores a stale check-details request after the tab context changes', () => { + const store = createEditorTabsStore() + const check = { + name: 'verify', + status: 'completed' as const, + conclusion: 'failure' as const, + url: null, + checkRunId: 42 + } + + store.getState().openCheckRunDetails('wt-1', 'repo:old', check, { + details: null, + loading: true, + error: null + }) + store.getState().openCheckRunDetails('wt-1', 'repo:new', check, { + details: null, + loading: true, + error: null + }) + store.getState().patchOpenCheckRunDetails('wt-1', 'repo:old', check, { + details: null, + loading: false, + error: 'stale request failed' + }) + + expect( + store.getState().openFiles.find((file) => file.id === 'wt-1::check-details::check-run:42') + ?.checkRunDetails + ).toEqual( + expect.objectContaining({ contextKey: 'repo:new', details: null, loading: true, error: null }) + ) + }) + + it('ignores an older check-details request in the same context', () => { + const store = createEditorTabsStore() + const check = { + name: 'verify', + status: 'completed' as const, + conclusion: 'failure' as const, + url: null, + checkRunId: 42 + } + + store.getState().openCheckRunDetails('wt-1', 'repo:99', check, { + requestId: 1, + details: null, + loading: true, + error: null + }) + store.getState().patchOpenCheckRunDetails('wt-1', 'repo:99', check, { + requestId: 2, + details: null, + loading: true, + error: null + }) + store.getState().patchOpenCheckRunDetails('wt-1', 'repo:99', check, { + requestId: 1, + details: null, + loading: false, + error: 'stale request failed' + }) + + expect( + store.getState().openFiles.find((file) => file.id === 'wt-1::check-details::check-run:42') + ?.checkRunDetails + ).toEqual(expect.objectContaining({ requestId: 2, details: null, loading: true, error: null })) + }) + + it('does not reopen a check-details tab with an older sidebar snapshot', () => { + const store = createEditorTabsStore() + const check = { + name: 'verify', + status: 'completed' as const, + conclusion: 'failure' as const, + url: null, + checkRunId: 42 + } + + store.getState().openCheckRunDetails('wt-1', 'repo:99', check, { + requestId: 2, + details: null, + loading: false, + error: 'newer result' + }) + store.getState().openFile({ + filePath: '/repo/other.ts', + relativePath: 'other.ts', + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().openCheckRunDetails('wt-1', 'repo:99', check, { + requestId: 1, + details: null, + loading: false, + error: 'stale result' + }) + + expect(store.getState().activeFileId).toBe('wt-1::check-details::check-run:42') + expect( + store.getState().openFiles.find((file) => file.id === 'wt-1::check-details::check-run:42') + ?.checkRunDetails + ).toEqual(expect.objectContaining({ requestId: 2, error: 'newer result' })) + }) + it('opens check full details as a center-pane editor tab', () => { const store = createEditorTabsStore() const check = { diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 971ab20cba6..4eda195dfa2 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -14,7 +14,9 @@ import { isPathInsideOrEqual } from '../../../../shared/cross-platform-path' import { resolveMarkdownLinkTarget } from '@/components/editor/markdown-internal-links' import { buildCheckRunDetailsTabId, + createCheckRunDetailsRequestId, getCheckRunDetailsTabLabel, + isSameGitHubRepository, isSameGitLabProjectRef, type CheckRunDetailsTabPatch, type OpenCheckRunDetailsState @@ -3802,14 +3804,21 @@ export const createEditorSlice: StateCreator = (s const checkRunDetails: OpenCheckRunDetailsState = { contextKey, check, + requestId: state.requestId, details: state.details, loading: state.loading, error: state.error, + githubRepository: state.githubRepository ?? null, gitlabProjectRef: state.gitlabProjectRef ?? null } set((s) => { const existing = s.openFiles.find((f) => f.id === id) if (existing) { + const existingDetails = existing.checkRunDetails + const incomingIsStale = + existingDetails?.contextKey === contextKey && + existingDetails.requestId !== undefined && + (state.requestId === undefined || state.requestId < existingDetails.requestId) return { openFiles: s.openFiles.map((f) => f.id === id @@ -3818,7 +3827,7 @@ export const createEditorSlice: StateCreator = (s mode: 'check-details' as const, relativePath: label, language: 'plaintext', - checkRunDetails + checkRunDetails: incomingIsStale ? existingDetails : checkRunDetails } : f ), @@ -3860,24 +3869,39 @@ export const createEditorSlice: StateCreator = (s return s } const current = existing.checkRunDetails + if (current.contextKey !== contextKey) { + return s + } + if ( + state.requestId !== undefined && + current.requestId !== undefined && + state.requestId < current.requestId + ) { + return s + } // Why: the sidebar resolves the MR's project asynchronously, so an early patch // must not blank a ref we already know. + const githubRepository = state.githubRepository ?? current.githubRepository ?? null const gitlabProjectRef = state.gitlabProjectRef ?? current.gitlabProjectRef ?? null const nextCheckRunDetails: OpenCheckRunDetailsState = { contextKey, check, + requestId: state.requestId ?? current.requestId, details: state.details, loading: state.loading, error: state.error, + githubRepository, gitlabProjectRef } if ( current.contextKey === nextCheckRunDetails.contextKey && + current.requestId === nextCheckRunDetails.requestId && current.check.status === nextCheckRunDetails.check.status && current.check.conclusion === nextCheckRunDetails.check.conclusion && current.loading === nextCheckRunDetails.loading && current.error === nextCheckRunDetails.error && current.details === nextCheckRunDetails.details && + isSameGitHubRepository(current.githubRepository ?? null, githubRepository) && isSameGitLabProjectRef(current.gitlabProjectRef ?? null, gitlabProjectRef) ) { return s @@ -3897,16 +3921,25 @@ export const createEditorSlice: StateCreator = (s if (!file || file.mode !== 'check-details' || !checkRunDetails) { return } + const { contextKey, check } = checkRunDetails + const requestId = createCheckRunDetailsRequestId() + const patch = (next: CheckRunDetailsTabPatch): void => { + get().patchOpenCheckRunDetails(file.worktreeId, contextKey, check, { ...next, requestId }) + } const worktree = findWorktreeById(state.worktreesByRepo, file.worktreeId) const repoId = worktree?.repoId ?? getRepoIdFromWorktreeId(file.worktreeId) const repo = state.repos.find((candidate) => candidate.id === repoId) if (!repo?.path) { + patch({ + details: checkRunDetails.details, + loading: false, + error: translate( + 'auto.store.slices.editor.checkRunDetailsRepoUnavailable', + 'Repository details are unavailable for this check.' + ) + }) return } - const { contextKey, check } = checkRunDetails - const patch = (next: CheckRunDetailsTabPatch): void => { - get().patchOpenCheckRunDetails(file.worktreeId, contextKey, check, next) - } patch({ details: checkRunDetails.details, loading: true, error: null }) try { // Why: refreshing a GitLab job tab through the GitHub check-runs API returns @@ -3927,7 +3960,7 @@ export const createEditorSlice: StateCreator = (s workflowRunId: check.workflowRunId, checkName: check.name, url: check.url, - prRepo: null + prRepo: checkRunDetails.githubRepository ?? null }, { repoId: repo.id } ) diff --git a/src/renderer/src/store/slices/github.ts b/src/renderer/src/store/slices/github.ts index 21b66f2e9bd..012df2ea2b7 100644 --- a/src/renderer/src/store/slices/github.ts +++ b/src/renderer/src/store/slices/github.ts @@ -76,6 +76,7 @@ import { } from '../../../../shared/task-source-context' import { normalizeGitHubPRForBranchOutcome } from '../../../../shared/github-pr-for-branch-outcome' import { restoreReactionOnSubject, setReactionOnSubject } from '@/lib/pr-comment-reactions' +import { withGitHubCheckDetailsTimeout } from '@/runtime/github-check-details-timeout' // ─── ProjectV2 cache types ──────────────────────────────────────────── // Why: separate from CacheEntry — project-view has a single GraphQL source (no issue/PR fallback) and a distinct error union. @@ -3588,16 +3589,18 @@ export const createGitHubSlice: StateCreator = (s }, { timeoutMs: 30_000 } ) - : ((await window.api.gh.prCheckDetails({ - repoPath, - repoId, - checkRunId: args.checkRunId, - workflowRunId: args.workflowRunId, - checkName: args.checkName, - url: args.url, - prRepo: args.prRepo ?? null, - sourceContext: options?.sourceContext - })) as PRCheckRunDetails | null) + : await withGitHubCheckDetailsTimeout( + window.api.gh.prCheckDetails({ + repoPath, + repoId, + checkRunId: args.checkRunId, + workflowRunId: args.workflowRunId, + checkName: args.checkName, + url: args.url, + prRepo: args.prRepo ?? null, + sourceContext: options?.sourceContext + }) + ) }, fetchPRComments: async (repoPath, prNumber, options): Promise => { diff --git a/src/shared/hosted-review-github.test.ts b/src/shared/hosted-review-github.test.ts index 640a87016b5..6aadaced3f2 100644 --- a/src/shared/hosted-review-github.test.ts +++ b/src/shared/hosted-review-github.test.ts @@ -122,7 +122,8 @@ describe('hostedReviewSummaryFromGitHubPRInfo', () => { }) it('maps PRInfo into sidebar hosted review metadata', () => { - const review = hostedReviewInfoFromGitHubPRInfo(pr) + const githubRepository = { owner: 'upstream', repo: 'orca' } + const review = hostedReviewInfoFromGitHubPRInfo({ ...pr, prRepo: githubRepository }) expect(review).toMatchObject({ provider: 'github', @@ -131,7 +132,8 @@ describe('hostedReviewSummaryFromGitHubPRInfo', () => { state: 'open', status: 'pending', mergeable: 'MERGEABLE', - headSha: 'abc123' + headSha: 'abc123', + githubRepository }) }) }) diff --git a/src/shared/hosted-review-github.ts b/src/shared/hosted-review-github.ts index 6fc90a39aaf..f5aa5905b45 100644 --- a/src/shared/hosted-review-github.ts +++ b/src/shared/hosted-review-github.ts @@ -98,6 +98,7 @@ export function hostedReviewInfoFromGitHubPRInfo(pr: PRInfo): HostedReviewInfo { ...(pr.mergeQueueRequired !== undefined ? { mergeQueueRequired: pr.mergeQueueRequired } : {}), ...(pr.mergeStateStatus !== undefined ? { mergeStateStatus: pr.mergeStateStatus } : {}), ...(pr.headSha ? { headSha: pr.headSha } : {}), + ...(pr.prRepo ? { githubRepository: pr.prRepo } : {}), ...(pr.confirmedContainedHeadOid ? { confirmedContainedHeadOid: pr.confirmedContainedHeadOid } : {}), diff --git a/src/shared/hosted-review.ts b/src/shared/hosted-review.ts index 90a97d0afb9..6a8685cbc81 100644 --- a/src/shared/hosted-review.ts +++ b/src/shared/hosted-review.ts @@ -1,4 +1,10 @@ -import type { CheckStatus, PRConflictSummary, PRMergeableState, PRReviewDecision } from './types' +import type { + CheckStatus, + GitHubRepositoryIdentity, + PRConflictSummary, + PRMergeableState, + PRReviewDecision +} from './types' export type HostedReviewProvider = | 'github' @@ -30,6 +36,8 @@ export type HostedReviewInfo = { mergeQueueRequired?: boolean | null mergeStateStatus?: string | null headSha?: string + /** GitHub repository that owns the PR; absent on older runtimes and other providers. */ + githubRepository?: GitHubRepositoryIdentity // Why: mirrors PRInfo.confirmedContainedHeadOid so merged-review staleness // checks accept a worktree head confirmed to be part of the merged PR. confirmedContainedHeadOid?: string