cmoriarty/braid
1
0
Fork 0
Code Issues 15 Pull requests Projects Releases Packages 1 Wiki Activity Actions

git.pr.merge doesn't wait for required checks, and reports Forgejo's refusal as a moved branch #129

New issue
Closed
opened 2026-10-02 17:49:49 -04:00 by cmoriarty · 1 comment
cmoriarty commented 2026-10-02 17:49:49 -04:00
Owner
Copy link

Run 38 (quick-fix on cmoriarty/soundcheck#71, the check of #123's fix) opened cmoriarty/soundcheck#72 at about 19:46 UTC on 2026-10-02. Auto-approve answered the merge gate at once, and git.pr.merge asked Forgejo to merge a minute later. Forgejo refused because the required check CI / full (pull_request) was still running. That check takes about 19 minutes. The step's output:

git.pr.merge refused for #72: not allowed to merge [reason: Not all required status checks successful]
error: Forgejo refused to merge cmoriarty/soundcheck#72: the branch moved or it cannot be merged cleanly

The run failed there (no non-empty file matched: .osf/merged.json). The check then failed on its own after 18m49s, although the run's summary reports npm run test:fast and svelte-check green. Nobody has looked at why it failed yet. The pull request is still open.

Two problems

  1. The merge does not wait for required checks. merge_pull_request (src/osf/forgejo.py) merges as soon as the gate is answered. With auto-approve on, that is seconds after the pull request opens. So in any repository with a required check, the merge is refused every time, before the check can finish.
  2. The refusal gives the wrong reason. merge_pr returns False for any 405 or 409 and logs Forgejo's message only as a warning. The step's error then guesses "the branch moved or it cannot be merged cleanly". But the branch had not moved, because the step checks the head sha itself first, and Forgejo had said why it refused.

What to change

  • Wait for the head commit's required checks before merging. Keep waiting while they are still running, with a cap only as a backstop. Most of the pieces exist already:
    • the ci-conclusion predicate polls GET /commits/{sha}/status (src/osf/pipeline/ci.py);
    • the pipeline YAML has a drafted but inactive ci.full.dispatch / ci.full.await pair;
    • Forgejo's merge API takes merge_when_checks_succeed, which is the "When checks succeed" button on the pull request page.
  • When a required check fails, fail the step with the check's name and a link to its log. Don't ask for a merge that will be refused.
  • Put Forgejo's own message in the step's error, for every refusal.
Run 38 (`quick-fix` on cmoriarty/soundcheck#71, the check of #123's fix) opened cmoriarty/soundcheck#72 at about 19:46 UTC on 2026-10-02. Auto-approve answered the merge gate at once, and `git.pr.merge` asked Forgejo to merge a minute later. Forgejo refused because the required check `CI / full (pull_request)` was still running. That check takes about 19 minutes. The step's output: ``` git.pr.merge refused for #72: not allowed to merge [reason: Not all required status checks successful] error: Forgejo refused to merge cmoriarty/soundcheck#72: the branch moved or it cannot be merged cleanly ``` The run failed there (`no non-empty file matched: .osf/merged.json`). The check then failed on its own after 18m49s, although the run's summary reports `npm run test:fast` and svelte-check green. Nobody has looked at why it failed yet. The pull request is still open. ## Two problems 1. **The merge does not wait for required checks.** `merge_pull_request` (`src/osf/forgejo.py`) merges as soon as the gate is answered. With auto-approve on, that is seconds after the pull request opens. So in any repository with a required check, the merge is refused every time, before the check can finish. 2. **The refusal gives the wrong reason.** `merge_pr` returns `False` for any 405 or 409 and logs Forgejo's message only as a warning. The step's error then guesses "the branch moved or it cannot be merged cleanly". But the branch had not moved, because the step checks the head sha itself first, and Forgejo had said why it refused. ## What to change - **Wait for the head commit's required checks before merging.** Keep waiting while they are still running, with a cap only as a backstop. Most of the pieces exist already: - the `ci-conclusion` predicate polls `GET /commits/{sha}/status` (`src/osf/pipeline/ci.py`); - the pipeline YAML has a drafted but inactive `ci.full.dispatch` / `ci.full.await` pair; - Forgejo's merge API takes `merge_when_checks_succeed`, which is the "When checks succeed" button on the pull request page. - **When a required check fails, fail the step with the check's name and a link to its log.** Don't ask for a merge that will be refused. - **Put Forgejo's own message in the step's error,** for every refusal.
cmoriarty commented 2026-10-02 22:38:25 -04:00
Author
Owner
Copy link

Shipped in d94cfae, deployed on 2026-10-02 at 22:17. Archived in 5d67ff9 as openspec/changes/archive/2026-10-02-merge-waits-for-checks.

What changed

  • git.pr.merge still asks for the merge at once, so a repository without required checks is not held up.
  • If Forgejo refuses while a check on the head commit is still running, the step waits. It polls every 15 s and asks again whenever a check appears or changes state, printing one line per change.
  • A refusal stands once nothing is running and nothing has changed for 2 minutes. The step then fails with Forgejo's own message and each check that did not succeed, with its state and link. The guessed "the branch moved or it cannot be merged cleanly" is gone.
  • The engine now exports a scripted step's deadline as OSF_STEP_DEADLINE, and the wait ends a minute before it. The merge step's limit went from 900 s to 7200 s.

Verified on production

  • Run 38 was resumed at 22:34:58.
  • Its merge step printed "Forgejo refused to merge cmoriarty/soundcheck#72 for now: not allowed to merge [reason: Not all required status checks successful]".
  • It waited out the 2-minute grace and failed at 22:37:08 with Forgejo's message and "Checks that did not succeed: CI / full (pull_request) failure /cmoriarty/soundcheck/actions/runs/145/jobs/1".

Left over

  • Forgejo Actions gives a check's link as a path without the host, so the link is not clickable from the console. Prefixing the forge URL is a small follow-up.
  • The path where a pending check passes and the merge then goes through is covered by tests against a fake forge. It has not been seen on production yet.
Shipped in d94cfae, deployed on 2026-10-02 at 22:17. Archived in 5d67ff9 as `openspec/changes/archive/2026-10-02-merge-waits-for-checks`. **What changed** - `git.pr.merge` still asks for the merge at once, so a repository without required checks is not held up. - If Forgejo refuses while a check on the head commit is still running, the step waits. It polls every 15 s and asks again whenever a check appears or changes state, printing one line per change. - A refusal stands once nothing is running and nothing has changed for 2 minutes. The step then fails with Forgejo's own message and each check that did not succeed, with its state and link. The guessed "the branch moved or it cannot be merged cleanly" is gone. - The engine now exports a scripted step's deadline as `OSF_STEP_DEADLINE`, and the wait ends a minute before it. The merge step's limit went from 900 s to 7200 s. **Verified on production** - Run 38 was resumed at 22:34:58. - Its merge step printed "Forgejo refused to merge cmoriarty/soundcheck#72 for now: not allowed to merge [reason: Not all required status checks successful]". - It waited out the 2-minute grace and failed at 22:37:08 with Forgejo's message and "Checks that did not succeed: CI / full (pull_request) failure /cmoriarty/soundcheck/actions/runs/145/jobs/1". **Left over** - Forgejo Actions gives a check's link as a path without the host, so the link is not clickable from the console. Prefixing the forge URL is a small follow-up. - The path where a pending check passes and the merge then goes through is covered by tests against a fake forge. It has not been seen on production yet.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
archive/prefect-kestra-experiments
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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
cmoriarty/braid#129
No description provided.