From aac161f1dbd2468508763356bcffa268fb3ac65f Mon Sep 17 00:00:00 2001 From: Emi Griffith Date: Mon, 9 Nov 2026 08:28:26 -0800 Subject: [PATCH] fix(server): wake PR watches when checks finish after a failure --- .../src/orchestration-v2/Orchestrator.ts | 3 +++ .../PullRequestWatchReactor.ts | 1 + .../orchestration-v2/pullRequestWatch.test.ts | 11 +++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 19 +++++++++++++++++++ .../src/orchestration-v2/runtimeLayer.test.ts | 6 ++++++ docs/user/source-control.md | 3 ++- packages/contracts/src/threadPullRequest.ts | 1 + 7 files changed, 43 insertions(+), 1 deletion(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 37c4a75f663b6..41f9cd0906819 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4161,6 +4161,9 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ...(existing.endedWatch?.checksAwaitedSince !== undefined && { checksAwaitedSince: existing.endedWatch.checksAwaitedSince, }), + ...(existing.endedWatch?.checksPending !== undefined && { + checksPending: existing.endedWatch.checksPending, + }), remarksThrough: startedAt, remarkIds: [], wakes: 0, diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 5050ac3272894..dbb326ec4a31d 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -182,6 +182,7 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.remarkIds.join("\n") === right.remarkIds.join("\n") && left.conflicting === right.conflicting && left.wakes === right.wakes && + left.checksPending === right.checksPending && left.checksAwaitedSince === right.checksAwaitedSince ); } diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 6ab0191649490..6740b3d892c5a 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -97,6 +97,17 @@ describe("evaluatePullRequestWatch", () => { assert.equal(evaluatePullRequestWatch(test.next, pushed, noRemarks).changes.length, 1); }); + it("reports completion after a known failure when the remaining checks finish", () => { + const optional = check("windows-arm-optional", "failure"); + const pending = detail({ checks: [optional, check("gate", "pending")] }); + const first = evaluatePullRequestWatch(watch(), pending, noRemarks); + const done = detail({ checks: [optional, check("gate", "success")] }); + const final = evaluatePullRequestWatch(first.next, done, noRemarks); + assert.deepEqual(final.changes, [{ kind: "checks-settled", count: 2, failed: 1 }]); + assert.isFalse(final.next.passed); + assert.deepEqual(evaluatePullRequestWatch(final.next, done, 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/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index f4c0000847236..9d3afd18ea3d4 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-settled"; readonly count: number; readonly failed: number } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "no-checks"; readonly minutes: number } | { readonly kind: "conflicting" }; @@ -50,6 +51,7 @@ export function evaluatePullRequestWatch( let passed = headMoved ? false : watch.passed; let passedChecks = headMoved ? [] : watch.passedChecks; let owedChecks = headMoved ? [] : (watch.owedChecks ?? []); + let checksPending = headMoved ? false : (watch.checksPending ?? false); if (detail.checks.length > 0) { const failed = detail.checks.filter(isFailedCheck); const newlyFailed = failed.filter((check) => !failedChecks.includes(check.name)); @@ -86,6 +88,17 @@ export function evaluatePullRequestWatch( changes.push({ kind: "checks-passed", count: detail.checks.length, required: false }); owedChecks = []; } + const pendingNow = detail.checks.some((check) => check.status === "pending"); + if ( + checksPending && + !pendingNow && + failed.length > 0 && + newlyFailed.length === 0 && + !changes.some((change) => change.kind === "checks-passed") + ) { + changes.push({ kind: "checks-settled", count: detail.checks.length, failed: failed.length }); + } + checksPending = pendingNow; passed = passedNow; passedChecks = passedNow ? gateNames : []; } @@ -153,6 +166,7 @@ export function evaluatePullRequestWatch( conflicting, wakes, checksAwaitedSince, + checksPending, }, exhausted: commentsOnly && wakes >= PULL_REQUEST_WATCH_WAKE_LIMIT, }; @@ -194,6 +208,10 @@ function changeLines( return [ `- All ${change.count} ${change.required ? "required " : ""}${change.count === 1 ? "check" : "checks"} passed${context.commit}.`, ]; + case "checks-settled": + return [ + `- All ${change.count} checks have finished${context.commit}; ${change.failed} remain failed or need attention. Inspect the results before merging.`, + ]; case "remarks": return [ `- ${change.remarks.length} new ${change.remarks.length === 1 ? "comment" : "comments"}:`, @@ -216,6 +234,7 @@ function changeLines( const SUMMARY: Record = { "checks-failed": "checks failed", "checks-passed": "checks passed", + "checks-settled": "checks finished", remarks: "new comments", "no-checks": "no checks reported", conflicting: "merge conflict", diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index b9f5cd049eec9..6436c5c20adba 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -3603,6 +3603,9 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { ["test"], ); // The fresh reactor reloads the nonempty obligation from the persisted watch. + assert.isTrue( + (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch?.checksPending, + ); const restarted = yield* PullRequestWatchReactor.make.pipe(Effect.provide(host)); yield* restarted.sweep; yield* worker.drain(); @@ -3611,6 +3614,9 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { // A new watch on the same pull request knows what the last one told. yield* watch(false, "stop"); yield* watch(true, "again"); + assert.isTrue( + (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch?.checksPending, + ); yield* restarted.sweep; yield* worker.drain(); assert.deepEqual(yield* summaries, ["#12: checks passed"]); diff --git a/docs/user/source-control.md b/docs/user/source-control.md index 12623f4f4790f..a862608156916 100644 --- a/docs/user/source-control.md +++ b/docs/user/source-control.md @@ -318,7 +318,8 @@ review is terminal. An open or unsynced link keeps it active. Ask the agent to watch, monitor, or babysit a pull request and it calls `watch_pull_request`. While the thread is active, the server checks the pull request every two minutes and wakes the agent when a check fails, the required checks pass, someone else comments or reviews, or the branch starts to -conflict. Threads in a project that watch the same pull request share one check. On GitHub, a check +conflict. It also wakes when the remaining checks finish after an earlier failure. +Threads in a project that watch the same pull request share one check. On GitHub, a check first asks whether anything changed and reads the pull request only when it did, which keeps watching inside GitHub's rate limit. Comments from your own account do not wake it. Watching ends when the pull request merges or closes, after 10 wakes in a row that bring only comments, after 8 diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 3daee9ce197e2..4851b6984a503 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -100,6 +100,7 @@ export const ThreadPullRequestWatch = Schema.Struct({ * none did. Null once either happened; absent on watches saved before it existed. */ checksAwaitedSince: Schema.optional(Schema.NullOr(IsoDateTime)), + checksPending: Schema.optional(Schema.Boolean), }); export type ThreadPullRequestWatch = typeof ThreadPullRequestWatch.Type;