Merge remote-tracking branch 'origin/staging' into mship-fixes

Author: icecrasher321

apps/sim/connectors/onedrive/onedrive.ts

@@ -6,7 +6,11 @@ import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/
 import {
   CONNECTOR_MAX_FILE_BYTES,
   ConnectorFileTooLargeError,
-  htmlToPlainText,
+  ConnectorTextExtractionError,
+  connectorFileExtension,
+  extractConnectorText,
+  extractionFailedSkipReason,
+  isIndexableConnectorFile,
   isSkippedDocument,
   markSkipped,
   parseTagDate,
@@ -18,23 +22,11 @@ import {
 
 const logger = createLogger('OneDriveConnector')
 
-const SUPPORTED_EXTENSIONS = new Set([
-  '.txt',
-  '.md',
-  '.html',
-  '.htm',
-  '.csv',
-  '.json',
-  '.xml',
-  '.yaml',
-  '.yml',
-  '.log',
-  '.rst',
-  '.tsv',
-])
-
 const MAX_FILE_SIZE = CONNECTOR_MAX_FILE_BYTES
 
+/** Distinct extensions named in the per-page skipped-file diagnostic. */
+const MAX_LOGGED_SKIPPED_EXTENSIONS = 10
+
 const GRAPH_API_ORIGIN = 'https://graph.microsoft.com'
 const GRAPH_BASE_URL = `${GRAPH_API_ORIGIN}/v1.0`
 
@@ -85,19 +77,9 @@ interface OneDriveListResponse {
 }
 
 /**
- * Checks whether a file has a supported text extension.
+ * Downloads the raw bytes of a OneDrive file.
  */
-function isSupportedTextFile(name: string): boolean {
-  const dotIndex = name.lastIndexOf('.')
-  if (dotIndex === -1) return false
-  const ext = name.slice(dotIndex).toLowerCase()
-  return SUPPORTED_EXTENSIONS.has(ext)
-}
-
-/**
- * Downloads the raw content of a OneDrive file.
- */
-async function downloadFileContent(accessToken: string, fileId: string): Promise<string> {
+async function downloadFileContent(accessToken: string, fileId: string): Promise<Buffer> {
   const url = `${GRAPH_BASE_URL}/me/drive/items/${encodeURIComponent(fileId)}/content`
 
   const response = await fetchWithRetry(url, {
@@ -114,25 +96,20 @@ async function downloadFileContent(accessToken: string, fileId: string): Promise
   if (!buffer) {
     throw new ConnectorFileTooLargeError(MAX_FILE_SIZE)
   }
-  return buffer.toString('utf8')
+  return buffer
 }
 
 /**
- * Fetches file content, converting HTML to plain text when applicable.
+ * 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(
   accessToken: string,
   fileId: string,
   fileName: string
 ): Promise<string> {
-  const ext = fileName.slice(fileName.lastIndexOf('.')).toLowerCase()
-  const raw = await downloadFileContent(accessToken, fileId)
-
-  if (ext === '.html' || ext === '.htm') {
-    return htmlToPlainText(raw)
-  }
-
-  return raw
+  const buffer = await downloadFileContent(accessToken, fileId)
+  return extractConnectorText(buffer, fileName)
 }
 
 /**
@@ -282,15 +259,39 @@ export const onedriveConnector: ConnectorConfig = {
       const items = data.value || []
 
       const files: OneDriveItem[] = []
+      /**
+       * Extensions this connector cannot index, tallied per page. A folder of
+       * unsupported files otherwise syncs as "success, 0 documents", which reads
+       * exactly like a wrong folder path — the failure mode this log exists for.
+       * Unsupported files are counted rather than turned into `failed` document
+       * rows, so a drive full of images does not fill the knowledge base with noise.
+       */
+      const skippedExtensions = new Map<string, number>()
+
       for (const item of items) {
         if (item.folder) {
           state.folderStack.push(item.id)
-        } else if (item.file && isSupportedTextFile(item.name)) {
-          // Keep oversized files; they are surfaced as skipped (failed) docs below.
-          files.push(item)
+        } else if (item.file) {
+          if (isIndexableConnectorFile(item.name)) {
+            // Keep oversized files; they are surfaced as skipped (failed) docs below.
+            files.push(item)
+          } else {
+            const extension = connectorFileExtension(item.name) ?? '(none)'
+            skippedExtensions.set(extension, (skippedExtensions.get(extension) ?? 0) + 1)
+          }
         }
       }
 
+      if (skippedExtensions.size > 0) {
+        let skippedCount = 0
+        for (const count of skippedExtensions.values()) skippedCount += count
+        logger.info('Skipped OneDrive files with unsupported extensions', {
+          folderId: state.currentFolder ?? 'root',
+          skippedCount,
+          extensions: Array.from(skippedExtensions.keys()).slice(0, MAX_LOGGED_SKIPPED_EXTENSIONS),
+        })
+      }
+
       const stubs = files.map((item) =>
         stubOrSkipBySize(fileToStub(item), item.size, MAX_FILE_SIZE)
       )
@@ -373,7 +374,7 @@ export const onedriveConnector: ConnectorConfig = {
 
     const item = (await response.json()) as OneDriveItem
 
-    if (!item.file || !isSupportedTextFile(item.name)) return null
+    if (!item.file || !isIndexableConnectorFile(item.name)) return null
 
     try {
       const content = await fetchFileContent(accessToken, item.id, item.name)
@@ -386,6 +387,13 @@ export const onedriveConnector: ConnectorConfig = {
         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

apps/sim/connectors/sharepoint/sharepoint.test.ts

@@ -3,12 +3,16 @@
  */
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
-const { mockFetchWithRetry } = vi.hoisted(() => ({ mockFetchWithRetry: vi.fn() }))
+const { mockFetchWithRetry, mockParseBuffer } = vi.hoisted(() => ({
+  mockFetchWithRetry: vi.fn(),
+  mockParseBuffer: 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 {
@@ -27,6 +31,8 @@ const POLICIES_DRIVE_ID = 'b!policies'
 interface GraphRoute {
   status?: number
   body?: unknown
+  /** Serve `body` as bytes, for the `/content` endpoint the downloader reads. */
+  raw?: boolean
 }
 
 /** Folder-shaped drive item for children listings. */
@@ -49,6 +55,9 @@ function mockGraph(routes: Record<string, GraphRoute>) {
       status,
       json: async () => route.body,
       text: async () => JSON.stringify(route.body ?? {}),
+      /** `readBodyWithLimit` falls back to this when there is no stream body. */
+      arrayBuffer: async () =>
+        Buffer.from(route.raw ? String(route.body ?? '') : JSON.stringify(route.body ?? {})),
     } as unknown as Response
   })
   return requested
@@ -411,6 +420,47 @@ describe('listDocuments', () => {
     expect(syncContext.listingCapped).toBeUndefined()
   })
 
+  /**
+   * The reported failure: a document library of Office SOPs synced as
+   * "success, 0 documents" because the listing filter accepted only plain text,
+   * which is indistinguishable from a wrong folder path.
+   */
+  it('lists Office documents and PDFs alongside text files', async () => {
+    mockGraph(
+      childrenRoute(DEFAULT_DRIVE_ID, null, [
+        file('f1', 'Market Data SOP.docx'),
+        file('f2', 'Vendor Contract.pdf'),
+        file('f3', 'User List.xlsx'),
+        file('f4', 'Overview.pptx'),
+        file('f5', 'notes.txt'),
+      ])
+    )
+
+    const result = await list(undefined, listContext())
+
+    expect(result.documents.map((doc) => doc.title)).toEqual([
+      'Market Data SOP.docx',
+      'Vendor Contract.pdf',
+      'User List.xlsx',
+      'Overview.pptx',
+      'notes.txt',
+    ])
+  })
+
+  it('still excludes files with no extractable text', async () => {
+    mockGraph(
+      childrenRoute(DEFAULT_DRIVE_ID, null, [
+        file('f1', 'diagram.png'),
+        file('f2', 'recording.mp4'),
+        file('f3', 'notes.txt'),
+      ])
+    )
+
+    const result = await list(undefined, listContext())
+
+    expect(result.documents.map((doc) => doc.externalId)).toEqual(['f3'])
+  })
+
   it('builds a metadata-only contentHash that getDocument can reproduce', async () => {
     mockGraph(childrenRoute(DEFAULT_DRIVE_ID, null, [file('f1', 'a.txt')]))
 
@@ -421,6 +471,73 @@ describe('listDocuments', () => {
   })
 })
 
+describe('getDocument content extraction', () => {
+  function itemRoute(itemId: string, name: string) {
+    return {
+      [`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/items/${itemId}?$select=${ITEM_SELECT}`]: {
+        body: file(itemId, name),
+      },
+    }
+  }
+
+  /** The content endpoint is fetched directly, not through the JSON `graphGet`. */
+  function contentRoute(itemId: string, body: string) {
+    return {
+      [`${GRAPH}/drives/${DEFAULT_DRIVE_ID}/items/${itemId}/content`]: { body, raw: true },
+    }
+  }
+
+  function get(externalId: string) {
+    return sharepointConnector.getDocument!(
+      'token',
+      { siteUrl: SITE_URL },
+      externalId,
+      listContext()
+    )
+  }
+
+  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' },
+    })
+
+    const doc = await get('f1')
+
+    expect(doc?.content).toBe('Approved vendor list')
+    expect(doc?.skippedReason).toBeUndefined()
+    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 },
+    })
+
+    const doc = await get('f2')
+
+    expect(doc?.content).toBe('')
+    expect(doc?.skippedReason).toContain('PPTX')
+    expect(doc?.externalId).toBe('f2')
+  })
+
+  it('reads a text file without invoking a parser', 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()
+  })
+})
+
 describe('serverRelativePathFromUrl', () => {
   it('strips the site prefix from a site-scoped URL', () => {
     expect(