feat(core): @Override / @ConditionalOnMissing proto precedence#6036
feat(core): @Override / @ConditionalOnMissing proto precedence#6036gxkl wants to merge 2 commits into
Conversation
Adds two proto-level decorators for deterministic override of a same-name proto in the IoC container, resolved at graph-build time before instantiation: - @ConditionalOnMissing marks a conditional default — used only when it is the sole candidate for its name; dropped when any other proto provides the name. - @OverRide marks an explicit replacement — it wins over a same-name plain or conditional proto. Precedence per name group: @OverRide > plain > @ConditionalOnMissing. The losers are pruned from the proto graph (and their module's proto list) in GlobalGraph.build() before any inject edge or instantiation, so a replaced default never runs its constructor / lifecycle side effects — unlike a select-at-injection model. Groups with no marker are untouched (existing resolution / MultiPrototypeFound preserved); ties within the winning tier fall back to normal qualifier disambiguation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds ChangesProto override precedence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ProtoClass
participant ProtoDescriptorHelper
participant GlobalGraph
participant Graph
ProtoClass->>ProtoDescriptorHelper: expose override metadata
ProtoDescriptorHelper->>GlobalGraph: create flagged proto descriptor
GlobalGraph->>GlobalGraph: identify competing protos
GlobalGraph->>Graph: remove losing vertices
GlobalGraph->>Graph: build dependency edges for winners
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. 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: |
9a9d42c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d0bb1c2c.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-tegg-proto-fallback-pri.egg-cci.pages.dev |
![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 introduces proto override precedence in the tegg IoC container using the @Override and @ConditionalOnMissing decorators. This feature allows deterministic same-name proto overrides (with precedence @Override > plain > @ConditionalOnMissing) by pruning the losing prototypes from the global graph before instantiation. The competition is scoped by name, init type, user qualifiers, and access level. The PR also includes comprehensive unit tests and documentation detailing the feature. There are no review comments provided, so I have no feedback to offer.
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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## next #6036 +/- ##
==========================================
+ Coverage 82.43% 82.48% +0.04%
==========================================
Files 686 688 +2
Lines 21335 21399 +64
Branches 4260 4272 +12
==========================================
+ Hits 17587 17650 +63
- Misses 3241 3242 +1
Partials 507 507 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Deploying egg-v3 with
|
| Latest commit: |
9a9d42c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://692f02d0.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-tegg-proto-fallback-pri.egg-v3.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
wiki/concepts/proto-override-precedence.md (1)
56-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFenced code block missing a language tag (MD040).
📝 Proposed fix
-``` +```text `@Override` > plain proto > `@ConditionalOnMissing`</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@wiki/concepts/proto-override-precedence.mdat line 56, Add the appropriate
language tag to the fenced code block containing the override precedence
example, using text for this plain-text content and preserving the example
unchanged.</details> <!-- cr-comment:v1:40a93762f9f295942472f4b6 --> _Source: Linters/SAST tools_ </blockquote></details> <details> <summary>tegg/core/metadata/src/model/graph/GlobalGraph.ts (1)</summary><blockquote> `144-209`: _🩺 Stability & Availability_ | _🔵 Trivial_ | _⚡ Quick win_ **Precedence pruning logic is correct; consider multi-app regression coverage.** The tiering (`override` > `plain` > `conditionalOnMissing`) and pre-edge-construction pruning is sound and matches the documented semantics. Since `GlobalGraph` instances are per-app (backed by `TeggScope`) and `#pruneOverriddenProtos` now directly determines which protos get instantiated/run lifecycle hooks, this is loader/runtime-affecting behavior. Consider adding a multi-app regression test (e.g. two apps where one defines `@Override`/`@ConditionalOnMissing` protos and the other doesn't) to confirm pruning in one app's graph never leaks into or is influenced by another concurrent app's graph. As per coding guidelines, "When changing loader, runtime, lifecycle, or eventbus behavior, add or update multi-app regression coverage (for example tests like `MultiApp.test.ts`) to verify two concurrent apps do not cross-talk." <details> <summary>🤖 Prompt for AI Agents</summary>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/core/metadata/src/model/graph/GlobalGraph.tsaround lines 144 - 209,
Add multi-app regression coverage for#pruneOverriddenProtosand
GlobalGraph.build, creating two concurrent app scopes where only one app defines
competing@Override/@ConditionalOnMissing protos. Assert each app instantiates
and runs lifecycle hooks only for its own correctly pruned protos, confirming
pruning and precedence state do not cross-talk between GlobalGraph instances.</details> <!-- cr-comment:v1:a027dbeef48b04fc93a02462 --> _Source: Coding guidelines_ </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@tegg/core/core-decorator/src/decorator/ConditionalOnMissing.ts:
- Line 3: Replace the TypeScript extensions with .js in the affected ESM import
specifiers: PrototypeUtil imports in ConditionalOnMissing.ts and Override.ts,
decorator imports in decorator/index.ts, and the src/index import in
decorators.test.ts. No other changes are needed.In
@wiki/concepts/proto-override-precedence.md:
- Around line 5-12: Update the wiki front matter to include both directly cited
ProtoDescriptor source files:
tegg/core/types/src/metadata/model/ProtoDescriptor.ts and
tegg/core/metadata/src/model/ProtoDescriptor/AbstractProtoDescriptor.ts. Mark
the Spring Boot, Guice, and Spring@Primarycomparison in the cross-framework
discussion as “Inference:” to distinguish synthesis from claims supported by the
listed sources.
Nitpick comments:
In@tegg/core/metadata/src/model/graph/GlobalGraph.ts:
- Around line 144-209: Add multi-app regression coverage for
#pruneOverriddenProtosand GlobalGraph.build, creating two concurrent app scopes
where only one app defines competing@Override/@ConditionalOnMissing protos.
Assert each app instantiates and runs lifecycle hooks only for its own correctly
pruned protos, confirming pruning and precedence state do not cross-talk between
GlobalGraph instances.In
@wiki/concepts/proto-override-precedence.md:
- Line 56: Add the appropriate language tag to the fenced code block containing
the override precedence example, using text for this plain-text content and
preserving the example unchanged.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `cea645de-55d3-4b36-9ccb-d156f1a3249e` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 8d7f6c78a550760d19d527ffc06671be6ca668e0 and 9a9d42c72fbcad42a22fe2782e31a8bf2276d60a. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `tegg/core/core-decorator/test/__snapshots__/index.test.ts.snap` is excluded by `!**/*.snap` * `tegg/core/tegg/test/__snapshots__/exports.test.ts.snap` is excluded by `!**/*.snap` </details> <details> <summary>📒 Files selected for processing (14)</summary> * `tegg/core/common-util/src/Graph.ts` * `tegg/core/core-decorator/src/decorator/ConditionalOnMissing.ts` * `tegg/core/core-decorator/src/decorator/Override.ts` * `tegg/core/core-decorator/src/decorator/index.ts` * `tegg/core/core-decorator/src/util/PrototypeUtil.ts` * `tegg/core/core-decorator/test/decorators.test.ts` * `tegg/core/metadata/src/model/ProtoDescriptor/AbstractProtoDescriptor.ts` * `tegg/core/metadata/src/model/ProtoDescriptorHelper.ts` * `tegg/core/metadata/src/model/graph/GlobalGraph.ts` * `tegg/core/metadata/test/GlobalGraph.test.ts` * `tegg/core/types/src/metadata/model/ProtoDescriptor.ts` * `wiki/concepts/proto-override-precedence.md` * `wiki/index.md` * `wiki/log.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| @@ -0,0 +1,22 @@ | |||
| import type { EggProtoImplClass } from '@eggjs/tegg-types'; | |||
|
|
|||
| import { PrototypeUtil } from '../util/PrototypeUtil.ts'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use .js extensions in module specifiers.
As per coding guidelines, Tegg TypeScript code must use ESM-only imports and .js extensions in import/export specifiers. Update the newly added or modified lines to use .js instead of .ts.
tegg/core/core-decorator/src/decorator/ConditionalOnMissing.ts#L3-L3: Change../util/PrototypeUtil.tsto../util/PrototypeUtil.js.tegg/core/core-decorator/src/decorator/Override.ts#L3-L3: Change../util/PrototypeUtil.tsto../util/PrototypeUtil.js.tegg/core/core-decorator/src/decorator/index.ts#L1-L1: Change./ConditionalOnMissing.tsto./ConditionalOnMissing.js.tegg/core/core-decorator/src/decorator/index.ts#L12-L12: Change./Override.tsto./Override.js.tegg/core/core-decorator/test/decorators.test.ts#L16-L16: Change../src/index.tsto../src/index.js.
📍 Affects 4 files
tegg/core/core-decorator/src/decorator/ConditionalOnMissing.ts#L3-L3(this comment)tegg/core/core-decorator/src/decorator/Override.ts#L3-L3tegg/core/core-decorator/src/decorator/index.ts#L1-L1tegg/core/core-decorator/src/decorator/index.ts#L12-L12tegg/core/core-decorator/test/decorators.test.ts#L16-L16
🤖 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/core/core-decorator/src/decorator/ConditionalOnMissing.ts` at line 3,
Replace the TypeScript extensions with .js in the affected ESM import
specifiers: PrototypeUtil imports in ConditionalOnMissing.ts and Override.ts,
decorator imports in decorator/index.ts, and the src/index import in
decorators.test.ts. No other changes are needed.
Source: Coding guidelines
| source_files: | ||
| - tegg/core/core-decorator/src/decorator/Override.ts | ||
| - tegg/core/core-decorator/src/decorator/ConditionalOnMissing.ts | ||
| - tegg/core/core-decorator/src/util/PrototypeUtil.ts | ||
| - tegg/core/metadata/src/model/graph/GlobalGraph.ts | ||
| - tegg/core/metadata/src/model/ProtoDescriptorHelper.ts | ||
| - tegg/core/common-util/src/Graph.ts | ||
| - tegg/core/metadata/test/GlobalGraph.test.ts |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
source_files is missing directly-cited files; cross-framework analogy isn't labeled Inference:.
The "Implementation surface" section cites override?/conditionalOnMissing? "on ProtoDescriptor", but neither tegg/core/types/src/metadata/model/ProtoDescriptor.ts nor tegg/core/metadata/src/model/ProtoDescriptor/AbstractProtoDescriptor.ts are listed in source_files. Separately, the Spring Boot @ConditionalOnMissingBean / Guice OptionalBinder.setDefault / Spring @Primary comparison (lines 78-84) is synthesis from outside the listed sources and should be flagged as such.
As per coding guidelines: "Every nontrivial wiki claim should be traceable to raw sources, list major source paths in source_files, label non-obvious synthesis as Inference:".
📝 Proposed fix
source_files:
- tegg/core/core-decorator/src/decorator/Override.ts
- tegg/core/core-decorator/src/decorator/ConditionalOnMissing.ts
- tegg/core/core-decorator/src/util/PrototypeUtil.ts
- tegg/core/metadata/src/model/graph/GlobalGraph.ts
- tegg/core/metadata/src/model/ProtoDescriptorHelper.ts
+ - tegg/core/types/src/metadata/model/ProtoDescriptor.ts
+ - tegg/core/metadata/src/model/ProtoDescriptor/AbstractProtoDescriptor.ts
- tegg/core/common-util/src/Graph.ts
- tegg/core/metadata/test/GlobalGraph.test.ts-Pruning ensures a replaced default never runs
-its constructor or lifecycle at all — matching `@ConditionalOnMissingBean`
-(Spring Boot) / `OptionalBinder.setDefault` (Guice) semantics rather than Spring's
-selection-only `@Primary`.
+Pruning ensures a replaced default never runs
+its constructor or lifecycle at all.
+
+Inference: this matches `@ConditionalOnMissingBean` (Spring Boot) /
+`OptionalBinder.setDefault` (Guice) semantics rather than Spring's
+selection-only `@Primary`.Also applies to: 78-84
🤖 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 `@wiki/concepts/proto-override-precedence.md` around lines 5 - 12, Update the
wiki front matter to include both directly cited ProtoDescriptor source files:
tegg/core/types/src/metadata/model/ProtoDescriptor.ts and
tegg/core/metadata/src/model/ProtoDescriptor/AbstractProtoDescriptor.ts. Mark
the Spring Boot, Guice, and Spring `@Primary` comparison in the cross-framework
discussion as “Inference:” to distinguish synthesis from claims supported by the
listed sources.
Source: Coding guidelines
|
Closing this approach. The concrete need that motivated |
Motivation
The IoC container has no way for one proto to deterministically replace another
that shares the same name. Today two same-name protos that survive qualifier
disambiguation throw
MultiPrototypeFound(there is no silent last-wins), so aframework-provided default cannot be overridden by an application without
forking. This adds a first-class, deterministic override capability.
What
Two standalone proto-level decorators (no new options on the existing proto
decorators):
@ConditionalOnMissing()— a conditional default: used only when it is thesole candidate for its name; dropped when any other proto provides the name.
@Override()— an explicit replacement: wins over a same-name plain orconditional proto.
Precedence within a competing group:
@Override> plain proto >@ConditionalOnMissing.Non-instantiation
The loser is pruned from the proto graph in
GlobalGraph.build()— before anyinject edge or instantiation — so a replaced default never runs its constructor
or
@LifecyclePostInjectside effects. This matches@ConditionalOnMissingBean(Spring Boot) /
OptionalBinder.setDefault(Guice) semantics rather than aselect-at-injection model where the loser would still be instantiated.
Competition scope
Override only applies among protos that would resolve to the same injection:
same name + init type + user qualifiers + access-level scope (
PUBLICcompetesglobally,
PRIVATEis module-local). Auto-addedLoadUnitName/InitTypequalifiers are excluded so cross-module PUBLIC override still groups. Protos that
differ in user qualifier / init type / private-module boundary are never pruned
against each other; groups with no marker keep the existing resolution.
@Primary/@Fallback(select-among-coexisting-candidates, losers stillinstantiated) are intentionally deferred — that need is already served by
qualifiers.
Tests
tegg/core/metadata/test/GlobalGraph.test.ts: precedence (all three tiers,sole-conditional, loser-not-instantiated) + competition scoping (different user
qualifier / init type / private-module not pruned; cross-module override with
the realistic auto-added qualifiers still applies).
tegg/core/core-decorator/test/decorators.test.ts: decorators set the protometadata.
Wiki:
wiki/concepts/proto-override-precedence.md.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
@Override()for explicitly replacing same-name proto definitions.@ConditionalOnMissing()for default definitions used only when no competing proto exists.@Override> plain proto >@ConditionalOnMissing.Tests