Conversation
- New API route: GET /api/organizations/memberships calls app.certified.groups.membership.list - Add RemoteMembership type and accepted field to Organization - Rewrite resolveOrganizations to merge remote (source of truth) + local (accepted state)
- Remove AddOrgModal and MembershipSyncModal (orgs appear automatically) - Remove 'Add existing organization' button and all sync logic - Add 'Accept membership publicly' button (UserCheck icon) for unaccepted orgs - Add 'Leave organization' button (LogOut icon) for accepted orgs - Add .org-list__accept-btn CSS with green border and hover fill
…groups - Sort: accepted first, then owner > admin > member, then alphabetical - Divider between accepted and pending groups with explanatory note - Rename all user-facing 'organization(s)' to 'group(s)' in page, sidebar, navbar
- 'Remove public membership' (UserX): deletes local PDS record only, keeps group access - 'Leave group' (LogOut): removes from group service + cleans up PDS record, warns about losing access - Leave button disabled for sole owners (fetches member list to check) - Accept button unchanged (UserCheck for unaccepted orgs) - CSS for new button styles (amber hover for remove-public, red hover for leave)
- Add avatarUrl to Organization type - Resolve PDS URL + compute avatar URL in resolveOrganizations - Show Avatar component with Building2 fallback when no avatar
- Remove Switch button (use profile switcher instead) - Update description to point users to profile switcher - Clean up unused imports (useRouter, switchOrg)
- Reuse account-switcher items (user + groups) in the mobile dropdown - Close dropdown on switch instead of just the desktop popover - Sign out button moved below the switcher
- Mobile: avatar button opens profile switcher dropdown (same as desktop) - Mobile: hamburger opens nav links only - Opening one closes the other - Outside click works for both desktop and mobile switcher refs - Sign out moved into the mobile switcher dropdown
- Mobile: tapping avatar opens a bottom sheet sliding up from screen edge - Dimmed backdrop dismisses on tap, body scroll locked while open - Scrollable content area with max 70vh, safe-area bottom padding - Pill-shaped drag handle at top, smooth slide-up + fade-in animations - Desktop dropdown completely unchanged
…ontext The navbar's backdrop-filter creates a new containing block, so position:fixed children are positioned relative to it instead of the viewport. Using createPortal renders the bottom sheet directly into document.body.
…sheet - Sign out is now a LogOut icon button inline with the user row - Removed standalone sign out button at the bottom of the sheet - Gray icon turns red on hover
- Touch drag on the sheet drags it down in real-time (no transition during drag) - Swipe down >80px dismisses with slide-out animation - Swipe less than 80px snaps back to position - Drag handle now actually feels interactive
- Touch handlers moved from entire sheet to handle only (no scroll conflict) - Swipe up on handle expands sheet to 92vh - Small swipe down while expanded collapses back to 70vh - Swipe down >80px dismisses the sheet - Handle has touch-action:none and cursor:grab for clarity - Content area scrolls independently without triggering drag
The bottom sheet is rendered via createPortal to document.body, so it's outside mobileSwitcherRef. The mousedown handler was closing the switcher before button onClick could fire. Now also checks sheetRef and backdrop class.
…button - Desktop user row now has LogOut icon (same as mobile bottom sheet) - Removed standalone 'Sign out' button from navbar - Removed dead .navbar__signout CSS
- API routes: group CRUD, members, audit, profile, metadata, handle, blob upload - Organization context, constants, proxy-agent, index - Organization UI components: settings, members, handle search, modals - Organization pages: create, group detail, edit profile, apps, settings - Hooks: use-org-profile - Updated layout, app-shell, settings, home-client for org support - resolveHandle export in did.ts - New assets: wordmarks, guilloches, sign-in buttons - Tests directory
- API route now passes limit and cursor query params through - Client function paginates (100 per page) until no cursor remains - Previously only fetched the first page (default server limit)
The audit.query endpoint only supports actorDid, action, collection filters — not limit/cursor. Sending limit caused a 400 error which was swallowed by the catch block, resulting in empty audit log. Still supports cursor pagination if the endpoint returns one.
- Show full actorDid instead of truncated (with word-break for overflow) - Show collection, rkey, and all detail fields as tags below each entry - Detail values rendered as key: value pills - Moved result badge next to action on the main row for cleaner layout
The cleanup effect was clearing activeOrg immediately on mount because isAuthenticated starts as false while auth loads. Now waits for auth to finish loading before deciding to clear — the persisted org from localStorage survives until auth resolves.
fetchOrgs also cleared activeOrg on its early return when !isAuthenticated, which fires on mount before auth resolves. Now returns early without clearing when authLoading is true. Added authLoading to dependency array.
- Split handle into prefix (editable) and suffix (.certified.one) - Suffix shown as fixed monospace text next to the input - Save recombines prefix + suffix into full handle for the API - Removed unused dynamic import
Target the actual Input wrapper div (not .input-wrapper) and align the suffix vertically with the input by accounting for the label height above it.
- Add groupDid prop to UsernameCard — routes handle changes through /api/organizations/[groupDid]/handle when set - Org settings now uses the same UsernameCard as individual users: edit certified handle, choose certified username, custom domain - Removed custom handle editing code from OrgSettings - Title shows 'Handle' for orgs, 'Username' for users - Removed unused handle-edit CSS (now uses UsernameCard's styles)
Both users and groups now edit only the prefix part of the handle. The PDS hostname suffix (.certified.one, etc.) is shown as fixed text next to the input. Same validation as the subdomain picker: 3-18 chars, lowercase alphanumeric + hyphens.
For groups, pdsUrl may differ from the user's PDS. Now derives the suffix from the handle itself (everything after the first dot), e.g. woop.pds1.test.certified.app → suffix is pds1.test.certified.app. Falls back to pdsUrl, then env var.
- Group handle is read-only (group service doesn't support identity.updateHandle — handle is set during registration only) - Restore limit param for audit log (API reference confirms support) - Paginate audit with limit=100 per page - Remove unused UsernameCard import from org-settings
The AtpAgent validates query params against the lexicon and strips undeclared params. limit and cursor were missing from the lexicon definition, so the proxy agent was dropping them — resulting in the group service returning a default page that may be empty or cause errors.
The detail object from the group service contains collection and rkey, which are already shown from the top-level entry fields. Filter them out to avoid showing the same info twice.
User profile: - Display name: maxLength 640 → 64 (matches validation) - Description: maxLength 2560 → 256 (matches validation + counter) - Website: maxLength 2560 → 256 Group profile: - Display name: maxLength 640 → 64 - Description: maxLength 2560 → 256 (matches counter) - Website: maxLength 2560 → 256 - Placeholders updated to say 'group' instead of 'organization'
The group service's member.add endpoint only accepts 'member' or 'admin' roles. To make someone an owner, add them first, then promote via role.set (the role dropdown on existing members).
Users who have created 5 organizations they are currently part of will see a limit-reached message instead of the create form, with a contact email for exceptions. Server-side check in the register route prevents bypass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
feat: limit users to max 5 self-created organizations
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR introduces comprehensive organization/group management functionality across API routes, client-side state management, UI pages, and utility libraries. It adds endpoints for organization CRUD, membership management, and audit logging; implements React context for org switching; creates UI pages for creating, viewing, and configuring organizations; and establishes shared types and helper functions for org-related operations. Changes
Sequence DiagramsequenceDiagram
participant User
participant BrowserClient
participant BFF as BFF API Routes
participant GroupService
participant PDS
rect rgb(200, 150, 255, 0.5)
Note over User,PDS: Organization Creation Flow
User->>BrowserClient: Submit create org form
BrowserClient->>BFF: POST /api/organizations/register
BFF->>GroupService: registerOrganization(handle, ownerDid)
GroupService-->>BFF: groupDid
BFF->>BFF: Attempt: createBskyProfile(groupDid)
BFF->>BFF: Attempt: putOrgProfile(groupDid, profile)
BFF->>BFF: Attempt: putOrgMetadata(groupDid, metadata)
BFF->>PDS: putMembership(ownerDid, groupDid, "owner")
BrowserClient->>BrowserClient: refetchOrgs() → navigate to /organizations
end
rect rgb(200, 200, 255, 0.5)
Note over User,PDS: Organization Switching & Viewing
User->>BrowserClient: Click org in switcher
BrowserClient->>BrowserClient: switchOrg(org) → persist to localStorage
BrowserClient->>BrowserClient: useOrg() context updates
BrowserClient->>BFF: GET /api/organizations/[groupDid]/profile
BFF->>PDS: Fetch from group's PDS
PDS-->>BFF: profile data
BFF-->>BrowserClient: profile JSON
BrowserClient->>BrowserClient: Render org profile page with nav
end
rect rgb(200, 255, 200, 0.5)
Note over User,PDS: Member Management
User->>BrowserClient: Add member to org
BrowserClient->>BFF: POST /api/organizations/[groupDid]/members
BFF->>GroupService: addOrgMember(groupDid, memberDid, role)
GroupService-->>BFF: OrgMember
BFF-->>BrowserClient: member JSON
BrowserClient->>BrowserClient: refetch members list
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~70 minutes 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 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 |
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.beads/.beads-credential-key:
- Line 1: Remove the committed secret file .beads/.beads-credential-key from the
PR and repository history: delete the file from the working tree and create a
follow-up commit that removes it, then rotate/reissue the exposed credential
immediately (revoke the old key and provision a new one). Add an ignore rule to
prevent future commits (e.g., add ".beads/.beads-credential-key" to .gitignore)
and, if the file was already pushed, purge it from history using git-rewrite
tools (git rm --cached + git filter-branch or BFG) and force-push the cleaned
branch; update any configuration or code that referenced the key to use a secure
secret manager or environment variable instead.
In `@src/app/api/organizations/memberships/route.ts`:
- Around line 22-25: The limit calculation using parseInt can yield NaN for
non-numeric limitParam, causing Math.min(NaN, 100) to become NaN; update the
logic in the route where limitParam is parsed (the code referencing limitParam,
parseInt, and Math.min) to parse into a temporary number, validate it (e.g.,
ensure it's a finite integer using Number.isFinite or isNaN check), and fall
back to the default (100) when invalid before applying Math.min so limit is
always a valid number <= 100.
In `@src/app/api/organizations/register/route.ts`:
- Around line 69-72: The member listing call to
groupAgent.call("app.certified.group.member.list", { limit: 100 }) paginates but
currently only fetches the first page, so large groups miss members; update the
code in route.ts where groupAgent.call is invoked to iterate through pages
(using the API's pagination tokens/offsets or nextCursor returned in each
response) and accumulate/scan all members for the addedBy check (or maintain a
running count/flag) until no more pages remain, then use that aggregated result
in the self-created count logic; ensure you handle empty/undefined cursors and
stop on completion or error.
In `@src/app/organizations/`[groupDid]/edit-profile/page.tsx:
- Around line 85-103: handleAvatarUpload and handleBannerUpload currently
swallow exceptions from uploadOrgBlob so users never see upload failures; add an
uploadError state (e.g., uploadError) and in both handlers catch the error, set
uploadError to a user-friendly message (or include error.message) before
rethrowing or returning, ensure setIsUploadingAvatar/setIsUploadingBanner still
run in finally, and only setAvatarBlob/setBannerBlob on success; update the UI
to display uploadError alongside saveError so upload failures are visible to
users.
In `@src/app/organizations/`[groupDid]/settings/page.tsx:
- Around line 200-208: The remove button with class org-members__remove-btn
(used in the JSX that renders the Trash2 icon and calls
handleRemoveMember(member.did)) does not meet the 44px minimum touch target;
update the CSS for .org-members__remove-btn to enforce min-width:44px and
min-height:44px and make it a centered flex container (display:flex;
align-items:center; justify-content:center) so the 14px icon remains visually
centered while meeting mobile accessibility touch-size requirements.
In `@src/app/organizations/create/page.tsx`:
- Line 207: The input's maxLength prop (maxLength={640}) conflicts with the name
validation that rejects names >64 chars; update the input to maxLength={64} so
the rendered limit matches the validation (or alternatively raise the validation
limit to 640 if longer names are intended). Locate the name input with the
maxLength prop and the validation block that checks length > 64 and make them
consistent (prefer setting maxLength to 64 on the input to match the existing
validation).
In `@src/app/organizations/page.tsx`:
- Around line 92-117: The three action buttons (classes
org-list__remove-public-btn, org-list__accept-btn, org-list__leave-btn used with
handlers handleRemovePublicMembership, handleAcceptMembership, setLeaveOrg and
state canLeaveMap) have icons too small for mobile touch targets; ensure each
button has a minimum 44x44px interactive area by adding CSS rules (e.g.,
min-width/min-height:44px; padding:...; display:inline-flex; align-items:center;
justify-content:center) for those classes or by applying equivalent inline
styles, while keeping the visual icon small (leave the icon size prop as-is or
adjust to a slightly larger value if needed); confirm focus/hover styles and
that disabled states still meet the size requirement on mobile.
In `@src/components/dashboard/username-card.tsx`:
- Around line 46-50: The current heuristic in isOurHandle (username-card.tsx)
that treats any handle with 2+ dots as "ours" is brittle; update isOurHandle to
instead verify the handle's hostname against known PDS hostnames or the resolved
pdsHostname (and still allow the .certified.app suffix). Concretely: parse the
hostname from handle, resolve or compare it to the app's pdsHostname (or a
maintained list of allowed PDS hostnames) and only return true when the hostname
equals/endsWith a trusted PDS host (or matches .certified.app), rather than just
counting dots; use the existing handle variable and the isOurHandle function to
implement this check.
In `@src/components/landing/home-client.tsx`:
- Around line 163-177: The rendered external links from orgMetadata.urls need
URL validation/sanitization before being used as hrefs to prevent
open-redirect/XSS; modify the rendering in the home-client component where
orgMetadata.urls.map(...) is used to first parse each u.url (e.g., with the URL
constructor or a URL parsing util) and only render the <a> when the protocol is
http: or https: and the hostname is acceptable (or matches a whitelist/regex);
for unsafe URLs, either omit the link or render the label as plain text, and
ensure any displayed URL is properly encoded (e.g., via encodeURI/escape) so you
never inject raw, unvalidated u.url into the href attribute.
In `@src/components/organizations/add-org-modal.tsx`:
- Around line 125-127: The close button element using className
"signin-modal__close" in add-org-modal.tsx currently renders at 32x32px which is
below the 44px mobile touch target; update the styling for the
"signin-modal__close" selector (or add an inline style on the button) to ensure
min-width and min-height are at least 44px (or set width/height to 44px and
adjust padding/margin so the X icon still centers), and verify the onClick
handler onClose and the <X> icon remain visually centered and accessible (add or
keep an appropriate aria-label if not present).
In `@src/components/organizations/handle-search.tsx`:
- Line 44: The ref debounceRef is typed as useRef<ReturnType<typeof setTimeout>>
but initialized with null which conflicts in strict TS; update the type to allow
null (e.g., useRef<ReturnType<typeof setTimeout> | null>) so debounceRef can be
initialized to null and later hold the timer handle; locate the debounceRef
declaration to change its generic type accordingly.
In `@src/components/organizations/membership-sync-modal.tsx`:
- Around line 37-39: The close button in membership-sync-modal.tsx uses the
signin-modal__close class which provides a 32x32 touch target; update the shared
CSS rule for .signin-modal__close (used by membership-sync-modal.tsx and
add-org-modal.tsx) to make the clickable area at least 44x44px (adjust
height/width/padding or min-width/min-height) and ensure visual centering of the
X icon (component X) inside that box; if the class is used elsewhere, verify
there are no layout regressions or create a new modal-close class and replace
signin-modal__close on modal components that need the larger touch target.
In `@src/components/organizations/org-settings.tsx`:
- Around line 51-53: The auditLoading state is initialized to false which can
cause a brief flash of "No activity recorded yet." before fetchAudit runs;
update the useState initialization for auditLoading in the org-settings
component to true (change the useState(false) call for auditLoading to
useState(true)) and ensure any places that set loading (setAuditLoading) still
toggle appropriately around fetchAudit (e.g., setAuditLoading(true) before fetch
and setAuditLoading(false) after) so the spinner displays on mount instead of
the empty message.
In `@src/components/profile/profile-edit-form.tsx`:
- Line 253: The displayName and description inputs in profile-edit-form.tsx
currently only use HTML maxLength (e.g., maxLength={64} and maxLength={256})
which limits characters not bytes and can allow multi-byte Unicode to exceed
backend byte limits from src/lib/atproto/types.ts (640 and 2560 bytes). Add a
UTF-8 byte-length check: implement a small helper (e.g., utf8ByteLength(str))
and enforce it in the displayName and description onChange handlers (or
validation routine) to trim or reject input that would exceed the byte limits;
also run the same byte-length validation on form submit to prevent backend
rejections. Ensure the UI still enforces grapheme limits (keep maxLength for
characters) but add byte-limit validation using the helper and show a clear
error message when the byte limit is reached.
In `@src/lib/organizations/org-context.tsx`:
- Line 63: Change the isLoading useState to initialize true (const [isLoading,
setIsLoading] = useState(true)) so the UI doesn't flash empty content, and
ensure the auth-check path calls setIsLoading(false) when the user is not
authenticated (where the auth logic currently branches) to avoid an infinite
loading state; update the relevant effect or handler that references
isLoading/setIsLoading in this module (org-context.tsx) so both the
authenticated and unauthenticated code paths always setIsLoading(false) after
resolution.
---
Nitpick comments:
In `@src/app/api/organizations/`[groupDid]/audit/route.ts:
- Around line 27-33: The code currently assigns the raw string limit from
request.nextUrl.searchParams into queryParams.limit; change this to parse and
validate the value: call parseInt on the retrieved limit string, check for NaN
and enforce a positive integer, clamp it to a configured max (e.g.,
DEFAULT_LIMIT and MAX_LIMIT constants) and fall back to a safe default if
invalid, then assign the numeric value to queryParams.limit (use the variables
limit, queryParams, and the route handler in
src/app/api/organizations/[groupDid]/audit/route.ts to locate the change).
In `@src/app/api/organizations/`[groupDid]/handle/route.ts:
- Around line 25-30: The handler currently only checks that the extracted handle
(const { handle } = body) is non-empty; add a format validation right after that
to reject handles with spaces or disallowed characters before calling the group
service. Implement a regex check (e.g., allow only letters, numbers,
dashes/underscores as your policy requires and disallow whitespace) against the
handle variable and return NextResponse.json({ error: "Invalid handle format" },
{ status: 400 }) when it fails; place this validation in the same route handler
immediately after the existing trim check so invalid formats are rejected early.
In `@src/app/api/organizations/`[groupDid]/members/route.ts:
- Around line 23-27: The request reads limit from query string into const limit
and passes Number(limit) to groupAgent.call("app.certified.group.member.list")
without validation; add validation to parse the incoming limit, enforce a sane
upper bound (e.g. MAX_LIMIT = 100 or 1000), and clamp values below 1 to the
default and values above MAX_LIMIT down to MAX_LIMIT before calling
groupAgent.call; update the variable used in the call (the local limit or a new
parsedLimit) so the agent never receives an unbounded numeric limit.
In `@src/app/api/organizations/`[groupDid]/metadata/route.ts:
- Around line 26-35: The fetch to the PDS in route.ts (the request that builds
the URL using pdsUrl and groupDid and checks res.ok) lacks a timeout; wrap that
fetch in an AbortController with a short configurable timeout (e.g., via a
setTimeout) and pass controller.signal to fetch, clear the timeout after
completion, and handle AbortError by returning an appropriate NextResponse.json
error (or throwing) instead of letting the request hang; update the fetch block
that constructs `${pdsUrl}/xrpc/com.atproto.repo.getRecord?...` to use this
AbortController pattern and ensure the timer is cleared on both success and
error.
- Around line 66-82: The handler currently reads unvalidated JSON via
request.json() and writes it directly with
createGroupAgent(...).call("app.certified.group.repo.putRecord") into collection
"app.certified.actor.organization" with rkey "self"; add explicit
validation/sanitization of the parsed body before calling groupAgent.call:
define an expected schema (required fields and types) for the organization
record (or use a validator like zod/ajv), validate the body returned from
request.json(), reject with a 400 on validation errors, and construct the record
object by picking only allowed fields plus $type before passing to
groupAgent.call to prevent extra/malicious properties from being written.
In `@src/app/api/organizations/`[groupDid]/profile/route.ts:
- Line 72: The console.log in the PUT handler that prints the request body (the
line with "PUT org profile: writing to" and JSON.stringify(body)) is too verbose
and may leak PII; replace it with a minimal log that records only non-sensitive
identifiers and metadata (e.g., groupDid, request id, body length or a boolean
indicating presence) and remove any profile content output; update the same
pattern at the other occurrence noted (around the code at lines ~90) and use the
existing application logger (processLogger or similar) rather than console.log
for consistent log levels.
In `@src/app/api/organizations/memberships/route.ts`:
- Around line 34-47: The fetch to the group service (the fetch call using
url.toString() with Authorization Bearer `token`) lacks a timeout; wrap the
request with an AbortController: create an AbortController, pass
controller.signal into fetch, start a setTimeout that calls controller.abort()
after a configured timeout (e.g., 5s), and clear the timeout on success; update
the surrounding code (the try/catch around the fetch/res handling) to catch an
abort (e.g., identify the AbortError) and return an appropriate
NextResponse.json error (using the same shape as the existing error branch that
uses `res.status`), ensuring no leaked timers and that `res` handling remains
unchanged for non-timeout failures.
In `@src/app/api/organizations/register/route.ts`:
- Around line 90-92: The current catch block in the register route
(src/app/api/organizations/register/route.ts) swallows errors from the limit
check and "fails open", which may let org creation bypass limits; change the
catch to fail-closed by logging the caught error (use the existing
logger/processLogger) and returning an appropriate error response (e.g., 429 Too
Many Requests or 503 Service Unavailable with a clear message) instead of
proceeding with creation; alternatively, implement a configurable policy (e.g.,
ORG_CREATION_FAIL_OPEN env flag) in the register route handler so behavior can
be toggled, but default to failing closed for compliance-sensitive deployments.
In `@src/app/globals.css`:
- Around line 4382-4398: Rename the keyframe identifiers bottomSheetSlideUp and
bottomSheetFadeIn to kebab-case (e.g. bottom-sheet-slide-up and
bottom-sheet-fade-in) in the CSS, and update all animation references that use
those names (e.g., animation, animation-name, or shorthand usages) so they match
the new kebab-case identifiers; ensure you change both the `@keyframes`
declarations and every place in the codebase that refers to bottomSheetSlideUp
or bottomSheetFadeIn.
In `@src/app/organizations/`[groupDid]/apps/page.tsx:
- Around line 19-25: The Back button with class dashboard__back-btn may be
smaller than the 44px mobile touch target; update the stylesheet for
dashboard__back-btn (used on the button in
src/app/organizations/[groupDid]/apps/page.tsx) to guarantee a minimum 44px
touch area by adding min-height:44px and min-width:44px and/or increasing
padding and using display:inline-flex; align-items:center;
justify-content:center to keep ArrowLeft and label centered; ensure the rule
applies to the actual button element (not a container) so router.push/back
button retains accessible touch sizing.
In `@src/app/organizations/`[groupDid]/edit-profile/page.tsx:
- Line 155: The redirect after saving currently calls router.push("/") — change
it to navigate to the organization's profile page so users see their updates
immediately; replace the call with router.push(`/organizations/${groupDid}`) (or
router.replace if you want to avoid back navigation), ensuring the groupDid
variable from the surrounding scope (used in this file) is available when the
save handler runs and that the save handler (e.g., the function invoking
router.push) uses the Next router instance (router) from
useRouter()/next/navigation.
In `@src/app/organizations/`[groupDid]/page.tsx:
- Around line 121-139: Replace the non-semantic h3 elements used for card titles
with h2 to follow the project's semantic HTML guideline: change the elements
that render the "Identity" and "Manage" titles (they use className
"dash-card__title") from h3 to h2 so the card headings are proper h2 elements
while preserving the existing classes and layout.
- Line 18: The destructured variable "did" from useAuth() in the component is
unused; remove the unused binding to eliminate the linter warning by deleting
"const { did } = useAuth()" (or if you intended to use it, reference the "did"
value where needed in this component); locate the call to useAuth() in the page
component (the line with const { did } = useAuth()) and remove the "did"
destructuring or the entire statement if auth is provided by a parent.
In `@src/app/organizations/`[groupDid]/settings/page.tsx:
- Around line 179-193: The role dropdown currently allows changing a member to
"owner" without confirmation; update the UI flow in the component rendering the
select (where isOwner is checked and onChange calls handleRoleChange(member.did,
e.target.value as OrgRole)) to intercept attempts to set role === 'owner' and
show a confirmation dialog before calling handleRoleChange; implement the dialog
invocation (modal/confirm) with clear text about transferring ownership and only
call handleRoleChange(member.did, 'owner') if the user confirms, otherwise
revert the select to the previous role value to avoid accidental escalation.
- Around line 261-265: The loading container for audit entries lacks an
accessibility role; update the JSX branch that renders the loading state (where
auditLoading is checked and the div uses className "org-audit__loading" and
renders <LoadingSpinner />) to add role="status" on the container so screen
readers announce the loading indicator; ensure the change targets the
conditional rendering that uses auditLoading and auditEntries and does not alter
the LoadingSpinner component itself.
- Around line 105-115: The use of native confirm() in handleRemoveMember should
be replaced with an accessible, non-blocking confirmation modal: add component
state (e.g., isRemoveModalOpen and pendingRemoveDid) and a UI ConfirmationModal
component; change handleRemoveMember to only set pendingRemoveDid and open the
modal (do not call removeOrgMember directly), then implement modal handlers
(onConfirm and onCancel) where onConfirm calls removeOrgMember(groupDid,
pendingRemoveDid), awaits fetchMembers(), handles errors via setMemberError, and
closes the modal, and onCancel simply clears pendingRemoveDid and closes the
modal; ensure the modal is focus-trapped, keyboard-navigable, and labeled for
accessibility.
In `@src/app/organizations/create/page.tsx`:
- Around line 47-50: The regex check for the organization handle is guarded by
an unnecessary "&& value.length > 1" which is redundant because the subsequent
minimum-length check (the code that sets handle error for <2 chars) already
enforces length; reorder so you validate length first (call the minimum-length
check using the existing logic), then run the regex test, and remove the "&&
value.length > 1" from the regex condition; update the error path to still call
setHandleError("Handle must be lowercase alphanumeric with hyphens") when the
regex fails and keep the existing setHandleError call for the length check—look
for the handle validation block that uses setHandleError and the regex
/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/ in page.tsx.
In `@src/app/organizations/page.tsx`:
- Around line 186-200: The empty-state card currently renders its title with an
h3 element (class org-list__empty-title); change that h3 to an h2 to comply with
the card-title guideline (update the JSX in the component that renders the div
with className "org-list__empty" so the element with className
"org-list__empty-title" is an h2 instead of h3, preserving classes and
surrounding content like the Building2 icon and description).
- Line 28: ROLE_ORDER is defined inside the React component causing it to be
recreated on every render; move the constant to module scope by declaring
ROLE_ORDER: Record<string, number> = { owner: 0, admin: 1, member: 2 } at the
top of the file (outside the component function) so the lookup table is created
once and reused, then remove the in-component declaration and keep all
references within the component unchanged.
- Around line 48-65: The checkCanLeave function is making sequential
listOrgMembers calls causing slow waterfall requests; refactor checkCanLeave to
build an array of promises (e.g., map over organizations and for each org return
a promise that either resolves true/false based on org.role and listOrgMembers
result, catching errors per-promise) and await Promise.all to run them in
parallel, then assemble the resulting map and call setCanLeaveMap; keep
references to checkCanLeave, listOrgMembers, organizations, did, and
setCanLeaveMap when implementing this change.
- Around line 122-163: The three handlers (handleRemovePublicMembership,
handleLeaveOrg, handleAcceptMembership) currently swallow errors; update each
catch block to surface failures by logging the error (e.g., console.error or
processLogger) and showing user feedback (toast/notification) describing the
failure and action attempted (e.g., "Failed to remove membership", "Failed to
leave organization", "Failed to accept membership"); keep existing cleanup steps
(setRemovingPublic(null), setIsLeaving(false), setAcceptingOrg(null)) in finally
blocks and continue calling the same service functions (deleteMembership,
removeOrgMember, putMembership, refetchOrgs) — ensure the notification includes
the groupDid or leaveOrg.groupDid to provide context.
In `@src/app/settings/page.tsx`:
- Line 8: The OrgSettings component is statically imported causing inconsistent
bundle behavior; replace the direct import in page.tsx with a dynamic import
using next/dynamic (e.g., const OrgSettings = dynamic(() =>
import("@/components/organizations/org-settings"), { /* options */ })), matching
how UsernameCard/EmailSection/PasswordSection are loaded; ensure you pass the
same dynamic options used elsewhere (e.g., ssr:false or loading fallback) so
OrgSettings is code-split and lazy-loaded for users who never activate an org.
In `@src/components/landing/home-client.tsx`:
- Around line 86-88: Replace the plain <img> in the component that renders the
banner (the JSX using displayBanner in src/components/landing/home-client.tsx)
with Next.js' Image: import Image from 'next/image' and render <Image
src={displayBanner} alt="" fill /> (or a defined width/height if not using fill)
instead of <img src={displayBanner} alt="" />; also ensure the parent wrapper of
the Image component (the element that currently conditionally renders
displayBanner) has CSS position: relative so the Image fill prop works
correctly, and set appropriate objectFit/object-position via className or style
on the Image to preserve layout.
- Around line 46-53: Replace raw <img> usage with next/image: import Image from
'next/image' at the top of the home-client.tsx component and swap the <img
src="/assets/certified_brandmark_black.svg" className="loading-screen__logo"
alt="" /> (and the other raw img instances referenced) for <Image> components;
for the loading-screen logo (src "/assets/certified_brandmark_black.svg" and
className "loading-screen__logo") use either explicit width/height props or use
layout/fill with an enclosing positioned container, preserve the alt text (make
it meaningful), and keep the same className for styling. Ensure you update any
other raw img occurrences in this file (the ones noted around lines 219 and 234)
to Image as well and adjust imports accordingly.
In `@src/components/layout/navbar.tsx`:
- Line 394: Replace the raw <img> used for the sign-in button in the Navbar
component with Next.js's Image component: add an import for Image from
'next/image', remove the <img src="/assets/sign_in_black_small.svg" ... />
element and render <Image> with the same src and alt, supply explicit width and
height matching the asset (or use layout="intrinsic" with width/height), and
preserve the className "navbar__signin-img" (or move styling to a wrapper) so
styles still apply; update any related props in the Navbar component to use the
Image API.
- Around line 40-51: ROLE_ORDER is defined inside the component causing needless
recreation; move the const ROLE_ORDER: Record<string, number> = { owner: 0,
admin: 1, member: 2 } to module scope (above the component) so it’s created
once, keep its type annotation, and leave sortedOrgs/useMemo unchanged to
continue referencing ROLE_ORDER from the outer scope.
- Line 179: Replace the plain <img> in the Navbar component with Next.js' Image
component: add "import Image from 'next/image'" at the top of the file and swap
the <img src="/assets/certified_wordmark_black_green.png" ... /> JSX for <Image
src="/assets/certified_wordmark_black_green.png" alt="Certified" width={/*
actual width */} height={/* actual height */} className="navbar__logo-img" />
(set the real width/height values); ensure the change is made where the logo
<img> appears in the navbar render so Next's automatic image optimization is
used.
In `@src/components/layout/sidebar.tsx`:
- Around line 46-49: Replace the raw <img> in the sidebar logo block with
Next.js' Image component: import Image from 'next/image' at the top of the
Sidebar component file, then replace the <img
src="/assets/certified_wordmark_white_green.svg" alt="Certified" /> inside the
Link in the sidebar__logo div with an <Image> using the same src and alt and
providing required sizing (width/height or appropriate layout props) so it
matches the visual size; keep the Link href="/" and ensure the import is added
and any className or wrapper styling is preserved.
In `@src/components/organizations/handle-search.tsx`:
- Around line 144-158: handleKeyDown currently only handles Enter; add ArrowDown
and ArrowUp handling to let keyboard users move through search results. Update
or introduce a selected index state (e.g., selectedIndex) and in handleKeyDown
respond to "ArrowDown" by incrementing selectedIndex (wrap to 0 or clamp),
"ArrowUp" by decrementing (wrap to end or clamp), call e.preventDefault() for
those keys, and ensure Enter uses selectedIndex to call
handleSelectActor(results[selectedIndex]) (fall back to resolvedDid logic). Also
ensure the component uses selectedIndex to highlight/announce the active result
(e.g., apply active class or aria-activedescendant) so the visual/focus changes
reflect arrow navigation.
In `@src/components/organizations/org-settings.tsx`:
- Around line 19-25: The ResolvedMember interface is defined between import
statements which violates import grouping; move the entire ResolvedMember
interface declaration so it appears after all import lines (e.g., below imports
like ErrorMessage and LoadingSpinner) while keeping its references to OrgMember
intact so ResolvedMember remains available where used in org-settings.tsx.
- Around line 63-83: The current code in the resolved computation (variable
resolved / type ResolvedMember) issues an authFetch to `/api/resolve-did` per
member inside m.map, causing an N+1 query pattern; change this to batch-resolve
all DIDs in one request (e.g., POST `/api/resolve-dids` with the array of
member.dids) using authFetch once, then merge the returned handles/displayNames
into each member (preserve fallbacks to the original member when data is
missing); alternatively add a simple in-memory cache keyed by did before calling
authFetch to reduce duplicate requests. Ensure you update the mapping logic that
currently references member.did and uses the single-response shape to instead
consume the batch response shape and still return the same ResolvedMember
structure.
- Around line 122-139: handleAddMembers currently awaits addOrgMember
sequentially for each item in pendingMembers which is slow; update
handleAddMembers to run adds in parallel using Promise.allSettled on
pendingMembers.map(m => addOrgMember(groupDid, m.did, newMemberRole)), then
inspect the settled results to detect failures, setAddError with a consolidated
message (or per-member errors) for any rejected results, still call
setPendingMembers([]), reset setNewMemberRole("member"), and await
fetchMembers() and fetchAudit(); keep setIsAdding(true/false) and error handling
flow intact and reference pendingMembers, addOrgMember, setAddError,
setPendingMembers, fetchMembers, fetchAudit, newMemberRole, and setIsAdding when
making the change.
- Around line 31-54: The OrgSettings component currently manages server state
(members, audit entries, add-member flow) with local useState variables like
members, membersLoading, memberError, membersPage, auditEntries, auditLoading,
auditPage, pendingMembers, isAdding and addError; replace these with React
Query: use useQuery for paginated members and audit entries (keys including
groupDid, membersPage/AUDIT_PER_PAGE), useMutation for adding/removing members
(handle pendingMembers/isAdding via mutation status), and use the queryClient
(invalidateQueries or optimistic updates) instead of setMembers/setAuditEntries;
update OrgSettings to remove the local loading/error state vars and wire UI to
query.data, query.isLoading, query.error and mutation.isLoading/isError to keep
behavior identical.
In `@src/components/profile/avatar-upload.tsx`:
- Around line 121-125: The error paragraph rendering in avatar-upload (the JSX
block that checks {error && ( <p className="text-caption ...">{error}</p> )})
needs an ARIA role for accessibility; update the <p> element that displays the
error (the conditional that references the error variable) to include
role="alert" so screen readers announce it immediately.
In `@src/components/profile/banner-upload.tsx`:
- Around line 89-92: Replace the plain <img> in the BannerUpload component with
Next.js Image: import Image from "next/image" and render <Image src={displayUrl}
alt="Profile banner" ...> using appropriate layout/width/height or fill props;
when displayUrl can be a blob URL (preview) pass unoptimized to Image so Next.js
doesn't attempt optimization; ensure any surrounding props (className, style) on
the original <img> are moved to the Image component or a wrapper element.
- Around line 121-125: The error message paragraph in
src/components/profile/banner-upload.tsx currently renders {error} without an
ARIA role; update the JSX that renders the error (the conditional block inside
the BannerUpload component that returns <p className="text-caption
text-red-600">{error}</p>) to include role="alert" so screen readers announce
the error (i.e., add role="alert" to that <p> element).
In `@src/hooks/use-org-profile.ts`:
- Line 74: The exported refetch currently points directly to fetchData so
external callers invoking refetch() send an undefined signal and cannot cancel
in-flight requests—wrap fetchData with a cancellable wrapper before returning it
from useOrgProfile: create a new AbortController inside the wrapper, call
fetchData({ signal: controller.signal }) and return that wrapper as refetch (and
optionally return the controller.abort function or ensure the wrapper returns a
promise with an attached cancel method); update the hook return (currently
returning fetchData) to return this wrapped function so external callers can
cancel the request and avoid state updates after unmount.
In `@src/lib/atproto/did.ts`:
- Around line 15-51: Extract the DID-to-URL mapping logic duplicated between
resolveHandle and resolvePdsUrl into a shared helper (e.g., a new function like
didToUrl or resolveDidUrl) that takes a did string and returns the computed URL
or null; update resolveHandle and resolvePdsUrl to call this helper, preserving
current behavior for "did:plc:" and "did:web:" (including the domain/path
handling and default ".well-known" path) and keep existing timeout/fetch/error
handling in resolveHandle.
In `@src/lib/organizations/api.ts`:
- Around line 416-422: The promise chain in the memberLists creation contains a
redundant .then((members) => members) no-op; simplify organizations.map(...) by
calling listOrgMembers(org.groupDid, signal) directly and keep the .catch(() =>
[] as OrgMember[]) to preserve error handling — update the expression that
builds memberLists (the organizations.map callback that calls listOrgMembers) to
remove the unnecessary .then layer.
- Line 15: Remove the unused imported constant MAX_SELF_CREATED_ORGS from the
import statement in the organizations API module; locate the import line that
currently reads "import { ORG_MEMBERSHIP_COLLECTION, MAX_SELF_CREATED_ORGS }
from \"./constants\"" and delete MAX_SELF_CREATED_ORGS so it only imports
ORG_MEMBERSHIP_COLLECTION, leaving server-side limit logic in the register route
unchanged.
In `@src/lib/organizations/constants.ts`:
- Around line 1-7: The current fallback values for GROUP_SERVICE and
GROUP_SERVICE_DID point to staging and will be used if
NEXT_PUBLIC_GROUP_SERVICE_URL / NEXT_PUBLIC_GROUP_SERVICE_DID are unset; update
the module that defines GROUP_SERVICE and GROUP_SERVICE_DID so that in
production (NODE_ENV === 'production') the absence of these env vars causes an
explicit failure or logged error instead of silently using the staging URLs—use
the existing symbols GROUP_SERVICE and GROUP_SERVICE_DID to locate the
constants, add a runtime check for production builds, and either throw an Error
or emit a clear warning/telemetry event so production does not accidentally hit
staging.
In `@src/lib/organizations/org-context.tsx`:
- Around line 31-39: The loadPersistedOrg function accesses localStorage
directly which can break during SSR; add a defensive check at the top of
loadPersistedOrg (e.g., verify typeof window !== "undefined" and that
window.localStorage exists) and return null immediately if not present, then
keep the existing try/catch/JSON.parse logic using ACTIVE_ORG_KEY to read and
parse the stored Organization; this makes loadPersistedOrg SSR-safe even if
called outside getInitialOrg.
In `@src/lib/organizations/use-org-limit.ts`:
- Around line 9-34: Replace the manual useEffect/useState logic in
useOrgCreationLimit with a react-query useQuery that calls
getSelfCreatedOrgCount (pass did and organizations) to handle loading, error and
caching; remove controller/abort handling and instead map query.isLoading to
isChecking, expose query.error as an error field, and compute limitReached from
query.data (compare against MAX_SELF_CREATED_ORGS); keep using useAuth/useOrg to
get did and organizations and return { selfCreatedCount: query.data ?? null,
isChecking: query.isLoading || orgsLoading, error: query.error, limitReached:
(query.data ?? null) !== null && (query.data ?? 0) >= MAX_SELF_CREATED_ORGS } so
callers can surface failures instead of silently swallowing them.
In `@tests/organizations.test-plan.md`:
- Around line 37-43: Add a new test case to the organizations test plan to
verify the 5-org self-creation limit is enforced: create a new section (e.g.,
"T5a: Create Organization — Limit enforcement") that describes using a user who
already has 5 self-created organizations, navigating to /organizations/create,
asserting a "limit reached" message is shown, and asserting the "Create
Organization" action is disabled or hidden (reference test plan entry T5 and the
create flow to place this new case adjacent to T5). Ensure the case names and
steps clearly state the precondition (user with 5 orgs), the expected UI
message, and the disabled/hidden state of the Create Organization control.
- Around line 75-85: Add an error-state test to the organization settings test
plan by introducing a new test case T11a titled "Organization Settings — Error
handling" that follows the existing T11/T12 structure: describe navigating to
/organizations/{groupDid}/settings, simulate a network failure or server error
when fetching the member list (or activity log), and assert that a user-facing
error message and a retry action/button are displayed; reference the existing
sections "Members & Roles" and "Activity Log" so the test clearly maps to those
UI areas and ensure the steps include how to simulate the failure and the
expected retry behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2cc62a4f-ada0-4bac-8a52-dadabec0a42c
⛔ Files ignored due to path filters (12)
.beads/dolt-server.lockis excluded by!**/*.lock.beads/dolt-server.logis excluded by!**/*.logpublic/assets/certified_brandmark_black.svgis excluded by!**/*.svgpublic/assets/certified_wordmark_black_gray.pngis excluded by!**/*.pngpublic/assets/certified_wordmark_black_gray.svgis excluded by!**/*.svgpublic/assets/certified_wordmark_white_gray.pngis excluded by!**/*.pngpublic/assets/certified_wordmark_white_gray.svgis excluded by!**/*.svgpublic/assets/guilloche_01.svgis excluded by!**/*.svgpublic/assets/guilloche_02.svgis excluded by!**/*.svgpublic/assets/guilloche_03.svgis excluded by!**/*.svgpublic/assets/guilloche_04.svgis excluded by!**/*.svgpublic/assets/sign_in_with_certified_white.svgis excluded by!**/*.svg
📒 Files selected for processing (56)
.beads/.beads-credential-key.beads/backup/backup_state.json.beads/backup/comments.jsonl.beads/backup/config.jsonl.beads/backup/dependencies.jsonl.beads/backup/events.jsonl.beads/backup/issues.jsonl.beads/backup/labels.jsonlAGENTS.mdsrc/app/api/organizations/[groupDid]/audit/route.tssrc/app/api/organizations/[groupDid]/bsky-profile/route.tssrc/app/api/organizations/[groupDid]/handle/route.tssrc/app/api/organizations/[groupDid]/members/route.tssrc/app/api/organizations/[groupDid]/metadata/route.tssrc/app/api/organizations/[groupDid]/profile/route.tssrc/app/api/organizations/[groupDid]/role/route.tssrc/app/api/organizations/[groupDid]/upload-blob/route.tssrc/app/api/organizations/memberships/route.tssrc/app/api/organizations/register/route.tssrc/app/api/resolve-did/route.tssrc/app/api/resolve-handle/route.tssrc/app/api/search-actors/route.tssrc/app/api/xrpc/[...method]/route.tssrc/app/globals.csssrc/app/layout.tsxsrc/app/organizations/[groupDid]/apps/page.tsxsrc/app/organizations/[groupDid]/edit-profile/page.tsxsrc/app/organizations/[groupDid]/page.tsxsrc/app/organizations/[groupDid]/settings/page.tsxsrc/app/organizations/create/page.tsxsrc/app/organizations/layout.tsxsrc/app/organizations/page.tsxsrc/app/settings/edit-profile/page.tsxsrc/app/settings/page.tsxsrc/components/dashboard/username-card.tsxsrc/components/landing/home-client.tsxsrc/components/layout/app-shell.tsxsrc/components/layout/navbar.tsxsrc/components/layout/sidebar.tsxsrc/components/organizations/add-org-modal.tsxsrc/components/organizations/handle-search.tsxsrc/components/organizations/membership-sync-modal.tsxsrc/components/organizations/org-settings.tsxsrc/components/profile/avatar-upload.tsxsrc/components/profile/banner-upload.tsxsrc/components/profile/profile-edit-form.tsxsrc/hooks/use-org-profile.tssrc/lib/atproto/did.tssrc/lib/organizations/api.tssrc/lib/organizations/constants.tssrc/lib/organizations/index.tssrc/lib/organizations/org-context.tsxsrc/lib/organizations/proxy-agent.tssrc/lib/organizations/types.tssrc/lib/organizations/use-org-limit.tstests/organizations.test-plan.md
| const limit = Math.min( | ||
| limitParam !== null ? parseInt(limitParam, 10) : 100, | ||
| 100 | ||
| ) |
There was a problem hiding this comment.
Handle non-numeric limit parameter.
If limitParam is a non-numeric string (e.g., "abc"), parseInt returns NaN, and Math.min(NaN, 100) evaluates to NaN. This would pass "NaN" to the group service.
🐛 Proposed fix
- const limit = Math.min(
- limitParam !== null ? parseInt(limitParam, 10) : 100,
- 100
- )
+ const parsedLimit = limitParam !== null ? parseInt(limitParam, 10) : 100
+ const limit = Math.min(
+ isNaN(parsedLimit) ? 100 : parsedLimit,
+ 100
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/organizations/memberships/route.ts` around lines 22 - 25, The
limit calculation using parseInt can yield NaN for non-numeric limitParam,
causing Math.min(NaN, 100) to become NaN; update the logic in the route where
limitParam is parsed (the code referencing limitParam, parseInt, and Math.min)
to parse into a temporary number, validate it (e.g., ensure it's a finite
integer using Number.isFinite or isNaN check), and fall back to the default
(100) when invalid before applying Math.min so limit is always a valid number <=
100.
| const handleAvatarUpload = async (file: File) => { | ||
| setIsUploadingAvatar(true) | ||
| try { | ||
| const blobRef = await uploadOrgBlob(groupDid, file) | ||
| setAvatarBlob(blobRef) | ||
| } finally { | ||
| setIsUploadingAvatar(false) | ||
| } | ||
| } | ||
|
|
||
| const handleBannerUpload = async (file: File) => { | ||
| setIsUploadingBanner(true) | ||
| try { | ||
| const blobRef = await uploadOrgBlob(groupDid, file) | ||
| setBannerBlob(blobRef) | ||
| } finally { | ||
| setIsUploadingBanner(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
Upload errors are silently swallowed.
If uploadOrgBlob throws, the error is not surfaced to the user. They won't know the upload failed and may save the profile without the intended image.
🐛 Proposed fix to surface upload errors
+ const [uploadError, setUploadError] = useState<string | null>(null)
+
const handleAvatarUpload = async (file: File) => {
setIsUploadingAvatar(true)
+ setUploadError(null)
try {
const blobRef = await uploadOrgBlob(groupDid, file)
setAvatarBlob(blobRef)
+ } catch (err) {
+ setUploadError(err instanceof Error ? err.message : "Failed to upload avatar")
} finally {
setIsUploadingAvatar(false)
}
}
const handleBannerUpload = async (file: File) => {
setIsUploadingBanner(true)
+ setUploadError(null)
try {
const blobRef = await uploadOrgBlob(groupDid, file)
setBannerBlob(blobRef)
+ } catch (err) {
+ setUploadError(err instanceof Error ? err.message : "Failed to upload banner")
} finally {
setIsUploadingBanner(false)
}
}Then display uploadError in the UI alongside saveError.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/organizations/`[groupDid]/edit-profile/page.tsx around lines 85 -
103, handleAvatarUpload and handleBannerUpload currently swallow exceptions from
uploadOrgBlob so users never see upload failures; add an uploadError state
(e.g., uploadError) and in both handlers catch the error, set uploadError to a
user-friendly message (or include error.message) before rethrowing or returning,
ensure setIsUploadingAvatar/setIsUploadingBanner still run in finally, and only
setAvatarBlob/setBannerBlob on success; update the UI to display uploadError
alongside saveError so upload failures are visible to users.
| {isAdmin && member.did !== did && ( | ||
| <button | ||
| className="org-members__remove-btn" | ||
| onClick={() => handleRemoveMember(member.did)} | ||
| title="Remove member" | ||
| > | ||
| <Trash2 size={14} /> | ||
| </button> | ||
| )} |
There was a problem hiding this comment.
Ensure remove button meets 44px minimum touch target.
The remove button uses a 14px icon, which likely doesn't meet the 44px minimum touch target requirement for mobile accessibility. Add padding or min-width/height to the button.
♻️ Proposed CSS fix
.org-members__remove-btn {
min-width: 44px;
min-height: 44px;
display: flex;
align-items: center;
justify-content: center;
}As per coding guidelines: "All interactive elements must have 44px minimum touch targets for mobile accessibility".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/organizations/`[groupDid]/settings/page.tsx around lines 200 - 208,
The remove button with class org-members__remove-btn (used in the JSX that
renders the Trash2 icon and calls handleRemoveMember(member.did)) does not meet
the 44px minimum touch target; update the CSS for .org-members__remove-btn to
enforce min-width:44px and min-height:44px and make it a centered flex container
(display:flex; align-items:center; justify-content:center) so the 14px icon
remains visually centered while meeting mobile accessibility touch-size
requirements.
| // Holds a resolved DID result shown in the dropdown, waiting for user confirmation | ||
| const [resolvedDid, setResolvedDid] = useState<Actor | null>(null) | ||
| const containerRef = useRef<HTMLDivElement>(null) | ||
| const debounceRef = useRef<ReturnType<typeof setTimeout>>(null) |
There was a problem hiding this comment.
Fix TypeScript type for debounceRef to include null.
The ref is initialized with null but the type ReturnType<typeof setTimeout> doesn't include null. This could cause TypeScript errors in strict mode.
🔧 Proposed fix
- const debounceRef = useRef<ReturnType<typeof setTimeout>>(null)
+ const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const debounceRef = useRef<ReturnType<typeof setTimeout>>(null) | |
| const debounceRef = useRef<ReturnType<typeof setTimeout> | null>(null) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/organizations/handle-search.tsx` at line 44, The ref
debounceRef is typed as useRef<ReturnType<typeof setTimeout>> but initialized
with null which conflicts in strict TS; update the type to allow null (e.g.,
useRef<ReturnType<typeof setTimeout> | null>) so debounceRef can be initialized
to null and later hold the timer handle; locate the debounceRef declaration to
change its generic type accordingly.
| <button className="signin-modal__close" onClick={onClose}> | ||
| <X size={18} /> | ||
| </button> |
There was a problem hiding this comment.
Close button touch target is below 44px minimum.
Same issue as add-org-modal.tsx - the close button uses the 32x32px signin-modal__close class. Consider updating the shared CSS class to 44px for all modals.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/organizations/membership-sync-modal.tsx` around lines 37 - 39,
The close button in membership-sync-modal.tsx uses the signin-modal__close class
which provides a 32x32 touch target; update the shared CSS rule for
.signin-modal__close (used by membership-sync-modal.tsx and add-org-modal.tsx)
to make the clickable area at least 44x44px (adjust height/width/padding or
min-width/min-height) and ensure visual centering of the X icon (component X)
inside that box; if the class is used elsewhere, verify there are no layout
regressions or create a new modal-close class and replace signin-modal__close on
modal components that need the larger touch target.
| const [auditLoading, setAuditLoading] = useState(false) | ||
| const [auditPage, setAuditPage] = useState(0) | ||
| const AUDIT_PER_PAGE = 20 |
There was a problem hiding this comment.
Initialize auditLoading to true to prevent flash of "No activity recorded yet."
Per coding guidelines, isLoading states should initialize to true to prevent flash of empty content. Since fetchAudit is called on mount, initializing to false may briefly show "No activity recorded yet." before the loading spinner appears.
🔧 Proposed fix
// Audit log
const [auditEntries, setAuditEntries] = useState<AuditEntry[]>([])
- const [auditLoading, setAuditLoading] = useState(false)
+ const [auditLoading, setAuditLoading] = useState(true)
const [auditPage, setAuditPage] = useState(0)As per coding guidelines: "isLoading must initialize to true (not false) to prevent flash of empty content".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [auditLoading, setAuditLoading] = useState(false) | |
| const [auditPage, setAuditPage] = useState(0) | |
| const AUDIT_PER_PAGE = 20 | |
| const [auditLoading, setAuditLoading] = useState(true) | |
| const [auditPage, setAuditPage] = useState(0) | |
| const AUDIT_PER_PAGE = 20 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/organizations/org-settings.tsx` around lines 51 - 53, The
auditLoading state is initialized to false which can cause a brief flash of "No
activity recorded yet." before fetchAudit runs; update the useState
initialization for auditLoading in the org-settings component to true (change
the useState(false) call for auditLoading to useState(true)) and ensure any
places that set loading (setAuditLoading) still toggle appropriately around
fetchAudit (e.g., setAuditLoading(true) before fetch and setAuditLoading(false)
after) so the spinner displays on mount instead of the empty message.
| onChange={handleDescriptionChange} | ||
| rows={4} | ||
| maxLength={2560} | ||
| maxLength={256} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any schema definitions or constants that define profile field limits
rg -n -i "maxLength|max_length|maxGraphemes|maxChars" --type=ts --type=json -C2
# Check if there are any lexicon or schema files defining profile constraints
fd -e json -e ts lexicon schema | xargs -I{} grep -l -i "profile\|displayName\|description" {} 2>/dev/null | head -10Repository: hypercerts-org/certified-app
Length of output: 4785
Align character limits with schema constraints, accounting for byte encoding.
The form uses maxLength={64} for displayName and maxLength={256} for description, matching the schema's graphemes limits. However, the schema defines separate byte limits (640 and 2560 in src/lib/atproto/types.ts). HTML's maxLength restricts character count, not byte length—multi-byte Unicode characters could exceed the backend's byte limits. Enforce both graphemes and byte limits, or ensure the character limit is restrictive enough to prevent byte-limit violations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/profile/profile-edit-form.tsx` at line 253, The displayName
and description inputs in profile-edit-form.tsx currently only use HTML
maxLength (e.g., maxLength={64} and maxLength={256}) which limits characters not
bytes and can allow multi-byte Unicode to exceed backend byte limits from
src/lib/atproto/types.ts (640 and 2560 bytes). Add a UTF-8 byte-length check:
implement a small helper (e.g., utf8ByteLength(str)) and enforce it in the
displayName and description onChange handlers (or validation routine) to trim or
reject input that would exceed the byte limits; also run the same byte-length
validation on form submit to prevent backend rejections. Ensure the UI still
enforces grapheme limits (keep maxLength for characters) but add byte-limit
validation using the helper and show a clear error message when the byte limit
is reached.
| const { isAuthenticated, isLoading: authLoading, did } = useAuth() | ||
| const [activeOrg, setActiveOrg] = useState<Organization | null>(getInitialOrg) | ||
| const [organizations, setOrganizations] = useState<Organization[]>([]) | ||
| const [isLoading, setIsLoading] = useState(false) |
There was a problem hiding this comment.
isLoading should initialize to true to prevent flash of empty content.
Per coding guidelines, isLoading must initialize to true. Currently set to false, which may cause a brief flash of the empty state before the organizations list loads.
🐛 Proposed fix
- const [isLoading, setIsLoading] = useState(false)
+ const [isLoading, setIsLoading] = useState(true)Note: You may also need to ensure setIsLoading(false) is called when auth is not authenticated (around line 78-83) to avoid an infinite loading state for logged-out users.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [isLoading, setIsLoading] = useState(false) | |
| const [isLoading, setIsLoading] = useState(true) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/organizations/org-context.tsx` at line 63, Change the isLoading
useState to initialize true (const [isLoading, setIsLoading] = useState(true))
so the UI doesn't flash empty content, and ensure the auth-check path calls
setIsLoading(false) when the user is not authenticated (where the auth logic
currently branches) to avoid an infinite loading state; update the relevant
effect or handler that references isLoading/setIsLoading in this module
(org-context.tsx) so both the authenticated and unauthenticated code paths
always setIsLoading(false) after resolution.
- Remove credential key from source control and add to .gitignore
- Fix maxLength={640} to match 64-char validation on org name input
- Paginate member list in register route limit check (>100 members)
- Init auditLoading to true to prevent flash of empty state
- Init org context isLoading to true to prevent flash of empty content
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/lib/organizations/org-context.tsx (1)
77-82:⚠️ Potential issue | 🟡 MinorStill clear
isLoadingon the logged-out path.This branch returns before the
finally, so once auth resolves unauthenticated,isLoadingcan staytrueindefinitely for everyuseOrg()consumer.🐛 Proposed fix
if (!isAuthenticated || !did) { setOrganizations([]) setActiveOrg(null) persistOrg(null) + setIsLoading(false) return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/organizations/org-context.tsx` around lines 77 - 82, The logged-out branch returns early without clearing the local loading state, leaving useOrg() consumers stuck; before returning from the "if (!isAuthenticated || !did)" block call the loading setter (e.g., setIsLoading(false) or setLoading(false) consistent with this file) so that isLoading is cleared, then perform setOrganizations([]), setActiveOrg(null), persistOrg(null) and return; alternatively, remove the early return and ensure the existing finally/cleanup path always calls setIsLoading(false) so the loading flag is reset for unauthenticated flows.
🧹 Nitpick comments (2)
src/app/organizations/create/page.tsx (1)
199-247: Use a real form for the create flow.Submit currently only works through the button click handler. Wrapping these controls in
<form onSubmit>restores Enter-key submission and native form semantics; once you do, keep the Back/Cancel actions astype="button".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/organizations/create/page.tsx` around lines 199 - 247, Wrap the name/handle inputs, error display, and action buttons in a real <form> and attach onSubmit to the existing handleCreate so Enter submits; either update handleCreate to accept an event and call event.preventDefault() or wrap with a small onSubmit={(e)=>{e.preventDefault(); handleCreate()}}. Ensure the Cancel/Back button remains type="button" and the Create Organization button is type="submit" (keep its loading/disabled props and reference isCreating), leaving Input handlers (setName/validateName, setHandle/validateHandle) and ErrorMessage usage unchanged.src/lib/organizations/org-context.tsx (1)
60-128: Back the organizations query with React Query instead of bespoke fetch state.This provider is manually handling query lifecycle concerns (
isLoading, aborts, refetch, error swallowing). KeepingactiveOrgin context but movingresolveOrganizations(did)behinduseQuerywould align with the repo standard and simplify this state machine.As per coding guidelines, "Use React Query (
@tanstack/react-query) for server state management."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/organizations/org-context.tsx` around lines 60 - 128, Replace the bespoke fetchOrgs state machine with a React Query useQuery that calls resolveOrganizations(did) and drives organizations state; specifically remove fetchOrgs, the AbortController effect, isLoading state and error swallowing, and instead create a useQuery keyed by ['organizations', did] that onSuccess sets setOrganizations and updates activeOrg (using the same logic that finds prev by groupDid and calls persistOrg or clears it); keep switchOrg and persistOrg as-is, and implement refetchOrgs to call the query's refetch function. Ensure the query is disabled while authLoading or !isAuthenticated and that Abort behavior is handled by React Query (no manual AbortController).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/api/organizations/register/route.ts`:
- Around line 52-60: The code currently treats failed fetches (membershipsRes.ok
false) and other downstream failures as soft failures and proceeds to
group.register, which undercounts orgs; change the logic in the registration
flow so any non-ok membership fetch or service-auth/members-list failure
immediately aborts the request with a 5xx response instead of setting
cursor/continuing. Specifically, in the block handling membershipsRes (and the
analogous blocks around lines 65-87 and 98-100), detect non-ok responses and
throw or return an HTTP 5xx error (include the fetch response status/text)
rather than setting cursor = undefined or pushing partial data; ensure the
calling handler returns that 5xx to stop invocation of group.register and any
subsequent counting logic (refer to membershipsRes, membershipToken, cursor,
allGroups, and the group.register call when locating the code).
In `@src/app/organizations/create/page.tsx`:
- Around line 119-135: When isChecking is true in the Create Organization page
(the conditional block using isChecking in
src/app/organizations/create/page.tsx), replace the silent page shell with an
explicit loading region: render a visible spinner/icon plus loading text such as
"Checking organization limits…" and add role="status" (and optionally
aria-live="polite") on the container so assistive tech announces the state; keep
the existing topbar/back button layout but show the loading UI in the main
content area so screen readers and sighted users see the check in progress.
---
Duplicate comments:
In `@src/lib/organizations/org-context.tsx`:
- Around line 77-82: The logged-out branch returns early without clearing the
local loading state, leaving useOrg() consumers stuck; before returning from the
"if (!isAuthenticated || !did)" block call the loading setter (e.g.,
setIsLoading(false) or setLoading(false) consistent with this file) so that
isLoading is cleared, then perform setOrganizations([]), setActiveOrg(null),
persistOrg(null) and return; alternatively, remove the early return and ensure
the existing finally/cleanup path always calls setIsLoading(false) so the
loading flag is reset for unauthenticated flows.
---
Nitpick comments:
In `@src/app/organizations/create/page.tsx`:
- Around line 199-247: Wrap the name/handle inputs, error display, and action
buttons in a real <form> and attach onSubmit to the existing handleCreate so
Enter submits; either update handleCreate to accept an event and call
event.preventDefault() or wrap with a small onSubmit={(e)=>{e.preventDefault();
handleCreate()}}. Ensure the Cancel/Back button remains type="button" and the
Create Organization button is type="submit" (keep its loading/disabled props and
reference isCreating), leaving Input handlers (setName/validateName,
setHandle/validateHandle) and ErrorMessage usage unchanged.
In `@src/lib/organizations/org-context.tsx`:
- Around line 60-128: Replace the bespoke fetchOrgs state machine with a React
Query useQuery that calls resolveOrganizations(did) and drives organizations
state; specifically remove fetchOrgs, the AbortController effect, isLoading
state and error swallowing, and instead create a useQuery keyed by
['organizations', did] that onSuccess sets setOrganizations and updates
activeOrg (using the same logic that finds prev by groupDid and calls persistOrg
or clears it); keep switchOrg and persistOrg as-is, and implement refetchOrgs to
call the query's refetch function. Ensure the query is disabled while
authLoading or !isAuthenticated and that Abort behavior is handled by React
Query (no manual AbortController).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 25b13b2c-7683-40ed-a93e-1db30f3c898b
📒 Files selected for processing (5)
.gitignoresrc/app/api/organizations/register/route.tssrc/app/organizations/create/page.tsxsrc/components/organizations/org-settings.tsxsrc/lib/organizations/org-context.tsx
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/organizations/org-settings.tsx
| // Check org creation limit: fetch all memberships, then check addedBy | ||
| try { | ||
| const { data: { token: membershipToken } } = | ||
| await auth.agent.com.atproto.server.getServiceAuth({ | ||
| aud: GROUP_SERVICE_DID, | ||
| lxm: "app.certified.groups.membership.list", | ||
| }) | ||
|
|
||
| const allGroups: { groupDid: string }[] = [] | ||
| let cursor: string | undefined | ||
| do { | ||
| const url = new URL(`${GROUP_SERVICE}/xrpc/app.certified.groups.membership.list`) | ||
| url.searchParams.set("limit", "100") | ||
| if (cursor) url.searchParams.set("cursor", cursor) | ||
|
|
||
| const membershipsRes = await fetch(url.toString(), { | ||
| headers: { Authorization: `Bearer ${membershipToken}` }, | ||
| }) | ||
| if (membershipsRes.ok) { | ||
| const data = await membershipsRes.json() | ||
| allGroups.push(...(data.groups || [])) | ||
| cursor = data.cursor | ||
| } else { | ||
| cursor = undefined | ||
| } | ||
| } while (cursor) | ||
|
|
||
| // For each group, check if the user's member entry has addedBy === ownerDid | ||
| const results = await Promise.all( | ||
| allGroups.map(async (g) => { | ||
| try { | ||
| const groupAgent = createGroupAgent(auth.agent, g.groupDid) | ||
| const allMembers: { did: string; addedBy: string }[] = [] | ||
| let memberCursor: string | undefined | ||
| do { | ||
| const params: Record<string, unknown> = { limit: 100 } | ||
| if (memberCursor) params.cursor = memberCursor | ||
| const { data } = await groupAgent.call( | ||
| "app.certified.group.member.list", | ||
| params | ||
| ) | ||
| const page = data as { members?: { did: string; addedBy: string }[]; cursor?: string } | ||
| allMembers.push(...(page.members || [])) | ||
| memberCursor = page.cursor | ||
| } while (memberCursor) | ||
| return allMembers.some( | ||
| (m) => m.did === ownerDid && m.addedBy === ownerDid | ||
| ) | ||
| } catch { | ||
| return false | ||
| } | ||
| }) | ||
| ) | ||
| const selfCreatedCount = results.filter(Boolean).length | ||
|
|
||
| if (selfCreatedCount >= MAX_SELF_CREATED_ORGS) { | ||
| return NextResponse.json( | ||
| { error: `You have reached the maximum number of organizations you can create (${MAX_SELF_CREATED_ORGS})` }, | ||
| { status: 403 } | ||
| ) | ||
| } | ||
| } catch { | ||
| // If limit check fails, allow creation (fail open) | ||
| } | ||
|
|
||
| // Get service auth JWT for group registration | ||
| const token = await getServiceAuthToken( | ||
| auth.agent, | ||
| "app.certified.group.register" | ||
| ) | ||
|
|
||
| // Call the group service directly (registration is the only direct call) | ||
| const res = await fetch( | ||
| `${GROUP_SERVICE}/xrpc/app.certified.group.register`, | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| Authorization: `Bearer ${token}`, | ||
| }, | ||
| body: JSON.stringify({ handle, ownerDid, email }), | ||
| } | ||
| ) |
There was a problem hiding this comment.
The 5-org cap is still bypassable via concurrent requests.
The quota check and the registration call are separate operations. Two parallel POSTs from the same DID can both observe selfCreatedCount < MAX_SELF_CREATED_ORGS and both succeed, which lets the caller overshoot the limit. This needs atomic enforcement in the registration backend, or a server-side lock/transaction around check + create.
| const membershipsRes = await fetch(url.toString(), { | ||
| headers: { Authorization: `Bearer ${membershipToken}` }, | ||
| }) | ||
| if (membershipsRes.ok) { | ||
| const data = await membershipsRes.json() | ||
| allGroups.push(...(data.groups || [])) | ||
| cursor = data.cursor | ||
| } else { | ||
| cursor = undefined |
There was a problem hiding this comment.
Don't fail open when quota enforcement is unavailable.
Any service-auth failure, membership-page failure, or per-group member-list failure currently degrades into a partial/zero count and still proceeds to group.register. That makes the “server-side” 5-org cap disappear exactly when a dependency is degraded. Return a 5xx and stop the create instead of undercounting.
Also applies to: 65-87, 98-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/organizations/register/route.ts` around lines 52 - 60, The code
currently treats failed fetches (membershipsRes.ok false) and other downstream
failures as soft failures and proceeds to group.register, which undercounts
orgs; change the logic in the registration flow so any non-ok membership fetch
or service-auth/members-list failure immediately aborts the request with a 5xx
response instead of setting cursor/continuing. Specifically, in the block
handling membershipsRes (and the analogous blocks around lines 65-87 and
98-100), detect non-ok responses and throw or return an HTTP 5xx error (include
the fetch response status/text) rather than setting cursor = undefined or
pushing partial data; ensure the calling handler returns that 5xx to stop
invocation of group.register and any subsequent counting logic (refer to
membershipsRes, membershipToken, cursor, allGroups, and the group.register call
when locating the code).
| if (isChecking) { | ||
| return ( | ||
| <div className="dashboard"> | ||
| <div className="dashboard__topbar"> | ||
| <h1 className="dashboard__page-title">Create Organization</h1> | ||
| <div className="dashboard__topbar-right"> | ||
| <button | ||
| className="dashboard__back-btn" | ||
| onClick={() => router.push("/organizations")} | ||
| > | ||
| <ArrowLeft size={16} /> | ||
| Back | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) |
There was a problem hiding this comment.
Announce the limit check as a loading state.
While isChecking is true, this renders only the page shell. Add visible loading copy/spinner plus role="status" so the page is not silent for assistive tech.
As per coding guidelines, "Use semantic HTML: dl/dt/dd for key-value pairs, h2 for card titles (not h3), ARIA roles on errors (role="alert"), loading states (role="status")."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/organizations/create/page.tsx` around lines 119 - 135, When
isChecking is true in the Create Organization page (the conditional block using
isChecking in src/app/organizations/create/page.tsx), replace the silent page
shell with an explicit loading region: render a visible spinner/icon plus
loading text such as "Checking organization limits…" and add role="status" (and
optionally aria-live="polite") on the container so assistive tech announces the
state; keep the existing topbar/back button layout but show the loading UI in
the main content area so screen readers and sighted users see the check in
progress.
Summary
Key changes
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Improvements