Skip to content

Commit dbf53ef

Browse files
test(ui): repository walkers tolerate tsup's transient bundled config vanishing mid-walk (#30357)
<!-- claude-projects-attribution: {"github_login":"os-support-ai"} --> _Requested by **Yibo Wang** · [project thread](https://claude.ai/code/project/chan_01BVUyyPie8gHwoPM18iABwB?thread=cmsg_01BVUyyPie8gHwoPM18iABwBFipqCKL82rffBrxJjboxqk)_ Fixes #30256 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:三个扫描整个仓库的测试,读文件时不再因为 tsup 构建临时写入又删掉的 `tsup.config.bundled_*.mjs` 而报 ENOENT;只改测试文件,不改产品代码。 - 为什么改:`strict-authoring-face-8345` 在合并队列里与并行分片的 tsup 构建竞态(队列构建 41962914067,`Test UI (4/16)`),把无关 PR 踢出队列。 - 风险与回滚:只容忍 ENOENT(读之前文件消失);其他读错误照样抛出,`scanned > 1000` 和唯一导入者断言一字未改。回滚即还原本 PR。 - 先看:`strict-authoring-face-8345.test.ts` 中提取出的 `scanForImporters` / `readIfPresent` 和新增的竞态钉子测试。 - 两分钟验证:`cd ui && pnpm exec vitest run packages/types/src/__tests__/strict-authoring-face-8345.test.ts`,预期 33 例全绿。 - 需要你做的:无,席位审阅并落地。 ## What changes - **`ui/packages/types/src/__tests__/strict-authoring-face-8345.test.ts`** — the importer walk is extracted to `scanForImporters(repoRoot, roots, list?)` inside the same `describe`. Two guards, placed as #20225 / #20269 placed them: - a file named `*.bundled_<hash>.mjs` (tsup's transient bundle of `tsup.config.ts`) is skipped by name; - `readIfPresent` returns `undefined` on **ENOENT only** and the file is not counted; every other read fault re-throws. - `scanned` now counts files actually read (it used to count before the read). The `scanned > 1000` floor and the `['packages/types/src/zod/index.zod.ts']` importer assertion are unchanged. - **New race pin** in the same file drives the extracted walker over a temp directory with a doctored listing: a vanished `tsup.config.bundled_x.mjs` and a vanished `vanished.ts` → no throw, `scanned: 1`, only `pkg/real.ts` found; an on-disk `tsup.config.bundled_y.mjs` carrying a specifier → skipped by name; a directory listed as a file → still throws `EISDIR`. The fixture's import line is split so this file never reads as an importer itself. - **Sweep, same race, fixed here** (walk reaches `ui/packages/cli`, a tsup package root, and reads `.mjs`): - `ui/packages/app-shell/src/views/metadata-admin/previews/__tests__/block-config-i18n.test.ts` (`shippedLiteralKeys`, walks `packages` + `apps`, ext `(ts|tsx|js|jsx|mjs)`). - `ui/packages/app-shell/src/__tests__/select-placeholder-literal-11252.test.ts` (walks `packages` + `apps`, `SOURCE_FILE` includes `mjs`; `filesRead` now counts files actually read). - Each got the same two guards inline. No shared helper: the three sites live in two packages and a cross-package test import would add a dependency edge for ten lines. ## Why tsup bundles each package's `tsup.config.ts` to `tsup.config.bundled_<hash>.mjs` beside it and deletes it when the build ends. A walk that lists the directory during a concurrent build can name that file and then fail to read it. `readdirSync` was already guarded; the per-file `readFileSync` was not. ## Verification - `cd ui && pnpm exec vitest run` on the three touched files: 3 files, 58 tests passed (under `os-verify-lock`, exit 0). - **CONTROL**: the importer test still finds exactly `packages/types/src/zod/index.zod.ts`. - **Ablations** on the race pin (each restored byte-identical after): 1. ENOENT guard removed (`throw err` unconditionally) → pin red. 2. Name skip removed → pin red (on-disk bundle read as an importer). 3. Guard widened to every error → pin red (`EISDIR` no longer thrown). - Sweep findings not fixed (read only; none reaches a tsup package root with an `.mjs` read): `machineLocaleCensus-9909` reads `[cm]?[jt]sx?` at package roots, but its population (packages depending on `@object-ui/i18n`) excludes `cli`, `create-plugin`, `data-objectstack`; `page-node-headings-drawn-11436` walks `examples` only (no tsup there); `sidebarGroupLabel.callSites-12795` (`tsx?|jsx?|mts`, no `.mjs`); `product-neutral-renderer.ratchet` lists paths but does not read them; the rest walk `src/` only or filter to `.ts`/`.tsx`/`.json`. - `dispatch-gates --commands` (26 commands) run on the final diff: see `--ran` record below. <details><summary>dispatch-gates --ran record (26 derived, 26 run, 0 NOT-MEASURED, 0 UNRUN)</summary> ``` node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-sdui-manifest.mjs :: exit 0 node scripts/check-sdui-manifest.mjs --self-test :: exit 0 pnpm --dir ui exec vitest run scripts/ :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:registry-log-declared :: exit 0 pnpm check:rest-log-spy-declared :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 ``` </details> - Whole-package vitest under `os-verify-lock`: `packages/types/` 387 files / 10,464 tests passed; `packages/app-shell/` 2,176 files passed, 1 skipped (pre-existing) / 19,426 tests passed. - `eslint` on the three files: exit 0. `tsc -p ui/packages/types/tsconfig.test.json --noEmit`: exit 0. ## Human Minimum - [x] 1. Schemas start as Zod; `packages/spec` holds no runtime logic. (not touched) - [x] 2. Every name I added uses the casing for its kind. - [x] 3. No formatter ran; eslint on the touched files passes (full `pnpm lint` not run locally). - [x] 4. One issue, one branch off an up-to-date `main`, one pull request. - [x] 5. A change that publishes carries a changeset — this diff is test-only and publishes nothing (`skip-changeset`); both declaration lines are filled in above. - [x] 6. `packages/spec` touched: n/a. - [x] 7. No hand edits to generated references, changelogs or release notes. - [x] 8. Title, body and comments are in English (the 维护者速读 block excepted, per project convention). - [x] The *Before Submitting* list in CONTRIBUTING.md is done. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01WGufziNut2sBjutK3Sers8 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5a878a3 commit dbf53ef

3 files changed

Lines changed: 108 additions & 12 deletions

File tree

‎ui/packages/app-shell/src/__tests__/select-placeholder-literal-11252.test.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,16 @@ const SKIP_DIRS = new Set([
7373
'__tests__', '__mocks__', 'test', 'tests', 'e2e',
7474
]);
7575
const SOURCE_FILE = /\.(?:tsx?|jsx?|mts|cts|mjs|cjs)$/;
76+
const TSUP_BUNDLED_CONFIG = /\.bundled_[^./]+\.mjs$/;
77+
/** A file that vanished between list and read (ENOENT only) is not read; every other fault throws. */
78+
const readIfPresent = (full: string): string | undefined => {
79+
try {
80+
return readFileSync(full, 'utf8');
81+
} catch (err) {
82+
if ((err as NodeJS.ErrnoException)?.code !== 'ENOENT') throw err;
83+
return undefined;
84+
}
85+
};
7686
const NOT_PRODUCT = /\.d\.ts$|\.(?:test|spec|stories)\.[cm]?[jt]sx?$/;
7787
const PACK_DIR = 'packages/i18n/src/locales/';
7888

@@ -148,9 +158,14 @@ function scanTree(): { filesRead: number; hits: Hit[] } {
148158
continue;
149159
}
150160
if (!SOURCE_FILE.test(entry.name) || NOT_PRODUCT.test(entry.name)) continue;
161+
// tsup's transient bundle of a package's `tsup.config.ts`, written and
162+
// deleted mid-build by a concurrent shard (#30256): skipped by name.
163+
if (TSUP_BUNDLED_CONFIG.test(entry.name)) continue;
164+
const text = readIfPresent(full);
165+
if (text === undefined) continue;
151166
filesRead += 1;
152167
const rel = relative(REPO_ROOT, full).split(sep).join('/');
153-
hits.push(...scanSource(rel, readFileSync(full, 'utf8')));
168+
hits.push(...scanSource(rel, text));
154169
}
155170
};
156171
for (const root of SCAN_ROOTS) {

‎ui/packages/app-shell/src/views/metadata-admin/previews/__tests__/block-config-i18n.test.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -626,9 +626,20 @@ function shippedLiteralKeys(): Map<string, string[]> {
626626
}
627627
if (!/\.(ts|tsx|js|jsx|mjs)$/.test(entry.name)) continue;
628628
if (/\.(test|spec)\.[tj]sx?$/.test(entry.name)) continue;
629+
// tsup's transient bundle of a package's `tsup.config.ts`, written and
630+
// deleted mid-build by a concurrent shard (#30256): skipped by name.
631+
if (/\.bundled_[^./]+\.mjs$/.test(entry.name)) continue;
629632
if (full === I18N_SOURCE_PATH) continue; // the DEFINITION site, not a reader
630633
if (full === STUDIO_HOME_PATH.en || full === STUDIO_HOME_PATH.zh) continue; // likewise
631-
const content = readFileSync(full, 'utf8');
634+
// A file that vanished between list and read is not a reader; any other
635+
// read fault still fails the walk.
636+
let content: string;
637+
try {
638+
content = readFileSync(full, 'utf8');
639+
} catch (err) {
640+
if ((err as NodeJS.ErrnoException)?.code === 'ENOENT') continue;
641+
throw err;
642+
}
632643
if (!content.includes(PREFIX)) continue;
633644
for (const m of content.matchAll(/['"`](engine\.inspector\.pageBlock\.[A-Za-z0-9_.:$-]+)['"`]/g)) {
634645
const rel = path.relative(ROOT, full).split('\\').join('/');

‎ui/packages/types/src/__tests__/strict-authoring-face-8345.test.ts‎

Lines changed: 80 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@
4444
* script) and by nothing else — a green vitest run says nothing about them.
4545
*/
4646

47-
import { readdirSync, readFileSync } from 'node:fs';
47+
import { mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
48+
import { tmpdir } from 'node:os';
4849
import { dirname, join } from 'node:path';
4950
import { fileURLToPath } from 'node:url';
5051

@@ -532,28 +533,97 @@ describe('the barrel is the sole entry into the module cycle', () => {
532533
).toEqual([]);
533534
});
534535

535-
it('no module in this repository imports the deep module except the barrel', () => {
536-
// A specifier, not a mention: `scripts/measure-strict-authoring-face.mjs`
537-
// shares the words in its own name and must not read as an importer.
538-
const REPO_ROOT = join(PACKAGE_ROOT, '..', '..');
536+
/**
537+
* The repository walk is a read of a tree OTHER processes are writing. tsup
538+
* bundles each package's `tsup.config.ts` to a transient
539+
* `tsup.config.bundled_<hash>.mjs` next to it and deletes it when the build
540+
* ends, so a listing taken during a concurrent build can name a file that is
541+
* gone by the time it is read (#30256: `Test UI (4/16)`, ENOENT). Two guards,
542+
* as #20225 / #20269 placed them: the transient bundle is skipped by name, and
543+
* a path that vanished between list and read (ENOENT only) is not counted.
544+
* Every other read fault still throws — a guard wider than ENOENT would let
545+
* an unreadable importer pass as an absent one.
546+
*/
547+
const TSUP_BUNDLED_CONFIG = /\.bundled_[^./]+\.mjs$/;
548+
const SPECIFIER = /(?:from|import|require)\s*\(?\s*['"][^'"]*strict-authoring-face[^'"]*['"]/;
549+
const readIfPresent = (full: string): string | undefined => {
550+
try {
551+
return readFileSync(full, 'utf8');
552+
} catch (err) {
553+
if ((err as NodeJS.ErrnoException)?.code !== 'ENOENT') throw err;
554+
return undefined;
555+
}
556+
};
557+
type Listing = (dir: string) => import('node:fs').Dirent[];
558+
const listDir: Listing = (dir) => readdirSync(dir, { withFileTypes: true });
559+
const scanForImporters = (
560+
repoRoot: string,
561+
roots: readonly string[],
562+
list: Listing = listDir,
563+
): { importers: string[]; scanned: number } => {
539564
const SKIP = new Set(['node_modules', 'dist', '.git', '.turbo', 'coverage', '.next', 'build', 'test-results']);
540565
const SOURCE = /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/;
541-
const SPECIFIER = /(?:from|import|require)\s*\(?\s*['"][^'"]*strict-authoring-face[^'"]*['"]/;
542566
const importers: string[] = [];
543567
let scanned = 0;
544568
const walk = (dir: string): void => {
545569
let entries: import('node:fs').Dirent[];
546-
try { entries = readdirSync(dir, { withFileTypes: true }); } catch { return; }
570+
try { entries = list(dir); } catch { return; }
547571
for (const entry of entries) {
548572
if (SKIP.has(entry.name)) continue;
549573
const full = join(dir, entry.name);
550574
if (entry.isDirectory()) { walk(full); continue; }
551-
if (!SOURCE.test(entry.name)) continue;
575+
if (!SOURCE.test(entry.name) || TSUP_BUNDLED_CONFIG.test(entry.name)) continue;
576+
const text = readIfPresent(full);
577+
if (text === undefined) continue;
552578
scanned += 1;
553-
if (SPECIFIER.test(readFileSync(full, 'utf8'))) importers.push(full.slice(REPO_ROOT.length + 1));
579+
if (SPECIFIER.test(text)) importers.push(full.slice(repoRoot.length + 1));
554580
}
555581
};
556-
for (const root of ['packages', 'apps', 'examples', 'scripts', 'e2e']) walk(join(REPO_ROOT, root));
582+
for (const root of roots) walk(join(repoRoot, root));
583+
return { importers, scanned };
584+
};
585+
586+
it('the repository walk survives a file that vanishes mid-walk, and still throws on any other read fault', () => {
587+
// A real directory whose listing is doctored: it names a transient tsup
588+
// bundle and an ordinary module, neither of which is on disk at read time.
589+
// The fixture specifier is split so this file never reads as an importer.
590+
const IMPORT_LINE = "import { x } from './strict-authoring" + "-face';\n";
591+
const root = mkdtempSync(join(tmpdir(), 'saf-8345-walk-'));
592+
try {
593+
mkdirSync(join(root, 'pkg'));
594+
writeFileSync(join(root, 'pkg', 'real.ts'), IMPORT_LINE);
595+
const ghost = (name: string): import('node:fs').Dirent =>
596+
({ name, isDirectory: () => false }) as unknown as import('node:fs').Dirent;
597+
const withGhosts: Listing = (dir) =>
598+
dir.endsWith('pkg') ? [...listDir(dir), ghost('tsup.config.bundled_x.mjs'), ghost('vanished.ts')] : listDir(dir);
599+
600+
const result = scanForImporters(root, ['pkg'], withGhosts);
601+
expect(result, 'only the file that exists is counted, and it is still found').toEqual({
602+
importers: ['pkg/real.ts'],
603+
scanned: 1,
604+
});
605+
606+
// The transient bundle is skipped by NAME even when it is on disk and
607+
// carries a specifier — it is a build by-product, never an importer.
608+
writeFileSync(join(root, 'pkg', 'tsup.config.bundled_y.mjs'), IMPORT_LINE);
609+
expect(scanForImporters(root, ['pkg']).importers).toEqual(['pkg/real.ts']);
610+
611+
// Any fault other than ENOENT still fails the walk: a directory listed as
612+
// a file reads as EISDIR.
613+
mkdirSync(join(root, 'pkg', 'odd.ts'));
614+
const dirAsFile: Listing = (dir) =>
615+
dir.endsWith('pkg') ? listDir(dir).map((e) => (e.name === 'odd.ts' ? ghost('odd.ts') : e)) : listDir(dir);
616+
expect(() => scanForImporters(root, ['pkg'], dirAsFile)).toThrow(/EISDIR/);
617+
} finally {
618+
rmSync(root, { recursive: true, force: true });
619+
}
620+
});
621+
622+
it('no module in this repository imports the deep module except the barrel', () => {
623+
// A specifier, not a mention: `scripts/measure-strict-authoring-face.mjs`
624+
// shares the words in its own name and must not read as an importer.
625+
const REPO_ROOT = join(PACKAGE_ROOT, '..', '..');
626+
const { importers, scanned } = scanForImporters(REPO_ROOT, ['packages', 'apps', 'examples', 'scripts', 'e2e']);
557627

558628
expect(scanned, 'the scan found almost no source files — it is pointed at the wrong tree').toBeGreaterThan(1000);
559629
expect(importers.sort(), 'something other than the barrel now enters the cycle at the deep module')

0 commit comments

Comments
 (0)