refactor(web): migrate all auth to withAuthV2/withOptionalAuthV2#1071
Merged
brendan-kellam merged 3 commits intomainfrom Apr 1, 2026
Merged
refactor(web): migrate all auth to withAuthV2/withOptionalAuthV2#1071brendan-kellam merged 3 commits intomainfrom
brendan-kellam merged 3 commits intomainfrom
Conversation
Replace all usages of the legacy withAuth and withOrgMembership helpers with the newer withAuthV2, withOptionalAuthV2, and withMinimumOrgRole APIs. This consolidates auth into a single context object that provides user, org, role, and a user-scoped prisma client, eliminating the need for separate domain parameters and nested auth wrappers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
@brendan-kellam your pull request is missing a changelog! |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughConsolidates auth wiring: replaces legacy withAuth/withOrgMembership and domain/user params with withAuthV2 variants (including withAuthV2_skipOrgMembershipCheck and withMinimumOrgRole). Server actions and UI call-sites now derive org/user/role from auth context and many OWNER-gate flows were added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ServerAction as Server Action (e.g., createInvites)
participant Auth as withAuthV2 / withMinimumOrgRole
participant DB as Prisma
Client->>ServerAction: call action() %% no domain/user args
ServerAction->>Auth: getAuthContext() / role check
Auth-->>ServerAction: { user, org, role, prisma } or ServiceError
ServerAction->>DB: prisma.query / write using org.id, user.id
DB-->>ServerAction: result
ServerAction-->>Client: success or ServiceError
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 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 |
![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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/web/src/actions.ts (2)
1193-1214:⚠️ Potential issue | 🟠 Major
originis undefined in the approval-email path.Line 1213 interpolates
origin, but nothing in this scope defines it. When SMTP is configured, this throws afteraddUserToOrganization(...)succeeds, so the user gets added while the action returns an unexpected error. Reuseenv.AUTH_URL(or another defined base URL) for the plain-text link.🐛 Suggested fix
+ const orgUrl = new URL(`/${org.domain}`, env.AUTH_URL).toString(); const transport = createTransport(smtpConnectionUrl); const result = await transport.sendMail({ to: request.requestedBy.email!, from: env.EMAIL_FROM_ADDRESS, subject: `Your request to join ${org.name} has been approved`, html, - text: `Your request to join ${org.name} on Sourcebot has been approved. You can now access the organization at ${origin}/${org.domain}`, + text: `Your request to join ${org.name} on Sourcebot has been approved. You can now access the organization at ${orgUrl}`, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/actions.ts` around lines 1193 - 1214, The plain-text email uses an undefined variable origin causing a runtime error after addUserToOrganization; update the transport.sendMail call in the approval-email path to use env.AUTH_URL (or env.AUTH_URL + `/${org.domain}`) instead of origin in the text body, and remove any accidental references to origin in this scope (look for render/JoinRequestApprovedEmail, createTransport, and transport.sendMail usages to locate the code).
836-845:⚠️ Potential issue | 🔴 Critical
withAuthV2blocks invite redemption for non-members.
withAuthV2returnsnotAuthenticated()whenrole === OrgRole.GUEST(packages/web/src/withAuthV2.ts:27-41). Invited users are still guests untilredeemInvite(...)succeeds, so bothgetInviteInfoandredeemInvitenow fail before they can read or consume the invite. These flows need an auth-only wrapper that requires a logged-in user but does not require existing org membership.Also applies to: 910-918
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/actions.ts` around lines 836 - 845, The redeemInvite and getInviteInfo handlers are using withAuthV2 which rejects users with role === OrgRole.GUEST, blocking invite redemption for invited-but-not-yet-members; replace their use of withAuthV2 with an auth-only wrapper that requires a logged-in user but does not enforce org membership (either a new withAuthUser/withAuthOnly helper or a withAuthV2 option e.g. { allowGuests: true }), and update redeemInvite (and the other handler at the 910-918 range) to call that wrapper so the route can read/consume the invite while the user is still a guest; keep the inner logic (user, prisma) the same and ensure the new wrapper still provides the same context shape used by redeemInvite/getInviteInfo.packages/web/src/app/[domain]/repos/layout.tsx (1)
27-32:⚠️ Potential issue | 🟡 MinorMissing error handling for
getCurrentUserRole()result.The result of
getCurrentUserRole()is used directly in the comparison at line 32 without checking forisServiceError. If the call fails,userRoleInOrgwould be aServiceErrorobject, and the comparison withOrgRole.OWNERwould silently evaluate tofalse.While this has a safe fallback (the info banner won't show), it silently swallows errors that might indicate authentication or authorization issues.
🛡️ Suggested fix to add error handling
const userRoleInOrg = await getCurrentUserRole(); + if (isServiceError(userRoleInOrg)) { + throw new ServiceErrorException(userRoleInOrg); + } return ( <div className="min-h-screen flex flex-col"> <NavigationMenu domain={domain} /> {(repoStats.numberOfRepos === 0 && userRoleInOrg === OrgRole.OWNER) && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/`[domain]/repos/layout.tsx around lines 27 - 32, The call to getCurrentUserRole() may return a ServiceError but userRoleInOrg is used directly in the OrgRole.OWNER comparison, so add explicit error handling after calling getCurrentUserRole(): check whether userRoleInOrg is a ServiceError (use your isServiceError helper or type guard), then handle it (e.g., log the error with context, return an error UI or rethrow) instead of silently letting the comparison proceed; only proceed to compare to OrgRole.OWNER when userRoleInOrg is a valid role value. Ensure you update the flow in layout.tsx around the getCurrentUserRole() call and the conditional that references userRoleInOrg.
🧹 Nitpick comments (4)
packages/web/src/app/components/anonymousAccessToggle.tsx (2)
53-67: Use Tailwind color classes instead of CSS variable syntax.Multiple lines in this component use CSS variable syntax (e.g.,
border-[var(--border)],bg-[var(--card)],text-[var(--foreground)]) instead of Tailwind's semantic color classes.As per coding guidelines: "Use Tailwind color classes directly instead of CSS variable syntax (e.g., use
border-border bg-card text-foregroundinstead ofborder-[var(--border)] bg-[var(--card)])."♻️ Suggested refactor for lines 53-67
- <div className={`p-4 rounded-lg border border-[var(--border)] bg-[var(--card)] ${(!hasAnonymousAccessEntitlement || forceEnableAnonymousAccess) ? 'opacity-60' : ''}`}> + <div className={`p-4 rounded-lg border border-border bg-card ${(!hasAnonymousAccessEntitlement || forceEnableAnonymousAccess) ? 'opacity-60' : ''}`}> <div className="flex items-start justify-between gap-4"> <div className="flex-1 min-w-0"> - <h3 className="font-medium text-[var(--foreground)] mb-2"> + <h3 className="font-medium text-foreground mb-2"> Enable anonymous access </h3> <div className="max-w-2xl"> - <p className="text-sm text-[var(--muted-foreground)] leading-relaxed"> + <p className="text-sm text-muted-foreground leading-relaxed"> When enabled, users can access your deployment without logging in. </p> {showPlanMessage && ( - <div className="mt-3 p-3 rounded-md bg-[var(--muted)] border border-[var(--border)]"> - <p className="text-sm text-[var(--foreground)] leading-relaxed flex items-center gap-2"> + <div className="mt-3 p-3 rounded-md bg-muted border border-border"> + <p className="text-sm text-foreground leading-relaxed flex items-center gap-2"> <svg - className="w-4 h-4 flex-shrink-0 text-[var(--muted-foreground)]" + className="w-4 h-4 flex-shrink-0 text-muted-foreground"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/components/anonymousAccessToggle.tsx` around lines 53 - 67, The JSX uses arbitrary CSS variable syntax in className (e.g., "border-[var(--border)]", "bg-[var(--card)]", "text-[var(--foreground)]", "text-[var(--muted-foreground)]", "bg-[var(--muted)]") inside the AnonymousAccessToggle component; replace those with the project's Tailwind semantic color classes (e.g., border-border, bg-card, text-foreground, text-muted-foreground, bg-muted) wherever they appear — specifically update the outer container div, the h3, the paragraph tags, the showPlanMessage wrapper div, and the svg className so all instances use the semantic classes instead of the var(...) syntax while preserving the existing conditional class logic (the same className strings like the opacity toggle using hasAnonymousAccessEntitlement and forceEnableAnonymousAccess should remain).
94-115: Additional CSS variable usages should also be updated.The same pattern of CSS variable syntax continues in the
showForceEnableMessageblock.♻️ Suggested refactor for lines 94-115
{showForceEnableMessage && ( - <div className="mt-3 p-3 rounded-md bg-[var(--muted)] border border-[var(--border)]"> - <p className="text-sm text-[var(--foreground)] leading-relaxed flex items-center gap-2"> + <div className="mt-3 p-3 rounded-md bg-muted border border-border"> + <p className="text-sm text-foreground leading-relaxed flex items-center gap-2"> <svg - className="w-4 h-4 flex-shrink-0 text-[var(--muted-foreground)]" + className="w-4 h-4 flex-shrink-0 text-muted-foreground" fill="none" viewBox="0 0 24 24" stroke="currentColor" > <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M13 16h-1v-4h-1m1-4h.01M21 12a9 9 0 11-18 0 9 9 0 0118 0z" /> </svg> <span> - The <code className="bg-[var(--secondary)] px-1 py-0.5 rounded text-xs font-mono">forceEnableAnonymousAccess</code> is set, so this cannot be changed from the UI. + The <code className="bg-secondary px-1 py-0.5 rounded text-xs font-mono">forceEnableAnonymousAccess</code> is set, so this cannot be changed from the UI. </span>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/components/anonymousAccessToggle.tsx` around lines 94 - 115, In the showForceEnableMessage block of anonymousAccessToggle.tsx, replace the inline Tailwind arbitrary CSS variable usages with the project's standardized utility classes: update "bg-[var(--muted)]", "border-[var(--border)]", "text-[var(--foreground)]", "text-[var(--muted-foreground)]", and "bg-[var(--secondary)]" to the equivalent design-system tokens (e.g., bg-muted, border-border, text-foreground, text-muted-foreground, bg-secondary) so the component uses the consistent CSS variable/class naming convention; adjust the className strings in the JSX inside the conditional render tied to showForceEnableMessage accordingly.packages/web/src/app/components/inviteLinkToggle.tsx (1)
69-122: Use Tailwind color classes instead of CSS variable syntax.This component uses CSS variable syntax throughout (e.g.,
border-[var(--border)],bg-[var(--card)],text-[var(--foreground)]). Consider refactoring to use Tailwind's semantic color classes for consistency.As per coding guidelines: "Use Tailwind color classes directly instead of CSS variable syntax."
♻️ Suggested refactor (partial example)
- <div className="p-4 rounded-lg border border-[var(--border)] bg-[var(--card)]"> + <div className="p-4 rounded-lg border border-border bg-card"> <div className="flex items-start justify-between gap-4"> <div className="flex-1 min-w-0"> - <h3 className="font-medium text-[var(--foreground)] mb-2"> + <h3 className="font-medium text-foreground mb-2"> Enable invite link </h3> <div className="max-w-2xl"> - <p className="text-sm text-[var(--muted-foreground)] leading-relaxed"> + <p className="text-sm text-muted-foreground leading-relaxed">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/components/inviteLinkToggle.tsx` around lines 69 - 122, The component uses CSS variable utility classes (e.g., border-[var(--border)], bg-[var(--card)], text-[var(--foreground)])—replace these with Tailwind semantic color classes across the InviteLinkToggle JSX (update className on the root div, headings, paragraphs, the Switch/Input/Button wrappers and dynamic className interpolation). Specifically, update occurrences in the render of Switch, Input, Button and container divs (references: enabled, isLoading, inviteLink, copied, handleToggle, handleCopy) to map your design tokens to Tailwind classes (for example map --border→border-gray-200, --card→bg-white or bg-gray-50, --foreground→text-gray-900, --muted-foreground→text-gray-500, --muted→bg-gray-100, --chart-2→text-green-500) and keep the conditional text-red-500 for error state; ensure the string template logic for the inviteLink Input and the transition div remain the same but use the new Tailwind class names.packages/web/src/ee/features/analytics/actions.ts (1)
174-179: Minor: Missing indentation consistency.The return block and closing braces have inconsistent indentation compared to the rest of the function body. The return statement at line 174 appears to be at the same level as the
const rowsassignment, but the closing braces suggest nested structure.♻️ Suggested formatting fix
- return { - rows, - retentionDays: env.SOURCEBOT_EE_AUDIT_RETENTION_DAYS, - oldestRecordDate: oldestRecord?.timestamp ?? null, - }; - })) -); + return { + rows, + retentionDays: env.SOURCEBOT_EE_AUDIT_RETENTION_DAYS, + oldestRecordDate: oldestRecord?.timestamp ?? null, + }; + }) + ) +);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/ee/features/analytics/actions.ts` around lines 174 - 179, The return block's indentation is inconsistent with the surrounding function body: align the return statement and its object properties (rows, retentionDays, oldestRecordDate) to the same indentation level as the preceding const rows assignment and ensure the closing braces line up with the opening ones; adjust the indentation for env.SOURCEBOT_EE_AUDIT_RETENTION_DAYS and oldestRecord?.timestamp ?? null so the entire returned object and the final closing parentheses/braces match the function's nesting (look for the return that returns { rows, retentionDays: env.SOURCEBOT_EE_AUDIT_RETENTION_DAYS, oldestRecordDate: oldestRecord?.timestamp ?? null } and reformat it to the same indent level as the surrounding code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/web/src/app/invite/actions.ts`:
- Around line 11-17: The joinOrganization function is wrapped with withAuthV2
which pre-rejects OrgRole.GUEST and prevents invite-link joins for logged-in
non-members; change the wrapper to an auth-only helper (so authentication is
enforced but guest role is not rejected) and perform the membership transition
inside joinOrganization. Concretely, replace the withAuthV2(...) call used in
joinOrganization with the equivalent auth-only wrapper (or adjust the withAuthV2
invocation so it does not deny OrgRole.GUEST), keep access to { user, prisma }
inside the callback, and ensure the function itself checks and upgrades
membership (handling inviteLinkId) rather than relying on the wrapper to permit
membership changes. Ensure references to joinOrganization and any OrgRole.GUEST
checks are updated accordingly.
---
Outside diff comments:
In `@packages/web/src/actions.ts`:
- Around line 1193-1214: The plain-text email uses an undefined variable origin
causing a runtime error after addUserToOrganization; update the
transport.sendMail call in the approval-email path to use env.AUTH_URL (or
env.AUTH_URL + `/${org.domain}`) instead of origin in the text body, and remove
any accidental references to origin in this scope (look for
render/JoinRequestApprovedEmail, createTransport, and transport.sendMail usages
to locate the code).
- Around line 836-845: The redeemInvite and getInviteInfo handlers are using
withAuthV2 which rejects users with role === OrgRole.GUEST, blocking invite
redemption for invited-but-not-yet-members; replace their use of withAuthV2 with
an auth-only wrapper that requires a logged-in user but does not enforce org
membership (either a new withAuthUser/withAuthOnly helper or a withAuthV2 option
e.g. { allowGuests: true }), and update redeemInvite (and the other handler at
the 910-918 range) to call that wrapper so the route can read/consume the invite
while the user is still a guest; keep the inner logic (user, prisma) the same
and ensure the new wrapper still provides the same context shape used by
redeemInvite/getInviteInfo.
In `@packages/web/src/app/`[domain]/repos/layout.tsx:
- Around line 27-32: The call to getCurrentUserRole() may return a ServiceError
but userRoleInOrg is used directly in the OrgRole.OWNER comparison, so add
explicit error handling after calling getCurrentUserRole(): check whether
userRoleInOrg is a ServiceError (use your isServiceError helper or type guard),
then handle it (e.g., log the error with context, return an error UI or rethrow)
instead of silently letting the comparison proceed; only proceed to compare to
OrgRole.OWNER when userRoleInOrg is a valid role value. Ensure you update the
flow in layout.tsx around the getCurrentUserRole() call and the conditional that
references userRoleInOrg.
---
Nitpick comments:
In `@packages/web/src/app/components/anonymousAccessToggle.tsx`:
- Around line 53-67: The JSX uses arbitrary CSS variable syntax in className
(e.g., "border-[var(--border)]", "bg-[var(--card)]", "text-[var(--foreground)]",
"text-[var(--muted-foreground)]", "bg-[var(--muted)]") inside the
AnonymousAccessToggle component; replace those with the project's Tailwind
semantic color classes (e.g., border-border, bg-card, text-foreground,
text-muted-foreground, bg-muted) wherever they appear — specifically update the
outer container div, the h3, the paragraph tags, the showPlanMessage wrapper
div, and the svg className so all instances use the semantic classes instead of
the var(...) syntax while preserving the existing conditional class logic (the
same className strings like the opacity toggle using
hasAnonymousAccessEntitlement and forceEnableAnonymousAccess should remain).
- Around line 94-115: In the showForceEnableMessage block of
anonymousAccessToggle.tsx, replace the inline Tailwind arbitrary CSS variable
usages with the project's standardized utility classes: update
"bg-[var(--muted)]", "border-[var(--border)]", "text-[var(--foreground)]",
"text-[var(--muted-foreground)]", and "bg-[var(--secondary)]" to the equivalent
design-system tokens (e.g., bg-muted, border-border, text-foreground,
text-muted-foreground, bg-secondary) so the component uses the consistent CSS
variable/class naming convention; adjust the className strings in the JSX inside
the conditional render tied to showForceEnableMessage accordingly.
In `@packages/web/src/app/components/inviteLinkToggle.tsx`:
- Around line 69-122: The component uses CSS variable utility classes (e.g.,
border-[var(--border)], bg-[var(--card)], text-[var(--foreground)])—replace
these with Tailwind semantic color classes across the InviteLinkToggle JSX
(update className on the root div, headings, paragraphs, the Switch/Input/Button
wrappers and dynamic className interpolation). Specifically, update occurrences
in the render of Switch, Input, Button and container divs (references: enabled,
isLoading, inviteLink, copied, handleToggle, handleCopy) to map your design
tokens to Tailwind classes (for example map --border→border-gray-200,
--card→bg-white or bg-gray-50, --foreground→text-gray-900,
--muted-foreground→text-gray-500, --muted→bg-gray-100, --chart-2→text-green-500)
and keep the conditional text-red-500 for error state; ensure the string
template logic for the inviteLink Input and the transition div remain the same
but use the new Tailwind class names.
In `@packages/web/src/ee/features/analytics/actions.ts`:
- Around line 174-179: The return block's indentation is inconsistent with the
surrounding function body: align the return statement and its object properties
(rows, retentionDays, oldestRecordDate) to the same indentation level as the
preceding const rows assignment and ensure the closing braces line up with the
opening ones; adjust the indentation for env.SOURCEBOT_EE_AUDIT_RETENTION_DAYS
and oldestRecord?.timestamp ?? null so the entire returned object and the final
closing parentheses/braces match the function's nesting (look for the return
that returns { rows, retentionDays: env.SOURCEBOT_EE_AUDIT_RETENTION_DAYS,
oldestRecordDate: oldestRecord?.timestamp ?? null } and reformat it to the same
indent level as the surrounding code).
🪄 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: CHILL
Plan: Pro
Run ID: f5c5168b-5266-48ea-82bb-10deb8a9688d
📒 Files selected for processing (22)
packages/web/src/actions.tspackages/web/src/app/[domain]/chat/[id]/page.tsxpackages/web/src/app/[domain]/chat/page.tsxpackages/web/src/app/[domain]/components/navigationMenu/index.tsxpackages/web/src/app/[domain]/components/searchBar/useSuggestionsData.tspackages/web/src/app/[domain]/repos/[id]/page.tsxpackages/web/src/app/[domain]/repos/layout.tsxpackages/web/src/app/[domain]/settings/apiKeys/apiKeysPage.tsxpackages/web/src/app/[domain]/settings/apiKeys/columns.tsxpackages/web/src/app/[domain]/settings/layout.tsxpackages/web/src/app/[domain]/settings/license/page.tsxpackages/web/src/app/[domain]/settings/members/components/inviteMemberCard.tsxpackages/web/src/app/[domain]/settings/members/components/invitesList.tsxpackages/web/src/app/[domain]/settings/members/components/requestsList.tsxpackages/web/src/app/[domain]/settings/members/page.tsxpackages/web/src/app/components/anonymousAccessToggle.tsxpackages/web/src/app/components/inviteLinkToggle.tsxpackages/web/src/app/components/memberApprovalRequiredToggle.tsxpackages/web/src/app/invite/actions.tspackages/web/src/app/onboard/components/completeOnboardingButton.tsxpackages/web/src/ee/features/analytics/actions.tspackages/web/src/ee/features/analytics/analyticsContent.tsx
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/web/src/actions.ts (2)
689-696:⚠️ Potential issue | 🟠 MajorDon't let invite-email failures turn a committed create into an error.
createMany()commits before thePromise.all()email fan-out. Ifrender()orsendMail()throws for one recipient, this action returns an unexpected error even though the invites already exist, and the next retry will hit the “pending invite already exists” path.Also applies to: 699-751
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/actions.ts` around lines 689 - 696, The invite creation commits via prisma.invite.createMany before the email fan-out, so if render() or sendMail() throws inside the Promise.all the whole action returns an error even though invites exist; update the flow around prisma.invite.createMany and the subsequent emails.map(...). Use createMany as the primary success path, then perform the email fan-out but ensure email send failures do not bubble up to fail the whole action: wrap the Promise.all of render()/sendMail() in a try/catch (or handle per-recipient promises with .catch), log/send telemetry for individual failures, and return success based on the completed createMany; apply the same pattern to the other email-sending block (lines ~699-751) so invite persistence remains authoritative while email errors are non-fatal.
1194-1214:⚠️ Potential issue | 🔴 CriticalUse a server-safe base URL instead of
origin.
originis a Window/Worker API global that is not available in Node.js. This bare identifier will throw aReferenceErrorwhen the approval-email branch executes on the server. The critical issue is that this failure occurs afteraddUserToOrganization()has already succeeded, meaning the user is added to the organization at the database level but the action fails when attempting to send the email.This same issue is already documented in the file at line 1053 with a TODO comment, where
env.AUTH_URLis correctly used instead. Use the same approach here:Suggested fix
- const html = await render(JoinRequestApprovedEmail({ - baseUrl: env.AUTH_URL, + const baseUrl = env.AUTH_URL; + const joinUrl = new URL(`/${org.domain}`, `${baseUrl}/`).toString(); + + const html = await render(JoinRequestApprovedEmail({ + baseUrl, user: { name: request.requestedBy.name ?? undefined, email: request.requestedBy.email!, avatarUrl: request.requestedBy.image ?? undefined, }, @@ - text: `Your request to join ${org.name} on Sourcebot has been approved. You can now access the organization at ${origin}/${org.domain}`, + text: `Your request to join ${org.name} on Sourcebot has been approved. You can now access the organization at ${joinUrl}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/actions.ts` around lines 1194 - 1214, The email-sending branch uses the undefined global identifier origin in the sendMail text which will throw on the server; update the transport.sendMail payload to use the server-safe auth URL (env.AUTH_URL or the already-passed baseUrl) instead of origin. Locate the block building the JoinRequestApprovedEmail and the sendMail call (symbols: render(JoinRequestApprovedEmail({...})), transport.sendMail({...})) and replace any ${origin} usage with env.AUTH_URL (ensuring proper slashes) so the URL is built safely on the server.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/web/src/actions.ts`:
- Around line 689-696: The invite creation commits via prisma.invite.createMany
before the email fan-out, so if render() or sendMail() throws inside the
Promise.all the whole action returns an error even though invites exist; update
the flow around prisma.invite.createMany and the subsequent emails.map(...). Use
createMany as the primary success path, then perform the email fan-out but
ensure email send failures do not bubble up to fail the whole action: wrap the
Promise.all of render()/sendMail() in a try/catch (or handle per-recipient
promises with .catch), log/send telemetry for individual failures, and return
success based on the completed createMany; apply the same pattern to the other
email-sending block (lines ~699-751) so invite persistence remains authoritative
while email errors are non-fatal.
- Around line 1194-1214: The email-sending branch uses the undefined global
identifier origin in the sendMail text which will throw on the server; update
the transport.sendMail payload to use the server-safe auth URL (env.AUTH_URL or
the already-passed baseUrl) instead of origin. Locate the block building the
JoinRequestApprovedEmail and the sendMail call (symbols:
render(JoinRequestApprovedEmail({...})), transport.sendMail({...})) and replace
any ${origin} usage with env.AUTH_URL (ensuring proper slashes) so the URL is
built safely on the server.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e79cfdc2-24ea-4a38-94e1-902329938b9b
📒 Files selected for processing (3)
packages/web/src/actions.tspackages/web/src/app/invite/actions.tspackages/web/src/withAuthV2.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/web/src/app/invite/actions.ts
…bershipCheck - Replace undefined `origin` global with `env.AUTH_URL` in the join request approval email plaintext body. - Add `withAuthV2_skipMembershipCheck` wrapper for actions where the user may not yet be an org member (joinOrganization, redeemInvite, getInviteInfo). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 2, 2026
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
withAuthandwithOrgMembershiphelpers and migrates all 18 server actions across 3 files to usewithAuthV2,withOptionalAuthV2, andwithMinimumOrgRoleprismaclient from the auth context instead of the raw import, and no longer require adomainparameter (org is resolved server-side)domainarguments and related importsTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit