fix(server-hono): reuse active Zod instance for Swagger schemas#1224
Merged
fix(server-hono): reuse active Zod instance for Swagger schemas#1224
Conversation
🦋 Changeset detectedLatest commit: be34b04 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Closed
This comment has been minimized.
This comment has been minimized.
Contributor
📝 WalkthroughWalkthroughA factory function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Deploying voltagent with
|
| Latest commit: |
be34b04
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://85b258a9.voltagent.pages.dev |
| Branch Preview URL: | https://fix-swagger-openapi-fallback.voltagent.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/server-hono/src/utils/custom-endpoints.ts (2)
312-379: Fallback reconstruction logic looks solid.The branch ordering (built-ins first, then
extractCustomEndpoints), theseenRoutesdedup, and theseenRoutes.size > 0 ? fallbackDoc : baseDocguard behave sensibly, and the try/catch keeps/docresponsive even on a badapp.routesshape. Two small observations:
- Line 336:
toOpenApiPath(routePath) === "/ui"—/uihas no:param, so the conversion is a no-op;routePath === "/ui"would read clearer and could be folded into the guard on line 332.- Line 285 (in
applyRouteDefinitionMetadata, referenced here for context): assigningoperation.description = route.descriptionwhenroute.descriptionis undefined will wipe the descriptive placeholder set byaddEndpointToDoc. Considerif (route.description) operation.description = route.description;for parity with theoperationIdhandling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-hono/src/utils/custom-endpoints.ts` around lines 312 - 379, The toOpenApiPath("/ui") check in getFallbackOpenApiDocFromRoutes is redundant and should be replaced by checking the raw routePath (fold into the existing guard that skips "*", "/", "/doc") — update the conditional that currently reads toOpenApiPath(routePath) === "/ui" to instead check routePath === "/ui" and merge it with the earlier guard; also update applyRouteDefinitionMetadata so it does not overwrite placeholder descriptions from addEndpointToDoc by changing the unconditional assignment operation.description = route.description to only assign when route.description is defined (e.g. if (route.description) operation.description = route.description), keeping the existing operationId handling behavior for parity.
201-256: Consider normalizing the path key to OpenAPI{param}form insideaddEndpointToDoc.The helper uses the raw
pathas thedoc.pathskey (line 212) but derives parameters from the convertedpathWithBraces(line 241). When this helper is reached viagetEnhancedOpenApiDoc→customEndpoints.forEach(... addEndpointToDoc(fullDoc, endpoint))for a custom Hono route like/users/:id, the resulting OpenAPI document getspaths["/users/:id"]with a parameter namedid— a non-compliant template that Swagger UI may render with the literal colon. The fallback path avoids this only because it passesopenApiPathexplicitly. Normalizing once in the helper keeps both call sites correct:♻️ Proposed refactor
-function addEndpointToDoc(doc: any, endpoint: ServerEndpointSummary, pathOverride?: string) { - const path = pathOverride ?? endpoint.path; - const method = endpoint.method.toLowerCase(); +function addEndpointToDoc(doc: any, endpoint: ServerEndpointSummary, pathOverride?: string) { + const rawPath = pathOverride ?? endpoint.path; + const path = toOpenApiPath(rawPath); + const method = endpoint.method.toLowerCase(); @@ - const pathWithBraces = toOpenApiPath(path); - if (pathWithBraces.includes("{")) { - const params = pathWithBraces.match(/\{([^}]+)\}/g); + if (path.includes("{")) { + const params = path.match(/\{([^}]+)\}/g);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-hono/src/utils/custom-endpoints.ts` around lines 201 - 256, In addEndpointToDoc: normalize the paths key to OpenAPI brace form by computing const openApiPath = toOpenApiPath(path) (or use pathOverride if it was already intended), and use openApiPath when reading/creating doc.paths (replace doc.paths[path] and pathObj derived from it) and when building the summary/description/parameters; keep extracting parameters from openApiPath (previously pathWithBraces) and preserve existing tags/description logic but reference openApiPath for the doc.paths key and any path-display text so the stored path uses {param} syntax consistently.packages/server-hono/src/app-factory.spec.ts (1)
287-300: Test correctly exercises the fallback path.Overriding
app.getOpenAPIDocumentto throw is a clean way to simulate the schema-generation failure in#1222, and asserting both/agentsand the:name→{name}conversion covers the two behaviors called out in the PR description. Optional: add a custom route viaconfigureAppand assert it also appears in the fallback doc — that would protect against regressions in the custom-endpoint branch ofgetFallbackOpenApiDocFromRoutes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server-hono/src/app-factory.spec.ts` around lines 287 - 300, The test should also verify the fallback behavior for custom routes: update the spec that sets app.getOpenAPIDocument to throw (in the test using createApp and createDeps) to pass a configureApp option that registers a custom route (via the configureApp callback used by createApp), then assert the fallback OpenAPI doc (returned for /doc) contains that custom path alongside "/agents" and the TOOL_ROUTES entries; this ensures getFallbackOpenApiDocFromRoutes includes custom endpoints and protects the custom-endpoint branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/server-hono/src/app-factory.spec.ts`:
- Around line 287-300: The test should also verify the fallback behavior for
custom routes: update the spec that sets app.getOpenAPIDocument to throw (in the
test using createApp and createDeps) to pass a configureApp option that
registers a custom route (via the configureApp callback used by createApp), then
assert the fallback OpenAPI doc (returned for /doc) contains that custom path
alongside "/agents" and the TOOL_ROUTES entries; this ensures
getFallbackOpenApiDocFromRoutes includes custom endpoints and protects the
custom-endpoint branch.
In `@packages/server-hono/src/utils/custom-endpoints.ts`:
- Around line 312-379: The toOpenApiPath("/ui") check in
getFallbackOpenApiDocFromRoutes is redundant and should be replaced by checking
the raw routePath (fold into the existing guard that skips "*", "/", "/doc") —
update the conditional that currently reads toOpenApiPath(routePath) === "/ui"
to instead check routePath === "/ui" and merge it with the earlier guard; also
update applyRouteDefinitionMetadata so it does not overwrite placeholder
descriptions from addEndpointToDoc by changing the unconditional assignment
operation.description = route.description to only assign when route.description
is defined (e.g. if (route.description) operation.description =
route.description), keeping the existing operationId handling behavior for
parity.
- Around line 201-256: In addEndpointToDoc: normalize the paths key to OpenAPI
brace form by computing const openApiPath = toOpenApiPath(path) (or use
pathOverride if it was already intended), and use openApiPath when
reading/creating doc.paths (replace doc.paths[path] and pathObj derived from it)
and when building the summary/description/parameters; keep extracting parameters
from openApiPath (previously pathWithBraces) and preserve existing
tags/description logic but reference openApiPath for the doc.paths key and any
path-display text so the stored path uses {param} syntax consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28a71899-03df-405b-8f1a-4ff0c7ec1ac8
📒 Files selected for processing (3)
.changeset/smart-swagger-paths.mdpackages/server-hono/src/app-factory.spec.tspackages/server-hono/src/utils/custom-endpoints.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/server-hono/src/utils/custom-endpoints.ts">
<violation number="1" location="packages/server-hono/src/utils/custom-endpoints.ts:340">
P1: Fallback OpenAPI generation drops/mismatches built-in routes when the app uses `basePath`, because built-in route matching and emitted path keys do not preserve the runtime-prefixed path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
omeraplak
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server-core/src/schemas/agent.schemas.ts`:
- Around line 1-5: The current ZodNamespace type (type ZodNamespace = typeof
defaultZ) pins createServerCoreSchemas to the default zod namespace and forces
callers using Zod v4 to cast to any; make the factory parameter generic so it
accepts other compatible Zod namespaces without unsafe casts. Replace the
ZodNamespace alias and update the function signature to a generic form such as
function createServerCoreSchemas<Z extends typeof defaultZ = typeof
defaultZ>(zod: Z = defaultZ as Z) (or provide an explicit overload/union of
supported namespace types), keeping references to defaultZ and
createServerCoreSchemas intact so callers using zodV4 can pass their namespace
without as any.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e15d623a-1e88-4dbc-b98a-aaad8be88276
📒 Files selected for processing (4)
.changeset/smart-swagger-paths.mdpackages/server-core/src/schemas/agent.schemas.tspackages/server-hono/src/app-factory.spec.tspackages/server-hono/src/routes/agent.routes.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .changeset/smart-swagger-paths.md
- packages/server-hono/src/app-factory.spec.ts
Comment on lines
+1
to
+5
| import { z as defaultZ } from "zod"; | ||
|
|
||
| type ZodNamespace = typeof defaultZ; | ||
|
|
||
| export function createServerCoreSchemas(zod: ZodNamespace = defaultZ) { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the factory parameter is pinned to the default zod namespace.
rg -n -C3 'type\s+ZodNamespace\s*=\s*typeof\s+defaultZ|createServerCoreSchemas\(zod: ZodNamespace' packages/server-core/src/schemas/agent.schemas.ts
# Verify alternate Zod usage currently requires an unsafe cast.
rg -n -C3 'createServerCoreSchemas\(\s*zodV4\s+as\s+any\s*\)' packages/server-hono/src/app-factory.spec.tsRepository: VoltAgent/voltagent
Length of output: 575
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for all usages of createServerCoreSchemas to see if this is a widespread issue
echo "=== All usages of createServerCoreSchemas ==="
rg -n 'createServerCoreSchemas\(' --type ts --type tsx
# Check if ZodNamespace is used elsewhere or redefined
echo -e "\n=== ZodNamespace references ==="
rg -n 'ZodNamespace' --type ts --type tsx
# Check if there's an existing union type or overloaded signatures
echo -e "\n=== Check for type definitions around the factory ==="
rg -n -A10 'export function createServerCoreSchemas' packages/server-core/src/schemas/agent.schemas.ts | head -20Repository: VoltAgent/voltagent
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for all usages of createServerCoreSchemas
echo "=== All usages of createServerCoreSchemas ==="
rg -n 'createServerCoreSchemas\(' --type ts
# Check if ZodNamespace is used elsewhere or redefined
echo -e "\n=== ZodNamespace references ==="
rg -n 'ZodNamespace' --type ts
# Check the full function signature and surrounding context
echo -e "\n=== Function definition context ==="
rg -n -A5 'export function createServerCoreSchemas' packages/server-core/src/schemas/agent.schemas.tsRepository: VoltAgent/voltagent
Length of output: 1012
Widen the factory parameter type so Zod v4 callers don't need as any.
ZodNamespace = typeof defaultZ pins createServerCoreSchemas to the default zod namespace type. The test demonstrates this requires an unsafe cast: createServerCoreSchemas(zodV4 as any). For a PR adding Zod v4 compatibility, this violates type safety at the call site. Define a proper union type or overload for the supported Zod namespaces instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server-core/src/schemas/agent.schemas.ts` around lines 1 - 5, The
current ZodNamespace type (type ZodNamespace = typeof defaultZ) pins
createServerCoreSchemas to the default zod namespace and forces callers using
Zod v4 to cast to any; make the factory parameter generic so it accepts other
compatible Zod namespaces without unsafe casts. Replace the ZodNamespace alias
and update the function signature to a generic form such as function
createServerCoreSchemas<Z extends typeof defaultZ = typeof defaultZ>(zod: Z =
defaultZ as Z) (or provide an explicit overload/union of supported namespace
types), keeping references to defaultZ and createServerCoreSchemas intact so
callers using zodV4 can pass their namespace without as any.
This was referenced Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
server-honocomposes some OpenAPI route schemas with its ownzod-openapi-compatZod instance, while nested schemas are imported as already-created Zod schemas fromserver-core.When package resolution gives those packages different Zod instances or major versions, the Zod v4 OpenAPI generator can reject nested schemas with errors like
Invalid element at key "data": expected a Zod schema. That causes/docgeneration to fail and Swagger UI to show an empty API list.What is the new behavior?
server-coreschema definitions can now be created through a schema factory.server-honouses that factory with the active Zod instance fromzod-openapi-compat, so built-in Swagger route schemas are composed from one compatible Zod instance.This fixes the OpenAPI generation failure directly instead of adding a Swagger fallback path.
fixes #1222
Notes for reviewers
Validation run:
pnpm --filter @voltagent/server-core buildpnpm --filter @voltagent/server-hono buildpnpm --filter @voltagent/server-hono test -- src/app-factory.spec.ts src/utils/custom-endpoints.spec.tspnpm --filter @voltagent/server-core test -- src/schemas/agent.schemas.spec.tspnpm exec biome check .changeset/smart-swagger-paths.md packages/server-core/src/schemas/agent.schemas.ts packages/server-hono/src/routes/agent.routes.ts packages/server-hono/src/app-factory.spec.tsI also reproduced the issue from
examples/baseand verified/docstays populated with Zod v3, Zod v4, and the previously failing mixed Zod resolution case.Docs were not updated because this is a runtime bug fix for existing Swagger behavior.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests