fix(npm): wait for npm to apply each publish, and drop the one-time @gitlawb/zero steps

.github/workflows/release-artifacts.yml

@@ -206,55 +206,10 @@ jobs:
       # Platform payloads first under the `platform` dist-tag, then the wrapper,
       # with the `latest` dist-tag checked before and after (platform versions
       # are semver PRERELEASES of the wrapper version and must never become
-      # `latest`). Versions already on the registry are skipped, so "Re-run
-      # failed jobs" is safe after a partial publish.
+      # `latest`). npm applies publishes asynchronously, so the script waits
+      # until npm shows every platform version before the wrapper goes out,
+      # and until `latest` moves after it. Versions already on the registry
+      # are skipped, so "Re-run failed jobs" is safe after a partial publish.
       - name: Publish npm packages
         shell: bash
         run: scripts/npm/publish-packages.sh dist/npm
-
-      # One final release under the pre-rename name @gitlawb/zero. Zero 0.9.0's
-      # `zero upgrade` runs `npm install -g @gitlawb/zero@latest`, and people
-      # reinstall the old name from shell history and old tutorials, so this
-      # release is published once more under it: the same verified artifacts,
-      # assembled as @gitlawb/zero, which the binary and wrapper recognise and
-      # answer with the switch steps (docs/NPM_PACKAGING.md). Runs after the
-      # @twigpine/zero publish, so the package it points at already exists.
-      #
-      # The gate keeps this to a single release: it is owed only while the old
-      # name's latest is still the last pre-rename version, or is this version
-      # (a re-run after it went out). After this release, `npm deprecate` the
-      # old package and delete these two steps.
-      - name: Check whether the final @gitlawb/zero release is owed
-        id: legacy-name
-        shell: bash
-        env:
-          LAST_PRE_RENAME_VERSION: 0.9.0
-        run: |
-          version="$(node -p "require('./package.json').version")"
-          latest=""
-          for _ in 1 2 3; do
-            latest="$(npm view @gitlawb/zero dist-tags.latest 2>/dev/null || true)"
-            if [ -n "$latest" ]; then
-              break
-            fi
-            sleep 10
-          done
-          if [ -z "$latest" ]; then
-            echo "::error::could not read the latest dist-tag of @gitlawb/zero from the registry" >&2
-            exit 1
-          fi
-          if [ "$latest" = "$LAST_PRE_RENAME_VERSION" ] || [ "$latest" = "$version" ]; then
-            owed=true
-          else
-            owed=false
-          fi
-          echo "@gitlawb/zero latest is ${latest}; final release under the old name owed: ${owed}"
-          echo "owed=${owed}" >> "$GITHUB_OUTPUT"
-
-      - name: Publish the final @gitlawb/zero release
-        if: ${{ steps.legacy-name.outputs.owed == 'true' }}
-        shell: bash
-        run: |
-          node scripts/npm/build-platform-packages.mjs \
-            --artifacts-dir dist/artifacts --out-dir dist/npm-legacy --package-name @gitlawb/zero
-          scripts/npm/publish-packages.sh dist/npm-legacy

docs/NPM_PACKAGING.md

@@ -130,9 +130,14 @@ registry.
    bootstrap publish) — a missing value must not abort the job between the
    platform and wrapper publishes, which would strand a half-published
    release.
-2. **Platform versions publish before the wrapper.** The wrapper's
-   `optionalDependencies` pin exact suffixed versions; publishing the wrapper
-   first would create a window where installs resolve aliases that 404.
+2. **Platform versions are on the registry before the wrapper.** The
+   wrapper's `optionalDependencies` pin exact suffixed versions; publishing
+   the wrapper first would create a window where installs resolve aliases
+   that 404. Ordering the uploads is not enough: npm applies a publish
+   asynchronously, and in 0.10.0 a platform version appeared ten minutes
+   after its upload returned, after the wrapper. The publish script waits
+   until npm shows every platform version before it publishes the wrapper,
+   and waits the same way for `latest` afterwards (up to 30 minutes in all).
 3. **Exact-version pinning.** The wrapper at `X.Y.Z` references platform
    versions `X.Y.Z-<platform>-<arch>` exactly — never ranges — so a wrapper
    and its binaries can never skew. The publish script refuses to start when
@@ -161,54 +166,31 @@ bin, so npm refuses to install one over the other (`EEXIST`), and forcing it
 lets a later uninstall of the old package delete the shared bin. The user
 has to uninstall the old package and install the new one, in that order.
 
-Deprecating the old package alone does not reach existing users. 0.9.0's
-`zero upgrade` runs `npm install -g @gitlawb/zero@latest`; with nothing newer
-under that name, npm reinstalls nothing, prints no deprecation warning (it
-only warns when it installs something), and Zero reports the update as done.
-So 0.10.0 is published once more under the old name, as a bridge:
-
-- The publish job assembles the same verified artifacts a second time with
-  `build-platform-packages.mjs --package-name @gitlawb/zero` and publishes
-  them with the same script, after the `@twigpine/zero` publish. A gate runs
-  it only while the old name's `latest` is still 0.9.0 (or is this version,
-  on a re-run), so it cannot happen twice.
+Deprecating the old package alone would not have reached existing users.
+0.9.0's `zero upgrade` runs `npm install -g @gitlawb/zero@latest`; with
+nothing newer under that name, npm reinstalls nothing, prints no deprecation
+warning (it only warns when it installs something), and Zero reports the
+update as done. So 0.10.0 was published once more under the old name, as a
+bridge:
+
+- The release job assembled the same verified artifacts a second time with
+  `build-platform-packages.mjs --package-name @gitlawb/zero` and published
+  them with the same script, after the `@twigpine/zero` publish. The steps
+  that did this ran once and have been removed from `release-artifacts.yml`.
 - The bridge build knows its name from the `package.json` beside it. The Go
   binary reports it as `InstallMethodNpmLegacy`: `zero upgrade` refuses
   with the switch steps instead of reinstalling it, and `zero update --check`
   adds them. `bin/zero.js` prints the same steps after `zero` exits, on a
-  terminal only.
-- Updating from 0.9.0 now installs a real version, so npm also shows the
+  terminal only. This handling stays in the code, since only a build
+  installed under the old name ever triggers it.
+- Updating from 0.9.0 installs that bridge build, so npm also shows the
   deprecation message.
 
-### Runbook
-
-Before the first release under the new name:
-
-1. Create the package, since npm only attaches a trusted publisher to a
-   package that exists: publish an empty `@twigpine/zero@0.0.0` by hand with
-   `npm publish --access public --tag bootstrap`, then
-   `npm deprecate @twigpine/zero@0.0.0 "Placeholder; install @twigpine/zero@latest"`.
-2. Trust this workflow for it:
-   `npm trust github @twigpine/zero --repo Twigpine/zero --file release-artifacts.yml --env npm-publish --allow-publish`.
-3. Point the old package's trusted publisher at this repository too, for the
-   bridge publish: `npm trust list @gitlawb/zero`, revoke the existing entry
-   with `npm trust revoke @gitlawb/zero --id <id>`, then run the same
-   `npm trust github` command for `@gitlawb/zero`.
-4. Confirm Settings → Environments → `npm-publish` has a required reviewer.
-   The environment exists either way; without a reviewer the job does not
-   wait for approval.
-
-After the release, once both packages show 0.10.0 as `latest`:
-
-1. `npm deprecate @gitlawb/zero "Zero moved to @twigpine/zero. Switch with: npm uninstall -g @gitlawb/zero && npm install -g @twigpine/zero"`
-   (no version range, so every version, including the platform payloads,
-   carries it). Never unpublish: existing installs resolve their platform
-   payload through these versions.
-2. Revoke the old package's trusted publisher (`npm trust revoke`), so
-   nothing can publish under that name again.
-3. Delete the two `@gitlawb/zero` steps from `release-artifacts.yml`. The Go
-   and wrapper handling of the old name can stay; it only activates in a
-   build installed under that name.
+`@gitlawb/zero` is deprecated on every version and no longer has a trusted
+publisher, so this release workflow cannot publish it again; another version
+would take a maintainer publishing by hand. Its versions stay on the
+registry: existing installs resolve their platform payload through them, so
+they must never be unpublished.
 
 ## Runbook: bumping the vendored helpers
 

internal/npmwrapper/publish_test.go

@@ -23,6 +23,10 @@ import (
 // FAKE_NPM_CLOBBER_LATEST=1 makes every publish take `latest`, the failure the
 // script must stop on. FAKE_NPM_VERSION_LOOKUP_ERRORS=<n> fails the first n
 // version lookups with a network error instead of answering them.
+// FAKE_NPM_APPLY_AFTER=<n> applies each publish only after n further lookups,
+// the way npm accepts an upload and shows the version minutes later; until
+// then the version and its dist-tag are invisible. When a wrapper is published,
+// the aliased versions not yet visible are recorded in missingAtWrapperPublish.
 const fakeNpm = `#!/usr/bin/env node
 const fs = require('fs');
 const stateFile = process.env.FAKE_NPM_STATE;
@@ -31,7 +35,17 @@ const args = process.argv.slice(2);
 fs.appendFileSync(stateFile + '.log', JSON.stringify({ args, cwd: process.cwd() }) + '\n');
 const save = () => fs.writeFileSync(stateFile, JSON.stringify(state));
 const notFound = (detail) => { process.stderr.write('npm error code E404\nnpm error 404 ' + detail + '\n'); process.exit(1); };
+state.pending = state.pending || [];
+const apply = (publish) => {
+  state.versions.push(publish.version);
+  state.distTags[publish.tag] = publish.version;
+  if (process.env.FAKE_NPM_CLOBBER_LATEST === '1') state.distTags.latest = publish.version;
+};
 if (args[0] === 'view') {
+  for (const publish of state.pending) publish.wait -= 1;
+  state.pending.filter((publish) => publish.wait <= 0).forEach(apply);
+  state.pending = state.pending.filter((publish) => publish.wait > 0);
+  save();
   const [spec, field] = args.slice(1);
   const at = spec.lastIndexOf('@');
   const name = at > 0 ? spec.slice(0, at) : spec;
@@ -62,14 +76,19 @@ if (args[0] === 'publish') {
     process.stderr.write('fake npm: refusing to publish ' + pkg.name + '\n');
     process.exit(1);
   }
-  if (state.versions.includes(pkg.version)) {
+  if (state.versions.includes(pkg.version) || state.pending.some((publish) => publish.version === pkg.version)) {
     process.stderr.write('npm error code EPUBLISHCONFLICT\n');
     process.exit(1);
   }
+  if (pkg.optionalDependencies) {
+    state.missingAtWrapperPublish = Object.values(pkg.optionalDependencies)
+      .map((spec) => spec.slice(spec.lastIndexOf('@') + 1))
+      .filter((version) => !state.versions.includes(version));
+  }
   const tagIndex = args.indexOf('--tag');
-  state.versions.push(pkg.version);
-  state.distTags[tagIndex === -1 ? 'latest' : args[tagIndex + 1]] = pkg.version;
-  if (process.env.FAKE_NPM_CLOBBER_LATEST === '1') state.distTags.latest = pkg.version;
+  const publish = { version: pkg.version, tag: tagIndex === -1 ? 'latest' : args[tagIndex + 1], wait: Number(process.env.FAKE_NPM_APPLY_AFTER || 0) };
+  if (publish.wait > 0) state.pending.push(publish);
+  else apply(publish);
   save();
   process.exit(0);
 }
@@ -88,6 +107,9 @@ type fakeRegistryState struct {
 	DistTags map[string]string `json:"distTags"`
 	// VersionLookups counts `npm view <name>@<version> version` calls.
 	VersionLookups int `json:"versionLookups,omitempty"`
+	// MissingAtWrapperPublish lists the platform versions the wrapper aliased
+	// that npm did not show yet when the wrapper was published.
+	MissingAtWrapperPublish []string `json:"missingAtWrapperPublish,omitempty"`
 }
 
 type npmCall struct {
@@ -325,3 +347,87 @@ func TestPublishPackagesFailsWhenTheRegistryCannotBeReached(t *testing.T) {
 		t.Fatalf("stderr does not explain the failure or show npm's error: %s", run.stderr)
 	}
 }
+
+// npm accepts an upload and shows the version minutes later (ten minutes for a
+// 0.10.0 platform payload). Publishing the wrapper as soon as the platform
+// uploads return would leave installs resolving aliases that 404, and checking
+// `latest` right after the wrapper upload reports a failure that is not one.
+func TestPublishPackagesWaitsForNpmToApplyEachPublish(t *testing.T) {
+	platforms := []string{"darwin-arm64", "linux-x64"}
+	npmDir := writePublishFixture(t, platforms, platforms)
+	run := runPublishPackages(t, npmDir, fakeRegistryState{Name: publishFixtureName}, "FAKE_NPM_APPLY_AFTER=8")
+	if run.exitCode != 0 {
+		t.Fatalf("exit = %d while npm applied the publishes late; stdout=%s stderr=%s", run.exitCode, run.stdout, run.stderr)
+	}
+	if len(run.state.MissingAtWrapperPublish) != 0 {
+		t.Fatalf("the wrapper went out while npm did not show %v yet; installs would resolve aliases that 404", run.state.MissingAtWrapperPublish)
+	}
+	if latest := run.state.DistTags["latest"]; latest != publishFixtureVersion {
+		t.Fatalf("latest = %q, want %q", latest, publishFixtureVersion)
+	}
+}
+
+// A publish npm never applies must fail the job at the deadline, before the
+// wrapper goes out, instead of waiting forever.
+func TestPublishPackagesGivesUpWhenNpmNeverAppliesAPublish(t *testing.T) {
+	platforms := []string{"linux-x64"}
+	npmDir := writePublishFixture(t, platforms, platforms)
+	run := runPublishPackages(t, npmDir, fakeRegistryState{Name: publishFixtureName},
+		"FAKE_NPM_APPLY_AFTER=1000000", "ZERO_NPM_PUBLISH_WAIT_TIMEOUT=1")
+	if run.exitCode == 0 {
+		t.Fatalf("exit = 0 although npm never showed the platform version; stdout=%s", run.stdout)
+	}
+	if got := strings.Join(run.publishedDirs(), ","); got != "zero-linux-x64" {
+		t.Fatalf("published %s, want only the platform payload (the wrapper must wait for it)", got)
+	}
+	if !strings.Contains(run.stderr, "still does not show "+publishFixtureName+"@"+publishFixtureVersion+"-linux-x64") {
+		t.Fatalf("stderr does not say what npm never showed: %s", run.stderr)
+	}
+}
+
+// Bash arithmetic reads a leading zero as octal and rejects 08 outright, so
+// the timeout has to be converted explicitly rather than used as typed. The
+// rejection does not stop a script: it prints an error, leaves the deadline
+// unset and carries on, so the failure only shows once the script has to wait,
+// after the platform payloads are out. npm applying late makes it wait here.
+func TestPublishPackagesReadsTheWaitTimeoutAsDecimal(t *testing.T) {
+	platforms := []string{"linux-x64"}
+	npmDir := writePublishFixture(t, platforms, platforms)
+	run := runPublishPackages(t, npmDir, fakeRegistryState{Name: publishFixtureName},
+		"ZERO_NPM_PUBLISH_WAIT_TIMEOUT=08", "FAKE_NPM_APPLY_AFTER=3")
+	if run.exitCode != 0 {
+		t.Fatalf("exit = %d with ZERO_NPM_PUBLISH_WAIT_TIMEOUT=08; stdout=%s stderr=%s", run.exitCode, run.stdout, run.stderr)
+	}
+	if got := strings.Join(run.publishedDirs(), ","); got != "zero-linux-x64,wrapper" {
+		t.Fatalf("published %s, want zero-linux-x64,wrapper", got)
+	}
+}
+
+// A setting the script cannot use must stop it before anything is published:
+// a negative timeout would otherwise end the wait before it starts, and a bad
+// delay would only fail at the first sleep, after the platform payloads.
+func TestPublishPackagesRejectsAnInvalidWaitSetting(t *testing.T) {
+	for _, tc := range []struct{ variable, value string }{
+		{"ZERO_NPM_PUBLISH_WAIT_TIMEOUT", "-5"},
+		{"ZERO_NPM_PUBLISH_WAIT_TIMEOUT", "abc"},
+		{"ZERO_NPM_PUBLISH_WAIT_TIMEOUT", "1e3"},
+		{"ZERO_NPM_PUBLISH_WAIT_TIMEOUT", "86401"},
+		{"ZERO_NPM_PUBLISH_WAIT_TIMEOUT", "9999999999999999999999"},
+		{"ZERO_NPM_PUBLISH_RETRY_DELAY", "soon"},
+	} {
+		t.Run(tc.variable+"="+tc.value, func(t *testing.T) {
+			platforms := []string{"linux-x64"}
+			npmDir := writePublishFixture(t, platforms, platforms)
+			run := runPublishPackages(t, npmDir, fakeRegistryState{Name: publishFixtureName}, tc.variable+"="+tc.value)
+			if run.exitCode == 0 {
+				t.Fatalf("exit = 0 with %s=%s; stdout=%s", tc.variable, tc.value, run.stdout)
+			}
+			if len(run.publishes) != 0 {
+				t.Fatalf("published %v before rejecting %s=%s, want nothing", run.publishedDirs(), tc.variable, tc.value)
+			}
+			if !strings.Contains(run.stderr, tc.variable) {
+				t.Fatalf("stderr does not name %s: %s", tc.variable, run.stderr)
+			}
+		})
+	}
+}

internal/update/installmethod.go

@@ -17,11 +17,11 @@ import (
 const npmPackageName = "@twigpine/zero"
 
 // legacyNpmPackageName is the name zero was published under on npm up to
-// 0.9.0. The 0.10.0 release is published once more under it (the legacy step
-// in .github/workflows/release-artifacts.yml), so that 0.9.0's `zero upgrade`
-// and anyone still installing the old name land on a build that says where
-// the package went. Nothing is published under it after that, which is why an
-// install of it is told to switch instead of offered an update.
+// 0.9.0. The 0.10.0 release was published once more under it, so that 0.9.0's
+// `zero upgrade` and anyone still installing the old name land on a build that
+// says where the package went (docs/NPM_PACKAGING.md). Nothing is published
+// under it after that, which is why an install of it is told to switch instead
+// of offered an update.
 const legacyNpmPackageName = "@gitlawb/zero"
 
 // InstallMethod identifies how the running zero binary was installed.

scripts/npm/build-platform-packages.mjs

@@ -25,10 +25,11 @@
 // CI must always assemble the full matrix, and the wrapper's
 // optionalDependencies always reference the full matrix regardless).
 //
-// --package-name assembles the same artifacts under another name. It exists
-// for the one final release under the pre-rename name @gitlawb/zero (see
-// docs/NPM_PACKAGING.md); everything in the payloads that names the package
-// follows it, so the binary and wrapper know which name they were installed as.
+// --package-name assembles the same artifacts under another name. It was
+// added for the one release (0.10.0) also published under the pre-rename name
+// @gitlawb/zero (see docs/NPM_PACKAGING.md); everything in the payloads that
+// names the package follows it, so the binary and wrapper know which name they
+// were installed as.
 //
 // Usage:
 //   node scripts/npm/build-platform-packages.mjs \