fix(release): leave Cargo.lock alone and move the skill pins in the version step - #1028
Conversation
|
freshtonic
left a comment
There was a problem hiding this comment.
The change is correct, and the tests cover it well. Step 3 now runs cargo only for a lock at another version. The check after cargo makes a failed refresh stop the release before the SQL build. The argument for --workspace instead of --package is clear in the comment and in the tests. The mutation table in the description also helps.
No changeset is necessary, because this change touches only repo tooling under scripts/. No skill is affected.
Follow-up (not in the diff). The failure message in scripts/__tests__/cargo-lock-freshness.test.mjs (lines 231–234 at 2c5f036) still tells a developer to refresh a stale lock with cargo update --package <crate>. This PR shows that this command rewrites unrelated windows-sys edges. A developer who follows that message gets the churn that this PR removes from the release path. Please change the message to cargo update --workspace --manifest-path <workspace>/Cargo.toml, in this PR or in a follow-up. The -p in the next sentence of that message needs the same change.
Nits (they do not block the merge):
scripts/__tests__/sync-lockstep-versions.test.mjs:366: the barecatchin "runs no cargo over this tree at its own EQL version" hides every error, not only the after-check error. IfcargoLockWorkspacesfinds no lock and throws,cargo.callsstays[]and the test passes without testing anything. The discovery test above holds that minimum, but a narrower catch (rethrow unless the message matches/cargo update ran in/) makes this test fail for the correct reason only.scripts/__tests__/sync-lockstep-versions.test.mjs:707: inrunWithLock, thermSyncis not in afinally. If areadFileSyncabove it throws, the temp directory stays. Atry/finally, as in the other fixtures in this file, prevents that.
2c5f036 to
c32b5bd
Compare
…ange On every release, scripts/sync-lockstep-versions.mjs ran `cargo update --package eql-bindings` in each workspace that locks eql-bindings from a path, even when the version had not moved. In Version Packages #1020 that rewrote one unrelated line in protect-ffi's Cargo.lock: winapi-util's windows-sys edge moved from 0.48.0 to 0.52.0. Two changes: - Step 3 now reads the version each lock records for eql-bindings, and runs cargo only for a lock at another version. A release that does not bump EQL runs no cargo and leaves every Cargo.lock as it was. After cargo runs, the step fails if the lock still disagrees. - The refresh uses `cargo update --workspace` in place of `--package eql-bindings`. With `--package`, cargo re-picks every edge whose requirement spans several semver-incompatible windows-sys versions (winapi-util 0.1.11 asks for >=0.48.0, <=0.61), and each run moves them again. Every pick is valid, so `--locked` passes either way. `--workspace` reads a path dependency's version from its manifest and keeps every other locked edge. The toolchain is not the cause. cargo 1.90.0, 1.94.1 and 1.99.0, the version the release job used, all reproduce #1020's lock exactly from the lock before it. With each of them, `--workspace` changes only the eql-bindings version line, with or without a bump. Refs: CIP-4285 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PaY5xYydZUWhv8Nex9Sw8a
The skills ship inside the stash tarball, and nothing rewrites the version pins in them. People moved them by hand in each Version Packages PR: #928 and #938 did, and #1020 did not, because no CI ran on it. main then failed release-train.test.ts until #1029. scripts/sync-skill-pins.mjs now runs from the root `version` script, right after `changeset version`. It rewrites every exact pin of a release-train package in skills/ to the stable version in the tree, so each Version Packages PR carries the pins with the versions they name. The packages are the changesets `fixed` group that holds `stash`, which a test holds equal to the CLI's RELEASE_TRAIN_MANIFESTS. Run on main's stale pins, the script makes exactly the change in #1029. Refs: CIP-4285 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PaY5xYydZUWhv8Nex9Sw8a
c32b5bd to
6de9b46
Compare
The main-guard comment in scripts/sync-lockstep-versions.mjs and the header of scripts/__tests__/script-main-guards.test.mjs both quoted the root `version` script as `changeset version && node scripts/sync-lockstep-versions.mjs`. Since #1028 it also runs scripts/sync-skill-pins.mjs between the two. Quote it as it is. Both comments keep their point: the `&&` chain reads a silent exit 0 as a completed bump. Refs: CIP-4285 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PaY5xYydZUWhv8Nex9Sw8a
This pull request does part 3 of Linear issue CIP-4285, and makes the version step move the skill version pins. A release that does not bump EQL now leaves every
Cargo.lockunchanged, and every Version Packages pull request now carries the skill pins. Parts 1 and 2 need decisions from an admin, so they are described below and left for a follow-up.This branch is based on pull request #1029, which moves the skill pins to 1.2.1 by hand. Without it,
release-train.test.tsfails here as it does onmain. Merge #1029 first, and this pull request then shows only its own changes.The version script no longer runs cargo for a lock that is already current
scripts/sync-lockstep-versions.mjsruns on every release, as the second half of the rootversionscript. Its step 3 rancargo update --package eql-bindingsin each workspace whoseCargo.lockrecordseql-bindingsfrom a path. It ran even when the EQL version had not moved. In Version Packages pull request #1020, that rewrote one unrelated line inlanguages/typescript/packages/protect-ffi/Cargo.lock.Step 3 now reads the version that each lock records for
eql-bindings. It runs cargo only for a lock at another version, and logs the locks it leaves alone. After cargo runs, the step fails if the lock still records another version, before the slow SQL build starts.The windows-sys edge moves because of
--package, not the Rust toolchainThe Linear issue suggested that the Rust toolchain from
packages/eql/mise.tomlwas the cause. That file asks for the latest Rust, which was 1.99.0 on 2 October 2026, while the rootmise.tomlpins 1.94.1. I tested this, and the toolchain is not the cause.Some crates ask for a range of
windows-sysversions that crosses several semver-incompatible releases. For example,winapi-util0.1.11 asks for>=0.48.0, <=0.61. The lock holds fivewindows-sysversions, and any of four of them satisfies that range. Socargo metadata --lockedpasses whichever one the lock names.cargo update --package eql-bindingspicks those edges again, and it picks differently from the lock it starts with. Onmain's lock it rewrites three edges. Run on its own output, it rewrites them again, so it never settles. I replayed #1020 from the lock before it with cargo 1.90.0, 1.94.1 and 1.99.0, and all three produced the lock that #1020 committed.cargo update --workspacereads a path dependency's version from its manifest and keeps every other locked edge. With each of the three cargo versions, in both workspaces, it changes nothing without a bump. With a bump to 3.0.7, it changes only theeql-bindingsversion line. Step 3 now uses--workspace. I did not trace the--packagebehaviour to a line in cargo's source.The version step now moves the skill version pins
stashcopiesskills/into its npm tarball, and the build does not rewrite the version pins inside it. People moved the pins by hand in each Version Packages pull request: #928 and #938 did, and #1020 did not, because no CI ran on it. Since then,mainhas failedrelease-train.test.ts.scripts/sync-skill-pins.mjsnow runs from the rootversionscript, right afterchangeset version. It rewrites each exact pin of a release-train package inskills/to the stable version in the tree. For example, it rewritesnpx --package=stash@1.2.0andnpm:@cipherstash/stack@1.2.0/wasm-inline. A range such as^1.0.0and a package outside the train, such as@cipherstash/eql@3.0.6, stay as they are.The release train is the Changesets
fixedgroup that holdsstash. A test holds that group equal toRELEASE_TRAIN_MANIFESTSin the CLI, which is the set thatrelease-train.test.tschecks. During a prerelease, a pin takes the stable version, as that test requires. I ran the script onmain's stale pins, and it made exactly the change in #1029.Its tests check the three forms the skills use, a prerelease, the pins it must leave alone, and a second run that changes nothing. A guard also fails when the committed pins differ from the tree's versions, and a test checks the order of the root
versionscript.Tests cover the skip, the refresh and the cargo arguments
--workspace, and passes neither--packagenor-p.misereplaced onPATHby a recorder. One shows a current lock untouched, and the other shows a lock one version behind refreshed with--workspace.I also ran step 3 with real cargo 1.94.1 against this tree. With no bump, it ran no cargo and changed nothing. With
eql-bindingsbumped to 3.0.7, both locks changed only theireql-bindingsversion line.Each new test fails when the code it covers breaks
A mutation check breaks the code on purpose and confirms that a test fails. I ran 12 mutations, and each one failed at least one test.
--package eql-bindingseql-bindingscounts as a path entrystashinside a longer namefixedgroupversionscript skips the pin syncThe first mutation run found a fault in one of my tests. The test over this tree used a fake cargo that writes the lock it is given. Under a mutation, it overwrote the real
protect-ffilock. That test now uses a fake that writes nothing.Checks
pnpm test:scripts: 63 files and 1,164 tests pass, with 1 skipped as onmain.pnpm run code:check: no errors, and no new warnings.Part 1 needs a GitHub App that this repository does not have yet
changesets/actionpushes the Version Packages branch withGITHUB_TOKEN, and GitHub starts no workflows for that push. An installation token from a GitHub App fixes this. This repository has no App credential: no workflow usesactions/create-github-app-token, and its secrets and variables hold none. I have not wired one in, because the release would fail without the credential.The
cipherstash/envelopersandcipherstash/vitamincrepositories already use a GitHub App for release-plz. Its ID is 5040119, and those repositories keep it in the variableRELEASE_PLZ_APP_CLIENT_IDand the secretRELEASE_PLZ_APP_PRIVATE_KEY. An admin can install that App on this repository, or create a new one. The App needs these repository permissions:The follow-up change then adds a SHA-pinned
actions/create-github-app-tokenstep to thereleasejob and passes its token tochangesets/action. It keepscommitMode: 'github-api', so GitHub still signs the commits. The new action also needs an entry in the allowlist ofscripts/lint-no-workflow-caching.mjs.Part 2 is a choice between three rules
The
mainruleset requires one approval, and it hasdismiss_stale_reviews_on_pushandrequire_last_push_approvalturned off. It does haverequire_extra_approval_for_unattributed_changesturned on, but that rule does not help here. It acts on commits that no GitHub account authored, andgithub-actions[bot]authors the Version Packages commits.main, so authors who push after approval need a new review.changeset-release/mainbranch, it fails unless an approval names the head commit; for other branches, it passes at once. The rule applies only to the Version Packages pull request. It must be a required check, and it must report on every pull request, or it blocks them. Without part 1, the bot's push leaves the check missing on the new head, so the pull request stays blocked until somebody approves it again.Linked issues
This PR fixed part 3 of #1044, and added the skill pin sync.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PaY5xYydZUWhv8Nex9Sw8a