feat(cluster): support bundle and startup snapshots - #6042
Conversation
Deploying egg with
|
| Latest commit: |
7ed8296
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8a376a13.egg-cci.pages.dev |
| Branch Preview URL: | https://codex-snapshot-cluster-integ.egg-cci.pages.dev |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds process and worker-thread cluster protocols, single and cluster V8 snapshot bundles, snapshot lifecycle guards, manifest-aware loader discovery, Leoric compatibility handling, and CLI support for worker files and snapshot blobs. It also updates tests, documentation, exports, and CI execution. ChangesRuntime, bundling, and loading
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
Deploying egg-v3 with
|
| Latest commit: |
7ed8296
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d79ead82.egg-v3.pages.dev |
| Branch Preview URL: | https://codex-snapshot-cluster-integ.egg-v3.pages.dev |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #6042 +/- ##
==========================================
+ Coverage 82.93% 83.08% +0.15%
==========================================
Files 724 730 +6
Lines 22293 22474 +181
Branches 4448 4520 +72
==========================================
+ Hits 18488 18673 +185
+ Misses 3305 3297 -8
- Partials 500 504 +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. |
There was a problem hiding this comment.
Pull request overview
This PR extends Egg’s bundling and V8 startup-snapshot support to fully cover cluster mode (separate app/agent workers), including process vs worker_threads launch behavior, role-specific snapshot blobs, and manifest-backed loader discovery that works consistently across Egg + tegg. It also updates docs/wiki to describe the supported workflows and constraints.
Changes:
- Add cluster-target bundling/snapshot outputs (
app_worker.js/agent_worker.js, plus role blobs) and update CLIs (egg-bin bundle --cluster,egg-bin snapshot build --cluster,egg-scripts start --bundle). - Refactor cluster worker startup into a shared protocol with process/worker-thread IO adapters and add snapshot/worker-file option parsing.
- Introduce authoritative manifest discovery (
LoaderFS.getKnownFiles) to prevent bundle +EGG_TS_ENABLE=falsefrom dropping manifest-indexed.tskeys, and document the behavior.
Reviewed changes
Copilot reviewed 77 out of 77 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wiki/packages/loader-fs.md | Document getKnownFiles |
| wiki/packages/egg-bundler.md | Update bundler/snapshot docs |
| wiki/log.md | Log new behaviors |
| wiki/index.md | Update bundler summary |
| tools/scripts/test/snapshot-start.test.ts | Add bundle/snapshot start tests |
| tools/scripts/src/commands/start.ts | Add --bundle + role blobs |
| tools/egg-bundler/tsdown.config.ts | Copy leoric loader |
| tools/egg-bundler/test/snapshot-lazy-external.test.ts | Add cluster + receiver tests |
| tools/egg-bundler/test/snapshot-lazy-bundler.test.ts | Leoric + cluster externals tests |
| tools/egg-bundler/test/single-file.realbuild.test.ts | Update entry generator mock shape |
| tools/egg-bundler/test/prelude.test.ts | Test plain runtime prelude |
| tools/egg-bundler/test/PackRunner.test.ts | Test module.rules passthrough |
| tools/egg-bundler/test/leoric-runtime-require-loader.test.ts | Test leoric require rewrite |
| tools/egg-bundler/test/leoric-loader.realbuild.test.ts | Realbuild snapshot restore coverage |
| tools/egg-bundler/test/EntryGenerator.test.ts | Cluster entries generation tests |
| tools/egg-bundler/test/cluster-entries.realbuild.test.ts | Realbuild cluster entries (proc/thread) |
| tools/egg-bundler/test/cjs-esm-interop.realbuild.test.ts | Update entry generator mock shape |
| tools/egg-bundler/test/Bundler.test.ts | Snapshot prelude for both entries |
| tools/egg-bundler/test/snapshots/EntryGenerator.worker.canonical.snap | Snapshot entry canonical output |
| tools/egg-bundler/src/lib/prelude.ts | Add cluster lazy externals + runtime path |
| tools/egg-bundler/src/lib/PackRunner.ts | Support module.rules passthrough |
| tools/egg-bundler/src/lib/Bundler.ts | Cluster target + leoric compat wiring |
| tools/egg-bundler/src/index.ts | Export new types/configs |
| tools/egg-bundler/src/compat/leoric/runtime-require-loader.cjs | Add leoric transform loader |
| tools/egg-bundler/src/compat/leoric/index.ts | Detect + configure leoric compat |
| tools/egg-bundler/README.md | Document cluster target |
| tools/egg-bundler/docs/output-structure.md | Update output + workflows |
| tools/egg-bin/test/commands/snapshot.test.ts | Cluster snapshot build tests |
| tools/egg-bin/test/commands/bundle.test.ts | Cluster bundle flag tests |
| tools/egg-bin/src/commands/snapshot.ts | Add --cluster role blobs |
| tools/egg-bin/src/commands/bundle.ts | Add --cluster bundle output |
| tegg/plugin/tegg/test/agent-loader-fs.test.ts | Test agent LoaderFS overlay |
| tegg/plugin/tegg/src/lib/install-tegg-loader-fs.ts | Install tegg LoaderFS overlay |
| tegg/plugin/tegg/src/lib/EggModuleLoader.ts | Use host loaderFS consistently |
| tegg/plugin/tegg/src/app.ts | Install overlay in app boot |
| tegg/plugin/tegg/src/agent.ts | Add agent boot hook |
| tegg/plugin/tegg/package.json | Export ./agent entry |
| tegg/core/loader/test/ModuleLoaderLoaderFS.test.ts | Test knownFiles + empty dirs |
| tegg/core/loader/src/impl/ModuleLoader.ts | Prefer getKnownFiles() |
| site/docs/zh-CN/core/bundle.md | Update CN bundle docs |
| site/docs/zh-CN/advanced/snapshot.md | Update CN snapshot docs |
| site/docs/zh-CN/advanced/snapshot-troubleshooting.md | Update CN troubleshooting |
| site/docs/core/bundle.md | Update EN bundle docs |
| site/docs/advanced/snapshot.md | Update EN snapshot docs |
| site/docs/advanced/snapshot-troubleshooting.md | Update EN troubleshooting |
| plugins/watcher/test/boot.test.ts | Ensure watcher init in configDidLoad |
| plugins/watcher/src/lib/boot.ts | Move watcher init to runtime phase |
| packages/loader-fs/test/manifest_loader_fs.test.ts | Test getKnownFiles() behavior |
| packages/loader-fs/src/manifest_loader_fs.ts | Implement getKnownFiles() |
| packages/loader-fs/src/index.ts | Add LoaderFS API hook |
| packages/loader-fs/README.md | Document getKnownFiles() |
| packages/egg/test/snapshot.test.ts | Test clusterWrapper snapshot guard |
| packages/egg/src/lib/egg.ts | Guard clusterWrapper during build |
| packages/core/test/loader/egg_loader.test.ts | Ensure loaderFS is “current” |
| packages/core/src/loader/egg_loader.ts | Make loaderFS mutable |
| packages/cluster/tsdown.config.ts | Build worker_protocol entry |
| packages/cluster/test/worker_threads.test.ts | Unskip worker_threads test |
| packages/cluster/test/worker_protocol.test.ts | Add protocol + IO tests |
| packages/cluster/test/options.test.ts | Add worker file/blob options tests |
| packages/cluster/test/fixtures/worker-thread-io.mjs | Add IO fixture |
| packages/cluster/src/worker_protocol/worker-thread.ts | Add worker_threads IO adapter |
| packages/cluster/src/worker_protocol/process.ts | Add process IO adapter |
| packages/cluster/src/worker_protocol/index.ts | Export protocol + IO |
| packages/cluster/src/worker_protocol/app.ts | Extract app worker protocol |
| packages/cluster/src/worker_protocol/agent.ts | Extract agent worker protocol |
| packages/cluster/src/utils/options.ts | Parse worker file/blob options |
| packages/cluster/src/utils/mode/impl/worker_threads/app.ts | Allow custom app worker file |
| packages/cluster/src/utils/mode/impl/worker_threads/agent.ts | Allow custom agent worker file |
| packages/cluster/src/utils/mode/impl/process/app.ts | Support app snapshot blob |
| packages/cluster/src/utils/mode/impl/process/agent.ts | Support agent snapshot blob |
| packages/cluster/src/index.ts | Re-export worker_protocol API |
| packages/cluster/src/app_worker.ts | Use shared worker protocol |
| packages/cluster/src/agent_worker.ts | Use shared worker protocol |
| packages/cluster/package.json | Export ./worker_protocol |
| AGENTS.md | Document snapshot lifecycle rules |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 78 out of 78 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/cluster/src/worker_protocol/worker-thread.ts:39
- In worker threads, the process 'exit' event does not wait for async handlers, so awaiting beforeExit() here will not reliably run cleanup and may also lead to unhandled promise rejections. Also calling process.exit() inside an 'exit' handler is unnecessary (the thread is already exiting). Prefer a synchronous handler and, if beforeExit returns a Promise, explicitly attach a .catch() to avoid unhandled rejections.
gracefulExit(options: GracefulExitOptions): void {
const { beforeExit } = options;
process.on('exit', async (code) => {
if (typeof beforeExit === 'function') {
await beforeExit();
}
process.exit(code);
});
},
packages/cluster/src/utils/options.ts:196
- Error message grammar: 'file should exists' → 'file should exist'. This text is user-facing when option validation fails.
assert(fs.existsSync(options[optionName]), `options.${optionName} file should exists: ${options[optionName]}`);
packages/cluster/src/utils/options.ts:190
- Error message grammar: 'file should exists' → 'file should exist'. This text is user-facing when option validation fails.
This issue also appears on line 196 of the same file.
assert(fs.existsSync(options[optionName]), `options.${optionName} file should exists: ${options[optionName]}`);
00af46f to
89d5acb
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/egg-bin/src/commands/snapshot.ts (1)
171-184: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRemove an existing blob before each snapshot build.
fs.accesscannot distinguish a new blob from a blob left by an earlier build. If Node exits with code 0 without writing the current snapshot, this check accepts the stale blob and reports a successful build.Remove the target before spawning Node. Do not remove it during
--dry-run.Proposed fix
for (const entry of snapshotEntries) { + if (!flags['dry-run']) { + await fs.rm(entry.blobPath, { force: true }); + } await this.#spawnNode(['--snapshot-blob', entry.blobPath, '--build-snapshot', entry.workerPath]); }🤖 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 `@tools/egg-bin/src/commands/snapshot.ts` around lines 171 - 184, Update the snapshot build flow around snapshotEntries and `#spawnNode` to remove each entry.blobPath immediately before spawning Node, ensuring stale blobs cannot satisfy the later fs.access check. Guard the removal so it is skipped when flags['dry-run'] is enabled, while preserving the existing dry-run and post-build verification behavior.
🧹 Nitpick comments (2)
tools/egg-bundler/test/cluster-entries.realbuild.test.ts (1)
140-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRace the worker
errorandexitevents withmessage.
once(worker, 'message')only settles on a message. If the bundled worker throws during startup, the promise never settles and the test waits for the 60s timeout. A race againsterrorandexitreports the real failure immediately.♻️ Proposed fail-fast wait
- const [message] = await once(worker, 'message'); + const [message] = await Promise.race([ + once(worker, 'message'), + once(worker, 'error').then(([err]) => { + throw err; + }), + once(worker, 'exit').then(([code]) => { + throw new Error(`worker exited early with code ${code}`); + }), + ]);🤖 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 `@tools/egg-bundler/test/cluster-entries.realbuild.test.ts` around lines 140 - 152, Update the worker startup wait around ThreadWorker and once(worker, 'message') to race the message event with worker error and exit events, rejecting or failing immediately when startup terminates without the expected message. Preserve the existing message assertions and ensure the finally block still terminates the worker.packages/cluster/test/worker_protocol.test.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename this test file to use hyphens.
Rename
worker_protocol.test.tstoworker-protocol.test.ts. Update explicit references if any.As per coding guidelines, keep file names lowercase with hyphens.
🤖 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 `@packages/cluster/test/worker_protocol.test.ts` at line 1, Rename the test file from worker_protocol.test.ts to worker-protocol.test.ts and update any explicit references to the old filename while preserving the test contents.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 `@packages/cluster/src/app_worker.ts`:
- Around line 35-36: In the worker startup flow before the workerIO selection,
validate that sticky mode is not combined with options.startMode set to
'worker_threads'. Reject this configuration with a clear error stating that
sticky mode requires process workers, then preserve the existing
createWorkerThreadIO and createProcessWorkerIO selection for valid
configurations.
In `@packages/cluster/src/utils/options.ts`:
- Around line 186-191: Normalize empty appWorkerFile and agentWorkerFile values
in parseOptions before worker startup, and update worker-thread launchers to use
truthy fallbacks rather than nullish fallbacks for unparsed options. Apply the
fallback consistently in agent.ts and app.ts, including app reforks. Add
regression tests in packages/cluster/test/options.test.ts covering empty values
for both worker paths.
In `@packages/cluster/src/worker_protocol/app.ts`:
- Line 80: Update the port assignment in the worker app initialization to use
nullish coalescing or an explicit undefined check instead of ||, so an
explicitly provided options.port value of 0 is preserved while undefined or null
still falls back to listenConfig.port.
In `@packages/cluster/src/worker_protocol/worker-thread.ts`:
- Around line 31-39: Update gracefulExit so the process exit listener performs
no asynchronous work. Move the beforeExit cleanup into an explicit shutdown
signal or parent-message handler, await beforeExit there, and call process.exit
only after it completes; keep the exit listener limited to synchronous
termination behavior.
In `@site/docs/advanced/snapshot-troubleshooting.md`:
- Around line 191-195: Qualify the diagnostic in the English paragraph around
“Pushing a suspect package” to state that a successful --force-external build
identifies the package or one of its import-time dependencies as the unsafe
inlined import path. Apply the same qualification to the corresponding Chinese
diagnostic in site/docs/zh-CN/advanced/snapshot-troubleshooting.md at lines
173-176.
In `@site/docs/advanced/snapshot.md`:
- Around line 23-26: Update the tables in site/docs/advanced/snapshot.md lines
23-26 and site/docs/zh-CN/advanced/snapshot.md lines 20-23 to scope the Node.js
24 requirement to launches restoring snapshot blobs; distinguish ordinary
--bundle cluster startup, which supports Node.js 22+, from --snapshot-blob or
--bundle launches with role snapshot blobs, which require Node.js 24.
In `@tegg/core/loader/test/ModuleLoaderLoaderFS.test.ts`:
- Around line 41-77: Add a concurrent two-application regression test alongside
the existing ModuleLoader tests, using separate ManifestLoaderFS instances with
different manifest file lists and app-specific StubLoaderFS fallbacks. Run both
loads concurrently, verify each returns only its own manifest-defined modules,
and assert neither loader invokes the other application’s fallback.
In `@tools/egg-bundler/docs/output-structure.md`:
- Around line 3-5: Update the bundle contract description in the output
structure documentation to state that everything except configured or
auto-detected externals is inlined, replacing the narrower “declared externals”
wording while preserving the existing CJS bundle and outputDir behavior.
In `@tools/egg-bundler/src/lib/EntryGenerator.ts`:
- Around line 625-633: Update generate() and `#renderClusterWorkerEntry`() so each
worker role receives only its role-specific bundle entries before
`#renderBundleRuntime`() emits imports: app workers must exclude agent-only
modules, and agent workers must exclude app-only modules. Preserve shared
entries for both roles and ensure the rendered app_worker.entry.ts and
agent_worker.entry.ts do not execute opposite-role modules.
In `@tools/egg-bundler/src/lib/prelude.ts`:
- Around line 303-307: Update the member proxy resolution used by the get, set,
apply, and construct traps so the first successful resolve() result is memoized
and reused for subsequent trap calls. Ensure chains ending in apply or construct
preserve mutations across accesses, while unresolved chains continue to build
deferred members normally.
In `@tools/egg-bundler/test/leoric-loader.realbuild.test.ts`:
- Around line 89-93: Update the snapshot test around the Node.js version guard
so `--build-snapshot` always runs for supported Node.js 22+, while the Node.js
24-or-newer check applies only to the restoration step. Preserve the existing
`execFileAsync` commands and ensure Node.js 22 builds the blob without
attempting restoration.
In `@tools/egg-bundler/test/snapshot-lazy-bundler.test.ts`:
- Around line 216-220: Strengthen the manifest validation after reading
bundle-manifest.json by asserting individually that manifest.externals does not
contain each database package ID: mysql, mysql2, sqlite3, pg, pg-types, and
sql.js. Remove the single negated arrayContaining assertion while preserving the
existing separate leoric assertion and matching the per-ID checks already used
for packConfig.externals.
In `@tools/scripts/test/snapshot-start.test.ts`:
- Around line 189-190: Update each test using pinNodeVersion, including the
sites around the referenced tests, to retain its returned restoration callback
and invoke it in a finally block. Ensure restoration runs whether the test
succeeds or throws, preventing the pinned process.versions state from leaking
into subsequent tests.
---
Outside diff comments:
In `@tools/egg-bin/src/commands/snapshot.ts`:
- Around line 171-184: Update the snapshot build flow around snapshotEntries and
`#spawnNode` to remove each entry.blobPath immediately before spawning Node,
ensuring stale blobs cannot satisfy the later fs.access check. Guard the removal
so it is skipped when flags['dry-run'] is enabled, while preserving the existing
dry-run and post-build verification behavior.
---
Nitpick comments:
In `@packages/cluster/test/worker_protocol.test.ts`:
- Line 1: Rename the test file from worker_protocol.test.ts to
worker-protocol.test.ts and update any explicit references to the old filename
while preserving the test contents.
In `@tools/egg-bundler/test/cluster-entries.realbuild.test.ts`:
- Around line 140-152: Update the worker startup wait around ThreadWorker and
once(worker, 'message') to race the message event with worker error and exit
events, rejecting or failing immediately when startup terminates without the
expected message. Preserve the existing message assertions and ensure the
finally block still terminates the worker.
🪄 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 Plus
Run ID: b28ca039-2ae2-48c9-8478-23b5e935c579
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (76)
AGENTS.mdpackages/cluster/package.jsonpackages/cluster/src/agent_worker.tspackages/cluster/src/app_worker.tspackages/cluster/src/index.tspackages/cluster/src/utils/mode/impl/process/agent.tspackages/cluster/src/utils/mode/impl/process/app.tspackages/cluster/src/utils/mode/impl/worker_threads/agent.tspackages/cluster/src/utils/mode/impl/worker_threads/app.tspackages/cluster/src/utils/options.tspackages/cluster/src/worker_protocol/agent.tspackages/cluster/src/worker_protocol/app.tspackages/cluster/src/worker_protocol/index.tspackages/cluster/src/worker_protocol/process.tspackages/cluster/src/worker_protocol/worker-thread.tspackages/cluster/test/fixtures/worker-thread-io.mjspackages/cluster/test/options.test.tspackages/cluster/test/worker_protocol.test.tspackages/cluster/test/worker_threads.test.tspackages/cluster/tsdown.config.tspackages/core/src/loader/egg_loader.tspackages/core/test/loader/egg_loader.test.tspackages/egg/src/lib/egg.tspackages/egg/test/snapshot.test.tspackages/loader-fs/README.mdpackages/loader-fs/src/index.tspackages/loader-fs/src/manifest_loader_fs.tspackages/loader-fs/test/manifest_loader_fs.test.tsplugins/watcher/src/lib/boot.tsplugins/watcher/test/boot.test.tssite/docs/advanced/snapshot-troubleshooting.mdsite/docs/advanced/snapshot.mdsite/docs/core/bundle.mdsite/docs/zh-CN/advanced/snapshot-troubleshooting.mdsite/docs/zh-CN/advanced/snapshot.mdsite/docs/zh-CN/core/bundle.mdtegg/core/loader/src/impl/ModuleLoader.tstegg/core/loader/test/ModuleLoaderLoaderFS.test.tstegg/plugin/tegg/package.jsontegg/plugin/tegg/src/agent.tstegg/plugin/tegg/src/app.tstegg/plugin/tegg/src/lib/EggModuleLoader.tstegg/plugin/tegg/src/lib/install-tegg-loader-fs.tstegg/plugin/tegg/test/agent-loader-fs.test.tstools/egg-bin/src/commands/bundle.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/test/commands/bundle.test.tstools/egg-bin/test/commands/snapshot.test.tstools/egg-bundler/README.mdtools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/compat/leoric/index.tstools/egg-bundler/src/compat/leoric/runtime-require-loader.cjstools/egg-bundler/src/index.tstools/egg-bundler/src/lib/Bundler.tstools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/src/lib/PackRunner.tstools/egg-bundler/src/lib/prelude.tstools/egg-bundler/test/Bundler.test.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/PackRunner.test.tstools/egg-bundler/test/cjs-esm-interop.realbuild.test.tstools/egg-bundler/test/cluster-entries.realbuild.test.tstools/egg-bundler/test/leoric-loader.realbuild.test.tstools/egg-bundler/test/leoric-runtime-require-loader.test.tstools/egg-bundler/test/prelude.test.tstools/egg-bundler/test/single-file.realbuild.test.tstools/egg-bundler/test/snapshot-lazy-bundler.test.tstools/egg-bundler/test/snapshot-lazy-external.test.tstools/egg-bundler/test/snapshot-lazy.realbuild.test.tstools/egg-bundler/tsdown.config.tstools/scripts/src/commands/start.tstools/scripts/test/snapshot-start.test.tswiki/index.mdwiki/log.mdwiki/packages/egg-bundler.mdwiki/packages/loader-fs.md
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 78 out of 78 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/cluster/src/worker_protocol/worker-thread.ts:39
gracefulExit()registers anexithandler declaredasyncand then callsprocess.exit(code)from inside theexitevent. Node does not run async work duringexit, sobeforeExitpromises will never be awaited, and re-callingprocess.exit()from withinexitis unnecessary and can cause recursion/extra exit events. This can preventapp.close()/agent.close()from running in worker_threads mode.
gracefulExit(options: GracefulExitOptions): void {
const { beforeExit } = options;
process.on('exit', async (code) => {
if (typeof beforeExit === 'function') {
await beforeExit();
}
process.exit(code);
});
},
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/egg-bin/src/commands/snapshot.ts (1)
171-187: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove stale blobs before each build.
fs.access()only confirms that a blob exists. Ifnode --build-snapshotexits without writing a new blob, a blob from an earlier build passes this check and can be deployed with a different worker entry. Remove each target blob before calling#spawnNode().Proposed fix
for (const entry of snapshotEntries) { + await fs.rm(entry.blobPath, { force: true }); await this.#spawnNode(['--snapshot-blob', entry.blobPath, '--build-snapshot', entry.workerPath]); }🤖 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 `@tools/egg-bin/src/commands/snapshot.ts` around lines 171 - 187, Remove each existing snapshot blob at entry.blobPath before invoking `#spawnNode` for that entry, so the subsequent fs.access validation confirms the current build produced it rather than reusing stale output. Keep the dry-run behavior and existing missing-blob error handling unchanged.
♻️ Duplicate comments (2)
tools/egg-bundler/src/lib/EntryGenerator.ts (1)
625-633: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftBoth cluster roles still receive the same shared entry list.
generate()collects oneentriesarray and passes it to#renderClusterWorkerEntry()for both roles.#renderBundleRuntime()emits a static import for every non-external entry, so an agent-only module executes its top-level effects inside the app worker, and an app-only module executes inside the agent worker. Split the entry collection by role, or add a real-build assertion that proves opposite-role modules do not execute.🤖 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 `@tools/egg-bundler/src/lib/EntryGenerator.ts` around lines 625 - 633, The cluster worker generation flow currently passes the same entries array to both roles, causing role-specific modules to be bundled and executed in the wrong worker. Update generate() and the surrounding entry collection to build role-specific entry lists, then pass the app entries only to `#renderClusterWorkerEntry`() with role "app" and agent entries only with role "agent"; alternatively, add a real-build assertion proving opposite-role modules are excluded from execution.packages/cluster/src/worker_protocol/worker-thread.ts (1)
31-39: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
gracefulExitstill performs asynchronous work inside aprocess.on('exit')listener.Node.js runs
exitlisteners synchronously and terminates immediately after they return. Theawait beforeExit()continuation is discarded, soapp.close()oragent.close()never completes. The trailingprocess.exit(code)inside anexitlistener is also a no-op. Trigger the shutdown from an explicit parent message or signal handler, awaitbeforeExit, then callprocess.exit.🤖 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 `@packages/cluster/src/worker_protocol/worker-thread.ts` around lines 31 - 39, The gracefulExit method attempts asynchronous work inside a process.on('exit') listener, but Node.js terminates synchronously after exit listeners return, so the await beforeExit() continuation and trailing process.exit(code) are never executed. Replace the exit listener pattern with an explicit signal handler (such as SIGTERM or SIGINT) or a message listener from the parent thread. In the new handler, await the beforeExit function to allow cleanup to complete, then call process.exit(code) after the cleanup promise resolves. This ensures graceful shutdown actually happens before the process terminates.
🧹 Nitpick comments (2)
tools/egg-bundler/test/cluster-entries.realbuild.test.ts (2)
140-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail fast when the worker thread errors.
once(worker, 'message')never settles if the worker throws during startup. The test then hangs until the 60 second timeout and reports a timeout instead of the real worker error. Race themessageevent against theerrorevent.♻️ Proposed change
try { - const [message] = await once(worker, 'message'); + const [message] = await Promise.race([ + once(worker, 'message'), + once(worker, 'error').then(([err]) => { + throw err; + }), + ]); expect(message).toMatchObject({🤖 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 `@tools/egg-bundler/test/cluster-entries.realbuild.test.ts` around lines 140 - 152, Update the worker startup await around ThreadWorker and once(worker, 'message') to race the expected message event against the worker's error event, rejecting immediately when startup fails. Preserve the existing message assertions and finally-block termination while ensuring worker errors surface directly instead of causing a timeout.
155-185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the app role in the snapshot assertions.
The snapshot build, restore,
options.requirerejection, and working-directory mismatch checks run only againstagent_worker.js. The app role generates a different runtime branch, including thestartAppWorkercall withmasterOptions. Loop over both roles, or add a short comment that states why the agent role alone is sufficient.The Node.js version gate also skips this block silently. Use
it.skipIf(Number(process.versions.node.split('.')[0]) < 24)on a separate test so the skip is visible in the report.🤖 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 `@tools/egg-bundler/test/cluster-entries.realbuild.test.ts` around lines 155 - 185, Expand the snapshot assertions around the agent worker flow to also cover the app-role runtime branch, including snapshot build/restore, unsupported require handling, and working-directory mismatch; reuse the appropriate generated app worker path and `startAppWorker` options. Move this coverage into a separate test guarded with `it.skipIf(Number(process.versions.node.split('.')[0]) < 24)` so unsupported Node versions report an explicit skip instead of silently bypassing the block.
🤖 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 `@tools/egg-bin/src/commands/snapshot.ts`:
- Around line 84-86: Update the description of the skip-bundle flag in the
snapshot command’s Flags.boolean configuration to state that it reuses
snapshot-ready worker entries from an earlier snapshot build, rather than
implying ordinary bundle entries are supported.
- Around line 127-146: Validate the resolved blob paths in the cluster branch of
the snapshot entry construction before building, and reject the configuration
when the app and agent paths are identical. Use the paths returned by
`#resolveClusterBlobPath` for each role, while preserving the existing single-role
behavior and distinct-path flow.
In `@tools/egg-bundler/test/snapshot-lazy.realbuild.test.ts`:
- Around line 155-186: Add a Node.js version check using an appropriate skip
gate (such as it.skipIf or a conditional within the test) before the snapshot
restore execution block that begins with the restored const declaration and the
RESTORE: marker. This should conditionally skip the restored probe assertions on
Node.js versions that do not support snapshot restore functionality, ensuring
the test does not attempt to execute process.execPath with --snapshot-blob on
unsupported versions.
In `@tools/scripts/src/commands/start.ts`:
- Around line 331-344: Update the validation near clusterSnapshotRequested in
the start command to reject --bundle when flags['snapshot-blob'] is also
provided, before selecting the snapshot launch branch. Fail fast with an
appropriate error and exit status, while preserving the existing options.require
validation for bundled cluster workers.
---
Outside diff comments:
In `@tools/egg-bin/src/commands/snapshot.ts`:
- Around line 171-187: Remove each existing snapshot blob at entry.blobPath
before invoking `#spawnNode` for that entry, so the subsequent fs.access
validation confirms the current build produced it rather than reusing stale
output. Keep the dry-run behavior and existing missing-blob error handling
unchanged.
---
Duplicate comments:
In `@packages/cluster/src/worker_protocol/worker-thread.ts`:
- Around line 31-39: The gracefulExit method attempts asynchronous work inside a
process.on('exit') listener, but Node.js terminates synchronously after exit
listeners return, so the await beforeExit() continuation and trailing
process.exit(code) are never executed. Replace the exit listener pattern with an
explicit signal handler (such as SIGTERM or SIGINT) or a message listener from
the parent thread. In the new handler, await the beforeExit function to allow
cleanup to complete, then call process.exit(code) after the cleanup promise
resolves. This ensures graceful shutdown actually happens before the process
terminates.
In `@tools/egg-bundler/src/lib/EntryGenerator.ts`:
- Around line 625-633: The cluster worker generation flow currently passes the
same entries array to both roles, causing role-specific modules to be bundled
and executed in the wrong worker. Update generate() and the surrounding entry
collection to build role-specific entry lists, then pass the app entries only to
`#renderClusterWorkerEntry`() with role "app" and agent entries only with role
"agent"; alternatively, add a real-build assertion proving opposite-role modules
are excluded from execution.
---
Nitpick comments:
In `@tools/egg-bundler/test/cluster-entries.realbuild.test.ts`:
- Around line 140-152: Update the worker startup await around ThreadWorker and
once(worker, 'message') to race the expected message event against the worker's
error event, rejecting immediately when startup fails. Preserve the existing
message assertions and finally-block termination while ensuring worker errors
surface directly instead of causing a timeout.
- Around line 155-185: Expand the snapshot assertions around the agent worker
flow to also cover the app-role runtime branch, including snapshot
build/restore, unsupported require handling, and working-directory mismatch;
reuse the appropriate generated app worker path and `startAppWorker` options.
Move this coverage into a separate test guarded with
`it.skipIf(Number(process.versions.node.split('.')[0]) < 24)` so unsupported
Node versions report an explicit skip instead of silently bypassing the block.
🪄 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 Plus
Run ID: 4fdb08a4-2789-4a5b-8af9-c160193c00bd
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (77)
.github/workflows/ci.ymlAGENTS.mdpackages/cluster/package.jsonpackages/cluster/src/agent_worker.tspackages/cluster/src/app_worker.tspackages/cluster/src/index.tspackages/cluster/src/utils/mode/impl/process/agent.tspackages/cluster/src/utils/mode/impl/process/app.tspackages/cluster/src/utils/mode/impl/worker_threads/agent.tspackages/cluster/src/utils/mode/impl/worker_threads/app.tspackages/cluster/src/utils/options.tspackages/cluster/src/worker_protocol/agent.tspackages/cluster/src/worker_protocol/app.tspackages/cluster/src/worker_protocol/index.tspackages/cluster/src/worker_protocol/process.tspackages/cluster/src/worker_protocol/worker-thread.tspackages/cluster/test/fixtures/worker-thread-io.mjspackages/cluster/test/options.test.tspackages/cluster/test/worker_protocol.test.tspackages/cluster/test/worker_threads.test.tspackages/cluster/tsdown.config.tspackages/core/src/loader/egg_loader.tspackages/core/test/loader/egg_loader.test.tspackages/egg/src/lib/egg.tspackages/egg/test/snapshot.test.tspackages/loader-fs/README.mdpackages/loader-fs/src/index.tspackages/loader-fs/src/manifest_loader_fs.tspackages/loader-fs/test/manifest_loader_fs.test.tsplugins/watcher/src/lib/boot.tsplugins/watcher/test/boot.test.tssite/docs/advanced/snapshot-troubleshooting.mdsite/docs/advanced/snapshot.mdsite/docs/core/bundle.mdsite/docs/zh-CN/advanced/snapshot-troubleshooting.mdsite/docs/zh-CN/advanced/snapshot.mdsite/docs/zh-CN/core/bundle.mdtegg/core/loader/src/impl/ModuleLoader.tstegg/core/loader/test/ModuleLoaderLoaderFS.test.tstegg/plugin/tegg/package.jsontegg/plugin/tegg/src/agent.tstegg/plugin/tegg/src/app.tstegg/plugin/tegg/src/lib/EggModuleLoader.tstegg/plugin/tegg/src/lib/install-tegg-loader-fs.tstegg/plugin/tegg/test/agent-loader-fs.test.tstools/egg-bin/src/commands/bundle.tstools/egg-bin/src/commands/snapshot.tstools/egg-bin/test/commands/bundle.test.tstools/egg-bin/test/commands/snapshot.test.tstools/egg-bundler/README.mdtools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/compat/leoric/index.tstools/egg-bundler/src/compat/leoric/runtime-require-loader.cjstools/egg-bundler/src/index.tstools/egg-bundler/src/lib/Bundler.tstools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/src/lib/PackRunner.tstools/egg-bundler/src/lib/prelude.tstools/egg-bundler/test/Bundler.test.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/PackRunner.test.tstools/egg-bundler/test/cjs-esm-interop.realbuild.test.tstools/egg-bundler/test/cluster-entries.realbuild.test.tstools/egg-bundler/test/leoric-loader.realbuild.test.tstools/egg-bundler/test/leoric-runtime-require-loader.test.tstools/egg-bundler/test/prelude.test.tstools/egg-bundler/test/single-file.realbuild.test.tstools/egg-bundler/test/snapshot-lazy-bundler.test.tstools/egg-bundler/test/snapshot-lazy-external.test.tstools/egg-bundler/test/snapshot-lazy.realbuild.test.tstools/egg-bundler/tsdown.config.tstools/scripts/src/commands/start.tstools/scripts/test/snapshot-start.test.tswiki/index.mdwiki/log.mdwiki/packages/egg-bundler.mdwiki/packages/loader-fs.md
🚧 Files skipped from review as they are similar to previous changes (54)
- tegg/plugin/tegg/test/agent-loader-fs.test.ts
- packages/loader-fs/README.md
- packages/loader-fs/src/manifest_loader_fs.ts
- tools/egg-bundler/tsdown.config.ts
- packages/cluster/src/utils/options.ts
- wiki/index.md
- packages/cluster/test/worker_threads.test.ts
- packages/cluster/test/worker_protocol.test.ts
- packages/cluster/tsdown.config.ts
- tools/egg-bundler/test/PackRunner.test.ts
- tools/egg-bundler/src/compat/leoric/runtime-require-loader.cjs
- packages/cluster/src/app_worker.ts
- packages/cluster/src/worker_protocol/index.ts
- tegg/plugin/tegg/src/lib/install-tegg-loader-fs.ts
- tools/egg-bundler/src/lib/prelude.ts
- packages/egg/src/lib/egg.ts
- packages/core/test/loader/egg_loader.test.ts
- packages/cluster/test/fixtures/worker-thread-io.mjs
- packages/cluster/src/worker_protocol/process.ts
- packages/cluster/src/index.ts
- .github/workflows/ci.yml
- packages/core/src/loader/egg_loader.ts
- tools/egg-bundler/test/prelude.test.ts
- packages/cluster/src/utils/mode/impl/process/app.ts
- tools/egg-bundler/src/lib/PackRunner.ts
- AGENTS.md
- plugins/watcher/test/boot.test.ts
- packages/cluster/src/worker_protocol/agent.ts
- tools/egg-bundler/README.md
- packages/cluster/src/utils/mode/impl/worker_threads/agent.ts
- tegg/plugin/tegg/src/lib/EggModuleLoader.ts
- tegg/plugin/tegg/package.json
- tegg/core/loader/test/ModuleLoaderLoaderFS.test.ts
- tools/egg-bundler/test/snapshot-lazy-external.test.ts
- plugins/watcher/src/lib/boot.ts
- tools/egg-bundler/src/lib/Bundler.ts
- tools/egg-bin/test/commands/snapshot.test.ts
- packages/loader-fs/test/manifest_loader_fs.test.ts
- tegg/core/loader/src/impl/ModuleLoader.ts
- packages/egg/test/snapshot.test.ts
- packages/cluster/src/agent_worker.ts
- packages/cluster/package.json
- tegg/plugin/tegg/src/agent.ts
- tools/egg-bin/src/commands/bundle.ts
- wiki/packages/egg-bundler.md
- packages/loader-fs/src/index.ts
- wiki/log.md
- tegg/plugin/tegg/src/app.ts
- tools/egg-bundler/src/compat/leoric/index.ts
- packages/cluster/src/utils/mode/impl/worker_threads/app.ts
- tools/egg-bundler/src/index.ts
- tools/egg-bundler/test/leoric-runtime-require-loader.test.ts
- tools/egg-bin/test/commands/bundle.test.ts
- tools/scripts/test/snapshot-start.test.ts
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 77 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
packages/cluster/src/worker_protocol/worker-thread.ts:37
process.on('exit', async ...)cannot reliably run an asyncbeforeExit(the event is synchronous and pending promises are not awaited). Additionally, callingprocess.exit(code)inside anexithandler is redundant and can lead to confusing re-entrancy. This currently makesbeforeExitbest-effort at best and may skip cleanup.
process.on('exit', async (code) => {
if (typeof beforeExit === 'function') {
await beforeExit();
}
process.exit(code);
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 78 out of 78 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/cluster/src/utils/options.ts:194
- The new validation for
appWorkerFile/agentWorkerFileonly checksexistsSync(), so a directory path (or other non-file) will pass validation but later fail when spawning the worker entry. Since the error message says “file should exist”, it should also verify the path is a regular file for clearer, earlier failures.
options[optionName] = path.resolve(options.baseDir!, workerFile);
assert(fs.existsSync(options[optionName]), `options.${optionName} file should exist: ${options[optionName]}`);
}
packages/cluster/src/utils/options.ts:203
appSnapshotBlob/agentSnapshotBlobvalidation also only checksexistsSync(). If a directory is provided by mistake, it will pass here but fail later when Node tries to use it as a snapshot blob. ValidatingstatSync(...).isFile()makes the contract explicit and errors clearer.
options[optionName] = path.resolve(options.baseDir!, snapshotBlob);
assert(fs.existsSync(options[optionName]), `options.${optionName} file should exist: ${options[optionName]}`);
assert(options.startMode !== 'worker_threads', `options.${optionName} only supports startMode "process"`);
bed5490 to
7ed8296
Compare
Summary
This PR adds end-to-end support for running Egg cluster applications from ordinary bundles and V8 startup snapshots.
worker_threadsmodesArtifact and launch model
worker.jsworker.jssnapshot.blobegg-scripts start --snapshot-blobapp_worker.js,agent_worker.jsegg-scripts start --bundleapp_worker.js,agent_worker.jsapp.snapshot.blob,agent.snapshot.blobegg-scripts start --bundlewith role blob flagsThe app and agent roles are baked into separate generated entries. Snapshot builds therefore create two independent V8 heaps instead of selecting a role through a runtime environment variable.
CLI
Build an ordinary cluster bundle:
Build role-specific cluster snapshots:
Start an ordinary cluster bundle:
Start with role-specific blobs:
--bundle-dirdefaults to./dist-bundle. App/agent worker files and blobs can be overridden independently.Implementation
Cluster workers
worker_threads.parentPortbehind transport adaptersBundle and snapshot entries
EntryGeneratorandPackRunnerto produce either one single worker or two self-contained cluster workersv8.startupSnapshot.isBuildingSnapshot()so a snapshot-capable bundle remains directly runnable without a blobrequire, loader hooks, web globals, lifecycle state, and cluster worker options before joining the master protocolcluster, network builtins,undici, andurlliblazy during snapshot constructionWhy Leoric needs a compatibility loader
Keeping all of Leoric external does not preserve ORM identity. Application and TEGG model modules are evaluated while the snapshot is built. If Leoric is represented by a lazy external proxy, model subclasses, prototype chains, and ORM metadata are frozen against that proxy. Restore can forward later member access to the real package, but it cannot replace prototype and metadata identity already serialized into the heap. Making every model runtime-lazy would avoid the proxy boundary, but would move the ORM model graph out of the snapshot and require Leoric/TEGG-specific lazy-loading behavior.
Inlining unmodified Leoric is not reliable either. Known driver, realm, and migration modules use expression-based CommonJS
require(...)calls for optional database clients and filesystem-discovered modules.@utoo/packcannot statically enumerate those expressions, so a self-contained build would otherwise fail or retain runtime loads that are absent from the bundle graph.The current solution is a scoped compromise. Snapshot mode forces the Leoric core into the bundle, applies a loader only to known Leoric source files, and rewrites the known runtime-selected requires to
globalThis.__RUNTIME_REQUIRE(...). Database clients remain runtime dependencies, and an unrecognized expression-based require fails the build instead of silently producing a partial artifact.Trade-offs: the loader is package- and version-sensitive. Changes to the Leoric file layout or require expressions can invalidate the path condition or rewrite patterns, so Leoric upgrades must pass the focused unit and real-build tests. Runtime migrations still require the documented asset-copy configuration. This shim should be removed when Leoric no longer needs expression-based runtime loading, or when
@utoo/packprovides an equivalent supported dynamic-require contract without modifying Turbopack.Lifecycle and loader integration
configWillLoadand resumes atconfigDidLoadconfigDidLoador laterLoaderFS.getKnownFilesCompatibility and constraints
worker_threads; custom snapshot blobs remain process-only because Node does not expose a per-Worker snapshot-blob APIoptions.requireinstead of silently ignoring bootstrap modulesValidation
Targeted repository validation:
@eggjs/cluster: 36 passed, 1 skipped@eggjs/binbundle/snapshot commands: 20 passed@eggjs/scriptssnapshot start: 17 passedgit diff --checkpassesThe no-source CNPMCore business smoke covered:
worker_threadscluster bundleEach supported path exercised admin authentication, npm publish, package metadata lookup, tarball download/content, dist-tag mutation, and unpublish. Source startup and health checks also passed in single, process-cluster, and worker-thread modes.
Documentation
The English and Chinese documentation now describe: