pal-e-docs

SOP: PR Rejection Recovery

sop-pr-rejection-recovery Sop

sop active
Project

SOP: PR Rejection Recovery

Purpose: Teach agents how to respond when a PR is rejected by QA, has merge conflicts, regresses after a fix push, or is missing required metadata. Covers the full review-fix loop recovery path.
Traceability: plan-pal-e-agency → Phase 5 (Error Recovery SOPs)

Failure Modes

Symptom Likely Cause Recovery Steps
QA sets <strong>status:needs-fix</strong> with nit findings QA found non-blocking issues (style, naming, minor gaps). 1. Read QA comments on the <strong>Forgejo issue</strong> (not the PR — QA comments on issues). 2. <strong>If the PR is otherwise approved:</strong> Merge first, then fix nits in a follow-up PR from a new issue. Do NOT push nit fixes to the same PR (forces full re-QA). 3. <strong>If the PR has blocking issues:</strong> Fix on the feature branch, push, set <code>status:qa</code> again for re-review. 4. Nits from approved PRs MUST go to the plan Epilogue as subphases during <code>/update-docs</code>. Never dismiss nits as "minor."
QA sets <strong>status:needs-fix</strong> with blocking issues QA found functional bugs, missing tests, or SOP violations. 1. Read QA findings on the Forgejo issue. 2. Fix each blocking issue on the feature branch. 3. Push the fixes. 4. Set <code>status:qa</code> on the issue to trigger re-review. 5. A fresh QA reviewer is spawned each round (per <code>pr-review-loop</code> SOP).
<strong>Merge conflict</strong> on the PR Main branch advanced while the feature branch was in review. 1. Rebase onto main: <code>git fetch origin &amp;&amp; git rebase origin/main</code>. 2. Resolve conflicts. 3. Force push: <code>git push --force-with-lease</code>. 4. Re-run tests locally before pushing. 5. Set <code>status:qa</code> for re-review if the rebase changed significant code.
<strong>CI regression after fix push</strong> — tests that passed before now fail The fix introduced a new bug, or the rebase brought in incompatible changes from main. 1. Run the full test suite locally: <code>pytest tests/ -v</code>. 2. Identify which test broke and why. 3. Fix the regression on the feature branch. 4. Push and verify CI passes before requesting re-review.
PR submission <strong>blocked</strong> — missing <code>Closes #N</code> <code>check-pr-template.sh</code> hook enforces that every PR body contains <code>Closes #N</code> for Forgejo auto-close. 1. Add <code>Closes #N</code> to the PR body (where N is the Forgejo issue number). 2. Re-submit. 3. Without this, the issue will not auto-close on merge, creating stale open issues.
PR merged but <strong>issue not closed</strong> <code>Closes #N</code> was missing from the PR body, or the syntax was wrong (must be exact: <code>Closes #N</code>). 1. Manually close the issue on Forgejo. 2. This is a known gap from the pre-hook era. 3. Going forward, the hook prevents this.
<strong>QA blind spot</strong> — QA cannot see files that only exist on the PR branch QA agent reads local filesystem (usually main checkout). For initial reviews, the Forgejo API diff is sufficient. For re-reviews after nit fixes pushed to the same branch, QA cannot see the updated files. 1. Prefer the "merge first, nits in follow-up" pattern. 2. If re-review is required on the same branch, the Dev agent should summarize what changed in a comment on the issue. 3. QA can use the Forgejo diff API to see the changes.

Decision Tree

When a PR is rejected or has issues:
  • Read the QA findings on the Forgejo issue (not the PR).
  • Classify each finding: blocking (functional bug, missing test) vs nit (style, naming).
  • If only nits and PR is otherwise approved: Merge the PR. Create a new issue for each nit. Fix in follow-up PRs. Record nits in plan Epilogue.
  • If blocking issues: Fix on the feature branch. Push. Set status:qa. Wait for re-review.
  • If merge conflict: Rebase onto main, resolve, force push, re-run tests.
  • If CI regresses after push: Run tests locally, fix the regression, push again.
  • After 3 review-fix cycles on the same PR: Escalate to Betty Sue — the scope may need re-evaluation.

Escalation Criteria

Escalate immediately (do NOT self-correct) when:
  • QA findings indicate a fundamental design problem (not just implementation bugs)
  • The PR has gone through 3+ review-fix cycles without converging
  • Merge conflicts involve files you do not own or understand
  • The issue scope has grown beyond the original spec during the fix cycle
  • You disagree with a QA finding — do not argue, escalate to Betty Sue for a ruling
  • pr-lifecycle — all 8 stages of the PR workflow
  • pr-review-loop — the review-fix loop SOP (fresh reviewer each round)
  • solo-dev-pr-workflow — present PR and STOP, never merge without approval
  • sop-post-merge-docs — what to do after merge (docs update is the gate)
  • sop-ci-pipeline-recovery — when CI fails after a fix push