feat(draft): add responsive draft workspace - #7985
Conversation
|
Important Review skippedToo many files! This PR contains 114 files, which is 14 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (114)
You can disable this status message by setting the 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 |
matthewevans
left a comment
There was a problem hiding this comment.
Changes requested — protected build/dependency surface.
🔴 Blocker
client/package.json:18, client/package.json:78, and client/package.json:90 change the package-script/dependency surface by adding test:browser:pack-layout, @vitest/browser-playwright, and playwright. Repository review policy classifies client/package.json as a hard-stop path, so this PR cannot be implementation-reviewed, approved, or enqueued while those changes are present without explicit maintainer review of that surface.
Please split or remove the package-manifest/script changes from this PR, or obtain explicit maintainer approval for that protected surface before resubmitting the current head. The responsive draft implementation itself was intentionally not reviewed in this disposition.
Recommendation: resolve the protected client/package.json change first, then request a fresh full review.
Resolve the HoverCardPreview conflict by preserving the responsive-workspace effective mode and the merged hover-preview docking fix. Co-authored-by: traemyn <traemyn@gmail.com>
|
Maintainer port complete — fresh review pending. I merged current The earlier requested-changes review was attached to |
matthewevans
left a comment
There was a problem hiding this comment.
Changes requested — bound the untrusted workspace snapshot.
🔴 Blocker
[MED] The new per-seat workspace transport accepts an unbounded placements map before it knows the authoritative pool. Evidence: client/src/components/draft/workspace/types.ts:67-106 iterates and clones every placement while only virtualBasics is capped at lines 109-114; client/src/adapter/p2p-draft-host.ts:964-977 runs that validation for each guest-owned draft_workspace_update before reconciliation; and client/src/network/draftPeerSession.ts:49-63 / client/src/network/draftProtocol.ts:868-881 impose no decoded-frame bound. The current test intentionally accepts more than the 1,000-entry cap at client/src/services/__tests__/draftPersistence.test.ts:384-397.
Why it matters: an authenticated peer can send an arbitrarily large valid-looking snapshot, forcing the host to allocate/clone it and then serialize a persisted session before reconciliation prunes the unknown instance IDs. This turns a display-only update into a host memory/CPU and persistence denial-of-service path.
Suggested fix: apply a bounded, pool-aware admission check at the host boundary (or a conservative schema cap) before cloning every placement, then add a transport-level regression test that rejects an oversized unknown-placement payload without persisting it. Keep the virtual-basic bound and make the placement bound explicit rather than relying on later reconciliation.
✅ Clean
The previous client/package.json hard-stop is absent from c0e715338edb989821d1496d271964ad7bb82c0e; current-head CI is green. The #7974 hover-preview conflict resolution itself preserves both the workspace effective mode and the dock-position API.
Recommendation: request changes for bounded workspace-update admission, then re-run the full current-head review.
Summary
Adds the responsive draft workspace, including persistent deck/sideboard placement, workspace drag interactions, responsive pack and pool UI, and compatible single- and multi-card draft picks. It also aligns multiplayer draft protocol and recovery behavior with the current engine contracts.
Files changed
.claude/wf/tablet-deck-header-sideboard-icon-phase-fit.mdclient/src/adapter/__tests__/draft-adapter.test.tsclient/src/adapter/__tests__/draftPodAdapter.cube.test.tsclient/src/adapter/__tests__/draftPodAdapter.test.tsclient/src/adapter/__tests__/p2pDraftHostBo3.test.tsclient/src/adapter/__tests__/p2pDraftWorkspacePersistence.test.tsclient/src/adapter/__tests__/server-draft-adapter.test.tsclient/src/adapter/draft-adapter.tsclient/src/adapter/draftPodGuestAdapter.tsclient/src/adapter/draftPodHostAdapter.tsclient/src/adapter/p2p-draft-guest.tsclient/src/adapter/p2p-draft-host.tsclient/src/components/card/HoverCardPreview.tsxclient/src/components/card/__tests__/HoverCardPreview.test.tsxclient/src/components/chrome/AppShell.tsxclient/src/components/chrome/ChromeControls.tsxclient/src/components/chrome/ShellContext.tsxclient/src/components/chrome/__tests__/AppShell.responsiveDraft.test.tsxclient/src/components/chrome/__tests__/ChromeControls.responsiveDraft.test.tsxclient/src/components/draft/DeckStatistics.tsxclient/src/components/draft/DraftCardFace.tsxclient/src/components/draft/DraftProgress.tsxclient/src/components/draft/DraftSteps.tsxclient/src/components/draft/HostControls.tsxclient/src/components/draft/LimitedDeckBuilder.tsxclient/src/components/draft/ManaCurve.tsxclient/src/components/draft/PackDisplay.tsxclient/src/components/draft/PodIcon.tsxclient/src/components/draft/PoolPanel.tsxclient/src/components/draft/SeatStatusRing.tsxclient/src/components/draft/__tests__/CardPoolBoard.test.tsxclient/src/components/draft/__tests__/DraftProgress.test.tsxclient/src/components/draft/__tests__/DraftSteps.test.tsxclient/src/components/draft/__tests__/DraftWorkspace.test.tsxclient/src/components/draft/__tests__/LimitedDeckBuilder.test.tsxclient/src/components/draft/__tests__/PackDisplay.pod.test.tsxclient/src/components/draft/__tests__/PackDisplay.workspace.test.tsxclient/src/components/draft/__tests__/PoolPanel.test.tsxclient/src/components/draft/__tests__/PoolPanel.workspace.test.tsxclient/src/components/draft/__tests__/SealedPackOpening.test.tsxclient/src/components/draft/__tests__/SeatStatusRing.test.tsxclient/src/components/draft/__tests__/VisualPackDraftSurfaces.test.tsxclient/src/components/draft/__tests__/useDraftWorkspaceDrag.test.tsxclient/src/components/draft/__tests__/workspaceMigration.test.tsclient/src/components/draft/__tests__/workspacePlacement.test.tsclient/src/components/draft/__tests__/workspacePreferences.test.tsclient/src/components/draft/__tests__/workspaceProjection.test.tsclient/src/components/draft/workspace/CardPoolBoard.tsxclient/src/components/draft/workspace/CardPoolColumn.tsxclient/src/components/draft/workspace/CompactSideboard.tsxclient/src/components/draft/workspace/DraftWorkspace.tsxclient/src/components/draft/workspace/DraftWorkspaceToolbar.tsxclient/src/components/draft/workspace/WorkspaceCard.tsxclient/src/components/draft/workspace/types.tsclient/src/components/draft/workspace/useDraftWorkspaceDrag.tsclient/src/components/draft/workspace/workspaceMigration.tsclient/src/components/draft/workspace/workspacePlacement.tsclient/src/components/draft/workspace/workspacePreferences.tsclient/src/components/draft/workspace/workspaceProjection.tsclient/src/components/menu/MenuShell.tsxclient/src/components/menu/PopoverMenu.tsxclient/src/components/menu/__tests__/MenuShell.test.tsxclient/src/components/menu/__tests__/PopoverMenu.test.tsxclient/src/components/settings/PreferencesModal.tsxclient/src/components/settings/__tests__/PreferencesModal.cardPreview.test.tsxclient/src/constants/__tests__/storage.test.tsclient/src/constants/storage.tsclient/src/game/__tests__/wasmLegalActions.integration.test.tsclient/src/hooks/__tests__/useShiftHeld.test.tsxclient/src/hooks/useShiftHeld.tsclient/src/i18n/__tests__/localeParity.test.tsclient/src/i18n/locales/de/draft.jsonclient/src/i18n/locales/de/settings.jsonclient/src/i18n/locales/en/draft.jsonclient/src/i18n/locales/en/settings.jsonclient/src/i18n/locales/es/draft.jsonclient/src/i18n/locales/es/settings.jsonclient/src/i18n/locales/fr/draft.jsonclient/src/i18n/locales/fr/settings.jsonclient/src/i18n/locales/it/draft.jsonclient/src/i18n/locales/it/settings.jsonclient/src/i18n/locales/pl/draft.jsonclient/src/i18n/locales/pl/settings.jsonclient/src/i18n/locales/pt/draft.jsonclient/src/i18n/locales/pt/settings.jsonclient/src/network/__tests__/draftProtocol.test.tsclient/src/network/draftProtocol.tsclient/src/pages/DraftLandingPage.tsxclient/src/pages/DraftPage.tsxclient/src/pages/DraftPodPage.tsxclient/src/pages/__tests__/DraftPage.pick.integration.test.tsxclient/src/pages/__tests__/DraftPage.workspace.test.tsxclient/src/pages/__tests__/DraftPodPage.betweenGames.test.tsxclient/src/pages/__tests__/DraftPodPage.modeEntry.test.tsxclient/src/pages/__tests__/DraftPodPage.podComplete.test.tsxclient/src/pages/__tests__/DraftPodPage.podError.test.tsxclient/src/pages/__tests__/DraftPodPage.workspace.test.tsxclient/src/services/__tests__/backup.test.tsclient/src/services/__tests__/draftPersistence.test.tsclient/src/services/__tests__/quickDraftPersistence.test.tsclient/src/services/__tests__/scryfall.test.tsclient/src/services/backup.tsclient/src/services/draftPersistence.tsclient/src/services/quickDraftPersistence.tsclient/src/services/scryfall.tsclient/src/stores/__tests__/draftStore.workspace.test.tsclient/src/stores/__tests__/multiplayerDraftStore.bo3.test.tsclient/src/stores/__tests__/multiplayerDraftStore.test.tsclient/src/stores/__tests__/preferencesStore.test.tsclient/src/stores/draftStore.tsclient/src/stores/multiplayerDraftStore.tsclient/src/stores/preferencesStore.tsclient/src/wasm/draft_wasm.d.tsclient/src/wasm/engine_wasm.d.tscrates/draft-core/src/view.rscrates/server-core/src/protocol.rsTrack
Developer
LLM
Model: GPT-5 (Codex; canonical id not exposed)
Tier: Frontier
Thinking: high
Implementation method (required)
Method: /engine-implementer
CR 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.
CI=true corepack pnpm@10.20.0 exec vitest run --silent --reporter=dot— 390 files passed, 2 skipped; 4,030 tests passed, 12 todo.CI=true corepack pnpm@10.20.0 run test:integration— 22 tests passed.CI=true corepack pnpm@10.20.0 run test:browser:pack-layout— 39 tests passed.CI=true corepack pnpm@10.20.0 run type-check— passed.CI=true corepack pnpm@10.20.0 run build— passed.CI=true PATH=/tmp/phase-pnpm-10.20:$PATH .githooks/pre-push— passed; runs cargo fmt, clippy, card-data validation, parser/AI tests, lint, and type-check. ESLint completed with 0 errors and 43 existing warnings.git diff --check upstream/main...HEAD— passed.Gate A
Gate A PASS head=581d5ba7caa39eb4480c4c7edb52bbe3b143ac0f base=6884506bfad8bc5bffa660dea10d0bcf2fd930c3
Anchored on
Final review-impl
Final review-impl PASS head=581d5ba7caa39eb4480c4c7edb52bbe3b143ac0f
Claimed parse impact
None.
Scope Expansion
None.
Validation Failures
The isolated pre-push hook reached card-data validation but could not locate its untracked
data/mtgjson/AtomicCards.jsoninput. Candidate-scoped frontend, integration, type, lint, parser, AI, and review checks passed.CI Failures
None.
PR handoff
Pipeline-reviewed head: 581d5ba
Current branch head: 581d5ba
Pipeline status: current
Current-head review: clean at 581d5ba