Skip to content

fix(release): leave Cargo.lock alone and move the skill pins in the version step - #1028

Merged
auxesis merged 2 commits into
mainfrom
fix/version-packages-lockfile
Oct 3, 2026
Merged

auxesis merged 2 commits into
mainfrom
fix/version-packages-lockfile

Conversation

@auxesis

@auxesis auxesis commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

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.lock unchanged, 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.ts fails here as it does on main. 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.mjs runs on every release, as the second half of the root version script. Its step 3 ran cargo update --package eql-bindings in each workspace whose Cargo.lock records eql-bindings from a path. It ran even when the EQL version had not moved. In Version Packages pull request #1020, that rewrote one unrelated line in languages/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 toolchain

The Linear issue suggested that the Rust toolchain from packages/eql/mise.toml was the cause. That file asks for the latest Rust, which was 1.99.0 on 2 October 2026, while the root mise.toml pins 1.94.1. I tested this, and the toolchain is not the cause.

Some crates ask for a range of windows-sys versions that crosses several semver-incompatible releases. For example, winapi-util 0.1.11 asks for >=0.48.0, <=0.61. The lock holds five windows-sys versions, and any of four of them satisfies that range. So cargo metadata --locked passes whichever one the lock names.

cargo update --package eql-bindings picks those edges again, and it picks differently from the lock it starts with. On main'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 --workspace reads 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 the eql-bindings version line. Step 3 now uses --workspace. I did not trace the --package behaviour to a line in cargo's source.

The version step now moves the skill version pins

stash copies skills/ 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, main has failed release-train.test.ts.

scripts/sync-skill-pins.mjs now runs from the root version script, right after changeset version. It rewrites each exact pin of a release-train package in skills/ to the stable version in the tree. For example, it rewrites npx --package=stash@1.2.0 and npm:@cipherstash/stack@1.2.0/wasm-inline. A range such as ^1.0.0 and a package outside the train, such as @cipherstash/eql@3.0.6, stay as they are.

The release train is the Changesets fixed group that holds stash. A test holds that group equal to RELEASE_TRAIN_MANIFESTS in the CLI, which is the set that release-train.test.ts checks. During a prerelease, a pin takes the stable version, as that test requires. I ran the script on main'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 version script.

Tests cover the skip, the refresh and the cargo arguments

  • With every lock at the version, step 3 runs no cargo and leaves each lock byte-for-byte the same.
  • With one lock behind, step 3 runs cargo for that lock only.
  • If cargo runs and the lock still records another version, step 3 fails.
  • Over this tree at its own EQL version, step 3 runs no cargo.
  • The cargo command passes --workspace, and passes neither --package nor -p.
  • Two process tests run the real script, with mise replaced on PATH by 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-bindings bumped to 3.0.7, both locks changed only their eql-bindings version 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.

Mutation Result
Step 3 always runs cargo 4 tests fail
Step 3 never runs cargo 3 tests fail
The command goes back to --package eql-bindings 3 tests fail
The check after cargo is removed 1 test fails
A registry entry for eql-bindings counts as a path entry 1 test fails
The pin sync never rewrites a pin 3 tests fail
The pin sync pins the raw prerelease version 1 test fails
The pin sync matches stash inside a longer name 1 test fails
The pin sync uses the first fixed group 1 test fails
The pin sync never writes the file 1 test fails
The root version script skips the pin sync 1 test fails
The pin sync rewrites a package outside the train 1 test fails

The 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-ffi lock. That test now uses a fake that writes nothing.

Checks

  • pnpm test:scripts: 63 files and 1,164 tests pass, with 1 skipped as on main.
  • pnpm run code:check: no errors, and no new warnings.

Part 1 needs a GitHub App that this repository does not have yet

changesets/action pushes the Version Packages branch with GITHUB_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 uses actions/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/envelopers and cipherstash/vitaminc repositories already use a GitHub App for release-plz. Its ID is 5040119, and those repositories keep it in the variable RELEASE_PLZ_APP_CLIENT_ID and the secret RELEASE_PLZ_APP_PRIVATE_KEY. An admin can install that App on this repository, or create a new one. The App needs these repository permissions:

  • Contents: read and write. It creates the branch, commits through the API, pushes tags and creates GitHub releases.
  • Pull requests: read and write. It opens and updates the Version Packages pull request.
  • Metadata: read, which every App has.

The follow-up change then adds a SHA-pinned actions/create-github-app-token step to the release job and passes its token to changesets/action. It keeps commitMode: 'github-api', so GitHub still signs the commits. The new action also needs an entry in the allowlist of scripts/lint-no-workflow-caching.mjs.

Part 2 is a choice between three rules

The main ruleset requires one approval, and it has dismiss_stale_reviews_on_push and require_last_push_approval turned off. It does have require_extra_approval_for_unattributed_changes turned on, but that rule does not help here. It acts on commits that no GitHub account authored, and github-actions[bot] authors the Version Packages commits.

  • Dismiss stale approvals on push. GitHub dismisses an approval when new commits arrive. It needs no workflow, and it works without part 1. It applies to every pull request into main, so authors who push after approval need a new review.
  • Require approval of the most recent push. Someone other than the last pusher must approve after that push. It also applies to every pull request, and it stops a reviewer from approving a fix that they pushed themselves.
  • A required check that compares the approved commit with the head. A small workflow runs on pull request and review events. For the changeset-release/main branch, 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

@auxesis
auxesis requested a review from a team as a code owner October 2, 2026 23:54
@changeset-bot

changeset-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 6de9b46

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@freshtonic freshtonic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 bare catch in "runs no cargo over this tree at its own EQL version" hides every error, not only the after-check error. If cargoLockWorkspaces finds no lock and throws, cargo.calls stays [] 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: in runWithLock, the rmSync is not in a finally. If a readFileSync above it throws, the temp directory stays. A try/finally, as in the other fixtures in this file, prevents that.

@auxesis
auxesis force-pushed the fix/version-packages-lockfile branch from 2c5f036 to c32b5bd Compare October 3, 2026 00:06
@auxesis auxesis changed the title fix(release): leave Cargo.lock alone when the EQL version does not change fix(release): leave Cargo.lock alone and move the skill pins in the version step Oct 3, 2026
auxesis and others added 2 commits October 3, 2026 10:20
…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
@auxesis
auxesis force-pushed the fix/version-packages-lockfile branch from c32b5bd to 6de9b46 Compare October 3, 2026 00:22
@auxesis
auxesis merged commit 3e8f6cb into main Oct 3, 2026
29 checks passed
@auxesis
auxesis deleted the fix/version-packages-lockfile branch October 3, 2026 00:38
auxesis added a commit that referenced this pull request Oct 3, 2026
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants