[codex] Fix org invite visibility and existing-user invite redirects#1877
Open
[codex] Fix org invite visibility and existing-user invite redirects#1877
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAuto-opens organization invitation dialogs from an Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser as Browser/Router
participant Frontend as Frontend App
participant EdgeFn as Edge Function
participant DB as Database
participant Bento as Bento Tracking
User->>Browser: Open URL with ?invite_org=<gid>
Browser->>Frontend: Navigate (route includes invite_org)
Frontend->>Frontend: fetchOrganizations()
Frontend->>Frontend: openInvitationFromRouteIfNeeded()
Frontend->>Browser: router.replace() to remove invite_org
User->>Frontend: Confirm invite / trigger notify
Frontend->>EdgeFn: POST /private/send_existing_user_org_invite { email, org_id }
EdgeFn->>DB: lookup org, inviter, invited user, org_membership
EdgeFn->>DB: verify membership.user_right startsWith("invite_")
EdgeFn->>Bento: emit org:invite_existing_capgo_user_to_org
Bento-->>EdgeFn: ack
EdgeFn-->>Frontend: { status: "ok" }
Frontend->>User: show success toast or failure toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7729f8f028
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
supabase/functions/_backend/private/send_existing_user_org_invite.ts (2)
20-38: Mixed error handling patterns:throwvsreturn.
validateRequestthrowssimpleErrorfor validation failures but returns aResponsefor authorization failures. This forces the caller to handle both exceptions and Response values. Consider returning consistently (either always throw or always return).♻️ Consistent return-based approach
async function validateRequest(c: Context, rawBody: unknown) { const validationResult = sendInviteSchema.safeParse(rawBody) if (!validationResult.success) { - throw simpleError('invalid_request', 'Invalid request', { errors: z.prettifyError(validationResult.error) }) + return quickError(400, 'invalid_request', 'Invalid request', { errors: z.prettifyError(validationResult.error) }) } const body = validationResult.data🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/send_existing_user_org_invite.ts` around lines 20 - 38, The validateRequest function mixes throwing (simpleError on validation failure) with returning Response-like values (quickError on auth failure); make the behavior consistent by returning Response-like errors for both cases instead of throwing. Update validateRequest to return quickError(400, 'invalid_request', ...) with z.prettifyError(validationResult.error) when sendInviteSchema.safeParse fails, keeping the existing quickError(403, ...) path; ensure callers of validateRequest expect and handle the returned { body, canUpdateUserRoles } or a quickError Response rather than catching exceptions from simpleError. Reference: validateRequest, sendInviteSchema, simpleError, quickError, checkPermission.
40-52: MissingrequestIdusage for structured logging.Per coding guidelines, backend endpoints should use
c.get('requestId')for structured logging withcloudlog(). This endpoint performs multiple database operations and a Bento event but has no logging for debugging or audit purposes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/send_existing_user_org_invite.ts` around lines 40 - 52, Retrieve the requestId via c.get('requestId') at the start of the handler and use cloudlog() for structured logging around key steps: after successful validation (log requestId, inviterId, canUpdateUserRoles and a redacted summary of body from validateRequest), before/after each major DB operation and before emitting the Bento event (include identifiers like targetUserId/orgId if present), and on error paths (including the quickError 401 branch) so failures include requestId and contextual fields; update send_existing_user_org_invite.ts to call cloudlog({ requestId, inviterId, ... }) in the parseBody/validateRequest success path, around database calls, and prior to the Bento event emission.tests/private-send-existing-user-org-invite.test.ts (2)
59-73: Test covers happy path only.The test validates the success case but doesn't cover error scenarios such as:
- Invalid email format
- Non-existent user
- Already accepted invitation (409 response)
- Missing permissions (403 response)
Consider adding edge case tests in a follow-up to improve coverage.
Would you like me to generate additional test cases for these error scenarios?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/private-send-existing-user-org-invite.test.ts` around lines 59 - 73, Add edge-case tests alongside the existing happy-path test for the POST /private/send_existing_user_org_invite endpoint: create tests that call the same endpoint with (1) an invalid email format (use USER_EMAIL_NONMEMBER or a new invalid string) and assert a 400/validation error, (2) a non-existent user email and assert a 404, (3) a user who has already accepted the invite and assert a 409, and (4) a caller without sufficient permissions (alter authHeaders or use a non-admin token) and assert a 403; reuse testOrgId, authHeaders, and the existing test structure to set up requests and assertions so the new tests mirror the style of the current "returns ok for an existing user with a pending org invitation" spec.
60-60: Consider usingit.concurrent()for parallel test execution.Per coding guidelines, tests should use
it.concurrent()when possible. This test appears to have isolated test data (unique UUIDs) and should be safe for concurrent execution.♻️ Use concurrent test
- it('returns ok for an existing user with a pending org invitation', async () => { + it.concurrent('returns ok for an existing user with a pending org invitation', async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/private-send-existing-user-org-invite.test.ts` at line 60, The test "returns ok for an existing user with a pending org invitation" should be changed to run concurrently: replace the test declaration it('returns ok for an existing user with a pending org invitation', async () => { ... }) with it.concurrent('returns ok for an existing user with a pending org invitation', async () => { ... }); ensure the test body (UUID generation and any shared fixtures) remains isolated so no shared state prevents concurrent execution and update only the test declaration in tests/private-send-existing-user-org-invite.test.ts.src/components/dashboard/InviteTeammateModal.vue (1)
244-252: Invite notification failure is silently ignored.The
notifyExistingUserInvitecall is awaited but its return value (success/failure) is not used. If the email notification fails, the user still sees the success toast. This appears intentional since the invite itself succeeded, but consider logging or showing a warning if the notification fails.💡 Optional: Add warning on notification failure
if (data === 'OK') { - await notifyExistingUserInvite(supabase, email, orgId) + const notified = await notifyExistingUserInvite(supabase, email, orgId) + if (!notified) { + console.warn('Failed to send invite email notification, but invite was created') + } toast.success(t('org-invited-user'))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/InviteTeammateModal.vue` around lines 244 - 252, The invite notification call notifyExistingUserInvite(supabase, email, orgId) is awaited but any failure is ignored, so update the block that handles the 'OK' response to catch notification errors: wrap the notifyExistingUserInvite call in a try/catch (or check its returned success flag) and if it fails log the error (e.g., console.error or a logger) and surface a non-blocking warning to the user (e.g., toast.warning('Could not send invite email') or similar) while still calling completeInviteSuccess({ email, firstName: '', lastName: '' }) and showing the original success toast; keep toast.success for the invite result but add the warning/log on notification failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/dashboard/DropdownOrganization.vue`:
- Line 33: The component-level ref handledInviteOrgId is preventing re-opening
the invite modal across the SPA session; change the behavior so invite deduping
is scoped to a single open-attempt rather than the whole component lifetime:
either make handledInviteOrgId a local variable inside
openInvitationFromRouteIfNeeded() (so it’s reset on each invocation) or
explicitly reset handledInviteOrgId when you call clearInviteOrgQuery() or when
the invitation dialog closes (e.g., in the handler that closes the modal).
Update logic in openInvitationFromRouteIfNeeded(), clearInviteOrgQuery(), and
the invite-close handler so the invite_org param can trigger the modal again if
the user re-clicks the same link in the same session. Ensure you keep references
to handledInviteOrgId, openInvitationFromRouteIfNeeded, and clearInviteOrgQuery
when locating the spots to change.
In `@src/modules/auth.ts`:
- Line 151: The two email verification redirects to '/resend_email' are
inconsistent: one sets return_to: to.fullPath while the other uses to.path
(losing query params like ?invite_org=). Update the second redirect (the branch
handling the already-authenticated but unverified email — the block checking the
'email_not_verified' condition that issues a redirect to '/resend_email') to set
return_to: to.fullPath instead of to.path so both redirects behave identically
and preserve query parameters; search for the redirect to '/resend_email' and
the return_to key in auth.ts to locate and fix the code.
In `@supabase/functions/_backend/private/send_existing_user_org_invite.ts`:
- Around line 11-14: The sendInviteSchema uses z.string().check(z.minLength(1))
for org_id which is invalid in Zod v4; update the schema so org_id is defined as
z.string().min(1) (i.e., change the org_id line inside sendInviteSchema to use
z.string().min(1)) and keep the email as z.email(); this will ensure proper
minimum-length validation with Zod 4.
- Around line 16-18: Replace the Hono instance construction so the endpoint uses
the centralized middleware: import createHono from ../utils/hono.ts, replace new
Hono<MiddlewareKeyVariables>() with createHono('', version), and keep existing
middleware (e.g., app.use('/', useCors)) so app is created via createHono to
include logger, requestId, and X-Worker-Source headers; ensure the createHono
import and the version variable are present in the file.
---
Nitpick comments:
In `@src/components/dashboard/InviteTeammateModal.vue`:
- Around line 244-252: The invite notification call
notifyExistingUserInvite(supabase, email, orgId) is awaited but any failure is
ignored, so update the block that handles the 'OK' response to catch
notification errors: wrap the notifyExistingUserInvite call in a try/catch (or
check its returned success flag) and if it fails log the error (e.g.,
console.error or a logger) and surface a non-blocking warning to the user (e.g.,
toast.warning('Could not send invite email') or similar) while still calling
completeInviteSuccess({ email, firstName: '', lastName: '' }) and showing the
original success toast; keep toast.success for the invite result but add the
warning/log on notification failure.
In `@supabase/functions/_backend/private/send_existing_user_org_invite.ts`:
- Around line 20-38: The validateRequest function mixes throwing (simpleError on
validation failure) with returning Response-like values (quickError on auth
failure); make the behavior consistent by returning Response-like errors for
both cases instead of throwing. Update validateRequest to return quickError(400,
'invalid_request', ...) with z.prettifyError(validationResult.error) when
sendInviteSchema.safeParse fails, keeping the existing quickError(403, ...)
path; ensure callers of validateRequest expect and handle the returned { body,
canUpdateUserRoles } or a quickError Response rather than catching exceptions
from simpleError. Reference: validateRequest, sendInviteSchema, simpleError,
quickError, checkPermission.
- Around line 40-52: Retrieve the requestId via c.get('requestId') at the start
of the handler and use cloudlog() for structured logging around key steps: after
successful validation (log requestId, inviterId, canUpdateUserRoles and a
redacted summary of body from validateRequest), before/after each major DB
operation and before emitting the Bento event (include identifiers like
targetUserId/orgId if present), and on error paths (including the quickError 401
branch) so failures include requestId and contextual fields; update
send_existing_user_org_invite.ts to call cloudlog({ requestId, inviterId, ... })
in the parseBody/validateRequest success path, around database calls, and prior
to the Bento event emission.
In `@tests/private-send-existing-user-org-invite.test.ts`:
- Around line 59-73: Add edge-case tests alongside the existing happy-path test
for the POST /private/send_existing_user_org_invite endpoint: create tests that
call the same endpoint with (1) an invalid email format (use
USER_EMAIL_NONMEMBER or a new invalid string) and assert a 400/validation error,
(2) a non-existent user email and assert a 404, (3) a user who has already
accepted the invite and assert a 409, and (4) a caller without sufficient
permissions (alter authHeaders or use a non-admin token) and assert a 403; reuse
testOrgId, authHeaders, and the existing test structure to set up requests and
assertions so the new tests mirror the style of the current "returns ok for an
existing user with a pending org invitation" spec.
- Line 60: The test "returns ok for an existing user with a pending org
invitation" should be changed to run concurrently: replace the test declaration
it('returns ok for an existing user with a pending org invitation', async () =>
{ ... }) with it.concurrent('returns ok for an existing user with a pending org
invitation', async () => { ... }); ensure the test body (UUID generation and any
shared fixtures) remains isolated so no shared state prevents concurrent
execution and update only the test declaration in
tests/private-send-existing-user-org-invite.test.ts.
🪄 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: 4b25cafc-d63b-410d-9afc-85e44b3bd94c
📒 Files selected for processing (8)
src/components/dashboard/DropdownOrganization.vuesrc/components/dashboard/InviteTeammateModal.vuesrc/modules/auth.tssrc/pages/settings/organization/Members.vuesrc/utils/invites.tssupabase/functions/_backend/private/send_existing_user_org_invite.tssupabase/functions/private/index.tstests/private-send-existing-user-org-invite.test.ts
supabase/functions/_backend/private/send_existing_user_org_invite.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af81685e66
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/stores/organization.ts (1)
397-411:⚠️ Potential issue | 🟠 MajorFilter invite-only orgs out of the app-id index.
Keeping
_organizationspopulated here is fine for the switcher, but the watcher at Lines 247-287 now also indexes apps for pending invites. If an app id gets mapped,hasPermissionsInRole()will treatinvite_*as an active role becausenormalizeLegacyRole()strips the prefix. Preserve invite-only orgs for the dropdown, but build_organizationsByAppIdonly from selectable orgs.💡 Suggested fix (outside this hunk)
- const organizations = Array.from(organizationsMap.values()) + const organizations = Array.from(organizationsMap.values()) + .filter(org => isSelectableOrganization(org.role))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/organization.ts` around lines 397 - 411, The _organizations map correctly keeps invite-only orgs for the switcher, but the watcher is indexing apps from all orgs which causes invite_* roles to be treated as active; change the code that builds _organizationsByAppId (the app-id index) to iterate only over selectableOrganizations (the result of mappedData.filter(isSelectableOrganization)) instead of mappedData; ensure the logic that sets _organizationsByAppId.value (and any helper that populates it) uses selectableOrganizations so invite-only orgs remain in _organizations but are excluded from the app-id index used by hasPermissionsInRole/normalizeLegacyRole.src/components/dashboard/DropdownOrganization.vue (1)
113-121:⚠️ Potential issue | 🟠 MajorScope invite rejection to the current org.
The deny path only filters
org_usersbyuser_id. Declining one invite will delete every row this user is allowed to delete, not just the invitation fororg.gid. Add the org predicate too (org_idif that's the FK column here).💡 Suggested fix
const { error } = await supabase .from('org_users') .delete() + .eq('org_id', org.gid) .eq('user_id', userId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dashboard/DropdownOrganization.vue` around lines 113 - 121, The delete handler currently removes org_users rows by only filtering on user_id (see the handler async function and the supabase.from('org_users').delete().eq('user_id', userId) call), which can delete invites across all orgs; update that query to also filter by the current organization (add an .eq('org_id', org.gid) or the correct FK column name) so the delete targets only the invite for the current org; ensure you read the org identifier from the component state (org.gid) and include it in the supabase query before executing the delete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/auth.ts`:
- Around line 99-100: The current logic sets hasInviteOrgQuery true for any
non-empty invite_org query and lets shouldRedirectToOrgOnboarding skip
onboarding even if the invite is stale; change this to defer the invite check
until after fetchOrganizations() completes: read invite_gid from
to.query.invite_org, call fetchOrganizations(), then check the loaded
organizations list for an entry with gid === invite_gid and a role that
startsWith('invite') (or matches invite*), and only then set
shouldRedirectToOrgOnboarding to false; update the same pattern used around
hasInviteOrgQuery/shouldRedirectToOrgOnboarding at the other occurrences (the
blocks around the noted alternate locations) so stale/revoked invites no longer
suppress onboarding and DropdownOrganization.vue receives a valid org context.
In `@tests/private-send-existing-user-org-invite.test.ts`:
- Around line 21-61: The shared fixture created in beforeAll (the
org/stripe/org_users seeded via getSupabaseClient() using
testOrgId/testCustomerId and users USER_ID/USER_ID_NONMEMBER) is mutated by one
test, preventing safe parallel runs; change to per-test seeding: move the
org/stripe/org_users inserts from beforeAll into beforeEach (or have the
mutating test create its own dedicated org and org_users) so each test gets an
isolated org id and customer id (generate a unique testOrgId/testCustomerId per
test) and avoid reusing the shared org_users entry that includes
USER_ID_NONMEMBER; ensure any test that modifies membership seeds its own data
rather than relying on the global beforeAll fixture.
---
Outside diff comments:
In `@src/components/dashboard/DropdownOrganization.vue`:
- Around line 113-121: The delete handler currently removes org_users rows by
only filtering on user_id (see the handler async function and the
supabase.from('org_users').delete().eq('user_id', userId) call), which can
delete invites across all orgs; update that query to also filter by the current
organization (add an .eq('org_id', org.gid) or the correct FK column name) so
the delete targets only the invite for the current org; ensure you read the org
identifier from the component state (org.gid) and include it in the supabase
query before executing the delete.
In `@src/stores/organization.ts`:
- Around line 397-411: The _organizations map correctly keeps invite-only orgs
for the switcher, but the watcher is indexing apps from all orgs which causes
invite_* roles to be treated as active; change the code that builds
_organizationsByAppId (the app-id index) to iterate only over
selectableOrganizations (the result of
mappedData.filter(isSelectableOrganization)) instead of mappedData; ensure the
logic that sets _organizationsByAppId.value (and any helper that populates it)
uses selectableOrganizations so invite-only orgs remain in _organizations but
are excluded from the app-id index used by
hasPermissionsInRole/normalizeLegacyRole.
🪄 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: 9bc045f8-7780-4aa2-aac3-d95cdec1c6fa
📒 Files selected for processing (7)
messages/en.jsonsrc/components/dashboard/DropdownOrganization.vuesrc/components/dashboard/InviteTeammateModal.vuesrc/modules/auth.tssrc/stores/organization.tssupabase/functions/_backend/private/send_existing_user_org_invite.tstests/private-send-existing-user-org-invite.test.ts
✅ Files skipped from review due to trivial changes (2)
- messages/en.json
- supabase/functions/_backend/private/send_existing_user_org_invite.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/dashboard/InviteTeammateModal.vue
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: daa9a1aa1a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
supabase/functions/_backend/private/send_existing_user_org_invite.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
supabase/functions/_backend/private/send_existing_user_org_invite.ts (1)
17-18: In-memory cooldown map is not shared across Edge Function instances.The
inviteNotificationCooldownsMap only provides per-instance rate limiting. In a distributed environment (Supabase Edge Functions or Cloudflare Workers), different requests may hit different instances, bypassing the in-memory check. TheCacheHelperprovides the durable layer, but the in-memory map offers minimal value and may give a false sense of protection.Consider removing the in-memory map and relying solely on
CacheHelper, or document that it's purely an optimization for rapid successive requests within the same instance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@supabase/functions/_backend/private/send_existing_user_org_invite.ts` around lines 17 - 18, The in-memory Map inviteNotificationCooldowns provides only per-instance cooldowns and should be removed; delete the inviteNotificationCooldowns declaration and any checks/updates that reference it, and instead rely solely on CacheHelper for durable cooldown checks and writes using the existing INVITE_RESEND_COOLDOWN_MINUTES as the TTL; ensure all cooldown logic in send_existing_user_org_invite (or any helper called by it) uses CacheHelper.get/check and CacheHelper.set with a deterministic key (e.g., user/org invite key) and TTL derived from INVITE_RESEND_COOLDOWN_MINUTES so distributed instances respect the same cooldown.tests/private-send-existing-user-org-invite.test.ts (1)
77-169: Good test isolation with per-test fixtures; consider adding coverage for edge cases.The test suite correctly uses
it.concurrent()with isolated fixtures per test, addressing the previous review feedback. The cleanup infinallyblocks ensures proper teardown.However, a few endpoint behaviors are not covered:
- Organization not found (404
organization_not_found)- Cooldown/rate limiting (409
user_already_invited)invite_super_adminpermission escalation checkWould you like me to generate additional test cases for these edge cases?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/private-send-existing-user-org-invite.test.ts` around lines 77 - 169, Add three new concurrent tests to cover missing edge cases: (1) "organization not found" — call postSendExistingUserOrgInvite with a non-existent org_id (e.g., fixture.orgId + '-missing') and assert response.status === 404 and returned error === 'organization_not_found'; (2) "cooldown / already invited" — simulate a prior invite by inserting an org_users row or calling postSendExistingUserOrgInvite twice for the same email/org and assert response.status === 409 and error === 'user_already_invited'; and (3) "invite_super_admin permission escalation" — attempt to invite with credentials lacking super-admin rights (use nonMemberAuthHeaders or create a low-priv fixture) targeting a payload that would grant elevated rights and assert response.status === 403 and error === 'not_authorized'; reuse createInviteTestFixture, authHeaders, nonMemberAuthHeaders, USER_EMAIL_NONMEMBER, USER_ID_NONMEMBER and ensure each test uses its own fixture and cleanup in finally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@supabase/functions/_backend/private/send_existing_user_org_invite.ts`:
- Around line 17-18: The in-memory Map inviteNotificationCooldowns provides only
per-instance cooldowns and should be removed; delete the
inviteNotificationCooldowns declaration and any checks/updates that reference
it, and instead rely solely on CacheHelper for durable cooldown checks and
writes using the existing INVITE_RESEND_COOLDOWN_MINUTES as the TTL; ensure all
cooldown logic in send_existing_user_org_invite (or any helper called by it)
uses CacheHelper.get/check and CacheHelper.set with a deterministic key (e.g.,
user/org invite key) and TTL derived from INVITE_RESEND_COOLDOWN_MINUTES so
distributed instances respect the same cooldown.
In `@tests/private-send-existing-user-org-invite.test.ts`:
- Around line 77-169: Add three new concurrent tests to cover missing edge
cases: (1) "organization not found" — call postSendExistingUserOrgInvite with a
non-existent org_id (e.g., fixture.orgId + '-missing') and assert
response.status === 404 and returned error === 'organization_not_found'; (2)
"cooldown / already invited" — simulate a prior invite by inserting an org_users
row or calling postSendExistingUserOrgInvite twice for the same email/org and
assert response.status === 409 and error === 'user_already_invited'; and (3)
"invite_super_admin permission escalation" — attempt to invite with credentials
lacking super-admin rights (use nonMemberAuthHeaders or create a low-priv
fixture) targeting a payload that would grant elevated rights and assert
response.status === 403 and error === 'not_authorized'; reuse
createInviteTestFixture, authHeaders, nonMemberAuthHeaders,
USER_EMAIL_NONMEMBER, USER_ID_NONMEMBER and ensure each test uses its own
fixture and cleanup in finally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f401019a-2314-434d-bb53-3dd630d7e4aa
📒 Files selected for processing (5)
src/auto-imports.d.tssrc/modules/auth.tssrc/pages/settings/organization/Members.vuesupabase/functions/_backend/private/send_existing_user_org_invite.tstests/private-send-existing-user-org-invite.test.ts
✅ Files skipped from review due to trivial changes (2)
- src/auto-imports.d.ts
- src/pages/settings/organization/Members.vue
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5af050c85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
supabase/functions/_backend/private/send_existing_user_org_invite.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 322f95ee42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary (AI generated)
/dashboard?invite_org=<orgId>and auto-open the existing accept-invite modal from the org switcherMotivation (AI generated)
Existing org invites already carried enough backend state to distinguish pending invites from accepted memberships, but that state was never surfaced in the members table. Separately, existing-account invites stopped after the RPC created the pending org membership, so no email notification was sent, and even when adding the email path the original redirect would have landed users on the dashboard without opening the invite modal.
Business Impact (AI generated)
This makes org invite state legible for admins, reduces confusion around whether an invite was accepted, and restores email-driven onboarding for invited users who already have an account. The direct modal deep-link should improve acceptance completion and reduce support/debug churn around “I got invited but don’t know what to do next.”
Test Plan (AI generated)
bunx eslint src/components/dashboard/DropdownOrganization.vue src/modules/auth.ts src/components/dashboard/InviteTeammateModal.vue src/pages/settings/organization/Members.vue src/utils/invites.ts supabase/functions/_backend/private/send_existing_user_org_invite.ts tests/private-send-existing-user-org-invite.test.tsbun run supabase:with-env -- bunx vitest run tests/private-send-existing-user-org-invite.test.tsbun run typecheckGenerated with AI
Summary by CodeRabbit
New Features
UI
Bug Fixes
Localization
Tests