-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: add list/listPaginated pattern and reorganize schemas #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add list (simple) + listPaginated (cursor-based) for customer, order, checkout - Move entity schemas from contracts/ to schemas/: - ProductSchema, ProductDetailSchema, ProductPriceSchema -> schemas/product.ts - CheckoutStatusSchema, CheckoutTypeSchema, CheckoutListItemSchema -> schemas/checkout.ts - OrderWithRelationsSchema -> schemas/order.ts - Create shared PaginatedInputSchema for DRY pagination inputs - Add externalId to CreateCustomerInput and UpdateCustomerInput
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73d04311c6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Replace refinement-based validation with a discriminated union for CustomerLookupInputSchema. This ensures exactly-one-of validation translates properly to JSON Schema for MCP/AI tool compatibility.
|
@codex review |
xmcp requires .shape property for tool schema generation, which only ZodObject has. ZodUnion (CustomerLookupInputSchema) lacks .shape. Add CustomerLookupToolSchema as a flat object with all optional fields for MCP tools, while keeping the discriminated union for contract validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65406b9eb6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export const CustomerLookupInputSchema = z.union([ | ||
| CustomerLookupByIdSchema, | ||
| CustomerLookupByEmailSchema, | ||
| CustomerLookupByExternalIdSchema, | ||
| ]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enforce exactly one customer lookup identifier
CustomerLookupInputSchema is documented as “exactly one identifier,” but the union of non‑strict objects will accept payloads with multiple identifiers (e.g., { id, email }) and silently strip extras. That can cause a caller to pass conflicting identifiers and still get a result based only on the first matching branch. If you need to enforce exclusivity, add .strict() and a refinement or use a discriminated union so multi‑field payloads are rejected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check 5c433b1
Summary
Test plan
🤖 Generated with Claude Code