You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Phase D of the multi-tenancy plan — team management UI and single-use invite links on top of the Phase A–C tenancy core:
org.ts team functions (D1):createOrg, renameOrg, createInvite, revokeInvite, previewInvite, acceptInvite, switchActiveOrg, listMembers, listOpenInvites, setMemberRole, removeMember, leaveOrg — all role-gated server-side (route checks are defense-in-depth, not the gate).
POST /org/switch (D2): nav team switcher target; 303s to the dashboard.
/org team settings (D3+D4): rename, member roster with role/remove controls, invite link creation/revocation, create-another-team, leave team. Error states via .error-box form failures (I12); every control labeled and target-named (I13).
Nav switcher (D5): appears only with 2+ teams; switching changes the session's active_org_id and re-scopes every channel query (Phase C ownership).
/invite/[token] (D6): public landing; unknown tokens are plain 404s (no existence leak), burned/expired links render "no longer valid", signed-out visitors are asked to sign in and return.
Deviations from the plan's verbatim code (reconciliations)
No duplicated role ranking. The plan's D1 re-declared ROLE_RANK/requireRole (already in ownership.ts). Single fail-closed requireRole now lives in org.ts; requireOrgRole delegates. Copy-paste rule.
listMembers orders in SQL (createdAt, userId) instead of localeCompare — locale-sensitive sorting was already (validly) flagged in the Phase B review.
Personal-org invites rejected at write AND accept time.createInvite 400s on personal_for orgs; acceptInvite refuses any legacy/hand-written invite row pointing at a personal org (Phase C review finding, carried into this phase as planned).
listOpenInvites fails loudly on an unknown invite role (same guard as previewInvite) instead of silently filtering rows — "never silent fallbacks".
guard() is generic so the invite action reuses it instead of duplicating the error-unwrapping try/catch.
D7's org tests shipped with D1 (failing-first, per repo rule) rather than as a separate step.
Verification
npm run check / npm run build / npm run test all green — 355 tests (30 new), including route tests for /org/switch, /org load+actions, and /invite/[token].
Mutation check (plan D7): removed the acceptedBy burn from acceptInvite → the single-use test failed; reverted → green.
Codacy: 0 issues on every changed file before each commit.
Manual dev-DB run (seeded fixture user with two teams, since removed): renamed a team, created an invite link, switched teams from the nav — the dashboard channel list follows the active team and back. Screenshots: .playwright-cli/pr-org-team.png, .playwright-cli/pr-dashboard-acme.png, .playwright-cli/pr-dashboard-side.png (gitignored scratch dir — ask and I'll attach them in a comment).
The second-profile invite-accept flow needs a real second Google account — covered instead by route tests (join → 303, membership written with the invite's role, token burned, reuse → 410, accepted flag on reload).
Note: PR #51 (chore-drizzle-skill) is unrelated work from a concurrent session and is not part of this phase.
CodeAnt-AI Description
Add team management, switching, and single-use invitations
What Changed
Users can create and rename shared teams, view members, change roles, remove teammates, and leave teams from a dedicated Team page
Admins can create and revoke member or admin invitation links that expire after seven days; each link can be used only once
Signed-out invite visitors are prompted to sign in, while unknown, expired, or used links show safe error states
Users with multiple teams can switch the active team from the navigation, and team-scoped data follows the selected team
Owner protections prevent teams from being left without an owner, and leaving a team redirects to the dashboard
Added coverage for team actions, invitations, role permissions, session switching, and failure cases
Impact
✅ Self-service team administration ✅ Single-use, expiring team invites ✅ Safer team switching and ownership changes
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
@codeant-ai ask: Your question here
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
@codeant-ai ask: Can you suggest a safer alternative to storing this secret?
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
@codeant-ai: Your feedback here
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
@codeant-ai: Do not flag unused imports.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
@codeant-ai: review
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
This PR adds team administration with single-use invite links and session-scoped team switching. The diagram shows invite creation, invite acceptance, and how both acceptance and switching update the active team.
sequenceDiagram
participant Owner
participant Team Settings
participant Invite Page
participant Org Backend
participant Session
Owner->>Team Settings: Create invite link
Team Settings->>Org Backend: Create single-use invite
Org Backend-->>Team Settings: Return invite token
Invite Page->>Org Backend: Preview invite
Org Backend-->>Invite Page: Return team and role
Invite Page->>Org Backend: Accept invite
Org Backend->>Session: Add member and set active team
User->>Org Backend: Switch active team
Org Backend->>Session: Update active team
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
This PR implements team management UI and invite functionality (Phase D). The implementation is comprehensive with good test coverage (355 tests total, 30 new). However, I've identified a critical security vulnerability that must be fixed before merge.
Critical Issue
Invite Token Reuse Vulnerability (src/lib/server/org.ts:258-261): The acceptInvite function fails to burn invite tokens when the accepting user is already a member. This violates the single-use design and creates a security risk where existing members can reuse the same invite multiple times, and worse, other users can still use the unburned token after an existing member accepts it.
Fix Required
The token burn (line 260) must happen unconditionally for all acceptInvite calls, not just for new memberships. The idempotent behavior (allowing existing members to switch teams via the invite) should be preserved, but every acceptance must mark the token as used to prevent reuse.
Test Coverage Note
The test at line 233 (acceptInvite by an existing member is idempotent and does not burn the token) appears to be testing for the buggy behavior rather than the correct behavior. After fixing the code, this test should be updated to verify the token IS burned even for existing members.
All other aspects of the implementation look solid - proper role-based authorization, defense in depth, and comprehensive error handling.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
The reason will be displayed to describe this comment to others. Learn more.
🛑 Security Vulnerability: The invite token is not burned when the user is already a member. This creates a token reuse vulnerability where existing members can repeatedly accept the same invite (line 258-261), and after they're done, a different user can still use the unburned token to join the org. Every acceptInvite call must burn the token regardless of existing membership to maintain single-use semantics and prevent unauthorized reuse.1
Suggested change
if(!existing){
awaittx.insert(memberships).values({ userId,orgId: inv.orgId, role });
Phase D: Team management UI, org switching, and single-use invites
✨ Enhancement🧪 Tests🕐 40+ Minutes
AI Description
• Add server-side org/team management primitives with strict role-gated authorization.
• Introduce Team settings UI, nav team switcher, and invite landing/accept flow.
• Expand test coverage for org actions, switching, and invite single-use/expiry behavior.
Diagram
graph TD
A["App nav (+layout)"] --> B["/org settings page"] --> D[["org.ts (team service)"] --> E[("DB: orgs/memberships/invites/sessions")]
A --> C["POST /org/switch"] --> D
F["/invite/[token] landing"] --> D
subgraph Legend
direction LR
_ui["UI / Routes"] ~~~ _svc[["Service module"]] ~~~ _db[("Database")]
end
Loading
High-Level Assessment
The following are alternative approaches to this PR:
1. JWT-signed invite tokens (stateless invites)
➕ No invites table reads for preview; can validate locally
➕ Simplifies revocation storage if short TTL only
➖ Single-use enforcement still needs server-side storage/nonce tracking
➖ Revocation and burn semantics become more complex to implement correctly
➖ Harder to avoid existence leaks while keeping good UX
2. Allow multi-use invites with max-uses counter
➕ Better UX for onboarding multiple teammates
➕ Fewer admin actions needed to invite many users
➖ Higher risk if link is forwarded; requires more safeguards/auditing
➖ More complex concurrency/atomic decrement handling
Recommendation: The current DB-backed, single-use invite design is the best fit for Phase D: it cleanly supports preview, explicit revocation, atomic burn-on-accept, and avoids token existence leaks (404 vs 410) while keeping authorization enforced in org.ts. Stateless/JWT invites would not remove the need for storage given the single-use requirement, and multi-use invites expand risk/complexity beyond the stated phase.
Files changed (12) +1361 / -14
Enhancement (7) +666 / -2
org.tsImplement team management + single-use invite lifecycle in org service+271/-2
Implement team management + single-use invite lifecycle in org service
• Adds role ranking and a fail-closed requireRole helper, plus core Phase D functions: create/rename org, create/revoke/preview/accept invites, switchActiveOrg, listMembers, listOpenInvites, setMemberRole, removeMember, and leaveOrg. Enforces server-side authorization, prevents invites into personal orgs, and burns invite tokens atomically while updating sessions.activeOrgId.
+layout.svelteAdd Team nav entry and conditional team switcher+17/-0
Add Team nav entry and conditional team switcher
• Adds a /org navigation link and a POST-backed team switch form that only renders when the user has 2+ orgs. Includes minimal styling for the switcher layout.
+page.server.tsServer load/actions for Team settings page with error-to-form guarding+105/-0
Server load/actions for Team settings page with error-to-form guarding
• Implements /org load to fetch member roster and (admin+) open invites, and exposes actions for rename, createTeam, invite, revokeInvite, setRole, remove, and leave. Wraps org.ts errors into SvelteKit form failures so UI can render consistent .error-box messaging.
Team settings UI: rename, members/roles, invites, create team, leave
• Adds the Team page markup with accessible labels and role-gated controls: rename (admin+), role changes (owner), remove member (admin/owner rules), invite link creation/revocation (admin+), create another team, and leave team. Displays form errors via .error-box and surfaces single-use invite URLs on success.
+server.tsAdd POST /org/switch endpoint for nav team switching+36/-0
Add POST /org/switch endpoint for nav team switching
• Introduces a POST handler that requires a signed-in user and session cookie, validates orgId, updates sessions.activeOrgId via switchActiveOrg, and 303-redirects to /dashboard to avoid POST replay.
• Implements invite preview via previewInvite, returning a plain 404 for unknown tokens to avoid existence leaks. Adds a default action that requires sign-in and session cookie, accepts the invite (burning token + updating session org), and 303-redirects to /dashboard.
+page.svelteInvite landing UI with expired/accepted handling and sign-in guidance+54/-0
Invite landing UI with expired/accepted handling and sign-in guidance
• Renders three states: invalid (expired/accepted), signed-out (prompt to sign in and reopen link), and signed-in (join button posting to accept). Shows org name and invited role clearly.
org.test.tsAdd comprehensive Phase D tests for org/team management+335/-5
Add comprehensive Phase D tests for org/team management
• Extends the existing org test suite with fixtures and behavior tests covering org creation/rename, invite creation/preview/accept/burn, member listing, role changes, removals, leaving teams, and switching active org. Verifies non-leak semantics (404 vs 410), personal-org invite rejection, and ordering guarantees.
page.server.test.tsRoute-level tests for /org load and actions+170/-0
Route-level tests for /org load and actions
• Adds tests for signed-out behavior, role-based visibility (invites only for admin+), and action outcomes including wrapped failures vs raw 403s. Verifies DB effects for rename, create team, invite creation, and member removal/leave error cases.
switch.test.tsTests for /org/switch authorization, validation, and redirect behavior+81/-0
Tests for /org/switch authorization, validation, and redirect behavior
• Covers 401 cases (signed-out/missing cookie), 400 for missing orgId, 404 for switching to a non-member org, and the successful path that updates sessions.activeOrgId and returns a 303 to /dashboard.
page.server.test.tsTests for invite landing preview and accept flow+107/-0
Tests for invite landing preview and accept flow
• Verifies unknown tokens are 404, known tokens return preview with signedIn flag, and accept requires auth + session cookie. Confirms accept joins membership, burns the token, redirects, and rejects reuse with 410 while load flags acceptance.
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Phase D implementation covers the core requirements for team management and invites but introduces critical concurrency risks in 'src/lib/server/org.ts'. Several acceptance criteria—specifically single-use invite enforcement and 'last owner' safeguards—are implemented with non-atomic checks, which could lead to inconsistent state or unauthorized access under load.
Codacy analysis indicates the PR is not up to standards due to a significant increase in complexity (+66 in 'org.ts') and 10 new code clones. A notable UX gap exists where the 'leave' action results in a 404 error rather than redirecting the user to a valid state. These issues should be addressed before merging to ensure system stability and a smooth user experience.
About this PR
The PR introduces significant technical debt with a high complexity delta and 10 new code clones across server logic and test files. Consider refactoring repetitive test setups and centralizing database query logic in 'src/lib/server/org.ts' to improve long-term maintainability.
Test suggestions
Creating a shared team correctly assigns the creator as 'owner' and blocks empty/long names.
Renaming a team is restricted to admin+ and validates the name input.
Invite links are single-use; the token is burned after a user joins and cannot be reused by another.
Invite acceptance is idempotent for existing members and does not consume the single-use token.
Attempting to create or accept an invite for a personal organization is rejected with a 400 error.
Role hierarchy: Admins can remove members, but only Owners can remove Admins or change roles.
Last-owner demotion or leaving a team with other members is blocked until a new owner is promoted.
Switching active organizations via /org/switch updates the session and redirects to the dashboard.
Public invite preview returns null (404) for unknown tokens to avoid leaking organization existence.
The reason will be displayed to describe this comment to others. Learn more.
🔴 HIGH RISK
The 'already accepted' check is not atomic. Move the check for acceptedBy inside the transaction, or use a conditional update (WHERE token = ? AND accepted_by IS NULL) and verify that exactly one row was affected to ensure the invite remains single-use.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Move the validation logic (checking for other members/owners) inside the transaction to prevent race conditions that could lead to an organization having no members or owners.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
This operation is prone to a race condition. Concurrent demotions could leave an organization without any owners. Perform the owner count check and the role update within a single transaction.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: Add a redirect (e.g., to /dashboard) after successfully leaving the organization to ensure the user is transitioned to a valid state and avoid a 404 error on the current page.
The reason will be displayed to describe this comment to others. Learn more.
⚪ LOW RISK
Suggestion: The database lookup logic for invites is duplicated between 'previewInvite' (lines 207-211) and 'acceptInvite'. Consider extracting a private helper function called 'getInviteWithOrg' that handles the join between 'invites' and 'organizations' based on the token.
Concurrent acceptance requests can both redeem a single-use invite
Invite consumption is not atomic: concurrent requests can both pass the pre-transaction acceptedBy check, insert memberships for different users, and update the same invite. The update must condition on acceptedBy IS NULL and the membership insertion must only succeed when that conditional claim succeeds, otherwise the documented single-use guarantee is violated.
❌ A single invite can grant membership to multiple users.
⚠️ Invite redemption violates the documented single-use guarantee.
⚠️ Concurrent acceptance can create unexpected team members.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/server/org.ts
**Line:** 259:260
**Comment:***Race Condition: Invite consumption is not atomic: concurrent requests can both pass the pre-transaction `acceptedBy` check, insert memberships for different users, and update the same invite. The update must condition on `acceptedBy IS NULL` and the membership insertion must only succeed when that conditional claim succeeds, otherwise the documented single-use guarantee is violated.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
2026-08-03 12:09
✅ Concurrent owner demotions can leave a team without any ownerSuggestion Impact:The role change was converted from a separate owner-count check and update into a conditional UPDATE whose SQL predicate requires more than one owner when demoting an owner. If the condition fails, no row is updated and a last-owner error is returned.
code diff:
@@ -322,22 +342,24 @@
if (role !== 'owner' && role !== 'admin' && role !== 'member') throw error(400, 'unknown role');
const target = await membershipOf(targetUserId, orgId);
if (!target) throw error(404, 'member not found');
- // Owner-count check and role update in ONE transaction: two concurrent- // demotions can never strand the org without an owner (PR #52 review).- await db.transaction(async (tx) => {- if (target.role === 'owner' && role !== 'owner') {- const owners = await tx- .select({ userId: memberships.userId })- .from(memberships)- .where(and(eq(memberships.orgId, orgId), eq(memberships.role, 'owner')))- .all();- if (owners.length <= 1) throw error(400, 'the last owner cannot be demoted — promote a teammate to owner first');- }- await tx- .update(memberships)- .set({ role })- .where(and(eq(memberships.userId, targetUserId), eq(memberships.orgId, orgId)));- });+ // Demoting an owner is a CONDITIONAL update: the write only lands while+ // another owner still exists, so concurrent demotions can never strand the+ // org ownerless — no separate check to race with (PR #52 review).+ const demotingOwner = target.role === 'owner' && role !== 'owner';+ const updated = await db+ .update(memberships)+ .set({ role })+ .where(+ and(+ eq(memberships.userId, targetUserId),+ eq(memberships.orgId, orgId),+ demotingOwner+ ? sql`(SELECT count(*) FROM memberships AS mo WHERE mo.org_id = ${orgId} AND mo.role = 'owner') > 1`+ : undefined+ )+ )+ .returning({ userId: memberships.userId });+ if (updated.length === 0) throw error(400, 'the last owner cannot be demoted — promote a teammate to owner first');
The last-owner check and role update are separate operations. Two owners can concurrently demote one another, with both seeing two owners before either update commits, leaving the organization with no owner. Perform the check and update atomically, or make the update conditional on another owner still existing.
⚠️ Team administration becomes operationally unrecoverable.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/server/org.ts
**Line:** 321:332
**Comment:***Race Condition: The last-owner check and role update are separate operations. Two owners can concurrently demote one another, with both seeing two owners before either update commits, leaving the organization with no owner. Perform the check and update atomically, or make the update conditional on another owner still existing.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
2026-08-03 12:09
Incomplete implementation
Existing-member acceptance is tested without enforcing that the invite is burned for all later users
The test explicitly accepts that an invite remains usable after an existing member accepts it, but invite links are documented as single-use. This assertion leaves the implementation free to let a different user redeem the same token after the first acceptance; the test should verify that any successful acceptance, including an idempotent existing-member acceptance, burns the invite and rejects subsequent users with 410.
❌ Multiple users can join through one intended single-use link.
⚠️ Invite consumption behavior differs from documented schema semantics.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/server/org.test.ts
**Line:** 233:246
**Comment:***Incomplete Implementation: The test explicitly accepts that an invite remains usable after an existing member accepts it, but invite links are documented as single-use. This assertion leaves the implementation free to let a different user redeem the same token after the first acceptance; the test should verify that any successful acceptance, including an idempotent existing-member acceptance, burns the invite and rejects subsequent users with 410.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
2026-08-03 12:09
Latest suggestions up to commit 06bddf0
Category
Suggestion
Severity
Generated at (UTC)
Security
Invite acceptance can burn an invite without updating the accepting user's authenticated session
Invite acceptance commits the membership and burns the token even when sessionToken is stale, deleted, or belongs to another user, because the session update is neither checked for ownership nor verified to affect a row. A logout or session deletion racing with this request can therefore consume a valid invite without switching the accepting user's session; verify the session row belongs to userId and require the update to affect exactly one live session within the transaction.
❌ Valid invite can be consumed during session invalidation.
⚠️ Dashboard may resolve a different active organization.
⚠️ User must obtain another invite link.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/lib/server/org.ts
**Line:** 266:266
**Comment:***Security: Invite acceptance commits the membership and burns the token even when `sessionToken` is stale, deleted, or belongs to another user, because the session update is neither checked for ownership nor verified to affect a row. A logout or session deletion racing with this request can therefore consume a valid invite without switching the accepting user's session; verify the session row belongs to `userId` and require the update to affect exactly one live session within the transaction.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
2026-08-03 12:25
Api mismatch
Signing in from an invite loses the invite URL and prevents automatic return to acceptance
The sign-in link drops the current invite URL. The login flow always redirects to /dashboard, so after completing OAuth the visitor cannot automatically return to this invite and must recover the original token manually, preventing the advertised sign-in-and-return flow. Preserve the invite URL through login and the OAuth callback.
⚠️ Signed-out invitees lose their pending invite destination.
⚠️ Invite acceptance requires manually reopening the original link.
⚠️ OAuth consent users also lack invite-context restoration.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/invite/[token]/+page.svelte
**Line:** 42:42
**Comment:***Api Mismatch: The sign-in link drops the current invite URL. The login flow always redirects to `/dashboard`, so after completing OAuth the visitor cannot automatically return to this invite and must recover the original token manually, preventing the advertised sign-in-and-return flow. Preserve the invite URL through login and the OAuth callback.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
acceptInvite() checks invite validity before the transaction and then burns the invite with an
unconditional UPDATE, so two concurrent accepts can both insert memberships before acceptedBy is
set, allowing multiple users to join from a supposedly single-use link.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The invite validity (acceptedBy/expiry) is checked before the transaction, and the burn inside the
transaction updates only by token. Because memberships are keyed by (userId, orgId), two different
users can both insert memberships for the same org and then race to update the same invite row,
violating single-use semantics.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`acceptInvite()` is intended to be single-use, but it is not concurrency-safe: multiple concurrent accept requests can both observe an unaccepted invite and both insert memberships.
## Issue Context
The invite is validated (acceptedBy/expiry) outside the transaction, and inside the transaction the invite is burned via `UPDATE invites SET acceptedBy=... WHERE token=...` without requiring `acceptedBy IS NULL` (or checking affected rows).
## Fix Focus Areas
- src/lib/server/org.ts[232-265]
## Fix outline
- Move the invite-claim/burn into the transaction and make it conditional:
- `UPDATE invites SET acceptedBy = :userId WHERE token = :token AND acceptedBy IS NULL AND expiresAt > :now RETURNING orgId, role, ...`
- If 0 rows are updated/returned, throw `error(410, ...)`.
- Only insert the membership after the invite has been successfully claimed.
- Keep the “already a member” idempotency, but ensure it cannot bypass the single-use guarantee for new members (e.g., still require the conditional claim for first-join, or structure logic as: check existing membership first, else claim invite + insert).
- Add a concurrency test if feasible (two parallel accepts) to lock the behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
2. Ownerless org race✓ Resolved🐞 Bug☼ Reliability
Description
setMemberRole() and leaveOrg() enforce the “last owner” invariant using reads outside the write
transaction, so concurrent owner demotions/leaves can interleave and leave an organization with zero
owners.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The code checks owner counts/other-owner existence via standalone reads and then performs
unconditional writes. Since memberships has only a composite PK and no constraint requiring an
owner, concurrent operations can result in an org with no owner memberships, breaking the admin
model.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The “there must always be at least one owner” invariant is checked with separate SELECTs and then enforced via unconditional UPDATE/DELETE, which is not safe under concurrent requests.
## Issue Context
- `setMemberRole()` counts owners, then performs an unconditional role update.
- `leaveOrg()` checks for other owners before its transaction, then deletes the membership in the transaction.
- There is no DB constraint guaranteeing an org must have an owner.
## Fix Focus Areas
- src/lib/server/org.ts[314-333]
- src/lib/server/org.ts[357-377]
## Fix outline
- Enforce the invariant atomically, e.g.:
- Wrap the owner-count check and the role update in a single transaction.
- Use conditional updates/deletes that only proceed if another owner exists at the time of write.
- Consider serializing owner-changing operations per org (SQLite write locking helps, but you still need to structure reads/writes so both requests can’t validate against the same pre-state).
- Ensure the demotion/update checks that the target is still an owner when demoting.
- Add a test that simulates concurrent demotion/leave (as best as the test harness allows) or at minimum unit-test the conditional WHERE behavior (0-row update => 400).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
switchActiveOrg()/acceptInvite()/leaveOrg() update sessions using only the session id and don’t
assert an affected row, so a mismatched valid session token could mutate another user’s session, and
an unknown token can silently fail while the org-side mutation succeeds (notably in acceptInvite).
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The sessions table has an owning userId column, but these functions update sessions using only the
session id; acceptInvite additionally doesn’t check whether the session update matched any row, so
it can complete the invite/membership mutation without actually updating the caller’s session state.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
Session mutations are not scoped to the owning user (`sessions.user_id`) and update results aren’t checked. This weakens invariants and can produce silent partial success.
## Issue Context
- `switchActiveOrg()` updates `sessions` by `id` only.
- `acceptInvite()` can burn the invite / insert membership even if the session update matched 0 rows.
- `leaveOrg()` clears `active_org_id` by session id only (plus activeOrgId match), not by user.
## Fix Focus Areas
- src/lib/server/org.ts[232-272]
- src/lib/server/org.ts[357-377]
## Fix outline
- Add `eq(sessions.userId, userId)` (or the caller user id) to all session updates:
- `where(and(eq(sessions.id, sessionToken), eq(sessions.userId, userId)))`
- For flows where updating the session is part of the contract (especially `acceptInvite` and `switchActiveOrg`), check the affected row count (or use `.returning(...)`) and throw `error(401, ...)` (or `500`) if no session row was updated.
- Consider doing the session-row existence/ownership check early in the function (before burning an invite) to avoid partial success.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
VALID → fixed (each behavioral change went failing-test-first):
Invite token not burned for existing members (Amazon Q, org.ts) — every acceptInvite now burns unconditionally inside the transaction; an already-a-member accept keeps their role and switches the session but leaves the link dead for everyone else. Test updated: asserts accepted_by set, no duplicate membership, subsequent accept by another user → 410. (Plan D7 explicitly allowed either behavior; unconditional burn is strictly safer.)
Single-use check not atomic (Codacy HIGH) — invite read, 410/400 checks, membership insert, burn, and session switch now happen in ONE transaction; two concurrent accepts can no longer both join.
Last-owner demotion race (Codacy) — owner-count check + role update wrapped in one transaction in setMemberRole.
leaveOrg validation race (Codacy) — others/last-owner checks moved inside the transaction with the delete + session clear.
Post-leave 404 (Codacy, +page.server.ts) — after a successful leave the action now 303s to /dashboard; the old request's frozen locals would otherwise re-list members of the org just left and 404. New route test (red before fix) asserts the 303, membership removal, and active_org_id cleared.
Duplicated invite+org join (Codacy LOW) — extracted shared inviteWithOrg(handle, token) used by previewInvite and acceptInvite (handle parameter supports the transaction).
Unused sessions import in page.server.test.ts (Codacy) — superseded: the new leave test seeds a session row, so the import is now used.
Local seedUser in org.test.ts duplicating testdb's (Codacy clones) — deleted; tests import the shared helper.
NOT VALID / no change:
Codacy complexity +66 / test-file clone metric — test-file duplication metric is known noise here; the one genuine shared-fixture case (seedUser) was extracted (item 8).
Qodo alternatives (JWT invites, multi-use invites) — Qodo itself recommends the current DB-backed single-use design; no change.
SonarQube "1 new issue" — quality gate passed, 0 security hotspots, 0% duplication; details are behind the SonarCloud login.
Gates: npm run check / npm run build / npm run test all green — 356 tests.
codeant-aiBot
added
size:XXL
This PR changes 1000+ lines, ignoring generated files
and removed
size:XXL
This PR changes 1000+ lines, ignoring generated files
labels
Aug 3, 2026
Review triage — round 2 (Qodo findings, fixed in ce670c7)
1. Invite burn race — VALID, hardened further. Round 1 had already moved read+check+burn into one transaction; this round makes the burn itself a conditional claim: UPDATE invites SET accepted_by=? WHERE token=? AND accepted_by IS NULL ... RETURNING — zero rows → 410. Even read-skewed concurrent accepts cannot both join. Claim precedes the membership insert; any later failure rolls the claim back.
switchActiveOrg and acceptInvite session updates are now scoped WHERE id=? AND user_id=? AND verified via RETURNING — a mismatched/unknown token throws 401 instead of silently updating nothing.
New tests: mismatched session on switch → 401 and the other user's session untouched; mismatched session on accept → 401 with NO burn and NO membership.
leaveOrg's session clear is now also caller-scoped; new test asserts a teammate's session in the same org is untouched.
3. Ownerless org race — VALID → fixed at statement level. Round 1 wrapped check+write in a transaction; this round removes the separate check entirely:
setMemberRole owner demotion is a conditional UPDATE ... WHERE (SELECT count(*) owner) > 1 RETURNING — zero rows → 400. No pre-state to race with.
leaveOrg's delete is conditional the same way (role != 'owner' OR owner count > 1) — zero rows → 400.
Gates: npm run check / npm run build / npm run test all green — 359 tests.
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 `@src/lib/server/org.test.ts`:
- Around line 253-274: Add a `setMemberRole` test case for an invalid role
value, using the existing raw-caller cast pattern and importing the role type
alongside the current functions. Assert that calling `setMemberRole` with the
unknown role rejects with status 400, covering the guard before any membership
update occurs.
In `@src/lib/server/org.ts`:
- Line 266: Scope every session update by both sessionToken and userId. In
src/lib/server/org.ts lines 266-266, update acceptInvite’s sessions predicate;
in lines 272-276, update switchActiveOrg’s predicate; and in lines 383-386,
extend leaveOrg’s existing and(...) with the userId condition so mismatched
tokens update no rows.
- Around line 318-341: Role-based checks use membership data read outside the
write transaction, allowing concurrent role changes to bypass invariants. In
src/lib/server/org.ts lines 318-341, update setMemberRole to move the target
lookup into db.transaction and use that fresh role for the owner-count check; in
lines 348-358, open a transaction in the member-removal function, re-read the
target membership inside it, perform owner/admin checks there, then delete; in
lines 365-388, re-read the leaver membership inside the transaction and use its
role for the last-owner check instead of outer m.role.
- Around line 135-140: Update requireSharedOrg to explicitly reject an undefined
organization row before checking personalFor, returning the existing 404 error
for missing organizations; preserve the current 400 error for personal
organizations.
In `@src/routes/`(app)/org/+page.server.ts:
- Around line 94-98: Update the remove action in
src/routes/(app)/org/+page.server.ts lines 94-98 to enforce the same route-level
organization-role authorization as rename, invite, revokeInvite, and setRole
before calling removeMember; add coverage in
src/routes/(app)/org/page.server.test.ts lines 146-160 asserting that a
member-role caller removing another member is rejected.
- Around line 86-93: Validate the form role in the setRole action before calling
setMemberRole, accepting only 'owner', 'admin', or 'member' and returning the
same 400 failure behavior used by invite for invalid values; remove reliance on
the unchecked OrgRole cast. In src/routes/(app)/org/page.server.test.ts lines
136-144, add coverage asserting that setRole with an out-of-range role returns a
400 failure.
In `@src/routes/`(app)/org/+page.svelte:
- Around line 109-121: Replace the empty branch of the data.invites each block
with the shared EmptyState component, preserving the “No open invite links.”
message and following the component’s existing usage conventions. Leave the
populated invite rendering unchanged.
- Around line 115-118: The revoke invite button in the form using the
revokeInvite action must identify its specific target, incorporating the
invite’s available identifying value such as its email or token. Preserve the
existing revoke behavior while making each button’s accessible name
distinguishable when multiple invites are listed.
In `@src/routes/invite/`[token]/+page.svelte:
- Around line 33-54: The invite route’s conditional rendering lacks the required
loading and EmptyState representations. Update the page component’s state
handling around the existing invite branches to render a loading skeleton while
data is pending, use EmptyState when the invite is unavailable, and retain
`.error-box` for expired or accepted invites while preserving the current
populated signed-in and signed-out states.
In `@src/routes/invite/`[token]/page.server.test.ts:
- Around line 96-101: Extend the test following the accept action assertion to
query the session identified by sess-1 and assert its activeOrgId is org-1. Keep
the existing membership and invite assertions unchanged, ensuring the test
verifies the session update performed by acceptInvite.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b9c0c1c2-08d2-403a-bce0-7a43f885bcc5
📥 Commits
Reviewing files that changed from the base of the PR and between cd3283a and 06bddf0.
The guard passes when org is undefined. A missing organization then reaches the invite insert and fails as a raw database error instead of a 404. Reject the missing row explicitly.
♻️ Proposed change
async function requireSharedOrg(orgId: string): Promise<void> {
const org = await db.select({ personalFor: organizations.personalFor }).from(organizations).where(eq(organizations.id, orgId)).get();
- if (org && org.personalFor !== null) {+ if (!org) throw error(404, 'team not found');+ if (org.personalFor !== null) {
throw error(400, "personal teams can't have members — create a shared team to collaborate");
}
}
📝 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.
throwerror(400,"personal teams can't have members — create a shared team to collaborate");
}
}
🤖 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 `@src/lib/server/org.ts` around lines 135 - 140, Update requireSharedOrg to
explicitly reject an undefined organization row before checking personalFor,
returning the existing 404 error for missing organizations; preserve the current
400 error for personal organizations.
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Roles that gate authorization checks are read outside the write transaction. All three functions load a membership role with membershipOf before the write, then use that stale role to decide whether an invariant or authorization check applies. A concurrent role change between the read and the write bypasses the check. Re-read the membership row inside the transaction, and put the write in a transaction where none exists.
src/lib/server/org.ts#L318-L341: move the target lookup inside the db.transaction callback and gate the owner-count check on the freshly read role.
src/lib/server/org.ts#L348-L358: open a transaction, re-read the target membership inside it, apply the owner and admin checks there, then delete.
src/lib/server/org.ts#L365-L388: re-read the leaver's membership inside the transaction and use that role for the last-owner check instead of the outer m.role.
📍 Affects 1 file
src/lib/server/org.ts#L318-L341 (this comment)
src/lib/server/org.ts#L348-L358
src/lib/server/org.ts#L365-L388
🤖 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 `@src/lib/server/org.ts` around lines 318 - 341, Role-based checks use
membership data read outside the write transaction, allowing concurrent role
changes to bypass invariants. In src/lib/server/org.ts lines 318-341, update
setMemberRole to move the target lookup into db.transaction and use that fresh
role for the owner-count check; in lines 348-358, open a transaction in the
member-removal function, re-read the target membership inside it, perform
owner/admin checks there, then delete; in lines 365-388, re-read the leaver
membership inside the transaction and use its role for the last-owner check
instead of outer m.role.
Missing runtime validation for role in setRole, and no test catches it. The setRole action casts the form value directly to OrgRole instead of validating it, unlike the sibling invite action; the test suite has no case that would expose this gap.
src/routes/(app)/org/+page.server.ts#L86-L93: validate role is one of 'owner' | 'admin' | 'member' before calling setMemberRole, mirroring the check in the invite action (Line 77).
src/routes/(app)/org/page.server.test.ts#L136-L144: add a test asserting that actions.setRole with an out-of-range role value returns a 400 failure.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/`(app)/org/+page.server.ts around lines 86 - 93, Validate the form
role in the setRole action before calling setMemberRole, accepting only 'owner',
'admin', or 'member' and returning the same 400 failure behavior used by invite
for invalid values; remove reliance on the unchecked OrgRole cast. In
src/routes/(app)/org/page.server.test.ts lines 136-144, add coverage asserting
that setRole with an out-of-range role returns a 400 failure.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the required loading and empty states.
Provide a loading skeleton for this route. Use EmptyState for the unavailable invite state. Keep .error-box for actionable errors.
As per coding guidelines, “Every page must provide loading skeleton, empty state using EmptyState, .error-box error state, and populated state.”
🤖 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 `@src/routes/invite/`[token]/+page.svelte around lines 33 - 54, The invite
route’s conditional rendering lacks the required loading and EmptyState
representations. Update the page component’s state handling around the existing
invite branches to render a loading skeleton while data is pending, use
EmptyState when the invite is unavailable, and retain `.error-box` for expired
or accepted invites while preserving the current populated signed-in and
signed-out states.
Review triage — round 3 (CodeRabbit, fixed in aa37376)
VALID → fixed:
remove lacked the route-level role gate — added requireOrgRole(user, 'admin'), matching the sibling actions; a member caller now gets the same raw 403 as rename/invite/revokeInvite/setRole. New route test (red before fix: the old path returned a wrapped 403 instead).
Role-guard test gaps — new tests: setMemberRole rejects an unknown role with 400 and leaves the role unchanged; setRole wraps an invalid form role as fail(400) instead of casting it through; invite accept asserts sessions.active_org_id lands in the joined org.
Invites empty state — now uses the shared EmptyState component (I12).
Revoke button names its target (I13) — "Revoke the {role} invite link expiring {date}".
ALREADY FIXED in ce670c7 (CodeRabbit reviewed the older commit): session updates scoped+verified by userId.
NOT VALID / no change:
requireSharedOrg passes when the org row is missing — unreachable: createInvite checks membershipOf first, and a membership row can only exist for an existing org (FK). The 404 fires there.
membershipOf role read outside the write transaction — the safety invariants (single-use invite, ownerless org) no longer depend on any pre-read state: they are statement-level conditional writes since ce670c7. What remains is a millisecond TOCTOU window on authorization (a just-demoted admin completing one in-flight action), which is inherent to request-scoped role resolution and acceptable here.
Invite landing page loading skeleton / EmptyState — the "no longer valid" state is plan-specified as .error-box (an actionable error, which I12 reserves for exactly this); unknown tokens 404 via the load. No blank or unstyled state exists.
Gates: npm run check / npm run build / npm run test all green — 363 tests.
Of CodeAnt's four suggestions, three were fixed in earlier rounds (conditional invite claim, unconditional burn with test, scoped+verified session update). The fourth:
"Signing in from an invite loses the invite URL" — NOT VALID for this phase. Plan step D6 specifies this exact behavior: "Logged-out visitors are asked to sign in and come back to this same URL (invites stay valid for 7 days)." The landing page instructs the user to sign in and reopen the link; nothing breaks, the invite remains valid. Threading a return-URL through the Google OAuth login flow is a real UX improvement but belongs to a follow-up — it touches the auth/consent flow, which is out of Phase D scope and explicitly not in the plan's file list.
Check-suite notes (no action needed from this PR):
Codacy "not up to standards" is metrics-only: 0 issues; complexity 193/100 is the file-aggregate for org.ts growing with Phase D's functions (each individually simple); duplication 10/1 is the known test-file metric noise.
Snyk — code/snyk failed on quota ("Code test limit reached"); security/snyk reports 1 failure behind the Snyk login. This PR changes no dependencies (package.json untouched), so a new-vulnerability signal is not actionable from the diff — flagging for maintainer visibility in the Snyk dashboard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
codeant-ai
Bot
added
the
size:XXL
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
User description
Behavior
Phase D of the multi-tenancy plan — team management UI and single-use invite links on top of the Phase A–C tenancy core:
org.tsteam functions (D1):createOrg,renameOrg,createInvite,revokeInvite,previewInvite,acceptInvite,switchActiveOrg,listMembers,listOpenInvites,setMemberRole,removeMember,leaveOrg— all role-gated server-side (route checks are defense-in-depth, not the gate).POST /org/switch(D2): nav team switcher target; 303s to the dashboard./orgteam settings (D3+D4): rename, member roster with role/remove controls, invite link creation/revocation, create-another-team, leave team. Error states via.error-boxform failures (I12); every control labeled and target-named (I13).active_org_idand re-scopes every channel query (Phase C ownership)./invite/[token](D6): public landing; unknown tokens are plain 404s (no existence leak), burned/expired links render "no longer valid", signed-out visitors are asked to sign in and return.Deviations from the plan's verbatim code (reconciliations)
ROLE_RANK/requireRole(already inownership.ts). Single fail-closedrequireRolenow lives inorg.ts;requireOrgRoledelegates. Copy-paste rule.listMembersorders in SQL (createdAt,userId) instead oflocaleCompare— locale-sensitive sorting was already (validly) flagged in the Phase B review.createInvite400s onpersonal_fororgs;acceptInviterefuses any legacy/hand-written invite row pointing at a personal org (Phase C review finding, carried into this phase as planned).listOpenInvitesfails loudly on an unknown invite role (same guard aspreviewInvite) instead of silently filtering rows — "never silent fallbacks".guard()is generic so theinviteaction reuses it instead of duplicating the error-unwrapping try/catch.Verification
npm run check/npm run build/npm run testall green — 355 tests (30 new), including route tests for/org/switch,/orgload+actions, and/invite/[token].acceptedByburn fromacceptInvite→ the single-use test failed; reverted → green..playwright-cli/pr-org-team.png,.playwright-cli/pr-dashboard-acme.png,.playwright-cli/pr-dashboard-side.png(gitignored scratch dir — ask and I'll attach them in a comment).The second-profile invite-accept flow needs a real second Google account — covered instead by route tests (join → 303, membership written with the invite's role, token burned, reuse → 410,
acceptedflag on reload).Note: PR #51 (
chore-drizzle-skill) is unrelated work from a concurrent session and is not part of this phase.CodeAnt-AI Description
Add team management, switching, and single-use invitations
What Changed
Impact
✅ Self-service team administration✅ Single-use, expiring team invites✅ Safer team switching and ownership changes💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.