feat: add role-based permissions system with channel edit/delete#18
Merged
BuckyMcYolo merged 6 commits intomainfrom Mar 12, 2026
Merged
feat: add role-based permissions system with channel edit/delete#18BuckyMcYolo merged 6 commits intomainfrom
BuckyMcYolo merged 6 commits intomainfrom
Conversation
- Define guild roles (owner, admin, warden, member) using Better Auth's createAccessControl with type-safe permission statements - Add channel update (PATCH) and delete (DELETE) API endpoints with permission checks on all mutating channel endpoints - Add edit/delete channel dialogs gated behind role permissions - Warden can create/edit channels but only owner/admin can delete - Gate drag-and-drop reordering and context menu behind canManage check - Fix auth route wildcard pattern (** → *) for Hono compatibility - Invalidate active guild member query on guild switch for correct roles - Display "Citizen" for member role and "Warden" for warden in UI
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds role-based guild permissions, channel update/delete endpoints and UI, member moderation APIs and dialogs, realtime per-guild message rate-limiting and communication blocks, DB schema for guild bans/timeouts, plus minor UI/docs tweaks. Changes
Sequence DiagramsequenceDiagram
participant User
participant Web as "Frontend (Dialog/UI)"
participant API as "API Handler"
participant Perm as "Server Permission Utils"
participant DB as "Database"
participant Cache as "Client Query Cache"
User->>Web: open edit/delete dialog & confirm
Web->>API: PATCH/DELETE /guilds/{slug}/channels/{id}
API->>Perm: assertGuildPermission / checkPermission
alt permission granted
Perm-->>API: ok
API->>DB: update/delete channel row
DB-->>API: result
API-->>Web: 200 OK / success
Web->>Cache: invalidate channel(s) queries
Web->>User: close dialog, show success
else permission denied
Perm-->>API: throw 403
API-->>Web: 403 Forbidden
Web->>User: show error
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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 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: 7
🤖 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/api/src/lib/permissions.ts`:
- Around line 15-24: The docstring example for checkPermission is misleading
because checkPermission either returns true or throws an HTTPException on
failure; it never returns false—update the example in the comment to reflect
that behavior by showing a direct call to checkPermission (or a try/catch
pattern) rather than checking a boolean, and mention that it throws
HTTPException(403) on insufficient permissions; reference the checkPermission
function name and the internal throw behavior so the example matches the actual
semantics.
In `@apps/api/src/routes/v1/channels/handlers.ts`:
- Line 61: The permission check currently calls
checkPermission(c.req.raw.headers, "channel", ["create"]) which checks the
session's active guild from headers; update the handlers in
apps/api/src/routes/v1/channels/handlers.ts to scope checks to the target guild
by passing the target guild ID (from the route params, e.g. c.req.params.guildId
or similar) into checkPermission or, better, use the pre-resolved memberRecord
that guild-auth middleware attaches to the context (e.g. memberRecord) to
perform permission checks for that guild; apply the same change to the other
checkPermission calls in this file (the other occurrences noted) so all
permission checks verify permissions against the target guild rather than
headers-only session guild.
In `@apps/api/src/routes/v1/channels/schema.ts`:
- Around line 53-55: The request schema updateChannelRequestSchema currently
aliases updateChannelSchema (which is all-optional via createUpdateSchema()) and
thus accepts an empty object; change updateChannelRequestSchema to be a refined
version of updateChannelSchema that rejects empty objects by adding a refinement
(e.g., via zod .refine or .superRefine) that checks the input has at least one
own property (Object.keys(obj).length > 0) and returns an appropriate validation
error if not; update code that references updateChannelRequestSchema accordingly
so the handler never receives {} and instead responds 400 on empty updates.
In `@apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx`:
- Around line 41-61: The mutation updateMutation lacks error handling and user
feedback; add an onError callback to the useMutation config (alongside the
existing mutationFn and onSuccess) that receives the error and either sets a
local error state (e.g., setError) used by the EditChannelDialog UI or calls the
app's notification/toast API to show the error message, and ensure mutationFn
throws a meaningful Error (include response text/json) so onError can surface
it; keep existing queryClient.invalidateQueries and onOpenChange behavior only
in onSuccess and do not close the dialog on error.
In `@apps/web/src/routes/_authenticated/`$guildSlug.tsx:
- Around line 35-43: The effect in useEffect is fire-and-forget:
authClient.organization.setActive({ organizationId: guild.id }) is not awaited,
so children (Outlet) and their queries (e.g., getActiveMember) can run against
stale guild context and caches; change the effect to await setActive() and only
invalidate queries after it resolves, and prevent Outlet/child rendering until
activeOrg?.id === guild.id (or track a local "isSwitching" state and render
children only when false) so the activeOrg, guild, guildSlug, setActive, and
queryClient.invalidateQueries flow is sequential and race-free.
In `@packages/auth/src/lib/auth-client.ts`:
- Line 12: Update the ESM import to use the .js extension so runtime resolution
matches the rest of the package: change the import that pulls in the permission
symbols (ac, admin, member, owner, warden) from "./permissions" to
"./permissions.js" so the module name used by auth-client.ts aligns with the
existing "./auth.js" style and Node.js ESM resolution.
In `@ROADMAP.md`:
- Around line 28-29: Update the ROADMAP.md checklist to mark the "Channel
edit/delete" item as completed by changing the checkbox from "[ ] Channel
edit/delete" to "[x] Channel edit/delete" so the roadmap accurately reflects
that channel edit and delete functionality has been implemented.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d5a45a03-9088-45de-b920-329113b0041b
📒 Files selected for processing (19)
CLAUDE.mdROADMAP.mdapps/api/src/app.tsapps/api/src/lib/permissions.tsapps/api/src/routes/v1/channels/handlers.tsapps/api/src/routes/v1/channels/index.tsapps/api/src/routes/v1/channels/routes.tsapps/api/src/routes/v1/channels/schema.tsapps/web/src/components/sidebar/channel-panel/channel-list.tsxapps/web/src/components/sidebar/channel-panel/delete-channel-dialog.tsxapps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsxapps/web/src/components/sidebar/right-panel/guild-members-panel.tsxapps/web/src/lib/permissions.tsapps/web/src/routes/_authenticated/$guildSlug.tsxpackages/auth/package.jsonpackages/auth/src/lib/auth-client.tspackages/auth/src/lib/auth.tspackages/auth/src/lib/permissions.tspackages/ui/src/components/alert-dialog.tsx
uses the already authenticated member form the middleware to check channel access
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/realtime/src/index.ts (1)
306-322: 🧹 Nitpick | 🔵 TrivialReuse the first channel access lookup for
createMessage().This handler resolves
accessibleChannelfor rate limiting, thencreateMessage()immediately callsassertUserCanAccessChannel()again. That adds a second DB read to every send and can make the rate-limit decision and the actual write operate on different membership/timeout snapshots. Pass the already-fetched channel intocreateMessage()or split creation from access resolution so the send path uses one consistent access check.🤖 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 306 - 322, The code redundantly calls assertUserCanAccessChannel twice; reuse the resolved accessibleChannel when creating messages so the access check and write use the same snapshot. Update createMessage (or add an overload) to accept the already-fetched accessibleChannel (or a membership/access DTO) instead of calling assertUserCanAccessChannel internally, and adjust the call site here to pass accessibleChannel along with userId and payload; keep the enforceGuildMessageRateLimit call as-is so rate-limit uses the same accessibleChannel data.ROADMAP.md (1)
33-36: 🧹 Nitpick | 🔵 TrivialConsider updating Phase 2 progress.
This PR implements significant portions of Phase 2 including member management UI (kick, ban, timeout, role assignment). Consider marking "Member management UI" as complete or in-progress.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ROADMAP.md` around lines 33 - 36, Update the roadmap to reflect implemented Phase 2 work: change the checklist item "Member management UI (kick, banish, silence, role assignment)" to either checked ([x]) or marked as in-progress (e.g., "[~]") and add a short note next to it referencing this PR for traceability; locate the string "Member management UI (kick, banish, silence, role assignment)" in ROADMAP.md and modify that line accordingly and optionally append a parenthetical "(implemented in PR #...)" or "(in-progress)".
♻️ Duplicate comments (1)
apps/web/src/routes/_authenticated/$guildSlug.tsx (1)
36-72:⚠️ Potential issue | 🟠 MajorOlder
setActive()calls can still win the race.
cancelledonly suppressessetIsSwitchingGuild(false). It does not stop an earlierauthClient.organization.setActive()from resolving after a later guild switch and leaving Better Auth's active org on the wrong guild while this route and its cache assume the latestguildSlug. Please serialize guild switches through a shared promise/ref, or re-apply the latest desired guild before invalidating and unblocking children.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authenticated/`$guildSlug.tsx around lines 36 - 72, The current effect can let an earlier authClient.organization.setActive() complete after a later guild switch, leaving Better Auth pointing at the wrong org; change the logic to serialize/revalidate the desired guild using a shared ref or promise and only apply the results for the latest requested guild: e.g., maintain a mutable ref like latestDesiredGuildRef (updated to guild.id at the top of the effect) or a shared pendingSwitch promise, call authClient.organization.setActive({ organizationId: guild.id }), then after that await completes check that latestDesiredGuildRef.current === guild.id (or that the pendingSwitch is still this call) before calling queryClient.invalidateQueries([...]) and before calling setIsSwitchingGuild(false); keep the cancelled flag for unmount safety but rely on the latestDesiredGuildRef/pendingSwitch check to prevent stale setActive resolutions from applying.
🤖 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/api/src/lib/permissions.ts`:
- Around line 2-9: The code is deriving StatementKey via "typeof statement"
while importing "statement" as a type-only import, which breaks compilation;
instead remove the typeof re-derivation and import the already-exported
StatementKey directly from "@repo/auth/permissions" (replace any uses of "typeof
statement" with the imported StatementKey) and update the related imports (e.g.,
where PermissionRequest, statement, StatementKey are referenced — also adjust
the similar occurrences around the
guildAuthorityHasPermissions/isGuildRole/canManageGuildAuthority usages at lines
16-19) so the file uses the concrete StatementKey symbol exported by the auth
package rather than deriving it from a type-only import.
In `@apps/api/src/routes/v1/channels/schema.ts`:
- Around line 53-58: The update path currently permits updating position and
parentId because updateChannelRequestSchema is built directly from
updateChannelSchema; remove those properties so reordering must go through the
dedicated logic. Replace the base used for updateChannelRequestSchema with an
omitted version of updateChannelSchema (e.g., updateChannelSchema.omit({
position: true, parentId: true })) and then apply the same refine check to that
omitted schema so at least one non-undefined field is required; this ensures
updateChannelSchema, updateChannelRequestSchema, and the update handler cannot
accept position or parentId and forces clients to use the reorderChannels
endpoint which contains the transactional/duplication/guild checks.
In `@apps/api/src/routes/v1/guilds/handlers.ts`:
- Around line 276-305: The upsert on schema.guildBan (inside the db.transaction
that builds insertedBan) currently updates bannedBy/reason/expiresAt and clears
revokedAt/revokeReason but leaves createdAt from the original row, causing
re-bans to appear to have the old createdAt; either change behavior to insert a
new row for each ban (remove the onConflictDoUpdate and insert a distinct
record) or, if you intend one-row-per-ban, update the onConflictDoUpdate to also
refresh createdAt (set createdAt = now) and any other audit fields so the
returned insertedBan reflects the new ban timestamp; look for the transaction
block, the tx.insert(schema.guildBan) call, the onConflictDoUpdate clause, and
the createdAt/revokedAt/revokeReason fields to implement the appropriate change.
In `@apps/api/src/routes/v1/guilds/schema.ts`:
- Around line 65-68: The expiresAt field in banGuildMemberRequestSchema must
reject past timestamps: update the schema so that expiresAt (when not
null/undefined) is validated to be a future datetime. Modify
banGuildMemberRequestSchema.expiresAt to include a refinement (or
preprocess+refine) that returns true for null/undefined but otherwise parses the
string and ensures new Date(parsed) is strictly greater than now, and provide a
clear validation message like "expiresAt must be in the future" so requests with
past dates are rejected before the handler.
In `@apps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsx`:
- Around line 42-49: The updateMutation currently assumes guildSlug is present
and uses a type assertion (guildSlug as string), which can result in the literal
"undefined" being sent; guard against undefined guildSlug before calling
apiClient.v1.guilds[":guildSlug"].channels[":channelId"].$patch by validating
guildSlug (from useParams) and aborting or returning an error if missing, e.g.,
throw or return early inside the mutationFn when guildSlug is undefined, and
only call the $patch with the validated guildSlug and channel.id when defined;
update references in this code path (updateMutation, mutationFn, guildSlug) to
use the validated value.
In `@apps/web/src/components/sidebar/right-panel/guild-members-panel.tsx`:
- Around line 310-317: The activeMember useQuery call currently ignores failures
which causes currentRole to become null and hides moderation controls silently;
update the useQuery for ["active-guild-member", view.guildSlug] (the queryFn
calling authClient.organization.getActiveMember) to handle errors via the
query's onError and/or return an error state so the UI can show a toast or an
error state instead of silently hiding controls—e.g., invoke the existing
toast/error handler or set a local error flag when onError fires and ensure
currentRole logic checks that error flag before hiding moderation controls.
- Around line 685-691: The AlertDialogAction's onClick must prevent the dialog's
default close behavior: update the onClick so it receives the click event and
calls event.preventDefault() before invoking the existing handler
(handleConfirmModeration) so the dialog doesn't close while
isModerationSubmitting/mutation runs; either wrap handleConfirmModeration in a
small inline function that calls event.preventDefault() or change
handleConfirmModeration signature to accept the event and call
event.preventDefault() at its start, keeping the existing
loading={isModerationSubmitting} and moderationActionLabel usage.
In `@packages/auth/src/lib/permissions.ts`:
- Around line 18-35: The helper roleHasPermissions currently only checks
guildPermissionGrants (channel/message/guildMember) and thus drops inherited
Better Auth permissions; update roleHasPermissions (and similar helpers around
lines referenced) to derive their grant set from the same role config used when
calling ac.newRole(...) or to merge the role's default/inherited statements into
guildPermissionGrants before permission checks; specifically, combine
PermissionRequest statements (including
defaultStatements/organization/member/invitation) with the guild-specific
entries so checks against PermissionRequest keys succeed, ensuring functions
like roleHasPermissions and any checks that reference guildPermissionGrants
consult the unified role policy rather than only the custom guild resources.
- Around line 100-101: The isGuildRole function currently uses the "in" operator
which can be true for prototype properties; change the check to use
Object.hasOwn(roles, value) (or Object.prototype.hasOwnProperty.call(roles,
value)) so only own keys of the roles object validate as GuildRole; update the
return in isGuildRole to that explicit own-property check to prevent prototype
key bypass.
---
Outside diff comments:
In `@apps/realtime/src/index.ts`:
- Around line 306-322: The code redundantly calls assertUserCanAccessChannel
twice; reuse the resolved accessibleChannel when creating messages so the access
check and write use the same snapshot. Update createMessage (or add an overload)
to accept the already-fetched accessibleChannel (or a membership/access DTO)
instead of calling assertUserCanAccessChannel internally, and adjust the call
site here to pass accessibleChannel along with userId and payload; keep the
enforceGuildMessageRateLimit call as-is so rate-limit uses the same
accessibleChannel data.
In `@ROADMAP.md`:
- Around line 33-36: Update the roadmap to reflect implemented Phase 2 work:
change the checklist item "Member management UI (kick, banish, silence, role
assignment)" to either checked ([x]) or marked as in-progress (e.g., "[~]") and
add a short note next to it referencing this PR for traceability; locate the
string "Member management UI (kick, banish, silence, role assignment)" in
ROADMAP.md and modify that line accordingly and optionally append a
parenthetical "(implemented in PR #...)" or "(in-progress)".
---
Duplicate comments:
In `@apps/web/src/routes/_authenticated/`$guildSlug.tsx:
- Around line 36-72: The current effect can let an earlier
authClient.organization.setActive() complete after a later guild switch, leaving
Better Auth pointing at the wrong org; change the logic to serialize/revalidate
the desired guild using a shared ref or promise and only apply the results for
the latest requested guild: e.g., maintain a mutable ref like
latestDesiredGuildRef (updated to guild.id at the top of the effect) or a shared
pendingSwitch promise, call authClient.organization.setActive({ organizationId:
guild.id }), then after that await completes check that
latestDesiredGuildRef.current === guild.id (or that the pendingSwitch is still
this call) before calling queryClient.invalidateQueries([...]) and before
calling setIsSwitchingGuild(false); keep the cancelled flag for unmount safety
but rely on the latestDesiredGuildRef/pendingSwitch check to prevent stale
setActive resolutions from applying.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8355e7fb-dce6-4e56-9652-89f0efcc5923
📒 Files selected for processing (24)
ROADMAP.mdapps/api/src/lib/permissions.tsapps/api/src/routes/v1/channels/handlers.tsapps/api/src/routes/v1/channels/schema.tsapps/api/src/routes/v1/guilds/handlers.tsapps/api/src/routes/v1/guilds/index.tsapps/api/src/routes/v1/guilds/routes.tsapps/api/src/routes/v1/guilds/schema.tsapps/api/src/routes/v1/uploads/handlers.tsapps/realtime/src/index.tsapps/realtime/src/services/channel-access.tsapps/realtime/src/services/messages.tsapps/realtime/src/services/rate-limit.tsapps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsxapps/web/src/components/sidebar/right-panel/guild-members-panel.tsxapps/web/src/components/sidebar/right-panel/right-sidebar-panel.tsxapps/web/src/lib/permissions.tsapps/web/src/routes/_authenticated/$guildSlug.tsxpackages/auth/src/lib/permissions.tspackages/db/src/schemas/guild-bans.tspackages/db/src/schemas/guild-members.tspackages/db/src/schemas/guilds.tspackages/db/src/schemas/index.tspackages/db/src/schemas/users.ts
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/realtime/src/services/messages.ts (1)
205-245:⚠️ Potential issue | 🟡 MinorAdd the timeout check to
deleteMessagefor consistency with other message operations.The
deleteMessagefunction does not callassertChannelCommunicationAllowed, whilecreateMessage,editMessage, andtoggleMessageReactionall do. This creates an inconsistency where timed-out users can delete messages but cannot create, edit, or react to them. Unless this distinction is intentional, add the check immediately after retrievingchannelRecord:const channelRecord = await assertUserCanAccessChannel( input.userId, input.payload.channelId ) assertChannelCommunicationAllowed(channelRecord)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/realtime/src/services/messages.ts` around lines 205 - 245, In deleteMessage, after retrieving channelRecord via assertUserCanAccessChannel, call assertChannelCommunicationAllowed(channelRecord) to enforce the same timeout/communication checks used by createMessage, editMessage, and toggleMessageReaction; this ensures timed-out users cannot delete messages—add the assertChannelCommunicationAllowed invocation immediately following the channelRecord assignment in the deleteMessage function.
🤖 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/api/src/routes/v1/channels/schema.ts`:
- Around line 53-56: The updateChannelRequestBaseSchema currently allows
updating the channel `type`; update the schema to omit `type` as well by adding
`type: true` to the call on updateChannelSchema.omit so that
updateChannelRequestBaseSchema = updateChannelSchema.omit({ position: true,
parentId: true, type: true }); locate the symbol updateChannelRequestBaseSchema
and the source updateChannelSchema to make this change and run related
schema/validation tests to confirm no breaking changes.
In `@apps/realtime/src/services/messages.ts`:
- Around line 15-21: The current error thrown in
assertChannelCommunicationAllowed reads "You are temporarily timed out and
cannot send messages", which is misleading for edits/reactions; update
assertChannelCommunicationAllowed to either accept an optional action parameter
(e.g., action: string) or simply change the thrown Error message to a generic
phrase like "You are temporarily timed out and cannot perform this action" so
the message fits edits, reactions, and sends; locate the throw in
assertChannelCommunicationAllowed and adjust the Error text (or add the optional
action param and use it in the message) accordingly.
In `@apps/web/src/components/sidebar/right-panel/guild-members-panel.tsx`:
- Around line 699-730: The dialog can be closed via Escape or outside click
while a moderation mutation is pending; update the AlertDialog's onOpenChange to
ignore close requests when isModerationSubmitting is true (e.g. if (!open &&
isModerationSubmitting) return) so setModerationDialog is not cleared
mid-mutation, and if your AlertDialog component supports props to disable
backdrop/escape closing (e.g. modal/closeOnEsc/disableOutsideClick), set those
when isModerationSubmitting is true; references: AlertDialog, onOpenChange,
isModerationDialogOpen, isModerationSubmitting, setModerationDialog,
AlertDialogAction, handleConfirmModeration.
In `@apps/web/src/routes/_authenticated/`$guildSlug.tsx:
- Around line 58-89: The async guild-switch immediately-invoked function
swallows errors from authClient.organization.setActive and
queryClient.invalidateQueries; wrap the awaited operations in a try/catch
(around the setActive and Promise.all calls) so failures are caught, call
setIsSwitchingGuild(false) in finally remains, and in the catch log the error
and surface user feedback (e.g., via existing toast/notification helper or a
setError state) referencing authClient.organization.setActive,
queryClient.invalidateQueries, latestDesiredGuildRef, switchRequestRef, and
setIsSwitchingGuild so the UI resets and the error is visible for debugging.
In `@packages/auth/src/lib/permissions.ts`:
- Around line 68-71: The runtime tuple assignableGuildRoles should be
constrained to be a subset of the keys of roles so it can't drift away from
actual guild roles or accidentally include "owner"; change its declaration to
use TypeScript's satisfies with the keys of roles (e.g. replace the current line
with something like: const assignableGuildRoles = ["admin", "warden", "member"]
as const satisfies ReadonlyArray<keyof typeof roles>) and keep
AssignableGuildRole = (typeof assignableGuildRoles)[number] so the exported type
stays in sync with the validated tuple; refer to assignableGuildRoles,
AssignableGuildRole, and roles when making the change.
---
Outside diff comments:
In `@apps/realtime/src/services/messages.ts`:
- Around line 205-245: In deleteMessage, after retrieving channelRecord via
assertUserCanAccessChannel, call
assertChannelCommunicationAllowed(channelRecord) to enforce the same
timeout/communication checks used by createMessage, editMessage, and
toggleMessageReaction; this ensures timed-out users cannot delete messages—add
the assertChannelCommunicationAllowed invocation immediately following the
channelRecord assignment in the deleteMessage function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f85bf3db-c328-452e-ae10-c15e6fb33ac2
📒 Files selected for processing (11)
ROADMAP.mdapps/api/src/lib/permissions.tsapps/api/src/routes/v1/channels/schema.tsapps/api/src/routes/v1/guilds/handlers.tsapps/api/src/routes/v1/guilds/schema.tsapps/realtime/src/index.tsapps/realtime/src/services/messages.tsapps/web/src/components/sidebar/channel-panel/edit-channel-dialog.tsxapps/web/src/components/sidebar/right-panel/guild-members-panel.tsxapps/web/src/routes/_authenticated/$guildSlug.tsxpackages/auth/src/lib/permissions.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/routes/_authenticated/$guildSlug.tsx (1)
25-31:⚠️ Potential issue | 🟠 MajorUse a global
["active-guild"]query key instead of scoping it toguildSlug.The query calls
authClient.organization.getFullOrganization(), which always returns the current active org from auth state and does not depend onguildSlug. By includingguildSlugin the cache key, previously visited guild slugs will reuse their stale cached org snapshots. When navigating back to a guild you've visited before, the short-circuit check at line 53 (activeOrg?.id === desiredGuildId) will incorrectly pass with stale data, causingsetActive()to be skipped. This allows the route to render children against the wrong guild context until React Query's background refetch catches up. Use a single["active-guild"]key and invalidate that global key aftersetActive()succeeds.🛠️ Suggested fix
- const { data: activeOrg } = useQuery({ - queryKey: ["active-guild", guildSlug], + const { data: activeOrg } = useQuery({ + queryKey: ["active-guild"], queryFn: async () => { const res = await authClient.organization.getFullOrganization() return res.data }, }) ... - queryClient.invalidateQueries({ - queryKey: ["active-guild", guildSlug], + queryClient.invalidateQueries({ + queryKey: ["active-guild"], }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/routes/_authenticated/`$guildSlug.tsx around lines 25 - 31, The query for active org uses useQuery with queryFn authClient.organization.getFullOrganization() but incorrectly scopes the cache to guildSlug; change the queryKey in that useQuery call from ["active-guild", guildSlug] to a single global ["active-guild"] key so cached org is shared, and after you call setActive(desiredGuildId) add an invalidate/refresh for the ["active-guild"] query (via your React Query queryClient.invalidateQueries or equivalent) so the global cache is updated; keep the existing activeOrg?.id === desiredGuildId short-circuit but rely on the global key to avoid stale org snapshots.
🤖 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/api/src/routes/v1/channels/schema.ts`:
- Around line 53-64: Replace the current use of omit() which leaves the schema
in strip mode by creating an explicit allowlist: derive
updateChannelRequestBaseSchema from updateChannelSchema using pick(...) to
enumerate allowed updatable keys, then call .strict() on that picked schema so
unexpected keys (e.g., position, parentId, type) are rejected; keep the existing
refine that ensures at least one field is present and assign the result to
updateChannelRequestSchema so the handler no longer silently strips forbidden
PATCH fields.
In `@packages/auth/src/lib/permissions.ts`:
- Around line 80-83: GuildAuthority currently allows conflicting ownership state
(role vs isOwner) which risks inconsistent auth decisions; change to a single
source of truth by normalizing the object before any authorization: create a
normalizeGuildAuthority(g: GuildAuthority) helper (used by existing helpers that
inspect GuildAuthority) that derives and enforces isOwner from role (e.g., set
isOwner = true iff role === "owner") and reject or overwrite any contradictory
isOwner input, or alternatively drop the isOwner field and compute ownership
from role at runtime; update all functions that accept GuildAuthority (including
helpers referenced around the GuildAuthority declaration) to call this
normalizer so permission checks always use the normalized, authoritative
ownership value.
---
Outside diff comments:
In `@apps/web/src/routes/_authenticated/`$guildSlug.tsx:
- Around line 25-31: The query for active org uses useQuery with queryFn
authClient.organization.getFullOrganization() but incorrectly scopes the cache
to guildSlug; change the queryKey in that useQuery call from ["active-guild",
guildSlug] to a single global ["active-guild"] key so cached org is shared, and
after you call setActive(desiredGuildId) add an invalidate/refresh for the
["active-guild"] query (via your React Query queryClient.invalidateQueries or
equivalent) so the global cache is updated; keep the existing activeOrg?.id ===
desiredGuildId short-circuit but rely on the global key to avoid stale org
snapshots.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4426e609-a798-4cc3-bc69-9913ab117e7e
📒 Files selected for processing (5)
apps/api/src/routes/v1/channels/schema.tsapps/realtime/src/services/messages.tsapps/web/src/components/sidebar/right-panel/guild-members-panel.tsxapps/web/src/routes/_authenticated/$guildSlug.tsxpackages/auth/src/lib/permissions.ts
This was referenced Mar 17, 2026
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.
Role-Based Permissions System with Channel Management
Overview
This PR implements a role-based access control system (owner, admin, warden, member) using Better Auth’s access-control primitives and adds server- and client-side channel management (edit/delete), member moderation (role updates, kick, ban, timeout), per-role message rate limits, and related UI and schema changes. It also gates drag-and-drop reordering and context actions behind management permissions and fixes an auth route wildcard for Hono compatibility.
Backend Highlights
Frontend Highlights
Small but important fixes
Public API / Exports
Risks & Review Notes
Code Quality
Confidence Score: 4/5
Good PR: well-structured, follows existing patterns, and types are sound. Main concerns are breadth (DB migration + realtime rate limiting + many endpoints/UI changes) which increases review/QA surface; ensure migration scripts and deployment ordering are validated and integration tests for moderation, rate limiting, and channel deletion flows are run.