From dbcc96d47d1cf338b9cb3defe8ac7fb2442f09ac Mon Sep 17 00:00:00 2001 From: ScottN-PV Date: Thu, 8 Oct 2026 18:26:32 -0400 Subject: [PATCH] fix(server): PR watch no longer reports passed while a second run of a check is still going A workflow on both push and pull_request runs the same check twice on one commit. The GitHub check deduper kept the run with the newest timestamp, so a run that finished hid one still going, and PR watch reported all checks passed. A run that has not finished now outranks finished runs of the same check in GitHub's rollup reader. Co-Authored-By: Claude Opus 5.5 --- .../src/server/gitHubPullRequestJson.test.ts | 40 +++++++++++++++++++ .../src/server/gitHubPullRequestJson.ts | 16 ++++++-- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index 06e25c6e4709..145337a86dba 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -435,6 +435,46 @@ describe("pull request detail decoding", () => { expect(detail.checksState).toBe("pending"); }); + it("keeps a check pending while another run of it on the same commit is still going", () => { + // A workflow on both `push` and `pull_request` runs the same check twice on one commit. The + // `pull_request` run finished after the `push` run started, and the `push` run is still going. + const finished = { + __typename: "CheckRun", + name: "check", + workflowName: "Checks", + status: "COMPLETED", + conclusion: "SUCCESS", + startedAt: "2026-10-08T04:06:10Z", + completedAt: "2026-10-08T04:18:58Z", + }; + const running = { + ...finished, + status: "IN_PROGRESS", + conclusion: "", + startedAt: "2026-10-08T04:06:13Z", + completedAt: "0001-01-01T00:00:00Z", + }; + // A run still queued has not started either, so it carries no time at all. + const queued = { ...running, status: "QUEUED", startedAt: "0001-01-01T00:00:00Z" }; + const raw = JSON.parse(detailJson) as Record; + const checksOf = (statusCheckRollup: ReadonlyArray) => { + const detail = expectSuccess( + decodePullRequestDetailJson(JSON.stringify({ ...raw, statusCheckRollup })), + ); + return [detail.checks.map((check) => [check.name, check.status]), detail.checksState]; + }; + + // Both listing orders, so the run still going wins on its own rather than by coming last. + for (const rollup of [ + [finished, running], + [running, finished], + [finished, queued], + [queued, finished], + ]) { + expect(checksOf(rollup)).toEqual([[["check", "pending"]], "pending"]); + } + }); + it("merges reviews with comments in time order and keeps a bodyless approval", () => { const detail = expectSuccess(activity(JSON.parse(detailJson) as Record)); // r2 approved without writing anything, which is still the event worth seeing. diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index 956f1ef5f4ad..e3768b04d7bc 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -1576,6 +1576,9 @@ function toCheckStatus(raw: Schema.Schema.Type): PullRequ /** What GitHub writes where a run has not reached that moment yet, which is not a time. */ const UNSET_TIMESTAMP = "0001-01-01T00:00:00Z"; +/** Sorts after every real timestamp, so the deduper keeps a run that has not finished. */ +const STILL_RUNNING = "9999-12-31T23:59:59Z"; + function realTimestamp(value: string | null | undefined): string | null { const at = trimmed(value); return at === null || at === UNSET_TIMESTAMP ? null : at; @@ -1588,8 +1591,9 @@ function isNamelessCheck(raw: Schema.Schema.Type): boolea /** * The rollup as the deduper reads it: a check, the workflow that owns it, and when the run last - * had something to say. A queued run reports a completion time it has not reached, so the start - * stands in for it rather than sorting the newest run to the bottom. + * had something to say. A run that has not finished outranks any finished run of the same check: + * a workflow triggered by both `push` and `pull_request` runs the check twice on one commit, and + * the run that finished first must not hide the one still going. */ function toCheckEntries( raw: ReadonlyArray> | null | undefined, @@ -1601,17 +1605,21 @@ function toCheckEntries( return (raw ?? []).flatMap((check) => { const name = trimmed(check.name) ?? trimmed(check.context); if (name === null) return []; + const status = toCheckStatus(check); return [ { check: { name, - status: toCheckStatus(check), + status, description: trimmed(check.description), url: trimmed(check.detailsUrl) ?? trimmed(check.targetUrl), ...(typeof check.isRequired === "boolean" ? { required: check.isRequired } : {}), }, workflowName: trimmed(check.workflowName), - at: realTimestamp(check.completedAt) ?? realTimestamp(check.startedAt), + at: + status === "pending" + ? STILL_RUNNING + : (realTimestamp(check.completedAt) ?? realTimestamp(check.startedAt)), }, ]; });