merge dev to main (v3.0.0-beta.2) #234
Conversation
* chore: fix telemetry env var for publish CI * update test and config * update * remove unused settings
Update packages/runtime/src/client/crud/operations/find.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
WalkthroughIntroduces telemetry env vars in release workflow, updates VSCode settings and extension grammar, bumps versions across packages, restructures Vitest config, skips two CLI test suites, and modifies runtime find/validation logic to add a findOne flag, adjust validateFindArgs signature/behavior, and update related tests and error exports. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 PR Review AnalysisI'll conduct a comprehensive review of this dev → main merge PR, focusing on: Review Checklist:
🔍 Detailed Review Findings✅ Positive Changes1. FindFirst Optimization (
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges development changes to main, primarily focusing on version bumps and test configuration improvements. The changes update all packages from version 3.0.0-beta.1 to 3.0.0-beta.2 and refactor the Vitest configuration from a workspace-based setup to a project-based approach.
Key changes include:
- Version bump across all packages from beta.1 to beta.2
- Vitest configuration refactoring from workspace to project-based setup
- Input validation improvements for findFirst operations to enforce take=1 limit
- Test stability improvements by skipping timeout-prone CLI tests
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.workspace.ts | Removed workspace configuration file |
| vitest.config.ts | Added new project-based Vitest configuration |
| vitest.base.config.ts | Removed base configuration file |
| Multiple package.json files | Version bumps from 3.0.0-beta.1 to 3.0.0-beta.2 |
| packages/runtime/src/client/crud/validator.ts | Enhanced input validation for findFirst operations |
| packages/runtime/src/client/crud/operations/find.ts | Improved findFirst logic to enforce take=1 |
| Test files | Updated tests and skipped timeout-prone CLI tests |
| .vscode/settings.json | Removed Vitest workspace configuration |
| .github/workflows/publish-release.yml | Added telemetry environment variables |
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: 2
🧹 Nitpick comments (9)
.github/workflows/publish-release.yml (1)
12-14: Optional: Enable provenance for npm publish.If publish-all supports npm provenance, grant OIDC and set the env flag.
Apply:
permissions: - contents: write + contents: write + id-token: write @@ - name: Publish packages run: pnpm run publish-all env: NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }} + NPM_CONFIG_PROVENANCE: 'true'packages/cli/test/migrate.test.ts (1)
12-13: Prefer conditional skip to retain local coverage and only skip in CI.Avoid permanently skipping the entire suite. Gate it by CI and keep it runnable locally; also consider per-test longer timeouts instead of blanket skip.
Apply within this hunk:
-// skip due to timeout in CI -describe.skip('CLI migrate commands test', () => { +// run locally; skip in CI to avoid timeouts +describe.runIf(!process.env.CI)('CLI migrate commands test', () => {Follow-ups:
- Increase timeouts for slow tests (e.g., it('...', { timeout: 120_000 }, ...)).
- Add a scheduled GitHub Actions job to run this suite nightly with relaxed timeouts to preserve signal.
.vscode/settings.json (1)
1-1: Empty settings file—consider deleting to reduce repo noise.packages/cli/test/init.test.ts (1)
7-8: Avoid permanent skip; conditionally skip in CI and raise timeoutSkip only on CI and keep the suite runnable locally. Also raise the timeout to reduce flakes.
-// skipping due to timeout during CI -describe.skip('Cli init command tests', () => { +const inCI = process.env.CI === 'true'; +// Skip only on CI, run locally with a higher timeout +const suite = inCI ? describe.skip : describe; + +suite('Cli init command tests', { timeout: 120_000 }, () => {vitest.config.ts (1)
3-7: Set CI-friendly timeouts to reduce flakes and avoid blanket skipsAdd higher timeouts for CI so long-running suites don’t need to be skipped.
export default defineConfig({ test: { - projects: ['packages/*'], + projects: ['packages/*'], + testTimeout: process.env.CI ? 120_000 : 30_000, + hookTimeout: 120_000, }, });packages/runtime/src/client/crud/validator.ts (1)
196-229: Return parsed data and defaulttake = 1forfindOne
z.literal(1).optional()enforcestake === 1but doesn’t inject it when args are omitted. Updatevalidate()to returnparsed.dataso Zod defaults flow, and inmakeFindSchemawrap the non-unique,findOneschema with.optional().default({ take: 1 }). (Operation layer’sparsedArgs.take = 1fallback remains intact.)Apply both diffs:
@@ packages/runtime/src/client/crud/validator.ts - const { error } = schema.safeParse(args); - if (error) { - throw new InputValidationError(`Invalid ${operation} args: ${error.message}`, error); - } - return args as T; + const parsed = schema.safeParse(args); + if (!parsed.success) { + throw new InputValidationError( + `Invalid ${operation} args: ${parsed.error.message}`, + parsed.error + ); + } + return parsed.data as T;@@ packages/runtime/src/client/crud/validator.ts - if (!options.unique) { - result = result.optional(); - } + if (!options.unique) { + // For findOne, default to { take: 1 } even when args are omitted + result = options.findOne + ? (result as any).optional().default({ take: 1 }) + : result.optional(); + }packages/runtime/src/client/crud/operations/find.ts (1)
13-24: Avoid mutating validated args; tighten typing forparsedArgsTwo tweaks:
- Don’t mutate the object returned by the validator; it could become readonly in the future. Build a new args object for the read path.
- Cast/annotate to
FindArgs<Schema, GetModels<Schema>, true> | undefinedsotakeis type-safe and you don’t rely on implicitany.Proposed diff:
- let parsedArgs = validateArgs - ? this.inputValidator.validateFindArgs(this.model, normalizedArgs, { - unique: operation === 'findUnique', - findOne, - }) - : (normalizedArgs as FindArgs<Schema, GetModels<Schema>, true> | undefined); - - if (findOne) { - // ensure "limit 1" - parsedArgs = parsedArgs ?? {}; - parsedArgs.take = 1; - } + const parsedArgs = validateArgs + ? (this.inputValidator.validateFindArgs(this.model, normalizedArgs, { + unique: operation === 'findUnique', + findOne, + }) as FindArgs<Schema, GetModels<Schema>, true> | undefined) + : (normalizedArgs as FindArgs<Schema, GetModels<Schema>, true> | undefined); + + // ensure "limit 1" for single-result ops without mutating validated input + const argsForRead = findOne ? { ...(parsedArgs ?? {}), take: 1 } : parsedArgs; @@ - const result = await this.read(this.client.$qb, this.model, parsedArgs); + const result = await this.read(this.client.$qb, this.model, argsForRead); @@ - const finalResult = findOne ? (result[0] ?? null) : result; + const finalResult = findOne ? (result[0] ?? null) : result;Also applies to: 27-27, 29-29
packages/runtime/test/client-api/computed-fields.test.ts (1)
76-79: Explicittake: 1is fine but redundant
findFirstis now enforced to single-row; this assertion still adds value, but consider removing the explicittakehere to test the default single-result path and keep one dedicated “take must be 1” test in find.test.ts.packages/runtime/test/client-api/find.test.ts (1)
57-60: Broaden validation coverage forfindFirst'stakeAdd cases for
take: 0andtake: -1to assert they reject withInputValidationError, and a case with notaketo assert it succeeds (implicitly 1).Apply:
// findFirst's take must be 1 await expect(client.user.findFirst({ take: 2 })).rejects.toThrow(InputValidationError); + await expect(client.user.findFirst({ take: 0 })).rejects.toThrow(InputValidationError); + await expect(client.user.findFirst({ take: -1 })).rejects.toThrow(InputValidationError); await expect(client.user.findFirst({ take: 1 })).toResolveTruthy(); + await expect(client.user.findFirst()).toResolveTruthy();
📜 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 selected for processing (28)
.github/workflows/publish-release.yml(1 hunks).vscode/settings.json(1 hunks)package.json(1 hunks)packages/cli/package.json(1 hunks)packages/cli/test/init.test.ts(1 hunks)packages/cli/test/migrate.test.ts(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(2 hunks)packages/language/package.json(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/src/client/crud/operations/find.ts(1 hunks)packages/runtime/src/client/crud/validator.ts(3 hunks)packages/runtime/test/client-api/computed-fields.test.ts(1 hunks)packages/runtime/test/client-api/find.test.ts(2 hunks)packages/sdk/package.json(1 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)tests/e2e/package.json(1 hunks)vitest.base.config.ts(0 hunks)vitest.config.ts(1 hunks)vitest.workspace.ts(0 hunks)
💤 Files with no reviewable changes (2)
- vitest.base.config.ts
- vitest.workspace.ts
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/sdk/package.jsonsamples/blog/package.jsonpackages/common-helpers/package.jsonpackages/vitest-config/package.jsonpackages/typescript-config/package.jsonpackages/zod/package.jsonpackages/cli/package.jsonpackages/cli/test/init.test.tspackages/runtime/package.jsonpackages/testtools/package.jsonpackages/language/package.jsonpackages/eslint-config/package.jsonpackages/tanstack-query/package.jsonpackages/dialects/sql.js/package.jsonpackages/create-zenstack/package.jsonpackages/runtime/src/client/crud/operations/find.tspackages/cli/test/migrate.test.tspackages/runtime/src/client/crud/validator.tspackages/runtime/test/client-api/computed-fields.test.tspackages/ide/vscode/package.jsontests/e2e/package.jsonpackages/runtime/test/client-api/find.test.ts
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
E2E tests are in
tests/e2e/directory
Files:
tests/e2e/package.json
🧠 Learnings (1)
📚 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.json
🧬 Code graph analysis (3)
packages/runtime/src/client/crud/operations/find.ts (2)
packages/runtime/src/client/crud-types.ts (1)
FindArgs(646-662)packages/sdk/src/schema/schema.ts (1)
GetModels(108-108)
packages/runtime/src/client/crud/validator.ts (2)
packages/sdk/src/schema/schema.ts (1)
GetModels(108-108)packages/runtime/src/client/crud-types.ts (1)
FindArgs(646-662)
packages/runtime/test/client-api/find.test.ts (1)
packages/runtime/src/client/errors.ts (1)
InputValidationError(4-8)
⏰ 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: claude-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (22)
packages/vitest-config/package.json (1)
4-4: Version bump to 3.0.0-beta.2 looks good.No export map changes; safe to publish.
packages/language/package.json (1)
4-4: Version bump acknowledged.Downstream workspace dependents should resolve seamlessly.
packages/testtools/package.json (1)
3-3: Version bump looks good.No API surface or exports changed here.
samples/blog/package.json (1)
3-3: Sample version updated.Dependencies already reference workspace ranges; no further action.
packages/cli/package.json (1)
6-6: Version bump consistency verified. All 15 package.json files now use 3.0.0-beta.2, no 3.0.0-beta.1 references remain, all internal deps use workspace:*, and Vitest config migration is in place.packages/eslint-config/package.json (1)
3-3: LGTM: version bump only.Private package; safe metadata update.
packages/create-zenstack/package.json (1)
3-3: LGTM: version bump aligns with repo-wide release.No functional/package boundary changes.
package.json (1)
3-3: Bump looks good; verify all packages are aligned to 3.0.0-beta.2.Process substitution isn’t available here—use this instead:
#!/bin/bash set -euo pipefail target="3.0.0-beta.2" echo "Verifying all package.json versions == $target" find . -type f -name package.json \ -not -path '*/node_modules/*' \ -not -path '*/dist/*' \ -not -path '*/build/*' \ -not -path '*/.turbo/*' \ -print0 | while IFS= read -r -d '' f; do name=$(jq -r '.name // empty' "$f") version=$(jq -r '.version // empty' "$f") if [[ -n "$name" && -n "$version" && "$version" != "$target" ]]; then echo "Mismatch: $f -> $name@$version" exit 1 fi done echo "All package versions aligned to $target."packages/tanstack-query/package.json (1)
3-3: LGTM: version bump only.No impact on peer dependency constraints.
packages/zod/package.json (1)
3-3: LGTM: version bump only.No API/external-facing changes in this package.
packages/sdk/package.json (1)
3-3: Approve version bump. All @zenstackhq/* packages are at 3.0.0-beta.2; no functional changes—safe to merge..github/workflows/publish-release.yml (1)
8-10: Confirm intent: DO_NOT_TRACK=1 alongside TELEMETRY token.This combo disables many CLIs’ telemetry while enabling your own tracking via token. Confirm this is intended.
packages/common-helpers/package.json (1)
3-3: LGTM: version bump aligns with release.No API/surface changes.
packages/dialects/sql.js/package.json (1)
3-3: LGTM: version bump only.Ensure publish includes matching peer ranges; looks consistent.
packages/typescript-config/package.json (1)
3-3: LGTM: private package version bump.Harmless for a private config package; fine to keep in sync with the release train.
tests/e2e/package.json (1)
3-3: Version bump aligns with the repo release. LGTM.packages/runtime/package.json (1)
3-3: Runtime package version bump looks good.packages/ide/vscode/package.json (1)
4-4: Extension version bump is fine.packages/runtime/src/client/crud/validator.ts (1)
43-47: validateFindArgs usage verified; no action requiredpackages/runtime/src/client/crud/operations/find.ts (2)
10-11: Good consolidation of single-result ops into afindOneflagThis improves readability and centralizes behavior for
findFirst/findUnique.
29-29: Confirmreadalways returns an array
Double-check thatprotected async readinpackages/runtime/src/client/crud/operations/base.tsis both typed and implemented to always return an array of records—otherwise indexing intoresult[0]will break.packages/runtime/test/client-api/find.test.ts (1)
3-3: ImportingInputValidationErrorlooks correctMatches the new public export in
src/client/errors.ts.
| env: | ||
| TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }} | ||
| DO_NOT_TRACK: '1' |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Scope the secret to the step that needs it.
Placing TELEMETRY_TRACKING_TOKEN at the workflow level exposes it to all steps, including third‑party actions. Scope it to the publish step; keep DO_NOT_TRACK at the top if you want global effect.
Apply:
-env:
- TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }}
- DO_NOT_TRACK: '1'
+env:
+ DO_NOT_TRACK: '1'
@@
- name: Publish packages
run: pnpm run publish-all
env:
NODE_AUTH_TOKEN: ${{ secrets.NPM_TOKEN }}
+ TELEMETRY_TRACKING_TOKEN: ${{ secrets.TELEMETRY_TRACKING_TOKEN }}Also applies to: 48-52
🤖 Prompt for AI Agents
.github/workflows/publish-release.yml lines 8-10 (and similarly lines 48-52):
TELEMETRY_TRACKING_TOKEN is currently defined at the workflow/env level which
exposes it to all steps; remove TELEMETRY_TRACKING_TOKEN from the top-level env,
keep DO_NOT_TRACK there if you want global effect, and add
TELEMETRY_TRACKING_TOKEN as an env only on the publish step that needs it (set
to ${{ secrets.TELEMETRY_TRACKING_TOKEN }}), ensuring other steps and
third‑party actions cannot access the secret.
| "scopeName": "source.zmodel-v3", | ||
| "path": "./syntaxes/zmodel.tmLanguage.json" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Grammar scope rename: verify tmLanguage and references updated to match.
Ensure syntaxes/zmodel.tmLanguage.json uses the same scopeName and that no code still references "source.zmodel".
Run:
Expected:
- Printed scopeName equals "source.zmodel-v3".
- No matches for source.zmodel.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm tmLanguage scopeName matches
GRAMMAR_FILE="$(fd -a 'zmodel.tmLanguage.json' packages/ide/vscode | head -n1)"
echo "Grammar file: ${GRAMMAR_FILE}"
jq -r '.scopeName' "${GRAMMAR_FILE}"
test "$(jq -r '.scopeName' "${GRAMMAR_FILE}")" = "source.zmodel-v3"
# 2) Find remaining references to old scope
rg -nP '\bsource\.zmodel\b' -g '!**/node_modules/**'Length of output: 219
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Finding zmodel grammar file(s)..."
find . -type f -name 'zmodel.tmLanguage.json'
echo -e "\nChecking scopeName in found file(s)..."
for f in $(find . -type f -name 'zmodel.tmLanguage.json'); do
echo "$f: $(jq -r '.scopeName' "$f")"
done
echo -e "\nSearching for references to old scope 'source.zmodel'..."
rg -nP '\bsource\.zmodel\b' -g '!**/node_modules/**' || echo "No references found"Length of output: 557
Missing tmLanguage grammar file
The path packages/ide/vscode/syntaxes/zmodel.tmLanguage.json referenced in package.json doesn’t exist; add the grammar file (or correct its path), then confirm its scopeName is "source.zmodel-v3" and that no code still references "source.zmodel".
🤖 Prompt for AI Agents
In packages/ide/vscode/package.json around lines 78-79, the extension references
a missing grammar file at ./syntaxes/zmodel.tmLanguage.json and a scopeName
"source.zmodel-v3"; add the missing zmodel.tmLanguage.json file at
packages/ide/vscode/syntaxes with scopeName set to "source.zmodel-v3" (or update
the path in package.json to the correct existing file), and then search the
codebase to replace any remaining references to "source.zmodel" with
"source.zmodel-v3" so all references and the package.json entry are consistent.
Summary by CodeRabbit