fix(native): harden resolve all transport - #7217
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughResolve All now uses requester-correlated rejection frames, server-owned AI capability metadata, bounded engine-log delivery, and protocol version 29. Client dispatch and WebSocket handling preserve pending requests until the matching server response arrives. ChangesResolve All correlation and server-owned AI
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Player
participant ActionButton
participant WebSocketAdapter
participant phase-server
Player->>ActionButton: Start Resolve All
ActionButton->>WebSocketAdapter: Send empty AI-seat list
WebSocketAdapter->>phase-server: Resolve All request with request_id
phase-server-->>WebSocketAdapter: Final state with bounded logs
phase-server-->>WebSocketAdapter: ResolveAllRejected(request_id, reason) on failure
WebSocketAdapter-->>Player: Resolve All result or matching rejection
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/adapter/ws-adapter.ts`:
- Around line 1529-1534: Update dispatchResolveAll in
client/src/game/dispatch.ts to catch Resolve All rejections and route the error
through the existing game error presentation path, preserving the
server-provided rejection reason. Ensure callers such as ActionButton do not
need to await or catch the promise.
In `@crates/phase-server/src/main.rs`:
- Around line 4270-4289: Update the post-transition handling around
guard_state_snapshot_broadcast and prepare_full_terminal so failures after the
engine accepts the batch never send ResolveAllRejected. Reconcile the requester
using the committed state snapshot when delivery, persistence, or terminal
preparation fails, or make the transition and preparation atomic with rollback;
preserve ResolveAllRejected only for failures occurring before the state
transition commits.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 98a32741-dfbf-42f0-8424-cb352c17a040
📒 Files selected for processing (11)
client/src/adapter/__tests__/ws-adapter.test.tsclient/src/adapter/types.tsclient/src/adapter/ws-adapter.tsclient/src/components/board/ActionButton.tsxclient/src/components/board/__tests__/ActionButton.test.tsxclient/src/game/__tests__/dispatchResolveAll.test.tsclient/src/game/dispatch.tscrates/lobby-broker/src/protocol.rscrates/phase-server/src/main.rscrates/server-core/src/protocol.rsscripts/check-protocol-version.mjs
Follow-up to #7215 addressing CodeRabbit review findings.
Validation: full pre-push suite (Rust lint/tests, card-data gates, frontend lint, protocol check, and TypeScript type-check).
Summary by CodeRabbit
New Features
Bug Fixes