feat: implement chat history and user sign-in features#2250
Open
HugoRCD wants to merge 32 commits into
Open
Conversation
Member
HugoRCD
commented
May 12, 2026
HugoRCD
commented
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR migrates the agent to per-user chats: adds users/chats/messages/votes schema and migration; replaces fingerprint-based agent storage with authenticated DB-backed chats; implements chat CRUD, streaming, branching, message deletion, votes, title and visibility APIs; reworks GitHub OAuth to persist users/roles and handle redirect cookies; adds a public login page, header user menu, dashboard layout and chat pages (with redirects); and refactors client composables and UI (agent panel, chat components, modals, icons, analytics names) and related MCP/admin tools and config. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
server/api/agent/feedback.post.ts (1)
118-137:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd an explicit timeout for the Linear API call.
This outbound request currently has no timeout, so upstream slowness can tie up server work and degrade reliability. The project already uses timeouts for other external APIs (e.g.,
search-github-issues.tswithtimeout: 5000), and Nuxt's $fetch fully supports this option.💡 Proposed fix
const response = await $fetch<{ data: { issueCreate: { success: boolean, issue: { id: string, url: string } } } }>(LINEAR_API, { method: 'POST', + timeout: 8000, headers: { 'Authorization': apiKey, 'Content-Type': 'application/json' },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/agent/feedback.post.ts` around lines 118 - 137, The $fetch call that posts the GraphQL mutation to LINEAR_API (the variable response = await $fetch<...>(LINEAR_API, { method: 'POST', headers: ..., body: ... })) has no timeout and can block; add a timeout option (e.g., timeout: 5000) to the options object passed to $fetch so the Linear API request will fail fast like other external calls (see search-github-issues.ts pattern), ensuring the timeout is included alongside method, headers, and body.server/mcp/tools/admin/list-agent-chats.ts (1)
82-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn the full filtered count instead of the current page length.
total: rows.lengthis capped bylimitand shifted byoffset, so pagination consumers can't tell how many chats actually match the filters. This needs a separate count query over the same filter set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/mcp/tools/admin/list-agent-chats.ts` around lines 82 - 92, The return currently sets total: rows.length which only reflects the current page; replace this by running a separate COUNT(*) query using the same filter/where conditions as the query that produced rows (preserving offset/limit for the paged rows) and set total to that count so consumers get the full filtered count; keep the existing mapping of rows (ChatRow -> converted numeric fields and estimatedCost rounding) and return offset and limit unchanged.
🧹 Nitpick comments (5)
app/composables/useChats.ts (1)
20-21: ⚡ Quick winUse
subWeeksfor clarity and precision in the "Last week" cutoff.Line 20 uses
subMonths(new Date(), 0.25), which relies on undocumented fractional month behavior. While this approximates a week (~7-8 days depending on the month), it's unintuitive and imprecise. UsingsubWeeks(new Date(), 1)explicitly represents exactly 7 days and makes the intent clearer.Proposed fix
-import { isToday, isYesterday, subMonths } from 'date-fns' +import { isToday, isYesterday, subMonths, subWeeks } from 'date-fns' @@ - const oneWeekAgo = subMonths(new Date(), 0.25) + const oneWeekAgo = subWeeks(new Date(), 1) const oneMonthAgo = subMonths(new Date(), 1)Also applies to: 28-29
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/composables/useChats.ts` around lines 20 - 21, Replace the imprecise fractional-month usage for the "last week" cutoff by using date-fns' subWeeks instead of subMonths(…, 0.25): update the oneWeekAgo constant (and any other occurrences around the second occurrence at lines 28-29) to call subWeeks(new Date(), 1), and ensure you import subWeeks from date-fns alongside subMonths (or remove subMonths if no longer used) so the intent is explicit and exactly 7 days.app/pages/chat/[id].vue (1)
4-10: ⚡ Quick winAdd validation for route parameter.
The
params.idcan bestring | string[]in Nuxt's type system. While the route should always provide a single value, adding a guard improves type safety and prevents potential runtime issues if the routing configuration changes.🛡️ Proposed improvement
middleware: [ function () { const id = useRoute().params.id + if (!id || Array.isArray(id)) { + return navigateTo('/dashboard/chat', { redirectCode: 301 }) + } return navigateTo(`/dashboard/chat/${id}`, { redirectCode: 301 }) } ]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/pages/chat/`[id].vue around lines 4 - 10, The middleware function uses useRoute().params.id which can be string | string[]; add a guard to ensure id is a single string before calling navigateTo. Inside the anonymous middleware function check that const id = useRoute().params.id is a string (and non-empty), handle the array or undefined case by either selecting the first element or returning a safe fallback/redirect (eg. navigateTo a 404 or dashboard) and only call navigateTo(`/dashboard/chat/${id}`, { redirectCode: 301 }) when id is validated; update the middleware logic (the anonymous function) to perform this validation to avoid runtime errors.app/middleware/admin.ts (1)
1-11: ⚡ Quick winAdd defensive check for undefined user.
If
loggedIn.valueistruebutuser.valueis unexpectedlyundefinedornull(e.g., due to session expiry or race conditions), the current logic redirects to/instead of prompting re-authentication. Consider handling this edge case explicitly.🛡️ Proposed improvement
export default defineNuxtRouteMiddleware((to) => { const { loggedIn, user } = useUserSession() - if (!loggedIn.value) { + if (!loggedIn.value || !user.value) { return navigateTo(`/api/auth/github?redirect=${encodeURIComponent(to.fullPath)}`, { external: true }) } - if (user.value?.role !== 'admin') { + if (user.value.role !== 'admin') { return navigateTo('/') } })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/middleware/admin.ts` around lines 1 - 11, The middleware currently assumes user.value exists when loggedIn.value is true; update defineNuxtRouteMiddleware to defensively check user.value (from useUserSession) and if it's null/undefined treat it as not authenticated by returning navigateTo(`/api/auth/github?redirect=${encodeURIComponent(to.fullPath)}`, { external: true }) so the user is prompted to re-authenticate instead of being redirected to '/'; keep the existing admin-role check (user.value?.role !== 'admin') after this defensive check.server/api/admin/mcp-install.get.ts (1)
2-6: Remove unnecessary optional chaining.Since
requireUserSessionthrows a 401 Unauthorized error when the session is missing, theuserobject is guaranteed to be defined after line 2. The optional chaining operator on line 4 is unnecessary—changeuser?.roletouser.rolefor clarity and precision.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/api/admin/mcp-install.get.ts` around lines 2 - 6, The optional chaining on user is unnecessary because requireUserSession(event) either returns a session with a user or throws; update the conditional in the admin check to use user.role instead of user?.role in the block that checks requireUserSession(event) and throws createError({ statusCode: 403, statusMessage: 'Admin access required' }) to make the intent explicit and avoid redundant syntax.app/pages/dashboard/chat/index.vue (1)
42-64: ⚡ Quick winAdd error handling with user feedback.
The
createChatfunction lacks error handling. If the chat creation request fails, the error silently bubbles up without user feedback.💡 Proposed improvement
async function createChat(prompt: string) { if (loading.value || rateLimitReached.value) return input.value = prompt loading.value = true try { const chat = await $fetch<ChatListItem>('/api/chats', { method: 'POST', body: { id: crypto.randomUUID(), message: { id: crypto.randomUUID(), role: 'user', parts: [{ type: 'text', text: prompt }] } } }) refreshChats() await navigateTo(`/dashboard/chat/${chat?.id}`) + } catch (error) { + // Show error toast/notification to user + console.error('Failed to create chat:', error) } finally { loading.value = false } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/pages/dashboard/chat/index.vue` around lines 42 - 64, The createChat function currently has no error handling so failures from the $fetch call silently bubble up; wrap the await $fetch(...) call in a try/catch (keeping the existing finally that sets loading.value = false) and in the catch block call your app's user-facing notifier (e.g., toast/notification utility) to show a readable error like "Failed to create chat" and include error.message or the caught error; also log the error to console or process logger and avoid navigatingTo when chat creation fails (i.e., only call refreshChats() and navigateTo(...) on success).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/agent/AgentChatSwitcher.vue`:
- Around line 51-52: Update the label fallback logic so existing chats with an
empty title render "Untitled" while creating a brand-new chat still shows "New
chat": change the binding for the label (currently ":label=\"activeChat?.title
|| 'New chat'\"") to check whether activeChat exists and then use
activeChat.title || 'Untitled', e.g. :label="activeChat ? (activeChat.title ||
'Untitled') : 'New chat'"; this keeps the component (AgentChatSwitcher) behavior
correct without changing other props or UI.
In `@app/components/agent/AgentPanelMain.vue`:
- Line 21: AGENT_CHAT_THEME is referenced by the UTheme component but not
defined or imported; fix by either importing AGENT_CHAT_THEME from the module
that exports your app themes (the file that exports theme constants) or by
defining a local constant named AGENT_CHAT_THEME in the AgentPanelMain component
and using that; update the top of AgentPanelMain.vue to add the import statement
or local definition so UTheme(:ui="AGENT_CHAT_THEME") has a valid value.
In `@app/components/chat/ChatMessageActions.vue`:
- Around line 43-50: The branch() action is non-idempotent and lacks error
handling; wrap its logic in a pending guard (e.g., an isBranching reactive flag)
to short-circuit duplicate triggers, set/reset that flag around the request, and
disable the UI trigger while true; also wrap the fetch and follow-up calls (the
$fetch call, refreshChats(), and navigateTo()) in try/catch, show a user-facing
error (toast or emit) on failure, and ensure the flag is cleared in finally so
state is always reset; optionally add an Idempotency-Key header (or reuse a
per-action UUID) when calling /api/chats/${props.chatId}/branch to make retries
safer.
In `@app/components/chat/ChatVisibility.vue`:
- Around line 150-155: The UButton that renders the icon-only copy action
(props: :icon, :color, variant, size, `@click`="copyLink") lacks an accessible
name; update the UButton usage to add a dynamic aria-label that reflects the
copied state (e.g., use the copied reactive to switch between "Copy link" and
"Link copied" or similar) so screen readers get meaningful feedback; ensure the
label updates when the copied boolean changes and apply it alongside existing
props on the UButton element.
In `@app/components/chat/ModalConfirm.vue`:
- Line 20: The confirmation button label is hardcoded to "Delete" in
ModalConfirm.vue; add a new prop (e.g., confirmLabel) to the component with a
sensible default ("Delete") and use that prop for the UButton :label binding
instead of the string literal; update the component props declaration
(ModalConfirm.vue) to include confirmLabel and ensure existing emit('close',
true) behavior remains unchanged so consumers can override the button text when
reusing the modal.
In `@app/pages/dashboard/chat/`[id].vue:
- Around line 73-95: The handler double-submits and maps a cleared vote to false
because it updates votes.value and posts via $fetch, then calls
castVote(message, next ?? false) which triggers a second submit and treats
neutral as false; remove the final castVote(...) call from the vote function so
this handler is the single source of truth (keep the $fetch + votes.value
snapshot/restore logic), and if UI components depend on the composable ensure
they read from votes.value/getVote(message.id) (or alternatively update castVote
to accept null/neutral explicitly before using it elsewhere).
In `@server/api/auth/github.get.ts`:
- Around line 54-63: The redirect handling in the auth flow incorrectly casts
query.redirect to string and allows protocol-relative URLs; update the logic
around getQuery(event) and setCookie usage so you first ensure query.redirect is
a single string (reject arrays), then validate the value explicitly: require
typeof redirect === 'string', disallow values that start with '//'
(protocol-relative), and only accept paths that start with a single '/' (e.g.
redirect.startsWith('/') && !redirect.startsWith('//')); then call
setCookie(event, 'oauth-redirect', redirect, ...) with the validated redirect.
In `@server/api/chats/`[id].post.ts:
- Around line 154-162: The current upsert uses
db.insert(schema.messages).values(...) with onConflictDoUpdate targeting
schema.messages.id which allows a global ID collision to overwrite message
content across chats; change this to insert-only conflict-ignored behavior (use
onConflictDoNothing / ignore) or, preferably, target a chat-scoped unique
constraint (e.g., conflict on composite of chatId and id) so collisions cannot
update contents of another chat; update the call around
db.insert(schema.messages)...onConflictDoUpdate(...) to either remove the update
and use onConflictDoNothing or switch the conflict target to the composite
(chatId, id) to ensure safe, non-destructive handling of duplicate IDs for
lastMessage.
In `@server/api/chats/`[id]/branch.post.ts:
- Around line 25-28: The arrow functions used to iterate chat.messages (e.g., in
the findIndex that defines cutIndex and the other callbacks around lines 40-47)
lack parameter type annotations; fix by giving the iterator parameter an
explicit type (preferably the project's Message type, e.g., m: Message) or use a
derived type (m: typeof chat.messages[number]) if Message isn't in scope, and
update all such callbacks (the findIndex callback and any filter/map callbacks
in the 40-47 region) to use that annotation so TypeScript no longer infers any.
- Line 10: Validate the request body after calling readBody(event) by checking
that messageId exists, is a string, and meets expected constraints (non-empty
and, if applicable, matches the expected format/regex or UUID pattern) before
proceeding; if validation fails, return an appropriate HTTP error response
(e.g., 400 Bad Request) and stop further processing. Update the branch.post
handler to perform this check on the messageId variable returned by readBody and
use the same error-handling path used elsewhere in this file to keep behavior
consistent.
In `@server/api/chats/index.get.ts`:
- Line 1: Import list includes an unused symbol `desc` which causes a lint
failure; update the import statement in the module that currently has "import {
desc, eq, sql } from 'drizzle-orm'" to remove `desc` so it reads only the used
symbols (e.g., `eq, sql`), leaving other logic (functions using `eq` or `sql`)
unchanged.
In `@server/api/chats/index.post.ts`:
- Around line 11-26: The chat creation and first message inserts (db.insert into
schema.chats and schema.messages) must be executed in a single DB transaction to
avoid orphan chats if the message insert fails; wrap both operations in a
transaction (e.g., using db.transaction or your DB client's transaction helper)
so you insert into schema.chats, obtain the returned chat, then insert the
message with chat.id and message.id inside the same transaction, and ensure you
roll back/throw if either insert fails rather than leaving a partial state.
- Around line 6-9: The message validator is too permissive: replace
z.custom<UIMessage>() in the readValidatedBody call with an explicit Zod schema
that enforces the UIMessage shape (e.g., message: z.object({...})). Ensure the
schema requires required fields: id (string, if part-level ids exist), parts
(z.array of objects with required text/content fields), and a role constrained
to the allowed values (e.g., z.enum(['user','assistant','system']) or
equivalent). Update the schema used in the readValidatedBody call (the same
invocation that destructures id and message) so incoming API payloads are
strictly validated before further processing.
In `@server/db/migrations/sqlite/0003_chat_template.sql`:
- Around line 15-28: The chats table definition in 0003_chat_template.sql is
missing a foreign key on user_id; update the CREATE TABLE `chats` statement to
add a FOREIGN KEY(user_id) REFERENCES users(id) ON DELETE CASCADE so chats are
removed when a user is deleted and to match messages/votes behavior; modify the
`chats` table definition (the CREATE TABLE in this file) to include that FK
constraint and ensure any test/migration expectations are updated accordingly.
In `@server/db/schema.ts`:
- Around line 58-62: The votes table currently has independent foreign keys
votes.chatId -> chats.id and votes.messageId -> messages.id which allows
mismatched pairs; fix by enforcing that a vote points to a message in the same
chat: either (A) replace the two independent FK constraints with a composite FK
from (chatId, messageId) -> messages.(chatId, id) (ensure messages has a
composite unique/index on (chatId, id)) or (B) remove votes.chatId and only
store votes.messageId, deriving chatId via JOINs to messages when grouping;
update schema defined in votes (and adjust any code referencing votes.chatId)
accordingly.
---
Outside diff comments:
In `@server/api/agent/feedback.post.ts`:
- Around line 118-137: The $fetch call that posts the GraphQL mutation to
LINEAR_API (the variable response = await $fetch<...>(LINEAR_API, { method:
'POST', headers: ..., body: ... })) has no timeout and can block; add a timeout
option (e.g., timeout: 5000) to the options object passed to $fetch so the
Linear API request will fail fast like other external calls (see
search-github-issues.ts pattern), ensuring the timeout is included alongside
method, headers, and body.
In `@server/mcp/tools/admin/list-agent-chats.ts`:
- Around line 82-92: The return currently sets total: rows.length which only
reflects the current page; replace this by running a separate COUNT(*) query
using the same filter/where conditions as the query that produced rows
(preserving offset/limit for the paged rows) and set total to that count so
consumers get the full filtered count; keep the existing mapping of rows
(ChatRow -> converted numeric fields and estimatedCost rounding) and return
offset and limit unchanged.
---
Nitpick comments:
In `@app/composables/useChats.ts`:
- Around line 20-21: Replace the imprecise fractional-month usage for the "last
week" cutoff by using date-fns' subWeeks instead of subMonths(…, 0.25): update
the oneWeekAgo constant (and any other occurrences around the second occurrence
at lines 28-29) to call subWeeks(new Date(), 1), and ensure you import subWeeks
from date-fns alongside subMonths (or remove subMonths if no longer used) so the
intent is explicit and exactly 7 days.
In `@app/middleware/admin.ts`:
- Around line 1-11: The middleware currently assumes user.value exists when
loggedIn.value is true; update defineNuxtRouteMiddleware to defensively check
user.value (from useUserSession) and if it's null/undefined treat it as not
authenticated by returning
navigateTo(`/api/auth/github?redirect=${encodeURIComponent(to.fullPath)}`, {
external: true }) so the user is prompted to re-authenticate instead of being
redirected to '/'; keep the existing admin-role check (user.value?.role !==
'admin') after this defensive check.
In `@app/pages/chat/`[id].vue:
- Around line 4-10: The middleware function uses useRoute().params.id which can
be string | string[]; add a guard to ensure id is a single string before calling
navigateTo. Inside the anonymous middleware function check that const id =
useRoute().params.id is a string (and non-empty), handle the array or undefined
case by either selecting the first element or returning a safe fallback/redirect
(eg. navigateTo a 404 or dashboard) and only call
navigateTo(`/dashboard/chat/${id}`, { redirectCode: 301 }) when id is validated;
update the middleware logic (the anonymous function) to perform this validation
to avoid runtime errors.
In `@app/pages/dashboard/chat/index.vue`:
- Around line 42-64: The createChat function currently has no error handling so
failures from the $fetch call silently bubble up; wrap the await $fetch(...)
call in a try/catch (keeping the existing finally that sets loading.value =
false) and in the catch block call your app's user-facing notifier (e.g.,
toast/notification utility) to show a readable error like "Failed to create
chat" and include error.message or the caught error; also log the error to
console or process logger and avoid navigatingTo when chat creation fails (i.e.,
only call refreshChats() and navigateTo(...) on success).
In `@server/api/admin/mcp-install.get.ts`:
- Around line 2-6: The optional chaining on user is unnecessary because
requireUserSession(event) either returns a session with a user or throws; update
the conditional in the admin check to use user.role instead of user?.role in the
block that checks requireUserSession(event) and throws createError({ statusCode:
403, statusMessage: 'Admin access required' }) to make the intent explicit and
avoid redundant syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fe4fd5ae-9839-48d7-ac7d-3b4f6392ad8b
⛔ Files ignored due to path filters (2)
app/assets/icons/new-chat.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (72)
.env.exampleapp/app.vueapp/components/AdminAnalytics.vueapp/components/agent/AgentChatSwitcher.vueapp/components/agent/AgentFloatingInput.vueapp/components/agent/AgentPanel.vueapp/components/agent/AgentPanelChat.vueapp/components/agent/AgentPanelMain.vueapp/components/chat/ChatContent.vueapp/components/chat/ChatMessageActions.vueapp/components/chat/ChatPanel.vueapp/components/chat/ChatTitle.vueapp/components/chat/ChatVisibility.vueapp/components/chat/ModalConfirm.vueapp/components/chat/ModalRename.vueapp/components/content-toc/ContentTocBottom.vueapp/components/feedback/FeedbackDatePicker.vueapp/components/header/Header.vueapp/components/header/HeaderUserMenu.vueapp/components/tools/FeedbackCard.vueapp/composables/useAgentChat.tsapp/composables/useChatActions.tsapp/composables/useChats.tsapp/composables/useChatsData.tsapp/composables/useNuxtAgent.tsapp/layouts/admin.vueapp/layouts/chat.vueapp/layouts/dashboard.vueapp/middleware/admin.tsapp/middleware/auth.tsapp/middleware/guest.tsapp/pages/admin/analytics.vueapp/pages/admin/index.vueapp/pages/admin/login.vueapp/pages/chat.vueapp/pages/chat/[id].vueapp/pages/chat/index.vueapp/pages/dashboard/chat/[id].vueapp/pages/dashboard/chat/index.vuenuxt.config.tspackage.jsonserver/api/admin/mcp-install.get.tsserver/api/agent/cleanup.get.tsserver/api/agent/feedback.post.tsserver/api/agent/vote.post.tsserver/api/auth/github.get.tsserver/api/chats/[id].delete.tsserver/api/chats/[id].get.tsserver/api/chats/[id].post.tsserver/api/chats/[id]/branch.post.tsserver/api/chats/[id]/messages.delete.tsserver/api/chats/[id]/title.patch.tsserver/api/chats/[id]/visibility.patch.tsserver/api/chats/[id]/votes.get.tsserver/api/chats/[id]/votes.post.tsserver/api/chats/index.get.tsserver/api/chats/index.post.tsserver/db/migrations/sqlite/0003_chat_template.sqlserver/db/migrations/sqlite/0004_chat_updated_at.sqlserver/db/migrations/sqlite/meta/0003_snapshot.jsonserver/db/migrations/sqlite/meta/_journal.jsonserver/db/schema.tsserver/mcp/tools/admin/agent-usage-stats.tsserver/mcp/tools/admin/get-agent-chat.tsserver/mcp/tools/admin/list-agent-chats.tsserver/mcp/tools/admin/list-agent-votes.tsserver/mcp/tools/modules/list-modules.tsserver/utils/agent-fingerprint.tsshared/types/auth.d.tsshared/types/chat.tsshared/types/db.tsshared/types/index.ts
💤 Files with no reviewable changes (8)
- app/layouts/chat.vue
- app/pages/admin/login.vue
- app/middleware/guest.ts
- server/utils/agent-fingerprint.ts
- app/pages/chat.vue
- app/layouts/admin.vue
- server/api/agent/vote.post.ts
- app/components/chat/ChatPanel.vue
Contributor
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 (3)
layers/nuxi/server/api/chats/index.post.ts (1)
10-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject non-user first messages instead of silently rewriting them.
This schema accepts
assistantandsystem, but Line 36 always persists the message asuser. That means the API currently acknowledges one role and stores another.🛡️ Proposed fix
message: z.object({ id: z.string(), - role: z.enum(['user', 'assistant', 'system']), + role: z.literal('user'), parts: z.array(z.object({ type: z.string() }).loose()) })Also applies to: 33-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/index.post.ts` around lines 10 - 12, The message schema currently allows role: z.enum(['user','assistant','system']) but the endpoint always persists the first message as 'user' (silently rewriting others); change the validation so first messages must be 'user' — either tighten the schema for the first-message path to role: z.literal('user') or add an explicit runtime check after parsing (where the code currently forces role to 'user') that returns a 4xx error if message.role !== 'user'; reference the message z.object(...) and the code path that persists/overwrites the role to locate and update the logic.layers/nuxi/server/api/chats/[id].get.ts (1)
22-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the message ordering deterministic here too.
Sorting only by
createdAtcan reshuffle messages that share the same timestamp. The truncate path already uses(createdAt, id); this read path should match it so the conversation order stays stable.🧭 Proposed fix
messages: { - orderBy: () => asc(schema.messages.createdAt) + orderBy: () => [ + asc(schema.messages.createdAt), + asc(schema.messages.id) + ] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/`[id].get.ts around lines 22 - 24, The messages ordering currently returns orderBy: () => asc(schema.messages.createdAt) which is non-deterministic for identical timestamps; update the messages.orderBy to sort by both createdAt and id (e.g. use an array/tuple of asc(schema.messages.createdAt) then asc(schema.messages.id) or the equivalent API call) so the read path matches the truncate path and conversation order is stable; modify the orderBy implementation in the messages block to include schema.messages.id as the secondary sort key.layers/nuxi/server/api/chats/[id]/branch.post.ts (1)
42-45:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBranching silently falls back to copying the entire chat when
messageIdis unknown.If the client supplies a
messageIdthat isn't part of this chat (typo, race after deletion, malformed client request),findIndexreturns-1and the handler clones the entire conversation instead of surfacing the error. The caller has no way to distinguish "branched from last message" from "your messageId was wrong." Prefer returning a 404 so the UI can react.🛠️ Proposed fix
- const cutIndex = chat.messages.findIndex((m: MessageRow) => m.id === messageId) - const messagesToCopy = cutIndex >= 0 - ? chat.messages.slice(0, cutIndex + 1) - : chat.messages + const cutIndex = chat.messages.findIndex((m: MessageRow) => m.id === messageId) + if (cutIndex < 0) { + throw createError({ message: 'Message not found in chat', status: 404 }) + } + const messagesToCopy = chat.messages.slice(0, cutIndex + 1)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/`[id]/branch.post.ts around lines 42 - 45, The current logic silently treats an unknown messageId as "not found" and copies the whole chat; instead, detect when messageId is provided but not present in chat.messages and return a 404 error so the client can handle it. Specifically, after computing cutIndex (from chat.messages.findIndex) and before building messagesToCopy, if messageId is truthy and cutIndex === -1, early-return a 404 response (or throw a not-found error) rather than falling back to copying the entire chat; otherwise proceed to set messagesToCopy as you already do. Ensure you reference cutIndex, messageId, messagesToCopy and chat.messages in your change.
♻️ Duplicate comments (1)
layers/nuxi/app/components/chat/ChatMessageActions.vue (1)
44-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBranch action still lacks a pending guard (partial follow-up to prior review).
The previous review flagged that this non-idempotent POST needed both error handling and a duplicate-trigger guard. The
try/catchand toast are now in place, but there's still noisBranchingflag short-circuiting concurrent invocations, and the dropdown item isn't disabled while a request is in flight. A user re-selecting "Branch in new chat" before the first call resolves will create two cloned chats.🛠️ Suggested guard
+const isBranching = ref(false) + async function branch() { - if (!props.chatId) return + if (!props.chatId || isBranching.value) return + isBranching.value = true try { const { id } = await $fetch<{ id: string }>(`/api/chats/${props.chatId}/branch`, { method: 'POST', body: { messageId: props.message.id } }) await refreshChats() await navigateTo(`/dashboard/chat/${id}`) } catch { toast.add({ description: 'Failed to branch chat', icon: 'i-lucide-alert-circle', color: 'error' }) + } finally { + isBranching.value = false } }Then forward
disabled: isBranching.valueinto the dropdown item for that branch entry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/app/components/chat/ChatMessageActions.vue` around lines 44 - 60, The branch action can be invoked concurrently; add a reactive guard by introducing an isBranching ref (e.g., const isBranching = ref(false)) and at the top of branch() short-circuit when isBranching.value is true and return early; set isBranching.value = true before the fetch and reset it to false in a finally block so both success and error paths clear the guard; keep existing try/catch around the $fetch to preserve toast behavior, and pass isBranching.value into the dropdown item's disabled prop so the "Branch in new chat" entry is disabled while branching is in flight (references: branch(), props.chatId, props.message.id, refreshChats, navigateTo, toast).
🧹 Nitpick comments (2)
layers/nuxi/server/api/chats/[id].delete.ts (1)
19-35: 💤 Low valueOptional: collapse the existence check + delete into a single statement.
The
findFirstis redundant —db.delete(...).returning()already returns the deleted rows (or an empty array on no match), so you can issue one round-trip and 404 when the result is empty.♻️ Proposed refactor
- const chat = await db.query.chats.findFirst({ - where: () => and( - eq(schema.chats.id, id), - eq(schema.chats.userId, ownerId) - ) - }) - - if (!chat) { - throw createError({ message: 'Chat not found', status: 404 }) - } - - return await db.delete(schema.chats) - .where(and( - eq(schema.chats.id, id), - eq(schema.chats.userId, ownerId) - )) - .returning() + const deleted = await db.delete(schema.chats) + .where(and( + eq(schema.chats.id, id), + eq(schema.chats.userId, ownerId) + )) + .returning() + + if (!deleted.length) { + throw createError({ message: 'Chat not found', status: 404 }) + } + + return deleted🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/`[id].delete.ts around lines 19 - 35, Remove the redundant pre-check using db.query.chats.findFirst and instead perform a single db.delete(schema.chats).where(...).returning() call (matching the same predicates: eq(schema.chats.id, id) and eq(schema.chats.userId, ownerId)); then check the returned rows from that delete, throw createError({ message: 'Chat not found', status: 404 }) if the returned array is empty, otherwise return the deleted row(s). Ensure you reference the same symbols used now (db.delete, schema.chats, eq(...), and createError) so the semantics and error behavior remain identical.layers/nuxi/server/api/chats/[id]/votes.post.ts (1)
13-16: 💤 Low valueMinor:
messageIdvalidation is weaker than the sibling endpoint.
branch.post.tsvalidatesmessageId: z.string().min(1); here it'sz.string(), so an empty string passes Zod and falls through to a 404 from the DB lookup. Aligning the constraint avoids round-tripping obviously bad inputs.const { messageId, isUpvoted } = await readValidatedBody(event, z.object({ - messageId: z.string(), + messageId: z.string().min(1), isUpvoted: z.boolean().optional() }).parse)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/`[id]/votes.post.ts around lines 13 - 16, The request body schema in the votes POST handler uses messageId: z.string() which allows empty strings; update the schema passed to readValidatedBody so messageId is validated with z.string().min(1) (keep isUpvoted: z.boolean().optional()) to match the sibling branch POST handler; locate the validation block that calls readValidatedBody with z.object and replace messageId's schema accordingly so empty messageId values are rejected before the DB lookup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/content/NuxiMoodGallery.vue`:
- Line 16: In NuxiMoodGallery.vue the div with class "nuxi-mood-label" currently
contains the mustache expression on the same line ({{ item.label }}), triggering
vue/singleline-html-element-content-newline; fix it by moving the interpolation
to its own line inside the element so the template becomes a multi-line element
(open div, newline with {{ item.label }}, newline and close div), referencing
the div with class "nuxi-mood-label" and the interpolation item.label.
In `@layers/nuxi/app/components/agent/AgentNuxiIcon.vue`:
- Around line 148-151: There are multiple consecutive blank lines in the
component stylesheet around the .nuxi-z-3 rule and the `@keyframes` nuxi-float
declaration; remove the extra blank line(s) so only a single blank line (or
none) separates the .nuxi-z-3 { animation-delay: 2.1s; } rule and the `@keyframes`
nuxi-float block to satisfy `@stylistic/no-multiple-empty-lines`.
In `@layers/nuxi/app/composables/useNuxiIcon.ts`:
- Around line 45-52: The MOOD_VISUALS block is failing style lint rules for
column-alignment spacing; fix by either reformatting the object to use single
spaces (remove aligned multi-spaces) or preserve the table layout by wrapping
the block with ESLint disable/enable comments for `@stylistic/key-spacing` and
`@stylistic/no-multi-spaces`. Locate the MOOD_VISUALS constant (types NuxiMood and
MoodVisual) in useNuxiIcon.ts and apply one of the two options consistently for
the entire object literal so CI lint passes.
---
Outside diff comments:
In `@layers/nuxi/server/api/chats/`[id].get.ts:
- Around line 22-24: The messages ordering currently returns orderBy: () =>
asc(schema.messages.createdAt) which is non-deterministic for identical
timestamps; update the messages.orderBy to sort by both createdAt and id (e.g.
use an array/tuple of asc(schema.messages.createdAt) then
asc(schema.messages.id) or the equivalent API call) so the read path matches the
truncate path and conversation order is stable; modify the orderBy
implementation in the messages block to include schema.messages.id as the
secondary sort key.
In `@layers/nuxi/server/api/chats/`[id]/branch.post.ts:
- Around line 42-45: The current logic silently treats an unknown messageId as
"not found" and copies the whole chat; instead, detect when messageId is
provided but not present in chat.messages and return a 404 error so the client
can handle it. Specifically, after computing cutIndex (from
chat.messages.findIndex) and before building messagesToCopy, if messageId is
truthy and cutIndex === -1, early-return a 404 response (or throw a not-found
error) rather than falling back to copying the entire chat; otherwise proceed to
set messagesToCopy as you already do. Ensure you reference cutIndex, messageId,
messagesToCopy and chat.messages in your change.
In `@layers/nuxi/server/api/chats/index.post.ts`:
- Around line 10-12: The message schema currently allows role:
z.enum(['user','assistant','system']) but the endpoint always persists the first
message as 'user' (silently rewriting others); change the validation so first
messages must be 'user' — either tighten the schema for the first-message path
to role: z.literal('user') or add an explicit runtime check after parsing (where
the code currently forces role to 'user') that returns a 4xx error if
message.role !== 'user'; reference the message z.object(...) and the code path
that persists/overwrites the role to locate and update the logic.
---
Duplicate comments:
In `@layers/nuxi/app/components/chat/ChatMessageActions.vue`:
- Around line 44-60: The branch action can be invoked concurrently; add a
reactive guard by introducing an isBranching ref (e.g., const isBranching =
ref(false)) and at the top of branch() short-circuit when isBranching.value is
true and return early; set isBranching.value = true before the fetch and reset
it to false in a finally block so both success and error paths clear the guard;
keep existing try/catch around the $fetch to preserve toast behavior, and pass
isBranching.value into the dropdown item's disabled prop so the "Branch in new
chat" entry is disabled while branching is in flight (references: branch(),
props.chatId, props.message.id, refreshChats, navigateTo, toast).
---
Nitpick comments:
In `@layers/nuxi/server/api/chats/`[id].delete.ts:
- Around line 19-35: Remove the redundant pre-check using
db.query.chats.findFirst and instead perform a single
db.delete(schema.chats).where(...).returning() call (matching the same
predicates: eq(schema.chats.id, id) and eq(schema.chats.userId, ownerId)); then
check the returned rows from that delete, throw createError({ message: 'Chat not
found', status: 404 }) if the returned array is empty, otherwise return the
deleted row(s). Ensure you reference the same symbols used now (db.delete,
schema.chats, eq(...), and createError) so the semantics and error behavior
remain identical.
In `@layers/nuxi/server/api/chats/`[id]/votes.post.ts:
- Around line 13-16: The request body schema in the votes POST handler uses
messageId: z.string() which allows empty strings; update the schema passed to
readValidatedBody so messageId is validated with z.string().min(1) (keep
isUpvoted: z.boolean().optional()) to match the sibling branch POST handler;
locate the validation block that calls readValidatedBody with z.object and
replace messageId's schema accordingly so empty messageId values are rejected
before the DB lookup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 924fc6b0-2fa0-4bb6-a654-ee31efdb892e
📒 Files selected for processing (16)
app/components/content/NuxiMoodGallery.vueapp/components/content/TryNuxi.vuecontent/blog/45.meet-nuxi.mdlayers/nuxi/app/components/agent/AgentNuxiIcon.vuelayers/nuxi/app/components/chat/ChatMessageActions.vuelayers/nuxi/app/composables/useNuxiIcon.tslayers/nuxi/server/api/chats/[id].delete.tslayers/nuxi/server/api/chats/[id].get.tslayers/nuxi/server/api/chats/[id]/branch.post.tslayers/nuxi/server/api/chats/[id]/messages.delete.tslayers/nuxi/server/api/chats/[id]/title.patch.tslayers/nuxi/server/api/chats/[id]/visibility.patch.tslayers/nuxi/server/api/chats/[id]/votes.get.tslayers/nuxi/server/api/chats/[id]/votes.post.tslayers/nuxi/server/api/chats/index.get.tslayers/nuxi/server/api/chats/index.post.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
layers/nuxi/server/api/chats/[id]/branch.post.ts (1)
57-65:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the original message timestamps when cloning.
GET /api/chats/:idsorts messages bycreatedAt, but this clone path lets the database assign fresh timestamps. In a batched insert, those rows can share the same time and then reorder by random UUID, which can scramble the branched conversation.💡 Minimal fix
await db.insert(schema.messages).values( messagesToCopy.map((m: MessageRow) => ({ id: crypto.randomUUID(), chatId: newChatId, + createdAt: m.createdAt, role: m.role, parts: m.parts as unknown[] })) )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/`[id]/branch.post.ts around lines 57 - 65, The batched insert for cloning messages (messagesToCopy -> db.insert(schema.messages).values(...)) is dropping original createdAt values so the DB assigns fresh identical timestamps and can reorder messages; update the mapped objects in the insert to explicitly preserve each message's original timestamp (use m.createdAt from MessageRow when building the inserted row for chatId newChatId) so the cloned conversation keeps the original ordering instead of relying on DB defaults.
♻️ Duplicate comments (2)
layers/nuxi/server/api/chats/index.post.ts (1)
23-38:⚠️ Potential issue | 🟠 MajorMake chat creation and first-message insert atomic.
If the
schema.messagesinsert fails after Line 23, this leaves an orphan chat row behind. These two writes still need a single transaction boundary.💡 Minimal fix
- const [chat] = await db.insert(schema.chats).values({ - id, - title: null, - userId: ownerId - }).returning() - - if (!chat) { - throw createError({ message: 'Failed to create chat', status: 500, why: 'Insert returned no row.' }) - } - - await db.insert(schema.messages).values({ - id: message.id, - chatId: chat.id, - role: 'user', - parts: message.parts - }) + const chat = await db.transaction(async (tx) => { + const [created] = await tx.insert(schema.chats).values({ + id, + title: null, + userId: ownerId + }).returning() + + if (!created) { + throw createError({ message: 'Failed to create chat', status: 500, why: 'Insert returned no row.' }) + } + + await tx.insert(schema.messages).values({ + id: message.id, + chatId: created.id, + role: 'user', + parts: message.parts + }) + + return created + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/index.post.ts` around lines 23 - 38, Wrap the chat and message inserts in a single database transaction so they commit or rollback together: use the DB client's transaction API (e.g., db.transaction or similar) to run both the schema.chats insert (the existing db.insert(...).returning() call) and the schema.messages insert inside the same transactional callback, use the transaction handle (tx) to call tx.insert(...) for both operations, check the returned chat row and throw to let the transaction rollback if the message insert fails, and rethrow any errors so upstream handling remains unchanged.layers/nuxi/server/api/chats/[id]/branch.post.ts (1)
50-65:⚠️ Potential issue | 🟠 MajorWrap branch creation and message cloning in one transaction.
If the bulk insert fails after Line 50, the new private chat is already committed and you end up with a partial branch. This endpoint needs the same atomic write boundary as chat creation.
💡 Minimal fix
- await db.insert(schema.chats).values({ - id: newChatId, - userId: chat.userId, - title: chat.title ? `Branch of ${chat.title}` : null, - visibility: 'private' - }) - - if (messagesToCopy.length) { - await db.insert(schema.messages).values( - messagesToCopy.map((m: MessageRow) => ({ - id: crypto.randomUUID(), - chatId: newChatId, - role: m.role, - parts: m.parts as unknown[] - })) - ) - } + await db.transaction(async (tx) => { + await tx.insert(schema.chats).values({ + id: newChatId, + userId: chat.userId, + title: chat.title ? `Branch of ${chat.title}` : null, + visibility: 'private' + }) + + if (messagesToCopy.length) { + await tx.insert(schema.messages).values( + messagesToCopy.map((m: MessageRow) => ({ + id: crypto.randomUUID(), + chatId: newChatId, + role: m.role, + parts: m.parts as unknown[] + })) + ) + } + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@layers/nuxi/server/api/chats/`[id]/branch.post.ts around lines 50 - 65, The creation of the new chat (db.insert(schema.chats).values(...)) and the subsequent bulk insert of messages (db.insert(schema.messages).values(...)) must be executed inside a single transaction so they commit or rollback together; refactor the logic to call the DB transaction API (e.g., db.transaction or equivalent) and move both inserts—including the messagesToCopy mapping that generates new message ids—into that transaction block so any failure in the messages insert rolls back the chat insert, and ensure errors from the transaction are propagated/handled by the handler.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@layers/nuxi/server/api/chats/`[id]/branch.post.ts:
- Around line 57-65: The batched insert for cloning messages (messagesToCopy ->
db.insert(schema.messages).values(...)) is dropping original createdAt values so
the DB assigns fresh identical timestamps and can reorder messages; update the
mapped objects in the insert to explicitly preserve each message's original
timestamp (use m.createdAt from MessageRow when building the inserted row for
chatId newChatId) so the cloned conversation keeps the original ordering instead
of relying on DB defaults.
---
Duplicate comments:
In `@layers/nuxi/server/api/chats/`[id]/branch.post.ts:
- Around line 50-65: The creation of the new chat
(db.insert(schema.chats).values(...)) and the subsequent bulk insert of messages
(db.insert(schema.messages).values(...)) must be executed inside a single
transaction so they commit or rollback together; refactor the logic to call the
DB transaction API (e.g., db.transaction or equivalent) and move both
inserts—including the messagesToCopy mapping that generates new message ids—into
that transaction block so any failure in the messages insert rolls back the chat
insert, and ensure errors from the transaction are propagated/handled by the
handler.
In `@layers/nuxi/server/api/chats/index.post.ts`:
- Around line 23-38: Wrap the chat and message inserts in a single database
transaction so they commit or rollback together: use the DB client's transaction
API (e.g., db.transaction or similar) to run both the schema.chats insert (the
existing db.insert(...).returning() call) and the schema.messages insert inside
the same transactional callback, use the transaction handle (tx) to call
tx.insert(...) for both operations, check the returned chat row and throw to let
the transaction rollback if the message insert fails, and rethrow any errors so
upstream handling remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eca3787c-9e4c-4568-8ddc-f88e485c1c10
📒 Files selected for processing (9)
app/components/content/NuxiMoodGallery.vuelayers/nuxi/app/components/agent/AgentNuxiIcon.vuelayers/nuxi/app/components/chat/ChatMessageActions.vuelayers/nuxi/app/composables/useNuxiIcon.tslayers/nuxi/server/api/chats/[id].delete.tslayers/nuxi/server/api/chats/[id].get.tslayers/nuxi/server/api/chats/[id]/branch.post.tslayers/nuxi/server/api/chats/[id]/votes.post.tslayers/nuxi/server/api/chats/index.post.ts
💤 Files with no reviewable changes (1)
- layers/nuxi/app/components/agent/AgentNuxiIcon.vue
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.