feat: implement @mention feature for calendar notes#500
feat: implement @mention feature for calendar notes#500ngoiyaeric wants to merge 5 commits intomainfrom
Conversation
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
WalkthroughThis PR implements Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Client)
participant UI as Calendar UI
participant Search as User Search<br/>(handleNoteContentChange)
participant DB as Database
participant Suggestions as Mention<br/>Dropdown
User->>UI: Type @ in textarea
UI->>Search: Detect `@mention` + extract query
Search->>DB: searchUsers(query)
DB-->>Search: Return matching users
Search->>Suggestions: Show dropdown with matches
User->>Suggestions: Click user in dropdown
Suggestions->>UI: handleSelectUser(user)
UI->>UI: Insert `@mention` into content
UI->>User: Display updated textarea
sequenceDiagram
participant User as User (Client)
participant UI as Calendar UI
participant Server as Server<br/>(saveNote)
participant Mapbox as Mapbox API
participant DB as Database
participant Realtime as Realtime<br/>Subscription
User->>UI: Save note with location tag
UI->>Server: saveNote(content, taggedLocation)
Server->>Server: extractAndValidateMentions()
Server->>Server: extractAndGeocodeLocation()
Server->>Mapbox: Geocode location name
Mapbox-->>Server: Return coordinates
Server->>DB: Create/update calendar_note<br/>with userTags, locationTags
DB-->>Server: Note persisted
Server->>Realtime: Trigger calendar_notes_changes
Realtime->>UI: Notify subscribers
UI->>UI: Refresh notes + derive<br/>noteMarkers from coordinates
UI->>User: Display updated map<br/>with markers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The current implementation has scaling and correctness problems: the UI triggers searchUsers() on every keystroke without debouncing and can display stale results, while server-side mention validation does a full users table scan. Mention parsing/matching is also too restrictive and ambiguous (@\w+ + startsWith) and can silently tag the wrong user. Finally, the migration adds users.email without indexes/constraints, which will hurt prefix-search performance.
Summary of changes
Summary
This PR adds an @mention feature for calendar notes end-to-end:
-
UI (Calendar Notepad)
- Adds an autocomplete dropdown that calls the new server action
searchUsers()while typing@.... - Highlights
@mentionsand#locationtokens when rendering note content. - Shows a
Usersicon whennote.userTagsexists.
- Adds an autocomplete dropdown that calls the new server action
-
Server actions / persistence
- Adds mention parsing/validation in
saveNote()by extracting@prefixtokens and mapping them to user IDs. - Adds
syncUserWithDatabase()andsearchUsers()inlib/actions/users.ts.
- Adds mention parsing/validation in
-
Database
- Adds
emailcolumn tousersvia0002_add_email_to_users.sql. - Updates Drizzle migration snapshots and journal.
- Adds
-
Dependencies
- Bumps
drizzle-ormfrom^0.29.0to^0.45.1.
- Bumps
| const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => { | ||
| const value = e.target.value; | ||
| setNoteContent(value); | ||
|
|
||
| const cursorPosition = e.target.selectionStart; | ||
| const textBeforeCursor = value.substring(0, cursorPosition); | ||
| const words = textBeforeCursor.split(/\s/); | ||
| const lastWord = words[words.length - 1]; | ||
|
|
||
| if (lastWord.startsWith("@")) { | ||
| const query = lastWord.slice(1); | ||
| setMentionQuery(query); | ||
| const results = await searchUsers(query); | ||
| setUserSuggestions(results); | ||
| setShowSuggestions(results.length > 0); | ||
| } else { | ||
| setShowSuggestions(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Mention search triggers a server request on every keystroke (no debouncing / cancellation)
handleNoteContentChange calls searchUsers(query) for every change while the last word starts with @. This can spam server actions and also introduces race conditions where a slower response overwrites a newer query’s suggestions.
Because this runs in a client component, you should debounce input and ignore out-of-order responses (or use AbortController if your server action/fetch supports it).
Suggestion
Debounce the query and guard against stale responses. For example:
const [mentionQuery, setMentionQuery] = useState("")
const latestQueryRef = useRef("")
useEffect(() => {
if (!mentionQuery) {
setShowSuggestions(false)
setUserSuggestions([])
return
}
latestQueryRef.current = mentionQuery
const handle = setTimeout(async () => {
const q = latestQueryRef.current
const results = await searchUsers(q)
// ignore stale responses
if (latestQueryRef.current !== q) return
setUserSuggestions(results)
setShowSuggestions(results.length > 0)
}, 150)
return () => clearTimeout(handle)
}, [mentionQuery])Then in onChange, only compute/set mentionQuery and avoid awaiting network work in the event handler.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const handleSelectUser = (email: string) => { | ||
| const prefix = email.split('@')[0]; | ||
| setNoteContent(prev => prev.replace(/@\w*$/, `@${prefix} `)); | ||
| setShowSuggestions(false); | ||
| }; |
There was a problem hiding this comment.
Selecting a user replaces the last token in the entire textarea, not the token at the cursor
prev.replace(/@\w*$/, ...) only matches an @... at the end of the entire string. If the cursor is in the middle of the textarea, or there is punctuation after the mention, the replacement will fail or replace the wrong text.
This makes the autocomplete feel broken in common editing scenarios (e.g., adding a mention mid-note).
Suggestion
Track the selection start/end and replace relative to the cursor, not the end of the string. One approach:
const [selectionStart, setSelectionStart] = useState<number | null>(null)
const handleNoteContentChange = (e: React.ChangeEvent<HTMLTextAreaElement>) => {
setNoteContent(e.target.value)
setSelectionStart(e.target.selectionStart)
// compute mentionQuery from textBeforeCursor and setMentionQuery(...)
}
const handleSelectUser = (email: string) => {
const prefix = email.split("@")[0]
setNoteContent(prev => {
const pos = selectionStart ?? prev.length
const before = prev.slice(0, pos)
const after = prev.slice(pos)
// replace the last @token immediately before cursor
const updatedBefore = before.replace(/(^|\s)@[^\s@]*$/, `$1@${prefix} `)
return updatedBefore + after
})
setShowSuggestions(false)
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const renderContent = (text: string) => { | ||
| if (!text) return null; | ||
| return text.split(/(@\w+|#location)/g).map((part, i) => { | ||
| if (part.startsWith('@')) return <span key={i} className="text-primary font-medium">{part}</span>; | ||
| if (part === '#location') return <span key={i} className="text-primary font-medium">{part}</span>; | ||
| return part; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Mention tokenization is too restrictive (@\w+ only)
Both the UI highlighter and the server-side extractor use @\w+, which excludes valid/expected characters commonly present in emails/usernames (e.g., ., -). This means mentions like @john.doe won't highlight correctly and won’t be validated.
Also, the UI uses split(/(@\w+|#location)/g) which can produce surprising results if the mention is followed by punctuation.
Suggestion
Broaden the mention pattern to match the same character set you intend to support (and keep it consistent with the server extractor). For example, support GitHub-like handles: [a-zA-Z0-9_.-]+.
const mentionPattern = /@([a-zA-Z0-9_.-]+)/gUpdate both renderContent and the server-side extractAndValidateMentions to use the same pattern.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const [dateOffset, setDateOffset] = useState(0) | ||
| const [taggedLocation, setTaggedLocation] = useState<any | null>(null) | ||
|
|
||
| const [showSuggestions, setShowSuggestions] = useState(false); | ||
| const [userSuggestions, setUserSuggestions] = useState<any[]>([]); | ||
| const [mentionQuery, setMentionQuery] = useState(""); |
There was a problem hiding this comment.
Multiple new any usages were introduced (taggedLocation: any, userSuggestions: any[], handleFlyTo(location: any)). Even if tsc passes, this makes the new feature brittle and harder to evolve safely.
Since the suggestions are coming from the users table, you can type the shape minimally (e.g., { id: string; email: string | null }).
Suggestion
Replace any with small, local types to keep the feature maintainable.
type UserSuggestion = { id: string; email: string | null };
const [userSuggestions, setUserSuggestions] = useState<UserSuggestion[]>([]);For locations, consider a minimal GeoJSON type instead of any.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| async function extractAndValidateMentions(content: string): Promise<string[]> { | ||
| const mentionRegex = /@(\w+)/g; | ||
| const matches = Array.from(content.matchAll(mentionRegex)); | ||
| const potentialEmails = matches.map(match => match[1]); | ||
|
|
||
| if (potentialEmails.length === 0) return []; | ||
|
|
||
| try { | ||
| // Fetch all users to match against prefixes | ||
| // In a larger system, we would use a more optimized search | ||
| const users = await db.select({ id: usersSchema.id, email: usersSchema.email }) | ||
| .from(usersSchema) | ||
| .execute(); | ||
|
|
||
| const validatedIds: string[] = []; | ||
| potentialEmails.forEach(mention => { | ||
| const found = users.find(u => u.email?.toLowerCase().startsWith(mention.toLowerCase())); | ||
| if (found) validatedIds.push(found.id); | ||
| }); | ||
|
|
||
| return Array.from(new Set(validatedIds)); | ||
| } catch (error) { |
There was a problem hiding this comment.
Mention validation does a full table scan of users
extractAndValidateMentions loads all users then does a find() for each mention. This will not scale and will get slower as the user table grows.
Given you already implemented searchUsers() with ilike(email, '${prefix}%'), the validation should query only the prefixes present in the note (or use OR over prefixes).
Suggestion
Avoid fetching the full users table. Instead, query only candidates for the extracted prefixes.
A simple improvement is to issue one query per unique mention prefix (usually small), or build a single OR clause:
const prefixes = Array.from(new Set(potentialMentions.map(m => m.toLowerCase())))
const candidates = await db
.select({ id: usersSchema.id, email: usersSchema.email })
.from(usersSchema)
.where(or(...prefixes.map(p => ilike(usersSchema.email, `${p}%`))))Then map prefix -> best match deterministically.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const validatedIds: string[] = []; | ||
| potentialEmails.forEach(mention => { | ||
| const found = users.find(u => u.email?.toLowerCase().startsWith(mention.toLowerCase())); | ||
| if (found) validatedIds.push(found.id); | ||
| }); |
There was a problem hiding this comment.
Prefix matching is ambiguous and can tag the wrong user
The validation logic treats @jo as a prefix and tags the first user whose email starts with that prefix. With multiple users sharing a prefix (e.g., john@... and joel@...), this can silently tag the wrong person.
At minimum, you need deterministic selection rules, and ideally you should persist an unambiguous identifier (e.g., tag by user id using the autocomplete selection, or require full email/local-part match).
Suggestion
Make tagging unambiguous:
- Prefer storing a stable token in content on selection (e.g.,
@[userId]or@{userId}) while rendering it as@prefixfor display. - If you must keep plain text
@prefix, require an exact match on local-part (email.split('@')[0]) rather thanstartsWith, and ignore partials.
Example exact match approach:
const found = users.find(u => u.email && u.email.split('@')[0].toLowerCase() === mention.toLowerCase())Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| export async function saveNote(noteData: NewCalendarNote | CalendarNote): Promise<CalendarNote | null> { | ||
| // Ensure current user is synced | ||
| await syncUserWithDatabase(); | ||
|
|
||
| const userId = await getCurrentUserIdOnServer(); | ||
|
|
||
| const userTags = await extractAndValidateMentions(noteData.content); | ||
| if (!userId) { | ||
| console.error('saveNote: User not authenticated'); |
There was a problem hiding this comment.
saveNote syncs user before checking authentication
saveNote calls syncUserWithDatabase() before it checks getCurrentUserIdOnServer(). If the user isn't authenticated, you're still doing extra work and potentially additional auth/session lookups.
This is minor but easy to fix and reduces unnecessary calls.
Suggestion
Check userId first, then sync (or have syncUserWithDatabase return the id and use that):
const userId = await getCurrentUserIdOnServer()
if (!userId) return null
await syncUserWithDatabase()Or:
const userId = await syncUserWithDatabase()
if (!userId) return nullReply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| @@ -0,0 +1 @@ | |||
| ALTER TABLE "users" ADD COLUMN "email" text; | |||
There was a problem hiding this comment.
Migration adds users.email without uniqueness / indexing
You’re now depending on email/prefix lookup for mentions (searchUsers via ilike and validation via prefix matching). Without an index, ilike 'prefix%' can degrade as the table grows.
Also, if you expect 1:1 mapping between Supabase users and rows, email duplication can cause ambiguous mentions and inconsistent results.
Suggestion
Add constraints/indexes that match how you query:
- If email should be unique:
CREATE UNIQUE INDEX users_email_unique_idx ON users (email); - If not unique, at least add an index:
CREATE INDEX users_email_idx ON users (email);
For prefix search on Postgres, consider varchar_pattern_ops or pg_trgm + GIN depending on desired search behavior.
Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/actions/users.ts (1)
20-51: 🛠️ Refactor suggestion | 🟠 MajorInconsistent user management: in-memory store vs. database.
The file retains an in-memory
usersStorewithgetUsersandaddUserfunctions that simulate database operations, while the newsyncUserWithDatabaseandsearchUsersfunctions use the actual database. This creates an inconsistent dual approach that may confuse maintainers.Consider removing the in-memory simulation if it's no longer needed, or clearly document the distinction if both approaches are intentionally preserved for different use cases.
lib/actions/calendar.ts (1)
97-106: 🛠️ Refactor suggestion | 🟠 MajorReorder operations: authenticate before database queries.
extractAndValidateMentions(line 102) performs a database query before the authentication check (line 103). If the user is unauthenticated, we've already incurred unnecessary database access.♻️ Proposed reordering
export async function saveNote(noteData: NewCalendarNote | CalendarNote): Promise<CalendarNote | null> { // Ensure current user is synced await syncUserWithDatabase(); const userId = await getCurrentUserIdOnServer(); - - const userTags = await extractAndValidateMentions(noteData.content); if (!userId) { console.error('saveNote: User not authenticated'); return null; } + + const userTags = await extractAndValidateMentions(noteData.content);
🤖 Fix all issues with AI agents
In `@components/calendar-notepad.tsx`:
- Around line 183-196: The suggestion list currently maps userSuggestions and
directly uses user.email for display and passing to handleSelectUser, but email
can be null; update the rendering in the showSuggestions block to skip or filter
out entries where user.email is null (e.g., filter userSuggestions by user.email
truthiness or add a conditional render inside the map), and ensure
handleSelectUser is only called with a non-null string; reference:
showSuggestions, userSuggestions, handleSelectUser, and user.email.
- Around line 218-222: The button rendering the MapPin when note.locationTags
exists is missing an explicit type and may submit surrounding forms (see the JSX
using note.locationTags and the handleFlyTo handler and MapPin icon); update
that button element to include type="button" so clicking it invokes handleFlyTo
without triggering form submission.
- Around line 94-112: handleNoteContentChange triggers searchUsers on every
keystroke and can produce out-of-order results; debounce the query or serialize
requests to prevent stale responses from overwriting newer ones: implement a
debounce around invoking searchUsers (or add a sequential requestId token stored
in a ref) inside handleNoteContentChange so only the latest search result calls
setUserSuggestions/setShowSuggestions and only for the current requestId, and
ensure setMentionQuery is updated with the debounced value; update references to
handleNoteContentChange, searchUsers, setUserSuggestions, setShowSuggestions and
setMentionQuery accordingly.
- Around line 27-29: The state userSuggestions is currently typed as any[];
define a proper TypeScript interface (e.g., UserSuggestion or UserRecord) that
matches the database user schema (fields like id, name, email, avatarUrl, etc.),
then replace useState<any[]>([]) with useState<UserSuggestion[]>([]) and update
any usages such as setUserSuggestions and places that read from userSuggestions
to use the new type; ensure imports/types are declared or exported where needed
so compiler and IDE get correct autocomplete for functions/components that
reference userSuggestions or mentionQuery.
In `@drizzle/migrations/0002_add_email_to_users.sql`:
- Line 1: The migration should make the "email" column NOT NULL and UNIQUE to
prevent duplicate emails; update the ALTER TABLE in the migration to add the
column with NOT NULL and a UNIQUE constraint (or create a UNIQUE index) for
"email". Also update the schema definition for the Users table in
lib/db/schema.ts (the Users/UsersTable export around line 12) to mark the email
field as non-nullable and unique so the TypeScript schema matches the DB change
and functions like extractAndValidateMentions and the calendar-notepad user
suggestions resolve unambiguously.
In `@drizzle/migrations/meta/0002_snapshot.json`:
- Around line 260-273: Add a database index on the "email" column to avoid
full-table scans for `@mention/autocomplete` queries by adding an entry for
"email" under the "indexes" object in the migration snapshot (the key names to
edit are "email" and "indexes"); also verify whether "email" should be UNIQUE
and if so add the corresponding entry under "uniqueConstraints" in the snapshot
to reflect the migration's UNIQUE constraint so the schema and constraints
remain consistent (update the JSON where "uniqueConstraints" and "indexes" are
defined).
- Line 75: The calendar_notes table currently has no indexes; add indexes for
the commonly filtered columns to improve query performance by updating the
migration/snapshot to define indexes on calendar_notes.user_id and
calendar_notes.date (or a composite index on (user_id, date) if queries often
filter by both). Modify the migration snapshot/definition that contains the
calendar_notes schema to include either individual indexes (e.g., index on
user_id and index on date) or a composite index, and generate/apply a new
migration that creates those indexes so the database and the Drizzle migration
metadata stay in sync.
In `@lib/actions/calendar.ts`:
- Around line 70-72: The current mentionRegex only captures word characters and
misses common email/name characters like dots and hyphens; update the
mentionRegex (used where mentionRegex, matches and potentialEmails are defined)
to allow letters, digits, dot, underscore, hyphen and plus so `@john.doe` and
similar tokens are fully captured, then regenerate matches and potentialEmails
from that updated pattern to ensure full local-part prefixes are extracted.
- Around line 69-93: The extractAndValidateMentions function currently fetches
all users which is inefficient; modify it to perform targeted DB queries for the
extracted mention prefixes (potentialEmails) instead of selecting the entire
users table: for example, perform a single query that filters users by email
ILIKE each mention% (or use a WHERE email ILIKE ANY(...) / multiple ORs) via the
same db/select pipeline that references usersSchema, then map the matching rows
to ids (dedupe) and return them; update the logic around potentialEmails,
validatedIds and the error handling to reflect the new query approach and avoid
loading all users into memory.
In `@lib/actions/users.ts`:
- Around line 57-80: The syncUserWithDatabase function may insert or update an
undefined email from getSupabaseUserAndSessionOnServer; update the logic in
syncUserWithDatabase so db.insert(usersSchema).values(...) only provides the
email field if user.email is defined (or explicitly set it to null if your
schema expects nullable), and change the update branch to only call .set({
email: user.email }) when user.email !== undefined (or use user.email ?? null if
you prefer an explicit null). Ensure comparisons against existingUser.email
handle undefined (e.g., check existingUser.email !== user.email && user.email
!== undefined) so you don't overwrite or insert an undefined email via
db.insert/db.update on usersSchema.
In `@lib/db/schema.ts`:
- Around line 10-14: The users table's email column is currently nullable and
not constrained, which can cause missed or ambiguous `@mention` matches; update
the users pgTable definition (the users export) to make the email column
non-nullable and add a uniqueness constraint (e.g., change text('email') to a
not-null text and add a unique constraint/index on the email column) so emails
mirror Supabase auth uniqueness and cannot be null or duplicated.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
components/calendar-notepad.tsxdrizzle/migrations/0002_add_email_to_users.sqldrizzle/migrations/meta/0000_snapshot.jsondrizzle/migrations/meta/0001_snapshot.jsondrizzle/migrations/meta/0002_snapshot.jsondrizzle/migrations/meta/_journal.jsonlib/actions/calendar.tslib/actions/users.tslib/db/schema.tspackage.json
🧰 Additional context used
🧬 Code graph analysis (3)
lib/actions/calendar.ts (5)
lib/db/schema.ts (1)
users(10-14)lib/db/index.ts (1)
db(25-25)lib/types/index.ts (2)
NewCalendarNote(92-92)CalendarNote(79-90)lib/actions/users.ts (1)
syncUserWithDatabase(57-81)lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)
components/calendar-notepad.tsx (2)
lib/actions/users.ts (1)
searchUsers(86-101)components/chat.tsx (1)
Chat(29-216)
lib/actions/users.ts (3)
lib/auth/get-current-user.ts (1)
getSupabaseUserAndSessionOnServer(22-103)lib/db/index.ts (1)
db(25-25)lib/auth/use-current-user.ts (1)
fetchUser(11-17)
🪛 Biome (2.3.13)
components/calendar-notepad.tsx
[error] 219-220: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (6)
drizzle/migrations/meta/0001_snapshot.json (1)
1-281: LGTM!The migration snapshot correctly defines the schema baseline with proper table structures, foreign key constraints with cascade deletes, and appropriate column definitions.
drizzle/migrations/meta/0000_snapshot.json (1)
1-178: LGTM!The snapshot format updates are consistent with the Drizzle ORM version upgrade. The changes include proper namespace prefixes (
public.), updated dialect identifier, and additional metadata fields required by the newer version.lib/actions/users.ts (1)
86-101: LGTM!The
searchUsersfunction correctly handles the@prefix stripping, uses parameterizedilikefor safe prefix matching, and limits results to 5. Error handling returns an empty array gracefully.components/calendar-notepad.tsx (1)
186-189: 🧹 Nitpick | 🔵 TrivialAdd
type="button"to suggestion buttons for consistency.The suggestion buttons should also have an explicit
type="button"attribute to prevent any potential form submission behavior.🔧 Proposed fix
<button key={user.id} - type="button" + type="button" onClick={() => handleSelectUser(user.email)}Note: I see
type="button"is already present in the code at line 188. This is correct.Actually, reviewing line 188 again -
type="button"is already present.Likely an incorrect or invalid review comment.
package.json (1)
65-65: Version bump is intentional and properly executed.The drizzle-orm upgrade from ^0.29.0 to ^0.45.1 is part of the Supabase integration work and is correctly paired with drizzle-kit ^0.31.1. Both packages are upgraded together, following the recommended migration path. The breaking changes in this version range (0.30.0–0.44.0) either do not apply to this PostgreSQL-based codebase or are already handled: migrations infrastructure is in place, timestamp columns use
{ withTimezone: true }, and no PostgreSQL index definitions require migration. No further action needed.drizzle/migrations/meta/_journal.json (1)
11-25: LGTM!The new migration journal entries are properly sequenced with incrementing indices and timestamps. The version upgrade from "5" to "7" in newer entries aligns with the drizzle-orm dependency update mentioned in the PR context.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const [showSuggestions, setShowSuggestions] = useState(false); | ||
| const [userSuggestions, setUserSuggestions] = useState<any[]>([]); | ||
| const [mentionQuery, setMentionQuery] = useState(""); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding proper TypeScript types for user suggestions.
userSuggestions is typed as any[]. For better type safety and IDE support, consider defining a proper type matching the database user schema.
♻️ Proposed typing
+type UserSuggestion = { id: string; email: string | null };
+
export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
// ...
- const [userSuggestions, setUserSuggestions] = useState<any[]>([]);
+ const [userSuggestions, setUserSuggestions] = useState<UserSuggestion[]>([]);🤖 Prompt for AI Agents
In `@components/calendar-notepad.tsx` around lines 27 - 29, The state
userSuggestions is currently typed as any[]; define a proper TypeScript
interface (e.g., UserSuggestion or UserRecord) that matches the database user
schema (fields like id, name, email, avatarUrl, etc.), then replace
useState<any[]>([]) with useState<UserSuggestion[]>([]) and update any usages
such as setUserSuggestions and places that read from userSuggestions to use the
new type; ensure imports/types are declared or exported where needed so compiler
and IDE get correct autocomplete for functions/components that reference
userSuggestions or mentionQuery.
| const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => { | ||
| const value = e.target.value; | ||
| setNoteContent(value); | ||
|
|
||
| const cursorPosition = e.target.selectionStart; | ||
| const textBeforeCursor = value.substring(0, cursorPosition); | ||
| const words = textBeforeCursor.split(/\s/); | ||
| const lastWord = words[words.length - 1]; | ||
|
|
||
| if (lastWord.startsWith("@")) { | ||
| const query = lastWord.slice(1); | ||
| setMentionQuery(query); | ||
| const results = await searchUsers(query); | ||
| setUserSuggestions(results); | ||
| setShowSuggestions(results.length > 0); | ||
| } else { | ||
| setShowSuggestions(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Potential race condition with async autocomplete on every keystroke.
handleNoteContentChange is an async function called on every keystroke. Rapid typing can cause out-of-order responses where an older search result overwrites a newer one. Consider debouncing the search or tracking request ordering.
🔧 Proposed debounce approach
+import { useRef } from "react";
+
export function CalendarNotepad({ chatId }: CalendarNotepadProps) {
// ... existing state ...
+ const searchTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => {
const value = e.target.value;
setNoteContent(value);
const cursorPosition = e.target.selectionStart;
const textBeforeCursor = value.substring(0, cursorPosition);
const words = textBeforeCursor.split(/\s/);
const lastWord = words[words.length - 1];
if (lastWord.startsWith("@")) {
const query = lastWord.slice(1);
setMentionQuery(query);
- const results = await searchUsers(query);
- setUserSuggestions(results);
- setShowSuggestions(results.length > 0);
+ if (searchTimeoutRef.current) clearTimeout(searchTimeoutRef.current);
+ searchTimeoutRef.current = setTimeout(async () => {
+ const results = await searchUsers(query);
+ setUserSuggestions(results);
+ setShowSuggestions(results.length > 0);
+ }, 150);
} else {
setShowSuggestions(false);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => { | |
| const value = e.target.value; | |
| setNoteContent(value); | |
| const cursorPosition = e.target.selectionStart; | |
| const textBeforeCursor = value.substring(0, cursorPosition); | |
| const words = textBeforeCursor.split(/\s/); | |
| const lastWord = words[words.length - 1]; | |
| if (lastWord.startsWith("@")) { | |
| const query = lastWord.slice(1); | |
| setMentionQuery(query); | |
| const results = await searchUsers(query); | |
| setUserSuggestions(results); | |
| setShowSuggestions(results.length > 0); | |
| } else { | |
| setShowSuggestions(false); | |
| } | |
| }; | |
| import { useRef } from "react"; | |
| export function CalendarNotepad({ chatId }: CalendarNotepadProps) { | |
| // ... existing state ... | |
| const searchTimeoutRef = useRef<NodeJS.Timeout | null>(null); | |
| const handleNoteContentChange = async (e: React.ChangeEvent<HTMLTextAreaElement>) => { | |
| const value = e.target.value; | |
| setNoteContent(value); | |
| const cursorPosition = e.target.selectionStart; | |
| const textBeforeCursor = value.substring(0, cursorPosition); | |
| const words = textBeforeCursor.split(/\s/); | |
| const lastWord = words[words.length - 1]; | |
| if (lastWord.startsWith("@")) { | |
| const query = lastWord.slice(1); | |
| setMentionQuery(query); | |
| if (searchTimeoutRef.current) clearTimeout(searchTimeoutRef.current); | |
| searchTimeoutRef.current = setTimeout(async () => { | |
| const results = await searchUsers(query); | |
| setUserSuggestions(results); | |
| setShowSuggestions(results.length > 0); | |
| }, 150); | |
| } else { | |
| setShowSuggestions(false); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@components/calendar-notepad.tsx` around lines 94 - 112,
handleNoteContentChange triggers searchUsers on every keystroke and can produce
out-of-order results; debounce the query or serialize requests to prevent stale
responses from overwriting newer ones: implement a debounce around invoking
searchUsers (or add a sequential requestId token stored in a ref) inside
handleNoteContentChange so only the latest search result calls
setUserSuggestions/setShowSuggestions and only for the current requestId, and
ensure setMentionQuery is updated with the debounced value; update references to
handleNoteContentChange, searchUsers, setUserSuggestions, setShowSuggestions and
setMentionQuery accordingly.
| {showSuggestions && ( | ||
| <div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto"> | ||
| {userSuggestions.map(user => ( | ||
| <button | ||
| key={user.id} | ||
| type="button" | ||
| onClick={() => handleSelectUser(user.email)} | ||
| className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate" | ||
| title={user.email} | ||
| > | ||
| {user.email} | ||
| </button> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
Add null check for user.email before rendering.
The searchUsers function returns users from the database where email could be null. Clicking a user with a null email would pass null to handleSelectUser, causing unexpected behavior.
🛡️ Proposed fix with null filtering
{showSuggestions && (
<div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto">
- {userSuggestions.map(user => (
+ {userSuggestions.filter(user => user.email).map(user => (
<button
key={user.id}
type="button"
onClick={() => handleSelectUser(user.email)}
className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate"
title={user.email}
>
{user.email}
</button>
))}
</div>
)}📝 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.
| {showSuggestions && ( | |
| <div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto"> | |
| {userSuggestions.map(user => ( | |
| <button | |
| key={user.id} | |
| type="button" | |
| onClick={() => handleSelectUser(user.email)} | |
| className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate" | |
| title={user.email} | |
| > | |
| {user.email} | |
| </button> | |
| ))} | |
| </div> | |
| {showSuggestions && ( | |
| <div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto"> | |
| {userSuggestions.filter(user => user.email).map(user => ( | |
| <button | |
| key={user.id} | |
| type="button" | |
| onClick={() => handleSelectUser(user.email)} | |
| className="w-full text-left px-3 py-2 hover:bg-accent rounded-sm text-sm truncate" | |
| title={user.email} | |
| > | |
| {user.email} | |
| </button> | |
| ))} | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
In `@components/calendar-notepad.tsx` around lines 183 - 196, The suggestion list
currently maps userSuggestions and directly uses user.email for display and
passing to handleSelectUser, but email can be null; update the rendering in the
showSuggestions block to skip or filter out entries where user.email is null
(e.g., filter userSuggestions by user.email truthiness or add a conditional
render inside the map), and ensure handleSelectUser is only called with a
non-null string; reference: showSuggestions, userSuggestions, handleSelectUser,
and user.email.
| {note.locationTags && ( | ||
| <button onClick={() => handleFlyTo(note.locationTags)} className="text-muted-foreground hover:text-foreground ml-2"> | ||
| <MapPin className="h-5 w-5" /> | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
Add explicit type="button" to prevent form submission.
The button element lacks an explicit type prop. In form contexts, buttons default to type="submit", which may cause unintended form submissions.
🔧 Proposed fix
- <button onClick={() => handleFlyTo(note.locationTags)} className="text-muted-foreground hover:text-foreground ml-2">
+ <button type="button" onClick={() => handleFlyTo(note.locationTags)} className="text-muted-foreground hover:text-foreground ml-2">
<MapPin className="h-5 w-5" />
</button>📝 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.
| {note.locationTags && ( | |
| <button onClick={() => handleFlyTo(note.locationTags)} className="text-muted-foreground hover:text-foreground ml-2"> | |
| <MapPin className="h-5 w-5" /> | |
| </button> | |
| )} | |
| {note.locationTags && ( | |
| <button type="button" onClick={() => handleFlyTo(note.locationTags)} className="text-muted-foreground hover:text-foreground ml-2"> | |
| <MapPin className="h-5 w-5" /> | |
| </button> | |
| )} |
🧰 Tools
🪛 Biome (2.3.13)
[error] 219-220: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In `@components/calendar-notepad.tsx` around lines 218 - 222, The button rendering
the MapPin when note.locationTags exists is missing an explicit type and may
submit surrounding forms (see the JSX using note.locationTags and the
handleFlyTo handler and MapPin icon); update that button element to include
type="button" so clicking it invokes handleFlyTo without triggering form
submission.
| "default": "now()" | ||
| } | ||
| }, | ||
| "indexes": {}, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding indexes for frequently queried columns.
The calendar_notes table has no indexes defined. For a calendar feature, queries likely filter by user_id and date. Adding indexes on these columns would improve query performance at scale.
🤖 Prompt for AI Agents
In `@drizzle/migrations/meta/0002_snapshot.json` at line 75, The calendar_notes
table currently has no indexes; add indexes for the commonly filtered columns to
improve query performance by updating the migration/snapshot to define indexes
on calendar_notes.user_id and calendar_notes.date (or a composite index on
(user_id, date) if queries often filter by both). Modify the migration
snapshot/definition that contains the calendar_notes schema to include either
individual indexes (e.g., index on user_id and index on date) or a composite
index, and generate/apply a new migration that creates those indexes so the
database and the Drizzle migration metadata stay in sync.
| "email": { | ||
| "name": "email", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| } | ||
| }, | ||
| "indexes": {}, | ||
| "foreignKeys": {}, | ||
| "compositePrimaryKeys": {}, | ||
| "uniqueConstraints": {}, | ||
| "policies": {}, | ||
| "checkConstraints": {}, | ||
| "isRLSEnabled": false |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding an index on the email column for mention search performance.
The @mention autocomplete feature will search users by email. Without an index, this becomes a full table scan that degrades as the user base grows.
Additionally, confirm whether uniqueConstraints should be populated here to match the UNIQUE constraint suggested for the migration.
🤖 Prompt for AI Agents
In `@drizzle/migrations/meta/0002_snapshot.json` around lines 260 - 273, Add a
database index on the "email" column to avoid full-table scans for
`@mention/autocomplete` queries by adding an entry for "email" under the "indexes"
object in the migration snapshot (the key names to edit are "email" and
"indexes"); also verify whether "email" should be UNIQUE and if so add the
corresponding entry under "uniqueConstraints" in the snapshot to reflect the
migration's UNIQUE constraint so the schema and constraints remain consistent
(update the JSON where "uniqueConstraints" and "indexes" are defined).
|
@coderabbitai when I add people on the team in settings the site completely refreshes losing all contexts, then after the users that have been added cannot be instantiated in the notes section, do an in depth analysis of this code issue. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 610 🏁 Script executed: Length of output: 26639 🏁 Script executed: Length of output: 13120 🏁 Script executed: Length of output: 5921
🔍 Root Cause AnalysisIssue 1: Page Refresh Losing ContextWhen you add users in settings, the
Here's the problematic code in // This is the OLD in-memory implementation still being used
let usersStore: Record<string, Array<User>> = {
'default-user': [
{ id: '1', email: 'admin@example.com', role: 'admin' },
{ id: '2', email: 'editor@example.com', role: 'editor' },
],
};
export async function addUser(userId: string = 'default-user', newUser: { email: string; role: UserRole }): Promise<{ user?: User; error?: string }> {
// ... validation ...
const userToAdd: User = { ...newUser, id: Math.random().toString(36).substr(2, 9) };
usersStore[userId].push(userToAdd); // ❌ Only adds to memory
revalidatePath('/settings'); // ❌ Causes page refresh
return { user: userToAdd };
}Issue 2: Users Not Available for MentionsThe calendar notepad uses export async function searchUsers(query: string) {
// ...
const results = await db.select()
.from(usersSchema)
.where(ilike(usersSchema.email, `${searchTerm}%`)) // ❌ Queries DB, not memory
.limit(5);
return results;
}The disconnect: Users added via settings are stored only in the in-memory 💡 Solution RecommendationsYou need to replace the old in-memory 1. Replace
|
I have completed an in-depth analysis and fixed the issues you mentioned:
|
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
|
@coderabbitai now resolution search has stopped working! can you make the add user and tag user as well as add user to notes feature work in memory as a flag for now before the backend is connected |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 14570
Plan: Add In-Memory User Management with Feature FlagI'll implement a feature flag system that allows you to switch between in-memory and database modes. By default, it will use in-memory storage until you're ready to connect the backend. Changes needed:
Summary of changes: ✅ In-memory mode by default - Set ✅ All features work in memory:
✅ Sample data included - 3 users pre-loaded: admin@example.com, editor@example.com, viewer@example.com ✅ Easy to switch - When backend is ready, just change the flag to The in-memory store persists during the server's runtime, so you won't lose users between operations. When you're ready to connect the real backend, simply change 💡 Use |
I've addressed the resolution search failure and implemented the in-memory fallback as requested:
|
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@drizzle/migrations/0003_bent_cammi.sql`:
- Line 1: The migration adds a nullable "role" column but lib/db/schema.ts
defines role as non-null with a default; update the migration to add the column
with the same NOT NULL and DEFAULT value used in lib/db/schema.ts (e.g., ALTER
TABLE "users" ADD COLUMN "role" text NOT NULL DEFAULT
'<exact_default_from_lib/db/schema.ts">'), ensuring the literal default matches
the schema file so existing rows get the default and runtime assumptions stay
consistent.
In `@drizzle/migrations/meta/0003_snapshot.json`:
- Around line 260-271: The snapshot currently marks users.email and users.role
as nullable; update the migration that defines the users table so the email
column has notNull: true and the role column has notNull: true plus the proper
default (e.g., default: 'user' or your app's default), then re-run your
migration/snapshot generation command (drizzle-kit or your migration tool) to
regenerate the 0003 snapshot so it reflects NOT NULL and the role default;
specifically locate the migration that creates/updates the users table (the
users table definition and its columns email and role) and change the column
metadata to notNull: true and add the default for role before regenerating the
snapshot.
In `@lib/actions/users.ts`:
- Around line 21-27: The module-level usersStore currently seeds hardcoded
default users which is unsafe for serverless and production; change usersStore
to initialize as an empty Record<string, Array<User>> and only populate sample
users when an explicit dev/test flag is set (e.g., NODE_ENV === 'development' or
a dedicated isDev/isTest toggle), add a clear processLogger.warn or similar
warning when the fallback in-memory store is activated so production incidents
are visible, and update the README/comments to state this fallback is intended
for local/dev testing only; ensure all references to usersStore (the
module-level variable) and any functions that call it check the dev/test flag
before using seeded data.
- Around line 34-50: getUsers currently ignores the userId when db is present,
causing different semantics vs the in-memory fallback; update getUsers to treat
userId as a tenant/context key: change the DB path (the query built from
usersSchema in getUsers) to filter by userId (e.g., add a where clause using the
schema's user id column) and return only that user's records, and keep the
existing in-memory fallback using usersStore[userId]; ensure the try/catch still
falls back to simulateDBDelay and usersStore[userId] on DB error so both code
paths return the same tenant-scoped results.
- Around line 56-73: The DB catch block currently logs the error and lets
execution fall through to the in-memory path, masking failures; change the catch
in the users add flow (the try around db.select/db.insert against usersSchema
and db) to surface the failure by returning an error result or rethrowing the
error instead of continuing: include the original error message (or error
object) in the returned error payload (e.g., return { error: 'Database error',
details: String(error) }) or throw error after logging so callers do not get a
successful in-memory response when db.insert or db.select fails. Ensure this
change is applied to the block that uses db.select().from(usersSchema)... and
db.insert(usersSchema)...returning() so the in-memory fallback is not executed
on DB errors.
- Around line 96-105: The DB update block using db.update(usersSchema).set({
role: newRole }).where(eq(usersSchema.email, userEmail)).returning() currently
falls through when the returned variable updated is falsy; detect that case
immediately after the update and short-circuit by returning or throwing a "user
not found" result (consistent with the function's error handling) instead of
continuing to the in-memory fallback branch—ensure you reference the updated
variable and userEmail to decide and avoid touching the in-memory store when the
DB reports zero rows affected.
- Around line 262-284: The current getSelectedModel and saveSelectedModel
functions use modelConfigPath to read/write a local config file which won't
persist in serverless/ephemeral environments; replace this file-based
persistence with a durable store (e.g., the app's database or environment-backed
config): change getSelectedModel to query the user/settings table or a shared
settings row for the selectedModel (falling back to the file only for
local/dev), and change saveSelectedModel to upsert the selectedModel into that
DB/settings store and return detailed error info on failure; ensure to keep
noStore() behavior where needed and update call sites that import
getSelectedModel/saveSelectedModel to work with the async DB-backed API.
- Around line 152-184: The code runs deletes and upserts one-by-one and without
a transaction, which can cause N+1 queries and partial commits; wrap the whole
operation in a transaction (use db.transaction or equivalent) and perform
set-based operations: compute incomingEmails from formData.users, use
tx.delete(usersSchema).where(email not in incomingEmails) to delete missing
users in one statement, fetch existingUsers via tx.select().from(usersSchema),
split formData.users into toInsert and toUpdate arrays by comparing to
existingUsers, then inside the same transaction perform bulk insert for toInsert
(assign id with crypto.randomUUID() when missing) and either use a single upsert
API (insert...onConflict...doUpdate) if supported or run Promise.all of
tx.update(...) calls for toUpdate so updates run in parallel but still inside
the transaction; return the final tx.select().from(usersSchema) and ensure the
transaction rolls back on error.
In `@lib/db/index.ts`:
- Around line 12-26: The current PoolConfig sets ssl.rejectUnauthorized = false
when databaseUrl includes 'supabase.co', which disables TLS verification; update
the poolConfig in the if (databaseUrl.includes('supabase.co')) branch to load
Supabase's root CA and keep verification enabled: read the CA file (e.g., via
fs.readFileSync) and set ssl = { ca: <caPem>, rejectUnauthorized: true }, and if
the CA file is not present fall back only when an explicit env flag (e.g.,
process.env.ALLOW_INSECURE_SSL === 'true') is set to allow rejectUnauthorized:
false; ensure this change is applied where Pool is constructed and dbInstance =
drizzle(...) is created so the Pool uses the trusted CA.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
components/settings/components/user-management-form.tsxdrizzle/migrations/0003_bent_cammi.sqldrizzle/migrations/meta/0003_snapshot.jsondrizzle/migrations/meta/_journal.jsonlib/actions/calendar.tslib/actions/users.tslib/db/index.tslib/db/schema.ts
🧰 Additional context used
🧬 Code graph analysis (2)
lib/db/index.ts (1)
lib/db/migrate.ts (1)
runMigrations(8-34)
lib/actions/users.ts (2)
lib/db/index.ts (1)
db(33-33)lib/auth/get-current-user.ts (1)
getSupabaseUserAndSessionOnServer(22-103)
🔇 Additional comments (8)
drizzle/migrations/meta/_journal.json (1)
11-31: Journal entries look consistent.Appended migration entries align with the new migration artifacts.
lib/db/schema.ts (1)
12-13: Good schema tightening for required user fields.Non-null
rolealign with expected user metadata.components/settings/components/user-management-form.tsx (1)
146-149: Nice fix to prevent accidental form submission.Adding
type="button"on action buttons avoids unintended submit behavior.lib/actions/calendar.ts (4)
4-5: Imports are appropriate for mention validation.
25-58: Graceful DB-unavailable guard is good.Returning an empty list with a warning keeps behavior predictable when db is unset.
129-163: User tag persistence in update/insert paths looks correct.
112-127: No changes needed —syncUserWithDatabase()is already safe to call unconditionally.The function has an explicit
if (db)guard at line 204 of lib/actions/users.ts that prevents database operations when db is unavailable. All database queries and mutations are wrapped within this guard, so the function handles db unavailability gracefully without throwing. The unconditional call insaveNote()is correct.lib/actions/users.ts (1)
241-260: LGTM!The search implementation correctly:
- Validates minimum query length
- Strips the
@prefix for autocomplete compatibility- Uses
ilikefor case-insensitive prefix matching (drizzle-orm parameterizes the value safely)- Limits results to 5 for autocomplete performance
- Falls back to in-memory search across all contexts
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "email": { | ||
| "name": "email", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| }, | ||
| "role": { | ||
| "name": "role", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| } |
There was a problem hiding this comment.
Snapshot reflects nullable fields that should be non-null.
users.email and users.role are marked nullable here, but the app schema treats them as required. After fixing the migration, regenerate the snapshot so it reflects NOT NULL (and role default).
🤖 Prompt for AI Agents
In `@drizzle/migrations/meta/0003_snapshot.json` around lines 260 - 271, The
snapshot currently marks users.email and users.role as nullable; update the
migration that defines the users table so the email column has notNull: true and
the role column has notNull: true plus the proper default (e.g., default: 'user'
or your app's default), then re-run your migration/snapshot generation command
(drizzle-kit or your migration tool) to regenerate the 0003 snapshot so it
reflects NOT NULL and the role default; specifically locate the migration that
creates/updates the users table (the users table definition and its columns
email and role) and change the column metadata to notNull: true and add the
default for role before regenerating the snapshot.
| export async function getUsers(userId: string = 'default-user'): Promise<{ users: User[] }> { | ||
| if (db) { | ||
| try { | ||
| const results = await db.select().from(usersSchema); | ||
| return { users: results as User[] }; | ||
| } catch (error) { | ||
| console.error('Error fetching users from DB:', error); | ||
| } | ||
| } | ||
|
|
||
| // Fallback | ||
| await simulateDBDelay(); | ||
| if (!usersStore[userId]) { | ||
| usersStore[userId] = []; | ||
| } | ||
| console.log(`[Action: getUsers] Fetched users for ${userId}:`, usersStore[userId]); | ||
| return { users: usersStore[userId] }; | ||
| } |
There was a problem hiding this comment.
userId parameter ignored in DB path creates inconsistent behavior.
When DB is available, getUsers returns all users globally (line 37). When DB is unavailable, it returns users keyed by userId (line 49). This semantic mismatch means callers will get different results depending on which code path executes.
If userId is intended as a tenant/context key, the DB query should filter by it. If global user list is intended, remove the parameter and the in-memory keying.
🔧 Proposed fix if userId should be ignored (simplify in-memory to match DB)
-let usersStore: Record<string, Array<User>> = {
- 'default-user': [
+let usersStore: Array<User> = [
{ id: '1', email: 'admin@example.com', role: 'admin' },
{ id: '2', email: 'editor@example.com', role: 'editor' },
- ],
-};
+];
export async function getUsers(userId: string = 'default-user'): Promise<{ users: User[] }> {
if (db) {
// ... DB path unchanged
}
await simulateDBDelay();
- if (!usersStore[userId]) {
- usersStore[userId] = [];
- }
- return { users: usersStore[userId] };
+ return { users: usersStore };
}🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 34 - 50, getUsers currently ignores the
userId when db is present, causing different semantics vs the in-memory
fallback; update getUsers to treat userId as a tenant/context key: change the DB
path (the query built from usersSchema in getUsers) to filter by userId (e.g.,
add a where clause using the schema's user id column) and return only that
user's records, and keep the existing in-memory fallback using
usersStore[userId]; ensure the try/catch still falls back to simulateDBDelay and
usersStore[userId] on DB error so both code paths return the same tenant-scoped
results.
| const [updated] = await db.update(usersSchema) | ||
| .set({ role: newRole }) | ||
| .where(eq(usersSchema.email, userEmail)) | ||
| .returning(); | ||
|
|
||
| if (updated) return { user: updated as User }; | ||
| } catch (error) { | ||
| console.error('Error updating role in DB:', error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing "user not found" handling when DB update affects zero rows.
If updated is falsy (no matching email in DB), the code falls through to the in-memory path instead of returning an error. This causes incorrect behavior: a user that doesn't exist in DB could be "updated" in memory, or the in-memory store could report "not found" for a user that exists in DB under a different context.
🐛 Proposed fix
const [updated] = await db.update(usersSchema)
.set({ role: newRole })
.where(eq(usersSchema.email, userEmail))
.returning();
- if (updated) return { user: updated as User };
+ if (updated) {
+ return { user: updated as User };
+ }
+ return { error: 'User not found.' };
} catch (error) {
console.error('Error updating role in DB:', error);
+ // Only fall through to in-memory on connection/query errors, not "not found"
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [updated] = await db.update(usersSchema) | |
| .set({ role: newRole }) | |
| .where(eq(usersSchema.email, userEmail)) | |
| .returning(); | |
| if (updated) return { user: updated as User }; | |
| } catch (error) { | |
| console.error('Error updating role in DB:', error); | |
| } | |
| } | |
| const [updated] = await db.update(usersSchema) | |
| .set({ role: newRole }) | |
| .where(eq(usersSchema.email, userEmail)) | |
| .returning(); | |
| if (updated) { | |
| return { user: updated as User }; | |
| } | |
| return { error: 'User not found.' }; | |
| } catch (error) { | |
| console.error('Error updating role in DB:', error); | |
| // Only fall through to in-memory on connection/query errors, not "not found" | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 96 - 105, The DB update block using
db.update(usersSchema).set({ role: newRole }).where(eq(usersSchema.email,
userEmail)).returning() currently falls through when the returned variable
updated is falsy; detect that case immediately after the update and
short-circuit by returning or throwing a "user not found" result (consistent
with the function's error handling) instead of continuing to the in-memory
fallback branch—ensure you reference the updated variable and userEmail to
decide and avoid touching the in-memory store when the DB reports zero rows
affected.
| if (db) { | ||
| try { | ||
| const existingUsers = await db.select().from(usersSchema); | ||
| const incomingEmails = new Set(formData.users.map(u => u.email)); | ||
|
|
||
| // Delete missing | ||
| const toDelete = existingUsers.filter((u: any) => u.email && !incomingEmails.has(u.email)); | ||
| for (const u of toDelete) { | ||
| if (u.email) await db.delete(usersSchema).where(eq(usersSchema.email, u.email)); | ||
| } | ||
|
|
||
| // Simulate saving systemPrompt and selectedModel | ||
| // ... (logic for other settings) | ||
| // Upsert incoming | ||
| for (const u of formData.users) { | ||
| if (!u.email) continue; | ||
| const existing = existingUsers.find((ex: any) => ex.email === u.email); | ||
| if (existing) { | ||
| await db.update(usersSchema) | ||
| .set({ role: u.role }) | ||
| .where(eq(usersSchema.id, existing.id)); | ||
| } else { | ||
| await db.insert(usersSchema).values({ | ||
| id: u.id || crypto.randomUUID(), | ||
| email: u.email, | ||
| role: u.role | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| // For users, the frontend form already constructs the 'users' array. | ||
| // Here, we could compare the incoming users list with the stored one | ||
| // and make granular calls to addUser, updateUserRole, removeUser if needed, | ||
| // or simply replace the user list if that's the desired behavior. | ||
| // For simplicity in this simulation, let's assume the form sends the complete new user list. | ||
| const updatedUsers = await db.select().from(usersSchema); | ||
| return { success: true, message: 'Settings and users updated successfully.', users: updatedUsers as User[] }; | ||
| } catch (error) { | ||
| console.error('Error updating settings and users in DB:', error); | ||
| } |
There was a problem hiding this comment.
Bulk update lacks transaction and uses N+1 queries.
The current implementation:
- Executes deletes and upserts one-by-one in a loop (N+1 queries)
- Has no transaction boundary—partial failures leave the database in an inconsistent state
If an insert fails mid-loop (line 172-177), previously deleted users are lost and only some new users are created.
🔧 Proposed fix using transaction
if (db) {
try {
+ return await db.transaction(async (tx) => {
const existingUsers = await tx.select().from(usersSchema);
const incomingEmails = new Set(formData.users.map(u => u.email));
// Delete missing
const toDelete = existingUsers.filter((u: any) => u.email && !incomingEmails.has(u.email));
- for (const u of toDelete) {
- if (u.email) await db.delete(usersSchema).where(eq(usersSchema.email, u.email));
- }
+ if (toDelete.length > 0) {
+ const emailsToDelete = toDelete.map(u => u.email).filter(Boolean);
+ // Use inArray for batch delete
+ await tx.delete(usersSchema).where(inArray(usersSchema.email, emailsToDelete));
+ }
// Upsert incoming (consider using onConflictDoUpdate for true upsert)
for (const u of formData.users) {
// ... existing upsert logic but use `tx` instead of `db`
}
const updatedUsers = await tx.select().from(usersSchema);
return { success: true, message: 'Settings and users updated successfully.', users: updatedUsers as User[] };
+ });
} catch (error) {🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 152 - 184, The code runs deletes and
upserts one-by-one and without a transaction, which can cause N+1 queries and
partial commits; wrap the whole operation in a transaction (use db.transaction
or equivalent) and perform set-based operations: compute incomingEmails from
formData.users, use tx.delete(usersSchema).where(email not in incomingEmails) to
delete missing users in one statement, fetch existingUsers via
tx.select().from(usersSchema), split formData.users into toInsert and toUpdate
arrays by comparing to existingUsers, then inside the same transaction perform
bulk insert for toInsert (assign id with crypto.randomUUID() when missing) and
either use a single upsert API (insert...onConflict...doUpdate) if supported or
run Promise.all of tx.update(...) calls for toUpdate so updates run in parallel
but still inside the transaction; return the final tx.select().from(usersSchema)
and ensure the transaction rolls back on error.
| if (databaseUrl) { | ||
| const poolConfig: PoolConfig = { | ||
| connectionString: databaseUrl, | ||
| }; | ||
| } | ||
|
|
||
| const pool = new Pool(poolConfig); | ||
| if (databaseUrl.includes('supabase.co')) { | ||
| poolConfig.ssl = { | ||
| rejectUnauthorized: false, | ||
| }; | ||
| } | ||
|
|
||
| try { | ||
| const pool = new Pool(poolConfig); | ||
| dbInstance = drizzle(pool, { schema, logger: process.env.NODE_ENV === 'development' }); | ||
| } catch (error) { |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Supabase PostgreSQL SSL connection rejectUnauthorized configuration
💡 Result:
For Supabase Postgres, rejectUnauthorized is a Node.js TLS option (used by drivers like pg, Sequelize, etc.). You generally only set rejectUnauthorized: false when you can’t validate the server cert (e.g., missing CA), but the recommended approach is to use Supabase’s root CA and keep verification on.
Recommended (verify server cert)
- Download the Server root certificate from your Supabase Dashboard (Database → SSL configuration). (supabase.com)
- Connect with full verification (Postgres
sslmode=verify-full). (supabase.com)
pg (node-postgres) example
import fs from "fs";
import { Pool } from "pg";
const pool = new Pool({
connectionString: process.env.DATABASE_URL, // keep this free of sslmode/sslrootcert if you set ssl below
ssl: {
ca: fs.readFileSync("./prod-supabase.cer").toString(),
rejectUnauthorized: true,
},
});pg passes the ssl object to Node’s TLSSocket, where rejectUnauthorized controls certificate verification. (node-postgres.com)
Important: if your connectionString includes sslmode, sslrootcert, etc., pg will overwrite the ssl object you provided. (node-postgres.com)
“Make it connect” (not recommended for production)
If you’re seeing cert errors and just need a quick workaround, you can disable verification:
ssl: { rejectUnauthorized: false }This is the common pattern for self-signed / unverified cert situations in pg, but it weakens security. (node-postgres.com)
Supabase-side note (enforcement)
Supabase can be configured to reject non-SSL connections (“SSL enforcement”). This is separate from rejectUnauthorized (which is client-side cert verification). (supabase.com)
Citations:
- 1: https://supabase.com/docs/guides/database/psql?utm_source=openai
- 2: https://supabase.com/docs/guides/database/psql?utm_source=openai
- 3: https://node-postgres.com/features/ssl?utm_source=openai
- 4: https://node-postgres.com/features/ssl?utm_source=openai
- 5: https://node-postgres.com/features/ssl?utm_source=openai
- 6: https://supabase.com/docs/guides/platform/ssl-enforcement?utm_source=openai
🌐 Web query:
Node.js pg pool SSL rejectUnauthorized Supabase best practices
💡 Result:
Best practice (Node.js pg / Pool) for Supabase SSL
1) Don’t use rejectUnauthorized: false in production.
That setting disables certificate verification (you’re encrypted, but vulnerable to MITM). Instead, verify the server cert using Supabase’s CA cert and keep rejectUnauthorized: true (default). Supabase recommends the strongest client mode, verify-full, which requires trusting their CA certificate. [1][2]
2) Download the Supabase database CA certificate and pass it as ssl.ca.
You can download the “Server root certificate” / SSL certificate from your Supabase project dashboard (Database settings / SSL Configuration). [2][3]
import fs from "node:fs";
import { Pool } from "pg";
const pool = new Pool({
connectionString: process.env.DATABASE_URL, // keep this "clean" (no sslmode/sslrootcert params)
ssl: {
ca: fs.readFileSync("./prod-ca-2021.cer", "utf8"), // from Supabase dashboard
// rejectUnauthorized: true, // optional; true is the default
},
});3) Avoid mixing sslmode / sslrootcert in the connection string with an ssl object.
node-postgres warns that if sslmode, sslrootcert, etc. are present in the connection string, it can overwrite/replace the ssl config you pass in code—leading to accidental misconfiguration. Best practice: keep TLS options in the ssl object (as above). [4]
4) If you use Supabase “SSL enforcement”, still verify properly client-side.
Supabase notes that enforcement ensures SSL is used, but for strongest protection you should use verify-full semantics by trusting the Supabase CA cert (i.e., don’t skip verification). [2]
5) Pooling note (common with Supabase):
If you’re using Supabase’s pooler (Supavisor / “session pooler”), use the pooler host/port shown in the “Connect” panel; Supabase docs highlight pooled connections typically use port 6543. [3]
Sources
[1] Supabase Docs — “Connecting with SSL” (psql example with sslmode=verify-full + sslrootcert)
[2] Supabase Docs — SSL Enforcement + recommendation to use verify-full and download CA cert
[3] Supabase Docs — Connecting to Postgres (mentions downloading cert; pooling port 6543)
[4] node-postgres Docs — SSL + warning about connectionString params overwriting the ssl object
🌐 Web query:
Supabase postgres client certificate SSL requirements
💡 Result:
Supabase Postgres connections use TLS/SSL server authentication, not client-certificate (mTLS) authentication.
- SSL is (optionally) enforced server-side per project, and Supabase recommends using Postgres
sslmode=verify-fullwhen enforcement is enabled. To do that you download the Supabase database CA/root certificate from the Dashboard (Database Settings → SSL Configuration) and trust it locally. [1] - Typical connection parameters are
sslmode=verify-full+sslrootcert=/path/to/prod-supabase.cer(example shown forpsql). [2] - Client certificates (
sslcert/sslkey) are only used if the Postgres server requests them (i.e., when the server is configured to verify client cert identity). Postgres/libpq documents these parameters, but Supabase’s published connection guidance only calls forsslmodeandsslrootcert, not client cert/key. [2][3]
Bottom line: For Supabase you generally need the server root CA cert (sslrootcert) and a secure SSL mode (ideally verify-full); you do not need a client certificate unless you’re running your own Postgres configured for mTLS. [1][2][3]
Sources: [1] Supabase “Postgres SSL Enforcement” docs. [2] Supabase “Connecting with PSQL” docs. [3] PostgreSQL libpq SSL support (client certificates).
Use Supabase's root CA certificate instead of disabling TLS verification.
Setting rejectUnauthorized: false disables certificate verification and creates a MITM vulnerability. Supabase's official recommendation is to download the root CA certificate from your Supabase Dashboard (Database Settings → SSL Configuration) and trust it locally:
import fs from "fs";
const poolConfig: PoolConfig = {
connectionString: databaseUrl,
ssl: {
ca: fs.readFileSync("./prod-supabase.cer", "utf8"), // from Supabase dashboard
rejectUnauthorized: true, // default; explicit for clarity
},
};If the CA certificate is unavailable at runtime, gate insecure SSL behind an explicit environment flag as a temporary workaround only.
🤖 Prompt for AI Agents
In `@lib/db/index.ts` around lines 12 - 26, The current PoolConfig sets
ssl.rejectUnauthorized = false when databaseUrl includes 'supabase.co', which
disables TLS verification; update the poolConfig in the if
(databaseUrl.includes('supabase.co')) branch to load Supabase's root CA and keep
verification enabled: read the CA file (e.g., via fs.readFileSync) and set ssl =
{ ca: <caPem>, rejectUnauthorized: true }, and if the CA file is not present
fall back only when an explicit env flag (e.g., process.env.ALLOW_INSECURE_SSL
=== 'true') is set to allow rejectUnauthorized: false; ensure this change is
applied where Pool is constructed and dbInstance = drizzle(...) is created so
the Pool uses the trusted CA.
|
@coderabbitai 18:25:55.197 Running build in Portland, USA (West) – pdx1 |
I've fixed the build error by explicitly typing the |
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
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 (1)
lib/actions/chat-db.ts (1)
223-223:⚠️ Potential issue | 🟡 MinorRemove module-level
console.logbefore merging.This runs on every import in production and leaks internal implementation details to server logs. It appears to be a development artifact.
Proposed fix
-console.log('Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable.');
🤖 Fix all issues with AI agents
In `@drizzle/migrations/0004_tense_darkstar.sql`:
- Around line 1-3: The migration alters users.email and users.role to NOT NULL
and sets a default for role, but will fail if existing rows contain NULLs; in
0004_tense_darkstar.sql, add a safe sequence: first ALTER TABLE "users" ALTER
COLUMN "role" SET DEFAULT 'viewer' (already present), then run UPDATE "users"
SET "role" = 'viewer' WHERE "role" IS NULL, and for email either UPDATE "users"
SET "email" = '<sentinel>' WHERE "email" IS NULL (or omit the ALTER until
app-level guarantees exist), and only after backfilling execute ALTER TABLE
"users" ALTER COLUMN "role" SET NOT NULL and ALTER TABLE "users" ALTER COLUMN
"email" SET NOT NULL so the NOT NULL changes won't fail on existing data.
In `@drizzle/migrations/meta/0004_snapshot.json`:
- Around line 249-281: The snapshot shows empty "indexes" — add indexes in the
Drizzle schema for the searchable/joinable columns to improve query performance:
create an index on users.email (used by searchUsers), on calendar_notes.user_id
and calendar_notes.date (used by calendar view filters), and on messages.chat_id
(used by getMessagesByChatId); update the table definitions in your Drizzle
schema (e.g., users, calendar_notes, messages) to declare these indexes and then
regenerate the migration so the new index entries appear in the migration
snapshot and are applied to the DB.
- Around line 48-53: The migration defines user_tags as a text[] which prevents
DB-level referential integrity; replace this with a proper join table (e.g.,
calendar_note_mentions) that contains columns like note_id and mentioned_user_id
and add foreign keys referencing the calendar_notes (or notes) primary key and
users table primary key, migrating existing entries from the user_tags array
into rows and then dropping or deprecating the user_tags column. Update any code
that reads/writes user_tags to use the new calendar_note_mentions join table and
ensure cascade/delete rules are set so referenced mentions are removed when a
user or note is deleted.
- Around line 260-265: The users.email column must be made DB-unique: update the
email column definition in lib/db/schema.ts (the users table column named
"email") to include .unique() so the schema enforces uniqueness, then regenerate
the migration (so the new constraint appears in the generated migration JSON,
e.g., update 0004_snapshot.json) and run the migration; ensure related code
paths that assume unique emails (searchUsers in lib/actions/users.ts and the
mention logic in components/calendar-notepad.tsx) continue to work with the
DB-enforced uniqueness.
In `@lib/actions/chat-db.ts`:
- Line 90: The root cause is that db is typed as any, so transactions use tx:
any; in lib/db/index.ts change the dbInstance declaration to use Drizzle's
NodePgDatabase generic (e.g., let dbInstance: NodePgDatabase<typeof schema> |
null) and export the properly typed db so callers get correct inference; then
remove the explicit tx: any in calls like db.transaction(async (tx) => { ... })
in lib/actions/chat-db.ts; also remove the module-load debug console.log in
chat-db.ts about "Chat DB actions loaded..." and ensure getCurrentUserId()
remains server-safe.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
drizzle/migrations/0004_tense_darkstar.sqldrizzle/migrations/meta/0004_snapshot.jsondrizzle/migrations/meta/_journal.jsonlib/actions/chat-db.ts
🧰 Additional context used
🧬 Code graph analysis (1)
drizzle/migrations/meta/0004_snapshot.json (1)
lib/db/schema.ts (2)
user(60-67)chats(54-58)
🔇 Additional comments (2)
drizzle/migrations/meta/_journal.json (1)
1-41: Journal entries are consistent and correctly ordered.Timestamps are monotonically increasing, tags align with the corresponding SQL migration files, and breakpoints are enabled on all entries. The version "5" → "7" transition for newer entries is expected after a Drizzle Kit upgrade.
drizzle/migrations/meta/0004_snapshot.json (1)
1-294: Snapshot is consistent with the migration SQL and schema.The four tables, their columns, defaults, NOT NULL constraints, and foreign key definitions correctly reflect the cumulative state after migrations 0000–0004. The
prevIdcorrectly chains to the prior snapshot.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint | ||
| ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL; No newline at end of file |
There was a problem hiding this comment.
Migration will fail if existing rows have NULL email or role values.
Migrations 0002 and 0003 added email and role as nullable columns. If any rows were inserted between those migrations and this one (or before backfilling), applying SET NOT NULL will throw a constraint violation at the database level. Add UPDATE statements before the ALTER to backfill NULLs:
Proposed safe migration
+UPDATE "users" SET "email" = '' WHERE "email" IS NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint
+UPDATE "users" SET "role" = 'viewer' WHERE "role" IS NULL;--> statement-breakpoint
ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL;Choose a meaningful sentinel for email (or skip NOT NULL until the application guarantees it). Setting the default for role before the UPDATE ensures new concurrent inserts are also safe.
📝 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.
| ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint | |
| ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint | |
| ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL; | |
| UPDATE "users" SET "email" = '' WHERE "email" IS NULL;--> statement-breakpoint | |
| ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL;--> statement-breakpoint | |
| ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer';--> statement-breakpoint | |
| UPDATE "users" SET "role" = 'viewer' WHERE "role" IS NULL;--> statement-breakpoint | |
| ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL; |
🤖 Prompt for AI Agents
In `@drizzle/migrations/0004_tense_darkstar.sql` around lines 1 - 3, The migration
alters users.email and users.role to NOT NULL and sets a default for role, but
will fail if existing rows contain NULLs; in 0004_tense_darkstar.sql, add a safe
sequence: first ALTER TABLE "users" ALTER COLUMN "role" SET DEFAULT 'viewer'
(already present), then run UPDATE "users" SET "role" = 'viewer' WHERE "role" IS
NULL, and for email either UPDATE "users" SET "email" = '<sentinel>' WHERE
"email" IS NULL (or omit the ALTER until app-level guarantees exist), and only
after backfilling execute ALTER TABLE "users" ALTER COLUMN "role" SET NOT NULL
and ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL so the NOT NULL
changes won't fail on existing data.
| "user_tags": { | ||
| "name": "user_tags", | ||
| "type": "text[]", | ||
| "primaryKey": false, | ||
| "notNull": false | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
user_tags as text[] has no referential integrity.
Storing mentioned user IDs in a text array means the database cannot enforce that those IDs reference valid users. If a user is deleted (cascading from other FKs), stale IDs will remain in user_tags. Consider whether a join table (e.g., calendar_note_mentions) would better serve data integrity, or document that the application layer must handle dangling references.
🤖 Prompt for AI Agents
In `@drizzle/migrations/meta/0004_snapshot.json` around lines 48 - 53, The
migration defines user_tags as a text[] which prevents DB-level referential
integrity; replace this with a proper join table (e.g., calendar_note_mentions)
that contains columns like note_id and mentioned_user_id and add foreign keys
referencing the calendar_notes (or notes) primary key and users table primary
key, migrating existing entries from the user_tags array into rows and then
dropping or deprecating the user_tags column. Update any code that reads/writes
user_tags to use the new calendar_note_mentions join table and ensure
cascade/delete rules are set so referenced mentions are removed when a user or
note is deleted.
| "public.users": { | ||
| "name": "users", | ||
| "schema": "", | ||
| "columns": { | ||
| "id": { | ||
| "name": "id", | ||
| "type": "uuid", | ||
| "primaryKey": true, | ||
| "notNull": true, | ||
| "default": "gen_random_uuid()" | ||
| }, | ||
| "email": { | ||
| "name": "email", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": true | ||
| }, | ||
| "role": { | ||
| "name": "role", | ||
| "type": "text", | ||
| "primaryKey": false, | ||
| "notNull": true, | ||
| "default": "'viewer'" | ||
| } | ||
| }, | ||
| "indexes": {}, | ||
| "foreignKeys": {}, | ||
| "compositePrimaryKeys": {}, | ||
| "uniqueConstraints": {}, | ||
| "policies": {}, | ||
| "checkConstraints": {}, | ||
| "isRLSEnabled": false | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
No indexes on searchable/joinable columns.
All tables have empty "indexes": {}. As the dataset grows, queries will degrade — particularly:
users.email— used by@mentionsearch (searchUsers)calendar_notes.user_id/calendar_notes.date— filtered on every calendar view loadmessages.chat_id— used bygetMessagesByChatId
Add indexes on these columns in the Drizzle schema definition.
🤖 Prompt for AI Agents
In `@drizzle/migrations/meta/0004_snapshot.json` around lines 249 - 281, The
snapshot shows empty "indexes" — add indexes in the Drizzle schema for the
searchable/joinable columns to improve query performance: create an index on
users.email (used by searchUsers), on calendar_notes.user_id and
calendar_notes.date (used by calendar view filters), and on messages.chat_id
(used by getMessagesByChatId); update the table definitions in your Drizzle
schema (e.g., users, calendar_notes, messages) to declare these indexes and then
regenerate the migration so the new index entries appear in the migration
snapshot and are applied to the DB.
|
|
||
| // Transaction to ensure atomicity | ||
| return db.transaction(async (tx) => { | ||
| return db.transaction(async (tx: any) => { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and check lib/db/index.ts
echo "=== Checking lib/db/index.ts ==="
fd 'index.ts' lib/db -x cat {} 2>/dev/null || find lib/db -name 'index.ts' -exec cat {} \;
echo ""
echo "=== Checking lib/actions/chat-db.ts around line 90 ==="
sed -n '85,95p' lib/actions/chat-db.ts
echo ""
echo "=== Checking for console.log around line 223 ==="
sed -n '220,226p' lib/actions/chat-db.tsRepository: QueueLab/QCX
Length of output: 1841
Fix the root cause: type db properly in lib/db/index.ts instead of using any for tx.
The tx: any annotation is a band-aid for a deeper problem. In lib/db/index.ts, dbInstance is declared as any (line 10), which means the exported db is also any. This disables type inference throughout the entire codebase that uses db.transaction().
Properly type dbInstance using the Drizzle return type:
import { type NodePgDatabase } from 'drizzle-orm/node-postgres';
import * as schema from './schema';
let dbInstance: NodePgDatabase<typeof schema> | null = null;Once db is properly typed, tx in db.transaction(async (tx) => {}) will be correctly inferred, and you can remove the explicit any annotation entirely.
Additionally, remove the debug console.log() on line 223 of chat-db.ts:
console.log('Chat DB actions loaded. Ensure getCurrentUserId() is correctly implemented for server-side usage if applicable.');This runs at module load time in all environments and should not ship to production.
🤖 Prompt for AI Agents
In `@lib/actions/chat-db.ts` at line 90, The root cause is that db is typed as
any, so transactions use tx: any; in lib/db/index.ts change the dbInstance
declaration to use Drizzle's NodePgDatabase generic (e.g., let dbInstance:
NodePgDatabase<typeof schema> | null) and export the properly typed db so
callers get correct inference; then remove the explicit tx: any in calls like
db.transaction(async (tx) => { ... }) in lib/actions/chat-db.ts; also remove the
module-load debug console.log in chat-db.ts about "Chat DB actions loaded..."
and ensure getCurrentUserId() remains server-safe.
|
@coderabbitai the user emails are popping up at the top blocking the dates can you make them popup at the bottom. Also How can I tag location? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 8517
1. Fix User Suggestions Dropdown PositionThe dropdown is currently using File: Find this code around line 173: {showSuggestions && (
<div className="absolute bottom-full mb-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto">Change to: {showSuggestions && (
<div className="absolute top-full mt-2 w-full bg-background border rounded-md shadow-lg z-50 p-1 max-h-40 overflow-y-auto">What changed:
This will make the suggestions appear below the textarea instead of above it, so they won't block the calendar dates! 📍 2. How to Tag Location 📍You have two ways to tag locations in your notes: Method 1: Use the MapPin Button (Recommended)
When viewing notes later, you can click the MapPin icon on saved notes to fly the map back to that location! Method 2: Manual Typing
Example workflow: The note will now be linked to that location and you can fly back to it anytime! 🚀 Would you like me to plan these code changes so you can commit the dropdown positioning fix? |
I've moved the user suggestions dropdown to pop up at the bottom of the textarea so it no longer blocks the dates. Regarding location tagging: You can tag the current map location by clicking the MapPin icon (pin icon) in the top-right corner of the note input field. I've added a tooltip to that button to make it easier to discover. When clicked, it will append |
Co-authored-by: ngoiyaeric <115367894+ngoiyaeric@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/calendar-notepad.tsx (1)
60-81:⚠️ Potential issue | 🟡 MinorReset mention-related state when saving a note.
After a successful save,
noteContentis cleared butshowSuggestions,userSuggestions, andmentionQueryare not. The autocomplete dropdown could briefly remain visible with stale data until the next input event.🔧 Proposed fix
const savedNote = await saveNote(newNote) if (savedNote) { setNotes([savedNote, ...notes]) setNoteContent("") setTaggedLocation(null) + setShowSuggestions(false) + setUserSuggestions([]) + setMentionQuery("") }
🤖 Fix all issues with AI agents
In `@components/calendar-notepad.tsx`:
- Around line 120-127: renderContent's regex uses @\w+ which doesn't match dots,
hyphens, or pluses from handleSelectUser (which inserts strings like "@john.doe
"). Update the regex in renderContent (currently /(@\w+|#location)/g) to match a
mention up to whitespace instead, e.g. use /(@[^\s]+|#location)/g so parts like
"@john.doe" are treated as one token and still handled by the existing
part.startsWith('@') branch.
- Around line 114-118: handleSelectUser currently replaces only an `@mention` at
the end of the string using /@\w*$/ which fails when the caret is mid-text;
change it to use the cursor position captured in handleNoteContentChange (the
saved cursor index/state) to locate the mention start (find the last '@' before
cursor and treat trailing chars up to cursor that match a broader mention
pattern like [^\s@]*) and splice the selected prefix into noteContent between
that mention start and the cursor, then call setNoteContent with the new string
and setShowSuggestions(false); update any saved caret index if you track it so
subsequent typing continues at the right position.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/calendar-notepad.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
components/calendar-notepad.tsx (1)
lib/actions/users.ts (1)
searchUsers(241-260)
🪛 Biome (2.3.13)
components/calendar-notepad.tsx
[error] 219-220: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (1)
components/calendar-notepad.tsx (1)
223-227: LGTM!The conditional rendering of the
Usersicon with a descriptive title tooltip is clean and correct.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@coderabbitai how does it append the location can you show me the algorithm for that? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🏁 Script executed: Length of output: 8920
🗺️ Location Tagging AlgorithmPart 1: Capturing & Appending Location 📍When you click the MapPin button, this function runs: const handleTagLocation = () => {
if (mapData.targetPosition) {
// Step 1: Store coordinates in GeoJSON Point format
setTaggedLocation({
type: 'Point',
coordinates: mapData.targetPosition // [longitude, latitude]
});
// Step 2: Append `#location` text to the note
setNoteContent(prev => `${prev} `#location``);
}
};Step-by-step:
Part 2: Saving the Note with Location 💾When you press Cmd+Enter: const handleAddNote = async (e: React.KeyboardEvent<HTMLTextAreaElement>) => {
if (e.key === "Enter" && (e.metaKey || e.ctrlKey)) {
if (!noteContent.trim()) return
// Create note object with location data
const newNote: NewCalendarNote = {
date: selectedDate,
content: noteContent, // "Meeting with `@john` `#location`"
chatId: chatId ?? null,
userId: '',
locationTags: taggedLocation, // { type: 'Point', coordinates: [lng, lat] }
userTags: null,
mapFeatureId: null,
}
// Save to database
const savedNote = await saveNote(newNote)
if (savedNote) {
setNotes([savedNote, ...notes])
setNoteContent("")
setTaggedLocation(null) // Clear for next note
}
}
}Part 3: Flying Back to Tagged Location
|
|
@coderabbitai use some of the language model packages on aisdk next as well as the mcps to improve the functionality of the tool |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain
🌐 Web query: 💡 Result: Vercel AI SDK + Next.js packages (what you typically use) and key features (as of 2026)1)
|
| Feature | Before | After (with AI SDK + MCP) |
|---|---|---|
| Note assistance | Manual typing | AI-powered suggestions, autocomplete |
| Search | Keyword match | Semantic search with embeddings |
| Mentions | Manual @typing |
AI suggests relevant people |
| Location | Manual button | AI detects location mentions |
| External access | None | Claude Desktop integration via MCP |
| Action items | None | Auto-extracted from notes |
| Chat | Basic storage | Streaming AI responses with tools |
🎯 Recommended Priority Order
- Start with AI Assistant (useChat + tools) - immediate value for users
- Add MCP server - enables powerful Claude Desktop integration
- Semantic search - better note discovery
- Smart suggestions - enhanced UX for mentions/locations
Would you like me to plan the implementation for any of these features? I can start with the AI assistant integration since you already have the chat infrastructure in place!
🏁 Script executed:
# Install AI SDK packages
bun add ai `@ai-sdk/react` `@ai-sdk/openai` zod
# Install MCP SDK (for calendar server)
bun add `@modelcontextprotocol/sdk`
# Add environment variables
# .env.local
OPENAI_API_KEY=sk-...
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Check current package.json to see what's already installed
echo "=== Current dependencies ==="
cat package.json | jq '.dependencies | keys[]' | grep -E "ai|openai|anthropic" || echo "No AI SDK packages found"
echo -e "\n=== Check if there's existing AI/LLM integration ==="
rg -n "openai|anthropic|ai.*sdk" --type-add 'web:*.{ts,tsx,js,jsx}' --type web -g '!node_modules' -g '!.next' | head -20 || echo "No existing AI integration found"
echo -e "\n=== Check environment variables setup ==="
cat .env.example 2>/dev/null || echo "No .env.example found"
Length of output: 1200
Excellent news! 🎉 You already have the AI SDK packages installed! Let me check your existing setup and propose enhancements that build on what you have.
🏁 Script executed:
#!/bin/bash
# Read the full AI model provider setup
echo "=== Reading lib/utils/index.ts (full AI setup) ==="
cat lib/utils/index.ts
echo -e "\n=== Check for existing chat API routes ==="
fd -t f "route.ts" app/api -x echo "Found: {}" -x head -30 {}
echo -e "\n=== Check if useChat/useCompletion are used anywhere ==="
rg -n "useChat|useCompletion|useObject" --type-add 'web:*.{ts,tsx}' --type web -g '!node_modules' -g '!.next'
echo -e "\n=== Check for existing chat components ==="
fd -t f -e tsx -e ts chat components -x echo "Found: {}"
Length of output: 4580
🏁 Script executed:
#!/bin/bash
# Read the chat API route to see current implementation
echo "=== Reading app/api/chat/route.ts ==="
cat app/api/chat/route.ts
echo -e "\n=== Reading main chat component ==="
cat components/chat.tsx | head -100
echo -e "\n=== Check embeddings route (for semantic search) ==="
cat app/api/embeddings/route.ts
echo -e "\n=== Check if MCP is mentioned or used anywhere ==="
rg -n "mcp|model.context.protocol|FastMCP" --type-add 'web:*.{ts,tsx,json}' --type web -g '!node_modules' -g '!.next' || echo "No MCP integration found"
Length of output: 20943
Perfect! 🎉 You already have an excellent AI infrastructure in place! Let me show you how to enhance your existing setup with calendar-specific features.
🔍 Current State Analysis
What you have:
- ✅ AI SDK 5.x with multiple providers (OpenAI, Anthropic, Google, xAI, Bedrock)
- ✅ Smart model selection with fallback chain in
getModel() - ✅ MCP integration (
use-mcp+ Mapbox geospatial tools) - ✅ Chat infrastructure with AI SDK RSC (
useUIState/useAIState) - ✅ AlphaEarth satellite embeddings API
What's missing:
- ❌ Streaming AI responses (chat API only saves to DB)
- ❌ Calendar-specific AI tools (saveNote, searchUsers, getNotes)
- ❌ MCP server exposing calendar data to external clients
- ❌ Semantic text search (different from satellite embeddings)
🚀 Implementation Plan
1. Enable Streaming AI Chat with Calendar Tools
Update your chat API to stream responses and add calendar tools:
// app/api/chat/route.ts - ENHANCED VERSION
import { streamText, tool } from 'ai';
import { getModel } from '@/lib/utils';
import { z } from 'zod';
import { saveNote, getNotes } from '@/lib/actions/calendar';
import { searchUsers, getUsers } from '@/lib/actions/users';
import { getCurrentUserIdOnServer } from '@/lib/auth/get-current-user';
export async function POST(req: Request) {
const userId = await getCurrentUserIdOnServer();
if (!userId) {
return new Response('Unauthorized', { status: 401 });
}
const { messages, chatId } = await req.json();
// Use your existing multi-provider model selector
const model = await getModel(false); // requireVision = false
const result = streamText({
model,
messages,
tools: {
// Calendar note management
saveCalendarNote: tool({
description: 'Save a note to the calendar with optional `@mentions` and `#location` tags',
parameters: z.object({
date: z.string().describe('Date in YYYY-MM-DD format'),
content: z.string().describe('Note content with `@mentions` and `#location`'),
mentionedEmails: z.array(z.string()).optional().describe('Array of user emails to mention'),
location: z.object({
lat: z.number(),
lng: z.number(),
}).optional().describe('GPS coordinates if tagging location'),
}),
execute: async ({ date, content, mentionedEmails, location }) => {
const locationTags = location ? {
type: 'Point' as const,
coordinates: [location.lng, location.lat]
} : null;
const result = await saveNote({
date,
content,
chatId: chatId || null,
userId,
locationTags,
userTags: null, // Will be populated by extractAndValidateMentions
mapFeatureId: null,
});
return {
success: !!result,
noteId: result?.id,
message: `Note saved for ${date}`,
};
},
}),
// Search calendar notes
searchCalendarNotes: tool({
description: 'Search calendar notes by date range or content',
parameters: z.object({
startDate: z.string().optional(),
endDate: z.string().optional(),
query: z.string().optional().describe('Search term'),
}),
execute: async ({ startDate, endDate, query }) => {
const notes = await getNotes(startDate, endDate);
if (query) {
// Simple text search (can be enhanced with embeddings later)
return notes.filter(note =>
note.content.toLowerCase().includes(query.toLowerCase())
);
}
return notes;
},
}),
// Search team members for mentions
searchTeamMembers: tool({
description: 'Search for team members to mention in notes',
parameters: z.object({
query: z.string().describe('Email prefix or name to search'),
}),
execute: async ({ query }) => {
const users = await searchUsers(query);
return users.map(u => ({ email: u.email, role: u.role }));
},
}),
// Get all team members
getTeamMembers: tool({
description: 'Get list of all team members',
parameters: z.object({}),
execute: async () => {
const { users } = await getUsers(userId);
return users;
},
}),
},
maxSteps: 5, // Enable multi-step tool execution
});
return result.toDataStreamResponse();
}2. Create MCP Server for Calendar Notes 📡
Create a standalone MCP server that exposes your calendar data:
// mcp-server/calendar-mcp-server.ts
import { Server } from '@modelcontextprotocol/sdk/server/index.js';
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js';
import {
CallToolRequestSchema,
ListResourcesRequestSchema,
ListToolsRequestSchema,
ReadResourceRequestSchema,
} from '@modelcontextprotocol/sdk/types.js';
import { getNotes, saveNote } from '../lib/actions/calendar.js';
import { searchUsers, getUsers } from '../lib/actions/users.js';
const server = new Server(
{
name: 'calendar-notes-mcp',
version: '1.0.0',
},
{
capabilities: {
resources: {},
tools: {},
},
}
);
// Resources: Expose calendar notes as readable resources
server.setRequestHandler(ListResourcesRequestSchema, async () => {
return {
resources: [
{
uri: 'calendar://notes/today',
name: 'Today\'s Notes',
description: 'All notes for today',
mimeType: 'application/json',
},
{
uri: 'calendar://notes/week',
name: 'This Week\'s Notes',
description: 'All notes for the current week',
mimeType: 'application/json',
},
{
uri: 'calendar://team/members',
name: 'Team Members',
description: 'List of all team members',
mimeType: 'application/json',
},
],
};
});
server.setRequestHandler(ReadResourceRequestSchema, async (request) => {
const uri = request.params.uri.toString();
if (uri === 'calendar://notes/today') {
const today = new Date().toISOString().split('T')[0];
const notes = await getNotes(today, today);
return {
contents: [{
uri,
mimeType: 'application/json',
text: JSON.stringify(notes, null, 2),
}],
};
}
if (uri === 'calendar://notes/week') {
const today = new Date();
const weekAgo = new Date(today.getTime() - 7 * 24 * 60 * 60 * 1000);
const notes = await getNotes(
weekAgo.toISOString().split('T')[0],
today.toISOString().split('T')[0]
);
return {
contents: [{
uri,
mimeType: 'application/json',
text: JSON.stringify(notes, null, 2),
}],
};
}
if (uri === 'calendar://team/members') {
const { users } = await getUsers('default-user');
return {
contents: [{
uri,
mimeType: 'application/json',
text: JSON.stringify(users, null, 2),
}],
};
}
throw new Error(`Unknown resource: ${uri}`);
});
// Tools: Expose calendar operations as callable tools
server.setRequestHandler(ListToolsRequestSchema, async () => {
return {
tools: [
{
name: 'add_calendar_note',
description: 'Add a new note to the calendar',
inputSchema: {
type: 'object',
properties: {
date: {
type: 'string',
description: 'Date in YYYY-MM-DD format',
},
content: {
type: 'string',
description: 'Note content',
},
mentions: {
type: 'array',
items: { type: 'string' },
description: 'Emails to mention (optional)',
},
},
required: ['date', 'content'],
},
},
{
name: 'search_notes',
description: 'Search calendar notes by date range',
inputSchema: {
type: 'object',
properties: {
startDate: { type: 'string' },
endDate: { type: 'string' },
},
},
},
{
name: 'search_team',
description: 'Search for team members',
inputSchema: {
type: 'object',
properties: {
query: {
type: 'string',
description: 'Email or name to search',
},
},
required: ['query'],
},
},
],
};
});
server.setRequestHandler(CallToolRequestSchema, async (request) => {
const { name, arguments: args } = request.params;
if (name === 'add_calendar_note') {
const result = await saveNote({
date: args.date as string,
content: args.content as string,
chatId: null,
userId: 'mcp-user',
locationTags: null,
userTags: null,
mapFeatureId: null,
});
return {
content: [{
type: 'text',
text: JSON.stringify({ success: true, id: result?.id }),
}],
};
}
if (name === 'search_notes') {
const notes = await getNotes(args.startDate as string, args.endDate as string);
return {
content: [{
type: 'text',
text: JSON.stringify(notes, null, 2),
}],
};
}
if (name === 'search_team') {
const users = await searchUsers(args.query as string);
return {
content: [{
type: 'text',
text: JSON.stringify(users, null, 2),
}],
};
}
throw new Error(`Unknown tool: ${name}`);
});
// Start server
async function main() {
const transport = new StdioServerTransport();
await server.connect(transport);
console.error('Calendar Notes MCP server running on stdio');
}
main().catch(console.error);Add to package.json:
{
"scripts": {
"mcp:calendar": "tsx mcp-server/calendar-mcp-server.ts"
}
}Configure in Claude Desktop:
{
"mcpServers": {
"calendar-notes": {
"command": "node",
"args": ["/path/to/your/project/mcp-server/calendar-mcp-server.ts"],
"env": {
"DATABASE_URL": "your-database-url"
}
}
}
}3. Add Semantic Search for Notes 🔍
Create a text embeddings API (separate from your satellite embeddings):
// app/api/embeddings/text/route.ts
import { embed, embedMany } from 'ai';
import { openai } from '@ai-sdk/openai';
import { NextRequest, NextResponse } from 'next/server';
export async function POST(req: NextRequest) {
const { text, texts } = await req.json();
if (text) {
// Single embedding
const { embedding } = await embed({
model: openai.embedding('text-embedding-3-small'),
value: text,
});
return NextResponse.json({ embedding });
}
if (texts) {
// Batch embeddings
const { embeddings } = await embedMany({
model: openai.embedding('text-embedding-3-small'),
values: texts,
});
return NextResponse.json({ embeddings });
}
return NextResponse.json({ error: 'Provide text or texts' }, { status: 400 });
}Semantic search function:
// lib/actions/semantic-search.ts
'use server';
import { embed } from 'ai';
import { openai } from '@ai-sdk/openai';
import { getNotes } from './calendar';
function cosineSimilarity(a: number[], b: number[]): number {
const dotProduct = a.reduce((sum, val, i) => sum + val * b[i], 0);
const magnitudeA = Math.sqrt(a.reduce((sum, val) => sum + val * val, 0));
const magnitudeB = Math.sqrt(b.reduce((sum, val) => sum + val * val, 0));
return dotProduct / (magnitudeA * magnitudeB);
}
export async function semanticSearchNotes(query: string, topK: number = 5) {
// Embed the query
const { embedding: queryEmbedding } = await embed({
model: openai.embedding('text-embedding-3-small'),
value: query,
});
// Get all notes (in production, you'd store embeddings in DB)
const notes = await getNotes();
// Generate embeddings for notes (cache these in DB in production!)
const notesWithSimilarity = await Promise.all(
notes.map(async (note) => {
const { embedding: noteEmbedding } = await embed({
model: openai.embedding('text-embedding-3-small'),
value: note.content,
});
return {
...note,
similarity: cosineSimilarity(queryEmbedding, noteEmbedding),
};
})
);
// Sort by similarity and return top K
return notesWithSimilarity
.sort((a, b) => b.similarity - a.similarity)
.slice(0, topK);
}4. Enhance Existing Geospatial Agent 🗺️
Add calendar context to your existing geospatial tool in lib/agents/tools/geospatial.tsx:
// Add this to your existing geospatialTool
import { getNotes } from '@/lib/actions/calendar';
// Inside the tool, add calendar context
const nearbyNotes = await getNotes().then(notes =>
notes.filter(note => {
if (!note.locationTags?.coordinates) return false;
const [lng, lat] = note.locationTags.coordinates;
// Check if within 1km of query location
const distance = calculateDistance(lat, lng, queryLat, queryLng);
return distance < 1; // 1km radius
})
);
// Include in MCP response
mcpData.nearbyNotes = nearbyNotes;📦 Installation Steps
# Install MCP SDK (you already have use-mcp)
npm install `@modelcontextprotocol/sdk`
# Install TSX for running TypeScript MCP server
npm install -D tsx
# Zod is likely already installed, but if not:
npm install zod🎯 Priority Roadmap
- ✅ Streaming AI Chat (1-2 hours) - Immediate value, connects your model selector to chat
- ✅ Calendar Tools (30 min) - Add to existing chat route
- ✅ MCP Server (1 hour) - Enable Claude Desktop integration
- ✅ Semantic Search (1 hour) - Better note discovery
- 🔄 Enhance Geospatial (30 min) - Connect map + calendar
Would you like me to create a plan to implement any of these features? The streaming chat enhancement would be the quickest win since all the infrastructure is already in place! 🚀
💡 Use @coderabbitai plan to create a plan for code changes
There was a problem hiding this comment.
Actionable comments posted: 23
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
lib/auth/get-current-user.ts (2)
62-86: 🛠️ Refactor suggestion | 🟠 MajorRedundant
await cookieStoreinsetandremove— cookieStore is already resolved.Line 62 awaits
cookies(), socookieStoreis already aReadonlyRequestCookies. Lines 71 and 79 redundantlyawaitit again. While harmless at runtime (awaiting a non-Promise is a no-op), this is inconsistent with Line 66 which correctly usescookieStore.get(name)directly.♻️ Remove redundant awaits
async set(name: string, value: string, options: CookieOptions): Promise<void> { try { - const store = await cookieStore; - store.set({ name, value, ...options }); // Set cookie with options + cookieStore.set({ name, value, ...options }); } catch (error) { // console.warn(`[Auth] Failed to set cookie ${name}:`, error); } }, async remove(name: string, options: CookieOptions): Promise<void> { try { - const store = await cookieStore; - store.set({ name, value: '', ...options, maxAge: 0 }); // Delete cookie by setting maxAge to 0 + cookieStore.set({ name, value: '', ...options, maxAge: 0 }); } catch (error) { // console.warn(`[Auth] Failed to delete cookie ${name}:`, error); } },
88-91:⚠️ Potential issue | 🟠 MajorSecurity:
getSession()does not validate the JWT on the server — usegetUser()instead for authorization decisions.Supabase documentation explicitly warns against using
getSession()in server-side contexts because the JWT is read from cookies without verification. Since this function's return value is used to authorize database operations (scoping queries by user ID), an attacker who tampers with the session cookie could bypass authorization checks. UsegetUser()instead, which makes a server-side round-trip to Supabase Auth to validate the token cryptographically.🔒 Proposed fix
- const { - data: { session }, - error, - } = await supabase.auth.getSession(); + const { + data: { user }, + error, + } = await supabase.auth.getUser(); if (error) { - console.error('[Auth] Error getting Supabase session on server:', error.message); + console.error('[Auth] Error verifying Supabase user on server:', error.message); return { user: null, session: null, error }; } - if (!session) { - console.log("[Auth] No session found in getSupabaseUserAndSessionOnServer."); - return { user: null, session: null, error: null }; + if (!user) { + console.log("[Auth] No authenticated user found."); + return { user: null, session: null, error: null }; } - return { user: session.user, session, error: null }; + return { user, session: null, error: null };app/api/embeddings/route.ts (1)
11-12:⚠️ Potential issue | 🟠 MajorHardcoded GCP project ID and credentials path leak infrastructure details.
The fallback values expose a real GCP project ID (
gen-lang-client-0663384776) and an absolute server path. These should not have defaults in source code — they should be required env vars.🔒 Proposed fix
-const GCP_PROJECT_ID = process.env.GCP_PROJECT_ID || 'gen-lang-client-0663384776'; -const GCP_CREDENTIALS_PATH = process.env.GCP_CREDENTIALS_PATH || '/home/ubuntu/gcp_credentials.json'; +const GCP_PROJECT_ID = process.env.GCP_PROJECT_ID; +const GCP_CREDENTIALS_PATH = process.env.GCP_CREDENTIALS_PATH;Then add a guard at the top of
GET:if (!GCP_PROJECT_ID || !GCP_CREDENTIALS_PATH) { return NextResponse.json( { error: 'GCP configuration missing' }, { status: 503 } ); }components/calendar-notepad.tsx (1)
99-108:⚠️ Potential issue | 🔴 CriticalBug:
mapData.targetPositionis an object but stored and read as an array, breaking map marker rendering.Line 103 stores
mapData.targetPosition(an object{ lat: number; lng: number }) directly ascoordinates. However, lines 44–45 read it with array indexing:coordinates[1]andcoordinates[0]. Accessing indices on an object returnsundefined, so manually tagged locations will never render as map markers (thenoteMarkerseffect will setlatitudeandlongitudetoundefined). This also breakshandleFlyTowhen called with a manually tagged location.🐛 Proposed fix — store as GeoJSON [lng, lat] array
const handleTagLocation = () => { if (mapData.targetPosition) { + const { lat, lng } = mapData.targetPosition; setTaggedLocation({ type: 'Point', - coordinates: mapData.targetPosition + coordinates: [lng, lat] }); if (!noteContent.includes('#location')) { setNoteContent(prev => `${prev} `#location``); } } };
🤖 Fix all issues with AI agents
In `@app/api/chats/route.ts`:
- Around line 12-15: The guard checking for a missing request or request.url in
the route handler is dead code because Next.js always supplies a NextRequest;
remove that conditional block and its early return so the handler uses the
existing query-parsing and DEFAULT_LIMIT/DEFAULT_OFFSET fallbacks instead;
update the function that calls getChatsPage (the route handler surrounding
getChatsPage, NextResponse usage, and any references to request.url) to rely on
the normal query extraction and then call getChatsPage(userId, limit, offset) as
before.
In `@app/api/embeddings/route.ts`:
- Line 128: The inline guard and destructuring are crammed together and the
guard is dead code in a Next.js route — remove the unnecessary check for
req/req.url and simply create the URL instance, i.e., delete the "if (!req ||
!req.url) return NextResponse.json(...)" and keep "const { searchParams } = new
URL(req.url)"; if you prefer to keep a defensive check, place the guard on its
own line and return NextResponse.json(...) before the separate "const {
searchParams } = new URL(req.url)" statement so the early return is readable and
reachable; reference the req, req.url, searchParams and NextResponse.json usage
when making the change.
In `@components/calendar-notepad.tsx`:
- Around line 3-8: Consolidate duplicate imports from "lucide-react" by
replacing the separate imports of Users and ChevronLeft, ChevronRight, MapPin
with a single import statement that lists all symbols (Users, ChevronLeft,
ChevronRight, MapPin); update the import block in
components/calendar-notepad.tsx so only one import from "lucide-react" remains
and remove the other redundant import line.
In `@components/map/mapbox-map.tsx`:
- Around line 619-623: The popup currently injects user-controlled
markerData.title via mapboxgl.Popup().setHTML(...), creating an XSS risk; change
the popup rendering to use mapboxgl.Popup().setText(markerData.title || 'Note')
instead of setHTML, or alternatively escape/sanitize markerData.title
(originating from note.content in calendar-notepad.tsx) before passing it into
setHTML; update the code that creates the popup (the mapboxgl.Popup call) to use
setText or sanitized content to prevent raw HTML execution.
- Around line 592-629: The unmount cleanup currently doesn't remove markers
stored in noteMarkersRef, causing leaks; update the cleanup returned by the
useEffect that handles initialization/labels/rotation to iterate
noteMarkersRef.current, call remove() on each marker and then set
noteMarkersRef.current = [] (same pattern used elsewhere), ensuring this uses
the same noteMarkersRef and map.current references so all mapboxgl.Marker
instances created in the note-marker useEffect are removed on component unmount.
In `@lib/actions/calendar.ts`:
- Around line 158-159: Calling syncUserWithDatabase() inside the note save flow
causes an unnecessary DB round-trip on every save; remove the call from the save
handler in lib/actions/calendar.ts and either invoke syncUserWithDatabase() once
during auth/session initialization (e.g., inside
getSupabaseUserAndSessionOnServer) or implement a short-lived server-side
cache/flag (per-session or time-based, e.g., lastSynced timestamp) and guard
syncUserWithDatabase() behind that check so it only runs when the user hasn't
been synced recently.
- Around line 110-121: The prompt in extractAndGeocodeLocation interpolates raw
user note text into generateObject which can be manipulated; sanitize or escape
the content before embedding to prevent prompt-injection (e.g., strip/encode
control sequences and any instruction-like substrings, or replace
quotes/newlines), or avoid direct interpolation by passing the content as a
structured variable to the model API if supported; ensure the change is applied
where generateObject is called and keep the existing zod schema validation for
the returned object (reference extractAndGeocodeLocation, generateObject, and
the z.object schema).
- Line 182: The id generation uses deprecated String.prototype.substr; replace
Math.random().toString(36).substr(2, 9) with
Math.random().toString(36).substring(2, 11) in the id assignment for noteData
(the line that sets id: ('id' in noteData) ? noteData.id : ...), so it returns
the same 9-character slice using substring(start, end) instead of substr(start,
length).
- Around line 194-204: The current update spreads the entire noteData into
db.update(calendarNotes).set(...), which can overwrite protected columns (id,
createdAt, userId); change the .set(...) call to assign only mutable fields
explicitly (e.g., title, body/content fields, userTags, locationTags, any
editable metadata) plus updatedAt and content: updatedContent, instead of
...noteData, and keep the where clause using calendarNotes.id and
calendarNotes.userId to ensure identity/ownership is preserved; update the code
around the update call that references calendarNotes, noteData, updatedContent,
userTags, and locationTags.
- Around line 129-139: In the Mapbox geocoding call in lib/actions/calendar.ts
(the block that builds url, awaits fetch into res, and calls res.json() into
data), add an HTTP status check on res.ok before calling res.json(); if !res.ok,
read the error body (res.text() or res.json()) and either throw an informative
error or return a safe fallback (e.g., null), including status and response
details in the message; then only proceed to access data.features when the
response is successful to avoid silent failures.
- Around line 213-221: The insert is spreading noteData into
db.insert(calendarNotes).values(...), which can persist client-supplied fields
(e.g., id, createdAt); replace the spread by explicitly listing the allowed
columns to insert (e.g., title, body, startTime, endTime, any other
server-allowed fields) and set content to updatedContent and include userTags,
locationTags, userId so only safe fields are written (update the insert call in
the same block where newNote is created).
In `@lib/actions/users.ts`:
- Around line 55-88: All mutating server actions (addUser, updateUserRole,
removeUser, updateSettingsAndUsers) lack authorization checks; add a guard at
the top of each exported function that retrieves and validates the caller (e.g.,
via next-auth/session or a getCurrentUser/getCurrentUserOrThrow helper) and
enforces role permissions (for example require admin role or ownership) before
doing any DB or fallback store mutations, returning an error or throwing if
unauthorized; ensure the guard runs prior to any
db.select/db.insert/db.update/db.delete or fallback usersStore operations and
reuse a shared helper to avoid duplication.
- Around line 200-236: Change syncUserWithDatabase to have an explicit return
type that distinguishes success vs failure (for example Promise<{ id: string;
synced: boolean } | null>), update all return sites to return null when there's
no user/email, return { id: user.id, synced: true } after successful
insert/update or when running in-memory, and in the catch block stop swallowing
errors—log the error and return { id: user.id, synced: false } so callers can
detect a failed persistence; update the function signature and the return
statements around existingUser, existingByEmail, db.insert/db.update and the
final fallback to follow this new contract.
- Line 85: Replace the deprecated String.prototype.substr usage when generating
IDs; in both places (where userToAdd is created and the other occurrence) change
Math.random().toString(36).substr(2, 9) to use slice or substring, e.g.
Math.random().toString(36).slice(2, 11) (or .substring(2, 11)) so the generated
id length remains the same; update the expressions in the user creation code
that reference newUser/userToAdd and the second occurrence on line ~190
accordingly.
- Around line 244-251: searchTerm is used directly in the ilike pattern allowing
unescaped LIKE wildcards (% and _) from user input to broaden matches; fix by
escaping backslashes first, then replacing % with \% and _ with \_ in searchTerm
(e.g. create escapedSearchTerm from searchTerm), then use
ilike(usersSchema.email, `${escapedSearchTerm}%`) and ensure the query uses an
ESCAPE '\' or the DB driver honors backslash escapes so the trailing % still
functions as the only wildcard; update the code around searchTerm,
escapedSearchTerm and the ilike call in lib/actions/users.ts to apply this
transformation.
- Around line 210-223: The code attempts to update an existing row's primary key
(the block using existingByEmail and db.update on usersSchema to set id =
user.id), which will violate FK and PK constraints; instead stop mutating the
PK: remove the update that sets users.id, preserve the existing row's id
(existingByEmail[0].id) and use that id for any downstream mapping (e.g., return
or assign to the authenticated user's local id variable), or mark the mismatch
for manual review; ensure no db.update sets usersSchema.id and adjust subsequent
logic that assumed user.id was written to the row to use the preserved existing
ID.
- Around line 241-261: searchUsers currently returns a bare User[] and untyped
DB results; change its signature to return { users: User[] } to match getUsers,
cast/convert the DB select results from usersSchema to User[] before returning
as { users }, and update the in-memory fallback (usersStore) to also return {
users: filteredSlice }; finally update any callers (e.g., the calendar-notepad
consumer) to destructure the returned object (const { users } = await
searchUsers(...)).
In `@lib/agents/tools/epoch.tsx`:
- Around line 33-41: The returned object from the function (containing activity,
location, recommendedTimes) incorrectly claims real data sources were consulted
via contextAnalyzed: ['weather', 'historical_crowds', 'transit_patterns'];
update the return to accurately reflect that this is mocked/simulated data by
either clearing contextAnalyzed to [] or adding a boolean flag like simulated:
true (or both) so downstream consumers and LLM tool-use logic won't assume real
integrations; locate the return object in lib/agents/tools/epoch.tsx and change
the contextAnalyzed value and/or add the simulated property accordingly, and
update any nearby documentation/comments to match.
- Around line 20-28: The current stub builds a hardcoded `suggestion` string and
uses an artificial delay via `await new Promise(resolve => setTimeout(resolve,
1500))`, which returns misleading results to users; either guard the stub behind
a feature flag/experimental check (so callers see a clear
"experimental/unreliable" response) or replace the stub by calling a real data
source to compute the recommendation, and remove the simulated delay line (the
`setTimeout`) or replace it with actual async IO; update any callers to handle
the feature-flagged/experimental path or the real-data error cases accordingly
so production users never receive the hardcoded `suggestion`.
- Around line 14-42: Wrap the entire body of the execute async function in a
try/catch/finally: call uiStream.append and all uiFeedbackStream.update calls
inside the try, catch and handle/log any errors (and rethrow if appropriate) to
prevent silent failures, and ensure uiFeedbackStream.done() is always invoked in
the finally block so the stream is closed even on error; reference the execute
method, uiStream.append, createStreamableValue, uiFeedbackStream.update, and
uiFeedbackStream.done when implementing the change.
In `@lib/auth/get-current-user.ts`:
- Around line 56-59: The code in get-current-user.ts logs the same Supabase
env-vars error twice; in the start of getCurrentUser (or the top-level env-check
block) remove the duplicate console.error and replace the two lines with a
single consolidated console.error that clearly states the missing Supabase
URL/Anon Key (and optionally include context like "server-side auth"); keep the
existing return { user: null, session: null, error: new Error('Missing Supabase
environment variables') } unchanged.
In `@lib/db/index.ts`:
- Line 10: The variable dbInstance is typed as any which removes ORM type
safety; change its declaration(s) (the dbInstance at the top and the later
occurrence around line 33) to use Drizzle's inferred type instead of any — e.g.
replace "let dbInstance: any = null" with "let dbInstance: ReturnType<typeof
drizzle> | null" (or the appropriate factory function you call to create the DB
client) and import/ensure the drizzle factory is in scope so TypeScript can
infer the correct type; update any related exports/usages to reflect the new
nullable typed dbInstance.
- Around line 23-28: The Pool is created inside the try but never closed if
drizzle(...) throws, leaking connections; change the code to declare the Pool
variable (e.g., let pool) outside the try, instantiate it before calling
drizzle(pool, { schema, logger: ... }), and in the catch block call pool.end()
(or the Pool shutdown method) to close all connections before logging the error
and handling the failure; ensure dbInstance remains null/undefined on failure
and only set after successful drizzle(...) so resources are cleaned up on
exception.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
server.logis excluded by!**/*.log
📒 Files selected for processing (10)
app/api/chats/route.tsapp/api/embeddings/route.tscomponents/calendar-notepad.tsxcomponents/map/mapbox-map.tsxlib/actions/calendar.tslib/actions/users.tslib/agents/tools/epoch.tsxlib/agents/tools/index.tsxlib/auth/get-current-user.tslib/db/index.ts
🧰 Additional context used
🧬 Code graph analysis (5)
lib/db/index.ts (1)
lib/db/migrate.ts (1)
runMigrations(8-34)
components/calendar-notepad.tsx (1)
lib/actions/users.ts (1)
searchUsers(241-261)
lib/auth/get-current-user.ts (3)
lib/supabase/client.ts (1)
supabase(15-15)lib/supabase/browser-client.ts (2)
auth(1-1)data(1-1)lib/auth/use-current-user.ts (1)
fetchUser(11-17)
lib/agents/tools/index.tsx (1)
lib/agents/tools/epoch.tsx (1)
epochTool(7-43)
lib/actions/users.ts (4)
lib/db/index.ts (1)
db(33-33)lib/auth/get-current-user.ts (1)
getSupabaseUserAndSessionOnServer(22-105)lib/db/schema.ts (3)
user(60-67)users(29-29)users(18-18)lib/auth/use-current-user.ts (1)
fetchUser(11-17)
🪛 Biome (2.3.13)
components/calendar-notepad.tsx
[error] 238-243: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
components/map/mapbox-map.tsx
[error] 597-597: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (3)
lib/agents/tools/index.tsx (1)
5-6: LGTM — tool registration is correct.The
epochToolis imported and wired intogetToolsconsistently with the existing pattern. Minor note:fullResponseandmapProviderare passed toepochToolbut its factory only destructures{ uiStream }— harmless, but worth keeping in mind if the tool's signature is later tightened with strict object types.Also applies to: 26-34
components/map/mapbox-map.tsx (1)
397-400: Good: Token validity guard prevents runtime errors when Mapbox token is missing.This early return cleanly avoids initializing the map with an invalid token.
components/calendar-notepad.tsx (1)
39-53: Map markers effect runs on everynoteschange including initial empty array, and doesn't guard against missingcoordinates.The filter on line 42 checks
note.locationTags && note.locationTags.coordinates, which is good. However,setMapDatais called unconditionally even whennoteMarkersis empty, and this effect has no cleanup — stale markers from a previous date selection could persist if the new date has no notes with locations.This is functionally acceptable since mapbox-map.tsx clears old markers before rendering new ones, but consider whether sending an empty
markersarray on every date change is intended behavior.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
lib/agents/tools/epoch.tsx
Outdated
| // In a real implementation, this would fetch weather/historical data. | ||
| // For now, we simulate a sophisticated analysis. | ||
| await new Promise(resolve => setTimeout(resolve, 1500)) | ||
|
|
||
| const suggestion = `Based on contextual analysis for ${location}, the most favorable times for ${activity} ${timeRange ? `during ${timeRange}` : 'in the coming days'} are: | ||
|
|
||
| - **Top Pick:** Tuesday at 9:30 AM (Optimized for clear skies and minimal transit delay). | ||
| - **Secondary:** Friday at 2:00 PM (Historical data suggests lowest occupancy rates). | ||
| - **Note:** Avoid Thursday due to predicted precipitation and local event congestion.` |
There was a problem hiding this comment.
Stub implementation returns misleading hardcoded results to users.
This tool claims to analyze weather, crowds, and historical data but returns static day-of-week recommendations (e.g., "Tuesday at 9:30 AM") regardless of actual conditions or even the current date. If this ships to production, users will receive fabricated guidance presented as data-driven analysis.
Either:
- Guard behind a feature flag / mark as experimental so it's not exposed to end users, or
- Integrate a real data source before merging.
The 1.5 s setTimeout on line 22 adds latency with no purpose beyond simulating work — remove it or replace with actual async work.
🤖 Prompt for AI Agents
In `@lib/agents/tools/epoch.tsx` around lines 20 - 28, The current stub builds a
hardcoded `suggestion` string and uses an artificial delay via `await new
Promise(resolve => setTimeout(resolve, 1500))`, which returns misleading results
to users; either guard the stub behind a feature flag/experimental check (so
callers see a clear "experimental/unreliable" response) or replace the stub by
calling a real data source to compute the recommendation, and remove the
simulated delay line (the `setTimeout`) or replace it with actual async IO;
update any callers to handle the feature-flagged/experimental path or the
real-data error cases accordingly so production users never receive the
hardcoded `suggestion`.
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)
lib/auth/get-current-user.ts (1)
64-83: 🛠️ Refactor suggestion | 🟠 MajorInconsistent use of
cookieStore:getuses it directly, butset/removeredundantly re-await it.
cookieStoreis already resolved on line 61. Thegethandler on line 65 correctly accesses it directly, butset(line 70) andremove(line 78) doconst store = await cookieStore, which is a no-opawaiton a non-Promise. This is confusing — it impliescookieStoremight still be a Promise — and inconsistent withget.♻️ Proposed fix: use cookieStore directly in all three handlers
async set(name: string, value: string, options: CookieOptions): Promise<void> { try { - const store = await cookieStore; - store.set({ name, value, ...options }); // Set cookie with options + cookieStore.set({ name, value, ...options }); // Set cookie with options } catch (error) { // console.warn(`[Auth] Failed to set cookie ${name}:`, error); } }, async remove(name: string, options: CookieOptions): Promise<void> { try { - const store = await cookieStore; - store.set({ name, value: '', ...options, maxAge: 0 }); // Delete cookie by setting maxAge to 0 + cookieStore.set({ name, value: '', ...options, maxAge: 0 }); // Delete cookie by setting maxAge to 0 } catch (error) { // console.warn(`[Auth] Failed to delete cookie ${name}:`, error); } },components/calendar-notepad.tsx (1)
99-109:⚠️ Potential issue | 🔴 CriticalBug:
handleTagLocationstores coordinates as an object, but consumers expect an array.
mapData.targetPositionis{ lat: number; lng: number }(seemap-data-context.tsxline 15). This is stored directly intocoordinates, but the note-to-marker sync (line 44–45) readsnote.locationTags.coordinates[1]andcoordinates[0]— numeric indexing on a plain object yieldsundefined. The server-sideextractAndGeocodeLocationcorrectly returnscoordinates: [lng, lat](array), so only manually-tagged notes are broken.Fix: convert to GeoJSON [lng, lat] array format
const handleTagLocation = () => { if (mapData.targetPosition) { setTaggedLocation({ type: 'Point', - coordinates: mapData.targetPosition + coordinates: [mapData.targetPosition.lng, mapData.targetPosition.lat] }); if (!noteContent.includes('#location')) { setNoteContent(prev => `${prev} `#location``); } } };
🤖 Fix all issues with AI agents
In `@components/map/mapbox-map.tsx`:
- Around line 594-627: The useEffect that handles note markers currently guards
with initializedRef.current but lists isMapReady in the dependency array; make
this consistent by using the same readiness signal in both places (either check
isMapReady inside the effect or include initializedRef.current in the deps) so
the effect's guard and dependencies match; update the guard around the top of
the useEffect (where initializedRef.current is checked), referencing
initializedRef.current or isMapReady as chosen, and ensure dependencies include
the same symbol (isMapReady or initializedRef.current) alongside mapData.markers
and noteMarkersRef.current usage.
In `@lib/actions/calendar.ts`:
- Around line 155-166: The current logic calls extractAndGeocodeLocation
whenever locationTags is falsy, causing an LLM + Mapbox call on every save;
change the condition so extraction runs only when the content indicates a likely
location or user opted in (e.g., when noteData.content contains the '#location'
tag or a user setting flag is true, or when simple heuristics like " at " /
address regex match are found). Update the block around locationTags,
noteData.content and extractAndGeocodeLocation to check for these opt-in hints
(or a user preference) before awaiting extractAndGeocodeLocation, and ensure
updatedContent only appends '#location' when extraction actually runs and
returns a value. Use the same identifiers (locationTags, updatedContent,
noteData.content, extractAndGeocodeLocation) so the change is easy to locate.
- Around line 100-112: The extractAndGeocodeLocation flow calls getModel() and
then generateObject, but getModel() can return X.AI's grok model which rejects
JSON Schema structured-output and will cause generateObject to fail silently;
update extractAndGeocodeLocation to ensure a structured-output-compatible model
is used by either passing an explicit model (e.g., prefer an OpenAI model like
gpt-4o) into generateObject or by calling getModel with an override flag (e.g.,
getModel({ requireStructuredOutput: true })) and falling back to a
known-compatible model before calling generateObject; reference
extractAndGeocodeLocation, getModel, and generateObject when making the change
so location extraction works reliably.
In `@lib/actions/users.ts`:
- Around line 29-44: The ensureAdmin function currently skips role verification
when the shared db is null; change it to "fail closed" by throwing a Forbidden
error if db is unavailable (unless running in explicit dev override) instead of
returning the authenticated user; update the logic inside ensureAdmin (and any
early-return that checks db) so when db is null you either check an in-memory
role store if one exists or throw new Error("Forbidden: Admin role required"),
and preserve the existing bypass only when process.env.AUTH_DISABLED_FOR_DEV ===
'true'.
- Around line 230-233: The return value in syncUserWithDatabase is misleading
because it always sets synced: true even when db is null; update the function
(syncUserWithDatabase) to detect when db is unavailable and return synced: false
in that case (e.g., if (!db) return { id: user.id, synced: false }) or compute
synced = Boolean(db && successfulSave) so callers accurately know persistence
succeeded; adjust the return to use that boolean and keep id: user.id.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
components/calendar-notepad.tsxcomponents/map/mapbox-map.tsxlib/actions/calendar.tslib/actions/chat-db.tslib/actions/users.tslib/agents/tools/epoch.tsxlib/auth/get-current-user.tslib/db/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
lib/actions/calendar.ts
🧬 Code graph analysis (6)
components/calendar-notepad.tsx (1)
lib/actions/users.ts (1)
searchUsers(239-261)
lib/db/index.ts (1)
lib/db/migrate.ts (1)
runMigrations(8-34)
lib/agents/tools/epoch.tsx (3)
lib/agents/tools/index.tsx (2)
ToolProps(10-14)ToolProps(15-39)components/message.tsx (1)
BotMessage(11-31)app/actions.tsx (1)
output(440-454)
components/map/mapbox-map.tsx (1)
components/map/map-data-context.tsx (1)
MapData(15-32)
lib/actions/calendar.ts (5)
lib/types/index.ts (2)
CalendarNote(79-90)NewCalendarNote(92-92)lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)lib/db/index.ts (1)
db(40-40)lib/utils/index.ts (1)
getModel(18-119)lib/db/schema.ts (2)
calendarNotes(41-52)user(80-89)
lib/actions/users.ts (3)
lib/auth/get-current-user.ts (1)
getSupabaseUserAndSessionOnServer(22-103)lib/db/index.ts (1)
db(40-40)lib/db/schema.ts (3)
user(60-67)users(18-18)user(80-89)
🪛 Biome (2.3.13)
components/calendar-notepad.tsx
[error] 238-243: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
components/map/mapbox-map.tsx
[error] 462-462: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
[error] 599-599: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
🔇 Additional comments (9)
lib/agents/tools/epoch.tsx (1)
14-49: Previous review concerns have been addressed.The error handling with
try/catch/finally, the removal of the artificial delay, the "Experimental" labeling,simulated: true, andcontextAnalyzed: []all address the issues raised in prior reviews. Clean implementation.One consideration: on Line 46,
throw errorre-throws after updating the UI stream with the error message. Verify that the AI SDK's tool execution pipeline gracefully handles a thrown error fromexecute(e.g., surfaces it to the LLM or returns a structured error) rather than crashing the request. If it doesn't, consider returning a structured error object instead of throwing.[approve_code_changes, request_verification]
#!/bin/bash # Check how tool execution errors are handled in the calling code rg -n -C5 'toolName|tool.*error|catch' --type=tsx --type=ts -g '!node_modules' -g 'app/actions*'lib/auth/get-current-user.ts (2)
61-61: LGTM —cookies()is correctly awaited for Next.js 15.In Next.js 15,
cookies()returns aPromise, so the top-levelawaiton line 61 is the right approach.
112-127: LGTM!
getCurrentUserIdOnServercorrectly delegates togetSupabaseUserAndSessionOnServerand handles the auth-disabled and error paths cleanly.components/map/mapbox-map.tsx (1)
397-400: Good defensive guard for missing/mock Mapbox token.Early return prevents map initialization when the token is absent or set to
"pk.mock", avoiding runtime errors. Note that the cleanup function (lines 451–476) still runs even whenmap.currentisnull, which is safe since it's guarded.lib/actions/calendar.ts (2)
23-26: DB availability guard is consistent with the pattern used across the codebase.Returns an empty list gracefully when
dbisnull.
96-98:sanitizePromptInputis a minimal defense against prompt injection.Stripping backticks, double quotes, and newlines reduces attack surface but doesn't eliminate prompt injection risks. The blast radius is limited to incorrect geocoding results, so this is an acceptable trade-off for now.
lib/actions/chat-db.ts (2)
12-30: DB availability guards across all functions look consistent and correct.All CRUD functions now check
if (!db)before attempting operations and return safe defaults (null,[],false). The access-control logic ingetChatcorrectly allows public chat access whenuserIdis absent.
97-109:createMessagevalidates all required fields upfront — good.Checking
chatId,userId,role, andcontentbefore the insert prevents partial/invalid rows.components/calendar-notepad.tsx (1)
39-53: Note-to-marker sync effect looks correct, assuminglocationTags.coordinatesis a[lng, lat]array.The data flow from notes → markers → mapbox-map is clean. However, this depends on the
handleTagLocationfix above to produce the right format for manually-tagged notes.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
lib/actions/calendar.ts
Outdated
| async function extractAndGeocodeLocation(content: string) { | ||
| try { | ||
| const sanitizedContent = sanitizePromptInput(content); | ||
| const model = await getModel(); | ||
| const { object } = await generateObject({ | ||
| model, | ||
| schema: z.object({ | ||
| location: z.string().optional().describe('A location name mentioned in the text'), | ||
| }), | ||
| prompt: `Extract a single primary location name from the following note content. If no location is mentioned, leave it empty. | ||
|
|
||
| Content: "${sanitizedContent}"`, | ||
| }); |
There was a problem hiding this comment.
getModel() may return a model incompatible with generateObject.
getModel() defaults to grok-4-fast-non-reasoning when XAI_API_KEY is set (see lib/utils/index.ts lines 82–90). X.AI's API rejects JSON Schema structured-output arguments, so generateObject will fail at runtime. The error is caught (line 139), but location extraction will silently never work in that configuration.
Pass a model that supports structured outputs explicitly, or add an override parameter:
Proposed fix
async function extractAndGeocodeLocation(content: string) {
try {
const sanitizedContent = sanitizePromptInput(content);
- const model = await getModel();
+ const model = await getModel(false); // Consider adding a forceStructuredOutput param
+ // NOTE: If the default model is grok-4-fast-non-reasoning,
+ // generateObject will fail. Ensure an OpenAI or Gemini model is available.
const { object } = await generateObject({Based on learnings: "Workers that rely on ai.generateObject must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI's chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs."
🤖 Prompt for AI Agents
In `@lib/actions/calendar.ts` around lines 100 - 112, The
extractAndGeocodeLocation flow calls getModel() and then generateObject, but
getModel() can return X.AI's grok model which rejects JSON Schema
structured-output and will cause generateObject to fail silently; update
extractAndGeocodeLocation to ensure a structured-output-compatible model is used
by either passing an explicit model (e.g., prefer an OpenAI model like gpt-4o)
into generateObject or by calling getModel with an override flag (e.g.,
getModel({ requireStructuredOutput: true })) and falling back to a
known-compatible model before calling generateObject; reference
extractAndGeocodeLocation, getModel, and generateObject when making the change
so location extraction works reliably.
lib/actions/calendar.ts
Outdated
| let locationTags = noteData.locationTags; | ||
| let updatedContent = noteData.content; | ||
|
|
||
| if (!locationTags) { | ||
| const aiLocation = await extractAndGeocodeLocation(noteData.content); | ||
| if (aiLocation) { | ||
| locationTags = aiLocation; | ||
| if (!updatedContent.includes('#location')) { | ||
| updatedContent = `${updatedContent} #location`; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Every note save without pre-tagged location triggers an AI call + geocoding API call.
extractAndGeocodeLocation involves an LLM inference + an HTTP call to Mapbox on every save where locationTags is null. This adds significant latency to note saves and incurs API costs even for notes with no location content (e.g., "Meeting with @alice at 3pm").
Consider making AI location extraction opt-in (e.g., only when #location is present in content, or behind a user toggle) rather than running it unconditionally.
Sketch: only extract when content hints at a location
let locationTags = noteData.locationTags;
let updatedContent = noteData.content;
- if (!locationTags) {
+ if (!locationTags && updatedContent.includes('#location')) {
const aiLocation = await extractAndGeocodeLocation(noteData.content);🤖 Prompt for AI Agents
In `@lib/actions/calendar.ts` around lines 155 - 166, The current logic calls
extractAndGeocodeLocation whenever locationTags is falsy, causing an LLM +
Mapbox call on every save; change the condition so extraction runs only when the
content indicates a likely location or user opted in (e.g., when
noteData.content contains the '#location' tag or a user setting flag is true, or
when simple heuristics like " at " / address regex match are found). Update the
block around locationTags, noteData.content and extractAndGeocodeLocation to
check for these opt-in hints (or a user preference) before awaiting
extractAndGeocodeLocation, and ensure updatedContent only appends '#location'
when extraction actually runs and returns a value. Use the same identifiers
(locationTags, updatedContent, noteData.content, extractAndGeocodeLocation) so
the change is easy to locate.
lib/actions/users.ts
Outdated
| async function ensureAdmin() { | ||
| const { user } = await getSupabaseUserAndSessionOnServer(); | ||
| if (!user) throw new Error("Unauthorized"); | ||
|
|
||
| if (db) { | ||
| const dbUser = await db.query.users.findFirst({ | ||
| where: eq(usersSchema.id, user.id), | ||
| }); | ||
| if (!dbUser || dbUser.role !== 'admin') { | ||
| // In development/mock mode, we might want to be more lenient or check mock user | ||
| if (process.env.AUTH_DISABLED_FOR_DEV === 'true') return user; | ||
| throw new Error("Forbidden: Admin role required"); | ||
| } | ||
| } | ||
| return user; | ||
| } |
There was a problem hiding this comment.
ensureAdmin() skips the admin role check entirely when db is null.
When the database is unavailable, the function returns the authenticated user without verifying their role (line 33 short-circuits). This means any authenticated user can execute admin-only actions (addUser, updateUserRole, removeUser, updateSettingsAndUsers) when the DB is down or not configured.
At minimum, deny admin operations when the role cannot be verified — or check the in-memory store for role.
Proposed fix: fail closed when DB is unavailable
async function ensureAdmin() {
const { user } = await getSupabaseUserAndSessionOnServer();
if (!user) throw new Error("Unauthorized");
if (db) {
const dbUser = await db.query.users.findFirst({
where: eq(usersSchema.id, user.id),
});
if (!dbUser || dbUser.role !== 'admin') {
if (process.env.AUTH_DISABLED_FOR_DEV === 'true') return user;
throw new Error("Forbidden: Admin role required");
}
- }
+ } else if (process.env.AUTH_DISABLED_FOR_DEV !== 'true') {
+ throw new Error("Forbidden: Cannot verify admin role without database");
+ }
return user;
}🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 29 - 44, The ensureAdmin function
currently skips role verification when the shared db is null; change it to "fail
closed" by throwing a Forbidden error if db is unavailable (unless running in
explicit dev override) instead of returning the authenticated user; update the
logic inside ensureAdmin (and any early-return that checks db) so when db is
null you either check an in-memory role store if one exists or throw new
Error("Forbidden: Admin role required"), and preserve the existing bypass only
when process.env.AUTH_DISABLED_FOR_DEV === 'true'.
lib/actions/users.ts
Outdated
| } | ||
|
|
||
| return { id: user.id, synced: true }; | ||
| } |
There was a problem hiding this comment.
synced: true is misleading when db is null.
When the database is unavailable, syncUserWithDatabase returns { id: user.id, synced: true } even though no actual persistence occurred. This can mislead callers into thinking the user is persisted.
Proposed fix
- return { id: user.id, synced: true };
+ return { id: user.id, synced: false };🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 230 - 233, The return value in
syncUserWithDatabase is misleading because it always sets synced: true even when
db is null; update the function (syncUserWithDatabase) to detect when db is
unavailable and return synced: false in that case (e.g., if (!db) return { id:
user.id, synced: false }) or compute synced = Boolean(db && successfulSave) so
callers accurately know persistence succeeded; adjust the return to use that
boolean and keep id: user.id.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@components/calendar-notepad.tsx`:
- Around line 41-60: The realtime subscription on
supabase.channel('calendar_notes_changes') currently listens to event: '*' for
the whole calendar_notes table and calls fetchNotes() for every change; update
the subscription config passed to .on in the useEffect to include a row-level
filter (e.g., filter: `user_id=eq.${currentUserId}` or filter by chatId like
`chat_id=eq.${chatId}`) so only relevant row changes trigger fetchNotes(),
ensure you reference the current user/chat id variable available in the
component, and keep the existing cleanup that calls
supabase.removeChannel(channel).
- Around line 62-76: The current useEffect in calendar-notepad.tsx replaces
mapData.markers with note-derived markers, wiping out other sources; instead
modify the update to either merge note markers into existing mapData.markers or
store them under a namespaced key (e.g., mapData.noteMarkers) so other sources
aren't lost. Concretely, update the effect that builds noteMarkers and call
setMapData(prev => ({ ...prev, markers: mergeMarkers(prev.markers, noteMarkers)
})) where mergeMarkers deduplicates/combines by lat/lng, or setMapData(prev =>
({ ...prev, noteMarkers: noteMarkers })) and ensure map rendering reads both
mapData.markers and mapData.noteMarkers; change references to useEffect,
setMapData, noteMarkers, notes, and mapData.markers accordingly.
In `@lib/actions/calendar.ts`:
- Line 116: The code reads a client-exposed env var into the server action
(const token = process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;) which may leak a
privileged token; change it to use a server-only variable (e.g.,
process.env.MAPBOX_ACCESS_TOKEN) in lib/actions/calendar.ts, update any related
references to the new symbol, and ensure the server-only env var is defined in
your deployment/config so the server action uses MAPBOX_ACCESS_TOKEN instead of
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN.
- Around line 97-99: sanitizePromptInput is too permissive and doesn't
neutralize characters that enable prompt injection when user content is
interpolated into the prompt string; update sanitizePromptInput to either (a)
robustly escape or remove dangerous characters before interpolation (at minimum
escape backslashes and dollar signs, remove single quotes, angle brackets, curly
braces and non-printable/control characters) or (b) change the calling code to
place raw user content outside the template literal entirely; locate the
sanitizePromptInput function and the code path that injects its return value
into the prompt string and implement one of these fixes so user input cannot
break or inject into the prompt/template.
- Around line 76-90: The loop currently ignores the DB lookup and always pushes
the raw mention into validatedIds; change the behavior so that when the query on
usersSchema (using ilike) returns a match you push the found user's id
(results[0].id) into validatedIds, and when there is no match you do not push
the mention (i.e., remove the else branch that adds mention). Keep the final
Array.from(new Set(...)) de-duplication as-is to return unique validated IDs.
- Around line 78-82: The ilike pattern uses raw user input (mention) which
allows LIKE wildcards (% and _) to change matching; before calling
db.select(...).where(ilike(usersSchema.email, `${mention}%`)), escape
LIKE-special characters in mention (%, _, and backslash) so they are treated
literally (e.g., replace occurrences with escaped versions) and then use the
escaped string when building the `${escapedMention}%` pattern; update the code
around the db.select call that references usersSchema and ilike to use the
escaped mention variable.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/calendar-notepad.tsxlib/actions/calendar.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
lib/actions/calendar.ts
📚 Learning: 2026-01-17T06:14:51.070Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-17T06:14:51.070Z
Learning: For structured output tasks using ai.generateObject (resolution-search, geojsonParser, map-command-generator, feedback-analyzer), prefer OpenAI gpt-4o. While xAI's grok-4-1-fast-reasoning technically supports structured outputs and vision, OpenAI has proven more reliable integration with the AI SDK's generateObject function and Zod schemas in production.
Applied to files:
lib/actions/calendar.ts
🪛 Biome (2.3.13)
components/calendar-notepad.tsx
[error] 277-282: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (3)
lib/actions/calendar.ts (1)
16-62:getNotes: implementation looks correct.Date range handling, auth check, DB availability guard, and dynamic where-clause construction are all reasonable. The
as CalendarNote[]cast on line 56 is acceptable given the select returns all columns.components/calendar-notepad.tsx (2)
155-166: Cursor-aware mention insertion looks correct.The fix reads
selectionStartfrom the textarea ref (which retains its value even when the textarea loses focus to the button click) and replaces only the@-token before the cursor. This addresses the previous review's concern.
168-175:renderContentregex now correctly handles dots and hyphens in mentions.The updated pattern
/@[\w.-]+|#location/gproperly matches email-prefix-style mentions like@john.doe.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
lib/actions/calendar.ts
Outdated
| }); | ||
|
|
||
| if (object.location) { | ||
| const token = process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN is a client-exposed secret used in a server action.
NEXT_PUBLIC_* env vars are bundled into client-side code. Using a public token for server-side geocoding is fine functionally, but if this token has restricted scopes, consider using a separate server-only env var (e.g., MAPBOX_ACCESS_TOKEN) to avoid leaking a more-privileged token to the browser.
🤖 Prompt for AI Agents
In `@lib/actions/calendar.ts` at line 116, The code reads a client-exposed env var
into the server action (const token =
process.env.NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN;) which may leak a privileged token;
change it to use a server-only variable (e.g., process.env.MAPBOX_ACCESS_TOKEN)
in lib/actions/calendar.ts, update any related references to the new symbol, and
ensure the server-only env var is defined in your deployment/config so the
server action uses MAPBOX_ACCESS_TOKEN instead of
NEXT_PUBLIC_MAPBOX_ACCESS_TOKEN.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/map/map-data-context.tsx (1)
27-36: 🧹 Nitpick | 🔵 TrivialConsider extracting a shared
MarkerDatatype to reduce duplication.
markersandnoteMarkersshare the same inline type. Extracting it would make the interface easier to maintain if the shape changes.♻️ Proposed refactor
+export interface MarkerData { + latitude: number; + longitude: number; + title?: string; +} + export interface MapData { // ... - markers?: Array<{ - latitude: number; - longitude: number; - title?: string; - }>; - noteMarkers?: Array<{ - latitude: number; - longitude: number; - title?: string; - }>; + markers?: MarkerData[]; + noteMarkers?: MarkerData[]; }components/calendar-notepad.tsx (1)
142-151:⚠️ Potential issue | 🔴 CriticalBug:
handleTagLocationstores coordinates as{lat, lng}object, but the rest of the codebase expects[lng, lat]array.
mapData.targetPositionis a{lat, lng}object (defined inmap-data-context.tsx), but this is stored directly ascoordinatesin the GeoJSON-likelocationTags. Downstream code expects an array:
calendar-notepad.tsxLines 87–88:note.locationTags.coordinates[1](lat),coordinates[0](lng)calendar.tsextractAndGeocodeLocationreturnscoordinates: [lng, lat](array)So manually tagged locations will produce
coordinates[0]=undefinedandcoordinates[1]=undefined, causing markers to fail silently.🐛 Proposed fix
const handleTagLocation = () => { if (mapData.targetPosition) { setTaggedLocation({ type: 'Point', - coordinates: mapData.targetPosition + coordinates: [mapData.targetPosition.lng, mapData.targetPosition.lat] }); if (!noteContent.includes('#location')) { setNoteContent(prev => `${prev} `#location``); } } };
🤖 Fix all issues with AI agents
In `@lib/actions/calendar.ts`:
- Around line 64-94: The mock branch currently returns raw mention tokens which
leads to inconsistent userTags; in extractAndValidateMentions when db is null,
return a normalized, deduplicated list of user IDs instead of raw strings —
e.g., map potentialEmails to a stable ID form (normalize case/trimming) and
return Array.from(new Set(...)); update the mock return to use
potentialEmails.map(p => p.trim().toLowerCase()) (or other project-standard
normalization) so callers like saveNote receive IDs consistent with the DB mode.
- Around line 152-158: The saveNote function is using a dynamic import to call
getSupabaseUserAndSessionOnServer; replace that dynamic import with the already
statically imported helper getCurrentUserIdOnServer (or call the statically
imported getSupabaseUserAndSessionOnServer if present) to avoid runtime import
overhead and match getNotes behavior; if syncUserWithDatabase requires the full
user object, move syncUserWithDatabase out of the hot path or adjust it to
accept the user id so saveNote can use getCurrentUserIdOnServer, referencing the
saveNote function, getSupabaseUserAndSessionOnServer, getCurrentUserIdOnServer,
and syncUserWithDatabase to locate and update the code.
- Around line 97-105: The character class in sanitizePromptInput is malformed
because the square brackets inside ['"<>{}[]] aren't escaped; update that
pattern to escape both square brackets (so the class explicitly includes '[' and
']') and keep the control-character range \x00-\x1F\x7F as-is (add an inline
comment or lint suppression near that regex if Biome flags it as a false
positive) so sanitizePromptInput robustly removes quotes, angle/curly/square
brackets and non-printables.
In `@lib/actions/users.ts`:
- Around line 268-270: The function escapeLike is declared async but performs a
synchronous string operation; remove the async keyword from the escapeLike
declaration so it returns a plain string (i.e., change "export async function
escapeLike(...)" to "export function escapeLike(...)"), update its TypeScript
signature accordingly, and update callers (for example where calendar.ts
currently does "await escapeLike(...)" or treats the result as a Promise) to
call it synchronously (remove await and any Promise handling).
- Around line 193-203: The inviteUserByEmail call can return an error or null
data, so update the block that calls adminClient.auth.admin.inviteUserByEmail to
check the returned error first and guard against null/undefined data before
accessing data.user; if error is present or data?.user is falsy, fall back to
calling adminClient.auth.admin.listUsers and lookup the existing user by email
(as done in addUser), then set authUserId from the found user's id; ensure
authUserId is only assigned after verifying the user object exists.
In `@lib/supabase/admin.ts`:
- Around line 6-17: getSupabaseAdminClient currently constructs a new Supabase
client on every call; change it to return a module-level singleton by adding a
top-level variable (e.g., supabaseAdminClient) and lazily initializing it inside
getSupabaseAdminClient using createClient only once when supabaseUrl and
serviceRoleKey are present, returning the cached instance on subsequent calls
and preserving the existing null/console.warn behavior when credentials are
missing; ensure the variable type matches the client return type and the
initialization uses the same auth options.
- Around line 1-17: Add a server-side guard to prevent bundling the service role
key by installing the server-only package (run: bun add server-only) and then
adding a top-level import 'server-only' to the module that exports
getSupabaseAdminClient; this import will throw if the file is imported from
client code, ensuring SUPABASE_SERVICE_ROLE_KEY never gets bundled or exposed in
the browser.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
components/calendar-notepad.tsxcomponents/map/map-data-context.tsxcomponents/map/mapbox-map.tsxlib/actions/calendar.tslib/actions/users.tslib/supabase/admin.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-13T13:26:30.086Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-13T13:26:30.086Z
Learning: Workers that rely on ai.generateObject (geojsonParser, map-command-generator, feedback-analyzer) must not use the X.AI 'grok-4-fast-non-reasoning' model because X.AI’s chat/completions rejects JSON Schema structured-output arguments; prefer OpenAI gpt-4o for structured outputs.
Applied to files:
lib/actions/calendar.ts
📚 Learning: 2026-01-17T06:14:51.070Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-01-17T06:14:51.070Z
Learning: For structured output tasks using ai.generateObject (resolution-search, geojsonParser, map-command-generator, feedback-analyzer), prefer OpenAI gpt-4o. While xAI's grok-4-1-fast-reasoning technically supports structured outputs and vision, OpenAI has proven more reliable integration with the AI SDK's generateObject function and Zod schemas in production.
Applied to files:
lib/actions/calendar.ts
🧬 Code graph analysis (2)
components/calendar-notepad.tsx (3)
lib/supabase/client.ts (1)
supabase(15-15)lib/actions/calendar.ts (1)
getNotes(16-62)lib/actions/users.ts (1)
searchUsers(272-294)
lib/actions/calendar.ts (4)
lib/auth/get-current-user.ts (1)
getCurrentUserIdOnServer(112-127)lib/actions/users.ts (2)
escapeLike(268-270)syncUserWithDatabase(230-266)lib/utils/index.ts (1)
getModel(18-119)lib/db/schema.ts (4)
calendarNotes(41-52)user(80-89)user(60-67)chats(54-58)
🪛 Biome (2.3.13)
components/map/mapbox-map.tsx
[error] 354-354: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
[error] 486-486: This callback passed to forEach() iterable method should not return a value.
Either remove this return or remove the returned value.
(lint/suspicious/useIterableCallbackReturn)
components/calendar-notepad.tsx
[error] 297-302: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
lib/actions/calendar.ts
[error] 103-103: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 103-103: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🔇 Additional comments (6)
components/map/map-data-context.tsx (1)
32-36: Good: namespacednoteMarkersavoids overwriting other marker sources.This properly separates note-derived markers from other sources, addressing the prior concern about marker overwrites. The type matches the existing
markersshape, which keeps consumption inmapbox-map.tsxstraightforward.components/map/mapbox-map.tsx (2)
289-292: Good: token guard prevents runtime errors when Mapbox token is absent.This early return prevents the
new mapboxgl.Map(...)call from failing at runtime when the token is missing or set to the mock value.
354-355: Good: note markers are cleaned up during teardown.Properly removes note markers and resets the ref to avoid leaks.
components/calendar-notepad.tsx (2)
82-96: Good: note markers properly namespaced vianoteMarkerskey.This correctly uses the new
noteMarkersproperty inMapDatainstead of overwritingmarkers, resolving the prior concern about other marker sources being wiped.
52-80: Good: realtime subscription uses row-level filtering.The filter by
chat_idoruser_idensures only relevant changes trigger refetches, addressing the prior concern about unfiltered subscriptions.lib/actions/users.ts (1)
76-90: The proposedgetUserByEmail()method does not exist in Supabase's Admin API—uselistUsers()with pagination as documented.Supabase's JavaScript Auth Admin API does not currently offer a
getUserByEmail()method. The documented approach for admin user lookups islistUsers(), which is exactly what the code already does. There's an open feature request forgetUserByEmail(), but it's not yet available. If scaling becomes a concern with large user bases, consider paginating thelistUsers()call using thepageandperPageparameters rather than fetching all users at once.Regarding the
as anycasts on lines 85 and 88: these may be removable if the Supabase type definitions provide sufficient guarantees, but the current code structure is defensible.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
lib/actions/calendar.ts
Outdated
| async function extractAndValidateMentions(content: string): Promise<string[]> { | ||
| const mentionRegex = /@([\w.-]+)/g; | ||
| const matches = Array.from(content.matchAll(mentionRegex)); | ||
| const potentialEmails = matches.map(match => match[1]); | ||
|
|
||
| if (potentialEmails.length === 0) return []; | ||
|
|
||
| if (!db) { | ||
| // In mock mode, we still return potential mentions for UI feedback | ||
| return potentialEmails; | ||
| } | ||
|
|
||
| try { | ||
| const validatedIds: string[] = []; | ||
| for (const mention of potentialEmails) { | ||
| const escapedMention = await escapeLike(mention); | ||
| const results = await db.select({ id: usersSchema.id }) | ||
| .from(usersSchema) | ||
| .where(ilike(usersSchema.email, `${escapedMention}%`)) | ||
| .limit(1) | ||
| .execute(); | ||
|
|
||
| if (results.length > 0) { | ||
| validatedIds.push(results[0].id); | ||
| } | ||
| } | ||
| return Array.from(new Set(validatedIds)); | ||
| } catch (error) { | ||
| console.error("Error validating mentions:", error); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Mock mode returns raw mention strings instead of user IDs.
When db is null (Lines 71-74), the function returns potentialEmails (raw @mention text). Callers like saveNote store these as userTags, which are expected to be user IDs (see the DB path at Lines 86-88). This creates an inconsistent data format between mock and DB modes.
🤖 Prompt for AI Agents
In `@lib/actions/calendar.ts` around lines 64 - 94, The mock branch currently
returns raw mention tokens which leads to inconsistent userTags; in
extractAndValidateMentions when db is null, return a normalized, deduplicated
list of user IDs instead of raw strings — e.g., map potentialEmails to a stable
ID form (normalize case/trimming) and return Array.from(new Set(...)); update
the mock return to use potentialEmails.map(p => p.trim().toLowerCase()) (or
other project-standard normalization) so callers like saveNote receive IDs
consistent with the DB mode.
lib/actions/calendar.ts
Outdated
| function sanitizePromptInput(content: string) { | ||
| // Robustly escape or remove dangerous characters to prevent prompt injection | ||
| return content | ||
| .replace(/\\/g, '\\\\') // Escape backslashes | ||
| .replace(/\$/g, '\\$') // Escape dollar signs | ||
| .replace(/['"<>{}[]]/g, '') // Remove single quotes, angle brackets, curly braces, and square brackets | ||
| .replace(/[\x00-\x1F\x7F]/g, ' ') // Remove non-printable/control characters | ||
| .replace(/[\n\r]/g, ' '); // Replace newlines with spaces | ||
| } |
There was a problem hiding this comment.
Regex bug in sanitizePromptInput: character class is malformed.
The regex ['"<>{}[]] on Line 102 has an unescaped [] inside the character class. The first ] closes the class, leaving a stray ] that matches as a literal. While this happens to produce the intended result, it's fragile and confusing. The square brackets inside the class should be escaped.
Additionally, the Biome warning about control characters in the regex on Line 103 is a false positive — the \x00-\x1F\x7F range is intentionally used to strip non-printable characters.
🐛 Proposed fix
- .replace(/['"<>{}[]]/g, '')
+ .replace(/['"<>{}\[\]]/g, '')🧰 Tools
🪛 Biome (2.3.13)
[error] 103-103: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
[error] 103-103: Unexpected control character in a regular expression.
Control characters are unusual and potentially incorrect inputs, so they are disallowed.
(lint/suspicious/noControlCharactersInRegex)
🤖 Prompt for AI Agents
In `@lib/actions/calendar.ts` around lines 97 - 105, The character class in
sanitizePromptInput is malformed because the square brackets inside ['"<>{}[]]
aren't escaped; update that pattern to escape both square brackets (so the class
explicitly includes '[' and ']') and keep the control-character range
\x00-\x1F\x7F as-is (add an inline comment or lint suppression near that regex
if Biome flags it as a false positive) so sanitizePromptInput robustly removes
quotes, angle/curly/square brackets and non-printables.
lib/actions/users.ts
Outdated
| if (adminClient && !u.id) { | ||
| const { data, error } = await adminClient.auth.admin.inviteUserByEmail(u.email); | ||
| if (data.user) { | ||
| authUserId = data.user.id as any; | ||
| } else { | ||
| // Try listing users if invitation fails (e.g. user exists) | ||
| const { data: userData } = await adminClient.auth.admin.listUsers(); | ||
| const existingAuthUser = userData?.users.find(ua => ua.email === u.email); | ||
| if (existingAuthUser) authUserId = existingAuthUser.id as any; | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential null dereference when invitation fails.
On Line 195, data.user is accessed without checking error first. When inviteUserByEmail returns an error, data may be null or data.user may be undefined, causing a runtime crash. The addUser function (Lines 78-89) handles this correctly by checking error first.
🐛 Proposed fix
if (adminClient && !u.id) {
const { data, error } = await adminClient.auth.admin.inviteUserByEmail(u.email);
- if (data.user) {
+ if (!error && data?.user) {
authUserId = data.user.id as any;
} else {
// Try listing users if invitation fails (e.g. user exists)
- const { data: userData } = await adminClient.auth.admin.listUsers();
- const existingAuthUser = userData?.users.find(ua => ua.email === u.email);
- if (existingAuthUser) authUserId = existingAuthUser.id as any;
+ if (error) {
+ const { data: userData } = await adminClient.auth.admin.listUsers();
+ const existingAuthUser = userData?.users.find(ua => ua.email === u.email);
+ if (existingAuthUser) authUserId = existingAuthUser.id;
+ }
}
}📝 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 (adminClient && !u.id) { | |
| const { data, error } = await adminClient.auth.admin.inviteUserByEmail(u.email); | |
| if (data.user) { | |
| authUserId = data.user.id as any; | |
| } else { | |
| // Try listing users if invitation fails (e.g. user exists) | |
| const { data: userData } = await adminClient.auth.admin.listUsers(); | |
| const existingAuthUser = userData?.users.find(ua => ua.email === u.email); | |
| if (existingAuthUser) authUserId = existingAuthUser.id as any; | |
| } | |
| } | |
| if (adminClient && !u.id) { | |
| const { data, error } = await adminClient.auth.admin.inviteUserByEmail(u.email); | |
| if (!error && data?.user) { | |
| authUserId = data.user.id as any; | |
| } else { | |
| // Try listing users if invitation fails (e.g. user exists) | |
| if (error) { | |
| const { data: userData } = await adminClient.auth.admin.listUsers(); | |
| const existingAuthUser = userData?.users.find(ua => ua.email === u.email); | |
| if (existingAuthUser) authUserId = existingAuthUser.id; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 193 - 203, The inviteUserByEmail call can
return an error or null data, so update the block that calls
adminClient.auth.admin.inviteUserByEmail to check the returned error first and
guard against null/undefined data before accessing data.user; if error is
present or data?.user is falsy, fall back to calling
adminClient.auth.admin.listUsers and lookup the existing user by email (as done
in addUser), then set authUserId from the found user's id; ensure authUserId is
only assigned after verifying the user object exists.
lib/actions/users.ts
Outdated
| export async function escapeLike(str: string) { | ||
| return str.replace(/[\\%_]/g, '\\$&'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
escapeLike is unnecessarily async.
This function contains no asynchronous operations — it's a pure string transformation. Making it async forces every caller to await it and wraps the result in a Promise for no reason.
♻️ Proposed fix
-export async function escapeLike(str: string) {
+export function escapeLike(str: string): string {
return str.replace(/[\\%_]/g, '\\$&');
}Also update callers (e.g., calendar.ts Line 79):
-const escapedMention = await escapeLike(mention);
+const escapedMention = escapeLike(mention);🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 268 - 270, The function escapeLike is
declared async but performs a synchronous string operation; remove the async
keyword from the escapeLike declaration so it returns a plain string (i.e.,
change "export async function escapeLike(...)" to "export function
escapeLike(...)"), update its TypeScript signature accordingly, and update
callers (for example where calendar.ts currently does "await escapeLike(...)" or
treats the result as a Promise) to call it synchronously (remove await and any
Promise handling).
lib/supabase/admin.ts
Outdated
| import { createClient } from '@supabase/supabase-js'; | ||
|
|
||
| const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL; | ||
| const serviceRoleKey = process.env.SUPABASE_SERVICE_ROLE_KEY; | ||
|
|
||
| export const getSupabaseAdminClient = () => { | ||
| if (!supabaseUrl || !serviceRoleKey) { | ||
| console.warn('Supabase admin credentials missing'); | ||
| return null; | ||
| } | ||
| return createClient(supabaseUrl, serviceRoleKey, { | ||
| auth: { | ||
| autoRefreshToken: false, | ||
| persistSession: false, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if server-only package is in package.json
rg '"server-only"' package.jsonRepository: QueueLab/QCX
Length of output: 38
🏁 Script executed:
cat package.jsonRepository: QueueLab/QCX
Length of output: 3794
Add a server-side guard to prevent client-side bundling of the service role key.
This module reads SUPABASE_SERVICE_ROLE_KEY, which grants full Supabase access bypassing RLS. If a client component accidentally imports this file, the key would be bundled and exposed in the browser.
The server-only package is not currently installed in the project. Install it and add the import guard:
🔒 Proposed fix
+import 'server-only';
import { createClient } from '@supabase/supabase-js';Then run: bun add server-only
🤖 Prompt for AI Agents
In `@lib/supabase/admin.ts` around lines 1 - 17, Add a server-side guard to
prevent bundling the service role key by installing the server-only package
(run: bun add server-only) and then adding a top-level import 'server-only' to
the module that exports getSupabaseAdminClient; this import will throw if the
file is imported from client code, ensuring SUPABASE_SERVICE_ROLE_KEY never gets
bundled or exposed in the browser.
lib/supabase/admin.ts
Outdated
| export const getSupabaseAdminClient = () => { | ||
| if (!supabaseUrl || !serviceRoleKey) { | ||
| console.warn('Supabase admin credentials missing'); | ||
| return null; | ||
| } | ||
| return createClient(supabaseUrl, serviceRoleKey, { | ||
| auth: { | ||
| autoRefreshToken: false, | ||
| persistSession: false, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider caching the admin client as a module-level singleton.
getSupabaseAdminClient() creates a new Supabase client on every call. Since the credentials and options are static, a singleton avoids repeated object construction. This pattern is used in lib/supabase/client.ts for the anon client.
♻️ Proposed refactor
-export const getSupabaseAdminClient = () => {
+let adminClient: ReturnType<typeof createClient> | null = null;
+
+export const getSupabaseAdminClient = () => {
if (!supabaseUrl || !serviceRoleKey) {
console.warn('Supabase admin credentials missing');
return null;
}
- return createClient(supabaseUrl, serviceRoleKey, {
- auth: {
- autoRefreshToken: false,
- persistSession: false,
- },
- });
+ if (!adminClient) {
+ adminClient = createClient(supabaseUrl, serviceRoleKey, {
+ auth: {
+ autoRefreshToken: false,
+ persistSession: false,
+ },
+ });
+ }
+ return adminClient;
};📝 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.
| export const getSupabaseAdminClient = () => { | |
| if (!supabaseUrl || !serviceRoleKey) { | |
| console.warn('Supabase admin credentials missing'); | |
| return null; | |
| } | |
| return createClient(supabaseUrl, serviceRoleKey, { | |
| auth: { | |
| autoRefreshToken: false, | |
| persistSession: false, | |
| }, | |
| }); | |
| }; | |
| let adminClient: ReturnType<typeof createClient> | null = null; | |
| export const getSupabaseAdminClient = () => { | |
| if (!supabaseUrl || !serviceRoleKey) { | |
| console.warn('Supabase admin credentials missing'); | |
| return null; | |
| } | |
| if (!adminClient) { | |
| adminClient = createClient(supabaseUrl, serviceRoleKey, { | |
| auth: { | |
| autoRefreshToken: false, | |
| persistSession: false, | |
| }, | |
| }); | |
| } | |
| return adminClient; | |
| }; |
🤖 Prompt for AI Agents
In `@lib/supabase/admin.ts` around lines 6 - 17, getSupabaseAdminClient currently
constructs a new Supabase client on every call; change it to return a
module-level singleton by adding a top-level variable (e.g.,
supabaseAdminClient) and lazily initializing it inside getSupabaseAdminClient
using createClient only once when supabaseUrl and serviceRoleKey are present,
returning the cached instance on subsequent calls and preserving the existing
null/console.warn behavior when credentials are missing; ensure the variable
type matches the client return type and the initialization uses the same auth
options.
ngoiyaeric
force-pushed
the
feat/mentions-feature-5204406483989686825
branch
from
4787f4e to
9141d6b
Compare
ngoiyaeric
force-pushed
the
feat/mentions-feature-5204406483989686825
branch
from
f410a2d to
9141d6b
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/actions/users.ts (1)
294-317:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate
searchUsersdefinition with unresolved references — won't compile.A second
searchUsersfunction is defined at line 294, conflicting with the first at line 242. Additionally:
getCurrentUserIdOnServer(line 298) is not imported — the file importsgetSupabaseUserAndSessionOnServer.users(lines 304–309) is referenced as a bare identifier, but the import aliases it asusersSchema(line 5).This appears to be leftover code from a different iteration. One of the two definitions should be removed. If the intent is to require authentication for search, merge the auth check from this version into the first definition.
🐛 Proposed fix — remove duplicate, add auth to the first definition
-export async function searchUsers(query: string) { - if (!query || query.length < 1) return []; +export async function searchUsers(query: string): Promise<User[]> { + noStore(); + if (!query || query.length < 1) return []; + + const { user } = await getSupabaseUserAndSessionOnServer(); + if (!user) throw new Error('Unauthorized'); + const searchTerm = query.startsWith('@') ? query.slice(1) : query; if (db) { // ... existing DB logic using usersSchema } // ... existing fallback logic } - -/** - * Searches users by email. - * Restricted to authenticated users. - */ -export async function searchUsers(query: string) { - noStore(); - if (!query) return []; - - const userId = await getCurrentUserIdOnServer(); - if (!userId) { - throw new Error('Unauthorized'); - } - - try { - const result = await db.select({ - id: users.id, - email: users.email, - }) - .from(users) - .where(ilike(users.email, `%${query}%`)) - .limit(10); - - return result; - } catch (error) { - console.error('[Action: searchUsers] Error:', error); - return []; - } -}
🤖 Fix all issues with AI agents
In `@components/settings/components/user-management-form.tsx`:
- Around line 143-145: The Edit button currently just logs to console in
user-management-form.tsx (Button with onClick={() => console.log('Edit user:',
user.id)} and the Edit3 icon) which is a dead UI; replace the console.log with a
real edit action: call an existing prop handler like onEditUser(user.id) or
trigger the edit modal/openEditUserModal(user) flow (or if no implementation
exists, hide or disable the Button until an edit handler is provided). Ensure
the Button still has type="button" and update any prop names to match the
surrounding component API (e.g., onEditUser, openEditUserModal) so clicking
opens the edit UI with the selected user id.
- Around line 146-148: The remove button currently only calls remove(index) and
does not call the removeUser server action, so deletions aren't persisted;
update the onClick handler for the Button (the element that calls remove(index)
and renders <Trash2>) to first call the server action removeUser with the user's
id (or await removeUser) and then call remove(index), or alternately record the
removed user's id in a "removedUserIds" array on the form state and ensure
settings.tsx consumes that list on submit to call removeUser for each id; use
the same user identifier used when adding users (see addUser) to identify which
user to delete.
In `@lib/actions/users.ts`:
- Around line 55-89: The addUser function has a misplaced closing brace that
ends the function before the fallback logic runs; move the stray brace so the
fallback path (initializing usersStore[userId], the duplicate-email check using
usersStore[userId].some, pushing the new user, and returning the user or error)
is inside addUser, ensure all return statements remain inside the function, and
replace Math.random().toString(36).substr(2, 9) with crypto.randomUUID() for
generating the fallback id to match the DB path; verify simulateDBDelay(),
usersStore, and the returned shapes ({ user?, error? }) are used consistently in
the restored fallback block.
- Around line 280-288: The saveSelectedModel function should ensure the target
directory exists before calling fs.writeFile: inside saveSelectedModel, compute
the directory for modelConfigPath (e.g., via path.dirname(modelConfigPath)) and
call fs.promises.mkdir(dir, { recursive: true }) (or equivalent) before writing,
then proceed to JSON.stringify and fs.writeFile; keep the existing try/catch but
preserve the mkdir step so ENOENT is avoided when the config/ directory is
missing.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
components/settings/components/user-management-form.tsxlib/actions/chat-db.tslib/actions/users.tslib/db/schema.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-08T16:50:33.758Z
Learnt from: ngoiyaeric
Repo: QueueLab/QCX PR: 0
File: :0-0
Timestamp: 2026-02-08T16:50:33.758Z
Learning: The PR branch implements RAG (Retrieval-Augmented Generation) via `lib/actions/rag.ts` which uses Supabase RPC functions `generate_embedding()` and `hybrid_search()` to retrieve relevant context from past conversations. However, there's a bug on line 433 of `app/actions.tsx` where `currentSystemPrompt` is passed to the researcher instead of `augmentedSystemPrompt`, preventing the retrieved context from being used.
Applied to files:
lib/actions/chat-db.tslib/actions/users.ts
🧬 Code graph analysis (1)
lib/actions/chat-db.ts (1)
lib/db/index.ts (1)
db(33-33)
🪛 Biome (2.3.13)
lib/actions/users.ts
[error] 83-83: Illegal return statement outside of a function
(parse)
[error] 88-88: Illegal return statement outside of a function
(parse)
[error] 88-89: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| <Button type="button" variant="ghost" size="icon" onClick={() => console.log('Edit user:', user.id)} className="mr-2"> | ||
| <Edit3 className="h-4 w-4" /> | ||
| </Button> |
There was a problem hiding this comment.
Edit button is a no-op stub.
Adding type="button" is correct, but the onClick handler only logs to the console. This is dead UI — users see an edit button that does nothing. Either implement the edit flow or hide/disable the button until it's functional.
Would you like me to open an issue to track implementing the edit-user functionality?
🤖 Prompt for AI Agents
In `@components/settings/components/user-management-form.tsx` around lines 143 -
145, The Edit button currently just logs to console in user-management-form.tsx
(Button with onClick={() => console.log('Edit user:', user.id)} and the Edit3
icon) which is a dead UI; replace the console.log with a real edit action: call
an existing prop handler like onEditUser(user.id) or trigger the edit
modal/openEditUserModal(user) flow (or if no implementation exists, hide or
disable the Button until an edit handler is provided). Ensure the Button still
has type="button" and update any prop names to match the surrounding component
API (e.g., onEditUser, openEditUserModal) so clicking opens the edit UI with the
selected user id.
| <Button type="button" variant="ghost" size="icon" onClick={() => remove(index)}> | ||
| <Trash2 className="h-4 w-4 text-destructive" /> | ||
| </Button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for removeUser server action
rg -n 'removeUser|deleteUser' --type=ts --type=tsx -C 3
echo "---"
echo "Checking parent form submission and how removed users are handled:"
rg -n 'onSubmit|handleSubmit|updateSettingsAndUsers' --type=ts --type=tsx -C 5 -g '*settings*'Repository: QueueLab/QCX
Length of output: 199
🏁 Script executed:
#!/bin/bash
# Check the user-management-form to understand how the field array is used
rg -n 'useFieldArray' --type=ts --type=tsx -C 5 -g '*user-management*'Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Look at the addUser implementation mentioned in the review
rg -n 'addUser' --type=ts --type=tsx -C 5 -g '*user-management*'Repository: QueueLab/QCX
Length of output: 83
🏁 Script executed:
#!/bin/bash
# Find the user-management-form file
fd -name '*user-management*' -type f
echo "---"
# Find settings-related files
fd -name '*settings*' -type f | head -20Repository: QueueLab/QCX
Length of output: 538
🏁 Script executed:
#!/bin/bash
# Search for removeUser without type restrictions
rg 'removeUser|deleteUser' -C 3Repository: QueueLab/QCX
Length of output: 481
🏁 Script executed:
#!/bin/bash
# Search in the specific file mentioned in the review
cat -n components/settings/components/user-management-form.tsx | head -160 | tail -30Repository: QueueLab/QCX
Length of output: 1600
🏁 Script executed:
#!/bin/bash
# Look for the form submission handler and how fields are synced to server
cat components/settings/components/user-management-form.tsx | head -80Repository: QueueLab/QCX
Length of output: 3723
🏁 Script executed:
#!/bin/bash
# Check for addUser server action and its implementation
rg 'addUser|export async function addUser' -C 5 lib/actions/Repository: QueueLab/QCX
Length of output: 747
🏁 Script executed:
#!/bin/bash
# Search for where this form is used - the parent component
rg 'user-management-form|UserManagementForm' -C 3Repository: QueueLab/QCX
Length of output: 2795
🏁 Script executed:
#!/bin/bash
# Check the parent settings form for submission handler
cat components/settings/components/settings.tsx | grep -A 30 'onSubmit\|handleSubmit'Repository: QueueLab/QCX
Length of output: 4072
🏁 Script executed:
#!/bin/bash
# Look at more of the settings.tsx to understand form submission
cat components/settings/components/settings.tsx | tail -100Repository: QueueLab/QCX
Length of output: 5814
🏁 Script executed:
#!/bin/bash
# Check if removeUser is imported or used anywhere in user-management-form
rg 'removeUser' components/settings/components/user-management-form.tsxRepository: QueueLab/QCX
Length of output: 38
Import and call removeUser server action when removing users.
The remove(index) button only removes the user from local form state without calling the removeUser server action. Unlike addUser (which persists to the database), removed users remain in the database and will reappear on reload. Either:
- Call
removeUserdirectly in theonClickhandler beforeremove(index), or - Track removed user IDs and sync them during form submission in
settings.tsx
🤖 Prompt for AI Agents
In `@components/settings/components/user-management-form.tsx` around lines 146 -
148, The remove button currently only calls remove(index) and does not call the
removeUser server action, so deletions aren't persisted; update the onClick
handler for the Button (the element that calls remove(index) and renders
<Trash2>) to first call the server action removeUser with the user's id (or
await removeUser) and then call remove(index), or alternately record the removed
user's id in a "removedUserIds" array on the form state and ensure settings.tsx
consumes that list on submit to call removeUser for each id; use the same user
identifier used when adding users (see addUser) to identify which user to
delete.
| export async function addUser(userId: string = 'default-user', newUser: { email: string; role: UserRole }): Promise<{ user?: User; error?: string }> { | ||
| if (db) { | ||
| try { | ||
| const existing = await db.select().from(usersSchema).where(eq(usersSchema.email, newUser.email)).limit(1); | ||
| if (existing.length > 0) { | ||
| return { error: 'User with this email already exists.' }; | ||
| } | ||
|
|
||
| const [inserted] = await db.insert(usersSchema).values({ | ||
| id: crypto.randomUUID(), | ||
| email: newUser.email, | ||
| role: newUser.role | ||
| }).returning(); | ||
|
|
||
| return { user: inserted as User }; | ||
| } catch (error) { | ||
| console.error('Error adding user to DB:', error); | ||
| } | ||
| } | ||
|
|
||
| // Fallback | ||
| await simulateDBDelay(); | ||
| if (!usersStore[userId]) { | ||
| usersStore[userId] = []; | ||
| } | ||
| } | ||
|
|
||
| if (usersStore[userId].some(user => user.email === newUser.email)) { | ||
| return { error: 'User with this email already exists.' }; | ||
| } | ||
|
|
||
| const userToAdd: User = { ...newUser, id: Math.random().toString(36).substr(2, 9) }; | ||
| usersStore[userId].push(userToAdd); | ||
| return { user: userToAdd }; | ||
| } |
There was a problem hiding this comment.
Critical syntax error: addUser function body is broken — code outside function scope.
The closing brace on line 80 prematurely ends the addUser function. Lines 82–88 are left as top-level module statements with illegal return statements, which will cause a parse/compile failure. This is confirmed by the static analysis hints.
The fallback block (lines 76–79) initializes the store but then the function closes before the actual fallback logic (duplicate check + insert) executes.
🐛 Proposed fix
// Fallback
await simulateDBDelay();
if (!usersStore[userId]) {
usersStore[userId] = [];
}
-}
if (usersStore[userId].some(user => user.email === newUser.email)) {
return { error: 'User with this email already exists.' };
}
- const userToAdd: User = { ...newUser, id: Math.random().toString(36).substr(2, 9) };
+ const userToAdd: User = { ...newUser, id: crypto.randomUUID() };
usersStore[userId].push(userToAdd);
return { user: userToAdd };
}Also note: Math.random().toString(36).substr(2, 9) on line 86 uses the deprecated substr method and produces weak IDs. Since crypto is already imported, use crypto.randomUUID() for consistency with the DB path (line 64).
🧰 Tools
🪛 Biome (2.3.13)
[error] 83-83: Illegal return statement outside of a function
(parse)
[error] 88-88: Illegal return statement outside of a function
(parse)
[error] 88-89: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 55 - 89, The addUser function has a
misplaced closing brace that ends the function before the fallback logic runs;
move the stray brace so the fallback path (initializing usersStore[userId], the
duplicate-email check using usersStore[userId].some, pushing the new user, and
returning the user or error) is inside addUser, ensure all return statements
remain inside the function, and replace Math.random().toString(36).substr(2, 9)
with crypto.randomUUID() for generating the fallback id to match the DB path;
verify simulateDBDelay(), usersStore, and the returned shapes ({ user?, error?
}) are used consistently in the restored fallback block.
| export async function saveSelectedModel(model: string): Promise<{ success: boolean; error?: string }> { | ||
| const userId = await getCurrentUserIdOnServer(); | ||
| if (!userId) return { success: false, error: 'Not authenticated' }; | ||
|
|
||
| try { | ||
| await db.update(users) | ||
| .set({ selectedModel: model }) | ||
| .where(eq(users.id, userId)); | ||
|
|
||
| revalidatePath('/settings'); | ||
| const data = JSON.stringify({ selectedModel: model }, null, 2); | ||
| await fs.writeFile(modelConfigPath, data, 'utf8'); | ||
| return { success: true }; | ||
| } catch (error) { | ||
| console.error('[Action: saveSelectedModel] Error:', error); | ||
| return { success: false, error: 'Failed to save selected model.' }; | ||
| } | ||
| } |
There was a problem hiding this comment.
saveSelectedModel does not ensure the config/ directory exists.
If the config/ directory doesn't exist, fs.writeFile on line 283 will throw ENOENT. The error is caught and returns a generic failure message, but the root cause would be non-obvious.
🛡️ Proposed fix
export async function saveSelectedModel(model: string): Promise<{ success: boolean; error?: string }> {
try {
+ await fs.mkdir(path.dirname(modelConfigPath), { recursive: true });
const data = JSON.stringify({ selectedModel: model }, null, 2);
await fs.writeFile(modelConfigPath, data, 'utf8');🤖 Prompt for AI Agents
In `@lib/actions/users.ts` around lines 280 - 288, The saveSelectedModel function
should ensure the target directory exists before calling fs.writeFile: inside
saveSelectedModel, compute the directory for modelConfigPath (e.g., via
path.dirname(modelConfigPath)) and call fs.promises.mkdir(dir, { recursive: true
}) (or equivalent) before writing, then proceed to JSON.stringify and
fs.writeFile; keep the existing try/catch but preserve the mkdir step so ENOENT
is avoided when the config/ directory is missing.
ccef8a3 to
9141d6b
Compare
2 participants
This change implements the planned @mention feature for calendar notes.
It includes:
PR created automatically by Jules for task 5204406483989686825 started by @ngoiyaeric
Summary by CodeRabbit
Release Notes
New Features
Improvements