Skip to content

test: hold pack-manifest and check-conformance in agreement #531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
allxsmith merged 2 commits into from
Aug 18, 2026
Merged

test: hold pack-manifest and check-conformance in agreement #531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c5e031
test: hold pack-manifest and check-conformance in agreement
allxsmith Aug 18, 2026
cf95eb7
test: hold the agreement for shapes nobody enumerated
allxsmith Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
234 changes: 155 additions & 79 deletions bestax-migrate/scripts/pack-manifest.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,25 @@
* risk npm not running `postpack` and leaving the repo manifest rewritten for
* @semantic-release/git to commit — the exact failure the backup below exists
* to prevent.
*
* The per-specifier decision is exported and takes its version lookup as an
* argument, and `main` only runs when this file is executed directly. That
* seam exists for one reason (#435): this script and
* scripts/check-conformance.mjs encode the same rule about which pnpm shapes
* are resolvable, and a shape this script REFUSES but the check EXCUSES is a
* green CI with a red release. That inversion happened twice during review of
* #417 — once for `catalog:`, once for the alias form — so a test now drives
* both real implementations and asserts they agree shape for shape. It can
* only do that if the decision is callable without packing anything.
*/
import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import process from 'node:process';
import { fileURLToPath, pathToFileURL } from 'node:url';

const pkgRoot = path.dirname(path.dirname(fileURLToPath(import.meta.url)));
const manifest = path.join(pkgRoot, 'package.json');
const backup = path.join(pkgRoot, 'package.json.pack-backup');
const defaultPkgRoot = path.dirname(
path.dirname(fileURLToPath(import.meta.url))
);

const DEP_SECTIONS = [
'dependencies',
Expand All @@ -45,23 +56,41 @@ const DEP_SECTIONS = [
'optionalDependencies',
];

/** Version of a workspace package, read through the linked node_modules entry. */
function resolveWorkspaceVersion(name) {
const linked = path.join(pkgRoot, 'node_modules', name, 'package.json');
if (!fs.existsSync(linked)) {
console.error(
`pack-manifest: cannot resolve the workspace dependency "${name}" — ` +
`${path.relative(pkgRoot, linked)} does not exist.\n` +
'Run `pnpm install` from the repo root before packing.'
);
process.exit(1);
}
const { version } = JSON.parse(fs.readFileSync(linked, 'utf8'));
if (!version) {
console.error(`pack-manifest: "${name}" has no version in its manifest`);
process.exit(1);
/**
* Raised for a specifier this script will not resolve. Thrown rather than
* exited so the decision can be exercised by a test; `main` turns it back into
* the same message-and-exit-1 the CLI has always produced.
*/
export class UnsupportedSpecifierError extends Error {
constructor(message) {
super(message);
this.name = 'UnsupportedSpecifierError';
}
return version;
}

/**
* Version of a workspace package, read through the linked node_modules entry.
* Curried over the package root so a test can supply its own lookup instead of
* needing a real pnpm-linked tree.
*/
export function makeWorkspaceVersionResolver(pkgRoot = defaultPkgRoot) {
return name => {
const linked = path.join(pkgRoot, 'node_modules', name, 'package.json');
if (!fs.existsSync(linked)) {
throw new UnsupportedSpecifierError(
`pack-manifest: cannot resolve the workspace dependency "${name}" — ` +
`${path.relative(pkgRoot, linked)} does not exist.\n` +
'Run `pnpm install` from the repo root before packing.'
);
}
const { version } = JSON.parse(fs.readFileSync(linked, 'utf8'));
if (!version) {
throw new UnsupportedSpecifierError(
`pack-manifest: "${name}" has no version in its manifest`
);
}
return version;
};
}

/**
Expand All @@ -71,89 +100,136 @@ function resolveWorkspaceVersion(name) {
* A bare `workspace:` is pnpm's shorthand for `workspace:*` — leaving it to the
* unwrap branch would emit an empty specifier.
*/
function resolveSpecifier(name, spec) {
export function resolveSpecifier(name, spec, resolveVersion, label = name) {
const rest = spec.slice('workspace:'.length);
if (rest === '*' || rest === '') return resolveWorkspaceVersion(name);
if (rest === '^' || rest === '~')
return `${rest}${resolveWorkspaceVersion(name)}`;
if (rest === '*' || rest === '') return resolveVersion(name);
if (rest === '^' || rest === '~') return `${rest}${resolveVersion(name)}`;
// `workspace:<name>@<range>` is pnpm's alias form, and it does NOT publish as
// a bare range: pnpm emits `npm:<name>@<version>`. Unwrapping it would write
// "@scope/pkg@^5", which no package manager can install — #412 again, wearing
// a different hat. A semver range never contains "/" or a non-leading "@",
// so this only catches the alias form.
if (rest.includes('/') || rest.lastIndexOf('@') > 0) {
console.error(
`pack-manifest: "${name}": "${spec}" is pnpm's alias form, which ` +
throw new UnsupportedSpecifierError(
`pack-manifest: ${label} is "${spec}", pnpm's alias form, which ` +
`publishes as \`npm:<name>@<version>\`.\n` +
'This script does not synthesize that. Depend on the package under its ' +
'real name, or teach pack-manifest.mjs the npm: alias rewrite.'
);
process.exit(1);
}
return rest;
}

const mode = process.argv[2];

if (mode === 'prepack') {
if (fs.existsSync(backup)) {
console.error(
'pack-manifest: package.json.pack-backup already exists — a previous ' +
'pack did not finish.\n' +
'Restore the workspace manifest first: mv package.json.pack-backup package.json'
/**
* The whole per-specifier decision, in one place: what this script does with
* any one dependency entry.
*
* Returns the rewritten specifier, or `null` when the entry is none of this
* script's business and should be left exactly as written. Throws
* UnsupportedSpecifierError for the shapes it refuses.
*
* This is the function scripts/pack-manifest.test.mjs drives, and it is the
* single source of "refused or resolved" that check-conformance.mjs's
* UNRESOLVABLE_AT_PACK has to stay in step with (#435).
*/
export function rewriteSpecifier(name, spec, resolveVersion, label = name) {
if (typeof spec !== 'string') return null;
// `catalog:` is the other protocol `pnpm publish` resolves and `npm publish`
// ships verbatim — the exact shape of #412. This script cannot resolve it
// (the range lives in pnpm-workspace.yaml, not in the linked package), so
// fail the release rather than pack a broken tarball.
if (spec.startsWith('catalog:')) {
throw new UnsupportedSpecifierError(
`pack-manifest: ${label} is "${spec}", and this script cannot resolve ` +
`the catalog: protocol.\n` +
'Give it a plain semver range, or teach pack-manifest.mjs to read ' +
'`catalog`/`catalogs` from pnpm-workspace.yaml.'
);
process.exit(1);
}
if (!spec.startsWith('workspace:')) return null;
return resolveSpecifier(name, spec, resolveVersion, label);
}

/**
* The CLI. Wrapped in a function and guarded below so importing this module
* for the agreement test does not pack anything, and so the refusals above can
* throw (testable) while the command still exits 1 with the same message.
*/
export function main(
argv = process.argv.slice(2),
{ pkgRoot = defaultPkgRoot } = {}
) {
const manifest = path.join(pkgRoot, 'package.json');
const backup = path.join(pkgRoot, 'package.json.pack-backup');
const resolveVersion = makeWorkspaceVersionResolver(pkgRoot);
const mode = argv[0];

if (mode === 'prepack') {
if (fs.existsSync(backup)) {
console.error(
'pack-manifest: package.json.pack-backup already exists — a previous ' +
'pack did not finish.\n' +
'Restore the workspace manifest first: mv package.json.pack-backup package.json'
);
return 1;
}

const pkg = JSON.parse(fs.readFileSync(manifest, 'utf8'));
const rewritten = [];
const pkg = JSON.parse(fs.readFileSync(manifest, 'utf8'));
const rewritten = [];

for (const section of DEP_SECTIONS) {
for (const [name, spec] of Object.entries(pkg[section] ?? {})) {
if (typeof spec !== 'string') continue;
// `catalog:` is the other protocol `pnpm publish` resolves and
// `npm publish` ships verbatim — the exact shape of #412. This script
// cannot resolve it (the range lives in pnpm-workspace.yaml, not in the
// linked package), so fail the release rather than pack a broken tarball.
if (spec.startsWith('catalog:')) {
console.error(
`pack-manifest: ${section}.${name} is "${spec}", and this script ` +
`cannot resolve the catalog: protocol.\n` +
'Give it a plain semver range, or teach pack-manifest.mjs to read ' +
'`catalog`/`catalogs` from pnpm-workspace.yaml.'
);
process.exit(1);
for (const section of DEP_SECTIONS) {
for (const [name, spec] of Object.entries(pkg[section] ?? {})) {
let resolved;
try {
resolved = rewriteSpecifier(
name,
spec,
resolveVersion,
`${section}.${name}`
);
} catch (err) {
if (!(err instanceof UnsupportedSpecifierError)) throw err;
console.error(err.message);
return 1;
}
if (resolved === null) continue;
pkg[section][name] = resolved;
rewritten.push(`${section}.${name}: ${spec} -> ${resolved}`);
}
if (!spec.startsWith('workspace:')) continue;
const resolved = resolveSpecifier(name, spec);
pkg[section][name] = resolved;
rewritten.push(`${section}.${name}: ${spec} -> ${resolved}`);
}
}

if (!rewritten.length) {
console.log('pack-manifest: no workspace: specifiers to resolve');
process.exit(0);
if (!rewritten.length) {
console.log('pack-manifest: no workspace: specifiers to resolve');
return 0;
}

fs.copyFileSync(manifest, backup);
// Keep npm's own formatting (2-space + trailing newline) so the restored
// file and the packed one differ only in the specifiers.
fs.writeFileSync(manifest, `${JSON.stringify(pkg, null, 2)}\n`);
console.log(
`pack-manifest: resolved ${rewritten.length} workspace specifier(s)`
);
for (const line of rewritten) console.log(` ${line}`);
return 0;
}

fs.copyFileSync(manifest, backup);
// Keep npm's own formatting (2-space + trailing newline) so the restored
// file and the packed one differ only in the specifiers.
fs.writeFileSync(manifest, `${JSON.stringify(pkg, null, 2)}\n`);
console.log(
`pack-manifest: resolved ${rewritten.length} workspace specifier(s)`
);
for (const line of rewritten) console.log(` ${line}`);
} else if (mode === 'postpack') {
if (!fs.existsSync(backup)) {
// prepack exits early when there is nothing to rewrite; that is not an error.
console.log('pack-manifest: no backup to restore');
process.exit(0);
if (mode === 'postpack') {
if (!fs.existsSync(backup)) {
// prepack exits early when there is nothing to rewrite; not an error.
console.log('pack-manifest: no backup to restore');
return 0;
}
fs.copyFileSync(backup, manifest);
fs.rmSync(backup);
console.log('pack-manifest: workspace manifest restored');
return 0;
}
fs.copyFileSync(backup, manifest);
fs.rmSync(backup);
console.log('pack-manifest: workspace manifest restored');
} else {

console.error('Usage: node scripts/pack-manifest.mjs <prepack|postpack>');
process.exit(1);
return 1;
}

if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) {
process.exitCode = main();
}
28 changes: 20 additions & 8 deletions scripts/check-conformance.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
*/
import { readFile, readdir, writeFile, access } from 'node:fs/promises';
import { join, relative, dirname } from 'node:path';
import { fileURLToPath } from 'node:url';
import { fileURLToPath, pathToFileURL } from 'node:url';

// `registerVarsKeys` lives in lib/ so the API-docs generator can share the same
// parser — it additionally exposes values and selector nesting, which the CSS
Expand Down Expand Up @@ -1025,8 +1025,15 @@ const hasPackTimeProtocol = spec =>

// The two shapes the pack hooks refuse rather than guess at, so devDependencies
// carrying them are violations no matter how the hooks are wired. Kept in step
// with bestax-migrate/scripts/pack-manifest.mjs, which exits 1 on both.
const UNRESOLVABLE_AT_PACK = [
// with bestax-migrate/scripts/pack-manifest.mjs, which refuses both.
//
// "Kept in step" used to mean "by reading both files carefully", and that
// failed twice during review of #417 — once for `catalog:`, once for the alias
// form. A shape the script REFUSES but this check EXCUSES is a green CI with a
// red release, which is the exact inversion this check exists to prevent. So
// these are exported and scripts/pack-manifest.test.mjs now drives them against
// the real resolver, asserting the two agree shape for shape (#435).
export const UNRESOLVABLE_AT_PACK = [
{
matches: spec => spec.startsWith('catalog:'),
why:
Expand All @@ -1047,7 +1054,7 @@ const UNRESOLVABLE_AT_PACK = [
},
];

const unresolvableAtPack = spec =>
export const unresolvableAtPack = spec =>
typeof spec === 'string' &&
UNRESOLVABLE_AT_PACK.find(rule => rule.matches(spec));

Expand Down Expand Up @@ -1307,7 +1314,12 @@ async function main() {
}
}

main().catch(err => {
console.error(err);
process.exit(1);
});
// Only run the suite when invoked as a command. The rules above are imported
// by scripts/pack-manifest.test.mjs, and importing a module should not run a
// repo-wide conformance sweep as a side effect.
if (import.meta.url === pathToFileURL(process.argv[1] ?? '').href) {
main().catch(err => {
console.error(err);
process.exit(1);
});
}
Loading
Loading