Skip to content

Build the commit prompt from the worktree being committed - #3996

Open
max-sixty wants to merge 4 commits into
mainfrom
land/step-commit-branch-diff
Open

Build the commit prompt from the worktree being committed#3996
max-sixty wants to merge 4 commits into
mainfrom
land/step-commit-branch-diff

Conversation

@max-sixty

@max-sixty max-sixty commented Sep 2, 2026

Copy link
Copy Markdown
Owner

wt step commit --branch <b> commits in another worktree, but built the LLM prompt from the invoking one. build_commit_prompt called Repository::current() and diffed from its discovery path, so with nothing staged in the cwd the generator got an empty diff, an empty diffstat, the invoking branch's name, and the invoking branch's recent commits — and produced a commit message describing none of the changes it was committing. wt step relocate --commit, which commits into each relocated worktree in turn, had the same bug for the same reason.

build_commit_prompt and generate_commit_message now take the target WorkingTree, and every input — diff, diffstat, branch, repo root, recent commits — is read from it. --show-prompt and --dry-run resolve --branch the same way the real run does, through a shared resolve_env, so a preview describes the commit the same flags would make. --dry-run's temp index is created in the target worktree too.

Two accessors move from Repository to WorkingTree, since both answer per-worktree questions:

  • recent_commit_subjects walks back from HEAD, and HEAD is per-worktree. On the repository it answered for whichever worktree the repo was discovered from.
  • diff_stats_summary — every caller diffs the index or HEAD.

Existing call sites that meant "the invoking worktree" say so explicitly (repo.current_worktree().diff_stats_summary(…)).

New coverage: --branch against a worktree with staged changes in tests/integration_tests/merge.rs, and the relocate path in tests/integration_tests/step_relocate.rs.

The wt step commit help said --branch "has no effect on --dry-run, which always previews the current worktree" — accurate before this change, and not after. It now says --branch selects the previewed worktree the same way it selects the committed one.

UX survey items: #103.

Reviewable files: src/llm.rs, src/commands/step/commit.rs, src/git/repository/diff.rs, src/commands/commit.rs, src/cli/step.rs, tests/integration_tests/merge.rs, tests/integration_tests/step_relocate.rs. Generated mirrors and snapshots are regenerated.

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

🤖 Generated with Claude Code

https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb

max-sixty and others added 2 commits September 1, 2026 18:59
`wt step commit --branch <b>` re-roots the command at `<b>`'s worktree —
the staging, the commit, the hooks, and the project config all follow it
— but the prompt did not. `build_commit_prompt` called
`Repository::current()` and read the staged diff, branch, and recent
commits from the *invoking* worktree, which is clean in exactly the case
`--branch` exists for. The generator received an empty `<diff>` and
`<diffstat>`, and the model's "I don't see any staged changes" reply
became the commit message over a full set of files. `wt step relocate
--commit` has the same shape: it commits each mismatched worktree while
running from another one.

`generate_commit_message` and `build_commit_prompt` now take the
`WorkingTree` being committed and read every prompt input from it.
`CommitGenerator::commit_staged_changes` already had that worktree in
hand and passes it through, so both callers are fixed at once, and
`preview_commit` resolves the same environment as the real run — so
`--show-prompt` / `--dry-run` honor `--branch` instead of silently
previewing the cwd.

`recent_commit_subjects` and `diff_stats_summary` move from `Repository`
to `WorkingTree`: both read HEAD or the index, which are per-worktree, so
asking the repository answers for whichever worktree it was discovered
from. That is the same wrong-worktree hazard the bug came from, and it
was already latent in the relocate path's progress line. The remaining
call sites (`step squash`, `step push`) run against the current worktree
and are unchanged in behavior.

`wt merge` and `wt step squash` have no such gap: both resolve through
`CommandEnv::for_action`, so they act where the user is standing.

Regression coverage: the two new tests in `merge.rs` and the assertion
added to `test_relocate_dirty_with_commit` all fail on the previous code
with an empty diff in the captured prompt.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BGS51k25qzfeJEJ8dfxEEY
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
worktrunk-bot
worktrunk-bot previously approved these changes Sep 2, 2026
@worktrunk-bot
worktrunk-bot dismissed their stale review September 2, 2026 23:54

codecov/patch is red (96.97% vs the 98.22% auto target) — CLAUDE.md requires explicit approval to merge over it. No concerns about the change itself; analysis of the two missed lines follows.

@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.

The change itself reads correct to me — I had no findings. Every call site that meant "the invoking worktree" says so explicitly, wt merge and wt step squash have no --branch so build_squash_prompt's Repository::current() is still right, src/summary.rs already threads an explicit worktree_path, and TempIndex was already rooted at worktree_root with the per-worktree gitdir index, so --dry-run --branch picks up the right index.

codecov/patch is red, though, and per CLAUDE.md → Coverage merging over it needs your explicit say-so — so I've dismissed my approval rather than leave an APPROVED state standing over a red gate. The code verdict is unchanged; this is the coverage gate, not a concern about the change. Both missed lines look like the moved/reformatted-line false positive, not a behavior gap:

The arithmetic: 66 patch lines, 64 hits, 2 misses = 96.97%, against an auto target of the base commit's project coverage, 98.22%. Two lines flip it.

Line Content Why it can't be covered
src/git/repository/diff.rs:194 None The empty-output branch of recent_commit_subjects, moved verbatim from Repository. It was already a miss on the base commit at src/git/repository/diff.rs:366. The move pulls a pre-existing miss into the patch without changing behavior.
src/commands/step/commit.rs:120 )?; The ? error-propagation region on the build_commit_prompt call. Adding &wt pushed the call past one line, so rustfmt broke it up and the ? landed on its own line — exposing a region that was never covered, only previously sharing a line with the covered call expression.

The second one has a control: the sibling generate_commit_message(...)?; five lines below was already multi-line on the base commit, and its )?; at src/commands/step/commit.rs:109 is already a miss on main. Same construct, same verdict — this PR just added a second instance of it by reformatting.

Covering either would mean making git diff --staged fail inside the target worktree, which has no deterministic trigger from an integration test. My read is that this is a "not real" patch failure in the sense the coverage section describes, and it's your call whether to merge over it.

How I checked

Codecov compare API (api.codecov.io/api/v2/github/max-sixty/repos/worktrunk), base fd10235f7598df15f6a599f45626b9c4ac19d9ab → head 2acc453dc32479bb61f65d872e05eabd02835d24:

src/commands/step/commit.rs   23 lines, 22 hits, 1 miss   (95.65%)
src/git/repository/diff.rs    28 lines, 27 hits, 1 miss   (96.42%)
src/commands/step/squash.rs    2 lines,  2 hits           (100%)
src/commands/commit.rs         2 lines,  2 hits           (100%)
src/llm.rs                    10 lines, 10 hits           (100%)
src/commands/worktree/push.rs  1 line,   1 hit            (100%)

Base-commit line coverage via file_report/<path>?sha=<base> (0 = hit, 1 = miss): src/git/repository/diff.rs line 366 → 1; src/commands/step/commit.rs line 100 (the old single-line build_commit_prompt(...)?;) → 0, line 109 (the multi-line generate_commit_message(...)?; closer) → 1.

Everything else is green: test (linux|macos|windows), lint, feature-check, code-coverage, codecov/project, changes, plan, fast-checks, pre-commit.ci, and the linux/windows advisory affected-test legs. affected tests (macos, advisory) was still running when I finished.

max-sixty and others added 2 commits September 2, 2026 19:45
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
#3997 landed "It has no effect on `--dry-run`, which always previews the
current worktree" — true of main at the time, and false once this branch makes
`preview_commit` resolve `--branch` through the same `resolve_env` the real
commit path uses.

Verified against a build of this branch: from the source worktree,
`wt step commit --branch feature --dry-run` renders the feature worktree's
diff and reports `Branch: feature`, leaving both worktrees untouched.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
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