5.0 KiB
Call to Play review findings evaluation
The implementation is fundamentally sound, but I would make four focused follow-ups before calling it finished. Two address real behavioral gaps; two complete the new lifecycle cleanly.
Worth fixing before finish
1. Propagate rootless terminal tombstones
This is the most important finding.
After 15 minutes, a peer retains only the Start or Cancel event. A fresh
peer receiving that tombstone during handshake currently rejects it as missing
its Create root, and the handshake path merely logs the missing root.
That means the fresh peer has no protection against a stale peer later resurrecting the finished call. It only self-heals if a tombstone-holder is still online afterward.
I would:
- Add an explicit handshake merge policy that permits rootless
StartandCancelevents as non-visible tombstones. - Keep local and live orphan actions returning
NeedHistory. - Never expose a rootless tombstone as a nomination in the UI.
- Verify that a later stale complete history is classified obsolete.
- Add store and handshake tests for this sequence.
This restores the intended "tombstone prevents resurrection for the rest of the session" invariant. I would fix the semantics, not merely downgrade the repeated warning.
2. Distinguish missing game directory from peer startup
Kimi is correct here. The peer is only started after a valid directory reaches
update_game_directory. Without one, "still connecting -- try again" never
becomes true.
I would:
- Model directory readiness as
checking | missing | ready, rather than passing only a boolean that conflates hydration with a known missing directory. - Pass that prerequisite state into
useCallToPlay. - Show folder guidance only for the confirmed
missingstate. - Preserve the current connecting message for
checkingorready-but-peer-starting. - Test both states and the transition after a valid directory is selected.
That finishes the original finding's full intent without returning to misleading folder advice during normal startup.
3. Add a local Launch action to Running receipts
Fable's UX point is persuasive. Participants currently reach the key moment and see only that the game is running.
I would add a local-only Launch button when:
- The call is
running. - The game is installed and launchable locally.
- No conflicting operation prevents launch.
This would not violate the read-only terminal invariant: launching the local game does not mutate the replicated call. I would use a dedicated play callback rather than the generic primary action, so a button labelled "Launch" cannot unexpectedly initiate an install or update.
4. Make terminal receipts visually static and correct the spec
The terminal receipt details are small but real:
MiniBubblescontinue advancing pending-ready countdowns after a call is Running or Cancelled.- Terminal cards still draw empty seats.
- The specification still says "one row per active call" and documents the old ticker rank, despite terminal rows and expired-first ordering.
I would:
- Freeze participant readiness at
terminalAt, or render terminal participants without countdown tags. - Suppress empty roster slots on terminal cards.
- Change Ready ticker text to name the creator.
- Update the ticker specification to match the actual visible-call and ranking behavior.
Coverage to add alongside those fixes
I would close the acknowledgement-heal test gap, but without introducing a full React test stack or waiting five real minutes:
- Exercise
NeedHistoryfrom a live orphan action. - Apply the full history through the handshake merge path.
- Assert the receiver is revived exactly once.
- Cover the new rootless-tombstone handshake followed by stale-history rejection.
- Rename the IP test so its name honestly describes what it proves; the current helper no longer accepts a source address.
Findings not worth pursuing now
- Backend/frontend semantic duplication: a real maintenance risk, but eliminating it would require an architectural rewrite. Shared conformance fixtures could be a later improvement.
- Full-store cloning per merge: acceptable under the 4,096 unresolved-event cap.
- Substring-matched frontend errors: brittle, but currently pinned by tests; a proper fix requires a typed peer-to-Tauri error contract and is disproportionate for finishing this branch.
- Missing component-test infrastructure: inspection plus pure reducer tests is adequate here; I would not add a UI test framework solely for these controls.
- Time's-up calls in the badge: intentional. These calls remain unresolved and visible, even for non-creators.
- The direct-reply observation: there is nothing to fix; the CLI already uses the reply channel.
Recommended finish scope
The recommended finish scope is:
- Tombstone propagation.
- Accurate directory/startup diagnosis.
- A Running-card Launch action.
- Terminal-receipt polish and specification corrections.
- Targeted replication tests.