diff --git a/lib/clean/apps.sh b/lib/clean/apps.sh index ada83103..16382ec0 100644 --- a/lib/clean/apps.sh +++ b/lib/clean/apps.sh @@ -50,7 +50,7 @@ clean_ds_store_tree() { size_human=$(bytes_to_human "$total_bytes") local size_kb=$(((total_bytes + 1023) / 1024)) if [[ "$DRY_RUN" == "true" ]]; then - echo -e " ${YELLOW}${ICON_DRY_RUN}${NC} $label${NC}, ${YELLOW}$file_count files, $(colorize_human_size "$size_human") ${YELLOW}dry${NC}" + echo -e " ${YELLOW}${ICON_DRY_RUN}${NC} $label${NC}, ${YELLOW}$file_count files, $size_human dry${NC}" else local line_color line_color=$(cleanup_result_color_kb "$size_kb") @@ -700,15 +700,14 @@ clean_orphaned_system_services() { local removed_kb=0 for orphan_file in "${orphaned_files[@]}"; do + if should_protect_path "$orphan_file"; then + debug_log "Skipping protected orphaned service: $orphan_file" + skipped_protected_count=$((skipped_protected_count + 1)) + continue + fi if [[ "$DRY_RUN" == "true" ]]; then debug_log "[DRY RUN] Would remove orphaned service: $orphan_file" else - if should_protect_path "$orphan_file"; then - debug_log "Skipping protected orphaned service: $orphan_file" - skipped_protected_count=$((skipped_protected_count + 1)) - continue - fi - local file_size_kb file_size_kb=$(sudo du -skP "$orphan_file" 2> /dev/null | awk '{print $1}' || echo "0") @@ -738,9 +737,14 @@ clean_orphaned_system_services() { echo -e " ${GREEN}${ICON_SUCCESS}${NC} Cleaned $removed_count orphaned services, about $orphaned_kb_display" note_activity fi - if [[ $skipped_protected_count -gt 0 || $failed_count -gt 0 ]]; then - echo -e " ${GRAY}${ICON_WARNING}${NC} Orphaned services skipped $skipped_protected_count protected, failed $failed_count" - fi + fi + # Surface protected/failed counts in BOTH dry-run and real-clean so the + # two modes agree on what gets touched. Before #886, dry-run silently + # reported protected files under "Would remove" and real-clean then + # skipped them, leaving the user confused about which files actually + # disappeared. + if [[ $skipped_protected_count -gt 0 || $failed_count -gt 0 ]]; then + echo -e " ${GRAY}${ICON_WARNING}${NC} Orphaned services skipped $skipped_protected_count protected, failed $failed_count" fi fi diff --git a/tests/clean_apps.bats b/tests/clean_apps.bats index 685bb290..582ce211 100644 --- a/tests/clean_apps.bats +++ b/tests/clean_apps.bats @@ -702,6 +702,93 @@ EOF [[ "$output" != *"unexpected-launchctl"* ]] } +@test "clean_orphaned_system_services dry-run skips protected paths (#886)" { + # MOLE_TEST_NO_AUTH=0 overrides the CI default (=1) so the function actually + # runs past the auth-skip guard in apps.sh; the sudo() mock satisfies the + # `sudo -n true` probe. + run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" MOLE_TEST_MODE=0 MOLE_TEST_NO_AUTH=0 DRY_RUN=true bash --noprofile --norc <<'EOF' +set -euo pipefail +source "$PROJECT_ROOT/lib/core/common.sh" +source "$PROJECT_ROOT/lib/clean/apps.sh" + +start_section_spinner() { :; } +stop_section_spinner() { :; } +note_activity() { :; } +debug_log() { echo "debug: $*"; } + +should_protect_path() { return 0; } + +tmp_dir="$(mktemp -d)" +tmp_plist="$tmp_dir/com.microsoft.office.licensingV2.helper.plist" +/usr/libexec/PlistBuddy -c "Add :Program string /Library/PrivilegedHelperTools/com.microsoft.office.licensingV2.helper" "$tmp_plist" 2>/dev/null || true + +sudo() { + if [[ "$1" == "-n" && "$2" == "true" ]]; then + return 0 + fi + if [[ "$1" == "find" ]]; then + case "$2" in + /Library/LaunchDaemons) printf '%s\0' "$tmp_plist" ;; + *) : ;; + esac + return 0 + fi + command "$@" +} + +clean_orphaned_system_services +EOF + + # `|| return 1` after each assertion ensures bats fails as soon as one fails + # (bare `[[ ]]` in the middle of a test body gets swallowed by the next + # passing command — see #886 review notes). + [ "$status" -eq 0 ] + [[ "$output" == *"Found 1 orphaned"* ]] || return 1 + [[ "$output" == *"skipped 1 protected"* ]] || return 1 + [[ "$output" != *"Would remove orphaned service"* ]] || return 1 +} + +@test "clean_orphaned_system_services dry-run reports unprotected orphans (#886)" { + # MOLE_TEST_NO_AUTH=0 overrides CI default so the function executes. + run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" MOLE_TEST_MODE=0 MOLE_TEST_NO_AUTH=0 DRY_RUN=true bash --noprofile --norc <<'EOF' +set -euo pipefail +source "$PROJECT_ROOT/lib/core/common.sh" +source "$PROJECT_ROOT/lib/clean/apps.sh" + +start_section_spinner() { :; } +stop_section_spinner() { :; } +note_activity() { :; } +debug_log() { echo "debug: $*"; } + +should_protect_path() { return 1; } + +tmp_dir="$(mktemp -d)" +tmp_plist="$tmp_dir/com.example.unprotected.orphan.plist" +/usr/libexec/PlistBuddy -c "Add :Program string $tmp_dir/missing-binary" "$tmp_plist" 2>/dev/null || true + +sudo() { + if [[ "$1" == "-n" && "$2" == "true" ]]; then + return 0 + fi + if [[ "$1" == "find" ]]; then + case "$2" in + /Library/LaunchDaemons) printf '%s\0' "$tmp_plist" ;; + *) : ;; + esac + return 0 + fi + command "$@" +} + +clean_orphaned_system_services +EOF + + [ "$status" -eq 0 ] + [[ "$output" == *"Found 1 orphaned"* ]] || return 1 + [[ "$output" == *"Would remove orphaned service"* ]] || return 1 + [[ "$output" != *"Skipping protected"* ]] || return 1 +} + @test "clean_orphaned_launch_agents preserves user launch agents" { run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc <<'EOF' set -euo pipefail