Review submissions land as PENDING drafts; merges proceed without a visible APPROVED verdict #244

New Issue

2026-07-06T10:39:36-05:00

jcwalker3 commented

2026-07-06 10:39:36 -05:00

Problem

Two merges on 2026-07-06 went through while the merging session's approval sat as a PENDING draft review:

Repo	PR	Merged	Review state at merge
`mcp-control-plane`	#94	00:38:45, merge `3f06e89`	reviewer `sysadmin`, verdict `PENDING`, approval body present, `approval_visible: false`
`Gitea-Tools`	#234	01:06:24, merge `7966e70`	reviewer `sysadmin`, verdict `PENDING`, approval body present, `approval_visible: false`

gitea_get_pr_review_feedback (#167) reports for both: latest_review_state_by_reviewer: {}, approval_visible: false, latest_reviewed_head_sha: null — i.e. the formal review record shows no completed approval, yet the merge executed.

Two defects (fix either or both):

Submission path leaves drafts. The review-submission tool (gitea_submit_pr_review / legacy gitea_review_pr) apparently creates the review without committing the APPROVE event, so Gitea stores it as PENDING. The tool must submit a completed review (correct event param on POST /repos/{owner}/{repo}/pulls/{n}/reviews) and fail closed if the resulting verdict is not the requested one.
Merge gate does not require a visible approval. gitea_merge_pr (#16) relies on Gitea's mergeable signal; with no branch-protection required-review rule, a PENDING draft is silently sufficient. The merge gate should re-read review feedback (#167 tool) and refuse when approval_visible is false, unless an explicit documented override applies.

Acceptance criteria

Submitting an APPROVE via the MCP tool yields a review with verdict APPROVED visible in gitea_get_pr_review_feedback (test with live-shaped fixture).
Fail closed if the API response leaves the review in PENDING.
gitea_merge_pr blocks when the current reviewer's approval is not visible (PENDING/absent), with a clear reason.
Tests cover: PENDING-after-submit failure, merge blocked on non-visible approval, merge allowed after committed APPROVE.
Runbook/template note: reviewers must verify their approval is visible before merging.

Provenance

Observed by reviewer sessions on PR #94 and PR #234 (2026-07-06). Related: #167 (review feedback discovery), #211 (review mutation proof), mcp-control-plane #16 (gated merge).

## Problem Two merges on 2026-07-06 went through while the merging session's approval sat as a **PENDING draft review**: | Repo | PR | Merged | Review state at merge | |---|---|---|---| | `mcp-control-plane` | #94 | 00:38:45, merge `3f06e89` | reviewer `sysadmin`, verdict `PENDING`, approval body present, `approval_visible: false` | | `Gitea-Tools` | #234 | 01:06:24, merge `7966e70` | reviewer `sysadmin`, verdict `PENDING`, approval body present, `approval_visible: false` | `gitea_get_pr_review_feedback` (#167) reports for both: `latest_review_state_by_reviewer: {}`, `approval_visible: false`, `latest_reviewed_head_sha: null` — i.e. the formal review record shows **no completed approval**, yet the merge executed. Two defects (fix either or both): 1. **Submission path leaves drafts.** The review-submission tool (`gitea_submit_pr_review` / legacy `gitea_review_pr`) apparently creates the review without committing the APPROVE event, so Gitea stores it as PENDING. The tool must submit a completed review (correct event param on `POST /repos/{owner}/{repo}/pulls/{n}/reviews`) and fail closed if the resulting verdict is not the requested one. 2. **Merge gate does not require a visible approval.** `gitea_merge_pr` (#16) relies on Gitea's `mergeable` signal; with no branch-protection required-review rule, a PENDING draft is silently sufficient. The merge gate should re-read review feedback (#167 tool) and refuse when `approval_visible` is false, unless an explicit documented override applies. ## Acceptance criteria 1. Submitting an APPROVE via the MCP tool yields a review with verdict `APPROVED` visible in `gitea_get_pr_review_feedback` (test with live-shaped fixture). 2. Fail closed if the API response leaves the review in PENDING. 3. `gitea_merge_pr` blocks when the current reviewer's approval is not visible (PENDING/absent), with a clear reason. 4. Tests cover: PENDING-after-submit failure, merge blocked on non-visible approval, merge allowed after committed APPROVE. 5. Runbook/template note: reviewers must verify their approval is visible before merging. ## Provenance Observed by reviewer sessions on PR #94 and PR #234 (2026-07-06). Related: #167 (review feedback discovery), #211 (review mutation proof), mcp-control-plane #16 (gated merge).

jcwalker3 referenced this issue

2026-07-06 10:43:14 -05:00

Issue-lock gate binds to the shared dev worktree, blocking all scratch-clone author sessions while any session is mid-branch #249

jcwalker3 commented

2026-07-06 10:43:30 -05:00

Claimed and released without implementation (2026-07-06). gitea_lock_issue failed closed: the issue-lock gate validates against the shared dev worktree, which a concurrent session holds on feat/issue-243-forbidden-git-gaps with tracked edits. Per the #233/#91 interference walls, that worktree must not be touched to satisfy the gate. Blocker filed as #249 (lock gate must validate the caller's scratch clone, not the shared worktree). Re-claim this issue once #249 lands or the shared worktree returns to a clean master.

Claimed and released without implementation (2026-07-06). `gitea_lock_issue` failed closed: the issue-lock gate validates against the shared dev worktree, which a concurrent session holds on `feat/issue-243-forbidden-git-gaps` with tracked edits. Per the #233/#91 interference walls, that worktree must not be touched to satisfy the gate. Blocker filed as #249 (lock gate must validate the caller's scratch clone, not the shared worktree). Re-claim this issue once #249 lands or the shared worktree returns to a clean master.

jcwalker3 referenced this issue

2026-07-06 10:46:52 -05:00

Issue-lock gate binds to the shared dev worktree, blocking all scratch-clone author sessions while any session is mid-branch #249

jcwalker3 added the status:in-progress label 2026-07-06 10:48:08 -05:00

jcwalker3 referenced this issue from a commit

2026-07-06 10:51:34 -05:00

fix(mcp): require visible APPROVED reviews before merge (closes #244)

jcwalker3 referenced a pull request that will close this issue

2026-07-06 10:51:46 -05:00

fix(mcp): require visible APPROVED reviews before merge (closes #244) #251

jcwalker3 referenced this issue

2026-07-06 10:54:44 -05:00