Conversation
WalkthroughThe updates introduce the Changes
Sequence Diagram(s)sequenceDiagram
participant App as ControllerAppBootHook
participant MCP as MCPControllerRegister
App->>MCP: clean()
MCP->>MCP: Clear controllerProtos and reset instance
Suggested reviewers
Poem
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
plugin/controller/app.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eggache". (The package "eslint-plugin-eggache" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-eggache" was referenced from the config file in ".eslintrc » eslint-config-egg/typescript » ./index.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. plugin/controller/lib/impl/mcp/MCPControllerRegister.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-eggache". (The package "eslint-plugin-eggache" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-eggache" was referenced from the config file in ".eslintrc » eslint-config-egg/typescript » ./index.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/controller-decorator/package.json(1 hunks)core/tegg/package.json(1 hunks)core/tegg/zod.ts(1 hunks)plugin/controller/package.json(1 hunks)plugin/mcp-proxy/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-ubuntu (22)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (typescript)
🔇 Additional comments (2)
core/tegg/zod.ts (1)
1-1: Re-export ofzodis correctly implemented
This passthrough module cleanly centralizeszodexports for downstream consumers.core/tegg/package.json (1)
55-56: Review added runtime dependencies
This file introduces both@eggjs/tegg-typesandzodunderdependencies. Whilezodaligns with the PR’s goal, adding@eggjs/tegg-typeswasn’t mentioned in the summary. Confirm whether@eggjs/tegg-typesis required at runtime—if it’s only for type definitions, it may belong indevDependencies.Run this script to check for usage:
#!/bin/bash # Check usage of @eggjs/tegg-types and zod in core/tegg code rg -n "from '@eggjs/tegg-types'" -A2 core/tegg rg -n "import.*zod" -A2 core/teggLikely an incorrect or invalid review comment.
akitaSummer
force-pushed
the
fix/zod-phantom-dependency
branch
5 times, most recently
from
cadf9ff to
e309819
Compare
akitaSummer
force-pushed
the
fix/zod-phantom-dependency
branch
from
e309819 to
5078e15
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugin/controller/lib/impl/mcp/MCPControllerRegister.ts (1)
149-154: Implement static method cleanup but address static contextThe
clean()method properly resets the singleton instance and clears controller prototypes for proper cleanup during application shutdown. However, usingthisin a static context could be confusing.Consider using the class name instead of
thisfor better clarity in the static context:static clean() { - if (this.instance) { - this.instance.controllerProtos = []; + if (MCPControllerRegister.instance) { + MCPControllerRegister.instance.controllerProtos = []; } - this.instance = undefined; + MCPControllerRegister.instance = undefined; }🧰 Tools
🪛 Biome (1.9.4)
[error] 153-153: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/controller-decorator/package.json(1 hunks)plugin/controller/app.ts(1 hunks)plugin/controller/lib/impl/mcp/MCPControllerRegister.ts(1 hunks)plugin/controller/package.json(1 hunks)plugin/mcp-proxy/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugin/controller/package.json
- plugin/mcp-proxy/package.json
- core/controller-decorator/package.json
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugin/controller/app.ts (1)
plugin/controller/lib/impl/mcp/MCPControllerRegister.ts (1)
MCPControllerRegister(69-510)
🪛 Biome (1.9.4)
plugin/controller/lib/impl/mcp/MCPControllerRegister.ts
[error] 153-153: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.
(lint/complexity/noThisInStatic)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Runner-macos (18)
- GitHub Check: Runner-ubuntu (22)
- GitHub Check: Runner-ubuntu (18)
- GitHub Check: Runner-ubuntu (16)
- GitHub Check: Runner-ubuntu (20)
- GitHub Check: Runner-macos (20)
- GitHub Check: Runner-macos (16)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (typescript)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (1)
plugin/controller/app.ts (1)
130-130: LGTM! Great addition for consistent resource cleanupAdding the
MCPControllerRegister.clean()call in thebeforeCloselifecycle method ensures proper cleanup of the MCP controller resources during application shutdown, similar to the existing HTTP controller cleanup pattern.
2 participants
Checklist
npm testpassesAffected core subsystem(s)
Description of change
Summary by CodeRabbit
Chores
New Features