feat: implement chat history and user sign-in features#2250
Draft
HugoRCD wants to merge 12 commits into
Draft
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
📝 WalkthroughWalkthroughThis pull request refactors the authentication and chat persistence system. It replaces GitHub-login-restricted admin-only access with open GitHub OAuth for all users, introducing role-based authorization ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes DetailsThe scope involves 50+ modified and created files spanning multiple interconnected domains: authentication middleware and OAuth handling, 15+ new API endpoints with varied logic patterns, database schema migrations and Drizzle ORM updates, substantial Vue component reorganization (10+ components added/refactored), composable refactoring for chat state and actions, new page layouts and route structures, and type contract updates across the codebase. While individual pieces follow standard patterns (CRUD endpoints, OAuth flows, Vue components), the breadth of changes and tight coupling between systems (user provisioning → chat ownership → vote tracking) require careful verification of ownership checks, data access patterns, and state synchronization across composables and components. ✨ Finishing Touches📝 Generate docstrings
🧪 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
Comment on lines
+11
to
+26
| const [chat] = await db.insert(schema.chats).values({ | ||
| id, | ||
| title: null, | ||
| userId: session.user?.id || session.id | ||
| }).returning() | ||
|
|
||
| if (!chat) { | ||
| throw createError({ statusCode: 500, statusMessage: 'Failed to create chat' }) | ||
| } | ||
|
|
||
| await db.insert(schema.messages).values({ | ||
| id: message.id, | ||
| chatId: chat.id, | ||
| role: 'user', | ||
| parts: message.parts | ||
| }) |
Contributor
There was a problem hiding this comment.
Create chat + first message should be atomic.
Lines 11–26 can leave orphan chats when message insert fails. Wrap both inserts in one DB transaction.
Suggested fix
- const [chat] = await db.insert(schema.chats).values({
- id,
- title: null,
- userId: session.user?.id || session.id
- }).returning()
-
- if (!chat) {
- throw createError({ statusCode: 500, statusMessage: 'Failed to create chat' })
- }
-
- 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: session.user?.id || session.id
+ }).returning()
+
+ if (!created) {
+ throw createError({ statusCode: 500, statusMessage: 'Failed to create chat' })
+ }
+
+ await tx.insert(schema.messages).values({
+ id: message.id,
+ chatId: created.id,
+ role: 'user',
+ parts: message.parts
+ })
+
+ return created
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const [chat] = await db.insert(schema.chats).values({ | |
| id, | |
| title: null, | |
| userId: session.user?.id || session.id | |
| }).returning() | |
| if (!chat) { | |
| throw createError({ statusCode: 500, statusMessage: 'Failed to create chat' }) | |
| } | |
| 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: session.user?.id || session.id | |
| }).returning() | |
| if (!created) { | |
| throw createError({ statusCode: 500, statusMessage: 'Failed to create chat' }) | |
| } | |
| 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 `@server/api/chats/index.post.ts` around lines 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.
Comment on lines
+15
to
+28
| CREATE TABLE `chats` ( | ||
| `id` text PRIMARY KEY NOT NULL, | ||
| `title` text, | ||
| `user_id` text NOT NULL, | ||
| `visibility` text DEFAULT 'private' NOT NULL, | ||
| `model` text, | ||
| `provider` text, | ||
| `input_tokens` integer DEFAULT 0 NOT NULL, | ||
| `output_tokens` integer DEFAULT 0 NOT NULL, | ||
| `estimated_cost` real DEFAULT 0 NOT NULL, | ||
| `duration_ms` integer DEFAULT 0 NOT NULL, | ||
| `request_count` integer DEFAULT 0 NOT NULL, | ||
| `created_at` integer NOT NULL | ||
| );--> statement-breakpoint |
Contributor
There was a problem hiding this comment.
Add foreign key constraint for chats.user_id.
The chats table has a user_id column but no foreign key constraint to users(id). This can result in orphaned chats if users are deleted, creating data integrity issues. The messages and votes tables properly define foreign key constraints with ON DELETE cascade, but chats is missing this protection.
🔒 Proposed fix to add FK constraint
CREATE TABLE `chats` (
`id` text PRIMARY KEY NOT NULL,
`title` text,
`user_id` text NOT NULL,
`visibility` text DEFAULT 'private' NOT NULL,
`model` text,
`provider` text,
`input_tokens` integer DEFAULT 0 NOT NULL,
`output_tokens` integer DEFAULT 0 NOT NULL,
`estimated_cost` real DEFAULT 0 NOT NULL,
`duration_ms` integer DEFAULT 0 NOT NULL,
`request_count` integer DEFAULT 0 NOT NULL,
- `created_at` integer NOT NULL
+ `created_at` integer NOT NULL,
+ FOREIGN KEY (`user_id`) REFERENCES `users`(`id`) ON UPDATE no action ON DELETE cascade
);--> statement-breakpoint🤖 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/db/migrations/sqlite/0003_chat_template.sql` around lines 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.
Comment on lines
+58
to
+62
| export const votes = sqliteTable('votes', { | ||
| chatId: text('chat_id').notNull().references(() => chats.id, { onDelete: 'cascade' }), | ||
| messageId: text('message_id').notNull().references(() => messages.id, { onDelete: 'cascade' }), | ||
| isUpvoted: integer('is_upvoted', { mode: 'boolean' }).notNull() | ||
| }, table => [primaryKey({ columns: [table.chatId, table.messageId] })]) |
Contributor
There was a problem hiding this comment.
Enforce that each vote points to a message from the same chat.
These foreign keys validate chatId and messageId independently, so SQLite will still accept a vote row that combines one chat with a message from another chat. The admin queries added in this PR group by votes.chatId, so a bad row will skew vote counts for the wrong chat. Use a composite constraint here instead, or store only messageId and derive the chat through messages.
🤖 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/db/schema.ts` around lines 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.
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.