feat: add Zod request validation for API endpoints#84
Conversation
Add zod v3 schemas for runtime request validation at API system boundaries. This prevents malformed payloads (e.g. from Pub/Sub redelivery) from reaching business logic and provides clear, structured error messages. Changes: - Install zod@3 dependency - Create api/lib/validation.ts with schemas: - createJobSchema (POST /api/jobs): deckIds, simulations, parallelism - updateSimulationSchema (PATCH /api/jobs/:id/simulations/:simId) - updateJobSchema (PATCH /api/jobs/:id) - parseBody() helper with formatted error messages - Replace manual validation in 3 route files with schema validation - Add 17 validation unit tests in api/lib/validation.test.ts - Add validation tests to test:unit script chain Behavior change: invalid parallelism values now return 400 instead of being silently ignored. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the API's robustness and maintainability by introducing Zod for schema-based request validation. By centralizing validation logic and removing boilerplate from individual route handlers, it improves code clarity, reduces the likelihood of validation bugs, and provides more consistent and informative error responses to clients. This change addresses a documented architectural issue by standardizing how incoming data is validated. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great improvement, introducing zod for robust request validation across several API endpoints. By replacing manual checks with declarative schemas in api/lib/validation.ts, the route handlers in api/app/api/jobs/ are now cleaner and more maintainable. The addition of a comprehensive suite of unit tests in api/lib/validation.test.ts ensures the new validation logic is correct and reliable. I have a couple of minor suggestions to further refine the new validation helper and its tests, which should be addressed immediately as per repository guidelines.
api/lib/validation.test.ts
Outdated
| simulations: 8, | ||
| }); | ||
| assertEqual(result.success, false, 'should fail'); | ||
| if (!result.success) assert(result.error.includes('4'), 'error should mention 4'); |
There was a problem hiding this comment.
This test assertion is a bit weak, as result.error.includes('4') could potentially match other parts of an error string, not just the expected deck count. To make the test more robust and prevent future regressions in user-facing error messages, it's better to assert against the exact error string.
| if (!result.success) assert(result.error.includes('4'), 'error should mention 4'); | |
| if (!result.success) assertEqual(result.error, 'deckIds: Exactly 4 deckIds are required', 'error message should be specific'); |
References
- This comment suggests a code quality improvement (more robust test assertion) that should be addressed immediately, aligning with the rule to not defer valid code quality suggestions.
api/lib/validation.ts
Outdated
| const messages = result.error.issues.map(i => `${i.path.join('.')}: ${i.message}`).filter(Boolean); | ||
| return { success: false, error: messages.join('; ') || 'Invalid request body' }; |
There was a problem hiding this comment.
The current error message formatting for Zod issues without a specific path (e.g., from .refine()) will produce a leading colon, like ': Invalid input'. This can be slightly confusing for API consumers. We can adjust the logic to omit the colon when there's no path. This change also removes the redundant .filter(Boolean) as Zod issues always contain a message.
const messages = result.error.issues.map(
(i) => (i.path.length > 0 ? `${i.path.join('.')}: ` : '') + i.message
);
return { success: false, error: messages.join('; ') || 'Invalid request body' };References
- This comment suggests a code quality improvement (better error message formatting for API consumers) that should be addressed immediately, aligning with the rule to not defer valid code quality suggestions.
- Handle path-less Zod errors without leading colon - Use exact string assertion for deckIds validation test Addresses Gemini review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Resolve package.json conflict: include both validation.test.ts and cancel-recover.test.ts in test:unit script. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
zodv3 dependency for runtime request validationapi/lib/validation.tswith schemas for:createJobSchema— POST /api/jobs (deckIds, simulations, parallelism)updateSimulationSchema— PATCH /api/jobs/:id/simulations/:simIdupdateJobSchema— PATCH /api/jobs/:idparseBody(schema, body)helper with formatted error messagesBehavior change: Invalid
parallelismvalues now return 400 instead of being silently ignored.Addresses architecture issue #3 from
docs/ARCHITECTURE_ISSUES.md.Test plan
npm run lint --prefix apipassesnpm run build --prefix apipasses🤖 Generated with Claude Code