fix: verify completed downloads and recover corrupt data - #8
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds engine-owned integrity verification for completed downloads (torrents and direct HTTPS), drives corrupt data back into a recoverable state, and surfaces verification state/results in the TUI—addressing the corruption report in #7.
Changes:
- Add cancellable
Engine.Verify(...)with a structuredVerifyResult, newStateVerifying, and safeguards to block conflicting operations during verification. - Verify completed torrent pieces (triggering repair/redownload on failures) and verify direct downloads via SHA-256 with safe quarantine to
.corrupt,.corrupt.1, etc. - Update the TUI to run verification, show verification toasts, and persist recovery state when repair/quarantine is required; document behavior in the README.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents completed-download verification behavior and quarantine naming. |
| internal/tui/messages.go | Adds verification-related Bubble Tea message types and toast-clear timer. |
| internal/tui/downloads.go | Implements verify action flow, blocks conflicting keys while verifying, persists recovery state updates, and shows verification result toast. |
| internal/tui/downloads_test.go | Adds tests for verification guards, persisted-direct activation hashing, and completion-state clearing logic. |
| internal/tui/app.go | Wires verification messages into the main update loop and ensures ticking continues during verification. |
| internal/engine/engine.go | Introduces Verify, VerifyResult, StateVerifying, verification cancellation/waiting on close, and operation guards during verification. |
| internal/engine/engine_integration_test.go | Adds integration coverage for torrent verification/repair and context cancellation behavior. |
| internal/engine/direct.go | Adds direct-download SHA-256 verification with quarantine and synchronous “persisted file is done” recognition on activation. |
| internal/engine/direct_test.go | Adds direct-download verification tests (valid, persisted, mismatch quarantine, missing checksum, missing file, canceled context). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Quickly looking this over, it looks solid enough to me. I can't test it from where I am right now but if you've tested it, its probably solid. The base patch was significantly better than the original and it looks like you improved upon it quite a bit. Thanks for the quick intervention. And thank you for this tool. ;) |
Summary
.corrupt,.corrupt.1, and so onRoot cause and impact
The existing verification action did not perform engine-owned integrity checking or drive damaged downloads back into a recoverable state. Corrupt completed data could therefore remain marked complete, and direct-download mismatches had no safe quarantine path.
This change limits verification to completed downloads, prevents conflicting operations while hashing, and ensures corrupt data is either repaired automatically (torrents) or quarantined safely (direct downloads).
Validation
Passed locally:
go mod verifygo mod tidy -diffgo vet ./...go build ./...go test -count=1 -timeout 5m ./...go test -race -count=1 -timeout 5m ./...go test -shuffle=on -count=3 -timeout 5m ./...CGO_ENABLED=0The exact CI
CGO_ENABLED=1 GOARCH=riscv64build passed in GitHub Actions.Stress-test disclosure
Repeated same-process
go test -race -count=Nruns can stall while repeatedly constructing and tearing down local anacrolix seeder/client stacks. The stalls occur during the initial loopback transfer before verification begins. Every verification cycle reached completed successfully, and 20 independent fresh-process race runs passed all 100 corruption/repair cycles. This isolates the observed flake to repeated local torrent stacks in one test process rather than the verification path, but it is disclosed here for review.Review focus
Please pay particular attention to:
Done/CompletedAtrecovery transitionsFixes #7