feat(tegg): isolate per-app state for concurrent multi-app via TeggScope#5986
Merged
Conversation
Allow multiple tegg apps to boot and serve requests concurrently in one process without cross-talk. Introduce a low-level, type-free `TeggScope` (AsyncLocalStorage<Map<symbol, unknown>>) in @eggjs/tegg-types. Each package backs its process-global statics with a per-app scope slot + fallback, so the static facade call sites stay unchanged; boot hooks, request middleware, unittest scope, standalone Runner, and escape points wrap work in `TeggScope.run(app._teggScopeBag, ...)`. Core - TeggScope keystone: current()/run()/createBag()/registerScope() + a single unified process-default bag for all no-scope fallback, plus a strict-mode escape fuse (dev throw / prod warn) that arms only under true multi-app (explicitScopeCount > 1); single-app stays silent. - metadata: EggPrototypeFactory.instance, GlobalGraph.instance, LoadUnitFactory maps (two-tier creator map: shared base for import-time creators + per-app overlay for boot-time app-capturing creators), per-app class->proto map (getPrototypeByClazz) to fix concurrent getEggObject(clazz) resolution. - runtime: LoadUnitInstanceFactory.instanceMap, EggObjectFactory.eggObjectMap, ContextHandler callbacks, all five lifecycle utils become per-app via a createScopedLifecycleUtil proxy in @eggjs/lifecycle. - common-util: ModuleConfigUtil.configNames per-app (fixes the standalone config-names race). Plugins / standalone - plugin/tegg: build per-app bag in configWillLoad, wrap boot/request/unittest + getEggObject + app.module/ctx.module proxies; per-app CompatibleUtil caches. - controller/aop/dal/eventbus/orm/schedule/langchain: wrap lifecycle-hook registration in the app scope (per-app lifecycle utils require it). - dal: per-app TableModel/MysqlDataSource/SqlMap managers; app extend getter pins to the app scope. - eventbus: SingletonEventBus captures its app bag and re-establishes it in doEmit (detached fire-and-forget dispatch isolation). - controller MCP + mcp-proxy: per-app register/hooks; transport captures its register; addHook runs in scope. - standalone Runner: per-Runner bag wrapping load/init/run/destroy. Tests - New plugin/tegg MultiApp.test.ts: two concurrent apps sharing one module — isolated singletons, per-app data store, EventBus emit/handler dispatch, background tasks, and sequential no-leak, all under strict mode. - Update additive export-stable snapshots. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds ChangesMulti-App Isolation and Scoped Runtime
Sequence Diagram(s)sequenceDiagram
participant TeggAppBoot
participant TeggScope
participant TEggPluginApplication
participant EggContainerFactory
participant EggPrototypeFactory
TeggAppBoot->>TeggScope: registerScope(app._teggScopeBag)
TeggAppBoot->>TeggScope: run(app._teggScopeBag, lifecycle work)
TEggPluginApplication->>TeggScope: runMaybe(app._teggScopeBag, doWork)
TeggScope->>EggContainerFactory: execute doWork in app bag
EggContainerFactory->>EggPrototypeFactory: getPrototypeByClazzOrGlobal(clazz)
EggPrototypeFactory-->>EggContainerFactory: scoped prototype
EggContainerFactory-->>TEggPluginApplication: egg object
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #5986 +/- ##
==========================================
+ Coverage 84.89% 84.92% +0.03%
==========================================
Files 669 676 +7
Lines 19942 20513 +571
Branches 3964 4058 +94
==========================================
+ Hits 16929 17420 +491
- Misses 2588 2664 +76
- Partials 425 429 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces TeggScope to provide per-app isolation for singletons, caches, and registries, resolving concurrency issues in multi-app environments where process-global statics would otherwise conflict. Key changes include wrapping lifecycle utilities in a scope-backed proxy via createScopedLifecycleUtil, refactoring core factories and registers to resolve instances from the active TeggScope bag, and updating plugins (AOP, Controller, DAL, EventBus, LangChain, ORM, Schedule) to execute boot and request-handling flows within the appropriate app scope. The review feedback highlights several opportunities to refine this implementation, such as caching bound methods in the lifecycle utility proxy to preserve referential equality, adding escape checks to TeggScope.set under multi-app mode, removing redundant legacy fallbacks in static setters, and defensively checking for the scope bag in context extensions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Add a "Multi-App Isolation (TeggScope)" section to tegg/CLAUDE.md with the rules agents/devs must follow when editing tegg core/plugins: no new process-global mutable runtime state (back per-app state with a TeggScope slot), wrap plugin lifecycle-hook registration in TeggScope.run(app._teggScopeBag, ...), resolve egg objects per-app, handle detached escape points, and the strict-mode fuse. Also note the (negligible) performance characteristics. Add a concise pointer rule under Coding Conventions in AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reduce the per-app isolation overhead and the boilerplate `TeggScope.run(...)` wrapping, following review feedback. Convergence (less defensive boilerplate): - `app.*LifecycleUtil` getters are now PINNED to the app's scope bag via new `xxxLifecycleUtilFromBag(bag)` helpers, so plugin boot hooks register lifecycle hooks WITHOUT wrapping each call in `TeggScope.run`. Removed the run wraps from aop/dal/eventbus/orm/schedule/langchain boot hooks (kept only where a scope is genuinely required: moduleHandler.init/destroy, sdk-base construction, leoricRegister, registerLoadUnitCreator overlay, request/getEggObject paths, DAL manager teardown, standalone Runner). - `GlobalGraph.instanceFor(bag)` lets aop/controller register build hooks pinned to the owning app's graph (no run wrap). Performance: - lifecycle utils: replace the per-access Proxy facade with an explicit delegating object (no get-trap / bound-function allocation): ~27ns -> ~8ns per lifecycle-util call. - `TeggScope.resolve`/`getOr`: single `getStore()` per call; the in-scope common path never reaches the escape check, and single-app never escapes. Correctness (latent multi-app bugs exposed by the convergence): - Resolve egg objects by class via the per-app `EggPrototypeFactory.getPrototypeByClazz` (falling back to the global `PrototypeUtil.getClazzProto`) in EggContextEventBus, dynamic-inject EggObjectFactory and aop LoadUnitAopHook — the global class->proto map is shared and overwritten by concurrent apps. Docs: document the TeggScope constraints + performance notes in tegg/CLAUDE.md (added previously) still apply. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
# Conflicts: # tegg/plugin/aop/src/app.ts # tegg/plugin/controller/src/lib/impl/mcp/MCPControllerRegister.ts
The aop and eventbus plugins declared @eggjs/tegg-types in dependencies but never import it (aop resolves the per-app graph via app._teggScopeBag and GlobalGraph from @eggjs/metadata; eventbus scopes via app.getEggObject / eggPrototypeFactory.getPrototypeByClazz). tsdown's unplugin-unused fails the build with 'Unused 1 dependencies found', which broke the cnpmcore/examples E2E build step. Remove the dead dependency. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per-app state (factories, GlobalGraph, the ContextHandler request-context callbacks, …) is installed into app._teggScopeBag at boot. But code that runs OUTSIDE an explicit TeggScope.run — a SingletonProto method called directly, a detached leoric SQL logger, a ContextProto method invoked in a test body — resolved the SEPARATE process-default bag, which never held that state. Boot state was therefore invisible to single-app call sites that did not go through a wrapped entry point. Pre-scoping these were process-global statics, so they were reachable from anywhere after boot. Restore that for single-app while keeping multi-app isolation: when EXACTLY ONE app is alive, the no-scope fallback now resolves to that app's bag (TeggScope tracks live scope bags in a Set instead of a bare counter). Zero apps (pure core unit tests / standalone helpers) or true multi-app keep using the process-default bag, and multi-app out-of-scope access still trips the escape fuse. Also make ContextHandler.getContext() return undefined instead of asserting when no callback is installed: 'no active context' is a valid read result (e.g. a singleton service or detached logger called with no request in flight). Multi-app escape detection is handled by the TeggScope fuse, not this assert. Fixes the @eggjs/orm-plugin regressions where leoric's SQL logger threw 'getContextCallback not set' (swallowed by leoric) so the SQL line was never logged, and the context-proto tests saw an undefined ctx. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- TeggScope.set: report an escape when it writes to the process-default bag under multi-app with no active scope, matching resolve()/getOr() so a missed scope wrap that mutates shared state is caught. - GlobalGraph / HTTPControllerRegister / MCPControllerRegister: drop the dead #legacyInstance fallback. TeggScope.set always returns true and writes to the active/sole-app/default bag, so the 'if (!set(...)) #legacyInstance = value' branch never ran and the getter's legacy() was always undefined. The bag is the single source of truth; simplify the getter/setter accordingly. - context.ts getEggObject/getEggObjectFromName: guard app._teggScopeBag before TeggScope.run, consistent with application.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add unit coverage for the multi-app scope primitive: run/current, live-scope tracking + isMultiApp, resolve memoization, the no-scope fallbacks (zero apps -> process-default bag; exactly one app -> that app's bag so out-of-scope access sees boot state), getOr legacy fallback, set, and the strict-mode escape fuse that throws on out-of-scope access under multi-app. Regression coverage for the single-app-state fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Behavior-preserving simplifications across the multi-app PR (all verified by the
full tegg suite):
- TeggScope: add runMaybe(bag, fn) and route the 8 copy-pasted
`bag ? run(bag, fn) : fn()` sites through it; inline the single-caller
#activeBag into set() (one getStore, mirrors resolve/getOr); drop the unused
_setDefaultBag.
- EggPrototypeFactory.getPrototypeByClazzOrGlobal: centralize the rule-4
`getPrototypeByClazz(clazz) ?? PrototypeUtil.getClazzProto(clazz)` fallback so
the 4 call sites (aop-runtime, dynamic-inject-runtime, runtime, eventbus) stop
re-implementing it; drop the now-unused PrototypeUtil imports/casts.
- lifecycle: add defineScopedLifecycleUtil(slot, desc) returning [util, fromBag]
and collapse the repeated declare-slot + createScopedLifecycleUtil +
lifecycleUtilFromBag boilerplate in the 5 model files.
- EggPrototypeFactory: use the canonical EggPrototypeWithClazz cast instead of an
ad-hoc `as unknown as { clazz }`.
- controller: drop the dead willReady MCP block (connectStatelessStreamTransport
is a no-op; transports are per-request now) and its scope wrap; remove the
redundant ControllerMetadataManager constructor and a self-nested csrf
Array.isArray re-test.
- standalone Runner: one uniform runInScope() helper across all 5 scope entries.
- orm/mcp-proxy: collapse trivial single-statement scope-run closures.
Net -56 lines; no public API or runtime behavior change.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three real correctness bugs from an adversarial review of the multi-app PR: - langchain: GraphLoadUnitHook captured app.eggPrototypeFactory in the boot CONSTRUCTOR (runs before any app scope exists), so under concurrent multi-app it resolved another live app's factory (cross-app proto pollution) or tripped the strict-mode escape fuse on a 3rd app's boot. Resolve EggPrototypeFactory .instance lazily inside preCreate, which runs within this app's TeggScope. - standalone Runner: TeggScope.registerScope ran in the constructor but unregisterScope only on destroy()'s success path, leaking the scope into liveScopeBags on construction/init/destroy failure — a single leak flips isMultiApp on and breaks the sole-app fallback + escape fuse for every later Runner. Now: destroy() unregisters in a finally, the constructor unregisters on failure, and main() tears the Runner down when init() throws. - plugin/tegg beforeClose: moved unregisterScope into a finally so a throwing moduleHandler.destroy() no longer leaks the app scope. All verified by the tegg suite (langchain/standalone/tegg + core/plugins green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ations) `export const [Util, utilFromBag] = defineScopedLifecycleUtil(...)` (a binding pattern) is rejected by the repo's --isolatedDeclarations d.ts emit (TS9019), breaking the build + both E2E suites. Restore the explicit `export const Util: LifecycleUtil<...> = createScopedLifecycleUtil(...)` + `export function utilFromBag(...)` form in the 5 model files and drop the unused defineScopedLifecycleUtil helper. The other simplifications are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
teggConfig boots before the tegg plugin (tegg depends on teggConfig), so it is the first tegg lifecycle to read/write the per-app scoped `configNames`. The tegg plugin only creates `app._teggScopeBag` in its own `configWillLoad` — too late for teggConfig's `#loadModuleConfigs` read. Under concurrent multi-app boot that read escaped to the process-default bag once a second app was live, tripping the strict-mode fuse. teggConfig now creates `app._teggScopeBag` if absent and runs its configNames set/read/clear inside `TeggScope.run`; the tegg plugin reuses the bag via `??=`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add MultiAppParallel.test.ts: `describe.concurrent` boots several `mm.app` instances at once (`cache: false` so same-baseDir bodies get distinct apps), each asserting per-app isolation of its singleton, data store and eventbus dispatch. Unlike MultiApp.test.ts (two different baseDirs driven sequentially), this reproduces same-baseDir concurrent boot, which surfaced the teggConfig configNames scope escape fixed in the previous commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Update: concurrent-boot regression test + a scope fix it surfacedReview feedback. All six inline points from the automated review are already implemented in the current revision (the simplification + adversarial-review passes) — replied inline and resolved each thread:
New test — Bug it caught + fixed. Under concurrent same-baseDir boot the strict-mode fuse threw Verified: parallel test stable across repeats; full plugin + standalone + config suite single-worker ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (1)
tegg/plugin/tegg/test/MultiApp.test.ts (1)
11-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace timer polling with the repository async-test helper.
This introduces
setTimeoutpolling in ategg/**/*.test.tsfile, which the repo explicitly avoids for async background work. Please switch this wait path to the existing helper pattern used in tegg tests.As per coding guidelines,
tegg/**/*.test.{ts,tsx}: UseBackgroundTaskHelperinstead ofsetTimeoutorsetImmediatein tests for async background work.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/plugin/tegg/test/MultiApp.test.ts` around lines 11 - 18, The waitFor helper in MultiApp.test.ts uses manual setTimeout polling, which violates the tegg test async-pattern guidelines. Replace this polling loop with the repository’s BackgroundTaskHelper-based helper pattern used in other tegg tests, and update the test flow to wait through that helper instead of repeatedly checking predicate with setTimeout.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 49-53: Narrow the tegg multi-app isolation summary so it only
applies to module-level lifecycle util access, since the current blanket
requirement conflicts with the bag-pinned `ScopedLifecycleUtil` and plugin boot
flow. Update the wording in the `tegg` guidance to say registrations using
`app.*LifecycleUtil` do not need an extra outer `TeggScope.run(...)` wrap when
they are already scoped to `app._teggScopeBag`, while keeping the
no-process-global-state rule intact.
In `@tegg/CLAUDE.md`:
- Around line 327-334: Update Rule 3 and the performance note to reflect the
shipped scoped facade instead of the old per-call TeggScope.run pattern. In the
CLAUDE.md guidance, revise the lifecycle-hook registration instructions so
plugin boot code uses the app-scoped lifecycle util getters exposed by
ScopedLifecycleUtil, which already delegate correctly, and remove language
implying every registerLifecycle/deleteLifecycle call must be wrapped manually.
Keep the references aligned with ScopedLifecycleUtil and the plugin boot
lifecycle registration examples so future changes follow the current design.
In `@tegg/core/types/src/scope/TeggScope.ts`:
- Around line 54-65: Fail closed for multi-app scope escapes in TeggScope:
reportEscape() currently only warns in production, allowing resolve/getOr/set to
continue into the process-default bag and mix app state. Update the
escape-handling path in TeggScope so that a missing active scope always aborts
the operation (for example by throwing or otherwise preventing fallback) and
make resolve, getOr, and set respect that failure instead of proceeding to the
default bag.
In `@tegg/plugin/dal/src/lib/MysqlDataSourceManager.ts`:
- Around line 9-17: The per-app teardown in MysqlDataSourceManager is incomplete
because MysqlDataSourceManager.instance.clear() only resets dataSourceIndices
while leaving dataSources populated. Update clear() in MysqlDataSourceManager to
also release the datasource cache, and if MysqlDataSource provides a
close/shutdown method, invoke it for each stored datasource before dropping the
map. Keep the fix localized to MysqlDataSourceManager so the app-close path in
app.ts באמת fully disposes per-app resources.
In `@tegg/plugin/mcp-proxy/src/app.ts`:
- Around line 15-16: The app-scoped hook registration is correct, but the later
MCPProxyHook callbacks read MCPControllerRegister.hooks outside the app’s
TeggScope, so they can see the wrong hook list. Capture the current app bag in
tegg/plugin/mcp-proxy/src/app.ts when registering the hook, and re-enter that
bag with TeggScope.run inside the proxy-handler callbacks before accessing
MCPControllerRegister.hooks. Use the existing symbols TeggScope,
MCPControllerRegister, and MCPProxyHook to locate the callback paths that need
wrapping.
In `@tegg/plugin/tegg/src/app/extend/application.unittest.ts`:
- Around line 17-29: The mock-context guard is still process-global, so parallel
apps can interfere with each other even though the helpers run in the app bag.
Move the hasMockModuleContext state into a TeggScope slot and update the related
helpers in mockModuleContextScope and destroyModuleContext to read/write through
that scoped getter/setter instead of a shared variable. Keep the existing
app-scoped flow in application.unittest.ts and ensure all guard checks use the
scoped value consistently.
In
`@tegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/BackgroundCounterService.ts`:
- Around line 18-24: The schedule method in BackgroundCounterService is still
executing the work synchronously because the callback passed to
BackgroundTaskHelper.run does not introduce an async boundary before using
counterService. Update the callback so it truly defers execution across an
awaited boundary before incrementing, and keep the logic inside schedule using
the existing BackgroundTaskHelper.run and counterService symbols so the test
exercises background-scoped resolution after an async escape.
In `@tegg/plugin/tegg/test/MultiApp.test.ts`:
- Around line 106-123: In the “should not leak state between sequential app
lifecycles” test, the first app instance is only closed after the assertions, so
a failure before that can leave app1 running and affect later tests. Update the
test flow around mm.app, app1.ready(), and CounterService usage so app1 is
always closed in a finally block, keeping the existing app2 cleanup pattern and
ensuring both app lifecycles are isolated even when an assertion fails.
In `@tegg/plugin/tegg/test/MultiAppParallel.test.ts`:
- Around line 10-18: Replace the raw timer-based polling and yield in
MultiAppParallel.test with BackgroundTaskHelper so the test waits on background
work deterministically instead of relying on setTimeout/setImmediate. Update the
waitFor helper and the explicit async pause to use BackgroundTaskHelper, keeping
the test synchronized through the helper APIs rather than time-based retries.
Make sure the changes are applied wherever the test currently uses those waits
so the concurrency check remains stable under load.
In `@tegg/standalone/standalone/src/EggModuleLoader.ts`:
- Around line 72-79: `EggModuleLoader.generateAppGraph` builds
`moduleConfigList`, but `loaderCache` is never populated before
`loaderCache.get(modulePath)!` is used, so
`LoadUnitFactory.createPreloadLoadUnit` receives an invalid loader. Update the
preload loop in `EggModuleLoader` to fill `loaderCache` with the correct
`Loader` instances for each `modulePath` before lookup, or derive the loader
directly from the available module metadata, and keep the `moduleConfig.path` /
`LoadUnitFactory.createPreloadLoadUnit` flow consistent.
In `@tegg/standalone/standalone/src/Runner.ts`:
- Around line 292-296: The Runner prototype resolution currently goes straight
to the global metadata lookup, which can ignore a per-scope registration. Update
the prototype lookup in Runner’s constructor/setup path to try
EggPrototypeFactory.instance.getPrototypeByClazz(runnerClass) first, and only
fall back to PrototypeUtil.getClazzProto(runnerClass) if the scoped lookup
returns nothing. Keep the existing error handling and assignment to
this.runnerProto, but make sure the scoped factory lookup is the primary path
used for runnerClass.
---
Nitpick comments:
In `@tegg/plugin/tegg/test/MultiApp.test.ts`:
- Around line 11-18: The waitFor helper in MultiApp.test.ts uses manual
setTimeout polling, which violates the tegg test async-pattern guidelines.
Replace this polling loop with the repository’s BackgroundTaskHelper-based
helper pattern used in other tegg tests, and update the test flow to wait
through that helper instead of repeatedly checking predicate with setTimeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb36df64-cd55-4ff5-bb2a-1faa82185fbb
⛔ Files ignored due to path filters (6)
tegg/core/lifecycle/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/core/metadata/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/core/runtime/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snaptegg/core/tegg/test/__snapshots__/exports.test.ts.snapis excluded by!**/*.snaptegg/core/tegg/test/__snapshots__/helper.test.ts.snapis excluded by!**/*.snaptegg/core/types/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (73)
AGENTS.mdtegg/CLAUDE.mdtegg/core/aop-runtime/src/LoadUnitAopHook.tstegg/core/common-util/src/ModuleConfig.tstegg/core/dynamic-inject-runtime/src/EggObjectFactory.tstegg/core/eventbus-runtime/src/SingletonEventBus.tstegg/core/lifecycle/src/ScopedLifecycleUtil.tstegg/core/lifecycle/src/index.tstegg/core/metadata/src/factory/EggPrototypeFactory.tstegg/core/metadata/src/factory/LoadUnitFactory.tstegg/core/metadata/src/impl/LoadUnitMultiInstanceProtoHook.tstegg/core/metadata/src/model/EggPrototype.tstegg/core/metadata/src/model/LoadUnit.tstegg/core/metadata/src/model/graph/GlobalGraph.tstegg/core/runtime/src/factory/EggContainerFactory.tstegg/core/runtime/src/factory/EggObjectFactory.tstegg/core/runtime/src/factory/LoadUnitInstanceFactory.tstegg/core/runtime/src/model/ContextHandler.tstegg/core/runtime/src/model/EggContext.tstegg/core/runtime/src/model/EggObject.tstegg/core/runtime/src/model/LoadUnitInstance.tstegg/core/types/package.jsontegg/core/types/src/index.tstegg/core/types/src/scope/TeggScope.tstegg/core/types/src/scope/index.tstegg/core/types/test/TeggScope.test.tstegg/plugin/aop/src/app.tstegg/plugin/config/package.jsontegg/plugin/config/src/app.tstegg/plugin/controller/src/app.tstegg/plugin/controller/src/lib/ControllerMetadataManager.tstegg/plugin/controller/src/lib/impl/http/HTTPControllerRegister.tstegg/plugin/controller/src/lib/impl/mcp/MCPControllerRegister.tstegg/plugin/dal/src/app.tstegg/plugin/dal/src/app/extend/application.tstegg/plugin/dal/src/lib/MysqlDataSourceManager.tstegg/plugin/dal/src/lib/SqlMapManager.tstegg/plugin/dal/src/lib/TableModelManager.tstegg/plugin/dal/test/transaction.test.tstegg/plugin/eventbus/src/app.tstegg/plugin/eventbus/src/lib/EggContextEventBus.tstegg/plugin/langchain/src/app.tstegg/plugin/langchain/src/lib/graph/GraphLoadUnitHook.tstegg/plugin/mcp-proxy/src/app.tstegg/plugin/orm/package.jsontegg/plugin/orm/src/app.tstegg/plugin/schedule/src/app.tstegg/plugin/tegg/package.jsontegg/plugin/tegg/src/app.tstegg/plugin/tegg/src/app/extend/application.tstegg/plugin/tegg/src/app/extend/application.unittest.tstegg/plugin/tegg/src/app/extend/context.tstegg/plugin/tegg/src/lib/CompatibleUtil.tstegg/plugin/tegg/src/lib/ctx_lifecycle_middleware.tstegg/plugin/tegg/src/types.tstegg/plugin/tegg/test/MultiApp.test.tstegg/plugin/tegg/test/MultiAppParallel.test.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation-b/config/config.default.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation-b/config/module.jsontegg/plugin/tegg/test/fixtures/apps/multi-app-isolation-b/config/plugin.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation-b/package.jsontegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/config/config.default.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/config/module.jsontegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/config/plugin.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/BackgroundCounterService.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/CounterEvent.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/CounterService.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/package.jsontegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/package.jsontegg/standalone/standalone/package.jsontegg/standalone/standalone/src/EggModuleLoader.tstegg/standalone/standalone/src/Runner.tstegg/standalone/standalone/src/main.ts
- EggModuleLoader.preLoad built a `loaderCache` it never populated, so `loaderCache.get(modulePath)!` passed `undefined` into createPreloadLoadUnit; create the loader inline like `load()` does. - Runner resolved the main runner proto via the process-global `PrototypeUtil.getClazzProto`; use the per-app `EggPrototypeFactory.instance.getPrototypeByClazzOrGlobal` so concurrent Runners don't read clobbered class metadata. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- MysqlDataSourceManager.clear() now also clears the `dataSources` map so a closed app releases its datasource objects (the map was left populated). - mcp-proxy proxy handlers run detached from the request; re-enter the owning app's TeggScope bag before reading the scope-backed MCPControllerRegister.hooks. - application.unittest `hasMockModuleContext` was a process-global guard; move it into a per-app TeggScope slot so concurrent multi-app tests don't block each other. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- MultiApp.test.ts: close app1 in a `finally` so an early assertion failure cannot leak it into later cases. - BackgroundCounterService fixture: yield (setImmediate) before incrementing so the background-task isolation check truly runs across an async boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
app.*LifecycleUtil getters are bag-pinned, so hook registration through them needs no TeggScope.run wrap, and the facade is an explicit delegating object, not a Proxy. Update tegg/CLAUDE.md Rule 3 + the performance note and the AGENTS.md summary accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tegg/plugin/tegg/src/app/extend/application.unittest.ts (1)
26-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHold the scoped guard for the full mock-context lifetime.
mockModuleContextsets the guard only afterinit,mockModuleContextScopenever sets it, anddestroyModuleContextclears it before destroy completes. Same-app overlapping calls can still pass the guard. Set it immediately after the check, and clear it only in teardown/finally. As per coding guidelines,tegg/**/*.{ts,tsx}runtime state should stay scoped per app; this comment relies on that scoped guard contract.Suggested guard-lifetime fix
const doWork = async (): Promise<Context> => { if (getHasMockModuleContext()) { throw new Error('should not call mockModuleContext twice.'); } + setHasMockModuleContext(true); // `@ts-expect-error` mockContext is not typed const ctx = this.mockContext(data) as Context; const teggCtx = new EggContextImpl(ctx); const lifecycle = {}; TEGG_LIFECYCLE_CACHE.set(teggCtx, lifecycle); - if (teggCtx.init) { - await teggCtx.init(lifecycle); + try { + if (teggCtx.init) { + await teggCtx.init(lifecycle); + } + return ctx; + } catch (err) { + setHasMockModuleContext(false); + throw err; } - setHasMockModuleContext(true); - return ctx; };const doWork = async (): Promise<void> => { - setHasMockModuleContext(false); const teggCtx = ctx.teggContext; - if (!teggCtx) { - return; - } - const lifecycle = TEGG_LIFECYCLE_CACHE.get(teggCtx); - if (teggCtx.destroy && lifecycle) { - await teggCtx.destroy(lifecycle); + try { + if (!teggCtx) { + return; + } + const lifecycle = TEGG_LIFECYCLE_CACHE.get(teggCtx); + if (teggCtx.destroy && lifecycle) { + await teggCtx.destroy(lifecycle); + } + } finally { + setHasMockModuleContext(false); } };if (getHasMockModuleContext()) { throw new Error( 'mockModuleContextScope can not use with mockModuleContext, should use mockModuleContextScope only.', ); } + setHasMockModuleContext(true); // `@ts-expect-error` mockContextScope only exists in MockApplication - return this.mockContextScope(async (ctx: Context) => { + return this.mockContextScope(async (ctx: Context) => { const teggCtx = new EggContextImpl(ctx); const lifecycle = {}; if (teggCtx.init) { await teggCtx.init(lifecycle); } try { return await fn(ctx); } finally { await teggCtx.destroy(lifecycle); + setHasMockModuleContext(false); } }, data);Also applies to: 43-55, 60-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/plugin/tegg/src/app/extend/application.unittest.ts` around lines 26 - 37, The scoped guard for mock-context lifecycle is being set too late and cleared too early, so overlapping calls can still bypass the single-app constraint. Update the mock-context flow in the application extension methods around mockModuleContext, mockModuleContextScope, and destroyModuleContext so setHasMockModuleContext(true) happens immediately after the initial guard check, and make sure the flag is only cleared in teardown/finally after init/destroy fully completes. Keep the guard lifetime consistent across the full mock-context lifetime and preserve the existing TEGG_LIFECYCLE_CACHE and EggContextImpl-based flow.Source: Coding guidelines
tegg/plugin/dal/src/lib/MysqlDataSourceManager.ts (1)
60-65: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftClose live datasources before clearing the cache.
clear()now releases references, but these entries are described as live datasource connections; dropping the maps may leave pools/sockets open after app teardown. IfMysqlDataSourceexposesclose/destroy/shutdown, call it before clearing and update teardown call sites to await it. This is the remaining part of the earlier teardown concern.#!/bin/bash # Verify MysqlDataSource shutdown API and clear() call sites. rg -nP 'class\s+MysqlDataSource\b|interface\s+MysqlDataSource\b|async\s+(close|destroy|shutdown)\s*\(|\b(close|destroy|shutdown)\s*\(' tegg -g '*.ts' -C3 rg -nP '\bMysqlDataSourceManager\.instance\.clear\s*\(' tegg -g '*.ts' -C3🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tegg/plugin/dal/src/lib/MysqlDataSourceManager.ts` around lines 60 - 65, Close active MysqlDataSource connections before dropping the cache in MysqlDataSourceManager.clear(), instead of only clearing dataSources and dataSourceIndices. Check whether MysqlDataSource exposes close, destroy, or shutdown, and invoke that lifecycle method for each live datasource before clearing the maps. Then update any MysqlDataSourceManager.instance.clear() teardown call sites to await the new async clear path so shutdown completes cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tegg/plugin/dal/src/lib/MysqlDataSourceManager.ts`:
- Around line 60-65: Close active MysqlDataSource connections before dropping
the cache in MysqlDataSourceManager.clear(), instead of only clearing
dataSources and dataSourceIndices. Check whether MysqlDataSource exposes close,
destroy, or shutdown, and invoke that lifecycle method for each live datasource
before clearing the maps. Then update any
MysqlDataSourceManager.instance.clear() teardown call sites to await the new
async clear path so shutdown completes cleanly.
In `@tegg/plugin/tegg/src/app/extend/application.unittest.ts`:
- Around line 26-37: The scoped guard for mock-context lifecycle is being set
too late and cleared too early, so overlapping calls can still bypass the
single-app constraint. Update the mock-context flow in the application extension
methods around mockModuleContext, mockModuleContextScope, and
destroyModuleContext so setHasMockModuleContext(true) happens immediately after
the initial guard check, and make sure the flag is only cleared in
teardown/finally after init/destroy fully completes. Keep the guard lifetime
consistent across the full mock-context lifetime and preserve the existing
TEGG_LIFECYCLE_CACHE and EggContextImpl-based flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4040eaad-ce6e-497e-bbc2-d4ddfe9dfde7
📒 Files selected for processing (9)
AGENTS.mdtegg/CLAUDE.mdtegg/plugin/dal/src/lib/MysqlDataSourceManager.tstegg/plugin/mcp-proxy/src/index.tstegg/plugin/tegg/src/app/extend/application.unittest.tstegg/plugin/tegg/test/MultiApp.test.tstegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/BackgroundCounterService.tstegg/standalone/standalone/src/EggModuleLoader.tstegg/standalone/standalone/src/Runner.ts
✅ Files skipped from review due to trivial changes (2)
- AGENTS.md
- tegg/CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (4)
- tegg/plugin/tegg/test/MultiApp.test.ts
- tegg/plugin/tegg/test/fixtures/apps/multi-app-isolation/modules/counter-module/BackgroundCounterService.ts
- tegg/standalone/standalone/src/EggModuleLoader.ts
- tegg/standalone/standalone/src/Runner.ts
killagu
added a commit
that referenced
this pull request
Jun 27, 2026
…t gate (#6004) ## Motivation The suite already runs with `pool: 'threads'` + `isolate: false` for full parallelism. This PR makes the **effect** of that parallelism visible in CI — so we can see how parallel the run actually was, per OS/Node matrix entry — **without changing gate semantics**. > Independent of #5986 (tegg `TeggScope`): every file touched here is identical to `next`, so this branch is cut from `next` and can merge on its own timeline. ## What - **`vitest.config.ts`** — adds a `json` reporter **only when `CI` is set**, writing `benchmark/ci-test/ci-run/vitest-results.json` as a side effect of the gating `ut run ci` (default console reporter preserved; no extra test run). - **`.github/workflows/ci.yml`** — a `Report parallelism metrics` step (`if: always()`) in the existing `test` job summarizes that JSON and appends the report to the **GitHub job summary**. Gating still comes solely from `ut run ci`. - **`scripts/ci-test-benchmark/`** — the existing harness gains a `--report-only --vitest-json` mode and computes **peak/avg concurrency, parallel efficiency, and critical path** from a per-file concurrency-timeline sweep. - **`test(supertest)`** *(separate commit)* — harden the over-strict `should handle connection error` test (hardcoded `127.0.0.1:1234`) to accept the connection-error family instead of exactly `ECONNREFUSED`, so it isn't environment-dependent (a local proxy on that port made it `socket hang up`). ## Reading the metrics honestly Vitest 4 derives a file's interval from **test-level timings only**, so the spans cover test bodies + per-test `beforeEach`/`afterEach` but **exclude suite-level `beforeAll`/`afterAll` (where egg boots its apps) and module transform/import**. Therefore **avg concurrency / parallel efficiency are lower bounds**; **peak concurrency** is the robust signal. The worker ceiling mirrors `vitest.config.ts` (Windows CI caps workers). Fully-skipped files are dropped from the timeline. The report footnote states all of this. ## Test evidence - Full **`isolate: false` suite is GREEN** (services up, CI-faithful `--maxWorkers 4`): **526 files / 3425 tests pass, 0 failures**. - The edited config loads in real Vitest (both reporters active, JSON at the configured path); harness math cross-checked independently; `--report-only` exits `0` on missing JSON; step-summary append is cross-platform; `oxfmt`/`oxlint` clean. - A 4-agent adversarial review pass was applied (Windows worker-ceiling, beforeAll-exclusion docs, skipped-file timeline, report-only run section all fixed). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Status update (latest)
next(resolved the aop + MCPControllerRegister conflicts) and CI is green across the full matrix (macOS/Ubuntu/Windows × Node 22/24), both E2E suites, typecheck/fmt/lint, and project coverage.TeggScopebag was unreachable from code running outside an explicitTeggScope.run(a singleton/ContextProto method or detached logger called directly). The no-scope fallback now resolves to the sole live app's bag when exactly one app is alive, restoring pre-scoping single-app behavior while keeping multi-app isolation + the escape fuse. This fixed a real@eggjs/orm-pluginregression (leoric's SQL logger threwgetContextCallback not set, swallowed → no SQL logged).TeggScope.runMaybe,EggPrototypeFactory.getPrototypeByClazzOrGlobal(centralizes the rule-4 fallback),defineScopedLifecycleUtiltuple helper, a uniformrunInScopein the standalone Runner, dropped dead code (_setDefaultBag, controllerwillReadyMCP block) — all behavior-preserving.EggPrototypeFactory.instancein langchain'sGraphLoadUnitHook.preCreate(it had captured the factory in the boot constructor → cross-app pollution under multi-app); guaranteeTeggScope.unregisterScopeon every teardown/error path in the standalone Runner +main()+ the tegg pluginbeforeClose(scope-registry leaks would have flippedisMultiAppon for later apps).TeggScopeunit coverage for the scope/fallback/escape behavior.Motivation
Today tegg keeps a lot of runtime state in process-global statics (factory registries keyed by deterministic name,
ContextHandlercallbacks, singleton managers, lifecycle utils…). This makes it a "one active app at a time" design — two tegg apps in one process clobber each other. This PR makes per-app state isolated so multiple apps can boot and serve requests concurrently in one process without cross-talk.Approach:
TeggScope+ per-package slotsA new low-level, type-free
TeggScope(AsyncLocalStorage<Map<symbol, unknown>>) in@eggjs/tegg-types:TeggScope— never another package. Dependency direction stays downward (metadata never imports runtime).EggPrototypeFactory.instance,GlobalGraph.instance,ContextHandler, dal managers, lifecycle utils, …) but resolve from the current app's bag.plugin/tegg(app._teggScopeBag, created inconfigWillLoad); boot hooks, request middleware,mockModuleContextScope,getEggObject, theapp.module/ctx.moduleproxies, the standaloneRunner, and escape points wrap work inTeggScope.run(bag, …).> 1live app) any access that escapes to the process-default bag throws in dev / warns in prod. Single-app keeps a silent lazy default — no behavior change for existing single-app code/tests.Scope (request-reachability)
EggPrototypeFactory(+ per-appclass→protomap),GlobalGraph,LoadUnitFactory(two-tier creator map),LoadUnitMultiInstanceProtoHook, prototype/load-unit lifecycle utils.LoadUnitInstanceFactory,EggObjectFactory,ContextHandler, object/context/load-unit-instance lifecycle utils.ModuleConfigUtil.configNames(fixes the standalone config-names race).SingletonEventBus,doEmitre-establishes the owning app's scope for detached emits), orm, schedule, langchain, mcp-proxy.Runnerscope wrapping load/init/run/destroy.Two issues found beyond the original design
PrototypeUtil.getClazzProtostores one proto per class globally, so concurrent boot races. Added per-appEggPrototypeFactory.getPrototypeByClazz, preferred ingetOrCreateEggObjectFromClazzandctx.getEggObject.Testing
plugin/tegg/test/MultiApp.test.ts: two concurrent apps sharing one module, asserting isolation of singletons, a per-app data store, EventBus emit/handler dispatch, and background tasks, plus sequential no-leak — all under strict mode (the escape fuse stays silent ⇒ zero escapes).@eggjs/orm-pluginUnknown database 'test',@eggjs/agent-runtime"stream … during reconnect") reproduce onnextunchanged — pre-existing env/flaky, not regressions. Standalone's previously-flakyshould work with envnow passes (per-Runner config names).Notes for reviewers
@eggjs/tegg-typesexportingTeggScope.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests