chore(scripts): update release script to include rivetkit

[NathanFlurry]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Oct 27, 2025 6:33pm
rivet-inspector	Ignored	Preview		Oct 27, 2025 6:33pm
rivet-site	Ignored	Preview		Oct 27, 2025 6:33pm

[NathanFlurry]

• chore(scripts): update release script to include rivetkit #3265 👈 (View in Graphite)
• chore(config): disable valid_hosts filter unless explicitly configured #3274
• main

---

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

PR Review: Update release script to include rivetkit

Summary

This PR consolidates the release scripts by merging the separate rivetkit release script into the main release script, adding support for rivetkit package publishing alongside the existing engine SDK publishing.

---

✅ Positive Changes

1. Good Consolidation: Merging scripts/rivetkit/release.ts into the main release script reduces duplication and makes the release process more unified.

2. Improved CLI Interface: The migration from minimist to commander provides better argument parsing and more descriptive help text.

3. Flexible Version Bumping: Adding --major, --minor, and --patch flags for automatic version calculation is a UX improvement over requiring explicit versions.

4. Better Step Control: The granular step system with --steps and --batch-steps provides excellent flexibility for running specific parts of the release process.

5. GitHub Release Automation: Adding createGitHubRelease() functionality automates a previously manual step.

---

🐛 Potential Issues

Critical Issues

1. Pre-release Detection Logic Inconsistency (sdk.ts:99, sdk.ts:124)

   const isReleaseCandidate = opts.version.includes("-rc.");

   In git.ts:68, pre-release detection uses:

   if (opts.version.includes("-")) {

   Problem: These two different checks will behave differently for versions like 1.0.0-alpha.1 or 1.0.0-beta.1. The SDK publishing will tag them as "latest" (incorrect) while GitHub will mark them as pre-releases (correct).

   Recommendation: Use a consistent approach, preferably leveraging the semver library that's already imported:

   const isPrerelease = semver.prerelease(opts.version) !== null;
   const tag = isPrerelease ? "next" : "latest"; // or "rc", but "next" is more conventional

2. Unsafe Force Push (git.ts:22)

   await $({ cwd: opts.root })`git push origin v${opts.version} -f`;

   Problem: Force-pushing tags can cause issues in distributed environments where others may have already pulled the tag.

   Recommendation: Only use force push if the tag already exists and needs updating. Consider adding a check or warning.

3. Error Handling Inconsistency (main.ts:258, main.ts:260)

   assertEquals(releaseOpts.commit.length, 7, "must use 8 char short commit");

   Problem: Error message says "8 char" but checks for length 7.

   Fix: Change to "must use 7 char short commit" or adjust the check to 8.

Medium Priority Issues

4. Missing Step Validation (main.ts:216-219)

   const shouldRunStep = (step: Step): boolean => {
       return requestedSteps.has(step);
   };

   Problem: When neither --steps nor --batch-steps is provided, requestedSteps will be empty, and NO steps will run. The script will complete successfully without doing anything.

   Recommendation: Add validation or default behavior:

   // After parsing steps, if none specified, error or show help
   if (requestedSteps.size === 0) {
       throw new Error("Must specify at least one step via --steps or --batch-steps");
   }

5. Unused Step in STEPS Array (main.ts:126)
   The step "merge-release" is defined in the STEPS array but never used in the code.

   Recommendation: Either implement it or remove it from the array.

6. Race Condition Risk (sdk.ts:53-64)
   Building rivetkit packages happens once before publishing, but if building fails partway through, some packages might be published with stale builds.

   Recommendation: Consider building packages individually before each publish, or add better error recovery.

7. Incomplete Type Safety (git.ts:49)

   const existingRelease = releases.find((r: any) => r.name === opts.version);

   Using any type bypasses TypeScript's type checking.

   Recommendation: Define a proper interface:

   interface GitHubRelease {
       name: string;
       tagName: string;
   }
   const releases = JSON.parse(releaseJson) as GitHubRelease[];

Low Priority / Style Issues

8. Misleading Variable Name (main.ts:260)

   if (opts.validateGit && !shouldRunStep("run-type-check")) {

   The logic inverts the meaning - when --no-validate-git is used, opts.validateGit is false, but the code checks it as a positive condition. This is confusing.

   Recommendation: Use clearer variable naming or add a comment explaining the hack mentioned on line 261.

9. Duplicate Version Check Logic (sdk.ts:85-86, sdk.ts:110-111)
   The same version existence check is duplicated for engine packages and rivetkit packages.

   Recommendation: Extract into a helper function to reduce duplication.

10. Missing cwd Parameter (sdk.ts:102-104)
    Engine SDK publishing doesn't specify cwd: opts.root while rivetkit publishing does (line 129).

    Impact: Minor, but inconsistent. If run from a different directory, engine SDK publishing might fail.

---

🔒 Security Considerations

1. No Credential Validation: The script doesn't verify that NPM credentials are available before attempting to publish. This could lead to partial releases.

2. Force Push Without Confirmation: Force-pushing tags without user confirmation could accidentally overwrite production releases.

---

📊 Test Coverage

Concern: No test files are included in this PR. The release script is critical infrastructure that would benefit from:

• Unit tests for version parsing logic
• Integration tests for step execution
• Mock tests for external commands (git, gh, npm)

---

📝 Code Quality

1. Good: Proper use of TypeScript types (Step, BatchStep, ReleaseOpts)
2. Good: Clear error messages with emoji indicators
3. Good: Logical separation of concerns across multiple files
4. Needs Improvement: Inconsistent error handling patterns (some throw, some log and throw, some log warnings)

---

🎯 Recommendations

Before Merging:

1. Fix the pre-release detection inconsistency ([SVC-2555] Set up issue templates #1) - CRITICAL
2. Fix the commit length error message ([SVC-2504] Fix 5 GB upload limit for local development from Cloudflare #3)
3. Add validation for when no steps are specified ([SVC-2483] Remove hardcoded uses of rivet.gg #4)
4. Remove or implement the merge-release step ([SVC-2358] Enable Redis memory overcommit #5)

Follow-up Work:

1. Add tests for the release script
2. Consider adding a dry-run mode for testing releases
3. Document the new CLI interface and step system
4. Add credential validation before publishing

---

Overall Assessment

Status: ✅ Approve with requested changes

This is a solid refactoring that improves the release process significantly. The main concerns are around edge cases in version detection and ensuring the script doesn't silently do nothing when misconfigured. Once the critical issues are addressed, this will be a great improvement to the release infrastructure.

🤖 Generated with Claude Code

[pkg-pr-new]

More templates

• example-ai-agent
• example-background-jobs
• example-better-auth-external-db
• example-bots
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-cursors
• example-cursors-raw-websocket
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• example-game
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• example-smoke-test
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc
• example-workflows

@rivetkit/actor

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/actor@3265

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@3265

@rivetkit/core

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/core@3265

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@3265

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@3265

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@3265

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@3265

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@3265

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@3265

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@3265

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@3265

commit: 4c52eca

[claude]

Pull Request Review

Summary

This PR refactors the release script to consolidate the Rivet and RivetKit release processes into a unified system. It replaces minimist with commander for better CLI argument parsing and introduces a phase-based execution model.

Positive Changes ✅

1. Better CLI Interface: The migration from minimist to commander provides better help text, validation, and user experience.

2. Phase-based Architecture: The introduction of STEPS and PHASES with PHASE_MAP makes the release process more modular and understandable.

3. Semver Support: Adding automatic version bumping with --major, --minor, and --patch flags is a great UX improvement.

4. RivetKit Integration: Properly includes RivetKit packages in the release process, removing the need for a separate script.

5. GitHub Release Automation: The createGitHubRelease function handles both creating new releases and updating existing ones.

Issues Found 🐛

1. Critical: Inconsistent Commit Length Assertion (scripts/release/main.ts:263)

The error message says "must use 8 char short commit" but the assertion checks for length 7. Either update the assertion to check for 8, or update the message to say "7 char".

2. Bug: Pre-release Detection Logic (scripts/release/sdk.ts:99 & 124)

The code only detects release candidates with "-rc." pattern, but other pre-release formats like "-beta", "-alpha", or "-rc1" (without dot) will not be tagged correctly. The GitHub release function (git.ts:68) uses a more permissive check. This inconsistency should be fixed.

3. Potential Issue: Removed Release Please Integration

The old code had configureReleasePlease functionality that has been removed. Make sure this is intentional and that Release Please configuration is handled elsewhere (or no longer needed).

4. Potential Issue: Missing merge-release Step Implementation

The merge-release step is defined in STEPS but never implemented in the execution logic. Either implement it or remove it from the list.

Code Quality Observations 🔍

1. Good: The use of TypeScript const assertions for STEPS and PHASES provides excellent type safety.

2. Good: Error handling with try-catch blocks and descriptive error messages.

3. Inconsistency: The getRivetkitPackages function filters packages but does not handle potential errors from pnpm command failures.

4. Minor: Unused variable initialization in sdk.ts:86 & 110 could be simplified from let/reassignment to const.

Security Considerations 🔒

1. Force Push Tags: The createAndPushTag function uses git push with -f flag. While this might be intentional for fixing mistakes, force-pushing tags can be dangerous in production. Consider adding confirmation prompts or documenting why this is necessary.

2. Command Injection: All git/gh commands use template literals with user-provided version strings. The semver regex validation helps, but ensure version strings are always validated before use.

Performance Considerations ⚡

1. The sequential processing of packages in publishSdk could be parallelized for faster releases.

2. The npmVersionExists check for each package adds latency but is acceptable for correctness.

Recommendations

1. High Priority: Fix the commit length assertion message
2. High Priority: Make pre-release detection consistent
3. Medium Priority: Handle or remove the merge-release step
4. Medium Priority: Add error handling to getRivetkitPackages
5. Low Priority: Consider making force-push behavior configurable
6. Low Priority: Optimize package publishing with parallelization

Overall Assessment

This is a solid refactoring that consolidates the release process and improves the CLI interface. The code is well-structured and mostly follows good practices. With the critical bugs fixed (commit length message and pre-release detection), this should be ready to merge. Great work on consolidating two separate release processes into one! 🚀

[claude]

Code Review

This PR refactors the release script to consolidate rivetkit releases with the main Rivet release process. Overall, this is a well-structured improvement that removes duplication and provides better control over the release workflow. Here is my detailed review:

---

Strengths

1. Excellent Architecture Improvements

• Consolidation: Removing the duplicate scripts/rivetkit/release.ts (555 lines) and integrating rivetkit into the main release script is a great refactoring
• Phase-based execution: The new --phase flag with setup-local, setup-ci, and complete-ci phases provides clear separation of concerns for CI/CD
• Granular control: The --only-steps flag allows running individual steps, which is excellent for debugging and testing

2. Better CLI Design

• Migration from minimist to commander provides better argument parsing, built-in help generation, and more robust validation
• Semantic version bumping (--major, --minor, --patch) is more intuitive than manual version strings
• Consistent flag naming (e.g., --no-validate-git instead of --noValidateGit)

3. Code Quality

• Good error handling with descriptive messages
• Clear separation of concerns across files (git.ts, sdk.ts, etc.)
• Type safety maintained throughout

---

Bugs and Issues

1. Critical: Incorrect Error Message (scripts/release/main.ts:263)

The error message says "8 char" but checks for length 7. Should be "must use 7 char short commit"

2. Logic Error: Git Validation Condition (scripts/release/main.ts:265)

This logic seems inverted. The comment says "Skip setup-ci" but the condition validates git when NOT running type-check. This could skip validation when it should not. Please review this logic carefully.

3. Prerelease Detection Issue (scripts/release/sdk.ts:99-100, 124-125)

The code only detects -rc. (with a dot). Other pre-release formats like -alpha, -beta, -rc1 will not be tagged as pre-releases. Recommend using proper semver.parse() to detect any prerelease version.

4. Inconsistent Tag Assignment

The opts.latest flag is not considered when assigning tags to non-prerelease versions. If --no-latest is specified, packages should probably be tagged as next instead of latest.

5. Error in Git Release Logic (scripts/release/git.ts:68)

This marks ANY version with a hyphen as prerelease, including build metadata. Use semver parsing instead of string matching.

---

Potential Issues

1. Missing Validation

getVersionFromArgs does not validate mutual exclusivity of --major/--minor/--patch flags. A user could pass --major --minor and only major would be processed.

2. Force Push Safety (scripts/release/git.ts:22)

Force pushing tags can be dangerous if multiple people are releasing. Consider warning the user or checking if the tag already exists remotely with a different commit.

3. Unused Step

The merge-release step is defined but never implemented in the code. Either implement it or remove it from the STEPS array.

4. Missing Type Check in Engine SDK Build

The publishSdk function builds rivetkit packages with type checking, but engine SDKs are not built before publishing. This could lead to publishing untested code.

---

Performance Considerations

1. Sequential Package Publishing

Packages are published sequentially. For many packages, this could be slow. Consider using Promise.all() to publish independent packages in parallel.

2. Redundant Version Checks

npmVersionExists line 86 has unnecessary let versionExists = false before the assignment.

---

Code Style and Best Practices

1. Magic Numbers

The commit hash length checks use magic numbers (40, 7). Consider defining constants.

2. Type Safety

scripts/release/git.ts:49 and scripts/release/sdk.ts:46 use any type. Consider defining proper interfaces for GitHub API responses.

---

Test Coverage

No tests for the new functionality. Given the critical nature of release scripts, consider adding unit tests for version parsing and validation, integration tests for phase execution, and mock tests for git/npm operations.

---

Recommendations

1. Fix the critical bugs identified above (commit length message, semver detection)
2. Add validation for mutually exclusive flags
3. Consider parallelizing package publishing
4. Add error recovery for idempotent operations
5. Document the new workflow
6. Consider dry-run mode

---

Summary

This is a solid refactoring that improves maintainability and provides better control over the release process. The main concerns are:

• The semver prerelease detection logic needs improvement
• The git validation condition needs clarification
• Consider adding tests for this critical infrastructure

Overall: Approve with minor changes requested

Great work consolidating the release processes!

[claude]

Code Review

Overview

This PR refactors the release script to consolidate rivetkit publishing with the main release process. The changes replace the old release approach with a phase-based system that is more modular and maintainable.

Positive Aspects

1. Better Code Organization: The new phase-based approach is much cleaner than the previous flag-based system
2. Improved Modularity: Extracting git operations into scripts/release/git.ts and SDK publishing into scripts/release/sdk.ts follows good separation of concerns
3. Rivetkit Integration: Successfully consolidates rivetkit package publishing into the main release workflow
4. Better Error Handling: The git validation and version existence checks prevent accidental double-publishing
5. Consistent Versioning: All packages now use the same version number

Issues and Recommendations

1. Critical: Incorrect Commit Length Assertion (scripts/release/main.ts:263)

The error message says 8 char but the check validates 7 characters. This is confusing.
Recommendation: Update error message to must use 7 char short commit

2. Potential Bug: Git Tag Force Push (scripts/release/git.ts:22)

Force pushing tags can be dangerous and may cause issues for users who have already pulled the tag.
Recommendation: Consider checking if the tag already exists remotely and either warn the user or use a different approach for RC versions vs stable releases.

3. Code Quality: Unused Parameter (scripts/release/git.ts:4)

The validateGit function has an unused _opts parameter.
Recommendation: Either use opts.root as the cwd for consistency, or remove the parameter if not needed.

4. Inconsistent npm Tag Logic

scripts/release/sdk.ts uses -rc. to detect release candidates, but scripts/release/git.ts uses just -
Recommendation: Use consistent logic across both files. Consider using semver prerelease() function for robustness.

5. Testing Concerns

No test coverage is visible for the new release scripts. Given the critical nature of release automation, this is a significant gap.
Recommendation: Add integration tests or at least dry-run functionality to validate the release process without actually publishing.

6. Documentation Gap

The justfile now uses --phase setup-local but there is no documentation explaining the different phases or when to use them.
Recommendation: Add a comment in the justfile or create a README in scripts/release/ explaining what each phase does and the complete release workflow

7. Minor: Duplicate Version Check Logic

In scripts/release/sdk.ts, lines 86 and 110 unnecessarily initialize versionExists to false before assignment.
Recommendation: Use const instead of let for cleaner code

Security Considerations

1. Force Push Tags: The -f flag on git tag push could potentially overwrite existing releases
2. No Git Validation in CI: Line 265 skips git validation during the run-type-check phase

Performance Considerations

1. Sequential Package Publishing: Rivetkit packages are published sequentially. Consider parallel publishing with rate limiting.
2. Build Force Flag: Line 59 uses --force which bypasses Turbo cache (appropriate for releases)

Summary

This is a solid refactoring that improves the release process significantly. The main concerns are:

1. The commit length assertion error message mismatch
2. Force-pushing tags without safeguards
3. Missing test coverage
4. Documentation gaps

The code quality is generally good, but addressing the force-push behavior and adding tests would greatly improve confidence in the release automation.

Overall Assessment: Approve with recommendations for follow-up improvements, particularly around testing and the force-push behavior.

[claude]

Pull Request Review: Release Script Refactor

Summary

This PR consolidates the release process by integrating rivetkit package publishing into the main release script, replacing the separate scripts/rivetkit/release.ts with enhanced functionality in the main release pipeline.

---

Positive Aspects

• Good Consolidation: Merging rivetkit release into the main release script reduces duplication
• Phase-Based Architecture: The new --phase flag system provides clear separation of concerns
• Version Validation: Strong validation using semver regex pattern at line 235-239
• Idempotent Publishing: The npmVersionExists check prevents accidental re-publishing
• Release Candidate Support: Proper handling of RC versions

---

Critical Issues

1. Git Force Push Without Safety Check (git.ts:19-22)
Force pushing tags can overwrite existing releases, potentially causing version conflicts. Add a check to only force push if explicitly requested via a flag.

2. Commit Length Assertion Bug (main.ts:263)
Error message says must use 8 char but checks for length 7. This is confusing and should be fixed.

---

High Priority Issues

3. Inconsistent CLI Flag Naming
Old flags like --setupCi changed to --phase setup-ci. This is a breaking change that should be documented.

4. Missing Error Context in Git Operations (git.ts:4-13)
Git validation doesn't show WHAT files have uncommitted changes. Include the actual git status output.

5. Unsafe Version Bump Logic (main.ts:84-110)
Gets current version only from rivetkit package but updates many packages. Validate all packages have the same version before bumping.

---

Medium Priority Issues

6. Hard-Coded Package Paths (sdk.ts:67-71)
If new SDK packages are added, this list must be manually updated. Use a glob pattern or workspace query instead.

7. No Rollback Mechanism
If publishing fails midway, there is no automated rollback. Document manual rollback procedures at minimum.

8. GitHub Actions Issue (main.ts:265-267)
HACK comment indicates an unresolved issue about changes appearing in CI. This should be investigated.

---

Code Quality

Good: Type safety, error handling, separation of concerns, commander CLI parsing
Needs Improvement: No unit tests, limited documentation, no dry-run mode

---

Recommendation: Request Changes

Must Fix Before Merge:

• Fix commit length assertion error message
• Add safety check before force pushing tags
• Address the HACK comment about CI changes

Should Fix Soon After:

• Add error context to git validation
• Implement version consistency checks
• Add dry-run mode and tests

[claude]

Pull Request Review

Summary

This PR consolidates the release scripts by integrating rivetkit package publishing into the main release workflow. The separate scripts/rivetkit/release.ts (555 lines) has been removed and its functionality merged into the main release scripts.

Positive Observations

1. Code Consolidation - removes duplication and improves maintainability
2. Phase-Based Architecture - clear separation with setup-local, setup-ci, complete-ci phases
3. Idempotent Publishing - npmVersionExists() check prevents duplicate publishes
4. Flexible CLI with Commander - multiple options for version bumping and phase control
5. Release Candidate Support - proper handling with --tag rc for NPM publishing
6. Good error messages throughout

Critical Issues

1. Git Force Push Safety (git.ts:22) - Force pushing tags can cause issues. Per CLAUDE.md: NEVER run force push to main/master, warn the user if they request it

2. Commit Hash Length Inconsistency (main.ts:267-271) - Error message says 8 char but checks for 7 characters

3. Git Validation HACK (main.ts:273-276) - Skips git validation for run-type-check phase with HACK comment suggesting workaround for deeper issue

Medium Issues

4. Brittle Error Handling (sdk.ts:19-35) - stderr matching could break if NPM changes error format
5. Missing Dependency Ordering - No validation that rivetkit packages dont depend on unpublished engine packages
6. No Rollback Mechanism - Partial failures could leave some packages published and others not
7. Hard-coded Paths (sdk.ts:67-71) - Brittle if project structure changes

Minor Issues

8. Variable Shadowing (update_version.ts:58) - glob variable shadows imported function
9. Complex Regex (main.ts:243) - Consider using semver.valid() instead
10. Type Safety (git.ts:48-49) - Using any type instead of proper types

Test Coverage

No automated tests found for release scripts. Critical infrastructure should have comprehensive tests including unit tests for version bumping, integration tests for git operations, and mock tests for NPM publishing.

Recommendations

High Priority:

• Fix 7 vs 8 character commit hash inconsistency
• Remove or gate force-push operations behind explicit flags
• Add proper TypeScript types
• Resolve git validation HACK

Medium Priority:

• Add comprehensive test coverage
• Document release process and recovery procedures
• Implement error recovery/rollback

Overall Assessment

This is a solid refactoring with good structure. Main concerns: force push safety, missing tests, and minor type safety issues. With fixes to critical issues, this would be production-ready.

Recommendation: Approve with requested changes for critical issues

[claude]

Pull Request Review

This PR consolidates the release process to include both Rivet and RivetKit packages in a unified release workflow. Overall, this is a solid refactor that improves maintainability.

Strengths

1. Unified Release Process - Consolidating release scripts reduces duplication
2. Improved CLI Design - The --phase parameter is more maintainable than boolean flags
3. Better Dependency Management - commander provides better CLI parsing
4. Version Discovery - Dynamic package discovery is maintainable
5. Duplicate Prevention - npmVersionExists check prevents re-publishing

Issues Found

Critical: Incorrect Commit Length Assertion (scripts/release/main.ts:271)
The assertion checks for length 7 but the error message says 8 char. This is inconsistent.

Medium: Version Consistency
Cargo.toml workspace version changes from 25.8.2 to 2.0.22-rc.1 but rivet-api-full stays at 25.8.2 creating inconsistency.

Medium: Force Push Concerns (scripts/release/git.ts:19-22)
Force pushing tags can cause problems for users.

Medium: Git Validation Skip (scripts/release/main.ts:273-276)
HACK comment indicates workaround for unresolved issues.

Minor: Pre-release Tag Logic (sdk.ts:99-100, 124-125)
Only checks for -rc. not other pre-release formats.

Minor: Removed Justfile Commands
Might break existing workflows.

Suggestions

1. Add more descriptive error messages
2. Consider --dry-run flag
3. Update release documentation
4. Add tests for version parsing

Summary

Approve with changes - Address the critical commit length assertion issue before merging.

Code Quality: Good | Best Practices: Good | Security: No concerns | Performance: No concerns

[claude]

Pull Request Review: Release Script Refactoring

Overview

This PR refactors the release process to consolidate rivetkit releases into the main release script. The changes include:

• Removal of 555 lines from scripts/rivetkit/release.ts
• Removal of release_please.ts
• Significant refactoring of scripts/release/main.ts with a phase-based architecture
• New git utilities extracted to scripts/release/git.ts
• Enhanced SDK publishing logic in scripts/release/sdk.ts

Code Quality & Best Practices ✅

Strengths:

1. Excellent Refactoring - The phase-based architecture (setup-local, setup-ci, complete-ci) is much cleaner than the previous approach
2. Good Separation of Concerns - Git operations extracted to git.ts, SDK publishing to sdk.ts
3. Better Error Handling - Proper error messages and validation throughout
4. Flexible CLI - The --phase and --only-steps flags provide good flexibility for testing and CI/CD
5. Version Validation - Semver regex validation at line 243 of main.ts
6. Idempotency - Checks if NPM packages already exist before publishing (lines 85-93 in sdk.ts)

Minor Suggestions:

1. Commit Hash Length Validation (scripts/release/main.ts:271)

   assertEquals(releaseOpts.commit.length, 7, "must use 8 char short commit");

   The error message says "8 char" but validates for 7. Should be: "must use 7 char short commit"

2. Force Push Safety (scripts/release/git.ts:22)

   await $({ cwd: opts.root })`git push origin v${opts.version} -f`;

   Consider adding a warning or confirmation for force pushes to avoid accidentally overwriting tags. However, this may be intentional for CI/CD.

3. Rivetkit Package Discovery (scripts/release/sdk.ts:38-50)
   The function uses pnpm -r list --json which is good, but consider caching this result if called multiple times.

Potential Bugs 🔍

1. Git Validation Skip Logic (scripts/release/main.ts:273-276)

   if (opts.validateGit && !shouldRunStep("run-type-check")) {
       // HACK: Skip setup-ci because for some reason there's changes in the setup step but only in GitHub Actions
       await validateGit(releaseOpts);
   }

   This logic seems inverted or confusing:

   • If validateGit is true AND we're NOT running type check, then validate
   • But the comment suggests this is a hack to skip validation during setup-ci
   • Consider clarifying this logic or adding more detailed comments

2. Tag Selection Logic (scripts/release/sdk.ts:99-100, 124-125)

   const isReleaseCandidate = opts.version.includes("-rc.");
   const tag = isReleaseCandidate ? "rc" : "latest";

   This checks for -rc. but semver pre-releases can have other formats like -alpha, -beta, etc. Consider:

   const isPrerelease = opts.version.includes("-");
   const tag = isPrerelease ? "next" : "latest";

3. Missing Validation for opts.latest Flag (scripts/release/sdk.ts)
   The SDK publish function tags packages as "latest" regardless of the opts.latest flag. Should respect this flag when deciding the NPM dist-tag.

Security Concerns 🔒

1. Force Push to Tags (scripts/release/git.ts:22)
   Force pushing tags can be dangerous in production. Ensure this is intentional and document why it's necessary.

2. No Verification of Git Remote
   The script pushes to origin without verifying it's the correct remote. Consider adding a check.

3. Command Injection Risk (scripts/release/main.ts:294)

   `git commit --allow-empty -m "chore(release): update version to ${releaseOpts.version}"`

   The version is validated with regex, so this should be safe, but the validation at line 243 provides good protection.

Performance Considerations ⚡

1. Sequential NPM Checks (scripts/release/sdk.ts:76-105, 108-131)
   The script checks NPM versions sequentially. Consider parallelizing these checks with Promise.all to speed up the release process.

2. Repeated Package Builds
   runTypeCheck (line 66-69) and publishSdk (line 54-64) both build rivetkit packages. Consider building once and reusing.

Test Coverage 🧪

The PR doesn't include tests for the new release scripts. Consider adding:

1. Unit tests for version validation logic
2. Integration tests for the phase system
3. Mock tests for git operations

However, release scripts are often tested manually in staging environments, so this may be acceptable.

Architecture Feedback 🏗️

Excellent improvements:

1. The phase-based architecture makes the release process much more maintainable
2. Extracting git operations to a separate module improves testability
3. The ability to run individual steps via --only-steps is great for debugging
4. Consolidating rivetkit and main releases into one script reduces duplication

Suggested enhancements:

1. Consider adding a dry-run mode for testing
2. Add progress indicators for long-running operations
3. Consider adding rollback functionality

Workflow Changes 📋

The GitHub Actions changes look good:

• --setupCi → --phase setup-ci
• --completeCi → --phase complete-ci
• --noValidateGit → --no-validate-git

These changes improve consistency with CLI conventions.

Summary

This is a solid refactoring that improves the release process significantly. The main concerns are:

Must Fix:

• Line 271: Fix error message (7 char vs 8 char)

Should Consider:

• Git validation logic clarity (line 273)
• Tag selection for non-rc pre-releases
• Respecting opts.latest flag in SDK publishing

Nice to Have:

• Parallelize NPM version checks
• Avoid duplicate builds
• Add dry-run mode

Overall: Approve with minor fixes ✅

The architecture is much better, the code quality is good, and the changes consolidate duplicate logic effectively. The issues identified are minor and can be addressed in follow-up commits if needed.

[claude]

Pull Request Review

This PR consolidates the release process by merging scripts/rivetkit/release.ts into the main release script.

Strengths

• Good code consolidation reducing maintenance burden
• Well-designed phase-based architecture (setup-local, setup-ci, complete-ci)
• Proper handling of build dependencies for rivetkit and Deno examples
• Enhanced SDK publishing for both engine and rivetkit packages

Critical Issues

1. Git Force Push (scripts/release/git.ts:22)
Force pushing tags without validation can overwrite existing tags and break CI/CD references. Add safety checks before force operations.

2. Error Message Bug (scripts/release/main.ts:271)
Error message says "8 char" but checks for length 7. Should be "must use 7 char short commit"

3. Error Handling (sdk.ts:19-35)
npm version checks may fail silently if error.stderr is empty, potentially causing duplicate publish attempts.

Code Quality Issues

4. Validation Hack (main.ts:273-276)
Comment indicates unexplained workaround. Root cause should be investigated and properly fixed.

5. Duplicate Builds
Rivetkit packages are built twice (main.ts:65-69 and sdk.ts:54-63), wasting CI resources.

6. Type Safety
Multiple uses of 'any' type (git.ts:49, sdk.ts:19) should be properly typed.

7. Missing Tests
No evidence of tests for critical release infrastructure with irreversible operations.

Summary

Approve with changes required. The consolidation is good architectural decision, but address:

Must Fix: 1-3
Should Fix: 4-6
Nice to Have: Add dry-run mode and unit tests

Good work on the consolidation!

[claude]

Pull Request Review

This PR consolidates the release scripts by integrating rivetkit package publishing into the main release workflow. Overall, the changes are well-structured and improve the release process. Here are my findings:

✅ Positive Aspects

1. Good Code Consolidation: The removal of scripts/rivetkit/release.ts (555 lines) and integration into the main release script reduces duplication and makes the release process more unified.

2. Improved Phase-Based Architecture: The new phase system (setup-local, setup-ci, complete-ci) provides better separation of concerns and makes the CI/CD workflow clearer.

3. Better Error Handling: The npmVersionExists function in sdk.ts has good error handling with specific checks for different error messages.

4. Graceful Degradation: The example-deno package.json now gracefully handles missing deno installations.

5. NPM Authentication Fix: The addition of NPM authentication to the complete job in the GitHub workflow is essential and was a good catch.

🔍 Code Quality and Best Practices

Good:

• Clean separation of concerns with separate files (main.ts, git.ts, sdk.ts)
• Type-safe with proper TypeScript types (ReleaseOpts, Step, Phase)
• Good use of assertions for validation

Concerns:

1. Command-line Flag Inconsistency (scripts/release/main.ts:271): The error message says must use 8 char short commit but checks for length 7. This is confusing.

2. Hardcoded Magic Numbers (scripts/release/main.ts:267-269): Should use named constants for commit lengths (40 and 7).

3. Git Force Operations (scripts/release/git.ts:19-22): Force pushing tags can be dangerous in production. Consider adding a warning or requiring explicit confirmation.

⚠️ Potential Issues

1. Race Condition in NPM Publishing (scripts/release/sdk.ts:52-132): The function builds rivetkit packages once, then iterates through packages. If multiple CI jobs run concurrently, there could be race conditions.

2. Missing Validation (scripts/release/main.ts:278-281): The update-version step does not validate if the version already exists in git tags before proceeding.

3. Silent Failure Possibility (scripts/release/sdk.ts:88-93): When a version already exists, it silently skips. In CI, this might mask issues. Consider warning-level logging.

🔒 Security Concerns

1. NPM Token Handling (.github/workflows/release.yaml:250-252): Writing the NPM token to ~/.npmrc is acceptable, but consider using NPM_TOKEN environment variable instead for better security.

2. Git Validation Skip (.github/workflows/release.yaml:262): The --no-validate-git flag bypasses git validation in CI. Ensure this does not allow publishing from dirty working directories.

🚀 Performance Considerations

1. Duplicate Build (scripts/release/main.ts:66-69 and scripts/release/sdk.ts:54-64): Rivetkit packages are built twice - once in runTypeCheck and once in publishSdk. Consider caching build artifacts.

2. Sequential NPM Publishing (scripts/release/sdk.ts:76-131): Packages are published sequentially. Consider publishing in parallel if there are no interdependencies.

📋 Test Coverage

Missing Test Scenarios:

• No validation that the version bump follows semver rules
• No test for the phase execution order
• No integration test for the complete release flow
• No test for rollback scenarios if publishing fails midway

📝 Suggestions

1. Add Dry Run Mode: Consider adding a --dry-run flag to test the release process without actually publishing.
2. Improve Logging: Add more structured logging with timestamps and step durations to help debug CI failures.
3. Version Validation: Add a check to ensure the new version is greater than the current version (unless forcing).
4. Changelog Integration: Consider auto-updating a CHANGELOG.md file as part of the release process.
5. Rollback Documentation: Document what to do if a release fails partway through.

🎯 Summary

Overall Assessment: This is a solid refactoring that improves the release process. The consolidation is a good move, and the phase-based approach is cleaner.

Risk Level: Medium - The force-push operations and potential race conditions need attention.

Recommendation: Approve with minor changes suggested. The commit length assertion message and duplicate builds should be fixed before merging.

---

Review generated with assistance from Claude Code

[claude]

Code Review - Part 1: Overview and Strengths

Overview

This PR consolidates the release scripts by merging the rivetkit release functionality into the main release script. The refactoring improves maintainability by eliminating duplicate release logic and providing a more unified approach to releasing both the main Rivet project and rivetkit packages.

Key Changes

1. Consolidated Release Script: Merged scripts/rivetkit/release.ts (555 lines) into scripts/release/main.ts with a cleaner phase-based architecture
2. Phase-based Architecture: Introduced phases (setup-local, setup-ci, complete-ci) that map to individual steps
3. Enhanced SDK Publishing: Updated sdk.ts to handle both engine SDKs and rivetkit packages
4. Git Operations: Extracted git operations into a dedicated git.ts module
5. Improved GitHub Actions: Updated workflow to use new CLI flags and added NPM authentication

✅ Strengths

1. Architecture Improvements

• Phase abstraction makes it easier to understand the release workflow stages
• Step granularity allows running specific parts of the release process independently
• Separation of concerns with dedicated modules

2. Code Quality

• Added proper type safety with TypeScript types
• Good use of assertion functions for validation
• Consistent error handling with clear messages

3. Publishing Logic

• Smart version checking with npmVersionExists() prevents duplicate publishes
• Proper handling of release candidates with --tag rc flag
• Builds rivetkit packages before publishing

4. Git Operations

• Force tag updates allow re-running releases if needed
• Supports both main branch and Graphite workflow
• GitHub release creation handles both new and existing releases

[claude]

Code Review - Part 2: Issues and Concerns

Critical Issues

1. Type Safety Bug: Assertion Length Check

Location: scripts/release/main.ts:271

Bug: Error message says 8 char but checks for length 7. This is inconsistent.

Fix: Either change to 8 or update the message to 7 char short commit.

2. Security: Force Push Operations

Location: scripts/release/git.ts:22

Issue: Force pushing tags can overwrite existing tags, which could be dangerous if someone has already pulled the tag.

Recommendation: Consider adding a confirmation prompt or making force push opt-in via a flag, especially for non-prerelease versions.

3. Error Handling: Version Regex

Location: scripts/release/main.ts:243-247

Issue: Hard-coded regex instead of using the semver library which is already a dependency.

Recommendation: Replace with semver.valid(version) for consistency and reliability.

4. Workflow: Missing NPM Auth Validation

Location: .github/workflows/release.yaml:250-252

Issue: No validation that NPM_TOKEN exists or is valid before attempting to publish.

Recommendation: Add a check to verify the token exists before proceeding with the release.

5. Code Duplication: Publishing Logic

Location: scripts/release/sdk.ts:76-105 and 108-131

Issue: Nearly identical code blocks for publishing engine SDKs and rivetkit packages.

Recommendation: Extract common publishing logic into a helper function to reduce duplication.

6. Hardcoded Release Candidate Detection

Issue: Only detects -rc. pattern. What about -alpha, -beta, or other prerelease formats?

Recommendation: Use semver.prerelease(opts.version) for prerelease detection instead of string matching.

7. Git Validation Skip Logic

Location: scripts/release/main.ts:273-276

Issue: The comment acknowledges this is a hack. This suggests a deeper issue that should be investigated.

Recommendation: Investigate why CI introduces changes and fix the root cause.

[claude]

Code Review - Part 3: Testing, Performance, and Recommendations

Testing Concerns

No Unit Tests

• No test files found for the release scripts
• Complex logic in updateVersion(), npmVersionExists(), etc. should be tested

No Integration Tests

• Release workflow is critical infrastructure
• Consider adding tests that validate the workflow phases work correctly

Recommendation: Add tests for:

• Version bump calculations
• Version validation
• NPM version existence checks
• File replacement logic in updateVersion()

Performance Considerations

Sequential Package Publishing

Location: scripts/release/sdk.ts

Issue: Packages are published sequentially. With many packages, this could be slow.

Recommendation: Consider publishing packages in parallel using Promise.all() or with a concurrency limit.

Additional Recommendations

1. Add Dry Run Mode - Add a --dry-run flag that shows what would be done without actually executing

2. Add Rollback Mechanism - For failed releases, provide a way to rollback changes:

   • Delete created tags
   • Unpublish packages (if within 72 hours on npm)
   • Revert version commits

3. Validate Workspace Versions - Before publishing, ensure all workspace packages have consistent versions to avoid dependency issues

4. Add Release Notes Template - The GitHub release uses --generate-notes, but a custom template could provide more structured changelog

5. Add JSDoc Comments - Functions like publishSdk(), createAndPushTag(), etc. would benefit from JSDoc comments explaining parameters and behavior

6. Missing Error Context - In scripts/release/sdk.ts:19-35, errors are silently swallowed. Log warnings for unexpected errors even if returning false.

[claude]

Code Review - Part 4: Summary

Summary Ratings

Code Quality: 7/10

• Well-structured with good separation of concerns
• Some duplication and hardcoded logic that could be improved

Security: 6/10

• Force push operations need safeguards
• Missing NPM token validation

Maintainability: 8/10

• Much better than having separate scripts
• Phase-based approach is clean and extensible

Testing: 2/10

• No test coverage for critical release infrastructure

Overall: This is a solid refactoring that consolidates release logic effectively. The main concerns are around safety mechanisms for destructive operations, test coverage, and a few bug fixes needed. The phase-based architecture is a significant improvement over the previous approach.

---

Required Fixes

1. Fix assertion message mismatch (line 271 in main.ts) - says 8 char but checks for 7
2. Add safeguards for force push operations
3. Use semver.valid() instead of hardcoded regex for version validation
4. Add NPM token validation in workflow

Recommended Improvements

1. Extract duplicate publishing logic into a helper function
2. Use semver.prerelease() for prerelease detection instead of string matching
3. Add unit tests for critical functions
4. Add dry-run mode for safer testing
5. Add JSDoc comments to public functions
6. Investigate and fix the git validation hack (line 273-276)
7. Add parallel publishing for better performance
8. Add rollback mechanism for failed releases

---

Great work on consolidating this! The codebase will be easier to maintain with this unified approach. The phase-based design is particularly well done - it makes the release process much more transparent and flexible. 🚀

[claude]

PR Review: Release Script Refactoring

I've reviewed this PR and found several improvements along with a few areas that need attention.

Strengths

1. Better Script Organization: The release script has been significantly refactored with clear separation of concerns
2. Duplicate Publishing Fix: The fix in sdk.ts:38-50 properly excludes @rivetkit/engine-* packages from the rivetkit packages list
3. NPM Authentication Fix: Added NPM authentication to the complete job in the GitHub Actions workflow
4. Improved Type Checking: Building rivetkit packages before type checking ensures examples that depend on rivetkit can be properly checked
5. Better Error Handling: The release script now has more robust validation and clearer error messages

Critical Issues

1. Force Push on Git Tags (git.ts:22) - Force pushing tags is dangerous and can cause issues for users who have already pulled the tag. Recommendation: Only use force push when explicitly requested or during RC releases.

2. Assertion on Commit Length (main.ts:271) - Error message says '8 char' but assertion checks for 7 characters. Fix: Change message to 'must use 7 char short commit'

3. Version Validation Logic Flaw (main.ts:273-276) - Git validation is skipped when running run-type-check step due to a HACK comment. This suggests there's an underlying issue that should be properly diagnosed.

Medium Priority Issues

4. Missing Deno Check in CI - The example-deno package now skips type checking when deno is not installed. Consider installing deno in the CI environment instead.

5. Inconsistent Error Handling in npmVersionExists (sdk.ts:19-35) - The function catches errors and checks stderr for specific strings, but this is fragile. Consider using npm view --json for more reliable parsing.

6. Missing Validation for --override-commit - No validation that the provided commit actually exists in the repository.

Security Considerations

• NPM Token Exposure: Properly handled through GitHub secrets
• Git Operations: Properly guarded by the validateGit flag

Testing Recommendations

1. Adding a dry-run mode
2. Testing with different version bump scenarios
3. Testing the duplicate publishing prevention thoroughly
4. Validating the behavior when tags already exist

Minor Suggestions

• Version Number Inconsistency: The Cargo workspace version changed from 25.8.2 to 2.0.22-rc.1. Please verify this is correct.
• Documentation: Consider adding a README in scripts/release/ explaining the release process

Overall, this is a solid refactoring that addresses real issues. The main concerns are around the force push behavior and the git validation hack, which should be addressed before merging.

[claude]

Pull Request Review

Overview

This PR refactors the release scripts to consolidate rivetkit and engine package publishing into a unified process. The changes include:

• Consolidating release scripts from separate workflows into a single main.ts
• Fixing CI/CD issues around NPM authentication and type checking
• Preventing duplicate publishing of engine packages
• Simplifying the justfile release commands

Code Quality & Best Practices

Positive:

• Good modularization with separate files for git, sdk, and artifact operations
• Proper use of TypeScript with clear types and interfaces (ReleaseOpts, Step, Phase)
• Clear phase-based workflow (setup-local, setup-ci, complete-ci)
• Idempotent NPM publishing - checks if versions exist before publishing
• Proper handling of release candidates with -rc tagging
• Good error handling and descriptive console output

Concerns:

1. scripts/release/main.ts:271 - Off-by-one error in assertion message. The error message says 8 char but checks for length 7. Based on line 268 which slices to 7 chars, the message should say 7 char or the check should be 8.

2. scripts/release/git.ts:19 - Force pushing tags without confirmation. This can cause issues for users who have already pulled those tags. Consider adding a warning or requiring explicit confirmation.

3. scripts/release/sdk.ts:48-49 - The engine package exclusion logic fixes the duplicate publishing bug, which is good. However, consider extracting this filter logic into a named function for clarity.

4. examples/deno/package.json:9 - Shell command in package.json script. While this works, consider using a small wrapper script in scripts/ for better cross-platform compatibility.

5. scripts/release/main.ts:273-275 - The HACK comment suggests there is an underlying issue that should be investigated. The condition is also inverted (validate when NOT running type check), which is counterintuitive.

Potential Bugs

1. Version mismatch in Cargo.toml: The workspace version was changed from 25.8.2 to 2.0.22-rc.1. This is a major version downgrade. Is this intentional? If this is a versioning scheme change, it should be documented.

2. Missing error handling for gh commands: In main.ts lines 316-322, if the gh CLI is not installed or authenticated, the workflow will fail without a helpful message.

3. Race condition potential: The publish-sdk step checks if versions exist on NPM, but there is a gap between checking and publishing where another process could publish the same version.

Performance Considerations

1. Sequential package publishing - All packages are published sequentially. Consider using Promise.all() to publish multiple packages in parallel.

2. Building rivetkit twice - The build happens in both runTypeCheck (line 69) and publishSdk (line 61). Consider caching the build output.

Security Concerns

1. NPM Token exposure: Standard practice but ensure the token is properly scoped (publish-only, not full access).

2. Force push operations: Could potentially overwrite legitimate releases if the script is misused. Consider adding safeguards.

3. No validation of package contents: Before publishing, there is no verification that the built packages contain expected files or that no sensitive data is included.

Test Coverage

No tests found for the release scripts. Consider adding tests for version parsing, phase-to-step mapping, NPM version existence checking, and package filtering logic.

Recommendations

1. Fix the off-by-one error in the commit length assertion message
2. Add tests for the release script logic
3. Remove the HACK - investigate and fix the root cause of git validation failures in CI
4. Parallelize package publishing to improve release performance
5. Document the versioning scheme change if intentional
6. Consider a dry-run mode for testing

Conclusion

This is a solid refactoring that improves the release process. The main concerns are around the off-by-one error, the HACK comment indicating unresolved issues, and the lack of tests. Overall, the changes align well with the repository patterns and represent a good step forward for the release automation.

Recommendation: Approve with minor changes requested (fix the commit length assertion message and investigate the HACK).

[claude]

PR Review: Release Script Consolidation

Summary

This PR consolidates the release process by integrating RivetKit releases into the main release script, removing the separate scripts/rivetkit/release.ts script and simplifying the workflow. The changes reduce code duplication (93 files changed, +501/-752 lines) and standardize the release process across all packages.

Code Quality & Architecture

Strengths:

1. Excellent Refactoring: The phase-based approach (setup-local, setup-ci, complete-ci) provides much better modularity than the previous --setupLocal, --setupCi, --completeCi flags.

2. Improved Maintainability: Consolidating RivetKit release logic into the main release script eliminates duplication and reduces the maintenance burden. The new getRivetkitPackages() function in sdk.ts is clean and reusable.

3. Better Error Handling: The npmVersionExists() function properly handles different error cases and provides clear error messages.

4. Consistent CLI Interface: Moving from camelCase flags to kebab-case is more conventional and consistent.

Potential Issues

Bug - Incorrect Assertion Message:

// scripts/release/main.ts:271
assertEquals(releaseOpts.commit.length, 7, "must use 8 char short commit");

The error message says "8 char" but the check validates for 7 characters. This should be "must use 7 char short commit".

Potential Logic Issue - Comparison Operator:

// scripts/release/main.ts:267
if (releaseOpts.commit.length == 40) {

Should use strict equality (===) instead of loose equality (==) for consistency with TypeScript best practices.

[claude]

Pull Request Review

This PR refactors the release script to consolidate rivetkit releases into the main release process. Overall, the changes are well-structured and improve maintainability by reducing code duplication.

✅ Strengths

1. Good Consolidation: Successfully merges separate rivetkit release logic (555 lines deleted) into the unified release system
2. Improved Phase Architecture: The new phase-based approach provides better separation of concerns
3. CLI Improvements: Better argument naming is more intuitive and extensible
4. Idempotent NPM Publishing: The npmVersionExists check prevents duplicate publishes gracefully
5. Smart Rivetkit Package Discovery: Dynamically discovers packages instead of hardcoding them

🐛 Potential Issues

Critical

1. Unsafe Git Force Push (git.ts:22)
Force pushing tags is dangerous in a release workflow and can cause issues for users who have already pulled the tag.

• Recommendation: Remove -f flag. If tag already exists, fail with clear error message
• Per CLAUDE.md: NEVER run destructive/irreversible git commands unless explicitly requested

2. Incorrect Error Message (main.ts:271)
Error message says "must use 8 char short commit" but checks for 7 characters - this is confusing

• Recommendation: Fix message to match the check

Medium Priority

3. Phase Resolution: Error messages could be more helpful with examples
4. Version Validation: Regex patterns in error messages are hard to read
5. Git Performance: Git operations run sequentially per file instead of batching

Low Priority

6. Package Build Verification: No verification that expected build artifacts exist before publish
7. Hardcoded Paths: Engine package paths are hardcoded while rivetkit packages are dynamically discovered

🔒 Security: All Good ✅

• NPM token handling is secure
• Git credentials properly configured
• No secrets in logs

Summary

This is a solid refactoring. Main concerns:

1. Must fix: Remove force push flag in git.ts:22
2. Should fix: Correct error message in main.ts:271
3. Nice to have: Add dry-run mode and better error context

Overall Assessment: ✅ Approved with minor changes requested

[claude]

Missing Validation:

The shouldRunStep() check at line 273 could be confusing:

// scripts/release/main.ts:273-276
if (opts.validateGit && !shouldRunStep("run-type-check")) {
    // HACK: Skip setup-ci because for some reason there's changes in the setup step but only in GitHub Actions
    await validateGit(releaseOpts);
}

The comment mentions it's a HACK - this logic seems fragile and could cause issues if the phase system is extended. Consider investigating why there are changes during setup-ci rather than working around it.

Security Considerations

Good Security Practices:

1. Secrets Handling: NPM_TOKEN is properly handled via .npmrc file creation in the GitHub workflow
2. Git Validation: The validateGit() function checks for uncommitted changes before proceeding
3. Force Push Protection: Git tags are force-pushed (-f flag), which is acceptable for release tags but the git.ts implementation should include a comment explaining why this is safe

Note on Force Operations:

Force pushing tags is potentially dangerous. Consider adding validation to ensure this only happens for RC versions or with explicit confirmation. The GitHub release creation logic already handles RC versions specially (line 68 in git.ts), so consider applying similar logic here.

Performance Considerations

Good Performance Patterns:

1. Early Version Check: The npmVersionExists() check prevents unnecessary builds when packages are already published
2. Parallel Builds: The GitHub workflow matrix strategy for binaries and Docker images runs builds in parallel
3. Build Caching: Using --force flag for type checks is good for avoiding Turborepo cache bugs during releases

Potential Optimization:

The getRivetkitPackages() function calls pnpm -r list --json which lists all workspace packages. For large workspaces, consider caching this result or using a glob pattern instead.

[claude]

Test Coverage

Missing Tests:

No test files are included in this PR. Given the critical nature of release scripts, consider adding:

1. Unit tests for version bumping logic
2. Integration tests for the phase system
3. Tests for error handling in npmVersionExists()
4. Mock tests for git operations

The release process is high-risk - failures could result in broken releases or version conflicts.

GitHub Workflow Changes

Workflow Improvements:

1. NPM Authentication: Added to the complete job (line 251-254), which is necessary for SDK publishing
2. GH_TOKEN: Properly added to environment variables (line 249) for GitHub CLI operations
3. Consistent Phase Usage: Both setup and complete jobs now use the phase system consistently

Recommendations

1. Fix the assertion message (8 char → 7 char)
2. Use strict equality (=== instead of ==)
3. Investigate the git validation HACK and either fix the root cause or add better documentation
4. Add integration tests for the release process
5. Document force push behavior in git.ts with comments explaining when it's safe
6. Consider adding a dry-run mode for testing release scripts without actually publishing

Minor Observations

• The justfile changes simplify the interface nicely - good UX improvement
• All 70+ SDK files show only version string updates (25.8.1 → 25.8.2) - this is expected for generated code
• Package.json updates for rivetkit packages show proper version synchronization

Conclusion

Overall Assessment: Approved with minor suggestions

This is a solid refactoring that significantly improves the release process. The code is well-structured and the phase-based approach is a clear improvement. The identified issues are minor and can be addressed in a follow-up PR if desired. The consolidation of RivetKit releases into the main script eliminates a source of potential divergence and makes the system easier to maintain.

The biggest risk is the lack of automated tests, but that's a broader issue with the existing release infrastructure rather than something introduced by this PR.

Recommendation: Merge after addressing the assertion message bug, or create a follow-up issue to track the test coverage improvements.