fix(bundler): map runtime paths for bundled workers#5921
Conversation
|
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 worker entry now derives runtime __outputDir from the executed entry (process.argv[1]), roots createRequire to __outputDir, and embeds a normalized inlined bundle map with relKey, output-dir absolute, original-app absolute, and manifest resolveCache aliases. startEgg is invoked as startEgg({ baseDir: __outputDir, framework, mode: 'single' }). ChangesRuntime Path Mapping (Output Directory Separation)
Sequence Diagram(s)sequenceDiagram
participant Worker as Worker entry
participant BundleMap as Inlined __BUNDLE_MAP
participant Manifest as ManifestStore
participant Egg as startEgg
Worker->>BundleMap: populate normalized keys & aliases (relKey, outputPath, appPath)
Worker->>Manifest: ManifestStore.fromBundle(__outputDir, MANIFEST_DATA)
Worker->>Egg: startEgg({ baseDir: __outputDir, framework, mode: 'single' })
Egg->>BundleMap: request module (relKey / output-abs / app-abs / resolveCache)
BundleMap-->>Egg: return module
Egg->>Worker: initialize app runtime
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Deploying egg with
|
| Latest commit: |
7558a04
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2324cb79.egg-cci.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-b5dba477.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5921 +/- ##
=======================================
Coverage 85.03% 85.04%
=======================================
Files 667 667
Lines 19123 19128 +5
Branches 3723 3725 +2
=======================================
+ Hits 16262 16268 +6
+ Misses 2468 2467 -1
Partials 393 393 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying egg-v3 with
|
| Latest commit: |
7558a04
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a2ad4e79.egg-v3.pages.dev |
| Branch Preview URL: | https://agent-egg-dev-b5dba477.egg-v3.pages.dev |
There was a problem hiding this comment.
Code Review
This pull request updates the egg-bundler to distinguish between the runtime output directory and the original application base directory. The generated worker entry now keys the bundle map using relative paths, output-dir absolute paths, original app absolute paths, and manifest resolveCache aliases to improve module resolution. Feedback was provided to optimize the __setBundleAliases function by skipping redundant path resolutions when the input is already an absolute path.
There was a problem hiding this comment.
Pull request overview
This PR updates the egg bundler’s generated worker bootstrap so bundled apps run from the deploy output directory while still recognizing module lookups that refer back to the original app paths. It primarily targets bundled-worker runtime resolution and keeps the surrounding tests/docs in sync.
Changes:
- Split bundled worker runtime
baseDirhandling into deployoutputDirvs original appbaseDir. - Expanded bundle module loader keying to cover relKeys, output-dir absolute paths, original app absolute paths, and
resolveCachealiases. - Updated EntryGenerator tests, deterministic expectations, snapshot output, and bundler documentation/wiki notes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wiki/packages/egg-bundler.md | Documents the new bundled worker runtime path-mapping model. |
| wiki/log.md | Records the wiki/documentation update for the bundler runtime change. |
| tools/egg-bundler/test/deterministic.test.ts | Adjusts determinism expectations to ignore embedded app baseDir literals. |
| tools/egg-bundler/test/snapshots/EntryGenerator.worker.canonical.snap | Updates the canonical generated worker snapshot to the new runtime bootstrap shape. |
| tools/egg-bundler/test/EntryGenerator.test.ts | Revises unit tests for generated worker content and new bundle-map aliasing behavior. |
| tools/egg-bundler/src/lib/EntryGenerator.ts | Implements the new worker entry generation logic for runtime/output path mapping. |
| tools/egg-bundler/docs/output-structure.md | Updates public docs for how bundled workers resolve modules at runtime. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/egg-bundler/src/lib/EntryGenerator.ts`:
- Around line 197-198: The code currently serializes the whole this.#baseDir
into appBaseDir (JSON.stringify(this.#baseDir)) which bakes a machine-specific
checkout path into the emitted runtime; remove that serialization and instead
derive and serialize only the concrete absolute aliases the runtime actually
needs (e.g., specific entry file, asset or handler paths). Update EntryGenerator
to stop emitting __appBaseDir (and any use of this.#baseDir in the generated
source around the appBaseDir and the similar block at lines 227-245) and replace
it with JSON.stringify(...) of the exact resolved paths you need (or relative
paths computed from the bundle root), keeping framework import specifier
generation via this.#toFrameworkImportSpecifier() unchanged. Ensure no full
checkout root string is present in generated output.
🪄 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: 157fd634-0b78-498b-978b-461f08c99e34
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
tools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/deterministic.test.tswiki/log.mdwiki/packages/egg-bundler.md
3d3db03 to
1465573
Compare
1465573 to
553c763
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/egg-bundler/test/deterministic.test.ts`:
- Around line 201-203: normalizeWorkerAppBaseDir currently only replaces the
platform-specific baseDir, so POSIX-normalized embedded paths (with '/') are not
replaced on Windows; update normalizeWorkerAppBaseDir to also compute a
POSIX-variant of baseDir (e.g. replace path.sep with '/') and replace both the
raw baseDir and the posixBaseDir with '<baseDir>' (call out the function name
normalizeWorkerAppBaseDir). Also change the test assertions (the assertions
around the normalized entry content) to be platform-agnostic by checking that
the resulting string contains '<baseDir>' (and/or does not contain either
baseDir or its posix variant) instead of asserting the platform-specific path
literal.
In `@tools/egg-bundler/test/EntryGenerator.test.ts`:
- Around line 193-196: The tests assert against raw path.join(tmpDir, ...) which
yields platform-specific backslashes on Windows while the generated worker
output normalizes paths to POSIX (via __normalizeBundleKey()), causing failures;
update the assertions that reference path.join(tmpDir, ...) (the expect lines
checking for controller/controller.ts and any other tmpDir assertions) to
normalize the expected paths to POSIX form (e.g., convert backslashes to forward
slashes or use path.posix.join) before JSON.stringify/comparison, and change the
snapshot regexp replacement (the regex around tmpDir at the later replacement)
to match both native and POSIX variants of tmpDir so snapshots are stable across
platforms. Ensure you modify the specific test expectations and the regex used
to scrub tmpDir in EntryGenerator.test.ts.
🪄 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: 6673d79a-34ca-4e54-bde3-50e53ddbbb45
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
tools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/deterministic.test.tswiki/log.mdwiki/packages/egg-bundler.md
✅ Files skipped from review due to trivial changes (2)
- tools/egg-bundler/docs/output-structure.md
- wiki/log.md
🚧 Files skipped from review as they are similar to previous changes (2)
- wiki/packages/egg-bundler.md
- tools/egg-bundler/src/lib/EntryGenerator.ts
553c763 to
55cca76
Compare
55cca76 to
b3b9c03
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tools/egg-bundler/test/EntryGenerator.test.ts (1)
200-202:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNormalize expected paths to POSIX form for cross-platform compatibility.
The assertions use
path.join(tmpDir, ...)which produces platform-specific paths. On Windows, this generates backslash paths (e.g.,"C:\\Users\\...\\app\\controller.ts"), but the generator normalizes all paths to forward slashes via#normalizeKey. Tests will fail on Windows CI.🛠️ Proposed fix
+ const toPosix = (p: string) => p.split(path.sep).join('/'); expect(worker).toContain('__APP_ABSOLUTE_ALIASES'); - expect(worker).toContain(JSON.stringify(path.join(tmpDir, 'app/controller.ts'))); + expect(worker).toContain(JSON.stringify(toPosix(path.join(tmpDir, 'app/controller.ts')))); expect(worker).toContain('__APP_RESOLVE_CACHE_ALIASES'); - expect(worker).toContain(JSON.stringify(path.join(tmpDir, 'app/controller'))); + expect(worker).toContain(JSON.stringify(toPosix(path.join(tmpDir, 'app/controller'))));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/egg-bundler/test/EntryGenerator.test.ts` around lines 200 - 202, The test assertions fail on Windows because path.join(tmpDir, ...) produces backslashes while the generator outputs normalized POSIX paths (see `#normalizeKey`); update the expectations in EntryGenerator.test.ts so the path strings are normalized to POSIX before comparing (e.g., convert path.join(...) results to forward-slash form or use path.posix to build them) for the three assertions referencing worker and the controller paths and the '__APP_RESOLVE_CACHE_ALIASES' check so they match the generator's normalized output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tools/egg-bundler/test/EntryGenerator.test.ts`:
- Around line 200-202: The test assertions fail on Windows because
path.join(tmpDir, ...) produces backslashes while the generator outputs
normalized POSIX paths (see `#normalizeKey`); update the expectations in
EntryGenerator.test.ts so the path strings are normalized to POSIX before
comparing (e.g., convert path.join(...) results to forward-slash form or use
path.posix to build them) for the three assertions referencing worker and the
controller paths and the '__APP_RESOLVE_CACHE_ALIASES' check so they match the
generator's normalized output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f749be3c-ad25-48c3-8e7a-f4bc8958c6f7
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
tools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/deterministic.test.tswiki/log.mdwiki/packages/egg-bundler.md
✅ Files skipped from review due to trivial changes (2)
- wiki/packages/egg-bundler.md
- tools/egg-bundler/test/deterministic.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- wiki/log.md
- tools/egg-bundler/docs/output-structure.md
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b1dd4b86-7dbe-4a9d-adf7-e589004269d4
⛔ Files ignored due to path filters (1)
tools/egg-bundler/test/__snapshots__/EntryGenerator.worker.canonical.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
tools/egg-bundler/docs/output-structure.mdtools/egg-bundler/src/lib/EntryGenerator.tstools/egg-bundler/test/EntryGenerator.test.tstools/egg-bundler/test/deterministic.test.tswiki/log.mdwiki/packages/egg-bundler.md
✅ Files skipped from review due to trivial changes (2)
- wiki/log.md
- wiki/packages/egg-bundler.md
🚧 Files skipped from review as they are similar to previous changes (2)
- tools/egg-bundler/docs/output-structure.md
- tools/egg-bundler/test/deterministic.test.ts
b3b9c03 to
e067f3a
Compare
## Summary - Stack on top of PR #5921 / EGG-65 runtime mapping commit because that dependency is not yet in origin/next. - Complete generated startup manifests with convention-based plugin dynamic files (agent, app, app/extend/*, and app/middleware) so bundled single-mode workers can load files skipped by metadataOnly agent startup. - Add a security-like @eggjs/security fixture verifying agent, app, app/extend/agent, app/extend/application, and app/middleware/securities are present in the manifest. ## Validation - pnpm exec vitest run packages/core/test/loader/manifest_coverage.test.ts - pnpm exec vitest run packages/core/test/loader/manifest.test.ts packages/core/test/loader/manifest_roundtrip.test.ts packages/core/test/loader/manifest_fingerprint.test.ts packages/core/test/loader/manifest_query.test.ts packages/core/test/loader/manifest_coverage.test.ts - pnpm --filter @eggjs/egg-bundler test -- EntryGenerator.test.ts - pnpm --filter @eggjs/core typecheck - pnpm --filter @eggjs/egg-bundler typecheck - pnpm exec oxfmt --check packages/core/src/loader/egg_loader.ts packages/core/test/loader/manifest_coverage.test.ts tools/egg-bundler/src/lib/EntryGenerator.ts tools/egg-bundler/test/EntryGenerator.test.ts - git diff --check origin/next...HEAD Notes: targeted oxlint on changed files exits 0 with existing warnings in pre-existing egg_loader.ts lines outside this patch. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Manifest post-processing now caches convention-based module resolution and records directory-discovered middleware lists for metadata-only runs. * Bundler/runtime treats framework as a package specifier, validates specifiers, and precomputes output↔original-app aliasing for deterministic module lookup. * **Documentation** * Updated bundler docs, CLI help, and wiki to explain runtime path mapping, output-dir semantics, and framework-specifier behavior. * **Tests & Chores** * Added fixtures and tests for manifest resolve-cache, file discovery, deterministic worker generation, and framework-specifier validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Tests
Summary by CodeRabbit
Documentation
Bug Fixes
Tests