Skip to content

Commit eea6dcd

Browse files
committed
fix(tooling): resolve the changeset LEVEL axis package by manifest, at any depth
wip Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU
1 parent 07c56b5 commit eea6dcd

1 file changed

Lines changed: 144 additions & 35 deletions

File tree

scripts/check-changeset-no-major.mjs

Lines changed: 144 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ export function render(result) {
765765
// the fixed `Clause-②: yes` line the PM protocol spells (read here through
766766
// `check-clause2-carriers.mjs`'s own `readClause2Line`, imported rather
767767
// than restated, so the two readers cannot drift);
768-
// ② the CHANGESET LEVEL for the package whose `packages/*/src/**` the diff
768+
// ② the CHANGESET LEVEL for the package whose `packages/**/src/**` the diff
769769
// moves.
770770
//
771771
// A declaration of ① plus a `patch` in ② is a self-contradiction inside one
@@ -812,7 +812,7 @@ export function render(result) {
812812
// asked at the declaration's own grain:
813813
//
814814
// A PR that declares clause-② `yes` must grade AT LEAST ONE package whose
815-
// `packages/*/src/**` it moves at `minor` or above.
815+
// `packages/**/src/**` it moves at `minor` or above.
816816
//
817817
// The widened package IS one of the packages the diff moved src of — a widening
818818
// moves source — so "the widened package is graded `minor`+" IMPLIES "some
@@ -889,19 +889,117 @@ export function render(result) {
889889
// a two-condition case with no motive, and the alternative — a second step
890890
// — is refused by `check-empty-changeset.mjs`'s pin on this job's failable
891891
// step count.
892+
//
893+
// ## THE DEPTH: which packages this axis can see at all (#16713)
894+
//
895+
// ⭐ Everything above describes what the axis DOES with a package it can see.
896+
// Until #16713 it could see 23 of this workspace's 74 packages, because the
897+
// package half of the path reading was one path segment wide and 51 packages
898+
// sit at `packages/<group>/<name>/`. That is not a weaker verdict on the other
899+
// 51 — it is NO verdict, rendered identically to a pass, and it is the failure
900+
// this whole file is otherwise built to refuse: the green printed for them said
901+
// "the axis looked and approved" while meaning "the axis did not look".
902+
//
903+
// It also escaped the one instrument that should have caught it. `unreadable`
904+
// exists so a package this reading cannot NAME is never mistaken for a package
905+
// the diff did not TOUCH (#4690) — and it stayed empty here, because a nested
906+
// package was not an unreadable reading, it was never a candidate. ⇒ A gate
907+
// cannot report a limb it never grew, and the residual an instrument names is
908+
// only ever a residual of what it looks at.
909+
//
910+
// The reach is now the manifest set rather than a depth, and the widening was
911+
// measured before it was chosen rather than after: over the 150 most recently
912+
// merged PRs, driving this file's own `judgeLevel` at each merge commit against
913+
// its parent, SEVEN verdicts move from exit 0 to exit 1 — six of them
914+
// `not-measured-moot` -> `not-measured-material`, which one `Clause-②:` line in
915+
// the PR body clears with no push, and one — PR #16650, `@objectstack/driver-sql`
916+
// and `@objectstack/driver-turso` graded `patch` under a durable `Clause-②: yes`
917+
// body line — `clean` -> `enforce`, which is this gate's own rule finding, on a
918+
// merged PR, the thing it exists to find. ⛔ That count is a reading for the
919+
// maintainer, never an argument for a tolerance: there is no allowlist and no
920+
// grandfathering here, and the six are cleared by declaring, not by softening.
892921

893-
/** The paths whose movement makes a package's PUBLISHED surface the thing that grew. */
894-
const PUBLISHED_SOURCE_PATH = /^packages\/([^/]+)\/src\//;
922+
/**
923+
* The `src` root whose movement makes a package's PUBLISHED surface the thing
924+
* that grew. WHICH roots ship is a different axis and is deliberately left
925+
* alone here — `bin/**` ships too, and that is #16692's card, not this one.
926+
* What this half answers is only HOW DEEP the package owning the root may sit.
927+
*/
928+
const PUBLISHED_SOURCE_ROOT = 'src';
929+
930+
/**
931+
* Every directory this path could be the published source OF: each ancestor `D`
932+
* under `packages/` for which the path reads `D/src/**`, SHALLOWEST FIRST.
933+
*
934+
* ## Why this is shape only, and not a deeper pattern (#16713)
935+
*
936+
* The reading used to be a single regular expression whose package segment was
937+
* one-path-segment-wide, so it saw `packages/<name>/src/**` and nothing else.
938+
* This workspace is not flat: 51 of its 74 packages live at
939+
* `packages/<group>/<name>/` — every driver, service, plugin, connector,
940+
* trigger, adapter and app — and for all of them the segment after `packages/`
941+
* is the GROUP, which no `/src/` follows. So 69% of the workspace was not a
942+
* candidate this gate could refuse, and — worse than unrefusable — not even
943+
* REPORTABLE: `unreadable` stayed empty too, because a nested package never
944+
* entered the reading at all. #4690's distinction ("a name it could not read
945+
* must not look like a package the diff did not touch") was kept for an
946+
* unreadable manifest and could not be kept here, because the gate cannot
947+
* report a limb it never grew.
948+
*
949+
* ⛔ The repair is NOT a second segment in the pattern. That re-encodes today's
950+
* layout in a second place and goes blind again the day a package sits one
951+
* level deeper — the same defect by the same means, its recurrence merely
952+
* postponed. What is enumerated here is SHAPE ONLY, at any depth; WHICH of the
953+
* candidates is a real package is decided by reading its manifest out of the
954+
* tree in `packagesTouched`, so the layout is read rather than written down.
955+
*
956+
* ## Why SHALLOWEST first — measured, not assumed
957+
*
958+
* A path can have more than one candidate, and this repo contains the case:
959+
* `packages/create-objectstack/src/templates/blank/src/objects/note.object.ts`
960+
* is `D/src/**` for BOTH `packages/create-objectstack` and the scaffold
961+
* template dir `packages/create-objectstack/src/templates/blank`, which carries
962+
* its own manifest (`objectstack-blank`, `private: true`) — template CONTENT
963+
* that create-objectstack ships, not a workspace member. Resolving to the
964+
* NEAREST manifest would name that private template and drop the real package:
965+
* a regression against the one-segment reading this replaces. Shallowest first
966+
* returns the old answer on every path the old pattern matched and adds the
967+
* nested ones — measured over the whole tree, 22 package dirs matched before,
968+
* 72 after, and none lost.
969+
*
970+
* @param {string} path a repo-relative path, as `git diff --name-only` prints it
971+
* @returns {string[]} candidate package directories, shallowest first
972+
*/
973+
export function publishedSourceOwners(path) {
974+
const segments = path.split('/');
975+
if (segments[0] !== 'packages') return [];
976+
const owners = [];
977+
// `i` indexes the `src` segment. It starts at 2 so the owner is at least
978+
// `packages/<something>` — `packages/src/**` names no package — and stops one
979+
// short of the end, so the path is INSIDE `src/` rather than a file called
980+
// `src`.
981+
for (let i = 2; i < segments.length - 1; i += 1) {
982+
if (segments[i] === PUBLISHED_SOURCE_ROOT) owners.push(segments.slice(0, i).join('/'));
983+
}
984+
return owners;
985+
}
895986

896987
/**
897988
* The workspace package names whose `src/**` this diff moves, read from the
898989
* HEAD tree rather than from the working directory — the self-test and the
899990
* acceptance run both drive commits that are not checked out.
900991
*
901-
* `unreadable` is returned beside them, never folded into them: a
902-
* `packages/<dir>/src/**` path whose manifest could not be read is a package
903-
* this reading could not name, and a name it could not read must not look like
904-
* a package the diff did not touch (#4690).
992+
* `unreadable` is returned beside them, never folded into them: a `<dir>/src/**`
993+
* path whose manifest could not be read is a package this reading could not
994+
* name, and a name it could not read must not look like a package the diff did
995+
* not touch (#4690).
996+
*
997+
* ⭐ That distinction is the bill the widening above has to keep paying, and it
998+
* is exactly where the NEW failure mode lands. A nested path now MATCHES, so a
999+
* nested directory whose manifest is missing or unparseable is REPORTED as
1000+
* unreadable instead of vanishing the way every nested path used to. The
1001+
* invariant, stated so it can be tested: a path that matches the shape lands in
1002+
* `packages` or in `unreadable` — never in neither.
9051003
*
9061004
* @param {{ cwd: string, from: string, head: string }} opts
9071005
* @returns {{ packages: string[], unreadable: string[] }}
@@ -913,27 +1011,38 @@ export function packagesTouched({ cwd, from, head }) {
9131011
} catch {
9141012
return { packages: [], unreadable: [] };
9151013
}
916-
const dirs = new Set();
917-
for (const line of out.split('\n')) {
918-
const m = PUBLISHED_SOURCE_PATH.exec(line.trim());
919-
if (m) dirs.add(m[1]);
920-
}
921-
const packages = [];
922-
const unreadable = [];
923-
for (const dir of [...dirs].sort()) {
924-
const manifest = showOrNull(head, `packages/${dir}/package.json`, cwd);
925-
let name = null;
926-
if (manifest !== null) {
927-
try {
928-
name = JSON.parse(manifest).name ?? null;
929-
} catch {
930-
name = null;
1014+
// One manifest read per candidate DIRECTORY rather than per changed file: a
1015+
// diff that moves forty files in one package would otherwise ask forty times.
1016+
const nameOfDir = new Map();
1017+
const nameFor = (dir) => {
1018+
if (!nameOfDir.has(dir)) {
1019+
const manifest = showOrNull(head, `${dir}/package.json`, cwd);
1020+
let name = null;
1021+
if (manifest !== null) {
1022+
try {
1023+
name = JSON.parse(manifest).name ?? null;
1024+
} catch {
1025+
name = null;
1026+
}
9311027
}
1028+
nameOfDir.set(dir, typeof name === 'string' && name ? name : null);
9321029
}
933-
if (typeof name === 'string' && name) packages.push(name);
934-
else unreadable.push(`packages/${dir}`);
1030+
return nameOfDir.get(dir);
1031+
};
1032+
1033+
const packages = new Set();
1034+
const unreadable = new Set();
1035+
for (const line of out.split('\n')) {
1036+
const owners = publishedSourceOwners(line.trim());
1037+
if (owners.length === 0) continue;
1038+
const owner = owners.find((dir) => nameFor(dir) !== null);
1039+
if (owner) packages.add(nameFor(owner));
1040+
// Nothing nameable on the chain: the SHALLOWEST candidate is what gets
1041+
// reported, because it is the directory the old one-segment reading named,
1042+
// so an unreadable manifest keeps reporting the dir it always reported.
1043+
else unreadable.add(owners[0]);
9351044
}
936-
return { packages, unreadable };
1045+
return { packages: [...packages].sort(), unreadable: [...unreadable].sort() };
9371046
}
9381047

9391048
/**
@@ -1180,7 +1289,7 @@ export function renderLevel(result) {
11801289
stdout.push(
11811290
'ℹ️ LEVEL AXIS: NOT MEASURED, and it could not have changed this verdict — no clause-② declaration was ' +
11821291
'readable for this PR, AND there is nothing here a `yes` would have refused: either no changeset grades ' +
1183-
'`patch` a package whose `packages/*/src/**` this PR moves, or one of the packages it moves is already ' +
1292+
'`patch` a package whose `packages/**/src/**` this PR moves, or one of the packages it moves is already ' +
11841293
'graded `minor` or above and carries the level for the PR (#16361). ' +
11851294
'`yes` and `no` reach the same answer on this diff, so this exit 0 is a decided one rather than an unread one (#16776).',
11861295
...readings,
@@ -1190,7 +1299,7 @@ export function renderLevel(result) {
11901299

11911300
case 'not-measured-material':
11921301
stderr.push('⛔ LEVEL AXIS: NOT MEASURED, and it is the one reading this PR needed.\n');
1193-
stderr.push(' The packages this PR moves `packages/*/src/**` of, and the level each is graded:');
1302+
stderr.push(' The packages this PR moves `packages/**/src/**` of, and the level each is graded:');
11941303
stderr.push(...patchLines(result.offenders));
11951304
stderr.push(' ⇒ none of them is graded `minor` or above, so a `yes` here would REFUSE (#16361).\n');
11961305
stderr.push(
@@ -1202,7 +1311,7 @@ export function renderLevel(result) {
12021311
'conclusion for a reading that passed and a reading that never happened, on every surface that reads\n' +
12031312
'conclusions rather than step logs (#16776, and #4690: a reading that cannot fail is indistinguishable from\n' +
12041313
'one that passed). Where the declaration could not have mattered this gate still exits 0 and says so — it is\n' +
1205-
'refusing HERE because every package this diff moves under `packages/*/src/**` is graded `patch`, which is\n' +
1314+
'refusing HERE because every package this diff moves under `packages/**/src/**` is graded `patch`, which is\n' +
12061315
'exactly the shape a `yes` refuses (#16361).\n' +
12071316
'\n' +
12081317
'DECLARE IT. One line, at the START of a line in the PR BODY (a `- `, `> ` or `**` prefix is read too):\n' +
@@ -1229,7 +1338,7 @@ export function renderLevel(result) {
12291338

12301339
case 'clean':
12311340
stdout.push(
1232-
'✓ LEVEL AXIS: this PR declares clause-② `yes`, and no package whose `packages/*/src/**` it moves is graded `patch`.',
1341+
'✓ LEVEL AXIS: this PR declares clause-② `yes`, and no package whose `packages/**/src/**` it moves is graded `patch`.',
12331342
...readings,
12341343
...unreadableNote,
12351344
);
@@ -1242,7 +1351,7 @@ export function renderLevel(result) {
12421351
// mode the filing card is about, one layer along.
12431352
case 'discharged':
12441353
stdout.push(
1245-
'✓ LEVEL AXIS: this PR declares clause-② `yes`, and it grades a package whose `packages/*/src/**` ' +
1354+
'✓ LEVEL AXIS: this PR declares clause-② `yes`, and it grades a package whose `packages/**/src/**` ' +
12461355
'it moves at `minor` or above — the declared widening is accounted for:',
12471356
...raisedLines(result.raised),
12481357
'',
@@ -1261,10 +1370,10 @@ export function renderLevel(result) {
12611370

12621371
case 'enforce':
12631372
stderr.push(
1264-
'⛔ This PR declares clause-② YES, and it grades NO package whose `packages/*/src/**` it moves\n' +
1373+
'⛔ This PR declares clause-② YES, and it grades NO package whose `packages/**/src/**` it moves\n' +
12651374
' at `minor` or above.\n',
12661375
);
1267-
stderr.push(' The packages this PR moves `packages/*/src/**` of, and the level each is graded:');
1376+
stderr.push(' The packages this PR moves `packages/**/src/**` of, and the level each is graded:');
12681377
stderr.push(...patchLines(result.offenders));
12691378
stderr.push(' ⇒ none of them is graded `minor` or above.\n');
12701379
stderr.push(
@@ -2323,7 +2432,7 @@ function selfTest() {
23232432
// The package axis, same patch head: `patch` for a package this diff did
23242433
// not grow is not this gate's business.
23252434
const untouched = judgeLevel({ levels: levelsFor(PATCH_HEAD), touched: { packages: ['@objectstack/spec'], unreadable: [] }, declaration: declaredYes });
2326-
assert(untouched.verdict === 'clean', `\`patch\` for a package the diff does not move under packages/*/src/** is not refused — got ${untouched.verdict}`);
2435+
assert(untouched.verdict === 'clean', `\`patch\` for a package the diff does not move under packages/**/src/** is not refused — got ${untouched.verdict}`);
23272436

23282437
// #4690, on this axis too.
23292438
assert(judgeLevel({ levels: null, touched: touchedCli, declaration: declaredYes }).verdict === 'unreadable-diff', 'an uncomputable diff is a failure on the level axis as well');
@@ -2402,7 +2511,7 @@ function selfTest() {
24022511
);
24032512
const scanned = scan({ cwd: dir, base });
24042513
const touched = packagesTouched({ cwd: dir, from: scanned.base, head: 'HEAD' });
2405-
assert(touched.packages.length === 0, `a diff outside packages/*/src/** grows no published surface — got ${JSON.stringify(touched.packages)}`);
2514+
assert(touched.packages.length === 0, `a diff outside packages/**/src/** grows no published surface — got ${JSON.stringify(touched.packages)}`);
24062515
assert(
24072516
judgeLevel({ levels: scanned.levels, touched, declaration: declaredYes }).verdict === 'clean',
24082517
'end to end: `patch` beside a tests-only diff is not this gate\'s business, even under a `yes` declaration',

0 commit comments

Comments
 (0)