ddidderr
be00a7a298
Peer A failed to learn Peer B's games. The handshake only carried
library_rev/library_digest metadata, and the post-handshake sync path
compared those revisions against per-peer revision numbers that were
never advanced via this code path, so the games map for the remote
peer stayed empty and the UI never showed them.
The fix is to put the authoritative library data into the handshake
itself. Hello and HelloAck now carry a LibrarySnapshot directly, and
both perform_handshake_with_peer (outbound) and accept_inbound_hello
(inbound) apply that snapshot to the peer DB before emitting the UI
events. The initial peer-game-list event is now driven by the
handshake rather than by a follow-up LibrarySummary/LibrarySnapshot
roundtrip.
Bumps PROTOCOL_VERSION to 4 because the wire layout of Hello/HelloAck
changed. Per CLAUDE.md's protocol policy there is no compatibility
shim; older peers will fail the version check and be ignored.
Cleanups that fall out of the new design:
- The Hello / HelloAck library_rev and library_digest fields were
duplicated by the embedded LibrarySnapshot (which carries its own
library_rev, and whose digest is recomputed on apply). Collapsed
both messages to just `library: LibrarySnapshot` to remove the
foot-gun where the two could diverge.
- Request::LibrarySummary and Request::LibrarySnapshot are now dead
on the sender side and were removed along with their stream.rs
handlers and the LibrarySummary struct. LibraryDelta stays — it
is still sent from handlers.rs when the local library changes.
- record_remote_library previously called update_peer_library and
then apply_library_snapshot, which immediately overwrote the
rev/digest just written. Added update_peer_features and rewired
the call site so each peer-DB field is written exactly once.
update_peer_library is retained because discovery.rs still uses
it for the mDNS TXT-record path, where no snapshot is available.
- Removed the now-unused LibraryUpdate enum, select_library_update,
send_local_library_summary, send_local_library_update_if_needed,
LocalLibraryState::delta_since, build_library_summary,
send_library_summary, and send_library_snapshot.
Behavior change visible to users: when two peers come up on the LAN
they now see each other's full game lists immediately after the
handshake instead of waiting for a follow-up sync that, in the broken
case, never made the games visible at all.
Test Plan
- just clippy (clean for the touched crates)
- just test (workspace: all suites pass, including the two new
handshake tests: outbound_hello_carries_local_library_snapshot
and inbound_hello_applies_remote_library_snapshot, the latter
asserting PeerDiscovered + PeerCountUpdated + ListGames events
fire with the remote game visible)
- Manual: start `just peer-cli-alpha` and `just peer-cli-bravo` in
separate terminals; confirm each peer's game list shows the
other's library entries after discovery completes, without
requiring any additional command.
Refs
- FINDINGS.md: triage note that Claude's review surfaced only
in-scope cleanups (dead variants, duplicated header fields,
redundant DB writes, stale test fixture), all addressed here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
26 lines
1.1 KiB
Markdown
26 lines
1.1 KiB
Markdown
# Findings
|
|
|
|
No open pre-merge findings are currently tracked here.
|
|
|
|
## 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.
|