From 03abb02672da0dd25adcb07cb3b590183c1d3e18 Mon Sep 17 00:00:00 2001 From: akash3444 Date: Thu, 17 Sep 2026 22:46:39 +0530 Subject: [PATCH 1/6] fix(pull-request): prioritize failed checks - Show failed and cancelled checks before other pull request checks - Preserve ordering within failing and remaining check groups --- .../bitbucketPullRequestJson.test.ts | 2 +- .../pullRequest/gitHubPullRequestJson.test.ts | 2 +- .../src/pullRequest/pullRequestChecks.test.ts | 22 +++++++++++++++++-- .../src/pullRequest/pullRequestChecks.ts | 14 +++++++++--- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts b/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts index 356c5da96f33..f975f8450c37 100644 --- a/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/bitbucketPullRequestJson.test.ts @@ -328,8 +328,8 @@ describe("decodeStatusesJson", () => { ); expect(decoded.items.map((check) => [check.name, check.status])).toEqual([ - ["build / Pipeline", "success"], ["deploy / Pipeline", "failure"], + ["build / Pipeline", "success"], ]); }); }); diff --git a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts index 1a041dc333b1..1c3c288891df 100644 --- a/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts +++ b/apps/server/src/pullRequest/gitHubPullRequestJson.test.ts @@ -234,8 +234,8 @@ describe("pull request detail decoding", () => { it("maps check-run status and commit-status state onto one vocabulary", () => { const detail = expectSuccess(decodePullRequestDetailJson(detailJson)); expect(detail.checks.map((check) => [check.name, check.status])).toEqual([ - ["build", "pending"], ["test", "failure"], + ["build", "pending"], ["ci/legacy", "success"], ]); }); diff --git a/apps/server/src/pullRequest/pullRequestChecks.test.ts b/apps/server/src/pullRequest/pullRequestChecks.test.ts index ba6850832924..11524bd43747 100644 --- a/apps/server/src/pullRequest/pullRequestChecks.test.ts +++ b/apps/server/src/pullRequest/pullRequestChecks.test.ts @@ -50,6 +50,24 @@ describe("dedupeChecks", () => { ]); }); + it("puts failed and cancelled checks first without reshuffling either group", () => { + const checks = dedupeChecks([ + entry("lint", "success"), + entry("unit", "failure"), + entry("types", "pending"), + entry("integration", "cancelled"), + entry("format", "success"), + ]); + + expect(checks.map((check) => [check.name, check.status])).toEqual([ + ["unit", "failure"], + ["integration", "cancelled"], + ["lint", "success"], + ["types", "pending"], + ["format", "success"], + ]); + }); + it("loses a run that never said when it happened to one that did, whichever came first", () => { const undated = dedupeChecks([ entry("build", "success", { at: "2026-08-11T16:00:00Z" }), @@ -76,8 +94,8 @@ describe("dedupeChecks", () => { ]); expect(checks.map((check) => [check.name, check.status])).toEqual([ - ["CI / build", "success"], ["Release / build", "failure"], + ["CI / build", "success"], ]); }); @@ -88,6 +106,6 @@ describe("dedupeChecks", () => { entry("build", "failure", { workflowName: "CI", at: "2026-08-11T16:00:00Z" }), ]); - expect(checks.map((check) => check.name)).toEqual(["build", "CI / build"]); + expect(checks.map((check) => check.name)).toEqual(["CI / build", "build"]); }); }); diff --git a/apps/server/src/pullRequest/pullRequestChecks.ts b/apps/server/src/pullRequest/pullRequestChecks.ts index 819cf47b01af..60470a001d73 100644 --- a/apps/server/src/pullRequest/pullRequestChecks.ts +++ b/apps/server/src/pullRequest/pullRequestChecks.ts @@ -19,8 +19,9 @@ function isAtLeastAsNew(candidate: string | null, kept: string | null): boolean * one before it, and a run that never said when it happened loses to one that did. A tie goes to * whichever came last, because a host lists a re-run after the run it repeats. * - * The order is the host's own, held at the place each check first appeared, so a re-run landing - * mid-read replaces a row where it stands instead of reshuffling the list under the reader. + * Failed and cancelled checks come first so the reason for a failing rollup is immediately visible. + * Within that group and the remaining checks, the order is the host's own, held at the place each + * check first appeared. A re-run landing mid-read replaces a row where it stands. * * Two checks that survive under the same name are then genuinely different ones, since they came * from different workflows — each is shown as `workflow / name`, the way GitHub writes it itself. @@ -46,10 +47,17 @@ export function dedupeChecks( for (const entry of survivors) { countsByName.set(entry.check.name, (countsByName.get(entry.check.name) ?? 0) + 1); } - return survivors.map((entry) => { + const checks = survivors.map((entry) => { const workflowName = entry.workflowName ?? ""; return workflowName.length > 0 && (countsByName.get(entry.check.name) ?? 0) > 1 ? { ...entry.check, name: `${workflowName} / ${entry.check.name}` } : entry.check; }); + const failing = checks.filter( + (check) => check.status === "failure" || check.status === "cancelled", + ); + return [ + ...failing, + ...checks.filter((check) => check.status !== "failure" && check.status !== "cancelled"), + ]; } From bbaa37894d45e7229273c82df24026e7ba9a3093 Mon Sep 17 00:00:00 2001 From: akash3444 Date: Thu, 17 Sep 2026 22:55:47 +0530 Subject: [PATCH 2/6] fix(server): prioritize failed pull request checks - Sort failed and cancelled checks before passing checks while preserving relative order --- apps/server/src/pullRequest/pullRequestChecks.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/apps/server/src/pullRequest/pullRequestChecks.ts b/apps/server/src/pullRequest/pullRequestChecks.ts index 60470a001d73..aa64ccc2bc2a 100644 --- a/apps/server/src/pullRequest/pullRequestChecks.ts +++ b/apps/server/src/pullRequest/pullRequestChecks.ts @@ -53,11 +53,9 @@ export function dedupeChecks( ? { ...entry.check, name: `${workflowName} / ${entry.check.name}` } : entry.check; }); - const failing = checks.filter( - (check) => check.status === "failure" || check.status === "cancelled", - ); - return [ - ...failing, - ...checks.filter((check) => check.status !== "failure" && check.status !== "cancelled"), - ]; + return checks.toSorted((left, right) => { + const leftFailed = left.status === "failure" || left.status === "cancelled"; + const rightFailed = right.status === "failure" || right.status === "cancelled"; + return Number(rightFailed) - Number(leftFailed); + }); } From b3b2263a8c551df3977d7a23096a07ab70b962a3 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 16:30:31 +0000 Subject: [PATCH 3/6] fix(source-control): show failed checks first --- .../src/server/pullRequestChecks.test.ts | 23 +++++++++++++++++-- .../src/server/pullRequestChecks.ts | 6 ++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/source-control-core/src/server/pullRequestChecks.test.ts b/packages/source-control-core/src/server/pullRequestChecks.test.ts index 34c8ed6fefd4..0596ecba2260 100644 --- a/packages/source-control-core/src/server/pullRequestChecks.test.ts +++ b/packages/source-control-core/src/server/pullRequestChecks.test.ts @@ -133,6 +133,25 @@ describe("dedupeChecks", () => { ]); }); + it("puts failed and cancelled checks first, preserving each group's order after reruns", () => { + const checks = dedupeChecks([ + entry("lint", "failure", { at: "2026-08-11T16:00:00Z" }), + entry("integration", "cancelled"), + entry("types", "pending"), + entry("unit", "failure"), + entry("format", "success"), + entry("lint", "success", { at: "2026-08-11T18:00:00Z" }), + ]); + + expect(checks.map((check) => [check.name, check.status])).toEqual([ + ["integration", "cancelled"], + ["unit", "failure"], + ["lint", "success"], + ["types", "pending"], + ["format", "success"], + ]); + }); + it("keeps distinct workflow and check pairs even when their names join to the same text", () => { const checks = dedupeChecks([ entry("Linux tests", "success", { @@ -181,8 +200,8 @@ describe("dedupeChecks", () => { ]); expect(checks.map((check) => [check.name, check.status])).toEqual([ - ["CI / build", "success"], ["Release / build", "failure"], + ["CI / build", "success"], ]); }); @@ -193,6 +212,6 @@ describe("dedupeChecks", () => { entry("build", "failure", { workflowName: "CI", at: "2026-08-11T16:00:00Z" }), ]); - expect(checks.map((check) => check.name)).toEqual(["build", "CI / build"]); + expect(checks.map((check) => check.name)).toEqual(["CI / build", "build"]); }); }); diff --git a/packages/source-control-core/src/server/pullRequestChecks.ts b/packages/source-control-core/src/server/pullRequestChecks.ts index 9119240dd315..a923ebc46bc1 100644 --- a/packages/source-control-core/src/server/pullRequestChecks.ts +++ b/packages/source-control-core/src/server/pullRequestChecks.ts @@ -56,7 +56,11 @@ export function dedupeChecks( const kept = newestByCheck.get(key); if (kept === undefined || isAtLeastAsNew(entry.at, kept.at)) newestByCheck.set(key, entry); } - const survivors = [...newestByCheck.values()]; + const survivors = [...newestByCheck.values()].sort( + (a, b) => + Number(b.check.status === "failure" || b.check.status === "cancelled") - + Number(a.check.status === "failure" || a.check.status === "cancelled"), + ); const countsByName = new Map(); for (const entry of survivors) { countsByName.set(entry.check.name, (countsByName.get(entry.check.name) ?? 0) + 1); From 46774eef620d563674dc0f8e6c42d4a03bb4e386 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 16:38:12 +0000 Subject: [PATCH 4/6] fix(source-control): order checks after page aggregation --- .../server/BitbucketPullRequestApi.test.ts | 27 +++++++++++++++-- .../src/server/BitbucketPullRequestApi.ts | 3 +- .../src/server/pullRequestChecks.ts | 30 ++++++++++++------- .../src/server/gitCafePullRequestJson.test.ts | 24 +++++++++++---- .../src/server/gitCafePullRequestJson.ts | 15 ++++++---- 5 files changed, 72 insertions(+), 27 deletions(-) diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts index 9132b936b9e7..f55ccae0224a 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts @@ -568,18 +568,39 @@ layer("BitbucketPullRequestApi.layer", (it) => { const next = "https://api.bitbucket.org/2.0/statuses?page=2"; mockedRequest .mockReturnValueOnce( - Effect.succeed(response(valuePage([{ name: "Build", state: "SUCCESSFUL" }], next))), + Effect.succeed( + response( + valuePage( + [ + { name: "Build", state: "SUCCESSFUL" }, + { name: "Tests", state: "STOPPED" }, + ], + next, + ), + ), + ), ) .mockReturnValueOnce( - Effect.succeed(response(valuePage([{ name: "Lint", state: "FAILED" }]))), + Effect.succeed( + response( + valuePage([ + { name: "Lint", state: "FAILED" }, + { name: "Deploy", state: "INPROGRESS" }, + { name: "Audit", state: "FAILED" }, + ]), + ), + ), ); const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; const checks = yield* api.listChecks({ repository: "acme/web", number: 7 }); expect(checks.map((check) => [check.name, check.status])).toEqual([ - ["Build", "success"], + ["Tests", "cancelled"], ["Lint", "failure"], + ["Audit", "failure"], + ["Build", "success"], + ["Deploy", "pending"], ]); expect(callAt(1).url).toBe(next); }), diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts index 4cb963ff30bb..9d80f26fcde4 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts @@ -23,6 +23,7 @@ import type { import * as BitbucketApi from "./BitbucketApi.ts"; import { parseDiffFileRevisions } from "@t3tools/source-control-core/server/diffFileRevisions"; +import { sortChecks } from "@t3tools/source-control-core/server/pullRequestChecks"; import { buildReviewThreads, decodeCommentsJson, @@ -672,7 +673,7 @@ export const make = Effect.gen(function* () { decode: decodeStatusesJson, items: [], prepend: false, - }), + }).pipe(Effect.map(sortChecks)), ), listReviewerCandidates: (input) => diff --git a/packages/source-control-core/src/server/pullRequestChecks.ts b/packages/source-control-core/src/server/pullRequestChecks.ts index a923ebc46bc1..159b802cd4c2 100644 --- a/packages/source-control-core/src/server/pullRequestChecks.ts +++ b/packages/source-control-core/src/server/pullRequestChecks.ts @@ -36,6 +36,16 @@ function isAtLeastAsNew(candidate: string | null, kept: string | null): boolean ); } +export function sortChecks( + checks: ReadonlyArray, +): ReadonlyArray { + return checks.toSorted( + (a, b) => + Number(b.status === "failure" || b.status === "cancelled") - + Number(a.status === "failure" || a.status === "cancelled"), + ); +} + /** * A host's rollup is a list of runs, not a list of checks: while a workflow is being re-run — or * while a second run of it is already live — the same check arrives twice, and both copies reach @@ -56,19 +66,17 @@ export function dedupeChecks( const kept = newestByCheck.get(key); if (kept === undefined || isAtLeastAsNew(entry.at, kept.at)) newestByCheck.set(key, entry); } - const survivors = [...newestByCheck.values()].sort( - (a, b) => - Number(b.check.status === "failure" || b.check.status === "cancelled") - - Number(a.check.status === "failure" || a.check.status === "cancelled"), - ); + const survivors = [...newestByCheck.values()]; const countsByName = new Map(); for (const entry of survivors) { countsByName.set(entry.check.name, (countsByName.get(entry.check.name) ?? 0) + 1); } - return survivors.map((entry) => { - const workflowName = entry.workflowName ?? ""; - return workflowName.length > 0 && (countsByName.get(entry.check.name) ?? 0) > 1 - ? { ...entry.check, name: `${workflowName} / ${entry.check.name}` } - : entry.check; - }); + return sortChecks( + survivors.map((entry) => { + const workflowName = entry.workflowName ?? ""; + return workflowName.length > 0 && (countsByName.get(entry.check.name) ?? 0) > 1 + ? { ...entry.check, name: `${workflowName} / ${entry.check.name}` } + : entry.check; + }), + ); } diff --git a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts index 01de875fa42d..9235efcc74fa 100644 --- a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts +++ b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.test.ts @@ -494,8 +494,12 @@ describe("toDiff", () => { describe("toChecks", () => { it("maps GitCafe's status and conclusion onto the shared check states", () => { - const check = (status: string, conclusion: string | null) => ({ - name: `${status}-${conclusion}`, + const check = ( + status: string, + conclusion: string | null, + name = `${status}-${conclusion}`, + ) => ({ + name, status, conclusion, summary: null, @@ -506,16 +510,24 @@ describe("toChecks", () => { items: [ check("queued", null), check("in_progress", null), - check("completed", "success"), + check("completed", "success", "Build"), + check("completed", "cancelled"), check("completed", "action_required"), - check("completed", "timed_out"), + check("completed", "timed_out", "Build"), ], next: null, }), ); assert.deepStrictEqual( - checks.map((item) => item.status), - ["pending", "pending", "success", "action-required", "failure"], + checks.map((item) => [item.name, item.status]), + [ + ["completed-cancelled", "cancelled"], + ["Build", "failure"], + ["queued-null", "pending"], + ["in_progress-null", "pending"], + ["Build", "success"], + ["completed-action_required", "action-required"], + ], ); }); }); diff --git a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts index 073e3fcebff4..66d1bf3a2b33 100644 --- a/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts +++ b/packages/source-control-gitcafe/src/server/gitCafePullRequestJson.ts @@ -1,4 +1,5 @@ import { quoteGitPatchPath } from "@t3tools/shared/gitPatchPath"; +import { sortChecks } from "@t3tools/source-control-core/server/pullRequestChecks"; import * as DateTime from "effect/DateTime"; import * as Schema from "effect/Schema"; import { @@ -549,10 +550,12 @@ function checkStatus(check: (typeof GitCafeChecks.Type.items)[number]): PullRequ } export function toChecks(checks: typeof GitCafeChecks.Type): ReadonlyArray { - return checks.items.map((check) => ({ - name: check.name, - status: checkStatus(check), - description: check.summary, - url: check.detailsUrl, - })); + return sortChecks( + checks.items.map((check) => ({ + name: check.name, + status: checkStatus(check), + description: check.summary, + url: check.detailsUrl, + })), + ); } From bed9a43a8b0ea5e314a3cf8739fe98732a4989df Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Mon, 19 Oct 2026 16:59:43 +0000 Subject: [PATCH 5/6] test(gitcafe): expect failed checks before passing checks --- .../src/server/GitCafePullRequestProvider.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts index 4a3521d5c7bb..7a9a4dcd6a42 100644 --- a/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts +++ b/packages/source-control-gitcafe/src/server/GitCafePullRequestProvider.test.ts @@ -646,7 +646,7 @@ describe("GitCafePullRequestProvider", () => { }); assert.deepStrictEqual( detail.checks.map((check) => check.name), - ["early", "late"], + ["late", "early"], ); assert.deepStrictEqual( detail.reviewers.map((reviewer) => reviewer.login), From 79724d551d7b764724eac9eb9a61adfd24eb1c6e Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Tue, 20 Oct 2026 04:29:45 +0000 Subject: [PATCH 6/6] test(forgejo): expect failed checks first Co-Authored-By: Claude Opus 5.5 --- .../ForgejoSourceControlProvider.test.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts index 8846a6b6af39..f71ef9559139 100644 --- a/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts +++ b/packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts @@ -240,11 +240,17 @@ it.effect.each([ for (const checks of [summary.checks, detail.checks]) { assert.deepStrictEqual( checks.map((check) => [check.name, check.status, check.required]), - [ - ["ci/required-build", "success", supported ? true : undefined], - ["ci/optional-lint", supported ? "failure" : "success", undefined], - [missing, "pending", supported ? true : undefined], - ], + supported + ? [ + ["ci/optional-lint", "failure", undefined], + ["ci/required-build", "success", true], + [missing, "pending", true], + ] + : [ + ["ci/required-build", "success", undefined], + ["ci/optional-lint", "success", undefined], + [missing, "pending", undefined], + ], ); if (!supported) { assert.strictEqual( @@ -298,8 +304,8 @@ it.effect.each([ assert.deepStrictEqual( checks.map((check) => [check.name, check.status, check.required]), [ - ["ci/required-build", "success", undefined], ["ci/optional-lint", "failure", undefined], + ["ci/required-build", "success", undefined], ], ); }