Skip to content

Fix quarantine dir re-scan and phantom runId on empty removal set#6

Merged
Therosin merged 3 commits intocodex/add-clean-mode-options-to-logic.tsfrom
copilot/sub-pr-5
Mar 1, 2026
Merged

Fix quarantine dir re-scan and phantom runId on empty removal set#6
Therosin merged 3 commits intocodex/add-clean-mode-options-to-logic.tsfrom
copilot/sub-pr-5

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 1, 2026

  • Analyze review comments
  • Replace all hardcoded .modcleaner_quarantine strings in main.ts with ${QUARANTINE_DIR} (4 occurrences: HTML template, inline JS, confirmation message, status summary)
  • Add QUARANTINE_DIR to the import in main.ts
  • Replace hardcoded .modcleaner_quarantine in logic_test.ts with QUARANTINE_DIR
  • Regenerate deno.lock cleanly with Deno v2.7.1
  • All 47 tests pass
  • Code review: no issues
  • Security scan: 0 alerts

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: Therosin <747653+Therosin@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 1, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.deno.land
    • Triggering command: /usr/bin/curl curl -s REDACTED (dns block)
  • jsr.io
    • Triggering command: /usr/local/bin/deno /usr/local/bin/deno test -A (dns block)
    • Triggering command: /usr/bin/ping ping -c 1 jsr.io (dns block)
    • Triggering command: /usr/local/bin/deno deno cache --lock=deno.lock logic_test.ts (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Add quarantine clean mode with UI selector and tests Fix quarantine dir re-scan and phantom runId on empty removal set Mar 1, 2026
@Therosin Therosin marked this pull request as ready for review March 1, 2026 12:39
Copilot AI review requested due to automatic review settings March 1, 2026 12:39
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes two quarantine-mode edge cases in the core cleaning logic: previously quarantined content could be re-discovered by subsequent scans/clean runs, and a quarantine runId could be returned even when no files were quarantined (leading to a non-existent directory reference).

Changes:

  • Export QUARANTINE_DIR and use it in quarantine path creation.
  • Update walkFiles() to skip traversing the quarantine directory so quarantined files are excluded from scanning/keeplist generation.
  • Add tests covering (a) undefined quarantineRunId when nothing is removable and (b) exclusion of pre-existing quarantine contents from scans.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

File Description
logic.ts Adds QUARANTINE_DIR, skips quarantine folder during traversal, and returns undefined from quarantining when there’s nothing to move.
logic_test.ts Adds regression tests for empty quarantine and for excluding quarantine contents from listRelativeFiles/buildScanPlan.
deno.lock Rewrites the lockfile format/content (currently appears inconsistent with the repo’s import map/dependency set).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 10 to 12
export const KEEPLIST_FILE = "#keeplist.txt";
export const QUARANTINE_DIR = ".modcleaner_quarantine";
export const GENERATED_KEEPLIST_HEADER = [
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description says QUARANTINE_DIR is used consistently “in place of inline strings”, but there are still hard-coded .modcleaner_quarantine string references elsewhere in the repo (e.g. main.ts UI text and an existing test path). Either update those remaining references to use QUARANTINE_DIR (where appropriate) or adjust the PR description to reflect the actual scope of the change.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback
make sure you are using deno v2 and that the lockfile is regenerated cleanly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in commit 3b919ce. All four hardcoded .modcleaner_quarantine references in main.ts (HTML help text, inline JS help string, confirmation message, and status summary) now use ${QUARANTINE_DIR} via the template literal, and QUARANTINE_DIR is added to the import. The logic_test.ts path also uses QUARANTINE_DIR. The lockfile was regenerated cleanly with Deno v2.7.1. All 47 tests pass.

… lockfile

Co-authored-by: Therosin <747653+Therosin@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 1, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • google.com
    • Triggering command: /usr/bin/ping ping -c 1 google.com (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@Therosin Therosin merged commit 3f080fe into codex/add-clean-mode-options-to-logic.ts Mar 1, 2026
@Therosin Therosin deleted the copilot/sub-pr-5 branch March 1, 2026 12:52
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.

3 participants