fix(completions): offer path completions and out-of-repo path remove

[avihut]

Summary

• daft remove and daft rename document worktree paths as a valid input form, but Tab on ./, ../, /, or ~/ produced no candidates.
• Running daft remove <abs-path> from outside any git repo bailed with Not inside a Git repository, even when the path argument unambiguously named a worktree.
• Per-shell rich-completion stubs now offer directory completion alongside (or short-circuited from) the dynamic branch source for the path-accepting commands; run_remove discovers the owning repo from a path arg when cwd is outside any repo and chdirs to its project root before delegating to the existing branch-delete pipeline.

Fixes #450

Test plan

• mise run fmt:check and mise run clippy clean
• mise run test:unit (1498 lib tests + new completion-stub tests) all green
• tests/manual/scenarios/branch-delete/daft-remove-outside-repo-by-path.yml passes (contained layout: success + bare-branch error + non-existent-path error)
• tests/manual/scenarios/branch-delete/daft-remove-outside-repo-sibling-layout.yml passes (sibling layout end-to-end)
• Existing branch-delete:basic, :by-absolute-path, :by-relative-path, :by-dot, and :daft-remove-force scenarios still pass — no regressions in inside-repo path resolution
• Spot-check Tab completion in zsh for daft remove ./, daft remove /tmp/, and daft remove <branch-prefix> from inside and outside a daft-managed repo

🤖 Generated with Claude Code

[avihut]

/claude review

[claude]

Claude finished @avihut's task in 3m 29s —— View job

---

PR Review: fix(completions): offer path completions and out-of-repo path remove

• Gather context and understand request
• Read changed files
• Review Rust correctness and idioms
• Review test coverage
• Review shell-eval hot-path constraints
• Review security and performance
• Post final review

---

Overall Assessment

Solid fix that addresses the documented behavior gap. The allows_path_completion() predicate is a clean single source of truth and the per-shell completion stubs are well-tested. A few issues worth addressing before merge:

---

src/commands/worktree_branch.rs

Medium — daft rename path completion UX inconsistency [worktree_branch.rs:342]

allows_path_completion() returns true for daft-rename, so shell Tab will now suggest directory completions for daft rename <TAB>. But run_rename() still bails unconditionally when outside a repo:

if !is_git_repository()? {
    anyhow::bail!("Not inside a Git repository");
}

A user who tabs to get daft rename /path/to/worktree from outside a repo then hits an immediate error when they run the command. Either rename should get the same out-of-repo path treatment as remove, or allows_path_completion("daft-rename") should only return true when inside a repo — but that's not possible at completion-script generation time.

The simplest fix is to keep the completion as-is (it IS valid when inside a repo), but it's worth a comment or a follow-up issue noting the limitation. Fix this →

---

Minor — to_string_lossy() for reconstructed args [worktree_branch.rs:286-290]

*args = absolute_paths
    .into_iter()
    .map(|p| p.to_string_lossy().into_owned())
    .collect();

Any non-UTF8 bytes in the path are silently replaced with U+FFFD. In practice git doesn't support non-UTF8 paths either, so this is a theoretical concern, but Path::display() would produce the same result. Acceptable as-is given the constraints.

---

Minor — common_dir().parent() for project root derivation [worktree_branch.rs:275-277]

let project_root = common_dir
    .parent()
    .ok_or_else(|| anyhow::anyhow!("could not determine project root for repository"))?;

For a standard worktree common_dir() = <main-worktree>/.git/ → parent() = <main-worktree>/. This is correct for both contained (chdir to test-repo/main/) and sibling layouts, as confirmed by the YAML scenarios. The edge case is a bare repo where the semantics differ, but daft doesn't officially support bare repos, so this is fine.

---

src/commands/completions/bash.rs

Minor — word-splitting in path_pre assignment [bash.rs generated path_pre]

COMPREPLY=( $(compgen -d -- "$cur") )

This unquoted command substitution splits on $IFS (space/tab/newline by default), so a directory named my worktree/ would produce two separate COMPREPLY entries (my and worktree/). The path_post correctly uses a while IFS=$'\n' read -r loop to avoid this. Consistency would suggest using mapfile -t COMPREPLY < <(compgen -d -- "$cur") in path_pre too. In practice, worktree directory names rarely contain spaces, but this is a latent bug. Fix this →

---

Observation — unconditional path_post appends dirs for non-path prefixes

The path_post runs after branch groups, so typing feat<TAB> will also append any directory starting with feat. This is the stated intent ("so a single Tab works in both cases") and the path_pre short-circuit prevents duplication when the prefix is clearly a path. The UX is debatable but not a bug.

---

src/commands/completions/zsh.rs

Same observation — path_post = "_files -/\n" is appended unconditionally after branch compadd calls, meaning directories starting with any prefix join the completion list alongside branches. Same design decision as bash. Users typing branch prefixes may see unexpected directory suggestions. A [[ "$curword" == */* || "$curword" == "~" ]] guard could scope this more tightly, but that's a follow-up.

---

Test Coverage

Coverage is good:

• bash_path_accepting_commands_offer_directory_completion — checks for compgen -d and path-prefix guard ✓
• zsh_path_accepting_commands_offer_directory_completion — checks for _files -/ and guard ✓
• fish_path_accepting_commands_offer_directory_completion — checks for __fish_complete_directories ✓
• fish_daft_umbrella_offers_directory_completion_for_remove_and_rename ✓
• non_path_commands_do_not_offer_directory_completion (negative check) ✓
• YAML scenario: outside-repo contained layout ✓
• YAML scenario: outside-repo sibling layout ✓
• YAML scenario: bare branch name from outside → error ✓
• YAML scenario: non-existent path → error ✓

Missing test: the "all paths must belong to the same repository" code path at worktree_branch.rs:268 is never exercised by a test. A step like:

- name: Cross-repo remove from outside is rejected
  run: daft-remove "$WORK_DIR/repo-a/worktree" "$WORK_DIR/repo-b/worktree" 2>&1
  cwd: "$WORK_DIR"
  expect:
    exit_code: 1
    output_contains:
      - "same repository"

would close that gap. Fix this →

---

Hot-Path Constraint (CLAUDE.md)

The completion scripts are evaluated at shell startup. The changes are:

• allows_path_completion() is a compile-time matches!() — zero runtime cost ✓
• The path_pre/path_post shell snippets are embedded static strings — no extra subprocess at startup ✓
• __fish_complete_directories is called at Tab-press time, not startup ✓

No hot-path violations.

---

Security

No injection vectors. Path arguments are canonicalized before passing to gix::discover() and set_current_dir(). The shell completion snippets don't pass user input to eval. ✓

---

Summary

The core prepare_out_of_repo_remove logic is correct and well-tested for the main use cases. The rename inconsistency (completion offered but command fails outside-repo) and the missing cross-repo validation test are the main items worth addressing. The word-splitting in bash path_pre is a latent quality issue. Everything else is clean.