Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Sep 23, 2025

Summary by CodeRabbit

  • New Features
    • Access policy support is now built into the core language, including allow/deny rules, a policy check helper, and restored future() for policy expressions.
  • Refactor
    • Consolidated policy capabilities from external plugins into the language, simplifying configuration and reducing build steps.
  • Chores
    • Bumped versions across packages to 3.0.0-beta.6.

github-actions bot and others added 2 commits September 22, 2025 22:31
* refactor: move policy zmodel back to stdlib

* update
Copilot AI review requested due to automatic review settings September 23, 2025 05:43
@coderabbitai
Copy link

coderabbitai bot commented Sep 23, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Policy DSL declarations were moved from runtime plugin to language stdlib. All callers were updated to load schemas without plugin model discovery. Post-build copy of the policy plugin was removed. Multiple package.json versions were bumped to 3.0.0-beta.6. Tests and utilities were adjusted accordingly.

Changes

Cohort / File(s) Summary
Version bumps
package.json, packages/cli/package.json, packages/common-helpers/package.json, packages/create-zenstack/package.json, packages/dialects/sql.js/package.json, packages/eslint-config/package.json, packages/language/package.json, packages/runtime/package.json, packages/sdk/package.json, packages/tanstack-query/package.json, packages/testtools/package.json, packages/typescript-config/package.json, packages/vitest-config/package.json, packages/zod/package.json, samples/blog/package.json, tests/e2e/package.json, tests/regression/package.json
Bumped versions from 3.0.0-beta.5 to 3.0.0-beta.6.
Language stdlib policy surface
packages/language/res/stdlib.zmodel
Added policy attributes @@allow, @allow, @@deny, @deny, function check, and re-added future() under AccessPolicy context.
Runtime policy plugin removal
packages/runtime/src/plugins/policy/plugin.zmodel, packages/runtime/tsup.config.ts
Removed policy plugin declarations and the build hook that copied the plugin file; dropped unused fs import.
Schema loading simplification
packages/language/test/utils.ts, packages/runtime/test/utils.ts, packages/runtime/test/scripts/generate.ts, tests/regression/generate.ts, packages/testtools/src/schema.ts
Stopped discovering/using plugin model files; updated loadDocument calls to accept only the main schema path; removed getPluginModules/glob helpers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

A bunny rewrote rules in moonlit code,
From runtime’s burrow to language’s abode.
No more plugins to fetch or sweep,
The loader hops light, a single leap.
Beta’s ticked—.6 we see,
Carrots raised to policy! 🥕✨

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dev

📜 Recent 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 495a512 and 9bf6d7f.

📒 Files selected for processing (25)
  • 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/language/package.json (1 hunks)
  • packages/language/res/stdlib.zmodel (1 hunks)
  • packages/language/test/utils.ts (2 hunks)
  • packages/runtime/package.json (1 hunks)
  • packages/runtime/src/plugins/policy/plugin.zmodel (0 hunks)
  • packages/runtime/test/scripts/generate.ts (1 hunks)
  • packages/runtime/test/utils.ts (2 hunks)
  • packages/runtime/tsup.config.ts (0 hunks)
  • packages/sdk/package.json (1 hunks)
  • packages/tanstack-query/package.json (1 hunks)
  • packages/testtools/package.json (1 hunks)
  • packages/testtools/src/schema.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/generate.ts (1 hunks)
  • tests/regression/package.json (1 hunks)

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

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 the dev branch to main, updating the version from 3.0.0-beta.5 to 3.0.0-beta.6 across all packages and consolidating plugin model definitions into the standard library.

  • Version bump from 3.0.0-beta.5 to 3.0.0-beta.6 across all packages
  • Removal of plugin model file distribution and loading mechanism
  • Consolidation of policy plugin definitions into the standard library

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file
File Description
package.json Root package version update
packages/*/package.json Version updates across all packages
packages/testtools/src/schema.ts Removed plugin model loading and getPluginModules function
packages/runtime/src/plugins/policy/plugin.zmodel Deleted plugin model file
packages/runtime/tsup.config.ts Removed plugin file copying in build
packages/language/res/stdlib.zmodel Added policy plugin definitions and future() function
tests/*/generate.ts Simplified loadDocument calls without plugin files
packages/language/test/utils.ts Removed plugin model loading utilities
packages/runtime/test/utils.ts Updated to use simplified loadDocument calls

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

@ymc9 ymc9 changed the title merge dev to main merge dev to main (v3.0.0-beta.6) Sep 23, 2025
@claude
Copy link

claude bot commented Sep 23, 2025

Claude finished @ymc9's task —— View job


PR Review: v3.0.0-beta.6 Release

I've completed my review of this PR that merges dev to main for version 3.0.0-beta.6. Here's my comprehensive analysis:

Review Tasks:

  • Analyze version update changes across packages
  • Review plugin model consolidation refactoring
  • Check stdlib.zmodel changes for correctness
  • Examine test utility updates
  • Review build configuration changes
  • Assess overall architectural impact

Summary of Changes

This PR implements two main changes:

  1. Version bump from 3.0.0-beta.5 to 3.0.0-beta.6 across all 25 packages
  2. Plugin model consolidation - moving policy plugin definitions from packages/runtime/src/plugins/policy/plugin.zmodel into the standard library

Code Quality Assessment

Good Practices Observed:

  • Clean refactoring that simplifies the build process
  • Consistent version bumping across all packages
  • Proper removal of build artifacts and related code

⚠️ Areas of Concern:

  1. Missing Test Coverage Analysis

    • No tests included in this PR to verify the plugin consolidation works correctly
    • Test utilities were updated but it's unclear if the existing test suite covers the new future() function
    • Recommendation: Ensure integration tests verify that policy plugins work identically after consolidation
  2. Build Process Changes

    • Removed plugin file copying from tsup.config.ts (lines 11-16 in packages/runtime/tsup.config.ts:d5bea54)
    • Risk: If any external consumers depend on accessing the plugin file directly, this could be a breaking change
    • Recommendation: Verify no external dependencies exist on the removed plugin file distribution
  3. New future() Function

    • Added in stdlib.zmodel:710-711 without comprehensive documentation in this PR
    • Recommendation: Ensure the function is properly tested and documented

Security Assessment

No security concerns identified - this is a straightforward refactoring that:

  • Moves definitions to a more centralized location
  • Doesn't modify access control logic
  • Simplifies the build process

Performance Considerations

Performance impact likely positive:

  • Eliminates runtime plugin file loading (packages/testtools/src/schema.ts:43)
  • Reduces build complexity by removing file copying operations
  • Consolidates definitions in the standard library for better compile-time optimization

Architecture Review

This refactoring improves the overall architecture by:

  • Centralizing definitions: Policy plugin models now live in the standard library where they belong
  • Simplifying distribution: No need to distribute separate plugin files
  • Reducing complexity: Eliminates the getPluginModules function and related loading logic

Potential Issues

  1. Breaking Change Risk: External consumers accessing plugin files directly

    • File removed: packages/runtime/src/plugins/policy/plugin.zmodel
    • Impact: Low risk since this appears to be internal infrastructure
  2. Incomplete Migration Evidence:

    • No test cases included demonstrating the policy functionality works identically
    • Should verify all policy features (@@allow, @@deny, check(), future()) function correctly

Recommendations

  1. Add Integration Tests: Include tests that verify policy functionality works with the consolidated model
  2. Documentation Update: Document the new future() function behavior and usage
  3. Verify External Dependencies: Confirm no external packages depend on the removed plugin file
  4. Runtime Verification: Test that loadDocument calls work correctly without plugin file parameters

Final Assessment

Overall Quality: Good

This is a well-executed architectural improvement that simplifies the codebase while maintaining functionality. The version bump is clean and consistent. The main risk is the lack of test coverage for the refactoring, but the changes appear sound.

Recommendation: Approve with minor follow-up testing


@ymc9 ymc9 added this pull request to the merge queue Sep 23, 2025
Merged via the queue into main with commit 1770b12 Sep 23, 2025
7 of 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