Skip to content

fix: do not create default-branch worktree for multi-branch bare clones#453

Merged
avihut merged 2 commits intomasterfrom
daft-451/fix/unwanted-default-branch-checkout-on-nonexistent-branch-param
May 2, 2026
Merged

fix: do not create default-branch worktree for multi-branch bare clones#453
avihut merged 2 commits intomasterfrom
daft-451/fix/unwanted-default-branch-checkout-on-nonexistent-branch-param

Conversation

@avihut
Copy link
Copy Markdown
Owner

@avihut avihut commented May 2, 2026

Summary

  • For bare layouts with BranchSource::Multiple, branch_plan.base is None by design, so the override at clone.rs:301-306 was a no-op and Phase 4 kept the remote-default target_branch set in detect_branches. That created an unwanted worktree for the default branch on top of the requested satellites (-b feature -b test produced master + feature instead of just feature).
  • Fix: in the bare branch of the override, target branch_plan.cd_target (the first valid requested branch) for Phase 4. When all requested branches are missing on the remote, fall back to branch_plan.not_found.first() with branch_exists = false so no worktree is created.
  • The existing filtered_satellites filter already prevents duplication once target_branch is one of the requested branches.

Test plan

  • mise run test:unit (1493 passed)
  • mise run clippy (clean)
  • mise run fmt:check (clean)
  • New clone:multi-branch-all-missing scenario covers the all-missing case.
  • clone:multi-branch-contained and clone:multi-branch-missing strengthened with files_not_exist for the default-branch path (regression for Clone with multiple -b flags creates extra default-branch worktree (bare layouts) #451).
  • clone:multi-branch-head-token, clone:multi-branch-sibling, clone:multi-branch-hooks all still pass.

Fixes #451

🤖 Generated with Claude Code

For bare layouts with BranchSource::Multiple, branch_plan.base is None by
design, so the override block in run_clone left bare_result.target_branch
pointing at the remote default. Phase 4 then created a worktree for the
default branch on top of the requested satellites — e.g. -b feature -b test
produced master + feature instead of just feature.

Add a bare-specific path that targets branch_plan.cd_target (the first valid
requested branch) for Phase 4, falling back to branch_plan.not_found.first()
with branch_exists = false when none of the requested branches exist on the
remote so no worktree is created.

Fixes #451

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@avihut avihut added this to the Public Launch milestone May 2, 2026
@avihut avihut added the fix Bug fix label May 2, 2026
@avihut avihut self-assigned this May 2, 2026
@avihut
Copy link
Copy Markdown
Owner Author

avihut commented May 2, 2026

/claude review

@claude
Copy link
Copy Markdown

claude Bot commented May 2, 2026

Claude finished @avihut's task in 1m 33s —— View job


PR Review

  • Gather context and read changed files
  • Review src/commands/clone.rs changes
  • Review new/modified YAML test scenarios
  • Check for regression test coverage per CLAUDE.md requirements
  • Verify Rust correctness and idioms
  • Final assessment

Summary

The fix correctly addresses the root cause: for BranchSource::Multiple with a bare layout, branch_plan.base is always None (by design in branch_source.rs:131-136), so the old if let Some(ref base) = branch_plan.base guard was a permanent silent no-op. bare_result.target_branch was never overridden and stayed at the remote default set by detect_branches, causing Phase 4 to create an unwanted default-branch worktree.


src/commands/clone.rs — Core Fix

Correctness: sound. The new branch correctly targets branch_plan.cd_target (first valid requested branch in original order) for bare multi-branch clones. The filtered_satellites logic at line 358–367 already filters out bare_result.target_branch from Phase 5, so once target_branch equals cd_target, the first requested branch won't be double-created.

All-missing fallback: The not_found.first() + branch_exists = false path correctly tells Phase 4 to skip worktree creation. This mirrors the pre-existing behavior for BranchSource::Single with a missing branch, so it's a known-safe path.

One observation — BranchSource::All is unaffected: is_multi_branch = matches!(branch_source, BranchSource::Multiple(_)) deliberately excludes --all-branches. For All with a bare layout, branch_plan.cd_target is None and base is None, so Phase 4 keeps the remote default. This is pre-existing behavior that's out of scope here, and correct since --all-branches will include the default branch as a satellite anyway.

Comment length at lines 298–305: The 8-line comment is longer than the CLAUDE.md default of "no comments unless the WHY is non-obvious." Given that the old code was a silent no-op and the base: None invariant for bare layouts is subtle, the comment earns its place. No objection.


Test coverage

Per CLAUDE.md: "Every bug fix must include a regression test."

Scenario What it checks
multi-branch-all-missing.yml (new) All-missing fallback: no default-branch or requested-branch dirs created
multi-branch-contained.yml Happy path: main dir absent (regression guard for #451)
multi-branch-missing.yml Mixed valid/missing: main dir absent (regression guard for #451)

Minor observation on multi-branch-all-missing.yml: The files_not_exist assertions for nope-one and nope-two will trivially pass even without the fix, since those branches don't exist. The load-bearing assertion is files_not_exist: $WORK_DIR/test-repo/main — that's what would have failed before this fix. Consider adding it explicitly (matching the other two scenarios) to make the regression intent clear. That said, the scenario description already calls this out, and the other two strengthened scenarios cover it directly.


Verdict

LGTM. The fix is minimal, correct, and well-scoped. The is_multi_branch && layout.needs_bare() guard correctly separates the two cases that were previously conflated under a single (broken) if let Some(ref base) guard. Regression coverage matches CLAUDE.md requirements.

Per PR review: the nope-* assertions trivially pass even without the fix.
Comment the YAML so future readers see that $WORK_DIR/test-repo/main is
the actual regression guard, matching the comment style used in the other
two #451 scenarios.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@avihut avihut merged commit 2add025 into master May 2, 2026
11 checks passed
@avihut avihut deleted the daft-451/fix/unwanted-default-branch-checkout-on-nonexistent-branch-param branch May 2, 2026 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clone with multiple -b flags creates extra default-branch worktree (bare layouts)

1 participant