diff --git a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts index e1669f011e6c0..2b588b1d2e23e 100644 --- a/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts +++ b/apps/server/src/orchestration-v2/PullRequestWatchReactor.ts @@ -508,9 +508,9 @@ export const make = Effect.gen(function* () { } readFailures.delete(group.key); const [detail, activity] = read.value; - const headSha = detail.headSha ?? null; for (const target of group.targets) { const life = lifeOf(target); + const headSha = detail.headSha ?? life.headSha; const pushed = life.headSha !== null && headSha !== life.headSha; lives.set(lifeKey(target), { ...life, diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts index 0b6070f364662..5f8034f756e04 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.test.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.test.ts @@ -339,6 +339,10 @@ describe("evaluatePullRequestWatch", () => { const conflicting = detail({ mergeability: "conflicting" }); const first = evaluatePullRequestWatch(watch(), conflicting, noRemarks); assert.deepEqual(first.changes, [{ kind: "conflicting" }]); + const { headSha: _unreadHead, ...headUnread } = conflicting; + const unread = evaluatePullRequestWatch(first.next, headUnread, noRemarks); + assert.deepEqual(unread.changes, []); + assert.deepEqual(evaluatePullRequestWatch(unread.next, conflicting, noRemarks).changes, []); const recomputing = evaluatePullRequestWatch( first.next, detail({ mergeability: "unknown" }), @@ -352,6 +356,20 @@ describe("evaluatePullRequestWatch", () => { assert.deepEqual(evaluatePullRequestWatch(clean.next, conflicting, noRemarks).changes, [ { kind: "conflicting" }, ]); + const pushed = { headSha: "bbbbbbbbbb" }; + const recomputed = evaluatePullRequestWatch( + first.next, + detail({ ...pushed, mergeability: "unknown" }), + noRemarks, + ); + assert.deepEqual( + evaluatePullRequestWatch( + recomputed.next, + detail({ ...pushed, mergeability: "conflicting" }), + noRemarks, + ).changes, + [{ kind: "conflicting" }], + ); }); it("does not spend the comment wake limit on check results", () => { diff --git a/apps/server/src/orchestration-v2/pullRequestWatch.ts b/apps/server/src/orchestration-v2/pullRequestWatch.ts index 033125a6d6c9d..6f4353539f5cc 100644 --- a/apps/server/src/orchestration-v2/pullRequestWatch.ts +++ b/apps/server/src/orchestration-v2/pullRequestWatch.ts @@ -41,7 +41,8 @@ export function evaluatePullRequestWatch( now: number, ): PullRequestWatchReport { const changes: Array = []; - const headSha = detail.headSha ?? null; + // Azure DevOps omits the head when its iteration read fails; that is not a push. + const headSha = detail.headSha ?? watch.headSha; const headMoved = headSha !== watch.headSha; // Hosts can return no checks on read failure; preserve prior state. @@ -126,12 +127,14 @@ export function evaluatePullRequestWatch( ...atLatest.map((remark) => remark.id), ]; - if (detail.mergeability === "conflicting" && !watch.conflicting) { + // Like a failed check, a conflict a push did not resolve is news for the new head. + const toldConflict = !headMoved && watch.conflicting; + if (detail.mergeability === "conflicting" && !toldConflict) { changes.push({ kind: "conflicting" }); } // "unknown" is a host still computing after a push; only a clean answer clears a conflict. const conflicting = - detail.mergeability === "unknown" ? watch.conflicting : detail.mergeability === "conflicting"; + detail.mergeability === "unknown" ? toldConflict : detail.mergeability === "conflicting"; const commentsOnly = changes.length > 0 && changes.every((change) => change.kind === "remarks"); const progress = headMoved || (changes.length > 0 && !commentsOnly);