Skip to content

fix(ci): let release-plz release again, only from a release PR, with the nested lockfiles in step - #463

Merged
coderdan merged 7 commits into
mainfrom
fix/release-plz-test-crates
Oct 17, 2026
Merged

coderdan merged 7 commits into
mainfrom
fix/release-plz-test-crates

Conversation

@coderdan

@coderdan coderdan commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Unblocks the v0.6.0 release (#389). Merge this first: the next push to main makes release-plz regenerate #389 with the fix below applied.

Why releases could not go out

Every Release run since e005ba4 (2026-10-15) has failed before doing anything:

Package `derive-tests-direct` has `publish = false` or `publish = []` in the Cargo.toml,
but it has `publish = true` in the release-plz configuration.

The four derive-tests crates said release = false in release-plz.toml but inherited publish = true from [workspace]. They now say publish = false as well. Merging #389 as things stand would have published nothing.

Fixing only that would have published from main. The job runs release on every push to main, and release-plz's default publishes any version not yet on crates.io. vitaminc-aead-frame, vitaminc-normalize and vitaminc-ore have never been published and sit at 0.5.1 on main, so the next push would have tried to release them at 0.5.1. release_always = false makes the job publish only when the commit is a merged release PR.

#389's tests fail on stale nested lockfiles. packages/aead-napi/tests/addon and bindings/go/vcencrypt/guest are separate workspaces with their own Cargo.lock, which pin the workspace crates by version. release-plz does not update them, and the Node addon builds with --locked, so the release PR always fails the Test and Go bindings jobs (as the review on #389 found). The release-pr job now re-syncs both lockfiles on the release branch after release-plz regenerates it, changing only the workspace crates' version lines (nine in each).

That step takes the PR from release-plz's own output (set whether it opened or updated the PR), not from a search, since GitHub's search index lags a just-opened PR. It holds a token that can push, so it trusts nothing on the branch: it checks the PR is open against main, from this repository, by the release-plz app and on a release-plz-* branch, and fails loudly otherwise. It runs no cargo there, which would honour the branch's .cargo/config.toml. Instead, main's .github/scripts/sync_nested_lockfiles.py copies each workspace crate's version from the root Cargo.lock into the nested ones as a text edit, in a separate worktree, and refuses a lockfile it cannot re-sync (a version-qualified reference to a workspace crate) rather than leaving it half done. Its tests, including one over main on real files and one that NESTED names every tracked lockfile, run in the Test job, alongside a test that release-plz.toml marks every unpublishable workspace member publish = false, which would have caught the outage above on the pull request.

Release process

RELEASING.md now covers the nested lockfiles in the manual version override, and a new crate's first release. crates.io trusted publishing can only be set up for a crate that already exists, so the job cannot publish vitaminc-aead-frame, vitaminc-normalize or vitaminc-ore. The steps: merge the release PR, let the job publish what it can, then finish from the merge commit locally with release-plz release and your own token, and set up trusted publishing for the new crates. It also corrects the Secrets section, which described a CARGO_REGISTRY_TOKEN secret the repository does not use.

Verified

With release-plz 0.3.172, the version CI runs:

  • On main before this change, release-plz release --dry-run fails with the error above.
  • With this change, it reports skipping release: current commit is not from a release PR.
  • On chore: release v0.6.0 #389's branch, the script makes the same 18-line change cargo metadata does (only the 0.5.1 → 0.6.0 lines), and both nested workspaces then pass --locked.

The workflow step itself only runs on main, so its first real run is the push that merges this PR.

…the nested lockfiles in step

Every Release run since e005ba4 has failed before doing anything: the
derive-tests crates are publish = false in their Cargo.toml but inherited
publish = true from the release-plz workspace config, which release-plz
refuses. They now say publish = false there too.

release_always = false makes the job publish only from a merged release
PR. Without it, fixing the config would have published the new crates
(aead-frame, normalize, ore) at 0.5.1 from main on the next push.

The release PR's tests failed because two test workspaces keep their own
lockfiles pinning the workspace crates at the old version, and the Node
addon builds with --locked. The release-pr job now re-syncs both on the
release branch after release-plz regenerates it.

RELEASING.md covers the nested lockfiles in the version override, a new
crate's first release (trusted publishing cannot publish a crate that does
not exist yet), and corrects the Secrets section: there is no crates.io
token secret.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T08:45:34.183871Z 52c6cda PR opened
🔒 Security Review ✅ Completed 2026-10-16T08:44:55.497452Z 52c6cda PR opened

Security findings

Advisory findings (1)

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛡️ Codex Security Review · Automatically triggered

Here are some automated security review suggestions for this pull request.

Reviewed commit: 52c6cdac88

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

Comment thread .github/workflows/release-plz.yml Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52c6cdac88

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/release-plz.yml Outdated
@coderdan
coderdan requested a review from auxesis October 16, 2026 08:45

Copilot AI 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.

🟡 Changes recommended

Release-branch discovery can silently skip lockfile synchronization, and the publishing workflow still needs maintainer validation.

1 open finding
What changed in this PR

Repairs release automation to unblock v0.6.0 while keeping nested test-workspace lockfiles aligned.

Changes:

  • Restricts publishing to release PR merges and excludes test-only crates.
  • Adds nested lockfile synchronization on the release branch.
  • Documents first-time crate publishing and authentication.
File Description
RELEASING.md Documents lockfile updates, first releases, and authentication.
release-plz.toml Gates releases and disables publishing for test crates.
.github/​workflows/​release-plz.yml Synchronizes nested lockfiles after release PR generation.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/release-plz.yml Outdated
…ode, and only on release-plz's own PR

The step held the app token, picked any open PR whose branch starts with
release-plz-, and ran cargo metadata on it. cargo honours the branch's
.cargo/config.toml, so a collaborator's release-plz-* branch with a
rustc-wrapper would run with the token and the workflow's crates.io
publishing permission, and a fork's PR of that name could be picked and
make the step fail.

The step now takes only the open PR that release-plz opened from this
repository, checks the branch out into its own worktree, and runs main's
sync_nested_lockfiles.py over it: a text edit that copies the workspace
crates' versions from the root Cargo.lock, so nothing from the branch
executes. On the current release branch it makes the same 18-line change
cargo did, and both nested workspaces then pass --locked. Its unit tests
run in the Test job.
gh pr list returns 30 PRs by default. The author filter is applied by
GitHub's search, so only release-plz's PRs come back, but the query now
sets a limit rather than depending on that.

Copilot AI 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.

🔵 Needs a closer look

The privileged release-branch update and publishing workflow needs end-to-end validation by a maintainer.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@cipherstash-bot cipherstash-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Recommendation: 🟢 merge as it is

Nothing must change before merging. All eight comments can wait for a follow-up. The most important one is about how the re-sync step finds the release PR: GitHub search can miss a PR that release-plz has just opened.

The publish = false and release_always = false changes are correct. The re-sync step runs no code from the release branch.

Other findings not posted as comments

  • Optional: sync() passes the version text from the release branch's root Cargo.lock to re.sub as a replacement template (.github/scripts/sync_nested_lockfiles.py:66). A backslash in a version changes the output or raises an error. Cargo does not write such versions, and only release-plz writes that branch. So this fix only protects against unexpected input. To fix it, pass a function: lambda _: f'version = "{new}"'. In this lambda, new stands for versions[name], the new version for the package.
How this review was made
Agent Model Review type Result
claude claude-opus-5-5 test-gap 4 found, 4 posted
claude claude-opus-5-5 infracode 5 found, 5 posted
claude claude-opus-5-5 python 3 found, 2 posted
codex gpt-5.6-terra test-gap 1 found, 1 posted
codex gpt-5.6-terra infracode 0 found, 0 posted
codex gpt-5.6-terra python 0 found, 0 posted

Synthesis: claude-opus-5-5 merged the findings, removed duplicates and dropped findings it could not confirm in the code. 1 posted finding(s) were raised by two or more models.

Plain language: claude-opus-5-5 read every comment as a new reader would. 2 comment(s) had a problem that stopped the reader acting; it rewrote 2. It also rewrote the review body.

Stack: not part of a stack.

Context loaded: the description, 0 linked issue(s) and 11 discussion entries.

Comment thread RELEASING.md
Comment thread .github/scripts/sync_nested_lockfiles.py
Comment thread .github/scripts/sync_nested_lockfiles.py
Comment thread release-plz.toml
Comment thread .github/scripts/sync_nested_lockfiles.py
Comment thread RELEASING.md Outdated
Comment thread .github/workflows/release-plz.yml
…a search

`gh pr list --author` goes through GitHub's search index, which lags a
just-opened PR, so the re-sync step could run a few seconds after
release-plz opened a PR and find nothing, leaving its nested lockfiles
stale until the next push to main. release-plz's action reports the PR
it opened or updated, so the step reads that one PR with `gh pr view`,
keeps the same checks on it (open against main, from this repository,
by the release-plz app, on a release-plz branch), and fails loudly if
they do not hold rather than exiting quietly. A comment keeps the
commit identity a `[bot]` login, which release-plz needs to go on
force-pushing its PR instead of closing and reopening it.
… it cannot re-sync

The release workflow runs `main`, which no test called; it now runs
over a repository in a temporary directory. A test pins that NESTED
names every tracked lockfile but the root's, so a new workspace with
its own lockfile fails here rather than on the release PR. The script
refuses a lockfile whose dependency entries name a workspace crate with
its version, which it could not re-sync and would have left broken
while reporting success, and the version replacement is a function so a
backslash in a version is copied rather than read as an escape.
…lish = false

The outage after e005ba4 was a workspace member with `publish = false`
that release-plz.toml still treated as publishable; every Release run
failed and nothing on the pull request said so. This test fails the
pull request instead.

@auxesis auxesis 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.

You love to see it — thanks @coderdan.

@coderdan
coderdan merged commit 6d8ff2f into main Oct 17, 2026
9 checks passed
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.

4 participants