fix: re-verify each file's hash before deleting it [minor]

[matt-edmondson]

Fixes #112

Problem

Deduplicator.DeleteDuplicates deleted every non-keeper file using only the hash grouping computed before the Proceed with deletion? (y/N) prompt in Deduplicate.Run. That prompt is an interactive pause of unbounded length, and nothing re-read the files afterwards — File.Delete ran straight off the stale hash map.

So a file modified during the pause (a sync client, an editor autosave, a restored backup) was deleted as a duplicate even though it was no longer identical to the kept copy. Permanent loss of content, from a tool whose whole contract is "these files are identical."

Reproduced on main: three copies of shared, rewrite bb.txt while the run is paused, answer y — bb.txt is deleted and the run reports a clean success.

Fix

Each file is re-hashed immediately before it is deleted, and skipped if it no longer matches its group's hash:

if (!StillMatchesGroup(file, group.Hash, out string? reason))
{
    Skip(file, reason, skipped);
    continue;
}

The full re-hash is used rather than a size + mtime check, per the triage note: the I/O cost is paid once per deleted file and the alternative is unrecoverable.

The kept copy is verified first. If it changed during the pause, the whole group is left alone — deleting the rest would destroy the only remaining copies of the content the group was formed from, which is the same data-loss failure by a different route.

Skips are reported rather than swallowed. DeduplicationResult gains SkippedFiles (a new SkippedFile record of path + reason), the delete loop prints each skip as it happens, and the Deduplicate summary lists the preserved files. A file that vanished before the delete pass now reports as skipped instead of landing in Errors, so DeletedCount and BytesReclaimed only ever describe files that were actually removed.

The remaining window between the verification read and File.Delete is inherent to the filesystem API; this narrows it from "however long the user takes to answer" to microseconds.

Out of scope, deliberately: the UnauthorizedAccessException around File.Delete itself (#113) is untouched — this change only handles that exception where its own verification read could raise it. #114 (showing what will be deleted before the prompt) is likewise untouched.

Tests

Five tests in DeduplicatorTests, driving the real filesystem through TempTree as the existing tests do:

• AFileThatChangedAfterGroupingIsPreservedAndReported — the issue's acceptance criterion: a "duplicate" rewritten after grouping survives, is reported in SkippedFiles, and the copy that is still identical is still deleted.
• AChangedKeeperPreservesTheWholeGroup — a rewritten keeper preserves every copy, deleting nothing.
• AFileThatVanishedBeforeDeletionIsReportedNotCounted — a file removed before the delete pass is skipped, not counted as deleted.
• APathReplacedByADirectoryIsSkippedRatherThanDeleted — the file at a grouped path becomes a directory, so re-reading it raises UnauthorizedAccessException; it is skipped, which also keeps File.Delete from raising the same exception uncaught.
• UntouchedDuplicatesAreDeletedWithNothingSkipped — guards against the verification degrading into "always skip".

Verified by neutralising the two verification guards and re-running: 4 of the 5 fail on the old behaviour (the fifth is the guard, and passes either way); AFileThatChangedAfterGroupingIsPreservedAndReported fails exactly as the issue describes, with the changed file deleted. All 5 pass with the fix.

Full suite: 26 total, 26 passed, 0 failed (21 before this change), green on ubuntu, windows and macos. dotnet build -c Release: 0 warnings, 0 errors. SonarCloud: quality gate passed, 0 new issues, 88.6% coverage on new code.

Follow-up commits

• b1ae8f0 adds APathReplacedByADirectoryIsSkippedRatherThanDeleted, after SonarCloud's gate failed on coverage of new code (78.1% against 80%). The uncovered lines were the UnauthorizedAccessException arm of StillMatchesGroup and the verb's summary block.
• 221c593 answers SonarCloud's S3776 on DeleteDuplicates (cognitive complexity 17 against 15 allowed, from the branches added here): the preserve-the-whole-group loop moved into SkipWholeGroup, and the keeper is filtered out of the deletion loop with Where rather than a continue in the body. Behaviour unchanged; the method scores 11.

🤖 Generated with Claude Code

https://claude.ai/code/session_011ghFwAUJnfH7mfoWm1bCCh

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
88.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud