Skip to content

Execute read-only git queries under --dry-run - #44

Open
bkildow wants to merge 2 commits into
mainfrom
fix/dry-run-read-only-queries
Open

Execute read-only git queries under --dry-run#44
bkildow wants to merge 2 commits into
mainfrom
fix/dry-run-read-only-queries

Conversation

@bkildow

@bkildow bkildow commented Aug 9, 2026

Copy link
Copy Markdown
Owner

The bug

wt prune --dry-run reports No merged worktrees to prune. in every repository, no matter how many branches are actually merged. Found while trying to reclaim disk space across two projects that had 29 and 5 fully-merged worktrees respectively — dry-run insisted there was nothing to do in both.

--verbose shows the tell — the only git command is prefixed, meaning it never ran:

$ wt prune --dry-run --verbose
[dry-run] git --git-dir /…/.bare worktree list --porcelain
No merged worktrees to prune.

Why

cmd/prune.go:35 builds its Runner with IsDryRun(), and Run() returned "", nil without executing under dry-run. WorktreeList() goes through Run(), so it parsed empty output into a nil slice, the loop had nothing to iterate, and prune fell through to the "nothing to do" branch.

IsBranchMerged() had the mirror-image problem — it returned true unconditionally under dry-run. That one was masked: the worktree list was already empty, so it was never reached. Fix only the list and dry-run would have proposed pruning every worktree, merged or not.

Dry-run should suppress the changes, not the analysis that decides which changes to propose.

The change

Split Query() out of Run(). Query always executes and takes the read-only callers (WorktreeList, ListRemoteBranches, HasLocalBranch, GetDefaultBranch, ResolveStartPoint); Run keeps the dry-run gate for state-changing commands. IsWorktreeDirty, IsBranchMerged, GetLastCommitAge and GetBehindCount build their own exec calls, so their dry-run early-returns are just dropped.

This also fixes wt status, which reported unknown commit ages and 0 behind-counts under --dry-run.

Tests

TestDryRunMode covered these queries against a /nonexistent git dir and passed — for the wrong reason. The stubs meant the bogus path was never touched, so the test proved only that the stubs existed. Narrowed it to state-changing commands, where a fake git dir is the right tool.

Added TestDryRunExecutesQueries, which builds a real repo with one merged and one unmerged branch. Against main it fails all five assertions:

--- FAIL: TestDryRunExecutesQueries
    dry-run WorktreeList returned nothing; queries must execute under dry-run
    dry-run IsBranchMerged(unmerged, main) = true, want false
    dry-run HasLocalBranch(no-such-branch) = true, want false
    dry-run IsWorktreeDirty = false on a dirty tree
    dry-run GetLastCommitAge = "unknown", want a real relative date

go build, go vet, and the full suite pass on the branch.

Verified against real repos

Built the patched binary and re-ran the case that started this. Both counts match an independent git merge-base --is-ancestor sweep, and --dry-run still removed nothing:

$ wt prune --dry-run      # affiliated
→ Merged worktrees:
  issue/ADBUILD-1655-xss-member-news  …
  …29 total…

$ wt prune --dry-run      # newyorkcares-drupal
→ Merged worktrees:
  …5 total…
✓ Pruned 5 worktree(s)

https://claude.ai/code/session_01FyzmAG45hK4qPXFgVEbFHn

bkildow added 2 commits August 9, 2026 10:21
`wt prune --dry-run` reported "No merged worktrees to prune" in every
repository, regardless of how many branches were actually merged.

cmd/prune.go builds its Runner with IsDryRun(), and Run() returned ""
without executing under dry-run. WorktreeList() goes through Run(), so
it parsed empty output into a nil slice, the loop had nothing to iterate,
and prune fell through to the "nothing to do" branch. IsBranchMerged()
had the mirror-image problem, returning true unconditionally -- had the
list been populated, dry-run would have proposed pruning everything.

Dry-run should suppress the changes, not the analysis that decides which
changes to propose. Split Query() out of Run(): Query always executes and
carries the read-only callers (WorktreeList, ListRemoteBranches,
HasLocalBranch, GetDefaultBranch, ResolveStartPoint), while Run keeps the
dry-run gate for state-changing commands. IsWorktreeDirty, IsBranchMerged,
GetLastCommitAge and GetBehindCount build their own exec calls, so their
dry-run early-returns are simply dropped.

TestDryRunMode covered these queries against a fake git dir, which passed
for the wrong reason: the stubs meant the bogus path was never touched.
Narrow it to state-changing commands and add TestDryRunExecutesQueries,
which builds a real repository with one merged and one unmerged branch.
It fails on all five assertions before this change.

Claude-Session: https://claude.ai/code/session_01FyzmAG45hK4qPXFgVEbFHn
Making read-only queries execute under --dry-run fixed `wt prune`, but it
also made `wt remove --dry-run` reachable for the first time. Previously it
bailed at "no worktrees found" -- WorktreeList returned empty -- so nothing
past the lookup had ever run. Two steps down that path were never guarded,
because nothing could reach them.

terminateBackgroundSetup really signalled the process: SIGTERM, poll for two
seconds, then SIGKILL. Killing a live setup process is exactly the kind of
change --dry-run promises not to make. It now takes a dryRun parameter, the
same way RunTeardownHooks already does, and reports the PID it would signal
instead. The Claude hook call site passes false; that path is always real.

The chdir out of the target worktree exists so git can delete the directory,
and the trailing project-root print tells the shell wrapper to follow. Under
dry-run nothing is deleted, so neither applies -- and moving the user's shell
is itself a side effect. Both now hang off `relocating`, which folds in the
dry-run check.

TestTerminateBackgroundSetup starts a real child, points a running setup
state at its PID, and asserts it survives the dry-run call and dies on the
real one. It fails on the first assertion without the guard. Unix-only:
IsProcessAlive is hardcoded false on Windows, so the assertions would be
vacuous there.

Claude-Session: https://claude.ai/code/session_01LkAd9DEmjfs62ZhpkbZtU7
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