feat(server): add OpenAPI spec generation for RPC API#2574
Conversation
- Add `RPCApiSpecGenerator` that produces an OpenAPI 3.1 spec for the RPC-style API - Extract `mayDenyAccess` and `getMetaDescription` helpers into shared `spec-utils.ts` (used by both REST and RPC generators) - Extend Zod factory with `makeProcedureArgsSchema` so procedure arg schemas are registered eagerly and referenced via `$ref` in the spec - Generate per-operation summaries and typed response schemas (array / nullable / batch-count) via an `OPERATION_INFO` map - Respect `@@meta`/`@meta` description attributes on models and scalar fields - Respect slicing options (includedModels, excludedModels, includedOperations, excludedOperations, includedProcedures, excludedProcedures) - Add 403 responses when `respectAccessPolicies` is enabled and non-constant-allow policies exist - Add comprehensive test suite (71 tests) with baseline YAML snapshot and OpenAPI 3.1 validation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review Complete ✅I've thoroughly reviewed this PR and here's my comprehensive assessment: Overall Assessment: Excellent ImplementationThis is a well-architected, comprehensive implementation that adds RPC OpenAPI spec generation with strong attention to detail and code quality. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture highlights:
Performance Considerations ⭐⭐⭐⭐Excellent optimizations:
Performance characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security features:
Security patterns:
Test Coverage ⭐⭐⭐⭐⭐Comprehensive test suite (919 lines):
Test quality highlights:
Minor Issues & SuggestionsCode Quality (Low priority):
Performance (Very low priority):
Changelog ValidationThe PR description accurately reflects the implementation:
Recommendation: LGTM 🚀This PR represents excellent software engineering:
The implementation is production-ready and well-documented. The minor suggestions above are cosmetic improvements that don't affect the core functionality. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded deterministic Zod JSON Schema generation and registry, refactored filter/value schema builders, exposed procedure-args schema creation, centralized access-policy deny/allow logic, updated REST spec generation to use it, and added an RPC OpenAPI spec generator plus comprehensive tests. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/orm/src/client/zod/factory.ts`:
- Around line 203-231: The toJSONSchema() method clears this.schemaRegistry but
leaves the `@cache`()-decorated builders (makeFindUniqueSchema,
makeFindFirstSchema, makeFindManySchema, makeExistsSchema, makeCreateSchema,
makeCreateManySchema, makeCreateManyAndReturnSchema, makeUpdateSchema,
makeUpdateManySchema, makeUpdateManyAndReturnSchema, makeUpsertSchema,
makeDeleteSchema, makeDeleteManySchema, makeCountSchema, makeAggregateSchema,
makeGroupBySchema and makeProcedureArgsSchema) cached, which can cause missing
registrations; fix by invalidating/clearing those builders' caches before
clearing schemaRegistry (or avoid clearing registry and instead rebuild with
cache-busting), so that subsequent eager calls run the builders on cache miss
and re-register schemas consistently.
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 347-359: The OpenAPI contract currently documents only the 'q'
parameter/requestBody and omits the sibling 'meta' query parameter and
requiredness rules used at runtime; update the handlers that build operation
parameters (where inputSchemaRef, isQueryOp, modelName, op, JSON_CT are
referenced) to: include an optional 'meta' query parameter alongside 'q'
wherever unmarshalQ(...) is used (match RPCApiHandler/unmarshalQ runtime
behavior), mark 'q' (for query ops) or the requestBody (for procedures) as
required when the operation's args shape contains required properties, and for
procedure requestBodies ensure the envelope requires the 'args' property when
the procedure declares parameters; mirror the logic used in
packages/server/src/api/rpc/index.ts (see RPCApiHandler and unmarshalQ
references) so the OpenAPI contract matches runtime validation.
- Around line 329-344: The OpenAPI responses for RPC endpoints are narrower than
runtime: ensure the responses object merges in the ORM-derived error responses
from makeORMErrorResponse() so documented errors match runtime behavior;
specifically, in the response builders that currently use mayDenyAccess(...) and
static ERROR_* entries (the blocks around buildOperationSuccessSchema(modelName,
op) and similar for procedures/transactions), call
makeORMErrorResponse(modelDef, op or procedure name, this.specOptions) or
otherwise merge its returned 403/404/422 entries into the responses map so
update/delete model ops include 404 and procedure/transaction endpoints include
ORM-derived 403/404/422, preserving existing 400/500 entries and conditional 403
from mayDenyAccess.
- Around line 623-675: In buildModelEntitySchema, optional fields are still
being added to the required list and optional to-one relations are emitted as
non-nullable $ref; update the logic so you only push fieldName into required
when fieldDef.optional is false, and for relation fields (the branch that builds
refSchema) wrap the ref (or array of refs) in an anyOf including { type: 'null'
} when fieldDef.optional is true so to-one optional relations validate null,
e.g. change the relation branch to produce properties[fieldName] =
fieldDef.optional ? { anyOf: [base, { type: 'null' }] } : base, and apply the
same conditional required.push(fieldName) check in the enum, builtin, typedef
and unknown branches (only push when !fieldDef.optional).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a0816fcd-6542-4128-a79e-6b9d7e86da35
📒 Files selected for processing (8)
packages/orm/src/client/zod/factory.tspackages/orm/src/client/zod/index.tspackages/server/src/api/common/spec-utils.tspackages/server/src/api/rest/openapi.tspackages/server/src/api/rpc/index.tspackages/server/src/api/rpc/openapi.tspackages/server/test/openapi/baseline/rpc.baseline.yamlpackages/server/test/openapi/rpc-openapi.test.ts
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent ImplementationThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (919 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Issues Identified & AnalysisI've reviewed the CodeRabbit findings and can confirm they identify legitimate issues that should be addressed: Critical Issues:1. Cache Invalidation Bug (
|
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent ImplementationThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (919 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Issues Identified & AnalysisI've reviewed the CodeRabbit findings and can confirm they identify legitimate issues that should be addressed: Critical Issues:1. Cache Invalidation Bug (
|
On a second call to toJSONSchema(), schemaRegistry.clear() emptied the registry but @cache()-decorated builders returned their cached ZodType without re-executing, so registerSchema() was never called and the serialized registry was empty. Fix by clearing schemaCache before schemaRegistry so all builders re-execute on the subsequent eager calls and re-register their schemas. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (919 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/server/test/openapi/rpc-openapi.test.ts (2)
883-887: Avoid pinning this test toanyOfordering.Line 886 assumes the non-null variant is always
anyOf[0]. A semantically equivalent reorder to[null, { ... }]would fail this test with no behavior change.More stable assertion
- const zipBase = addressSchema?.properties?.zip?.anyOf?.[0]; + const zipBase = addressSchema?.properties?.zip?.anyOf?.find( + (variant: any) => variant.type !== 'null' + ); expect(zipBase?.description).toBe('Postal code');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rpc-openapi.test.ts` around lines 883 - 887, The test currently assumes the non-null variant is anyOf[0]; update the assertion to locate the non-null/object variant dynamically instead of indexing into anyOf. In the test "optional typedef field `@meta` description appears on field schema" use the addressSchema?.properties?.zip?.anyOf array and find the member that represents the actual schema (e.g., where member.type !== 'null' or member.description is defined), assign that to zipBase and assert zipBase.description === 'Postal code' so the test is stable regardless of anyOf ordering.
235-250: Resolve the schema ref before asserting field coverage.Line 240 and Line 249 only prove the schema is object-like or a
$ref; they never verify thatFindUniqueArgs.whereorCreateArgs.datais actually present. If the generator points to the wrong component schema, both tests still pass.Proposed tightening
it('findUnique q schema contains where field', () => { const operation = spec.paths['/user/findUnique'].get; const qSchema = operation.parameters.find((p: any) => p.name === 'q').content['application/json'].schema; - // The schema describes FindUniqueArgs which has a required where field - expect(qSchema).toBeDefined(); - expect(qSchema.type === 'object' || qSchema.properties || qSchema.$defs || qSchema.$ref).toBeTruthy(); + const resolved = qSchema.$ref + ? spec.components.schemas[qSchema.$ref.replace('#/components/schemas/', '')] + : qSchema; + expect(resolved.properties?.where).toBeDefined(); + expect(resolved.required ?? []).toContain('where'); }); it('create request body schema contains data field', () => { const operation = spec.paths['/user/create'].post; const bodySchema = operation.requestBody.content['application/json'].schema; - expect(bodySchema).toBeDefined(); - // CreateArgs has a data field - expect( - bodySchema.type === 'object' || bodySchema.properties || bodySchema.$defs || bodySchema.$ref, - ).toBeTruthy(); + const resolved = bodySchema.$ref + ? spec.components.schemas[bodySchema.$ref.replace('#/components/schemas/', '')] + : bodySchema; + expect(resolved.properties?.data).toBeDefined(); + expect(resolved.required ?? []).toContain('data'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rpc-openapi.test.ts` around lines 235 - 250, The tests for "findUnique q schema contains where field" and "create request body schema contains data field" only check that qSchema and bodySchema are object-like or $ref; update both to resolve $ref targets from spec.components.schemas (or inline $defs) before asserting field presence: for spec.paths['/user/findUnique'].get inspect operation.parameters to get qSchema, if qSchema.$ref follow the reference to the actual schema and then assert it has a required/defined "where" property; likewise for spec.paths['/user/create'].post get requestBody.content['application/json'].schema, dereference any $ref to the concrete CreateArgs schema and assert it contains a "data" property (and/or required includes "data") so the tests fail if the generator pointed to the wrong component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/test/openapi/rest-openapi.test.ts`:
- Line 793: The test currently uses a loose matcher—replace the weak matcher
call expect(spec).toMatchObject(baseline) with the strict deep-equality
assertion expect(spec).toEqual(baseline) so the baseline comparison for
variables spec and baseline fails on any extra or changed keys and will detect
API-surface regressions; update the single expectation in the test where spec is
compared to baseline (in rest-openapi.test) accordingly.
In `@packages/server/test/openapi/rpc-openapi.test.ts`:
- Around line 903-904: The test uses toMatchObject which permits extra keys and
won't catch additive OpenAPI contract drift; update the assertion in
rpc-openapi.test.ts to assert exact equality by replacing
expect(spec).toMatchObject(baseline) with expect(spec).toEqual(baseline) (or
toStrictEqual if you want strict object/array equality), ensuring
loadBaseline(baselineFile) is compared exactly to spec so any added
paths/components fail the test.
---
Nitpick comments:
In `@packages/server/test/openapi/rpc-openapi.test.ts`:
- Around line 883-887: The test currently assumes the non-null variant is
anyOf[0]; update the assertion to locate the non-null/object variant dynamically
instead of indexing into anyOf. In the test "optional typedef field `@meta`
description appears on field schema" use the
addressSchema?.properties?.zip?.anyOf array and find the member that represents
the actual schema (e.g., where member.type !== 'null' or member.description is
defined), assign that to zipBase and assert zipBase.description === 'Postal
code' so the test is stable regardless of anyOf ordering.
- Around line 235-250: The tests for "findUnique q schema contains where field"
and "create request body schema contains data field" only check that qSchema and
bodySchema are object-like or $ref; update both to resolve $ref targets from
spec.components.schemas (or inline $defs) before asserting field presence: for
spec.paths['/user/findUnique'].get inspect operation.parameters to get qSchema,
if qSchema.$ref follow the reference to the actual schema and then assert it has
a required/defined "where" property; likewise for
spec.paths['/user/create'].post get
requestBody.content['application/json'].schema, dereference any $ref to the
concrete CreateArgs schema and assert it contains a "data" property (and/or
required includes "data") so the tests fail if the generator pointed to the
wrong component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3755a036-fa44-4dbd-b3e2-c37c0d4e3438
📒 Files selected for processing (2)
packages/server/test/openapi/rest-openapi.test.tspackages/server/test/openapi/rpc-openapi.test.ts
- update/delete model operations can throw NOT_FOUND at runtime; add 404 - procedure and transaction endpoints delegate to the ORM and can produce 403 (policy rejection), 404 (not found), and 422 (validation error); document all three to match runtime behavior Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (919 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
- Add optional 'meta' query parameter to all GET endpoints (model query ops and query procedures) to document the SuperJSON companion parameter consumed by unmarshalQ alongside 'q' - Mark 'args' as required in procedure envelope schema when the procedure declares at least one non-optional parameter, matching mapProcedureArgs which throws 'missing procedure arguments' in that case Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (919 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/server/src/api/rpc/openapi.ts (2)
641-675:⚠️ Potential issue | 🟠 MajorOptional response fields are still emitted as required/non-nullable.
required.push(fieldName)still runs for optional enum/builtin/typedef/unknown fields, and optional to-one relations are still emitted as bare$refs. Valid RPC responses with optional scalars or nullable optional relations will fail schema validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 641 - 675, The code incorrectly marks optional fields as required and emits nullable relations as non-null $ref; update the branches that currently call required.push(fieldName) (the enum branch, the isBuiltinType branch, the schema.typeDefs branch, and the final unknown-type branch) to only push the fieldName when fieldDef.optional is false, and for relation handling (the if (fieldDef.relation) block) wrap the refSchema (or array-of-ref) in a nullable union when fieldDef.optional is true (e.g., anyOf: [baseRef, { type: 'null' }]) so to-one optional relations are nullable instead of bare $ref; use the existing symbols fieldDef, properties, required, refSchema, base, and typed to implement these changes.
352-372:⚠️ Potential issue | 🟠 MajorDocument the full RPC input contract (
metaplus requiredq/args).These builders still only advertise
q/{ args }and leave them optional. The runtime parser also reads a siblingmetavalue and rejects omitted input when the args shape has required properties, so the OpenAPI contract is still out of sync for required-arg calls and SuperJSON-only values.Also applies to: 389-444
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 352 - 372, The OpenAPI output currently exposes only a loose "q" or "{ args }" schema via inputSchemaRef and leaves those fields optional, but runtime expects a sibling "meta" and enforces required properties on the args shape; update the builder that sets operation['parameters'] and operation['requestBody'] (the branches using inputSchemaRef, isQueryOp, operation, JSON_CT, modelName, op) to instead construct and reference a combined object schema (e.g., combinedInputSchemaRef) that has properties: "meta" (SuperJSON-compatible schema or the existing meta schema), and either "q" (for query ops) or "args" (for non-query ops) whose schema is inputSchemaRef; set the combined schema's "required" array to include the child fields that are required by the args shape (and include "q"/"args" when the args schema has required properties) so the OpenAPI contract matches runtime validation, and keep using JSON_CT for the content type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/orm/src/client/zod/factory.ts`:
- Around line 1379-1383: The schema for createManyAndReturn is being made
optional which allows empty requests; remove the .optional() call so the schema
built from
this.refineForSelectHasTruthyField(this.refineForSelectOmitMutuallyExclusive(result))
remains required, then register it with registerSchema using the same name
(`${model}CreateManyAndReturnArgs`) and ensure the ZodType casting still matches
CreateManyAndReturnArgs<Schema, Model, Options, ExtQueryArgs>; in short, drop
.optional() and keep the type cast to enforce that `data` is required at
runtime.
- Around line 57-68: The current removeNumberBounds override removes all numeric
constraints from schemas; update it so it only strips Zod's implicit
safe-integer bounds rather than user-specified constraints: in
removeNumberBounds (and the other similar block around 203-233) check the
jsonSchema's minimum/maximum/exclusiveMinimum/exclusiveMaximum values and only
delete them when they equal JavaScript's safe integer limits
(Number.MIN_SAFE_INTEGER / Number.MAX_SAFE_INTEGER or ±(2**53-1) equivalents)
that Zod injects; leave any other numeric bounds (from .min(), .max(),
.nonnegative(), or attributes) intact so explicit validations are preserved in
the exported JSON Schema.
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 136-154: The schema factory is constructed without queryOptions so
generateSharedSchemas still builds every model and leaks excluded schemas;
update creation/use of the factory and registry conversion to respect
handlerOptions.queryOptions: either pass queryOptions into
createQuerySchemaFactory in the constructor (e.g.,
createQuerySchemaFactory(this.handlerOptions.schema,
this.handlerOptions.queryOptions)) or recreate the factory inside generateSpec
with options, then call factory.toJSONSchema() or an equivalent API that accepts
queryOptions; finally ensure transformRegistrySchemas/generateSharedSchemas
filter or slice registrySchemas by handlerOptions.queryOptions so only included
models/relations become OpenAPI components (refer to constructor,
createQuerySchemaFactory, generateSpec, transformRegistrySchemas, and
generateSharedSchemas).
---
Duplicate comments:
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 641-675: The code incorrectly marks optional fields as required
and emits nullable relations as non-null $ref; update the branches that
currently call required.push(fieldName) (the enum branch, the isBuiltinType
branch, the schema.typeDefs branch, and the final unknown-type branch) to only
push the fieldName when fieldDef.optional is false, and for relation handling
(the if (fieldDef.relation) block) wrap the refSchema (or array-of-ref) in a
nullable union when fieldDef.optional is true (e.g., anyOf: [baseRef, { type:
'null' }]) so to-one optional relations are nullable instead of bare $ref; use
the existing symbols fieldDef, properties, required, refSchema, base, and typed
to implement these changes.
- Around line 352-372: The OpenAPI output currently exposes only a loose "q" or
"{ args }" schema via inputSchemaRef and leaves those fields optional, but
runtime expects a sibling "meta" and enforces required properties on the args
shape; update the builder that sets operation['parameters'] and
operation['requestBody'] (the branches using inputSchemaRef, isQueryOp,
operation, JSON_CT, modelName, op) to instead construct and reference a combined
object schema (e.g., combinedInputSchemaRef) that has properties: "meta"
(SuperJSON-compatible schema or the existing meta schema), and either "q" (for
query ops) or "args" (for non-query ops) whose schema is inputSchemaRef; set the
combined schema's "required" array to include the child fields that are required
by the args shape (and include "q"/"args" when the args schema has required
properties) so the OpenAPI contract matches runtime validation, and keep using
JSON_CT for the content type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d7d51933-fb90-4336-b7cc-104585c16070
📒 Files selected for processing (3)
packages/orm/src/client/zod/factory.tspackages/server/src/api/rpc/openapi.tspackages/server/test/openapi/baseline/rpc.baseline.yaml
There was a problem hiding this comment.
♻️ Duplicate comments (3)
packages/server/src/api/rpc/openapi.ts (3)
524-528:⚠️ Potential issue | 🟠 MajorModel entity schemas don't respect slicing options.
generateSharedSchemas()iterates over all models (this.schema.models) without filtering byqueryOptions. This leaks excluded models intocomponents/schemas, violating the promised slicing behavior.Proposed fix
// Generate a response-side entity schema for every model const modelEntitySchemas: Record<string, SchemaObject> = {}; -for (const [modelName, modelDef] of Object.entries(this.schema.models)) { +for (const modelName of getIncludedModels(this.schema, this.queryOptions)) { + const modelDef = this.schema.models[modelName]!; modelEntitySchemas[modelName] = this.buildModelEntitySchema(modelDef); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 524 - 528, generateSharedSchemas currently iterates over this.schema.models and unconditionally builds entries into modelEntitySchemas via buildModelEntitySchema, which ignores any slicing/queryOptions and leaks excluded models into components/schemas; update generateSharedSchemas to consult the same queryOptions/slicing logic used elsewhere (e.g., the code path that selects models for operations) and only iterate/build for models allowed by those options (filter this.schema.models by the queryOptions-approved model names before calling buildModelEntitySchema), ensuring modelEntitySchemas and resulting components/schemas contain only sliced-in models.
136-138:⚠️ Potential issue | 🟠 MajorFactory doesn't respect slicing options.
The
createQuerySchemaFactoryis called withoutqueryOptions, sotoJSONSchema()builds schemas for all models regardless ofincludedModels/excludedModelssettings. Excluded models will leak into the OpenAPI components.Pass
queryOptionsto the factory:Proposed fix
constructor(private readonly handlerOptions: RPCApiHandlerOptions<Schema>) { - this.factory = createQuerySchemaFactory(handlerOptions.schema); + this.factory = createQuerySchemaFactory(handlerOptions.schema, handlerOptions.queryOptions); }Does ZenStack createQuerySchemaFactory accept queryOptions as a second parameter?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 136 - 138, The factory is created without query slicing options so toJSONSchema() emits schemas for all models; in the constructor of the class that takes handlerOptions: RPCApiHandlerOptions<Schema>, pass the queryOptions into createQuerySchemaFactory (call createQuerySchemaFactory(handlerOptions.schema, handlerOptions.queryOptions)) so the resulting factory respects includedModels/excludedModels when building OpenAPI components.
652-704:⚠️ Potential issue | 🟠 MajorOptional fields incorrectly marked as required.
The
buildModelEntitySchemamethod pushes all non-relation fields to therequiredarray unconditionally, ignoringfieldDef.optional. This causes generated clients/validators to reject valid RPC responses where optional fields are absent.Additionally, optional to-one relations should be modeled as nullable (
anyOf: [ref, { type: 'null' }]) when included but absent.Proposed fix
for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) { if (fieldDef.omit) continue; if (fieldDef.relation) { // Relation fields appear only with `include` — mark as optional const refSchema: ReferenceObject = { $ref: `#/components/schemas/${fieldDef.type}` }; - properties[fieldName] = fieldDef.array ? { type: 'array', items: refSchema } : refSchema; + const base: SchemaObject | ReferenceObject = fieldDef.array + ? { type: 'array', items: refSchema } + : refSchema; + properties[fieldName] = + fieldDef.optional && !fieldDef.array ? { anyOf: [base, { type: 'null' }] } : base; } else if (this.schema.enums?.[fieldDef.type]) { // Enum field const refSchema: ReferenceObject = { $ref: `#/components/schemas/${fieldDef.type}` }; const base: SchemaObject | ReferenceObject = fieldDef.array ? { type: 'array', items: refSchema } : refSchema; properties[fieldName] = fieldDef.optional ? { anyOf: [base, { type: 'null' }] } : base; - required.push(fieldName); + if (!fieldDef.optional) required.push(fieldName); } else if (this.isBuiltinType(fieldDef.type)) { // Scalar builtin field const base = this.builtinTypeToJsonSchema(fieldDef.type as BuiltinType); const fieldDesc = getMetaDescription(fieldDef.attributes); if (fieldDesc) base.description = fieldDesc; const typed: SchemaObject = fieldDef.array ? { type: 'array', items: base } : base; properties[fieldName] = fieldDef.optional ? { anyOf: [typed, { type: 'null' }] } : typed; - required.push(fieldName); + if (!fieldDef.optional) required.push(fieldName); } else if (this.schema.typeDefs?.[fieldDef.type]) { // TypeDef field — reference the registered typedef schema const refSchema: ReferenceObject = { $ref: `#/components/schemas/${fieldDef.type}` }; const base: SchemaObject | ReferenceObject = fieldDef.array ? { type: 'array', items: refSchema } : refSchema; properties[fieldName] = fieldDef.optional ? { anyOf: [base, { type: 'null' }] } : base; - required.push(fieldName); + if (!fieldDef.optional) required.push(fieldName); } else { // Unknown type — represent as a generic object const typed: SchemaObject = fieldDef.array ? { type: 'array', items: { type: 'object' } } : { type: 'object' }; properties[fieldName] = typed; - required.push(fieldName); + if (!fieldDef.optional) required.push(fieldName); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 652 - 704, The buildModelEntitySchema function is marking non-relation fields as required regardless of fieldDef.optional and not modeling optional to-one relations as nullable; update buildModelEntitySchema so that required.push(fieldName) only happens when fieldDef.optional is false (for enum, builtin, typedef and fallback branches) and do not push relation fields at all; for relation fields, when fieldDef.optional is true and the relation is to-one (i.e., not fieldDef.array), wrap the reference in a nullable union (anyOf: [refSchema, { type: 'null' }]) instead of a plain $ref so optional relations are represented as nullable. Ensure properties assignments use the new nullable/anyOf when appropriate and only add names to required when explicitly non-optional.
🧹 Nitpick comments (1)
packages/server/src/api/rpc/openapi.ts (1)
246-287: Missing tag definitions for$procsand$transaction.The
generateTags()method only creates tags for models, but procedures use$procs(line 408) and transactions use$transaction(line 488). These tags aren't defined in the OpenAPI document'stagsarray, which degrades spec quality for documentation generators.Proposed fix
private generateTags(): OpenAPIV3_1.TagObject[] { - return getIncludedModels(this.schema, this.queryOptions).map((modelName) => ({ + const modelTags = getIncludedModels(this.schema, this.queryOptions).map((modelName) => ({ name: lowerCaseFirst(modelName), description: `${modelName} operations`, })); + return [ + ...modelTags, + { name: '$procs', description: 'Procedure operations' }, + { name: '$transaction', description: 'Transaction operations' }, + ]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 246 - 287, generateTags() currently returns tags only for models, but procedure and transaction endpoints use the tags "$procs" and "$transaction" (see PROCEDURE_ROUTE_PREFIXES, buildProcedureOperation, buildTransactionOperation), so add TagObject entries for these two tags to the array returned by generateTags(): append { name: '$procs', description: 'Procedures' } and { name: '$transaction', description: 'Transactions' } (or similar descriptive text) so the OpenAPI document's tags include those used by the procedure and transaction paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 524-528: generateSharedSchemas currently iterates over
this.schema.models and unconditionally builds entries into modelEntitySchemas
via buildModelEntitySchema, which ignores any slicing/queryOptions and leaks
excluded models into components/schemas; update generateSharedSchemas to consult
the same queryOptions/slicing logic used elsewhere (e.g., the code path that
selects models for operations) and only iterate/build for models allowed by
those options (filter this.schema.models by the queryOptions-approved model
names before calling buildModelEntitySchema), ensuring modelEntitySchemas and
resulting components/schemas contain only sliced-in models.
- Around line 136-138: The factory is created without query slicing options so
toJSONSchema() emits schemas for all models; in the constructor of the class
that takes handlerOptions: RPCApiHandlerOptions<Schema>, pass the queryOptions
into createQuerySchemaFactory (call
createQuerySchemaFactory(handlerOptions.schema, handlerOptions.queryOptions)) so
the resulting factory respects includedModels/excludedModels when building
OpenAPI components.
- Around line 652-704: The buildModelEntitySchema function is marking
non-relation fields as required regardless of fieldDef.optional and not modeling
optional to-one relations as nullable; update buildModelEntitySchema so that
required.push(fieldName) only happens when fieldDef.optional is false (for enum,
builtin, typedef and fallback branches) and do not push relation fields at all;
for relation fields, when fieldDef.optional is true and the relation is to-one
(i.e., not fieldDef.array), wrap the reference in a nullable union (anyOf:
[refSchema, { type: 'null' }]) instead of a plain $ref so optional relations are
represented as nullable. Ensure properties assignments use the new
nullable/anyOf when appropriate and only add names to required when explicitly
non-optional.
---
Nitpick comments:
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 246-287: generateTags() currently returns tags only for models,
but procedure and transaction endpoints use the tags "$procs" and "$transaction"
(see PROCEDURE_ROUTE_PREFIXES, buildProcedureOperation,
buildTransactionOperation), so add TagObject entries for these two tags to the
array returned by generateTags(): append { name: '$procs', description:
'Procedures' } and { name: '$transaction', description: 'Transactions' } (or
similar descriptive text) so the OpenAPI document's tags include those used by
the procedure and transaction paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8af2befa-4177-4e3f-a1e1-620e662c121e
📒 Files selected for processing (2)
packages/server/src/api/rpc/openapi.tspackages/server/test/openapi/baseline/rpc.baseline.yaml
- Mark 'q' as required for GET ops whose input schema has required fields
(findUnique, delete, update etc. require 'where'); aggregate and groupBy
are also required since _count/_by must be specified
- Add optional 'meta' query parameter to all GET endpoints (companion to
'q' for SuperJSON deserialization via unmarshalQ)
- Mark procedure envelope 'args' as required when the proc has non-optional
parameters; mark 'q' as required for query procs with required params
- Omit 'q'/'meta' parameters entirely for parameter-less query procedures;
omit requestBody entirely for parameter-less mutation procedures
- Fix aggregate args schema: use .meta({ required: ['_count'] }) to surface
_count as required in JSON Schema without changing the TypeScript type
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (919 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
A to-one relation marked optional can return null when not included via
`include`. Wrap such fields in `anyOf: [$ref, { type: 'null' }]` to
match runtime behavior. Array relations and required to-one relations
are unaffected.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (919 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
…chema generation - Use toEqual instead of toMatchObject in baseline tests for strict deep equality (no extra fields tolerated, key order irrelevant) - Remove removeNumberBounds override from toJSONSchema — no longer needed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (934 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
Register _integer and _nonNegativeInteger as shared component schemas and replace all inline occurrences (219 total) with $refs during the registry transform pass. Reduces spec size and improves readability. Also switch baseline assertions from toMatchObject to toEqual for strict deep equality (key-order insensitive). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (934 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
… OpenAPI spec - Pass queryOptions to factory constructor so Zod registry respects slicing (excluded models' arg schemas are not registered) - Add QueryOptions overload to createQuerySchemaFactory for clean typing - Filter generateSharedSchemas entity schemas by getIncludedModels so excluded models don't appear in components/schemas - Add generateSpec() helper in tests that auto-validates every spec against OpenAPI 3.1 — all 77 tests now validate the produced spec - Add tests for excludedModels/includedModels component schema filtering - Add tests for excludedFilterKinds/includedFilterKinds/$all field slicing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (934 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (934 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (934 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
|
Claude finished @ymc9's task —— View job Code Review of RPC OpenAPI Spec Generation ✅Todo List:
Overall Assessment: Excellent Implementation with Critical Issues to FixThis PR delivers a well-architected, comprehensive RPC OpenAPI spec generator with strong attention to detail and code quality. The implementation follows established patterns and integrates cleanly with the existing codebase. However, there are several critical issues identified by CodeRabbit that must be addressed before merge. Code Quality & Architecture ⭐⭐⭐⭐⭐Strengths:
Architecture Highlights:
Performance Considerations ⭐⭐⭐⭐Excellent Optimizations:
Performance Characteristics:
Security Analysis ⭐⭐⭐⭐⭐Security Features:
The Test Coverage ⭐⭐⭐⭐⭐Comprehensive Test Suite (934 lines):
The test quality is exceptional with proper baseline/snapshot testing and validation against OpenAPI 3.1 specification. Critical Issues Found (CodeRabbit Analysis Confirmed)
|
1 participant
Summary
RPCApiSpecGeneratorthat produces an OpenAPI 3.1 spec for the RPC-style API, mirroring the existing REST spec generatormayDenyAccess,getMetaDescription) intospec-utils.tsso both REST and RPC generators use them without duplicationmakeProcedureArgsSchemato eagerly register procedure arg schemas, allowing the spec to reference them via$refinstead of inliningOPERATION_INFOmap@@meta/@meta("description", ...)on models and scalar fieldsincludedModels,excludedModels, per-modelincludedOperations/excludedOperations,includedProcedures/excludedProcedures)respectAccessPoliciesis enabled and access cannot be statically proven to always be allowedTest plan
packages/server/test/openapi/rpc-openapi.test.tscovering all generator behaviorrpc.baseline.yaml) validated against OpenAPI 3.1🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests