Finishing a Development Branch

Overview

All subagent dispatches use disk-mediated dispatch. See

shared/dispatch-convention.md

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

Core principle: Verify tests -> Code review -> Red-team -> Present options -> Execute choice -> Clean up.

Announce at start: "I'm using the finish skill to complete this work."

The Process

Step 1: Verify Tests

Before presenting options, verify tests pass:

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

If tests fail:

Tests failing (<N> failures). Must fix before completing:

[Show failures]

Cannot proceed with merge/PR until tests pass.

Stop. Don't proceed to Step 2.

If tests pass: Continue to Step 2.

Step 2: Code Review (Mandatory)

Before presenting options, run a full code review.

REQUIRED SUB-SKILL: Use crucible:code-review

1. Get base and head SHAs:

BASE_SHA=$(git merge-base HEAD main 2>/dev/null || git merge-base HEAD master)
HEAD_SHA=$(git rev-parse HEAD)

2. Check diff size to determine review approach:

git diff --stat $(git merge-base HEAD main 2>/dev/null || git merge-base HEAD master)...HEAD

3. Dispatch a code review subagent (general-purpose) using the

   code-review/code-reviewer.md

   template with:
   • What was implemented (summary of branch work)
   • The plan or requirements it was built against
   • Base and head SHAs
   • Brief description
   • For large diffs (20+ files changed): provide the

     --stat

     summary and key files list, let the reviewer pull targeted diffs rather than receiving the entire diff. Consider splitting into multiple focused reviewers -- one per subsystem.

4. Act on feedback:

   • Critical issues: Fix immediately. Re-run tests. Do NOT proceed.
   • Important issues: Fix before proceeding. Re-run tests.
   • Minor issues: Note them. Fix if quick, otherwise include in PR description.

5. If fixes were made, re-run tests to confirm nothing broke.

Do NOT skip this step. The orchestrator did lightweight review during execution -- this is the comprehensive review before integration.

Step 2.5: Test Alignment Audit

RECOMMENDED SUB-SKILL: Use crucible:test-coverage — audit whether existing tests are still aligned with the changes on this branch. Invoke with:

• Code diff:

  git diff <base-branch>..HEAD

• Affected test files: test files in the diff or test files that import changed modules
• Context: "Finish pre-merge audit for [branch description]"

The test-coverage skill handles its own fix dispatch and revert-on-failure logic.

Skip this step when:

• The branch diff contains no behavioral source changes (only

  .md

  ,

  .json

  ,

  .yaml

  , config files)
• Build told finish to skip Step 2.5 (test-coverage already ran per-task in Phase 3)

Step 2.75: Forge Retrospective

RECOMMENDED SUB-SKILL: Use crucible:forge (retrospective mode) — capture what happened vs what was planned while execution context is still fresh. Run this BEFORE red-team so the retrospective has access to the full execution state.

Step 3: Red-Team the Implementation (Mandatory)

After code review passes, red-team the full implementation.

REQUIRED SUB-SKILL: Use crucible:red-team

1. Dispatch

   crucible:red-team

   on the full implementation:
   • Artifact: the complete set of changes on this branch (provide

     git diff --stat

     and key files)
   • Context: the design doc or plan this was built against
   • Fix mechanism: dispatch fix subagent for any findings
2. The red-team skill handles the iterative loop (fresh Devil's Advocate each round, stagnation detection)
3. Fix all Fatal/Significant findings before proceeding

Do NOT skip this step. Code review checks quality; red-teaming checks whether the system will actually work and survive real use.

Step 3.5: Noticed But Not Touching — Optional Issue Conversion

Check for

docs/plans/*-noticed.md

Found <N> noticed-but-not-touching entries in <noticed.md path>. Convert any to GitHub issues?

On confirmation, display a numbered list of entries and ask which to convert. For each selected entry, create an issue via

gh issue create

noticed

why it matters

suggested follow-up

-noticed.md

Step 4: Determine Base Branch

# Try common base branches
git merge-base HEAD main 2>/dev/null || git merge-base HEAD master 2>/dev/null

Or ask: "This branch split from main - is that correct?"

Step 5: 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?

Don't add explanation - keep options concise.

MANDATORY CHECKPOINT - DO NOT SKIP

Step 5.5: Pre-Push Validation (Non-Negotiable)

BLOCK semantics: you CANNOT proceed to Option 1 (merge) or Option 2 (push + PR) until local validation passes. A failing check is a hard stop. Do not push "then fix in CI"; do not merge "then fix on main."

Detect the project's toolchain first by reading manifest files at repo root:

package.json

Cargo.toml

pyproject.toml

requirements.txt

go.mod

*.csproj

*.sln

Gemfile

build.gradle

pom.xml

• Monorepo / polyglot root: if multiple manifests coexist at root (e.g., a Rust product with a Node.js tooling script), scope validation to whichever ecosystem owns the files changed in the diff (

  git diff --name-only <base>...HEAD

  ). If ambiguous, ask the user before running the full matrix.
• Nested manifests: if the diff touches files under a subdirectory with its own manifest, scope to that subdirectory.

Silently-missing tools are NOT a pass — they are an "unknown." Either run the real tool, or narrate the skip ("no type-check configured — skipping"), or ask the user. Never mask a failure with

|| true

2>/dev/null

Validation matrix (run every applicable check; each must exit 0 unless its documented exit-code contract says otherwise):

npx tsc --noEmit

tsconfig.json

npm run lint

pnpm lint

biome check

prettier --check .

biome format --check

npm test

pnpm test

vitest run

jest

cargo check --all-targets --all-features

cargo clippy --all-targets --all-features -- -D warnings

cargo fmt -- --check

cargo test --workspace --all-features

mypy

pyright

ruff check

flake8

ruff format --check

black --check

pytest

go build ./...

go vet ./...

golangci-lint run

out=$(gofmt -l .) && [ -z "$out" ]

go test ./...

dotnet build -p:TreatWarningsAsErrors=true

-p:TreatWarningsAsErrors=true

dotnet format --verify-no-changes

dotnet test

bundle exec rubocop

bundle exec standardrb

bundle exec rspec

bundle exec rake test

./gradlew build

./gradlew checkstyleMain spotbugsMain

./mvnw spotbugs:check

./gradlew spotlessCheck

./gradlew test

./mvnw test

For ecosystems not in this matrix, extend it: manifest → type-check → lint → format-check → test. Do not skip an ecosystem because it isn't listed.

Exit-code interpretation: treat each tool's documented exit-code contract authoritatively, not just 0 vs non-0. Example:

gh pr checks

On ANY unhandled non-zero exit (after exit-code interpretation): STOP. Report the failure, dispatch a fix, and re-run the full matrix from scratch. Do not partially re-run — a fix in one layer can regress another.

Step 6: Execute Choice

Option 1: Merge Locally

# Switch to base branch
git checkout <base-branch>

# Pull latest
git pull

# Merge feature branch
git merge <feature-branch>

# Verify tests on merged result
<test command>

# If tests pass
git branch -d <feature-branch>

Then: If using a worktree, clean it up (Step 7)

Option 2: Push and Create PR

Repository Safety Check (before push):

# Check if repo is public
IS_PRIVATE=$(gh repo view --json isPrivate -q .isPrivate)

If the repo is public: scan the PR title, body, and commit messages for proprietary company information, internal names, internal URLs, or sensitive data. STOP and confirm with the user if anything looks sensitive. This check is mandatory — a prior incident involved filing proprietary information to a public repo.

# Push branch
git push -u origin <feature-branch>

# Create PR and capture the URL gh emits (the PR it just created).
# This is the only deterministic PR reference — using `gh pr view`
# instead would use branch-to-PR mapping, which breaks on repos
# with multiple open PRs per branch.
PR_URL=$(gh pr create --title "<title>" --body "$(cat <<'EOF'
## Summary
<2-3 bullets of what changed>

## Test Plan
- [ ] <verification steps>
EOF
)")
PR_NUMBER="${PR_URL##*/}"

# Guard: if gh pr create failed (e.g., a PR already exists for this branch),
# PR_URL is empty — surface the real diagnostic instead of falling through
# into CI monitoring and emitting a misleading "CI failed" message.
if [ -z "$PR_URL" ] || [ -z "$PR_NUMBER" ]; then
  echo "gh pr create did not return a PR URL — possible duplicate PR. Inspect: gh pr list --head <feature-branch>"
  exit 1
fi

Post-Push CI Monitoring (Non-Negotiable): after

gh pr create

# Primary: --watch streams status and returns aggregate exit code.
# Exit 0 = all terminal passes OR no checks configured (must disambiguate).
# Non-zero = at least one check failed/cancelled.
gh pr checks "$PR_NUMBER" --watch
WATCH_RC=$?

# Disambiguate the no-CI case from real success.
CHECK_COUNT=$(gh pr checks "$PR_NUMBER" --json bucket --jq 'length')

if [ "$CHECK_COUNT" = "0" ]; then
  echo "No CI checks configured — record in final report and recommend adding CI"
elif [ "$WATCH_RC" -ne 0 ]; then
  echo "CI failed (watch exit $WATCH_RC)"; exit 1
else
  echo "CI green"
fi

Fallback (if

--watch

{pass, skipping}

bucket

state

NEUTRAL

ACTION_REQUIRED

gh exit-code contract: per gh's own documentation,

gh pr checks

while true; do
  BUCKETS=$(gh pr checks "$PR_NUMBER" --json bucket --jq '[.[].bucket] | unique')
  RC=$?
  echo "CI buckets: $BUCKETS"
  case "$RC" in
    0|1) break ;;                                 # 0 = all pass, 1 = at least one failed; both terminal — assertion classifies
    8) sleep 20 ;;                                # at least one pending — keep polling
    *) echo "gh pr checks errored (rc=$RC) — cannot determine CI state"; exit 1 ;;
  esac
done

# Three terminal cases: empty (no CI), all allow-list (green), or anything else (red).
if [ "$BUCKETS" = "[]" ]; then
  echo "No CI checks configured — record in final report and recommend adding CI"
elif echo "$BUCKETS" | jq -e 'all(. == "pass" or . == "skipping")' >/dev/null; then
  echo "CI green"
else
  echo "CI not all-green: $BUCKETS"; exit 1
fi

If the exit is non-zero (either via

--watch

gh run view <run-id> --log-failed

gh pr checks "$PR_NUMBER"

If the block exits with the "No CI checks configured" message, record that in the final report so the user knows local validation was the only gate, and recommend they add CI.

Then: If using a worktree, clean it up (Step 7)

Option 3: Keep As-Is

Report: "Keeping branch <name>."

If using a worktree: "Worktree preserved at <path>."

Option 4: Discard

Confirm first:

This will permanently delete:
- Branch <name>
- All commits: <commit-list>

Type 'discard' to confirm.

Wait for exact confirmation.

If confirmed:

git checkout <base-branch>
git branch -D <feature-branch>

Then: If using a worktree, clean it up (Step 7)

Step 7: Cleanup Worktree (If Applicable)

Skip this step if not using git worktrees.

For Options 1, 2, and 4:

Check if in worktree:

git worktree list | grep $(git branch --show-current)

If yes:

git worktree remove <worktree-path>

For Option 3: Keep worktree.

Quick Reference

Common Mistakes

Skipping test verification

• Problem: Merge broken code, create failing PR
• Fix: Always verify tests before offering options

Skipping code review

• Problem: Subtle bugs, architectural violations, and style drift make it into the branch
• Fix: Always run crucible:code-review before presenting options. The orchestrator's lightweight review during execution is not sufficient.

Open-ended questions

• Problem: "What should I do next?" -> ambiguous
• Fix: Present exactly 4 structured options

Automatic worktree cleanup

• Problem: Remove worktree when might need it
• Fix: Only cleanup worktree for Options 1, 2, and 4 -- and only if actually using worktrees

No confirmation for discard

• Problem: Accidentally delete work
• Fix: Require typed "discard" confirmation

Skipping pre-push validation

• Problem: "Tests passed during /build so this should be fine" — but refactors between Phase 3 and finish, plus drift in companion files, can silently break the build. Pushing broken code wastes CI minutes and creates a red PR for reviewers.
• Fix: Always run Step 5.5's full validation matrix before Option 1 merge or Option 2 push. Every applicable check must exit 0.

Silencing validation failures

• Problem:

  2>/dev/null || true

  patterns hide tool failures and make "no output" indistinguishable from "tool missing" — a failing tsc looks identical to a repo without TypeScript.
• Fix: Detect the toolchain from manifest files first, then run only the checks that apply with strict exit-code discipline. Explicitly narrate skipped checks rather than silencing errors.

Pushing and moving on

• Problem:

  git push

  returns,

  gh pr create

  returns a URL, task feels done. CI runs later, fails, and nobody notices until the next review session.
• Fix: Block on

  gh pr checks <pr-number> --watch

  (or equivalent poll loop). Treat a red PR as a hard stop — never report success to the user on a failing PR.

Red Flags

Never:

• Proceed with failing tests
• Skip code review because "it looks fine" or "subagents already reviewed it"
• Skip red-team because "code review already passed"
• Merge without verifying tests on result
• Delete work without confirmation
• Force-push without explicit request
• Push code that has not passed the full local validation matrix in Step 5.5
• Mask a non-zero exit code without interpreting it against the tool's documented contract (e.g.,

  || true

  ,

  2>/dev/null

  ). Known non-failure non-zero codes must be matched to their meaning —

  gh pr checks

  exit 8 is "checks pending," not failure. When a tool's contract is unclear, treat non-zero as failure.
• Report success to the user after

  gh pr create

  without confirming all CI checks pass
• Abandon a watched PR to work on something else — a red PR is an actionable blocker

Always:

• Verify tests before code review
• Run full code review before presenting options
• Run red-team after code review passes, before presenting options
• Fix Critical/Important review findings before proceeding
• Detect the project toolchain from manifest files before Step 5.5 dispatch
• Run every applicable validation check with strict exit-code discipline
• Watch CI to completion after push (

  gh pr checks --watch

  ) before declaring Option 2 done
• Present exactly 4 options
• Get typed confirmation for Option 4
• Clean up worktree (if applicable) for Options 1, 2 & 4 only

Integration

Called by:

• build (Phase 4) - After all tasks complete

Pairs with:

• worktree - Cleans up worktree (if applicable)
• crucible:red-team — Adversarial review before presenting options. Note: finish uses

  crucible:red-team

  directly rather than

  crucible:quality-gate

  because it doesn't produce a typed artifact — it's a pre-completion sanity check, not an iterative gate.

Recommended:

• crucible:test-coverage — Test alignment audit between code review and red-team (Step 2.5)
• crucible:forge — Retrospective between test audit and red-team (Step 2.75)
• crucible:merge-pr — Handles merge execution with CI verification (Step 6, Option 2)

Gate Execution Ledger

Before completing this skill, confirm every mandatory checkpoint was executed:

• Test verification
• Code review
• Test alignment audit (if applicable)
• Red-team review
• Pre-push validation passed
• Repository safety checked (if public repo, Option 2)
• Post-push CI monitoring completed green (Option 2 only)

If any checkbox is unchecked, STOP. Go back and execute the missed gate.