ddidderr
95e70ef520
Local operation spinners were driven by begin, finish, and failure event history. If one of those lifecycle events was missed, the Tauri bridge could keep a stale active operation and the React state would keep showing an in-progress spinner until restart. Peer local scan updates now carry an authoritative active-operation snapshot. The peer still suppresses active game roots from peer-facing library deltas, but it emits LocalGamesUpdated to the UI even when no library delta changed so the snapshot can clear stale state after rollback or completion. The Tauri bridge replaces its active-operation map from that snapshot, emits it with the games-list payload, and the React merge uses it to restore download, install, update, and uninstall spinners from current peer state rather than event history alone. This also enables the Tauri lib unit-test target so the reconciliation helper can stay covered by the workspace test recipe. Test Plan: - git diff --check - just fmt - just clippy - just test Follow-up-Plan: FOLLOW_UP_2.md
73 lines
4.5 KiB
Markdown
73 lines
4.5 KiB
Markdown
# Follow-up Plan #2
|
|
|
|
State of FOLLOW_UP_PLAN.md after two implementation rounds. Items here are
|
|
what's still open, plus notes for follow-up items completed after this file was
|
|
created.
|
|
|
|
## Context for next time
|
|
|
|
Branch `p2p-codex-muenchhausen` has uncommitted work that completes the bulk of FOLLOW_UP_PLAN.md (52 tests pass via `just test`). Specifically these items in the original plan are **done** and shouldn't be re-opened: #1, #2, #3, #4, #5, #6, #7, #8, #9, #15, #17, #20, plus parts of #12 (update success, version-ini begin/rollback, multi-eti sorted order, corrupt + mismatched-id intent JSON).
|
|
|
|
The OperationGuard ordering fix in `handlers.rs` is the structurally most important change: install/update/uninstall now drop the guard _before_ the finish event and `refresh_local_game`, so peers see settled state in the next announcement instead of waiting for a scan tick. Tests `*_refreshes_settled_state_after_guard_release` pin this.
|
|
|
|
## Completed after this file was created
|
|
|
|
- Tauri-side `active_operations` reconciliation: `PeerEvent::LocalGamesUpdated`
|
|
now carries an authoritative active-operation snapshot, the Tauri bridge
|
|
replaces its UI operation map from that snapshot, and the React game-list
|
|
merge uses the snapshot to clear stale download/install/update/uninstall
|
|
spinners even when a begin/finish/fail event was missed.
|
|
|
|
## Still open
|
|
|
|
### Tests still missing
|
|
|
|
#### 2. Install-recovery matrix (~10 rows)
|
|
|
|
Only two intent-driven rows are covered: `Updating | no local | .local.installing | .local.backup` (`recovery_restores_backup_for_interrupted_update`) and the active-skip case (`startup_recovery_skips_active_game_roots`). The other ~9 combinations of `(intent_state ∈ {Installing, Updating, Uninstalling}, local present?, .local.installing present?, .local.backup present?)` are unverified.
|
|
|
|
**Approach:** convert `recovery_restores_backup_for_interrupted_update` into a table-driven test. Iterate over the named matrix rows in PLAN.md, set up FS + intent, run `recover_game_root`, assert resulting FS + intent.
|
|
|
|
#### 3. Update rollback on commit-rename failure
|
|
|
|
Only extract-failure is tested (`update_failure_restores_previous_local`). The branch where extract succeeds but the staging→local rename fails is unexercised. Force it by making `local/` un-renamable (e.g. swap in a directory under that name post-extract) or by injecting a rename hook.
|
|
|
|
#### 4. Uninstall delete-failure restore
|
|
|
|
`restore_backup` rollback in `transaction.rs:107-114` is unreached by tests.
|
|
|
|
#### 5. Installed-only → Ready rescan
|
|
|
|
PLAN listed this. No test exercises the transition where a user drops `version.ini` into a `local/`-only game root and the next rescan flips it to Ready.
|
|
|
|
#### 6. Scanner gating dispatch
|
|
|
|
`handle_watch_event` and `RescanGate` / `run_gated_rescan` (`local_monitor.rs:208-287`) have zero tests. Cover:
|
|
|
|
- Event for ID under active operation is dropped.
|
|
- Burst of events collapses to ≤2 rescans for the same ID.
|
|
- Sideload picked up by fallback scan.
|
|
- Non-catalog game produces no library index entry and no `LibraryDelta`.
|
|
|
|
#### 7. Serve gating dispatch
|
|
|
|
`local_download_available` predicate is tested; `handle_get_game_command`, `handle_get_game_file_data`, `handle_get_game_file_chunk` dispatch paths aren't. Small tests against an in-memory `Ctx` covering: non-catalog ID, mid-operation, missing sentinel.
|
|
|
|
### Code hygiene
|
|
|
|
#### 8. Consolidate `TempDir` test helper (getting worse)
|
|
|
|
Reimplemented in **five** test modules now: `install/intent.rs`, `install/transaction.rs`, `local_games.rs`, `download.rs`, `handlers.rs`. Each round of new tests adds another copy. Move to a single `test_support` module (or use the `tempfile` crate, already stable). The longer this waits, the bigger the eventual consolidation diff.
|
|
|
|
#### 9. `availability` field is `String`, not a typed enum
|
|
|
|
`lanspread-db::Game::availability: String` with `AVAILABILITY_*` consts works but allows invalid values. `remote_peer::summary_from_game` already has a defensive fallback for unknown strings. A typed enum + serde would be tighter, but is a wire-format change — coordinate if/when other peers are upgraded.
|
|
|
|
#### 10. `save_library_index` non-atomic
|
|
|
|
`local_games.rs:141-148` writes without temp+rename. Corrupt index → next scan rebuilds, so blast radius is small. Match the intent-log atomic pattern if touching that module.
|
|
|
|
#### 11. Split `download.rs` (1162 lines, growing)
|
|
|
|
Mixes chunk planning, retry orchestration, version-sentinel transaction, in-memory buffer, and the main loop. Future split into `download/{transaction,chunks,orchestrator}.rs`. Pure cosmetic.
|