From e7c27ecaa14d2dd171d7e025109044b5c24f0c1d Mon Sep 17 00:00:00 2001 From: Ryan McDonough Date: Sat, 31 Oct 2026 00:16:41 -0700 Subject: [PATCH 1/8] fix(server): PR watches wake when checks finish after an early failure With no required checks, the watch only sends checks-passed once every check passes. A check that fails early, such as a cancelled advisory analysis, woke the agent while the rest were still running, and nothing woke it again when they finished. The gate checks still running at a failure are now owed one checks-finished wake. Co-Authored-By: Claude Opus 5.5 --- .../orchestration-v2/pullRequestWatch.test.ts | 24 ++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 28 ++++++++++++++++++- packages/contracts/src/threadPullRequest.ts | 2 +- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 6ab0191649490..2163e2c8ab927 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -205,6 +205,30 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(evaluatePullRequestWatch(final.next, recovered, noRemarks).changes, []); }); + it("wakes once when the other checks finish after one failed where none is required", () => { + const sonar = check("sonar", "cancelled"); + const early = detail({ checks: [sonar, check("unit", "pending"), check("e2e", "pending")] }); + const first = evaluatePullRequestWatch(watch(), early, noRemarks); + assert.deepEqual(first.changes, [{ kind: "checks-failed", failed: [sonar] }]); + + const partway = detail({ checks: [sonar, check("unit", "success"), check("e2e", "pending")] }); + const waiting = evaluatePullRequestWatch(first.next, partway, noRemarks); + assert.deepEqual(waiting.changes, []); + + const done = detail({ checks: [sonar, check("unit", "success"), check("e2e", "success")] }); + const final = evaluatePullRequestWatch(waiting.next, done, noRemarks); + assert.deepEqual(final.changes, [{ kind: "checks-finished", failing: [sonar] }]); + assert.deepEqual(evaluatePullRequestWatch(final.next, done, noRemarks).changes, []); + + // A later failure is its own wake and leaves nothing more owed. + const broke = detail({ checks: [sonar, check("unit", "success"), check("e2e", "failure")] }); + const failure = evaluatePullRequestWatch(waiting.next, broke, noRemarks); + assert.deepEqual(failure.changes, [ + { kind: "checks-failed", failed: [check("e2e", "failure")] }, + ]); + assert.deepEqual(evaluatePullRequestWatch(failure.next, broke, noRemarks).changes, []); + }); + it("clears old-head owed checks when a new head is pushed", () => { const required = { ...check("test", "success"), required: true }; const early = detail({ checks: [required, check("e2e", "pending")] }); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index f4c0000847236..51cbe6356d7a1 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -17,6 +17,7 @@ const SNIPPET_LENGTH = 200; export type PullRequestWatchChange = | { readonly kind: "checks-failed"; readonly failed: ReadonlyArray } | { readonly kind: "checks-passed"; readonly count: number; readonly required: boolean } + | { readonly kind: "checks-finished"; readonly failing: ReadonlyArray } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "no-checks"; readonly minutes: number } | { readonly kind: "conflicting" }; @@ -51,6 +52,7 @@ export function evaluatePullRequestWatch( let passedChecks = headMoved ? [] : watch.passedChecks; let owedChecks = headMoved ? [] : (watch.owedChecks ?? []); if (detail.checks.length > 0) { + const owedBefore = owedChecks; const failed = detail.checks.filter(isFailedCheck); const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); if (newlyFailed.length > 0) changes.push({ kind: "checks-failed", failed: newlyFailed }); @@ -85,6 +87,18 @@ export function evaluatePullRequestWatch( } else if (settled && owed.length > 0) { changes.push({ kind: "checks-passed", count: detail.checks.length, required: false }); owedChecks = []; + } else if (!passedNow && failed.length > 0) { + // Gate checks still running when one failed are owed one wake when they finish, since + // the gate can no longer pass and so never sends the checks-passed wake. + const running = gate.filter((check) => check.status === "pending").map((check) => check.name); + owedChecks = + newlyFailed.length > 0 ? running : owedBefore.filter((name) => running.includes(name)); + if (newlyFailed.length === 0 && owedBefore.length > 0 && owedChecks.length === 0) { + changes.push({ + kind: "checks-finished", + failing: failed.filter((check) => gateNames.includes(check.name)), + }); + } } passed = passedNow; passedChecks = passedNow ? gateNames : []; @@ -204,6 +218,14 @@ function changeLines( return ` - ${remark.author?.login ?? "someone"}${where}: ${said}${remark.url ? ` ${remark.url}` : ""}`; }), ]; + case "checks-finished": + return [ + `- The checks still running when one failed have finished${context.commit}. Still failing:`, + ...listed( + change.failing, + (check) => ` - ${check.name}${check.url ? ` ${check.url}` : ""}`, + ), + ]; case "no-checks": return [ `- No checks have reported${context.commit} after ${change.minutes} minutes. None may be configured to run on this change.`, @@ -216,6 +238,7 @@ function changeLines( const SUMMARY: Record = { "checks-failed": "checks failed", "checks-passed": "checks passed", + "checks-finished": "checks finished", remarks: "new comments", "no-checks": "no checks reported", conflicting: "merge conflict", @@ -242,7 +265,10 @@ export function pullRequestWatchMessage(input: { : "Look into each item and act on it as your task requires. This PR stays linked and watched, so do not link, watch, or list it again. End your turn when you are done, and T3 Code wakes you on the next change. When you hand the work back to the user, call unwatch_pull_request first so the thread returns to their inbox.", ].join("\n"); const failed = changes.some( - (change) => change.kind === "checks-failed" || change.kind === "conflicting", + (change) => + change.kind === "checks-failed" || + change.kind === "checks-finished" || + change.kind === "conflicting", ); const summary = changes.map((change) => SUMMARY[change.kind]); if (exhausted) summary.push("stopped watching"); diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 3daee9ce197e2..5cbeb4ded2e0c 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -87,7 +87,7 @@ export const ThreadPullRequestWatch = Schema.Struct({ passedChecks: Schema.Array(TrimmedNonEmptyString).pipe( Schema.withDecodingDefault(Effect.succeed([])), ), - /** Checks still pending when required ones passed early, owed one wake once all settle. */ + /** Checks still pending when required ones passed early or a gate check failed, owed one wake once they settle. */ owedChecks: Schema.optional(Schema.Array(TrimmedNonEmptyString)), remarksThrough: IsoDateTime, /** Reported remarks at remarksThrough, so late arrivals at that timestamp still count. */ From cbfcff009272980e68bdb3512581b6764b363a4d Mon Sep 17 00:00:00 2001 From: Ryan McDonough Date: Sat, 31 Oct 2026 00:38:09 -0700 Subject: [PATCH 2/8] fix(server): only a failing gate check owes the checks-finished wake An advisory failure alongside a required check rerun sent checks-finished with an empty failing list. Cover required gates, partial failures, and the wake message. Co-Authored-By: Claude Opus 5.5 --- .../orchestration-v2/pullRequestWatch.test.ts | 70 +++++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 7 +- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 2163e2c8ab927..3d798b16ce64e 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -227,6 +227,76 @@ describe("evaluatePullRequestWatch", () => { { kind: "checks-failed", failed: [check("e2e", "failure")] }, ]); assert.deepEqual(evaluatePullRequestWatch(failure.next, broke, noRemarks).changes, []); + + // A failure while another owed check still runs keeps that one owed. + const unitBroke = detail({ + checks: [sonar, check("unit", "failure"), check("e2e", "pending")], + }); + const second = evaluatePullRequestWatch(first.next, unitBroke, noRemarks); + assert.deepEqual(second.changes, [ + { kind: "checks-failed", failed: [check("unit", "failure")] }, + ]); + const settled = detail({ checks: [sonar, check("unit", "failure"), check("e2e", "success")] }); + const last = evaluatePullRequestWatch(second.next, settled, noRemarks); + assert.deepEqual(last.changes, [ + { kind: "checks-finished", failing: [sonar, check("unit", "failure")] }, + ]); + }); + + it("wakes once when the other required checks finish after a required one failed", () => { + const required = (name: string, status: PullRequestCheck["status"]) => ({ + ...check(name, status), + required: true, + }); + const early = detail({ + checks: [ + required("lint", "failure"), + required("unit", "pending"), + check("bot", "failure"), + check("e2e", "pending"), + ], + }); + const first = evaluatePullRequestWatch(watch(), early, noRemarks); + assert.equal(first.changes.length, 1); + assert.deepEqual(first.next.owedChecks, ["unit"]); + + const done = detail({ + checks: [ + required("lint", "failure"), + required("unit", "success"), + check("bot", "failure"), + check("e2e", "pending"), + ], + }); + const final = evaluatePullRequestWatch(first.next, done, noRemarks); + assert.deepEqual(final.changes, [ + { kind: "checks-finished", failing: [required("lint", "failure")] }, + ]); + assert.deepEqual(evaluatePullRequestWatch(final.next, done, noRemarks).changes, []); + const message = pullRequestWatchMessage({ + number: 7, + url: "https://github.com/o/r/pull/7", + baseBranch: "main", + headSha: "aaaaaaaaaa", + report: final, + }); + assert.deepEqual(message.notification.outcome, "failed"); + assert.equal(message.notification.summary, "#7: checks finished"); + assert.include( + message.text, + "- The checks still running when one failed have finished on aaaaaaa. Still failing:\n - lint https://ci.example/lint", + ); + + // A rerun of the failed required check is not the gate finishing, even with an advisory failure. + const rerun = detail({ + checks: [ + required("lint", "pending"), + required("unit", "success"), + check("bot", "failure"), + check("e2e", "pending"), + ], + }); + assert.deepEqual(evaluatePullRequestWatch(first.next, rerun, noRemarks).changes, []); }); it("clears old-head owed checks when a new head is pushed", () => { diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 51cbe6356d7a1..8525f9492f520 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -87,17 +87,14 @@ export function evaluatePullRequestWatch( } else if (settled && owed.length > 0) { changes.push({ kind: "checks-passed", count: detail.checks.length, required: false }); owedChecks = []; - } else if (!passedNow && failed.length > 0) { + } else if (gate.some(isFailedCheck)) { // Gate checks still running when one failed are owed one wake when they finish, since // the gate can no longer pass and so never sends the checks-passed wake. const running = gate.filter((check) => check.status === "pending").map((check) => check.name); owedChecks = newlyFailed.length > 0 ? running : owedBefore.filter((name) => running.includes(name)); if (newlyFailed.length === 0 && owedBefore.length > 0 && owedChecks.length === 0) { - changes.push({ - kind: "checks-finished", - failing: failed.filter((check) => gateNames.includes(check.name)), - }); + changes.push({ kind: "checks-finished", failing: gate.filter(isFailedCheck) }); } } passed = passedNow; From 9b2cb8ef3c50649507d9043f6ad6028cb7f33055 Mon Sep 17 00:00:00 2001 From: Ryan McDonough Date: Sat, 31 Oct 2026 00:55:07 -0700 Subject: [PATCH 3/8] fix(server): an advisory failure keeps the owed gate checks Only a new gate failure re-arms the owed set; a new advisory failure no longer owes gate checks that started after the gate failed. Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/pullRequestWatch.test.ts | 14 ++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 4 ++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 3d798b16ce64e..7e47bebb7230e 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -287,6 +287,20 @@ describe("evaluatePullRequestWatch", () => { "- The checks still running when one failed have finished on aaaaaaa. Still failing:\n - lint https://ci.example/lint", ); + // A new advisory failure keeps what was owed rather than owing a required check started since. + const advisory = detail({ + checks: [ + required("lint", "failure"), + required("unit", "pending"), + required("smoke", "pending"), + check("bot", "failure"), + check("e2e", "failure"), + ], + }); + const later = evaluatePullRequestWatch(first.next, advisory, noRemarks); + assert.deepEqual(later.changes, [{ kind: "checks-failed", failed: [check("e2e", "failure")] }]); + assert.deepEqual(later.next.owedChecks, ["unit"]); + // A rerun of the failed required check is not the gate finishing, even with an advisory failure. const rerun = detail({ checks: [ diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 8525f9492f520..e84387d5f90c8 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -91,8 +91,8 @@ export function evaluatePullRequestWatch( // Gate checks still running when one failed are owed one wake when they finish, since // the gate can no longer pass and so never sends the checks-passed wake. const running = gate.filter((check) => check.status === "pending").map((check) => check.name); - owedChecks = - newlyFailed.length > 0 ? running : owedBefore.filter((name) => running.includes(name)); + const gateNewlyFailed = newlyFailed.some((check) => gateNames.includes(check.name)); + owedChecks = gateNewlyFailed ? running : owedBefore.filter((name) => running.includes(name)); if (newlyFailed.length === 0 && owedBefore.length > 0 && owedChecks.length === 0) { changes.push({ kind: "checks-finished", failing: gate.filter(isFailedCheck) }); } From 76ba89f24f6bc598bb967915132d1ec5b1e1b357 Mon Sep 17 00:00:00 2001 From: Ryan McDonough Date: Sat, 31 Oct 2026 01:28:01 -0700 Subject: [PATCH 4/8] fix(server): owed checks stay owed while they run, whatever the gate When a failed check turned required, the gate shrank to the required checks and an owed advisory check still running was dropped, sending checks-finished early. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/orchestration-v2/pullRequestWatch.test.ts | 8 ++++++++ apps/server/src/orchestration-v2/pullRequestWatch.ts | 8 ++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 7e47bebb7230e..f48b71636692b 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -220,6 +220,14 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(final.changes, [{ kind: "checks-finished", failing: [sonar] }]); assert.deepEqual(evaluatePullRequestWatch(final.next, done, noRemarks).changes, []); + // An owed check stays owed while it runs, even when the failed one turns required. + const gated = detail({ + checks: [{ ...sonar, required: true }, check("unit", "success"), check("e2e", "pending")], + }); + const still = evaluatePullRequestWatch(waiting.next, gated, noRemarks); + assert.deepEqual(still.changes, []); + assert.deepEqual(still.next.owedChecks, ["e2e"]); + // A later failure is its own wake and leaves nothing more owed. const broke = detail({ checks: [sonar, check("unit", "success"), check("e2e", "failure")] }); const failure = evaluatePullRequestWatch(waiting.next, broke, noRemarks); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index e84387d5f90c8..c88ff819fbe49 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -90,9 +90,13 @@ export function evaluatePullRequestWatch( } else if (gate.some(isFailedCheck)) { // Gate checks still running when one failed are owed one wake when they finish, since // the gate can no longer pass and so never sends the checks-passed wake. - const running = gate.filter((check) => check.status === "pending").map((check) => check.name); + const running = detail.checks + .filter((check) => check.status === "pending") + .map((check) => check.name); const gateNewlyFailed = newlyFailed.some((check) => gateNames.includes(check.name)); - owedChecks = gateNewlyFailed ? running : owedBefore.filter((name) => running.includes(name)); + owedChecks = gateNewlyFailed + ? running.filter((name) => gateNames.includes(name)) + : owedBefore.filter((name) => running.includes(name)); if (newlyFailed.length === 0 && owedBefore.length > 0 && owedChecks.length === 0) { changes.push({ kind: "checks-finished", failing: gate.filter(isFailedCheck) }); } From f357c58eff77c1a63b8850e55d98cf9e10d42514 Mon Sep 17 00:00:00 2001 From: Ryan McDonough Date: Sat, 31 Oct 2026 01:52:34 -0700 Subject: [PATCH 5/8] fix(server): owed checks survive another failure, and finishing wakes beside one A new gate failure adds the gate's running checks to those still owed instead of replacing them. The last owed check finishing still sends checks-finished when an unrelated check fails in the same read. Co-Authored-By: Claude Opus 5.5 --- .../orchestration-v2/pullRequestWatch.test.ts | 17 +++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 8 +++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index f48b71636692b..b564527450043 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -228,6 +228,23 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(still.changes, []); assert.deepEqual(still.next.owedChecks, ["e2e"]); + // Another gate failure owes the gate's running checks on top of those still owed. + const lint = { ...check("lint", "failure"), required: true }; + const another = detail({ checks: [...gated.checks, lint] }); + const both = evaluatePullRequestWatch(still.next, another, noRemarks); + assert.deepEqual(both.changes, [{ kind: "checks-failed", failed: [lint] }]); + assert.deepEqual(both.next.owedChecks, ["e2e"]); + + // The last owed check finishing still wakes when an unrelated check fails in the same read. + const bot = check("bot", "failure"); + const finishedWithBot = detail({ + checks: [sonar, check("unit", "success"), check("e2e", "success"), bot], + }); + assert.deepEqual(evaluatePullRequestWatch(waiting.next, finishedWithBot, noRemarks).changes, [ + { kind: "checks-failed", failed: [bot] }, + { kind: "checks-finished", failing: [sonar, bot] }, + ]); + // A later failure is its own wake and leaves nothing more owed. const broke = detail({ checks: [sonar, check("unit", "success"), check("e2e", "failure")] }); const failure = evaluatePullRequestWatch(waiting.next, broke, noRemarks); diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index c88ff819fbe49..9569ab2c1595e 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -93,11 +93,13 @@ export function evaluatePullRequestWatch( const running = detail.checks .filter((check) => check.status === "pending") .map((check) => check.name); + const stillOwed = owedBefore.filter((name) => running.includes(name)); const gateNewlyFailed = newlyFailed.some((check) => gateNames.includes(check.name)); owedChecks = gateNewlyFailed - ? running.filter((name) => gateNames.includes(name)) - : owedBefore.filter((name) => running.includes(name)); - if (newlyFailed.length === 0 && owedBefore.length > 0 && owedChecks.length === 0) { + ? [...new Set([...stillOwed, ...running.filter((name) => gateNames.includes(name))])] + : stillOwed; + const owedFailed = newlyFailed.some((check) => owedBefore.includes(check.name)); + if (!owedFailed && owedBefore.length > 0 && owedChecks.length === 0) { changes.push({ kind: "checks-finished", failing: gate.filter(isFailedCheck) }); } } From a904b1f7e8bbfb8c328c454a2b88d419c0a9e2f1 Mon Sep 17 00:00:00 2001 From: Ryan McDonough Date: Wed, 4 Nov 2026 16:13:55 -0800 Subject: [PATCH 6/8] fix(server): a checks-finished wake says when only required checks finished With required checks, only the required checks still running are owed the wake, so it named all checks as finished while advisory ones still ran. Co-Authored-By: Claude Opus 5.5 --- .../src/orchestration-v2/pullRequestWatch.test.ts | 12 +++++++----- .../src/orchestration-v2/pullRequestWatch.ts | 14 +++++++++++--- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index b564527450043..f46c846ffed84 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -217,7 +217,9 @@ describe("evaluatePullRequestWatch", () => { const done = detail({ checks: [sonar, check("unit", "success"), check("e2e", "success")] }); const final = evaluatePullRequestWatch(waiting.next, done, noRemarks); - assert.deepEqual(final.changes, [{ kind: "checks-finished", failing: [sonar] }]); + assert.deepEqual(final.changes, [ + { kind: "checks-finished", failing: [sonar], required: false }, + ]); assert.deepEqual(evaluatePullRequestWatch(final.next, done, noRemarks).changes, []); // An owed check stays owed while it runs, even when the failed one turns required. @@ -242,7 +244,7 @@ describe("evaluatePullRequestWatch", () => { }); assert.deepEqual(evaluatePullRequestWatch(waiting.next, finishedWithBot, noRemarks).changes, [ { kind: "checks-failed", failed: [bot] }, - { kind: "checks-finished", failing: [sonar, bot] }, + { kind: "checks-finished", failing: [sonar, bot], required: false }, ]); // A later failure is its own wake and leaves nothing more owed. @@ -264,7 +266,7 @@ describe("evaluatePullRequestWatch", () => { const settled = detail({ checks: [sonar, check("unit", "failure"), check("e2e", "success")] }); const last = evaluatePullRequestWatch(second.next, settled, noRemarks); assert.deepEqual(last.changes, [ - { kind: "checks-finished", failing: [sonar, check("unit", "failure")] }, + { kind: "checks-finished", failing: [sonar, check("unit", "failure")], required: false }, ]); }); @@ -295,7 +297,7 @@ describe("evaluatePullRequestWatch", () => { }); const final = evaluatePullRequestWatch(first.next, done, noRemarks); assert.deepEqual(final.changes, [ - { kind: "checks-finished", failing: [required("lint", "failure")] }, + { kind: "checks-finished", failing: [required("lint", "failure")], required: true }, ]); assert.deepEqual(evaluatePullRequestWatch(final.next, done, noRemarks).changes, []); const message = pullRequestWatchMessage({ @@ -309,7 +311,7 @@ describe("evaluatePullRequestWatch", () => { assert.equal(message.notification.summary, "#7: checks finished"); assert.include( message.text, - "- The checks still running when one failed have finished on aaaaaaa. Still failing:\n - lint https://ci.example/lint", + "- The required checks still running when one failed have finished on aaaaaaa. Still failing:\n - lint https://ci.example/lint", ); // A new advisory failure keeps what was owed rather than owing a required check started since. diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 9569ab2c1595e..811c54a719c61 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -17,7 +17,11 @@ const SNIPPET_LENGTH = 200; export type PullRequestWatchChange = | { readonly kind: "checks-failed"; readonly failed: ReadonlyArray } | { readonly kind: "checks-passed"; readonly count: number; readonly required: boolean } - | { readonly kind: "checks-finished"; readonly failing: ReadonlyArray } + | { + readonly kind: "checks-finished"; + readonly failing: ReadonlyArray; + readonly required: boolean; + } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "no-checks"; readonly minutes: number } | { readonly kind: "conflicting" }; @@ -100,7 +104,11 @@ export function evaluatePullRequestWatch( : stillOwed; const owedFailed = newlyFailed.some((check) => owedBefore.includes(check.name)); if (!owedFailed && owedBefore.length > 0 && owedChecks.length === 0) { - changes.push({ kind: "checks-finished", failing: gate.filter(isFailedCheck) }); + changes.push({ + kind: "checks-finished", + failing: gate.filter(isFailedCheck), + required: owedBefore.every((name) => required.some((check) => check.name === name)), + }); } } passed = passedNow; @@ -223,7 +231,7 @@ function changeLines( ]; case "checks-finished": return [ - `- The checks still running when one failed have finished${context.commit}. Still failing:`, + `- The ${change.required ? "required " : ""}checks still running when one failed have finished${context.commit}. Still failing:`, ...listed( change.failing, (check) => ` - ${check.name}${check.url ? ` ${check.url}` : ""}`, From f2544f8dd64a679442ab35d5ac7edad06f162aed Mon Sep 17 00:00:00 2001 From: Ryan McDonough Date: Wed, 4 Nov 2026 16:23:27 -0800 Subject: [PATCH 7/8] test(server): drop the copy assertion on the checks-finished summary Co-Authored-By: Claude Opus 5.5 --- apps/server/src/orchestration-v2/pullRequestWatch.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index f46c846ffed84..8429cd44a2fe2 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -308,7 +308,6 @@ describe("evaluatePullRequestWatch", () => { report: final, }); assert.deepEqual(message.notification.outcome, "failed"); - assert.equal(message.notification.summary, "#7: checks finished"); assert.include( message.text, "- The required checks still running when one failed have finished on aaaaaaa. Still failing:\n - lint https://ci.example/lint", From 9a48b3481d2e382b0b6245328cc38b7694b63be5 Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Thu, 5 Nov 2026 03:44:36 +0000 Subject: [PATCH 8/8] revert(contracts): keep the owedChecks doc comment as on main --- packages/contracts/src/threadPullRequest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 5cbeb4ded2e0c..3daee9ce197e2 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -87,7 +87,7 @@ export const ThreadPullRequestWatch = Schema.Struct({ passedChecks: Schema.Array(TrimmedNonEmptyString).pipe( Schema.withDecodingDefault(Effect.succeed([])), ), - /** Checks still pending when required ones passed early or a gate check failed, owed one wake once they settle. */ + /** Checks still pending when required ones passed early, owed one wake once all settle. */ owedChecks: Schema.optional(Schema.Array(TrimmedNonEmptyString)), remarksThrough: IsoDateTime, /** Reported remarks at remarksThrough, so late arrivals at that timestamp still count. */