diff --git a/.github/actions/docker-build/action.yml b/.github/actions/docker-build/action.yml index a607a720afd..b8727501f52 100644 --- a/.github/actions/docker-build/action.yml +++ b/.github/actions/docker-build/action.yml @@ -19,6 +19,13 @@ inputs: tags: description: Comma-separated list of tags to push. required: true + max-cache-size-mb: + description: >- + Layer cache to retain after the post-job prune, in MB. Must stay above one + build's working set (base + dependency layers + RUN --mount=type=cache + dirs) or every build evicts what the next one needs. Falls back to the + small-image default below when empty. + required: false # Registry logins must precede this action. provenance/sbom stay off: attestation # manifests break `imagetools create` retagging in promote-images. @@ -42,11 +49,24 @@ runs: PLATFORMS: ${{ inputs.platforms }} run: echo "value=${GITHUB_REPOSITORY##*/}/${FILE#./}/${PLATFORMS//\//-}" >> "$GITHUB_OUTPUT" + # max-cache-size-mb is what bounds the disk: BuildKit's default GC is + # time-based only (layers unused for 8 days), and setup-docker-builder skips + # pruning altogether when the value is empty. On a repo that builds this + # often nothing ever ages out, so the disks grew without limit — + # app.Dockerfile/linux-amd64 reached 351 GB inside a day, and realtime, whose + # image is under 300 MB, sat at 249 GB. Sticky disks bill at ~$0.51/GB-month, + # so that was real money for layers no build would ever read again. + # + # The fallback is here rather than an input `default:` because callers pass + # this from a matrix field, and an unset matrix key arrives as the empty + # string — which counts as "provided", so a `default:` would never apply and + # a row that forgot the field would silently go back to unbounded growth. - name: Set up Blacksmith builder if: inputs.provider == '' || inputs.provider == 'blacksmith' uses: useblacksmith/setup-docker-builder@a5256a73e30f09e37e3eceb8ca36043d17621d24 # v2 with: cache-key: ${{ steps.cache-key.outputs.value }} + max-cache-size-mb: ${{ inputs.max-cache-size-mb || '25600' }} - name: Build and push (Blacksmith) if: inputs.provider == '' || inputs.provider == 'blacksmith' diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml index b8e56f708d9..f50b20410aa 100644 --- a/.github/codeql/codeql-config.yml +++ b/.github/codeql/codeql-config.yml @@ -31,3 +31,11 @@ paths-ignore: - '**/dist/**' - '**/.next/**' - 'apps/docs/content/**' + +# Do NOT add `queries:`, `packs:`, `query-filters:`, or `disable-default-queries` +# here to try to speed the scan up. Under the code-scanning feature flag the +# action's checkOverlayAnalysisFeatureEnabled treats any of those as +# OverlayDisabledReason.NonDefaultQueries and permanently turns off overlay +# (incremental) analysis. Extraction is ~53% of a run and is exactly what overlay +# skips, so scoping the queries trades a documented up-to-10x win for a few +# percent off the 27% query phase. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 746c341cae8..aa1be99ea97 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,7 +76,7 @@ jobs: # (/api/desktop/update) starts offering automatically. detect-desktop-changes: name: Detect Desktop Changes - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }} timeout-minutes: 5 if: github.event_name == 'push' && (github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/staging') outputs: @@ -165,7 +165,15 @@ jobs: # build` ~260s). The same `next build` runs on 16 vCPU in the separate # Build App verification job, which does not gate anything; this one # was doing comparable work on half the cores. + # + # cache_mb is the layer cache the post-job prune retains, and it is the + # only reason the sticky disks stay bounded — see docker-build's + # action.yml. Rows that omit it take the small-image default there. The + # app image overrides because it carries ~34 layers plus apt and bun + # cache mounts for the whole monorepo; 100 GB is several builds' worth + # of headroom over that working set. - dockerfile: ./docker/app.Dockerfile + cache_mb: '102400' ecr_repo_secret: ECR_APP gh_runner: linux-x64-8-core bs_runner: blacksmith-16vcpu-ubuntu-2404 @@ -176,11 +184,11 @@ jobs: - dockerfile: ./docker/realtime.Dockerfile ecr_repo_secret: ECR_REALTIME gh_runner: ubuntu-latest - bs_runner: blacksmith-4vcpu-ubuntu-2404 + bs_runner: blacksmith-2vcpu-ubuntu-2404 - dockerfile: ./docker/pii.Dockerfile ecr_repo_secret: ECR_PII gh_runner: ubuntu-latest - bs_runner: blacksmith-4vcpu-ubuntu-2404 + bs_runner: blacksmith-2vcpu-ubuntu-2404 steps: - name: Checkout code uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 @@ -214,6 +222,7 @@ jobs: file: ${{ matrix.dockerfile }} platforms: linux/amd64 tags: ${{ steps.login-ecr.outputs.registry }}/${{ steps.ecr-repo.outputs.name }}:dev + max-cache-size-mb: ${{ matrix.cache_mb }} # Dev: deploy Trigger.dev background tasks to the preview "dev-sim" branch. # Gated after migrate-dev for the same reason as build-dev — the new task @@ -280,6 +289,7 @@ jobs: matrix: include: - dockerfile: ./docker/app.Dockerfile + cache_mb: '102400' ghcr_image: ghcr.io/simstudioai/simstudio ecr_repo_secret: ECR_APP gh_runner: linux-x64-8-core @@ -293,12 +303,12 @@ jobs: ghcr_image: ghcr.io/simstudioai/realtime ecr_repo_secret: ECR_REALTIME gh_runner: ubuntu-latest - bs_runner: blacksmith-4vcpu-ubuntu-2404 + bs_runner: blacksmith-2vcpu-ubuntu-2404 - dockerfile: ./docker/pii.Dockerfile ghcr_image: ghcr.io/simstudioai/pii ecr_repo_secret: ECR_PII gh_runner: ubuntu-latest - bs_runner: blacksmith-4vcpu-ubuntu-2404 + bs_runner: blacksmith-2vcpu-ubuntu-2404 # No ECR repo is provisioned for cron, so it publishes to GHCR only. # The tag step below omits the ECR tag when the repo name is empty. - dockerfile: ./docker/cron.Dockerfile @@ -382,6 +392,7 @@ jobs: file: ${{ matrix.dockerfile }} platforms: linux/amd64 tags: ${{ steps.meta.outputs.tags }} + max-cache-size-mb: ${{ matrix.cache_mb }} # Promote the sha-tagged ECR images to the deploy tags once tests and # migrations pass. Pushing the ECR latest/staging tag is what triggers @@ -484,6 +495,7 @@ jobs: # hang a release in `queued` rather than fail a PR. include: - dockerfile: ./docker/app.Dockerfile + cache_mb: '102400' image: ghcr.io/simstudioai/simstudio gh_runner: linux-arm64-8-core bs_runner: blacksmith-8vcpu-ubuntu-2404-arm @@ -522,6 +534,7 @@ jobs: file: ${{ matrix.dockerfile }} platforms: linux/arm64 tags: ${{ matrix.image }}:${{ github.sha }}-arm64 + max-cache-size-mb: ${{ matrix.cache_mb }} # Publish all mutable GHCR tags (latest, latest-amd64/arm64, version tags) # and the multi-arch manifests from the immutable sha tags — only on main, @@ -675,7 +688,7 @@ jobs: # Job-level `if:` cannot read the secrets context, hence the probe job. check-desktop-signing: name: Check Desktop Signing Secrets - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }} timeout-minutes: 2 needs: [detect-version, detect-desktop-changes] # !cancelled(): detect-desktop-changes is skipped on main (and @@ -724,7 +737,7 @@ jobs: # remains testable end to end with a manual download. create-desktop-prerelease: name: Create Desktop Prerelease - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }} timeout-minutes: 5 needs: [detect-desktop-changes, check-desktop-signing] # Requires the signing probe to have actually succeeded (not just "not @@ -813,7 +826,7 @@ jobs: # point of view. publish-desktop-prerelease: name: Publish Desktop Prerelease - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }} timeout-minutes: 5 needs: [create-desktop-prerelease, desktop-prerelease] permissions: @@ -837,7 +850,7 @@ jobs: # are always garbage by this point — the current run's release is published. prune-desktop-prereleases: name: Prune Desktop Prereleases - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }} timeout-minutes: 5 needs: [publish-desktop-prerelease] permissions: diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index f44ca430c7c..d6e5122f2c4 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -20,8 +20,16 @@ on: # created, prevents developers from introducing new vulnerabilities." push: branches: [main] + # main only, not staging. Feature PRs land on staging and are ~90% of PR scan + # volume, and every one of them is scanned again — against the exact tree being + # promoted — when the staging->main PR opens. Scanning at the promotion + # boundary defers the signal rather than dropping it. + # + # Deliberately a branch cut and not an activity-type cut: dropping + # `synchronize` would have scanned each PR's first commit and never its final + # state, which is backwards, since review fixups land in later pushes. pull_request: - branches: [main, staging] + branches: [main] # `ready_for_review` is not a default activity type, so it has to be listed # alongside the defaults it replaces. Without it, a PR opened as a draft and # then marked ready is skipped by the job-level draft guard and never @@ -41,7 +49,15 @@ on: # Safety net behind the push trigger, and the thing that keeps the # default-branch alert view fresh when main is quiet. Only fires once this # file is on the default branch — schedule events ignore other branches. - - cron: '17 8 * * 1' + # + # Daily rather than weekly. Pushes to main are rare, and with PR scans now + # limited to main the alert view leans on this more than it used to; a week + # is too long to leave it stale. It also reseeds the overlay-base database + # that PR runs restore from — that cache key embeds the CodeQL bundle + # version, so a bundle bump invalidates it, and an unused Actions cache is + # evicted after 7 days. One 8 vCPU default-branch scan a day is a few + # dollars a month against a PR scan that halves when the base is warm. + - cron: '17 8 * * *' workflow_dispatch: concurrency: @@ -54,7 +70,12 @@ permissions: jobs: analyze: name: Analyze ${{ matrix.language }} - runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-8vcpu-ubuntu-2404' || 'ubuntu-latest' }} + # Sized per language, not per workflow. The two analyses are nothing alike: + # javascript-typescript peaks at 19.5 GB (p95 over 3090 runs), so it needs + # the 8 vCPU tier's 30.4 GB and would OOM on the 4 vCPU tier's 15.2 GB; the + # actions analysis peaks at 1.3 GB and averages 22% CPU over a 39s median + # run, so 8 vCPU was 4x more machine than it ever used. + runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && matrix.bs_runner || 'ubuntu-latest' }} timeout-minutes: 60 if: github.event.pull_request.draft != true permissions: @@ -71,7 +92,11 @@ jobs: # entries default setup listed were one analysis, not three. # `javascript-typescript` is the documented spelling. Python dropped: # 7 files in the tree. - language: [javascript-typescript, actions] + include: + - language: javascript-typescript + bs_runner: blacksmith-8vcpu-ubuntu-2404 + - language: actions + bs_runner: blacksmith-4vcpu-ubuntu-2404 steps: - name: Checkout repository diff --git a/apps/sim/background/knowledge-connector-sync.ts b/apps/sim/background/knowledge-connector-sync.ts index f7ba8e80e4d..ca34b89c62c 100644 --- a/apps/sim/background/knowledge-connector-sync.ts +++ b/apps/sim/background/knowledge-connector-sync.ts @@ -98,7 +98,14 @@ export async function executeConnectorSyncJob(payload: unknown) { export const knowledgeConnectorSync = task({ id: 'knowledge-connector-sync', maxDuration: CONNECTOR_SYNC_MAX_DURATION_SECONDS, - machine: 'large-2x', + /** + * Sized from production telemetry: peak sampled RSS 2.6 GB and peak 1.4 vCPU, + * so `large-1x` holds ~3x memory and ~2.8x CPU headroom. No `outOfMemory` + * escalation: an OOM is a SIGKILL, so the run never reaches the terminal + * write that clears `syncLockToken`, and the escalated attempt would find the + * row still `syncing` and skip. The stale-lock reaper owns that recovery. + */ + machine: 'large-1x', retry: { maxAttempts: 3, factor: 2, diff --git a/apps/sim/background/knowledge-processing.ts b/apps/sim/background/knowledge-processing.ts index 43256bed6d6..8ff7e1aedf2 100644 --- a/apps/sim/background/knowledge-processing.ts +++ b/apps/sim/background/knowledge-processing.ts @@ -135,7 +135,13 @@ export async function runDocumentProcessing( export const processDocument = task({ id: 'knowledge-process-document', maxDuration: envNumber(env.KB_CONFIG_MAX_DURATION, 600), - machine: 'large-1x', // 4 vCPU, 8GB RAM - needed for large PDF processing + /** + * Sized from production telemetry: peak sampled RSS 902 MB and peak 1.2 vCPU + * across a corpus where no document exceeded 2 GB, so `medium-2x` holds ~4x + * memory and ~1.7x CPU headroom over the observed worst case. The prior + * `large-1x` reserved 8 GB against a worst case using an eighth of it. + */ + machine: 'medium-2x', retry: { maxAttempts: envNumber(env.KB_CONFIG_MAX_ATTEMPTS, 3), factor: envNumber(env.KB_CONFIG_RETRY_FACTOR, 2), diff --git a/apps/sim/lib/api/contracts/knowledge/documents.ts b/apps/sim/lib/api/contracts/knowledge/documents.ts index bc8ad27d841..151cae080e5 100644 --- a/apps/sim/lib/api/contracts/knowledge/documents.ts +++ b/apps/sim/lib/api/contracts/knowledge/documents.ts @@ -319,16 +319,23 @@ export const listKnowledgeDocumentsContract = defineRouteContract({ }, }) -export const createKnowledgeDocumentsContract = defineRouteContract({ - method: 'POST', - path: '/api/knowledge/[id]/documents', +/** + * Document creation from inline content has no HTTP route: `POST + * /api/knowledge/[id]/documents` was retired when tool operations moved + * in-process, and the surviving `GET`/`PATCH` on that path would answer a `POST` + * with 405. So these stay plain schemas rather than a `defineRouteContract` — + * `lib/internal/knowledge/execute-tool.ts` validates `knowledge_create_document` + * against them directly. Callers wanting an HTTP upload use v1 or v2, both of + * which take multipart file bodies rather than inline content. + */ +export const createKnowledgeDocumentsSchemas = { params: knowledgeBaseParamsSchema, body: createKnowledgeDocumentsBodySchema, - response: { - mode: 'json', - schema: successResponseSchema(z.union([bulkCreateDocumentsResponseSchema, documentDataSchema])), - }, -}) +} as const + +export const createKnowledgeDocumentsResponseSchema = successResponseSchema( + z.union([bulkCreateDocumentsResponseSchema, documentDataSchema]) +) export const updateKnowledgeDocumentContract = defineRouteContract({ method: 'PUT', diff --git a/apps/sim/lib/api/contracts/selectors/confluence.ts b/apps/sim/lib/api/contracts/selectors/confluence.ts index b680a1d8c93..360e61911ee 100644 --- a/apps/sim/lib/api/contracts/selectors/confluence.ts +++ b/apps/sim/lib/api/contracts/selectors/confluence.ts @@ -399,14 +399,16 @@ export const confluencePageSelectorContract = definePostSelector( z.object({ id: z.string(), title: z.string() }).passthrough() ) -export const confluenceUpdatePageContract = defineConfluencePutContract( - '/api/tools/confluence/page', - confluenceUpdatePageBodySchema -) -export const confluenceDeletePageContract = defineConfluenceDeleteContract( - '/api/tools/confluence/page', - confluenceDeletePageBodySchema -) +/** + * Page update and delete have no contract because they have no route: the + * `PUT`/`DELETE` handlers on `/api/tools/confluence/page` were retired when the + * tool moved in process, and the surviving selector `POST` on that path would + * answer either verb with 405. `lib/internal/confluence/execute-tool.ts` + * validates both against `confluenceUpdatePageBodySchema` / + * `confluenceDeletePageBodySchema` directly. + */ +export type ConfluenceUpdatePageBody = z.output +export type ConfluenceDeletePageBody = z.output export const confluenceDeleteAttachmentContract = defineConfluenceDeleteContract( '/api/tools/confluence/attachment', confluenceDeleteAttachmentBodySchema @@ -562,8 +564,6 @@ export const confluenceUserContract = defineConfluencePostContract( export type ConfluencePagesBody = ContractBody export type ConfluencePageBody = ContractBody -export type ConfluenceUpdatePageBody = ContractBody -export type ConfluenceDeletePageBody = ContractBody export type ConfluenceDeleteAttachmentBody = ContractBody export type ConfluenceListAttachmentsQuery = ContractQuery export type ConfluenceListBlogPostsQuery = ContractQuery diff --git a/apps/sim/lib/api/contracts/tools/docusign.ts b/apps/sim/lib/api/contracts/tools/docusign.ts deleted file mode 100644 index 5ebdfae4a87..00000000000 --- a/apps/sim/lib/api/contracts/tools/docusign.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { z } from 'zod' -import { defineRouteContract } from '@/lib/api/contracts/types' - -export const docusignToolBodySchema = z - .object({ - accessToken: z.string().min(1, 'Access token is required'), - operation: z.string().min(1, 'Operation is required'), - }) - .passthrough() - -export const docusignToolContract = defineRouteContract({ - method: 'POST', - path: '/api/tools/docusign', - body: docusignToolBodySchema, - response: { - mode: 'json', - // untyped-response: forwards DocuSign API response unchanged; shape varies by operation (envelope, listing, base64 download, etc.) - schema: z.unknown(), - }, -}) diff --git a/apps/sim/lib/api/contracts/tools/index.ts b/apps/sim/lib/api/contracts/tools/index.ts index 69859fe778e..dfc47d74ab8 100644 --- a/apps/sim/lib/api/contracts/tools/index.ts +++ b/apps/sim/lib/api/contracts/tools/index.ts @@ -4,7 +4,6 @@ export * from './communication' export * from './crowdstrike' export * from './custom' export * from './databases' -export * from './docusign' export * from './file' export * from './google' export * from './imap' diff --git a/apps/sim/lib/api/contracts/tools/media/document-parse.ts b/apps/sim/lib/api/contracts/tools/media/document-parse.ts index 39b7e1ed15c..3344a497b5e 100644 --- a/apps/sim/lib/api/contracts/tools/media/document-parse.ts +++ b/apps/sim/lib/api/contracts/tools/media/document-parse.ts @@ -3,7 +3,7 @@ import { resolvedSecretTraceProvenanceSchema } from '@/lib/api/contracts/primiti import { AWS_REGION_PATTERN, toolJsonResponseSchema } from '@/lib/api/contracts/tools/media/shared' import { defineRouteContract } from '@/lib/api/contracts/types' import { RESOLVED_SECRET_PROVENANCE_FIELD } from '@/lib/execution/private-tool-metadata' -import { FileInputSchema, RawFileInputSchema } from '@/lib/uploads/utils/file-schemas' +import { RawFileInputSchema } from '@/lib/uploads/utils/file-schemas' const textractQuerySchema = z.object({ Text: z.string().min(1), @@ -110,19 +110,6 @@ export const textractAnalyzeIdBodySchema = z } }) -export const mistralParseBodySchema = z.object({ - apiKey: z.string().min(1, 'API key is required'), - filePath: z.string().min(1, 'File path is required').optional(), - fileData: FileInputSchema.optional(), - file: FileInputSchema.optional(), - resultType: z.string().optional(), - pages: z.array(z.number()).optional(), - includeImageBase64: z.boolean().optional(), - imageLimit: z.number().optional(), - imageMinSize: z.number().optional(), - [RESOLVED_SECRET_PROVENANCE_FIELD]: resolvedSecretTraceProvenanceSchema.optional(), -}) - export const textractParseContract = defineRouteContract({ method: 'POST', path: '/api/tools/textract/parse', @@ -143,10 +130,3 @@ export const textractAnalyzeIdContract = defineRouteContract({ body: textractAnalyzeIdBodySchema, response: { mode: 'json', schema: toolJsonResponseSchema }, }) - -export const mistralParseContract = defineRouteContract({ - method: 'POST', - path: '/api/tools/mistral/parse', - body: mistralParseBodySchema, - response: { mode: 'json', schema: toolJsonResponseSchema }, -}) diff --git a/apps/sim/lib/api/contracts/tools/media/index.ts b/apps/sim/lib/api/contracts/tools/media/index.ts index d8d3f5817bb..17dcec1dbe8 100644 --- a/apps/sim/lib/api/contracts/tools/media/index.ts +++ b/apps/sim/lib/api/contracts/tools/media/index.ts @@ -1,5 +1,4 @@ export * from '@/lib/api/contracts/tools/media/document-parse' export * from '@/lib/api/contracts/tools/media/image' export * from '@/lib/api/contracts/tools/media/shared' -export * from '@/lib/api/contracts/tools/media/tts' export * from '@/lib/api/contracts/tools/media/video' diff --git a/apps/sim/lib/api/contracts/tools/media/tts.ts b/apps/sim/lib/api/contracts/tools/media/tts.ts deleted file mode 100644 index fdff389ccde..00000000000 --- a/apps/sim/lib/api/contracts/tools/media/tts.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { z } from 'zod' -import { toolJsonResponseSchema } from '@/lib/api/contracts/tools/media/shared' -import { defineRouteContract } from '@/lib/api/contracts/types' - -export const ttsToolBodySchema = z.object({ - text: z.string({ error: 'Missing required parameters' }).min(1, 'Missing required parameters'), - voiceId: z.string({ error: 'Missing required parameters' }).min(1, 'Missing required parameters'), - apiKey: z.string({ error: 'Missing required parameters' }).min(1, 'Missing required parameters'), - modelId: z.string().optional().default('eleven_monolingual_v1'), - stability: z.coerce.number().min(0).max(1).optional(), - similarityBoost: z.coerce.number().min(0).max(1).optional(), - workspaceId: z.string().optional(), - workflowId: z.string().optional(), - executionId: z.string().optional(), -}) - -export const ttsOutputFormatSchema = z.union([z.record(z.string(), z.unknown()), z.string()]) -export const playHtOutputFormatSchema = z.enum(['mp3', 'wav', 'ogg', 'flac', 'mulaw']) - -export const ttsUnifiedToolBodySchema = z - .object({ - provider: z.enum( - ['openai', 'deepgram', 'elevenlabs', 'cartesia', 'google', 'azure', 'playht'], - { - error: 'Missing required fields: provider, text, and apiKey', - } - ), - text: z - .string({ error: 'Missing required fields: provider, text, and apiKey' }) - .min(1, 'Missing required fields: provider, text, and apiKey'), - apiKey: z - .string({ error: 'Missing required fields: provider, text, and apiKey' }) - .min(1, 'Missing required fields: provider, text, and apiKey'), - model: z.enum(['tts-1', 'tts-1-hd', 'gpt-4o-mini-tts']).optional(), - voice: z.string().optional(), - responseFormat: z.enum(['mp3', 'opus', 'aac', 'flac', 'wav', 'pcm']).optional(), - speed: z.coerce.number().optional(), - encoding: z.enum(['linear16', 'mp3', 'opus', 'aac', 'flac', 'mulaw', 'alaw']).optional(), - sampleRate: z.coerce.number().optional(), - bitRate: z.coerce.number().optional(), - container: z.enum(['none', 'wav', 'ogg']).optional(), - voiceId: z.string().optional(), - modelId: z.string().optional(), - stability: z.coerce.number().optional(), - similarityBoost: z.coerce.number().optional(), - style: z.union([z.coerce.number(), z.string()]).optional(), - useSpeakerBoost: z.boolean().optional(), - language: z.string().optional(), - outputFormat: ttsOutputFormatSchema.optional().nullable(), - emotion: z.array(z.string()).optional(), - languageCode: z.string().optional(), - gender: z.enum(['MALE', 'FEMALE', 'NEUTRAL']).optional(), - audioEncoding: z.enum(['LINEAR16', 'MP3', 'OGG_OPUS', 'MULAW', 'ALAW']).optional(), - speakingRate: z.coerce.number().optional(), - pitch: z.union([z.number(), z.string()]).optional(), - volumeGainDb: z.coerce.number().optional(), - sampleRateHertz: z.coerce.number().optional(), - effectsProfileId: z.array(z.string()).optional(), - region: z - .string() - .regex( - /^[a-z][a-z0-9-]{1,30}[a-z0-9]$/, - 'region must be a valid Azure region identifier (e.g. eastus, westeurope)' - ) - .optional(), - rate: z.string().optional(), - styleDegree: z.coerce.number().optional(), - role: z.string().optional(), - userId: z.string().optional(), - quality: z.enum(['draft', 'standard', 'premium']).optional(), - temperature: z.coerce.number().optional(), - voiceGuidance: z.coerce.number().optional(), - textGuidance: z.coerce.number().optional(), - workspaceId: z.string().optional(), - workflowId: z.string().optional(), - executionId: z.string().optional(), - }) - .passthrough() - -export const ttsToolContract = defineRouteContract({ - method: 'POST', - path: '/api/tools/tts', - body: ttsToolBodySchema, - response: { mode: 'json', schema: toolJsonResponseSchema }, -}) - -export const ttsUnifiedToolContract = defineRouteContract({ - method: 'POST', - path: '/api/tools/tts/unified', - body: ttsUnifiedToolBodySchema, - response: { mode: 'json', schema: toolJsonResponseSchema }, -}) diff --git a/apps/sim/lib/api/contracts/types.ts b/apps/sim/lib/api/contracts/types.ts index 713801f75f6..eaaf2d8911b 100644 --- a/apps/sim/lib/api/contracts/types.ts +++ b/apps/sim/lib/api/contracts/types.ts @@ -52,6 +52,33 @@ export type ResponseMode = | StreamResponseMode | RedirectResponseMode +/** + * A contract is consumed in one of two modes, and `method`/`path` only describe + * the first. + * + * **Boundary mode** — the common one. The contract bridges the client/server + * gap: a route builder under `app/api/**` serves `method` at `path`, and + * `requestJson(contract, …)` on the client parses the request out and validates + * the response back. Both sides read the same declaration, so `method` and + * `path` are load-bearing. + * + * **In-process mode.** Tool operations that once self-hopped over HTTP now + * execute in the same process (`lib/internal//execute-tool.ts`), and + * they kept their contract as the input/response schema bundle — + * `parseInternalContractInput` reads only `params`, `query`, and `body`, and + * never looks at `method` or `path`. For these there is no route and no client + * fetch; `method` and `path` are vestigial, describing the HTTP endpoint the + * operation *used* to expose. Do not read them as evidence that an endpoint + * exists, and do not point a client at one. + * + * The distinction is not expressed in the type, so which mode a contract is in + * is derived, never annotated per file — `bun run check:api-contract-routes + * --list-in-process` enumerates the in-process set from the tree rather than + * from a hand-maintained list that would drift. That same audit enforces the + * part which actually matters: an in-process contract may not claim a `path` + * whose live route serves other methods, because a caller trusting the + * declaration gets a 405 rather than an honest 404. + */ export interface ApiRouteContract< TParams extends ApiSchema | undefined = undefined, TQuery extends ApiSchema | undefined = undefined, diff --git a/apps/sim/lib/atlassian/discovery.test.ts b/apps/sim/lib/atlassian/discovery.test.ts index d3c3f4b34bc..d839c95f662 100644 --- a/apps/sim/lib/atlassian/discovery.test.ts +++ b/apps/sim/lib/atlassian/discovery.test.ts @@ -205,6 +205,30 @@ describe('resolveAtlassianCloudId', () => { it('rejects when the token can see no sites', async () => { fetchMock.mockResolvedValue(sites([])) - await expect(resolveAtlassianCloudId(options())).rejects.toThrow('No Jira resources found') + await expect(resolveAtlassianCloudId(options())).rejects.toThrow( + 'No Jira sites are accessible to this credential. Reconnect the credential and grant access to the configured Atlassian site.' + ) + }) + + it('distinguishes a malformed discovery payload from an empty site grant', async () => { + fetchMock.mockResolvedValue(createMockResponse({ json: { id: CLOUD_ID, url: SITE } })) + + await expect(resolveAtlassianCloudId(options())).rejects.toThrow( + 'Invalid Jira accessible-resources response' + ) + }) + + it.each([ + [{ url: SITE }], + [{ id: CLOUD_ID }], + [{ id: '', url: SITE }], + [{ id: CLOUD_ID, url: '' }], + [null], + ])('rejects malformed resource entries in an otherwise valid array', async (resources) => { + fetchMock.mockResolvedValue(createMockResponse({ json: resources })) + + await expect(resolveAtlassianCloudId(options())).rejects.toThrow( + 'Invalid Jira accessible-resources response' + ) }) }) diff --git a/apps/sim/lib/atlassian/discovery.ts b/apps/sim/lib/atlassian/discovery.ts index 5a27b134dc2..c1a55718807 100644 --- a/apps/sim/lib/atlassian/discovery.ts +++ b/apps/sim/lib/atlassian/discovery.ts @@ -102,6 +102,17 @@ interface AccessibleResource { url: string } +function isAccessibleResource(value: unknown): value is AccessibleResource { + if (typeof value !== 'object' || value === null) return false + const resource = value as Record + return ( + typeof resource.id === 'string' && + resource.id.trim().length > 0 && + typeof resource.url === 'string' && + resource.url.trim().length > 0 + ) +} + interface ResolveAtlassianCloudIdOptions { domain: string accessToken: string @@ -203,21 +214,26 @@ export function selectAtlassianCloudId( domain: string, product: string ): string { - if (!Array.isArray(resources) || resources.length === 0) { - throw new Error(`No ${product} resources found`) + if (!Array.isArray(resources) || !resources.every(isAccessibleResource)) { + throw new Error(`Invalid ${product} accessible-resources response`) + } + + if (resources.length === 0) { + throw new Error( + `No ${product} sites are accessible to this credential. ` + + 'Reconnect the credential and grant access to the configured Atlassian site.' + ) } const siteUrl = normalizeAtlassianSiteUrl(domain) - const match = (resources as AccessibleResource[]).find( - (r) => normalizeAtlassianSiteUrl(r.url) === siteUrl - ) + const match = resources.find((r) => normalizeAtlassianSiteUrl(r.url) === siteUrl) if (match) return match.id - if (resources.length === 1) return (resources as AccessibleResource[])[0].id + if (resources.length === 1) return resources[0].id throw new Error( `Could not match ${product} domain "${domain}" to any accessible resource. ` + - `Available sites: ${(resources as AccessibleResource[]).map((r) => r.url).join(', ')}` + `Available sites: ${resources.map((r) => r.url).join(', ')}` ) } diff --git a/apps/sim/lib/internal/confluence/execute-tool.ts b/apps/sim/lib/internal/confluence/execute-tool.ts index 671902f4a8a..acc85ad3225 100644 --- a/apps/sim/lib/internal/confluence/execute-tool.ts +++ b/apps/sim/lib/internal/confluence/execute-tool.ts @@ -1,5 +1,10 @@ import { getErrorMessage } from '@sim/utils/errors' -import type { AnyApiRouteContract, ContractBody, ContractQuery } from '@/lib/api/contracts' +import type { + AnyApiRouteContract, + ApiSchema, + ContractBody, + ContractQuery, +} from '@/lib/api/contracts' import { confluenceBlogPostOperationContract, confluenceCreateCommentContract, @@ -10,7 +15,7 @@ import { confluenceDeleteBlogPostContract, confluenceDeleteCommentContract, confluenceDeleteLabelContract, - confluenceDeletePageContract, + confluenceDeletePageBodySchema, confluenceDeletePagePropertyContract, confluenceDeleteSpaceContract, confluenceGetSpaceContract, @@ -37,7 +42,7 @@ import { confluenceTasksContract, confluenceUpdateBlogPostContract, confluenceUpdateCommentContract, - confluenceUpdatePageContract, + confluenceUpdatePageBodySchema, confluenceUpdateSpaceContract, confluenceUploadAttachmentContract, confluenceUserContract, @@ -94,12 +99,10 @@ import type { type ContractInput = NonNullable | ContractQuery> -function parsePreparedRequest( - contract: C, +function parsePreparedInput( + schema: ApiSchema, request: InternalToolOperationCall -): { success: true; data: ContractInput } | { success: false; response: Response } { - const schema = contract.query ?? contract.body - if (!schema) throw new Error(`Confluence contract ${contract.path} has no request input`) +): { success: true; data: T } | { success: false; response: Response } { const parsed = schema.safeParse(request.input) if (!parsed.success) { return { @@ -110,16 +113,21 @@ function parsePreparedRequest( ), } } - return { success: true, data: parsed.data as ContractInput } + return { success: true, data: parsed.data as T } } -async function executeOperation( - contract: C, +/** + * Operations whose HTTP route was retired hold a bare request schema rather than + * a contract, so they cannot declare a `method` and `path` nothing serves. The + * contract form below feeds this the schema it would have parsed anyway. + */ +async function executeSchemaOperation( + schema: ApiSchema, request: InternalToolOperationCall, - execute: (input: ContractInput, context: ConfluenceOperationContext) => Promise + execute: (input: T, context: ConfluenceOperationContext) => Promise ): Promise { request.signal?.throwIfAborted() - const parsed = parsePreparedRequest(contract, request) + const parsed = parsePreparedInput(schema, request) if (!parsed.success) return parsed.response try { const result = await execute(parsed.data, { @@ -141,6 +149,16 @@ async function executeOperation( } } +function executeOperation( + contract: C, + request: InternalToolOperationCall, + execute: (input: ContractInput, context: ConfluenceOperationContext) => Promise +): Promise { + const schema = contract.query ?? contract.body + if (!schema) throw new Error(`Confluence contract ${contract.path} has no request input`) + return executeSchemaOperation>(schema, request, execute) +} + export const executeConfluenceTool: InternalToolOperationHandler = async (request) => { switch (request.toolId) { case 'confluence_add_label': @@ -194,7 +212,11 @@ export const executeConfluenceTool: InternalToolOperationHandler = async (reques case 'confluence_delete_label': return executeOperation(confluenceDeleteLabelContract, request, executeConfluenceDeleteLabel) case 'confluence_delete_page': - return executeOperation(confluenceDeletePageContract, request, executeConfluenceDeletePage) + return executeSchemaOperation( + confluenceDeletePageBodySchema, + request, + executeConfluenceDeletePage + ) case 'confluence_delete_page_property': return executeOperation( confluenceDeletePagePropertyContract, @@ -327,7 +349,11 @@ export const executeConfluenceTool: InternalToolOperationHandler = async (reques executeConfluenceSearchInSpace ) case 'confluence_update': - return executeOperation(confluenceUpdatePageContract, request, executeConfluenceUpdatePage) + return executeSchemaOperation( + confluenceUpdatePageBodySchema, + request, + executeConfluenceUpdatePage + ) case 'confluence_update_blogpost': return executeOperation( confluenceUpdateBlogPostContract, diff --git a/apps/sim/lib/internal/knowledge/execute-tool.ts b/apps/sim/lib/internal/knowledge/execute-tool.ts index 15f59baeb92..a9fc25818ae 100644 --- a/apps/sim/lib/internal/knowledge/execute-tool.ts +++ b/apps/sim/lib/internal/knowledge/execute-tool.ts @@ -1,8 +1,9 @@ import { createLogger } from '@sim/logger' -import type { AnyApiRouteContract } from '@/lib/api/contracts' +import type { AnyApiRouteContract, ApiSchema } from '@/lib/api/contracts' import { createKnowledgeChunkContract, - createKnowledgeDocumentsContract, + createKnowledgeDocumentsResponseSchema, + createKnowledgeDocumentsSchemas, deleteKnowledgeChunkContract, deleteKnowledgeDocumentContract, getKnowledgeConnectorContract, @@ -36,7 +37,10 @@ import { upsertDocumentOperation, } from '@/lib/internal/knowledge/operations' import { createExecutorPrincipalFromExecutionContext } from '@/lib/internal/principals/executor' -import { parseInternalContractInput } from '@/lib/internal/tool-operations/parse-contract-input' +import { + parseInternalContractInput, + parseInternalOperationInput, +} from '@/lib/internal/tool-operations/parse-contract-input' import type { InternalToolOperationHandler } from '@/lib/internal/tool-operations/types' import { internalKnowledgeErrorPolicies } from '@/lib/knowledge/api/route-policies' import { KNOWLEDGE_DELEGATION_AUDIENCE } from '@/lib/knowledge/application/authorization' @@ -92,6 +96,11 @@ function projectError( ) } +function schemaSuccessResponse(schema: ApiSchema, result: KnowledgeOperationResponse): Response { + const validated = schema.parse(result.body) as Record + return Response.json({ ...validated, ...result.bodyFields }, { headers: result.headers }) +} + function successResponse( contract: C, result: KnowledgeOperationResponse @@ -99,8 +108,7 @@ function successResponse( if (contract.response.mode !== 'json') { throw new Error('Knowledge tool contract must return JSON') } - const validated = contract.response.schema.parse(result.body) as Record - return Response.json({ ...validated, ...result.bodyFields }, { headers: result.headers }) + return schemaSuccessResponse(contract.response.schema, result) } /** Executes every Knowledge tool through the same authorized application use cases as HTTP. */ @@ -132,10 +140,10 @@ export const executeKnowledgeTool: InternalToolOperationHandler = async (request switch (toolId) { case 'knowledge_create_document': { policy = internalKnowledgeErrorPolicies.uploads - const parsed = parseInternalContractInput(createKnowledgeDocumentsContract, input) + const parsed = parseInternalOperationInput(createKnowledgeDocumentsSchemas, input) if (!parsed.success) return parsed.response - return successResponse( - createKnowledgeDocumentsContract, + return schemaSuccessResponse( + createKnowledgeDocumentsResponseSchema, await createDocumentsOperation(parsed.data.params.id, parsed.data.body, context) ) } diff --git a/apps/sim/lib/internal/tool-operations/parse-contract-input.ts b/apps/sim/lib/internal/tool-operations/parse-contract-input.ts index b7fffc5201f..a7432849f14 100644 --- a/apps/sim/lib/internal/tool-operations/parse-contract-input.ts +++ b/apps/sim/lib/internal/tool-operations/parse-contract-input.ts @@ -1,9 +1,11 @@ import type { z } from 'zod' import type { AnyApiRouteContract, + ApiSchema, ContractBody, ContractParams, ContractQuery, + EmptySchemaOutput, } from '@/lib/api/contracts' import { serializeZodIssues } from '@/lib/api/server/validation' @@ -13,6 +15,21 @@ export interface ParsedInternalContractInput { body: B } +/** + * The request slices an in-process operation validates, for an operation whose + * HTTP route has been retired: it passes its schemas directly rather than + * keeping a contract that declares a `method` and `path` nothing serves. + */ +export interface InternalOperationSchemas { + params?: ApiSchema + query?: ApiSchema + body?: ApiSchema +} + +type ParseResult = + | { success: true; data: ParsedInternalContractInput } + | { success: false; response: Response } + function validationError(error: z.ZodError): Response { return Response.json( { error: 'Validation error', details: serializeZodIssues(error) }, @@ -20,16 +37,33 @@ function validationError(error: z.ZodError): Response { ) } +/** + * Contract callers keep their own entry point because `ContractParams` and + * friends `infer` each slice out of the contract's generics. Reading the same + * slices off an optional-property shape widens every one of them with + * `undefined`, which breaks narrowing at every call site. + */ export function parseInternalContractInput( contract: C, input: unknown, options: { maxInputBytes?: number } = {} -): - | { - success: true - data: ParsedInternalContractInput, ContractQuery, ContractBody> - } - | { success: false; response: Response } { +): ParseResult, ContractQuery, ContractBody> { + return parseInternalOperationInput(contract, input, options) as ParseResult< + ContractParams, + ContractQuery, + ContractBody + > +} + +export function parseInternalOperationInput( + schemas: S, + input: unknown, + options: { maxInputBytes?: number } = {} +): ParseResult< + EmptySchemaOutput, + EmptySchemaOutput, + EmptySchemaOutput +> { if (options.maxInputBytes !== undefined) { let serialized: string try { @@ -53,21 +87,21 @@ export function parseInternalContractInput( } } - const params = contract.params?.safeParse(input) + const params = schemas.params?.safeParse(input) if (params && !params.success) return { success: false, response: validationError(params.error) } - const query = contract.query?.safeParse(input) + const query = schemas.query?.safeParse(input) if (query && !query.success) return { success: false, response: validationError(query.error) } - const body = contract.body?.safeParse(input) + const body = schemas.body?.safeParse(input) if (body && !body.success) return { success: false, response: validationError(body.error) } return { success: true, data: { - params: (params?.data ?? undefined) as ContractParams, - query: (query?.data ?? undefined) as ContractQuery, - body: (body?.data ?? undefined) as ContractBody, + params: (params?.data ?? undefined) as EmptySchemaOutput, + query: (query?.data ?? undefined) as EmptySchemaOutput, + body: (body?.data ?? undefined) as EmptySchemaOutput, }, } } diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts index dbb58359347..7ce9da29933 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.test.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.test.ts @@ -2215,6 +2215,30 @@ describe('buildSyncFailureUpdate', () => { expect(buildSyncFailureUpdate(now, undefined, 'boom').nextSyncAt).toEqual(minutesAfter(30)) }) + it('does not schedule before a longer provider retry deadline', async () => { + const { buildSyncFailureUpdate } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(buildSyncFailureUpdate(now, 0, 'rate limited', 45 * 60 * 1000).nextSyncAt).toEqual( + minutesAfter(45) + ) + }) + + it('does not let a shorter provider delay weaken the failure backoff', async () => { + const { buildSyncFailureUpdate } = await import('@/lib/knowledge/connectors/sync-engine') + + expect(buildSyncFailureUpdate(now, 0, 'rate limited', 5 * 60 * 1000).nextSyncAt).toEqual( + minutesAfter(30) + ) + }) + + it('caps an unreasonable provider delay at the existing one-day retry ceiling', async () => { + const { buildSyncFailureUpdate } = await import('@/lib/knowledge/connectors/sync-engine') + + expect( + buildSyncFailureUpdate(now, 0, 'rate limited', 30 * 24 * 60 * 60 * 1000).nextSyncAt + ).toEqual(minutesAfter(24 * 60)) + }) + it('disables exactly at the threshold, not before it', async () => { const { buildSyncFailureUpdate } = await import('@/lib/knowledge/connectors/sync-engine') const { MAX_CONSECUTIVE_FAILURES } = await import('@/lib/knowledge/connectors/sync-limits') diff --git a/apps/sim/lib/knowledge/connectors/sync-engine.ts b/apps/sim/lib/knowledge/connectors/sync-engine.ts index bdca8b716f0..c57713c31e8 100644 --- a/apps/sim/lib/knowledge/connectors/sync-engine.ts +++ b/apps/sim/lib/knowledge/connectors/sync-engine.ts @@ -35,6 +35,7 @@ import { getInternalApiBaseUrl } from '@/lib/core/utils/urls' import { resolveCredentialTokenIdentity } from '@/lib/credentials/access' import { CONNECTOR_AUTO_DISABLED_ERROR, + CONNECTOR_FAILURE_BACKOFF_CAP_MINUTES, connectorFailureBackoffMinutes, MAX_CONSECUTIVE_FAILURES, SYNC_LOCK_HEARTBEAT_INTERVAL_MS, @@ -52,6 +53,7 @@ import { MAX_PROCESSING_ATTEMPTS, QUEUED_DISPATCH_GRACE_MS, } from '@/lib/knowledge/documents/types' +import { getRetryAfterMs } from '@/lib/knowledge/documents/utils' import { refreshAccessTokenIfNeeded } from '@/lib/oauth/credential-service' import { StorageService } from '@/lib/uploads' import { buildStorageKeySegment } from '@/lib/uploads/core/storage-key' @@ -1321,22 +1323,32 @@ export function buildReconciliationHoldNotice( * it applies need to be assertable without standing up the whole sync. The * in-process ladder here and the reaper's SQL ladder must agree — they are two * writers of one policy, both sourced from - * {@link connectorFailureBackoffMinutes}. + * {@link connectorFailureBackoffMinutes}. A validated provider retry delay is + * an additional lower bound, capped at the same one-day ceiling: a short hint + * cannot weaken the failure ladder, while an untrusted extreme value cannot + * pin the connector indefinitely. */ export function buildSyncFailureUpdate( now: Date, previousFailures: number | null | undefined, - errorMessage: string + errorMessage: string, + retryAfterMs?: number ) { const failures = (previousFailures ?? 0) + 1 const disabled = failures >= MAX_CONSECUTIVE_FAILURES + const failureBackoffMs = connectorFailureBackoffMinutes(failures) * 60 * 1000 + const maximumBackoffMs = CONNECTOR_FAILURE_BACKOFF_CAP_MINUTES * 60 * 1000 + const providerBackoffMs = + typeof retryAfterMs === 'number' && Number.isFinite(retryAfterMs) && retryAfterMs > 0 + ? Math.min(retryAfterMs, maximumBackoffMs) + : 0 return { status: (disabled ? 'disabled' : 'error') as 'disabled' | 'error', lastSyncError: disabled ? CONNECTOR_AUTO_DISABLED_ERROR : errorMessage, nextSyncAt: disabled ? null - : new Date(now.getTime() + connectorFailureBackoffMinutes(failures) * 60 * 1000), + : new Date(now.getTime() + Math.max(failureBackoffMs, providerBackoffMs)), consecutiveFailures: failures, // Releases the lock so a stale token can never match a later run, and closes // its lease so the reaper is not left waiting out a TTL on a finished run. @@ -3160,7 +3172,12 @@ export async function executeSync( } const errorMessage = toError(error).message - logger.error('Sync failed', { connectorId, error: errorMessage }) + const retryAfterMs = getRetryAfterMs(error) + logger.error('Sync failed', { + connectorId, + error: errorMessage, + ...(retryAfterMs === undefined ? {} : { retryAfterMs }), + }) try { await completeSyncLog(syncLogId, 'failed', result, { errorMessage }) @@ -3168,7 +3185,12 @@ export async function executeSync( const failureUpdate = error instanceof ConnectorSyncCapacityError ? buildSyncCapacityUpdate(new Date(), connector.consecutiveFailures, errorMessage) - : buildSyncFailureUpdate(new Date(), connector.consecutiveFailures, errorMessage) + : buildSyncFailureUpdate( + new Date(), + connector.consecutiveFailures, + errorMessage, + retryAfterMs + ) if (failureUpdate.status === 'disabled') { logger.warn('Connector disabled after repeated failures', { diff --git a/apps/sim/lib/knowledge/documents/secure-fetch.server.ts b/apps/sim/lib/knowledge/documents/secure-fetch.server.ts index 8c40b863542..818174760af 100644 --- a/apps/sim/lib/knowledge/documents/secure-fetch.server.ts +++ b/apps/sim/lib/knowledge/documents/secure-fetch.server.ts @@ -4,12 +4,9 @@ import { secureFetchWithValidation, } from '@/lib/core/security/input-validation.server' import { - attachRetryHeaders, - type HTTPError, + createRetryableHttpError, isRetryableError, type RetryOptions, - readBoundedHttpErrorBody, - resolveRetryDelayMs, retryWithExponentialBackoff, } from '@/lib/knowledge/documents/utils' @@ -56,17 +53,7 @@ export async function secureFetchWithRetry( * limit) use instead. */ if (!response.ok && isRetryableError({ status: response.status, headers: response.headers })) { - const errorText = await readBoundedHttpErrorBody(response) - const error: HTTPError = new Error(`HTTP ${response.status} - ${errorText}`) - error.status = response.status - attachRetryHeaders(error, response.headers) - - const waitMs = resolveRetryDelayMs(response.headers) - if (waitMs !== undefined) { - error.retryAfterMs = waitMs - } - - throw error + throw await createRetryableHttpError(response) } return response diff --git a/apps/sim/lib/knowledge/documents/utils.test.ts b/apps/sim/lib/knowledge/documents/utils.test.ts index 260743cf5d9..4f8c307a4e1 100644 --- a/apps/sim/lib/knowledge/documents/utils.test.ts +++ b/apps/sim/lib/knowledge/documents/utils.test.ts @@ -14,6 +14,7 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({ import { secureFetchWithRetry } from './secure-fetch.server' import { fetchWithRetry, + getRetryAfterMs, type HTTPError, hasRateLimitEvidence, isRetryableError, @@ -535,13 +536,37 @@ describe('fetchWithRetry rate-limit handling', () => { .mockResolvedValueOnce(response(200)) globalThis.fetch = fetchMock - await expect(fetchWithRetry('https://api.github.com/repos', {}, FAST_RETRY)).rejects.toThrow( - 'HTTP 403' + const error = await fetchWithRetry('https://api.github.com/repos', {}, FAST_RETRY).then( + () => undefined, + (caught) => caught as Error ) + expect(error?.message).toBe('HTTP 403 - upstream rate limit exceeded') + expect(getRetryAfterMs(error)).toBeGreaterThan(899_000) + expect(getRetryAfterMs(error)).toBeLessThanOrEqual(900_000) expect(fetchMock).toHaveBeenCalledTimes(1) }) + it('cancels an omitted rate-limit response body before throwing', async () => { + let cancelled = false + const body = new ReadableStream({ + cancel() { + cancelled = true + }, + }) + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(body, { + status: 429, + headers: { 'retry-after': '900' }, + }) + ) + + await expect( + fetchWithRetry('https://api.github.com/repos', {}, { ...FAST_RETRY, maxRetries: 0 }) + ).rejects.toThrow('HTTP 429 - upstream rate limit exceeded') + expect(cancelled).toBe(true) + }) + it('waits until an admitted x-rate-limit-reset instant before retrying', async () => { vi.useFakeTimers() const now = 1_700_000_000_000 @@ -601,6 +626,20 @@ describe('fetchWithRetry rate-limit handling', () => { }) }) +describe('getRetryAfterMs', () => { + it('finds a validated retry delay through an error cause chain', () => { + const providerError = Object.assign(new Error('rate limited'), { retryAfterMs: 45_000 }) + expect(getRetryAfterMs(new Error('connector failed', { cause: providerError }))).toBe(45_000) + }) + + it.each([undefined, null, 0, -1, Number.NaN, Number.POSITIVE_INFINITY, '30000'])( + 'ignores an invalid retry delay: %s', + (retryAfterMs) => { + expect(getRetryAfterMs(Object.assign(new Error('invalid'), { retryAfterMs }))).toBeUndefined() + } + ) +}) + describe('retryWithExponentialBackoff retry budget', () => { afterEach(() => { vi.useRealTimers() diff --git a/apps/sim/lib/knowledge/documents/utils.ts b/apps/sim/lib/knowledge/documents/utils.ts index 5a54d0d7204..dc61d725553 100644 --- a/apps/sim/lib/knowledge/documents/utils.ts +++ b/apps/sim/lib/knowledge/documents/utils.ts @@ -183,6 +183,30 @@ export function attachRetryHeaders(error: HTTPError, headers: HeaderReader): voi }) } +/** + * Reads a validated provider retry delay from an error or one of its causes. + * + * The HTTP retry layer attaches this value when a provider supplies + * `Retry-After` or an exhausted-quota reset header. Keeping the accessor here + * lets longer-lived schedulers honor the same evidence without depending on a + * concrete error class or parsing a diagnostic message. + */ +export function getRetryAfterMs(error: unknown): number | undefined { + const seen = new Set() + let current = error + + while (current instanceof Error && !seen.has(current) && seen.size < 10) { + seen.add(current) + const retryAfterMs = (current as HTTPError).retryAfterMs + if (typeof retryAfterMs === 'number' && Number.isFinite(retryAfterMs) && retryAfterMs > 0) { + return retryAfterMs + } + current = current.cause + } + + return undefined +} + /** * True when response headers positively identify a rate-limit rejection rather * than an authorization denial. @@ -254,6 +278,52 @@ export function resolveRetryDelayMs( return undefined } +interface RetryableHttpResponse { + status: number + headers: { get(name: string): string | null } + body?: ReadableStream | null + arrayBuffer?: () => Promise + text?: () => Promise +} + +/** Releases a response stream when its provider-controlled body is intentionally omitted. */ +async function cancelHttpResponseBody(response: RetryableHttpResponse): Promise { + if (!response.body) return + try { + await response.body.cancel() + } catch { + return + } +} + +/** + * Builds the bounded error shared by direct and SSRF-safe connector fetches. + * Rate-limit responses are named from trusted status/header evidence while all + * provider-controlled bodies remain omitted. + */ +export async function createRetryableHttpError( + response: RetryableHttpResponse +): Promise { + const rateLimited = + response.status === 429 || (response.status === 403 && hasRateLimitEvidence(response.headers)) + if (rateLimited) { + await cancelHttpResponseBody(response) + } + const diagnostic = rateLimited + ? 'upstream rate limit exceeded' + : await readBoundedHttpErrorBody(response) + const error: HTTPError = new Error(`HTTP ${response.status} - ${diagnostic}`) + error.status = response.status + attachRetryHeaders(error, response.headers) + + const waitMs = resolveRetryDelayMs(response.headers) + if (waitMs !== undefined) { + error.retryAfterMs = waitMs + } + + return error +} + /** * Default retry condition for rate limiting errors */ @@ -471,22 +541,7 @@ export async function fetchWithRetry( const response = await fetch(url, options) if (!response.ok && isRetryableError({ status: response.status, headers: response.headers })) { - const errorText = await readBoundedHttpErrorBody(response) - const error: HTTPError = new Error(`HTTP ${response.status} - ${errorText}`) - error.status = response.status - // The retry loop re-runs the retry condition against this error, so the - // headers must travel with it or a rate-limit 403 would throw immediately. - attachRetryHeaders(error, response.headers) - - // Pass the server-stated wait to the retry loop so it replaces exponential - // backoff. Falls back to the epoch-seconds reset header when the provider - // sends no Retry-After (X never does). - const waitMs = resolveRetryDelayMs(response.headers) - if (waitMs !== undefined) { - error.retryAfterMs = waitMs - } - - throw error + throw await createRetryableHttpError(response) } return response diff --git a/package.json b/package.json index 45d05f955ee..35e5619bd11 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "check": "turbo run format:check", "check:boundaries": "bun run scripts/check-monorepo-boundaries.ts", "check:api-validation": "bun run scripts/check-api-validation-contracts.ts --check", + "check:api-contract-routes": "bun run scripts/check-api-contract-routes.ts", "check:fork-dependent-coverage": "bun run scripts/check-fork-dependent-coverage.ts", "generate:openapi": "bun run scripts/generate-openapi.ts", "check:openapi": "bun run scripts/check-openapi.ts", diff --git a/scripts/check-api-contract-routes.ts b/scripts/check-api-contract-routes.ts new file mode 100644 index 00000000000..7dd5e8b78dd --- /dev/null +++ b/scripts/check-api-contract-routes.ts @@ -0,0 +1,206 @@ +#!/usr/bin/env bun +/** + * Fails when a route contract declares a `method` on a `path` whose route file + * exists but does not export that method. + * + * Contracts are consumed in two modes (see `ApiRouteContract`). A boundary + * contract is served by a route under `app/api/**` and fetched by a client. An + * in-process contract is only an input/response schema bundle for a tool + * operation in `lib/internal//execute-tool.ts`, where `method` and + * `path` are vestigial. + * + * A vestigial path whose route segment no longer exists is harmless: a caller + * gets an honest 404. A vestigial path that still resolves to a live route + * serving *other* methods is not — Next.js answers 405, which reads as "wrong + * verb, endpoint is fine" and sends the caller looking in the wrong place. That + * is the only case this script rejects, so it stays silent on the in-process + * contracts whose routes were deleted outright. + * + * Contracts are read by importing each contract module and inspecting its + * exported objects, the same way `check-route-verbs.ts` resolves the contract + * behind a route. Scanning the source text instead would have to re-implement a + * TypeScript lexer to know which braces are code and which sit inside a string, + * template literal, regex or comment, and it could only ever see contracts whose + * `method`/`path` are inline literals — the 70-plus built through helpers like + * `definePostSelector(path, …)` would be invisible. Route files stay a static + * scan on purpose: importing one drags in `@sim/db`, auth and `next/server`, + * whereas contract modules are pure Zod. + */ +import { existsSync } from 'node:fs' +import { readdir, readFile, stat } from 'node:fs/promises' +import path from 'node:path' + +const ROOT = path.resolve(import.meta.dir, '..') +const CONTRACTS_DIR = path.join(ROOT, 'apps/sim/lib/api/contracts') +const APP_API_DIR = path.join(ROOT, 'apps/sim/app/api') +const SKIP_DIRS = new Set(['node_modules', 'dist', '.next', '.turbo', 'coverage', '__tests__']) +const HTTP_METHODS = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'HEAD', 'OPTIONS'] as const + +type HttpMethod = (typeof HTTP_METHODS)[number] + +interface DeclaredContract { + name: string + method: HttpMethod + routePath: string + module: string +} + +async function listContractModules(dir: string, results: string[] = []): Promise { + for (const entry of await readdir(dir, { withFileTypes: true })) { + if (SKIP_DIRS.has(entry.name)) continue + const full = path.join(dir, entry.name) + if (entry.isDirectory()) await listContractModules(full, results) + else if (/\.tsx?$/.test(entry.name) && !/\.test\.tsx?$/.test(entry.name)) results.push(full) + } + return results +} + +function isRouteContract(value: unknown): value is { method: HttpMethod; path: string } { + if (typeof value !== 'object' || value === null) return false + const candidate = value as Record + return ( + typeof candidate.method === 'string' && + (HTTP_METHODS as readonly string[]).includes(candidate.method) && + typeof candidate.path === 'string' && + typeof candidate.response === 'object' && + candidate.response !== null + ) +} + +async function readIfFile(candidate: string): Promise { + try { + if (!(await stat(candidate)).isFile()) return null + return await readFile(candidate, 'utf8') + } catch { + return null + } +} + +/** + * Resolves a contract path the way Next.js does: an exact segment match wins, + * and only when none exists does the nearest catch-all ancestor + * (`[...all]`, `[[...segments]]`) take the request. Without the fallback every + * path served by a catch-all — all of `/api/auth/**`, `/api/v2/**` without its + * own file — would look routeless and be silently exempted from the check. + */ +async function readRouteFile(routePath: string): Promise { + if (!routePath.startsWith('/api/')) return null + const segments = routePath.slice('/api/'.length).split('/').filter(Boolean) + + const exact = await readIfFile(path.join(APP_API_DIR, ...segments, 'route.ts')) + if (exact !== null) return exact + + for (let depth = segments.length; depth > 0; depth--) { + const ancestor = path.join(APP_API_DIR, ...segments.slice(0, depth - 1)) + if (!existsSync(ancestor)) continue + for (const entry of await readdir(ancestor, { withFileTypes: true })) { + if (!entry.isDirectory()) continue + if (!entry.name.startsWith('[...') && !entry.name.startsWith('[[...')) continue + const source = await readIfFile(path.join(ancestor, entry.name, 'route.ts')) + if (source !== null) return source + } + } + return null +} + +function exportedMethods(source: string): Set { + const methods = new Set() + const group = HTTP_METHODS.join('|') + for (const m of source.matchAll( + new RegExp(`export\\s+(?:const|async\\s+function|function)\\s+(${group})\\b`, 'g') + )) { + methods.add(m[1]) + } + for (const block of source.matchAll(/export\s*(?:const\s*)?\{([^}]*)\}/g)) { + for (const clause of block[1].split(',')) { + const local = clause + .split(/\s+as\s+|:/) + .pop() + ?.trim() + if (local && (HTTP_METHODS as readonly string[]).includes(local)) methods.add(local) + } + } + return methods +} + +async function collectContracts(): Promise { + const modules = await listContractModules(CONTRACTS_DIR) + // Barrels re-export the same object, so keying by identity keeps one entry per + // contract. Defining modules sort before `index.ts` so the report names them. + modules.sort((a, b) => { + const aBarrel = path.basename(a) === 'index.ts' + const bBarrel = path.basename(b) === 'index.ts' + return aBarrel === bBarrel ? a.localeCompare(b) : aBarrel ? 1 : -1 + }) + + const seen = new Map() + for (const file of modules) { + let loaded: Record + try { + loaded = (await import(file)) as Record + } catch (error) { + console.error(`✗ Could not import ${path.relative(ROOT, file)} to read its contracts:`) + console.error(` ${error instanceof Error ? error.message : String(error)}`) + process.exit(1) + } + for (const [name, value] of Object.entries(loaded)) { + if (!isRouteContract(value)) continue + if (seen.has(value)) continue + seen.set(value, { + name, + method: value.method, + routePath: value.path, + module: path.relative(ROOT, file), + }) + } + } + return [...seen.values()] +} + +async function main() { + const contracts = await collectContracts() + + const violations: Array = [] + const inProcess: DeclaredContract[] = [] + for (const contract of contracts) { + const routeSource = await readRouteFile(contract.routePath) + if (routeSource === null) { + inProcess.push(contract) + continue + } + const served = exportedMethods(routeSource) + if (!served.has(contract.method)) violations.push({ ...contract, served: [...served].sort() }) + } + + if (process.argv.includes('--list-in-process')) { + for (const c of [...inProcess].sort((a, b) => a.routePath.localeCompare(b.routePath))) { + console.log(` ${c.method.padEnd(6)} ${c.routePath} ${c.name} (${c.module})`) + } + } + + if (violations.length > 0) { + console.error( + `✗ ${violations.length} contract(s) declare a method their live route does not serve:\n` + ) + for (const v of violations) { + console.error(` ${v.method} ${v.routePath}`) + console.error(` contract: ${v.name} (${v.module})`) + console.error(` route serves: ${v.served.join(', ') || '(no methods)'}`) + console.error( + ` fix: export ${v.method} from the route, or drop the declaration if the endpoint is retired\n` + ) + } + process.exit(1) + } + + console.log( + `✓ ${contracts.length} route contracts agree with the methods their routes serve ` + + `(${contracts.length - inProcess.length} boundary, ${inProcess.length} in-process; ` + + `--list-in-process to enumerate)` + ) +} + +main().catch((error) => { + console.error(error) + process.exit(1) +}) diff --git a/turbo.json b/turbo.json index bdb8730df0f..619997da6ee 100644 --- a/turbo.json +++ b/turbo.json @@ -1,6 +1,23 @@ { "$schema": "https://v2-9-12.turborepo.dev/schema.json", "envMode": "loose", + // Local cache eviction is opt-in until Turborepo 3.0, so without these the + // filesystem cache grows forever. In CI each cache dir is a Blacksmith sticky + // disk that is mounted many times a day, so it never goes idle long enough for + // Blacksmith's own 7-day inactivity purge to fire, and one cache-missing app + // build writes a ~400 MB artifact: the build cache reached 206 GB in 43 days. + // + // Size is the real bound and age is hygiene. A hit only happens when a task's + // input hash is unchanged — a re-run, or a commit touching only unrelated + // workspaces — which recurs within hours, so nothing older than a day or two + // can ever be read again. Measured hit rate on the app build is ~17%, worth + // ~7 minutes each, so the cache earns its keep; it just needs a ceiling. + // 40 GB is ~100 app-sized artifacts against ~4.8 GB/day of real accumulation. + // + // Neither key participates in the task hash, so changing them does not + // invalidate the cache. + "cacheMaxAge": "7d", + "cacheMaxSize": "40GB", "tasks": { "transit": { "dependsOn": ["^transit"],