Skip to content

Commit 299c5f2

Browse files
committed
fix(connectors): act on the final validation sweep
Findings from a read-only /validate-connector pass over all 59 changed connectors, verified against provider specs before acting. Silent-drop fixes (a fulfilled null from getDocument records no failure and no log, so the document vanishes): - ashby: candidate.info returning success with an unusable payload. Ashby sets contentDeferred, so this path is live. - azure-devops: an unresolvable branch, likewise live. - dropbox: 409 covers the whole LookupError union, and restricted_content and locked both mean the file still exists. Only not_found is absence. - docusign: fetchFormValues swallowed every non-OK status, baking a permanently incomplete document since the hash is metadata-only. typeform: 'all' sent response_type=started,partial,completed, but Typeform documents only partial and completed. An unknown enum member risks a 400 that fails the whole sync, and staging omitted the parameter entirely, so this shipped as a regression. Now requests the widest documented set. github: removes a utf-8 blob branch justified by a misattributed quote — that sentence describes the encoding REQUEST parameter of Create a blob; the GET response is documented as always base64. Also corrects two comments that hid a real drop: >1 MB files under vnd.github+json 403 rather than returning encoding: none. hubspot: routes HTML detection through a shared anchored helper. The loose pattern matched angle-bracketed prose such as an email address, and htmlToPlainText deletes the span and collapses line structure. This matters now because the hubspot:v2: bump rewrites every live document once. youtube: drops an invented channel-ID format quote.
1 parent aaf83d0 commit 299c5f2

12 files changed

Lines changed: 128 additions & 58 deletions

File tree

apps/sim/connectors/ashby/ashby.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -605,12 +605,24 @@ export const ashbyConnector: ConnectorConfig = {
605605
externalId: string
606606
): Promise<ExternalDocument | null> => {
607607
try {
608-
if (!externalId) return null
608+
/**
609+
* These are API-shape faults, not absence: `candidate.info` answered
610+
* `success: true` with an unusable payload. Returning `null` would read as
611+
* documented absence, and on an `add` the engine's `Promise.allSettled`
612+
* hydration treats a fulfilled `null` as neither success nor failure — no
613+
* `docsFailed`, no `failedExternalIds`, no log — so the candidate would
614+
* vanish silently. Ashby sets `contentDeferred`, so this path is live.
615+
*/
616+
if (!externalId) throw new Error('Ashby getDocument called without a candidate id')
609617

610618
const infoData = await ashbyPost(accessToken, 'candidate.info', { id: externalId })
611-
if (!infoData.results) return null
619+
if (!infoData.results) {
620+
throw new Error(`Ashby candidate.info returned no results for candidate ${externalId}`)
621+
}
612622
const candidate = mapCandidate(infoData.results)
613-
if (!candidate.id) return null
623+
if (!candidate.id) {
624+
throw new Error(`Ashby candidate.info returned a candidate with no id for ${externalId}`)
625+
}
614626

615627
const notes = await fetchAllNotes(accessToken, candidate.id)
616628

apps/sim/connectors/azure-devops/azure-devops.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,9 +1079,15 @@ async function getFileDocument(
10791079
branchOverride,
10801080
syncContext
10811081
)
1082+
/**
1083+
* A branch that will not resolve is a lookup failure, not an absent file — the
1084+
* file only reached hydration because the listing already resolved its repo.
1085+
* Returning `null` reads as documented absence, and on an `add` the engine's
1086+
* `Promise.allSettled` hydration counts a fulfilled `null` as neither success
1087+
* nor failure, so the file would vanish with no `docsFailed` and no log.
1088+
*/
10821089
if (!branch) {
1083-
logger.warn('Cannot resolve branch for Azure DevOps file', { externalId })
1084-
return null
1090+
throw new Error(`Cannot resolve branch for Azure DevOps file ${externalId}`)
10851091
}
10861092

10871093
const metadataParams = new URLSearchParams({

apps/sim/connectors/docusign/docusign.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,17 @@ async function fetchFormValues(
384384
},
385385
}
386386
)
387-
if (!response.ok) return []
387+
/**
388+
* Only a 404 means the envelope carries no form data. Swallowing every other
389+
* status would bake a permanently incomplete document: `buildContentHash` is
390+
* metadata-only, so the next sync computes the identical hash, classifies the
391+
* document `unchanged`, and the missing form-data section is never recovered
392+
* until the envelope's status changes again.
393+
*/
394+
if (response.status === 404) return []
395+
if (!response.ok) {
396+
throw new Error(`Failed to fetch DocuSign form data: ${response.status}`)
397+
}
388398
const data = (await response.json()) as DocuSignFormData
389399
const values: DocuSignFormValue[] = []
390400
if (Array.isArray(data.formData)) values.push(...data.formData)

apps/sim/connectors/dropbox/dropbox.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -289,13 +289,22 @@ export const dropboxConnector: ConnectorConfig = {
289289
})
290290

291291
/**
292-
* Dropbox reports endpoint-specific errors — including `path/not_found` — as 409,
293-
* the only status that means the file is genuinely gone. Every other failure
294-
* (429, 5xx, network faults) propagates so the sync engine records a failed row
295-
* and keeps the already-indexed file out of deletion reconciliation.
292+
* Dropbox reports every endpoint-specific error as 409, so the status alone
293+
* does not mean the file is gone. For `get_metadata` the error union is
294+
* `path: LookupError`, whose variants include `restricted_content` and
295+
* `locked` — the file still exists in both. Only `not_found` is absence, and
296+
* only that returns `null`; anything else propagates so the sync engine
297+
* records a failed row instead of silently dropping the document.
296298
*/
297299
if (!response.ok) {
298-
if (response.status === 409) return null
300+
if (response.status === 409) {
301+
const body = (await response.json().catch(() => null)) as {
302+
error?: { '.tag'?: string; path?: { '.tag'?: string } }
303+
} | null
304+
if (body?.error?.['.tag'] === 'path' && body.error.path?.['.tag'] === 'not_found') {
305+
return null
306+
}
307+
}
299308
throw new Error(`Failed to get metadata: ${response.status}`)
300309
}
301310

apps/sim/connectors/github/github.ts

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,14 @@ async function fetchBlobContent(
176176
return buf.toString('utf8')
177177
}
178178
/**
179-
* https://docs.github.com/en/rest/git/blobs documents two encodings —
180-
* "Currently, 'utf-8' and 'base64' are supported" — so a utf-8 body is already
181-
* plain text and is used as written. Any other encoding would silently persist
182-
* empty content, so it throws and surfaces as a failed document instead.
179+
* `GET /repos/{owner}/{repo}/git/blobs/{sha}` documents a single response
180+
* encoding: "The `content` in the response will always be Base64 encoded."
181+
* The "Currently, `utf-8` and `base64` are supported" sentence belongs to the
182+
* `encoding` REQUEST parameter of `POST .../git/blobs` (Create a blob) and does
183+
* not describe this response, so no `utf-8` branch is warranted here. Any other
184+
* encoding would silently persist empty content, so it throws and surfaces as a
185+
* failed document instead.
183186
*/
184-
if (encoding === 'utf-8') return content
185187
throw new Error(`Unexpected git blob encoding for ${sha}: ${encoding ?? 'undefined'}`)
186188
}
187189

@@ -334,8 +336,14 @@ export const githubConnector: ConnectorConfig = {
334336
/**
335337
* A rate-limit 403 never reaches here: `fetchWithRetry` treats a 403 carrying
336338
* `retry-after` or `x-ratelimit-remaining: 0` as retryable and throws once the
337-
* retries are spent, so it lands in the catch below as a failure. A 403 that
338-
* survives to this point is a genuine authorization denial.
339+
* retries are spent, so it lands in the catch below as a failure.
340+
*
341+
* A 403 that survives is usually an authorization denial, but NOT always: this
342+
* request sends `application/vnd.github+json`, and the Contents API documents
343+
* that files between 1-100 MB support "only the `raw` or `object` custom media
344+
* types". A >1 MB text file therefore also lands here and is dropped, which on
345+
* an `add` is silent (a fulfilled `null` records no failure). Reconciliation is
346+
* unaffected — the file is already in `seenExternalIds` from the listing.
339347
*/
340348
if (response.status === 403) {
341349
logger.info('Skipping GitHub file rejected by Contents API', {
@@ -381,10 +389,16 @@ export const githubConnector: ConnectorConfig = {
381389
} else if (encoding === 'none' && data.sha && size > 0) {
382390
/**
383391
* Per https://docs.github.com/en/rest/repos/contents, for files of 1-100 MB
384-
* "the content field will be an empty string and the encoding field will be
385-
* `none`". The Contents page points at the raw media type; the Git Blobs API
386-
* is used instead because it returns the same blob as JSON and is documented
387-
* to support blobs up to 100 MB.
392+
* "only the `raw` or `object` custom media types are supported", and it is
393+
* specifically "when using the `object` media type" that "the `content` field
394+
* will be an empty string and the `encoding` field will be `none`".
395+
*
396+
* This request sends `application/vnd.github+json`, so that precondition does
397+
* not hold and this branch is currently unreachable — such files 403 above
398+
* instead. Reaching it would require requesting
399+
* `application/vnd.github.object+json`. The fallback itself is correct: the Git
400+
* Blobs API returns the same blob as JSON and is documented to support blobs up
401+
* to 100 MB.
388402
*/
389403
const blobContent = await fetchBlobContent(accessToken, owner, repo, data.sha as string)
390404
if (blobContent === null) {

apps/sim/connectors/granola/granola.ts

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { getErrorMessage, toError } from '@sim/utils/errors'
33
import { fetchWithRetry, VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
44
import { granolaConnectorMeta } from '@/connectors/granola/meta'
55
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
6-
import { htmlToPlainText, joinTagArray, parseTagDate } from '@/connectors/utils'
6+
import { htmlToPlainText, joinTagArray, looksLikeHtml, parseTagDate } from '@/connectors/utils'
77

88
const logger = createLogger('GranolaConnector')
99

@@ -138,24 +138,6 @@ function parseDateFilter(sourceConfig: Record<string, unknown>, key: string): st
138138
return Number.isNaN(parsed.getTime()) ? undefined : parsed.toISOString()
139139
}
140140

141-
/**
142-
* Matches a real HTML tag, anchored to a known tag name rather than any
143-
* `<word…>` run. Markdown autolinks (`<https://acme.com>`, `<jane@acme.com>`)
144-
* and angle-bracketed prose (`<redacted>`) are common in meeting summaries, and
145-
* a looser pattern would route the whole markdown document through
146-
* `htmlToPlainText`, which collapses every newline and destroys its structure.
147-
*/
148-
const HTML_TAG_PATTERN =
149-
/<\/?(?:p|div|br|hr|ul|ol|li|h[1-6]|table|thead|tbody|tr|td|th|span|strong|em|b|i|u|a|code|pre|blockquote|img|figure)\b[^>]*>/i
150-
151-
/**
152-
* Detects HTML markup in a summary. Granola documents `summary_markdown` as
153-
* markdown, so this only guards against the API ever emitting HTML instead.
154-
*/
155-
function looksLikeHtml(value: string): boolean {
156-
return HTML_TAG_PATTERN.test(value)
157-
}
158-
159141
/**
160142
* Assembles the document content from a note's title and summary. Prefers the
161143
* markdown summary, falling back to plain-text summary. HTML is stripped only

apps/sim/connectors/hubspot/hubspot.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { getErrorMessage } from '@sim/utils/errors'
33
import { fetchWithRetry, VALIDATE_RETRY_OPTIONS } from '@/lib/knowledge/documents/utils'
44
import { hubspotConnectorMeta } from '@/connectors/hubspot/meta'
55
import type { ConnectorConfig, ExternalDocument, ExternalDocumentList } from '@/connectors/types'
6-
import { htmlToPlainText, parseTagDate } from '@/connectors/utils'
6+
import { htmlToPlainText, looksLikeHtml, parseTagDate } from '@/connectors/utils'
77

88
const logger = createLogger('HubSpotConnector')
99

@@ -177,9 +177,14 @@ function buildRecordTitle(objectType: string, properties: Record<string, string
177177
/**
178178
* HubSpot rich-text properties (ticket `content`, company `description`, custom
179179
* rich-text fields) come back as raw HTML, so they are stripped before indexing.
180+
*
181+
* The detection is deliberately the shared anchored one: CRM free text regularly
182+
* carries angle brackets that are not markup (`Reply from John <john@acme.com>`),
183+
* and a false positive here does not pass the value through — `htmlToPlainText`
184+
* would delete the bracketed span and collapse the value's line structure.
180185
*/
181186
function toPlainTextValue(value: string): string {
182-
return /<[a-z!/][^>]*>/i.test(value) ? htmlToPlainText(value) : value
187+
return looksLikeHtml(value) ? htmlToPlainText(value) : value
183188
}
184189

185190
/**

apps/sim/connectors/typeform/meta.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ export const typeformConnectorMeta: ConnectorMeta = {
3939
options: [
4040
{ label: 'Completed only', id: 'completed' },
4141
{ label: 'Partial & completed', id: 'partial' },
42-
{ label: 'All (started, partial & completed)', id: 'all' },
42+
{ label: 'All available (partial & completed)', id: 'all' },
4343
],
4444
description: 'Which responses to sync by completion status. Defaults to completed only.',
4545
},

apps/sim/connectors/typeform/typeform.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,17 @@ describe('typeform listDocuments', () => {
5555
expect(requestUrl(1).searchParams.get('response_type')).toBe('completed')
5656
})
5757

58-
it('requests all three response types for the "all" choice', async () => {
58+
/**
59+
* Typeform documents only `partial` and `completed` as `response_type` members,
60+
* so `all` must not send an undocumented `started`: an unknown enum member risks
61+
* a 400 that fails the entire sync.
62+
*/
63+
it('requests only the documented response types for the "all" choice', async () => {
5964
mockFormThenResponses({ items: [] })
6065

6166
await typeformConnector.listDocuments(ACCESS_TOKEN, { ...FORM_CONFIG, responseType: 'all' })
6267

63-
expect(requestUrl(1).searchParams.get('response_type')).toBe('started,partial,completed')
68+
expect(requestUrl(1).searchParams.get('response_type')).toBe('partial,completed')
6469
})
6570

6671
it('derives an incremental since filter at the second precision the API documents', async () => {

apps/sim/connectors/typeform/typeform.ts

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -124,16 +124,22 @@ function getResponseTypeChoice(sourceConfig: Record<string, unknown>): ResponseT
124124
/**
125125
* Appends the `response_type` filter for a given choice. Omitting the parameter
126126
* would fall back to the API default of `completed` only, so every choice is sent
127-
* explicitly. `partial` requests both partial and completed so partially-answered
128-
* submissions are included alongside finished ones; `all` additionally requests
129-
* `started`, the third response type (the documented `sort` default names all
130-
* three: `submitted_at` for completed, `staged_at` for partial, `landed_at` for
131-
* started).
127+
* explicitly.
128+
*
129+
* Typeform documents exactly two members: "It is expected to be passed as a comma
130+
* separated list of values, e.g. `response_type=partial,completed`", defaulting to
131+
* `completed`. There is no documented `started` member — the `sort` docs imply a
132+
* started *state* exists (ordering falls back to `landed_at`), but that does not
133+
* make it a valid filter value, and sending an undocumented member risks a 400
134+
* that fails the entire sync. `all` therefore requests the widest documented set,
135+
* which is the same as `partial`.
132136
*/
133137
function appendResponseType(params: URLSearchParams, choice: ResponseTypeChoice): void {
134-
if (choice === 'partial') params.append('response_type', 'partial,completed')
135-
else if (choice === 'all') params.append('response_type', 'started,partial,completed')
136-
else params.append('response_type', 'completed')
138+
if (choice === 'partial' || choice === 'all') {
139+
params.append('response_type', 'partial,completed')
140+
} else {
141+
params.append('response_type', 'completed')
142+
}
137143
}
138144

139145
/**

0 commit comments

Comments
 (0)