address comments

Author: icecrasher321

apps/sim/app/api/organizations/[id]/invitations/route.ts

@@ -502,7 +502,7 @@ export const POST = withRouteHandler(
         let grantedAny = false
         for (const grant of memberInvite.grants) {
           try {
-            await grantWorkspaceAccessDirectly({
+            const grantResult = await grantWorkspaceAccessDirectly({
               userId: memberUserId,
               email: memberInvite.email,
               workspaceId: grant.workspaceId,
@@ -514,7 +514,8 @@ export const POST = withRouteHandler(
               actorEmail: session.user.email,
               request,
             })
-            grantedAny = true
+
+            if (grantResult.outcome === 'added') grantedAny = true
           } catch (grantError) {
             logger.error('Failed to grant workspace access directly', {
               email: memberInvite.email,

apps/sim/app/api/workspaces/invitations/batch/route.ts

@@ -62,7 +62,6 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
 
     const successful: string[] = []
     const added: string[] = []
-    const upgraded: string[] = []
     const failed: BatchInvitationFailure[] = []
     const invitations: WorkspaceInvitationResult[] = []
     const seenEmails = new Set<string>()
@@ -86,8 +85,9 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
           request: req,
         })
         if (invitation.instantAdd) {
-          added.push(invitation.email)
-          if (invitation.outcome === 'upgraded') upgraded.push(invitation.email)
+          // Only report an actual insertion; an `unchanged` outcome means the
+          // user already had access (rare race) and is a silent no-op.
+          if (invitation.outcome === 'added') added.push(invitation.email)
         } else {
           successful.push(invitation.email)
         }
@@ -110,7 +110,6 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
       success: failed.length === 0,
       successful,
       added,
-      upgraded,
       failed,
       invitations,
     })

apps/sim/hooks/queries/invitations.ts

@@ -72,7 +72,6 @@ type BatchSendInvitationsParams = ContractBodyInput<typeof batchWorkspaceInvitat
 
 type BatchInvitationResult = Pick<BatchInvitationResultContract, 'successful' | 'failed'> & {
   added: string[]
-  upgraded: string[]
 }
 
 /**
@@ -99,7 +98,6 @@ export function useBatchSendWorkspaceInvitations() {
       return {
         successful: result.successful ?? [],
         added: result.added ?? [],
-        upgraded: result.upgraded ?? [],
         failed: result.failed ?? [],
       }
     },

apps/sim/lib/api/contracts/invitations.ts

@@ -54,7 +54,6 @@ export const batchInvitationResultSchema = z
     success: z.boolean(),
     successful: z.array(z.string()),
     added: z.array(z.string()).optional(),
-    upgraded: z.array(z.string()).optional(),
     failed: z.array(z.object({ email: z.string(), error: z.string() })),
     invitations: z.array(z.record(z.string(), z.unknown())),
   })

apps/sim/lib/core/telemetry.ts

@@ -563,14 +563,12 @@ export const PlatformEvents = {
     addedBy: string
     addedUserId: string
     role: string
-    outcome: 'added' | 'upgraded'
   }) => {
     trackPlatformEvent('platform.workspace.member_added', {
       'workspace.id': attrs.workspaceId,
       'user.id': attrs.addedBy,
       'member.id': attrs.addedUserId,
       'member.role': attrs.role,
-      'member.add_outcome': attrs.outcome,
     })
   },
 

apps/sim/lib/invitations/direct-grant.test.ts

@@ -41,10 +41,6 @@ vi.mock('@/lib/credentials/environment', () => ({
   syncWorkspaceEnvCredentials: mockSyncWorkspaceEnvCredentials,
 }))
 
-vi.mock('@/lib/invitations/core', () => ({
-  PERMISSION_RANK: { read: 0, write: 1, admin: 2 },
-}))
-
 vi.mock('@/lib/invitations/send', () => ({
   cancelPendingInvitation: mockCancelPendingInvitation,
   sendWorkspaceAddedEmail: mockSendWorkspaceAddedEmail,
@@ -95,6 +91,8 @@ describe('grantWorkspaceAccessDirectly', () => {
     vi.clearAllMocks()
     resetDbChainMock()
     mockSendWorkspaceAddedEmail.mockResolvedValue({ success: true })
+    // Insert path reports the new row via `.returning()`.
+    dbChainMockFns.returning.mockResolvedValue([{ id: 'perm-new' }])
   })
 
   it('inserts a permission row when the user has no existing access', async () => {
@@ -107,25 +105,38 @@ describe('grantWorkspaceAccessDirectly', () => {
       expect.objectContaining({ action: 'member.added', resourceId: 'ws-1' })
     )
     expect(mockWorkspaceMemberAdded).toHaveBeenCalledWith(
-      expect.objectContaining({ workspaceId: 'ws-1', outcome: 'added' })
+      expect.objectContaining({ workspaceId: 'ws-1' })
     )
     expect(mockSendWorkspaceAddedEmail).toHaveBeenCalledWith(
       expect.objectContaining({ email: 'member@example.com', workspaceId: 'ws-1' })
     )
   })
 
-  it('upgrades an existing lower permission', async () => {
+  it('reports unchanged (no audit/email) when a concurrent insert wins the race', async () => {
+    dbChainMockFns.returning.mockResolvedValueOnce([])
+
+    const result = await grantWorkspaceAccessDirectly({ ...baseInput })
+
+    expect(result).toEqual({ outcome: 'unchanged', permission: 'write' })
+    expect(dbChainMockFns.insert).toHaveBeenCalled()
+    expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
+    expect(mockWorkspaceMemberAdded).not.toHaveBeenCalled()
+    expect(mockSendWorkspaceAddedEmail).not.toHaveBeenCalled()
+  })
+
+  it('does not upgrade an existing lower permission (invites never modify access)', async () => {
     dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'perm-1', permissionType: 'read' }])
 
     const result = await grantWorkspaceAccessDirectly({ ...baseInput, permission: 'admin' })
 
-    expect(result).toEqual({ outcome: 'upgraded', from: 'read', to: 'admin' })
-    expect(dbChainMockFns.update).toHaveBeenCalled()
+    expect(result).toEqual({ outcome: 'unchanged', permission: 'read' })
+    expect(dbChainMockFns.update).not.toHaveBeenCalled()
     expect(dbChainMockFns.insert).not.toHaveBeenCalled()
-    expect(mockSendWorkspaceAddedEmail).toHaveBeenCalled()
+    expect(auditMockFns.mockRecordAudit).not.toHaveBeenCalled()
+    expect(mockSendWorkspaceAddedEmail).not.toHaveBeenCalled()
   })
 
-  it('no-ops when the user already has equal or higher access', async () => {
+  it('no-ops when the user already has access', async () => {
     dbChainMockFns.limit.mockResolvedValueOnce([{ id: 'perm-1', permissionType: 'admin' }])
 
     const result = await grantWorkspaceAccessDirectly({ ...baseInput, permission: 'write' })

apps/sim/lib/invitations/direct-grant.ts

@@ -14,7 +14,6 @@ import type { NextRequest } from 'next/server'
 import { getUserOrganization } from '@/lib/billing/organizations/membership'
 import { PlatformEvents } from '@/lib/core/telemetry'
 import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment'
-import { PERMISSION_RANK, type PermissionLevel } from '@/lib/invitations/core'
 import { cancelPendingInvitation, sendWorkspaceAddedEmail } from '@/lib/invitations/send'
 import { captureServerEvent } from '@/lib/posthog/server'
 import type { PermissionType } from '@/lib/workspaces/permissions/utils'
@@ -23,7 +22,6 @@ const logger = createLogger('InvitationDirectGrant')
 
 export type DirectGrantOutcome =
   | { outcome: 'added'; permission: PermissionType }
-  | { outcome: 'upgraded'; from: PermissionType; to: PermissionType }
   | { outcome: 'unchanged'; permission: PermissionType }
 
 export interface GrantWorkspaceAccessDirectlyInput {
@@ -89,15 +87,14 @@ async function supersedePendingWorkspaceInvites(
 /**
  * Grants a user workspace access immediately, without an invitation or
  * acceptance step. Intended for users who already belong to the workspace's
- * organization. Idempotent: no-ops when the user already has equal or higher
- * access, upgrades when the new permission is higher.
+ * organization and are not yet members of the workspace. Idempotent: when a
+ * permission already exists it is left untouched (no-op) — invites never modify
+ * or upgrade an existing member's permission.
  */
 export async function grantWorkspaceAccessDirectly(
   input: GrantWorkspaceAccessDirectlyInput
 ): Promise<DirectGrantOutcome> {
   const normalizedEmail = normalizeEmail(input.email)
-  const newPermission = input.permission as PermissionLevel
-  const newRank = PERMISSION_RANK[newPermission] ?? 0
 
   const result = await db.transaction(async (tx): Promise<DirectGrantOutcome> => {
     const [existing] = await tx
@@ -112,33 +109,29 @@ export async function grantWorkspaceAccessDirectly(
       )
       .limit(1)
 
-    if (!existing) {
-      await tx
-        .insert(permissions)
-        .values({
-          id: generateId(),
-          entityType: 'workspace',
-          entityId: input.workspaceId,
-          userId: input.userId,
-          permissionType: newPermission,
-          createdAt: new Date(),
-          updatedAt: new Date(),
-        })
-        .onConflictDoNothing()
-      return { outcome: 'added', permission: input.permission }
+    if (existing) {
+      return { outcome: 'unchanged', permission: existing.permissionType as PermissionType }
     }
 
-    const existingPermission = existing.permissionType as PermissionType
-    const existingRank = PERMISSION_RANK[existingPermission as PermissionLevel] ?? 0
-    if (newRank > existingRank) {
-      await tx
-        .update(permissions)
-        .set({ permissionType: newPermission, updatedAt: new Date() })
-        .where(eq(permissions.id, existing.id))
-      return { outcome: 'upgraded', from: existingPermission, to: input.permission }
+    const inserted = await tx
+      .insert(permissions)
+      .values({
+        id: generateId(),
+        entityType: 'workspace',
+        entityId: input.workspaceId,
+        userId: input.userId,
+        permissionType: input.permission,
+        createdAt: new Date(),
+        updatedAt: new Date(),
+      })
+      .onConflictDoNothing()
+      .returning({ id: permissions.id })
+
+    if (inserted.length === 0) {
+      return { outcome: 'unchanged', permission: input.permission }
     }
 
-    return { outcome: 'unchanged', permission: existingPermission }
+    return { outcome: 'added', permission: input.permission }
   })
 
   if (result.outcome === 'unchanged') {
@@ -182,7 +175,6 @@ export async function grantWorkspaceAccessDirectly(
       addedBy: input.actorId,
       addedUserId: input.userId,
       role: input.permission,
-      outcome: result.outcome,
     })
   } catch {
     /**
@@ -196,7 +188,6 @@ export async function grantWorkspaceAccessDirectly(
     {
       workspace_id: input.workspaceId,
       member_role: input.permission,
-      outcome: result.outcome,
     },
     {
       groups: { workspace: input.workspaceId },
@@ -212,17 +203,13 @@ export async function grantWorkspaceAccessDirectly(
     resourceType: AuditResourceType.WORKSPACE,
     resourceId: input.workspaceId,
     resourceName: normalizedEmail,
-    description:
-      result.outcome === 'upgraded'
-        ? `Added existing organization member ${normalizedEmail} (upgraded to ${input.permission})`
-        : `Added existing organization member ${normalizedEmail} as ${input.permission}`,
+    description: `Added existing organization member ${normalizedEmail} as ${input.permission}`,
     metadata: {
       targetEmail: normalizedEmail,
       targetRole: input.permission,
       organizationId: input.organizationId,
       workspaceName: input.workspaceName,
       addedUserId: input.userId,
-      outcome: result.outcome,
     },
     request: input.request,
   })

apps/sim/lib/invitations/send.ts

@@ -22,6 +22,7 @@ import { getBaseUrl } from '@/lib/core/utils/urls'
 import { computeInvitationExpiry } from '@/lib/invitations/core'
 import { sendEmail } from '@/lib/messaging/email/mailer'
 import { getFromEmailAddress } from '@/lib/messaging/email/utils'
+import { getBrandConfig } from '@/ee/whitelabeling'
 
 const logger = createLogger('InvitationSend')
 
@@ -206,10 +207,11 @@ export async function sendInvitationEmail(
       inviteUrl
     )
 
+    const brandName = getBrandConfig().name
     const subject =
       workspaceNames.length === 1
-        ? `You've been invited to join "${workspaceNames[0]}" on Sim`
-        : `You've been invited to join ${workspaceNames.length} workspaces on Sim`
+        ? `You've been invited to join "${workspaceNames[0]}" on ${brandName}`
+        : `You've been invited to join ${workspaceNames.length} workspaces on ${brandName}`
 
     const result = await sendEmail({
       to: input.email,
@@ -306,7 +308,7 @@ export async function sendWorkspaceAddedEmail(
 
   const result = await sendEmail({
     to: input.email,
-    subject: `You've been added to "${input.workspaceName}" on Sim`,
+    subject: getEmailSubject('workspace-added'),
     html: emailHtml,
     from: getFromEmailAddress(),
     emailType: 'transactional',

apps/sim/lib/invitations/workspace-invitations.test.ts

@@ -130,7 +130,7 @@ describe('createWorkspaceInvitation', () => {
   })
 
   it('directly grants access to an existing member of the workspace organization', async () => {
-    queueWhereResponses([[{ id: 'user-2', email: 'member@example.com' }]])
+    queueWhereResponses([[{ id: 'user-2', email: 'member@example.com' }], []])
     mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', role: 'member' })
 
     const result = await createWorkspaceInvitation({
@@ -154,6 +154,26 @@ describe('createWorkspaceInvitation', () => {
     expect(mockSendInvitationEmail).not.toHaveBeenCalled()
   })
 
+  it('rejects an existing workspace member without upgrading their permission', async () => {
+    queueWhereResponses([
+      [{ id: 'user-2', email: 'member@example.com' }],
+      [{ id: 'perm-1', permissionType: 'read' }],
+    ])
+    mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', role: 'member' })
+
+    await expect(
+      createWorkspaceInvitation({
+        context: makeContext(),
+        email: 'member@example.com',
+        permission: 'admin',
+        request,
+      })
+    ).rejects.toThrow('already has access')
+
+    expect(mockGrantWorkspaceAccessDirectly).not.toHaveBeenCalled()
+    expect(mockCreatePendingInvitation).not.toHaveBeenCalled()
+  })
+
   it('creates an external pending invitation when the user belongs to a different org', async () => {
     queueWhereResponses([[{ id: 'user-3', email: 'ext@example.com' }], []])
     mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-2', role: 'member' })