Skip to content

Conversation

apricot13
Copy link

@apricot13 apricot13 commented Sep 15, 2025

This afternoon we noticed an issue where header validation was being overwritten by query validation. Specifically:

  • When both header and query validation was defined, the query values overwrote the headers.
  • When only headers were provided, they were ignored entirely.
  • It appears in 1.4.* but not in 1.3.*.

This PR fixes the issue and adds targeted tests to prevent regression.

Steps to replicate

import { Elysia, t } from "elysia";
import openapi from "@elysiajs/openapi";

const app = new Elysia()
  .use(openapi())
  .get("/", () => "hello", {
    headers: t.Object({
      "test-header": t.String(),
    }),
    query: t.Object({
      page: t.String(),
      resultsPerPage: t.String(),
    }),
    response: t.String(),
  })
  .listen(3000);

console.log(
  `🦊 Elysia is running at ${app.server?.hostname}:${app.server?.port}`
);
{
  "name": "app",
  "version": "1.0.50",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "dev": "bun run --watch src/index.ts"
  },
  "dependencies": {
    "elysia": "1.4.5"
    "@elysiajs/openapi": "1.4.2"
  },
  "devDependencies": {
    "bun-types": "latest"
  },
  "module": "src/index.js"
}
image

Summary by CodeRabbit

  • Bug Fixes

    • Corrected OpenAPI generation to derive header parameters from the header schema, ensuring headers appear with in: "header" and proper required handling.
  • Tests

    • Added integration test verifying parameter locations (header, path, query, cookie) in generated OpenAPI for Elysia routes.

Copy link

coderabbitai bot commented Sep 15, 2025

Walkthrough

Fixes header parameter extraction in toOpenAPISchema to use hooks.headers instead of hooks.query. Adds a test that builds an Elysia app and asserts generated OpenAPI parameters are correctly placed in header, path, query, and cookie.

Changes

Cohort / File(s) Summary
OpenAPI header handling fix
src/openapi.ts
Corrects header parameter source by unwrapping from hooks.headers rather than hooks.query; no other logic modified.
OpenAPI schema generation test
test/openapi.test.ts
Adds test constructing an Elysia app and verifying OpenAPI parameter locations (header, path, query, cookie); updates imports to include Elysia, t, and toOpenAPISchema.

Sequence Diagram(s)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I hop through headers, neat and bright,
Unwrap the right nest, set paths to light.
Cookies crumb, queries queue,
Params in line, all labeled true.
With tidy paws, I sign the spec—
A carrot check for each request.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately and concisely summarizes the primary change — fixing headers being overwritten by query — and directly reflects the modifications in src/openapi.ts plus the added tests that ensure header parameters are preserved; it is clear, specific, and appropriate for a teammate scanning PR history.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
test/openapi.test.ts (2)

17-66: Strengthen assertions and add a guard for the “headers-only” case

Add checks for required flags and ensure GET has no requestBody. Also add a dedicated test where only headers are provided to prevent regressions reported in 1.4.*.

Apply:

@@
 describe('Convert Elysia routes to OpenAPI 3.0.3 paths schema', () => {
   describe('with path, header, query and cookie params', () => {
@@
-    it('marks each parameter with the correct OpenAPI parameter location', () => {
+    it('marks each parameter with the correct OpenAPI parameter location', () => {
       const map = Object.fromEntries(
         parameters.map((p: any) => [p.name, p.in])
       )
       expect(map).toMatchObject({
         testheader: 'header',
         testparam: 'path',
         testquery: 'query',
         testcookie: 'cookie'
       })
     })
+
+    it('marks parameters as required based on schema', () => {
+      const reqMap = Object.fromEntries(
+        parameters.map((p: any) => [p.name, Boolean(p.required)])
+      )
+      expect(reqMap).toMatchObject({
+        testheader: true,
+        testparam: true,
+        testquery: true,
+        testcookie: true
+      })
+    })
+
+    it('does not emit requestBody for GET', () => {
+      expect(path?.get?.requestBody).toBeUndefined()
+    })
   })
+
+  describe('with only header params', () => {
+    const app2 = new Elysia().get('/only-header', () => 'hi', {
+      headers: t.Object({ only: t.String() })
+    })
+    const {
+      paths: { ['/only-header']: onlyHeaderPath }
+    } = toOpenAPISchema(app2)
+
+    it('includes header parameters even when no query is present', () => {
+      const params = onlyHeaderPath?.get?.parameters ?? []
+      const names = params.map((p: any) => p.name)
+      expect(names).toEqual(expect.arrayContaining(['only']))
+      expect(params.find((p: any) => p.name === 'only')?.in).toBe('header')
+    })
+  })
 })

6-14: Optional: de-duplicate getPossiblePath outputs

The helper currently returns duplicates (e.g., '/user/name' twice). If stability matters, consider returning unique paths and adjusting this expectation accordingly.

Proposed change in src/openapi.ts:

 export const getPossiblePath = (path: string): string[] => {
   const optionalParams = path.match(optionalParamsRegex)
   if (!optionalParams) return [path]

   const originalPath = path.replace(/\?/g, '')
-  const paths = [originalPath]
+  const paths = new Set<string>([originalPath])

   for (let i = 0; i < optionalParams.length; i++) {
     const newPath = path.replace(optionalParams[i], '')
-    paths.push(...getPossiblePath(newPath))
+    for (const p of getPossiblePath(newPath)) paths.add(p)
   }

-  return paths
+  return [...paths]
 }

And update the test expectation to remove the duplicate '/user/name'.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 480f7f7 and 1c8020c.

📒 Files selected for processing (2)
  • src/openapi.ts (1 hunks)
  • test/openapi.test.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/openapi.test.ts (1)
src/openapi.ts (1)
  • toOpenAPISchema (105-498)
🔇 Additional comments (1)
src/openapi.ts (1)

245-261: Fix correctly sources header schema from hooks.headers

This resolves the overwrite bug where headers were derived from query. Good catch.

@apricot13 apricot13 force-pushed the fix/headers-overwritten-with-query branch from 1c8020c to 52bb50f Compare September 19, 2025 14:21
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.

1 participant