ci: keep Cargo.lock in sync through releases and build --locked (#242)

[beardthelion]

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:

1. Sync Cargo.lock to the 0.6.0 manifests (cargo update --workspace). The diff is the five crate version fields plus a base64 dep edge for gl that the lock had also dropped. cargo metadata --locked passes after, fails exit 101 before.
2. Build --locked in PR CI (clippy, test, release build, MSRV check, Windows test), so any future manifest/lock drift fails loudly at PR time instead of surfacing as mystery-dirty trees.
3. A sync-release-lock job in release.yml: while a release PR is open, check out its branch (release-please's pr output, headBranchName), run cargo update --workspace, and push the lock bump onto the release branch. The release PR then carries its own lock sync, so the tagged tree is internally consistent and its --locked CI passes.

Two caveats, stated as such because a workflow change is only proven by its runs:

• The sync job pushes with GITHUB_TOKEN. Release-please's own branch pushes do trigger pr-checks in this repo (chore(main): release 0.7.0 #240's head has a pull_request run), and the sync push uses the same mechanism, but the first release cycle after this merges is the real proof. If the sync commit fails to re-trigger checks on the release PR, the job needs an app or PAT token instead.
• Merge order vs chore(main): release 0.7.0 #240: this branch syncs the lock to 0.6.0. If the 0.7.0 release merges first, I will rebase and re-run the sync so the committed lock matches 0.7.0 before this lands; merging this first is simpler (the sync job then fixes chore(main): release 0.7.0 #240's branch automatically on its next refresh, once release-please force-pushes it against the new main).

Summary by CodeRabbit

• Chores
  • Improved build and test reliability by enforcing Cargo lockfile consistency across CI checks.
  • Updated PR-checks to support on-demand runs for release-branch heads, improving coverage despite automation limits.
  • Enhanced release preparation by syncing Cargo.lock with manifest version bumps and committing it only when it changes.
  • Improved release workflow coordination by determining the current release PR branch and triggering PR checks on it.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7f5700a2-dd65-460e-8fc7-4943c293140b

📥 Commits

Reviewing files that changed from the base of the PR and between b835d2f and 5091f2d.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• .github/workflows/pr-checks.yml
• .github/workflows/release.yml

---

📝 Walkthrough

Walkthrough

The release workflow synchronizes Cargo.lock on release PR branches, then dispatches pull-request checks. Cargo CI commands now require the existing lockfile across formatting, tests, builds, MSRV, and Windows validation.

Changes

Cargo lock consistency

Layer / File(s)	Summary
Release PR lock synchronization .github/workflows/release.yml	Release automation resolves the release PR branch, runs cargo update --workspace, conditionally commits and pushes Cargo.lock, and dispatches pr-checks.
Locked Cargo CI gates .github/workflows/pr-checks.yml	The workflow supports manual dispatch, and formatting, testing, release builds, MSRV checks, and Windows tests invoke Cargo with --locked.

Estimated code review effort: 3 (Moderate) | ~20 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

Loading

Possibly related PRs

• Gitlawb/node#57 — Modifies the same PR checks workflow and overlapping Cargo CI jobs.
• Gitlawb/node#130 — Changes release-please configuration and Cargo manifest version updates used by the release flow.
• Gitlawb/node#237 — Modifies the Windows shipped-crates test lane updated here.

Suggested labels: workflow-change

Suggested reviewers: jatmn

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the change, but it doesn't follow the required template sections or include the reviewer verification block.	Add the template sections (Summary, Motivation & context, Kind of change, What changed, How to verify) and fill in the checklist.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: keeping Cargo.lock synced and enforcing --locked CI during releases.
Linked Issues check	✅ Passed	The workflows add a release lock-sync job and add --locked to PR CI jobs, matching the core requirements of #242.
Out of Scope Changes check	✅ Passed	The changes stay focused on release and CI workflow updates; no unrelated code churn is evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ci/242-lock-sync-release

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

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

📥 Commits

Reviewing files that changed from the base of the PR and between b484a24 and 42f1d8f.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• .github/workflows/pr-checks.yml
• .github/workflows/release.yml

[kevincodex1]

hello bro @beardthelion please rebase to main and kindly address some coderabbit feedback

[coderabbitai]

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

📥 Commits

Reviewing files that changed from the base of the PR and between 42f1d8f and b835d2f.

📒 Files selected for processing (2)

• .github/workflows/pr-checks.yml
• .github/workflows/release.yml

[jatmn]

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-merged v0.7.0 release and is now CONFLICTING/DIRTY with main. A three-way merge retains main's 0.7.0 manifests but applies this branch's Cargo.lock workspace records for 0.6.0; a full cargo metadata --locked on that merge result exits 101. Rebase onto current main and regenerate Cargo.lock for 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 new workflow_dispatch trigger. 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 --locked validation. 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 fresh pr output, the fallback chooses the first open PR whose branch name merely starts with release-please--. Any fork can open a lookalike branch; actions/checkout then 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.

[beardthelion]

All three findings were real. Rebased onto current main and pushed; the branch is now 0 behind.

[P2] Rebase and regenerate the lockfile ("a full cargo metadata --locked on that merge result exits 101")

Reproduced exactly. On the rebased tree, main's 0.7.0 manifests against lock records still at 0.6.0:

$ cargo metadata --locked
error: cannot update the lock file ... because --locked was passed
exit=101

Regenerated with cargo update --workspace, the same command the sync job runs. Only the 5 workspace crates move 0.6.0 -> 0.7.0 and cargo reports 252 unchanged dependencies, so the lock diff is 5 insertions / 5 deletions and every line is a version =. This is a version resync, not a dependency bump. cargo metadata --locked now exits 0, and both gates this PR adds pass: cargo build --locked --workspace and cargo check --locked --workspace --all-targets.

The only rebase conflict was pr-checks.yml, where main's new core-deps-purity job landed on the line this branch adds --locked to. Both sides kept.

[P1] Fail when the release branch cannot dispatch PR checks

Agreed, and the tolerance is gone rather than narrowed. Your reasoning is the part I had wrong: I assumed release-please would rewrite the branch and heal it, but a rewrite only happens when a later releasable commit lands, so an open release PR nobody touches is never healed, and the tolerated path left the lock-sync commit the job had just pushed sitting on the release head with no --locked run. That is the exact state the job exists to prevent.

Two things make removing it clean rather than risky. There is no open release-please PR right now, so nothing is stranded by the stricter behavior (round one observed #240 in exactly that state; it has since merged). And once this lands, every release branch is cut from a main that already carries the trigger, so the 422 has no remaining source. The step is now a bare gh api ... /dispatches; confirmed under bash -e that a failing call aborts the step rather than falling through.

[P1] Restrict fallback discovery to the repository-owned release PR

Confirmed, including that it is exploitable and not just untidy. Driving the old expression against hostile listings, it selects the attacker's branch in every case:

old expr, fork lookalike only        -> release-please--branches--main
old expr, fork listed before real PR -> release-please--branches--main
old expr, same-repo human branch     -> release-please--branches--main

Discovery now requires the head repo to be this repository and the author to be the release-please app, and selects max_by(.number) instead of [0], since gh's default ordering is not a documented guarantee and a second component release PR would otherwise make the choice flap between runs.

I tested the new filter using the jq program extracted from the parsed YAML, so it is the text CI will actually run, across 11 cases. Rejected: a fork lookalike, a fork PR claiming app/github-actions as its author, and a same-repo branch named release-please--* opened by a human. Still selected: the real release PR when a fork is listed ahead of it, and when an older component release PR is also open. Empty listing yields empty.

On the PAT half of CodeRabbit's request, which I am declining as a design call rather than a verified claim: HOMEBREW_TAP_PAT and WEB_SYNC_PAT are cross-repo credentials for the tap and the web sync, and reusing them for same-repo pushes widens their blast radius for a problem that failing loudly already closes. CodeRabbit accepted the same reasoning on the earlier thread at release.yml:70.

Two limits worth stating. shellcheck is not installed on my box, so actionlint's shellcheck-backed checks on the edited run: blocks were skipped rather than passed; actionlint 1.7.7 itself is clean on our lines, with its one finding (macos-15-intel) reproducing identically on main. And a workflow change is only really validated by its own CI run, so the discovery and dispatch behavior above is verified at the shell and jq level; the release path itself first executes on the next real release.

[jatmn]

Thanks for addressing the earlier review. I do not see any actionable issues on the current head.

Prior review

All three items from my CHANGES_REQUESTED review are fixed here:

• Cargo.lock is regenerated for the 0.7.0 manifests (cargo metadata --locked exits 0 on head).
• Dispatch failures are no longer tolerated; the 422 tolerance is gone.
• Fallback release-PR discovery is scoped to same-repo app/github-actions branches with deterministic max_by(.number) selection.

What I verified

• Cargo.lock workspace records match the five release-please crates at 0.7.0, including the restored gl → base64 edge.
• --locked is on the five PR cargo gates this PR claims: clippy, test, release build, MSRV, and Windows.
• sync-release-lock + workflow_dispatch dispatch wiring is structurally sound; ubuntu-latest ships Rust, so the sync job can run cargo update --workspace without a missing-toolchain failure.
• PR Checks, CodeQL, and CodeRabbit are green on 5091f2dba0f6957d6d9eb456e5f4270a0cc045e9.

Optional follow-ups (non-blocking)

These are consistency polish, not merge blockers for #242:

• [P3] Pin dtolnay/rust-toolchain in sync-release-lock before cargo update --workspace so the bot uses the same toolchain as pr-checks, rather than whatever Rust the runner image happens to ship.
• [P3] Extend --locked to release-binaries, Dockerfile, and docker-build in a follow-up if you want belt-and-suspenders at the shipping boundary; this PR's fix is the release-PR lock sync plus dispatched --locked validation, which is sufficient for the stated goal.
• [P3] Update the stale comment in scripts/check-gitlawb-core-deps.sh that says build/test resolve without --locked; that gate intentionally avoids --locked, and the new blocking cargo jobs already catch lock staleness.