Skip to content

Commit cca2156

Browse files
committed
fix(scim): push Stop for runs a directory sync ends, after its transaction commits
A directory sync that withdrew a workspace grant or demoted an organization admin recorded Stop for the user's live Chat runs but never pushed it, so the runs ended only on the worker's next run-control read. - Projection deltas carry the runs their withdrawals stopped; withdrawGrant and setOrganizationRole return them instead of dropping them - Group writes and user updates deliver them once their transaction commits; the batched reconcile delivers each batch after it commits, so a rolled-back batch delivers nothing - Member removal, transfer and external-collaborator removal results carry stoppedRuns as a required field, returned from inside the transaction
1 parent 0c6c375 commit cca2156

17 files changed

Lines changed: 374 additions & 125 deletions

File tree

‎apps/sim/app/api/organizations/[id]/transfer-ownership/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ export const POST = withRouteHandler(
151151
memberId: selfMember.id,
152152
spareSessionToken: session.session.token,
153153
})
154-
deliverRunStops(removeResult.stoppedRuns ?? [])
154+
deliverRunStops(removeResult.stoppedRuns)
155155

156156
if (!removeResult.success) {
157157
logger.error('Transfer succeeded but self-removal failed', {

‎apps/sim/app/api/v1/admin/organizations/[id]/members/[memberId]/route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ export const DELETE = withRouteHandler(
272272
memberId,
273273
skipBillingLogic,
274274
})
275-
deliverRunStops(result.stoppedRuns ?? [])
275+
deliverRunStops(result.stoppedRuns)
276276

277277
if (!result.success) {
278278
if (result.error === 'Cannot remove organization owner') {

‎apps/sim/ee/scim/lib/application/groups/manage-groups.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ describe('replaceScimGroup', () => {
9494
memberIds.delete(scimUserId)
9595
)
9696
mocks.countMembers.mockImplementation(async () => memberIds.size)
97+
mocks.reconcile.mockResolvedValue({ added: [], removed: [], raised: [], stoppedRuns: [] })
9798
mocks.touch.mockImplementation(async () => {
9899
group = { ...group, updatedAt: new Date('2026-01-02T00:00:00.000Z') }
99100
})

‎apps/sim/ee/scim/lib/application/groups/manage-groups.ts‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { and, eq, ne } from 'drizzle-orm'
55
import type { ScimPatchOperation } from '@/lib/api/contracts/scim'
66
import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership'
77
import type { DbOrTx, DbTransaction } from '@/lib/db/types'
8+
import type { StoppedRun } from '@/lib/mothership/async-runs/repository'
9+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
810
import {
911
defineAuthorizedScimUseCase,
1012
type ScimUseCaseArgs,
@@ -50,16 +52,19 @@ import {
5052
/**
5153
* Every group write runs in one transaction under the organization lock, which
5254
* heads the documented lock order, so two full-membership PATCHes cannot both
53-
* compute from the same stale membership and keep members from both.
55+
* compute from the same stale membership and keep members from both. The runs
56+
* its projection stopped are delivered once that transaction commits.
5457
*/
55-
function withGroupWrite<T>(
58+
async function withGroupWrite<T extends { stoppedRuns: StoppedRun[] }>(
5659
context: ScimUseCaseContext,
5760
work: (tx: DbTransaction) => Promise<T>
58-
): Promise<T> {
59-
return db.transaction(async (tx) => {
61+
): Promise<Omit<T, 'stoppedRuns'>> {
62+
const { stoppedRuns, ...result } = await db.transaction(async (tx) => {
6063
await acquireOrganizationMutationLock(tx, context.organizationId)
6164
return work(tx)
6265
})
66+
deliverRunStops(stoppedRuns)
67+
return result
6368
}
6469

6570
async function assertDisplayNameAvailable(
@@ -205,7 +210,7 @@ export const createScimGroup = defineAuthorizedScimUseCase({
205210
})
206211
: 'no-match'
207212

208-
await reconcileUsersProjection(tx, {
213+
const projection = await reconcileUsersProjection(tx, {
209214
connectionId: context.connection.id,
210215
organizationId: context.organizationId,
211216
scimUserIds: memberIds,
@@ -225,6 +230,7 @@ export const createScimGroup = defineAuthorizedScimUseCase({
225230
resource: toGroupResource({ ...created, members }, context.baseUrl),
226231
touchedUserIds: memberIds,
227232
renamed: true,
233+
stoppedRuns: projection.stoppedRuns,
228234
}
229235
})
230236
},
@@ -295,7 +301,7 @@ export const replaceScimGroup = defineAuthorizedScimUseCase({
295301
await assertMemberCount(tx, current.id)
296302
if (touched.size > 0) await touchScimGroup(tx, current.id)
297303

298-
await reconcileUsersProjection(tx, {
304+
const projection = await reconcileUsersProjection(tx, {
299305
connectionId: context.connection.id,
300306
organizationId: context.organizationId,
301307
scimUserIds: [...touched],
@@ -317,6 +323,7 @@ export const replaceScimGroup = defineAuthorizedScimUseCase({
317323
resource: toGroupResource({ ...refreshed, members }, context.baseUrl),
318324
touchedUserIds: [...touched],
319325
renamed: renamed || (current.externalId ?? null) !== (refreshed.externalId ?? null),
326+
stoppedRuns: projection.stoppedRuns,
320327
}
321328
})
322329
},
@@ -428,7 +435,7 @@ export const patchScimGroup = defineAuthorizedScimUseCase({
428435
await assertMemberCount(tx, current.id)
429436
if (touched.size > 0) await touchScimGroup(tx, current.id)
430437

431-
await reconcileUsersProjection(tx, {
438+
const projection = await reconcileUsersProjection(tx, {
432439
connectionId: context.connection.id,
433440
organizationId: context.organizationId,
434441
scimUserIds: [...touched],
@@ -447,6 +454,7 @@ export const patchScimGroup = defineAuthorizedScimUseCase({
447454
patch.kind === 'full' ? (patch.displayName ?? current.displayName) : current.displayName,
448455
touchedUserIds: [...touched],
449456
renamed,
457+
stoppedRuns: projection.stoppedRuns,
450458
}
451459
})
452460
},
@@ -481,7 +489,7 @@ export const deleteScimGroup = defineAuthorizedScimUseCase({
481489
*/
482490
const memberIds = await loadGroupMemberIds(tx, current.id)
483491
await deleteScimGroupRow(tx, current.id)
484-
await reconcileUsersProjection(tx, {
492+
const projection = await reconcileUsersProjection(tx, {
485493
connectionId: context.connection.id,
486494
organizationId: context.organizationId,
487495
scimUserIds: memberIds,
@@ -491,6 +499,7 @@ export const deleteScimGroup = defineAuthorizedScimUseCase({
491499
groupId: current.id,
492500
displayName: current.displayName,
493501
memberCount: memberIds.length,
502+
stoppedRuns: projection.stoppedRuns,
494503
}
495504
})
496505
},

‎apps/sim/ee/scim/lib/application/users/deprovision-user.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export const deprovisionScimUser = defineAuthorizedScimUseCase({
7575
memberId: membership.id,
7676
revokePersonalApiKeys: true,
7777
})
78-
deliverRunStops(removal.stoppedRuns ?? [])
78+
deliverRunStops(removal.stoppedRuns)
7979
if (!removal.success) {
8080
throw new ScimError(409, undefined, removal.error ?? 'The member could not be removed')
8181
}

‎apps/sim/ee/scim/lib/application/users/provision-user.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ export const provisionScimUser = defineAuthorizedScimUseCase({
215215
connectionId: context.connection.id,
216216
externalId: attributes.externalId,
217217
})
218+
/** The new directory record has no provenance to withdraw, so this stops no runs. */
218219
await reconcileUserProjection(tx, {
219220
connectionId: context.connection.id,
220221
organizationId: context.organizationId,

‎apps/sim/ee/scim/lib/application/users/update-user.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ describe('user updates', () => {
133133
beforeEach(() => {
134134
resetDbChainMock()
135135
mocks.loadGroups.mockResolvedValue(new Map())
136-
mocks.reconcile.mockResolvedValue({ added: [], removed: [], raised: [] })
136+
mocks.reconcile.mockResolvedValue({ added: [], removed: [], raised: [], stoppedRuns: [] })
137137
})
138138

139139
it('serializes on the organization and user locks before computing the update', async () => {

‎apps/sim/ee/scim/lib/application/users/update-user.ts‎

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { normalizeEmail } from '@sim/utils/string'
55
import type { ScimPatchOperation } from '@/lib/api/contracts/scim'
66
import { acquireOrganizationUserMutationLocks } from '@/lib/billing/organizations/membership'
77
import type { DbOrTx, DbTransaction } from '@/lib/db/types'
8+
import type { StoppedRun } from '@/lib/mothership/async-runs/repository'
9+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
810
import { suspendMemberTx, unsuspendMemberTx } from '@/lib/organizations/members/lifecycle'
911
import {
1012
invalidateAfterSessionRevocation,
@@ -42,18 +44,24 @@ import {
4244
* deactivation actually does.
4345
*/
4446

45-
export interface UpdateOutcome {
47+
interface UpdateOutcome {
4648
emailChanged: boolean
4749
deactivated: boolean
4850
reactivated: boolean
4951
}
5052

53+
/** An applied update and the runs its projection stopped, delivered once the write commits. */
54+
interface AppliedUserUpdate {
55+
outcome: UpdateOutcome
56+
stoppedRuns: StoppedRun[]
57+
}
58+
5159
async function applyUserUpdate(
5260
tx: DbTransaction,
5361
context: ScimUseCaseContext,
5462
current: ScimUserRecord,
5563
next: ScimUserAttributes
56-
): Promise<UpdateOutcome> {
64+
): Promise<AppliedUserUpdate> {
5765
const nextEmail = primaryEmail(next)
5866
const nextName = accountName(next)
5967
const emailChanged = accountEmailDiverged(current, next)
@@ -108,14 +116,17 @@ async function applyUserUpdate(
108116
active: next.active,
109117
})
110118

111-
await reconcileUserProjection(tx, {
119+
const projection = await reconcileUserProjection(tx, {
112120
connectionId: context.connection.id,
113121
organizationId: context.organizationId,
114122
scimUserId: current.id,
115123
settings: context.connection.settings,
116124
})
117125

118-
return { emailChanged, deactivated, reactivated }
126+
return {
127+
outcome: { emailChanged, deactivated, reactivated },
128+
stoppedRuns: projection.stoppedRuns,
129+
}
119130
}
120131

121132
export interface UpdateScimUserResult {
@@ -125,6 +136,27 @@ export interface UpdateScimUserResult {
125136
resource: ReturnType<typeof toUserResource>
126137
}
127138

139+
/** Runs a user write in one transaction and delivers the runs it stopped once that commits. */
140+
async function writeUser(
141+
context: ScimUseCaseContext,
142+
scimUserId: string,
143+
update: (tx: DbTransaction, current: ScimUserRecord) => Promise<AppliedUserUpdate | null>
144+
): Promise<UpdateScimUserResult> {
145+
const { stoppedRuns, ...result } = await db.transaction(async (tx) => {
146+
const current = await loadUserForUpdate(tx, context, scimUserId)
147+
const applied = await update(tx, current)
148+
return {
149+
scimUserId: current.id,
150+
userId: current.userId,
151+
outcome: applied?.outcome ?? null,
152+
resource: await renderUpdated(tx, context.connection.id, current.id, context.baseUrl),
153+
stoppedRuns: applied?.stoppedRuns ?? [],
154+
}
155+
})
156+
deliverRunStops(stoppedRuns)
157+
return result
158+
}
159+
128160
/**
129161
* Loads the stored resource under the organization and user advisory locks.
130162
*
@@ -229,9 +261,7 @@ export const replaceScimUser = defineAuthorizedScimUseCase({
229261
input,
230262
context,
231263
}: ScimUseCaseArgs<ReplaceScimUserInput>): Promise<UpdateScimUserResult> {
232-
return db.transaction(async (tx) => {
233-
const current = await loadUserForUpdate(tx, context, input.scimUserId)
234-
264+
return writeUser(context, input.scimUserId, async (tx, current) => {
235265
/**
236266
* A replace keeps attributes Sim does not model that the directory sent on
237267
* a previous write but omitted now, so a partial mapping does not erase
@@ -249,16 +279,10 @@ export const replaceScimUser = defineAuthorizedScimUseCase({
249279
* changes nothing must not write, audit, or re-project, or a 2,000-user
250280
* organization produces 2,000 spurious audit rows per sync.
251281
*/
252-
const outcome =
253-
userAttributesEqual(current.attributes, next) && !accountIdentityDiverged(current, next)
254-
? null
255-
: await applyUserUpdate(tx, context, current, next)
256-
return {
257-
scimUserId: current.id,
258-
userId: current.userId,
259-
outcome,
260-
resource: await renderUpdated(tx, context.connection.id, current.id, context.baseUrl),
261-
}
282+
return userAttributesEqual(current.attributes, next) &&
283+
!accountIdentityDiverged(current, next)
284+
? null
285+
: applyUserUpdate(tx, context, current, next)
262286
})
263287
},
264288
projectAudit: ({ result }) => auditEntries(result),
@@ -277,9 +301,7 @@ export const patchScimUser = defineAuthorizedScimUseCase({
277301
input,
278302
context,
279303
}: ScimUseCaseArgs<PatchScimUserInput>): Promise<UpdateScimUserResult> {
280-
return db.transaction(async (tx) => {
281-
const current = await loadUserForUpdate(tx, context, input.scimUserId)
282-
304+
return writeUser(context, input.scimUserId, async (tx, current) => {
283305
const { next, changed } = applyUserPatch(current.attributes, input.operations)
284306

285307
/**
@@ -289,16 +311,9 @@ export const patchScimUser = defineAuthorizedScimUseCase({
289311
* projection pass, and a `lastModified` bump for a request that meant
290312
* nothing.
291313
*/
292-
const outcome =
293-
changed || accountIdentityDiverged(current, next)
294-
? await applyUserUpdate(tx, context, current, next)
295-
: null
296-
return {
297-
scimUserId: current.id,
298-
userId: current.userId,
299-
outcome,
300-
resource: await renderUpdated(tx, context.connection.id, current.id, context.baseUrl),
301-
}
314+
return changed || accountIdentityDiverged(current, next)
315+
? applyUserUpdate(tx, context, current, next)
316+
: null
302317
})
303318
},
304319
projectAudit: ({ result }) => auditEntries(result),

0 commit comments

Comments
 (0)