Fix stale pull request merge revisions

[gjkim42]

Summary

• Resolve the live refs/pull/<number>/merge ref for every open pull request event.
• Confirm the generated merge commit has the webhook head SHA as a parent.
• Retry stale or temporarily missing merge refs with increasing delays and fail the delivery after a bounded two-minute wait.
• Pin the first accepted merge SHA before workflow discovery so transient retries remain idempotent.

Root cause

GitHub can deliver a synchronize webhook before its pull_request.merge_commit_sha field catches up. In #9, the event identified the current pull request head, but Open Actions copied an older payload merge SHA into the WorkflowRun. The runner therefore checked out the prior revision and did not see the latest workflow changes.

Impact

Open pull request workflows now run against the merge revision built from the event head. Superseded deliveries cannot poll indefinitely, and an accepted revision cannot change midway through delivery processing. This uses the existing GitHub App Contents: read permission; no additional permission is required.

Validation

• make verify
• make test

[gjkim42]

/kelos claude-review

[kelos-bot]

🤖 Kelos Task Status

Task open-actions-claude-reviewer-issue-comment-d7093cbdc56d has succeeded. ✅

[kelos-bot]

🤖 Open Actions Claude Reviewer Agent @gjkim42

Review Summary

Verdict: APPROVE
Overall correctness: patch is correct
Scope: Resolve the live refs/pull/<n>/merge ref for open PR events, verify the webhook head SHA is a merge-commit parent, bound the wait for a current merge ref to two minutes with growing retry intervals, and pin the first accepted merge SHA in the delivery ConfigMap so retries stay idempotent.

Findings Overview

Priority	Count	File:Line	Summary
P0	0	—	none
P1	0	—	none
P2	1	README.md:233	Webhook delivery contract changed (live merge-ref resolution, bounded Failed timeout) without a README update
P3	2	internal/github/client.go:205	Second API call per poll is redundant; the commits-list response already includes parents
P3		internal/webhook/delivery.go:232	Missing merge ref (404) wait path has no reconciler-level test

Findings

Documentation

• [P2] README.md:233 — The webhook contract section still says only "Pull request deliveries whose merge ref is still being prepared are queued and resolved asynchronously", but this PR changes the behavior it documents: every open PR delivery now resolves the live merge ref (the payload merge_commit_sha is ignored), requires the webhook head SHA to be a parent of the merge commit, and terminates as Failed with an explanatory message after a bounded two-minute wait. Superseded deliveries (rapid successive pushes) now surface as Failed ConfigMaps retained for 24 hours, which operators will see; the README documents delivery states at exactly this level of detail, so it should describe the bounded wait and the failure outcome.

Code quality

• [P3] internal/github/client.go:205 — ResolvePullRequestRevision issues a second GET repos/{owner}/{repo}/git/commits/{sha} request just to read parents, but the GET repos/{owner}/{repo}/commits response that ResolveRevision already fetches includes each commit's parents array. Reading parents from the first response would remove one GitHub API call from every polling attempt — relevant because this code runs in a retry loop against the installation rate limit (currently token + 2 reads per attempt, up to ~15 attempts per stalled delivery).

Tests

• [P3] internal/webhook/delivery.go:232 — The missingPullRequestMergeRef branch (merge ref returns 404 — the common state right after opened or after the PR is closed) has no reconciler-level test: nothing verifies that a 404 produces a requeue rather than a reconcile error, and that a persistent 404 reaches the Failed timeout. The two new delivery tests exercise only the stale-parent path. Extending the timeout test's stub to return 404 for the merge ref would cover it.

Key takeaways

• Both blocking findings from the previous review round are resolved by 75f259a: the not-ready wait is now bounded by a two-minute deadline measured from the delivery ConfigMap's creation with growing retry intervals (2s/5s/15s), superseded and closed-PR deliveries reach a terminal Failed state with an explanatory message, and a closed PR's 404 merge ref folds into the same bounded wait instead of error-backoff retrying forever.
• The resolved merge SHA is persisted into the delivery ConfigMap before workflow discovery, so transient retries reuse the pinned revision instead of re-resolving — TestDeliveryPinsCurrentPullRequestMergeRevision proves resolution is not repeated after a mid-delivery failure and that the WorkflowRun carries the pinned SHA.
• The remaining items are non-blocking: a README update for the changed delivery semantics, one saved API call per poll, and a small test gap on the 404 wait path.

[gjkim42]

/kelos claude-review

[kelos-bot]

🤖 Kelos Task Status

Task open-actions-claude-reviewer-issue-comment-a7a19973aba5 has succeeded. ✅