Finishing A Development Branch

---
name: finishing-a-development-branch
disable-model-invocation: true
source: superpowers-plus
augment_menu: true
# Override rationale: Adds mandatory Step 0 (autonomous code review via
# code-review-battery) before presenting integration options. The upstream
# version goes straight to "verify tests → present options," which allows
# unreviewed work to be presented as ready for human action.
triggers:
  - /sp-finish
  - finishing a branch
  - branch is done
  - ready to merge
  - implementation complete
  - all tasks done
  - work is finished
anti_triggers:
  - starting a branch
  - creating a branch
description: "Use when implementation is complete, all tests pass, and you need to decide how to integrate the work. Mandates autonomous code review (Step 0) before presenting options. Guides completion of development work by presenting structured options for merge, PR, or cleanup."
summary: "Use when: branch work is done. Enforces review-before-options via code-review-battery. Skip when: still actively implementing."
coordination:
  group: completion-gate
  order: 3
  requires:
    - verification-before-completion
  enables: []
  escalates_to: []
  internal: false
composition:
  consumes: [code-changes]
  produces: [merge-ready-branch]
  capabilities: [orchestrates-workflow, sequences-skills]
  priority: 10
---

> **Wrong skill?** Quick pre-commit check → `unified-commit-gate`. Presenting results mid-work → `verification-before-completion`. Reviewing someone's PR → `providing-code-review`.

# Finishing a Development Branch

## When to Use

Invoke when development work on a branch is complete and ready to review, commit, or ship.

## Overview

Guide completion of development work by presenting clear options and handling chosen workflow.

**Core principle:** Autonomous code review → Verify tests → Present options → Execute choice → Clean up.

**Announce at start:** "I'm using the finishing-a-development-branch skill to complete this work."

## The Process

### 🔴 Step 0: Autonomous Code Review (NON-NEGOTIABLE)

**Before verifying tests, before presenting options — check the sentinel, then run battery if needed.**

First, run the `code-review-battery` Phase 0 sentinel check:

```bash
SENTINEL="$(git rev-parse --show-toplevel 2>/dev/null || echo .)/.code-review-cleared"
cat "$SENTINEL" 2>/dev/null || echo "NO CLEARANCE"
echo "HEAD: $(git rev-parse HEAD 2>/dev/null)"
git diff --quiet && git diff --cached --quiet && echo "WORKTREE_CLEAN" || echo "WORKTREE_DIRTY"
```

Then, **regardless of sentinel state**, enumerate which `.md` files were changed via the canonical helper, then split the result into PHR-eligible vs. llm-skill-review-owned (`skills/*.md`, `.ai-guidance/*.md`, and any AGENTS.md-family file — `AGENTS.md`/`CLAUDE.md`/`GEMINI.md`/`CODEX.md`/`COPILOT.md`/`AGENT.md`, at any path depth; see `tools/md-files-changed.sh`'s `LLM_OWNED_REGEX` for the single source of truth on this boundary — owned exclusively by `llm-skill-review`, not PHR):

```bash
# Exit 0 → files listed on stdout; exit 1 → no PHR-relevant files; exit 2 → NO_BASE_FOUND.
MD_HITS="$(tools/md-files-changed.sh)"; MD_STATUS=$?
case $MD_STATUS in
    0)
        LLM_OWNED_MD=$(tools/md-files-changed.sh --files "$MD_HITS" --llm-owned 2>/dev/null || true)
        DESIGN_MD=$(tools/md-files-changed.sh --files "$MD_HITS" --exclude-llm-owned 2>/dev/null || true)
        [[ -n "$DESIGN_MD" ]] && echo ">>> PHR REQUIRED for: $DESIGN_MD"
        [[ -n "$LLM_OWNED_MD" ]] && echo ">>> LLM-SKILL-REVIEW REQUIRED for: $LLM_OWNED_MD"
        ;;
    1) echo "NO_MD_FILES_CHANGED" ;;
    2) echo "NO_BASE_FOUND — cannot determine diff scope; review the full branch manually" ;;
esac
```

The helper is the single source of truth for the PHR-trigger regex, the exclusions (`README.md`, `CHANGELOG.md`), and the PHR-vs-llm-skill-review ownership split (`--llm-owned` / `--exclude-llm-owned`); `tools/run-battery.sh` consumes the same script. If exit code is 2 (`NO_BASE_FOUND`), treat both gates as required by default and review the full branch manually.

| Result | Action |
|--------|--------|
| Valid sentinel for HEAD AND `WORKTREE_CLEAN` AND md-file check → `NO_MD_FILES_CHANGED` | Battery evidence confirmed. Proceed directly to Step 1. |
| Valid sentinel for HEAD AND `WORKTREE_CLEAN` AND md-file check lists design (PHR-owned) files | Battery passed. **Also invoke PHR** (`/sp-phr`) on the listed files before Step 1 — see below. |
| Valid sentinel for HEAD AND `WORKTREE_CLEAN` AND md-file check lists llm-skill-review-owned files | Battery passed (PHR-owned portion only, if any). **Invoke `llm-skill-review`** on the listed files instead of PHR — it supersedes, not supplements, PHR for that file class. |
| Any other result | Dispatch `code-review-battery` (via `sub-agent-code-reviewer`). Fix all Critical and Important findings. Re-dispatch if fixes were made. **Only proceed when the battery verdict is PASS or PASS_WITH_NITS.** Then re-run the md-file check above and apply the rows above (PHR for design files, llm-skill-review for llm-owned files). |

**PHR is mandatory when the md-file check lists any non-llm-owned `.md` files. llm-skill-review-owned files (`skills/*.md`, `.ai-guidance/*.md`, AGENTS.md-family) require `llm-skill-review` instead — never both *for the same file*. A push touching both file classes (e.g. a `docs/architecture.md` change alongside a `skills/x/skill.md` change) still needs both sentinels, one per file class.**

Scope (PHR-owned, i.e. the `--exclude-llm-owned` subset): any `.md` file under `docs/`, plus repo-root `.md` files whose names start with an uppercase letter (e.g., `DESIGN.md`, `ARCHITECTURE.md`) that are NOT one of the AGENTS.md-family basenames. Excludes: `CHANGELOG.md`, `README.md` (excluded by the post-grep `grep -vE` filter, not by the main regex — the main regex matches them), and everything llm-skill-review owns (`skills/*.md`, `.ai-guidance/*.md`, AGENTS.md-family — routed to `llm-skill-review` instead, per the split above).

The battery runs automated linting and tests — it does NOT run PHR or llm-skill-review. Invoke `progressive-harsh-review` (`/sp-phr`) for design `.md` files, or `llm-skill-review` for `skills/*.md`, `.ai-guidance/*.md`, or AGENTS.md-family files, before proceeding to Step 1. A passing battery sentinel is NOT a substitute for either.

> **Why separate?** `run-battery.sh` calls `harsh-review.sh` (a shell linter), not the multi-persona PHR/llm-skill-review skills. Those are AI judgment gates; the battery is an automated script gate. Both an AI judgment gate (PHR or llm-skill-review, whichever applies) and the battery are required for design/skill changes — but never both AI gates for the same file.

If you skip this step and present work as "ready" to the human, you have violated the gate.

**Why this step exists:** See the 2026-04-02 incident in `verification-before-completion` Incident History.

### Step 1: Verify Tests

**After Step 0 passes, verify tests pass:**

```bash
# Run project's test suite
npm test / cargo test / pytest / go test ./...
```

**If tests fail:** Stop. Show failures. Do not proceed to Step 1.5.

**If tests pass:** Continue to Step 1.5.

### Step 1.5: Verify Branch Base — HARD STOP

**Before presenting options, verify the branch is not behind `origin/main`.**

> **Assumption:** This step assumes `origin/main` is the integration branch. If the branch targets a different base (feature branch, release branch), set `BASE_REF` to the correct remote ref (e.g., `origin/dev`, `origin/release-X`). Use the same `BASE_REF` in the recovery rebase below. If the repo uses a fork model with separate `origin`/`upstream` remotes, substitute accordingly.

```bash
BASE_REF="origin/main"      # ← substitute if branch targets a different remote
FORCE_PUSH_REQUIRED=false   # default; overridden in recovery steps if a rebase occurs
git fetch origin || { echo "FETCH FAILED — cannot verify branch base. Do not proceed."; false; }
git log --oneline "HEAD..${BASE_REF}"
```

If `git log` errors rather than producing empty output, the remote ref does not exist — run `git ls-remote origin` to inspect the remote's branch list before proceeding.

| Result | Action |
|--------|--------|
| Empty output | Branch is current. Proceed to Step 2. |
| Any commits listed | **HARD STOP** — *(do not open or update any PR until recovery completes)* — see recovery steps below. |

**Recovery when branch is behind `origin/main`** (track attempt number using `ATTEMPT_FILE="$(git rev-parse --git-common-dir)/rebase-attempt-count"`; read with `cat "$ATTEMPT_FILE" 2>/dev/null || echo 0`; if absent this is attempt 1; write `echo 1 > "$ATTEMPT_FILE"` or `echo 2 > "$ATTEMPT_FILE"`; using `--git-common-dir` works in worktrees):

1. Check for rebase in progress: `git rebase --show-current-patch 2>/dev/null && echo REBASE_IN_PROGRESS || echo CLEAN`. If `REBASE_IN_PROGRESS`, ask human to confirm abort before running `git rebase --abort`.
2. Capture stash state: `git status --short`. If uncommitted changes exist, run `git stash push -m "pre-rebase WIP" && STASHED=true || { echo "STASH FAILED — manual intervention required."; STASHED=false; return 1; }`. If clean, note `STASHED=false` — do not run `git stash pop` later.
3. Detect force-push requirement: `git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null && echo FORCE_PUSH_REQUIRED=true || echo FORCE_PUSH_REQUIRED=false`. Then rebase: `git rebase "${BASE_REF:-origin/main}"`. If `FORCE_PUSH_REQUIRED=true`, push will need `--force-with-lease`; confirm with human at Step 4 Option 2.
4. If rebase produced conflicts: resolve them, `git add <files>`, `git rebase --continue`, then follow `unified-commit-gate § Post-Conflict Trap` (typecheck → lint → test). If no conflicts: proceed to step 5.
5. Restore stash (only if `STASHED=true` and rebase complete): `git stash pop`. If conflicts, resolve then re-run typecheck → lint → test.
6. Re-run Step 0 (rebase rewrites HEAD, sentinel stale), re-run Step 1 (tests), then re-run Step 1.5. If clean, run `rm -f "$ATTEMPT_FILE"`. If this was attempt 2 and Step 1.5 still shows commits: **escalate to human** — do not run a third rebase.

**Why this step exists:** A prior production incident occurred when a branch was cut before a revert landed on `main`. No branch-base check was run before pushing. Conflicts appeared only in the remote MR diff, requiring multi-session destructive rebasing and a lost typecheck that left a broken build in CI.

### Step 2: Determine Base Branch

Prefer the branch's own tracking upstream, then this repo's actual workflow
base (`dev`), then `main`/`master` as later fallbacks -- falling
back straight to `main` first (as an earlier version of this step did) picks
a base that can be many commits behind `dev` on repos where `dev` is the
real integration branch, producing a wrong "what changed" diff. The exact
candidate order below is a verbatim match of `tools/md-files-changed.sh`'s
`resolve_base()`, which this repo's own PHR-eligibility check already
relies on for the identical problem (see also
`tools/lib/pre-push-diff-range.sh`'s `resolve_push_base_ref()`, a related
but remote-qualified superset used by the pre-push hook itself).

```bash
tracking="$(git rev-parse --abbrev-ref '@{upstream}' 2>/dev/null || true)"
for candidate in "$tracking" origin/dev dev origin/main main origin/master master; do
    [[ -n "$candidate" ]] || continue
    git rev-parse --verify "$candidate" >/dev/null 2>&1 || continue
    if merge_base=$(git merge-base HEAD "$candidate" 2>/dev/null); then
        echo "$merge_base"
        break
    fi
done
```

If any candidate returns a SHA, use that branch as the base. If all fail, the branch has no recognizable integration ancestor — surface this explicitly: "Could not determine base branch automatically (no tracking/dev/main/master ancestor found). What branch did this work split from?"

### Step 3: Present Options

Present exactly these 4 options:

```
Implementation complete. What would you like to do?

1. Merge back to <base-branch> locally
2. Push and create a Pull Request
3. Keep the branch as-is (I'll handle it later)
4. Discard this work

Which option?
```

### Step 4: Execute Choice

Capture the worktree state before any option below `cd`'s away — see `references/worktree-cleanup.md` for the exact commands and why order matters.

**Option 1 (Merge locally):** Checkout base → pull → merge → verify tests on result → delete branch → cleanup worktree (using `$WORKTREE_PATH`).

**Option 2 (Push and create PR):**

1. Run `tools/branch-flow-preflight.sh <current-branch> <base-branch>` to validate the (source, target) pair against the canonical flow and write `.branch-flow-cleared`. The pre-push hook reads this sentinel and refuses pushes to dev/main without it.
2. Push the branch. `unified-commit-gate` (push mode) fires before push.
3. Create the PR: `gh pr create --fill` (`--fill` pre-fills title/body from commits; override with `--title`/`--body`). If the work has an issue identifier (per git-branch-conventions' "Before Naming the Branch"), the title should include it, matching branch/commit.
4. GitHub auto-links AND auto-closes an issue if the PR body contains `Fixes #123` or `Closes #123` **and the PR merges into the repo's default branch** (not just any branch) — confirm that's actually intended before using that phrasing; a plain reference (`#123`) links without closing. If citing an issue, verify it resolves via `issue-verify` first — never construct the reference from memory.
5. `gh pr edit` has a body-replacement footgun when adding a link after the fact — see `external-cli-audit`'s CLI Default Hazards table.
6. Cleanup the worktree per **Step 5: Cleanup Worktree** below.

**Option 3 (Keep as-is):** Report status. Keep worktree.

**Option 4 (Discard):** Confirm first (require typed "discard"). Then delete branch and cleanup worktree (using `$WORKTREE_PATH`).

### Step 5: Cleanup Worktree

For Options 1, 2, 4 — using the values captured in Step 4, per `references/worktree-cleanup.md`. For Option 3 — keep worktree, skip this step.

## Anti-Patterns

| Wrong | Right |
|-------|-------|
| Presenting options without completing Step 0 | Review first, options second |
| Skipping test verification because review passed | Review ≠ tests. Both required. |
| Open-ended "What should I do next?" | Present exactly 4 structured options |
| Auto-cleanup worktree for Option 3 | Only cleanup for Options 1, 2, and 4 |
| Deleting work without confirmation | Require typed "discard" for Option 4 |
| Citing an issue reference in a PR body without verifying it resolves | Verify via `issue-verify` before citing; never build the reference from memory |

## Red Flags

- Presenting work as "ready" without completing Step 0
- Proceeding with failing tests
- Force-pushing without explicit request
- Merging without verifying tests on result
- `--force`-removing a worktree that refused removal — see `references/worktree-cleanup.md`

## Companion Skills

- **code-review-battery**: The review engine Step 0 dispatches
- **progressive-code-review-gate**: Verdict mapping and dispatch procedure
- **verification-before-completion**: Fires before this skill (completion-gate order 2)
- **unified-commit-gate** (push mode): Fires when Option 2 triggers a push; § Post-Conflict Trap mandates typecheck after rebase/stash-pop conflicts
- **subagent-driven-development**: Calls this skill after all tasks complete
- **issue-verify**: Verify an issue identifier before citing it in a branch, commit, or PR
- **external-cli-audit**: CLI default hazards (e.g. `gh pr edit`'s body-replacement behavior) referenced from Option 2