Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant Loader as Skill Loader
participant FS as Filesystem
participant Runner as Test Runner
participant Judge as LLM Judge (Anthropic)
participant Reporter as Report Generator
Loader->>FS: read SKILL.md, references, eval JSON
Runner->>Loader: request eval cases (routing / quality)
Runner->>Judge: send prompt + references for evaluation
Judge->>Judge: score response vs expected_output (LLM)
Judge->>Reporter: emit JSON score + rationale
Reporter->>Runner: aggregate results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 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 |
Contributor
Summary of ChangesHello @gxkl, 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! 本次拉取请求旨在初始化一个名为 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying egg with
|
| Latest commit: |
33935c4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://60da9bc3.egg-cci.pages.dev |
| Branch Preview URL: | https://docs-skills.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
33935c4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ac6b432c.egg-v3.pages.dev |
| Branch Preview URL: | https://docs-skills.egg-v3.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request initializes the documentation for @eggjs/skills, adding several well-structured markdown files that cover core concepts, controllers, and decision guides. The documentation is a great start. I've identified a few typos in the markdown files and a potentially incorrect Node.js version requirement in package.json that should be addressed.
| "dependencies": {}, | ||
| "devDependencies": {}, | ||
| "engines": { | ||
| "node": ">=22.18.0" |
Contributor
There was a problem hiding this comment.
The required Node.js version >=22.18.0 seems overly restrictive and is likely a typo. Node.js 22 is very recent, and a minor version of 18 does not exist. Other documentation in the project suggests a minimum version of Node.js 18. I recommend changing this to a more common and reasonable requirement, such as >=18.0.0.
Suggested change
| "node": ">=22.18.0" | |
| "node": ">=18.0.0" |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5783 +/- ##
=======================================
Coverage 85.74% 85.74%
=======================================
Files 665 665
Lines 12981 12981
Branches 1490 1490
=======================================
Hits 11130 11130
Misses 1731 1731
Partials 120 120 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
Author
|
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces a new packages/skills/ workspace to publish Egg AI-agent “skills” as structured Markdown guidance, plus initial eval fixtures and repository documentation updates describing the skills system.
Changes:
- Add
@eggjs/skillspackage (markdown-only) with entry skill (egg) and specialized skills (egg-core,egg-controller) plus reference docs. - Add initial evaluation fixtures under
packages/skills/eval/. - Update
CLAUDE.mdto document the new skills package and conventions.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/skills/package.json | Defines the new publishable @eggjs/skills workspace package. |
| packages/skills/README.md | Minimal package README. |
| packages/skills/PLAN.md | Design doc for planned static/dynamic evaluation approach. |
| packages/skills/eval/.gitignore | Ignores eval workspaces/artifacts. |
| packages/skills/eval/evals-routing.json | Routing evaluation cases for entry skill decision-making. |
| packages/skills/eval/evals-egg-core.json | Core skill evaluation prompts/expected outputs. |
| packages/skills/eval/evals-egg-controller.json | Controller skill evaluation prompts/expected outputs. |
| packages/skills/egg/SKILL.md | Entry skill that routes between egg-core and egg-controller. |
| packages/skills/egg-core/SKILL.md | Core concepts skill overview content. |
| packages/skills/egg-core/references/module.md | Module structure/reference guidance. |
| packages/skills/egg-core/references/proto.md | Proto (Singleton/Context) reference guidance. |
| packages/skills/egg-core/references/inject.md | DI/inject reference guidance. |
| packages/skills/egg-core/references/dynamic-inject.md | Dynamic injection reference guidance. |
| packages/skills/egg-controller/SKILL.md | Controller skill overview/decision tree. |
| packages/skills/egg-controller/references/http-controller.md | HTTP controller reference guidance. |
| packages/skills/egg-controller/references/mcp-controller.md | MCP controller reference guidance. |
| packages/skills/egg-controller/references/schedule.md | Schedule reference guidance. |
| CLAUDE.md | Adds repo-level documentation about the new skills package. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+2
to
+28
| "name": "@eggjs/skills", | ||
| "version": "4.0.0-beta.36", | ||
| "description": "agent skills for egg", | ||
| "keywords": [ | ||
| "egg", | ||
| "skill" | ||
| ], | ||
| "homepage": "https://github.com/eggjs/egg/tree/next/packages/skills", | ||
| "bugs": { | ||
| "url": "https://github.com/eggjs/egg/issues" | ||
| }, | ||
| "license": "MIT", | ||
| "author": "eggjs", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "git+https://github.com/eggjs/egg.git", | ||
| "directory": "packages/skills" | ||
| }, | ||
| "files": [ | ||
| "**/*.md" | ||
| ], | ||
| "publishConfig": { | ||
| "access": "public" | ||
| }, | ||
| "engines": { | ||
| "node": ">=22.18.0" | ||
| } |
Comment on lines
+19
to
+20
| ├── controller/ | ||
| ├── tegg-core/ |
Comment on lines
+124
to
+127
|
|
||
| const client = new Anthropic(); | ||
| const AVAILABLE_SKILLS = ['controller', 'tegg-core']; | ||
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
packages/skills/egg-core/references/inject.md (1)
254-258: Consider documenting the SingletonProto → ContextProto injection constraint.The "重要约束" section covers important constraints but doesn't mention that
SingletonProtocannot injectContextProto. This is a common pitfall that could cause runtime errors.📝 Suggested addition
## 重要约束 - **不能有循环依赖**：Proto 之间或模块之间都不能形成循环引用 - **不能有同名对象**：同一模块内不能存在相同名称和相同初始化类型的 Proto - **按需注入**：不要直接注入 `app` 或 `ctx`，按需注入具体的属性（如 `logger`、`config`） +- **生命周期兼容**：`SingletonProto` 不能注入 `ContextProto`（单例无法持有请求级实例），但 `ContextProto` 可以注入任意 ProtoBased on learnings: "ContextProto objects can inject any prototype, but SingletonProto cannot inject ContextProto".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/egg-core/references/inject.md` around lines 254 - 258, Add a new constraint to the "重要约束" section stating that SingletonProto cannot inject ContextProto to prevent runtime errors: explicitly document that while ContextProto may inject any prototype, SingletonProto must not depend on ContextProto (i.e., forbid SingletonProto → ContextProto injection), give a short rationale and an example mention (use the symbols SingletonProto and ContextProto) so readers know to inject only context-independent prototypes into singletons.packages/skills/egg-controller/SKILL.md (1)
13-22: Consider adding a language specifier to the decision tree code block.Static analysis flagged the missing language specifier on line 13. Adding
textwould satisfy linting and maintain consistency.♻️ Proposed fix
-``` +```text 需要暴露什么接口/客户端协议？🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/egg-controller/SKILL.md` around lines 13 - 22, Update the fenced code block in SKILL.md that contains the decision tree ("需要暴露什么接口/客户端协议？") to include a language specifier; change the opening triple backticks to include "text" (i.e., ```text) so the block is properly annotated for linters and consistent with other docs; locate the code block by searching for the exact decision-tree content in SKILL.md and edit the opening fence accordingly.packages/skills/egg-core/SKILL.md (1)
32-50: Consider adding language specifiers to directory structure code blocks.Static analysis flagged missing language specifiers on the directory structure examples at lines 32 and 42. Adding
textorplaintextwould satisfy linting.♻️ Example fix
-``` +```text app/ └── userModule/ ✅ 驼峰命名🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/egg-core/SKILL.md` around lines 32 - 50, The two directory-structure code blocks in SKILL.md (the example starting with "app/ └── userModule/ ..." and the "错误示例" block starting with "app/ ├── user-module/ ...") lack a language specifier; update their fenced code blocks from ``` to ```text (or ```plaintext) so the linter stops complaining and the blocks are treated as plain text; ensure both occurrences are changed (the first userModule example and the 错误示例 block).packages/skills/egg-core/references/module.md (1)
52-62: Consider adding language specifiers to fenced code blocks for directory structures.The static analysis tool flagged missing language specifiers on directory structure diagrams. While these are ASCII trees rather than code, adding a language specifier like
textorplaintextwould satisfy linting and improve consistency.♻️ Example fix for line 52
-``` +```text app/ ├── fooModule/ ✅ 被扫描到Similar changes can be applied to the other directory structure blocks at lines 83, 166, 178, and 192.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/egg-core/references/module.md` around lines 52 - 62, Add a language specifier (e.g., ```text) to the fenced code blocks that contain ASCII directory trees such as the block starting with "app/" (and the other similar directory-structure blocks in module.md) so the linter is satisfied; locate the triple-backtick blocks that show the tree (containing lines like "app/", "├── fooModule/", ".hidden/", "common/") and change their opening fence from ``` to ```text (or ```plaintext) for each occurrence.packages/skills/PLAN.md (2)
148-150: Consider adding error handling for JSON parsing.The code uses
JSON.parse()without try-catch blocks. For production evaluation code, consider adding error handling to gracefully handle malformed AI responses.🛡️ Example with error handling
For line 148-150:
const text = response.content[0].type === 'text' ? response.content[0].text : ''; let parsed; try { parsed = JSON.parse(text); } catch (e) { throw new Error(`Failed to parse routing response: ${text}\n${e}`); }Similar pattern can be applied at line 304-305 in the judge function.
Also applies to: 304-305
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/PLAN.md` around lines 148 - 150, The JSON.parse call that converts the AI response text into the routing object is unprotected and can throw on malformed input; wrap the parse in a try/catch in the same block where response.content[0] is read (the code that sets text and parsed) and in the judge function's analogous parse (the block around lines referencing judge), and on error either throw a new Error with the original text and the caught error or return a clear failure value—this ensures functions that depend on parsed (the parsed variable) won’t crash on invalid AI output and the logs/assertions include the offending text for debugging.
16-37: Add language identifier to fenced code block.The directory structure code block should specify a language for better syntax highlighting and rendering.
📝 Suggested improvement
-``` +```text packages/skills/ ├── egg/ ├── controller/🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/PLAN.md` around lines 16 - 37, Update the fenced code block in PACKAGES/skills/PLAN.md's directory listing to include a language identifier (e.g., "text" or "bash") so the block becomes ```text (or ```bash) instead of just ```, ensuring proper syntax highlighting for the tree output; locate the block shown under the packages/skills/ directory listing and add the language token immediately after the opening backticks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 143-148: Update the documentation entries under the
**`packages/skills/`** listing to use the actual directory names introduced in
the PR: replace references to `controller/` with `egg-controller/` and
`tegg-core/` with `egg-core` (the same change should be applied to the other
occurrences noted in the comment). Locate the paragraph that lists `egg/`,
`controller/`, and `tegg-core/` and change the two mismatched names to
`egg-controller/` and `egg-core/` so the documented paths match the repository
directories.
In `@packages/skills/egg-controller/references/http-controller.md`:
- Around line 9-19: The import list is missing the HTTPParam decorator used in
DemoController. Update the import statement to include HTTPParam alongside
HTTPController, HTTPMethod, and HTTPMethodEnum so that the `@HTTPParam`()
decorator referenced in the hello method is properly imported and available.
In `@packages/skills/egg-controller/references/mcp-controller.md`:
- Around line 23-30: The markdown file contains fenced code blocks without a
language, causing MD040; update those fences (e.g. the block starting with
"app/module-name/" and the block containing "用户需要什么？", as well as the similar
block around lines 76-94) to include a language identifier such as text (replace
``` with ```text) so all fenced blocks declare a language and satisfy
markdownlint.
In `@packages/skills/egg-core/references/proto.md`:
- Around line 62-79: Update the doc to enforce the DI rule that long-lived
scopes cannot depend on short-lived scopes: replace the incorrect claim that
`@SingletonProto` can inject `@ContextProto` and instead state that `@SingletonProto`
must not inject `@ContextProto`; instruct readers to reverse the dependency (have
`@ContextProto` inject `@SingletonProto`) or access per-request data via context
APIs; update the example by removing the `@Inject`() requestContext on AppService
and show the allowed pattern where RequestContext `@Inject`()s AppService,
referencing the symbols SingletonProto, ContextProto, Inject, AppService, and
RequestContext so readers can find and edit the examples accordingly.
In `@packages/skills/egg-core/SKILL.md`:
- Around line 150-153: The async method hello() in HelloService is declared to
return Promise<string> but currently only logs a value and returns nothing;
update the implementation in the HelloService class so it obtains the string
from this.fooService.hello() (await or return the Promise), log that value via
this.logger.info, and then return the string (e.g., const msg = await
this.fooService.hello(); this.logger.info(`[HelloService] ${msg}`); return
msg;), ensuring the method actually resolves to a string.
In `@packages/skills/eval/evals-egg-core.json`:
- Around line 51-54: The eval's expected_output is wrong: update the JSON entry
for id 8 so it states that `@SingletonProto` cannot inject `@ContextProto`
(injection is not possible) and reflect the official tegg/egg rule that
"ContextProto can inject any Proto, but SingletonProto cannot inject
ContextProto"; locate the object with "id": 8 and replace the current
expected_output string to clearly say injection is not allowed and explain that
singleton cannot access per-request ContextProto.
In `@packages/skills/package.json`:
- Around line 1-29: The package manifest is missing the required npm script for
TypeScript type checking; add a "typecheck" entry to the package.json "scripts"
object that runs "tsc --noEmit" (ensuring package "@eggjs/skills" has a scripts
block and the new "typecheck" script is included so CI and local checks execute
tsc without emitting files).
---
Nitpick comments:
In `@packages/skills/egg-controller/SKILL.md`:
- Around line 13-22: Update the fenced code block in SKILL.md that contains the
decision tree ("需要暴露什么接口/客户端协议？") to include a language specifier; change the
opening triple backticks to include "text" (i.e., ```text) so the block is
properly annotated for linters and consistent with other docs; locate the code
block by searching for the exact decision-tree content in SKILL.md and edit the
opening fence accordingly.
In `@packages/skills/egg-core/references/inject.md`:
- Around line 254-258: Add a new constraint to the "重要约束" section stating that
SingletonProto cannot inject ContextProto to prevent runtime errors: explicitly
document that while ContextProto may inject any prototype, SingletonProto must
not depend on ContextProto (i.e., forbid SingletonProto → ContextProto
injection), give a short rationale and an example mention (use the symbols
SingletonProto and ContextProto) so readers know to inject only
context-independent prototypes into singletons.
In `@packages/skills/egg-core/references/module.md`:
- Around line 52-62: Add a language specifier (e.g., ```text) to the fenced code
blocks that contain ASCII directory trees such as the block starting with "app/"
(and the other similar directory-structure blocks in module.md) so the linter is
satisfied; locate the triple-backtick blocks that show the tree (containing
lines like "app/", "├── fooModule/", ".hidden/", "common/") and change their
opening fence from ``` to ```text (or ```plaintext) for each occurrence.
In `@packages/skills/egg-core/SKILL.md`:
- Around line 32-50: The two directory-structure code blocks in SKILL.md (the
example starting with "app/ └── userModule/ ..." and the "错误示例" block starting
with "app/ ├── user-module/ ...") lack a language specifier; update their fenced
code blocks from ``` to ```text (or ```plaintext) so the linter stops
complaining and the blocks are treated as plain text; ensure both occurrences
are changed (the first userModule example and the 错误示例 block).
In `@packages/skills/PLAN.md`:
- Around line 148-150: The JSON.parse call that converts the AI response text
into the routing object is unprotected and can throw on malformed input; wrap
the parse in a try/catch in the same block where response.content[0] is read
(the code that sets text and parsed) and in the judge function's analogous parse
(the block around lines referencing judge), and on error either throw a new
Error with the original text and the caught error or return a clear failure
value—this ensures functions that depend on parsed (the parsed variable) won’t
crash on invalid AI output and the logs/assertions include the offending text
for debugging.
- Around line 16-37: Update the fenced code block in PACKAGES/skills/PLAN.md's
directory listing to include a language identifier (e.g., "text" or "bash") so
the block becomes ```text (or ```bash) instead of just ```, ensuring proper
syntax highlighting for the tree output; locate the block shown under the
packages/skills/ directory listing and add the language token immediately after
the opening backticks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7045ab57-d7d6-4e08-bf83-c2b731c7c0db
📒 Files selected for processing (18)
CLAUDE.mdpackages/skills/PLAN.mdpackages/skills/README.mdpackages/skills/egg-controller/SKILL.mdpackages/skills/egg-controller/references/http-controller.mdpackages/skills/egg-controller/references/mcp-controller.mdpackages/skills/egg-controller/references/schedule.mdpackages/skills/egg-core/SKILL.mdpackages/skills/egg-core/references/dynamic-inject.mdpackages/skills/egg-core/references/inject.mdpackages/skills/egg-core/references/module.mdpackages/skills/egg-core/references/proto.mdpackages/skills/egg/SKILL.mdpackages/skills/eval/.gitignorepackages/skills/eval/evals-egg-controller.jsonpackages/skills/eval/evals-egg-core.jsonpackages/skills/eval/evals-routing.jsonpackages/skills/package.json
Comment on lines
+62
to
+79
| **SingletonProto 可以注入 ContextProto**，框架会自动处理生命周期差异。 | ||
|
|
||
| 实现机制：框架不会直接注入 ContextProto 实例，而是通过 lazy getter/Proxy，在每次访问时从当前请求上下文动态解析，确保每个请求拿到各自的实例。 | ||
|
|
||
| ```typescript | ||
| import { SingletonProto, ContextProto, Inject } from 'egg'; | ||
|
|
||
| @ContextProto() | ||
| export class RequestContext { | ||
| userId: string; | ||
| } | ||
|
|
||
| @SingletonProto() | ||
| export class AppService { | ||
| @Inject() | ||
| requestContext: RequestContext; // ✅ 每次访问自动解析当前请求的实例 | ||
| } | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Egg.js/tegg 官方文档中，@SingletonProto与@ContextProto 的注入方向限制是什么？请给出明确规则和示例。
💡 Result:
明确规则（注入方向限制）
在 Tegg 官方文档中，依赖注入的作用域方向规则是：长生命周期不能持有短生命周期。具体到这两个 Proto：
@ContextProto可以注入@SingletonProto（也可以注入同为@ContextProto的对象）。[1]@SingletonProto不能注入@ContextProto（因为单例跨请求常驻，禁止持有请求级对象）。[1]
示例
允许：Context → Singleton
import { ContextProto, SingletonProto, Inject } from '@eggjs/tegg';
`@SingletonProto`()
class RepoClient {}
`@ContextProto`()
class OrderService {
constructor(`@Inject`() private readonly repo: RepoClient) {}
}允许：Context → Context
import { ContextProto, Inject } from '@eggjs/tegg';
`@ContextProto`()
class UserContextService {}
`@ContextProto`()
class OrderService {
constructor(`@Inject`() private readonly userCtx: UserContextService) {}
}禁止：Singleton → Context（不允许的注入方向）
import { SingletonProto, ContextProto, Inject } from '@eggjs/tegg';
`@ContextProto`()
class RequestService {}
`@SingletonProto`()
class GlobalService {
// ❌ 文档规则：SingletonProto 不能注入 ContextProto
constructor(`@Inject`() private readonly req: RequestService) {}
}来源
[1] Tegg 官方文档《装饰器》页，“关键规则（非常重要）”：ContextProto 可注入 SingletonProto / SingletonProto 不能注入 ContextProto。 (tegg.mintlify.app)
Citations:
Correct the SingletonProto / ContextProto injection rule.
The documentation currently states SingletonProto can inject ContextProto with automatic lifecycle handling, but this violates tegg's dependency injection constraint: @SingletonProto cannot inject @ContextProto because singletons persist across requests and cannot safely hold request-scoped instances.
The rule is: long-lived scopes cannot inject short-lived scopes. Reverse the direction—have @ContextProto inject @SingletonProto instead, or access request context through the framework's provided context APIs.
Corrected example
// ✅ Allowed: ContextProto injects SingletonProto
`@SingletonProto`()
export class AppService {
// shared logic
}
`@ContextProto`()
export class RequestContext {
`@Inject`()
appService: AppService; // each request's own instance, can safely access the singleton
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg-core/references/proto.md` around lines 62 - 79, Update
the doc to enforce the DI rule that long-lived scopes cannot depend on
short-lived scopes: replace the incorrect claim that `@SingletonProto` can inject
`@ContextProto` and instead state that `@SingletonProto` must not inject
`@ContextProto`; instruct readers to reverse the dependency (have `@ContextProto`
inject `@SingletonProto`) or access per-request data via context APIs; update the
example by removing the `@Inject`() requestContext on AppService and show the
allowed pattern where RequestContext `@Inject`()s AppService, referencing the
symbols SingletonProto, ContextProto, Inject, AppService, and RequestContext so
readers can find and edit the examples accordingly.
Comment on lines
+150
to
+153
| async hello(): Promise<string> { | ||
| this.logger.info(`[HelloService] ${this.fooService.hello()}`); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Missing return statement in async method.
The hello() method is declared with return type Promise<string> but doesn't return a value. This would cause a TypeScript error or return undefined.
🐛 Proposed fix
async hello(): Promise<string> {
- this.logger.info(`[HelloService] ${this.fooService.hello()}`);
+ this.logger.info(`[HelloService] ${this.fooService.hello()}`);
+ return this.fooService.hello();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async hello(): Promise<string> { | |
| this.logger.info(`[HelloService] ${this.fooService.hello()}`); | |
| } | |
| } | |
| async hello(): Promise<string> { | |
| this.logger.info(`[HelloService] ${this.fooService.hello()}`); | |
| return this.fooService.hello(); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg-core/SKILL.md` around lines 150 - 153, The async method
hello() in HelloService is declared to return Promise<string> but currently only
logs a value and returns nothing; update the implementation in the HelloService
class so it obtains the string from this.fooService.hello() (await or return the
Promise), log that value via this.logger.info, and then return the string (e.g.,
const msg = await this.fooService.hello(); this.logger.info(`[HelloService]
${msg}`); return msg;), ensuring the method actually resolves to a string.
Comment on lines
+51
to
+54
| "id": 8, | ||
| "prompt": "SingletonProto 能注入 ContextProto 吗？我需要在单例服务里获取当前请求的用户信息", | ||
| "expected_output": "可以注入，框架通过 lazy getter/Proxy 自动处理，每次访问从当前请求上下文动态解析" | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Egg.js tegg, is a @SingletonProtoallowed to inject a@ContextProto? Please provide the official rule and an example from official docs/source.
💡 Result:
No. In @eggjs/tegg, a @SingletonProto must not inject a @ContextProto.
Official rule (from eggjs/tegg README, “限制”): “ContextProto 可以注入任何 Proto 但是 SingletonProto 不能注入 ContextProto” (“ContextProto can inject any Proto, but SingletonProto cannot inject ContextProto”). [1]
Official example (same README, showing @ContextProto using @Inject):
import { EggLogger } from 'egg';
import { Inject } from '@eggjs/tegg';
`@ContextProto`()
export class HelloService {
constructor(`@Inject`() readonly logger: EggLogger) {}
}
``` [1]
Note: Egg’s website DI page also states the opposite (“you can inject `ContextProto` objects within `SingletonProto`”). [2] If you need to decide which rule to follow for tegg specifically, the `eggjs/tegg` repository README is the direct tegg rule. [1]
**Sources:** [1] [2]
---
</details>
**Eval expectation for Singleton→Context injection is incorrect.**
Line 53 expects that `@SingletonProto` can inject `@ContextProto`, but the official eggjs/tegg rule explicitly prohibits this: "ContextProto can inject any Proto, but SingletonProto cannot inject ContextProto." The expected output should state that injection is **not possible**.
<details>
<summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @packages/skills/eval/evals-egg-core.json around lines 51 - 54, The eval's
expected_output is wrong: update the JSON entry for id 8 so it states that
@SingletonProto cannot inject @ContextProto (injection is not possible) and
reflect the official tegg/egg rule that "ContextProto can inject any Proto, but
SingletonProto cannot inject ContextProto"; locate the object with "id": 8 and
replace the current expected_output string to clearly say injection is not
allowed and explain that singleton cannot access per-request ContextProto.
</details>
<!-- fingerprinting:phantom:poseidon:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+1
to
+29
| { | ||
| "name": "@eggjs/skills", | ||
| "version": "4.0.0-beta.36", | ||
| "description": "agent skills for egg", | ||
| "keywords": [ | ||
| "egg", | ||
| "skill" | ||
| ], | ||
| "homepage": "https://github.com/eggjs/egg/tree/next/packages/skills", | ||
| "bugs": { | ||
| "url": "https://github.com/eggjs/egg/issues" | ||
| }, | ||
| "license": "MIT", | ||
| "author": "eggjs", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "git+https://github.com/eggjs/egg.git", | ||
| "directory": "packages/skills" | ||
| }, | ||
| "files": [ | ||
| "**/*.md" | ||
| ], | ||
| "publishConfig": { | ||
| "access": "public" | ||
| }, | ||
| "engines": { | ||
| "node": ">=22.18.0" | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add required typecheck script to package manifest.
This new package is missing the mandatory type-check script.
🧩 Minimal manifest update
{
"name": "@eggjs/skills",
"version": "4.0.0-beta.36",
"description": "agent skills for egg",
+ "scripts": {
+ "typecheck": "tsc --noEmit"
+ },
"keywords": [
"egg",
"skill"
],As per coding guidelines **/package.json: "All packages must include TypeScript type checking using tsc --noEmit in a typecheck script".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| { | |
| "name": "@eggjs/skills", | |
| "version": "4.0.0-beta.36", | |
| "description": "agent skills for egg", | |
| "keywords": [ | |
| "egg", | |
| "skill" | |
| ], | |
| "homepage": "https://github.com/eggjs/egg/tree/next/packages/skills", | |
| "bugs": { | |
| "url": "https://github.com/eggjs/egg/issues" | |
| }, | |
| "license": "MIT", | |
| "author": "eggjs", | |
| "repository": { | |
| "type": "git", | |
| "url": "git+https://github.com/eggjs/egg.git", | |
| "directory": "packages/skills" | |
| }, | |
| "files": [ | |
| "**/*.md" | |
| ], | |
| "publishConfig": { | |
| "access": "public" | |
| }, | |
| "engines": { | |
| "node": ">=22.18.0" | |
| } | |
| } | |
| { | |
| "name": "@eggjs/skills", | |
| "version": "4.0.0-beta.36", | |
| "description": "agent skills for egg", | |
| "scripts": { | |
| "typecheck": "tsc --noEmit" | |
| }, | |
| "keywords": [ | |
| "egg", | |
| "skill" | |
| ], | |
| "homepage": "https://github.com/eggjs/egg/tree/next/packages/skills", | |
| "bugs": { | |
| "url": "https://github.com/eggjs/egg/issues" | |
| }, | |
| "license": "MIT", | |
| "author": "eggjs", | |
| "repository": { | |
| "type": "git", | |
| "url": "git+https://github.com/eggjs/egg.git", | |
| "directory": "packages/skills" | |
| }, | |
| "files": [ | |
| "**/*.md" | |
| ], | |
| "publishConfig": { | |
| "access": "public" | |
| }, | |
| "engines": { | |
| "node": ">=22.18.0" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/package.json` around lines 1 - 29, The package manifest is
missing the required npm script for TypeScript type checking; add a "typecheck"
entry to the package.json "scripts" object that runs "tsc --noEmit" (ensuring
package "@eggjs/skills" has a scripts block and the new "typecheck" script is
included so CI and local checks execute tsc without emitting files).
gxkl
added a commit
that referenced
this pull request
Mar 15, 2026
- CLAUDE.md: rename tegg-core/ to egg-core/, controller/ to egg-controller/ - http-controller.md: add missing HTTPParam import in quick-start example - inject.md: remove unused EggLogger import in Service example - mcp-controller.md: remove unused Inject import in Service example Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
CLAUDE.md (1)
465-466:⚠️ Potential issue | 🟡 MinorUse
egg-controller/consistently in Skills path references.
controller/is still referenced in this section, but the documented skill naming convention and package layout useegg-controller/(see Line 469 and Line 147). Please update these occurrences to avoid path confusion.🛠️ Suggested doc fix
- - `controller/` — 实现指导：HTTPController、MCPController、Schedule + - `egg-controller/` — 实现指导：HTTPController、MCPController、Schedule ... -| **索引型**（如 `controller/`） | Workflow-Based | 多种实现方式的选择 | 精简的决策树 + 快速参考 | 每种实现的详细指南 | +| **索引型**（如 `egg-controller/`） | Workflow-Based | 多种实现方式的选择 | 精简的决策树 + 快速参考 | 每种实现的详细指南 | ... -6. 如果 skill 涉及 controller 类型，同时更新 `controller/SKILL.md` 决策树 +6. 如果 skill 涉及 controller 类型，同时更新 `egg-controller/SKILL.md` 决策树Also applies to: 493-494, 550-550
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 465 - 466, Replace inconsistent "controller/" path references with the established "egg-controller/" name throughout the document; locate occurrences like the line that reads "controller/ — 实现指导：HTTPController、MCPController、Schedule" and update them to "egg-controller/ — 实现指导：HTTPController、MCPController、Schedule", and similarly change any other "controller/" mentions (including duplicates) so they match existing "egg-controller/" references used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/skills/egg-core/references/inject.md`:
- Line 44: The fenced code block that contains the line 'something: any; // 优先级
3：类型无元数据，回退到属性名 → "something"' is missing a language tag; update the opening
triple-backtick fence to include a language identifier (e.g., ```typescript) so
the block is labeled (typescript) and markdownlint MD040 is resolved.
---
Duplicate comments:
In `@CLAUDE.md`:
- Around line 465-466: Replace inconsistent "controller/" path references with
the established "egg-controller/" name throughout the document; locate
occurrences like the line that reads "controller/ —
实现指导：HTTPController、MCPController、Schedule" and update them to "egg-controller/
— 实现指导：HTTPController、MCPController、Schedule", and similarly change any other
"controller/" mentions (including duplicates) so they match existing
"egg-controller/" references used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bdb3b10-da55-4ed6-9087-72f587ec54d3
📒 Files selected for processing (5)
CLAUDE.mdpackages/skills/egg-controller/references/http-controller.mdpackages/skills/egg-controller/references/mcp-controller.mdpackages/skills/egg-core/references/inject.mdpackages/skills/package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/skills/egg-controller/references/mcp-controller.md
- packages/skills/package.json
Contributor
|
先合并吧，这个 PR 太长了。 |
…es (#5786) - Add references/mcp-controller.md with common errors, file conventions, scenario-based decision tree, and end-to-end examples - Update CLAUDE.md with skill writing methodology: prioritize gap-filling over doc reformatting, interview maintainers for undocumented knowledge - Fix typo in controller SKILL.md (refercens -> references) Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
#5788) …ment Introduces an evaluation system using Vitest to test skill routing decisions and answer quality via LLM-as-Judge. Includes PTY-based claude CLI wrapper (via macOS `script`) to bypass Vitest stdio capture, robust JSON extraction from LLM output, and static skill YAML validation tests. Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
- Fix double hyphen in egg/SKILL.md: @eggjs/skills--core → @eggjs/skills-core - Fix missing hyphen in controller/SKILL.md: references/httpcontroller.md → references/http-controller.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…layout Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- module.md: module creation, discovery mechanism (auto-scan vs manual), directory organization - proto.md: SingletonProto vs ContextProto comparison, decorator params, injection rules, lifecycle hooks - inject.md: @Inject usage, name resolution, optional injection, Egg built-in objects, qualifiers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- module.md: add module config section (file format, env merging, injection) - inject.md: add ConfigSourceQualifier for cross-module config injection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add dynamic-inject.md with step-by-step guide - Update SKILL.md: add dynamic inject section, fix import paths and Logger type Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ference - Rename packages/skills/controller/ to packages/skills/egg-controller/ - Add schedule.md with step-by-step guide for interval/cron modes, worker types and run params Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- SKILL.md: add MCP reference link, fix core skill name, fix MCP description - http-controller.md: fix import path egg/tegg to egg, rename SSE to streaming, fix table format - mcp-controller.md: fix Service import path to egg - schedule.md: fix file path to use moduleName placeholder Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove automated eval logic (lib/, dynamic/, static/, fixtures/) - Add evals-routing.json, evals-egg-core.json, evals-egg-controller.json - 44 eval cases covering implementation, troubleshooting, and migration scenarios Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- egg-core SKILL.md: add common issues table (naming, AccessLevel, interface type, mixed inject, optional) - egg-controller SKILL.md: add troubleshooting table (MCP imports, z.object, schedule directory) - mcp-controller.md: expand decision tree with @extra use cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove JS-specific fields (main, module, types, exports), unused scripts and devDependencies (gray-matter, vitest) after eval framework removal. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- CLAUDE.md: rename tegg-core/ to egg-core/, controller/ to egg-controller/ - http-controller.md: add missing HTTPParam import in quick-start example - inject.md: remove unused EggLogger import in Service example - mcp-controller.md: remove unused Inject import in Service example Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pure markdown package with no src/ directory, skip in build. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new @eggjs/skills package under packages/skills/, intended as a Markdown-only “skills” knowledge base to guide AI assistants when working with the Egg framework, and wires it into repo documentation/build config.
Changes:
- Added the new
packages/skills/package (published with only.mdfiles) and documented it inCLAUDE.md. - Added the Egg skills content: entry routing skill (
egg/) plus specialized skills (egg-core/,egg-controller/) and their reference guides. - Excluded
packages/skillsfrom roottsdownworkspace build; added initial eval case JSON fixtures and ignores.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tsdown.config.ts | Excludes packages/skills from workspace build (markdown-only). |
| packages/skills/package.json | Defines new publishable @eggjs/skills package (md-only files). |
| packages/skills/README.md | Package README stub. |
| packages/skills/PLAN.md | Draft plan for future static/dynamic evaluation harness. |
| packages/skills/eval/.gitignore | Ignores local eval workspaces. |
| packages/skills/eval/evals-routing.json | Routing evaluation cases for entry skill. |
| packages/skills/eval/evals-egg-core.json | Core-skill evaluation cases. |
| packages/skills/eval/evals-egg-controller.json | Controller-skill evaluation cases. |
| packages/skills/egg/SKILL.md | Entry routing skill documentation and decision framework. |
| packages/skills/egg-core/SKILL.md | Core concepts skill overview and quick decision guidance. |
| packages/skills/egg-core/references/module.md | Module structure/discovery/config reference. |
| packages/skills/egg-core/references/proto.md | Proto (Singleton/Context) reference and lifecycle notes. |
| packages/skills/egg-core/references/inject.md | Injection rules, qualifiers, and patterns reference. |
| packages/skills/egg-core/references/dynamic-inject.md | Dynamic injection pattern reference. |
| packages/skills/egg-controller/SKILL.md | Controller skill index/decision tree. |
| packages/skills/egg-controller/references/http-controller.md | HTTPController patterns and examples. |
| packages/skills/egg-controller/references/mcp-controller.md | MCPController conventions and end-to-end example. |
| packages/skills/egg-controller/references/schedule.md | Schedule controller patterns and caveats. |
| CLAUDE.md | Documents the skills package structure and authoring conventions. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+21
to
+29
| MCPController 放在 module 的 `controller/` 目录下，命名规则为 `{Name}MCPController.ts`： | ||
|
|
||
| ``` | ||
| app/module-name/ | ||
| ├── controller/ | ||
| │ ├── PackageMCPController.ts ← MCP 控制器 | ||
| │ └── PackageHTTPController.ts ← 同模块可共存 HTTP 控制器 | ||
| └── service/ | ||
| └── PackageService.ts |
Comment on lines
+124
to
+127
|
|
||
| const client = new Anthropic(); | ||
| const AVAILABLE_SKILLS = ['controller', 'tegg-core']; | ||
|
|
Comment on lines
+170
to
+186
| // eval/fixtures/quality-cases.ts | ||
| export const qualityCases: QualityCase[] = [ | ||
| { | ||
| skill: 'controller', | ||
| query: '如何创建一个 POST 接口接收 JSON body？', | ||
| criteria: [ | ||
| '使用 @HTTPController 装饰器', | ||
| '使用 @HTTPMethod 且 method 为 POST', | ||
| '使用 @HTTPBody() 获取请求体', | ||
| '包含完整可运行的代码示例', | ||
| ], | ||
| references: ['references/http-controller.md'], // 需要加载的参考文档 | ||
| }, | ||
| { | ||
| skill: 'tegg-core', | ||
| query: '如何让一个服务可以被其他模块访问？', | ||
| criteria: [ |
Comment on lines
+330
to
+348
| { | ||
| "query": "...", | ||
| "expected": "tegg-core", | ||
| "actual": "controller", | ||
| "reason": "..." | ||
| } | ||
| ] | ||
| }, | ||
| "quality": { | ||
| "controller": { | ||
| "cases": 5, | ||
| "avg_score": 0.85, | ||
| "details": [...] | ||
| }, | ||
| "tegg-core": { | ||
| "cases": 5, | ||
| "avg_score": 0.90, | ||
| "details": [...] | ||
| } |
| - **入口 skill** (`egg/`) — 分析用户意图，通过关键词匹配和决策逻辑路由到专业 skill | ||
| - **专业 skills** — 提供特定领域的深度指导： | ||
| - `egg-core/` — 核心概念：模块、依赖注入、生命周期、AccessLevel | ||
| - `controller/` — 实现指导：HTTPController、MCPController、Schedule |
Comment on lines
+95
to
+110
| export const routingCases: RoutingCase[] = [ | ||
| { | ||
| query: '如何创建 HTTP controller？', | ||
| expectedSkill: 'controller', | ||
| reason: '明确提到 controller，属于协议实现', | ||
| }, | ||
| { | ||
| query: '@SingletonProto 和 @ContextProto 有什么区别？', | ||
| expectedSkill: 'tegg-core', | ||
| reason: '关于对象生命周期，属于核心概念', | ||
| }, | ||
| { | ||
| query: '我需要创建一个可以被 HTTP 控制器使用的服务', | ||
| expectedSkill: 'tegg-core', | ||
| reason: '模糊意图，按规则 1（基础优先）应路由到 core', | ||
| }, |
|
|
||
| 1. 使用 `egg-core` skill | ||
| 2. 解释概念（例如，`AccessLevel.PUBLIC`） | ||
| 3. 核心解释后，建议："如果你需要在特定控制器中使用它，请使用 `egg-controller` skill |
| **重要提示**： | ||
|
|
||
| - 模块名称不能包含 `-` 或其他特殊字符；使用驼峰命名规则。 | ||
| - module 的 `package.json` 文件中，仅包含 `name` 以及 `eggModule.name` 字段，不应该有其他额外内容。 |
| 3. 创建 `references/` 目录（初始为空时放置 `.gitkeep`） | ||
| 4. 按需在 `references/*.md` 中添加详细参考文档 | ||
| 5. 更新入口 skill（`egg/SKILL.md`）的路由逻辑以包含新 skill | ||
| 6. 如果 skill 涉及 controller 类型，同时更新 `controller/SKILL.md` 决策树 |
Comment on lines
+19
to
+20
| ├── controller/ | ||
| ├── tegg-core/ |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
packages/skills/egg-controller/references/mcp-controller.md (1)
23-30:⚠️ Potential issue | 🟡 MinorDeclare fence languages for both diagram/code blocks.
Both fenced blocks are still missing language identifiers (MD040).
🛠️ Minimal fix
-``` +```text app/module-name/ ... -``` +``` -``` +```text 用户需要什么？ ... -``` +```Also applies to: 76-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/egg-controller/references/mcp-controller.md` around lines 23 - 30, The two fenced blocks in mcp-controller.md (the ASCII tree starting with "app/module-name/" and the other fenced block around lines 76-94) are missing language identifiers; add a language tag (e.g., ```text) immediately after each opening ``` to satisfy MD040 so both blocks are declared as fenced code blocks with a language (ensure you update the ASCII tree block that contains "PackageMCPController.ts" and "PackageHTTPController.ts" and the separate block referenced at 76-94).packages/skills/eval/evals-egg-core.json (1)
51-54:⚠️ Potential issue | 🟠 MajorFix eval expectation for
SingletonProto→ContextProtoinjection.This expected output is inverted.
SingletonProtoshould not injectContextProto; keeping this as-is will score incorrect answers as correct.🛠️ Suggested correction
- "expected_output": "可以注入，框架通过 lazy getter/Proxy 自动处理，每次访问从当前请求上下文动态解析" + "expected_output": "不可以注入。ContextProto 可以注入任何 Proto，但 SingletonProto 不能注入 ContextProto；单例无法直接持有请求级对象"#!/bin/bash # Verify current rule references and this eval entry. rg -n -S "ContextProto.*SingletonProto|SingletonProto.*ContextProto" tegg rg -n -C2 '"id": 8|SingletonProto 能注入 ContextProto|expected_output' packages/skills/eval/evals-egg-core.jsonBased on learnings: "ContextProto objects can inject any prototype, but SingletonProto cannot inject ContextProto".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/eval/evals-egg-core.json` around lines 51 - 54, The eval entry with id 8 is wrong: it asserts "SingletonProto 能注入 ContextProto" when the framework behavior is the opposite. Update the expected_output for the eval entry (id 8) so it states that SingletonProto cannot inject ContextProto and explain that ContextProto can be injected into other prototypes but SingletonProto cannot access per-request ContextProto (e.g., change the expected_output text to a short statement like "不能注入，SingletonProto 无法注入 ContextProto；ContextProto 可以注入其他原型，但单例无法访问每次请求的上下文"). Ensure you modify the JSON entry that contains "id": 8 and "prompt": "SingletonProto 能注入 ContextProto 吗？" and keep the rest of the object intact.packages/skills/package.json (1)
1-29:⚠️ Potential issue | 🟠 MajorAdd the required
typecheckscript to this package manifest.
packages/skills/package.jsonis missing the mandatory TypeScript check script.🛠️ Minimal manifest update
{ "name": "@eggjs/skills", "version": "4.1.2-beta.4", "description": "agent skills for egg", + "scripts": { + "typecheck": "tsc --noEmit" + }, "keywords": [ "egg", "skill" ],As per coding guidelines
**/package.json: "All packages must include TypeScript type checking usingtsc --noEmitin a typecheck script".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/package.json` around lines 1 - 29, The package manifest is missing the required TypeScript typecheck script; add a "scripts" object (or extend it if present) in packages/skills/package.json and include a "typecheck" entry that runs "tsc --noEmit" (i.e., "typecheck": "tsc --noEmit") so the package exposes the mandatory type checking command referenced in the repo guidelines.packages/skills/egg-core/references/proto.md (1)
62-79:⚠️ Potential issue | 🟠 MajorCorrect the DI direction rule (
SingletonProtomust not injectContextProto).Line 62 and the example in Lines 74-78 invert the lifecycle rule.
@SingletonProtocannot depend on@ContextProto; only the reverse direction is safe.💡 Suggested doc fix
-**SingletonProto 可以注入 ContextProto**，框架会自动处理生命周期差异。 +**SingletonProto 不能注入 ContextProto**。生命周期更长的对象不能持有生命周期更短的对象。 -实现机制：框架不会直接注入 ContextProto 实例，而是通过 lazy getter/Proxy，在每次访问时从当前请求上下文动态解析，确保每个请求拿到各自的实例。 +正确方向是：`ContextProto` 可以注入 `SingletonProto`，或者通过上下文 API 获取请求级数据。 ... `@SingletonProto`() export class AppService { - `@Inject`() - requestContext: RequestContext; // ✅ 每次访问自动解析当前请求的实例 + // ✅ 不注入 ContextProto }Based on learnings "ContextProto objects can inject any prototype, but SingletonProto cannot inject ContextProto".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/egg-core/references/proto.md` around lines 62 - 79, The doc incorrectly states and demonstrates that `@SingletonProto` can inject `@ContextProto`; update the paragraph to say SingletonProto must not depend on ContextProto and only ContextProto may inject SingletonProto, and replace the example so RequestContext is annotated with `@ContextProto` and injects AppService (or show AppService as `@SingletonProto` being injected into RequestContext via `@Inject`), referencing the symbols SingletonProto, ContextProto, Inject, RequestContext and AppService so readers see the allowed DI direction.packages/skills/egg-core/SKILL.md (1)
150-153:⚠️ Potential issue | 🟡 MinorReturn a value from
hello(): Promise<string>example.Line 150 declares
Promise<string>, but Lines 151-152 only log and don’t return. The example currently impliesundefinedand is type-incorrect.💡 Suggested doc fix
async hello(): Promise<string> { - this.logger.info(`[HelloService] ${this.fooService.hello()}`); + const msg = await this.fooService.hello(); + this.logger.info(`[HelloService] ${msg}`); + return msg; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/skills/egg-core/SKILL.md` around lines 150 - 153, The example method hello(): Promise<string> logs the result of this.fooService.hello() but never returns it, making the signature incorrect; update the HelloService.hello implementation to capture (and await if necessary) the value from this.fooService.hello(), log it via this.logger.info, and then return that string (e.g., const res = await this.fooService.hello(); this.logger.info(...); return res) so the Promise<string> contract is honored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/skills/egg-controller/references/http-controller.md`:
- Around line 44-72: The fenced decision-tree code block is missing a language
identifier (MD040); update the opening triple-backtick that precedes the "需要从
HTTP 请求获取什么信息？" block to include a language token (e.g., change ``` to ```text)
and keep the closing ``` unchanged so the block is recognized as a text code
fence; ensure the fence around the tree containing `@HTTPParam`, `@HTTPQuery`,
`@HTTPBody`, `@HTTPHeaders`, `@Cookies`, `@Request`, and `@Context` is updated.
In `@packages/skills/egg-controller/SKILL.md`:
- Around line 13-21: The fenced code block in SKILL.md is missing a language
identifier; update the opening fence from ``` to ```text so the block becomes a
labeled "text" code fence (the content beginning with "需要暴露什么接口/客户端协议？" and
listing HTTPController/Schedule/MCPController) which will satisfy MD040 and
preserve formatting.
In `@packages/skills/egg-core/references/inject.md`:
- Line 87: The markdown heading "#### 注入配置" jumps from an `##` parent and
triggers MD001; locate the heading line containing "#### 注入配置" and change its
level to `### 注入配置` (or insert an intermediate `###` section above if a
different structure is required) so the heading hierarchy no longer jumps.
In `@packages/skills/egg-core/references/module.md`:
- Line 52: Several fenced code blocks in
packages/skills/egg-core/references/module.md are missing language tags causing
MD040 failures; update each unlabeled triple-fence (the tree/example blocks
around the examples at the locations previously noted) to use the language tag
"text". Locate the unlabeled fences in module.md (the examples referenced around
lines 52, 83, 166, 178, 192) and add "text" after the opening ``` for each
fenced block so the markdown linter recognizes them.
In `@packages/skills/egg-core/SKILL.md`:
- Line 32: The markdown file SKILL.md contains unlabeled fenced code blocks (the
triple-backtick blocks used for directory-tree examples); add a language tag
(e.g., use ```text) to each of those fenced blocks so the opened fences at the
directory-tree examples are labeled and resolve MD040 for consistent rendering;
update both occurrences of the unlabeled ``` fences found in SKILL.md (the two
directory-tree code blocks) to include the language tag.
In `@packages/skills/egg/SKILL.md`:
- Around line 103-106: Close the unmatched quote in the step-3 recommendation
sentence in SKILL.md: find the sentence that begins with "如果你需要在特定控制器中使用它，请使用
`egg-controller` skill and add the missing closing quotation mark after the
backticked `egg-controller` reference so the quoted recommendation is properly
terminated and renders correctly.
In `@packages/skills/PLAN.md`:
- Around line 16-21: The plan file lists skill folders as "controller" and
"tegg-core" but the actual packages are named "egg-controller" and "egg-core",
which will break examples and AVAILABLE_SKILLS; update the plan (and the
examples at lines referenced) to use the canonical package names
"egg-controller" and "egg-core", adjust any routing fixture examples and the
AVAILABLE_SKILLS constant/array to reference those exact names, and ensure any
other occurrences (around lines 93-110 and 126-127) are similarly renamed to
keep names consistent with the package structure.
---
Duplicate comments:
In `@packages/skills/egg-controller/references/mcp-controller.md`:
- Around line 23-30: The two fenced blocks in mcp-controller.md (the ASCII tree
starting with "app/module-name/" and the other fenced block around lines 76-94)
are missing language identifiers; add a language tag (e.g., ```text) immediately
after each opening ``` to satisfy MD040 so both blocks are declared as fenced
code blocks with a language (ensure you update the ASCII tree block that
contains "PackageMCPController.ts" and "PackageHTTPController.ts" and the
separate block referenced at 76-94).
In `@packages/skills/egg-core/references/proto.md`:
- Around line 62-79: The doc incorrectly states and demonstrates that
`@SingletonProto` can inject `@ContextProto`; update the paragraph to say
SingletonProto must not depend on ContextProto and only ContextProto may inject
SingletonProto, and replace the example so RequestContext is annotated with
`@ContextProto` and injects AppService (or show AppService as `@SingletonProto`
being injected into RequestContext via `@Inject`), referencing the symbols
SingletonProto, ContextProto, Inject, RequestContext and AppService so readers
see the allowed DI direction.
In `@packages/skills/egg-core/SKILL.md`:
- Around line 150-153: The example method hello(): Promise<string> logs the
result of this.fooService.hello() but never returns it, making the signature
incorrect; update the HelloService.hello implementation to capture (and await if
necessary) the value from this.fooService.hello(), log it via this.logger.info,
and then return that string (e.g., const res = await this.fooService.hello();
this.logger.info(...); return res) so the Promise<string> contract is honored.
In `@packages/skills/eval/evals-egg-core.json`:
- Around line 51-54: The eval entry with id 8 is wrong: it asserts
"SingletonProto 能注入 ContextProto" when the framework behavior is the opposite.
Update the expected_output for the eval entry (id 8) so it states that
SingletonProto cannot inject ContextProto and explain that ContextProto can be
injected into other prototypes but SingletonProto cannot access per-request
ContextProto (e.g., change the expected_output text to a short statement like
"不能注入，SingletonProto 无法注入 ContextProto；ContextProto 可以注入其他原型，但单例无法访问每次请求的上下文").
Ensure you modify the JSON entry that contains "id": 8 and "prompt":
"SingletonProto 能注入 ContextProto 吗？" and keep the rest of the object intact.
In `@packages/skills/package.json`:
- Around line 1-29: The package manifest is missing the required TypeScript
typecheck script; add a "scripts" object (or extend it if present) in
packages/skills/package.json and include a "typecheck" entry that runs "tsc
--noEmit" (i.e., "typecheck": "tsc --noEmit") so the package exposes the
mandatory type checking command referenced in the repo guidelines.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ff4bd68-f97f-4ecc-b1c7-db4c9e6dbf5c
📒 Files selected for processing (19)
CLAUDE.mdpackages/skills/PLAN.mdpackages/skills/README.mdpackages/skills/egg-controller/SKILL.mdpackages/skills/egg-controller/references/http-controller.mdpackages/skills/egg-controller/references/mcp-controller.mdpackages/skills/egg-controller/references/schedule.mdpackages/skills/egg-core/SKILL.mdpackages/skills/egg-core/references/dynamic-inject.mdpackages/skills/egg-core/references/inject.mdpackages/skills/egg-core/references/module.mdpackages/skills/egg-core/references/proto.mdpackages/skills/egg/SKILL.mdpackages/skills/eval/.gitignorepackages/skills/eval/evals-egg-controller.jsonpackages/skills/eval/evals-egg-core.jsonpackages/skills/eval/evals-routing.jsonpackages/skills/package.jsontsdown.config.ts
✅ Files skipped from review due to trivial changes (2)
- packages/skills/README.md
- packages/skills/egg-core/references/dynamic-inject.md
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/skills/eval/evals-routing.json
- tsdown.config.ts
- packages/skills/eval/.gitignore
- packages/skills/egg-controller/references/schedule.md
- CLAUDE.md
Comment on lines
+44
to
+72
| ``` | ||
| 需要从 HTTP 请求获取什么信息？ | ||
|
|
||
| ├─ URL 路径参数 | ||
| │ └─ → @HTTPParam() | ||
| │ | ||
| ├─ URL 查询参数 | ||
| │ ├─ 单个值 → @HTTPQuery() | ||
| │ └─ 多个值（数组） → @HTTPQueries() | ||
| │ | ||
| ├─ 请求体（POST/PUT） | ||
| │ └─ → @HTTPBody() | ||
| │ ├─ json → 对象 | ||
| │ ├─ text → 字符串 | ||
| │ | ||
| ├─ 请求头 | ||
| │ └─ → @HTTPHeaders() | ||
| │ └─ 注意：key 自动转小写 | ||
| │ | ||
| ├─ Cookie | ||
| │ └─ → @Cookies() | ||
| │ | ||
| ├─ 原始 HTTP 请求对象 | ||
| │ └─ → @Request() | ||
| │ └─ 注意：不要和 @HTTPBody 一起消费请求体 | ||
| │ | ||
| └─ Egg Context（框架功能） | ||
| └─ → @Context() | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language identifier to the decision-tree fence.
The fenced block is missing a language (MD040).
🛠️ Minimal fix
-```
+```text
需要从 HTTP 请求获取什么信息？
...
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg-controller/references/http-controller.md` around lines 44
- 72, The fenced decision-tree code block is missing a language identifier
(MD040); update the opening triple-backtick that precedes the "需要从 HTTP
请求获取什么信息？" block to include a language token (e.g., change ``` to ```text) and
keep the closing ``` unchanged so the block is recognized as a text code fence;
ensure the fence around the tree containing `@HTTPParam`, `@HTTPQuery`, `@HTTPBody`,
`@HTTPHeaders`, `@Cookies`, `@Request`, and `@Context` is updated.
Comment on lines
+13
to
+21
| ``` | ||
| 需要暴露什么接口/客户端协议？ | ||
|
|
||
| 1. HTTP 接口？例如 HTML/JSON/SSR/SSE，可以使用 HTTPController，参考 `references/http-controller.md` | ||
|
|
||
| 2. 定时任务，可以使用 Schedule，参考 `references/schedule.md` | ||
|
|
||
| 3. MCP 接口，可以使用 MCPController，参考 `references/mcp-controller.md` | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to the decision-tree fenced block.
This fence is missing a language identifier (MD040).
🛠️ Minimal fix
-```
+```text
需要暴露什么接口/客户端协议？
...
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg-controller/SKILL.md` around lines 13 - 21, The fenced
code block in SKILL.md is missing a language identifier; update the opening
fence from ``` to ```text so the block becomes a labeled "text" code fence (the
content beginning with "需要暴露什么接口/客户端协议？" and listing
HTTPController/Schedule/MCPController) which will satisfy MD040 and preserve
formatting.
|
|
||
| 框架会自动遍历 `Application` 和 `Context` 对象的所有属性，均可通过 `@Inject()` 注入。 | ||
|
|
||
| #### 注入配置 |
Contributor
There was a problem hiding this comment.
Fix heading level jump to satisfy markdownlint.
Line 87 uses #### directly under an ## section. Change it to ### (or insert an intermediate ###) to avoid MD001 violations.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 87-87: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg-core/references/inject.md` at line 87, The markdown
heading "#### 注入配置" jumps from an `##` parent and triggers MD001; locate the
heading line containing "#### 注入配置" and change its level to `### 注入配置` (or
insert an intermediate `###` section above if a different structure is required)
so the heading hierarchy no longer jumps.
|
|
||
| `extraFilePattern` 使用 [globby](https://github.com/sindresorhus/globby) 语法，以 `!` 开头表示排除。 | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Label all unlabeled fenced blocks with a language.
Lines 52, 83, 166, 178, and 192 start fences without a language tag. Please use text for tree/examples to fix MD040 consistently.
Also applies to: 83-83, 166-166, 178-178, 192-192
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 52-52: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg-core/references/module.md` at line 52, Several fenced
code blocks in packages/skills/egg-core/references/module.md are missing
language tags causing MD040 failures; update each unlabeled triple-fence (the
tree/example blocks around the examples at the locations previously noted) to
use the language tag "text". Locate the unlabeled fences in module.md (the
examples referenced around lines 52, 83, 166, 178, 192) and add "text" after the
opening ``` for each fenced block so the markdown linter recognizes them.
|
|
||
| #### 正确示例 | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add language tags to fenced directory-tree blocks.
Lines 32 and 42 open unlabeled code fences. Use a language tag like text to resolve MD040 and keep rendering/linting consistent.
Also applies to: 42-42
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg-core/SKILL.md` at line 32, The markdown file SKILL.md
contains unlabeled fenced code blocks (the triple-backtick blocks used for
directory-tree examples); add a language tag (e.g., use ```text) to each of
those fenced blocks so the opened fences at the directory-tree examples are
labeled and resolve MD040 for consistent rendering; update both occurrences of
the unlabeled ``` fences found in SKILL.md (the two directory-tree code blocks)
to include the language tag.
Comment on lines
+103
to
+106
| 1. 使用 `egg-core` skill | ||
| 2. 解释概念（例如，`AccessLevel.PUBLIC`） | ||
| 3. 核心解释后，建议："如果你需要在特定控制器中使用它，请使用 `egg-controller` skill | ||
|
|
Contributor
There was a problem hiding this comment.
Fix the unmatched quote in the step-3 recommendation sentence.
The quoted recommendation is not properly closed, which hurts readability.
🛠️ Minimal fix
-3. 核心解释后，建议："如果你需要在特定控制器中使用它，请使用 `egg-controller` skill
+3. 核心解释后，建议：“如果你需要在特定控制器中使用它，请使用 `egg-controller` skill。”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/egg/SKILL.md` around lines 103 - 106, Close the unmatched
quote in the step-3 recommendation sentence in SKILL.md: find the sentence that
begins with "如果你需要在特定控制器中使用它，请使用 `egg-controller` skill and add the missing
closing quotation mark after the backticked `egg-controller` reference so the
quoted recommendation is properly terminated and renders correctly.
Comment on lines
+16
to
+21
| ``` | ||
| packages/skills/ | ||
| ├── egg/ | ||
| ├── controller/ | ||
| ├── tegg-core/ | ||
| ├── eval/ # 新增：评测目录 |
Contributor
There was a problem hiding this comment.
Unify skill names with the package structure used in this PR.
The plan still uses controller / tegg-core, but the actual skills here are egg-controller / egg-core. This mismatch will propagate wrong examples for routing fixtures and AVAILABLE_SKILLS.
🛠️ Suggested alignment
-├── controller/
-├── tegg-core/
+├── egg-controller/
+├── egg-core/- expectedSkill: 'controller',
+ expectedSkill: 'egg-controller',
...
- expectedSkill: 'tegg-core',
+ expectedSkill: 'egg-core',
...
-const AVAILABLE_SKILLS = ['controller', 'tegg-core'];
+const AVAILABLE_SKILLS = ['egg-controller', 'egg-core'];Also applies to: 93-110, 126-127
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/skills/PLAN.md` around lines 16 - 21, The plan file lists skill
folders as "controller" and "tegg-core" but the actual packages are named
"egg-controller" and "egg-core", which will break examples and AVAILABLE_SKILLS;
update the plan (and the examples at lines referenced) to use the canonical
package names "egg-controller" and "egg-core", adjust any routing fixture
examples and the AVAILABLE_SKILLS constant/array to reference those exact names,
and ensure any other occurrences (around lines 93-110 and 126-127) are similarly
renamed to keep names consistent with the package structure.
This was referenced Mar 17, 2026
Open
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit