From 44bfd9a9bc16db9ac9618d6dcb387a47eeea6e37 Mon Sep 17 00:00:00 2001 From: chriswong6031-creator <292968551+chriswong6031-creator@users.noreply.github.com> Date: Mon, 9 Nov 2026 19:18:22 -0700 Subject: [PATCH 1/2] fix(watch): wake once when a green PR release stalls A green PR can remain unmerged indefinitely when its merge queue or repository controller cannot finish; ordinary head-check fingerprints do not change. Add a single time-bound actionable wake, persisted with the existing watch state, and reset it only after material progress. No automatic requeue, merge, retry, or new lifecycle owner. --- .../PullRequestWatchReactor.ts | 17 +++++- .../orchestration-v2/pullRequestWatch.test.ts | 58 +++++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 32 +++++++++- packages/contracts/src/threadPullRequest.ts | 4 ++ 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index 5050ac3272894..0d8e2b8e99b10 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -38,6 +38,7 @@ import * as ProjectionStore from "./ProjectionStore.ts"; import { evaluatePullRequestWatch, PULL_REQUEST_WATCH_NO_CHECKS_MS, + PULL_REQUEST_WATCH_RELEASE_STALL_MS, pullRequestWatchMessage, } from "./pullRequestWatch.ts"; @@ -152,20 +153,28 @@ function planRead( watch.checksAwaitedSince != null && now - Date.parse(watch.checksAwaitedSince) >= PULL_REQUEST_WATCH_NO_CHECKS_MS, ); + const stalledDue = group.targets.some( + ({ watch }) => + watch.passed && + watch.releaseStallNotifiedAt === undefined && + now - Date.parse(watch.lastProgressAt ?? watch.startedAt) >= + PULL_REQUEST_WATCH_RELEASE_STALL_MS, + ); if (fingerprint === null) { const read = last.inFlight || !last.remarksComplete || last.fingerprint !== group.fingerprint || now - last.at >= QUIET_REREAD_MS; - return { detail: read || checksDue, activity: read }; + return { detail: read || checksDue || stalledDue, activity: read }; } const activity = last.remarks !== fingerprint.remarks || !last.remarksComplete || now - last.at >= FINGERPRINT_REREAD_MS; return { - detail: activity || checksDue || last.status !== fingerprint.status || last.inFlight, + detail: + activity || checksDue || stalledDue || last.status !== fingerprint.status || last.inFlight, activity, }; } @@ -182,7 +191,9 @@ function watchesEqual(left: ThreadPullRequestWatch, right: ThreadPullRequestWatc left.remarkIds.join("\n") === right.remarkIds.join("\n") && left.conflicting === right.conflicting && left.wakes === right.wakes && - left.checksAwaitedSince === right.checksAwaitedSince + left.checksAwaitedSince === right.checksAwaitedSince && + left.lastProgressAt === right.lastProgressAt && + left.releaseStallNotifiedAt === right.releaseStallNotifiedAt ); } diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 6ab0191649490..6616f503692ac 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -389,6 +389,64 @@ describe("evaluatePullRequestWatch", () => { ); }); + it("wakes only once when a green PR has no release progress for an hour", () => { + const green = detail({ checks: [check("test", "success")] }); + const first = evaluate(watch(), green, noRemarks, Date.parse(STARTED)); + assert.deepEqual(first.changes, [{ kind: "checks-passed", count: 1, required: false }]); + const before = evaluate(first.next, green, noRemarks, Date.parse(STARTED) + 59 * 60_000); + assert.deepEqual(before.changes, []); + const stalled = evaluate(before.next, green, noRemarks, Date.parse(STARTED) + 61 * 60_000); + assert.deepEqual(stalled.changes, [{ kind: "release-stalled", minutes: 61 }]); + const again = evaluate(stalled.next, green, noRemarks, Date.parse(STARTED) + 24 * 60 * 60_000); + assert.deepEqual(again.changes, []); + const { text } = pullRequestWatchMessage({ + number: 1, + url: "https://github.com/o/r/pull/1", + baseBranch: "main", + headSha: green.headSha ?? null, + report: stalled, + }); + assert.include(text, "merge-group CI"); + assert.include(text, "Do not just re-watch"); + }); + + it("does not stall pending checks or immediately after new material progress", () => { + const green = detail({ checks: [check("test", "success")] }); + const start = Date.parse(STARTED); + const initial = watch({ + headSha: "aaaaaaaaaa", + passed: true, + passedChecks: ["test"], + lastProgressAt: STARTED, + }); + const pending = evaluate(initial, detail(), noRemarks, start + 61 * 60_000); + assert.deepEqual(pending.changes, []); + const changed = evaluate( + initial, + detail({ ...green, headSha: "bbbbbbbbbb" }), + noRemarks, + start + 61 * 60_000, + ); + assert.deepEqual(changed.changes, [{ kind: "checks-passed", count: 1, required: false }]); + assert.equal(changed.next.releaseStallNotifiedAt, undefined); + assert.deepEqual( + evaluate( + changed.next, + detail({ ...green, headSha: "bbbbbbbbbb" }), + noRemarks, + start + 62 * 60_000, + ).changes, + [], + ); + const later = evaluate( + changed.next, + detail({ ...green, headSha: "bbbbbbbbbb" }), + noRemarks, + start + 122 * 60_000, + ); + assert.deepEqual(later.changes, [{ kind: "release-stalled", minutes: 61 }]); + }); + it("does not spend the comment wake limit on check results", () => { const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); const result = evaluatePullRequestWatch( diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index f4c0000847236..36ba7e8a42741 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -11,6 +11,8 @@ import * as DateTime from "effect/DateTime"; export const PULL_REQUEST_WATCH_WAKE_LIMIT = 10; /** How long a head may go without any check before the agent is told none reported. */ export const PULL_REQUEST_WATCH_NO_CHECKS_MS = 15 * 60_000; +/** Give one actionable wake when a green head makes no release progress for an hour. */ +export const PULL_REQUEST_WATCH_RELEASE_STALL_MS = 60 * 60_000; const LISTED_ITEMS = 10; const SNIPPET_LENGTH = 200; @@ -19,7 +21,8 @@ export type PullRequestWatchChange = | { readonly kind: "checks-passed"; readonly count: number; readonly required: boolean } | { readonly kind: "remarks"; readonly remarks: ReadonlyArray } | { readonly kind: "no-checks"; readonly minutes: number } - | { readonly kind: "conflicting" }; + | { readonly kind: "conflicting" } + | { readonly kind: "release-stalled"; readonly minutes: number }; export interface PullRequestWatchReport { readonly changes: ReadonlyArray; @@ -136,6 +139,26 @@ export function evaluatePullRequestWatch( const conflicting = detail.mergeability === "unknown" ? toldConflict : detail.mergeability === "conflicting"; + // The PR head can be green while a repository-specific merge controller refuses + // release. Don't turn a quiet watch into an indefinite no-action state. + const materialProgress = headMoved || changes.some((change) => change.kind !== "remarks"); + const lastProgressAt = materialProgress + ? DateTime.formatIso(DateTime.makeUnsafe(now)) + : (watch.lastProgressAt ?? watch.startedAt); + let releaseStallNotifiedAt = materialProgress ? undefined : watch.releaseStallNotifiedAt; + if ( + !materialProgress && + passed && + detail.checks.length > 0 && + releaseStallNotifiedAt === undefined && + now - Date.parse(lastProgressAt) >= PULL_REQUEST_WATCH_RELEASE_STALL_MS + ) { + changes.push({ + kind: "release-stalled", + minutes: Math.floor((now - Date.parse(lastProgressAt)) / 60_000), + }); + releaseStallNotifiedAt = DateTime.formatIso(DateTime.makeUnsafe(now)); + } const commentsOnly = changes.length > 0 && changes.every((change) => change.kind === "remarks"); const progress = headMoved || (changes.length > 0 && !commentsOnly); const wakes = (progress ? 0 : watch.wakes) + (commentsOnly ? 1 : 0); @@ -153,6 +176,8 @@ export function evaluatePullRequestWatch( conflicting, wakes, checksAwaitedSince, + lastProgressAt, + ...(releaseStallNotifiedAt === undefined ? {} : { releaseStallNotifiedAt }), }, exhausted: commentsOnly && wakes >= PULL_REQUEST_WATCH_WAKE_LIMIT, }; @@ -210,6 +235,10 @@ function changeLines( ]; case "conflicting": return [`- The branch now conflicts with ${context.baseBranch}.`]; + case "release-stalled": + return [ + `- This open PR has shown no material release progress for ${change.minutes} minutes despite passing head checks. Inspect the actual host merge queue, merge-group CI, or repository-specific merge controller, and repair its blocking verdict. Do not just re-watch or blindly requeue.`, + ]; } } @@ -219,6 +248,7 @@ const SUMMARY: Record = { remarks: "new comments", "no-checks": "no checks reported", conflicting: "merge conflict", + "release-stalled": "release progress stalled", }; export function pullRequestWatchMessage(input: { diff --git a/packages/contracts/src/threadPullRequest.ts b/packages/contracts/src/threadPullRequest.ts index 3daee9ce197e2..be06c3af2edcc 100644 --- a/packages/contracts/src/threadPullRequest.ts +++ b/packages/contracts/src/threadPullRequest.ts @@ -100,6 +100,10 @@ export const ThreadPullRequestWatch = Schema.Struct({ * none did. Null once either happened; absent on watches saved before it existed. */ checksAwaitedSince: Schema.optional(Schema.NullOr(IsoDateTime)), + /** Last substantive progress for this watched head; absent in older watches. */ + lastProgressAt: Schema.optional(IsoDateTime), + /** Last one-shot stalled-release notification; resets on substantive progress. */ + releaseStallNotifiedAt: Schema.optional(IsoDateTime), }); export type ThreadPullRequestWatch = typeof ThreadPullRequestWatch.Type; From eac5dc33e6626e98695442134eb94f1afc207253 Mon Sep 17 00:00:00 2001 From: chriswong6031-creator <292968551+chriswong6031-creator@users.noreply.github.com> Date: Mon, 9 Nov 2026 20:25:42 -0700 Subject: [PATCH 2/2] fix(watch): preserve stall watermarks and test runtime wake Carry one-shot stalled-release progress across explicit re-watches, treat a cleared conflict as meaningful progress, and assert the time-bound notification in runtime-layer integration. Keep all existing release checks and no auto-requeue policy. --- .../src/orchestration-v2/Orchestrator.ts | 8 +++++ .../orchestration-v2/pullRequestWatch.test.ts | 29 +++++++++++++++++++ .../src/orchestration-v2/pullRequestWatch.ts | 5 +++- .../src/orchestration-v2/runtimeLayer.test.ts | 24 ++++++++++++++- 4 files changed, 64 insertions(+), 2 deletions(-) diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 37c4a75f663b6..17978c64e8e0f 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -4161,6 +4161,14 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio ...(existing.endedWatch?.checksAwaitedSince !== undefined && { checksAwaitedSince: existing.endedWatch.checksAwaitedSince, }), + // A user re-watch is not new PR progress. Restore the one-shot + // stall watermark so re-watching cannot manufacture a retry wake. + ...(existing.endedWatch?.lastProgressAt !== undefined && { + lastProgressAt: existing.endedWatch.lastProgressAt, + }), + ...(existing.endedWatch?.releaseStallNotifiedAt !== undefined && { + releaseStallNotifiedAt: existing.endedWatch.releaseStallNotifiedAt, + }), remarksThrough: startedAt, remarkIds: [], wakes: 0, diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 6616f503692ac..f90093865b36c 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -447,6 +447,35 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(later.changes, [{ kind: "release-stalled", minutes: 61 }]); }); + it("clears the stalled-release clock when a same-head conflict is resolved", () => { + const green = detail({ checks: [check("test", "success")] }); + const t0 = Date.parse(STARTED); + const before = evaluate( + watch({ + headSha: "aaaaaaaaaa", + passed: true, + passedChecks: ["test"], + conflicting: true, + lastProgressAt: STARTED, + releaseStallNotifiedAt: STARTED, + }), + green, + noRemarks, + t0 + 3 * 60 * 60_000, + ); + assert.deepEqual(before.changes, []); + assert.equal(before.next.conflicting, false); + assert.equal(before.next.releaseStallNotifiedAt, undefined); + assert.deepEqual( + evaluate(before.next, green, noRemarks, t0 + 3 * 60 * 60_000 + 60_000).changes, + [], + ); + assert.deepEqual( + evaluate(before.next, green, noRemarks, t0 + 4 * 60 * 60_000 + 60_000).changes, + [{ kind: "release-stalled", minutes: 61 }], + ); + }); + it("does not spend the comment wake limit on check results", () => { const tired = watch({ headSha: "aaaaaaaaaa", wakes: PULL_REQUEST_WATCH_WAKE_LIMIT - 1 }); const result = evaluatePullRequestWatch( diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 36ba7e8a42741..1a1e453e79860 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -141,7 +141,10 @@ export function evaluatePullRequestWatch( // The PR head can be green while a repository-specific merge controller refuses // release. Don't turn a quiet watch into an indefinite no-action state. - const materialProgress = headMoved || changes.some((change) => change.kind !== "remarks"); + const materialProgress = + headMoved || + conflicting !== watch.conflicting || + changes.some((change) => change.kind !== "remarks"); const lastProgressAt = materialProgress ? DateTime.formatIso(DateTime.makeUnsafe(now)) : (watch.lastProgressAt ?? watch.startedAt); diff --git a/apps/server/src/orchestration-v2/runtimeLayer.test.ts b/apps/server/src/orchestration-v2/runtimeLayer.test.ts index b9f5cd049eec9..0d05f49357bd9 100644 --- a/apps/server/src/orchestration-v2/runtimeLayer.test.ts +++ b/apps/server/src/orchestration-v2/runtimeLayer.test.ts @@ -3632,6 +3632,27 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch?.owedChecks, [], ); + // An already-delivered release-stall alert survives an explicit stop/re-watch. + yield* TestClock.adjust("61 minutes"); + yield* restarted.sweep; + yield* worker.drain(); + assert.deepEqual(yield* summaries, [ + "#12: checks passed", + "#12: checks passed", + "#12: release progress stalled", + ]); + const beforeRewatch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0] + ?.watch; + assert.isDefined(beforeRewatch?.releaseStallNotifiedAt); + assert.isDefined(beforeRewatch?.lastProgressAt); + yield* watch(false, "end-first"); + yield* watch(true, "end-rewatch"); + const afterRewatch = (yield* orchestrator.getThreadShell(threadId))?.pullRequests?.[0]?.watch; + assert.equal(afterRewatch?.releaseStallNotifiedAt, beforeRewatch?.releaseStallNotifiedAt); + assert.equal(afterRewatch?.lastProgressAt, beforeRewatch?.lastProgressAt); + yield* restarted.sweep; + yield* worker.drain(); + assert.equal((yield* summaries).length, 3); yield* watch(false, "end"); }), ); @@ -3963,7 +3984,8 @@ it.layer(layerTest)("OrchestrationV2LayerLive lifecycle", (it) => { yield* TestClock.adjust("1 hour"); yield* sweep; const settled = yield* wakes; - assert.lengthOf(settled, 2); + assert.lengthOf(settled, 3); + assert.equal(settled[2]!.notification?.summary, "#14: release progress stalled"); assert.lengthOf( settled.filter((message) => message.text.includes("No checks have reported")), 1,