From e6b7b92e73e2203ea64a0bbd58b2da7eefdff640 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 30 Jul 2026 16:46:11 -0700 Subject: [PATCH 1/4] improvement(admin): uupdate defaults for better UX --- .../dashboard/workspaces/[id]/move/route.ts | 1 + .../lib/admin/dashboard-credit-grant.test.ts | 83 ++++++++- apps/sim/lib/admin/dashboard.ts | 49 ++--- .../v1/admin/dashboard-workspaces.ts | 5 +- .../lib/billing/validation/seat-management.ts | 2 +- apps/sim/lib/invitations/send.ts | 2 +- apps/sim/lib/workspaces/admin-move.test.ts | 131 +++++++++++-- apps/sim/lib/workspaces/admin-move.ts | 176 +++++++++++++----- .../organization-workspaces.test.ts | 10 + .../lib/workspaces/organization-workspaces.ts | 18 +- 10 files changed, 367 insertions(+), 110 deletions(-) diff --git a/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts b/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts index 8199fb3d78f..2b9f9d284d3 100644 --- a/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts +++ b/apps/sim/app/api/v1/admin/dashboard/workspaces/[id]/move/route.ts @@ -27,6 +27,7 @@ export const POST = withRouteHandler( const data = await moveWorkspaceToOrganization({ workspaceId: parsed.data.params.id, destinationOrganizationId: parsed.data.body.destinationOrganizationId, + expectedOwnerId: parsed.data.body.expectedOwnerId, adminEmail: request.headers.get('x-admin-email') ?? 'admin-api@sim.ai', }) return NextResponse.json({ data }) diff --git a/apps/sim/lib/admin/dashboard-credit-grant.test.ts b/apps/sim/lib/admin/dashboard-credit-grant.test.ts index d4a189418c8..614b952352e 100644 --- a/apps/sim/lib/admin/dashboard-credit-grant.test.ts +++ b/apps/sim/lib/admin/dashboard-credit-grant.test.ts @@ -57,6 +57,9 @@ vi.mock('@/lib/billing/organizations/seats', () => ({ vi.mock('@/lib/billing/core/usage', () => ({ syncUsageLimitsFromSubscription: mocks.syncUsageLimits, })) +vi.mock('@/lib/workspaces/organization-workspaces', () => ({ + ownedAttachableWorkspacesWhere: vi.fn(() => undefined), +})) vi.mock('@/lib/workspaces/admin-move', () => ({ moveWorkspaceToOrganization: mocks.moveWorkspace, })) @@ -211,6 +214,9 @@ describe('addDashboardOrganizationMember', () => { resetDbChainMock() mocks.billingSubscriptions = [] mocks.idempotencyCalls = [] + mocks.ensureMembership.mockReset() + mocks.transferMembership.mockReset() + mocks.moveWorkspace.mockReset() }) it('rejects an existing member inside the transaction before touching their cap', async () => { @@ -239,7 +245,50 @@ describe('addDashboardOrganizationMember', () => { expect(mocks.recordAudit).not.toHaveBeenCalled() }) - it('uses the canonical transfer service and reports each selected personal workspace move', async () => { + it('moves every selected workspace through the invitation-aware service after adding a member', async () => { + queueTableRows(workspace, [{ id: 'workspace-1' }, { id: 'workspace-2' }]) + queueTableRows(subscription, [{ plan: 'enterprise' }]) + mocks.ensureMembership.mockResolvedValue({ + success: true, + memberId: 'member-new', + alreadyMember: false, + billingActions: { proUsageSnapshotted: false, proCancelledAtPeriodEnd: false }, + }) + mocks.moveWorkspace.mockResolvedValue({}) + + const result = await addDashboardOrganizationMember( + 'org-1', + { + userId: 'user-1', + role: 'member', + personalWorkspaceIds: ['workspace-1', 'workspace-2'], + }, + { id: 'admin-1', name: 'Admin', email: 'admin@sim.ai' } + ) + + expect(mocks.moveWorkspace).toHaveBeenNthCalledWith(1, { + workspaceId: 'workspace-1', + destinationOrganizationId: 'org-1', + adminEmail: 'admin@sim.ai', + expectedOwnerId: 'user-1', + }) + expect(mocks.moveWorkspace).toHaveBeenNthCalledWith(2, { + workspaceId: 'workspace-2', + destinationOrganizationId: 'org-1', + adminEmail: 'admin@sim.ai', + expectedOwnerId: 'user-1', + }) + expect(result).toEqual({ + memberId: 'member-new', + transferredFromOrganizationId: null, + workspaceMoves: [ + { workspaceId: 'workspace-1', success: true }, + { workspaceId: 'workspace-2', success: true }, + ], + }) + }) + + it('uses the canonical transfer service and reports each selected workspace move', async () => { queueTableRows(workspace, [{ id: 'workspace-1' }, { id: 'workspace-2' }]) queueTableRows(member, [{ id: 'member-old', organizationId: 'org-old' }]) mocks.transferMembership.mockResolvedValue({ @@ -289,4 +338,36 @@ describe('addDashboardOrganizationMember', () => { expect(mocks.reconcileSeats).toHaveBeenCalledTimes(2) expect(mocks.recordAudit).toHaveBeenCalledTimes(2) }) + + it('uses the expected owner guard for an administrator-selected subset', async () => { + queueTableRows(workspace, [{ id: 'workspace-1' }, { id: 'workspace-2' }]) + queueTableRows(member, [{ id: 'member-old', organizationId: 'org-old' }]) + mocks.transferMembership.mockResolvedValue({ + success: true, + memberId: 'member-new', + workspaceAccessRevoked: 0, + credentialMembershipsRevoked: 0, + pendingInvitationsCancelled: 0, + usageCaptured: 0, + }) + mocks.moveWorkspace.mockResolvedValue({}) + + const result = await addDashboardOrganizationMember( + 'org-new', + { + userId: 'user-1', + role: 'member', + personalWorkspaceIds: ['workspace-1'], + }, + { id: 'admin-1', name: 'Admin', email: 'admin@sim.ai' } + ) + + expect(mocks.moveWorkspace).toHaveBeenCalledWith({ + workspaceId: 'workspace-1', + destinationOrganizationId: 'org-new', + adminEmail: 'admin@sim.ai', + expectedOwnerId: 'user-1', + }) + expect(result.workspaceMoves).toEqual([{ workspaceId: 'workspace-1', success: true }]) + }) }) diff --git a/apps/sim/lib/admin/dashboard.ts b/apps/sim/lib/admin/dashboard.ts index bbb983d567e..3cba4f5db28 100644 --- a/apps/sim/lib/admin/dashboard.ts +++ b/apps/sim/lib/admin/dashboard.ts @@ -12,19 +12,7 @@ import { } from '@sim/db/schema' import { getErrorMessage } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' -import { - and, - count, - countDistinct, - desc, - eq, - ilike, - inArray, - isNull, - ne, - or, - sql, -} from 'drizzle-orm' +import { and, count, countDistinct, desc, eq, ilike, inArray, isNull, or, sql } from 'drizzle-orm' import { getOrganizationUsageLimitFallbackDollars, getTeamOrganizationEconomics, @@ -64,6 +52,7 @@ import { executeTransactionallyIdempotent } from '@/lib/core/idempotency/transac import { enqueueOutboxEvent } from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' import { moveWorkspaceToOrganization } from '@/lib/workspaces/admin-move' +import { ownedAttachableWorkspacesWhere } from '@/lib/workspaces/organization-workspaces' interface PaginationInput { search: string @@ -317,7 +306,7 @@ async function getDashboardOrganizationSummary(organizationId: string) { member, and(eq(member.userId, permissions.userId), eq(member.organizationId, organizationId)) ) - .where(and(isNull(member.id), isNull(workspace.archivedAt))), + .where(isNull(member.id)), getLatestSubscription(organizationId), getLatestEnterpriseProvisionings([organizationId]), ]) @@ -398,13 +387,7 @@ export async function listDashboardOrganizations({ search, limit, offset }: Pagi eq(member.organizationId, workspace.organizationId) ) ) - .where( - and( - inArray(workspace.organizationId, organizationIds), - isNull(member.id), - isNull(workspace.archivedAt) - ) - ) + .where(and(inArray(workspace.organizationId, organizationIds), isNull(member.id))) .groupBy(workspace.organizationId), db .selectDistinctOn([subscription.referenceId]) @@ -496,7 +479,7 @@ export async function getDashboardOrganization(organizationId: string) { member, and(eq(member.userId, permissions.userId), eq(member.organizationId, organizationId)) ) - .where(and(isNull(member.id), isNull(workspace.archivedAt))) + .where(isNull(member.id)) .groupBy(user.id, user.name, user.email) .orderBy(user.name), db @@ -939,7 +922,7 @@ export async function getDashboardMemberTransferPreflight( db .select({ id: workspace.id, name: workspace.name, archivedAt: workspace.archivedAt }) .from(workspace) - .where(and(eq(workspace.ownerId, userId), ne(workspace.workspaceMode, 'organization'))) + .where(ownedAttachableWorkspacesWhere({ userId, includeArchived: true })) .orderBy(workspace.name, workspace.id), ]) if (!destination) throw new Error('Destination organization not found') @@ -985,22 +968,15 @@ export async function addDashboardOrganizationMember( actor: AdminMutationActor ) { const selectedWorkspaceIds = [...new Set(values.personalWorkspaceIds ?? [])] - if (selectedWorkspaceIds.length > 0) { - const selectable = await db + const attachableWorkspaceIds = ( + await db .select({ id: workspace.id }) .from(workspace) - .where( - and( - inArray(workspace.id, selectedWorkspaceIds), - eq(workspace.ownerId, values.userId), - ne(workspace.workspaceMode, 'organization') - ) - ) - if (selectable.length !== selectedWorkspaceIds.length) { - throw new Error('One or more selected personal workspaces can no longer be moved') - } + .where(ownedAttachableWorkspacesWhere({ userId: values.userId, includeArchived: true })) + ).map((row) => row.id) + if (selectedWorkspaceIds.some((id) => !attachableWorkspaceIds.includes(id))) { + throw new Error('One or more selected personal workspaces can no longer be moved') } - const [existingMembership] = await db .select({ id: member.id, organizationId: member.organizationId }) .from(member) @@ -1095,6 +1071,7 @@ export async function addDashboardOrganizationMember( workspaceId, destinationOrganizationId: organizationId, adminEmail: actor.email ?? 'admin-api', + expectedOwnerId: values.userId, }) workspaceMoves.push({ workspaceId, success: true }) } catch (error) { diff --git a/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts b/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts index 4818e11688a..552daafc1e4 100644 --- a/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts +++ b/apps/sim/lib/api/contracts/v1/admin/dashboard-workspaces.ts @@ -24,6 +24,7 @@ export const adminDashboardWorkspacePreflightQuerySchema = z.object({ export const adminDashboardWorkspaceMoveBodySchema = z.object({ destinationOrganizationId: z.string().min(1).max(200), + expectedOwnerId: z.string().min(1).max(200).optional(), }) const adminDashboardWorkspaceCandidateSchema = z.object({ @@ -78,9 +79,7 @@ const adminDashboardWorkspacePreflightResponseSchema = z.object({ }) const adminDashboardWorkspaceMoveResponseSchema = z.object({ - data: adminDashboardWorkspacePreflightSchema.extend({ - invitationEmailFailures: z.array(z.string()), - }), + data: adminDashboardWorkspacePreflightSchema, }) export const adminDashboardWorkspaceSearchContract = defineRouteContract({ diff --git a/apps/sim/lib/billing/validation/seat-management.ts b/apps/sim/lib/billing/validation/seat-management.ts index 9bc0c17713d..efea6edc333 100644 --- a/apps/sim/lib/billing/validation/seat-management.ts +++ b/apps/sim/lib/billing/validation/seat-management.ts @@ -67,7 +67,7 @@ export async function countPendingSeatInvitations( * Resolves the organization's seat capacity, honouring an Enterprise seat * change that is still in flight to Stripe. */ -async function resolveSeatCapacity( +export async function resolveSeatCapacity( organizationSubscription: { id: string; plan: string; metadata?: unknown } & Record< string, unknown diff --git a/apps/sim/lib/invitations/send.ts b/apps/sim/lib/invitations/send.ts index 661cc968be5..894ce1943d3 100644 --- a/apps/sim/lib/invitations/send.ts +++ b/apps/sim/lib/invitations/send.ts @@ -69,7 +69,7 @@ export interface CreatePendingInvitationResult { * person per organization, which is what makes coalescing mandatory rather * than optional. */ -const PENDING_INVITATION_UNIQUE_INDEX = 'invitation_pending_email_org_unique' +export const PENDING_INVITATION_UNIQUE_INDEX = 'invitation_pending_email_org_unique' /** * Raised when the granted workspaces changed organization between the diff --git a/apps/sim/lib/workspaces/admin-move.test.ts b/apps/sim/lib/workspaces/admin-move.test.ts index a9572771c9e..a8ee7e18599 100644 --- a/apps/sim/lib/workspaces/admin-move.test.ts +++ b/apps/sim/lib/workspaces/admin-move.test.ts @@ -8,6 +8,8 @@ import type { WorkspaceMoveError } from '@/lib/workspaces/admin-move' import { buildPendingInvitationMergeScopeCondition, classifyWorkspaceMoveState, + invitationMigrationOutboxHandlers, + MIGRATED_INVITATION_EMAIL_EVENT_TYPE, moveWorkspaceToOrganization, } from '@/lib/workspaces/admin-move' import { WORKSPACE_MODE } from '@/lib/workspaces/policy' @@ -19,11 +21,19 @@ const { enqueueOutboxEvent, invalidateWorkspaceTableLimitsCache, changeWorkspaceStoragePayerInTx, + acquireInvitationMutationLocks, + getInvitationById, + isInvitationExpired, + sendInvitationEmail, } = vi.hoisted(() => ({ recordAudit: vi.fn(), enqueueOutboxEvent: vi.fn(), invalidateWorkspaceTableLimitsCache: vi.fn(), changeWorkspaceStoragePayerInTx: vi.fn(), + acquireInvitationMutationLocks: vi.fn(), + getInvitationById: vi.fn(), + isInvitationExpired: vi.fn(() => false), + sendInvitationEmail: vi.fn(), })) vi.mock('@sim/audit', () => ({ @@ -36,9 +46,15 @@ vi.mock('@/lib/billing/organizations/membership', () => ({ })) vi.mock('@/lib/billing/storage/payer-transfer', () => ({ changeWorkspaceStoragePayerInTx })) vi.mock('@/lib/core/outbox/service', () => ({ enqueueOutboxEvent })) -vi.mock('@/lib/invitations/core', () => ({ getInvitationById: vi.fn() })) -vi.mock('@/lib/invitations/locks', () => ({ acquireInvitationMutationLocks: vi.fn() })) -vi.mock('@/lib/invitations/send', () => ({ sendInvitationEmail: vi.fn() })) +vi.mock('@/lib/invitations/core', () => ({ + getInvitationById, + isInvitationExpired, +})) +vi.mock('@/lib/invitations/locks', () => ({ acquireInvitationMutationLocks })) +vi.mock('@/lib/invitations/send', () => ({ + PENDING_INVITATION_UNIQUE_INDEX: 'invitation_pending_email_org_unique', + sendInvitationEmail, +})) vi.mock('@/lib/table/billing', () => ({ invalidateWorkspaceTableLimitsCache })) const movedWorkspace = { @@ -71,13 +87,12 @@ const destination = { } /** - * The move flow reads the workspace three times in order — the pre-lock - * `FOR UPDATE` select (rows ignored), the classification row, and the final - * summary reload — so the workspace queue gets one set per read. All - * invitation/grant/permission selects resolve the queue-less empty default. + * The move flow reads the workspace twice in order — the locked classification + * row and the final summary reload — so the workspace queue gets one set per + * read. All invitation/grant/permission selects resolve the queue-less empty + * default. */ function queueMoveSelects(workspaceRow: Record) { - queueTableRows(workspace, [workspaceRow]) queueTableRows(workspace, [workspaceRow]) queueTableRows(workspace, [workspaceRow]) queueTableRows(organization, [destination]) @@ -127,6 +142,23 @@ describe('classifyWorkspaceMoveState', () => { ) }) + it('rejects a drifted non-organization mode when an organization is still assigned', () => { + expect(() => + classifyWorkspaceMoveState( + { + workspaceMode: WORKSPACE_MODE.PERSONAL, + organizationId: 'org-source', + archivedAt: null, + }, + 'org-destination' + ) + ).toThrowError( + expect.objectContaining>({ + code: 'already-organization-workspace', + }) + ) + }) + it('keeps archived personal workspaces movable so they cannot dodge organization purview', () => { expect( classifyWorkspaceMoveState( @@ -140,22 +172,79 @@ describe('classifyWorkspaceMoveState', () => { describe('pending invitation destination identity', () => { it('matches by email and organization without splitting internal/external intent', () => { const dialect = new PgDialect() + const now = new Date('2026-07-30T12:00:00.000Z') const query = dialect.sqlToQuery( buildPendingInvitationMergeScopeCondition({ email: 'Invitee@Example.com', organizationId: 'org-1', excludeInvitationId: 'invite-source', + now, })! ) expect(query.sql).not.toContain('membership_intent') + expect(query.sql).toContain(' > ') expect(query.params).toContain('invitee@example.com') expect(query.params).toContain('org-1') + expect(query.params).toContain(now) expect(query.params).not.toContain('internal') expect(query.params).not.toContain('external') }) }) +describe('migrated invitation email outbox', () => { + it('re-reads the surviving invitation and sends its final grants', async () => { + getInvitationById.mockResolvedValue({ + id: 'invite-surviving', + status: 'pending', + token: 'final-token', + kind: 'workspace', + email: 'invitee@example.com', + inviterName: 'Workspace Admin', + inviterEmail: 'admin@example.com', + organizationId: 'org-1', + role: 'member', + expiresAt: new Date(Date.now() + 60_000), + grants: [ + { workspaceId: 'workspace-1', permission: 'write' }, + { workspaceId: 'workspace-2', permission: 'read' }, + ], + }) + isInvitationExpired.mockReturnValue(false) + sendInvitationEmail.mockResolvedValue({ success: true }) + + await invitationMigrationOutboxHandlers[MIGRATED_INVITATION_EMAIL_EVENT_TYPE]( + { invitationId: 'invite-surviving' }, + {} as never + ) + + expect(sendInvitationEmail).toHaveBeenCalledWith( + expect.objectContaining({ + invitationId: 'invite-surviving', + token: 'final-token', + grants: [ + { workspaceId: 'workspace-1', permission: 'write' }, + { workspaceId: 'workspace-2', permission: 'read' }, + ], + }) + ) + }) + + it('skips a split token that was cancelled before the settle window elapsed', async () => { + getInvitationById.mockResolvedValue({ + id: 'invite-transient', + status: 'cancelled', + }) + + await invitationMigrationOutboxHandlers[MIGRATED_INVITATION_EMAIL_EVENT_TYPE]( + { invitationId: 'invite-transient' }, + {} as never + ) + + expect(sendInvitationEmail).not.toHaveBeenCalled() + }) +}) + describe('moveWorkspaceToOrganization retries', () => { it('returns the existing destination summary without repeating side effects', async () => { queueMoveSelects(movedWorkspace) @@ -179,7 +268,7 @@ describe('moveWorkspaceToOrganization retries', () => { expect(changeWorkspaceStoragePayerInTx).not.toHaveBeenCalled() }) - it('pre-locks a nonzero workspace before changing its storage payer', async () => { + it('takes shared advisory locks before the workspace row lock and payer mutation', async () => { queueMoveSelects(personalWorkspace) await moveWorkspaceToOrganization({ @@ -188,12 +277,30 @@ describe('moveWorkspaceToOrganization retries', () => { adminEmail: 'admin@sim.ai', }) - // The first `.for('update')` in the move path is the workspace pre-lock - // select (the earlier invitation-scan selects carry no row lock), so its - // invocation order against the payer mutation proves lock-before-payer. + const advisoryLock = acquireInvitationMutationLocks.mock.invocationCallOrder[0] const firstForUpdate = dbChainMockFns.for.mock.invocationCallOrder[0] const payerMutation = changeWorkspaceStoragePayerInTx.mock.invocationCallOrder[0] + expect(advisoryLock).toBeGreaterThan(0) + expect(firstForUpdate).toBeGreaterThan(advisoryLock) expect(firstForUpdate).toBeGreaterThan(0) expect(payerMutation).toBeGreaterThan(firstForUpdate) }) + + it('rejects a stale batch selection when workspace ownership changed', async () => { + queueMoveSelects({ ...personalWorkspace, ownerId: 'new-owner' }) + + await expect( + moveWorkspaceToOrganization({ + workspaceId: personalWorkspace.id, + destinationOrganizationId: destination.id, + adminEmail: 'admin@sim.ai', + expectedOwnerId: personalWorkspace.ownerId, + }) + ).rejects.toMatchObject>({ + code: 'workspace-owner-changed', + }) + + expect(changeWorkspaceStoragePayerInTx).not.toHaveBeenCalled() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + }) }) diff --git a/apps/sim/lib/workspaces/admin-move.ts b/apps/sim/lib/workspaces/admin-move.ts index fa6243fdaad..a898c4e04f4 100644 --- a/apps/sim/lib/workspaces/admin-move.ts +++ b/apps/sim/lib/workspaces/admin-move.ts @@ -12,16 +12,18 @@ import { } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { PERMISSION_RANK, type PermissionType } from '@sim/platform-authz/workspace' +import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { normalizeEmail } from '@sim/utils/string' -import { and, asc, count, eq, ilike, inArray, isNull, ne, or, sql } from 'drizzle-orm' +import { and, asc, count, eq, gt, ilike, inArray, isNull, lte, ne, or, sql } from 'drizzle-orm' import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership' import { changeWorkspaceStoragePayerInTx } from '@/lib/billing/storage/payer-transfer' +import { planHasFixedSeatCap, resolveSeatCapacity } from '@/lib/billing/validation/seat-management' import { enqueueOutboxEvent, type OutboxHandler } from '@/lib/core/outbox/service' import type { DbOrTx } from '@/lib/db/types' -import { getInvitationById } from '@/lib/invitations/core' +import { getInvitationById, isInvitationExpired } from '@/lib/invitations/core' import { acquireInvitationMutationLocks } from '@/lib/invitations/locks' -import { sendInvitationEmail } from '@/lib/invitations/send' +import { PENDING_INVITATION_UNIQUE_INDEX, sendInvitationEmail } from '@/lib/invitations/send' import { invalidateWorkspaceTableLimitsCache } from '@/lib/table/billing' import { mergeInvitationMembershipIntent, @@ -32,6 +34,10 @@ import { WORKSPACE_MODE } from '@/lib/workspaces/policy' const logger = createLogger('AdminWorkspaceMove') const ENTITLED_STATUSES = ['active', 'past_due'] as const +// A dashboard member add may move several grants from one invitation in +// consecutive short transactions. Let that split/merge sequence settle before +// the outbox resolves the live invitation and sends its final token. +const MIGRATED_INVITATION_EMAIL_SETTLE_MS = 60_000 export class WorkspaceMoveError extends Error { constructor( @@ -39,6 +45,7 @@ export class WorkspaceMoveError extends Error { readonly code: | 'workspace-not-found' | 'organization-not-found' + | 'workspace-owner-changed' | 'already-organization-workspace' ) { super(message) @@ -117,6 +124,13 @@ class InvitationSetChangedError extends Error { } } +function isConcurrentPendingInvitationInsert(error: unknown): boolean { + return ( + getPostgresErrorCode(error) === '23505' && + getPostgresConstraintName(error) === PENDING_INVITATION_UNIQUE_INDEX + ) +} + /** Returns movable personal/grandfathered workspaces by case-insensitive name or exact UUID. */ export async function searchWorkspaceMoveCandidates( search: string, @@ -142,6 +156,7 @@ export async function searchWorkspaceMoveCandidates( .where( and( ne(workspace.workspaceMode, WORKSPACE_MODE.ORGANIZATION), + isNull(workspace.organizationId), or(eq(workspace.id, query), ilike(workspace.name, `%${query}%`)) ) ) @@ -195,6 +210,7 @@ export async function getWorkspaceMovePreflight( .where(eq(member.organizationId, destinationOrganizationId)), db .select({ + id: subscription.id, plan: subscription.plan, status: subscription.status, metadata: subscription.metadata, @@ -212,7 +228,15 @@ export async function getWorkspaceMovePreflight( const pendingInternalCount = invitationRows.filter( (row) => row.membershipIntent === 'internal' ).length - const seatCapacity = getEnterpriseSeatCapacity(subscriptionRows[0]) + const organizationSubscription = subscriptionRows[0] + const seatCapacity = + organizationSubscription && + ENTITLED_STATUSES.includes( + organizationSubscription.status as (typeof ENTITLED_STATUSES)[number] + ) && + planHasFixedSeatCap(organizationSubscription.plan) + ? await resolveSeatCapacity(organizationSubscription) + : null const currentMembers = memberCountRows[0]?.value ?? 0 const warning = seatCapacity !== null && currentMembers + pendingInternalCount > seatCapacity @@ -235,15 +259,17 @@ export async function getWorkspaceMovePreflight( } /** - * Moves one workspace and migrates every pending grant without changing - * ownership, historical usage, credentials, storage attribution, or existing - * collaborator permissions. + * Moves one workspace and migrates every pending grant. Workspace ownership, + * historical usage, credentials, and collaborator permissions are preserved; + * the current billing/storage payer changes to the destination organization. */ export async function moveWorkspaceToOrganization(params: { workspaceId: string destinationOrganizationId: string adminEmail: string -}): Promise { + /** Reject a stale batch selection instead of moving a newly owned workspace. */ + expectedOwnerId?: string +}): Promise { let candidateInvitationIds = await findInvitationMigrationLockIds( params.workspaceId, params.destinationOrganizationId @@ -253,16 +279,10 @@ export async function moveWorkspaceToOrganization(params: { for (let attempt = 0; attempt < 5; attempt += 1) { try { result = await db.transaction(async (tx) => { - await tx - .select({ id: workspace.id }) - .from(workspace) - .where(eq(workspace.id, params.workspaceId)) - .orderBy(asc(workspace.id)) - .for('update') - - // Acceptance takes invitation/workspace locks before the organization - // lock. Use the identical order here so a concurrent accept and move - // cannot wait on one another in opposite directions. + // Acceptance takes invitation/workspace advisory locks before it + // row-locks the workspace. Keep the exact same order here: taking the + // row lock first can deadlock when acceptance owns an invitation lock, + // waits for the workspace row, and this move waits for that invitation. await acquireInvitationMutationLocks(tx, { invitationIds: candidateInvitationIds, workspaceIds: [params.workspaceId], @@ -295,6 +315,12 @@ export async function moveWorkspaceToOrganization(params: { if (!workspaceRow) { throw new WorkspaceMoveError('Workspace not found', 'workspace-not-found') } + if (params.expectedOwnerId && workspaceRow.ownerId !== params.expectedOwnerId) { + throw new WorkspaceMoveError( + 'Workspace owner changed after it was selected', + 'workspace-owner-changed' + ) + } const moveState = classifyWorkspaceMoveState(workspaceRow, params.destinationOrganizationId) const destination = await getDestinationOrganization(params.destinationOrganizationId, tx) @@ -316,8 +342,9 @@ export async function moveWorkspaceToOrganization(params: { } satisfies MoveTransactionResult } - const lockedInvitationIds = await lockCurrentPendingInvitations(tx, params.workspaceId) const now = new Date() + await expireLockedPendingInvitations(tx, candidateInvitationIds, now) + const lockedInvitationIds = await lockCurrentPendingInvitations(tx, params.workspaceId, now) const migration = await migratePendingInvitations(tx, { workspaceId: params.workspaceId, destinationOrganizationId: params.destinationOrganizationId, @@ -325,7 +352,12 @@ export async function moveWorkspaceToOrganization(params: { now, }) for (const invitationId of migration.invitationsToEmail) { - await enqueueOutboxEvent(tx, MIGRATED_INVITATION_EMAIL_EVENT_TYPE, { invitationId }) + await enqueueOutboxEvent( + tx, + MIGRATED_INVITATION_EMAIL_EVENT_TYPE, + { invitationId }, + { availableAt: new Date(now.getTime() + MIGRATED_INVITATION_EMAIL_SETTLE_MS) } + ) } await changeWorkspaceStoragePayerInTx(tx, { @@ -374,8 +406,18 @@ export async function moveWorkspaceToOrganization(params: { }) break } catch (error) { - if (!(error instanceof InvitationSetChangedError)) throw error - candidateInvitationIds = error.invitationIds + if (error instanceof InvitationSetChangedError) { + candidateInvitationIds = error.invitationIds + continue + } + if (isConcurrentPendingInvitationInsert(error)) { + candidateInvitationIds = await findInvitationMigrationLockIds( + params.workspaceId, + params.destinationOrganizationId + ) + continue + } + throw error } } @@ -383,13 +425,12 @@ export async function moveWorkspaceToOrganization(params: { throw new Error('Pending invitations kept changing; retry the workspace move') } - const invitationEmailFailures: string[] = [] if (!result.performedMove) { logger.info('Workspace was already in destination organization', { workspaceId: params.workspaceId, destinationOrganizationId: params.destinationOrganizationId, }) - return { ...result.summary, invitationEmailFailures } + return result.summary } invalidateWorkspaceTableLimitsCache(params.workspaceId) @@ -433,10 +474,9 @@ export async function moveWorkspaceToOrganization(params: { workspaceId: params.workspaceId, destinationOrganizationId: params.destinationOrganizationId, invitationEvents: result.invitationEvents.length, - invitationEmailFailures: invitationEmailFailures.length, }) - return { ...result.summary, invitationEmailFailures } + return result.summary } async function searchWorkspaceById(workspaceId: string): Promise { @@ -465,8 +505,15 @@ async function searchWorkspaceById(workspaceId: string): Promise).seats - const parsed = typeof seats === 'number' ? seats : Number(seats) - return Number.isInteger(parsed) && parsed > 0 ? parsed : null -} - /** * Lock the source invitations plus every pending invitation for the same * invitees. Those rows are potential split/merge targets and acceptance locks @@ -566,12 +603,17 @@ async function findInvitationMigrationLockIds( _destinationOrganizationId: string, executor: DbOrTx = db ): Promise { + const now = new Date() const sourceRows = await executor .select({ id: invitation.id, email: invitation.email }) .from(invitation) .innerJoin(invitationWorkspaceGrant, eq(invitationWorkspaceGrant.invitationId, invitation.id)) .where( - and(eq(invitation.status, 'pending'), eq(invitationWorkspaceGrant.workspaceId, workspaceId)) + and( + eq(invitation.status, 'pending'), + gt(invitation.expiresAt, now), + eq(invitationWorkspaceGrant.workspaceId, workspaceId) + ) ) if (sourceRows.length === 0) return [] @@ -590,13 +632,39 @@ async function findInvitationMigrationLockIds( ].sort() } -async function lockCurrentPendingInvitations(tx: DbOrTx, workspaceId: string): Promise { +async function expireLockedPendingInvitations( + tx: DbOrTx, + invitationIds: string[], + now: Date +): Promise { + if (invitationIds.length === 0) return + await tx + .update(invitation) + .set({ status: 'expired', updatedAt: now }) + .where( + and( + inArray(invitation.id, invitationIds), + eq(invitation.status, 'pending'), + lte(invitation.expiresAt, now) + ) + ) +} + +async function lockCurrentPendingInvitations( + tx: DbOrTx, + workspaceId: string, + now: Date +): Promise { const rows = await tx .select({ id: invitation.id }) .from(invitation) .innerJoin(invitationWorkspaceGrant, eq(invitationWorkspaceGrant.invitationId, invitation.id)) .where( - and(eq(invitation.status, 'pending'), eq(invitationWorkspaceGrant.workspaceId, workspaceId)) + and( + eq(invitation.status, 'pending'), + gt(invitation.expiresAt, now), + eq(invitationWorkspaceGrant.workspaceId, workspaceId) + ) ) .orderBy(invitation.id) .for('update') @@ -619,7 +687,13 @@ async function migratePendingInvitations( const [source] = await tx .select() .from(invitation) - .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) + .where( + and( + eq(invitation.id, invitationId), + eq(invitation.status, 'pending'), + gt(invitation.expiresAt, params.now) + ) + ) .limit(1) if (!source) continue @@ -639,6 +713,7 @@ async function migratePendingInvitations( email: source.email, organizationId: params.destinationOrganizationId, excludeInvitationId: source.id, + now: params.now, }) const partition = partitionInvitationGrantsForWorkspaceMove({ grants, @@ -678,6 +753,7 @@ async function migratePendingInvitations( email: source.email, organizationId, excludeInvitationId: source.id, + now: params.now, }) const siblingId = sibling?.id ?? @@ -734,6 +810,7 @@ async function findPendingInvitationForScope( email: string organizationId: string | null excludeInvitationId: string + now: Date } ) { // Membership intent is deliberately not part of destination identity. A @@ -748,6 +825,7 @@ async function findPendingInvitationForScope( .from(invitation) .where(buildPendingInvitationMergeScopeCondition(params)) .orderBy(invitation.createdAt) + .for('update') .limit(1) return row ?? null } @@ -756,6 +834,7 @@ export function buildPendingInvitationMergeScopeCondition(params: { email: string organizationId: string | null excludeInvitationId: string + now?: Date }) { const scope = params.organizationId ? eq(invitation.organizationId, params.organizationId) @@ -763,6 +842,7 @@ export function buildPendingInvitationMergeScopeCondition(params: { return and( sql`lower(${invitation.email}) = ${normalizeEmail(params.email)}`, eq(invitation.status, 'pending'), + gt(invitation.expiresAt, params.now ?? new Date()), ne(invitation.id, params.excludeInvitationId), scope ) @@ -855,7 +935,7 @@ async function mergeGrant( const sendMigratedInvitationLink: OutboxHandler<{ invitationId: string }> = async (payload) => { const migrated = await getInvitationById(payload.invitationId) - if (!migrated || migrated.status !== 'pending') return + if (!migrated || migrated.status !== 'pending' || isInvitationExpired(migrated)) return const result = await sendInvitationEmail({ invitationId: migrated.id, token: migrated.token, diff --git a/apps/sim/lib/workspaces/organization-workspaces.test.ts b/apps/sim/lib/workspaces/organization-workspaces.test.ts index 43b5fe564ef..91ba3ce8cf7 100644 --- a/apps/sim/lib/workspaces/organization-workspaces.test.ts +++ b/apps/sim/lib/workspaces/organization-workspaces.test.ts @@ -11,6 +11,7 @@ const { mockAcquireOrganizationMutationLock, mockAcquireInvitationMutationLocks, mockChangeWorkspaceStoragePayersInTx, + mockInvalidateWorkspaceTableLimitsCache, } = vi.hoisted(() => ({ mockEnsureUserInOrganizationTx: vi.fn(), mockSyncUsageLimitsFromSubscription: vi.fn(), @@ -18,6 +19,7 @@ const { mockAcquireOrganizationMutationLock: vi.fn(), mockAcquireInvitationMutationLocks: vi.fn(), mockChangeWorkspaceStoragePayersInTx: vi.fn(), + mockInvalidateWorkspaceTableLimitsCache: vi.fn(), })) vi.mock('@/lib/billing/organizations/membership', () => ({ @@ -34,6 +36,10 @@ vi.mock('@/lib/invitations/locks', () => ({ acquireInvitationMutationLocks: mockAcquireInvitationMutationLocks, })) +vi.mock('@/lib/table/billing', () => ({ + invalidateWorkspaceTableLimitsCache: mockInvalidateWorkspaceTableLimitsCache, +})) + vi.mock('@/lib/billing/core/usage', () => ({ syncUsageLimitsFromSubscription: mockSyncUsageLimitsFromSubscription, })) @@ -122,10 +128,14 @@ describe('organization workspace helpers', () => { 'owner-1', 'org-1' ) + expect(mockAcquireInvitationMutationLocks.mock.invocationCallOrder[0]).toBeLessThan( + dbChainMockFns.for.mock.invocationCallOrder[0] + ) expect(dbChainMockFns.set).toHaveBeenCalledWith( expect.objectContaining({ organizationAssignedAt: expect.any(Date) }) ) expect(mockChangeWorkspaceStoragePayersInTx).toHaveBeenCalledTimes(1) + expect(mockInvalidateWorkspaceTableLimitsCache).toHaveBeenCalledTimes(2) expect(dbChainMockFns.for.mock.invocationCallOrder[0]).toBeLessThan( mockEnsureUserInOrganizationTx.mock.invocationCallOrder[0] ) diff --git a/apps/sim/lib/workspaces/organization-workspaces.ts b/apps/sim/lib/workspaces/organization-workspaces.ts index c6ab1fac739..42b54dd0ca0 100644 --- a/apps/sim/lib/workspaces/organization-workspaces.ts +++ b/apps/sim/lib/workspaces/organization-workspaces.ts @@ -13,6 +13,7 @@ import { import { changeWorkspaceStoragePayersInTx } from '@/lib/billing/storage/payer-transfer' import type { DbOrTx } from '@/lib/db/types' import { acquireInvitationMutationLocks } from '@/lib/invitations/locks' +import { invalidateWorkspaceTableLimitsCache } from '@/lib/table/billing' import { getOrganizationOwnerId, WORKSPACE_MODE } from '@/lib/workspaces/policy' const logger = createLogger('OrganizationWorkspaces') @@ -93,10 +94,7 @@ export function ownedAttachableWorkspacesWhere({ ) } -/** - * Locks workspace rows before any membership billing path can lock user or - * organization payer rows. - */ +/** Locks workspace rows before any payer or membership mutation. */ async function lockWorkspaceRowsForPayerChanges(tx: DbOrTx, workspaceIds: string[]): Promise { if (workspaceIds.length === 0) return await tx @@ -134,14 +132,15 @@ export async function attachOwnedWorkspacesToOrganization({ } const attached = await db.transaction(async (tx) => { - await lockWorkspaceRowsForPayerChanges(tx, ownedWorkspaceIds) - // Match admin move and invitation acceptance: workspace/invitation scope - // first, then organization. Membership and assignment now commit or roll - // back together, so a concurrent move cannot leave stray org members. + // Match admin move and invitation acceptance: shared advisory scope first, + // then the workspace rows, then organization/membership locks. Reversing + // the first two can deadlock with an acceptance that already owns an + // invitation lock and is waiting to row-lock one of these workspaces. await acquireInvitationMutationLocks(tx, { invitationIds: [], workspaceIds: ownedWorkspaceIds, }) + await lockWorkspaceRowsForPayerChanges(tx, ownedWorkspaceIds) await acquireOrganizationMutationLock(tx, organizationId) return attachOwnedWorkspacesToOrganizationTx(tx, { ownerUserId, @@ -153,6 +152,9 @@ export async function attachOwnedWorkspacesToOrganization({ }) }) + for (const workspaceId of attached.attachedWorkspaceIds) { + invalidateWorkspaceTableLimitsCache(workspaceId) + } for (const userId of attached.usageLimitUserIds) { try { await syncUsageLimitsFromSubscription(userId) From 244bff654d187d9c26f480f1ab0f3b1402d2f597 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 30 Jul 2026 18:27:31 -0700 Subject: [PATCH 2/4] refactor: address review nits on the admin/invitation lock work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correct the attach lock-order comment. The order matches admin move, and what makes it mandatory is invitation acceptance: it holds `workspace-invitations:` while waiting for the workspace row, so row-locking first (as this did) deadlocks against it. The previous ownership-transfer justification did not hold — that path takes the organization lock before its workspace rows too, so the two agree on order rather than inverting. Drop `cancelInvitation`. The `revokeInvitationAsAdmin` extraction left it with no callers, and an unlocked, unauthorized `status = 'cancelled'` flip sitting next to the fenced replacement is easy to reach for by mistake. Drop the unused `executor` parameters from `hasWorkspaceAdminAccess` and `isOrganizationAdminOrOwner`. No caller threads a transaction through either, and the former goes back to delegating to `checkWorkspaceAccess` instead of re-deriving the same permission itself. Import `chunkArray` from `@sim/utils/helpers` everywhere and remove the re-export from `batch-delete.ts`, so the symbol has one source rather than a non-barrel shim plus the package. Restore the bounded attachability check in `addDashboardOrganizationMember`: scope the query to the selected ids instead of listing every attachable workspace and scanning that array per selection. Resolve the credential-creation permission through `getEffectiveWorkspacePermission` rather than a second copy of the org-admin derivation ladder, so the rule cannot drift from the shared resolver. Co-Authored-By: Claude --- apps/sim/background/cleanup-logs.test.ts | 7 --- apps/sim/background/cleanup-logs.ts | 2 +- .../background/cleanup-soft-deletes.test.ts | 7 --- apps/sim/background/cleanup-soft-deletes.ts | 2 +- apps/sim/background/cleanup-tasks.ts | 2 +- .../lib/admin/dashboard-credit-grant.test.ts | 4 +- apps/sim/lib/admin/dashboard.ts | 18 +++++--- .../lib/billing/cleanup-dispatcher.test.ts | 3 -- apps/sim/lib/billing/cleanup-dispatcher.ts | 2 +- apps/sim/lib/cleanup/batch-delete.ts | 2 - apps/sim/lib/cleanup/chat-cleanup.ts | 2 +- apps/sim/lib/credentials/environment.ts | 43 ++++++------------- .../payloads/large-value-metadata.ts | 2 +- apps/sim/lib/invitations/core.ts | 10 ----- .../lib/workspaces/organization-workspaces.ts | 10 +++-- apps/sim/lib/workspaces/permissions/utils.ts | 13 ++---- 16 files changed, 43 insertions(+), 86 deletions(-) diff --git a/apps/sim/background/cleanup-logs.test.ts b/apps/sim/background/cleanup-logs.test.ts index 9ee5306a39b..2b1adb3cdc5 100644 --- a/apps/sim/background/cleanup-logs.test.ts +++ b/apps/sim/background/cleanup-logs.test.ts @@ -49,13 +49,6 @@ vi.mock('@trigger.dev/sdk', () => ({ task: mockTask })) vi.mock('@/lib/cleanup/batch-delete', () => ({ batchDeleteByWorkspaceAndTimestamp: mockBatchDeleteByWorkspaceAndTimestamp, - chunkArray: (items: string[], size: number) => { - const chunks: string[][] = [] - for (let index = 0; index < items.length; index += size) { - chunks.push(items.slice(index, index + size)) - } - return chunks - }, chunkedBatchDelete: mockChunkedBatchDelete, })) diff --git a/apps/sim/background/cleanup-logs.ts b/apps/sim/background/cleanup-logs.ts index 2ee922fb7e0..2d560ef74c0 100644 --- a/apps/sim/background/cleanup-logs.ts +++ b/apps/sim/background/cleanup-logs.ts @@ -9,12 +9,12 @@ import { workspaceFiles, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { task } from '@trigger.dev/sdk' import { and, asc, eq, inArray, isNull, lt, notInArray, or, sql } from 'drizzle-orm' import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' import { batchDeleteByWorkspaceAndTimestamp, - chunkArray, chunkedBatchDelete, type TableCleanupResult, } from '@/lib/cleanup/batch-delete' diff --git a/apps/sim/background/cleanup-soft-deletes.test.ts b/apps/sim/background/cleanup-soft-deletes.test.ts index eab9ff06e63..ef3e8be58aa 100644 --- a/apps/sim/background/cleanup-soft-deletes.test.ts +++ b/apps/sim/background/cleanup-soft-deletes.test.ts @@ -49,13 +49,6 @@ vi.mock('@/lib/cleanup/batch-delete', () => ({ batchDeleteByWorkspaceAndTimestamp: mockBatchDeleteByWorkspaceAndTimestamp, chunkedBatchDelete: mockChunkedBatchDelete, DEFAULT_DELETE_CHUNK_SIZE: 1000, - chunkArray: (items: string[], size: number) => { - const chunks: string[][] = [] - for (let index = 0; index < items.length; index += size) { - chunks.push(items.slice(index, index + size)) - } - return chunks - }, deleteRowsById: mockDeleteRowsById, selectRowsByIdChunks: mockSelectRowsByIdChunks, })) diff --git a/apps/sim/background/cleanup-soft-deletes.ts b/apps/sim/background/cleanup-soft-deletes.ts index f49be176a3e..42bf64f7237 100644 --- a/apps/sim/background/cleanup-soft-deletes.ts +++ b/apps/sim/background/cleanup-soft-deletes.ts @@ -13,6 +13,7 @@ import { workspaceFiles, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { task } from '@trigger.dev/sdk' import { and, asc, eq, inArray, isNotNull, isNull, lt, sql } from 'drizzle-orm' import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' @@ -23,7 +24,6 @@ import { } from '@/lib/billing/storage' import { batchDeleteByWorkspaceAndTimestamp, - chunkArray, chunkedBatchDelete, DEFAULT_DELETE_CHUNK_SIZE, selectRowsByIdChunks, diff --git a/apps/sim/background/cleanup-tasks.ts b/apps/sim/background/cleanup-tasks.ts index 1745e3bba14..1d35d143498 100644 --- a/apps/sim/background/cleanup-tasks.ts +++ b/apps/sim/background/cleanup-tasks.ts @@ -7,12 +7,12 @@ import { mothershipInboxTask, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { task } from '@trigger.dev/sdk' import { and, inArray, lt } from 'drizzle-orm' import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' import { batchDeleteByWorkspaceAndTimestamp, - chunkArray, DEFAULT_DELETE_CHUNK_SIZE, deleteRowsById, selectRowsByIdChunks, diff --git a/apps/sim/lib/admin/dashboard-credit-grant.test.ts b/apps/sim/lib/admin/dashboard-credit-grant.test.ts index 614b952352e..b8c26bb0348 100644 --- a/apps/sim/lib/admin/dashboard-credit-grant.test.ts +++ b/apps/sim/lib/admin/dashboard-credit-grant.test.ts @@ -340,7 +340,9 @@ describe('addDashboardOrganizationMember', () => { }) it('uses the expected owner guard for an administrator-selected subset', async () => { - queueTableRows(workspace, [{ id: 'workspace-1' }, { id: 'workspace-2' }]) + // The attachability query is scoped to the selected ids, so it returns only + // `workspace-1` even though the user owns more. + queueTableRows(workspace, [{ id: 'workspace-1' }]) queueTableRows(member, [{ id: 'member-old', organizationId: 'org-old' }]) mocks.transferMembership.mockResolvedValue({ success: true, diff --git a/apps/sim/lib/admin/dashboard.ts b/apps/sim/lib/admin/dashboard.ts index 3cba4f5db28..a2af65f11bf 100644 --- a/apps/sim/lib/admin/dashboard.ts +++ b/apps/sim/lib/admin/dashboard.ts @@ -968,15 +968,21 @@ export async function addDashboardOrganizationMember( actor: AdminMutationActor ) { const selectedWorkspaceIds = [...new Set(values.personalWorkspaceIds ?? [])] - const attachableWorkspaceIds = ( - await db + if (selectedWorkspaceIds.length > 0) { + const selectable = await db .select({ id: workspace.id }) .from(workspace) - .where(ownedAttachableWorkspacesWhere({ userId: values.userId, includeArchived: true })) - ).map((row) => row.id) - if (selectedWorkspaceIds.some((id) => !attachableWorkspaceIds.includes(id))) { - throw new Error('One or more selected personal workspaces can no longer be moved') + .where( + and( + ownedAttachableWorkspacesWhere({ userId: values.userId, includeArchived: true }), + inArray(workspace.id, selectedWorkspaceIds) + ) + ) + if (selectable.length !== selectedWorkspaceIds.length) { + throw new Error('One or more selected personal workspaces can no longer be moved') + } } + const [existingMembership] = await db .select({ id: member.id, organizationId: member.organizationId }) .from(member) diff --git a/apps/sim/lib/billing/cleanup-dispatcher.test.ts b/apps/sim/lib/billing/cleanup-dispatcher.test.ts index e1d3f8977ac..27160138359 100644 --- a/apps/sim/lib/billing/cleanup-dispatcher.test.ts +++ b/apps/sim/lib/billing/cleanup-dispatcher.test.ts @@ -23,9 +23,6 @@ vi.mock('@/lib/billing/core/billing', () => ({ vi.mock('@/lib/billing/core/subscription', () => ({ getHighestPriorityPersonalSubscription: vi.fn(), })) -vi.mock('@/lib/cleanup/batch-delete', () => ({ - chunkArray: vi.fn((items: unknown[]) => (items.length > 0 ? [items] : [])), -})) vi.mock('@/lib/core/async-jobs', () => ({ getJobQueue: vi.fn(() => ({ enqueue: mockEnqueue })), })) diff --git a/apps/sim/lib/billing/cleanup-dispatcher.ts b/apps/sim/lib/billing/cleanup-dispatcher.ts index 8b8608d63a4..b53f7ae7faf 100644 --- a/apps/sim/lib/billing/cleanup-dispatcher.ts +++ b/apps/sim/lib/billing/cleanup-dispatcher.ts @@ -2,13 +2,13 @@ import { db } from '@sim/db' import type { DataRetentionSettings, WorkspaceMode } from '@sim/db/schema' import { organization, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { tasks } from '@trigger.dev/sdk' import { and, asc, eq, gt, isNull } from 'drizzle-orm' import { getOrganizationSubscription } from '@/lib/billing/core/billing' import { getHighestPriorityPersonalSubscription } from '@/lib/billing/core/subscription' import { getPlanType, type PlanCategory } from '@/lib/billing/plan-helpers' import { type RetentionHoursKey, resolveEffectiveRetentionHours } from '@/lib/billing/retention' -import { chunkArray } from '@/lib/cleanup/batch-delete' import { getJobQueue } from '@/lib/core/async-jobs' import { shouldExecuteInline } from '@/lib/core/async-jobs/config' import { resolveTriggerRegion } from '@/lib/core/async-jobs/region' diff --git a/apps/sim/lib/cleanup/batch-delete.ts b/apps/sim/lib/cleanup/batch-delete.ts index f72a9901a93..84b12a039ec 100644 --- a/apps/sim/lib/cleanup/batch-delete.ts +++ b/apps/sim/lib/cleanup/batch-delete.ts @@ -26,8 +26,6 @@ export const DEFAULT_WORKSPACE_CHUNK_SIZE = 50 /** Bounds FK cascade trigger queue (per-statement in-memory) and bind-parameter count. */ export const DEFAULT_DELETE_CHUNK_SIZE = 1000 -export { chunkArray } - export interface SelectByIdChunksOptions { /** Cap on rows returned across all chunks. Defaults to a full per-table cleanup budget. */ overallLimit?: number diff --git a/apps/sim/lib/cleanup/chat-cleanup.ts b/apps/sim/lib/cleanup/chat-cleanup.ts index ba84f1f0279..01ca0777e2d 100644 --- a/apps/sim/lib/cleanup/chat-cleanup.ts +++ b/apps/sim/lib/cleanup/chat-cleanup.ts @@ -1,8 +1,8 @@ import { dbFor } from '@sim/db' import { copilotChats, copilotMessages, workspaceFiles } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { and, inArray, isNull } from 'drizzle-orm' -import { chunkArray } from '@/lib/cleanup/batch-delete' import { SIM_AGENT_API_URL } from '@/lib/copilot/constants' import { env } from '@/lib/core/config/env' import type { StorageContext } from '@/lib/uploads' diff --git a/apps/sim/lib/credentials/environment.ts b/apps/sim/lib/credentials/environment.ts index b692a2d07e4..d0d33a11b97 100644 --- a/apps/sim/lib/credentials/environment.ts +++ b/apps/sim/lib/credentials/environment.ts @@ -1,16 +1,15 @@ import { db } from '@sim/db' -import { credential, credentialMember, member, permissions, workspace } from '@sim/db/schema' -import { - isOrgAdminRole, - type PermissionType, - permissionSatisfies, -} from '@sim/platform-authz/workspace' +import { credential, credentialMember, permissions, workspace } from '@sim/db/schema' +import { permissionSatisfies } from '@sim/platform-authz/workspace' import { chunkArray } from '@sim/utils/helpers' import { generateId } from '@sim/utils/id' import { and, eq, inArray, isNotNull, isNull, notInArray, or, sql } from 'drizzle-orm' import { acquireUserBillingIdentityLock } from '@/lib/billing/organizations/billing-identity-lock' import type { DbOrTx } from '@/lib/db/types' -import { hasWorkspaceAdminAccess } from '@/lib/workspaces/permissions/utils' +import { + getEffectiveWorkspacePermission, + hasWorkspaceAdminAccess, +} from '@/lib/workspaces/permissions/utils' const PERSONAL_ENV_CREDENTIAL_WRITE_CHUNK_SIZE = 500 @@ -78,35 +77,17 @@ export async function getCredentialCreationWorkspaceContext(params: { if (!workspaceRow) return null const permissionRows = await params.executor - .select({ - userId: permissions.userId, - permissionType: permissions.permissionType, - }) + .select({ userId: permissions.userId }) .from(permissions) .where( and(eq(permissions.entityType, 'workspace'), eq(permissions.entityId, params.workspaceId)) ) - const explicitPermission = - (permissionRows.find((row) => row.userId === params.userId)?.permissionType as - | PermissionType - | undefined) ?? null - let effectivePermission = explicitPermission - if (workspaceRow.organizationId && explicitPermission !== 'admin') { - const [organizationMembership] = await params.executor - .select({ role: member.role }) - .from(member) - .where( - and( - eq(member.userId, params.userId), - eq(member.organizationId, workspaceRow.organizationId) - ) - ) - .limit(1) - if (isOrgAdminRole(organizationMembership?.role)) { - effectivePermission = 'admin' - } - } + const effectivePermission = await getEffectiveWorkspacePermission( + params.userId, + { id: params.workspaceId, organizationId: workspaceRow.organizationId }, + params.executor + ) const memberUserIds = new Set(permissionRows.map((row) => row.userId)) memberUserIds.add(workspaceRow.ownerId) diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.ts b/apps/sim/lib/execution/payloads/large-value-metadata.ts index cc0fc07f6f6..5c1a9520113 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.ts @@ -7,8 +7,8 @@ import { workflowExecutionLogs, } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { chunkArray } from '@sim/utils/helpers' import { and, eq, inArray, notInArray, sql } from 'drizzle-orm' -import { chunkArray } from '@/lib/cleanup/batch-delete' import { collectLargeValueKeys } from '@/lib/execution/payloads/large-execution-value' const logger = createLogger('LargeValueMetadata') diff --git a/apps/sim/lib/invitations/core.ts b/apps/sim/lib/invitations/core.ts index 7c871dc1aa8..0c09db7e685 100644 --- a/apps/sim/lib/invitations/core.ts +++ b/apps/sim/lib/invitations/core.ts @@ -1643,16 +1643,6 @@ export async function rejectInvitation( }) } -export async function cancelInvitation(invitationId: string): Promise { - const result = await db - .update(invitation) - .set({ status: 'cancelled', updatedAt: new Date() }) - .where(and(eq(invitation.id, invitationId), eq(invitation.status, 'pending'))) - .returning({ id: invitation.id }) - - return result.length > 0 -} - export type AuthorizedInvitationRevocationResult = | { success: true diff --git a/apps/sim/lib/workspaces/organization-workspaces.ts b/apps/sim/lib/workspaces/organization-workspaces.ts index ebca1298670..49c261f0d5a 100644 --- a/apps/sim/lib/workspaces/organization-workspaces.ts +++ b/apps/sim/lib/workspaces/organization-workspaces.ts @@ -132,10 +132,12 @@ export async function attachOwnedWorkspacesToOrganization({ } const attached = await db.transaction(async (tx) => { - // Match admin move and invitation acceptance: shared advisory scope first, - // then the organization mutation lock, then workspace rows. Ownership - // transfer also takes organization before workspace rows; taking the row - // first here would create a row/org lock inversion with that path. + // Shared advisory scope first, then the organization lock, then workspace + // rows — the order admin move uses. What makes it mandatory is acceptance: + // it holds `workspace-invitations:` while waiting for the workspace + // row, so row-locking first here (as this did) deadlocks against it. + // Taking the organization lock before the rows also matches ownership + // transfer, so those two cannot invert on the org/row pair either. await acquireInvitationMutationLocks(tx, { invitationIds: [], workspaceIds: ownedWorkspaceIds, diff --git a/apps/sim/lib/workspaces/permissions/utils.ts b/apps/sim/lib/workspaces/permissions/utils.ts index b5a6831bf8d..8832e56641e 100644 --- a/apps/sim/lib/workspaces/permissions/utils.ts +++ b/apps/sim/lib/workspaces/permissions/utils.ts @@ -465,13 +465,9 @@ export async function getWorkspacePermissionsForViewer( */ export async function hasWorkspaceAdminAccess( userId: string, - workspaceId: string, - options?: { executor?: DbOrTx } + workspaceId: string ): Promise { - const executor = options?.executor ?? db - const ws = await getWorkspaceWithOwner(workspaceId, { executor }) - if (!ws) return false - return (await getEffectiveWorkspacePermission(userId, ws, executor)) === 'admin' + return (await checkWorkspaceAccess(workspaceId, userId)).canAdmin } /** @@ -483,10 +479,9 @@ export async function hasWorkspaceAdminAccess( */ export async function isOrganizationAdminOrOwner( userId: string, - organizationId: string, - executor: DbOrTx = db + organizationId: string ): Promise { - const [row] = await executor + const [row] = await db .select({ role: member.role }) .from(member) .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) From 91efd369c09b7da97f90f7a600a8a5ebd1a5ac73 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 30 Jul 2026 19:15:51 -0700 Subject: [PATCH 3/4] refactor(invitations): drop dead code left by the revocation extraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `revokeInvitationWorkspaceGrant` lost its only caller when the DELETE route moved to `revokeInvitationAsAdmin`, leaving a locked wrapper nothing invoked. Remove it and fold its documentation into `revokeInvitationWorkspaceGrantTx`, which direct grants and scoped revocation still call. The grant-revocation test now drives the transactional form directly, so the sibling-grant and final-grant-cancels behaviour it covers stays under test. `isSameOrgMember` has had no caller since before this branch — direct grant resolves membership through `getUserOrganization` inside its own transaction — so it and its tests go too. `getWorkspaceMembership` is no longer imported outside its module now that credential creation reads `getCredentialCreationWorkspaceContext`; make it module-private rather than leave it on the public surface. Co-Authored-By: Claude --- apps/sim/lib/credentials/environment.ts | 2 +- apps/sim/lib/invitations/core.ts | 21 ++------------ apps/sim/lib/invitations/direct-grant.test.ts | 28 ------------------- apps/sim/lib/invitations/direct-grant.ts | 14 ---------- .../lib/invitations/grant-revocation.test.ts | 22 +++++---------- 5 files changed, 10 insertions(+), 77 deletions(-) diff --git a/apps/sim/lib/credentials/environment.ts b/apps/sim/lib/credentials/environment.ts index d0d33a11b97..da4f743cbab 100644 --- a/apps/sim/lib/credentials/environment.ts +++ b/apps/sim/lib/credentials/environment.ts @@ -24,7 +24,7 @@ export interface WorkspaceMembership { * Credential-admin status is derived from workspace role at access time, so * members are seeded only for use access (the owner plus permission holders). */ -export async function getWorkspaceMembership(workspaceId: string): Promise { +async function getWorkspaceMembership(workspaceId: string): Promise { const [workspaceRows, permissionRows] = await Promise.all([ db .select({ ownerId: workspace.ownerId }) diff --git a/apps/sim/lib/invitations/core.ts b/apps/sim/lib/invitations/core.ts index 0c09db7e685..13be762010e 100644 --- a/apps/sim/lib/invitations/core.ts +++ b/apps/sim/lib/invitations/core.ts @@ -1746,25 +1746,8 @@ export async function revokeInvitationAsAdmin(input: { * admin of one workspace has no authority over the others. Removing the final * grant would otherwise strand a pending invitation that grants nothing, so * that case cancels it instead. - */ -export async function revokeInvitationWorkspaceGrant({ - invitationId, - workspaceId, -}: { - invitationId: string - workspaceId: string -}): Promise<{ revoked: boolean; invitationCancelled: boolean }> { - return db.transaction(async (tx) => { - await acquireInvitationMutationLocks(tx, { - invitationIds: [invitationId], - workspaceIds: [workspaceId], - }) - return revokeInvitationWorkspaceGrantTx(tx, { invitationId, workspaceId }) - }) -} - -/** - * Transaction-local form used by callers that already hold the canonical + * + * Transaction-local: callers must already hold the canonical * invitation/workspace advisory lock set. Keeping the grant deletion and * final-grant cancellation in one implementation prevents direct grants, * scoped revocation, and future callers from drifting on multi-workspace diff --git a/apps/sim/lib/invitations/direct-grant.test.ts b/apps/sim/lib/invitations/direct-grant.test.ts index d2985ccaca7..58b3dda1058 100644 --- a/apps/sim/lib/invitations/direct-grant.test.ts +++ b/apps/sim/lib/invitations/direct-grant.test.ts @@ -74,7 +74,6 @@ vi.mock('@/lib/posthog/server', () => ({ import { DirectGrantContextChangedError, grantWorkspaceAccessDirectly, - isSameOrgMember, } from '@/lib/invitations/direct-grant' const baseInput = { @@ -246,30 +245,3 @@ describe('grantWorkspaceAccessDirectly', () => { expect(dbChainMockFns.insert).not.toHaveBeenCalled() }) }) - -describe('isSameOrgMember', () => { - beforeEach(() => { - vi.clearAllMocks() - resetDbChainMock() - }) - - it('returns false when the workspace has no organization', async () => { - expect(await isSameOrgMember('user-2', null)).toBe(false) - expect(mockGetUserOrganization).not.toHaveBeenCalled() - }) - - it('returns false when the user belongs to no organization', async () => { - mockGetUserOrganization.mockResolvedValueOnce(null) - expect(await isSameOrgMember('user-2', 'org-1')).toBe(false) - }) - - it('returns true when the user belongs to the workspace organization', async () => { - mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-1', role: 'member' }) - expect(await isSameOrgMember('user-2', 'org-1')).toBe(true) - }) - - it('returns false when the user belongs to a different organization', async () => { - mockGetUserOrganization.mockResolvedValueOnce({ organizationId: 'org-2', role: 'member' }) - expect(await isSameOrgMember('user-2', 'org-1')).toBe(false) - }) -}) diff --git a/apps/sim/lib/invitations/direct-grant.ts b/apps/sim/lib/invitations/direct-grant.ts index bfbeb1f3461..29601204537 100644 --- a/apps/sim/lib/invitations/direct-grant.ts +++ b/apps/sim/lib/invitations/direct-grant.ts @@ -67,20 +67,6 @@ export interface GrantWorkspaceAccessDirectlyInput { notify?: boolean } -/** - * Returns whether the given user is already a member of the workspace's - * organization. Only same-org members are eligible for direct (no-acceptance) - * workspace access. - */ -export async function isSameOrgMember( - userId: string, - workspaceOrganizationId: string | null -): Promise { - if (!workspaceOrganizationId) return false - const membership = await getUserOrganization(userId) - return !!membership && membership.organizationId === workspaceOrganizationId -} - async function getPendingWorkspaceInvitationIds( executor: DbOrTx, workspaceId: string, diff --git a/apps/sim/lib/invitations/grant-revocation.test.ts b/apps/sim/lib/invitations/grant-revocation.test.ts index 7834bc9e4ab..2142863a02c 100644 --- a/apps/sim/lib/invitations/grant-revocation.test.ts +++ b/apps/sim/lib/invitations/grant-revocation.test.ts @@ -1,45 +1,37 @@ /** * @vitest-environment node */ +import { db } from '@sim/db' import { invitation, invitationWorkspaceGrant } from '@sim/db/schema' import { auditMock, dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockAcquireInvitationMutationLocks } = vi.hoisted(() => ({ - mockAcquireInvitationMutationLocks: vi.fn(), -})) - vi.mock('@sim/audit', () => auditMock) vi.mock('@/lib/invitations/locks', () => ({ - acquireInvitationMutationLocks: mockAcquireInvitationMutationLocks, + acquireInvitationMutationLocks: vi.fn(), })) -import { revokeInvitationWorkspaceGrant } from '@/lib/invitations/core' +import { revokeInvitationWorkspaceGrantTx } from '@/lib/invitations/core' -describe('revokeInvitationWorkspaceGrant', () => { +describe('revokeInvitationWorkspaceGrantTx', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() - mockAcquireInvitationMutationLocks.mockResolvedValue(undefined) }) - it('uses the shared lock fence and preserves sibling workspace grants', async () => { + it('preserves sibling workspace grants', async () => { queueTableRows(invitation, [{ id: 'inv-1' }]) queueTableRows(invitationWorkspaceGrant, [{ value: 1 }]) dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'grant-1' }]) await expect( - revokeInvitationWorkspaceGrant({ + revokeInvitationWorkspaceGrantTx(db, { invitationId: 'inv-1', workspaceId: 'ws-1', }) ).resolves.toEqual({ revoked: true, invitationCancelled: false }) - expect(mockAcquireInvitationMutationLocks).toHaveBeenCalledWith(expect.anything(), { - invitationIds: ['inv-1'], - workspaceIds: ['ws-1'], - }) expect(dbChainMockFns.set).toHaveBeenCalledWith( expect.not.objectContaining({ status: 'cancelled' }) ) @@ -51,7 +43,7 @@ describe('revokeInvitationWorkspaceGrant', () => { dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'grant-1' }]) await expect( - revokeInvitationWorkspaceGrant({ + revokeInvitationWorkspaceGrantTx(db, { invitationId: 'inv-1', workspaceId: 'ws-1', }) From 1f26f7f13a8ff63fa6edbf332d527c77782f22dc Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Thu, 30 Jul 2026 19:53:40 -0700 Subject: [PATCH 4/4] refactor: remove four uncalled billing and large-value helpers Each was checked by hand across every file type, including barrel re-exports and string references, rather than taken from a static analyzer. `isUserMemberOfOrganization` has no reference anywhere. `reapplyPaidOrgJoinBillingForExistingMember` only ever ran from two lock-ordering tests. The transaction-enlisted form it delegated to is what the subscription webhooks call and what those tests actually assert on, so they now drive it directly. The assertions are unchanged: the wrapper contributed a transaction, an organization lock and a membership existence check, none of which appear in the recorded operations. `replaceLargeValueReferences` and `replaceLargeValueReferencesWithClient` are both thin wrappers over `replaceLargeValueReferenceKeysWithClient`, which execution logging, human-in-the-loop resume and the trace backfill all still call. The single test covering a wrapper now composes the key collection itself and targets that live helper. Co-Authored-By: Claude --- .../billing/organizations/lock-order.test.ts | 8 +-- .../lib/billing/organizations/membership.ts | 45 -------------- .../payloads/large-value-metadata.test.ts | 62 ++++++++++--------- .../payloads/large-value-metadata.ts | 28 --------- 4 files changed, 36 insertions(+), 107 deletions(-) diff --git a/apps/sim/lib/billing/organizations/lock-order.test.ts b/apps/sim/lib/billing/organizations/lock-order.test.ts index 453828e0c0d..fb1d96893ae 100644 --- a/apps/sim/lib/billing/organizations/lock-order.test.ts +++ b/apps/sim/lib/billing/organizations/lock-order.test.ts @@ -34,7 +34,7 @@ vi.mock('@/lib/billing/storage/payer-transfer', () => ({ })) import { - reapplyPaidOrgJoinBillingForExistingMember, + reapplyPaidOrgJoinBillingForExistingMemberTx, restoreUserProSubscription, transferOrganizationOwnership, withInvitationSafeOrganizationAccessMutation, @@ -113,9 +113,8 @@ describe('paid-org join billing lock ordering', () => { it('locks the personal subscription before mutating userStats', async () => { const { tx, ops } = createRecordingTx() - dbChainMockFns.transaction.mockImplementation(async (cb: (t: unknown) => unknown) => cb(tx)) - await reapplyPaidOrgJoinBillingForExistingMember('user-1', 'org-1') + await reapplyPaidOrgJoinBillingForExistingMemberTx(tx as DbOrTx, 'user-1', 'org-1') const firstUserStatsUpdate = ops.findIndex((o) => o.op === 'update' && o.table === userStats) const subscriptionLock = ops.findIndex((o) => o.op === 'lock' && o.table === subscriptionTable) @@ -127,9 +126,8 @@ describe('paid-org join billing lock ordering', () => { it('still locks an already-paused personal Pro so a concurrent restore cannot pass it', async () => { const { tx, ops } = createRecordingTx({ ...GENERIC_ROW, cancelAtPeriodEnd: true }) - dbChainMockFns.transaction.mockImplementation(async (cb: (t: unknown) => unknown) => cb(tx)) - await reapplyPaidOrgJoinBillingForExistingMember('user-1', 'org-1') + await reapplyPaidOrgJoinBillingForExistingMemberTx(tx as DbOrTx, 'user-1', 'org-1') expect(ops.some((op) => op.op === 'lock' && op.table === subscriptionTable)).toBe(true) }) diff --git a/apps/sim/lib/billing/organizations/membership.ts b/apps/sim/lib/billing/organizations/membership.ts index 5a303bbb71b..1430c5d6732 100644 --- a/apps/sim/lib/billing/organizations/membership.ts +++ b/apps/sim/lib/billing/organizations/membership.ts @@ -882,34 +882,6 @@ async function applyPaidOrgJoinBillingTx( return actions } -/** - * Re-applies paid-org join billing for a user who is already a member of - * the organization. Used on re-upgrade after a dormant transition: members - * kept their org membership but had their personal Pro subscriptions - * restored (`cancelAtPeriodEnd=false`) during the cancel/downgrade. When - * the org becomes paid again, those Pros must be re-paused so the user - * isn't double-billed. - * - * No-op when the org has no active Team/Enterprise subscription. - */ -export async function reapplyPaidOrgJoinBillingForExistingMember( - userId: string, - organizationId: string -): Promise { - return db.transaction(async (tx) => { - await acquireOrganizationMutationLock(tx, organizationId) - const [existingMembership] = await tx - .select({ id: member.id }) - .from(member) - .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) - .limit(1) - if (!existingMembership) { - return { proUsageSnapshotted: false, proCancelledAtPeriodEnd: false } - } - return reapplyPaidOrgJoinBillingForExistingMemberTx(tx, userId, organizationId) - }) -} - /** * Transaction-enlisted variant used by subscription webhooks. Keeping the * subscription upsert, effective-limit update, provisioning completion, and @@ -2077,23 +2049,6 @@ export async function isSoleOwnerOfPaidOrganization(userId: string): Promise<{ } } -export async function isUserMemberOfOrganization( - userId: string, - organizationId: string -): Promise<{ isMember: boolean; role?: string; memberId?: string }> { - const [memberRecord] = await db - .select({ id: member.id, role: member.role }) - .from(member) - .where(and(eq(member.userId, userId), eq(member.organizationId, organizationId))) - .limit(1) - - if (memberRecord) { - return { isMember: true, role: memberRecord.role, memberId: memberRecord.id } - } - - return { isMember: false } -} - /** * Get user's current organization membership (if any). */ diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.test.ts b/apps/sim/lib/execution/payloads/large-value-metadata.test.ts index 2675a1f5bd2..4b1e4e7a45e 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.test.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.test.ts @@ -2,16 +2,18 @@ * @vitest-environment node */ +import { db } from '@sim/db' import { executionLargeValueDependencies, executionLargeValueReferences } from '@sim/db/schema' import { dbChainMockFns, resetDbChainMock } from '@sim/testing' import { eq, notInArray } from 'drizzle-orm' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' import { addLargeValueReference, + collectLargeValueReferenceKeys, MAX_LARGE_VALUE_REFERENCES_PER_SCOPE, pruneLargeValueMetadata, registerLargeValueOwner, - replaceLargeValueReferences, + replaceLargeValueReferenceKeysWithClient, } from '@/lib/execution/payloads/large-value-metadata' function largeValueKey(id: string, executionId = 'source-execution'): string { @@ -205,42 +207,44 @@ describe('large value metadata', () => { const otherWorkspaceKey = 'execution/workspace-2/workflow-1/source-execution/large-value-lv_abcdefghijkl.json' - await replaceLargeValueReferences( + const value = { + a: { + __simLargeValueRef: true, + version: 1, + id: 'lv_abcdefghijkl', + kind: 'json', + size: 123, + key: matchingKey, + }, + duplicate: { + __simLargeValueRef: true, + version: 1, + id: 'lv_abcdefghijkl', + kind: 'json', + size: 123, + key: matchingKey, + }, + ignored: { + __simLargeValueRef: true, + version: 1, + id: 'lv_abcdefghijkl', + kind: 'json', + size: 123, + key: otherWorkspaceKey, + }, + } + + await replaceLargeValueReferenceKeysWithClient( + db, { workspaceId: 'workspace-1', workflowId: 'workflow-1', executionId: 'execution-2', source: 'execution_log', }, - { - a: { - __simLargeValueRef: true, - version: 1, - id: 'lv_abcdefghijkl', - kind: 'json', - size: 123, - key: matchingKey, - }, - duplicate: { - __simLargeValueRef: true, - version: 1, - id: 'lv_abcdefghijkl', - kind: 'json', - size: 123, - key: matchingKey, - }, - ignored: { - __simLargeValueRef: true, - version: 1, - id: 'lv_abcdefghijkl', - kind: 'json', - size: 123, - key: otherWorkspaceKey, - }, - } + collectLargeValueReferenceKeys(value, 'workspace-1') ) - expect(dbChainMockFns.transaction).toHaveBeenCalledOnce() expect(dbChainMockFns.delete).toHaveBeenCalledOnce() expect(eq).toHaveBeenCalledWith(executionLargeValueReferences.source, 'execution_log') expect(dbChainMockFns.values).toHaveBeenCalledWith([ diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.ts b/apps/sim/lib/execution/payloads/large-value-metadata.ts index 5c1a9520113..dcbc03f4ff6 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.ts @@ -227,22 +227,6 @@ export async function registerLargeValueOwner( return true } -export async function replaceLargeValueReferencesWithClient( - client: LargeValueMetadataClient, - scope: LargeValueReferenceScope, - value: unknown -): Promise { - if (!scope.workspaceId || !scope.executionId) { - return - } - - await replaceLargeValueReferenceKeysWithClient( - client, - scope, - collectLargeValueReferenceKeys(value, scope.workspaceId) - ) -} - export async function replaceLargeValueReferenceKeysWithClient( client: LargeValueMetadataClient, scope: LargeValueReferenceScope, @@ -359,18 +343,6 @@ export async function addLargeValueReference( .onConflictDoNothing() } -export async function replaceLargeValueReferences( - scope: LargeValueReferenceScope, - value: unknown -): Promise { - const referenceKeys = scope.workspaceId - ? collectLargeValueReferenceKeys(value, scope.workspaceId) - : [] - await dbFor('exec').transaction(async (tx) => { - await replaceLargeValueReferenceKeysWithClient(tx, scope, referenceKeys) - }) -} - export async function markLargeValuesDeleted( keys: string[], dbClient: LargeValueMetadataClient = db