Add HTTP status code descriptions and enhance response handling in Op…#170
Conversation
…enAPI spec generation
There was a problem hiding this comment.
Pull request overview
This PR extends @cleverbrush/server endpoint typing and @cleverbrush/server-openapi spec generation to better model per-status-code responses, including default HTTP status descriptions and richer handler return typing.
Changes:
- Added
.responses()metadata/type plumbing onEndpointBuilderand updated handler return typing to support per-status-code response unions. - Enhanced
ActionResult/JsonResult/StatusCodeResulttyping to preserve status/body generics and added additional convenience factories. - Updated OpenAPI response generation to support multi-code responses with default status descriptions and auto-added error responses.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/server/src/index.ts | Re-exports new endpoint response typing helpers (AllowedResponseReturn, ResponsesOf). |
| libs/server/src/Server.ts | Updates handle() generic constraint to match the expanded EndpointBuilder type parameters. |
| libs/server/src/Endpoint.ts | Introduces responsesSchemas metadata, .responses() builder API, and handler return-type enforcement for declared status codes. |
| libs/server/src/ActionResult.ts | Adds/strengthens typed ActionResult factories and makes JsonResult/StatusCodeResult generic over status/body. |
| libs/server-openapi/src/generateOpenApiSpec.ts | Generates OpenAPI responses from multi-code metadata and adds default HTTP status descriptions + auto-added error responses. |
Comments suppressed due to low confidence (1)
libs/server/src/Endpoint.ts:382
- The
.body()JSDoc says validation failures return 422, but parameter validation errors are currently serialized as a 400 Problem Details response (resolveArgs()returnscreateValidationProblemDetails(...)andServerwrites400). This comment should be updated to match the actual behavior (and similarly for.query()).
/** Define the request body schema. Validation failures return 422 Problem Details. */
body<TSchema extends SchemaBuilder<any, any, any, any, any>>(
schema: TSchema
): EndpointBuilder<
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| responses< | ||
| const T extends Record< | ||
| number, | ||
| SchemaBuilder<any, any, any, any, any> | null | ||
| > | ||
| >( | ||
| map: T | ||
| ): EndpointBuilder< | ||
| TParams, | ||
| TBody, | ||
| TQuery, | ||
| THeaders, | ||
| TServices, | ||
| TPrincipal, | ||
| TRoles, | ||
| TResponse, | ||
| InferResponsesMap<T> | ||
| > { |
There was a problem hiding this comment.
.responses() accepts an empty object, which results in responsesSchemas being a truthy empty map at runtime. That produces an OpenAPI operation with no successful responses (and HasResponses stays false at the type level because keyof {} is never). Suggest rejecting empty maps (runtime check) and/or tightening the generic to require at least one status-code key so the type-level and runtime behavior stay consistent.
| /** 200 OK — serializes value using content negotiation. */ | ||
| static ok(body: unknown, headers?: Record<string, string>): JsonResult { | ||
| return new JsonResult(body, 200, headers); | ||
| static ok<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<200, T> { | ||
| return new JsonResult(body, 200, headers) as JsonResult<200, T>; | ||
| } | ||
|
|
||
| /** 201 Created — serializes value using content negotiation. */ | ||
| static created( | ||
| body: unknown, | ||
| static created<T>( | ||
| body: T, | ||
| location?: string, | ||
| headers?: Record<string, string> | ||
| ): JsonResult { | ||
| ): JsonResult<201, T> { | ||
| const h: Record<string, string> = { ...headers }; | ||
| if (location) h['location'] = location; | ||
| return new JsonResult(body, 201, h); | ||
| return new JsonResult(body, 201, h) as JsonResult<201, T>; | ||
| } | ||
|
|
||
| /** 202 Accepted — serializes value using content negotiation. */ | ||
| static accepted<T>( | ||
| body: T, | ||
| headers?: Record<string, string> | ||
| ): JsonResult<202, T> { | ||
| return new JsonResult(body, 202, headers) as JsonResult<202, T>; | ||
| } |
There was a problem hiding this comment.
The factory methods ok() / created() / accepted() are documented as using content negotiation, but they construct JsonResult, whose executeAsync() always writes content-type: application/json and ignores the ContentNegotiator. Either adjust the docs to reflect the actual behavior, or introduce a result type that actually uses contentNegotiator.selectResponseHandler() so the comment is true.
| // Minimal inline schema for ProblemDetails error responses | ||
| const PROBLEM_DETAILS_SCHEMA = { | ||
| type: 'object', | ||
| properties: { | ||
| status: { type: 'integer' }, | ||
| title: { type: 'string' }, | ||
| detail: { type: 'string' } | ||
| } |
There was a problem hiding this comment.
PROBLEM_DETAILS_SCHEMA doesn’t match the server’s actual RFC 9457 Problem Details payload (ProblemDetails includes a required type field and may include instance plus extension members like errors). Consider including at least type and allowing additional properties (and/or reusing the ProblemDetails shape from @cleverbrush/server) so the generated OpenAPI schema matches real responses.
| // Minimal inline schema for ProblemDetails error responses | |
| const PROBLEM_DETAILS_SCHEMA = { | |
| type: 'object', | |
| properties: { | |
| status: { type: 'integer' }, | |
| title: { type: 'string' }, | |
| detail: { type: 'string' } | |
| } | |
| // Inline schema for RFC 9457 Problem Details error responses. | |
| // Keep this permissive so extension members (for example `errors`) are | |
| // reflected in the generated OpenAPI schema. | |
| const PROBLEM_DETAILS_SCHEMA = { | |
| type: 'object', | |
| properties: { | |
| type: { type: 'string' }, | |
| title: { type: 'string' }, | |
| status: { type: 'integer' }, | |
| detail: { type: 'string' }, | |
| instance: { type: 'string' } | |
| }, | |
| required: ['type'], | |
| additionalProperties: true |
| // Auto-add framework-generated error responses | ||
| if (meta.bodySchema && !result['422']) { | ||
| result['422'] = { | ||
| description: 'Validation error', | ||
| content: { | ||
| 'application/problem+json': { schema: PROBLEM_DETAILS_SCHEMA } | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
The generator auto-adds a 422 validation error response when meta.bodySchema is set, but the server currently writes validation failures as a 400 Problem Details response (see Server writing 400 on !resolveResult.valid). Also, query/header validation uses the same path, so this should likely trigger when any of bodySchema, querySchema, or headerSchema is present. Please align the auto-added status code/content type/schema with the server’s actual behavior to avoid generating incorrect OpenAPI specs.
| // Multi-code path — .responses() was called | ||
| if (meta.responsesSchemas) { | ||
| for (const [codeStr, schema] of Object.entries(meta.responsesSchemas)) { | ||
| const code = Number(codeStr); | ||
| const desc = | ||
| HTTP_STATUS_DESCRIPTIONS[code] ?? `Response ${codeStr}`; | ||
| if (schema) { | ||
| const jsonSchema = convertSchema(schema); | ||
| const respInfo = schema.introspect() as any; | ||
| const customDesc = | ||
| typeof respInfo.description === 'string' && | ||
| respInfo.description !== '' | ||
| ? respInfo.description | ||
| : desc; | ||
| result[codeStr] = { | ||
| description: customDesc, | ||
| content: { 'application/json': { schema: jsonSchema } } | ||
| }; | ||
| } else { | ||
| result[codeStr] = { description: desc }; | ||
| } | ||
| } | ||
| } else if (meta.responseSchema) { |
There was a problem hiding this comment.
buildResponses() now supports meta.responsesSchemas and adds default descriptions per HTTP status code, but there are no tests covering the multi-code .responses() path (e.g., multiple status entries, null schema for 204, description fallback, precedence over responseSchema, and interaction with the auto-added auth/validation responses). Since this package already has generateOpenApiSpec.test.ts, adding a few focused tests here would prevent regressions.
| export type AllowedResponseReturn<TResponses extends Record<number, any>> = | ||
| | { | ||
| [K in keyof TResponses & number]: TResponses[K] extends null | ||
| ? K extends 204 | ||
| ? NoContentResult | ||
| : StatusCodeResult<K> | ||
| : JsonResult<K, TResponses[K]>; | ||
| }[keyof TResponses & number] |
There was a problem hiding this comment.
AllowedResponseReturn allows NoContentResult for a declared 204: null, but it does not allow StatusCodeResult<204> (e.g. ActionResult.status(204)), even though the server can execute it. This makes valid handler returns fail type-checking. Consider allowing StatusCodeResult<K> for 204 as well (or making NoContentResult extend StatusCodeResult<204>).
…enAPI spec generation
Description
Type of Change
Checklist
npm run lintand fixed any issuesnpm run testand all tests passnpx changeset) if this changes package behavior