Skip to content

Commit ec5d1c4

Browse files
brendan-kellambrendan-kellam
committed
feedback
1 parent 55b06a7 commit ec5d1c4

9 files changed

Lines changed: 83 additions & 45 deletions

File tree

‎packages/web/src/app/[domain]/chat/[id]/page.tsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { ChatVisibility } from '@sourcebot/db';
1818
import { Metadata } from 'next';
1919
import { SBChatMessage } from '@/features/chat/types';
2020
import { env, hasEntitlement } from '@sourcebot/shared';
21+
2122
import { captureEvent } from '@/lib/posthog';
2223

2324
interface PageProps {
@@ -95,7 +96,10 @@ export default async function Page(props: PageProps) {
9596
// Claim any anonymous chats created by this user before they signed in.
9697
// This must happen before getChatInfo so the chat ownership is updated.
9798
if (session) {
98-
await claimAnonymousChats();
99+
const claimResult = await claimAnonymousChats();
100+
if (isServiceError(claimResult)) {
101+
throw new ServiceErrorException(claimResult);
102+
}
99103
}
100104

101105
const languageModels = await getConfiguredLanguageModelsInfo();
@@ -158,6 +162,7 @@ export default async function Page(props: PageProps) {
158162
name={name}
159163
id={params.id}
160164
isOwner={isOwner}
165+
isAuthenticated={!!session}
161166
/>
162167
}
163168
actions={isOwner ? (

‎packages/web/src/app/[domain]/chat/components/chatActionsDropdown.tsx‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ interface ChatActionsDropdownProps {
88
onRenameClick: () => void;
99
onDuplicateClick: () => void;
1010
onDeleteClick: () => void;
11+
showDelete?: boolean;
1112
align?: "start" | "center" | "end";
1213
}
1314

@@ -16,6 +17,7 @@ export const ChatActionsDropdown = ({
1617
onRenameClick,
1718
onDuplicateClick,
1819
onDeleteClick,
20+
showDelete = true,
1921
align = "start",
2022
}: ChatActionsDropdownProps) => {
2123
return (
@@ -47,16 +49,18 @@ export const ChatActionsDropdown = ({
4749
<CopyIcon className="w-4 h-4 mr-2" />
4850
Duplicate
4951
</DropdownMenuItem>
50-
<DropdownMenuItem
51-
className="cursor-pointer text-destructive focus:text-destructive"
52-
onClick={(e) => {
53-
e.stopPropagation();
54-
onDeleteClick();
55-
}}
56-
>
57-
<TrashIcon className="w-4 h-4 mr-2" />
58-
Delete
59-
</DropdownMenuItem>
52+
{showDelete && (
53+
<DropdownMenuItem
54+
className="cursor-pointer text-destructive focus:text-destructive"
55+
onClick={(e) => {
56+
e.stopPropagation();
57+
onDeleteClick();
58+
}}
59+
>
60+
<TrashIcon className="w-4 h-4 mr-2" />
61+
Delete
62+
</DropdownMenuItem>
63+
)}
6064
</DropdownMenuContent>
6165
</DropdownMenu>
6266
);

‎packages/web/src/app/[domain]/chat/components/chatName.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ interface ChatNameProps {
1717
name: string | null;
1818
id: string;
1919
isOwner?: boolean;
20+
isAuthenticated?: boolean;
2021
}
2122

22-
export const ChatName = ({ name, id, isOwner = false }: ChatNameProps) => {
23+
export const ChatName = ({ name, id, isOwner = false, isAuthenticated = false }: ChatNameProps) => {
2324
const [isRenameDialogOpen, setIsRenameDialogOpen] = useState(false);
2425
const [isDuplicateDialogOpen, setIsDuplicateDialogOpen] = useState(false);
2526
const [isDeleteDialogOpen, setIsDeleteDialogOpen] = useState(false);
@@ -93,6 +94,7 @@ export const ChatName = ({ name, id, isOwner = false }: ChatNameProps) => {
9394
onRenameClick={() => setIsRenameDialogOpen(true)}
9495
onDuplicateClick={() => setIsDuplicateDialogOpen(true)}
9596
onDeleteClick={() => setIsDeleteDialogOpen(true)}
97+
showDelete={isAuthenticated}
9698
align="center"
9799
>
98100
<Button

‎packages/web/src/app/[domain]/chat/components/shareChatPopover/ee/invitePanel.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,8 +182,11 @@ export const InvitePanel = ({
182182
<LoadingButton
183183
onClick={async () => {
184184
setIsInviting(true);
185-
await onShareChatWithUsers(selectedUsers);
186-
setIsInviting(false);
185+
try {
186+
await onShareChatWithUsers(selectedUsers);
187+
} finally {
188+
setIsInviting(false);
189+
}
187190
}}
188191
disabled={selectedUsers.length === 0 || isInviting}
189192
loading={isInviting}

‎packages/web/src/app/[domain]/chat/components/shareChatPopover/index.tsx‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,14 +75,14 @@ export const ShareChatPopover = ({
7575
});
7676
return false;
7777
} else {
78-
setSharedWithUsers(sharedWithUsers.filter(user => user.id !== userId));
78+
setSharedWithUsers(prev => prev.filter(u => u.id !== userId));
7979
captureEvent('wa_chat_user_removed', { chatId });
8080
toast({
8181
description: "✅ Access removed"
8282
});
8383
return true;
8484
}
85-
}, [chatId, toast, sharedWithUsers]);
85+
}, [chatId, toast]);
8686

8787

8888
const onShareChatWithUsers = useCallback(async (users: SessionUser[]) => {
@@ -98,7 +98,7 @@ export const ShareChatPopover = ({
9898
});
9999
return false;
100100
} else {
101-
setSharedWithUsers([...sharedWithUsers, ...users]);
101+
setSharedWithUsers(prev => [...prev, ...users]);
102102
captureEvent('wa_chat_users_invited', {
103103
chatId,
104104
numUsersInvited: users.length,
@@ -109,7 +109,7 @@ export const ShareChatPopover = ({
109109
setView('main');
110110
return true;
111111
}
112-
}, [chatId, toast, sharedWithUsers]);
112+
}, [chatId, toast]);
113113

114114
const onOpenChange = useCallback((open: boolean) => {
115115
_setIsOpen(open);

‎packages/web/src/app/[domain]/chat/components/shareChatPopover/shareSettings.tsx‎

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,22 @@ export const ShareSettings = ({
5151
const pathname = usePathname();
5252
const isAuthenticated = !!currentUser;
5353

54-
const handleCopyLink = useCallback(() => {
55-
navigator.clipboard.writeText(window.location.href);
56-
captureEvent('wa_chat_link_copied', {
57-
chatId,
58-
visibility,
59-
});
60-
toast({
61-
description: "✅ Link copied to clipboard",
62-
});
54+
const handleCopyLink = useCallback(async () => {
55+
try {
56+
await navigator.clipboard.writeText(window.location.href);
57+
captureEvent('wa_chat_link_copied', {
58+
chatId,
59+
visibility,
60+
});
61+
toast({
62+
description: "✅ Link copied to clipboard",
63+
});
64+
} catch (e) {
65+
toast({
66+
description: `Failed to copy link: ${e instanceof Error ? e.message : "Unknown error"}`,
67+
variant: "destructive",
68+
});
69+
}
6370
}, [chatId, visibility, toast]);
6471

6572
const getInitials = (name?: string | null, email?: string | null) => {
@@ -147,12 +154,16 @@ export const ShareSettings = ({
147154
disabled={removingUserIds.has(user.id)}
148155
onClick={async () => {
149156
setRemovingUserIds(prev => new Set(prev).add(user.id));
150-
await onRemoveSharedWithUser(user.id);
151-
setRemovingUserIds(prev => {
152-
const next = new Set(prev);
153-
next.delete(user.id);
154-
return next;
155-
});
157+
try {
158+
159+
await onRemoveSharedWithUser(user.id);
160+
} finally {
161+
setRemovingUserIds(prev => {
162+
const next = new Set(prev);
163+
next.delete(user.id);
164+
return next;
165+
});
166+
}
156167
}}
157168
>
158169
{removingUserIds.has(user.id) ? (
@@ -187,8 +198,11 @@ export const ShareSettings = ({
187198
value={visibility}
188199
onValueChange={async (value) => {
189200
setIsVisibilityUpdating(true);
190-
await onVisibilityChange(value as ChatVisibility);
191-
setIsVisibilityUpdating(false);
201+
try {
202+
await onVisibilityChange(value as ChatVisibility);
203+
} finally {
204+
setIsVisibilityUpdating(false);
205+
}
192206
}}
193207
disabled={isVisibilityUpdating || !isAuthenticated}
194208
>

‎packages/web/src/app/api/(server)/ee/chat/[chatId]/searchMembers/route.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ export const GET = apiHandler(async (
3636
if (env.EXPERIMENT_ASK_GH_ENABLED === 'true') {
3737
return serviceErrorResponse({
3838
statusCode: StatusCodes.FORBIDDEN,
39-
errorCode: ErrorCode.NOT_FOUND,
39+
errorCode: ErrorCode.UNEXPECTED_ERROR,
4040
message: "This API is not enabled with this experiment.",
4141
})
4242
}
4343

4444
if (!hasEntitlement('chat-sharing')) {
4545
return serviceErrorResponse({
4646
statusCode: StatusCodes.FORBIDDEN,
47-
errorCode: ErrorCode.NOT_FOUND,
47+
errorCode: ErrorCode.UNEXPECTED_ERROR,
4848
message: "Chat sharing is not enabled for your license",
4949
})
5050
}

‎packages/web/src/app/login/components/loginForm.tsx‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ interface LoginFormProps {
1919
export const LoginForm = ({ callbackUrl, error, providers, context, isAnonymousAccessEnabled = false }: LoginFormProps) => {
2020
const captureEvent = useCaptureEvent();
2121

22+
const safeCallbackUrl = useMemo(() => {
23+
if (!callbackUrl) return "/";
24+
// Allow only relative paths that start with "/" but not "//" (protocol-relative URLs)
25+
if (callbackUrl.startsWith("/") && !callbackUrl.startsWith("//")) {
26+
return callbackUrl;
27+
}
28+
return "/";
29+
}, [callbackUrl]);
30+
2231
const errorMessage = useMemo(() => {
2332
if (!error) {
2433
return "";
@@ -94,7 +103,7 @@ export const LoginForm = ({ callbackUrl, error, providers, context, isAnonymousA
94103
</p>
95104
{isAnonymousAccessEnabled && (
96105
<p className="text-sm text-muted-foreground">
97-
<Link className="underline" href={callbackUrl ?? "/"}>Continue as guest</Link>
106+
<Link className="underline" href={safeCallbackUrl}>Continue as guest</Link>
98107
</p>
99108
)}
100109
</Card>

‎packages/web/src/features/chat/actions.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ export const _isOwnerOfChat = async (chat: Chat, user: User | undefined): Promis
4545
return true;
4646
}
4747

48-
// Anonymous user owns the chat (check cookie-based anonymous ID)
49-
if (!user && chat.anonymousCreatorId) {
48+
// Only check the anonymous cookie for unclaimed chats (createdById === null).
49+
// Once a chat has been claimed by an authenticated user, the anonymous path
50+
// must not grant access — even if the same browser still holds the original cookie.
51+
if (!chat.createdById && chat.anonymousCreatorId) {
5052
const anonymousId = await getAnonymousId();
5153
if (anonymousId && chat.anonymousCreatorId === anonymousId) {
5254
return true;
@@ -386,6 +388,7 @@ export const claimAnonymousChats = async () => sew(() =>
386388
},
387389
data: {
388390
createdById: user.id,
391+
anonymousCreatorId: null,
389392
},
390393
});
391394

@@ -550,12 +553,10 @@ export const unshareChatWithUser = async ({ chatId, userId }: { chatId: string,
550553
return notFound();
551554
}
552555

553-
await prisma.chatAccess.delete({
556+
await prisma.chatAccess.deleteMany({
554557
where: {
555-
chatId_userId: {
556-
chatId,
557-
userId,
558-
},
558+
chatId,
559+
userId,
559560
},
560561
});
561562

0 commit comments

Comments
 (0)