Harden PR review and merge workflow with enforced state machine gates #290

New Issue

2026-07-06T23:27:43-05:00

jcwalker3 commented

2026-07-06 23:27:43 -05:00

LLM PR review and merge workflows currently rely too much on prompt compliance and stale session memory. This has produced unsafe or confusing behavior such as preserving “ready to merge” after MCP failure, including approve/merge commands in blocked recovery handoffs, advancing PR queue state after infra_stop, and running diagnostics without clear control-checkout repair mode.

This must be fixed at the tooling, template, and test level so every LLM follows the same enforced workflow.

Required system behavior:

PR review and merge must follow an explicit state machine:
PRECHECK → INVENTORY → SELECT_PR → PIN_HEAD_SHA → CREATE_BRANCHES_WORKTREE → VALIDATE → REVIEW_DECISION → APPROVE_OR_REQUEST_CHANGES → PRE_MERGE_RECHECK → MERGE → POST_MERGE_REPORT.
If any state fails, downstream states are forbidden.
If gitea_resolve_task_capability returns infra_stop or blocked, PR queue work must stop immediately.
A blocked recovery handoff must not include direct approve or merge replay commands.
Approval and merge require separate exact capabilities.
Merge requires a fresh second preflight:
- whoami
- profile/runtime
- exact merge capability
- PR re-fetch
- reviewed head SHA unchanged
- PR mergeable
- checks passed
- reviewer is not author
- worktree clean
All PR validation must happen in a session-owned worktree under branches/.
The main checkout must stay on master/main/dev and must not be used for PR validation, fixes, or merge prep.
infra_stop diagnostics must report the exact inspected root, active worktree, MCP control checkout, conflict/dirty path, merge/rebase control file if any, and whether the result is cached or live.
Final reports must be validated before output. Reports must not claim reviewed/validated/ready-to-merge state unless the corresponding state machine gates passed.

Acceptance criteria:

A PR cannot be approved unless all review gates passed.
A PR cannot be merged unless all pre-merge gates passed in the current session.
infra_stop blocks PR selection, review, validation, approval, and merge.
Blocked recovery handoffs say “restart full workflow after blocker clears,” not “approve/merge this PR.”
Main checkout is never used for normal PR validation.
Stale MCP capability state cannot be reused after reconnect failure.
Tests cover blocked infra_stop, failed MCP reconnect, changed head SHA, dirty worktree, reviewer-is-author, incomplete PR inventory, validation failure, and blocked recovery handoff wording.

Important framing:
This is not a prompt-only improvement. Implement durable enforcement through MCP gates, helper checks, tests, and templates so all LLMs are forced into the correct workflow.

LLM PR review and merge workflows currently rely too much on prompt compliance and stale session memory. This has produced unsafe or confusing behavior such as preserving “ready to merge” after MCP failure, including approve/merge commands in blocked recovery handoffs, advancing PR queue state after infra_stop, and running diagnostics without clear control-checkout repair mode. This must be fixed at the tooling, template, and test level so every LLM follows the same enforced workflow. Required system behavior: 1. PR review and merge must follow an explicit state machine: PRECHECK → INVENTORY → SELECT_PR → PIN_HEAD_SHA → CREATE_BRANCHES_WORKTREE → VALIDATE → REVIEW_DECISION → APPROVE_OR_REQUEST_CHANGES → PRE_MERGE_RECHECK → MERGE → POST_MERGE_REPORT. 2. If any state fails, downstream states are forbidden. 3. If gitea_resolve_task_capability returns infra_stop or blocked, PR queue work must stop immediately. 4. A blocked recovery handoff must not include direct approve or merge replay commands. 5. Approval and merge require separate exact capabilities. 6. Merge requires a fresh second preflight: - whoami - profile/runtime - exact merge capability - PR re-fetch - reviewed head SHA unchanged - PR mergeable - checks passed - reviewer is not author - worktree clean 7. All PR validation must happen in a session-owned worktree under branches/. 8. The main checkout must stay on master/main/dev and must not be used for PR validation, fixes, or merge prep. 9. infra_stop diagnostics must report the exact inspected root, active worktree, MCP control checkout, conflict/dirty path, merge/rebase control file if any, and whether the result is cached or live. 10. Final reports must be validated before output. Reports must not claim reviewed/validated/ready-to-merge state unless the corresponding state machine gates passed. Acceptance criteria: - A PR cannot be approved unless all review gates passed. - A PR cannot be merged unless all pre-merge gates passed in the current session. - infra_stop blocks PR selection, review, validation, approval, and merge. - Blocked recovery handoffs say “restart full workflow after blocker clears,” not “approve/merge this PR.” - Main checkout is never used for normal PR validation. - Stale MCP capability state cannot be reused after reconnect failure. - Tests cover blocked infra_stop, failed MCP reconnect, changed head SHA, dirty worktree, reviewer-is-author, incomplete PR inventory, validation failure, and blocked recovery handoff wording. Important framing: This is not a prompt-only improvement. Implement durable enforcement through MCP gates, helper checks, tests, and templates so all LLMs are forced into the correct workflow.

jcwalker3 referenced this issue

2026-07-07 01:49:32 -05:00

Reviewer approval must prove prior request-changes blockers are resolved or invalid #326

jcwalker3 added the status:in-progress label 2026-07-07 08:13:43 -05:00

jcwalker3 referenced this issue from a commit

2026-07-07 08:14:33 -05:00

feat: enforce PR review/merge workflow state machine gates (Closes #290)

jcwalker3 referenced a pull request that will close this issue

2026-07-07 08:14:40 -05:00

feat: enforce PR review/merge workflow state machine gates (Closes #290) #378

jcwalker3 commented

2026-07-07 08:14:42 -05:00

Progress update

PR opened: #378 — feat/issue-290-review-merge-state-machine → master

Commit: 8d32af1
Tests: 13 passed
PR mergeable: True

Ready for review.

**Progress update** PR opened: **#378** — `feat/issue-290-review-merge-state-machine` → `master` - Commit: `8d32af1` - Tests: 13 passed - PR mergeable: True Ready for review.

jcwalker3 referenced this issue

2026-07-07 08:51:09 -05:00

Reviewer mutations require canonical workflow load proof #389

jcwalker3 referenced this issue

2026-07-07 08:51:11 -05:00

Add PR-only queue cleanup mode that dispatches one canonical review per PR #390

jcwalker3 referenced this issue from a commit

2026-07-07 08:57:06 -05:00

fix: resolve conflicts for PR #378

sysadmin closed this issue

2026-07-07 09:02:43 -05:00

sysadmin removed the status:in-progress label 2026-07-07 09:02:44 -05:00