fix(release): support version-based engine reuse and fix Cargo.toml indent matching#4288
Conversation
|
🚅 Deployed to the rivet-pr-4288 environment in rivet-frontend
|
How to use the Graphite Merge QueueAdd 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. |
PR Review: fix(release): support version-based engine reuse and fix Cargo.toml indent matchingSummaryThis PR adds support for reusing version-tagged Docker manifests and S3 artifacts (not just commit-tagged ones), fixing a chain-release scenario where a previous release itself reused artifacts from an even earlier release. It also fixes a Cargo.toml regex that was failing when the workspace.package section had leading whitespace. The overall approach is sound. The separation between version-based and commit-based artifact paths is clear and consistent across docker.ts, promote-artifacts.ts, and main.ts. Bug: Docker image name mismatchThe validateReuseVersion function in main.ts checks rivetkit/engine, but the REPOS array in docker.ts publishes to rivetdev/engine. The validation would always fail (or silently pass against the wrong registry) at runtime. Worth confirming whether rivetkit/engine is an intentional org migration or an oversight. Minor: Misleading parameter name in promotePathAfter this PR, the sourceCommit parameter in promotePath can hold a version string like 2.0.33, not just a commit hash. Renaming to sourcePrefix or sourceRef would match the call-site variable name and avoid confusion. Positive: --latest auto-detectionRemoving the hardcoded true default and replacing it with shouldTagAsLatest auto-detection is a good improvement. The guard if opts.latest is not undefined correctly handles explicit --latest/--no-latest overrides while defaulting to semver-based auto-detection. Positive: Cargo.toml regex fixThe updated regex captures leading whitespace in a capture group and restores it via backreference in the replacement string. The prior regex would silently strip indentation when workspace.package appeared inside a nested TOML context with leading whitespace. Observation: Version vs commit detection heuristicUsing .includes('.') to distinguish version strings from commit hashes is applied consistently across all three files and matches the existing versionOrCommitToRef utility. Works correctly for the expected inputs (2.0.33 vs bb7f292). A semver regex would be more defensive but is likely overkill for this controlled context. VerdictThe architecture is correct and the refactor is clean. The main issue worth addressing is the Docker org name mismatch (rivetkit/engine vs rivetdev/engine) in validateReuseVersion. |

Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: