ci: keep Cargo.lock in sync through releases and build --locked (#242)#243
ci: keep Cargo.lock in sync through releases and build --locked (#242)#243beardthelion wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughThe release workflow synchronizes ChangesCargo lock consistency
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ReleasePlease
participant SyncReleaseLock
participant Cargo
participant Git
participant PRChecks
ReleasePlease->>SyncReleaseLock: provide release PR branch
SyncReleaseLock->>Cargo: run cargo update --workspace
Cargo-->>SyncReleaseLock: update Cargo.lock
SyncReleaseLock->>Git: commit and push changed Cargo.lock
SyncReleaseLock->>PRChecks: dispatch checks on release branch
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8).github/workflows/pr-checks.ymlTraceback (most recent call last): .github/workflows/release.ymlTraceback (most recent call last): Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 67-70: Update the “Checkout release branch” step to authenticate
with a GitHub App or PAT credential instead of the default GITHUB_TOKEN,
ensuring the later lockfile git push triggers the PR and release-check
workflows. Preserve the existing release-branch ref and checkout action
configuration, and use the repository’s established secret or credential symbol.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 949615b0-4515-4819-b295-6614b973ca86
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/pr-checks.yml.github/workflows/release.yml
|
hello bro @beardthelion please rebase to main and kindly address some coderabbit feedback |
…till get validated
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 151-154: Update the workflow-dispatch error handling in the
release-please validation flow to fail when the target release branch lacks a
workflow_dispatch trigger instead of exiting successfully. Configure the
release-please and lockfile-push authentication to use a GitHub App or PAT,
ensuring branch updates emit an immediate synchronize check run and the current
Cargo.lock commit receives --locked validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c7ae8c7-f787-4e6a-a94f-4ffd78fc0e73
📒 Files selected for processing (2)
.github/workflows/pr-checks.yml.github/workflows/release.yml
| if echo "$out" | grep -qi "does not have a workflow_dispatch trigger"; then | ||
| echo "release branch predates the dispatch trigger; checks attach after release-please next rebases it" | ||
| echo "$out" | ||
| exit 0 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Do not allow a synchronized release branch to remain unchecked.
This accepted 422 leaves the current Cargo.lock commit without the required --locked validation. A dispatch uses the workflow at the requested ref, so a branch predating workflow_dispatch cannot self-validate until a later rewrite. Fail this path and use a GitHub App/PAT for release-please and lockfile pushes so the PR receives a synchronize check run immediately. (docs.github.com)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release.yml around lines 151 - 154, Update the
workflow-dispatch error handling in the release-please validation flow to fail
when the target release branch lacks a workflow_dispatch trigger instead of
exiting successfully. Configure the release-please and lockfile-push
authentication to use a GitHub App or PAT, ensuring branch updates emit an
immediate synchronize check run and the current Cargo.lock commit receives
--locked validation.
jatmn
left a comment
There was a problem hiding this comment.
I found issues that need to be addressed before this is ready.
Findings
-
[P2] Rebase and regenerate the lockfile from the current release state
.release-please-manifest.json:2
This is stale-base drift rather than an intentional rollback: the branch forked before the already-mergedv0.7.0release and is nowCONFLICTING/DIRTYwithmain. A three-way merge retains main's0.7.0manifests but applies this branch'sCargo.lockworkspace records for0.6.0; a fullcargo metadata --lockedon that merge result exits 101. Rebase onto currentmainand regenerateCargo.lockfor the current manifests before this can be merged. -
[P1] Fail when the release branch cannot dispatch PR checks
.github/workflows/release.yml:151
A release PR created before this change does not contain the newworkflow_dispatchtrigger. After this job synchronizes and pushes its lockfile, dispatching against that branch returns this exact 422; exiting 0 marks the job successful while the release head has no--lockedvalidation. A later release-please rewrite is not guaranteed for an unchanged open release PR, so this is precisely the checkless branch the workflow is intended to prevent. Let the 422 fail (or otherwise ensure the branch is updated and checked). This also leaves the current CodeRabbit request to fail this error unaddressed. -
[P1] Restrict fallback discovery to the repository-owned release PR
.github/workflows/release.yml:93
When release-please does not return a freshproutput, the fallback chooses the first open PR whose branch name merely starts withrelease-please--. Any fork can open a lookalike branch;actions/checkoutthen cannot resolve that fork-only ref in this repository (or can target an unrelated same-named ref), causing the write-token sync/dispatch job to fail and leaving the real release PR stale and unchecked. Filter by same head repository/owner and the expected release-please PR metadata, and select it deterministically.
Closes #242.
Every release-please PR bumps the workspace crate versions in the Cargo.tomls but never touches Cargo.lock, so each release merge ships a tag whose lockfile disagrees with its manifests. Verified on current main: all five workspace crates sit at 0.5.1 in the lock against 0.6.0 manifests, and cargo metadata --locked exits 101. Because CI never built --locked, the drift stayed green while release binaries were built from a resolver-fresh dependency set instead of the committed lock, and local builds kept dirtying the lock for contributors.
Three parts:
Two caveats, stated as such because a workflow change is only proven by its runs:
Summary by CodeRabbit