Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Sep 4, 2025

Summary by CodeRabbit

  • New Features

    • sql.js package now exposes its manifest via module exports, allowing consumers to import package metadata.
  • Documentation

    • Added a note explaining the VSCode extension uses a different versioning scheme due to marketplace restrictions.
  • Chores

    • Bumped versions to 3.0.0-beta.3 across packages.
    • CI and release workflows now run an additional post-build step to finalize artifacts for more consistent releases.

ymc9 and others added 3 commits September 3, 2025 17:09
* chore: fix telemetry token

* update

* update

* try again

* update

* update

* update

* update

* update

* update

* update

* fix
Copilot AI review requested due to automatic review settings September 4, 2025 11:47
@coderabbitai
Copy link

coderabbitai bot commented Sep 4, 2025

Walkthrough

Adds a new CLI post-build script and updates CI workflows to invoke it. Moves telemetry token replacement from tsup’s onSuccess hook to the standalone script. Bumps package versions to 3.0.0-beta.3 across the repo. Updates sql.js package exports to include ./package.json. Minor doc note added.

Changes

Cohort / File(s) Summary
CI workflows
.github/workflows/build-test.yml, .github/workflows/publish-release.yml
Build step changed to a multi-line script running pnpm run build then pnpm tsx packages/cli/scripts/post-build.ts.
CLI build pipeline refactor
packages/cli/scripts/post-build.ts, packages/cli/tsup.config.ts
New post-build script replaces <TELEMETRY_TRACKING_TOKEN> in dist/index.{js,cjs} using env var; removed tsup onSuccess hook performing the same replacement.
Version bumps
package.json, packages/*/package.json, samples/blog/package.json, tests/e2e/package.json
Bumped versions from 3.0.0-beta.2 to 3.0.0-beta.3 across root, packages, sample, and tests.
Dialects export update
packages/dialects/sql.js/package.json
Added exports["./package.json"] entry for both import and require.
Docs
CLAUDE.md
Noted ide/vscode uses a different version due to VSCode marketplace pre-release restriction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Poem

A rabbit taps the build with care, 🐇
From tsup’s hook to scripts we share,
Tokens tucked where bundles live,
CI now knows the steps to give.
Versions hop to beta three—
Exports bloom for all to see.
Thump-thump, ship it joyfully!

✨ 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

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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 dev to main and upgrades the project version from 3.0.0-beta.2 to 3.0.0-beta.3 across all packages while refactoring the CLI build process to use a dedicated post-build script for telemetry token injection.

  • Version bump from 3.0.0-beta.2 to 3.0.0-beta.3 across all packages
  • Refactored CLI build telemetry handling from tsup config to dedicated post-build script
  • Added package.json export for sql.js dialect package

Reviewed Changes

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

Show a summary per file
File Description
package.json (root + packages) Version bump to 3.0.0-beta.3
packages/cli/tsup.config.ts Removed onSuccess telemetry handling
packages/cli/scripts/post-build.ts New dedicated script for telemetry token replacement
packages/dialects/sql.js/package.json Added package.json export alongside version bump
.github/workflows/*.yml Updated CI workflows to run post-build script
CLAUDE.md Added note about VSCode package versioning

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

@claude
Copy link

claude bot commented Sep 4, 2025

Claude finished @ymc9's task —— View job


Pull Request Review

Reviewing this PR that merges dev to main with version bump to 3.0.0-beta.3 and CLI build refactoring.

Review Tasks

  • Analyze version consistency across all packages
  • Review CLI build process refactoring (tsup.config.ts → post-build.ts)
  • Check new sql.js package.json export
  • Review GitHub workflow changes
  • Assess security implications of telemetry token handling
  • Check for potential bugs or issues
  • Evaluate overall code quality

🔍 Review Findings

Strengths

  1. Version Consistency: All packages correctly updated to 3.0.0-beta.3
  2. Clean Refactoring: The CLI telemetry token handling has been properly separated from the tsup config
  3. Proper Package Export: The sql.js dialect now includes ./package.json export for better compatibility
  4. Workflow Integration: CI/CD workflows correctly updated to run the new post-build script

🏗️ CLI Build Refactoring Analysis - packages/cli/scripts/post-build.ts:1-21

Positive changes:

  • Clean separation of concerns - telemetry token injection moved out of build config
  • Simple, focused script that does one thing well
  • Proper error handling with warning when token is missing
  • Processes both ESM and CJS outputs correctly

Code quality: ✅ Good

  • Clean, readable code
  • Appropriate use of Node.js APIs
  • Clear file path resolution using __dirname equivalent

🔧 Workflow Changes Analysis

Both build-test.yml:70-74 and publish-release.yml:38-42 correctly updated to:

  1. Run pnpm run build
  2. Execute pnpm tsx packages/cli/scripts/post-build.ts

This ensures telemetry tokens are injected in both CI testing and release builds.

📦 Package Export Enhancement - packages/dialects/sql.js/package.json:29-33

Added proper ./package.json export:

"./package.json": {
    "import": "./package.json",
    "require": "./package.json"
}

This follows modern Node.js packaging best practices and improves compatibility.

🔒 Security Assessment

Telemetry Token Handling: ✅ Secure

  • Token sourced from environment variables only
  • No hardcoded secrets
  • Graceful handling when token is missing
  • Replacement happens at build time, not runtime
  • Token scope limited to telemetry tracking

📋 Overall Assessment

Code Quality: ✅ Excellent

  • Clean, maintainable code changes
  • Follows established patterns and conventions
  • Proper error handling

Performance: ✅ No concerns

  • Post-build script is lightweight
  • No runtime performance impact

Security: ✅ Secure

  • Proper secret handling via environment variables
  • No security vulnerabilities introduced

Testing: ⚠️ Recommendation

  • While the refactoring is straightforward, consider adding a test to verify the post-build script correctly replaces tokens in generated files

Compatibility: ✅ Good

  • Package exports follow Node.js standards
  • Maintains backward compatibility

Recommendation

This PR is ready to merge. The changes are well-implemented, secure, and maintain code quality standards. The CLI build refactoring is a good improvement that separates concerns appropriately.

Minor suggestion for future: Consider adding a simple test for the post-build script to verify token replacement works correctly.

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

@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: 2

🧹 Nitpick comments (6)
packages/cli/package.json (1)

6-6: Version bump looks good; consider wiring post-build via npm lifecycle for local builds.

Since telemetry token replacement moved to a standalone script, add a postbuild to ensure pnpm build locally also performs substitution (not only CI).

   "scripts": {
-    "build": "tsc --noEmit && tsup-node",
+    "build": "tsc --noEmit && tsup-node",
+    "postbuild": "tsx scripts/post-build.ts",
     "watch": "tsup-node --watch",
CLAUDE.md (1)

85-85: Tighten wording for clarity.

Small grammar tweak and remove quotes around the package path.

-- The "ide/vscode" package by-design has a different version from the rest of the packages as VSCode doesn't allow pre-release versions in its marketplace.
+- The ide/vscode package is intentionally versioned differently from the rest because the VSCode marketplace disallows pre-release versions.
packages/dialects/sql.js/package.json (1)

28-32: Nit: simplify package.json export.
You can map "./package.json" directly without import/require conditions; Node and bundlers resolve it fine and it’s shorter.

Apply this diff:

         },
-        "./package.json": {
-            "import": "./package.json",
-            "require": "./package.json"
-        }
+        "./package.json": "./package.json"
packages/cli/scripts/post-build.ts (1)

7-9: Decide policy when TELEMETRY_TRACKING_TOKEN is missing.
Warning-only may ship placeholders; consider an opt-in enforcement for release contexts.

Apply this diff to optionally enforce via an env flag:

 if (!token) {
-    console.warn('TELEMETRY_TRACKING_TOKEN is not set.');
+    console.warn('TELEMETRY_TRACKING_TOKEN is not set.');
+    if (process.env.ENFORCE_TELEMETRY_TOKEN === '1') {
+        throw new Error('TELEMETRY_TRACKING_TOKEN is required but missing');
+    }
 }
.github/workflows/build-test.yml (1)

71-73: Centralize post-build step
Move the inline tsx invocation into a shared repo script (tsx is already installed at root at version ^4.20.3).

-              run: |
-                  pnpm run build
-                  pnpm tsx packages/cli/scripts/post-build.ts
+              run: |
+                  pnpm run build
+                  pnpm run postbuild:telemetry

Add to the root package.json:

{
  "scripts": {
    "postbuild:telemetry": "tsx packages/cli/scripts/post-build.ts"
  }
}
.github/workflows/publish-release.yml (1)

39-41: Mirror centralization and add guardrails for releases.

  • Reuse a repo script for the post-build.
  • Optionally fail early if the token is required for published artifacts.

Change this step:

-              run: |
-                  pnpm run build
-                  pnpm tsx packages/cli/scripts/post-build.ts
+              run: |
+                  pnpm run build
+                  pnpm run postbuild:telemetry

Optionally enforce token for releases (before Build):

- name: Ensure telemetry token is set (optional)
  if: github.ref == 'refs/heads/main'
  run: |
    if [ -z "${TELEMETRY_TRACKING_TOKEN}" ]; then
      echo "TELEMETRY_TRACKING_TOKEN is required for release"; exit 1;
    fi

Add a verification step after Build:

- name: Verify telemetry token substitution
  run: rg -n "<TELEMETRY_TRACKING_TOKEN>" packages/cli/dist && exit 1 || true
📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between a115040 and 0f9764f.

📒 Files selected for processing (21)
  • .github/workflows/build-test.yml (1 hunks)
  • .github/workflows/publish-release.yml (1 hunks)
  • CLAUDE.md (1 hunks)
  • package.json (1 hunks)
  • packages/cli/package.json (1 hunks)
  • packages/cli/scripts/post-build.ts (1 hunks)
  • packages/cli/tsup.config.ts (0 hunks)
  • packages/common-helpers/package.json (1 hunks)
  • packages/create-zenstack/package.json (1 hunks)
  • packages/dialects/sql.js/package.json (2 hunks)
  • packages/eslint-config/package.json (1 hunks)
  • packages/language/package.json (1 hunks)
  • packages/runtime/package.json (1 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)
💤 Files with no reviewable changes (1)
  • packages/cli/tsup.config.ts
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**

📄 CodeRabbit inference engine (CLAUDE.md)

Packages are located in packages/, samples/, and tests/

Files:

  • packages/typescript-config/package.json
  • samples/blog/package.json
  • packages/testtools/package.json
  • packages/tanstack-query/package.json
  • packages/sdk/package.json
  • packages/vitest-config/package.json
  • packages/eslint-config/package.json
  • packages/zod/package.json
  • tests/e2e/package.json
  • packages/runtime/package.json
  • packages/common-helpers/package.json
  • packages/dialects/sql.js/package.json
  • packages/cli/package.json
  • packages/create-zenstack/package.json
  • packages/cli/scripts/post-build.ts
  • packages/language/package.json
tests/e2e/**

📄 CodeRabbit inference engine (CLAUDE.md)

E2E tests are in tests/e2e/ directory

Files:

  • tests/e2e/package.json
🧠 Learnings (2)
📚 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: Database migrations still use Prisma CLI under the hood

Applied to files:

  • CLAUDE.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: `zenstack generate` compiles ZModel to TypeScript schema (`schema.ts`)

Applied to files:

  • packages/language/package.json
⏰ 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 (15)
packages/eslint-config/package.json (1)

3-3: LGTM: version bump only

Private package; no publish surface impact. All good.

packages/sdk/package.json (1)

3-3: LGTM: version aligned to beta.3

No other changes; workspace dependencies remain unaffected.

samples/blog/package.json (1)

3-3: LGTM: sample app version bump

No runtime/build script changes here; safe.

packages/testtools/package.json (1)

3-3: LGTM: testtools version bump

Exports/peer deps unchanged; good to go.

packages/typescript-config/package.json (1)

3-3: LGTM: version bump only.

packages/create-zenstack/package.json (1)

3-3: LGTM: version bump only.

packages/common-helpers/package.json (2)

3-3: Version bump looks good.

No behavioral changes here; safe metadata update.


3-3: Release bump verified; CI post-build and tsup hook removal confirmed.

All package.json versions align with 3.0.0-beta.3 (excluding the intentional VSCode extension), workflows invoke the new post-build script, and the onSuccess hook is absent.

packages/vitest-config/package.json (1)

4-4: LGTM on version bump.

Matches the repo-wide pre-release versioning.

tests/e2e/package.json (1)

3-3: E2E package version bump approved.

No script/dependency changes; safe.

packages/runtime/package.json (1)

3-3: Runtime package version bump looks good.

No export/deps changes; API surface unaffected.

packages/zod/package.json (1)

3-3: Zod adapter version bump approved.

No functional changes introduced here.

packages/tanstack-query/package.json (1)

3-3: Version bump to beta.3 looks good.
No further concerns from this file.

packages/dialects/sql.js/package.json (1)

3-3: Version bump to beta.3 looks good.
No issues spotted.

packages/cli/scripts/post-build.ts (1)

14-20: Replace content.replace with replaceAll and verify post-build artifacts
Use a placeholder check and replaceAll (logging a warning if missing). After updating, run the CLI build (e.g. npm run build --workspace packages/cli or equivalent) then:

rg -n '<TELEMETRY_TRACKING_TOKEN>' packages/cli/dist

to confirm no placeholders remain.

@ymc9 ymc9 added this pull request to the merge queue Sep 4, 2025
Merged via the queue into main with commit 28ab57b Sep 4, 2025
10 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