Files

T

docs(findings): note crash-during-download leaves orphan archives

While reviewing the cancel-cleanup fix (`fix(peer): delete partial files
when a download is cancelled`), it became clear that the new
`discard_cancelled_download` sweep only runs from inside the in-flight
orchestrator. If the process dies mid-download (kill, crash, power loss)
the partial `.eti` archives are left behind: `install::recover_on_startup`
calls `recover_download_transients`, which only removes
`.version.ini.tmp` and `.version.ini.discarded`. The user is left with
a card that looks "downloaded" but with corrupted archives that can only
be cleared via the explicit `Remove files` action.

Closing the gap would mean running the same discard pass during recovery
for any game root whose install intent is `None` and whose `version.ini`
is absent (intent log already distinguishes installed-and-then-broken
from interrupted-download). Not blocking — the user-initiated cancel
button is now correct in its scope; this is the symmetric crash recovery
case captured for a future cleanup pass.

Refs: c380046 (fix(peer): delete partial files when a download is cancelled)

2026-05-21 00:07:24 +02:00

2.6 KiB

Raw Blame History

Findings

Open

Crash-during-download leaves orphan archive files

crates/lanspread-peer/src/install/transaction.rs:329 — recover_download_transients sweeps only .version.ini.tmp and .version.ini.discarded on startup. The new cancel-cleanup (download/storage.rs::discard_cancelled_download) is only invoked from the in-flight orchestrator, so a crash mid-download leaves partial .eti archives in the game root. After restart the user sees a game that looks half-downloaded with no way to clean it up except RemoveDownloadedGame. Closing this would mean calling the same discard pass during recovery for any game root whose intent is None and whose version.ini is absent.

Not blocking. The cancel-button fix is correct in its scope; this is the symmetric crash-recovery case.

`handleErrorEvent` still writes status fields directly

crates/lanspread-tauri-deno-ts/src/hooks/useGames.ts:80-89 — the error handler writes install_status, status_message, status_level, and download_progress from a lifecycle event, which is the same "two sources of truth" pattern that commit 5df82aa ("fix(ui): derive operation status from snapshots") removed everywhere else. That commit explicitly carved out error messages as a preserved side effect, so this is a documented exception rather than a regression — but if we want strict snapshot-is-truth, the error handler should stop writing status fields and let the next snapshot reconcile the card, keeping only the error message overlay (which the snapshot does not carry).

Not blocking. Captured here for a future cleanup pass.

Claude Review Scope Triage

No out-of-scope code smells or issues were identified in Claude's review. All four points were direct follow-up cleanup for the current protocol change and were handled in code.

The previous three findings have landed in code and tests:

update_game now uses PeerCommand::FetchLatestFromPeers to skip local manifest serving and fetch fresh peer metadata. Covered by update_fetch_emits_fresh_manifest_from_latest_peer and update_request_skips_local_manifest_even_when_download_exists.
Download-to-install handoff no longer relies on OperationGuard::Drop for ordered state transitions. Covered by download_handoff_waits_for_readers_and_auto_installs and the liveness cancellation tests.
Library index reads and writes are serialized by LIBRARY_INDEX_LOCK. Covered by concurrent_rescans_preserve_both_index_updates.

Manual install/update/uninstall smoke testing is still a useful release check, but there are no known blocking findings left in this file.

2.6 KiB Raw Blame History