fix(finish): detect a CI-gated merge and wait (backgroundable) instead of failing #438

Closed
opened 2026-08-17 21:50:56 +00:00 by hexajon · 2 comments
Owner

madtea finish does commit + push + PR + merge + return-to-mainline in one call. On a repo with branch protection that requires a status check, the merge step fails: the forge rejects it with 405 "not all required status checks successful", because the required CI check has only just started when finish tries to merge.

By then finish has already committed, pushed, and opened the PR, so no work is lost. But the merge and the return-to-mainline do not happen. The caller is dropped back into the exact manual flow finish exists to remove: watch the run, merge the PR by hand once it is green, then pull and delete the branch. On every protected-branch repo, finish is a partial operation.

The consolidated finish flow simplified the common case, but the common case for anyone shipping real work is a protected default branch with required CI. That is precisely where finish stops short.

What it should do

  1. Detect the gated-merge case. Distinguish "merge blocked, required checks still pending" from a genuine merge failure (conflict, denied permission). The 405 reason string and the PR head's combined check status both carry this.
  2. On a gated merge, do not fail - wait. Poll the required checks, report progress inline (check name + state), then complete the merge and the return-to-mainline once they pass. On a check failure, stop and report which check failed.
  3. Make the wait backgroundable. CI can run for many minutes (say 30). Blocking the whole time is unacceptable. Offer a non-blocking path: either schedule a forge-side auto-merge-on-green and return immediately, or run the wait as a backgroundable task the caller can leave and be notified about.

Acceptance

  • finish on a branch-protected repo with required CI completes the full commit -> merge -> mainline flow, or cleanly schedules an auto-merge and returns - no manual prs merge + pull + branch-delete needed.
  • The gated case reports as "waiting on CI", not a hard error.
  • A long-running check does not force a long-blocking call: a background / auto-merge option exists.
  • A real merge failure (conflict, denied) is still reported as an error, never silently waited on.

Repro

  • Any repo whose default branch requires a status check before merge.
  • Run madtea finish from a feature branch. The push and PR succeed; the merge is rejected with 405 "not all required status checks successful" while the check is still running.
`madtea finish` does commit + push + PR + merge + return-to-mainline in one call. On a repo with branch protection that requires a status check, the merge step fails: the forge rejects it with `405 "not all required status checks successful"`, because the required CI check has only just started when finish tries to merge. By then finish has already committed, pushed, and opened the PR, so no work is lost. But the merge and the return-to-mainline do not happen. The caller is dropped back into the exact manual flow finish exists to remove: watch the run, merge the PR by hand once it is green, then pull and delete the branch. On every protected-branch repo, finish is a partial operation. The consolidated finish flow simplified the common case, but the common case for anyone shipping real work is a protected default branch with required CI. That is precisely where finish stops short. ## What it should do 1. **Detect the gated-merge case.** Distinguish "merge blocked, required checks still pending" from a genuine merge failure (conflict, denied permission). The 405 reason string and the PR head's combined check status both carry this. 2. **On a gated merge, do not fail - wait.** Poll the required checks, report progress inline (check name + state), then complete the merge and the return-to-mainline once they pass. On a check failure, stop and report which check failed. 3. **Make the wait backgroundable.** CI can run for many minutes (say 30). Blocking the whole time is unacceptable. Offer a non-blocking path: either schedule a forge-side auto-merge-on-green and return immediately, or run the wait as a backgroundable task the caller can leave and be notified about. ## Acceptance - `finish` on a branch-protected repo with required CI completes the full commit -> merge -> mainline flow, or cleanly schedules an auto-merge and returns - no manual `prs merge` + `pull` + branch-delete needed. - The gated case reports as "waiting on CI", not a hard error. - A long-running check does not force a long-blocking call: a background / auto-merge option exists. - A real merge failure (conflict, denied) is still reported as an error, never silently waited on. ## Repro - Any repo whose default branch requires a status check before merge. - Run `madtea finish` from a feature branch. The push and PR succeed; the merge is rejected with `405 "not all required status checks successful"` while the check is still running.
Author
Owner

Landed in PR #464. A new gitea.IsMergeRequiredChecksPending predicate catches the CI-gated 405, and MergeWithRetry re-issues the merge with merge_when_checks_succeed so the forge merges on green, returning an ErrAutoMergeScheduled sentinel. Every merge caller honors it - finish (local and remote), pr merge, the MCP prs merge action, and orchestrate batch-merge and revert - each reporting the scheduled merge and skipping return-to-mainline (no pull, no branch delete). Design: forge-side auto-merge-on-green, not a blocking poll (the issue offered either).

Two assumptions ride the live forge and cannot be tested hermetically, so they want confirming on the first real CI-gated finish:

  1. The 405 body wording. The predicate matches "not all required status checks successful" (the wording this issue quotes) and the broader "required status check". If a forge worded it differently, the 405 falls through to the pre-#438 behavior - a clear failure, no lost work.
  2. A 2xx on the schedule POST means the auto-merge actually queued. A failed schedule POST surfaces verbatim rather than a false "scheduled", but a 2xx-that-does-not-queue would report scheduled while nothing merges. The shipped --auto flag already relies on 2xx-means-queued.

Also worth flagging: batch-merge and revert now schedule-and-disclose on a CI-gated 405 too, replacing the old behavior that mislabeled it as a moved base and failed with the wrong message. That is the correct, consistent handling, but it is a behavior change on those paths.

Keeping this open on the two live-forge checks.

Landed in PR #464. A new gitea.IsMergeRequiredChecksPending predicate catches the CI-gated 405, and MergeWithRetry re-issues the merge with merge_when_checks_succeed so the forge merges on green, returning an ErrAutoMergeScheduled sentinel. Every merge caller honors it - finish (local and remote), pr merge, the MCP prs merge action, and orchestrate batch-merge and revert - each reporting the scheduled merge and skipping return-to-mainline (no pull, no branch delete). Design: forge-side auto-merge-on-green, not a blocking poll (the issue offered either). Two assumptions ride the live forge and cannot be tested hermetically, so they want confirming on the first real CI-gated finish: 1. The 405 body wording. The predicate matches "not all required status checks successful" (the wording this issue quotes) and the broader "required status check". If a forge worded it differently, the 405 falls through to the pre-#438 behavior - a clear failure, no lost work. 2. A 2xx on the schedule POST means the auto-merge actually queued. A failed schedule POST surfaces verbatim rather than a false "scheduled", but a 2xx-that-does-not-queue would report scheduled while nothing merges. The shipped --auto flag already relies on 2xx-means-queued. Also worth flagging: batch-merge and revert now schedule-and-disclose on a CI-gated 405 too, replacing the old behavior that mislabeled it as a moved base and failed with the wrong message. That is the correct, consistent handling, but it is a behavior change on those paths. Keeping this open on the two live-forge checks.
Author
Owner

Verified live on the forge, 2026-09-09, both assumptions the fix in PR #464 rode on.

Setup: a temporary protected branch verify-438-base on this repo with enable_status_check and required context gate-438 (nothing reports it), a one-file branch verify-438-head, and PR #477 between them. All three are removed again.

  1. The 405 wording. The forge returned 405 {"message":"not allowed to merge [reason: Not all required status checks successful]"}. gitea.IsMergeRequiredChecksPending (internal/gitea/client_errors.go) matches it case-insensitively, and a binary built from main did: madtea pr merge 477 printed "Required status checks are still running; scheduled an auto-merge that completes when they pass" and "Enabled auto-merge for pull request #477", exit 0, no manual merge and no branch update attempted.

  2. A 2xx on the schedule request queues a real auto-merge. I then posted a success status for context gate-438 on the head sha by hand. The forge merged PR #477 on its own at 22:20:21Z (merged: true, merged_by: hexajon, merge commit equals the head sha). No further madtea call was involved, so the 2xx did queue the merge.

Also confirmed the negative case by accident: a stale 0.18.3 binary sent the same 405 down the old behind-base path ("branch update failed ... HeadBranch of PR 477 is up to date"), which is exactly the pre-fix behaviour this issue describes. Current binaries carry the fix; the release v0.18.5 includes it.

Verified live on the forge, 2026-09-09, both assumptions the fix in PR #464 rode on. Setup: a temporary protected branch `verify-438-base` on this repo with `enable_status_check` and required context `gate-438` (nothing reports it), a one-file branch `verify-438-head`, and PR #477 between them. All three are removed again. 1. The 405 wording. The forge returned `405 {"message":"not allowed to merge [reason: Not all required status checks successful]"}`. `gitea.IsMergeRequiredChecksPending` (`internal/gitea/client_errors.go`) matches it case-insensitively, and a binary built from main did: `madtea pr merge 477` printed "Required status checks are still running; scheduled an auto-merge that completes when they pass" and "Enabled auto-merge for pull request #477", exit 0, no manual merge and no branch update attempted. 2. A 2xx on the schedule request queues a real auto-merge. I then posted a `success` status for context `gate-438` on the head sha by hand. The forge merged PR #477 on its own at 22:20:21Z (`merged: true`, `merged_by: hexajon`, merge commit equals the head sha). No further madtea call was involved, so the 2xx did queue the merge. Also confirmed the negative case by accident: a stale 0.18.3 binary sent the same 405 down the old behind-base path ("branch update failed ... HeadBranch of PR 477 is up to date"), which is exactly the pre-fix behaviour this issue describes. Current binaries carry the fix; the release v0.18.5 includes it.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
sixfold-space/madtea#438
No description provided.