mirror of
https://github.com/stablyai/orca.git
synced 2026-11-14 01:47:24 +00:00
Keep partial check details from replacing complete PR status (#38514)
* Keep PR check status tied to complete observations * Correct checks presentation types and count import * Preserve localized empty states and declared runtime support * Correct discovery fixture stack types * Omit empty failed-check subtitles * Preserve check identity through refresh and row publication * Withhold row identity for malformed check errors * Limit local capability wakes to local task sources * Correct PR check regression fixtures * Preserve browser APIs in PR check regression fixture * Settle PR check adoption after failed detail reads * Use the parsed PR check detail type during refresh * Preserve PR metadata across checks discovery and committed query lifetimes * Keep complete checks proof separate from legacy publication policy * Keep PR lookup fixtures independent from complete checks collection * Keep complete-check fixtures within lint contracts * test: align PR recordings with source check discovery
This commit is contained in:
@@ -13,6 +13,7 @@ export type PRCheckDetail = {
|
||||
| 'action_required'
|
||||
| null
|
||||
url: string | null
|
||||
observedHeadSha?: string
|
||||
checkRunId?: number
|
||||
workflowRunId?: number
|
||||
// GitLab trace retrieval requires numeric job id, so carry it through the Checks row.
|
||||
|
||||
@@ -0,0 +1,65 @@
|
||||
import { expect, it } from 'vitest'
|
||||
import {
|
||||
acceptPRCheckObservation,
|
||||
isCompletePRCheckOwner,
|
||||
projectPRCheckOwner
|
||||
} from './complete-pr-check-owner'
|
||||
|
||||
const summary = {
|
||||
state: 'success' as const,
|
||||
total: 101,
|
||||
passed: 101,
|
||||
failed: 0,
|
||||
pending: 0,
|
||||
neutral: 0
|
||||
}
|
||||
const pr = {
|
||||
number: 42,
|
||||
url: 'https://github.com/acme/widgets/pull/42',
|
||||
headSha: 'h1',
|
||||
checksStatus: 'success' as const,
|
||||
checksSummary: summary
|
||||
}
|
||||
const context = { scope: 'account:runtime1', complete: true, current: true }
|
||||
|
||||
it('retains last-good only for the same proved source, target and nonempty head', () => {
|
||||
const accepted = acceptPRCheckObservation(pr, null, context)!
|
||||
expect(
|
||||
acceptPRCheckObservation({ ...pr, checksSummary: undefined }, accepted, context)?.checksSummary
|
||||
).toEqual(summary)
|
||||
for (const next of [
|
||||
{ ...pr, headSha: 'h2' },
|
||||
{ ...pr, headSha: '' },
|
||||
{ ...pr, number: 43, url: 'https://github.com/acme/widgets/pull/43' },
|
||||
{ ...pr, url: 'https://github.com/other/widgets/pull/42' }
|
||||
]) {
|
||||
expect(
|
||||
acceptPRCheckObservation({ ...next, checksSummary: undefined }, accepted, context)
|
||||
?.checksSummary
|
||||
).toBeUndefined()
|
||||
}
|
||||
expect(
|
||||
acceptPRCheckObservation({ ...pr, checksSummary: undefined }, accepted, {
|
||||
...context,
|
||||
scope: 'account:runtime2'
|
||||
})?.checksSummary
|
||||
).toBeUndefined()
|
||||
expect(acceptPRCheckObservation(pr, accepted, { ...context, current: false })).toBeNull()
|
||||
expect(
|
||||
acceptPRCheckObservation({ ...pr, checksSummary: undefined }, pr, context)?.checksSummary
|
||||
).toBeUndefined()
|
||||
})
|
||||
|
||||
it('carries unavailable authority explicitly through display and review projections without lending it to another head', () => {
|
||||
const unavailable = acceptPRCheckObservation({ ...pr, checksSummary: undefined }, null, context)!
|
||||
const display = projectPRCheckOwner(unavailable, { ...unavailable })
|
||||
expect(isCompletePRCheckOwner(display)).toBe(true)
|
||||
const review = projectPRCheckOwner(display, { ...display, status: display.checksStatus })
|
||||
expect(isCompletePRCheckOwner(review)).toBe(true)
|
||||
expect(isCompletePRCheckOwner({ ...review })).toBe(false)
|
||||
expect(isCompletePRCheckOwner(projectPRCheckOwner(review, { ...review, headSha: 'h2' }))).toBe(
|
||||
false
|
||||
)
|
||||
const full = acceptPRCheckObservation(pr, null, context)!
|
||||
expect(isCompletePRCheckOwner({ ...pr, checksSummary: full.checksSummary })).toBe(false)
|
||||
})
|
||||
@@ -0,0 +1,133 @@
|
||||
import type { CheckStatus, GitHubOwnerRepo, ProviderCheckSummary } from './pull-request-types'
|
||||
import { githubRepoIdentityKey } from './repository-identity-key'
|
||||
import { matchesGitHubWorkItemTarget, parseGitHubItemRepository } from './work-item-target'
|
||||
|
||||
type CheckOwner = {
|
||||
number?: number
|
||||
url?: string
|
||||
headSha?: string
|
||||
prRepo?: GitHubOwnerRepo
|
||||
githubRepository?: GitHubOwnerRepo
|
||||
checksSummary?: ProviderCheckSummary
|
||||
checksStatus?: CheckStatus
|
||||
status?: CheckStatus
|
||||
}
|
||||
type Receipt = {
|
||||
observation: object
|
||||
scope: string
|
||||
target: string
|
||||
head: string | undefined
|
||||
summary: ProviderCheckSummary | undefined
|
||||
status: CheckStatus | undefined
|
||||
}
|
||||
export type PRCheckObservationContext = {
|
||||
scope: string
|
||||
complete: boolean
|
||||
current: boolean
|
||||
trusted?: boolean
|
||||
}
|
||||
const receipts = new WeakMap<object, Receipt>()
|
||||
|
||||
function targetKey(owner: CheckOwner): string | undefined {
|
||||
if (!owner.url || !owner.number) {
|
||||
return undefined
|
||||
}
|
||||
const repository = owner.prRepo ?? owner.githubRepository ?? parseGitHubItemRepository(owner.url)
|
||||
if (
|
||||
!repository ||
|
||||
!matchesGitHubWorkItemTarget(
|
||||
{ url: owner.url, number: owner.number, type: 'pr', prRepo: repository },
|
||||
{ repository, number: owner.number, type: 'pr' }
|
||||
)
|
||||
) {
|
||||
return undefined
|
||||
}
|
||||
return `${githubRepoIdentityKey(repository)}:${owner.number}`
|
||||
}
|
||||
function status(owner: CheckOwner): CheckStatus | undefined {
|
||||
return owner.checksStatus ?? owner.status
|
||||
}
|
||||
function receipt(owner: CheckOwner): Receipt | undefined {
|
||||
const proof = receipts.get(owner)
|
||||
return proof &&
|
||||
proof.target === targetKey(owner) &&
|
||||
proof.head === owner.headSha &&
|
||||
proof.summary === owner.checksSummary &&
|
||||
proof.status === status(owner)
|
||||
? proof
|
||||
: undefined
|
||||
}
|
||||
|
||||
// Receipts stay on accepted objects; neither deserialization nor borrowing a summary transfers authority.
|
||||
export function markCompletePRCheckOwner<T extends CheckOwner>(owner: T, scope: string): T {
|
||||
const target = targetKey(owner)
|
||||
if (target) {
|
||||
receipts.set(owner, {
|
||||
observation: owner,
|
||||
scope,
|
||||
target,
|
||||
head: owner.headSha,
|
||||
summary: owner.checksSummary,
|
||||
status: status(owner)
|
||||
})
|
||||
}
|
||||
return owner
|
||||
}
|
||||
export function completePRCheckOwnerScope(owner: CheckOwner): string | undefined {
|
||||
return receipt(owner)?.scope
|
||||
}
|
||||
export function isCompletePRCheckOwner(owner: CheckOwner): boolean {
|
||||
return receipt(owner) !== undefined
|
||||
}
|
||||
|
||||
export function samePRCheckObservation(a: CheckOwner, b: CheckOwner): boolean {
|
||||
const first = receipt(a)
|
||||
return Boolean(first && first.observation === receipt(b)?.observation)
|
||||
}
|
||||
export function projectPRCheckOwner<T extends CheckOwner>(owner: CheckOwner, projected: T): T {
|
||||
const proof = receipt(owner)
|
||||
if (
|
||||
proof &&
|
||||
proof.target === targetKey(projected) &&
|
||||
proof.head === projected.headSha &&
|
||||
proof.summary === projected.checksSummary
|
||||
) {
|
||||
receipts.set(projected, { ...proof, status: status(projected) })
|
||||
}
|
||||
return projected
|
||||
}
|
||||
|
||||
/** The existing request/selection owner decides current; this rule owns checks identity and last-good. */
|
||||
export function acceptPRCheckObservation<T extends CheckOwner>(
|
||||
next: T,
|
||||
previous: CheckOwner | null | undefined,
|
||||
context: PRCheckObservationContext
|
||||
): (T & { checksSummary?: ProviderCheckSummary }) | null {
|
||||
if (!context.current) {
|
||||
return null
|
||||
}
|
||||
if (!context.complete) {
|
||||
return next
|
||||
}
|
||||
const prior = previous && receipt(previous)
|
||||
const supplied = context.trusted !== false && Boolean(next.headSha) && Boolean(next.checksSummary)
|
||||
const keep =
|
||||
!supplied &&
|
||||
prior &&
|
||||
prior.scope === context.scope &&
|
||||
next.headSha &&
|
||||
next.headSha === prior.head &&
|
||||
targetKey(next) === prior.target
|
||||
const checksSummary = supplied ? next.checksSummary : keep ? previous?.checksSummary : undefined
|
||||
const checksStatus =
|
||||
checksSummary && checksSummary.state !== 'none' ? checksSummary.state : 'neutral'
|
||||
const accepted = {
|
||||
...next,
|
||||
checksSummary,
|
||||
...('checksStatus' in next ? { checksStatus } : {}),
|
||||
...('status' in next ? { status: checksStatus } : {})
|
||||
}
|
||||
return supplied && receipt(next)?.scope === context.scope
|
||||
? projectPRCheckOwner(next, accepted)
|
||||
: markCompletePRCheckOwner(accepted, context.scope)
|
||||
}
|
||||
@@ -0,0 +1,34 @@
|
||||
export function completePRCheckGraphQLResponse(
|
||||
number: number,
|
||||
head: string,
|
||||
contexts: Record<string, unknown>[] = []
|
||||
): { stdout: string } {
|
||||
return {
|
||||
stdout: JSON.stringify({
|
||||
data: {
|
||||
repository: {
|
||||
pullRequest: {
|
||||
number,
|
||||
headRefOid: head,
|
||||
commits: {
|
||||
nodes: [
|
||||
{
|
||||
commit: {
|
||||
oid: head,
|
||||
statusCheckRollup: {
|
||||
contexts: {
|
||||
nodes: contexts,
|
||||
pageInfo: { hasNextPage: false, endCursor: null }
|
||||
}
|
||||
},
|
||||
checkSuites: { nodes: [], pageInfo: { hasNextPage: false, endCursor: null } }
|
||||
}
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,6 @@
|
||||
/** Only an immutable full commit ID can identify the provider read behind a check row. */
|
||||
export function normalizePRCheckHeadSha(value: unknown): string | undefined {
|
||||
return typeof value === 'string' && /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/i.test(value)
|
||||
? value.toLowerCase()
|
||||
: undefined
|
||||
}
|
||||
Reference in New Issue
Block a user