GH-5263: fix(autopilot): merging stage attempts to merge draft PRs and ignores CHANGES_REQUESTED reviews — burns attempts, trips breaker, and only the draft flag prevents merging over a reviewer

[alekspetrov]

Summary

Automated PR created by Pilot for task GH-5263.

Closes #5263

Changes

GitHub Issue GH-5263: fix(autopilot): merging stage attempts to merge draft PRs and ignores CHANGES_REQUESTED reviews — burns attempts, trips breaker, and only the draft flag prevents merging over a reviewer

Problem

Live incident 2026-08-30 ~13:47-13:52Z, founder box, PR#5258. The autopilot merging stage attempted to merge a DRAFT PR three times, failed each time with GitHub 405 "Pull Request is still a draft", and tripped the per-PR circuit breaker — producing a failure alert for what is actually a healthy, deliberately-held PR.

Ledger evidence (autopilot_pr_state + daemon.log): stage=merging, "merge attempt 3 failed: ... status 405 ... still a draft"; log sequence per attempt is "verifyCIBeforeMerge: CI passed" → "handleMerging: merge failed" → "autopilot stage failed", then "per-PR circuit breaker open" every tick after.

Context: PR#5258 had green CI, then received a REQUEST-CHANGES review (2026-08-29 17:20Z) which spawned revision issue #5261 and converted the PR to draft. When #5261 began executing (13:47Z), the controller re-processed PR#5258 and went straight to merging.

Two defects

1. The merge path never checks draft status. verifyCIBeforeMerge passes and handleMerging calls the merge API blind; every attempt is a guaranteed 405 that burns the attempt budget and feeds the breaker. A draft is a deliberate hold — it should park the PR (no attempts, no breaker feed, no failure alert), and resume automatically when the PR is marked ready.

2. The merge path never checks review state. On this repo (no branch protection) the ONLY thing that stopped a PR with an outstanding REQUEST-CHANGES review from auto-merging was the draft flag that the revision flow happens to set. If a REQUEST-CHANGES review lands without the revision flow drafting the PR (race, manual review without the trigger phrase, revision-spawn failure), the next green tick merges over the reviewer's objection. The merge gate should treat an outstanding changes-requested review as a hold until re-review or dismissal, independent of draft state.

Fix shape

In the merging stage (and ideally as a pre-check in verifyCIBeforeMerge's caller):

• Query PR draft status; if draft → do not attempt, do not count an attempt, do not feed the breaker; log a hold at INFO and re-check next tick (drafts flip to ready via a webhook/poll-visible field — no new event plumbing needed).
• Query review state; if the latest review by any maintainer is CHANGES_REQUESTED and not dismissed/superseded by a later APPROVE → same hold semantics, distinct log reason.
• The existing 405-on-draft failure path stays as a backstop but should be unreachable.

Acceptance

• A draft PR at stage merging is parked with zero merge attempts, zero breaker feeds, zero failure alerts; marking it ready resumes the merge on the next tick (test both).
• A non-draft PR with an outstanding CHANGES_REQUESTED review is parked the same way; a subsequent APPROVE or review dismissal unparks it (test both).
• Genuine merge failures (conflicts, real API errors) still count attempts and feed the breaker exactly as today.
• The PR#5258 shape (draft + changes-requested + green CI) reproduced in a table-driven test: no attempt made.

Refs

• Incident: PR#5258 / revision fix(briefs): PR#5258 revision — receipts digest window drops in-flight/post-18:00 runs permanently #5261 (2026-08-30) · breaker semantics GH-5236: fix(autopilot): platform-outage breaker is blind to the outage's actual shape — CI timeouts and missing checks never feed correlation #5237/fix(autopilot): post-merge CI treated absent signal as success and never fed the breaker #5239 · revision-cycle drafting behavior (review-feedback loop) · TASK-460 family (a failure alert on a healthy hold is false evidence)

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[alekspetrov]

Post-merge review verdict: APPROVE-w-notes

Verified adversarially at origin/main; build + autopilot suite green.

• Guards correctly placed: both checks sit at the top of handleMerging, before MergeAttempts++ and before the merge call; the hold is a nil return (no failure record, no breaker feed, no alert), and hold ticks actively clear an existing failure counter via resetPRFailures. Distinct INFO reasons per spec.
• Real park, fresh reads: stage stays merging; draft status and reviews are re-fetched per tick (nothing cached on prState) — resume verified by tests that mutate the mock between ticks.
• No revision-cycle deadlock, two independent reasons: the standard revision flow closes the old PR (a revision lands as a NEW PR with empty review history), and live-checked: PR#5258 has zero formal GitHub reviews (the founder review was comment-based), so it passes both guards. Where a formal CHANGES_REQUESTED stands un-dismissed, the hold persists until APPROVE/dismissal — exactly as specified; the ReviewFeedback poller intercepts first in the normal path, making this a backstop.
• PR#5258 burned state self-recovers: no explicit reset shipped, but the breaker auto-resets on FailureResetTimeout (default 30m, long past), the first hold tick clears counters, and MergeAttempts only matters in the failure branch.
• Tests catch the adversarial bugs: full-ProcessPR table asserting zero attempts/failures/breaker; resume tests; the exact PR#5258 shape; genuine-failure case still feeds attempts+breaker.

Notes (both pre-existing in the reused review logic, filed as follow-up):

• N1: hasChangesRequested ignores reviews older than prState.CreatedAt, and both registration paths set CreatedAt=now — so after a daemon restart or orphan re-adoption, a standing CHANGES_REQUESTED older than the adoption is invisible and the PR merges over it. Within a tracked lifetime the protection holds; across re-adoption it doesn't.
• N2: the latest-per-user map lets a later COMMENTED review supersede CHANGES_REQUESTED — GitHub itself doesn't treat COMMENTED as superseding. Fail-open.
• Minor: fail-open on GetPullRequest error can burn one attempt during API flakiness (documented, mirrors precedent); one extra GET + review-list per merging tick.