From b4fb7fa1965b10812055f06d7f58928381bb4d30 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Fri, 28 Aug 2026 00:31:38 -0700 Subject: [PATCH 1/2] fix(api): retire route contracts that no route serves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staging integ alarm fired because the session knowledge-document inline-create check got a 405. #7179 moved tool operations in process and deleted the routes that only existed to serve them, but `createKnowledgeDocumentsContract` kept declaring `POST /api/knowledge/[id]/documents` — a path whose surviving GET/PATCH make Next.js answer POST with 405 rather than an honest 404. Nothing in the repo called it: the KB UI creates documents through the presigned upload flow, and the capability itself is unaffected because `knowledge_create_document` reaches the same use case in process. Audited all 1125 contracts for the same drift. It was the only one whose path resolves to a live route missing the declared method; 259 others declare paths of routes that were deleted outright, which 404 honestly and are left alone. - Drop the create-documents route contract for plain `params`/`body` schemas plus a named response schema, so nothing declares an endpoint we do not serve. The schemas stay in the contracts tree next to the siblings they share (`documentDataSchema` is used by the v2 contracts, and `createKnowledgeDocumentsBodySchema` already backed the in-process operation). - Delete four contracts with no consumer at all — both TTS contracts, docusign, and mistral. Their handlers own better schemas: TTS dispatches by `toolId` with eight per-provider schemas instead of one passthrough superset, and mistral bounds `pages` by the OCR request policy. crowdstrike and windchill look similar but are load-bearing (schema and derived types are imported by live code), so they stay. - Add `check:api-contract-routes`, picked up automatically by `run-audits`. `check:route-verbs` scans routes to contracts, so a contract whose route method was deleted is invisible to it — verified it passes clean against the exact regression this catches. Co-Authored-By: Claude Opus 5 (1M context) --- .../lib/api/contracts/knowledge/documents.ts | 23 ++- apps/sim/lib/api/contracts/tools/docusign.ts | 20 -- apps/sim/lib/api/contracts/tools/index.ts | 1 - .../contracts/tools/media/document-parse.ts | 22 +- .../lib/api/contracts/tools/media/index.ts | 1 - apps/sim/lib/api/contracts/tools/media/tts.ts | 92 --------- apps/sim/lib/api/contracts/types.ts | 27 +++ .../lib/internal/knowledge/execute-tool.ts | 24 ++- .../tool-operations/parse-contract-input.ts | 58 ++++-- package.json | 1 + scripts/check-api-contract-routes.ts | 188 ++++++++++++++++++ 11 files changed, 294 insertions(+), 163 deletions(-) delete mode 100644 apps/sim/lib/api/contracts/tools/docusign.ts delete mode 100644 apps/sim/lib/api/contracts/tools/media/tts.ts create mode 100644 scripts/check-api-contract-routes.ts 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/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/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/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..daefeeb2609 --- /dev/null +++ b/scripts/check-api-contract-routes.ts @@ -0,0 +1,188 @@ +#!/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. + */ +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 + +interface DeclaredContract { + name: string + method: string + routePath: string + file: string + line: number +} + +async function walk(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 walk(full, results) + else if (/\.tsx?$/.test(entry.name) && !/\.test\.tsx?$/.test(entry.name)) results.push(full) + } + return results +} + +/** Reads the balanced object literal passed to each `defineRouteContract(` call. */ +function parseContracts(source: string, file: string): DeclaredContract[] { + const found: DeclaredContract[] = [] + const opener = /defineRouteContract\s*\(\s*\{/g + let match: RegExpExecArray | null + while ((match = opener.exec(source))) { + let cursor = match.index + match[0].length - 1 + const start = cursor + let depth = 0 + for (; cursor < source.length; cursor++) { + const char = source[cursor] + if (char === '{') depth++ + else if (char === '}' && --depth === 0) break + } + const literal = source.slice(start, cursor + 1) + const method = literal.match(/(?:^|[\s,{])method\s*:\s*'([A-Z]+)'/)?.[1] + const routePath = literal.match(/(?:^|[\s,{])path\s*:\s*'([^']+)'/)?.[1] + if (!method || !routePath) continue + const preceding = source.slice(0, match.index) + found.push({ + name: [...preceding.matchAll(/export\s+const\s+([A-Za-z0-9_]+)/g)].pop()?.[1] ?? 'anonymous', + method, + routePath, + file: path.relative(ROOT, file), + line: preceding.split('\n').length, + }) + } + return found +} + +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)) + let entries + try { + entries = await readdir(ancestor, { withFileTypes: true }) + } catch { + continue + } + for (const entry of entries) { + 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 main() { + const contracts: DeclaredContract[] = [] + for (const file of await walk(CONTRACTS_DIR)) { + contracts.push(...parseContracts(await readFile(file, 'utf8'), file)) + } + + 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.file}:${c.line})`) + } + } + + 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.file}:${v.line})`) + 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) +}) From 1bf6017426b91f15c69be8cd3da658a0495bf863 Mon Sep 17 00:00:00 2001 From: Vikhyath Mondreti Date: Fri, 28 Aug 2026 00:46:51 -0700 Subject: [PATCH 2/2] fix(api): read contracts by import, and retire two more stale declarations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Greptile flagged the audit's brace counter as blind to braces inside strings, template literals, regexes and comments. It was, but the bigger problem was that a text scan can only see contracts whose `method`/`path` are inline literals — the 70-plus built through `definePostSelector(path, …)` and friends were never checked at all. Comparing raw `defineRouteContract(` occurrences against parsed ones showed the scanner silently skipping declarations. Read the contracts by importing each contract module and inspecting its exported objects instead, the way `check-route-verbs.ts` already resolves the contract behind a route. Contract modules are pure Zod so importing them is safe; route files stay a static scan because importing one drags in `@sim/db`, auth and `next/server`. Barrels re-export the same object, so entries are keyed by identity. Coverage goes from 1125 contracts to 1283. That immediately surfaced two more instances of exactly what this PR retires. `/api/tools/confluence/page` kept its `PUT` and `DELETE` contracts after #7179 reduced the route to the selector `POST`, so both declared verbs the live route answers with 405. Neither is fetched — `lib/internal/confluence/execute-tool.ts` is the only consumer — so they become plain schemas like the knowledge one, and `executeOperation` now delegates to a schema form rather than growing a second pattern beside it. Co-Authored-By: Claude Opus 5 (1M context) --- .../lib/api/contracts/selectors/confluence.ts | 20 +-- .../lib/internal/confluence/execute-tool.ts | 56 ++++++--- scripts/check-api-contract-routes.ts | 116 ++++++++++-------- 3 files changed, 118 insertions(+), 74 deletions(-) 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/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/scripts/check-api-contract-routes.ts b/scripts/check-api-contract-routes.ts index daefeeb2609..7dd5e8b78dd 100644 --- a/scripts/check-api-contract-routes.ts +++ b/scripts/check-api-contract-routes.ts @@ -15,7 +15,18 @@ * 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' @@ -25,52 +36,35 @@ 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: string + method: HttpMethod routePath: string - file: string - line: number + module: string } -async function walk(dir: string, results: string[] = []): Promise { +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 walk(full, results) + if (entry.isDirectory()) await listContractModules(full, results) else if (/\.tsx?$/.test(entry.name) && !/\.test\.tsx?$/.test(entry.name)) results.push(full) } return results } -/** Reads the balanced object literal passed to each `defineRouteContract(` call. */ -function parseContracts(source: string, file: string): DeclaredContract[] { - const found: DeclaredContract[] = [] - const opener = /defineRouteContract\s*\(\s*\{/g - let match: RegExpExecArray | null - while ((match = opener.exec(source))) { - let cursor = match.index + match[0].length - 1 - const start = cursor - let depth = 0 - for (; cursor < source.length; cursor++) { - const char = source[cursor] - if (char === '{') depth++ - else if (char === '}' && --depth === 0) break - } - const literal = source.slice(start, cursor + 1) - const method = literal.match(/(?:^|[\s,{])method\s*:\s*'([A-Z]+)'/)?.[1] - const routePath = literal.match(/(?:^|[\s,{])path\s*:\s*'([^']+)'/)?.[1] - if (!method || !routePath) continue - const preceding = source.slice(0, match.index) - found.push({ - name: [...preceding.matchAll(/export\s+const\s+([A-Za-z0-9_]+)/g)].pop()?.[1] ?? 'anonymous', - method, - routePath, - file: path.relative(ROOT, file), - line: preceding.split('\n').length, - }) - } - return found +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 { @@ -98,13 +92,8 @@ async function readRouteFile(routePath: string): Promise { for (let depth = segments.length; depth > 0; depth--) { const ancestor = path.join(APP_API_DIR, ...segments.slice(0, depth - 1)) - let entries - try { - entries = await readdir(ancestor, { withFileTypes: true }) - } catch { - continue - } - for (const entry of entries) { + 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')) @@ -134,11 +123,42 @@ function exportedMethods(source: string): Set { return methods } -async function main() { - const contracts: DeclaredContract[] = [] - for (const file of await walk(CONTRACTS_DIR)) { - contracts.push(...parseContracts(await readFile(file, 'utf8'), file)) +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[] = [] @@ -149,14 +169,12 @@ async function main() { continue } const served = exportedMethods(routeSource) - if (!served.has(contract.method)) { - violations.push({ ...contract, served: [...served].sort() }) - } + 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.file}:${c.line})`) + console.log(` ${c.method.padEnd(6)} ${c.routePath} ${c.name} (${c.module})`) } } @@ -166,7 +184,7 @@ async function main() { ) for (const v of violations) { console.error(` ${v.method} ${v.routePath}`) - console.error(` contract: ${v.name} (${v.file}:${v.line})`) + 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`