Add quarantine clean mode with UI selector and tests#5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a “quarantine” clean mode to ModCleaner so users can choose between permanently deleting removable files or moving them into a timestamped quarantine folder, while preserving existing scan/rename semantics and adding test coverage for the new behavior.
Changes:
- Extend
cleanFolderDetailedwith amodeoption (deletevsquarantine) and return mode metadata inCleanResult. - Add a UI selector for clean mode, update confirmation messaging, and surface quarantine location in the success status.
- Add Deno tests validating quarantine move behavior and rename behavior under quarantine mode.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| main.ts | Adds clean-mode selector UI, mode-aware confirmation copy, and passes selected mode to cleanFolderDetailed. |
| logic.ts | Introduces CleanMode, quarantine move implementation, and returns mode/quarantineRunId in CleanResult. |
| logic_test.ts | Adds tests for quarantine move layout/data integrity and rename behavior under quarantine mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const runId = buildQuarantineRunId(); | ||
| const quarantineRoot = join(root, ".modcleaner_quarantine", runId); | ||
|
|
||
| for (const file of removableFiles) { | ||
| const sourcePath = join(root, file); | ||
| const targetPath = join(quarantineRoot, file); | ||
| await Deno.mkdir(dirname(targetPath), { recursive: true }); | ||
| await Deno.rename(sourcePath, targetPath); | ||
| } |
There was a problem hiding this comment.
Quarantine mode stores removed files under .modcleaner_quarantine/<runId>/ inside the cleaned root, but buildScanPlan() / listRelativeFiles() will include that folder on subsequent scans. That means a later Scan/Clean can delete or re-quarantine previously quarantined files (and writeKeeplist() can also end up enumerating them), which defeats the purpose of quarantine. Consider hard-excluding .modcleaner_quarantine/** from file walking / removable-file selection (similar to how #keeplist.txt is excluded), or add it to the protected paths set so it is never considered removable.
| async function quarantineFiles( | ||
| root: string, | ||
| removableFiles: string[], | ||
| ): Promise<string> { | ||
| const runId = buildQuarantineRunId(); | ||
| const quarantineRoot = join(root, ".modcleaner_quarantine", runId); | ||
|
|
||
| for (const file of removableFiles) { | ||
| const sourcePath = join(root, file); | ||
| const targetPath = join(quarantineRoot, file); | ||
| await Deno.mkdir(dirname(targetPath), { recursive: true }); | ||
| await Deno.rename(sourcePath, targetPath); | ||
| } | ||
|
|
||
| return runId; | ||
| } |
There was a problem hiding this comment.
quarantineFiles() always generates and returns a runId even when removableFiles is empty, but it never creates the corresponding .modcleaner_quarantine/<runId>/ directory in that case. This can lead the UI/status output to claim files were quarantined at a path that doesn't actually exist. Consider either (a) returning undefined when there are no removable files, or (b) ensuring the run folder is created even when empty so the returned runId is always valid.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71a8ba12ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Co-authored-by: Therosin <747653+Therosin@users.noreply.github.com>
… lockfile Co-authored-by: Therosin <747653+Therosin@users.noreply.github.com>
Fix quarantine dir re-scan and phantom runId on empty removal set
Motivation
Description
CleanModeandCleanFolderDetailedOptionsand extendcleanFolderDetailedto accept amodeoption with `Codex Task