From 8b2e3125b3b5312ac239e529cdc5543c606faa94 Mon Sep 17 00:00:00 2001 From: royalaid <2439803+royalaid@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:53:05 -0800 Subject: [PATCH] fix(gitlab): a watch reports a pipeline retry that fails again GitLab reports a merge request's head pipeline as one check whose URL is the pipeline's. Retrying failed jobs reruns them inside that pipeline, so the URL never changes and the watch's name-plus-URL run key cannot tell a retry that failed again from the failure it already told. Checks gain an optional runId for hosts whose check URL survives a rerun. The watch keys failed runs on runId, falling back to the URL. GitLab sets it to the pipeline ID and its finish time, which moves each time the pipeline finishes again. Co-Authored-By: Claude Opus 5.5 --- .../orchestration-v2/pullRequestWatch.test.ts | 23 ++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 9 ++++--- packages/contracts/src/pullRequest.ts | 5 ++++ .../src/server/gitLabMergeRequestJson.test.ts | 26 +++++++++++++++++++ .../src/server/gitLabMergeRequestJson.ts | 7 +++++ 5 files changed, 67 insertions(+), 3 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 0b3939800497..12251211671d 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -266,6 +266,29 @@ describe("evaluatePullRequestWatch", () => { } }); + it("reports a retry that fails again where the run keeps its URL", () => { + // GitLab retries failed jobs inside the same pipeline, so only the run ID moves. + const pipeline = (finishedAt: string): PullRequestCheck => ({ + ...check("Pipeline", "failure"), + url: "https://gitlab.example/o/r/-/pipelines/7", + runId: `7@${finishedAt}`, + }); + const first = pipeline("2026-11-10T00:28:57.000Z"); + const again = pipeline("2026-11-10T00:39:53.000Z"); + const told = evaluatePullRequestWatch(watch(), detail({ checks: [first] }), noRemarks); + assert.deepEqual(told.changes, [{ kind: "checks-failed", failed: [first] }]); + assert.deepEqual( + evaluatePullRequestWatch(told.next, detail({ checks: [first] }), noRemarks).changes, + [], + ); + const retried = evaluatePullRequestWatch(told.next, detail({ checks: [again] }), noRemarks); + assert.deepEqual(retried.changes, [{ kind: "checks-failed", failed: [again] }]); + assert.deepEqual( + evaluatePullRequestWatch(retried.next, detail({ checks: [again] }), noRemarks).changes, + [], + ); + }); + it("does not repeat a failure an older watch told by name", () => { const older = watch({ headSha: "aaaaaaaaaa", failedChecks: ["lint"] }); const failed = detail({ checks: [check("lint", "failure")] }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index e5f87081fe37..82442f6f3662 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -63,10 +63,13 @@ export function evaluatePullRequestWatch( const failed = checks.filter(isFailedCheck); const awaiting = checks.filter((check) => check.status === "action-required"); if (checks.length > 0) { - // A new run URL under a failing name is a rerun that failed again between reads. + // A new run under a failing name is a rerun that failed again between reads. A run is its + // URL, or the host's run ID where the URL stays the same across reruns. const toldRuns = failedRuns; - const runOf = (check: PullRequestCheck) => - check.url === null ? null : JSON.stringify([check.name, check.url]); + const runOf = (check: PullRequestCheck) => { + const run = check.runId ?? check.url; + return run === null ? null : JSON.stringify([check.name, run]); + }; const newlyFailed = failed.filter((check) => { const run = runOf(check); return ( diff --git a/packages/contracts/src/pullRequest.ts b/packages/contracts/src/pullRequest.ts index f0f692da3f8d..8abba5d17946 100644 --- a/packages/contracts/src/pullRequest.ts +++ b/packages/contracts/src/pullRequest.ts @@ -114,6 +114,11 @@ export const PullRequestCheck = Schema.Struct({ * approval when that read fails, so nothing waits on it or counts it as passing. */ placeholder: Schema.optional(Schema.Boolean), + /** + * Tells this run of the check apart from earlier ones where its URL does not change on a + * rerun, such as a GitLab pipeline whose failed jobs were retried in place. + */ + runId: Schema.optional(TrimmedNonEmptyString), }); export type PullRequestCheck = typeof PullRequestCheck.Type; diff --git a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts index 181b73a44b73..122916fefa31 100644 --- a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts +++ b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.test.ts @@ -154,6 +154,32 @@ describe("decodeMergeRequestDetailJson", () => { ]); }); + it("tells each finish of a pipeline apart, since a job retry keeps its URL", () => { + const pipelineCheck = (pipeline: Record) => + expectSuccess(decodeMergeRequestDetailJson(detailJson({ head_pipeline: pipeline }))) + .checks[0]; + const failed = (finishedAt: string) => + pipelineCheck({ + id: 3047545127, + status: "failed", + web_url: "https://gitlab.com/acme/web/-/pipelines/3047545127", + finished_at: finishedAt, + }); + + const first = failed("2026-11-10T00:28:57.753Z"); + const retried = failed("2026-11-10T00:39:53.753Z"); + expect(first?.url).toBe(retried?.url); + expect(first?.runId).toBe("3047545127@2026-11-10T00:28:57.753Z"); + expect(retried?.runId).toBe("3047545127@2026-11-10T00:39:53.753Z"); + // A running pipeline has no finish time yet, and one without an ID has nothing to key on. + expect(pipelineCheck({ id: 3047545127, status: "running", finished_at: null })?.runId).toBe( + undefined, + ); + expect( + pipelineCheck({ status: "failed", finished_at: "2026-11-10T00:39:53.753Z" })?.runId, + ).toBe(undefined); + }); + it.each(["canceled", "canceling"])("maps a %s pipeline to cancelled", (status) => { const detail = expectSuccess( decodeMergeRequestDetailJson(detailJson({ head_pipeline: { status } })), diff --git a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts index 6af07442403f..6782f39903d7 100644 --- a/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts +++ b/packages/source-control-gitlab/src/server/gitLabMergeRequestJson.ts @@ -40,6 +40,7 @@ const RawPipelineSchema = Schema.Struct({ status: Schema.optional(Schema.NullOr(Schema.String)), web_url: Schema.optional(Schema.NullOr(Schema.String)), source: Schema.optional(Schema.NullOr(Schema.String)), + finished_at: Schema.optional(Schema.NullOr(Schema.String)), }); const RawTriggerJobSchema = Schema.Struct({ @@ -306,12 +307,18 @@ function toChecks( ): ReadonlyArray { const pipeline = raw.head_pipeline; if (!pipeline) return []; + // Retrying jobs reruns them inside the same pipeline, so its URL stays the same. Each time it + // finishes again, its finish time moves, and that tells a retry that failed again apart. + const finishedAt = trimmed(pipeline.finished_at); return [ { name: "Pipeline", status: toPipelineStatus(pipeline.status), description: trimmed(pipeline.source), url: trimmed(pipeline.web_url), + ...(pipeline.id == null || finishedAt === null + ? {} + : { runId: `${pipeline.id}@${finishedAt}` }), }, ]; }