From d8faae0aa175b6a35d5f44983f9c26dc7eb33e55 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 10 Sep 2026 18:27:54 -0700 Subject: [PATCH 1/3] 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. --- apps/sim/blocks/blocks/mothership.ts | 10 +++- .../lib/remap/block-identity.ts | 26 +------- .../mothership/mothership-handler.test.ts | 14 +++-- .../handlers/mothership/mothership-handler.ts | 43 +++++++------ apps/sim/lib/core/utils/uuid-v5.test.ts | 23 +++++++ apps/sim/lib/core/utils/uuid-v5.ts | 26 ++++++++ .../lib/mothership/conversation-id.test.ts | 60 +++++++++++++++++++ apps/sim/lib/mothership/conversation-id.ts | 45 ++++++++++++++ 8 files changed, 196 insertions(+), 51 deletions(-) create mode 100644 apps/sim/lib/core/utils/uuid-v5.test.ts create mode 100644 apps/sim/lib/core/utils/uuid-v5.ts create mode 100644 apps/sim/lib/mothership/conversation-id.test.ts create mode 100644 apps/sim/lib/mothership/conversation-id.ts diff --git a/apps/sim/blocks/blocks/mothership.ts b/apps/sim/blocks/blocks/mothership.ts index e74c7f70f15..2f58bceacf7 100644 --- a/apps/sim/blocks/blocks/mothership.ts +++ b/apps/sim/blocks/blocks/mothership.ts @@ -49,7 +49,7 @@ export const MothershipBlock: BlockConfig = { id: 'conversationId', title: 'Conversation ID', type: 'short-input', - placeholder: 'e.g., user-123, session-abc, customer-456', + placeholder: 'e.g., customer-456 (reuse the same value to continue a thread)', }, { id: 'attachmentFiles', @@ -117,7 +117,8 @@ export const MothershipBlock: BlockConfig = { }, conversationId: { type: 'string', - description: 'Chat ID to continue; generated when omitted', + description: + 'Stable id of the thread to continue; the same value in this workspace continues the same conversation. Generated when omitted', }, files: { type: 'file', @@ -131,7 +132,10 @@ export const MothershipBlock: BlockConfig = { outputs: { content: { type: 'string', description: 'Generated response content' }, model: { type: 'string', description: 'Model used for generation' }, - conversationId: { type: 'string', description: 'Chat ID used for this request' }, + conversationId: { + type: 'string', + description: 'Conversation id for this thread; pass it to another Sim block to continue it', + }, tokens: { type: 'json', description: 'Token usage statistics' }, toolCalls: { type: 'json', description: 'Tool calls made during execution' }, cost: { type: 'json', description: 'Cost of the execution' }, diff --git a/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts b/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts index 6360584cf69..1b0a94d540a 100644 --- a/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts +++ b/apps/sim/ee/workspace-forking/lib/remap/block-identity.ts @@ -1,4 +1,4 @@ -import { createHash } from 'node:crypto' +import { uuidV5 } from '@/lib/core/utils/uuid-v5' /** * Fixed namespace UUID for fork block-identity derivation. Changing this value @@ -8,30 +8,6 @@ import { createHash } from 'node:crypto' */ const FORK_BLOCK_NAMESPACE = '6f1c0e2a-9b3d-5e47-8a1c-2d4f6b8e0c13' -function uuidToBytes(uuid: string): Buffer { - return Buffer.from(uuid.replace(/-/g, ''), 'hex') -} - -/** - * Deterministic UUIDv5 (SHA-1) of `name` within `namespace`. The same inputs - * always yield the same UUID, which is how fork block identity stays stable. - * - * SHA-1 is mandated by RFC 4122 for UUIDv5 and is used here only for deterministic id derivation, - * never for secrecy or integrity — not a security use of the algorithm. Swapping it would change - * every derived id, breaking webhook URLs and stored block-id references across existing forks - * (see {@link FORK_BLOCK_NAMESPACE}). - */ -function uuidV5(name: string, namespace: string): string { - const hash = createHash('sha1') - hash.update(uuidToBytes(namespace)) // lgtm[js/weak-cryptographic-algorithm] - hash.update(Buffer.from(name, 'utf8')) // lgtm[js/weak-cryptographic-algorithm] - const bytes = hash.digest().subarray(0, 16) - bytes[6] = (bytes[6] & 0x0f) | 0x50 - bytes[8] = (bytes[8] & 0x3f) | 0x80 - const hex = bytes.toString('hex') - return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}` -} - /** * Derive the target block id for a source block copied into a target workflow. * diff --git a/apps/sim/executor/handlers/mothership/mothership-handler.test.ts b/apps/sim/executor/handlers/mothership/mothership-handler.test.ts index 0eff6c6a6cc..258e79b3bec 100644 --- a/apps/sim/executor/handlers/mothership/mothership-handler.test.ts +++ b/apps/sim/executor/handlers/mothership/mothership-handler.test.ts @@ -2,6 +2,7 @@ import '@sim/testing/mocks/executor' import { loggerMock, resetEnvMock, setEnv } from '@sim/testing' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { resolveMothershipConversation } from '@/lib/mothership/conversation-id' import { BlockType } from '@/executor/constants' import { MothershipBlockHandler } from '@/executor/handlers/mothership/mothership-handler' import type { ExecutionContext, StreamingExecution } from '@/executor/types' @@ -792,7 +793,7 @@ describe('MothershipBlockHandler', () => { messages: [{ role: 'user', content: 'Hello from workflow' }], workspaceId: 'workspace-1', userId: 'user-1', - chatId: 'chat-uuid', + chatId: resolveMothershipConversation('workspace-1', 'chat-uuid').chatId, messageId: 'message-uuid', requestId: 'request-uuid', secretScope: 'all', @@ -876,7 +877,7 @@ describe('MothershipBlockHandler', () => { messages: [{ role: 'user', content: 'Continue this thread' }], workspaceId: 'workspace-1', userId: 'user-1', - chatId: 'existing-chat-id', + chatId: resolveMothershipConversation('workspace-1', 'existing-chat-id').chatId, messageId: 'message-uuid', requestId: 'request-uuid', secretScope: 'all', @@ -887,7 +888,7 @@ describe('MothershipBlockHandler', () => { expect(mockGenerateId).toHaveBeenCalledTimes(2) }) - it('keeps a resolved conversation ID out of logs while forwarding it unchanged', async () => { + it('keeps a resolved conversation ID out of logs and off the wire', async () => { const conversationId = 'chat-plaintext-secret-__var_API_KEY-__sim_secret_API_KEY' mockGenerateId.mockReturnValueOnce('message-uuid').mockReturnValueOnce('request-uuid') fetchMock.mockResolvedValue( @@ -907,7 +908,8 @@ describe('MothershipBlockHandler', () => { const [, options] = fetchMock.mock.calls[0] as [string, RequestInit] const body = JSON.parse(String(options.body)) - expect(body.chatId).toBe(conversationId) + expect(body.chatId).toBe(resolveMothershipConversation('workspace-1', conversationId).chatId) + expect(body.chatId).not.toContain('chat-plaintext-secret') const logged = JSON.stringify(mockMothershipLogger.info.mock.calls) expect(logged).not.toContain('chat-plaintext-secret') @@ -936,7 +938,9 @@ describe('MothershipBlockHandler', () => { const result = await handler.execute(context, block, inputs) const [, options] = fetchMock.mock.calls[0] as [string, RequestInit] - expect(JSON.parse(String(options.body)).chatId).toBe('x') + expect(JSON.parse(String(options.body)).chatId).toBe( + resolveMothershipConversation('workspace-1', 'x').chatId + ) expect(result).toMatchObject({ conversationId: 'x' }) expect(inputs.conversationId).toBe('x') expect(context.resolvedSecretTraceRegistry?.getActiveMatches()).toEqual([]) diff --git a/apps/sim/executor/handlers/mothership/mothership-handler.ts b/apps/sim/executor/handlers/mothership/mothership-handler.ts index ba9a5335b2c..f367402ae7d 100644 --- a/apps/sim/executor/handlers/mothership/mothership-handler.ts +++ b/apps/sim/executor/handlers/mothership/mothership-handler.ts @@ -24,6 +24,7 @@ import { import { discoverMcpServerToolsAsExecutor } from '@/lib/internal/mcp/discover-tools' import { assertValidMcpServerToolBindings, MCP_SERVER_ADVANCED_TOOL_TYPE } from '@/lib/mcp/shared' import { resolveMcpToolBinding } from '@/lib/mcp/tool-binding' +import { resolveMothershipConversation } from '@/lib/mothership/conversation-id' import { areModelSafeWorkspaceFileKeys, MODEL_UNSAFE_WORKSPACE_FILE_ERROR_MESSAGE, @@ -485,7 +486,7 @@ function parseMothershipExecuteStreamLine(line: string): MothershipExecuteStream function formatMothershipBlockOutput( result: MothershipExecuteResult, - fallbackChatId: string + conversationId: string ): NormalizedBlockOutput { const formattedList = (result.toolCalls || []).map((tc: Record) => ({ name: typeof tc.name === 'string' ? tc.name : String(tc.name ?? ''), @@ -503,7 +504,7 @@ function formatMothershipBlockOutput( return { content: result.content || '', model: result.model || 'mothership', - conversationId: result.conversationId || fallbackChatId, + conversationId, tokens: (result.tokens || {}) as NormalizedBlockOutput['tokens'], toolCalls, cost: result.cost as NormalizedBlockOutput['cost'] | undefined, @@ -605,7 +606,7 @@ async function readMothershipExecuteResponse( function createMothershipStreamingExecution( response: Response, - fallbackChatId: string, + conversationId: string, blockId: string, options: { onCancel?: (reason?: unknown) => void @@ -618,7 +619,7 @@ function createMothershipStreamingExecution( throw new Error('Sim execution stream ended without a response body') } - const output = formatMothershipBlockOutput({}, fallbackChatId) + const output = formatMothershipBlockOutput({}, conversationId) let reader: ReadableStreamDefaultReader | undefined let cancelled = false let cleanedUp = false @@ -664,7 +665,7 @@ function createMothershipStreamingExecution( if (event.type === 'final') { await consumeMothershipProvenance(event.data, response, options.registry) sawFinal = true - Object.assign(output, formatMothershipBlockOutput(event.data, fallbackChatId)) + Object.assign(output, formatMothershipBlockOutput(event.data, conversationId)) return } @@ -890,9 +891,10 @@ export class MothershipBlockHandler implements BlockHandler { content: modelInputProjection.value.prompt, }, ] - const providedConversationId = - typeof inputs.conversationId === 'string' ? inputs.conversationId.trim() : '' - const chatId = providedConversationId || generateId() + const { conversationId, chatId } = resolveMothershipConversation( + ctx.workspaceId ?? '', + inputs.conversationId + ) const messageId = generateId() const requestId = generateId() const secretMountPolicy = normalizeSecretMountPolicy({ @@ -1003,15 +1005,20 @@ export class MothershipBlockHandler implements BlockHandler { } if (isContentSelectedForStreaming(ctx, block)) { - const streamingExecution = createMothershipStreamingExecution(response, chatId, block.id, { - onCancel: (reason) => { - if (!abortController.signal.aborted) { - abortController.abort(reason ?? 'mothership_stream_cancelled') - } - }, - onDone: cleanupAbortListeners, - registry: resultRegistry, - }) + const streamingExecution = createMothershipStreamingExecution( + response, + conversationId, + block.id, + { + onCancel: (reason) => { + if (!abortController.signal.aborted) { + abortController.abort(reason ?? 'mothership_stream_cancelled') + } + }, + onDone: cleanupAbortListeners, + registry: resultRegistry, + } + ) streamingExecution.diagnosticResolvedSecretTraceRegistry = resultRegistry if (resultRegistry) ctx.resolvedSecretTraceRegistry = resultRegistry cleanupImmediately = false @@ -1019,7 +1026,7 @@ export class MothershipBlockHandler implements BlockHandler { } const result = await readMothershipExecuteResponse(response, resultRegistry) - const output = formatMothershipBlockOutput(result, chatId) + const output = formatMothershipBlockOutput(result, conversationId) if (resultRegistry) ctx.resolvedSecretTraceRegistry = resultRegistry return output } catch (error) { diff --git a/apps/sim/lib/core/utils/uuid-v5.test.ts b/apps/sim/lib/core/utils/uuid-v5.test.ts new file mode 100644 index 00000000000..d7bfa33b1af --- /dev/null +++ b/apps/sim/lib/core/utils/uuid-v5.test.ts @@ -0,0 +1,23 @@ +/** + * @vitest-environment node + */ +import { isValidUuid } from '@sim/utils/id' +import { describe, expect, it } from 'vitest' +import { uuidV5 } from '@/lib/core/utils/uuid-v5' + +const DNS_NAMESPACE = '6ba7b810-9dad-11d1-80b4-00c04fd430c8' + +describe('uuidV5', () => { + it('matches the RFC 4122 reference vector', () => { + expect(uuidV5('python.org', DNS_NAMESPACE)).toBe('886313e1-3b8a-5372-9b90-0c9aee199e5d') + }) + + it('is deterministic and namespace-scoped', () => { + const a = uuidV5('customer-456', DNS_NAMESPACE) + expect(uuidV5('customer-456', DNS_NAMESPACE)).toBe(a) + expect(uuidV5('customer-457', DNS_NAMESPACE)).not.toBe(a) + expect(uuidV5('customer-456', '00000000-0000-0000-0000-000000000000')).not.toBe(a) + expect(isValidUuid(a)).toBe(true) + expect(a[14]).toBe('5') + }) +}) diff --git a/apps/sim/lib/core/utils/uuid-v5.ts b/apps/sim/lib/core/utils/uuid-v5.ts new file mode 100644 index 00000000000..9dda65176d4 --- /dev/null +++ b/apps/sim/lib/core/utils/uuid-v5.ts @@ -0,0 +1,26 @@ +import { createHash } from 'node:crypto' + +function uuidToBytes(uuid: string): Buffer { + return Buffer.from(uuid.replace(/-/g, ''), 'hex') +} + +/** + * Deterministic UUIDv5 (SHA-1) of `name` within `namespace`, per RFC 4122 §4.3. + * The same inputs always yield the same UUID, which is how callers derive a + * stable identity from a caller-chosen string. + * + * SHA-1 is mandated by RFC 4122 for UUIDv5 and is used here only for + * deterministic id derivation, never for secrecy or integrity. Every caller + * pins its own namespace constant; changing a namespace re-keys every id that + * caller ever derived. + */ +export function uuidV5(name: string, namespace: string): string { + const hash = createHash('sha1') + hash.update(uuidToBytes(namespace)) // lgtm[js/weak-cryptographic-algorithm] + hash.update(Buffer.from(name, 'utf8')) // lgtm[js/weak-cryptographic-algorithm] + const bytes = hash.digest().subarray(0, 16) + bytes[6] = (bytes[6] & 0x0f) | 0x50 + bytes[8] = (bytes[8] & 0x3f) | 0x80 + const hex = bytes.toString('hex') + return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}` +} diff --git a/apps/sim/lib/mothership/conversation-id.test.ts b/apps/sim/lib/mothership/conversation-id.test.ts new file mode 100644 index 00000000000..24fa3f79eda --- /dev/null +++ b/apps/sim/lib/mothership/conversation-id.test.ts @@ -0,0 +1,60 @@ +/** + * @vitest-environment node + */ +import { isValidUuid } from '@sim/utils/id' +import { describe, expect, it } from 'vitest' +import { resolveMothershipConversation } from '@/lib/mothership/conversation-id' + +const WORKSPACE_A = '11111111-1111-4111-8111-111111111111' +const WORKSPACE_B = '22222222-2222-4222-8222-222222222222' + +describe('resolveMothershipConversation', () => { + it('mints a fresh token when no conversation id is given', () => { + const first = resolveMothershipConversation(WORKSPACE_A, undefined) + const second = resolveMothershipConversation(WORKSPACE_A, ' ') + expect(isValidUuid(first.conversationId)).toBe(true) + expect(isValidUuid(first.chatId)).toBe(true) + expect(first.chatId).not.toBe(first.conversationId) + expect(first.chatId).not.toBe(second.chatId) + }) + + it('derives a stable chat id scoped to the workspace and exposes the given id', () => { + const resolved = resolveMothershipConversation(WORKSPACE_A, ' customer-456 ') + expect(resolved.conversationId).toBe('customer-456') + expect(isValidUuid(resolved.chatId)).toBe(true) + expect(resolveMothershipConversation(WORKSPACE_A, 'customer-456').chatId).toBe(resolved.chatId) + expect(resolveMothershipConversation(WORKSPACE_A, 'customer-457').chatId).not.toBe( + resolved.chatId + ) + expect(resolveMothershipConversation(WORKSPACE_B, 'customer-456').chatId).not.toBe( + resolved.chatId + ) + }) + + it('derives UUID-shaped ids too, so a block can never reach a chat it did not derive', () => { + const pasted = '3b2f0d4e-8a6c-4f1b-9e2d-5c7a1b3d9f00' + const resolved = resolveMothershipConversation(WORKSPACE_A, pasted) + expect(resolved.conversationId).toBe(pasted) + expect(resolved.chatId).not.toBe(pasted) + expect(isValidUuid(resolved.chatId)).toBe(true) + }) + + it('continues the same thread when the exposed id is chained into another block', () => { + const first = resolveMothershipConversation(WORKSPACE_A, undefined) + const chained = resolveMothershipConversation(WORKSPACE_A, first.conversationId) + expect(chained.chatId).toBe(first.chatId) + }) + + it('never forwards the literal string', () => { + const secretish = 'chat-plaintext-secret-__var_API_KEY-__sim_secret_API_KEY' + const { chatId } = resolveMothershipConversation(WORKSPACE_A, secretish) + expect(chatId).not.toContain('secret') + expect(chatId).not.toContain('__') + }) + + it('refuses to resolve without a workspace', () => { + expect(() => resolveMothershipConversation('', 'customer-456')).toThrow( + 'Workspace context is required' + ) + }) +}) diff --git a/apps/sim/lib/mothership/conversation-id.ts b/apps/sim/lib/mothership/conversation-id.ts new file mode 100644 index 00000000000..47224e109bf --- /dev/null +++ b/apps/sim/lib/mothership/conversation-id.ts @@ -0,0 +1,45 @@ +import { generateId } from '@sim/utils/id' +import { uuidV5 } from '@/lib/core/utils/uuid-v5' + +/** + * Namespace for chat ids derived from block conversation ids. Changing it + * re-keys every derived conversation, so every running block thread would + * lose its history. It must never change. + */ +const MOTHERSHIP_CONVERSATION_NAMESPACE = '2b7c5e0a-4f61-5d3e-9a8b-7c1d0e2f3a45' + +export interface ResolvedMothershipConversation { + /** The id the block exposes; feeding it to another Sim block continues the thread. */ + conversationId: string + /** The chat id sent over the wire, derived from the workspace and the conversation id. */ + chatId: string +} + +/** + * Resolves the chat id a Sim block conversation runs under. + * + * Builders may choose any stable string such as `customer-456` so a workflow + * continues one thread per customer, and chat ids are UUIDs everywhere + * downstream (`copilot_chats.id`, `workspace_files.chat_id`, and the copilot + * service's own conversation store). Every conversation id, UUID-shaped or not, + * is mapped to a UUID derived from the workspace and the id: the same value + * keeps the same thread across runs, two workspaces choosing the same value + * never share a conversation, a block can never reach a chat it did not derive + * (the copilot store has no ownership check of its own), and the literal value + * (which may carry a resolved secret) never leaves the executor. An omitted id + * mints a fresh token so the exposed id still continues the thread when chained. + */ +export function resolveMothershipConversation( + workspaceId: string, + conversationId: unknown +): ResolvedMothershipConversation { + if (!workspaceId) { + throw new Error('Workspace context is required to resolve a Sim conversation') + } + const provided = typeof conversationId === 'string' ? conversationId.trim() : '' + const token = provided || generateId() + return { + conversationId: token, + chatId: uuidV5(`${workspaceId}:${token}`, MOTHERSHIP_CONVERSATION_NAMESPACE), + } +} From 1622f02be8f7265757de6e5f313de938861aa61a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 10 Sep 2026 18:30:10 -0700 Subject: [PATCH 2/3] 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. --- .../knowledge-connector-sync.test.ts | 14 +++++ .../knowledge/connectors/sync-engine.test.ts | 61 +++++++++++++++++++ .../lib/knowledge/connectors/sync-engine.ts | 61 ++++++++++--------- 3 files changed, 106 insertions(+), 30 deletions(-) diff --git a/apps/sim/background/knowledge-connector-sync.test.ts b/apps/sim/background/knowledge-connector-sync.test.ts index 3a34db9c311..4dd9c91e776 100644 --- a/apps/sim/background/knowledge-connector-sync.test.ts +++ b/apps/sim/background/knowledge-connector-sync.test.ts @@ -182,6 +182,20 @@ describe('knowledge connector sync worker', () => { ).toBe('completed') }) + it('keeps a held deletion pass a completed task', () => { + const clean = { + docsAdded: 0, + docsUpdated: 0, + docsDeleted: 0, + docsUnchanged: 90, + docsSkipped: 0, + docsFailed: 0, + processingDispatch: { requested: 0, accepted: 0, failed: 0 }, + } + expect(classifyConnectorSyncResult({ ...clean, listingIncomplete: false })).toBe('completed') + expect(classifyConnectorSyncResult({ ...clean, listingIncomplete: true })).toBe('partial') + }) + it('classifies an isolated processing dispatch failure as partial', () => { expect( classifyConnectorSyncResult({ diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts index 61a6fdbe07b..e4b2c551769 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts @@ -2107,6 +2107,25 @@ describe('completeSyncLog', () => { }) }) +describe('isContentPassIncomplete', () => { + it('is true only when the listing has not finished or a source read failed', async () => { + const { isContentPassIncomplete } = await import('@/lib/knowledge/connectors/sync-engine') + const checkpoint = { startedAt: '2026-09-04T00:00:00Z', listedCount: 4 } + for (const complete of [true, false]) { + for (const unsafe of [true, false]) { + for (const contentFailures of [true, false, undefined]) { + expect( + isContentPassIncomplete({ + complete, + checkpoint: { ...checkpoint, unsafe, contentFailures }, + }) + ).toBe(!complete || contentFailures === true) + } + } + } + }) +}) + describe('completeSuccessfulSync', () => { const RESULT = { docsAdded: 1, @@ -2203,6 +2222,48 @@ describe('completeSuccessfulSync', () => { expect((connectorUpdate.nextSyncAt as Date).getTime()).toBeGreaterThan(Date.now() + 50 * 60_000) }) + it('records a held listing as a completed sync whose watermark advances', async () => { + const { completeSuccessfulSync } = await import('@/lib/knowledge/connectors/sync-engine') + queueTableRows(schemaMock.knowledgeBase, [{ id: 'kb-1' }]) + queueTableRows(schemaMock.knowledgeConnector, [{ id: 'c-1' }]) + queueTableRows(schemaMock.document, [{ count: 4 }]) + dbChainMockFns.returning + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ id: 'log-1' }]) + .mockResolvedValueOnce([{ id: 'c-1' }]) + const holdNotice = 'Source listing is incomplete; unlisted documents were kept.' + + expect( + await completeSuccessfulSync( + 'c-1', + 'kb-1', + 'log-1', + 60, + { ...RESULT, docsFailed: 0 }, + holdNotice, + { + complete: true, + checkpoint: { + unsafe: true, + contentFailures: false, + startedAt: '2026-09-04T00:00:00Z', + listedCount: 4, + }, + } + ) + ).toBe(true) + expect(dbChainMockFns.set).toHaveBeenCalledWith( + expect.objectContaining({ status: 'completed', docsFailed: 0, listedCount: 4 }) + ) + const connectorUpdate = dbChainMockFns.set.mock.calls.find( + (call) => (call[0] as Record | undefined)?.status === 'active' + )?.[0] as Record + expect(connectorUpdate.lastSyncAt).toEqual(new Date('2026-09-04T00:00:00Z')) + expect(connectorUpdate.lastSyncError).toBe(holdNotice) + expect(connectorUpdate.listingCheckpoint).toBeNull() + expect((connectorUpdate.nextSyncAt as Date).getTime()).toBeGreaterThan(Date.now() + 50 * 60_000) + }) + it('does not publish connector state when the guarded log close is refused', async () => { const { completeSuccessfulSync } = await import('@/lib/knowledge/connectors/sync-engine') diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index 98348acc60e..d1e9ef65b85 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -289,6 +289,32 @@ class SyncCompletionOwnershipLost extends Error { } } +/** What a finished content pass reports to the sync-log and connector close. */ +export interface ContentPassOutcome { + complete: boolean + checkpoint: { + unsafe: boolean + contentFailures?: boolean + startedAt: string + listedCount: number + incrementalSince?: string | null + } +} + +/** + * A content pass is incomplete when the listing has not reached the end of the + * source (the generation resumes on the next run) or a source read failed (the + * next pass replays it). `checkpoint.unsafe` is deliberately not part of this: + * it means "do not infer deletions from this listing" and is honored by the + * deletion hold in `reconcileCompletedListing`. A held pass is still a + * completed sync whose watermark advances. + */ +export function isContentPassIncomplete( + contentPass: Pick +): boolean { + return !contentPass.complete || contentPass.checkpoint.contentFailures === true +} + /** * Atomically publishes the completed log and connector terminal state. * @@ -303,16 +329,7 @@ export async function completeSuccessfulSync( syncIntervalMinutes: number, result: SyncResult, reconciliationHoldNotice: string | null, - contentPass?: { - complete: boolean - checkpoint: { - unsafe: boolean - contentFailures?: boolean - startedAt: string - listedCount: number - incrementalSince?: string | null - } - } + contentPass?: ContentPassOutcome ): Promise { try { return await db.transaction(async (tx) => { @@ -361,13 +378,7 @@ export async function completeSuccessfulSync( const [closedLog] = await tx .update(knowledgeConnectorSyncLog) .set({ - status: - contentPass && - (!contentPass.complete || - contentPass.checkpoint.unsafe || - contentPass.checkpoint.contentFailures) - ? 'partial' - : 'completed', + status: contentPass && isContentPassIncomplete(contentPass) ? 'partial' : 'completed', completedAt: now, listedCount: contentPass?.complete ? contentPass.checkpoint.incrementalSince @@ -398,19 +409,12 @@ export async function completeSuccessfulSync( actualDocCount, contentPass && !contentPass.complete ? now : calculateNextSyncTime(syncIntervalMinutes), reconciliationHoldNotice, - result.docsFailed === 0 && - (!contentPass || - (contentPass.complete && - !contentPass.checkpoint.unsafe && - !contentPass.checkpoint.contentFailures)) + result.docsFailed === 0 && (!contentPass || !isContentPassIncomplete(contentPass)) ), /** Restored above under this same lock, or hidden by the admin pass before the ACLs it wrote. */ accessRewritePending: false, ...(contentPass?.complete ? { listingCheckpoint: null } : {}), - ...(contentPass?.complete && - !contentPass.checkpoint.unsafe && - !contentPass.checkpoint.contentFailures && - result.docsFailed === 0 + ...(contentPass && !isContentPassIncomplete(contentPass) && result.docsFailed === 0 ? { lastSyncAt: new Date(contentPass.checkpoint.startedAt) } : {}), }) @@ -1119,10 +1123,7 @@ export async function executeSync( : undefined, }) - result.listingIncomplete = - !contentPass.complete || - contentPass.checkpoint.unsafe || - contentPass.checkpoint.contentFailures + result.listingIncomplete = isContentPassIncomplete(contentPass) const reconciliationHoldNotice = contentPass.holdNotice const directoryError = await directoryRefreshed if (directoryError) throw directoryError From 1941a0c9b22cb57ba81245d946e6a64974b41c6e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 10 Sep 2026 18:33:22 -0700 Subject: [PATCH 3/3] 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. --- .../app/api/users/me/deletion/route.test.ts | 81 +++++++++++++++++++ apps/sim/app/api/users/me/deletion/route.ts | 19 +++++ apps/sim/app/api/workspaces/route.test.ts | 33 +++++++- apps/sim/app/api/workspaces/route.ts | 9 +++ .../server/routes/internal-json-route.test.ts | 31 +++++++ .../api/server/routes/internal-json-route.ts | 5 ++ apps/sim/lib/workspaces/create.test.ts | 30 ++++++- apps/sim/lib/workspaces/create.ts | 15 ++++ apps/sim/lib/workspaces/policy.ts | 12 +++ 9 files changed, 232 insertions(+), 3 deletions(-) create mode 100644 apps/sim/app/api/users/me/deletion/route.test.ts diff --git a/apps/sim/app/api/users/me/deletion/route.test.ts b/apps/sim/app/api/users/me/deletion/route.test.ts new file mode 100644 index 00000000000..7369ad3bed4 --- /dev/null +++ b/apps/sim/app/api/users/me/deletion/route.test.ts @@ -0,0 +1,81 @@ +/** + * @vitest-environment node + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + defineInternalJsonRoute: vi.fn(() => vi.fn()), + signOut: vi.fn(), +})) + +vi.mock('@/lib/api/server/routes', () => ({ + defineInternalJsonRoute: mocks.defineInternalJsonRoute, + internalOrchestrationErrorPolicy: { project: vi.fn(), unhandled: vi.fn() }, + internalRateLimits: { none: vi.fn(() => ({ kind: 'none' })) }, + internalSessionAuth: { authenticate: vi.fn() }, +})) + +vi.mock('@/lib/auth', () => ({ + auth: { api: { signOut: mocks.signOut } }, + getSession: vi.fn(), +})) + +vi.mock('@/lib/users/application/delete-account', () => ({ + deleteAccountUseCase: { execute: vi.fn() }, + previewAccountDeletionUseCase: { execute: vi.fn() }, +})) + +vi.mock('@/lib/users/application/operations', () => ({ + userAccountOperations: { delete: { id: 'user.delete' }, previewDeletion: { id: 'user.preview' } }, +})) + +import '@/app/api/users/me/deletion/route' + +type RouteOptions = { + finalizeResponse?: (args: { request: Request }) => Promise<{ headers?: HeadersInit }> +} + +/** Captured at import time; the route registers itself once when the module loads. */ +const routeOptions = mocks.defineInternalJsonRoute.mock.calls.map((call) => call[0] as RouteOptions) + +describe('POST /api/users/me/deletion', () => { + beforeEach(() => { + mocks.signOut.mockReset() + }) + + /** + * The session row is deleted with the account, but the signed cookie cache + * still authenticates the browser for its TTL; the response must clear it. + */ + it('clears the session cookies on the deletion response', async () => { + const options = routeOptions.find( + (candidate) => typeof candidate.finalizeResponse === 'function' + ) + expect(options?.finalizeResponse).toBeDefined() + + const cleared = new Headers([['set-cookie', 'better-auth.session_token=; Max-Age=0']]) + mocks.signOut.mockResolvedValue({ headers: cleared, response: { success: true } }) + const request = new Request('http://localhost/api/users/me/deletion', { + method: 'POST', + headers: { cookie: 'better-auth.session_token=abc' }, + }) + + const finalization = await options!.finalizeResponse!({ request }) + + expect(mocks.signOut).toHaveBeenCalledWith({ headers: request.headers, returnHeaders: true }) + expect(finalization.headers).toBe(cleared) + }) + + it('never fails a completed deletion because the cookies could not be cleared', async () => { + const options = routeOptions.find( + (candidate) => typeof candidate.finalizeResponse === 'function' + ) + mocks.signOut.mockRejectedValue(new Error('sign-out unavailable')) + + await expect( + options!.finalizeResponse!({ + request: new Request('http://localhost/api/users/me/deletion', { method: 'POST' }), + }) + ).resolves.toEqual({}) + }) +}) diff --git a/apps/sim/app/api/users/me/deletion/route.ts b/apps/sim/app/api/users/me/deletion/route.ts index ebb8b64aca7..2b148f889e0 100644 --- a/apps/sim/app/api/users/me/deletion/route.ts +++ b/apps/sim/app/api/users/me/deletion/route.ts @@ -1,3 +1,4 @@ +import { createLogger } from '@sim/logger' import { deleteAccountContract, getAccountDeletionPlanContract } from '@/lib/api/contracts' import { defineInternalJsonRoute, @@ -5,12 +6,15 @@ import { internalRateLimits, internalSessionAuth, } from '@/lib/api/server/routes' +import { auth } from '@/lib/auth' import { deleteAccountUseCase, previewAccountDeletionUseCase, } from '@/lib/users/application/delete-account' import { userAccountOperations } from '@/lib/users/application/operations' +const logger = createLogger('AccountDeletionRoute') + export const dynamic = 'force-dynamic' export const GET = defineInternalJsonRoute({ @@ -39,4 +43,19 @@ export const POST = defineInternalJsonRoute({ mapInput: ({ body }) => ({ confirmEmail: body.confirmEmail }), useCase: deleteAccountUseCase, present: () => ({ success: true as const }), + /** + * The session row is gone, but the signed cookie cache authenticates this + * browser for up to five more minutes. Clear the cookies on the deletion + * response itself so nothing the page does afterwards can carry them. + */ + finalizeResponse: async ({ request }) => { + try { + const { headers } = await auth.api.signOut({ headers: request.headers, returnHeaders: true }) + return { headers } + } catch (error) { + /** The account is gone either way; the client's own sign-out and full reload still drop the cookie. */ + logger.warn('Could not clear session cookies after account deletion', { error }) + return {} + } + }, }) diff --git a/apps/sim/app/api/workspaces/route.test.ts b/apps/sim/app/api/workspaces/route.test.ts index 54fca6aa265..2ec4cedd881 100644 --- a/apps/sim/app/api/workspaces/route.test.ts +++ b/apps/sim/app/api/workspaces/route.test.ts @@ -47,15 +47,21 @@ vi.mock('@sim/audit', () => ({ vi.mock('@/lib/workspaces/policy', async () => { class WorkspaceCreationCapabilityWithheldError extends Error {} class WorkspaceCreationContextChangedError extends Error {} + class WorkspaceOwnerMissingError extends Error {} return { getWorkspaceCreationPolicy: mockGetWorkspaceCreationPolicy, WorkspaceCreationCapabilityWithheldError, WorkspaceCreationContextChangedError, + WorkspaceOwnerMissingError, } }) -import { WorkspaceCreationCapabilityWithheldError } from '@/lib/workspaces/policy' -import { POST } from '@/app/api/workspaces/route' +import { listWorkspacesForViewer } from '@/lib/workspaces/list' +import { + WorkspaceCreationCapabilityWithheldError, + WorkspaceOwnerMissingError, +} from '@/lib/workspaces/policy' +import { GET, POST } from '@/app/api/workspaces/route' function createRequest() { return createMockRequest('POST', { name: 'New workspace' }) @@ -116,4 +122,27 @@ describe('POST /api/workspaces capability refusal', () => { expect(body.error).toBe('Your organization subscription is inactive.') expect(body.details).toBeUndefined() }) + + /** + * After account deletion the browser's cached session cookie stays valid for + * a few minutes, and the next list load finds no workspaces and tries to + * create the default one for a user who no longer exists. + */ + it('answers a default-workspace insert for a deleted user with 401', async () => { + mockGetSession.mockResolvedValue({ user: { id: 'user-1', name: 'Gone' } }) + vi.mocked(listWorkspacesForViewer).mockResolvedValue({ + workspaces: [], + lastActiveWorkspaceId: null, + pinnedWorkspaceIds: [], + creationPolicy: { canCreate: true, organizationId: null, billedAccountUserId: 'user-1' }, + } as never) + mockCreateWorkspace.mockRejectedValue(new WorkspaceOwnerMissingError('user-1')) + + const response = await GET( + createMockRequest('GET', undefined, undefined, 'http://localhost/api/workspaces?scope=active') + ) + + expect(response.status).toBe(401) + await expect(response.json()).resolves.toEqual({ error: 'Unauthorized' }) + }) }) diff --git a/apps/sim/app/api/workspaces/route.ts b/apps/sim/app/api/workspaces/route.ts index c5b398fa3f1..f2eebe797de 100644 --- a/apps/sim/app/api/workspaces/route.ts +++ b/apps/sim/app/api/workspaces/route.ts @@ -19,6 +19,7 @@ import { getWorkspaceCreationPolicy, WorkspaceCreationCapabilityWithheldError, WorkspaceCreationContextChangedError, + WorkspaceOwnerMissingError, } from '@/lib/workspaces/policy' const logger = createLogger('Workspaces') @@ -87,6 +88,10 @@ export const GET = withRouteHandler(async (request: Request) => { }) return NextResponse.json(refreshedPayload) } + /** A cached session cookie outlived the account it belongs to. */ + if (error instanceof WorkspaceOwnerMissingError) { + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } throw error } @@ -207,6 +212,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => { { status: 409 } ) } + /** A cached session cookie outlived the account it belongs to. */ + if (error instanceof WorkspaceOwnerMissingError) { + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }) + } /** * A lock timeout is contention, not a fault: creation serializes on the * organization's mutation locks and now also on `permission_group:`, diff --git a/apps/sim/lib/api/server/routes/internal-json-route.test.ts b/apps/sim/lib/api/server/routes/internal-json-route.test.ts index 32c7dd2a1e5..ace040a22a2 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.test.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.test.ts @@ -492,6 +492,37 @@ describe('defineInternalJsonRoute', () => { }) }) + it('keeps every cookie a finalizer clears on its own header line', async () => { + const handler = defineInternalJsonRoute({ + contract, + auth, + operation, + rateLimit: internalRateLimits.none({ reason: 'Unit test' }), + errorPolicy: internalOrchestrationErrorPolicy, + mapInput: () => undefined, + useCase: { + operation, + async execute() { + return { value: 'ok' } + }, + }, + finalizeResponse: () => ({ + headers: new Headers([ + ['set-cookie', 'session_token=; Max-Age=0; Path=/'], + ['set-cookie', 'session_data=; Max-Age=0; Path=/'], + ]), + }), + }) + + const response = await handler(new NextRequest('http://localhost/api/test/internal-json-route')) + + expect(response.status).toBe(200) + expect(response.headers.getSetCookie()).toEqual([ + 'session_token=; Max-Age=0; Path=/', + 'session_data=; Max-Age=0; Path=/', + ]) + }) + it('selects a declared success status from the application result', async () => { const replayableContract = defineRouteContract({ method: 'POST', diff --git a/apps/sim/lib/api/server/routes/internal-json-route.ts b/apps/sim/lib/api/server/routes/internal-json-route.ts index ba0d6a5fe0e..ddacfca77da 100644 --- a/apps/sim/lib/api/server/routes/internal-json-route.ts +++ b/apps/sim/lib/api/server/routes/internal-json-route.ts @@ -327,6 +327,11 @@ function appendFinalizedHeaders(base: HeadersInit | undefined, additions?: Heade const headers = new Headers(base) if (!additions) return headers new Headers(additions).forEach((value, key) => { + /** A finalizer may clear several cookies at once; each needs its own header line. */ + if (key === 'set-cookie') { + headers.append(key, value) + return + } if (headers.has(key)) { throw new Error(`Internal JSON response finalizer cannot replace header "${key}"`) } diff --git a/apps/sim/lib/workspaces/create.test.ts b/apps/sim/lib/workspaces/create.test.ts index 771740f8fca..af282f9c778 100644 --- a/apps/sim/lib/workspaces/create.test.ts +++ b/apps/sim/lib/workspaces/create.test.ts @@ -47,7 +47,7 @@ import { createDefaultPersonalWorkspaceInTransaction, createWorkspace, } from '@/lib/workspaces/create' -import { WORKSPACE_MODE } from '@/lib/workspaces/policy' +import { WORKSPACE_MODE, WorkspaceOwnerMissingError } from '@/lib/workspaces/policy' const params = { userId: 'creator-1', @@ -127,6 +127,34 @@ describe('createWorkspace capability-gate placement', () => { }) }) + /** + * A cached session cookie can outlive the user row by a few minutes. The + * insert then fails on a `workspace` -> `user` foreign key, which the caller + * must be able to tell apart from a fault so it answers 401, not 500. + */ + it('reports a missing owner as a typed error instead of a fault', async () => { + mockResolveGoverningPermissionGroupOrganization.mockResolvedValue('org-1') + dbChainMockFns.transaction.mockRejectedValue( + Object.assign(new Error('insert or update on table "workspace" violates foreign key'), { + code: '23503', + constraint_name: 'workspace_billed_account_user_id_user_id_fk', + }) + ) + + await expect(createWorkspace(params)).rejects.toBeInstanceOf(WorkspaceOwnerMissingError) + }) + + it('rethrows other foreign key violations untouched', async () => { + mockResolveGoverningPermissionGroupOrganization.mockResolvedValue('org-1') + const failure = Object.assign(new Error('violates foreign key'), { + code: '23503', + constraint_name: 'workspace_organization_id_organization_id_fk', + }) + dbChainMockFns.transaction.mockRejectedValue(failure) + + await expect(createWorkspace(params)).rejects.toBe(failure) + }) + /** * The preflight policy resolved this value microseconds earlier in the same * request, and React's `cache()` memo does not span the two calls, so a diff --git a/apps/sim/lib/workspaces/create.ts b/apps/sim/lib/workspaces/create.ts index 44d4e8e77b3..8e5d8f46c0b 100644 --- a/apps/sim/lib/workspaces/create.ts +++ b/apps/sim/lib/workspaces/create.ts @@ -1,6 +1,7 @@ import { db } from '@sim/db' import { permissions, type WorkspaceMode, workflow, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' +import { getPostgresConstraintName, getPostgresErrorCode } from '@sim/utils/errors' import { generateId } from '@sim/utils/id' import { PlatformEvents } from '@/lib/core/telemetry' import type { DbOrTx } from '@/lib/db/types' @@ -12,8 +13,15 @@ import { resolveGoverningPermissionGroupOrganization, resolveInviteFlags, WORKSPACE_MODE, + WorkspaceOwnerMissingError, } from '@/lib/workspaces/policy' +/** Foreign keys from `workspace` to `user`; a violation means the acting user's row is gone. */ +const WORKSPACE_USER_FK_CONSTRAINTS = new Set([ + 'workspace_owner_id_user_id_fk', + 'workspace_billed_account_user_id_user_id_fk', +]) + const logger = createLogger('WorkspaceCreate') export interface CreateWorkspaceParams { @@ -208,6 +216,13 @@ export async function createWorkspace(params: CreateWorkspaceParams) { createWorkspaceInTransaction(tx, { ...params, governingPermissionGroupOrganizationId }) ) } catch (error) { + if ( + getPostgresErrorCode(error) === '23503' && + WORKSPACE_USER_FK_CONSTRAINTS.has(getPostgresConstraintName(error) ?? '') + ) { + logger.warn('Workspace creation raced account deletion', { userId: params.userId }) + throw new WorkspaceOwnerMissingError(params.userId) + } logger.error('Failed to create workspace', { userId: params.userId, error }) throw error } diff --git a/apps/sim/lib/workspaces/policy.ts b/apps/sim/lib/workspaces/policy.ts index 3b511b15945..42c2e336eb1 100644 --- a/apps/sim/lib/workspaces/policy.ts +++ b/apps/sim/lib/workspaces/policy.ts @@ -111,6 +111,18 @@ export interface WorkspaceCreationPolicy { blockedReasonCode?: 'organization-subscription-inactive' | 'permission-group-denied' } +/** + * The acting user's row is gone, so no workspace can reference it. Reached + * when a request still carrying a cached session cookie arrives after the + * account was deleted; the caller should answer as unauthenticated. + */ +export class WorkspaceOwnerMissingError extends Error { + constructor(userId: string) { + super(`User ${userId} no longer exists`) + this.name = 'WorkspaceOwnerMissingError' + } +} + export class WorkspaceCreationContextChangedError extends Error { constructor(message = 'Workspace creation context changed before the workspace was inserted') { super(message)