feat: add realtime presence indicators for project collaboration #2954
Conversation
|
@naaa760 is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds realtime user presence: database schema and RLS, Supabase trigger/broadcast, TRPC presence router (join/leave/list/cleanup), client presence state manager and provider, UI component to display online users, and integration into project providers and top bar. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant PD as PresenceDisplay (UI)
participant PM as PresenceManager
participant API as TRPC presenceRouter
participant DB as DB (user_presence)
participant RT as Supabase Realtime
U->>PD: Open project view
PD->>PM: setContext(userId, projectId)
PM->>API: joinProject(projectId)
API->>DB: upsert presence (is_online=true, last_seen=now)
DB-->>RT: trigger presence_changes broadcast
API-->>PM: { success: true }
PM->>API: getProjectPresence(projectId)
API->>DB: SELECT online users
DB-->>API: users list
API-->>PM: presence list
PM-->>PD: render avatars
RT-->>PM: INSERT/UPDATE/DELETE events
PM-->>PD: update UI
sequenceDiagram
autonumber
actor U as User
participant PM as PresenceManager
participant API as TRPC presenceRouter
participant DB as DB (user_presence)
participant RT as Supabase Realtime
U->>PM: navigate away / dispose
PM->>API: leaveProject()
API->>DB: update presence (is_online=false, updated_at=now)
DB-->>RT: trigger presence_changes broadcast
PM->>PM: unsubscribe from realtime
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 18
🧹 Nitpick comments (2)
apps/web/client/src/server/api/routers/presence.ts (1)
41-59: Consider adding access control check for consistency.While the update is scoped to the user's own presence record, adding an access control check (like in
joinProject) would ensure consistency and prevent users from leaving projects they never had access to.leaveProject: protectedProcedure .input(z.object({ projectId: z.string() })) .mutation(async ({ ctx, input }) => { const { projectId } = input; const userId = ctx.user.id; + + const projectAccess = await ctx.db.query.userProjects.findFirst({ + where: and( + eq(userProjects.userId, userId), + eq(userProjects.projectId, projectId) + ), + }); + + if (!projectAccess) { + throw new TRPCError({ + code: 'FORBIDDEN', + message: 'User does not have access to this project' + }); + } await ctx.db.update(userPresence)apps/web/client/src/components/store/presence/manager.ts (1)
62-75: Ensure cleanup always happens even on API error.If
leaveProject.mutatefails, the cleanup (unsubscribe, clear state) still occurs, which is correct. However, consider using afinallyblock to guarantee cleanup regardless of success or failure.async leaveProject() { if (!this.currentProjectId || !this.currentUserId) return; + const projectId = this.currentProjectId; + try { - await api.presence.leaveProject.mutate({ projectId: this.currentProjectId }); - + await api.presence.leaveProject.mutate({ projectId }); + } catch (error) { + console.error('Error leaving project:', error); + } finally { this.unsubscribeFromPresenceUpdates(); this.onlineUsers = []; this.currentProjectId = null; this.isConnected = false; - } catch (error) { - console.error('Error leaving project:', error); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/backend/supabase/migrations/0008_user_presence.sql(1 hunks)apps/web/client/src/app/project/[id]/_components/presence-display.tsx(1 hunks)apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx(2 hunks)apps/web/client/src/app/project/[id]/providers.tsx(2 hunks)apps/web/client/src/components/store/presence/index.ts(1 hunks)apps/web/client/src/components/store/presence/manager.ts(1 hunks)apps/web/client/src/components/store/presence/provider.tsx(1 hunks)apps/web/client/src/server/api/root.ts(2 hunks)apps/web/client/src/server/api/routers/index.ts(1 hunks)apps/web/client/src/server/api/routers/presence.ts(1 hunks)packages/db/src/schema/user/index.ts(1 hunks)packages/db/src/schema/user/presence.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/db/src/schema/user/index.tsapps/web/client/src/server/api/routers/presence.tsapps/web/client/src/components/store/presence/manager.tsapps/web/client/src/components/store/presence/index.tsapps/web/client/src/server/api/root.tsapps/web/client/src/app/project/[id]/_components/presence-display.tsxapps/web/client/src/server/api/routers/index.tsapps/web/client/src/components/store/presence/provider.tsxpackages/db/src/schema/user/presence.tsapps/web/client/src/app/project/[id]/providers.tsxapps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/db/src/schema/user/index.tsapps/web/client/src/server/api/routers/presence.tsapps/web/client/src/components/store/presence/manager.tsapps/web/client/src/components/store/presence/index.tsapps/web/client/src/server/api/root.tsapps/web/client/src/app/project/[id]/_components/presence-display.tsxapps/web/client/src/server/api/routers/index.tsapps/web/client/src/components/store/presence/provider.tsxpackages/db/src/schema/user/presence.tsapps/web/client/src/app/project/[id]/providers.tsxapps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/presence.tsapps/web/client/src/server/api/routers/index.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/server/api/routers/presence.tsapps/web/client/src/components/store/presence/manager.tsapps/web/client/src/components/store/presence/index.tsapps/web/client/src/server/api/root.tsapps/web/client/src/app/project/[id]/_components/presence-display.tsxapps/web/client/src/server/api/routers/index.tsapps/web/client/src/components/store/presence/provider.tsxapps/web/client/src/app/project/[id]/providers.tsxapps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/server/api/root.ts
📄 CodeRabbit inference engine (AGENTS.md)
Export all tRPC routers from apps/web/client/src/server/api/root.ts
Export all tRPC routers from src/server/api/root.ts
Files:
apps/web/client/src/server/api/root.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/presence-display.tsxapps/web/client/src/app/project/[id]/providers.tsxapps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/presence-display.tsxapps/web/client/src/components/store/presence/provider.tsxapps/web/client/src/app/project/[id]/providers.tsxapps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/presence-display.tsxapps/web/client/src/app/project/[id]/providers.tsxapps/web/client/src/app/project/[id]/_components/top-bar/index.tsx
🧠 Learnings (4)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/presence.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/routers/**/*.ts : Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Applied to files:
apps/web/client/src/server/api/routers/presence.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from apps/web/client/src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/root.tsapps/web/client/src/server/api/routers/index.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from src/server/api/root.ts
Applied to files:
apps/web/client/src/server/api/root.tsapps/web/client/src/server/api/routers/index.ts
🧬 Code graph analysis (7)
apps/web/client/src/server/api/routers/presence.ts (3)
apps/web/client/src/server/api/trpc.ts (1)
protectedProcedure(131-150)packages/db/src/schema/user/user-project.ts (1)
userProjects(10-23)packages/db/src/schema/user/presence.ts (1)
userPresence(7-23)
apps/web/client/src/components/store/presence/manager.ts (1)
apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/server/api/root.ts (1)
apps/web/client/src/server/api/routers/presence.ts (1)
presenceRouter(6-141)
apps/web/client/src/app/project/[id]/_components/presence-display.tsx (1)
apps/web/client/src/components/store/editor/index.tsx (1)
useEditorEngine(10-14)
apps/web/client/src/components/store/presence/provider.tsx (1)
apps/web/client/src/components/store/presence/manager.ts (1)
PresenceManager(12-163)
packages/db/src/schema/user/presence.ts (2)
packages/db/src/schema/project/project.ts (1)
projects(13-32)packages/db/src/schema/user/user.ts (1)
users(12-25)
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx (1)
apps/web/client/src/app/project/[id]/_components/presence-display.tsx (1)
PresenceDisplay(15-74)
🔇 Additional comments (28)
packages/db/src/schema/user/index.ts (1)
1-1: LGTM!The export follows the established pattern and correctly exposes the presence module through the user schema barrel.
apps/backend/supabase/migrations/0008_user_presence.sql (2)
62-86: LGTM! Realtime broadcast logic is correct.The trigger function correctly:
- Handles INSERT, UPDATE, and DELETE operations
- Uses
COALESCE(NEW.project_id, OLD.project_id)to get the project_id for all operations- Broadcasts to a project-specific channel (
presence:${project_id})- Passes operation type and row data to the realtime system
14-27: RLS policies are well-designed.The policies correctly implement:
- View access: Users can see presence for projects they have access to (via
user_projectsjoin)- Manage access: Users can only manage their own presence records (both USING and WITH CHECK clauses)
This follows the principle of least privilege while enabling the required functionality.
apps/web/client/src/app/project/[id]/providers.tsx (1)
20-22: LGTM! PresenceProvider integration is correct.The provider is properly nested within the existing provider hierarchy, allowing presence features to access project context and editor state as needed.
apps/web/client/src/server/api/routers/index.ts (1)
7-7: LGTM! Router export follows the established pattern.The presence router export is correctly added to the barrel file, making it available for registration in the root router.
apps/web/client/src/app/project/[id]/_components/top-bar/index.tsx (1)
57-57: LGTM! PresenceDisplay placement is logical.The component is positioned appropriately in the UI flow, between collaborative features (Members) and the current user avatar, with consistent spacing.
apps/web/client/src/server/api/root.ts (1)
9-9: LGTM! Presence router is correctly registered.The router is properly imported and registered in the
appRouter, following the established pattern and exposing presence procedures to the client.Based on learnings
Also applies to: 39-39
apps/web/client/src/components/store/presence/index.ts (1)
1-1: LGTM! Public API exports are clean and follow conventions.The barrel export correctly exposes the presence context provider and hook, following the established pattern for store modules in the codebase.
apps/web/client/src/app/project/[id]/_components/presence-display.tsx (3)
25-27: LGTM!The early return logic correctly hides the presence display when not connected or when there are no other online users. The MobX observer wrapper ensures the component reactively updates when these conditions change.
19-23: Confirm effect dependencies foreditorEngine.user.
Unable to locate a definition foreditorEngine.user; please verify whether its object reference can change and, if so, includeeditorEngine.userin the effect’s dependency array instead of onlyeditorEngine.user?.id.
32-73: LGTM!The rendering logic correctly displays up to 3 online users with avatars, tooltips, and an online indicator badge. The overflow button with tooltip for additional users is well-implemented and provides a good user experience.
apps/web/client/src/components/store/presence/provider.tsx (2)
30-36: LGTM!The hook follows the standard pattern for context consumption with a clear error message when used outside the provider.
30-36: LGTM!The
usePresenceManagerhook correctly retrieves the manager from context and throws a clear error if used outside a provider, following the established pattern used in other store hooks likeuseEditorEngine.apps/web/client/src/server/api/routers/presence.ts (6)
104-126: LGTM!The procedure correctly queries the user's own presence across all projects without requiring additional access control checks. The return format is suitable for tRPC serialization.
7-39: LGTM with verification pending!The
joinProjectprocedure correctly validates project access, uses Zod input validation, and employsonConflictDoUpdateto refresh presence on re-join. Assuming theidfield concern is resolved, the logic is sound.
41-59: LGTM!The
leaveProjectprocedure correctly marks the user as offline and updates timestamps. The logic is straightforward and appropriate.
61-102: LGTM!The
getProjectPresenceprocedure correctly validates project access, fetches online users with necessary user fields, and handles the display name fallback gracefully. The ordering bylastSeendescending is a good UX touch.
104-126: LGTM!The
getMyPresenceprocedure correctly fetches the current user's presence across all projects and returns relevant project and presence details.
24-36: No explicitidneeded for user_presence inserts Theuser_presence.idcolumn defaults togen_random_uuid()in the migration, so omittingidon insert is correct.Likely an incorrect or invalid review comment.
apps/web/client/src/components/store/presence/manager.ts (9)
37-40: LGTM!The method correctly sets the context properties atomically, triggering the reaction when
currentProjectIdchanges.
1-10: LGTM!The imports and
PresenceUserinterface are well-defined and appropriately typed.
12-35: LGTM!The constructor correctly sets up MobX observability and a reaction to automatically join/leave projects when
currentProjectIdchanges. This is a clean declarative approach.
42-60: LGTM!The
joinProjectmethod correctly sequences the join mutation, loading initial presence, subscribing to updates, and setting the connected state. Error handling with try-catch and finally for loading state is appropriate.
62-75: LGTM!The
leaveProjectmethod correctly calls the leave mutation, unsubscribes, and clears state. Error handling is appropriate.
77-89: LGTM!The
loadProjectPresencemethod correctly fetches and maps initial presence data with error handling.
91-112: LGTM!The
subscribeToPresenceUpdatesmethod correctly sets up a Supabase realtime subscription with appropriate filtering and event handling. Unsubscribing before subscribing prevents duplicate subscriptions.
114-119: LGTM!The
unsubscribeFromPresenceUpdatesmethod correctly cleans up the subscription.
152-163: LGTM!The computed
otherOnlineUsersgetter andisUserOnlinehelper are correctly implemented. Thedisposemethod appropriately delegates toleaveProjectfor cleanup.
| CREATE OR REPLACE FUNCTION public.cleanup_offline_users() | ||
| RETURNS void | ||
| LANGUAGE plpgsql | ||
| SECURITY DEFINER | ||
| AS $$ | ||
| BEGIN | ||
| UPDATE public.user_presence | ||
| SET is_online = false, updated_at = now() | ||
| WHERE is_online = true | ||
| AND last_seen < now() - interval '5 minutes'; | ||
| END; | ||
| $$; |
There was a problem hiding this comment.
🧩 Analysis chain
Ensure cleanup_offline_users is invoked periodically.
The cleanup_offline_users function marks users as offline after 5 minutes of inactivity, but there's no scheduler or cron job configured in this migration to call it periodically. Without scheduled execution, stale presence records will persist indefinitely.
Verify that a scheduler (e.g., pg_cron, Supabase Edge Function, or external cron) is configured to call this function. If not, consider adding a scheduled job:
🏁 Script executed:
#!/bin/bash
# Description: Search for presence cleanup scheduling in the codebase
# Search for pg_cron, scheduled jobs, or cleanup invocations
rg -nP --type=sql 'pg_cron|SELECT cron\.|cleanup_offline_users' -A 3 -B 3
# Search for Edge Functions or serverless schedulers
fd -e ts -e js 'cron|schedule|cleanup' --exec rg -nP 'cleanup_offline_users|presence.*cleanup' {} -A 3 -B 3Length of output: 527
Schedule cleanup_offline_users to run periodically. No invocation was found; add a pg_cron job, Supabase scheduled function, or equivalent to call this function every 5 minutes.
🤖 Prompt for AI Agents
In apps/backend/supabase/migrations/0008_user_presence.sql around lines 49 to
60, the cleanup_offline_users() function is never scheduled; create a periodic
job to invoke it every 5 minutes. Add a migration that (1) ensures the pg_cron
extension exists (or uses Supabase scheduled functions if pg_cron isn't
available), and (2) creates a cron job that runs every 5 minutes and executes
the function (e.g., schedule a SQL statement that calls SELECT
public.cleanup_offline_users();). Also ensure the job runs with appropriate
privileges (owner or SECURITY DEFINER context) and include a DROP JOB or
idempotent guard so the migration can be re-run safely.
| export const PresenceDisplay = observer(({ className = '' }: PresenceDisplayProps) => { | ||
| const presenceManager = usePresenceManager(); | ||
| const editorEngine = useEditorEngine(); | ||
|
|
||
| useEffect(() => { | ||
| if (editorEngine.user && editorEngine.projectId) { | ||
| presenceManager.setContext(editorEngine.user.id, editorEngine.projectId); | ||
| } | ||
| }, [editorEngine.user?.id, editorEngine.projectId, presenceManager]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid placing MobX store instances in effect dependency arrays.
Line 23 includes presenceManager in the dependency array, which can cause unnecessary re-runs if the manager reference changes (even though it's stable via the provider). According to coding guidelines, avoid placing MobX store instances in effect dependencies if it causes loops or unnecessary re-execution.
Apply this diff to remove presenceManager from the dependency array:
- }, [editorEngine.user?.id, editorEngine.projectId, presenceManager]);
+ }, [editorEngine.user?.id, editorEngine.projectId]);As per coding guidelines.
📝 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.
| export const PresenceDisplay = observer(({ className = '' }: PresenceDisplayProps) => { | |
| const presenceManager = usePresenceManager(); | |
| const editorEngine = useEditorEngine(); | |
| useEffect(() => { | |
| if (editorEngine.user && editorEngine.projectId) { | |
| presenceManager.setContext(editorEngine.user.id, editorEngine.projectId); | |
| } | |
| }, [editorEngine.user?.id, editorEngine.projectId, presenceManager]); | |
| export const PresenceDisplay = observer(({ className = '' }: PresenceDisplayProps) => { | |
| const presenceManager = usePresenceManager(); | |
| const editorEngine = useEditorEngine(); | |
| useEffect(() => { | |
| if (editorEngine.user && editorEngine.projectId) { | |
| presenceManager.setContext(editorEngine.user.id, editorEngine.projectId); | |
| } | |
| }, [editorEngine.user?.id, editorEngine.projectId]); |
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/presence-display.tsx around
lines 15 to 23, the effect currently lists presenceManager in its dependency
array which can trigger unnecessary re-runs; remove presenceManager from the
dependency array and keep only editorEngine.user?.id and editorEngine.projectId,
and to satisfy linting add a single-line disable comment (//
eslint-disable-next-line react-hooks/exhaustive-deps) immediately above the
useEffect to document the intentional omission; ensure the effect still
references presenceManager inside its body so it uses the stable provided
instance.
| <AvatarFallback className="text-xs bg-muted text-muted-foreground"> | ||
| {user.displayName.split(' ').map(n => n[0]).join('').slice(0, 2)} |
There was a problem hiding this comment.
Handle empty displayName edge case.
If displayName is an empty string, the fallback will produce an empty string. Consider providing a default fallback like "?" or "U" when displayName is empty or when the computed initials are empty.
Apply this diff to add a fallback:
<AvatarFallback className="text-xs bg-muted text-muted-foreground">
- {user.displayName.split(' ').map(n => n[0]).join('').slice(0, 2)}
+ {user.displayName.split(' ').map(n => n[0]).join('').slice(0, 2) || '?'}
</AvatarFallback>📝 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.
| <AvatarFallback className="text-xs bg-muted text-muted-foreground"> | |
| {user.displayName.split(' ').map(n => n[0]).join('').slice(0, 2)} | |
| <AvatarFallback className="text-xs bg-muted text-muted-foreground"> | |
| {user.displayName | |
| .split(' ') | |
| .map(n => n[0]) | |
| .join('') | |
| .slice(0, 2) || '?'} | |
| </AvatarFallback> |
🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/presence-display.tsx around
lines 39-40, the AvatarFallback currently derives initials from user.displayName
without handling empty or missing displayName so it can render an empty string;
change the logic to compute initials defensively (e.g. const initials =
(user.displayName ?? '').trim().split(/\s+/).map(n => n[0]).join('').slice(0,2)
) and render a default fallback (such as '?' or 'U') when initials is empty by
using initials || '?' so the fallback always shows a visible character.
| <Avatar className="h-6 w-6 border-2 border-background-primary"> | ||
| <AvatarImage src={user.avatarUrl || undefined} alt={user.displayName} /> | ||
| <AvatarFallback className="text-xs bg-muted text-muted-foreground"> | ||
| {user.displayName.split(' ').map(n => n[0]).join('').slice(0, 2)} |
There was a problem hiding this comment.
Potential runtime error with undefined array element access.
Line 40 constructs initials by accessing n[0] for each word in displayName.split(' '). If displayName contains consecutive spaces or trailing spaces, split(' ') can produce empty strings, and n[0] will be undefined, causing the initials to include undefined in the string.
Apply this diff to filter out empty strings before mapping:
- {user.displayName.split(' ').map(n => n[0]).join('').slice(0, 2)}
+ {user.displayName.split(' ').filter(n => n).map(n => n[0]).join('').slice(0, 2)}🤖 Prompt for AI Agents
In apps/web/client/src/app/project/[id]/_components/presence-display.tsx around
line 40, the initials generation accesses n[0] directly which can be undefined
when displayName has consecutive or trailing spaces; update the expression to
first normalize/filter the split results (e.g., trim the displayName and split
on whitespace or filter out empty strings) before mapping to the first character
of each word and then join/slice, so you never access n[0] on an empty string or
undefined.
|
|
||
| isLoading = false; | ||
|
|
||
| private subscription: any = null; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any type for subscription with a more specific type.
Line 20 declares subscription as any, which violates the coding guideline to avoid any unless necessary. The Supabase client returns a RealtimeChannel type that can be used here.
Apply this diff to improve type safety:
+import type { RealtimeChannel } from '@supabase/supabase-js';
+
export class PresenceManager {
// ...
- private subscription: any = null;
+ private subscription: RealtimeChannel | null = null;As per coding guidelines.
📝 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.
| private subscription: any = null; | |
| import type { RealtimeChannel } from '@supabase/supabase-js'; | |
| export class PresenceManager { | |
| // ... | |
| private subscription: RealtimeChannel | null = null; | |
| // ... | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/presence/manager.ts around line 20, the
subscription is typed as `any`; change it to the Supabase realtime channel type
by importing and using `RealtimeChannel` and declaring the field as `private
subscription: RealtimeChannel | null = null;` (or the appropriate nullable alias
used in the codebase), update any assignments and checks to respect the new
type, and add the import from '@supabase/supabase-js' (or the correct Supabase
types entry) so the file compiles with stronger typing.
| if (!projectAccess) { | ||
| throw new Error('User does not have access to this project'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use TRPCError for consistent error handling.
Throwing a generic Error does not leverage tRPC's error handling. Use TRPCError with an appropriate code (e.g., FORBIDDEN) for consistent error responses.
Apply this diff:
+import { TRPCError } from '@trpc/server';
+
export const presenceRouter = createTRPCRouter({
joinProject: protectedProcedure
.input(z.object({ projectId: z.string() }))
.mutation(async ({ ctx, input }) => {
const { projectId } = input;
const userId = ctx.user.id;
const projectAccess = await ctx.db.query.userProjects.findFirst({
where: and(
eq(userProjects.userId, userId),
eq(userProjects.projectId, projectId)
),
});
if (!projectAccess) {
- throw new Error('User does not have access to this project');
+ throw new TRPCError({
+ code: 'FORBIDDEN',
+ message: 'User does not have access to this project'
+ });
}🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/presence.ts around lines 20 to 22,
replace the thrown generic Error with a tRPC-specific error: import TRPCError
from '@trpc/server' (or add to existing imports) and throw new TRPCError({ code:
'FORBIDDEN', message: 'User does not have access to this project' }) instead of
throw new Error(...), so the router returns a consistent tRPC error shape.
| await ctx.db.insert(userPresence).values({ | ||
| projectId, | ||
| userId, | ||
| isOnline: true, | ||
| lastSeen: new Date(), | ||
| }).onConflictDoUpdate({ | ||
| target: [userPresence.projectId, userPresence.userId], | ||
| set: { | ||
| isOnline: true, | ||
| lastSeen: new Date(), | ||
| updatedAt: new Date(), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Provide id field in insert values.
The userPresence table has id: uuid().primaryKey() but the insert does not provide an id value. Unless the database has a default UUID generator (e.g., gen_random_uuid()), the insert will fail.
Apply this diff to generate a UUID:
+import { randomUUID } from 'crypto';
+
await ctx.db.insert(userPresence).values({
+ id: randomUUID(),
projectId,
userId,
isOnline: true,
lastSeen: new Date(),
}).onConflictDoUpdate({📝 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.
| await ctx.db.insert(userPresence).values({ | |
| projectId, | |
| userId, | |
| isOnline: true, | |
| lastSeen: new Date(), | |
| }).onConflictDoUpdate({ | |
| target: [userPresence.projectId, userPresence.userId], | |
| set: { | |
| isOnline: true, | |
| lastSeen: new Date(), | |
| updatedAt: new Date(), | |
| }, | |
| }); | |
| // at the top of the file, alongside your other imports | |
| import { randomUUID } from 'crypto'; | |
| await ctx.db.insert(userPresence).values({ | |
| id: randomUUID(), | |
| projectId, | |
| userId, | |
| isOnline: true, | |
| lastSeen: new Date(), | |
| }).onConflictDoUpdate({ | |
| target: [userPresence.projectId, userPresence.userId], | |
| set: { | |
| isOnline: true, | |
| lastSeen: new Date(), | |
| updatedAt: new Date(), | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/presence.ts around lines 24 to 36, the
insert into userPresence omits the id column (the table defines id as
uuid().primaryKey()), so the insert will fail unless the DB generates a UUID;
generate a UUID in the application and include it in the insert values (e.g.,
call a UUID generator such as Node's crypto.randomUUID() or your project's UUID
helper) and set id: <generated-uuid> in the values object; keep the
onConflictDoUpdate target and set fields unchanged and ensure the inserted id is
a string/UUID type matching the DB schema.
| if (!projectAccess) { | ||
| throw new Error('User does not have access to this project'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use TRPCError for consistent error handling.
Same issue as joinProject: use TRPCError with code FORBIDDEN instead of generic Error.
Apply this diff:
if (!projectAccess) {
- throw new Error('User does not have access to this project');
+ throw new TRPCError({
+ code: 'FORBIDDEN',
+ message: 'User does not have access to this project'
+ });
}📝 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.
| if (!projectAccess) { | |
| throw new Error('User does not have access to this project'); | |
| } | |
| if (!projectAccess) { | |
| throw new TRPCError({ | |
| code: 'FORBIDDEN', | |
| message: 'User does not have access to this project' | |
| }); | |
| } |
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/presence.ts around lines 73 to 75,
replace the generic Error thrown when a user lacks project access with a
TRPCError using code 'FORBIDDEN' to match project-wide error handling; update
the throw to throw new TRPCError({ code: 'FORBIDDEN', message: 'User does not
have access to this project' }) and ensure TRPCError is imported from
'@trpc/server' at the top of the file if it isn't already.
| cleanupOffline: protectedProcedure | ||
| .mutation(async ({ ctx }) => { | ||
| await ctx.db.update(userPresence) | ||
| .set({ | ||
| isOnline: false, | ||
| updatedAt: new Date(), | ||
| }) | ||
| .where(and( | ||
| eq(userPresence.isOnline, true), | ||
| )); | ||
|
|
||
| return { success: true }; | ||
| }), |
There was a problem hiding this comment.
Critical: Missing authorization for admin endpoint.
The cleanupOffline procedure marks ALL online users across ALL projects as offline without any authorization check. Any authenticated user can call this endpoint and disconnect everyone.
This appears to be a maintenance/admin endpoint. Consider one of these solutions:
Option 1: Restrict to admin users
cleanupOffline: protectedProcedure
.mutation(async ({ ctx }) => {
+ if (ctx.user.role !== 'admin') {
+ throw new TRPCError({ code: 'FORBIDDEN', message: 'Admin access required' });
+ }
+
await ctx.db.update(userPresence)Option 2: Scope to current user only
cleanupOffline: protectedProcedure
.mutation(async ({ ctx }) => {
await ctx.db.update(userPresence)
.set({
isOnline: false,
updatedAt: new Date(),
})
.where(and(
eq(userPresence.isOnline, true),
+ eq(userPresence.userId, ctx.user.id),
));Option 3: Move to internal cron job
Remove this procedure from the public API and implement as a server-side cron job or scheduled task.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/presence.ts around lines 128–140, the
cleanupOffline mutation currently marks all users offline with no authorization
check; restrict or remove this admin maintenance endpoint. Fix by either (A)
adding an admin-only guard: require ctx.session.user.role === 'admin' (or use
your existing isAdmin helper) and throw an unauthorized error if not admin, then
keep the bulk update; or (B) scope behavior to the caller: update only records
for ctx.session.user.id (or project-scoped IDs) instead of all records; or (C)
remove this procedure from the public API and implement the bulk update as a
server-side scheduled job (cron) that runs with service credentials. Ensure any
chosen path also logs the actor and uses transactions/timeouts as appropriate.
Consider scoping cleanupOffline to avoid unintended global impact.
Lines 128-140 define cleanupOffline as a mutation that marks all online users offline across the entire database. This is a broad operation that could unintentionally affect users in unrelated projects if called by any authenticated user.
Consider one of the following approaches:
- Restrict this procedure to admin users only (add role check in middleware).
- Scope the cleanup to a specific project (add
projectIdinput and filter by it). - Implement this as a scheduled background job rather than a user-callable procedure.
If this is intended as an admin-only cleanup tool, apply this diff to add an admin check:
cleanupOffline: protectedProcedure
.mutation(async ({ ctx }) => {
+ // Add admin check
+ if (ctx.user.role !== 'admin') {
+ throw new TRPCError({ code: 'FORBIDDEN', message: 'Admin access required' });
+ }
+
await ctx.db.update(userPresence)Alternatively, if this is for project-specific cleanup, add input validation and filtering:
cleanupOffline: protectedProcedure
+ .input(z.object({ projectId: z.string() }))
- .mutation(async ({ ctx }) => {
+ .mutation(async ({ ctx, input }) => {
await ctx.db.update(userPresence)
.set({
isOnline: false,
updatedAt: new Date(),
})
.where(and(
+ eq(userPresence.projectId, input.projectId),
eq(userPresence.isOnline, true),
));🤖 Prompt for AI Agents
In apps/web/client/src/server/api/routers/presence.ts around lines 128-140, the
cleanupOffline mutation currently marks all online users offline globally;
change it so it cannot affect unrelated data by either (A) adding an admin role
check in the procedure middleware and returning a 403 if the caller is not an
admin, or (B) scoping the mutation to a specific project by adding a validated
input { projectId: string } and adding a where clause to the DB update that
includes eq(userPresence.projectId, input.projectId); choose one approach and
implement the corresponding input/authorization check and DB filter (or move
this logic to a scheduled background job if preferred).
| import { users } from './user'; | ||
|
|
||
| export const userPresence = pgTable('user_presence', { | ||
| id: uuid('id').primaryKey(), |
There was a problem hiding this comment.
Add .defaultRandom() to the id field.
The id field is missing .defaultRandom(), which means inserts without an explicit id will fail. The migration in apps/backend/supabase/migrations/0008_user_presence.sql uses DEFAULT gen_random_uuid(), so the schema should match.
Apply this diff:
- id: uuid('id').primaryKey(),
+ id: uuid('id').primaryKey().defaultRandom(),📝 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.
| id: uuid('id').primaryKey(), | |
| id: uuid('id').primaryKey().defaultRandom(), |
🤖 Prompt for AI Agents
In packages/db/src/schema/user/presence.ts around line 8, the uuid column
definition currently reads id: uuid('id').primaryKey() and lacks a default
value; update the column to chain .defaultRandom() (e.g.,
uuid('id').defaultRandom().primaryKey()) so inserts without an explicit id use a
generated UUID and match the migration (DEFAULT gen_random_uuid()).
Description
Related Issues
Closes #2945 - Add realtime presence feature
Type of Change
Testing
Important
Adds real-time presence indicators for project collaboration with backend and frontend support.
user_presencetable in0008_user_presence.sqlfor tracking user presence in projects.0008_user_presence.sql.update_user_presence,cleanup_offline_users, andpresence_changesin0008_user_presence.sql.PresenceDisplaycomponent inpresence-display.tsxto show online users.PresenceDisplayintoTopBarinindex.tsx.PresenceProviderinproviders.tsxfor managing presence state.PresenceManagerclass inmanager.tsfor handling presence logic.presenceRouterinpresence.tswith procedures for joining, leaving, and querying project presence.presenceRouterinroot.tsandindex.ts.This description was created by
for e9cf1aa. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit