Report the specific reason a PR merge was rejected #162

Merged

stephen merged 1 commit from fix/159-merge-reason into main

2026-06-15 14:50:19 +00:00

stephen commented

2026-06-15 14:20:34 +00:00

Owner

Closes #159, #160.

fj pr merge returned a bare 405 Method Not Allowed (sometimes 500) for every reason a PR could not merge: conflicts, a disabled merge style, failing required checks, missing approvals, an out-of-date branch, an already-merged PR. The opaque status is indistinguishable across causes and cost real debugging time during the 2026-06-15 drain.

What changed:

API layer: pull::merge and pull::update_branch used reqwest's error_for_status, which drops the response body. They now go through a new Client::send that runs the same ensure_success path as the JSON helpers, so Forgejo's own error message survives instead of collapsing to the status line.
Diagnosis (cli::pr_merge_check): on a merge failure the CLI inspects the PR's mergeability and prints one specific, actionable reason. It reads the PR state/mergeable/draft flags, the repo's allowed merge styles, branch protection (required approvals, required status checks, block-on-outdated), reviews, and the head commit status, then reports the most fundamental blocker. Gathering is best-effort: a permission error on branch protection (common for non-admins) degrades to unknown rather than masking the diagnosis. When nothing specific is found it falls back to the now-surfaced raw message.
fj pr view and fj pr checks gained a cheap Mergeable: line (no extra API calls).

Tests: 11 unit tests over the pure reason-picker plus 5 wiremock tests (405 message passthrough, and end-to-end disabled-style / conflict / failing-check / missing-approvals diagnosis). Full suite green, clippy -D warnings clean.

Closes #159, #160. `fj pr merge` returned a bare `405 Method Not Allowed` (sometimes `500`) for every reason a PR could not merge: conflicts, a disabled merge style, failing required checks, missing approvals, an out-of-date branch, an already-merged PR. The opaque status is indistinguishable across causes and cost real debugging time during the 2026-06-15 drain. What changed: - API layer: `pull::merge` and `pull::update_branch` used reqwest's `error_for_status`, which drops the response body. They now go through a new `Client::send` that runs the same `ensure_success` path as the JSON helpers, so Forgejo's own error message survives instead of collapsing to the status line. - Diagnosis (`cli::pr_merge_check`): on a merge failure the CLI inspects the PR's mergeability and prints one specific, actionable reason. It reads the PR state/mergeable/draft flags, the repo's allowed merge styles, branch protection (required approvals, required status checks, block-on-outdated), reviews, and the head commit status, then reports the most fundamental blocker. Gathering is best-effort: a permission error on branch protection (common for non-admins) degrades to unknown rather than masking the diagnosis. When nothing specific is found it falls back to the now-surfaced raw message. - `fj pr view` and `fj pr checks` gained a cheap `Mergeable:` line (no extra API calls). Tests: 11 unit tests over the pure reason-picker plus 5 wiremock tests (405 message passthrough, and end-to-end disabled-style / conflict / failing-check / missing-approvals diagnosis). Full suite green, clippy -D warnings clean.

stephen added 1 commit

2026-06-15 14:20:34 +00:00

Report the specific reason a PR merge was rejected

ci / check (pull_request) Failing after 36s

Details

ci / coverage (pull_request) Has been skipped

Details

ci / live-e2e (pull_request) Has been skipped

Details

440ae18bd9

fj pr merge returned a bare 405 (or 500) for every non-mergeable cause:
conflicts, a disabled merge style, failing required checks, missing
approvals, an out-of-date branch, an already-merged PR. The opaque status
is indistinguishable across causes and cost real debugging time.

Two parts:

- API layer: the merge and update-branch calls used reqwest's
  error_for_status, which discards the response body. Route them through
  a new Client::send that runs the same ensure_success path as the JSON
  helpers, so Forgejo's own error message survives instead of collapsing
  to the status line.

- Diagnosis: on a merge failure, inspect the PR's mergeability and emit
  one specific, actionable message (cli::pr_merge_check). It reads the PR
  state/mergeable/draft flags, the repo's allowed merge styles, branch
  protection (required approvals, required status checks, block-on-
  outdated), reviews, and the head commit status, then picks the most
  fundamental blocker. Gathering is best-effort: a permission error on
  branch protection (common for non-admins) degrades to "unknown" rather
  than masking the diagnosis. When nothing specific is found it falls back
  to the now-surfaced raw message.

fj pr view and fj pr checks gain a cheap Mergeable: line (no extra calls).

Closes #159, #160.

stephen added 1 commit

2026-06-15 14:22:38 +00:00

rustfmt

ci / check (pull_request) Successful in 9m57s

Details

ci / live-e2e (pull_request) Successful in 1m49s

Details

ci / coverage (pull_request) Successful in 1m50s

Details

6af87fa464