Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions apps/server/src/orchestration-v2/pullRequestWatch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" }),
Expand All @@ -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", () => {
Expand Down
9 changes: 6 additions & 3 deletions apps/server/src/orchestration-v2/pullRequestWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ export function evaluatePullRequestWatch(
now: number,
): PullRequestWatchReport {
const changes: Array<PullRequestWatchChange> = [];
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.
Expand Down Expand Up @@ -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;
Comment thread
saphid marked this conversation as resolved.
Resolved
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);
Expand Down
Loading