Skip to content

Commit 91f65c4

Browse files
claude[bot]claude
andauthored
fix(scripts): read nested exports conditions, and refuse unreadable ones by name (#17100)
`resolveConditions()` handed `exports[subpath][flavour]` straight to `node:path`'s `resolve()` with no string check. The one guarded row (`@objectstack/objectql/core`) spells its conditions flat, so the gate's live verdict was correct and nothing was mis-measured. The cost fell entirely on the next author: `packages/metadata`'s `./errors` spells them NESTED (`import`/`require` -> objects of `types`/`default`), and that row died with a raw `TypeError: The "paths[2]" argument must be of type string` — no gate name, no entry id, no repair. - `conditionTarget()` reads both spellings this workspace uses: a path string, or a nested condition object's `default`. - Any other shape is REFUSED with this gate's own diagnostic naming the entry id, subpath, flavour and the shape found — never a crash from a node builtin. The gate does not guess a target, because a closure measured from the wrong artifact would be reported as if it were the right one. The shape guard one level up already had the right form; it was checking the wrong level. - Absent now means `undefined`/`null` only. The previous `if (!rel)` silently skipped `""` and `false`, which are not a manifest declining to publish a condition. - The header no longer sizes a second `GUARDED_ENTRIES` row as "a one-row change": a second entry needs its own `ADMITTED_PACKAGES` set, and admitting a package there moves the ADR-0076 D2 boundary and carries `RATCHET_AUTHORITY_MARKER`. That sentence was false before this change and stays false after it, so it says the real price instead. - Self-test gains a `reading a condition` battery: one case per branch (flat, nested, absent) plus two refusals and one end-to-end nested manifest. Battery floor ratcheted 5 -> 6; cases 16 -> 22. No `GUARDED_ENTRIES` row is added and `ADMITTED_PACKAGES` is untouched. Claude-Session: https://claude.ai/code/session_012GKcPZbMoGq7WPzKLfRBTU Co-authored-by: Claude <noreply@anthropic.com>
1 parent ad308fc commit 91f65c4

1 file changed

Lines changed: 181 additions & 6 deletions

File tree

scripts/check-lean-entry-closure.mjs

Lines changed: 181 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,18 @@
117117
* lean-entry promise (`@objectstack/metadata/errors`, 2.4 MB across 83 modules
118118
* standalone) and is the object a gate like this would measure — but it is a
119119
* separate card with a separate owner, and adding its row here would decide its
120-
* open question by writing down today's number as tomorrow's contract. The
121-
* table shape is what makes that a one-row change for whoever owns it.
120+
* open question by writing down today's number as tomorrow's contract.
121+
*
122+
* ⛔ A second entry is NOT a one-row change, and an earlier revision of this
123+
* header said it was (#16980). The table row is the cheap half. The other half
124+
* is that `ADMITTED_PACKAGES` below is ONE set belonging to ONE entry: a second
125+
* entry measures its own closure and therefore needs its own admitted set, and
126+
* a package entering an admitted set is the ADR-0076 D2 boundary moving —
127+
* which is why every widening remedy this gate prints carries
128+
* `RATCHET_AUTHORITY_MARKER`. ⇒ Whoever owns a second entry owns a MAINTAINER
129+
* decision, not just a row. `resolveConditions()` reads both `exports`
130+
* spellings this workspace uses, so the row itself is at least honest now; that
131+
* is the part this header used to get wrong, not the price.
122132
*/
123133

124134
import { spawnSync } from 'node:child_process';
@@ -430,6 +440,85 @@ export function judge(grouped, label) {
430440

431441
// ── The run ─────────────────────────────────────────────────────────────────
432442

443+
/**
444+
* Describe a condition value this gate refused, in terms a manifest author can
445+
* act on — the JS shape, and for an object the keys it does carry.
446+
*
447+
* ⛔ It reports what was found and nothing more. It does not name a cause,
448+
* because from here the gate cannot tell a malformed manifest from an `exports`
449+
* spelling it has simply never been taught.
450+
*
451+
* @param {unknown} value
452+
* @returns {string}
453+
*/
454+
export function describeConditionShape(value) {
455+
if (value === null) return 'null';
456+
if (Array.isArray(value)) return `an array of ${value.length} element(s)`;
457+
if (typeof value === 'object') {
458+
const keys = Object.keys(value);
459+
return keys.length
460+
? `an object with keys [${keys.join(', ')}] and no string "default"`
461+
: 'an empty object';
462+
}
463+
if (typeof value === 'string') return 'an empty string';
464+
return `a ${typeof value} (${JSON.stringify(value) ?? String(value)})`;
465+
}
466+
467+
/**
468+
* The path ONE `exports` condition names — for the two spellings this workspace
469+
* actually uses — or a refusal that says what it found instead.
470+
*
471+
* flat "./core": { "import": "./dist/core.mjs" }
472+
* nested "./errors": { "import": { "types": "…", "default": "./dist/errors.js" } }
473+
*
474+
* Both spellings are live here, so reading only the flat one was never a
475+
* simplification: it was a gate that could not read half of its own repo.
476+
* Before #16980 the nested spelling was handed to `node:path` as an object and
477+
* died with a raw `TypeError: The "paths[2]" argument must be of type string`
478+
* — no gate name, no entry id, no repair. The shape guard one level up (the
479+
* `exports[subpath]` object check in `resolveConditions()`) already had the
480+
* right form; it was checking the wrong level.
481+
*
482+
* ⛔ Any other shape is REFUSED here rather than guessed at. A guess would pick
483+
* some file and then report a closure measured from the wrong artifact as if it
484+
* had measured the right one — a confident wrong answer, which is strictly
485+
* worse than the crash it would replace.
486+
*
487+
* ⚠️ Absent means absent, and only `undefined` and `null` mean it. Every other
488+
* falsy value was skipped silently by the previous `if (!rel) continue`; they
489+
* are refusals now, because `""` and `false` are not a manifest declining to
490+
* publish a condition — they are a manifest this gate cannot read.
491+
*
492+
* @param {{pkgDir: string, subpath: string, id: string}} entry
493+
* @param {'import' | 'require'} flavour
494+
* @param {unknown} value the raw `exports[subpath][flavour]`
495+
* @returns {string | null} the relative path, or null when the condition is not published
496+
*/
497+
export function conditionTarget(entry, flavour, value) {
498+
if (value === undefined || value === null) return null;
499+
if (typeof value === 'string' && value !== '') return value;
500+
if (
501+
typeof value === 'object'
502+
&& !Array.isArray(value)
503+
&& typeof value.default === 'string'
504+
&& value.default !== ''
505+
) {
506+
return value.default;
507+
}
508+
throw new Error(
509+
`${entry.pkgDir}/package.json declares exports["${entry.subpath}"]["${flavour}"] as `
510+
+ `${describeConditionShape(value)}; this gate reads a condition only as a path string, or as `
511+
+ 'a nested condition object whose "default" is a path string.\n'
512+
+ ` entry: ${entry.id}\n`
513+
+ ` subpath: ${entry.subpath}\n`
514+
+ ` flavour: ${flavour}\n`
515+
+ ' ⇒ Either that manifest is wrong, or it uses an exports spelling conditionTarget() '
516+
+ 'in this file has not been taught. This gate cannot tell which from here, so it names the '
517+
+ 'shape and stops. ⛔ What it may not do is resolve something anyway: a closure measured from '
518+
+ 'the wrong artifact would be reported as if it were the right one.',
519+
);
520+
}
521+
433522
/**
434523
* Resolve a guarded entry's conditions from its own manifest.
435524
*
@@ -446,8 +535,8 @@ export function resolveConditions(entry) {
446535
const conditions = [];
447536
const missing = [];
448537
for (const flavour of /** @type {const} */ (['import', 'require'])) {
449-
const rel = map[flavour];
450-
if (!rel) continue;
538+
const rel = conditionTarget(entry, flavour, map[flavour]);
539+
if (rel === null) continue;
451540
const target = resolve(ROOT, entry.pkgDir, rel);
452541
if (existsSync(target)) conditions.push({ flavour, target });
453542
else missing.push(`${entry.id} (${flavour}) -> ${relative(ROOT, target)}`);
@@ -581,11 +670,12 @@ const SELF_TEST_BATTERIES = Object.freeze({
581670
'the three judgements': 5,
582671
'the disjointness wall': 2,
583672
'measuring a real load': 3,
673+
'reading a condition': 6,
584674
'the prerequisite': 2,
585675
});
586676

587677
/** Deleting a roster entry silences its floor as surely as zeroing it. */
588-
const SELF_TEST_BATTERY_FLOOR = 5;
678+
const SELF_TEST_BATTERY_FLOOR = 6;
589679

590680
const UNATTRIBUTED_BATTERY = '(no battery open)';
591681

@@ -749,6 +839,90 @@ export function selfTest() {
749839
const broken = measure(join(fixture, 'no-such-entry.mjs'), 'import');
750840
t('an entry that does not load is an ERROR, never an empty closure', 'error' in broken, JSON.stringify(broken).slice(0, 200));
751841

842+
// ── reading a condition ───────────────────────────────────────────────
843+
//
844+
// One case per branch of conditionTarget(), because the branch that was
845+
// MISSING is exactly what #16980 cost: a nested condition was handed to
846+
// node:path as an object and died there. The refusal branch is pinned in
847+
// the same battery — an unreadable shape has to arrive as this gate's own
848+
// sentence, naming the entry, and never as a crash from a node builtin.
849+
battery('reading a condition');
850+
const condEntry = { id: '@fixture/pkg/errors', pkgDir: 'packages/fixture', subpath: './errors' };
851+
t(
852+
'a flat condition is the path string itself',
853+
conditionTarget(condEntry, 'import', './dist/errors.js') === './dist/errors.js',
854+
);
855+
t(
856+
'a nested condition is read through its "default"',
857+
conditionTarget(condEntry, 'require', { types: './dist/errors.d.cts', default: './dist/errors.cjs' })
858+
=== './dist/errors.cjs',
859+
);
860+
t(
861+
'an unpublished condition is absent, not a refusal',
862+
conditionTarget(condEntry, 'import', undefined) === null
863+
&& conditionTarget(condEntry, 'require', null) === null,
864+
);
865+
866+
/** @param {unknown} value */
867+
const refuseCondition = (value) => {
868+
try {
869+
conditionTarget(condEntry, 'import', value);
870+
return { threw: false, ctor: '(none)', message: '(returned a target instead of refusing)' };
871+
} catch (e) {
872+
return { threw: true, ctor: e.constructor.name, message: String(e.message) };
873+
}
874+
};
875+
876+
const noDefault = refuseCondition({ types: './dist/errors.d.ts', browser: './dist/errors.browser.js' });
877+
t(
878+
'a condition object with no "default" is refused by this gate, naming entry, subpath and shape',
879+
noDefault.threw && noDefault.ctor === 'Error'
880+
&& noDefault.message.includes(condEntry.id)
881+
&& noDefault.message.includes(condEntry.subpath)
882+
&& noDefault.message.includes('types, browser'),
883+
`${noDefault.ctor}: ${noDefault.message.slice(0, 220)}`,
884+
);
885+
886+
const thirdShape = refuseCondition(['./dist/a.js', './dist/b.js']);
887+
t(
888+
'a third shape is refused too, and the refusal reports what it found rather than guessing',
889+
thirdShape.threw && thirdShape.ctor === 'Error'
890+
&& thirdShape.message.includes('an array of 2 element(s)')
891+
&& thirdShape.message.includes(condEntry.id),
892+
`${thirdShape.ctor}: ${thirdShape.message.slice(0, 220)}`,
893+
);
894+
895+
t(
896+
'resolveConditions reads a NESTED manifest end to end — the shape that used to reach node:path',
897+
(() => {
898+
const nestedPkg = join(scratch, 'nestedpkg');
899+
mkdirSync(nestedPkg, { recursive: true });
900+
writeFileSync(
901+
join(nestedPkg, 'package.json'),
902+
JSON.stringify({
903+
name: 'nestedpkg',
904+
exports: {
905+
'./errors': {
906+
import: { types: './dist/errors.d.ts', default: './dist/errors.js' },
907+
require: { types: './dist/errors.d.cts', default: './dist/errors.cjs' },
908+
},
909+
},
910+
}),
911+
);
912+
const { conditions, missing } = resolveConditions({
913+
id: 'nestedpkg/errors',
914+
pkgDir: relative(ROOT, nestedPkg),
915+
subpath: './errors',
916+
});
917+
// Unbuilt by construction, so both conditions land in `missing` — which
918+
// is the point: they are NAMED, having been resolved to real paths,
919+
// rather than never reaching a diagnostic at all.
920+
return conditions.length === 0 && missing.length === 2
921+
&& missing[0].endsWith(join('dist', 'errors.js'))
922+
&& missing[1].endsWith(join('dist', 'errors.cjs'));
923+
})(),
924+
);
925+
752926
// ── the prerequisite ──────────────────────────────────────────────────
753927
battery('the prerequisite');
754928
t(
@@ -792,7 +966,8 @@ export function selfTest() {
792966
}
793967
console.log(
794968
`✓ check-lean-entry-closure self-test: ${cases.length} cases pass `
795-
+ '(attribution, all three judgements, the disjointness wall, two real child loads, the prerequisite).',
969+
+ '(attribution, all three judgements, the disjointness wall, two real child loads, '
970+
+ 'both condition spellings plus two refusals, the prerequisite).',
796971
);
797972
selfTestReachedVerdict = true;
798973
return 0;

0 commit comments

Comments
 (0)