Skip to content

docs(push): record that destination-worktree safety matches git - #3614

Merged
max-sixty merged 2 commits into
mainfrom
step-rebase-push-docs
Jul 26, 2026
Merged

docs(push): record that destination-worktree safety matches git#3614
max-sixty merged 2 commits into
mainfrom
step-rebase-push-docs

Conversation

@max-sixty

@max-sixty max-sixty commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Comment-only. It records a decision rather than changing one: wt merge and wt step push overwrite an ignored file in the destination worktree whose path the incoming commits track, and that is intended.

prepare_target_worktree works from git status --porcelain, which omits ignored files. So the existing protection covers modified-tracked and untracked collisions (refused as ConflictingChanges) and autostashes the rest of a dirty tree, while an ignored file at a colliding path is neither refused nor stashed.

git draws the line in the same place. A plain git merge run in that worktree, against a scratch repo:

ignored locally   → Fast-forward, db.sqlite | 1 +      (content: TRACKED-FROM-BRANCH)
untracked locally → error: The following untracked working tree files
                    would be overwritten by merge  (content: REAL-LOCAL-DATABASE)

So a probe that refused here would make wt stricter than the tool it wraps, over data git treats as expendable. Three places say so now, each where someone would otherwise reach the opposite conclusion:

  • src/commands/worktree/push.rs — the module spec, at the point where such a probe would be added. It names the mechanisms rather than arguing by analogy: the fast-forward hands the checkout to receive.denyCurrentBranch=updateInstead, --no-ff syncs with read-tree -m -u, and both run git's unpack-trees checks. Confirmed against a scratch repo — read-tree -m -u silently overwrites an ignored file at a colliding path and refuses an untracked one (exit 128, file intact), the same split git merge produces.
  • src/commands/repository_ext.rsprepare_target_worktree, which owns the status read, pointing at that spec from the implementation site. (A [crate::…] intra-doc link was the first attempt; push is a private module, so RUSTDOCFLAGS=-D warnings rejected it. Plain path instead.)
  • CLAUDE.md — Data Safety, bounding "prefer failure over silent loss" at git's edge. Read unqualified, that rule is what invites the stricter check.

Follows #3601, which fixed the user-facing half: the FAQ had recommended git worktree lock for "precious ignored data", which the lock does not protect — it blocks removal only.

Verification

RUSTDOCFLAGS="-D warnings" cargo doc, cargo clippy --all-targets -- -D warnings, and pre-commit all clean.

The local gate flagged test_readme_example_hooks_pre_merge, which is the known load flake — an extra Waiting for the commit generation command (4s) spinner line under full-suite CPU contention. It passes in isolation with no retries. The diff contains no executable lines at all (every changed line is ///, //!, or markdown), so it cannot be causal.

This was written by Claude Code on behalf of Maximilian Roos

🤖 Generated with Claude Code

`prepare_target_worktree` works from `git status --porcelain`, which omits
ignored files, so a push overwrites an ignored file in the destination
worktree whose path the incoming commits track. That has always been the
behavior; what was missing is that it is intended.

git draws the line in the same place — a `git merge` run in that worktree
refuses an untracked collision and overwrites an ignored one — so a probe
that refused here would make `wt` stricter than the tool it wraps, over
data git treats as expendable.

The push module spec now says so at the point where such a probe would be
added, `prepare_target_worktree` points there from the implementation
site, and Data Safety in CLAUDE.md bounds "prefer failure over silent
loss" at git's edge, since that rule read unqualified is what invites the
change.

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

Verified the documented behavior against the code, and it holds — including the one place the spec's framing and the implementation diverge in wording but not in effect.

prepare_target_worktree reads git status --porcelain -z (no --ignored), so ignored files are absent from wt_files — never matched into the ConflictingChanges refusal, and the autostash uses --include-untracked (not --all), which also skips them. That confirms the "neither refused nor stashed" half.

For the overwrite half: the destination worktree isn't synced with a literal git mergehandle_no_ff_merge runs read-tree -m -u <old> <new>. The two agree on exactly the distinction the spec rests on, though, since both go through git's unpack-trees checks: read-tree -m -u refuses to clobber an untracked non-ignored file ("would be overwritten by merge") and silently overwrites an ignored one, same as git merge. So the "matches git" justification transfers to the actual code path, not just to a hypothetical git merge run in that worktree.

Docs are accurate and internally consistent. I'm not issuing an approval: this touches the deletion/overwrite surface (push.rs's read-tree -m -u sync), which per the repo's data-safety review policy is a maintainer's call rather than the bot's — moot here since you authored it, but recording the disposition.

The spec justified matching git by what a `git merge` run in the
destination worktree would do, which is an analogy — neither push path
runs `git merge`. Name the mechanisms instead: the fast-forward hands the
checkout to `receive.denyCurrentBranch=updateInstead`, `--no-ff` syncs with
`read-tree -m -u`, and both go through git's unpack-trees checks.

Verified against a scratch repo — `read-tree -m -u <old> <new>` with a
local file at a path the new tree tracks:

    ignored   → silently overwritten (content: TRACKED-FROM-BRANCH)
    untracked → "error: Untracked working tree file 'db.sqlite' would be
                 overwritten by merge", exit 128, file intact

Same split `git merge` produces, so the justification transfers to the
code path rather than resting on a hypothetical.
@max-sixty
max-sixty merged commit b8299ac into main Jul 26, 2026
37 checks passed
@max-sixty
max-sixty deleted the step-rebase-push-docs branch July 26, 2026 15:02
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