From f2731062102fc8c62a2ee3826c367365ce65ee1b Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 22:57:35 -0700 Subject: [PATCH 1/4] fix(knowledge): key vision checkpoints by title and keep excluded images off while disabled --- .../confluence-images.integration.ts | 24 ++++++++++ .../lib/knowledge/application/connectors.ts | 46 +++++++++++++++++++ .../knowledge/documents/image-extraction.ts | 1 + .../documents/vision-checkpoints.test.ts | 4 +- .../knowledge/documents/vision-checkpoints.ts | 9 ++-- 5 files changed, 80 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts b/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts index 07bfbae0788..3e19532b400 100644 --- a/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts @@ -116,6 +116,7 @@ import { import { createKnowledgeConnector, updateKnowledgeConnector, + updateKnowledgeConnectorDocuments, } from '@/lib/knowledge/application/connectors' import * as connectorTokens from '@/lib/knowledge/connectors/access-token' import { executeSync } from '@/lib/knowledge/connectors/sync-engine' @@ -179,6 +180,12 @@ describe('Confluence image attachments through sync, indexing and reconciliation }) await syncFinished(ids.connectorId) } + function setExcluded(documentId: string, operation: 'exclude' | 'restore') { + return updateKnowledgeConnectorDocuments.execute({ + principal: { kind: 'session', userId: ids.aliceId, sessionId: 'confluence-images' }, + input: { connectorId: ids.connectorId, operation, documentIds: [documentId] }, + }) + } async function imageRemovalPending() { const [row] = await db .select({ pending: knowledgeConnector.imageRemovalPending }) @@ -306,6 +313,23 @@ describe('Confluence image attachments through sync, indexing and reconciliation expect(fixture.extractions.sort()).toEqual(['image-a.png', 'image-b.png']) }, 120_000) + it('keeps an excluded image excluded while Image Attachments is off', async () => { + fixture.images.set('image-a', 1) + await sync() + const [excluded] = await images() + await setExcluded(excluded.id, 'exclude') + + await setImageAttachments('off') + await expect(setExcluded(excluded.id, 'restore')).rejects.toThrow( + 'Turn on image attachments for this connector before restoring an image.' + ) + const [kept] = await images() + expect(kept).toMatchObject({ id: excluded.id, userExcluded: true, enabled: false }) + + await setImageAttachments('semantic') + expect(await setExcluded(excluded.id, 'restore')).toMatchObject({ count: 1 }) + }, 120_000) + it('keeps images off for a connector created before the setting existed', async () => { fixture.images.set('image-a', 1) await configure(undefined) diff --git a/apps/sim/lib/knowledge/application/connectors.ts b/apps/sim/lib/knowledge/application/connectors.ts index 50cf438aa61..210e6e1ffee 100644 --- a/apps/sim/lib/knowledge/application/connectors.ts +++ b/apps/sim/lib/knowledge/application/connectors.ts @@ -83,6 +83,10 @@ import { MAX_KNOWLEDGE_CONNECTOR_DOCUMENT_PAGE_SIZE, MAX_KNOWLEDGE_CONNECTOR_DOCUMENT_SEARCH_LENGTH, } from '@/lib/knowledge/constants' +import { + isImageExtractionEnabled, + SEMANTIC_IMAGE_MIME_TYPES, +} from '@/lib/knowledge/documents/image-extraction-policy' import { documentProcessingOutcomeSelection, failedDocumentCondition, @@ -1270,6 +1274,47 @@ export const listKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCase( }, }) +/** + * An excluded image keeps its indexed text, and turning image attachments off + * skips excluded rows so the exclusion survives turning them back on. Restoring + * one while images are off would re-enable text the source no longer lists. + */ +async function assertImageRestoreAllowed( + connector: { id: string; connectorType: string }, + documentIds: string[] +) { + const imageExtraction = getConnectorMeta(connector.connectorType)?.imageExtraction + if (!imageExtraction) return + const [stored] = await db + .select({ sourceConfig: knowledgeConnector.sourceConfig }) + .from(knowledgeConnector) + .where(eq(knowledgeConnector.id, connector.id)) + .limit(1) + if ( + !stored || + isImageExtractionEnabled(imageExtraction, stored.sourceConfig as Record) + ) { + return + } + const [image] = await db + .select({ id: document.id }) + .from(document) + .where( + and( + eq(document.connectorId, connector.id), + inArray(document.id, documentIds), + inArray(document.mimeType, [...SEMANTIC_IMAGE_MIME_TYPES]) + ) + ) + .limit(1) + if (image) { + throw new OrchestrationError( + 'validation', + 'Turn on image attachments for this connector before restoring an image.' + ) + } +} + export const updateKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCase({ operation: knowledgeOperations.updateConnectorDocuments, resolveContext: ({ @@ -1294,6 +1339,7 @@ export const updateKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCas if (restoring && !requiresConnectorIndexing(context.knowledgeBase.isSearchIndex)) { throw new OrchestrationError('validation', 'This search index is inactive; use Sim Search.') } + if (restoring) await assertImageRestoreAllowed(context.connector, documentIds) const updated = await db .update(document) .set({ userExcluded: !restoring, enabled: restoring }) diff --git a/apps/sim/lib/knowledge/documents/image-extraction.ts b/apps/sim/lib/knowledge/documents/image-extraction.ts index bd7b5c75287..c0eed783044 100644 --- a/apps/sim/lib/knowledge/documents/image-extraction.ts +++ b/apps/sim/lib/knowledge/documents/image-extraction.ts @@ -298,6 +298,7 @@ export async function extractImageText(input: { ? createVisionCheckpoint({ context: input.checkpoint, source, + filename, model, contractVersion: IMAGE_EXTRACTION_REPRESENTATION_VERSION, }) diff --git a/apps/sim/lib/knowledge/documents/vision-checkpoints.test.ts b/apps/sim/lib/knowledge/documents/vision-checkpoints.test.ts index 73e643f8c69..ae6194632f6 100644 --- a/apps/sim/lib/knowledge/documents/vision-checkpoints.test.ts +++ b/apps/sim/lib/knowledge/documents/vision-checkpoints.test.ts @@ -34,6 +34,7 @@ function checkpoint(overrides: Partial return createVisionCheckpoint({ context, source, + filename: 'architecture.png', model: 'gemini-3.8-flash', contractVersion: 'image-extract-v1', ...overrides, @@ -94,12 +95,13 @@ describe('vision extraction checkpoints', () => { it.each([ { source: Buffer.from('changed image bytes') }, + { filename: 'renamed.png' }, { context: { ...context, documentId: 'another-document' } }, { context: { ...context, indexingPassId: 'new-pass' } }, { model: 'gpt-5.4-mini' }, { contractVersion: 'image-extract-v2' }, ])( - 'never reuses a checkpoint across source, pass, model or contract changes: %o', + 'never reuses a checkpoint across source, title, pass, model or contract changes: %o', async (change) => { await checkpoint().save(result) expect(await checkpoint(change).load()).toBeNull() diff --git a/apps/sim/lib/knowledge/documents/vision-checkpoints.ts b/apps/sim/lib/knowledge/documents/vision-checkpoints.ts index e13ac4c10b7..661fd260876 100644 --- a/apps/sim/lib/knowledge/documents/vision-checkpoints.ts +++ b/apps/sim/lib/knowledge/documents/vision-checkpoints.ts @@ -35,16 +35,18 @@ export type VisionExtractionCheckpoint = z.infer /** * Retry cache for one image's vision extraction, never a record: a pass that dies * after the paid call but before the index swap resumes from it instead of paying - * again. Source bytes, the canonical document/pass identity, the model and the - * extraction contract version all participate in the address. + * again. Every model input (source bytes and the title the prompt names), the + * canonical document/pass identity, the model and the extraction contract version + * participate in the address. */ export function createVisionCheckpoint(options: { context: OcrCheckpointContext source: Buffer + filename: string model: string contractVersion: string }) { - const { context, source, model, contractVersion } = options + const { context, source, filename, model, contractVersion } = options if (!context.knowledgeBaseId || !context.documentId || !context.indexingPassId) { throw new Error('Vision checkpoint requires a canonical processing identity') } @@ -53,6 +55,7 @@ export function createVisionCheckpoint(options: { version: 1, ...context, sourceHash: sha256Hex(source), + filename, model, contractVersion, }) From 55d284070627a837504244dd1374145b29b9539e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 23:07:23 -0700 Subject: [PATCH 2/4] fix(knowledge): check the image setting and restore in one locked transaction --- .../confluence-images.integration.ts | 7 ++- .../lib/knowledge/application/connectors.ts | 55 +++++++++---------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts b/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts index 3e19532b400..8d7280230c6 100644 --- a/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts +++ b/apps/sim/lib/knowledge/__integration__/confluence-images.integration.ts @@ -183,7 +183,12 @@ describe('Confluence image attachments through sync, indexing and reconciliation function setExcluded(documentId: string, operation: 'exclude' | 'restore') { return updateKnowledgeConnectorDocuments.execute({ principal: { kind: 'session', userId: ids.aliceId, sessionId: 'confluence-images' }, - input: { connectorId: ids.connectorId, operation, documentIds: [documentId] }, + input: { + knowledgeBaseId: ids.knowledgeBaseId, + connectorId: ids.connectorId, + operation, + documentIds: [documentId], + }, }) } async function imageRemovalPending() { diff --git a/apps/sim/lib/knowledge/application/connectors.ts b/apps/sim/lib/knowledge/application/connectors.ts index 210e6e1ffee..394843900cb 100644 --- a/apps/sim/lib/knowledge/application/connectors.ts +++ b/apps/sim/lib/knowledge/application/connectors.ts @@ -11,7 +11,7 @@ import { } from '@sim/db/schema' import { toError } from '@sim/utils/errors' import { escapeLikePattern } from '@sim/utils/string' -import { and, asc, desc, eq, inArray, isNull, lt, or, sql } from 'drizzle-orm' +import { and, asc, desc, eq, inArray, isNull, lt, or, type SQL, sql } from 'drizzle-orm' import type { ConnectorDocumentFilter } from '@/lib/api/contracts/knowledge/connectors' import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' import { requireCurrentHumanRole } from '@/lib/core/application' @@ -29,6 +29,7 @@ import { import { redactKnownSensitiveValues } from '@/lib/core/security/redaction' import { generateRequestId } from '@/lib/core/utils/request' import { resolveCredentialTokenIdentity } from '@/lib/credentials/access' +import type { DbTransaction } from '@/lib/db/types' import { parseExactEnvironmentReference } from '@/lib/environment/reference' import { resolveEffectiveEnvironmentVariables } from '@/lib/environment/utils' import { requireKnowledgeMemberAccessAvailable } from '@/lib/knowledge/access/availability' @@ -1277,35 +1278,32 @@ export const listKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCase( /** * An excluded image keeps its indexed text, and turning image attachments off * skips excluded rows so the exclusion survives turning them back on. Restoring - * one while images are off would re-enable text the source no longer lists. + * one while images are off would re-enable text the source no longer lists. The + * connector row stays locked until the restore commits, so a concurrent change of + * the setting either sees the restored image or is seen by this check. */ async function assertImageRestoreAllowed( + tx: DbTransaction, connector: { id: string; connectorType: string }, - documentIds: string[] + restorable: SQL | undefined ) { const imageExtraction = getConnectorMeta(connector.connectorType)?.imageExtraction if (!imageExtraction) return - const [stored] = await db + const [stored] = await tx .select({ sourceConfig: knowledgeConnector.sourceConfig }) .from(knowledgeConnector) .where(eq(knowledgeConnector.id, connector.id)) - .limit(1) + .for('update') if ( !stored || isImageExtractionEnabled(imageExtraction, stored.sourceConfig as Record) ) { return } - const [image] = await db + const [image] = await tx .select({ id: document.id }) .from(document) - .where( - and( - eq(document.connectorId, connector.id), - inArray(document.id, documentIds), - inArray(document.mimeType, [...SEMANTIC_IMAGE_MIME_TYPES]) - ) - ) + .where(and(restorable, inArray(document.mimeType, [...SEMANTIC_IMAGE_MIME_TYPES]))) .limit(1) if (image) { throw new OrchestrationError( @@ -1339,21 +1337,22 @@ export const updateKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCas if (restoring && !requiresConnectorIndexing(context.knowledgeBase.isSearchIndex)) { throw new OrchestrationError('validation', 'This search index is inactive; use Sim Search.') } - if (restoring) await assertImageRestoreAllowed(context.connector, documentIds) - const updated = await db - .update(document) - .set({ userExcluded: !restoring, enabled: restoring }) - .where( - and( - eq(document.connectorId, context.connectorId), - inArray(document.id, documentIds), - eq(document.userExcluded, restoring), - isNull(document.archivedAt), - isNull(document.deletedAt), - knowledgeAccessCondition(await context.access.get()) - ) - ) - .returning({ id: document.id }) + const target = and( + eq(document.connectorId, context.connectorId), + inArray(document.id, documentIds), + eq(document.userExcluded, restoring), + isNull(document.archivedAt), + isNull(document.deletedAt), + knowledgeAccessCondition(await context.access.get()) + ) + const updated = await db.transaction(async (tx) => { + if (restoring) await assertImageRestoreAllowed(tx, context.connector, target) + return tx + .update(document) + .set({ userExcluded: !restoring, enabled: restoring }) + .where(target) + .returning({ id: document.id }) + }) return { operation: input.operation, count: updated.length, From 851d2651a76040938f8dced5ca2148d17276d0fc Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 23:17:15 -0700 Subject: [PATCH 3/4] fix(knowledge): take the image restore locks in sync's order --- .../lib/knowledge/application/connectors.ts | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/apps/sim/lib/knowledge/application/connectors.ts b/apps/sim/lib/knowledge/application/connectors.ts index 394843900cb..1df907184a9 100644 --- a/apps/sim/lib/knowledge/application/connectors.ts +++ b/apps/sim/lib/knowledge/application/connectors.ts @@ -11,7 +11,7 @@ import { } from '@sim/db/schema' import { toError } from '@sim/utils/errors' import { escapeLikePattern } from '@sim/utils/string' -import { and, asc, desc, eq, inArray, isNull, lt, or, type SQL, sql } from 'drizzle-orm' +import { and, asc, desc, eq, inArray, isNull, lt, or, sql } from 'drizzle-orm' import type { ConnectorDocumentFilter } from '@/lib/api/contracts/knowledge/connectors' import type { BillingAttributionSnapshot } from '@/lib/billing/core/billing-attribution' import { requireCurrentHumanRole } from '@/lib/core/application' @@ -1278,34 +1278,31 @@ export const listKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCase( /** * An excluded image keeps its indexed text, and turning image attachments off * skips excluded rows so the exclusion survives turning them back on. Restoring - * one while images are off would re-enable text the source no longer lists. The - * connector row stays locked until the restore commits, so a concurrent change of - * the setting either sees the restored image or is seen by this check. + * one while images are off would re-enable text the source no longer lists. + * + * Runs after the restore's document update in the same transaction, matching a + * sync's lock order (documents, then a share lock on the connector). The share + * lock holds off a concurrent change of the setting until the restore commits, + * so that change queues removal of the restored image or this check sees it. */ async function assertImageRestoreAllowed( tx: DbTransaction, connector: { id: string; connectorType: string }, - restorable: SQL | undefined + restored: { mimeType: string }[] ) { const imageExtraction = getConnectorMeta(connector.connectorType)?.imageExtraction - if (!imageExtraction) return + if (!imageExtraction || !restored.some((row) => SEMANTIC_IMAGE_MIME_TYPES.has(row.mimeType))) { + return + } const [stored] = await tx .select({ sourceConfig: knowledgeConnector.sourceConfig }) .from(knowledgeConnector) .where(eq(knowledgeConnector.id, connector.id)) - .for('update') + .for('share') if ( - !stored || - isImageExtractionEnabled(imageExtraction, stored.sourceConfig as Record) + stored && + !isImageExtractionEnabled(imageExtraction, stored.sourceConfig as Record) ) { - return - } - const [image] = await tx - .select({ id: document.id }) - .from(document) - .where(and(restorable, inArray(document.mimeType, [...SEMANTIC_IMAGE_MIME_TYPES]))) - .limit(1) - if (image) { throw new OrchestrationError( 'validation', 'Turn on image attachments for this connector before restoring an image.' @@ -1337,21 +1334,24 @@ export const updateKnowledgeConnectorDocuments = defineAuthorizedKnowledgeUseCas if (restoring && !requiresConnectorIndexing(context.knowledgeBase.isSearchIndex)) { throw new OrchestrationError('validation', 'This search index is inactive; use Sim Search.') } - const target = and( - eq(document.connectorId, context.connectorId), - inArray(document.id, documentIds), - eq(document.userExcluded, restoring), - isNull(document.archivedAt), - isNull(document.deletedAt), - knowledgeAccessCondition(await context.access.get()) - ) + const access = await context.access.get() const updated = await db.transaction(async (tx) => { - if (restoring) await assertImageRestoreAllowed(tx, context.connector, target) - return tx + const rows = await tx .update(document) .set({ userExcluded: !restoring, enabled: restoring }) - .where(target) - .returning({ id: document.id }) + .where( + and( + eq(document.connectorId, context.connectorId), + inArray(document.id, documentIds), + eq(document.userExcluded, restoring), + isNull(document.archivedAt), + isNull(document.deletedAt), + knowledgeAccessCondition(access) + ) + ) + .returning({ id: document.id, mimeType: document.mimeType }) + if (restoring) await assertImageRestoreAllowed(tx, context.connector, rows) + return rows }) return { operation: input.operation, From 2201de64ced07a577711195e11ab1072739e6121 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 23:29:27 -0700 Subject: [PATCH 4/4] improvement(knowledge): read the connector image setting with toRecord --- apps/sim/lib/knowledge/application/connectors.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/knowledge/application/connectors.ts b/apps/sim/lib/knowledge/application/connectors.ts index 1df907184a9..aee24d4ba78 100644 --- a/apps/sim/lib/knowledge/application/connectors.ts +++ b/apps/sim/lib/knowledge/application/connectors.ts @@ -10,6 +10,7 @@ import { knowledgeConnectorSyncLog, } from '@sim/db/schema' import { toError } from '@sim/utils/errors' +import { toRecord } from '@sim/utils/object' import { escapeLikePattern } from '@sim/utils/string' import { and, asc, desc, eq, inArray, isNull, lt, or, sql } from 'drizzle-orm' import type { ConnectorDocumentFilter } from '@/lib/api/contracts/knowledge/connectors' @@ -1299,10 +1300,7 @@ async function assertImageRestoreAllowed( .from(knowledgeConnector) .where(eq(knowledgeConnector.id, connector.id)) .for('share') - if ( - stored && - !isImageExtractionEnabled(imageExtraction, stored.sourceConfig as Record) - ) { + if (stored && !isImageExtractionEnabled(imageExtraction, toRecord(stored.sourceConfig))) { throw new OrchestrationError( 'validation', 'Turn on image attachments for this connector before restoring an image.'