feat: notification system with unread indicators, browser/desktop#27
Merged
BuckyMcYolo merged 5 commits intomainfrom Mar 31, 2026
Merged
feat: notification system with unread indicators, browser/desktop#27BuckyMcYolo merged 5 commits intomainfrom
BuckyMcYolo merged 5 commits intomainfrom
Conversation
notifications, and Tauri app setup
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a cross-platform notification system: DB schema and API for user notification settings, realtime unread-state bootstrap/events, Tauri desktop notification integration, browser/Tauri notification dispatch, auto mark-as-read behavior, and UI wiring for settings and sidebar unread/mention indicators. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Web/Tauri)
participant Realtime as Realtime Server
participant API as API Server
participant DB as Database
Client->>Realtime: socket connect (auth)
Realtime->>API: request unread bootstrap for user
API->>DB: query memberships, read-states, counts
DB-->>API: NotificationBootstrap { readStates }
API-->>Realtime: NotificationBootstrap
Realtime-->>Client: notification:bootstrap
Client->>Client: UnreadProvider replaces stateMap -> UI updates
sequenceDiagram
participant User as User
participant Client as Client (Web)
participant Realtime as Realtime Server
participant DB as Database
User->>Client: view / receive message in channel
Client->>Client: useAutoMarkRead debounced trigger
Client->>Realtime: channel:mark-read
Realtime->>DB: upsert channel_read_state
DB-->>Realtime: updated read state
Realtime-->>Client: channel:read-state (user room & requester)
Client->>Client: UnreadProvider clears/updates entry -> UI updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/src/components/sidebar/channel-panel/channel-list.tsx (1)
599-611:⚠️ Potential issue | 🟡 MinorAvoid two competing
ml-automargins between trailing controls.The mention badge (line 600) and DropdownMenuTrigger (line 609) both use
ml-autowithin the same flex row container. When both are rendered (mentionCount > 0andcanManageboth true), they create competing margin claims—the badge gets pushed left and the menu button pushed right, leaving a gap. Keepml-autoon only one element or group both trailing controls in a wrapper element with appropriate spacing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/components/sidebar/channel-panel/channel-list.tsx` around lines 599 - 611, The trailing controls (the mention badge rendered when mentionCount > 0 and the DropdownMenuTrigger used when canManage is true) both use ml-auto causing competing margins; remove ml-auto from one of them or wrap both (the span showing {mentionCount} and the DropdownMenuTrigger) in a single container (e.g., a flex row right-aligned) so only that wrapper gets ml-auto, and adjust spacing between the badge and menu button via gap or margin; update JSX around mentionCount, DropdownMenuTrigger, menuOpen and the surrounding container to reflect the single right-aligned wrapper.apps/realtime/src/services/read-states.ts (1)
52-68:⚠️ Potential issue | 🟠 MajorDon't advance
lastReadAtpast the message snapshot.Replacing
latestMessage.createdAtwithnew Date()widens the read window beyond the row you just selected. Any message inserted after thelatestMessagequery but before this write can now fall behindlastReadAtand get marked read even though the client never saw it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/realtime/src/services/read-states.ts` around lines 52 - 68, The code advances lastReadAt to new Date(), which can move the read cursor past the message snapshot; instead assign the timestamp from the selected row so the read pointer does not exceed the snapshot: set lastReadAt = latestMessage.createdAt (ensure any necessary type conversion preserves the exact timestamp returned by the DB) when latestMessage exists, keeping the existing assignment to lastReadMessageId; update this in the block that references latestMessage and lastReadAt in read-states.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/realtime/src/index.ts`:
- Around line 282-293: The bootstrap of unread state using
getUnreadStatesForUser() is sent fire-and-forget after the socket joins
userRoom(), which allows in-flight notification:unread/notification:mention
messages to arrive and then be wiped out when
socket.emit("notification:bootstrap", ...) replaces the unread map; fix by
serializing bootstrap with live delivery: either send the bootstrap snapshot
before joining the userRoom() in initializeConnection so live fanout cannot race
it, or add a monotonic bootstrap cursor/version field to the bootstrap payload
and include that same cursor/version on all live unread events
(notification:unread/notification:mention) so the client can ignore any
bootstrap older than the latest cursor; update code paths around
getUnreadStatesForUser, socket.emit("notification:bootstrap"), and the live
fanout that emits notification:unread/notification:mention to carry/compare the
cursor/version.
In `@apps/web/src/context/unread-context.tsx`:
- Around line 134-149: The markChannelRead callback currently deletes the
channel entry from stateMap optimistically and emits "channel:mark-read" without
handling the server ack; change markChannelRead to snapshot the previous entry
from stateMap (or the whole map) before deleting, pass an acknowledgement
callback to socket.emit("channel:mark-read", ..., ack) and if the ack returns {
ok: false } restore the saved snapshot via setStateMap so the local unread state
is rolled back; keep the optimistic delete for fast UX but ensure rollback on
failure using the saved snapshot in the same markChannelRead function.
In `@apps/web/src/hooks/use-auto-mark-read.ts`:
- Around line 25-32: The mount effect currently calls debouncedMarkRead() even
when the tab is hidden; change the effect in use-auto-mark-read.ts so it only
triggers debouncedMarkRead() when document.visibilityState === "visible" (or
when document.hidden is false) and channelId is set, leaving the existing
cleanup of timerRef intact; this ensures the existing visibilitychange effect
still handles the first visible read while preventing off-screen mounts from
marking the channel read.
In `@apps/web/src/hooks/use-browser-notifications.ts`:
- Around line 32-45: onMention currently treats settings === undefined as
enabled and also ignores DM mute preferences; update the handler (onMention
inside useEffect) to first bail if settings is not yet loaded, then honor both
settings.desktopNotifications === "nothing" and—when payload.guildId ===
null—settings.dmNotifications === "nothing" before calling showNotification.
Reference the onMention handler, the MentionNotification payload
(payload.guildId and payload.messageId), settings.desktopNotifications and
settings.dmNotifications, and showNotification so the code returns early when
settings are undefined or when the appropriate notification preference is
"nothing".
In `@apps/web/src/lib/notification-dispatcher.ts`:
- Around line 61-70: The Tauri branch in notification-dispatcher ignores the
notification "tag" and the "onClick" callback, so update the isTauri() handling
to preserve feature parity: when importing sendNotification, pass through the
tag option (e.g., include tag in the sendNotification call) and wire up click
handling by registering a Tauri event or fallback that invokes the provided
onClick callback (or document/emit a warning if Tauri cannot support click
callbacks). Locate the isTauri() conditional and the sendNotification usage and
ensure both tag and onClick are forwarded or gracefully handled.
- Around line 26-33: Update getNotificationPermissionSync to avoid returning the
misleading "default" for Tauri by introducing a distinct sentinel and updating
the return type: instead of unconditionally returning "default" when isTauri()
is true, return a clearly named sentinel (e.g., "tauri-unknown" or null) and add
that sentinel to the function's union return type; adjust callers to handle the
new sentinel (or document the limitation in the function JSDoc) so UI components
can render a Tauri-specific state rather than treating it as the browser
"default". Ensure references to getNotificationPermissionSync and isTauri are
updated accordingly.
In `@packages/db/src/schemas/user-notification-settings.ts`:
- Around line 36-59: The current schema exports
(selectUserNotificationSettingsSchema, insertUserNotificationSettingsSchema,
notificationSettingsResponseSchema, updateNotificationSettingsSchema) are
generated via drizzle-zod which requires Zod v3 and conflicts with Zod v4;
replace the drizzle-zod usage with Drizzle ORM's built-in Zod generators by
importing createSelectSchema and createInsertSchema from "drizzle-orm/zod" (or
regenerate these schemas with that API) so they are compatible with Zod v4, and
if you cannot migrate immediately apply the drizzle-zod zod/v4 namespace
workaround or install with --legacy-peer-deps as a temporary measure.
In `@packages/realtime-types/src/events.ts`:
- Around line 223-230: The NotificationBootstrap type duplicates the shape of
ChannelReadState; replace the inline readStates item with a derived type to
avoid drift by making readStates: Array<ChannelReadState> (or
ChannelReadState[]) and import or reference the existing ChannelReadState type
where NotificationBootstrap is defined so both contracts stay in sync; update
any usages/imports that expect the old inline shape if necessary.
---
Outside diff comments:
In `@apps/realtime/src/services/read-states.ts`:
- Around line 52-68: The code advances lastReadAt to new Date(), which can move
the read cursor past the message snapshot; instead assign the timestamp from the
selected row so the read pointer does not exceed the snapshot: set lastReadAt =
latestMessage.createdAt (ensure any necessary type conversion preserves the
exact timestamp returned by the DB) when latestMessage exists, keeping the
existing assignment to lastReadMessageId; update this in the block that
references latestMessage and lastReadAt in read-states.ts.
In `@apps/web/src/components/sidebar/channel-panel/channel-list.tsx`:
- Around line 599-611: The trailing controls (the mention badge rendered when
mentionCount > 0 and the DropdownMenuTrigger used when canManage is true) both
use ml-auto causing competing margins; remove ml-auto from one of them or wrap
both (the span showing {mentionCount} and the DropdownMenuTrigger) in a single
container (e.g., a flex row right-aligned) so only that wrapper gets ml-auto,
and adjust spacing between the badge and menu button via gap or margin; update
JSX around mentionCount, DropdownMenuTrigger, menuOpen and the surrounding
container to reflect the single right-aligned wrapper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0557efad-a91a-4290-90ef-d36415d6cb72
⛔ Files ignored due to path filters (2)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
.gitignoreROADMAP.mdapps/api/src/app.tsapps/api/src/routes/v1/notification-settings/handlers.tsapps/api/src/routes/v1/notification-settings/index.tsapps/api/src/routes/v1/notification-settings/routes.tsapps/api/src/routes/v1/notification-settings/schema.tsapps/desktop/package.jsonapps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/capabilities/default.jsonapps/desktop/src-tauri/src/lib.rsapps/realtime/src/index.tsapps/realtime/src/services/read-states.tsapps/web/package.jsonapps/web/src/components/settings/notification-settings.tsxapps/web/src/components/settings/settings-dialog.tsxapps/web/src/components/sidebar/channel-panel/channel-list.tsxapps/web/src/components/sidebar/dm-panel/dm-list.tsxapps/web/src/context/unread-context.tsxapps/web/src/hooks/use-auto-mark-read.tsapps/web/src/hooks/use-browser-notifications.tsapps/web/src/lib/notification-dispatcher.tsapps/web/src/routes/_authenticated.tsxapps/web/src/routes/_authenticated/$guildSlug/$channelId.tsxapps/web/src/routes/_authenticated/dms/$dmId.tsxpackages/db/src/schemas/index.tspackages/db/src/schemas/user-notification-settings.tspackages/realtime-types/src/events.ts
Comment on lines
+206
to
+250
| // For each channel, compute unread and mention counts | ||
| const results = await Promise.all( | ||
| channelIds.map(async (channelId) => { | ||
| const readState = readStateMap.get(channelId) | ||
| const lastReadAt = readState?.lastReadAt ?? new Date(0) | ||
|
|
||
| const [unreadRow, mentionRow] = await Promise.all([ | ||
| db | ||
| .select({ count: count() }) | ||
| .from(schema.message) | ||
| .where( | ||
| and( | ||
| eq(schema.message.channelId, channelId), | ||
| gt(schema.message.createdAt, lastReadAt), | ||
| ne(schema.message.authorId, userId) | ||
| ) | ||
| ) | ||
| .then((rows) => rows[0]), | ||
| db | ||
| .select({ count: count() }) | ||
| .from(schema.messageMention) | ||
| .where( | ||
| and( | ||
| eq(schema.messageMention.channelId, channelId), | ||
| eq(schema.messageMention.mentionedUserId, userId), | ||
| gt(schema.messageMention.createdAt, lastReadAt) | ||
| ) | ||
| ) | ||
| .then((rows) => rows[0]), | ||
| ]) | ||
|
|
||
| const unreadCount = Number(unreadRow?.count ?? 0) | ||
| const mentionCount = Number(mentionRow?.count ?? 0) | ||
|
|
||
| // Only include channels with unread activity | ||
| if (unreadCount === 0 && mentionCount === 0) return null | ||
|
|
||
| return { | ||
| channelId, | ||
| unreadCount, | ||
| mentionCount, | ||
| lastReadMessageId: readState?.lastReadMessageId ?? null, | ||
| } | ||
| }) | ||
| ) |
There was a problem hiding this comment.
Avoid 2N aggregate queries during unread bootstrap.
This runs two count queries per channel on every socket connect/reconnect. A user in 100 channels turns one bootstrap into 200 aggregate selects before the client is ready. Aggregate unread and mention counts across all channelIds with GROUP BY channelId and merge the results in memory instead of querying per channel.
Comment on lines
+72
to
+74
| const handleChange = (key: keyof NotificationSettings, value: string) => { | ||
| updateSettings({ [key]: value }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/components/settings/notification-settings.tsx | head -80Repository: BuckyMcYolo/townhall
Length of output: 3250
🏁 Script executed:
rg "type NotificationSettings|interface NotificationSettings" -A 5Repository: BuckyMcYolo/townhall
Length of output: 1047
🏁 Script executed:
rg "mutationFn|updateSettings" apps/web/src/components/settings/notification-settings.tsx -B 2 -A 2Repository: BuckyMcYolo/townhall
Length of output: 403
🏁 Script executed:
rg "handleChange" apps/web/src/components/settings/notification-settings.tsx -B 2 -A 2Repository: BuckyMcYolo/townhall
Length of output: 670
🏁 Script executed:
grep -n "onValueChange\|onChange" apps/web/src/components/settings/notification-settings.tsxRepository: BuckyMcYolo/townhall
Length of output: 216
🏁 Script executed:
cat -n apps/web/src/components/settings/notification-settings.tsx | sed -n '130,160p'Repository: BuckyMcYolo/townhall
Length of output: 1495
🏁 Script executed:
fd "tsconfig.json|biome.json|biome.jsonc" -t fRepository: BuckyMcYolo/townhall
Length of output: 405
🏁 Script executed:
cat biome.jsonRepository: BuckyMcYolo/townhall
Length of output: 4193
🏁 Script executed:
cat apps/web/tsconfig.jsonRepository: BuckyMcYolo/townhall
Length of output: 275
🏁 Script executed:
cat apps/web/src/components/settings/notification-settings.tsx | sed -n '20,75p'Repository: BuckyMcYolo/townhall
Length of output: 1834
Use a generic type-safe handler to preserve per-setting union types.
{ [key]: value } with value: string is typed as Record<keyof NotificationSettings, string>, which is incompatible with Partial<NotificationSettings>. The value: string parameter accepts any string instead of restricting to valid enum values per key.
Suggested fix
- const handleChange = (key: keyof NotificationSettings, value: string) => {
- updateSettings({ [key]: value })
+ const handleChange = <K extends keyof NotificationSettings>(
+ key: K,
+ value: NotificationSettings[K]
+ ) => {
+ updateSettings({ [key]: value } as Pick<NotificationSettings, K>)
}Update the Select call sites to pass narrowed values:
onValueChange={(v) =>
handleChange(
"desktopNotifications",
v as NotificationSettings["desktopNotifications"]
)
}
Comment on lines
+26
to
+33
| export function getNotificationPermissionSync(): | ||
| | "granted" | ||
| | "denied" | ||
| | "default" { | ||
| if (isTauri()) return "default" | ||
| if (!("Notification" in window)) return "denied" | ||
| return Notification.permission | ||
| } |
There was a problem hiding this comment.
getNotificationPermissionSync() returns misleading value for Tauri.
Returning "default" unconditionally for Tauri doesn't reflect the actual permission state. If UI components rely on this for rendering permission prompts or status indicators, they may display incorrect information.
Consider documenting this limitation clearly or returning a distinct sentinel value that callers can handle appropriately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/notification-dispatcher.ts` around lines 26 - 33, Update
getNotificationPermissionSync to avoid returning the misleading "default" for
Tauri by introducing a distinct sentinel and updating the return type: instead
of unconditionally returning "default" when isTauri() is true, return a clearly
named sentinel (e.g., "tauri-unknown" or null) and add that sentinel to the
function's union return type; adjust callers to handle the new sentinel (or
document the limitation in the function JSDoc) so UI components can render a
Tauri-specific state rather than treating it as the browser "default". Ensure
references to getNotificationPermissionSync and isTauri are updated accordingly.
Comment on lines
+61
to
+70
| if (isTauri()) { | ||
| try { | ||
| const { sendNotification } = await import( | ||
| "@tauri-apps/plugin-notification" | ||
| ) | ||
| sendNotification({ title, body }) | ||
| } catch { | ||
| // Tauri notification plugin not available | ||
| } | ||
| return |
There was a problem hiding this comment.
Tauri notifications lack feature parity with browser.
The Tauri path ignores the tag option (useful for notification grouping/replacement) and the onClick callback. If users expect click-to-navigate behavior in the desktop app, this will silently fail.
📝 Suggested improvement to document the limitation
if (isTauri()) {
try {
const { sendNotification } = await import(
"@tauri-apps/plugin-notification"
)
+ // Note: Tauri notifications don't support tag grouping or click handlers
sendNotification({ title, body })
} catch {
// Tauri notification plugin not available
}
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (isTauri()) { | |
| try { | |
| const { sendNotification } = await import( | |
| "@tauri-apps/plugin-notification" | |
| ) | |
| sendNotification({ title, body }) | |
| } catch { | |
| // Tauri notification plugin not available | |
| } | |
| return | |
| if (isTauri()) { | |
| try { | |
| const { sendNotification } = await import( | |
| "@tauri-apps/plugin-notification" | |
| ) | |
| // Note: Tauri notifications don't support tag grouping or click handlers | |
| sendNotification({ title, body }) | |
| } catch { | |
| // Tauri notification plugin not available | |
| } | |
| return |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/notification-dispatcher.ts` around lines 61 - 70, The Tauri
branch in notification-dispatcher ignores the notification "tag" and the
"onClick" callback, so update the isTauri() handling to preserve feature parity:
when importing sendNotification, pass through the tag option (e.g., include tag
in the sendNotification call) and wire up click handling by registering a Tauri
event or fallback that invokes the provided onClick callback (or document/emit a
warning if Tauri cannot support click callbacks). Locate the isTauri()
conditional and the sendNotification usage and ensure both tag and onClick are
forwarded or gracefully handled.
Comment on lines
+36
to
+59
| export const selectUserNotificationSettingsSchema = createSelectSchema( | ||
| userNotificationSettings | ||
| ) | ||
| export const insertUserNotificationSettingsSchema = createInsertSchema( | ||
| userNotificationSettings | ||
| ).omit({ | ||
| id: true, | ||
| createdAt: true, | ||
| updatedAt: true, | ||
| }) | ||
|
|
||
| export const notificationSettingsResponseSchema = | ||
| selectUserNotificationSettingsSchema.pick({ | ||
| desktopNotifications: true, | ||
| dmNotifications: true, | ||
| }) | ||
|
|
||
| export const updateNotificationSettingsSchema = | ||
| insertUserNotificationSettingsSchema | ||
| .pick({ | ||
| desktopNotifications: true, | ||
| dmNotifications: true, | ||
| }) | ||
| .partial() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
drizzle-zod Zod 4 compatibility
💡 Result:
drizzle-zod has partial compatibility with Zod v4 features via Zod v3's /v4 namespace (import { z } from 'zod/v4'), supported since drizzle-zod@0.8.1 (May 2025). However, it is not fully compatible with the standalone Zod v4 package (^4.0.0) due to peerDependency on Zod ^3.25.1, causing installation conflicts (ERESOLVE). Workarounds include using --legacy-peer-deps, pnpm overrides, or pinning to drizzle-zod@^0.6.0. Full native Zod v4 support (import { z } from 'zod') remains unresolved (open issue #4746 as of late 2025). Drizzle ORM itself now provides first-class Zod schema generation (deprecated drizzle-zod), explicitly using zod/v4 in docs (e.g., import { z } from 'zod/v4';, createSelectSchema from 'drizzle-orm/zod'). This is the recommended approach for new projects, with built-in support for Zod v4 features.
Citations:
- 1: Support Zod v4 in drizzle-zod drizzle-team/drizzle-orm#4478
- 2: [FEATURE]: drizzle-zod support for zod v4 drizzle-team/drizzle-orm#4625
- 3: https://orm.drizzle.team/docs/zod
- 4: [FEATURE]: drizzle-zod adopt zod v4 drizzle-team/drizzle-orm#4406
- 5: [FEATURE]: True Zod Objects from drizzle-zod drizzle-team/drizzle-orm#4641
- 6: [BUG]: Bump
drizzle-zodtozodv4 drizzle-team/drizzle-orm#4746 - 7: https://www.npmjs.com/package/drizzle-zod
- 8: https://github.com/drizzle-team/drizzle-orm/tree/main/drizzle-zod
🏁 Script executed:
# Find the package.json and check drizzle-zod and zod versions
fd -n "package\.json" packages/db | head -5 | xargs -I {} cat {}Repository: BuckyMcYolo/townhall
Length of output: 294
🏁 Script executed:
# Read the full user-notification-settings.ts file to check imports and usage
cat -n packages/db/src/schemas/user-notification-settings.tsRepository: BuckyMcYolo/townhall
Length of output: 2440
🏁 Script executed:
# Find package.json and check versions
find . -name "package.json" -path "*/packages/*" | head -1 | xargs cat | grep -A 5 '"dependencies"'Repository: BuckyMcYolo/townhall
Length of output: 210
🏁 Script executed:
# Also check root package.json
cat package.json | grep -E '"drizzle-zod"|"zod"' | head -10Repository: BuckyMcYolo/townhall
Length of output: 46
🏁 Script executed:
# Find all package.json files and search for drizzle-zod version
find . -name "package.json" -type f | xargs grep -l "drizzle-zod" 2>/dev/nullRepository: BuckyMcYolo/townhall
Length of output: 90
🏁 Script executed:
# Get the full root package.json to see all dependencies
cat package.jsonRepository: BuckyMcYolo/townhall
Length of output: 811
🏁 Script executed:
# Read packages/db/package.json to see actual versions
cat packages/db/package.jsonRepository: BuckyMcYolo/townhall
Length of output: 714
Resolve drizzle-zod and Zod v4 incompatibility.
The project uses drizzle-zod@0.8.3 which declares a peerDependency on zod@^3.25.1, not v4. Using zod@^4.3.6 will cause ERESOLVE dependency conflicts during installation. The recommended fix is to migrate to Drizzle ORM's built-in Zod schema generation (createSelectSchema and createInsertSchema from drizzle-orm/zod), which has first-class support for Zod v4. Alternatively, if staying with drizzle-zod, implement the zod/v4 namespace workaround (supported since drizzle-zod@0.8.1) or use --legacy-peer-deps flag, though the built-in Drizzle ORM approach is preferred for new projects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/db/src/schemas/user-notification-settings.ts` around lines 36 - 59,
The current schema exports (selectUserNotificationSettingsSchema,
insertUserNotificationSettingsSchema, notificationSettingsResponseSchema,
updateNotificationSettingsSchema) are generated via drizzle-zod which requires
Zod v3 and conflicts with Zod v4; replace the drizzle-zod usage with Drizzle
ORM's built-in Zod generators by importing createSelectSchema and
createInsertSchema from "drizzle-orm/zod" (or regenerate these schemas with that
API) so they are compatible with Zod v4, and if you cannot migrate immediately
apply the drizzle-zod zod/v4 namespace workaround or install with
--legacy-peer-deps as a temporary measure.
Comment on lines
+223
to
+230
| export type NotificationBootstrap = { | ||
| readStates: Array<{ | ||
| channelId: string | ||
| unreadCount: number | ||
| mentionCount: number | ||
| lastReadMessageId: string | null | ||
| }> | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid type drift by deriving bootstrap item shape from ChannelReadState.
The readStates item duplicates fields already defined in ChannelReadState. Deriving it keeps both event contracts in sync.
♻️ Suggested refactor
+type NotificationBootstrapReadState = Pick<
+ ChannelReadState,
+ "channelId" | "unreadCount" | "mentionCount" | "lastReadMessageId"
+>
+
export type NotificationBootstrap = {
- readStates: Array<{
- channelId: string
- unreadCount: number
- mentionCount: number
- lastReadMessageId: string | null
- }>
+ readStates: NotificationBootstrapReadState[]
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/realtime-types/src/events.ts` around lines 223 - 230, The
NotificationBootstrap type duplicates the shape of ChannelReadState; replace the
inline readStates item with a derived type to avoid drift by making readStates:
Array<ChannelReadState> (or ChannelReadState[]) and import or reference the
existing ChannelReadState type where NotificationBootstrap is defined so both
contracts stay in sync; update any usages/imports that expect the old inline
shape if necessary.
- Add Resend email integration to auth package with new env vars - Enrich unread notifications with author name, content preview, and channel name for better browser/Tauri notifications - Fix notification bootstrap race by awaiting unread state before joining user presence room - Add optimistic rollback for failed mark-read acknowledgements - Skip auto-mark-read when tab is not visible - Allow tauri://localhost in CORS origins for desktop app - Fix Tauri config identifier and frontendDist path - Fix channel list layout so badge and menu don't overlap - Improve login/signup pages with PasswordInput, loading, and errors - Add `pnpm desktop` script and switch Turbo UI to stream mode
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/realtime/src/index.ts (1)
92-100:⚠️ Potential issue | 🟠 MajorKeep
tauri://localhostwhenREALTIME_CORS_ORIGINis configured.The current fallback only includes the Tauri origin when the env var is empty. Any environment that already sets
REALTIME_CORS_ORIGINwill still reject the desktop client.🐛 Proposed fix
const defaultOrigins = [ "http://localhost:3000", "http://localhost:3001", - "tauri://localhost", ] -const corsOrigins = (env.REALTIME_CORS_ORIGIN || defaultOrigins.join(",")) - .split(",") - .map((origin) => origin.trim()) - .filter(Boolean) +const corsOrigins = [ + ...new Set([ + ...(env.REALTIME_CORS_ORIGIN || defaultOrigins.join(",")) + .split(",") + .map((origin) => origin.trim()) + .filter(Boolean), + "tauri://localhost", + ]), +]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/realtime/src/index.ts` around lines 92 - 100, The CORS setup uses defaultOrigins only when env.REALTIME_CORS_ORIGIN is empty, which drops "tauri://localhost" when REALTIME_CORS_ORIGIN is set; change the logic in the corsOrigins calculation to always include "tauri://localhost" (from defaultOrigins) by parsing env.REALTIME_CORS_ORIGIN into an array, trimming/filtering, then ensuring "tauri://localhost" is present (append if missing) before assigning corsOrigins; adjust references in this block around defaultOrigins, corsOrigins, and env.REALTIME_CORS_ORIGIN.
♻️ Duplicate comments (4)
apps/realtime/src/index.ts (1)
285-298:⚠️ Potential issue | 🔴 CriticalThe unread bootstrap can still race live fanout.
socket.join(userPresenceRoom)still happens before the snapshot emit, sonotification:unread/notification:mentioncan arrive first and then be overwritten bynotification:bootstrap.🐛 Proposed fix
- await socket.join(userPresenceRoom) - // Bootstrap unread state before joining userRoom so live notifications // arriving between join and bootstrap don't get wiped try { const bootstrap = await getUnreadStatesForUser(socket.data.user.id) socket.emit("notification:bootstrap", bootstrap) } catch (err) { console.error("Failed to bootstrap unread states:", { socketId: socket.id, userId: socket.data.user.id, error: err, }) } + + await socket.join(userPresenceRoom)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/realtime/src/index.ts` around lines 285 - 298, The bootstrap emit races with live notifications because socket.join(userPresenceRoom) occurs before fetching and emitting the snapshot; move the unread bootstrap before joining the presence room: call getUnreadStatesForUser(socket.data.user.id) and socket.emit("notification:bootstrap", ...) (keeping the existing try/catch and error logging) prior to calling socket.join(userPresenceRoom) so that subsequent fanout into userPresenceRoom cannot overwrite the bootstrapped state.packages/realtime-types/src/events.ts (1)
226-233: 🧹 Nitpick | 🔵 TrivialConsider deriving from
ChannelReadStateto avoid type drift.The
readStatesitem shape duplicates fields already inChannelReadState. This was flagged in a previous review.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/realtime-types/src/events.ts` around lines 226 - 233, The NotificationBootstrap.readStates duplicates ChannelReadState; replace the inline object type in NotificationBootstrap with Array<ChannelReadState> (importing ChannelReadState if not already) so the shape is derived from the canonical type, and make sure the lastReadMessageId and numeric fields’ nullability/optional status match ChannelReadState exactly to avoid type drift.apps/web/src/hooks/use-browser-notifications.ts (1)
35-39:⚠️ Potential issue | 🟠 MajorHonor DM mute preference for mention notifications.
For
guildId === null, mentions currently bypassdmNotificationsand still notify.Proposed fix
const onMention = (payload: MentionNotification) => { if (document.visibilityState === "visible") return if (!settings) return if (settings.desktopNotifications === "nothing") return + if (payload.guildId === null && settings.dmNotifications === "nothing") { + return + } const mentionType = payload.type === "everyone_mention" ? "@everyone" : "a mention"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/use-browser-notifications.ts` around lines 35 - 39, The onMention handler currently skips honoring DM mute settings; update the early-return logic inside onMention(MentionNotification) to check payload.guildId and the user's DM preference: if payload.guildId === null and settings?.dmNotifications === "none" (or whatever the "mute" value is in your settings) then return early to suppress the notification; keep the existing checks for document.visibilityState and desktopNotifications and perform this DM-pref check before creating/sending the notification.apps/web/src/lib/notification-dispatcher.ts (1)
26-31:⚠️ Potential issue | 🟡 Minor
getNotificationPermissionSync()still returns an ambiguous Tauri value.Returning
"default"for all Tauri cases conflates unknown state with browser-default permission and can mislead UI state handling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/notification-dispatcher.ts` around lines 26 - 31, getNotificationPermissionSync currently returns "default" for Tauri via the isTauri() branch, which conflates Tauri-specific unknown state with the browser "default" permission; change the function signature to include a distinct Tauri value (e.g., add "tauri" or "unknown-tauri" to the union) and return that value from the isTauri() branch instead of "default"; update any callers of getNotificationPermissionSync (UI state logic, notification-dispatcher consumers) to handle the new Tauri-specific value appropriately so UI can distinguish browser-default from Tauri unknown state; reference getNotificationPermissionSync and isTauri when locating the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/realtime/src/services/notifications.ts`:
- Around line 125-131: Normalize mention tokens in message content before
computing contentPreview: convert patterns like `<@uuid>` and `[@... id="uuid"]`
to a human-readable form (e.g., `@username` or `@user`) or strip them to `@user`
placeholders. Implement or call a helper (e.g., normalizeMentions(content:
string): string) and use it when reading input.message.content in the
contentPreview expression so the preview displays normalized text instead of raw
mention tokens; keep the attachments fallback unchanged.
In `@apps/web/src/context/unread-context.tsx`:
- Line 149: The ack callback currently types the parameter as (res: { ok:
boolean }) => ...; change that to use the full MarkChannelReadAck union type
(the exported type used for the mark-read ack) so you can access success data or
error details in the handler; update the handler in unread-context (the arrow
callback where res is typed) to narrow on res.ok and use res.state on success
and log or handle res.error on failure.
In `@apps/web/src/hooks/use-auto-mark-read.ts`:
- Around line 18-21: The debounce callback scheduled by debouncedMarkRead
currently calls markChannelRead(channelIdRef.current) without re-checking tab
visibility; update the timeout handler inside useAutoMarkRead (the function
using timerRef.current and channelIdRef.current) to first verify the page is
still visible (e.g., check document.visibilityState === 'visible' or an existing
isVisibleRef) and only then call markChannelRead, otherwise skip/cancel marking
to avoid clearing unread while the tab is hidden.
In `@apps/web/src/lib/notification-dispatcher.ts`:
- Around line 18-19: Guard accesses to the browser globals by checking for their
existence before using them: replace direct uses like if (!("Notification" in
window)) return false and if (Notification.permission === "granted") return true
with checks that first ensure a browser environment (e.g. if (typeof window ===
"undefined" || typeof Notification === "undefined") return false) and then
safely read Notification.permission (e.g. const perm = typeof Notification !==
"undefined" ? Notification.permission : "default";). Apply the same pattern for
every occurrence that touches window or Notification (the instances currently
checking "Notification" in window, Notification.permission, and any other direct
window/Notification access at the other noted locations).
In `@apps/web/src/routes/check-email.tsx`:
- Around line 14-19: The validateSearch currently uses an unsafe assertion
(search.email as string); replace it with a Zod schema to validate and coerce
the search params: import Zod (z), define a schema like z.object({ email:
z.string().email().optional().default("") }), and use that schema inside the
Route's validateSearch (or pass it directly if createFileRoute/TanStack Router
supports a schema) to parse/validate the incoming search and return the typed {
email } for CheckEmailPage instead of the manual cast.
- Around line 24-38: The mutation currently destructures mutate: resendEmail,
isPending and isSuccess but omits the error state, so failures from
authClient.sendVerificationEmail aren't surfaced to users; update the
useMutation call to also destructure the error (or isError) from useMutation and
propagate the thrown error so the UI can render feedback, then update the resend
button/UI (which uses resendEmail, isPending, isSuccess) to show an error
message or error state when error/isError is set; reference mutate: resendEmail,
isPending, isSuccess, useMutation and authClient.sendVerificationEmail when
making the changes.
In `@apps/web/src/routes/forgot-password.tsx`:
- Around line 24-39: The mutation currently reads the live `email` state inside
`mutationFn`, so if the input changes while the request is pending the UI may
report a different address; update `sendReset`/`useMutation` to capture the
submitted value instead — either change `mutationFn` to accept an `email`
argument and call `sendReset(submittedEmail)`, or inside the form submit handler
copy `const submittedEmail = email` and pass that into `sendReset` (or set it on
a closure) so `authClient.requestPasswordReset({ email: submittedEmail,
redirectTo: "/reset-password" })` uses the frozen value; apply the same pattern
for the other mutation usages referenced (lines 54-56, 86-89).
In `@apps/web/src/routes/reset-password.tsx`:
- Around line 38-47: The password mismatch check currently inside mutationFn
(checking password vs confirmPassword) should be moved out of mutationFn and
performed before calling the mutation so it becomes a client-side validation
rather than a mutation error; ensure you set a local error state (e.g.,
localError) when password !== confirmPassword and only call
authClient.resetPassword({ newPassword: password, token }) from mutationFn when
validation passes, then display errors using localError || error?.message so
client validation shows before any mutation error.
In `@apps/web/src/routes/signup.tsx`:
- Around line 172-180: The Terms/Privacy text is rendered as non-focusable
<span> elements, causing accessibility/UX issues; replace those <span
className="underline underline-offset-4 hover:text-primary">Terms of
Service</span> and Privacy Policy spans in the signup component's JSX with real
interactive links (use your router's <Link> or a semantic <a>), provide
appropriate hrefs (to the Terms and Privacy routes or external URLs), keep the
existing classes for styling, and ensure the links are keyboard-focusable and
include target/rel if opening externally.
In `@packages/auth/src/lib/auth.ts`:
- Around line 168-169: Replace the two informational console.error calls in
auth.ts with an info-level logger: locate the console.error call that logs
"[TOWNHALL EMAIL] Sending verification email to ${user.email}..." and the later
console.error that logs the success message, and change them to use an
info/info-level logging method (for example console.info or your existing
logger.info) so non-error informational messages aren't written to error logs;
keep the message text unchanged and only update the logging level.
- Around line 141-163: The sendResetPassword function currently uses a
fire-and-forget .then() on resend.emails.send which silences failures; change
sendResetPassword to await the promise returned by resend.emails.send (like
sendVerificationEmail does), catch and rethrow or return the error so callers
can handle it, and replace the .then() logging with proper try/catch around
resend.emails.send to log and propagate failures from sendResetPassword.
In `@packages/env/src/server.ts`:
- Around line 45-46: The server env schema currently requires RESEND_API_KEY
which causes serverSchema.parse(process.env) to fail on services that don't use
email; update the schema so RESEND_API_KEY is not mandatory (e.g., change
RESEND_API_KEY: z.string().min(1) to RESEND_API_KEY:
z.string().min(1).optional()) or refactor by extracting email fields
(RESEND_API_KEY and EMAIL_FROM) into a separate emailSchema and only
parse/validate that schema in modules that actually send email (where
RESEND_API_KEY is needed) while keeping serverSchema.parse(process.env)
non-blocking for non-email apps.
In `@turbo.json`:
- Line 3: The Turborepo configuration uses an undocumented experimental UI value
("stream-with-experimental-timestamps") for the "ui" key; update that "ui" value
to a documented, stable option such as "stream" (non-interactive) or "tui"
(interactive) to avoid breaking changes—locate the "ui" entry in the turbo.json
(the "ui" key and its current value) and replace the experimental string with
either "stream" or "tui" depending on desired behavior.
---
Outside diff comments:
In `@apps/realtime/src/index.ts`:
- Around line 92-100: The CORS setup uses defaultOrigins only when
env.REALTIME_CORS_ORIGIN is empty, which drops "tauri://localhost" when
REALTIME_CORS_ORIGIN is set; change the logic in the corsOrigins calculation to
always include "tauri://localhost" (from defaultOrigins) by parsing
env.REALTIME_CORS_ORIGIN into an array, trimming/filtering, then ensuring
"tauri://localhost" is present (append if missing) before assigning corsOrigins;
adjust references in this block around defaultOrigins, corsOrigins, and
env.REALTIME_CORS_ORIGIN.
---
Duplicate comments:
In `@apps/realtime/src/index.ts`:
- Around line 285-298: The bootstrap emit races with live notifications because
socket.join(userPresenceRoom) occurs before fetching and emitting the snapshot;
move the unread bootstrap before joining the presence room: call
getUnreadStatesForUser(socket.data.user.id) and
socket.emit("notification:bootstrap", ...) (keeping the existing try/catch and
error logging) prior to calling socket.join(userPresenceRoom) so that subsequent
fanout into userPresenceRoom cannot overwrite the bootstrapped state.
In `@apps/web/src/hooks/use-browser-notifications.ts`:
- Around line 35-39: The onMention handler currently skips honoring DM mute
settings; update the early-return logic inside onMention(MentionNotification) to
check payload.guildId and the user's DM preference: if payload.guildId === null
and settings?.dmNotifications === "none" (or whatever the "mute" value is in
your settings) then return early to suppress the notification; keep the existing
checks for document.visibilityState and desktopNotifications and perform this
DM-pref check before creating/sending the notification.
In `@apps/web/src/lib/notification-dispatcher.ts`:
- Around line 26-31: getNotificationPermissionSync currently returns "default"
for Tauri via the isTauri() branch, which conflates Tauri-specific unknown state
with the browser "default" permission; change the function signature to include
a distinct Tauri value (e.g., add "tauri" or "unknown-tauri" to the union) and
return that value from the isTauri() branch instead of "default"; update any
callers of getNotificationPermissionSync (UI state logic,
notification-dispatcher consumers) to handle the new Tauri-specific value
appropriately so UI can distinguish browser-default from Tauri unknown state;
reference getNotificationPermissionSync and isTauri when locating the change.
In `@packages/realtime-types/src/events.ts`:
- Around line 226-233: The NotificationBootstrap.readStates duplicates
ChannelReadState; replace the inline object type in NotificationBootstrap with
Array<ChannelReadState> (importing ChannelReadState if not already) so the shape
is derived from the canonical type, and make sure the lastReadMessageId and
numeric fields’ nullability/optional status match ChannelReadState exactly to
avoid type drift.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e324f10-26aa-47ee-8c67-eddcf59a97d8
⛔ Files ignored due to path filters (52)
apps/desktop/src-tauri/icons/128x128.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/128x128@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/32x32.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/64x64.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square107x107Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square142x142Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square150x150Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square284x284Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square30x30Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square310x310Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square44x44Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square71x71Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/Square89x89Logo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/StoreLogo.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-hdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-mdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xhdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xxhdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_foreground.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/android/mipmap-xxxhdpi/ic_launcher_round.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/icon.icois excluded by!**/*.icoapps/desktop/src-tauri/icons/icon.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-20x20@1x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-20x20@2x-1.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-20x20@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-20x20@3x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-29x29@1x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-29x29@2x-1.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-29x29@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-29x29@3x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-40x40@1x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-40x40@2x-1.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-40x40@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-40x40@3x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-512@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-60x60@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-60x60@3x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-76x76@1x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-76x76@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/icons/ios/AppIcon-83.5x83.5@2x.pngis excluded by!**/*.pngapps/desktop/src-tauri/src/icon.pngis excluded by!**/*.pngapps/web/public/townhallicon.pngis excluded by!**/*.pngpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/desktop/src-tauri/icons/android/mipmap-anydpi-v26/ic_launcher.xmlapps/desktop/src-tauri/icons/android/values/ic_launcher_background.xmlapps/desktop/src-tauri/icons/icon.icnsapps/desktop/src-tauri/tauri.conf.jsonapps/realtime/src/index.tsapps/realtime/src/services/channel-access.tsapps/realtime/src/services/notifications.tsapps/web/src/components/auth/auth-layout.tsxapps/web/src/components/auth/password-input.tsxapps/web/src/components/sidebar/channel-panel/channel-list.tsxapps/web/src/context/unread-context.tsxapps/web/src/hooks/use-auto-mark-read.tsapps/web/src/hooks/use-browser-notifications.tsapps/web/src/lib/notification-dispatcher.tsapps/web/src/routes/check-email.tsxapps/web/src/routes/forgot-password.tsxapps/web/src/routes/login.tsxapps/web/src/routes/reset-password.tsxapps/web/src/routes/signup.tsxpackage.jsonpackages/auth/package.jsonpackages/auth/src/lib/auth.tspackages/env/src/server.tspackages/realtime-types/src/events.tsturbo.json
Comment on lines
+125
to
+131
| const contentPreview = input.message.content | ||
| ? input.message.content.length > 100 | ||
| ? `${input.message.content.slice(0, 100)}…` | ||
| : input.message.content | ||
| : input.message.attachments.length > 0 | ||
| ? `sent ${input.message.attachments.length} attachment${input.message.attachments.length > 1 ? "s" : ""}` | ||
| : null |
There was a problem hiding this comment.
Normalize mention syntax before filling contentPreview.
This preview uses raw message.content, but the same file accepts <@uuid> and [@... id="uuid"] mention formats. Browser/Tauri notifications will surface those tokens verbatim instead of readable text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/realtime/src/services/notifications.ts` around lines 125 - 131,
Normalize mention tokens in message content before computing contentPreview:
convert patterns like `<@uuid>` and `[@... id="uuid"]` to a human-readable form
(e.g., `@username` or `@user`) or strip them to `@user` placeholders. Implement
or call a helper (e.g., normalizeMentions(content: string): string) and use it
when reading input.message.content in the contentPreview expression so the
preview displays normalized text instead of raw mention tokens; keep the
attachments fallback unchanged.
| socket.emit( | ||
| "channel:mark-read", | ||
| { channelId, lastReadMessageId }, | ||
| (res: { ok: boolean }) => { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Ack response type is simplified.
The callback types res as { ok: boolean } but MarkChannelReadAck provides { ok: true; state: ChannelReadState } | { ok: false; error: string }. This works but you could use the full type to access state on success or log error on failure.
♻️ Use full ack type
+import type { ChannelReadState } from "@repo/realtime-types"
+
+type MarkReadAckResult = { ok: true; state: ChannelReadState } | { ok: false; error: string }
+
socket.emit(
"channel:mark-read",
{ channelId, lastReadMessageId },
- (res: { ok: boolean }) => {
- if (!res.ok && snapshot) {
+ (res: MarkReadAckResult) => {
+ if (!res.ok) {
+ console.warn("mark-read failed:", res.error)
+ if (snapshot) {
const restore = snapshot
setStateMap((prev) => {
const next = new Map(prev)
next.set(channelId, restore)
return next
})
+ }
}
}
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/context/unread-context.tsx` at line 149, The ack callback
currently types the parameter as (res: { ok: boolean }) => ...; change that to
use the full MarkChannelReadAck union type (the exported type used for the
mark-read ack) so you can access success data or error details in the handler;
update the handler in unread-context (the arrow callback where res is typed) to
narrow on res.ok and use res.state on success and log or handle res.error on
failure.
Comment on lines
+18
to
+21
| timerRef.current = setTimeout(() => { | ||
| if (channelIdRef.current) { | ||
| markChannelRead(channelIdRef.current) | ||
| } |
There was a problem hiding this comment.
Re-check visibility inside the debounce callback before marking read.
debouncedMarkRead() schedules while visible, but the tab can become hidden before the timer fires. In that case, unread can be cleared off-screen.
Proposed fix
timerRef.current = setTimeout(() => {
- if (channelIdRef.current) {
+ if (
+ channelIdRef.current &&
+ document.visibilityState === "visible"
+ ) {
markChannelRead(channelIdRef.current)
}
}, DEBOUNCE_MS)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| timerRef.current = setTimeout(() => { | |
| if (channelIdRef.current) { | |
| markChannelRead(channelIdRef.current) | |
| } | |
| timerRef.current = setTimeout(() => { | |
| if ( | |
| channelIdRef.current && | |
| document.visibilityState === "visible" | |
| ) { | |
| markChannelRead(channelIdRef.current) | |
| } | |
| }, DEBOUNCE_MS) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/hooks/use-auto-mark-read.ts` around lines 18 - 21, The debounce
callback scheduled by debouncedMarkRead currently calls
markChannelRead(channelIdRef.current) without re-checking tab visibility; update
the timeout handler inside useAutoMarkRead (the function using timerRef.current
and channelIdRef.current) to first verify the page is still visible (e.g., check
document.visibilityState === 'visible' or an existing isVisibleRef) and only
then call markChannelRead, otherwise skip/cancel marking to avoid clearing
unread while the tab is hidden.
Comment on lines
+18
to
+19
| if (!("Notification" in window)) return false | ||
| if (Notification.permission === "granted") return true |
There was a problem hiding this comment.
Guard window access for non-browser execution paths.
These functions assume window exists and can throw when invoked outside the browser runtime.
Proposed fix
const isTauri = () =>
typeof window !== "undefined" && "__TAURI_INTERNALS__" in window
+const isBrowser = () => typeof window !== "undefined"
export async function requestNotificationPermission(): Promise<boolean> {
+ if (!isBrowser()) return false
if (isTauri()) {
@@
export function getNotificationPermissionSync():
| "granted"
| "denied"
| "default" {
+ if (!isBrowser()) return "denied"
if (isTauri()) return "default"
@@
export async function getNotificationPermission(): Promise<
"granted" | "denied" | "default"
> {
+ if (!isBrowser()) return "denied"
if (isTauri()) {
@@
export async function showNotification(
@@
) {
+ if (!isBrowser()) return
if (isTauri()) {Also applies to: 31-32, 49-50, 78-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/lib/notification-dispatcher.ts` around lines 18 - 19, Guard
accesses to the browser globals by checking for their existence before using
them: replace direct uses like if (!("Notification" in window)) return false and
if (Notification.permission === "granted") return true with checks that first
ensure a browser environment (e.g. if (typeof window === "undefined" || typeof
Notification === "undefined") return false) and then safely read
Notification.permission (e.g. const perm = typeof Notification !== "undefined" ?
Notification.permission : "default";). Apply the same pattern for every
occurrence that touches window or Notification (the instances currently checking
"Notification" in window, Notification.permission, and any other direct
window/Notification access at the other noted locations).
Comment on lines
+14
to
+19
| export const Route = createFileRoute("/check-email")({ | ||
| component: CheckEmailPage, | ||
| validateSearch: (search: Record<string, unknown>) => ({ | ||
| email: (search.email as string) ?? "", | ||
| }), | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using Zod for search param validation.
The manual type assertion (search.email as string) bypasses type safety. TanStack Router supports Zod schemas for validateSearch.
♻️ Suggested refactor using Zod
+import { z } from "zod"
+
+const searchSchema = z.object({
+ email: z.string().default(""),
+})
+
export const Route = createFileRoute("/check-email")({
component: CheckEmailPage,
- validateSearch: (search: Record<string, unknown>) => ({
- email: (search.email as string) ?? "",
- }),
+ validateSearch: searchSchema,
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const Route = createFileRoute("/check-email")({ | |
| component: CheckEmailPage, | |
| validateSearch: (search: Record<string, unknown>) => ({ | |
| email: (search.email as string) ?? "", | |
| }), | |
| }) | |
| import { z } from "zod" | |
| import { createFileRoute } from "@tanstack/react-router" | |
| import { CheckEmailPage } from "../components/check-email" | |
| const searchSchema = z.object({ | |
| email: z.string().default(""), | |
| }) | |
| export const Route = createFileRoute("/check-email")({ | |
| component: CheckEmailPage, | |
| validateSearch: searchSchema, | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/check-email.tsx` around lines 14 - 19, The validateSearch
currently uses an unsafe assertion (search.email as string); replace it with a
Zod schema to validate and coerce the search params: import Zod (z), define a
schema like z.object({ email: z.string().email().optional().default("") }), and
use that schema inside the Route's validateSearch (or pass it directly if
createFileRoute/TanStack Router supports a schema) to parse/validate the
incoming search and return the typed { email } for CheckEmailPage instead of the
manual cast.
Comment on lines
+38
to
+47
| mutationFn: async () => { | ||
| if (password !== confirmPassword) { | ||
| throw new Error("Passwords do not match") | ||
| } | ||
| const { error } = await authClient.resetPassword({ | ||
| newPassword: password, | ||
| token, | ||
| }) | ||
| if (error) throw new Error(error.message ?? "Failed to reset password") | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating password match before mutation.
The password mismatch check inside mutationFn creates a mutation error state for form validation. This works but could confuse error tracking/monitoring.
♻️ Move validation before mutation call
const {
mutate: resetPassword,
isPending,
isSuccess,
error,
} = useMutation({
mutationFn: async () => {
- if (password !== confirmPassword) {
- throw new Error("Passwords do not match")
- }
const { error } = await authClient.resetPassword({
newPassword: password,
token,
})
if (error) throw new Error(error.message ?? "Failed to reset password")
},
})
+ const [localError, setLocalError] = useState<string | null>(null)
+
+ const handleSubmit = (e: FormEvent) => {
+ e.preventDefault()
+ setLocalError(null)
+ if (password !== confirmPassword) {
+ setLocalError("Passwords do not match")
+ return
+ }
+ resetPassword()
+ }Then use localError || error?.message for display.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/reset-password.tsx` around lines 38 - 47, The password
mismatch check currently inside mutationFn (checking password vs
confirmPassword) should be moved out of mutationFn and performed before calling
the mutation so it becomes a client-side validation rather than a mutation
error; ensure you set a local error state (e.g., localError) when password !==
confirmPassword and only call authClient.resetPassword({ newPassword: password,
token }) from mutationFn when validation passes, then display errors using
localError || error?.message so client validation shows before any mutation
error.
Comment on lines
+172
to
+180
| <p className="px-6 text-center text-xs text-muted-foreground"> | ||
| By clicking continue, you agree to our{" "} | ||
| <span className="underline underline-offset-4 hover:text-primary"> | ||
| Terms of Service | ||
| </span>{" "} | ||
| and{" "} | ||
| <span className="underline underline-offset-4 hover:text-primary"> | ||
| Privacy Policy | ||
| </span> |
There was a problem hiding this comment.
Use real links for Terms/Privacy instead of styled spans.
These elements look clickable but are not focusable or navigable, which is an accessibility/UX issue. Replace them with <Link>/<a> targets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/signup.tsx` around lines 172 - 180, The Terms/Privacy
text is rendered as non-focusable <span> elements, causing accessibility/UX
issues; replace those <span className="underline underline-offset-4
hover:text-primary">Terms of Service</span> and Privacy Policy spans in the
signup component's JSX with real interactive links (use your router's <Link> or
a semantic <a>), provide appropriate hrefs (to the Terms and Privacy routes or
external URLs), keep the existing classes for styling, and ensure the links are
keyboard-focusable and include target/rel if opening externally.
Comment on lines
+141
to
+163
| async sendResetPassword({ user, url }) { | ||
| resend.emails | ||
| .send({ | ||
| from: env.EMAIL_FROM, | ||
| to: user.email, | ||
| subject: "Reset your Townhall password", | ||
| html: ` | ||
| <div style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; max-width: 480px; margin: 0 auto; padding: 32px 0;"> | ||
| <h2 style="margin: 0 0 8px; font-size: 24px; color: #1a1a1a;">Reset Your Password</h2> | ||
| <p style="margin: 0 0 24px; color: #555; font-size: 16px; line-height: 1.5;">Click the button below to reset your password.</p> | ||
| <a href="${url}" style="display: inline-block; background: #8B6914; color: #fff; padding: 12px 28px; border-radius: 8px; text-decoration: none; font-weight: 600; font-size: 16px;">Reset Password</a> | ||
| <p style="color: #999; font-size: 13px; margin-top: 24px; line-height: 1.4;">If you didn't request a password reset, you can safely ignore this email.</p> | ||
| </div> | ||
| `, | ||
| }) | ||
| .then(({ data, error }) => { | ||
| if (error) { | ||
| console.error("Failed to send reset password email:", error) | ||
| } else { | ||
| console.log("Reset password email sent:", data?.id) | ||
| } | ||
| }) | ||
| }, |
There was a problem hiding this comment.
sendResetPassword silently fails on email errors.
Unlike sendVerificationEmail which awaits and handles errors, this function uses fire-and-forget (.then()). If the email fails, the user sees success but never receives the reset link.
Consider awaiting the send operation and propagating errors, or at minimum ensuring the user experience handles this edge case.
🛠️ Suggested fix to await and propagate errors
- async sendResetPassword({ user, url }) {
- resend.emails
- .send({
+ async sendResetPassword({ user, url }) {
+ try {
+ const { error } = await resend.emails.send({
from: env.EMAIL_FROM,
to: user.email,
subject: "Reset your Townhall password",
html: `...`,
})
- .then(({ data, error }) => {
- if (error) {
- console.error("Failed to send reset password email:", error)
- } else {
- console.log("Reset password email sent:", data?.id)
- }
- })
+ if (error) {
+ console.error("Failed to send reset password email:", error)
+ throw new Error("Failed to send reset password email")
+ }
+ } catch (err) {
+ console.error("Exception sending reset password email:", err)
+ throw err
+ }
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async sendResetPassword({ user, url }) { | |
| resend.emails | |
| .send({ | |
| from: env.EMAIL_FROM, | |
| to: user.email, | |
| subject: "Reset your Townhall password", | |
| html: ` | |
| <div style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; max-width: 480px; margin: 0 auto; padding: 32px 0;"> | |
| <h2 style="margin: 0 0 8px; font-size: 24px; color: #1a1a1a;">Reset Your Password</h2> | |
| <p style="margin: 0 0 24px; color: #555; font-size: 16px; line-height: 1.5;">Click the button below to reset your password.</p> | |
| <a href="${url}" style="display: inline-block; background: #8B6914; color: #fff; padding: 12px 28px; border-radius: 8px; text-decoration: none; font-weight: 600; font-size: 16px;">Reset Password</a> | |
| <p style="color: #999; font-size: 13px; margin-top: 24px; line-height: 1.4;">If you didn't request a password reset, you can safely ignore this email.</p> | |
| </div> | |
| `, | |
| }) | |
| .then(({ data, error }) => { | |
| if (error) { | |
| console.error("Failed to send reset password email:", error) | |
| } else { | |
| console.log("Reset password email sent:", data?.id) | |
| } | |
| }) | |
| }, | |
| async sendResetPassword({ user, url }) { | |
| try { | |
| const { error } = await resend.emails.send({ | |
| from: env.EMAIL_FROM, | |
| to: user.email, | |
| subject: "Reset your Townhall password", | |
| html: ` | |
| <div style="font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; max-width: 480px; margin: 0 auto; padding: 32px 0;"> | |
| <h2 style="margin: 0 0 8px; font-size: 24px; color: `#1a1a1a`;">Reset Your Password</h2> | |
| <p style="margin: 0 0 24px; color: `#555`; font-size: 16px; line-height: 1.5;">Click the button below to reset your password.</p> | |
| <a href="${url}" style="display: inline-block; background: `#8B6914`; color: `#fff`; padding: 12px 28px; border-radius: 8px; text-decoration: none; font-weight: 600; font-size: 16px;">Reset Password</a> | |
| <p style="color: `#999`; font-size: 13px; margin-top: 24px; line-height: 1.4;">If you didn't request a password reset, you can safely ignore this email.</p> | |
| </div> | |
| `, | |
| }) | |
| if (error) { | |
| console.error("Failed to send reset password email:", error) | |
| throw new Error("Failed to send reset password email") | |
| } | |
| } catch (err) { | |
| console.error("Exception sending reset password email:", err) | |
| throw err | |
| } | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/auth/src/lib/auth.ts` around lines 141 - 163, The sendResetPassword
function currently uses a fire-and-forget .then() on resend.emails.send which
silences failures; change sendResetPassword to await the promise returned by
resend.emails.send (like sendVerificationEmail does), catch and rethrow or
return the error so callers can handle it, and replace the .then() logging with
proper try/catch around resend.emails.send to log and propagate failures from
sendResetPassword.
Comment on lines
+45
to
+46
| RESEND_API_KEY: z.string().min(1), | ||
| EMAIL_FROM: z.string().default("Townhall <team@townhall.chat>"), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all apps that import `@repo/env/server`
rg -l '@repo/env/server' --type ts -g '!*.d.ts' apps/Repository: BuckyMcYolo/townhall
Length of output: 229
🏁 Script executed:
# Check if RESEND_API_KEY is referenced in any of the importing apps
rg 'RESEND_API_KEY|EMAIL_FROM' apps/worker apps/realtime apps/api --type ts -g '!*.d.ts'Repository: BuckyMcYolo/townhall
Length of output: 46
🏁 Script executed:
# Find where RESEND_API_KEY and EMAIL_FROM are used in the codebase
rg 'RESEND_API_KEY|EMAIL_FROM' --type ts -g '!*.d.ts' -A 2Repository: BuckyMcYolo/townhall
Length of output: 1059
🏁 Script executed:
# Check the packages/env/src/server.ts file to understand the validation schema
cat -n packages/env/src/server.ts | head -100Repository: BuckyMcYolo/townhall
Length of output: 2432
🏁 Script executed:
# Check packages/auth to see if it uses these new fields
rg 'RESEND_API_KEY|EMAIL_FROM|Resend' packages/auth --type ts -g '!*.d.ts' -B 2 -A 2Repository: BuckyMcYolo/townhall
Length of output: 1919
🏁 Script executed:
# Check if packages/auth is imported by the apps
rg 'packages/auth|@repo/auth' apps/worker apps/realtime apps/api --type ts -g '!*.d.ts' | head -20Repository: BuckyMcYolo/townhall
Length of output: 842
🏁 Script executed:
# Check how packages/auth is exported and if it's forced to load
cat -n packages/auth/src/index.ts | head -50Repository: BuckyMcYolo/townhall
Length of output: 122
🏁 Script executed:
# Check what happens at the top level of these apps - do they import auth?
head -30 apps/worker/src/index.ts
head -30 apps/realtime/src/index.ts
head -30 apps/api/src/index.tsRepository: BuckyMcYolo/townhall
Length of output: 2427
🏁 Script executed:
# Find where `@repo/auth` package is configured
find packages -name "package.json" -type f -exec grep -l '"name".*auth' {} \;Repository: BuckyMcYolo/townhall
Length of output: 90
🏁 Script executed:
# Find the actual auth entry point
find packages -path "*/auth/*" -name "index.ts" -o -path "*/auth/*" -name "index.js"Repository: BuckyMcYolo/townhall
Length of output: 46
🏁 Script executed:
# Check the monorepo structure to understand how packages are resolved
cat -n pnpm-workspace.yaml 2>/dev/null || cat -n tsconfig.json | head -50Repository: BuckyMcYolo/townhall
Length of output: 124
🏁 Script executed:
# Check the auth package.json entry point
cat packages/auth/package.jsonRepository: BuckyMcYolo/townhall
Length of output: 673
🏁 Script executed:
# Look for where auth is actually exported
rg 'export.*auth|export.*from' packages/auth --type ts -g '!*.d.ts' | head -20Repository: BuckyMcYolo/townhall
Length of output: 580
🏁 Script executed:
# Verify the exact startup behavior - when does env validation happen?
rg 'serverSchema.parse|export const env' packages/env --type ts -g '!*.d.ts'Repository: BuckyMcYolo/townhall
Length of output: 208
Making RESEND_API_KEY required breaks deployment for apps that don't use email.
The env schema validates at import time (export const env = serverSchema.parse(process.env)). Making RESEND_API_KEY a required field will cause apps/worker to fail at startup, even though it doesn't use email functionality and never imports @repo/auth. The apps/realtime and apps/api servers also import the env but may not always require email configuration depending on deployment setup.
Consider making RESEND_API_KEY optional with .optional(), or split email-related fields into a separate schema that's only validated where needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/env/src/server.ts` around lines 45 - 46, The server env schema
currently requires RESEND_API_KEY which causes serverSchema.parse(process.env)
to fail on services that don't use email; update the schema so RESEND_API_KEY is
not mandatory (e.g., change RESEND_API_KEY: z.string().min(1) to RESEND_API_KEY:
z.string().min(1).optional()) or refactor by extracting email fields
(RESEND_API_KEY and EMAIL_FROM) into a separate emailSchema and only
parse/validate that schema in modules that actually send email (where
RESEND_API_KEY is needed) while keeping serverSchema.parse(process.env)
non-blocking for non-email apps.
| { | ||
| "$schema": "https://turborepo.dev/schema.json", | ||
| "ui": "tui", | ||
| "ui": "stream-with-experimental-timestamps", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check Turborepo version and verify UI configuration options
# Check package.json files for turborepo version
echo "=== Turborepo version in package.json files ==="
fd -t f '^package\.json$' --exec cat {} \; | jq -r 'select(.devDependencies.turbo or .dependencies.turbo) | {turbo: (.devDependencies.turbo // .dependencies.turbo)}'
# Search for any documentation or comments about this UI option
echo -e "\n=== References to UI configuration in codebase ==="
rg -n "stream-with-experimental-timestamps" -C 2Repository: BuckyMcYolo/townhall
Length of output: 379
🌐 Web query:
What are the valid UI configuration options for Turborepo, and is "stream-with-experimental-timestamps" a stable feature?
💡 Result:
The valid UI configuration options for Turborepo are "stream" (default, non-interactive streamed logs) and "tui" (interactive terminal UI for viewing and interacting with task logs). These can be set in turbo.json under the "ui" key, via the TURBO_UI environment variable, or the --ui CLI flag. "stream-with-experimental-timestamps" is not a valid or documented stable UI option in official Turborepo documentation. It appears in the Turborepo source code (turborepo-types/src/lib.rs) as an enum variant (StreamWithTimestamps), suggesting it may be an internal or experimental feature added in a canary release around late 2024/early 2025 (e.g., v2.7.6-canary.2 changelog mentions "feat: Add stream-with-experimental-timestamps UI mode"). However, it is not listed in current official docs or stable releases as of 2026-03-29, so it is not stable.
Citations:
- 1: https://turborepo.dev/docs/reference/options-overview
- 2: https://turborepo.dev/docs/reference/configuration
- 3: https://fossies.org/linux/www/turborepo-2.8.19.tar.gz/turborepo-2.8.19/crates/turborepo-types/src/lib.rs?M=1169
- 4: https://turborepo.com/docs/reference/options-overview
- 5: https://newreleases.io/project/github/vercel/turborepo/release/v2.7.6-canary.2
Use a documented, stable UI mode instead of the experimental feature.
stream-with-experimental-timestamps is not an officially documented or stable Turborepo UI configuration option. While it appears to exist in canary releases, it is not listed in official documentation and remains experimental. Turborepo v2.8.3 (currently in use) supports the stable options "stream" (default, non-interactive) and "tui" (interactive terminal UI). Switch to one of these documented options to avoid potential breaking changes or unexpected behavior in future Turborepo updates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@turbo.json` at line 3, The Turborepo configuration uses an undocumented
experimental UI value ("stream-with-experimental-timestamps") for the "ui" key;
update that "ui" value to a documented, stable option such as "stream"
(non-interactive) or "tui" (interactive) to avoid breaking changes—locate the
"ui" entry in the turbo.json (the "ui" key and its current value) and replace
the experimental string with either "stream" or "tui" depending on desired
behavior.
page styles, and made pinned messages open the right panel when collapsed
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
apps/realtime/src/index.ts (1)
285-299:⚠️ Potential issue | 🟠 MajorUnread bootstrap still has a race window that can drop updates.
Between Line 288 and Line 299, live unread events that occur after the DB snapshot but before
socket.join(userPresenceRoom)are lost for this socket. This can leave unread counts stale until another event arrives. Please add a cursor/version protocol (bootstrap + live unread/mention events) so stale bootstraps are ignored while still avoiding missed events.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/realtime/src/index.ts` around lines 285 - 299, The bootstrap snapshot from getUnreadStatesForUser and the subsequent socket.emit("notification:bootstrap", ...) can race with live unread/mention events sent after the DB read but before socket.join(userPresenceRoom); fix by adding a monotonic cursor/version: have getUnreadStatesForUser return a snapshot plus a cursor/version token, include that cursor in the bootstrap payload emitted to the client, store it on the socket (e.g., socket.data.unreadCursor), and ensure all live events emitted to the room (notification:unread, notification:mention, etc.) also carry their cursor/version so the client or server-side delivery logic can ignore any event with cursor <= bootstrap cursor; update producers of live unread events to attach the same cursor/version semantics and compare against socket.data.unreadCursor when deciding to deliver to this socket.packages/env/src/server.ts (1)
45-46:⚠️ Potential issue | 🟠 MajorDon't make the shared env parse depend on Resend.
Line 49 parses
serverSchemaeagerly. RequiringRESEND_API_KEYhere forces every process that imports@repo/env/serverto define a mail secret, even if that service never sends email. Keep it optional in this shared schema and validate it next tonew Resend(...)instead.🛠️ Suggested change
- RESEND_API_KEY: z.string().min(1), + RESEND_API_KEY: z.string().min(1).optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/env/src/server.ts` around lines 45 - 46, The shared serverSchema currently requires RESEND_API_KEY and is parsed eagerly, forcing every importer of `@repo/env/server` to provide that secret; change serverSchema to make RESEND_API_KEY optional (or remove it from the required fields) and stop depending on its presence during the shared parse; then perform a dedicated validation (e.g., assert or zod parse) for RESEND_API_KEY right where you construct the Resend client (the new Resend(...) call) so only services that instantiate Resend must supply the key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx`:
- Around line 51-56: The initial step state uses hasUsername computed from
authClient.useSession() at mount, but never updates when session hydrates; add
an effect that watches session (from authClient.useSession()) and recomputes
hasUsername (using MIN_USERNAME_LENGTH) and calls setStep("welcome") when a
valid username exists (and optionally setStep("username") if it becomes invalid)
to keep step in sync with session hydration; reference the existing symbols
session, hasUsername, step, setStep, MIN_USERNAME_LENGTH and implement the logic
inside a useEffect that depends on session.
- Around line 67-109: The username availability flow can be overwritten by stale
network responses; update handleUsernameChange to use a monotonic request ID
(e.g., a useRef like usernameCheckRequestId) that you increment each time you
schedule/check availability, capture the current ID in the async timeout
callback before calling authClient.isUsernameAvailable, and only update
usernameAvailability when the captured ID still matches the ref (ignore response
if it doesn't). Also ensure the request ID ref is reset/managed on clearTimeout
and component unmount so in-flight or old timers cannot flip state incorrectly.
In `@apps/web/src/hooks/use-browser-notifications.ts`:
- Around line 53-61: In onUnread, settings is already guarded by the earlier if
(!settings) return, so remove the unnecessary optional chaining on
settings?.dmNotifications and use settings.dmNotifications for consistency with
onMention; update the conditional that checks payload.guildId === null and
settings?.dmNotifications === "nothing" to reference settings.dmNotifications
instead.
In `@apps/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx:
- Line 19: A pending timeout in useAutoMarkRead can call markChannelRead for the
wrong channel after navigation because channelIdRef is read inside the delayed
callback; fix by capturing the current channelId in the closure when scheduling
the timeout (e.g., const id = channelId) and ensure any existing timer is
cleared on channel change/unmount (clearTimeout(timerRef.current)) before
setting a new one so the delayed callback always uses the captured id and cannot
mark a different channel; update functions/refs in useAutoMarkRead
(channelIdRef, timerRef, markChannelRead) accordingly.
In `@apps/web/src/routes/login.tsx`:
- Around line 39-55: The mutation currently treats the 403 path as success
because mutationFn returns normally; update mutationFn (the async function
passed to mutationFn that calls authClient.signIn.email) to return a
discriminator object like { needsVerification: true } when error.status === 403
instead of just navigating and returning, and then update onSuccess to accept
the mutation result and only call navigate({ to: "/" }) when result is not {
needsVerification: true } (otherwise navigate to "/check-email" there or do
nothing); alternatively you can throw a specific error from mutationFn for the
403 case so onSuccess won't run—reference mutationFn, authClient.signIn.email,
and onSuccess/navigate when making the change.
In `@packages/auth/src/lib/auth.ts`:
- Around line 170-171: The log in auth.ts that calls console.error(`[TOWNHALL
EMAIL] Sending verification email to ${user.email} from ${env.EMAIL_FROM}`)
leaks PII; change it to avoid printing user.email — either log a non-PII
identifier (e.g., user.id or a generated messageId) or redact the address (e.g.,
use user.email.replace(/(.+)@(.+)/,'***@***')). Update the console.error call to
reference the chosen non-PII symbol and keep env.EMAIL_FROM if needed, ensuring
the log message still conveys context without exposing the full email.
---
Duplicate comments:
In `@apps/realtime/src/index.ts`:
- Around line 285-299: The bootstrap snapshot from getUnreadStatesForUser and
the subsequent socket.emit("notification:bootstrap", ...) can race with live
unread/mention events sent after the DB read but before
socket.join(userPresenceRoom); fix by adding a monotonic cursor/version: have
getUnreadStatesForUser return a snapshot plus a cursor/version token, include
that cursor in the bootstrap payload emitted to the client, store it on the
socket (e.g., socket.data.unreadCursor), and ensure all live events emitted to
the room (notification:unread, notification:mention, etc.) also carry their
cursor/version so the client or server-side delivery logic can ignore any event
with cursor <= bootstrap cursor; update producers of live unread events to
attach the same cursor/version semantics and compare against
socket.data.unreadCursor when deciding to deliver to this socket.
In `@packages/env/src/server.ts`:
- Around line 45-46: The shared serverSchema currently requires RESEND_API_KEY
and is parsed eagerly, forcing every importer of `@repo/env/server` to provide
that secret; change serverSchema to make RESEND_API_KEY optional (or remove it
from the required fields) and stop depending on its presence during the shared
parse; then perform a dedicated validation (e.g., assert or zod parse) for
RESEND_API_KEY right where you construct the Resend client (the new Resend(...)
call) so only services that instantiate Resend must supply the key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3dded95b-27e6-49e5-9eee-ed70542c21d1
📒 Files selected for processing (9)
apps/realtime/src/index.tsapps/web/src/components/onboarding/onboarding-dialog.tsxapps/web/src/hooks/use-browser-notifications.tsapps/web/src/routes/_authenticated/$guildSlug/$channelId.tsxapps/web/src/routes/check-email.tsxapps/web/src/routes/login.tsxapps/web/src/routes/signup.tsxpackages/auth/src/lib/auth.tspackages/env/src/server.ts
Comment on lines
+51
to
+56
| const { data: session } = authClient.useSession() | ||
| const hasUsername = !!( | ||
| session?.user?.username && | ||
| session.user.username.length >= MIN_USERNAME_LENGTH | ||
| ) | ||
| const [step, setStep] = useState<Step>(hasUsername ? "welcome" : "username") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/web/src/components/onboarding/onboarding-dialog.tsx | head -130Repository: BuckyMcYolo/townhall
Length of output: 4917
🏁 Script executed:
# Search for authClient.useSession usage pattern in the codebase
rg "useSession" --type ts --type tsx -B 2 -A 2 | head -50Repository: BuckyMcYolo/townhall
Length of output: 91
🏁 Script executed:
# Find the authClient implementation to understand useSession behavior
fd -t f -name "*auth*" | head -20Repository: BuckyMcYolo/townhall
Length of output: 294
🏁 Script executed:
# Find authClient definition
rg "export.*useSession|function useSession" --type ts --type tsxRepository: BuckyMcYolo/townhall
Length of output: 91
🏁 Script executed:
# Read more of the component to see all useEffect hooks
tail -n +73 apps/web/src/components/onboarding/onboarding-dialog.tsx | head -200Repository: BuckyMcYolo/townhall
Length of output: 6010
🏁 Script executed:
# Search for authClient package location
fd "auth" | grep -E "(client|index)" | head -20Repository: BuckyMcYolo/townhall
Length of output: 100
🏁 Script executed:
cat packages/auth/src/lib/auth-client.tsRepository: BuckyMcYolo/townhall
Length of output: 874
🏁 Script executed:
# Search for any useEffect that might sync step with session or hasUsername
grep -n "useEffect\|step\|session" apps/web/src/components/onboarding/onboarding-dialog.tsx | head -40Repository: BuckyMcYolo/townhall
Length of output: 597
🏁 Script executed:
# Check if there's any other handling of session changes for step
rg "setStep" apps/web/src/components/onboarding/onboarding-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 351
🏁 Script executed:
# Check the full component for any conditional step logic after mount
sed -n '50,150p' apps/web/src/components/onboarding/onboarding-dialog.tsx | cat -nRepository: BuckyMcYolo/townhall
Length of output: 3850
Sync step after session hydration.
When authClient.useSession() hydrates after mount, hasUsername recalculates on re-render but step remains at its initialized value. Users with existing usernames stay on the "username" step instead of advancing to "welcome", causing unwanted rename prompts.
Suggested fix
const [step, setStep] = useState<Step>(hasUsername ? "welcome" : "username")
+
+ useEffect(() => {
+ if (hasUsername) {
+ setStep((current) => (current === "username" ? "welcome" : current))
+ }
+ }, [hasUsername])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx` around lines 51 -
56, The initial step state uses hasUsername computed from
authClient.useSession() at mount, but never updates when session hydrates; add
an effect that watches session (from authClient.useSession()) and recomputes
hasUsername (using MIN_USERNAME_LENGTH) and calls setStep("welcome") when a
valid username exists (and optionally setStep("username") if it becomes invalid)
to keep step in sync with session hydration; reference the existing symbols
session, hasUsername, step, setStep, MIN_USERNAME_LENGTH and implement the logic
inside a useEffect that depends on session.
Comment on lines
+67
to
+109
| const [username, setUsername] = useState("") | ||
| const [usernameAvailability, setUsernameAvailability] = useState< | ||
| "idle" | "checking" | "available" | "taken" | "invalid" | ||
| >("idle") | ||
| const usernameCheckTimer = useRef<ReturnType<typeof setTimeout> | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| return () => { | ||
| if (usernameCheckTimer.current) clearTimeout(usernameCheckTimer.current) | ||
| } | ||
| }, []) | ||
|
|
||
| const handleUsernameChange = useCallback((value: string) => { | ||
| setUsername(value) | ||
| if (usernameCheckTimer.current) clearTimeout(usernameCheckTimer.current) | ||
|
|
||
| const trimmed = value.trim() | ||
| if (!trimmed) { | ||
| setUsernameAvailability("idle") | ||
| return | ||
| } | ||
| if ( | ||
| trimmed.length < MIN_USERNAME_LENGTH || | ||
| trimmed.length > MAX_USERNAME_LENGTH || | ||
| !USERNAME_REGEX.test(trimmed) | ||
| ) { | ||
| setUsernameAvailability("invalid") | ||
| return | ||
| } | ||
|
|
||
| setUsernameAvailability("checking") | ||
| usernameCheckTimer.current = setTimeout(async () => { | ||
| try { | ||
| const { data } = await authClient.isUsernameAvailable({ | ||
| username: trimmed, | ||
| }) | ||
| setUsernameAvailability((prev) => | ||
| prev === "checking" ? (data?.available ? "available" : "taken") : prev | ||
| ) | ||
| } catch { | ||
| setUsernameAvailability((prev) => (prev === "checking" ? "idle" : prev)) | ||
| } | ||
| }, 500) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "onboarding-dialog.tsx" --full-pathRepository: BuckyMcYolo/townhall
Length of output: 120
🏁 Script executed:
wc -l apps/web/src/components/onboarding/onboarding-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 124
🏁 Script executed:
sed -n '60,120p' apps/web/src/components/onboarding/onboarding-dialog.tsx | cat -nRepository: BuckyMcYolo/townhall
Length of output: 2417
🏁 Script executed:
sed -n '67,109p' apps/web/src/components/onboarding/onboarding-dialog.tsx | cat -nRepository: BuckyMcYolo/townhall
Length of output: 1711
🏁 Script executed:
head -20 apps/web/src/components/onboarding/onboarding-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 775
🏁 Script executed:
sed -n '67,109p' apps/web/src/components/onboarding/onboarding-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 1410
🏁 Script executed:
cat -n apps/web/src/components/onboarding/onboarding-dialog.tsx | sed -n '100,110p'Repository: BuckyMcYolo/townhall
Length of output: 503
🏁 Script executed:
sed -n '98,110p' apps/web/src/components/onboarding/onboarding-dialog.tsx | cat -ARepository: BuckyMcYolo/townhall
Length of output: 509
Ignore stale availability responses.
clearTimeout only cancels checks not yet started. After the network request at line 100 is in flight, an older response can still resolve while a newer username is also "checking", because lines 103-107 guard off the status rather than the specific request. This can display the wrong availability state and briefly enable submit for an unchecked username.
The suggested fix using a request ID correctly prevents stale responses from overwriting newer ones.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx` around lines 67 -
109, The username availability flow can be overwritten by stale network
responses; update handleUsernameChange to use a monotonic request ID (e.g., a
useRef like usernameCheckRequestId) that you increment each time you
schedule/check availability, capture the current ID in the async timeout
callback before calling authClient.isUsernameAvailable, and only update
usernameAvailability when the captured ID still matches the ref (ignore response
if it doesn't). Also ensure the request ID ref is reset/managed on clearTimeout
and component unmount so in-flight or old timers cannot flip state incorrectly.
| import { TypingIndicator } from "@/components/chat/typing-indicator" | ||
| import { useRightSidebar } from "@/components/sidebar/right-panel/right-sidebar-context" | ||
| import { useSocket } from "@/context/socket-context" | ||
| import { useAutoMarkRead } from "@/hooks/use-auto-mark-read" |
There was a problem hiding this comment.
Pending debounce from previous channel can mark the next channel as read
Line 57 wires useAutoMarkRead(channelId), but apps/web/src/hooks/use-auto-mark-read.ts currently uses a mutable channelIdRef inside the timeout callback. A timer scheduled in channel A can fire after navigation and call markChannelRead for channel B, causing incorrect unread state.
Proposed fix (in apps/web/src/hooks/use-auto-mark-read.ts)
- const channelIdRef = useRef(channelId)
- channelIdRef.current = channelId
-
- const debouncedMarkRead = () => {
- if (!channelIdRef.current) return
+ const debouncedMarkRead = (targetChannelId: string) => {
if (timerRef.current) clearTimeout(timerRef.current)
timerRef.current = setTimeout(() => {
- if (channelIdRef.current) {
- markChannelRead(channelIdRef.current)
- }
+ markChannelRead(targetChannelId)
}, DEBOUNCE_MS)
}
useEffect(() => {
if (!channelId) return
if (document.visibilityState !== "visible") return
- debouncedMarkRead()
+ debouncedMarkRead(channelId)
return () => {
if (timerRef.current) clearTimeout(timerRef.current)
}
}, [channelId])
useEffect(() => {
if (!channelId) return
const onVisibilityChange = () => {
if (document.visibilityState === "visible") {
- debouncedMarkRead()
+ debouncedMarkRead(channelId)
}
}
document.addEventListener("visibilitychange", onVisibilityChange)
return () => document.removeEventListener("visibilitychange", onVisibilityChange)
}, [channelId])
useEffect(() => {
if (!socket || !channelId) return
const onMessageCreated = (message: RealtimeMessage) => {
- if (message.channelId === channelIdRef.current && document.visibilityState === "visible") {
- debouncedMarkRead()
+ if (message.channelId === channelId && document.visibilityState === "visible") {
+ debouncedMarkRead(channelId)
}
}
socket.on("message:created", onMessageCreated)
return () => socket.off("message:created", onMessageCreated)
}, [socket, channelId])Also applies to: 57-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/routes/_authenticated/`$guildSlug/$channelId.tsx at line 19, A
pending timeout in useAutoMarkRead can call markChannelRead for the wrong
channel after navigation because channelIdRef is read inside the delayed
callback; fix by capturing the current channelId in the closure when scheduling
the timeout (e.g., const id = channelId) and ensure any existing timer is
cleared on channel change/unmount (clearTimeout(timerRef.current)) before
setting a new one so the delayed callback always uses the captured id and cannot
mark a different channel; update functions/refs in useAutoMarkRead
(channelIdRef, timerRef, markChannelRead) accordingly.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
notifications, and Tauri app setup
Notification System with Unread Indicators and Desktop/Browser Support
This PR implements a full-stack notification system with unread/mention indicators, browser notifications, and Tauri desktop notification support. It spans DB schema, API endpoints, realtime bootstrapping, frontend state and UI, Tauri integration, and email delivery integration.
Key Features Added
notification:bootstrapbefore joining presence room.notification:mentionandnotification:unread, respects NotificationSettings, suppresses when document is visible, and deduplicates via tags.notification:default.desktop.tauri://localhost.Notable Code/Behavior Changes
Deployment / Operational Notes
Potential Issues / Recommendations
Confidence Score: 3/5
The PR is well-scoped and integrates end-to-end (DB, API, realtime, frontend, desktop, and email). Primary concerns before merging are operational: ensure DB migration is applied, provide RESEND_API_KEY, and add test coverage for critical notification flows. Minor error-handling and runtime edge cases (dynamic Tauri import failures, permission edge cases) remain to be hardened.