Fix PR check refresh and terminal tombstone test (#2375)
* Prefetch task PR checks by head SHA * Fix terminal host tombstone cap test
This commit is contained in:
Jinjing
GitHub
parent
79eaea4399
commit
9d5f27fa3c
|
|
@ -299,7 +299,10 @@ describe('TerminalHost', () => {
|
|||
|
||||
describe('tombstones', () => {
|
||||
it('caps tombstones at limit', async () => {
|
||||
for (let i = 0; i < 1005; i++) {
|
||||
host.dispose()
|
||||
host = new TerminalHost({ spawnSubprocess: spawnFn as MockSpawnFn, maxTombstones: 3 })
|
||||
|
||||
for (let i = 0; i < 5; i++) {
|
||||
await host.createOrAttach({
|
||||
sessionId: `session-${i}`,
|
||||
cols: 80,
|
||||
|
|
@ -311,7 +314,7 @@ describe('TerminalHost', () => {
|
|||
|
||||
// Oldest tombstones should be evicted
|
||||
expect(host.isKilled('session-0')).toBe(false)
|
||||
expect(host.isKilled('session-1004')).toBe(true)
|
||||
expect(host.isKilled('session-4')).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import { resolveProcessCwd } from '../providers/process-cwd'
|
|||
import type { SessionInfo, TerminalSnapshot, ShellReadyState } from './types'
|
||||
import { SessionNotFoundError } from './types'
|
||||
|
||||
const MAX_TOMBSTONES = 1000
|
||||
const DEFAULT_MAX_TOMBSTONES = 1000
|
||||
|
||||
export type CreateOrAttachOptions = {
|
||||
sessionId: string
|
||||
|
|
@ -46,6 +46,9 @@ export type TerminalHostOptions = {
|
|||
// sessions before killing them. This bypasses the RPC round-trip — the daemon
|
||||
// writes checkpoints in-process, guaranteeing completion before teardown.
|
||||
onFinalCheckpoint?: (sessionId: string, snapshot: TerminalSnapshot) => void
|
||||
// Why: production keeps a large cap, but tests need a small deterministic cap
|
||||
// without spawning thousands of full terminal sessions.
|
||||
maxTombstones?: number
|
||||
}
|
||||
|
||||
export class TerminalHost {
|
||||
|
|
@ -53,10 +56,12 @@ export class TerminalHost {
|
|||
private killedTombstones = new Map<string, number>()
|
||||
private spawnSubprocess: TerminalHostOptions['spawnSubprocess']
|
||||
private onFinalCheckpoint: TerminalHostOptions['onFinalCheckpoint']
|
||||
private maxTombstones: number
|
||||
|
||||
constructor(opts: TerminalHostOptions) {
|
||||
this.spawnSubprocess = opts.spawnSubprocess
|
||||
this.onFinalCheckpoint = opts.onFinalCheckpoint
|
||||
this.maxTombstones = opts.maxTombstones ?? DEFAULT_MAX_TOMBSTONES
|
||||
}
|
||||
|
||||
async createOrAttach(opts: CreateOrAttachOptions): Promise<CreateOrAttachResult> {
|
||||
|
|
@ -266,7 +271,7 @@ export class TerminalHost {
|
|||
this.killedTombstones.delete(sessionId)
|
||||
this.killedTombstones.set(sessionId, Date.now())
|
||||
|
||||
if (this.killedTombstones.size > MAX_TOMBSTONES) {
|
||||
if (this.killedTombstones.size > this.maxTombstones) {
|
||||
const oldest = this.killedTombstones.keys().next().value
|
||||
if (oldest) {
|
||||
this.killedTombstones.delete(oldest)
|
||||
|
|
|
|||
|
|
@ -114,6 +114,7 @@ describe('listWorkItems', () => {
|
|||
author: { login: 'octocat' },
|
||||
isDraft: false,
|
||||
headRefName: 'feature/add-feature',
|
||||
headRefOid: 'head-42',
|
||||
baseRefName: 'main',
|
||||
reviewRequests: [
|
||||
{
|
||||
|
|
@ -156,7 +157,7 @@ describe('listWorkItems', () => {
|
|||
'--limit',
|
||||
'10',
|
||||
'--json',
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests',
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--assignee',
|
||||
|
|
@ -192,6 +193,8 @@ describe('listWorkItems', () => {
|
|||
author: 'octocat',
|
||||
branchName: 'feature/add-feature',
|
||||
baseRefName: 'main',
|
||||
headSha: 'head-42',
|
||||
prRepo: { owner: 'acme', repo: 'widgets' },
|
||||
reviewRequests: [
|
||||
{
|
||||
login: 'AmethystLiang',
|
||||
|
|
@ -218,6 +221,7 @@ describe('listWorkItems', () => {
|
|||
author: { login: 'octocat' },
|
||||
isDraft: true,
|
||||
headRefName: 'draft/work',
|
||||
headRefOid: 'head-7',
|
||||
baseRefName: 'main'
|
||||
}
|
||||
])
|
||||
|
|
@ -231,7 +235,7 @@ describe('listWorkItems', () => {
|
|||
'--limit',
|
||||
'10',
|
||||
'--json',
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests',
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--state',
|
||||
|
|
@ -252,7 +256,9 @@ describe('listWorkItems', () => {
|
|||
updatedAt: '2026-03-30T00:00:00Z',
|
||||
author: 'octocat',
|
||||
branchName: 'draft/work',
|
||||
baseRefName: 'main'
|
||||
baseRefName: 'main',
|
||||
headSha: 'head-7',
|
||||
prRepo: { owner: 'acme', repo: 'widgets' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
|
@ -304,6 +310,7 @@ describe('listWorkItems', () => {
|
|||
author: { login: 'octocat' },
|
||||
isDraft: false,
|
||||
headRefName: 'feature/open-pr',
|
||||
headRefOid: 'head-2',
|
||||
baseRefName: 'main'
|
||||
}
|
||||
])
|
||||
|
|
@ -331,7 +338,7 @@ describe('listWorkItems', () => {
|
|||
'--limit',
|
||||
'10',
|
||||
'--json',
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests',
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests',
|
||||
'--repo',
|
||||
'acme/widgets',
|
||||
'--state',
|
||||
|
|
@ -362,7 +369,9 @@ describe('listWorkItems', () => {
|
|||
updatedAt: '2026-03-30T00:00:00Z',
|
||||
author: 'octocat',
|
||||
branchName: 'feature/open-pr',
|
||||
baseRefName: 'main'
|
||||
baseRefName: 'main',
|
||||
headSha: 'head-2',
|
||||
prRepo: { owner: 'acme', repo: 'widgets' }
|
||||
}
|
||||
])
|
||||
})
|
||||
|
|
@ -381,6 +390,7 @@ describe('listWorkItems', () => {
|
|||
user: { login: 'contributor' },
|
||||
head: {
|
||||
ref: 'feat/onboarding-model-choice-782',
|
||||
sha: 'head-1849',
|
||||
repo: null,
|
||||
label: 'contributor:feat/onboarding-model-choice-782'
|
||||
},
|
||||
|
|
@ -403,6 +413,8 @@ describe('listWorkItems', () => {
|
|||
author: 'contributor',
|
||||
branchName: 'feat/onboarding-model-choice-782',
|
||||
baseRefName: 'main',
|
||||
headSha: 'head-1849',
|
||||
prRepo: { owner: 'stablyai', repo: 'orca' },
|
||||
isCrossRepository: true
|
||||
}
|
||||
])
|
||||
|
|
|
|||
|
|
@ -329,14 +329,14 @@ export async function getAuthenticatedViewer(): Promise<GitHubViewer | null> {
|
|||
type MainWorkItem = Omit<GitHubWorkItem, 'repoId'>
|
||||
|
||||
const WORK_ITEM_PR_LIST_JSON_FIELDS =
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,reviewRequests'
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,reviewRequests'
|
||||
|
||||
// Why: these fields are intentionally excluded from `gh pr list` because
|
||||
// statusCheckRollup/review decision/merge metadata fan out into expensive
|
||||
// GraphQL work across every row. Requested reviewers are kept in the list
|
||||
// payload because the Tasks table renders that column on first paint.
|
||||
const WORK_ITEM_PR_DETAIL_JSON_FIELDS =
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRepositoryOwner,additions,deletions,changedFiles,reviewDecision,reviewRequests,latestReviews,assignees,statusCheckRollup,mergeable,mergeStateStatus,maintainerCanModify'
|
||||
'number,title,state,url,labels,updatedAt,author,isDraft,headRefName,baseRefName,headRefOid,headRepositoryOwner,additions,deletions,changedFiles,reviewDecision,reviewRequests,latestReviews,assignees,statusCheckRollup,mergeable,mergeStateStatus,maintainerCanModify'
|
||||
|
||||
function mapIssueWorkItem(item: Record<string, unknown>): MainWorkItem {
|
||||
return {
|
||||
|
|
@ -548,10 +548,10 @@ function deriveWorkItemCheckSummary(value: unknown): GitHubWorkItem['checksSumma
|
|||
|
||||
function mapPullRequestWorkItem(
|
||||
item: Record<string, unknown>,
|
||||
baseOwnerLogin: string | null = null
|
||||
baseOwnerRepo: OwnerRepo | null = null
|
||||
): MainWorkItem {
|
||||
// Why: fork PRs are disabled in the Start-from picker. We compare the PR head's
|
||||
// owner to the selected repo's owner; when baseOwnerLogin is unknown we default
|
||||
// owner to the selected repo's owner; when the base repo is unknown we default
|
||||
// to false so non-picker call sites see the same shape as before.
|
||||
const headOwnerLogin = extractHeadOwnerLogin(item)
|
||||
// Why: only emit isCrossRepository when we actually know the head owner. If
|
||||
|
|
@ -559,7 +559,9 @@ function mapPullRequestWorkItem(
|
|||
// that fixture, or gh not returning it), leave the field undefined instead
|
||||
// of falsely claiming "not a fork".
|
||||
const isCrossRepository =
|
||||
headOwnerLogin !== null && baseOwnerLogin !== null ? headOwnerLogin !== baseOwnerLogin : null
|
||||
headOwnerLogin !== null && baseOwnerRepo !== null
|
||||
? headOwnerLogin !== baseOwnerRepo.owner
|
||||
: null
|
||||
const state = String(item.state ?? '').toLowerCase()
|
||||
const additions = numberFromUnknown(item.additions)
|
||||
const deletions = numberFromUnknown(item.deletions)
|
||||
|
|
@ -569,6 +571,14 @@ function mapPullRequestWorkItem(
|
|||
(item.files as { totalCount?: unknown } | undefined)?.totalCount
|
||||
)
|
||||
const mergeable = normalizePRMergeable(item.mergeable)
|
||||
const headSha =
|
||||
typeof item.headRefOid === 'string'
|
||||
? item.headRefOid
|
||||
: typeof item.head === 'object' && item.head !== null
|
||||
? typeof (item.head as { sha?: unknown }).sha === 'string'
|
||||
? (item.head as { sha: string }).sha
|
||||
: undefined
|
||||
: undefined
|
||||
return {
|
||||
id: `pr:${String(item.number)}`,
|
||||
type: 'pr',
|
||||
|
|
@ -607,6 +617,8 @@ function mapPullRequestWorkItem(
|
|||
typeof item.base === 'object' && item.base !== null && 'ref' in item.base
|
||||
? String((item.base as { ref?: unknown }).ref ?? '')
|
||||
: String(item.baseRefName ?? ''),
|
||||
...(headSha ? { headSha } : {}),
|
||||
...(baseOwnerRepo ? { prRepo: { owner: baseOwnerRepo.owner, repo: baseOwnerRepo.repo } } : {}),
|
||||
...(additions !== undefined ? { additions } : {}),
|
||||
...(deletions !== undefined ? { deletions } : {}),
|
||||
...(changedFiles !== undefined ? { changedFiles } : {}),
|
||||
|
|
@ -674,7 +686,7 @@ async function fetchPullRequestWorkItem(
|
|||
['api', `repos/${ownerRepo.owner}/${ownerRepo.repo}/pulls/${number}`],
|
||||
ghOptions
|
||||
)
|
||||
return mapPullRequestWorkItem(JSON.parse(stdout) as Record<string, unknown>, ownerRepo.owner)
|
||||
return mapPullRequestWorkItem(JSON.parse(stdout) as Record<string, unknown>, ownerRepo)
|
||||
}
|
||||
|
||||
const { stdout } = await ghExecFileAsync(
|
||||
|
|
@ -843,7 +855,7 @@ async function listRecentWorkItems(
|
|||
let prs: MainWorkItem[] = []
|
||||
if (prsSettled.status === 'fulfilled') {
|
||||
prs = (JSON.parse(prsSettled.value.stdout) as Record<string, unknown>[]).map((item) =>
|
||||
mapPullRequestWorkItem(item, prOwnerRepo?.owner ?? null)
|
||||
mapPullRequestWorkItem(item, prOwnerRepo)
|
||||
)
|
||||
} else {
|
||||
// Why: PR-side failures must preserve the pre-diff behavior of
|
||||
|
|
@ -971,7 +983,7 @@ async function listQueriedWorkItems(
|
|||
try {
|
||||
const { stdout } = await ghExecFileAsync(args, ghOptions)
|
||||
return (JSON.parse(stdout) as Record<string, unknown>[]).map((item) =>
|
||||
mapPullRequestWorkItem(item, prOwnerRepo?.owner ?? null)
|
||||
mapPullRequestWorkItem(item, prOwnerRepo)
|
||||
)
|
||||
} catch (err) {
|
||||
console.warn('listQueriedWorkItems PRs partial failure:', err)
|
||||
|
|
|
|||
|
|
@ -111,6 +111,7 @@ import {
|
|||
selectTaskPageWorkItemsCacheEntries,
|
||||
type TaskPageRepoSourceState
|
||||
} from '@/components/task-page-cache-selectors'
|
||||
import { deriveTaskPagePRCheckSummary } from '@/components/task-page-pr-check-summary'
|
||||
import type {
|
||||
GitHubOwnerRepo,
|
||||
GitHubAssignableUser,
|
||||
|
|
@ -223,6 +224,7 @@ const LINEAR_PRESETS: LinearPreset[] = [
|
|||
|
||||
const TASK_SEARCH_DEBOUNCE_MS = 300
|
||||
const LINEAR_ITEM_LIMIT = 36
|
||||
const PR_CHECKS_EAGER_PREFETCH_LIMIT = 20
|
||||
|
||||
const GITHUB_TASK_GRID_CLASS =
|
||||
'min-w-[860px] grid-cols-[72px_minmax(260px,2fr)_minmax(130px,0.8fr)_100px_92px_158px]'
|
||||
|
|
@ -881,6 +883,17 @@ function getChecksTone(item: GitHubWorkItem): string {
|
|||
return 'border-border/60 bg-background/70 text-muted-foreground'
|
||||
}
|
||||
|
||||
function sameOptionalGitHubOwnerRepo(
|
||||
left: GitHubOwnerRepo | null | undefined,
|
||||
right: GitHubOwnerRepo | null | undefined
|
||||
): boolean {
|
||||
const leftValue = left ?? null
|
||||
const rightValue = right ?? null
|
||||
return leftValue === null && rightValue === null
|
||||
? true
|
||||
: sameGitHubOwnerRepo(leftValue, rightValue)
|
||||
}
|
||||
|
||||
function getMergeLabel(item: GitHubWorkItem): string {
|
||||
if (item.mergeable === undefined && item.mergeStateStatus === undefined) {
|
||||
return 'Merge'
|
||||
|
|
@ -1360,11 +1373,39 @@ function PRReviewCell({
|
|||
|
||||
function PRChecksCell({
|
||||
item,
|
||||
onOpen
|
||||
onOpen,
|
||||
onLoadChecks
|
||||
}: {
|
||||
item: GitHubWorkItem
|
||||
onOpen: () => void
|
||||
onLoadChecks: () => void
|
||||
}): React.JSX.Element {
|
||||
const triggerRef = useRef<HTMLButtonElement | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
if (item.type !== 'pr' || item.checksSummary) {
|
||||
return
|
||||
}
|
||||
const node = triggerRef.current
|
||||
if (!node || typeof IntersectionObserver === 'undefined') {
|
||||
return
|
||||
}
|
||||
let requested = false
|
||||
const observer = new IntersectionObserver(
|
||||
(entries) => {
|
||||
if (requested || !entries.some((entry) => entry.isIntersecting)) {
|
||||
return
|
||||
}
|
||||
requested = true
|
||||
onLoadChecks()
|
||||
observer.disconnect()
|
||||
},
|
||||
{ rootMargin: '160px 0px' }
|
||||
)
|
||||
observer.observe(node)
|
||||
return () => observer.disconnect()
|
||||
}, [item.checksSummary, item.type, onLoadChecks])
|
||||
|
||||
if (item.type !== 'pr') {
|
||||
return <span className="text-[11px] text-muted-foreground">Issue</span>
|
||||
}
|
||||
|
|
@ -1381,9 +1422,13 @@ function PRChecksCell({
|
|||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<button
|
||||
ref={triggerRef}
|
||||
type="button"
|
||||
onFocus={onLoadChecks}
|
||||
onMouseEnter={onLoadChecks}
|
||||
onClick={(event) => {
|
||||
event.stopPropagation()
|
||||
onLoadChecks()
|
||||
onOpen()
|
||||
}}
|
||||
className={cn(
|
||||
|
|
@ -1647,6 +1692,7 @@ export default function TaskPage(): React.JSX.Element {
|
|||
const openModal = useAppStore((s) => s.openModal)
|
||||
const updateSettings = useAppStore((s) => s.updateSettings)
|
||||
const fetchWorkItemsAcrossRepos = useAppStore((s) => s.fetchWorkItemsAcrossRepos)
|
||||
const fetchPRChecks = useAppStore((s) => s.fetchPRChecks)
|
||||
const getCachedWorkItems = useAppStore((s) => s.getCachedWorkItems)
|
||||
const setIssueSourcePreference = useAppStore((s) => s.setIssueSourcePreference)
|
||||
// Why: bumped by `setIssueSourcePreference` after cache eviction so the
|
||||
|
|
@ -1941,7 +1987,11 @@ export default function TaskPage(): React.JSX.Element {
|
|||
)
|
||||
|
||||
const patchTaskPageWorkItemRows = useCallback(
|
||||
(itemKey: { id: string; repoId: string }, patch: Partial<GitHubWorkItem>): void => {
|
||||
(
|
||||
itemKey: { id: string; repoId: string },
|
||||
patch: Partial<GitHubWorkItem>,
|
||||
shouldPatch?: (item: GitHubWorkItem) => boolean
|
||||
): void => {
|
||||
setPages((current) => {
|
||||
let changed = false
|
||||
const nextPages = current.map((page) => {
|
||||
|
|
@ -1950,6 +2000,9 @@ export default function TaskPage(): React.JSX.Element {
|
|||
if (item.id !== itemKey.id || item.repoId !== itemKey.repoId) {
|
||||
return item
|
||||
}
|
||||
if (shouldPatch && !shouldPatch(item)) {
|
||||
return item
|
||||
}
|
||||
pageChanged = true
|
||||
changed = true
|
||||
return { ...item, ...patch }
|
||||
|
|
@ -2661,6 +2714,48 @@ export default function TaskPage(): React.JSX.Element {
|
|||
? GITHUB_PR_TASK_GRID_CLASS
|
||||
: GITHUB_TASK_GRID_CLASS
|
||||
|
||||
const ensurePRChecksLoaded = useCallback(
|
||||
(item: GitHubWorkItem): void => {
|
||||
if (item.type !== 'pr' || item.checksSummary) {
|
||||
return
|
||||
}
|
||||
const repo = repoMap.get(item.repoId)
|
||||
if (!repo) {
|
||||
return
|
||||
}
|
||||
const requestedHeadSha = item.headSha
|
||||
const requestedPRRepo = item.prRepo ?? null
|
||||
void fetchPRChecks(
|
||||
repo.path,
|
||||
item.number,
|
||||
item.branchName,
|
||||
item.headSha,
|
||||
item.prRepo ?? null,
|
||||
{ repoId: repo.id }
|
||||
).then((checks) => {
|
||||
patchTaskPageWorkItemRows(
|
||||
{ id: item.id, repoId: item.repoId },
|
||||
{ checksSummary: deriveTaskPagePRCheckSummary(checks) },
|
||||
(currentItem) =>
|
||||
currentItem.type === 'pr' &&
|
||||
currentItem.headSha === requestedHeadSha &&
|
||||
sameOptionalGitHubOwnerRepo(currentItem.prRepo, requestedPRRepo)
|
||||
)
|
||||
})
|
||||
},
|
||||
[fetchPRChecks, patchTaskPageWorkItemRows, repoMap]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
if (taskSource !== 'github' || githubMode !== 'items' || !showPRManagementColumns) {
|
||||
return
|
||||
}
|
||||
|
||||
for (const item of filteredWorkItems.slice(0, PR_CHECKS_EAGER_PREFETCH_LIMIT)) {
|
||||
ensurePRChecksLoaded(item)
|
||||
}
|
||||
}, [ensurePRChecksLoaded, filteredWorkItems, githubMode, showPRManagementColumns, taskSource])
|
||||
|
||||
// Why: totalPages is derived from the search API count when available,
|
||||
// so the pagination bar shows the full range (with ellipsis) upfront.
|
||||
// Falls back to the loaded page count when the count hasn't returned yet.
|
||||
|
|
@ -4327,6 +4422,7 @@ export default function TaskPage(): React.JSX.Element {
|
|||
<PRChecksCell
|
||||
item={item}
|
||||
onOpen={() => setDialogWorkItem(item, 'checks')}
|
||||
onLoadChecks={() => ensurePRChecksLoaded(item)}
|
||||
/>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,58 @@
|
|||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { deriveTaskPagePRCheckSummary } from './task-page-pr-check-summary'
|
||||
import type { PRCheckDetail } from '../../../shared/types'
|
||||
|
||||
function check(patch: Partial<PRCheckDetail>): PRCheckDetail {
|
||||
return {
|
||||
name: 'ci',
|
||||
status: 'completed',
|
||||
conclusion: 'success',
|
||||
url: null,
|
||||
...patch
|
||||
}
|
||||
}
|
||||
|
||||
describe('deriveTaskPagePRCheckSummary', () => {
|
||||
it('returns a none summary for PRs with no checks', () => {
|
||||
expect(deriveTaskPagePRCheckSummary([])).toEqual({
|
||||
state: 'none',
|
||||
total: 0,
|
||||
passed: 0,
|
||||
failed: 0,
|
||||
pending: 0
|
||||
})
|
||||
})
|
||||
|
||||
it('counts failing checks before pending and passing checks', () => {
|
||||
expect(
|
||||
deriveTaskPagePRCheckSummary([
|
||||
check({ conclusion: 'success' }),
|
||||
check({ conclusion: 'failure' }),
|
||||
check({ status: 'in_progress', conclusion: null })
|
||||
])
|
||||
).toEqual({
|
||||
state: 'failure',
|
||||
total: 3,
|
||||
passed: 1,
|
||||
failed: 1,
|
||||
pending: 1
|
||||
})
|
||||
})
|
||||
|
||||
it('treats neutral and skipped checks as passed for the compact PR table label', () => {
|
||||
expect(
|
||||
deriveTaskPagePRCheckSummary([
|
||||
check({ conclusion: 'success' }),
|
||||
check({ conclusion: 'neutral' }),
|
||||
check({ conclusion: 'skipped' })
|
||||
])
|
||||
).toEqual({
|
||||
state: 'success',
|
||||
total: 3,
|
||||
passed: 3,
|
||||
failed: 0,
|
||||
pending: 0
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,48 @@
|
|||
import type { GitHubPRCheckSummary, PRCheckDetail } from '../../../shared/types'
|
||||
|
||||
function getCheckConclusion(check: PRCheckDetail): NonNullable<PRCheckDetail['conclusion']> {
|
||||
return check.conclusion ?? 'pending'
|
||||
}
|
||||
|
||||
function isPendingCheck(check: PRCheckDetail): boolean {
|
||||
return (
|
||||
check.status === 'queued' ||
|
||||
check.status === 'in_progress' ||
|
||||
getCheckConclusion(check) === 'pending'
|
||||
)
|
||||
}
|
||||
|
||||
export function deriveTaskPagePRCheckSummary(checks: PRCheckDetail[]): GitHubPRCheckSummary {
|
||||
if (checks.length === 0) {
|
||||
return { state: 'none', total: 0, passed: 0, failed: 0, pending: 0 }
|
||||
}
|
||||
|
||||
let passed = 0
|
||||
let failed = 0
|
||||
let pending = 0
|
||||
|
||||
for (const check of checks) {
|
||||
const conclusion = getCheckConclusion(check)
|
||||
if (conclusion === 'success' || conclusion === 'neutral' || conclusion === 'skipped') {
|
||||
passed += 1
|
||||
} else if (
|
||||
conclusion === 'failure' ||
|
||||
conclusion === 'timed_out' ||
|
||||
conclusion === 'cancelled'
|
||||
) {
|
||||
failed += 1
|
||||
} else if (isPendingCheck(check)) {
|
||||
pending += 1
|
||||
} else {
|
||||
passed += 1
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
state: failed > 0 ? 'failure' : pending > 0 ? 'pending' : 'success',
|
||||
total: checks.length,
|
||||
passed,
|
||||
failed,
|
||||
pending
|
||||
}
|
||||
}
|
||||
|
|
@ -3,7 +3,7 @@ envelope, and IssueSourceIndicator suppression tests in one file keeps the
|
|||
GitHub slice's cross-cutting invariants verifiable in one place. */
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { create } from 'zustand'
|
||||
import { createGitHubSlice, workItemsCacheKey } from './github'
|
||||
import { createGitHubSlice, prChecksCacheSuffix, workItemsCacheKey } from './github'
|
||||
import type { AppState } from '../types'
|
||||
import type { GitHubWorkItem, PRInfo } from '../../../../shared/types'
|
||||
import {
|
||||
|
|
@ -441,10 +441,14 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
)
|
||||
|
||||
expect(
|
||||
store.getState().checksCache[`${repoId}::pr-checks::acme/widgets::12`]?.data?.[0].name
|
||||
store.getState().checksCache[
|
||||
`${repoId}::${prChecksCacheSuffix(12, { owner: 'Acme', repo: 'Widgets' }, 'head-a')}`
|
||||
]?.data?.[0].name
|
||||
).toBe('upstream')
|
||||
expect(
|
||||
store.getState().checksCache[`${repoId}::pr-checks::fork/widgets::12`]?.data?.[0].name
|
||||
store.getState().checksCache[
|
||||
`${repoId}::${prChecksCacheSuffix(12, { owner: 'Fork', repo: 'Widgets' }, 'head-b')}`
|
||||
]?.data?.[0].name
|
||||
).toBe('fork')
|
||||
expect(mockApi.gh.prChecks).toHaveBeenNthCalledWith(1, {
|
||||
repoPath,
|
||||
|
|
@ -492,7 +496,9 @@ describe('createGitHubSlice.fetchPRChecks', () => {
|
|||
|
||||
expect(store.getState().prCache[prCacheKey]?.data?.checksStatus).toBe('pending')
|
||||
expect(
|
||||
store.getState().checksCache[`${repoId}::pr-checks::acme/widgets::12`]?.data?.[0].name
|
||||
store.getState().checksCache[
|
||||
`${repoId}::${prChecksCacheSuffix(12, { owner: 'Acme', repo: 'Widgets' }, 'head-a')}`
|
||||
]?.data?.[0].name
|
||||
).toBe('build')
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -426,11 +426,21 @@ function normalizedRepoIdentity(repo: GitHubOwnerRepo): string {
|
|||
return `${repo.owner.toLowerCase()}/${repo.repo.toLowerCase()}`
|
||||
}
|
||||
|
||||
export function prChecksCacheSuffix(prNumber: number, prRepo?: GitHubOwnerRepo | null): string {
|
||||
if (!prRepo) {
|
||||
return `pr-checks::${prNumber}`
|
||||
}
|
||||
return `pr-checks::${normalizedRepoIdentity(prRepo)}::${prNumber}`
|
||||
function normalizedHeadSha(headSha?: string): string | null {
|
||||
const trimmed = headSha?.trim()
|
||||
return trimmed ? trimmed.toLowerCase() : null
|
||||
}
|
||||
|
||||
export function prChecksCacheSuffix(
|
||||
prNumber: number,
|
||||
prRepo?: GitHubOwnerRepo | null,
|
||||
headSha?: string
|
||||
): string {
|
||||
const headSuffix = normalizedHeadSha(headSha)
|
||||
const base = prRepo
|
||||
? `pr-checks::${normalizedRepoIdentity(prRepo)}::${prNumber}`
|
||||
: `pr-checks::${prNumber}`
|
||||
return headSuffix ? `${base}::head::${headSuffix}` : base
|
||||
}
|
||||
|
||||
export function prCommentsCacheSuffix(prNumber: number, prRepo?: GitHubOwnerRepo | null): string {
|
||||
|
|
@ -1549,9 +1559,16 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
options
|
||||
): Promise<PRCheckDetail[]> => {
|
||||
const repoId = options?.repoId ?? get().repos?.find((repo) => repo.path === repoPath)?.id
|
||||
const cacheKey = repoScopedCacheKey(repoPath, repoId, prChecksCacheSuffix(prNumber, prRepo))
|
||||
const inflightKey = `${cacheKey}::${headSha ?? 'unknown'}`
|
||||
const cached = get().checksCache[cacheKey]
|
||||
const cacheKey = repoScopedCacheKey(
|
||||
repoPath,
|
||||
repoId,
|
||||
prChecksCacheSuffix(prNumber, prRepo, headSha)
|
||||
)
|
||||
const legacyCacheKey = headSha
|
||||
? repoScopedCacheKey(repoPath, repoId, prChecksCacheSuffix(prNumber, prRepo))
|
||||
: cacheKey
|
||||
const inflightKey = cacheKey
|
||||
const cached = get().checksCache[cacheKey] ?? get().checksCache[legacyCacheKey]
|
||||
if (
|
||||
!options?.force &&
|
||||
isFresh(cached, CHECKS_CACHE_TTL) &&
|
||||
|
|
@ -1630,7 +1647,7 @@ export const createGitHubSlice: StateCreator<AppState, [], [], GitHubSlice> = (s
|
|||
return checks
|
||||
} catch (err) {
|
||||
console.error('Failed to fetch PR checks:', err)
|
||||
const latestCached = get().checksCache[cacheKey]
|
||||
const latestCached = get().checksCache[cacheKey] ?? get().checksCache[legacyCacheKey]
|
||||
if (latestCached?.data && (!headSha || latestCached.headSha === headSha)) {
|
||||
return latestCached.data
|
||||
}
|
||||
|
|
|
|||
|
|
@ -843,6 +843,10 @@ export type GitHubWorkItem = {
|
|||
author: string | null
|
||||
branchName?: string
|
||||
baseRefName?: string
|
||||
// Why: PR checks are keyed by head commit; carrying this lets task rows use
|
||||
// the cached check-runs endpoint instead of one `gh pr checks` call per row.
|
||||
headSha?: string
|
||||
prRepo?: GitHubRepositoryIdentity
|
||||
additions?: number
|
||||
deletions?: number
|
||||
changedFiles?: number
|
||||
|
|
|
|||
Loading…
Reference in New Issue