Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Oct 1, 2025

Summary by CodeRabbit

  • New Features

    • Introduced post-update policy operation with a new before() helper for referencing pre-update values.
    • Enforced validation: before() only allowed in post-update rules and cannot access relation fields.
    • Enhanced runtime policy enforcement and mutation flow to support post-update checks.
    • Improved query execution with managed transactions and clearer error handling.
  • Documentation

    • Updated policy docs and completions to reflect before() and post-update; added checklist items.
  • Tests

    • Added and re-enabled extensive policy, CRUD, and regression tests.
  • Chores

    • Bumped versions across packages.

ymc9 and others added 6 commits September 24, 2025 09:21
…on previous errors (#273)

* fix(language): some references are not resolved; suppress validation on previous errors

* update

* addressing comments
* fix(policy): always wrap mutation in a transaction

* fix error handling
…278)

* fix: prisma schema generation issue with `default()` using `auth()`

fixes #274

* update

* addressing pr comment
Copilot AI review requested due to automatic review settings October 1, 2025 00:22
@coderabbitai
Copy link

coderabbitai bot commented Oct 1, 2025

Warning

Rate limit exceeded

@ymc9 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 34 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 546ba8e and 6eaf1cd.

📒 Files selected for processing (1)
  • packages/runtime/src/plugins/policy/policy-handler.ts (14 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Introduces a new policy operation "post-update" and replaces the policy helper function future() with before(). Updates language tooling (stdlib, validation, linking, document building), runtime policy evaluation and executor/transaction flow, error hierarchy, SDK schema generation, and extensive tests/schemas. Also bumps package versions to 3.0.0-beta.8.

Changes

Cohort / File(s) Summary of Changes
Documentation
TODO.md
Added checklist items under Access Policy.
Version bumps
package.json, packages/*/package.json, samples/blog/package.json, tests/e2e/package.json, tests/regression/package.json
Bump versions to 3.0.0-beta.8 (and VSCode ext 3.0.9); metadata only.
Language stdlib and semantics
packages/language/res/stdlib.zmodel
Added "post-update" to policy ops; renamed future()before(); updated texts.
Language build/validation/linking
packages/language/src/index.ts, .../zmodel-document-builder.ts, .../validators/attribute-application-validator.ts, .../validators/expression-validator.ts, .../zmodel-linker.ts, .../zmodel-scope.ts, .../utils.ts
Validation config now uses granular stop flags; add validation for before() usage, including relation access restriction; replace future* utilities with isBeforeInvocation; refactor linker to new reference resolution flow.
Runtime contract and options
packages/runtime/src/client/contract.ts, .../options.ts
Added CRUD_EXT (CRUD ∪ 'post-update'); updated context type to use CRUD_EXT.
Runtime errors
packages/runtime/src/client/errors.ts, packages/runtime/src/plugins/policy/errors.ts
Introduced ZenStackError; core errors extend it; RejectedByPolicyError now extends ZenStackError and accepts optional message.
Executor and CRUD base
packages/runtime/src/client/executor/zenstack-query-executor.ts, .../crud/operations/base.ts
Executor refactored for explicit connection/transaction handling, hook flow adjustments, and error wrapping; CRUD base returns .rows via executeQuery.
Policy runtime core
packages/runtime/src/plugins/policy/policy-handler.ts, .../plugin.ts, .../expression-transformer.ts, .../types.ts, .../utils.ts
Added post-update policy flow, before-update entity loading, and evaluation; before() handling in expression transformer; replace CRUD with CRUD_EXT; remove tx param from plugin handler path; add isBeforeInvocation.
Runtime expression utilities
packages/runtime/src/utils/expression-utils.ts
Added ExpressionVisitor traversal utility class.
SDK generator
packages/sdk/src/prisma/prisma-schema-generator.ts
Switched default invocation filtering to auth-based; renamed helper to isDefaultWithAuthInvocation; import isAuthInvocation.
Test tooling
packages/testtools/src/project.ts
createTestProject accepts optional zmodelContent to write schema file.
Language tests
packages/language/test/attribute-application.test.ts
New test ensuring before() only allowed in "post-update" policies.
Runtime policy tests (new/updated)
packages/runtime/test/policy/crud/post-update.test.ts, .../migrated/*, .../todo-sample.test.ts, .../toplevel-operations.test.ts, .../petstore-sample.test.ts
Added comprehensive post-update tests; unskipped and adjusted tests replacing future() with before() and new post-update rules; updated expectations.
Test schemas
packages/runtime/test/schemas/petstore/schema.ts, .../schema.zmodel, packages/runtime/test/schemas/todo/schema.ts, .../todo.zmodel
Added/modified @@allow/@@deny('post-update', ...) using before(); simplified update rules.
Regression tests
tests/regression/test/issue-204/regression.test.ts, .../issue-274/regression.test.ts
Simplified wrapper structure; added new migration regression test using createTestProject and CLI.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

A whisk of code, a hop of fate,
We swapped the future for “before” of late.
Post-update rules now guide our trail,
With carrots of tests that never fail.
I nose the schema, tap-tap—approve!
Version bumped—let’s bunny-groove. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title “merge dev to main (v3.0.0-beta.8)” merely describes the branch operation and version bump without conveying the substantive changes introduced in this pull request, making it overly generic and not immediately informative to reviewers browsing the commit history. Please update the title to clearly summarize the primary changes in this merge, for example by highlighting the version bump to v3.0.0-beta.8 along with the key feature additions such as post-update policy support and the renaming of future() to before().
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@claude
Copy link

claude bot commented Oct 1, 2025

Claude finished @ymc9's task —— View job


PR Review for v3.0.0-beta.8 Release

Reviewing this PR that merges dev → main for the v3.0.0-beta.8 release with 831 additions and 373 deletions across 59 files.

Todo List

  • Set up development environment and dependencies
  • Review major architectural changes in runtime policy handling
  • Analyze new post-update policy implementation
  • Check language validation improvements
  • Review test coverage and new test files
  • Evaluate security implications of policy changes
  • Check for potential performance issues
  • Provide comprehensive feedback summary

🔍 Comprehensive Review Summary

Strengths & Well-Implemented Features

1. Post-Update Policies - Major New Feature

  • Robust implementation: The new post-update policy operation is well-designed with proper validation flow
  • Complex SQL generation: Smart handling of before/after state comparison using VALUES clauses and JOINs
  • Comprehensive validation: Prevents relation field access from before() calls (security-conscious)
  • Transaction safety: All mutations properly wrapped in RepeatableRead transactions

2. Code Quality & Architecture

  • Strong type safety: Extensive use of TypeScript discriminated unions and type guards
  • Error handling: Proper error propagation with meaningful error messages
  • Test coverage: 191 lines of comprehensive test cases for the new feature
  • Backward compatibility: Changes are additive and don't break existing functionality

3. Language & Validation Improvements

  • Parser enhancements: Updated ZModel grammar to support post-update in policy rules
  • Runtime validation: Proper validation that before() calls are only allowed in post-update contexts
  • Developer experience: Good error messages and validation feedback

⚠️ Areas of Concern & Recommendations

1. Performance Considerations

  • Before-state loading: The system loads entire "before" entity state for post-update policies, which could be expensive for large entities

    // packages/runtime/src/plugins/policy/policy-handler.ts:226-243
    private async loadBeforeUpdateEntities(...) {
      // Loads all fields referenced in before() calls
    }

    Recommendation: Consider optimization for cases where only specific fields are needed

  • Complex SQL generation: Post-update validation generates nested queries with VALUES clauses

    // Lines 138-160: Complex table construction for before-state comparison
    const beforeUpdateTable: SelectQueryNode | undefined = beforeUpdateInfo ? { /*complex query*/ } : undefined;

    Recommendation: Monitor query performance on large datasets

2. Security Assessment

  • Access control: Proper prevention of relation field access from before() calls
  • SQL injection: Uses parameterized queries and proper SQL construction through Kysely
  • Transaction isolation: RepeatableRead isolation level prevents concurrent modification issues
  • No security vulnerabilities identified

3. Minor Issues

Expression Transformer Safety

The expression transformer properly handles field access validation:

// packages/language/src/validators/attribute-application-validator.ts:185-189
accept('error', `relation fields cannot be accessed from \`before()\``, { node: beforeCall });

Version Bumps

All package versions consistently bumped to 3.0.0-beta.8 - good release hygiene.

🚀 Technical Excellence

1. Policy Engine Robustness

  • Smart field collection: Only loads fields actually referenced in before() calls
  • Efficient ID handling: Ensures ID fields are always included for proper joining
  • Proper SQL casting: Handles type casting for different field types in VALUES clauses

2. Error Handling

  • Meaningful errors: Clear error messages for policy violations
  • Proper context: Errors include model context and operation type
  • Graceful failures: Transactions properly rolled back on policy failures

📊 Test Coverage Quality

The new test file (post-update.test.ts) covers:

  • ✅ Basic post-update validation
  • ✅ Simple allow/deny rules
  • ✅ Relation field access
  • before() function usage
  • ✅ Query builder API compatibility
  • ✅ Proper error validation

🏆 Overall Assessment

This is a high-quality release that introduces a significant new feature (post-update policies) with:

  • Solid architecture and implementation
  • Comprehensive testing
  • Good security practices
  • Proper error handling
  • Backward compatibility

The implementation demonstrates deep understanding of database transactions, SQL generation, and policy evaluation. The code follows established patterns and maintains high code quality standards.

🎯 Recommendation: APPROVE

This PR successfully implements post-update policies as a major feature enhancement with proper testing, security considerations, and maintainable code. The minor performance considerations noted above are acceptable for a beta release and can be optimized in future iterations based on real-world usage patterns.

The v3.0.0-beta.8 release appears ready for deployment.


Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR merges development branch to main for the v3.0.0-beta.8 release, implementing support for post-update policies using before() function and transitioning away from the previous future() approach.

  • Adds post-update policy functionality with before() function to access entity values before updates
  • Replaces previous future() implementation with new before() approach for accessing pre-update state
  • Updates version numbers across all packages to 3.0.0-beta.8

Reviewed Changes

Copilot reviewed 56 out of 59 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/runtime/src/plugins/policy/policy-handler.ts Implements core post-update policy logic with before-update entity loading and validation
packages/language/res/stdlib.zmodel Updates function name from future() to before() with corrected documentation
packages/runtime/test/policy/crud/post-update.test.ts Comprehensive test suite for new post-update policy functionality
tests/regression/test/issue-274/regression.test.ts New regression test for issue 274 using updated test infrastructure
packages/testtools/src/project.ts Enhanced test project creation to accept zmodel content parameter

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/language/res/stdlib.zmodel (1)

669-711: Document the new post-update operation in the attribute docs.

'post-update' is now part of the completion hints, but the parameter docs above @@allow/@@deny still say the list is just “create/read/update/delete/all”. Please update the text so the documentation matches the accepted operations and avoids confusing users.

packages/sdk/src/prisma/prisma-schema-generator.ts (1)

262-273: Add test coverage for @default(auth()) invocations
No existing test schemas include a @default(auth(...)) attribute. Add both simple and nested auth() cases in packages/runtime/test/schemas (and packages/language/test) to ensure isDefaultWithAuthInvocation reliably detects them.

🧹 Nitpick comments (2)
TODO.md (1)

104-105: Fix Markdown list indentation

These new sub-items use four leading spaces, but our markdownlint config (MD007) expects a two-space indent for nested bullets. Please drop the indent to two spaces so the checklist passes lint.

packages/runtime/test/policy/migrated/petstore-sample.test.ts (1)

5-5: Consider adding test cases for post-update constraint validation.

The test suite has been re-enabled to work with the new before()/post-update semantics. However, the current test only validates basic CRUD operations and doesn't verify the post-update constraints defined in the schema (e.g., preventing updates to name/category fields, or preventing orderId from being changed once set).

Consider adding test cases such as:

  • Attempting to update name or category on an existing pet (should fail post-update validation)
  • Attempting to change orderId after it's been set (should fail post-update validation)
  • Verifying that orderId CAN be set from null (should succeed)

Would you like me to generate these additional test cases?

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f639de and 546ba8e.

📒 Files selected for processing (56)
  • TODO.md (1 hunks)
  • package.json (1 hunks)
  • packages/cli/package.json (1 hunks)
  • packages/common-helpers/package.json (1 hunks)
  • packages/create-zenstack/package.json (1 hunks)
  • packages/dialects/sql.js/package.json (1 hunks)
  • packages/eslint-config/package.json (1 hunks)
  • packages/ide/vscode/package.json (1 hunks)
  • packages/language/package.json (1 hunks)
  • packages/language/res/stdlib.zmodel (3 hunks)
  • packages/language/src/index.ts (1 hunks)
  • packages/language/src/utils.ts (1 hunks)
  • packages/language/src/validators/attribute-application-validator.ts (3 hunks)
  • packages/language/src/validators/expression-validator.ts (2 hunks)
  • packages/language/src/zmodel-document-builder.ts (1 hunks)
  • packages/language/src/zmodel-linker.ts (8 hunks)
  • packages/language/src/zmodel-scope.ts (2 hunks)
  • packages/language/test/attribute-application.test.ts (1 hunks)
  • packages/runtime/package.json (1 hunks)
  • packages/runtime/src/client/contract.ts (1 hunks)
  • packages/runtime/src/client/crud/operations/base.ts (1 hunks)
  • packages/runtime/src/client/errors.ts (3 hunks)
  • packages/runtime/src/client/executor/zenstack-query-executor.ts (4 hunks)
  • packages/runtime/src/client/options.ts (2 hunks)
  • packages/runtime/src/plugins/policy/errors.ts (2 hunks)
  • packages/runtime/src/plugins/policy/expression-transformer.ts (4 hunks)
  • packages/runtime/src/plugins/policy/plugin.ts (1 hunks)
  • packages/runtime/src/plugins/policy/policy-handler.ts (14 hunks)
  • packages/runtime/src/plugins/policy/types.ts (2 hunks)
  • packages/runtime/src/plugins/policy/utils.ts (2 hunks)
  • packages/runtime/src/utils/expression-utils.ts (1 hunks)
  • packages/runtime/test/policy/crud/post-update.test.ts (1 hunks)
  • packages/runtime/test/policy/migrated/multi-field-unique.test.ts (1 hunks)
  • packages/runtime/test/policy/migrated/multi-id-fields.test.ts (6 hunks)
  • packages/runtime/test/policy/migrated/nested-to-one.test.ts (2 hunks)
  • packages/runtime/test/policy/migrated/petstore-sample.test.ts (1 hunks)
  • packages/runtime/test/policy/migrated/todo-sample.test.ts (1 hunks)
  • packages/runtime/test/policy/migrated/toplevel-operations.test.ts (3 hunks)
  • packages/runtime/test/policy/todo-sample.test.ts (1 hunks)
  • packages/runtime/test/schemas/petstore/schema.ts (1 hunks)
  • packages/runtime/test/schemas/petstore/schema.zmodel (1 hunks)
  • packages/runtime/test/schemas/todo/schema.ts (2 hunks)
  • packages/runtime/test/schemas/todo/todo.zmodel (2 hunks)
  • packages/sdk/package.json (1 hunks)
  • packages/sdk/src/prisma/prisma-schema-generator.ts (4 hunks)
  • packages/tanstack-query/package.json (1 hunks)
  • packages/testtools/package.json (1 hunks)
  • packages/testtools/src/project.ts (2 hunks)
  • packages/typescript-config/package.json (1 hunks)
  • packages/vitest-config/package.json (1 hunks)
  • packages/zod/package.json (1 hunks)
  • samples/blog/package.json (1 hunks)
  • tests/e2e/package.json (1 hunks)
  • tests/regression/package.json (1 hunks)
  • tests/regression/test/issue-204/regression.test.ts (1 hunks)
  • tests/regression/test/issue-274/regression.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
{packages,samples,tests}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place packages only under packages/, samples/, or tests/

Files:

  • packages/runtime/package.json
  • packages/language/package.json
  • packages/runtime/src/plugins/policy/types.ts
  • packages/language/src/utils.ts
  • packages/language/src/zmodel-scope.ts
  • packages/runtime/test/policy/migrated/todo-sample.test.ts
  • packages/cli/package.json
  • packages/runtime/src/plugins/policy/errors.ts
  • packages/runtime/src/plugins/policy/plugin.ts
  • packages/runtime/src/client/crud/operations/base.ts
  • packages/runtime/src/client/options.ts
  • packages/testtools/src/project.ts
  • packages/language/src/index.ts
  • packages/typescript-config/package.json
  • tests/regression/package.json
  • packages/runtime/src/client/errors.ts
  • packages/runtime/src/plugins/policy/utils.ts
  • packages/runtime/test/policy/migrated/petstore-sample.test.ts
  • packages/sdk/src/prisma/prisma-schema-generator.ts
  • packages/sdk/package.json
  • packages/runtime/src/client/contract.ts
  • packages/runtime/test/schemas/todo/schema.ts
  • packages/common-helpers/package.json
  • tests/regression/test/issue-204/regression.test.ts
  • tests/regression/test/issue-274/regression.test.ts
  • packages/runtime/test/schemas/petstore/schema.zmodel
  • packages/ide/vscode/package.json
  • packages/create-zenstack/package.json
  • packages/runtime/test/policy/migrated/nested-to-one.test.ts
  • samples/blog/package.json
  • packages/language/src/validators/expression-validator.ts
  • packages/runtime/test/policy/crud/post-update.test.ts
  • packages/vitest-config/package.json
  • packages/language/src/validators/attribute-application-validator.ts
  • packages/runtime/src/plugins/policy/expression-transformer.ts
  • packages/runtime/test/policy/migrated/toplevel-operations.test.ts
  • packages/runtime/src/client/executor/zenstack-query-executor.ts
  • packages/zod/package.json
  • packages/language/test/attribute-application.test.ts
  • packages/language/res/stdlib.zmodel
  • packages/language/src/zmodel-linker.ts
  • packages/tanstack-query/package.json
  • packages/dialects/sql.js/package.json
  • tests/e2e/package.json
  • packages/runtime/test/policy/migrated/multi-field-unique.test.ts
  • packages/runtime/test/schemas/petstore/schema.ts
  • packages/eslint-config/package.json
  • packages/language/src/zmodel-document-builder.ts
  • packages/testtools/package.json
  • packages/runtime/test/policy/todo-sample.test.ts
  • packages/runtime/src/utils/expression-utils.ts
  • packages/runtime/test/schemas/todo/todo.zmodel
  • packages/runtime/test/policy/migrated/multi-id-fields.test.ts
  • packages/runtime/src/plugins/policy/policy-handler.ts
package.json

📄 CodeRabbit inference engine (CLAUDE.md)

Pin the repository package manager to [email protected] via the packageManager field

Files:

  • package.json
**/schema.ts

📄 CodeRabbit inference engine (CLAUDE.md)

The generated TypeScript schema should be named schema.ts

Files:

  • packages/runtime/test/schemas/todo/schema.ts
  • packages/runtime/test/schemas/petstore/schema.ts
**/schema.zmodel

📄 CodeRabbit inference engine (CLAUDE.md)

Name ZModel schema files schema.zmodel

Files:

  • packages/runtime/test/schemas/petstore/schema.zmodel
packages/ide/vscode/package.json

📄 CodeRabbit inference engine (CLAUDE.md)

The ide/vscode package is intentionally versioned differently from other packages

Files:

  • packages/ide/vscode/package.json
tests/e2e/**

📄 CodeRabbit inference engine (CLAUDE.md)

End-to-end tests must live under tests/e2e/

Files:

  • tests/e2e/package.json
🧠 Learnings (1)
📚 Learning: 2025-09-04T12:38:14.150Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-04T12:38:14.150Z
Learning: Applies to packages/ide/vscode/package.json : The `ide/vscode` package is intentionally versioned differently from other packages

Applied to files:

  • packages/ide/vscode/package.json
🧬 Code graph analysis (22)
packages/runtime/src/plugins/policy/types.ts (1)
packages/runtime/src/client/contract.ts (2)
  • CRUD_EXT (219-219)
  • CRUD_EXT (229-229)
packages/language/src/utils.ts (4)
packages/runtime/src/plugins/policy/utils.ts (1)
  • isBeforeInvocation (158-160)
packages/language/src/ast.ts (2)
  • AstNode (4-4)
  • AstNode (66-71)
packages/language/src/generated/ast.ts (1)
  • isInvocationExpr (542-544)
packages/sdk/src/model-utils.ts (1)
  • isFromStdlib (85-88)
packages/language/src/zmodel-scope.ts (2)
packages/language/src/utils.ts (1)
  • isBeforeInvocation (449-451)
packages/runtime/src/plugins/policy/utils.ts (1)
  • isBeforeInvocation (158-160)
packages/runtime/src/plugins/policy/errors.ts (1)
packages/runtime/src/client/errors.ts (1)
  • ZenStackError (4-4)
packages/runtime/src/plugins/policy/plugin.ts (2)
packages/runtime/src/client/plugin.ts (1)
  • OnKyselyQueryArgs (188-193)
packages/runtime/src/plugins/policy/policy-handler.ts (1)
  • PolicyHandler (62-990)
packages/runtime/src/client/crud/operations/base.ts (2)
packages/runtime/src/client/executor/zenstack-query-executor.ts (1)
  • kysely (60-62)
packages/runtime/src/plugins/policy/policy-handler.ts (1)
  • kysely (70-72)
packages/runtime/src/client/options.ts (1)
packages/runtime/src/client/contract.ts (2)
  • CRUD_EXT (219-219)
  • CRUD_EXT (229-229)
packages/runtime/src/plugins/policy/utils.ts (2)
packages/language/src/utils.ts (1)
  • isBeforeInvocation (449-451)
packages/runtime/src/schema/expression.ts (1)
  • ExpressionUtils (16-120)
packages/sdk/src/prisma/prisma-schema-generator.ts (3)
packages/language/src/generated/ast.ts (2)
  • DataFieldAttribute (348-353)
  • DataFieldAttribute (355-355)
packages/language/src/utils.ts (1)
  • isAuthInvocation (64-66)
packages/sdk/src/ts-schema-generator.ts (1)
  • isAuthInvocation (651-655)
packages/runtime/test/schemas/todo/schema.ts (1)
packages/runtime/src/schema/expression.ts (1)
  • ExpressionUtils (16-120)
tests/regression/test/issue-204/regression.test.ts (1)
tests/regression/test/issue-204/models.ts (3)
  • Configuration (11-11)
  • ShirtColor (12-12)
  • ShirtColor (13-13)
tests/regression/test/issue-274/regression.test.ts (2)
packages/testtools/src/project.ts (1)
  • createTestProject (5-71)
packages/cli/src/utils/exec-utils.ts (1)
  • execSync (6-15)
packages/language/src/validators/expression-validator.ts (2)
packages/language/src/generated/ast.ts (3)
  • MemberAccessExpr (546-551)
  • MemberAccessExpr (553-553)
  • isDataModel (391-393)
packages/language/src/utils.ts (1)
  • isBeforeInvocation (449-451)
packages/runtime/test/policy/crud/post-update.test.ts (1)
packages/runtime/test/policy/utils.ts (1)
  • createPolicyTestClient (14-25)
packages/language/src/validators/attribute-application-validator.ts (1)
packages/language/src/utils.ts (1)
  • isBeforeInvocation (449-451)
packages/runtime/src/plugins/policy/expression-transformer.ts (5)
packages/sdk/src/schema/schema.ts (2)
  • SchemaDef (10-18)
  • GetModels (109-109)
packages/runtime/src/client/contract.ts (2)
  • CRUD_EXT (219-219)
  • CRUD_EXT (229-229)
packages/sdk/src/schema/expression.ts (1)
  • MemberExpression (27-31)
packages/runtime/src/plugins/policy/utils.ts (1)
  • isBeforeInvocation (158-160)
packages/common-helpers/src/tiny-invariant.ts (1)
  • invariant (4-14)
packages/runtime/src/client/executor/zenstack-query-executor.ts (2)
packages/runtime/src/client/executor/zenstack-driver.ts (2)
  • compiledQuery (181-188)
  • connection (129-170)
packages/runtime/src/client/errors.ts (2)
  • ZenStackError (4-4)
  • QueryError (18-22)
packages/language/test/attribute-application.test.ts (1)
packages/language/test/utils.ts (1)
  • loadSchemaWithError (21-39)
packages/language/src/zmodel-linker.ts (1)
packages/language/src/utils.ts (1)
  • isBeforeInvocation (449-451)
packages/runtime/test/schemas/petstore/schema.ts (1)
packages/runtime/src/schema/expression.ts (1)
  • ExpressionUtils (16-120)
packages/runtime/src/utils/expression-utils.ts (1)
packages/sdk/src/schema/expression.ts (9)
  • LiteralExpression (12-15)
  • ArrayExpression (17-20)
  • FieldExpression (22-25)
  • MemberExpression (27-31)
  • BinaryExpression (39-44)
  • UnaryExpression (33-37)
  • CallExpression (46-50)
  • ThisExpression (52-54)
  • NullExpression (56-58)
packages/runtime/src/plugins/policy/policy-handler.ts (7)
packages/runtime/src/client/plugin.ts (1)
  • ProceedKyselyQueryFunction (195-195)
packages/runtime/src/client/query-utils.ts (3)
  • requireField (38-56)
  • requireIdFields (63-70)
  • requireModel (25-31)
packages/runtime/src/plugins/policy/utils.ts (6)
  • conjunction (49-67)
  • isBeforeInvocation (158-160)
  • trueNode (21-23)
  • falseNode (28-30)
  • disjunction (69-87)
  • buildIsFalse (127-139)
packages/runtime/src/utils/expression-utils.ts (1)
  • ExpressionVisitor (15-58)
packages/sdk/src/schema/expression.ts (1)
  • MemberExpression (27-31)
packages/common-helpers/src/tiny-invariant.ts (1)
  • invariant (4-14)
packages/runtime/src/client/contract.ts (2)
  • CRUD_EXT (219-219)
  • CRUD_EXT (229-229)
🪛 markdownlint-cli2 (0.18.1)
TODO.md

104-104: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)


105-105: Unordered list indentation
Expected: 2; Actual: 4

(MD007, ul-indent)

⏰ 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 (20.x, sqlite)
  • GitHub Check: build-test (20.x, postgresql)
  • GitHub Check: claude-review
🔇 Additional comments (50)
tests/regression/test/issue-204/regression.test.ts (1)

1-9: LGTM! Clean simplification of test structure.

Removing the describe wrapper for a single test case is appropriate and reduces unnecessary nesting. The test effectively serves as a regression check for issue 204, verifying that the Configuration type works correctly with optional teamColors and that the code compiles and runs without errors.

packages/testtools/package.json (1)

3-3: LGTM! Version bump aligned with beta.8 release.

The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.

packages/zod/package.json (1)

3-3: LGTM! Version bump aligned with beta.8 release.

The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.

packages/sdk/package.json (1)

3-3: LGTM! Version bump aligned with beta.8 release.

The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.

packages/language/package.json (1)

4-4: LGTM! Version bump aligned with beta.8 release.

The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump. This version bump correctly accompanies the language changes for before()/post-update semantics mentioned in the AI summary.

packages/dialects/sql.js/package.json (1)

3-3: LGTM! Version bump aligned with beta.8 release.

The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.

packages/runtime/test/policy/migrated/todo-sample.test.ts (1)

373-430: Post-update checks back under coverage

Re-enabling this suite restores the regression guardrails for the policy handoff; the assertions still line up with the shared fixtures.

packages/ide/vscode/package.json (1)

4-4: VSCode extension version bump confirmed

Keeping this package on its distinct 3.0.9 track honors the prescribed divergence. As per coding guidelines

packages/cli/package.json (1)

6-6: CLI version aligned with beta.8 release

The isolated bump to 3.0.0-beta.8 looks good for the monorepo release.

packages/vitest-config/package.json (1)

4-4: Vitest config version bump looks good

Nothing else changed; ready for the beta.8 publish.

package.json (1)

3-3: Root package version synchronized

3.0.0-beta.8 matches the coordinated release numbering.

packages/runtime/test/policy/todo-sample.test.ts (1)

386-444: Thanks for re-enabling this coverage. The scenario now exercises the new post-update policy checks end-to-end, so having it active will prevent regressions around owner reassignment.

tests/regression/package.json (1)

3-3: Version bump looks good. Matches the repo-wide 3.0.0-beta.8 release alignment.

packages/runtime/package.json (1)

3-3: Confirming runtime version sync. Keeping @zenstackhq/runtime at 3.0.0-beta.8 stays consistent with the coordinated release.

samples/blog/package.json (1)

3-3: Sample app version updated appropriately. This aligns the blog sample with the beta.8 release train.

packages/create-zenstack/package.json (1)

3-3: CLI scaffold version matches the release. No other changes needed for beta.8.

packages/tanstack-query/package.json (1)

3-3: Version bump aligns with beta.8 release

Matches the coordinated monorepo version lift; nothing else required here.

packages/runtime/test/policy/migrated/nested-to-one.test.ts (1)

200-255: Activated test confirms post-update guard

Re-enabling the scenario alongside the new @@allow('post-update', value > 2) keeps the legacy behavior (pre-state must exceed 1, post-state must exceed 2) and gives us coverage for the new hook—looks solid.

packages/runtime/test/schemas/petstore/schema.ts (1)

95-97: Generated post-update rule mirrors new policy semantics

The generated expression correctly encodes the new @@allow('update') plus @@allow('post-update') logic with before() accessors, so the runtime sees the same constraints our tests expect.

packages/runtime/test/schemas/petstore/schema.zmodel (1)

38-41: Verify the interaction between general update permission and post-update constraint.

The new policy structure uses two rules: Line 38 permits any authenticated user to update, while Line 41 enforces post-update constraints. This means update attempts are allowed upfront but will fail post-update if name or category changed, or if orderId was already set.

This is a significant semantic shift from the previous approach. Ensure that:

  1. The runtime correctly rolls back transactions when post-update rules fail
  2. Error messages clearly indicate which fields can be updated
  3. The comment on Line 40 accurately reflects that only orderId transitions from null are permitted (name/category must remain unchanged)

Based on learnings about the new before() and post-update semantics introduced in this PR.

packages/sdk/src/prisma/prisma-schema-generator.ts (2)

33-33: LGTM: Import updated to support auth invocation detection.

The import correctly adds isAuthInvocation utility to support the refactored default attribute filtering logic.


244-245: LGTM: Comment and filter logic updated for auth-based invocations.

The comment and filter predicate have been correctly updated to reflect that @default attributes using auth() are now handled outside Prisma, replacing the previous plugin-invocation pattern.

packages/runtime/test/schemas/todo/schema.ts (1)

1-4: Generated file - changes look correct.

This is an auto-generated schema file. The new post-update deny policies have been correctly generated using the before() function and proper ExpressionUtils API calls. The policies enforce that ownerId cannot be changed during updates.

packages/runtime/src/utils/expression-utils.ts (1)

15-58: LGTM! Well-designed visitor pattern implementation.

The ExpressionVisitor class provides a clean, extensible visitor pattern for traversing expression ASTs. The implementation correctly:

  • Uses exhaustive pattern matching via ts-pattern
  • Provides protected methods for subclass customization
  • Recursively visits all child expressions
  • Handles optional fields appropriately (e.g., args in CallExpression)
packages/language/src/index.ts (1)

72-78: LGTM! Improved validation configuration.

The updated validation configuration provides explicit, fine-grained control over when document building should stop on errors. Setting all three flags to true maintains fail-fast behavior while being more self-documenting than a simple boolean.

packages/testtools/src/project.ts (2)

5-5: LGTM! Clean API extension.

The optional zmodelContent parameter maintains backward compatibility while enabling test scenarios to inject custom schema definitions.


66-68: LGTM! Proper conditional file writing.

The conditional block correctly writes the provided zmodel content to schema.zmodel when supplied, following the expected naming convention.

packages/language/src/zmodel-scope.ts (1)

40-40: LGTM! Consistent rename from future() to before().

The import, usage, and comment have been consistently updated to reflect the rename from isFutureInvocation to isBeforeInvocation. The scope resolution logic correctly resolves before() invocations to the containing model.

Also applies to: 173-176

packages/runtime/src/plugins/policy/types.ts (1)

1-12: CRUD_EXT integration looks sound

PolicyOperation now reuses the extended CRUD set from the client contract, aligning policy typing with the new 'post-update' operation. Nice and tidy.

tests/regression/test/issue-274/regression.test.ts (1)

5-27: Regression scenario wired end-to-end

The temp-project scaffold plus CLI migrate run reproduces issue 274 realistically, giving us solid coverage on the reported workflow.

packages/runtime/src/plugins/policy/utils.ts (1)

16-160: Helper covers runtime before() detection

Adding isBeforeInvocation centralizes the call-expression check for before(), keeping runtime policy logic aligned with the language validators.

packages/runtime/src/plugins/policy/errors.ts (1)

1-33: Error now fits the ZenStackError hierarchy

Extending ZenStackError keeps policy rejection errors aligned with the shared error stack while preserving the customizable message.

packages/runtime/src/client/contract.ts (1)

217-229: Contract exposes extended CRUD literal set

Publishing CRUD_EXT as both a type and tuple constant makes it straightforward for callers to consume the new 'post-update' operation.

packages/language/src/validators/attribute-application-validator.ts (1)

169-182: Good guard for before() usage

Restricting before() to pure "post-update" rules prevents models from sneaking pre-update reads into other operations, which keeps policy semantics consistent. Nice catch.

packages/language/src/validators/expression-validator.ts (1)

73-77: Great validation for relation access in before()

Blocking before().relation at the expression layer closes an easy loophole for reaching fresh relation snapshots. This error path is precise and keeps diagnostics user-friendly.

packages/runtime/src/plugins/policy/plugin.ts (1)

25-28: Signature alignment looks good

Updating the handler call to the new (query, proceed) contract keeps the plugin wired correctly with the latest policy handler API.

packages/runtime/test/schemas/todo/todo.zmodel (2)

120-122: LGTM! Post-update policy correctly prevents owner changes.

The new @@deny('post-update', before().ownerId != ownerId) rule properly uses the before() helper to compare the previous owner with the new owner, denying updates that attempt to transfer ownership.


149-150: LGTM! Consistent post-update policy implementation.

The @@deny('post-update', before().ownerId != ownerId) rule matches the pattern established in the List model, ensuring consistent enforcement of ownership immutability across the schema.

packages/language/src/zmodel-document-builder.ts (1)

4-25: LGTM! Constructor-based validation configuration is cleaner.

The new approach correctly configures validation to stop after lexing, parsing, and linking errors at construction time. The conditional logic properly handles both boolean and object validation options.

packages/runtime/src/client/errors.ts (2)

1-4: LGTM! Proper error hierarchy base class.

The new ZenStackError base class correctly extends Error and provides a common ancestor for all ZenStack runtime errors, enabling centralized error handling.


9-36: LGTM! Error classes correctly extend the new base.

All four error classes now properly extend ZenStackError while maintaining their existing constructor signatures, ensuring backward compatibility.

packages/language/src/zmodel-linker.ts (5)

1-15: LGTM! Import updates align with the before() rename.

The addition of ReferenceInfo and replacement of isFutureExpr with isBeforeInvocation correctly support the refactored linking architecture and the renamed policy helper.

Also applies to: 54-63


97-109: LGTM! Refactored linking logic with proper early exits.

The new linkReference signature using ReferenceInfo enables cleaner separation of concerns, with early exit for already-linked references and prioritized scope provider resolution before default linking.


276-301: LGTM! Invocation resolution updated for before() semantics.

The updated linkReference call and the replacement of isFutureExpr with isBeforeInvocation correctly handle the renamed helper function, resolving before() to the containing data model.


115-133: LGTM! Scope provider resolution properly stores results.

The updated resolveFromScopeProviders correctly accepts a DefaultReference, stores the resolution result, creates the appropriate description, and updates the document's reference array.


495-502: LGTM! Cleaner reference iteration with streamReferences.

Replacing manual property iteration with AstUtils.streamReferences is more idiomatic and maintainable while preserving the same linking behavior.

packages/runtime/src/plugins/policy/expression-transformer.ts (4)

23-23: LGTM! Type updates support post-update operations.

The import and type changes from CRUD to CRUD_EXT correctly support the new post-update operation, and the isBeforeInvocation import enables the new before() handling logic.

Also applies to: 53-53, 58-58


440-443: LGTM! Consistent use of class property.

Using this.auth instead of extracting from context is more consistent with the class design and leverages the existing getter.


460-467: LGTM! Correct before() member access implementation.

The new before() handling properly:

  • Validates usage in post-update context only
  • Restricts access to scalar fields (single-level member access)
  • Returns a table-qualified reference to the $before join table created by the policy handler

The invariants provide clear error messages for misuse.


455-458: LGTM! Comment clarity improvement.

Adding parentheses to "auth() member access" improves readability and consistency with the new before() comment.

@claude
Copy link

claude bot commented Oct 1, 2025

Claude finished @ymc9's task —— View job


PR Review for v3.0.0-beta.8 Release

Reviewing this PR that merges dev → main for the v3.0.0-beta.8 release with 831 additions and 373 deletions across 59 files.

Todo List

  • Set up development environment and dependencies
  • Review major architectural changes in runtime policy handling
  • Analyze new post-update policy implementation
  • Check language validation improvements
  • Review test coverage and new test files
  • Evaluate security implications of policy changes
  • Check for potential performance issues
  • Provide comprehensive feedback summary

🔍 Comprehensive Review Summary

Strengths & Well-Implemented Features

1. Post-Update Policies - Major New Feature

  • Robust implementation: The new post-update policy operation is well-designed with proper validation flow
  • Complex SQL generation: Smart handling of before/after state comparison using VALUES clauses and JOINs
  • Comprehensive validation: Prevents relation field access from before() calls (security-conscious)
  • Transaction safety: All mutations properly wrapped in RepeatableRead transactions

2. Code Quality & Architecture

  • Strong type safety: Extensive use of TypeScript discriminated unions and type guards
  • Error handling: Proper error propagation with meaningful error messages
  • Test coverage: 191 lines of comprehensive test cases for the new feature
  • Backward compatibility: Changes are additive and don't break existing functionality

3. Language & Validation Improvements

  • Parser enhancements: Updated ZModel grammar to support post-update in policy rules
  • Runtime validation: Proper validation that before() calls are only allowed in post-update contexts
  • Developer experience: Good error messages and validation feedback

⚠️ Areas of Concern & Recommendations

1. Performance Considerations

  • Before-state loading: The system loads entire "before" entity state for post-update policies, which could be expensive for large entities

    // packages/runtime/src/plugins/policy/policy-handler.ts:226-243
    private async loadBeforeUpdateEntities(...) {
      // Loads all fields referenced in before() calls
    }

    Recommendation: Consider optimization for cases where only specific fields are needed

  • Complex SQL generation: Post-update validation generates nested queries with VALUES clauses

    // Lines 138-160: Complex table construction for before-state comparison
    const beforeUpdateTable: SelectQueryNode | undefined = beforeUpdateInfo ? { /*complex query*/ } : undefined;

    Recommendation: Monitor query performance on large datasets

2. Security Assessment

  • Access control: Proper prevention of relation field access from before() calls
  • SQL injection: Uses parameterized queries and proper SQL construction through Kysely
  • Transaction isolation: RepeatableRead isolation level prevents concurrent modification issues
  • No security vulnerabilities identified

3. Minor Issues

Expression Transformer Safety

The expression transformer properly handles field access validation:

// packages/language/src/validators/attribute-application-validator.ts:185-189
accept('error', `relation fields cannot be accessed from \`before()\``, { node: beforeCall });

Version Bumps

All package versions consistently bumped to 3.0.0-beta.8 - good release hygiene.

🚀 Technical Excellence

1. Policy Engine Robustness

  • Smart field collection: Only loads fields actually referenced in before() calls
  • Efficient ID handling: Ensures ID fields are always included for proper joining
  • Proper SQL casting: Handles type casting for different field types in VALUES clauses

2. Error Handling

  • Meaningful errors: Clear error messages for policy violations
  • Proper context: Errors include model context and operation type
  • Graceful failures: Transactions properly rolled back on policy failures

📊 Test Coverage Quality

The new test file (post-update.test.ts) covers:

  • ✅ Basic post-update validation
  • ✅ Simple allow/deny rules
  • ✅ Relation field access
  • before() function usage
  • ✅ Query builder API compatibility
  • ✅ Proper error validation

🏆 Overall Assessment

This is a high-quality release that introduces a significant new feature (post-update policies) with:

  • Solid architecture and implementation
  • Comprehensive testing
  • Good security practices
  • Proper error handling
  • Backward compatibility

The implementation demonstrates deep understanding of database transactions, SQL generation, and policy evaluation. The code follows established patterns and maintains high code quality standards.

🎯 Recommendation: APPROVE

This PR successfully implements post-update policies as a major feature enhancement with proper testing, security considerations, and maintainable code. The minor performance considerations noted above are acceptable for a beta release and can be optimized in future iterations based on real-world usage patterns.

The v3.0.0-beta.8 release appears ready for deployment.


@ymc9 ymc9 added this pull request to the merge queue Oct 1, 2025
Merged via the queue into main with commit 35a9e0d Oct 1, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants