webtc video integration - #447
Merged
Luluameh merged 1 commit intoJul 29, 2026
Merged
Conversation
|
@Agbeleshe is attempting to deploy a commit to the luluameh's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds API-backed WebRTC signaling, real peer connection and media controls, reconnect handling, picture-in-picture session integration, and Playwright coverage for signaling and seeker/expert connection flows. ChangesWebRTC calling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Seeker
participant Expert
participant VideoCall
participant SignalRoute
participant RTCPeerConnection
Seeker->>VideoCall: initialize as seeker
VideoCall->>RTCPeerConnection: acquire media and create offer
VideoCall->>SignalRoute: POST offer and ICE candidates
Expert->>SignalRoute: GET pending signals
SignalRoute-->>Expert: return seeker signals
Expert->>RTCPeerConnection: apply offer and create answer
Expert->>SignalRoute: POST answer and ICE candidates
Seeker->>SignalRoute: GET pending signals
SignalRoute-->>Seeker: return expert signals
Seeker->>RTCPeerConnection: apply answer
RTCPeerConnection-->>VideoCall: report remote tracks and connection state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/components/session/VideoCall.tsx (2)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLint blockers: unused
MessageCircleimport andanyonsendSignal.Drop
MessageCirclefrom the import list and type the signal payload (unknown, or a discriminated union keyed ontype).Also applies to: 63-63
🤖 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 `@src/components/session/VideoCall.tsx` at line 4, Remove the unused MessageCircle import from VideoCall.tsx. Update sendSignal to avoid any by typing its signal payload as unknown or an appropriate discriminated union keyed by type, while preserving its existing signaling behavior.Source: Linters/SAST tools
57-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winToast timer is never cleared.
Back-to-back toasts share no timer bookkeeping — the first timeout clears the second message early — and a pending timeout fires
setToastMessageafter unmount.♻️ Track the timer in a ref
+ const toastTimerRef = useRef<NodeJS.Timeout | null>(null); const showToast = useCallback((message: string) => { + if (toastTimerRef.current) clearTimeout(toastTimerRef.current); setToastMessage(message); - setTimeout(() => setToastMessage(null), 3000); + toastTimerRef.current = setTimeout(() => setToastMessage(null), 3000); }, []);Clear it in
cleanupConnection(or a dedicated unmount effect).🤖 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 `@src/components/session/VideoCall.tsx` around lines 57 - 60, Update showToast to store the timeout handle in a ref, clearing any existing timer before scheduling the next dismissal so back-to-back toasts remain visible for the full duration. Clear the tracked timer during cleanupConnection or a dedicated unmount effect, and reset the ref so no timeout updates state after unmount.src/app/api/session/[id]/signal/route.ts (2)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace
anyto clear the ESLint failures.Both
SignalMessage.dataand the parsedbodytrip@typescript-eslint/no-explicit-any;unknownworks for both here sincedatais only stored and echoed.♻️ Proposed typing
- data: any; + data: unknown;- let body: any; + let body: unknown;
bodywill then need a narrowing check before destructuring, which pairs well with the validation refactor above.Also applies to: 26-26
🤖 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 `@src/app/api/session/`[id]/signal/route.ts at line 7, Replace the explicit any types in SignalMessage.data and the parsed body with unknown. Before destructuring body, add the necessary narrowing or validation so the existing signal handling remains type-safe while preserving the stored-and-echoed data behavior.Source: Linters/SAST tools
11-17: 🩺 Stability & Availability | 🔵 TrivialIn-memory store won't survive multiple instances or serverless isolates — and never expires.
Acknowledged in the PR description, but worth two operational notes: (1) with more than one server instance or on serverless, seeker and expert can land on different processes and never exchange signals; (2)
sessionSignalsentries only shrink per-session at 200 and are only removed by an explicitDELETE, so abandoned rooms accumulate for the process lifetime. A TTL sweep (or Redis with per-key expiry) would bound growth before this ships beyond single-node dev.Also applies to: 68-70
🤖 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 `@src/app/api/session/`[id]/signal/route.ts around lines 11 - 17, Replace the process-local sessionSignals store used by the signal route with a shared external store such as Redis, using per-session key expiry so seeker and expert requests can communicate across instances and abandoned sessions are automatically removed. If retaining the in-memory implementation, add a bounded TTL cleanup mechanism for entries that are not explicitly deleted.tests/e2e/webrtc.spec.ts (2)
76-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueContexts leak when an assertion fails.
The
toBeVisibleassertions above can throw, skipping bothclose()calls. Create the contexts in abeforeEach/fixture or wrap intry/finally.🤖 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 `@tests/e2e/webrtc.spec.ts` around lines 76 - 77, Ensure the contexts created for the WebRTC test are always closed when assertions fail by moving their setup into a beforeEach/fixture with automatic teardown or wrapping the test flow in try/finally. Preserve the existing contextA and contextB usage and close both contexts during cleanup.
53-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
baseURLand explicit test ids over hardcoded origins and text regexes.The first test uses relative paths, so a
baseURLis already configured; the hardcodedhttp://localhost:3000here will break against any other environment.text=/Connected/iis also broad — the wallet card renders a "Connected" label on this page, so.first()can latch onto unrelated UI. Adata-testidon the connection-state badge inVideoCallwould make both assertions deterministic.🤖 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 `@tests/e2e/webrtc.spec.ts` around lines 53 - 73, Update the WebRTC test setup and navigation to use relative URLs so Playwright’s configured baseURL applies instead of hardcoding localhost. Add a dedicated data-testid to the connection-state badge rendered by VideoCall, then replace the broad “Connected” text locators in both pageA and pageB assertions with that test id.
🤖 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 `@src/app/api/session/`[id]/signal/route.ts:
- Around line 21-47: The session signaling flow trusts an unauthenticated
client-supplied role, allowing callers to impersonate either participant. In
src/app/api/session/[id]/signal/route.ts lines 21-47, authenticate the caller,
verify membership in session id, derive sender from the server-side session
role, and apply the same guard to GET and DELETE; in
src/app/session/[id]/page.tsx lines 93-97, resolve userRole from the
authenticated session record instead of searchParams; in
src/components/session/VideoCall.tsx lines 63-77, remove sender: role from the
signal POST body while retaining role for local UI and offerer decisions.
- Around line 34-40: Update the request validation around the body destructuring
to validate type against the supported signal-type union before writing it to
the store, and treat data as missing only when it is undefined or null so
legitimate falsy payloads such as 0, false, and an empty string are accepted.
Preserve the existing 400 response for invalid or missing required fields.
- Around line 83-101: Validate the since query parameter in the signal route
before filtering: reject non-numeric or invalid values with a 400 response
instead of allowing NaN. Update the cursor/filtering logic around otherRole and
sessionSignals to use a monotonic per-signal sequence identifier, preserving all
signals after the client’s cursor, including signals sharing the same timestamp.
In `@src/app/session/`[id]/page.tsx:
- Around line 197-252: Render a single VideoCall instance in the session page
instead of maintaining separate non-PiP and isPictureInPicture branches.
Consolidate the shared VideoCall props into one render location and let its
isPictureInPicture prop control the layout, while preserving the surrounding
Card content for PiP mode and the existing container styling for normal mode.
In `@src/components/session/VideoCall.tsx`:
- Around line 498-517: Keep the local and remote video elements mounted in the
main and PiP layouts so their MediaStream audio and video bindings persist;
replace conditional unmounting with avatar/status overlays when video is
unavailable. Update the relevant VideoCall rendering branches around the remote
video and corresponding local/PiP frames, and ensure any existing srcObject
attachment logic reuses the mounted refs when camera state changes.
- Line 213: Update initializeConnection and its mount effect in VideoCall so
isMuted and isVideoOn are read through refs rather than captured dependencies,
then remove them from the callback dependency array. Preserve the toggleMute and
toggleVideo handlers’ direct track.enabled updates, and ensure the connection
initializes once without rebuilding when either setting changes.
- Around line 111-213: The initializeConnection callback must avoid stale
startPolling and handleReconnect closures while preserving the dependency lint
contract. Add startPollingRef and handleReconnectRef, assign the current helper
implementations to those refs in an effect, and have initializeConnection invoke
the ref-backed helpers; update handleReconnect’s implementation and dependencies
as needed to preserve reconnect behavior without creating a circular callback
dependency.
- Around line 132-142: Update the fallback audio setup near the local stream
construction to remove the unused oscillator and use the destination stream
directly for the placeholder audio track. Store the created AudioContext in an
audioContextRef, then update cleanupConnection to close it and clear the ref so
failed getUserMedia attempts and reconnects do not leak contexts.
In `@tests/e2e/webrtc.spec.ts`:
- Around line 37-48: Move the fake media flags from the context options in the
WebRTC test setup to use.launchOptions.args in playwright.config.ts. Remove args
from both contextA and contextB browser.newContext calls, and configure both
--use-fake-device-for-media-stream and --use-fake-ui-for-media-stream at browser
launch.
---
Nitpick comments:
In `@src/app/api/session/`[id]/signal/route.ts:
- Line 7: Replace the explicit any types in SignalMessage.data and the parsed
body with unknown. Before destructuring body, add the necessary narrowing or
validation so the existing signal handling remains type-safe while preserving
the stored-and-echoed data behavior.
- Around line 11-17: Replace the process-local sessionSignals store used by the
signal route with a shared external store such as Redis, using per-session key
expiry so seeker and expert requests can communicate across instances and
abandoned sessions are automatically removed. If retaining the in-memory
implementation, add a bounded TTL cleanup mechanism for entries that are not
explicitly deleted.
In `@src/components/session/VideoCall.tsx`:
- Line 4: Remove the unused MessageCircle import from VideoCall.tsx. Update
sendSignal to avoid any by typing its signal payload as unknown or an
appropriate discriminated union keyed by type, while preserving its existing
signaling behavior.
- Around line 57-60: Update showToast to store the timeout handle in a ref,
clearing any existing timer before scheduling the next dismissal so back-to-back
toasts remain visible for the full duration. Clear the tracked timer during
cleanupConnection or a dedicated unmount effect, and reset the ref so no timeout
updates state after unmount.
In `@tests/e2e/webrtc.spec.ts`:
- Around line 76-77: Ensure the contexts created for the WebRTC test are always
closed when assertions fail by moving their setup into a beforeEach/fixture with
automatic teardown or wrapping the test flow in try/finally. Preserve the
existing contextA and contextB usage and close both contexts during cleanup.
- Around line 53-73: Update the WebRTC test setup and navigation to use relative
URLs so Playwright’s configured baseURL applies instead of hardcoding localhost.
Add a dedicated data-testid to the connection-state badge rendered by VideoCall,
then replace the broad “Connected” text locators in both pageA and pageB
assertions with that test id.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c39d0ad-8530-4751-97d1-c7a05c636001
📒 Files selected for processing (4)
src/app/api/session/[id]/signal/route.tssrc/app/session/[id]/page.tsxsrc/components/session/VideoCall.tsxtests/e2e/webrtc.spec.ts
Comment on lines
+21
to
+47
| export async function POST( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ id: string }> } | ||
| ) { | ||
| const { id } = await params; | ||
| let body: any; | ||
|
|
||
| try { | ||
| body = await request.json(); | ||
| } catch { | ||
| return NextResponse.json({ error: "Invalid JSON body" }, { status: 400 }); | ||
| } | ||
|
|
||
| const { sender, type, data } = body; | ||
| if (!sender || !type || !data) { | ||
| return NextResponse.json( | ||
| { error: "Missing required fields: sender, type, data" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| if (sender !== "seeker" && sender !== "expert") { | ||
| return NextResponse.json( | ||
| { error: "sender must be 'seeker' or 'expert'" }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
Peer identity originates from an unauthenticated URL query param and is never verified server-side. The role travels from ?role= → VideoCall prop → sender in the signal body → the store, with no authentication anywhere along the path, so anyone with a session id can claim to be the expert and pick up the seeker's offer and media.
src/app/api/session/[id]/signal/route.ts#L21-L47: authenticate the caller, verify membership in sessionid, and derivesenderfrom the server-side session/role rather than the request body; apply the same guard toGETandDELETE.src/app/session/[id]/page.tsx#L93-L97: stop derivinguserRolefromsearchParams; resolve the viewer's role from the authenticated session record forsessionId.src/components/session/VideoCall.tsx#L63-L77: dropsender: rolefrom the POST body once the server derives it, keepingrolefor local UI/offerer decisions only.
🧰 Tools
🪛 ESLint
[error] 26-26: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
📍 Affects 3 files
src/app/api/session/[id]/signal/route.ts#L21-L47(this comment)src/app/session/[id]/page.tsx#L93-L97src/components/session/VideoCall.tsx#L63-L77
🤖 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 `@src/app/api/session/`[id]/signal/route.ts around lines 21 - 47, The session
signaling flow trusts an unauthenticated client-supplied role, allowing callers
to impersonate either participant. In src/app/api/session/[id]/signal/route.ts
lines 21-47, authenticate the caller, verify membership in session id, derive
sender from the server-side session role, and apply the same guard to GET and
DELETE; in src/app/session/[id]/page.tsx lines 93-97, resolve userRole from the
authenticated session record instead of searchParams; in
src/components/session/VideoCall.tsx lines 63-77, remove sender: role from the
signal POST body while retaining role for local UI and offerer decisions.
Comment on lines
+34
to
+40
| const { sender, type, data } = body; | ||
| if (!sender || !type || !data) { | ||
| return NextResponse.json( | ||
| { error: "Missing required fields: sender, type, data" }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
!data rejects legitimate falsy payloads and type is never validated.
type is written straight into the store without checking it against the union, so any arbitrary string is persisted and polled back to peers. Meanwhile !data rejects data: 0/false/"".
♻️ Tighter validation
- const { sender, type, data } = body;
- if (!sender || !type || !data) {
+ const { sender, type, data } = body;
+ const VALID_TYPES = ["offer", "answer", "candidate", "status"] as const;
+ if (!sender || !type || data === undefined || data === null) {
return NextResponse.json(
{ error: "Missing required fields: sender, type, data" },
{ status: 400 }
);
}
+ if (!VALID_TYPES.includes(type)) {
+ return NextResponse.json({ error: "Invalid signal type" }, { status: 400 });
+ }📝 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.
Suggested change
| const { sender, type, data } = body; | |
| if (!sender || !type || !data) { | |
| return NextResponse.json( | |
| { error: "Missing required fields: sender, type, data" }, | |
| { status: 400 } | |
| ); | |
| } | |
| const { sender, type, data } = body; | |
| const VALID_TYPES = ["offer", "answer", "candidate", "status"] as const; | |
| if (!sender || !type || data === undefined || data === null) { | |
| return NextResponse.json( | |
| { error: "Missing required fields: sender, type, data" }, | |
| { status: 400 } | |
| ); | |
| } | |
| if (!VALID_TYPES.includes(type)) { | |
| return NextResponse.json({ error: "Invalid signal type" }, { status: 400 }); | |
| } |
🤖 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 `@src/app/api/session/`[id]/signal/route.ts around lines 34 - 40, Update the
request validation around the body destructuring to validate type against the
supported signal-type union before writing it to the store, and treat data as
missing only when it is undefined or null so legitimate falsy payloads such as
0, false, and an empty string are accepted. Preserve the existing 400 response
for invalid or missing required fields.
Comment on lines
+83
to
+101
| const sinceStr = searchParams.get("since"); | ||
| const since = sinceStr ? parseInt(sinceStr, 10) : 0; | ||
|
|
||
| if (!role || (role !== "seeker" && role !== "expert")) { | ||
| return NextResponse.json( | ||
| { error: "role query param must be 'seeker' or 'expert'" }, | ||
| { status: 400 } | ||
| ); | ||
| } | ||
|
|
||
| if (!globalSignals.sessionSignals || !globalSignals.sessionSignals[id]) { | ||
| return NextResponse.json({ signals: [] }, { status: 200 }); | ||
| } | ||
|
|
||
| // Get signals sent by the OTHER peer after the since timestamp | ||
| const otherRole = role === "seeker" ? "expert" : "seeker"; | ||
| const signals = globalSignals.sessionSignals[id].filter( | ||
| (sig) => sig.sender === otherRole && sig.timestamp > since | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Unvalidated since yields NaN and silently returns zero signals forever.
parseInt("abc") → NaN, and sig.timestamp > NaN is always false, so a malformed since makes the peer never receive any signaling message with no error surfaced. Also, timestamp > since at millisecond granularity drops any signal written in the same millisecond as the cursor the client last saw — ICE candidates are bursty, so this is a realistic cause of stuck connections. Consider a monotonically increasing sequence number as the cursor.
🐛 Proposed fix for NaN handling
- const since = sinceStr ? parseInt(sinceStr, 10) : 0;
+ const parsedSince = sinceStr ? Number.parseInt(sinceStr, 10) : 0;
+ if (Number.isNaN(parsedSince) || parsedSince < 0) {
+ return NextResponse.json(
+ { error: "since must be a non-negative integer" },
+ { status: 400 }
+ );
+ }
+ const since = parsedSince;📝 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.
Suggested change
| const sinceStr = searchParams.get("since"); | |
| const since = sinceStr ? parseInt(sinceStr, 10) : 0; | |
| if (!role || (role !== "seeker" && role !== "expert")) { | |
| return NextResponse.json( | |
| { error: "role query param must be 'seeker' or 'expert'" }, | |
| { status: 400 } | |
| ); | |
| } | |
| if (!globalSignals.sessionSignals || !globalSignals.sessionSignals[id]) { | |
| return NextResponse.json({ signals: [] }, { status: 200 }); | |
| } | |
| // Get signals sent by the OTHER peer after the since timestamp | |
| const otherRole = role === "seeker" ? "expert" : "seeker"; | |
| const signals = globalSignals.sessionSignals[id].filter( | |
| (sig) => sig.sender === otherRole && sig.timestamp > since | |
| ); | |
| const sinceStr = searchParams.get("since"); | |
| const parsedSince = sinceStr ? Number.parseInt(sinceStr, 10) : 0; | |
| if (Number.isNaN(parsedSince) || parsedSince < 0) { | |
| return NextResponse.json( | |
| { error: "since must be a non-negative integer" }, | |
| { status: 400 } | |
| ); | |
| } | |
| const since = parsedSince; | |
| if (!role || (role !== "seeker" && role !== "expert")) { | |
| return NextResponse.json( | |
| { error: "role query param must be 'seeker' or 'expert'" }, | |
| { status: 400 } | |
| ); | |
| } | |
| if (!globalSignals.sessionSignals || !globalSignals.sessionSignals[id]) { | |
| return NextResponse.json({ signals: [] }, { status: 200 }); | |
| } | |
| // Get signals sent by the OTHER peer after the since timestamp | |
| const otherRole = role === "seeker" ? "expert" : "seeker"; | |
| const signals = globalSignals.sessionSignals[id].filter( | |
| (sig) => sig.sender === otherRole && sig.timestamp > since | |
| ); |
🤖 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 `@src/app/api/session/`[id]/signal/route.ts around lines 83 - 101, Validate the
since query parameter in the signal route before filtering: reject non-numeric
or invalid values with a 400 response instead of allowing NaN. Update the
cursor/filtering logic around otherRole and sessionSignals to use a monotonic
per-signal sequence identifier, preserving all signals after the client’s
cursor, including signals sharing the same timestamp.
Comment on lines
+197
to
+252
| {isPictureInPicture ? ( | ||
| <Card | ||
| variant="glow" | ||
| className="relative overflow-hidden flex-1 min-h-[300px]" | ||
| > | ||
| <div className="absolute inset-0 bg-gradient-to-b from-primary/5 to-transparent pointer-events-none" /> | ||
| <CardContent className="flex flex-col items-center justify-center h-full py-16"> | ||
| <LiveCounter | ||
| ratePerSecond={session.ratePerSecond} | ||
| onTotalChange={setTotalStreamed} | ||
| remainingSeconds={remainingSeconds} | ||
| className="mb-8" | ||
| /> | ||
|
|
||
| <div className="flex items-center gap-3 mb-4"> | ||
| <Badge variant="success" className="text-xs"> | ||
| <span className="size-1.5 rounded-full bg-emerald-400 mr-1.5 animate-pulse" /> | ||
| Session Active | ||
| </Badge> | ||
| <Badge variant="info" className="text-xs"> | ||
| <Zap className="size-3" /> | ||
| Live | ||
| </Badge> | ||
| </div> | ||
| <p className="text-sm text-foreground/50">Video call is floating</p> | ||
| </CardContent> | ||
| </Card> | ||
| ) : ( | ||
| <div className="relative flex-1 min-h-[450px] rounded-2xl overflow-hidden border border-purple-500/30 bg-black"> | ||
| <VideoCall | ||
| expertName={session.expertName} | ||
| seekerName="You" | ||
| expertAvatar={session.expertAvatar || "/assets/Avatar.svg"} | ||
| seekerAvatar="/assets/Avatar.svg" | ||
| onEndCall={handleEndSession} | ||
| isPictureInPicture={isPictureInPicture} | ||
| onTogglePIP={() => setIsPictureInPicture(!isPictureInPicture)} | ||
| sessionId={sessionId} | ||
| role={userRole} | ||
| /> | ||
| </div> | ||
| )} | ||
|
|
||
| <div className="flex items-center gap-3"> | ||
| <Badge variant="success" className="text-xs"> | ||
| <span className="size-1.5 rounded-full bg-emerald-400 mr-1.5 animate-pulse" /> | ||
| Session Active | ||
| </Badge> | ||
| <Badge variant="info" className="text-xs"> | ||
| <Zap className="size-3" /> | ||
| Live | ||
| </Badge> | ||
| </div> | ||
| </CardContent> | ||
| </Card> | ||
| {isPictureInPicture && ( | ||
| <VideoCall | ||
| expertName={session.expertName} | ||
| seekerName="You" | ||
| expertAvatar={session.expertAvatar || "/assets/Avatar.svg"} | ||
| seekerAvatar="/assets/Avatar.svg" | ||
| onEndCall={handleEndSession} | ||
| isPictureInPicture={isPictureInPicture} | ||
| onTogglePIP={() => setIsPictureInPicture(!isPictureInPicture)} | ||
| sessionId={sessionId} | ||
| role={userRole} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Two distinct VideoCall instances means toggling PiP destroys and re-establishes the call.
The non-PiP branch and the isPictureInPicture && ... block are separate elements in different tree positions, so React unmounts one and mounts the other on every toggle. VideoCall's cleanup stops the local tracks and closes the peer connection, then the new instance re-acquires media and (as seeker) posts a fresh offer — the remote peer sees the call drop. Render one instance and let it choose its own layout.
🐛 Single instance, layout-only branching
- {isPictureInPicture ? (
- <Card ...>
+ {isPictureInPicture && (
+ <Card
+ variant="glow"
+ className="relative overflow-hidden flex-1 min-h-[300px]"
+ >
...
</Card>
- ) : (
- <div className="relative flex-1 min-h-[450px] rounded-2xl overflow-hidden border border-purple-500/30 bg-black">
- <VideoCall ... />
- </div>
)}
-
- {isPictureInPicture && (
- <VideoCall ... />
- )}
+ <div
+ className={cn(
+ !isPictureInPicture &&
+ "relative flex-1 min-h-[450px] rounded-2xl overflow-hidden border border-purple-500/30 bg-black",
+ )}
+ >
+ <VideoCall
+ expertName={session.expertName}
+ seekerName="You"
+ expertAvatar={session.expertAvatar || "/assets/Avatar.svg"}
+ seekerAvatar="/assets/Avatar.svg"
+ onEndCall={handleEndSession}
+ isPictureInPicture={isPictureInPicture}
+ onTogglePIP={() => setIsPictureInPicture((prev) => !prev)}
+ sessionId={sessionId}
+ role={userRole}
+ />
+ </div>Note the PiP layout in VideoCall renders fixed-positioned markup, so it detaches from this container visually either way.
📝 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.
Suggested change
| {isPictureInPicture ? ( | |
| <Card | |
| variant="glow" | |
| className="relative overflow-hidden flex-1 min-h-[300px]" | |
| > | |
| <div className="absolute inset-0 bg-gradient-to-b from-primary/5 to-transparent pointer-events-none" /> | |
| <CardContent className="flex flex-col items-center justify-center h-full py-16"> | |
| <LiveCounter | |
| ratePerSecond={session.ratePerSecond} | |
| onTotalChange={setTotalStreamed} | |
| remainingSeconds={remainingSeconds} | |
| className="mb-8" | |
| /> | |
| <div className="flex items-center gap-3 mb-4"> | |
| <Badge variant="success" className="text-xs"> | |
| <span className="size-1.5 rounded-full bg-emerald-400 mr-1.5 animate-pulse" /> | |
| Session Active | |
| </Badge> | |
| <Badge variant="info" className="text-xs"> | |
| <Zap className="size-3" /> | |
| Live | |
| </Badge> | |
| </div> | |
| <p className="text-sm text-foreground/50">Video call is floating</p> | |
| </CardContent> | |
| </Card> | |
| ) : ( | |
| <div className="relative flex-1 min-h-[450px] rounded-2xl overflow-hidden border border-purple-500/30 bg-black"> | |
| <VideoCall | |
| expertName={session.expertName} | |
| seekerName="You" | |
| expertAvatar={session.expertAvatar || "/assets/Avatar.svg"} | |
| seekerAvatar="/assets/Avatar.svg" | |
| onEndCall={handleEndSession} | |
| isPictureInPicture={isPictureInPicture} | |
| onTogglePIP={() => setIsPictureInPicture(!isPictureInPicture)} | |
| sessionId={sessionId} | |
| role={userRole} | |
| /> | |
| </div> | |
| )} | |
| <div className="flex items-center gap-3"> | |
| <Badge variant="success" className="text-xs"> | |
| <span className="size-1.5 rounded-full bg-emerald-400 mr-1.5 animate-pulse" /> | |
| Session Active | |
| </Badge> | |
| <Badge variant="info" className="text-xs"> | |
| <Zap className="size-3" /> | |
| Live | |
| </Badge> | |
| </div> | |
| </CardContent> | |
| </Card> | |
| {isPictureInPicture && ( | |
| <VideoCall | |
| expertName={session.expertName} | |
| seekerName="You" | |
| expertAvatar={session.expertAvatar || "/assets/Avatar.svg"} | |
| seekerAvatar="/assets/Avatar.svg" | |
| onEndCall={handleEndSession} | |
| isPictureInPicture={isPictureInPicture} | |
| onTogglePIP={() => setIsPictureInPicture(!isPictureInPicture)} | |
| sessionId={sessionId} | |
| role={userRole} | |
| /> | |
| )} | |
| {isPictureInPicture && ( | |
| <Card | |
| variant="glow" | |
| className="relative overflow-hidden flex-1 min-h-[300px]" | |
| > | |
| <div className="absolute inset-0 bg-gradient-to-b from-primary/5 to-transparent pointer-events-none" /> | |
| <CardContent className="flex flex-col items-center justify-center h-full py-16"> | |
| <LiveCounter | |
| ratePerSecond={session.ratePerSecond} | |
| onTotalChange={setTotalStreamed} | |
| remainingSeconds={remainingSeconds} | |
| className="mb-8" | |
| /> | |
| <div className="flex items-center gap-3 mb-4"> | |
| <Badge variant="success" className="text-xs"> | |
| <span className="size-1.5 rounded-full bg-emerald-400 mr-1.5 animate-pulse" /> | |
| Session Active | |
| </Badge> | |
| <Badge variant="info" className="text-xs"> | |
| <Zap className="size-3" /> | |
| Live | |
| </Badge> | |
| </div> | |
| <p className="text-sm text-foreground/50">Video call is floating</p> | |
| </CardContent> | |
| </Card> | |
| )} | |
| <div | |
| className={cn( | |
| !isPictureInPicture && | |
| "relative flex-1 min-h-[450px] rounded-2xl overflow-hidden border border-purple-500/30 bg-black", | |
| )} | |
| > | |
| <VideoCall | |
| expertName={session.expertName} | |
| seekerName="You" | |
| expertAvatar={session.expertAvatar || "/assets/Avatar.svg"} | |
| seekerAvatar="/assets/Avatar.svg" | |
| onEndCall={handleEndSession} | |
| isPictureInPicture={isPictureInPicture} | |
| onTogglePIP={() => setIsPictureInPicture((prev) => !prev)} | |
| sessionId={sessionId} | |
| role={userRole} | |
| /> | |
| </div> |
🤖 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 `@src/app/session/`[id]/page.tsx around lines 197 - 252, Render a single
VideoCall instance in the session page instead of maintaining separate non-PiP
and isPictureInPicture branches. Consolidate the shared VideoCall props into one
render location and let its isPictureInPicture prop control the layout, while
preserving the surrounding Card content for PiP mode and the existing container
styling for normal mode.
Comment on lines
+111
to
+213
| const initializeConnection = useCallback(async () => { | ||
| try { | ||
| // 1. Acquire Local Camera and Mic | ||
| let localStream: MediaStream; | ||
| try { | ||
| localStream = await navigator.mediaDevices.getUserMedia({ | ||
| video: true, | ||
| audio: true, | ||
| }); | ||
| } catch (err) { | ||
| console.warn("Could not access camera/microphone. Using fallback empty streams.", err); | ||
| showToast("Device warning: Camera/Microphone not accessible."); | ||
| // Fallback: Create silent audio track & blank video track if media unavailable | ||
| const canvas = document.createElement("canvas"); | ||
| canvas.width = 640; | ||
| canvas.height = 480; | ||
| const ctx = canvas.getContext("2d"); | ||
| if (ctx) { | ||
| ctx.fillStyle = "black"; | ||
| ctx.fillRect(0, 0, 640, 480); | ||
| } | ||
| const videoTrack = (canvas as any).captureStream?.(25)?.getVideoTracks()[0] || null; | ||
| const audioContext = new (window.AudioContext || (window as any).webkitAudioContext)(); | ||
| const oscillator = audioContext.createOscillator(); | ||
| const dst = audioContext.createMediaStreamDestination(); | ||
| oscillator.connect(dst); | ||
| const audioTrack = dst.stream.getAudioTracks()[0] || null; | ||
|
|
||
| const tracks = []; | ||
| if (videoTrack) tracks.push(videoTrack); | ||
| if (audioTrack) tracks.push(audioTrack); | ||
| localStream = new MediaStream(tracks); | ||
| } | ||
|
|
||
| localStreamRef.current = localStream; | ||
| if (videoRefLocal.current) { | ||
| videoRefLocal.current.srcObject = localStream; | ||
| } | ||
|
|
||
| // Apply initial mute/video toggle settings | ||
| localStream.getAudioTracks().forEach(t => t.enabled = !isMuted); | ||
| localStream.getVideoTracks().forEach(t => t.enabled = isVideoOn); | ||
|
|
||
| // 2. Setup RTCPeerConnection | ||
| const pc = new RTCPeerConnection({ | ||
| iceServers: ICE_SERVERS, | ||
| }); | ||
| peerConnectionRef.current = pc; | ||
|
|
||
| // 3. Add tracks to Connection | ||
| localStream.getTracks().forEach((track) => { | ||
| pc.addTrack(track, localStream); | ||
| }); | ||
|
|
||
| // 4. Handle Remote Track | ||
| pc.ontrack = (event) => { | ||
| if (event.streams && event.streams[0]) { | ||
| remoteStreamRef.current = event.streams[0]; | ||
| setIsRemoteConnected(true); | ||
| if (videoRefRemote.current) { | ||
| videoRefRemote.current.srcObject = event.streams[0]; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| // 5. Handle ICE Candidates | ||
| pc.onicecandidate = (event) => { | ||
| if (event.candidate) { | ||
| sendSignal("candidate", event.candidate); | ||
| } | ||
| }; | ||
|
|
||
| // 6. Monitor Connection State | ||
| pc.onconnectionstatechange = () => { | ||
| setConnectionState(pc.connectionState); | ||
| if (pc.connectionState === "connected") { | ||
| setIsRemoteConnected(true); | ||
| isReconnectingRef.current = false; | ||
| } else if (pc.connectionState === "disconnected" || pc.connectionState === "failed") { | ||
| setIsRemoteConnected(false); | ||
| handleReconnect(); | ||
| } | ||
| }; | ||
|
|
||
| // 7. Seeker Initiates Offer | ||
| if (role === "seeker") { | ||
| const offer = await pc.createOffer(); | ||
| await pc.setLocalDescription(offer); | ||
| await sendSignal("offer", offer); | ||
| } | ||
|
|
||
| // 8. Start Signaling Polling Loop | ||
| startPolling(); | ||
|
|
||
| // Publish initial state status | ||
| sendSignal("status", { video: isVideoOn, audio: !isMuted }); | ||
|
|
||
| } catch (err) { | ||
| console.error("Failed to initialize connection:", err); | ||
| showToast("WebRTC connection failed. Retrying..."); | ||
| handleReconnect(); | ||
| } | ||
| }, [role, isMuted, isVideoOn, sendSignal, showToast]); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Circular useCallback dependencies leave initializeConnection bound to stale startPolling/handleReconnect.
initializeConnection calls both but declares neither (they're defined below it and handleReconnect depends back on initializeConnection). It works today only because the omitted identities happen to be reachable via closure over the first render's values — fragile, and react-hooks/exhaustive-deps will flag it. Hoisting the two helpers into refs (startPollingRef, handleReconnectRef) assigned in an effect breaks the cycle without silencing the lint rule wholesale.
Also applies to: 216-278
🧰 Tools
🪛 ESLint
[error] 132-132: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 133-133: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 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 `@src/components/session/VideoCall.tsx` around lines 111 - 213, The
initializeConnection callback must avoid stale startPolling and handleReconnect
closures while preserving the dependency lint contract. Add startPollingRef and
handleReconnectRef, assign the current helper implementations to those refs in
an effect, and have initializeConnection invoke the ref-backed helpers; update
handleReconnect’s implementation and dependencies as needed to preserve
reconnect behavior without creating a circular callback dependency.
Comment on lines
+132
to
+142
| const videoTrack = (canvas as any).captureStream?.(25)?.getVideoTracks()[0] || null; | ||
| const audioContext = new (window.AudioContext || (window as any).webkitAudioContext)(); | ||
| const oscillator = audioContext.createOscillator(); | ||
| const dst = audioContext.createMediaStreamDestination(); | ||
| oscillator.connect(dst); | ||
| const audioTrack = dst.stream.getAudioTracks()[0] || null; | ||
|
|
||
| const tracks = []; | ||
| if (videoTrack) tracks.push(videoTrack); | ||
| if (audioTrack) tracks.push(audioTrack); | ||
| localStream = new MediaStream(tracks); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Fallback audio track is silent-by-accident and the AudioContext is never released.
oscillator is created but start() is never called, and the AudioContext is neither stored nor closed, so each failed getUserMedia (including every reconnect attempt) leaks an audio context. If the intent is a silent placeholder track, drop the oscillator entirely and use the destination stream directly; either way, close the context in cleanupConnection.
♻️ Simplify the fallback
- const videoTrack = (canvas as any).captureStream?.(25)?.getVideoTracks()[0] || null;
- const audioContext = new (window.AudioContext || (window as any).webkitAudioContext)();
- const oscillator = audioContext.createOscillator();
- const dst = audioContext.createMediaStreamDestination();
- oscillator.connect(dst);
- const audioTrack = dst.stream.getAudioTracks()[0] || null;
+ const videoTrack = canvas.captureStream?.(25)?.getVideoTracks()[0] ?? null;
+ const audioContext = new AudioContext();
+ audioContextRef.current = audioContext;
+ const dst = audioContext.createMediaStreamDestination();
+ const audioTrack = dst.stream.getAudioTracks()[0] ?? null;Then in cleanupConnection: audioContextRef.current?.close(); audioContextRef.current = null;
🧰 Tools
🪛 ESLint
[error] 132-132: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 133-133: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 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 `@src/components/session/VideoCall.tsx` around lines 132 - 142, Update the
fallback audio setup near the local stream construction to remove the unused
oscillator and use the destination stream directly for the placeholder audio
track. Store the created AudioContext in an audioContextRef, then update
cleanupConnection to close it and clear the ref so failed getUserMedia attempts
and reconnects do not leak contexts.
Source: Linters/SAST tools
| showToast("WebRTC connection failed. Retrying..."); | ||
| handleReconnect(); | ||
| } | ||
| }, [role, isMuted, isVideoOn, sendSignal, showToast]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Muting or toggling the camera tears down and rebuilds the entire call.
initializeConnection lists isMuted and isVideoOn in its dep array, so every toggleMute/toggleVideo produces a new callback identity, which re-runs the mount effect: cleanup stops all local tracks and closes the RTCPeerConnection, then a brand-new connection with a fresh offer is created. The toggle handlers already mutate track.enabled directly, so these deps are unnecessary — read them from refs inside initializeConnection and mount once.
🐛 Proposed fix
+ const isMutedRef = useRef(isMuted);
+ const isVideoOnRef = useRef(isVideoOn);
+ useEffect(() => { isMutedRef.current = isMuted; }, [isMuted]);
+ useEffect(() => { isVideoOnRef.current = isVideoOn; }, [isVideoOn]);- localStream.getAudioTracks().forEach(t => t.enabled = !isMuted);
- localStream.getVideoTracks().forEach(t => t.enabled = isVideoOn);
+ localStream.getAudioTracks().forEach(t => t.enabled = !isMutedRef.current);
+ localStream.getVideoTracks().forEach(t => t.enabled = isVideoOnRef.current);- sendSignal("status", { video: isVideoOn, audio: !isMuted });
+ sendSignal("status", { video: isVideoOnRef.current, audio: !isMutedRef.current });- }, [role, isMuted, isVideoOn, sendSignal, showToast]);
+ }, [role, sendSignal, showToast]); useEffect(() => {
initializeConnection();
...
- }, [initializeConnection, cleanupConnection]);
+ // eslint-disable-next-line react-hooks/exhaustive-deps
+ }, []);Also applies to: 281-293
🤖 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 `@src/components/session/VideoCall.tsx` at line 213, Update
initializeConnection and its mount effect in VideoCall so isMuted and isVideoOn
are read through refs rather than captured dependencies, then remove them from
the callback dependency array. Preserve the toggleMute and toggleVideo handlers’
direct track.enabled updates, and ensure the connection initializes once without
rebuilding when either setting changes.
Comment on lines
+498
to
517
| {isRemoteConnected && isRemoteVideoOn ? ( | ||
| <video | ||
| ref={videoRefRemote} | ||
| className="w-full h-full object-cover" | ||
| autoPlay | ||
| playsInline | ||
| /> | ||
| ) : ( | ||
| <div className="absolute inset-0 flex flex-col items-center justify-center bg-gradient-to-br from-purple-900/40 to-black/80"> | ||
| <img | ||
| src={expertAvatar} | ||
| alt={expertName} | ||
| className="w-32 h-32 rounded-full border-4 border-purple-500/50 object-cover" | ||
| src={remoteUserAvatar || "/assets/Avatar.svg"} | ||
| alt={remoteUserName} | ||
| className="w-24 h-24 sm:w-32 sm:h-32 rounded-full border-4 border-purple-500/30 object-cover shadow-2xl" | ||
| /> | ||
| <p className="mt-4 text-lg font-semibold">{expertName}</p> | ||
| <p className="mt-4 text-base font-semibold text-purple-200">{remoteUserName}</p> | ||
| <p className="text-xs text-muted-foreground mt-1"> | ||
| {!isRemoteConnected ? "Waiting for peer to join..." : "Camera is off"} | ||
| </p> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Conditionally unmounting the <video> elements kills remote audio and never re-attaches srcObject.
The remote <video> is the only sink for the remote audio track. When the peer turns their camera off (isRemoteVideoOn === false), the element unmounts and you stop hearing them entirely. Worse, when it remounts, ontrack has already fired, so videoRefRemote.current.srcObject is never set again and video never comes back. The local frame has the same re-attach problem after toggling the camera back on.
Keep the elements mounted and overlay the avatar instead (and/or re-attach srcObject in an effect keyed on the refs).
🐛 Overlay instead of unmount (remote frame)
- {isRemoteConnected && isRemoteVideoOn ? (
- <video
- ref={videoRefRemote}
- className="w-full h-full object-cover"
- autoPlay
- playsInline
- />
- ) : (
- <div className="absolute inset-0 flex flex-col items-center justify-center bg-gradient-to-br from-purple-900/40 to-black/80">
+ <video
+ ref={videoRefRemote}
+ className="w-full h-full object-cover"
+ autoPlay
+ playsInline
+ />
+ {!(isRemoteConnected && isRemoteVideoOn) && (
+ <div className="absolute inset-0 flex flex-col items-center justify-center bg-gradient-to-br from-purple-900/40 to-black/80">
...
</div>
)}Apply the same pattern to the local frame and to both frames in the PiP layout.
Also applies to: 529-536
🤖 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 `@src/components/session/VideoCall.tsx` around lines 498 - 517, Keep the local
and remote video elements mounted in the main and PiP layouts so their
MediaStream audio and video bindings persist; replace conditional unmounting
with avatar/status overlays when video is unavailable. Update the relevant
VideoCall rendering branches around the remote video and corresponding local/PiP
frames, and ensure any existing srcObject attachment logic reuses the mounted
refs when camera state changes.
Comment on lines
+37
to
+48
| const contextA = await browser.newContext({ | ||
| permissions: ["camera", "microphone"], | ||
| args: ["--use-fake-device-for-media-stream", "--use-fake-ui-for-media-stream"] | ||
| }); | ||
| const pageA = await contextA.newPage(); | ||
|
|
||
| // Tab B: Expert Context | ||
| const contextB = await browser.newContext({ | ||
| permissions: ["camera", "microphone"], | ||
| args: ["--use-fake-device-for-media-stream", "--use-fake-ui-for-media-stream"] | ||
| }); | ||
| const pageB = await contextB.newPage(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'playwright.config.*' | xargs -r cat -nRepository: LightForgeHub/SkillSphere-Dapp
Length of output: 1784
🏁 Script executed:
#!/bin/bash
set -eu
echo "== Locate relevant files =="
git ls-files | grep -E '(^|/)(webrtc\.spec\.(ts|tsx)|VideoCall|Media|video-call|user-media|playwright\.config\.)' | sed -n '1,120p'
echo
echo "== Search fake device args and media stream handling =="
rg -n --fixed-strings "--use-fake-device-for-media-stream|--use-fake-ui-for-media-stream|browser.newContext|launchOptions|args:|getUserMedia|videoEl|canvas|audioContext|OscillatorNode" . -g '!node_modules' -g '!dist' -g '!build' | sed -n '1,240p'
echo
echo "== Playwright test package type if present =="
if [ -f package.json ]; then cat package.json | python3 - <<'PY'
import json,sys
data=json.load(sys.stdin)
for k in ["dependencies","devDependencies"]:
if k in data:
for n,v in data[k].items():
if n == "`@playwright/test`" or n.startswith("playwright") or n.endswith("playwright"):
print(f"{k}:{n}={v}")
PY
fi
echo
echo "== Inspect e2e spec around context creation and video capture =="
spec=$(fd -t f 'webrtc\.spec\.(ts|tsx)' . | head -n1 || true)
if [ -n "${spec:-}" ]; then
wc -l "$spec"
sed -n '1,180p' "$spec" | cat -n
fiRepository: LightForgeHub/SkillSphere-Dapp
Length of output: 1207
🏁 Script executed:
#!/bin/bash
set -eu
echo "== Relevant files =="
for f in playwright.config.ts src/app/ui-demo/video-call/page.tsx src/components/session/VideoCall.tsx tests/e2e/webrtc.spec.ts; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
cat -n "$f"
fi
done
echo
echo "== Exact-flag/text search with explicit args =="
rg -n --fixed-strings --use-fake-device-for-media-stream --use-fake-ui-for-media-stream \
browser.newContext launchOptions args getUserMedia videoEl canvas audioContext OscillatorNode . -g '!node_modules' -g '!dist' -g '!build' || true
echo
echo "== package files =="
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock; do
if [ -f "$f" ]; then
echo "--- $f ---"
case "$f" in
package.json) sed -n '1,220p' "$f";;
*) sed -n '1,80p' "$f";;
esac
fi
doneRepository: LightForgeHub/SkillSphere-Dapp
Length of output: 46900
Move the fake media-device flags to the browser launch options.
browser.newContext() options do not include args, so --use-fake-device-for-media-stream and --use-fake-ui-for-media-stream are ignored. Without real fake camera/mic support, VideoCall falls through to its canvas/oscillator fallback path instead of exercising media capture and WebRTC signaling. Remove args from the context options and add these flags under use.launchOptions.args in playwright.config.ts.
🤖 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 `@tests/e2e/webrtc.spec.ts` around lines 37 - 48, Move the fake media flags
from the context options in the WebRTC test setup to use.launchOptions.args in
playwright.config.ts. Remove args from both contextA and contextB
browser.newContext calls, and configure both --use-fake-device-for-media-stream
and --use-fake-ui-for-media-stream at browser launch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Description - WebRTC Video Calling Integration
Problem being solved
The existing active session room and video component (
VideoCall.tsx) used static placeholder layouts and simulated peer joining and leaving. To support real collaboration, the system needed a functional peer-to-peer WebRTC video and audio channel allowing Seekers and Experts to communicate dynamically in real-time.Implemented Solution
We integrated a real peer-to-peer WebRTC connection using native browser APIs and a Next.js polling signaling channel:
/api/session/[id]/signalthat handles signaling messages (offers, answers, ICE candidates, and status sync) in-memory on the server.VideoCall.tsx): Refactored the video call component to connect to the signaling route, request media stream permissions, bind audio/video tracks to local and remote video elements, handle track replacements for screen sharing, and gracefully handle reconnections if the stream drops.<VideoCall>component directly inside the active session view (/session/[id]), handling inline rendering and Picture-in-Picture layout states.Files and components changed
<VideoCall>in place of the static card and added Picture-in-Picture display support.Technical Decisions Made
RTCPeerConnectionandnavigator.mediaDevices) to keep the codebase lightweight and fully compatible with React 19/Next 15. This avoids compilation and runtime polyfill problems of Node-centric libraries likesimple-peerin modern Next.js bundlers.Tests added or updated
tests/e2e/webrtc.spec.tsusing Playwright, verifying:Validation steps performed
Limitations, assumptions, or follow-up work
Closes #434
Summary by CodeRabbit
New Features
Bug Fixes
Tests