| author |  Scott Norteman <snorteman@gmail.com> 2026-10-10 07:38:26-04:00 |
| committer |  GitHub <noreply@github.com> 2026-10-10 06:38:26-05:00 |
| log | a418075154d1d4f4263d5fa55804b9288f61d3dd |
| tree | 9574d875b8652f5e21b913e06fc2b6a282466b4e |
| parent | 02612c28564056ddb7ae6b06cedd31dafc235ffd |
| signature | Signed by PGP key B5690EEEBB952194 |
fix(server): PR watch no longer reports passed while a second run of a check is still going (#17344)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>2 files changed, 52 insertions(+), 4 deletions(-)
packages
source-control-github
packages/source-control-github/src/server/gitHubPullRequestJson.test.ts+40
| ... | ... | @@ -435,6 +435,46 @@ describe("pull request detail decoding", () => { |
| 435 | 435 | expect(detail.checksState).toBe("pending"); |
| 436 | 436 | }); |
| 437 | 437 | |
| 438 | it("keeps a check pending while another run of it on the same commit is still going", () => { | |
| 439 | // A workflow on both `push` and `pull_request` runs the same check twice on one commit. The | |
| 440 | // `pull_request` run finished after the `push` run started, and the `push` run is still going. | |
| 441 | const finished = { | |
| 442 | __typename: "CheckRun", | |
| 443 | name: "check", | |
| 444 | workflowName: "Checks", | |
| 445 | status: "COMPLETED", | |
| 446 | conclusion: "SUCCESS", | |
| 447 | startedAt: "2026-10-08T04:06:10Z", | |
| 448 | completedAt: "2026-10-08T04:18:58Z", | |
| 449 | }; | |
| 450 | const running = { | |
| 451 | ...finished, | |
| 452 | status: "IN_PROGRESS", | |
| 453 | conclusion: "", | |
| 454 | startedAt: "2026-10-08T04:06:13Z", | |
| 455 | completedAt: "0001-01-01T00:00:00Z", | |
| 456 | }; | |
| 457 | // A run still queued has not started either, so it carries no time at all. | |
| 458 | const queued = { ...running, status: "QUEUED", startedAt: "0001-01-01T00:00:00Z" }; | |
| 459 | const raw = JSON.parse(detailJson) as Record<string, unknown>; | |
| 460 | const checksOf = (statusCheckRollup: ReadonlyArray<object>) => { | |
| 461 | const detail = expectSuccess( | |
| 462 | decodePullRequestDetailJson(JSON.stringify({ ...raw, statusCheckRollup })), | |
| 463 | ); | |
| 464 | return [detail.checks.map((check) => [check.name, check.status]), detail.checksState]; | |
| 465 | }; | |
| 466 | ||
| 467 | // Both listing orders, so the run still going wins on its own rather than by coming last. | |
| 468 | for (const rollup of [ | |
| 469 | [finished, running], | |
| 470 | [running, finished], | |
| 471 | [finished, queued], | |
| 472 | [queued, finished], | |
| 473 | ]) { | |
| 474 | expect(checksOf(rollup)).toEqual([[["check", "pending"]], "pending"]); | |
| 475 | } | |
| 476 | }); | |
| 477 | ||
| 438 | 478 | it("merges reviews with comments in time order and keeps a bodyless approval", () => { |
| 439 | 479 | const detail = expectSuccess(activity(JSON.parse(detailJson) as Record<string, unknown>)); |
| 440 | 480 | // r2 approved without writing anything, which is still the event worth seeing. |
packages/source-control-github/src/server/gitHubPullRequestJson.ts+12-4
| ... | ... | @@ -1576,6 +1576,9 @@ function toCheckStatus(raw: Schema.Schema.Type<typeof RawCheckSchema>): PullRequ |
| 1576 | 1576 | /** What GitHub writes where a run has not reached that moment yet, which is not a time. */ |
| 1577 | 1577 | const UNSET_TIMESTAMP = "0001-01-01T00:00:00Z"; |
| 1578 | 1578 | |
| 1579 | /** Sorts after every real timestamp, so the deduper keeps a run that has not finished. */ | |
| 1580 | const STILL_RUNNING = "9999-12-31T23:59:59Z"; | |
| 1581 | ||
| 1579 | 1582 | function realTimestamp(value: string | null | undefined): string | null { |
| 1580 | 1583 | const at = trimmed(value); |
| 1581 | 1584 | return at === null || at === UNSET_TIMESTAMP ? null : at; |
| ... | ... | @@ -1588,8 +1591,9 @@ function isNamelessCheck(raw: Schema.Schema.Type<typeof RawCheckSchema>): boolea |
| 1588 | 1591 | |
| 1589 | 1592 | /** |
| 1590 | 1593 | * The rollup as the deduper reads it: a check, the workflow that owns it, and when the run last |
| 1591 | * had something to say. A queued run reports a completion time it has not reached, so the start | |
| 1592 | * stands in for it rather than sorting the newest run to the bottom. | |
| 1594 | * had something to say. A run that has not finished outranks any finished run of the same check: | |
| 1595 | * a workflow triggered by both `push` and `pull_request` runs the check twice on one commit, and | |
| 1596 | * the run that finished first must not hide the one still going. | |
| 1593 | 1597 | */ |
| 1594 | 1598 | function toCheckEntries( |
| 1595 | 1599 | raw: ReadonlyArray<Schema.Schema.Type<typeof RawCheckSchema>> | null | undefined, |
| ... | ... | @@ -1601,17 +1605,21 @@ function toCheckEntries( |
| 1601 | 1605 | return (raw ?? []).flatMap((check) => { |
| 1602 | 1606 | const name = trimmed(check.name) ?? trimmed(check.context); |
| 1603 | 1607 | if (name === null) return []; |
| 1608 | const status = toCheckStatus(check); | |
| 1604 | 1609 | return [ |
| 1605 | 1610 | { |
| 1606 | 1611 | check: { |
| 1607 | 1612 | name, |
| 1608 | status: toCheckStatus(check), | |
| 1613 | status, | |
| 1609 | 1614 | description: trimmed(check.description), |
| 1610 | 1615 | url: trimmed(check.detailsUrl) ?? trimmed(check.targetUrl), |
| 1611 | 1616 | ...(typeof check.isRequired === "boolean" ? { required: check.isRequired } : {}), |
| 1612 | 1617 | }, |
| 1613 | 1618 | workflowName: trimmed(check.workflowName), |
| 1614 | at: realTimestamp(check.completedAt) ?? realTimestamp(check.startedAt), | |
| 1619 | at: | |
| 1620 | status === "pending" | |
| 1621 | ? STILL_RUNNING | |
| 1622 | : (realTimestamp(check.completedAt) ?? realTimestamp(check.startedAt)), | |
| 1615 | 1623 | }, |
| 1616 | 1624 | ]; |
| 1617 | 1625 | }); |