Glob-match required status contexts when diagnosing a merge #275

Merged
stephen merged 1 commit from fix-required-check-glob into main 2026-09-30 03:46:57 +00:00
Owner

fj pr merge blames a required check on fully green PRs whenever the base branch is protected with status_check_contexts: ["*"], which is every rasterstate repo:

$ fj pr checks 1634 --repo rasterstate/fjord-ios
Combined: success  7 checks on 3ef3a4a
$ fj pr merge 1634 --repo rasterstate/fjord-ios --style squash --delete-branch
error: PR #1634 is blocked by a required check: "*" is not green (state: missing).

Forgejo compiles each entry of status_check_contexts as a glob, so * means every reported context and CI / * every context with that prefix. failing_required in src/cli/pr_merge_check.rs looked each entry up by name in the combined status instead. No check is literally called *, so it was always missing, and because checks rank above approvals and the raw error in diagnose, that invented reason also buried whatever actually made the server refuse the merge.

The comparison is now a pure required_check_failures, modelled on Forgejo's merge gate, MergeRequiredContextsCommitStatus in services/pull/commit_status.go (read from a Forgejo 16 dev checkout). Each pattern must match at least one reported status, and every status it matches must be success or skipped; pending, warning, error and failure block. skipped passes because Forgejo ranks it above success, so it never lowers the verdict; that is what lets paragon's promotion-only lint / develop-superset-of-main sit skipped on every develop PR. A pattern that matches nothing is still missing, as Forgejo treats it as pending.

Two wording changes follow from that. A failing check is now reported under its real name, so a * rule says "CI / test (pull_request)" is not green (state: failure) rather than quoting the pattern. And a glob that matches nothing says no reported check matches the required pattern "*". Literal context names keep the existing message.

The matcher is about a hundred lines, hand-written, covering the syntax gobwas/glob accepts without separators (*, ?, [a-z], [!x], {a,b}, \ escapes). The glob crate is already in Cargo.lock (transitively), but it rejects ** anywhere other than a whole path component and has no {a,b} alternation, so it would disagree with Forgejo on patterns Forgejo accepts. A pattern that fails to compile is ignored, which is what Forgejo does with it.

This was the only place fj compared required contexts. The Mergeable: line in fj pr view and fj pr checks reads only the PR's own mergeable flag, and --auto reaches the same diagnosis after a rejected merge, so both pick the fix up without changes.

One consequence worth knowing: the diagnosis only runs after Forgejo has already refused the merge. On #1634 the server did refuse something, the web UI and a raw POST .../merge with Do=squash both succeeded, and with the fake * blocker gone that underlying reason, or the raw API error, is what fj pr merge will now print. If it turns out to be another fj defect it wants its own issue.

Closes #238

Verified on macOS: cargo fmt --all clean, cargo clippy --all-targets --all-features -- -D warnings clean, cargo test --all green (831 + 5 + 1), pre-push hook passed.

`fj pr merge` blames a required check on fully green PRs whenever the base branch is protected with `status_check_contexts: ["*"]`, which is every rasterstate repo: ``` $ fj pr checks 1634 --repo rasterstate/fjord-ios Combined: success 7 checks on 3ef3a4a $ fj pr merge 1634 --repo rasterstate/fjord-ios --style squash --delete-branch error: PR #1634 is blocked by a required check: "*" is not green (state: missing). ``` Forgejo compiles each entry of `status_check_contexts` as a glob, so `*` means every reported context and `CI / *` every context with that prefix. `failing_required` in `src/cli/pr_merge_check.rs` looked each entry up by name in the combined status instead. No check is literally called `*`, so it was always `missing`, and because checks rank above approvals and the raw error in `diagnose`, that invented reason also buried whatever actually made the server refuse the merge. The comparison is now a pure `required_check_failures`, modelled on Forgejo's merge gate, `MergeRequiredContextsCommitStatus` in `services/pull/commit_status.go` (read from a Forgejo 16 dev checkout). Each pattern must match at least one reported status, and every status it matches must be `success` or `skipped`; `pending`, `warning`, `error` and `failure` block. `skipped` passes because Forgejo ranks it above `success`, so it never lowers the verdict; that is what lets paragon's promotion-only `lint / develop-superset-of-main` sit skipped on every develop PR. A pattern that matches nothing is still `missing`, as Forgejo treats it as pending. Two wording changes follow from that. A failing check is now reported under its real name, so a `*` rule says `"CI / test (pull_request)" is not green (state: failure)` rather than quoting the pattern. And a glob that matches nothing says `no reported check matches the required pattern "*"`. Literal context names keep the existing message. The matcher is about a hundred lines, hand-written, covering the syntax gobwas/glob accepts without separators (`*`, `?`, `[a-z]`, `[!x]`, `{a,b}`, `\` escapes). The `glob` crate is already in `Cargo.lock` (transitively), but it rejects `**` anywhere other than a whole path component and has no `{a,b}` alternation, so it would disagree with Forgejo on patterns Forgejo accepts. A pattern that fails to compile is ignored, which is what Forgejo does with it. This was the only place fj compared required contexts. The `Mergeable:` line in `fj pr view` and `fj pr checks` reads only the PR's own `mergeable` flag, and `--auto` reaches the same diagnosis after a rejected merge, so both pick the fix up without changes. One consequence worth knowing: the diagnosis only runs after Forgejo has already refused the merge. On #1634 the server did refuse something, the web UI and a raw `POST .../merge` with `Do=squash` both succeeded, and with the fake `*` blocker gone that underlying reason, or the raw API error, is what `fj pr merge` will now print. If it turns out to be another fj defect it wants its own issue. Closes #238 Verified on macOS: `cargo fmt --all` clean, `cargo clippy --all-targets --all-features -- -D warnings` clean, `cargo test --all` green (831 + 5 + 1), pre-push hook passed.
Glob-match required status contexts when diagnosing a merge
All checks were successful
ci / check (pull_request) Successful in 11m58s
ci / live-e2e (pull_request) Successful in 2m17s
ci / coverage (pull_request) Successful in 2m34s
8d91bfa471
Branch protection's status_check_contexts are glob patterns in Forgejo,
not context names. The merge diagnosis looked each one up literally, so
a repo protected with ["*"] reported every rejected merge as
`"*" is not green (state: missing)`, even on a fully green head, and
hid whatever actually refused the merge.

Required contexts are now evaluated the way Forgejo's merge gate does
(MergeRequiredContextsCommitStatus in services/pull/commit_status.go):
each pattern must match at least one reported status, and every status
it matches must be success or skipped. A failure is reported under the
real check's name, and a glob that matches nothing says so plainly. The
matcher is a small hand-written one covering the gobwas/glob syntax
Forgejo compiles these with, so no dependency is added.

Closes #238
stephen deleted branch fix-required-check-glob 2026-09-30 03:46:57 +00:00
Sign in to join this conversation.
No description provided.