Skip to content

Commit e79bd40

Browse files
authored
fix(mothership): stop live runs when their chat, owner or access is removed (#9089)
* fix(mothership): stop live runs when their chat, owner or access is removed Deleting a chat, an account or a workspace, or removing someone's access, left the worker's run going: no path issued a Stop, and run control answered the missing chat or owner with a 404/403 the worker can only retry. - Run control answers Stop when the run's chat is deleted, its owner or workspace is gone, or the owner lost access; only a failure to read is an error - Chat delete, copilot chat delete, account deletion, workspace archive, member removal, organization removal, transfer and external-collaborator removal record Stop in their own transaction and push it after commit - Stop delivery is shared with the user's Stop - A takeover never claims a run whose chat was deleted * fix(mothership): deliver removal stops from callers outside page graphs Membership and workspace lifecycle modules sit in page server graphs, so they return the runs they stopped and their callers deliver the Stop. Fixes the organization run-control scope narrowing and shrinks the baselines this change paid down. * fix(mothership): keep stop delivery complete when its rows or marker are gone - Read a stopped run's client workflows and sandbox commands inside the removal's transaction, before a hard delete cascades them away - Abort this process's controller even when the abort marker cannot be written, and keep delivering the rest of the Stop - Account deletion no longer depends on the run-routing lookup - The operator archive repair keeps the delivery out of its script graph - Race test orders admission and deletion with a row-lock barrier; delivery assertions only count the test's own runs * fix(mothership): signal a stopped run before reading its work, and deliver each removal stop once - The user's Stop signals the worker and controller while its workflow and sandbox ownership is still being read - Workspace deletion starts delivering its Stop as soon as the archive commits, before the slower post-archive cleanup - External-collaborator removal delivers the Stop it recorded even when no access was left to remove - Leaving a workspace that also ends the organization membership delivers each run's Stop once * fix(mothership): never answer Stop for a refused delegation or an admin who keeps access - Run control rethrows a refusal of the caller's own delegation (expired mid-request or out of scope) instead of answering Stop; only the chat, owner or access standing answers Stop - Withdrawing a workspace grant stops the user's runs only when no effective access remains, so an organization admin keeps their run * fix(mothership): stop an organization admin's runs when a demotion ends their access Withdrawing a workspace grant and demoting an organization admin both end access only where no effective access remains, so the check runs after either: a role change that drops admin stops the member's runs in the organization's workspaces they can no longer reach, whatever order a directory sync withdraws the grant and the role in. * fix(mothership): keep stopped-run identities out of removal results, check access only where runs live - Organization member removal delivers its stopped runs and strips them from the result it returns to the route and audit - Withdrawing a grant or demoting an admin checks remaining access only in workspaces where the user has a live run, instead of every organization workspace under the mutation lock
1 parent 6a9a79d commit e79bd40

41 files changed

Lines changed: 1690 additions & 197 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎apps/sim/app/api/copilot/chat/abort/route.test.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,30 @@ describe('POST /api/copilot/chat/abort', () => {
135135
expect(mockAbortWorkflow).toHaveBeenCalledExactlyOnceWith('client-execution')
136136
})
137137

138+
it('still cancels the run’s workflows when the abort marker cannot be written', async () => {
139+
mockUnsettledWorkflows.mockResolvedValue(['client-execution'])
140+
mockAbortActiveStream.mockRejectedValueOnce(new Error('redis unavailable'))
141+
const response = await POST(abortRequest())
142+
expect(response.status).toBe(200)
143+
expect(mockRequestExplicitStreamAbort).toHaveBeenCalled()
144+
expect(mockCancelWorkflow).toHaveBeenCalledExactlyOnceWith('client-execution')
145+
})
146+
147+
it('signals the worker before the run’s workflow ownership has been read', async () => {
148+
let releaseRead: (ids: string[]) => void = () => {}
149+
mockUnsettledWorkflows.mockReturnValue(
150+
new Promise<string[]>((resolve) => {
151+
releaseRead = resolve
152+
})
153+
)
154+
const response = POST(abortRequest())
155+
await vi.waitFor(() => expect(mockRequestExplicitStreamAbort).toHaveBeenCalled())
156+
expect(mockAbortActiveStream).toHaveBeenCalled()
157+
releaseRead(['client-execution'])
158+
expect((await response).status).toBe(200)
159+
expect(mockCancelWorkflow).toHaveBeenCalledExactlyOnceWith('client-execution')
160+
})
161+
138162
it('does not signal workflows when the atomic Stop and admission closure cannot commit', async () => {
139163
mockRequestRunStop.mockRejectedValueOnce(new Error('database unavailable'))
140164
const response = await POST(abortRequest())

‎apps/sim/app/api/copilot/chat/delete/route.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
1-
import { db } from '@sim/db'
2-
import { copilotChats } from '@sim/db/schema'
31
import { createLogger } from '@sim/logger'
4-
import { and, eq } from 'drizzle-orm'
52
import { type NextRequest, NextResponse } from 'next/server'
63
import { deleteCopilotChatContract } from '@/lib/api/contracts/copilot'
74
import { parseRequest } from '@/lib/api/server'
85
import { getSession } from '@/lib/auth'
96
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
7+
import { deleteCopilotChat } from '@/lib/mothership/chat/deletion'
108
import { getAccessibleCopilotChatAuth } from '@/lib/mothership/chat/lifecycle'
119
import { chatPubSub } from '@/lib/mothership/chat-status'
1210

@@ -45,10 +43,7 @@ export const DELETE = withRouteHandler(async (request: NextRequest) => {
4543
)
4644
}
4745

48-
const [deleted] = await db
49-
.delete(copilotChats)
50-
.where(and(eq(copilotChats.id, parsed.chatId), eq(copilotChats.userId, session.user.id)))
51-
.returning({ workspaceId: copilotChats.workspaceId })
46+
const deleted = await deleteCopilotChat({ chatId: parsed.chatId, userId: session.user.id })
5247

5348
if (!deleted) {
5449
return NextResponse.json({ success: false, error: 'Chat not found' }, { status: 404 })

‎apps/sim/app/api/mothership/chats/[chatId]/route.ts‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
} from '@/lib/api/contracts/mothership-chats'
1212
import { parseRequest } from '@/lib/api/server'
1313
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
14+
import { softDeleteMothershipChat } from '@/lib/mothership/chat/deletion'
1415
import { buildEffectiveChatTranscript } from '@/lib/mothership/chat/effective-transcript'
1516
import {
1617
getAccessibleCopilotChatAuth,
@@ -255,21 +256,7 @@ export const DELETE = withRouteHandler(
255256
return NextResponse.json({ success: true })
256257
}
257258

258-
const [deletedChat] = await db
259-
.update(copilotChats)
260-
.set({ deletedAt: new Date() })
261-
.where(
262-
and(
263-
eq(copilotChats.id, chatId),
264-
eq(copilotChats.userId, userId),
265-
eq(copilotChats.type, 'mothership'),
266-
isNull(copilotChats.deletedAt)
267-
)
268-
)
269-
.returning({
270-
workspaceId: copilotChats.workspaceId,
271-
organizationId: copilotChats.organizationId,
272-
})
259+
const deletedChat = await softDeleteMothershipChat({ chatId, userId })
273260

274261
if (!deletedChat) {
275262
return NextResponse.json({ success: false, error: 'Chat not found' }, { status: 404 })

‎apps/sim/app/api/mothership/runs/control/route.test.ts‎

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
workspaceContextMockFns,
1616
} from '@sim/testing/mocks/workspace-context.mock'
1717
import { beforeEach, describe, expect, it, vi } from 'vitest'
18+
import { OrchestrationError } from '@/lib/core/orchestration/types'
1819

1920
vi.unmock('@/lib/mothership/request/http')
2021
vi.mock('@/lib/auth/ban', () => authBanMock)
@@ -73,17 +74,44 @@ describe('worker run control boundary', () => {
7374
expect(mocks.run).not.toHaveBeenCalled()
7475
expect(mocks.stopped).not.toHaveBeenCalled()
7576
})
76-
it('conceals a chat belonging to another actor', async () => {
77-
expect((await POST(request({ 'x-mothership-user-id': 'other-actor' }))).status).toBe(404)
77+
it("answers Stop for a chat that is not the actor's, without reading its state", async () => {
78+
const response = await POST(request({ 'x-mothership-user-id': 'other-actor' }))
79+
expect(response.status).toBe(200)
80+
expect(await response.json()).toEqual({ stopped: true })
7881
expect(mocks.stopped).not.toHaveBeenCalled()
7982
})
83+
it('answers Stop once the chat is deleted', async () => {
84+
resetDbChainMock()
85+
queueTableRows(schemaMock.copilotChats, [])
86+
const response = await POST(request())
87+
expect(response.status).toBe(200)
88+
expect(await response.json()).toEqual({ stopped: true })
89+
expect(mocks.stopped).not.toHaveBeenCalled()
90+
})
91+
it('answers Stop once the actor lost workspace access', async () => {
92+
mocks.authorize.mockRejectedValue(
93+
new OrchestrationError('forbidden', 'Insufficient workspace permissions')
94+
)
95+
const response = await POST(request())
96+
expect(response.status).toBe(200)
97+
expect(await response.json()).toEqual({ stopped: true })
98+
expect(mocks.stopped).not.toHaveBeenCalled()
99+
})
100+
it('lets a live run continue', async () => {
101+
mocks.stopped.mockResolvedValue(false)
102+
const response = await POST(request())
103+
expect(response.status).toBe(200)
104+
expect(await response.json()).toEqual({ stopped: false })
105+
})
80106
it.each([
81107
null,
82108
{ chatId: 'other-chat', workspaceId: 'workspace' },
83109
{ chatId, workspaceId: 'other-workspace' },
84-
])('refuses a missing or mismatched canonical run: %j', async (run) => {
110+
])('answers Stop for a missing or mismatched canonical run: %j', async (run) => {
85111
mocks.run.mockResolvedValue(run)
86-
expect((await POST(request())).status).toBe(404)
112+
const response = await POST(request())
113+
expect(response.status).toBe(200)
114+
expect(await response.json()).toEqual({ stopped: true })
87115
expect(mocks.stopped).not.toHaveBeenCalled()
88116
})
89117
it('does not turn a database outage into permission to continue', async () => {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
transferOrganizationOwnership,
1414
} from '@/lib/billing/organizations/membership'
1515
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
16+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
1617

1718
const logger = createLogger('TransferOwnershipAPI')
1819

@@ -150,6 +151,7 @@ export const POST = withRouteHandler(
150151
memberId: selfMember.id,
151152
spareSessionToken: session.session.token,
152153
})
154+
deliverRunStops(removeResult.stoppedRuns ?? [])
153155

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

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
} from '@/lib/billing/organizations/membership'
4444
import { isBillingEnabled } from '@/lib/core/config/env-flags'
4545
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
46+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
4647
import { withAdminAuthParams } from '@/app/api/v1/admin/middleware'
4748
import {
4849
adminBadRequestResponse,
@@ -271,6 +272,7 @@ export const DELETE = withRouteHandler(
271272
memberId,
272273
skipBillingLogic,
273274
})
275+
deliverRunStops(result.stoppedRuns ?? [])
274276

275277
if (!result.success) {
276278
if (result.error === 'Cannot remove organization owner') {

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import {
3333
} from '@/lib/api/contracts/v1/admin'
3434
import { parseRequest } from '@/lib/api/server'
3535
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
36+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
3637
import { ProjectConflictError } from '@/lib/projects/errors'
3738
import { revokeWorkspaceAccessTx } from '@/lib/workspaces/access/workspace-access'
3839
import { getWorkspaceById } from '@/lib/workspaces/permissions/utils'
@@ -281,12 +282,13 @@ export const DELETE = withRouteHandler(
281282
expectedPermissionId: existingMember.id,
282283
})
283284
if (!result.revoked) {
284-
if (result.reason === 'membership-changed') return false
285+
if (result.reason === 'membership-changed') return null
285286
throw new WorkspaceBillingAccountRemovalError()
286287
}
287-
return true
288+
return result
288289
})
289290
if (!removed) return adminNotFoundResponse('Workspace member')
291+
deliverRunStops(removed.stoppedRuns)
290292

291293
logger.info(`Admin API: Removed member ${memberId} from workspace ${workspaceId}`, {
292294
userId: existingMember.userId,

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ import {
5050
import { parseRequest } from '@/lib/api/server'
5151
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
5252
import { syncWorkspaceEnvCredentials } from '@/lib/credentials/environment'
53+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
5354
import { ProjectConflictError } from '@/lib/projects/errors'
5455
import { revokeWorkspaceAccessTx } from '@/lib/workspaces/access/workspace-access'
5556
import { getWorkspaceById } from '@/lib/workspaces/permissions/utils'
@@ -397,12 +398,13 @@ export const DELETE = withRouteHandler(
397398
expectedPermissionId: existingPermission.id,
398399
})
399400
if (!result.revoked) {
400-
if (result.reason === 'membership-changed') return false
401+
if (result.reason === 'membership-changed') return null
401402
throw new WorkspaceBillingAccountRemovalError()
402403
}
403-
return true
404+
return result
404405
})
405406
if (!removed) return adminNotFoundResponse('Workspace member')
407+
deliverRunStops(removed.stoppedRuns)
406408

407409
logger.info(`Admin API: Removed user ${userId} from workspace ${workspaceId}`)
408410

‎apps/sim/app/api/workspaces/[id]/route.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const logger = createLogger('WorkspaceByIdAPI')
1717
import { db } from '@sim/db'
1818
import { permissions, workspace } from '@sim/db/schema'
1919
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
20+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
2021
import {
2122
getEffectiveWorkspacePermission,
2223
getUserEntityPermissions,
@@ -284,6 +285,7 @@ export const DELETE = withRouteHandler(
284285

285286
const archiveResult = await archiveWorkspace(workspaceId, {
286287
requestId: `workspace-${workspaceId}`,
288+
deliverStops: deliverRunStops,
287289
})
288290

289291
/** Recorded first: a retry that finishes an earlier partial deletion still archives it. */

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { createLogger } from '@sim/logger'
55
import { and, eq } from 'drizzle-orm'
66
import { removeUserFromOrganization } from '@/lib/billing/organizations/membership'
77
import { reconcileOrganizationSeats } from '@/lib/billing/organizations/seats'
8+
import { deliverRunStops } from '@/lib/mothership/request/application/stop-delivery'
89
import {
910
defineAuthorizedScimUseCase,
1011
type ScimUseCaseArgs,
@@ -74,6 +75,7 @@ export const deprovisionScimUser = defineAuthorizedScimUseCase({
7475
memberId: membership.id,
7576
revokePersonalApiKeys: true,
7677
})
78+
deliverRunStops(removal.stoppedRuns ?? [])
7779
if (!removal.success) {
7880
throw new ScimError(409, undefined, removal.error ?? 'The member could not be removed')
7981
}

0 commit comments

Comments
 (0)