fix(syncdeck): recover incomplete waiting room identity - #300
Conversation
|
Warning Review limit reached
Next review available in: 48 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughSession participant context now requires both participant name and ID for routing checks. Waiting-room handoffs can persist server-issued participant context through dedicated storage even when token storage is unavailable, while token and fallback persistence remain storage-gated. Playwright coverage now validates the waiting-room state for anonymous participants. ChangesSession context flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Student
participant WaitingRoomHandoff
participant Server
participant ParticipantContextStorage
participant SessionRouter
Student->>WaitingRoomHandoff: request session handoff
WaitingRoomHandoff->>Server: request participant handoff
Server-->>WaitingRoomHandoff: return participantId and values
WaitingRoomHandoff->>ParticipantContextStorage: persist participant context
SessionRouter->>ParticipantContextStorage: read stored context
ParticipantContextStorage-->>SessionRouter: return name and ID
SessionRouter->>Student: render routing state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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: 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/components/common/sessionParticipantContext.ts`:
- Around line 8-10: Update hasCompleteSessionParticipantContext to use optional
chaining when accessing both studentName and studentId, preserving the existing
non-null validation and boolean result for null or incomplete contexts.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 5ba0ff3e-879e-40c9-918d-899d8cd31c32
📒 Files selected for processing (5)
client/src/components/common/SessionRouter.tsxclient/src/components/common/sessionParticipantContext.test.tsclient/src/components/common/sessionParticipantContext.tsclient/src/components/common/waitingRoomHandoffUtils.test.tsclient/src/components/common/waitingRoomHandoffUtils.ts
There was a problem hiding this comment.
Pull request overview
This PR tightens SyncDeck waiting-room identity/session continuity by treating participant identity as valid only when both name and participant ID are present, while also making server-backed handoff persistence resilient to missing browser storage (e.g., blocked/disabled sessionStorage).
Changes:
- Persist session participant context using server-returned
participantIdeven whensessionStorageisn’t available, and avoid token/value persistence attempts when storage is missing. - Update session routing to consider stored participant context “present” only when both
studentNameandstudentIdexist (prevents incomplete identities from being treated as accepted). - Add/adjust unit + Playwright coverage for incomplete identity and no-sessionStorage handoff scenarios.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| client/src/components/common/waitingRoomHandoffUtils.ts | Allows server-backed handoff to run without sessionStorage, persists participant context when possible, and skips token/value persistence when storage is unavailable. |
| client/src/components/common/waitingRoomHandoffUtils.test.ts | Adds test ensuring server-issued participant ID is persisted even when storage is null. |
| client/src/components/common/SessionRouter.tsx | Uses hasCompleteSessionParticipantContext so partial stored identities no longer count as “stored participant context”. |
| client/src/components/common/sessionParticipantContext.ts | Adds hasCompleteSessionParticipantContext helper (requires both name + ID). |
| client/src/components/common/sessionParticipantContext.test.ts | Adds tests for completeness checks. |
| activities/postboard/playwright/flow.spec.ts | Updates Playwright flow to expect waiting room when student identity is incomplete (missing participant ID). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/components/common/waitingRoomHandoffUtils.ts (1)
39-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove redundant
?? storagefallback.The
participantContextStorageparameter already defaults tostoragevia parameter destructuring (participantContextStorage = storage). Using?? storagehere overrides an explicitly passednull, preventing the caller from intentionally disabling participant context persistence when token storage is available.♻️ Proposed fix
- const resolvedParticipantContextStorage = participantContextStorage ?? storage + const resolvedParticipantContextStorage = participantContextStorage🤖 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/components/common/waitingRoomHandoffUtils.ts` at line 39, Remove the redundant nullish fallback from the resolved participant context storage assignment in the waiting-room handoff utility, using the already-defaulted participantContextStorage value directly so an explicitly passed null remains disabled.
🤖 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.
Outside diff comments:
In `@client/src/components/common/waitingRoomHandoffUtils.ts`:
- Line 39: Remove the redundant nullish fallback from the resolved participant
context storage assignment in the waiting-room handoff utility, using the
already-defaulted participantContextStorage value directly so an explicitly
passed null remains disabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f4d92acd-30a3-46af-bc60-2832fbad9d96
📒 Files selected for processing (4)
activities/postboard/playwright/flow.spec.tsclient/src/components/common/sessionParticipantContext.tsclient/src/components/common/waitingRoomHandoffUtils.test.tsclient/src/components/common/waitingRoomHandoffUtils.ts
Summary by CodeRabbit
Bug Fixes
Tests
null.