diff --git a/packages/utils/src/import.ts b/packages/utils/src/import.ts index 8313cfbf2c..4bed1dedb3 100644 --- a/packages/utils/src/import.ts +++ b/packages/utils/src/import.ts @@ -55,6 +55,12 @@ try { // If import.meta is not available, it's likely CJS isESM = false; } +// Remember the auto-detected module format. `setSnapshotModuleLoader` flips +// `isESM` to false while a snapshot loader is active; clearing the loader must +// restore this value so consumers running later in the same realm are not left +// stuck in CJS mode. This matters under vitest `isolate: false`, where module +// state persists across test files. +const detectedIsESM = isESM; const nodeMajorVersion = parseInt(process.versions.node.split('.', 1)[0], 10); const supportImportMetaResolve = nodeMajorVersion >= 18; @@ -415,10 +421,15 @@ let _snapshotModuleLoader: SnapshotModuleLoader | undefined; * * Also sets `isESM = false` because the snapshot bundle is CJS and * esbuild's `import.meta` polyfill causes incorrect ESM detection. + * + * Pass `undefined` to clear the loader and restore the auto-detected `isESM` + * value. Always clear it once snapshot mode is no longer needed (e.g. in test + * teardown) so the module-level state does not leak into other files when + * vitest runs with `isolate: false`. */ -export function setSnapshotModuleLoader(loader: SnapshotModuleLoader): void { +export function setSnapshotModuleLoader(loader: SnapshotModuleLoader | undefined): void { _snapshotModuleLoader = loader; - isESM = false; + isESM = loader ? false : detectedIsESM; } export type { BundleModuleLoader } from '@eggjs/typings'; diff --git a/packages/utils/test/snapshot-import.test.ts b/packages/utils/test/snapshot-import.test.ts index 5a8eec7889..b1b5e83595 100644 --- a/packages/utils/test/snapshot-import.test.ts +++ b/packages/utils/test/snapshot-import.test.ts @@ -7,18 +7,12 @@ import { getFilepath } from './helper.ts'; describe('test/snapshot-import.test.ts', () => { describe('setSnapshotModuleLoader', () => { - // We need to capture and restore isESM since setSnapshotModuleLoader mutates it. - // Use dynamic import to read the current value. - afterEach(async () => { - // Reset the snapshot loader by setting it to a no-op then clearing via - // module internals. Since there's no public "unset" API, we re-import - // and the module-level _snapshotModuleLoader remains set — but tests - // are isolated enough that this is fine. We'll use a different approach: - // just call setSnapshotModuleLoader with a passthrough that calls the - // real import, but that changes isESM. Instead, we accept that these - // tests run with the loader set and each test overrides it. - // Reset by overwriting with undefined via the setter trick: - // Actually we can't unset. Let's just re-import fresh for isolation. + // setSnapshotModuleLoader mutates module-level state (_snapshotModuleLoader + // and isESM). Clear it after each test, otherwise the state leaks into every + // other test file sharing this realm when vitest runs with `isolate: false`, + // breaking their module resolution ("Can not find plugin ..."). + afterEach(() => { + setSnapshotModuleLoader(undefined); }); it('should intercept importModule with registered loader', async () => { diff --git a/plugins/mock/src/app/extend/application.ts b/plugins/mock/src/app/extend/application.ts index 1ae1351f88..d3900c2d88 100644 --- a/plugins/mock/src/app/extend/application.ts +++ b/plugins/mock/src/app/extend/application.ts @@ -100,7 +100,12 @@ export default abstract class ApplicationUnittest extends Application { const res = new http.ServerResponse(req); if (options.reuseCtxStorage !== false) { - if (this.currentContext && !this.currentContext[REUSED_CTX]) { + // Only reuse the active async-local context when it actually belongs to + // this app. The context storage can be shared across app instances (e.g. + // multiple `mm.app()` apps in one realm, or vitest `isolate: false` where + // a previous app's context still lingers); reusing a foreign app's + // context would bind helpers/services to the wrong app config. + if (this.currentContext && this.currentContext.app === this && !this.currentContext[REUSED_CTX]) { mockRequest(this.currentContext.request.req); this.currentContext[REUSED_CTX] = true; return this.currentContext as MockContext; diff --git a/wiki/concepts/vitest-isolate-false-state-leaks.md b/wiki/concepts/vitest-isolate-false-state-leaks.md new file mode 100644 index 0000000000..4237599d54 --- /dev/null +++ b/wiki/concepts/vitest-isolate-false-state-leaks.md @@ -0,0 +1,93 @@ +--- +title: Vitest isolate:false state leaks +type: concept +summary: Why the root vitest config (pool:threads + isolate:false) exposes cross-file/cross-project state leaks, the concrete leaks found, and how they were fixed. +source_files: + - vitest.config.ts + - packages/utils/src/import.ts + - packages/utils/test/snapshot-import.test.ts + - plugins/mock/src/app/extend/application.ts + - plugins/mock/src/lib/mock_agent.ts + - plugins/multipart/test/file-mode.test.ts +updated_at: 2026-06-08 +status: active +--- + +## Context + +The root `vitest.config.ts` runs the whole monorepo with `pool: 'threads'` and +`isolate: false`. Under this mode every test **file** in a worker shares one +Node realm: the module registry, `globalThis`, module-level `let` bindings, the +undici global dispatcher, `process` env/listeners and timers are all shared +across files (and across `projects`, since projects share the worker pool). + +This is much faster, but any module that caches process- or realm-global state +without resetting it leaks that state into later files. Because vitest schedules +files across threads, _which_ test loses is order/timing dependent — so failures +are **nondeterministic** and move from run to run. That nondeterminism is the +signature of this class of bug, not flaky tests per se. + +## How to reproduce / triage (CI-faithful) + +- Use Node 22 or 24 (CI matrix). Node 26 introduces unrelated undici/deprecation + failures that are NOT isolate bugs — do not diagnose on Node 26. +- Install with utoo (`ut install --from pnpm`), not a bare `pnpm install`. utoo + hoists workspace packages (e.g. `egg`) to the root `node_modules`; tests like + `cluster/options` and `mock/format_options` resolve the framework via + `getFrameworkPath('egg')` from a fixture `baseDir` and only pass with that + hoisting. A non-utoo install causes phantom "egg is not found" / + "Cannot find module" failures that masquerade as isolate bugs. +- Clear `dist/` first (see AGENTS.md "Local CI" / duplicate-proto note). +- Triage rule: run a failing file **alone**. Passes alone but fails in the full + run ⇒ genuine cross-file leak. Fails alone too ⇒ a real bug or an + environmental dependency (MySQL/redis/DNS), not isolation. + +## Root causes found + +1. **Module-resolution poisoning via `@eggjs/utils` import.ts** (cross-project). + `setSnapshotModuleLoader()` set a module-level `_snapshotModuleLoader` and + flipped the module-level `isESM` to `false`, with no way to unset it. + `snapshot-import.test.ts` had a no-op `afterEach`, so after it ran, every + later file in the worker resolved modules in CJS + snapshot mode and failed + with `Can not find plugin @eggjs/` / `Cannot find module +'@eggjs//package.json'`. This single leak caused most of the cross-project + failures (ajv-plugin, typebox-validate, view-nunjucks, standalone, …). + **Fix:** `setSnapshotModuleLoader(undefined)` now clears the loader and + restores the auto-detected `isESM`; the test clears it in `afterEach`. + (`setBundleModuleLoader` and the tegg/core loader tests already reset their + `globalThis.__EGG_BUNDLE_MODULE_LOADER__` in `afterEach`, so they were fine.) + +2. **Wrong-app context reuse in `@eggjs/mock` `mockContext()`.** + `mockContext()` reused `this.currentContext` without checking the context + belongs to `this` app. With a shared/lingering async-local context (multiple + `mm.app()` apps in one realm, or `isolate:false` carry-over), `app2.mockContext()` + returned app1's context, binding helpers/services to the wrong app config. + This produced e.g. `security/surl` "custom white protocol" → `''` (app2's + helper read app1's whitelist) and `security/csrf` 401s. + **Fix:** only reuse when `this.currentContext.app === this`. + +3. **Potential undici global dispatcher carry-over in `@eggjs/mock` + `mock_agent.ts`.** Dispatcher state is stored on `globalThis` + (`__globalDispatcher`, `__mockAgent`) and `__globalDispatcher` is captured + once and never cleared. Mitigated in practice by the global + `afterEach(mock.restore)` in `setup_vitest.ts`; noted as a latent risk. + +## Not isolate bugs (do not chase as such) + +- `orm-plugin` (`Table 'test.apps' doesn't exist`) needs MySQL; `redis` needs a + redis server; `security/ssrf` (`IllegalAddressError: illegal address`) needs + DNS/network. These pass in CI where the services exist; they fail in a bare + local run regardless of isolation. +- `multipart/file-mode` is **load-sensitive and flakes even with `isolate:true`** + (fails ~1/4 when run alongside the other multipart files). Deep tracing showed + the uploaded file is reported (200 + path) but `existsSync` is already false + right after the save `pipeline()` resolves under load — a race in the multipart + file-save path, not a state leak. It passes 100% as a single file. Treat as a + pre-existing flaky test to fix in the multipart save path or test, separately + from the isolate:false work. + +## Result + +Full Node-22 suite under `isolate:false`: 15 failing files → 3, of which 2 are +environmental (MySQL/DNS, green in CI) and 1 (`multipart/file-mode`) is a +pre-existing load flake independent of isolation. diff --git a/wiki/index.md b/wiki/index.md index a9449c7a06..d70f99296a 100644 --- a/wiki/index.md +++ b/wiki/index.md @@ -7,6 +7,7 @@ Read this file before exploring raw sources. ## Concepts - [Repository Map](./concepts/repository-map.md) - High-level map of the main repository areas and where to look first. +- [Vitest isolate:false state leaks](./concepts/vitest-isolate-false-state-leaks.md) - Why pool:threads + isolate:false exposes cross-file/cross-project state leaks, the concrete leaks (import.ts snapshot loader, mock mockContext), and how to triage them. ## Workflows diff --git a/wiki/log.md b/wiki/log.md index cb4ae3398b..df46fac1c7 100644 --- a/wiki/log.md +++ b/wiki/log.md @@ -2,6 +2,12 @@ Dates use the workspace-local Asia/Shanghai calendar date. +## [2026-06-08] concept | vitest isolate:false state leaks diagnosed and fixed + +- sources touched: `packages/utils/src/import.ts`, `packages/utils/test/snapshot-import.test.ts`, `plugins/mock/src/app/extend/application.ts` +- pages updated: `wiki/index.md`, `wiki/log.md`, `wiki/concepts/vitest-isolate-false-state-leaks.md` +- note: Under root `pool:threads` + `isolate:false`, two realm-global leaks caused nondeterministic cross-file/cross-project failures. (1) `setSnapshotModuleLoader` left module-level `_snapshotModuleLoader`/`isESM=false` set (no-op test teardown), poisoning module resolution for later files (`Can not find plugin …`). (2) `mock.mockContext()` reused `currentContext` from a different app, binding helpers to the wrong app config (surl/csrf failures). Fixed both at the source. Full Node-22 suite: 15 → 3 failing files (remaining 2 environmental MySQL/DNS; `multipart/file-mode` is a pre-existing load flake that also fails under `isolate:true`). Reproduce on Node 22/24 with a utoo install — not Node 26 / bare pnpm. + ## [2026-05-10] package | extract shared LoaderFS package - sources touched: `packages/loader-fs/src/index.ts`, `packages/loader-fs/package.json`, `packages/core/src/index.ts`, `packages/core/src/loader/file_loader.ts`, `packages/core/src/loader/egg_loader.ts`