Skip to content

ums: post-merge worktree-tidy pitfalls - #158

Merged
d-morrison merged 3 commits into
mainfrom
ums-post-merge-worktree-tidy
Jun 22, 2026
Merged

ums: post-merge worktree-tidy pitfalls#158
d-morrison merged 3 commits into
mainfrom
ums-post-merge-worktree-tidy

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Summary

  • Extends post-merge step 2 with two worktree-specific pitfalls hit during PR #141's post-merge:
    • git worktree remove <path> fails when called from inside the worktree being removed — documents the git -C "$REPO" worktree remove "$(pwd)" fix using git rev-parse --git-common-dir
    • git pull --ff-only fails when the main checkout has diverged from a concurrent session's local commits — documents skipping the pull rather than force-merging another session's work
  • Adds an anti-pattern: don't force-pull or reset a diverged main checkout

Test plan

  • Read step 2 of the updated skill and confirm both worktree cases are clear
  • Verify the anti-pattern list covers the diverged-main scenario

🤖 Generated with Claude Code

…verged main

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 22, 2026 19:54
@d-morrison

Copy link
Copy Markdown
Collaborator Author

Working on this — paws off until I'm done.

@github-actions
github-actions Bot removed the request for review from Copilot June 22, 2026 19:54
@claude

This comment has been minimized.

**Diverged main checkout:** `git pull --ff-only` fails when the main checkout
has local commits from a concurrent session that haven't been pushed. Don't
force-merge or reset their work — skip the pull and delete the branch only.
The branch deletion is what matters; another session will pull main when it's

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gap: after a diverged git pull --ff-only fails, GitHub typically auto-deletes the remote PR branch on merge. The failed pull still ran git fetch, so origin/<pr-branch> may now be gone. With no upstream ref to check, git branch -d <merged-branch> falls back to checking reachability from HEAD (the diverged local main), which doesn't contain the merge commit yet — so it refuses with "not fully merged."

Step 1 already confirms via gh pr view that the PR is merged, so we know it's safe. Suggest noting that if git branch -d refuses here, the verification from step 1 makes it safe to use git branch -D:

Suggested change
The branch deletion is what matters; another session will pull main when it's
has local commits from a concurrent session that hasn't been pushed. Don't
force-merge or reset their work — skip the pull and delete the branch only.
The branch deletion is what matters; another session will pull main when it's
ready. If `git branch -d` refuses because local `main` doesn't yet include
the merge (diverged HEAD, remote branch already deleted), use `git branch -D`
step 1 already confirmed the PR is merged, so the force-delete is safe here.

Comment thread skills/post-merge/SKILL.md Outdated
```

**Diverged main checkout:** `git pull --ff-only` fails when the main checkout
has local commits from a concurrent session that haven't been pushed. Don't

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor grammar: "a concurrent session that haven't" should be "hasn't" (the session hasn't pushed, or: "local commits … that haven't been pushed").

Suggested change
has local commits from a concurrent session that haven't been pushed. Don't
has local commits from a concurrent session that hasn't been pushed. Don't

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 22, 2026 20:24
@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD Summary — round 1

# Finding Disposition Notes
1 Grammar: "haven't" → "hasn't" (line 79) ✅ Addressed Fixed
2 git branch -d may refuse when remote branch is already deleted (diverged HEAD) ✅ Addressed Added git branch -D escape hatch with note that step 1's gh pr view confirmation makes it safe

@github-actions
github-actions Bot removed the request for review from Copilot June 22, 2026 20:25
@claude

This comment has been minimized.

Comment thread skills/post-merge/SKILL.md Outdated

```bash
REPO="$(git rev-parse --git-common-dir)/.." # main checkout root
git -C "$REPO" worktree remove "$(pwd)" # remove this worktree

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$(pwd) gives the shell's current directory, which may be a subdirectory of the worktree (e.g. worktree/src/). git worktree remove expects the registered worktree root path — a subdirectory path won't match and will error. git rev-parse --show-toplevel always returns the worktree root, regardless of where in the tree the shell is:

Suggested change
git -C "$REPO" worktree remove "$(pwd)" # remove this worktree
git -C "$REPO" worktree remove "$(git rev-parse --show-toplevel)" # remove this worktree

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 22, 2026 20:27
@d-morrison

Copy link
Copy Markdown
Collaborator Author

ARD Summary — round 2

# Finding Disposition Notes
1 $(pwd) should be $(git rev-parse --show-toplevel)git worktree remove requires the registered worktree root, not any subdirectory ✅ Addressed Fixed

@github-actions
github-actions Bot removed the request for review from Copilot June 22, 2026 20:28
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 2m 10s —— View job


Code Review — Round 3

  • Gather context
  • Read changed file and diff
  • Review for correctness, hallucinations, clarity, and style
  • Post findings

All round-1 and round-2 findings are incorporated correctly. Here's the full verification:

REPO="$(git rev-parse --git-common-dir)/.." (line 73): Sound. Inside a linked worktree --git-common-dir returns the absolute path to the shared .git dir (e.g. /repo/.git), so /.. resolves to the main checkout root. Matches the comment. --git-common-dir exists since git 2.5.

git rev-parse --show-toplevel (line 74): Correct fix for the round-2 finding. Returns the registered worktree root regardless of subdirectory depth; git worktree remove requires the root, not a subdirectory. $(pwd) is gone.

Grammar (line 79): "hasn't" is correct. Round-1 fix applied.

git branch -D escape hatch (lines 82–84): The safety justification — step 1 already confirmed state == MERGED — is sound. A force-delete of the local branch cannot lose commits that have already landed in a merge commit on the remote. Round-1 finding properly handled.

Anti-pattern (line 150): Clean match to the diverged-main prose in step 2. No tension with the existing git branch -D anti-pattern two lines above (this one targets the pull/reset, not the delete).

No hallucinations. No AI-tell clusters. Prose is direct throughout.

Verdict: clean. Ready to merge.

@d-morrison

Copy link
Copy Markdown
Collaborator Author

Done — PR is clean and free.

@d-morrison
d-morrison merged commit 4cd34ab into main Jun 22, 2026
4 checks passed
@d-morrison
d-morrison deleted the ums-post-merge-worktree-tidy branch June 22, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant