Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
117 changes: 110 additions & 7 deletions scripts/__tests__/check-sdui-registration-pins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
countChunksCarrying,
derivePinnedKeys,
main,
sourceFirstEntries,
} from '../check-sdui-registration-pins.mjs';

/**
Expand All @@ -28,8 +29,18 @@ import {

const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..');

/** A fixture root: one array package with one registrar, plus a fake console dist. */
function fixture(chunks: Record<string, string>): string {
/** The registrar body both spellings of the fixture package carry. */
const REGISTRAR = "import { ComponentRegistry } from 'somewhere';\nComponentRegistry.register('fixture:widget', 1);\n";

/**
* A fixture root: one array package with one registrar, plus a fake console dist.
*
* `built` writes the package's PUBLISHED spelling too. The array always names
* both (that is the `check-side-effects-array` rule), but only a built tree has
* both on disk — so this flag is how a case states which build state it means
* instead of inheriting the runner's, which is objectui#6893's whole subject.
*/
function fixture(chunks: Record<string, string>, { built = false }: { built?: boolean } = {}): string {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'objectui-6683-pins-'));
fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n");
const pkg = path.join(dir, 'packages/pkg');
Expand All @@ -42,10 +53,11 @@ function fixture(chunks: Record<string, string>): string {
sideEffects: ['./dist/index.js', './src/index.ts'],
}),
);
fs.writeFileSync(
path.join(pkg, 'src/index.ts'),
"import { ComponentRegistry } from 'somewhere';\nComponentRegistry.register('fixture:widget', 1);\n",
);
fs.writeFileSync(path.join(pkg, 'src/index.ts'), REGISTRAR);
if (built) {
fs.mkdirSync(path.join(pkg, 'dist'), { recursive: true });
fs.writeFileSync(path.join(pkg, 'dist/index.js'), REGISTRAR);
}
const assets = path.join(dir, 'apps/console/dist/assets');
fs.mkdirSync(assets, { recursive: true });
for (const [name, code] of Object.entries(chunks)) fs.writeFileSync(path.join(assets, name), code);
Expand Down Expand Up @@ -132,6 +144,91 @@ describe('the fixture console', () => {
});
});

describe('the source spelling wins, whatever the tree has been built to', () => {
// objectui#6893. A `sideEffects` array names every registrar TWICE — once as
// `src/x.tsx`, once as `dist/x.js` — and `derivePinnedKeys` attributes a key
// to the FIRST module it read it from. With no order of its own, the winner
// was decided by the array's literal order in `package.json` AND by whether
// `dist/` happened to be on disk. `packages/app-shell/dist` is gitignored, so
// the SAME COMMIT answered the source spelling on an unbuilt checkout and the
// published one on a built checkout — a verdict that is a function of hidden
// local state, which is the family this repo keeps paying for.
//
// The two cases below are ONE assertion run over the two build states, which
// is the property itself. They use a fixture rather than the workspace on
// purpose: a fixture owns its own build state, so these cases keep asserting
// the preference on a machine where nothing has been built — exactly the
// machines a conditional skip would have stopped running on.
const chunk = { 'index-abc.js': "R.register('fixture:widget',1)" };

it('attributes the key to the SOURCE spelling when BOTH spellings are on disk', () => {
const dir = fixture(chunk, { built: true });
try {
const { keys, sources, modulesRead } = derivePinnedKeys(dir);
// Both spellings were still READ: this is a reordering, not a filter, so
// the derived population — the only input to the gate's verdict — cannot
// move. If this drops to 1 the fix has started hiding modules instead.
expect(modulesRead).toBe(2);
expect(keys).toEqual(['fixture:widget']);
expect(sources.get('fixture:widget')).toBe('packages/pkg/src/index.ts');
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});

it('gives that same answer when only the source spelling is on disk', () => {
const dir = fixture(chunk);
try {
const { keys, sources, modulesRead } = derivePinnedKeys(dir);
expect(modulesRead).toBe(1);
expect(keys).toEqual(['fixture:widget']);
expect(sources.get('fixture:widget')).toBe('packages/pkg/src/index.ts');
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});

it('THROWS rather than falling back to array order when the two spellings cannot be told apart', () => {
// The ordering is derived from the package's own spelling map. When that map
// cannot be derived there is no source-first answer, and quietly reverting
// to array order would restore the build-state-dependent attribution above
// — silently, which is the failure this gate exists to refuse. Loud instead;
// `scripts/check-side-effects-array.mjs` owns this condition and already
// reports it as exit 2, so such a workspace is red there too.
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'objectui-6893-nomap-'));
try {
fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n");
const pkg = path.join(dir, 'packages/pkg');
fs.mkdirSync(path.join(pkg, 'src'), { recursive: true });
// `main` names the SOURCE barrel, so the manifest publishes no
// `index.js`-shaped entry and the map has nothing to anchor on.
fs.writeFileSync(
path.join(pkg, 'package.json'),
JSON.stringify({ name: '@fixture/pkg', main: './src/index.ts', sideEffects: ['./src/index.ts'] }),
);
fs.writeFileSync(path.join(pkg, 'src/index.ts'), REGISTRAR);
expect(() => derivePinnedKeys(dir)).toThrow(/source spelling from its published one/);
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});

it('keeps every declared entry — it reorders the read, it does not filter it', () => {
// The population guard for the reordering itself: a partition that dropped
// an entry would shrink the derived key set without any assertion noticing.
const declared = ['dist/index.js', 'src/index.ts', 'src/styles.css', 'dist/a/b.js', 'src/a/b.tsx'];
const dir = fixture(chunk, { built: true });
try {
const pkg = { name: '@fixture/pkg', dir: 'packages/pkg', declared, manifest: { main: './dist/index.js' } };
const ordered = sourceFirstEntries(pkg, dir);
expect([...ordered].sort()).toEqual([...declared].sort());
expect(ordered.slice(0, 3).every((e) => e.startsWith('src/'))).toBe(true);
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});
});

describe('the real workspace', () => {
it('derives the keys from the arrays, and the ruled controls are among them', () => {
const { keys, sources, unreadable, modulesRead } = derivePinnedKeys(repoRoot);
Expand All @@ -142,7 +239,13 @@ describe('the real workspace', () => {
expect(keys, `${control} is one of the three registrations the 2026-08-29 ruling pins`).toContain(control);
}
// The derivation must point at the module it read the key from, or a drop
// would be reported without saying which array entry promised it.
// would be reported without saying which array entry promised it — and it
// must point at the SOURCE module, the one an author can go and fix, not at
// the gitignored build artifact beside it. Before objectui#6893 this line
// was the workspace's build state in disguise: green on an unbuilt checkout,
// red on a built one, same commit. The preference itself is pinned above on
// a fixture that owns its build state; this line is the real workspace's
// half, and it now holds in every build state.
expect(sources.get('mcp:connect-agent')).toBe(
'packages/app-shell/src/console/connect/ConnectAgentWidget.tsx',
);
Expand Down
57 changes: 55 additions & 2 deletions scripts/check-sdui-registration-pins.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import fs from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { findComponentRegistrations } from './component-registrations.mjs';
import { readArrayPackages } from './check-side-effects-array.mjs';
import { deriveSpellingMap, readArrayPackages } from './check-side-effects-array.mjs';
import { isEntrypoint } from './invoked-as.mjs';

const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
Expand Down Expand Up @@ -91,10 +91,63 @@ export const RULED_CONTROLS = Object.freeze([
*/
export const NEGATIVE_CONTROL_KEY = 'objectui:6683-registration-pin-negative-control';

/**
* `pkg.declared`, ordered so a module's SOURCE spelling is read before its
* PUBLISHED spelling.
*
* ## Why the order is fixed here rather than left to the array (objectui#6893)
*
* A `sideEffects` array names every registering module TWICE -- once as
* `src/x.tsx`, once as `dist/x.js` (see `check-side-effects-array.mjs`: both
* spellings are required, because consumers resolve the published one and
* in-repo bundler aliases resolve the source one). {@link derivePinnedKeys}
* attributes each key to the FIRST module it read the key from, so without an
* order of its own the attribution was decided by two things that have nothing
* to say about it: the literal order of the entries in `package.json`, and
* whether `dist/` happens to be on disk. `packages/app-shell/dist` is
* gitignored, so the SAME COMMIT attributed `mcp:connect-agent` to
* `.../src/console/connect/ConnectAgentWidget.tsx` on an unbuilt checkout and to
* `.../dist/console/connect/ConnectAgentWidget.js` on a built one.
*
* The published spelling is still READ -- `keys` is a union deduplicated by key,
* so reordering cannot change the derived population, only which spelling is
* reported for it. Source-first is the right end of that choice because the
* attribution is a diagnostic pointing an author at the module to go fix, and
* `dist/` is a build artifact nobody edits.
*
* `srcRoot` comes from the package's own derived spelling map rather than from a
* hardcoded `dist` test, so there is no second answer here to "which prefix is
* the source one" that could rot away from the one `check-side-effects-array`
* derives and round-trip-checks.
*
* @param {{name: string, dir: string, manifest: object, declared: string[]}} pkg
* @param {string} [root]
* @returns {string[]}
*/
export function sourceFirstEntries(pkg, root = REPO_ROOT) {
const map = deriveSpellingMap(pkg, root);
if (map.error) {
// Falling back to the array's own order would restore exactly the
// build-state-dependent attribution above, and it would do it silently.
// `scripts/check-side-effects-array.mjs` owns this condition and already
// reports it as exit 2, so a workspace that reaches here is red there too.
throw new Error(
`${pkg.name}: this gate cannot tell the package's source spelling from its published one — ` +
`${map.error}. Without that, the key attribution below falls back to array order, which is what ` +
`made this derivation answer differently on a built and an unbuilt checkout (objectui#6893).`,
);
}
const isSource = (entry) => entry === map.srcRoot || entry.startsWith(`${map.srcRoot}/`);
return [...pkg.declared.filter(isSource), ...pkg.declared.filter((entry) => !isSource(entry))];
}

/**
* Every component key registered by a module some package's `sideEffects` array
* names. Derived; see the header.
*
* The per-package read order is {@link sourceFirstEntries}, so `sources`
* answers the same spelling whether or not the tree has been built.
*
* @param {string} [root]
* @returns {{keys: string[], sources: Map<string, string>, unreadable: string[], modulesRead: number}}
*/
Expand All @@ -105,7 +158,7 @@ export function derivePinnedKeys(root = REPO_ROOT) {
let modulesRead = 0;

for (const pkg of readArrayPackages(root)) {
for (const entry of pkg.declared) {
for (const entry of sourceFirstEntries(pkg, root)) {
if (!/\.(ts|tsx|mts|js|jsx|mjs)$/.test(entry)) continue;
const abs = path.join(root, pkg.dir, entry);
if (!fs.existsSync(abs)) continue; // a `dist/*` spelling in an unbuilt tree
Expand Down
Loading