--- description: Review subcommand — push branch, create PR, invoke /review-pr with --auto-merge argument-hint: "[issue number] [--repo GH_REPO] [--gh-flag GH_FLAG] [--worktree PATH] [--branch BRANCH] [--base PR_BASE]" --- # work-on/review — Review & PR Creation Subcommand **Input**: $ARGUMENTS **Invoked by**: `work-on.md` Phase 4–5, after `build/validate.md` returns `GATE_PASSED: true`. **Output**: Push branch, create PR, invoke /review-pr --auto-merge, return result to caller. **Agent model policy**: `model: "{DEFAULT_MODEL}"` — resolved from forge.yaml `agents.default_model`, else "sonnet" (standard tier). Fallback: `model: "opus"` if rate-limited. Feature gate: pass `effort` in Task/Skill spawns only on Claude Code >= 2.1.154. This file's mechanical bits (label transitions, `FORGE:CHECKPOINT` writes) stay at this tier because they're interleaved with the review/merge-decision steps in the same `Skill()` invocation — see `work-on.md` section "Model and Effort Tiering — What Actually Applies". **NEVER use plan mode (EnterPlanMode).** --- ## Inputs Parse from $ARGUMENTS: - `{NUMBER}` — issue number (required) - `--repo {GH_REPO}` — GitHub repo (e.g. `{owner}/{repo}` — resolved from `forge.yaml → project`) - `--gh-flag {GH_FLAG}` — gh CLI repo flag (e.g. `-R {owner}/{repo}`) - `--worktree {WORKTREE_PATH}` — absolute path to the git worktree - `--branch {BRANCH}` — feature branch name (e.g. `feat/my-feature`) - `--base {PR_BASE}` — PR target branch (e.g. `milestone/modular-pipeline-architecture` or `staging`) --- ## Phase R0: Load State from GitHub (MANDATORY) Re-read current state before doing anything: ```bash gh issue view {NUMBER} {GH_FLAG} --json number,title,body,labels,state,milestone # Get builder comment (for branch + commit info) gh api repos/{GH_REPO}/issues/{NUMBER}/comments \ --jq '.[] | select(.body | contains("FORGE:BUILDER")) | .body' # Check if PR already exists for this branch gh pr list {GH_FLAG} --head {BRANCH} --json number,state,url 2>/dev/null ``` **Resume check**: - If PR already exists AND is OPEN → run the **HEAD-unchanged re-review guard** below before proceeding to Phase R3 - If PR already exists AND is MERGED → return `REVIEW_RESULT: status: ALREADY_MERGED` - If no `` comment exists → EXIT with `REVIEW_RESULT: status: BLOCKED`, blocker: "FORGE:BUILDER comment not found — implement phase may not have completed" **HEAD-unchanged re-review guard** (MANDATORY when a PR already exists and is OPEN) — a PR whose most recent review verdict was CHANGES REQUESTED must not be resubmitted for a full domain-agent review fan-out if nothing has changed since that verdict. `/review-pr` records the exact commit it reviewed in its verdict comment (`CHANGES REQUESTED: commit {sha} — ...`, see `commands/review-pr.md` Phase 8/9); compare that recorded sha against the PR's current `headRefOid`: ```bash # NOTE: the CHANGES REQUESTED verdict is posted via `gh pr review --comment` (see # commands/review-pr.md Phase 7B) — that creates a PullRequestReview, which surfaces under # the `reviews` field, NOT `comments` (issue/PR comments are a separate GraphQL object). # Query both --json reviews and --json comments (same combined-read pattern already used in # work-on.md's REVIEW_BODIES/REVIEW_PRESENT checks) so this guard works regardless of which # GitHub object type carries the verdict text. LAST_VERDICT=$(gh pr view {PR_NUMBER} {GH_FLAG} --json reviews,comments \ --jq '([.reviews[] | {body, created_at: (.submittedAt // "")}] + [.comments[] | {body, created_at: (.createdAt // "")}]) | map(select(.body | test("CHANGES REQUESTED: commit "))) | sort_by(.created_at) | last | .body // ""' 2>/dev/null) LAST_VERDICT_SHA=$(echo "$LAST_VERDICT" | grep -oE 'CHANGES REQUESTED: commit [0-9a-f]+' | grep -oE '[0-9a-f]+$' | head -1) CURRENT_SHA_SHORT=$(gh pr view {PR_NUMBER} {GH_FLAG} --json headRefOid --jq '.headRefOid' 2>/dev/null | cut -c1-7) if [ -n "$LAST_VERDICT_SHA" ] && [ "$LAST_VERDICT_SHA" = "$CURRENT_SHA_SHORT" ]; then echo "HEAD unchanged since last CHANGES REQUESTED verdict ($LAST_VERDICT_SHA) — skipping re-review." # No DRY_RUN/governor guard here — consistent with every other gh issue comment/edit call # already in this file (e.g. the Push Failed / Push Blocked sections below), none of which # are gated either. This is a report-and-stop action (blocks re-review, does not merge or # delete anything), same risk class as those pre-existing calls. REREVIEW_SKIP_BODY=$(cat < SKIP_EOF ) gh issue comment {NUMBER} {GH_FLAG} --body "$REREVIEW_SKIP_BODY" # gh issue edit {NUMBER} {GH_FLAG} --add-label needs-human 2>/dev/null || true # # Return REVIEW_RESULT: status: BLOCKED — do not invoke /review-pr again on unchanged HEAD exit 1 fi ``` If `LAST_VERDICT_SHA` is empty (no prior CHANGES REQUESTED verdict found) or differs from the current HEAD (a new commit was pushed since the last verdict — e.g. by remediation), proceed normally to Phase R3. --- ## Phase R1: Pre-Push Ancestry Guard Before pushing, verify the branch contains no merge commits from branches outside the PR base ancestry. This is the final defense against milestone-code-onto-staging contamination. ```bash cd {WORKTREE_PATH} # Skip if PR_BASE does not exist on origin yet (new branch — no contamination possible) if git ls-remote --exit-code origin {PR_BASE} >/dev/null 2>&1; then MERGE_COMMITS=$(git log --merges {BRANCH} ^origin/{PR_BASE} 2>/dev/null) if [ -n "$MERGE_COMMITS" ]; then echo "PRE-PUSH ANCESTRY GUARD FAILED: merge commits from outside {PR_BASE} detected" gh issue comment {NUMBER} {GH_FLAG} --body "## Pre-Push Ancestry Guard Failed Branch \`{BRANCH}\` contains merge commits from branches outside the PR base (\`{PR_BASE}\`). Pushing this branch risks contaminating \`{PR_BASE}\` with unapproved code (e.g. milestone code leaking onto staging). **Detected merge commits**: \`\`\` ${MERGE_COMMITS} \`\`\` Do NOT push this branch. Human review required to identify the source of the merge commits and clean the branch history (e.g. via \`git rebase\` to replay only the intended commits onto \`origin/{PR_BASE}\`). " gh issue edit {NUMBER} {GH_FLAG} --add-label "needs-human" # Return REVIEW_RESULT: status: BLOCKED — do not push exit 1 fi fi ``` ## Phase R1: Non-Empty Commit Guard (MANDATORY — run before push) Before pushing, verify the branch has at least one commit ahead of the PR base. This is the last-line defense against the phantom-commit hazard: a session that resumed from a partial FORGE:BUILDER comment (without `:COMPLETE`) would have skipped the commit step and could otherwise push an empty branch. ```bash cd {WORKTREE_PATH} # Count commits on this branch that are not reachable from origin/{PR_BASE} COMMIT_COUNT=$(git rev-list --count HEAD ^origin/{PR_BASE} 2>/dev/null || echo "0") if [ "$COMMIT_COUNT" -eq 0 ]; then gh issue comment {NUMBER} {GH_FLAG} --body "## Push Blocked — No Commits Ahead of Base Branch \`{BRANCH}\` has 0 commits ahead of \`origin/{PR_BASE}\`. Pushing this branch would create an empty PR. **Likely cause**: Build was interrupted after the FORGE:BUILDER comment was posted (implement.md Phase I6) but before the commit was created (validate.md Phase V5). The branch was pushed with no implementation on it. **Resolution**: Delete this branch, re-run \`/work-on {NUMBER}\` to restart the build phase. The partial FORGE:BUILDER comment (lacking \`FORGE:BUILDER:COMPLETE\`) will be detected and deleted, and the build will restart cleanly. " gh issue edit {NUMBER} {GH_FLAG} --add-label "needs-human" exit 1 fi echo "Commit count ahead of origin/{PR_BASE}: $COMMIT_COUNT — OK to push" ``` ## Phase R1: Push Branch ```bash cd {WORKTREE_PATH} git push origin {BRANCH} ``` If push fails, retry with `--force-with-lease`: ```bash git push origin {BRANCH} --force-with-lease ``` If still fails: ```bash gh issue comment {NUMBER} {GH_FLAG} --body "## Push Failed Branch \`{BRANCH}\` could not be pushed to origin. **Error**: {ERROR_OUTPUT} This may indicate a merge conflict or remote rejection. Human review required. " gh issue edit {NUMBER} {GH_FLAG} --add-label "needs-human" ``` Return `REVIEW_RESULT: status: BLOCKED`, blocker: "git push failed". --- ## Phase R2: Create PR ### R2A: Determine PR title Derive from issue title: - `fix(...):` → `Fix: {description}` - `feat(...):` → `Feat: {description}` - `refactor(...):` → `Refactor: {description}` - `docs(...):` → `Docs: {description}` - fallback: use issue title as-is ### R2B: Resolve attribution footer (optional) Before creating the PR, check `forge.yaml → attribution.pr_footer`: ```bash ATTRIBUTION_PR_FOOTER=$(grep -A5 "^attribution:" forge.yaml 2>/dev/null | grep "pr_footer:" | awk '{print $2}' | tr -d '"' || echo "false") ``` If `ATTRIBUTION_PR_FOOTER` is `true`, append the following footer to the PR body (once — never duplicate on retries): ``` > ⚒️ Orchestrated with [ForgeDock](https://github.com/RapierCraftStudios/ForgeDock) — state, scheduling, review, and memory on GitHub. ``` ### R2C: Create PR For a batch issue, construct a non-closing reference line from `BATCH_MEMBERS` before creating the PR: ```bash BATCH_MEMBER_REFS="" if [ "${IS_BATCH:-0}" = "1" ]; then BATCH_MEMBER_REFS=$(printf 'Refs #%s\n' "${BATCH_MEMBERS[@]}") fi ``` ```bash gh pr create {GH_FLAG} \ --base {PR_BASE} \ --head {BRANCH} \ --title "{PR_TITLE}" \ --body "## Summary {BRIEF_DESCRIPTION_FROM_ISSUE_BODY} ## Changes {BULLETED_LIST_OF_KEY_CHANGES_FROM_BUILDER_COMMENT} ## Testing {TESTING_CHECKLIST_FROM_BUILDER_COMMENT} --- Closes #{NUMBER} ${BATCH_MEMBER_REFS} **Batch member disposition**: The batch issue is the code unit being closed. Referenced members that require human or operator action remain open as a split outcome. **Implementation branch**: \`{BRANCH}\` **Base**: \`{PR_BASE}\` {IF_ATTRIBUTION_PR_FOOTER_TRUE: > ⚒️ Orchestrated with [ForgeDock](https://github.com/RapierCraftStudios/ForgeDock) — state, scheduling, review, and memory on GitHub.}" ``` **Note**: `Closes #{NUMBER}` documents intent but does NOT auto-close for non-default-branch PRs. The close subcommand handles explicit closure after merge. **Attribution guard**: The footer line is appended once at PR creation. If the PR already exists (resume path), do NOT append the footer again — check the existing PR body first. **No assistant attribution**: The PR body is exactly the sections above (plus the optional ForgeDock footer). Do NOT add a `🤖 Generated with Claude Code` line, a `Co-Authored-By: Claude` trailer, or any assistant-tool attribution — the pipeline is ForgeDock-branded. A PreToolUse guard hard-blocks it as a backstop (`bin/hooks/pre-tool-use.mjs` Rule 5). If PR creation fails because a PR already exists for this branch: ```bash gh pr list {GH_FLAG} --head {BRANCH} --json number,url --jq '.[0]' ``` Use the existing PR number and continue. ### R2D: Update labels ```bash gh issue edit {NUMBER} {GH_FLAG} \ --add-label "workflow:in-review" \ --remove-label "workflow:building" ``` --- ## Phase R3: Invoke /review-pr with --auto-merge Re-read the PR number (from creation or from resume check): ```bash PR_NUMBER=$(gh pr list {GH_FLAG} --head {BRANCH} --json number --jq '.[0].number') ``` Post a progress comment before delegating: ```bash gh issue comment {NUMBER} {GH_FLAG} --body "## Submitting for Review PR #${PR_NUMBER} created targeting \`{PR_BASE}\`. Invoking /review-pr with --auto-merge. Review will: analyze changes → spawn domain agents → post findings → merge → close issue → clean up worktree. " ``` Invoke the review command: ``` Skill(skill="review-pr", args="{PR_NUMBER} --auto-merge --issue {NUMBER} --base {PR_BASE} --gh-flag {GH_FLAG}") ``` **OpenCode joined-child contract**: When `FORGE_RUNTIME=opencode` (or an OpenCode runtime marker is present), invoke this load-bearing review through one native foreground `task` instead of treating the `Skill(...)` line as an asynchronous handoff: ``` if DRY_RUN=true: record "Would invoke the foreground review task for PR #{PR_NUMBER}." else: task( description="Review PR #{PR_NUMBER}", subagent_type="general", background=false, prompt="Load commands/review-pr.md and execute it for PR {PR_NUMBER} with --auto-merge --issue {NUMBER} --base {PR_BASE} --gh-flag {GH_FLAG}. Return only the structured REVIEW_RESULT block after the review reaches its outcome." ) ``` Wait for that task's completed result before Phase R4. Propagate its `REVIEW_RESULT` as this module's child state; do not return `REVIEW_RESULT`, report progress, release an orchestrator slot, or begin close work while the child is running. If the child errors or returns no parseable `REVIEW_RESULT`, return `REVIEW_RESULT: status: BLOCKED` with the child failure as the blocker. The normal `Skill(...)` invocation above remains the non-OpenCode path. /review-pr handles: full domain-agent review → post findings as separate issues (non-blocking) → merge the PR → close the issue → clean up worktree. --- ## Phase R4: Verify Review Outcome After /review-pr returns, verify the outcome: ```bash # Check PR state gh pr view {PR_NUMBER} {GH_FLAG} --json state,mergedAt --jq '{state: .state, mergedAt: .mergedAt}' # Check issue state gh issue view {NUMBER} {GH_FLAG} --json state --jq '.state' ``` **Cases**: - PR MERGED (issue OPEN or CLOSED) → write checkpoint, then return `REVIEW_RESULT: status: COMPLETE` — do NOT close the issue or add labels here; the router will route to `work-on:close` which handles issue closure, label updates, project board, trajectory log, and worktree cleanup. Write machine-readable phase checkpoint before returning (MANDATORY when PR is MERGED): ```bash CHECKPOINT_TIMESTAMP=$(date -u +"%Y-%m-%dT%H:%M:%SZ") gh issue comment {NUMBER} {GH_FLAG} --body " \`\`\`json {\"phase\": \"REVIEW\", \"status\": \"COMPLETE\", \"next_phase\": \"CLOSE\", \"timestamp\": \"${CHECKPOINT_TIMESTAMP}\"} \`\`\`" ``` - PR NOT MERGED → attempt manual merge: ```bash gh pr merge {PR_NUMBER} {GH_FLAG} --merge --auto ``` If merge fails: post comment, add `needs-human`, return `REVIEW_RESULT: status: BLOCKED` --- ## Output **After posting this result, immediately proceed to the close subcommand — do NOT stop here. `REVIEW_RESULT: status: COMPLETE` is an intermediate result, NOT a terminal state. The pipeline is not done. You MUST invoke `Skill("work-on:close", ...)` now to close the issue, update labels to `workflow:merged`, post the trajectory log, and clean up the worktree.** Output this structured block: ``` REVIEW_RESULT: status: COMPLETE | ALREADY_MERGED | BLOCKED pr_number: {PR_NUMBER} pr_url: {PR_URL} merged_to: {PR_BASE} blocker: {description if status=BLOCKED} ``` --- ## Integration Point in work-on.md This module runs at **Phases 4–5** — after validate returns `GATE_PASSED: true`, before close: ``` 3F.5 → Validate (by build/validate.md) — gate passed 4 → [THIS MODULE] Push + PR creation + /review-pr invocation + merge verification 5 → Close (by close.md) — trajectory, parent tracker, summary ``` /review-pr is invoked within this module (not by the router). The router waits for REVIEW_RESULT before invoking close.md.