feat(release): build the release pipeline for the seven FFI packages - #878
Open
tobyhede wants to merge 17 commits into
Open
feat(release): build the release pipeline for the seven FFI packages#878tobyhede wants to merge 17 commits into
tobyhede wants to merge 17 commits into
Conversation
🦋 Changeset detectedLatest commit: 71a2436 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
tobyhede
marked this pull request as ready for review
August 11, 2026 02:30
Applies the /simplify findings: - The gate and matrix jobs no longer install the workspace. release-gate.mjs reads pnpm-workspace.yaml's own globs instead of shelling out to `pnpm ls`, and the matrix reads the Rust triples out of the six committed platform manifests instead of asking `neon list-platforms` — both were paying a cold ~1GB install (caching is forbidden in a publishing workflow) to compute something already in the tree. The gate runs on every push to main. - AUDITED_ACTIONS carries what the audit found, not just that it happened: one Map entry per action with the input that disables its caching. That was two hand-maintained lists, which is the shape the file's own header warns about, and it also fixes a case-sensitivity gap — the allowlist matched case-insensitively while the explicit-false rules tested the raw `uses:`, so `Actions/Setup-Node@v6.5.0` was audited and then skipped its requirement. - lint-release.yml drops to actionlint alone. tests.yml runs test:scripts on every PR unfiltered and tests-supply-chain.yml runs both checks, so the copy here was the second and third run behind a third install. That workflow's path filter now names _build-ffi-artifacts.yml, which it lints. - The superseded tests.yml frozen-lockfile check is deleted: same rule, one file instead of the whole graph, per step body instead of per command. It passed on inputs the new one fails. The new check now also matches `pnpm i`, which the node-gyp guard already treats as an install. - lintWiring's dead-workflow machinery collapses to one assertion that the package holds no .github at all — stronger than the .ya?ml scan it replaces, and reachable, which the citation half no longer was. - The wrapper tarball check walks the manifest's own exports map. The hand written list had silently omitted all three types targets, so a wrapper packed without declarations passed. - Narrower work: the aarch64 cross toolchain installs only for the platform that links it, zig only where the matrix picked zigbuild, each linker export only in the branch that reads it, and the preflight counts platforms from its own table.
Contributor
There was a problem hiding this comment.
Pull request overview
Builds an inert, registry-gated release pipeline for the seven FFI packages.
Changes:
- Adds cross-platform artifact build, preflight, ordered publishing, tagging, and release workflows.
- Adds release gates and supply-chain checks.
- Updates FFI metadata, documentation, tests, and changesets.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
skills/stash-supply-chain-security/SKILL.md |
Documents FFI release security. |
scripts/release-gate.mjs |
Detects unpublished versions. |
scripts/lint-no-workflow-caching.mjs |
Extends cache enforcement. |
scripts/ffi-release-matrix.mjs |
Generates the platform matrix. |
scripts/__tests__/workflow-paths-filter-parity.test.mjs |
Registers release-lint asymmetry. |
scripts/__tests__/workflow-dispatch-job-conditions.test.mjs |
Registers dispatch workflows. |
scripts/__tests__/release-gate.test.mjs |
Tests release classification. |
scripts/__tests__/lint-no-workflow-caching.test.mjs |
Tests expanded cache checks. |
scripts/__tests__/fixtures/lint-no-workflow-caching/mise-default-cache.yml |
Covers mise’s cache default. |
scripts/__tests__/ffi-repository-urls.test.mjs |
Validates repository metadata. |
scripts/__tests__/ffi-release-matrix.test.mjs |
Validates matrix generation. |
scripts/__tests__/cargo-publish-opt-out.test.mjs |
Guards crates.io opt-out. |
packages/protect-ffi/src/lintWiring.test.ts |
Prevents dead package workflows. |
packages/protect-ffi/README.md |
Documents workflow migration. |
packages/protect-ffi/platforms/win32-x64-msvc/package.json |
Updates repository metadata. |
packages/protect-ffi/platforms/linux-x64-musl/package.json |
Updates repository metadata. |
packages/protect-ffi/platforms/linux-x64-gnu/package.json |
Updates repository metadata. |
packages/protect-ffi/platforms/linux-arm64-gnu/package.json |
Updates repository metadata. |
packages/protect-ffi/platforms/darwin-x64/package.json |
Updates repository metadata. |
packages/protect-ffi/platforms/darwin-arm64/package.json |
Updates repository metadata. |
packages/protect-ffi/package.json |
Repoints package links. |
packages/protect-ffi/crates/protect-ffi/src/lib.rs |
Repoints issue reporting. |
packages/protect-ffi/crates/protect-ffi/Cargo.toml |
Disables crate publishing. |
packages/protect-ffi/.github/workflows/test.yml |
Removes inert workflow. |
packages/protect-ffi/.github/workflows/release.yml |
Removes inert workflow. |
packages/protect-ffi/.github/workflows/build.yml |
Removes superseded build workflow. |
packages/protect-ffi/.github/actions/setup/action.yml |
Removes inert setup action. |
packages/protect-ffi/.github/.env |
Removes obsolete configuration. |
package.json |
Adds the release-gate command. |
e2e/tests/supply-chain.e2e.test.ts |
Expands frozen-lockfile enforcement. |
docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md |
Records Phase 3 completion. |
AGENTS.md |
Documents the release pipeline. |
.github/workflows/tests-supply-chain.yml |
Extends workflow path coverage. |
.github/workflows/release.yml |
Adds ordered FFI publishing. |
.github/workflows/lint-release.yml |
Adds release workflow linting. |
.github/workflows/ffi-preflight.yml |
Adds artifact smoke testing. |
.github/workflows/_build-ffi-artifacts.yml |
Builds seven release tarballs. |
.github/actionlint.yaml |
Registers the Blacksmith runner. |
.changeset/supply-chain-skill-ffi-release-path.md |
Versions the skill update. |
.changeset/protect-ffi-repository-url.md.deferred |
Parks the metadata changeset. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…is pinned commandsOf split on newlines alone, so two commands sharing a physical line were one entry — and `pnpm install; pnpm install --frozen-lockfile` therefore contained the flag and reported no offender, while the first install resolved outside the lockfile. A run block is shell, and shell does not need a newline to start a second command. Splits on `;`, `&&`, `||`, `|` and `&` as well as newlines. The split is quote-blind — `echo "pnpm install; ok"` becomes two fragments — which can only over-report, and the pattern was already quote-blind before this since it matched an install inside an echo just the same. Regression test covers each separator, that a pinned install sharing a line with other work still passes, and the reported case verbatim. Mutation-checked against a real workflow: the repo sweep now reports `.github/workflows/lint-release.yml: pnpm install` for that shape.
coderdan
reviewed
Aug 11, 2026
coderdan
approved these changes
Aug 11, 2026
coderdan
left a comment
Contributor
There was a problem hiding this comment.
Agent findings should be addressed but otherwise LGTM
…typing what they check
Quality pass over the release pipeline. No change to what any workflow
publishes; the gate answers the same question and the matrix builds the same
six platforms.
The gate paid a cold full-workspace install to parse one YAML file.
`release-gate.mjs` imported `js-yaml` for a flat `packages:` list, which
obliged its job to run pnpm/action-setup, a global node-gyp and an uncached
`pnpm install --frozen-lockfile` on EVERY push to main — docs-only merges
included — before it could answer. Its own header argued exactly that against
`pnpm ls -r` and then paid the same cost for the parse. It is builtins-only
now, and the job is checkout, setup-node, one `node` call.
That parse is a block scan, not a YAML implementation, so `release-gate.test.mjs`
pins it against js-yaml reading the real file. Narrowing is the failure that
matters — a dropped pattern is a package never looked up, reported as "nothing
to publish", then published binary-less — so an empty pattern list or an empty
manifest set throws rather than returning short.
Three checks asserted their subject by restating it:
- `ffi-release-matrix.test.mjs` re-implemented `buildFor`'s ternary and
`runnerFor`'s three-arm ladder inside its own expectations, so a wrong rule
in the script passed as long as the copy in the test was edited the same
wrong way. Now a literal six-row table, plus one case for the fallback arm
a table cannot reach.
- The wrapper's `optionalDependencies` were counted against a literal 6 while
the manifest's own `neon.platforms` sat in the same parsed object. Now
compared by NAME against it — six entries naming five platforms twice would
have satisfied the old check — and a seventh platform is covered the day it
is added.
- `lint-release.yml` stated its scope twice, as a paths filter and as an
actionlint argument list, with nothing binding them. Both drift directions
are silent: in `paths:` alone runs the gate without linting the file; in the
argument list alone lints it except on the PRs that change it.
`lint-release-scope.test.mjs` asserts they are the same set, with a floor so
it cannot pass on two empty lists.
Dead and duplicated work:
- `gate.outputs.js` was declared, plumbed and defended by five tests, and read
by nothing — nor can it be, since `release` must run on every push to open
the Version Packages PR. Dropped, with the unreachable `unpublished=` line.
- `ffi-release-matrix.mjs` emitted the same value three ways and its only
caller discarded the third with `> /dev/null`, which reads as suppressing
something. Also dropped the `argv[2]` override, which had no caller.
- `lint-release.yml` triggered on four paths its job cannot observe, since it
deliberately does not run `test:scripts`.
- The wrapper's export check decompressed the whole tarball once per required
path; it lists once now.
- `actionPath(uses)` was recomputed when `path` already held it, and
`ffi-repository-urls.test.mjs` read `platforms/` without the directory
filter its sibling uses — a stray .DS_Store would crash it on a missing
package.json.
Also corrected a step name in tests-supply-chain.yml that still named two
workflows after _build-ffi-artifacts.yml joined the target set.
Verified: 406 script tests pass (28 files), `lint:workflow-cache` green, all
four touched workflows parse, biome clean on every changed file.
`_build-ffi-artifacts.yml` verifies the packed wrapper with a `node -e '…'`
whose JS said, in a `//` comment, "the manifests own `neon.platforms`". Shell
does not know that is a comment: the whole argument is one single-quoted string,
so shellcheck reads the backtick as command substitution that will not expand
and reports SC2016. Fifteen lines further down the SAME argument spells the rule
out, and a later edit broke it anyway, because nothing was checking.
It went red on its own overnight, on a run of unchanged code. actionlint is
pinned to v1.7.7 but shellcheck is not — it comes from the `ubuntu-latest`
image, whose shellcheck reported SC2016 only for `$` until 0.11.0 extended it to
backticks. So this sat as a latent failure keyed to an image bump and surfaced
as a red `lint-release` on a pull request that did not touch it, which is the
"fails at the worst possible moment" that workflow exists to prevent.
`workflow-inline-node-quoting.test.mjs` pins it without that dependency: no
backtick and no `${` in a single-quoted inline `node` argument, across the
workflows actionlint lints — read out of `lint-release.yml` rather than copied,
so a fifth release workflow is covered the day it lands. It runs in
`test:scripts`, with no binary to download and no version to drift.
Scoped to inline `node` arguments rather than all single-quoted text: shell `#`
comments in these four workflows carry twenty backticks and shellcheck is right
to ignore every one, because a comment is not a string. The dangerous construct
is the inverse — a JS `//` comment that looks like a comment while sitting
inside shell single quotes. Scanning the argument is exact and needs no shell
lexer whose disagreement with the real one would be the next silent hole.
…workflow `release.yml` declared `id-token: write`, `contents: write` and `pull-requests: write` at the workflow level, because the two jobs that publish need them. Workflow-level permissions are a default, not a ceiling applied only where asked, so `gate` — a checkout and one `node` call answering "is anything unpublished?" — inherited all three, as did `ffi-artifacts`. npm trusted publishing is bound to a repository AND A WORKFLOW FILENAME. Once this file is the registered publisher, an `id-token: write` token minted by any job in it is one npm accepts for a publish: the registry cannot tell the cheap every-push gate apart from `publish-ffi`. The other two scopes are their own reach — a job able to write the tree the publish jobs then build from does not need to forge a token, it can change what is about to be published. The workflow-level grant is `contents: read` and the two publishing jobs escalate for themselves. Overriding the two offenders would close today's hole and leave tomorrow's: the next job added here would inherit the credential by omission, which is how this one happened. Inverted, omission is the safe answer and a new job has to ask for the credential in its own diff. `workflow-publish-permissions.test.mjs` pins the property rather than the text — in any workflow where a job holds `id-token: write`, every job that does not publish must grant nothing writable — and scans the workflow directory, so a second publishing workflow is held to the same bar the day it lands. The jobs allowed to hold it are an equality rather than a floor: an eighth publisher should not arrive without a word in review. A `uses:` job needs no special case. The caller job's grant is the ceiling for the reusable workflow it calls, so a caller held to `contents: read` cannot hand `id-token: write` to anything downstream, whatever that file declares. The bundled supply-chain skill said `permissions: id-token: write` is what mints the token and stopped there, which reads as an instruction to put it at the top. It now says where the grant belongs and why.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds the release pipeline for the seven FFI packages — CIP-3718, Phase 3 Tasks 1–8 of
docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md.Everything here is reversible and inert: nothing publishes until a committed version is missing from npm, and
scripts/lint-no-ffi-changeset.mjsis what keeps that from happening early. The irreversible cutover is CIP-3719.The shape
One version authority (Changesets), N idempotent publishers ordered by dependency — the model already proven in
cipherstash/encrypt-query-language.changeset publishpacks from the workspace, whereindex.nodeis a build output nobody produced — so the FFI tarballs must publish first, and once they have, changesets skips them by itself. The ordering is the interlock.Three things that are easy to get wrong and are handled explicitly:
release-gate.mjsthrows rather than reporting "nothing to publish".ffi-artifactsfails,publish-ffiis skipped, so a bareresult != 'failure'check passes. Thereleasejob keys on the gate's own output instead: if FFI was in scope, its publish must have succeeded.tagPublishreceives onlyresult === "published"). The seven tags and the GitHub release are this pipeline's job.Tasks
dc6db45cpublish = false, guarded across every workspace member6ce84816cipherstash/stacka9210f69scripts/release-gate.mjs+ tests,pnpm release:gate3f6ca3e8_build-ffi-artifacts.yml,scripts/ffi-release-matrix.mjs,.github/actionlint.yamlc1a40299release.yml, tags and release included37c89ecblint-release.yml54fd265effi-preflight.yml872f6f97packages/protect-ffi/.github/deleted;lintWiringguards its absenceTask 2 is the one with a deadline attached: npm rejects a trusted publish whose
repository.urldoes not match the publishing repository, and it does not warn first.repository.directorymoves with it — it resolves from that repository's root, soplatforms/<p>addressed a real directory in the old repo and nothing here.Where the build departed from the plan
node -ein YAML. The plan argues all three derived fields are silently wrong if ported verbatim (buildistschere, not upstream's cargo script;cargo-buildandzig-buildredirect to different logs) — that is an argument for testing them. The test derives the log filename from the package's own scripts, so moving a redirect fails it.rustup target add, notdtolnay/rust-toolchain. Every runner image ships rustup,tests-rust.ymlandbuild-ffi-bindingalready do it this way, and it avoids allowlisting a fourth remote action in the no-caching gate.mv. Verified: under--dir, a relative--pack-destinationresolves against the package directory, not the CWD. Only the relative form behaves identically on the Windows runner.AUDITED_ACTIONSneeded three entries the plan did not mention —actions/upload-artifact,actions/download-artifact,jdx/mise-action. Without them the gate fails, correctly.release.ymlran a barepnpm installfrom the day it was written; the check that should have caught it readtests.ymlalone. It now scans every workflow and composite action, per command.Review findings fixed on this branch
Three CodeRabbit rounds, 9 findings, 8 taken. Two were real defects in the gate itself:
jdx/mise-actionwith nocache:key. Itscacheinput defaults totrue, while the gate's generic rule fires only on a truthy value — so an omitted key cached and passed, in the workflow whose artifacts are published with provenance. Reproduced, fixed, fixture + self-test added.actions/setup-node'spackage-manager-cachealso defaults totrue; defaults were read from the pinned actions' ownaction.ymlrather than from memory.wgetfrom musl.cc, no signature, and whatever comes back links the binary published to npm — provenance attests where a build ran, not that its inputs were intended. Now SHA-256 pinned and checked before extraction. Trust-on-first-use: the digest came from two independent fetches of the current artifact, so it does not authenticate musl.cc, it makes later substitution a hard failure. Redundant second extraction removed.Also: the tag lookup moved to
git/matching-refswith an exact-ref filter, because these tag names contain a slash andgit/ref/{ref}answers a non-exact match with an array —.object.shawould yield nothing and a re-run would 422 instead of no-op'ing. And npm's rule forworkflow_callturned out to be more specific than the plan said (validation checks the calling workflow's name, which npm documents as a known issue); the placement is unchanged and correct either way, but the stated reason was wrong in three files.One finding rejected: the claim that the workflow glob missed nested files. It matches all 22 workflows and composites, and GitHub does not read workflows from subdirectories. The actions glob was widened to
**anyway — grouped composite actions are valid and a flat glob would stop covering one silently.Verification
actionlintclean on all four release workflows (this PR introduces actionlint to the repo, with.github/actionlint.yamlfor the Blacksmith label)package-manager-cache: falseis removed, and red on the mise fixture — both mutation-checkedrelease.yml's fix is reverted, while the oldtests.yml-scoped check stays green — the coverage gap was realnode scripts/release-gate.mjsagainst the live registry:nothing to publish — ffi=false js=falsepnpm packcarries the newrepository.urland six concreteoptionalDependenciesNot done, and why
Task 7 Step 3 — dispatching
ffi-preflight.yml— cannot run until this merges. GitHub resolvesworkflow_dispatchfrom the default branch, so the button does not exist yet. That run is the first end-to-end proof of the matrix, target selection, packing, architecture and install, and nothing before Phase 4 substitutes for it. The workflow is inert until dispatched, so merging it early costs nothing:Changesets
supply-chain-skill-ffi-release-path.md—stashpatch;skills/ships in the tarballprotect-ffi-repository-url.md.deferred— parked, per the guard. Two are now waiting; the cutover PR renames bothCloses CIP-3718 · https://linear.app/cipherstash/issue/CIP-3718