Allow unknown schema keys across protocol and API (fix #2)#4
Conversation
📝 WalkthroughWalkthroughThe pull request systematically replaces strict object validation with passthrough validation across Zod schemas throughout the codebase. This allows unknown fields in inputs and responses to pass through preservation rather than rejection. Affected areas include HTTP request/response schemas, JSON-RPC protocols, and thread/app-server protocols. Some schemas receive explicit type annotations while maintaining runtime behavior changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/codex-protocol/README.md (1)
7-7:⚠️ Potential issue | 🟠 MajorInconsistency between stated goal and validation policy.
Line 7 states "Fail fast when payloads drift" as a goal, but the new passthrough policy (lines 49-51) allows unknown fields to pass through without failing. This creates a contradiction—passthrough validation is the opposite of failing fast on schema drift.
Consider either:
- Updating line 7 to reflect the new permissive validation philosophy (e.g., "Allow forward compatibility while validating known fields"), or
- Adding context explaining why passthrough is used despite the fail-fast goal (e.g., "to support protocol evolution while maintaining backward compatibility").
Also applies to: 49-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/codex-protocol/README.md` at line 7, The README currently claims "Fail fast when payloads drift" but the schema validation policy uses passthrough, which contradicts that goal; either update the headline text to reflect a permissive/forward-compatible stance (e.g., change "Fail fast when payloads drift" to "Allow forward compatibility while validating known fields") or add a brief clarifying sentence near the passthrough policy explaining why passthrough is chosen (for example: "Passthrough is used to allow protocol evolution and forward compatibility while still validating known fields"), and ensure references to "passthrough" and the original headline are consistent so readers aren't confused by the divergence.
🧹 Nitpick comments (1)
packages/codex-protocol/README.md (1)
49-51: Consider documenting the rationale for passthrough validation.While the new policy is clearly stated, adding a brief explanation of why passthrough validation was chosen would help users understand the design decision and trade-offs. For example, you might mention protocol evolution, forward compatibility with new server fields, or interoperability requirements.
📝 Example addition
## Strictness Policy +- This package uses `.passthrough()` to enable forward compatibility as the protocol evolves. +- New fields added by the server will not break existing clients. - Schemas use `.passthrough()` by default. - Unknown fields are allowed and preserved. - Required known fields are still validated with exact types.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/codex-protocol/README.md` around lines 49 - 51, Add a short rationale paragraph next to the existing lines about "Schemas use `.passthrough()`" explaining why passthrough validation was chosen: mention protocol evolution and forward-compatibility (servers may add new fields), interoperability with other clients, and the trade-off that unknown fields are preserved but will not be type-checked; reference the `.passthrough()` policy and clarify that required known fields remain strictly validated to reassure users.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/codex-protocol/README.md`:
- Line 7: The README currently claims "Fail fast when payloads drift" but the
schema validation policy uses passthrough, which contradicts that goal; either
update the headline text to reflect a permissive/forward-compatible stance
(e.g., change "Fail fast when payloads drift" to "Allow forward compatibility
while validating known fields") or add a brief clarifying sentence near the
passthrough policy explaining why passthrough is chosen (for example:
"Passthrough is used to allow protocol evolution and forward compatibility while
still validating known fields"), and ensure references to "passthrough" and the
original headline are consistent so readers aren't confused by the divergence.
---
Nitpick comments:
In `@packages/codex-protocol/README.md`:
- Around line 49-51: Add a short rationale paragraph next to the existing lines
about "Schemas use `.passthrough()`" explaining why passthrough validation was
chosen: mention protocol evolution and forward-compatibility (servers may add
new fields), interoperability with other clients, and the trade-off that unknown
fields are preserved but will not be type-checked; reference the
`.passthrough()` policy and clarify that required known fields remain strictly
validated to reassure users.
Note
Medium Risk
Broadly relaxes Zod validation from
strict()topassthrough()across server/web/protocol boundaries, which can hide unexpected payload drift and allow unvalidated data to propagate. Behavior is mostly additive/forward-compatible but affects many request/response validation points.Overview
Relaxes schema strictness across the stack by switching many Zod schemas from
.strict()to.passthrough()so unknown fields are accepted (and typically preserved) for HTTP request bodies, web API responses, and protocol/app-server/IPC/thread payloads.Updates JSON-RPC parsing to accept extra fields and improves
parseJsonRpcIncomingMessageerror reporting by validating response vs notification separately and combining Zod issues when neither matches. Protocol docs and tests are updated to reflect/verify acceptance of unknown keys (e.g., modelhiddenflags and extra top-level response fields).Written by Cursor Bugbot for commit 0f229c8. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Refactor
Tests