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
12 changes: 4 additions & 8 deletions src/github/webhook-coalesce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,10 @@ const COALESCABLE_PULL_REQUEST_ACTIONS = new Set([
const COALESCABLE_PULL_REQUEST_LABEL_ACTIONS = new Set(["labeled", "unlabeled"]);

// #selfhost-backlog-convergence: mirrors shouldProcessPullRequestPublicSurface's (processors.ts) own action
// allowlist per event -- these three event types are the ones that trigger a full re-review pipeline run for a
// PR (readiness/review/publish), so a burst of review activity on the same head (e.g. several reviewers
// submitting close together, or one reviewer leaving many inline comments) fans out one FULL re-review per
// delivery today. The re-review pipeline always re-fetches live review/comment state from GitHub rather than
// acting on the specific webhook payload's content, so collapsing a burst into one job loses nothing -- exactly
// the same safety property the existing pull_request coalescing above already relies on.
// allowlist for review comments/threads. Unlike `pull_request_review`, these events do not carry review-cache
// invalidation or changes-requested notification side effects, so only payload-interchangeable event families
// may coalesce with each other.
const REVIEW_SURFACE_ACTIONS_BY_EVENT: Record<string, ReadonlySet<string>> = {
pull_request_review: new Set(["submitted", "edited", "dismissed"]),
pull_request_review_comment: new Set(["created", "edited", "deleted"]),
pull_request_review_thread: new Set(["resolved", "unresolved"]),
};
Expand Down Expand Up @@ -78,7 +74,7 @@ export function githubWebhookCoalesceKey(
const pr = normalizedNumber(payload.pull_request?.number);
const headSha = normalizedSha(payload.pull_request?.head?.sha);
return pr !== null
? `github-webhook:pr-review:${repo}#${pr}${headSha ? `@${headSha}` : ""}`
? `github-webhook:${eventName}:${repo}#${pr}${headSha ? `@${headSha}` : ""}`
: null;
}
return null;
Expand Down
50 changes: 28 additions & 22 deletions test/unit/github-webhook-coalesce.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,15 +129,15 @@ describe("githubWebhookCoalesceKey", () => {
).toBeNull();
});

it("coalesces review-surface bursts (pull_request_review) into one job per PR+head (#selfhost-backlog-convergence)", () => {
it("does not coalesce pull_request_review events because their payloads drive invalidation and notifications", () => {
for (const action of ["submitted", "edited", "dismissed"]) {
expect(
githubWebhookCoalesceKey("pull_request_review", {
action,
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12, head: { sha: "CAFE123" } },
} as GitHubWebhookPayload),
).toBe("github-webhook:pr-review:jsonbored/gittensory#12@cafe123");
).toBeNull();
}
});

Expand All @@ -149,7 +149,7 @@ describe("githubWebhookCoalesceKey", () => {
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12, head: { sha: "CAFE123" } },
} as GitHubWebhookPayload),
).toBe("github-webhook:pr-review:jsonbored/gittensory#12@cafe123");
).toBe("github-webhook:pull_request_review_comment:jsonbored/gittensory#12@cafe123");
}
});

Expand All @@ -161,39 +161,45 @@ describe("githubWebhookCoalesceKey", () => {
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12, head: { sha: "CAFE123" } },
} as GitHubWebhookPayload),
).toBe("github-webhook:pr-review:jsonbored/gittensory#12@cafe123");
).toBe("github-webhook:pull_request_review_thread:jsonbored/gittensory#12@cafe123");
}
});

it("coalesces a mixed burst of review/comment/thread events for the same PR+head into ONE key", () => {
const burstKeys = [
["pull_request_review", "submitted"],
["pull_request_review_comment", "created"],
["pull_request_review_thread", "resolved"],
].map(([eventName, action]) =>
githubWebhookCoalesceKey(eventName as string, {
action,
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12, head: { sha: "CAFE123" } },
} as GitHubWebhookPayload),
);
expect(new Set(burstKeys).size).toBe(1);
it("keeps distinct review-surface event families separate so coalescing cannot drop review-only side effects", () => {
const reviewKey = githubWebhookCoalesceKey("pull_request_review", {
action: "submitted",
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12, head: { sha: "CAFE123" } },
} as GitHubWebhookPayload);
const commentKey = githubWebhookCoalesceKey("pull_request_review_comment", {
action: "created",
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12, head: { sha: "CAFE123" } },
} as GitHubWebhookPayload);
const threadKey = githubWebhookCoalesceKey("pull_request_review_thread", {
action: "resolved",
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12, head: { sha: "CAFE123" } },
} as GitHubWebhookPayload);

expect(reviewKey).toBeNull();
expect(commentKey).not.toBe(threadKey);
});

it("omits the head sha suffix for a review-surface event with no resolvable head", () => {
expect(
githubWebhookCoalesceKey("pull_request_review", {
action: "submitted",
githubWebhookCoalesceKey("pull_request_review_comment", {
action: "created",
repository: { full_name: "JSONbored/Gittensory" },
pull_request: { number: 12 },
} as GitHubWebhookPayload),
).toBe("github-webhook:pr-review:jsonbored/gittensory#12");
).toBe("github-webhook:pull_request_review_comment:jsonbored/gittensory#12");
});

it("returns null for a review-surface event with no resolvable PR number", () => {
expect(
githubWebhookCoalesceKey("pull_request_review", {
action: "submitted",
githubWebhookCoalesceKey("pull_request_review_comment", {
action: "created",
repository: { full_name: "JSONbored/Gittensory" },
} as GitHubWebhookPayload),
).toBeNull();
Expand Down
Loading