feat(teams): add teams as first-class org primitive#4403
Conversation
Adds teams + team_members tables under better-auth's organization plugin, plus a Settings → Teams UI for CRUD and member management. Invariants (Linear-style): - Every org has ≥1 team. Default team is seeded on org creation; backfill gives every existing org one team named after the org. - New org members auto-land in the oldest team. Existing members are backfilled into the default team so pre-teams users aren't dropped into an empty state. - Removing a team member is blocked if it would leave them with zero teams in that org (beforeRemoveTeamMember hook). - Deleting a team re-homes any would-be orphans into the next-oldest team before the FK cascade fires (beforeDeleteTeam hook). - Deleting the org's only team is blocked (allowRemovingAllTeams: false). - Leaving the org clears all team_members rows for that user. Implementation notes: - team_members.organization_id is denormalized via BEFORE INSERT trigger so Electric can shape-filter with a plain equality. - Unique (team_id, user_id) prevents duplicate memberships. - Reads use Electric collections + useLiveQuery; writes go through tRPC wrappers around ctx.auth.api.* so better-auth hooks fire server-side. - Settings layout grows a real top chrome (NavigationControls) so users can back out of settings via the same affordance as the workspace UI.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR implements teams as a core organizational feature: adds DB schema and migration, configures Better Auth for teams with lifecycle hooks, exposes TRPC team mutations, syncs teams via Electric collections, extends settings navigation with a Teams section, and adds desktop UI for listing, creating, editing teams and managing membership. ChangesTeams Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Sequence Diagram(s)sequenceDiagram
participant DesktopUI
participant ElectricCollections
participant TRPC_Server
participant Auth_Service
participant Database
DesktopUI->>ElectricCollections: subscribe teams/teamMembers
DesktopUI->>TRPC_Server: team.addMember(teamId,userId)
TRPC_Server->>Auth_Service: ctx.auth.api.addTeamMember
Auth_Service->>Database: insert auth.team_members
Database-->>ElectricCollections: changefeed emits update
ElectricCollections-->>DesktopUI: UI updates
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Old desktop clients still go through afterAddMember when they add an org member. If a stale (team_id, user_id) row exists from a prior race, the hook's insert would throw a unique violation and fail the entire addMember call — breaking old clients on flows they don't know anything about. Make the insert idempotent. Same .onConflictDoNothing on the backfill.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Greptile SummaryThis PR introduces
Confidence Score: 3/5Two present defects on the core member-management paths need to be fixed before merging. The afterAddMember hook inserts into team_members without a conflict guard, so a duplicate key error aborts the entire add-org-member flow for any user already holding that membership. The tRPC addMember and removeMember procedures never inspect the response from ctx.auth.api.*, so hook-level rejections (including the at-least-one-team invariant) are silently swallowed and the client sees success regardless. packages/auth/src/server.ts (afterAddMember insert) and packages/trpc/src/router/team/team.ts (unchecked auth API responses)
|
| Filename | Overview |
|---|---|
| packages/auth/src/server.ts | Adds team lifecycle hooks; afterAddMember is missing .onConflictDoNothing() and beforeDeleteTeam silently no-ops when no fallback team exists |
| packages/trpc/src/router/team/team.ts | New tRPC team router; neither addMember nor removeMember checks ctx.auth.api.* responses, so hook rejections are silently swallowed |
| packages/db/drizzle/0049_add_teams.sql | Creates auth.teams and auth.team_members with correct indexes, FK cascades, trigger, and backfills; comment incorrectly states new members are not auto-added |
| packages/db/src/schema/auth.ts | Clean Drizzle schema definitions for teams and teamMembers matching the SQL migration; indexes and unique constraints are consistent |
| apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/TeamDetailSettings.tsx | Team detail page with rename, slug edit, member table, leave and delete flows; error handling and navigation are correct |
| apps/electric-proxy/src/where.ts | Adds Electric shape-filter clauses for auth.teams and auth.team_members using the denormalized organizationId; correct |
| apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | Adds Electric-backed teams and teamMembers collections scoped by organizationId; consistent with existing pattern |
Sequence Diagram
sequenceDiagram
participant UI as Settings UI
participant tRPC as tRPC team router
participant BA as better-auth server
participant DB as Postgres (auth schema)
Note over UI,DB: Add member to team
UI->>tRPC: team.addMember(teamId, userId)
tRPC->>tRPC: requireActiveOrgId + verifyOrgAdmin
tRPC->>BA: ctx.auth.api.addTeamMember(...)
BA->>DB: INSERT INTO team_members (trigger fills org_id)
BA-->>tRPC: response (error not checked)
tRPC-->>UI: "{ success: true }"
Note over UI,DB: Remove member from team
UI->>tRPC: team.removeMember(teamId, userId)
tRPC->>BA: ctx.auth.api.removeTeamMember(...)
BA->>BA: beforeRemoveTeamMember hook
BA-->>tRPC: response (error not checked)
tRPC-->>UI: "{ success: true }"
Note over DB: Org add member lifecycle
DB-->>BA: afterAddMember hook fires
BA->>DB: INSERT INTO team_members (no conflict guard)
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
packages/auth/src/server.ts:519-525
**`afterAddMember` missing `onConflictDoNothing` guard**
The direct `db.insert(authSchema.teamMembers)` call in `afterAddMember` has no conflict guard. `team_members` carries a unique index on `(team_id, user_id)`, so if the row already exists — e.g., after a manual backfill or a retry of a failed invite — Postgres throws a unique-constraint violation that bubbles up through better-auth and causes the entire "add org member" operation to fail. The sibling `beforeDeleteTeam` handler already adds `.onConflictDoNothing()` for exactly this reason; `afterAddMember` should do the same.
### Issue 2 of 4
packages/trpc/src/router/team/team.ts:36-66
**`ctx.auth.api.*` return value not inspected**
Both `addMember` and `removeMember` `await` the better-auth server API call and then unconditionally `return { success: true }`. better-auth's server API can return an error response object rather than throwing — for example when the `beforeRemoveTeamMember` hook rejects with "You should be a member of at least one team". In that case the error is silently swallowed and the tRPC caller receives `{ success: true }`, so the client never shows the blocking toast and the invariant appears bypassed. The response should be checked and a `TRPCError` thrown when the auth call fails.
### Issue 3 of 4
packages/db/drizzle/0049_add_teams.sql:52-55
The comment says "new org members are NOT auto-added", but the `afterAddMember` hook in `server.ts` explicitly auto-adds every new org member to the oldest team. The comment contradicts the actual behaviour and will mislead anyone reading this migration in the future.
```suggestion
-- Backfill: every existing org member is added to that org's default team
-- so pre-teams users don't land on an empty Teams page. Going forward, new
-- org members are automatically placed in the oldest team via the
-- afterAddMember hook. organization_id is filled in by the trigger above.
```
### Issue 4 of 4
packages/auth/src/server.ts:433-437
**Silent no-op when `nextTeam` is null in `beforeDeleteTeam`**
If `nextTeam` is `null` and there are orphans, the hook returns without re-homing them. `allowRemovingAllTeams: false` is supposed to prevent this scenario, but if it is ever toggled or better-auth's guard misfires, members are silently left with zero teams and no error is surfaced. Consider throwing an error or logging a warning instead of a silent `return`.
Reviews (1): Last reviewed commit: "fix(teams): harden hooks against duplica..." | Re-trigger Greptile
| teamId: defaultTeam.id, | ||
| userId: member.userId, | ||
| organizationId: organization.id, | ||
| }) | ||
| .onConflictDoNothing(); | ||
| } | ||
|
|
There was a problem hiding this comment.
afterAddMember missing onConflictDoNothing guard
The direct db.insert(authSchema.teamMembers) call in afterAddMember has no conflict guard. team_members carries a unique index on (team_id, user_id), so if the row already exists — e.g., after a manual backfill or a retry of a failed invite — Postgres throws a unique-constraint violation that bubbles up through better-auth and causes the entire "add org member" operation to fail. The sibling beforeDeleteTeam handler already adds .onConflictDoNothing() for exactly this reason; afterAddMember should do the same.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/auth/src/server.ts
Line: 519-525
Comment:
**`afterAddMember` missing `onConflictDoNothing` guard**
The direct `db.insert(authSchema.teamMembers)` call in `afterAddMember` has no conflict guard. `team_members` carries a unique index on `(team_id, user_id)`, so if the row already exists — e.g., after a manual backfill or a retry of a failed invite — Postgres throws a unique-constraint violation that bubbles up through better-auth and causes the entire "add org member" operation to fail. The sibling `beforeDeleteTeam` handler already adds `.onConflictDoNothing()` for exactly this reason; `afterAddMember` should do the same.
How can I resolve this? If you propose a fix, please make it concise.| await ctx.auth.api.addTeamMember({ | ||
| body: { teamId: input.teamId, userId: input.userId }, | ||
| headers: ctx.headers, | ||
| }); | ||
| return { success: true }; | ||
| }), | ||
|
|
||
| removeMember: protectedProcedure | ||
| .input( | ||
| z.object({ | ||
| teamId: z.string().uuid(), | ||
| userId: z.string().uuid(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const organizationId = requireActiveOrgId(ctx); | ||
| const isSelf = input.userId === ctx.session.user.id; | ||
| if (!isSelf) { | ||
| await verifyOrgAdmin(ctx.session.user.id, organizationId); | ||
| } | ||
| await requireTeamInActiveOrg(input.teamId, organizationId); | ||
|
|
||
| // The ≥1-team invariant is enforced by the beforeRemoveTeamMember | ||
| // org hook, so any caller (this procedure, direct authClient, future | ||
| // API surfaces) gets the same guarantee. | ||
| await ctx.auth.api.removeTeamMember({ | ||
| body: { teamId: input.teamId, userId: input.userId }, | ||
| headers: ctx.headers, | ||
| }); | ||
| return { success: true }; | ||
| }), |
There was a problem hiding this comment.
ctx.auth.api.* return value not inspected
Both addMember and removeMember await the better-auth server API call and then unconditionally return { success: true }. better-auth's server API can return an error response object rather than throwing — for example when the beforeRemoveTeamMember hook rejects with "You should be a member of at least one team". In that case the error is silently swallowed and the tRPC caller receives { success: true }, so the client never shows the blocking toast and the invariant appears bypassed. The response should be checked and a TRPCError thrown when the auth call fails.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/trpc/src/router/team/team.ts
Line: 36-66
Comment:
**`ctx.auth.api.*` return value not inspected**
Both `addMember` and `removeMember` `await` the better-auth server API call and then unconditionally `return { success: true }`. better-auth's server API can return an error response object rather than throwing — for example when the `beforeRemoveTeamMember` hook rejects with "You should be a member of at least one team". In that case the error is silently swallowed and the tRPC caller receives `{ success: true }`, so the client never shows the blocking toast and the invariant appears bypassed. The response should be checked and a `TRPCError` thrown when the auth call fails.
How can I resolve this? If you propose a fix, please make it concise.| -- Backfill: every existing org member is added to that org's default team | ||
| -- so pre-teams users don't land on an empty Teams page. Going forward | ||
| -- membership is opt-in (new org members are NOT auto-added). organization_id | ||
| -- is filled in by the trigger above. |
There was a problem hiding this comment.
The comment says "new org members are NOT auto-added", but the
afterAddMember hook in server.ts explicitly auto-adds every new org member to the oldest team. The comment contradicts the actual behaviour and will mislead anyone reading this migration in the future.
| -- Backfill: every existing org member is added to that org's default team | |
| -- so pre-teams users don't land on an empty Teams page. Going forward | |
| -- membership is opt-in (new org members are NOT auto-added). organization_id | |
| -- is filled in by the trigger above. | |
| -- Backfill: every existing org member is added to that org's default team | |
| -- so pre-teams users don't land on an empty Teams page. Going forward, new | |
| -- org members are automatically placed in the oldest team via the | |
| -- afterAddMember hook. organization_id is filled in by the trigger above. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/db/drizzle/0049_add_teams.sql
Line: 52-55
Comment:
The comment says "new org members are NOT auto-added", but the `afterAddMember` hook in `server.ts` explicitly auto-adds every new org member to the oldest team. The comment contradicts the actual behaviour and will mislead anyone reading this migration in the future.
```suggestion
-- Backfill: every existing org member is added to that org's default team
-- so pre-teams users don't land on an empty Teams page. Going forward, new
-- org members are automatically placed in the oldest team via the
-- afterAddMember hook. organization_id is filled in by the trigger above.
```
How can I resolve this? If you propose a fix, please make it concise.| orphanUserIds.map((userId) => ({ | ||
| teamId: nextTeam.id, | ||
| userId, | ||
| organizationId: team.organizationId, | ||
| })), |
There was a problem hiding this comment.
Silent no-op when
nextTeam is null in beforeDeleteTeam
If nextTeam is null and there are orphans, the hook returns without re-homing them. allowRemovingAllTeams: false is supposed to prevent this scenario, but if it is ever toggled or better-auth's guard misfires, members are silently left with zero teams and no error is surfaced. Consider throwing an error or logging a warning instead of a silent return.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/auth/src/server.ts
Line: 433-437
Comment:
**Silent no-op when `nextTeam` is null in `beforeDeleteTeam`**
If `nextTeam` is `null` and there are orphans, the hook returns without re-homing them. `allowRemovingAllTeams: false` is supposed to prevent this scenario, but if it is ever toggled or better-auth's guard misfires, members are silently left with zero teams and no error is surfaced. Consider throwing an error or logging a warning instead of a silent `return`.
How can I resolve this? If you propose a fix, please make it concise.The duplicate-row scenario the defensive guard was protecting against isn't actually reachable through any flow.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/TeamDetailSettings.tsx (1)
253-259: ⚡ Quick winConsider memoizing the member user ID set.
The expression
new Set(members.map((m) => m.userId))creates a new Set instance on every render, potentially causing unnecessary re-renders ofAddMemberButton. Wrap it inuseMemoto avoid this.♻️ Proposed fix
Add near the top of the component (after the
membersderivation around line 106):+ const currentMemberUserIds = useMemo( + () => new Set(members.map((m) => m.userId)), + [members], + ); + const currentMember = members.find((m) => m.userId === currentUserId);Then update the JSX:
<AddMemberButton teamId={teamId} currentUserId={currentUserId} - currentMemberUserIds={new Set(members.map((m) => m.userId))} + currentMemberUserIds={currentMemberUserIds} orgUsers={orgUsers ?? []} />Don't forget to import
useMemofrom React.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/routes/_authenticated/settings/teams/`$teamId/components/TeamDetailSettings/TeamDetailSettings.tsx around lines 253 - 259, The expression new Set(members.map((m) => m.userId)) used as the currentMemberUserIds prop recreates a Set on every render and can trigger unnecessary re-renders of AddMemberButton; wrap that computation in React's useMemo (import useMemo) to memoize the Set based on members (e.g., compute currentMemberUserIds = useMemo(() => new Set(members.map(m => m.userId)), [members])) near the top of the component after members is derived, and then pass that memoized currentMemberUserIds to AddMemberButton instead of creating a new Set inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`:
- Around line 479-509: The teamMembers collection id uses a hyphenated name but
should preserve the table underscore to match existing naming conventions;
update the id passed into electricCollectionOptions for the
createPersistedElectricCollection call that constructs teamMembers (look for the
teamMembers variable and its electricCollectionOptions call) to use
`team_members-${organizationId}` instead of `team-members-${organizationId}` so
the collection id matches the table naming pattern.
In `@apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx`:
- Around line 49-50: The SECTION_ORDER constant used to pick the first matching
section is missing "teams", so when search is active a teams-only route won't be
auto-selected; update the SECTION_ORDER array to include "teams" in the
appropriate place in the ordering (consistent with where other settings sections
live) so the code path that reads SECTION_ORDER (used to pick the first matching
section) will consider teams; ensure any other logic that enumerates sections
(the pathname.includes checks for "/settings/teams") stays consistent with the
new SECTION_ORDER.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/components/CreateTeamButton/CreateTeamButton.tsx`:
- Around line 57-71: The createTeam call can throw but the current try...finally
only checks result.error; wrap the await authClient.organization.createTeam(...)
in a try/catch/finally (or add a catch block) so thrown exceptions are caught,
call toast.error with a helpful message including error.message (or a fallback
like "Failed to create team"), optionally log the error, and return early to
avoid continuing to toast.success/reset/setIsOpen; keep the existing
setIsSubmitting(false) in the finally block so submission state is always
cleared.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/TeamsSettings.tsx`:
- Around line 86-94: The table row navigation is mouse-only; make the TableRow
focusable and keyboard-activatable so keyboard users can open team details: add
tabIndex={0}, role="button" (or an appropriate ARIA role) and a descriptive
aria-label, and implement an onKeyDown handler on the same component that calls
the existing navigate({ to: "/settings/teams/$teamId", params: { teamId: team.id
} }) when Enter or Space is pressed (matching the existing onClick behavior used
in TeamsSettings TableRow). Ensure the handler prevents default for Space and
preserves visual focus styles.
In `@packages/db/drizzle/0049_add_teams.sql`:
- Around line 32-45: The trigger function
"auth.team_members_set_organization_id" currently only sets NEW.organization_id
when it's NULL, allowing callers to insert mismatched non-null org IDs; update
the function so it always queries the "auth.teams" table and assigns
NEW.organization_id = organization_id for the NEW.team_id regardless of the
incoming value, and adjust the trigger (or add a BEFORE INSERT OR UPDATE
trigger) "team_members_set_organization_id" to ensure the organization_id is
consistently derived from team_id on insert and on updates where team_id may
change.
In `@plans/20260510-teams-model.md`:
- Around line 17-77: The document must be updated to reflect the implemented
teams contract: add the new teams.slug field and the denormalized
team_members.organization_id field to the schema section, describe the new
beforeRemoveTeamMember lifecycle hook and exactly what it does (cleanup and
constraints), and document the delete-time rehoming behavior (how members/tasks
are rehomed when a team is deleted) plus any restrictions on leaving teams
introduced by these changes; update Membership semantics and Lifecycle hooks to
mention teams.slug, team_members.organization_id, beforeRemoveTeamMember, and
delete-time rehoming so the reference matches the implemented behavior (use
these exact symbols to locate the corresponding code/PR changes).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/teams/`$teamId/components/TeamDetailSettings/TeamDetailSettings.tsx:
- Around line 253-259: The expression new Set(members.map((m) => m.userId)) used
as the currentMemberUserIds prop recreates a Set on every render and can trigger
unnecessary re-renders of AddMemberButton; wrap that computation in React's
useMemo (import useMemo) to memoize the Set based on members (e.g., compute
currentMemberUserIds = useMemo(() => new Set(members.map(m => m.userId)),
[members])) near the top of the component after members is derived, and then
pass that memoized currentMemberUserIds to AddMemberButton instead of creating a
new Set inline.
🪄 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: df8f419b-6f3b-4e7e-bec0-4e952ef6dd06
📒 Files selected for processing (28)
apps/desktop/src/renderer/lib/auth-client.tsapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.tsapps/desktop/src/renderer/routes/_authenticated/settings/components/SettingsSidebar/GeneralSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/TeamDetailSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/components/AddMemberButton/AddMemberButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/components/AddMemberButton/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/TeamsSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/components/CreateTeamButton/CreateTeamButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/components/CreateTeamButton/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/index.tsapps/desktop/src/renderer/routes/_authenticated/settings/teams/page.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/renderer/stores/settings-state.tsapps/electric-proxy/src/where.tsapps/mobile/lib/auth/client.tspackages/auth/src/client.tspackages/auth/src/server.tspackages/db/drizzle/0049_add_teams.sqlpackages/db/drizzle/meta/0049_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schema/auth.tspackages/trpc/src/root.tspackages/trpc/src/router/team/index.tspackages/trpc/src/router/team/team.tsplans/20260510-teams-model.md
| const teams = createPersistedElectricCollection( | ||
| electricCollectionOptions<SelectTeam>({ | ||
| id: `teams-${organizationId}`, | ||
| shapeOptions: { | ||
| url: electricUrl, | ||
| params: { | ||
| table: "auth.teams", | ||
| organizationId, | ||
| }, | ||
| headers: electricHeaders, | ||
| columnMapper, | ||
| }, | ||
| getKey: (item) => item.id, | ||
| }), | ||
| ); | ||
|
|
||
| const teamMembers = createPersistedElectricCollection( | ||
| electricCollectionOptions<SelectTeamMember>({ | ||
| id: `team-members-${organizationId}`, | ||
| shapeOptions: { | ||
| url: electricUrl, | ||
| params: { | ||
| table: "auth.team_members", | ||
| organizationId, | ||
| }, | ||
| headers: electricHeaders, | ||
| columnMapper, | ||
| }, | ||
| getKey: (item) => item.id, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Fix collection id to match the established naming pattern.
The teamMembers collection id uses team-members-${organizationId} (with hyphen), but the pattern throughout this file is to preserve table name underscores in collection ids. For consistency with existing collections like task_statuses, v2_hosts, and automation_runs, the id should be team_members-${organizationId}.
♻️ Proposed fix
const teamMembers = createPersistedElectricCollection(
electricCollectionOptions<SelectTeamMember>({
- id: `team-members-${organizationId}`,
+ id: `team_members-${organizationId}`,
shapeOptions: {
url: electricUrl,
params: {📝 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 teams = createPersistedElectricCollection( | |
| electricCollectionOptions<SelectTeam>({ | |
| id: `teams-${organizationId}`, | |
| shapeOptions: { | |
| url: electricUrl, | |
| params: { | |
| table: "auth.teams", | |
| organizationId, | |
| }, | |
| headers: electricHeaders, | |
| columnMapper, | |
| }, | |
| getKey: (item) => item.id, | |
| }), | |
| ); | |
| const teamMembers = createPersistedElectricCollection( | |
| electricCollectionOptions<SelectTeamMember>({ | |
| id: `team-members-${organizationId}`, | |
| shapeOptions: { | |
| url: electricUrl, | |
| params: { | |
| table: "auth.team_members", | |
| organizationId, | |
| }, | |
| headers: electricHeaders, | |
| columnMapper, | |
| }, | |
| getKey: (item) => item.id, | |
| }), | |
| ); | |
| const teams = createPersistedElectricCollection( | |
| electricCollectionOptions<SelectTeam>({ | |
| id: `teams-${organizationId}`, | |
| shapeOptions: { | |
| url: electricUrl, | |
| params: { | |
| table: "auth.teams", | |
| organizationId, | |
| }, | |
| headers: electricHeaders, | |
| columnMapper, | |
| }, | |
| getKey: (item) => item.id, | |
| }), | |
| ); | |
| const teamMembers = createPersistedElectricCollection( | |
| electricCollectionOptions<SelectTeamMember>({ | |
| id: `team_members-${organizationId}`, | |
| shapeOptions: { | |
| url: electricUrl, | |
| params: { | |
| table: "auth.team_members", | |
| organizationId, | |
| }, | |
| headers: electricHeaders, | |
| columnMapper, | |
| }, | |
| getKey: (item) => item.id, | |
| }), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`
around lines 479 - 509, The teamMembers collection id uses a hyphenated name but
should preserve the table underscore to match existing naming conventions;
update the id passed into electricCollectionOptions for the
createPersistedElectricCollection call that constructs teamMembers (look for the
teamMembers variable and its electricCollectionOptions call) to use
`team_members-${organizationId}` instead of `team-members-${organizationId}` so
the collection id matches the table naming pattern.
| <button | ||
| type="button" | ||
| key={user.id} | ||
| disabled={isPending} | ||
| onClick={() => toggleMembership(user, isMember)} | ||
| className="flex items-center gap-2.5 w-full px-2 py-1.5 rounded-md text-left text-sm hover:bg-accent disabled:opacity-60 disabled:cursor-not-allowed" | ||
| > | ||
| <Checkbox checked={isMember} aria-hidden tabIndex={-1} /> | ||
| <Avatar |
There was a problem hiding this comment.
Expose membership state to assistive tech
At Line 134 the checkbox is hidden from accessibility APIs, and the clickable row (Line 127) doesn’t expose checked state. Screen readers can activate the control but can’t reliably tell whether the user is currently a team member.
💡 Suggested fix
<button
type="button"
key={user.id}
disabled={isPending}
onClick={() => toggleMembership(user, isMember)}
+ aria-pressed={isMember}
+ aria-label={`${isMember ? "Remove" : "Add"} ${user.name || user.email}`}
className="flex items-center gap-2.5 w-full px-2 py-1.5 rounded-md text-left text-sm hover:bg-accent disabled:opacity-60 disabled:cursor-not-allowed"
>| <TableRow | ||
| key={team.id} | ||
| className="cursor-pointer hover:bg-accent/50" | ||
| onClick={() => | ||
| navigate({ | ||
| to: "/settings/teams/$teamId", | ||
| params: { teamId: team.id }, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Row navigation is mouse-only
Line 86 uses click-only row navigation. Keyboard users can’t open team details from this table, which blocks this flow for non-pointer navigation.
💡 Suggested fix
+import { Link, useNavigate } from "@tanstack/react-router";
...
-<TableRow
- key={team.id}
- className="cursor-pointer hover:bg-accent/50"
- onClick={() =>
- navigate({
- to: "/settings/teams/$teamId",
- params: { teamId: team.id },
- })
- }
->
+<TableRow key={team.id} className="hover:bg-accent/50">
<TableCell className="font-medium">
- {team.name}
+ <Link
+ to="/settings/teams/$teamId"
+ params={{ teamId: team.id }}
+ className="block w-full cursor-pointer focus-visible:underline"
+ >
+ {team.name}
+ </Link>
</TableCell>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/TeamsSettings.tsx`
around lines 86 - 94, The table row navigation is mouse-only; make the TableRow
focusable and keyboard-activatable so keyboard users can open team details: add
tabIndex={0}, role="button" (or an appropriate ARIA role) and a descriptive
aria-label, and implement an onKeyDown handler on the same component that calls
the existing navigate({ to: "/settings/teams/$teamId", params: { teamId: team.id
} }) when Enter or Space is pressed (matching the existing onClick behavior used
in TeamsSettings TableRow). Ensure the handler prevents default for Space and
preserves visual focus styles.
| Better-auth's organization plugin owns the team primitive. We extend it via `additionalFields` per PR. | ||
|
|
||
| ### `auth.teams` | ||
|
|
||
| Better-auth defaults: `id`, `name`, `organizationId`, `createdAt`, `updatedAt`. Custom fields added per PR: | ||
|
|
||
| | Field | PR | Notes | | ||
| |---|---|---| | ||
| | `key` text | β | Identifier prefix, e.g. `SUPER`. Unique per org. | | ||
| | `lastTaskNumber` integer | β | Atomic counter for per-team task numbering. | | ||
| | `externalProvider` integration_provider | β | Linear linkage (added in same PR that adds outbound sync routing through team). | | ||
| | `externalId` text | β | Linear team UUID. | | ||
| | `externalKey` text | β | Linear team key (denormalized for display). | | ||
|
|
||
| No `isDefault` column — Linear doesn't model a default team either, and the only invariant we need is "at least one team exists," which better-auth's `allowRemovingAllTeams: false` already enforces. | ||
|
|
||
| ### `auth.team_members` | ||
|
|
||
| Better-auth defaults: `id`, `teamId`, `userId`, `createdAt`. No custom fields. | ||
|
|
||
| ## Invariants | ||
|
|
||
| 1. **Every organization has at least one team.** Enforced via `afterCreateOrganization` hook (creates a team for new orgs) + migration backfill (creates a team for existing orgs). Better-auth's `allowRemovingAllTeams: false` blocks deletion of the last team. | ||
| 2. **Tasks belong to exactly one team** (PR β). `tasks.team_id` NOT NULL, FK to `auth.teams`. | ||
| 3. **Team identifier prefix is unique within an org** (PR β). `unique(organizationId, key)` via additionalFields uniqueness. | ||
| 4. **`team_members` is opt-in.** Org members are auto-added to their org's first team for convenience, but can leave any team freely. Org membership is what grants visibility. | ||
| 5. **Removing an org member removes all their `team_members` rows in that org.** Enforced via `beforeRemoveMember` hook (no FK cascade because `team_members.userId` references `users`, not `members`). | ||
|
|
||
| ## Membership semantics | ||
|
|
||
| - **Auto-add on org join:** When a user joins an org (signup, invite acceptance, admin add), they're added to that org's first (and initially only) team. Via `afterAddMember` hook. | ||
| - **Manual leave/join:** Once multi-team UI ships (PR α), users can join and leave teams freely. No "at least one team" check. | ||
| - **Org leave cascade:** When a user is removed from an org, all their `team_members` rows in that org are deleted. Via `beforeRemoveMember` hook. | ||
|
|
||
| ## Permission model | ||
|
|
||
| Inherits better-auth's org-level RBAC. Team management permissions are tied to org role: | ||
|
|
||
| - `team:create` — owner + admin (members cannot create teams) | ||
| - `team:update` — owner + admin | ||
| - `team:delete` — owner + admin | ||
|
|
||
| **No per-team roles in PR α/β.** Every `team_members` row is just a membership marker; there is no `team-scoped admin` vs `team-scoped member` distinction. If we ever need delegated team leadership (e.g., "ENG lead can rename ENG and add members without being an org admin"), add a `role` column to `auth.team_members` via better-auth's `additionalFields` and define team-scoped roles via better-auth's permission extension. Strictly additive. | ||
|
|
||
| Visibility is org-scoped: | ||
|
|
||
| - Task lists show all tasks in the user's org regardless of team membership. | ||
| - `team_members` does not gate read access. | ||
| - Future team-scoped private teams (à la Notion private teamspaces) would be a deliberate opt-in feature, not the default. | ||
|
|
||
| ## Lifecycle hooks | ||
|
|
||
| | Hook | When | Action | | ||
| |---|---|---| | ||
| | `afterCreateOrganization` | New org created | Insert first team named after the org | | ||
| | `afterAddMember` | User added to org | Insert `team_members` row into the org's first team | | ||
| | `beforeRemoveMember` | User removed from org | Delete all `team_members` rows for that user in teams belonging to this org | | ||
|
|
||
| No default-team protection hooks. `allowRemovingAllTeams: false` is the only invariant we enforce — any team is deletable except the last one. | ||
|
|
||
| `setActiveTeam` is intentionally not used. The complexity of session-level "current team" state isn't worth it until we have a team-scoped UI surface that demands it. In PR β, task creation can default to the user's most-recently-used team via a `member.lastUsedTeamId` (or similar) rather than an active-team primitive. |
There was a problem hiding this comment.
Update this reference doc to match the implemented teams contract.
This still describes team_members as having no custom fields and members as able to leave all teams freely, but the PR now adds teams.slug, denormalized team_members.organization_id, beforeRemoveTeamMember, and delete-time rehoming. Leaving the reference model in the old state will send follow-up PRs toward the wrong invariants.
🧰 Tools
🪛 LanguageTool
[grammar] ~64-~64: Use a hyphen to join words.
Context: ...mbership. - team_members does not gate read access. - Future team-scoped privat...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@plans/20260510-teams-model.md` around lines 17 - 77, The document must be
updated to reflect the implemented teams contract: add the new teams.slug field
and the denormalized team_members.organization_id field to the schema section,
describe the new beforeRemoveTeamMember lifecycle hook and exactly what it does
(cleanup and constraints), and document the delete-time rehoming behavior (how
members/tasks are rehomed when a team is deleted) plus any restrictions on
leaving teams introduced by these changes; update Membership semantics and
Lifecycle hooks to mention teams.slug, team_members.organization_id,
beforeRemoveTeamMember, and delete-time rehoming so the reference matches the
implemented behavior (use these exact symbols to locate the corresponding
code/PR changes).
There was a problem hiding this comment.
8 issues found across 28 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/components/AddMemberButton/AddMemberButton.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/components/AddMemberButton/AddMemberButton.tsx:125">
P2: Pending state is tracked per single user id, which allows overlapping add/remove requests and can re-enable controls while another mutation is still in flight.</violation>
</file>
<file name="packages/db/drizzle/0049_add_teams.sql">
<violation number="1" location="packages/db/drizzle/0049_add_teams.sql:35">
P1: The trigger allows mismatched `organization_id` values when a non-null value is provided, which can corrupt team-to-organization scoping.</violation>
</file>
<file name="packages/db/src/schema/auth.ts">
<violation number="1" location="packages/db/src/schema/auth.ts:144">
P1: `team_members.organization_id` is not guaranteed to match `team_id`’s organization, so denormalized org filtering can become inconsistent. Enforce derivation unconditionally (and on `team_id` updates) or add a DB constraint tying team/org together.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx:49">
P2: Teams was added as a section but not added to `SECTION_ORDER`, so search auto-navigation can miss valid Teams-only matches.</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx:148">
P2: Parent-path Escape navigation loops on `/settings/project/$projectId/cloud/secrets` because the parent route auto-redirects back to secrets.</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/TeamDetailSettings.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/TeamDetailSettings.tsx:141">
P2: `handleGeneralSave` is missing a `catch` for rejected async calls, so thrown errors from `updateTeam` are not surfaced and can become unhandled rejections.
(Based on your team's feedback about handling async rejections explicitly.) [FEEDBACK_USED]</violation>
<violation number="2" location="apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/TeamDetailSettings.tsx:159">
P2: `handleDelete` is missing a `catch` for rejected async calls, so thrown errors from `removeTeam` are not handled and may cause unhandled rejection behavior.
(Based on your team's feedback about handling async rejections explicitly.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/components/CreateTeamButton/CreateTeamButton.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/settings/teams/components/TeamsSettings/components/CreateTeamButton/CreateTeamButton.tsx:58">
P2: Handle rejected `createTeam` calls with a `catch` so submit failures surface a toast instead of bubbling as an unhandled async error.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
| id: uuid("id").primaryKey().defaultRandom(), | ||
| name: text("name").notNull(), | ||
| slug: text("slug").notNull(), | ||
| organizationId: uuid("organization_id") |
There was a problem hiding this comment.
P1: team_members.organization_id is not guaranteed to match team_id’s organization, so denormalized org filtering can become inconsistent. Enforce derivation unconditionally (and on team_id updates) or add a DB constraint tying team/org together.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/schema/auth.ts, line 144:
<comment>`team_members.organization_id` is not guaranteed to match `team_id`’s organization, so denormalized org filtering can become inconsistent. Enforce derivation unconditionally (and on `team_id` updates) or add a DB constraint tying team/org together.</comment>
<file context>
@@ -135,6 +135,59 @@ export const members = authSchema.table(
+ id: uuid("id").primaryKey().defaultRandom(),
+ name: text("name").notNull(),
+ slug: text("slug").notNull(),
+ organizationId: uuid("organization_id")
+ .notNull()
+ .references(() => organizations.id, { onDelete: "cascade" }),
</file context>
| ) : ( | ||
| sortedUsers.map((user) => { | ||
| const isMember = currentMemberUserIds.has(user.id); | ||
| const isPending = pendingUserId === user.id; |
There was a problem hiding this comment.
P2: Pending state is tracked per single user id, which allows overlapping add/remove requests and can re-enable controls while another mutation is still in flight.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/teams/$teamId/components/TeamDetailSettings/components/AddMemberButton/AddMemberButton.tsx, line 125:
<comment>Pending state is tracked per single user id, which allows overlapping add/remove requests and can re-enable controls while another mutation is still in flight.</comment>
<file context>
@@ -0,0 +1,161 @@
+ ) : (
+ sortedUsers.map((user) => {
+ const isMember = currentMemberUserIds.has(user.id);
+ const isPending = pendingUserId === user.id;
+ return (
+ <button
</file context>
| const isPending = pendingUserId === user.id; | |
| const isPending = pendingUserId !== null; |
| if (segments.length <= 2) return; | ||
| event.preventDefault(); | ||
| navigate({ to: originRoute }); | ||
| const parent = `/${segments.slice(0, -1).join("/")}`; |
There was a problem hiding this comment.
P2: Parent-path Escape navigation loops on /settings/project/$projectId/cloud/secrets because the parent route auto-redirects back to secrets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx, line 148:
<comment>Parent-path Escape navigation loops on `/settings/project/$projectId/cloud/secrets` because the parent route auto-redirects back to secrets.</comment>
<file context>
@@ -137,11 +139,17 @@ function SettingsLayout() {
+ if (segments.length <= 2) return;
event.preventDefault();
- navigate({ to: originRoute });
+ const parent = `/${segments.slice(0, -1).join("/")}`;
+ navigate({ to: parent });
},
</file context>
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/db/drizzle/0049_add_teams.sql">
<violation number="1" location="packages/db/drizzle/0049_add_teams.sql:59">
P1: This backfill insert is no longer conflict-safe and can fail migration on duplicate org member rows. Restore `ON CONFLICT (team_id, user_id) DO NOTHING` to keep the migration robust.</violation>
</file>
<file name="packages/auth/src/server.ts">
<violation number="1" location="packages/auth/src/server.ts:512">
P1: Handle duplicate team-membership inserts in `afterAddMember` as a benign race. Without conflict/error handling, a unique-constraint hit can fail member-add flows even when the membership already exists.
(Based on your team's feedback about handling duplicate-key races in async inserts.) [FEEDBACK_USED]</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
| INSERT INTO "auth"."team_members" (team_id, user_id) | ||
| SELECT t.id, m.user_id | ||
| FROM "auth"."teams" t | ||
| JOIN "auth"."members" m ON m.organization_id = t.organization_id; |
There was a problem hiding this comment.
P1: This backfill insert is no longer conflict-safe and can fail migration on duplicate org member rows. Restore ON CONFLICT (team_id, user_id) DO NOTHING to keep the migration robust.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/drizzle/0049_add_teams.sql, line 59:
<comment>This backfill insert is no longer conflict-safe and can fail migration on duplicate org member rows. Restore `ON CONFLICT (team_id, user_id) DO NOTHING` to keep the migration robust.</comment>
<file context>
@@ -56,5 +56,4 @@ SELECT id, name, slug FROM "auth"."organizations";--> statement-breakpoint
FROM "auth"."teams" t
-JOIN "auth"."members" m ON m.organization_id = t.organization_id
-ON CONFLICT (team_id, user_id) DO NOTHING;
+JOIN "auth"."members" m ON m.organization_id = t.organization_id;
</file context>
| JOIN "auth"."members" m ON m.organization_id = t.organization_id; | |
| JOIN "auth"."members" m ON m.organization_id = t.organization_id | |
| ON CONFLICT (team_id, user_id) DO NOTHING; |
Tip: Review your code locally with the cubic CLI to iterate faster.
- afterAddMember: idempotent insert (onConflictDoNothing) so a stale row can't fail an addMember call - team_members trigger: always overwrite organization_id from team_id rather than only filling NULL — a caller can't persist a mismatched denormalized org and break Electric's shape filter - migration backfill comment: now accurately describes the auto-add semantics (had drifted across iterations) - settings layout SECTION_ORDER: include "teams" so search auto-selects the teams page when teams-only matches
createTeam/updateTeam/removeTeam all return { data, error } in normal
flow, but the underlying fetch can still throw on network failure or
abort. Match the try/catch pattern already used by handleLeaveTeam so
users see a toast instead of an unhandled rejection.
Summary
Adds teams as a first-class primitive under better-auth's organization plugin, with a Settings → Teams UI for CRUD and member management. Linear-style invariants — every org has ≥1 team, every member belongs to ≥1 team, and the system self-heals around the orphan edges.
auth.teamsandauth.team_memberstables; reads via Electric collections +useLiveQuery, writes via tRPC wrappers aroundctx.auth.api.*so org hooks fire server-side.NavigationControls) so users can back out of settings with the same affordance as elsewhere.Invariants (Linear-style)
allowRemovingAllTeams: falseblocks deleting the last team.afterAddMember). Existing members are backfilled into the default team so pre-teams users aren't dropped into an empty state.beforeRemoveTeamMemberrejects removal if it would leave the user with zero teams ("You should be a member of at least one team").beforeDeleteTeamre-homes would-be orphans into the next-oldest team before the FK cascade fires.beforeRemoveMemberclears allteam_membersrows for that user.Implementation notes
team_members.organization_idis denormalized via a BEFORE INSERT trigger so Electric can shape-filter with plain equality (no join throughteamsin the proxy).(team_id, user_id)prevents duplicate memberships; unique(organization_id, slug)enforces per-org slug uniqueness.team.addMember/team.removeMemberexist purely to enforce the active-org admin gate and route throughctx.auth.api.*so all org hooks fire. ThebeforeRemoveTeamMemberinvariant guards every caller (tRPC, direct authClient, future API surfaces).Test plan
team_membersrows disappearSummary by cubic
Adds teams as a first-class org primitive with a Settings → Teams UI for create, rename, slug edit, member management, leave, and delete. Enforces Linear-style invariants (≥1 team per org/member, self-healing deletes) and updates schema, hooks, clients, collections, proxy, settings search, plus better CRUD error handling with toasts.
New Features
better-auth(max 25,allowRemovingAllTeams: false, per‑org uniqueslug); hooks seed a default team on org create, auto‑add new members to the oldest team (idempotent insert), block removing a user’s last team, re‑home would‑be orphans on team delete, and clear memberships on org leave; settings top chrome addsNavigationControlswith safer Esc back‑nav; settings search includes Teams.auth.teamsandauth.team_memberswith indexes and a trigger that always overwritesteam_members.organization_idfrom its team; Electric collections for both; proxy adds where‑clause support forauth.teamsandauth.team_members.tRPCteam.addMember/team.removeMemberwith admin gate (self‑leave allowed); enable teams and requiredslugin desktop, mobile, and sharedauthClient; Settings → Teams list and per‑team detail (rename, edit slug, add/remove members, leave, delete); team create/update/delete now catch thrown rejections and show toasts.Migration
Written for commit 66a8a91. Summary will update on new commits.
Summary by CodeRabbit
New Features
Chores
UX