Skip to content

feat: add JSON body mode for engine API endpoints#294

Merged
tonyxiao merged 5 commits intov2from
tx/json-body-type-safety
Apr 15, 2026
Merged

feat: add JSON body mode for engine API endpoints#294
tonyxiao merged 5 commits intov2from
tx/json-body-type-safety

Conversation

@tonyxiao
Copy link
Copy Markdown
Collaborator

@tonyxiao tonyxiao commented Apr 15, 2026

Summary

  • Add an application/json request mode for engine POST routes so callers can send typed config in the body (pipeline, optional state, optional input body, and source for discover) while preserving the existing header and NDJSON flows.
  • Update sync-hono-zod-openapi to validate JSON bodies only for exact application/json requests, keep JSON-only routes strict, and preserve correct OpenAPI output for JSON-content headers and mixed request modes.
  • Update sync-ts-cli and regenerate the engine OpenAPI artifacts so header-based CLI usage keeps working when routes also expose JSON body alternatives.

Test plan

  • pnpm --filter @stripe/sync-hono-zod-openapi build
  • pnpm --filter @stripe/sync-hono-zod-openapi test
  • pnpm --filter sync-engine exec vitest run src/api/app.test.ts src/lib/remote-engine.test.ts
  • GitHub Actions checks passed

@tonyxiao tonyxiao force-pushed the tx/json-body-type-safety branch from ce981cb to 63e0309 Compare April 15, 2026 03:27
@tonyxiao tonyxiao marked this pull request as ready for review April 15, 2026 03:36
Copilot AI review requested due to automatic review settings April 15, 2026 03:36
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes OpenAPIHono’s runtime JSON-body validation so it doesn’t eagerly parse/validate JSON when requests are sent with non-JSON Content-Type (e.g. NDJSON), while restoring strict JSON schemas in route definitions for accurate OpenAPI generation and typed c.req.valid('json').

Changes:

  • Unwrap optional/nullable Zod wrappers when extracting header param content-type metadata and descriptions for spec generation.
  • Introduce a content-type-guarded JSON validator middleware and use it for application/json request bodies in OpenAPIHono.
  • Add tests covering JSON vs NDJSON vs missing Content-Type behavior and spec generation for multi-content request bodies.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
packages/hono-zod-openapi/src/index.ts Adds guarded JSON validation and improves optional/nullable header schema introspection for OpenAPI/spec metadata.
packages/hono-zod-openapi/src/tests/json-content-header.test.ts Adds regression tests for content-type-aware JSON body validation and multi-content OpenAPI output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +262 to +268
function contentTypeGuardedJsonValidator(
schema: AnyZod,
hook?: DefaultHook
): MiddlewareHandler {
const jsonValidator = zValidator('json', schema, hook as never)
return async (c, next) => {
if (c.req.header('content-type')?.includes('application/json')) {
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Content-Type detection uses .includes('application/json'), which is case-sensitive and can produce false positives (e.g. application/json-seq would match and re-introduce the parsing/validation failure for streaming JSON types). Consider normalizing and parsing the media type (e.g. split on ;, trim, lowercase) and then doing an exact equality check against application/json.

Suggested change
function contentTypeGuardedJsonValidator(
schema: AnyZod,
hook?: DefaultHook
): MiddlewareHandler {
const jsonValidator = zValidator('json', schema, hook as never)
return async (c, next) => {
if (c.req.header('content-type')?.includes('application/json')) {
function isApplicationJsonContentType(contentType?: string): boolean {
const mediaType = contentType?.split(';', 1)[0]?.trim().toLowerCase()
return mediaType === 'application/json'
}
function contentTypeGuardedJsonValidator(
schema: AnyZod,
hook?: DefaultHook
): MiddlewareHandler {
const jsonValidator = zValidator('json', schema, hook as never)
return async (c, next) => {
if (isApplicationJsonContentType(c.req.header('content-type'))) {

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — middleware now uses isApplicationJsonContentType() which normalizes (lowercase, strip params) and does exact === 'application/json' match.

Comment on lines +286 to +289
const ct = c.req.header('content-type')
if (ct?.includes('application/json')) {
const body = c.req.valid('json')
return c.json({ mode: 'json', pipeline: body.pipeline }, 200)
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test route handler uses ct?.includes('application/json'), which has the same false-positive/case-sensitivity issues as the middleware change. Align this with the production Content-Type parsing logic (exact media type match) and add a regression assertion for a streaming JSON-ish type like application/json-seq (should not trigger JSON body validation).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — test handler now calls the same isApplicationJsonContentType() helper, and we have explicit regression tests for application/json-seq and Application/JSON; charset=utf-8.

Comment on lines +350 to +356
// Crucially, skip JSON body parsing entirely when the request's Content-Type
// is not application/json, so NDJSON/header-only requests aren't affected.
const jsonSchema = op.requestBody?.content?.['application/json']?.schema
if (jsonSchema instanceof Object && 'parse' in (jsonSchema as object)) {
middlewares.push(zValidator('json', jsonSchema as AnyZod, this._defaultHook as never))
middlewares.push(
contentTypeGuardedJsonValidator(jsonSchema as AnyZod, this._defaultHook)
)
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applying the content-type guard unconditionally changes behavior for JSON-only endpoints: requests with a missing/incorrect Content-Type will now skip validation entirely, but many handlers in this repo call c.req.valid('json') unconditionally (expecting the validator to have run). Consider only using the guarded validator when the route actually supports additional non-JSON content types, and otherwise keep strict JSON validation (or explicitly return 415/400 when a non-JSON Content-Type is sent to a JSON-only route).

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — we now split into strictJsonBodyValidator (JSON-only routes) vs contentTypeGuardedJsonValidator (mixed-content / header-alternative routes). The choice is made at route registration time based on hasNonJsonRequestBody || hasJsonHeaderAlternatives.

@tonyxiao tonyxiao force-pushed the tx/json-body-type-safety branch from 63e0309 to ce981cb Compare April 15, 2026 03:42
Reintroduce application/json config bodies for the engine API while keeping
legacy header and NDJSON flows working. Fix OpenAPIHono and the OpenAPI-based
CLI generation so typed header schemas, streaming endpoints, and connector
loading all continue to work with the new spec.

Made-with: Cursor
Committed-By-Agent: cursor
@tonyxiao tonyxiao force-pushed the tx/json-body-type-safety branch from ce981cb to 29cb2c7 Compare April 15, 2026 03:51
tonyxiao pushed a commit that referenced this pull request Apr 15, 2026
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Move JSON-vs-header requiredness into shared helpers so handlers stop
merging two conditional code paths into nullable locals.

Made-with: Cursor
Committed-By-Agent: cursor
Keep JSON validation strict for JSON routes while avoiding false positives for NDJSON and JSON-header alternatives.

Made-with: Cursor
Committed-By-Agent: cursor
const hookResult = await hook({ data: value, ...result, target: 'json' }, c)
if (hookResult) {
if (hookResult instanceof Response) return hookResult
if (typeof hookResult === 'object' && hookResult !== null && 'response' in hookResult) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional — typeof null === 'object' in JS, so the !== null guard is necessary before the 'response' in hookResult check. Not a real type error.

@tonyxiao tonyxiao changed the title fix: content-type-aware JSON body validation in OpenAPIHono feat: add JSON body mode for engine API endpoints Apr 15, 2026
Export isApplicationJsonContentType from hono-zod-openapi and reuse it
in app.ts so handler and middleware agree on what counts as JSON body
mode. Fixes mixed-case and json-seq false positives/negatives.

Made-with: Cursor
Committed-By-Agent: cursor
Made-with: Cursor
Committed-By-Agent: cursor

# Conflicts:
#	apps/engine/src/api/app.ts
@tonyxiao tonyxiao merged commit 3bf414e into v2 Apr 15, 2026
19 checks passed
@tonyxiao tonyxiao deleted the tx/json-body-type-safety branch April 15, 2026 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants