Skip to content

Drop post-commit when the merge removed the worktree it runs in - #4049

Merged
max-sixty merged 5 commits into
mainfrom
fix-post-commit-worktree-anchor
Sep 9, 2026
Merged

max-sixty merged 5 commits into
mainfrom
fix-post-commit-worktree-anchor

Conversation

@max-sixty

@max-sixty max-sixty commented Sep 9, 2026 •

Copy link
Copy Markdown
Owner

Problem

wt merge prints ◎ Running post-commit: … and then spawns that pipeline into a path the removal has already emptied. post-commit is the only hook in the merge's background batch anchored on the feature worktree, and HookAnnouncer flushes after finish_after_merge has removed that worktree.

run_pipeline calls Repository::at on what's left, and git discovery walks up from it:

  • Worktree outside the repository (the default ../{{ repo }}.{{ branch }}): nothing above it is a repository, so the runner logs failed to open repository for pipeline to .git/wt/logs/<branch>/<source>/post-commit/runner.log, which nothing reads back. The hook doesn't run.
  • Worktree nested inside the repository ({{ repo_path }}/.worktrees/{{ branch | sanitize }}, one of the config page's own examples): discovery resolves to the primary worktree, so the hook's commands — arbitrary project code from .config/wt.toml — run against a checkout the user never chose.

#4026 documented both outcomes rather than fixing them.

Solution

The removal is the only thing that knows the anchor is gone, so it says so: spawn_hooks_after_remove calls HookAnnouncer::mark_worktree_removed(ctx.worktree_path), and the flush drops any pending pipeline anchored on a marked path. The survivors get the usual Running … line; each dropped pipeline gets its own warning.

▲ Skipped post-commit: mark (user) — worktree removed @ ~/code/myproject/.worktrees/feature
↳ To run commands in a worktree before it is removed, use pre-remove
◎ Running post-remove: cleanup (user); post-switch: notify (user); post-merge: sync (user) @ ~/code/myproject

Reading the fact from the removal rather than probing the filesystem is what makes the skip unconditional. The two removal paths leave the anchor in different states: the fast path renames the worktree into .git/wt/trash/ before the flush, but where that rename fails — cross-filesystem, permissions, Windows file locks — BackgroundFallbackMode::Detached spawns git worktree remove and the anchor is still on disk, intact, when the flush runs. A "does this path still hold git data" probe answers true there and spawns the hook into a worktree being deleted underneath it. The mark doesn't depend on which path ran.

Deciding at the flush keeps every case where the removal doesn't happen: --no-remove, merging on the target branch, merging from the primary worktree, and a removal blocked by a dirty worktree all leave the anchor in place, and post-commit runs there as before — as it does on wt step commit and wt step squash. Only wt merge can reach the drop: every other background hook anchors on a worktree its command keeps (post-merge, post-switch and post-remove all render against the destination).

There is no earlier moment to spawn it. Between the commit and the removal the worktree is rebased and runs pre-merge, so a background pipeline started there would race both.

Testing

test_merge_post_commit_runs_only_when_its_worktree_survives in tests/integration_tests/user_hooks.rs is parameterized over removal. The worktree is nested inside the repository so that a regression executes rather than merely fails: the hook writes git rev-parse --show-toplevel to a marker in the primary worktree.

  • removed — asserts the marker never appears, and reports the resolved toplevel if it does. Dropping the partition writes the primary worktree's path there.
  • kept (--no-remove) — the control: same hook, same marker path, worktree survives, and the marker names the feature worktree.

The mark is unconditional in spawn_hooks_after_remove, which every removal path reaches after its removal, so there is no fast-path/fallback branch left for a test to distinguish — the first revision of this PR put a holds_git_data probe there instead, and test (windows) caught the fallback taking the other answer.

The two merge announce snapshots now show the Skipped lines and a Running line without post-commit; both tests are renamed to the three hook types they still combine. cargo run -- hook pre-merge --yes passes.

Manual repro

A scratch repo whose post-commit hook is git rev-parse --show-toplevel > {{ repo_path }}/toplevel.txt, a feature worktree with one commit and one uncommitted file, then wt merge main --yes.

Without the drop, the nested layout runs the hook and the marker names the primary worktree, while the hook was anchored on .worktrees/feat:

/private/var/folders/.../tmp.6MtN6Ufppv/repo

With it:

Layout Flags Result
nested ({{ repo_path }}/.worktrees/{{ branch | sanitize }}) default (squash) ▲ Skipped post-commit, no marker
nested --no-squash ▲ Skipped post-commit, no marker
external (../{{ repo }}.{{ branch }}) default (squash) ▲ Skipped post-commit, no marker
external --no-remove ◎ Running post-commit, marker names the feature worktree

This was written by Claude Code on behalf of max-sixty

🤖 Generated with Claude Code

https://claude.ai/code/session_01QG3SjDDtiVkvmk3eZDc6vu

max-sixty and others added 2 commits September 8, 2026 17:01
…uns in

`wt merge` printed `◎ Running post-commit: …` and then spawned that pipeline
into a path the removal had already emptied. Where the worktree lived outside
the repository, the runner logged `failed to open repository for pipeline` to a
file nothing reads back. Where it was nested inside the repository, git
discovery walked up to the **primary** worktree instead, and the hook's
commands — arbitrary project code — ran there, with `git` inside them resolving
against a checkout the user never chose. #4026 documented both outcomes rather
than fixing them.

A background pipeline's anchor is its cwd, and the runner opens the repository
by discovery from it, so an anchor that no longer holds git data cannot resolve
to the worktree the pipeline was registered for. `run_hooks_background` now
partitions the pending pipelines on `holds_git_data` before it announces
anything: the survivors get the usual `Running …` line, and each dropped
pipeline gets a `Skipped <type>: …` warning naming the worktree, plus one hint
pointing at `pre-remove`.

Checking at the flush means the answer follows what actually happened.
`--no-remove`, merging on the target branch, merging from the primary worktree,
and a removal blocked by a dirty worktree all leave the anchor in place, and
post-commit runs there as before. There is no earlier moment to spawn it —
between the commit and the removal the worktree is rebased and runs
`pre-merge`, so a background pipeline started there would race both.

`holds_git_data` already existed for `path_selector_directory`; it is the same
predicate ("would git's discovery walk stop here"), so it moves from private to
`worktrunk::git`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QG3SjDDtiVkvmk3eZDc6vu
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QG3SjDDtiVkvmk3eZDc6vu

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two claims the diff's prose makes that the code doesn't hold up.

The holds_git_data docstring now covers both callers with one cost model, but they read the answer in opposite directions: path_selector_directory withholds on true, run_hooks_background spawns on true. "A false positive costs only caution" is inverted for the new caller — a false positive there is exactly the wrong-worktree spawn the partition exists to prevent, which matters for whoever reads this before adding a third caller or loosening the check. Suggestion inline.

The skip is documented as unconditional (wt merge --help and hook.md: "reports post-commit as skipped instead of running it"), but it holds only where the removal's fast path runs. When stage_worktree_removal returns None — its own comment names cross-filesystem, permissions, and Windows file locking — the merge's BackgroundFallbackMode::Detached spawns a detached git worktree remove and leaves the anchor holding git data at the flush, so holds_git_data answers true and post-commit is announced and spawned into a worktree being deleted underneath it. That outcome isn't worse than today's, but it's a fifth exception neither the enumeration in handle_merge's comment nor the help text carries.

Comment thread src/git/repository/worktrees.rs Outdated
Comment thread src/commands/merge.rs
max-sixty and others added 2 commits September 8, 2026 17:25
…e filesystem

The first cut of this fix partitioned the announcer's pending pipelines on
`holds_git_data(anchor)` — a filesystem probe standing in for a fact the
removal already had. The two removal paths leave the anchor in different
states, so the probe answered differently on each: the fast path renames the
worktree into `.git/wt/trash/` before the flush, but where that rename fails
(cross-filesystem, permissions, Windows file locks) the fallback
`git worktree remove` runs detached and the anchor is still on disk, intact,
when the flush runs. `test (windows)` took that path and post-commit was
announced and spawned exactly as before.

`spawn_hooks_after_remove` — which every removal path reaches after its removal
— now calls `HookAnnouncer::mark_worktree_removed(ctx.worktree_path)`, and the
flush drops pipelines anchored on a marked path. There is no branch left for a
removal path to take differently, so the skip the help text promises holds
unconditionally.

`holds_git_data` goes back to private, with its original docstring: it has one
caller again, and the cost model that docstring states was written for that
caller reading `true` as "withhold".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QG3SjDDtiVkvmk3eZDc6vu
The docstring said "Only runs if `ctx.verify` is true (hooks approved)".
`WorktreeRemovalContext` has no `verify` field — selection is the frozen
`ApprovedHookPlan`, and an empty plan registers nothing. The line also now
sat directly above an unconditional statement, since the removed-worktree
mark records what the removal did rather than what the hooks may do.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QG3SjDDtiVkvmk3eZDc6vu
@max-sixty
max-sixty merged commit 28ebf24 into main Sep 9, 2026
37 checks passed
@max-sixty
max-sixty deleted the fix-post-commit-worktree-anchor branch September 9, 2026 01:22
social4hyq pushed a commit to social4hyq/homebrew-core that referenced this pull request Sep 20, 2026
worktrunk 0.78.0

Created-by: HarmonybrewBot
Commit-by: HarmonybrewBot
Merged-by: HarmonybrewBot
Description: Created by `brew bump`

---

Created with `brew bump-formula-pr`.<details>
  <summary>release notes</summary>
  <pre>## Release Notes

### Improved

- **Hook scripts read renamed context keys**: the JSON piped to every hook drops `worktree`, `repo_root`, `main_worktree`, and `main_worktree_path` in favour of `worktree_path`, `repo_path`, `repo`, and `primary_worktree_path`. Config templates migrate on load, so hooks written with the old names keep working; scripts reading the JSON, `--execute` templates, and `--var` overrides need updating. (Breaking.) ([#4080](max-sixty/worktrunk#4080))

- **`wt config plugins pi` installs for Pi; oh-my-pi moves to `wt config plugins omp`**: `pi install` wrote an oh-my-pi hook, which Pi never loads; Pi users re-run it to get a Pi extension. Existing oh-my-pi hooks keep working, listed as outdated until `wt config plugins omp install`. (Breaking: scripts that run `wt config plugins pi` for oh-my-pi need `omp`.) [Docs](https://worktrunk.dev/claude-code/#oh-my-pi) ([#4135](max-sixty/worktrunk#4135), thanks @ashebanow)

- **Interactive prompts no longer open with a blank line**: `wt config shell install`, `wt config plugins claude install`, and the commit-message setup offer in `wt merge` began with one. ([#4059](max-sixty/worktrunk#4059))

- **`wt config show` gives a single `wt config shell install` hint**: an outdated wrapper, a fish wrapper at the deprecated `conf.d` path, and missing fish completions each printed their own hint. The zsh `compinit` snippet is now syntax-highlighted. ([#4059](max-sixty/worktrunk#4059))

- **`wt config update --output <path>` confirms the write**: it now prints `✓ Wrote user config migration @ ~/migrated.toml` where it printed nothing. `--output=-` stays silent. ([#4053](max-sixty/worktrunk#4053))

### Fixed

- **Removal no longer discards untracked files hidden by `status.showUntrackedFiles = no`**: under that setting, a worktree holding only untracked files looked clean, so `wt remove`, `wt merge`, and `wt step prune` removed it without `--force`. Safety checks now always count untracked files, and auto-staging warnings list every file they stage. ([#4111](max-sixty/worktrunk#4111), thanks @Duang777)

- **`wt remove` and `wt merge` check the right worktree under an inherited `GIT_DIR`**: run from a `!wt` git alias or a git hook, which pass `GIT_DIR` down, `wt remove` could delete a dirty worktree without `--force`, and `wt merge` could refuse a clean merge. Both now check the worktree they act on. ([#4082](max-sixty/worktrunk#4082), thanks @Duang777)

- **`wt merge` no longer deletes a worktree locked with `git worktree lock`**: the merge completes and prints `Worktree preserved (locked)`. Every Worktrunk removal refuses a locked worktree, even with `--force`; run `git worktree unlock <path>` first. ([#4073](max-sixty/worktrunk#4073), thanks @Duang777)

- **`wt config update` no longer overwrites a config edited while its prompt is open**: it wrote the pre-prompt snapshot over the newer file and reported success. It now fails with the path and leaves the newer file; re-running previews the current contents. ([#4127](max-sixty/worktrunk#4127), thanks @zach-hammad-vs for reporting)

- **`wt step prune` no longer deletes a branch created minutes ago from an older commit**: `--min-age` aged a branch without a worktree by its commit's date, so a new branch off a day-old commit was pruned on the next run. Age now comes from the branch's oldest reflog entry. ([#4077](max-sixty/worktrunk#4077))

- **A comment or blank line above an inline config section no longer breaks the config**: a save or migration that rewrote `commit = { … }` as `[commit]` put that line inside the brackets. `wt switch`, `wt merge`, `wt remove`, and several `wt step` commands then failed with `Failed to load config`; `wt list` warned and ran without the config. Saves also dropped unrecognized keys from inline sections. ([#4120](max-sixty/worktrunk#4120), thanks @zach-hammad-vs for reporting)

- **Config saves keep the trailing comment on a value they rewrite**: a save such as answering the commit-generation offer dropped the comment after each value it rewrote, including one after an inline section's closing brace. ([#4138](max-sixty/worktrunk#4138))

- **Ignored project-config keys warn from every worktree**: an unknown key in `.config/wt.toml`, or one that belongs in user config such as `[merge]`, warned only in the main worktree, so `wt merge` from a linked worktree ignored it silently. ([#4145](max-sixty/worktrunk#4145), thanks @ee-prog for reporting)

- **`wt merge` skips `post-commit` when it removes that hook's worktree**: before, the hook ran in the primary worktree when the removed one sat inside the repository, and otherwise didn't run. The merge now prints `▲ Skipped post-commit`; move work that must finish there to `pre-remove`. ([#4049](max-sixty/worktrunk#4049))

- **`wt switch` recovers from a removed worktree in a bare repository**: `wt switch` and the picker surfaced git's raw `fatal: Unable to read current working directory` instead, and the removed-directory message lacked `wt switch ^`. ([#4067](max-sixty/worktrunk#4067))

- **`wt config shell install fish` puts the wrapper where fish reads it under a custom `$XDG_CONFIG_HOME`**: the wrapper always went to `~/.config/fish/functions/`, so install reported success while `wt switch` changed no directory. On Windows, the fish completion moves from `%APPDATA%\fish\completions` to the wrapper's directory. ([#4107](max-sixty/worktrunk#4107))

- **`wt config shell install zsh` honours `$ZDOTDIR` only when it is absolute**: an empty or relative value resolved against the current directory, so install appended to a `.zshrc` under the current directory and reported success while the file zsh reads went untouched. A non-absolute value now falls back to `$HOME`. ([#4085](max-sixty/worktrunk#4085))

- **`wt config shell install` for Nushell no longer deletes a `wt.nu` under the current directory**: an empty or relative `$XDG_CONFIG_HOME` resolved against it, so install removed a `nushell/vendor/autoload/wt.nu` beneath it unread, as a stale wrapper. It now falls back to `~/.config`, which also fixes unread `tea` logins that let `wt switch pr:<n>` treat a Gitea host as GitHub. ([#4104](max-sixty/worktrunk#4104))

- **`wt switch --create` names the branch a failed worktree add leaves behind**: `git worktree add -b` writes the branch ref before populating the worktree, so a failure in between left the branch with no worktree, and the next attempt reported a name collision. The error now says how to delete or reuse it. ([#4109](max-sixty/worktrunk#4109), thanks @technicalpickles for reporting)

- **`wt config shell install` no longer tells an already-wrapped shell to restart**: reinstalling from a shell with the wrapper loaded, such as after a version bump, printed `↳ Restart shell to activate shell integration`. ([#4059](max-sixty/worktrunk#4059))

- **`wt config shell install` migrates a deprecated `conf.d` fish wrapper when `~/.config/fish/functions` is missing**: before, only `wt config shell install fish` migrated it. `wt config show` now reports that wrapper even when fish isn't on `PATH`. ([#4059](max-sixty/worktrunk#4059))

- **Plugin status and uninstall checks ask the agent CLIs**: `wt config show` could list an installed Claude Code plugin or Gemini CLI extension as not installed, and `wt config plugins claude|codex uninstall` could report success over a marketplace still configured. A repeat Claude uninstall now runs both removals instead of stopping at `Plugin not installed`. ([#4048](max-sixty/worktrunk#4048), [#4054](max-sixty/worktrunk#4054))

- **`wt config plugins opencode install` no longer writes the plugin into the current directory**: with `OPENCODE_CONFIG_DIR` set but empty, it wrote `plugins/worktrunk.ts` there, typically inside a repository, reporting success though OpenCode never saw it. An empty value now counts as unset. ([#4084](max-sixty/worktrunk#4084))

- **The OpenCode plugin clears its activity marker when OpenCode 1 disposes it**: only explicit session deletion cleared it, so a finished session kept its `💬` and `wt list` showed it as active until `wt config state marker clear` was run by hand. ([#4129](max-sixty/worktrunk#4129), thanks @drewdas for reporting)

- **`wt config update` migrates `[ci] platform` into a `[forge]` section that only sets `hostname`**: that config, common with GitHub Enterprise and self-hosted GitLab, kept the deprecated key with no deprecation warning. ([#4061](max-sixty/worktrunk#4061))

- **Deprecated keys in an inline `[projects]` entry now migrate**: `"example.com/org/repo" = { merge = { no-ff = true } }` skipped the migration the section form got, so the per-project setting was ignored. ([#4122](max-sixty/worktrunk#4122), thanks @zach-hammad-vs for reporting)

- **A deprecated config section the migration can't rewrite now warns**: `select = "not a table"`, or a `[select]` whose `[switch.picker]` destination is already set, was ignored without a message. It now reports `unknown field select (will be ignored)`. ([#4121](max-sixty/worktrunk#4121), thanks @zach-hammad-vs for reporting)

- **`wt config update` no longer changes what a template renders**: it could rename `{{ repo_root }}` inside a quoted s

See merge request: Harmonybrew/homebrew-core!20435
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.

2 participants