merge dev to main (v3.0.0-beta.1) #224
Conversation
* chore: update readme * update
* chore: separate extension and language id for vscode extension * update * update * update
* chore: misc CLI improvements * update
* feat: CLI telemetry * update * update * update * update
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
* feat: new version check * update
|
Note Other AI code review bot(s) detectedCodeRabbit 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. Warning Rate limit exceeded@ymc9 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 0 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
WalkthroughAdds CLI telemetry, version-check gating, and a silent mode for generate. Introduces environment detection utilities and build-time token injection. Updates scaffolding to use “zen” commands and create VSCode configs. Tightens language validation, improves SDK unique-key generation, revises VSCode extension to v3, trims README, updates CI env, and bumps versions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude finished @ymc9's task —— View job Code Review for v3.0.0-beta.1 ReleaseI've completed my comprehensive review of this significant release PR. Here's my detailed analysis:
Overall AssessmentThis is a well-structured release with significant new features and improvements. The code quality is high overall, but there are several important considerations below. Code Quality & Best Practices ✅Strengths:
Security Considerations
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges the dev branch to main, primarily involving a version bump from 3.0.0-alpha.33 to 3.0.0-beta.1 across the monorepo packages along with several feature additions and improvements.
- Version bump from alpha.33 to beta.1 across all packages
- Addition of telemetry functionality to the CLI
- VS Code extension improvements for v3 with better side-by-side support
- New multi-field unique constraint features and testing
Reviewed Changes
Copilot reviewed 42 out of 44 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json files | Version bump to 3.0.0-beta.1 across all packages |
| packages/cli/src/* | Added telemetry tracking, version checking, and improved CLI functionality |
| packages/ide/vscode/* | Updated VS Code extension for v3 with new naming and configuration |
| packages/sdk/src/ts-schema-generator.ts | Added support for named compound unique constraints |
| packages/runtime/test/policy/* | Added comprehensive test coverage for multi-field unique constraints and mixins |
| packages/language/src/validators/typedef-validator.ts | Added validation to prevent relation fields in type definitions |
| scripts/bump-version.ts | Added exclusion logic for VS Code package version bumping |
| .github/workflows/build-test.yml | Added telemetry environment variables |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/create-zenstack/package.json (1)
14-19: Potential publish break: bin/cli may be excluded from package tarballWith
"files": ["dist"], npm may excludebin/clifrom the published package, breaking the CLI. Includebinexplicitly to be safe.Apply:
"files": [ - "dist" + "dist", + "bin" ],Optionally also include README and license if desired:
"files": [ "dist", "bin", + "README.md", + "LICENSE" ],packages/cli/src/utils/version-utils.ts (1)
10-18: tsup.config.ts: package.json isn’t copied intodist, sogetVersion()will always returnundefinedat runtime. Add a copy step forpackage.jsonin your build (e.g. via tsup’scopyoption) or inject the version at build time usingdefine(for example,__ZENSTACK_CLI_VERSION__).
♻️ Duplicate comments (3)
packages/runtime/test/policy/client-extensions.test.ts (3)
56-62: Same rename; all good.
87-93: Same rename; all good.
118-124: Same rename; all good.
🧹 Nitpick comments (34)
packages/language/test/mixin.test.ts (1)
109-123: Good: test covers new rule for relation fields in typesAccurately asserts the new validation. Consider adding variants (optional, list) to guard regressions.
packages/language/src/validators/typedef-validator.ts (1)
25-29: Correctly blocks relations in TypeDef fields; consider early return to cut error noiseThe check is right and should also catch list/optional forms. Optionally short-circuit after emitting the error to avoid cascading attribute errors on invalid nodes.
if (isDataModel(field.type.reference?.ref)) { accept('error', 'Type field cannot be a relation', { node: field.type, }); + return; }packages/sdk/src/ts-schema-generator.ts (1)
829-836: Constrainnameto a string literal; otherwise fall backEnsure the returned key only uses a string literal; other literal types (number/boolean) should not be accepted for constraint names.
- private getCompoundUniqueKey(attr: DataModelAttribute, fieldNames: string[]) { - const nameArg = attr.args.find((arg) => arg.$resolvedParam.name === 'name'); - if (nameArg && isLiteralExpr(nameArg.value)) { - return nameArg.value.value as string; - } else { - return fieldNames.join('_'); - } - } + private getCompoundUniqueKey(attr: DataModelAttribute, fieldNames: string[]) { + const nameArg = attr.args.find((arg) => arg.$resolvedParam.name === 'name'); + if (nameArg && isLiteralExpr(nameArg.value)) { + const lit = nameArg.value as LiteralExpr; + if (lit.$type === 'StringLiteral' && typeof lit.value === 'string' && lit.value.trim().length > 0) { + return lit.value; + } + } + return fieldNames.join('_'); + }packages/runtime/test/policy/client-extensions.test.ts (2)
25-31: Renamed plugin id looks good; consider deduping boilerplate.OK to shorten the id to "queryOverride". If you want to reduce repetition across tests, extract a small factory for this extension and call it here.
Apply this diff locally and add the helper (see snippet below):
- const ext = definePlugin({ - id: 'queryOverride', - onQuery: async ({ args, proceed }: any) => { - args = args ?? {}; - args.where = { ...args.where, y: { lt: 300 } }; - return proceed(args); - }, - }); + const ext = makeQueryOverrideExt(300);Add once near the top of the file:
function makeQueryOverrideExt(threshold: number) { return definePlugin({ id: 'queryOverride', onQuery: async ({ args, proceed }: any) => { args = args ?? {}; args.where = { ...args.where, y: { lt: threshold } }; return proceed(args); }, }); }
147-155: Non-mutating result mutation for clarity/safety.Mutating the array returned by proceed in-place can surprise if downstream relies on immutability. Return a mapped copy instead.
const ext = definePlugin({ id: 'resultMutation', onQuery: async ({ args, proceed }: any) => { - const r: any = await proceed(args); - for (let i = 0; i < r.length; i++) { - r[i].value = r[i].value + 1; - } - return r; + const r: any[] = await proceed(args); + return r.map((item) => ({ ...item, value: item.value + 1 })); }, });packages/runtime/test/policy/multi-field-unique.test.ts (1)
39-41: Add a brief comment to clarify why the first update succeeds.The transition “x:1 -> x:2” passes because deny(update, x > 1) evaluates on the pre-update state (x=1). A short comment will help future readers.
- await expect(db.model.update({ where: { a_b: { a: 'a1', b: 'b1' } }, data: { x: 2 } })).toResolveTruthy(); + // Deny applies to the pre-update row (x == 1), so this update is allowed. + await expect(db.model.update({ where: { a_b: { a: 'a1', b: 'b1' } }, data: { x: 2 } })).toResolveTruthy();packages/runtime/test/policy/mixin.test.ts (1)
84-91: Positive path for user id=1 is correct.Optionally, add a quick read-back assertion to verify the relation linkage materialized as expected.
await expect( dbUser1.user.update({ where: { id: 1 }, data: { profile: { connect: { id: profile.id } } } }), ).toResolveTruthy(); + await expect(db.user.findUnique({ where: { id: 1 } })).resolves.toMatchObject({ + profile: expect.objectContaining({ id: profile.id }), + });TODO.md (1)
10-10: Track REPL with an issue IDAdd an issue link or acceptance criteria (scope, UX, minimal commands) to avoid this staying stale.
I can draft a minimal spec and open an issue if helpful.
packages/cli/package.json (2)
38-38: Check ESM/CJS interop for mixpanel in an ESM CLI.The package is historically CJS-first; with
"type": "module"and tsup bundling, confirm import shape works at runtime (no default/named import mismatch). If needed, enable cjsInterop in tsup or adapt imports.If you hit interop issues, consider:
- import mixpanel from 'mixpanel' + import pkg from 'mixpanel' + const mixpanel = pkg as unknown as { init: Function }or set tsup option:
- // tsup.config.ts + export default defineConfig({ + splitting: false, + cjsInterop: true, + })
31-43: Telemetry dependency: ensure hard-off in CI and on missing token.Confirm the CLI fully no-ops telemetry when TELEMETRY_TRACKING_TOKEN is absent or DO_NOT_TRACK/CI is set, and that importing mixpanel is deferred so builds don’t fail without a token or network.
- Lazy-load telemetry module after config gating.
- Guard all calls behind a single isTelemetryEnabled() check.
- Make network timeouts short and fire-and-forget to avoid slowing commands.
packages/cli/src/actions/info.ts (1)
60-63: Good: skip private packages; minor type-safety tweak.Return
undefinedfor private is fine; prefer safe version extraction to avoid accidentalundefined as string.Apply:
- if (depPkgJson.private) { - return undefined; - } - return { pkg, version: depPkgJson.version as string }; + if (depPkgJson.private) { + return undefined; + } + const version = typeof depPkgJson.version === 'string' ? depPkgJson.version : undefined; + return { pkg, version };scripts/bump-version.ts (1)
6-7: Exclude list: clarify intent and consider glob/relative matching.Hard-coding a single file path is OK short-term; consider supporting globs (e.g., VSCode will be excluded regardless of subpath) or making entries relative to workspace root explicitly.
Example:
-const excludes = ['packages/ide/vscode/package.json']; +// Paths relative to workspace root; supports globs +const excludes = ['packages/ide/vscode/package.json'];(See next comment for cross-platform matching fix.)
packages/cli/src/utils/is-wsl.ts (1)
1-18: Solid WSL detection; consider env fast-path and memoization.Optional: check
WSL_DISTRO_NAME/WSL_INTEROPbefore disk I/O and cache the result if called repeatedly.Apply:
export const isWsl = () => { if (process.platform !== 'linux') { return false; } + // Fast-path via environment + if (process.env.WSL_DISTRO_NAME || process.env.WSL_INTEROP) { + return true; + } + if (os.release().toLowerCase().includes('microsoft')) { return true; } try { return fs.readFileSync('/proc/version', 'utf8').toLowerCase().includes('microsoft'); } catch { return false; } };packages/cli/src/utils/is-docker.ts (2)
24-31: Use nullish coalescing assignment now (Node 14+), simplify caching.Cleaner and avoids the extra branch.
export default function isDocker() { - // TODO: Use `??=` when targeting Node.js 16. - if (isDockerCached === undefined) { - isDockerCached = hasDockerEnv() || hasDockerCGroup(); - } - - return isDockerCached; + return (isDockerCached ??= hasDockerEnv() || hasDockerCGroup()); }
16-22: Consider cgroup v2 and non-docker runtimes.Searching only for 'docker' in /proc/self/cgroup can miss containerd/podman/k8s. If you intend generic container detection, either broaden matches (e.g., 'kubepods', 'containerd', 'podman') here or delegate to is-container which composes broader checks.
README.md (2)
32-36: Fix list indentation to satisfy MD007.markdownlint flags these bullets for inconsistent indentation. Align them with the surrounding list level.
- - Built-in access control and data validation (coming soon) + - Built-in access control and data validation (coming soon) - - Automatic CRUD web APIs with adapters for popular frameworks (coming soon) + - Automatic CRUD web APIs with adapters for popular frameworks (coming soon) - - Automatic [TanStack Query](https://github.com/TanStack/query) hooks for easy CRUD from the frontend (coming soon) + - Automatic [TanStack Query](https://github.com/TanStack/query) hooks for easy CRUD from the frontend (coming soon)
85-87: Minor wording/format: convert to sentence and avoid redundant header.You already have a “Documentation” header; use a concise sentence and link.
-# Documentation - -Please visit the [doc site](https://zenstack.dev/docs/3.x/) for detailed documentation. +See the [documentation site](https://zenstack.dev/docs/3.x/) for details.packages/cli/src/utils/version-utils.ts (2)
20-33: Skip logs in CI/silent contexts and provide upgrade hint.Avoid noisy output in CI; also include a quick upgrade command for convenience.
-export async function checkNewVersion() { +export async function checkNewVersion({ silent }: { silent?: boolean } = {}) { const currVersion = getVersion(); let latestVersion: string; try { latestVersion = await getLatestVersion(); } catch { // noop return; } - if (latestVersion && currVersion && semver.gt(latestVersion, currVersion)) { - console.log(`A newer version ${colors.cyan(latestVersion)} is available.`); + if (latestVersion && currVersion && semver.gt(latestVersion, currVersion)) { + if (silent || (await import('../utils/is-ci.js')).isInCi) return; + console.log( + `A newer version ${colors.cyan(latestVersion)} is available. ` + + `Update with: ${colors.cyan('npm i -g @zenstackhq/cli@next')}` + ); } }
49-50: Prefer a more precise error to aid debugging.Include status and text in the thrown error for easier diagnosis.
- throw new Error('invalid npm registry response'); + throw new Error(`Invalid npm registry response for tag "${VERSION_CHECK_TAG}"`);packages/cli/src/actions/generate.ts (3)
15-16: Make silent optional with a default.Keeps the action API backward-compatible for any internal callers not yet passing silent.
-type Options = { +type Options = { schema?: string; output?: string; - silent: boolean; + silent?: boolean; };
89-94: Remove unnecessary reverse() call.reverse() on a single-item array is a no-op.
- const defaultPlugins = [corePlugins['typescript']].reverse(); + const defaultPlugins = [corePlugins['typescript']];
103-118: Consider fail-fast or clear exit status on plugin errors.Currently errors are logged but generation proceeds; this can mask failures in CI. Provide a flag or aggregate and exit non-zero if any plugin fails.
- try { + try { await cliPlugin.generate({ schemaFile, model, defaultOutputPath: outputPath, pluginOptions, }); spinner?.succeed(); } catch (err) { spinner?.fail(); console.error(err); + // Optionally: rethrow or record failure + // if (process.env.CI || options.silent) throw err; }packages/create-zenstack/src/index.ts (1)
125-129: Default stdio ‘ignore’ hides useful output.For long-running steps (generate/db push), you’re already inheriting in the patch above. Keep default as-is, but ensure all user-facing ops pass
'inherit'.packages/cli/src/utils/machine-id-utils.ts (2)
21-27: Use a safer hasOwnProperty check.Avoid calling env.hasOwnProperty directly.
Apply:
- if (process.arch === 'ia32' && process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432')) { + if (process.arch === 'ia32' && Object.prototype.hasOwnProperty.call(process.env, 'PROCESSOR_ARCHITEW6432')) {
62-76: Annotate return type and consider memoizing.Add explicit return type; optionally cache to avoid repeated shelling out.
Apply:
-export function getMachineId() { +export function getMachineId(): string {If you’d like, I can add a simple in-memory cache.
packages/ide/vscode/package.json (1)
10-11: Update repository URL to the v3 repo.Apply:
- "url": "https://github.com/zenstackhq/zenstack" + "url": "https://github.com/zenstackhq/zenstack-v3"packages/ide/vscode/src/extension/main.ts (1)
50-55: Avoid local variable shadowing of the module-level client.Rename the local const to reduce confusion.
- const client = new LanguageClient('zmodel-v3', 'ZenStack Model V3', serverOptions, clientOptions); + const languageClient = new LanguageClient('zmodel-v3', 'ZenStack Model V3', serverOptions, clientOptions); - client.start(); - return client; + languageClient.start(); + return languageClient;packages/cli/src/constants.ts (1)
1-2: Add runtime guard for unreplaced telemetry token
Import and useIS_PLACEHOLDER_TELEMETRY_TOKENto skip initializing Mixpanel when the token remains the build-time placeholder:--- packages/cli/src/constants.ts @@ export const TELEMETRY_TRACKING_TOKEN = '<TELEMETRY_TRACKING_TOKEN>'; +export const IS_PLACEHOLDER_TELEMETRY_TOKEN = + TELEMETRY_TRACKING_TOKEN === '<TELEMETRY_TRACKING_TOKEN>';--- packages/cli/src/telemetry.ts @@ -import { TELEMETRY_TRACKING_TOKEN } from './constants'; +import { + TELEMETRY_TRACKING_TOKEN, + IS_PLACEHOLDER_TELEMETRY_TOKEN, +} from './constants'; @@ constructor() { - if (process.env['DO_NOT_TRACK'] !== '1' && TELEMETRY_TRACKING_TOKEN) { + if (process.env['DO_NOT_TRACK'] === '1' || IS_PLACEHOLDER_TELEMETRY_TOKEN) { + return; + } + if (TELEMETRY_TRACKING_TOKEN) { this.mixpanel = init(TELEMETRY_TRACKING_TOKEN, { geolocate: true, });packages/cli/src/utils/is-container.ts (2)
6-14: Use a cheaper existence check and explicit return type.Replace statSync with accessSync (less metadata work) and declare the function’s return type for clarity.
-const hasContainerEnv = () => { +const hasContainerEnv = (): boolean => { try { - fs.statSync('/run/.containerenv'); + fs.accessSync('/run/.containerenv', fs.constants.F_OK); return true; } catch { return false; } };
16-23: Consider nullish-coalescing assignment for caching (when target allows).If your Node target supports it, simplify caching with ??= to match is-docker’s TODO.
-export function isInContainer() { - // TODO: Use `??=` when targeting Node.js 16. - if (cachedResult === undefined) { - cachedResult = hasContainerEnv() || isDocker(); - } - - return cachedResult; -} +export function isInContainer(): boolean { + // TODO: Use `??=` when targeting supported Node runtime. + cachedResult ??= hasContainerEnv() || isDocker(); + return cachedResult; +}packages/cli/src/index.ts (1)
145-149: Version-check gate works, but consider centralizing the option.PreAction reads versionCheck from each command. If any command forgets to add the option, the check always runs. If that’s intended, fine; otherwise, define it at the root command so all subcommands inherit it.
Would you like a small diff to add the option at the root so it’s available everywhere?
packages/cli/src/telemetry.ts (3)
30-45: Avoid eager, costly env/machine detection when telemetry is disabled.These fields run at import time even if DO_NOT_TRACK=1 or token is missing, adding overhead. Lazily compute them only when mixpanel is active.
Option A (minimal): move initializations into constructor after mixpanel is created.
Option B (clean): convert to getters that compute on first access and cache the result. I can provide a diff if you prefer B.
83-88: Error stacks may leak sensitive paths; consider trimming or hashing.Sending full stack traces can expose local paths/usernames. Prefer message-only or sanitized frames.
31-45: Nit: uniform casing for sessionId.Rename sessionid -> sessionId for consistency with common casing and payload field names.
- private readonly sessionid = randomUUID(); + private readonly sessionId = randomUUID(); @@ - session: this.sessionid, + session: this.sessionId,
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
packages/language/src/generated/module.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (42)
.github/workflows/build-test.yml(1 hunks)README.md(3 hunks)TODO.md(1 hunks)package.json(1 hunks)packages/cli/package.json(2 hunks)packages/cli/src/actions/generate.ts(5 hunks)packages/cli/src/actions/info.ts(1 hunks)packages/cli/src/constants.ts(1 hunks)packages/cli/src/index.ts(6 hunks)packages/cli/src/telemetry.ts(1 hunks)packages/cli/src/utils/is-ci.ts(1 hunks)packages/cli/src/utils/is-container.ts(1 hunks)packages/cli/src/utils/is-docker.ts(1 hunks)packages/cli/src/utils/is-wsl.ts(1 hunks)packages/cli/src/utils/machine-id-utils.ts(1 hunks)packages/cli/src/utils/version-utils.ts(2 hunks)packages/cli/tsup.config.ts(2 hunks)packages/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/create-zenstack/src/index.ts(3 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/README.md(1 hunks)packages/ide/vscode/package.json(4 hunks)packages/ide/vscode/src/extension/main.ts(1 hunks)packages/language/package.json(1 hunks)packages/language/src/validators/typedef-validator.ts(2 hunks)packages/language/test/mixin.test.ts(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/test/policy/client-extensions.test.ts(5 hunks)packages/runtime/test/policy/mixin.test.ts(1 hunks)packages/runtime/test/policy/multi-field-unique.test.ts(1 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/ts-schema-generator.ts(2 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 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)scripts/bump-version.ts(2 hunks)tests/e2e/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/cli/src/utils/is-wsl.tspackages/language/test/mixin.test.tspackages/vitest-config/package.jsonpackages/common-helpers/package.jsonpackages/runtime/package.jsonpackages/typescript-config/package.jsonpackages/zod/package.jsonpackages/cli/src/utils/is-ci.tspackages/eslint-config/package.jsonpackages/create-zenstack/package.jsonpackages/sdk/package.jsonpackages/sdk/src/ts-schema-generator.tspackages/cli/src/constants.tspackages/cli/src/utils/is-docker.tspackages/testtools/package.jsonsamples/blog/package.jsontests/e2e/package.jsonpackages/dialects/sql.js/package.jsonpackages/tanstack-query/package.jsonpackages/cli/src/utils/machine-id-utils.tspackages/language/package.jsonpackages/cli/src/actions/info.tspackages/cli/src/utils/version-utils.tspackages/cli/src/telemetry.tspackages/runtime/test/policy/multi-field-unique.test.tspackages/cli/package.jsonpackages/language/src/validators/typedef-validator.tspackages/runtime/test/policy/mixin.test.tspackages/cli/src/actions/generate.tspackages/runtime/test/policy/client-extensions.test.tspackages/cli/tsup.config.tspackages/cli/src/index.tspackages/cli/src/utils/is-container.tspackages/ide/vscode/src/extension/main.tspackages/ide/vscode/package.jsonpackages/create-zenstack/src/index.tspackages/ide/vscode/README.md
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
🧠 Learnings (4)
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)
Applied to files:
packages/language/package.jsonpackages/cli/src/actions/generate.tsREADME.mdpackages/create-zenstack/src/index.ts
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Schema is used to instantiate `ZenStackClient` with type-safe CRUD operations
Applied to files:
packages/cli/src/actions/generate.ts
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Kysely-Based ORM: V3 uses Kysely as query builder instead of Prisma runtime dependency
Applied to files:
README.md
📚 Learning: 2025-08-04T08:43:33.161Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-04T08:43:33.161Z
Learning: Schema-first approach with ZModel DSL extension of Prisma schema language
Applied to files:
README.md
🧬 Code graph analysis (11)
packages/language/test/mixin.test.ts (1)
packages/language/test/utils.ts (1)
loadSchemaWithError(18-30)
packages/sdk/src/ts-schema-generator.ts (1)
packages/language/src/generated/ast.ts (3)
DataModelAttribute(395-400)DataModelAttribute(402-402)isLiteralExpr(133-135)
packages/cli/src/utils/machine-id-utils.ts (1)
packages/cli/src/utils/exec-utils.ts (1)
execSync(6-15)
packages/cli/src/telemetry.ts (7)
packages/cli/src/utils/machine-id-utils.ts (1)
getMachineId(62-76)packages/cli/src/utils/version-utils.ts (1)
getVersion(10-18)packages/cli/src/utils/is-docker.ts (1)
isDocker(24-31)packages/cli/src/utils/is-wsl.ts (1)
isWsl(4-18)packages/cli/src/utils/is-container.ts (1)
isInContainer(16-23)packages/cli/src/utils/is-ci.ts (1)
isInCi(2-5)packages/cli/src/constants.ts (1)
TELEMETRY_TRACKING_TOKEN(2-2)
packages/runtime/test/policy/multi-field-unique.test.ts (2)
packages/runtime/test/policy/utils.ts (1)
createPolicyTestClient(14-25)packages/runtime/src/client/errors.ts (1)
QueryError(13-17)
packages/language/src/validators/typedef-validator.ts (1)
packages/language/src/generated/ast.ts (1)
isDataModel(391-393)
packages/runtime/test/policy/mixin.test.ts (1)
packages/runtime/test/policy/utils.ts (1)
createPolicyTestClient(14-25)
packages/cli/src/actions/generate.ts (1)
packages/language/src/generated/ast.ts (2)
Model(559-563)Model(565-565)
packages/cli/src/index.ts (3)
packages/cli/src/telemetry.ts (1)
telemetry(139-139)packages/cli/src/utils/version-utils.ts (1)
checkNewVersion(20-33)packages/cli/src/cli-error.ts (1)
CliError(4-4)
packages/cli/src/utils/is-container.ts (1)
packages/cli/src/utils/is-docker.ts (1)
isDocker(24-31)
packages/create-zenstack/src/index.ts (1)
packages/cli/src/utils/exec-utils.ts (1)
execSync(6-15)
🪛 markdownlint-cli2 (0.17.2)
README.md
32-32: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
33-33: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
34-34: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
35-35: Unordered list indentation
Expected: 6; Actual: 12
(MD007, ul-indent)
36-36: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
50-50: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
51-51: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
52-52: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
85-85: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
86-86: Unordered list indentation
Expected: 4; Actual: 8
(MD007, ul-indent)
87-87: Unordered list indentation
Expected: 4; Actual: 8
(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). (2)
- GitHub Check: build-test (20.x)
- GitHub Check: claude-review
🔇 Additional comments (47)
packages/language/src/validators/typedef-validator.ts (1)
2-2: Import style is correctBrings in isDataModel at runtime while keeping DataField/TypeDef as type-only imports. No issues.
packages/runtime/test/policy/multi-field-unique.test.ts (10)
33-36: Duplicate create correctly asserts unique violation.Good use of QueryError to distinguish DB constraint from policy rejection.
42-43: Follow-up NotFound assertion is appropriate.After x becomes 2, the subsequent update fails due to policy filtering or deny, returning NotFound.
61-71: Named composite unique path validated well.Solid coverage for the named constraint alias resolution.
96-103: Nested create coverage is strong; unique vs. policy paths are well distinguished.
117-118: Count assertion helps detect accidental duplicates.Nice invariant check.
119-133: Good exercise of connectOrCreate on composite unique.Covers the “create when missing” branch under allow rules.
134-147: Policy-rejected connectOrCreate is correctly asserted.Covers the “reject on policy” branch; complements the previous success path.
148-164: Update-by-composite and verification read are clear.End-to-end assertion on the updated value is good.
165-178: Delete-by-composite with post-condition count is tidy.Completes the lifecycle coverage.
9-15: CWD restore guard is sufficient; no other process.chdir usages detected.packages/runtime/test/policy/mixin.test.ts (5)
6-27: Schemas concisely model the mixin use-case.The mixin-based policy composition reads clearly and sets up the relation semantics well.
30-39: Unauthorized connect attempts correctly assert NotFound.Good to assert both Profile→User and User→Profile directions.
40-47: Authorized updates for id=1 validate the mixin rule.Both directions succeeding confirms symmetry.
50-71: Second schema variant broadens coverage.Switching “all” to User and restricting Profile via mixin exercises a realistic permutation.
74-83: Negative path for user id=2 is correct.packages/zod/package.json (1)
3-3: Version bump to beta looks goodNo API/config changes here; aligns with the repo-wide pre-release.
packages/tanstack-query/package.json (1)
3-3: Version bumped to 3.0.0-beta.1 — OKMatches workspace transition; no other changes.
packages/create-zenstack/package.json (2)
3-3: Version bump acknowledgedNo functional changes detected.
17-19: bin/cli verified as present and executable
bin/cliexists post-build, includes a POSIX shebang, has its exec bit set, and is correctly packaged in the npm tarball.samples/blog/package.json (2)
3-3: Sample app version bump — OKScripts already use the new
zenCLI; deps look consistent.
7-11: Confirm CLI availability during sample scriptsSince
zenis in devDependencies, ensure consumers run scripts via the repo workspace (pnpm -w) or have PATH resolution; otherwise document it.tests/e2e/package.json (1)
3-3: Bump verified—no alpha refs in package.json, lockfile, or snapshots.packages/cli/package.json (1)
6-6: CLI version bump: OK.Aligned with the monorepo beta transition.
packages/runtime/package.json (1)
3-3: Runtime version bump: OK.No other manifest changes; looks safe.
packages/eslint-config/package.json (1)
3-3: eslint-config version bump: OK.Private package; no publishing risk.
packages/dialects/sql.js/package.json (1)
3-3: Dialect package version bump: OK.Exports unchanged; no ABI concerns.
packages/sdk/package.json (1)
3-3: Version bump to 3.0.0-beta.1: LGTM; verify dependents aren’t pinned to alpha.
No issues here. Please double-check that any consumers within the monorepo don’t specify alpha ranges for @zenstackhq/sdk.Run the root-level verification script I shared on the root package.json comment to ensure no alpha remnants or non-workspace pins remain.
packages/language/package.json (1)
4-4: Bump to 3.0.0-beta.1: LGTM.
No changes to exports or scripts; version-only update is fine. Ensure downstream packages relying on @zenstackhq/language remain on workspace:*.packages/common-helpers/package.json (1)
3-3: Version-only change: LGTM.
Nothing else altered. Confirm no consumers pin a mismatched pre-release range for @zenstackhq/common-helpers.packages/typescript-config/package.json (1)
3-3: Private package version bump: LGTM.
Safe to keep versions aligned even for private packages; no action needed.packages/vitest-config/package.json (1)
4-4: Version bump looks good.No issues spotted with the alpha→beta transition here.
packages/testtools/package.json (1)
3-3: Version bump approved.Consistent with the workspace-wide beta.1 move.
packages/cli/src/actions/generate.ts (2)
31-45: LGTM: silent mode correctly gates UX-only output.The success banner, example snippet, and doc link are suppressed when silent=true.
29-29: All runPlugins calls include the new options parameter
Only one invocation exists (generate.ts L29), and it already passesoptions. No stale call sites remain.packages/create-zenstack/src/index.ts (2)
64-66: VS Code bootstrap looks good and matches the new language id.Creates .vscode with zmodel-v3 association and recommends zenstack-v3 extension. Solid.
Also applies to: 136-151
3-3: Type import for StdioOptions is correct.Matches Node’s child_process types and fits the updated runCommand signature.
packages/cli/src/utils/machine-id-utils.ts (2)
1-1: Confirm license compliance for borrowed implementation.You reference an external repo. Ensure its license (likely MIT) is honored with proper attribution/copyright header.
Do you want me to add an MIT-style header here if confirmed?
12-19: LGTM overall: robust cross-platform retrieval with hashing and fallbacks.Good error handling and normalization; avoiding the shared exec wrapper is appropriate to keep output quiet.
Also applies to: 33-60, 67-73
packages/ide/vscode/package.json (1)
60-67: Language id transition to zmodel-v3 is consistent.IDs, grammar mapping, and activation event align with the new config scaffolding.
Also applies to: 75-80, 84-85
packages/ide/vscode/src/extension/main.ts (1)
46-47: Document selector and activation events aligned
Contributes.languages[].id and activationEvents usezmodel-v3as expected. Optionally add{ scheme: 'file' }to the document selector to exclude untitled/virtual documents.packages/ide/vscode/README.md (4)
21-35: Side-by-side mapping is clear.Instructions for files.associations are accurate and concise.
37-44: Nice: format-on-save guidance.Clear and minimal; no changes needed.
55-60: Links updated to v3 look good.
48-51: No update required: the formatter identifier in the README (zenstack.zenstack-v3) matches thepublisher.nameinpackage.json.packages/cli/tsup.config.ts (1)
15-29: Align read/write paths & replaceAll tokens
- Compute
const outPath = path.join(__dirname, file)and use it for both reading and writing- Use
fs.readFileSync(outPath, 'utf8')&fs.writeFileSync(outPath, 'utf8')(prefer'utf8'over'utf-8')- Replace all occurrences with
content.replaceAll(...)(or a global regex) and throw if no replacement occurredApply:
- for (const file of filesToProcess) { - console.log(`Processing ${file} for telemetry token...`); - const content = fs.readFileSync(path.join(__dirname, file), 'utf-8'); - const updatedContent = content.replace( - '<TELEMETRY_TRACKING_TOKEN>', - process.env['TELEMETRY_TRACKING_TOKEN'], - ); - fs.writeFileSync(file, updatedContent, 'utf-8'); - } + for (const file of filesToProcess) { + const outPath = path.join(__dirname, file); + console.log(`Processing ${outPath} for telemetry token...`); + const content = fs.readFileSync(outPath, 'utf8'); + const token = process.env['TELEMETRY_TRACKING_TOKEN']!; + const updatedContent = content.replaceAll('<TELEMETRY_TRACKING_TOKEN>', token); + if (updatedContent === content) { + throw new Error(`Telemetry token placeholder not found in ${outPath}`); + } + fs.writeFileSync(outPath, updatedContent, 'utf8'); + }Ensure you build
packages/clibefore running this post-build step and verify no placeholders remain underpackages/cli/dist.packages/cli/src/telemetry.ts (1)
1-12: Verify Mixpanel import shape in telemetry.ts
Ensure theinitfunction andMixpaneltype align with the installedmixpanelSDK’s entry points (ESM vs CJS) to prevent runtime import errors.
File: packages/cli/src/telemetry.ts lines 1–12
* chore: update vscode engine version * update * update * update
|
Claude finished @ymc9's task —— View job Code Review for v3.0.0-beta.1 ReleaseI've completed my comprehensive review of this significant release PR. Here's my detailed analysis:
Overall AssessmentThis is a well-structured release with significant new features and improvements. The code quality is high overall, but there are several important considerations below. Code Quality & Best Practices ✅Strengths:
Security Considerations
|
* chore: update vscode engine version * update * update * update * update
|
Claude finished @ymc9's task —— View job Code Review for v3.0.0-beta.1 ReleaseI've completed my comprehensive review of this significant release PR. Here's my detailed analysis:
Overall AssessmentThis is a well-structured release with significant new features and improvements. The code quality is high overall, but there are several important considerations below. Code Quality & Best Practices ✅Strengths:
Security Considerations
|
Summary by CodeRabbit
New Features
Improvements
Documentation
Chores