Skip to content

Commit 21492b4

Browse files
committed
fix: do not read CI's version stamp as a release bump
CI stamps an experimental version into package.json before packing, so the gate sees `4.2.6-exp.<sha>` while 4.2.6 is published. Two comparisons assumed the candidate and the published release would be identical outside a real release, and both were wrong in CI. The version rule compared version strings. Since the stamped version is never equal to the published one, every pull-request build read as a release candidate, and since the numeric core matches, as a patch bump. The first pull request to legitimately change the type surface would have failed with a version error unrelated to the change. It compares the numeric core now, so a stamped build of the published version is not a release while a genuine bump, including its prereleases, still is. The size acknowledgment compared recorded byte counts exactly. Those numbers come from `gate:accept` run locally and are checked against a CI build, which the stamp alone makes differ (measured ~+0.5% on the tarball), so no acknowledgment would ever have applied in CI and an intended size change could not be landed at all. Recorded sizes are now matched within the existing tolerance, with the entry set still required to agree so an acknowledgment cannot silently cover a new entry point. Neither surfaced on this branch because the type surface is currently identical to 4.2.6, so the checks returned before reaching either comparison. Found by reading the CI log rather than from a failure. Tests go 64 to 69, covering the stamped-version case, a prerelease of a genuine bump in both directions, and acknowledgment matching at CI-sized drift, beyond tolerance, and across a changed entry set.
1 parent b9b12a8 commit 21492b4

3 files changed

Lines changed: 131 additions & 9 deletions

File tree

‎release-gate/README.md‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,20 @@ was accepted, so editing the types afterwards invalidates it rather than riding
131131
along. And it is scoped to the published version it was taken against, so it
132132
expires on the next release instead of silently carrying forward.
133133

134-
At release time the gate also checks the bump itself: once `package.json` moves
135-
off the published version, a changed type surface may not ship as a patch. During
136-
normal development the two match (the bump is its own commit, e.g. `7f93210`
137-
"4.2.6"), so there is nothing to assert and the rule stays quiet.
134+
At release time the gate also checks the bump itself: once `package.json` carries
135+
a different `major.minor.patch` from the published release, a changed type
136+
surface may not ship as a patch. During normal development the two match (the
137+
bump is its own commit, e.g. `7f93210` "4.2.6"), so there is nothing to assert
138+
and the rule stays quiet.
139+
140+
The comparison is on the numeric core, not the version string, because CI stamps
141+
an experimental version into `package.json` before packing
142+
(`4.2.6-exp.<sha>` while 4.2.6 is published). A string comparison read every
143+
pull-request build as a release candidate and then, since the core matched, as a
144+
patch bump, which would have failed the first pull request to legitimately change
145+
the type surface. For the same reason the recorded sizes are matched within the
146+
tolerance rather than byte-exactly: `gate:accept` runs locally, its numbers are
147+
compared against a CI build, and the stamp alone makes those differ.
138148

139149
Semantic additive/non-additive classification via api-extractor would remove the
140150
judgement call, but it is a much larger project and deliberately not attempted

‎scripts/release-gate.mjs‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,27 @@ export function parseVersion(version) {
268268
return match ? [Number(match[1]), Number(match[2]), Number(match[3])] : null;
269269
}
270270

271+
/**
272+
* Whether the candidate is a release being cut, rather than an ordinary build.
273+
*
274+
* Not a string comparison against the published version. CI stamps an
275+
* experimental version into package.json before packing (`4.2.6-exp.a0f4f4c`
276+
* for published 4.2.6), so `!==` treats every pull-request build as a release
277+
* candidate, and since the numeric core matches it then reads as a patch bump.
278+
* That would have failed the first pull request to legitimately change the type
279+
* surface, with a version error that had nothing to do with the change.
280+
*
281+
* Comparing the numeric core instead means a stamped build of the published
282+
* version is correctly seen as "not a release", while a genuine bump (4.2.7,
283+
* 4.3.0, and their prereleases) still is.
284+
*/
285+
export function isReleaseCandidate(candidate, published) {
286+
const a = parseVersion(candidate);
287+
const b = parseVersion(published);
288+
if (!a || !b) return false;
289+
return a[0] !== b[0] || a[1] !== b[1] || a[2] !== b[2];
290+
}
291+
271292
/**
272293
* Whether `candidate` is more than a patch bump over `published`.
273294
*
@@ -613,7 +634,7 @@ export function checkTypes(pkgDir, pkg, published, report, { acceptedFile = ACCE
613634
// so there is nothing to assert. Once it moves, this is a release candidate
614635
// and a changed type surface may not ship as a patch, which is exactly how
615636
// 4.2.4 broke consumer builds.
616-
if (pkg.version === published.version) return;
637+
if (!isReleaseCandidate(pkg.version, published.version)) return;
617638
const bumped = isMinorOrMajorBump(pkg.version, published.version);
618639
if (bumped === false) {
619640
report.fail(
@@ -641,6 +662,25 @@ export function measurePackage(pkgDir, pkg, tarball) {
641662
return { packed: fs.statSync(tarball).size, sizes };
642663
}
643664

665+
/**
666+
* Whether a recorded set of measurements still describes the current build,
667+
* within `SIZE_TOLERANCE`.
668+
*
669+
* Every entry must be present on both sides: an acknowledgment that predates a
670+
* new entry point should not silently cover it.
671+
*/
672+
export function sizesMatch(accepted, current) {
673+
if (!accepted || typeof accepted.packed !== 'number') return false;
674+
const within = (a, b) => b > 0 && Math.abs((a - b) / b) <= SIZE_TOLERANCE;
675+
if (!within(current.packed, accepted.packed)) return false;
676+
677+
const acceptedKeys = Object.keys(accepted.sizes ?? {}).sort();
678+
const currentKeys = Object.keys(current.sizes ?? {}).sort();
679+
if (acceptedKeys.join() !== currentKeys.join()) return false;
680+
681+
return currentKeys.every((key) => within(current.sizes[key].gzip, accepted.sizes[key].gzip));
682+
}
683+
644684
export function checkSize(pkgDir, pkg, tarball, published, report, { acceptedFile = ACCEPTED_FILE, publishedMetrics } = {}) {
645685
const current = measurePackage(pkgDir, pkg, tarball);
646686

@@ -657,10 +697,14 @@ export function checkSize(pkgDir, pkg, tarball, published, report, { acceptedFil
657697
// Same acknowledgment as the type surface: an intended size move is recorded
658698
// once, against a named published version, and stops applying when that
659699
// version moves on.
660-
const acknowledged =
661-
accepted?.against === published.version &&
662-
accepted?.size?.packed === current.packed &&
663-
JSON.stringify(accepted?.size?.sizes ?? null) === JSON.stringify(current.sizes);
700+
//
701+
// Matched within the tolerance rather than byte-exactly. `gate:accept` is run
702+
// locally, but the numbers it records get compared against a CI build, and the
703+
// two are never byte-identical: CI stamps an experimental version into the
704+
// bundle before packing (4.2.6-exp.<sha>), which measured ~+0.5% on the
705+
// tarball. Exact matching would mean no acknowledgment ever applied in CI, so
706+
// an intended size change could not be landed at all.
707+
const acknowledged = accepted?.against === published.version && sizesMatch(accepted?.size, current);
664708

665709
const over = [];
666710

‎test/release-gate.test.mjs‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ import {
1212
diffLines,
1313
isAllowedExternal,
1414
isMinorOrMajorBump,
15+
isReleaseCandidate,
1516
measurePackage,
1617
parseVersion,
1718
runChecks,
19+
sizesMatch,
1820
typesDigest,
1921
writeAccepted,
2022
} from '../scripts/release-gate.mjs';
@@ -417,6 +419,46 @@ describe('runChecks', () => {
417419
expect(failures.find((f) => f.check === 'size')).toBeUndefined();
418420
});
419421

422+
// `gate:accept` runs locally, but its numbers are compared against a CI build,
423+
// and CI stamps a version into the bundle so the two are never byte-identical.
424+
// Exact matching meant no acknowledgment ever applied in CI.
425+
it('honours an acknowledgment whose sizes differ slightly, as a CI build does', () => {
426+
const built = build({ esm: ESM_CLEAN });
427+
accept(built);
428+
const recorded = JSON.parse(fs.readFileSync(built.options.acceptedFile, 'utf8'));
429+
// Stand in for the version stamp: ~+0.5% on the tarball, ~+0.2% on entries.
430+
recorded.size.packed = Math.round(recorded.size.packed / 1.005);
431+
recorded.size.sizes['dist/index.js'].gzip = Math.round(recorded.size.sizes['dist/index.js'].gzip / 1.002);
432+
fs.writeFileSync(built.options.acceptedFile, JSON.stringify(recorded));
433+
const publishedMetrics = publishedMetricsScaled(built, { entry: 1.5 });
434+
const { failures } = runChecks(built.pkgDir, built.pkg, built.tarball, built.published, { ...built.options, publishedMetrics });
435+
expect(failures.find((f) => f.check === 'size')).toBeUndefined();
436+
});
437+
438+
// Tolerant matching must not become "any size is acknowledged".
439+
it('does not honour an acknowledgment whose sizes moved beyond the tolerance', () => {
440+
const built = build({ esm: ESM_CLEAN });
441+
accept(built);
442+
const recorded = JSON.parse(fs.readFileSync(built.options.acceptedFile, 'utf8'));
443+
recorded.size.sizes['dist/index.js'].gzip = Math.round(recorded.size.sizes['dist/index.js'].gzip / 1.5);
444+
fs.writeFileSync(built.options.acceptedFile, JSON.stringify(recorded));
445+
const publishedMetrics = publishedMetricsScaled(built, { entry: 1.5 });
446+
const { failures } = runChecks(built.pkgDir, built.pkg, built.tarball, built.published, { ...built.options, publishedMetrics });
447+
expect(failures.find((f) => f.check === 'size')).toBeDefined();
448+
});
449+
450+
// An acknowledgment taken before a new entry point existed should not cover it.
451+
it('does not honour an acknowledgment that predates a new entry point', () => {
452+
const built = build({ esm: ESM_CLEAN });
453+
accept(built);
454+
const recorded = JSON.parse(fs.readFileSync(built.options.acceptedFile, 'utf8'));
455+
delete recorded.size.sizes['dist/index.umd.cjs'];
456+
fs.writeFileSync(built.options.acceptedFile, JSON.stringify(recorded));
457+
const publishedMetrics = publishedMetricsScaled(built, { entry: 1.5 });
458+
const { failures } = runChecks(built.pkgDir, built.pkg, built.tarball, built.published, { ...built.options, publishedMetrics });
459+
expect(failures.find((f) => f.check === 'size')).toBeDefined();
460+
});
461+
420462
// --- acknowledgment (#749) ---------------------------------------------
421463

422464
const withChangedTypes = (overrides = {}) =>
@@ -483,6 +525,32 @@ describe('runChecks', () => {
483525
expect(failures).toEqual([]);
484526
});
485527

528+
// CI stamps an experimental version into package.json before packing
529+
// (`4.2.6-exp.<sha>` while 4.2.6 is published), so a `!==` comparison read
530+
// every pull-request build as a release candidate and then, because the
531+
// numeric core matches, as a patch bump. That would have failed the first PR
532+
// to legitimately change types, with an error about the version.
533+
it('does not treat CI’s experimental version stamp as a release', () => {
534+
const built = withChangedTypes({ version: '4.2.6-exp.a0f4f4c', publishedVersion: '4.2.6' });
535+
accept(built);
536+
const { failures } = runChecks(built.pkgDir, built.pkg, built.tarball, built.published, built.options);
537+
expect(failures).toEqual([]);
538+
});
539+
540+
// The stamp must not become a blanket exemption: a prerelease of a real bump
541+
// is still a release, and still has to clear the rule.
542+
it('still applies the rule to a prerelease of a genuine bump', () => {
543+
const patch = withChangedTypes({ version: '4.2.7-exp.a0f4f4c', publishedVersion: '4.2.6' });
544+
accept(patch);
545+
const patchRun = runChecks(patch.pkgDir, patch.pkg, patch.tarball, patch.published, patch.options);
546+
expect(patchRun.failures.find((f) => f.check === 'types')?.message).toContain('patch bump');
547+
548+
const minor = withChangedTypes({ version: '4.3.0-exp.a0f4f4c', publishedVersion: '4.2.6' });
549+
accept(minor);
550+
const minorRun = runChecks(minor.pkgDir, minor.pkg, minor.tarball, minor.published, minor.options);
551+
expect(minorRun.failures).toEqual([]);
552+
});
553+
486554
it('allows a patch release that does not touch the type surface', () => {
487555
const built = build({ esm: ESM_CLEAN, version: '4.2.7', publishedVersion: '4.2.6' });
488556
const { failures } = runChecks(built.pkgDir, built.pkg, built.tarball, built.published, built.options);

0 commit comments

Comments
 (0)