diff --git a/framework-dev.md b/framework-dev.md index ec16ba31a..550576070 100644 --- a/framework-dev.md +++ b/framework-dev.md @@ -324,6 +324,8 @@ changelog/core/0.7.52.md changelog/core/0.7.53.md changelog/server/0.8.66.md Space or comma separated. Every path is validated to exist before anything publishes, because a typo that silently published nothing would look exactly like success. The set is then sorted by its `date:` frontmatter ASC, the same ordering the push path uses, so core still publishes before server and npm's `latest` tag lands on the newest version rather than on whichever path was typed last. Every publish script is idempotent, so naming an already-published version is a no-op. Listing a `cli` changelog also republishes the two unscoped wrappers at that cli version. +**You can only republish a version the workspace still holds.** `npm publish --workspace=` ships whatever is in that package's `package.json`, not the version the changelog file is named for, so once a package bumps past a version, that older version can never be republished from `main`. `scripts/publish-npm.js` compares the two and skips with a message naming both, rather than publishing the tree's version under the older version's name. That is exactly what it used to do: republishing `changelog/core/0.7.52.md` shipped 0.7.53 under the log line `published @webjsdev/core@0.7.52`, and the next file in the batch then failed with `E403 cannot publish over the previously published versions`, taking every remaining package with it. Getting a skipped version onto the registry after the fact is genuinely awkward, so weigh it against just releasing the next patch. Trusted publishing is bound to `release.yml` runs on this repo, so a local checkout of the old commit has no credential path: OIDC does not exist outside the workflow, and that commit's workflow file expects a `NPM_TOKEN` that no longer exists. It would take a granular token minted for the occasion, or a staged publish. A superseded version is usually not worth either. + This replaced a `NPM_TOKEN` repo secret that expired at npm's 90-day cap on granular write tokens and silently failed two consecutive releases (#1456). Tokens were a dead end regardless: npm removes direct publishing for bypass-2FA tokens around January 2027, leaving only OIDC or a staged publish that a human approves with 2FA. **When `server` or the scaffold consumes a NEW `@webjsdev/core` export, core MUST publish first.** `packages/server/src/dev/handler.js` and `context.js` import core symbols statically (`setAssetUrlProvider`, `setCspNonceProvider`), and `webjs create` emits an app that imports them too. A server published against an older core dies at module load with `does not provide an export named ...`, and a cli published first makes every freshly scaffolded app 500 on every route. Two things force the right order: diff --git a/scripts/publish-npm.js b/scripts/publish-npm.js index 625b7e03b..4fe5ef0cf 100644 --- a/scripts/publish-npm.js +++ b/scripts/publish-npm.js @@ -9,6 +9,14 @@ * check whether that version is already on the npm registry, skip * if yes, publish if no. * + * It ALSO skips when the workspace's package.json has moved past the + * version this file names, because `npm publish` ships the tree's + * version rather than the changelog's. That only matters on the + * `republish_paths` recovery path in .github/workflows/release.yml, + * where an older changelog file can be named deliberately; on the + * normal release path the bump and its changelog land in one commit, + * so the two are always equal. + * * Auth: relies on the standard `npm publish` token resolution * (NODE_AUTH_TOKEN env var via setup-node's .npmrc on CI, or * `npm login` locally). The script does not write any .npmrc. @@ -66,6 +74,56 @@ if (!pkgName || !version) { process.exit(2); } +// The version `npm publish` actually ships is the one in the WORKSPACE's +// package.json, never the one this changelog file is named for. Those agree +// on the normal release path, where the bump and its generated changelog land +// in the same commit, so nothing here ever noticed they were two different +// numbers. They diverge the moment an OLDER changelog file is republished, +// and the old code published the tree's version while logging the changelog's: +// republishing changelog/core/0.7.52.md shipped 0.7.53 under the line +// `published @webjsdev/core@0.7.52`, and the next file in the batch then died +// with `E403 cannot publish over the previously published versions: 0.7.53`, +// taking every remaining package with it under `set -e`. +// +// So refuse to publish when the tree has moved on. Resolve the version through +// the SAME `--workspace=` lookup `npm publish` uses, rather than guessing the +// directory from the package name, so this compares exactly what would be sent +// (`packages/` for most, `packages/editors/intellisense` and +// `packages/wrappers/*` for the rest). +// +// This runs BEFORE the registry check, so it needs no network: a version whose +// source is gone cannot be published whatever the registry says. +const treeView = spawnSync( + 'npm', ['pkg', 'get', 'version', `--workspace=${pkgName}`], + { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }, +); +let treeVersion = null; +if (treeView.status === 0) { + try { + const parsed = JSON.parse(treeView.stdout); + // `npm pkg get --workspace=` answers `{"": ""}`; a bare + // string is accepted too in case that shape ever changes. + const v = typeof parsed === 'string' ? parsed : parsed[pkgName]; + // Anything else parseable means the output shape drifted. Treat it like + // unparseable output (fail open, let the publish decide) rather than + // letting a non-string value trip the mismatch branch below, which would + // silently skip every publish while the workflow stays green. + treeVersion = typeof v === 'string' ? v : null; + } catch { + // Unparseable output is not proof of a mismatch, so fall through and let + // the publish itself decide. Failing open here keeps a workspace-resolution + // quirk from blocking a release that would otherwise be fine. + } +} +if (treeVersion && treeVersion !== version) { + console.log( + `[publish-npm] skip ${pkgName}@${version}: the workspace holds ${treeVersion}, ` + + `so ${version} can no longer be published from this tree ` + + `(publishing would ship ${treeVersion} under the wrong name)`, + ); + process.exit(0); +} + // Idempotency: is this version already on the registry? // `npm view @ version` prints the version on success, // non-zero exit on 404. We swallow stderr to avoid noisy "E404" log. @@ -89,4 +147,8 @@ if (pub.status !== 0) { console.error(`[publish-npm] npm publish failed for ${pkgName}@${version}`); process.exit(pub.status || 1); } -console.log(`[publish-npm] published ${pkgName}@${version} (${basename(file)})`); +// Report the version that was actually SHIPPED, not the one this file is named +// for. The guard above makes them equal, so this is belt and braces: the old +// line took its number from the changelog filename unconditionally, which is +// what let a publish of the wrong version read as a success in the log. +console.log(`[publish-npm] published ${pkgName}@${treeVersion ?? version} (${basename(file)})`); diff --git a/test/packaging/publish-npm-version-match.test.mjs b/test/packaging/publish-npm-version-match.test.mjs new file mode 100644 index 000000000..fa1e8600d --- /dev/null +++ b/test/packaging/publish-npm-version-match.test.mjs @@ -0,0 +1,102 @@ +/** + * `npm publish --workspace=` ships the version in the WORKSPACE's + * package.json, not the version the changelog file being processed is named + * for. scripts/publish-npm.js took its idempotency check from the changelog + * and its publish from the tree, so the two disagreed the moment an older + * changelog file was republished through the `republish_paths` dispatch input + * in .github/workflows/release.yml. + * + * That is not hypothetical. Republishing changelog/core/0.7.52.md shipped + * 0.7.53 (the tree's version) while logging `published @webjsdev/core@0.7.52`, + * and the next file in the batch then failed with + * `E403 cannot publish over the previously published versions: 0.7.53`, + * killing every remaining package under `set -e`. + * + * This locks the guard: + * 1. A changelog version the workspace has moved past is SKIPPED (exit 0) + * with a message naming both versions. + * 2. Counterfactual: a changelog version that MATCHES the workspace does + * NOT take that skip path, proving the guard is keyed on the mismatch + * rather than skipping everything. + * + * Both cases are offline. The guard deliberately runs before the `npm view` + * registry call, since a version whose source is gone cannot be published + * whatever the registry says, and that ordering is what keeps this test from + * needing the network. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { readFileSync, mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { fileURLToPath } from 'node:url'; +import { dirname, join, resolve } from 'node:path'; + +const ROOT = resolve(dirname(fileURLToPath(import.meta.url)), '../..'); +const SCRIPT = join(ROOT, 'scripts/publish-npm.js'); + +/** The version @webjsdev/core actually carries in this tree. */ +function treeVersion() { + return JSON.parse(readFileSync(join(ROOT, 'packages/core/package.json'), 'utf8')).version; +} + +/** Write a changelog file naming @webjsdev/core at `version`, run the script. */ +function runFor(version) { + const dir = mkdtempSync(join(tmpdir(), 'publish-npm-')); + const file = join(dir, `${version}.md`); + writeFileSync( + file, + `---\npackage: "@webjsdev/core"\nversion: ${version}\ndate: 2026-01-01T00:00:00.000Z\ncommit_count: 1\n---\n## Fixes\n\n- something\n`, + ); + try { + return spawnSync('node', [SCRIPT, file], { + cwd: ROOT, + encoding: 'utf8', + // The deny-live-hosts preload does not reach a spawned child, and on a + // release branch (where the tree version is not on the registry yet) + // the script would fall through the view check into a real + // `npm publish` with whatever auth is ambient on the machine. An + // unroutable registry fails both npm calls fast, keeping this + // genuinely offline. `npm pkg get` is a purely local read, so the + // guard under test is unaffected. + // + // fetch-retries MUST be 0: npm's default of 2 retries with exponential + // backoff turns an instant connection-refused into a ~72 second test. + env: { + ...process.env, + npm_config_registry: 'http://127.0.0.1:1', + npm_config_fetch_retries: '0', + npm_config_fetch_timeout: '2000', + }, + }); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test('a changelog version the workspace has moved past is skipped, not published', () => { + // 0.0.1 can never be the tree's version, so this is a guaranteed mismatch + // without depending on which release the repo currently sits on. + const r = runFor('0.0.1'); + assert.equal(r.status, 0, `expected a clean skip, got status ${r.status}\n${r.stderr}`); + assert.match(r.stdout, /skip @webjsdev\/core@0\.0\.1/); + // The message must name the version that WOULD have shipped, since that is + // the fact the old log line hid. + assert.ok( + r.stdout.includes(treeVersion()), + `the skip message should name the tree version ${treeVersion()}, got: ${r.stdout}`, + ); +}); + +test('counterfactual: a changelog version matching the workspace is not skipped by the guard', () => { + const r = runFor(treeVersion()); + // It proceeds past the guard to the registry check, which may then skip as + // already-published or fail offline. Either is fine. What must NOT appear is + // the mismatch skip, which would mean the guard fires unconditionally and + // the first assertion proves nothing. + assert.doesNotMatch( + r.stdout, + /can no longer be published from this tree/, + `the guard must not fire when the versions agree, got: ${r.stdout}`, + ); +});