-
-
Notifications
You must be signed in to change notification settings - Fork 17
merge dev to main (v3.3.0) #636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* feat: `between` operator * chore: add tests * Document inclusivity. * Add reversed order test. * chore: add REST server test * Fix test typo. * Verify correct object returned. * Additional test cases. * Make AI suggested changes. * Remove filter. * Fix styling. * Add more REST tests. * Add more int tests for REST.
…sion (#603) * feat(orm): implement client API extensions, refactor query args extension * address PR comments * Fix upsert validation to merge $create and $update schemas (#604) * Initial plan * fix: merge $create and $update schemas for upsert validation - Handle upsert operation specially to match TypeScript type behavior - When both $create and $update schemas exist, merge them for upsert - Add test case to verify the fix works correctly Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> * fix: improve comment accuracy about Zod merge behavior Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> * minor fixes --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
* feat: audit policy collection aliases provides a means to alias collections in @@Allow collections by extending the ast this allows for utilizing collections inside of @@Allow like: ``` memberships?[m, auth().memberships?[ tenantId == m.tenantId ... ] ] ``` * fix: code review comments + syntax fixes * refactor: extract collection predicate binding to its own language construct (#2) - adjusted language processing chain accordingly - fixed several issues in policy transformer/evaluator - more test cases * addressing PR comments --------- Co-authored-by: Yiming Cao <yiming@whimslab.io> Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* feat(cli): ZenStack proxy * add the missing change * resolve comments * feat(cli): add alias for proxy command and improve console messages * fix(cli): update output option description for ZenStack proxy command
* add test for pg computed values across multiple schema * Add an any cast to type check test file --------- Co-authored-by: Yiming Cao <yiming@whimslab.io>
* WIP(orm): mysql support * WIP: more progress with fixing tests * WIP: get all client api tests pass * WIP: get all tests pass * fix executor * add MySQL to CI matrix * fix sqlite test runs * fix test * fix delete readback check * set mysql container max connections * fix tests * fix test * refactor: extract duplicated mysql/pg code into base class * address PR comments * refactor: remove order by duplicated code * refactor: optimize stripTableReference * addressing PR comments * fix tests
* WIP: more aggressive caching of validation zod schemas * refactor: use a decorator-based approach for caching * update * update * update * update * update * remove object-type args from cache key * update cache key
* fix(orm): deal with node-pg timezone issue more reliably fixes #606 * addressing PR comments
… (#632) Fixed issue where access policy rules couldn't reference fields inherited from mixins defined in separate imported files. The language service now correctly resolves these fields during scope computation. ## Root Cause The `getRecursiveBases()` function only searched for mixin declarations in the current document (`decl.$container.declarations`), which failed for imported mixins. ## Solution - Modified `getRecursiveBases()` to accept optional `LangiumDocuments` parameter - Implemented two-strategy approach: 1. Use resolved reference if available (post-linking) 2. Search by name across all documents including imports (pre-linking) - Updated `ZModelScopeComputation.processNode()` to pass `LangiumDocuments` - Leverages existing `getAllDeclarationsIncludingImports()` helper ## Changes - **packages/language/src/utils.ts**: Fixed `getRecursiveBases()` to search imported documents - **packages/language/src/zmodel-scope.ts**: Pass LangiumDocuments to scope computation - **packages/language/test/mixin.test.ts**: Added tests for imported mixin field resolution - **packages/testtools**: Added `extraZModelFiles` option for multi-file test schemas - **tests/regression/test/issue-598.test.ts**: Regression test for the issue ## Test Results ✅ All language package tests pass (65 tests) ✅ Regression test validates policy rules can access imported mixin fields ✅ Handles edge cases: cyclic imports, nested mixins, transitive imports Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
* feat(cli): add dotenv support * [WIP] Update dotenv support implementation in CLI based on review (#631) * Initial plan * fix(cli): move dotenv import to top of file Co-authored-by: jiashengguo <16688722+jiashengguo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: jiashengguo <16688722+jiashengguo@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
* fix(cli): handle error for proxy server * fix(cli): update default port for ZenStack proxy server from 8008 to 2311
* feat: `ignore` argument for `@updatedAt` * chore: add tests * Trigger Build * Check test. * Use `getTime` * Retry. * Retry. * Retry. * Retry. * Retry. * Retry. * Retry. * Clean up. * Document param. * Extract to function. * Relocate function. * Adjust formatting. * Use `getAttributeArg` * Use `$resolvedParam` * Null check. * fix: resolve a merge error * fix: delay data clone to right before changing it --------- Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* feat(cli): add mysql support to proxy * Initial plan * fix: redact credentials from database URL logs Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> * fix: also redact SQLite URLs for consistency Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> * Merge dev branch and remove SQLite URL redaction Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> * docs: clean up merge conflicts by rebasing on correct base * Push new branch to create fresh PR Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> * clean up agent mess --------- Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
|
Important Review skippedToo many files! This PR contains 176 files, which is 26 over the limit of 150. You can disable this status message by setting the
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job Code Review of v3.3.0 Release PRI've completed a comprehensive review of this large PR merging dev to main for the 3.3.0 release. Review Tasks:
Key Findings🚨 Critical Issues1. Security Vulnerability in URL Evaluation ( const urlFn = new Function('env', `return ${value}`);This uses 2. Missing Test Coverage for Major Features
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
Claude finished @ymc9's task —— View job Code Review of v3.3.0 Release PRI've completed a comprehensive review of this large PR merging dev to main for the 3.3.0 release. Review Tasks:
Key Findings🚨 Critical Issues1. Security Vulnerability in URL Evaluation ( const urlFn = new Function('env', `return ${value}`);This uses 2. Missing Test Coverage for Major Features
|
…r datasource url (#637) * refactor(cli): replace dynamic function call eval with regex match for datasource url * fix(proxy): improve regex for env() function call matching
|
Claude finished @ymc9's task —— View job Code Review of v3.3.0 Release PRI've completed a comprehensive review of this large PR merging dev to main for the 3.3.0 release. Review Tasks:
Key Findings✅ Improvements from Previous Reviews1. Security Issues Resolved
2. Code Quality Improvements
🔧 MySQL Dialect Implementation QualityStrengths:
Observations:
🔐 Security AssessmentGood Security Practices:
Areas of Concern:
📊 Test Coverage AnalysisGood Coverage:
Coverage Gaps:
⚡ Performance ConsiderationsPotential Performance Impacts:
🔄 Breaking Changes & CompatibilityVersion Management:
RecommendationsBefore Merging:
Post-Release:
SummaryThis PR represents a significant improvement over previous versions, with the critical security vulnerability properly addressed. The MySQL implementation is well-architected and follows established patterns. The better-auth integration is comprehensive and tested. Key Highlights:
Recommendation: ✅ APPROVE for merge with the understanding that the remaining security and testing improvements should be addressed in follow-up PRs. The code quality is good, security issues are resolved, and new features are well-implemented. While test coverage could be improved and some security hardening remains, this is a solid release that significantly advances the platform. |
|
Claude finished @ymc9's task —— View job Code Review of v3.3.0 Release PRI've completed a comprehensive review of this large PR merging dev to main for the 3.3.0 release. Review Tasks:
Key Findings✅ Improvements from Previous Reviews1. Security Issues Resolved
2. Code Quality Improvements
🔧 MySQL Dialect Implementation QualityStrengths:
Observations:
🔐 Security AssessmentGood Security Practices:
Areas of Concern:
📊 Test Coverage AnalysisGood Coverage:
Coverage Gaps:
⚡ Performance ConsiderationsPotential Performance Impacts:
🔄 Breaking Changes & CompatibilityVersion Management:
RecommendationsBefore Merging:
Post-Release:
SummaryThis PR represents a significant improvement over previous versions, with the critical security vulnerability properly addressed. The MySQL implementation is well-architected and follows established patterns. The better-auth integration is comprehensive and tested. Key Highlights:
Recommendation: ✅ APPROVE for merge with the understanding that the remaining security and testing improvements should be addressed in follow-up PRs. The code quality is good, security issues are resolved, and new features are well-implemented. While test coverage could be improved and some security hardening remains, this is a solid release that significantly advances the platform. |
|
Claude finished @ymc9's task —— View job Code Review of v3.3.0 Release PRI've completed a comprehensive review of this large PR merging dev to main for the 3.3.0 release. Review Tasks:
Key Findings✅ Improvements from Previous Reviews1. Security Issues Resolved
2. Code Quality Improvements
🔧 MySQL Dialect Implementation QualityStrengths:
Observations:
🔐 Security AssessmentGood Security Practices:
Areas of Concern:
📊 Test Coverage AnalysisGood Coverage:
Coverage Gaps:
⚡ Performance ConsiderationsPotential Performance Impacts:
🔄 Breaking Changes & CompatibilityVersion Management:
RecommendationsBefore Merging:
Post-Release:
SummaryThis PR represents a significant improvement over previous versions, with the critical security vulnerability properly addressed. The MySQL implementation is well-architected and follows established patterns. The better-auth integration is comprehensive and tested. Key Highlights:
Recommendation: ✅ APPROVE for merge with the understanding that the remaining security and testing improvements should be addressed in follow-up PRs. The code quality is good, security issues are resolved, and new features are well-implemented. While test coverage could be improved and some security hardening remains, this is a solid release that significantly advances the platform. |
No description provided.