fix(knowledge): parse the stored artifact, not the document's display name

A connector document's `filename` is a display name that deliberately disagrees
with the bytes on disk: the sync engine records the source file's name
(`Report.pdf`) while storing the text the connector already extracted from it
under a `.txt` storage key, with `mimeType: 'text/plain'`.

`processDocumentAsync` discards the `.txt` processing filename the sync engine
computes and rebuilds its input from the document row, so `processDocument`
chose a parser from the display name and re-parsed extracted text as the source
binary. In production this failed 1,379 SharePoint PDFs with `Invalid PDF
structure.` and silently double-wrapped 364 spreadsheets — those reported
`completed`, with `=== Sheet: Sheet1 ===` wrapped around the connector's own
`=== Sheet: <real name> ===`, because SheetJS accepts almost any input.

Parser selection now prefers the extension of the object actually fetched,
falling back to the filename/MIME path when the URL is not ours or the key
carries no extension a parser claims. Both ingestion paths are honest under this
rule because `fitStorageKeyName` preserves extensions through truncation: an
upload keys on its original name, a connector document keys on what it stored.
This also covers the stuck-document retry sweep, which builds its own input from
the same display name.

The defect predates the connectors that expose it — Box fetches Box-side text
representations for `pdf`/`docx`/`xlsx` and stores them under the source name
too, so it was latent there before SharePoint and OneDrive reached binary
formats.

Raises the connector sync ceiling from 30 to 60 minutes; a 2,600-document
library exhausted the old budget mid-listing and was killed, leaving its
`syncing` lock set until the scheduler reclaimed it. The ceiling and the
stale-lock TTL now derive from one another in `sync-limits.ts`, because
reclaiming a lock frees it for a second sync: a TTL at or below the run ceiling
would hand the lock to a successor while the first sync is still writing. A test
pins that invariant so the next raise cannot silently break it.

Author: waleedlatif1

apps/sim/app/api/knowledge/connectors/sync/route.ts

@@ -9,6 +9,7 @@ import { mapWithConcurrency } from '@/lib/core/utils/concurrency'
 import { generateRequestId } from '@/lib/core/utils/request'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
 import { dispatchSync } from '@/lib/knowledge/connectors/queue'
+import { CONNECTOR_SYNC_STALE_LOCK_TTL_MS } from '@/lib/knowledge/connectors/sync-limits'
 
 export const dynamic = 'force-dynamic'
 
@@ -39,8 +40,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
   try {
     const now = new Date()
 
-    const STALE_SYNC_TTL_MS = 120 * 60 * 1000
-    const staleCutoff = new Date(now.getTime() - STALE_SYNC_TTL_MS)
+    const staleCutoff = new Date(now.getTime() - CONNECTOR_SYNC_STALE_LOCK_TTL_MS)
 
     const recoveredConnectors = await db
       .update(knowledgeConnector)

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

@@ -5,6 +5,7 @@ import {
   type ConnectorSyncPayload,
 } from '@/lib/knowledge/connectors/queue'
 import { executeSync } from '@/lib/knowledge/connectors/sync-engine'
+import { CONNECTOR_SYNC_MAX_DURATION_SECONDS } from '@/lib/knowledge/connectors/sync-limits'
 
 const logger = createLogger('TriggerKnowledgeConnectorSync')
 
@@ -39,7 +40,7 @@ export async function executeConnectorSyncJob(payload: unknown) {
 
 export const knowledgeConnectorSync = task({
   id: 'knowledge-connector-sync',
-  maxDuration: 1800,
+  maxDuration: CONNECTOR_SYNC_MAX_DURATION_SECONDS,
   machine: 'large-2x',
   retry: {
     maxAttempts: 3,

apps/sim/lib/knowledge/connectors/sync-engine.ts

@@ -60,7 +60,16 @@ const STALE_PROCESSING_MINUTES = 45
 const RETRY_WINDOW_DAYS = 7
 const MAX_CONSECUTIVE_FAILURES = 10
 
-/** Sanitizes a document title for use in S3 storage keys. */
+/**
+ * Sanitizes a document title for use in S3 storage keys.
+ *
+ * The `.txt` suffix every caller appends to the result is load-bearing, not
+ * cosmetic: a connector stores already-extracted text, while `document.filename`
+ * keeps the source file's name for display. `resolveStoredArtifactExtension`
+ * reads the parser choice off the storage key, so a key that carried the source
+ * extension instead would re-parse extracted text as the original binary —
+ * `Invalid PDF structure.` for PDFs, a silent double-wrap for spreadsheets.
+ */
 function sanitizeStorageTitle(title: string): string {
   return title.replace(/[^a-zA-Z0-9.-]/g, '_').slice(0, MAX_SAFE_TITLE_LENGTH)
 }

apps/sim/lib/knowledge/connectors/sync-limits.test.ts

@@ -0,0 +1,33 @@
+/**
+ * @vitest-environment node
+ */
+import { describe, expect, it } from 'vitest'
+import {
+  CONNECTOR_SYNC_MAX_DURATION_SECONDS,
+  CONNECTOR_SYNC_STALE_LOCK_TTL_MS,
+} from '@/lib/knowledge/connectors/sync-limits'
+
+describe('connector sync limits', () => {
+  /**
+   * Reclaiming a stale lock flips the connector row to `error` and clears the way
+   * for another sync. If the TTL were not greater than the run ceiling, a sync
+   * still inside its own budget could have its lock reclaimed and then run
+   * concurrently with its successor, both writing the same documents.
+   */
+  it('reclaims a stale lock only well after a run could still be alive', () => {
+    expect(CONNECTOR_SYNC_STALE_LOCK_TTL_MS).toBeGreaterThan(
+      CONNECTOR_SYNC_MAX_DURATION_SECONDS * 1000
+    )
+  })
+
+  it('keeps at least a 2x margin between the run ceiling and the reclaim', () => {
+    expect(CONNECTOR_SYNC_STALE_LOCK_TTL_MS).toBeGreaterThanOrEqual(
+      CONNECTOR_SYNC_MAX_DURATION_SECONDS * 2 * 1000
+    )
+  })
+
+  /** A 2,600-document library exhausted the previous 1800s budget mid-listing. */
+  it('allows a run longer than the half hour that timed out in production', () => {
+    expect(CONNECTOR_SYNC_MAX_DURATION_SECONDS).toBeGreaterThan(1800)
+  })
+})

apps/sim/lib/knowledge/connectors/sync-limits.ts

@@ -0,0 +1,23 @@
+/**
+ * Wall-clock ceiling for a single connector sync run.
+ *
+ * A large document library routinely needs more than the half hour this used to
+ * allow: a 2,600-document SharePoint site exhausted the old 1800s budget and the
+ * run was killed mid-listing, which also left the connector's `syncing` lock set
+ * until the scheduler reclaimed it.
+ */
+export const CONNECTOR_SYNC_MAX_DURATION_SECONDS = 3600
+
+/**
+ * How long a connector may sit in `syncing` before the scheduler reclaims its lock.
+ *
+ * This MUST stay above {@link CONNECTOR_SYNC_MAX_DURATION_SECONDS}. Reclaiming
+ * flips the connector row to `error` and clears the way for another sync, so a TTL
+ * at or below the run ceiling would hand a second sync the lock while the first is
+ * still writing documents — two concurrent syncs on one connector, racing on the
+ * same `(connectorId, externalId)` rows.
+ *
+ * Deriving it from the run ceiling rather than hard-coding minutes is what keeps
+ * that invariant true the next time the ceiling is raised.
+ */
+export const CONNECTOR_SYNC_STALE_LOCK_TTL_MS = CONNECTOR_SYNC_MAX_DURATION_SECONDS * 2 * 1000

apps/sim/lib/knowledge/documents/document-processor.ts

@@ -18,7 +18,10 @@ import { env, envNumber } from '@/lib/core/config/env'
 import { OCR_CAPABILITY, requireCapability } from '@/lib/core/config/env-capabilities'
 import { parseBuffer } from '@/lib/file-parsers'
 import type { FileParseMetadata } from '@/lib/file-parsers/types'
-import { resolveParserExtension } from '@/lib/knowledge/documents/parser-extension'
+import {
+  resolveParserExtension,
+  resolveStoredArtifactExtension,
+} from '@/lib/knowledge/documents/parser-extension'
 import { retryWithExponentialBackoff } from '@/lib/knowledge/documents/utils'
 import {
   assertKnowledgeOpaqueModelInputSafe,
@@ -841,7 +844,14 @@ async function parseHttpFile(
 ): Promise<{ content: string; metadata?: FileParseMetadata }> {
   const buffer = await downloadFileWithTimeout(fileUrl, userId)
 
-  const extension = resolveParserExtension(filename, mimeType)
+  /**
+   * Prefer what the storage key says we just downloaded over what the document is
+   * *called*. The two agree for an upload; for a connector document they do not,
+   * and trusting the display name re-parses already-extracted text as the source
+   * binary. See `resolveStoredArtifactExtension`.
+   */
+  const extension =
+    resolveStoredArtifactExtension(fileUrl) ?? resolveParserExtension(filename, mimeType)
   const result = await parseBuffer(buffer, extension)
   return result
 }

apps/sim/lib/knowledge/documents/parser-extension.ts

@@ -1,4 +1,8 @@
-import { getExtensionFromMimeType } from '@/lib/uploads/utils/file-utils'
+import {
+  extractStorageKey,
+  getExtensionFromMimeType,
+  isInternalFileUrl,
+} from '@/lib/uploads/utils/file-utils'
 import {
   isAlphanumericExtension,
   isSupportedExtension,
@@ -36,3 +40,34 @@ export function resolveParserExtension(
 
   throw new Error(`Could not determine file type for ${filename || 'document'}`)
 }
+
+/**
+ * Extension of the object actually stored, taken from its storage key.
+ *
+ * A knowledge base document's `filename` is a *display* name, and for connector
+ * documents it deliberately disagrees with the bytes on disk: the sync engine
+ * records the source file's name (`Report.pdf`) while storing the text the
+ * connector already extracted from it under a `.txt` key. Choosing a parser from
+ * the display name therefore re-parses extracted text as the original binary
+ * format — `Invalid PDF structure.` for PDFs, and for spreadsheets a silent
+ * double-wrap, since SheetJS accepts almost anything.
+ *
+ * The storage key is the honest signal for both ingestion paths, because
+ * `fitStorageKeyName` preserves a file's extension through truncation: an upload
+ * keys on its original name (`kb/<id>-Report.pdf`) and a connector document keys
+ * on what it stored (`kb/<id>-Report.pdf.txt`).
+ *
+ * Returns `undefined` — leaving the caller on the filename/MIME path — for
+ * anything not served from our own storage, and for a key whose extension no
+ * parser claims, so this can only ever redirect a document to a parser that
+ * exists.
+ */
+export function resolveStoredArtifactExtension(fileUrl: string): string | undefined {
+  if (!isInternalFileUrl(fileUrl)) return undefined
+
+  const key = extractStorageKey(fileUrl)
+  const raw = key.includes('.') ? key.split('.').pop()?.toLowerCase() : undefined
+  if (!raw || !isAlphanumericExtension(raw)) return undefined
+
+  return isSupportedExtension(raw) ? raw : undefined
+}

apps/sim/lib/knowledge/documents/stored-artifact-extension.test.ts

@@ -0,0 +1,73 @@
+/**
+ * @vitest-environment node
+ *
+ * A knowledge base document's `filename` is a display name. For connector
+ * documents it deliberately disagrees with the stored bytes — the sync engine
+ * records `Report.pdf` while storing the text the connector already extracted
+ * under a `.txt` key — so choosing a parser from the display name re-parsed
+ * extracted text as the source binary. In production that failed 1,379
+ * SharePoint PDFs with `Invalid PDF structure.` and silently double-wrapped
+ * every spreadsheet, which "succeeded" because SheetJS accepts almost anything.
+ */
+import { describe, expect, it } from 'vitest'
+import { resolveStoredArtifactExtension } from '@/lib/knowledge/documents/parser-extension'
+
+const CONNECTOR_PDF_URL =
+  '/api/files/serve/s3/kb%2F1786986883507-abc-Report.pdf.txt?context=knowledge-base'
+const UPLOADED_PDF_URL =
+  '/api/files/serve/s3/kb%2F1786986883507-abc-Report.pdf?context=knowledge-base'
+
+describe('resolveStoredArtifactExtension', () => {
+  it('reports txt for a connector document whose display name is a PDF', () => {
+    expect(resolveStoredArtifactExtension(CONNECTOR_PDF_URL)).toBe('txt')
+  })
+
+  it('reports txt for a connector spreadsheet, which SheetJS would otherwise re-wrap', () => {
+    expect(
+      resolveStoredArtifactExtension(
+        '/api/files/serve/s3/kb%2F1-abc-Vendor_Spend.xlsx.txt?context=knowledge-base'
+      )
+    ).toBe('txt')
+  })
+
+  it('leaves an uploaded document on its real extension', () => {
+    expect(resolveStoredArtifactExtension(UPLOADED_PDF_URL)).toBe('pdf')
+  })
+
+  it('handles the blob and gcs storage prefixes', () => {
+    expect(resolveStoredArtifactExtension('/api/files/serve/blob/kb%2F1-a-x.docx')).toBe('docx')
+    expect(resolveStoredArtifactExtension('/api/files/serve/gcs/kb%2F1-a-x.csv')).toBe('csv')
+  })
+
+  it('ignores URLs that are not served from our own storage', () => {
+    expect(resolveStoredArtifactExtension('https://example.com/files/Report.pdf')).toBeUndefined()
+    expect(resolveStoredArtifactExtension('data:application/pdf;base64,AAAA')).toBeUndefined()
+  })
+
+  /**
+   * `fitStorageKeyName` drops the extension when it cannot fit, and a key may
+   * carry no extension at all. Returning undefined puts the caller back on the
+   * filename/MIME path rather than guessing.
+   */
+  it('returns undefined when the key carries no usable extension', () => {
+    expect(resolveStoredArtifactExtension('/api/files/serve/s3/kb%2F1-a-Report')).toBeUndefined()
+    expect(resolveStoredArtifactExtension('/api/files/serve/s3/kb%2F1-a-Report.')).toBeUndefined()
+  })
+
+  /**
+   * Only ever redirects to a parser that exists — an unknown suffix falls back
+   * instead of routing the document at a parser that cannot handle it.
+   */
+  it('returns undefined for an extension no parser claims', () => {
+    expect(
+      resolveStoredArtifactExtension('/api/files/serve/s3/kb%2F1-a-archive.zip')
+    ).toBeUndefined()
+    expect(
+      resolveStoredArtifactExtension('/api/files/serve/s3/kb%2F1-a-Report.v2.final')
+    ).toBeUndefined()
+  })
+
+  it('is case-insensitive', () => {
+    expect(resolveStoredArtifactExtension('/api/files/serve/s3/kb%2F1-a-Report.PDF')).toBe('pdf')
+  })
+})