Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Oct 29, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 29, 2025

📝 Walkthrough

Walkthrough

Updates documentation with ZenStack v3 beta information, bumps JetBrains IDE plugin version, enhances policy field-level visibility checks for select/include semantics, adds Zod major version support (v3/v4) to code generation, extends test tooling with database file configuration, and adds a new regression test.

Changes

Cohort / File(s) Change Summary
Documentation
README.md
Added new "V3 is in Beta now!" section describing ZenStack v3's Kysely-based implementation and separate repository.
IDE Build Configuration
packages/ide/jetbrains/build.gradle.kts
Bumped project version from 2.20.1 to 2.21.0.
Policy Runtime
packages/runtime/src/enhancements/node/policy/policy-utils.ts
Added field-level visibility logic respecting select/include semantics; removes non-selected scalar fields and non-included relational fields during policy processing, operating alongside omit and unreadable-field checks.
Zod Plugin — Generator & Types
packages/schema/src/plugins/zod/generator.ts, packages/schema/src/plugins/zod/types.ts
Introduced Zod version selection (v3|v4) via new option; parameterized all zod imports and refactored generated refinement function signature from (schema: z.ZodType<T, D, T>) to (schema: z.ZodType<T>). Added zodVersion: 'v3' | 'v4' to TransformerParams.
Zod Plugin — Transformer
packages/schema/src/plugins/zod/transformer.ts
Added private zodVersion property and makeZodType() helper for version-aware type generation; updated imports and schema type declarations to use dynamic Zod version paths and type signatures.
Test Infrastructure
packages/testtools/src/schema.ts
Added optional dbFile?: string field to SchemaLoadOptions; added logic to copy provided database file into project when specified, else preserve existing push behavior.
Test Suite
packages/server/tests/adapter/elysia.test.ts
Skipped "custom load path" test case with TODO and eslint-disable comments.
Zod Import Path & Regression
tests/regression/tests/issue-1378.test.ts, tests/regression/tests/issue-2283/regression.test.ts
Updated Zod import from 'zod' to 'zod/v3' in issue-1378 test; added new regression test for issue-2283 validating nested query resolution with access control on deeply nested relationships.

Sequence Diagram

sequenceDiagram
    participant Query as Query Layer
    participant PolicyEval as Policy Evaluation
    participant FieldVis as Field Visibility
    participant Result as Result

    Query->>PolicyEval: queryArgs with select/include
    PolicyEval->>FieldVis: Check hasFieldLevelPolicy
    alt Field-Level Policy Enabled
        FieldVis->>FieldVis: Apply select/include filters
        Note over FieldVis: Remove non-selected scalars<br/>Remove non-included relations
        FieldVis->>FieldVis: Apply existing omit handling
        FieldVis->>PolicyEval: Filtered fields + checks
    else No Field-Level Policy
        FieldVis->>PolicyEval: Original fields
    end
    PolicyEval->>PolicyEval: Evaluate read authorization
    PolicyEval->>FieldVis: Identify unreadable fields
    FieldVis->>FieldVis: Remove unreadable fields
    FieldVis->>Result: Final sanitized result
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Policy utilities logic (policy-utils.ts): Added multi-stage field pruning logic (select/include filters, omit handling, unreadable-field checks) that interacts with existing policy evaluation; requires careful verification of field removal order and interaction with recursive processing.
  • Zod version-aware generation (generator.ts, transformer.ts, types.ts): Version branching logic for imports and type signatures; dynamic makeZodType() helper with different outputs for v3 vs v4; refactored refinement function signature affects downstream usage.
  • Regression test (issue-2283/regression.test.ts): Large inlined schema with complex nested relationships and back-references; requires validation that access control and field visibility logic work correctly across deep nesting.

Possibly related PRs

  • #2288: Adds zodVersion support to Transformer/generator with version-aware imports and type generation, directly overlapping with this PR's Zod plugin changes.
  • #2286: Modifies policy-utils.ts to drop fields added for policy evaluation with select/include and omit handling, addressing the same field visibility concerns as this PR.
  • #2289: Updates README with "V3 is in Beta now!" section and bumps JetBrains IDE plugin version, combining the documentation and build changes in this PR.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The pull request title "merge dev to main (v2.21.0)" describes the mechanical action of merging branches rather than highlighting the substantive changes in the pull request. While the version number 2.21.0 relates to an actual change in the codebase (version bump in build.gradle.kts), the title does not convey meaningful information about the key features and improvements included in this release, such as Zod version support improvements, policy processing enhancements, or the V3 beta announcement. The title is generic and uses non-descriptive terminology that primarily explains the branching action rather than the actual content of the changes.
Description Check ❓ Inconclusive No pull request description was provided by the author. The description field is entirely absent, which means it conveys zero meaningful information about the changeset. While the description check is intentionally lenient, the complete absence of any descriptive content leaves no context for reviewers about the objectives, scope, or specific changes included in this version 2.21.0 release. This extreme vagueness makes it impossible to assess whether the description relates to the actual changes without examining the raw summaries.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

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.

Copy link
Contributor

@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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9cae5c and a7a2fe2.

⛔ Files ignored due to path filters (15)
  • package.json is excluded by !**/*.json
  • packages/ide/jetbrains/package.json is excluded by !**/*.json
  • packages/language/package.json is excluded by !**/*.json
  • packages/misc/redwood/package.json is excluded by !**/*.json
  • packages/plugins/openapi/package.json is excluded by !**/*.json
  • packages/plugins/swr/package.json is excluded by !**/*.json
  • packages/plugins/tanstack-query/package.json is excluded by !**/*.json
  • packages/plugins/trpc/package.json is excluded by !**/*.json
  • packages/runtime/package.json is excluded by !**/*.json
  • packages/schema/package.json is excluded by !**/*.json
  • packages/sdk/package.json is excluded by !**/*.json
  • packages/server/package.json is excluded by !**/*.json
  • packages/testtools/package.json is excluded by !**/*.json
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/*.yaml
  • tests/regression/tests/issue-2283/dev.db is excluded by !**/*.db, !**/*.db
📒 Files selected for processing (10)
  • README.md (1 hunks)
  • packages/ide/jetbrains/build.gradle.kts (1 hunks)
  • packages/runtime/src/enhancements/node/policy/policy-utils.ts (1 hunks)
  • packages/schema/src/plugins/zod/generator.ts (8 hunks)
  • packages/schema/src/plugins/zod/transformer.ts (8 hunks)
  • packages/schema/src/plugins/zod/types.ts (1 hunks)
  • packages/server/tests/adapter/elysia.test.ts (1 hunks)
  • packages/testtools/src/schema.ts (2 hunks)
  • tests/regression/tests/issue-1378.test.ts (1 hunks)
  • tests/regression/tests/issue-2283/regression.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/regression/tests/issue-2283/regression.test.ts (1)
packages/testtools/src/schema.ts (1)
  • loadSchema (173-249)
packages/schema/src/plugins/zod/generator.ts (2)
packages/sdk/src/types.ts (1)
  • PluginError (115-119)
packages/schema/src/plugins/zod/index.ts (1)
  • name (5-5)
packages/schema/src/plugins/zod/transformer.ts (2)
packages/runtime/src/local-helpers/upper-case-first.ts (1)
  • upperCaseFirst (1-3)
packages/sdk/src/utils.ts (1)
  • indentString (129-132)
⏰ 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). (5)
  • GitHub Check: OSSAR-Scan
  • GitHub Check: build-test (20.x)
  • GitHub Check: build-test (20.x)
  • GitHub Check: dependency-review
  • GitHub Check: build-test (20.x)
🔇 Additional comments (6)
packages/ide/jetbrains/build.gradle.kts (1)

12-12: Version bump correctly applied and consistent with PR objectives.

The version has been properly updated from 2.20.1 to 2.21.0, matching the PR title and objectives. The version string is correctly referenced in the changelog header generation on line 84 via ${version.get()}, ensuring consistency across the plugin metadata.

packages/server/tests/adapter/elysia.test.ts (1)

87-89: Verify and document the root cause of the CI failure before merging to main.

The test is skipped with only a vague "failing in CI" comment, but lacks:

  • The specific failure mode or error message
  • A tracked GitHub issue number for this regression
  • Clarity on whether this is a flaky/temporal issue or a real bug

Since this change is being merged to main (v2.21.0), skipping important functionality testing without proper tracking could mask real issues. Verify that:

  1. There's a corresponding GitHub issue tracking this failure
  2. The root cause has been diagnosed
  3. The skip is intentional (rather than a workaround for an unresolved bug)

Can you provide the GitHub issue number that tracks this failure and clarify the specific error occurring in CI?

packages/testtools/src/schema.ts (1)

153-153: LGTM! Clean API extension.

The new optional dbFile parameter extends the test tooling surface appropriately for pre-populated database scenarios.

tests/regression/tests/issue-2283/regression.test.ts (2)

4-683: Well-structured regression test for nested query field visibility.

The test appropriately exercises:

  • Complex schema with multiple models and relations
  • Access control rules using @@allow directives
  • Nested queries with explicit select clauses
  • Field-level visibility verification (unselected field should be undefined)

The assertion on line 681 correctly validates that the module field is undefined on the Class object since it wasn't included in the select clause, which tests proper field filtering in deeply nested queries under access control.


637-637: No issues found—the dev.db file exists at the expected location.

The verification confirms that tests/regression/tests/issue-2283/dev.db is present in the repository and properly committed.

packages/runtime/src/enhancements/node/policy/policy-utils.ts (1)

1537-1547: Select/include pre-filter keeps payload aligned

Respecting select/include before policy checks plugs the gap where supplemental selectors leaked into responses when no field-level policy was present. This matches Prisma semantics and makes the later readability pruning consistent. Nice catch.

@ymc9 ymc9 merged commit 1b05fec into main Oct 30, 2025
13 of 14 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