Track the exact regular files owned by each completed and in-flight peer download instead of sweeping every non-reserved path after cancellation. Bind the record to the canonical games directory, publish pending ownership before payload mutation, and use the final version.ini rename as the recovery commit point. Make replacement, cancellation, and startup recovery preserve unknown files and install state while removing stale or partial downloader-owned bytes. Add a new-format baseline so legacy discarded sentinels cannot make partially modified payloads ready, sync payload and journal state in transaction order, and serialize startup recovery against operation admission. Document ambiguous legacy target adoption, portable alias transitions, and the other ownership tradeoffs in the refactor decision log. Test Plan: - `just clippy` -- passed - `just test` -- passed (182 peer-core tests plus the full workspace) - `just fmt` -- Rust, TOML, and Prettier formatting completed; the command then stopped on 40 pre-existing rumdl findings in unrelated Markdown content - `git diff --cached --check` -- passed
44 lines
2.1 KiB
Markdown
44 lines
2.1 KiB
Markdown
# Findings
|
|
|
|
## Open
|
|
|
|
### `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 four findings have landed in code and tests:
|
|
|
|
- Download ownership is now journaled before payload mutation. Cancellation and
|
|
startup recovery remove only exact downloader-owned paths and preserve unknown
|
|
root files, instead of leaving crashed partial archives or broadly deleting
|
|
the game root.
|
|
- `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.
|