fix: wire P2P draft server backups to the shared draft-code contract - #6818
fix: wire P2P draft server backups to the shared draft-code contract#6818RealDiligent wants to merge 3 commits into
Conversation
github-actions
Bot
added
the
needs-maintainer
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe client now generates server-compatible P2P draft codes, resolves and wires the production backup endpoint, enables immediate backup eligibility, reports non-OK responses, and cleans up backups. Server validation is shared and integration-tested through POST, GET, and DELETE flows. ChangesP2P draft backup
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant DraftPodHostAdapter
participant P2PDraftHost
participant PhaseServer
DraftPodHostAdapter->>P2PDraftHost: initialize with resolved backup endpoint
P2PDraftHost->>P2PDraftHost: startDraft generates six-character draft code
P2PDraftHost->>PhaseServer: POST /p2p-draft-backup with snapshot
PhaseServer-->>P2PDraftHost: HTTP response status
P2PDraftHost->>PhaseServer: DELETE backup on cleanup
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Production omitted backupEndpoint and sent draft-xxxxxxxx IDs that phase-server rejects, so D-08 backups never landed. Align client generation with is_valid_draft_code, pass the lobby HTTP origin, and surface non-2xx upload failures. Closes phase-rs#6728 Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/config/multiplayerServer.ts`:
- Around line 35-36: Update the WebSocket URL predicate in the multiplayer
server URL converter to recognize both `/ws` and `/ws/` suffixes, while
preserving the existing pathname normalization. Extend the converter tests to
cover a URL ending in `/ws/` and verify it produces the root backup path.
🪄 Autofix (Beta)
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: 7e2af08c-605e-45fc-8105-d37824640aeb
📒 Files selected for processing (8)
client/src/adapter/__tests__/p2pDraftHostBackup.test.tsclient/src/adapter/draftPodHostAdapter.tsclient/src/adapter/p2p-draft-host.tsclient/src/config/multiplayerServer.tscrates/phase-server/src/admin.rscrates/phase-server/src/main.rscrates/server-core/src/draft_session.rscrates/server-core/src/lib.rs
| if (url.pathname === "/ws" || url.pathname.endsWith("/ws")) { | ||
| url.pathname = url.pathname.replace(/\/ws\/?$/, "") || "/"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle WebSocket URLs ending in /ws/.
For wss://host/ws/, pathname is /ws/, so the condition is false and backups are sent to /ws/p2p-draft-backup instead of /p2p-draft-backup. Match the optional trailing slash in the predicate and add that case to the converter test.
Proposed fix
- if (url.pathname === "/ws" || url.pathname.endsWith("/ws")) {
+ if (/\/ws\/?$/.test(url.pathname)) {
url.pathname = url.pathname.replace(/\/ws\/?$/, "") || "/";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (url.pathname === "/ws" || url.pathname.endsWith("/ws")) { | |
| url.pathname = url.pathname.replace(/\/ws\/?$/, "") || "/"; | |
| if (/\/ws\/?$/.test(url.pathname)) { | |
| url.pathname = url.pathname.replace(/\/ws\/?$/, "") || "/"; |
🤖 Prompt for 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.
In `@client/src/config/multiplayerServer.ts` around lines 35 - 36, Update the
WebSocket URL predicate in the multiplayer server URL converter to recognize
both `/ws` and `/ws/` suffixes, while preserving the existing pathname
normalization. Extend the converter tests to cover a URL ending in `/ws/` and
verify it produces the root backup path.
Source: Path instructions
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Closed without implementation-diff review. This PR was opened on 2026-07-30, after the 2026-07-24 applicability cutoff in The version of that policy live before this PR was opened (commit This is a policy gate, not a technical assessment of the change or of the contributor. Please open a fresh PR produced with an accepted Frontier-tier model and the required contribution evidence. |
3 participants
Summary
Fixes the inert D-08 P2P draft server-backup path (#6728): production now supplies a
backupEndpoint, client draft codes match the shared 6-char uppercase contract used byphase-server, and non-2xx upload/cleanup responses are logged instead of treated as success. Also seeds the first backup upload on draft start so the host claims the row immediately.Closes #6728
Related: completes the production wiring gap called out on #5370.
Files changed
client/src/adapter/draftPodHostAdapter.tsclient/src/adapter/p2p-draft-host.tsclient/src/adapter/__tests__/p2pDraftHostBackup.test.tsclient/src/config/multiplayerServer.tscrates/server-core/src/draft_session.rscrates/server-core/src/lib.rscrates/phase-server/src/admin.rscrates/phase-server/src/main.rsTrack
Non-developer
LLM
Model: Composer
Tier: Standard
Thinking: high
Implementation method (required)
Method: not-applicable — multiplayer P2P backup HTTP contract / client wiring (#6728), not
crates/engineparser or rules workCR references
None.
Verification
Required checks ran clean, or the exact CI-owned alternative is stated below.
Gate A output below is for the current committed head.
Final review-impl below is clean for the current committed head.
Both anchors cite existing analogous code at the same seam.
npx vitest run src/adapter/__tests__/p2pDraftHostBackup.test.ts— 4 passednpx tsc --noEmit -p tsconfig.json— cleanGitHub Actions CI — pending on this PR (Rust unit/integration tests for
server-core+phase-serverbackup handlers; no localcargoon this Windows agent)Gate A
Gate A N/A — no
crates/engineparser changes; combinator script not applicable to this multiplayer backup contract fix.head=103f4373c1b3fddefa375c68141ffe27183c47c4 base=daf5c1df7b2f372042b1a093558312c1dd150bb4
Anchored on
crates/server-core/src/draft_session.rs:674— existinggenerate_draft_code()6-char uppercase alphabet (shared contract source)crates/phase-server/src/main.rs:8203— existingp2p_backup_delete_testsHTTP harness for GET/DELETE ownership checks (extended for POST round-trip + legacy-code rejection)client/src/adapter/p2p-draft-host.ts:174— existingbackupEndpointupload gate / D-08 persist path (wired from production constructor)Final review-impl
Final review-impl PASS head=103f4373c1b3fddefa375c68141ffe27183c47c4
Self-review: production constructor now passes endpoint; draft code generation matches
is_valid_draft_code; HTTP status checked; server validator centralized; regression tests cover client shape + server POST reject ofdraft-xxxxxxxx.Claimed parse impact
None.
Scope Expansion
None beyond #6728 acceptance criteria (endpoint wiring, shared ID contract, non-2xx diagnostics, HTTP integration coverage). Immediate first-upload seed is included so a fixed path actually claims the backup row at draft start.
Validation Failures
None.
CI Failures
None.
Summary by CodeRabbit
New Features
Bug Fixes
Tests