fix(clean): apply should_protect_path in dry-run for orphaned system services

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
 

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