aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-05-22 14:54:44 -0500
committerCraig Jennings <c@cjennings.net>2026-05-22 14:54:44 -0500
commitbf1c57d7b9f750af68cb74a2f6fc1477ad7f0e44 (patch)
tree4ff8d0fd222c2c38e94214bdac99cee9418476e7
parent6efa7f13b7e901e8909e5edc652d0bc9b2a2d3df (diff)
downloadrulesets-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.md78
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
```