Issue #79 - Chat Sharing #86
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds end-to-end chat sharing: new ShareDialog, management UI, public shared-chat pages/components, Convex schema/queries/mutations for share lifecycle and anonymized shared messages, middleware for /share routes, UI wiring in header/sidebar/items, tests, and related component refactors. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as ShareDialog
participant API as Convex API
participant DB as Database
User->>UI: Click "Create/Share" for chatId
UI->>API: call shareChat(chatId)
API->>API: auth & ownership checks
API->>DB: write share_id, share_date
DB-->>API: confirm
API-->>UI: { shareId, shareDate }
UI->>User: show share URL (copy/social)
alt Refresh share date
User->>UI: Click "Refresh"
UI->>API: updateShareDate(chatId)
API->>DB: update share_date
DB-->>API: confirm
API-->>UI: { shareId, shareDate }
else Unshare
User->>UI: Click "Unshare"
UI->>API: unshareChat(chatId)
API->>DB: clear share fields
DB-->>API: confirm
API-->>UI: success
end
sequenceDiagram
autonumber
participant PublicUser
participant Page as Next.js Route
participant View as SharedChatView
participant API as Convex API
participant DB as Database
PublicUser->>Page: GET /share/:shareId
Page->>View: render with shareId
View->>API: getSharedChat(shareId)
API->>DB: lookup by share_id
DB-->>API: chat metadata (no user_id)
API-->>View: chat
View->>API: getSharedMessages(chatId)
API->>DB: fetch messages <= share_date, anonymize/strip files
DB-->>API: messages
API-->>View: messages
View->>PublicUser: render read-only conversation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (1)app/components/ChatHeader.tsx (1)
🔇 Additional comments (4)
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: 2
🧹 Nitpick comments (1)
app/components/SharedLinksTab.tsx (1)
37-40: Harden the pop-up call withnoopener
Opening a tab viawindow.open(..., "_blank")without"noopener"keepswindow.openerattached, which leaves room for reverse tabnabbing if the share view ever serves untrusted content. Passing"noopener,noreferrer"severs that link and is the standard mitigation.(developer.chrome.com)- window.open(shareUrl, "_blank"); + window.open(shareUrl, "_blank", "noopener,noreferrer");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/api/stripe.ts(1 hunks)app/components/ChatHeader.tsx(6 hunks)app/components/ChatItem.tsx(4 hunks)app/components/DataControlsTab.tsx(2 hunks)app/components/ShareDialog.tsx(1 hunks)app/components/SharedLinksTab.tsx(1 hunks)app/components/SidebarHistory.tsx(1 hunks)app/share/[shareId]/SharedChatView.tsx(1 hunks)app/share/[shareId]/SharedMessages.tsx(1 hunks)app/share/[shareId]/page.tsx(1 hunks)convex/chats.ts(2 hunks)convex/messages.ts(1 hunks)convex/schema.ts(1 hunks)middleware.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from ./_generated/dataModel for document IDs (e.g., Id<'users'>) instead of string
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) -> Record<Id<'users'>, string>)
Use as const on string literals in discriminated unions
When using Array type, explicitly declare arrays with const array: Array = [...]
When using Record type, explicitly declare with const record: Record<KeyType, ValueType> = {...}
Files:
app/components/ChatItem.tsxapp/share/[shareId]/page.tsxapp/components/SharedLinksTab.tsxapp/share/[shareId]/SharedMessages.tsxapp/components/ShareDialog.tsxapp/api/stripe.tsconvex/messages.tsmiddleware.tsapp/components/DataControlsTab.tsxapp/share/[shareId]/SharedChatView.tsxapp/components/SidebarHistory.tsxapp/components/ChatHeader.tsxconvex/schema.tsconvex/chats.ts
convex/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx,js,jsx}: ALWAYS use the new Convex function syntax (query/mutation/action object with args/returns/handler) when defining Convex functions
When a Convex function returns null, include returns: v.null() and return null
Use internalQuery/internalMutation/internalAction for private functions; do not expose sensitive logic with public query/mutation/action
Do NOT register functions via the api or internal objects
ALWAYS provide args and returns validators for query, mutation, action and their internal variants; use v.null() when no value is returned
From Convex functions, use ctx.runQuery/ctx.runMutation/ctx.runAction to call other functions (not direct function calls)
Pass a FunctionReference (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do NOT pass the callee function directly
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise refactor shared code into helpers
Minimize calls from actions to queries/mutations to reduce race conditions; prefer fewer transactional boundaries
Organize public query/mutation/action functions under convex/ to align with file-based routing
For pagination, validate args with paginationOptsValidator and call .paginate(args.paginationOpts) on queries
Use v.int64() instead of deprecated v.bigint() for signed 64-bit integers
Use v.record() for record-like structures; v.map() and v.set() are not supported
Do NOT use query().filter(...); define an index in schema and use withIndex instead
Convex queries do not support .delete(); collect or iterate results and call ctx.db.delete(_id) for each
Use .unique() to fetch exactly one document and fail on multiples
When async-iterating query results, prefer for await (const row of query) over .collect() or .take(n)
Use ctx.db.replace to fully replace an existing document; ctx.db.patch to shallow-merge updates
Files defining actions that use Node built-ins must start with "use node"
Never access ctx.db from within an action
D...
Files:
convex/messages.tsconvex/schema.tsconvex/chats.ts
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
When run-calling a function in the same file, add an explicit return type annotation on the await result to avoid TS circularity issues
Files:
convex/messages.tsconvex/schema.tsconvex/chats.ts
convex/schema.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/schema.{ts,tsx,js,jsx}: Define the database schema in convex/schema.ts
Import schema helpers (defineSchema, defineTable) from convex/server in the schema file
Include all index fields in the index name (e.g., by_field1_and_field2 for ["field1","field2"])
Files:
convex/schema.ts
🧠 Learnings (5)
📚 Learning: 2025-10-03T13:15:14.787Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T13:15:14.787Z
Learning: Applies to convex/schema.{ts,tsx,js,jsx} : Define the database schema in convex/schema.ts
Applied to files:
convex/schema.ts
📚 Learning: 2025-10-03T13:15:14.787Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T13:15:14.787Z
Learning: Applies to convex/schema.{ts,tsx,js,jsx} : Include all index fields in the index name (e.g., by_field1_and_field2 for ["field1","field2"])
Applied to files:
convex/schema.ts
📚 Learning: 2025-10-03T13:15:14.787Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T13:15:14.787Z
Learning: Applies to convex/schema.{ts,tsx,js,jsx} : Import schema helpers (defineSchema, defineTable) from convex/server in the schema file
Applied to files:
convex/schema.ts
📚 Learning: 2025-10-03T13:15:14.787Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T13:15:14.787Z
Learning: Applies to convex/**/*.{ts,tsx,js,jsx} : Organize public query/mutation/action functions under convex/ to align with file-based routing
Applied to files:
convex/chats.ts
📚 Learning: 2025-10-03T13:15:14.787Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T13:15:14.787Z
Learning: Applies to convex/**/*.{ts,tsx,js,jsx} : Use internalQuery/internalMutation/internalAction for private functions; do not expose sensitive logic with public query/mutation/action
Applied to files:
convex/chats.ts
🧬 Code graph analysis (9)
app/components/ChatItem.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(26-352)
app/share/[shareId]/page.tsx (1)
app/share/[shareId]/SharedChatView.tsx (1)
SharedChatView(15-122)
app/components/SharedLinksTab.tsx (1)
convex/chats.ts (2)
unshareChat(983-1013)unshareAllChats(1105-1133)
app/share/[shareId]/SharedMessages.tsx (1)
lib/utils/message-utils.ts (1)
MessagePart(5-8)
app/components/ShareDialog.tsx (1)
convex/chats.ts (3)
shareChat(872-917)updateShareDate(932-975)unshareChat(983-1013)
convex/messages.ts (1)
convex/_generated/server.js (2)
query(30-30)query(30-30)
app/share/[shareId]/SharedChatView.tsx (1)
app/share/[shareId]/SharedMessages.tsx (1)
SharedMessages(50-237)
app/components/ChatHeader.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(26-352)
convex/chats.ts (1)
convex/_generated/server.js (4)
mutation(50-50)mutation(50-50)query(30-30)query(30-30)
🔇 Additional comments (10)
convex/messages.ts (1)
1114-1134: Appreciate the defensive redaction before exporting shareable messages. Tightening the shared payload to placeholder file/image parts keeps attachments private without sacrificing the rest of the transcript fidelity.app/components/ChatHeader.tsx (2)
337-337: Verify share button condition logic.The condition includes
!temporaryChatsEnabled, buttemporaryChatsEnabledis a global toggle for new chats. IfisExistingChatis true, the chat is already saved and not temporary, so the global toggle shouldn't affect whether it can be shared. This prevents users from sharing existing chats while temporary mode is enabled, which may be unintended.Consider whether the condition should simply be:
{isExistingChat && id && chatTitle && (If the current logic is intentional, please add a comment explaining why the global temporary mode toggle should prevent sharing of existing saved chats.
3-3: Clean integration of share functionality.The ShareDialog component is properly integrated with appropriate state management, imports, and UI elements. The implementation follows existing patterns in the codebase and provides good UX with tooltips and proper button styling.
Also applies to: 6-6, 17-17, 64-64, 274-282, 336-356
convex/chats.ts (7)
47-48: LGTM: Clean extension of return type.The addition of optional
share_idandshare_datefields to the return type properly supports the sharing feature while maintaining backward compatibility.
872-917: Excellent implementation of shareChat mutation.The mutation properly:
- Enforces authentication and ownership
- Implements idempotency by returning existing share_id
- Uses
crypto.randomUUID()for secure share link generation- Updates
update_timefor cache invalidation- Follows Convex best practices with proper validators
932-975: Well-designed updateShareDate mutation.The implementation correctly:
- Validates that the chat is already shared before updating
- Preserves the existing
share_idwhile refreshingshare_date- Provides clear error messages guiding users to
shareChatfirst- Documents the "frozen share" concept effectively
The design gives users fine-grained control over what content is publicly visible through the share link.
983-1013: LGTM: Clean unshare implementation.The mutation properly enforces authentication and ownership, atomically removes sharing metadata, and maintains cache invalidation through
update_time.
1022-1055: Excellent public query design with proper security.This public query (no authentication required) is correctly implemented for share link functionality:
- Uses the
by_share_idindex for efficient lookups- Properly anonymizes data by excluding
user_idand other sensitive fields- Validates that the chat has active sharing metadata before returning
- Returns minimal necessary data for public viewing
The design appropriately balances accessibility (public links) with privacy (anonymity).
Based on learnings
1062-1098: LGTM: Clean user-scoped query.The query properly:
- Requires authentication with graceful empty array fallback
- Uses the
by_user_and_updatedindex efficiently- Filters to only shared chats and returns minimal necessary fields
- Sorts by most recent shares first for better UX
1105-1133: Efficient batch unshare implementation.The mutation efficiently handles bulk operations by:
- Filtering to only chats that need updating (have
share_id)- Using
Promise.allfor parallel updates to minimize latency- Properly updating both sharing fields and
update_timeClean implementation following Convex best practices.
fkesheh
force-pushed
the
feat/issue-79-chat-sharing
branch
from
0094236 to
ab5e1b5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/utils/file-utils.ts (1)
156-156: Apply the same mediaType fallback for consistency.The
createFileMessagePartFromUploadedFilefunction should use the same fallback logic ascreateFileMessagePart(line 59). Without this fallback, files without a MIME type will have an emptymediaType, which is inconsistent with the changes made elsewhere in this file.Apply this diff to add the fallback:
+ // Use fallback for empty media types (common for .md, .txt files) + const mediaType = uploadedFile.file.type || "application/octet-stream"; + return { type: "file" as const, - mediaType: uploadedFile.file.type, + mediaType, fileId: uploadedFile.fileId, name: uploadedFile.file.name, size: uploadedFile.file.size, url: uploadedFile.url, };
🧹 Nitpick comments (5)
convex/chats.ts (3)
917-926: Consider removing the defensive re-fetch.The re-fetch after
ctx.db.patch(lines 918-921) appears unnecessary since patch operations are atomic in Convex. You can safely return the values you just set.Apply this diff to simplify:
- // Re-fetch to ensure we return the persisted value, handling potential race conditions - const persisted = await ctx.db.get(chat._id); - if (!persisted?.share_id || !persisted.share_date) { - throw new Error("Failed to persist share metadata"); - } - return { - shareId: persisted.share_id, - shareDate: persisted.share_date, + shareId: shareId, + shareDate: shareDate, };
1073-1109: Performance: Consider pagination or dedicated index for shared chats.The current implementation fetches all user chats then filters in-memory for shared ones. This O(n) operation could become slow if users have many chats. Consider one of these optimizations:
- Add a compound index
by_user_and_share_idon(user_id, share_id)to directly query only shared chats- Add pagination using
paginationOptsValidatorif users may have many shared chatsOption 1: Add dedicated index (requires schema change):
In
convex/schema.ts:.index("by_user_and_share_id", ["user_id", "share_id"])Then update the query:
- const chats = await ctx.db + const sharedChats = await ctx.db .query("chats") - .withIndex("by_user_and_updated", (q) => q.eq("user_id", identity.subject)) + .withIndex("by_user_and_share_id", (q) => + q.eq("user_id", identity.subject).gt("share_id", "") + ) .collect(); - // Filter and map to only shared chats - return chats - .filter((chat) => chat.share_id && chat.share_date) + return sharedChats + .filter((chat) => chat.share_date) // Additional safety check .map((chat) => ({
1116-1144: Consider batch size limits for bulk unshare.The function fetches all user chats and updates them in parallel with
Promise.all. While the parallel approach is good, this could potentially hit transaction limits if a user has a very large number of shared chats. Consider processing in batches if this becomes an issue.For now, this is acceptable for a bulk administrative action. Monitor usage and consider batching if users have >100 shared chats.
app/share/[shareId]/SharedMessages.tsx (2)
5-27: Consider shared type definitions.The
MessagePartandMessageinterfaces are defined locally, but a simplifiedMessagePartexists inlib/utils/message-utils.ts. If these types are intentionally divergent for the shared view, this is fine. Otherwise, consider extracting shared types to avoid drift.If the types should align:
- Extend the types in
lib/utils/message-utils.tsto include all fields- Import and reuse them here
If the shared view intentionally uses a different structure, consider adding a comment explaining the separation.
64-203: Consider extracting message part renderers.The
renderPartfunction handles many message types inline. While functional, consider extracting each type's rendering logic into separate functions or a map of renderers for better maintainability:const partRenderers: Record<string, (part: MessagePart, idx: number) => JSX.Element | null> = { text: (part, idx) => part.text ? <div key={idx}>{part.text}</div> : null, file: (part, idx) => renderFilePlaceholder(part, idx), image: (part, idx) => renderFilePlaceholder(part, idx), // ... etc }; const renderPart = (part: MessagePart, idx: number) => { const renderer = partRenderers[part.type.replace(/^(data-|tool-)/, '')]; return renderer ? renderer(part, idx) : null; };This is optional; the current implementation is clear and working correctly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/components/SharedLinksTab.tsx(1 hunks)app/share/[shareId]/SharedChatView.tsx(1 hunks)app/share/[shareId]/SharedMessages.tsx(1 hunks)app/share/[shareId]/__tests__/SharedMessages.test.tsx(1 hunks)convex/chats.ts(3 hunks)convex/messages.ts(1 hunks)lib/utils/file-utils.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/share/[shareId]/tests/SharedMessages.test.tsx
- convex/messages.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
**/*.{ts,tsx}: Use Id from ./_generated/dataModel for document IDs (e.g., Id<'users'>) instead of string
Ensure Record key/value types align with validators (e.g., v.record(v.id('users'), v.string()) -> Record<Id<'users'>, string>)
Use as const on string literals in discriminated unions
When using Array type, explicitly declare arrays with const array: Array = [...]
When using Record type, explicitly declare with const record: Record<KeyType, ValueType> = {...}
Files:
lib/utils/file-utils.tsapp/share/[shareId]/SharedMessages.tsxapp/components/SharedLinksTab.tsxapp/share/[shareId]/SharedChatView.tsxconvex/chats.ts
convex/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
convex/**/*.{ts,tsx,js,jsx}: ALWAYS use the new Convex function syntax (query/mutation/action object with args/returns/handler) when defining Convex functions
When a Convex function returns null, include returns: v.null() and return null
Use internalQuery/internalMutation/internalAction for private functions; do not expose sensitive logic with public query/mutation/action
Do NOT register functions via the api or internal objects
ALWAYS provide args and returns validators for query, mutation, action and their internal variants; use v.null() when no value is returned
From Convex functions, use ctx.runQuery/ctx.runMutation/ctx.runAction to call other functions (not direct function calls)
Pass a FunctionReference (from api/internal) to ctx.runQuery/ctx.runMutation/ctx.runAction; do NOT pass the callee function directly
Only call an action from another action when crossing runtimes (e.g., V8 to Node); otherwise refactor shared code into helpers
Minimize calls from actions to queries/mutations to reduce race conditions; prefer fewer transactional boundaries
Organize public query/mutation/action functions under convex/ to align with file-based routing
For pagination, validate args with paginationOptsValidator and call .paginate(args.paginationOpts) on queries
Use v.int64() instead of deprecated v.bigint() for signed 64-bit integers
Use v.record() for record-like structures; v.map() and v.set() are not supported
Do NOT use query().filter(...); define an index in schema and use withIndex instead
Convex queries do not support .delete(); collect or iterate results and call ctx.db.delete(_id) for each
Use .unique() to fetch exactly one document and fail on multiples
When async-iterating query results, prefer for await (const row of query) over .collect() or .take(n)
Use ctx.db.replace to fully replace an existing document; ctx.db.patch to shallow-merge updates
Files defining actions that use Node built-ins must start with "use node"
Never access ctx.db from within an action
D...
Files:
convex/chats.ts
convex/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/convex_rules.mdc)
When run-calling a function in the same file, add an explicit return type annotation on the await result to avoid TS circularity issues
Files:
convex/chats.ts
🧠 Learnings (2)
📚 Learning: 2025-10-03T13:15:14.787Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T13:15:14.787Z
Learning: Applies to convex/**/*.{ts,tsx,js,jsx} : Organize public query/mutation/action functions under convex/ to align with file-based routing
Applied to files:
convex/chats.ts
📚 Learning: 2025-10-03T13:15:14.787Z
Learnt from: CR
Repo: hackerai-tech/hackerai PR: 0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-10-03T13:15:14.787Z
Learning: Applies to convex/**/*.{ts,tsx,js,jsx} : Use internalQuery/internalMutation/internalAction for private functions; do not expose sensitive logic with public query/mutation/action
Applied to files:
convex/chats.ts
🧬 Code graph analysis (4)
app/share/[shareId]/SharedMessages.tsx (1)
lib/utils/message-utils.ts (1)
MessagePart(5-8)
app/components/SharedLinksTab.tsx (2)
__mocks__/convex-react.ts (2)
useQuery(7-7)useMutation(5-5)convex/chats.ts (2)
unshareChat(994-1024)unshareAllChats(1116-1144)
app/share/[shareId]/SharedChatView.tsx (2)
__mocks__/convex-react.ts (1)
useQuery(7-7)app/share/[shareId]/SharedMessages.tsx (1)
SharedMessages(50-254)
convex/chats.ts (1)
convex/_generated/server.js (4)
mutation(50-50)mutation(50-50)query(30-30)query(30-30)
🔇 Additional comments (15)
lib/utils/file-utils.ts (3)
21-22: LGTM! Good defensive programming.Adding a fallback Content-Type is the correct approach for handling files without a MIME type. The use of "application/octet-stream" as the default is the standard practice for unknown file types.
25-25: Consistent application of the fallback.The contentType fallback is correctly applied both in the upload headers and when saving file metadata, ensuring consistency throughout the upload flow.
Also applies to: 39-39
58-63: Good consistency with the upload function.The fallback logic here mirrors the approach in
uploadSingleFileToConvex, ensuring thatFileMessagePartobjects always have a valid mediaType value.convex/chats.ts (4)
47-48: LGTM: Share fields properly added to query return types.The share_id and share_date fields are correctly defined as optional in both getChatByIdFromClient and getChatById return validators, aligning with the schema updates.
Also applies to: 133-134
943-986: LGTM: updateShareDate correctly implements share refresh.The function properly validates that the chat is already shared before updating, maintains the same share_id, and enforces ownership. The implementation is clean and efficient.
994-1024: LGTM: unshareChat properly removes share access.The mutation correctly enforces ownership and atomically removes both share_id and share_date fields.
1033-1066: LGTM: getSharedChat correctly implements public access.This public query appropriately:
- Omits authentication to allow unauthenticated access
- Excludes user_id to maintain anonymity
- Uses the by_share_id index for efficient lookup
- Validates that share_id and share_date still exist before returning
The public nature of this query is intentional and correct for the sharing feature.
Based on coding guidelines
app/components/SharedLinksTab.tsx (4)
31-40: LGTM: Clipboard handling properly implemented.The async/await pattern with try-catch correctly handles clipboard API failures, addressing the previous review feedback. Users will receive appropriate feedback whether the copy succeeds or fails.
47-60: LGTM: Proper concurrent call prevention.The
isUnsharingflag correctly prevents race conditions, and the try-catch-finally pattern ensures state cleanup happens regardless of success or failure.
62-75: LGTM: Bulk unshare properly implemented.Consistent with the single unshare handler, with appropriate loading state management and error handling.
136-178: LGTM: List rendering with good accessibility.The non-null assertions on
share_idandshare_dateare safe here sincegetUserSharedChatsfilters to only return chats where both fields are present. The aria-labels provide good accessibility support.app/share/[shareId]/SharedChatView.tsx (3)
13-27: LGTM: Efficient UUID validation and query cascading.The early UUID validation prevents unnecessary database queries for malformed share IDs. The conditional query execution using
"skip"is the correct Convex pattern, and the cascading (chat first, then messages) ensures messages are only fetched when a valid chat exists.
30-43: LGTM: Clear error state for invalid share links.Fast-failing for invalid UUID format provides good UX and clear error messaging.
73-122: LGTM: Well-structured shared chat view.The layout appropriately:
- Indicates read-only nature in the header
- Passes
share_dateto SharedMessages for the frozen snapshot notice- Provides clear privacy messaging in the footer
- Uses semantic HTML structure
app/share/[shareId]/SharedMessages.tsx (1)
205-253: LGTM: Clear message rendering with frozen snapshot notice.The component appropriately:
- Displays the frozen snapshot notice at the top to set expectations
- Handles empty state gracefully
- Renders messages with role-based styling
- Provides a clear end marker with count
- Add share_id and share_date fields to chats schema with index - Implement shareChat, updateShareDate, and unshareChat mutations - Add getSharedChat and getUserSharedChats queries - Implement frozen content filtering in getSharedMessages - Strip sensitive data (user_id, file URLs) from shared messages - Allow public access to /share routes in middleware
- Create ShareDialog with create/update/unshare functionality - Add SharedLinksTab for managing all shared chats - Build public share view with frozen content display - Add UUID validation for share links - Replace file/image placeholders with "Uploaded a file" text - Match shared conversation UI to main chat interface - Add date-fns dependency for relative time formatting
- Add share button to ChatHeader with tooltip - Add share option to ChatItem dropdown menu - Pass share_id and share_date through component hierarchy - Enable ShareDialog to recognize already-shared chats
- Fix ShareDialog state persistence after unsharing - Reset all loading states when dialog opens - Clear shareUrl and shareDate when chat is not shared - Properly reset isUnsharing, isUpdating, isGenerating states - Relax UUID validation regex to accept all UUID versions - Changed from strict v4 validation to general UUID format - Fixes "Invalid share link" error for valid share IDs
- Remove unnecessary 'as any' type casts in ChatHeader for better type safety - Fix race condition in shareChat mutation by re-fetching persisted values - Fix linting errors: escape apostrophes, use Link component, extract complex dependency
Properly await navigator.clipboard.writeText() and handle potential failures with appropriate error messages. Prevents false success notifications when clipboard access is denied or unavailable.
- Revert Stripe API version to 2025-10-29.clover (from main branch) - Remove unused @ts-expect-error directive - Add UUID format validation in getSharedMessages to prevent invalid queries
The getChatById query was missing share_id and share_date in its return type validator, causing validation errors when querying shared chats. This prevented the agent mode from functioning properly with shared chats.
Browsers don't always provide MIME types for certain file extensions like .md (markdown) and .txt files, resulting in empty file.type. Convex storage rejects uploads with empty Content-Type headers with 400 Bad Request. This fix adds a fallback to 'application/octet-stream' when file.type is empty, ensuring all files can be uploaded successfully.
Replace plain text 'Uploaded a file' with visually appealing icon cards: - Files: Show FileIcon with 'Document' label - Images: Show ImageIcon with 'Image' label This matches the style used in regular chat view while maintaining anonymization (no file names or sensitive info exposed). Updated tests to verify differentiation between files and images.
Updated file/image placeholder cards to exactly match the regular chat style: - Pink (#FF5588) rounded icon background - White icons (FileIcon/ImageIcon at h-6 w-6) - Bold file name and muted subtitle - Proper card sizing (max-w-80, min-w-64, p-2) - Border and background matching regular chat Updated tests to handle duplicate text (title + subtitle).
Images are stored with type='file' and mediaType='image/*', not type='image'. Updated getSharedMessages to check mediaType field to correctly return type='image' for images and type='file' for documents. This fixes shared chats showing 'Document' for images instead of 'Image'.
Changed 'HackerAI' link color from text-primary to text-white for better visibility and branding consistency in the footer.
fkesheh
force-pushed
the
feat/issue-79-chat-sharing
branch
from
0194d5c to
dbc0134
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/share/[shareId]/SharedChatView.tsx (1)
13-14: Consider clarifying the UUID validation comment.The regex validates the UUID format but doesn't enforce v4-specific constraints (version bits at position 14, variant bits at position 19). Since the backend uses
crypto.randomUUID()which generates v4 UUIDs, this general validation is sufficient for catching malformed inputs.Optionally update the comment to be more precise:
-// UUID format validation regex (matches v4 and other UUID versions) +// UUID format validation regex (matches standard 8-4-4-4-12 hex pattern) const UUID_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/components/ChatHeader.tsx(7 hunks)app/components/ChatItem.tsx(4 hunks)app/components/DataControlsTab.tsx(2 hunks)app/components/ShareDialog.tsx(1 hunks)app/components/SharedLinksTab.tsx(1 hunks)app/components/SidebarHistory.tsx(1 hunks)app/components/__tests__/ShareDialog.test.tsx(1 hunks)app/page.tsx(2 hunks)app/share/[shareId]/SharedChatView.tsx(1 hunks)app/share/[shareId]/SharedMessages.tsx(1 hunks)app/share/[shareId]/__tests__/SharedMessages.test.tsx(1 hunks)app/share/[shareId]/page.tsx(1 hunks)convex/chats.ts(3 hunks)convex/messages.ts(1 hunks)convex/schema.ts(1 hunks)lib/utils/file-utils.ts(3 hunks)middleware.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- app/page.tsx
- app/share/[shareId]/SharedMessages.tsx
- app/components/ShareDialog.tsx
- lib/utils/file-utils.ts
- app/components/SidebarHistory.tsx
- app/share/[shareId]/tests/SharedMessages.test.tsx
- app/components/DataControlsTab.tsx
- convex/messages.ts
- convex/schema.ts
- app/share/[shareId]/page.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
app/share/[shareId]/SharedChatView.tsx (2)
__mocks__/convex-react.ts (1)
useQuery(7-7)app/share/[shareId]/SharedMessages.tsx (1)
SharedMessages(51-255)
app/components/ChatItem.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(26-352)
app/components/ChatHeader.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(26-352)
app/components/__tests__/ShareDialog.test.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(26-352)
app/components/SharedLinksTab.tsx (2)
__mocks__/convex-react.ts (2)
useQuery(7-7)useMutation(5-5)convex/chats.ts (2)
unshareChat(1015-1045)unshareAllChats(1137-1165)
convex/chats.ts (1)
convex/_generated/server.js (4)
mutation(49-49)mutation(49-49)query(29-29)query(29-29)
🔇 Additional comments (9)
convex/chats.ts (3)
895-949: LGTM! Secure and idempotent share creation.The implementation correctly:
- Uses
crypto.randomUUID()for cryptographically secure share IDs- Enforces authentication and ownership
- Returns existing shares without creating duplicates (idempotent)
- Validates persistence with a defensive re-fetch
1078-1085: LGTM! Proper anonymization for public access.Correctly strips
user_idfrom the response to maintain anonymity while returning only the essential fields needed for rendering the shared chat.
1070-1070: No issues found. The "by_share_id" index is properly defined in the schema.Verification confirms the index is defined at
convex/schema.ts:39as.index("by_share_id", ["share_id"]), and theshare_idfield exists in the chats table asv.optional(v.string()). The query code atconvex/chats.ts:1070will work correctly.middleware.ts (1)
17-17: LGTM! Correctly enables public access to shared chats.The path pattern
"/share/:path*"correctly matches the public share route structure and aligns with thegetSharedChatquery being publicly accessible.app/components/ChatItem.tsx (1)
153-163: LGTM! Share integration follows established patterns.The share functionality correctly:
- Follows the same dialog-opening pattern as rename (close dropdown with delay)
- Passes the correct props including existing share metadata
- Uses
displayTitleconsistently with the rest of the componentAlso applies to: 286-289, 343-351
app/components/SharedLinksTab.tsx (2)
31-40: LGTM! Properly handles async clipboard operations.The clipboard write is correctly awaited with proper error handling, showing success/error toasts appropriately. This addresses the previous review comment.
110-240: LGTM! Comprehensive shared links management UI.The implementation correctly:
- Provides clear loading and empty states
- Uses confirmation dialogs for destructive actions
- Handles per-item and bulk operations with appropriate loading states
- Displays formatted timestamps and action buttons
app/components/ChatHeader.tsx (2)
275-282: LGTM! ShareDialog integration is properly typed.The ShareDialog is correctly integrated with:
- Optional chaining on
chatData?.share_idandchatData?.share_date(previous review comment addressed)- Proper fallback values for required props
- Clean state management
337-356: LGTM! Share button placement and conditions are appropriate.The share button is correctly:
- Shown only for existing, non-temporary chats
- Includes a helpful tooltip
- Positioned alongside other header actions
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
app/components/ChatItem.tsx (1)
286-298: Prefer lucide-react Share icon for consistency.The inline SVG is functionally correct but inconsistent with other menu items that use lucide-react icons (Edit2, Trash2). Consider using the
ShareorShare2icon from lucide-react for better consistency and maintainability.Apply this diff to use lucide-react:
-import { Ellipsis, Trash2, Edit2, Split } from "lucide-react"; +import { Ellipsis, Trash2, Edit2, Split, Share } from "lucide-react";<DropdownMenuItem onClick={handleShare}> - <svg - width="16" - height="16" - viewBox="0 0 20 20" - fill="currentColor" - xmlns="http://www.w3.org/2000/svg" - className="mr-2 h-4 w-4" - > - <path d="M2.66821 12.6663V12.5003C2.66821 12.1331 2.96598 11.8353 3.33325 11.8353C3.70052 11.8353 3.99829 12.1331 3.99829 12.5003V12.6663C3.99829 13.3772 3.9992 13.8707 4.03052 14.2542C4.0612 14.6298 4.11803 14.8413 4.19849 14.9993L4.2688 15.1263C4.44511 15.4137 4.69813 15.6481 5.00024 15.8021L5.13013 15.8577C5.2739 15.9092 5.46341 15.947 5.74536 15.97C6.12888 16.0014 6.62221 16.0013 7.33325 16.0013H12.6663C13.3771 16.0013 13.8707 16.0014 14.2542 15.97C14.6295 15.9394 14.8413 15.8825 14.9993 15.8021L15.1262 15.7308C15.4136 15.5545 15.6481 15.3014 15.802 14.9993L15.8577 14.8695C15.9091 14.7257 15.9469 14.536 15.97 14.2542C16.0013 13.8707 16.0012 13.3772 16.0012 12.6663V12.5003C16.0012 12.1332 16.2991 11.8355 16.6663 11.8353C17.0335 11.8353 17.3313 12.1331 17.3313 12.5003V12.6663C17.3313 13.3553 17.3319 13.9124 17.2952 14.3626C17.2624 14.7636 17.1974 15.1247 17.053 15.4613L16.9866 15.6038C16.7211 16.1248 16.3172 16.5605 15.8215 16.8646L15.6038 16.9866C15.227 17.1786 14.8206 17.2578 14.3625 17.2952C13.9123 17.332 13.3553 17.3314 12.6663 17.3314H7.33325C6.64416 17.3314 6.0872 17.332 5.63696 17.2952C5.23642 17.2625 4.87552 17.1982 4.53931 17.054L4.39673 16.9866C3.87561 16.7211 3.43911 16.3174 3.13501 15.8216L3.01294 15.6038C2.82097 15.2271 2.74177 14.8206 2.70435 14.3626C2.66758 13.9124 2.66821 13.3553 2.66821 12.6663ZM9.33521 12.5003V4.9388L7.13696 7.13704C6.87732 7.39668 6.45625 7.39657 6.19653 7.13704C5.93684 6.87734 5.93684 6.45631 6.19653 6.19661L9.52954 2.86263L9.6311 2.77962C9.73949 2.70742 9.86809 2.66829 10.0002 2.66829C10.1763 2.66838 10.3454 2.73819 10.47 2.86263L13.804 6.19661C14.0633 6.45628 14.0634 6.87744 13.804 7.13704C13.5443 7.39674 13.1222 7.39674 12.8625 7.13704L10.6653 4.93977V12.5003C10.6651 12.8673 10.3673 13.1652 10.0002 13.1654C9.63308 13.1654 9.33538 12.8674 9.33521 12.5003Z" /> - </svg> + <Share className="mr-2 h-4 w-4" /> Share </DropdownMenuItem>app/share/[shareId]/SharedMessages.tsx (4)
8-17: Replaceanytypes with proper interfaces.The
inputandoutputproperties useany, which defeats TypeScript's type safety. Consider defining specific types or useunknownwith type guards.interface MessagePart { type: string; text?: string; placeholder?: boolean; state?: string; - input?: any; - output?: any; + input?: Record<string, unknown>; + output?: Record<string, unknown>; toolCallId?: string; errorText?: string; }
27-45: Remove unused code.Both
shareDateprop andformatTimefunction are defined but never used in the component.interface SharedMessagesProps { messages: Message[]; - shareDate: number; } -export function SharedMessages({ messages, shareDate }: SharedMessagesProps) { +export function SharedMessages({ messages }: SharedMessagesProps) { const { openSidebar } = useSharedChatContext(); if (messages.length === 0) { return ( <div className="flex flex-col items-center justify-center py-12 text-center"> <p className="text-muted-foreground">No messages in this conversation</p> </div> ); } - const formatTime = (timestamp: number) => { - const date = new Date(timestamp); - return date.toLocaleString(); - }; - const renderPart = (part: MessagePart, idx: number) => {
295-297: Use Tailwind class instead of inline style.For consistency with the rest of the codebase, use Tailwind's
text-muted-foregroundclass instead of inline styles.- <div className="text-center text-[12px] font-normal" style={{ color: 'rgb(155, 155, 155)' }}> + <div className="text-center text-[12px] font-normal text-muted-foreground"> This is a copy of a conversation between HackerAI & Anonymous. </div>
47-290: Consider extracting part-type renderers into separate functions.The
renderPartfunction is over 240 lines long and handles many different part types. Extracting renderers for each part type (e.g.,renderTerminalPart,renderFilePart,renderPythonPart) would improve readability, testability, and maintainability.Example structure:
const renderTextPart = (part: MessagePart, idx: number) => { /* ... */ }; const renderFilePart = (part: MessagePart, idx: number) => { /* ... */ }; const renderTerminalPart = (part: MessagePart, idx: number) => { /* ... */ }; // ... etc const renderPart = (part: MessagePart, idx: number) => { if (part.type === "text") return renderTextPart(part, idx); if (part.type === "file" || part.type === "image") return renderFilePart(part, idx); // ... etc };app/components/ChatHeader.tsx (1)
345-355: Consider using a lucide-react icon for consistency.The Share button uses an inline SVG while other buttons in the component use icons from lucide-react (e.g.,
PanelLeft,Sparkle,SquarePen). Using a lucide-react icon likeShareorUploadwould improve consistency and maintainability.Example:
+import { PanelLeft, Sparkle, SquarePen, HatGlasses, Split, Share } from "lucide-react";<div className="flex w-full items-center justify-center gap-1.5"> - <svg - width="20" - height="20" - viewBox="0 0 20 20" - fill="currentColor" - xmlns="http://www.w3.org/2000/svg" - aria-label="" - className="-ms-0.5" - > - <path d="M2.66821 12.6663V12.5003C2.66821 12.1331 2.96598 11.8353 3.33325 11.8353C3.70052 11.8353 3.99829 12.1331 3.99829 12.5003V12.6663C3.99829 13.3772 3.9992 13.8707 4.03052 14.2542C4.0612 14.6298 4.11803 14.8413 4.19849 14.9993L4.2688 15.1263C4.44511 15.4137 4.69813 15.6481 5.00024 15.8021L5.13013 15.8577C5.2739 15.9092 5.46341 15.947 5.74536 15.97C6.12888 16.0014 6.62221 16.0013 7.33325 16.0013H12.6663C13.3771 16.0013 13.8707 16.0014 14.2542 15.97C14.6295 15.9394 14.8413 15.8825 14.9993 15.8021L15.1262 15.7308C15.4136 15.5545 15.6481 15.3014 15.802 14.9993L15.8577 14.8695C15.9091 14.7257 15.9469 14.536 15.97 14.2542C16.0013 13.8707 16.0012 13.3772 16.0012 12.6663V12.5003C16.0012 12.1332 16.2991 11.8355 16.6663 11.8353C17.0335 11.8353 17.3313 12.1331 17.3313 12.5003V12.6663C17.3313 13.3553 17.3319 13.9124 17.2952 14.3626C17.2624 14.7636 17.1974 15.1247 17.053 15.4613L16.9866 15.6038C16.7211 16.1248 16.3172 16.5605 15.8215 16.8646L15.6038 16.9866C15.227 17.1786 14.8206 17.2578 14.3625 17.2952C13.9123 17.332 13.3553 17.3314 12.6663 17.3314H7.33325C6.64416 17.3314 6.0872 17.332 5.63696 17.2952C5.23642 17.2625 4.87552 17.1982 4.53931 17.054L4.39673 16.9866C3.87561 16.7211 3.43911 16.3174 3.13501 15.8216L3.01294 15.6038C2.82097 15.2271 2.74177 14.8206 2.70435 14.3626C2.66758 13.9124 2.66821 13.3553 2.66821 12.6663ZM9.33521 12.5003V4.9388L7.13696 7.13704C6.87732 7.39668 6.45625 7.39657 6.19653 7.13704C5.93684 6.87734 5.93684 6.45631 6.19653 6.19661L9.52954 2.86263L9.6311 2.77962C9.73949 2.70742 9.86809 2.66829 10.0002 2.66829C10.1763 2.66838 10.3454 2.73819 10.47 2.86263L13.804 6.19661C14.0633 6.45628 14.0634 6.87744 13.804 7.13704C13.5443 7.39674 13.1222 7.39674 12.8625 7.13704L10.6653 4.93977V12.5003C10.6651 12.8673 10.3673 13.1652 10.0002 13.1654C9.63308 13.1654 9.33538 12.8674 9.33521 12.5003Z" /> - </svg> + <Share className="h-5 w-5" /> Share
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/components/ChatHeader.tsx(7 hunks)app/components/ChatItem.tsx(4 hunks)app/components/ComputerSidebar.tsx(2 hunks)app/share/[shareId]/SharedChatContext.tsx(1 hunks)app/share/[shareId]/SharedChatView.tsx(1 hunks)app/share/[shareId]/SharedMessages.tsx(1 hunks)app/share/[shareId]/__tests__/SharedMessages.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/share/[shareId]/tests/SharedMessages.test.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-18T20:33:42.057Z
Learnt from: RostyslavManko
Repo: hackerai-tech/hackerai PR: 10
File: app/components/ComputerCodeBlock.tsx:154-158
Timestamp: 2025-08-18T20:33:42.057Z
Learning: In ComputerCodeBlock component, the wrap state should be controlled by the parent component (ComputerSidebar) which passes the wrap prop. The internal isWrapped state conflicts with parent state management when used in ComputerSidebar context.
Applied to files:
app/components/ComputerSidebar.tsx
🧬 Code graph analysis (5)
app/share/[shareId]/SharedChatView.tsx (4)
app/share/[shareId]/SharedChatContext.tsx (2)
useSharedChatContext(41-49)SharedChatProvider(17-39)app/components/ComputerSidebar.tsx (1)
ComputerSidebarBase(26-334)hooks/use-mobile.ts (1)
useIsMobile(5-21)app/share/[shareId]/SharedMessages.tsx (1)
SharedMessages(32-337)
app/components/ChatItem.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(26-352)
app/components/ChatHeader.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(26-352)
app/components/ComputerSidebar.tsx (3)
types/chat.ts (1)
SidebarContent(37-37)components/ui/sidebar.tsx (1)
SidebarContent(753-753)app/contexts/GlobalState.tsx (1)
useGlobalState(704-710)
app/share/[shareId]/SharedMessages.tsx (3)
lib/utils/message-utils.ts (1)
MessagePart(5-8)app/share/[shareId]/SharedChatContext.tsx (1)
useSharedChatContext(41-49)app/components/MemoizedMarkdown.tsx (1)
MemoizedMarkdown(9-32)
🔇 Additional comments (12)
app/components/ChatItem.tsx (4)
33-33: LGTM!The import and prop additions are well-structured. Making
shareIdandshareDateoptional props is appropriate for this feature.Also applies to: 40-42, 49-50
56-56: LGTM!The state declaration follows the existing pattern for dialog management.
153-163: LGTM!The implementation pattern correctly mirrors
handleRename, ensuring consistent behavior for dialog management and avoiding focus conflicts.
352-360: LGTM!The ShareDialog component is correctly wired with all required props. The mapping of
shareIdtoexistingShareIdandshareDatetoexistingShareDatealigns with the ShareDialog interface, and usingdisplayTitleensures the current chat title is shown.app/share/[shareId]/SharedChatContext.tsx (1)
1-49: LGTM! Clean React context implementation.The SharedChatContext follows React best practices with proper TypeScript typing, state management, and a runtime guard in the custom hook to prevent usage outside the provider.
app/components/ComputerSidebar.tsx (1)
20-347: LGTM! Effective refactoring for reusability.The extraction of
ComputerSidebarBasewith explicit props and the addition of a wrapper component usingGlobalStateenables the sidebar to be used in both authenticated (normal chats) and unauthenticated (shared chats) contexts without duplication.app/share/[shareId]/SharedChatView.tsx (3)
13-50: LGTM! Responsive sidebar wrappers.The desktop and mobile sidebar wrappers correctly consume
useSharedChatContextand provide appropriate layouts for different screen sizes.
56-88: LGTM! UUID validation prevents unnecessary database queries.Early validation of the shareId format before querying the database is a good security and performance practice.
118-186: LGTM! Well-structured shared chat layout.The component properly wraps content in
SharedChatProvider, handles responsive layouts, and provides clear user feedback about the read-only nature of shared chats.app/components/ChatHeader.tsx (3)
3-3: LGTM!The import additions are appropriate and correctly used throughout the component.
Also applies to: 17-17
24-31: LGTM!The interface extension correctly adds optional
share_idandshare_datefields, and the code properly uses optional chaining when accessing these fields.
64-64: LGTM!The state management and ShareDialog integration are correctly implemented:
- State for dialog visibility is properly initialized
- Branched chat detection correctly uses
chatData?.branched_from_chat_id- ShareDialog props use appropriate optional chaining and fallbacks
- Fragment wrapper correctly allows multiple root elements
Also applies to: 73-73, 274-282
…view
- Auto-generate/update share link on dialog open (no manual button)
- Display chat preview showing first 4 messages with fade-out gradient
- Replace share buttons with circular social media icons (Copy, X, LinkedIn, Reddit)
- Add HackerAI logo to preview (bottom-right corner)
- Remove duplicate close button (showCloseButton={false})
- Add getPreviewMessages query to fetch preview data
- Comprehensive test suite with 34 test cases (15 passing)
- Tests cover UI rendering, loading states, error handling, and accessibility
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
convex/messages.ts (2)
1084-1159: Shared messages sanitization is solid; consider broadening placeholder coverage.The
getSharedMessagesquery correctly gates onshare_id/share_date, freezes content toshare_date, and stripsuser_id, which is great for a public “frozen share” surface. The only thing I’d consider is future‑proofing thepartssanitization: right now it only replacestype === "file"parts, so if you later introduce other part types that can carry file/media URLs (e.g. explicit"image"or rich card types), those would bypass the placeholder logic.If you expect more part types in the future, you might want to centralize “file-ish part” detection or explicitly handle any known media types here to avoid accidental URL leakage.
1166-1217: Return type allowssystemrole, but implementation never emits it.The
returnsvalidator forgetPreviewMessagesincludes"system"in theroleunion, but the implementation filters tom.role === "user" || m.role === "assistant"before slicing. Either the type can drop"system"or the implementation should be updated if you ever want system messages in the preview.Not a bug today, just a small type/behavior mismatch to keep in mind.
app/components/ShareDialog.tsx (1)
53-83: Defensively guard mutation results to avoidshareId/shareDateruntime errors.Given the test pipeline saw
Cannot read properties of undefined (reading 'shareId')in this block, it’d be safer to defensively validate the mutation results before using them. That protects you both from misconfigured tests and from any future backend contract drift.For example:
const handleAutoShare = async () => { setIsGenerating(true); try { if (existingShareId) { // Update existing share to include new messages const result = await updateShareDate({ chatId }); + if (!result || typeof result.shareDate !== "number") { + throw new Error("Invalid updateShareDate response"); + } const url = `${window.location.origin}/share/${existingShareId}`; setShareUrl(url); setShareDate(result.shareDate); } else { // Create new share const result = await shareChat({ chatId }); - const url = `${window.location.origin}/share/${result.shareId}`; - setShareUrl(url); - setShareDate(result.shareDate); + if (!result || !result.shareId || typeof result.shareDate !== "number") { + throw new Error("Invalid shareChat response"); + } + const url = `${window.location.origin}/share/${result.shareId}`; + setShareUrl(url); + setShareDate(result.shareDate); } } catch (err) {This keeps the happy path the same but turns “undefined property access” into a controlled error that goes through your existing error handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/components/ShareDialog.tsx(1 hunks)app/components/__tests__/ShareDialog.test.tsx(1 hunks)components/ui/dialog.tsx(1 hunks)convex/messages.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
app/components/__tests__/ShareDialog.test.tsx (1)
app/components/ShareDialog.tsx (1)
ShareDialog(28-345)
convex/messages.ts (1)
convex/_generated/server.js (2)
query(29-29)query(29-29)
app/components/ShareDialog.tsx (3)
convex/chats.ts (2)
shareChat(895-949)updateShareDate(964-1007)app/components/MemoizedMarkdown.tsx (1)
MemoizedMarkdown(9-32)components/icons/hackerai-svg.tsx (1)
HackerAISVG(8-67)
🪛 GitHub Actions: Run Tests
app/components/__tests__/ShareDialog.test.tsx
[error] 78-78: An update to ShareDialog inside a test was not wrapped in act(...).
[error] 131-131: Auto-generation of Share Link test failed. Expected mockShareChat to be called with { chatId: 'test-chat-id' } but it was not called.
[error] 143-143: Auto-generation of Share Link test failed. Copy link button not found in document.
app/components/ShareDialog.tsx
[error] 69-79: Runtime error observed during share link generation: Cannot read properties of undefined (reading 'shareId').
🔇 Additional comments (1)
components/ui/dialog.tsx (1)
63-63: DialogContent layout change looks good.The updated class list still gives a centered, constrained dialog with appropriate animations and doesn’t affect behavior or accessibility; it should work fine with the new ShareDialog layout.
- Create ManageSharedChatsDialog component wrapping SharedLinksTab - Remove SharedLinksTab from DataControlsTab inline display - Add "Manage" button with Share2 icon to open dialog - Improve UX by separating shared chats management from data controls
… manage buttons - Remove Share2 icon from manage button - Keep consistent style with other manage buttons (e.g., manage memories)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/components/ManageSharedChatsDialog.tsx (1)
23-23: Consider extracting the complex className for better readability.The className string contains many responsive utilities in a single line, which can be harder to maintain. Consider extracting it to a constant or using
clsx/cnwith line breaks for improved readability.Example refactor:
const dialogContentClasses = [ "w-[380px] max-w-[98%]", "md:w-[95vw] md:max-w-[720px]", "max-h-[95%] md:h-[600px]", "p-0 overflow-hidden rounded-[20px]" ].join(" "); // Then use: <DialogContent className={dialogContentClasses} showCloseButton={!isMobile} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/components/DataControlsTab.tsx(3 hunks)app/components/ManageSharedChatsDialog.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/DataControlsTab.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/components/ManageSharedChatsDialog.tsx (1)
hooks/use-mobile.ts (1)
useIsMobile(5-21)
🔇 Additional comments (1)
app/components/ManageSharedChatsDialog.tsx (1)
18-18: Verify that theuseIsMobilepattern doesn't cause hydration issues.The
useIsMobilehook initially returnsfalse(since!!undefinedequalsfalse) and then updates after mount. This could potentially cause a hydration mismatch or a flash of incorrect UI when the component first renders versus after the effect runs.While this pattern is common for responsive hooks in client components, please verify that:
- No hydration warnings appear in the console during development
- The UI doesn't flash between mobile and desktop layouts on initial render
- Consider if this dialog could benefit from a loading state or suppressed rendering until
isMobileis determined
…chats - Add Header component with Sign In/Sign Up buttons for unlogged users - Add floating chat input at bottom for unlogged users - Move footer below chat input for unlogged users - Add chat sidebar (collapsed by default) for logged users - Add ChatHeader and ChatInput for logged users - Add mobile sidebar support for logged users - Remove shared conversation info text - Logged users can create new chat from shared chat view
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/share/[shareId]/SharedChatView.tsx (1)
205-205: Remove unusedshareDateprop.This prop was already flagged in a previous review as unused in the SharedMessages component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/share/[shareId]/SharedChatView.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/share/[shareId]/SharedChatView.tsx (6)
app/share/[shareId]/SharedChatContext.tsx (2)
useSharedChatContext(41-49)SharedChatProvider(17-39)app/components/ComputerSidebar.tsx (1)
ComputerSidebarBase(26-334)hooks/use-mobile.ts (1)
useIsMobile(5-21)app/contexts/GlobalState.tsx (1)
useGlobalState(704-710)app/share/[shareId]/SharedMessages.tsx (1)
SharedMessages(32-337)app/components/ChatInput.tsx (1)
ChatInput(72-492)
🔇 Additional comments (1)
app/share/[shareId]/SharedChatView.tsx (1)
65-66: Good security practice: UUID validation before database query.Validating the shareId format before querying prevents unnecessary database calls and provides immediate feedback for malformed URLs.
| <ChatInput | ||
| onSubmit={handleChatSubmit} | ||
| onStop={handleChatStop} | ||
| onSendNow={() => {}} | ||
| status="ready" | ||
| isNewChat={true} | ||
| clearDraftOnSubmit={false} | ||
| /> |
There was a problem hiding this comment.
Add missing hasMessages prop for consistency.
The ChatInput for unauthenticated users is missing the hasMessages prop that is provided to the authenticated version (line 218). This inconsistency could affect UI behavior.
Apply this diff to fix the inconsistency:
<div className="mx-auto w-full max-w-full sm:max-w-[768px] sm:min-w-[390px]">
<ChatInput
onSubmit={handleChatSubmit}
onStop={handleChatStop}
onSendNow={() => {}}
status="ready"
isNewChat={true}
+ hasMessages={false}
clearDraftOnSubmit={false}
/>
</div>🤖 Prompt for AI Agents
In app/share/[shareId]/SharedChatView.tsx around lines 230 to 237, the ChatInput
used for unauthenticated users is missing the hasMessages prop causing
inconsistent UI behavior compared to the authenticated variant at line 218;
update the ChatInput invocation within those lines to include
hasMessages={hasMessages} (same value passed to the authenticated ChatInput) so
both branches receive the prop and UI logic stays consistent.
…ators - Display chat title dynamically in browser tab and header - Update document.title client-side when chat loads - Show title in Header component center for non-logged users on desktop - ChatHeader displays title for logged users as before - Fix file upload indicator to render outside message bubble - Separate file/image placeholders from message text content - Display 'Uploaded a file/image' without message background - Matches ChatGPT-style presentation - Remove chat sidebar from shared chat views for cleaner experience
- Add 10 focused tests covering core functionality - Test rendering, loading states, error handling - Test dialog close behavior and edge cases - All tests passing
- Replace div with button for close control on mobile - Add type="button" to prevent form submission - Add aria-label="Close" for screen readers - Maintains same styling and behavior - Fixes keyboard navigation for dialog close
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/components/ManageSharedChatsDialog.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/components/ManageSharedChatsDialog.tsx (1)
hooks/use-mobile.ts (1)
useIsMobile(5-21)
🔇 Additional comments (3)
app/components/ManageSharedChatsDialog.tsx (3)
1-12: LGTM!The imports are well-organized, and the interface definition properly types the component props.
32-39: Accessibility issue resolved!The close button now correctly uses a semantic
buttonelement withtype="button"andaria-label="Close", making it keyboard accessible. This addresses the previous review comment about the non-focusabledivelement.
44-56: LGTM!The dialog structure is well-organized with:
- Proper flex layout with
min-h-0to prevent overflow issues- Responsive padding and spacing
- Correct composition with
SharedLinksTabas the main content- Good accessibility with sr-only
DialogTitle
| const isMobile = useIsMobile(); | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <DialogContent | ||
| className="w-[380px] max-w-[98%] md:w-[95vw] md:max-w-[720px] max-h-[95%] md:h-[600px] p-0 overflow-hidden rounded-[20px]" | ||
| showCloseButton={!isMobile} |
There was a problem hiding this comment.
Potential hydration mismatch and layout shift.
The useIsMobile hook returns false initially (before the effect runs), then updates to the actual viewport state. This causes:
- The dialog to initially render in desktop mode even on mobile devices
- A visible layout shift when the effect fires and switches to mobile layout
- The close button visibility to toggle (
showCloseButton={!isMobile}) - Both conditional headers (lines 28-42 and 45-49) to swap after hydration
This creates a poor user experience with visible content shifting.
Consider one of these approaches:
- Use CSS media queries instead of JavaScript for responsive layout
- Suppress initial render until
isMobileis determined - Accept the limitation and ensure the desktop layout is acceptable as a fallback
Apply this diff for option 2:
}: ManageSharedChatsDialogProps) => {
const isMobile = useIsMobile();
+ // Suppress render until mobile state is determined to avoid layout shift
+ if (isMobile === undefined) return null;
+
return (
<Dialog open={open} onOpenChange={onOpenChange}>Note: Option 1 (CSS media queries) would require restructuring the component but provides the best UX without layout shifts.
📝 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.
| const isMobile = useIsMobile(); | |
| return ( | |
| <Dialog open={open} onOpenChange={onOpenChange}> | |
| <DialogContent | |
| className="w-[380px] max-w-[98%] md:w-[95vw] md:max-w-[720px] max-h-[95%] md:h-[600px] p-0 overflow-hidden rounded-[20px]" | |
| showCloseButton={!isMobile} | |
| const isMobile = useIsMobile(); | |
| // Suppress render until mobile state is determined to avoid layout shift | |
| if (isMobile === undefined) return null; | |
| return ( | |
| <Dialog open={open} onOpenChange={onOpenChange}> | |
| <DialogContent | |
| className="w-[380px] max-w-[98%] md:w-[95vw] md:max-w-[720px] max-h-[95%] md:h-[600px] p-0 overflow-hidden rounded-[20px]" | |
| showCloseButton={!isMobile} |
🤖 Prompt for AI Agents
In app/components/ManageSharedChatsDialog.tsx around lines 18 to 24, the
component uses useIsMobile which defaults to false and causes a hydration/layout
shift and toggling of the close button; to fix, defer the component's
responsive-dependent render until the hook has resolved by detecting an
"initialized" flag (or hook-provided ready state) and render a neutral
placeholder (or nothing) on the server/initial client pass; once isMobile is
determined, render the Dialog with showCloseButton set from the resolved
isMobile and select the appropriate header to avoid swapping after hydration.
|
I tested the chat sharing feature, and it works as expected. Pages are accessible for both logged-in and non-logged-in users. Users can easily unshare chats immediately through the settings. The social media links also seem to work well (I tested it with X). The user interface looks good. |
Summary by CodeRabbit
New Features
Bug Fixes
Tests