Repository navigation
fix: broadcast message created events - #11
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces bulk message inserts with a typed per-message createMessage flow, adds a TRPC sendMessage mutation, emits realtime MESSAGE_CREATED events (broadcast via WS), updates REST handlers to call createMessage, adds client-side conversation message cache helpers, and expands message schema with parentMessageId and modelUsed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Web UI
participant REST as REST API
participant TRPC as TRPC API
participant Util as createMessage (utils/message)
participant DB as DB (sendMessages / insert)
participant Pub as PubSub (publish)
participant WS as WS Router
participant Clients as Subscribed Clients
UI->>TRPC: sendMessage({conversationId, websiteSlug, bodyMd, type, visibility})
TRPC->>Util: createMessage({db, orgId, websiteId, conversationId, message})
Util->>DB: insert message (per-message)
DB-->>Util: created Message
Util->>Pub: publish(MESSAGE_CREATED, { message, conversationId, websiteId, organizationId })
Util-->>TRPC: Message
TRPC-->>UI: { message }
Pub->>WS: routeEvent(MESSAGE_CREATED)
WS->>Clients: emitToAll(conversationId, websiteId, "MESSAGE_CREATED", data)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/src/rest/routers/messages.ts (1)
195-204: Return 400 for missing Visitor ID (currently returns 200)OpenAPI declares 400 for invalid requests, but this path returns a 200 with an error payload.
- if (!visitorId) { - return c.json( - validateResponse( - { error: "Visitor ID is required" }, - z.object({ error: z.string() }), - ), - ); - } + if (!visitorId) { + return c.json( + { error: "Visitor ID is required" }, + 400, + ); + }apps/api/src/rest/routers/conversation.ts (1)
404-414: Remove sensitive debug logging.This logs Authorization/Public keys; violates least‑logging and “don’t use console.”
- // Debug: Check what's in the context - console.log("[DEBUG] GET /conversations/{id} - Context check:", { - hasDb: !!c.get("db"), - hasWebsite: !!c.get("website"), - hasOrganization: !!c.get("organization"), - hasApiKey: !!c.get("apiKey"), - headers: { - authorization: c.req.header("Authorization"), - publicKey: c.req.header("X-Public-Key"), - visitorId: c.req.header("X-Visitor-Id"), - }, - }); + // Debug logging removed: avoid emitting secrets in logs
🧹 Nitpick comments (11)
apps/web/src/data/use-conversation-messages.tsx (2)
7-10: Naming consistency: this is “messages”, not “headers”Rename the options interface to match the hook’s domain.
-interface UseConversationHeadersOptions { +interface UseConversationMessagesOptions { limit?: number; enabled?: boolean; } … -export function useConversationMessages({ +export function useConversationMessages({ websiteSlug, conversationId, options, }: { websiteSlug: string; conversationId: string; - options?: UseConversationHeadersOptions; + options?: UseConversationMessagesOptions; }) {Also applies to: 12-20
29-39: Optional: avoid double-caching via fetchQueryUsing
queryClient.fetchQuerywith a different key thanuseInfiniteQuery.queryKeywrites twice to cache. Prefer calling the TRPC fetcher directly (orqueryFninline) to keep a single cache path.apps/web/src/data/conversation-message-cache.ts (1)
79-110: Prune empty pages after deletionsAfter removals, pages with zero items are retained. Trim them to reduce noise and re-renders.
const pages = existing.pages.map((page) => { const filtered = page.items.filter((item) => item.id !== messageId); if (filtered.length !== page.items.length) { removed = true; return { ...page, items: filtered, }; } return page; }); if (!removed) { return existing; } - return { - pages, - pageParams: [...existing.pageParams], - }; + const prunedPages = pages.filter((p) => p.items.length > 0); + const prunedParams = + prunedPages.length === pages.length + ? [...existing.pageParams] + : existing.pageParams.slice(0, prunedPages.length); + return { + pages: prunedPages, + pageParams: prunedParams, + };apps/api/src/ws/router.ts (1)
32-44: Replace console. with centralized logger*Guidelines prohibit
consolein runtime code. Use a structured logger (info/warn/error) and ensure PII is not logged.- console.log(`[USER_CONNECTED] User ${data.userId} connected`, { … }); + logger.info(`[USER_CONNECTED] User ${data.userId} connected`, { … }); - console.error(`[EventRouter] Error handling ${event.type}:`, error); + logger.error({ err: error, eventType: event.type }, "[EventRouter] handler error");Also applies to: 46-58, 60-72, 74-89, 91-105, 134-145
packages/types/src/realtime-events.ts (2)
34-58: Use zod’s datetime validator for timestamp stringsDeclare timestamp strings as ISO datetimes for stronger validation and clearer intent.
- createdAt: z.string(), - updatedAt: z.string(), - deletedAt: z.string().nullable(), + createdAt: z.string().datetime(), + updatedAt: z.string().datetime(), + deletedAt: z.string().datetime().nullable(),Also consider documenting timezone expectations (UTC vs local) to avoid ambiguity.
61-67: Minor: document event field units and constraintsAdd brief JSDoc that
timestampand presencelastSeenare epoch millis, and thatmessage.type/visibilitymirror the enums from@cossistant/types.Also applies to: 69-82
apps/api/src/utils/message.ts (2)
79-99: Defensive check: ensure DB returned a row.
sendMessagesshould return one row; handle empty/undefined to avoid runtime errors.const [createdMessage] = await sendMessages(db, { ... }); - const parsedMessage = messageSchema.parse(createdMessage); + if (!createdMessage) { + throw new Error("Failed to create message: insert returned no rows"); + } + const parsedMessage = messageSchema.parse(createdMessage);
16-20: Optional: expand normalization + docs.Consider trimming and normalizing newlines; add JSDoc since this is part of the public flow.
-export function prepareMessageForInsert(bodyMd: string) { - const normalized = bodyMd.normalize("NFC"); - return { bodyMd: normalized }; +/** + * Normalize message markdown before insert. + * - NFC normalization + * - Optional whitespace/newline normalization (non-destructive) + */ +export function prepareMessageForInsert(bodyMd: string) { + const normalized = bodyMd.normalize("NFC"); + return { bodyMd: normalized }; }apps/web/src/components/conversation/index.tsx (3)
71-75: More collision‑resistant optimistic IDs.
Date.now()can collide under fast sends. Prefercrypto.randomUUID()with a fallback.- const optimisticMessageId = `optimistic-${Date.now()}`; + const optimisticMessageId = + `optimistic-${(globalThis.crypto?.randomUUID?.() ?? + `${Date.now()}-${Math.random().toString(36).slice(2)}`)}`;
157-167: Handle MESSAGE_CREATED by updating the cache, not logging.Apply event to the infinite list so other clients’ messages appear in real time.
- const handleRealtimeEvent = useCallback( - (event: RealtimeEvent) => { - if ( - event.type === "MESSAGE_CREATED" && - event.data.conversationId === conversationId - ) { - console.log("[Conversation] MESSAGE_CREATED event received", event.data); - } - }, - [conversationId], - ); + const handleRealtimeEvent = useCallback( + (event: RealtimeEvent) => { + if (event.type !== "MESSAGE_CREATED") return; + if (event.data.conversationId !== conversationId) return; + const m = event.data.message; + const normalized: ConversationMessage = { + ...m, + createdAt: new Date(m.createdAt), + updatedAt: new Date(m.updatedAt), + deletedAt: m.deletedAt ? new Date(m.deletedAt) : null, + }; + upsertConversationMessageInCache( + queryClient, + messagesInfiniteQueryKey, + normalized, + ); + }, + [conversationId, queryClient, messagesInfiniteQueryKey], + );
163-164: Remove console usage in UI.Avoid
console.*per guidelines; the cache update above supersedes logging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/api/src/rest/routers/conversation.ts(2 hunks)apps/api/src/rest/routers/messages.ts(2 hunks)apps/api/src/trpc/routers/conversation.ts(3 hunks)apps/api/src/utils/message.ts(1 hunks)apps/api/src/ws/router.test.ts(3 hunks)apps/api/src/ws/router.ts(2 hunks)apps/web/src/components/conversation/index.tsx(3 hunks)apps/web/src/data/conversation-message-cache.ts(1 hunks)apps/web/src/data/use-conversation-messages.tsx(2 hunks)packages/types/src/realtime-events.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/global-rules.mdc)
**/*.{ts,tsx}: Manage shared component state via React Context
Implement full TypeScript support for all public APIs
Provide proper type definitions for all props and refs
Use appropriate generic constraints where necessary
Export type definitions for the public API
Avoid TypeScript enums; prefer const objects with 'as const' or Object.freeze
Provide clear, descriptive runtime errors for missing context/props and misuse
Expose dedicated hooks for context access that throw helpful errors when context is missing
Add explicit error messages for: outside-context usage, missing required props, invalid prop combinations, and validation failures
Document each component with JSDoc: purpose, props (required/optional), context requirements, example usage, pitfalls
State management: use React Context for global state, perform correct state updates, and handle side effects appropriately
Validation: implement runtime checks and validate prop combinations with clear errors
Create context with createContext<ContextType|null>(null) and a useContext hook that throws a clear error when missing
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't have unused function parameters
Don't have unused private class members
Don't return a value from a function that has a 'void' return type
Don't use the TypeScript directive @ts-ignore
Ensure switch-case statements are exhaustive (especially with unions)
Don't use TypeScript enums
Don't add type annotations to variables, parameters, and class properties initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types (where discouraged by policy)
Use as const instead of literal types and type annotations
Use either T[] or Array consistently
Use consistent accessibili...
Files:
apps/api/src/ws/router.test.tsapps/web/src/components/conversation/index.tsxapps/api/src/trpc/routers/conversation.tsapps/api/src/ws/router.tsapps/web/src/data/conversation-message-cache.tsapps/api/src/rest/routers/messages.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/utils/message.tsapps/api/src/rest/routers/conversation.tspackages/types/src/realtime-events.ts
**/*.{test.ts,test.tsx,spec.ts,spec.tsx,test.js,test.jsx,spec.js,spec.jsx}
📄 CodeRabbit inference engine (.cursor/rules/global-rules.mdc)
Include tests: unit tests for components, integration tests for composed components, accessibility tests, and error boundary tests
Files:
apps/api/src/ws/router.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't use theaccessKeyattribute on any HTML element
Use correct ISO language/country codes for the lang attribute
Don't hardcode sensitive data like API keys and tokens
Use === and !==
Files:
apps/api/src/ws/router.test.tsapps/web/src/components/conversation/index.tsxapps/api/src/trpc/routers/conversation.tsapps/api/src/ws/router.tsapps/web/src/data/conversation-message-cache.tsapps/api/src/rest/routers/messages.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/utils/message.tsapps/api/src/rest/routers/conversation.tspackages/types/src/realtime-events.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of statements instead of Array.forEach
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules that don't change anything
Don't use unnecessary escape sequences in regular expression literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports, exports, and destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when there are no escape sequences
Don't use useless case statements in switch statements
Avoid ternary operators when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use void operators
Use Date.now() to get milliseconds since the Unix Epoch
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed property access
Don't use parseInt() or Number.parseInt() when numeric literals suffice (binary/octal/hex)
Use concise optional chaining instead of chained logical expressions
Prefer regular expression literals over the RegExp constructor when possible
Don't use non–base-10 or underscore-separated number literal object member names
Remove redundant terms from logical expressions
Prefer while loops over for loops when no initializer/update is needed
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp values when result is constant
Don't use empty character classes in regular expression literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions and vars that are acce...
Files:
apps/api/src/ws/router.test.tsapps/api/src/trpc/routers/conversation.tsapps/api/src/ws/router.tsapps/web/src/data/conversation-message-cache.tsapps/api/src/rest/routers/messages.tsapps/api/src/utils/message.tsapps/api/src/rest/routers/conversation.tspackages/types/src/realtime-events.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Don't nest describe() blocks too deeply in test files
Don't use callbacks in asynchronous tests and hooks
Don't have duplicate hooks in describe blocks
Don't use export or module.exports in test files
Don't use focused tests
Ensure the assertion function (e.g., expect) is placed inside an it() function call
Don't use disabled tests
Files:
apps/api/src/ws/router.test.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts,tsx}: Don't create classes that only have static members
Don't return a value from a constructor
Don't use super() incorrectly inside classes; ensure super() is called in derived constructors
Ensure super() is called exactly once on every code path in a class constructor before this is accessed if the class has a superclass
Place getters and setters for the same property next to each other in class and object definitions
Ensure JSDoc comment lines start with a single asterisk, except for the first one
Don't use duplicate class members
Files:
apps/api/src/ws/router.test.tsapps/web/src/components/conversation/index.tsxapps/api/src/trpc/routers/conversation.tsapps/api/src/ws/router.tsapps/web/src/data/conversation-message-cache.tsapps/api/src/rest/routers/messages.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/utils/message.tsapps/api/src/rest/routers/conversation.tspackages/types/src/realtime-events.ts
**/*.{js,tsx,ts,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Use arrow functions instead of function expressions
Files:
apps/api/src/ws/router.test.tsapps/web/src/components/conversation/index.tsxapps/api/src/trpc/routers/conversation.tsapps/api/src/ws/router.tsapps/web/src/data/conversation-message-cache.tsapps/api/src/rest/routers/messages.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/utils/message.tsapps/api/src/rest/routers/conversation.tspackages/types/src/realtime-events.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts,jsx,tsx}: Prevent duplicate polyfills from Polyfill.io
Don't use irregular whitespace characters
Files:
apps/api/src/ws/router.test.tsapps/web/src/components/conversation/index.tsxapps/api/src/trpc/routers/conversation.tsapps/api/src/ws/router.tsapps/web/src/data/conversation-message-cache.tsapps/api/src/rest/routers/messages.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/utils/message.tsapps/api/src/rest/routers/conversation.tspackages/types/src/realtime-events.ts
**/*.{js,ts,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't redeclare variables, functions, classes, and types in the same scope
Files:
apps/api/src/ws/router.test.tsapps/web/src/components/conversation/index.tsxapps/api/src/trpc/routers/conversation.tsapps/api/src/ws/router.tsapps/web/src/data/conversation-message-cache.tsapps/api/src/rest/routers/messages.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/utils/message.tsapps/api/src/rest/routers/conversation.tspackages/types/src/realtime-events.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/global-rules.mdc)
**/*.tsx: Use the Compound Components pattern with namespaced exports (e.g., FormWizard.Root, FormWizard.Step) so pieces compose under a single parent
Ensure accessibility: keyboard navigation support
Ensure accessibility: include appropriate ARIA attributes
Ensure accessibility: use semantic markup for screen readers
Maintain proper focus management in interactive components
Structure components with forwardRef and typed props/refs as shown (ComponentRef, HTMLAttributes, ReactNode)
Root component acts as main context provider, manages global state, and handles initialization
Child components must be used within Root, read context via hooks, support asChild, and follow consistent props
Props ordering: list required props first; optional props have clear defaults; support common HTML attributes; expose className
Support composition with an asChild prop using Radix UI Slot to allow element replacement
Styling: support className, use data-* attributes for state, and allow style customization
Event handling: support standard DOM events, add custom events when needed, and manage propagation correctly
Base component pattern: forwardRef, choose Slot or intrinsic element via asChild, pass ref/className and spread props
Files:
apps/web/src/components/conversation/index.tsxapps/web/src/data/use-conversation-messages.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include the words "image", "picture", or "photo" in img alt prop
Don't use an explicit role that's the same as the element's implicit role
Static elements with click handlers must have a valid role attribute
Always include a <title> element within SVG elements
Provide meaningful alt text for elements that require alt text
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements only when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Elements with interactive roles and handlers must be focusable
Heading elements must have content accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements instead of role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid ARIA state and property values
Use valid values for the autocomplete attribute on input elements
Don't use unnecessary fragmen...Files:
apps/web/src/components/conversation/index.tsxapps/web/src/data/use-conversation-messages.tsx**/*.{jsx,tsx,html}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx,html}: Ensure the preconnect attribute is used when using Google Fonts
Don't use target="_blank" without rel="noopener"
Use a recommended display strategy with Google FontsFiles:
apps/web/src/components/conversation/index.tsxapps/web/src/data/use-conversation-messages.tsxpackages/types/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/types/src/**/*.ts: Keep shared TypeScript types and Zod schemas in packages/types/src/ and update them alongside schema changes
Ensure Zod schemas mirror any API/database shape changes for end-to-end type safetyFiles:
packages/types/src/realtime-events.ts🧬 Code graph analysis (7)
apps/api/src/ws/router.test.ts (3)
apps/api/src/lib/pubsub.ts (1)
emitToAll(591-598)packages/types/src/realtime-events.ts (1)
RealtimeEvent(63-67)apps/api/src/ws/router.ts (1)
routeEvent(128-146)apps/web/src/components/conversation/index.tsx (6)
apps/web/src/lib/trpc/client.tsx (1)
useTRPC(18-18)apps/web/src/data/conversation-message-cache.ts (5)
createConversationMessagesInfiniteQueryKey(112-116)ConversationMessagesPage(4-5)ConversationMessage(6-7)upsertConversationMessageInCache(118-127)removeConversationMessageFromCache(129-138)packages/types/src/enums.ts (4)
MessageType(1-5)MessageType(7-7)MessageVisibility(33-36)MessageVisibility(74-75)packages/react/src/hooks/use-multimodal-input.ts (1)
useMultimodalInput(31-187)packages/types/src/realtime-events.ts (1)
RealtimeEvent(63-67)apps/web/src/app/(dashboard)/[websiteSlug]/providers/websocket.tsx (1)
useDashboardRealtime(251-274)apps/api/src/trpc/routers/conversation.ts (5)
apps/api/src/trpc/init.ts (1)
protectedProcedure(52-54)packages/types/src/enums.ts (4)
MessageType(1-5)MessageType(7-7)MessageVisibility(33-36)MessageVisibility(74-75)packages/types/src/schemas.ts (1)
messageSchema(9-21)apps/api/src/db/queries/website.ts (1)
getWebsiteBySlugWithAccess(229-243)apps/api/src/utils/message.ts (1)
createMessage(68-112)apps/api/src/ws/router.ts (1)
apps/api/src/lib/pubsub.ts (2)
emitToDashboard(583-589)emitToAll(591-598)apps/api/src/rest/routers/messages.ts (3)
apps/api/src/utils/message.ts (1)
createMessage(68-112)apps/api/src/utils/validate.ts (1)
validateResponse(6-20)packages/types/src/api/message.ts (1)
sendMessageResponseSchema(40-42)apps/web/src/data/use-conversation-messages.tsx (1)
apps/web/src/data/conversation-message-cache.ts (1)
createConversationMessagesInfiniteQueryKey(112-116)apps/api/src/utils/message.ts (6)
apps/api/src/db/index.ts (2)
Database(34-34)db(36-41)packages/types/src/enums.ts (4)
MessageType(1-5)MessageType(7-7)MessageVisibility(33-36)MessageVisibility(74-75)packages/types/src/schemas.ts (2)
Message(23-23)messageSchema(9-21)packages/types/src/realtime-events.ts (1)
RealtimeEventData(69-71)apps/api/src/db/queries/message.ts (1)
sendMessages(7-46)apps/api/src/lib/pubsub.ts (1)
emitToAll(591-598)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (7)
apps/api/src/ws/router.ts (1)
107-122: MESSAGE_CREATED broadcast looks correctForwarding to all relevant channels with
emitToAll(conversationId, websiteId, "MESSAGE_CREATED", data)aligns with the new event contract.apps/api/src/ws/router.test.ts (2)
80-121: Good coverage: verifies MESSAGE_CREATED is forwarded via emitToAllThe mock and assertion validate the pubsub forwarding path end-to-end.
5-10: Nice: emitToAll is mocked/reset alongside emitToDashboardKeeps tests isolated and avoids cross-test leakage.
Also applies to: 18-21
apps/api/src/trpc/routers/conversation.ts (1)
211-269: sendMessage mutation is well‑scoped and consistent with server invariantsValidates access, binds conversation to website, creates a single message via
createMessage, and returnsmessageSchema. Looks good.apps/api/src/rest/routers/messages.ts (1)
204-223: No change required — request schema already requires Date for createdAtsendMessageRequestSchema declares createdAt: z.date().optional() (packages/types/src/api/message.ts), so validated body.message.createdAt should already be a Date; only add a conversion if you plan to accept string timestamps (use z.coerce.date() or convert before calling createMessage).
apps/web/src/data/conversation-message-cache.ts (1)
38-77: Pagination invariant: don't assume the last page is the newestServer-side message queries use both DESC and ASC on createdAt (apps/api/src/db/queries/message.ts — .orderBy(desc(message.createdAt)) and .orderBy(asc(message.createdAt)); getConversationMessages is invoked from apps/api/src/trpc/routers/conversation.ts:132). Current upsert appends a missing message to the last page — instead insert by createdAt into the page that matches the message timestamp (or re-fetch/merge/sort pages), or standardize the server ordering/cursor semantics.
apps/api/src/utils/message.ts (1)
38-66: No change required — realtime schema expects ISO strings.
MESSAGE_CREATED's message.createdAt/updatedAt/deletedAt are defined as z.string() in packages/types/src/realtime-events.ts, so using toISOString() is correct.
| await emitToAll( | ||
| conversationId, | ||
| websiteId, | ||
| "MESSAGE_CREATED", | ||
| serializeMessageForRealtime(parsedMessage, { | ||
| conversationId, | ||
| websiteId, | ||
| organizationId, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Don’t fail writes if pubsub emit errors; catch and downgrade to telemetry.
Event emission shouldn’t break message creation. Wrap in try/catch (or .catch) and avoid console logging.
Apply:
- await emitToAll(
- conversationId,
- websiteId,
- "MESSAGE_CREATED",
- serializeMessageForRealtime(parsedMessage, {
- conversationId,
- websiteId,
- organizationId,
- }),
- );
+ try {
+ await emitToAll(
+ conversationId,
+ websiteId,
+ "MESSAGE_CREATED",
+ serializeMessageForRealtime(parsedMessage, {
+ conversationId,
+ websiteId,
+ organizationId,
+ }),
+ );
+ } catch {
+ // Non-blocking: record to telemetry/logging; do not throw
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await emitToAll( | |
| conversationId, | |
| websiteId, | |
| "MESSAGE_CREATED", | |
| serializeMessageForRealtime(parsedMessage, { | |
| conversationId, | |
| websiteId, | |
| organizationId, | |
| }), | |
| ); | |
| try { | |
| await emitToAll( | |
| conversationId, | |
| websiteId, | |
| "MESSAGE_CREATED", | |
| serializeMessageForRealtime(parsedMessage, { | |
| conversationId, | |
| websiteId, | |
| organizationId, | |
| }), | |
| ); | |
| } catch { | |
| // Non-blocking: record to telemetry/logging; do not throw | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/utils/message.ts around lines 100 to 109, the call to emitToAll
can throw and currently can break message creation; wrap the emit call in a
try/catch (or append .catch) so any errors are swallowed and do not propagate,
and in the catch send the error to the project's telemetry/monitoring API (e.g.,
telemetry.captureException or equivalent) with contextual metadata
(conversationId, websiteId, organizationId) instead of using console.log, then
continue normally without rethrowing.
| return [...messages].sort( | ||
| (a, b) => a.createdAt.getTime() - b.createdAt.getTime(), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Realtime dates are strings; cache helpers expect Date objects
sortMessagesByCreatedAt calls getTime(), so any realtime payload with ISO strings will crash unless coerced. Export a small helper to normalize dates before calling upsertConversationMessageInCache.
Apply this addition near the bottom of the file:
+// Normalize ISO date strings from realtime into Date objects for cache helpers.
+export function coerceMessageDates<T extends { createdAt: unknown; updatedAt: unknown; deletedAt: unknown }>(
+ m: T,
+): T & { createdAt: Date; updatedAt: Date; deletedAt: Date | null } {
+ return {
+ ...m,
+ createdAt: m.createdAt instanceof Date ? m.createdAt : new Date(m.createdAt as string),
+ updatedAt: m.updatedAt instanceof Date ? m.updatedAt : new Date(m.updatedAt as string),
+ deletedAt:
+ m.deletedAt == null
+ ? null
+ : m.deletedAt instanceof Date
+ ? (m.deletedAt as Date)
+ : new Date(m.deletedAt as string),
+ };
+}Use this in the UI’s realtime handler before calling upsertConversationMessageInCache(...).
📝 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.
| return [...messages].sort( | |
| (a, b) => a.createdAt.getTime() - b.createdAt.getTime(), | |
| ); | |
| } | |
| return [...messages].sort( | |
| (a, b) => a.createdAt.getTime() - b.createdAt.getTime(), | |
| ); | |
| } | |
| // Normalize ISO date strings / numeric timestamps from realtime into Date objects for cache helpers. | |
| export function coerceMessageDates<T extends { createdAt: unknown; updatedAt: unknown; deletedAt?: unknown }>( | |
| m: T, | |
| ): T & { createdAt: Date; updatedAt: Date; deletedAt: Date | null } { | |
| const toDate = (v: unknown): Date => { | |
| if (v instanceof Date) return v; | |
| if (typeof v === 'number') return new Date(v); | |
| if (typeof v === 'string') return new Date(v); | |
| throw new TypeError('Invalid date value: expected Date, string, or number.'); | |
| }; | |
| return { | |
| ...m, | |
| createdAt: toDate(m.createdAt), | |
| updatedAt: toDate(m.updatedAt), | |
| deletedAt: m.deletedAt === undefined || m.deletedAt === null ? null : toDate(m.deletedAt), | |
| }; | |
| } |
🤖 Prompt for AI Agents
In apps/web/src/data/conversation-message-cache.ts around lines 12–15,
sortMessagesByCreatedAt assumes message.createdAt is a Date and calls getTime(),
which will throw for realtime payloads where createdAt is an ISO string; add and
export a small helper (e.g., normalizeMessageDates or
ensureMessageCreatedAtIsDate) near the bottom of this file that accepts a
message or array of messages and coerces any string createdAt into new
Date(createdAt) while leaving Date objects untouched, then update callers (the
UI realtime handler) to run messages through this helper before calling
upsertConversationMessageInCache(...).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/data/use-conversation-messages.tsx (2)
15-15: Wrong staleTime value (off by 1,000×) — prevents timely refreshesComment says 5 minutes, but 300_000_000 ms ≈ 83 hours.
Apply:
-const STALE_TIME = 300_000_000; +const STALE_TIME = 300_000;
38-45: Cursor type mismatch: pageParam should be string (message id), not DateAPI expects cursor: string (message id). Initial and subsequent pageParam values are strings. Typing it as Date risks incorrect values and confusion.
- getNextPageParam: (lastPage) => lastPage.nextCursor, - initialPageParam: null as Date | null, + getNextPageParam: (lastPage) => lastPage.nextCursor, + initialPageParam: null as string | null,Also applies to: 51-51
♻️ Duplicate comments (1)
apps/web/src/data/use-conversation-messages.tsx (1)
29-35: LGTM: queryKey now includes limit (prevents cache collisions)The base key includes limit; this resolves the previously reported collision.
🧹 Nitpick comments (2)
apps/api/src/ws/router.ts (1)
33-44: Replace console. with structured logger*Repository guidelines disallow console.*. Use a shared logger (e.g., pino/winston) or your existing logging util, with consistent levels and metadata.
Example (illustrative):
- console.log(`[USER_CONNECTED] User ${data.userId} connected`, { ... }) + logger.info({ evt: "USER_CONNECTED", userId: data.userId, ... }, "user connected") - console.error(`[EventRouter] Error handling ${event.type}:`, error); + logger.error({ evt: "EventRouterError", type: event.type, err: error }, "handler error")Also applies to: 47-58, 61-72, 75-89, 92-105, 108-114, 135-145
apps/web/src/data/use-conversation-messages.tsx (1)
33-33: Use DEFAULT_PAGE_LIMIT for consistency with queryFnAvoid hardcoded 50 so keys and fetch use the same default.
- limit: options?.limit ?? 50, + limit: options?.limit ?? DEFAULT_PAGE_LIMIT,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/api/src/rest/routers/conversation.ts(2 hunks)apps/api/src/rest/routers/messages.ts(2 hunks)apps/api/src/trpc/routers/conversation.ts(1 hunks)apps/api/src/ws/router.test.ts(1 hunks)apps/api/src/ws/router.ts(2 hunks)apps/web/src/components/conversation/index.tsx(2 hunks)apps/web/src/data/use-conversation-messages.tsx(2 hunks)packages/types/src/realtime-events.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/components/conversation/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/api/src/rest/routers/conversation.ts
- apps/api/src/trpc/routers/conversation.ts
- apps/api/src/ws/router.test.ts
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/global-rules.mdc)
**/*.{ts,tsx}: Manage shared component state via React Context
Implement full TypeScript support for all public APIs
Provide proper type definitions for all props and refs
Use appropriate generic constraints where necessary
Export type definitions for the public API
Avoid TypeScript enums; prefer const objects with 'as const' or Object.freeze
Provide clear, descriptive runtime errors for missing context/props and misuse
Expose dedicated hooks for context access that throw helpful errors when context is missing
Add explicit error messages for: outside-context usage, missing required props, invalid prop combinations, and validation failures
Document each component with JSDoc: purpose, props (required/optional), context requirements, example usage, pitfalls
State management: use React Context for global state, perform correct state updates, and handle side effects appropriately
Validation: implement runtime checks and validate prop combinations with clear errors
Create context with createContext<ContextType|null>(null) and a useContext hook that throws a clear error when missing
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't have unused function parameters
Don't have unused private class members
Don't return a value from a function that has a 'void' return type
Don't use the TypeScript directive @ts-ignore
Ensure switch-case statements are exhaustive (especially with unions)
Don't use TypeScript enums
Don't add type annotations to variables, parameters, and class properties initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types (where discouraged by policy)
Use as const instead of literal types and type annotations
Use either T[] or Array consistently
Use consistent accessibili...
Files:
apps/api/src/ws/router.tspackages/types/src/realtime-events.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/rest/routers/messages.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't use theaccessKeyattribute on any HTML element
Use correct ISO language/country codes for the lang attribute
Don't hardcode sensitive data like API keys and tokens
Use === and !==
Files:
apps/api/src/ws/router.tspackages/types/src/realtime-events.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/rest/routers/messages.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of statements instead of Array.forEach
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules that don't change anything
Don't use unnecessary escape sequences in regular expression literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports, exports, and destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when there are no escape sequences
Don't use useless case statements in switch statements
Avoid ternary operators when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use void operators
Use Date.now() to get milliseconds since the Unix Epoch
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed property access
Don't use parseInt() or Number.parseInt() when numeric literals suffice (binary/octal/hex)
Use concise optional chaining instead of chained logical expressions
Prefer regular expression literals over the RegExp constructor when possible
Don't use non–base-10 or underscore-separated number literal object member names
Remove redundant terms from logical expressions
Prefer while loops over for loops when no initializer/update is needed
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp values when result is constant
Don't use empty character classes in regular expression literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions and vars that are acce...
Files:
apps/api/src/ws/router.tspackages/types/src/realtime-events.tsapps/api/src/rest/routers/messages.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts,tsx}: Don't create classes that only have static members
Don't return a value from a constructor
Don't use super() incorrectly inside classes; ensure super() is called in derived constructors
Ensure super() is called exactly once on every code path in a class constructor before this is accessed if the class has a superclass
Place getters and setters for the same property next to each other in class and object definitions
Ensure JSDoc comment lines start with a single asterisk, except for the first one
Don't use duplicate class members
Files:
apps/api/src/ws/router.tspackages/types/src/realtime-events.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/rest/routers/messages.ts
**/*.{js,tsx,ts,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Use arrow functions instead of function expressions
Files:
apps/api/src/ws/router.tspackages/types/src/realtime-events.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/rest/routers/messages.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts,jsx,tsx}: Prevent duplicate polyfills from Polyfill.io
Don't use irregular whitespace characters
Files:
apps/api/src/ws/router.tspackages/types/src/realtime-events.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/rest/routers/messages.ts
**/*.{js,ts,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't redeclare variables, functions, classes, and types in the same scope
Files:
apps/api/src/ws/router.tspackages/types/src/realtime-events.tsapps/web/src/data/use-conversation-messages.tsxapps/api/src/rest/routers/messages.ts
packages/types/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/types/src/**/*.ts: Keep shared TypeScript types and Zod schemas in packages/types/src/ and update them alongside schema changes
Ensure Zod schemas mirror any API/database shape changes for end-to-end type safety
Files:
packages/types/src/realtime-events.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/global-rules.mdc)
**/*.tsx: Use the Compound Components pattern with namespaced exports (e.g., FormWizard.Root, FormWizard.Step) so pieces compose under a single parent
Ensure accessibility: keyboard navigation support
Ensure accessibility: include appropriate ARIA attributes
Ensure accessibility: use semantic markup for screen readers
Maintain proper focus management in interactive components
Structure components with forwardRef and typed props/refs as shown (ComponentRef, HTMLAttributes, ReactNode)
Root component acts as main context provider, manages global state, and handles initialization
Child components must be used within Root, read context via hooks, support asChild, and follow consistent props
Props ordering: list required props first; optional props have clear defaults; support common HTML attributes; expose className
Support composition with an asChild prop using Radix UI Slot to allow element replacement
Styling: support className, use data-* attributes for state, and allow style customization
Event handling: support standard DOM events, add custom events when needed, and manage propagation correctly
Base component pattern: forwardRef, choose Slot or intrinsic element via asChild, pass ref/className and spread props
Files:
apps/web/src/data/use-conversation-messages.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx}: Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like or
Only use the scope prop on elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Ensure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include the words "image", "picture", or "photo" in img alt prop
Don't use an explicit role that's the same as the element's implicit role
Static elements with click handlers must have a valid role attribute
Always include a <title> element within SVG elements
Provide meaningful alt text for elements that require alt text
Ensure anchors have content accessible to screen readers
Assign tabIndex to non-interactive elements only when using aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Ensure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Elements with interactive roles and handlers must be focusable
Heading elements must have content accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Prefer semantic elements instead of role attributes in JSX
Ensure all anchors are valid and navigable
Ensure all aria-* properties are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid ARIA state and property values
Use valid values for the autocomplete attribute on input elements
Don't use unnecessary fragmen...Files:
apps/web/src/data/use-conversation-messages.tsx**/*.{jsx,tsx,html}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{jsx,tsx,html}: Ensure the preconnect attribute is used when using Google Fonts
Don't use target="_blank" without rel="noopener"
Use a recommended display strategy with Google FontsFiles:
apps/web/src/data/use-conversation-messages.tsx🧬 Code graph analysis (3)
apps/api/src/ws/router.ts (2)
packages/types/src/realtime-events.ts (3)
RealtimeEventType(36-36)RealtimeEventData(44-46)RealtimeEvent(38-42)apps/api/src/lib/pubsub.ts (2)
emitToDashboard(584-590)emitToAll(592-599)apps/web/src/data/use-conversation-messages.tsx (1)
apps/web/src/data/conversation-message-cache.ts (1)
createConversationMessagesInfiniteQueryKey(112-116)apps/api/src/rest/routers/messages.ts (4)
packages/types/src/api/message.ts (4)
getMessagesRequestSchema(5-9)getMessagesResponseSchema(14-18)sendMessageRequestSchema(23-35)sendMessageResponseSchema(40-42)apps/api/src/utils/validate.ts (3)
safelyExtractRequestQuery(120-162)validateResponse(6-20)safelyExtractRequestData(56-107)apps/api/src/db/queries/message.ts (1)
getMessages(49-96)apps/api/src/utils/message.ts (1)
createMessage(68-112)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
apps/api/src/rest/routers/messages.ts (1)
204-223: LGTM: route delegates to createMessage and validates responseCreation flows through a single utility and response is schema-validated. Good.
apps/api/src/ws/router.ts (1)
116-121: ```shell
#!/bin/bash
set -euo pipefailecho "PWD: $(pwd)"
echoecho "---- apps/api/src/ws/router.ts (first 240 lines) ----"
if [ -f apps/api/src/ws/router.ts ]; then
cat -n apps/api/src/ws/router.ts | sed -n '1,240p'
else
echo "MISSING: apps/api/src/ws/router.ts"
fiecho
echo "---- apps/api/src/lib/pubsub.ts (lines 540-720) ----"
if [ -f apps/api/src/lib/pubsub.ts ]; then
rg -n "emitToAll" apps/api/src/lib/pubsub.ts || true
cat -n apps/api/src/lib/pubsub.ts | sed -n '540,720p'
else
echo "MISSING: apps/api/src/lib/pubsub.ts"
fiecho
echo "---- apps/api/src/utils/message.ts (first 240 lines) ----"
if [ -f apps/api/src/utils/message.ts ]; then
cat -n apps/api/src/utils/message.ts | sed -n '1,240p'
else
echo "MISSING: apps/api/src/utils/message.ts"
fiecho
echo "---- Search for routeEvent definition ----"
rg -n --hidden "function\s+routeEvent|export\s+function\s+routeEvent|const\s+routeEvent\s*[:=]" || trueecho
echo "---- Search for any code that forwards Pub/Sub messages into routeEvent or websocket handlers ----"
rg -n --hidden "routeEvent\(|subscribe\(|onMessage\(|handleMessage\(|deliver\(|fromPubSub|pubsub" apps || trueecho
echo "---- Search for 'source' field usage in event objects ----"
rg -n --hidden ""source"\s*[:=]|source:\s*" || true</blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| if (!visitorId) { | ||
| return c.json( | ||
| validateResponse( | ||
| { error: "Visitor ID is required" }, | ||
| z.object({ error: z.string() }) | ||
| ) | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
Return proper 400 for missing Visitor ID (don’t respond 200 with error body)
Current code returns 200 with an error payload. Throw an HTTPException 400 for consistent error handling and OpenAPI semantics.
- if (!visitorId) {
- return c.json(
- validateResponse(
- { error: "Visitor ID is required" },
- z.object({ error: z.string() })
- )
- );
- }
+ if (!visitorId) {
+ throw new HTTPException(400, { message: "Visitor ID is required" });
+ }And add import:
import { OpenAPIHono, z } from "@hono/zod-openapi";
+import { HTTPException } from "hono/http-exception";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!visitorId) { | |
| return c.json( | |
| validateResponse( | |
| { error: "Visitor ID is required" }, | |
| z.object({ error: z.string() }) | |
| ) | |
| ); | |
| } | |
| import { OpenAPIHono, z } from "@hono/zod-openapi"; | |
| import { HTTPException } from "hono/http-exception"; | |
| if (!visitorId) { | |
| throw new HTTPException(400, { message: "Visitor ID is required" }); | |
| } |
🤖 Prompt for AI Agents
In apps/api/src/rest/routers/messages.ts around lines 195 to 203, the handler
currently returns a 200 JSON payload containing an error object when visitorId
is missing; change this to throw an HTTPException with status 400 instead.
Replace the c.json(validateResponse(...)) return with: throw new
HTTPException(400, validateResponse({ error: "Visitor ID is required" },
z.object({ error: z.string() }))); and add the necessary import for
HTTPException at the top of the file (import { HTTPException } from the
framework/bootstrap module used in this codebase). Ensure no other code path
expects the old 200 response.
| @@ -1,63 +1,64 @@ | |||
| import { z } from "zod"; | |||
| import { MessageType, MessageVisibility } from "./enums"; | |||
There was a problem hiding this comment.
Add MESSAGE_CREATED schema and remove unused imports
The WS/router and utils emit MESSAGE_CREATED, but this file doesn’t define it. Also, MessageType/MessageVisibility are imported but unused.
Apply:
-import { MessageType, MessageVisibility } from "./enums";
+import { messageSchema } from "./api/message"; export const RealtimeEvents = {
USER_CONNECTED: z.object({
userId: z.string(),
connectionId: z.string(),
timestamp: z.number(),
}),
USER_DISCONNECTED: z.object({
userId: z.string(),
connectionId: z.string(),
timestamp: z.number(),
}),
VISITOR_CONNECTED: z.object({
visitorId: z.string(),
connectionId: z.string(),
timestamp: z.number(),
}),
VISITOR_DISCONNECTED: z.object({
visitorId: z.string(),
connectionId: z.string(),
timestamp: z.number(),
}),
USER_PRESENCE_UPDATE: z.object({
userId: z.string(),
status: z.enum(["online", "away", "offline"]),
lastSeen: z.number(),
}),
+ MESSAGE_CREATED: z.object({
+ conversationId: z.string(),
+ websiteId: z.string(),
+ organizationId: z.string(),
+ // Align with serializeMessageForRealtime; adjust if that serializer changes.
+ message: messageSchema,
+ }),
} as const;If createdAt is serialized (e.g., to ISO string/number), refine messageSchema here or introduce a dedicated messageRealtimeSchema to match the wire format.
Also applies to: 9-34
🤖 Prompt for AI Agents
In packages/types/src/realtime-events.ts around lines 2 and 9-34, the file
currently imports MessageType and MessageVisibility but doesn't use them and
lacks a MESSAGE_CREATED event schema — add a MESSAGE_CREATED schema (or
messageRealtimeSchema) describing the wire format emitted by WS/router and
utils, remove the unused MessageType/MessageVisibility imports, and ensure the
schema reflects how createdAt is serialized on the wire (if createdAt is sent as
ISO string or epoch number, adjust the field type accordingly or create a
dedicated messageRealtimeSchema matching the serialized format).
| parentMessageId: z.string().nullable(), | ||
| aiAgentId: z.string().nullable(), | ||
| modelUsed: z.string().nullable(), | ||
| visibility: z.string(), |
There was a problem hiding this comment.
Bug: Event Schema Validation Flaw
The MESSAGE_CREATED event schema defines type and visibility as z.string(). This allows invalid values to pass validation, creating a type safety issue. The imported MessageType and MessageVisibility enums could be used to properly constrain these fields, consistent with other message schemas.
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)
apps/api/src/rest/routers/conversation.ts (1)
404-416: Remove debug logging of headers (secrets leak) and console usage.This logs Authorization/Public keys to stdout. Remove or guard behind a debug flag and redact values. Also aligns with the “Don’t use console” guideline.
- // Debug: Check what's in the context - console.log("[DEBUG] GET /conversations/{id} - Context check:", { - hasDb: !!c.get("db"), - hasWebsite: !!c.get("website"), - hasOrganization: !!c.get("organization"), - hasApiKey: !!c.get("apiKey"), - headers: { - authorization: c.req.header("Authorization"), - publicKey: c.req.header("X-Public-Key"), - visitorId: c.req.header("X-Visitor-Id"), - }, - }); + // Consider using a structured logger at debug level without sensitive headers if needed.
♻️ Duplicate comments (3)
apps/api/src/rest/routers/messages.ts (1)
195-202: Return 400 via HTTPException for missing Visitor ID (don’t 200-with-error).Aligns with REST semantics and prior guidance.
- if (!visitorId) { - return c.json( - validateResponse( - { error: "Visitor ID is required" }, - z.object({ error: z.string() }) - ) - ); - } + if (!visitorId) { + throw new HTTPException(400, { message: "Visitor ID is required" }); + }Add the import:
-import { OpenAPIHono, z } from "@hono/zod-openapi"; +import { OpenAPIHono, z } from "@hono/zod-openapi"; +import { HTTPException } from "hono/http-exception";apps/api/src/rest/routers/conversation.ts (1)
143-163: Guarding defaultMessages and per-message creation look good; consider atomicity.Nice normalization via defaults = body.defaultMessages ?? []. However, concurrent per-message inserts can leave partial state if one fails; wrap in a DB transaction and emit events after commit.
Example (pseudo-code):
await db.transaction(async (trx) => { const results = []; for (const msg of defaults) { results.push( await createMessage({ db: trx, organizationId, websiteId, conversationId, message: {/*...*/} }) ); } return results; }); // after commit: emit/broadcast if neededapps/api/src/ws/router.test.ts (1)
5-12: Export emitToAll from the pubsub mock (tests currently fail).The mock module doesn’t export emitToAll, so the router uses the real function and your expectations see 0 calls. Export it alongside emitToDashboard.
Apply this diff:
-const emitToAll = mock(async () => {}); +const emitToAll = mock(async () => {}); mock.module("@api/lib/pubsub", () => ({ - emitToDashboard, + emitToDashboard, + emitToAll, pubsub: { publish: async () => {}, }, }));
🧹 Nitpick comments (3)
apps/api/src/ws/router.test.ts (1)
17-19: Reset all mocks between tests.Add emitToAll.mockReset() in beforeEach to avoid cross-test leakage.
beforeEach(() => { emitToDashboard.mockReset(); + emitToAll.mockReset(); });apps/api/src/rest/routers/messages.ts (1)
204-218: Creation flow LGTM; defaults delegated to createMessage.Mapping to createMessage is correct; undefined values fall back to TEXT/PUBLIC inside createMessage. Consider accepting optional parentMessageId/modelUsed when available so API parity matches the Message shape end-to-end.
apps/api/src/rest/routers/conversation.ts (1)
76-101: Gitleaks false positive: obfuscate token examples in OpenAPI.Examples currently match Stripe key patterns and trip scanners. Use obviously fake placeholders that don’t match the regex.
Example replacements:
- Authorization example: "Bearer sk_test_not_a_real_token_example"
- X-Public-Key example: "pk_test_not_a_real_public_key"
Also applies to: 236-251, 371-389
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/api/src/rest/routers/conversation.ts(2 hunks)apps/api/src/rest/routers/messages.ts(2 hunks)apps/api/src/trpc/routers/conversation.ts(2 hunks)apps/api/src/utils/message.ts(1 hunks)apps/api/src/ws/router.test.ts(1 hunks)apps/api/src/ws/router.ts(2 hunks)apps/web/src/app/(lander-docs)/components/docs/mdx-components.tsx(1 hunks)apps/web/src/components/conversation/index.tsx(1 hunks)apps/web/src/data/conversation-message-cache.ts(1 hunks)apps/web/src/data/use-conversation-messages.tsx(2 hunks)packages/types/src/realtime-events.ts(1 hunks)packages/types/src/schemas.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/app/(lander-docs)/components/docs/mdx-components.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/src/data/conversation-message-cache.ts
- apps/web/src/components/conversation/index.tsx
- apps/api/src/utils/message.ts
- apps/api/src/ws/router.ts
- apps/web/src/data/use-conversation-messages.tsx
- apps/api/src/trpc/routers/conversation.ts
- packages/types/src/realtime-events.ts
🧰 Additional context used
📓 Path-based instructions (10)
packages/types/src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
packages/types/src/**/*.ts: Keep shared TypeScript types and Zod schemas in packages/types/src/ and update them alongside schema changes
Ensure Zod schemas mirror any API/database shape changes for end-to-end type safety
Files:
packages/types/src/schemas.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/global-rules.mdc)
**/*.{ts,tsx}: Manage shared component state via React Context
Implement full TypeScript support for all public APIs
Provide proper type definitions for all props and refs
Use appropriate generic constraints where necessary
Export type definitions for the public API
Avoid TypeScript enums; prefer const objects with 'as const' or Object.freeze
Provide clear, descriptive runtime errors for missing context/props and misuse
Expose dedicated hooks for context access that throw helpful errors when context is missing
Add explicit error messages for: outside-context usage, missing required props, invalid prop combinations, and validation failures
Document each component with JSDoc: purpose, props (required/optional), context requirements, example usage, pitfalls
State management: use React Context for global state, perform correct state updates, and handle side effects appropriately
Validation: implement runtime checks and validate prop combinations with clear errors
Create context with createContext<ContextType|null>(null) and a useContext hook that throws a clear error when missing
**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use any or unknown as type constraints
Don't have unused function parameters
Don't have unused private class members
Don't return a value from a function that has a 'void' return type
Don't use the TypeScript directive @ts-ignore
Ensure switch-case statements are exhaustive (especially with unions)
Don't use TypeScript enums
Don't add type annotations to variables, parameters, and class properties initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types (where discouraged by policy)
Use as const instead of literal types and type annotations
Use either T[] or Array consistently
Use consistent accessibili...
Files:
packages/types/src/schemas.tsapps/api/src/rest/routers/messages.tsapps/api/src/ws/router.test.tsapps/api/src/rest/routers/conversation.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,jsx,ts,tsx}: Don't use theaccessKeyattribute on any HTML element
Use correct ISO language/country codes for the lang attribute
Don't hardcode sensitive data like API keys and tokens
Use === and !==
Files:
packages/types/src/schemas.tsapps/api/src/rest/routers/messages.tsapps/api/src/ws/router.test.tsapps/api/src/rest/routers/conversation.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts}: Don't use consecutive spaces in regular expression literals
Don't use the arguments object
Don't use the comma operator
Don't use unnecessary boolean casts
Don't use unnecessary callbacks with flatMap
Prefer for...of statements instead of Array.forEach
Don't use this and super in static contexts
Don't use unnecessary catch clauses
Don't use unnecessary constructors
Don't use unnecessary continue statements
Don't export empty modules that don't change anything
Don't use unnecessary escape sequences in regular expression literals
Don't use unnecessary labels
Don't use unnecessary nested block statements
Don't rename imports, exports, and destructured assignments to the same name
Don't use unnecessary string or template literal concatenation
Don't use String.raw in template literals when there are no escape sequences
Don't use useless case statements in switch statements
Avoid ternary operators when simpler alternatives exist
Don't use useless this aliasing
Don't initialize variables to undefined
Don't use void operators
Use Date.now() to get milliseconds since the Unix Epoch
Prefer .flatMap() over map().flat() when possible
Use literal property access instead of computed property access
Don't use parseInt() or Number.parseInt() when numeric literals suffice (binary/octal/hex)
Use concise optional chaining instead of chained logical expressions
Prefer regular expression literals over the RegExp constructor when possible
Don't use non–base-10 or underscore-separated number literal object member names
Remove redundant terms from logical expressions
Prefer while loops over for loops when no initializer/update is needed
Don't reassign const variables
Don't use constant expressions in conditions
Don't use Math.min/Math.max to clamp values when result is constant
Don't use empty character classes in regular expression literals
Don't use empty destructuring patterns
Don't call global object properties as functions
Don't declare functions and vars that are acce...
Files:
packages/types/src/schemas.tsapps/api/src/rest/routers/messages.tsapps/api/src/ws/router.test.tsapps/api/src/rest/routers/conversation.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts,tsx}: Don't create classes that only have static members
Don't return a value from a constructor
Don't use super() incorrectly inside classes; ensure super() is called in derived constructors
Ensure super() is called exactly once on every code path in a class constructor before this is accessed if the class has a superclass
Place getters and setters for the same property next to each other in class and object definitions
Ensure JSDoc comment lines start with a single asterisk, except for the first one
Don't use duplicate class members
Files:
packages/types/src/schemas.tsapps/api/src/rest/routers/messages.tsapps/api/src/ws/router.test.tsapps/api/src/rest/routers/conversation.ts
**/*.{js,tsx,ts,jsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Use arrow functions instead of function expressions
Files:
packages/types/src/schemas.tsapps/api/src/rest/routers/messages.tsapps/api/src/ws/router.test.tsapps/api/src/rest/routers/conversation.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{js,ts,jsx,tsx}: Prevent duplicate polyfills from Polyfill.io
Don't use irregular whitespace characters
Files:
packages/types/src/schemas.tsapps/api/src/rest/routers/messages.tsapps/api/src/ws/router.test.tsapps/api/src/rest/routers/conversation.ts
**/*.{js,ts,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
Don't redeclare variables, functions, classes, and types in the same scope
Files:
packages/types/src/schemas.tsapps/api/src/rest/routers/messages.tsapps/api/src/ws/router.test.tsapps/api/src/rest/routers/conversation.ts
**/*.{test.ts,test.tsx,spec.ts,spec.tsx,test.js,test.jsx,spec.js,spec.jsx}
📄 CodeRabbit inference engine (.cursor/rules/global-rules.mdc)
Include tests: unit tests for components, integration tests for composed components, accessibility tests, and error boundary tests
Files:
apps/api/src/ws/router.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)
**/*.{test,spec}.{js,jsx,ts,tsx}: Don't nest describe() blocks too deeply in test files
Don't use callbacks in asynchronous tests and hooks
Don't have duplicate hooks in describe blocks
Don't use export or module.exports in test files
Don't use focused tests
Ensure the assertion function (e.g., expect) is placed inside an it() function call
Don't use disabled tests
Files:
apps/api/src/ws/router.test.ts
🧬 Code graph analysis (4)
packages/types/src/schemas.ts (1)
packages/react/src/provider.tsx (1)
DefaultMessage(28-32)
apps/api/src/rest/routers/messages.ts (1)
apps/api/src/utils/message.ts (1)
createMessage(70-109)
apps/api/src/ws/router.test.ts (3)
apps/api/src/lib/pubsub.ts (2)
emitToAll(592-599)emitToDashboard(584-590)packages/types/src/realtime-events.ts (1)
RealtimeEvent(60-64)apps/api/src/ws/router.ts (1)
routeEvent(128-146)
apps/api/src/rest/routers/conversation.ts (1)
apps/api/src/utils/message.ts (1)
createMessage(70-109)
🪛 GitHub Check: API Tests
apps/api/src/ws/router.test.ts
[failure] 115-115: error: expect(received).toHaveBeenCalledTimes(expected)
Expected number of calls: 1
Received number of calls: 0
at <anonymous> (/home/runner/work/cossistant/cossistant/apps/api/src/ws/router.test.ts:115:23)
[failure] 69-69: error: expect(received).toHaveBeenCalledTimes(expected)
Expected number of calls: 1
Received number of calls: 0
at <anonymous> (/home/runner/work/cossistant/cossistant/apps/api/src/ws/router.test.ts:69:29)
[failure] 41-41: error: expect(received).toHaveBeenCalledTimes(expected)
Expected number of calls: 1
Received number of calls: 0
at <anonymous> (/home/runner/work/cossistant/cossistant/apps/api/src/ws/router.test.ts:41:29)
🪛 GitHub Actions: PR Tests
apps/api/src/ws/router.test.ts
[error] 1-1: Expected number of calls: 1
🪛 Gitleaks (8.27.2)
apps/api/src/rest/routers/conversation.ts
[high] 160-160: Found a Stripe Access Token, posing a risk to payment processing services and sensitive financial data.
(stripe-access-token)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (5)
packages/types/src/schemas.ts (1)
15-16: Schema extension looks good; confirm storage and event parity.parentMessageId/modelUsed as nullable strings are fine. Please verify:
- DB layer has matching nullable columns and serializers.
- Realtime payload serializers include these fields consistently with the MESSAGE_CREATED event.
apps/api/src/ws/router.test.ts (3)
21-47: Why 0 calls? Double-check handler wiring for USER_PRESENCE_UPDATE.Once the pubsub mock is fixed, this should pass. If it still fails, verify the event handler actually calls emitToDashboard for visitor-originated presence updates.
49-75: VISITOR_CONNECTED test depends on the same mock fix.After exporting emitToAll/emitToDashboard in the mock, this should pass. If not, re-check the handler path.
78-123: Realtime date shape: ensure test payload matches event schema.createdAt/updatedAt are ISO strings here. Confirm RealtimeEvents expect strings (not Date objects). If Dates are expected, switch to new Date() instead of toISOString().
apps/api/src/rest/routers/conversation.ts (1)
7-7: Right import.Switch to createMessage aligns with the new per-message flow.
This branch had an error being deployed
1 participant
Summary
createMessagehelper that emits realtime message events and reuse it from the REST and TRPC conversation APIsMESSAGE_CREATEDnotifications, log them on the websocket router, and broadcast them to connected clientsTesting
https://chatgpt.com/codex/tasks/task_e_68cac2ed3048832b8efce8621e9880cd
Summary by CodeRabbit
New Features
Refactor
Tests