fix(attw): the gate's nested npm pack no longer inherits a dry run (CONFIG-PREPUBLISH-ATTW-ENOENT) - #47
Merged
Merged
Conversation
…ONFIG-PREPUBLISH-ATTW-ENOENT) `packages/test-utils/test/attw-gate.test.ts` shells out to a real `attw --pack`, `prepublishOnly` runs `pnpm test`, and `pnpm -r publish --dry-run` runs `prepublishOnly`. Seven cases red with `ENOENT: ... attw-gate-fixture-unpacked-1.0.0.tgz` on the 0.0.3 Version PR (#46), on a tree whose only change under `packages/test-utils/` was its CHANGELOG. It was invisible until then because `publish --dry-run` SKIPS a version already on npm, so the chain runs on nothing but a version bump. THE MECHANISM, MEASURED AT BOTH ENDS. `pnpm publish --dry-run` exports `npm_config_dry_run=true` into every lifecycle script it runs; `npm pack` honours it and writes no tarball; `attw` then opens the path it computed from the manifest (`<dir>/<name>-<version>.tgz`) and never asks npm where the file went. `npm_config_pack_destination` is the same fault from the other side. npm honours both keys upper-cased and hyphenated too. THE FIX IS AT THE SOURCE. `scripts/attw.mjs` strips those two keys, and only those two, from the environment of the `attw` child, in both byte-identical copies, so every scaffolded parser inherits it. `npm_config_registry` and the rest are left alone deliberately: they change what attw RESOLVES rather than where npm writes. Nothing about what the gate checks changed. PLANTED, NOT ASSERTED. The suite strips the same two keys from its own subprocesses (it is itself run from `prepublishOnly`) and plants them back: on the bare CLI, where they must still produce ENOENT with the untyped sentence absent, and on the wrapper, where they must not, in each spelling. A negative control pins that a package which reds on its own merits still reds with the plant. MEASURED. Base, under the failing condition: 7 of 29 red, matching #46's CI exactly. With the fix: 35 of 35 green, and `pnpm -r publish --dry-run --no-git-checks` with the version bumped to 0.0.3 exits 0 with all 81 tests passing inside `prepublishOnly`. TWO CLAIMS RETRACTED, and the prose corrected rather than quietly reworded. The pack does not land where attw cannot find it in a staging context: it is never written. And a real publish would NOT have failed identically, because a non-dry-run `pnpm publish` sets no `dry_run` (measured: its lifecycle environment carries registry, cache and user_agent, and nothing that moves a pack). This class has never broken a release. It breaks the dry run that exists to prove one, which is what blocks a Version PR. No changeset, deliberately: `scripts/` is in no package's `files`, so no published tarball changes and a changeset would burn a version on identical bytes.
…e older one
Pass 1 of the gate refuted a claim, not the fix. The strip said "both of npm's
spellings of each were measured to take effect", listing `npm_config_dry-run`.
It does take effect on npm launched directly, but attw packs with
`execSync("npm pack")` and dash will not export a name that is not a valid shell
identifier, so it never arrives. Measured both ways round:
env npm_config_dry-run=true npm pack -> no tarball
env npm_config_dry-run=true sh -c 'npm pack' -> tarball written
So the case that planted the hyphen through the wrapper was pinning nothing: it
passed against the unfixed wrapper too. It is replaced by an honest one on the
BARE CLI, asserting the limit itself, with a note to grow the spelling list
rather than delete the case if the shell hop ever goes away. The regex keeps the
hyphen as a superset, which costs a character and stays right if anything ever
spawns npm without a shell. `RELEASING.md` and the root CHANGELOG say the same
narrower thing.
Also corrected, PRE-EXISTING and in the paragraph this slice already rewrites:
`RELEASING.md` said attw "still runs, twice", counting the `Pack integrity` job.
That job runs `npm pack --dry-run` and never invokes attw. It is a second net on
the tarball, not a second attw run.
Still 35 of 35 green under `npm_config_dry_run=true`, 81 of 81 for the package,
82 of 82 at the root, and the two wrapper copies remain byte-identical.
…ures the shell
Pass 2 of the gate refuted the wording of pass 1's remedy, not the fix. "A shell
will not export a name that is not a valid shell identifier" is dash's answer
stated as every shell's. Measured here, on the same probe package:
env npm_config_dry-run=true /bin/sh -c 'npm pack' -> tarball written (dash drops it)
env npm_config_dry-run=true /bin/bash -c 'npm pack' -> NO tarball (bash forwards it)
So the hyphen is dead on Debian and on ubuntu-latest, and live wherever /bin/sh
is bash. The previous case asserted dash's answer unconditionally and would have
reddened on such a box, which is a test that fails for a reason outside what it
is about.
It now PROBES `/bin/sh` for the answer and asserts the counterfactual that shell
actually produces, then pins the only claim this gate makes on both: the wrapper
strips the hyphen either way, which is what its superset regex is for. The
docblock, RELEASING.md and the root CHANGELOG say the narrower thing, and record
both wrong drafts rather than quietly replacing them.
No change to the fix, to what the gate checks, or to the counts: 35 of 35 green
under `npm_config_dry_run=true`, and the wrapper copies stay byte-identical.
…a pin Pass 3 graded the remedy only, per ADR 0016, and returned two minor findings. Both are taken verbatim as the refuter framed them, neither grows the guard, and there is no fourth pass. 1. The shell probe resolved `sh` from PATH while attw's `execSync` takes `/bin/sh` literally. With a PATH `sh` pointing at bash on a dash box the case measured one shell and attw used the other, and reddened for a reason that is not about this gate. It now spawns `/bin/sh`. 2. The wrapper half of that case was labelled THE PIN. On dash it pins nothing: the hyphen never reaches npm, so the pre-fix wrapper satisfies those three assertions identically. It is relabelled as coverage that becomes a pin on a bash-as-`sh` box, with the underscore cases named as the non-vacuous ones. Measured against the pre-fix wrapper on the same fixture: hyphen gives exit 1 with the untyped sentence and no ENOENT either way; underscore gives exit 3 with ENOENT before the fix and exit 1 with the sentence after it. Also narrowed, from the same pass: the root CHANGELOG said the plants must produce ENOENT "in each spelling", which is only true of the two underscore ones, and the "ubuntu-latest runner" phrasing is now stated as what Debian and Ubuntu ship rather than as something measured on a runner from here. Counts unchanged: 35 of 35 under `npm_config_dry_run=true`, 82 of 82 at the root, copies byte-identical, no change to the fix or to what the gate checks.
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
1 participant
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.
Closes
CONFIG-PREPUBLISH-ATTW-ENOENT, which is what redsrelease-dry-runon the Version PR #46. #46 is not touched here and must not be merged to get green:@cosyte/test-utilsis the only one of the six with aprepublishOnly, so merging it in this state burns five versions and fails the sixth.The mechanism, measured at both ends
pnpm publish --dry-runexportsnpm_config_dry_run=trueinto every lifecycle script it runs.prepublishOnlyrunspnpm test,packages/test-utils/test/attw-gate.test.tsshells out to a realattw --pack, andattwpacks withexecSync("npm pack")and then opens a path it computed from the manifest (<dir>/<name>-<version>.tgz). Under that variablenpm packprints its listing and writes nothing, so attw opens a file that was never written: seven cases red withENOENT, on a tree whose only change underpackages/test-utils/was aCHANGELOG.md.npm_config_pack_destinationis the same fault from the other side.Reproduced locally, byte for byte: base under
npm_config_dry_run=truegives 7 of 29 red, the same seven CI reported.Two claims retracted, with evidence
pnpm publishsets nodry_run(its lifecycle environment carriesregistry,cache,user_agentand nothing that moves a pack). This class has never broken a release. It breaks the dry run that exists to prove one, which is what blocks a Version PR.RELEASING.md,scripts/parser-template/CLAUDE.mdandtest/attw-scaffold.test.tsare corrected in place; the rootCHANGELOG.mdrecords the retraction rather than rewriting the shipped entry.The fix
scripts/attw.mjsstrips those two keys, and only those two, from the environment of theattwchild, in both byte-identical copies, so every scaffolded parser inherits it.npm_config_registryand the rest are left alone deliberately: they change what attw resolves, not where npm writes. Nothing about what the gate checks changed (same argument allow-list, same.attw.jsonrefusal, same preflight, same post-check, same exit-code forwarding). The one behavioural delta in the wrapper is four lines plusenvon thespawnSync.The suite strips the same two keys from its own subprocesses (it is itself run from
prepublishOnly) and then plants them back: on the bare CLI, where the underscore spellings must still produceENOENT, and on the wrapper, where they must not. The hyphenated key npm also honours is a shell question rather than an npm one (dash drops a name that is not a valid shell identifier, bash forwards it), so that case probes/bin/shand asserts whichever answer the box gives.Verification
attw-gate.test.ts: 35 of 35 green, including undernpm_config_dry_run=true. Package: 81 of 81. Root: 82 of 82.npm_config_ignore_scripts=false pnpm -r publish --dry-run --no-git-checkswith@cosyte/test-utilsbumped to0.0.3(the shape of Version Packages #46): exit 0,prepublishOnlyactually executing.typecheck,lint --max-warnings=0,format:check,check:no-emdashgreen.pnpm auditand the license allowlist fail identically onmain(PRE-EXISTING, untouched here: this slice changes no manifest and no lockfile).Gate
Three
gate-refuterpasses (ADR 0016, no fourth). Pass 1NOT REFUTEDwith oneINTRODUCEDminor (a spelling claim the gate could not support, and one vacuous test case); pass 2NOT REFUTEDwith oneINTRODUCEDminor (that remedy stated dash's behaviour as every shell's); pass 3, narrow, against the remedy diff only,NOT REFUTEDwith twoINTRODUCEDminors (probeshfrom PATH rather than/bin/sh; a mislabelled assertion). Every remedy was a correction to a claim or a label. The guard was never grown to answer a finding.Changeset
None, deliberately.
scripts/is in no package'sfiles, so no published tarball changes; a changeset would burn a version on identical bytes. The repo-level entry goes in the rootCHANGELOG.md, which is exactly whatRELEASING.mdsays to do with a repo-level note.node scripts/changeset-guard.mjsis green (zero changesets is the publish arm, not a failure).Not done here, on purpose
The umbrella's
documentation/repos/config.mdstill carries the retracted "staging context" cause and is stale about the template'sprepublishOnly. That is outside this submodule and belongs to the pointer bump.