diff --git a/lib/account/sync/dispatcher.zsh b/lib/account/sync/dispatcher.zsh index 1ee9ef7..c032d97 100644 --- a/lib/account/sync/dispatcher.zsh +++ b/lib/account/sync/dispatcher.zsh @@ -95,15 +95,39 @@ _ckipper_account_sync_run() { fi if (( ${#_CKIPPER_SYNC_TARGETS} == 0 )); then _CKIPPER_SYNC_TARGETS=( ${(f)"$(_ckipper_account_sync_pick_targets "$_CKIPPER_SYNC_FROM")"} ) - (( ${#_CKIPPER_SYNC_TARGETS} == 0 )) && return 1 + if (( ${#_CKIPPER_SYNC_TARGETS} == 0 )); then + _ckipper_account_sync_empty_select_hint "target accounts" + return 1 + fi fi _ckipper_account_sync_validate_accounts || return 1 local -a types types=( ${(f)"$(_ckipper_account_sync_resolve_types)"} ) - (( ${#types} == 0 )) && { echo "No types selected." >&2; return 1; } + if (( ${#types} == 0 )); then + _ckipper_account_sync_empty_select_hint "sync types" + return 1 + fi _ckipper_account_sync_run_targets types } +# Print a friendly hint when a multi-select picker returns nothing. +# +# `gum choose --no-limit` exits 0 with empty stdout when the user presses +# ENTER without first toggling items with SPACE — easy to do because the +# single-select source picker right before it accepts ENTER on its own. +# Without this hint, the wizard exits silently and users believe the picker +# is broken (see PR adding this for the reproduction). +# +# Args: $1 — what was being selected ("target accounts" | "sync types"). +# Returns: 0 always. +# Errors (stderr): "No selected." plus a SPACE/ENTER hint. +_ckipper_account_sync_empty_select_hint() { + { + echo "No $1 selected." + echo "Hint: in the picker, press SPACE to mark items, then ENTER to confirm." + } >&2 +} + # Walk every target and apply the resolved type list. # # Args: $1 — name of array variable holding type ids. diff --git a/lib/account/sync/dispatcher_test.bats b/lib/account/sync/dispatcher_test.bats index 735df2e..31c8afd 100644 --- a/lib/account/sync/dispatcher_test.bats +++ b/lib/account/sync/dispatcher_test.bats @@ -180,6 +180,29 @@ run_full() { [ "$status" -eq 0 ] } +# Regression: with no positional targets and an empty multi-select reply, +# the wizard previously returned 1 silently — users perceived the picker +# as broken. Both pickers now print a SPACE/ENTER hint before exiting. +@test "interactive sync prints a SPACE/ENTER hint when no targets are picked" { + setup_two_accounts + # Pipe enough blank lines to satisfy every prompt the fallback path asks + # for (source = "1", targets = "", types = ""). The empty targets line + # is what we're exercising. + run_full 'printf "1\n\n\n" | ckipper account sync' + [ "$status" -ne 0 ] + [[ "$output" == *"No target accounts selected"* ]] + [[ "$output" == *"press SPACE to mark items"* ]] +} + +@test "interactive sync prints a SPACE/ENTER hint when no types are picked" { + setup_two_accounts + # Source = "1", targets = "dst", types = "" (empty -> hint). + run_full 'printf "1\ndst\n\n" | ckipper account sync' + [ "$status" -ne 0 ] + [[ "$output" == *"No sync types selected"* ]] + [[ "$output" == *"press SPACE to mark items"* ]] +} + # Regression: when the user picks "View changes" then "Apply", the diff # output written by drill_down_loop must NOT pollute the captured action, # else the [[ "$action" == "apply" ]] check downstream silently skips apply. diff --git a/lib/worktree/worktree.zsh b/lib/worktree/worktree.zsh index 09beac9..adc3ae3 100644 --- a/lib/worktree/worktree.zsh +++ b/lib/worktree/worktree.zsh @@ -3,6 +3,18 @@ readonly _CKIPPER_WT_FIND_MAX_DEPTH=3 +# Directory names pruned during the worktree-list scan. Branches can contain +# slashes (e.g. `feature/foo`), so we cannot bound the search by depth — but +# we CAN skip the heavy build/cache trees that inflate scan time without +# containing real worktrees. Empirically takes a 6 GB / 380k-file worktrees +# tree from ~700 ms to ~30 ms. Also prunes `.git` directories (the parent +# repo's, not the worktree's `.git` *file*) so we don't descend into git's +# internal storage. Add new entries as the worktrees tree grows. +readonly _CKIPPER_WT_PRUNE_DIRS=( + .git node_modules .next .nuxt dist build target + .venv venv __pycache__ vendor .turbo .cache +) + # Print all worktrees under $CKIPPER_WORKTREES_DIR, grouped by project. # # Reads CKIPPER_PROJECTS_DIR and CKIPPER_WORKTREES_DIR globals. @@ -11,16 +23,19 @@ _ckipper_worktree_list_worktrees() { [[ ! -d "$CKIPPER_WORKTREES_DIR" ]] && return 0 local previous_project_for_grouping="" - find "$CKIPPER_WORKTREES_DIR" -name ".git" -type f -not -path "*/node_modules/*" 2>/dev/null \ + # Hoist loop locals out of the body: re-declaring `local var` (no =value) + # on a subsequent iteration causes zsh to print `var='prior_value'`, + # leaking lines onto the worktree list. + local wt_dir="" rel="" project="" after_first="" branch="" + _ckipper_worktree_find_worktree_git_files \ | sort \ | while IFS= read -r git_metadata_file; do - local wt_dir="${git_metadata_file:h}" - local rel="${wt_dir#$CKIPPER_WORKTREES_DIR/}" - local project="${rel%%/*}" - local after_first="${rel#*/}" + wt_dir="${git_metadata_file:h}" + rel="${wt_dir#$CKIPPER_WORKTREES_DIR/}" + project="${rel%%/*}" + after_first="${rel#*/}" [[ "$after_first" == "$rel" ]] && continue - local branch IFS=$'\t' read -r project branch < <(_ckipper_worktree_get_project_and_branch "$project" "$after_first") if [[ "$project" != "$previous_project_for_grouping" ]]; then @@ -32,6 +47,26 @@ _ckipper_worktree_list_worktrees() { done } +# Emit the `.git` files of every worktree under $CKIPPER_WORKTREES_DIR, +# pruning known-heavy directories (see _CKIPPER_WT_PRUNE_DIRS) so the scan +# does not descend into node_modules / dist / build / etc. Worktrees mark +# their root with a `.git` *file* (a gitdir reference), not a directory — +# we test for `-type f` to filter accordingly. +# +# Args: none. +# Returns: 0 always; prints absolute paths to `.git` files, one per line. +_ckipper_worktree_find_worktree_git_files() { + local -a prune_args=() + local d + for d in "${_CKIPPER_WT_PRUNE_DIRS[@]}"; do + (( ${#prune_args} > 0 )) && prune_args+=(-o) + prune_args+=(-name "$d") + done + find "$CKIPPER_WORKTREES_DIR" \ + \( -type d \( "${prune_args[@]}" \) -prune \) \ + -o \( -type f -name .git -print \) 2>/dev/null +} + # Resolve project and branch from path components for nested-project worktrees. # # Args: diff --git a/lib/worktree/worktree_test.bats b/lib/worktree/worktree_test.bats index af01dd6..ba3cdc0 100644 --- a/lib/worktree/worktree_test.bats +++ b/lib/worktree/worktree_test.bats @@ -46,6 +46,63 @@ _run_worktree() { [[ "$output" =~ "Worktrees" ]] } +# Regression: scanning was unbounded and pruned only node_modules. With a +# 6 GB worktrees tree the scan took ~700 ms. Pruning the heavy build dirs +# (`dist`, `.next`, `target`, `__pycache__`, etc.) brings it to ~30 ms and +# avoids reporting any phantom `.git` files nested inside those trees. +@test "_ckipper_worktree_list_worktrees skips .git files inside pruned dirs" { + # Real worktree (should appear in output). + mkdir -p "$CKIPPER_WORKTREES_DIR/myapp/feature-x" + touch "$CKIPPER_WORKTREES_DIR/myapp/feature-x/.git" + + # Phantom .git files buried inside dirs we should prune. Each filename + # is unique so we can assert it does NOT show up by name. + mkdir -p "$CKIPPER_WORKTREES_DIR/myapp/feature-x/node_modules/pkg-a" + touch "$CKIPPER_WORKTREES_DIR/myapp/feature-x/node_modules/pkg-a/.git" + mkdir -p "$CKIPPER_WORKTREES_DIR/myapp/feature-x/dist/pkg-b" + touch "$CKIPPER_WORKTREES_DIR/myapp/feature-x/dist/pkg-b/.git" + mkdir -p "$CKIPPER_WORKTREES_DIR/myapp/feature-x/__pycache__/pkg-c" + touch "$CKIPPER_WORKTREES_DIR/myapp/feature-x/__pycache__/pkg-c/.git" + + _run_worktree "_ckipper_worktree_list_worktrees" + + [ "$status" -eq 0 ] + [[ "$output" == *"feature-x"* ]] + [[ "$output" != *"pkg-a"* ]] + [[ "$output" != *"pkg-b"* ]] + [[ "$output" != *"pkg-c"* ]] +} + +# Regression: branches contain slashes (`feature/foo`, `fix/bar`) so the +# scan must not be depth-bounded — the pruned find still has to surface a +# worktree whose `.git` lives several levels deep. +@test "_ckipper_worktree_list_worktrees finds worktrees whose branch name contains slashes" { + mkdir -p "$CKIPPER_WORKTREES_DIR/myapp/feature/OGD-320-deep-branch" + touch "$CKIPPER_WORKTREES_DIR/myapp/feature/OGD-320-deep-branch/.git" + + _run_worktree "_ckipper_worktree_list_worktrees" + + [ "$status" -eq 0 ] + [[ "$output" == *"feature/OGD-320-deep-branch"* ]] +} + +# Regression: `local branch` (no assignment) inside the per-worktree loop +# behaves like `typeset -p branch` once `branch` carries a value from a +# prior iteration, leaking literal `branch='…'` lines onto stdout. The fix +# is `local branch=""`. Two worktrees are needed to trigger iteration N>1 +# on the same loop scope. +@test "_ckipper_worktree_list_worktrees does not leak local-redeclare echoes" { + mkdir -p "$CKIPPER_WORKTREES_DIR/myapp/feature/one" + touch "$CKIPPER_WORKTREES_DIR/myapp/feature/one/.git" + mkdir -p "$CKIPPER_WORKTREES_DIR/myapp/feature/two" + touch "$CKIPPER_WORKTREES_DIR/myapp/feature/two/.git" + + _run_worktree "_ckipper_worktree_list_worktrees" + + [ "$status" -eq 0 ] + [[ "$output" != *"branch="* ]] +} + @test "_ckipper_worktree_remove_worktree fails when worktree path does not exist" { _run_worktree "_ckipper_worktree_remove_worktree myapp nonexistent-branch"