From 08fe3ed76a0ad764a022d644f071465ed865e427 Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 14:00:20 -0700 Subject: [PATCH 01/22] chore(skills): make public-repo scrubbing explicit in ship and babysit (#6819) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The repo is public and both skills publish permanently. `/ship` already listed what to omit, but the rule only fired inside that skill — a PR opened directly with `gh pr create` skipped it entirely, which is how a customer name, a knowledge base id, and verbatim sheet and column names reached a public PR description. Ship's list now covers every artifact rather than just the title and body, names verbatim customer content as its own category, draws the line on aggregate counts (fine detached from a tenant, not fine attributed to one), and carries a pre-publish grep so the check is mechanical instead of remembered. Babysit had no such guidance at all despite posting replies continuously, and triage is precisely where prod evidence gets pasted in. It now has a short section plus a hard rule, pointing at ship's list rather than restating it. --- .agents/skills/babysit/SKILL.md | 12 ++++++++++++ .agents/skills/ship/SKILL.md | 15 +++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.agents/skills/babysit/SKILL.md b/.agents/skills/babysit/SKILL.md index 8f324d190b3..42a57b5ecd8 100644 --- a/.agents/skills/babysit/SKILL.md +++ b/.agents/skills/babysit/SKILL.md @@ -134,9 +134,21 @@ round. Always check both conditions freshly after every push. When the loop ends, summarize: how many rounds it took, what was actually fixed (one line each), what was pushed back on as a false positive and why, and the final Greptile score / thread count. +## Public-repo hygiene + +Every reply, comment and commit you post here is public and permanent, and review bots quote +your replies back so a leak propagates. Before each post, strip anything that ties the change to +a tenant: customer/company names, workspace/user/org/KB/connector IDs, emails, tenant hostnames, +verbatim document/sheet/folder names, log lines, and per-tenant DB output. Cite the mechanism and +aggregate numbers instead — see `/ship`'s "What to Omit" for the full list and the pre-publish +grep. Triaging a finding often means pasting evidence you gathered from prod; that is exactly the +moment this gets violated. Check before posting, not after: editing a comment does not unsend its +notification email. + ## Hard rules - Never post the two re-review mentions as a single combined comment. +- Never paste prod evidence into a reply without scrubbing it first (see above). - Never resolve a thread without replying to it first. - Never fix a finding with a hacky workaround — if the clean fix isn't obvious, find the sibling pattern elsewhere in the codebase solving the same class of problem and match it. diff --git a/.agents/skills/ship/SKILL.md b/.agents/skills/ship/SKILL.md index 8d30b1b63d2..76c5bbf3643 100644 --- a/.agents/skills/ship/SKILL.md +++ b/.agents/skills/ship/SKILL.md @@ -102,13 +102,20 @@ chore(scope): description for maintenance ## What to Omit -The repo is public. Keep the title and description to the code change and its reasoning — never: +The repo is public. **Everything you publish — title, description, commit messages, and every later comment — must stand on its own without the incident that produced it.** Never include: -- Customer, company, or user names; workspace/user/org IDs; email addresses +- Customer, company, or user names; workspace/user/org/KB/connector IDs; email addresses - Prod or staging operational data: log lines, DB rows, metrics, timestamps, incident details, canary/alert output -- Infrastructure specifics: hostnames, ARNs, internal URLs, env var values, secret names +- Infrastructure specifics: hostnames (incl. tenant subdomains), ARNs, internal URLs, env var values, secret names +- Verbatim customer content: file names, document titles, sheet/column names, folder paths -Describe the bug by its mechanism, not by how you found it. "Expired OAuth credentials fail to refresh in the worker" — not "the Sheets canary failed at 16:31Z for workspace abc-123". +Describe the bug by its mechanism, not by how you found it. "Expired OAuth credentials fail to refresh in the worker" — not "the Sheets canary failed at 16:31Z for workspace abc-123". Aggregate counts are fine once detached from the tenant ("1,379 PDFs failed"); the same number attributed to a named customer is not. Replace real examples with placeholders (``) rather than cutting them — the illustration is usually the useful part. + +**Scrub before publishing, not after** — a leak is public the instant it posts, and editing later does not unsend the notification email. This applies to every PR you open, including ones created directly with `gh pr create` rather than through this skill. Grep the title, body, and `git log origin/staging..HEAD` before publishing: + +```bash +grep -niE 'customer-or-company-name|@[a-z0-9.-]+\.(com|io|ai)|[0-9a-f]{8}-[0-9a-f]{4}-|\.sharepoint\.com|arn:aws|https?://[a-z0-9.-]*\.internal' +``` ## PR Description Format From d1e3eeea9e2b8e3e13d0b57d2b9c509c9ccd75fa Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 14:11:47 -0700 Subject: [PATCH 02/22] fix(knowledge): parse the stored artifact, not the document's display name (#6817) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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` key, with `mimeType: 'text/plain'`. `processDocumentAsync` discards the processing filename the sync engine computes and rebuilds its input from the document row, so the parser was chosen from the display name and re-parsed extracted text as the source binary. In production that failed 1,379 SharePoint PDFs with `Invalid PDF structure.` and silently double-wrapped 364 spreadsheets — those reported `completed`, wrapping a second fake sheet around the connector's own extraction, 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 that rule because `fitStorageKeyName` preserves extensions through truncation: an upload keys on its original name, a connector document keys on what it stored. This layer is what covers the stuck-document retry sweep, which rebuilds its own input from the same display name — the sweep is the path that reprocesses the already-failed documents, so a fix confined to `processDocumentAsync` would have left the remediation itself broken. 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. `connectorArtifactFileName` now owns the `.txt` suffix that the parser choice depends on, so the invariant is structural instead of a convention repeated at four call sites per function. * fix(knowledge): raise the connector sync ceiling and tie it to the stale lock A 2,600-document library exhausted the 30-minute budget and the run was killed mid-listing, leaving the connector's `syncing` lock set until the scheduler reclaimed it. Raising the ceiling is not a lone constant, because reclaiming a stale lock flips the connector to `error` and frees it for another sync. A TTL at or below the run ceiling would hand the lock to a successor while the first sync is still writing — two syncs racing the same `(connectorId, externalId)` rows. The previous values, a 1800s run against a hard-coded 120-minute TTL declared in a different file, held that invariant only by coincidence. Both now derive from one another, with a test pinning the margin so the next raise cannot silently break it. --- .../api/knowledge/connectors/sync/route.ts | 4 +- .../background/knowledge-connector-sync.ts | 3 +- .../lib/knowledge/connectors/sync-engine.ts | 38 ++++++---- .../knowledge/connectors/sync-limits.test.ts | 26 +++++++ .../lib/knowledge/connectors/sync-limits.ts | 16 ++++ .../knowledge/documents/document-processor.ts | 9 ++- .../knowledge/documents/parser-extension.ts | 39 +++++++++- .../stored-artifact-extension.test.ts | 73 +++++++++++++++++++ 8 files changed, 186 insertions(+), 22 deletions(-) create mode 100644 apps/sim/lib/knowledge/connectors/sync-limits.test.ts create mode 100644 apps/sim/lib/knowledge/connectors/sync-limits.ts create mode 100644 apps/sim/lib/knowledge/documents/stored-artifact-extension.test.ts diff --git a/apps/sim/app/api/knowledge/connectors/sync/route.ts b/apps/sim/app/api/knowledge/connectors/sync/route.ts index fcec6ace21e..e6dd8177b1f 100644 --- a/apps/sim/app/api/knowledge/connectors/sync/route.ts +++ b/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) diff --git a/apps/sim/background/knowledge-connector-sync.ts b/apps/sim/background/knowledge-connector-sync.ts index ee17426f2bc..6efe6dbd26e 100644 --- a/apps/sim/background/knowledge-connector-sync.ts +++ b/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, diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index ccd804a50df..dcbc354173b 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -64,6 +64,20 @@ const MAX_CONSECUTIVE_FAILURES = 10 function sanitizeStorageTitle(title: string): string { return title.replace(/[^a-zA-Z0-9.-]/g, '_').slice(0, MAX_SAFE_TITLE_LENGTH) } + +/** + * Name a connector document's stored object carries. + * + * 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. + */ +function connectorArtifactFileName(title: string): string { + return `${sanitizeStorageTitle(title)}.txt` +} type KnowledgeBaseLockingTx = Pick type DocOp = @@ -1657,17 +1671,17 @@ async function addDocument( ): Promise { const documentId = generateId() const contentBuffer = Buffer.from(extDoc.content, 'utf-8') - const safeTitle = sanitizeStorageTitle(extDoc.title) - const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${documentId}-`, `${safeTitle}.txt`)}` + const storedFileName = connectorArtifactFileName(extDoc.title) + const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${documentId}-`, storedFileName)}` const fileInfo = await StorageService.uploadFile({ file: contentBuffer, - fileName: `${safeTitle}.txt`, + fileName: storedFileName, contentType: 'text/plain', context: 'knowledge-base', customKey, preserveKey: true, - metadata: kbOwnershipMetadata(kbOwner, `${safeTitle}.txt`), + metadata: kbOwnershipMetadata(kbOwner, storedFileName), }) const fileUrl = `${getInternalApiBaseUrl()}${fileInfo.path}?context=knowledge-base` @@ -1676,8 +1690,6 @@ async function addDocument( ? resolveTagMapping(connectorType, extDoc.metadata, sourceConfig) : undefined - const processingFilename = `${safeTitle}.txt` - try { await db.transaction(async (tx) => { const isActive = await isKnowledgeBaseActiveInTx(tx, knowledgeBaseId) @@ -1718,7 +1730,7 @@ async function addDocument( return { documentId, - filename: processingFilename, + filename: storedFileName, fileUrl, fileSize: contentBuffer.length, mimeType: 'text/plain', @@ -1746,17 +1758,17 @@ async function updateDocument( const oldFileUrl = existingRows[0]?.fileUrl const contentBuffer = Buffer.from(extDoc.content, 'utf-8') - const safeTitle = sanitizeStorageTitle(extDoc.title) - const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${existingDocId}-`, `${safeTitle}.txt`)}` + const storedFileName = connectorArtifactFileName(extDoc.title) + const customKey = `kb/${buildStorageKeySegment(`${Date.now()}-${existingDocId}-`, storedFileName)}` const fileInfo = await StorageService.uploadFile({ file: contentBuffer, - fileName: `${safeTitle}.txt`, + fileName: storedFileName, contentType: 'text/plain', context: 'knowledge-base', customKey, preserveKey: true, - metadata: kbOwnershipMetadata(kbOwner, `${safeTitle}.txt`), + metadata: kbOwnershipMetadata(kbOwner, storedFileName), }) const fileUrl = `${getInternalApiBaseUrl()}${fileInfo.path}?context=knowledge-base` @@ -1765,8 +1777,6 @@ async function updateDocument( ? resolveTagMapping(connectorType, extDoc.metadata, sourceConfig) : undefined - const processingFilename = `${safeTitle}.txt` - try { await db.transaction(async (tx) => { const isActive = await isKnowledgeBaseActiveInTx(tx, knowledgeBaseId) @@ -1839,7 +1849,7 @@ async function updateDocument( return { documentId: existingDocId, - filename: processingFilename, + filename: storedFileName, fileUrl, fileSize: contentBuffer.length, mimeType: 'text/plain', diff --git a/apps/sim/lib/knowledge/connectors/sync-limits.test.ts b/apps/sim/lib/knowledge/connectors/sync-limits.test.ts new file mode 100644 index 00000000000..41f5a0da19e --- /dev/null +++ b/apps/sim/lib/knowledge/connectors/sync-limits.test.ts @@ -0,0 +1,26 @@ +/** + * @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 frees it for another sync, so a TTL at or below the + * run ceiling would start a second sync while the first is still writing. This + * guards the invariant against a future hard-coded TTL, not the derivation. + */ + 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) + }) +}) diff --git a/apps/sim/lib/knowledge/connectors/sync-limits.ts b/apps/sim/lib/knowledge/connectors/sync-limits.ts new file mode 100644 index 00000000000..5752d05c6a1 --- /dev/null +++ b/apps/sim/lib/knowledge/connectors/sync-limits.ts @@ -0,0 +1,16 @@ +/** + * Wall-clock ceiling for a single connector sync run. A large document library + * needs more than the half hour this used to allow: a 2,600-document site + * exhausted the old budget and was killed mid-listing, leaving its `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. + * + * MUST stay above {@link CONNECTOR_SYNC_MAX_DURATION_SECONDS}: reclaiming frees the + * lock for another sync, so a TTL at or below the run ceiling would start a second + * sync while the first is still writing, both racing the same documents. + */ +export const CONNECTOR_SYNC_STALE_LOCK_TTL_MS = CONNECTOR_SYNC_MAX_DURATION_SECONDS * 2 * 1000 diff --git a/apps/sim/lib/knowledge/documents/document-processor.ts b/apps/sim/lib/knowledge/documents/document-processor.ts index 5ec1a39bda7..baa36447948 100644 --- a/apps/sim/lib/knowledge/documents/document-processor.ts +++ b/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,9 @@ async function parseHttpFile( ): Promise<{ content: string; metadata?: FileParseMetadata }> { const buffer = await downloadFileWithTimeout(fileUrl, userId) - const extension = resolveParserExtension(filename, mimeType) + /** Prefer what we actually downloaded over what the document is *called*. */ + const extension = + resolveStoredArtifactExtension(fileUrl) ?? resolveParserExtension(filename, mimeType) const result = await parseBuffer(buffer, extension) return result } diff --git a/apps/sim/lib/knowledge/documents/parser-extension.ts b/apps/sim/lib/knowledge/documents/parser-extension.ts index 39e7728b674..974b6235b1b 100644 --- a/apps/sim/lib/knowledge/documents/parser-extension.ts +++ b/apps/sim/lib/knowledge/documents/parser-extension.ts @@ -1,4 +1,9 @@ -import { getExtensionFromMimeType } from '@/lib/uploads/utils/file-utils' +import { + extractStorageKey, + getExtensionFromMimeType, + getFileExtension, + isInternalFileUrl, +} from '@/lib/uploads/utils/file-utils' import { isAlphanumericExtension, isSupportedExtension, @@ -12,8 +17,8 @@ export function resolveParserExtension( mimeType?: string, fallback?: string ): string { - const raw = filename.includes('.') ? filename.split('.').pop()?.toLowerCase() : undefined - const filenameExtension = raw && isAlphanumericExtension(raw) ? raw : undefined + const raw = getFileExtension(filename) + const filenameExtension = isAlphanumericExtension(raw) ? raw : undefined if (filenameExtension && isSupportedExtension(filenameExtension)) { return filenameExtension @@ -36,3 +41,31 @@ 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/-Report.pdf`) and a connector document keys + * on what it stored (`kb/-Report.pdf.txt`). + * + * 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. + */ +export function resolveStoredArtifactExtension(fileUrl: string): string | undefined { + if (!isInternalFileUrl(fileUrl)) return undefined + + const extension = getFileExtension(extractStorageKey(fileUrl)) + if (!isAlphanumericExtension(extension)) return undefined + + return isSupportedExtension(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 new file mode 100644 index 00000000000..25760202244 --- /dev/null +++ b/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') + }) +}) From 3a03774e4274670401b90e3208cb19399e4e757c Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 18 Aug 2026 14:23:19 -0700 Subject: [PATCH 03/22] fix(forks): stop copying connector-managed knowledge base documents (#6818) * fix(forks): stop copying connector-managed knowledge base documents A fork copies a KB's documents but never its connectors, so a connector-sourced document arrives with `connector_id` nulled and its `external_id` intact. The sync engine keys every existing/tombstone/ exclusion lookup off `connector_id`, so that copy is invisible to it - never updated, reconciled, or purged - and `doc_connector_external_id_idx` does not constrain it either, since its `connector_id` is NULL. Attaching a connector in the child then re-ingests every page as a NEW row on top of the snapshot. Each fork hop re-copies the previous hop's orphans and adds one more generation, so a prod -> UAT -> staging chain leaves three rows per page and a knowledge search returns the same page three times, one of them serving content frozen at the fork date. Exclude connector-managed documents from all four doors a document can enter a fork through: the whole-KB content copy, the in-transaction placeholder pre-creation, the sync-only copy into an already-mapped KB, and the content fill (guarded for payloads planned by a pre-change worker mid-rollout). The placeholder path matters as much as the copy loop - filtering only the content phase would leave a permanently archived row behind a persisted `knowledge_document` mapping. Skipped on both sides, the reference clears like any other uncopied document's. A document whose connector was deleted already has a null `connector_id` (the FK is ON DELETE SET NULL) and is static in the source too, so it still copies. One count(*) per copied KB logs what was left behind, since a fully connector-synced KB now forks to zero documents. Co-Authored-By: Claude Opus 5 (1M context) * fix(forks): keep the skipped-document count from failing a copied KB The connector-managed count feeds a log line, but it sat inside the KB's try block, so a transient failure on a COUNT(*) would roll back a copy that had otherwise succeeded and clear every reference to it. Move it into a helper that swallows its own error. Counting is not copying: only the copy itself may fail a resource. Test proven red by removing the catch - the mutation reports a knowledge-base failure. Co-Authored-By: Claude Opus 5 (1M context) * fix(forks): clean up full-KB placeholders planned before the exclusion The mapped-KB fill guarded a pre-change plan, but the full-KB path did not: a placeholder planned by an old worker for a connector-managed document is simply no longer returned by the page query, so nothing fills it and it stays archived behind a live mapping that a remapped document-selector still resolves to. Report those child ids as failed documents so the shared cleanup clears their references and drops the rows, and delete their persisted identity so a later sync does not resolve to a row cleanup removes. Keyed on the SOURCE being connector-managed, which can never become copyable, so it cannot race a concurrent attempt mid-fill the way a "source is gone" check could. The mapping drop is now one helper shared with the mapped-KB catch. Test proven red by removing the reconciliation block. Co-Authored-By: Claude Opus 5 (1M context) * fix(forks): make the stale-plan probe best-effort The probe ran inside the KB try, so a transient SELECT would reach the catch, roll back a complete copy, delete the child base, and clear every reference to it. Weighing it as "load-bearing, so fail closed" was wrong: the probe runs on EVERY copied KB that has referenced documents, while the state it repairs exists only inside a rollout window. Failing closed traded a common-path outage against a rare-squared one. It now swallows its own failure with a loud error log, leaving that pre-existing state in place rather than destroying a good copy. Test proven red by removing the catch - the mutation reports the KB failure. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- .../docs/en/platform/enterprise/forks.mdx | 23 ++- .../lib/copy/copy-resources.test.ts | 191 +++++++++++++++++- .../lib/copy/copy-resources.ts | 166 +++++++++++++-- 3 files changed, 356 insertions(+), 24 deletions(-) diff --git a/apps/docs/content/docs/en/platform/enterprise/forks.mdx b/apps/docs/content/docs/en/platform/enterprise/forks.mdx index 7185eb39627..ebaa22fb6ea 100644 --- a/apps/docs/content/docs/en/platform/enterprise/forks.mdx +++ b/apps/docs/content/docs/en/platform/enterprise/forks.mdx @@ -174,7 +174,7 @@ How each resource behaves at **fork** time vs **sync** time. Use this when you a | [Excluded workflows](#excluded-workflows) | Never | Never — not sent, not overwritten, not archived | | Files | Optional copy (default on) | Map or copy | | Tables | Optional copy (default on) | Map or copy | -| Knowledge bases (+ documents) | Optional copy; referenced docs come with the KB | Map or copy; documents follow the KB | +| Knowledge bases (+ documents) | Optional copy; uploaded documents come with the KB, [connector-synced ones do not](#connector-synced-documents-are-not-copied) | Map or copy; documents follow the KB | | Custom tools | Optional copy (default on) | Map or copy | | Skills | Optional copy (default on) | Map or copy | | External MCP servers | Optional copy (config only; sign-in cleared) | Map or copy (config only; sign-in cleared) | @@ -227,10 +227,27 @@ Only **deployed** workflows move. Deploy is the commit; sync is the force push/p | | Behavior | |---|----------| -| **Fork** | Optional copy (default on). Tag definitions come with the knowledge base. Documents that the forked workflows actually reference are included. Deselect → knowledge base / document fields clear. | +| **Fork** | Optional copy (default on). Tag definitions come with the knowledge base, along with every **uploaded** document in it. Deselect → knowledge base / document fields clear. | | **Sync** | Map or copy the knowledge base. Documents are not mapped by themselves — they follow the knowledge base (copied with it, or re-picked when you map to an existing one). | -**Example:** An agent searches knowledge base “Product docs.” Fork with that knowledge base selected → the child gets the base, tags, and the documents the agent used. On sync, mapping to the child’s existing “Product docs” means re-picking which document the tool should use. +**Example:** An agent searches knowledge base “Product docs.” Fork with that knowledge base selected → the child gets the base, tags, and the uploaded documents. On sync, mapping to the child’s existing “Product docs” means re-picking which document the tool should use. + +#### Connector-synced documents are not copied + +Connectors themselves never cross a fork edge — the child gets no Confluence, Notion, Google Drive, or other sync running against it. Documents that a **connector** put in the knowledge base are therefore not copied either. Only documents you **uploaded** come across. + + + Fork a knowledge base whose content is entirely connector-synced and the child gets the base, its tags, and its settings — but **no documents**. Add the connector in the child to fill it. + + +This is deliberate. A copied connector document would arrive detached from any connector, so nothing would ever update, re-sync, or remove it — and when you added the connector in the child it would ingest every page again *alongside* the stale copy. Chain a few forks (prod → UAT → staging) and each hop leaves another dead generation behind, so one page comes back several times in a single knowledge search. Skipping them keeps the child’s own connector the single owner of that content. + +| To get connector content into the child | Do this | +|---|---| +| Keep it live | Add the same connector in the child and let it sync. It re-ingests everything, so nothing is lost. | +| Keep a frozen snapshot | Download the documents from the source and upload them to the child’s knowledge base — uploaded documents copy on every later fork. | + +A document whose connector was **deleted** in the source is no longer connector-managed, so it copies like any other uploaded document. --- diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts index b965aa0654e..274605ad669 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.test.ts @@ -6,7 +6,9 @@ import { folder as folderTable } from '@sim/db/schema' import { sha256Hex } from '@sim/security/hash' import { dbChainMockFns, + flattenMockConditions, resetDbChainMock, + schemaMock, storageServiceMock, storageServiceMockFns, } from '@sim/testing' @@ -311,6 +313,106 @@ describe('copyForkResourceContent', () => { expect(mockPersistCopiedResourceMappings).not.toHaveBeenCalled() }) + it('never copies a connector-managed document out of the source knowledge base', async () => { + dbChainMockFns.limit.mockResolvedValueOnce([]) + + const result = await copyForkResourceContent({ + contentPlan: basePlan({ + knowledgeBases: [{ sourceId: 'src-kb', childId: 'child-kb', documentIdMap: {} }], + }), + requestId: 'test', + }) + + expect(result).toEqual({ copied: 1, failed: 0, failures: [] }) + // The row queue returns whatever is enqueued regardless of the predicate, so the exclusion + // is only observable in the condition tree. Pinned to the column so the assertion keeps its + // meaning if another nullable filter joins the same clause. + const pageWhere = dbChainMockFns.where.mock.calls.at(-1)?.[0] + expect( + flattenMockConditions(pageWhere).some( + (node) => node.type === 'isNull' && node.column === schemaMock.document.connectorId + ) + ).toBe(true) + }) + + it('drops a full-KB placeholder a pre-change worker planned for a connector-managed doc', async () => { + // Rolling deploy: the fork tx ran on the old code and planned a placeholder for a + // connector-managed document, which this worker's page query no longer returns. Nothing + // would ever fill it, so it must be reported for cleanup rather than left archived behind a + // live mapping that a remapped document-selector still resolves to. + dbChainMockFns.where.mockImplementationOnce(() => ({ + // The skipped-document count. + then: (resolve: (rows: unknown[]) => unknown) => resolve([{ total: 1 }]), + })) + dbChainMockFns.where.mockImplementationOnce(() => ({ + // The stale-plan probe: the planned source is connector-managed. + then: (resolve: (rows: unknown[]) => unknown) => resolve([{ id: 'doc-1' }]), + })) + dbChainMockFns.limit.mockResolvedValueOnce([]) + + const result = await copyForkResourceContent({ + contentPlan: basePlan({ + knowledgeBases: [ + { sourceId: 'src-kb', childId: 'child-kb', documentIdMap: { 'doc-1': 'child-doc-1' } }, + ], + documentMappingContext: { edgeChildWorkspaceId: 'edge-child-ws', sourceIsParent: false }, + }), + requestId: 'test', + }) + + expect(result.failed).toBe(1) + expect(result.failures).toEqual([{ kind: 'knowledge-document', childId: 'child-doc-1' }]) + // The persisted identity goes too, or a later sync resolves to the row cleanup deletes. + expect(mockDeleteCopiedResourceMappingsByTargets).toHaveBeenCalledWith({ + executor: expect.anything(), + edgeChildWorkspaceId: 'edge-child-ws', + sourceIsParent: false, + targets: [{ resourceType: 'knowledge_document', resourceId: 'child-doc-1' }], + }) + }) + + it('keeps a copied KB alive when the stale-plan probe fails', async () => { + // The probe runs on every KB with referenced documents, but the state it repairs only exists + // inside a rollout window. Letting it reach the KB catch would delete a complete copy and + // clear every reference to it over a transient SELECT. + dbChainMockFns.where.mockImplementationOnce(() => ({ + then: (resolve: (rows: unknown[]) => unknown) => resolve([{ total: 0 }]), + })) + dbChainMockFns.where.mockImplementationOnce(() => { + throw new Error('stale-plan probe failed') + }) + dbChainMockFns.limit.mockResolvedValueOnce([]) + + const result = await copyForkResourceContent({ + contentPlan: basePlan({ + knowledgeBases: [ + { sourceId: 'src-kb', childId: 'child-kb', documentIdMap: { 'doc-1': 'child-doc-1' } }, + ], + }), + requestId: 'test', + }) + + expect(result).toEqual({ copied: 1, failed: 0, failures: [] }) + }) + + it('keeps a copied KB alive when the skipped-document count fails', async () => { + // The count only feeds a log line. Letting it throw into the KB's catch would roll back a + // perfectly good copy and clear every reference to it over a failed COUNT(*). + dbChainMockFns.where.mockImplementationOnce(() => { + throw new Error('count failed') + }) + dbChainMockFns.limit.mockResolvedValueOnce([]) + + const result = await copyForkResourceContent({ + contentPlan: basePlan({ + knowledgeBases: [{ sourceId: 'src-kb', childId: 'child-kb', documentIdMap: {} }], + }), + requestId: 'test', + }) + + expect(result).toEqual({ copied: 1, failed: 0, failures: [] }) + }) + it('uses the blob content digest so a retry cannot adopt an older failed snapshot', async () => { dbChainMockFns.limit .mockResolvedValueOnce([sourceDoc]) @@ -1051,6 +1153,25 @@ describe('copyForkResourceContent', () => { }) }) + it('U-docs: refuses a connector-managed source planned before the exclusion existed', async () => { + // A payload queued by a pre-change worker during a rolling deploy: the planner would no + // longer emit this entry, so the fill must drop the placeholder rather than detach a copy + // of a connector-managed document into the existing target KB. + dbChainMockFns.limit + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([{ ...sourceDoc, connectorId: 'connector-1' }]) + + const result = await copyForkResourceContent({ + contentPlan: mappedDocumentPlan(), + requestId: 'test', + }) + + expect(result.copied).toBe(0) + expect(result.failures).toEqual([{ kind: 'knowledge-document', childId: 'child-doc-1' }]) + expect(storageServiceMockFns.mockDownloadFile).not.toHaveBeenCalled() + expect(mockIncrementStorageUsageInTx).not.toHaveBeenCalled() + }) + it('U-docs: refuses to charge when the target knowledge base moved workspaces', async () => { queueMappedDocumentCopy() dbChainMockFns.for.mockResolvedValueOnce([{ workspaceId: 'other-workspace' }]) @@ -1359,10 +1480,12 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => { // would make every source folder look already-present and suppress the mirroring. let folderCall = 0 const inserts: Array>> = [] + const wheres: Array<{ table: unknown; condition: unknown }> = [] const tx = { select: () => ({ from: (table: unknown) => ({ - where: () => { + where: (condition: unknown) => { + wheres.push({ table, condition }) if (table === folderTable) { return Promise.resolve(folderCall++ === 0 ? sourceFolders : []) } @@ -1377,7 +1500,7 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => { }, }), } - return { tx: tx as unknown as DbOrTx, inserts } + return { tx: tx as unknown as DbOrTx, inserts, wheres } } const kbSelection = { @@ -1458,6 +1581,35 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => { expect(inserts).toHaveLength(1) }) + it('does not pre-create a placeholder for a referenced connector-managed document', async () => { + const { tx, wheres } = makeKbTx([[sourceBase], [], []]) + + const result = await copyForkResourceContainers({ + tx, + sourceWorkspaceId: 'src-ws', + childWorkspaceId: 'child-ws', + userId: 'user-1', + now: new Date(), + selection: kbSelection, + workflowIdMap: new Map(), + referencedDocumentIds: ['doc-1'], + documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true }, + }) + + // Must agree with the content phase's exclusion: a placeholder with no content copy behind + // it would stay archived forever while its persisted mapping pointed at it. + const placeholderWhere = wheres.find(({ table }) => table === schemaMock.document)?.condition + expect( + flattenMockConditions(placeholderWhere).some( + (node) => node.type === 'isNull' && node.column === schemaMock.document.connectorId + ) + ).toBe(true) + expect(result.mappingEntries.some((entry) => entry.resourceType === 'knowledge_document')).toBe( + false + ) + expect(result.contentPlan.knowledgeBases[0].documentIdMap).toEqual({}) + }) + it('mirrors the source knowledge-base folder and copies the KB into it, not the target root', async () => { const foldered = { ...sourceBase, folderId: 'kb-folder' } const { tx, inserts } = makeKbTx( @@ -1510,7 +1662,9 @@ describe('planForkMappedKbDocumentCopies', () => { fileSize: 123, filename: `${id}.pdf`, mimeType: 'application/pdf', - connectorId: 'connector-1', + // Hand-uploaded: connector-managed documents are filtered out by the candidate query and + // can never reach the placeholder insert. + connectorId: null, deletedAt: null, archivedAt: null, }) @@ -1526,11 +1680,19 @@ describe('planForkMappedKbDocumentCopies', () => { }> = [] ) { const inserted: Array> = [] + const wheres: unknown[] = [] let selectCalls = 0 const tx = { select: () => { const rows = selectCalls++ === 0 ? docs : existingTargets - return { from: () => ({ where: () => Promise.resolve(rows) }) } + return { + from: () => ({ + where: (condition: unknown) => { + wheres.push(condition) + return Promise.resolve(rows) + }, + }), + } }, insert: () => ({ values: (rows: Array>) => { @@ -1539,7 +1701,7 @@ describe('planForkMappedKbDocumentCopies', () => { }, }), } - return { tx: tx as unknown as DbOrTx, inserted, selectCalls: () => selectCalls } + return { tx: tx as unknown as DbOrTx, inserted, wheres, selectCalls: () => selectCalls } } const mappedKbResolver: ForkReferenceResolver = (kind, id) => @@ -1584,6 +1746,25 @@ describe('planForkMappedKbDocumentCopies', () => { ]) }) + it('never considers a connector-managed doc as a candidate for the mapped target KB', async () => { + const { tx, wheres } = makeTx([]) + await planForkMappedKbDocumentCopies({ + tx, + resolver: mappedKbResolver, + referencedDocumentIds: ['doc-1'], + alreadyCopiedSourceDocIds: new Set(), + now, + }) + + // The tx mock returns its rows regardless of the predicate, so the exclusion is only + // observable in the condition tree. + expect( + flattenMockConditions(wheres[0]).some( + (node) => node.type === 'isNull' && node.column === schemaMock.document.connectorId + ) + ).toBe(true) + }) + it('skips a referenced doc whose parent KB is not mapped (reference is left to be cleared)', async () => { const { tx, inserted } = makeTx([sourceRow('doc-1', 'unmapped-kb')]) const result = await planForkMappedKbDocumentCopies({ diff --git a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts index dc3c9dbcbdd..a50830cebce 100644 --- a/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts +++ b/apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts @@ -368,6 +368,11 @@ type SkillSkeletonInsert = Omit & { conten * {@link copyForkResourceContent} to copy best-effort after commit. Secrets are * never copied: MCP OAuth tokens are omitted (re-auth required) and KB connectors * are not copied (the child is a content snapshot without live sync). + * + * Because the child gets no connector, connector-MANAGED documents are not copied + * either - only hand-uploaded ones. A detached copy is unreachable by the sync engine + * (which keys off `connector_id`), so re-attaching a connector in the child would layer + * a fresh generation on top of it instead of updating it. See {@link copyForkResourceContent}. */ export async function copyForkResourceContainers( params: CopyResourcesParams @@ -784,6 +789,12 @@ export async function copyForkResourceContainers( * Each deterministic placeholder is archived with no storage key and zero bytes, so it is * non-billable until {@link copyForkResourceContent} activates it atomically with accounting. * Documents whose parent KB is not copied are skipped, leaving their references to be cleared. + * + * Connector-managed documents are skipped for the same reason {@link copyForkResourceContent} + * excludes them from the bulk copy - a detached snapshot the child's connector would duplicate. + * Skipping them HERE too is what keeps the two sides consistent: a placeholder with no content + * phase behind it would stay archived forever while its persisted `knowledge_document` mapping + * pointed at it. Their references clear like any other uncopied document's. */ async function createForkDocumentPlaceholders(params: { tx: DbOrTx @@ -803,6 +814,7 @@ async function createForkDocumentPlaceholders(params: { and( inArray(document.id, referencedDocumentIds), inArray(document.knowledgeBaseId, Array.from(kbIdMap.keys())), + isNull(document.connectorId), isNull(document.deletedAt), isNull(document.archivedAt) ) @@ -845,7 +857,9 @@ async function createForkDocumentPlaceholders(params: { * Documents whose parent KB is being copied THIS sync are handled by * {@link createForkDocumentPlaceholders} under that copied KB and are excluded here via * `alreadyCopiedSourceDocIds`. A referenced document whose parent KB is not mapped at all is left - * untouched, so its reference is cleared as before. + * untouched, so its reference is cleared as before. Connector-managed documents are excluded for + * the reason given on {@link copyForkResourceContent} - and the exclusion matters MORE here, since + * the target KB is an existing one that may already run its own connector over the same source. */ export async function planForkMappedKbDocumentCopies(params: { tx: DbOrTx @@ -877,6 +891,7 @@ export async function planForkMappedKbDocumentCopies(params: { .where( and( inArray(document.id, candidateIds), + isNull(document.connectorId), isNull(document.deletedAt), isNull(document.archivedAt) ) @@ -1016,6 +1031,114 @@ export async function copyForkResourceContent(params: { billingContext ??= await resolveStorageBillingContext(childWorkspaceId) return billingContext } + /** + * Drop the persisted `knowledge_document` identity for a copied document that will not exist, + * so a later sync resolves the reference afresh instead of to a row the cleanup removes. + * Isolated like the rest of the post-commit phase: a mapping-cleanup failure is logged, never + * rethrown, since the caller is already reporting the document as failed. + */ + const dropCopiedDocumentMapping = async (childDocumentId: string): Promise => { + const mappingContext = contentPlan.documentMappingContext + if (!mappingContext) return + try { + await deleteCopiedResourceMappingsByTargets({ + executor: db, + edgeChildWorkspaceId: mappingContext.edgeChildWorkspaceId, + sourceIsParent: mappingContext.sourceIsParent, + targets: [{ resourceType: 'knowledge_document', resourceId: childDocumentId }], + }) + } catch (mappingCleanupError) { + logger.error(`[${requestId}] Failed to clean mapping for a failed copied document`, { + childDocumentId, + error: getErrorMessage(mappingCleanupError), + }) + } + } + /** + * Find the placeholders a worker from before this exclusion (a rolling deploy) planned for + * connector-managed documents, drop their persisted identities, and return the child ids to + * report as failed documents - the page query no longer returns their sources, so nothing + * would ever fill them, leaving archived empty rows that a mapping and a remapped + * `document-selector` still resolve to. + * + * Keyed on the SOURCE being connector-managed, which is deterministic: such a document can + * never become copyable, so this cannot race a concurrent attempt sitting between + * {@link ensureKbDocumentPlaceholder} and {@link finalizeKbDocument} (a "planned but unfilled" + * sweep would). + * + * Best-effort, like the count above: this probe runs on EVERY copied KB that has referenced + * documents, while the state it repairs exists only inside a rollout window. Letting a + * transient failure reach the KB's catch would delete an otherwise-complete copy and clear + * every reference to it - far worse, and far more likely, than the dangling placeholder it + * guards against. A failure is logged loudly and leaves that pre-existing state in place. + */ + const reconcileStalePlannedDocuments = async (kb: ForkContentKbEntry): Promise => { + const plannedSourceIds = Object.keys(kb.documentIdMap) + if (plannedSourceIds.length === 0) return [] + try { + const stalePlanned = await db + .select({ id: document.id }) + .from(document) + .where(and(inArray(document.id, plannedSourceIds), isNotNull(document.connectorId))) + const staleChildIds: string[] = [] + for (const { id } of stalePlanned) { + const childDocumentId = kb.documentIdMap[id] + if (!childDocumentId) continue + // Left in `documentIdMap` deliberately: if the KB itself later fails, its failure lists + // the same child id again, and the cleanup keys failed ids by kind in a Set. + await dropCopiedDocumentMapping(childDocumentId) + staleChildIds.push(childDocumentId) + logger.warn( + `[${requestId}] Dropping a fork placeholder planned for a connector-managed document`, + { sourceDocumentId: id, childDocumentId, childKnowledgeBaseId: kb.childId } + ) + } + return staleChildIds + } catch (error) { + logger.error( + `[${requestId}] Failed to reconcile fork placeholders planned for connector-managed documents`, + { + sourceKnowledgeBaseId: kb.sourceId, + childKnowledgeBaseId: kb.childId, + error: getErrorMessage(error), + } + ) + return [] + } + } + /** + * Report the connector-managed documents a copied KB leaves behind, since a fully + * connector-synced base lands in the child with no documents at all. Strictly observability, + * so it swallows its own failure: counting is not copying, and a transient error here must not + * take down the KB the way a failed document does. + */ + const logSkippedConnectorDocuments = async (kb: ForkContentKbEntry): Promise => { + try { + const [row] = await db + .select({ total: sql`count(*)` }) + .from(document) + .where( + and( + eq(document.knowledgeBaseId, kb.sourceId), + isNotNull(document.connectorId), + isNull(document.deletedAt), + isNull(document.archivedAt) + ) + ) + const skipped = Number(row?.total ?? 0) + if (skipped === 0) return + logger.info(`[${requestId}] Skipped connector-managed documents in a copied knowledge base`, { + sourceKnowledgeBaseId: kb.sourceId, + childKnowledgeBaseId: kb.childId, + skipped, + }) + } catch (error) { + logger.warn(`[${requestId}] Failed to count the documents a copied knowledge base skipped`, { + sourceKnowledgeBaseId: kb.sourceId, + error: getErrorMessage(error), + }) + } + } for (const table of contentPlan.tables) { try { @@ -1124,13 +1247,29 @@ export async function copyForkResourceContent(params: { for (const kb of contentPlan.knowledgeBases) { try { + await logSkippedConnectorDocuments(kb) + for (const childDocumentId of await reconcileStalePlannedDocuments(kb)) { + failedResources += 1 + failures.push({ kind: 'knowledge-document', childId: childDocumentId }) + } let afterDocId: string | null = null for (;;) { // Only copy LIVE documents - exclude soft-deleted and archived rows, matching // how the rest of the KB system treats them as gone (chunks/tags/search filter // both). A fork must not resurrect documents removed from the source base. + // + // Connector-managed documents are excluded too, because a copy could only ever be a + // DETACHED snapshot: the child gets no connector (see `copyForkResourceContainers`), and + // the sync engine keys every existing/tombstone/exclusion lookup off `connector_id`, so + // the copy is invisible to it - never updated, reconciled, or purged. Attaching a + // connector in the child then re-ingests every page as a NEW row on top of the snapshot, + // stacking one dead generation per fork hop. Skipping them leaves the child's own + // connector as the single owner of that content. A source document whose connector was + // DELETED already has a null `connector_id` (the FK is ON DELETE SET NULL) and is static + // content in the source too, so it still copies. const liveDocs = and( eq(document.knowledgeBaseId, kb.sourceId), + isNull(document.connectorId), isNull(document.deletedAt), isNull(document.archivedAt) ) @@ -1273,6 +1412,15 @@ export async function copyForkResourceContent(params: { if (!source) { throw new Error(`Source document ${docEntry.sourceDocId} is missing`) } + if (source.connectorId) { + // Only reachable from a payload planned before connector-managed documents were excluded + // (a rolling deploy). Fail the entry instead of filling it: the per-document cleanup + // below drops the archived placeholder and clears its references, which is the outcome + // the planner would now produce anyway. + throw new Error( + `Source document ${docEntry.sourceDocId} is connector-managed and is not copied across a fork edge` + ) + } const resolvedBillingContext = await getBillingContext() await copyKbDocument({ source, @@ -1284,21 +1432,7 @@ export async function copyForkResourceContent(params: { }) copiedResources += 1 } catch (error) { - if (contentPlan.documentMappingContext) { - try { - await deleteCopiedResourceMappingsByTargets({ - executor: db, - edgeChildWorkspaceId: contentPlan.documentMappingContext.edgeChildWorkspaceId, - sourceIsParent: contentPlan.documentMappingContext.sourceIsParent, - targets: [{ resourceType: 'knowledge_document', resourceId: docEntry.childDocId }], - }) - } catch (mappingCleanupError) { - logger.error(`[${requestId}] Failed to clean mapping for a failed copied document`, { - childDocumentId: docEntry.childDocId, - error: getErrorMessage(mappingCleanupError), - }) - } - } + await dropCopiedDocumentMapping(docEntry.childDocId) failedResources += 1 failures.push({ kind: 'knowledge-document', childId: docEntry.childDocId }) logger.warn(`[${requestId}] Failed to copy document into mapped KB during sync`, { From 56a270ed549694052f43e9d93316203735fb70b3 Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 14:31:50 -0700 Subject: [PATCH 04/22] feat(emails): add sub-processor change notification template (#6820) * feat(emails): add sub-processor change notification template * improvement(emails): link the objection address and preference URL --- .../components/emails/notifications/index.ts | 5 + .../subprocessor-change-email.tsx | 138 ++++++++++++++++++ .../emails/render-notifications.test.ts | 61 ++++++++ apps/sim/components/emails/render.ts | 18 ++- apps/sim/components/emails/subjects.ts | 3 + 5 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 apps/sim/components/emails/notifications/subprocessor-change-email.tsx diff --git a/apps/sim/components/emails/notifications/index.ts b/apps/sim/components/emails/notifications/index.ts index c930a723ce9..2a40138acf7 100644 --- a/apps/sim/components/emails/notifications/index.ts +++ b/apps/sim/components/emails/notifications/index.ts @@ -1 +1,6 @@ export { ScheduleDisabledEmail } from './schedule-disabled-email' +export { + type SubprocessorChange, + SubprocessorChangeEmail, + type SubprocessorChangeType, +} from './subprocessor-change-email' diff --git a/apps/sim/components/emails/notifications/subprocessor-change-email.tsx b/apps/sim/components/emails/notifications/subprocessor-change-email.tsx new file mode 100644 index 00000000000..fbb02da0a9e --- /dev/null +++ b/apps/sim/components/emails/notifications/subprocessor-change-email.tsx @@ -0,0 +1,138 @@ +import { Link, Section, Text } from '@react-email/components' +import { baseStyles } from '@/components/emails/_styles' +import { EmailButton, EmailLayout, EmailStrong } from '@/components/emails/components' +import { getBrandConfig } from '@/ee/whitelabeling' + +/** How a sub-processor's role on the list is changing. */ +export type SubprocessorChangeType = 'added' | 'replaced' | 'removed' + +const CHANGE_TYPE_LABEL: Record = { + added: 'New sub-processor', + replaced: 'Replacement sub-processor', + removed: 'Sub-processor being removed', +} + +/** + * Dates in a notice period have to be unambiguous, so the month is spelled out + * rather than left to the recipient's locale ordering of a numeric date. The + * zone is pinned because the notice window is contractual — the rendered day + * must not shift with the server the send happens to run on. + */ +const NOTICE_DATE_FORMAT: Intl.DateTimeFormatOptions = { + month: 'long', + day: 'numeric', + year: 'numeric', + timeZone: 'UTC', +} + +function formatNoticeDate(date: Date): string { + return date.toLocaleDateString('en-US', NOTICE_DATE_FORMAT) +} + +export interface SubprocessorChange { + /** Legal entity name of the sub-processor. */ + name: string + /** What it is used for, in plain language. */ + purpose: string + /** Categories of customer personal data it will process. */ + dataCategories: string + /** Primary processing location, e.g. `United States`. */ + location: string + changeType: SubprocessorChangeType +} + +interface SubprocessorChangeEmailProps { + recipientName?: string + /** The sub-processors being added, replaced, or removed in this notice. */ + changes: SubprocessorChange[] + /** When the change takes effect. Sent at least 30 days ahead of this date. */ + effectiveDate: Date + /** Last day an objection can be raised. Falls on or before {@link effectiveDate}. */ + objectionDeadline: Date + /** Where an objection is sent. */ + objectionEmail: string + /** The public sub-processor list, which reflects the change once it is live. */ + subprocessorListUrl: string + /** Where the recipient manages whether they receive these notices. */ + subscriptionUrl?: string +} + +/** + * Advance notice to subscribed customers that the sub-processors handling their + * personal data are changing, with the window and address for objecting. + */ +export function SubprocessorChangeEmail({ + recipientName, + changes, + effectiveDate, + objectionDeadline, + objectionEmail, + subprocessorListUrl, + subscriptionUrl, +}: SubprocessorChangeEmailProps) { + const brand = getBrandConfig() + const effectiveDateLabel = formatNoticeDate(effectiveDate) + const previewText = `${brand.name} is changing its sub-processors on ${effectiveDateLabel}` + + return ( + + {recipientName ? `Hi ${recipientName},` : 'Hi,'} + + + We are giving you advance notice of a change to the sub-processors {brand.name} uses to + process customer personal data. The change takes effect on{' '} + {effectiveDateLabel}. + + + {changes.map((change) => ( +
+ + {change.name} — {CHANGE_TYPE_LABEL[change.changeType]} + + + Purpose: {change.purpose} +
+ Data processed: {change.dataCategories} +
+ Processing location: {change.location} +
+ Effective: {effectiveDateLabel} + +
+ ))} + + + If you object to this change, reply to this email or write to{' '} + + {objectionEmail} + {' '} + by {formatNoticeDate(objectionDeadline)}. We will work with you + on a resolution, and you may terminate the affected service if we cannot reach one. + + + + No action is needed if you have no objection. The full list of sub-processors stays current + at the link below. + + + View sub-processor list + +
+ + + Sent to customers subscribed to sub-processor change notices. + {subscriptionUrl ? ( + <> + {' '} + + Manage whether you receive them + + . + + ) : null} + + + ) +} + +export default SubprocessorChangeEmail diff --git a/apps/sim/components/emails/render-notifications.test.ts b/apps/sim/components/emails/render-notifications.test.ts index b33ef4d8757..fb5286cfc08 100644 --- a/apps/sim/components/emails/render-notifications.test.ts +++ b/apps/sim/components/emails/render-notifications.test.ts @@ -8,6 +8,7 @@ import { describe, expect, it } from 'vitest' import { renderScheduleDisabledEmail, + renderSubprocessorChangeEmail, renderUsageLimitReachedEmail, } from '@/components/emails/render' @@ -93,3 +94,63 @@ describe('renderUsageLimitReachedEmail', () => { expect(html).not.toContain('$20') }) }) + +describe('renderSubprocessorChangeEmail', () => { + const notice = { + changes: [ + { + name: 'Example Analytics Inc.', + purpose: 'Product usage analytics', + dataCategories: 'Account identifiers, usage events', + location: 'United States', + changeType: 'added' as const, + }, + ], + effectiveDate: new Date('2026-10-01T00:00:00Z'), + objectionDeadline: new Date('2026-09-24T00:00:00Z'), + objectionEmail: 'privacy@example.com', + subprocessorListUrl: 'https://example.com/subprocessors', + } + + it('renders the change details, the objection window, and the list link', async () => { + const html = await renderSubprocessorChangeEmail({ recipientName: 'John', ...notice }) + + expect(html).toContain('Example Analytics Inc.') + expect(html).toContain('New sub-processor') + expect(html).toContain('Account identifiers, usage events') + expect(html).toContain('October 1, 2026') + expect(html).toContain('September 24, 2026') + expect(html).toContain('mailto:privacy@example.com') + expect(html).toContain('https://example.com/subprocessors') + }) + + it('renders every change in a multi-sub-processor notice', async () => { + const html = await renderSubprocessorChangeEmail({ + ...notice, + changes: [ + ...notice.changes, + { + name: 'Legacy Mail Co.', + purpose: 'Transactional email delivery', + dataCategories: 'Email addresses', + location: 'Ireland', + changeType: 'removed' as const, + }, + ], + }) + + expect(html).toContain('Legacy Mail Co.') + expect(html).toContain('Sub-processor being removed') + }) + + it('mentions the subscription setting only when one is given', async () => { + const withSetting = await renderSubprocessorChangeEmail({ + ...notice, + subscriptionUrl: 'https://example.com/preferences', + }) + const withoutSetting = await renderSubprocessorChangeEmail(notice) + + expect(withSetting).toContain('href="https://example.com/preferences"') + expect(withoutSetting).not.toContain('Manage whether you receive them') + }) +}) diff --git a/apps/sim/components/emails/render.ts b/apps/sim/components/emails/render.ts index 83d285fe17f..4d79c01cf10 100644 --- a/apps/sim/components/emails/render.ts +++ b/apps/sim/components/emails/render.ts @@ -25,7 +25,11 @@ import { WorkspaceAddedEmail, WorkspaceInvitationEmail, } from '@/components/emails/invitations' -import { ScheduleDisabledEmail } from '@/components/emails/notifications' +import { + ScheduleDisabledEmail, + type SubprocessorChange, + SubprocessorChangeEmail, +} from '@/components/emails/notifications' import { HelpConfirmationEmail } from '@/components/emails/support' import type { UpgradeReason } from '@/lib/billing/upgrade-reasons' import { getBaseUrl } from '@/lib/core/utils/urls' @@ -159,6 +163,18 @@ export async function renderScheduleDisabledEmail(params: { return await render(ScheduleDisabledEmail(params)) } +export async function renderSubprocessorChangeEmail(params: { + recipientName?: string + changes: SubprocessorChange[] + effectiveDate: Date + objectionDeadline: Date + objectionEmail: string + subprocessorListUrl: string + subscriptionUrl?: string +}): Promise { + return await render(SubprocessorChangeEmail(params)) +} + export async function renderFreeTierUpgradeEmail(params: { userName?: string percentUsed: number diff --git a/apps/sim/components/emails/subjects.ts b/apps/sim/components/emails/subjects.ts index a56b542d606..1d27fc720ec 100644 --- a/apps/sim/components/emails/subjects.ts +++ b/apps/sim/components/emails/subjects.ts @@ -20,6 +20,7 @@ export type EmailSubjectType = | 'abandoned-checkout' | 'free-tier-exhausted' | 'schedule-disabled' + | 'subprocessor-change' | 'onboarding-followup' | 'welcome' @@ -66,6 +67,8 @@ export function getEmailSubject(type: EmailSubjectType): string { return `You've run out of free credits on ${brandName}` case 'schedule-disabled': return `A schedule was turned off on ${brandName}` + case 'subprocessor-change': + return `Upcoming change to ${brandName} sub-processors` case 'onboarding-followup': return `Quick question about ${brandName}` case 'welcome': From e522bc4c5dc78f6402bb91a77bfc3870d6824229 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Tue, 18 Aug 2026 14:56:39 -0700 Subject: [PATCH 05/22] fix(forks): name the workspace a sync overwrites instead of "target" (#6822) * fix(forks): name the workspace a sync overwrites instead of "target" Co-Authored-By: Claude Opus 5 (1M context) * fix(forks): name the target workspace in the blocker resolution line too Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- .../fork-sync/cleared-refs-list.test.ts | 29 ++++++++++++++----- .../components/fork-sync/cleared-refs-list.ts | 13 +++++++-- .../components/fork-sync/fork-sync-view.tsx | 7 +++-- .../components/fork-sync/use-fork-sync.ts | 14 ++++++--- .../ee/workspace-forking/components/forks.tsx | 11 +++++-- 5 files changed, 55 insertions(+), 19 deletions(-) diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.test.ts b/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.test.ts index fce97ef41ca..aab10731aa4 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.test.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.test.ts @@ -149,21 +149,36 @@ describe('splitForkClearedRefs', () => { describe('forkBlockerResolution', () => { it('phrases each blocker reason with its actionable resolution', () => { - expect(forkBlockerResolution(referenceRef('table', 'tbl-1'))).toBe( + expect(forkBlockerResolution(referenceRef('table', 'tbl-1'), 'Acme Prod')).toBe( 'map it to a target or select it for copy' ) - expect(forkBlockerResolution(referenceRef('mcp-server', 'srv-1'))).toBe( + expect(forkBlockerResolution(referenceRef('mcp-server', 'srv-1'), 'Acme Prod')).toBe( 'map it to a target or select it for copy' ) - expect(forkBlockerResolution(referenceRef('knowledge-base', 'kb-gone', 'KB', true))).toBe( - 'deleted in the source — map it to an existing knowledge base in the target' - ) - expect(forkBlockerResolution(workflowRef('wf-other', 'Workflow'))).toBe( + expect( + forkBlockerResolution(referenceRef('knowledge-base', 'kb-gone', 'KB', true), 'Acme Prod') + ).toBe('deleted in the source — map it to an existing knowledge base in Acme Prod') + expect(forkBlockerResolution(workflowRef('wf-other', 'Workflow'), 'Acme Prod')).toBe( 'deploy "Source" in the source or remove the reference' ) }) + /** + * The source-deleted line phrases the same resolution as the mapping row's hint, so it must + * name the workspace the sync writes - "the target" is what this copy set out to remove. + */ + it('names the target workspace in the source-deleted resolution', () => { + const resolution = forkBlockerResolution( + referenceRef('knowledge-base', 'kb-gone', 'KB', true), + 'this workspace' + ) + expect(resolution).toBe( + 'deleted in the source — map it to an existing knowledge base in this workspace' + ) + expect(resolution).not.toContain('in the target') + }) + it('returns null for non-blocking dependent entries', () => { - expect(forkBlockerResolution(dependentRef('credential', 'cred-1'))).toBeNull() + expect(forkBlockerResolution(dependentRef('credential', 'cred-1'), 'Acme Prod')).toBeNull() }) }) diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts b/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts index 0e928112356..6bf7fa453b2 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/cleared-refs-list.ts @@ -77,15 +77,24 @@ export const FORK_RESOURCE_KIND_LABEL: Record = { /** * The actionable resolution line for a blocking entry, phrased for "{block} would lose {field} * in {workflow} - {resolution}". Null for non-blocking (dependent) entries. + * + * `targetWorkspaceName` is required rather than defaulted: the source-deleted line phrases the + * same resolution as the mapping row's own hint, and naming the workspace is the only way either + * says WHICH side the sync writes. A default would let "the target" quietly return. */ -export function forkBlockerResolution(ref: ForkClearedRef): string | null { +export function forkBlockerResolution( + ref: ForkClearedRef, + targetWorkspaceName: string +): string | null { const reason = forkSyncBlockerReasonFor(ref) if (!reason) return null switch (reason) { + // "a target" here is the target RESOURCE picked in the mapping row, not the workspace - + // it matches the picker's own "Select target" label, so it stays unnamed. case 'unmapped-copyable': return 'map it to a target or select it for copy' case 'source-deleted': - return `deleted in the source — map it to an existing ${FORK_RESOURCE_KIND_LABEL[ref.kind] ?? 'resource'} in the target` + return `deleted in the source — map it to an existing ${FORK_RESOURCE_KIND_LABEL[ref.kind] ?? 'resource'} in ${targetWorkspaceName}` case 'workflow-missing': return `deploy "${ref.sourceLabel}" in the source or remove the reference` } diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx index 9a27dbf54eb..487255b5839 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx +++ b/apps/sim/ee/workspace-forking/components/fork-sync/fork-sync-view.tsx @@ -457,8 +457,8 @@ function MappingEntry({ controller, group, entry }: MappingEntryProps) { {entry.sourceDeleted ? (

Deleted in the source — its name can't be shown. Map it to an existing{' '} - {FORK_RESOURCE_KIND_LABEL[entry.kind] ?? 'resource'} in the target, or fix the reference - in the source and redeploy. + {FORK_RESOURCE_KIND_LABEL[entry.kind] ?? 'resource'} in {controller.targetWorkspaceName} + , or fix the reference in the source and redeploy.

) : null} {entry.candidatesTruncated ? ( @@ -961,7 +961,8 @@ export function ForkSyncView({ controller, onDirectionChange }: ForkSyncViewProp {ref.blockLabel} would lose{' '} {ref.fieldLabel} in{' '} - {ref.workflowName} — {forkBlockerResolution(ref)} + {ref.workflowName} —{' '} + {forkBlockerResolution(ref, controller.targetWorkspaceName)} {/* Only a source-deleted reference can be dropped: an unmapped copyable can still be copied and a missing workflow can still be deployed, so neither is a dead diff --git a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts index 61aadc7bce5..f5c89c22112 100644 --- a/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts +++ b/apps/sim/ee/workspace-forking/components/fork-sync/use-fork-sync.ts @@ -104,8 +104,10 @@ export interface ForkSyncController { otherWorkspaceName: string /** * The workspace this sync WRITES, named for user-facing copy: the other workspace on push, - * "this workspace" on pull. Derived once here so every surface that names the target - the - * overwrite confirm, the Trigger URLs heading - says the same thing. + * this one on pull. Always a NAME rather than "the target" - the page header shows the OTHER + * workspace's name, so an unnamed target reads as that one even on pull. Falls back to + * "this workspace" only until the name loads. Derived once here so every surface that names + * it - the overwrite confirm, the Trigger URLs heading - says the same thing. */ targetWorkspaceName: string isLoading: boolean @@ -272,12 +274,15 @@ function takenTargetOwners( */ export function useForkSync(params: { workspaceId: string + /** This workspace's name, for copy that must say which side a pull overwrites. */ + workspaceName?: string otherWorkspaceId?: string otherWorkspaceName: string direction: ForkDirection enabled: boolean }): ForkSyncController { - const { workspaceId, otherWorkspaceId, otherWorkspaceName, direction, enabled } = params + const { workspaceId, workspaceName, otherWorkspaceId, otherWorkspaceName, direction, enabled } = + params // User's IN-SESSION mapping overrides only - NOT the source of truth. The displayed/persisted // target falls back to each entry's stored `targetId` (see `targetFor`), so a reopened edge @@ -982,7 +987,8 @@ export function useForkSync(params: { return { direction, otherWorkspaceName, - targetWorkspaceName: direction === 'push' ? otherWorkspaceName : 'this workspace', + targetWorkspaceName: + direction === 'push' ? otherWorkspaceName : workspaceName || 'this workspace', isLoading: enabled && mapping.isLoading, isError: mapping.isError, errorMessage: mapping.isError ? getErrorMessage(mapping.error, 'Failed to load mapping') : null, diff --git a/apps/sim/ee/workspace-forking/components/forks.tsx b/apps/sim/ee/workspace-forking/components/forks.tsx index 61026ca0ffb..7acc581eba9 100644 --- a/apps/sim/ee/workspace-forking/components/forks.tsx +++ b/apps/sim/ee/workspace-forking/components/forks.tsx @@ -85,6 +85,8 @@ function ForkListRow({ name, actions }: ForkListRowProps) { interface ForkSyncDetailViewProps { title: string workspaceId: string + /** This workspace's name — a pull overwrites it, and the copy has to say which side that is. */ + workspaceName?: string /** The other side of the edge being synced (this workspace's parent). */ otherWorkspaceId: string otherWorkspaceName: string @@ -105,6 +107,7 @@ interface ForkSyncDetailViewProps { function ForkSyncDetailView({ title, workspaceId, + workspaceName, otherWorkspaceId, otherWorkspaceName, onBack, @@ -118,6 +121,7 @@ function ForkSyncDetailView({ const controller = useForkSync({ workspaceId, + workspaceName, otherWorkspaceId, otherWorkspaceName, direction, @@ -186,11 +190,11 @@ function ForkSyncDetailView({ open={confirmSyncOpen} onOpenChange={setConfirmSyncOpen} srTitle='Sync workspace' - title='Overwrite target workspace' + title={`Overwrite ${targetWorkspaceName}`} text={[ - 'The target may have been modified since the last sync. Syncing will ', + 'Syncing will ', { text: 'overwrite any changes', bold: true }, - ' there. Continue?', + ` made in ${targetWorkspaceName} since the last sync. Continue?`, ]} confirm={{ label: 'Sync', @@ -431,6 +435,7 @@ export function Forks() { key={parent.id} title={parent.name} workspaceId={workspaceId} + workspaceName={workspaceName} otherWorkspaceId={parent.id} otherWorkspaceName={parent.name} onBack={() => void setSelectedForkId(null, { history: 'replace' })} From 3ff91f04392a157bed024a88e2d92001c00ea708 Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 15:28:02 -0700 Subject: [PATCH 06/22] improvement(docs): clean up leftovers from the code-block alignment PR (#6825) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * improvement(docs): clear leftovers from the reverted revisions A cleanup pass over the final state. Every finding was residue from an approach this PR tried and abandoned, or a claim that stopped being true when it did. - Delete the copy-button svg sizing rule: a later rule sets `display: none` on that same element ungated, so sizing it was never observable. Superseded by the mask approach. - Drop the paragraph in page.tsx arguing about a custom Shiki factory. The factory was deleted; nothing configures one now. - Correct shiki-curl-json.ts, which still claimed the grammar "reaches the client path too". It does not — that was the justification for choosing a grammar over a transformer, so leaving it stated the opposite of the truth. Now records where it applies, where it does not, and why not to retry. - Correct the global.css section header, which claimed the component owns the shell while the next rule defines it here. - Qualify the `--copy-glyph` declarations with `:has(> svg[class*="lucide"])`, which the group's own comment asserts of every rule in it. - Correct `getCode`'s TSDoc: the gutter is a `::before`, and pseudo-element content never reaches `textContent`, so line numbers were never what the clone guards. It guards transformer-emitted `.nd-copy-ignore` nodes. - Compose `chipGeometryClass` and emcn's `ChipChevronDown` in the API example selector instead of restating their literals. - Merge the duplicated `div[role="region"]` rule. The tablist pair stays split: biome's `noDuplicateProperties` reads a nested `@variant` setting the same property as a duplicate and fails the build — recorded so it is not remerged. - Note that fumadocs ships its own gutter for `lines`-meta fences, which cannot be suppressed from here and would paint a second column. * fix(docs): drop a highlighter registration that can never fire fumadocs-openapi calls `renderCodeBlock` with a hard-coded `"json"` from both of its call sites (`request-tabs.js:76`, `response-tabs.js:48`), so the docs `CodeBlock` it routes through never receives a shell language. The `getHighlighter('js', { langs: [curlJsonBodyGrammar] })` registering the shell-scoped JSON-body injection therefore did nothing but await on every API sample render, and the docblock claiming the grammar covers those samples was wrong. - Delete the call and its imports. - State the grammar's real coverage: prose fences only, via `langs`. Both API reference paths are unreachable — samples are JSON, and the cURL usage tabs highlight client-side off fumadocs' own factory. - Correct `code-block.tsx`'s TSDoc, which still said API samples come from fumadocs' own renderer. They come through this component; `UsageTab` is the renderer that bypasses it. - Re-home a comment orphaned when two CSS rules merged — it had drifted onto the rule below and read as documenting it. - Drop a `.nd-copy-ignore` claim about transformers emitting those nodes; nothing here does, and upstream parity is the reason the clone exists. --- apps/docs/app/[lang]/[[...slug]]/page.tsx | 12 +---- apps/docs/app/global.css | 50 ++++++++----------- .../components/ui/api-example-selector.tsx | 15 ++++-- apps/docs/components/ui/code-block.tsx | 26 ++++++---- apps/docs/lib/shiki-curl-json.ts | 16 ++++-- 5 files changed, 61 insertions(+), 58 deletions(-) diff --git a/apps/docs/app/[lang]/[[...slug]]/page.tsx b/apps/docs/app/[lang]/[[...slug]]/page.tsx index 7b3f114b592..87436d3f664 100644 --- a/apps/docs/app/[lang]/[[...slug]]/page.tsx +++ b/apps/docs/app/[lang]/[[...slug]]/page.tsx @@ -1,5 +1,5 @@ import type React from 'react' -import { getHighlighter, highlight } from 'fumadocs-core/highlight' +import { highlight } from 'fumadocs-core/highlight' import type { Root } from 'fumadocs-core/page-tree' import { findNeighbour } from 'fumadocs-core/page-tree' import type { ApiPageProps } from 'fumadocs-openapi/ui' @@ -18,7 +18,6 @@ import { Heading } from '@/components/ui/heading' import { ResponseSection } from '@/components/ui/response-section' import { i18n } from '@/lib/i18n' import { getApiSpecContent, getAuthenticatedCodeSamples, openapi } from '@/lib/openapi' -import { curlJsonBodyGrammar } from '@/lib/shiki-curl-json' import { simShikiOptions } from '@/lib/shiki-theme' import { type PageData, source } from '@/lib/source' import { DOCS_BASE_URL } from '@/lib/urls' @@ -77,17 +76,8 @@ function stripLocalePrefix(url: string, lang: string): string { * rather than fumadocs-openapi's built-in one, so those blocks get the emcn copy control * instead of fumadocs' lucide clipboard. Mirrors the default renderer — same `highlight` call, * same `Pre` component, same `my-0` — differing only in which shell wraps the result. - * - * One asymmetry: `highlight` resolves fumadocs' shared `defaultShikiFactory`, while the renderer - * this replaces uses whatever `shiki` factory the page was configured with. They are the same - * object because that factory is also the default; passing a custom one would be honored on API - * markdown and ignored here. */ async function ApiCodeBlock({ lang, code }: { lang: string; code: string }) { - // Registers the injection on the shared highlighter `highlight` resolves; an injection is a - // property of the highlighter, not a per-call option. Idempotent — already-loaded grammars are - // skipped. - await getHighlighter('js', { langs: [curlJsonBodyGrammar] }) return ( {await highlight(code, { lang, ...simShikiOptions, components: { pre: Pre } })} diff --git a/apps/docs/app/global.css b/apps/docs/app/global.css index 8d1dfc1277e..4e493df5ce9 100644 --- a/apps/docs/app/global.css +++ b/apps/docs/app/global.css @@ -1603,15 +1603,17 @@ main article tbody tr:last-child td { /* Code blocks — platform field chrome. - The shell itself (radius, border, fill) is owned by components/ui/code-block.tsx, the way - an emcn component owns its chrome. What remains here is the styling of fumadocs internals - that component cannot reach: the title row it renders from a `title` prop, the scroll - viewport, and the tab strip that `` puts above a fence. */ - -/* Local aliases, not new design values: each is an existing platform token whose light and - dark halves differ. Naming the pair once lets the rules below be written without a `.dark` - twin, which would otherwise have to restate their `:has()` selectors — the double-`:has()` - the tabbed group is matched by, and the sibling-combinator one the gutter is matched by. */ + Three renderers emit these figures and only two go through components/ui/code-block.tsx, so + the shell and the fumadocs internals that component cannot reach — the title row, the scroll + viewport, the tab strip `` puts above a fence — are all defined here. The component + keeps only the copy control and the prose margin. */ + +/* Local aliases, not new design values: each names an existing platform token pair whose light + and dark halves differ (`--surface-5`/`--code-bg`, `--text-muted`/`--code-line-number`). + + `--code-surface` has three consumers. `--code-gutter` has one, and is an alias anyway because + a `.dark` twin would have to restate the sibling-combinator `:has()` its consumer is matched + by — the expensive form, which re-checks on every line insertion. */ :root { --code-surface: var(--surface-5); --code-gutter: var(--text-muted); @@ -1689,10 +1691,9 @@ figure.shiki > div:first-child:has(figcaption) svg { The `!important` and the viewport selector are belt-and-braces, not strictly required — fumadocs' declaration is (0,2,0) and these selectors are (0,3,1) and (0,4,1), so they - already win, and nothing declares this property on the viewport. They are kept because - getting this wrong puts the line numbers on top of the code, which shipped once already, and - because the specificity of `:has()` and `:not()` is easy to miscount in exactly the - direction that reintroduces it. Remove them only alongside a visual check. + already win, and nothing declares this property on the viewport. They are kept because the + specificity of `:has()` and `:not()` is easy to miscount in the direction that puts the line + numbers on top of the code. Remove them only alongside a visual check. */ figure.shiki:has(.line ~ .line), figure.shiki:has(.line ~ .line) > div[role="region"], @@ -1733,6 +1734,9 @@ figure.shiki code:has(.line ~ .line) .line::before { the code surface. The viewport is the one box all three renderers agree on. */ figure.shiki > div[role="region"] { background-color: var(--code-surface); + /* fumadocs ships 14px of vertical padding, the platform's viewer 8px; 10px splits them and + keeps a single-line fence from looking hollow at the tighter 21px line box. */ + padding-block: 10px; } /* Shiki emits `--shiki-*-bg` custom properties under `defaultColor: false`; keep the `pre` clear @@ -1741,13 +1745,6 @@ figure.shiki pre { background-color: transparent; } -/* Viewport padding — fumadocs ships 14px vertical; the platform's viewer uses 8px. Split the - difference at 10px, which keeps a single-line fence from looking hollow at the tighter - 21px line box. */ -figure.shiki > div[role="region"] { - padding-block: 10px; -} - /* Untitled blocks float the copy control over the code, so the last column has to clear it: an 8px offset plus emcn's 20px icon button, with room to breathe. fumadocs reserves 32px, which the glyphs run into. @@ -1802,12 +1799,6 @@ figure.shiki button[aria-label="Copied Text"]:has(> svg[class*="lucide"]) { color 150ms; } -figure.shiki button[aria-label="Copy Text"] > svg[class*="lucide"], -figure.shiki button[aria-label="Copied Text"] > svg[class*="lucide"] { - width: 14px; - height: 14px; -} - figure.shiki button[aria-label="Copy Text"]:has(> svg[class*="lucide"]) { @variant hover-hover { background-color: var(--surface-active); @@ -1849,11 +1840,11 @@ figure.shiki button[aria-label$="Text"]:has(> svg[class*="lucide"])::before { -webkit-mask-size: contain; } -figure.shiki button[aria-label="Copy Text"] { +figure.shiki button[aria-label="Copy Text"]:has(> svg[class*="lucide"]) { --copy-glyph: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='-1 -2 24 24' fill='none' stroke='%23000' stroke-width='1.25' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='M14.25 0.75H2.75C1.64543 0.75 0.75 1.64543 0.75 2.75V14.25'/%3E%3Crect x='5.25' y='5.25' width='14' height='14' rx='2'/%3E%3C/svg%3E"); } -figure.shiki button[aria-label="Copied Text"] { +figure.shiki button[aria-label="Copied Text"]:has(> svg[class*="lucide"]) { --copy-glyph: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='-1 -2 24 24' fill='none' stroke='%23000' stroke-width='1.25' stroke-linecap='round' stroke-linejoin='round'%3E%3Cpath d='M18.25 2.75L7.25 15.75L1.75 10.25'/%3E%3C/svg%3E"); } @@ -1901,6 +1892,9 @@ div:has(> div > figure.shiki) > [role="tablist"] button { color 150ms; } +/* Kept as its own rule rather than nested into the block above: biome's + `noDuplicateProperties` reads a nested `@variant` setting the same property as a duplicate + declaration and fails the build. */ div:has(> div > figure.shiki) > [role="tablist"] button { @variant hover-hover { color: var(--text-body); diff --git a/apps/docs/components/ui/api-example-selector.tsx b/apps/docs/components/ui/api-example-selector.tsx index b59743385f5..7f517ceba88 100644 --- a/apps/docs/components/ui/api-example-selector.tsx +++ b/apps/docs/components/ui/api-example-selector.tsx @@ -2,8 +2,14 @@ import type { ComponentProps } from 'react' import { useId } from 'react' -import { chipFieldSurfaceClass, chipFieldTextClass, chipHoverSurfaceClass, cn } from '@sim/emcn' -import { ChevronDown } from '@sim/emcn/icons' +import { + ChipChevronDown, + chipFieldSurfaceClass, + chipFieldTextClass, + chipGeometryClass, + chipHoverSurfaceClass, + cn, +} from '@sim/emcn' import type { APIPageClientOptions } from 'fumadocs-openapi/ui/client' type FumadocsAPIExampleSelector = NonNullable< @@ -35,10 +41,11 @@ export function APIExampleSelector({ items, value, onValueChange }: APIExampleSe * screen-reader behavior a custom listbox would have to rebuild. */ className={cn( + chipGeometryClass, chipFieldSurfaceClass, chipFieldTextClass, chipHoverSurfaceClass, - 'h-[30px] w-full appearance-none ps-2 pe-8 text-left' + 'w-full appearance-none pe-8' )} > {items.map((item) => ( @@ -47,7 +54,7 @@ export function APIExampleSelector({ items, value, onValueChange }: APIExampleSe ))} - +
{selectedItem?.description && (

{selectedItem.description}

diff --git a/apps/docs/components/ui/code-block.tsx b/apps/docs/components/ui/code-block.tsx index f01416fd231..43239464dae 100644 --- a/apps/docs/components/ui/code-block.tsx +++ b/apps/docs/components/ui/code-block.tsx @@ -33,21 +33,24 @@ function CopyButton({ getCode }: { getCode: () => string }) { } /** - * Docs code block for prose fences, wired into the MDX `pre` mapping. + * Docs code block for prose fences and the API reference's request/response samples — the MDX + * `pre` mapping and fumadocs-openapi's `renderCodeBlock` both render it. * - * The shell — radius, hairline, fill — is not set here. Request and response samples in the - * API reference come from fumadocs-openapi's own renderer, so the two share chrome through a - * `figure.shiki` rule in `global.css` instead; see the note there. What stays here is the - * part only this path has: the copy control, and the `my-4` prose rhythm that API samples, - * which sit flush in their panel, must not inherit. + * The shell — radius, hairline, fill — is not set here. A third renderer, fumadocs-openapi's + * `UsageTab`, emits these figures without going through any component, so all three share + * chrome through a `figure.shiki` rule in `global.css` instead; see the note there. What stays + * here is the copy control, and the `my-4` prose rhythm that API samples, which sit flush in + * their panel, override with `my-0`. */ export function CodeBlock({ title, ...props }: React.ComponentProps) { const figureRef = useRef(null) /** - * Reads the block's text the way fumadocs does: from a clone, with `.nd-copy-ignore` nodes - * replaced by newlines. Those nodes carry rendered gutter and diff markers, so copying the - * live `textContent` would paste line numbers along with the code. + * Reads the block's text the way fumadocs' own `CopyButton` does: from a clone, with + * `.nd-copy-ignore` nodes replaced by newlines — kept in step with upstream so a fence that + * gains such a node copies the same text there and here. (The line-number gutter is a + * `::before`, and pseudo-element content never reaches `textContent`, so it is not what this + * guards.) */ function getCode() { const pre = figureRef.current?.getElementsByTagName('pre').item(0) @@ -65,8 +68,9 @@ export function CodeBlock({ title, ...props }: React.ComponentProps (
diff --git a/apps/docs/lib/shiki-curl-json.ts b/apps/docs/lib/shiki-curl-json.ts index 9cb2b6b0275..14ea4a52660 100644 --- a/apps/docs/lib/shiki-curl-json.ts +++ b/apps/docs/lib/shiki-curl-json.ts @@ -14,10 +14,18 @@ import type { LanguageRegistration } from 'shiki' * when it owns the opening brace. Entering mid-string, keys keep `string.quoted.double.json` * and stay string-colored, which is the entire difference this exists to remove. Hence the * hand-written patterns below, which name that scope directly. - * - **A Shiki transformer.** A transformer can re-tokenize the body correctly, but it is a - * function, and the API reference's request tabs highlight in the browser off a `shikiOptions` - * object passed through RSC — where "Functions cannot be passed directly to Client - * Components". A grammar is plain data, so it reaches the client path too. + * - **A Shiki transformer.** A transformer re-tokenizes the body correctly, but it is a function, + * and `shikiOptions` is forwarded into a client component — "Functions cannot be passed + * directly to Client Components" takes down every API reference page. A grammar is plain data, + * so it survives that boundary. + * + * Applies to prose fences only, via `langs` on the MDX pipeline. Not the API reference: + * fumadocs-openapi calls `renderCodeBlock` with a hard-coded `"json"` for request and response + * samples, so a shell injection can never fire there, and its cURL usage tabs highlight in the + * browser off fumadocs' own factory — `ClientCodeBlockProvider` sits in a `"use client"` module + * the package does not expose through its `exports` map, so reaching it means importing + * `fumadocs-openapi/ui/base` from client code and dragging `remark` and + * `@fumari/json-schema-ts` into the browser bundle. That broke the deployment once. * * The opening brace requires a `}`, a quoted key, or end-of-line after it. That is what keeps * `awk '{print $1}'` out, while still matching a body whose brace ends the line — Oniguruma From 19230bfcd78115665246e3aff3144fc94a86abe1 Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 15:30:41 -0700 Subject: [PATCH 07/22] feat(connectors): hand source files to the document pipeline instead of extracting them (#6821) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(connectors): hand source files to the document pipeline instead of extracting them 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. * fix(knowledge): guard every parser against empty output, not just the file parsers 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. * fix(knowledge): validate the stored artifact against the parser registry 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. * fix(knowledge): carry the MIME type through hydration 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. --- 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 +++++------- .../knowledge/connectors/sync-engine.test.ts | 75 ++++++++ .../lib/knowledge/connectors/sync-engine.ts | 163 +++++++++++++----- ...cument-processor-secret-provenance.test.ts | 1 + .../knowledge/documents/document-processor.ts | 40 ++++- .../knowledge/documents/parser-extension.ts | 9 +- .../stored-artifact-extension.test.ts | 15 ++ .../documents/unreadable-document.test.ts | 81 +++++++++ 13 files changed, 474 insertions(+), 293 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.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 dcbc354173b..14e3bb7f13a 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -66,17 +66,56 @@ function sanitizeStorageTitle(title: string): string { } /** - * Name a connector document's stored object carries. + * Sanitizes a source file's name for a storage key, keeping its extension. * - * 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. + * `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 connectorArtifactFileName(title: string): string { - return `${sanitizeStorageTitle(title)}.txt` +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. + * + * 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 connectorStoredArtifact(extDoc: ExternalDocument): { + bytes: Buffer + fileName: string + mimeType: string +} { + if (extDoc.sourceFile) { + return { + bytes: extDoc.sourceFile.bytes, + fileName: sanitizeStorageFileName(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 +147,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 +172,42 @@ 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 +} + /** 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 +1093,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. @@ -1040,18 +1118,7 @@ export async function executeSync( result.docsUnchanged++ return null } - return { - ...op, - extDoc: { - ...op.extDoc, - title: fullDoc.title || op.extDoc.title, - content: fullDoc.content, - contentHash: hydratedHash, - contentDeferred: false, - sourceUrl: fullDoc.sourceUrl ?? op.extDoc.sourceUrl, - metadata: { ...op.extDoc.metadata, ...fullDoc.metadata }, - }, - } + return { ...op, extDoc: mergeHydratedDocument(op.extDoc, fullDoc, hydratedHash) } }) ) @@ -1670,18 +1737,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 +1769,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 +1796,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 +1823,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 +1855,11 @@ 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 +1918,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-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/document-processor.ts b/apps/sim/lib/knowledge/documents/document-processor.ts index baa36447948..542b84af54c 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 { @@ -199,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 ?? {} @@ -782,6 +799,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,10 +841,6 @@ async function parseWithFileParser( ) } - if (!content.trim()) { - throw new Error('File parser returned empty content') - } - 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/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 new file mode 100644 index 00000000000..84b880902fb --- /dev/null +++ b/apps/sim/lib/knowledge/documents/unreadable-document.test.ts @@ -0,0 +1,81 @@ +/** + * @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, + 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' + +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/) + }) + + /** + * 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', + metadata: { extractionMethod: 'mammoth' }, + }) + + const result = await parse('SOP.docx') + + expect(result.chunks.length).toBeGreaterThan(0) + }) +}) From 374c1064ea3afd0c6940211014540e5de3d166ab Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 15:33:08 -0700 Subject: [PATCH 08/22] chore(cli): release 2.1.0 (#6826) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The publish workflow derives the npm version from this field, and on `main` it uses it verbatim: `2.0.0` was already on the registry, so the release step skipped and every change since then stayed unpublished. Staging and dev never showed it because they append `-preview.N` and `-dev.N`, which are always new. So the merged CLI work — the redirect refusal, the default endpoint, folder path encoding, the request bound, the User-Agent, `run --follow`, `runs wait` and `logs follow` — is on `main` but not on npm, and `sim@latest` is still the build whose every write is dropped by the apex redirect. Minor rather than patch: three commands are new and the default endpoint changed. --- packages/sim-cli/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sim-cli/package.json b/packages/sim-cli/package.json index 856473edcee..4d7d0a760e0 100644 --- a/packages/sim-cli/package.json +++ b/packages/sim-cli/package.json @@ -1,6 +1,6 @@ { "name": "sim", - "version": "2.0.0", + "version": "2.1.0", "description": "Sim CLI - talk to the Sim API from your terminal", "type": "module", "bin": { From 1e2572dd8a96f76846e30c762176a72a2f81e85c Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 15:38:03 -0700 Subject: [PATCH 09/22] fix(desktop): guarantee the update feed resolves the newest release (#6827) The stable channel reads a GitHub release list it shares with web-app releases, SDK tags, and legacy prereleases, but only ever looked at the first 30 entries. Once enough unrelated releases stack on top, the feed 404s and every stable shell silently stops updating. Walk pages (100 per page, up to 5) until one yields a release for the channel, and fail the feed rather than serving an older build when a page cannot be read. Also point the update gate's manual download at a new /api/desktop/update/download redirect, which resolves through the same channel selection. It previously opened GitHub's repository-wide latest release, which can be a tag carrying no desktop artifact at all. --- apps/sim/app/_shell/desktop-update-gate.tsx | 17 ++- .../api/desktop/update/download/route.test.ts | 111 ++++++++++++++++++ .../app/api/desktop/update/download/route.ts | 77 ++++++++++++ .../update/latest-mac.yml/route.test.ts | 61 +++++++++- .../desktop/update/latest-mac.yml/route.ts | 38 +++--- apps/sim/lib/desktop/update-feed.ts | 64 ++++++++++ scripts/check-api-validation-contracts.ts | 7 +- 7 files changed, 354 insertions(+), 21 deletions(-) create mode 100644 apps/sim/app/api/desktop/update/download/route.test.ts create mode 100644 apps/sim/app/api/desktop/update/download/route.ts diff --git a/apps/sim/app/_shell/desktop-update-gate.tsx b/apps/sim/app/_shell/desktop-update-gate.tsx index 8a622f5c188..f9c114cfc43 100644 --- a/apps/sim/app/_shell/desktop-update-gate.tsx +++ b/apps/sim/app/_shell/desktop-update-gate.tsx @@ -6,8 +6,23 @@ import { Button, useNativeSurfaceOcclusionReady } from '@sim/emcn' import { getDesktopBridge, getDesktopShellVersion, getDesktopUpdates } from '@/lib/desktop' import { isShellOutdated } from '@/lib/desktop/min-version' +/** + * Resolves this deployment's channel to the newest release's installer, so a + * manual download lands on the same build the updater would have installed. + */ +const DOWNLOAD_REDIRECT_PATH = '/api/desktop/update/download' + const DOWNLOAD_FALLBACK_URL = 'https://github.com/simstudioai/sim/releases/latest' +function manualDownloadUrl(): string { + const origin = window.location.origin + // openExternal only accepts https, so an http self-hosted origin cannot + // serve the redirect to the system browser. + return origin.startsWith('https://') + ? `${origin}${DOWNLOAD_REDIRECT_PATH}` + : DOWNLOAD_FALLBACK_URL +} + interface GateAction { label: string disabled?: boolean @@ -21,7 +36,7 @@ function gateActionFor(state: DesktopUpdateState): GateAction { // background; the button covers the manual path. return { label: 'Get the latest version', - onClick: () => void getDesktopBridge()?.openExternal(DOWNLOAD_FALLBACK_URL), + onClick: () => void getDesktopBridge()?.openExternal(manualDownloadUrl()), } } switch (state.status) { diff --git a/apps/sim/app/api/desktop/update/download/route.test.ts b/apps/sim/app/api/desktop/update/download/route.test.ts new file mode 100644 index 00000000000..c85a16cce05 --- /dev/null +++ b/apps/sim/app/api/desktop/update/download/route.test.ts @@ -0,0 +1,111 @@ +/** + * @vitest-environment node + */ +import { setEnv } from '@sim/testing' +import { NextRequest } from 'next/server' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + DESKTOP_PRERELEASE_REPOSITORY, + DESKTOP_STABLE_RELEASE_REPOSITORY, + MANIFEST_ASSET_NAME, + releasesApiUrl, +} from '@/lib/desktop/update-feed' +import { GET } from '@/app/api/desktop/update/download/route' + +const STABLE_RELEASES_URL = releasesApiUrl(DESKTOP_STABLE_RELEASE_REPOSITORY, 1) +const PRERELEASE_RELEASES_URL = releasesApiUrl(DESKTOP_PRERELEASE_REPOSITORY, 1) + +function release(tag: string, repository: string) { + const base = `https://github.com/${repository}/releases/download/${tag}` + const version = tag.replace(/^v/, '') + return { + tag_name: tag, + draft: false, + prerelease: tag.includes('-'), + assets: [ + { name: MANIFEST_ASSET_NAME, browser_download_url: `${base}/${MANIFEST_ASSET_NAME}` }, + { + name: `Sim-${version}-universal.zip`, + browser_download_url: `${base}/Sim-${version}-universal.zip`, + }, + { + name: `Sim-${version}-universal.dmg`, + browser_download_url: `${base}/Sim-${version}-universal.dmg`, + }, + ], + } +} + +async function getDownload(): Promise { + return GET(new NextRequest('https://www.sim.ai/api/desktop/update/download'), undefined) +} + +describe('desktop update download route', () => { + const fetchMock = vi.fn() + + beforeEach(() => { + fetchMock.mockReset() + vi.stubGlobal('fetch', fetchMock) + setEnv({ APPCONFIG_ENVIRONMENT: undefined }) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('redirects to the newest stable installer', async () => { + fetchMock.mockResolvedValueOnce( + Response.json([ + release('v1.1.0', DESKTOP_STABLE_RELEASE_REPOSITORY), + release('v1.3.0', DESKTOP_STABLE_RELEASE_REPOSITORY), + release('v1.2.0', DESKTOP_STABLE_RELEASE_REPOSITORY), + ]) + ) + + const response = await getDownload() + + expect(response.status).toBe(302) + expect(response.headers.get('location')).toBe( + `https://github.com/${DESKTOP_STABLE_RELEASE_REPOSITORY}/releases/download/v1.3.0/Sim-1.3.0-universal.dmg` + ) + expect(fetchMock).toHaveBeenCalledWith(STABLE_RELEASES_URL, expect.any(Object)) + }) + + it('serves its own deployment channel rather than the stable stream', async () => { + setEnv({ APPCONFIG_ENVIRONMENT: 'dev' }) + fetchMock.mockResolvedValueOnce( + Response.json([ + release('v1.3.0-dev.4', DESKTOP_PRERELEASE_REPOSITORY), + release('v1.4.0-staging.1', DESKTOP_PRERELEASE_REPOSITORY), + ]) + ) + + const response = await getDownload() + + expect(response.status).toBe(302) + expect(response.headers.get('location')).toContain('Sim-1.3.0-dev.4-universal.dmg') + expect(fetchMock).toHaveBeenCalledWith(PRERELEASE_RELEASES_URL, expect.any(Object)) + }) + + it('reports no release when the channel has none', async () => { + fetchMock.mockResolvedValueOnce( + Response.json([release('v1.3.0-dev.4', DESKTOP_PRERELEASE_REPOSITORY)]) + ) + + const response = await getDownload() + + expect(response.status).toBe(404) + expect(await response.json()).toMatchObject({ + error: 'No desktop release for channel latest', + }) + }) + + it('surfaces an unreadable release list instead of redirecting', async () => { + fetchMock.mockResolvedValueOnce(new Response(null, { status: 500 })) + + const response = await getDownload() + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release feed unavailable' }) + }) +}) diff --git a/apps/sim/app/api/desktop/update/download/route.ts b/apps/sim/app/api/desktop/update/download/route.ts new file mode 100644 index 00000000000..520e4761452 --- /dev/null +++ b/apps/sim/app/api/desktop/update/download/route.ts @@ -0,0 +1,77 @@ +import { createLogger } from '@sim/logger' +import { type NextRequest, NextResponse } from 'next/server' +import { env } from '@/lib/core/config/env' +import { withRouteHandler } from '@/lib/core/utils/with-route-handler' +import { + channelForDeploymentEnvironment, + type DesktopReleaseCandidate, + releaseRepositoryForChannel, + releasesApiUrl, + resolveLatestRelease, + selectInstallerAsset, +} from '@/lib/desktop/update-feed' + +const logger = createLogger('DesktopUpdateDownloadAPI') + +/** Matches the manifest feed so both paths resolve the same release. */ +const REVALIDATE_SECONDS = 300 + +/** + * Redirects to the installer for the newest release of this deployment's + * channel (see `lib/desktop/update-feed.ts`). + * + * This is the manual escape hatch behind the blocking update gate: shells too + * old to expose the updater bridge send the user here instead of + * self-updating. It resolves through the same channel selection as the + * manifest feed, so a manual download lands on exactly the build + * electron-updater would have installed — never an intermediate version, and + * never a repository release carrying no desktop artifact. + * + * Public by the same reasoning as the manifest feed: it only points at public + * GitHub release assets. + */ +export const GET = withRouteHandler(async (_request: NextRequest): Promise => { + const channel = channelForDeploymentEnvironment(env.APPCONFIG_ENVIRONMENT) + const releaseRepository = releaseRepositoryForChannel(channel) + + const githubToken = process.env.GITHUB_TOKEN + const resolved = await resolveLatestRelease(channel, async (page) => { + const response = await fetch(releasesApiUrl(releaseRepository, page), { + headers: { + accept: 'application/vnd.github+json', + ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), + }, + next: { revalidate: REVALIDATE_SECONDS }, + }) + if (!response.ok) { + logger.error('GitHub releases lookup failed', { + status: response.status, + page, + channel, + releaseRepository, + }) + return null + } + return (await response.json()) as DesktopReleaseCandidate[] + }) + if ('error' in resolved) { + return NextResponse.json({ error: 'Release feed unavailable' }, { status: 502 }) + } + + const release = resolved.release + const asset = release ? selectInstallerAsset(release) : null + if (!release || !asset) { + if (release) { + logger.error('Release has no installer artifact', { tag: release.tag_name, channel }) + } + return NextResponse.json( + { error: `No desktop release for channel ${channel}` }, + { status: 404 } + ) + } + + return NextResponse.redirect(asset.browser_download_url, { + status: 302, + headers: { 'cache-control': `public, max-age=${REVALIDATE_SECONDS}` }, + }) +}) diff --git a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts index 754b5fff8b4..256551d910a 100644 --- a/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts +++ b/apps/sim/app/api/desktop/update/latest-mac.yml/route.test.ts @@ -6,13 +6,15 @@ import { NextRequest } from 'next/server' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { DESKTOP_PRERELEASE_REPOSITORY, + DESKTOP_RELEASES_PAGE_SIZE, DESKTOP_STABLE_RELEASE_REPOSITORY, MANIFEST_ASSET_NAME, + releasesApiUrl, } from '@/lib/desktop/update-feed' import { GET } from '@/app/api/desktop/update/latest-mac.yml/route' -const STABLE_RELEASES_URL = `https://api.github.com/repos/${DESKTOP_STABLE_RELEASE_REPOSITORY}/releases?per_page=30` -const PRERELEASE_RELEASES_URL = `https://api.github.com/repos/${DESKTOP_PRERELEASE_REPOSITORY}/releases?per_page=30` +const STABLE_RELEASES_URL = releasesApiUrl(DESKTOP_STABLE_RELEASE_REPOSITORY, 1) +const PRERELEASE_RELEASES_URL = releasesApiUrl(DESKTOP_PRERELEASE_REPOSITORY, 1) const FEED_STATUS_HEADER = 'x-sim-desktop-update-feed' function release(tag: string) { @@ -156,6 +158,61 @@ describe('desktop update manifest route', () => { expect(fetchMock).toHaveBeenCalledTimes(1) }) + it('walks past a page of unrelated releases to reach the newest desktop build', async () => { + const filler = Array.from({ length: DESKTOP_RELEASES_PAGE_SIZE }, (_, index) => ({ + tag_name: `python-sdk-v0.${index}.0`, + draft: false, + prerelease: false, + assets: [], + })) + fetchMock.mockImplementation(async (input: string | URL | Request) => { + const url = String(input) + if (url === releasesApiUrl(DESKTOP_STABLE_RELEASE_REPOSITORY, 1)) { + return Response.json(filler) + } + if (url === releasesApiUrl(DESKTOP_STABLE_RELEASE_REPOSITORY, 2)) { + return Response.json([release('v1.1.0')]) + } + if (url === `https://downloads.example/v1.1.0/${MANIFEST_ASSET_NAME}`) { + return new Response(manifest('1.1.0')) + } + return new Response(null, { status: 404 }) + }) + + const response = await getFeed('www.sim.ai') + + expect(response.status).toBe(200) + expect(await response.text()).toContain('version: 1.1.0') + }) + + it('stops walking at a short page rather than requesting empty ones', async () => { + fetchMock.mockResolvedValueOnce( + Response.json([release('v1.2.0-dev.4'), release('v1.2.0-staging.5')]) + ) + + const response = await getFeed('www.sim.ai') + + expect(response.status).toBe(404) + expect(fetchMock).toHaveBeenCalledTimes(1) + }) + + it('fails the feed instead of serving an older release when a page cannot be read', async () => { + const filler = Array.from({ length: DESKTOP_RELEASES_PAGE_SIZE }, (_, index) => ({ + tag_name: `python-sdk-v0.${index}.0`, + draft: false, + prerelease: false, + assets: [], + })) + fetchMock + .mockResolvedValueOnce(Response.json(filler)) + .mockResolvedValueOnce(new Response(null, { status: 500 })) + + const response = await getFeed('www.sim.ai') + + expect(response.status).toBe(502) + expect(await response.json()).toMatchObject({ error: 'Release feed unavailable' }) + }) + it('rejects a manifest whose version does not match its selected release', async () => { setEnv({ APPCONFIG_ENVIRONMENT: 'dev' }) fetchMock diff --git a/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts b/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts index a731ba3be6f..6c443caa43e 100644 --- a/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts +++ b/apps/sim/app/api/desktop/update/latest-mac.yml/route.ts @@ -7,8 +7,9 @@ import { type DesktopReleaseCandidate, MANIFEST_ASSET_NAME, releaseRepositoryForChannel, + releasesApiUrl, + resolveLatestRelease, rewriteManifestUrls, - selectReleaseForChannel, } from '@/lib/desktop/update-feed' const logger = createLogger('DesktopUpdateFeedAPI') @@ -37,29 +38,34 @@ export const GET = withRouteHandler(async (_request: NextRequest): Promise { + const response = await fetch(releasesApiUrl(releaseRepository, page), { + headers: { + accept: 'application/vnd.github+json', + ...(githubToken ? { authorization: `Bearer ${githubToken}` } : {}), + }, + next: { revalidate: REVALIDATE_SECONDS }, }) + if (!response.ok) { + logger.error('GitHub releases lookup failed', { + status: response.status, + page, + channel, + releaseRepository, + }) + return null + } + return (await response.json()) as DesktopReleaseCandidate[] + }) + if ('error' in resolved) { return NextResponse.json({ error: 'Release feed unavailable' }, { status: 502 }) } - const releases = (await releasesResponse.json()) as DesktopReleaseCandidate[] - const release = selectReleaseForChannel(releases, channel) + const release = resolved.release if (!release) { return NextResponse.json( { error: `No desktop release for channel ${channel}` }, diff --git a/apps/sim/lib/desktop/update-feed.ts b/apps/sim/lib/desktop/update-feed.ts index b3b59f8be17..ed8d14c16f3 100644 --- a/apps/sim/lib/desktop/update-feed.ts +++ b/apps/sim/lib/desktop/update-feed.ts @@ -134,3 +134,67 @@ export function rewriteManifestUrls( return `${prefix}${base}${encodeURIComponent(value)}` }) } + +/** + * GitHub's maximum page size for the releases API. The stable channel reads a + * release list it shares with web-app releases, SDK releases, and legacy + * prereleases, so the window has to be wide enough that desktop releases are + * never pushed out of it. + */ +export const DESKTOP_RELEASES_PAGE_SIZE = 100 + +/** + * How far back the resolver walks before giving up. A channel whose newest + * release is buried deeper than this is already unreachable to its clients, + * and an unbounded walk would let an unrelated tag family stall the feed. + */ +export const MAX_DESKTOP_RELEASE_PAGES = 5 + +/** One page of the GitHub releases API, newest release first. */ +export function releasesApiUrl(repository: DesktopReleaseRepository, page: number): string { + return `https://api.github.com/repos/${repository}/releases?per_page=${DESKTOP_RELEASES_PAGE_SIZE}&page=${page}` +} + +/** + * The newest release of a channel, walking pages until one yields a match. + * + * GitHub returns releases newest-first, so the first page containing any + * release of the channel also contains its newest one — every later page is + * strictly older. The walk exists only so unrelated releases stacked on top + * (other tag families, other channels) cannot push a channel's newest build + * out of the window and take the whole channel's updates down. + * + * `fetchPage` returns null when the page could not be read; the resolver + * surfaces that as a failure rather than silently serving an older release. + */ +export async function resolveLatestRelease( + channel: DesktopUpdateChannel, + fetchPage: (page: number) => Promise +): Promise<{ release: DesktopReleaseCandidate | null } | { error: 'fetch-failed' }> { + for (let page = 1; page <= MAX_DESKTOP_RELEASE_PAGES; page++) { + const releases = await fetchPage(page) + if (releases === null) return { error: 'fetch-failed' } + const release = selectReleaseForChannel(releases, channel) + if (release) return { release } + // A short page is the end of the list; nothing older remains to walk. + if (releases.length < DESKTOP_RELEASES_PAGE_SIZE) break + } + return { release: null } +} + +/** + * The human-installable artifact of a release, preferred over the zip the + * updater consumes. Selected per-release rather than through GitHub's + * repository-wide "latest release", which the stable repository shares with + * web-app and SDK tags that carry no desktop artifact at all. + */ +export function selectInstallerAsset( + release: DesktopReleaseCandidate +): { name: string; browser_download_url: string } | null { + const assets = release.assets ?? [] + return ( + assets.find((asset) => asset.name.endsWith('.dmg')) ?? + assets.find((asset) => asset.name.endsWith('.zip')) ?? + null + ) +} diff --git a/scripts/check-api-validation-contracts.ts b/scripts/check-api-validation-contracts.ts index ee60c365129..e19d7138b6d 100644 --- a/scripts/check-api-validation-contracts.ts +++ b/scripts/check-api-validation-contracts.ts @@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries') const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors') const BASELINE = { - totalRoutes: 1120, - zodRoutes: 1120, + totalRoutes: 1121, + zodRoutes: 1121, nonZodRoutes: 0, } as const @@ -42,6 +42,9 @@ const INDIRECT_ZOD_ROUTES = new Set([ // Public updater feed: input-less GET, session-less, returns YAML (not JSON), // so it can't be JSON-contract-bound. Wrapped in withRouteHandler. 'apps/sim/app/api/desktop/update/latest-mac.yml/route.ts', + // Public updater download redirect: input-less GET, session-less, whose only + // response is a 302 to a GitHub release asset. Wrapped in withRouteHandler. + 'apps/sim/app/api/desktop/update/download/route.ts', 'apps/sim/app/api/invitations/route.ts', 'apps/sim/app/api/logs/export/route.ts', 'apps/sim/app/api/tools/docusign/route.ts', From 0c09f1de7e1a44fb5f4f61ca7c49293bb68c7f9c Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 15:41:58 -0700 Subject: [PATCH 10/22] improvement(logs-block): filter runs by trigger type (#6824) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * improvement(logs-block): filter runs by trigger type The Logs block could filter runs by workflow, status, time, cost, and duration, but not by how the run started — even though the underlying tool, the /api/logs contract, and the indexed trigger column all already accepted a comma-separated triggers filter. Adds a basic multi-select and an advanced free-text field behind the canonical `triggers` param, mirroring the block's existing workflow filter. Options come from the same registry the Logs page reads, so both surfaces name a run's origin identically; values sharing a label are merged into one option. Leaving the filter empty omits the param, so existing blocks query exactly as before. * fix(logs-block): declare the triggers input as the string it becomes The generic handler JSON.parses any post-transform input declared 'array' or 'json'. Since `joinIds` has already turned the selection into a comma-separated string by then, the array declaration logged a parse warning on every run, and JSON-looking advanced input would have been turned into an array the tool does not accept. Matches the legacy Logs block, which already declares triggers as a string, and locks the invariant with a test. Also drops `any` from the new test helper. * fix(logs-block): trim each entry when joining filter ids joinIds trimmed only the ends of an advanced-mode string, so a hand-typed 'api, schedule, slack' reached the query as ' schedule' and ' slack'. The filters split on commas without trimming, so those tokens matched no stored trigger and the filter silently returned nothing. Splits and trims every entry instead, which also covers empty tokens from a trailing comma and the multi-value ids behind merged trigger labels. * test(logs-block): lock the shared joinIds output for existing filters joinIds is shared with the workflow and status filters, so the per-entry trimming added for hand-typed triggers must not move their output. Covers every value a stored multi-select or advanced field can hold, plus the block-saved-before-the-filter case where triggers must not reach the query. * test(logs-block): exercise the trigger options against the real registry The fetcher reaches the block and trigger registries through a lazy import to avoid an initialization cycle, so a mocked test cannot show that the import resolves or that the registry is populated when the dropdown asks. Covers the populated list, unique labels, and the merged Sim agent option. --- apps/sim/blocks/blocks/logs.test.ts | 125 ++++++++++++++++++ apps/sim/blocks/blocks/logs.ts | 58 ++++++-- .../lib/workflows/subblocks/options.test.ts | 43 +++++- apps/sim/lib/workflows/subblocks/options.ts | 27 ++++ .../subblocks/trigger-options-live.test.ts | 30 +++++ 5 files changed, 274 insertions(+), 9 deletions(-) create mode 100644 apps/sim/blocks/blocks/logs.test.ts create mode 100644 apps/sim/lib/workflows/subblocks/trigger-options-live.test.ts diff --git a/apps/sim/blocks/blocks/logs.test.ts b/apps/sim/blocks/blocks/logs.test.ts new file mode 100644 index 00000000000..58b1734845b --- /dev/null +++ b/apps/sim/blocks/blocks/logs.test.ts @@ -0,0 +1,125 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it, vi } from 'vitest' + +vi.mock('@/lib/workflows/subblocks/options', () => ({ + fetchTriggerTypeOptions: vi.fn(), + fetchWorkspaceWorkflowOptions: vi.fn(), +})) + +import { LogsV2Block } from '@/blocks/blocks/logs' + +function buildQueryParams(params: Record) { + return LogsV2Block.tools.config!.params!({ operation: 'query', ...params }) +} + +describe('LogsV2Block trigger filter', () => { + it('omits triggers when the filter is untouched, leaving pre-existing queries unfiltered', () => { + expect(buildQueryParams({}).triggers).toBeUndefined() + expect(buildQueryParams({ triggers: [] }).triggers).toBeUndefined() + expect(buildQueryParams({ triggers: '' }).triggers).toBeUndefined() + }) + + it('joins a multi-select selection into the comma-separated list the API expects', () => { + expect(buildQueryParams({ triggers: ['api', 'schedule'] }).triggers).toBe('api,schedule') + }) + + it('flattens a merged option id so one label can select several trigger values', () => { + expect(buildQueryParams({ triggers: ['api', 'copilot,mothership'] }).triggers).toBe( + 'api,copilot,mothership' + ) + }) + + it('accepts an advanced-mode string of provider ids', () => { + expect(buildQueryParams({ triggers: ' slack,gmail ' }).triggers).toBe('slack,gmail') + }) + + it('trims each hand-typed entry, not just the ends of the string', () => { + // The filters split on commas without trimming, so a surviving space would + // match no stored trigger and silently narrow the result set to nothing. + expect(buildQueryParams({ triggers: 'api, schedule, slack' }).triggers).toBe( + 'api,schedule,slack' + ) + expect(buildQueryParams({ triggers: 'api,,schedule,' }).triggers).toBe('api,schedule') + expect(buildQueryParams({ triggers: ' , ' }).triggers).toBeUndefined() + }) + + it('trims entries inside a multi-select selection too', () => { + expect(buildQueryParams({ triggers: ['api ', ' copilot, mothership'] }).triggers).toBe( + 'api,copilot,mothership' + ) + }) + + it('never sends triggers on the run-details operation', () => { + expect( + LogsV2Block.tools.config!.params!({ + operation: 'get_run_details', + runId: 'run-1', + triggers: ['api'], + }) + ).toEqual({ runId: 'run-1' }) + }) +}) + +describe('LogsV2Block backwards compatibility', () => { + // `joinIds` is shared with the pre-existing workflow and status filters, so the + // per-entry trimming added for hand-typed triggers must not move their output. + // Every value a stored multi-select or advanced field can hold is listed here: + // option ids and workflow ids contain neither spaces nor commas. + const UUID = '3f2504e0-4f89-11d3-9a0c-0305e82c3301' + + it.each([ + ['unset', undefined, undefined], + ['empty selection', [], undefined], + ['one workflow', [UUID], UUID], + [ + 'two workflows', + [UUID, 'b7a1c2d3-0000-4000-8000-000000000001'], + `${UUID},b7a1c2d3-0000-4000-8000-000000000001`, + ], + ['advanced string', 'id-one,id-two', 'id-one,id-two'], + ['empty string', '', undefined], + ])('leaves workflowIds untouched for %s', (_name, value, expected) => { + expect(buildQueryParams({ workflowIds: value }).workflowIds).toBe(expected) + }) + + it.each([ + ['unset', undefined, undefined], + ['empty selection', [], undefined], + ['one status', ['info'], 'info'], + ['several statuses', ['info', 'error', 'cancelled'], 'info,error,cancelled'], + ])('leaves level untouched for %s', (_name, value, expected) => { + expect(buildQueryParams({ level: value }).level).toBe(expected) + }) + + it('omits triggers entirely for a block saved before the filter existed', () => { + const params = buildQueryParams({ workflowIds: [UUID], level: ['info'] }) + expect(params.triggers).toBeUndefined() + // Undefined values are dropped on serialization, so nothing reaches the query. + expect(JSON.stringify(params)).not.toContain('triggers') + }) +}) + +describe('LogsV2Block trigger subblocks', () => { + const subBlockIds = LogsV2Block.subBlocks.map((subBlock) => subBlock.id) + + it('declares triggers as the string it is transformed into', () => { + // The generic handler JSON.parses any post-transform input declared 'array' or + // 'json', so declaring the joined string as an array would warn on every run + // and would turn JSON-looking advanced input into an array the tool rejects. + expect(LogsV2Block.inputs.triggers.type).toBe('string') + expect(typeof buildQueryParams({ triggers: ['api', 'schedule'] }).triggers).toBe('string') + }) + + it('exposes basic and advanced modes behind one canonical param', () => { + expect(subBlockIds).toContain('triggerSelector') + expect(subBlockIds).toContain('manualTriggers') + + for (const id of ['triggerSelector', 'manualTriggers']) { + const subBlock = LogsV2Block.subBlocks.find((candidate) => candidate.id === id) + expect(subBlock?.canonicalParamId).toBe('triggers') + expect(subBlock?.condition).toEqual({ field: 'operation', value: 'query' }) + } + }) +}) diff --git a/apps/sim/blocks/blocks/logs.ts b/apps/sim/blocks/blocks/logs.ts index bef6bd2238d..a01d3535ec0 100644 --- a/apps/sim/blocks/blocks/logs.ts +++ b/apps/sim/blocks/blocks/logs.ts @@ -1,5 +1,8 @@ import { Library } from '@sim/emcn/icons' -import { fetchWorkspaceWorkflowOptions } from '@/lib/workflows/subblocks/options' +import { + fetchTriggerTypeOptions, + fetchWorkspaceWorkflowOptions, +} from '@/lib/workflows/subblocks/options' import type { BlockConfig } from '@/blocks/types' export const LogsBlock: BlockConfig = { @@ -298,19 +301,30 @@ const TIME_RANGE_MS: Record = { 'past-30-days': 30 * 24 * 60 * 60 * 1000, } -/** Normalizes multi-select arrays or comma strings into a comma-separated string. */ +/** + * Normalizes multi-select arrays or comma strings into a comma-separated string. + * + * Every entry is itself split on commas and trimmed: a single option id can hold + * several values (the merged trigger labels), and advanced-mode fields are typed + * by hand. The filters this feeds split on commas without trimming, so a stray + * space would silently match nothing. + */ function joinIds(value: unknown): string | undefined { - if (Array.isArray(value)) { - const ids = value.filter((id): id is string => typeof id === 'string' && id.length > 0) - return ids.length > 0 ? ids.join(',') : undefined - } - if (typeof value === 'string' && value.trim().length > 0) return value.trim() - return undefined + const entries = Array.isArray(value) ? value : [value] + const ids = entries + .filter((entry): entry is string => typeof entry === 'string') + .flatMap((entry) => entry.split(',')) + .map((id) => id.trim()) + .filter((id) => id.length > 0) + return ids.length > 0 ? ids.join(',') : undefined } /** Workflow filter, whichever mode the card is in. */ const WORKFLOW_FIELD = ['workflowSelector', 'manualWorkflowIds'] as const +/** Trigger filter, whichever mode the card is in. */ +const TRIGGER_FIELD = ['triggerSelector', 'manualTriggers'] as const + export const LogsV2Block: BlockConfig = { type: 'logs_v2', name: 'Logs', @@ -335,6 +349,7 @@ export const LogsV2Block: BlockConfig = { 'Query workflow runs', { text: 'for', field: WORKFLOW_FIELD }, { text: ', with status', field: 'level' }, + { text: ', triggered by', field: TRIGGER_FIELD }, { text: ', over', field: 'timeRange' }, ], get_run_details: [{ text: 'Read the trace for run', field: 'runId', core: true }], @@ -389,6 +404,28 @@ export const LogsV2Block: BlockConfig = { placeholder: 'All statuses', condition: { field: 'operation', value: 'query' }, }, + { + id: 'triggerSelector', + title: 'Triggers', + type: 'dropdown', + multiSelect: true, + options: [], + placeholder: 'All triggers', + description: 'Only include runs started this way. Leave empty for all.', + mode: 'basic', + canonicalParamId: 'triggers', + condition: { field: 'operation', value: 'query' }, + fetchOptions: () => fetchTriggerTypeOptions(), + }, + { + id: 'manualTriggers', + title: 'Triggers', + type: 'short-input', + placeholder: 'Comma-separated trigger types (api, schedule, slack)', + mode: 'advanced', + canonicalParamId: 'triggers', + condition: { field: 'operation', value: 'query' }, + }, { id: 'timeRange', title: 'Time Range', @@ -548,6 +585,7 @@ export const LogsV2Block: BlockConfig = { return { workflowIds: joinIds(params.workflowIds), level, + triggers: joinIds(params.triggers), startDate: params.startDate || presetStartDate, endDate: params.endDate || undefined, costOperator: costValue !== undefined ? params.costOperator || undefined : undefined, @@ -566,6 +604,10 @@ export const LogsV2Block: BlockConfig = { operation: { type: 'string', description: 'Operation to perform' }, workflowIds: { type: 'array', description: 'Workflow IDs to filter by (canonical param)' }, level: { type: 'array', description: 'Statuses to include (empty for all)' }, + triggers: { + type: 'string', + description: 'Comma-separated trigger types to include (canonical param, empty for all)', + }, timeRange: { type: 'string', description: 'Preset time window' }, startDate: { type: 'string', description: 'ISO 8601 lower bound (overrides Time Range)' }, endDate: { type: 'string', description: 'ISO 8601 upper bound' }, diff --git a/apps/sim/lib/workflows/subblocks/options.test.ts b/apps/sim/lib/workflows/subblocks/options.test.ts index a5146093bbc..ad70db442af 100644 --- a/apps/sim/lib/workflows/subblocks/options.test.ts +++ b/apps/sim/lib/workflows/subblocks/options.test.ts @@ -3,9 +3,10 @@ */ import { beforeEach, describe, expect, it, vi } from 'vitest' -const { mockFetchQuery, mockGetSubBlockValue } = vi.hoisted(() => ({ +const { mockFetchQuery, mockGetSubBlockValue, mockTriggerOptions } = vi.hoisted(() => ({ mockFetchQuery: vi.fn(), mockGetSubBlockValue: vi.fn(), + mockTriggerOptions: vi.fn(), })) vi.mock('@/app/_shell/providers/get-query-client', () => ({ @@ -24,6 +25,10 @@ vi.mock('@/stores/workflows/registry/store', () => ({ }, })) +vi.mock('@/lib/logs/get-trigger-options', () => ({ + getTriggerOptions: () => mockTriggerOptions(), +})) + vi.mock('@/stores/workflows/subblock/store', () => ({ useSubBlockStore: { getState: () => ({ getValue: mockGetSubBlockValue }), @@ -31,6 +36,7 @@ vi.mock('@/stores/workflows/subblock/store', () => ({ })) import { + fetchTriggerTypeOptions, fetchWorkspaceSandboxOption, fetchWorkspaceSandboxOptions, } from '@/lib/workflows/subblocks/options' @@ -101,3 +107,38 @@ describe('workspace sandbox options', () => { }) }) }) + +describe('fetchTriggerTypeOptions', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('merges values that share a label into one comma-joined option', async () => { + mockTriggerOptions.mockReturnValue([ + { value: 'api', label: 'API', color: '#2563eb' }, + { value: 'copilot', label: 'Sim agent', color: '#ec4899' }, + { value: 'mothership', label: 'Sim agent', color: '#ec4899' }, + { value: 'slack', label: 'Slack', color: '#611f69' }, + ]) + + await expect(fetchTriggerTypeOptions()).resolves.toEqual([ + { id: 'api', label: 'API' }, + { id: 'copilot,mothership', label: 'Sim agent' }, + { id: 'slack', label: 'Slack' }, + ]) + }) + + it('preserves registry order so core trigger types lead the list', async () => { + mockTriggerOptions.mockReturnValue([ + { value: 'manual', label: 'Manual', color: '#6b7280' }, + { value: 'api', label: 'API', color: '#2563eb' }, + { value: 'airtable', label: 'Airtable', color: '#181d1f' }, + ]) + + await expect(fetchTriggerTypeOptions()).resolves.toEqual([ + { id: 'manual', label: 'Manual' }, + { id: 'api', label: 'API' }, + { id: 'airtable', label: 'Airtable' }, + ]) + }) +}) diff --git a/apps/sim/lib/workflows/subblocks/options.ts b/apps/sim/lib/workflows/subblocks/options.ts index 168ac0e8e3c..3e6b3f5e954 100644 --- a/apps/sim/lib/workflows/subblocks/options.ts +++ b/apps/sim/lib/workflows/subblocks/options.ts @@ -143,3 +143,30 @@ export async function fetchWorkspaceSandboxOption( } return option } + +/** + * Loads the trigger vocabulary the Logs page filter offers — the core trigger + * types plus one entry per registered webhook provider — for the Logs block's + * trigger filter, so both surfaces name a run's origin identically. + * + * The registry is reached lazily: `getTriggerOptions` reads the block and trigger + * registries, and importing it at module scope from a module that block + * definitions themselves import would close an initialization cycle. + * + * Entries sharing a label are merged into one option whose id is the comma-joined + * set of values (`copilot,mothership` for "Sim agent"). The filter is a + * comma-separated list end to end, so a merged id selects every value behind the + * label instead of offering two identical rows. + */ +export async function fetchTriggerTypeOptions(): Promise { + const { getTriggerOptions } = await import('@/lib/logs/get-trigger-options') + + const valuesByLabel = new Map() + for (const option of getTriggerOptions()) { + const values = valuesByLabel.get(option.label) + if (values) values.push(option.value) + else valuesByLabel.set(option.label, [option.value]) + } + + return Array.from(valuesByLabel, ([label, values]) => ({ id: values.join(','), label })) +} diff --git a/apps/sim/lib/workflows/subblocks/trigger-options-live.test.ts b/apps/sim/lib/workflows/subblocks/trigger-options-live.test.ts new file mode 100644 index 00000000000..c7ae616b54d --- /dev/null +++ b/apps/sim/lib/workflows/subblocks/trigger-options-live.test.ts @@ -0,0 +1,30 @@ +/** + * @vitest-environment node + */ +import { describe, expect, it } from 'vitest' +import { fetchTriggerTypeOptions } from '@/lib/workflows/subblocks/options' + +/** + * Exercises the real block and trigger registries rather than a mock: the + * fetcher reaches them through a lazy import specifically to avoid an + * initialization cycle, and a mocked test cannot show that the import resolves + * or that the registry is populated by the time the dropdown asks for options. + */ +describe('fetchTriggerTypeOptions against the real registry', () => { + it('resolves the lazy import into a populated list of unique labels', async () => { + const options = await fetchTriggerTypeOptions() + + expect(options.length).toBeGreaterThan(10) + expect(options.every((option) => option.id.length > 0 && option.label.length > 0)).toBe(true) + + const labels = options.map((option) => option.label) + expect(new Set(labels).size).toBe(labels.length) + }) + + it('merges the two Sim agent trigger values behind one option', async () => { + const options = await fetchTriggerTypeOptions() + + expect(options.find((option) => option.label === 'Sim agent')?.id).toBe('copilot,mothership') + expect(options.find((option) => option.label === 'API')?.id).toBe('api') + }) +}) From 63c4b1c51f5f37f0d208b4f4234efd9d540cf8a7 Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 16:18:44 -0700 Subject: [PATCH 11/22] improvement(ui): give the two full-screen takeovers named design-system layers (#6829) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * improvement(ui): align the two full-screen takeovers on one design-system layer Both the session-expired screen and the desktop minimum-version gate are full-screen takeovers, but they disagreed on every piece of chrome: an ad-hoc z-[9999] against a z-50 that sat below --z-dropdown, --bg against --surface-1, a legacy Button against a Chip, and muted grey on their failure copy. The z-index was a real bug, not just an inconsistency. At z-50 the session-expired takeover rendered underneath the desktop browser panel's replacement snapshot, which paints at calc(var(--z-modal) - 1). Adds --z-takeover to the z-scale as the layer above every popper, points both takeovers at it, and aligns their background, primary action, body-copy and error tokens. Also corrects the z-scale in the design-review skill, which listed --z-toast at 500 when it has long been 150. * fix(ui): keep the shell gate above in-app takeovers and stop pre-paint click capture Tying both takeovers to one layer let document order decide which wins, and the desktop update gate mounts earlier than the session-expired screen, so the session overlay silently started covering it — reversing the precedence the old z-[9999] vs z-50 pair had. Names the precedence instead: --z-shell-gate sits above --z-takeover, because an incompatible shell invalidates everything the web app renders inside it. Both takeovers also hid their pre-paint state with opacity, which still hit-tests, so a full-viewport surface could swallow clicks before it painted. Switches them to the visibility toggle ModalContent already uses for the same handshake. --- .agents/skills/emcn-design-review/SKILL.md | 2 +- apps/sim/app/_shell/desktop-update-gate.tsx | 16 ++++++++-------- apps/sim/app/_styles/globals.css | 7 +++++++ .../session-expired/session-expired.tsx | 13 +++++++++---- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/.agents/skills/emcn-design-review/SKILL.md b/.agents/skills/emcn-design-review/SKILL.md index 78253e5e772..09a9932d4b1 100644 --- a/.agents/skills/emcn-design-review/SKILL.md +++ b/.agents/skills/emcn-design-review/SKILL.md @@ -39,7 +39,7 @@ Use CSS variable pattern (`text-[var(--text-primary)]`), never Tailwind semantic **Surfaces**: `--bg`, `--surface-1` through `--surface-7`, `--surface-hover`, `--surface-active` **Borders**: `--border`, `--border-1`, `--border-muted` **Brand/accent**: `--brand-secondary`, `--brand-accent` -**Z-Index**: `--z-dropdown` (100), `--z-modal` (200), `--z-popover` (300), `--z-tooltip` (400), `--z-toast` (500) +**Z-Index**: `--z-dropdown` (100), `--z-toast` (150), `--z-modal` (200), `--z-popover` (300), `--z-tooltip` (400), `--z-takeover` (500), `--z-shell-gate` (600) **Shadows**: `shadow-subtle`, `shadow-medium`, `shadow-overlay`, `shadow-card` **Badges**: `--badge-*` semantic families (success/error/gray/blue/purple/orange/amber/teal/cyan/pink, each with `-bg`/`-text`) diff --git a/apps/sim/app/_shell/desktop-update-gate.tsx b/apps/sim/app/_shell/desktop-update-gate.tsx index f9c114cfc43..a48cc3b2de8 100644 --- a/apps/sim/app/_shell/desktop-update-gate.tsx +++ b/apps/sim/app/_shell/desktop-update-gate.tsx @@ -2,7 +2,7 @@ import { useEffect, useState } from 'react' import type { DesktopUpdateState } from '@sim/desktop-bridge' -import { Button, useNativeSurfaceOcclusionReady } from '@sim/emcn' +import { Chip, useNativeSurfaceOcclusionReady } from '@sim/emcn' import { getDesktopBridge, getDesktopShellVersion, getDesktopUpdates } from '@/lib/desktop' import { isShellOutdated } from '@/lib/desktop/min-version' @@ -41,11 +41,11 @@ function gateActionFor(state: DesktopUpdateState): GateAction { } switch (state.status) { case 'checking': - return { label: 'Checking for updates...', disabled: true, onClick: () => {} } + return { label: 'Checking for updates…', disabled: true, onClick: () => {} } case 'downloading': return { label: - state.percent !== undefined ? `Downloading ${state.percent}%` : 'Downloading update...', + state.percent !== undefined ? `Downloading ${state.percent}%` : 'Downloading update…', disabled: true, onClick: () => {}, } @@ -97,8 +97,8 @@ export function DesktopUpdateGate() { return (
@@ -108,11 +108,11 @@ export function DesktopUpdateGate() { the update to keep going.

- + {updateState.status === 'error' && ( -

+

The update could not be downloaded. Check your connection and try again.

)} diff --git a/apps/sim/app/_styles/globals.css b/apps/sim/app/_styles/globals.css index ae53a936065..62cb18c353f 100644 --- a/apps/sim/app/_styles/globals.css +++ b/apps/sim/app/_styles/globals.css @@ -58,6 +58,13 @@ --z-modal: 200; --z-popover: 300; --z-tooltip: 400; + /* Full-screen takeovers replace the app entirely, so they sit above every + other layer including poppers. The desktop shell gate is the outermost of + them: an incompatible shell invalidates everything the web app renders + inside it, so it outranks in-app takeovers rather than tying with them and + letting document order decide. */ + --z-takeover: 500; + --z-shell-gate: 600; /* Shadow scale */ --shadow-subtle: 0 2px 4px 0 rgba(0, 0, 0, 0.08); diff --git a/apps/sim/app/workspace/[workspaceId]/components/session-expired/session-expired.tsx b/apps/sim/app/workspace/[workspaceId]/components/session-expired/session-expired.tsx index 03b8b416467..788951a6cd5 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/session-expired/session-expired.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/session-expired/session-expired.tsx @@ -1,7 +1,7 @@ 'use client' import { useCallback, useEffect, useRef, useState } from 'react' -import { Chip, useNativeSurfaceOcclusionReady } from '@sim/emcn' +import { Chip, cn, useNativeSurfaceOcclusionReady } from '@sim/emcn' import { useSession } from '@/lib/auth/auth-client' import { recoverFromStaleSession } from '@/lib/auth/stale-session-recovery' @@ -64,11 +64,16 @@ export function SessionExpired() { return (
-

+

{failed ? `${subject}, but signing out failed.` : `${subject}. Signing you out…`}

{failed && ( From cb3b93e2cc0d767bbb000ef3a753e3c23624a837 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Tue, 18 Aug 2026 16:24:09 -0700 Subject: [PATCH 12/22] fix(search): prioritize actions and prevent palette clipping (#6816) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(search): show new chat first for chats * fix(search): prioritize exact blocks on workflow editor * fix(search): keep the command palette inside small viewports The palette dialog was a fixed 500px box centered over the content area (offset right by the sidebar, and the panel on the canvas), so narrow windows pushed it past the right edge — clipping the Ask Sim adornment and the empty state. Its 448px list could also extend below the fold on short windows, where cmdk aligns the selected row against the off-screen bottom edge: the selection parked below the viewport and held arrow keys juddered rows against an edge the user could not see. Clamp the centered left position to a 16px gutter, shrink the width once the viewport is narrower than the dialog plus gutters, and cap the list height so the whole dialog stays on-screen. Co-Authored-By: Claude Fable 5 * fix(search): stop clipping the first glyph in the command search input Inputs clip glyph ink at their padding box, and the palette input had no left padding, so a leading glyph whose ink reaches its pen origin (the brand font's j) lost its left edge — worst at low browser zoom, where the clip boundary snaps to whole device pixels and eats up to 2 CSS px of the first letter. Give the input 3px of left ink clearance with a compensating negative margin so the text keeps its exact alignment with the result-row titles. Co-Authored-By: Claude Fable 5 * fix(search): give the first glyph real ink clearance via text-indent Chrome clips input text at the content box, not the padding box, so the previous padding-based clearance was dead space — glyph ink still started exactly at the clip edge, and the first letter kept losing its left edge under low browser zoom. text-indent starts the text 3px inside the clip region, which is clearance the renderer can actually paint into; the compensating negative margin keeps the text aligned with the result-row titles as before. Co-Authored-By: Claude Fable 5 * fix(search): stabilize command input glyph clearance --------- Co-authored-by: Claude Fable 5 --- .../command-chrome/command-chrome.test.tsx | 3 + .../command-chrome/command-chrome.tsx | 9 ++- .../search-modal/search-modal.test.tsx | 57 +++++++++++++++++++ .../components/search-modal/search-modal.tsx | 35 ++++++++++-- 4 files changed, 96 insertions(+), 8 deletions(-) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.test.tsx index f08c6adda6d..68b290bff6d 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.test.tsx @@ -57,9 +57,12 @@ describe('CommandFadedList', () => { }) const list = container.querySelector('[cmdk-list]') + const input = container.querySelector('[cmdk-input]') const search = container.querySelector('[cmdk-input]')?.parentElement expect(list?.className).toContain('transparent_36px,black_58px,black_calc(100%_-_13px)') expect(list?.className).not.toContain('scrollbar-track') + expect(input?.className).toContain('-ml-1') + expect(input?.className).toContain('indent-1') expect(search?.className).toContain('var(--bg)') }) diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.tsx index a71d83db3eb..bedee9fff8e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/components/command-chrome/command-chrome.tsx @@ -55,7 +55,12 @@ const LIST_FADE_CLASSNAME = { '[-webkit-mask-image:linear-gradient(to_bottom,transparent_0px,transparent_36px,black_58px,black_calc(100%_-_13px),transparent_100%)] [mask-image:linear-gradient(to_bottom,transparent_0px,transparent_36px,black_58px,black_calc(100%_-_13px),transparent_100%)]', } as const -/** Borderless search field layered over a fading command-result list. */ +/** + * Borderless search field layered over a fading command-result list. + * + * The matching indent and negative margin give leading glyphs room inside + * Chrome's input clip edge without moving the text out of alignment. + */ export const CommandSearch = forwardRef( function CommandSearch( { surface, cycleResultsOnTab = false, endAdornment, onKeyDown, ...props }, @@ -85,7 +90,7 @@ export const CommandSearch = forwardRef( diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.test.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.test.tsx index fcdd9e3c14b..4024fa40810 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.test.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.test.tsx @@ -252,6 +252,45 @@ describe('SearchModal', () => { expect(rows[2]).toContain('Onboarding') }) + it('puts an exact-name block before its page and contents on the workflow editor', async () => { + const Icon = () => null + const original = { ...mockSearchState.data } + mockSearchState.data = { + ...mockSearchState.data, + blocks: [{ id: 'logs', name: 'Logs', icon: Icon, bgColor: '#111', type: 'logs' }], + } + const logs = [ + { + id: 'log-1', + name: 'Billing sync', + href: '/workspace/workspace-1/logs?executionId=e1', + date: 'Aug 8, 1:00 PM', + }, + { + id: 'log-2', + name: 'Onboarding', + href: '/workspace/workspace-1/logs?executionId=e2', + date: 'Aug 8, 2:00 PM', + }, + ] + try { + await act(async () => { + root.render() + }) + + await enterSearchQuery('Logs') + const rows = Array.from(document.querySelectorAll('[cmdk-item]')) + expect(rows.slice(0, 4).map((row) => row.textContent ?? '')).toEqual([ + 'Logs', + 'Logs⇧⌘L', + 'Billing syncAug 8, 1:00 PM', + 'OnboardingAug 8, 2:00 PM', + ]) + } finally { + mockSearchState.data = original + } + }) + it('puts Create workflow first for the module-name query, then the workflows', async () => { const workflows = [ { id: 'workflow-a', name: 'Alpha', href: '/workspace/workspace-1/w/workflow-a' }, @@ -278,6 +317,24 @@ describe('SearchModal', () => { expect(rows[2]).toContain('Beta') }) + it('puts New chat first for the module-name query, then the chats', async () => { + const chats = [ + { id: 'chat-a', name: 'Alpha', href: '/workspace/workspace-1/home?chatId=chat-a' }, + { id: 'chat-b', name: 'Beta', href: '/workspace/workspace-1/home?chatId=chat-b' }, + ] + await act(async () => { + root.render() + }) + + await enterSearchQuery('chats') + const rows = Array.from(document.querySelectorAll('[cmdk-item]')).map( + (el) => el.textContent ?? '' + ) + expect(rows[0]).toContain('New chat') + expect(rows[1]).toContain('Alpha') + expect(rows[2]).toContain('Beta') + }) + it('shows an empty state when search has no results', async () => { await act(async () => { root.render() diff --git a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.tsx b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.tsx index 0073598ccab..58a79fad7e6 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/search-modal/search-modal.tsx @@ -99,6 +99,17 @@ import { useSearchModalStore } from '@/stores/modals/search/store' import type { SearchBlockItem, SearchToolOperationItem } from '@/stores/modals/search/types' const logger = createLogger('SearchModal') + +/** + * Half of the dialog's effective width (`min(500px, 100% - 32px)`), used to + * clamp the centered `left` position. The dialog centers over the content + * area — offset right by the sidebar (and panel on the canvas) — so on narrow + * viewports the unclamped position pushes it past the right edge, clipping + * the input adornment and the empty state. Clamping keeps a 16px gutter on + * both sides; when the viewport is narrower than the dialog plus gutters, + * both clamp bounds collapse to `50%` and the dialog re-centers. + */ +const PALETTE_HALF_WIDTH = 'min(250px, 50% - 16px)' /** * Global row budget for the browse (empty-query) list, applied cumulatively in * section order. Individual sections are never capped in browse — the budget @@ -377,7 +388,8 @@ function SearchModalContent({ list.push({ id: 'new-chat', name: 'New chat', - keywords: 'chat message ask sim assistant home', + keywords: 'chat chats message ask sim assistant home', + exactQueries: ['chats'], icon: Home, context: 'global', run: () => routerRef.current.push(`/workspace/${workspaceId}/home`), @@ -1065,7 +1077,11 @@ function SearchModalContent({ availableBlocks, (item) => item.name, (item) => item.searchValue - ).map(({ item, score }) => ({ section: 'blocks', item, score })), + ).map(({ item, score }) => ({ + section: 'blocks', + item, + score: item.name.toLowerCase() === query.toLowerCase() ? PAGE_MATCH_TIER : score, + })), triggers: rank( 'triggers', displayTriggers, @@ -1301,14 +1317,15 @@ function SearchModalContent({ aria-hidden={!visuallyOpen} aria-label='Search' className={cn( - '-translate-x-1/2 fixed top-[15%] z-[var(--z-modal)] w-[500px] rounded-xl border border-[var(--border-muted)] bg-[var(--surface-4)] p-[3px] shadow-[var(--shadow-overlay)] dark:bg-[var(--surface-5)]', + '-translate-x-1/2 fixed top-[15%] z-[var(--z-modal)] w-[min(500px,calc(100%-32px))] rounded-xl border border-[var(--border-muted)] bg-[var(--surface-4)] p-[3px] shadow-[var(--shadow-overlay)] dark:bg-[var(--surface-5)]', visuallyOpen ? 'visible opacity-100' : 'invisible opacity-0' )} style={{ - left: + left: `clamp(calc(16px + ${PALETTE_HALF_WIDTH}), ${ pageContext === 'workflow' ? 'calc(50% + (var(--sidebar-width) - var(--panel-width)) / 2)' - : 'calc(var(--sidebar-width) / 2 + 50%)', + : 'calc(var(--sidebar-width) / 2 + 50%)' + }, calc(100% - 16px - ${PALETTE_HALF_WIDTH}))`, }} >
@@ -1319,11 +1336,17 @@ function SearchModalContent({ value={askMode ? askSimLabel : undefined} >
+ {/* 85dvh - 26px = viewport minus the 15% top offset, 10px of + dialog chrome, and a 16px bottom gutter. The cap keeps the + scroll box fully on-screen: cmdk aligns the selected row to + the box's bottom edge, so a box past the fold parks the + selection below the viewport and held arrow keys judder + rows against an edge the user cannot see. */} Date: Tue, 18 Aug 2026 16:46:49 -0700 Subject: [PATCH 13/22] chore(deps): collapse stale transitive js-yaml pins onto the patched releases (#6830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GHSA-5p4m-2wfm-xmqj (quadratic CPU in !!omap resolution) is patched in js-yaml 4.3.1 and 3.15.1. Both direct dependents already pin 4.3.1, but bun.lock still held ten stale nested resolutions — 4.2.0, 4.3.0, and 3.14.2 — under fumadocs, electron-builder/updater, gray-matter, and json-schema-to-typescript. Every one of those ranges (^4.1.0, ^4.1.1, ^3.13.1) already admits the patched release, so this is a lockfile-only dedupe: no manifest change and no overrides block, which would force gray-matter's 3.x range onto js-yaml 4 and break it. --- bun.lock | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/bun.lock b/bun.lock index 9eb05193a71..36bcacd7309 100644 --- a/bun.lock +++ b/bun.lock @@ -4614,8 +4614,6 @@ "@ai-sdk/provider-utils/@standard-schema/spec": ["@standard-schema/spec@1.1.0", "", {}, "sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w=="], - "@apidevtools/json-schema-ref-parser/js-yaml": ["js-yaml@4.2.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw=="], - "@asamuzakjp/css-color/lru-cache": ["lru-cache@10.4.3", "", {}, "sha512-JNAzZcXrCt42VGLuYz0zfAzDfAvJWW6AfYlDBQyDV5DClI2m5sAmK+OIO7s59XfsRsWHp02jAJrRadPRGTt6SQ=="], "@authenio/xml-encryption/xpath": ["xpath@0.0.32", "", {}, "sha512-rxMJhSIoiO8vXcWvSifKqhvV96GjiD5wYb8/QHdoRyQvraTpp4IEv944nhGausZZ3u7dhQXteZuZbaqfpB7uYw=="], @@ -4996,8 +4994,6 @@ "app-builder-lib/jiti": ["jiti@2.7.0", "", { "bin": { "jiti": "lib/jiti-cli.mjs" } }, "sha512-AC/7JofJvZGrrneWNaEnJeOLUx+JlGt7tNa0wZiRPT4MY1wmfKjt2+6O2p2uz2+skll8OZZmJMNqeke7kKbNgQ=="], - "app-builder-lib/js-yaml": ["js-yaml@4.3.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q=="], - "app-builder-lib/semver": ["semver@7.7.4", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA=="], "app-builder-lib/which": ["which@5.0.0", "", { "dependencies": { "isexe": "^3.1.1" }, "bin": { "node-which": "bin/which.js" } }, "sha512-JEdGzHwwkrbWoGOlIHqQ5gtprKGOenpDHpxE9zVR1bWbOtYRyPPHMe9FaP6x61CmNaTThSkb0DAJte5jD+DmzQ=="], @@ -5016,8 +5012,6 @@ "builder-util/chalk": ["chalk@4.1.2", "", { "dependencies": { "ansi-styles": "^4.1.0", "supports-color": "^7.1.0" } }, "sha512-oKnbhFyRIXpUuez8iBMmyEa4nbj4IOQyuhc/wy9kY7/WVPcwIO9VA668Pu8RkO7+0G76SLROeyw9CpQ061i4mA=="], - "builder-util/js-yaml": ["js-yaml@4.3.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q=="], - "c12/confbox": ["confbox@0.2.4", "", {}, "sha512-ysOGlgTFbN2/Y6Cg3Iye8YKulHw+R2fNXHrgSmXISQdMnomY6eNDprVdW9R5xBguEqI954+S6709UyiO7B+6OQ=="], "c12/dotenv": ["dotenv@16.6.1", "", {}, "sha512-uBq4egWHTcTt33a72vpSG0z3HnPuIl6NqYcTrKEg2azoEyl2hpW0zqlxysq2pK9HlDIHyHyakeYaYnSAwd8bow=="], @@ -5060,8 +5054,6 @@ "d3-sankey/d3-shape": ["d3-shape@1.3.7", "", { "dependencies": { "d3-path": "1" } }, "sha512-EUkvKjqPFUAZyOlhY5gzCxCeI0Aep04LwIRpsZ/mLFelJiUfnK56jo5JMDSE7yyP2kLSb6LtF+S5chMk7uqPqw=="], - "dmg-builder/js-yaml": ["js-yaml@4.3.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q=="], - "docs/tailwind-merge": ["tailwind-merge@3.6.0", "", {}, "sha512-uxL7qAVQriqRQPAyK3pj66VqskWqoZ37PW94jwOTwNfq/z9oyu1V+eqrZqtR2+fCiXdYOZe/Modt8GtvqNzu+w=="], "docs/tailwindcss": ["tailwindcss@4.3.1", "", {}, "sha512-hk+TB1m+K8CYNrP6rjQaq/Y+4Zylwpa87mLYBKCunwnnQ9p+fHb7kmSfGqyEJoxF/O6CDyABWVFEafNSYKll+Q=="], @@ -5094,8 +5086,6 @@ "electron-publish/mime": ["mime@2.6.0", "", { "bin": { "mime": "cli.js" } }, "sha512-USPkMeET31rOMiarsBNIHZKLGgvKc/LrjofAnBlOttf5ajRvqiRA8QsenbcooctK6d6Ts6aqZXBA+XbkKthiQg=="], - "electron-updater/js-yaml": ["js-yaml@4.3.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q=="], - "electron-updater/semver": ["semver@7.7.4", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA=="], "electron-winstaller/fs-extra": ["fs-extra@7.0.1", "", { "dependencies": { "graceful-fs": "^4.1.2", "jsonfile": "^4.0.0", "universalify": "^0.1.0" } }, "sha512-YJDaCJZEnBmcbw13fvdAM9AwNOJwOzrE4pqMqBq5nFiEqXUqHwlK4B+3pUw6JNvfSPtX05xFHtYy/1ni01eGCw=="], @@ -5130,22 +5120,16 @@ "form-data/mime-types": ["mime-types@2.1.35", "", { "dependencies": { "mime-db": "1.52.0" } }, "sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw=="], - "fumadocs-core/js-yaml": ["js-yaml@4.2.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw=="], - "fumadocs-mdx/@standard-schema/spec": ["@standard-schema/spec@1.1.0", "", {}, "sha512-l2aFy5jALhniG5HgqrD6jXLi/rUWrKvqN/qJx6yoJsgKhblVd+iqqU4RCXavm/jPityDo5TCvKMnpjKnOriy0w=="], "fumadocs-mdx/chokidar": ["chokidar@5.0.0", "", { "dependencies": { "readdirp": "^5.0.0" } }, "sha512-TQMmc3w+5AxjpL8iIiwebF73dRDF4fBIieAqGn9RGCWaEVwQ6Fb2cGe31Yns0RRIzii5goJ1Y7xbMwo1TxMplw=="], - "fumadocs-mdx/js-yaml": ["js-yaml@4.2.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw=="], - "fumadocs-openapi/@radix-ui/react-slot": ["@radix-ui/react-slot@1.3.0", "", { "dependencies": { "@radix-ui/react-compose-refs": "1.1.3" }, "peerDependencies": { "@types/react": "*", "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" }, "optionalPeers": ["@types/react"] }, "sha512-MojKku4U/miO8Av4Dkb+ctMAQx7JmY96LmtDQlAarCRtd7rN52QCSzBF+XAvr5S6coSVj9HEPBgHAHKEJVk/WA=="], "fumadocs-openapi/ajv": ["ajv@8.20.0", "", { "dependencies": { "fast-deep-equal": "^3.1.3", "fast-uri": "^3.0.1", "json-schema-traverse": "^1.0.0", "require-from-string": "^2.0.2" } }, "sha512-Thbli+OlOj+iMPYFBVBfJ3OmCAnaSyNn4M1vz9T6Gka5Jt9ba/HIR56joy65tY6kx/FCF5VXNB819Y7/GUrBGA=="], "fumadocs-openapi/chokidar": ["chokidar@5.0.0", "", { "dependencies": { "readdirp": "^5.0.0" } }, "sha512-TQMmc3w+5AxjpL8iIiwebF73dRDF4fBIieAqGn9RGCWaEVwQ6Fb2cGe31Yns0RRIzii5goJ1Y7xbMwo1TxMplw=="], - "fumadocs-openapi/js-yaml": ["js-yaml@4.2.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw=="], - "fumadocs-openapi/lucide-react": ["lucide-react@1.23.0", "", { "peerDependencies": { "react": "^16.5.1 || ^17.0.0 || ^18.0.0 || ^19.0.0" } }, "sha512-38BpJcD0JhFosxHApP/BYsBetLpQFRoTRzEzstM/XCc3jsAG7wqaY1lgVwxiUe3xqYE+lNxo2PkCmYwXWrwwIw=="], "fumadocs-openapi/tailwind-merge": ["tailwind-merge@3.6.0", "", {}, "sha512-uxL7qAVQriqRQPAyK3pj66VqskWqoZ37PW94jwOTwNfq/z9oyu1V+eqrZqtR2+fCiXdYOZe/Modt8GtvqNzu+w=="], @@ -5170,7 +5154,7 @@ "google-auth-library/gaxios": ["gaxios@7.1.5", "", { "dependencies": { "extend": "^3.0.2", "https-proxy-agent": "^7.0.1", "node-fetch": "^3.3.2" } }, "sha512-5FZy72Rh8LhtjmvDrKkI+lVhrsQrVKVsItxMoDm5mNQE+xR0WVIIs+jzPSJgBvKVsLi24fZhXJIsNI0bihDzFg=="], - "gray-matter/js-yaml": ["js-yaml@3.14.2", "", { "dependencies": { "argparse": "^1.0.7", "esprima": "^4.0.0" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-PMSmkqxr106Xa156c2M265Z+FTrPl+oxd/rgOQy2tijQeK5TxQ43psO1ZCwhVOSdnn+RzkzlRz/eY4BgJBYVpg=="], + "gray-matter/js-yaml": ["js-yaml@3.15.1", "", { "dependencies": { "argparse": "^1.0.7", "esprima": "^4.0.0" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-S99WuO3HlhO3XN41EtYUNl9zzXjoJx7QvmipxsJVxtCBT0YHEFy+iOJhjSvrmV12nYhWpZaM8lPHkJm0yUMbag=="], "groq-sdk/@types/node": ["@types/node@18.19.130", "", { "dependencies": { "undici-types": "~5.26.4" } }, "sha512-GRaXQx6jGfL8sKfaIDD6OupbIHBr9jv7Jnaml9tB7l4v068PAOXqfcujMMo5PhbIs6ggR1XODELqahT2R8v0fg=="], @@ -5188,8 +5172,6 @@ "is-inside-container/is-docker": ["is-docker@3.0.0", "", { "bin": { "is-docker": "cli.js" } }, "sha512-eljcgEDlEns/7AXFosB5K/2nCM4P7FQPkGc/DWLy5rmFEWvZayGrik1d9/QIY5nJ4f9YsVvBkA6kJpHn9rISdQ=="], - "json-schema-to-typescript/js-yaml": ["js-yaml@4.2.0", "", { "dependencies": { "argparse": "^2.0.1" }, "bin": { "js-yaml": "bin/js-yaml.js" } }, "sha512-ePWsvanv0DWuDRsW8dnt+R4jQ31SCRCQ7hhNcPXZPsoBZiemuZNYGf7adZdqX2D86j6rvKp3RpCxVTSb8WQlOw=="], - "jsonwebtoken/semver": ["semver@7.8.4", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-rUCObTnP32Q08R2uuIrt7r9PlEonuTmtuXYcW6s5kjdlj3xbnwe+21yXptAUYcMAABLkYYTtnmzb3w3EDZfueA=="], "jszip/readable-stream": ["readable-stream@2.3.8", "", { "dependencies": { "core-util-is": "~1.0.0", "inherits": "~2.0.3", "isarray": "~1.0.0", "process-nextick-args": "~2.0.0", "safe-buffer": "~5.1.1", "string_decoder": "~1.1.1", "util-deprecate": "~1.0.1" } }, "sha512-8p0AUk4XODgIewSi0l8Epjs+EVnWiK7NoDIEGU0HhE7+ZyY8D1IMY7odu5lRrFXGg71L15KG8QrPmum45RTtdA=="], From b446b42018fcfb948f099424001d7396cfc4236f Mon Sep 17 00:00:00 2001 From: Waleed Date: Tue, 18 Aug 2026 17:08:27 -0700 Subject: [PATCH 14/22] feat(consent): cookie consent banner and cookie policy (#6832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(consent): add a hosted-only cookie consent banner Adds a c15t-backed consent runtime and a Sim-styled banner, mounted from the root layout only when `isHosted` is true. A self-hosted deployment never mounts the runtime, so it makes no request to Sim's consent backend and never sees the banner. The banner is a non-modal card docked bottom-left, opposite the toast stack, built from the same chrome (border, --bg, --shadow-overlay) and from Chip/Switch/Label rather than c15t's own components — the runtime is imported from `@c15t/nextjs/headless`, which ships no UI or stylesheet. "Customize" expands the same card into per-category switches instead of opening a dialog over the app. Visibility and the available actions come from the jurisdiction policy the runtime resolves, and accept and reject are rendered with identical weight. * feat(consent): cookie policy page, CSP allowance, and design-system alignment The consent backend was blocked by our own CSP, so the runtime silently fell back to an offline policy that showed the banner to every visitor worldwide and recorded nothing. The backend origin now lives in lib/consent/constants and the CSP builder allows it from that single source. Banner: mount the runtime beside the app rather than wrapping it, behind a dynamic() boundary, so consent state cannot re-render the page tree and a self-hosted build never fetches the chunk. Align chrome with the toast card (z token, font scale, --text-body/--text-muted pairing) and mirror the light token layer the public shells pin, which a dark-theme visitor on a landing route outside ThemeProvider's forced list would otherwise miss. Read the category list from the store's own getDisplayedConsents() — the shipped defaults mark every category except necessary as display:false, so the hand-rolled filter rendered a one-row list. Docs: add /cookie-policy as a third ProsePage consumer with the cookie inventory in tables (a new table block kind on the shared primitive), cross-reference it from the Privacy Policy, and wire it into the sitemap and llms.txt. The policy promises consent can be changed at any time, so the banner can be reopened from it. * refactor(consent): apply the cleanup pass - Drop the .light DOM probe: it matched the banner's own element, so once set it could never flip back, and it went stale on a theme toggle with no navigation. The card now pins the light layer unconditionally, as every other public surface does. - Hoist the motion/style objects to module scope. - Move a chip's mr-auto into the row layout; chips carry no outer margin. - Use the shadow-overlay utility and --border rather than the legacy alias. - Render before , which the HTML spec requires. - Make the code formatting of a table column a renderer concern (codeColumns) instead of JSX smuggled into the row content. - Raise the table caption above body weight, and tighten comments. * refactor(consent): apply the simplify pass The consent runtime installs a childList+subtree MutationObserver on document.body for its iframe blocker, for the life of every hosted page — including the workflow canvas — and re-scans each added subtree. Sim gates no iframes by consent, so disableAutomaticBlocking turns it off. Also: collapse the ConsentProvider passthrough into the dynamic() export; move ConsentPreferencesLink under (landing)/cookie-policy so a shell module no longer imports landing chrome; build the three cookie tables from one shape; move the table column widths into the prose chrome layer; only compute the category list when the card is expanded; drop the ConsentCategory cast; express the card width in Tailwind rather than an inline style. Comment corrections: the sibling mount is forced by ssr:false, not by re-render concerns; lib/consent/constants must stay dependency-free because next.config loads it and the browser bundles it; codeColumns exists for biome's useJsxKeyInIterable, not for React; the headless entry omits the components but the provider still injects an inert --c15t-* style block. * fix(consent): address the first review round - Add /cookie-policy to LANDING_ROUTES. It is an app/(landing) route, and every one of those must be exempt from COEP: the header is inherited across soft navigations, so an isolated landing page navigating into /demo leaves the Cal.com booker loading uncredentialed. - Render the withdrawal control as plain text on a self-hosted deployment, where the consent runtime is never mounted and the button had no listener. - Give ConsentPreferencesLink a named props interface. --- .../components/legal-block/legal-block.tsx | 45 ++- .../components/prose-page/constants.ts | 17 + .../(landing)/components/prose-page/types.ts | 27 ++ .../consent-preferences-link.tsx | 31 ++ .../cookie-policy/cookie-policy-content.tsx | 295 ++++++++++++++++++ .../(landing)/cookie-policy/cookie-policy.tsx | 12 + apps/sim/app/(landing)/cookie-policy/page.tsx | 18 ++ .../app/(landing)/privacy/privacy-content.tsx | 12 +- .../sim/app/_shell/consent/consent-banner.tsx | 192 ++++++++++++ .../app/_shell/consent/consent-provider.tsx | 21 ++ .../_shell/consent/consent-runtime.test.tsx | 57 ++++ .../app/_shell/consent/consent-runtime.tsx | 39 +++ apps/sim/app/layout.tsx | 3 + apps/sim/app/llms-full.txt/route.ts | 1 + apps/sim/app/llms.txt/route.ts | 1 + apps/sim/app/sitemap.ts | 6 +- apps/sim/lib/consent/constants.ts | 48 +++ apps/sim/lib/core/security/csp.ts | 7 +- apps/sim/next.config.ts | 1 + apps/sim/package.json | 1 + bun.lock | 19 +- 21 files changed, 847 insertions(+), 6 deletions(-) create mode 100644 apps/sim/app/(landing)/cookie-policy/consent-preferences-link.tsx create mode 100644 apps/sim/app/(landing)/cookie-policy/cookie-policy-content.tsx create mode 100644 apps/sim/app/(landing)/cookie-policy/cookie-policy.tsx create mode 100644 apps/sim/app/(landing)/cookie-policy/page.tsx create mode 100644 apps/sim/app/_shell/consent/consent-banner.tsx create mode 100644 apps/sim/app/_shell/consent/consent-provider.tsx create mode 100644 apps/sim/app/_shell/consent/consent-runtime.test.tsx create mode 100644 apps/sim/app/_shell/consent/consent-runtime.tsx create mode 100644 apps/sim/lib/consent/constants.ts diff --git a/apps/sim/app/(landing)/components/prose-page/components/legal-block-group/components/legal-block/legal-block.tsx b/apps/sim/app/(landing)/components/prose-page/components/legal-block-group/components/legal-block/legal-block.tsx index 8cbee9c18ad..0c3f4cda18d 100644 --- a/apps/sim/app/(landing)/components/prose-page/components/legal-block-group/components/legal-block/legal-block.tsx +++ b/apps/sim/app/(landing)/components/prose-page/components/legal-block-group/components/legal-block/legal-block.tsx @@ -5,8 +5,9 @@ import type { LegalBlock } from '@/app/(landing)/components/prose-page/types' /** * Renders a single {@link LegalBlock} into its canonical chrome. The block's * `kind` discriminant selects the element (paragraph / subheading `

` / - * bulleted list / callout box); all sizing and color come from `PROSE_TYPE`, so - * Terms and Privacy share one visual treatment for every block type. Content + * bulleted list / callout box / reference table); all sizing and color come + * from `PROSE_TYPE`, so Terms and Privacy share one visual treatment for every + * block type. Content * only - no layout knob. Server Component. */ @@ -35,6 +36,46 @@ export function LegalBlockView({ block }: LegalBlockViewProps) { ) case 'callout': return
{block.content}
+ case 'table': + return ( +
+ + {block.caption ? ( + + ) : null} + {block.columnWidths ? ( + + {block.columnWidths.map((width, index) => ( + + ))} + + ) : null} + + + {block.columns.map((column) => ( + + ))} + + + + {block.rows.map((row, rowIndex) => { + const rowKey = `row-${rowIndex}` + return ( + + {row.map((cell, cellIndex) => ( + + ))} + + ) + })} + +
{block.caption}
+ {column} +
+ {block.codeColumns?.includes(cellIndex) ? {cell} : cell} +
+
+ ) default: return null } diff --git a/apps/sim/app/(landing)/components/prose-page/constants.ts b/apps/sim/app/(landing)/components/prose-page/constants.ts index d388afa1f11..4fda51fc303 100644 --- a/apps/sim/app/(landing)/components/prose-page/constants.ts +++ b/apps/sim/app/(landing)/components/prose-page/constants.ts @@ -36,6 +36,16 @@ export const PROSE_SPACING = { listIndent: 'pl-6', } as const +/** + * Column widths for the cookie-inventory tables. Sibling tables sharing a header + * must be given a fixed layout or each sizes itself to its own content and the + * group reads as unaligned grids — so the widths are chrome, and live here + * rather than as class strings in a content config. + */ +export const PROSE_TABLE_WIDTHS = { + cookieInventory: ['w-[24%]', 'w-[16%]', 'w-[45%]', 'w-[15%]'], +} as const + /** * Prose type tokens - the single source of truth for every heading size, body * color, list, callout, and inline-link treatment. Centralized alongside the @@ -53,4 +63,11 @@ export const PROSE_TYPE = { callout: 'rounded-lg border border-[var(--border)] bg-[var(--surface-2)] px-4 py-3 text-[14px] text-[var(--text-body)] leading-[1.6]', link: 'text-[var(--text-primary)] underline underline-offset-2 transition-colors hover:text-[var(--text-body)]', + tableWrap: 'w-full overflow-x-auto', + tableCaption: 'pb-2 text-left text-[15px] text-[var(--text-primary)]', + table: 'w-full min-w-[560px] table-fixed border-collapse text-left', + tableHeadCell: + 'border-[var(--border)] border-b px-3 py-2 align-bottom font-medium text-[13px] text-[var(--text-primary)] first:pl-0 last:pr-0', + tableCell: + 'border-[var(--border)] border-b px-3 py-2.5 align-top text-[14px] text-[var(--text-body)] leading-[1.55] first:pl-0 last:pr-0 [&_code]:font-mono [&_code]:text-[13px]', } as const diff --git a/apps/sim/app/(landing)/components/prose-page/types.ts b/apps/sim/app/(landing)/components/prose-page/types.ts index bf8d7853e2e..b8cfc2cc505 100644 --- a/apps/sim/app/(landing)/components/prose-page/types.ts +++ b/apps/sim/app/(landing)/components/prose-page/types.ts @@ -22,6 +22,33 @@ export type LegalBlock = | { kind: 'list'; items: ReactNode[] } /** An emphasized callout box (e.g. the arbitration / GDPR notices). */ | { kind: 'callout'; content: ReactNode } + /** + * A reference table — the cookie inventory's name / provider / purpose / + * retention grid. Rows are positional against `columns`, so every row must + * have the same length as the header. + */ + | { + kind: 'table' + caption?: string + columns: string[] + /** + * Tailwind width fragments applied per column, e.g. `['w-[22%]', …]`. Set + * them whenever a page renders sibling tables with the same columns: + * without a fixed layout each table sizes itself to its own content and + * the group reads as three unaligned grids. + */ + columnWidths?: string[] + /** + * Indices of columns rendered as inline code — cookie names, config keys. + * A marker rather than a `` in the row data, because a row carries + * content and the renderer owns chrome. It is also what keeps the rows + * plain strings: biome's `useJsxKeyInIterable` fires on JSX inside an + * array literal, and the key it wants means nothing to a cell the + * renderer already keys by column. + */ + codeColumns?: number[] + rows: ReactNode[][] + } /** A numbered (or named) legal section - an `

` plus its ordered blocks. */ export interface LegalSection { diff --git a/apps/sim/app/(landing)/cookie-policy/consent-preferences-link.tsx b/apps/sim/app/(landing)/cookie-policy/consent-preferences-link.tsx new file mode 100644 index 00000000000..dac9ce50048 --- /dev/null +++ b/apps/sim/app/(landing)/cookie-policy/consent-preferences-link.tsx @@ -0,0 +1,31 @@ +'use client' + +import type { ReactNode } from 'react' +import { OPEN_CONSENT_PREFERENCES_EVENT } from '@/lib/consent/constants' +import { PROSE_TYPE } from '@/app/(landing)/components/prose-page/constants' + +interface ConsentPreferencesLinkProps { + children: ReactNode +} + +/** + * Inline control that reopens the consent banner with its category switches + * expanded, so a recorded choice can be withdrawn or changed. Wearing the + * prose link chrome, it reads as part of the sentence it sits in. + * + * Only rendered where the consent runtime is mounted — see the call site. On a + * self-hosted deployment nothing would listen for the event, so the Cookie + * Policy renders the phrase as plain text rather than a control that does + * nothing when clicked. + */ +export function ConsentPreferencesLink({ children }: ConsentPreferencesLinkProps) { + return ( + + ) +} diff --git a/apps/sim/app/(landing)/cookie-policy/cookie-policy-content.tsx b/apps/sim/app/(landing)/cookie-policy/cookie-policy-content.tsx new file mode 100644 index 00000000000..bb8bec8c565 --- /dev/null +++ b/apps/sim/app/(landing)/cookie-policy/cookie-policy-content.tsx @@ -0,0 +1,295 @@ +import type { ReactNode } from 'react' +import { isHosted } from '@/lib/core/config/env-flags' +import { + type LegalBlock, + type LegalPageConfig, + ProseLink, +} from '@/app/(landing)/components/prose-page' +import { PROSE_TABLE_WIDTHS } from '@/app/(landing)/components/prose-page/constants' +import { ConsentPreferencesLink } from '@/app/(landing)/cookie-policy/consent-preferences-link' + +/** + * One cookie-inventory table per consent category. The three share a header and + * a column layout, so they are built from one shape rather than repeated. + */ +/** + * The withdrawal control, or the bare phrase on a self-hosted deployment. The + * consent runtime is hosted-only, so there the button would have no listener + * and clicking it would do nothing. + */ +const CHANGE_CHOICES: ReactNode = isHosted ? ( + change your cookie choices +) : ( + 'change your cookie choices' +) + +function cookieTable(caption: string, rows: ReactNode[][]): LegalBlock { + return { + kind: 'table', + caption, + columns: ['Cookie', 'Provider', 'Purpose', 'Retention'], + columnWidths: [...PROSE_TABLE_WIDTHS.cookieInventory], + codeColumns: [0], + rows, + } +} + +/** + * Cookie Policy content — the inventory a consent banner has to stand on, + * expressed as the typed {@link LegalPageConfig} that `ProsePage` renders, so it + * shares its layout and rhythm with Terms and Privacy and cannot drift. + * + * The tables describe what Sim and its providers actually set, grouped by the + * three categories the banner offers. Keep them in step with the banner's + * categories (`lib/consent/constants`) and with the tags configured in Google + * Tag Manager: naming a cookie the site no longer sets is as wrong as omitting + * one it does. + */ +export const COOKIE_POLICY_CONFIG: LegalPageConfig = { + title: 'Cookie Policy', + description: + 'What cookies Sim sets, why, how long they last, and how to change your choice at any time.', + lastUpdated: 'August 18, 2026', + intro: [ + { + kind: 'paragraph', + content: ( + <> + This Cookie Policy explains how Sim uses cookies and similar technologies on sim.ai and in + the Sim application, what each one does, and the choices you have. It forms part of our{' '} + Privacy Policy, which describes how we handle + personal data more broadly. + + ), + }, + { + kind: 'paragraph', + content: ( + <> + If you are in the EU, the UK, or another region where consent is required, we ask before + setting anything that is not strictly necessary. You can {CHANGE_CHOICES} at any time. + + ), + }, + ], + sections: [ + { + id: 'what-are-cookies', + heading: 'What are cookies?', + blocks: [ + { + kind: 'paragraph', + content: `A cookie is a small text file a site stores on your device so it can recognize your browser on a later request. Cookies are how a site keeps you signed in between pages, remembers a preference, or counts a visit.`, + }, + { + kind: 'list', + items: [ + <> + Session cookies are deleted when you close your browser.{' '} + Persistent cookies stay until they expire or you delete them. + , + <> + First-party cookies are set by the site you are visiting.{' '} + Third-party cookies are set by another company whose code the site + loads, such as an analytics or advertising provider. + , + ], + }, + { + kind: 'paragraph', + content: `We also use technologies that behave like cookies without being one. Local storage and session storage keep data in your browser rather than sending it with each request; pixels (also called web beacons or tags) are tiny images or scripts that record that a page or email was opened. Where this policy says "cookies", it means all of these.`, + }, + ], + }, + { + id: 'how-we-use-cookies', + heading: 'How we use cookies', + blocks: [ + { + kind: 'paragraph', + content: `We group cookies into the three categories the consent banner offers. Necessary cookies are always on because the service cannot run without them. The other two are off until you turn them on.`, + }, + { + kind: 'list', + items: [ + <> + Necessary — sign-in, session security, abuse prevention, and + remembering the choice you made in the consent banner. These do not require consent + because the service you asked for cannot be delivered without them. + , + <> + Analytics — how many people use Sim, which pages and features they + reach, and where errors happen, so we can improve the product. Measurement only; we do + not use these to target advertising. + , + <> + Marketing — measuring which campaigns bring builders to Sim and + showing relevant ads on other sites. + , + ], + }, + ], + }, + { + id: 'cookies-we-use', + heading: 'Cookies we use', + blocks: [ + { + kind: 'paragraph', + content: `Retention periods are the maximum lifetime set when the cookie is written; a cookie can be cleared sooner at any time. Third-party providers occasionally rename or re-scope their cookies, so treat the provider column as the authoritative reference for anything not set by Sim.`, + }, + cookieTable('Necessary', [ + [ + 'better-auth.session_token', + 'Sim', + 'Keeps you signed in and identifies your session.', + '30 days', + ], + [ + 'better-auth.session_data', + 'Sim', + 'Short-lived signed cache of your session so each page load does not re-read the database.', + '5 minutes', + ], + [ + 'c15t', + 'Sim (via c15t)', + 'Records the cookie choice you made so the banner is not shown again.', + '365 days', + ], + [ + 'sidebar_collapsed', + 'Sim', + 'Remembers whether the workspace sidebar is collapsed, so the layout does not jump on load.', + '1 year', + ], + [ + '__cf_bm', + 'Cloudflare', + 'Bot-management check on requests to providers we load, such as HubSpot and X.', + '30 minutes', + ], + ]), + cookieTable('Analytics', [ + ['_ga', 'Google Analytics', 'Distinguishes one visitor from another.', '13 months'], + [ + '_ga_*', + 'Google Analytics', + 'Holds the session state for a specific Analytics property.', + '13 months', + ], + ['__hstc', 'HubSpot', 'Tracks visits across sessions for the main tracker.', '6 months'], + ['hubspotutk', 'HubSpot', 'Identifies a visitor across form submissions.', '6 months'], + ['__hssc', 'HubSpot', 'Tracks the current session.', '30 minutes'], + ['__hssrc', 'HubSpot', 'Detects whether the visitor restarted their browser.', 'Session'], + ]), + cookieTable('Marketing', [ + [ + 'guest_id', + 'X (Twitter)', + 'Identifies a browser to the X conversion pixel.', + '13 months', + ], + ['guest_id_ads', 'X (Twitter)', 'Measures conversions from X advertising.', '13 months'], + [ + 'guest_id_marketing', + 'X (Twitter)', + 'Measures the performance of X marketing campaigns.', + '13 months', + ], + ['personalization_id', 'X (Twitter)', 'Personalizes the ads shown on X.', '13 months'], + ['muc_ads', 'X (Twitter)', 'Measures ad conversions across X domains.', '13 months'], + ['_gcl_*', 'Google Ads', 'Attributes a sign-up to the ad that led to it.', '90 days'], + ]), + ], + }, + { + id: 'your-choices', + heading: 'Your choices', + blocks: [ + { + kind: 'paragraph', + content: ( + <> + Where consent is required, the banner appears on your first visit with accept and + reject offered equally, and "Customize" lets you turn each category on or off + individually. To revisit that decision later — including withdrawing consent you + already gave — {CHANGE_CHOICES}. We ask again after 365 days. + + ), + }, + { + kind: 'paragraph', + content: `Independently of the banner, every major browser lets you block or delete cookies from its privacy settings, and can be set to clear them each time you close it. Blocking necessary cookies will sign you out and prevent parts of Sim from working.`, + }, + { + kind: 'paragraph', + content: `We honor Global Privacy Control (GPC). If your browser or an extension sends a GPC signal, we treat it as an instruction to opt out of analytics and marketing cookies without your having to use the banner.`, + }, + { + kind: 'paragraph', + content: ( + <> + You can also opt out with the providers directly:{' '} + + Google Analytics + + , Google Ads,{' '} + X (Twitter), + and HubSpot. + + ), + }, + ], + }, + { + id: 'third-party-cookies', + heading: 'Third-party cookies', + blocks: [ + { + kind: 'paragraph', + content: `Some cookies above are set by companies we work with rather than by Sim. We choose these providers and decide when their code loads, but the data they collect is also governed by their own policies, which we cannot change on your behalf.`, + }, + { + kind: 'paragraph', + content: ( + <> + The providers currently in use are{' '} + Google{' '} + (Analytics, Tag Manager, and Ads),{' '} + HubSpot,{' '} + X (Twitter),{' '} + Ahrefs, and{' '} + Cloudflare. + + ), + }, + ], + }, + { + id: 'changes-to-this-policy', + heading: 'Changes to this policy', + blocks: [ + { + kind: 'paragraph', + content: `We update this policy when the cookies we set change, and we revise the "Last updated" date above whenever we do. If a change materially widens what we collect, we will ask for your consent again rather than rely on a choice you made under the previous version.`, + }, + ], + }, + { + id: 'contact', + heading: 'Contact', + blocks: [ + { + kind: 'paragraph', + content: ( + <> + Questions about this policy, or about how we use cookies, can go to{' '} + privacy@sim.ai. + + ), + }, + ], + }, + ], +} diff --git a/apps/sim/app/(landing)/cookie-policy/cookie-policy.tsx b/apps/sim/app/(landing)/cookie-policy/cookie-policy.tsx new file mode 100644 index 00000000000..1214fb5b2d0 --- /dev/null +++ b/apps/sim/app/(landing)/cookie-policy/cookie-policy.tsx @@ -0,0 +1,12 @@ +import { ProsePage } from '@/app/(landing)/components/prose-page' +import { COOKIE_POLICY_CONFIG } from '@/app/(landing)/cookie-policy/cookie-policy-content' + +/** + * Cookie Policy page - a thin consumer of the shared {@link ProsePage} + * primitive, alongside Terms and Privacy. The whole document is one typed + * config ({@link COOKIE_POLICY_CONFIG}) rendered inside the shared route-group + * layout chrome, so the three legal pages share a layout and cannot drift. + */ +export default function CookiePolicy() { + return +} diff --git a/apps/sim/app/(landing)/cookie-policy/page.tsx b/apps/sim/app/(landing)/cookie-policy/page.tsx new file mode 100644 index 00000000000..703faf0b2d4 --- /dev/null +++ b/apps/sim/app/(landing)/cookie-policy/page.tsx @@ -0,0 +1,18 @@ +import { buildLandingMetadata } from '@/lib/landing/seo' +import CookiePolicy from '@/app/(landing)/cookie-policy/cookie-policy' + +export const revalidate = 3600 + +const TITLE = 'Cookie Policy | Sim, the AI Workspace' +const DESCRIPTION = + 'What cookies Sim sets, why, how long they last, and how to change your choice at any time.' + +export const metadata = buildLandingMetadata({ + title: TITLE, + description: DESCRIPTION, + path: '/cookie-policy', +}) + +export default function Page() { + return +} diff --git a/apps/sim/app/(landing)/privacy/privacy-content.tsx b/apps/sim/app/(landing)/privacy/privacy-content.tsx index f72fb96d3b5..bf5240e4695 100644 --- a/apps/sim/app/(landing)/privacy/privacy-content.tsx +++ b/apps/sim/app/(landing)/privacy/privacy-content.tsx @@ -10,7 +10,7 @@ export const PRIVACY_CONFIG: LegalPageConfig = { title: 'Privacy Policy', description: 'How Sim, the open-source AI workspace, collects, uses, and protects your data, including data obtained from Google APIs, and the controls you have over it.', - lastUpdated: 'October 11, 2025', + lastUpdated: 'August 18, 2026', intro: [ { kind: 'paragraph', @@ -214,6 +214,16 @@ export const PRIVACY_CONFIG: LegalPageConfig = { kind: 'paragraph', content: `You can instruct your browser to refuse all cookies or to indicate when a cookie is being sent. However, if you do not accept cookies, you may not be able to use some portions of our Service.`, }, + { + kind: 'paragraph', + content: ( + <> + Our Cookie Policy lists every cookie we + and our providers set, what each one does, how long it lasts, and how to change or + withdraw your choice. + + ), + }, ], }, { diff --git a/apps/sim/app/_shell/consent/consent-banner.tsx b/apps/sim/app/_shell/consent/consent-banner.tsx new file mode 100644 index 00000000000..230033ff19c --- /dev/null +++ b/apps/sim/app/_shell/consent/consent-banner.tsx @@ -0,0 +1,192 @@ +'use client' + +import { useEffect } from 'react' +import { useConsentManager, useHeadlessConsentUI } from '@c15t/nextjs/headless' +import { Chip, Label, Switch } from '@sim/emcn' +import { AnimatePresence, motion, useReducedMotion } from 'framer-motion' +import Link from 'next/link' +import { type ConsentCategory, OPEN_CONSENT_PREFERENCES_EVENT } from '@/lib/consent/constants' + +interface ConsentCategoryCopy { + title: string + description: string +} + +/** + * Sim's own wording per category. The runtime ships generic descriptions; these + * say what the cookies actually do here. + * + * Typed by name rather than by {@link ConsentCategory} because the runtime's + * union is wider than the three categories we configure — a policy that adds + * one server-side falls back to the runtime's description instead of + * disappearing. The `satisfies` still requires an entry for each of ours. + */ +const CONSENT_CATEGORY_COPY: Record = { + necessary: { + title: 'Necessary', + description: 'Sign-in and security. Always on.', + }, + measurement: { + title: 'Analytics', + description: 'Shows us how Sim is used so we can make it better.', + }, + marketing: { + title: 'Marketing', + description: 'Measures which campaigns bring builders to Sim.', + }, +} satisfies Record + +/** Shared expo-out easing and timings, matching the toast stack's motion. */ +const EASE = [0.22, 1, 0.36, 1] as const +const ENTER_TRANSITION = { duration: 0.28, ease: EASE } as const +const EXPAND_TRANSITION = { duration: 0.22, ease: EASE } as const + +const NO_CATEGORIES: ReturnType['getDisplayedConsents']> = [] + +const CATEGORIES_COLLAPSED = { height: 0, opacity: 0 } as const +const CATEGORIES_OPEN = { height: 'auto', opacity: 1 } as const + +/** + * A copy of `PROSE_TYPE.link` rather than an import: the banner lives in the + * app shell and the token lives in the landing route group, and a shell module + * reaching into a route group is the wrong direction for one class string. + */ +const LINK_CLASS = + 'text-[var(--text-primary)] underline underline-offset-2 transition-colors hover:text-[var(--text-body)]' + +/** + * Cookie consent banner — a non-modal card docked bottom-left, opposite the + * toast stack and wearing the same chrome. It never dims, blocks, or reflows + * the page, and "Customize" expands this same card into per-category switches + * rather than opening a dialog over the app. + * + * Visibility and the available actions come from the jurisdiction policy the + * consent runtime resolves, so the banner is absent entirely where no consent + * is required and never offers an action the policy does not allow. Accept and + * reject carry identical weight, which GDPR requires. + * + * The card pins the `light` token layer rather than following the visitor's + * theme, as every other public surface does (`LandingShell`, `AuthShell`, the + * chat interfaces, the public file view). Consent is asked for on a first + * visit, which lands on one of those. A record expiring against a live session + * is the one path that renders this card over the themed app, where it will + * read light-on-dark; accepted as the rarer case. + */ +export function ConsentBanner() { + const { consents, selectedConsents, setSelectedConsent, getDisplayedConsents } = + useConsentManager() + const { banner, dialog, openDialog, performAction, saveCustomPreferences } = + useHeadlessConsentUI() + const prefersReducedMotion = useReducedMotion() + + useEffect(() => { + window.addEventListener(OPEN_CONSENT_PREFERENCES_EVENT, openDialog) + return () => window.removeEventListener(OPEN_CONSENT_PREFERENCES_EVENT, openDialog) + }, [openDialog]) + + const isExpanded = dialog.isVisible + const surfaceName = isExpanded ? 'dialog' : 'banner' + const { allowedActions } = isExpanded ? dialog : banner + /** + * The store's own selector, not a hand-rolled filter over `consentTypes`: the + * shipped defaults mark every category except `necessary` as `display: false`, + * so filtering on that flag silently renders a one-row list. It re-filters and + * re-allocates on every call, so only the expanded card pays for it. + */ + const categories = isExpanded ? getDisplayedConsents() : NO_CATEGORIES + const enterOffset = prefersReducedMotion ? 0 : 8 + + return ( + + {(banner.isVisible || dialog.isVisible) && ( + +
+

Cookies

+

+ We use cookies to run Sim, understand how it is used, and improve it. Read our{' '} + + Cookie Policy + + . +

+
+ + + {isExpanded && ( + +
    + {categories.map((type) => { + const copy = CONSENT_CATEGORY_COPY[type.name] + const inputId = `consent-${type.name}` + return ( +
  • +
    + +

    + {copy?.description ?? type.description} +

    +
    + setSelectedConsent(type.name, checked)} + /> +
    • + ) + })} +
+ + )} + + + {/* Two clusters, not `mr-auto` on the chip: chips carry no outer margin. */} +
+
+ {!isExpanded && allowedActions.includes('customize') && ( + Customize + )} +
+
+ {allowedActions.includes('reject') && ( + void performAction('reject', { surface: surfaceName })} + > + Reject all + + )} + {allowedActions.includes('accept') && ( + void performAction('accept', { surface: surfaceName })} + > + Accept all + + )} + {isExpanded && ( + void saveCustomPreferences()}> + Save + + )} +
+
+ + )} + + ) +} diff --git a/apps/sim/app/_shell/consent/consent-provider.tsx b/apps/sim/app/_shell/consent/consent-provider.tsx new file mode 100644 index 00000000000..d1c4f4d6dde --- /dev/null +++ b/apps/sim/app/_shell/consent/consent-provider.tsx @@ -0,0 +1,21 @@ +'use client' + +import dynamic from 'next/dynamic' + +/** + * The cookie-consent runtime, loaded on the client only and only once this + * component is rendered — the root layout renders it behind `isHosted`, so a + * self-hosted deployment never fetches the chunk, never reaches Sim's consent + * backend, and never sees the banner. Deferring it also keeps the third-party + * store out of the server render and off the landing page's hydration path; the + * banner cannot paint before its geo lookup resolves anyway. + * + * It mounts alongside the app rather than wrapping it because an `ssr: false` + * boundary around the tree would disable SSR for every route. Nothing can reach + * the store through context as a result, which is what + * `OPEN_CONSENT_PREFERENCES_EVENT` exists for. + */ +export const ConsentProvider = dynamic( + () => import('@/app/_shell/consent/consent-runtime').then((m) => m.ConsentRuntime), + { ssr: false } +) diff --git a/apps/sim/app/_shell/consent/consent-runtime.test.tsx b/apps/sim/app/_shell/consent/consent-runtime.test.tsx new file mode 100644 index 00000000000..142bfb47c4a --- /dev/null +++ b/apps/sim/app/_shell/consent/consent-runtime.test.tsx @@ -0,0 +1,57 @@ +/** + * @vitest-environment jsdom + */ +import type { ReactNode } from 'react' +import { act } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const { mockConsentManagerProvider, mockConsentBanner } = vi.hoisted(() => ({ + mockConsentManagerProvider: vi.fn(), + mockConsentBanner: vi.fn(), +})) + +vi.mock('@c15t/nextjs/headless', () => ({ + ConsentManagerProvider: (props: { children: ReactNode; options: unknown }) => { + mockConsentManagerProvider(props.options) + return props.children + }, +})) + +vi.mock('@/app/_shell/consent/consent-banner', () => ({ + ConsentBanner: () => { + mockConsentBanner() + return + }, +})) + +import { ConsentRuntime } from '@/app/_shell/consent/consent-runtime' + +let root: Root | null = null + +afterEach(() => { + act(() => root?.unmount()) + root = null + vi.clearAllMocks() +}) + +describe('ConsentRuntime', () => { + it('mounts the banner against the hosted consent backend', () => { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + const container = document.createElement('div') + document.body.appendChild(container) + root = createRoot(container) + act(() => root?.render()) + + expect(container.querySelector('[data-testid="banner"]')).not.toBeNull() + expect(mockConsentBanner).toHaveBeenCalled() + // `toMatchObject`, not exact equality: `DEV_CONSENT_COUNTRY` adds an + // `overrides` key whenever a developer has NEXT_PUBLIC_CONSENT_COUNTRY set + // locally, and the assertion is about the shipped configuration. + expect(mockConsentManagerProvider.mock.calls[0]?.[0]).toMatchObject({ + mode: 'hosted', + backendURL: 'https://sim-sim.inth.app', + consentCategories: ['necessary', 'measurement', 'marketing'], + }) + }) +}) diff --git a/apps/sim/app/_shell/consent/consent-runtime.tsx b/apps/sim/app/_shell/consent/consent-runtime.tsx new file mode 100644 index 00000000000..adfe5c381cb --- /dev/null +++ b/apps/sim/app/_shell/consent/consent-runtime.tsx @@ -0,0 +1,39 @@ +'use client' + +import { type ConsentManagerOptions, ConsentManagerProvider } from '@c15t/nextjs/headless' +import { + CONSENT_BACKEND_URL, + CONSENT_CATEGORIES, + DEV_CONSENT_COUNTRY, +} from '@/lib/consent/constants' +import { ConsentBanner } from '@/app/_shell/consent/consent-banner' + +/** + * Imported from `@c15t/nextjs/headless`, not the package root: the headless + * entry leaves the runtime's own components and stylesheet out of the bundle, + * so {@link ConsentBanner} is the only consent UI that exists. The provider + * still injects a `