Skip to content

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

Merged
tw93 merged 1 commit into
tw93:mainfrom
xronocode:fix/886-dry-run-protection-ordering
May 15, 2026
Merged

fix(clean): apply should_protect_path in dry-run for orphaned system services#895
tw93 merged 1 commit into
tw93:mainfrom
xronocode:fix/886-dry-run-protection-ordering

Conversation

@xronocode
Copy link
Copy Markdown
Contributor

Summary

  • Move should_protect_path check before the dry-run / real-clean branch in clean_orphaned_system_services() so both modes agree on which paths are protected.
  • Add test verifying dry-run skips protected orphaned services.

Problem

mo clean --dry-run reported an orphaned LaunchDaemon plist as removable:

[DRY RUN] Would remove orphaned service: /Library/LaunchDaemons/com.microsoft.office.licensingV2.helper.plist

But real mo clean silently skipped it because should_protect_path matched the com.microsoft.* protection rule — but only in the real-clean branch. The dry-run branch never called should_protect_path.

Fix

Hoist the protection check above the if [[ "$DRY_RUN" == "true" ]] conditional so it runs unconditionally. Now both dry-run and real clean will report the same set of removable orphans.

Testing

  • All 4 clean_orphaned_system_services bats tests pass, including new regression test.
  • bats tests/clean_apps.bats --filter "orphaned_system_services" → 4/4 ok

Fixes #886

@xronocode xronocode requested a review from tw93 as a code owner May 14, 2026 07:35
…sts (tw93#886)

`mo clean --dry-run` reported orphaned LaunchDaemons (e.g. the Microsoft
Office helper at `com.microsoft.office.licensingV2.helper.plist`) as
"would remove", while the subsequent real `mo clean` silently skipped
them: the `should_protect_path` check ran only inside the real-clean
branch, so dry-run never consulted protection rules and printed an
unreachable promise.

Fix in `lib/clean/apps.sh`:

  - Lift `should_protect_path` above the `DRY_RUN` branch so both modes
    consult it. Protected files now stop the loop body with a single
    `Skipping protected orphaned service` debug entry regardless of
    mode.
  - Ungate the summary line `"Orphaned services skipped N protected,
    failed N"` so it prints in dry-run too. The user now sees what was
    held back; previously dry-run was silent about protected hits.

Tests (`tests/clean_apps.bats`):

  - `dry-run skips protected paths (tw93#886)` — protects every orphan,
    asserts dry-run does NOT print "Would remove" and DOES print the
    skipped summary. Without the fix this case used to print "Would
    remove" in dry-run.
  - `dry-run reports unprotected orphans (tw93#886)` — confirms the
    non-protected path is unchanged: dry-run still prints "Would
    remove" and never prints "Skipping protected".

Both tests pass `MOLE_TEST_NO_AUTH=0` explicitly so the function runs
past the CI auth-skip guard (`scripts/test.sh` exports
`MOLE_TEST_NO_AUTH=1`), and use `|| return 1` after each `[[ ]]` so a
failing assertion in the middle of the body fails the test instead of
being swallowed by a later passing one.

Closes tw93#886
@xronocode xronocode force-pushed the fix/886-dry-run-protection-ordering branch from 611802e to 2ce6a36 Compare May 14, 2026 08:59
@tw93 tw93 merged commit cfe1460 into tw93:main May 15, 2026
7 of 9 checks passed
xronocode added a commit to xronocode/pages that referenced this pull request May 15, 2026
- tw93/Mole#895 — fix(clean): apply should_protect_path in dry-run for orphaned system services
- tw93/Mole#896 — fix(clean): bump simctl probe timeout and retry on cold boot

Both merged by @tw93 2026-05-14; Mole card now has 5 merged PRs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mo clean --dry-run` says orphaned service will be removed, but real clean skips it

2 participants