Skip to content

feat(clean): add dirs_cleaner cleanup command#927

Closed
richardgetz wants to merge 1 commit into
tw93:mainfrom
richardgetz:rick/dirs-cleaner-cleanup
Closed

feat(clean): add dirs_cleaner cleanup command#927
richardgetz wants to merge 1 commit into
tw93:mainfrom
richardgetz:rick/dirs-cleaner-cleanup

Conversation

@richardgetz
Copy link
Copy Markdown

Summary

  • audit /private/var/dirs_cleaner during normal system cleanup and report suspicious stuck cleanup staging without deleting by default
  • add explicit mo clean --dirs-cleaner command with --dry-run support for deliberate stale staging cleanup
  • constrain cleanup to same-filesystem, non-symlink, non-mounted, protected/whitelist-respecting top-level or shallow staging entries with fail-closed scans

Why this is safe

  • normal mo clean remains report-only for dirs_cleaner findings
  • cleanup never removes /private/var/dirs_cleaner itself
  • cleanup uses Mole's existing sudo/session and safe_sudo_remove path, with a dirs_cleaner-specific validation context
  • removal candidates must pass full recursive same-filesystem newest-mtime staleness proof; scan failures skip instead of deleting
  • non-empty top-level buckets are not removed as a unit; Mole checks shallow children instead

Testing

  • bash -n bin/clean.sh lib/clean/system.sh lib/core/file_ops.sh lib/core/help.sh tests/clean_system_maintenance.bats tests/core_safe_functions.bats tests/cli.bats
  • git diff --check main...HEAD
  • GOPATH=/private/tmp/mole-gopath GOMODCACHE=/private/tmp/mole-gopath/pkg/mod GOCACHE=/private/tmp/mole-gocache ./scripts/check.sh --no-format
  • GOPATH=/private/tmp/mole-gopath GOMODCACHE=/private/tmp/mole-gopath/pkg/mod GOCACHE=/private/tmp/mole-gocache go test ./cmd/...
  • GOPATH=/private/tmp/mole-gopath GOMODCACHE=/private/tmp/mole-gopath/pkg/mod GOCACHE=/private/tmp/mole-gocache MOLE_GO_TEST_CACHE=/private/tmp/mole-go-build-cache ./scripts/test.sh

Note: bats, shellcheck, and shfmt were not installed in this environment; the repo scripts skipped/fell back as designed.

@richardgetz richardgetz requested a review from tw93 as a code owner May 18, 2026 15:21
@tw93
Copy link
Copy Markdown
Owner

tw93 commented May 22, 2026

@richardgetz Thanks for the very careful work and the detailed safety thinking. I am going to close this PR in its current shape because the explicit cleanup path is too risky for Mole’s current boundary.

The report-only audit idea for /private/var/dirs_cleaner is worth discussing, but I do not want to accept a mo clean --dirs-cleaner command that performs sudo deletion under /private/var/dirs_cleaner, or broaden safe_sudo_remove around that cleanup context. I also reproduced local test failures in the new dirs_cleaner safety coverage, so this is not ready as-is.

If you want to continue this, the mergeable shape would be a much smaller PR that only adds a read-only audit: no deletion command, no safe_sudo_remove context changes, and focused tests for the warning output.

@tw93 tw93 closed this May 22, 2026
@richardgetz
Copy link
Copy Markdown
Author

richardgetz commented May 22, 2026

It started as read-only, but I added the explicit cleanup for others assuming that would be the end goal for anyone else facing stuck "to be deleted" files. Is the expectation to never clean out system files that are meant for deletion? Tbh not worth my time if the tool isn't managing the deletion. Best of luck!

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.

2 participants