Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, unknown>;
const checksOf = (statusCheckRollup: ReadonlyArray<object>) => {
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<string, unknown>));
// r2 approved without writing anything, which is still the event worth seeing.
Expand Down
16 changes: 12 additions & 4 deletions packages/source-control-github/src/server/gitHubPullRequestJson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1576,6 +1576,9 @@ function toCheckStatus(raw: Schema.Schema.Type<typeof RawCheckSchema>): 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;
Expand All @@ -1588,8 +1591,9 @@ function isNamelessCheck(raw: Schema.Schema.Type<typeof RawCheckSchema>): 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<Schema.Schema.Type<typeof RawCheckSchema>> | null | undefined,
Expand All @@ -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)),
},
];
});
Expand Down
Loading