Skip to content

Commit dcd24d1

Browse files
committed
fix(admin): report a completed workspace move's true source and its credentials
The durable move payload records `sourceOrganizationId` as a tri-state: absent means the operation predates the field, an explicit `null` means the workspace came from a personal source. The payload parser collapsed both to `null`, so every reload or retry of a personal-source move claimed its source organization had failed to persist. The applied and reloaded responses also blanked the credential summary. Unlike the source impact, those rows are workspace-scoped and travel with the move untouched, so an admin who had just confirmed a move was told the workspace carried no secrets, environment variables, or BYOK keys.
1 parent 553849a commit dcd24d1

2 files changed

Lines changed: 320 additions & 58 deletions

File tree

apps/sim/lib/workspaces/admin-move.test.ts

Lines changed: 178 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ const {
4646
sendInvitationEmail,
4747
countPendingSeatInvitations,
4848
resolveSeatCapacity,
49+
collectWorkspaceCredentialSummary,
4950
} = vi.hoisted(() => ({
5051
resolveMoveEntitlements: vi.fn(() =>
5152
Promise.resolve({
@@ -73,8 +74,33 @@ const {
7374
sendInvitationEmail: vi.fn(),
7475
countPendingSeatInvitations: vi.fn(() => Promise.resolve(0)),
7576
resolveSeatCapacity: vi.fn(() => Promise.resolve(10)),
77+
collectWorkspaceCredentialSummary: vi.fn(),
7678
}))
7779

80+
const EMPTY_CREDENTIALS = {
81+
items: [] as Array<{
82+
id: string
83+
displayName: string
84+
type: string
85+
backedBySourceOrgMember: boolean
86+
}>,
87+
credentialGroupCount: 0,
88+
environmentVariableKeys: [] as string[],
89+
byokKeyCount: 0,
90+
truncatedCredentials: 0,
91+
truncatedEnvironmentVariableKeys: 0,
92+
}
93+
94+
const POPULATED_CREDENTIALS = {
95+
...EMPTY_CREDENTIALS,
96+
items: [
97+
{ id: 'credential-1', displayName: 'Slack', type: 'oauth', backedBySourceOrgMember: true },
98+
],
99+
credentialGroupCount: 1,
100+
environmentVariableKeys: ['OPENAI_API_KEY'],
101+
byokKeyCount: 2,
102+
}
103+
78104
vi.mock('@sim/audit', () => ({
79105
AuditAction: {
80106
WORKSPACE_UPDATED: 'workspace.updated',
@@ -121,16 +147,7 @@ vi.mock('@/lib/table/billing', () => ({ invalidateWorkspaceTableLimitsCache }))
121147
vi.mock('@/lib/workflows/custom-blocks/operations', () => ({ deleteCustomBlock }))
122148
vi.mock('@/lib/workspaces/admin-move-source-impact', () => ({
123149
cleanupSourceOrganizationArtifactsTx,
124-
collectWorkspaceCredentialSummary: vi.fn(() =>
125-
Promise.resolve({
126-
items: [],
127-
credentialGroupCount: 0,
128-
environmentVariableKeys: [],
129-
byokKeyCount: 0,
130-
truncatedCredentials: 0,
131-
truncatedEnvironmentVariableKeys: 0,
132-
})
133-
),
150+
collectWorkspaceCredentialSummary,
134151
countRetentionRulesForWorkspace: vi.fn(() => ({
135152
piiRedactionRules: 0,
136153
retentionOverrides: 0,
@@ -210,6 +227,30 @@ function queueMoveSelects(workspaceRow: Record<string, unknown>) {
210227
queueTableRows(organization, [destination])
211228
}
212229

230+
/**
231+
* The reload path reads the completed operation, then the workspace twice — the
232+
* applied-state check and the summary reload — and the destination once.
233+
*/
234+
function queueMoveOperationSelects(audit: Record<string, unknown>) {
235+
queueTableRows(outboxEvent, [
236+
{
237+
eventType: 'admin.workspace-move-operation',
238+
status: 'completed',
239+
payload: {
240+
request: {
241+
workspaceId: movedWorkspace.id,
242+
destinationOrganizationId: destination.id,
243+
expectedOwnerId: movedWorkspace.ownerId,
244+
},
245+
audit,
246+
},
247+
},
248+
])
249+
queueTableRows(workspace, [movedWorkspace])
250+
queueTableRows(workspace, [movedWorkspace])
251+
queueTableRows(organization, [destination])
252+
}
253+
213254
afterAll(resetDbChainMock)
214255

215256
beforeEach(() => {
@@ -227,6 +268,7 @@ beforeEach(() => {
227268
destinationIsEnterprise: false,
228269
capabilitiesLost: [],
229270
})
271+
collectWorkspaceCredentialSummary.mockResolvedValue(EMPTY_CREDENTIALS)
230272
changeWorkspaceStoragePayerInTx.mockResolvedValue({
231273
billableBytes: 128,
232274
newPayer: { type: 'organization', id: destination.id },
@@ -788,6 +830,132 @@ describe('moveWorkspaceToOrganization retries', () => {
788830
)
789831
})
790832

833+
/**
834+
* A completed move records `sourceOrganizationId` even when it is `null`, so
835+
* a reload can tell "this workspace came from a personal source" apart from
836+
* "this operation predates the field". Collapsing the two made every reload
837+
* of a personal-source move claim its origin had failed to persist.
838+
*/
839+
it('does not warn about an unpersisted source for a move recorded as personal', async () => {
840+
queueMoveOperationSelects({
841+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
842+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
843+
newBillingOwnerId: destination.ownerId,
844+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
845+
sourceOrganizationId: null,
846+
})
847+
848+
const view = await getWorkspaceMoveOperation(
849+
movedWorkspace.id,
850+
destination.id,
851+
movedWorkspace.ownerId,
852+
'operation-1'
853+
)
854+
855+
expect(view.notices).toEqual([])
856+
expect(view.sourceOrganization).toBeNull()
857+
})
858+
859+
it('still warns when the payload never recorded a source organization', async () => {
860+
queueMoveOperationSelects({
861+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
862+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
863+
newBillingOwnerId: destination.ownerId,
864+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
865+
})
866+
867+
const view = await getWorkspaceMoveOperation(
868+
movedWorkspace.id,
869+
destination.id,
870+
movedWorkspace.ownerId,
871+
'operation-1'
872+
)
873+
874+
expect(view.notices).toEqual([
875+
'This move was recorded before the source organization was persisted, so it cannot be reported.',
876+
])
877+
})
878+
879+
it('reports the workspace credentials when a completed operation is reloaded', async () => {
880+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS)
881+
queueMoveOperationSelects({
882+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
883+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
884+
newBillingOwnerId: destination.ownerId,
885+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
886+
sourceOrganizationId: 'org-source',
887+
})
888+
889+
const view = await getWorkspaceMoveOperation(
890+
movedWorkspace.id,
891+
destination.id,
892+
movedWorkspace.ownerId,
893+
'operation-1'
894+
)
895+
896+
/** Resolved against the recorded source, so `backedBySourceOrgMember` means something. */
897+
expect(collectWorkspaceCredentialSummary).toHaveBeenCalledWith(movedWorkspace.id, 'org-source')
898+
expect(view.credentials).toEqual(POPULATED_CREDENTIALS)
899+
})
900+
901+
it('reports the workspace credentials in the applied summary', async () => {
902+
queueMoveSelects(organizationWorkspace)
903+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS)
904+
905+
const summary = await moveWorkspaceToOrganization({
906+
workspaceId: organizationWorkspace.id,
907+
destinationOrganizationId: destination.id,
908+
adminEmail: 'admin@sim.ai',
909+
durableOperationId: 'operation-1',
910+
})
911+
912+
/** The PRE-move organization: that is what `backedBySourceOrgMember` compares against. */
913+
expect(collectWorkspaceCredentialSummary).toHaveBeenCalledWith(
914+
organizationWorkspace.id,
915+
'org-source',
916+
expect.anything()
917+
)
918+
expect(summary.credentials).toEqual(POPULATED_CREDENTIALS)
919+
})
920+
921+
it('reports the workspace credentials on a retry of a completed move', async () => {
922+
queueMoveSelects(movedWorkspace)
923+
queueTableRows(outboxEvent, [
924+
{
925+
eventType: 'admin.workspace-move-operation',
926+
status: 'completed',
927+
payload: {
928+
request: {
929+
workspaceId: movedWorkspace.id,
930+
destinationOrganizationId: destination.id,
931+
expectedOwnerId: movedWorkspace.ownerId,
932+
},
933+
audit: {
934+
actor: { id: null, name: 'Admin Panel', email: 'admin@sim.ai' },
935+
previousBillingOwnerId: personalWorkspace.billedAccountUserId,
936+
newBillingOwnerId: destination.ownerId,
937+
organizationAssignedAt: '2026-08-20T00:00:00.000Z',
938+
sourceOrganizationId: null,
939+
},
940+
},
941+
},
942+
])
943+
collectWorkspaceCredentialSummary.mockResolvedValueOnce(POPULATED_CREDENTIALS)
944+
945+
const summary = await moveWorkspaceToOrganization({
946+
workspaceId: movedWorkspace.id,
947+
destinationOrganizationId: destination.id,
948+
adminEmail: 'admin@sim.ai',
949+
expectedOwnerId: movedWorkspace.ownerId,
950+
auditOperationId: 'operation-1',
951+
operationCorrelationId: 'operation-1',
952+
durableOperationId: 'operation-1',
953+
})
954+
955+
expect(summary.credentials).toEqual(POPULATED_CREDENTIALS)
956+
expect(summary.notices).toEqual([])
957+
})
958+
791959
it('takes shared advisory locks before the workspace row lock and payer mutation', async () => {
792960
queueMoveSelects(personalWorkspace)
793961

0 commit comments

Comments
 (0)