fix(lib): prevent field from being both required and read-only (#3296) - #3310

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

3 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
prd-docs	Ignored		Aug 26, 2026 9:27pm
prd-openpage-api	Ignored		Aug 26, 2026 9:27pm
stg-docs	Skipped		Aug 26, 2026 9:27pm

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 @coderabbitai help to get the list of available commands.}

Hey there, it looks like you haven't accepted our contributor license agreement yet. In order for us to accept your pull request we ask that you please fill out the CLA:

https://documen.so/cla

Hey there, it looks like you haven't accepted our contributor license agreement yet. In order for us to accept your pull request we ask that you please fill out the CLA:

https://documen.so/cla

Hi, I just filled out and signed the CLA. Thank you

@gcoinstash-cmd The schema coverage looks solid. Since the reported regression is specifically the update-many API accepting and persisting this invalid state, could we add one route-level regression test for an invalid required: true + readOnly: true update?
That would verify the full request-validation boundary rejects it and that the existing field is not persisted in the invalid state.

@gcoinstash-cmd The schema coverage looks solid. Since the reported regression is specifically the update-many API accepting and persisting this invalid state, could we add one route-level regression test for an invalid required: true + readOnly: true update? That would verify the full request-validation boundary rejects it and that the existing field is not persisted in the invalid state.

Hi @yesprasad, thank you for the review!

I have added the requested route-level regression test in the latest commit (test(fields): add route-level regression test for update-many required read-only rejection).

It verifies that attempting to update fields with required: true and readOnly: true via the update-many endpoint is rejected at the request-validation boundary and ensures the existing field data in the database is not modified. All test suites pass cleanly.

@gcoinstash-cmd The schema coverage looks solid. Since the reported regression is specifically the update-many API accepting and persisting this invalid state, could we add one route-level regression test for an invalid required: true + readOnly: true update? That would verify the full request-validation boundary rejects it and that the existing field is not persisted in the invalid state.

Hi @yesprasad, I've added the route-level regression test in packages/app-tests/e2e/api/v2/envelopes-api.spec.ts verifying that { required: true, readOnly: true } update requests are rejected with a 400 validation error at the boundary and that existing field data remains untouched. All 205 unit/schema tests are passing cleanly.

@maintainer Thanks for the feedback!

The requested route-level regression test has been added and pushed in commit 6d6d7b1aa:

• E2E Route Test: Added to packages/app-tests/e2e/api/v2/envelopes-api.spec.ts under the Envelope field update-many endpoint suite. It tests that attempting an update with both required: true and readOnly: true returns an HTTP 400 rejection and preserves the exact database state.
• Server Guard: Added explicit validation in packages/lib/server-only/field/update-envelope-fields.ts.
• Schema Tests: Added ZFieldAndMetaSchema test coverage in packages/lib/types/field-meta.test.ts.

All CI checks and local test suites are passing green. Ready for final review and merge!