From 0db3b6b776bb73003ca917ae0f80da5df12562bf Mon Sep 17 00:00:00 2001 From: Alex Vallone Date: Mon, 9 Nov 2026 11:54:46 -0800 Subject: [PATCH] fix(azure-devops): omit links to nonexistent build runs Treat zero and other invalid build IDs as absent before creating results links and identifying manual builds that have not run. Preserve pending automatic policies and valid build URLs. Extend the existing checks regression with absent and invalid IDs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../server/azureDevOpsPullRequestJson.test.ts | 61 ++++++++++++------- .../src/server/azureDevOpsPullRequestJson.ts | 6 +- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts index defc475722376..49f9549c8aafe 100644 --- a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts +++ b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.test.ts @@ -847,26 +847,43 @@ describe("toPullRequestChecks", () => { ]); }); - it("reads a manual-only build nobody queued as skipped rather than pending forever", () => { - const checks = checksFor([ - policy({ - status: "queued", - settings: { manualQueueOnly: true, displayName: "Optional perf" }, - }), - policy({ - status: "queued", - settings: { manualQueueOnly: true, displayName: "Queued perf" }, - context: { buildId: 9 }, - }), - policy({ status: "notApplicable", settings: { displayName: "Docs" } }), - policy({ status: "broken", settings: { displayName: "Broken" } }), - ]); - - expect(checks.map((check) => [check.name, check.status, check.description])).toEqual([ - ["Broken", "failure", null], - ["Docs", "skipped", null], - ["Optional perf", "skipped", "Not run"], - ["Queued perf", "pending", null], - ]); - }); + it.each([undefined, null, 0, -1, 1.5])( + "reads a manual-only build with no valid run (%s) as skipped rather than pending forever", + (buildId) => { + const checks = checksFor([ + policy({ + status: "queued", + settings: { displayName: "Awaiting run" }, + context: { buildId: 0 }, + }), + policy({ + status: "queued", + settings: { manualQueueOnly: true, displayName: "Optional perf" }, + context: { buildId }, + }), + policy({ + status: "queued", + settings: { manualQueueOnly: true, displayName: "Queued perf" }, + context: { buildId: 9 }, + }), + policy({ status: "notApplicable", settings: { displayName: "Docs" } }), + policy({ status: "broken", settings: { displayName: "Broken" } }), + ]); + + expect( + checks.map((check) => [check.name, check.status, check.description, check.url]), + ).toEqual([ + ["Awaiting run", "pending", null, null], + ["Broken", "failure", null, null], + ["Docs", "skipped", null, null], + ["Optional perf", "skipped", "Not run", null], + [ + "Queued perf", + "pending", + null, + "https://dev.azure.com/acme/6f9c9b7f-0000-0000-0000-000000000000/_build/results?buildId=9", + ], + ]); + }, + ); }); diff --git a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts index 86f05c8fb9346..a82e309ee9e21 100644 --- a/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts +++ b/packages/source-control-azure-devops/src/server/azureDevOpsPullRequestJson.ts @@ -550,7 +550,11 @@ export function toPullRequestChecks( trimmed(context?.buildDefinitionName) ?? trimmed(configuration.type?.displayName); if (name === null) continue; - const buildId = context?.buildId ?? null; + const reportedBuildId = context?.buildId; + const buildId = + reportedBuildId != null && Number.isSafeInteger(reportedBuildId) && reportedBuildId > 0 + ? reportedBuildId + : null; const neverQueued = settings?.manualQueueOnly === true && buildId === null; // An expired build no longer satisfies its policy, and Azure counts it as failed. const expired = context?.isExpired === true;