merge dev to main (v3.2.0) #580
Conversation
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* WIP * WIP: implement read policies * fix tests * Update packages/plugins/policy/src/policy-handler.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix name mapper * fix tests * fix build * implement update field-level policies * update tests * update tests * add more tests * add more tests * simplify queries --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix: add missing "files" entries to package.json * Update packages/cli/package.json Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* feat(cli): implement watch mode for generate * chore(root): update pnpm-lock.yaml * chore(cli): track all model declaration and removed paths, logs in past tense * fix(cli): typo, unused double array from * fix(orm): preserve zod validation errors when validating custom json types * update * chore(cli): move import, fix parallel generation on watch * feat(common-helpers): implement single-debounce * chore(cli): use single-debounce for debouncing * feat(common-helpers): implement single-debounce * fix(common-helpers): re run single-debounce --------- Co-authored-by: Yiming Cao <yiming@whimslab.io> Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* feat: custom procs
* chore: cleanup
* fix: remove $procedures from client
* fix: failing test due to previous alias
* feat(custom-procs)!: make procedures envelope-only via $procs
- Switch procedure calls to `db.$procs.name({ args: {...} })` (no positional args)
- Remove legacy `$procedures` alias entirely (client API + server routing/logging)
- Validate procedure envelope input (`args` object, required/unknown keys)
- Keep TanStack Query procedure hooks as `(args, options)` (with conditional args optionality)
- Update server/ORM/client tests for the envelope API
* fix: code review feedback
* fix: code review comments
* fix: coderabbit review comments
* fix: remove useless proxy method
* test: add a couple of e2e tests that verify both typing and runtime
* test: improve e2e tests
* test: add missing mutation flag
* regenerate test schema
* refactor: procedure params generation fix and type refactors
- Simplified procedure's params definition from a tuple an object, since procs are now called with an envelop now
- Refactored procedure related typing to make them more consistent with other CURD types (that usually takes the schema as the first type parameter, and a name as the second)
- Moved detailed procedure's types to "crud-types" where other ORM client detailed types are defined
- Removed some type duplication from hooks side
- Updated the "orm" sample to demonstrate procedures
* fix: disable infinite custom proc queries for now
---------
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* feat(cli): implement watch mode for generate * chore(root): update pnpm-lock.yaml * chore(cli): track all model declaration and removed paths, logs in past tense * fix(cli): typo, unused double array from * fix(orm): preserve zod validation errors when validating custom json types * update * chore(cli): move import, fix parallel generation on watch * feat(common-helpers): implement single-debounce * chore(cli): use single-debounce for debouncing * feat(common-helpers): implement single-debounce * fix(common-helpers): re run single-debounce * fix(tanstack): avoid invalidating queries for custom proc mutations * add missing file * fix formatting --------- Co-authored-by: FTB_lag <tabolskyy.git@gmail.com>
* feat(cli): implement watch mode for generate * chore(root): update pnpm-lock.yaml * chore(cli): track all model declaration and removed paths, logs in past tense * fix(cli): typo, unused double array from * fix(orm): preserve zod validation errors when validating custom json types * update * chore(cli): move import, fix parallel generation on watch * feat(common-helpers): implement single-debounce * chore(cli): use single-debounce for debouncing * feat(common-helpers): implement single-debounce * fix(common-helpers): re run single-debounce * fix(qaas): add options validation * fix pr comments --------- Co-authored-by: FTB_lag <tabolskyy.git@gmail.com>
* feat(cli): implement watch mode for generate * chore(root): update pnpm-lock.yaml * chore(cli): track all model declaration and removed paths, logs in past tense * fix(cli): typo, unused double array from * fix(orm): preserve zod validation errors when validating custom json types * update * chore(cli): move import, fix parallel generation on watch * feat(common-helpers): implement single-debounce * chore(cli): use single-debounce for debouncing * feat(common-helpers): implement single-debounce * fix(common-helpers): re run single-debounce * chore: regenerate schemas --------- Co-authored-by: FTB_lag <tabolskyy.git@gmail.com>
* fix(orm): add special treatment to enum arrays for postgres db - For input, the string array needs to be casted with "Enum"[] - For output, the raw pg array string needs to be parsed back to a proper JS string array fixes #576 * fix enum array filtering and improve test cases * update * addressing PR comments
📝 WalkthroughWalkthroughIntroduces first-class procedures and exists() across ORM, server, and client layers; adds CLI generate --watch with file-watching and debounce; extends policy validation and policy handler for field-level rules; adds Postgres enum-array support and multiple SDK/sample updates; bumps many package versions to 3.2.0. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
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
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 |
|
Claude finished @ymc9's task —— View job Code Review for v3.2.0 Release (dev → main)I've reviewed this substantial PR merging dev to main for the v3.2.0 release. This introduces significant new functionality including custom procedures, exists operation, and CLI improvements. Review Tasks:
🔍 Major Features Introduced1. Custom Procedures (PR #551, #2172614)
2. Exists Operation (PR #550, #446a483)
3. CLI Watch Mode (PR #573, #554)
✅ Code Quality & Best PracticesStrengths:
Best Practices Followed:
|
There was a problem hiding this comment.
Pull request overview
This pull request merges the dev branch to main for v3.2.0 release. The changes introduce two major new features: the exists query operation and procedures support. Additionally, comprehensive field-level policy tests have been migrated from v2.
Key Changes:
- Added
ExistsArgstype export andexists()method support across all generated schema files - Introduced procedures functionality with query and mutation support, including runtime implementations and tests
- Version bumped from 3.1.1 to 3.2.0 across all packages and samples
- Migrated and added extensive field-level policy tests
- Enhanced sample applications with procedures demonstrations (signup, listPublicPosts)
- Consolidated schemas in samples using symlinks to a shared schema file
Reviewed changes
Copilot reviewed 144 out of 151 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/regression/test/issue-576.test.ts | New regression test for enum array field support |
| tests/e2e/orm/policy/migrated/field-level-policy.test.ts | Comprehensive field-level policy tests migrated from v2 (1766 lines) |
| tests/e2e/orm/policy/crud/field-level.test.ts | New field-level policy tests for mixins and delegates (594 lines) |
| tests/e2e/orm/client-api/procedures.test.ts | New test suite for procedures functionality |
| tests/e2e/orm/client-api/exists.test.ts | New test suite for exists() API |
| tests/e2e/orm/schemas/procedures/* | New schema files for procedures testing |
| packages/server/test/api/*.test.ts | Added procedure-related tests and options validation |
| Multiple input.ts files | Added ExistsArgs type exports for all models |
| Multiple schema*.ts files | Added procedures definitions |
| samples/* | Enhanced with procedures and routing changes |
| scripts/bump-version.ts | Removed VSCode package exclusion, code formatting improvements |
| packages/testtools/src/client.ts | Added debug option for prisma db push |
| pnpm-lock.yaml | Updated postgres-array dependency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
samples/nuxt/zenstack/seed.ts (1)
13-13: Add error handling for the async operation.The
main()call is not wrapped in error handling. If any database operation fails, the error will be unhandled and could go unnoticed during seeding.🛡️ Proposed fix to add error handling
-main(); +main() + .then(() => { + console.log('Seeding completed successfully'); + process.exit(0); + }) + .catch((error) => { + console.error('Seeding failed:', error); + process.exit(1); + });packages/plugins/policy/plugin.zmodel (1)
9-16: Stale documentation references removedoverrideparameter.The JSDoc comment at lines 10-12 still mentions "You can pass a third argument as
trueto make it override the model-level policies," but the attribute declaration at line 16 only accepts two parameters (operationandcondition). Theoverrideparameter appears to have been removed.📝 Proposed fix to update the documentation
/** * Defines an access policy that allows the annotated field to be read or updated. - * You can pass a third argument as `true` to make it override the model-level policies. * * @param operation: comma-separated list of "read", "update". Use "all" to denote all operations. * @param condition: a boolean expression that controls if the operation should be allowed. */packages/clients/tanstack-query/src/svelte/index.svelte.ts (1)
371-421: Missing merge for rootOptions in multiple hooks.Several hooks don't merge
rootOptionswith per-calloptions, inconsistent withuseFindUnique,useFindFirst,useExists, anduseFindManywhich do usemerge(rootOptions, options). This means context-level options (endpoint, fetch, logging) won't be applied to these hooks.Affected hooks:
useInfiniteFindMany(line 372)- All mutation hooks:
useCreate,useCreateMany,useCreateManyAndReturn,useUpdate,useUpdateMany,useUpdateManyAndReturn,useUpsert,useDelete,useDeleteManyuseCount,useAggregate,useGroupByThe Vue implementation correctly uses
merge(rootOptions, options)for all of these.🐛 Proposed fix
useInfiniteFindMany: (args: any, options?: any) => { - return useInternalInfiniteQuery(schema, modelName, 'findMany', args, options); + return useInternalInfiniteQuery(schema, modelName, 'findMany', args, merge(rootOptions, options)); }, useCreate: (options?: any) => { - return useInternalMutation(schema, modelName, 'POST', 'create', options); + return useInternalMutation(schema, modelName, 'POST', 'create', merge(rootOptions, options)); }, useCreateMany: (options?: any) => { - return useInternalMutation(schema, modelName, 'POST', 'createMany', options); + return useInternalMutation(schema, modelName, 'POST', 'createMany', merge(rootOptions, options)); }, useCreateManyAndReturn: (options?: any) => { - return useInternalMutation(schema, modelName, 'POST', 'createManyAndReturn', options); + return useInternalMutation(schema, modelName, 'POST', 'createManyAndReturn', merge(rootOptions, options)); }, useUpdate: (options?: any) => { - return useInternalMutation(schema, modelName, 'PUT', 'update', options); + return useInternalMutation(schema, modelName, 'PUT', 'update', merge(rootOptions, options)); }, useUpdateMany: (options?: any) => { - return useInternalMutation(schema, modelName, 'PUT', 'updateMany', options); + return useInternalMutation(schema, modelName, 'PUT', 'updateMany', merge(rootOptions, options)); }, useUpdateManyAndReturn: (options?: any) => { - return useInternalMutation(schema, modelName, 'PUT', 'updateManyAndReturn', options); + return useInternalMutation(schema, modelName, 'PUT', 'updateManyAndReturn', merge(rootOptions, options)); }, useUpsert: (options?: any) => { - return useInternalMutation(schema, modelName, 'POST', 'upsert', options); + return useInternalMutation(schema, modelName, 'POST', 'upsert', merge(rootOptions, options)); }, useDelete: (options?: any) => { - return useInternalMutation(schema, modelName, 'DELETE', 'delete', options); + return useInternalMutation(schema, modelName, 'DELETE', 'delete', merge(rootOptions, options)); }, useDeleteMany: (options?: any) => { - return useInternalMutation(schema, modelName, 'DELETE', 'deleteMany', options); + return useInternalMutation(schema, modelName, 'DELETE', 'deleteMany', merge(rootOptions, options)); }, useCount: (args: any, options?: any) => { - return useInternalQuery(schema, modelName, 'count', args, options); + return useInternalQuery(schema, modelName, 'count', args, merge(rootOptions, options)); }, useAggregate: (args: any, options?: any) => { - return useInternalQuery(schema, modelName, 'aggregate', args, options); + return useInternalQuery(schema, modelName, 'aggregate', args, merge(rootOptions, options)); }, useGroupBy: (args: any, options?: any) => { - return useInternalQuery(schema, modelName, 'groupBy', args, options); + return useInternalQuery(schema, modelName, 'groupBy', args, merge(rootOptions, options)); },
🤖 Fix all issues with AI agents
In @packages/cli/package.json:
- Line 42: The package.json that declares "chokidar": "^5.0.0" needs a Node.js
engine constraint to guarantee compatibility with ESM-only chokidar v5; add an
"engines" field to that package.json (or the repository root package.json if you
centralize engine policy) with "node": ">=20" so tooling and consumers know Node
20+ is required and avoid runtime failures.
In @packages/clients/tanstack-query/src/svelte/index.svelte.ts:
- Around line 151-183: The ProcedureHookGroup type currently comments out
useInfiniteQuery while the runtime still exposes a useInfiniteQuery hook,
causing a type/runtime mismatch; either remove the runtime hook or reintroduce
the type. To fix, update ProcedureHookGroup to include the useInfiniteQuery
entry (uncomment and use the same signature pattern as useQuery) with the
correct generic types: useInfiniteQuery: ProcedureHookFn<Schema, Name,
ModelInfiniteQueryOptions<ProcedureReturn<Schema, Name>>,
ModelInfiniteQueryResult<InfiniteData<ProcedureReturn<Schema, Name>>>> so the
declared type matches the runtime-provided useInfiniteQuery, or alternatively
remove the runtime creation of useInfiniteQuery to match the current
commented-out type.
In @packages/clients/tanstack-query/src/vue.ts:
- Around line 144-185: The ProcedureHookGroup type currently omits
useInfiniteQuery (it’s commented out) while the runtime exposes useInfiniteQuery
(mismatch between type and implementation); update ProcedureHookGroup to include
a useInfiniteQuery signature that mirrors the runtime implementation (use the
same generics as useQuery but with
ModelInfiniteQueryOptions<ProcedureReturn<Schema, Name>> and return type
ModelInfiniteQueryResult<InfiniteData<ProcedureReturn<Schema, Name>>>), or
alternatively remove/guard the runtime exposure of useInfiniteQuery where it’s
created — ensure the unique symbols ProcedureHookGroup, useInfiniteQuery,
ProcedureHookFn, ModelInfiniteQueryOptions, and ModelInfiniteQueryResult are
used so the type and runtime match.
In @packages/server/src/api/rest/index.ts:
- Around line 58-67: The Zod schema for the REST handler options currently
rejects Infinity for pageSize; update the validation in validateOptions (the
schema that defines pageSize) to explicitly allow Infinity (for example by
accepting z.number().positive().or(z.literal(Infinity)).optional()) so the JSDoc
“Set to Infinity to disable pagination” remains valid, and make the same change
for the other occurrences noted (the schemas referenced around the other
validateOptions usages). Ensure the change targets the pageSize field in the
option schema(s) so Infinity no longer triggers an "Invalid options" error.
In @samples/next.js/app/page.tsx:
- Line 147: The <ul> in samples/next.js/app/page.tsx currently includes the
"container" utility which can conflict with the parent <main> that already uses
"max-w-3xl" in layout.tsx; remove the "container" class from the <ul> (leave the
other classes like "flex flex-col gap-2") so the parent width control is not
overridden and spacing remains consistent.
- Around line 156-167: Buttons rendered in the Post list (the button elements
that call onDeletePost(post.id) and onTogglePublishPost(post)) use "text-white"
without a background, causing invisible text on the parent bg-white; update the
className for both buttons to remove "text-white" and apply visible styling such
as a readable text color (e.g., "text-gray-700" or "text-gray-900") or add a
contrasting background (e.g., "bg-gray-800 text-white") plus appropriate
hover/focus styles so the labels "Delete" and "Publish/Unpublish" are visible on
light backgrounds.
In @samples/next.js/lib/db.ts:
- Around line 11-14: The signUp procedure is insecure because it spreads args
directly into client.user.create; instead validate and whitelist allowed fields
before calling client.user.create. Update the signUp handler to parse/validate
args (e.g., using schema validation or explicit field selection) and build a
safe data object containing only permitted properties (such as email, password,
name) — do not pass through unexpected keys like admin flags or internal IDs —
then call client.user.create with that sanitized data; refer to the signUp
function and client.user.create to locate and modify the code.
In @samples/nuxt/app/pages/index.vue:
- Around line 36-54: The onCreatePost handler currently only checks that
users.value is defined but not that it's non-empty; before computing
Math.floor(Math.random() * users.value.length) add a guard (e.g., if
(!users.value || users.value.length === 0) { /* early return or show error */
return; }) and remove the non-null assertion when selecting forUser (use a safe
indexed access after the length check) so you never pick from an empty array and
avoid a masked runtime error in onCreatePost.
In @samples/orm/main.ts:
- Around line 20-23: The signUp procedure currently spreads args directly into
client.user.create (signUp and client.user.create), which allows callers to set
sensitive fields like role; fix it by adding explicit input validation and field
whitelisting: define a strict input schema (e.g., Zod or your framework's
schema) that only permits allowed fields (email, password, name, etc.), map or
pick those validated fields into a new payload object, and pass that payload to
client.user.create instead of ...args; also ensure server-side enforcement by
excluding/overwriting sensitive fields (role, id, createdAt, isAdmin) before
create and update procedure types/tests to cover attempts to set forbidden
fields.
In @samples/sveltekit/src/lib/db.ts:
- Around line 11-14: The signUp procedure is directly spreading args into
client.user.create which allows callers to set sensitive fields (e.g., role,
id); update signUp to validate and whitelist input fields before calling
client.user.create: define an explicit input schema (or use a zod schema)
listing only allowed properties (e.g., email, password, name), reject or strip
any extra keys, and pass only those whitelisted values to client.user.create;
ensure any sensitive server-only fields (role, id, createdAt, etc.) are never
included and are set server-side if needed.
In @samples/sveltekit/src/routes/+layout.svelte:
- Around line 22-31: The img tag with src="/svelte.png" declares incorrect
dimensions width="100" height="20", which distorts the actual 429x513 image;
update the <img src="/svelte.png" ...> element to use dimensions that preserve
the 429:513 aspect ratio (e.g., width="100" height="120" or width="50"
height="60"), or remove the hardcoded width/height and use CSS to constrain size
while preserving aspect ratio.
In @samples/sveltekit/src/routes/+page.svelte:
- Around line 147-158: The Delete and Publish buttons render with only
text-white and no background, making them illegible on light backgrounds; update
the button elements that call onDeletePost(post.id) and
onTogglePublishPost(post) (and reference post.published) to include contrasting
background utility classes (e.g., bg-red-600 for Delete, bg-green-600 or
bg-blue-600 for Publish/Unpublish), add appropriate hover/focus variants (e.g.,
hover:bg-..., focus:outline-none focus:ring) and ensure sufficient color
contrast and accessible labels so the actions remain visible and
keyboard/narration-friendly.
In @scripts/bump-version.ts:
- Line 7: The excludes array (symbol: excludes) is empty so the VSCode extension
gets bumped; update the excludes array in scripts/bump-version.ts to include the
VSCode extension package identifier (e.g., "packages/ide/vscode" or
"packages/ide/vscode/") so that the VSCode extension is explicitly omitted from
automated version bumps, keeping the rest of the script logic unchanged.
In @tests/e2e/orm/client-api/procedures.test.ts:
- Around line 35-41: The comment above the setAdmin procedure is misleading: it
says "Query procedure that returns Void" but setAdmin performs a mutation via
client.user.update; update the comment to accurately describe it as a mutation
(e.g., "Mutation procedure that sets a user as ADMIN and returns Void" or
similar) so the intent matches the implementation in the setAdmin async
function.
In @tests/regression/test/issue-576.test.ts:
- Line 4: The describe block declaration describe('regression test for issue
576', async () => { should not use an async callback; remove the async keyword
so the callback is synchronous (describe('regression test for issue 576', () =>
{) to avoid Vitest test discovery issues and unexpected behavior.
🧹 Nitpick comments (27)
tests/e2e/orm/policy/crud/field-level.test.ts (1)
343-593: Consider adding create operation tests for delegate models.The mixin tests (lines 168-195) include create operations with read-back validation, but the delegate model tests section doesn't have equivalent coverage. For consistency and completeness, consider adding similar create tests to validate that field-level policies from parent delegates are enforced on read-back after creation.
💡 Example create test for delegate models
// Test create operations with read-back in delegate models r = await anonDb.blogPost.create({ data: { id: 2, title: 'New Post', secretNotes: 'NEW_SECRET', publicContent: 'NEW_PUBLIC', createdBy: 'author2', body: 'New body', category: 'news', tags: 'new', internalNotes: 'NEW_INTERNAL', }, }); expect(r.secretNotes).toBeNull(); // from BaseContent, can't read back expect(r.createdBy).toBeNull(); // from BaseContent, can't read back expect(r.category).toBeNull(); // from Article, can't read back expect(r.internalNotes).toBeNull(); // from BlogPost, can't read back expect(r.publicContent).toEqual('NEW_PUBLIC');tests/regression/test/issue-576.test.ts (1)
5-73: Consider expanding test coverage for array filter operations.The test covers the core enum array functionality well. For more comprehensive coverage, consider adding tests for:
- Additional array filters:
isEmpty,hasSome,hasEvery- Edge cases: empty arrays (
tags: []), null handling- Bulk operations with enum filters:
count(),updateMany(),deleteMany()Based on learnings, ORM tests should be comprehensive. However, as a regression test targeting a specific issue, the current coverage may be sufficient.
tests/e2e/orm/policy/migrated/field-level-policy.test.ts (1)
64-74: Duplicate test assertions detected.Lines 64-66 and 72-74 contain identical queries and assertions. This appears to be unintentional duplication from the migration.
♻️ Suggested cleanup
r = await db.user.findUnique({ where: { id: 1 }, select: { models: { select: { y: true } } } }); expect(r.models[0].y).toBeNull(); expect(r.models[0].z).toBeUndefined(); r = await db.user.findUnique({ where: { id: 1 }, include: { models: true } }); expect(r.models[0].y).toBeNull(); expect(r.models[0].z).toBeNull(); - - r = await db.user.findUnique({ where: { id: 1 }, select: { models: { select: { y: true } } } }); - expect(r.models[0].y).toBeNull(); - expect(r.models[0].z).toBeUndefined();samples/nuxt/app/pages/index.vue (2)
56-67: Consider adding error handling for mutations.While acceptable for a sample application, the mutation operations lack error handling. Consider adding
.onErrorcallbacks to provide user feedback when operations fail.💡 Example error handling pattern
const onDeletePost = (postId: string) => { deletePost.mutate( { where: { id: postId }, }, { onError: (error) => { console.error('Failed to delete post:', error); // Show user-friendly error notification }, } ); };
70-154: Well-structured template with proper conditional rendering.The template correctly handles edge cases:
- Shows appropriate message when no users exist (line 71-73)
- Displays "pending" state for optimistic updates (line 132)
- Conditionally renders author information only for non-optimistic posts (line 149), preventing potential null access during optimistic updates
Consider enhancing accessibility by adding:
aria-labelattributes to buttonsaria-live="polite"region for optimistic update feedback- Proper heading hierarchy
packages/plugins/policy/src/policy-handler.ts (3)
289-296: Consider using optional chaining for defensive coding.At line 290,
preUpdateResult.rows[0].$filteredCountaccesses the first row directly. While the aggregate query (withCOALESCE) should always return a single row, using optional chaining would make this more resilient to edge cases and satisfy TypeScript's type checker.🔧 Proposed fix
const preUpdateResult = await proceed(preUpdateCheckQuery.toOperationNode()); - if (preUpdateResult.rows[0].$filteredCount > 0) { + if ((preUpdateResult.rows[0]?.$filteredCount ?? 0) > 0) { throw createRejectedByPolicyError( mutationModel, RejectedByPolicyReason.NO_ACCESS, 'some rows cannot be updated due to field policies', ); }
645-654: Misleading comment about default policy behavior.The comment at lines 645-646 states "'post-update' is by default allowed, other operations are by default denied," but this appears to be copied from model-level policy logic. For field-level policies:
post-updateis not a valid operation (excluded byFieldLevelPolicyOperationstype)- The actual behavior (lines 648-650) is that field access is allowed by default when no
@allowrules exist📝 Proposed fix to clarify the comment
- // 'post-update' is by default allowed, other operations are by default denied + // field access is allowed by default when no @allow rules are defined let combinedPolicy: OperationNode; if (allows.length === 0) { // field access is allowed by default combinedPolicy = trueNode(this.dialect);
1277-1284: Minor naming inconsistency:NonexistingvsNonexistent.The existing method at line 1271 uses
tryRejectNonexistentModelwhile this new method usestryRejectNonexistingTables. Consider aligning the naming for consistency.🔧 Proposed fix
- private tryRejectNonexistingTables(tables: readonly OperationNode[]) { + private tryRejectNonexistentTables(tables: readonly OperationNode[]) {Also update the call sites at lines 309, 444, and 480.
samples/sveltekit/src/routes/+page.svelte (1)
135-135: Verify container class configuration.Ensure the
containerutility is properly configured in your Tailwind CSS setup, as Tailwind v4 may require explicit configuration for container behavior.packages/language/test/procedure-validation.test.ts (2)
5-16: Consider more specific error assertions.The test uses a disjunction pattern
/unknown type|could not resolve reference/i, which passes if either message appears. This makes the test less strict and could hide changes in error reporting.♻️ Suggestion for more specific assertion
Choose one canonical error message pattern or test for both separately:
it('rejects unknown parameter type', async () => { await loadSchemaWithError( ` model User { id Int @id } procedure foo(a: NotAType): Int `, - /unknown type|could not resolve reference/i, + /could not resolve reference.*NotAType/i, ); });
31-42: Consider testing multiple reserved names.The test validates that
__proto__is rejected, but other reserved names (if any) are not tested. Consider parameterizing this test or adding coverage for all reserved procedure names.💡 Example with test.each for multiple reserved names
it.each(['__proto__', '__defineGetter__', '__defineSetter__', '__lookupGetter__', '__lookupSetter__'])( 'rejects reserved procedure name: %s', async (reservedName) => { await loadSchemaWithError( ` model User { id Int @id } procedure ${reservedName}(): Int `, /reserved/i, ); } );packages/common-helpers/src/single-debounce.ts (1)
1-1: Typo in parameter name:debounceMcshould bedebounceMs.The parameter
debounceMcappears to be a typo fordebounceMs(milliseconds). Consider renaming for clarity.Proposed fix
-export function singleDebounce(cb: () => void | PromiseLike<void>, debounceMc: number, reRunOnInProgressCall: boolean = false) { +export function singleDebounce(cb: () => void | PromiseLike<void>, debounceMs: number, reRunOnInProgressCall: boolean = false) {Also update line 32:
- timeout = setTimeout(run, debounceMc); + timeout = setTimeout(run, debounceMs);packages/server/src/api/common/utils.ts (1)
7-29: Consider removingasyncsince noawaitis used.The function is declared
asyncbut contains noawaitexpressions. Thetry/catcharoundSuperJSON.deserializeis synchronous. Making this function synchronous would simplify the return type and avoid unnecessary Promise wrapping.♻️ Suggested refactor
-export async function processSuperJsonRequestPayload(payload: unknown) : Promise<{ result: unknown; error: string | undefined; }> { +export function processSuperJsonRequestPayload(payload: unknown): { result: unknown; error: string | undefined } {packages/cli/src/index.ts (1)
224-227: Consider a more robust watch mode detection mechanism.The current implementation checks raw
program.argsarray for string matches of 'generate' and watch flags. This approach is fragile because:
- Commander's internal
program.argsstructure may not reliably contain parsed option strings- String matching against raw arguments is brittle and could break with Commander version changes or argument reordering
- The comment itself acknowledges this is a "hack"
♻️ Recommended approach using a module-level flag
Instead of parsing args manually, track watch mode state explicitly:
+let watchModeActive = false; + const generateAction = async (options: Parameters<typeof actions.generate>[0]): Promise<void> => { + if (options.watch) { + watchModeActive = true; + } await telemetry.trackCommand('generate', () => actions.generate(options)); };Then in the main function:
- if (program.args.includes('generate') && (program.args.includes('-w') || program.args.includes('--watch'))) { - // A "hack" way to prevent the process from terminating because we don't want to stop it. + if (watchModeActive) { + // Keep process alive for watch mode return; }This approach:
- Uses parsed option values instead of raw args
- Is more maintainable and less dependent on Commander internals
- Removes the string-matching brittleness
samples/sveltekit/src/routes/feeds/+page.svelte (1)
37-56: Consider merging the duplicate conditional blocks.Lines 37-50 and 52-56 both check
posts && posts.length > 0. These could be combined into a single conditional block for cleaner code.♻️ Suggested refactor
{#if posts && posts.length > 0} <ul class="flex flex-col gap-4 w-full"> {#each posts as post} <li class="border border-gray-200 dark:border-zinc-700 rounded-lg p-4 hover:shadow-md transition-shadow" > <h2 class="text-xl font-semibold text-black dark:text-zinc-50">{post.title}</h2> <p class="text-sm text-gray-500 dark:text-gray-400 mt-2"> Published on {new Date(post.createdAt).toLocaleDateString()} </p> </li> {/each} </ul> -{/if} -{#if posts && posts.length > 0} <div class="text-sm text-gray-600 dark:text-gray-400 mt-4"> Showing {posts.length} public {posts.length === 1 ? 'post' : 'posts'} </div> {/if}packages/server/test/api/procedures.e2e.test.ts (1)
1-86: Good E2E coverage for REST procedure routing.The test suite validates:
- REST API procedure invocation via
/$procsroutes (lines 57-66)- Input validation returning proper 422 errors with validation-error codes (lines 68-85)
- Proper integration between RestApiHandler and the procedure system
The tests follow E2E best practices with proper lifecycle management and realistic schema definitions.
💡 Optional: Consider adding mutation procedure test
The schema defines a
createTwoAndFailmutation procedure that isn't tested. Consider adding a test to validate:
- POST-only enforcement for mutations
- Error handling when procedures throw exceptions
- Transaction rollback behavior (if applicable)
This would provide more complete E2E coverage of the procedure system.
samples/next.js/app/feeds/page.tsx (2)
26-30: Consider more defensive error handling for production.While adequate for a sample, displaying raw error messages could potentially leak internal implementation details in production scenarios. Consider abstracting error messages or using error codes for production deployments.
🛡️ Optional: More defensive error handling
{error && ( <div className="w-full p-4 rounded-md bg-red-50 border border-red-200 text-red-800 dark:bg-red-900/20 dark:border-red-800 dark:text-red-400"> - Error loading posts: {error instanceof Error ? error.message : 'Unknown error'} + Failed to load public posts. Please try again later. </div> )}
44-46: Consider handling invalid dates gracefully.While
new Date()handles invalid input by displaying "Invalid Date", consider validating or formatting the date more defensively to ensure a consistent user experience.📅 Optional: Defensive date formatting
<p className="text-sm text-gray-500 dark:text-gray-400 mt-2"> - Published on {new Date(post.createdAt).toLocaleDateString()} + Published on {post.createdAt ? new Date(post.createdAt).toLocaleDateString() : 'Unknown date'} </p>samples/sveltekit/src/routes/signup/+page.svelte (1)
17-28: Consider validating email format before submission.While HTML5
type="email"provides basic validation, client-side regex validation could provide better UX by catching invalid emails before the network request.💡 Optional: Add client-side email validation
function handleSubmit(e: SubmitEvent) { e.preventDefault(); + + // Basic email validation + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(email)) { + errorMessage = 'Please enter a valid email address'; + return; + } + successMessage = ''; errorMessage = '';samples/nuxt/app/pages/signup.vue (1)
20-26: Consider typing the success and error callback parameters.The
useranderrorparameters are typed asany, which loses type safety. The procedure's return type (User) should be inferable from the schema.🔧 Suggested improvement
- onSuccess: (user: any) => { + onSuccess: (user) => { successMessage.value = `Successfully created user: ${user.email}`; email.value = ''; }, - onError: (error: any) => { + onError: (error: Error) => { errorMessage.value = error instanceof Error ? error.message : 'Failed to sign up'; },If the type inference doesn't work automatically, consider importing the User type from the generated schema.
packages/cli/src/actions/generate.ts (2)
42-50: Consider extracting the CST node traversal logic for clarity and safety.The nested property access chain with multiple optional chaining operators and non-null assertions is complex. While the filter ensures the properties exist before the map, this could be made more explicit.
🔧 Suggested refactor for readability
const getRootModelWatchPaths = (model: Model) => new Set<string>( - ( - model.declarations.filter( - (v) => - v.$cstNode?.parent?.element.$type === 'Model' && - !!v.$cstNode.parent.element.$document?.uri?.fsPath, - ) as AbstractDeclaration[] - ).map((v) => v.$cstNode!.parent!.element.$document!.uri!.fsPath), + model.declarations + .map((v) => v.$cstNode?.parent?.element) + .filter((element): element is Model & { $document: { uri: { fsPath: string } } } => + element?.$type === 'Model' && !!element.$document?.uri?.fsPath + ) + .map((element) => element.$document.uri.fsPath), );
97-100: Error handling could provide more actionable feedback.When regeneration fails, users only see the raw error. Consider adding context about what action to take.
🔧 Suggested improvement
} catch (e) { - console.error(e); + console.error(colors.red('Generation failed during watch:'), e); + console.log(colors.yellow('Fix the error and save again to retry.')); }packages/server/src/api/rpc/index.ts (1)
39-45: Runtime schema validation lacks structural enforcement.
z.object()without arguments accepts any object. This means theschemaproperty is only validated to be an object-type value, not that it conforms to theSchemaDefstructure (which requiresprovider,models,pluginsfields). While TypeScript's type system enforces this at compile time viaRPCApiHandlerOptions<Schema extends SchemaDef>, the runtime check provides no protection against invalid schema objects passed at runtime. Consider either adding schema validation (e.g.,z.object({ provider: z.any(), models: z.any(), plugins: z.any() })) or documenting that TypeScript's structural validation is sufficient.packages/server/src/api/common/procedures.ts (1)
4-12: mapProcedureArgs is robust; consider normalizing direct-object payloadsThe payload validation is thorough, but for procedures where all params are optional, the “direct object” form (
{ foo: 1 }) bypasses the unknown-key checks you apply to theargsenvelope. If you want consistent behavior, consider normalizingpayloadinto an{ args: ... }shape and always validatingargs(including for the all-optional case), while still allowingpayloadto beundefinedfor “no args”.Also applies to: 61-133
packages/clients/tanstack-query/src/react.ts (1)
35-54: Procedure hook typing and$procswiring are sound; consider aligning infinite‑query storyThe new
ProcedureHookFn/ProcedureHookGrouptyping gives the right optional vs required input behavior and$procsis wired throughCUSTOM_PROC_ROUTE_NAMEconsistently with the server. One minor nit: the type-level comments say infinite procedure queries are disabled, butuseClientQueriesstill exposesuseInfiniteQuery/useSuspenseInfiniteQueryat runtime (viaany). For clarity, you may want to either:
- Re-enable the infinite variants in
ProcedureHookGroup, or- Remove the infinite wiring from
buildProcedureHooksuntil you intend to support them.Also applies to: 70-79, 150-153, 154-205, 336-395
packages/orm/src/client/contract.ts (1)
64-74: AlignProcedureOperationsconditional with other procedure helper typesHere you use
Schema['procedures'] extends Record<string, ProcedureDef>to decide whether$procsexists, whereas helpers likeGetProcedureNamesuseSchema extends { procedures: Record<string, ProcedureDef> }. IfSchema['procedures']is typed asRecord<string, ProcedureDef> | undefined(optional property), this conditional will resolve to{}and$procswill disappear from the client type even when procedures are present.To avoid that pitfall, consider rewriting as:
export type ProcedureOperations<Schema extends SchemaDef> = Schema extends { procedures: Record<string, ProcedureDef> } ? { $procs: { [Key in GetProcedureNames<Schema>]: ProcedureFunc<Schema, Key> } } : {};Also applies to: 196-199, 208-218
packages/clients/tanstack-query/src/vue.ts (1)
291-297: Consider extracting the duplicatemergehelper.The
mergehelper is defined identically in bothuseClientQueries(lines 291-297) anduseModelQueries(lines 366-370). Consider extracting it to a module-level function for maintainability.♻️ Suggested refactor
+function merge(rootOpt: MaybeRefOrGetter<unknown> | undefined, opt: MaybeRefOrGetter<unknown> | undefined): any { + return computed(() => { + const rootVal = toValue(rootOpt) ?? {}; + const optVal = toValue(opt) ?? {}; + return { ...(rootVal as object), ...(optVal as object) }; + }); +} + export function useClientQueries<Schema extends SchemaDef, Options extends QueryOptions<Schema> = QueryOptions<Schema>>( schema: Schema, options?: MaybeRefOrGetter<QueryContext>, ): ClientHooks<Schema, Options> { - const merge = (rootOpt: MaybeRefOrGetter<unknown> | undefined, opt: MaybeRefOrGetter<unknown> | undefined): any => { - return computed(() => { - const rootVal = toValue(rootOpt) ?? {}; - const optVal = toValue(opt) ?? {}; - return { ...(rootVal as object), ...(optVal as object) }; - }); - }; - const result = Object.keys(schema.models).reduce(And similarly remove the duplicate from
useModelQueries.Also applies to: 366-370
|
Claude finished @ymc9's task —— View job Code Review for v3.2.0 Release (dev → main)I've reviewed this substantial PR merging dev to main for the v3.2.0 release. This introduces significant new functionality including custom procedures, exists operation, and CLI improvements. Review Tasks:
🔍 Major Features Introduced1. Custom Procedures (PR #551, #2172614)
2. Exists Operation (PR #550, #446a483)
3. CLI Watch Mode (PR #573, #554)
✅ Code Quality & Best PracticesStrengths:
Best Practices Followed:
|
* fix(server): allow Infinity as pageSize option for REST api * fix schema
|
Claude finished @ymc9's task —— View job Code Review for v3.2.0 Release (dev → main)I've reviewed this substantial PR merging dev to main for the v3.2.0 release. This introduces significant new functionality including custom procedures, exists operation, and CLI improvements. Review Tasks:
🔍 Major Features Introduced1. Custom Procedures (PR #551, #2172614)
2. Exists Operation (PR #550, #446a483)
3. CLI Watch Mode (PR #573, #554)
✅ Code Quality & Best PracticesStrengths:
Best Practices Followed:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/server/test/api/options-validation.test.ts:
- Around line 75-84: The comment in the test is incorrect: replace or remove the
misleading lines that claim "Zod z.string() validation allows empty strings" and
"The endpoint validation doesn't enforce non-empty string" with a brief note
that the RestApiHandler's validation uses string().min(1) (in the REST schema)
and therefore correctly rejects empty endpoint values; alternatively just delete
the comment so the test (which expects an error for endpoint: '') stands on its
own.
🧹 Nitpick comments (2)
tests/e2e/github-repos/cal.com/input.ts (1)
11-2047: Bulk*ExistsArgsexports look consistent with existing generated*Find*Argspatterns.
No red flags in the aliasing approach ($ExistsArgs<$Schema, "Model">) and it matches the pattern used in other fixtures (e.g.,samples/sveltekit/src/zenstack/input.ts). As this is an E2E “real-world schema” fixture, please ensure there’s at least one E2E runtime assertion that callsexists()against the cal.com schema so this isn’t only a compile-time check. As per coding guidelines, E2E tests should validate real-world schema compatibility with established projects.packages/server/test/api/options-validation.test.ts (1)
618-626: Consider clarifying this test's intent.This test validates that an empty schema object fails, but it's testing internal implementation details (type map building failure) rather than the validation API contract. The generic
toThrow()assertion without a specific error message also differs from other tests that check for 'Invalid options'.Consider either:
- Moving this to a separate test suite focused on internal behavior, or
- Making the test more explicit about what implementation detail it's verifying, or
- Removing it if the Zod validation should catch empty schemas at the validation layer
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/cli/package.jsonpackages/server/src/api/rest/index.tspackages/server/test/api/options-validation.test.tssamples/sveltekit/src/zenstack/input.tssamples/sveltekit/src/zenstack/schema-lite.tssamples/sveltekit/src/zenstack/schema.tstests/e2e/apps/rally/zenstack/input.tstests/e2e/github-repos/cal.com/input.tstests/e2e/github-repos/formbricks/input.tstests/e2e/github-repos/trigger.dev/input.tstests/e2e/orm/schemas/auth-type/input.tstests/e2e/orm/schemas/basic/input.tstests/e2e/orm/schemas/default-auth/input.ts
✅ Files skipped from review due to trivial changes (1)
- packages/server/src/api/rest/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- samples/sveltekit/src/zenstack/input.ts
- samples/sveltekit/src/zenstack/schema.ts
🧰 Additional context used
📓 Path-based instructions (1)
tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests should validate real-world schema compatibility with established projects
Files:
tests/e2e/orm/schemas/default-auth/input.tstests/e2e/orm/schemas/basic/input.tstests/e2e/orm/schemas/auth-type/input.tstests/e2e/apps/rally/zenstack/input.tstests/e2e/github-repos/formbricks/input.tstests/e2e/github-repos/trigger.dev/input.tstests/e2e/github-repos/cal.com/input.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects
Applied to files:
tests/e2e/orm/schemas/default-auth/input.tspackages/server/test/api/options-validation.test.tstests/e2e/orm/schemas/basic/input.tstests/e2e/orm/schemas/auth-type/input.tstests/e2e/apps/rally/zenstack/input.tstests/e2e/github-repos/formbricks/input.tstests/e2e/github-repos/trigger.dev/input.tstests/e2e/github-repos/cal.com/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests
Applied to files:
tests/e2e/orm/schemas/default-auth/input.tspackages/server/test/api/options-validation.test.tstests/e2e/apps/rally/zenstack/input.tstests/e2e/github-repos/formbricks/input.tstests/e2e/github-repos/cal.com/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Implement plugin hooks at ORM, Kysely, and entity mutation levels for query interception and customization
Applied to files:
tests/e2e/orm/schemas/default-auth/input.tstests/e2e/orm/schemas/basic/input.tstests/e2e/orm/schemas/auth-type/input.tstests/e2e/apps/rally/zenstack/input.tstests/e2e/github-repos/formbricks/input.tstests/e2e/github-repos/trigger.dev/input.tstests/e2e/github-repos/cal.com/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.{ts,tsx} : Use Kysely as the query builder interface for low-level database queries, avoiding raw SQL when possible
Applied to files:
tests/e2e/orm/schemas/default-auth/input.tstests/e2e/orm/schemas/basic/input.tstests/e2e/orm/schemas/auth-type/input.tstests/e2e/apps/rally/zenstack/input.tstests/e2e/github-repos/formbricks/input.tstests/e2e/github-repos/trigger.dev/input.tstests/e2e/github-repos/cal.com/input.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests
Applied to files:
packages/server/test/api/options-validation.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/cli/**/*.test.{ts,tsx} : CLI package tests should focus on action-specific tests for each command
Applied to files:
packages/server/test/api/options-validation.test.ts
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Use `pnpm` with workspaces for package management, pinned to version `pnpm10.12.1`
Applied to files:
packages/cli/package.json
📚 Learning: 2025-11-26T01:55:04.540Z
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to **/*.zmodel : ZModel schema files should define database structure and policies that compile to TypeScript via `zenstack generate`
Applied to files:
tests/e2e/apps/rally/zenstack/input.tssamples/sveltekit/src/zenstack/schema-lite.ts
📚 Learning: 2025-12-30T15:07:10.092Z
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:10.092Z
Learning: In the zenstackhq/zenstack-v3 codebase, ts-expect-error should be heavily avoided in production code (packages/*). It is acceptable in test files but production code should use proper type annotations, type assertions, or refactoring to resolve type errors instead.
Applied to files:
tests/e2e/apps/rally/zenstack/input.ts
🧬 Code graph analysis (4)
packages/server/test/api/options-validation.test.ts (4)
packages/orm/src/client/contract.ts (1)
ClientContract(64-198)packages/testtools/src/client.ts (1)
createTestClient(101-248)packages/server/src/api/rest/index.ts (1)
RestApiHandler(129-2194)packages/server/src/api/rpc/index.ts (1)
RPCApiHandler(34-346)
tests/e2e/orm/schemas/auth-type/input.ts (3)
tests/e2e/orm/schemas/json/input.ts (4)
FooFindManyArgs(11-11)FooFindUniqueArgs(12-12)FooFindFirstArgs(13-13)FooExistsArgs(14-14)tests/regression/test/issue-204/input.ts (4)
FooFindManyArgs(11-11)FooFindUniqueArgs(12-12)FooFindFirstArgs(13-13)FooExistsArgs(14-14)packages/clients/tanstack-query/test/schemas/basic/input.ts (4)
FooFindManyArgs(74-74)FooFindUniqueArgs(75-75)FooFindFirstArgs(76-76)FooExistsArgs(77-77)
tests/e2e/github-repos/trigger.dev/input.ts (8)
samples/sveltekit/src/zenstack/input.ts (4)
UserFindManyArgs(11-11)UserFindUniqueArgs(12-12)UserFindFirstArgs(13-13)UserExistsArgs(14-14)tests/e2e/github-repos/cal.com/input.ts (4)
UserFindManyArgs(158-158)UserFindUniqueArgs(159-159)UserFindFirstArgs(160-160)UserExistsArgs(161-161)tests/e2e/github-repos/formbricks/input.ts (7)
UserFindManyArgs(515-515)UserFindUniqueArgs(516-516)UserFindFirstArgs(517-517)UserExistsArgs(518-518)OrganizationExistsArgs(392-392)ProjectExistsArgs(371-371)DataMigrationExistsArgs(329-329)tests/e2e/orm/schemas/basic/input.ts (4)
UserFindManyArgs(11-11)UserFindUniqueArgs(12-12)UserFindFirstArgs(13-13)UserExistsArgs(14-14)tests/e2e/orm/schemas/default-auth/input.ts (4)
UserFindManyArgs(11-11)UserFindUniqueArgs(12-12)UserFindFirstArgs(13-13)UserExistsArgs(14-14)samples/orm/zenstack/input.ts (4)
UserFindManyArgs(11-11)UserFindUniqueArgs(12-12)UserFindFirstArgs(13-13)UserExistsArgs(14-14)tests/runtimes/edge-runtime/schemas/input.ts (4)
UserFindManyArgs(11-11)UserFindUniqueArgs(12-12)UserFindFirstArgs(13-13)UserExistsArgs(14-14)tests/runtimes/bun/schemas/input.ts (4)
UserFindManyArgs(11-11)UserFindUniqueArgs(12-12)UserFindFirstArgs(13-13)UserExistsArgs(14-14)
tests/e2e/github-repos/cal.com/input.ts (10)
samples/sveltekit/src/zenstack/input.ts (1)
UserExistsArgs(14-14)tests/e2e/github-repos/formbricks/input.ts (2)
UserExistsArgs(518-518)TeamExistsArgs(686-686)tests/e2e/github-repos/trigger.dev/input.ts (1)
UserExistsArgs(14-14)tests/e2e/orm/schemas/basic/input.ts (2)
UserExistsArgs(14-14)ProfileExistsArgs(77-77)tests/e2e/orm/schemas/default-auth/input.ts (2)
UserExistsArgs(14-14)ProfileExistsArgs(35-35)samples/orm/zenstack/input.ts (2)
UserExistsArgs(14-14)ProfileExistsArgs(35-35)tests/e2e/orm/schemas/petstore/input.ts (1)
UserExistsArgs(14-14)tests/e2e/orm/schemas/todo/input.ts (1)
UserExistsArgs(56-56)tests/regression/test/issue-422/input.ts (2)
UserExistsArgs(35-35)ProfileExistsArgs(56-56)tests/e2e/orm/schemas/typing/input.ts (1)
ProfileExistsArgs(56-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (22.x, sqlite)
- GitHub Check: claude-review
- GitHub Check: build-test (22.x, postgresql)
🔇 Additional comments (18)
samples/sveltekit/src/zenstack/schema-lite.ts (1)
104-117: LGTM! Generated procedures object looks correct.The auto-generated
proceduresobject correctly defines two procedures that align with the PR's goal of introducing first-class procedures:
signUp: A mutation procedure that accepts an email parameter and returns a UserlistPublicPosts: A query procedure that returns an array of PostsThe structure is consistent with the rest of the schema definition, properly typed with
as const, and follows the expected patterns for the new procedural API surface.Note: Since this is an auto-generated file (as indicated in the header), any changes to procedures should be made in the source
.zmodelschema file, not directly in this TypeScript output.tests/e2e/github-repos/formbricks/input.ts (1)
9-9: LGTM! Consistent generated type additions for exists() API.The generated file correctly adds
ExistsArgstype support for all 35 entities in the Formbricks schema, validating real-world compatibility with the new exists() feature introduced in this release.Based on coding guidelines: E2E tests should validate real-world schema compatibility with established projects.
Also applies to: 14-14, 35-35, 56-56, 77-77, 98-98, 119-119, 140-140, 161-161, 182-182, 203-203, 224-224, 245-245, 266-266, 287-287, 308-308, 329-329, 350-350, 371-371, 392-392, 413-413, 434-434, 455-455, 476-476, 497-497, 518-518, 539-539, 560-560, 581-581, 602-602, 623-623, 644-644, 665-665, 686-686, 707-707, 728-728
tests/e2e/orm/schemas/auth-type/input.ts (1)
9-14: LGTM!The auto-generated changes correctly add
ExistsArgsimport andFooExistsArgstype export, following the established pattern consistent with other schema files.tests/e2e/orm/schemas/basic/input.ts (1)
9-14: LGTM!The auto-generated changes correctly add
ExistsArgstype exports for all entities (User, Post, Comment, Profile), maintaining consistent placement afterFindFirstArgsand beforeCreateArgs. Based on learnings, this ORM schema test supports comprehensive client API testing.Also applies to: 35-35, 56-56, 77-77
tests/e2e/orm/schemas/default-auth/input.ts (1)
9-14: LGTM!The auto-generated changes correctly add
ExistsArgstype exports for User, Profile, and Address entities with consistent placement and import pattern.Also applies to: 35-35, 56-56
tests/e2e/apps/rally/zenstack/input.ts (1)
9-14: LGTM!The auto-generated changes correctly add
ExistsArgstype exports for all Rally app entities. As per coding guidelines, this E2E test validates real-world schema compatibility with an established project pattern.tests/e2e/github-repos/trigger.dev/input.ts (1)
9-14: LGTM!The auto-generated changes correctly add
ExistsArgstype exports for all trigger.dev entities. As per coding guidelines, this E2E test validates real-world schema compatibility with an established open-source project.tests/e2e/github-repos/cal.com/input.ts (1)
8-10: No action required —ExistsArgsis properly exported and runtime coverage exists.
ExistsArgsis correctly exported from@zenstackhq/orm(packages/orm/src/client/crud-types.ts:1087) with the expected generic signatureExistsArgs<Schema extends SchemaDef, Model extends GetModels<Schema>>. Theexists()operation has comprehensive E2E runtime test coverage intests/e2e/orm/client-api/exists.test.ts, validating the operation across multiple models and argument patterns. The cal.com E2E fixture import is valid and correctly validates real-world schema compatibility.packages/cli/package.json (4)
6-6: LGTM: Version bump aligns with release.The version bump to 3.2.0 is appropriate for this release and aligns with the PR title and new features being introduced.
24-27: LGTM: Files field properly configured.The "files" field correctly specifies the distribution artifacts (dist and bin) that should be published to npm. This is a good practice to control the package's published contents.
42-42: Chokidar v5.0.0 is valid and secure.Version 5.0.0 exists in the npm registry, is not deprecated, and has no known security vulnerabilities. The MIT-licensed dependency is appropriate for the new CLI --watch feature.
67-69: [Your rewritten review comment text here]
[Exactly ONE classification tag]packages/server/test/api/options-validation.test.ts (6)
1-21: LGTM! Clean test setup.The imports are appropriate, and the test setup with a minimal schema is well-suited for validation testing. The beforeEach hook ensures a fresh client for each test.
24-74: Excellent comprehensive validation coverage.The test suite thoroughly covers all validation scenarios including:
- Valid configurations with required and optional fields
- Missing required fields (schema, endpoint)
- Type validation for all option fields
- Boundary conditions (zero, negative, NaN, decimal for pageSize)
- String validation (empty strings, non-strings)
- Object/record validation (modelNameMapping, externalIdMapping)
- Log configuration variants (array, function)
The test organization and naming conventions are clear and maintainable.
Also applies to: 86-312
314-435: LGTM! Thorough RPCApiHandler validation tests.The test coverage for RPCApiHandler mirrors the quality of RestApiHandler tests with appropriate adjustments for the simpler option set. The tests effectively validate:
- Schema requirement and type checking (missing, null, undefined, invalid types)
- Log configuration validation (array, function, invalid types, edge cases)
- strictObject behavior rejecting extra properties
437-477: LGTM! Explicit strictObject validation coverage.The dedicated test suite for extra property rejection provides clear documentation of the strictObject validation behavior. Testing both single and multiple extra properties ensures the validation is consistent.
479-552: LGTM! Good edge case and API contract coverage.These tests verify important behavioral guarantees:
- Graceful handling of explicit
undefinedvalues for optional fields- Public property exposure (schema, log) matching the API contract
- Empty mappings being valid inputs
This ensures the handlers behave correctly in edge cases that might occur in real usage.
554-596: LGTM! Valuable real-world scenario tests.These tests demonstrate realistic production configurations with multiple options combined, including custom logging logic and pagination control. They serve as both validation tests and usage examples.
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.