fix(release): name the crate for neon dist, which pnpm exec cannot default

.github/workflows/_build-ffi-artifacts.yml

@@ -255,9 +255,18 @@ jobs:
         # load.cts, which is right for local development and wrong here. The log
         # file differs per build script; `neon dist` reads it to locate the
         # artifact.
+        #
+        # `-n` is not optional here, though it looks it. The crate name defaults
+        # to `basename($npm_package_name)`, and that variable is set by a
+        # package-script run — NOT by `pnpm exec`, which is how this step has to
+        # invoke the binary to pass `-o`. Without it every platform fails with
+        # `error: $npm_package_name is not defined`. Upstream never met this:
+        # its `postcargo-build` / `postzig-build` hooks run `neon dist` as
+        # lifecycle scripts, where the variable exists.
         run: |
           set -euo pipefail
-          pnpm exec neon dist -o "platforms/${PLATFORM}/index.node" < "${BUILD_LOG}"
+          pnpm exec neon dist -n protect-ffi \
+            -o "platforms/${PLATFORM}/index.node" < "${BUILD_LOG}"
           test -s "platforms/${PLATFORM}/index.node"
 
       # `pnpm pack` writes into the packed package's own directory by default,
@@ -290,7 +299,20 @@ jobs:
           test "$name" = "@cipherstash/protect-ffi-${PLATFORM}" || {
             echo "::error::packed $name, expected the ${PLATFORM} platform package"
             exit 1; }
-          tar tzf "$tgz" | grep -qx package/index.node || {
+          # Listed into a variable rather than piped into `grep -q`, and that is
+          # not a style preference. `grep -q` exits at the FIRST match, the
+          # writer upstream then takes SIGPIPE, and under `pipefail` — on by
+          # default for `shell: bash` — the pipeline reports 141. A successful
+          # match is read as a failure.
+          #
+          # It is worse than an ordinary bug because it is platform-split: GNU
+          # tar (Linux, and Git-for-Windows on the win32 runner) writes an entry
+          # at a time and hits it, while bsdtar buffers this four-entry listing
+          # into a single write and finishes first. So it passed on both Darwin
+          # legs and failed on the other four, which reads as a cross-compile
+          # problem rather than a shell one.
+          listing=$(tar tzf "$tgz")
+          grep -qx package/index.node <<< "$listing" || {
             echo "::error::$tgz has no index.node"; exit 1; }
 
       - uses: actions/upload-artifact@v4

.github/workflows/ffi-preflight.yml

@@ -97,12 +97,23 @@ jobs:
             # The ABI is only visible in the dynamic section: the gnu build
             # links libc.so.6, the musl build does not (RUSTFLAGS drops
             # crt-static, so it stays dynamic against musl's own libc).
+            #
+            # `readelf` into a VARIABLE, never into `grep -q` — the same
+            # SIGPIPE-under-pipefail trap documented in
+            # _build-ffi-artifacts.yml, and here the musl branch fails OPEN.
+            # A poisoned pipeline is non-zero, the `if` below is therefore
+            # false, and the check reports "no glibc NEEDED entry" for the
+            # exact binary it exists to reject. A dynamic section is far longer
+            # than a tarball listing, so the writer is all but certain to still
+            # be writing when grep leaves.
             case "$platform" in
               linux-x64-gnu|linux-arm64-gnu)
-                readelf -d x/package/index.node | grep -q 'NEEDED.*libc\.so\.6' || {
+                dynamic=$(readelf -d x/package/index.node)
+                grep -q 'NEEDED.*libc\.so\.6' <<< "$dynamic" || {
                   echo "::error::$platform does not link glibc"; exit 1; } ;;
               linux-x64-musl)
-                if readelf -d x/package/index.node | grep -q 'NEEDED.*libc\.so\.6' ; then
+                dynamic=$(readelf -d x/package/index.node)
+                if grep -q 'NEEDED.*libc\.so\.6' <<< "$dynamic" ; then
                   echo "::error::linux-x64-musl links glibc — it is the gnu binary"
                   exit 1
                 fi

docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md

@@ -178,10 +178,16 @@ The coupling is correct and should stay: an exact pin is how a wrapper/binary mi
 
 npm strips the `--` in `npm run x -- --release`; pnpm forwards it, landing it after the `> cargo.log` redirect where cargo rejects it as a positional. The release matrix passes `--target "${CARGO_BUILD_TARGET}.2.28"` this way, so the port depends on the separator-free spelling.
 
-### `neon dist` needs `-o`
+### `neon dist` needs `-o` — and therefore also `-n`
 
 Bare `neon dist < cargo.log` writes `./index.node` — the `debug:` fallback in `load.cts`. Populating a platform package needs `neon dist -o platforms/<p>/index.node`.
 
+Passing `-o` is what forces the command to be invoked directly rather than through the `postcargo-build` / `postzig-build` lifecycle hooks, and that is where the second half of this bites. `neon dist` defaults its crate name to `basename($npm_package_name)`, a variable the package-script runner sets. `npx` sets it too — the spelling this plan originally carried — but **`pnpm exec` does not**, so the switch to `pnpm exec` (correct on its own terms: `npx` will fetch a missing binary over the network) silently removed the default's only source. Every platform leg then fails with `error: $npm_package_name is not defined`.
+
+So the invocation is `pnpm exec neon dist -n protect-ffi -o …`, naming the **crate**, not the npm package. `scripts/__tests__/neon-dist-crate-name.test.mjs` holds both halves: the flag is present, and its value matches `crates/protect-ffi/Cargo.toml`. The second matters more than it looks — a renamed crate with a stale `-n` does not error, it finds no artifact.
+
+Caught by the first dispatch of `ffi-preflight.yml` (run 31555436823), against `main`, after the pipeline had merged. This is the failure mode the pre-flight exists for: it would otherwise have surfaced on the first real release, mid-cutover.
+
 ### This package's `build` is not upstream's `build`
 
 The matrix was ported verbatim from upstream, including `platform.includes('gnu') ? "zigbuild" : "build"`. Upstream's `build` was the cargo script. **Here `build` is `tsc` and nothing else** — phase 1 moved cargo to `build:native` precisely so the default path stays Rust-free. Ported as-is, four of the six platforms would have run a TypeScript compile, produced no binary, and failed one step later on a missing `cargo.log`.
@@ -988,9 +994,14 @@ jobs:
         working-directory: packages/protect-ffi
         # Bare `neon dist` writes ./index.node — the `debug:` fallback in
         # load.cts. Populating a platform package needs an explicit -o.
+        #
+        # `-n` because the crate name defaults to `basename($npm_package_name)`,
+        # which `pnpm exec` does not set (`npx` did). See "`neon dist` needs
+        # `-o` — and therefore also `-n`" above.
         run: |
           set -euo pipefail
-          npx neon dist -o "platforms/${{ matrix.cfg.platform }}/index.node" \
+          pnpm exec neon dist -n protect-ffi \
+            -o "platforms/${{ matrix.cfg.platform }}/index.node" \
             < "${{ matrix.cfg.log }}"
           test -s "platforms/${{ matrix.cfg.platform }}/index.node"
 

scripts/__tests__/neon-dist-crate-name.test.mjs

@@ -0,0 +1,153 @@
+import { readFileSync } from 'node:fs'
+import { join } from 'node:path'
+import { describe, expect, it } from 'vitest'
+import { REPO_ROOT } from './lib/repo-root.mjs'
+import { readWorkflow, workflowFiles } from './lib/workflows.mjs'
+
+/**
+ * Every `neon dist` invoked through `pnpm exec` must pass `-n <crate name>`,
+ * and that name must be the crate actually being built.
+ *
+ * `neon dist` locates the compiled cdylib in the cargo log by crate name, and
+ * defaults that name to `basename($npm_package_name)` — see
+ * `@neon-rs/cli/index.js`, `ensureDefined(process.env['npm_package_name'],
+ * '$npm_package_name')`. That variable is populated by a package-script run and
+ * NOT by `pnpm exec`, so the default is unavailable precisely where this
+ * repository needs the command: `_build-ffi-artifacts.yml` calls the binary
+ * directly in order to pass `-o platforms/<platform>/index.node`, because a
+ * bare `neon dist` writes `./index.node` — the `debug:` fallback in `load.cts`,
+ * which is right for local development and wrong for a published tarball.
+ *
+ * Upstream never met this failure, which is why it was not inherited along with
+ * the rest of the build: `postcargo-build` and `postzig-build` run `neon dist`
+ * as npm lifecycle scripts, where the variable exists. The first direct call is
+ * this repository's, and it failed all six platform legs with
+ * `error: $npm_package_name is not defined`.
+ *
+ * Three ways for that to come back:
+ *
+ *   - the flag is dropped while editing the surrounding shell, and every
+ *     platform fails again;
+ *   - the crate is renamed and the flag is not, which does NOT fail loudly in
+ *     the same way — `neon dist` finds no matching artifact in the log, and the
+ *     mode it fails in is a build that produced nothing rather than a build
+ *     that errored;
+ *   - a second `neon dist` is added to a step that already has a correct one,
+ *     and inherits none of its flags.
+ *
+ * The third is why the scan is per INVOCATION rather than per step. Matching
+ * once over a step body answers for whichever call came first, and the `-n` it
+ * finds need not belong to a `neon dist` at all — `echo -n protect-ffi` on an
+ * earlier line satisfied it. The last two `it` blocks below hold the scanner
+ * itself to that, since no workflow in the tree exercises either shape.
+ *
+ * Discovery, not a list: any workflow that grows a `neon dist` call is covered
+ * the day it lands.
+ */
+
+/** The crate whose cdylib becomes `index.node`. */
+function crateName() {
+  const toml = readFileSync(
+    join(REPO_ROOT, 'packages/protect-ffi/crates/protect-ffi/Cargo.toml'),
+    'utf8',
+  )
+  const match = toml.match(/^\s*name\s*=\s*"([^"]+)"/m)
+  if (!match)
+    throw new Error('no [package] name in crates/protect-ffi/Cargo.toml')
+  return match[1]
+}
+
+/** Every `run:` script in a workflow, with the job and step that carry it. */
+function runSteps(workflow) {
+  return Object.entries(workflow.jobs ?? {}).flatMap(([jobId, job]) =>
+    (job.steps ?? [])
+      .filter((step) => typeof step.run === 'string')
+      .map((step) => ({
+        jobId,
+        name: step.name ?? '(unnamed)',
+        run: step.run,
+      })),
+  )
+}
+
+const NEON_DIST = /pnpm\s+exec\s+neon\s+dist\b/
+
+/**
+ * Each `pnpm exec neon dist` in a run block, split out as its own command.
+ *
+ * Per INVOCATION, not per step, and the distinction is the whole point. A
+ * single regex over the step body finds one `-n` and stops, so a step holding
+ * two calls is judged by whichever came first — and worse, the flag it finds
+ * need not belong to a `neon dist` at all. `sort -n` or `echo -n protect-ffi`
+ * on an earlier line would satisfy a body-wide match.
+ *
+ * Continuations are joined first, so a newline is a real statement boundary
+ * rather than a wrapped one. `||` is listed before `|` because split()
+ * alternation is ordered and would otherwise cut on the first bar.
+ */
+function neonDistInvocations(run) {
+  return run
+    .replace(/\\\n\s*/g, ' ')
+    .split(/\n|;|&&|\|\||\|/)
+    .map((command) => command.trim())
+    .filter((command) => NEON_DIST.test(command))
+}
+
+/** The `-n` / `--name` value of one command, or undefined if it carries none. */
+function crateNameFlag(command) {
+  return command.match(/(?:^|\s)(?:-n|--name)\s+(\S+)/)?.[1]
+}
+
+describe('neon dist through pnpm exec', () => {
+  const invocations = workflowFiles().flatMap((file) =>
+    runSteps(readWorkflow(file)).flatMap((step) =>
+      neonDistInvocations(step.run).map((command, i) => ({
+        file,
+        jobId: step.jobId,
+        name: step.name,
+        nth: i + 1,
+        command,
+      })),
+    ),
+  )
+
+  it('is invoked somewhere, or this guard is checking nothing', () => {
+    expect(invocations.length).toBeGreaterThan(0)
+  })
+
+  it.each(
+    invocations,
+  )('$file › $jobId › $name › call $nth passes the crate name', ({
+    command,
+  }) => {
+    expect(
+      crateNameFlag(command),
+      'neon dist needs -n; $npm_package_name is unset under pnpm exec',
+    ).toBe(crateName())
+  })
+
+  // The scanner's own coverage. Both of these describe a workflow that does not
+  // exist yet, so nothing above would notice if the splitting regressed to a
+  // body-wide match — which is what it was when this guard first landed.
+  it('sees a second invocation hiding behind a correctly named first', () => {
+    const run = [
+      `pnpm exec neon dist -n ${crateName()} -o one < cargo.log`,
+      'pnpm exec neon dist -o two < cargo.log',
+    ].join('\n')
+
+    expect(neonDistInvocations(run)).toHaveLength(2)
+    expect(neonDistInvocations(run).map(crateNameFlag)).toEqual([
+      crateName(),
+      undefined,
+    ])
+  })
+
+  it('does not count a -n belonging to another command in the same step', () => {
+    const run = [
+      `echo -n ${crateName()}`,
+      'pnpm exec neon dist -o out < cargo.log',
+    ].join('\n')
+
+    expect(neonDistInvocations(run).map(crateNameFlag)).toEqual([undefined])
+  })
+})

scripts/__tests__/workflow-grep-q-pipelines.test.mjs

@@ -0,0 +1,96 @@
+import { readdirSync } from 'node:fs'
+import { join } from 'node:path'
+import { describe, expect, it } from 'vitest'
+import { REPO_ROOT } from './lib/repo-root.mjs'
+import { readWorkflow, workflowFiles } from './lib/workflows.mjs'
+
+/**
+ * No `run:` block may pipe a command into `grep -q`.
+ *
+ * `grep -q` exits at the first match. Whatever is writing upstream then takes
+ * SIGPIPE, and `pipefail` — which GitHub turns on for every `shell: bash` step,
+ * before any `set -euo pipefail` the block writes itself — makes the pipeline's
+ * status that of the killed writer, 141. So the pipeline reports FAILURE on a
+ * successful match. The more the writer has left to say, the likelier it is.
+ *
+ * Two things make this worth a guard rather than a fix in place.
+ *
+ * It is platform-split, so it does not look like a shell bug. GNU tar writes an
+ * entry at a time and hits it; bsdtar buffers a short listing into one write
+ * and does not. `_build-ffi-artifacts.yml`'s "does the tarball contain
+ * index.node" check therefore passed on both Darwin legs of the FFI matrix and
+ * failed on Linux and Windows — presenting as a cross-compilation problem, on
+ * the packaging step, in a pipeline that had just been rewritten.
+ *
+ * And the direction it fails in is not fixed. `cmd | grep -q x || die` fails
+ * CLOSED — noisy, and someone investigates. `if cmd | grep -q x ; then die ; fi`
+ * fails OPEN: the poisoned status makes the condition false and the check
+ * silently passes. `ffi-preflight.yml` had one of each, and the open one was
+ * the check that stops a glibc binary shipping inside the musl platform
+ * package — a failure that lands on an Alpine user at `dlopen`, not in CI.
+ *
+ * The fix is to capture first and match against the variable:
+ *
+ *   listing=$(tar tzf "$tgz")
+ *   grep -qx package/index.node <<< "$listing" || die
+ *
+ * `grep -q` reading a FILE or a here-string is fine and stays allowed — there
+ * is no writer to signal. Only pipelines are rejected.
+ */
+
+/** Composite actions are part of the same call tree, and run the same shells. */
+function actionManifests() {
+  const dir = '.github/actions'
+  return readdirSync(join(REPO_ROOT, dir), { withFileTypes: true })
+    .filter((entry) => entry.isDirectory())
+    .map((entry) => `${dir}/${entry.name}/action.yml`)
+    .sort()
+}
+
+/** Every `run:` script in a workflow or composite action, with its location. */
+function runBlocks(doc, file) {
+  const fromJobs = Object.entries(doc.jobs ?? {}).flatMap(([jobId, job]) =>
+    (job.steps ?? [])
+      .filter((step) => typeof step.run === 'string')
+      .map((step) => ({
+        file,
+        where: `${jobId} › ${step.name ?? '(unnamed)'}`,
+        run: step.run,
+      })),
+  )
+  const fromAction = (doc.runs?.steps ?? [])
+    .filter((step) => typeof step.run === 'string')
+    .map((step) => ({ file, where: step.name ?? '(unnamed)', run: step.run }))
+  return [...fromJobs, ...fromAction]
+}
+
+/**
+ * A pipe into grep carrying `-q` in any spelling: `-q`, `-qx`, `--quiet`, and
+ * the same after other flags. Deliberately not trying to parse shell — a
+ * pattern that over-matches here costs a comment on a line that should be
+ * rewritten anyway.
+ */
+const PIPED_QUIET_GREP = /\|\s*grep\s+(?:-[a-zA-Z]*q[a-zA-Z]*|--quiet)\b/
+
+describe('no run: block pipes into grep -q', () => {
+  const files = [...workflowFiles(), ...actionManifests()]
+
+  it('finds workflows and composite actions to check', () => {
+    expect(files.length).toBeGreaterThan(0)
+  })
+
+  const offenders = files.flatMap((file) =>
+    runBlocks(readWorkflow(file), file)
+      .flatMap(({ where, run }) =>
+        run
+          .split('\n')
+          .map((line, i) => ({ where, line: line.trim(), number: i + 1 }))
+          .filter(({ line }) => PIPED_QUIET_GREP.test(line)),
+      )
+      .map((hit) => `${file} › ${hit.where} › line ${hit.number}: ${hit.line}`),
+  )
+
+  it('has none', () => {
+    expect(offenders).toEqual([])
+  })
+})