From bdbef5c4dd92bbd138cd8bca397cd7d16f8ec0a2 Mon Sep 17 00:00:00 2001 From: Eemeli Palotie Date: Sat, 14 Nov 2026 06:45:53 +0200 Subject: [PATCH] fix(server): PR watches notify when retried Actions jobs finish --- .../orchestration-v2/pullRequestWatch.test.ts | 81 +++++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 20 +++-- packages/contracts/src/threadPullRequest.ts | 2 +- 3 files changed, 97 insertions(+), 6 deletions(-) diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index eb2ae2756d540..7640cb0a1322c 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -524,6 +524,87 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(evaluatePullRequestWatch(green.next, botDone, noRemarks).changes, []); }); + it.each(["failure", "cancelled", "success"] as const)( + "wakes once when a same-head workflow retry of a %s finishes", + (status) => { + const required = { ...check("test", "success"), required: true }; + const job = (name: string, status: PullRequestCheck["status"], id: number) => ({ + ...check(name, status), + url: `${run(101)}/job/${id}`, + }); + const initial = detail({ + checks: [required, job("seed", status, 1), job("smoke", "skipped", 2)], + }); + const baseline = evaluatePullRequestWatch( + watch({ awaitingBaseline: "all" }), + initial, + noRemarks, + ); + assert.deepEqual(baseline.changes, []); + assert.deepEqual(baseline.next.owedChecks, []); + + const retrying = detail({ + checks: [required, job("seed", "pending", 3), job("smoke", "skipped", 2)], + }); + const retry = evaluatePullRequestWatch(baseline.next, retrying, noRemarks); + assert.deepEqual(retry.changes, []); + const repeated = evaluatePullRequestWatch(retry.next, retrying, noRemarks); + assert.deepEqual(repeated.changes, []); + + // The downstream job registers after the retry, with the same head and workflow run ID. + const smoking = detail({ + checks: [required, job("seed", "success", 3), job("smoke", "pending", 4)], + }); + const comment = remark("deploy-bot", "2026-10-02T12:06:00Z", "Preview deployed."); + const deployed = evaluatePullRequestWatch(repeated.next, smoking, [comment]); + assert.deepEqual(deployed.changes, [{ kind: "remarks", remarks: [comment] }]); + + const done = detail({ + checks: [required, job("seed", "success", 3), job("smoke", "success", 4)], + }); + const final = evaluatePullRequestWatch(deployed.next, done, [comment]); + assert.deepEqual(final.changes, [{ kind: "checks-passed", count: 3, required: false }]); + assert.deepEqual(final.next.owedChecks, []); + assert.deepEqual(evaluatePullRequestWatch(final.next, done, [comment]).changes, []); + + const advisory = detail({ checks: [...done.checks, check("bot", "pending")] }); + const churn = evaluatePullRequestWatch(final.next, advisory, [comment]); + assert.deepEqual(churn.changes, []); + assert.deepEqual( + evaluatePullRequestWatch( + churn.next, + detail({ checks: [...done.checks, check("bot", "success")] }), + [comment], + ).changes, + [], + ); + }, + ); + + it.each([false, true])( + "keeps completion owed when an unfinished workflow disappears (jobs observed: %s)", + (jobsObserved) => { + const required = { ...check("test", "success"), required: true }; + const green = detail({ checks: [required] }); + const first = evaluatePullRequestWatch(watch(), green, noRemarks); + const queued = evaluatePullRequestWatch( + first.next, + detail({ checks: [required, { ...check("Preview", "pending"), url: run(101) }] }), + noRemarks, + ); + assert.deepEqual(queued.changes, []); + const job = { ...check("smoke", "pending"), url: `${run(101)}/job/1` }; + const running = jobsObserved + ? evaluatePullRequestWatch(queued.next, detail({ checks: [required, job] }), noRemarks) + : queued; + assert.deepEqual(running.changes, []); + const done = detail({ checks: [required, { ...job, status: "success" }] }); + const final = evaluatePullRequestWatch(running.next, done, noRemarks); + assert.deepEqual(final.changes, [{ kind: "checks-passed", count: 2, required: false }]); + assert.deepEqual(evaluatePullRequestWatch(final.next, done, noRemarks).changes, []); + }, + ); + it("reports an owed non-required failure once after the required notice", () => { 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 e12d9a124df8a..a7323495e2058 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -126,13 +126,23 @@ export function evaluatePullRequestWatch( const told = passed && passedChecks.length === 0 ? gateNames : passedChecks; // Required checks can finish between sweeps; detect new names. With no required checks, ignore advisory gate growth. const gateGrew = required.length > 0 && gateNames.some((name) => !told.includes(name)); - // Other checks still pending when required ones passed are owed one wake when all settle. - // Only those are owed, so later advisory churn never wakes. One turned required is gate news. + // Actions work can start after the gate passes, including same-head retries. Keep its debt + // when a workflow placeholder disappears; unrelated advisory churn still stays quiet. const owed = passedNow && passed - ? checks - .filter((check) => check.required !== true && owedChecks.includes(check.name)) - .map((check) => check.name) + ? [ + ...new Set([ + ...owedChecks, + ...checks + .filter( + (check) => + check.required !== true && + check.status === "pending" && + workflowRunOf(check.url) !== null, + ) + .map((check) => check.name), + ]), + ].filter((name) => !required.some((check) => check.name === name)) : []; const settled = failed.length === 0 && diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 7775f0755ff0b..7774e27e90353 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. */ + /** Pending work owed one wake once all checks settle, including late Actions jobs and retries. */ owedChecks: Schema.optional(Schema.Array(TrimmedNonEmptyString)), /** Approval holds keyed by workflow run URL, or check name when no run URL is known. */ awaitingApproval: Schema.optional(Schema.Array(TrimmedNonEmptyString)),