diff options
| author | Craig Jennings <c@cjennings.net> | 2026-05-22 14:54:44 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-05-22 14:54:44 -0500 |
| commit | bf1c57d7b9f750af68cb74a2f6fc1477ad7f0e44 (patch) | |
| tree | 4ff8d0fd222c2c38e94214bdac99cee9418476e7 | |
| parent | 6efa7f13b7e901e8909e5edc652d0bc9b2a2d3df (diff) | |
| download | rulesets-bf1c57d7b9f750af68cb74a2f6fc1477ad7f0e44.tar.gz rulesets-bf1c57d7b9f750af68cb74a2f6fc1477ad7f0e44.zip | |
docs(commands): fix finish-branch base detection and merge safety
Two audit fixes. Base-branch detection returned a merge-base SHA where a branch name was needed. Phase 2 now resolves the branch name (open PR base, then origin/HEAD, then ask) and computes the merge-base SHA separately. Option 1's merge gained pre-flight safety: a dirty-tree refusal with no auto-stash, protected-branch awareness, an upstream-gated ff-only pull, and merge-commit-vs-rebase as a team-policy choice. Worktree detection moved from grepping branch names to a git-dir vs git-common-dir comparison.
| -rw-r--r-- | .claude/commands/finish-branch.md | 78 |
1 files changed, 68 insertions, 10 deletions
diff --git a/.claude/commands/finish-branch.md b/.claude/commands/finish-branch.md index 9638f5b..cba11a9 100644 --- a/.claude/commands/finish-branch.md +++ b/.claude/commands/finish-branch.md @@ -44,21 +44,41 @@ If any verification step fails and triggers sub-investigations, follow `subagent ## Phase 2 — Determine Base Branch -The four outcomes need to know what the branch is being integrated into. Find the base: +The four outcomes need to know what the branch is being integrated into. This is the **branch name** — the thing you check out, pull, merge into, and pass to `gh pr create --base`. It is not a merge-base SHA; the merge-base is a separate value computed below. + +Resolve the base branch name in priority order: + +1. **The base of an existing PR.** If the branch already has a PR open, that PR's base is authoritative: + + ```bash + gh pr view --json baseRefName --jq .baseRefName 2>/dev/null + ``` + +2. **The remote's configured default branch.** When there's no PR, ask the remote what it defaults to and strip the result down to the bare branch name: + + ```bash + git symbolic-ref --short refs/remotes/origin/HEAD 2>/dev/null | sed 's@^origin/@@' + ``` + + `refs/remotes/origin/HEAD` is set by `git clone`; if it's missing, refresh it with `git remote set-head origin --auto` and retry. + +3. **Ask the user.** If neither resolves: "I couldn't determine the base branch from an open PR or the remote's default. What's the base branch for this work?" Don't guess. + +Store the answer as `<base-branch>` (the name) and use it everywhere a branch name is needed. + +**Compute the merge-base separately.** A few steps need the merge-base *commit*, not the branch name — the commit range to integrate, the count of branch-only commits, the discard preview. Derive it from the resolved branch name: ```bash -git merge-base HEAD main 2>/dev/null \ - || git merge-base HEAD master 2>/dev/null \ - || git merge-base HEAD develop 2>/dev/null +git merge-base <base-branch> HEAD # the commit where this branch diverged ``` -If none resolves, ask: "I couldn't find a merge base against `main`, `master`, or `develop`. What's the base branch for this work?" Don't guess. +Keep the two values distinct: `<base-branch>` is a ref name for checkout/pull/merge/PR; the merge-base SHA is only for computing what's on the branch. Also collect: - Current branch name: `git branch --show-current` -- Commit range to integrate: `git log <base>..HEAD --oneline` +- Commit range to integrate: `git log <base-branch>..HEAD --oneline` - Unpushed commits on the remote: `git log @{u}..HEAD` if the branch tracks a remote, otherwise all commits are local -- Whether the current directory is in a git worktree: `git worktree list | grep $(git branch --show-current)` +- Whether the current directory is in a git worktree (see Phase 5 for the detection command) These details feed the outcome prompt and the execution phases. @@ -87,12 +107,38 @@ If the branch already has a remote and the user chose 1 (merge locally), note: " ### Option 1 — Merge Locally +**Pre-flight checks — run before touching any branch.** + +1. **Worktree must be clean.** A checkout or merge on a dirty tree can fail mid-way or clobber uncommitted work: + + ```bash + git status --porcelain + ``` + + If this prints anything, stop. Surface the dirty files and ask whether to commit, stash (`git stash push -u`), or abort. Don't auto-stash — the user decides what happens to their uncommitted work. + +2. **Protected-branch awareness.** If `<base-branch>` is a protected branch (often `main`, `master`, or `develop`, but confirm against the project's convention — a local push to a protected branch may be rejected or violate team policy), say so and confirm the user actually wants a local merge rather than a PR. Direct merges into a protected base are usually a mistake. + +Now integrate. Check out the base and update it only if it tracks a remote: + ```bash git checkout <base-branch> -git pull # ensure base is current -git merge --no-ff <branch-name> # --no-ff preserves the branch point in history + +# Update the base from its upstream only if one is configured. +if git rev-parse --abbrev-ref --symbolic-full-name @{u} >/dev/null 2>&1; then + git pull --ff-only # fast-forward only; a non-ff base means it diverged — surface, don't merge blindly +else + echo "No upstream for <base-branch>; skipping pull, merging local state as-is." +fi ``` +**Integration shape is a team-policy choice, not a hardcoded flag.** Ask which the project uses before integrating: + +- **Merge commit** — `git merge --no-ff <branch-name>` preserves the branch point in history (the default where the team keeps merge commits). +- **Rebase** — `git rebase <base-branch> <branch-name>` then fast-forward the base, for a linear history. After rebasing, `git checkout <base-branch> && git merge --ff-only <branch-name>`. + +Pick per the project's convention; if it's unclear, ask. Don't assume `--no-ff`. + **Re-verify the merged result.** Run tests / lint / type check on the merged state — the merge may have integrated cleanly at the text level while breaking semantically. If verification passes: @@ -185,10 +231,22 @@ Clean up the worktree (Phase 5). | 3. Keep as-is | **No** (user explicitly wants it) | | 4. Discard | **Yes** | +**Detecting whether you're in a worktree.** Don't grep `git worktree list` for the branch name — branch names can collide, substring-match, or be detached. Compare the per-worktree git dir against the shared common dir instead; they differ only inside a linked worktree: + +```bash +test "$(git rev-parse --git-dir)" != "$(git rev-parse --git-common-dir)" \ + && echo "in a linked worktree" || echo "in the main working tree" +``` + +To find the worktree's path for the removal command, read the machine-readable listing rather than parsing columns: + +```bash +git worktree list --porcelain # `worktree <path>` / `branch refs/heads/<name>` per block +``` + **If cleanup applies:** ```bash -git worktree list # confirm you're in a worktree git worktree remove <worktree-path> # non-destructive if clean ``` |
