mirror of
https://github.com/simstudioai/sim.git
synced 2026-08-30 17:05:18 +08:00
improvement(api): retire route contracts that no route serves (#7206)
* fix(api): retire route contracts that no route serves 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) <noreply@anthropic.com> * fix(api): read contracts by import, and retire two more stale declarations 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) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Vikhyath Mondreti
committed by
GitHub
GitHub
parent
4b9c3b78aa
commit
498cdb6af0
@@ -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',
|
||||
|
||||
@@ -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<typeof confluenceUpdatePageBodySchema>
|
||||
export type ConfluenceDeletePageBody = z.output<typeof confluenceDeletePageBodySchema>
|
||||
export const confluenceDeleteAttachmentContract = defineConfluenceDeleteContract(
|
||||
'/api/tools/confluence/attachment',
|
||||
confluenceDeleteAttachmentBodySchema
|
||||
@@ -562,8 +564,6 @@ export const confluenceUserContract = defineConfluencePostContract(
|
||||
|
||||
export type ConfluencePagesBody = ContractBody<typeof confluencePagesSelectorContract>
|
||||
export type ConfluencePageBody = ContractBody<typeof confluencePageSelectorContract>
|
||||
export type ConfluenceUpdatePageBody = ContractBody<typeof confluenceUpdatePageContract>
|
||||
export type ConfluenceDeletePageBody = ContractBody<typeof confluenceDeletePageContract>
|
||||
export type ConfluenceDeleteAttachmentBody = ContractBody<typeof confluenceDeleteAttachmentContract>
|
||||
export type ConfluenceListAttachmentsQuery = ContractQuery<typeof confluenceListAttachmentsContract>
|
||||
export type ConfluenceListBlogPostsQuery = ContractQuery<typeof confluenceListBlogPostsContract>
|
||||
|
||||
@@ -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(),
|
||||
},
|
||||
})
|
||||
@@ -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'
|
||||
|
||||
@@ -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 },
|
||||
})
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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 },
|
||||
})
|
||||
@@ -52,6 +52,33 @@ export type ResponseMode<S extends ApiSchema = ApiSchema> =
|
||||
| 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/<domain>/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,
|
||||
|
||||
@@ -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<C extends AnyApiRouteContract> = NonNullable<ContractBody<C> | ContractQuery<C>>
|
||||
|
||||
function parsePreparedRequest<C extends AnyApiRouteContract>(
|
||||
contract: C,
|
||||
function parsePreparedInput<T>(
|
||||
schema: ApiSchema,
|
||||
request: InternalToolOperationCall
|
||||
): { success: true; data: ContractInput<C> } | { 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<C extends AnyApiRouteContract>(
|
||||
),
|
||||
}
|
||||
}
|
||||
return { success: true, data: parsed.data as ContractInput<C> }
|
||||
return { success: true, data: parsed.data as T }
|
||||
}
|
||||
|
||||
async function executeOperation<C extends AnyApiRouteContract>(
|
||||
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<T>(
|
||||
schema: ApiSchema,
|
||||
request: InternalToolOperationCall,
|
||||
execute: (input: ContractInput<C>, context: ConfluenceOperationContext) => Promise<unknown>
|
||||
execute: (input: T, context: ConfluenceOperationContext) => Promise<unknown>
|
||||
): Promise<Response> {
|
||||
request.signal?.throwIfAborted()
|
||||
const parsed = parsePreparedRequest(contract, request)
|
||||
const parsed = parsePreparedInput<T>(schema, request)
|
||||
if (!parsed.success) return parsed.response
|
||||
try {
|
||||
const result = await execute(parsed.data, {
|
||||
@@ -141,6 +149,16 @@ async function executeOperation<C extends AnyApiRouteContract>(
|
||||
}
|
||||
}
|
||||
|
||||
function executeOperation<C extends AnyApiRouteContract>(
|
||||
contract: C,
|
||||
request: InternalToolOperationCall,
|
||||
execute: (input: ContractInput<C>, context: ConfluenceOperationContext) => Promise<unknown>
|
||||
): Promise<Response> {
|
||||
const schema = contract.query ?? contract.body
|
||||
if (!schema) throw new Error(`Confluence contract ${contract.path} has no request input`)
|
||||
return executeSchemaOperation<ContractInput<C>>(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,
|
||||
|
||||
@@ -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<string, unknown>
|
||||
return Response.json({ ...validated, ...result.bodyFields }, { headers: result.headers })
|
||||
}
|
||||
|
||||
function successResponse<C extends AnyApiRouteContract>(
|
||||
contract: C,
|
||||
result: KnowledgeOperationResponse
|
||||
@@ -99,8 +108,7 @@ function successResponse<C extends AnyApiRouteContract>(
|
||||
if (contract.response.mode !== 'json') {
|
||||
throw new Error('Knowledge tool contract must return JSON')
|
||||
}
|
||||
const validated = contract.response.schema.parse(result.body) as Record<string, unknown>
|
||||
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)
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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<P, Q, B> {
|
||||
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<P, Q, B> =
|
||||
| { success: true; data: ParsedInternalContractInput<P, Q, B> }
|
||||
| { 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<C>` 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<C extends AnyApiRouteContract>(
|
||||
contract: C,
|
||||
input: unknown,
|
||||
options: { maxInputBytes?: number } = {}
|
||||
):
|
||||
| {
|
||||
success: true
|
||||
data: ParsedInternalContractInput<ContractParams<C>, ContractQuery<C>, ContractBody<C>>
|
||||
}
|
||||
| { success: false; response: Response } {
|
||||
): ParseResult<ContractParams<C>, ContractQuery<C>, ContractBody<C>> {
|
||||
return parseInternalOperationInput(contract, input, options) as ParseResult<
|
||||
ContractParams<C>,
|
||||
ContractQuery<C>,
|
||||
ContractBody<C>
|
||||
>
|
||||
}
|
||||
|
||||
export function parseInternalOperationInput<S extends InternalOperationSchemas>(
|
||||
schemas: S,
|
||||
input: unknown,
|
||||
options: { maxInputBytes?: number } = {}
|
||||
): ParseResult<
|
||||
EmptySchemaOutput<S['params']>,
|
||||
EmptySchemaOutput<S['query']>,
|
||||
EmptySchemaOutput<S['body']>
|
||||
> {
|
||||
if (options.maxInputBytes !== undefined) {
|
||||
let serialized: string
|
||||
try {
|
||||
@@ -53,21 +87,21 @@ export function parseInternalContractInput<C extends AnyApiRouteContract>(
|
||||
}
|
||||
}
|
||||
|
||||
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<C>,
|
||||
query: (query?.data ?? undefined) as ContractQuery<C>,
|
||||
body: (body?.data ?? undefined) as ContractBody<C>,
|
||||
params: (params?.data ?? undefined) as EmptySchemaOutput<S['params']>,
|
||||
query: (query?.data ?? undefined) as EmptySchemaOutput<S['query']>,
|
||||
body: (body?.data ?? undefined) as EmptySchemaOutput<S['body']>,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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/<domain>/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<string[]> {
|
||||
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<string, unknown>
|
||||
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<string | null> {
|
||||
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<string | null> {
|
||||
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<string> {
|
||||
const methods = new Set<string>()
|
||||
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<DeclaredContract[]> {
|
||||
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<object, DeclaredContract>()
|
||||
for (const file of modules) {
|
||||
let loaded: Record<string, unknown>
|
||||
try {
|
||||
loaded = (await import(file)) as Record<string, unknown>
|
||||
} 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<DeclaredContract & { served: string[] }> = []
|
||||
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)
|
||||
})
|
||||
Reference in New Issue
Block a user