Skip to content

Conversation

@Tobshub
Copy link
Owner

@Tobshub Tobshub commented Oct 25, 2025

Summary by CodeRabbit

Release Notes

  • New Features
    • Real-time incoming call notifications with modal controls to accept or deny calls
    • Call buttons integrated into conversation list for direct calling
    • Optimistic message display with live status updates while sending
    • Automatic call connection via URL parameters for streamlined access

@vercel
Copy link

vercel bot commented Oct 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
tutorlink Error Error Oct 25, 2025 3:05pm
tutorlink-dczq Error Error Oct 25, 2025 3:05pm

@coderabbitai
Copy link

coderabbitai bot commented Oct 25, 2025

Walkthrough

This pull request implements a real-time calling feature using Pusher for signaling. It adds a call router to the tRPC API, introduces a client-side dashboard layout that subscribes to Pusher for incoming calls, displays a modal for call interactions, integrates call initiation from the conversation list, adds URL-driven auto-call functionality to the calls page, and replaces socket.io with WebSocket for message delivery.

Changes

Cohort / File(s) Summary
Dependencies
package.json
Added Pusher (^5.2.0) and Pusher JS (^8.4.0) as new dependencies.
Server-side Pusher & Call Infrastructure
src/server/pusher.ts, src/server/api/routers/call.ts, src/server/api/root.ts
New Pusher client module instantiation and initialization. New tRPC call router with three procedures (initiateCall, answerCall, denyCall) that trigger Pusher private channel events. Call router registered in appRouter.
Server-side Chat Updates
src/server/api/routers/chat.ts
Enhanced sendMessage to resolve sender user by clerkUid; added error handling for user lookup. Minor formatting adjustments to imports and field references.
Dashboard Layout & Client-side Call Wiring
src/app/dashboard/layout.tsx, src/app/dashboard/_components/dashboard-client-layout.tsx
New DashboardClientLayout component that subscribes to Pusher private channel on user authentication, handles incoming-call/call-answered/call-denied events, renders CallModal for incoming calls, and triggers answer/deny mutations. Layout wrapped children with DashboardClientLayout.
Call UI Components
src/app/dashboard/_components/call-modal.tsx, src/app/dashboard/_components/conversation-list.tsx
New CallModal component displaying fullscreen overlay with caller name and Deny/Answer buttons. Conversation list enhanced with Call button that invokes initiateCall mutation with target user's clerkUid and current user's full name.
Call & Messaging Pages
src/app/dashboard/calls/page.tsx, src/app/dashboard/messages/[conversationId]/page.tsx
Calls page now auto-initiates outgoing call when userId URL parameter is present. Messages page replaced socket.io with persistent WebSocket, added optimistic message UI (sending/failed states), tRPC-based message sync, improved input handling (Enter key, placeholder), and enhanced error feedback.

Sequence Diagram(s)

sequenceDiagram
    participant User1 as Caller (User A)
    participant API as tRPC API
    participant Pusher as Pusher Server
    participant User2 as Recipient (User B)
    participant UI as Dashboard UI

    User1->>API: initiateCall(to, callerName)
    API->>Pusher: publish to private-user-b: {type: incoming-call, callerName}
    Note over Pusher: Event published
    
    Pusher->>UI: incoming-call event received
    activate UI
    UI->>UI: setState(incomingCall)
    UI->>UI: Render CallModal
    deactivate UI
    
    User2->>UI: Click Answer Button
    UI->>API: answerCall(to)
    API->>Pusher: publish to private-user-a: {type: call-answered}
    Pusher->>UI: call-answered event received
    UI->>UI: Navigate to /dashboard/calls
    
    rect rgba(200, 150, 255, 0.2)
    Note over User1,User2: Alternative: User clicks Deny
    User2->>UI: Click Deny Button
    UI->>API: denyCall(to)
    API->>Pusher: publish to private-user-a: {type: call-denied}
    Pusher->>UI: call-denied event received
    UI->>UI: Clear incomingCall, close modal
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Pusher integration: Verify Pusher client initialization, environment config, and private channel scoping with user IDs
  • DashboardClientLayout lifecycle: Review subscription/unsubscription logic, event binding, and cleanup on user change or unmount
  • WebSocket replacement in messages page: Check WebSocket lifecycle (open, message, error, close), connection persistence in ref, and cleanup handlers
  • Optimistic message UI & state: Verify optimisticMessages state management, sync logic with tRPC refetch, and error state recovery
  • Call flow wiring: Ensure initiateCall → Pusher event → CallModal → answer/deny → navigation is correctly ordered and state is properly cleared
  • Chat router changes: Confirm clerkUid-based user resolution and error handling don't break existing message flows

Possibly related PRs

  • initial call process #25 — Modifies calls page to implement peer UI/logic; this PR adds URL-driven auto-call initiation to the same file
  • custom chat #29 — Modifies conversation-list and touches real-time infrastructure; overlaps on call button integration and messaging
  • Feat/dashboard wip #28 — Modifies appRouter to add real-time routers; both PRs extend tRPC API with new signaling routes

Poem

🐰 Hop, hop—Pusher calls arise!
Real-time bells, no more goodbyes,
WebSocket streams and modals bright,
Conversations now take flight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Feat/better chat flow" is partially related to the changeset. The title accurately captures one significant aspect of the changes—improvements to the chat messaging flow, including WebSocket-based messaging, optimistic UI for messages, and enhanced message handling. However, the title is incomplete as it omits a substantial set of new features: the real-time call signaling infrastructure (Pusher integration, CallModal component, call router, and call initiation flow). While the PR does include notable chat flow improvements, the calling feature represents a comparable scope of work and is a primary addition to the system.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/better-chat-flow

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/server/api/routers/chat.ts (4)

6-17: Use Clerk UID consistently for filtering the current user.

getConversations filters by User: { some: { id: ctx.user.id }} which assumes DB id. Everywhere else you treat ctx.user.id as Clerk UID. Align to clerkUid to avoid empty results or leakage.

Apply:

-          some: {
-            id: ctx.user.id,
-          },
+          some: { clerkUid: ctx.user.id },

Also applies to: 19-21


27-41: Authorize access to conversation messages.

Anyone who knows a conversationId can fetch messages. Enforce membership.

Apply:

   .query(async ({ ctx, input }) => {
-    return ctx.db.message.findMany({
+    const member = await ctx.db.conversation.findFirst({
+      where: {
+        id: input.conversationId,
+        User: { some: { clerkUid: ctx.user.id } },
+      },
+      select: { id: true },
+    });
+    if (!member) {
+      throw new TRPCError({ code: "FORBIDDEN" });
+    }
+    return ctx.db.message.findMany({
       where: {
         conversationId: input.conversationId,
       },
       include: {
         sender: true,
         conversation: { include: { User: true } },
       },
       orderBy: {
         createdAt: "asc",
       },
     });
   }),

44-65: Validate sender membership and use Clerk UID consistently.

  • Good: resolving sender via clerkUid.
  • Missing: verify the sender belongs to the conversation before insert.

Apply:

   .mutation(async ({ ctx, input }) => {
     const user = await ctx.db.user.findUnique({
       where: { clerkUid: ctx.user.id },
     });
     if (!user) {
       throw new TRPCError({ code: "NOT_FOUND", message: "User not found." });
     }
+    const member = await ctx.db.conversation.findFirst({
+      where: {
+        id: input.conversationId,
+        User: { some: { id: user.id } },
+      },
+      select: { id: true },
+    });
+    if (!member) {
+      throw new TRPCError({ code: "FORBIDDEN" });
+    }
     return ctx.db.message.create({
       data: {
         conversationId: input.conversationId,
         content: input.content,
         senderId: user.id,
       },
     });
   }),

67-104: Fix identifier mismatch in conversation queries.

The getConversations query (line 11) and createConversation findFirst (line 76) both incorrectly use id: ctx.user.id in the WHERE clause, but ctx.user.id contains the Clerk UID, not the database ID. This pattern is inconsistent with other routers (student, tutor, signal) which correctly use clerkUid: ctx.user.id.

Update lines 11 and 76:

-                  id: ctx.user.id,
+                  clerkUid: ctx.user.id,

The connect operation at line 98 is already correct: it uses clerkUid for the current user and id for the tutor (which is a database ID).

🧹 Nitpick comments (5)
package.json (1)

45-46: Pusher deps added — a couple of hygiene checks.

  • Ensure client code imports from pusher-js (never pusher).
  • If Socket.IO is fully replaced, remove socket.io-client to trim bundle.
  • Consider pinning exact versions for infra libs to avoid silent breakage during rollout.
src/app/dashboard/_components/call-modal.tsx (1)

11-26: Add dialog semantics and verify Button variants.

  • Add role="dialog", aria-modal, initial focus, and ESC handling; consider Radix Dialog for accessibility.
  • Confirm your Button supports variant="success"; if not, use default or secondary.

Example:

-  return (
-    <div className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center">
-      <div className="bg-white p-8 rounded-lg">
+  return (
+    <div className="fixed inset-0 bg-black/50 flex items-center justify-center">
+      <div
+        role="dialog"
+        aria-modal="true"
+        aria-label="Incoming call"
+        tabIndex={-1}
+        className="bg-white p-8 rounded-lg outline-none"
+      >
         <h2 className="text-xl font-semibold mb-4">{callerName} is calling...</h2>
         <div className="flex justify-end gap-4">
           <Button variant="destructive" onClick={onDeny}>
             Deny
           </Button>
-          <Button variant="success" onClick={onAnswer}>
+          <Button onClick={onAnswer}>
             Answer
           </Button>
         </div>
       </div>
     </div>
   );
src/app/dashboard/_components/dashboard-client-layout.tsx (1)

17-23: Effect dependencies: avoid re-creating Pusher on route changes.

router is stable; including it as a dependency needlessly re-initializes the connection.

-  }, [user, router]);
+  }, [user]);

Also applies to: 41-41

src/app/dashboard/messages/[conversationId]/page.tsx (2)

12-14: Replace Message = any and remove ts-expect-error by inferring types from tRPC.

This improves safety for optimistic updates and UI accessors (sender.name).

-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-type Message = any;
+type RouterOutputs = inferRouterOutputs<AppRouter>;
+type MessagesOutput = RouterOutputs['chat']['getMessages'];
+type Message = MessagesOutput[number] & { status?: 'sending' | 'failed' };
...
-    { 
+    {
       enabled: !!conversationId,
-      // @ts-expect-error help
-      onSuccess: (data) => {
-        // eslint-disable-next-line
-        setOptimisticMessages(data);
-      }
+      onSuccess: (data) => setOptimisticMessages(data as Message[]),
     }

If chat.getMessages can be empty, ensure Message indexing matches the API contract.

Also applies to: 23-33


1-1: Avoid disabling ESLint for the entire file.

Re-enable ESLint and add targeted disables only where necessary.

-/* eslint-disable */
+/* keep linting enabled; add local eslint-disable-next-line where needed */
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e25aff and bc23086.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (11)
  • package.json (1 hunks)
  • src/app/dashboard/_components/call-modal.tsx (1 hunks)
  • src/app/dashboard/_components/conversation-list.tsx (2 hunks)
  • src/app/dashboard/_components/dashboard-client-layout.tsx (1 hunks)
  • src/app/dashboard/calls/page.tsx (1 hunks)
  • src/app/dashboard/layout.tsx (2 hunks)
  • src/app/dashboard/messages/[conversationId]/page.tsx (3 hunks)
  • src/server/api/root.ts (2 hunks)
  • src/server/api/routers/call.ts (1 hunks)
  • src/server/api/routers/chat.ts (4 hunks)
  • src/server/pusher.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/app/dashboard/_components/dashboard-client-layout.tsx (2)
src/trpc/react.tsx (1)
  • api (25-25)
src/app/dashboard/_components/call-modal.tsx (1)
  • CallModal (11-27)
src/app/dashboard/_components/conversation-list.tsx (2)
src/trpc/react.tsx (1)
  • api (25-25)
src/components/ui/button.tsx (1)
  • Button (63-63)
src/app/dashboard/layout.tsx (1)
src/app/dashboard/_components/dashboard-client-layout.tsx (1)
  • DashboardClientLayout (10-70)
src/server/api/root.ts (1)
src/server/api/routers/call.ts (1)
  • callRouter (5-28)
src/app/dashboard/messages/[conversationId]/page.tsx (1)
src/trpc/react.tsx (1)
  • api (25-25)
src/server/api/routers/call.ts (2)
src/server/api/trpc.ts (2)
  • createTRPCRouter (78-78)
  • protectedProcedure (104-104)
src/server/pusher.ts (1)
  • pusher (4-10)
src/app/dashboard/_components/call-modal.tsx (1)
src/components/ui/button.tsx (1)
  • Button (63-63)

Comment on lines +15 to +17
const handleCall = (to: string, callerName: string) => {
initiateCall.mutate({ to, callerName });
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Navigate caller to the call and harden payload.

  • After initiating, push the caller to /dashboard/calls?userId=<otherUser.clerkUid> so the auto-call path executes.
  • Don’t trust callerName from the client; let the server resolve it from ctx.user (see call router comment).

Apply:

-const handleCall = (to: string, callerName: string) => {
-  initiateCall.mutate({ to, callerName });
-};
+const handleCall = (to: string) => {
+  initiateCall.mutate(
+    { to, callerName: user?.fullName ?? "Unknown" },
+    {
+      onSuccess: () => router.push(`/dashboard/calls?userId=${to}`),
+    },
+  );
+};
-<Button
-  onClick={() => handleCall(otherUser.clerkUid, user.fullName!)}
->
+<Button
+  disabled={initiateCall.isPending}
+  onClick={() => handleCall(otherUser.clerkUid)}
+>
   Call
 </Button>

Note: Once the server derives caller display name, drop callerName from the client call entirely.

Also applies to: 46-52


🏁 Script executed:

#!/bin/bash
# Read the conversation-list component to verify context
cat src/app/dashboard/_components/conversation-list.tsx

Length of output: 1858


🏁 Script executed:

#!/bin/bash
# Search for initiateCall mutation definition
rg -n "initiateCall" --type=ts --type=tsx -B2 -A2 | head -50

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Search for router usage in the component and nearby files
rg -n "useRouter\|router\.push" src/app/dashboard/_components/ --type=ts --type=tsx -B1 -A1

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Find call router definition
find . -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs grep -l "initiateCall" | grep -E "(router|call)" | head -5

Length of output: 32


🏁 Script executed:

#!/bin/bash
# Search for call.ts or similar in trpc routers
fd -e ts -e tsx "call" src/server/api/routers/ 2>/dev/null || fd -e ts "call" src/server/ 2>/dev/null || echo "Looking for api patterns..."

Length of output: 30


🏁 Script executed:

#!/bin/bash
# Search for ctx.user pattern in routers to understand available context
rg "ctx\.user" src/server/ -A2 -B2 --max-count=3 2>/dev/null || rg "ctx\.user" src/ -A2 -B2 --max-count=3

Length of output: 4745


🏁 Script executed:

#!/bin/bash
# Read the call router to understand initiateCall mutation
cat src/server/api/routers/call.ts

Length of output: 886


Navigate caller to the call and harden payload.

  • After initiating, push the caller to /dashboard/calls?userId=<otherUser.clerkUid> so the auto-call path executes.
  • Don't trust callerName from the client; the mutation has access to ctx.user via protectedProcedure and should derive the caller's display name from Clerk data (matching the pattern used in student.ts and tutor.ts).

Apply:

-const handleCall = (to: string, callerName: string) => {
-  initiateCall.mutate({ to, callerName });
-};
+const handleCall = (to: string) => {
+  initiateCall.mutate(
+    { to },
+    {
+      onSuccess: () => router.push(`/dashboard/calls?userId=${to}`),
+    },
+  );
+};
-<Button
-  onClick={() => handleCall(otherUser.clerkUid, user.fullName!)}
->
+<Button
+  disabled={initiateCall.isPending}
+  onClick={() => handleCall(otherUser.clerkUid)}
+>
   Call
 </Button>

Update src/server/api/routers/call.ts:

 initiateCall: protectedProcedure
-  .input(z.object({ to: z.string(), callerName: z.string() }))
-  .mutation(({ input }) => {
+  .input(z.object({ to: z.string() }))
+  .mutation(({ ctx, input }) => {
+    const callerName = `${ctx.user.firstName ?? ""} ${ctx.user.lastName ?? ""}`.trim() || "Unknown";
     pusher.trigger(`private-${input.to}`, 'incoming-call', {
       from: callerName,
     });
     return { success: true };
   }),

Also applies to: 46-52

🤖 Prompt for AI Agents
In src/app/dashboard/_components/conversation-list.tsx lines 15-17 and in
src/server/api/routers/call.ts lines 46-52: after initiating a call, navigate
the caller to /dashboard/calls?userId=<otherUser.clerkUid> so the auto-call path
runs, and remove/trustless the client-supplied callerName—stop passing
callerName from the component and instead have the server derive the caller's
display name from ctx.user (same pattern used in student.ts and tutor.ts).
Update the call router to ignore any callerName in the input schema (or remove
that field), read the authenticated user's Clerk display name from ctx.user, set
that as caller displayName in the created call record/payload, and return the
necessary data; in the component, call initiateCall.mutate({ to:
otherUser.clerkUid }) and on success push the user to
/dashboard/calls?userId=otherUser.clerkUid.

Comment on lines +20 to +25
const pusher = new Pusher(process.env.NEXT_PUBLIC_PUSHER_KEY!, {
cluster: process.env.NEXT_PUBLIC_PUSHER_CLUSTER!,
});

const channel = pusher.subscribe(`private-${user.id}`);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Private channels require auth; add authEndpoint (and force TLS).

Subscribing to private- channels without an authEndpoint will fail. Configure client auth and use TLS.

-    const pusher = new Pusher(process.env.NEXT_PUBLIC_PUSHER_KEY!, {
-      cluster: process.env.NEXT_PUBLIC_PUSHER_CLUSTER!,
-    });
+    const pusher = new Pusher(process.env.NEXT_PUBLIC_PUSHER_KEY!, {
+      cluster: process.env.NEXT_PUBLIC_PUSHER_CLUSTER!,
+      forceTLS: true,
+      authEndpoint: '/api/pusher/auth', // implement server route
+    });

Optionally centralize a singleton Pusher instance to avoid multiple connections per route render.

📝 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 pusher = new Pusher(process.env.NEXT_PUBLIC_PUSHER_KEY!, {
cluster: process.env.NEXT_PUBLIC_PUSHER_CLUSTER!,
});
const channel = pusher.subscribe(`private-${user.id}`);
const pusher = new Pusher(process.env.NEXT_PUBLIC_PUSHER_KEY!, {
cluster: process.env.NEXT_PUBLIC_PUSHER_CLUSTER!,
forceTLS: true,
authEndpoint: '/api/pusher/auth', // implement server route
});
const channel = pusher.subscribe(`private-${user.id}`);
🤖 Prompt for AI Agents
In src/app/dashboard/_components/dashboard-client-layout.tsx around lines 20 to
25, the client currently subscribes to a private channel without auth or TLS;
update the Pusher client initialization to include an authEndpoint and forceTLS:
set authEndpoint to your server auth route (e.g. /api/pusher/auth) and configure
auth headers/cookies as required (Authorization or credentials), and set
forceTLS: true to enforce TLS; additionally, consider centralizing the Pusher
client as a singleton (module-level export) so re-renders don’t create multiple
connections.

Comment on lines +26 to +33
channel.bind('incoming-call', (data: { from: string }) => {
setIncomingCall(data);
});

channel.bind('call-answered', () => {
router.push('/dashboard/calls');
});

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Reply is sent to a name, not a user ID — events won’t reach the caller.

incoming-call sets { from: string } (a display name). handleAnswer/handleDeny then send to that name, constructing private-John Doe which doesn’t exist. Use a stable ID (fromId) for routing and keep fromName for UI.

-  const [incomingCall, setIncomingCall] = useState<{ from: string } | null>(null);
+  const [incomingCall, setIncomingCall] = useState<{ fromId: string; fromName: string } | null>(null);
...
-    channel.bind('incoming-call', (data: { from: string }) => {
-      setIncomingCall(data);
+    channel.bind('incoming-call', (data: { fromId: string; fromName: string }) => {
+      setIncomingCall(data);
     });
...
-  const handleAnswer = () => {
+  const handleAnswer = () => {
     if (incomingCall) {
-      answerCall.mutate({ to: incomingCall.from });
-      router.push('/dashboard/calls');
+      answerCall.mutate(
+        { to: incomingCall.fromId },
+        { onSuccess: () => router.push('/dashboard/calls') }
+      );
       setIncomingCall(null);
     }
   };
...
-  const handleDeny = () => {
+  const handleDeny = () => {
     if (incomingCall) {
-      denyCall.mutate({ to: incomingCall.from });
+      denyCall.mutate({ to: incomingCall.fromId });
       setIncomingCall(null);
     }
   };
...
-          callerName={incomingCall.from}
+          callerName={incomingCall.fromName}

Note: Server must emit { fromId, fromName } as suggested in call.ts. Keep event shapes consistent across all call events.

Also applies to: 43-49, 51-56

🤖 Prompt for AI Agents
In src/app/dashboard/_components/dashboard-client-layout.tsx around lines 26 to
33 (and similarly for lines 43-49 and 51-56), the client is using the human
display name (from) as the destination for private channel messages which breaks
routing; change the event handling and any subsequent send/route logic to use a
stable identifier (fromId) for addressing the caller and preserve fromName (or
from) only for UI display. Update bindings to expect { fromId, fromName } from
the server, use fromId when constructing private channel names or router pushes,
and leave fromName for labels; ensure all call-related event shapes are kept
consistent client/server (and adjust server emitters if needed).

Comment on lines +38 to +41
return () => {
pusher.unsubscribe(`private-${user.id}`);
};
}, [user, router]);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tighten cleanup: unbind events and disconnect the client.

Unsubscribing the channel is good; also unbind handlers and disconnect to prevent leaked connections on route changes.

-    return () => {
-      pusher.unsubscribe(`private-${user.id}`);
-    };
+    return () => {
+      channel.unbind('incoming-call');
+      channel.unbind('call-answered');
+      channel.unbind('call-denied');
+      pusher.unsubscribe(`private-${user.id}`);
+      pusher.disconnect();
+    };
📝 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
return () => {
pusher.unsubscribe(`private-${user.id}`);
};
}, [user, router]);
return () => {
channel.unbind('incoming-call');
channel.unbind('call-answered');
channel.unbind('call-denied');
pusher.unsubscribe(`private-${user.id}`);
pusher.disconnect();
};
}, [user, router]);
🤖 Prompt for AI Agents
In src/app/dashboard/_components/dashboard-client-layout.tsx around lines 38 to
41, the effect cleanup currently only calls
pusher.unsubscribe(`private-${user.id}`); which can leave event handlers and the
underlying connection open; update the cleanup to also unbind event handlers
from the channel (e.g., call channel.unbind_all() or channel.unbind for specific
handlers) before unsubscribing and then call pusher.disconnect() to fully close
the connection, guarding against null/undefined pusher/channel references so the
cleanup is safe on route changes.

Comment on lines +35 to +46
const params = new URLSearchParams(window.location.search);
const remoteId = params.get('userId');
if (remoteId) {
setRemotePeerId(remoteId);
const outgoingCall = peerInstance.call(remoteId, stream);
outgoingCall.on("stream", (remoteStream) => {
if (remoteVideoRef.current) {
remoteVideoRef.current.srcObject = remoteStream;
}
});
setCall(outgoingCall);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid double getUserMedia; reuse a single local stream and auto-call safely.

You’re calling getUserMedia here and again in the “call” listener. Reuse one MediaStream and stop tracks on teardown to release camera/mic.

Apply within this block:

-      startVideoStream((stream) => {
+      startVideoStream((stream) => {
         if (localVideoRef.current) {
           localVideoRef.current.srcObject = stream;
         }
-        const params = new URLSearchParams(window.location.search);
-        const remoteId = params.get('userId');
+        // reuse the same stream elsewhere
+        (call as any)?._localStream = stream;
+        const params = new URLSearchParams(window.location.search);
+        const remoteId = params.get("userId");
         if (remoteId) {
           setRemotePeerId(remoteId);
           const outgoingCall = peerInstance.call(remoteId, stream);
           outgoingCall.on("stream", (remoteStream) => {
             if (remoteVideoRef.current) {
               remoteVideoRef.current.srcObject = remoteStream;
             }
           });
           setCall(outgoingCall);
         }
       });

And outside this range, consider:

// top-level
const localStreamRef = useRef<MediaStream|null>(null);

// whenever you obtain a stream:
localStreamRef.current = stream;

// in 'peerInstance.on("call")' path, reuse:
incomingCall.answer(localStreamRef.current ?? stream);

// cleanup
return () => {
  call?.close();
  localStreamRef.current?.getTracks().forEach(t => t.stop());
  peerInstance.destroy();
};

Optionally, navigate the caller here when answering events to ensure both peers land on /dashboard/calls?userId=.

Comment on lines +5 to +6
export const callRouter = createTRPCRouter({
initiateCall: protectedProcedure
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify Pusher private-channel auth flow is implemented.

This router emits to private- channels. Ensure the client config provides an authEndpoint/authorizer; otherwise all private subscriptions will fail with 401/403. Cross-file: dashboard-client-layout.tsx currently subscribes to private- without auth config.

Would you like a minimal Next.js route handler for Pusher auth?


I need to verify the Pusher private-channel authentication setup across the codebase. Let me search for the relevant files and their implementation.


---

🏁 Script executed:

```shell
#!/bin/bash
# Find and examine the call router and dashboard client layout
fd -type f -name "call.ts" -o -name "dashboard-client-layout.tsx" | head -20

Length of output: 170


🏁 Script executed:

#!/bin/bash
# Search for Pusher channel subscriptions and auth configuration
rg -n "private-|subscribe|authEndpoint|authorizer" --type=ts --type=tsx -C2 | head -100

Length of output: 27


🏁 Script executed:

#!/bin/bash
# Search specifically for Pusher client initialization and configuration
rg -n "Pusher|pusher" --type=ts --type=tsx -C3 | head -100

Length of output: 27


Let me correct the shell syntax and search again:


---

🏁 Script executed:

```shell
#!/bin/bash
# Find the relevant files
fd "call\.ts$" -o "dashboard-client-layout\.tsx$"

Length of output: 177


🏁 Script executed:

#!/bin/bash
# Search for Pusher client setup and auth configuration across TypeScript/JavaScript files
rg -n "Pusher|authEndpoint|authorizer|private-" --type=ts -C2

Length of output: 3092


🏁 Script executed:

#!/bin/bash
# Search for channel subscriptions
rg -n "subscribe\(|channel\(" --type=ts -C2

Length of output: 843


Let me verify if there's an auth endpoint defined elsewhere:


---

🏁 Script executed:

```shell
#!/bin/bash
# Search for any auth endpoint configuration or auth-related Pusher setup
rg -n "auth|Endpoint" src/app/dashboard/_components/dashboard-client-layout.tsx -A5 -B5

Length of output: 0


🏁 Script executed:

#!/bin/bash
# Search for Pusher auth handler routes
rg -n "pusher.*auth|pusher.*channel" --type=ts

Length of output: 0


Add Pusher private-channel authentication to dashboard-client-layout.tsx.

The client subscribes to private channels without authentication (line 24 of src/app/dashboard/_components/dashboard-client-layout.tsx). Pusher requires either an authEndpoint or auth callback in the client config. Without this, all private channel subscriptions will fail with 401/403 errors.

Add an auth endpoint:

const pusher = new Pusher(process.env.NEXT_PUBLIC_PUSHER_KEY!, {
  cluster: process.env.NEXT_PUBLIC_PUSHER_CLUSTER!,
  authEndpoint: '/api/pusher-auth',  // Add this
});

Then create /api/pusher-auth route to handle channel authentication using the server-side Pusher instance.

🤖 Prompt for AI Agents
In src/server/api/routers/call.ts around lines 5-6, the PR needs Pusher
private-channel auth support for the dashboard client: update the client to
instantiate Pusher with authEndpoint '/api/pusher-auth' and add a new server
route POST /api/pusher-auth that reads socket_id and channel_name from the
request body, uses the server-side Pusher instance to authorize the subscription
(call the Pusher server-side authorize method), and returns the resulting auth
JSON to the client; ensure the route is authenticated/permissioned as needed and
returns proper 401/403 on failure and application/json on success.

Comment on lines +7 to +11
.input(z.object({ to: z.string(), callerName: z.string() }))
.mutation(({ input }) => {
pusher.trigger(`private-${input.to}`, 'incoming-call', {
from: input.callerName,
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Spoofing risk and weak validation: derive caller identity server-side and tighten input.

  • Do not trust callerName from client; derive from the authenticated user to prevent impersonation.
  • Validate to to meet Pusher channel constraints (length/charset).

Apply:

-    .input(z.object({ to: z.string(), callerName: z.string() }))
+    .input(z.object({
+      to: z.string()
+        .min(1)
+        .max(200)
+        .regex(/^[A-Za-z0-9_\-=@,.;]+$/, 'Invalid channel segment'),
+    }))

And remove callerName usage; use ctx-auth values when building payloads.

Also applies to: 16-17, 23-24

🤖 Prompt for AI Agents
In src/server/api/routers/call.ts around lines 7-11 (and also apply same changes
to lines 16-17 and 23-24), the handler currently accepts callerName from the
client and uses it in the Pusher payload and channel name — change the input
schema to only accept a validated "to" field (tighten with a regex/length to
match Pusher channel constraints such as allowed charset and max length), remove
callerName from input and payload, and instead derive the caller identity
server-side from the request context (e.g., ctx.auth or
ctx.session.user.{id,name}) when building the Pusher payload; ensure all other
references at lines 16-17 and 23-24 are updated to use the server-derived
identity and that the "to" validation matches Pusher rules.

Comment on lines +8 to +13
.mutation(({ input }) => {
pusher.trigger(`private-${input.to}`, 'incoming-call', {
from: input.callerName,
});
return { success: true };
}),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Await Pusher triggers and handle errors to avoid unhandled rejections.

pusher.trigger returns a Promise. Not awaiting means failures are swallowed and the mutation reports success even if the event wasn’t sent.

Apply:

-    .mutation(({ input }) => {
-      pusher.trigger(`private-${input.to}`, 'incoming-call', {
-        from: input.callerName,
-      });
-      return { success: true };
-    }),
+    .mutation(async ({ ctx, input }) => {
+      try {
+        await pusher.trigger(`private-${input.to}`, 'incoming-call', {
+          // see payload refactor below
+          fromId: ctx.auth.userId,
+          fromName: ctx.auth.user?.fullName ?? ctx.auth.user?.username ?? 'Unknown',
+        });
+        return { success: true };
+      } catch (err) {
+        throw new Error('Failed to send incoming-call event');
+      }
+    }),
📝 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
.mutation(({ input }) => {
pusher.trigger(`private-${input.to}`, 'incoming-call', {
from: input.callerName,
});
return { success: true };
}),
.mutation(async ({ ctx, input }) => {
try {
await pusher.trigger(`private-${input.to}`, 'incoming-call', {
// see payload refactor below
fromId: ctx.auth.userId,
fromName: ctx.auth.user?.fullName ?? ctx.auth.user?.username ?? 'Unknown',
});
return { success: true };
} catch (err) {
throw new Error('Failed to send incoming-call event');
}
}),
🤖 Prompt for AI Agents
In src/server/api/routers/call.ts around lines 8 to 13, the mutation calls
pusher.trigger but doesn't await it, so Promise rejections are swallowed and the
mutation always returns success; change the mutation handler to async, await
pusher.trigger, wrap the await in a try/catch, log the error and return or throw
an appropriate error (e.g., throw a TRPC/internal server error or return {
success: false }) when pusher.trigger fails so callers receive the real result
and no unhandled rejections occur.

Comment on lines +15 to +20
answerCall: protectedProcedure
.input(z.object({ to: z.string() }))
.mutation(({ input }) => {
pusher.trigger(`private-${input.to}`, 'call-answered', {});
return { success: true };
}),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Inconsistent/empty payloads; include fromId/fromName and await trigger for all events.

Send a consistent shape for call-answered and call-denied so clients can correlate who responded. Await triggers and handle errors.

-  answerCall: protectedProcedure
-    .input(z.object({ to: z.string() }))
-    .mutation(({ input }) => {
-      pusher.trigger(`private-${input.to}`, 'call-answered', {});
-      return { success: true };
-    }),
+  answerCall: protectedProcedure
+    .input(z.object({ to: z.string().min(1).max(200).regex(/^[A-Za-z0-9_\-=@,.;]+$/) }))
+    .mutation(async ({ ctx, input }) => {
+      await pusher.trigger(`private-${input.to}`, 'call-answered', {
+        fromId: ctx.auth.userId,
+        fromName: ctx.auth.user?.fullName ?? ctx.auth.user?.username ?? 'Unknown',
+      });
+      return { success: true };
+    }),
 
-  denyCall: protectedProcedure
-    .input(z.object({ to: z.string() }))
-    .mutation(({ input }) => {
-      pusher.trigger(`private-${input.to}`, 'call-denied', {});
-      return { success: true };
-    }),
+  denyCall: protectedProcedure
+    .input(z.object({ to: z.string().min(1).max(200).regex(/^[A-Za-z0-9_\-=@,.;]+$/) }))
+    .mutation(async ({ ctx, input }) => {
+      await pusher.trigger(`private-${input.to}`, 'call-denied', {
+        fromId: ctx.auth.userId,
+      });
+      return { success: true };
+    }),

Also applies to: 22-27

🤖 Prompt for AI Agents
In src/server/api/routers/call.ts around lines 15-20 (and also apply same
changes to lines 22-27), the mutation currently triggers pusher events with
empty payloads and does not await or handle the async trigger; update both
call-answered and call-denied to include a consistent payload containing fromId
and fromName (use the caller's id/name available in context or input), await the
pusher.trigger call (async/await), and wrap it in try/catch to log or surface
errors and return an appropriate success/failure response.

Comment on lines +4 to +10
export const pusher = new Pusher({
appId: env.PUSHER_APP_ID,
key: env.PUSHER_KEY,
secret: env.PUSHER_SECRET,
cluster: env.PUSHER_CLUSTER,
useTLS: true,
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Harden server-only usage and wire private-channel auth.

  • Add a server-only guard to prevent accidental client imports of secrets.
  • Private channels won’t subscribe without an auth endpoint; client snippet shows none. Implement a signing route and configure pusher-js v8 via channelAuthorization.

Apply this guard:

+import "server-only";
 import Pusher from 'pusher';
 import { env } from '@/env';
 
 export const pusher = new Pusher({
   appId: env.PUSHER_APP_ID,
   key: env.PUSHER_KEY,
   secret: env.PUSHER_SECRET,
   cluster: env.PUSHER_CLUSTER,
   useTLS: true,
 });

Implement an auth route (new file):

// src/app/api/pusher/auth/route.ts
import "server-only";
import { NextResponse } from "next/server";
import { auth } from "@clerk/nextjs/server";
import { pusher } from "@/server/pusher";

export async function POST(req: Request) {
  const { userId } = auth();
  if (!userId) return NextResponse.json({ error: "Unauthorized" }, { status: 401 });

  const body = await req.formData();
  const socketId = String(body.get("socket_id") ?? "");
  const channelName = String(body.get("channel_name") ?? "");

  // Authorize only the caller’s own channel
  if (channelName !== `private-${userId}`) {
    return NextResponse.json({ error: "Forbidden" }, { status: 403 });
  }

  const authResponse = pusher.authorizeChannel(socketId, channelName);
  return NextResponse.json(authResponse);
}

And on the client (pusher-js v8):

const p = new Pusher(process.env.NEXT_PUBLIC_PUSHER_KEY!, {
  cluster: process.env.NEXT_PUBLIC_PUSHER_CLUSTER!,
  channelAuthorization: { endpoint: "/api/pusher/auth", transport: "ajax" },
});
🤖 Prompt for AI Agents
In src/server/pusher.ts around lines 4 to 10, the Pusher client is created using
secret env vars but lacks a server-only guard and there is no private-channel
auth endpoint; add "server-only" at the top of this file to prevent accidental
client-side imports and keep secrets server-only, then implement the auth route
at src/app/api/pusher/auth/route.ts that validates the authenticated user,
verifies socket_id and channel_name (only allow channelName ===
`private-${userId}`), calls pusher.authorizeChannel and returns the auth
response, and finally ensure the client uses pusher-js v8 with
channelAuthorization pointing to /api/pusher/auth (transport: "ajax") so private
channels can subscribe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants