Skip to content

Commit c79eda5

Browse files
fix(ui/scripts): check:node-esm-load and check:published-dist build modes run the release's build (#27014)
<!-- claude-projects-attribution: {"github_login":"os-warren"} --> _Requested by **Warren** · [project thread](https://claude.ai/code/project/chan_01U5RdnAAmkSQmZbqh6FtqZ8?thread=cmsg_01U5RdnAAmkSQmZbqh6FtqZ8W3o2SDQXgpRSRQTXHo1oX3)_ Fixes #26860 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:`check:node-esm-load` 与 `check:published-dist` 不带 `--no-build` 手跑时,改为在仓根跑发布用的那条 `pnpm build`;两脚本共用同一个构建函数。 - 为什么改:之前一个在 `ui/` 里跑不存在的 `build` 脚本直接崩,另一个构建了含文档站的 128 个任务,读到的不是发布的产物,本地复现 CI 红灯无从对照。 - 风险与回滚:只动 `ui/scripts` 两个工具脚本与其测试,不发布任何包;CI 与发布路径本来就带 `--no-build`,行为不变;回滚即 revert 本 PR。 - 先看:`ui/scripts/check-published-dist-tooling.mjs` 的 `buildWorkspace`;`ui/scripts/check-node-esm-load.mjs` 的构建步骤;`ui/scripts/__tests__/check-published-dist-tooling.test.ts` 钉发布构建的那条用例。 - 两分钟验证:`pnpm --dir ui exec vitest run scripts/__tests__/check-node-esm-load.test.ts scripts/__tests__/check-published-dist-tooling.test.ts`,预期 89 个测试全绿。 - 需要你做的:无,席位落地。 ## What changes - `buildWorkspace` (published-dist gate) now runs the **release's own build**: the workspace root's `build` script, run from the workspace root (`workspaceRoot()` from `ui/scripts/workspace-root.mjs`). `--concurrency` / `--force` are appended and pnpm hands them to turbo. A workspace root with no `build` script is refused by name instead of surfacing as pnpm's exit 254. - `check-node-esm-load.mjs` full mode now calls that same `buildWorkspace` at the workspace root (`--force-build` maps to turbo `--force`), so both gates read one build. `main` takes an injectable `build` seam so a test can read which build it asks for without paying one. - Stale docblock text refreshed: both gates said `--no-build` "is wired into nothing" (it is wired into `scripts/release-publish.sh` and ci.yml's `UI Published Artifacts` job), and the published-dist header named `pnpm changeset:publish` as the publish path. ## Why Shape chosen by measurement, per the card's two options: "what the release builds" is **one stable command**. `pnpm run release` opens with `pnpm run build`; release.yml's `publish` job runs `pnpm run build`; ci.yml's `UI Published Artifacts` job runs `pnpm build` before both gates read with `--no-build`. So the build mode runs that command rather than being removed. Measured on `5e99bd93`: the old published-dist build (`turbo run build --filter=!@object-ui/site` from `ui/`) planned 128 tasks including `@objectstack/docs`; root `pnpm build` plans 127 tasks, neither docs nor site, 41 of them `@object-ui/*`. The publish path is unchanged: `scripts/release-publish.sh` and ci.yml both pass `--no-build`, so neither builds through this code. ## Tests - `check-published-dist-tooling.test.ts`: `buildWorkspace` runs `pnpm build` with `cwd` at the given workspace root; forwards `--concurrency=2 --force`; refuses a manifest with no `build` script (naming the manifest, without exec); still throws when the build fails; and a pin that the workspace root's `build` script is what `pnpm run release` opens with and what release.yml's `publish` job runs. - `check-node-esm-load.test.ts`: `main(['--force-build'])` asks the injected build for `workspaceRoot(ui root)` with `force: true`. - Both files: 89/89 pass. No ablation: the diff is tooling scripts only (os-dev: scripts are not ablated). ## Acceptance notes - `ui/package.json` `changeset:publish` still runs the published-dist gate without `--no-build`; after this PR it would build the release's build too. Nothing in the repository invokes it (the release publishes through `scripts/release-publish.sh`), and the `wiring` test's comment still calls it "the command changesets/action runs". Left as is: out of this card's scope. - Follow-up owed: `.github/workflows/release.yml`'s comment over the publish step is stale in the way the card names (governed, Tier H; not edited here). ## Verification All on head `cf75e342`, in a fresh worktree, under the shared verify lock (shared-box seconds). <details><summary>Acceptance: both aliases by hand, exit 0</summary> - `pnpm --dir ui run check:node-esm-load --force-build` → exit 0. turbo: 116 successful, 0 cached. Provenance leg: 35 of 35 gradable entries built by this tree. Load leg: 33 of 36 published ESM entries imported and evaluated (plugin-dashboard and plugin-map by ruling; one bin-only CLI). - `pnpm --dir ui run check:published-dist` → exit 0. turbo: 116 cached of 116 (this tree's own forced build). "Inspected 36 published package(s): 5813 tarball file(s), 5668 of them build output, 0 tooling artifact(s) in build output." - `pnpm --dir ui run check:node-esm-load` → exit 0. Provenance 35 of 35 own; load 33 of 36. - Before the fix (`5e99bd93`): `check:node-esm-load` exit 1, `ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "build" not found`. </details> <details><summary>Counter-probes, one per script (restored and re-run green)</summary> - published-dist: a test file copied into `ui/packages/core/dist/probe-copied.test.js`, then `check:published-dist --no-build` → exit 1, `@object-ui/core [tooling-in-published-output] packages/core/dist/probe-copied.test.js`. Removed → exit 0. - node-esm-load: an extensionless relative re-export (`export * from './probe-extensionless'`) appended to `ui/packages/core/dist/index.js`, then `check:node-esm-load --no-build` → exit 1, `@object-ui/core: ERR_MODULE_NOT_FOUND` (and its dependants, attributed via `@object-ui/core`). Restored by hash → exit 0. </details> <details><summary>Gates</summary> `dispatch-gates --commands` derived 18 families; all 18 exit 0, reconciled with `--ran` (0 NOT-MEASURED). Includes `pnpm --dir ui exec vitest run scripts/` (178 files passed, 2 skipped; 5069 tests passed). Also run: `node scripts/check-sdui-manifest.mjs`, ui eslint on the four changed files, `check:new-line-citations`, `check:spec-symbols`, all exit 0. </details> No changeset: `ui/scripts` ships in no package (`skip-changeset`). --- _Generated by [Claude Code](https://claude.ai/code/session_01JQwxbXWtZtWh3syXudgoFj)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6d9e5a1 commit c79eda5

4 files changed

Lines changed: 139 additions & 34 deletions

File tree

‎ui/scripts/__tests__/check-node-esm-load.test.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it } from 'vitest';
1+
import { describe, expect, it, vi } from 'vitest';
22
import fs from 'node:fs';
33
import os from 'node:os';
44
import path from 'node:path';
@@ -22,12 +22,14 @@ import {
2222
emittedSources,
2323
esmEntryOf,
2424
importEntry,
25+
main,
2526
readTsconfig,
2627
relativeSpecifiers,
2728
resolvesToModule,
2829
scanSpecifiers,
2930
} from '../check-node-esm-load.mjs';
3031
import { SKIP_DIRS, discoverPackages } from '../check-phantom-dependencies.mjs';
32+
import { workspaceRoot } from '../workspace-root.mjs';
3133

3234
/**
3335
* objectui#4538 — a published entry plain Node ESM cannot load.
@@ -832,3 +834,34 @@ describe('objectui#7276 — the load leg grades only what this tree produced', (
832834
}
833835
});
834836
});
837+
838+
/**
839+
* objectstack#26860 — the build mode runs the release's build, where the release runs it.
840+
*
841+
* The gate's full mode ran `pnpm build` from `ui/`, whose manifest has no
842+
* `build` script, so a maintainer running the alias by hand got pnpm's exit 254
843+
* and no reading. It now asks for the published-dist gate's `buildWorkspace`
844+
* (the workspace root's `build` script, from the workspace root — pinned over
845+
* there against what `pnpm run release` runs), so both gates read one build.
846+
*/
847+
describe('objectstack#26860 — the load leg is graded over the release build', () => {
848+
it('asks for the release build at the workspace root, with --force-build as turbo --force', () => {
849+
const calls: Array<{ workspace: string; force: unknown }> = [];
850+
const quiet = [
851+
vi.spyOn(console, 'log').mockImplementation(() => {}),
852+
vi.spyOn(console, 'error').mockImplementation(() => {}),
853+
];
854+
try {
855+
const exit = main(['--force-build'], {
856+
build: (workspace: string, options?: { force?: boolean }) => {
857+
calls.push({ workspace, force: options?.force });
858+
throw new Error('stop before grading: this case reads only which build was asked for');
859+
},
860+
});
861+
expect(exit).toBe(1);
862+
} finally {
863+
for (const spy of quiet) spy.mockRestore();
864+
}
865+
expect(calls).toEqual([{ workspace: workspaceRoot(REPO_ROOT), force: true }]);
866+
});
867+
});

‎ui/scripts/__tests__/check-published-dist-tooling.test.ts‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
toolingConventionFrom,
2121
} from '../check-published-dist-tooling.mjs';
2222
import { jobBlock, rootCi, rootWorkflow, rootWorkflowFiles, uiRun, withoutComments } from './helpers/root-ci';
23+
import { rootManifest, workspaceRoot } from '../workspace-root.mjs';
2324

2425
/**
2526
* objectui#4846 — no published package's build output may carry tooling material.
@@ -359,41 +360,79 @@ describe('analyze', () => {
359360
// ── 4. a build that fails is a failure ───────────────────────────────────────
360361

361362
describe('buildWorkspace', () => {
362-
it('runs the repository build and reports how long it took', () => {
363+
/** A scratch workspace root whose manifest declares the given scripts. */
364+
const workspaceWith = (scripts: Record<string, string>) => {
365+
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'published-dist-build-'));
366+
fixtures.push(root);
367+
fs.writeFileSync(path.join(root, 'package.json'), JSON.stringify({ name: 'fixture-root', scripts }));
368+
return root;
369+
};
370+
371+
it("runs the workspace root's `build` script, from the workspace root, and reports how long it took", () => {
372+
// objectstack#26860: the gate used to run `turbo run build` from `ui/`,
373+
// 128 tasks including the docs app, which is not the build that publishes.
374+
const root = workspaceWith({ build: 'turbo run build' });
363375
const calls: unknown[][] = [];
364-
const seconds = buildWorkspace('/repo', {
376+
const seconds = buildWorkspace(root, {
365377
exec: (...args: unknown[]) => {
366378
calls.push(args);
367379
},
368380
});
369381
expect(typeof seconds).toBe('number');
370382
expect(calls).toHaveLength(1);
371383
expect(calls[0][0]).toBe('pnpm');
372-
expect(calls[0][1]).toEqual(['exec', 'turbo', 'run', 'build', '--filter=!@object-ui/site']);
384+
expect(calls[0][1]).toEqual(['build']);
385+
expect((calls[0][2] as { cwd: string }).cwd).toBe(root);
373386
});
374387

375388
it('forwards --concurrency and --force when asked', () => {
376389
const calls: unknown[][] = [];
377-
buildWorkspace('/repo', {
390+
buildWorkspace(workspaceWith({ build: 'turbo run build' }), {
378391
concurrency: 2,
379392
force: true,
380393
exec: (...args: unknown[]) => {
381394
calls.push(args);
382395
},
383396
});
384-
expect(calls[0][1]).toContain('--concurrency=2');
385-
expect(calls[0][1]).toContain('--force');
397+
expect(calls[0][1]).toEqual(['build', '--concurrency=2', '--force']);
398+
});
399+
400+
it('refuses, naming the manifest, when the workspace root declares no `build` script', () => {
401+
// The defect objectstack#26860 was filed from: `check:node-esm-load` ran a
402+
// `build` script `ui/package.json` does not have, and pnpm's exit 254 was
403+
// the whole diagnosis.
404+
const root = workspaceWith({ test: 'vitest' });
405+
const exec = () => {
406+
throw new Error('must not run');
407+
};
408+
expect(() => buildWorkspace(root, { exec })).toThrow(/declares no `build` script/);
409+
expect(() => buildWorkspace(root, { exec })).toThrow(new RegExp(`${root}/package\\.json`));
386410
});
387411

388412
it('throws — never returns — when the build fails', () => {
389413
expect(() =>
390-
buildWorkspace('/repo', {
414+
buildWorkspace(workspaceWith({ build: 'turbo run build' }), {
391415
exec: () => {
392416
throw new Error('turbo exited 1');
393417
},
394418
}),
395419
).toThrow(/nothing trustworthy to inspect/);
396420
});
421+
422+
it("is the release's build: the workspace root's `build` script, which `pnpm run release` opens with", () => {
423+
// What the publish path runs before `scripts/release-publish.sh` reads the
424+
// artifacts with `--no-build`. If the release ever builds something else,
425+
// this gate's build mode no longer reads what publishes, and this goes red.
426+
// `repoRoot` in this file is the objectui root, `ui/` under the monorepo.
427+
const workspace = workspaceRoot(repoRoot);
428+
const scripts = rootManifest(repoRoot).scripts as Record<string, string>;
429+
expect(scripts.build).toEqual(expect.any(String));
430+
expect(scripts.release.startsWith('pnpm run build && ')).toBe(true);
431+
expect(withoutComments(jobBlock(rootWorkflow('release.yml'), 'publish'))).toContain('run: pnpm run build\n');
432+
expect(scripts.build).toBe(
433+
JSON.parse(fs.readFileSync(path.join(workspace, 'package.json'), 'utf8')).scripts.build,
434+
);
435+
});
397436
});
398437

399438
// ── 5. scope: private out, apps/ in ──────────────────────────────────────────

‎ui/scripts/check-node-esm-load.mjs‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@
9797
* about 1065x. Paying that on every run to make "I produced these" true by
9898
* construction is the wrong trade for a gate people run locally, so the
9999
* default keeps the cache and leg 3 carries the invariant for a few file
100-
* reads. `--force-build` buys the expensive form (`TURBO_FORCE`) for the
100+
* reads. `--force-build` buys the expensive form (turbo's `--force`) for the
101101
* caller who wants a reading with no shared-cache question in it at all.
102102
*
103103
* Leg 2 also carries this repository's one PRODUCT boundary: three
@@ -111,8 +111,8 @@
111111
* - "Scan `dist/` if it exists." With no per-PR full-repo build `dist/` is
112112
* usually absent, so such a gate passes because it looked at NOTHING. Leg 1
113113
* therefore reads SOURCES, which are always present, and leg 2 BUILDS
114-
* rather than hoping someone else did (`--no-build` exists for callers that
115-
* just built, and is wired into nothing).
114+
* rather than hoping someone else did (`--no-build` is for callers that
115+
* just ran the release's build, as ci.yml's full battery does).
116116
* - "Resolve the entry." Measured and rejected above: resolution succeeded on
117117
* the very package the card was filed from.
118118
* - "Import it and count the successes." A run where every package failed for
@@ -141,15 +141,17 @@
141141
* under objectui#5367: TypeScript's own tsconfig reader.
142142
*/
143143

144-
import { execFileSync, spawnSync } from 'node:child_process';
144+
import { spawnSync } from 'node:child_process';
145145
import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs';
146146
import { dirname, join, resolve } from 'node:path';
147147
import { fileURLToPath } from 'node:url';
148148

149149
import ts from 'typescript';
150150

151151
import { SKIP_DIRS, TOOLING_FILE, discoverPackages, moduleSpecifiers } from './check-phantom-dependencies.mjs';
152+
import { buildWorkspace } from './check-published-dist-tooling.mjs';
152153
import { isEntrypoint } from './invoked-as.mjs';
154+
import { workspaceRoot } from './workspace-root.mjs';
153155
import { UI_PUBLISHED_PACKAGE_FLOOR } from './__tests__/helpers/ui-package-population.mjs';
154156

155157
const scriptDir = dirname(fileURLToPath(import.meta.url));
@@ -822,7 +824,12 @@ export function attributeMissingModule(missingPath, packages, root) {
822824

823825
// ── the run ──────────────────────────────────────────────────────────────────
824826

825-
function main(argv) {
827+
/**
828+
* @param {string[]} argv
829+
* @param {{ build?: typeof buildWorkspace }} [seams] `build` is the release's build (see
830+
* `buildWorkspace`); a test swaps it to read which build this gate asks for without paying one.
831+
*/
832+
function main(argv, { build = buildWorkspace } = {}) {
826833
const specifiersOnly = argv.includes('--specifiers-only');
827834
const noBuild = argv.includes('--no-build');
828835
// Buys the expensive form of the same invariant: with the cache off, every
@@ -909,19 +916,18 @@ function main(argv) {
909916
if (!noBuild) {
910917
console.log(
911918
forceBuild
912-
? '\nBuilding every published package with the cache OFF (TURBO_FORCE), so every artifact the ' +
919+
? '\nRunning the release\'s build with the cache OFF (--force), so every artifact the ' +
913920
'load leg grades below is one this run produced…'
914-
: '\nBuilding every published package. turbo shares one cache across every worktree of this ' +
921+
: '\nRunning the release\'s build. turbo shares one cache across every worktree of this ' +
915922
'checkout, so this build can be a replay of a sibling worktree\'s — the provenance leg ' +
916923
'below reads each task log and refuses to grade what this tree did not produce ' +
917924
'(objectui#7276)…',
918925
);
919926
try {
920-
execFileSync('pnpm', ['build'], {
921-
cwd: root,
922-
stdio: 'inherit',
923-
env: forceBuild ? { ...process.env, TURBO_FORCE: 'true' } : process.env,
924-
});
927+
// The release's own build, at the workspace root: `ui/` has no `build`
928+
// script, and a build of some other task set would be graded as if it
929+
// were the one that publishes (objectstack#26860).
930+
build(workspaceRoot(root), { force: forceBuild, log: (line) => console.log(line) });
925931
} catch (error) {
926932
console.error(`\n✗ the build failed, so the load leg has nothing to grade: ${error.message}`);
927933
return 1;

‎ui/scripts/check-published-dist-tooling.mjs‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
* packages) are never built there. Paying a full build on every pull request to
5555
* catch a defect that can only reach a user at publish time is the wrong trade;
5656
* this gate therefore runs where the harm materialises — the publish path
57-
* (`pnpm changeset:publish`) — plus nightly, at zero per-PR cost.
57+
* (`scripts/release-publish.sh`) — plus `main`'s full battery, at zero per-PR cost.
5858
*
5959
* ## The vacuous form this gate must never take
6060
*
@@ -64,8 +64,8 @@
6464
* guarded. Three separate rules make that impossible here:
6565
*
6666
* 1. The gate BUILDS. It runs the repository's own build itself and fails
67-
* loudly if the build fails. `--no-build` exists for callers that just
68-
* built (and for the tests), and it is wired into nothing.
67+
* loudly if the build fails. `--no-build` is for callers that just ran
68+
* the release's build: the publish path and ci.yml's full battery both do.
6969
* 2. A published package that contributes NO file under a build output
7070
* directory is a FINDING (`no-build-output`), not a skip. That is what
7171
* keeps `--no-build` safe: a missing `dist/` is red, never green.
@@ -135,6 +135,7 @@ import { fileURLToPath } from 'node:url';
135135

136136
import { PACKAGE_ROOTS, TOOLING_FILE, readReleaseGroup } from './check-phantom-dependencies.mjs';
137137
import { isEntrypoint } from './invoked-as.mjs';
138+
import { workspaceRoot } from './workspace-root.mjs';
138139
import { UI_PUBLISHED_PACKAGE_FLOOR } from './__tests__/helpers/ui-package-population.mjs';
139140

140141
const scriptDir = dirname(fileURLToPath(import.meta.url));
@@ -291,27 +292,35 @@ export function discoverPublishedPackages(root, released = readReleaseGroup(root
291292
// ── the build ────────────────────────────────────────────────────────────────
292293

293294
/**
294-
* The repository's own build, run by the gate.
295+
* The release's own build, run by the gate.
295296
*
296-
* Deliberately the same command the release workflow runs (`pnpm build`,
297-
* i.e. turbo's `build` task minus the docs site), so the artifacts inspected
298-
* below are the artifacts a release produces, rather than a second build shape
299-
* invented here that could differ from it.
297+
* The workspace root's `build` script, run FROM the workspace root: the
298+
* command `pnpm run release` opens with, the `Build` step of release.yml's
299+
* `publish` job, and the build ci.yml's `UI Published Artifacts` job runs
300+
* before it reads with `--no-build`. So the artifacts inspected below are the
301+
* artifacts a release produces, rather than a second build shape invented here
302+
* that could differ from it. Under the monorepo that root is the PARENT of the
303+
* objectui tree (`workspaceRoot`), whose own manifest has no `build` script, so
304+
* the objectui root is the one directory this must never run in (objectstack#26860).
300305
*
301-
* @param {string} root
306+
* `--concurrency` and `--force` are appended to that script's command line,
307+
* which pnpm hands through to turbo.
308+
*
309+
* @param {string} workspace the pnpm workspace root, `workspaceRoot(uiRoot)`
302310
* @param {{ concurrency?: number|null, force?: boolean, log?: (line: string) => void, exec?: Function }} options
303311
*/
304312
export function buildWorkspace(
305-
root,
313+
workspace,
306314
{ concurrency = null, force = false, log = () => {}, exec = execFileSync } = {},
307315
) {
308-
const args = ['exec', 'turbo', 'run', 'build', '--filter=!@object-ui/site'];
316+
const script = readBuildScript(workspace);
317+
const args = ['build'];
309318
if (concurrency !== null) args.push(`--concurrency=${concurrency}`);
310319
if (force) args.push('--force');
311-
log(`Building: pnpm ${args.join(' ')}`);
320+
log(`Building: pnpm ${args.join(' ')} (in ${workspace}: ${script})`);
312321
const started = Date.now();
313322
try {
314-
exec('pnpm', args, { cwd: root, stdio: 'inherit' });
323+
exec('pnpm', args, { cwd: workspace, stdio: 'inherit' });
315324
} catch (error) {
316325
throw new Error(
317326
`the workspace build failed (${error.message}), so there is nothing trustworthy to inspect. ` +
@@ -324,6 +333,24 @@ export function buildWorkspace(
324333
return seconds;
325334
}
326335

336+
/** The workspace root's `build` script, or a loud refusal naming where it looked. */
337+
function readBuildScript(workspace) {
338+
const manifestPath = join(workspace, 'package.json');
339+
let script;
340+
try {
341+
script = JSON.parse(readFileSync(manifestPath, 'utf8')).scripts?.build;
342+
} catch (error) {
343+
throw new Error(`cannot read ${manifestPath} (${error.message}), so the release's build is unknown.`);
344+
}
345+
if (typeof script !== 'string' || script.trim() === '') {
346+
throw new Error(
347+
`${manifestPath} declares no \`build\` script, so there is no release build to run. Run the ` +
348+
"release's build yourself and pass `--no-build`.",
349+
);
350+
}
351+
return script;
352+
}
353+
327354
/** Remove each package's build output, so a stale artifact cannot be read as a fresh one. */
328355
export function cleanOutputs(root, packages, log = () => {}) {
329356
let removed = 0;
@@ -500,7 +527,7 @@ if (invokedDirectly) {
500527
'published package with no build output is reported as `no-build-output`.',
501528
);
502529
} else {
503-
buildSeconds = buildWorkspace(root, { concurrency, force, log: (line) => console.log(line) });
530+
buildSeconds = buildWorkspace(workspaceRoot(root), { concurrency, force, log: (line) => console.log(line) });
504531
}
505532
result = analyze(root, { packages });
506533
} catch (error) {

0 commit comments

Comments
 (0)