Skip to content

Commit f2f8311

Browse files
committed
refactor(v2): derive the body 413 from the contract in every document
Two mechanisms encoded one rule. `withRequestBodyErrors` derived the 413 from `route.contract.body` for the tables document, while the resources document hand-picked RESOURCE_BODY_ERRORS / RESOURCE_CONFLICT_BODY_ERRORS at nine sites. The cross-document sweep caught drift, but only after the fact: a new body operation that forgot the _BODY_ variant published a reachable 413 nowhere until a test failed. Hoists the mapper to openapi/shared.ts and applies it in both documents, so the rule is derived rather than remembered. The two hand-picked sets and their shared TSDoc are gone. Regenerating all seven specs produces zero drift, which is the proof the two mechanisms were computing the same thing.
1 parent 28ccfb3 commit f2f8311

3 files changed

Lines changed: 29 additions & 57 deletions

File tree

apps/sim/lib/api/contracts/v2/openapi/resources.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,14 @@ import {
2121
FULL_SET_LIST,
2222
HEAD_MIRRORS_GET,
2323
RATE_LIMIT_HEADERS,
24-
RESOURCE_BODY_ERRORS,
25-
RESOURCE_CONFLICT_BODY_ERRORS,
2624
RESOURCE_CONFLICT_ERRORS,
2725
RESOURCE_ERRORS,
2826
V2_API_KEY_SECURITY,
2927
V2_API_KEY_SECURITY_SCHEMES,
3028
V2_COMMON_HEADERS,
3129
V2_ERROR_SCHEMA,
3230
WORKSPACE_API_KEY_DENIED,
31+
withRequestBodyErrors,
3332
} from '@/lib/api/contracts/v2/openapi/shared'
3433
import {
3534
v2DeleteSecretContract,
@@ -214,7 +213,7 @@ function resourceOperation(
214213
}
215214
}
216215

217-
const routes = [
216+
const declaredRoutes = [
218217
defineOpenApiRoute(
219218
v2GetWorkspaceContract,
220219
resourceOperation('Workspaces', {
@@ -307,7 +306,7 @@ const routes = [
307306
summary: 'Create MCP Server',
308307
description:
309308
'Register an MCP server in a workspace. The endpoint URL is the server identity, so a URL already registered here is a `409` — reconfigure that server with `PATCH /api/v2/mcp-servers/{id}` instead. Registration never connects to the endpoint: the server comes back `disconnected` and stays unavailable until `GET /api/v2/mcp-servers/{id}/tools` succeeds.',
310-
errors: RESOURCE_CONFLICT_BODY_ERRORS,
309+
errors: RESOURCE_CONFLICT_ERRORS,
311310
success: { description: 'The MCP server was registered.' },
312311
}),
313312
{
@@ -375,7 +374,7 @@ const routes = [
375374
summary: 'Update MCP Server',
376375
description:
377376
'Update the supplied MCP server fields. Omitted fields are retained, except where a field says otherwise. Any change that invalidates authentication revokes the stored OAuth grant, resets `connectionStatus` to `disconnected`, and clears `lastConnected` and `lastError`, so the server must be rediscovered.',
378-
errors: RESOURCE_BODY_ERRORS,
377+
errors: RESOURCE_ERRORS,
379378
success: { description: 'The updated MCP server.' },
380379
}),
381380
{
@@ -497,7 +496,7 @@ const routes = [
497496
operationId: 'createSkill',
498497
summary: 'Create Skill',
499498
description: `Create one skill in a workspace. Its kebab-case name must be unique and cannot be reserved by a built-in skill. ${WORKSPACE_API_KEY_DENIED}`,
500-
errors: RESOURCE_CONFLICT_BODY_ERRORS,
499+
errors: RESOURCE_CONFLICT_ERRORS,
501500
success: { description: 'The skill was created.' },
502501
}),
503502
{
@@ -563,7 +562,7 @@ const routes = [
563562
operationId: 'updateSkill',
564563
summary: 'Update Skill',
565564
description: `Update the supplied fields on a workspace skill. Omitted fields retain their stored values. Built-in skills are read-only. ${WORKSPACE_API_KEY_DENIED}`,
566-
errors: RESOURCE_CONFLICT_BODY_ERRORS,
565+
errors: RESOURCE_CONFLICT_ERRORS,
567566
success: { description: 'The updated skill.' },
568567
}),
569568
{
@@ -654,7 +653,7 @@ const routes = [
654653
summary: 'Create Custom Tool',
655654
description:
656655
'Create a code-backed custom tool in a workspace. Its title must be unique because tools resolve by title at call time.',
657-
errors: RESOURCE_CONFLICT_BODY_ERRORS,
656+
errors: RESOURCE_CONFLICT_ERRORS,
658657
success: { description: 'The custom tool was created.' },
659658
}),
660659
{
@@ -720,7 +719,7 @@ const routes = [
720719
summary: 'Update Custom Tool',
721720
description:
722721
'Update the supplied custom tool fields. Omitted fields retain their stored values, and titles must remain unique within the workspace.',
723-
errors: RESOURCE_CONFLICT_BODY_ERRORS,
722+
errors: RESOURCE_CONFLICT_ERRORS,
724723
success: { description: 'The updated custom tool.' },
725724
}),
726725
{
@@ -836,7 +835,7 @@ const routes = [
836835
operationId: 'setSecret',
837836
summary: 'Set Secret',
838837
description: `Create or replace a workspace or caller-owned personal secret. The value is encrypted at rest, is write-only, and is never included in the response. ${WORKSPACE_API_KEY_DENIED}`,
839-
errors: RESOURCE_BODY_ERRORS,
838+
errors: RESOURCE_ERRORS,
840839
success: {
841840
byStatus: {
842841
200: { description: 'The existing secret value was replaced.' },
@@ -915,6 +914,8 @@ const routes = [
915914
),
916915
] as const
917916

917+
const routes = declaredRoutes.map(withRequestBodyErrors)
918+
918919
export const resourcesOpenApiDocument = defineOpenApiDocument({
919920
output: 'apps/docs/openapi-v2-resources.json',
920921
info: {

apps/sim/lib/api/contracts/v2/openapi/shared.ts

Lines changed: 17 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { v2ErrorResponseSchema } from '@/lib/api/contracts/v2/shared'
33
import type {
44
OpenApiErrorResponse,
55
OpenApiHeader,
6+
OpenApiRouteDefinition,
67
OpenApiSecurityScheme,
78
} from '@/lib/api/openapi/types'
89

@@ -153,33 +154,25 @@ export const RESOURCE_MUTATION_ERRORS = [
153154
] as const satisfies readonly ErrorResponseId[]
154155

155156
/**
156-
* The two sets below add the `413` that every body-carrying operation can emit.
157+
* Adds the `413` a body-carrying operation can emit.
157158
*
158-
* It is not a property of the resource but of the request: `parseRequest` reads
159-
* the JSON body through `parseJsonBody` under `DEFAULT_MAX_JSON_BODY_BYTES`
160-
* before schema validation runs, and the v2 builders supply
161-
* `V2_PARSE_DEFAULTS.payloadTooLargeResponse`, so a body over
162-
* the cap is answered `413` on any route whose contract declares one. That made
163-
* `413` reachable-but-unpublished across a whole family, which is the mirror of
164-
* the defect these sets exist to prevent — a caller cannot handle a status the
165-
* spec never mentions.
159+
* `parseRequest` reads the JSON body under `DEFAULT_MAX_JSON_BODY_BYTES` before
160+
* schema validation, and the v2 builders supply
161+
* `V2_PARSE_DEFAULTS.payloadTooLargeResponse`, so an oversized body is a real
162+
* `413` on any route whose contract declares one — and a status a caller can
163+
* receive but the spec omits is an unhandled branch in every generated client.
166164
*
167-
* Reachability is not automatic, so these are opt-in rather than folded into the
168-
* base sets. An operation with no request body cannot emit this `413` at all,
169-
* and neither can one whose handler reads its payload through a path that
170-
* applies no cap; documenting it there would publish a response that can never
171-
* arrive.
165+
* Derived from the contract rather than chosen per operation, so a new body
166+
* route cannot forget it. One-directional: it never removes a `413` from a
167+
* bodyless read, several of which publish one for the folder-tree ceiling.
172168
*/
173-
export const RESOURCE_BODY_ERRORS = [
174-
...RESOURCE_ERRORS,
175-
'PayloadTooLarge',
176-
] as const satisfies readonly ErrorResponseId[]
177-
178-
/** {@link RESOURCE_CONFLICT_ERRORS} plus the body-size `413`. */
179-
export const RESOURCE_CONFLICT_BODY_ERRORS = [
180-
...RESOURCE_CONFLICT_ERRORS,
181-
'PayloadTooLarge',
182-
] as const satisfies readonly ErrorResponseId[]
169+
export function withRequestBodyErrors(route: OpenApiRouteDefinition): OpenApiRouteDefinition {
170+
if (!route.contract.body || route.operation.errors.includes('PayloadTooLarge')) return route
171+
return {
172+
...route,
173+
operation: { ...route.operation, errors: [...route.operation.errors, 'PayloadTooLarge'] },
174+
}
175+
}
183176

184177
export const V2_API_KEY_SECURITY = [{ apiKey: [] }] as const
185178

apps/sim/lib/api/contracts/v2/openapi/tables.ts

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
V2_COMMON_HEADERS,
1414
V2_ERROR_SCHEMA,
1515
WORKSPACE_ERRORS,
16+
withRequestBodyErrors,
1617
} from '@/lib/api/contracts/v2/openapi/shared'
1718
import {
1819
v2AddTableColumnContract,
@@ -64,7 +65,6 @@ import {
6465
defineOpenApiDocument,
6566
defineOpenApiRoute,
6667
type OpenApiOperationMetadata,
67-
type OpenApiRouteDefinition,
6868
type OpenApiSuccessMetadata,
6969
} from '@/lib/api/openapi/types'
7070

@@ -1580,28 +1580,6 @@ const declaredRoutes = [
15801580
),
15811581
] as const
15821582

1583-
/**
1584-
* Publishes `413` on every operation that accepts a request body.
1585-
*
1586-
* The v2 JSON builder reads the body through `parseJsonBody` under
1587-
* `DEFAULT_MAX_JSON_BODY_BYTES` (50 MB) BEFORE schema validation, rendering
1588-
* `V2_PARSE_DEFAULTS.payloadTooLargeResponse`. That makes the status reachable
1589-
* on every body-carrying operation, not just the two that set a tighter
1590-
* `maxBodyBytes` of their own — and a status a caller can receive but the spec
1591-
* does not declare is an unhandled branch in every generated client.
1592-
*
1593-
* Deliberately one-directional: it adds `413` where a body exists and never
1594-
* removes it where none does, because several bodyless reads publish `413` for
1595-
* the folder-tree materialization ceiling instead.
1596-
*/
1597-
function withRequestBodyErrors(route: OpenApiRouteDefinition): OpenApiRouteDefinition {
1598-
if (!route.contract.body || route.operation.errors.includes('PayloadTooLarge')) return route
1599-
return {
1600-
...route,
1601-
operation: { ...route.operation, errors: [...route.operation.errors, 'PayloadTooLarge'] },
1602-
}
1603-
}
1604-
16051583
const routes = declaredRoutes.map(withRequestBodyErrors)
16061584

16071585
export const tablesOpenApiDocument = defineOpenApiDocument({

0 commit comments

Comments
 (0)