fix(bundler): absolutize tegg manifest paths in bundled runtime#5973
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe PR fixes manifest-derived module reference path resolution in two places: ChangesManifest Module Reference Path Absolutization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the tegg module path resolution logic to correctly handle relative paths in bundled environments by resolving them directly against baseDir instead of baseDir/config. It also updates the egg-bundler to emit a runtime block in the generated worker entry that resolves relative manifest paths to absolute paths under the output directory. The review feedback suggests adding defensive checks to this runtime block to prevent potential runtime crashes when mutating the manifest data.
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.
| const __teggExt: any = (MANIFEST_DATA as any).extensions?.tegg; | ||
| if (__teggExt) { | ||
| const __toAbs = (p: string) => (path.isAbsolute(p) ? p : path.resolve(__outputDir, p)); | ||
| if (Array.isArray(__teggExt.moduleReferences)) { | ||
| for (const __ref of __teggExt.moduleReferences) __ref.path = __toAbs(__ref.path); | ||
| } | ||
| if (Array.isArray(__teggExt.moduleDescriptors)) { | ||
| for (const __desc of __teggExt.moduleDescriptors) __desc.unitPath = __toAbs(__desc.unitPath); | ||
| } | ||
| } |
There was a problem hiding this comment.
In the generated worker entry, the manifest data is cast to any and mutated at runtime. To prevent potential runtime crashes (e.g., TypeError if __ref or __desc is null/undefined, or if path/unitPath is not a string), we should add defensive checks before accessing and resolving these properties.
| const __teggExt: any = (MANIFEST_DATA as any).extensions?.tegg; | |
| if (__teggExt) { | |
| const __toAbs = (p: string) => (path.isAbsolute(p) ? p : path.resolve(__outputDir, p)); | |
| if (Array.isArray(__teggExt.moduleReferences)) { | |
| for (const __ref of __teggExt.moduleReferences) __ref.path = __toAbs(__ref.path); | |
| } | |
| if (Array.isArray(__teggExt.moduleDescriptors)) { | |
| for (const __desc of __teggExt.moduleDescriptors) __desc.unitPath = __toAbs(__desc.unitPath); | |
| } | |
| } | |
| const __teggExt: any = (MANIFEST_DATA as any).extensions?.tegg; | |
| if (__teggExt) { | |
| const __toAbs = (p: string) => (typeof p === 'string' ? (path.isAbsolute(p) ? p : path.resolve(__outputDir, p)) : p); | |
| if (Array.isArray(__teggExt.moduleReferences)) { | |
| for (const __ref of __teggExt.moduleReferences) { | |
| if (__ref) __ref.path = __toAbs(__ref.path); | |
| } | |
| } | |
| if (Array.isArray(__teggExt.moduleDescriptors)) { | |
| for (const __desc of __teggExt.moduleDescriptors) { | |
| if (__desc) __desc.unitPath = __toAbs(__desc.unitPath); | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Good catch — applied defensive guards in the generated worker block: __toAbs now no-ops on non-string values (typeof p === 'string'), and both loops skip null/undefined entries (if (__ref) / if (__desc)). Snapshot and the EntryGenerator assertions updated accordingly. Done in 096bf27.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5973 +/- ##
=======================================
Coverage 85.57% 85.57%
=======================================
Files 669 669
Lines 19849 19850 +1
Branches 3923 3924 +1
=======================================
+ Hits 16985 16986 +1
Misses 2475 2475
Partials 389 389 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Deploying egg with
|
| Latest commit: |
096bf27
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://96fed1de.egg-cci.pages.dev |
| Branch Preview URL: | https://fix-tegg-manifest-abs-path.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
096bf27
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://eaa629fb.egg-v3.pages.dev |
| Branch Preview URL: | https://fix-tegg-manifest-abs-path.egg-v3.pages.dev |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
killagu
force-pushed
the
fix/tegg-manifest-abs-path
branch
from
ba024fe to
d66b90d
Compare
## Motivation In bundle mode the tegg manifest stores `moduleReferences[].path` and `moduleDescriptors[].unitPath` relative to baseDir (normalized by #5932). `LoaderFactory.loadApp` matches a module reference to its descriptor by exact path equality, and the descriptor carries the precomputed `decoratedFiles`. For that match to feed the precomputed files into the loader, both sides must agree on the same absolute-path contract that a non-bundle run sees. ## Scope - EntryGenerator: the generated worker entry now resolves both `extensions.tegg.moduleReferences[].path` and `moduleDescriptors[].unitPath` to absolute paths under the runtime `__outputDir` (`path.isAbsolute(p) ? p : path.resolve(__outputDir, p)`). The bundler copies each module's package.json under `__outputDir`, so these resolve correctly. Canonical worker snapshot updated to match. - tegg-config App#loadModuleConfigs: resolve `reference.path` against baseDir directly instead of `ModuleConfigUtil.resolveModuleDir`, which joins relative paths under `baseDir/config` (the `config/module.json` convention) — wrong for manifest-sourced references. Manifest reference paths are kept as-is in `#scanModuleReferences` so they keep matching the descriptor unitPath keys. Non-bundle module references are always absolute (ModuleScanner / `config/module.json` are resolved at read time), so their behavior is unchanged: both the old and new code pass absolute paths through untouched. This deliberately does NOT touch `ModuleLoader.ts` or introduce any global bundle-store direct read; that loader-fs path is handled separately. ## Tests - tools/egg-bundler EntryGenerator.test.ts: relative tegg manifest paths emit the absolutization block and collect controller/repository decorated files. - tegg/plugin/config ManifestModuleReference.test.ts: a manifest-relative reference resolves under baseDir (regression: previously ENOENT under baseDir/config); an absolute reference is passed through unchanged. Verified: egg-bundler EntryGenerator suite, tegg/plugin/config (5 tests), tegg/core/loader (16 tests); lint, fmtcheck, typecheck clean on touched files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
killagu
force-pushed
the
fix/tegg-manifest-abs-path
branch
from
d66b90d to
096bf27
Compare
Motivation
In bundle mode the tegg manifest stores
moduleReferences[].pathandmoduleDescriptors[].unitPathrelative to baseDir (normalized by #5932).LoaderFactory.loadAppmatches a module reference to its descriptor by exact path equality, and the descriptor carries the precomputeddecoratedFiles. For that match to feed the precomputed files into the loader, both sides must agree on the same absolute-path contract that a non-bundle run sees — so the decorated controller/repository files flow through the manifest descriptor intoLoaderFactory.loadAppwithout any global-store direct read.Scope
Two small, self-contained changes:
tools/egg-bundlerEntryGenerator — the generated worker entry now resolves bothextensions.tegg.moduleReferences[].pathandmoduleDescriptors[].unitPathto absolute paths under the runtime__outputDir:The bundler copies each module's
package.jsonunder__outputDir, so these resolve correctly. Canonical worker snapshot updated to match.tegg/plugin/configApp#loadModuleConfigs— resolvereference.pathagainstbaseDirdirectly instead ofModuleConfigUtil.resolveModuleDir, which joins relative paths underbaseDir/config(theconfig/module.jsonconvention) — wrong for manifest-sourced references. Manifest reference paths are kept as-is in#scanModuleReferencesso they keep matching the descriptorunitPathkeys.Non-bundle behavior is unchanged
Non-bundle module references are always absolute (
ModuleScanner/config/module.jsonare resolved at read time viapath.join(configDir, ...)), so both the oldresolveModuleDirand the new code pass them through untouched.Out of scope
This deliberately does not touch
ModuleLoader.tsor introduce any global bundle-store direct read; that loader-fs path is handled separately. It also does not overlap with #5932 — that PR normalized the manifest paths to relative form; this PR re-absolutizes them at runtime so every consumer sees the same contract.Tests
tools/egg-bundler/test/EntryGenerator.test.ts— relative tegg manifest paths emit the absolutization block and collect controller/repository decorated files.tegg/plugin/config/test/ManifestModuleReference.test.ts— a manifest-relative reference resolves underbaseDir(regression: previouslyENOENTunderbaseDir/config); an absolute reference is passed through unchanged.Verified locally:
EntryGeneratorsuite (new + existing tests pass; the 8 pre-existing/private/varrealpath failures are macOS-local and present onnext)tegg/plugin/config(5 tests) andtegg/core/loader(16 tests)oxlint --type-aware,oxfmt --check, andtsgo --noEmitclean on all touched files🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
moduleReferences.pathandmoduleDescriptors.unitPathunder the output directory, without altering already-absolute values.Tests