Code optimization as a whole

bin/clean.sh

@@ -11,6 +11,7 @@ export LANG=C
 # Get script directory and source common functions
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 source "$SCRIPT_DIR/../lib/common.sh"
+source "$SCRIPT_DIR/../lib/sudo_manager.sh"
 source "$SCRIPT_DIR/../lib/clean_brew.sh"
 source "$SCRIPT_DIR/../lib/clean_caches.sh"
 source "$SCRIPT_DIR/../lib/clean_apps.sh"
@@ -119,7 +120,6 @@ SECTION_ACTIVITY=0
 files_cleaned=0
 total_size_cleaned=0
 whitelist_skipped_count=0
-SUDO_KEEPALIVE_PID=""
 
 note_activity() {
     if [[ $TRACK_SECTION -eq 1 ]]; then
@@ -140,12 +140,6 @@ cleanup() {
     CLEANUP_DONE=true
 
     # Stop all spinners and clear the line
-    if [[ -n "$SPINNER_PID" ]]; then
-        kill "$SPINNER_PID" 2> /dev/null || true
-        wait "$SPINNER_PID" 2> /dev/null || true
-        SPINNER_PID=""
-    fi
-
     if [[ -n "$INLINE_SPINNER_PID" ]]; then
         kill "$INLINE_SPINNER_PID" 2> /dev/null || true
         wait "$INLINE_SPINNER_PID" 2> /dev/null || true
@@ -157,12 +151,8 @@ cleanup() {
         printf "\r\033[K"
     fi
 
-    # Stop sudo keepalive
+    # Stop sudo session
-    if [[ -n "$SUDO_KEEPALIVE_PID" ]]; then
+    stop_sudo_session
-        kill "$SUDO_KEEPALIVE_PID" 2> /dev/null || true
-        wait "$SUDO_KEEPALIVE_PID" 2> /dev/null || true
-        SUDO_KEEPALIVE_PID=""
-    fi
 
     show_cursor
 
@@ -177,51 +167,6 @@ trap 'cleanup EXIT $?' EXIT
 trap 'cleanup INT 130; exit 130' INT
 trap 'cleanup TERM 143; exit 143' TERM
 
-# Loading animation functions
-SPINNER_PID=""
-start_spinner() {
-    local message="$1"
-
-    if [[ ! -t 1 ]]; then
-        echo -n "  ${BLUE}${ICON_CONFIRM}${NC} $message"
-        return
-    fi
-
-    echo -n "  ${BLUE}${ICON_CONFIRM}${NC} $message"
-    (
-        local delay=0.5
-        while true; do
-            printf "\r  ${BLUE}${ICON_CONFIRM}${NC} $message.  "
-            sleep $delay
-            printf "\r  ${BLUE}${ICON_CONFIRM}${NC} $message.. "
-            sleep $delay
-            printf "\r  ${BLUE}${ICON_CONFIRM}${NC} $message..."
-            sleep $delay
-            printf "\r  ${BLUE}${ICON_CONFIRM}${NC} $message   "
-            sleep $delay
-        done
-    ) &
-    SPINNER_PID=$!
-}
-
-stop_spinner() {
-    local result_message="${1:-Done}"
-
-    if [[ ! -t 1 ]]; then
-        echo " ✓ $result_message"
-        return
-    fi
-
-    if [[ -n "$SPINNER_PID" ]]; then
-        kill "$SPINNER_PID" 2> /dev/null
-        wait "$SPINNER_PID" 2> /dev/null
-        SPINNER_PID=""
-        printf "\r  ${GREEN}${ICON_SUCCESS}${NC} %s\n" "$result_message"
-    else
-        echo "  ${GREEN}${ICON_SUCCESS}${NC} $result_message"
-    fi
-}
-
 start_section() {
     TRACK_SECTION=1
     SECTION_ACTIVITY=0
@@ -459,38 +404,10 @@ start_cleanup() {
         # Enter = yes, do system cleanup
         elif [[ "$choice" == "ENTER" ]]; then
             printf "\r\033[K" # Clear the prompt line
-            if request_sudo_access "System cleanup requires admin access"; then
+            if ensure_sudo_session "System cleanup requires admin access"; then
                 SYSTEM_CLEAN=true
                 echo -e "${GREEN}${ICON_SUCCESS}${NC} Admin access granted"
                 echo ""
-                # Start sudo keepalive with robust parent checking
-                # Store parent PID to ensure keepalive exits if parent dies
-                parent_pid=$$
-                (
-                    # Initial delay to let sudo cache stabilize after password entry
-                    # This prevents immediately triggering Touch ID again
-                    sleep 2
-
-                    local retry_count=0
-                    while true; do
-                        # Check if parent process still exists first
-                        if ! kill -0 "$parent_pid" 2> /dev/null; then
-                            exit 0
-                        fi
-
-                        if ! sudo -n true 2> /dev/null; then
-                            ((retry_count++))
-                            if [[ $retry_count -ge 3 ]]; then
-                                exit 1
-                            fi
-                            sleep 5
-                            continue
-                        fi
-                        retry_count=0
-                        sleep 30
-                    done
-                ) 2> /dev/null &
-                SUDO_KEEPALIVE_PID=$!
             else
                 SYSTEM_CLEAN=false
                 echo ""

lib/clean_system.sh

@@ -7,31 +7,31 @@ set -euo pipefail
 # Deep system cleanup (requires sudo)
 clean_deep_system() {
     # Clean old system caches
-    safe_sudo_find_delete "/Library/Caches" "*.cache" "$MOLE_TEMP_FILE_AGE_DAYS" "f"
+    safe_sudo_find_delete "/Library/Caches" "*.cache" "$MOLE_TEMP_FILE_AGE_DAYS" "f" || true
-    safe_sudo_find_delete "/Library/Caches" "*.tmp" "$MOLE_TEMP_FILE_AGE_DAYS" "f"
+    safe_sudo_find_delete "/Library/Caches" "*.tmp" "$MOLE_TEMP_FILE_AGE_DAYS" "f" || true
-    safe_sudo_find_delete "/Library/Caches" "*.log" "$MOLE_LOG_AGE_DAYS" "f"
+    safe_sudo_find_delete "/Library/Caches" "*.log" "$MOLE_LOG_AGE_DAYS" "f" || true
 
     # Clean old temp files
     local tmp_cleaned=0
     local tmp_count=$(sudo find /tmp -type f -mtime +"${MOLE_TEMP_FILE_AGE_DAYS}" 2> /dev/null | wc -l | tr -d ' ')
     if [[ "$tmp_count" -gt 0 ]]; then
-        safe_sudo_find_delete "/tmp" "*" "${MOLE_TEMP_FILE_AGE_DAYS}" "f"
+        safe_sudo_find_delete "/tmp" "*" "${MOLE_TEMP_FILE_AGE_DAYS}" "f" || true
         tmp_cleaned=1
     fi
     local var_tmp_count=$(sudo find /var/tmp -type f -mtime +"${MOLE_TEMP_FILE_AGE_DAYS}" 2> /dev/null | wc -l | tr -d ' ')
     if [[ "$var_tmp_count" -gt 0 ]]; then
-        safe_sudo_find_delete "/var/tmp" "*" "${MOLE_TEMP_FILE_AGE_DAYS}" "f"
+        safe_sudo_find_delete "/var/tmp" "*" "${MOLE_TEMP_FILE_AGE_DAYS}" "f" || true
         tmp_cleaned=1
     fi
     [[ $tmp_cleaned -eq 1 ]] && log_success "Old system temp files (${MOLE_TEMP_FILE_AGE_DAYS}+ days)"
 
     # Clean crash reports
-    safe_sudo_find_delete "/Library/Logs/DiagnosticReports" "*" "$MOLE_CRASH_REPORT_AGE_DAYS" "f"
+    safe_sudo_find_delete "/Library/Logs/DiagnosticReports" "*" "$MOLE_CRASH_REPORT_AGE_DAYS" "f" || true
     log_success "Old system crash reports (${MOLE_CRASH_REPORT_AGE_DAYS}+ days)"
 
     # Clean system logs
-    safe_sudo_find_delete "/var/log" "*.log" "$MOLE_LOG_AGE_DAYS" "f"
+    safe_sudo_find_delete "/var/log" "*.log" "$MOLE_LOG_AGE_DAYS" "f" || true
-    safe_sudo_find_delete "/var/log" "*.gz" "$MOLE_LOG_AGE_DAYS" "f"
+    safe_sudo_find_delete "/var/log" "*.gz" "$MOLE_LOG_AGE_DAYS" "f" || true
     log_success "Old system logs (${MOLE_LOG_AGE_DAYS}+ days)"
 
     # Clean Library Updates safely - skip if SIP is enabled to avoid error messages
@@ -40,7 +40,7 @@ clean_deep_system() {
         if is_sip_enabled; then
             # SIP is enabled, skip /Library/Updates entirely to avoid error messages
             # These files are system-protected and cannot be removed
-            :  # No-op, silently skip
+            : # No-op, silently skip
         else
             # SIP is disabled, attempt cleanup with restricted flag check
             local updates_cleaned=0

lib/clean_user_apps.sh

@@ -101,8 +101,8 @@ clean_media_players() {
     local has_offline_music=false
 
     # Check for offline music database or large cache (>500MB)
-    if [[ -f "$spotify_data/PersistentCache/Storage/offline.bnk" ]] || \
+    if [[ -f "$spotify_data/PersistentCache/Storage/offline.bnk" ]] ||
-       [[ -d "$spotify_data/PersistentCache/Storage" && -n "$(find "$spotify_data/PersistentCache/Storage" -type f -name "*.file" 2>/dev/null | head -1)" ]]; then
+        [[ -d "$spotify_data/PersistentCache/Storage" && -n "$(find "$spotify_data/PersistentCache/Storage" -type f -name "*.file" 2> /dev/null | head -1)" ]]; then
         has_offline_music=true
     elif [[ -d "$spotify_cache" ]]; then
         local cache_size_kb

lib/common.sh

@@ -211,9 +211,9 @@ safe_find_delete() {
     local type_filter="${4:-f}"
 
     # Validate base directory exists and is not a symlink
-    # Silently skip if directory does not exist (e.g., old macOS paths)
     if [[ ! -d "$base_dir" ]]; then
-        return 0
+        log_error "Directory does not exist: $base_dir"
+        return 1
     fi
 
     if [[ -L "$base_dir" ]]; then
@@ -247,9 +247,9 @@ safe_sudo_find_delete() {
     local type_filter="${4:-f}"
 
     # Validate base directory exists and is not a symlink
-    # Silently skip if directory does not exist (e.g., old macOS paths)
     if [[ ! -d "$base_dir" ]]; then
-        return 0
+        log_error "Directory does not exist: $base_dir"
+        return 1
     fi
 
     if [[ -L "$base_dir" ]]; then

tests/optimization_tasks.bats

@@ -3,6 +3,29 @@
 setup_file() {
     PROJECT_ROOT="$(cd "${BATS_TEST_DIRNAME}/.." && pwd)"
     export PROJECT_ROOT
+
+    ORIGINAL_HOME="${HOME:-}"
+    export ORIGINAL_HOME
+
+    HOME="$(mktemp -d "${BATS_TEST_DIRNAME}/tmp-opt-home.XXXXXX")"
+    export HOME
+
+    mkdir -p "$HOME"
+}
+
+teardown_file() {
+    rm -rf "$HOME"
+    if [[ -n "${ORIGINAL_HOME:-}" ]]; then
+        export HOME="$ORIGINAL_HOME"
+    fi
+}
+
+setup() {
+    export TERM="dumb"
+    rm -rf "${HOME:?}"/*
+    mkdir -p "$HOME/Library/Application Support/com.apple.sharedfilelist"
+    mkdir -p "$HOME/Library/Caches"
+    mkdir -p "$HOME/Library/Saved Application State"
 }
 
 @test "run_with_timeout succeeds without GNU timeout" {
@@ -24,3 +47,164 @@ setup_file() {
     '
     [ "$status" -eq 124 ]
 }
+
+@test "opt_recent_items removes shared file lists" {
+    local shared_dir="$HOME/Library/Application Support/com.apple.sharedfilelist"
+    mkdir -p "$shared_dir"
+    touch "$shared_dir/test.sfl2"
+    touch "$shared_dir/recent.sfl2"
+
+    run env HOME="$HOME" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+# Mock sudo and defaults to avoid system changes
+sudo() { return 0; }
+defaults() { return 0; }
+export -f sudo defaults
+opt_recent_items
+EOF
+
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"Recent items cleared"* ]]
+}
+
+@test "opt_recent_items handles missing shared directory" {
+    rm -rf "$HOME/Library/Application Support/com.apple.sharedfilelist"
+
+    run env HOME="$HOME" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+sudo() { return 0; }
+defaults() { return 0; }
+export -f sudo defaults
+opt_recent_items
+EOF
+
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"Recent items cleared"* ]]
+}
+
+@test "opt_saved_state_cleanup removes old saved states" {
+    local state_dir="$HOME/Library/Saved Application State"
+    mkdir -p "$state_dir/com.example.app.savedState"
+    touch "$state_dir/com.example.app.savedState/data.plist"
+
+    # Make the file old (8+ days) - MOLE_SAVED_STATE_AGE_DAYS defaults to 7
+    touch -t 202301010000 "$state_dir/com.example.app.savedState/data.plist"
+
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+opt_saved_state_cleanup
+EOF
+
+    [ "$status" -eq 0 ]
+}
+
+@test "opt_saved_state_cleanup handles missing state directory" {
+    rm -rf "$HOME/Library/Saved Application State"
+
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+opt_saved_state_cleanup
+EOF
+
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"No saved states directory"* ]]
+}
+
+@test "opt_cache_refresh cleans Quick Look cache" {
+    mkdir -p "$HOME/Library/Caches/com.apple.QuickLook.thumbnailcache"
+    touch "$HOME/Library/Caches/com.apple.QuickLook.thumbnailcache/test.db"
+
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+# Mock qlmanage and cleanup_path to avoid system calls
+qlmanage() { return 0; }
+cleanup_path() {
+    local path="$1"
+    local label="${2:-}"
+    [[ -e "$path" ]] && rm -rf "$path" 2>/dev/null || true
+}
+export -f qlmanage cleanup_path
+opt_cache_refresh
+EOF
+
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"Finder and Safari caches updated"* ]]
+}
+
+@test "opt_mail_downloads skips cleanup when size below threshold" {
+    mkdir -p "$HOME/Library/Mail Downloads"
+    # Create small file (below threshold of 5MB)
+    echo "test" > "$HOME/Library/Mail Downloads/small.txt"
+
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+# MOLE_MAIL_DOWNLOADS_MIN_KB is readonly, defaults to 5120 KB (~5MB)
+opt_mail_downloads
+EOF
+
+    [ "$status" -eq 0 ]
+    [[ "$output" == *"skipping cleanup"* ]]
+    [ -f "$HOME/Library/Mail Downloads/small.txt" ]
+}
+
+@test "opt_mail_downloads removes old attachments" {
+    mkdir -p "$HOME/Library/Mail Downloads"
+    touch "$HOME/Library/Mail Downloads/old.pdf"
+    # Make file old (31+ days) - MOLE_LOG_AGE_DAYS defaults to 30
+    touch -t 202301010000 "$HOME/Library/Mail Downloads/old.pdf"
+
+    # Create large enough size to trigger cleanup (>5MB threshold)
+    dd if=/dev/zero of="$HOME/Library/Mail Downloads/dummy.dat" bs=1024 count=6000 2>/dev/null
+
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+# MOLE_MAIL_DOWNLOADS_MIN_KB and MOLE_LOG_AGE_DAYS are readonly constants
+opt_mail_downloads
+EOF
+
+    [ "$status" -eq 0 ]
+}
+
+@test "_opt_get_dir_size_kb returns zero for missing directory" {
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+size=$(_opt_get_dir_size_kb "/nonexistent/path")
+echo "$size"
+EOF
+
+    [ "$status" -eq 0 ]
+    [ "$output" = "0" ]
+}
+
+@test "_opt_get_dir_size_kb calculates directory size" {
+    mkdir -p "$HOME/test_size"
+    dd if=/dev/zero of="$HOME/test_size/file.dat" bs=1024 count=10 2>/dev/null
+
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/optimization_tasks.sh"
+size=$(_opt_get_dir_size_kb "$HOME/test_size")
+echo "$size"
+EOF
+
+    [ "$status" -eq 0 ]
+    # Should be >= 10 KB
+    [ "$output" -ge 10 ]
+}

tests/uninstall.bats

@@ -115,3 +115,73 @@ EOF
 
     [ "$status" -eq 0 ]
 }
+
+@test "decode_file_list validates base64 encoding" {
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/uninstall_batch.sh"
+
+# Valid base64 encoded path list
+valid_data=$(printf '/path/one\n/path/two' | base64)
+result=$(decode_file_list "$valid_data" "TestApp")
+[[ -n "$result" ]] || exit 1
+EOF
+
+    [ "$status" -eq 0 ]
+}
+
+@test "decode_file_list rejects invalid base64" {
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/uninstall_batch.sh"
+
+# Invalid base64 - function should return empty and fail
+if result=$(decode_file_list "not-valid-base64!!!" "TestApp" 2>/dev/null); then
+    # If decode succeeded, result should be empty
+    [[ -z "$result" ]]
+else
+    # Function returned error, which is expected
+    true
+fi
+EOF
+
+    [ "$status" -eq 0 ]
+}
+
+@test "decode_file_list handles empty input" {
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/uninstall_batch.sh"
+
+# Empty base64
+empty_data=$(printf '' | base64)
+result=$(decode_file_list "$empty_data" "TestApp" 2>/dev/null) || true
+# Empty result is acceptable
+[[ -z "$result" ]]
+EOF
+
+    [ "$status" -eq 0 ]
+}
+
+@test "decode_file_list rejects non-absolute paths" {
+    run env HOME="$HOME" PROJECT_ROOT="$PROJECT_ROOT" bash --noprofile --norc << 'EOF'
+set -euo pipefail
+source "$PROJECT_ROOT/lib/common.sh"
+source "$PROJECT_ROOT/lib/uninstall_batch.sh"
+
+# Relative path - function should reject it
+bad_data=$(printf 'relative/path' | base64)
+if result=$(decode_file_list "$bad_data" "TestApp" 2>/dev/null); then
+    # Should return empty string
+    [[ -z "$result" ]]
+else
+    # Or return error code
+    true
+fi
+EOF
+
+    [ "$status" -eq 0 ]
+}