Skip to content

Commit 0bae22b

Browse files
authored
fix(platform): stable block chat ids, held-listing sync status, session cleanup on account deletion (#7755)
* fix(mothership): derive block chat ids from the conversation id The Sim block passed its Conversation ID input straight through as the copilot chat id. Builders use stable strings such as `customer-456` there, but every chat column in Sim is a uuid, so each read keyed by the id failed, and the copilot service keys conversations by id alone, so two workspaces choosing the same string shared one thread. Every conversation id is now mapped to a UUID v5 of `workspaceId:id` under a fixed namespace. The same value keeps the same thread, workspaces never collide, a block can only reach a chat it derived, and the literal value never leaves the executor. An omitted id mints a token that the block exposes so a chained block continues the thread. The fork feature's UUID v5 helper moves to a shared module. * fix(knowledge): keep a held deletion pass a completed sync Since #7477 `completeSuccessfulSync` folded `checkpoint.unsafe` into the same predicate as an unfinished listing and a failed source read, so every connector whose provider cannot promise a stable listing (Fireflies and Notion always, Gmail and Slack at their caps) was recorded as `partial` with a frozen `last_sync_at` even when every document listed. `unsafe` only ever meant "do not infer deletions from this listing", and the deletion hold in `reconcileCompletedListing` still honors it. One `isContentPassIncomplete` predicate now drives the sync-log status, the watermark advance, and the task outcome, and it excludes `unsafe`, restoring the pre-#7477 behavior at all four sites. * fix(auth): end the session on account deletion and refuse a deleted owner Deleting an account removes the user and session rows, but the signed cookie cache keeps authenticating that browser for up to five minutes. The settings page then clears the query cache concurrently with signing out, every mounted query refetches, and the workspace list finds nothing and tries to create a default workspace for a user who no longer exists. The insert fails on the `workspace` -> `user` foreign key and surfaces as an unhandled 500 on roughly half of all deletions. The deletion response now clears the session cookies itself, so no later request from that browser carries them. The route builder keeps each cleared cookie on its own header line. Workspace creation classifies a user foreign key violation as `WorkspaceOwnerMissingError`, which both workspace routes answer with 401 instead of a logged fault.
1 parent 0262662 commit 0bae22b

20 files changed

Lines changed: 534 additions & 84 deletions

File tree

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
6+
const mocks = vi.hoisted(() => ({
7+
defineInternalJsonRoute: vi.fn(() => vi.fn()),
8+
signOut: vi.fn(),
9+
}))
10+
11+
vi.mock('@/lib/api/server/routes', () => ({
12+
defineInternalJsonRoute: mocks.defineInternalJsonRoute,
13+
internalOrchestrationErrorPolicy: { project: vi.fn(), unhandled: vi.fn() },
14+
internalRateLimits: { none: vi.fn(() => ({ kind: 'none' })) },
15+
internalSessionAuth: { authenticate: vi.fn() },
16+
}))
17+
18+
vi.mock('@/lib/auth', () => ({
19+
auth: { api: { signOut: mocks.signOut } },
20+
getSession: vi.fn(),
21+
}))
22+
23+
vi.mock('@/lib/users/application/delete-account', () => ({
24+
deleteAccountUseCase: { execute: vi.fn() },
25+
previewAccountDeletionUseCase: { execute: vi.fn() },
26+
}))
27+
28+
vi.mock('@/lib/users/application/operations', () => ({
29+
userAccountOperations: { delete: { id: 'user.delete' }, previewDeletion: { id: 'user.preview' } },
30+
}))
31+
32+
import '@/app/api/users/me/deletion/route'
33+
34+
type RouteOptions = {
35+
finalizeResponse?: (args: { request: Request }) => Promise<{ headers?: HeadersInit }>
36+
}
37+
38+
/** Captured at import time; the route registers itself once when the module loads. */
39+
const routeOptions = mocks.defineInternalJsonRoute.mock.calls.map((call) => call[0] as RouteOptions)
40+
41+
describe('POST /api/users/me/deletion', () => {
42+
beforeEach(() => {
43+
mocks.signOut.mockReset()
44+
})
45+
46+
/**
47+
* The session row is deleted with the account, but the signed cookie cache
48+
* still authenticates the browser for its TTL; the response must clear it.
49+
*/
50+
it('clears the session cookies on the deletion response', async () => {
51+
const options = routeOptions.find(
52+
(candidate) => typeof candidate.finalizeResponse === 'function'
53+
)
54+
expect(options?.finalizeResponse).toBeDefined()
55+
56+
const cleared = new Headers([['set-cookie', 'better-auth.session_token=; Max-Age=0']])
57+
mocks.signOut.mockResolvedValue({ headers: cleared, response: { success: true } })
58+
const request = new Request('http://localhost/api/users/me/deletion', {
59+
method: 'POST',
60+
headers: { cookie: 'better-auth.session_token=abc' },
61+
})
62+
63+
const finalization = await options!.finalizeResponse!({ request })
64+
65+
expect(mocks.signOut).toHaveBeenCalledWith({ headers: request.headers, returnHeaders: true })
66+
expect(finalization.headers).toBe(cleared)
67+
})
68+
69+
it('never fails a completed deletion because the cookies could not be cleared', async () => {
70+
const options = routeOptions.find(
71+
(candidate) => typeof candidate.finalizeResponse === 'function'
72+
)
73+
mocks.signOut.mockRejectedValue(new Error('sign-out unavailable'))
74+
75+
await expect(
76+
options!.finalizeResponse!({
77+
request: new Request('http://localhost/api/users/me/deletion', { method: 'POST' }),
78+
})
79+
).resolves.toEqual({})
80+
})
81+
})

apps/sim/app/api/users/me/deletion/route.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,20 @@
1+
import { createLogger } from '@sim/logger'
12
import { deleteAccountContract, getAccountDeletionPlanContract } from '@/lib/api/contracts'
23
import {
34
defineInternalJsonRoute,
45
internalOrchestrationErrorPolicy,
56
internalRateLimits,
67
internalSessionAuth,
78
} from '@/lib/api/server/routes'
9+
import { auth } from '@/lib/auth'
810
import {
911
deleteAccountUseCase,
1012
previewAccountDeletionUseCase,
1113
} from '@/lib/users/application/delete-account'
1214
import { userAccountOperations } from '@/lib/users/application/operations'
1315

16+
const logger = createLogger('AccountDeletionRoute')
17+
1418
export const dynamic = 'force-dynamic'
1519

1620
export const GET = defineInternalJsonRoute({
@@ -39,4 +43,19 @@ export const POST = defineInternalJsonRoute({
3943
mapInput: ({ body }) => ({ confirmEmail: body.confirmEmail }),
4044
useCase: deleteAccountUseCase,
4145
present: () => ({ success: true as const }),
46+
/**
47+
* The session row is gone, but the signed cookie cache authenticates this
48+
* browser for up to five more minutes. Clear the cookies on the deletion
49+
* response itself so nothing the page does afterwards can carry them.
50+
*/
51+
finalizeResponse: async ({ request }) => {
52+
try {
53+
const { headers } = await auth.api.signOut({ headers: request.headers, returnHeaders: true })
54+
return { headers }
55+
} catch (error) {
56+
/** The account is gone either way; the client's own sign-out and full reload still drop the cookie. */
57+
logger.warn('Could not clear session cookies after account deletion', { error })
58+
return {}
59+
}
60+
},
4261
})

apps/sim/app/api/workspaces/route.test.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,21 @@ vi.mock('@sim/audit', () => ({
4747
vi.mock('@/lib/workspaces/policy', async () => {
4848
class WorkspaceCreationCapabilityWithheldError extends Error {}
4949
class WorkspaceCreationContextChangedError extends Error {}
50+
class WorkspaceOwnerMissingError extends Error {}
5051
return {
5152
getWorkspaceCreationPolicy: mockGetWorkspaceCreationPolicy,
5253
WorkspaceCreationCapabilityWithheldError,
5354
WorkspaceCreationContextChangedError,
55+
WorkspaceOwnerMissingError,
5456
}
5557
})
5658

57-
import { WorkspaceCreationCapabilityWithheldError } from '@/lib/workspaces/policy'
58-
import { POST } from '@/app/api/workspaces/route'
59+
import { listWorkspacesForViewer } from '@/lib/workspaces/list'
60+
import {
61+
WorkspaceCreationCapabilityWithheldError,
62+
WorkspaceOwnerMissingError,
63+
} from '@/lib/workspaces/policy'
64+
import { GET, POST } from '@/app/api/workspaces/route'
5965

6066
function createRequest() {
6167
return createMockRequest('POST', { name: 'New workspace' })
@@ -116,4 +122,27 @@ describe('POST /api/workspaces capability refusal', () => {
116122
expect(body.error).toBe('Your organization subscription is inactive.')
117123
expect(body.details).toBeUndefined()
118124
})
125+
126+
/**
127+
* After account deletion the browser's cached session cookie stays valid for
128+
* a few minutes, and the next list load finds no workspaces and tries to
129+
* create the default one for a user who no longer exists.
130+
*/
131+
it('answers a default-workspace insert for a deleted user with 401', async () => {
132+
mockGetSession.mockResolvedValue({ user: { id: 'user-1', name: 'Gone' } })
133+
vi.mocked(listWorkspacesForViewer).mockResolvedValue({
134+
workspaces: [],
135+
lastActiveWorkspaceId: null,
136+
pinnedWorkspaceIds: [],
137+
creationPolicy: { canCreate: true, organizationId: null, billedAccountUserId: 'user-1' },
138+
} as never)
139+
mockCreateWorkspace.mockRejectedValue(new WorkspaceOwnerMissingError('user-1'))
140+
141+
const response = await GET(
142+
createMockRequest('GET', undefined, undefined, 'http://localhost/api/workspaces?scope=active')
143+
)
144+
145+
expect(response.status).toBe(401)
146+
await expect(response.json()).resolves.toEqual({ error: 'Unauthorized' })
147+
})
119148
})

apps/sim/app/api/workspaces/route.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
getWorkspaceCreationPolicy,
2020
WorkspaceCreationCapabilityWithheldError,
2121
WorkspaceCreationContextChangedError,
22+
WorkspaceOwnerMissingError,
2223
} from '@/lib/workspaces/policy'
2324

2425
const logger = createLogger('Workspaces')
@@ -87,6 +88,10 @@ export const GET = withRouteHandler(async (request: Request) => {
8788
})
8889
return NextResponse.json(refreshedPayload)
8990
}
91+
/** A cached session cookie outlived the account it belongs to. */
92+
if (error instanceof WorkspaceOwnerMissingError) {
93+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
94+
}
9095
throw error
9196
}
9297

@@ -207,6 +212,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
207212
{ status: 409 }
208213
)
209214
}
215+
/** A cached session cookie outlived the account it belongs to. */
216+
if (error instanceof WorkspaceOwnerMissingError) {
217+
return NextResponse.json({ error: 'Unauthorized' }, { status: 401 })
218+
}
210219
/**
211220
* A lock timeout is contention, not a fault: creation serializes on the
212221
* organization's mutation locks and now also on `permission_group:<org>`,

apps/sim/background/knowledge-connector-sync.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,20 @@ describe('knowledge connector sync worker', () => {
182182
).toBe('completed')
183183
})
184184

185+
it('keeps a held deletion pass a completed task', () => {
186+
const clean = {
187+
docsAdded: 0,
188+
docsUpdated: 0,
189+
docsDeleted: 0,
190+
docsUnchanged: 90,
191+
docsSkipped: 0,
192+
docsFailed: 0,
193+
processingDispatch: { requested: 0, accepted: 0, failed: 0 },
194+
}
195+
expect(classifyConnectorSyncResult({ ...clean, listingIncomplete: false })).toBe('completed')
196+
expect(classifyConnectorSyncResult({ ...clean, listingIncomplete: true })).toBe('partial')
197+
})
198+
185199
it('classifies an isolated processing dispatch failure as partial', () => {
186200
expect(
187201
classifyConnectorSyncResult({

apps/sim/blocks/blocks/mothership.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export const MothershipBlock: BlockConfig<MothershipResponse> = {
4949
id: 'conversationId',
5050
title: 'Conversation ID',
5151
type: 'short-input',
52-
placeholder: 'e.g., user-123, session-abc, customer-456',
52+
placeholder: 'e.g., customer-456 (reuse the same value to continue a thread)',
5353
},
5454
{
5555
id: 'attachmentFiles',
@@ -117,7 +117,8 @@ export const MothershipBlock: BlockConfig<MothershipResponse> = {
117117
},
118118
conversationId: {
119119
type: 'string',
120-
description: 'Chat ID to continue; generated when omitted',
120+
description:
121+
'Stable id of the thread to continue; the same value in this workspace continues the same conversation. Generated when omitted',
121122
},
122123
files: {
123124
type: 'file',
@@ -131,7 +132,10 @@ export const MothershipBlock: BlockConfig<MothershipResponse> = {
131132
outputs: {
132133
content: { type: 'string', description: 'Generated response content' },
133134
model: { type: 'string', description: 'Model used for generation' },
134-
conversationId: { type: 'string', description: 'Chat ID used for this request' },
135+
conversationId: {
136+
type: 'string',
137+
description: 'Conversation id for this thread; pass it to another Sim block to continue it',
138+
},
135139
tokens: { type: 'json', description: 'Token usage statistics' },
136140
toolCalls: { type: 'json', description: 'Tool calls made during execution' },
137141
cost: { type: 'json', description: 'Cost of the execution' },

apps/sim/ee/workspace-forking/lib/remap/block-identity.ts

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { createHash } from 'node:crypto'
1+
import { uuidV5 } from '@/lib/core/utils/uuid-v5'
22

33
/**
44
* Fixed namespace UUID for fork block-identity derivation. Changing this value
@@ -8,30 +8,6 @@ import { createHash } from 'node:crypto'
88
*/
99
const FORK_BLOCK_NAMESPACE = '6f1c0e2a-9b3d-5e47-8a1c-2d4f6b8e0c13'
1010

11-
function uuidToBytes(uuid: string): Buffer {
12-
return Buffer.from(uuid.replace(/-/g, ''), 'hex')
13-
}
14-
15-
/**
16-
* Deterministic UUIDv5 (SHA-1) of `name` within `namespace`. The same inputs
17-
* always yield the same UUID, which is how fork block identity stays stable.
18-
*
19-
* SHA-1 is mandated by RFC 4122 for UUIDv5 and is used here only for deterministic id derivation,
20-
* never for secrecy or integrity — not a security use of the algorithm. Swapping it would change
21-
* every derived id, breaking webhook URLs and stored block-id references across existing forks
22-
* (see {@link FORK_BLOCK_NAMESPACE}).
23-
*/
24-
function uuidV5(name: string, namespace: string): string {
25-
const hash = createHash('sha1')
26-
hash.update(uuidToBytes(namespace)) // lgtm[js/weak-cryptographic-algorithm]
27-
hash.update(Buffer.from(name, 'utf8')) // lgtm[js/weak-cryptographic-algorithm]
28-
const bytes = hash.digest().subarray(0, 16)
29-
bytes[6] = (bytes[6] & 0x0f) | 0x50
30-
bytes[8] = (bytes[8] & 0x3f) | 0x80
31-
const hex = bytes.toString('hex')
32-
return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`
33-
}
34-
3511
/**
3612
* Derive the target block id for a source block copied into a target workflow.
3713
*

apps/sim/executor/handlers/mothership/mothership-handler.test.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import '@sim/testing/mocks/executor'
22

33
import { loggerMock, resetEnvMock, setEnv } from '@sim/testing'
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { resolveMothershipConversation } from '@/lib/mothership/conversation-id'
56
import { BlockType } from '@/executor/constants'
67
import { MothershipBlockHandler } from '@/executor/handlers/mothership/mothership-handler'
78
import type { ExecutionContext, StreamingExecution } from '@/executor/types'
@@ -792,7 +793,7 @@ describe('MothershipBlockHandler', () => {
792793
messages: [{ role: 'user', content: 'Hello from workflow' }],
793794
workspaceId: 'workspace-1',
794795
userId: 'user-1',
795-
chatId: 'chat-uuid',
796+
chatId: resolveMothershipConversation('workspace-1', 'chat-uuid').chatId,
796797
messageId: 'message-uuid',
797798
requestId: 'request-uuid',
798799
secretScope: 'all',
@@ -876,7 +877,7 @@ describe('MothershipBlockHandler', () => {
876877
messages: [{ role: 'user', content: 'Continue this thread' }],
877878
workspaceId: 'workspace-1',
878879
userId: 'user-1',
879-
chatId: 'existing-chat-id',
880+
chatId: resolveMothershipConversation('workspace-1', 'existing-chat-id').chatId,
880881
messageId: 'message-uuid',
881882
requestId: 'request-uuid',
882883
secretScope: 'all',
@@ -887,7 +888,7 @@ describe('MothershipBlockHandler', () => {
887888
expect(mockGenerateId).toHaveBeenCalledTimes(2)
888889
})
889890

890-
it('keeps a resolved conversation ID out of logs while forwarding it unchanged', async () => {
891+
it('keeps a resolved conversation ID out of logs and off the wire', async () => {
891892
const conversationId = 'chat-plaintext-secret-__var_API_KEY-__sim_secret_API_KEY'
892893
mockGenerateId.mockReturnValueOnce('message-uuid').mockReturnValueOnce('request-uuid')
893894
fetchMock.mockResolvedValue(
@@ -907,7 +908,8 @@ describe('MothershipBlockHandler', () => {
907908

908909
const [, options] = fetchMock.mock.calls[0] as [string, RequestInit]
909910
const body = JSON.parse(String(options.body))
910-
expect(body.chatId).toBe(conversationId)
911+
expect(body.chatId).toBe(resolveMothershipConversation('workspace-1', conversationId).chatId)
912+
expect(body.chatId).not.toContain('chat-plaintext-secret')
911913

912914
const logged = JSON.stringify(mockMothershipLogger.info.mock.calls)
913915
expect(logged).not.toContain('chat-plaintext-secret')
@@ -936,7 +938,9 @@ describe('MothershipBlockHandler', () => {
936938
const result = await handler.execute(context, block, inputs)
937939

938940
const [, options] = fetchMock.mock.calls[0] as [string, RequestInit]
939-
expect(JSON.parse(String(options.body)).chatId).toBe('x')
941+
expect(JSON.parse(String(options.body)).chatId).toBe(
942+
resolveMothershipConversation('workspace-1', 'x').chatId
943+
)
940944
expect(result).toMatchObject({ conversationId: 'x' })
941945
expect(inputs.conversationId).toBe('x')
942946
expect(context.resolvedSecretTraceRegistry?.getActiveMatches()).toEqual([])

0 commit comments

Comments
 (0)