diff --git a/apps/sim/lib/workspaces/admin-move.test.ts b/apps/sim/lib/workspaces/admin-move.test.ts index f5bee867a8d..92f7d858949 100644 --- a/apps/sim/lib/workspaces/admin-move.test.ts +++ b/apps/sim/lib/workspaces/admin-move.test.ts @@ -46,6 +46,8 @@ const { sendInvitationEmail, countPendingSeatInvitations, resolveSeatCapacity, + collectWorkspaceCredentialSummary, + getSourceOrganization, } = vi.hoisted(() => ({ resolveMoveEntitlements: vi.fn(() => Promise.resolve({ @@ -73,8 +75,49 @@ const { sendInvitationEmail: vi.fn(), countPendingSeatInvitations: vi.fn(() => Promise.resolve(0)), resolveSeatCapacity: vi.fn(() => Promise.resolve(10)), + collectWorkspaceCredentialSummary: vi.fn(), + getSourceOrganization: vi.fn(), })) +const SOURCE_ORGANIZATION = { + id: 'org-source', + name: 'Source', + ownerId: 'source-owner', + ownerName: 'Source Owner', + ownerEmail: 'source-owner@example.com', +} + +const EMPTY_CREDENTIALS = { + items: [] as Array<{ + id: string + displayName: string + type: string + backedBySourceOrgMember: boolean + }>, + credentialGroupCount: 0, + environmentVariableKeys: [] as string[], + byokKeyCount: 0, + truncatedCredentials: 0, + truncatedEnvironmentVariableKeys: 0, +} + +const POPULATED_CREDENTIALS = { + ...EMPTY_CREDENTIALS, + items: [ + { id: 'credential-1', displayName: 'Slack', type: 'oauth', backedBySourceOrgMember: true }, + ], + credentialGroupCount: 1, + environmentVariableKeys: ['OPENAI_API_KEY'], + byokKeyCount: 2, +} + +/** A workspace whose secrets exceed the response bounds, so rows were dropped. */ +const TRUNCATED_CREDENTIALS = { + ...POPULATED_CREDENTIALS, + truncatedCredentials: 3, + truncatedEnvironmentVariableKeys: 7, +} + vi.mock('@sim/audit', () => ({ AuditAction: { WORKSPACE_UPDATED: 'workspace.updated', @@ -121,16 +164,7 @@ vi.mock('@/lib/table/billing', () => ({ invalidateWorkspaceTableLimitsCache })) vi.mock('@/lib/workflows/custom-blocks/operations', () => ({ deleteCustomBlock })) vi.mock('@/lib/workspaces/admin-move-source-impact', () => ({ cleanupSourceOrganizationArtifactsTx, - collectWorkspaceCredentialSummary: vi.fn(() => - Promise.resolve({ - items: [], - credentialGroupCount: 0, - environmentVariableKeys: [], - byokKeyCount: 0, - truncatedCredentials: 0, - truncatedEnvironmentVariableKeys: 0, - }) - ), + collectWorkspaceCredentialSummary, countRetentionRulesForWorkspace: vi.fn(() => ({ piiRedactionRules: 0, retentionOverrides: 0, @@ -140,15 +174,7 @@ vi.mock('@/lib/workspaces/admin-move-source-impact', () => ({ findRetainedCollaboratorCaps: vi.fn(() => Promise.resolve([])), findUnpublishableCustomBlocks, findSourceOrgCustomBlocksForWorkspace, - getSourceOrganization: vi.fn(() => - Promise.resolve({ - id: 'org-source', - name: 'Source', - ownerId: 'source-owner', - ownerName: 'Source Owner', - ownerEmail: 'source-owner@example.com', - }) - ), + getSourceOrganization, resolveMoveEntitlements, willBrandingChange: vi.fn(() => Promise.resolve(false)), })) @@ -210,6 +236,30 @@ function queueMoveSelects(workspaceRow: Record) { queueTableRows(organization, [destination]) } +/** + * The reload path reads the completed operation, then the workspace twice — the + * applied-state check and the summary reload — and the destination once. + */ +function queueMoveOperationSelects(audit: Record) { + queueTableRows(outboxEvent, [ + { + eventType: 'admin.workspace-move-operation', + status: 'completed', + payload: { + request: { + workspaceId: movedWorkspace.id, + destinationOrganizationId: destination.id, + expectedOwnerId: movedWorkspace.ownerId, + }, + audit, + }, + }, + ]) + queueTableRows(workspace, [movedWorkspace]) + queueTableRows(workspace, [movedWorkspace]) + queueTableRows(organization, [destination]) +} + afterAll(resetDbChainMock) beforeEach(() => { @@ -227,6 +277,8 @@ beforeEach(() => { destinationIsEnterprise: false, capabilitiesLost: [], }) + collectWorkspaceCredentialSummary.mockResolvedValue(EMPTY_CREDENTIALS) + getSourceOrganization.mockResolvedValue(SOURCE_ORGANIZATION) changeWorkspaceStoragePayerInTx.mockResolvedValue({ billableBytes: 128, newPayer: { type: 'organization', id: destination.id }, @@ -788,6 +840,205 @@ describe('moveWorkspaceToOrganization retries', () => { ) }) + /** + * A completed move records `sourceOrganizationId` even when it is `null`, so + * a reload can tell "this workspace came from a personal source" apart from + * "this operation predates the field". Collapsing the two made every reload + * of a personal-source move claim its origin had failed to persist. + */ + it('does not warn about an unpersisted source for a move recorded as personal', async () => { + queueMoveOperationSelects({ + actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' }, + previousBillingOwnerId: personalWorkspace.billedAccountUserId, + newBillingOwnerId: destination.ownerId, + organizationAssignedAt: '2026-08-20T00:00:00.000Z', + sourceOrganizationId: null, + }) + + const view = await getWorkspaceMoveOperation( + movedWorkspace.id, + destination.id, + movedWorkspace.ownerId, + 'operation-1' + ) + + expect(view.notices).toEqual([]) + expect(view.sourceOrganization).toBeNull() + }) + + it('still warns when the payload never recorded a source organization', async () => { + queueMoveOperationSelects({ + actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' }, + previousBillingOwnerId: personalWorkspace.billedAccountUserId, + newBillingOwnerId: destination.ownerId, + organizationAssignedAt: '2026-08-20T00:00:00.000Z', + }) + + const view = await getWorkspaceMoveOperation( + movedWorkspace.id, + destination.id, + movedWorkspace.ownerId, + 'operation-1' + ) + + expect(view.notices).toEqual([ + 'This move was recorded before the source organization was persisted, so it cannot be reported.', + ]) + }) + + it('reports the workspace credentials when a completed operation is reloaded', async () => { + collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS) + queueMoveOperationSelects({ + actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' }, + previousBillingOwnerId: personalWorkspace.billedAccountUserId, + newBillingOwnerId: destination.ownerId, + organizationAssignedAt: '2026-08-20T00:00:00.000Z', + sourceOrganizationId: 'org-source', + }) + + const view = await getWorkspaceMoveOperation( + movedWorkspace.id, + destination.id, + movedWorkspace.ownerId, + 'operation-1' + ) + + /** Resolved against the recorded source, so `backedBySourceOrgMember` means something. */ + expect(collectWorkspaceCredentialSummary).toHaveBeenCalledWith(movedWorkspace.id, 'org-source') + expect(view.credentials).toEqual(POPULATED_CREDENTIALS) + }) + + /** + * A recorded id whose organization has since been deleted is the third state: + * the payload answered, but the answer can no longer be resolved to a name. + */ + it('distinguishes a deleted source organization from an unrecorded one', async () => { + getSourceOrganization.mockResolvedValueOnce(null) + queueMoveOperationSelects({ + actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' }, + previousBillingOwnerId: personalWorkspace.billedAccountUserId, + newBillingOwnerId: destination.ownerId, + organizationAssignedAt: '2026-08-20T00:00:00.000Z', + sourceOrganizationId: 'org-source', + }) + + const view = await getWorkspaceMoveOperation( + movedWorkspace.id, + destination.id, + movedWorkspace.ownerId, + 'operation-1' + ) + + expect(view.sourceOrganization).toBeNull() + expect(view.notices).toEqual([ + 'The organization this workspace came from has since been deleted, so it can no longer be named.', + ]) + }) + + it('reports the workspace credentials in the applied summary', async () => { + queueMoveSelects(organizationWorkspace) + collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS) + + const summary = await moveWorkspaceToOrganization({ + workspaceId: organizationWorkspace.id, + destinationOrganizationId: destination.id, + adminEmail: 'admin@sim.ai', + durableOperationId: 'operation-1', + }) + + /** The PRE-move organization: that is what `backedBySourceOrgMember` compares against. */ + expect(collectWorkspaceCredentialSummary).toHaveBeenCalledWith( + organizationWorkspace.id, + 'org-source', + expect.anything() + ) + expect(summary.credentials).toEqual(POPULATED_CREDENTIALS) + /** Nothing was dropped, so the review is complete and says nothing about truncation. */ + expect(summary.sourceOrganizationImpact.truncated).toBeNull() + }) + + /** + * The applied path used to hardcode these two counters to zero, which would + * present a truncated credential list as a complete one. + */ + it('carries dropped credential counts into the applied truncation record', async () => { + queueMoveSelects(organizationWorkspace) + collectWorkspaceCredentialSummary.mockResolvedValueOnce(TRUNCATED_CREDENTIALS) + + const summary = await moveWorkspaceToOrganization({ + workspaceId: organizationWorkspace.id, + destinationOrganizationId: destination.id, + adminEmail: 'admin@sim.ai', + durableOperationId: 'operation-1', + }) + + expect(summary.sourceOrganizationImpact.truncated).toMatchObject({ + credentials: 3, + environmentVariableKeys: 7, + }) + }) + + it('carries dropped credential counts into a reloaded truncation record', async () => { + collectWorkspaceCredentialSummary.mockResolvedValueOnce(TRUNCATED_CREDENTIALS) + queueMoveOperationSelects({ + actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' }, + previousBillingOwnerId: personalWorkspace.billedAccountUserId, + newBillingOwnerId: destination.ownerId, + organizationAssignedAt: '2026-08-20T00:00:00.000Z', + sourceOrganizationId: 'org-source', + }) + + const view = await getWorkspaceMoveOperation( + movedWorkspace.id, + destination.id, + movedWorkspace.ownerId, + 'operation-1' + ) + + expect(view.sourceOrganizationImpact.truncated).toMatchObject({ + credentials: 3, + environmentVariableKeys: 7, + }) + }) + + it('reports the workspace credentials on a retry of a completed move', async () => { + queueMoveSelects(movedWorkspace) + queueTableRows(outboxEvent, [ + { + eventType: 'admin.workspace-move-operation', + status: 'completed', + payload: { + request: { + workspaceId: movedWorkspace.id, + destinationOrganizationId: destination.id, + expectedOwnerId: movedWorkspace.ownerId, + }, + audit: { + actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' }, + previousBillingOwnerId: personalWorkspace.billedAccountUserId, + newBillingOwnerId: destination.ownerId, + organizationAssignedAt: '2026-08-20T00:00:00.000Z', + sourceOrganizationId: null, + }, + }, + }, + ]) + collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS) + + const summary = await moveWorkspaceToOrganization({ + workspaceId: movedWorkspace.id, + destinationOrganizationId: destination.id, + adminEmail: 'admin@sim.ai', + expectedOwnerId: movedWorkspace.ownerId, + auditOperationId: 'operation-1', + operationCorrelationId: 'operation-1', + durableOperationId: 'operation-1', + }) + + expect(summary.credentials).toEqual(POPULATED_CREDENTIALS) + expect(summary.notices).toEqual([]) + }) + it('takes shared advisory locks before the workspace row lock and payer mutation', async () => { queueMoveSelects(personalWorkspace) diff --git a/apps/sim/lib/workspaces/admin-move.ts b/apps/sim/lib/workspaces/admin-move.ts index b795187b34d..2a662d07d2e 100644 --- a/apps/sim/lib/workspaces/admin-move.ts +++ b/apps/sim/lib/workspaces/admin-move.ts @@ -407,7 +407,14 @@ function parseAdminWorkspaceMoveOperationPayload( previousBillingOwnerId: auditRecord.previousBillingOwnerId, newBillingOwnerId: auditRecord.newBillingOwnerId, organizationAssignedAt: auditRecord.organizationAssignedAt, - sourceOrganizationId: (auditRecord.sourceOrganizationId as string | null | undefined) ?? null, + /** + * Deliberately NOT collapsed to `null`. A recorded `null` is an answer — + * the workspace came from a personal source, so there is no organization + * to name and nothing was lost. Only an absent key leaves the origin + * unknown, and merging the two made every reload of a personal-source + * move report that its source organization had failed to persist. + */ + sourceOrganizationId: auditRecord.sourceOrganizationId as string | null | undefined, unpublishedCustomBlocks: (auditRecord.unpublishedCustomBlocks as | Array<{ id: string; type: string; name: string }> @@ -417,6 +424,62 @@ function parseAdminWorkspaceMoveOperationPayload( } } +/** + * Where a completed move came from, rebuilt from its durable payload alone. + * The payer transfer has already overwritten `workspace.organizationId` by the + * time any of these paths run, so the payload is the only surviving record. + * + * `unknown` is true only when the payload genuinely cannot answer: it predates + * {@link AdminWorkspaceMoveOperationPayload.audit.sourceOrganizationId}, or it + * names an organization that has since been deleted. A personal source is a + * recorded answer, not a gap, and must not be reported as one. + */ +async function resolveRecordedSourceOrganization( + audit: AdminWorkspaceMoveOperationPayload['audit'] | null, + executor: DbOrTx +): Promise<{ + id: string | null + organization: WorkspaceMoveSourceOrganization | null + /** False only for a payload written before the field existed. */ + recorded: boolean + unknown: boolean +}> { + const recorded = audit ? audit.sourceOrganizationId !== undefined : false + const id = audit?.sourceOrganizationId ?? null + const organization = id ? await getSourceOrganization(id, executor) : null + return { + id, + organization, + recorded, + unknown: !recorded || (id !== null && organization === null), + } +} + +/** + * The truncation record for an applied summary, merging what the move's own + * lists dropped with what the credential summary dropped — the same merge + * preflight performs, so a partial applied review is never presented as a + * complete one. + */ +function buildAppliedTruncation(params: { + unpublishedCustomBlocks: number + detachedPermissionGroups: number + credentials: WorkspaceMoveCredentialSummary +}): WorkspaceMoveSourceImpact['truncated'] { + const truncated = { + customBlocks: Math.max(params.unpublishedCustomBlocks - PREFLIGHT_LIST_LIMITS.customBlocks, 0), + permissionGroups: Math.max( + params.detachedPermissionGroups - PREFLIGHT_LIST_LIMITS.permissionGroups, + 0 + ), + collaboratorCaps: 0, + forkEdges: 0, + credentials: params.credentials.truncatedCredentials, + environmentVariableKeys: params.credentials.truncatedEnvironmentVariableKeys, + } + return Object.values(truncated).some((dropped) => dropped > 0) ? truncated : null +} + function workspaceMoveOperationMatches( payload: unknown, params: AdminWorkspaceMoveOperationRequest @@ -1033,15 +1096,22 @@ export async function moveWorkspaceToOrganization(params: { * them — discarding it here made the retry claim the source was * unrecoverable while the payload was sitting right there. */ - const recordedSourceOrgId = recordedAudit?.sourceOrganizationId ?? null - const replayedSourceOrganization = recordedSourceOrgId - ? await getSourceOrganization(recordedSourceOrgId, tx) - : null + const recordedSource = await resolveRecordedSourceOrganization(recordedAudit, tx) + /** + * The workspace's own secrets are untouched by a move and by this + * no-op retry, so they are read rather than blanked: a retry that + * reported zero credentials told the admin the workspace had none. + */ + const replayedCredentials = await collectWorkspaceCredentialSummary( + params.workspaceId, + recordedSource.id, + tx + ) return { performedMove: false, - sourceOrganizationOutcome: recordedSourceOrgId + sourceOrganizationOutcome: recordedSource.id ? { - sourceOrganizationId: recordedSourceOrgId, + sourceOrganizationId: recordedSource.id, unpublishedCustomBlocks: recordedAudit?.unpublishedCustomBlocks ?? [], detachedPermissionGroupIds: recordedAudit?.detachedPermissionGroupIds ?? [], } @@ -1052,19 +1122,26 @@ export async function moveWorkspaceToOrganization(params: { durableAudit: recordedAudit, invitationEvents: [], summary: await getMovedWorkspaceSummary(tx, params.workspaceId, destination, { - sourceOrganization: replayedSourceOrganization, - sourceOrganizationImpact: EMPTY_SOURCE_IMPACT, - credentials: EMPTY_CREDENTIAL_SUMMARY, + sourceOrganization: recordedSource.organization, + sourceOrganizationImpact: { + ...EMPTY_SOURCE_IMPACT, + truncated: buildAppliedTruncation({ + unpublishedCustomBlocks: 0, + detachedPermissionGroups: 0, + credentials: replayedCredentials, + }), + }, + credentials: replayedCredentials, entitlements: { sourceIsEnterprise: false, destinationIsEnterprise: false, capabilitiesLost: [], }, - notices: replayedSourceOrganization - ? [] - : [ + notices: recordedSource.unknown + ? [ 'This workspace was already in the destination organization, so the organization it originally came from is no longer recoverable.', - ], + ] + : [], }), } satisfies MoveTransactionResult } @@ -1339,6 +1416,19 @@ export async function moveWorkspaceToOrganization(params: { set: { permissionType: 'admin', updatedAt: now }, }) + /** + * Read against the PRE-move source organization, which is what + * `backedBySourceOrgMember` means. Every row this counts is workspace- + * scoped and travels with the move untouched, so unlike the source + * impact it is still fully reportable here — blanking it told the admin + * who just confirmed the move that the workspace carried no secrets. + */ + const movedCredentials = await collectWorkspaceCredentialSummary( + params.workspaceId, + sourceOrganizationId, + tx + ) + return { performedMove: true, previousBillingOwnerId: workspaceRow.billedAccountUserId, @@ -1385,27 +1475,13 @@ export async function moveWorkspaceToOrganization(params: { PREFLIGHT_LIST_LIMITS.permissionGroups ).items.map((permissionGroupId) => ({ permissionGroupId, name: '' })), /** The applied response is bounded by the same limits as preflight. */ - truncated: - unpublishedCustomBlocks.length > PREFLIGHT_LIST_LIMITS.customBlocks || - cleanup.detachedPermissionGroupIds.length > PREFLIGHT_LIST_LIMITS.permissionGroups - ? { - customBlocks: Math.max( - unpublishedCustomBlocks.length - PREFLIGHT_LIST_LIMITS.customBlocks, - 0 - ), - permissionGroups: Math.max( - cleanup.detachedPermissionGroupIds.length - - PREFLIGHT_LIST_LIMITS.permissionGroups, - 0 - ), - collaboratorCaps: 0, - forkEdges: 0, - credentials: 0, - environmentVariableKeys: 0, - } - : null, + truncated: buildAppliedTruncation({ + unpublishedCustomBlocks: unpublishedCustomBlocks.length, + detachedPermissionGroups: cleanup.detachedPermissionGroupIds.length, + credentials: movedCredentials, + }), }, - credentials: EMPTY_CREDENTIAL_SUMMARY, + credentials: movedCredentials, /** * The fenced result, not the optimistic one: the response must * describe the entitlement state the move was actually allowed @@ -1827,20 +1903,17 @@ export async function getWorkspaceMoveOperation( * response, which is exactly when the operator most needs to see what the * move did and where it came from. */ - const recordedSourceOrganizationId = operationPayload.audit.sourceOrganizationId ?? null - const sourceOrganization = recordedSourceOrganizationId - ? await getSourceOrganization(recordedSourceOrganizationId) - : null + const recordedSource = await resolveRecordedSourceOrganization(operationPayload.audit, db) /** * Replay the source organization's loss audit. `recordAuditOnce` keys make it * idempotent, so this is a no-op when the original write landed and a repair * when the process died between commit and that fire-and-forget write. */ - if (recordedSourceOrganizationId) { + if (recordedSource.id) { await recordSourceOrganizationMoveAudit({ workspaceId, - sourceOrganizationId: recordedSourceOrganizationId, + sourceOrganizationId: recordedSource.id, destinationOrganizationId, adminEmail: operationPayload.audit.actor.email ?? 'admin-api@sim.ai', auditActor: operationPayload.audit.actor, @@ -1850,21 +1923,42 @@ export async function getWorkspaceMoveOperation( }) } + /** + * The workspace's secrets are workspace-scoped and travel with the move, so + * the reload reads them rather than reporting a blank. `backedBySourceOrgMember` + * resolves against the recorded source; a personal or unrecorded source has no + * members to match, which is exactly what a `null` id asks for. + */ + const credentials = await collectWorkspaceCredentialSummary(workspaceId, recordedSource.id) + return toWorkspaceMoveOperationView( await getMovedWorkspaceSummary(db, workspaceId, destination, { - sourceOrganization, - sourceOrganizationImpact: EMPTY_SOURCE_IMPACT, - credentials: EMPTY_CREDENTIAL_SUMMARY, + sourceOrganization: recordedSource.organization, + sourceOrganizationImpact: { + ...EMPTY_SOURCE_IMPACT, + truncated: buildAppliedTruncation({ + unpublishedCustomBlocks: 0, + detachedPermissionGroups: 0, + credentials, + }), + }, + credentials, entitlements: { sourceIsEnterprise: false, destinationIsEnterprise: false, capabilitiesLost: [], }, - notices: recordedSourceOrganizationId - ? [] - : [ - 'This move was recorded before the source organization was persisted, so it cannot be reported.', - ], + /** + * A recorded `null` means the workspace came from a personal source and + * there is nothing to name — not that the record is defective. + */ + notices: recordedSource.unknown + ? [ + recordedSource.recorded + ? 'The organization this workspace came from has since been deleted, so it can no longer be named.' + : 'This move was recorded before the source organization was persisted, so it cannot be reported.', + ] + : [], }), operationId )