Merge UI treats CHANGES_REQUESTED as blocking with no protection rule while the server merges it #612

New issue

Closed

opened 2026-09-15 23:05:17 +00:00 by crueber · 1 comment

crueber commented

2026-09-15 23:05:17 +00:00

Owner

Evidence (live field test, scratch stack :18099, throwaway repo field/demo)

PR #2 (clean, no policy configured): submitted CHANGES_REQUESTED as author → POST …/pulls/2/merge {strategy:merge} → task ok: merged as 9cc0080…. Server merges fine with an outstanding change request when no required-reviews rule exists (GitHub-like: no protection, no block).
The UI disagrees: mergeState() returns blocked on any reviewDecision === CHANGES_REQUESTED (MergeBox.jsx:43), so the button disables (enabled() needs mergeable) with tooltip blocked: changes requested, and the #588 headline reads Changes requested soft-red — while POST merge succeeds. A user told no by the UI can merge anyway via API/SDK with zero friction.

What's requested

Make UI and server agree. Recommended: the client should only treat CHANGES_REQUESTED as merge-blocking when a required-reviews rule actually applies to the base ref — the page already fetches policy for checks (Pull.jsx getPolicy, requiredChecks()), so the same rules list can drive a reviews gate (any required-reviews effect matching base → changes-requested blocks; none → red phrase stays as information but the button enables). Server behavior (merge allowed without a rule) stays as-is. Alternative (server blocks always): contradicts GitHub semantics and the #586 Decision-1b design — not recommended; note the choice.

Acceptance criteria

With no required-reviews rule: CHANGES_REQUESTED shows the red phrase but the merge button enables (tooltip no longer claims blocked), and a merge succeeds through both UI and API identically.
With a matching required-reviews rule: button stays disabled with the blocked tooltip, and the server refuses (already does).
Unit tests pin both halves (policy-absent enables, policy-present blocks).

## Evidence (live field test, scratch stack :18099, throwaway repo field/demo) - PR #2 (clean, no policy configured): submitted CHANGES_REQUESTED as author → POST …/pulls/2/merge {strategy:merge} → task ok: merged as 9cc0080…. Server merges fine with an outstanding change request when no required-reviews rule exists (GitHub-like: no protection, no block). - The UI disagrees: mergeState() returns blocked on any reviewDecision === CHANGES_REQUESTED (MergeBox.jsx:43), so the button disables (enabled() needs mergeable) with tooltip blocked: changes requested, and the #588 headline reads Changes requested soft-red — while POST merge succeeds. A user told no by the UI can merge anyway via API/SDK with zero friction. ## What's requested Make UI and server agree. Recommended: the client should only treat CHANGES_REQUESTED as merge-blocking when a required-reviews rule actually applies to the base ref — the page already fetches policy for checks (Pull.jsx getPolicy, requiredChecks()), so the same rules list can drive a reviews gate (any required-reviews effect matching base → changes-requested blocks; none → red phrase stays as information but the button enables). Server behavior (merge allowed without a rule) stays as-is. Alternative (server blocks always): contradicts GitHub semantics and the #586 Decision-1b design — not recommended; note the choice. ## Acceptance criteria - [ ] With no required-reviews rule: CHANGES_REQUESTED shows the red phrase but the merge button enables (tooltip no longer claims blocked), and a merge succeeds through both UI and API identically. - [ ] With a matching required-reviews rule: button stays disabled with the blocked tooltip, and the server refuses (already does). - [ ] Unit tests pin both halves (policy-absent enables, policy-present blocks).

crueber added this to the v1 milestone

2026-09-15 23:05:17 +00:00

crueber added the

bug

issues

pull requests

labels

2026-09-15 23:05:17 +00:00

crueber referenced this issue

2026-09-15 23:05:34 +00:00

Exploration: full field test of the PR UI (blocked merges, all merge strategies, commits land in repo) #600

crueber referenced this issue from a commit

2026-09-15 23:18:03 +00:00

Gate client CHANGES_REQUESTED merge-block on required-reviews rule presence (Forgejo #612)

crueber referenced this issue from a pull request that will close it,

2026-09-15 23:18:06 +00:00

Gate client CHANGES_REQUESTED merge-block on required-reviews rule (fixes #612) #615

crueber referenced this issue

2026-09-15 23:20:10 +00:00

Gate client CHANGES_REQUESTED merge-block on required-reviews rule (fixes #612) #615

crueber closed this issue

2026-09-15 23:20:26 +00:00

crueber referenced this issue from a commit

2026-09-15 23:20:26 +00:00

Merge pull request 'Gate client CHANGES_REQUESTED merge-block on required-reviews rule (fixes #612)' (#615) from fix/issue-612 into main

crueber commented

2026-09-15 23:20:26 +00:00

Author

Owner

Fixed by #615 (merged): client gates the CHANGES_REQUESTED block on a matching required-reviews rule (same policy list as checks); no-rule → red phrase stays informational but button enables, matching the server; with-rule → disabled + blocked tooltip. Server untouched. Simplification (presence-only, exact ref) documented; server authoritative. Verified: 1579 unit green (smoke excluded, pre-existing), vite/esbuild green, independent review APPROVE.

crueber referenced this issue

2026-09-28 18:55:18 +00:00

README v1: mark pulls + releases done (checkboxes lag reality) #633