From e3b3b1b75746781849770547028f90dfa865232d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 03:30:05 +0000 Subject: [PATCH 1/5] test: reproduce #32277 missed check failures A rerun that fails again between two PR watch reads never wakes the agent, and on GitHub a failed push run of a check is hidden by a running or passing pull_request run of the same check. These tests fail on main. Co-Authored-By: Claude Opus 5.5 --- .../orchestration-v2/pullRequestWatch.test.ts | 16 ++++++++ .../src/server/gitHubPullRequestJson.test.ts | 39 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 6ab0191649490..570450ab6d3ff 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -97,6 +97,22 @@ describe("evaluatePullRequestWatch", () => { assert.equal(evaluatePullRequestWatch(test.next, pushed, noRemarks).changes.length, 1); }); + it("reports a rerun that fails again between two reads", () => { + const run = (job: number): PullRequestCheck => ({ + ...check("lint", "failure"), + url: `https://ci.example/lint/job/${job}`, + required: true, + }); + const first = evaluatePullRequestWatch(watch(), detail({ checks: [run(1)] }), noRemarks); + assert.deepEqual(first.changes, [{ kind: "checks-failed", failed: [run(1)] }]); + const again = evaluatePullRequestWatch(first.next, detail({ checks: [run(2)] }), noRemarks); + assert.deepEqual(again.changes, [{ kind: "checks-failed", failed: [run(2)] }]); + assert.deepEqual( + evaluatePullRequestWatch(again.next, detail({ checks: [run(2)] }), noRemarks).changes, + [], + ); + }); + it("reports passed once the required checks pass, whatever the others do", () => { const required = (name: string, status: PullRequestCheck["status"]) => ({ ...check(name, status), diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index f06c847471d0e..c9982192c4e86 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -581,6 +581,45 @@ describe("pull request detail decoding", () => { } }); + it("keeps a failed push run failing while the pull_request run of it runs or passes", () => { + const run = (event: string, fields: Record) => + check({ + name: "lint", + checkSuite: { workflowRun: { event, workflow: { name: "CI" } } }, + ...fields, + }); + const pushFailed = run("push", { + status: "COMPLETED", + conclusion: "FAILURE", + startedAt: "2026-10-08T04:06:10Z", + completedAt: "2026-10-08T04:08:00Z", + }); + const pullRequestRunning = run("pull_request", { + status: "IN_PROGRESS", + conclusion: null, + startedAt: "2026-10-08T04:06:13Z", + completedAt: "0001-01-01T00:00:00Z", + }); + const pullRequestPassed = run("pull_request", { + status: "COMPLETED", + conclusion: "SUCCESS", + startedAt: "2026-10-08T04:06:13Z", + completedAt: "2026-10-08T04:12:00Z", + }); + const checksOf = (rollup: ReadonlyArray>) => { + const detail = detailOf(rollup); + return [detail.checks.map((check) => [check.name, check.status]), detail.checksState]; + }; + + const rollups = [ + [pushFailed, pullRequestRunning], + [pullRequestRunning, pushFailed], + [pushFailed, pullRequestPassed], + [pullRequestPassed, pushFailed], + ]; + expect(rollups.map(checksOf)).toEqual(rollups.map(() => [[["lint", "failure"]], "failing"])); + }); + it("merges reviews with comments in time order and keeps a bodyless approval", () => { const detail = expectSuccess(activity(JSON.parse(detailJson) as Record)); expect(detail.comments.map((comment) => comment.id)).toEqual(["r1", "c1", "r2"]); From 3c3cd500a3e2f35e4fcfe39d5c5eda4f2a3eac30 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 03:35:31 +0000 Subject: [PATCH 2/5] fix(server): report check failures a rerun or a push run would hide A PR watch remembered failing checks by name only, so a check rerun that failed again between two reads never woke the agent. The watch now also remembers the failing runs' URLs, and a failing check with a new run URL is news. Watches saved before the field still compare by name. On GitHub, runs of one check for push and pull_request were deduped by newest timestamp, so a failed push run read as pending while the pull_request run ran and was lost when that run passed. The check read now selects the workflow run's event: within one event the newest run still wins, and across events the worse run is reported, with a run cancelled beside a passing one not counted as a failure. Hosts that do not report an event dedupe as before. Fixes pingdotgg/t3code#32277 Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/Orchestrator.ts | 3 ++ .../PullRequestWatchReactor.ts | 1 + .../orchestration-v2/pullRequestWatch.test.ts | 8 ++++ .../src/orchestration-v2/pullRequestWatch.ts | 11 +++++- packages/contracts/src/threadPullRequest.ts | 2 + .../src/server/pullRequestChecks.ts | 38 +++++++++++++++++-- .../src/server/GitHubPullRequestApi.test.ts | 8 +++- .../src/server/gitHubPullRequestJson.test.ts | 25 +++++++++++- .../src/server/gitHubPullRequestJson.ts | 12 +++++- .../src/server/pullRequestReads.graphql | 1 + .../src/server/pullRequestReads.graphql.ts | 6 ++- 11 files changed, 103 insertions(+), 12 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 5d3c4391b1fe8..418f6944d3337 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4054,6 +4054,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio passedChecks: existing.endedWatch.passedChecks, conflicting: existing.endedWatch.conflicting, }), + ...(existing.endedWatch?.failedCheckUrls !== undefined && { + failedCheckUrls: existing.endedWatch.failedCheckUrls, + }), ...(existing.endedWatch?.owedChecks !== undefined && { owedChecks: existing.endedWatch.owedChecks, }), diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 369bead0ec428..ff1cd4ec51067 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -175,6 +175,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.startedAt === right.startedAt && left.headSha === right.headSha && left.failedChecks.join("\n") === right.failedChecks.join("\n") && + left.failedCheckUrls?.join("\n") === right.failedCheckUrls?.join("\n") && left.passed === right.passed && left.passedChecks.join("\n") === right.passedChecks.join("\n") && (left.owedChecks ?? []).join("\n") === (right.owedChecks ?? []).join("\n") && diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 570450ab6d3ff..7009ff76b797c 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -113,6 +113,14 @@ describe("evaluatePullRequestWatch", () => { ); }); + 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")] }); + const report = evaluatePullRequestWatch(older, failed, noRemarks); + assert.deepEqual(report.changes, []); + assert.deepEqual(report.next.failedCheckUrls, ["https://ci.example/lint"]); + }); + it("reports passed once the required checks pass, whatever the others do", () => { const required = (name: string, status: PullRequestCheck["status"]) => ({ ...check(name, status), diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index f4c0000847236..8248866d115a9 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -47,14 +47,22 @@ export function evaluatePullRequestWatch( // Hosts can return no checks on read failure; preserve prior state. let failedChecks = headMoved ? [] : watch.failedChecks; + let failedCheckUrls = headMoved ? [] : watch.failedCheckUrls; let passed = headMoved ? false : watch.passed; let passedChecks = headMoved ? [] : watch.passedChecks; let owedChecks = headMoved ? [] : (watch.owedChecks ?? []); if (detail.checks.length > 0) { const failed = detail.checks.filter(isFailedCheck); - const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); + // A new run URL under a failing name is a rerun that failed again between reads. + const toldUrls = failedCheckUrls; + const newlyFailed = failed.filter( + (check) => + !failedChecks.includes(check.name) || + (toldUrls !== undefined && check.url !== null && !toldUrls.includes(check.url)), + ); if (newlyFailed.length > 0) changes.push({ kind: "checks-failed", failed: newlyFailed }); failedChecks = failed.map((check) => check.name); + failedCheckUrls = failed.flatMap((check) => (check.url === null ? [] : [check.url])); const required = detail.checks.filter((check) => check.required === true); const gate = required.length > 0 ? required : detail.checks; @@ -145,6 +153,7 @@ export function evaluatePullRequestWatch( startedAt: watch.startedAt, headSha, failedChecks, + ...(failedCheckUrls !== undefined && { failedCheckUrls }), passed, passedChecks, owedChecks, diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 3daee9ce197e2..b53faf09f3897 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -83,6 +83,8 @@ export const ThreadPullRequestWatch = Schema.Struct({ startedAt: IsoDateTime, headSha: Schema.NullOr(TrimmedNonEmptyString), failedChecks: Schema.Array(TrimmedNonEmptyString), + /** Run URLs of the failing checks, so a rerun that fails again is news; absent on older watches. */ + failedCheckUrls: Schema.optional(Schema.Array(Schema.String)), passed: Schema.Boolean, passedChecks: Schema.Array(TrimmedNonEmptyString).pipe( Schema.withDecodingDefault(Effect.succeed([])), diff --git a/packages/source-control-core/src/server/pullRequestChecks.ts b/packages/source-control-core/src/server/pullRequestChecks.ts index 18b34d39baea5..447b412ad8bd4 100644 --- a/packages/source-control-core/src/server/pullRequestChecks.ts +++ b/packages/source-control-core/src/server/pullRequestChecks.ts @@ -46,9 +46,18 @@ export function sortChecks( ); } +// Runs of one check for different events: a failure outranks a run in progress, which outranks a +// pass, which outranks a run cancelled beside it (a concurrency group superseding it). +function eventSeverity(status: PullRequestCheck["status"]): number { + if (status === "failure" || status === "action-required") return 3; + if (status === "pending") return 2; + return status === "success" ? 1 : 0; +} + /** * Rollups repeat checks during reruns or overlapping runs, and checks carry no ids. * Identify checks by workflow plus name because different workflows may share job names. + * Within one event the newest run wins; across events the worse run does. */ export function dedupeChecks( entries: ReadonlyArray<{ @@ -57,12 +66,15 @@ export function dedupeChecks( readonly at: string | null; // Hosts with one-second timestamps can date a final and an earlier status alike. readonly sequence?: number; + /** What triggered the run, where the host says (GitHub's `push` or `pull_request`). */ + readonly event?: string | null; }>, ): ReadonlyArray { - const newestByCheck = new Map(); + type Entry = (typeof entries)[number]; + const newestByRun = new Map(); for (const entry of entries) { - const key = JSON.stringify([entry.workflowName ?? "", entry.check.name]); - const kept = newestByCheck.get(key); + const key = JSON.stringify([entry.workflowName ?? "", entry.check.name, entry.event ?? null]); + const kept = newestByRun.get(key); if (kept !== undefined) { if (!isAtLeastAsNew(entry.at, kept.at)) continue; if ( @@ -73,7 +85,25 @@ export function dedupeChecks( ) continue; } - newestByCheck.set(key, entry); + newestByRun.set(key, entry); + } + const newestByCheck = new Map(); + for (const entry of newestByRun.values()) { + const key = JSON.stringify([entry.workflowName ?? "", entry.check.name]); + const kept = newestByCheck.get(key); + if (kept === undefined) { + newestByCheck.set(key, entry); + continue; + } + const severity = eventSeverity(entry.check.status) - eventSeverity(kept.check.status); + const worse = severity > 0 || (severity === 0 && isAtLeastAsNew(entry.at, kept.at)); + const chosen = worse ? entry : kept; + newestByCheck.set( + key, + entry.check.required === true || kept.check.required === true + ? { ...chosen, check: { ...chosen.check, required: true } } + : chosen, + ); } const survivors = [...newestByCheck.values()]; const countsByName = new Map(); diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index f46c91fd69570..372070c33b896 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -3517,13 +3517,17 @@ layer("GitHubPullRequestApi.layer", (it) => { name: "build", status: "COMPLETED", conclusion: "SUCCESS", - checkSuite: { workflowRun: { workflow: { name: "linux" } } }, + checkSuite: { + workflowRun: { event: "push", workflow: { name: "linux" } }, + }, }), checkRun({ name: "build", status: "COMPLETED", conclusion: "FAILURE", - checkSuite: { workflowRun: { workflow: { name: "windows" } } }, + checkSuite: { + workflowRun: { event: "push", workflow: { name: "windows" } }, + }, }), ], pageInfo: { hasNextPage: false, endCursor: null }, diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index c9982192c4e86..f2792178166b0 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -328,7 +328,7 @@ describe("pull request detail decoding", () => { const inWorkflow = ({ workflowName, ...fields }: Record) => check({ ...fields, - checkSuite: { workflowRun: { workflow: { name: workflowName } } }, + checkSuite: { workflowRun: { event: "pull_request", workflow: { name: workflowName } } }, }); const coreJson = ( @@ -620,6 +620,29 @@ describe("pull request detail decoding", () => { expect(rollups.map(checksOf)).toEqual(rollups.map(() => [[["lint", "failure"]], "failing"])); }); + it("does not fail a check whose run of one event was cancelled beside a passing run", () => { + const run = (event: string, conclusion: string, completedAt: string) => + check({ + name: "lint", + conclusion, + completedAt, + checkSuite: { workflowRun: { event, workflow: { name: "CI" } } }, + }); + const passed = run("push", "SUCCESS", "2026-10-08T04:10:00Z"); + const cancelled = run("pull_request", "CANCELLED", "2026-10-08T04:12:00Z"); + + for (const rollup of [ + [passed, cancelled], + [cancelled, passed], + ]) { + const detail = detailOf(rollup); + expect([detail.checks.map((check) => check.status), detail.checksState]).toEqual([ + ["success"], + "passing", + ]); + } + }); + it("merges reviews with comments in time order and keeps a bodyless approval", () => { const detail = expectSuccess(activity(JSON.parse(detailJson) as Record)); expect(detail.comments.map((comment) => comment.id)).toEqual(["r1", "c1", "r2"]); diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.ts index a4c509e944488..22965337da5dd 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.ts @@ -137,6 +137,8 @@ interface CheckFields { readonly targetUrl?: string | null | undefined; /** Empty for app-owned runs; absent for commit statuses. */ readonly workflowName?: string | null | undefined; + /** Absent for commit statuses and `gh` JSON; null for app-owned runs. */ + readonly workflowRun?: { readonly event: string } | null | undefined; readonly startedAt?: string | null | undefined; readonly completedAt?: string | null | undefined; /** Branch protection requires this check; asked for on github.com only. */ @@ -600,11 +602,12 @@ function isNamelessCheck(raw: CheckFields): boolean { return trimmed(raw.name) === null && trimmed(raw.context) === null; } -/** Unfinished runs outrank finished runs of the same check across push/PR workflows. */ +/** Unfinished runs outrank finished runs of the same check, so a rerun replaces its run. */ function toCheckEntries(raw: ReadonlyArray | null | undefined): ReadonlyArray<{ readonly check: PullRequestCheck; readonly workflowName: string | null; readonly at: string | null; + readonly event: string | null; }> { return (raw ?? []).flatMap((check) => { const name = trimmed(check.name) ?? trimmed(check.context); @@ -620,6 +623,7 @@ function toCheckEntries(raw: ReadonlyArray | null | undefined): Rea ...(typeof check.isRequired === "boolean" ? { required: check.isRequired } : {}), }, workflowName: trimmed(check.workflowName), + event: trimmed(check.workflowRun?.event), at: status === "pending" ? STILL_RUNNING @@ -1185,7 +1189,11 @@ export type GitHubCheckContext = CheckFields; function toCheckContexts(nodes: ReadonlyArray): ReadonlyArray { return nodes.map((check) => check.__typename === "CheckRun" - ? { ...check, workflowName: check.checkSuite.workflowRun?.workflow.name ?? null } + ? { + ...check, + workflowName: check.checkSuite.workflowRun?.workflow.name ?? null, + workflowRun: check.checkSuite.workflowRun, + } : check, ); } diff --git a/packages/source-control-github/src/server/pullRequestReads.graphql b/packages/source-control-github/src/server/pullRequestReads.graphql index eb728b1ee062c..88fa162437122 100644 --- a/packages/source-control-github/src/server/pullRequestReads.graphql +++ b/packages/source-control-github/src/server/pullRequestReads.graphql @@ -42,6 +42,7 @@ fragment CheckContext on StatusCheckRollupContext { isRequired(pullRequestNumber: $number) @include(if: $required) checkSuite { workflowRun { + event workflow { name } diff --git a/packages/source-control-github/src/server/pullRequestReads.graphql.ts b/packages/source-control-github/src/server/pullRequestReads.graphql.ts index 90ee834c3181e..8bee4e809ae1e 100644 --- a/packages/source-control-github/src/server/pullRequestReads.graphql.ts +++ b/packages/source-control-github/src/server/pullRequestReads.graphql.ts @@ -30,6 +30,8 @@ export interface Workflow extends Schema.Struct.Type {} /** A workflow run. */ export const WorkflowRun = Schema.Struct({ + /** The event that triggered the workflow run */ + event: Schema.String, /** The workflow executed in this workflow run. */ workflow: Workflow }) @@ -506,7 +508,7 @@ export const PullRequestCore = GraphQL.query("PullRequestCore", { { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){mergeCommitAllowed squashMergeAllowed rebaseMergeAllowed autoMergeAllowed viewerPermission pullRequest(number:$", "number", "){number title url body state isDraft mergeable mergeStateStatus reviewDecision additions deletions changedFiles createdAt updatedAt mergedAt closedAt headRefName baseRefName headRefOid isCrossRepository maintainerCanModify headRepositoryOwner{__typename login}headRepository{nameWithOwner viewerPermission}headRef{id}author{__typename login avatarUrl...on User{id name}}autoMergeRequest{mergeMethod}viewerCanUpdate viewerDidAuthor viewerCanUpdateBranch baseRef{compare(headRef:$", "headRef", "){behindBy}}reviewRequests(first:$", "requestedReviewers", "){nodes{requestedReviewer{__typename...on User{login name}...on Bot{login}...on Team{slug combinedSlug teamName:name teamAvatarUrl:avatarUrl}}}}labels(first:100){nodes{name color}}commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage endCursor}}}}}}}}"] } ], fragments: [ - { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } + { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{event workflow{name}}}}}"] } ] }, variables: { @@ -557,7 +559,7 @@ export const PullRequestCheckContexts = GraphQL.query("PullRequestCheckContexts" { name: "repository", body: ["(owner:$", "owner", " name:$", "name", "){pullRequest(number:$", "number", "){headRefOid commits(last:1){nodes{commit{statusCheckRollup{contexts(first:100 after:$", "after", "){nodes{__typename...", "CheckContext", "}pageInfo{hasNextPage endCursor}}}}}}}}"] } ], fragments: [ - { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{workflow{name}}}}}"] } + { name: "CheckContext", body: ["on StatusCheckRollupContext{__typename...on StatusContext{context state targetUrl createdAt description isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")}...on CheckRun{name status conclusion startedAt completedAt detailsUrl isRequired(pullRequestNumber:$", "number", ")@include(if:$", "required", ")checkSuite{workflowRun{event workflow{name}}}}}"] } ] }, variables: { From ec13a3263184483556a0dfb4d312765102a554d3 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 03:46:27 +0000 Subject: [PATCH 3/5] fix(server): break cross-event check ties by event so a watch's run cannot flip Co-Authored-By: Claude Opus 5.5 --- .../src/server/pullRequestChecks.test.ts | 15 +++++++++++++++ .../src/server/pullRequestChecks.ts | 6 +++++- .../src/server/gitHubPullRequestJson.test.ts | 12 ++++++++++-- 3 files changed, 30 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 4f6538f833f9a..f083735804653 100644 --- a/packages/source-control-core/src/server/pullRequestChecks.test.ts +++ b/packages/source-control-core/src/server/pullRequestChecks.test.ts @@ -59,6 +59,21 @@ describe("dedupeChecks", () => { expect([forward[0]?.status, reversed[0]?.status]).toEqual(["failure", "failure"]); }); + it("reports the same run for two events' failures finished at once, whichever is listed first", () => { + const failed = (event: string) => ({ + ...entry("lint", "failure", { at: "2026-11-07T00:01:00Z" }), + check: { name: "lint", status: "failure" as const, description: null, url: event }, + event, + }); + + expect( + [ + [failed("push"), failed("pull_request")], + [failed("pull_request"), failed("push")], + ].map((runs) => dedupeChecks(runs)[0]?.url), + ).toEqual(["push", "push"]); + }); + it("takes the higher sequence when instants tie, whichever is listed first", () => { const final = entry("build", "success", { at: "2026-10-12T10:14:29Z", sequence: 15 }); const pending = entry("build", "pending", { at: "2026-10-12T10:14:29Z", sequence: 14 }); diff --git a/packages/source-control-core/src/server/pullRequestChecks.ts b/packages/source-control-core/src/server/pullRequestChecks.ts index 447b412ad8bd4..c9c388f57280f 100644 --- a/packages/source-control-core/src/server/pullRequestChecks.ts +++ b/packages/source-control-core/src/server/pullRequestChecks.ts @@ -96,7 +96,11 @@ export function dedupeChecks( continue; } const severity = eventSeverity(entry.check.status) - eventSeverity(kept.check.status); - const worse = severity > 0 || (severity === 0 && isAtLeastAsNew(entry.at, kept.at)); + const newer = isAtLeastAsNew(entry.at, kept.at); + // Equal times break on the event, so a reordered rollup cannot flip the run a watch was told. + const tied = newer && isAtLeastAsNew(kept.at, entry.at); + const later = tied ? (entry.event ?? "") > (kept.event ?? "") : newer; + const worse = severity > 0 || (severity === 0 && later); const chosen = worse ? entry : kept; newestByCheck.set( key, diff --git a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts index f2792178166b0..ca552911543ba 100644 --- a/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts +++ b/packages/source-control-github/src/server/gitHubPullRequestJson.test.ts @@ -594,13 +594,16 @@ describe("pull request detail decoding", () => { startedAt: "2026-10-08T04:06:10Z", completedAt: "2026-10-08T04:08:00Z", }); + // Only the losing run says the check is required; the merged check must still say so. const pullRequestRunning = run("pull_request", { + isRequired: true, status: "IN_PROGRESS", conclusion: null, startedAt: "2026-10-08T04:06:13Z", completedAt: "0001-01-01T00:00:00Z", }); const pullRequestPassed = run("pull_request", { + isRequired: true, status: "COMPLETED", conclusion: "SUCCESS", startedAt: "2026-10-08T04:06:13Z", @@ -608,7 +611,10 @@ describe("pull request detail decoding", () => { }); const checksOf = (rollup: ReadonlyArray>) => { const detail = detailOf(rollup); - return [detail.checks.map((check) => [check.name, check.status]), detail.checksState]; + return [ + detail.checks.map((check) => [check.name, check.status, check.required]), + detail.checksState, + ]; }; const rollups = [ @@ -617,7 +623,9 @@ describe("pull request detail decoding", () => { [pushFailed, pullRequestPassed], [pullRequestPassed, pushFailed], ]; - expect(rollups.map(checksOf)).toEqual(rollups.map(() => [[["lint", "failure"]], "failing"])); + expect(rollups.map(checksOf)).toEqual( + rollups.map(() => [[["lint", "failure", true]], "failing"]), + ); }); it("does not fail a check whose run of one event was cancelled beside a passing run", () => { From 9a03aa72a785c8f3332f332012d524f9948d01c8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 03:53:44 +0000 Subject: [PATCH 4/5] fix(server): a watch keeps the failed runs it told for the head Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/pullRequestWatch.test.ts | 11 +++++++---- apps/server/src/orchestration-v2/pullRequestWatch.ts | 8 +++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 7009ff76b797c..ecc95fc564aea 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -107,10 +107,13 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(first.changes, [{ kind: "checks-failed", failed: [run(1)] }]); const again = evaluatePullRequestWatch(first.next, detail({ checks: [run(2)] }), noRemarks); assert.deepEqual(again.changes, [{ kind: "checks-failed", failed: [run(2)] }]); - assert.deepEqual( - evaluatePullRequestWatch(again.next, detail({ checks: [run(2)] }), noRemarks).changes, - [], - ); + // Neither the same run nor one told earlier on this head is news again. + for (const job of [2, 1]) { + assert.deepEqual( + evaluatePullRequestWatch(again.next, detail({ checks: [run(job)] }), noRemarks).changes, + [], + ); + } }); it("does not repeat a failure an older watch told by name", () => { diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 8248866d115a9..bc1cfa45738b7 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -62,7 +62,13 @@ export function evaluatePullRequestWatch( ); if (newlyFailed.length > 0) changes.push({ kind: "checks-failed", failed: newlyFailed }); failedChecks = failed.map((check) => check.name); - failedCheckUrls = failed.flatMap((check) => (check.url === null ? [] : [check.url])); + // Runs told stay told for the head: GitHub can show either event's failed run of one check. + failedCheckUrls = [ + ...new Set([ + ...(toldUrls ?? []), + ...failed.flatMap((check) => (check.url === null ? [] : [check.url])), + ]), + ]; const required = detail.checks.filter((check) => check.required === true); const gate = required.length > 0 ? required : detail.checks; From 3c0110de88fc63f5c5ac57631900a4da579d0be8 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 7 Nov 2026 03:56:07 +0000 Subject: [PATCH 5/5] fix(server): a watch tells failed runs apart by check name and URL Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/Orchestrator.ts | 4 +-- .../PullRequestWatchReactor.ts | 2 +- .../orchestration-v2/pullRequestWatch.test.ts | 13 +++++++++- .../src/orchestration-v2/pullRequestWatch.ts | 25 ++++++++++--------- packages/contracts/src/threadPullRequest.ts | 4 +-- 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 418f6944d3337..3b8541d22480b 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4054,8 +4054,8 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio passedChecks: existing.endedWatch.passedChecks, conflicting: existing.endedWatch.conflicting, }), - ...(existing.endedWatch?.failedCheckUrls !== undefined && { - failedCheckUrls: existing.endedWatch.failedCheckUrls, + ...(existing.endedWatch?.failedRuns !== undefined && { + failedRuns: existing.endedWatch.failedRuns, }), ...(existing.endedWatch?.owedChecks !== undefined && { owedChecks: existing.endedWatch.owedChecks, diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index ff1cd4ec51067..fd937f3f27e5f 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -175,7 +175,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.startedAt === right.startedAt && left.headSha === right.headSha && left.failedChecks.join("\n") === right.failedChecks.join("\n") && - left.failedCheckUrls?.join("\n") === right.failedCheckUrls?.join("\n") && + left.failedRuns?.join("\n") === right.failedRuns?.join("\n") && left.passed === right.passed && left.passedChecks.join("\n") === right.passedChecks.join("\n") && (left.owedChecks ?? []).join("\n") === (right.owedChecks ?? []).join("\n") && diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index ecc95fc564aea..7884fcf18433e 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -107,6 +107,17 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(first.changes, [{ kind: "checks-failed", failed: [run(1)] }]); const again = evaluatePullRequestWatch(first.next, detail({ checks: [run(2)] }), noRemarks); assert.deepEqual(again.changes, [{ kind: "checks-failed", failed: [run(2)] }]); + // A status can link a build shared with another check; it is that check's run, not lint's. + const shared = { ...check("test", "failure"), url: "https://ci.example/lint/job/3" }; + const both = evaluatePullRequestWatch( + again.next, + detail({ checks: [run(2), shared] }), + noRemarks, + ); + assert.deepEqual( + evaluatePullRequestWatch(both.next, detail({ checks: [run(3), shared] }), noRemarks).changes, + [{ kind: "checks-failed", failed: [run(3)] }], + ); // Neither the same run nor one told earlier on this head is news again. for (const job of [2, 1]) { assert.deepEqual( @@ -121,7 +132,7 @@ describe("evaluatePullRequestWatch", () => { const failed = detail({ checks: [check("lint", "failure")] }); const report = evaluatePullRequestWatch(older, failed, noRemarks); assert.deepEqual(report.changes, []); - assert.deepEqual(report.next.failedCheckUrls, ["https://ci.example/lint"]); + assert.deepEqual(report.next.failedRuns, [JSON.stringify(["lint", "https://ci.example/lint"])]); }); it("reports passed once the required checks pass, whatever the others do", () => { diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index bc1cfa45738b7..34e0e82b4285f 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -47,27 +47,28 @@ export function evaluatePullRequestWatch( // Hosts can return no checks on read failure; preserve prior state. let failedChecks = headMoved ? [] : watch.failedChecks; - let failedCheckUrls = headMoved ? [] : watch.failedCheckUrls; + let failedRuns = headMoved ? [] : watch.failedRuns; let passed = headMoved ? false : watch.passed; let passedChecks = headMoved ? [] : watch.passedChecks; let owedChecks = headMoved ? [] : (watch.owedChecks ?? []); if (detail.checks.length > 0) { const failed = detail.checks.filter(isFailedCheck); // A new run URL under a failing name is a rerun that failed again between reads. - const toldUrls = failedCheckUrls; - const newlyFailed = failed.filter( - (check) => + const toldRuns = failedRuns; + const runOf = (check: PullRequestCheck) => + check.url === null ? null : JSON.stringify([check.name, check.url]); + const newlyFailed = failed.filter((check) => { + const run = runOf(check); + return ( !failedChecks.includes(check.name) || - (toldUrls !== undefined && check.url !== null && !toldUrls.includes(check.url)), - ); + (toldRuns !== undefined && run !== null && !toldRuns.includes(run)) + ); + }); if (newlyFailed.length > 0) changes.push({ kind: "checks-failed", failed: newlyFailed }); failedChecks = failed.map((check) => check.name); // Runs told stay told for the head: GitHub can show either event's failed run of one check. - failedCheckUrls = [ - ...new Set([ - ...(toldUrls ?? []), - ...failed.flatMap((check) => (check.url === null ? [] : [check.url])), - ]), + failedRuns = [ + ...new Set([...(toldRuns ?? []), ...failed.flatMap((check) => runOf(check) ?? [])]), ]; const required = detail.checks.filter((check) => check.required === true); @@ -159,7 +160,7 @@ export function evaluatePullRequestWatch( startedAt: watch.startedAt, headSha, failedChecks, - ...(failedCheckUrls !== undefined && { failedCheckUrls }), + ...(failedRuns !== undefined && { failedRuns }), passed, passedChecks, owedChecks, diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index b53faf09f3897..a80dbb0aa197c 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -83,8 +83,8 @@ export const ThreadPullRequestWatch = Schema.Struct({ startedAt: IsoDateTime, headSha: Schema.NullOr(TrimmedNonEmptyString), failedChecks: Schema.Array(TrimmedNonEmptyString), - /** Run URLs of the failing checks, so a rerun that fails again is news; absent on older watches. */ - failedCheckUrls: Schema.optional(Schema.Array(Schema.String)), + /** Name and run URL of each failed run told on this head, so a rerun that fails again is news. */ + failedRuns: Schema.optional(Schema.Array(Schema.String)), passed: Schema.Boolean, passedChecks: Schema.Array(TrimmedNonEmptyString).pipe( Schema.withDecodingDefault(Effect.succeed([])),