From a7dad9526f998c0610d0531130a4dea859f9f315 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 18 Aug 2026 14:19:44 -0700 Subject: [PATCH 1/4] feat(connectors): hand source files to the document pipeline instead of extracting them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connector that extracted text itself stranded the document on a second, weaker parser. The shared pipeline routes PDFs to OCR — the only way a scanned page is readable at all — and owns every other format's parser, but its OCR branch is gated on `mimeType === 'application/pdf'` and connector documents were stored as `text/plain`, so a connector PDF could never reach it. The same file dragged into the UI was read by OCR; synced through a connector it got the local parser. `ExternalDocument` can now carry the source file itself, and SharePoint and OneDrive hand over anything the knowledge base can parse rather than extracting it. The sync engine stores those bytes under the file's own name and type, so the pipeline parses them exactly as it would an upload of the same file. Formats that are already text stay on the text path: HTML still reduces to plain text and the rest are UTF-8 decodes, so nothing already indexed changes representation. The MIME type is derived from the extension rather than the source's own declaration, so a provider that omits or mislabels it cannot strand a PDF on the non-OCR path. Re-syncing an existing document now rewrites `mimeType` too, which is what lets one stored as connector-extracted text stop declaring `text/plain`. This removes the duplicate extraction path rather than leaving both in place: `extractConnectorText` is text-only, and the guard against fabricated content moves to the pipeline where parsing now happens. That guard still matters — `DocParser` and `PptxParser` never throw, returning a placeholder sentence or scraped archive bytes on a legacy binary or an image-only deck — so a `degraded` result now fails the document with the same actionable message it produced before, naming the modern container for legacy formats. The in-flight byte budget already accounted for this: `estimateOpSizeBytes` reads the true source size from listing metadata, so batching reserved against the real file all along and merely over-reserved while only text was stored. --- apps/sim/connectors/onedrive/onedrive.ts | 28 ++-- .../connectors/sharepoint/sharepoint.test.ts | 54 ++++--- apps/sim/connectors/sharepoint/sharepoint.ts | 31 ++-- apps/sim/connectors/types.ts | 21 ++- apps/sim/connectors/utils.test.ts | 137 +++++------------- apps/sim/connectors/utils.ts | 112 +++++--------- .../lib/knowledge/connectors/sync-engine.ts | 100 ++++++++----- .../knowledge/documents/document-processor.ts | 30 +++- .../documents/unreadable-document.test.ts | 65 +++++++++ 9 files changed, 300 insertions(+), 278 deletions(-) create mode 100644 apps/sim/lib/knowledge/documents/unreadable-document.test.ts diff --git a/apps/sim/connectors/onedrive/onedrive.ts b/apps/sim/connectors/onedrive/onedrive.ts index 54d1caba7f2..63a427d59d5 100644 --- a/apps/sim/connectors/onedrive/onedrive.ts +++ b/apps/sim/connectors/onedrive/onedrive.ts @@ -6,14 +6,13 @@ import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/ import { CONNECTOR_MAX_FILE_BYTES, ConnectorFileTooLargeError, - ConnectorTextExtractionError, connectorFileExtension, extractConnectorText, - extractionFailedSkipReason, isIndexableConnectorFile, isSkippedDocument, markSkipped, parseTagDate, + pipelineParsedMimeType, readBodyWithLimit, sizeLimitSkipReason, stubOrSkipBySize, @@ -103,13 +102,19 @@ async function downloadFileContent(accessToken: string, fileId: string): Promise * Fetches a file and extracts its indexable text — a UTF-8 decode for text * formats, and the shared knowledge-base parsers for Office documents and PDFs. */ -async function fetchFileContent( +async function fetchFilePayload( accessToken: string, fileId: string, fileName: string -): Promise { +): Promise> { const buffer = await downloadFileContent(accessToken, fileId) - return extractConnectorText(buffer, fileName) + + const mimeType = pipelineParsedMimeType(fileName) + if (mimeType) { + return { content: '', mimeType, sourceFile: { bytes: buffer, fileName, mimeType } } + } + + return { content: extractConnectorText(buffer, fileName), mimeType: 'text/plain' } } /** @@ -377,23 +382,16 @@ export const onedriveConnector: ConnectorConfig = { if (!item.file || !isIndexableConnectorFile(item.name)) return null try { - const content = await fetchFileContent(accessToken, item.id, item.name) - if (!content.trim()) return null + const payload = await fetchFilePayload(accessToken, item.id, item.name) + if (!payload.sourceFile && !payload.content.trim()) return null const stub = fileToStub(item) - return { ...stub, content, contentDeferred: false } + return { ...stub, ...payload, contentDeferred: false } } catch (error) { if (error instanceof ConnectorFileTooLargeError) { logger.info('Skipping oversized OneDrive file', { fileId: item.id, name: item.name }) return markSkipped(fileToStub(item), sizeLimitSkipReason(error.limitBytes)) } - if (error instanceof ConnectorTextExtractionError) { - logger.info('Skipping OneDrive file with no extractable text', { - fileId: item.id, - name: item.name, - }) - return markSkipped(fileToStub(item), extractionFailedSkipReason(error.extension)) - } /** * A transport or Graph failure that survived `fetchWithRetry`. Returning * `null` would drop the file from the run with no `failed` row and no error diff --git a/apps/sim/connectors/sharepoint/sharepoint.test.ts b/apps/sim/connectors/sharepoint/sharepoint.test.ts index 9d9a048eef1..8c0def7b1be 100644 --- a/apps/sim/connectors/sharepoint/sharepoint.test.ts +++ b/apps/sim/connectors/sharepoint/sharepoint.test.ts @@ -3,16 +3,12 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockFetchWithRetry, mockParseBuffer } = vi.hoisted(() => ({ - mockFetchWithRetry: vi.fn(), - mockParseBuffer: vi.fn(), -})) +const { mockFetchWithRetry } = vi.hoisted(() => ({ mockFetchWithRetry: vi.fn() })) vi.mock('@/lib/knowledge/documents/utils', () => ({ fetchWithRetry: mockFetchWithRetry, VALIDATE_RETRY_OPTIONS: {}, })) -vi.mock('@/lib/file-parsers', () => ({ parseBuffer: mockParseBuffer })) vi.mock('@/components/icons', () => ({ MicrosoftSharepointIcon: () => null })) import { @@ -496,45 +492,45 @@ describe('getDocument content extraction', () => { ) } - it('indexes the parsed text of an Office document', async () => { - mockGraph({ ...itemRoute('f1', 'SOP.docx'), ...contentRoute('f1', 'ignored') }) - mockParseBuffer.mockResolvedValue({ - content: 'Approved vendor list', - metadata: { extractionMethod: 'mammoth' }, - }) + /** + * The connector hands an Office document over untouched so the shared pipeline + * parses it — the same path an upload of the same file takes, which is what + * routes PDFs through OCR. + */ + it('delivers an Office document as its source file rather than extracting it', async () => { + mockGraph({ ...itemRoute('f1', 'SOP.docx'), ...contentRoute('f1', 'PK-docx-bytes') }) const doc = await get('f1') - expect(doc?.content).toBe('Approved vendor list') - expect(doc?.skippedReason).toBeUndefined() + expect(doc?.content).toBe('') + expect(doc?.sourceFile?.fileName).toBe('SOP.docx') + expect(doc?.sourceFile?.mimeType).toBe( + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' + ) + expect(doc?.sourceFile?.bytes.toString()).toBe('PK-docx-bytes') + expect(doc?.mimeType).toBe( + 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' + ) expect(doc?.contentDeferred).toBe(false) }) - /** - * A degraded extraction must become a visible `failed` row, not a silent drop - * and not indexed placeholder text — the same treatment oversized files get. - */ - it('surfaces a degraded extraction as a skipped document with an actionable reason', async () => { - mockGraph({ ...itemRoute('f2', 'Deck.ppt'), ...contentRoute('f2', 'ole2') }) - mockParseBuffer.mockResolvedValue({ - content: 'Unable to extract text from PowerPoint file.', - metadata: { extractionMethod: 'fallback', degraded: true }, - }) + it('declares a PDF as application/pdf so the pipeline can route it to OCR', async () => { + mockGraph({ ...itemRoute('f4', 'Contract.pdf'), ...contentRoute('f4', '%PDF-1.7 bytes') }) - const doc = await get('f2') + const doc = await get('f4') - expect(doc?.content).toBe('') - expect(doc?.skippedReason).toContain('PPTX') - expect(doc?.externalId).toBe('f2') + expect(doc?.mimeType).toBe('application/pdf') + expect(doc?.sourceFile?.mimeType).toBe('application/pdf') }) - it('reads a text file without invoking a parser', async () => { + it('still extracts a text file itself, since there is nothing for a parser to do', async () => { mockGraph({ ...itemRoute('f3', 'notes.txt'), ...contentRoute('f3', 'plain notes') }) const doc = await get('f3') expect(doc?.content).toBe('plain notes') - expect(mockParseBuffer).not.toHaveBeenCalled() + expect(doc?.sourceFile).toBeUndefined() + expect(doc?.mimeType).toBe('text/plain') }) }) diff --git a/apps/sim/connectors/sharepoint/sharepoint.ts b/apps/sim/connectors/sharepoint/sharepoint.ts index c257ef4b3b1..3ebd325555b 100644 --- a/apps/sim/connectors/sharepoint/sharepoint.ts +++ b/apps/sim/connectors/sharepoint/sharepoint.ts @@ -6,14 +6,13 @@ import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/ import { CONNECTOR_MAX_FILE_BYTES, ConnectorFileTooLargeError, - ConnectorTextExtractionError, connectorFileExtension, extractConnectorText, - extractionFailedSkipReason, isIndexableConnectorFile, isSkippedDocument, markSkipped, parseTagDate, + pipelineParsedMimeType, readBodyWithLimit, sizeLimitSkipReason, stubOrSkipBySize, @@ -214,14 +213,20 @@ async function downloadFileContent( * Fetches a file and extracts its indexable text — a UTF-8 decode for text * formats, and the shared knowledge-base parsers for Office documents and PDFs. */ -async function fetchFileContent( +async function fetchFilePayload( accessToken: string, driveId: string, itemId: string, fileName: string -): Promise { +): Promise> { const buffer = await downloadFileContent(accessToken, driveId, itemId, fileName) - return extractConnectorText(buffer, fileName) + + const mimeType = pipelineParsedMimeType(fileName) + if (mimeType) { + return { content: '', mimeType, sourceFile: { bytes: buffer, fileName, mimeType } } + } + + return { content: extractConnectorText(buffer, fileName), mimeType: 'text/plain' } } /** @@ -925,11 +930,11 @@ export const sharepointConnector: ConnectorConfig = { } try { - const content = await fetchFileContent(accessToken, driveId, item.id, item.name) - if (!content.trim()) return null + const payload = await fetchFilePayload(accessToken, driveId, item.id, item.name) + if (!payload.sourceFile && !payload.content.trim()) return null const stub = itemToStub(item, siteName ?? siteUrl) - return { ...stub, content, contentDeferred: false } + return { ...stub, ...payload, contentDeferred: false } } catch (error) { if (error instanceof ConnectorFileTooLargeError) { logger.info('Skipping oversized SharePoint file', { fileId: item.id, name: item.name }) @@ -938,16 +943,6 @@ export const sharepointConnector: ConnectorConfig = { sizeLimitSkipReason(error.limitBytes) ) } - if (error instanceof ConnectorTextExtractionError) { - logger.info('Skipping SharePoint file with no extractable text', { - fileId: item.id, - name: item.name, - }) - return markSkipped( - itemToStub(item, siteName ?? siteUrl), - extractionFailedSkipReason(error.extension) - ) - } /** * A transport or Graph failure that survived `fetchWithRetry`. Returning * `null` would drop the file from the run with no `failed` row and no error diff --git a/apps/sim/connectors/types.ts b/apps/sim/connectors/types.ts index cc96e68a7af..f16984208c3 100644 --- a/apps/sim/connectors/types.ts +++ b/apps/sim/connectors/types.ts @@ -29,10 +29,29 @@ export interface ExternalDocument { externalId: string /** Document title / filename */ title: string - /** Extracted text content */ + /** Extracted text content. Empty when {@link ExternalDocument.sourceFile} carries the document instead. */ content: string /** MIME type of the content */ mimeType: string + /** + * The source file itself, for connectors that hand over the original document + * rather than text they extracted from it. + * + * Preferred for any format the knowledge base can parse. Extracting inside a + * connector strands the document on a second, weaker parser: the shared + * pipeline routes PDFs to OCR (so scanned pages are readable at all) and owns + * every other format's parser, while a connector doing its own extraction + * stores plain text that no longer declares what it came from. + * + * Carried as one object so the bytes can never disagree with the name and type + * that describe them. + */ + sourceFile?: { + bytes: Buffer + /** Name whose extension names the format, e.g. `Report.pdf`. */ + fileName: string + mimeType: string + } /** Link back to the original document */ sourceUrl?: string /** Hash of content for change detection (format varies by connector) */ diff --git a/apps/sim/connectors/utils.test.ts b/apps/sim/connectors/utils.test.ts index 9474414bceb..6ba40e5200a 100644 --- a/apps/sim/connectors/utils.test.ts +++ b/apps/sim/connectors/utils.test.ts @@ -1,11 +1,9 @@ /** * @vitest-environment node */ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { ExternalDocument } from '@/connectors/types' -const { mockParseBuffer } = vi.hoisted(() => ({ mockParseBuffer: vi.fn() })) - vi.mock('@/components/icons', () => ({ JiraIcon: () => null, ConfluenceIcon: () => null, @@ -31,7 +29,6 @@ vi.mock('@/lib/knowledge/documents/utils', () => ({ fetchWithRetry: vi.fn(), VALIDATE_RETRY_OPTIONS: {}, })) -vi.mock('@/lib/file-parsers', () => ({ parseBuffer: mockParseBuffer })) vi.mock('@/tools/jira/utils', () => ({ extractAdfText: vi.fn(), getJiraCloudId: vi.fn() })) vi.mock('@/tools/confluence/utils', () => ({ getConfluenceCloudId: vi.fn() })) vi.mock('@/tools/jsm/utils', () => ({ @@ -65,13 +62,12 @@ import { sentryConnector } from '@/connectors/sentry/sentry' import { typeformConnector } from '@/connectors/typeform/typeform' import { ConnectorFileTooLargeError, - ConnectorTextExtractionError, extractConnectorText, - extractionFailedSkipReason, htmlToPlainText, isIndexableConnectorFile, isSkippedDocument, markSkipped, + pipelineParsedMimeType, readBodyWithLimit, sizeLimitSkipReason, takeIndexableWithinCap, @@ -1442,114 +1438,47 @@ describe('isIndexableConnectorFile', () => { }) describe('extractConnectorText', () => { - beforeEach(() => { - vi.clearAllMocks() + it('decodes a text format as UTF-8', () => { + expect(extractConnectorText(Buffer.from('a,b'), 'data.csv')).toBe('a,b') }) - it('routes a binary document format through the shared parsers', async () => { - mockParseBuffer.mockResolvedValue({ content: 'extracted docx text' }) - const buffer = Buffer.from('PK binary') - - const content = await extractConnectorText(buffer, 'Market Data SOP.docx') - - expect(content).toBe('extracted docx text') - expect(mockParseBuffer).toHaveBeenCalledWith(buffer, 'docx') + it('reduces HTML to plain text', () => { + expect(extractConnectorText(Buffer.from('

Hello world

'), 'page.htm')).toBe( + 'Hello world' + ) }) - it('passes each parsed variant to the parser under its own extension', async () => { - mockParseBuffer.mockResolvedValue({ content: 'text' }) - - for (const extension of ['docm', 'xlsm', 'xlsb', 'pptm', 'odt', 'ods', 'odp']) { - await extractConnectorText(Buffer.from('PK'), `file.${extension}`) - expect(mockParseBuffer).toHaveBeenLastCalledWith(expect.any(Buffer), extension) - } + it('leaves whitespace-only content alone for the caller to reject', () => { + expect(extractConnectorText(Buffer.from(' '), 'blank.txt')).toBe(' ') }) +}) +describe('pipelineParsedMimeType', () => { /** - * The formats that synced before this change must keep taking the byte-for-byte - * identical path. Sending `.csv` through `CsvParser` would silently reformat - * every already-indexed connector document on its next re-index. + * A format the shared parsers handle is delivered to them verbatim. Extracting + * it here would strand the document on a weaker parser — notably skipping the + * OCR the pipeline routes PDFs through — and discard the original bytes. */ - it('decodes already-supported text formats as UTF-8 without invoking a parser', async () => { - for (const name of ['notes.txt', 'data.csv', 'config.yaml', 'rows.tsv', 'feed.xml']) { - const content = await extractConnectorText(Buffer.from('a,b'), name) - expect(content).toBe('a,b') + it.each([ + ['Report.pdf', 'application/pdf'], + ['Deck.pptx', 'application/vnd.openxmlformats-officedocument.presentationml.presentation'], + ['Book.xlsm', 'application/vnd.ms-excel.sheet.macroEnabled.12'], + ['Notes.odt', 'application/vnd.oasis.opendocument.text'], + ['Legacy.doc', 'application/msword'], + ])('hands %s to the pipeline as %s', (fileName, mimeType) => { + expect(pipelineParsedMimeType(fileName)).toBe(mimeType) + }) + + it('leaves text formats to the connector', () => { + for (const name of ['notes.txt', 'data.csv', 'page.htm', 'feed.xml', 'rows.tsv']) { + expect(pipelineParsedMimeType(name)).toBeUndefined() } - expect(mockParseBuffer).not.toHaveBeenCalled() - }) - - it('reduces HTML to plain text rather than parsing it', async () => { - const content = await extractConnectorText(Buffer.from('

Hello world

'), 'page.htm') - - expect(content).toBe('Hello world') - expect(mockParseBuffer).not.toHaveBeenCalled() - }) - - it('falls back to a UTF-8 decode for an extension with no parser', async () => { - const content = await extractConnectorText(Buffer.from('plain'), 'notes.unknownext') - - expect(content).toBe('plain') - expect(mockParseBuffer).not.toHaveBeenCalled() - }) - - it('propagates a parser failure so the sync records a failed document', async () => { - mockParseBuffer.mockRejectedValue(new Error('corrupt archive')) - - await expect(extractConnectorText(Buffer.from('bad'), 'broken.docx')).rejects.toThrow( - 'corrupt archive' - ) - }) - - /** - * `DocParser` and `PptxParser` never throw by design: on a legacy binary or an - * image-only deck they return scraped ZIP internals or an English placeholder - * sentence so an interactive upload still shows the user something. Indexing - * that would embed junk, so a degraded result must not become content. - */ - it('rejects a degraded extraction instead of indexing placeholder text', async () => { - mockParseBuffer.mockResolvedValue({ - content: 'Unable to extract text from PowerPoint file. Please ensure the file contains text.', - metadata: { extractionMethod: 'fallback', degraded: true }, - }) - - await expect(extractConnectorText(Buffer.from('ole2'), 'Deck.ppt')).rejects.toThrow( - ConnectorTextExtractionError - ) - }) - - it('rejects an extraction that produced only whitespace', async () => { - mockParseBuffer.mockResolvedValue({ content: ' \n ', metadata: {} }) - - await expect(extractConnectorText(Buffer.from('pdf'), 'scanned.pdf')).rejects.toThrow( - ConnectorTextExtractionError - ) - }) - - it('carries the extension so the caller can name the format in its skip reason', async () => { - mockParseBuffer.mockResolvedValue({ content: '', metadata: {} }) - - await expect(extractConnectorText(Buffer.from('x'), 'Deck.PPT')).rejects.toMatchObject({ - extension: 'ppt', - fileName: 'Deck.PPT', - }) - }) - - it('does not apply the degraded check to text formats', async () => { - const content = await extractConnectorText(Buffer.from(' '), 'blank.txt') - - expect(content).toBe(' ') - expect(mockParseBuffer).not.toHaveBeenCalled() - }) -}) - -describe('extractionFailedSkipReason', () => { - it('tells the user which modern format to re-save a legacy file as', () => { - expect(extractionFailedSkipReason('doc')).toContain('DOCX') - expect(extractionFailedSkipReason('ppt')).toContain('PPTX') - expect(extractionFailedSkipReason('xls')).toContain('XLSX') }) - it('explains the likely cause for a modern format', () => { - expect(extractionFailedSkipReason('pdf')).toMatch(/scanned, image-only, or password-protected/) + /** Derived from the extension, so a mislabelled source cannot misroute a PDF. */ + it('is case-insensitive and ignores unknown formats', () => { + expect(pipelineParsedMimeType('REPORT.PDF')).toBe('application/pdf') + expect(pipelineParsedMimeType('archive.zip')).toBeUndefined() + expect(pipelineParsedMimeType('README')).toBeUndefined() }) }) diff --git a/apps/sim/connectors/utils.ts b/apps/sim/connectors/utils.ts index 930b5e32c0b..608de615bd1 100644 --- a/apps/sim/connectors/utils.ts +++ b/apps/sim/connectors/utils.ts @@ -171,26 +171,32 @@ const CONNECTOR_TEXT_EXTENSIONS = [ * `rtf` is deliberately absent: no bundled library extracts it, and `DocParser` * would pass its control words through as if they were prose. See * {@link CONNECTOR_INDEXABLE_EXTENSIONS} for how an unsupported format surfaces. + * + * Mapping each to its MIME type rather than listing extensions alone lets the + * stored object declare what it is, which is what the pipeline's OCR routing + * reads. Derived from the extension rather than trusting the source's own + * declaration, so a provider that omits or mislabels it cannot strand a PDF on + * the non-OCR path. */ -const CONNECTOR_PARSED_EXTENSIONS = [ - 'pdf', - 'doc', - 'docx', - 'docm', - 'dotx', - 'xls', - 'xlsx', - 'xlsm', - 'xlsb', - 'xltx', - 'ppt', - 'pptx', - 'pptm', - 'potx', - 'odt', - 'ods', - 'odp', -] as const +const PIPELINE_PARSED_MIME_TYPES = new Map([ + ['pdf', 'application/pdf'], + ['doc', 'application/msword'], + ['docx', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'], + ['docm', 'application/vnd.ms-word.document.macroEnabled.12'], + ['dotx', 'application/vnd.openxmlformats-officedocument.wordprocessingml.template'], + ['xls', 'application/vnd.ms-excel'], + ['xlsx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet'], + ['xlsm', 'application/vnd.ms-excel.sheet.macroEnabled.12'], + ['xlsb', 'application/vnd.ms-excel.sheet.binary.macroEnabled.12'], + ['xltx', 'application/vnd.openxmlformats-officedocument.spreadsheetml.template'], + ['ppt', 'application/vnd.ms-powerpoint'], + ['pptx', 'application/vnd.openxmlformats-officedocument.presentationml.presentation'], + ['pptm', 'application/vnd.ms-powerpoint.presentation.macroEnabled.12'], + ['potx', 'application/vnd.openxmlformats-officedocument.presentationml.template'], + ['odt', 'application/vnd.oasis.opendocument.text'], + ['ods', 'application/vnd.oasis.opendocument.spreadsheet'], + ['odp', 'application/vnd.oasis.opendocument.presentation'], +]) /** * Every extension a file-based connector will download and index. @@ -202,7 +208,7 @@ const CONNECTOR_PARSED_EXTENSIONS = [ */ export const CONNECTOR_INDEXABLE_EXTENSIONS: ReadonlySet = new Set([ ...CONNECTOR_TEXT_EXTENSIONS, - ...CONNECTOR_PARSED_EXTENSIONS, + ...PIPELINE_PARSED_MIME_TYPES.keys(), ]) /** Extracts a lowercased, dotless extension from a file name. */ @@ -221,72 +227,32 @@ export function isIndexableConnectorFile(fileName: string): boolean { } /** - * Raised when a binary document yielded no text a search index should hold — - * either the parser produced nothing, or it reported a degraded extraction whose - * "content" is scraped bytes or a placeholder message. Callers surface it as a - * skipped document, the same way {@link ConnectorFileTooLargeError} is handled, - * so the file stays visible with an actionable reason instead of polluting the - * index or vanishing. - */ -export class ConnectorTextExtractionError extends Error { - constructor( - readonly fileName: string, - readonly extension: string - ) { - super(`No text could be extracted from "${fileName}"`) - this.name = 'ConnectorTextExtractionError' - } -} - -/** - * Human-readable skip reason for a document whose text could not be extracted. - * Legacy formats get the concrete remedy — re-saving genuinely fixes them, because - * the modern container is one the bundled parsers read. + * MIME type to store a file under when the shared pipeline should parse it, or + * `undefined` when the connector should decode it as text itself. + * + * A format the knowledge base can parse is handed over untouched: the pipeline + * routes PDFs to OCR and owns every other parser, so extracting here would strand + * the document on a weaker one and discard the original. */ -export function extractionFailedSkipReason(extension: string): string { - const legacyFormats: Record = { doc: 'DOCX', ppt: 'PPTX', xls: 'XLSX' } - const modernFormat = legacyFormats[extension] - return modernFormat - ? `No text could be extracted from this ${extension.toUpperCase()} file. Re-save it as ${modernFormat} to index it.` - : 'No text could be extracted from this file — it may be scanned, image-only, or password-protected.' +export function pipelineParsedMimeType(fileName: string): string | undefined { + const extension = connectorFileExtension(fileName) + return extension ? PIPELINE_PARSED_MIME_TYPES.get(extension) : undefined } /** * Converts a downloaded file body to indexable text. * - * Text formats are decoded as UTF-8 (with HTML additionally reduced to plain text), - * and binary document formats go through `parseBuffer`, which applies the OOXML - * zip-bomb guard and each parser's own extraction limits. An extension with no - * parser falls back to a UTF-8 decode rather than failing the file. - * - * A parsed format that yields no usable text throws {@link ConnectorTextExtractionError} - * rather than returning what the parser handed back. The `doc` and `ppt` parsers - * never throw by design — on a legacy binary or an image-only deck they return a - * placeholder sentence or raw ZIP internals, which an interactive upload can show - * a user but an automated sync must never embed. + * Only for formats that are already text — anything the shared parsers handle is + * delivered to them verbatim instead, via {@link pipelineParsedMimeType}. HTML is + * additionally reduced to plain text; everything else is a UTF-8 decode. */ -export async function extractConnectorText(buffer: Buffer, fileName: string): Promise { +export function extractConnectorText(buffer: Buffer, fileName: string): string { const extension = connectorFileExtension(fileName) if (extension === 'html' || extension === 'htm') { return htmlToPlainText(buffer.toString('utf8')) } - if (extension && (CONNECTOR_PARSED_EXTENSIONS as readonly string[]).includes(extension)) { - /** - * Imported here rather than at module scope: every connector imports this - * file, but only the file-based ones ever reach a binary document, and the - * parser registry pulls in SheetJS and friends. Mirrors how the parsers - * themselves defer `officeparser`/`mammoth`/`unpdf`. - */ - const { parseBuffer } = await import('@/lib/file-parsers') - const result = await parseBuffer(buffer, extension) - if (result.metadata?.degraded || !result.content.trim()) { - throw new ConnectorTextExtractionError(fileName, extension) - } - return result.content - } - return buffer.toString('utf8') } diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index dcbc354173b..160871be58e 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -66,17 +66,34 @@ function sanitizeStorageTitle(title: string): string { } /** - * Name a connector document's stored object carries. + * The bytes to store for a connector document, together with the name and type + * that describe them. * - * Connectors store already-extracted text while `document.filename` keeps the - * source file's name for display, so the stored object has to declare the format - * it actually holds: `resolveStoredArtifactExtension` picks the parser off this - * key, and a key ending in the source extension would re-parse extracted text as - * the original binary. Owning the `.txt` suffix here makes that structural rather - * than a convention each call site has to remember. + * The stored object must declare the format it actually holds, because + * `resolveStoredArtifactExtension` picks the parser off its storage key. A + * connector that hands over the source file keeps that file's own name and type, + * so the shared pipeline parses it exactly as an upload of the same file — which + * is what routes PDFs to OCR. A connector that extracted text itself stores + * `.txt`, since that is what the bytes now are; keeping the source extension + * there would re-parse extracted text as the original binary. */ -function connectorArtifactFileName(title: string): string { - return `${sanitizeStorageTitle(title)}.txt` +function connectorStoredArtifact(extDoc: ExternalDocument): { + bytes: Buffer + fileName: string + mimeType: string +} { + if (extDoc.sourceFile) { + return { + bytes: extDoc.sourceFile.bytes, + fileName: sanitizeStorageTitle(extDoc.sourceFile.fileName), + mimeType: extDoc.sourceFile.mimeType, + } + } + return { + bytes: Buffer.from(extDoc.content, 'utf-8'), + fileName: `${sanitizeStorageTitle(extDoc.title)}.txt`, + mimeType: 'text/plain', + } } type KnowledgeBaseLockingTx = Pick @@ -108,14 +125,17 @@ type DocClassification = * are left `unchanged` (re-indexing identical content would be pointless). */ export function classifyExternalDoc( - extDoc: Pick, + extDoc: Pick< + ExternalDocument, + 'content' | 'sourceFile' | 'contentDeferred' | 'contentHash' | 'skippedReason' + >, existing: { id: string; contentHash: string | null } | undefined, forceRehydrate = false ): DocClassification { if (extDoc.skippedReason) { return existing ? { type: 'unchanged' } : { type: 'skip' } } - if (!extDoc.content.trim() && !extDoc.contentDeferred) { + if (!hasPayload(extDoc) && !extDoc.contentDeferred) { return { type: 'drop' } } if (!existing) { @@ -130,6 +150,11 @@ export function classifyExternalDoc( return { type: 'unchanged' } } +/** Whether a document carries anything to index — extracted text or the source file. */ +function hasPayload(extDoc: Pick): boolean { + return extDoc.sourceFile !== undefined || extDoc.content.trim().length > 0 +} + /** Estimated source bytes for a pending op, taken from its listing metadata. */ function estimateOpSizeBytes(op: DocOp): number { // Skip ops load no content (just a row insert), so they do not count against the @@ -1015,7 +1040,7 @@ export async function executeSync( } return null } - if (!fullDoc?.content.trim()) { + if (!fullDoc || !hasPayload(fullDoc)) { // An empty re-fetch leaves an already-indexed update as last-known-good; count // it as unchanged so the totals still reconcile with documents seen. Not a // verified refresh, though — see failedExternalIds below. @@ -1046,6 +1071,7 @@ export async function executeSync( ...op.extDoc, title: fullDoc.title || op.extDoc.title, content: fullDoc.content, + sourceFile: fullDoc.sourceFile, contentHash: hydratedHash, contentDeferred: false, sourceUrl: fullDoc.sourceUrl ?? op.extDoc.sourceUrl, @@ -1670,18 +1696,17 @@ async function addDocument( sourceConfig?: Record ): Promise { const documentId = generateId() - const contentBuffer = Buffer.from(extDoc.content, 'utf-8') - const storedFileName = connectorArtifactFileName(extDoc.title) - const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${documentId}-`, storedFileName)}` + const artifact = connectorStoredArtifact(extDoc) + const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${documentId}-`, artifact.fileName)}` const fileInfo = await StorageService.uploadFile({ - file: contentBuffer, - fileName: storedFileName, - contentType: 'text/plain', + file: artifact.bytes, + fileName: artifact.fileName, + contentType: artifact.mimeType, context: 'knowledge-base', customKey, preserveKey: true, - metadata: kbOwnershipMetadata(kbOwner, storedFileName), + metadata: kbOwnershipMetadata(kbOwner, artifact.fileName), }) const fileUrl = `${getInternalApiBaseUrl()}${fileInfo.path}?context=knowledge-base` @@ -1703,8 +1728,8 @@ async function addDocument( filename: extDoc.title, fileUrl, storageKey: fileInfo.key, - fileSize: contentBuffer.length, - mimeType: 'text/plain', + fileSize: artifact.bytes.length, + mimeType: artifact.mimeType, chunkCount: 0, tokenCount: 0, characterCount: 0, @@ -1730,10 +1755,10 @@ async function addDocument( return { documentId, - filename: storedFileName, + filename: artifact.fileName, fileUrl, - fileSize: contentBuffer.length, - mimeType: 'text/plain', + fileSize: artifact.bytes.length, + mimeType: artifact.mimeType, } } @@ -1757,18 +1782,17 @@ async function updateDocument( .limit(1) const oldFileUrl = existingRows[0]?.fileUrl - const contentBuffer = Buffer.from(extDoc.content, 'utf-8') - const storedFileName = connectorArtifactFileName(extDoc.title) - const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${existingDocId}-`, storedFileName)}` + const artifact = connectorStoredArtifact(extDoc) + const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${existingDocId}-`, artifact.fileName)}` const fileInfo = await StorageService.uploadFile({ - file: contentBuffer, - fileName: storedFileName, - contentType: 'text/plain', + file: artifact.bytes, + fileName: artifact.fileName, + contentType: artifact.mimeType, context: 'knowledge-base', customKey, preserveKey: true, - metadata: kbOwnershipMetadata(kbOwner, storedFileName), + metadata: kbOwnershipMetadata(kbOwner, artifact.fileName), }) const fileUrl = `${getInternalApiBaseUrl()}${fileInfo.path}?context=knowledge-base` @@ -1790,7 +1814,13 @@ async function updateDocument( filename: extDoc.title, fileUrl, storageKey: fileInfo.key, - fileSize: contentBuffer.length, + fileSize: artifact.bytes.length, + /** + * Re-stated on every update: a document first stored as connector-extracted + * text and later re-synced as its source file has to stop declaring + * `text/plain`, or the pipeline's OCR routing never sees it as a PDF. + */ + mimeType: artifact.mimeType, contentHash: extDoc.contentHash, sourceUrl: extDoc.sourceUrl ?? null, ...tagValues, @@ -1849,9 +1879,9 @@ async function updateDocument( return { documentId: existingDocId, - filename: storedFileName, + filename: artifact.fileName, fileUrl, - fileSize: contentBuffer.length, - mimeType: 'text/plain', + fileSize: artifact.bytes.length, + mimeType: artifact.mimeType, } } diff --git a/apps/sim/lib/knowledge/documents/document-processor.ts b/apps/sim/lib/knowledge/documents/document-processor.ts index baa36447948..b68b5b74709 100644 --- a/apps/sim/lib/knowledge/documents/document-processor.ts +++ b/apps/sim/lib/knowledge/documents/document-processor.ts @@ -29,7 +29,7 @@ import { } from '@/lib/knowledge/model-input-provenance' import { StorageService } from '@/lib/uploads' import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key' -import { isInternalFileUrl } from '@/lib/uploads/utils/file-utils' +import { getFileExtension, isInternalFileUrl } from '@/lib/uploads/utils/file-utils' import { downloadFileFromUrl } from '@/lib/uploads/utils/file-utils.server' import { MAX_FILE_SIZE } from '@/lib/uploads/utils/validation' import { mistralParserTool } from '@/tools/mistral/parser' @@ -56,6 +56,13 @@ type OCRPage = { markdown?: string } +/** Legacy binary formats and the modern container that replaces them. */ +const LEGACY_FORMAT_REPLACEMENTS: Record = { + doc: 'DOCX', + ppt: 'PPTX', + xls: 'XLSX', +} + const MISTRAL_MAX_PAGES = 1000 async function getPdfPageCount(buffer: Buffer): Promise { @@ -782,6 +789,23 @@ async function processMistralOCRInBatches( } } +/** + * Why a document could not be read, phrased for whoever has to act on it. + * + * The `doc` and `ppt` parsers never throw: on a legacy OLE binary or a deck with + * no text they return a placeholder sentence or scraped archive bytes, which an + * interactive upload can show a user but an automated sync must never embed. They + * report that as `degraded`, and it is treated here exactly like empty output. + * Legacy formats get the concrete remedy, since re-saving genuinely fixes them — + * the modern container is one the bundled parsers read. + */ +function unreadableDocumentMessage(filename: string): string { + const modernFormat = LEGACY_FORMAT_REPLACEMENTS[getFileExtension(filename)] + return modernFormat + ? `No text could be extracted from this file. Re-save it as ${modernFormat} to index it.` + : 'No text could be extracted from this file — it may be scanned, image-only, or password-protected.' +} + async function parseWithFileParser( fileUrl: string, filename: string, @@ -807,8 +831,8 @@ async function parseWithFileParser( ) } - if (!content.trim()) { - throw new Error('File parser returned empty content') + if (metadata.degraded || !content.trim()) { + throw new Error(unreadableDocumentMessage(filename)) } return { content, processingMethod: 'file-parser' as const, cloudUrl: undefined, metadata } diff --git a/apps/sim/lib/knowledge/documents/unreadable-document.test.ts b/apps/sim/lib/knowledge/documents/unreadable-document.test.ts new file mode 100644 index 00000000000..456e9c89461 --- /dev/null +++ b/apps/sim/lib/knowledge/documents/unreadable-document.test.ts @@ -0,0 +1,65 @@ +/** + * @vitest-environment node + * + * Connectors now hand their source files to this pipeline instead of extracting + * text themselves, so the guard against fabricated content has to live here. + * `DocParser` and `PptxParser` never throw by design: on a legacy OLE binary or a + * deck with no text they return a placeholder sentence or scraped archive bytes, + * reporting it as `degraded`. Indexing that would embed junk, so it must fail the + * document exactly as empty output does. + */ +import { describe, expect, it, vi } from 'vitest' + +const { mockParseBuffer, mockDownload } = vi.hoisted(() => ({ + mockParseBuffer: vi.fn(), + mockDownload: vi.fn(), +})) + +vi.mock('@/lib/file-parsers', () => ({ parseBuffer: mockParseBuffer })) +vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ downloadFileFromUrl: mockDownload })) + +import { processDocument } from '@/lib/knowledge/documents/document-processor' + +const CONNECTOR_PDF_URL = '/api/files/serve/s3/kb%2F1-abc-Report.pdf?context=knowledge-base' + +function parse(filename: string, mimeType = 'text/plain') { + mockDownload.mockResolvedValue(Buffer.from('bytes')) + return processDocument(CONNECTOR_PDF_URL, filename, mimeType) +} + +describe('unreadable document handling', () => { + it('fails a degraded extraction instead of indexing placeholder text', async () => { + mockParseBuffer.mockResolvedValue({ + content: 'Unable to extract text from PowerPoint file.', + metadata: { extractionMethod: 'fallback', degraded: true }, + }) + + await expect(parse('Deck.pptx')).rejects.toThrow(/No text could be extracted/) + }) + + it('names the modern container for a legacy format, which re-saving genuinely fixes', async () => { + mockParseBuffer.mockResolvedValue({ + content: 'Unable to extract text from DOC file.', + metadata: { degraded: true }, + }) + + await expect(parse('Contract.doc')).rejects.toThrow(/Re-save it as DOCX/) + }) + + it('explains the likely cause for a modern format', async () => { + mockParseBuffer.mockResolvedValue({ content: ' ', metadata: {} }) + + await expect(parse('Scan.pdf')).rejects.toThrow(/scanned, image-only, or password-protected/) + }) + + it('accepts a real extraction', async () => { + mockParseBuffer.mockResolvedValue({ + content: 'Approved vendor list', + metadata: { extractionMethod: 'mammoth' }, + }) + + const result = await parse('SOP.docx') + + expect(result.chunks.length).toBeGreaterThan(0) + }) +}) From 22b441e4656218133dff02d1dac78e3967bec00a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 18 Aug 2026 14:25:14 -0700 Subject: [PATCH 2/4] fix(knowledge): guard every parser against empty output, not just the file parsers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving connector parsing into the pipeline exposed a gap on the OCR branch. OCR reads a scanned page with no recoverable text as empty, and the empty-content guard lived inside the file-parser path, so such a document chunked to nothing and reported success — the same silently-complete-but-useless outcome the guard exists to prevent. The check now sits above the parser choice and covers OCR too. Also preserves a source file's extension when its name is too long for a storage key. The extension is what picks the parser; a truncated name would still parse correctly by falling back to the display name, but only by luck. --- .../lib/knowledge/connectors/sync-engine.ts | 24 ++++++++++++++++++- .../knowledge/documents/document-processor.ts | 14 +++++++---- .../documents/unreadable-document.test.ts | 13 ++++++++++ 3 files changed, 46 insertions(+), 5 deletions(-) diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index 160871be58e..16adaf8b204 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -65,6 +65,28 @@ function sanitizeStorageTitle(title: string): string { return title.replace(/[^a-zA-Z0-9.-]/g, '_').slice(0, MAX_SAFE_TITLE_LENGTH) } +/** + * Sanitizes a source file's name for a storage key, keeping its extension. + * + * `sanitizeStorageTitle` truncates a long title outright, which for a source file + * would cut the extension off the end — and the extension is what + * `resolveStoredArtifactExtension` reads to pick a parser. Such a document would + * still parse correctly by falling back to its display name, but only by luck; + * preserving the suffix keeps the storage key authoritative for every file rather + * than for most of them. + */ +function sanitizeStorageFileName(fileName: string): string { + const dotIndex = fileName.lastIndexOf('.') + if (dotIndex <= 0) return sanitizeStorageTitle(fileName) + + const extension = sanitizeStorageTitle(fileName.slice(dotIndex)) + const base = sanitizeStorageTitle(fileName.slice(0, dotIndex)).slice( + 0, + Math.max(1, MAX_SAFE_TITLE_LENGTH - extension.length) + ) + return base + extension +} + /** * The bytes to store for a connector document, together with the name and type * that describe them. @@ -85,7 +107,7 @@ function connectorStoredArtifact(extDoc: ExternalDocument): { if (extDoc.sourceFile) { return { bytes: extDoc.sourceFile.bytes, - fileName: sanitizeStorageTitle(extDoc.sourceFile.fileName), + fileName: sanitizeStorageFileName(extDoc.sourceFile.fileName), mimeType: extDoc.sourceFile.mimeType, } } diff --git a/apps/sim/lib/knowledge/documents/document-processor.ts b/apps/sim/lib/knowledge/documents/document-processor.ts index b68b5b74709..542b84af54c 100644 --- a/apps/sim/lib/knowledge/documents/document-processor.ts +++ b/apps/sim/lib/knowledge/documents/document-processor.ts @@ -206,6 +206,16 @@ export async function processDocument( const { content, processingMethod } = parseResult const cloudUrl = 'cloudUrl' in parseResult ? parseResult.cloudUrl : undefined + /** + * Guards every parser, not just the file parsers: OCR reads a scanned page + * that has no recoverable text as empty, and chunking empty content yields a + * document that reports success while holding nothing. Failing here keeps it + * visible with a reason instead. + */ + if (parseResult.metadata?.degraded || !content.trim()) { + throw new Error(unreadableDocumentMessage(filename)) + } + let chunks: Chunk[] const metadata: FileParseMetadata = parseResult.metadata ?? {} @@ -831,10 +841,6 @@ async function parseWithFileParser( ) } - if (metadata.degraded || !content.trim()) { - throw new Error(unreadableDocumentMessage(filename)) - } - return { content, processingMethod: 'file-parser' as const, cloudUrl: undefined, metadata } } catch (error) { logger.error('File parser failed', { errorType: toError(error).name }) diff --git a/apps/sim/lib/knowledge/documents/unreadable-document.test.ts b/apps/sim/lib/knowledge/documents/unreadable-document.test.ts index 456e9c89461..accc3aa2c26 100644 --- a/apps/sim/lib/knowledge/documents/unreadable-document.test.ts +++ b/apps/sim/lib/knowledge/documents/unreadable-document.test.ts @@ -52,6 +52,19 @@ describe('unreadable document handling', () => { await expect(parse('Scan.pdf')).rejects.toThrow(/scanned, image-only, or password-protected/) }) + /** + * OCR reads a scanned page with no recoverable text as empty. Chunking that + * yields a document reporting success while holding nothing — the same silent + * failure the file-parser guard exists to prevent, so it has to cover OCR too. + */ + it('fails an OCR result that came back empty', async () => { + mockParseBuffer.mockResolvedValue({ content: '', metadata: {} }) + + await expect(parse('Scanned.pdf', 'application/pdf')).rejects.toThrow( + /No text could be extracted/ + ) + }) + it('accepts a real extraction', async () => { mockParseBuffer.mockResolvedValue({ content: 'Approved vendor list', From bdb9979ee532951690c83aca46df515107cf5b00 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 18 Aug 2026 14:45:43 -0700 Subject: [PATCH 3/4] fix(knowledge): validate the stored artifact against the parser registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten of the formats a connector now hands over — docm, dotx, xlsm, xlsb, xltx, pptm, potx, odt, ods and odp — parse fine but are deliberately not offered as upload types. `resolveStoredArtifactExtension` gated on the upload allowlist, so it rejected every one of them and processing failed with `Unsupported file type`. They worked before only because the connector extracted them itself and stored the result as text. The question the gate is asking is whether a parser can read the stored object, which the parser registry answers; the upload allowlist answers a different question about what we accept from a user. Also matches the sibling comment style in the object literal it sits in, and teaches two test mocks the newly imported symbol. --- apps/sim/lib/knowledge/connectors/sync-engine.ts | 8 +++----- .../document-processor-secret-provenance.test.ts | 1 + .../lib/knowledge/documents/parser-extension.ts | 9 ++++++++- .../documents/stored-artifact-extension.test.ts | 15 +++++++++++++++ .../documents/unreadable-document.test.ts | 5 ++++- 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index 16adaf8b204..78f3c4d0cf6 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -1837,11 +1837,9 @@ async function updateDocument( fileUrl, storageKey: fileInfo.key, fileSize: artifact.bytes.length, - /** - * Re-stated on every update: a document first stored as connector-extracted - * text and later re-synced as its source file has to stop declaring - * `text/plain`, or the pipeline's OCR routing never sees it as a PDF. - */ + // Re-stated on every update: a document first stored as connector-extracted + // text and later re-synced as its source file has to stop declaring + // `text/plain`, or the pipeline's OCR routing never sees it as a PDF. mimeType: artifact.mimeType, contentHash: extDoc.contentHash, sourceUrl: extDoc.sourceUrl ?? null, diff --git a/apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts b/apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts index 345e01f3002..75751211211 100644 --- a/apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts +++ b/apps/sim/lib/knowledge/documents/document-processor-secret-provenance.test.ts @@ -26,6 +26,7 @@ vi.mock('@/lib/core/utils/urls', async (importOriginal) => ({ vi.mock('@/lib/file-parsers', () => ({ parseBuffer: mockParseBuffer, + isSupportedFileType: (extension: string) => ['pdf', 'docx', 'txt', 'csv'].includes(extension), })) vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ diff --git a/apps/sim/lib/knowledge/documents/parser-extension.ts b/apps/sim/lib/knowledge/documents/parser-extension.ts index 974b6235b1b..2b7dbc6b1d6 100644 --- a/apps/sim/lib/knowledge/documents/parser-extension.ts +++ b/apps/sim/lib/knowledge/documents/parser-extension.ts @@ -1,3 +1,4 @@ +import { isSupportedFileType } from '@/lib/file-parsers' import { extractStorageKey, getExtensionFromMimeType, @@ -58,6 +59,12 @@ export function resolveParserExtension( * keys on its original name (`kb/-Report.pdf`) and a connector document keys * on what it stored (`kb/-Report.pdf.txt`). * + * Validated against the parser registry rather than the upload allowlist, because + * the question here is whether a parser can read the stored object — not whether + * we would accept it as an upload. The two sets differ: macro-enabled, template + * and OpenDocument formats all parse, but are deliberately not offered as upload + * types, and a connector delivers exactly those. + * * Falls back to `undefined` — leaving the caller on the filename/MIME path — * rather than guessing, so this can only ever redirect to a parser that exists. */ @@ -67,5 +74,5 @@ export function resolveStoredArtifactExtension(fileUrl: string): string | undefi const extension = getFileExtension(extractStorageKey(fileUrl)) if (!isAlphanumericExtension(extension)) return undefined - return isSupportedExtension(extension) ? extension : undefined + return isSupportedFileType(extension) ? extension : undefined } diff --git a/apps/sim/lib/knowledge/documents/stored-artifact-extension.test.ts b/apps/sim/lib/knowledge/documents/stored-artifact-extension.test.ts index 25760202244..b30446cd547 100644 --- a/apps/sim/lib/knowledge/documents/stored-artifact-extension.test.ts +++ b/apps/sim/lib/knowledge/documents/stored-artifact-extension.test.ts @@ -67,6 +67,21 @@ describe('resolveStoredArtifactExtension', () => { ).toBeUndefined() }) + /** + * The question is whether a parser can read the object, which the parser + * registry answers — not whether we would accept it as an upload. The two lists + * differ: macro-enabled, template and OpenDocument formats all parse but are not + * in the upload allowlist, and gating on that list rejected every one of them. + */ + it.each(['docm', 'dotx', 'xlsm', 'xlsb', 'xltx', 'pptm', 'potx', 'odt', 'ods', 'odp'])( + 'resolves %s, which parses but is not an accepted upload type', + (extension) => { + expect(resolveStoredArtifactExtension(`/api/files/serve/s3/kb%2F1-a-Book.${extension}`)).toBe( + extension + ) + } + ) + it('is case-insensitive', () => { expect(resolveStoredArtifactExtension('/api/files/serve/s3/kb%2F1-a-Report.PDF')).toBe('pdf') }) diff --git a/apps/sim/lib/knowledge/documents/unreadable-document.test.ts b/apps/sim/lib/knowledge/documents/unreadable-document.test.ts index accc3aa2c26..84b880902fb 100644 --- a/apps/sim/lib/knowledge/documents/unreadable-document.test.ts +++ b/apps/sim/lib/knowledge/documents/unreadable-document.test.ts @@ -15,7 +15,10 @@ const { mockParseBuffer, mockDownload } = vi.hoisted(() => ({ mockDownload: vi.fn(), })) -vi.mock('@/lib/file-parsers', () => ({ parseBuffer: mockParseBuffer })) +vi.mock('@/lib/file-parsers', () => ({ + parseBuffer: mockParseBuffer, + isSupportedFileType: (extension: string) => ['pdf', 'docx', 'pptx', 'doc'].includes(extension), +})) vi.mock('@/lib/uploads/utils/file-utils.server', () => ({ downloadFileFromUrl: mockDownload })) import { processDocument } from '@/lib/knowledge/documents/document-processor' From d3dfab063e5a47306744efc9cec21b57a24b1f64 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Tue, 18 Aug 2026 14:59:13 -0700 Subject: [PATCH 4/4] fix(knowledge): carry the MIME type through hydration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A listing stub is built before the file is fetched and declares `text/plain` for everything, so a hydrated PDF kept claiming plain text at the top level. Nothing broke today only because storage reads `sourceFile.mimeType` — which is exactly what makes it a trap: anything later reaching for `extDoc.mimeType`, the obvious field, silently loses the OCR routing this change exists to restore. The merge is now `mergeHydratedDocument` rather than an inline spread, so what hydration must carry is a stated contract with a test behind it instead of a literal that is easy to under-specify — which is how the field was missed. --- .../knowledge/connectors/sync-engine.test.ts | 75 +++++++++++++++++++ .../lib/knowledge/connectors/sync-engine.ts | 45 +++++++---- 2 files changed, 107 insertions(+), 13 deletions(-) diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts index 2036c417664..9750b49a2db 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts @@ -7,8 +7,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { classifySuspectListing, evaluateListingSafety, + mergeHydratedDocument, type PreviousListingObservation, } from '@/lib/knowledge/connectors/sync-engine' +import type { ExternalDocument } from '@/connectors/types' vi.mock('drizzle-orm', () => ({ and: vi.fn(), @@ -627,3 +629,76 @@ describe('evaluateListingSafety', () => { }) }) }) + +describe('mergeHydratedDocument', () => { + const stub = (): ExternalDocument => ({ + externalId: 'file-1', + title: 'Report.pdf', + content: '', + mimeType: 'text/plain', + contentHash: 'sharepoint:file-1:v1', + contentDeferred: true, + metadata: { fileSize: 2_400_000 }, + }) + + /** + * A stub is built during listing, before the file is fetched, so it declares + * `text/plain` for everything. Leaving that behind makes a hydrated PDF keep + * claiming plain text — invisible while storage reads `sourceFile.mimeType`, + * and a trap for anything that reaches for the obvious field instead. + */ + it('carries the hydrated MIME type over the stub placeholder', () => { + const merged = mergeHydratedDocument( + stub(), + { + ...stub(), + content: '', + mimeType: 'application/pdf', + sourceFile: { + bytes: Buffer.from('%PDF'), + fileName: 'Report.pdf', + mimeType: 'application/pdf', + }, + }, + 'sharepoint:file-1:v2' + ) + + expect(merged.mimeType).toBe('application/pdf') + expect(merged.sourceFile?.mimeType).toBe('application/pdf') + }) + + it('carries the source file and clears the deferred flag', () => { + const merged = mergeHydratedDocument( + stub(), + { ...stub(), sourceFile: { bytes: Buffer.from('x'), fileName: 'a.pdf', mimeType: 'a/b' } }, + 'h' + ) + + expect(merged.sourceFile?.bytes.toString()).toBe('x') + expect(merged.contentDeferred).toBe(false) + expect(merged.contentHash).toBe('h') + }) + + it('keeps text-path content and merges metadata over the stub', () => { + const merged = mergeHydratedDocument( + stub(), + { ...stub(), content: 'plain notes', metadata: { createdBy: 'A' } }, + 'h' + ) + + expect(merged.content).toBe('plain notes') + expect(merged.sourceFile).toBeUndefined() + expect(merged.metadata).toEqual({ fileSize: 2_400_000, createdBy: 'A' }) + }) + + it('falls back to the stub title and sourceUrl when hydration omits them', () => { + const merged = mergeHydratedDocument( + { ...stub(), sourceUrl: 'https://example.com/a' }, + { ...stub(), title: '', content: 'x' }, + 'h' + ) + + expect(merged.title).toBe('Report.pdf') + expect(merged.sourceUrl).toBe('https://example.com/a') + }) +}) diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index 78f3c4d0cf6..14e3bb7f13a 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -172,6 +172,37 @@ export function classifyExternalDoc( return { type: 'unchanged' } } +/** + * Merges a hydrated document over the listing stub it was fetched for. + * + * Every field the connector restates on hydration has to be carried, not just the + * content. A stub is built before the file is fetched and declares `text/plain`, + * so any field left behind keeps a value that is wrong for the bytes now attached + * — which is how a hydrated PDF ends up still claiming plain text. Storage reads + * `sourceFile.mimeType`, so that particular staleness is invisible until + * something reaches for the obvious field instead. + * + * Extracted from the hydration loop so the merge is a stated contract with a test + * rather than an inline spread that is easy to under-specify. + */ +export function mergeHydratedDocument( + stub: ExternalDocument, + hydrated: ExternalDocument, + contentHash: string +): ExternalDocument { + return { + ...stub, + title: hydrated.title || stub.title, + content: hydrated.content, + sourceFile: hydrated.sourceFile, + mimeType: hydrated.mimeType, + contentHash, + contentDeferred: false, + sourceUrl: hydrated.sourceUrl ?? stub.sourceUrl, + metadata: { ...stub.metadata, ...hydrated.metadata }, + } +} + /** Whether a document carries anything to index — extracted text or the source file. */ function hasPayload(extDoc: Pick): boolean { return extDoc.sourceFile !== undefined || extDoc.content.trim().length > 0 @@ -1087,19 +1118,7 @@ export async function executeSync( result.docsUnchanged++ return null } - return { - ...op, - extDoc: { - ...op.extDoc, - title: fullDoc.title || op.extDoc.title, - content: fullDoc.content, - sourceFile: fullDoc.sourceFile, - contentHash: hydratedHash, - contentDeferred: false, - sourceUrl: fullDoc.sourceUrl ?? op.extDoc.sourceUrl, - metadata: { ...op.extDoc.metadata, ...fullDoc.metadata }, - }, - } + return { ...op, extDoc: mergeHydratedDocument(op.extDoc, fullDoc, hydratedHash) } }) )