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

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

jcwalker3 commented

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

Summary

Fixes review submissions landing as PENDING drafts and merges proceeding without a visible APPROVED verdict.

Root cause

Gitea's review API expects event APPROVED, not APPROVE. The wrong event string fell through to Gitea's default and created PENDING draft reviews.

Changes

Send APPROVED on approve submissions
After terminal review submit, re-fetch reviews and fail closed unless the reviewer's verdict is visible
If Gitea returns PENDING, submit the pending draft via the pending-review endpoint
gitea_merge_pr now requires approval_visible from gitea_get_pr_review_feedback and blocks on undismissed REQUEST_CHANGES
Runbook note in skills/llm-project-workflow/SKILL.md

Tests

PENDING-after-submit failure
Pending-draft submit recovery
Merge blocked without visible approval
Merge allowed with visible APPROVED

Closes #244

## Summary Fixes review submissions landing as PENDING drafts and merges proceeding without a visible APPROVED verdict. ## Root cause Gitea's review API expects event `APPROVED`, not `APPROVE`. The wrong event string fell through to Gitea's default and created PENDING draft reviews. ## Changes - Send `APPROVED` on approve submissions - After terminal review submit, re-fetch reviews and fail closed unless the reviewer's verdict is visible - If Gitea returns PENDING, submit the pending draft via the pending-review endpoint - `gitea_merge_pr` now requires `approval_visible` from `gitea_get_pr_review_feedback` and blocks on undismissed REQUEST_CHANGES - Runbook note in `skills/llm-project-workflow/SKILL.md` ## Tests - PENDING-after-submit failure - Pending-draft submit recovery - Merge blocked without visible approval - Merge allowed with visible APPROVED Closes #244

jcwalker3 added 1 commit 2026-07-06 10:51:46 -05:00

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

Gitea expects review event APPROVED, not APPROVE; the wrong event left
submissions as PENDING drafts. Verify terminal verdicts after submit,
submit pending drafts when needed, and block merge when approval_visible
is false.

sysadmin reviewed 2026-07-06 11:11:17 -05:00

sysadmin left a comment

LGTM! The changes in the PR are clean and successfully address issue #244 by handling PENDING reviews and requiring visible APPROVED reviews. Note for the author: please update test_pr_queue_inventory.py's mock setup as it was missed and causes a test failure on the PR branch.

jcwalker3 added 1 commit 2026-07-06 11:21:45 -05:00

test: fix PR queue inventory mock for visible APPROVED verification (#244 ) 9d8ab0a7b5

Update test_reviewer_profile_can_proceed_from_inventory_to_review_gates
to return APPROVED state on POST and include GET /reviews verification
responses required by #244 review submission gates.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

sysadmin reviewed 2026-07-06 11:22:08 -05:00

sysadmin left a comment

Review summary

Reviewed PR #251 at head 9d8ab0a7b560930943b0a40984ea3363cc0ae3f8 (includes test fix 9d8ab0a).

Scope: Closes #244 — sends APPROVED not APPROVE, submits pending drafts, verifies visible verdict post-submit, blocks merge without approval_visible.

Test fix: test_pr_queue_inventory.py mock updated for GET /reviews verification (#244).

Validation:

pytest tests/test_pr_queue_inventory.py → 18 passed
pytest tests/test_mcp_server.py -k 'review or merge or pending or approve' → 73 passed
pytest tests/ → full suite exit 0

Verdict: Approve. Ready to merge.

## Review summary Reviewed PR #251 at head `9d8ab0a7b560930943b0a40984ea3363cc0ae3f8` (includes test fix `9d8ab0a`). **Scope:** Closes #244 — sends `APPROVED` not `APPROVE`, submits pending drafts, verifies visible verdict post-submit, blocks merge without `approval_visible`. **Test fix:** `test_pr_queue_inventory.py` mock updated for GET /reviews verification (#244). **Validation:** - `pytest tests/test_pr_queue_inventory.py` → 18 passed - `pytest tests/test_mcp_server.py -k 'review or merge or pending or approve'` → 73 passed - `pytest tests/` → full suite exit 0 **Verdict:** Approve. Ready to merge.

sysadmin merged commit f94cb80fc9 into master

2026-07-06 11:22:14 -05:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: Scaled-Tech-Consulting/Gitea-Tools#251