Dev#51
Merged
BuckyMcYolo merged 10 commits intomainfrom Apr 9, 2026
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 19 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds Tauri process plugin and permission, bumps desktop app version, introduces a self‑hosted env flag, integrates native updater+relaunch flow, enriches channel/category create/edit/delete UX (including DnD changes), adds a reusable Checkbox component, and creates an auto‑tag workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp
participant TauriUpdater as Tauri Updater
participant TauriProcess as Tauri Process
User->>WebApp: Click "Update" / call refresh()
WebApp->>WebApp: set isInstalling = true
alt Running under Tauri and updater cached
WebApp->>TauriUpdater: downloadAndInstall()
TauriUpdater-->>WebApp: resolved
WebApp->>TauriProcess: import plugin & relaunch()
TauriProcess-->>WebApp: relaunch initiated
WebApp-->>User: app relaunches
else Web / no updater
WebApp-->>User: window.location.reload()
end
WebApp->>WebApp: set isInstalling = false
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/hooks/use-update-check.ts (1)
17-23:⚠️ Potential issue | 🟠 MajorSeparate web updates from native-installable updates.
Lines 17-23 and Lines 31-39 both raise the same
updateAvailableflag, but only Lines 58-63 can actually install a native update. In a Tauri session with only a webbuildIdmismatch,apps/web/src/routes/_authenticated.tsxLines 36-45 will still show an Update CTA even thoughrefresh()can only reload. Return distinct web/native update state (or acanInstallNativeUpdateflag) so the banner can present the correct action.Also applies to: 31-39, 56-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/hooks/use-update-check.ts` around lines 17 - 23, The hook currently sets a single updateAvailable flag from both web and native checks (see checkWeb and the native update check), which prevents the UI from distinguishing reload-only web updates from native-installable updates; change the state to two distinct flags (e.g., webUpdateAvailable and nativeUpdateAvailable or a canInstallNativeUpdate boolean), update checkWeb to only set webUpdateAvailable when buildId !== __BUILD_ID__, and update the native update check (the code that currently sets updateAvailable when a native update is detected) to only set nativeUpdateAvailable/canInstallNativeUpdate; keep refresh() behavior tied to webUpdateAvailable (reload) and ensure the banner/routes use the new flags to show the correct CTA for reload vs install.
🤖 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 193-197: Extract the duplicated fire-and-forget invite accept flow
into a single helper (e.g., acceptTownhallInvite) that calls
apiClient.v1.invites[":code"].accept.$post({ param: { code: TOWNHALL_INVITE_CODE
} }), then invalidates the guilds cache via queryClient.invalidateQueries({
queryKey: ["guilds"] }) and preserves the current swallowed-error behavior
(.catch(() => {})); replace the duplicated blocks in the join flow (where
joinTownhall is used) and the create flow with calls to this helper so the
invite code, invalidation, and fallback handling remain centralized.
- Around line 398-418: The card is a clickable <button> wrapping an interactive
Checkbox which causes nested-button HTML issues and double toggles; change the
outer element used around Checkbox from a <button> to a non-button interactive
container (e.g., <div role="button" tabIndex={0}>) and keep the click handler
that calls setJoinTownhall(prev => !prev), also add keyboard handlers (onKeyDown
handling Enter/Space) to preserve accessibility; additionally, prevent events
from bubbling from the Checkbox by adding an onClick={(e) =>
e.stopPropagation()} on the Checkbox (while keeping its onCheckedChange={checked
=> setJoinTownhall(checked === true)}) so toggling the Checkbox doesn't also
trigger the container's toggle.
In `@apps/web/src/components/sidebar/channel-panel/channel-list.tsx`:
- Around line 226-245: handleDragOver is optimistically reordering category
arrays via queryClient.setQueryData when activeItem.isCategory &&
overItem.isCategory, and handleDragEnd is also applying a reorder which causes
double-mutation (e.g., A -> over C becomes B, A, C). Fix by centralizing
category reordering to a single phase: either remove the categories mutation
from handleDragOver (the block that checks activeItem.isCategory &&
overItem.isCategory) or add a guard/flag on the drag session (e.g., on the drag
state or payload) so handleDragEnd skips reapplying if setQueryData already
moved the category; update code paths that reference newData.categories,
active.id, over.id, and queryClient.setQueryData to use the single chosen
strategy consistently.
- Around line 588-592: The object currently created as categoryAsChannel is
being force-cast to Channel even though it lacks many Channel fields; instead
define a proper discriminated union or shared interface (e.g., type MinimalItem
= { id: string; name: string; type: "category" | "channel" } or union Channel |
CategoryMinimal) and use that type for categoryAsChannel (and the other
occurrence) rather than "as Channel"; then update consumers like
EditChannelDialog props (or the place reading channel.topic) to accept the union
or to narrow/check type/typeguard before accessing Channel-only fields (or
provide safe defaults), so you remove the unsafe cast and preserve correct
typing between Category and Channel.
In `@apps/web/src/components/sidebar/channel-panel/create-channel-dialog.tsx`:
- Around line 61-65: The submitted channel name is being transformed differently
than the preview (preview uses normalizedName while the payload builds name from
trimmed.toLowerCase().replace(/\s+/g, "-")), causing mismatches for punctuation;
fix by computing a single normalizedName once (using the same rules currently
used for preview) and use that normalizedName for the submitted payload (i.e.,
set name: isCategory ? trimmed : normalizedName) and anywhere the preview
displays it so both UI and server use identical normalization; update references
to trimmed, isCategory, parentId accordingly to preserve existing
category/parent logic.
In `@apps/web/src/hooks/use-update-check.ts`:
- Around line 31-42: The checkTauri function leaves tauriUpdateRef.current stale
when no update is found or an error occurs; modify checkTauri so that when the
imported check() returns null/undefined you explicitly clear
tauriUpdateRef.current and setUpdateAvailable(false), and likewise in the catch
block clear tauriUpdateRef.current and setUpdateAvailable(false) so
refresh()/downloadAndInstall() cannot use stale updater metadata; locate these
changes in the checkTauri async function and update the branches that handle the
null-return path and the catch path.
In `@packages/env/src/client.ts`:
- Line 24: The env schema uses z.coerce.boolean() for NEXT_PUBLIC_SELF_HOSTED
which will parse the string "false" as true; replace that coercion with a
preprocessing boolean parser: use z.preprocess to map string inputs (e.g.,
normalize toLowerCase and treat "true" or "1" as true, "false" or "0" as false)
then validate with z.boolean(), and apply the same change to the other boolean
entry referenced on line 33 so both NEXT_PUBLIC_SELF_HOSTED and the other env
key use z.preprocess(..., z.boolean()) instead of z.coerce.boolean().
In `@packages/env/src/server.ts`:
- Line 36: The NEXT_PUBLIC_SELF_HOSTED env entry currently uses
z.coerce.boolean() which treats any non-empty string (e.g. "false") as true;
change the schema for NEXT_PUBLIC_SELF_HOSTED to parse strings explicitly (or
use z.stringbool() if on Zod v4) so that string "false"/"0"/"" map to false and
"true"/"1" map to true, e.g. replace the z.coerce.boolean() usage in the
NEXT_PUBLIC_SELF_HOSTED declaration with a preprocessing/string-parse approach
that lowercases the input and returns a proper boolean before validating with
z.boolean(), and keep the .default(false) behavior.
---
Outside diff comments:
In `@apps/web/src/hooks/use-update-check.ts`:
- Around line 17-23: The hook currently sets a single updateAvailable flag from
both web and native checks (see checkWeb and the native update check), which
prevents the UI from distinguishing reload-only web updates from
native-installable updates; change the state to two distinct flags (e.g.,
webUpdateAvailable and nativeUpdateAvailable or a canInstallNativeUpdate
boolean), update checkWeb to only set webUpdateAvailable when buildId !==
__BUILD_ID__, and update the native update check (the code that currently sets
updateAvailable when a native update is detected) to only set
nativeUpdateAvailable/canInstallNativeUpdate; keep refresh() behavior tied to
webUpdateAvailable (reload) and ensure the banner/routes use the new flags to
show the correct CTA for reload vs install.
🪄 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: 19a29b35-184a-4dc1-877f-88f2aa552b1f
⛔ 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 (16)
apps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/capabilities/default.jsonapps/desktop/src-tauri/src/lib.rsapps/desktop/src-tauri/tauri.conf.jsonapps/web/package.jsonapps/web/src/components/guild/guild-settings-dialog.tsxapps/web/src/components/onboarding/onboarding-dialog.tsxapps/web/src/components/sidebar/channel-panel/channel-list.tsxapps/web/src/components/sidebar/channel-panel/create-channel-dialog.tsxapps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsxapps/web/src/hooks/use-update-check.tsapps/web/src/main.tsxapps/web/src/routes/_authenticated.tsxpackages/env/src/client.tspackages/env/src/server.tspackages/ui/src/components/checkbox.tsx
💤 Files with no reviewable changes (2)
- apps/web/src/components/guild/guild-settings-dialog.tsx
- apps/web/src/main.tsx
Comment on lines
+588
to
+592
| const categoryAsChannel = { | ||
| id, | ||
| name, | ||
| type: "category" as const, | ||
| } as Channel |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
apps/web/src/lib/api-types.ts \
apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx \
apps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsx
do
echo "===== $f ====="
nl -ba "$f" | sed -n '1,220p'
echo
done
echo "===== direct channel field access in dialogs ====="
rg -n 'channel\.' \
apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx \
apps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# View api-types.ts to see Category and Channel definitions
echo "===== apps/web/src/lib/api-types.ts (lines 1-50) ====="
sed -n '1,50p' apps/web/src/lib/api-types.ts
echo ""
echo "===== channel-list.tsx (lines 580-600) ====="
sed -n '580,600p' apps/web/src/components/sidebar/channel-panel/channel-list.tsx
echo ""
echo "===== channel-list.tsx (lines 650-675) ====="
sed -n '650,675p' apps/web/src/components/sidebar/channel-panel/channel-list.tsxRepository: BuckyMcYolo/townhall
Length of output: 3053
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== edit-channel-dialog.tsx (first 200 lines) ====="
sed -n '1,200p' apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx
echo ""
echo "===== delete-channel-dialog.tsx (first 150 lines) ====="
sed -n '1,150p' apps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 7594
🏁 Script executed:
#!/bin/bash
set -euoripefail
# Look at the actual channel types from the API
rg -n "topic" apps/web/src/lib/api-types.ts
# Also check what fields are accessed on channel objects in the codebase
echo ""
echo "===== Fields accessed on 'channel' object in dialogs ====="
rg -n 'channel\.[a-zA-Z]' \
apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx \
apps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 738
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look at the actual channel types from the API
rg -n "topic" apps/web/src/lib/api-types.ts
# Also check what fields are accessed on channel objects in the dialogs
echo ""
echo "===== Fields accessed on 'channel' object in dialogs ====="
rg -n 'channel\.[a-zA-Z]' \
apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx \
apps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsxRepository: BuckyMcYolo/townhall
Length of output: 46
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try simpler search
echo "===== Searching for 'channel.' field access ====="
grep -n "channel\." apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx
echo ""
grep -n "channel\." apps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsx
echo ""
echo "===== Check for topic field definition in api response ====="
cat apps/web/src/lib/api-types.ts | head -100Repository: BuckyMcYolo/townhall
Length of output: 3993
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the second location (660-668) is indeed similar usage
sed -n '655,670p' apps/web/src/components/sidebar/channel-panel/channel-list.tsxRepository: BuckyMcYolo/townhall
Length of output: 473
Use a union type or shared interface instead of casting a partial object as Channel.
EditChannelDialog reads channel.topic (lines 33, 38, 40, 93 in edit-channel-dialog.tsx), but the categoryAsChannel object created here only includes id, name, and type. The cast as Channel masks this incompleteness from TypeScript's type checker. While the code works at runtime thanks to the ?? "" fallback, the type safety violation makes the dialogs' dependency on missing fields invisible.
Refactor to a proper union type (Channel | CategoryWithChannels) or a shared interface ({ id; name; type }) that accurately reflects what both categories and channels can be, preserving the architectural distinction between these types.
Also applies to: 660-668
🤖 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
588 - 592, The object currently created as categoryAsChannel is being force-cast
to Channel even though it lacks many Channel fields; instead define a proper
discriminated union or shared interface (e.g., type MinimalItem = { id: string;
name: string; type: "category" | "channel" } or union Channel | CategoryMinimal)
and use that type for categoryAsChannel (and the other occurrence) rather than
"as Channel"; then update consumers like EditChannelDialog props (or the place
reading channel.topic) to accept the union or to narrow/check type/typeguard
before accessing Channel-only fields (or provide safe defaults), so you remove
the unsafe cast and preserve correct typing between Category and Channel.
Comment on lines
+31
to
+42
| const checkTauri = async () => { | ||
| if (!isTauri) return | ||
| try { | ||
| const { check } = await import("@tauri-apps/plugin-updater") | ||
| const update = await check() | ||
| if (update) { | ||
| tauriUpdateRef.current = update | ||
| setUpdateAvailable(true) | ||
| } | ||
| } catch { | ||
| // Updater not available or check failed, ignore | ||
| } |
There was a problem hiding this comment.
Clear the cached updater when the native check finds nothing.
After a successful poll, tauriUpdateRef.current is never reset when Line 35 returns null or Lines 40-42 catch. That leaves refresh() free to call downloadAndInstall() with stale updater metadata on a later click.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/hooks/use-update-check.ts` around lines 31 - 42, The checkTauri
function leaves tauriUpdateRef.current stale when no update is found or an error
occurs; modify checkTauri so that when the imported check() returns
null/undefined you explicitly clear tauriUpdateRef.current and
setUpdateAvailable(false), and likewise in the catch block clear
tauriUpdateRef.current and setUpdateAvailable(false) so
refresh()/downloadAndInstall() cannot use stale updater metadata; locate these
changes in the checkTauri async function and update the branches that handle the
null-return path and the catch path.
normalization, drag-drop double mutation - Fix z.coerce.boolean() treating "false" as true for NEXT_PUBLIC_SELF_HOSTED - Extract duplicated Townhall invite accept into shared helper - Fix nested button causing double toggle on onboarding checkbox - Use consistent normalizedName for channel creation preview and submit - Remove redundant category reorder from handleDragEnd (already handled in handleDragOver)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/web/src/components/onboarding/onboarding-dialog.tsx (1)
399-420:⚠️ Potential issue | 🟠 MajorAssociate the checkbox text with the control and drop the mouse-only wrapper toggle.
The clickable
divis not keyboard reachable, and theCheckboxhas no accessible label right now. Give the checkbox anidand bind the copy with<Label htmlFor>so the text remains clickable without relying on a non-semantic container.Suggested fix
- {showTownhallJoin && ( - // biome-ignore lint/a11y/useKeyWithClickEvents: Radix Checkbox handles keyboard a11y - <div - onClick={() => setJoinTownhall((prev) => !prev)} - className="mt-4 flex w-full cursor-pointer items-start gap-3 rounded-lg border border-border bg-card p-4 text-left transition-colors hover:bg-accent" - > + {showTownhallJoin && ( + <div className="mt-4 flex w-full items-start gap-3 rounded-lg border border-border bg-card p-4"> <Checkbox + id="join-townhall" checked={joinTownhall} onCheckedChange={(checked) => setJoinTownhall(checked === true) } - onClick={(e) => e.stopPropagation()} className="mt-0.5" /> - <div className="select-none"> + <Label + htmlFor="join-townhall" + className="cursor-pointer select-none" + > <p className="font-medium">Join Townhall's Townhall</p> <p className="text-sm text-muted-foreground"> Join our open-source community and help shape the product roadmap </p> - </div> + </Label> </div> )}🤖 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 399 - 420, The current keyboard-inaccessible wrapper toggle around the Checkbox (controlled by showTownhallJoin, joinTownhall, setJoinTownhall) should be replaced by associating the text with the control: give the Checkbox a stable id (e.g., townhallCheckboxId), remove the non-semantic div onClick toggle, and wrap the text copy in a semantic <Label htmlFor={id}> so clicking the label toggles the Checkbox and remains keyboard-accessible; keep the Checkbox props (checked, onCheckedChange) and remove or adjust the onClick stopPropagation if it blocks label interaction.packages/env/src/server.ts (1)
36-38:⚠️ Potential issue | 🟠 MajorThis still masks invalid boolean env values.
Like the client schema, this treats every non-
"true"/"1"value asfalse, so"TRUE"," false", or any typo silently change behavior instead of surfacing a bad deploy config. Use a normalized parser that only maps recognized literals and lets unknown values fail validation.Suggested fix
+const parseEnvBoolean = z.preprocess((value) => { + if (typeof value !== "string") return value + const normalized = value.trim().toLowerCase() + if (normalized === "true" || normalized === "1") return true + if (normalized === "false" || normalized === "0" || normalized === "") return false + return value +}, z.boolean()) + const serverSchema = z.object({ @@ - NEXT_PUBLIC_SELF_HOSTED: z - .preprocess((v) => v === "true" || v === "1", z.boolean()) - .default(false), + NEXT_PUBLIC_SELF_HOSTED: parseEnvBoolean.default(false),🤖 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 36 - 38, The NEXT_PUBLIC_SELF_HOSTED schema currently treats any non-"true"/"1" input as false and thus silently accepts invalid values; update the z.preprocess for NEXT_PUBLIC_SELF_HOSTED to first coerce the raw value to a trimmed lowercase string, then explicitly map "true" and "1" to true and "false" and "0" to false, and return the original raw value (or throw) for any other literal so Zod's boolean validation fails — modify the preprocess callback used with NEXT_PUBLIC_SELF_HOSTED to perform this strict normalization and allow unknown values to surface as validation errors.packages/env/src/client.ts (1)
24-26:⚠️ Potential issue | 🟠 MajorDon't silently coerce unknown env values to
false.This parser turns every non-
"true"/"1"input intofalse, so values like"TRUE"or"false "quietly disable self-hosted mode instead of failing fast. Normalize the string and let unknown values fall through soz.boolean()rejects bad config.Suggested fix
+const parseEnvBoolean = z.preprocess((value) => { + if (typeof value !== "string") return value + const normalized = value.trim().toLowerCase() + if (normalized === "true" || normalized === "1") return true + if (normalized === "false" || normalized === "0" || normalized === "") return false + return value +}, z.boolean()) + const clientSchema = z.object({ @@ - NEXT_PUBLIC_SELF_HOSTED: z - .preprocess((v) => v === "true" || v === "1", z.boolean()) - .default(false), + NEXT_PUBLIC_SELF_HOSTED: parseEnvBoolean.default(false),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/env/src/client.ts` around lines 24 - 26, The preprocess for NEXT_PUBLIC_SELF_HOSTED currently coerces any non-"true"/"1" value to false; instead normalize string input (trim() and toLowerCase()), map only explicit "true"/"1" to true and "false"/"0" to false, and otherwise return the original value so z.boolean() can reject invalid values; update the preprocess used on NEXT_PUBLIC_SELF_HOSTED (the .preprocess callback alongside z.boolean()) to perform this normalization and selective mapping rather than defaulting all other inputs to false.
🤖 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/sidebar/channel-panel/channel-list.tsx`:
- Around line 650-654: EditChannelDialog currently renders and reads
channel.topic even for category channels; update EditChannelDialog to check
channel.type (e.g., channel.type === "category") and hide/omit the topic input
for categories, avoid reading channel.topic for categories (use a safe default
or skip initializing topic state when channel.type === "category"), and mirror
the pattern used by DeleteChannelDialog to ensure categories do not show a topic
field or attempt topic updates.
In `@apps/web/src/components/sidebar/channel-panel/create-channel-dialog.tsx`:
- Line 52: The submit currently silently returns when normalizedName is empty
(e.g., user types "!!!") because of the early return "if (forceType !==
'category' && !normalizedName) return"; update the form to prevent the button
from being enabled and provide user feedback instead: compute a disabled
condition for the Create button that includes (forceType !== 'category' &&
!normalizedName), and in the submit handler (the block using normalizedName and
the early return) replace the silent return with a validation error state or
message so users see why submission failed; apply the same change to the second
usage referenced around lines 173-176 so both entry points use the
normalizedName check and show validation feedback rather than silently
returning.
---
Duplicate comments:
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx`:
- Around line 399-420: The current keyboard-inaccessible wrapper toggle around
the Checkbox (controlled by showTownhallJoin, joinTownhall, setJoinTownhall)
should be replaced by associating the text with the control: give the Checkbox a
stable id (e.g., townhallCheckboxId), remove the non-semantic div onClick
toggle, and wrap the text copy in a semantic <Label htmlFor={id}> so clicking
the label toggles the Checkbox and remains keyboard-accessible; keep the
Checkbox props (checked, onCheckedChange) and remove or adjust the onClick
stopPropagation if it blocks label interaction.
In `@packages/env/src/client.ts`:
- Around line 24-26: The preprocess for NEXT_PUBLIC_SELF_HOSTED currently
coerces any non-"true"/"1" value to false; instead normalize string input
(trim() and toLowerCase()), map only explicit "true"/"1" to true and "false"/"0"
to false, and otherwise return the original value so z.boolean() can reject
invalid values; update the preprocess used on NEXT_PUBLIC_SELF_HOSTED (the
.preprocess callback alongside z.boolean()) to perform this normalization and
selective mapping rather than defaulting all other inputs to false.
In `@packages/env/src/server.ts`:
- Around line 36-38: The NEXT_PUBLIC_SELF_HOSTED schema currently treats any
non-"true"/"1" input as false and thus silently accepts invalid values; update
the z.preprocess for NEXT_PUBLIC_SELF_HOSTED to first coerce the raw value to a
trimmed lowercase string, then explicitly map "true" and "1" to true and "false"
and "0" to false, and return the original raw value (or throw) for any other
literal so Zod's boolean validation fails — modify the preprocess callback used
with NEXT_PUBLIC_SELF_HOSTED to perform this strict normalization and allow
unknown values to surface as validation errors.
🪄 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: e9864a09-d007-406a-8088-4085e70a3c5e
📒 Files selected for processing (5)
apps/web/src/components/onboarding/onboarding-dialog.tsxapps/web/src/components/sidebar/channel-panel/channel-list.tsxapps/web/src/components/sidebar/channel-panel/create-channel-dialog.tsxpackages/env/src/client.tspackages/env/src/server.ts
Comment on lines
+650
to
+654
| <EditChannelDialog | ||
| channel={categoryAsChannel} | ||
| open={editOpen} | ||
| onOpenChange={setEditOpen} | ||
| /> |
There was a problem hiding this comment.
EditChannelDialog displays a topic field for categories, which is inappropriate.Categories in Discord are organizational containers used to group channels into sections, and they manage permissions rather than content like topics. Unlike text channels, categories don't have a "topic" field.
From the relevant snippet, EditChannelDialog renders a topic input field (line 93) and accesses channel.topic (lines 33, 38, 40). When a user clicks "Edit Category", they'll see an irrelevant topic field, which is confusing UX.
Consider adding type-specific handling in EditChannelDialog to hide the topic field for categories, similar to how DeleteChannelDialog already checks channel.type === "category".
🤖 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
650 - 654, EditChannelDialog currently renders and reads channel.topic even for
category channels; update EditChannelDialog to check channel.type (e.g.,
channel.type === "category") and hide/omit the topic input for categories, avoid
reading channel.topic for categories (use a safe default or skip initializing
topic state when channel.type === "category"), and mirror the pattern used by
DeleteChannelDialog to ensure categories do not show a topic field or attempt
topic updates.
| e.preventDefault() | ||
| const trimmed = name.trim() | ||
| if (!trimmed || !guildSlug) return | ||
| if (forceType !== "category" && !normalizedName) return |
There was a problem hiding this comment.
Silent failure when channel name normalizes to empty string.
If a user enters a name like "!!!" for a channel, name.trim() is truthy so the button is enabled, but normalizedName becomes empty. The form silently returns with no error feedback.
Consider disabling the submit button when normalizedName would be empty for non-category channels:
💡 Suggested fix
<Button
type="submit"
className="w-full"
- disabled={loading || !name.trim()}
+ disabled={loading || !name.trim() || (forceType !== "category" && !normalizedName)}
>Also applies to: 173-176
🤖 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/create-channel-dialog.tsx` at
line 52, The submit currently silently returns when normalizedName is empty
(e.g., user types "!!!") because of the early return "if (forceType !==
'category' && !normalizedName) return"; update the form to prevent the button
from being enabled and provide user feedback instead: compute a disabled
condition for the Create button that includes (forceType !== 'category' &&
!normalizedName), and in the submit handler (the block using normalizedName and
the early return) replace the silent return with a validation error state or
message so users see why submission failed; apply the same change to the second
usage referenced around lines 173-176 so both entry points use the
normalizedName check and show validation feedback rather than silently
returning.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/web/src/components/sidebar/channel-panel/create-channel-dialog.tsx (1)
52-52:⚠️ Potential issue | 🟡 MinorSurface invalid normalized names instead of silently returning.
The disabled button closes the main click path, but Line 52 still bails out without
setError(...). Non-click submit paths can still no-op with no feedback when the input normalizes to an empty channel name.💡 Proposed fix
const trimmed = name.trim() if (!trimmed || !guildSlug) return - if (forceType !== "category" && !normalizedName) return + if (forceType !== "category" && !normalizedName) { + setError("Channel names must include at least one letter or number") + return + } setError(null) setLoading(true)Also applies to: 176-178
🤖 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/create-channel-dialog.tsx` at line 52, The early return "if (forceType !== 'category' && !normalizedName) return" silently aborts submission; update the submit/validation logic in create-channel-dialog.tsx so that when normalizedName is empty (and forceType !== "category") it calls setError(...) with a clear message instead of returning; change the same pattern at the other occurrence (lines ~176-178) to use the same setError flow and prevent silent no-ops, ensuring any submit path (click or non-click) surfaces the validation error to the user.
🤖 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 399-420: The card visually indicates it's clickable but only the
Checkbox/Label are interactive; make the whole card truly interactive by adding
an onClick handler to the outer div that toggles joinTownhall via
setJoinTownhall(!joinTownhall) while keeping the current classNames
(cursor-pointer and hover:bg-accent), and ensure the Checkbox itself prevents
event bubbling by adding an onClick that calls e.stopPropagation() (leave
Checkbox's onCheckedChange using setJoinTownhall(checked === true)); reference
showTownhallJoin, joinTownhall, setJoinTownhall, Checkbox, Label and the outer
div when making these edits.
In `@apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx`:
- Around line 96-108: The dialog is category-aware via the isCategory flag but
the fallback error messages still say "channel"; update the fallback strings to
use the same isCategory conditional so failed edits report "category" when
isCategory is true (i.e., replace hardcoded "channel" fallbacks with isCategory
? "category" : "channel"); locate the error/fallback texts referenced near the
component's isCategory usage (e.g., in EditChannelDialog where isCategory is
defined and where error/fallback messages are produced) and apply the
conditional wording so titles, descriptions, and error fallbacks consistently
reflect category vs channel.
- Around line 96-97: The issue: category channels still include the hidden topic
field in the outgoing update and change-detection. Fix by excluding topic when
channel.type === "category": use the existing isCategory constant to filter out
topic from the update payload before sending the PATCH and remove topic from the
fields considered by hasChanges; specifically update the code that builds the
patch/update payload (the function that sends the PATCH request) and the
hasChanges comparison so that when isCategory is true topic is omitted. Apply
the same change to the second block referenced (around the other save/update
logic lines 130-142).
---
Duplicate comments:
In `@apps/web/src/components/sidebar/channel-panel/create-channel-dialog.tsx`:
- Line 52: The early return "if (forceType !== 'category' && !normalizedName)
return" silently aborts submission; update the submit/validation logic in
create-channel-dialog.tsx so that when normalizedName is empty (and forceType
!== "category") it calls setError(...) with a clear message instead of
returning; change the same pattern at the other occurrence (lines ~176-178) to
use the same setError flow and prevent silent no-ops, ensuring any submit path
(click or non-click) surfaces the validation error to the user.
🪄 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: 040398bc-6402-43d1-85c1-afb33e0ebbbd
📒 Files selected for processing (3)
apps/web/src/components/onboarding/onboarding-dialog.tsxapps/web/src/components/sidebar/channel-panel/create-channel-dialog.tsxapps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx
- Hide topic field and exclude from payload/hasChanges for category edits - Use category-aware error messages in EditChannelDialog - Make entire onboarding checkbox card clickable, not just label area
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-tag.yml:
- Around line 21-27: The step "Get version from tauri.conf.json" currently uses
VERSION=$(jq -r '.version' apps/desktop/src-tauri/tauri.conf.json) and
immediately emits outputs, which can produce "vnull"; update the step to
validate VERSION after the jq call (check for empty string or "null") and, if
invalid, print an error and exit non‑zero before writing to GITHUB_OUTPUT; only
echo "version=$VERSION" and "tag=v$VERSION" when the value passes this
validation.
- Around line 28-41: The current tag existence check uses local git rev-parse
(steps.check) which causes a TOCTOU race; replace that local check with a
remote-aware check using git ls-remote against origin for the tag name from
steps.version.outputs.tag (or steps.version.outputs.version if that's the tag)
and set exists based on that remote query, then only push if remote reports
missing; additionally make the push idempotent by handling a "remote already
exists" push failure as success (i.e., after a failed git push origin
"refs/tags/${{ steps.version.outputs.tag }}" re-run git ls-remote to confirm if
the tag now exists and treat as success if so).
- Line 17: Replace the mutable reference actions/checkout@v4 with the action
pinned to its full commit SHA to eliminate supply-chain risk; locate the usage
of actions/checkout@v4 in the workflow (the checkout step) and update it to the
specific commit SHA from the actions/checkout repository (replace the tag with
the full SHA string), ensuring the workflow still has the required permissions
and testing the workflow after the change.
In `@apps/web/src/components/onboarding/onboarding-dialog.tsx`:
- Around line 78-84: The acceptTownhallInvite fire-and-forget call currently
swallows errors; update its catch handler (inside the acceptTownhallInvite
callback) to log the error for observability instead of ignoring it.
Specifically, in acceptTownhallInvite where
apiClient.v1.invites[":code"].accept.$post(...).catch(() => {}), replace the
empty catch with a minimal log (e.g., console.error or the app logger) that
includes context (e.g., "acceptTownhallInvite failed" and the error) and still
does not alter UX or throw.
🪄 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: 27254ff0-bf8c-4f2c-b7b3-139b35be1a08
📒 Files selected for processing (3)
.github/workflows/auto-tag.ymlapps/web/src/components/onboarding/onboarding-dialog.tsxapps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx
| auto-tag: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify workflow actions not pinned to 40-char SHAs
rg -nP '^\s*uses:\s*[^@]+@(?![0-9a-fA-F]{40}\b).+$' .github/workflowsRepository: BuckyMcYolo/townhall
Length of output: 230
🏁 Script executed:
# Check if auto-tag.yml exists and list workflow files
ls -la .github/workflows/ | grep -E "(auto-tag|\.yml)"Repository: BuckyMcYolo/townhall
Length of output: 250
🏁 Script executed:
# Read the auto-tag.yml file to see the actual content around line 17
cat -n .github/workflows/auto-tag.yml | head -30Repository: BuckyMcYolo/townhall
Length of output: 907
🏁 Script executed:
# Check for write permissions in the workflow file
grep -n "permissions:" .github/workflows/auto-tag.yml -A 10Repository: BuckyMcYolo/townhall
Length of output: 258
Pin GitHub Action to a full commit SHA.
Line 17 uses a mutable ref (actions/checkout@v4) in a workflow with write permissions. Pin to a full SHA to reduce supply-chain risk.
🔒 Suggested change
- - uses: actions/checkout@v4
+ - uses: actions/checkout@<full-40-char-commit-sha>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/auto-tag.yml at line 17, Replace the mutable reference
actions/checkout@v4 with the action pinned to its full commit SHA to eliminate
supply-chain risk; locate the usage of actions/checkout@v4 in the workflow (the
checkout step) and update it to the specific commit SHA from the
actions/checkout repository (replace the tag with the full SHA string), ensuring
the workflow still has the required permissions and testing the workflow after
the change.
Comment on lines
+28
to
+41
| - name: Check if tag already exists | ||
| id: check | ||
| run: | | ||
| if git rev-parse "v${{ steps.version.outputs.version }}" >/dev/null 2>&1; then | ||
| echo "exists=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "exists=false" >> "$GITHUB_OUTPUT" | ||
| fi | ||
|
|
||
| - name: Create and push tag | ||
| if: steps.check.outputs.exists == 'false' | ||
| run: | | ||
| git tag "${{ steps.version.outputs.tag }}" | ||
| git push origin "${{ steps.version.outputs.tag }}" |
There was a problem hiding this comment.
Make tag check/push remote-aware and idempotent.
Lines 31–41 use a local git rev-parse check followed by push. That introduces a TOCTOU race and can fail when concurrent runs target the same version.
🛠️ Suggested reliability fix
- name: Check if tag already exists
id: check
run: |
- if git rev-parse "v${{ steps.version.outputs.version }}" >/dev/null 2>&1; then
+ TAG="${{ steps.version.outputs.tag }}"
+ if git ls-remote --exit-code --tags origin "refs/tags/${TAG}" >/dev/null 2>&1; then
echo "exists=true" >> "$GITHUB_OUTPUT"
else
echo "exists=false" >> "$GITHUB_OUTPUT"
fi
- name: Create and push tag
if: steps.check.outputs.exists == 'false'
run: |
- git tag "${{ steps.version.outputs.tag }}"
- git push origin "${{ steps.version.outputs.tag }}"
+ TAG="${{ steps.version.outputs.tag }}"
+ git tag "${TAG}"
+ if ! git push origin "refs/tags/${TAG}"; then
+ # If another run won the race, treat as success.
+ git ls-remote --exit-code --tags origin "refs/tags/${TAG}" >/dev/null 2>&1
+ fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/auto-tag.yml around lines 28 - 41, The current tag
existence check uses local git rev-parse (steps.check) which causes a TOCTOU
race; replace that local check with a remote-aware check using git ls-remote
against origin for the tag name from steps.version.outputs.tag (or
steps.version.outputs.version if that's the tag) and set exists based on that
remote query, then only push if remote reports missing; additionally make the
push idempotent by handling a "remote already exists" push failure as success
(i.e., after a failed git push origin "refs/tags/${{ steps.version.outputs.tag
}}" re-run git ls-remote to confirm if the tag now exists and treat as success
if so).
Add validation to ensure version exists in tauri.conf.json before using it. Also improve error handling in onboarding dialog by logging join failures instead of silently catching them.
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.
Summary
This PR adds Tauri desktop update support with native install/relaunch flow, introduces a self-hosted mode flag, implements category-based channel organization and related UI, adds a Checkbox UI component, and includes an auto-tagging workflow for desktop releases.
Key Changes
Desktop / Tauri
Update Checking & Installer Flow
Self-Hosted Mode
Onboarding
Channel Management / Categories
UI & Misc
Public API / Export Changes
Files of Note
Confidence Score: 3/5
3 - Mostly solid with some minor issues. A few things to address before merging (see recommended follow-ups).
Concerns / Recommended Follow-ups