Skip to content

Add --author override to merge/commit/squash - #1

Merged
Bennyjitsu merged 6 commits into
mainfrom
add-author-flag
Sep 19, 2026
Merged

Bennyjitsu merged 6 commits into
mainfrom
add-author-flag

Conversation

@Bennyjitsu

Copy link
Copy Markdown
Owner

Summary

  • Adds --author <Name <email>> to wt merge, wt step commit, and wt step squash, threaded through the shared commit path, the squash commit, and wt merge --no-ff's commit-tree merge commit (via GIT_AUTHOR_NAME/GIT_AUTHOR_EMAIL, since commit-tree has no --author flag).
  • Fixes a data-safety bug in the squash commit: a failed git commit after the reset --soft used to leave the branch's original commits collapsed into anonymous staged changes with no rollback. Now restores the pre-reset HEAD on failure, with the original error chained rather than dropped.
  • Fixes a bug in has_staged_changes() that collapsed real git failures (e.g. a corrupted index) into Ok(true) alongside the intended "has changes" case, which made the squash rollback's error-check path unreachable.
  • split_author_spec (for the --no-ff path) matches git commit --author's own leniency: closes at the first > after <, tolerates trailing whitespace/junk.

Reviewed across four rounds by Codex and Grok against the local branch before this PR; all findings addressed.

Known follow-up

Generated docs (docs/src/content/docs/{merge,step}.md, skill mirrors) and help snapshots (help_merge, help_step_commit, help_step_squash) haven't been regenerated yet — that check is #![cfg(not(windows))] and couldn't be run locally. This PR's CI run should surface the exact diff needed.

Test plan

  • cargo build --release clean
  • cargo clippy --all-targets clean
  • cargo test --lib / --bins / --test integration — zero regressions (pre-existing unrelated Windows-environment failures only)
  • 7 new tests (5 integration, 2 unit) covering --author on all four commit paths, the squash-rollback-on-failure case, and the author-spec parser
  • CI docs/help-snapshot check (expected to fail until regenerated — that's what this PR run is for)

🤖 Generated with Claude Code

Claude added 6 commits September 19, 2026 14:13
wt merge, wt step commit, and wt step squash always committed under the
ambient git identity with no way to override it, which blocks any workflow
that needs per-agent commit authorship enforced by a pre-commit hook (e.g.
a hook that requires AI-driven commits to pass --author explicitly).

Adds --author <spec> to all three commands, threaded through the shared
commit_staged_changes path (commit.rs) and the squash commit (squash.rs).

Also fixes a data-safety bug in wt step squash / wt merge --squash: the
squash commit ran `git reset --soft <merge-base>` before attempting the
final commit, with no rollback if that commit failed (e.g. blocked by a
pre-commit hook). A failure there left the branch's original per-author
commits collapsed into anonymous staged changes instead of restored.
Captures the pre-reset HEAD and resets back to it if the commit fails.

wt step relocate --commit passes author: None (unchanged behavior; no
author concept in that path).
- Thread --author into wt merge --no-ff's commit-tree merge commit
  (src/commands/worktree/push.rs). commit-tree has no --author flag, so
  identity is set via GIT_AUTHOR_NAME/EMAIL env vars instead; committer is
  left unset and falls through to the ambient identity, matching
  `git commit --author`. Adds Repository::run_command_with_env for this.
- Squash rollback: chain the original commit error into the restore-failure
  error instead of dropping it, and extend the same rollback to a failed
  has_staged_changes() check (a post-reset failure, same as the commit
  failure it already covered). NoNetChanges is left untouched per explicit
  decision - it's pre-existing upstream behavior, out of scope for this PR.
- Fix help text: --author overrides the commit author, not "the ambient
  identity" - the committer stays ambient. Same wording on all three flags.
- CommitOptions.author is now Option<&'a str> instead of Option<String>,
  matching MergeOptions/handle_squash's existing style.
- Add regression tests (tests/integration_tests/merge.rs): --author on step
  commit/step squash/merge/merge --no-ff, asserting author changes and
  committer doesn't; and a squash failure at the git commit stage (invalid
  --author, which fires after the reset - a worktrunk pre-commit hook runs
  before it) restores the original per-commit history.
…aged_changes bug

- push.rs: split_author_spec's doc comment had swallowed handle_no_ff_merge's
  own rustdoc (inserted with no blank line between them, so both attached to
  the wrong item). Restored handle_no_ff_merge's doc, gave split_author_spec
  its own.
- push.rs: split_author_spec was stricter than git commit --author, which
  tolerates trailing whitespace/junk after the closing `>`. On a default
  wt merge --no-ff (squash still on), squash could accept an author spec via
  git commit --author and rewrite the feature branch, then the no-ff
  commit-tree step would reject the same spec with my stricter parser -
  failing after history was already rewritten. Now uses rsplit_once('>') to
  match git's own leniency, plus empty name/email rejection. Verified against
  the reproduction: --author "Bot <bot@example.com> " (trailing space) now
  succeeds through squash and no-ff alike.
- merge.rs: MergeOptions.author doc comment was stale (said "git commit
  --author" only; it now also feeds commit-tree for --no-ff).
- relocate.rs: dropped a redundant inline comment.
- working_tree.rs: has_staged_changes() collapsed a real git failure (e.g. a
  corrupted index) into Ok(true) alongside the intended "diff exists" case,
  because it read run_command's Err (any non-zero exit) as "has changes".
  Squash's rollback wrapper around this call was therefore unreachable dead
  code. Now reads the exit code directly: 0 = false, 1 = true, anything else
  propagates as a real error. Added a unit test reproducing the failure via
  a corrupted (directory) index file.

Known open item (not fixed): the generated docs/help snapshots for the new
--author flag (docs/src/content/docs/{merge,step}.md, skills mirrors,
help_merge/help_step_commit/help_step_squash snapshots, test_docs_are_in_sync)
still need regenerating. That test suite and the help snapshots are
#![cfg(not(windows))] by this project's own design (clap renders markdown
differently on Windows) - genuinely can't run or regenerate them on this
machine. No WSL/Linux toolchain available here either.

Verified: cargo build --release clean, cargo clippy --all-targets clean,
cargo test --lib (1465/1469, 4 pre-existing unrelated failures) + --bins
(985/985) + --test integration (1982/2010, same 28 pre-existing unrelated
failures as prior rounds) - zero regressions from this round's changes.
git's own author-ident parser closes at the first '>' after '<', discarding
everything after it - including another literal '>'. rsplit_once('>') took
the LAST one instead, so "Bot <bot@example.com> trailing >" parsed as email
"bot@example.com> trailing" rather than "bot@example.com", diverging from
what git commit --author (used by squash/commit) would have accepted for
the same spec. Verified against real git and against the wt binary
end-to-end. Grok's round-4 review independently surfaced the same
first-vs-last-'>' imprecision but judged it unrealistic to hit and left it
optional; skipping its other optional item (accepting an empty email, which
git allows) as genuinely not realistic for this flag's purpose.
CI's lint job caught this - never ran cargo fmt locally on Windows (no reason
to expect it'd differ, and it didn't produce any semantic change, just
line-wrapped two multi-line macro calls).
@Bennyjitsu
Bennyjitsu merged commit 6b2ac6c into main Sep 19, 2026
18 of 23 checks passed
@Bennyjitsu
Bennyjitsu deleted the add-author-flag branch September 23, 2026 12:22
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