diff --git a/CHANGELOG.md b/CHANGELOG.md index ab37b22..3a8df84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ include the full version, for example `## 0.7.0-beta.1`. ### Fixed +- Honor formal approvals for the exact verified PR commit even when publication + or description recovery finishes later. Retain review revocation, authorization, + and merge-readiness checks; keep timestamp gating for unbound merge comments. + - Retry PR head propagation after a successful push when the remote branch still matches the verified commit. Preserve a durable push checkpoint across restarts, avoid repeating acknowledged pushes, and distinguish closed PRs from changed diff --git a/docs/advanced.md b/docs/advanced.md index 67b443a..a10e539 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -297,7 +297,7 @@ session and verification/report snapshots. `controlVersion` increases on operato transitions, so stale TUI events cannot undo explicit resumption. Watching continues PR-state discovery and accepts new authorized issue comments. -`publishedHead` retains the successfully published SHA and approval-window time +`publishedHead` retains the successfully published SHA and merge-comment window time across rounds; merge checks use it while watching. No saved published head means no automatic merge until the next successful publication. Old round checkpoints are not treated as a new successful publication. Explicit resumption validates diff --git a/docs/architecture.md b/docs/architecture.md index a2356e4..6e82828 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -45,6 +45,8 @@ loads a generic scheduler, a GitHub dispatcher, and a terminal UI component. preserve manual notes outside the managed description. 7. Merge only after eligible approval of the published head, repository permission checks, and GitHub merge readiness checks. Post a signed acknowledgement. + Formal reviews are bound to the exact commit and survive later description + recovery; unbound merge comments must follow the latest publication. A failure retains the current phase and retry state. An eligible `running` task with a saved session takes priority over other ready tasks. Unknown prompt diff --git a/docs/bot-workflow.md b/docs/bot-workflow.md index b9918d5..5ef862d 100644 --- a/docs/bot-workflow.md +++ b/docs/bot-workflow.md @@ -557,7 +557,7 @@ flowchart TD Idle[Worker has no eligible execution task] --> Eligible{Auto-merge enabled and done or watching task with published head eligible?} Eligible -->|No| Later[Wait for a later worker pass] Eligible -->|Yes| Since{publishedAt exists?} - Since -->|No| Window[Record current time as fresh approval window] + Since -->|No| Window[Record current time as fresh merge-comment window] Window --> Later Since -->|Yes| Scan[Scan again before considering merge] Scan --> Fresh{Pending feedback or closed PR?} @@ -567,7 +567,7 @@ flowchart TD Already -->|Yes| Ack[Post or reconcile signed merge acknowledgement, persist merged and closed PR] Already -->|No| Head{Open, non-draft PR with saved published head?} Head -->|No| Poll[Clear mergeError, set mergeNextAt at least 60 seconds later] - Head -->|Yes| Review[Evaluate latest decisive reviews and exact approval comments] + Head -->|Yes| Review[Match latest decisive reviews to exact SHA, match merge comments after publication] Review --> Author{No outstanding changes request and eligible approver has write, maintain or admin access?} Author -->|No| Poll Author -->|Yes| Ready{mergeable and mergeable_state clean?} @@ -602,23 +602,27 @@ flowchart TD Merge eligibility requires `done`, or `watching` with a saved `publishedHead`, a tracked nonclosed PR, a saved published commit, no merged flag, no pending feedback, and an elapsed `mergeNextAt`. Missing -`publishedAt` in an older queue starts a fresh approval window rather than using -historical approval. Merge checks run when the worker has no execution task to +`publishedAt` in an older queue starts a fresh window for merge comments, which +have no commit binding. Formal reviews still require the exact verified SHA. +Merge checks run when the worker has no execution task to advance, rather than immediately after every publication. For each reviewer, the latest `APPROVED`, `CHANGES_REQUESTED`, or `DISMISSED` review is decisive. Any outstanding changes request suppresses all approval candidates, including comment approvals. An approval review must reference the -current verified SHA and have been submitted after `publishedAt`. An approval -comment must have been created after that time and match a configured phrase +current verified SHA and have a valid submission timestamp; it may precede +`publishedAt`. Finishing or retrying a PR description update does not invalidate +approval of unchanged code. An approval comment must have been created after +`publishedAt` and match a configured phrase as a whole message after case, whitespace, and trailing `.`/`!` normalization. Bot comments and marked automation comments are excluded. Default phrases are `/merge`, `lgtm, merge`, and `approved, merge`; default merge method is `squash`. The permission check then requires an allowlisted candidate with repository write, maintain, or admin access. GitHub still enforces merge requirements. -Every successful round updates `publishedAt`, so old approvals cannot authorize -the next published round. When the approval method returns false (for example, +Every successful round updates `publishedAt`, so old merge comments cannot +authorize the next published round. Formal reviews remain valid for the same +commit only; a different published SHA requires a matching review. When the approval method returns false (for example, no eligible approval or a mismatched head), the dispatcher clears `mergeError` and schedules another check after 60 seconds. An approved PR that GitHub says is not ready, a rejected merge, or a request failure records `mergeError`; error retries also respect GitHub timing. diff --git a/docs/configuration.md b/docs/configuration.md index 4bbe8ba..1488da3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -156,8 +156,12 @@ verified head SHA; a changed branch cannot be merged using an older approval. The bot does not request a protection bypass. Configure required checks and review rules on GitHub for your repository's policy. -Approvals must be newer than the bot's latest publication. On upgrade, old tasks -start watching for new approvals; historical approvals do not cause a merge. +Formal **Approve** reviews must reference the exact published commit and remain +the reviewer's latest decisive review. They can precede completion of publication: +retrying a description update does not invalidate approval of unchanged code. +Merge comments have no commit binding, so they must be newer than the bot's latest +publication. Old tasks without a publication timestamp start a fresh comment +window on upgrade; existing reviews still require the exact verified commit. Pending issue feedback is processed before attempting a merge. Merge failures are retried at intervals of at least 60 seconds and appear as `mergeError` in `status` and in `/bot`. Successful merges receive a signed PR comment. diff --git a/docs/runtime.md b/docs/runtime.md index fe68166..ca5a63f 100644 --- a/docs/runtime.md +++ b/docs/runtime.md @@ -431,6 +431,12 @@ durable as before. ## PR descriptions +A formal GitHub **Approve** review applies to its exact commit even when submitted +before the bot finishes updating the PR description. Retrying publication does +not invalidate that review. A new commit needs a matching review, and dismissed +approvals or outstanding change requests are not accepted. Plain merge comments +still must follow the latest publication because they do not identify a commit. + After pushing a verified commit, GitHub may briefly show the previous commit in the PR. If the remote branch already matches the verified commit, the bot waits and automatically retries publication within its configured attempt limit. It diff --git a/src/approval.ts b/src/approval.ts index 2cc3c2e..770320f 100644 --- a/src/approval.ts +++ b/src/approval.ts @@ -12,7 +12,10 @@ export function approvalAuthors(reviews: Review[], comments: DatedComment[], hea } // An outstanding request for changes takes precedence over a merge comment. if ([...latest.values()].some(r => r.state === "CHANGES_REQUESTED")) return []; - const authors = [...latest.values()].filter(r => r.state === "APPROVED" && r.commit_id === head && Date.parse(r.submitted_at ?? "") > since).map(r => r.user.login); + // Reviews are bound to an exact commit. Description/publication retries must + // not invalidate an approval already submitted for that same verified head. + // Plain merge comments have no commit binding and still need the time window. + const authors = [...latest.values()].filter(r => r.state === "APPROVED" && r.commit_id === head && Number.isFinite(Date.parse(r.submitted_at ?? ""))).map(r => r.user.login); for (const comment of comments) { if (comment.user.type === "Bot" || comment.body.includes("