feat(sync): make the WebRTC room password reachable from the UI - #777
Conversation
Plan 134 added `password` to `joinRoom`'s options, but the only production caller passed none and no UI existed to set one — so the AES-GCM encryption y-webrtc derives from that password could never actually be switched on. Adds a masked, labelled room-password field to the disconnected panel, threaded through SyncStatusCard to SyncView and forwarded on join. A blank field calls `joinRoom(id)` with a single argument so the unencrypted path stays byte-for-byte unchanged. The secret is held in component state only: never logged, never added to the sync event history, never persisted, and cleared on join and on leave. 🧪 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Blocked merge diagnosis — blocked |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 9 |
| Duplication | 1 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
GitNexus Review · PR #777No issues found in 5 changed files. 🤖 Agent context — 14 symbols · 5 filesSummaryThis appears to be a concentrated React UI change for exposing WebRTC room password access within the studio sync views. The graph marks the overall blast radius as critical, with impact centered on sync view behavior and its supporting UI components. 🔴 CRITICAL blast radius. A frontend change to the studio sync UI in Most of the change is concentrated in The impact lands across the human-named
What changedSymbol Changes (14)
Changed Files (5)
What it affectsArchitecture Impact
Blast Radius
Direct dependents (d1)
What to checkFile Risk (1)
🔀 Structural changes ·
|
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Sep 11, 2026 11:50a.m. | Review ↗ |
| Python | |
Sep 11, 2026 11:50a.m. | Review ↗ |
| Shell | |
Sep 11, 2026 11:50a.m. | Review ↗ |
| SQL | |
Sep 11, 2026 11:50a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
…-0067
DeepSource's JavaScript analyzer failed this branch on JS-0067: a top-level
`export function` is read as a global-scope declaration. The `.deepsource.toml`
skip does not suppress findings on new code (LESSON-031), so the fix is at code
level, using the repo's `const fn = () => {}` convention. No behaviour change —
the export stays a named export consumed identically.
🧪 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
![deepsource-io[bot]](https://avatars.githubusercontent.com/in/16372?s=60&v=4){kind=small}
🔄 What's new in this push (
|
Two further DeepSource findings on the file this branch touches: - JS-R1005: SyncView measured a cyclomatic complexity of 8. The provider-event and LAN-discovery effects move into `useSyncProviderEvents` and `useRoomDiscovery`; both accept only stable setters/useCallback, so the subscriptions are still established exactly once and behaviour is unchanged. - JS-0116: `handleJoin` was `async` with no `await` anywhere. It is now a plain callback passed straight to `onJoin`. Fixed at code level rather than with suppression comments. 🧪 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
🔄 What's new in this push (
|
DeepSource still sized SyncView at complexity 8 after the effect extraction,
because the driver is JSX branching rather than the effects. The header, conflict
card, sync-history card, and online-users card move into named components, and
the per-event colour chain becomes an exhaustive switch helper. Only the room-id
fallback and the error-message ternary remain in the component body.
Also braces the `stopDiscovery` cleanup body: DeepSource JS-0045 reads the
one-line `() => { stopDiscovery() }` form as returning a value.
🧪 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
🔄 What's new in this push (
|
… default DeepSource JS-0045 objected to the wrapped cleanup arrow, so the hook now returns `stopDiscovery` itself — it already takes no arguments and returns void, so it is a valid cleanup as-is. JS-0047/JS-0045 also wanted the exhaustive event-colour switch to carry a default arm. 🧪 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
🔄 What's new in this push (
|
DeepSource JS-0045 is a consistent-return rule: the early bare `return` mixed with a cleanup return made the effect callback's return paths inconsistent. The guarded branch form was previously accepted and keeps the behaviour identical. 🧪 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
🔄 What's new in this push (
|
1 participant
Makes the encrypted-sync control from Plan 134 actually usable. Tracked as Plan 136 (
plans/136-sync-room-password.md).Problem
Plan 134 added
password?: stringtojoinRoom's options, whichy-webrtcuses to derive an AES-GCM key for the signaling and data channels. That landed at the API boundary only — the sole production caller invokedjoinRoom(id)with no options, and no UI existed to set a password. The encryption could never be switched on. (Reported as a P2 finding during the pre-merge review of #776.)Change
SyncStatusCardtoSyncViewand forwarded on join.joinRoom(id)with a single argument, so the unencrypted path stays byte-for-byte unchanged.y-webrtccannot report it as an auth error.Tests
7 new tests across
sync-view.test.tsxandsync-helpers.test.tsx: masked rendering, change forwarding,aria-describedbylinkage, Enter-to-join, the password reachingjoinRoom, the single-argument unencrypted path, and an assertion that the password never appears in the sync history.Review note: passing
undefinedas an explicit second argument broke 11 pre-existing assertions insync-view-coverage.test.tsx(toHaveBeenCalledWith('room-123')does not match('room-123', undefined)). Rather than loosen those assertions, the implementation branches to a single-argument call when no password is set.Verification
pnpm run lint,pnpm run typecheck,pnpm run buildall clean.pnpm run test: 159 files, 2374 passed, 1 pre-existing documented skip.sync-helpers.tsx424 LOC /sync-view.tsx298 LOC — both under the 500 limit.🤖 Generated with Codebuff
📝 Summary by GitNexus
Summary
This appears to be a concentrated React UI change for exposing WebRTC room password access within the studio sync views. The graph marks the overall blast radius as critical, with impact centered on sync view behavior and its supporting UI components.
🔴 CRITICAL blast radius. A frontend change to the studio sync UI in
src/components/studio/views/sync-view.tsxandsrc/components/studio/views/sync-helpers.tsx, reaching two direct dependents.Most of the change is concentrated in
src/components/studio/views/sync-view.tsx, includingSyncView,entities,claims,addEvent, and the sync lifecycle handlershandlePeers,handleJoin,handleLeave,handleResync,handleConflictResolve,handleConflictDismiss, andhandleQrScan. Review the interactions among these handlers first, since this file carries 11 changed symbols and sits on 18 affected flows.The impact lands across the human-named
ViewsandSyncmodules. Supporting UI changes insrc/components/studio/views/sync-helpers.tsxaffectDisconnectedPanelandSyncStatusCard, with corresponding coverage updates insrc/components/studio/views/sync-helpers.test.tsxandsrc/components/studio/views/sync-view.test.tsx.Added by GitNexus for PR #777. Edit freely — this block is replaced on the next review, everything above it is left untouched.