Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions apps/backend/supabase/migrations/0008_user_presence.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
CREATE TABLE IF NOT EXISTS public.user_presence (
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
project_id uuid NOT NULL REFERENCES public.projects(id) ON DELETE CASCADE,
user_id uuid NOT NULL REFERENCES public.users(id) ON DELETE CASCADE,
last_seen timestamp with time zone DEFAULT now() NOT NULL,
is_online boolean DEFAULT true NOT NULL,
created_at timestamp with time zone DEFAULT now() NOT NULL,
updated_at timestamp with time zone DEFAULT now() NOT NULL,
UNIQUE(project_id, user_id)
);

ALTER TABLE public.user_presence ENABLE ROW LEVEL SECURITY;

CREATE POLICY "Users can view presence for accessible projects" ON public.user_presence
FOR SELECT
USING (
EXISTS (
SELECT 1 FROM public.user_projects
WHERE user_projects.project_id = user_presence.project_id
AND user_projects.user_id = auth.uid()
)
);

CREATE POLICY "Users can manage their own presence" ON public.user_presence
FOR ALL
USING (user_id = auth.uid())
WITH CHECK (user_id = auth.uid());

CREATE OR REPLACE FUNCTION public.update_user_presence(
p_project_id uuid,
p_user_id uuid,
p_is_online boolean DEFAULT true
)
RETURNS void
LANGUAGE plpgsql
SECURITY DEFINER
AS $$
BEGIN
INSERT INTO public.user_presence (project_id, user_id, is_online, last_seen, updated_at)
VALUES (p_project_id, p_user_id, p_is_online, now(), now())
ON CONFLICT (project_id, user_id)
DO UPDATE SET
is_online = p_is_online,
last_seen = now(),
updated_at = now();
END;
$$;

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;
$$;
Comment on lines +49 to +60
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

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 3

Length 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.


CREATE OR REPLACE FUNCTION public.presence_changes()
RETURNS TRIGGER
SECURITY DEFINER
LANGUAGE plpgsql
AS $$
DECLARE
topic_project_id uuid;
BEGIN
topic_project_id := COALESCE(NEW.project_id, OLD.project_id);

IF topic_project_id IS NOT NULL THEN
PERFORM realtime.broadcast_changes(
'presence:' || topic_project_id::text,
TG_OP,
TG_OP,
TG_TABLE_NAME,
TG_TABLE_SCHEMA,
NEW,
OLD
);
END IF;

RETURN NULL;
END;
$$;

DROP TRIGGER IF EXISTS handle_presence_changes ON public.user_presence;
CREATE TRIGGER handle_presence_changes
AFTER INSERT OR UPDATE OR DELETE
ON public.user_presence
FOR EACH ROW
EXECUTE FUNCTION presence_changes();

DROP POLICY IF EXISTS "Authenticated users can receive presence broadcasts" ON "realtime"."messages";
CREATE POLICY "Authenticated users can receive presence broadcasts"
ON "realtime"."messages"
FOR SELECT
TO authenticated
USING (
CASE
WHEN payload->>'table' = 'user_presence' THEN
EXISTS (
SELECT 1 FROM public.user_projects
WHERE user_projects.project_id = (payload->>'project_id')::uuid
AND user_projects.user_id = auth.uid()
)
ELSE false
END
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
'use client';

import { usePresenceManager } from '@/components/store/presence';
import { useEditorEngine } from '@/components/store/editor';
import { Avatar, AvatarFallback, AvatarImage } from '@onlook/ui/avatar';
import { Button } from '@onlook/ui/button';
import { Tooltip, TooltipContent, TooltipTrigger } from '@onlook/ui/tooltip';
import { observer } from 'mobx-react-lite';
import { useEffect } from 'react';

interface PresenceDisplayProps {
className?: string;
}

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]);
Comment on lines +15 to +23
Copy link

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
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.


if (!presenceManager.isConnected || presenceManager.otherOnlineUsers.length === 0) {
return null;
}

const onlineUsers = presenceManager.otherOnlineUsers;

return (
<div className={`flex items-center gap-1 ${className}`}>
{onlineUsers.slice(0, 3).map((user) => (
<Tooltip key={user.userId}>
<TooltipTrigger asChild>
<div className="relative">
<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)}
Comment on lines +39 to +40
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

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.

Suggested change
<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.

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

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.

</AvatarFallback>
</Avatar>
<div className="absolute -bottom-0.5 -right-0.5 h-3 w-3 rounded-full bg-green-500 border-2 border-background-primary" />
</div>
</TooltipTrigger>
<TooltipContent side="bottom" className="mt-1" hideArrow>
<p className="text-sm">{user.displayName} is online</p>
</TooltipContent>
</Tooltip>
))}

{onlineUsers.length > 3 && (
<Tooltip>
<TooltipTrigger asChild>
<Button
variant="outline"
size="sm"
className="h-6 w-6 rounded-full p-0 text-xs bg-muted hover:bg-muted/80"
>
+{onlineUsers.length - 3}
</Button>
</TooltipTrigger>
<TooltipContent side="bottom" className="mt-1" hideArrow>
<div className="text-sm">
{onlineUsers.slice(3).map(user => (
<div key={user.userId}>{user.displayName}</div>
))}
</div>
</TooltipContent>
</Tooltip>
)}
</div>
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { motion } from 'motion/react';
import { useTranslations } from 'next-intl';
import { useState } from 'react';
import { Members } from '../members';
import { PresenceDisplay } from '../presence-display';
import { BranchDisplay } from './branch';
import { ModeToggle } from './mode-toggle';
import { ProjectBreadcrumb } from './project-breadcrumb';
Expand Down Expand Up @@ -53,6 +54,7 @@ export const TopBar = observer(() => {
<div className={`transition-all duration-200 ${isMembersPopoverOpen ? 'mr-2' : '-mr-2 group-hover:mr-2'}`}>
<Members onPopoverOpenChange={setIsMembersPopoverOpen} />
</div>
<PresenceDisplay className="mr-2" />
<Tooltip>
<TooltipTrigger asChild>
<div className="flex items-center">
Expand Down
5 changes: 4 additions & 1 deletion apps/web/client/src/app/project/[id]/providers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { EditorEngineProvider } from '@/components/store/editor';
import { HostingProvider } from '@/components/store/hosting';
import { PresenceProvider } from '@/components/store/presence';
import type { Branch, Project } from '@onlook/models';

export const ProjectProviders = ({
Expand All @@ -16,7 +17,9 @@ export const ProjectProviders = ({
return (
<EditorEngineProvider project={project} branches={branches}>
<HostingProvider>
{children}
<PresenceProvider>
{children}
</PresenceProvider>
</HostingProvider>
</EditorEngineProvider>
);
Expand Down
1 change: 1 addition & 0 deletions apps/web/client/src/components/store/presence/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { PresenceProvider, usePresenceManager } from './provider';
163 changes: 163 additions & 0 deletions apps/web/client/src/components/store/presence/manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
import { makeAutoObservable, reaction } from 'mobx';
import { api } from '@/trpc/react';
import { createClient } from '@/utils/supabase/client';

export interface PresenceUser {
userId: string;
displayName: string;
avatarUrl?: string | null;
lastSeen: Date;
}

export class PresenceManager {
onlineUsers: PresenceUser[] = [];
currentUserId: string | null = null;
currentProjectId: string | null = null;
isConnected = false;

isLoading = false;

private subscription: any = null;
Copy link

Choose a reason for hiding this comment

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

🛠️ 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.

Suggested change
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.


constructor() {
makeAutoObservable(this);

reaction(
() => this.currentProjectId,
(projectId) => {
if (projectId) {
this.joinProject(projectId);
} else {
this.leaveProject();
}
}
);
Comment on lines +25 to +34
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

Guard against concurrent join/leave operations.

The reaction triggers joinProject or leaveProject without awaiting completion. If currentProjectId changes rapidly, concurrent operations could overlap, leading to race conditions (e.g., joining project A while still leaving project B).

Consider adding a guard:

 export class PresenceManager {
     onlineUsers: PresenceUser[] = [];
     currentUserId: string | null = null;
     currentProjectId: string | null = null;
     isConnected = false;
 
     isLoading = false;
 
     private subscription: any = null;
+    private pendingOperation: Promise<void> | null = null;
 
     constructor() {
         makeAutoObservable(this);
 
         reaction(
             () => this.currentProjectId,
-            (projectId) => {
+            async (projectId) => {
+                await this.pendingOperation;
                 if (projectId) {
-                    this.joinProject(projectId);
+                    this.pendingOperation = this.joinProject(projectId);
                 } else {
-                    this.leaveProject();
+                    this.pendingOperation = this.leaveProject();
                 }
+                await this.pendingOperation;
+                this.pendingOperation = null;
             }
         );
     }
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/presence/manager.ts around lines 25–34,
the reaction calls joinProject/leaveProject without awaiting so rapid changes
can cause overlapping join/leave operations; fix by serializing these calls: add
an async guard (e.g., a simple mutex flag or an inFlight Promise) so the
reaction awaits the previous operation before starting the next, ensure each
operation re-checks the latest currentProjectId before performing its final
state change (to avoid acting on stale requests), and clear/reset the guard on
completion or error so subsequent reactions proceed.

}

setContext(userId: string, projectId: string) {
this.currentUserId = userId;
this.currentProjectId = projectId;
}

async joinProject(projectId: string) {
if (!this.currentUserId) return;

try {
this.isLoading = true;
this.currentProjectId = projectId;

await api.presence.joinProject.mutate({ projectId });

await this.loadProjectPresence(projectId);
this.subscribeToPresenceUpdates(projectId);

this.isConnected = true;
} catch (error) {
console.error('Error joining project:', error);
} finally {
this.isLoading = false;
}
}
Comment on lines +45 to +60
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

Improve error handling and state cleanup.

Issues:

  1. If joinProject.mutate fails, the method still proceeds to load presence and subscribe, marking the user as connected despite the failed join
  2. Line 47 redundantly sets currentProjectId (already set by setContext)
  3. No cleanup of partial state on error

Apply this diff:

     async joinProject(projectId: string) {
         if (!this.currentUserId) return;
 
         try {
             this.isLoading = true;
-            this.currentProjectId = projectId;
 
             await api.presence.joinProject.mutate({ projectId });
 
             await this.loadProjectPresence(projectId);
             this.subscribeToPresenceUpdates(projectId);
 
             this.isConnected = true;
         } catch (error) {
             console.error('Error joining project:', error);
+            this.isConnected = false;
+            this.onlineUsers = [];
+            throw error;
         } finally {
             this.isLoading = false;
         }
     }
📝 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
try {
this.isLoading = true;
this.currentProjectId = projectId;
await api.presence.joinProject.mutate({ projectId });
await this.loadProjectPresence(projectId);
this.subscribeToPresenceUpdates(projectId);
this.isConnected = true;
} catch (error) {
console.error('Error joining project:', error);
} finally {
this.isLoading = false;
}
}
async joinProject(projectId: string) {
if (!this.currentUserId) return;
try {
this.isLoading = true;
await api.presence.joinProject.mutate({ projectId });
await this.loadProjectPresence(projectId);
this.subscribeToPresenceUpdates(projectId);
this.isConnected = true;
} catch (error) {
console.error('Error joining project:', error);
this.isConnected = false;
this.onlineUsers = [];
throw error;
} finally {
this.isLoading = false;
}
}
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/presence/manager.ts around lines 45 to
60, the try/catch flow lets execution continue after a failed join and
redundantly sets currentProjectId; change the flow so that you await
api.presence.joinProject.mutate and immediately return or throw if it fails (so
you do not call loadProjectPresence or subscribeToPresenceUpdates on failure),
remove the redundant assignment to this.currentProjectId (since setContext
already sets it), and in the catch ensure you perform proper cleanup of partial
state (reset this.currentProjectId to null/undefined, set this.isConnected =
false, unsubscribe any in-progress subscriptions or listeners) while still
leaving the existing finally to set this.isLoading = false.


async leaveProject() {
if (!this.currentProjectId || !this.currentUserId) return;

try {
await api.presence.leaveProject.mutate({ projectId: this.currentProjectId });

this.unsubscribeFromPresenceUpdates();
this.onlineUsers = [];
this.currentProjectId = null;
this.isConnected = false;
} catch (error) {
console.error('Error leaving project:', error);
}
}

private async loadProjectPresence(projectId: string) {
try {
const presenceData = await api.presence.getProjectPresence.query({ projectId });
this.onlineUsers = presenceData.map(p => ({
userId: p.userId,
displayName: p.displayName,
avatarUrl: p.avatarUrl,
lastSeen: p.lastSeen,
}));
} catch (error) {
console.error('Error loading project presence:', error);
}
}

private subscribeToPresenceUpdates(projectId: string) {
this.unsubscribeFromPresenceUpdates();

const supabase = createClient();
const channel = supabase
.channel(`presence:${projectId}`)
.on(
'postgres_changes',
{
event: '*',
schema: 'public',
table: 'user_presence',
filter: `project_id=eq.${projectId}`,
},
(payload) => {
this.handlePresenceUpdate(payload);
}
)
.subscribe();

this.subscription = channel;
}

private unsubscribeFromPresenceUpdates() {
if (this.subscription) {
this.subscription.unsubscribe();
this.subscription = null;
}
}
Comment on lines +114 to +119
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify if unsubscribe should be awaited.

The Supabase channel unsubscribe() method may return a promise. If so, it should be awaited to ensure proper cleanup before proceeding.


🌐 Web query:

Does Supabase JS client channel.unsubscribe() return a promise and should it be awaited?

💡 Result:

Yes — RealtimeChannel.unsubscribe() returns a Promise (resolves to "ok" | "error" | "timed out") and should be awaited when you need to ensure the leave completed (e.g., in short‑lived contexts like serverless functions). [1][2]

Sources:

  • API TypeDoc for @supabase/supabase-js (unsubscribe signature). [1]
  • Unminified client code showing unsubscribe returns a Promise. [2]

Await Supabase channel.unsubscribe() for proper cleanup
Make unsubscribeFromPresenceUpdates async and use await this.subscription.unsubscribe() before nulling the subscription.

🤖 Prompt for AI Agents
In apps/web/client/src/components/store/presence/manager.ts around lines 114 to
119, change unsubscribeFromPresenceUpdates to be async and await the Supabase
channel unsubscribe call: check this.subscription, await
this.subscription.unsubscribe(), then set this.subscription = null after the
await; also update any callers to await unsubscribeFromPresenceUpdates() (or
handle the returned Promise) to ensure proper cleanup.


private handlePresenceUpdate(payload: any) {
const { eventType, new: newRecord, old: oldRecord } = payload;

switch (eventType) {
case 'INSERT':
case 'UPDATE':
if (newRecord && newRecord.is_online) {
const existingIndex = this.onlineUsers.findIndex(u => u.userId === newRecord.user_id);
const user: PresenceUser = {
userId: newRecord.user_id,
displayName: newRecord.user?.display_name || `${newRecord.user?.first_name || ''} ${newRecord.user?.last_name || ''}`.trim(),
avatarUrl: newRecord.user?.avatar_url,
lastSeen: new Date(newRecord.last_seen),
};

if (existingIndex >= 0) {
this.onlineUsers[existingIndex] = user;
} else {
this.onlineUsers.push(user);
}
} else {
this.onlineUsers = this.onlineUsers.filter(u => u.userId !== newRecord?.user_id);
}
break;

case 'DELETE':
this.onlineUsers = this.onlineUsers.filter(u => u.userId !== oldRecord?.user_id);
break;
}
}
Comment on lines +121 to +150
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Add type safety and null checks for payload handling.

Issues:

  1. The payload parameter is typed as any, reducing type safety
  2. No null check for newRecord or oldRecord before accessing properties
  3. Assumes newRecord.user relation is populated but could be null/undefined

Consider adding type definitions and null checks:

-    private handlePresenceUpdate(payload: any) {
+    private handlePresenceUpdate(payload: {
+        eventType: 'INSERT' | 'UPDATE' | 'DELETE';
+        new: any;
+        old: any;
+    }) {
         const { eventType, new: newRecord, old: oldRecord } = payload;
 
         switch (eventType) {
             case 'INSERT':
             case 'UPDATE':
-                if (newRecord && newRecord.is_online) {
+                if (newRecord?.user && newRecord.is_online) {
                     const existingIndex = this.onlineUsers.findIndex(u => u.userId === newRecord.user_id);
                     const user: PresenceUser = {
                         userId: newRecord.user_id,
-                        displayName: newRecord.user?.display_name || `${newRecord.user?.first_name || ''} ${newRecord.user?.last_name || ''}`.trim(),
+                        displayName: newRecord.user.display_name || `${newRecord.user.first_name || ''} ${newRecord.user.last_name || ''}`.trim() || 'Unknown User',
                         avatarUrl: newRecord.user?.avatar_url,
                         lastSeen: new Date(newRecord.last_seen),
                     };
📝 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
private handlePresenceUpdate(payload: any) {
const { eventType, new: newRecord, old: oldRecord } = payload;
switch (eventType) {
case 'INSERT':
case 'UPDATE':
if (newRecord && newRecord.is_online) {
const existingIndex = this.onlineUsers.findIndex(u => u.userId === newRecord.user_id);
const user: PresenceUser = {
userId: newRecord.user_id,
displayName: newRecord.user?.display_name || `${newRecord.user?.first_name || ''} ${newRecord.user?.last_name || ''}`.trim(),
avatarUrl: newRecord.user?.avatar_url,
lastSeen: new Date(newRecord.last_seen),
};
if (existingIndex >= 0) {
this.onlineUsers[existingIndex] = user;
} else {
this.onlineUsers.push(user);
}
} else {
this.onlineUsers = this.onlineUsers.filter(u => u.userId !== newRecord?.user_id);
}
break;
case 'DELETE':
this.onlineUsers = this.onlineUsers.filter(u => u.userId !== oldRecord?.user_id);
break;
}
}
private handlePresenceUpdate(payload: {
eventType: 'INSERT' | 'UPDATE' | 'DELETE';
new: any;
old: any;
}) {
const { eventType, new: newRecord, old: oldRecord } = payload;
switch (eventType) {
case 'INSERT':
case 'UPDATE':
if (newRecord?.user && newRecord.is_online) {
const existingIndex = this.onlineUsers.findIndex(u => u.userId === newRecord.user_id);
const user: PresenceUser = {
userId: newRecord.user_id,
displayName: newRecord.user.display_name || `${newRecord.user.first_name || ''} ${newRecord.user.last_name || ''}`.trim() || 'Unknown User',
avatarUrl: newRecord.user?.avatar_url,
lastSeen: new Date(newRecord.last_seen),
};
if (existingIndex >= 0) {
this.onlineUsers[existingIndex] = user;
} else {
this.onlineUsers.push(user);
}
} else {
this.onlineUsers = this.onlineUsers.filter(u => u.userId !== newRecord?.user_id);
}
break;
case 'DELETE':
this.onlineUsers = this.onlineUsers.filter(u => u.userId !== oldRecord?.user_id);
break;
}
}
🤖 Prompt for AI Agents
In apps/web/client/src/components/store/presence/manager.ts around lines 121 to
150, the handler uses an untyped payload and dereferences newRecord/oldRecord
and newRecord.user without guards; add a proper TypeScript type for the payload
(e.g., an interface with eventType: 'INSERT'|'UPDATE'|'DELETE' and optional
new/old records with typed fields user_id, is_online, last_seen, and optional
user relation), change the method signature from payload: any to that type, and
add null/undefined checks before accessing newRecord/oldRecord and
newRecord.user (early-return or branch when missing), safely compute displayName
falling back to first/last name when user is absent, guard parsing of last_seen
into Date (handle invalid/missing values), and update array operations
accordingly so no property access occurs on undefined objects.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Replace any type for payload and add type guards for safe access.

Line 121 declares payload as any, which violates the coding guideline. Additionally, lines 127-143 access properties like newRecord.user_id, newRecord.user?.display_name, etc., without type safety. This can lead to runtime errors if the payload structure changes or is malformed.

Define an explicit type or interface for the expected payload structure:

+interface PresencePayload {
+    eventType: 'INSERT' | 'UPDATE' | 'DELETE';
+    new?: {
+        user_id: string;
+        is_online: boolean;
+        last_seen: string;
+        user?: {
+            display_name?: string;
+            first_name?: string;
+            last_name?: string;
+            avatar_url?: string | null;
+        };
+    };
+    old?: {
+        user_id: string;
+    };
+}
+
-    private handlePresenceUpdate(payload: any) {
+    private handlePresenceUpdate(payload: PresencePayload) {
         const { eventType, new: newRecord, old: oldRecord } = payload;

Additionally, verify that the Supabase realtime payload always includes the user relation when newRecord.is_online is true by checking the migration or running:

As per coding guidelines.


🏁 Script executed:

#!/bin/bash
# Check if the user_presence trigger/function includes user relation
rg -A 10 'CREATE.*FUNCTION.*presence' apps/backend/supabase/migrations/

Length of output: 1782


🏁 Script executed:

#!/bin/bash
rg -n -C3 'CREATE (TABLE|VIEW).*user_presence' apps/backend/supabase/migrations/

Length of output: 484


Add explicit PresencePayload type and guard nested user fields
Define a PresencePayload interface instead of any, and remove or guard newRecord.user accesses—Supabase realtime payload for user_presence does not include the user object by default. Fetch user details separately or include them in your subscription query.

🤖 Prompt for AI Agents
In apps/web/client/src/components/store/presence/manager.ts around lines 121 to
150, the handler currently types payload as any and dereferences newRecord.user
(which Supabase realtime for user_presence doesn't include by default); define a
PresencePayload interface for the expected shape (including eventType, new and
old records with user_id, is_online, last_seen and optional nested user), change
the method signature to use that type, and guard all accesses to newRecord.user
(use optional chaining and fallback values) or remove those accesses and instead
fetch the user's profile separately (or include the user relation in the
subscription query) before constructing displayName/avatarUrl; ensure
null/undefined checks for newRecord and oldRecord are used before reading fields
and update tests/usage accordingly.


get otherOnlineUsers(): PresenceUser[] {
return this.onlineUsers.filter(u => u.userId !== this.currentUserId);
}

isUserOnline(userId: string): boolean {
return this.onlineUsers.some(u => u.userId === userId);
}

dispose() {
this.leaveProject();
}
}
Loading