feat: add AgentController support to standalone Runner#5837
feat: add AgentController support to standalone Runner#5837jerryliang64 wants to merge 1 commit intonextfrom
Conversation
The standalone Runner bypasses egg plugin lifecycle, so AgentControllerProto/Object registration needs to be done manually, similar to how it's done in the controller plugin's app.ts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Deploying egg with
|
| Latest commit: |
6281827
|
| Status: | 🚫 Build failed. |
📝 WalkthroughWalkthroughThe pull request extends the standalone tegg runner by adding the AgentController plugin as a dependency and integrating AgentController prototype creation, object instantiation, and logging into the runner's initialization sequence. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can get early access to new features in CodeRabbit.Enable the |
Deploying egg-v3 with
|
| Latest commit: |
6281827
|
| Status: | 🚫 Build failed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tegg/standalone/standalone/src/Runner.ts (1)
310-353:⚠️ Potential issue | 🟠 MajorAdd cleanup for factory registrations in
destroy()method.The
initLoaderInstance()method registers creators withEggPrototypeCreatorFactory(line 256) and object creation methods withEggObjectFactory(line 260), but these static factory registrations are never unregistered indestroy(). Since these factories maintain static Maps without clear/unregister methods, repeated Runner instantiation (particularly in test scenarios) will accumulate stale registrations.The pattern should be consistent with how other lifecycle hooks are already cleaned up (DAL, AOP hooks at lines 325-347). Consider adding an
unregisterPrototypeCreator()method toEggPrototypeCreatorFactoryand corresponding cleanup forEggObjectFactory.eggObjectCreateMapto properly release these registrations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/standalone/standalone/src/Runner.ts` around lines 310 - 353, The destroy() method fails to unregister static registrations made in initLoaderInstance(): remove prototype creators and object creators registered with EggPrototypeCreatorFactory and EggObjectFactory to avoid accumulating stale entries; add methods such as EggPrototypeCreatorFactory.unregisterPrototypeCreator(name) (or a clear entry API) and an EggObjectFactory.unregisterEggObjectCreator(key) to remove the specific entries the initLoaderInstance() added, and call them from Runner.destroy() (matching the symbols initLoaderInstance, destroy, EggPrototypeCreatorFactory, EggObjectFactory, and eggObjectCreateMap) so repeated Runner instantiation does not leak static factory registrations.
🧹 Nitpick comments (2)
tegg/standalone/standalone/src/Runner.ts (2)
53-55: Add.jsextension to ESM imports.Per coding guidelines, all imports should use
.jsextensions for ESM files. Additionally, importing from internallib/paths creates coupling to implementation details—consider whether the controller plugin should export these from its main entry point instead.♻️ Proposed fix
-import { AgentControllerObject } from '@eggjs/controller-plugin/lib/AgentControllerObject'; -import { AgentControllerProto } from '@eggjs/controller-plugin/lib/AgentControllerProto'; +import { AgentControllerObject } from '@eggjs/controller-plugin/lib/AgentControllerObject.js'; +import { AgentControllerProto } from '@eggjs/controller-plugin/lib/AgentControllerProto.js';As per coding guidelines: "All imports should use
.jsextensions for ESM files"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/standalone/standalone/src/Runner.ts` around lines 53 - 55, The imports in Runner.ts use internal lib paths without .js extensions—replace the two imports of AgentControllerObject and AgentControllerProto to import them from the package's public entry with a .js extension (or ask the controller plugin to re-export these symbols) so ESM resolution and guidelines are satisfied; locate the lines importing AgentControllerObject and AgentControllerProto and change them to import the same exported symbols from the module's main entry point with a trailing .js extension (or update the plugin to export AgentControllerObject/AgentControllerProto) to remove the internal lib/ coupling.
256-259: Use the shared constant instead of a hardcoded string.The controller plugin (
tegg/plugin/controller/src/app.ts) imports and usesAGENT_CONTROLLER_PROTO_IMPL_TYPEfrom@eggjs/tegg-types. Using the same constant here ensures consistency and prevents subtle bugs if the value changes.♻️ Proposed fix
Add the import:
import { AGENT_CONTROLLER_PROTO_IMPL_TYPE } from '@eggjs/tegg-types';Then update the registration:
EggPrototypeCreatorFactory.registerPrototypeCreator( - 'AGENT_CONTROLLER_PROTO', + AGENT_CONTROLLER_PROTO_IMPL_TYPE, AgentControllerProto.createProto, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/standalone/standalone/src/Runner.ts` around lines 256 - 259, Replace the hardcoded registration key 'AGENT_CONTROLLER_PROTO' with the shared constant by importing AGENT_CONTROLLER_PROTO_IMPL_TYPE from '@eggjs/tegg-types' and using it in the call to EggPrototypeCreatorFactory.registerPrototypeCreator (which currently passes AgentControllerProto.createProto); update the import list to include AGENT_CONTROLLER_PROTO_IMPL_TYPE and change the first argument of registerPrototypeCreator to that constant to ensure consistency with the controller plugin.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tegg/standalone/standalone/src/Runner.ts`:
- Line 264: Replace the unsafe cast by ensuring the logger passed to
AgentControllerObject.setLogger has the correct EggLogger shape: import
EggLogger from the egg package and either (a) assert/convert the existing logger
variable (typed Logger from `@eggjs/tegg`) to EggLogger after mapping or wrapping
it with an adapter function that implements EggLogger methods, or (b) add a
small type-guard that verifies the logger implements the EggLogger interface and
choose a proper fallback (avoid using raw console); then call
AgentControllerObject.setLogger(with the adapted/typed EggLogger) instead of
using `as any`.
---
Outside diff comments:
In `@tegg/standalone/standalone/src/Runner.ts`:
- Around line 310-353: The destroy() method fails to unregister static
registrations made in initLoaderInstance(): remove prototype creators and object
creators registered with EggPrototypeCreatorFactory and EggObjectFactory to
avoid accumulating stale entries; add methods such as
EggPrototypeCreatorFactory.unregisterPrototypeCreator(name) (or a clear entry
API) and an EggObjectFactory.unregisterEggObjectCreator(key) to remove the
specific entries the initLoaderInstance() added, and call them from
Runner.destroy() (matching the symbols initLoaderInstance, destroy,
EggPrototypeCreatorFactory, EggObjectFactory, and eggObjectCreateMap) so
repeated Runner instantiation does not leak static factory registrations.
---
Nitpick comments:
In `@tegg/standalone/standalone/src/Runner.ts`:
- Around line 53-55: The imports in Runner.ts use internal lib paths without .js
extensions—replace the two imports of AgentControllerObject and
AgentControllerProto to import them from the package's public entry with a .js
extension (or ask the controller plugin to re-export these symbols) so ESM
resolution and guidelines are satisfied; locate the lines importing
AgentControllerObject and AgentControllerProto and change them to import the
same exported symbols from the module's main entry point with a trailing .js
extension (or update the plugin to export
AgentControllerObject/AgentControllerProto) to remove the internal lib/
coupling.
- Around line 256-259: Replace the hardcoded registration key
'AGENT_CONTROLLER_PROTO' with the shared constant by importing
AGENT_CONTROLLER_PROTO_IMPL_TYPE from '@eggjs/tegg-types' and using it in the
call to EggPrototypeCreatorFactory.registerPrototypeCreator (which currently
passes AgentControllerProto.createProto); update the import list to include
AGENT_CONTROLLER_PROTO_IMPL_TYPE and change the first argument of
registerPrototypeCreator to that constant to ensure consistency with the
controller plugin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4dae786d-ddaf-4621-bcb0-859c6aba0b8b
📒 Files selected for processing (2)
tegg/standalone/standalone/package.jsontegg/standalone/standalone/src/Runner.ts
| AgentControllerProto, | ||
| AgentControllerObject.createObject, | ||
| ); | ||
| AgentControllerObject.setLogger(logger as any); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the EggLogger interface in egg.js framework?
💡 Result:
The EggLogger interface in the Egg.js framework refers to the Logger class provided by the egg-logger module, which is the core logging system for Egg.js applications. It is not a formal TypeScript "interface" but the primary Logger class and its extensions used throughout Egg.js (e.g., app.logger, ctx.logger). Key aspects from official sources: - Logger is the base class from egg-logger that supports multiple Transports (e.g., file, console, custom). - Common methods: debug(msg), info(msg), warn(msg), error(err), supports printf-style formatting (%s, %j, %d). - Additional methods: set(name, transport), disable(name), enable(name), duplicate(target, options), redirect(level, logger). - Levels: NONE, DEBUG, INFO, WARN, ERROR. - Egg.js provides specialized loggers: appLogger (app-web.log), coreLogger (egg-web.log), errorLogger (common-error.log), agentLogger (egg-agent.log). - Custom loggers via config.customLogger. - Accessed as app.logger, ctx.logger, this.logger in controllers/services. Usage example: const Logger = require('egg-logger').Logger; const logger = new Logger; logger.set('console', new require('egg-logger').ConsoleTransport({ level: 'INFO' })); logger.info('Hello %s', 'world'); Egg.js loggers are instances of or wrap this Logger for enterprise features like multi-process safety, log rotation, and JSON output.
Citations:
- 1: https://www.eggjs.org/core/logger
- 2: https://github.com/eggjs/egg-logger
- 3: https://www.npmjs.com/package/egg-logger
- 4: https://eggjs.github.io/zh/guide/logger.html
🏁 Script executed:
# First, let's examine the Runner.ts file around line 264
fd -t f "Runner.ts" | xargs wc -l | head -20Repository: eggjs/egg
Length of output: 172
🏁 Script executed:
# Get the specific file path
find . -name "Runner.ts" -path "*/tegg/standalone/*" -type fRepository: eggjs/egg
Length of output: 95
🏁 Script executed:
# Read the Runner.ts file to understand the context
fd -t f "Runner.ts" -path "*/tegg/standalone/*" | head -1 | xargs cat -n | sed -n '240,280p'Repository: eggjs/egg
Length of output: 221
🏁 Script executed:
# Find AgentControllerObject and its setLogger method
rg "AgentControllerObject" -A 20 -B 5 | head -50Repository: eggjs/egg
Length of output: 3863
🏁 Script executed:
# Search for AgentControllerObject class definition and setLogger method
fd -t f "\.ts$" -path "*/tegg/*" | xargs rg "setLogger" -B 3 -A 3Repository: eggjs/egg
Length of output: 1702
🏁 Script executed:
# Read the Runner.ts constructor to find the logger type definition
cat -n tegg/standalone/standalone/src/Runner.ts | sed -n '230,270p'Repository: eggjs/egg
Length of output: 2184
🏁 Script executed:
# Find the imports and logger type in Runner.ts
cat -n tegg/standalone/standalone/src/Runner.ts | sed -n '1,50p'Repository: eggjs/egg
Length of output: 1547
🏁 Script executed:
# Read AgentControllerObject.ts to find EggLogger import
cat -n tegg/plugin/controller/src/lib/AgentControllerObject.ts | sed -n '1,50p'Repository: eggjs/egg
Length of output: 2161
🏁 Script executed:
# Check for Logger and EggLogger type definitions
rg "type Logger|interface Logger|class Logger" -B 2 -A 5 | head -80Repository: eggjs/egg
Length of output: 5588
Remove the as any cast—use a compatible logger type or adjust the signature.
AgentControllerObject.setLogger() expects EggLogger from the egg package, but the logger variable is typed as Logger from @eggjs/tegg. The fallback to console further complicates this, since console is not compatible with EggLogger. The as any cast violates the no-any guideline and masks a real type mismatch.
Consider:
- Importing
EggLoggerand ensuring type compatibility before callingsetLogger() - Adding a type guard or adapter function to validate logger compatibility
- Adjusting
setLogger()to accept a more general logger interface
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tegg/standalone/standalone/src/Runner.ts` at line 264, Replace the unsafe
cast by ensuring the logger passed to AgentControllerObject.setLogger has the
correct EggLogger shape: import EggLogger from the egg package and either (a)
assert/convert the existing logger variable (typed Logger from `@eggjs/tegg`) to
EggLogger after mapping or wrapping it with an adapter function that implements
EggLogger methods, or (b) add a small type-guard that verifies the logger
implements the EggLogger interface and choose a proper fallback (avoid using raw
console); then call AgentControllerObject.setLogger(with the adapted/typed
EggLogger) instead of using `as any`.
![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 adds support for @AgentController in the standalone Runner by manually registering necessary components from @eggjs/controller-plugin. The changes are logical and follow the description. I have a few suggestions to improve maintainability and robustness by avoiding direct imports from internal lib directories, removing an as any type assertion, and replacing a magic string with a constant if one is available.
| import { AgentControllerObject } from '@eggjs/controller-plugin/lib/AgentControllerObject'; | ||
| import { AgentControllerProto } from '@eggjs/controller-plugin/lib/AgentControllerProto'; |
There was a problem hiding this comment.
Importing directly from the lib directory of @eggjs/controller-plugin is generally discouraged as it relies on internal implementation details that can change without notice, leading to potential breakages. It would be better to import these from the package's public API if they are exposed, for example: import { AgentControllerObject, AgentControllerProto } from '@eggjs/controller-plugin';. If they are not exported, it might be worth considering exposing them from the plugin package to create a stable API contract.
|
|
||
| // AgentController support | ||
| EggPrototypeCreatorFactory.registerPrototypeCreator( | ||
| 'AGENT_CONTROLLER_PROTO', |
There was a problem hiding this comment.
Using a magic string like 'AGENT_CONTROLLER_PROTO' is brittle. If this string is defined as a constant within the @eggjs/controller-plugin package (e.g., on AgentControllerProto itself as a static property), it would be safer to import and use that constant instead. This would prevent potential issues if the value changes in the source package.
| AgentControllerProto, | ||
| AgentControllerObject.createObject, | ||
| ); | ||
| AgentControllerObject.setLogger(logger as any); |
There was a problem hiding this comment.
The use of as any here bypasses type safety. It suggests a type mismatch between the logger object (of type Logger from @eggjs/tegg) and the logger expected by AgentControllerObject.setLogger. Please investigate the type incompatibility and resolve it. If the logger types are structurally compatible but differ by name, you could use a more specific type assertion to the expected type instead of any, or create a simple adapter. Using any can hide bugs and makes the code harder to maintain.
2 participants
Summary
AgentControllerProto.createProtoviaEggPrototypeCreatorFactoryin standalone Runner'sinitLoaderInstance()AgentControllerObject.createObjectviaEggObjectFactoryfor object instantiationAgentControllerObjectusing the Runner's available logger@eggjs/controller-pluginas a dependency of@eggjs/standaloneThe standalone Runner bypasses egg plugin lifecycle (
app.ts), so@AgentControllerdecorator support requires manual registration of the prototype creator and object factory method — mirroring what@eggjs/controller-plugin/app.tsdoes in the plugin boot hook.Test plan
@AgentController🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Chores
Refactor