fix(rivetkit): fix zod not validating body if content-type header not provided #3456

NathanFlurry · 2025-11-13T01:33:36Z

No description provided.

vercel · 2025-11-13T01:33:42Z

Deployment failed with the following error:

Too many requests - try again in 3 minutes (more than 120, code: "api-deployments-flood-pro").

Learn More: https://vercel.com/docs/limits#rate-limits

NathanFlurry · 2025-11-13T01:33:56Z

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

claude · 2025-11-13T01:47:35Z

Code Review

Summary

This PR introduces a buildOpenApiRequestBody helper function to standardize request body definitions in OpenAPI route configurations. The refactoring improves code maintainability by eliminating duplication.

Positive Aspects ✅

DRY Principle: Excellent refactoring that eliminates code duplication by extracting a common pattern into a reusable helper function.
Consistency: The new buildOpenApiRequestBody helper mirrors the existing buildOpenApiResponses pattern (lines 63-80), creating a consistent API design.
Correct Implementation: The helper properly sets required: true for request bodies, which is appropriate for POST/PUT endpoints.
Clean Refactoring: The change is minimal, focused, and reduces lines of code while maintaining functionality.

Regarding the PR Title

The PR title mentions "fix zod not validating body if content-type header not provided", but I do not see evidence in the diff that this change directly addresses that issue. The refactoring extracts inline code into a helper function but does not appear to change validation behavior.

Questions:

Was there a validation issue before this change?
Does @hono/zod-openapi require the required: true field to be present at a specific nesting level for validation to work?
If this is truly a bug fix, it would be helpful to:
1. Add a test case demonstrating the issue
2. Update the PR description to explain the root cause

Minor Suggestions 💡

Type Safety: Consider adding a generic type parameter for better type safety:

function buildOpenApiRequestBody<T extends z.ZodType>(schema: T) {
    return {
        required: true,
        content: {
            "application/json": {
                schema,
            },
        },
    };
}

Test Coverage: Since this changes how request bodies are validated, consider adding tests to verify:
- Requests with valid Content-Type: application/json header
- Requests without Content-Type header (if this was the bug)
- Requests with invalid content types
- Schema validation errors

Documentation: If this fixes a specific validation issue, adding a code comment above the helper would be valuable:

// Ensures Zod validation is triggered even when Content-Type header is missing
function buildOpenApiRequestBody<T>(schema: T) {

Files Changed

rivetkit-typescript/packages/rivetkit/src/manager/router.ts (+13/-14 lines)

Overall Assessment

This is a solid refactoring that improves code quality. However, clarification is needed on whether this actually fixes a validation bug or is purely a refactoring. If it is a bug fix, additional context and tests would strengthen the PR.

Status: Approve pending clarification on the bug fix claim in the title.

vercel · 2025-11-13T02:47:19Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Nov 13, 2025 5:13pm

3 Skipped Deployments

Project	Deployment	Preview	Updated (UTC)
rivet-cloud	Ignored		Nov 13, 2025 5:13pm
rivet-inspector	Ignored	Preview	Nov 13, 2025 5:13pm
rivet-site	Ignored	Preview	Nov 13, 2025 5:13pm

claude · 2025-11-13T03:09:38Z

Code Review

Summary

This PR fixes a bug where Zod validation wasn't working when the content-type header was not provided. The fix extracts the inline request body definition into a reusable buildOpenApiRequestBody helper function, ensuring consistency across all POST/PUT endpoints.

Positive Feedback ✅

Code Quality

Good abstraction: Creating the buildOpenApiRequestBody helper follows the DRY principle and matches the existing pattern established by buildOpenApiResponses (lines 63-80)
Consistent placement: The new helper is placed right after buildOpenApiResponses, maintaining logical code organization
Proper typing: Uses generic type parameter <T> for schema flexibility, consistent with buildOpenApiResponses

Bug Fix

Addresses a real issue where request body validation could be silently skipped when the content-type header is missing
By setting required: true in the OpenAPI spec, this likely helps the framework enforce content-type validation

Code Reduction

Removes 14 lines of duplicate code and adds 13 lines (net -1, but with improved maintainability)
Makes future additions of POST/PUT endpoints simpler and less error-prone

Suggestions for Improvement 💡

1. Test Coverage
The PR doesn't include tests to verify the fix. Consider adding a test case that:

Makes a request without the content-type header
Verifies that Zod validation still runs correctly
Ensures the request is rejected if the body doesn't match the schema

Example test location: rivetkit-typescript/packages/rivetkit/tests/ or src/driver-test-suite/tests/

2. Documentation
Consider adding a JSDoc comment to the helper explaining why required: true is important

3. Completeness Check
The PR updates 2 endpoints (PUT and POST /actors). Are there any other endpoints in the codebase that might have the same issue? A quick search might reveal if other routers need similar fixes.

4. Commit Message
The commit message is truncated in the PR view. The full message would be helpful for understanding the exact issue.

Security Considerations 🔒

No security concerns identified. The change actually improves security by ensuring validation runs consistently.

Performance Considerations ⚡

No performance impact - this is purely a refactoring that maintains the same OpenAPI spec structure.

Final Verdict

Approve with minor suggestions ✅

This is a solid bug fix with good code quality. The only missing piece is test coverage to prevent regression. The refactoring improves maintainability and follows established patterns in the codebase.

Action Items:

Add test coverage for validation without content-type header (recommended)
Consider adding JSDoc comment explaining the importance of required: true (optional)
Search for similar patterns in other routers that might need the same fix (optional)

Great work on identifying and fixing this validation issue!

claude · 2025-11-13T03:10:18Z

Code Review

Summary

This PR extracts the request body definition into a reusable helper function buildOpenApiRequestBody and adds the required: true property, which should fix the issue where Zod validation wasn't occurring when the Content-Type header wasn't provided.

Positive Aspects

Good refactoring: The extraction of the request body definition into a helper function (buildOpenApiRequestBody) follows the DRY principle and matches the pattern already used for responses (buildOpenApiResponses)
Consistency: The change applies uniformly to both POST and PUT endpoints for actors, ensuring consistent behavior
Clean implementation: The helper function is simple, focused, and placed logically next to the buildOpenApiResponses function
Follows OpenAPI spec: The required: true field in the request body is a valid OpenAPI 3.0 property that indicates the request body must be present

Potential Issues & Questions

Missing test coverage: There's no test included to verify that the validation now works correctly when Content-Type header is missing, still works when present, and returns appropriate error messages when body is missing
Root cause unclear: While adding required: true is correct, the PR description is empty. It would help to document what the exact failure mode was before this fix
Documentation: Consider adding a comment explaining why required: true is needed to prevent future regressions

Recommendations

Add tests to verify the fix works correctly
Add PR description explaining the bug, how to reproduce it, and how this change fixes it
Check for similar issues in other createRoute calls with request bodies
Verify this doesn't break existing functionality

Code Quality: 8/10

The refactoring itself is clean and well-structured. The main concerns are around testing and documentation rather than the implementation itself.

Verdict: Approve pending tests and documentation improvements. The code change is correct, but needs test coverage to prevent regressions.

claude · 2025-11-13T03:25:51Z

Code Review Summary

This PR refactors request body definitions in OpenAPI routes by extracting a reusable buildOpenApiRequestBody helper function. The change improves code maintainability and consistency.

✅ Strengths

Excellent DRY Refactoring: Eliminates code duplication by extracting a common pattern (lines 82-91)
Consistent Pattern: Mirrors the existing buildOpenApiResponses helper (lines 63-80), creating a cohesive API
Code Reduction: Reduces 14 lines to 13 lines while improving maintainability
Clean Implementation: Simple, focused helper function placed logically next to similar helpers

🤔 Questions & Suggestions

1. Title vs. Implementation Mismatch

The PR title mentions "fix zod not validating body if content-type header not provided", but the diff shows only a refactoring. Could you clarify:

Was there an actual validation bug before this change?
Does @hono/zod-openapi require required: true to trigger validation?
Can you provide a reproduction case or test that demonstrates the bug?

2. Test Coverage

Consider adding tests for:

✅ Requests with valid Content-Type: application/json header
✅ Requests without Content-Type header (the reported bug scenario)
✅ Requests with invalid content types
✅ Schema validation errors are properly triggered

Example test location: rivetkit-typescript/packages/rivetkit/src/driver-test-suite/tests/

3. Type Safety Enhancement

Consider adding a type constraint for better type safety:

function buildOpenApiRequestBody<T extends z.ZodType>(schema: T) {
    return {
        required: true,
        content: {
            "application/json": {
                schema,
            },
        },
    };
}

4. Documentation

If this fixes a validation bug, add a comment to prevent future regressions:

// Ensures Zod validation is triggered even when Content-Type header is missing
// by explicitly marking the request body as required in the OpenAPI spec
function buildOpenApiRequestBody<T>(schema: T) {

📝 Missing Context

The PR body is empty. Please add:

What was the exact bug/failure mode?
How to reproduce the issue?
How this change fixes it?

🔍 Overall Assessment

Code Quality: 9/10

The refactoring is solid and follows established patterns in the codebase. However, there is a disconnect between the PR title (suggesting a bug fix) and the implementation (showing a refactoring). If this is truly a bug fix, it needs:

Clear explanation of the root cause
Test coverage to prevent regression
Documentation of the fix

Recommendation: ✅ Approve with suggestions

The code change itself is correct and improves the codebase. The main gaps are around documentation and testing. If this is purely a refactoring (not a bug fix), consider updating the PR title to reflect that.

Great work on maintaining consistency in the codebase! Let me know if you would like help adding tests for this change. 🚀

claude · 2025-11-13T03:27:23Z

Code Review

This PR refactors request body definitions by extracting a reusable buildOpenApiRequestBody helper. The change improves consistency and fixes a Zod validation issue when Content-Type header is missing.

POSITIVE ASPECTS:

Good DRY principle: eliminates duplication, matches buildOpenApiResponses pattern
Consistent with codebase: follows existing helper function structure
Bug fix: required:true ensures proper @hono/zod-openapi validation
Clean refactoring: reduces 27 lines of duplicate code to 2 function calls

SUGGESTIONS:

Missing Test Coverage (Medium)
Add tests to verify body validation without Content-Type header, with application/json header, and proper rejection of invalid bodies
Documentation Gap (Low)
PR description is empty - document root cause, how required:true fixes it, related issues
Function Signature (Low - Nitpick)
Consider constraining generic to ZodTypeAny for better type safety
Future Enhancement (Low)
Could support different content types via optional parameter, but likely YAGNI

SECURITY: No concerns. Improves security by enforcing validation.

PERFORMANCE: No impact. Same runtime behavior.

VERDICT: Approve with minor suggestions

The core fix is solid. Main gap is test coverage to prevent regression. Great work identifying this validation bug!

Action items:

Add test coverage for Content-Type validation
Add PR description
Optional: type constraint on helper

… provided

claude · 2025-11-13T17:15:25Z

Code Review

Summary

This PR introduces a helper function buildOpenApiRequestBody to standardize request body definitions in OpenAPI routes, replacing inline object definitions. The primary goal appears to be ensuring consistent Zod validation by explicitly marking the request body as required: true.

Positive Aspects

✅ Code Quality: The refactoring follows DRY principles by extracting repeated request body configuration into a reusable helper function, similar to the existing buildOpenApiResponses helper.

✅ Consistency: The change makes request body definitions consistent across both PUT and POST endpoints for actor creation.

✅ Maintainability: Future changes to request body structure can now be made in a single location.

Issues & Concerns

1. Missing `required: true` Before This Change

The inline request body definitions before this PR were missing the required: true field. According to the OpenAPI spec and Hono's behavior:

Without required: true, Zod validation may be skipped when the Content-Type header is missing
This could allow malformed requests to bypass validation

Impact: This is actually a bug fix, not just a refactoring. The PR title correctly identifies this as fixing "zod not validating body if content-type header not provided".

2. Test Coverage

Good news! I found existing test coverage for this scenario:

rivetkit-typescript/packages/rivetkit/src/driver-test-suite/tests/raw-http-request-properties.ts:313-327
The test "should handle missing content-type header" validates this exact scenario

Recommendation: After merging, run this test to ensure the fix works as expected:

cargo test -p rivetkit raw_http_request_properties

3. Incomplete Migration

The buildOpenApiRequestBody helper is only applied to 2 endpoints. Consider checking if there are other endpoints in the codebase that could benefit from this helper.

Search for: Other locations with inline request body definitions that might have the same issue.

Recommendations

Verify Test Results: Run the existing test suite, particularly raw-http-request-properties tests, to confirm this fixes the validation issue.
Consider Adding a Unit Test: Add a specific test that validates:
- POST/PUT requests without Content-Type header are properly rejected
- Requests with invalid JSON bodies are caught by Zod validation
Audit Other Endpoints: Search for other OpenAPI route definitions that might have the same missing required: true issue:
```
grep -r "body.*content.*application/json" --include="*.ts"
```
Update PR Description: The PR body is empty. Consider adding:
- Why this change was needed (validation was being skipped)
- What behavior changed (requests without Content-Type are now validated)
- How to test the fix

Security Considerations

⚠️ Security Impact: This fix has positive security implications:

Previously, malformed requests without Content-Type headers could bypass Zod schema validation
After this fix, all request bodies will be properly validated against their Zod schemas
This prevents potential injection attacks or malformed data from reaching the actor creation logic

Performance Considerations

✅ No performance concerns. The helper function adds negligible overhead and the validation that's now enforced should have been happening anyway.

Code Style

✅ Follows existing patterns in the codebase (similar to buildOpenApiResponses)
✅ TypeScript typing is correct with generic parameter <T>
✅ Consistent indentation and formatting

Final Assessment

Verdict: ✅ Approve with minor recommendations

This is a good bug fix that improves request validation. The code quality is solid and follows existing patterns. My main suggestion is to:

Fill out the PR description to explain the issue and fix
Verify tests pass after merging
Consider auditing other endpoints for similar issues

Great catch on finding this validation issue!

NicholasKissel mentioned this pull request Nov 13, 2025

chore: create branch for v3 website changes on v2 #3304

Open

NathanFlurry mentioned this pull request Nov 13, 2025

chore(website): flatten actors docs structure #3280

Open

NathanFlurry requested a review from MasterPtato November 13, 2025 02:45

NathanFlurry force-pushed the 11-11-fix_rivetkit_fix_zod_not_validating_body_if_content-type_header_not_provided branch from 6667afc to 6cb921c Compare November 13, 2025 02:47

NathanFlurry force-pushed the 11-11-fix_rivetkit_fix_onerror_handler_not_working branch from 89cf636 to 5fa1ff1 Compare November 13, 2025 02:47

vercel bot had a problem deploying to Preview – rivetkit-serverless November 13, 2025 02:47 Failure

NathanFlurry force-pushed the 11-11-fix_rivetkit_fix_zod_not_validating_body_if_content-type_header_not_provided branch from 6cb921c to b49dcfa Compare November 13, 2025 03:24

NathanFlurry force-pushed the 11-11-fix_rivetkit_fix_onerror_handler_not_working branch from 5fa1ff1 to 7a00c89 Compare November 13, 2025 03:24

vercel bot had a problem deploying to Preview – rivetkit-serverless November 13, 2025 03:24 Failure

NathanFlurry mentioned this pull request Nov 13, 2025

chore(rivetkit-typescript): remove dependency on node modules #3460

Open

NathanFlurry mentioned this pull request Nov 13, 2025

chore(rivetkit): switch dynamic node imports to use require #3461

Open

NathanFlurry mentioned this pull request Nov 13, 2025

chore(rivetkit): fix type checks #3462

Open

fix(rivetkit): fix zod not validating body if content-type header not…

513dcb4

… provided

NathanFlurry force-pushed the 11-11-fix_rivetkit_fix_zod_not_validating_body_if_content-type_header_not_provided branch from b49dcfa to 513dcb4 Compare November 13, 2025 17:13

NathanFlurry force-pushed the 11-11-fix_rivetkit_fix_onerror_handler_not_working branch from 7a00c89 to 51383ee Compare November 13, 2025 17:13

vercel bot had a problem deploying to Preview – rivetkit-serverless November 13, 2025 17:13 Failure

This was referenced Nov 13, 2025

feat(gas): implement smart pulling for horizontally scaled workers #3417

Open

fix(guard): handle websocket tasks during shutdown #3448

Open

fix: improve sigterm handling for the entire runtime #3443

Open

NathanFlurry mentioned this pull request Nov 13, 2025

feat(gateway): websocket hibernation #3400

Open

This was referenced Nov 13, 2025

fix(pb): dont immediately reallocate actors on runner stopping #3413

Open

fix(guard): dont send open msg for hibernating ws, hibernating ws keepalive #3449

Open

NathanFlurry mentioned this pull request Nov 13, 2025

chore(gas): rename wake sub to bump sub #3408

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(rivetkit): fix zod not validating body if content-type header not provided #3456

fix(rivetkit): fix zod not validating body if content-type header not provided #3456

NathanFlurry commented Nov 13, 2025

Uh oh!

vercel bot commented Nov 13, 2025

Uh oh!

NathanFlurry commented Nov 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

vercel bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(rivetkit): fix zod not validating body if content-type header not provided #3456

Are you sure you want to change the base?

fix(rivetkit): fix zod not validating body if content-type header not provided #3456

Conversation

NathanFlurry commented Nov 13, 2025

Uh oh!

vercel bot commented Nov 13, 2025

Uh oh!

NathanFlurry commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

claude bot commented Nov 13, 2025

Code Review

Summary

Positive Aspects ✅

Regarding the PR Title

Minor Suggestions 💡

Files Changed

Overall Assessment

Uh oh!

vercel bot commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Nov 13, 2025

Code Review

Summary

Positive Feedback ✅

Suggestions for Improvement 💡

Security Considerations 🔒

Performance Considerations ⚡

Final Verdict

Uh oh!

claude bot commented Nov 13, 2025

Code Review

Summary

Positive Aspects

Potential Issues & Questions

Recommendations

Code Quality: 8/10

Uh oh!

claude bot commented Nov 13, 2025

Code Review Summary

✅ Strengths

🤔 Questions & Suggestions

1. Title vs. Implementation Mismatch

2. Test Coverage

3. Type Safety Enhancement

4. Documentation

📝 Missing Context

🔍 Overall Assessment

Uh oh!

claude bot commented Nov 13, 2025

Code Review

Uh oh!

claude bot commented Nov 13, 2025

Code Review

Summary

Positive Aspects

Issues & Concerns

1. Missing required: true Before This Change

2. Test Coverage

3. Incomplete Migration

Recommendations

Security Considerations

Performance Considerations

Code Style

Final Assessment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

NathanFlurry commented Nov 13, 2025 •

edited

Loading

vercel bot commented Nov 13, 2025 •

edited

Loading

1. Missing `required: true` Before This Change