fix(integration): find squash merges past the patch-id cap by scanning the branch's paths, oldest first

[mbark]

The bug

The patch-id squash-merge fallback counted every commit in merge-base..target and bailed when there were more than PATCH_ID_SCAN_MAX_COMMITS (500). On a fast-moving default branch that count passes 500 within days, so a squash-merged branch whose files were edited again after the merge — the only case that reaches this fallback at all — stops being detected as integrated, and wt step prune / wt remove keep it.

I hit this on a GitOps repo where a bot commits image bumps to main continuously (~1,200 commits/week). Branches squash-merged days or weeks earlier sat 600–5,600 commits behind main, every one was over the cap, and none of the 27 merged branches were pruned — though their squash commits' patch-ids matched the branches' exactly.

The fix

Two commits.

1. Only diff target commits that touch the branch's paths

A target commit can only share the branch's patch-id if it changes exactly the branch's files. So the scan now lists only the target commits that touch one of the branch's paths, and the cap applies to that list:

git --literal-pathspecs rev-list --full-history --stdin
stdin: <merge-base>..<target>, then `--`, then the branch's paths

• The paths come from diff-tree -r -z --name-only, the same plumbing and rename handling as the branch-side patch. They stay raw bytes end to end, so a non-UTF-8 filename filters correctly.
• --no-merges: diff-tree emits no patch for a merge and a squash merge has one parent, so a merge could only take a slot under the cap.
• --stdin keeps a branch touching thousands of files off the argv; a new Repository::run_command_with_stdin runs it.
• --literal-pathspecs stops *, ?, [ or a leading : in a filename from acting as pathspec magic.
• --full-history stops history simplification from dropping a commit that reached the target through the side of a merge.
• A path containing a line break can't be written one per line, so that branch falls back to the whole range.
• diff-tree --stdin -p still gets no pathspec: each candidate's patch must cover its whole change, or a commit that also touched other files would falsely match.

The filter only drops commits that could never match, so it adds no false positives. The path-limited walk compares trees but builds no patches: over the full 26,200-commit history of the repo above it takes 0.17–0.37s, against 0.05s for the old rev-list --count pre-flight.

That took the repo from 0 to 19 of 27 detected. The other 8 edit values files the bot re-touched 1,079–1,669 times since, so they still exceeded the cap — hence:

2. Diff the oldest candidates past the cap instead of giving up

List the candidates oldest first (rev-list --reverse) and diff the first PATCH_ID_SCAN_MAX_COMMITS of them instead of none. The cut is made in Rust: --max-count applies before --reverse, so it would keep the newest.

A squash merge lands on the target soon after the branch forks, and updating the branch before merging (a rebase, a merge from the target) moves the merge-base up to just before it. What piles up behind it is the target's later edits to the same files. For all 8 branches above, the squash commit was the oldest candidate.

Nothing changes up to the cap. Past it, a check now diffs PATCH_ID_SCAN_MAX_COMMITS patches where it used to diff none — the cost the cap already allowed for a branch just under it.

Result on that repo: all 27 branches whose merged PR head equals the local branch tip are now detected, with no false positives. The 4 that still show as unintegrated are genuinely unmerged (2 closed PRs, 1 with no PR, 1 whose tip differs from the merged head).

Still missed: a squash that landed after more than PATCH_ID_SCAN_MAX_COMMITS other commits to the branch's files, counted from the merge-base. That takes a branch left un-updated while the target edited its files hundreds of times, then squash-merged as is; by then the target has usually changed the patch's context lines too, so no patch-id would match anyway. wt remove's "Branch cleanup" help now says so.

Tests

patch_id_tests' fixture builds its topology with a single git fast-import stream, so 500 padding commits stay instant. Padding is now typed (Empty / OtherFile / SameFile) and can be placed before or after the squash:

• detects_squash_when_range_is_under_cap — unchanged behaviour under the cap.
• detects_squash_among_oldest_candidates_past_cap — 500 same-file pads after the squash no longer hide it.
• misses_squash_past_oldest_candidates — 500 same-file pads before the squash push it out of reach; pins the boundary against the test above.
• detects_squash_behind_commits_to_other_paths — 500 pads on another file don't count toward the cap.
• branch_paths_are_literal_not_pathspec_magic — a file named [o]the[r] isn't treated as a pattern (not *, which git on Windows rejects as a path).
• rejects_target_commit_with_extra_changes — a target commit making the branch's change plus an unrelated one doesn't match.
• non_utf8_branch_paths_still_filter (Unix) — a Latin-1 café.txt behind 500 pads on another file is still found.
• branch_path_with_line_break_scans_whole_range (Unix) — a path containing \n falls back to the whole range instead of splitting into two pathspecs.
• branch_that_changes_nothing_is_not_squash_merged — covers the early return.

Each was checked against a deliberately broken build (no path filter, no --literal-pathspecs, diff-tree narrowed to the paths, newest-first, bail past the cap, lossy path decode, no line-break guard) and fails on the mutation it pins. --no-merges has no dedicated test.

Locally: the lib suite, clippy (-D warnings), pre-commit and the docs sync check pass. My full integration run's only failures are environmental (no nushell/pwsh installed) and match unmodified main.

Disclosure

Written with Claude Code. I've read every line, run the full suite, and validated the behaviour against the real repository that prompted it; the numbers above are measured, not estimated.

https://claude.ai/code/session_01XwVSv9XpxotCADuKxRyUbA

[worktrunk-bot]

Not approving: test (windows) and codecov/patch are red on this head, and one of the findings below is a confirmed regression. Separately, a true from this function is what makes wt remove delete a branch without -D and what makes wt step prune remove it, so this diff widens that deletion surface. The repo's review guidance holds that surface for a human — @max-sixty, the merge call is yours regardless of the items below.

test (windows) fails to build the fixture. branch_paths_are_literal_not_pathspec_magic dies with fast-import failed: fatal: invalid path '*' (job log) — git on Windows rejects * in a path even through fast-import. [o]ther pins the same property: [o] is a wildmatch bracket expression, so as a pathspec it matches other and drags every pad into the candidate list, while --literal-pathspecs matches only the file literally named [o]ther. Brackets are legal on NTFS. Suggested inline; it passes here.

A non-UTF-8 filename now defeats the path filter. run_command returns String::from_utf8_lossy(&output.stdout), so diff-tree -r -z --name-only's raw bytes come back with each invalid sequence collapsed to U+FFFD. That replacement char is then written into the rev-list pathspec, where it matches nothing — the squash commit is filtered out of the candidates and the branch reports as unintegrated. The comment on that call says "NUL-separated so any filename survives"; -z does strip the quoting, but the lossy decode puts the damage back.

This is a regression rather than a pre-existing limit. Same fixture both ways — a file named café.txt (as the bytes caf\xe9.txt) changed on the branch and squash-committed onto target, four commits total, nowhere near the cap: 705c7c6 returns true, this head returns false. Adding '\u{FFFD}' to the existing line-break guard routes it through the same whole-range fallback; I checked that flips the fixture back to true with the rest of patch_id_tests still green. Suggested inline. If you'd rather keep the filter working in that case instead of falling back, run_command_output hands back the raw stdout and run_command_with_stdin already takes Vec<u8>, so the paths can stay bytes end to end — that would make the comment true as written.

Merge commits take cap slots they can never fill. diff-tree --stdin -p emits no patch for a merge, and a squash merge always has one parent, so every merge --full-history keeps among the candidates is a slot that cannot match. On a target whose PRs land as merge commits rather than squashes, that is a large share of the 500, and it comes straight off the reach the second commit is buying. --no-merges reclaims them and can't drop a real candidate. Suggested inline.

codecov/patch: 97.67% against the 98.30% auto target, 3 misses, all in src/git/repository/integration.rs. The root CLAUDE.md makes it a merge gate. Two are branches this PR deliberately adds that no test reaches — the return Ok(false) when the branch changed no paths, and the untaken side of the line-break guard. A fixture with a path containing a newline covers the second, and it's the same fixture the U+FFFD case above wants. The third is the ? on the rev-list call, which has no deterministic trigger.

On the approach itself, the path filter is sound in the direction that matters: a commit whose full patch-id equals the branch's combined patch-id must touch exactly the branch's paths, so the filter can only remove non-matches — the U+FFFD case above is the one way a real candidate escapes it. rev-list --stdin does drain its whole input before emitting, so the write-all-then-read shape in run_command_inner won't deadlock on a large path list; I checked with 128 KB of pathspecs.

[worktrunk-bot]

The three items from the previous review are closed on this head, and test (linux), test (macos), test (windows), codecov/patch and codecov/project are all green here.

Both new tests pin what they claim — mutating the fix back fails each one and only it: reading the path list through a lossy decode fails non_utf8_branch_paths_still_filter, and taking the --/pathspec branch unconditionally fails branch_path_with_line_break_scans_whole_range.

--no-merges ships without a test and doesn't need one: it is a commit-output filter, not a traversal one, so it can't drop a non-merge commit --full-history reached, and diff-tree -p emits no patch for a merge, so no merge could have matched before either.

Not approving, for the reason already on this PR rather than anything in this diff: a true from this function is what lets wt remove delete the branch without -D and what lets wt step prune remove it, so the merge call stays with @max-sixty.