Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 50 additions & 42 deletions apps/sim/connectors/onedrive/onedrive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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`

Expand Down Expand Up @@ -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, {
Expand All @@ -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)
}

/**
Expand Down Expand Up @@ -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)
)
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
119 changes: 118 additions & 1 deletion apps/sim/connectors/sharepoint/sharepoint.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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. */
Expand All @@ -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
Expand Down Expand Up @@ -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')]))

Expand All @@ -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(
Expand Down
Loading
Loading