From 25540a9b9e6581af7dd8edf582cf718e8f6f918d Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 12 Aug 2026 12:24:29 +1000 Subject: [PATCH 1/3] fix(release): name the crate for `neon dist`, which pnpm exec cannot default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every platform leg of `_build-ffi-artifacts.yml` failed with `error: $npm_package_name is not defined`, so the FFI release pipeline could not have produced a single tarball. Found by the first dispatch of `ffi-preflight.yml` (run 31555436823) — which is what it is for. `neon dist` locates the compiled cdylib in the cargo log by crate name, defaulting it to `basename($npm_package_name)`. That variable comes from the package-script runner. The step invokes the binary directly, because that is the only way to pass `-o platforms//index.node` and so avoid the bare-`neon dist` default of `./index.node` — the `debug:` fallback in `load.cts`, right locally and wrong in a published tarball. Upstream never met this: its `postcargo-build` / `postzig-build` hooks run `neon dist` as lifecycle scripts, where the variable exists. Nor would the plan's spelling have, which was `npx` — npx sets `npm_package_name`, `pnpm exec` does not. Moving to `pnpm exec` was right on its own terms, npx will fetch a missing binary over the network, but it removed the default's only source and nothing said so. `scripts/__tests__/neon-dist-crate-name.test.mjs` pins both halves across every workflow, discovered rather than listed: the flag is present, and its value equals the crate in `crates/protect-ffi/Cargo.toml`. The second is the quiet one — a renamed crate with a stale `-n` does not error, it generates nothing. No changeset: repo tooling, no published surface. The pipeline remains inert until the trusted-publishing cutover. --- .github/workflows/_build-ffi-artifacts.yml | 11 ++- ...6-08-04-protect-ffi-monorepo-absorption.md | 15 +++- .../__tests__/neon-dist-crate-name.test.mjs | 89 +++++++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 scripts/__tests__/neon-dist-crate-name.test.mjs diff --git a/.github/workflows/_build-ffi-artifacts.yml b/.github/workflows/_build-ffi-artifacts.yml index 62d0865cf..ae97048c9 100644 --- a/.github/workflows/_build-ffi-artifacts.yml +++ b/.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, diff --git a/docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md b/docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md index be4635bd6..003637198 100644 --- a/docs/plans/2026-08-04-protect-ffi-monorepo-absorption.md +++ b/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/

/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" diff --git a/scripts/__tests__/neon-dist-crate-name.test.mjs b/scripts/__tests__/neon-dist-crate-name.test.mjs new file mode 100644 index 000000000..2238c4d53 --- /dev/null +++ b/scripts/__tests__/neon-dist-crate-name.test.mjs @@ -0,0 +1,89 @@ +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 `, + * 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//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`. + * + * Two ways for that to come back, hence two assertions: + * + * - 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. + * + * 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/ + +describe('neon dist through pnpm exec', () => { + const calls = workflowFiles().flatMap((file) => + runSteps(readWorkflow(file)) + .filter((step) => NEON_DIST.test(step.run)) + .map((step) => ({ file, ...step })), + ) + + it('is invoked somewhere, or this guard is checking nothing', () => { + expect(calls.length).toBeGreaterThan(0) + }) + + it.each(calls)('$file › $jobId › $name passes the crate name', ({ run }) => { + // Line continuations first: the flag and its value are allowed to be split + // across lines, and without this the pattern below would not see them. + const flat = run.replace(/\\\n\s*/g, ' ') + const named = flat.match(/(?:^|\s)(?:-n|--name)\s+(\S+)/) + expect( + named, + 'neon dist needs -n; $npm_package_name is unset under pnpm exec', + ).not.toBeNull() + expect(named[1]).toBe(crateName()) + }) +}) From 8105da2b47fd83c957fc7bd76387f92f0c53610f Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 12 Aug 2026 12:50:55 +1000 Subject: [PATCH 2/3] fix(release): stop piping into `grep -q`, which pipefail reads as failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second defect the pre-flight found, and the one the first was hiding. With the crate name fixed, both Darwin legs went green and the other four failed one step later: ` has no index.node`, on a tarball whose own pack listing shows index.node present. `grep -q` exits at the first match. The writer upstream then takes SIGPIPE, and under `pipefail` — which GitHub sets for every `shell: bash` step before the block's own `set -euo pipefail` — the pipeline's status becomes the killed writer's 141. The match succeeded and the pipeline reports failure. It presents as a platform problem, which is why it cost a full matrix run to see. GNU tar writes an entry at a time and hits it; bsdtar buffers a four-entry listing into one write and does not. Hence macOS green, Linux and Windows red, on the packaging step, in a job that had just changed. Three call sites, all in the FFI release path, and they do not fail in the same direction: - `_build-ffi-artifacts.yml` `tar tzf | grep -qx package/index.node` fails CLOSED — this run's visible failure. - `ffi-preflight.yml`'s glibc check on the two gnu platforms, also closed. - `ffi-preflight.yml`'s musl check fails OPEN. `if readelf | grep -q` with a poisoned status is false, so it reports "no glibc NEEDED entry" for precisely the binary it exists to reject — a gnu build shipped inside the musl package, landing on an Alpine user at dlopen rather than in CI. A dynamic section is much longer than a tarball listing, so it would have been reliably wrong. Neither preflight check had ever executed. All three capture first and match against the variable. `scripts/__tests__/workflow-grep-q-pipelines.test.mjs` rejects the pattern across every workflow and composite action. `grep -q` against a file or a here-string is untouched: no writer, no signal. --- .github/workflows/_build-ffi-artifacts.yml | 15 ++- .github/workflows/ffi-preflight.yml | 15 ++- .../workflow-grep-q-pipelines.test.mjs | 96 +++++++++++++++++++ 3 files changed, 123 insertions(+), 3 deletions(-) create mode 100644 scripts/__tests__/workflow-grep-q-pipelines.test.mjs diff --git a/.github/workflows/_build-ffi-artifacts.yml b/.github/workflows/_build-ffi-artifacts.yml index ae97048c9..84d951239 100644 --- a/.github/workflows/_build-ffi-artifacts.yml +++ b/.github/workflows/_build-ffi-artifacts.yml @@ -299,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 diff --git a/.github/workflows/ffi-preflight.yml b/.github/workflows/ffi-preflight.yml index 0600416d1..729e20760 100644 --- a/.github/workflows/ffi-preflight.yml +++ b/.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 diff --git a/scripts/__tests__/workflow-grep-q-pipelines.test.mjs b/scripts/__tests__/workflow-grep-q-pipelines.test.mjs new file mode 100644 index 000000000..7d0b3401b --- /dev/null +++ b/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([]) + }) +}) From 0038284bd29d0c6169164462b2d7416924087e87 Mon Sep 17 00:00:00 2001 From: Toby Hede Date: Wed, 12 Aug 2026 13:43:15 +1000 Subject: [PATCH 3/3] test(release): scan each `neon dist` call, not the step that holds it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding, and the gap was wider than reported. The guard matched a single non-global regex over the whole `run:` body, so a step with two `neon dist` calls was judged by whichever `-n` appeared first — and the flag it found did not have to belong to a `neon dist` at all. `echo -n protect-ffi` on an earlier line satisfied it, as the new coverage shows. The scan now splits a body into individual commands (continuations joined first, so a newline is a real boundary) and holds every invocation to the crate name separately. Two tests drive the scanner over shapes no workflow in the tree has: a second unnamed call behind a correctly named one, and a `-n` belonging to a neighbouring command. Both fail against the previous body-wide match — `length 1, expected 2` and `['protect-ffi'], expected [undefined]` — so they pin the fix rather than restating it. Reported alongside a suggested test that scanned an inline string with logic defined in the test body. That version never calls the scanner, so its result does not move when the scanner changes: it would have failed before and after. These call the real thing. --- .../__tests__/neon-dist-crate-name.test.mjs | 94 ++++++++++++++++--- 1 file changed, 79 insertions(+), 15 deletions(-) diff --git a/scripts/__tests__/neon-dist-crate-name.test.mjs b/scripts/__tests__/neon-dist-crate-name.test.mjs index 2238c4d53..f544c25d5 100644 --- a/scripts/__tests__/neon-dist-crate-name.test.mjs +++ b/scripts/__tests__/neon-dist-crate-name.test.mjs @@ -24,14 +24,22 @@ import { readWorkflow, workflowFiles } from './lib/workflows.mjs' * this repository's, and it failed all six platform legs with * `error: $npm_package_name is not defined`. * - * Two ways for that to come back, hence two assertions: + * 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. + * 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. @@ -64,26 +72,82 @@ function runSteps(workflow) { 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 calls = workflowFiles().flatMap((file) => - runSteps(readWorkflow(file)) - .filter((step) => NEON_DIST.test(step.run)) - .map((step) => ({ file, ...step })), + 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(calls.length).toBeGreaterThan(0) + expect(invocations.length).toBeGreaterThan(0) }) - it.each(calls)('$file › $jobId › $name passes the crate name', ({ run }) => { - // Line continuations first: the flag and its value are allowed to be split - // across lines, and without this the pattern below would not see them. - const flat = run.replace(/\\\n\s*/g, ' ') - const named = flat.match(/(?:^|\s)(?:-n|--name)\s+(\S+)/) + it.each( + invocations, + )('$file › $jobId › $name › call $nth passes the crate name', ({ + command, + }) => { expect( - named, + crateNameFlag(command), 'neon dist needs -n; $npm_package_name is unset under pnpm exec', - ).not.toBeNull() - expect(named[1]).toBe(crateName()) + ).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]) }) })