Skip to content

Commit 57c0909

Browse files
fix(webhooks): heal pr_state from closed to open on synchronize after missed reopen (#4424)
The pr_state monotonic guard treated 'closed' as sticky except for the exact 'reopened' action. If a reopened webhook was missed or deduplicated, subsequent events like synchronize (new commits pushed) never flipped pr_state back to open, leaving the UI stuck showing 'closed' for an actually-open PR. Synchronize deliveries carrying a new head sha (evidence of an actual new push, which can only happen on a genuinely open PR) now heal pr_state from closed back to open/draft. A synchronize redelivery with an unchanged head sha is still treated as a stale/duplicate event and does not heal the state, preserving existing out-of-order-delivery protections. Co-authored-by: eshurakov <54751+eshurakov@users.noreply.github.com> Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
1 parent 9fea41f commit 57c0909

2 files changed

Lines changed: 84 additions & 0 deletions

File tree

‎apps/web/src/lib/integrations/platforms/github/webhook-handlers/upsert-cli-session-pull-requests.test.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,76 @@ describe('upsertCliSessionPullRequestsFromWebhook', () => {
644644
expect(rows[0].pr_head_sha).toBe('sha-203-reopen');
645645
});
646646

647+
it('heals pr_state from closed -> open on synchronize when the reopened webhook was missed', async () => {
648+
const branch = 'feature/missed-reopen-then-sync';
649+
await seedSession({ branch, owner: testOwner });
650+
651+
// PR gets closed.
652+
await upsertCliSessionPullRequestsFromWebhook(
653+
makePayload({
654+
action: 'closed',
655+
prNumber: 210,
656+
state: 'closed',
657+
merged: false,
658+
headRef: branch,
659+
headSha: 'sha-210',
660+
}),
661+
testOwner
662+
);
663+
664+
// The `reopened` webhook for this PR is never delivered (missed/deduped),
665+
// but new commits are pushed to the now-actually-open PR, so GitHub
666+
// delivers a `synchronize` event carrying a new head sha and the PR's
667+
// real current state (open).
668+
await upsertCliSessionPullRequestsFromWebhook(
669+
makePayload({
670+
action: 'synchronize',
671+
prNumber: 210,
672+
state: 'open',
673+
headRef: branch,
674+
headSha: 'sha-210-new-commit',
675+
}),
676+
testOwner
677+
);
678+
679+
const rows = await readUserRow({ userId: testUserId, branch });
680+
expect(rows[0].pr_state).toBe('open');
681+
expect(rows[0].pr_head_sha).toBe('sha-210-new-commit');
682+
});
683+
684+
it('does not heal pr_state from closed -> open on a stale synchronize redelivery with the same head sha', async () => {
685+
const branch = 'feature/stale-sync-same-sha';
686+
await seedSession({ branch, owner: testOwner });
687+
688+
await upsertCliSessionPullRequestsFromWebhook(
689+
makePayload({
690+
action: 'closed',
691+
prNumber: 211,
692+
state: 'closed',
693+
merged: false,
694+
headRef: branch,
695+
headSha: 'sha-211',
696+
}),
697+
testOwner
698+
);
699+
700+
// A redelivered/duplicate `synchronize` event for the same commit is not
701+
// evidence of a new push, so pr_state stays closed.
702+
await upsertCliSessionPullRequestsFromWebhook(
703+
makePayload({
704+
action: 'synchronize',
705+
prNumber: 211,
706+
state: 'open',
707+
headRef: branch,
708+
headSha: 'sha-211',
709+
}),
710+
testOwner
711+
);
712+
713+
const rows = await readUserRow({ userId: testUserId, branch });
714+
expect(rows[0].pr_state).toBe('closed');
715+
});
716+
647717
it('does not regress pr_state from merged -> closed on stale closed-unmerged redelivery', async () => {
648718
const branch = 'feature/monotonic-merged-closed';
649719
await seedSession({ branch, owner: testOwner });

‎apps/web/src/lib/integrations/platforms/github/webhook-handlers/upsert-cli-session-pull-requests.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,19 @@ export async function upsertCliSessionPullRequestsFromWebhook(
145145
const state = derivePrState(pull_request, action);
146146

147147
// Defense-in-depth against out-of-order webhook deliveries.
148+
//
149+
// `closed` must not be permanently sticky: if the `reopened` webhook that
150+
// should have flipped it back to open is missed or deduplicated, a later
151+
// `synchronize` delivery carrying a new head sha (i.e. new commits were
152+
// actually pushed, which can only happen on a PR that is really open) is
153+
// used to self-heal pr_state instead of trusting only the `reopened`
154+
// action name. A `synchronize` redelivery with the *same* head sha is
155+
// treated as a stale/duplicate event and does not heal the state.
156+
const closedStaysStickyUnlessNewCommit =
157+
action === GITHUB_ACTION.SYNCHRONIZE
158+
? sql`AND ${github_branch_pull_requests.pr_head_sha} = excluded.pr_head_sha`
159+
: sql``;
160+
148161
const prStateSet =
149162
action === GITHUB_ACTION.REOPENED
150163
? sql`excluded.pr_state`
@@ -156,6 +169,7 @@ export async function upsertCliSessionPullRequestsFromWebhook(
156169
WHEN ${github_branch_pull_requests.pr_number} = excluded.pr_number
157170
AND ${github_branch_pull_requests.pr_state} = 'closed'
158171
AND excluded.pr_state IN ('open', 'draft')
172+
${closedStaysStickyUnlessNewCommit}
159173
THEN ${github_branch_pull_requests.pr_state}
160174
ELSE excluded.pr_state
161175
END`;

0 commit comments

Comments
 (0)